Filed unassigned by the flight-6 skills sweep (PR #13808, card #13793). Out of that PR's scope — the defect is in packages/spec, not in skills/** — so it is recorded here instead of ridden along.
The disagreement
packages/spec/src/automation/approval.zod.ts:
minApprovals: z.number().int().min(1).optional()
.describe('Approvals required — total (quorum) or per group (per_group). Default 1'),
packages/plugins/plugin-approvals/src/approval-service.ts, isApprovalSatisfied:
if (behavior === 'quorum') {
const n = original.length || 1;
const need = Math.min(Math.max(1, config.minApprovals ?? n), n); // omitted => n, not 1
return approved.size >= need;
}
if (behavior === 'per_group') {
const perGroupNeed = Math.max(1, config.minApprovals ?? 1); // omitted => 1
...
}
The same method's own doc comment states the runtime behaviour correctly — "quorum — at least minApprovals distinct approvals (default = all)" — so the two readings sit four lines apart in one file, and the schema's .describe() is the one that disagrees with them both.
The key carries no Zod .default(), so nothing materialises a value at parse time and the ?? on each branch is the whole story: the effective default is per behaviour, not a single number.
Why it matters
.describe() on this surface is not a comment — it lands in the generated reference docs and in the JSON schema published on the Approval action descriptor's configSchema, which is what the Studio property form and registerFlow() config validation are built from. So an author (or an AI author) reading the reference for behavior: 'quorum' is told that omitting minApprovals means one approval, when it actually means every resolvable approver must sign off.
The failure is quiet and in the unsafe direction: the node does not error, it simply waits for N approvals instead of 1, and the request looks "stuck" rather than misconfigured. quorum is also the one behaviour minApprovals exists for, so this is the default an author is most likely to lean on.
Suggested shape (not a decision — needs triage)
Two readings, and they are not equivalent:
- The text is wrong — correct the
.describe() to state the per-behaviour default ("omitted: all resolvable approvers under quorum, 1 per group"). Cheapest, changes no behaviour, and makes declared match enforced.
- The runtime is wrong — give the key a real
.default(1) so declared and enforced converge on the documented number. This CHANGES approval behaviour for every stored flow that omits the key under quorum (from unanimous-in-effect to first-approval), so it is a semantic migration, not a text fix.
Reading 1 is the one this filing recommends: the runtime's ?? n is deliberate (its doc comment names it, and the clamp beside it exists to stop a mis-set value deadlocking a request), so the text is the half that drifted. Reading 2 should not be taken without a maintainer ruling.
Related, already handled: the same wrong default had propagated into the published skill and is corrected in PR #13808 (skills/objectstack-automation/SKILL.md, the minApprovals node-config row). That correction is text-only and independent of whichever reading wins here.
Generated by Claude Code
Filed unassigned by the flight-6 skills sweep (PR #13808, card #13793). Out of that PR's scope — the defect is in
packages/spec, not inskills/**— so it is recorded here instead of ridden along.The disagreement
packages/spec/src/automation/approval.zod.ts:packages/plugins/plugin-approvals/src/approval-service.ts,isApprovalSatisfied:The same method's own doc comment states the runtime behaviour correctly — "
quorum— at leastminApprovalsdistinct approvals (default = all)" — so the two readings sit four lines apart in one file, and the schema's.describe()is the one that disagrees with them both.The key carries no Zod
.default(), so nothing materialises a value at parse time and the??on each branch is the whole story: the effective default is per behaviour, not a single number.Why it matters
.describe()on this surface is not a comment — it lands in the generated reference docs and in the JSON schema published on the Approval action descriptor'sconfigSchema, which is what the Studio property form andregisterFlow()config validation are built from. So an author (or an AI author) reading the reference forbehavior: 'quorum'is told that omittingminApprovalsmeans one approval, when it actually means every resolvable approver must sign off.The failure is quiet and in the unsafe direction: the node does not error, it simply waits for N approvals instead of 1, and the request looks "stuck" rather than misconfigured.
quorumis also the one behaviourminApprovalsexists for, so this is the default an author is most likely to lean on.Suggested shape (not a decision — needs triage)
Two readings, and they are not equivalent:
.describe()to state the per-behaviour default ("omitted: all resolvable approvers underquorum, 1 per group"). Cheapest, changes no behaviour, and makes declared match enforced..default(1)so declared and enforced converge on the documented number. This CHANGES approval behaviour for every stored flow that omits the key underquorum(from unanimous-in-effect to first-approval), so it is a semantic migration, not a text fix.Reading 1 is the one this filing recommends: the runtime's
?? nis deliberate (its doc comment names it, and the clamp beside it exists to stop a mis-set value deadlocking a request), so the text is the half that drifted. Reading 2 should not be taken without a maintainer ruling.Related, already handled: the same wrong default had propagated into the published skill and is corrected in PR #13808 (
skills/objectstack-automation/SKILL.md, theminApprovalsnode-config row). That correction is text-only and independent of whichever reading wins here.Generated by Claude Code