Skip to content

fix(plugin-approvals)!: retire the role: position-address arm and write the canonical fallback slot literal (ADR-0090 D3) - #21770

Merged
objectstack-fleet[bot] merged 9 commits into
mainfrom
claude/issue-21387-retire-role-arm
Oct 4, 2026
Merged

objectstack-fleet[bot] merged 9 commits into
mainfrom
claude/issue-21387-retire-role-arm

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #21387
Clause-②: no (narrowing)

Executes ruling A2 (record 5979854058, director batch #278, maintainer 「同意」) as one PR. Draft. The ADR-0087 disposition step runs in the Check Changeset job, which is NOT one of the seven required contexts. It read red at 022bb0fa because the honest disposition needed a ledger entry outside the card's first surface. Patch round 1 (below, surface revision 2 in seat verdict 5982629646) adds that entry, and the step is now green: Check Changeset read success on 3f4333ef, and check-adr-0087-registration --base origin/main exits 0 at 24f5c5a9. The patch reaches packages/spec/src/**, so an at-tier contract review PASS is owed before landing. The review of 24f5c5a9 failed (record 5983490043) on the entry's step and the changeset's package list; patch round 2 (below) fixes both, and a fresh review is owed on d0a53cc6.

What changes

  • The equivalence. role: comes out of POSITION_ADDRESS_PREFIXES (packages/plugins/plugin-approvals/src/approver-address.ts). position:POSITION is now the one spelling of a position address. Every reader already takes its addresses from this one module: resolveActor, the "My Pending" filter (approverRequestIds), the participant gate (visibleRequestIds), can_act (attachViewers), and every decision's slot test (takenSlot / heldSlot). So the prefix is the only line of logic that changes on the reader side. No named === role:... compare existed on main: resolveActor already read positionAddresses.
  • The writer. resolveApproverSpec's empty-lookup fallback returned ${a.type}:${a.value}, which is the AUTHORED type. It now returns ${type}:${a.value}, where type is canonicalApproverType(String(a.type)). A flow that still authors the deprecated { type: 'role', value: X } and whose membership-tier lookup finds no one now opens an org_membership_level:X slot, never role:X. DEPRECATED_APPROVER_TYPES holds only role, so no other type's literal moves. No stored slot is rewritten, there is no alias window, and the spec's ApproverType role alias is untouched.
  • Comments. Every comment in approver-address.ts and approval-service.ts that called role:POSITION a live spelling is rewritten. The two exemplar comments (remind fan-out, display-name lookup) now use position: as their example of a type:value literal.
  • Bounded in-place fix, same defect class. sys-approval-approver.object.ts named role: as the example slot literal in its docblock and in the approver field's description. Both now say position:. All four conditions hold: it is the same class (role: called a live slot spelling); the fix is mechanical; the dispatch reported no claim on plugin-approvals; and it is in the same package and gate family. git grep finds the description string in no generated artifact or translation bundle. The claim's file surface did not list this file.
  • Docs. content/docs/automation/approvals.mdx had four sentences that said a position matches, acts or takes a slot "under either spelling". They now name position: as the one address. The deprecated-type callout states which slot the fallback leaves and gives the one-line fix. The admin-override callout names both request classes. The retired prefix is named in words, because check:role-word holds that page at its one existing occurrence.
  • Changeset. .changeset/21387-retire-role-arm.md is minor, with the Clause-②: no (narrowing) line, the FROM role:POSITION → TO position:POSITION line, the author's one-line fix { type: 'position', value: ... }, and the admin's one-line handling for both request classes.

Pins

In plugin-approvals. Every turned pin keeps its expects, flipped to the retired answer.

  • The card's three pins. These are in the new block "the role: arm retired; the override decides what it leaves (ADR-0090 D3)" in approval-service.test.ts, plus the enumeration in approver-address-readers.test.ts.

    1. A position:POSITION slot is listed, counted, can_act: true, and decided by its holder with the default actor. The decision records actor_id as the holder, acted_as as the slot, and via_override: false.

    2. role:POSITION is no longer an address of that slot. It is not listed and not counted for the holder, not matched for SYS, and a named actor role:POSITION is refused (FORBIDDEN: cannot act as). The slot and the action log are untouched.

    3. Enumeration. No plugin-approvals writer produces a role: slot, and no reader compares one. There are two scans over every non-test .ts under src/, approver-address.ts included:

      • the string literals, read from the syntax tree, so comments are masked by construction. No literal may carry a role: prefix. The positive control is the scan finding position: in approver-address.ts.
      • every template of the shape ${X}:${Y}, classified in a ledger. None may interpolate an X.type property (the authored type), and resolveApproverSpec's writer is pinned by name to canonicalApproverType(...).

      A planted-source case shows both scans catch what they name, and that a comment does not trip them.

  • The admin rescue, permanent. This is the measurement from 5979222082. Two request classes run, each against three admin doors:

    • The two classes: a stored 15.x-era role:POSITION slot, planted in the CSV and then indexed by the plugin's own rebuildApproverIndex; and a new { type: 'role', value: POSITION } request whose tier lookup finds no one.
    • The three doors: admin_full_access, PLATFORM_ADMIN posture, and a same-org TENANT_ADMIN posture.

    Each admin decides through decide() with actorId set to the caller, which is what POST .../approve passes. The result is finalized, approved and resumed: true, with the resume signal branchLabel: approve, and the action records actor_id as the admin, acted_as: null and via_override: true. A reassign to the holder, after which the holder decides with the default actor, also rescues both classes. The holder of the same-named position is refused on both classes: the request is not listed, not visible, can_act is false, and the default actor and role:POSITION are both refused. Ruling A's stop condition is not triggered: the rescue decides both classes on the post-change code.

  • Turned pins. approver-address.test.ts: the prefix list, the fold, the colon split, the acting addresses, the named actor and heldSlot, with role: added to the "equivalent only to itself" table. approval-service.test.ts: the approvals: "My Pending" never lists a request routed to a position — the console filters with approverId=role:<p>, the request stores position:<p>, and the list filter matches literally #21350 block (list under role: is empty; a stored role: slot is invisible to the holder; the SYS filter miss), the resolveActor oracle (the pre-extraction predicate minus its role: arm; admitted count pinned at exactly 5), and the approvals: a holder of a position whose slot reads position:<p> can see the request but cannot decide it with the default actor (can_act false, approve 403), and loses sight of it after deciding (404) #21379 decision block (the holder naming role: is refused, and a stored role: slot is refused to the default actor). The fallback pin "keeps its legacy literal" is now "falls back to the canonical literal, in the slate and in the index": ['org_membership_level:admin'], plus the index row.

  • Dogfood (cross-lane domain:cli, declared on [PM seat] domain:cli — 🟢 os-warren · session_01RWZbGvPFcRKvUqASZtunCU #6024 5981995488). In my-pending-position-address, the reviewer and submitter rows under role: now assert no rows, role: joins the admin's "must not fold" list, and the reviewer approving as role:POSITION asserts [403, 'FORBIDDEN']. In position-address-readers, the holder's can_act row and the bystander row ask under position:, the holder under role: lists nothing, and approving as role:POSITION asserts [403, 'FORBIDDEN'] with nothing recorded. The holder then decides the same request under position:. Every position: row is unchanged.

Ablation (one-shot, not kept)

Both legs use scripts/ablation-replace.mjs in wrap mode on committed HEAD bce0f4a2, under the verify lock. Each leg proves the anchor count 1 → 0, the replacement count 0 → 1 and the blob change, and each restore proves the blob equals HEAD and git diff HEAD is empty. Both suites resolve plugin-approvals SOURCE, through relative imports in the package and the dogfood vitest alias to src/index.ts, so there is no dist leg.

  1. The role: arm put back (['position:'] → ['position:', 'role:'], blob c5691d3d → 8afd6cd0): the unit suites went red, 3 files, 16 failed / 347 passed. The dogfood went red, 2 of 2 failed. For example, the readers scan reported approver-address.ts:64 · "role:", and the dogfood reported reviewer under 'role:my_pending_reviewer': expected [ {...} ] to deeply equal []. Restored to c5691d3d.
  2. The authored fallback put back (${type} → ${a.type}, blob 3f5ec962 → 27d7355e): the unit suites had 4 failed / 359 passed. The template scan reported approval-service.ts · resolveApproverSpec · ${a.type}:${a.value} as unclassified, and the fallback pin got expected [ 'role:admin' ] to deeply equal [ 'org_membership_level:admin' ]. The dogfood stayed green, as expected: both fixtures route type: 'position'. Restored to 3f5ec962.

Tests and gates

All of these were run at 022bb0fa (the round-0 head; the plugin-approvals code half has not changed since):

  • plugin-approvals, full: 59 files, 891 passed.
  • The two dogfood files: 2 passed.
  • pnpm --filter @objectstack/plugin-approvals typecheck: tsc, the scripts project and check:test-typecheck all OK, with no new pinned signature.
  • dispatch-gates --commands: 95 derived commands. 94 exit 0 and 1 exits 1, check-adr-0087-registration --base origin/main (below). The --ran reconciliation reads 95 derived, 95 run, 0 NOT-MEASURED, 0 UNRUN.
  • Narrowed eslint over the 8 changed .ts files: 8 files, 0 errors, 0 warnings (--format json). All 8 are in the config's population (--print-config resolves each). Type-aware linting is not enabled anywhere in eslint.config.mjs (no parserOptions.project, as its own header states), so this diff cannot move any untouched file's verdict.

The ADR-0087 disposition (resolved)

Resolved by the seat: verdict 5982629646 answered A, patch round 1 added the entry, and patch round 2 moved it to step 18. The round-0 analysis is kept below as written.

The changeset is declared-breaking: it has the (narrowing) arm and the !. It therefore needs exactly one ADR-0087 marker. The gate reads the ruling-mandated FROM → TO and the author's fix as a migration prescription (--list shows prescription=yes). That refuses not-required (no-migration-prescription) as a self-contradiction, and runtime-interface-only inherits the same refusal. unpublished is false, and no existing ledger id covers approval slot addresses, so already-registered would be a dishonest claim. The honest disposition is registered with a new D3 semantic ledger entry under packages/spec/src/migrations/entries/semantic/. This is the pattern of the ADR-0090 runtime faces actor-user-roles-to-positions and action-session-roles-to-positions. That entry is a packages/spec edit, and this card's claim and dispatch declare ⛔ no packages/spec edit, so the marker is left absent rather than written false. The question is in the dev report on the card.

Acceptance notes

  • Pre-snapshot multi-approver requests. A unanimous, quorum or per_group request opened before the 17.0 open-time snapshot re-resolves its slate at every tally (decideNode's back-compat path). If such a request has a deprecated role approver whose lookup was empty, the re-resolved slot now spells org_membership_level:X while the stored one is role:X, and the tally writes the re-resolved slate back. Decidability is the same: only an admin can decide it either way. One edge is new: an approval a position holder recorded under role:X before the upgrade no longer satisfies the re-resolved slot, so the request needs the admin override to finalize. This falls within the accepted class of stored 15.x-era role: slots. It is the pre-existing re-resolution, not a rewrite this PR adds. carrier: this PR, noted, not filed.

  • Free-text slots are not covered. A reassign's to and a user approver's value are free text. A caller or author can still name the literal role:X there, which makes a slot nobody takes. This is the same pre-existing class as any type:value literal handed over there, and the admin rescue covers it. The enumeration pin scans code literals and templates, not data. carrier: none, noted, not filed.

  • Comment drift outside this surface. These comments in other packages now describe the old spelling:

    • the packages/lint/src/validate-approval-approvers.ts docblock (about :11) says the deprecated type "falls back to the role:sales_manager literal". The literal is now org_membership_level:sales_manager.
    • a packages/rest/src/rest-server.ts comment (about :12865) lists role: among the console's identities.
    • packages/spec/src/contracts/approval-service.ts (about :532) names role: as a stored acted_as spelling. That is still true for historical rows.

    carrier: whoever next touches each file. Noted, not filed.

  • Unreleased changesets disagree. .changeset/21350-my-pending-position-address.md and .changeset/21379-position-address-readers.md are still unreleased, and they describe role: as a second spelling. If they ship in the same version as this changeset, both statements reach the CHANGELOG, and this one's FROM → TO is the later fact. They are other cards' release inputs, so they are not edited here.

  • How the refusals are asserted. At the service layer the refusal is asserted by the service's own code prefix (FORBIDDEN:), because the service throws plain errors and the REST door assigns the status. The [status, code] pair [403, 'FORBIDDEN'] is asserted at the REST door in both dogfood pins. A role: ask on the list door is a filter miss: 200 with no rows, not an error. Those rows assert the 200 and the empty set.

Patch round 1 (head 24f5c5a)

Appended by the seat (domain:services#1, session_011K3zqE8Pv1Evw5hc8tZCnN) from the dev's patch-round report 5983095337, after seat verdict 5982629646. The opening paragraph above was corrected in the same act.

Seat verdict 5982629646 answered the ADR-0087 question with A (surface revision 2). The cross-lane declaration is on #6017 (5982635753).

  • One D3 semantic ledger entry, approval-position-address-role-retired, in packages/spec/src/migrations/entries/semantic/17.approval-position-address-role-retired.ts (moved to 18.approval-position-address-role-retired.ts in patch round 2). It is modelled on actor-user-roles-to-positions and action-session-roles-to-positions.

    • Surface: the approvals position address role:POSITION, meaning the approverId filter, a decision's actorId, and a stored pending_approvers slot.
    • Replacement: position:POSITION. An author's { type: 'role', value: POSITION } becomes { type: 'position', value: POSITION }.
    • Reason: the ADR-0090 D3 retirement; the deprecated role type's fallback now writing org_membership_level:VALUE; the two request classes that become admin-decided; no stored-slot rewrite; and why this is a D3 entry and not D2 (the address is runtime data, and which type the author meant is the author's judgment).
    • Acceptance criteria: what an upgrader verifies.
  • Generated with the repo's own tooling, never by hand: gen:migration-registry (the registry.ts region), gen:spec-changes (packages/spec/spec-changes.json) and gen:upgrade-guide (docs/protocol-upgrade-guide.md). check:migration-registry, check:spec-changes and check:upgrade-guide exit 0. (After patch round 2, spec-changes.json and the upgrade guide are back to main's bytes: neither projects step 18.)

  • The changeset's ADR-0087 marker now reads registered approval-position-address-role-retired, spelled from the gate's own output. check-adr-0087-registration --base origin/main exits 0.

  • Nothing else changes in packages/spec: no Zod schema, no ApproverType alias, no contract docblock, no stored slot. (Superseded in patch round 2: the changeset now also names "@objectstack/spec": patch.) The entry reaches 17.x upgraders through os migrate meta --from 17, which composes step 18; measured on the built spec at d0a53cc6, composeMigrationChain(17, 18) lists approval-position-address-role-retired. spec-changes.json and docs/protocol-upgrade-guide.md project only up to PROTOCOL_MAJOR (17), so after patch round 2 they carry no row for it and are byte-identical to main.

  • Hot file. origin/main was merged at 3f4333ef. feat(spec)!: object-metric drillDown.report is ReportSchema, object-timeline items the entry kind its variant selects, and action:group / action:menu members a closed inline action (#21464, S-final) #21764 landed after that, and 24f5c5a9 merges it: a clean merge that keeps both sides. The three ledger checks, check-adr-0087-registration and check-changeset-no-major exit 0 at 24f5c5a9.

  • Tests at 3f4333ef:

    • the spec build;
    • the 58 spec test files that read the ledger: 2219 passed;
    • the CLI unit-tier ledger readers: 22 passed;
    • driver-sql sql-driver-query-signature: 15 passed.

    Declared to CI: the full spec suite, which runs past this container's 10-minute foreground cap, and the CLI integration-tier ledger readers.

  • Gates at 3f4333ef: dispatch-gates derives 117 commands (22 new spec families) and all 117 exit 0. --ran reads 117 derived, 117 run, 0 NOT-MEASURED, 0 UNRUN. CI on 3f4333ef: 35 of 35 check runs completed, Check Changeset success.

  • The Clause-② gate's path limb now hits packages/spec/src/**, so a contract review is owed. The seat runs it.

Patch round 2 (head d0a53cc)

Appended by the seat (domain:services#1, session_011K3zqE8Pv1Evw5hc8tZCnN) from the dev's patch-round report 5983807880. Stale statements above were annotated in the same act.

The contract review of 24f5c5a9 (record 5983490043) returned FAIL on two defects. The seat adopted it in 5983502467 and ordered this round. Cross-lane addenda: #6017 5983506965 and #6024 5983511854.

  • The ledger entry moves to step 18. git mv renames entries/semantic/17.approval-position-address-role-retired.ts to 18.approval-position-address-role-retired.ts. The id and the content are unchanged; the prose names no step-17 boundary, so it needed no edit.
    • The step18 docblock says that a narrowing which lands after the v17.0.0 cut is told "where migrate meta users are told, at the major boundary where they look".
    • pnpm --filter @objectstack/spec gen:migration-registry regenerated registry.ts, and the entry now sits in the step18 block. Against origin/main (025008ae), registry.ts reads 1 file changed, 59 insertions(+).
  • The projections, regenerated and measured, not predicted. check:generated named two stale artifacts, spec-changes.json and docs/protocol-upgrade-guide.md, and check:generated --fix regenerated them from a fresh spec build. Both are now byte-identical to origin/main: git diff --shortstat origin/main HEAD prints nothing for either, and both have left the PR's file list.
    • Step 18 is projected by neither generator. Both loop up to and including PROTOCOL_MAJOR (17): build-spec-changes.ts:254 and build-upgrade-guide.ts:78. Neither file carries any 18.* id; for example, ui-action-group-menu-members-typed appears 0 times in each.
  • The channel, measured on the built packages/spec dist.
    • composeMigrationChain(17, 18) lists the entry. That is the chain os migrate meta --from 17 composes, because CHAIN_TERMINUS_MAJOR = max(PROTOCOL_MAJOR, MIGRATION_MAJORS) = 18 and MIGRATION_MAJORS = [17, 18].
    • composeMigrationChain(16, 17) no longer lists it.
    • Control: the step-18 entry ui-action-group-menu-members-typed is listed by (17, 18).
  • The changeset now names "@objectstack/spec": patch next to "@objectstack/plugin-approvals": minor. Every other line is unchanged: the Clause-②: no (narrowing) line, FROM → TO, the author's fix, the admin handling, and the marker registered approval-position-address-role-retired. check-changeset-no-major --base origin/main exits 0 with patch. check-adr-0087-registration --base origin/main exits 0: the id resolves at HEAD and is new in the diff.
  • Bounded comment fix. The header of packages/qa/dogfood/test/fixtures/my-pending-position-fixture.ts (about :10) no longer says "under both approver-address spellings". It now says that position:POSITION lists the request and the retired role:POSITION spelling (ADR-0090 D3) does not. The change is comment only.
  • Nothing else changed: no Zod schema, no ApproverType alias, no contract docblock, no stored-slot rewrite, and no plugin-approvals code.
  • Hot file. origin/main 025008ae was merged at d0a53cc6. The merge was clean, and main brought no change to packages/spec.
  • Gates at d0a53cc6:
    • check:migration-registry, check:spec-changes, check:upgrade-guide, check-adr-0087-registration --base origin/main and check-changeset-no-major --base origin/main all exit 0.
    • dispatch-gates derives 116 commands. check:generated dropped out, because no generated artifact is in the diff any more. All 116 exit 0, and --ran reads 116 derived, 116 run, 0 NOT-MEASURED, 0 UNRUN.
  • Tests at d0a53cc6:
    • 57 of round 1's 58 ledger-reading spec test files: 2134 passed.
    • The CLI unit-tier ledger readers: 22 passed.
    • driver-sql sql-driver-query-signature: 15 passed.
    • Declared to CI: scripts/build-schemas-check-mode.test.ts (it spawns full schema builds and runs past the 10-minute foreground cap; it was green in round 1's run at 3f4333ef), the full spec suite, and the CLI integration-tier ledger readers.
    • The plugin-approvals code half is unchanged and reused from round 0.
  • A fresh at-tier contract review is owed on d0a53cc6. The seat runs it once CI on this head has settled; landing waits for its PASS.

Generated by Claude Code

claude added 4 commits October 4, 2026 16:37
…nicalize the fallback literal (WIP)

position: is the one spelling of a position address (ADR-0090 D3); the
deprecated role approver type's empty-lookup fallback writes the canonical
org_membership_level: literal. Pins turned; enumeration and admin-rescue pins added.

Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN
Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN
Co-authored-by: Claude <noreply@anthropic.com>
…es position: as the one address

Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-approvals, @objectstack/spec, touching 7 documentable anchor(s).

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

  • content/docs/automation/flows.mdx (via ApprovalService (symbol, a top-level class))
  • content/docs/permissions/system-context.mdx (via resolveActor (symbol, a method of class ApprovalService))

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

  • content/docs/releases/v17/17-6.mdx (via ApprovalService (symbol, a top-level class))

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
  • 5 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 138 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 025008ae954311ab91ef264d1b750a00ed193b32 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 025008ae954311ab91ef264d1b750a00ed193b32

⚠️ 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 025008ae954311ab91ef264d1b750a00ed193b32 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

claude added 3 commits October 4, 2026 17:39
…ement in the ADR-0087 ledger

One D3 semantic entry, approval-position-address-role-retired, with its
generated registry region, spec-changes.json and upgrade-guide rows; the
changeset's disposition marker names it.

Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 24f5c5a9f9b441e6522b3008d7ae5311cc9abda6
Local-runs: none

Inputs read: card #21387 (body and all 11 comments — rulings 5978658250 and 5979854058, dev reports 5979222082 / 5982595664 / 5983095337, seat verdict 5982629646); PR #21770 (body, the 14-file list, the net diff against main at merge-base 50b5e033); the check-runs on the head, latest run per name — every gate-carrying check success, Console Pin Gate and Packed-tarball smoke (opt-in) skipped by design, Check PR Size and Auto Label ran success at 18:31 and skipped on the later labeled/edited re-trigger by their own if:. The PR is a same-repo draft, 740+159 changed lines, no governed-surface path; Governed Surface Queue Guard success.

① Derived judgments

Right:

  1. Accept-set narrowing on the approvals REST surface. POSITION_ADDRESS_PREFIXES is ['position:'] (packages/plugins/plugin-approvals/src/approver-address.ts). role:POSITION no longer names a position for the approverId filter, a decision's actorId, the participant gate, can_act and heldSlot; every reader takes the one module and no second fold exists (git grep at the head finds no role: literal in plugin-approvals non-test source). Ruling A2 executed as written.
  2. The writer. resolveApproverSpec's fallback returns ${type}:${a.value} with type = canonicalApproverType(String(a.type)), so a deprecated { type: 'role', value: X } with an empty tier lookup opens org_membership_level:X; DEPRECATED_APPROVER_TYPES holds only role; no stored slot is rewritten; the spec's ApproverType alias is untouched. Right per A2.
  3. No TS public surface moves. approver-address.ts is not re-exported from the package index and no package outside plugin-approvals imports it; api-surface is unchanged (all four Type Check gates green). The sys_approval_approver.approver description string moves role: to position:; no generated bundle or translation carries that string. Right (bounded in-place fix, accepted by the seat).
  4. Pins. The card's three pins; the enumeration pin (AST string-literal scan with comments masked by construction and position: as the positive control; a ${X}:${Y} template ledger that forbids interpolating an authored .type and pins the fallback writer to canonicalApproverType by name; a planted-source control for all three); the admin rescue made permanent (two request classes, three override doors through decide() with via_override: true and resumed: true, plus a reassign to the holder); every turned pin keeps its expects. Dogfood: role: list rows assert 200 with no rows (a filter miss), decision rows assert [403, 'FORBIDDEN'] with nothing recorded. Right. Ruling A's stop condition is pinned not triggered.
  5. Docs. content/docs/automation/approvals.mdx: the four "either spelling" sentences now name position: as the one address; the deprecated-type callout names the slot it leaves and the one-line fix; the override callout names both classes. check:role-word green.

Wrong:
6. The ADR-0087 ledger entry is registered under the wrong step. packages/spec/src/migrations/entries/semantic/17.approval-position-address-role-retired.ts lands in step17 (toMajor: 17). The registry's own rule (registry.ts, the step18 docblock) is that a narrowing landing after v17.0.0 was cut — it was published 2026-08-14 (content/docs/releases/v17/17-0.mdx), and this one lands on a 17.6.0 tree — belongs to the uncut step 18, "where migrate meta users are told, at the major boundary where they look". The mechanism makes the placement load-bearing: packages/cli/src/commands/migrate/meta.ts composes --from 17 up to CHAIN_TERMINUS_MAJOR, which is max(PROTOCOL_MAJOR 17, MIGRATION_MAJORS) = 18, and composeMigrationChain keeps only the majors strictly above fromMajor and at most toMajor. A 17.x deployment — the only consumer this retirement affects — running os migrate meta --from 17 is therefore shown step 18 and never step 17. Filed under 17, the entry is invisible to the one tool the ledger exists to feed, which is the silent-upgrade-path gap check-adr-0087-registration was built against, reached through a different door; the registered marker is satisfied syntactically and defeated in effect. Precedent on main: since the clone's history horizon (2026-09-18) 158 18.* semantic entries were added and 0 17.*; the two entries this one was modelled on (actor-user-roles-to-positions, action-session-roles-to-positions) are pre-cut (2026-08-06), which is why they are 17; the retirement skill's §3 checklist also treats step 18 as the live step. The PR body's sentence "the new ledger rows reach upgraders through spec-changes.json and the upgrade guide" is true only because both projections stop at PROTOCOL_MAJOR (packages/spec/scripts/build-spec-changes.ts:254, build-upgrade-guide.ts:78) — the price of that reach is that the CLI never lists it, and the row sits in the "Protocol 16 → 17" section a 17.x upgrader believes they have already done. Fix: rename the file to 18.approval-position-address-role-retired.ts (id unchanged), run pnpm --filter @objectstack/spec gen:migration-registry, then check:generated --fix (the 17 rows leave spec-changes.json and docs/protocol-upgrade-guide.md; check:migration-registry, check:spec-changes, check:upgrade-guide and check-adr-0087-registration --base origin/main must exit 0 — the marker registered approval-position-address-role-retired stays valid, since the id still resolves at HEAD and is new in the diff). The seat then corrects the body's reach sentence to name os migrate meta --from 17 as the channel.

Residuals, not defects of this diff: (a) the pinned console sending position: only was measured on the card at objectui ab18797215 (comment 5975844577); .objectui-sha on main is now 2e818d0b, Console Pin Gate skips by design, and the sibling is not readable from this review's inputs — one read of sharedUserFeeds.ts approverIdentities() at the current pin before landing is cheap insurance. (b) packages/qa/dogfood/test/fixtures/my-pending-position-fixture.ts:10 still says "under both approver-address spellings" — comment drift inside the admitted cross-lane surface; it can ride the patch round.

② Semver level

Clause-②: no (narrowing) — right, on PR body line 2 and in the changeset, one spelling as the seat set out. Nothing widens: a ledger row is neither a new accepted key nor a new exported symbol. The accept set narrows: a role:POSITION actor no longer addresses a position slot and the fallback stops writing role:. A breaking narrowing ships minor in the launch window, with breaking-ness carried by the ! banner and the ADR-0087 disposition (ADR-0087, amended 2026-09-13); check-changeset-no-major and check-adr-0087-registration are both green inside Check Changeset at this head. The FROM role:POSITION → TO position:POSITION line, the author's one-line fix and the admin's one-line handling for both request classes are in the body as ruling A2 requires, and the marker reads registered with the new id, the only honest disposition once the body carries a prescription.

Defect: the frontmatter names only @objectstack/plugin-approvals while the diff moves @objectstack/spec's published source — src/migrations/registry.ts ships in dist under the package's ./migrations export, and spec-changes.json is in its files. The Check Changeset prose says "name the packages it releases"; the precedent this entry was modelled on, the actor-user-roles-to-positions backfill, named @objectstack/spec (its text is in packages/spec/CHANGELOG.md under Patch Changes). Lockstep makes the version identical either way, but a spec consumer grepping spec's CHANGELOG for this retirement finds nothing, and CHANGELOG is the text an upgrading agent greps. Fix: add '@objectstack/spec': patch to .changeset/21387-retire-role-arm.md (a ledger row; no spec accept-set or export change — minor would also pass, both are below major).

③ Boundary flags

Earlier dispatch (dev report 5979222082): both open questions ruled — A2 (5979854058) and the two dogfood files admitted to the surface, declared cross-lane on #6024. Answered.

Round 0 (dev report 5982595664): the ADR-0087 disposition open question → the seat answered A (5982629646) and round 1 executed it; answered, and its step placement is the defect in ① 6. The three surface deviations — sys-approval-approver.object.ts bounded in-place fix, the two example comments in approval-service.ts, and the dogfood mechanism correction (list rows 200/empty, decision rows 403) — accepted by the seat; answered. The PR-body correction the dev asked for → made by the seat (corrected opening paragraph, appended Patch round 1); answered. Ruling A's stop condition (the admin rescue) → measured and pinned, not triggered; answered.

Round 1 (dev report 5983095337): open_questions is empty. Deviations: the head moved to 24f5c5a9 with only the ledger gates rerun locally → CI on 24f5c5a9 is complete and every gate-carrying check is green; answered. @objectstack/spec left out of the frontmatter → judged a defect in ② above. The entry's surface string carries no backticks → right, the upgrade-guide generator wraps it.

Acceptance notes, each judged: the pre-snapshot tally re-resolution falls inside A2's accepted class and is the pre-existing back-compat path, not a rewrite this PR adds — noted, right. Free-text reassign.to and user values can still spell a dead role:X slot as data — pre-existing class, the rescue covers it, the enumeration pin scans code not data — noted, right. Comment-only drift in packages/lint/src/validate-approval-approvers.ts, packages/rest/src/rest-server.ts and the spec contract docblock — noted, right (the lint rule's user-facing message does not name role:). The unreleased #21350 / #21379 changesets describing the retired second spelling — release notes are compiled centrally and this card's FROM → TO is the later fact — noted, right.

Escalation: none owed. Defect ① 6 is settled by the registry's own step rule and the CLI's terminus, not a maintainer question; the fix leaves every ⛔ line of ruling A2 (no stored-slot rewrite, no alias window, ApproverType alias untouched) where it stands. Both defects are patch-round items on this branch; a fresh at-tier record is owed on the patched head.

Implemented-by: claude/issue-21387-retire-role-arm
Reviewed-by: session_011K3zqE8Pv1Evw5hc8tZCnN

VERDICT: FAIL

claude added 2 commits October 4, 2026 19:28
…18, and name spec in the changeset

The ledger entry approval-position-address-role-retired moves from step 17
to the uncut step 18 (the step18 docblock: a narrowing after the v17.0.0 cut
is told where os migrate meta --from 17 looks); id unchanged. The registry is
regenerated, and spec-changes.json and the upgrade guide return to main's
bytes (both project only up to PROTOCOL_MAJOR). The changeset names
@objectstack/spec at patch. One fixture comment stops claiming both spellings.

Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: d0a53cc6e715acee7dd94e55b5f87361350ac66d
Local-runs: none

Inputs read: card #21387 (body and all 13 comments: rulings 5978658250 and 5979854058, dev reports 5979222082 / 5982595664 / 5983095337 / 5983807880, seat verdicts 5982629646 and 5983502467); PR #21770 (body, the 13-file list, the net diff against main at merge-base 025008ae, and its two comments, the earlier record 5983490043 on head 24f5c5a9 included); the 42 check-runs on d0a53cc6, read latest-run-per-name: every gate-carrying check success; Console Pin Gate and Packed-tarball smoke (opt-in) skipped by design; Auto Label and Check PR Size ran success at 19:28 and skipped on the 19:59 re-trigger by their own if:; Check Changeset success on both of its runs. Same-repo draft, 726+160 lines, no governed-surface path in the file list, Governed Surface Queue Guard success. Reading git objects of the shared clone (git diff / git show / git grep at the head) was the only local act.

① Derived judgments

Right:

  1. Accept-set narrowing on the approvals surface. POSITION_ADDRESS_PREFIXES is ['position:'] (packages/plugins/plugin-approvals/src/approver-address.ts). role:POSITION no longer names a position for the approverId filter, a decision's actorId, the participant gate, can_act and heldSlot; every reader takes the one module, and git grep at the head finds no role: string literal in plugin-approvals non-test source (comments only; the { role: tier } at approval-service.ts:2506 is a sys_member column, not a slot). No in-repo producer outside the package builds a role: slot or asks under one: the remaining role: hits in packages/** source are the messaging subscriber selector over sys_member, a different thing. Ruling A2 executed as written.
  2. The writer. resolveApproverSpec's empty-lookup fallback returns ${type}:${a.value} with type = canonicalApproverType(String(a.type)), so a deprecated { type: 'role', value: X } opens org_membership_level:X; DEPRECATED_APPROVER_TYPES holds only role; no stored slot is rewritten (the only role: slot writer in the diff is the test helper that plants one); the spec's ApproverType alias is untouched (no schema file under packages/spec is in the diff). Right per A2.
  3. No TS public surface moves. approver-address.ts is not re-exported from the package index and nothing outside the package imports its functions; all four Type Check gates are green. The sys_approval_approver.approver description moves role: to position:; at the head that string exists in no generated artifact or translation bundle. Right, the seat's accepted bounded in-place fix.
  4. Pins. The card's three pins; the enumeration pin (an AST string-literal scan with comments masked by construction and position: in approver-address.ts as the positive control; a ${X}:${Y} template ledger that refuses an interpolated authored .type and pins the fallback writer to canonicalApproverType by name; a planted-source control for literals, comments and templates); the admin rescue made permanent (two request classes, three override doors through decide() with via_override: true and resumed: true, plus a reassign to the holder). Every turned pin keeps its expects: the fallback pin went from one expect to two, the resolveActor oracle's admitted count tightened from greater-than-5 to exactly 5. Dogfood: role: list rows assert 200 with no rows (a filter miss), decision rows assert [403, 'FORBIDDEN'] with nothing recorded; Dogfood Regression Gate 3/3 success. Ruling A's stop condition is pinned not triggered.
  5. Docs. content/docs/automation/approvals.mdx: the four "either spelling" sentences now name position: as the one address; the deprecated-type callout names the slot it leaves and the one-line fix; the override callout names both request classes; the retired prefix is named in words (check:role-word inside Lint & Repo Gates, green). No other hand-written page under content/docs still calls the retired spelling a position address, and the two pages Docs Drift flagged (flows.mdx, system-context.mdx) carry no sentence about position spellings.
  6. The ADR-0087 ledger entry, now in the right step. Defect ① 6 of record 5983490043 is fixed: the file is packages/spec/src/migrations/entries/semantic/18.approval-position-address-role-retired.ts, and registry.ts is +59 lines inside the os-generated semantic:18 region, in id order (between api-runtime-config-durations-unit-in-key and assembled-package-body-plugins-envelope); the entry's prose names no step, major or 16-to-17 boundary. Reach, read off the tree: PROTOCOL_MAJOR is 17 (protocol-version.ts), MIGRATION_MAJORS is [17, 18], CHAIN_TERMINUS_MAJOR is max of the two = 18 (packages/cli/src/commands/migrate/meta.ts:91), and composeMigrationChain keeps majors strictly above fromMajor and at most toMajor (chain.ts:40-42), so os migrate meta --from 17 lists the entry and a 16-to-17 chain does not. spec-changes.json and docs/protocol-upgrade-guide.md being out of the diff is right, not an omission: both generators loop MIGRATION_SUPPORT_FLOOR + 1 .. PROTOCOL_MAJOR (build-spec-changes.ts:254, build-upgrade-guide.ts:78), so step 18 projects to neither and nothing is stale; check:migration-registry (Lint & Repo Gates) and check:spec-changes / check:upgrade-guide / check:generated --reconcile-only (Type Check · source gates) are green. D3 rather than D2 is right: the address is runtime data, never a sys_metadata key, and the authored { type: 'role' } is ambiguous between a tier and a position.
  7. Fixture comment. Residual (b) of the earlier record is fixed: the header of packages/qa/dogfood/test/fixtures/my-pending-position-fixture.ts now says position: lists and the retired spelling does not.

Residuals, not defects of this diff:
(a) The console pin. .objectui-sha is 2e818d0b on both main and the head (not in the diff). The card measured position:-only sending at ab18797215 (5975844577); the seat read the pin bump PR #21710 as carrying no approvals change (5983502467); objectui is outside this review's inputs, so the card's measurement stands as declared. No escalation is owed: nothing in this diff moves the pin, and a regression there would fail loud (an empty "My Pending" for every position holder at the first console dogfood), not silently.
(b) Comment-only drift outside the surface. Beyond the three sites the dev named (packages/lint/src/validate-approval-approvers.ts about :11, packages/rest/src/rest-server.ts about :12865, packages/spec/src/contracts/approval-service.ts about :532), two more of the same class: packages/spec/src/contracts/approval-service.ts about :232 and :858 (the pending_approver_names and approverId docblocks still list role: entries among a caller's identities) and packages/rest/src/query-allowlist.ts about :33 (the Console asks approverId=id,role:user). The seat's patch-round fence ("no contract docblock") kept them out of this PR; they share the carrier of the three named, a follow-up, and the lint rule's user-facing messages do not name role:.

② Semver level

Clause-②: no (narrowing) is right, on PR body line 2 and in the changeset, one spelling. Nothing widens: no new accepted key, no new exported symbol; the ledger row is data inside an existing export and api-surface is unchanged. The accept set narrows: a role:POSITION actor or filter no longer addresses a position slot, and the fallback stops writing role:. In the launch window a breaking narrowing ships minor, with breaking-ness carried by the ! banner and the ADR-0087 disposition (check-changeset-no-major header), so @objectstack/plugin-approvals: minor is right. @objectstack/spec: patch is right and fixes defect ② of the earlier record: the diff moves spec's published source (src/migrations/registry.ts ships under the ./migrations export), so the package must be named; the LEVEL axis binds only a Clause-②: yes PR, and no symbol or accept set of spec moves, so patch is the honest level (minor would equally pass; the fixed group makes the bump identical). The ADR-0087 marker reads registered approval-position-address-role-retired, the only honest disposition once the body carries a FROM → TO; the id resolves at HEAD (registry.ts:7732) and is new in the diff. The changeset body carries FROM role:NAME → TO position:NAME for the approverId filter and every decision's actorId, the author's one-line fix { type: 'position', value: ... }, and the admin's one-line handling for both request classes, exactly as ruling A2's Execution lists. Check Changeset is success on both runs at this head.

③ Boundary flags

Round 2 (dev report 5983807880): open_questions empty. Deviations: (1) the entry's prose needed no step correction, verified against the file, it names no step, major or boundary; answered. (2) scripts/build-schemas-check-mode.test.ts declared to CI, Test Core 6/6 success; answered. (3) check:generated dropped from the derived set and was run with --fix: right, no generated artifact remains in the diff, and the two projections are byte-identical to main by construction since both generators stop at PROTOCOL_MAJOR; answered.

Record 5983490043 (head 24f5c5a9): defect ① 6 (step 17) fixed in ① 6 above; defect ② (spec missing from the frontmatter) fixed in ② above; residual (a) judged above, stands as declared, no escalation; residual (b) fixed in ① 7.

Round 1 (dev report 5983095337): no open questions. Deviations: the head moved to 24f5c5a9 with only the ledger gates rerun locally, since superseded by full CI on d0a53cc6; @objectstack/spec left out of the frontmatter, now fixed; the surface string carries no backticks, right. Answered.

Round 0 (dev report 5982595664): the ADR-0087 disposition open question, answered A by the seat (5982629646) and executed in rounds 1 and 2; the surface deviations (sys-approval-approver.object.ts bounded fix, the two example comments in approval-service.ts, the dogfood mechanism correction with list rows 200/empty and decision rows 403, the bystander row asking under position:) accepted by the seat; the PR-body correction made by the seat. Answered. Earlier dispatch (dev report 5979222082): both open questions ruled, A2 (5979854058) and the two dogfood files admitted to the surface, declared cross-lane on #6024. Answered.

Acceptance notes, each judged: the pre-snapshot tally re-resolution falls inside A2's accepted class and is the pre-existing back-compat path, right; free-text reassign.to and user values can still spell a dead role:X slot as data, pre-existing class, the rescue covers it and the enumeration pin scans code not data, right; comment drift in other packages, right (carrier noted in ① residual (b)); the unreleased #21350 / #21379 changesets describing the retired second spelling, release notes are compiled centrally and this card's FROM → TO is the later fact, right.

Ruling A's stop condition (the admin rescue): measured and pinned on the post-change code, not triggered. Every ⛔ line of ruling A2 holds: no stored-slot rewrite, no alias window, the spec's ApproverType alias untouched. Escalation: none owed.

Implemented-by: claude/issue-21387-retire-role-arm
Reviewed-by: session_011K3zqE8Pv1Evw5hc8tZCnN

VERDICT: PASS

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 4, 2026 20:12
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 4, 2026 20:12
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 4, 2026
Merged via the queue into main with commit c9c555a Oct 4, 2026
47 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-21387-retire-role-arm branch October 4, 2026 20:38
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/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

approvals: retire the role: arm of the position-address equivalence once the pinned console sends position: (ADR-0090 D3; split from #21379 item 5)

2 participants