Skip to content

feat(spec): an approval step's excludeSubmitter option, default off (protocol half of #25573) - #25632

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-25609-approval-exclude-submitter
Oct 24, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-25609-approval-exclude-submitter

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #25609

Clause-②: yes (widening)
Metadata-protocol: added
Ruling-ref: 6377731715

The protocol half of #25573: an approval step's excludeSubmitter option, default off. #25573 (the runtime half in plugin-approvals) remains open and is Blocked-by: this card.

The ruling this implements

Ruling 6377731715 on #25496 (batch #388 item 4, the maintainer's 「同意」), item 5, verbatim:

#25446 → B, default off. A step-level option excludeSubmitter: true removes the submitter from the resolved approver set; an emptied set takes the node's existing onEmptyApprovers fallback (else ADR-0042 escalation), never a silent pass; the approval record shows when the exclusion changed the route. Off, behaviour is unchanged. Priority p1 (protocol addition). Workload S–M.

The later ruling 6377842908 agrees on this row: 「B 步骤选项 excludeSubmitter,默认关;排除后无人可批按 ADR-0042 升级 … 默认关,不改现有审批行为;审批记录显示排除改了路由」.

What changed

  • packages/spec/src/automation/approval.zod.ts: ApprovalNodeConfigSchema gains excludeSubmitter: z.boolean().optional(), placed directly after approvers.
    • Where it lives. ADR-0019 made each approval STEP an approval NODE, so the step-level option is a node-config key. It applies to the whole slate approvers resolves to, whatever approver type produced each person. It is one switch per node: the approver row stays closed and refuses it.
    • Who the submitter is. The user whose action started the flow run. plugin-approvals hands the run's trigger user to openNodeRequest (approval-node.ts, runTriggerUser) and records it as sys_approval_request.submitter_id. The describe names submitter_id.
    • Off by default, and absent when off. I used .optional(), not .default(false). A node that does not author the key parses to exactly the output it parsed to before. So stored approval nodes, and the node_config_json snapshot each request row keeps, are unchanged.
    • The describe tells the truth today. It opens with [PLANNED — not yet enforced] and ends by telling the author that the approval engine does not read the key yet, so the submitter is NOT removed. It says to keep any existing guard (an expression approver or a validation rule) until the engine honours the key. Per check:doc-authoring Rule 3 it carries no tracker id. The carrier, 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 named in the TSDoc and in the ledger note.
  • packages/spec/liveness/flow.json: a PLANNED RECORD sentence in _note. Why it is a note and not a row is under Measured below.
  • Generated: the authorable-surface/automation.json row automation/ApprovalNodeConfig:excludeSubmitter, and content/docs/references/automation/approval.mdx. authorable-defaults/ is unchanged, because the key has no default.
  • Test: packages/spec/src/automation/approval-exclude-submitter-25609.test.ts (16 tests):
    • §1: the key parses as true and as false, stays absent when omitted, and a non-boolean is refused at the key's own path.
    • §2: FlowSchema accepts the key inside an approval node's config. This is the parse that os validate, compile and the metadata save door run.
    • §3: the same key on an approver row, on the flow node beside config, or on the flow is still refused, each at its own path.
    • §4: the published JSON schema has it as an optional boolean with no default.
    • §5: the describe opens with the [PLANNED marker and names submitter_id and onEmptyApprovers.
  • Changeset .changeset/25609-approval-exclude-submitter.md: @objectstack/spec minor, with both declaration lines.

Measured (premise and the PM's mechanism assumptions)

  • Premise holds on origin/main f7b0298968. git grep excludeSubmitter gives 0 hits. Today the key is refused at every placement: inside config (custom at nodes.1.config.excludeSubmitter), on the node (unrecognized_keys at nodes.1), on the flow (unrecognized_keys at the root) and on an approver row (custom at nodes.1.config.approvers.0.excludeSubmitter).
  • The liveness-ledger-row assumption is disproved. No row can carry a node-config key. The flow walk ends at nodes.config, which is z.record(z.string(), z.unknown()), and childShape() does not descend into it. I planted children: { excludeSubmitter: { status: 'planned', authorWarn: true, … } } under flow.json nodes.config and ran check:liveness. It exited 1 with ✗ 1 UNCLASSIFIED … flow/nodes.config (declared children but property is not a container). I restored the file and confirmed the blob hash matched HEAD.
    • The liveness lint reads no deeper than a container's direct children, so no authorWarn row could reach this key's authors either.
    • None of the existing approval-config keys (onEmptyApprovers, fallbackApprovers, …) has a row for the same reason.
    • So I followed the spec lane's own precedent for a key the walk cannot reach (liveness/page.json's PLANNED RECORD for a component-level key): a _note sentence naming the carrier, plus the [PLANNED — …] describe marker. The checkable record is the authorable-surface row.
    • With the key added, check:liveness is green.

Tests and proofs

Head: 4823c57936 (the spec suite and typecheck were read there; the ablations ran on the pre-merge commits named below and touch no merged file).

  • pnpm --filter @objectstack/spec test: Test Files 715 passed (715), Tests 21703 passed | 1 todo (21704).
  • pnpm --filter @objectstack/spec typecheck: exit 0, including check:test-typecheck, which compiles the new test file.
  • Ablation 1 (proves the tests catch the key going missing). Through scripts/ablation-replace.mjs, I renamed the declared key to excludeSubmitterAblated: anchor count 1 → 0, blob 27c795830a90 → 50ecb7c5d102. Result: Tests 10 failed | 6 passed (16). Every test that needs the key went red (§1 ×6, §2, §4, §5 ×2). The §2 control, the four §3 refusals and §1 "omitted stays absent" stayed green. The tool restored the file (blob == HEAD, git diff HEAD empty).
    • The first run of this ablation errored the whole file at collection, because §5 read the description at describe time. I moved that read inside each it (commit c6f43c7960) and re-ran the ablation for the per-test result above.
  • Ablation 2 (proves "absent when off"). I changed .optional() to .default(false). Result: Tests 2 failed | 14 passed: §1 "omitted stays absent" and §4 "no default" went red. Restored the same way.
  • No dist leg: the test imports ./approval.zod from src.

Gates

All of these were read at head 4823c57936, which carries one merge of origin/main (481ac62116) through scripts/pm/os-regen-merge.sh, with the spec rebuilt afterwards.

  • Derived families: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derives 114 commands from this diff. I ran every one and captured each exit code before any pipe:
    • 108 exited 0 on the first pass.
    • 6 first exited 3, PREREQUISITE NOT MET (no dist for lint, formula, client-react or objectql in a fresh worktree): check:doc-formula-expressions, check:doc-security-posture, check:skill-examples, check:docs-transcript-drift, check:dual-build-cjs-loads, check:lean-entry-closure. I built --filter=@objectstack/lint... --filter=@objectstack/client-react... --filter=@objectstack/objectql... (turbo, 37 tasks) and re-ran them; all 6 then exited 0 with measured verdicts. For example, ✓ check:dual-build-cjs-loads — 112 published require entry point(s) across 69 package(s) load and ✅ 262 prose examples type-check across 3 surface(s).
  • Reconciliation: dispatch-gates --ran answers ✓ dispatch-gates --ran: 114 derived famil(ies) accounted for — 114 run, 0 NOT-MEASURED (a DERIVED zero — all 114 recorded an exit code and none of them is 3).
  • pnpm --filter @objectstack/spec check:generated: ✓ All 14 generated artifacts are up to date. This needed gen:docs once, for references/automation/approval.mdx.
  • check:liveness: exit 0 with the key in place. With the planted row it exited 1 (see Measured).
  • Local-only family: pnpm check:dev-prereqs exit 0, @objectstack/spec … built from the sources on disk.
  • Not run locally, CI's: test:repo (the diff touches no repo-project test), and the repo-wide scans led by pnpm lint. CI check-runs on this PR are the record for every family CI runs.
  • Dogfood: n/a. The title carries no ! and Clause-② is (widening).

Acceptance notes


Generated by Claude Code

…col half)

Adds the step-level excludeSubmitter key to ApprovalNodeConfigSchema,
optional (absent when off), with a describe that opens with the planned
marker: the approval engine does not read it yet.

Claude-Session: https://claude.ai/code/session_01PC4NPXPgPmpDRG9DdDnyXT
Co-authored-by: Claude <noreply@anthropic.com>
…low ledger note

The key parses on the node config and stays absent when off; it is refused
on an approver row, on the flow node and on the flow. The ledger walk ends
at flow/nodes.config, so the planned record lives in flow.json's _note.

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 labels Oct 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 1 documentable anchor(s). ⚠️ 2 changed file(s) yielded no anchor (packages/spec/authorable-surface/automation.json, packages/spec/liveness/flow.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

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

  • 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
  • 2 changed file(s) yielded no anchor (packages/spec/authorable-surface/automation.json, packages/spec/liveness/flow.json) — pages documenting those are invisible to this run
  • 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 42d9fe8f5be01cf8822c324af6142103193199fe → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 98054b2ce24fd0165ad7aa140f9147370a3971e4 — the merge of head 4823c579361ad3bfd8025170edac4fabe1ab6a4c into base 42d9fe8f5be01cf8822c324af6142103193199fe, 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 98054b2ce24fd0165ad7aa140f9147370a3971e4 && git checkout 98054b2ce24fd0165ad7aa140f9147370a3971e4
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 42d9fe8f5be01cf8822c324af6142103193199fe 4823c579361ad3bfd8025170edac4fabe1ab6a4c && git checkout -B drift-repro 42d9fe8f5be01cf8822c324af6142103193199fe && git merge --no-ff 4823c579361ad3bfd8025170edac4fabe1ab6a4c

node scripts/docs-audit/affected-docs.mjs --json 42d9fe8f5be01cf8822c324af6142103193199fe

⚠️ 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 42d9fe8f5be01cf8822c324af6142103193199fe → 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 · #25609 · domain:spec seat 2 (seat post #18549) · sunhaolin · session session_01PC4NPXPgPmpDRG9DdDnyXT · 2026-10-24T11:42Z · head 4823c57936 · report 6383750916 · seat review at TIER_DEFAULT. Both clause-② limbs hit (path: packages/spec/src/automation/approval.zod.ts; declaration: Clause-②: yes), so a same-shape ## Contract review PASS at CONTRACT_REVIEW_TIER on the landing head is still owed. needs:contract-review is added in this stroke.

  1. Shape. Draft, base main, first line Fixes #25609, no other closing keyword. Clause-②: yes (widening), Metadata-protocol: added and Ruling-ref: 6377731715 are line-initial in the body; the changeset carries both declarations, with @objectstack/spec at minor. Six files, +259/−1 (get_files), all inside the claim's surface: approval.zod.ts, a new test file, the generated approval.mdx row and authorable-surface/automation.json row, the liveness/flow.json _note, and the changeset. No runtime file (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 that half), no governed path. origin/main was merged once (481ac62116) through os-regen-merge.sh.
  2. The ruling, measured on the diff. ApprovalNodeConfigSchema.excludeSubmitter: z.boolean().optional() sits on the node config (each approval step is a node, ADR-0019). It is absent when off, so every stored node and each request's config snapshot parse unchanged ("off, behaviour is unchanged" holds at the parse). The approver row and the flow node still refuse it as an unknown key. The describe opens with [PLANNED — not yet enforced], defines the submitter (submitter_id, the run's trigger user), sends an emptied slate to onEmptyApprovers ("never passes silently"), and tells the author the engine does not read it yet.
  3. Gates. Check-runs on 4823c57936 at this record: 23 success, 6 skipped, 11 in progress, 0 failed. Local, per the report: --ran 114/114 with no NOT MEASURED; spec 715 files / 21703 tests, typecheck clean; the new test file 16/16. Ablations: the key renamed, 10/16 red; .optional() → .default(false), 2/16 red; both restored. Landing waits for every check green and the contract-review PASS.
  4. Deviations accepted. (a) The PM's assumption of a planned ledger row was disproved by measurement: a row planted under flow/nodes.config makes check:liveness exit 1 ("declared children but property is not a container"), because the walk ends at a z.record(z.string(), z.unknown()). The record moved to a _note PLANNED RECORD sentence, following the page.json precedent, plus the describe marker and the authorable-surface row. Accepted. (b) A model-named trailer was amended to the model-free pair before any push. (c) gen:docs was run directly instead of check:generated --fix (then green). mcp_calls 0; relay writes 2.
  5. Open questions and out-of-scope outcomes.
    • excludeSubmitter: true beside onEmptyApprovers: 'auto_approve': an exclusion-emptied slate would be waved through, against the ruling's "never a silent pass". This is a contract-shape fork: A, the spec refuses the pair; B, the runtime overrides auto_approve; C, accept it. It goes to the maintainer in its own decision card (number below). It does not hold this PR: the key is unreleased, so adding a refusal before the next release narrows no published surface. The describe states the ruled invariant, which A or B makes true; under C the describe would change.
    • Does the exclusion also screen the fallbackApprovers slate and admin_rescue? A runtime choice inside the ruling ("the resolved approver set"), carried by 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, the runtime half. That card's seat answers it, and the describe widens when it lands.
    • The liveness walk reaches no approval node-config key at all (onEmptyApprovers, fallbackApprovers included): Acceptance notes (a coverage observation, recorded in the PR body's Measured section).
    • The header of ui/component-record-blocks-empty-state-24266.test.ts claims a gate reads its [PLANNED] marker that none does: dropped — a comment inaccuracy with no reader.
  6. Landing. Not governed, 260 changed lines. Once the 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. On merge, 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 (the runtime half) unblocks for its lane.

Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

The decision card named in 6383784354 item 5 is #25635 (needs-user-decision): excludeSubmitter: true beside onEmptyApprovers: 'auto_approve', with A (refuse the pair at publish) recommended. It does not hold this PR · domain:spec seat 2 · 2026-10-24T11:46Z.


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 4823c579361ad3bfd8025170edac4fabe1ab6a4c
Local-runs: none
Scope: packages/spec/src/automation/approval.zod.ts + packages/spec/liveness/flow.json, packages/spec/scripts/liveness/check-liveness.mts, packages/spec/src/automation/flow-node-config-refusals.ts, packages/plugins/plugin-approvals/src/approval-node.ts, packages/plugins/plugin-approvals/src/approval-service.ts, packages/plugins/plugin-approvals/src/sys-approval-request.object.ts, docs/adr/0019-approval-as-flow-node.md, docs/adr/0042-approval-sla-escalation.md, docs/adr/0049-no-unenforced-security-properties.md

On the scope: hits is the one schema file. The cited files are every file its hunks make a claim about, read in the code: the liveness ledger and the gate that reads it (pnpm --filter @objectstack/spec check:liveness is packages/spec/scripts/liveness/check-liveness.mts; no scripts/check-liveness* exists), the FlowSchema door that judges an approval node's config, the executor and service the describe's "who the submitter is" and "the engine does not read this key" sentences are about, and the three ADRs the TSDoc and the ruling lean on. The rest of the diff (the changeset, the regenerated authorable-surface/automation.json and references/automation/approval.mdx rows, the test file) is the lane seat's ACCEPT at TIER_DEFAULT, read here only where the schema hunk makes a claim about it.

Inputs: card #25609 (body; comments 6382817354, 6383187041, 6383750916), rulings 6377731715, 6377842908 and the reconciliation 6377965040 on #25496, runtime card #25573 (body; comments 6382188555, 6382833455), the PR body, its file list and the net diff against main at its head (merge-base 481ac6211641ccb7292508ed423f02e432322e4a = origin/main at read time; 6 files, +259/−1), and the check-runs on the head. The PR's own comment thread was not read before this verdict was formed.

① Derived judgments

Accept-set changes the diff implies, each judged:

  1. ApprovalNodeConfigSchema gains excludeSubmitter: z.boolean().optional() — one new optional key on the approval node's config. Three doors widen together through the one schema: the direct parse; the FlowSchema per-node door (flow-node-config-refusals.ts:174 registers ApprovalNodeConfigSchema for approval nodes, which is why the key is refused today as custom at nodes.N.config.excludeSubmitter); and the executor's own ApprovalNodeConfigSchema.safeParse(node.config ?? {}) (approval-node.ts:161). RIGHT — a widening; nothing that parsed before is refused.
  2. No default: .optional(), not .default(false). A node that omits the key parses byte-identical to before, so the node_config_json snapshot each request row keeps is unchanged, and authorable-defaults/ correctly carries no new row. RIGHT — pinned by §1 "omitted stays absent" and §4 "no default".
  3. The approver row (ApprovalNodeApproverSchema, a strictObject), the flow node beside config, and the flow root stay closed to the key — pinned by §3. RIGHT: one switch per node is the ruled "step-level option"; ADR-0019 D3 maps steps[] to successive approval nodes, so node config is the step.
  4. Public surface: no export added or removed (api-surface/ untouched is correct — it records export existence, not shape); ApprovalNodeConfig and ApprovalNodeConfigParsed gain one optional member; getApprovalNodeConfigJsonSchema() gains one optional boolean property under additionalProperties: false (pinned by §4). No exhaustive consumer of the key set exists on origin/main (only tests read individual .shape members), and objectui's FlowNodeInspector derives the approval node form from the engine-published configSchema, so no sibling hand-list goes stale. RIGHT.
  5. Generated artifacts: the authorable-surface/automation.json row automation/ApprovalNodeConfig:excludeSubmitter and the approval.mdx row carry exactly the describe; the TSDoc's tracker ids do not project. RIGHT — Type Check · source gates, which runs check:authorable-surface, check:docs and check:generated --reconcile-only, is green on this head.

The describe's statements, against the cited code:

  • "the user whose action started the flow run, recorded on the approval request as submitter_id" — TRUE today. approval-node.ts:196 passes submitterId: runTriggerUser(context); runTriggerUser (:107) returns the recorded triggerUserId and otherwise userId; approval-service.ts:3862 writes it as submitter_id; the column is sys-approval-request.object.ts:307.
  • "The approval engine does not read this key yet, so today the submitter is NOT removed" — TRUE. excludeSubmitter has 0 hits on origin/main; the executor parses the whole config (:161) and reads none of it. Correctly marked: the describe opens with [PLANNED — not yet enforced] and closes with this sentence.
  • "takes the node's onEmptyApprovers route (admin_rescue by default, or fallbackApprovers under 'fallback')" — PLANNED, and each named item is true of the enum (onEmptyApprovers defaults to admin_rescue; fallbackApprovers is paired by checkFallbackApproversPairing) and of the runtime routes (approval-service.ts:3664 to :3812). The parenthetical is not exhaustive — the enum also has fail and auto_approve. A precision point for the 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 rewrite, not a falsehood.
  • "never passes silently" — the ruling's letter (6377731715 item 5; 6377842908 「排除后无人可批按 ADR-0042 升级」), marked planned. It is NOT derivable from the cited code for one legal pairing: under onEmptyApprovers: 'auto_approve' an empty slate opens no request and completes down the approve edge (approval-service.ts:3674, approval-node.ts:221). As written the describe therefore binds 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 to make the invariant true (shape A or B in ③); it does not describe today's runtime and does not claim to.
  • "the approval record shows that the exclusion changed the route" — planned, the runtime half, consistent with both rulings.
  • The TSDoc's "ADR-0019 made each approval STEP an approval NODE" — TRUE (D1; the D3 re-home table).

The marker and the ledger:

  • [PLANNED — not yet enforced] is the liveness vocabulary's planned marker (check-liveness.mts:499, MARKER_RE.planned). ADR-0049 applies in substance — a segregation-of-duties key is a control boundary, and the ADR allows exactly enforced, marked-as-not-enforced, or absent — and this key is marked, with the describe saying the submitter is not removed today. ADR-0049's letter spells the marker [EXPERIMENTAL — not enforced]; the lane's precedents for a key with a named carrier (ui/app.zod.ts:422, ui/component.zod.ts:1175) use the PLANNED form, which is the more precise disposition here (a committed carrier, 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). RIGHT.
  • No ledger row is possible, as the _note claims: flow.json nodes.children.config is a leaf row (live, evidence in service-automation/src/engine.ts); FlowNodeSchema.config is z.record(z.string(), z.unknown()); childShape() (check-liveness.mts:662) returns shapeOf(unknown), which is null; drillChildren (:1067 to :1070) then reports declared children but property is not a container — the dev's measurement is what the code does. _note is free prose the gate never parses (no reference to it in check-liveness.mts). The PLANNED RECORD sentence follows liveness/page.json's _note precedent for keys the walk cannot reach. Spec property liveness is green on this head. RIGHT.
  • The window: once a runtime carries this spec, the designer offers the toggle with the PLANNED help text, and the key is carried unread until 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 lands. That is the order the ruling chose ("spec first, then plugin-approvals"); the describe's last sentence is the only author-facing guard, the hand-written Approvals guide is deliberately not updated, and the PR records the window for release timing. Accepted as ruled; it sets the clock on open question 1 below.

② Semver level

@objectstack/spec minor — RIGHT. Metadata-protocol: added (a new authorable key) takes at least minor on the package the diff moves. The PR's Clause-②: yes (widening) line is right: one optional key is added and nothing that parsed before is refused. Both declaration lines are present in the PR body and in .changeset/25609-approval-exclude-submitter.md; Check Changeset is green. No ADR-0087 disposition is owed (not breaking). No other published package moves — plugin-approvals is untouched and its behaviour on this head is unchanged. The changeset body carries no tracker id and no model identifier, and it tells the upgrading reader the key is declared ahead of the engine that honours it.

③ Boundary flags

Dev report 6383750916, each flag answered:

open_questions

  1. excludeSubmitter: true beside onEmptyApprovers: 'auto_approve' — ESCALATED TO THE MAINTAINER as a scoped follow-up to ruling 6377731715 item 5. The tension is in the ruling's own letter ("takes the node's existing onEmptyApprovers fallback … never a silent pass", while auto_approve IS the silent pass). My reading: shape A (a parse-time refinement in the checkFallbackApproversPairing style, refusing the pairing) makes the ruled invariant structurally true at the contract and keeps the runtime free of a policy override; shape B is acceptable; shape C contradicts the describe as written. Timing: A is a widening-era edit only until the first release carries this key — after that it is a Clause-②: no (narrowing) with an ADR-0087 disposition. The maintainer should rule before the Version Packages PR that carries this spec minor, or knowingly accept that later cost. Not a FAIL point: the describe states only the ruled invariant, is marked planned, and the choice is not the dev's to make.
  2. Whether the exclusion also screens the fallbackApprovers slate and the admin_rescue administrators — ESCALATED TO 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, a runtime decision. The describe today says "the slate approvers resolves to", the ruling's literal "resolved approver set"; recommendation B (every slate the node opens on) for segregation of duties, with the describe widened when the runtime lands. Not blocking.

out_of_scope_findings

  1. The liveness walk cannot reach any approval node config key — ACCEPTED as noted, not filed. A gate coverage gap, not a defect in this diff; flow-node-config-refusals.ts already holds the node-type to config-schema registry the walk could consult, so a spec-lane card is cheap if ledger coverage of node config keys is wanted. "carrier: none" stands.
  2. A test header comment in ui/component-record-blocks-empty-state-24266.test.ts misattributes a marker read to the gate — ACCEPTED as noted; a comment-only inaccuracy outside this diff.

deviations

  1. Ledger row measured impossible; shipped _note plus marker — ACCEPTED: measured, precedent-backed, and the gate is green.
  2. First commit's model-named trailer amended before push — VERIFIED: all five non-merge commits on origin/main..4823c57936 carry Claude-Session: plus the model-free co-author line; no pushed commit carries a model identifier.
  3. PR body footer in the session-URL form — CORRECT per AGENTS.md.
  4. gen:docs run directly rather than check:generated --fix — ACCEPTED: CI's Type Check · source gates ran check:authorable-surface and check:docs green on this head.
  5. test:repo not run locally — ACCEPTED: Test Core is the record and is named below.
  6. No reverse-verification for a cross-package type change — VERIFIED: no exhaustive consumer of the key set on origin/main; the only widening is an optional member.

Check-runs on the head (46 runs), read at 2026-10-24T12:05Z: every concluded run is success or skipped (opt-in harnesses and matrix placeholders); none is red. Four of the five required contexts are green: Lint & Repo Gates, TypeScript Type Check (all four lanes — source gates, workspace, consumer gates, debt ledger), Build Core, Governed Surface Queue Guard; also green: Spec property liveness, Check Changeset, Dogfood Regression Gate, Build Docs. NOT concluded at posting: Test Core (3/6) and Test Core (4/6) are in_progress, so the bare required context Test Core has not reported; the other four shards concluded success. This verdict is on the contract; the queue still requires Test Core to conclude green before anything is armed.

Implemented-by: claude/issue-25609-approval-exclude-submitter
Reviewed-by: session_01PC4NPXPgPmpDRG9DdDnyXT

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 24, 2026 12:15
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 24, 2026 12:15
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 24, 2026
Merged via the queue into main with commit 49653f9 Oct 24, 2026
51 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-25609-approval-exclude-submitter branch October 24, 2026 12:53
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec(automation): an approval step's excludeSubmitter option, default off — the protocol half of #25573 (ruling 6377731715, letter B)

2 participants