fix(workflow): let a waiting run report the approvals it is blocked on - #3914
Conversation
`WorkflowRun` declares `pendingApprovals`, but nothing ever wrote it. Approvals
are persisted to their own store so they can be reserved atomically against a
worker, which leaves the field empty on the stored record. A waiting run
therefore reported zero approvals while the approval manager held one:
manager: 1
run.pendingApprovals: 0
run.status: waiting
That is not cosmetic. `useWorkflow` takes approvals off the run body to announce
them through `onApprovalRequired`, so a paused workflow never surfaced its
approval to a UI at all, and the run a caller reads did not say what it was
waiting for.
Join the two on the public read path instead of writing a second copy at
approval time, which would leave a duplicate free to disagree with the store
that the atomic reservation protects. `listRuns` is deliberately left alone: it
would issue one approval query per run, and the individual run is the right
place to ask.
|
Warning Review limit reached
Next review available in: 42 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesPending approval hydration
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change makes waiting runs expose their pending approvals while preserving the approval store as the source of truth; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant WorkflowClient
participant MemoryBackend
participant PendingApprovals
WorkflowClient->>MemoryBackend: getRun(runId)
MemoryBackend->>PendingApprovals: read pending approvals
PendingApprovals-->>MemoryBackend: approval records
MemoryBackend-->>WorkflowClient: run with pendingApprovals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b9944809c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The approval join tests already close their workflow clients and settle without leaked operations or resources, so they should remain under Deno's default sanitizer checks. Constraint: CI rejects sanitizer opt-outs above the repository baseline. Rejected: Raise the sanitizer baseline | the focused suite passes with sanitizers enabled. Confidence: high Scope-risk: narrow Tested: Deno 2.7.7 fmt, lint, typecheck, 45 focused workflow-client steps, sanitizer baseline 404/404.
Make pending-approval hydration part of the backend getRun contract so Memory and Redis return the same public run shape without a second client-layer query. Constraint: Approval records remain separate for atomic reservation semantics. Rejected: Add a backend capability flag | the built-in backends can honor one clear getRun contract directly. Confidence: high Scope-risk: narrow Directive: Backend getRun implementations must hydrate current pending approvals. Tested: Deno 2.7.7 format, lint, check, sanitizer baseline, and 79 focused MemoryBackend and WorkflowClient steps.
Adding lines to workflow-client.ts shifted the source line pins the generated reference carries, which fails `docs:api-reference:check` in CI. Regenerated rather than hand-edited, using the Deno version CI pins (2.7.7). A newer Deno reports unrelated pages as stale, so regenerating with one would have produced a much larger and wrong diff.
Merge the current main branch after the approval-hydration change conflicted with the generated workflow reference. Keep the HTTP handler routes and browser-safe projection while consuming the approvals already hydrated by backend getRun, avoiding a duplicate approval-store read. Constraint: PR #3914 moved approval hydration into the backend getRun contract before PR #3913 could merge. Rejected: Keep the handler-level approval query | It duplicates the backend read and violates the newly merged getRun contract. Confidence: high Scope-risk: moderate Directive: Workflow run HTTP reads must project the hydrated getRun result without separately querying pending approvals. Tested: Deno 2.7.7 full lint and typecheck; 213 focused workflow steps; Linux docs:api-reference:check with all 45 files current. Not-tested: Live Redis service integration; Redis behavior is covered by the repository mock suite.
What
WorkflowRundeclares apendingApprovalsfield. Nothing ever wrote it.Approvals are persisted to their own store so they can be reserved atomically against a worker (
savePendingApprovalIfStatusAndWorker), which leaves the field empty on the stored run record. Measured directly on a paused run:Why it matters
useWorkflowtakes approvals off the run body to announce them throughonApprovalRequired. Because the run never carried them, a paused workflow never surfaced its approval to a UI at all — the human-in-the-loop path was unreachable from the client hooks.More generally, a caller reading a run reasonably expects it to say what the run is waiting for, and it did not.
Approach
Join the two on the public read path (
WorkflowClient.getRun) rather than writing a second copy at approval time.Dual-writing was the obvious alternative and is the wrong call here: the approval store is what the atomic worker reservation protects, so a second copy on the run record would be free to disagree with it under exactly the concurrent conditions that reservation exists to handle. One source of truth, joined on read.
listRunsis deliberately unchanged — it would issue one approval query per run. The individual run is the right place to ask, and the doc comment says so.Verification
Three tests in
src/workflow/api/workflow-client.test.ts. I confirmed the first fails against the unfixed client before keeping it:src/workflow/suite: 78 passed, 0 failed. fmt and lint clean. Full pre-push green.Relationship to #3913
#3913 works around this in its HTTP layer by fetching approvals and merging them into the response. Once this lands, that merge is redundant and the route can just serialize the run it was given. I have deliberately not touched #3913 here, since another session is actively working that branch.
Found while dogfooding the documented developer journey.
Summary by CodeRabbit
New Features
Documentation