Skip to content

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

Closed

Description

@objectstack-fleet

Filing gate: ① a product defect with a named location and a live reproduction (finding class (a)).
Reader: the plugin-approvals lane (ApprovalService.reassign), next PR touching the approval decision doors.
Dedupe (open AND closed): "approval reassign to nonexistent user unresolvable principal accepted" → 4 hits, none on this (nearest: #22824, empty-slate admin rescue — a different path).
reach: public HTTP entry, measured — POST /api/v1/approvals/requests/:id/reassign {"to":"usr_does_not_exist"} answers 200 and strands the request.
Checklist item priority: P1
type: Bug
QA-source: #25479 · approvals.decision-action-matrix · observed beside the clauses

What happens

POST /api/v1/approvals/requests/ID/reassign accepts a to that resolves to no principal, answers 200, and puts the unresolvable address on the slate. The request then waits on nobody, and only an override actor can move it again.

  • As admin: POST /api/v1/approvals/requests/ID/reassign {"to":"usr_does_not_exist"} → 200, pending_approvers: ["usr_does_not_exist"].
  • As a plain pending approver (no override): POST …/EXP-2001-request/reassign {"to":"nobody-at-all"} → 200, with the same stranding.
  • Empty or missing to is correctly refused with 400 VALIDATION_FAILED.

Measured on main 77392e9, stock showcase.

Mechanism

ApprovalService.reassign (packages/plugins/plugin-approvals) checks only that to is non-empty and not already pending. It never resolves the address.

Slot addresses can legitimately be a user id, an email, or position:x. So the check has to be "this address resolves to at least one principal under the request's organization", not a user-id format test.

Expected: an unresolvable to is refused with a named 4xx that names the address, and the slate is unchanged.

Activity

  1. added
    area:workflowApprovals and automation — the work that runs without a person driving it
    bugSomething isn't working
    domain:serverDomain lane — seat card indexed by label:pm:seat
    and removed on Oct 23, 2026
  2. objectstack-fleet commented on Oct 23, 2026

    @objectstack-fleet
    ContributorAuthor

    Triage: lands in packages/plugins/plugin-approvals (ApprovalService.reassign); rationale: finding class (a), reach measured. reassign checks only that to is non-empty; it must resolve the address (user id, email or position:x) to at least one principal in the request's organization and refuse a named 4xx otherwise.
    Path: workflow · 审批的每个动作都由服务端状态机兜底 | approvals.decision-action-matrix (P1, passes for valid targets) | step ②
    Grade: priority:p2 — a pending approver can strand a request, but an override actor can still move it, so no data is lost.
    Type: Bug

    Session: session_01AkmasAfXt3cu83xGzkuKhe (objectstack triage, run on the maintainer's request in the project thread)


    Generated by Claude Code

  3. objectstack-fleet commented on Oct 24, 2026

    @objectstack-fleet
    ContributorAuthor

    Claim: card thread · domain:server (services seat 1, seat post #6021, project run by the Sales account) · p2 · the whole card · 2026-10-24T02:27Z
    Session: session_01HopxxRtFE6z3z91wUD54tp
    Account: os-sales (the seat's linked user; the card's assignee)
    Branch: claude/project-thread-h5z02d (the harness-pinned branch for this session; it names no issue, so the Duplicate Fix Guard's branch-name hint will not match — this claim is the identity record)
    Worktree: objectstack-issue-25485
    Domain: domain:server
    Seat: domain:services#1 (seat post #6021, which serves the server lane)
    File surface (read on origin/main ba9e0c374; stop on a breach and explain it in the report):

    • packages/plugins/plugin-approvals/src/approval-service.ts: ApprovalService.reassign resolves to before the audit write and refuses an address that resolves to no one with 400 VALIDATION_FAILED naming the address; the slate, the audit trail and the approver index stay unchanged. Resolution is one private helper: a user id or an email must match a sys_user row and must not be provably a member of other organizations only (the same sys_member screen the manager and team expansions apply); position:x must have at least one holder in the request's organization (expandPositionUsers); any other type:value literal names no address an actor takes (approver-address.ts) and is refused. The check runs after the authorization branch, so a caller who may not reassign learns nothing about the directory.
    • Tests: a new pin file in packages/plugins/plugin-approvals/src/ for the card's acceptance (refusal: status code + slate unchanged + no audit row; preservation: user id, email and position:x targets still move the slot). Any existing fixture in the package that reassigns to an address with no sys_user row is staffed with one and named in the PR.
    • One patch changeset for @objectstack/plugin-approvals, declaring the break (**BREAKING** banner, the (narrowing) arm, ADR-0087 disposition not-required (no-migration-prescription): no metadata key moves).
    • Not packages/rest (the route already maps VALIDATION_FAILED to 400; no new error code), not packages/spec, not the escalation reassign action (a separate, config-authored path).
      Container & model: M, card thread (in-session executor), model: default tier (dispatch-gates.mjs --tier on this surface: "no path-derived mandate"), effort: high; the narrowing owes one CONTRACT_REVIEW_TIER review before the queue
      Clause-②: no (narrowing)
      Metadata-protocol: none
      Basis for the Clause-② line: no because no published accept set or public surface widens and no error code is added (VALIDATION_FAILED is already this door's refusal). (narrowing) because the public door POST /api/v1/approvals/requests/:id/reassign stops accepting a to it answered 200 for; the full dogfood battery runs once on the final head. The request body is runtime input, not metadata an author writes, so Metadata-protocol: none takes patch.
      Responsibility: plugin-approvals (ApprovalService.reassign) produces the risk | no platform path covers it: the only exit for a stranded slate is the privileged override (admin rescue) | it reaches every pending approver and every override actor through the public route; measured on main 77392e9 in the card body
      Thread-read: 6370266628
      Serial constraints cleared:
    • No open PR declares packages/plugins/plugin-approvals (open PR search at claim time: chore: version packages #24876, refactor(plugin-security): move the authorization middleware out of security-plugin.ts (#25223 cut 12) #25545, docs(adr): the EXTERNAL rung admits explicit grants — ADR-0095, 0099, 0090 and 0135 record ruling 6355206139 B #25557, refactor(service-automation): move the run-context and resume bodies into run-resume.ts (#25222 slice 6) #25567, fix(pm): drop the cross-account review rule — the queue guard accepts a same-account Tier S record again, and contract-review.md stops requiring Reviewed-by-account #25585; none touches the package).
    • plugin-approvals: the admin_rescue system comment is written into the approval timeline as English prose, so a zh-CN viewer reads it untranslated #25471, which the coordinator expected to edit approval-service.ts next, is closed as a duplicate.
      Resume-from: none

    Generated by Claude Code

  4. objectstack-fleet commented on Oct 24, 2026

    @objectstack-fleet
    ContributorAuthor

    os-dev-report

    {
      "issue": 25485,
      "status": "done",
      "branch": "claude/project-thread-h5z02d",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/25592",
      "session": "session_01HopxxRtFE6z3z91wUD54tp",
      "premise_still_valid": true,
      "summary": "ApprovalService.reassign resolves `to` after the authorization branches and before the audit write, and refuses an address that resolves to no one with VALIDATION_FAILED (route: 400) naming it; slate, approver index, audit trail and notices stay unchanged. A user id or email must match a sys_user row and not be provably a member of other organizations only (the manager/team screens' sys_member posture); position:x needs a holder in the request's organization (expandPositionUsers); any other type:value literal is refused. approver-address.ts gains the internal addressedPosition(); no export, error code or route changes. Patch changeset declares the break (narrowing arm, ADR-0087 not-required (no-migration-prescription)).",
      "tests": "pnpm --filter @objectstack/plugin-approvals exec vitest run: 95 files, 1244 passed. New block in approval-service.test.ts (10 cases, refusal pins assert message + slate + index + no reassign row + no notice; preservation pins read the moved slate) and new dogfood pin approval-reassign-target-resolves.dogfood.test.ts (REST door on a booted stack: 4 refusals at 400 VALIDATION_FAILED, nothing moved, then a member reassign at 200 listed in their My Pending): 1 passed. Ablation with approval-service.ts written from origin/main: 6 refusal pins red, 4 preservation pins green (expected), dogfood pin red after rebuilding the plugin dist (expected 200 to be 400). Ordering ablation (check moved above authorization): the FORBIDDEN-first pin red. git status --porcelain empty after each restore. Fixtures staffed: approval-service.test.ts (staffDirectory helper, 11 call sites), admin-exemption-retired.test.ts and approval-override-audit.test.ts (sys_user u7 in beforeEach), actor-lookups-hold-ids.integration.test.ts (minimal sys_user_position, holder for position:finance). pnpm --filter @objectstack/plugin-approvals typecheck: pass (test-typecheck ledger held at 324). pnpm --filter @objectstack/dogfood typecheck: pass. eslint on the 7 changed source/test files: clean.",
      "gates": "dispatch-gates --commands (76 families) on 4eeb4690 (80e07fae later changed one test helper's typing only; the merge brought no change to these packages): 74 exit 0; node scripts/check-plugin-teardown-shape.mjs --self-test :: exit 3 and pnpm check:dual-build-cjs-loads :: exit 3 (prerequisites: shallow clone's pinned fixture commit, 7 dists), both exit 0 after git fetch of the fixture sha and building the dists. --ran: 76 derived, 76 run, 0 NOT-MEASURED, 0 UNRUN. CI on 00d5635ad9: 42 check-runs, 35 success, 7 skipped (by design on PRs: Console Pin Gate, Temporal Conformance, Dogfood shards, Dogfood Verify CLI, Build Docs, two opt-in harnesses), 0 failed; required set green: Lint & Repo Gates jobs, TypeScript Type Check jobs, Test Core 1-6, Build Core, Governed Surface Queue Guard.",
      "dogfood_battery": "00d5635ad9 · os-verify-lock: VERDICT command-exit 0 · held the lock 762s · Test Files 292 passed, 3 skipped (295) · Tests 2506 passed, 18 skipped (2524) · no failures",
      "mcp_calls": "7 — all reads: issue_read x4 (#25485 get + comments x2, #25471 get), search_pull_requests x1, pull_request_read x2 (#25439 get, #25592 get_check_runs); zero MCP write tools",
      "api_writes": "6 — all through the fleet-write relay: label-write (labels_add pm:dispatched, labels_remove pm:queue, assign os-sales) on #25485; post-stamped claim 6378753403; pr_create #25592 (assignee os-sales); pr_request_reviewers os-sales on #25592; the contract-review record comment on #25592; this report",
      "open_questions": [],
      "out_of_scope_findings": [
        "carrier: services seat 1, to file or cite · noted, not filed · class: a · reach: named producer (flow metadata `escalation.escalateTo` with `action: reassign`) · evidence: ApprovalService.escalateRequest expands escalateTo as a position and, on an empty expansion, writes the literal escalateTo onto the slate, so a nonexistent user id or an unstaffed position strands the request on timeout the way this card's door did · dedupe words: approval escalation reassign escalateTo unresolvable slate"
      ]
    }

    Contract review: CONTRACT_REVIEW_TIER record on head 00d5635, PR #25592 comment 6379337914, VERDICT PASS (narrowing face; it escalates the escalateTo fallback above to the lane seat).

    Notes: the unit pins went into the existing approval-service.test.ts instead of the claimed new pin file (no new engine double needed); the new file is the dogfood pin. The Docs Drift Check comment listed pages that mention sys_member or ApprovalService; none documents reassign target rules, so no doc changes.


    Generated by Claude Code

  5. objectstack-fleet commented on Oct 24, 2026

    @objectstack-fleet
    ContributorAuthor

    Landed · domain:services seat 1 (seat post #6021) · os-sales · session_016g7nYP9wdNx1GDa5XDr7Uy · 2026-10-24T03:54Z

    reading value
    PR #25592 MERGED through the queue (ACCEPT 6379381303), head 00d5635ad9
    origin/main carries a57643d3de

    State: the card closed completed through the PR; this stroke clears pm:dispatched and the assignee. The out-of-scope SLA escalation fallback is #25598: the domain:server lane has claimed the runtime leg, and the registration-time leg is pointed to the spec lane.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:workflowApprovals and automation — the work that runs without a person driving itbugSomething isn't workingdomain:serverDomain lane — seat card indexed by label:pm:seatpriority:p2Medium: important, M3

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions