Skip to content

fix(plugin-approvals): every slot reader takes the caller's acting addresses — can_act, the decision slot test, the already-acted probe and the email-keyed slot - #21410

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-21379-position-address-readers
Oct 2, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-21379-position-address-readers

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #21379
Clause-②: no

What changes

PR #21378 moved the slot-address equivalence into packages/plugins/plugin-approvals/src/approver-address.ts and pointed the "My Pending" filter and the participant gate at it. Four readers still compared a slot with the bare user id. Each now takes the caller's acting addresses from that module. That set is the user id, the email the caller's own sys_user row carries, and both spellings of each position in the server-resolved context.positions.

approver-address.ts gains three functions, and there is still one fold:

  • actingAddresses(caller) returns the default actor's addresses, in the order a slot is taken: user id, account email, then position:P and the deprecated prefix for each held position.
  • actorAddresses(actorId, caller) covers one resolved actor. The caller's own user id expands to actingAddresses. A named position address expands to both spellings of that position. Anything else, such as a named email or a machine actor, matches only itself.
  • heldSlot(pending, actorId, caller) is the slot test. It returns the first of those addresses that the slate holds.

The readers:

reader before now
attachViewers' viewer.can_act pending.includes(uid) heldSlot(pending, uid, caller), the same call the decision methods make with no actorId
decision slot test: decideNode, sendBack, reassign, requestInfo, comment pending.includes(actorId) takenSlot(), which wraps heldSlot
visibleRequestIds, already-acted probe actor_id: uid actor_id in actingAddresses(caller)
visibleRequestIds, current approver user id plus position addresses actingAddresses(caller), which adds the email half (item 4)

resolveActor is unchanged. It still admits exactly the identities it admitted before, and the #21350 oracle pin still holds. Nobody new may decide. The holder could always decide by naming position:P. Now the default actor and the console's spelling reach the same slot. A user who holds a different position is still refused: 403 at the route, pinned in the unit and dogfood suites.

What sys_approval_action.actor_id records. A slot-gated action records the slot it took, in that slot's stored spelling: position:P on a position:P slot (whichever spelling the caller used), the deprecated-prefix literal on a 15.x slot, and the email on an email slot. Naming that slot has always recorded exactly this; the measured row on main is approve:position:m21379_reviewer. It must stay this way because the multi-approver tally in decideNode and the decision_progress enrichment count approvals by matching actor_id against the slate. A decision recorded under any other spelling would leave its slot pending after its holder approved, so a unanimous request would never finalize. The tally pin goes red if this changes. The cost is noted under Acceptance notes.

#21387 (retiring the deprecated prefix) is not addressed here.

Measured on a booted app (bootStack, real /api/v1/approvals routes)

The fixture opens a request on a position nobody holds, then staffs the holder. Readings were taken with a scratch measurement test, which was not committed. The committed dogfood pin below replaces it.

request main at ecb6ca025 this branch
holder: viewer.can_act false true
holder: approve, no actorId 403 FORBIDDEN 200 approved
holder: approve, actorId with the deprecated prefix 403 FORBIDDEN 200 (dogfood pin, on its own request)
holder: approve, actorId: position:P 200 200 (unit pin)
holder: GET /requests/:id after deciding 404 200
bystander (holds another position): approve, no actorId 403 403

Item 4, the email-keyed slot: it reproduced and is fixed here. A user approver authored as an email stores the email as its slot. The table below is for the reviewer who owns that email and is not the submitter.

request main at ecb6ca025 this branch
"My Pending" (approverId = id, email) [] the request; can_act: true
GET /requests/:id 404 200
approve, no actorId 403 200
approve, actorId = the email 200 (decided above)
GET /requests/:id after deciding 404 200

Pins

  • approval-service.test.ts, describe every slot reader takes the acting addresses (#21379), has one pin per reader:
    • can_act as a table over holder, bystander, submitter and admin. It asserts that can_act equals "admitted as a slot holder" with no actorId, and that can_override covers the admin.
    • The decision slot test under the default actor, the deprecated prefix and position:P, each on its own request, with the recorded spelling. A 15.x slot keeps its own spelling. The bystander is refused under the default actor and under a named position.
    • The four sibling methods: holder admitted, bystander refused.
    • The unanimous tally consumes exactly the position slot.
    • The already-acted probe: the holder keeps sight of the request, the bystander does not, and a successor holder of the position does.
    • The email-keyed slot: listed, counted, can_act, decided by the default actor, and still in sight afterwards. Another account's email is a negative control.
    • The user-id slot is taken before a position slot.
  • approver-address.test.ts pins the three new pure functions, with negative controls.
  • approver-address-readers.test.ts is the enumeration pin:
    1. READERS lists every reader of the equivalence: acting path, list filter, participant gate, can_act, takenSlot, and the five decision methods. Each listed method must read its approver-address.ts export, which the pin checks through the TypeScript AST. It also checks that both participant-gate probes read the one acting set.

    2. The pin parses every non-test .ts file under src/ except approver-address.ts. It collects every site shaped like a slot comparison:

      • a membership call (includes, has, some, and so on) on a receiver whose text names a pending slate;
      • a slot column (approver, actor_id, pending_approvers) inside a where or filter predicate, or assigned to where.COLUMN;
      • a declarative field: 'pending_approvers' row under a filter.

      Every site must be classified in SLOT_SITES by file, enclosing function and exact text. Today there are 3 reader sites, 6 slot-vs-slot sites, such as a hand-off target or a token's bound slot, and 1 declarative site (see Acceptance notes). A new site fails with its location. So does a changed or vanished site. A planted-source case shows that each of the four shapes is detected.

      What the pin does not see: a comparison written in none of those shapes, such as a hand-written loop using ===.

Ablations

Each ablation is committed first and runs through node scripts/ablation-replace.mjs in WRAP mode. A belt trap restores from git checkout HEAD on an absolute path. Each mutation was checked on disk: anchor count went from 1 to 0, replacement count from 0 to 1, and the blob changed. Each restore was proven by blob == HEAD and an empty git diff HEAD.

The unit pins import src directly, and dogfood's isolated project aliases @objectstack/plugin-approvals to src/index.ts, so no dist leg applies.

Final run at dabba36f3:

ablation unit (3 files, 338 tests) dogfood (2 files)
A1: can_act back to pending.includes(caller.userId) 4 red: table, email, both enumeration tests red: holder can_act expected true
A2: takenSlot back to the literal actor 8 red red: holder approve 403
A3: already-acted probe back to actor_id: uid 3 red red: holder detail 404
A4: current-approver probe back to the user id alone 5 red, including both #21350 pins both red
A5: default actor widened to a non-held position (heldSlot takes any position: slot) 8 red: bystander rows, the bystander refusals, and 4 existing #3424 override pins red: submitter can_act became true

The first A5 attempt was refused by the tool: the replacement contained the anchor text, so the anchor count did not drop. No test ran on that attempt. It was redone with a replacement that does not contain the anchor. Every direction was red, as predicted.

Gates and tests, at dabba36f3

  • pnpm --filter @objectstack/plugin-approvals test: 56 files, 855 tests, all passed.
  • typecheck for plugin-approvals and dogfood: exit 0. check:test-typecheck holds 8 files and 324 errors in the ledger, with no new debt. All three new or edited test files are in tsconfig.test.json's program (checked with --listFilesOnly).
  • Dogfood (--project isolated): the new position-address-readers pin, 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 pin, approval-override-composite-pin and approval-snapshot-masked-field ran: 4 files, all passed.
  • dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derived 95 commands. All 95 exited 0 at dabba36f3, and --ran reconciled them as 95 derived, 95 run, 0 NOT-MEASURED, 0 UNRUN. Three earlier readings were superseded:
    • check-system-context-census was red on the first pass, because a new context.isSystem read in actingCaller had no row. Commit dabba36f3 removes that read, and the census holds at 114 sites.
    • check:skill-examples and check:dual-build-cjs-loads answered PREREQUISITE NOT MET until the eight packages without dist/ were built.
    • check:dual-build-cjs-loads then went red once on @objectstack/mcp's index.d.cts. That file is outside this diff, and its timestamp shows it was being written during the run. The rerun was green.
  • eslint --no-inline-config --format json on the 7 touched .ts files: 7 files, 0 errors, 0 warnings. eslint.config.mjs enables no type-aware linting (--print-config shows no parserOptions.project or projectService), so no untouched file's result can move. The repo-wide pnpm lint is left to CI.
  • origin/main was merged in as 8312775bd. The two incoming commits do not touch these files.

Docs

content/docs/automation/approvals.mdx: corrected the sentences this change made false.

  • the inbox and participant counts now include the account email;
  • "already acted" is counted by the same identities;
  • the decision paragraph now describes the default actor's slot and the recorded spelling;
  • the can_act bullet;
  • the admin-override callout's "no concrete user can act", which no longer holds for a staffed position:P slot.

"A position-addressed approver is never wrongly hidden" is now true and stays. The deprecated prefix is described in words, and check:role-word holds.

Acceptance notes

  • Admin who holds the routed position: this admin now decides as a slot holder (via_override: false, one vote in a multi-approver tally), exactly as when they named the slot. An admin who holds no slot is unchanged. The changeset says this.
  • Already acted belongs to the position: a decision recorded under position:P stays visible to every current holder of P. A former holder who decided it stops seeing it once they leave P. Pinned (successor) and documented.
  • Attribution: sys_approval_action.actor_id is a sys_user lookup, but for a position or email slot it holds the slot literal. The person who decided is not on the row. This was already true when the slot was named; the default actor now reaches it too. Reported to the seat.
  • Email casing: the default actor matches the account's email exactly as stored. resolveActor admits a named email case-insensitively. So a slot authored in a casing that differs from the account's stored email can be decided only by naming that exact spelling, as before, and the participant gate does not count it. NOT MEASURED through a door.
  • The my_pending list view in sys-approval-request.object.ts filters pending_approvers contains {current_user_id}. It is the one declarative slot-against-caller comparison, and metadata cannot read approver-address.ts. It is served only by the generic data door. On this fixture that door answers 403 PERMISSION_DENIED to a member without read on sys_approval_request. The admin read returned the email-slot row. It is classified declarative in the enumeration pin, so a second such filter goes red.
  • The spec contract's can_act TSDoc (packages/spec/src/contracts/approval-service.ts) still says "their user id is in the request's resolved pending_approvers". Its main sentence, "mirrors the exact check the service uses to authorize a decision", is now true. The parenthetical is narrower than the behavior. packages/spec was not edited under this order.

Generated by Claude Code

claude added 6 commits October 2, 2026 11:56
…dresses from approver-address

The default actor, can_act, the decision methods' slot test and the
already-acted probe now read the one equivalence in approver-address.ts
(actingAddresses / actorAddresses / heldSlot), so a holder of a position
whose slot reads position:P (or a user approver authored as an email)
decides it with the default actor or the console's role:P spelling,
sees can_act true, and keeps sight of it after deciding.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
… pin

approval-service.test.ts pins can_act as a table (holder, bystander,
submitter, admin), the decision slot test under the default actor and
both spellings (and the recorded slot spelling), the sibling decision
methods, the unanimous tally, the already-acted probe and the
email-keyed slot. approver-address-readers.test.ts lists every reader and
fails on a slot-against-caller comparison written outside
approver-address.ts.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…ed app

A position-literal slot staffed after open: can_act matches the default
actor's decision for holder, bystander, submitter and admin; the holder
decides with no actorId and with the console's role: spelling, the
decision records the slot's stored spelling, and the holder keeps sight
of it afterwards. An email-keyed slot is listed, flagged, decided and
kept in sight for its non-submitter reviewer.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…he caller's acting addresses

Correct the approvals page sentences this change makes false (the
decision's slot test, can_act, the participant and already-acted counts,
the admin-override callout's unstaffed-position line), and add the
plugin-approvals patch changeset.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…elevation read

actingCaller no longer branches on context.isSystem to skip the email
read: the only system context naming a user is the action link's, whose
token approver's own account is the identity the token proves. This keeps
the isSystem census at its declared 114 sites.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Oct 2, 2026
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-approvals, touching 28 documentable anchor(s).

28 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 6d67ad5eca0f1694b5e74effbc91e0f5e1634581.

⛔ 6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 4 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 — 6 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 6d67ad5eca0f1694b5e74effbc91e0f5e1634581 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 6d67ad5eca0f1694b5e74effbc91e0f5e1634581

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

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/xl tests tooling

Projects

None yet

2 participants