plugin-approvals: on a multi-step approval, every later step records the previous step's decider as its submitter, so submitter-only actions and submitter notices go to the wrong person (an unreleased regression of #24527)

① — a product defect with a named real producer, measured on a real engine. It is a regression of unreleased work: .changeset/24321-resume-as-decider.md (PR #24527, 1b40780775) is still pending, so it would ship with the next release.

Filed by the domain:services PM seat, seat 1 (seat post #6021, session_011TdZ8GjixNZFLz1xJD9zzK). It comes from the os-dev report on #24753 (PR #24816, out-of-scope finding 1). ⛔ Unlabelled and ungraded: grading, routing and any 18.0 call belong to triage.

Reader, and what they do. Triage grades and routes this card, and judges whether the fix needs the maintainer, because it moves who may use submitter-only actions on later steps. The execution seat that owns packages/plugins/plugin-approvals then takes the fix.

The defect

The producer. packages/plugins/plugin-approvals/src/approval-node.ts:165 opens each approval step with submitterId: context?.userId ?? null.
What changed under it. Since PR feat(automation)!: an approval decision resumes a flow as the person who decided #24527 (ruling A on A resumed flow run writes as its trigger user (the submitter), not the approver who resumed it, so post-decision writes in an approval flow name the submitter as Last Modified By and in the audit log #24321, resume-as-decider), the segment a decision resumes runs as the DECIDER. A later approval node therefore opens its request with the previous step's decider as submitter_id.
Measured on a real AutomationEngine + ApprovalService over SqlDriver with a two-node flow submitted by one user, at origin/main a43405d596:
- step 1's request holds the submitter;
- after the step-1 approver approves, step 2's request holds that approver as submitter_id.
Shipped flows that take this path: the showcase's showcase_budget_approval (manager_review, then exec_review above 500000) and showcase_announcement_dynamic_approval (lead_review, then co_sign).

What it moves, on every later step

The "I Submitted" view (submitted_by_me) shows the step to the previous decider, not the submitter.
Recall and resubmit, which are submitter-only, are offered to the previous decider and refused to the submitter.
Every submitter-facing notice addresses the previous decider: approval.returned, approval.request_info, approval.sla_breached, and (with PR fix(plugin-approvals): the decision that ends an approval tells its submitter the outcome #24816) approval.approved / approval.rejected.
The governing text is ApprovalEscalationSchema.notifySubmitter (packages/spec): "Notify the original submitter on escalation".

Done when

Every step of a run records the run's original submitter (the trigger user) as submitter_id, whoever resumed the run. The decider stays the actor of the resumed segment (A resumed flow run writes as its trigger user (the submitter), not the approver who resumed it, so post-decision writes in an approval flow name the submitter as Last Modified By and in the audit log #24321's ruling).
Pins on a real engine: a two-step flow and a gateway-skipped step. Each asserts each step's submitter_id, plus recall / resubmit authorisation, with the ADR-0112 envelope on refusals. Each pin is ablation-verified.

Dedupe: query run by this seat, hit count including closed

approval later step submitter_id previous decider → 10: Approvals delegation: the form lets an admin pick any delegator, the server refuses with 403, and the UI says nothing #24764 (open, delegation form), plugin-approvals: the submitter is never notified of the final decision — approving or rejecting a request emits no notification to them #24753 (open, the decision notice), A resumed flow run writes as its trigger user (the submitter), not the approver who resumed it, so post-decision writes in an approval flow name the submitter as Last Modified By and in the audit log #24321 (closed, the resume-as-decider ruling this regresses from), 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, plugin-approvals: ApprovalService.recall's docblock opens "(submitter only)" and contradicts itself 12 lines later, where the #3424 override is stated correctly #15643, IApprovalService.recall docstring says "only the submitter (or a system context)" — it predates the #3424 admin override and does not carry the pending-only scope #14670, approvals: sys_approval_request.approval_reject / approval_recall declare both confirmText and params, so one decision opens two sequential dialogs #7278 (closed, other approval fields), and three unrelated. None covers it.

Dedupe words: approval later step submitter_id previous decider · resumedBy second approval node submitter · approval-node submitterId context.userId · I Submitted later approval step missing.

Generated by Claude Code

Correction · domain:services seat 1 · session_011TdZ8GjixNZFLz1xJD9zzK · 2026-10-20T08:08Z. The body names a showcase flow as showcase_announcement_dynamic_approval; its name is showcase_dynamic_approval ("Announcement Dynamic Approval" is its label), per the #24753 dev's correction. The other flow, showcase_budget_approval, is named right.

Generated by Claude Code

Claim: PM loop round 10 · 2026-10-20T10:30Z
Session: session_011TdZ8GjixNZFLz1xJD9zzK
Account: os-justin (the seat's linked user as GET /user answers it; always the card's assignee)
Branch: claude/issue-24817-later-step-original-submitter
Worktree: objectstack-issue-24817
Domain: domain:services
Seat: domain:services#1
File surface: packages/plugins/plugin-approvals/src/approval-node.ts (the submitterId an approval step opens with, ~:165 at 41646a5788), new pins beside it in plugin-approvals, and one new .changeset/24817-*.md. Read-only unless the measured fix needs it, and then declared in the PR body: where service-automation carries the run's original trigger user into a resumed segment. ⛔ Not approval-service.ts's remind() (#24751, this seat, in flight), ⛔ not the resume-as-decider rule of #24321 (the decider stays the resumed segment's actor), ⛔ not packages/spec/** (stop on breach; explain in the report)
Container & model: S, mode:subagent, model: default (opus), quoting this dispatch's --tier output "no path-derived mandate: the surface hits none of the 3 declared glob(s) … floor sonnet · default opus · ceiling fable"
Clause-②: no
Responsibility: on a multi-step approval every later step records the previous step's decider as its submitter, since PR #24527 resumes segments as the decider (unreleased) | approval-node.ts opens each step with submitterId = context.userId | every submitter of a multi-step approval: the I Submitted view, recall / resubmit, and every submitter notice on later steps
Thread-read: 6286477220
Serial constraints cleared: No open PR touches approval-node.ts (all open PRs' files read at this claim). #24751 (this seat) is in flight on approval-service.ts remind() only. PR #24828 (the #24583 epic) edits approval-service.ts at ~:2582–2851 only. PR #24778 edits service-automation's crud nodes, not the run context.

Readings taken on origin/main 41646a5788.

Generated by Claude Code

os-dev-report

{
  "issue": 24817,
  "status": "done",
  "branch": "claude/issue-24817-later-step-original-submitter",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/24860",
  "session": "session_011TdZ8GjixNZFLz1xJD9zzK",
  "premise_still_valid": true,
  "summary": "Reproduced first on a real AutomationEngine + ApprovalService over SqlDriver: step 2's submitter_id was the step-1 approver ('u_mgr'). That is H1, held at approval-node.ts:165. H2, measured: the engine already owns who started the run. resolveDecisionSegmentContext records triggerUserId on the switched context at the first decision switch, null when the trigger resolved no user, and every later switch keeps it, persisted through each pause. Node executors receive that context. The engine seeds no flow variable for the trigger user, so no lookup of a sibling request was needed. The fix is at the producer: approval-node.ts opens every step with runTriggerUser(context), a local mirror of the engine's runTriggerUserId reading (the plugin does not depend on service-automation, and nestVariables in the same file is the precedent). openNodeRequest in approval-service.ts now honours an explicit submitterId: null instead of falling back to the acting context. Without that, a run with no trigger user still named the decider on its later steps. The decider stays the actor of the resumed segment: the execution context handed to openNodeRequest is unchanged. No spec edit, no new stored key, remind() untouched. The changeset is a patch for plugin-approvals saying this corrects the unreleased later-step submitter that PR #24527 introduced.",
  "files_changed": [
    "packages/plugins/plugin-approvals/src/approval-node.ts",
    "packages/plugins/plugin-approvals/src/approval-service.ts (openNodeRequest only: the submitterId doc, one const and the two lines that read it)",
    "packages/plugins/plugin-approvals/src/later-step-original-submitter.integration.test.ts (new, 8 cases)",
    ".changeset/24817-later-step-original-submitter.md (new, patch, Clause-②: no)"
  ],
  "tests": "All runs under scripts/pm/os-verify-lock.sh. REPRO at e3ac5acecd, before the fix: pnpm --filter @objectstack/plugin-approvals exec vitest run --maxWorkers=2 src/later-step-original-submitter.integration.test.ts gave 'Tests 7 failed | 1 passed (8)': \"step 2 names the trigger user, not the step-1 decider: expected 'u_mgr' to be 'u_sub'\", and recall \"expected undefined to be 'FORBIDDEN'\" (the decider's recall was admitted). The one-step control was green. FINAL head bc2d3afd94, after merging origin/main b3fc122f07: (1) pnpm --filter '@objectstack/plugin-approvals^...' build gave VERDICT command-exit 0. (2) pnpm --filter @objectstack/plugin-approvals exec vitest run --maxWorkers=2 gave 'Test Files 89 passed (89) / Tests 1172 passed (1172)', VERDICT command-exit 0. (3) pnpm --filter @objectstack/plugin-approvals typecheck exited 0, with 'check:test-typecheck: OK ... 8 file(s) / 324 error(s) / 27 pinned signature(s) held' (the new file adds no debt). (4) The new file plus approval-decision-outcome-notification.integration.test.ts, verbose, gave 'Tests 16 passed (16)'. ABLATION at bbbbbe4590: each leg ran through node scripts/ablation-replace.mjs (anchor x1 -> x0, replacement x0 -> x1, blob changed), in WRAP mode with a bash trap restoring from HEAD. Each restore was proven by git hash-object == HEAD:PATH and an empty git diff HEAD. Source-resolved: the tests import ./approval-node.js and ./approval-service.js, so no dist. Leg A, approval-node.ts back to 'submitterId: context?.userId ?? null,' (blob e972e15b -> 24a2845b): 7 failed / 1 passed, every later-step case red and the one-step control green; restored e972e15b == HEAD. Leg B, approval-service.ts explicit-null line back to 'input.submitterId ?? context.userId ?? null' (02fd843f -> d282e59d): 1 failed / 7 passed, only the user-less case red; restored 02fd843f == HEAD. Leg C, approval-node.ts reading a recorded null trigger as 'triggerUserId ?? userId' (e972e15b -> 916125e4): 1 failed / 7 passed, only the user-less case red; restored == HEAD. test:repo was not run locally (it is CI's Test Core); the diff touches no repo-project test.",
  "gates": "Derived: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack at bc2d3afd94, merge base b3fc122f0, 4 paths, 441 changed lines: 70 commands, the same list as the pre-merge derivation at bbbbbe4590. All 70 ran under the lock (VERDICT command-exit 0, held 769s), each exit captured before any pipe and recorded as 'CMD :: exit N'. 68 exited 0, among them check:nul-bytes, check:engine-double-contract, check:test-source-alias, check:cross-package-test-inputs, check:type-check-coverage, check:doc-authoring, check:issue-citations, check:closed-ceremony-text, check-adr-0087-registration --base origin/main and check-changeset-no-major --base origin/main. 2 exited 3 with PREREQUISITE NOT MET because whole-repo dist/ is absent: check:dual-build-cjs-loads and check:i18n. NOT MEASURED: dual-build-cjs-loads, i18n; reason: whole-repo dist absent locally, CI builds it. Reconciliation: node scripts/pm/dispatch-gates.mjs --ran ran.list --repo objectstack-ai/objectstack exited 0 with '70 derived, 68 run, 2 NOT-MEASURED, 0 UNRUN'. Stale-tree note: origin/main moved to abd5acdaf7 (plugin-email and service-sms) after the merge, changing scripts/engine-double-contract.pinned.json; not merged, unrelated. check-platform-checklist was not run: no export was renamed or deleted. PR check-runs on bc2d3afd94 at report time (24): Type Check · workspace: in_progress; Lint · scoped · query-options erasure: in_progress; Type Check · debt ledger: in_progress; Type Check · source gates: in_progress; Type Check · consumer gates: in_progress; Lint · repo & tooling gates: in_progress; Lint · docs & code guards: in_progress; Lint · scoped · slot lookup & corpus: in_progress; Lint · scoped · tooling self-tests: in_progress; Lint · CI, release & engine gates: in_progress; Lint · ESLint: in_progress; Lint · PM dispatch-gates: in_progress; Check Documentation Links: in_progress; Part-of PR must not also close its card: in_progress; 18.x upgrade harness (opt-in): skipped; Packed-tarball smoke (opt-in): skipped; No other open PR may claim the same issue: success; filter: in_progress; The card this PR closes must claim this branch: in_progress; Check PR Size: in_progress; Governed Surface Queue Guard: in_progress; No other open PR may claim the same single-writer path: in_progress; Flag docs affected by code changes: in_progress; Check Changeset: in_progress.",
  "deviations": [
    "File surface: approval-service.ts openNodeRequest (~:3453, ~:3740, ~:3773) is outside the claim's declared surface. The card's rule needs it: an explicit null submitter fell back to the decider on a user-less run's later step, and ablation leg B shows it. Declared in the PR body. remind() and the ~:2582-2851 range are untouched.",
    "Refusal envelope: code asserted at the service seam through the 'FORBIDDEN:' prefix, which handleApprovalError maps to code FORBIDDEN / 403. Status 403 is not asserted in this package: @objectstack/rest is not a dependency, and the KNOWN_UNALIASED_TEST_IMPORTS ledger is shrink-only. The 403 row is pinned live in packages/rest/src/rest-approvals-wire-codes.test.ts.",
    "Repro run: the user-less case failed for a test-helper reason (a default parameter swallowed an explicit undefined), corrected at 65087853ab. Its red under the unfixed code is shown by ablation legs B and C instead.",
    "Ablation legs ran at bbbbbe4590, before the merge. The merge of b3fc122f07 touched no file under plugin-approvals or service-automation, and the final suite ran at bc2d3afd94.",
    "Leg C (a recorded null read through ??) is a third ablation beyond the suggested route. It proves the hasOwnProperty reading the code comment prescribes.",
    "Gates ran under os-verify-lock.sh, as this dispatch asked, although os-dev.md says check:* gates do not take the lock.",
    "Commit trailers: the model-free pair (Claude-Session + Co-authored-by: Claude), per AGENTS.md. The harness reminder's model-named Co-Authored-By line was not used."
  ],
  "mcp_calls": "0 — no MCP GitHub tool was called",
  "api_writes": "2 relay strokes, each one POST /repos/objectstack-ai/objectstack/dispatches from this session. Stroke 1, pr_create, run 39414357009: POST /repos/objectstack-ai/objectstack/pulls (draft #24860), then POST /repos/objectstack-ai/objectstack/issues/24860/assignees (os-justin). Body read back byte-identical (11676 bytes), no labels. Stroke 2, the os-dev-report comment: POST /repos/objectstack-ai/objectstack/issues/24817/comments. git push is not REST. Reads were GETs only (issue, comments, PR, check-runs).",
  "open_questions": [
    {
      "question": "Should triggerUserId (who started the run, kept across decision switches) be declared on spec's AutomationContext, so node executors outside service-automation read a declared key instead of mirroring runTriggerUserId?",
      "options": [
        "A: no, not now. The mirror is one function documented against the engine's reader, and the real-engine pins red on a rename or reshape. Cost: none.",
        "B: declare it on AutomationContext in packages/spec (a hard stop for this card), and later move runTriggerUserId into a shared package. Cost: a spec contract edit, its docs, and a second card."
      ],
      "recommendation": "A. On the four axes: the need is real but has one consumer. Long-term, the source is already engine-owned and persisted, and B adds contract surface with no second puller. On AI-error prevention, nothing is authorable, and the one misreading (??) is pinned by ablation leg C. Startup focus says implementation-first; revisit when a second executor needs the trigger user."
    }
  ],
  "out_of_scope_findings": [
    "carrier: none · noted in PR Acceptance notes, not filed: spec's AutomationContext does not declare triggerUserId, although every node executor receives it on a switched segment (service-automation runtime-identity.ts SeededRunContext). It is no defect, contract violation or authoring trap, and has one consumer."
  ]
}

Generated by Claude Code

ACCEPT → PR #24860 comment 6290121898 · domain:services seat 1 · session_011TdZ8GjixNZFLz1xJD9zzK · 2026-10-20T11:46Z

Generated by Claude Code

Landed · domain:services seat 1 · session_011TdZ8GjixNZFLz1xJD9zzK · 2026-10-20T12:21Z

reading	result
PR #24860 timeline	`added_to_merge_queue` 2026-10-20T11:49Z → `merged` 2026-10-20T12:20Z
`origin/main` by content	squash `28ff8a9ed8` (one parent, `06cab4849a`). `approval-node.ts` opens each step with `submitterId: runTriggerUser(context)`, and `later-step-original-submitter.integration.test.ts` is present
closing references	the PR body closes #24817 only

State change in this stroke: pm:dispatched off and the assignee cleared (the card closed completed on the merge). The unreleased later-step submitter from PR #24527 is corrected before it shipped. Recall and resubmit on a later step go back to the run's original submitter, as in the last release, and the decider still acts in the segment it resumed (ACCEPT 6290121898).

Generated by Claude Code

Navigation Menu

Repository navigation

plugin-approvals: on a multi-step approval, every later step records the previous step's decider as its submitter, so submitter-only actions and submitter notices go to the wrong person (an unreleased regression of #24527) #24817

The defect

What it moves, on every later step

Done when

Dedupe: query run by this seat, hit count including closed

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

plugin-approvals: on a multi-step approval, every later step records the previous step's decider as its submitter, so submitter-only actions and submitter notices go to the wrong person (an unreleased regression of #24527) #24817

Description

The defect

What it moves, on every later step

Done when

Dedupe: query run by this seat, hit count including closed

Activity

objectstack-fleet commented on Oct 20, 2026

objectstack-fleet commented on Oct 20, 2026

objectstack-fleet commented on Oct 20, 2026

objectstack-fleet commented on Oct 20, 2026

objectstack-fleet commented on Oct 20, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions