fix(orchestrator): check for resumable workflow run on all platforms (closes #1741)#1749
fix(orchestrator): check for resumable workflow run on all platforms (closes #1741)#1749kagura-agent wants to merge 3 commits into
Conversation
…loses coleam00#1741) Chat platforms (Slack, Telegram, Discord, GitHub) never resumed approval/interactive-loop workflows because findResumableRunByParentConversation was only called inside the web-specific branch. Lift the resume check to run before the platform type conditional so all platforms discover and hydrate a prior paused run. Platform-specific dispatch (background vs foreground) only triggers when there is nothing to resume.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe PR moves resumable-run detection and hydration out of the web-only branch into a shared path after isolation resolution. The orchestrator checks for a resumable run by parent conversation; if found it hydrates and resumes on the prior working_path (or warns and runs fresh if hydration yields no prepared state); otherwise it continues with platform-specific dispatch logic. ChangesPlatform-agnostic workflow resumption
Sequence DiagramsequenceDiagram
participant Conversation
participant OrchestratorAgent
participant WorkflowDB
participant Hydrator as hydrateResumableRun
participant Executor as executeWorkflow
Conversation->>OrchestratorAgent: dispatchOrchestratorWorkflow(conversation)
OrchestratorAgent->>WorkflowDB: findResumableRunByParentConversation(workflow.name, conversation.id)
WorkflowDB-->>OrchestratorAgent: resumableRun (with working_path) / null
alt resumableRun with working_path
OrchestratorAgent->>Hydrator: hydrateResumableRun(resumableRun)
Hydrator-->>OrchestratorAgent: preparedState / null
alt preparedState
OrchestratorAgent->>Executor: executeWorkflow(preparedState, working_path, options)
else no preparedState
OrchestratorAgent->>Conversation: sendWarning("no completed nodes, running fresh in same working_path")
OrchestratorAgent->>Executor: executeWorkflow(fresh, working_path, options)
end
else no resumableRun
OrchestratorAgent->>Executor: platform-specific dispatch / executeWorkflow(cwd,...)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/orchestrator/orchestrator-agent.ts`:
- Around line 372-377: Remove the resumable run working path from the resume log
payload to avoid logging PII: in the block using getLog().info where the object
currently includes workflowName: workflow.name, resumableRunId: resumableRun.id,
workingPath: resumableRun.working_path, delete the workingPath/working_path
entry so only workflowName and resumableRunId are logged (leave getLog().info
and the other fields intact).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: be0f7c83-2d4a-4203-92ea-11127b668766
📒 Files selected for processing (1)
packages/core/src/orchestrator/orchestrator-agent.ts
Review SummaryVerdict: minor-fixes-needed This PR extends the resumable-run check to all platforms, fixing a silent regression where non-web platforms with approval/loop gates were restarting workflows instead of resuming them. Code logic and error handling look correct. One test is needed before merge. Blocking issuesNone. Suggested fixes
Minor / nice-to-have
Compliments
Reviewed via maintainer-review-pr workflow (Pi/Minimax). Aspects run: code-review, error-handling, test-coverage. |
|
Closing in favor of #1756. Thanks @kagura-agent for surfacing this and the first-pass fix — your diagnosis of the Net of #1756 vs this PR: same primary fix + the codebase-collision bug closed in one shot + test coverage. Credit's yours on the diagnosis. |
Summary
findResumableRunByParentConversationcheck out of theplatform.getPlatformType() === "web"conditional so it runs for all platforms before dispatch.UX Journey
Before
After
Architecture Diagram
Before
After
Validation
bun run test— all tests passfindResumableRunByParentConversationquery: usesworkflow_name+parent_conversation_id(DB fields), both populated identically for web and chat platformsCloses #1741
Summary by CodeRabbit