fix(plugin-approvals)!: retire the role: position-address arm and write the canonical fallback slot literal (ADR-0090 D3) - #21770
Conversation
…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>
…heck:role-word) Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 2 package(s): 2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 138 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # 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
|
…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>
Contract reviewServed-tier: 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 ① Derived judgmentsRight:
Wrong: Residuals, not defects of this diff: (a) the pinned console sending ② Semver level
Defect: the frontmatter names only ③ Boundary flagsEarlier 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 — Round 1 (dev report 5983095337): 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 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, Implemented-by: VERDICT: FAIL |
…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>
Contract reviewServed-tier: 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 ① Derived judgmentsRight:
Residuals, not defects of this diff: ② Semver level
③ Boundary flagsRound 2 (dev report 5983807880): Record 5983490043 (head Round 1 (dev report 5983095337): no open questions. Deviations: the head moved to 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 ( 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 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 Implemented-by: VERDICT: PASS |
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 Changesetjob, which is NOT one of the seven required contexts. It read red at022bb0fabecause 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 Changesetread success on3f4333ef, andcheck-adr-0087-registration --base origin/mainexits 0 at24f5c5a9. The patch reachespackages/spec/src/**, so an at-tier contract review PASS is owed before landing. The review of24f5c5a9failed (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 ond0a53cc6.What changes
role:comes out ofPOSITION_ADDRESS_PREFIXES(packages/plugins/plugin-approvals/src/approver-address.ts).position:POSITIONis 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. Nonamed === role:...compare existed onmain:resolveActoralready readpositionAddresses.resolveApproverSpec's empty-lookup fallback returned${a.type}:${a.value}, which is the AUTHORED type. It now returns${type}:${a.value}, wheretypeiscanonicalApproverType(String(a.type)). A flow that still authors the deprecated{ type: 'role', value: X }and whose membership-tier lookup finds no one now opens anorg_membership_level:Xslot, neverrole:X.DEPRECATED_APPROVER_TYPESholds onlyrole, so no other type's literal moves. No stored slot is rewritten, there is no alias window, and the spec'sApproverTyperolealias is untouched.approver-address.tsandapproval-service.tsthat calledrole:POSITIONa live spelling is rewritten. The two exemplar comments (remind fan-out, display-name lookup) now useposition:as their example of atype:valueliteral.sys-approval-approver.object.tsnamedrole:as the example slot literal in its docblock and in theapproverfield'sdescription. Both now sayposition:. All four conditions hold: it is the same class (role: called a live slot spelling); the fix is mechanical; the dispatch reported no claim onplugin-approvals; and it is in the same package and gate family.git grepfinds the description string in no generated artifact or translation bundle. The claim's file surface did not list this file.content/docs/automation/approvals.mdxhad four sentences that said a position matches, acts or takes a slot "under either spelling". They now nameposition: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, becausecheck:role-wordholds that page at its one existing occurrence..changeset/21387-retire-role-arm.mdisminor, with theClause-②: no (narrowing)line, the FROMrole:POSITION→ TOposition:POSITIONline, 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 itsexpects, 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 inapprover-address-readers.test.ts.A
position:POSITIONslot is listed, counted,can_act: true, and decided by its holder with the default actor. The decision recordsactor_idas the holder,acted_asas the slot, andvia_override: false.role:POSITIONis no longer an address of that slot. It is not listed and not counted for the holder, not matched for SYS, and a named actorrole:POSITIONis refused (FORBIDDEN: cannot act as). The slot and the action log are untouched.Enumeration. No
plugin-approvalswriter produces arole:slot, and no reader compares one. There are two scans over every non-test.tsundersrc/,approver-address.tsincluded:role:prefix. The positive control is the scan findingposition:inapprover-address.ts.${X}:${Y}, classified in a ledger. None may interpolate anX.typeproperty (the authored type), andresolveApproverSpec's writer is pinned by name tocanonicalApproverType(...).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:role:POSITIONslot, planted in the CSV and then indexed by the plugin's ownrebuildApproverIndex; and a new{ type: 'role', value: POSITION }request whose tier lookup finds no one.admin_full_access,PLATFORM_ADMINposture, and a same-orgTENANT_ADMINposture.Each admin decides through
decide()withactorIdset to the caller, which is whatPOST .../approvepasses. The result isfinalized,approvedandresumed: true, with the resume signalbranchLabel: approve, and the action recordsactor_idas the admin,acted_as: nullandvia_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_actis false, and the default actor androle:POSITIONare 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 andheldSlot, withrole: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 withapproverId=role:<p>, the request storesposition:<p>, and the list filter matches literally #21350 block (list underrole:is empty; a storedrole:slot is invisible to the holder; the SYS filter miss), theresolveActororacle (the pre-extraction predicate minus itsrole:arm; admitted count pinned at exactly 5), and the approvals: a holder of a position whose slot readsposition:<p>can see the request but cannot decide it with the default actor (can_actfalse, approve 403), and loses sight of it after deciding (404) #21379 decision block (the holder namingrole:is refused, and a storedrole: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 #60245981995488). Inmy-pending-position-address, the reviewer and submitter rows underrole:now assert no rows,role:joins the admin's "must not fold" list, and the reviewer approving asrole:POSITIONasserts[403, 'FORBIDDEN']. Inposition-address-readers, the holder'scan_actrow and the bystander row ask underposition:, the holder underrole:lists nothing, and approving asrole:POSITIONasserts[403, 'FORBIDDEN']with nothing recorded. The holder then decides the same request underposition:. Everyposition:row is unchanged.Ablation (one-shot, not kept)
Both legs use
scripts/ablation-replace.mjsin wrap mode on committed HEADbce0f4a2, 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 andgit diff HEADis empty. Both suites resolveplugin-approvalsSOURCE, through relative imports in the package and the dogfood vitest alias tosrc/index.ts, so there is no dist leg.role:arm put back (['position:']→['position:', 'role:'], blobc5691d3d→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 reportedapprover-address.ts:64 · "role:", and the dogfood reportedreviewer under 'role:my_pending_reviewer': expected [ {...} ] to deeply equal []. Restored toc5691d3d.${type}→${a.type}, blob3f5ec962→27d7355e): the unit suites had 4 failed / 359 passed. The template scan reportedapproval-service.ts · resolveApproverSpec · ${a.type}:${a.value}as unclassified, and the fallback pin gotexpected [ 'role:admin' ] to deeply equal [ 'org_membership_level:admin' ]. The dogfood stayed green, as expected: both fixtures routetype: 'position'. Restored to3f5ec962.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.pnpm --filter @objectstack/plugin-approvals typecheck: tsc, the scripts project andcheck:test-typecheckall 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--ranreconciliation reads 95 derived, 95 run, 0 NOT-MEASURED, 0 UNRUN..tsfiles: 8 files, 0 errors, 0 warnings (--format json). All 8 are in the config's population (--print-configresolves each). Type-aware linting is not enabled anywhere ineslint.config.mjs(noparserOptions.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 (--listshowsprescription=yes). That refusesnot-required (no-migration-prescription)as a self-contradiction, andruntime-interface-onlyinherits the same refusal.unpublishedis false, and no existing ledger id covers approval slot addresses, soalready-registeredwould be a dishonest claim. The honest disposition isregisteredwith a new D3 semantic ledger entry underpackages/spec/src/migrations/entries/semantic/. This is the pattern of the ADR-0090 runtime facesactor-user-roles-to-positionsandaction-session-roles-to-positions. That entry is apackages/specedit, and this card's claim and dispatch declare ⛔ nopackages/specedit, 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,quorumorper_grouprequest 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 deprecatedroleapprover whose lookup was empty, the re-resolved slot now spellsorg_membership_level:Xwhile the stored one isrole: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 underrole:Xbefore 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-erarole: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
toand auserapprover'svalueare free text. A caller or author can still name the literalrole:Xthere, which makes a slot nobody takes. This is the same pre-existing class as anytype:valueliteral 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:
packages/lint/src/validate-approval-approvers.tsdocblock (about :11) says the deprecated type "falls back to therole:sales_managerliteral". The literal is noworg_membership_level:sales_manager.packages/rest/src/rest-server.tscomment (about :12865) listsrole:among the console's identities.packages/spec/src/contracts/approval-service.ts(about :532) namesrole:as a storedacted_asspelling. 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.mdand.changeset/21379-position-address-readers.mdare still unreleased, and they describerole: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. Arole: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 report5983095337, after seat verdict5982629646. 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, inpackages/spec/src/migrations/entries/semantic/17.approval-position-address-role-retired.ts(moved to18.approval-position-address-role-retired.tsin patch round 2). It is modelled onactor-user-roles-to-positionsandaction-session-roles-to-positions.role:POSITION, meaning theapproverIdfilter, a decision'sactorId, and a storedpending_approversslot.position:POSITION. An author's{ type: 'role', value: POSITION }becomes{ type: 'position', value: POSITION }.roletype's fallback now writingorg_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).Generated with the repo's own tooling, never by hand:
gen:migration-registry(theregistry.tsregion),gen:spec-changes(packages/spec/spec-changes.json) andgen:upgrade-guide(docs/protocol-upgrade-guide.md).check:migration-registry,check:spec-changesandcheck:upgrade-guideexit 0. (After patch round 2,spec-changes.jsonand the upgrade guide are back tomain'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/mainexits 0.Nothing else changes in
packages/spec: no Zod schema, noApproverTypealias, 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 throughos migrate meta --from 17, which composes step 18; measured on the built spec atd0a53cc6,composeMigrationChain(17, 18)listsapproval-position-address-role-retired.spec-changes.jsonanddocs/protocol-upgrade-guide.mdproject only up toPROTOCOL_MAJOR(17), so after patch round 2 they carry no row for it and are byte-identical tomain.Hot file.
origin/mainwas merged at3f4333ef. 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, and24f5c5a9merges it: a clean merge that keeps both sides. The three ledger checks,check-adr-0087-registrationandcheck-changeset-no-majorexit 0 at24f5c5a9.Tests at
3f4333ef: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-gatesderives 117 commands (22 new spec families) and all 117 exit 0.--ranreads 117 derived, 117 run, 0 NOT-MEASURED, 0 UNRUN. CI on3f4333ef: 35 of 35 check runs completed,Check Changesetsuccess.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 report5983807880. 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.git mvrenamesentries/semantic/17.approval-position-address-role-retired.tsto18.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.step18docblock says that a narrowing which lands after the v17.0.0 cut is told "wheremigrate metausers are told, at the major boundary where they look".pnpm --filter @objectstack/spec gen:migration-registryregeneratedregistry.ts, and the entry now sits in thestep18block. Againstorigin/main(025008ae),registry.tsreads1 file changed, 59 insertions(+).check:generatednamed two stale artifacts,spec-changes.jsonanddocs/protocol-upgrade-guide.md, andcheck:generated --fixregenerated them from a fresh spec build. Both are now byte-identical toorigin/main:git diff --shortstat origin/main HEADprints nothing for either, and both have left the PR's file list.PROTOCOL_MAJOR(17):build-spec-changes.ts:254andbuild-upgrade-guide.ts:78. Neither file carries any18.*id; for example,ui-action-group-menu-members-typedappears 0 times in each.packages/specdist.composeMigrationChain(17, 18)lists the entry. That is the chainos migrate meta --from 17composes, becauseCHAIN_TERMINUS_MAJOR= max(PROTOCOL_MAJOR,MIGRATION_MAJORS) = 18 andMIGRATION_MAJORS= [17, 18].composeMigrationChain(16, 17)no longer lists it.ui-action-group-menu-members-typedis listed by(17, 18)."@objectstack/spec": patchnext to"@objectstack/plugin-approvals": minor. Every other line is unchanged: theClause-②: no (narrowing)line, FROM → TO, the author's fix, the admin handling, and the markerregistered approval-position-address-role-retired.check-changeset-no-major --base origin/mainexits 0 withpatch.check-adr-0087-registration --base origin/mainexits 0: the id resolves at HEAD and is new in the diff.packages/qa/dogfood/test/fixtures/my-pending-position-fixture.ts(about :10) no longer says "under both approver-address spellings". It now says thatposition:POSITIONlists the request and the retiredrole:POSITIONspelling (ADR-0090 D3) does not. The change is comment only.ApproverTypealias, no contract docblock, no stored-slot rewrite, and no plugin-approvals code.origin/main025008aewas merged atd0a53cc6. The merge was clean, and main brought no change topackages/spec.d0a53cc6:check:migration-registry,check:spec-changes,check:upgrade-guide,check-adr-0087-registration --base origin/mainandcheck-changeset-no-major --base origin/mainall exit 0.dispatch-gatesderives 116 commands.check:generateddropped out, because no generated artifact is in the diff any more. All 116 exit 0, and--ranreads 116 derived, 116 run, 0 NOT-MEASURED, 0 UNRUN.d0a53cc6:sql-driver-query-signature: 15 passed.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 at3f4333ef), the full spec suite, and the CLI integration-tier ledger readers.d0a53cc6. The seat runs it once CI on this head has settled; landing waits for its PASS.Generated by Claude Code