Skip to content

docs(automation): say when the fallbackApprovers pairing is actually enforced - #18868

Merged
os-try-charles merged 1 commit into
mainfrom
claude/issue-18551-approvals-refusal-timing
Sep 18, 2026
Merged

os-try-charles merged 1 commit into
mainfrom
claude/issue-18551-approvals-refusal-timing

Conversation

@os-try-charles

@os-try-charles os-try-charles commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Fixes #18551

The correction

content/docs/automation/approvals.mdx said the onEmptyApprovers: 'fallback' / fallbackApprovers pairing is "refused at authoring time". The pairing is enforced, in both directions — but not at authoring time.

Re-measured on this branch's base, objectstack-ai/objectstack @ a09d79708 (the card and the dispatch measured 18cc3b1df; main moved, and every leg still holds):

  • The only non-test parser of ApprovalNodeConfigSchema across packages, apps and examples is packages/plugins/plugin-approvals/src/approval-node.ts:133ApprovalNodeConfigSchema.safeParse(node.config ?? {}), the first statement of the executor's execute(). Every other hit in those trees is the schema's own definition, a test, a doc comment or a generated artifact.
  • Both arms live in checkFallbackApproversPairing (packages/spec/src/automation/approval.zod.ts:728), attached with .superRefine(...) at approval.zod.ts:947.
  • os lint has no pairing arm for it. packages/lint/src/validate-approval-approvers.ts declares ten rule ids and none of them pairs the two keys; its fallbackApprovers mentions are remedy prose inside other rules' messages, and its own header records that the rule "reads SHAPE".

So the refusal fires at flow execution, when the approval node parses its config at node entry.

Why the old wording misled rather than merely being imprecise

The same page uses "authoring time" three other times, and every one of them correctly means os lint, or its correctly-stated absence:

  • :25 — "os lint rejects the shapes it can prove at authoring time"
  • :27 — "Nothing can flag that at authoring time"
  • :102 — "os lint reports the shape at authoring time"

A reader arriving at the fallback paragraph from those expects os lint to catch a mis-paired fallback. It does not.

What the replacement keeps

This is a precision fix, not a hole. All three properties of the enforcement are real and survive the rewrite:

  • The refusal is loud — the executor returns success: false with Approval node '...' has invalid config: ... built from the issue paths and messages.
  • It names both keys — both messages in checkFallbackApproversPairing name onEmptyApprovers and fallbackApprovers.
  • It lands before any approval request opens — the safeParse guard returns ahead of the service.openNodeRequest(...) call in the same function.

The card also warned against inheriting PR #18525's body framing ("degrades to admin_rescue"). The replacement sentence does not use it: through the executor a mis-paired node fails at the parse, and the admin_rescue degrade the next paragraph describes is the separate, correct case of a declared fallback list resolving empty at run time.

Scope

Arm (a) of the card only — reword the sentence. Arm (b) (a lint pairing arm) is not in this PR and is not dropped: the dispatching seat files it as its own card, which is why the card's own scope is fully delivered here. (The Fixes line at the top is the one and only closing binding in this body; this sentence deliberately carries none — see check:partof-closing-keyword RULE 3, which red-flagged an earlier wording that bound a second keyword to that number inside a sentence about what is not here.)

Nothing here touches packages/plugins/plugin-approvals/**, any schema, or os lint. :25, :27 and :102 are deliberately untouched — they are the card's evidence, not its target.

Verification

Gate families derived mechanically from the change set, not from a hand-written list: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack (exit 0), which reported the tree and commit it derived against and resolved 1 changed path to 39 runnable commands.

All 39 were run, each exit code captured before any pipe (cmd > log 2>&1; EXIT=$?):

  • 34 green.
  • 5 reported PREREQUISITE NOT MET on a fresh worktree (check:doc-formula-expressions, check:doc-security-posture, check:docs-transcript-drift at exit 3; check:docs and check:skill-examples at exit 1 with their own "nothing was measured" text). Each names an unbuilt @objectstack/lint / @objectstack/spec. Re-run after building those closures — results in a comment below.

Local scope readings, per the change set:

  • No package build closure is owed. The diff touches one path, content/docs/automation/approvals.mdx, which sits under no package directory — the nearest package.json above it is the private repo root.
  • No package test or typecheck is owed for the same reason: no workspace package is affected.
  • pnpm lint is not narrowed, it is inapplicable. .mdx is outside eslint's covered population: eslint.config.mjs names the extension zero times, and every files: entry is TS/JS only. Measured directly — pnpm exec eslint --no-inline-config --format json content/docs/automation/approvals.mdx exits 0 over 1 reported file with 0 errors and the single warning File ignored because no matching configuration was supplied.
  • Control characters: grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]' over the edited file returns no hits, beside a green check:nul-bytes.

Firing controls, read mechanically from the probed file itself rather than from a diff or any report's prose (a single-line grep was not used to establish absence — the scan is whole-file and wrap-tolerant):

  • refused at authoring time now occurs 0 times in the page.
  • authoring time still occurs at :25, :27 and :102, plus once inside the new sentence.

Changeset

skip-changeset. The criterion is whether anything published moves, and it was measured rather than assumed: no non-private package declares any content/docs path in its files[], and the edited file lies under no package directory at all. Positive control on the same scan — 70 of 70 non-private packages declare a dist path in files[] — so the scan does read files[]. apps/docs, which renders this tree, is private: true.

Acceptance notes

Noted, not filed — neither is a defect, and neither has a carrier:

  • The card and the dispatch both say the page uses "authoring time" "twice more". It is three times more: :27 ("Nothing can flag that at authoring time") is a third correct usage the card did not count. This strengthens the card's argument rather than weakening it, and :27 needs no change.
  • dispatch-gates reports 14 families that "apply once this card's changeset exists". With skip-changeset measured above, no changeset path is created, so those 14 stay out of scope here rather than being skipped.

Generated by Claude Code


Generated by Claude Code

`approvals.mdx` said `fallback` without a `fallbackApprovers` list is
"refused at authoring time". The pairing is real and enforced in both
directions, but not at authoring time: the only non-test parser of
`ApprovalNodeConfigSchema` is the executor
(`plugin-approvals/src/approval-node.ts`), which `safeParse`s the node
config at node entry, so both arms of `checkFallbackApproversPairing`
fire at flow execution. `os lint` has no pairing arm for this.

The page uses "authoring time" three other times (`:25`, `:27`, `:102`)
where it correctly means `os lint`, so a reader arriving at this
sentence expects `os lint` to catch a mis-paired fallback. It does not.

Reword to state the real timing while keeping what is true: the refusal
is loud, names both keys, and lands before the node opens any approval
request. Precision fix, not a hole.

Claude-Session: https://claude.ai/code/session_017ef78bLdybu3AffehKkhfk
Co-authored-by: Claude <noreply@anthropic.com>
@os-try-charles os-try-charles added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 18, 2026 — with Claude
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Sep 18, 2026
@os-try-charles
os-try-charles marked this pull request as ready for review September 18, 2026 03:11
@os-try-charles
os-try-charles added this pull request to the merge queue Sep 18, 2026
Merged via the queue into main with commit b0b5f31 Sep 18, 2026
44 of 45 checks passed
@os-try-charles
os-try-charles deleted the claude/issue-18551-approvals-refusal-timing branch September 18, 2026 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/xs skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants