Skip to content

feat(spec): ApprovalActionRow declares acted_as, the slot an approval action was taken as - #21479

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-21458-approval-action-acted-as
Oct 2, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-21458-approval-action-acted-as

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #21458
Clause-②: yes

What this does

ApprovalActionRow in @objectstack/spec/contracts gains one optional member, acted_as?: string, declared next to via_override. This is the row type of an approval request's action log: IApprovalService.listActions returns it, GET /api/v1/approvals/requests/:id/actions passes the rows through as they are, and the client SDK types them.

The docblock takes via_override's form:

  • What it records. The pending-approver slot the action was admitted under, in the slot's stored spelling as it stood in the request's pending_approvers: a position:NAME address (or role:NAME, the deprecated pre-rename spelling), an email, or a user id.
  • That it is never a person. The person who acted is actor_id, which under ADR-0118 D1 holds a sys_user id or nothing. A slot addressed by a user id carries that id in acted_as as the slot's address. That makes no claim about who acted.
  • When it is absent. Either the action was not admitted through a slot (a submitter's own action, a system action, or an admin override, which via_override marks), or the row was written before the slot was recorded. So absent alone never proves that no slot was involved.

There is no authorable surface, no Zod schema, no request-side change, and no producer. Triage ordered this member to land first (#21411 ruling B, 5954022700; the pm:retriage answer Q1, 5960329631). The approvals card's PR then writes it from rowFromAction.

The member's name

The name is acted_as. It is read from the #21411 dev report 5959282267:

  • plan_on_yes.column: "sys-approval-action.object.ts gets acted_as: Field.text, maxLength 255";
  • open_questions[0]: "add one optional member, acted_as?: string, to ApprovalActionRow".

The seat's claim 5958740584 names no spelling ("a plugin-object field recording the slot"). Its branch claude/issue-21411-approval-actor-person holds no commit past 53fd35e3e, the empty claim marker. The card's spelling and the approvals side's spelling agree, so there is nothing to reconcile.

Files

  • packages/spec/src/contracts/approval-service.ts: the member and its docblock. The docblock names no tracker number.
  • packages/spec/src/contracts/approval-service.test.ts: the pins.
  • .changeset/21458-spec-approval-action-acted-as.md: @objectstack/spec minor, Clause-②: yes. Its level and header follow the precedent's spec changeset (.changeset/21333-spec-boolean-comparand-contract.md in 9f13c949b0).

Pins (the card's three)

  1. The member is optional and string-typed. Two exported type aliases, which check:test-typecheck compiles:
    • ActedAsIsOptional asks whether an empty object type extends the Pick of ApprovalActionRow on acted_as. An expectTypeOf equality against string | undefined cannot tell an optional member from a REQUIRED acted_as: string | undefined, and this alias can.
    • ActedAsIsAString asserts the indexed type ApprovalActionRow['acted_as'] equals string | undefined exactly.
  2. A row without it still type-checks. A row literal with no acted_as is assigned to ApprovalActionRow. No Zod schema exists for this row type (it is a TypeScript interface), so "parses" means type-checks. The existing producer (rowFromAction) and the client SDK reference the type only as ApprovalActionRow[]. No keyof ApprovalActionRow or Required of it exists anywhere in the repo.
  3. The api-surface row is present. api-surface/contracts.json carries ApprovalActionRow (interface), and export-origins/contracts.json carries its origin. Regenerating both after a real build changed no byte, because these artifacts record export names, not members. check:api-surface reports "public API surface + factory signatures unchanged". This gate cannot see a member being added, so the pins in 1 and 2 are what hold the member.

Ablation (one-shot, from the committed state)

Each mutation went through scripts/ablation-replace.mjs (WRAP mode, trap restore), with pnpm check:test-typecheck as the measurement:

  • acted_as?: string → acted_as: string (required): anchor 1 → 0, blob 6ed13aee9a82 → 04260649fc42. RED: "src/contracts/approval-service.test.ts: 5 type error(s)". It was restored with blob == HEAD 6ed13aee9a82 and git diff HEAD empty.
  • acted_as?: string → acted_as?: string | null: blob 6ed13aee9a82 → 675062e3d238. RED: 3 type errors. It was restored the same way.

The test file imports ./approval-service from src, so neither ablation goes through dist/.

Reverse check against the rebuilt .d.ts. This proves that a consumer reads the rebuilt type. A scratch module loaded @objectstack/spec/contracts from packages/spec/dist/contracts/index.d.mts (--traceResolution) and ran two legs:

  • acted_as: 'position:finance' is green.
  • acted_as: 42 is red with TS2322: Type 'number' is not assignable to type 'string'.

A stale .d.ts would have rejected both legs with an excess-property error instead.

Verification at cb86024a83

cb86024a83 is this branch with origin/main 49524f6906 merged in, which touched other spec contracts and no file of this diff. Every build and test ran under os-verify-lock, and each exit code was written to disk before any pipe.

command exit verdict line
pnpm --filter @objectstack/spec build (with the objectql and lint closures) 0 VERDICT command-exit 0
pnpm --filter @objectstack/spec check:generated 0 All 15 generated artifacts are up to date
pnpm --filter @objectstack/spec check:api-surface 0 public API surface + factory signatures unchanged
pnpm --filter @objectstack/spec test 0 Test Files 602 passed (602), Tests 17738 passed, 1 todo
pnpm --filter @objectstack/spec typecheck 0 check:test-typecheck: OK, 52 files / 246 errors / 135 pinned signatures held
vitest run src/contracts/approval-service.test.ts (at 828d3f2db3; the full suite above includes it) 0 7 passed (6 before this diff)
node scripts/check-changeset-no-major.mjs --base origin/main 0 This diff introduces no major bump
the same, with --event carrying this PR body 0 LEVEL AXIS: this PR declares clause-② yes, and no package whose packages/**/src/** it moves is graded patch
node scripts/check-adr-0087-registration.mjs --base origin/main 0 this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen)
node scripts/check-empty-changeset.mjs --base origin/main 0 No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added)
pnpm check:doc-authoring 0 17285 customer-facing string(s) across 1236 spec sources clean
pnpm check:nul-bytes 0 scanned 9810 text file(s) ... no raw ASCII control bytes
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran FILE 0 85 derived famil(ies) accounted for — 85 run, 0 NOT-MEASURED

The derivation (--commands, no paths) gave the same 85 commands at 828d3f2db3, d576d5b4c7 and cb86024a83, and all 85 exited 0 at cb86024a83. In the first pass (d576d5b4c7), two gates exited 3 (PREREQUISITE NOT MET):

  • check:lean-entry-closure passed after an objectql-closure build.
  • check:dual-build-cjs-loads passed in the final pass.
    The dists that the final pass of check:dual-build-cjs-loads read were partly built by an earlier gate from the tree before the merge. This diff emits no JavaScript (an interface member), and CI builds fresh.

origin/main has since moved one more commit (aa4632235b, a page-block spec change). It touches no file of this diff and was not merged in. CI runs on the merge ref.

Acceptance notes

  • The window before the producer lands. Until the approvals card's PR merges, no producer writes acted_as, and every row omits it. That is the member's declared absent case. On slot-gated rows, actor_id still holds the slot literal. That is the ADR-0118 D1 violation the approvals card exists to remove, and it is not touched here (the card forbids a producer change). The docblock describes the ruled contract. The changeset says that the approvals service's own changeset states when listActions starts returning the member.
  • No REST or client edit. REST passes listActions rows through as { data: rows }, and packages/client re-exports the spec type. No gate asked for an edit.
  • The DTO's actor_id has no docblock. The ADR-0118 D1 description the approvals card plans is for the object field (sys-approval-action.object.ts), not for this row type. Noted, not filed. Carrier: none.
  • A contract-tier review is owed before the queue, as the claim says (a published response contract widens).

Generated by Claude Code

claude added 3 commits October 2, 2026 20:40
… action was taken as

One optional string member on the published action-log row type, beside
via_override: the pending-approver slot the action was admitted under, in
the slot's stored spelling. It is never a person (that is actor_id, per
ADR-0118 D1). Absent means not admitted through a slot, or not recorded.

Pins: optional (a type alias that a required member turns red), string-typed,
and a row without it still conforms. Changeset: @objectstack/spec minor.

Claude-Session: https://claude.ai/code/session_01YDt3PzwfrkuFzUBF89WPmM
Co-authored-by: Claude <noreply@anthropic.com>
… do, and say what absent cannot prove

Claude-Session: https://claude.ai/code/session_01YDt3PzwfrkuFzUBF89WPmM
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added the size/s label Oct 2, 2026
@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

2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

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

Coarse fallback — 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 aa4632235ba571ef800b95e6bc18d00a30aa1d57 → packageMentionDocs.

Which tree this was computed on

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

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

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

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

Read-only, against these inputs and nothing else: card #21458 (body, the claim 5961025256, the dev report 5962451577); the #21411 ruling context (ruling B 5954022700, the pm:retriage answers 5960329631, the dev report 5959282267); the precedent changeset .changeset/21333-spec-boolean-comparand-contract.md at 9f13c949b0; PR #21479 (body, file list, net diff against main); the head's check-runs. The tree at the head was read with git show; nothing was built, run or re-run, and no ablation was repeated.

① Derived judgments

The member — right. ApprovalActionRow (packages/spec/src/contracts/approval-service.ts) gains exactly one member, acted_as?: string, declared immediately after via_override as the interface's last member. Optional, string-typed, no null arm. Nothing else in the file moves (+23 −0, all inside the one interface and its docblock).

The docblock — right, sentence by sentence, against ruling B and the #21411 plan.

  • What it records. "The pending-approver slot this action was taken as: the slot's address in its stored spelling, exactly as it stood in the request's pending_approvers" — a position:NAME address, the deprecated role:NAME, an email, or a user id. That is plan_on_yes.column of 5959282267 ("the pending-approver slot the action was taken under, in its stored spelling") and ruling B's "the slot is its own declared fact, as via_override is". pending_approvers is the real request column (sys-approval-request.object.ts) and a member of ApprovalRequestRow on the same contract; the role: spelling is the 15.x-era one the approvals suite still pins (approval-service.test.ts, the role:sales_manager / position:sales_manager pair). The three slot kinds are the three the approvals: sys_approval_action.actor_id (a sys_user lookup) records the slot literal (position:<p>, or an email) instead of the deciding user, so the person who decided is on no column (ADR-0118 D1) #21411 measurement observed (position, email, user id). TRUE.
  • Never a person. "The person who acted is actor_id (a sys_user id or nothing, ADR-0118 D1)". ADR-0118 D1 at the head: a sys_user actor column carries an id or null, never a sentinel; ruling B: "actor_id is the person, and the slot gets its own column". The rider — a slot addressed by a user id carries that id here as the slot's address, "which makes no claim about who acted" — follows the plan (acted_as: slot ?? null, and a user-id slot IS the user id) and ruling B's reason against option C (one person can hold several slots). TRUE.
  • When absent. "Not admitted through a slot (a submitter's own action, a system action, or an admin override — see via_override), or the row was written before the slot was recorded." plan_on_yes.column: empty on actions no slot gated and on an admin override; plan_on_yes.writes: submit, cancel, escalate and dead-run are unchanged. The second arm is what covers the historical user-id-slot rows the narrow backfill (retriage Q3, option A — no guessed slots) deliberately leaves unstamped, so "absent alone never proves that no slot was involved" is TRUE, and it is the sentence a consumer needs. The form is via_override's own closing paragraph ("not recorded" is not the same claim).
  • One precision note, not a fault: after approvals: sys_approval_action.actor_id (a sys_user lookup) records the slot literal (position:<p>, or an email) instead of the deciding user, so the person who decided is on no column (ADR-0118 D1) #21411's backfill Pass 1, a historical override row whose actor_id held a NAMED literal (the slot ?? actorId arm measured in 5959282267) will carry that literal in acted_as with via_override set. The docblock states what absent MEANS; it does not claim an override row is always absent, so no sentence is falsified. Any sharper wording on that one case belongs in approvals: sys_approval_action.actor_id (a sys_user lookup) records the slot literal (position:<p>, or an email) instead of the deciding user, so the person who decided is on no column (ADR-0118 D1) #21411's PR, which writes the member.
  • The source docblock names no tracker number; its one citation is the ADR, which is the sanctioned provenance.

The name — right. acted_as, read from where the ruling context puts it: 5959282267 plan_on_yes.column ("sys-approval-action.object.ts gets acted_as: Field.text, maxLength 255") and open_questions[0] ("add one optional member, acted_as?: string, to ApprovalActionRow"); ruling B allows "acted_as or the claim's name"; the card fixes it to acted_as. The DTO member and the planned object field share one spelling, so #21411's rowFromAction maps column to member without a rename. snake_case matches the sibling members (actor_id, via_override).

The pins — right, and the type-level pins really distinguish optional from required-with-undefined.

  • ActedAsIsOptional asserts that the empty object type extends the Pick of ApprovalActionRow on acted_as. The empty object type extends an object type whose only property is optional, and does NOT extend one whose only property is required (the property is missing), even when that property's type includes undefined. So this alias is the one that tells acted_as?: string from a required acted_as: string | undefined; the test's own comment is right that expectTypeOf equality against string | undefined cannot, because the indexed access type is string | undefined in both shapes.
  • ActedAsIsAString asserts the indexed type ApprovalActionRow['acted_as'] equals string | undefined exactly; it refuses boolean, an object, and a | null arm.
  • The row literal withoutSlot, typed ApprovalActionRow with no acted_as, is the "a row without it still conforms" pin — and the right reading of the card's "still parses": no Zod schema exists for this row type anywhere at the head (it is a TypeScript interface; git grep finds no *Schema, no keyof and no Required of it), so a type-check is the only parse there is.
  • The aliases evaluate, so this is not a phantom check: packages/spec's typecheck runs check:test-typecheck --project tsconfig.test.json, whose include reaches src/**/*.test.ts; src/contracts/approval-service.test.ts is NOT in test-typecheck-debt.json; the aliases are exported so noUnusedLocals does not strike them.
  • Ablation, as the dev report and PR body record it (not repeated here): required acted_as: string → RED, 5 type errors; acted_as?: string | null → RED, 3 type errors; both through scripts/ablation-replace.mjs with blob hashes and a git diff HEAD empty restore. Both directions are the ones the alias logic above predicts. The reverse check against the rebuilt .d.ts (acted_as: 42 refused with TS2322, a string accepted) rules out a stale-dist reading.
  • The shared Eq / Assert helpers move up the file so both pin blocks use them; the [#15389] block still compiles against the same definitions. Net zero.

The api-surface claim — right. packages/spec/scripts/build-api-surface.ts records "one name (kind) row per DECLARED KIND of every export", plus a signature hash for the defineX factories only; an interface member is invisible to both artifacts by design. The card's pin "the api-surface row is present" is met by the existing row ApprovalActionRow (interface) in api-surface/contracts.json, with its origin in export-origins/contracts.json; regeneration is a zero-byte diff and check:api-surface saying "unchanged" is the expected reading, not a gap. The type pins therefore carry the member, as the dev says, and the dispatch's expectation that "the regenerated artifact carries it" was the wrong one — the dev was right to say so rather than fake a row.

No producer, REST or client edit — right. The file list is three files: the contract, its test, one changeset. git grep acted_as at the head hits only those two spec files; rowFromAction, the REST route and packages/client are untouched, as the card and the claim order. No authorable surface, no Zod schema, no request-side type moves.

② Semver level

@objectstack/spec: minor, Clause-②: yes, no direction arm — right. Adding an optional member to a published response contract widens the public surface (强制条款², the card's and the retriage's reading), which takes at least minor and is not breaking, so no ADR-0087 marker is owed. The changeset is the precedent's shape line for line: 9f13c949b0's .changeset/21333-spec-boolean-comparand-contract.md is "@objectstack/spec": minor, a feat(spec): header, Clause-②: yes, then What it declares. and What moves for consumers. — this one has the same frontmatter, header form, declaration line and two sections, and its consumer paragraph says the right thing: nothing in this package writes the member, a row without it conforms exactly as before, and the approvals service's own changeset states when listActions starts returning it. The PR body's second line is Clause-②: yes, which the changeset level axis reads; the head's Check Changeset run is success.

③ Boundary flags

Implemented-by: claude/issue-21458-approval-action-acted-as
Reviewed-by: session_01YDt3PzwfrkuFzUBF89WPmM

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 2, 2026 22:45
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 2, 2026 22:45
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 2, 2026
Merged via the queue into main with commit 72217cd Oct 2, 2026
37 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-21458-approval-action-acted-as branch October 2, 2026 23:14
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/s tests tooling

Projects

None yet

2 participants