plugin-approvals: the submitter is never notified of the final decision — approving or rejecting a request emits no notification to them #24753

Activity

1. objectstack-fleet

   added

   area:workflowApprovals and automation — the work that runs without a person driving itApprovals and automation — the work that runs without a person driving it

   bugSomething isn't workingSomething isn't working

   domain:services

   pm:queue

   priority:p2Medium: important, M3Medium: important, M3

   on Oct 20, 2026
2. objectstack-fleet commented on Oct 20, 2026

   

   objectstack-fleetBot

   on Oct 20, 2026

   ContributorAuthor

   More actions

   Claim: PM loop round 6 · 2026-10-20T06:48Z
   Session: session_011TdZ8GjixNZFLz1xJD9zzK
   Account: os-justin (the seat's linked user as GET /user answers it; always the card's assignee)
   Branch: claude/issue-24753-approval-decision-notifies-submitter
   Worktree: objectstack-issue-24753
   Domain: domain:services
   Seat: domain:services#1
   File surface: packages/plugins/plugin-approvals/src/approval-service.ts (the approve and reject decision paths: one submitter-facing notification, on the shape approval.returned / approval.request_info already use), its tests, content/docs/automation/approvals.mdx (the topic list), and one new .changeset/24753-*.md. ⛔ Not service-messaging, ⛔ not packages/spec/**, ⛔ not objectui (stop on breach; explain in the report)
   Container & model: M, 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"; default because the recipient rules (self-decision, multi-step, auto-decided) are design judgement
   Clause-②: no
   Responsibility: plugin-approvals' approve and reject paths notify nobody of the final decision, while send-back and request-info already notify the submitter | no platform path covers it: the approval service is the only producer of approval topics | every submitter of an approval request, in every app with an approval process
   Thread-read: none
   Serial constraints cleared: No open PR touches packages/plugins/plugin-approvals or content/docs/automation/approvals.mdx (all 13 open PRs' files read). The epic's declared territory (#15194, body read at this act) names neither. #23256 (epic, pm:blocked) edits plugin-approvals' expandPositionUsers, a different function. #24751 (p3, approvals remind()) is not dispatched with this card.

   Readings taken on origin/main 46bb5e3e54.

   ---

   Generated by Claude Code
3. objectstack-fleet

   added

   pm:dispatched

   and removed

   pm:queue

   on Oct 20, 2026
4. objectstack-fleet

   assigned

   os-justin

   on Oct 20, 2026
5. objectstack-fleet commented on Oct 20, 2026

   

   objectstack-fleetBot

   on Oct 20, 2026

   ContributorAuthor

   More actions

   os-dev-report

   {
     "issue": 24753,
     "status": "done",
     "branch": "claude/issue-24753-approval-decision-notifies-submitter",
     "pr": "https://github.com/objectstack-ai/objectstack/pull/24816",
     "session": "session_011TdZ8GjixNZFLz1xJD9zzK",
     "premise_still_valid": true,
     "summary": "ApprovalService.decide() now tells the submitter the outcome on two new topics, approval.approved / approval.rejected, once the decision ends the approval process. The notice goes to the request's own submitter_id, in the shape the sibling notices use: the record named by nameSubjectRecord, a non-empty body with the decider's comment as its own paragraph, a deep link through notify(), the decider as actor, and the request's organization. Nobody is told on a co-sign vote that leaves the request pending, when the resume opened a further approval step on the same run (measured on the run, so a gateway that skips the later step still counts), when the submitter decided the request themselves, or for the SLA sweep's auto-decision; the escalation's own notifySubmitter notice stays the one message there, via a private decideAndResume(..., 'escalation'). Readings: H1 holds, and no approval notify() call site reads the caller's locale. H2 is refined: the status write is shared in decideNode, but the notice has to sit in decide() after the resume, because only the continuation shows whether the step was the last one. No topic registry is needed, and the preference resolver (exact topic or *) picks up both topics with no wiring.",
     "tests": "head 2a13930f15. Package suite: `bash scripts/pm/os-verify-lock.sh -c 'pnpm --filter @objectstack/plugin-approvals test && pnpm --filter @objectstack/plugin-approvals typecheck'` gave VERDICT command-exit 0: 'Test Files 87 passed (87) / Tests 1155 passed (1155)', and 'check:test-typecheck: OK — 8 file(s) / 324 error(s) / 27 pinned signature(s)', so the new file is unledgered and error-free. The new src/approval-decision-outcome-notification.integration.test.ts has 8 cases, run on the real ApprovalService over a real ObjectQL + SqlDriver (better-sqlite3 :memory:) with the real AutomationEngine, registerApprovalNode and registerLogicNodes: approved; rejected with a comment; self-decided with a control; unanimous co-sign; two-step ladder with no notice at step 1; reject at step 1; a gateway skipping step 2; SLA auto_approve under notifySubmitter true and false with a human control. src/approval-notification-body.integration.test.ts was extended so its every-call-site battery drives the new site (now 13 of 14 sites), and its body, record-name and organization pins cover both topics. Ablations: no build was needed, because the specs import ./approval-service.js from source. Each leg went through `node scripts/ablation-replace.mjs --file ABS --anchor A --replacement R -- vitest`, which reported anchor x1 to x0 and the blob changed (3382971460ce to a new blob); each restore used git checkout HEAD -- ABS and reported 'blob == HEAD (3382971460ce) and git diff HEAD is empty'; my own check afterwards read git diff HEAD = 0 bytes and porcelain clean. Results: (1) dropping the notifyDecisionOutcome call reddened 11 of 15 tests across both files, every negative case included through its positive control; (2) removing the self-decider guard reddened only the self-decided case; (3) removing the next-step guard reddened only the two-step case; (4) passing 'outcome' from the SLA sweep reddened only the SLA case. Logs are in the scratchpad (ablate-*.log).",
     "gates": "Local subset at head 2a13930f15. `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (no paths, merge base a43405d59, 5 paths) derived 98 commands; all 98 gave exit 0, each exit code captured right after redirecting the command's output to a log, before any pipe. `--ran` with ':: exit N' records answered: '98 derived famil(ies) accounted for — 98 run, 0 NOT-MEASURED (a DERIVED zero — all 98 recorded an exit code and none of them is 3)'. An earlier pass at 9827a640fb had check:skill-examples, check:dual-build-cjs-loads and check:i18n at exit 3 (PREREQUISITE NOT MET, nothing measured); after their prerequisites were built they gave exit 0, and again in the final pass. The same earlier pass had check:query-options-erasure at exit 1 (test surface 236 to 238, from my two any-cast query bags); 2a13930f15 types them, and the gate reads 236 and holds. No exported symbol was renamed or deleted, so check-platform-checklist was not owed. The CI check-runs on 2a13930f15, read once after opening: 'No other open PR may claim the same issue' success; 'Packed-tarball smoke (opt-in)' skipped; 22 more in_progress, among them Lint · ESLint, Lint · repo & tooling gates, Type Check · workspace / source gates / consumer gates / debt ledger, Check Changeset, Governed Surface Queue Guard, Spec property liveness, Check PR Size and 'The card this PR closes must claim this branch'. Test Core and Build Core were not yet listed. Convergence is left to the seat.",
     "mcp_calls": "0 — no MCP GitHub tool was called",
     "api_writes": "2 relay strokes, each one POST /repos/objectstack-ai/objectstack/dispatches executed by fleet-write as objectstack-fleet[bot]: (1) pr_create, which became POST /repos/objectstack-ai/objectstack/pulls (draft) and POST /repos/objectstack-ai/objectstack/issues/24816/assignees ['os-justin'], with no labels (run 39390230259, read-back reported 7793 bytes sent and stored identically, assignees os-justin); (2) comment, POST /repos/objectstack-ai/objectstack/issues/24753/comments (this report). git push is not REST: the empty-branch probe plus 4 commit pushes. Reads used were gh api single-card / PR / check-runs reads plus the public issue page payload.",
     "deviations": [
       "test:repo was not run locally: the diff edits no repo-project test, so it is left to CI (Test Core). The full-battery dogfood pin packages/qa/dogfood/test/approval-notification-body.dogfood.test.ts was not run either and is left to CI as well.",
       "To clear the three PREREQUISITE NOT MET gates I built under the lock: `pnpm --filter @objectstack/client-react build`, the turbo closure check:i18n names, and `pnpm exec turbo run build --concurrency=2 --filter='!@objectstack/docs'` (76 tasks, mostly cache hits, 26s held). That last one is wider than the 'targeted builds' rule, and I am declaring it.",
       "A runner script with a mis-resolved output path wrote two empty files (gates-results.tsv, gates-ran.txt) and an empty gates/ directory into the worktree root. I deleted them before any commit; they were never staged.",
       "Ablations: beyond the dispatch's 'drop the new notify call' leg, I ran three guard ablations (self, next-step, escalation switch) so that each negative rule is shown able to fail on its own.",
       "A throwaway probe spec (zz-probe-24753.test.ts) measured step-2's submitter_id. It was deleted before the first commit and never committed.",
       "Changeset: patch under AGENTS.md Post-Task Checklist step 3 ('A bug fix in a released package takes a patch changeset'). The Clause-② line was copied as dispatched ('Clause-②: no'); the conflict with the minor/widening precedent is in open_questions.",
       "Attribution: the commits carry the AGENTS.md model-free trailer pair (Claude-Session + Co-authored-by: Claude) rather than the harness reminder's model-named Co-Authored-By, and the PR body ends with the AGENTS.md session-URL footer rather than the harness's footer. The harness reminder defers to the repo's own rule.",
       "Worktree cleanup (rm -rf node_modules, then git worktree remove ../objectstack-issue-24753) is the step right after this comment, because the relay tool runs from the worktree."
     ],
     "files_changed": [
       ".changeset/24753-approval-decision-notifies-submitter.md",
       "content/docs/automation/approvals.mdx",
       "packages/plugins/plugin-approvals/src/approval-decision-outcome-notification.integration.test.ts",
       "packages/plugins/plugin-approvals/src/approval-notification-body.integration.test.ts",
       "packages/plugins/plugin-approvals/src/approval-service.ts"
     ],
     "open_questions": [
       {
         "question": "Topic spelling: one topic per outcome, or one topic carrying the outcome in its payload? (axes: business need · long-term soundness · AI-error-proofing · startup scope)",
         "options": [
           "A (implemented): approval.approved + approval.rejected. Business need: measured, PreferenceResolver matches the exact topic or *, and email/SMS templates are keyed per topic, so only A lets a tenant or user mute or template one outcome. Long-term: matches the vocabulary's grammar (requested / returned / escalated are what happened to the request, and approved / rejected are its terminal statuses). AI-error-proofing: the closed ApprovalNotificationPayload stays closed, with no outcome discriminant for consumers to parse. Startup scope: two topic names and no new surface type.",
           "B: approval.decided with an outcome in the payload. Needs the closed payload type widened (that type exists to keep stored payloads credential-free); consumers would branch on a payload field; no preference row can tell the two outcomes apart."
         ],
         "recommendation": "A: it wins on all four axes, and B's only gain (one name) costs the closed payload."
       },
       {
         "question": "Who is NOT notified? (axes: business need · long-term soundness · AI-error-proofing · startup scope)",
         "options": [
           "A (implemented): skip the self-decider (recordedActor(context) === submitter_id), and skip the SLA auto-decision, which escalateRequest already tells the submitter via approval.sla_breached under esc.notifySubmitter. The skip comes from a private decideAndResume(..., 'escalation') parameter, not from reading SLA_ACTOR_ID. Business need: a self-decider already knows, and the SLA case keeps one message that names the action. Long-term and AI-error-proofing: notifySubmitter: false stays a switch that does what it declares (declared means enforced). Startup scope: no new option.",
           "B: notify every terminal decision, the SLA one included. By default the submitter gets two messages for one auto-decision, and notifySubmitter: false would no longer silence the submitter about an escalation outcome, so the declared switch stops being fully enforced."
         ],
         "recommendation": "A: B breaks the declared notifySubmitter switch."
       },
       {
         "question": "What does 'final' mean on a multi-step process? (axes: business need · long-term soundness · AI-error-proofing · startup scope)",
         "options": [
           "A (implemented): after the resume, no pending sys_approval_request remains on the same flow_run_id. Business need: the showcase showcase_budget_approval is a real two-step flow whose gateway skips exec_review at or below 500000; the pin covers both routes. Long-term: measured on the run, never inferred. AI-error-proofing: there is no graph walk that a conditional edge could fool. Startup scope: one indexed read on finalised decisions only. Boundary, not measured: a subflow-hosted approval followed by another approval in the PARENT run is announced at the child's decision.",
           "B: notify per request (per step). The submitter reads 'approved' while the next step is still pending.",
           "C: static graph reachability (any approval node downstream). Wrong when a gateway skips the step; the submitter is never told."
         ],
         "recommendation": "A. The code models no per-step submitter notices, so 'never per step' holds."
       },
       {
         "question": "Who receives the terminal notice on a multi-step flow? Measured: a later step's request records the PREVIOUS step's decider as submitter_id. (axes: business need · long-term soundness · AI-error-proofing · startup scope)",
         "options": [
           "A (implemented here, upstream left unchanged): keep submitter_id as the one source, and fix the producer in its own card. approval-node.ts would pass the run's trigger user (service-automation runTriggerUserId) rather than the post-decision segment user. Business need: the original submitter, for example Mei on the showcase budget ladder, is the person the card is about. Long-term and AI-error-proofing: contract-first (spec ApprovalEscalationSchema.notifySubmitter describes 'the original submitter'), and one spelling of who submitted. Startup scope: one producer line plus its pins, in another card.",
           "B: here, resolve the process submitter as the run's first request's submitter_id. A consumer-side second answer to 'who submitted' that disagrees with the stored field; rejected by contract-first rule 5.",
           "C: accept that a later step's submitter is the previous decider (a reading of runAs ruling A) and document it. Leaves 'I Submitted', recall, resubmit, remind and every submitter notice on later steps addressed to a prior approver."
         ],
         "recommendation": "A: the producer is where the defect lives. It needs a maintainer decision, because the change moves submitter-only authorisation (recall / resubmit) on later steps."
       },
       {
         "question": "Do the existing preference and mute seams pick the new topics up with no wiring? (axes: business need · long-term soundness · AI-error-proofing · startup scope)",
         "options": [
           "A (implemented, measured): yes. PreferenceResolver resolves (user, topic exact or *, channel), defaults to on and fails open; mandatoryTopics is host-configured and does not list approval topics; service-messaging has no topic registry; objectui (local checkout 90518cd) groups arrivals by type generically. No new preference surface was built.",
           "B: declare the topics somewhere (a mandatory list or a registry). Nothing reads such a declaration; it would be a new surface with no consumer."
         ],
         "recommendation": "A."
       },
       {
         "question": "Clause-② conflict: the dispatch's 'Clause-②: no' (copied verbatim) disagrees with the precedent, whose changeset for the new approval.requested topic (#22607, .changeset/pre/22607-approval-opened-topic.md) declared 'Clause-②: yes (widening)' with minor. (axes: business need · long-term soundness · AI-error-proofing · startup scope)",
         "options": [
           "A (as shipped): 'Clause-②: no' + patch, read as a bug fix that adds observed output, not accepted input or a published API.",
           "B: re-declare 'Clause-②: yes (widening)' + minor, treating two new subscribable topic names as a widened public surface, as the precedent did."
         ],
         "recommendation": "Seat's call. The line is the claim's, and I have not changed it. If B is wanted, the edit is the changeset's Clause-② line plus its bump, and the PR body line 3; the PR body is the seat's to rewrite."
       }
     ],
     "out_of_scope_findings": [
       "class: a (also b) · reach: named real producer: packages/plugins/plugin-approvals/src/approval-node.ts:165 passes `submitterId: context?.userId ?? null`, and since resume-as-decider (commit 1b40780775, its .changeset/24321-resume-as-decider.md still pending, so unreleased) the post-decision segment's userId is the decider. It is exercised by the showcase flows showcase_budget_approval (manager_review then exec_review above 500000) and showcase_announcement_dynamic_approval (lead_review then co_sign). Seam: spec:ApprovalEscalationSchema.notifySubmitter (describe 'Notify the original submitter on escalation') → runtime:ApprovalService.escalateRequest audience raw.submitter_id. · evidence: on a real AutomationEngine + ApprovalService over SqlDriver with a two-node flow submitted by u_sub, step 1's request holds submitter_id u_sub; after u_mgr approves, step 2's request holds submitter_id u_mgr (measured on origin/main a43405d596). Consequences: on every later step, the I Submitted view (submitted_by_me), recall and resubmit (submitter-only), remind, and the submitter-facing approval.returned / request_info / sla_breached and now approved / rejected notices address the previous decider, not the person who submitted. · dedupe words: 'approval later step submitter_id previous decider', 'resumedBy second approval node submitter', 'multi-step approval submitter identity', 'approval-node submitterId context.userId', 'I Submitted later approval step missing'",
       "carrier: none (承接者:无) · noted in PR Acceptance notes, not filed: the ADR-0044 revision-limit auto-reject still tells the submitter on approval.returned ('Approval auto-rejected'), not on approval.rejected, so a mute of approval.rejected does not cover it",
       "carrier: none (承接者:无) · noted in PR Acceptance notes, not filed: the finality read covers the request's own run only; a subflow-hosted approval followed by another approval in the parent run is announced at the child's decision (boundary, not measured)",
       "carrier: none (承接者:无) · noted in PR Acceptance notes, not filed: an action-link decision records the comment 'Via action link', and the outcome notice carries it as the comment paragraph",
       "carrier: none (承接者:无) · noted in PR Acceptance notes, not filed: a decision whose resume strands sends no notice (the same as a stranded send-back), and continueRestoredRun does not send one later"
     ]
   }

   ---

   Generated by Claude Code
6. objectstack-fleet commented on Oct 20, 2026

   

   objectstack-fleetBot

   on Oct 20, 2026

   ContributorAuthor

   More actions

   Claim amendment · domain:services seat 1 · session_011TdZ8GjixNZFLz1xJD9zzK · 2026-10-20T08:06Z · amends claim 6285153236.
   Clause-②: yes
   The declaration moves from no to yes (widening). Two new subscribable notification topics, approval.approved and approval.rejected, are a widened public surface in this lane's own precedent: #22607 shipped approval.requested as Clause-②: yes (widening) and minor (.changeset/pre/22607-approval-opened-topic.md). A yes declaration is never wrong. The changeset moves to minor in a patch round on PR #24816, and the review owed on the declaration face (TIER_DEFAULT) is this seat's, recorded on the PR before the queue.

   ---

   Generated by Claude Code
7. objectstack-fleet commented on Oct 20, 2026

   

   objectstack-fleetBot

   on Oct 20, 2026

   ContributorAuthor

   More actions

   ACCEPT → PR #24816 comment 6287125357 · domain:services seat 1 · session_011TdZ8GjixNZFLz1xJD9zzK · 2026-10-20T08:46Z

   ---

   Generated by Claude Code
8. objectstack-fleet

   closed this as completedin #24816on Oct 20, 2026
9. objectstack-fleet commented on Oct 20, 2026

   

   objectstack-fleetBot

   on Oct 20, 2026

   ContributorAuthor

   More actions

   Landed · domain:services seat 1 · session_011TdZ8GjixNZFLz1xJD9zzK · 2026-10-20T09:07Z

   reading	result
   PR #24816 timeline	added_to_merge_queue 2026-10-20T08:48Z → merged 2026-10-20T09:07Z
   origin/main by content	squash 7282a950b1 (one parent, c3a8e07b6a). .changeset/24753-approval-decision-notifies-submitter.md and approval-decision-outcome-notification.integration.test.ts are present. approval-service.ts carries notifyDecisionOutcome, and the SLA sweep calls decideAndResume(…, 'escalation')
   closing references	the PR body closes #24753 only

   State change in this stroke: pm:dispatched off and the assignee cleared (the card closed completed on the merge). Follow-ups:

   • 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, filed at review: a later approval step records the previous decider as its submitter.
   • The four Acceptance notes items stay in ACCEPT 6287125357.
   • approvals: remind() reports notified per slot, not per person — an approver listed twice on a per-group slate counts as two while one reminder is delivered #24751 (remind()), serial behind this PR on approval-service.ts, is free to dispatch.

   ---

   Generated by Claude Code
10. objectstack-fleet

    removed

    pm:dispatched

    on Oct 20, 2026
11. objectstack-fleet

    unassigned

    os-justin

    on Oct 20, 2026