Skip to content

Commit 4ade442

Browse files
DylanMerigaudclaude
DylanMerigaud
and
claude
committed
Co-approvers per gate ("Also requires") + chat add/remove
A gate can route to several people, not just one. The primary approver (approverName) is unchanged; extra co-approvers live in a new optional approvers[] and approversOf(step) returns the full roster. - Engine names every approver on a gate (one Approve still clears it — the demo has one operator, so no quorum, nothing fake to defend). - Graph node shows a +N chip and the extra names; diff preview now reports a co-approver change instead of "unchanged". - Segregation-of-duties checks every approver, so the same person as a co-approver across gates on one path is caught. - Panel: an "Also requires" section (pills + add combobox). - Chat edits: add-approver / remove-approver ops (append/drop one person), both prompts teach them and now show who's on each gate; set-approvers stays the panel's replace-the-list op. Tests + free eval cases for every new op. Full verify gate passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 49ee76d commit 4ade442

12 files changed

Lines changed: 449 additions & 44 deletions

‎components/node-edit-panel.tsx‎

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -115,13 +115,20 @@ const ApprovalFields = ({
115115
}) => {
116116
const ordered = peopleFor(people, step.approverTitle);
117117
const unresolved = step.approverName === null;
118-
const personOptions: ComboboxOption[] = ordered.map((p) => ({
119-
value: p.name,
120-
label: p.name,
121-
sublabel: p.title || undefined,
122-
keywords: `${p.title} ${p.department}`,
123-
render: () => <PersonRow name={p.name} title={p.title} />,
124-
}));
118+
const optionsFor = (taken: string[]): ComboboxOption[] =>
119+
ordered
120+
.filter((p) => !taken.includes(p.name))
121+
.map((p) => ({
122+
value: p.name,
123+
label: p.name,
124+
sublabel: p.title || undefined,
125+
keywords: `${p.title} ${p.department}`,
126+
render: () => <PersonRow name={p.name} title={p.title} />,
127+
}));
128+
129+
const extras = step.approvers ?? [];
130+
const setExtras = (next: string[]) =>
131+
onApply({ op: "set-approvers", stepId: step.id, approvers: next });
125132

126133
return (
127134
<>
@@ -131,13 +138,47 @@ const ApprovalFields = ({
131138
onChange={(name) =>
132139
onApply({ op: "set-approver", stepId: step.id, approverName: name })
133140
}
134-
options={personOptions}
141+
options={optionsFor(extras)}
135142
placeholder={unresolved ? "⚠ Choose a person…" : "Choose a person…"}
136143
invalid={unresolved}
137144
testid="approver-combobox"
138145
/>
139146
</Field>
140147

148+
<Field label="Also requires">
149+
{extras.length > 0 && (
150+
<div className="mb-1.5 flex flex-wrap gap-1.5">
151+
{extras.map((name) => (
152+
<span
153+
key={name}
154+
data-testid={`extra-approver-${name}`}
155+
className="inline-flex items-center gap-1 rounded-full bg-accent-soft py-0.5 pl-2 pr-1 text-[11.5px] font-medium text-accent"
156+
>
157+
{name}
158+
<button
159+
type="button"
160+
aria-label={`Remove ${name}`}
161+
onClick={() => setExtras(extras.filter((n) => n !== name))}
162+
className="grid size-4 place-items-center rounded-full text-accent/70 hover:bg-accent/10 hover:text-accent"
163+
>
164+
×
165+
</button>
166+
</span>
167+
))}
168+
</div>
169+
)}
170+
<Combobox
171+
value=""
172+
onChange={(name) => name && setExtras([...extras, name])}
173+
options={optionsFor([
174+
...(step.approverName ? [step.approverName] : []),
175+
...extras,
176+
])}
177+
placeholder="Add another approver…"
178+
testid="add-approver-combobox"
179+
/>
180+
</Field>
181+
141182
<Field label="Triggers when">
142183
<ConditionEditor
143184
value={step.when}

‎components/workflow-graph.tsx‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,8 @@ const StepNode = ({ data }: NodeProps<Node<NodeData>>) => {
161161
const cb = changeBadge(change);
162162
const isApproval = step.kind === "approval";
163163
const condition = humanizeCondition(step.when);
164+
// Co-approvers beyond the primary (the panel's "Also requires").
165+
const extraApprovers = step.kind === "approval" ? (step.approvers ?? []) : [];
164166
const unconditional = step.when.kind === "always";
165167

166168
const badge = cb ?? st;
@@ -217,18 +219,30 @@ const StepNode = ({ data }: NodeProps<Node<NodeData>>) => {
217219
Approver
218220
</div>
219221
{step.approverName ? (
220-
<div className="mt-1 flex items-center gap-1.5">
221-
<span className="grid size-[18px] place-items-center rounded-full bg-accent-soft text-[8px] font-semibold uppercase text-accent">
222-
{step.approverName
223-
.split(" ")
224-
.map((p) => p[0])
225-
.slice(0, 2)
226-
.join("")}
227-
</span>
228-
<span className="truncate text-[12px] font-medium text-ink">
229-
{step.approverName}
230-
</span>
231-
</div>
222+
<>
223+
<div className="mt-1 flex items-center gap-1.5">
224+
<span className="grid size-[18px] place-items-center rounded-full bg-accent-soft text-[8px] font-semibold uppercase text-accent">
225+
{step.approverName
226+
.split(" ")
227+
.map((p) => p[0])
228+
.slice(0, 2)
229+
.join("")}
230+
</span>
231+
<span className="truncate text-[12px] font-medium text-ink">
232+
{step.approverName}
233+
</span>
234+
{extraApprovers.length > 0 && (
235+
<span className="shrink-0 rounded-full bg-accent-soft px-1.5 py-0.5 text-[10px] font-semibold text-accent">
236+
+{extraApprovers.length}
237+
</span>
238+
)}
239+
</div>
240+
{extraApprovers.length > 0 && (
241+
<div className="mt-1 truncate text-[11px] font-medium text-faint">
242+
with {extraApprovers.join(", ")}
243+
</div>
244+
)}
245+
</>
232246
) : (
233247
<div className="mt-1 text-[11.5px] font-medium text-warn">
234248
⚠ unresolved · {step.approverTitle}

‎eval/edit-agent-cases.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,4 +128,37 @@ export const AGENT_CASES: AgentCase[] = [
128128
},
129129
],
130130
},
131+
{
132+
id: "add-co-approver",
133+
instruction:
134+
"The director review should also need Jordan Ellis to sign off",
135+
minOps: 1,
136+
why: "a co-approver added to an existing gate; the result still validates clean",
137+
stub: [
138+
{
139+
op: "add-approver",
140+
stepId: "director-review",
141+
approverName: "Jordan Ellis",
142+
},
143+
],
144+
},
145+
{
146+
id: "add-then-remove-co-approver",
147+
instruction:
148+
"Add Jordan Ellis as a co-approver on the director review, then actually drop them again",
149+
minOps: 2,
150+
why: "add then remove a co-approver in one plan; the gate ends as it began and stays sound",
151+
stub: [
152+
{
153+
op: "add-approver",
154+
stepId: "director-review",
155+
approverName: "Jordan Ellis",
156+
},
157+
{
158+
op: "remove-approver",
159+
stepId: "director-review",
160+
approverName: "Jordan Ellis",
161+
},
162+
],
163+
},
131164
];

‎eval/edit-cases.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,28 @@ export const EDIT_CASES: EditCase[] = [
105105
/sam patel/i.test(op.approverName),
106106
why: "set the person on the existing IT gate",
107107
},
108+
{
109+
id: "add-director-co-approver",
110+
instruction:
111+
"The director review should also require Jordan Ellis to sign off",
112+
expectedOp: "add-approver",
113+
check: (op) =>
114+
op.op === "add-approver" &&
115+
op.stepId === "director-review" &&
116+
/jordan ellis/i.test(op.approverName),
117+
why: "ADD a co-approver to the director gate (keep Cameron Diaz), not replace the approver",
118+
},
119+
{
120+
id: "remove-director-co-approver",
121+
instruction:
122+
"Jordan Ellis no longer needs to sign off on the director review",
123+
expectedOp: "remove-approver",
124+
check: (op) =>
125+
op.op === "remove-approver" &&
126+
op.stepId === "director-review" &&
127+
/jordan ellis/i.test(op.approverName),
128+
why: "drop a CO-approver from the director gate, not remove the whole step or the primary",
129+
},
108130
{
109131
id: "drop-it",
110132
instruction: "Remove the IT review step entirely",

‎eval/edit-run.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,18 @@ const stubOp = (c: EditCase): WorkflowEditOp => {
123123
stepId: "it-review",
124124
approverName: "Sam Patel",
125125
};
126+
case "add-approver":
127+
return {
128+
op: "add-approver",
129+
stepId: "director-review",
130+
approverName: "Jordan Ellis",
131+
};
132+
case "remove-approver":
133+
return {
134+
op: "remove-approver",
135+
stepId: "director-review",
136+
approverName: "Jordan Ellis",
137+
};
126138
case "remove-step":
127139
return { op: "remove-step", stepId: "it-review" };
128140
case "none":

‎lib/approval-engine.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
evaluateCondition,
33
describeCondition,
4+
approversOf,
45
type ApprovalWorkflow,
56
type WorkflowStep,
67
type InvoiceContext,
@@ -142,17 +143,18 @@ export const executeWorkflow = (
142143
};
143144
}
144145

145-
// Approval step, condition true → look at the human decision.
146+
// Approval step, condition true → look at the human decision. `who` names every
147+
// approver on the gate (primary + extras), or the role if none is resolved yet.
146148
const decision = decisions[step.id];
149+
const who = approversOf(step).join(", ") || step.approverTitle;
147150
if (decision === "approve") {
148151
return {
149152
id: step.id,
150153
status: "approved",
151-
detail: `Approved by ${step.approverName ?? step.approverTitle}.`,
154+
detail: `Approved by ${who}.`,
152155
};
153156
}
154157
if (decision === "reject") {
155-
const who = step.approverName ?? step.approverTitle;
156158
const reason = reasons[step.id]?.trim();
157159
return {
158160
id: step.id,
@@ -165,7 +167,7 @@ export const executeWorkflow = (
165167
return {
166168
id: step.id,
167169
status: "pending",
168-
detail: `Awaiting ${step.approverName ?? step.approverTitle}${
170+
detail: `Awaiting ${who}${
169171
condText === "always" ? "" : ` (${condText})`
170172
}.`,
171173
};

‎lib/approval-workflow.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ import { test } from "node:test";
33

44
import {
55
ApprovalWorkflow,
6+
type ApprovalStep,
67
type Condition,
8+
approversOf,
79
evaluateCondition,
810
describeCondition,
911
humanizeCondition,
@@ -294,3 +296,37 @@ test("an unknown step kind is rejected by the schema", () => {
294296
};
295297
assert.throws(() => ApprovalWorkflow.parse(bad));
296298
});
299+
300+
/** A minimal approval step for the roster helper. */
301+
const gate = (over: Partial<ApprovalStep> = {}): ApprovalStep => ({
302+
id: "g",
303+
kind: "approval",
304+
label: "Gate",
305+
when: { kind: "always" },
306+
approverTitle: "Director",
307+
approverName: "Jordan Ellis",
308+
next: [],
309+
...over,
310+
});
311+
312+
test("approversOf: primary then the extras, in order", () => {
313+
assert.deepEqual(
314+
approversOf(gate({ approvers: ["Cameron Diaz", "Sam Patel"] })),
315+
["Jordan Ellis", "Cameron Diaz", "Sam Patel"],
316+
);
317+
});
318+
319+
test("approversOf: just the primary when there are no extras", () => {
320+
assert.deepEqual(approversOf(gate()), ["Jordan Ellis"]);
321+
});
322+
323+
test("approversOf: drops an unresolved primary but keeps the extras", () => {
324+
assert.deepEqual(
325+
approversOf(gate({ approverName: null, approvers: ["Cameron Diaz"] })),
326+
["Cameron Diaz"],
327+
);
328+
});
329+
330+
test("approversOf: empty when unresolved with no extras", () => {
331+
assert.deepEqual(approversOf(gate({ approverName: null })), []);
332+
});

‎lib/approval-workflow.ts‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,11 @@ export const Condition: z.ZodType<Condition> = z.lazy(() =>
106106

107107
/**
108108
* An approval step: a named human gate. `approverTitle` is the ROLE the agent
109-
* derived (e.g. "Director", "VP of IT"); `approverName` is the person the agent
110-
* resolved from the org chart, or null if it couldn't (which is itself something
111-
* the human resolves at validation time).
109+
* derived (e.g. "Director", "VP of IT"); `approverName` is the PRIMARY person the
110+
* agent resolved from the org chart, or null if it couldn't (which is itself
111+
* something the human resolves at validation time). `approvers` are ADDITIONAL
112+
* co-approvers a human added (the primary is never repeated there); the people on
113+
* the gate are `approversOf(step)`.
112114
*/
113115
/** @public — an approval gate in the workflow (rendered by the UI/canvas). */
114116
export const ApprovalStep = z
@@ -119,12 +121,22 @@ export const ApprovalStep = z
119121
when: Condition,
120122
approverTitle: z.string(),
121123
approverName: z.string().nullable(),
124+
/** Additional co-approvers beyond the primary (`approverName`). Optional so every
125+
existing gate/literal stays valid; absent means "just the primary". */
126+
approvers: z.array(z.string()).optional(),
122127
/** Ids of the steps that run after this one (parallel fan-out when >1). */
123128
next: z.array(z.string()),
124129
})
125130
.strict();
126131
export type ApprovalStep = z.infer<typeof ApprovalStep>;
127132

133+
/** @public — everyone who approves a gate: the primary (if resolved) then the extras,
134+
in order. Empty when the gate is unresolved with no extras. */
135+
export const approversOf = (step: ApprovalStep): string[] =>
136+
[step.approverName, ...(step.approvers ?? [])].filter(
137+
(n): n is string => !!n,
138+
);
139+
128140
/** @public — the system actions an integration step can run. */
129141
export const IntegrationKind = z.enum(["slack", "jira", "netsuite"]);
130142
export type IntegrationKind = z.infer<typeof IntegrationKind>;
@@ -391,8 +403,10 @@ const stepFieldDiffs = (a: WorkflowStep, b: WorkflowStep): string[] => {
391403
if (a.label !== b.label) fields.push("label");
392404
if (describeCondition(a.when) !== describeCondition(b.when))
393405
fields.push("condition");
394-
const aAppr = a.kind === "approval" ? a.approverName : null;
395-
const bAppr = b.kind === "approval" ? b.approverName : null;
406+
// "approver" covers the whole gate roster (primary + co-approvers), so adding or
407+
// dropping an "Also requires" person reads as a change, not "unchanged".
408+
const aAppr = a.kind === "approval" ? approversOf(a).join(",") : "";
409+
const bAppr = b.kind === "approval" ? approversOf(b).join(",") : "";
396410
if (aAppr !== bAppr) fields.push("approver");
397411
const aTitle = a.kind === "approval" ? a.approverTitle : null;
398412
const bTitle = b.kind === "approval" ? b.approverTitle : null;

0 commit comments

Comments
 (0)