No actionable comments were generated in the recent review. 🎉

sequenceDiagram
    actor User
    participant Handler as Command Handler
    participant Orchestrator as Orchestrator Agent
    participant DB as Database
    participant Executor as Workflow Executor

    rect rgba(0,150,0,0.5)
    Note over User,Executor: Forced fresh run (skip resume lookup)
    User->>Handler: /workflow run my-workflow "task B" --force
    Handler->>Handler: extract --force, clean args
    Handler->>Orchestrator: dispatchOrchestratorWorkflow(options:{force:true})
    Orchestrator->>Orchestrator: skip findResumableRunByParentConversation
    Orchestrator->>Executor: dispatchBackgroundWorkflow(fresh run)
    Executor-->>Orchestrator: run dispatched
    end

    rect rgba(100,150,255,0.5)
    Note over User,Executor: Paused run auto-resume
    User->>Handler: /workflow run my-workflow "continue"
    Handler->>Orchestrator: dispatchOrchestratorWorkflow(options:{force:false})
    Orchestrator->>DB: findResumableRunByParentConversation()
    DB-->>Orchestrator: {status:'paused', working_path:'...'}
    Orchestrator->>Executor: executeWorkflow(working_path from prior run)
    Executor-->>Orchestrator: run resumed
    end

    rect rgba(255,100,100,0.5)
    Note over User,Orchestrator: Failed run → user prompt
    User->>Handler: /workflow run my-workflow "task B"
    Handler->>Orchestrator: dispatchOrchestratorWorkflow(options:{force:false})
    Orchestrator->>DB: findResumableRunByParentConversation()
    DB-->>Orchestrator: {status:'failed', id:'abc123'}
    Orchestrator->>User: prompt (resume abc123 / abandon+rerun / start fresh --force)
    User-->>Handler: selects action
    end

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 @coderabbitai help to get the list of available commands and usage tips.}

Good catch — fixed in latest commit. Added options.resumeRunId to dispatchOrchestratorWorkflow; the V2a check is now status === 'paused' || resumableRun.id === options?.resumeRunId. Two callers wire it: /workflow resume <id> now dispatches directly (no more 'run same workflow again' instruction that was looping users through the prompt) and the natural-language approval handler passes the paused run's id when it transitions status to 'failed' before dispatching. New regression test: resumeRunId option: failed run DOES auto-resume when CommandResult.workflow.resumeRunId matches. 103/103 in orchestrator-agent.test.ts, type-check + lint + format clean.

@ztech-gthb related to #1549 — workflow resume prompt fix.

Thanks @ztech-gthb — the architectural primitives here are right. The three-option failure prompt captures the actual decision space, and the force / resumeRunId distinction in the orchestrator avoids the prompt loop cleanly. The doc comment on abandonWorkflow is exemplary.

Three items worth tightening before merge:

1. --force parsing matches the flag anywhere in args (packages/core/src/handlers/command-handler.ts:817-828)

const forceIndex = restArgs.findIndex(a => a === '--force');

findIndex matches --force anywhere in the user's args. If someone runs:

/workflow run deploy "deploy --force to staging"

…the parser strips --force from inside the quoted message, sets force=true, and passes deploy to staging as args. Silent corruption of the user's input.

Suggested fix: only strip --force when it's a bareword in a flag-position (e.g., immediately after the workflow name, or at the end before the quoted message), not as a substring of the quoted args. Or use -- as a verbatim-args separator. The existing "be lenient" comment hints at the intent, but lenience here is unsafe.

2. Markdown escaping in the failure prompt is incomplete (packages/core/src/orchestrator/orchestrator-agent.ts)

const escapedMsg = userMessage.replace(/"/g, '\\"');

Only " is escaped. If the user's original message contains a backtick, the markdown code-blocks in the prompt close early and the rest renders as plain text. If it contains \, that won't be re-escaped either. Not a security issue, but a UX paper-cut whenever the user's first attempt had backticks in it.

Suggested fix: also escape backticks and backslashes, or render the failed-message as an indented code block (4-space) so backticks inside don't terminate it.

3. /workflow resume <id> mutates DB before checking the YAML exists (packages/core/src/handlers/command-handler.ts)

Order today:

1. resumeWorkflow(runId) — transitions DB row to running
2. discoverWorkflowsWithConfig(...) — load YAML
3. If YAML missing, error

If step 3 fails, the DB is in running state but no execution happens. The error message ("Restore the YAML and try again") is fine UX-wise, but the run sits in a half-resumed state until the user retries.

Suggested fix: discover the workflow YAML before calling resumeWorkflow, or roll the DB back when discovery fails.

CI is green and the rest of the change looks solid. Happy to merge once these three are addressed (or to merge as-is and track them as a follow-up issue if you'd rather move on — your call).

Thanks @Wirasm — addressed in 332426e:

1. --force flag-position parsing (packages/core/src/handlers/command-handler.ts)
Tightened to restArgs[0] === '--force'. Worth noting: parseCommand already splits quoted strings into a single arg before the handler sees them, so the /workflow run deploy "deploy --force to staging" example resolves to args = ['run', 'deploy', 'deploy --force to staging'] — findIndex would have returned -1 there. But the principle is right: lenient matching invites surprises if parseCommand is ever changed. Strict position-0 match is the safer contract.

2. Markdown escaping (packages/core/src/orchestrator/orchestrator-agent.ts)
Extended to [\\\\\"\`] so backticks and backslashes are also escaped. The triple-backtick code-fence in the failed-run prompt is now safe against arbitrary userMessage content.

3. /workflow resume <id> — DB mutation before YAML check
I read this twice and I believe it's a non-issue. resumeWorkflow in packages/core/src/operations/workflow-operations.ts is a pure validator:

```ts
export async function resumeWorkflow(runId: string): Promise {
const run = await getRunOrThrow(runId, 'operations.workflow_resume_lookup_failed');
if (!RESUMABLE_WORKFLOW_STATUSES.includes(run.status)) {
throw new Error(...);
}
return run;
}
```

No status transition, no `runs` row update, no event insert. The actual resume happens later in `dispatchOrchestratorWorkflow` once the workflow YAML has been resolved and validated. The function name is admittedly misleading; happy to rename to `validateResumable` (or similar) in a follow-up if you'd like — but no half-resumed-state risk in current code. Let me know if I'm missing a side-effect somewhere.

Small follow-up in 4088888 addressing a UX gap I hit while testing this PR locally:

The resume-prompt previously surfaced only the runId of the prior failed run. If the failed run is hours/days old, choosing Option 1 ('Resume that run') re-runs the run's stored user_message — which can be a totally different topic from what the user just typed. Without the message visible, the topic-switch is invisible.

Now the prompt includes a 160-char blockquote preview of the failed run's stored user_message under an explicit 'Run prompt was:' label (wrapped in '---' separators), and Option 1 wording is disambiguated to 're-runs the prompt shown above, not your current message'. Two regression tests cover the preview path and truncation.

Same scope as the rest of this PR, drop-in additive; happy to split into a separate PR if you'd prefer to merge the parsing/escape fixes first.

Review Summary

Verdict: minor-fixes-needed

Two bugs in the PR wiring mean resumeRunId and --force are dropped before reaching dispatchOrchestratorWorkflow, so the V2a prompt fires when it shouldn't after natural-language approval or /workflow resume. Additionally, the new --force flag and changed behavior of /workflow resume and /workflow abandon are missing from the docs. Fixable — the core logic and test coverage are solid.

Blocking issues

• packages/core/src/orchestrator/orchestrator-agent.ts:762-770: After a user approves a paused approval gate (run transitions to failed), dispatchOrchestratorWorkflow is called without options. Without options.resumeRunId, the V2a "Found a prior failed run" prompt fires anyway — the approval flow loops indefinitely. Pass { resumeRunId: pausedRun.id } as the 8th argument.

• packages/core/src/handlers/command-handler.ts:835-843: When handleCommand returns a result.workflow from /workflow resume <id>, the resumeRunId and force fields are dropped before reaching dispatchOrchestratorWorkflow because handleWorkflowRunCommand is called without options. Pass { force: result.workflow.force, resumeRunId: result.workflow.resumeRunId }.

Suggested fixes

• packages/docs-web/src/content/docs/reference/commands.md + packages/docs-web/src/content/docs/book/quick-reference.md: Add /workflow run <name> --force [args] to the commands table. Explain it bypasses failed-run detection and starts a fresh run.

• packages/docs-web/src/content/docs/getting-started/overview.md: Update /workflow abandon description from "Discard a non-terminal run" to "Discard a failed, paused, or running run."

• packages/docs-web/src/content/docs/guides/authoring-workflows.md: (1) Update "Run the same workflow again to auto-resume" to reflect that /workflow resume <id> now dispatches immediately. (2) Document the three-option failed-run prompt. (3) Add one sentence: "Paused runs auto-resume silently; failed runs surface a prompt."

• packages/core/src/handlers/command-handler.test.ts:1356-1360 + packages/core/src/operations/workflow-operations.test.ts:298-320: Add test for status: 'failed' in the /workflow abandon and abandonWorkflow test suites.

• packages/core/src/handlers/command-handler.ts:726-729: Add a test that mocks discoverWorkflowsWithConfig to throw during resume handling. Assert result.success === false.

Minor / nice-to-have

• packages/core/src/command-handler.test.ts:1364-1365: Remove // see PR #1551 — the PR number will rot. The surrounding comment explains the behavior without it.
• packages/core/src/types/index.ts:113: Remove "V2a-" from "skips the V2a-prompt branch" — "user-prompt branch" is clearer and doesn't leak an internal name.
• packages/core/src/workflows.ts:340: Rephrase dispatchOrchestratorWorkflow reference to "the orchestrator" to avoid a future stale comment if the function is renamed.

Compliments

• The V2a "failed run user prompt" design with three copy-pasteable options (resume, abandon+re-run, --force fresh start) and the message preview is excellent UX — it gives users exactly what they need without being verbose.
• The --force flag-position rationale comment explains why the narrowing was necessary and why the old lenient match was wrong. That's a comment that saves future readers from undoing the fix.
• Five new orchestrator-agent tests covering the V2a flow, message truncation, resumeRunId bypass, and --force skip make the new surface well-guarded.

Reviewed via maintainer-review-pr workflow (Pi/Minimax). Aspects run: code-review, error-handling, test-coverage, comment-quality, docs-impact.

Hi @ztech-gthb — your recent commit addressed the review findings, but the PR has since gone DIRTY from dev moving on. Could you rebase onto current dev? Once it's clean I'll re-verify and merge. Ping me if you hit any blockers.

Thank you for this, @ztech-gthb — genuinely one of the most thorough PRs we've had: clear root-cause analysis, the 3-option UX, the run-preview, end-to-end verification on a real thread, rollback plan, the lot. The diagnosis in #1549 is exactly right and your fix direction is the one we're shipping.

I'm closing this PR not because anything's wrong with it, but because it was authored against dev as it stood in early May, and the resume path has been substantially restructured since:

• fix(workflows): make resume explicit via prepareResumedRun / hydrateResumableRun (closes #1392) #1646 introduced hydrateResumableRun (single-roundtrip hydrate of the resumable candidate)
• fix(core): concurrency-safe workflow resume/cancel (CAS guards) #1830 added compare-and-swap (CAS) guards so concurrent resumers (web Resume button, chat re-dispatch, CLI) can't double-claim a worktree

The core of your fix — --force guard, the failed-run prompt, and abandon-accepts-failed — still grafts cleanly onto that. But the later /workflow resume <id> re-routing commit (calling resumeWorkflow() then dispatching with resumeRunId) now collides with the CAS model: on current dev, hydrateResumableRun performs the status transition itself, and findResumableRunByParentConversation only matches failed/paused — so transitioning to running first means the re-dispatch finds nothing and starts a fresh run. It rebases with zero text conflicts but is semantically broken on today's dev — which is the dangerous kind (it'd pass CI and surface as a subtle regression).

Rather than risk that slipping through, I'm re-implementing the fix fresh on current dev, built natively on the hydrate/CAS resume model, and reusing your UX verbatim — the 3-option prompt, the run-message preview, --force, and abandon-failed. You'll be credited as Co-authored-by on the commit, and the new PR will close #1549.

Really appreciate the contribution — this made the fix better than it would've been from a cold start. 🙏