Skip to content

fix(approvals): a recall ends the run without the "rejected" notice - #2353

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/project-thread-1fgwkb
Oct 28, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/project-thread-1fgwkb

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Irene · project thread

Fixes #2349

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:销售撤回自己的审批请求时,不再收到"已拒绝"通知。大额商机审批(经理、总监两级)、客户审批、线索转化审批三个流程都改了;审批人真正拒绝时照常通知。
  • 为什么改:平台把"撤回"沿审批节点的拒绝分支续跑,这三个流程的拒绝分支直接发"已拒绝"通知,于是没人拒绝过的请求也告诉销售"未获批准",站内信和邮件都发。
  • 风险与回滚:风险低,只改三个流程的拒绝连线,把它拆成按撤回与否分流的一对。读不到决定时按旧行为发通知,不会漏掉真拒绝。回滚即还原本 PR。
  • 先看:src/sales/flows/opportunity-approval.flow.ts(e4/e8 两对连线及注释);test/approval-recall-no-rejected-notice.test.ts;test/flow-variable-conditions.test.ts 的 binds() 一行。
  • 两分钟验证:pnpm exec vitest run test/approval-recall-no-rejected-notice.test.ts,预期 9/9 通过;在 main 上同一文件 4 个撤回用例失败,每个都收到一条"已拒绝"通知。
  • 需要你做的:无,席位落地。

What changes

@objectstack/plugin-approvals 18.2.0 ApprovalService.recall (dist/index.mjs:7357-7361) resumes the waiting run down the approval node's reject branch with output: { decision: 'recall', requestId }. An approver's rejection resumes the same branch with decision: 'reject' (decideAndResume, dist/index.mjs:7126 and :7139), and so does an auto-rejection (:7620). There is no separate recall label: APPROVAL_BRANCH_LABELS in @objectstack/spec 18.2.0 is approve / reject / revise / resubmit.

The flow can see the difference. @objectstack/service-automation 18.2.0 lands each resume output key as the flow variable NODE_ID.KEY (applyResumeSignal, dist/index.js:4224), celScope (dist/index.js:1480) nests dotted names under vars, and traverseNext (dist/index.js:1319) narrows to the out-edges carrying the resumed label and then evaluates their conditions. So an edge out of manager_review reads vars.manager_review.decision.

Each reject edge that notified is now a pair, both labelled reject, with conditions that partition in opposite polarity (the e5/e6 pattern):

Flow Notifies (decision is not recall, or cannot be read) Ends the run (decision is recall)
opportunity_approval and its insert twin e4, e8 → notify_rejected e4_recall, e8_recall → end
account_approval e4 → notify_rejected e4_recall → end
lead_conversion_approval e4 → notify_rejected e4_recall → end

A decision that cannot be read notifies, which is today's behaviour, so a platform change cannot silently drop a real rejection's notice. opportunity_qualification_approval and opportunity_status_change_approval route reject to clear_request (an update_record) and then end, with no notify node: confirmed, and left alone. The platform itself sends nothing on a recall either: ApprovalService.recall (dist/index.mjs:7271-7457) makes no notification call, unlike decide.

test/flow-variable-conditions.test.ts (the CLASS 2 "every variable a condition reads is bound" sweep) did not know that an approval node binds its own id for its out-edges, and flagged all twelve new edge reads. Its binds() now counts an approval node's id, with the engine citation; the end-to-end test below is the measurement that backs it. The lead gate's armed-install helper moved from approval-decision-attribution.test.ts into test/helpers/verify-stack.ts, since two suites now boot it.

Tests

test/approval-recall-no-rejected-notice.test.ts, new, 9 cases. Each drives the real door: POST /api/v1/approvals/requests/{id}/recall as the submitting rep, /reject (and /approve for the first tier) as a real Sales Manager or Sales Director. It reads the notices off the messaging outbox (sys_notification_delivery, the notificationsTo helper; notify writes them inside the resumed run) and the run's own node summary.

  • opportunity_approval: recall at Sales Manager review (e4), recall at Sales Director sign-off after the manager approved (e8): no opportunity_rejected notice, run completed, notify_rejected never ran. Manager rejection and director rejection: one notice each.
  • account_approval: recall sends no account_rejected notice; a rejection sends one.
  • lead_conversion_approval, on a boot with the gate armed (the one way an install arms it): recall sends no lead_conversion_rejected notice; a rejection sends one.

Mechanism assumption 2 measured: a real recall in account_approval and lead_conversion_approval took the same path as in the opportunity flow. On main each wrote exactly one rejected notice.

Before: the new file on main b8332aa, 4 failed / 5 passed
     × recalling at Sales Manager review (e4) sends no notice 360ms
     × recalling at Sales Director sign-off (e8) sends no notice 403ms
     × recalling sends no notice 211ms
     × recalling sends no notice 155ms
 Test Files  1 failed (1)
      Tests  4 failed | 5 passed (9)
AssertionError: expected [ { …(22) } ] to deeply equal []

Each failure is one sys_notification_delivery row, for example "actionUrl": "/crm_opportunity/KNuxJZgUSUYOzbuB" addressed to the rep.

After: 9/9
 Test Files  1 passed (1)
      Tests  9 passed (9)
Ablation: e4's notify condition set to true, on the fix commit

The notify edge's condition replaced by true (marker grep 1 hit), the opportunity block run, then restored with git checkout HEAD -- (git diff HEAD empty, blob hash equal to HEAD's):

     × recalling at Sales Manager review (e4) sends no notice 369ms
 Test Files  1 failed (1)
      Tests  1 failed | 3 passed | 5 skipped (9)

Only the e4 case goes red. The e8 case stays green because it reads its own edge.

Acceptance notes

  • File surface: the three flows, tests under test/, one changeset, as claimed. No docs change: content/docs/revenue/approvals.mdx § Recalled never said a recall notifies, so it is not false. Its last bullet there reads "The rejection is captured in the approval request's history" under Recalled, which looks like a slip for "the recall"; noted, not changed (outside the surface).
  • The auto-rejection path (decision: 'reject', autoRejected: true) still notifies, like an approver's rejection.
  • A recall in the revise window (a returned request) cancels the run instead of resuming it (plugin-approvals dist/index.mjs:7338-7354), so no edge runs; no HotCRM flow has a revise node today.

Verification

All on head 9e90106 (the fix plus a merge of origin/main 1f1ea12), local, exit code captured before any pipe:

Command Result
pnpm validate exit 0
pnpm typecheck exit 0
pnpm lint (os lint --strict) exit 0
pnpm hygiene, pnpm hygiene:tokens exit 0, exit 0
pnpm build exit 0
pnpm test (full suite) Test Files 237 passed (237) · Tests 4562 passed, 1 skipped (4563) · 688 s

The first full run, before the merge and before binds() learned approval nodes, was 4542 passed and 1 failed: the CLASS 2 sweep in test/flow-variable-conditions.test.ts, naming the twelve new edge reads. That file is 91/91 after the change. CI runs the same chain.

Generated by Claude Code

… three notifying approval flows (#2349)

Red on b8332aa: each of the four recall cases writes the flow's rejected
notice. The rejection controls pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8VVAkMkort1hh8jUGGbLB
The platform resumes a recalled request down the approval node's reject
edge with decision 'recall'. Split each notifying reject edge into a
partitioned pair on vars.NODE.decision: a recall ends the run, anything
else notifies as before.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8VVAkMkort1hh8jUGGbLB
@vercel

vercel Bot commented Oct 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
hotcrm Ignored Ignored Oct 28, 2026 4:20am UTC

Request Review

@github-actions github-actions Bot added ci/cd CI plumbing and the verification pipeline backend Server-side behaviour — hooks, flows, actions labels Oct 28, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

Verdict: ACCEPT · repo:hotcrm seat 1 · huangyiirene · session session_01J8VVAnPkgK6CWrJa6t7Fk5 · 2026-10-28T04:23Z · head 9e90106
Implemented-by: card thread session session_01J8VVAkMkort1hh8jUGGbLB (report 6460597444 on #2349) · Reviewed-by: the seat, against the 8-file diff and hotcrm origin/main 1f1ea12

  • Shape: the PR is a draft, based on main, assigned to huangyiirene, and its body reads Fixes #2349. No governed path is touched. The source diff covers the three claimed flows. The test-side additions are inside test/: the bound-variable guard now counts an approval node's own id, and armedLeadGate moved into the test helpers. git merge-tree against 1f1ea12 is clean.
  • Ruling held:
    • In opportunity_approval (e4 and e8), account_approval (e4) and lead_conversion_approval (e4), each notifying reject edge is now a pair. Both edges carry the label and their conditions are opposite. vars.<node>.decision == "recall" ends the run silently. Anything else, including a decision that cannot be read, still goes to notify_rejected, so a real rejection never loses its notice.
    • The qualification and status-change flows were confirmed silent on reject and left alone.
  • Mechanism assumptions:
    • 1 holds, with option A. @objectstack/plugin-approvals 18.2.0 resumes a recall down reject with decision = 'recall' (dist/index.mjs:7357–7361), and an edge condition reads it.
    • 2 holds. Measured on main, the new suite's 4 recall cases are red, one per flow plus the director step.
  • Tests:
    • test/approval-recall-no-rejected-notice.test.ts passes 9 of 9. It drives real recalls and has rejection controls that still notify.
    • The ablation of e4 turns it red.
  • Gates:
    • The full suite passed at 9e90106: 237 of 237 files, 4562 tests passed, 1 skipped. validate, typecheck, lint, hygiene and build all exit 0.
    • CI on 9e90106: 8 of 9 check runs are success. Build and Test (22.x) is still running, and auto-merge waits for it.
  • Deviation noted: commit 8ea85a5 carries a model-named trailer. History is not rewritten.
  • Landing: non-governed. The seat flips the PR to ready and arms auto-merge in this stroke.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 28, 2026 04:25
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit 3801c17 Oct 28, 2026
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend Server-side behaviour — hooks, flows, actions ci/cd CI plumbing and the verification pipeline

Projects

None yet

2 participants