Repository navigation
fix(approvals): a recall ends the run without the "rejected" notice - #2353
Merged
Merged
Conversation
… 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
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J8VVAkMkort1hh8jUGGbLB
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J8VVAkMkort1hh8jUGGbLB
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J8VVAkMkort1hh8jUGGbLB
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
Author
席位核对Verdict: ACCEPT ·
Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by Irene · project thread
Fixes #2349
Clause-②: no
Metadata-protocol: none
维护者速读
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-approvals18.2.0ApprovalService.recall(dist/index.mjs:7357-7361) resumes the waiting run down the approval node'srejectbranch withoutput: { decision: 'recall', requestId }. An approver's rejection resumes the same branch withdecision: 'reject'(decideAndResume,dist/index.mjs:7126and:7139), and so does an auto-rejection (:7620). There is no separate recall label:APPROVAL_BRANCH_LABELSin@objectstack/spec18.2.0 isapprove/reject/revise/resubmit.The flow can see the difference.
@objectstack/service-automation18.2.0 lands each resume output key as the flow variableNODE_ID.KEY(applyResumeSignal,dist/index.js:4224),celScope(dist/index.js:1480) nests dotted names undervars, andtraverseNext(dist/index.js:1319) narrows to the out-edges carrying the resumed label and then evaluates their conditions. So an edge out ofmanager_reviewreadsvars.manager_review.decision.Each reject edge that notified is now a pair, both labelled
reject, with conditions that partition in opposite polarity (thee5/e6pattern):recall, or cannot be read)recall)opportunity_approvaland its insert twine4,e8→notify_rejectede4_recall,e8_recall→endaccount_approvale4→notify_rejectede4_recall→endlead_conversion_approvale4→notify_rejectede4_recall→endA 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_approvalandopportunity_status_change_approvalroute reject toclear_request(anupdate_record) and thenend, with nonotifynode: confirmed, and left alone. The platform itself sends nothing on a recall either:ApprovalService.recall(dist/index.mjs:7271-7457) makes no notification call, unlikedecide.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. Itsbinds()now counts anapprovalnode'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 fromapproval-decision-attribution.test.tsintotest/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}/recallas the submitting rep,/reject(and/approvefor the first tier) as a real Sales Manager or Sales Director. It reads the notices off the messaging outbox (sys_notification_delivery, thenotificationsTohelper;notifywrites 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): noopportunity_rejectednotice, run completed,notify_rejectednever ran. Manager rejection and director rejection: one notice each.account_approval: recall sends noaccount_rejectednotice; a rejection sends one.lead_conversion_approval, on a boot with the gate armed (the one way an install arms it): recall sends nolead_conversion_rejectednotice; a rejection sends one.Mechanism assumption 2 measured: a real recall in
account_approvalandlead_conversion_approvaltook the same path as in the opportunity flow. Onmaineach wrote exactly one rejected notice.Before: the new file on main b8332aa, 4 failed / 5 passed
Each failure is one
sys_notification_deliveryrow, for example"actionUrl": "/crm_opportunity/KNuxJZgUSUYOzbuB"addressed to the rep.After: 9/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 withgit checkout HEAD --(git diff HEADempty, blob hash equal to HEAD's):Only the
e4case goes red. Thee8case stays green because it reads its own edge.Acceptance notes
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).decision: 'reject', autoRejected: true) still notifies, like an approver's rejection.returnedrequest) cancels the run instead of resuming it (plugin-approvalsdist/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 oforigin/main1f1ea12), local, exit code captured before any pipe:pnpm validatepnpm typecheckpnpm lint(os lint --strict)pnpm hygiene,pnpm hygiene:tokenspnpm buildpnpm test(full suite)The first full run, before the merge and before
binds()learned approval nodes, was 4542 passed and 1 failed: the CLASS 2 sweep intest/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