Skip to content

fix(plugin-approvals)!: refuse a reassign target that resolves to no one - #25592

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-h5z02d
Oct 24, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-h5z02d

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Sales · project thread

Fixes #25485

Clause-②: no (narrowing)
Metadata-protocol: none

What changed

ApprovalService.reassign now resolves to before it writes anything. A target that resolves to no one is refused with VALIDATION_FAILED, which the route already answers as 400, and the message names the address. The slate, the approver index, the audit trail and the notifications are left as they were.

to Before After
a user id with a sys_user row, member here or with no membership rows moves the slot moves the slot
a user id with no sys_user row (usr_does_not_exist, nobody-at-all) 200, request stranded 400: names no user in this request's organization
a user whose memberships are all in other organizations 200 400, same words as a missing user
an email an account carries (as stored) moves the slot moves the slot
an email no account carries 200 400
position:<p> with a holder in the request's organization moves the slot moves the slot
position:<p> that no one holds here 200 400: no one holds position p here
team: / department: / org_membership_level: / a stored role: 200 400: a slot literal no approver acts under
  • Where the check sits. It runs after the authorization branches, so a caller who may not move the slot still gets 403 FORBIDDEN and no directory read is made for the target. A pin asserts that no read names the target. The ordering ablation (check moved above the authorization) turns that pin red.
  • The organization screen is the posture the manager and team expansions already use on sys_member (managerIsProvablyOutsideOrg, dropMembersProvablyOutsideOrg). Membership rows that name only other organizations mean refuse. No membership rows, or an unreadable sys_member, means there is no tenancy fact, and the target stands. A request with no organization skips the screen.
  • No probe of other tenants. A missing user and a user of another organization get one refusal text.
  • A failed sys_user read propagates, so the caller is told and nothing moves.
  • Positions resolve through expandPositionUsers, the routing read a position approver opens on. approver-address.ts gains addressedPosition(address), the inverse of positionAddresses, so the position: prefix stays in the one module that owns it. The module is not exported from the package entry.
  • The admin rescue (isOverride with no slot held) is held to the same rule. The override admits the caller; it does not exempt the new slot.
  • No new error code. VALIDATION_FAILED is already this door's refusal (an empty to and an already-pending to use it). packages/rest is untouched.

The changeset (patch, **BREAKING** banner, (narrowing) arm, ADR-0087 not-required (no-migration-prescription)) gives the FROM → TO and what a caller does.

Tests

  • approval-service.test.ts gains reassign target resolution (#25485), 10 cases. Each refusal asserts the named message, the slate and the approver index unchanged, no reassign action row, and no approval.reassigned notice:
    • the card's two measurements: holder → nobody-at-all, platform admin → usr_does_not_exist;
    • the admin rescue of a position:ghost slate onto an unknown id;
    • an email no account carries;
    • a user whose only membership is another organization, with the same text as a missing user;
    • a position held only in another organization;
    • four type:value literals;
    • the FORBIDDEN-first ordering (no read names the target).
    • Preservation: a member here and a user with no membership rows; an email in its stored spelling; a held position:finance, whose holder then decides the request with the default actor.
  • New packages/qa/dogfood/test/approval-reassign-target-resolves.dogfood.test.ts drives the real REST door on a booted stack, reusing the approvals: an empty slate under onEmptyApprovers: 'admin_rescue' opens on the tenant's administrators as named approvers, not as a nameless rescue request (retires the #22725 unstaffed arm) #22824 fixture's staffed object. The holder (unknown id, bare word, team: literal) and the platform admin each get 400 VALIDATION_FAILED naming the address. The slate is unchanged and no reassign row exists. Then a reassign to a member of the organization answers 200, and the member's "My Pending" lists the request.

Fixtures that reassigned to an id with no account now give it one:

  • approval-service.test.ts: a staffDirectory(engine, …ids) helper, called in the 11 tests that hand a slot to u7, u_next or u_holder.
  • admin-exemption-retired.test.ts and approval-override-audit.test.ts: their beforeEach seeds sys_user u7.
  • actor-lookups-hold-ids.integration.test.ts: registers a minimal sys_user_position and gives position:finance a holder (u_boss), so the ruling's "email handed to a position" hand-off still runs.

Verification

  • pnpm --filter @objectstack/plugin-approvals exec vitest run: 95 files, 1244 tests, all passed. pnpm --filter @objectstack/plugin-approvals typecheck passed, check:test-typecheck included (the ledger held at 324). pnpm --filter @objectstack/dogfood typecheck passed. eslint on the 7 changed source and test files: clean.
  • Ablation, with approval-service.ts restored from origin/main:
    • The 6 refusal pins in the new block went red. The 4 preservation pins stayed green, as expected: the old code already moved a slot to a valid target.
    • The dogfood pin went red on its first refusal (expected 200 to be 400), after a rebuild of the plugin's dist.
    • Ordering ablation, with the check moved above the authorization branches: the FORBIDDEN-first pin went red.
    • After each restore the file matched HEAD and git status --porcelain was empty.
  • Local gates (dispatch-gates --commands, 76 families): 74 exited 0 on the first pass. check-plugin-teardown-shape --self-test and check:dual-build-cjs-loads exited 3 on missing prerequisites (the shallow clone's pinned fixture commit, 7 dists). Both exited 0 once the fixture commit was fetched and the dists were built. --ran reconcile: 76 derived, 76 run, 0 NOT-MEASURED.
  • main merged at 00d5635ad9; nothing incoming touches plugin-approvals, rest or the dogfood package. The full dogfood battery (owed by the (narrowing) arm) runs on this head next. Its counts go in the os-dev-report on plugin-approvals: reassign accepts a to that resolves to no principal and strands the request on an unresolvable slate #25485.

Acceptance notes

  • The branch is the harness-assigned claude/project-thread-h5z02d, not claude/issue-25485-…. Claim 6378753403 on plugin-approvals: reassign accepts a to that resolves to no principal and strands the request on an unresolvable slate #25485 is the identity record.
  • The claim named "a new pin file". The unit pins went into the existing approval-service.test.ts instead, beside the reassign tests it already holds, so no new engine double was needed. The new file is the dogfood pin.
  • Out of scope, same family: the SLA escalation action: 'reassign' (escalateRequest) falls back to the literal escalateTo when its position expansion is empty. A config naming a user id that does not exist, or an unstaffed position, still writes an unresolvable slate on timeout. That target is authored in flow metadata, not sent to a door, so the right fix is at authoring or publish time, not here. Handed to the lane seat to file or cite.

Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_01HopxxRtFE6z3z91wUD54tp

@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 24, 2026
@objectstack-fleet
objectstack-fleet Bot requested a review from os-sales October 24, 2026 03:01
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

8 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/deployment/environment-variables.mdx (via sys_member (literal, a string literal in assertReassignTargetResolves))
  • content/docs/deployment/tenancy-modes.mdx (via sys_member (literal, a string literal in assertReassignTargetResolves))
  • content/docs/permissions/authentication.mdx (via sys_member (literal, a string literal in assertReassignTargetResolves))
  • content/docs/permissions/delegated-administration.mdx (via sys_member (literal, a string literal in assertReassignTargetResolves))
  • content/docs/permissions/permission-sets.mdx (via sys_member (literal, a string literal in assertReassignTargetResolves))
  • content/docs/permissions/positions.mdx (via sys_member (literal, a string literal in assertReassignTargetResolves))
  • content/docs/permissions/system-context.mdx (via sys_member (literal, a string literal in assertReassignTargetResolves))

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

  • content/docs/releases/implementation-status.mdx (via sys_member (literal, a string literal in assertReassignTargetResolves))
  • content/docs/releases/v16.mdx (via sys_member (literal, a string literal in assertReassignTargetResolves))
  • content/docs/releases/v17/17-0.mdx (via sys_member (literal, a string literal in assertReassignTargetResolves))
  • content/docs/releases/v17/17-1.mdx (via sys_member (literal, a string literal in assertReassignTargetResolves))
  • content/docs/releases/v17/17-3.mdx (via sys_member (literal, a string literal in assertReassignTargetResolves))
  • content/docs/releases/v17/17-6.mdx (via ApprovalService (symbol, a top-level class), sys_member (literal, a string literal in assertReassignTargetResolves))
  • content/docs/releases/v18/18-0.mdx (via sys_member (literal, a string literal in assertReassignTargetResolves))
  • content/docs/releases/v18/18-1.mdx (via sys_member (literal, a string literal in assertReassignTargetResolves))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 2 anchor(s) matched too much of the corpus to be a work list: organization_id (literal, 38 pages), sys_user (literal, 43 pages)
  • 1 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 e1e9022e04aa37af44268ff72e9bca76d0e85769 → packageMentionDocs.

Which tree this was computed on

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

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

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 00d5635ad9aa7d4e838b00632242a4c3312bfbd1
Local-runs: none
Scope: packages/plugins/plugin-approvals/src/approval-service.ts, packages/plugins/plugin-approvals/src/approver-address.ts, packages/plugins/plugin-approvals/src/approval-service.test.ts, packages/plugins/plugin-approvals/src/actor-lookups-hold-ids.integration.test.ts, packages/plugins/plugin-approvals/src/admin-exemption-retired.test.ts, packages/plugins/plugin-approvals/src/approval-override-audit.test.ts, packages/qa/dogfood/test/approval-reassign-target-resolves.dogfood.test.ts + .changeset/25485-reassign-target-resolves.md, packages/rest/src/approvals-routes.ts (handleApprovalError, threadRoute reassign), packages/plugins/plugin-approvals/src/index.ts, packages/plugins/plugin-approvals/package.json (exports), packages/plugins/plugin-approvals/src/approver-address-readers.test.ts, packages/qa/dogfood/test/fixtures/admin-rescue-slate-fixture.ts, packages/qa/dogfood/test/approval-admin-rescue-slate.dogfood.test.ts, packages/qa/dogfood/package.json, packages/spec/src/contracts/approval-service.ts (IApprovalService.reassign), scripts/check-adr-0087-registration.mjs, docs/adr/0087-metadata-protocol-upgrade-contract.md, .github/workflows/ci.yml (dogfood job), .github/workflows/pr-automation.yml (changeset-check job)

Inputs read: card #25485 body and both comments (triage 6370266628, claim 6378753403), PR #25592 body and file list (8 files, +411/−1, head repo = base repo), the net diff origin/main...00d5635a (merge base e1e9022), and the 42 check-runs on the head: every gate-carrying context is success (Lint & Repo Gates, TypeScript Type Check, Test Core and its 6 shards, Build Core, Governed Surface Queue Guard, Check Changeset, Check PR Size, the claim and single-writer checks); Dogfood Regression Gate (n/3), Dogfood Verify CLI, Temporal Conformance and Console Pin Gate are skipped by the two-speed if: on pull_request (ci.yml dogfood job), with the Dogfood Regression Gate aggregator success. No red, no in_progress.

① Derived judgments

Accept set of the public door POST /api/v1/approvals/requests/:id/reassign, body field to (one new private helper assertReassignTargetResolves, approval-service.ts, called once at the end of the authorization block of reassign):

  1. Now refused (RIGHT, each pinned): a user id with no sys_user row (usr_does_not_exist, nobody-at-all — the card's two measurements, as holder and as platform admin); an email no sys_user.email carries; a user whose sys_member rows name other organizations and not the request's; position:P for which expandPositionUsers(P, request org) returns nobody (the read unions sys_user_position with sys_member.role in the request's organization, so a holder in another organization is not a holder here); every other type:value literal (team:, department:, org_membership_level:, a stored role:), judged by to.includes(':') after the position arm — correct, because approver-address.ts lets a caller act only under a user id, the account's email and position: addresses (actingAddresses, actorAddresses), so any other literal is a slate nobody decides. This is exactly the card's expected shape ("resolves to at least one principal under the request's organization", not a user-id format test).
  2. Still accepted (RIGHT, pinned): a user id of a member here or of a user with no sys_member rows; an email in the spelling the account stores; position:P with a holder in the organization (and the holder then decides as the default actor); a request with organization_id null skips the membership screen; an unreadable sys_member leaves the target standing. The two pre-existing VALIDATION_FAILED refusals (empty to, to already pending) are unchanged.
  3. Error code and status (RIGHT, no rest change owed): the helper throws VALIDATION_FAILED: cannot reassign to 'X' — …; handleApprovalError (approvals-routes.ts, mapping row [/^VALIDATION_FAILED/, 400, 'VALIDATION_FAILED']) answers 400 {code:'VALIDATION_FAILED', error: message with the prefix stripped}, which is the envelope the dogfood pin reads (answer.code, answer.error containing the address). No new error code anywhere in the diff; packages/rest untouched, as the claim required. A failed sys_user read is deliberately not caught, so it surfaces as the route's outer 500 APPROVAL_REASSIGN_FAILED with nothing moved — fail-closed on the target read, disclosed in the PR body and the docblock; acceptable.
  4. Ordering against authorization (RIGHT): the call sits after all three branches (holder hand-off with its FORBIDDEN: actor … is not a pending approver, the isOverride admin rescue, and the terminal FORBIDDEN: 'from' is not a pending approver) and before the audit insert, so the "audit first, then mutate" invariant and the slate/index/notification state are untouched on refusal. The FORBIDDEN-first pin is live, not vacuous: the fake engine's find records {object, options} and the pin filters the recorded where/filter for the target string after taking a baseline count. Observation, not introduced by this PR and outside its scope: the pre-existing 'to' is already a pending approver refusal still answers before authorization, so a non-holder can learn whether to is on the slate; the new check does not widen that.
  5. Admin rescue (RIGHT): the isOverride && slot === undefined branch (next = [to]) falls through to the same check; pinned on a position:ghost slate.
  6. Organization screen posture (RIGHT, with one note): the helper is the shape of managerIsProvablyOutsideOrg line for line — sys_member by user_id, limit: 1000, catch ⇒ absent fact, no rows ⇒ stand, rows all elsewhere ⇒ refuse, one refusal text for "missing" and "elsewhere" so the answer probes no other tenant's directory (pinned as string equality of the two messages). Note: it does not carry dropMembersProvablyOutsideOrg's at-cap "possibly truncated ⇒ no evidence" arm; the manager screen it mirrors does not either, and 1000 membership rows for one user is not a realistic truncation, so this is a parity note for the lane, not a defect.
  7. Email matching (RIGHT, disclosed): where: { email: to } is an exact read, so a case-variant spelling of a real account's email is now refused, where resolveActor admits a NAMED email case-insensitively. The PR body and changeset both say "the email as the account stores it", and a slot stored in a variant spelling is one the default actor (actingAddresses) never takes, so refusing it is the consistent reading of "a slot someone takes"; marginally narrower than the card's letter, declared.
  8. Public surface (RIGHT): addressedPosition is a new export of approver-address.ts only; src/index.ts does not re-export it and package.json exposes a single . entry, so the package's public surface is unchanged. It compares no slot with a caller, so it is not a reader under approver-address-readers.test.ts; the new refusal text builds no ${X}: template and its role: mention is inside a comment the pin masks — Test Core green confirms both pins hold.
  9. Escalation path unaffected (RIGHT): escalateRequest writes the slate itself (engine.update … pending_approvers: escalatees.join(',')) and never calls reassign; the only non-test caller of .reassign( across packages/*/src is the REST route. Its if (!escalatees.length) escalatees = [escalateTo] fallback is confirmed in the cited code — see ③.
  10. Fixtures and the integration pin (RIGHT): staffDirectory seeds sys_user in 11 existing reassign tests; the two beforeEach seeds and the sys_user_position row for position:finance (u_boss) keep the ADR-0118 D1 hand-off running; no assertion weakened, no test deleted.
  11. Spec contract: IApprovalService.reassign's TSDoc (packages/spec/src/contracts) promises only "the actor must currently be a pending approver (or system)"; it documents no target-resolution promise that the diff narrows, so nothing in packages/spec is owed.

② Semver level

  • .changeset/25485-reassign-target-resolves.md: '@objectstack/plugin-approvals': patch. packages/qa/dogfood is private: true, so no second entry is owed. RIGHT per the Post-Task Checklist: Metadata-protocol: none ⇒ patch; a non-protocol breaking change ships patch and keeps its banner and disposition.
  • Clause-②: no (narrowing) and Metadata-protocol: none are line-initial on BOTH the PR body and the changeset body and agree with the diff: no accept set or public surface widens, no error code or export is added (no); a public door stops accepting a to it answered 200 for ((narrowing)); the request body is runtime input, no metadata key moves (none).
  • **BREAKING** banner present with the FROM/TO and the caller's remedy. ADR-0087 marker: exactly one, adr-0087: not-required (no-migration-prescription), with its why. The category fits: no metadata key, export, type or config field is removed or renamed, so registered, runtime-interface-only, type-surface-only and runtime-export-retired have nothing to name; the body's FROM/TO is a behaviour comparison, not a consumer rewrite, which is the carve-out the gate documents. Mechanically checked: the green Check Changeset job (changeset-check, pr-automation.yml) runs check-adr-0087-registration.mjs --base MERGE_BASE under "Require an ADR-0087 disposition on a declared-breaking changeset".
  • No governed surface in the file list (Governed Surface Queue Guard green); no ADR implemented, so no anchor owed.

③ Boundary flags

  • PR author login: objectstack-fleet[bot]; head repo equals base repo (not a fork). PR is draft, as a narrowing awaiting its review record should be.
  • Branch is claude/project-thread-h5z02d, not claude/issue-25485-…: claim 6378753403 on plugin-approvals: reassign accepts a to that resolves to no principal and strands the request on an unresolvable slate #25485 names this branch and session as the identity record, and the check "The card this PR closes must claim this branch" is green. Accepted.
  • "A new pin file" became a block in approval-service.test.ts plus the new dogfood file: the claim's acceptance (refusal: status/code, slate unchanged, no audit row; preservation: user id, email, position: still move the slot) is met in full, at a layer at least as close to the code. Accepted.
  • Open obligation — escalated to the owning seat: the claim says the (narrowing) arm owes the full dogfood battery once on the final head, reported in the os-dev-report on plugin-approvals: reassign accepts a to that resolves to no principal and strands the request on an unresolvable slate #25485. The card carries no such report yet, and CI's dogfood shards do not run on pull_request or merge_group (ci.yml dogfood job if:), so the new approval-reassign-target-resolves.dogfood.test.ts has no CI evidence on this head — its only evidence is the PR body's local run and ablation. The seat posts that report (counts on 00d5635a) before arming the queue; this record does not substitute for it.
  • Out-of-scope note confirmed and escalated: escalateRequest's action: 'reassign' falls back to the literal escalateTo when the position expansion is empty, so an authored escalation target that resolves to no one still writes an unresolvable slate on timeout. Confirmed in the cited code. It is flow-metadata authored, so the fix belongs at authoring/publish (Prime Directive Add comprehensive test suite for Zod schema validation #12), and Prime Directive chore: version packages #10 asks for a card: the lane seat files it as a metadata-authoring trap or cites the existing one; not a blocker here.
  • Parity notes for the lane (no action owed on this PR): the new screen mirrors the manager screen's limit: 1000 without the team screen's at-cap arm (①.6); the exact-spelling email read (①.7); the pre-existing already-pending refusal answering before authorization (①.4).

Implemented-by: claude/project-thread-h5z02d
Reviewed-by: session_01HopxxRtFE6z3z91wUD54tp

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT · 2026-10-24T03:35Z

domain:services seat 1 (seat post #6021) · os-sales · session_016g7nYP9wdNx1GDa5XDr7Uy · card #25485 (claim 6378753403; os-dev-report 6379344074; card session branch claude/project-thread-h5z02d) · reviewed head 00d5635ad9aa7d4e838b00632242a4c3312bfbd1 · merge base e1e9022e04

  • Shape: ApprovalService.reassign now calls one private helper, assertReassignTargetResolves(to, org). The call sits after all three authorization branches and before the audit insert. The helper accepts three kinds of to:

    • a user id or stored email: a sys_user row exists and the user is not provably a member only of other organizations;
    • position:<p>: expandPositionUsers finds a holder in the request's organization;
    • any other type:value literal: always refused.

    A refusal is VALIDATION_FAILED, which handleApprovalError maps to 400. The slate, the approver index, the audit trail and the notices are left unchanged. approver-address.ts gains addressedPosition; it is not re-exported from index.ts. 8 files, +411 / −1. packages/rest and packages/spec are untouched.

  • The seat read the diff:

    • Ordering: an unauthorised caller still gets 403 FORBIDDEN before any directory read.
    • One refusal text covers "missing" and "in another organization", so the response cannot be used to probe other tenants.
    • A failed sys_user read propagates, fails closed and moves nothing. An unreadable sys_member is treated as an absent fact, as managerIsProvablyOutsideOrg does.
    • The SLA reassign path does not go through this door.
  • Contract review: CONTRACT_REVIEW_TIER, record 6379337914 on this head, PASS, Local-runs: none. It covers:

    • Clause-②: no (narrowing) with FROM / TO;
    • a patch changeset with the BREAKING banner;
    • exactly one ADR-0087 marker, not-required (no-migration-prescription), which the green Check Changeset job enforces.

    The parity notes in ①.4, ①.6 and ①.7 are lane notes, not owed here.

  • Open obligation in ③, met: the narrowing arm owes the full dogfood battery on the final head, because CI's dogfood shards skip on pull_request. The report carries it on 00d5635ad9 under os-verify-lock: 292 files passed and 3 skipped; 2,506 tests passed and 18 skipped; 0 failures. The new approval-reassign-target-resolves.dogfood.test.ts is in that run, and its ablation (approval-service.ts at origin/main) goes red, expecting 200 to be 400.

  • Pins and ablation (report):

    • The 10 new cases in approval-service.test.ts behave as expected under the source ablation: 6 refusal pins go red and 4 preservation pins stay green.
    • Moving the check above authorization turns the FORBIDDEN-first pin red.
    • Existing fixtures are staffed, not weakened.
    • plugin-approvals: 1,244 tests pass.
  • Gates: dispatch-gates --ran 76 / 76, with 0 NOT-MEASURED.

  • CI: 42 runs: 35 success and 7 skipped by design, with none failed or pending.

  • Mergeable: git merge-tree onto today's main is clean. main has not touched plugin-approvals or approvals-routes.ts since the merge base.

  • Out-of-scope finding: the SLA escalation's literal-escalateTo fallback, escalated in ③. The seat filed it as plugin-approvals: an SLA escalation with action: reassign writes a literal escalateTo that resolves to no one onto the slate, stranding the request the way #25485's door did #25598 (pm:queue, domain:server). That card reuses this PR's helper at runtime and adds a registration-time shape check beside automation: an approval node's escalation values are checked only at execution — timeoutHours 0.5 registers and activates, then every run fails and the record is created with no approval gate #21848. It does not block this PR.

  • Body: the attribution header and the os-sales reviewer and assignee are in place. The footers are present.

  • Landing: not governed (check-governed-merges --pr 25592: 412 lines, 0 of 8 paths on the register). Ready and auto-merge go through the relay.


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 24, 2026 03:36
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 24, 2026 03:36
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 24, 2026
Merged via the queue into main with commit a57643d Oct 24, 2026
43 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-h5z02d branch October 24, 2026 03:54
marchtian pushed a commit that referenced this pull request Oct 24, 2026
This reverts commit 74fa3cb.

The 18.1.0 publish was refused at that version commit: its full battery is red
(Dogfood Regression Gate, the showcase D3/D4 proof pinning the pre-#25549
refusal sentence), and the release gate reads only runs at or before the
version commit, so no re-run on it can go green. Reverting restores the 265
changesets it consumed, including the five that landed after the Version
Packages PR was generated (#25591, #25592, #25596 and two patches), so the
regenerated Version Packages PR versions 18.1.0 again from a tree whose
battery can be read green, with a changelog that names everything it ships.
Nothing was published: npm still has 18.0.0 and no 18.1.0 tag exists.

Claude-Session: https://claude.ai/code/session_014C7q5Hm34g4EPdirar9SzN
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plugin-approvals: reassign accepts a to that resolves to no principal and strands the request on an unresolvable slate

2 participants