Skip to content

feat(spec)!: refuse excludeSubmitter: true beside onEmptyApprovers: 'auto_approve' on an approval node - #25700

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-25635-exclude-submitter-auto-approve-refusal
Oct 24, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-25635-exclude-submitter-auto-approve-refusal

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #25635

Clause-②: no (narrowing)
Metadata-protocol: narrowed
Ruling-ref: 6386627950

What changes

An approval node's config now refuses excludeSubmitter: true beside onEmptyApprovers: 'auto_approve' at the parse. This is letter A of ruling 6386627950 (batch #12 item 1, maintainer 「25635 25647 25675 同意」):

A = 发布时拒绝 excludeSubmitter: true 与 onEmptyApprovers: 'auto_approve' 的组合:审批节点多一条配对校验(形如 checkFallbackApproversPairing),提示改用 admin_rescue / fail / fallback。让裁决 6377842908 的「绝不静默放行」在契约里结构性成立。

  • packages/spec/src/automation/approval.zod.ts: a new refinement, checkExcludeSubmitterPolicyPairing, follows the shape of checkFallbackApproversPairing and is wired the same way, as a second .superRefine on ApprovalNodeConfigSchema. It raises one custom issue when excludeSubmitter === true and onEmptyApprovers === 'auto_approve', and nothing otherwise.
  • The issue sits on onEmptyApprovers, not on excludeSubmitter. The ruled fix is to choose a different policy, so onEmptyApprovers is the key the author has to edit. The message names both keys and offers the three ruled policies: 'admin_rescue' (the default), 'fail', or 'fallback' with fallbackApprovers. Unlike the fallback pairing, it does not offer "remove the other key" as an equal fix. Removing excludeSubmitter drops the control the author asked for, and an author or an AI told that either key will do usually makes the cheaper edit.
  • Only true triggers it. With excludeSubmitter set to false or left out, auto_approve keeps its meaning. Every other policy still parses beside true.
  • Describe and TSDoc. The excludeSubmitter describe said an emptied slate "never passes silently", but did not mention that auto_approve is refused beside it. That said less than the code now enforces. It gains one sentence naming the refused value and the three policies to choose instead. The [PLANNED — not yet enforced] marker stays, because the runtime exclusion (plugin-approvals: a field or position approver step has no "not the submitter" guard, so a manager who submits their own request can approve it #25573) is still to come. The auto_approve bullet in the onEmptyApprovers TSDoc now points at the new rule. content/docs/references/automation/approval.mdx is regenerated (gen:docs, one row).
  • No runtime change. plugin-approvals: a field or position approver step has no "not the submitter" guard, so a manager who submits their own request can approve it #25573 keeps its scope, and no other onEmptyApprovers value is touched.

Release-window reading (no published surface narrows)

The ruling requires this to merge before the next objectstack release. Reading taken 2026-10-24:

  • npm view @objectstack/spec: latest 18.1.0, next 18.0.0-next.5, rc 17.0.0-rc.6. In the 18.1.0 tarball (npm pack), excludeSubmitter appears in 0 files. Controls in the same tarball: onEmptyApprovers in 28 files, fallbackApprovers in 30.
  • packages/spec/CHANGELOG.md at 3cee74ecf0: 0 hits for excludeSubmitter. The key's own changeset, .changeset/25609-approval-exclude-submitter.md, is still pending, so it will ship in the same release as this one.
  • Producers: git grep excludeSubmitter at 3cee74ecf0 hits only the spec schema, its test, the generated reference, the authorable-surface shard, the liveness/flow.json note and the two changesets. No example, platform object, plugin or service writes it, so there is nothing to migrate. Control: onEmptyApprovers is written by examples/app-showcase flows.

Absent from every release, the refusal narrows nothing a published version accepted. The changeset is '@objectstack/spec': minor, because a narrowed kind takes at least minor. It carries the three declaration lines above, and its ADR-0087 marker reads not-required (no-migration-prescription) on those facts (check-adr-0087-registration exit 0, classifying it [clause-②-narrowing+metadata-protocol-narrowed]).

Consumers

  • plugin-approvals parses ApprovalNodeConfigSchema when it executes. A stored node with the pair would now fail at execute, but no producer writes the key, so no such row exists.
  • packages/lint and metadata-protocol: no code change. Their full suites are green at 3cee74ecf0 (below).
  • The flow doors pass the refusal through in the contract's own words. FlowSchema reports it at nodes.N.config.onEmptyApprovers, and the build judge flowNodeConfigRefusals reports it as node-config-refused-by-contract on onEmptyApprovers. Both are pinned in §6.
  • JSON Schema: a refinement has no projection, so automation/ApprovalNodeConfig keeps its existing dropped-refinements site "". That ledger is unchanged and check:generated is green.

Tests and gates (all readings at 3cee74ecf0 unless stated)

  • Test first. approval-exclude-submitter-25609.test.ts gains §6 and one §5 assertion. Run before the implementation: Tests 6 failed | 22 passed (28). The six were exactly the new refusal pins and the describe pin; the controls were already green.
  • Ablation, committed first. node scripts/ablation-replace.mjs --delete removed the wiring .superRefine(checkExcludeSubmitterPolicyPairing) (anchor 1 → 0, blob 8c310f2bea7b → a85bce9b6733). Predicted direction: the five §6 refusal pins go red and the controls stay green. Observed: Tests 5 failed | 23 passed (28). Restored: blob equals HEAD 8c310f2bea7b, git diff HEAD empty.
  • pnpm --filter @objectstack/spec test: Test Files 719 passed (719), Tests 21800 passed | 1 todo (21801).
  • pnpm --filter @objectstack/spec test:repo (at 4c615790bf, before the merge of origin/main): Test Files 79 passed (79), Tests 1269 passed (1269). Not re-run after the merge. CI's Test Core runs it on the merged head.
  • pnpm --filter @objectstack/spec typecheck: exit 0 (check:test-typecheck OK, ledger held).
  • Consumers, each a full suite: plugin-approvals 96 passed (96) / 1259 passed. lint 161 passed (161) / 7269 passed. metadata-protocol 263 passed | 3 skipped (266) / 21475 passed | 19 skipped. These packages have no test:repo script.
  • Full dogfood (owed by the (narrowing) arm and the ! title): pnpm --filter @objectstack/dogfood test under the lock, after building @objectstack/dogfood^.... Result: Test Files 295 passed | 3 skipped (298), Tests 2520 passed | 18 skipped (2538), VERDICT command-exit 0.
  • pnpm --filter @objectstack/spec check:generated after the merge: all 14 artifacts up to date.
  • node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 113 commands. All 113 exit 0. Four needed a re-run: three ran while the dogfood closure build was rewriting dist/ (check:exported-any 3, check:generated 1, check:dual-build-cjs-loads 1), and check:type-check-debt hit a lock queue-timeout (3). Each re-run after the build exited 0. --ran reconciliation: 113 derived famil(ies) accounted for — 113 run, 0 NOT-MEASURED (a DERIVED zero — all 113 recorded an exit code and none of them is 3).
  • node scripts/docs-audit/affected-docs.mjs --json origin/main named 7 docs. Four are hand-written (automation/approvals.mdx, automation/flows.mdx, capabilities/approvals.mdx, getting-started/common-patterns.mdx) and three are release-owned (not touched). None of the hand-written pages mentions excludeSubmitter, and their auto_approve sentences stay true, so no hand-written sentence is made false.

Acceptance notes

  • Lint hint, noted rather than filed. The approval-expression-no-empty-policy hint lists 'auto_approve' among the policies to declare, whatever excludeSubmitter says. An author with excludeSubmitter: true and an expression approver who follows it verbatim is refused at once by this rule, with the prescription. That is loud, and nobody writes the key today. carrier: none.
  • Boundary, noted rather than filed. SLA escalation.action: 'auto_approve' is a different key, a timed policy that notifies the submitter (approval.sla_breached). The ruling covers onEmptyApprovers only, so it is untouched. carrier: none.
  • Declaration lines. The card's own "After the ruling" text anticipated Clause-②: no for a pre-release landing. The PM's claim carries Clause-②: no (narrowing), and those lines are copied verbatim as the claim prescribes. Metadata-protocol: narrowed alone already classifies the changeset as breaking for the ADR-0087 gate, so the arm changes no gate verdict here. The release-window reading above is the evidence that no published surface narrows.
  • The window: this must merge before the next objectstack release, or it becomes a narrowing of a published key that needs a fresh ADR-0087 disposition.

Generated by Claude Code

…auto_approve' on an approval node

An approval node that removes the submitter from its slate and auto-approves
an emptied slate would approve a self-submitted request whose submitter was
the only approver. The pair is now refused at the parse with one issue at
onEmptyApprovers, prescribing 'admin_rescue', 'fail' or 'fallback'
(ruling 6386627950, letter A). The key ships in no release yet.

Claude-Session: https://claude.ai/code/session_01PC4NPXPgPmpDRG9DdDnyXT
Co-authored-by: Claude <noreply@anthropic.com>
…for the refused pair

Claude-Session: https://claude.ai/code/session_01PC4NPXPgPmpDRG9DdDnyXT
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface labels Oct 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 5 documentable anchor(s).

4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/approvals.mdx (via admin_rescue (literal, a string literal in checkExcludeSubmitterPolicyPairing), auto_approve (literal, a string literal in a comment in ApprovalNodeConfigSchema; a string literal in a comment on a changed line; a string literal in checkExcludeSubmitterPolicyPairing), onEmptyApprovers (literal, a string literal in checkExcludeSubmitterPolicyPairing))
  • content/docs/automation/flows.mdx (via auto_approve (literal, a string literal in a comment in ApprovalNodeConfigSchema; a string literal in a comment on a changed line; a string literal in checkExcludeSubmitterPolicyPairing))
  • content/docs/capabilities/approvals.mdx (via onEmptyApprovers (literal, a string literal in checkExcludeSubmitterPolicyPairing))
  • content/docs/getting-started/common-patterns.mdx (via auto_approve (literal, a string literal in a comment in ApprovalNodeConfigSchema; a string literal in a comment on a changed line; a string literal in checkExcludeSubmitterPolicyPairing))

⛔ 3 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-0.mdx (via admin_rescue (literal, a string literal in checkExcludeSubmitterPolicyPairing), auto_approve (literal, a string literal in a comment in ApprovalNodeConfigSchema; a string literal in a comment on a changed line; a string literal in checkExcludeSubmitterPolicyPairing), onEmptyApprovers (literal, a string literal in checkExcludeSubmitterPolicyPairing))
  • content/docs/releases/v17/17-5.mdx (via onEmptyApprovers (literal, a string literal in checkExcludeSubmitterPolicyPairing))
  • content/docs/releases/v17/17-7.mdx (via ApprovalNodeConfigSchema (symbol, a top-level const))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 144 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json b77bb37eb2c738c40093acb04f19fa8fc2dd605a → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 1aad63f8d0623425884ca79a4ee1c8306e4ed4cf — the merge of head 3cee74ecf009d339aaf97ba19c83c061df7e0414 into base b77bb37eb2c738c40093acb04f19fa8fc2dd605a, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 1aad63f8d0623425884ca79a4ee1c8306e4ed4cf && git checkout 1aad63f8d0623425884ca79a4ee1c8306e4ed4cf
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin b77bb37eb2c738c40093acb04f19fa8fc2dd605a 3cee74ecf009d339aaf97ba19c83c061df7e0414 && git checkout -B drift-repro b77bb37eb2c738c40093acb04f19fa8fc2dd605a && git merge --no-ff 3cee74ecf009d339aaf97ba19c83c061df7e0414

node scripts/docs-audit/affected-docs.mjs --json b77bb37eb2c738c40093acb04f19fa8fc2dd605a

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs b77bb37eb2c738c40093acb04f19fa8fc2dd605a → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

ACCEPT · #25635 · domain:spec seat 2 (seat post #18549) · sunhaolin · session session_01PC4NPXPgPmpDRG9DdDnyXT · 2026-10-24T18:31Z · head 3cee74ecf0 · report 6387948614 (on #25635; os-dev subagent of this session) · seat review at TIER_DEFAULT. The path limb hits (packages/spec/src/automation/approval.zod.ts), and the change is a narrowing, so a same-shape ## Contract review PASS at CONTRACT_REVIEW_TIER on the landing head is owed. needs:contract-review is added in this stroke. Release window: this must merge before the next objectstack release, so the review is asked for first.

  1. Shape. Draft, base main, first line Fixes #25635. Clause-②: no (narrowing), Metadata-protocol: narrowed and Ruling-ref: 6386627950 are line-initial in the body and the changeset (@objectstack/spec minor, ADR-0087 not-required (no-migration-prescription) with its reasons). 4 files, +172/−7: approval.zod.ts, its test file, the regenerated references/automation/approval.mdx row, and the changeset. Not governed. origin/main merged once through os-regen-merge.sh, clean.
  2. The ruling, measured on the diff. Letter A: checkExcludeSubmitterPolicyPairing is a second .superRefine on ApprovalNodeConfigSchema, chained after checkFallbackApproversPairing, in its shape. It raises one custom issue at onEmptyApprovers only when excludeSubmitter === true and onEmptyApprovers === 'auto_approve'. The message names both keys and prescribes admin_rescue / fail / fallback with fallbackApprovers. It deliberately does not offer dropping excludeSubmitter, which would drop the control. false or an omitted key leaves auto_approve its meaning. The excludeSubmitter describe gains one sentence naming the refused value; the [PLANNED] marker stays.
  3. Changeset, checked line by line.
    • "Narrows nothing a released version accepted": the report's reading of the npm @objectstack/spec 18.1.0 tarball, 0 files with excludeSubmitter (controls onEmptyApprovers 28, fallbackApprovers 30), and the key's own changeset still pending.
    • "Only true arms it" and "every other policy parses": the guard's two strict equalities.
    • "The approval engine is unchanged": no plugin-approvals file in the diff.
  4. Gates. Check-runs on 3cee74ecf0 at this record: 39; 22 completed with none failed, 17 still running. Local, per the report:
    • 113/113 derived, --ran DERIVED zero. Four needed a re-run (three during a concurrent dist rebuild, one lock timeout); all exited 0.
    • spec 719 files / 21800 tests. Consumers in full: plugin-approvals 96 / 1259, lint 161 / 7269, metadata-protocol 263 / 21475.
    • spec test:repo 79 / 1269, before the merge. The dogfood battery is 295 files, green.
    • TDD red: 6 failed before the implementation. Ablation on the .superRefine anchor: 5 red, restored blob-identical.
    • Landing waits for every check green and the contract-review PASS.
  5. Deviations and the open question.
    • (a) test:repo was run once, before the merge; CI Test Core owns the post-merge run. The named consumers declare no test:repo. Accepted.
    • (b) The declaration arm: the dev asks whether Clause-②: no (narrowing) should stay, though no published accept set narrows. Stays (A): the claim set it, Metadata-protocol: narrowed already makes the gate classify the change identically, and the changeset says in words that nothing published narrows. The contract review may say otherwise.
    • mcp_calls 0; relay writes 2.
  6. Out-of-scope outcomes. Two existing prescriptions still list 'auto_approve' among the policies to choose: the lint hint of approval-expression-no-empty-policy (validate-approval-approvers.ts about :625, the dev's finding), and, in the seat's own reading, checkFallbackApproversPairing's message (approval.zod.ts about :814). An author with excludeSubmitter: true who follows either is refused at once by the new arm, with its prescription. That is loud, not silent, and no producer writes excludeSubmitter: Acceptance notes, no carrier. SLA escalation.action: 'auto_approve' is a different key, outside the ruling.
  7. Landing. Not governed, 179 changed lines. Once a CONTRACT_REVIEW_TIER PASS is on record and every check on the then-current head is green, the seat sends pr_ready + automerge_enable through the relay.

Generated by Claude Code

Copy link
Copy Markdown
Collaborator

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 3cee74ecf009d339aaf97ba19c83c061df7e0414
Local-runs: none
Scope: packages/spec/src/automation/approval.zod.ts + packages/spec/src/automation/approval-exclude-submitter-25609.test.ts, packages/spec/src/automation/flow-node-config-refusals.ts, packages/spec/src/automation/flow.zod.ts (prose faces .changeset/25635-exclude-submitter-auto-approve.md, content/docs/references/automation/approval.mdx read, not scope)

① Derived judgments

  • Declaration is right. Clause-②: no (narrowing) + Metadata-protocol: narrowed: an approval node config that previously parsed (excludeSubmitter: true with onEmptyApprovers: 'auto_approve') is now refused at the parse. That is a served-set narrowing of a metadata-protocol key, so both lines are owed and both are present. Ruling-ref: 6386627950 matches batch Add comprehensive test suite for Zod schema validation #12 item 1 letter A (maintainer 「25635 25647 25675 同意」).
  • Implementation matches the ruling letter. checkExcludeSubmitterPolicyPairing raises exactly one custom issue when excludeSubmitter === true && onEmptyApprovers === 'auto_approve' and nothing otherwise; it is chained as a second .superRefine after checkFallbackApproversPairing, so the two pairings are independent (pinned in §6). false / absent excludeSubmitter keeps auto_approve legal; every other policy still parses beside true. The ruling asked for 「多一条配对校验(形如 checkFallbackApproversPairing),提示改用 admin_rescue / fail / fallback」: the message names both keys and offers exactly those three policies.
  • Issue placement on onEmptyApprovers is accepted. The ruled fix is to change the policy, so that is the key the author edits. The PR's reason for not offering "remove excludeSubmitter" as an equal fix is sound (the cheaper edit drops the control the author asked for).
  • Doors carry the refusal in the contract's own words. FlowSchema reports it at nodes.N.config.onEmptyApprovers; the build judge flowNodeConfigRefusals reports node-config-refused-by-contract on the same key. Both pinned. No door-side rewording, no second copy of the rule.
  • Describe/TSDoc now say what the code enforces. The excludeSubmitter describe gains the refused value and the three alternatives; [PLANNED — not yet enforced] stays, which is correct because the runtime exclusion (plugin-approvals: a field or position approver step has no "not the submitter" guard, so a manager who submits their own request can approve it #25573) is still open. approval.mdx is the regenerated row only.
  • No published surface narrows. The PR's release-window reading (18.1.0 tarball: 0 hits for excludeSubmitter; CHANGELOG 0 hits; the key's own changeset .changeset/25609-approval-exclude-submitter.md still pending) is consistent with the card and ruling history: spec(automation): an approval step's excludeSubmitter option, default off — the protocol half of #25573 (ruling 6377731715, letter B) #25609 landed after 18.1.0 (2026-10-24 09:18Z). So the refusal and the key ship in the same release. ADR-0087 marker not-required (no-migration-prescription) is justified on those facts.
  • Window. This must merge before the next objectstack release. If a release cuts first, the changeset needs a fresh ADR-0087 disposition and the declaration stays narrowed but the no-migration-prescription reading no longer holds. Spec seat: land it ahead of the next release train.

② Semver level

'@objectstack/spec': minor is correct: Metadata-protocol: narrowed takes at least minor (AGENTS.md step 3), and nothing here reaches major. Title carries ! and the changeset carries the three declaration lines. All 47 check runs on head 3cee74ecf0 are success or skipped (Test Core 1–6/6, Build Core, Dogfood Regression Gate, Type Check ×5, Lint ×9, Check Changeset, Governed Surface Queue Guard, claim gates); none red, none pending.

③ Boundary flags

Implemented-by: claude/issue-25635-exclude-submitter-auto-approve-refusal
Reviewed-by: session_01Bf4vhia9fHNXpqC1PCfxie

VERDICT: PASS

needs:contract-review removed in this stroke.


Generated by Claude Code

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/m tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Projects

None yet

3 participants