Skip to content

fix(workflows): include completed durable runs in /workflow resume picker - #1536

Closed
flora131 wants to merge 1 commit into
mainfrom
fix/workflow-resume-completed
Closed

fix(workflows): include completed durable runs in /workflow resume picker#1536
flora131 wants to merge 1 commit into
mainfrom
fix/workflow-resume-completed

Conversation

@flora131

@flora131 flora131 commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Extends the /workflow resume picker to surface successful completed durable workflows alongside live/resumable entries, opening them as read-only snapshots without replaying side effects or re-dispatching work.

Closes #1532

Key Changes

  • listCompletedWorkflows() backend method — new interface method added to all durable backend implementations (InMemoryDurableBackend, FileDurableBackend, WorkflowFileDurableBackend, DbosDurableBackend, ScopedDurableBackend) to query completed workflows that still have checkpoint data
  • completed-open.ts — new module providing listOpenableCompletedWorkflows() and openCompletedDurableWorkflow(), which reconstruct a RunSnapshot from durable checkpoint data for read-only inspection without triggering durable replay
  • File backend retentionisPrunableTerminalStatus() no longer prunes completed workflow files; they are now retained so /workflow resume can reopen them for inspection (only cancelled and explicitly non-resumable failed/blocked runs are pruned)
  • TUI selector — added { kind: "completed"; workflowId: string } result type; completed runs appear with ✓ completed labeling in the interactive picker and are deduplicated against resumable entries
  • Run control commandhandleDurableResume routes "completed" picker selections to openCompletedDurableWorkflow instead of the durable resume/replay dispatcher; handleRunControlCommand passes completed entries to the selector
  • Stale filtering — completed entries lacking backend checkpoint data or referenced session files are hidden from the picker to prevent surfacing unresolvable entries
  • Headless outputformatResumableWorkflowList renders ✓ completed status for completed entries and updates the header to "Workflow resume targets" when mixed with resumable entries
  • Docs — updated packages/coding-agent/docs/workflows.md to reflect that /workflow resume opens completed runs for inspection and that the file backend retains completed state
  • Changelogs — added ### Fixed entries under [Unreleased] in both packages/coding-agent/CHANGELOG.md and packages/workflows/CHANGELOG.md

Tests

New regression coverage:

  • test/unit/durable-backend.test.tslistCompletedWorkflows() returns completed entries, listResumableWorkflows() excludes them, and completed files are retained rather than pruned
  • test/unit/durable-resume-catalog.test.tslistCompletedFromBackend / listResumableFromBackend return disjoint sets
  • test/unit/durable-resume-runtime.test.ts — snapshot reconstruction from stage checkpoints, stale-entry filtering, listCompletedWorkflows delegation in hydrating backend
  • test/unit/workflow-resume-selector.test.ts — mixed selector items carry distinct "durable" and "completed" kinds and correct path/label formatting
  • test/unit/workflow-run-control-completed-resume.test.ts — command layer opens a completed snapshot and emits the overlay open call without invoking resumeDurableWorkflow

Validation

bun test test/unit/durable-backend.test.ts test/unit/durable-resume-catalog.test.ts test/unit/durable-resume-runtime.test.ts test/unit/workflow-resume-selector.test.ts test/unit/workflow-run-control-completed-resume.test.ts
bun run typecheck
bun run lint
bun run test:unit
bun run check:file-length

@mintlify

mintlify Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
bastani 🟢 Ready View Preview Jun 28, 2026, 4:10 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@claude claude Bot changed the title Show completed workflows in /workflow resume fix(workflows): include completed durable runs in /workflow resume picker Jun 28, 2026
@claude

claude Bot commented Jun 28, 2026

Copy link
Copy Markdown

PR Review: Show completed workflows in /workflow resume

Nice, focused change. The new read-only "open completed" path is the right design — it never flows through the durable resume/re-dispatch path, so there's no risk of replaying side effects. The backend interface is extended consistently across all four implementations (InMemory, DBOS, File, Scoped), and the regression coverage is good — especially workflow-run-control-completed-resume.test.ts asserting that resumeDurableWorkflow is not called for a completed target. Docs and both changelogs are updated.

A few things worth considering:

1. Unbounded retention / disk growth (most important)

isPrunableTerminalStatus no longer prunes completed:

function isPrunableTerminalStatus(status, resumable) {
  if (status === "cancelled") return true;
  return (status === "failed" || status === "blocked") && resumable === false;
}

Previously completed durable files self-cleaned on completion. Now every successful workflow's plaintext durable state (inputs, ctx.tool outputs, answered ctx.ui responses, stage outputs, session paths) is retained indefinitely under ~/.atomic/workflow-durable with no TTL, count cap, or automatic GC — the only cleanup is manual /workflow kill or file deletion. The privacy implication is documented, but the directory now grows without bound over a machine's lifetime. Consider a retention policy (keep N most-recent completed, or age-based pruning) so this self-limits.

2. Redundant disk I/O on every /workflow resume

In handleDurableResume, the completed set is read multiple times:

  • listOpenableCompletedWorkflows(backend) calls backend.listCompletedWorkflows() internally and does getWorkflow + listCheckpoints + per-checkpoint existsSync to build a full snapshot just to filter, then
  • backend.listCompletedWorkflows() is called again directly into completedCatalog.

On WorkflowFileDurableBackend, listCompletedWorkflows() reads every state file from disk (mergeRecords([], this.readAllRecords()) with a fresh InMemoryDurableBackend each call — no caching). So the whole durable dir is re-read several times per invocation. Fine interactively today, but it scales linearly with the now-unbounded number of retained completed files (compounding #1). Computing the openable list once and reusing it would avoid the duplicate scans.

3. @ts-nocheck in the new test

workflow-run-control-completed-resume.test.ts starts with // @ts-nocheck, which disables type checking for the whole file — against the repo's strict-types stance (noUnusedLocals, no any/unknown). The stubs could be given (even loose) types so the test stays type-checked, or the suppression narrowed.

4. Dense, duplicated match/ambiguity logic

The targeted-id branch in workflow-run-control-command.ts reimplements exact/prefix/ambiguous resolution inline (durableMatches / completedMatches / completedCatalogMatches / durableExact / completedExact / completedTarget), while resolveDurableEntry and openCompletedDurableWorkflow already do prefix+ambiguity resolution. This block is hard to follow and easy to let drift from the canonical resolver. Extracting a shared "resolve target across durable+completed catalogs" helper would reduce the surface for divergence.

5. Minor: paused live run can be clobbered

In openCompletedDurableWorkflow, when a store run with the same id exists and is paused (not completed), it is removeRun'd and replaced with the completed snapshot. The targeted /workflow resume <id> path does not filter completed entries against active live ids (only the no-arg selector path does), so a same-id active/paused live run could in principle be overwritten. It is a contradictory state and low risk, but a guard or comment clarifying intent would help.

Nits

  • completedWorkflowSnapshot uses a double cast for inputs ({ ...(handle.inputs as WorkflowSerializableObject) } as WorkflowInputValues); slightly against the "avoid casts" grain, though consistent with surrounding code.
  • formatResumableWorkflowList switching the header to "Workflow resume targets" only when a completed row is present is a nice touch — just confirm headless/-p consumers don't parse on the old "Resumable workflows:" prefix.

Overall this looks solid and well-tested; #1 (retention/GC) is the one I would want addressed (or explicitly accepted) before merge, with #2 as a cheap follow-up since it compounds it.

@lavaman131 lavaman131 closed this Jun 28, 2026
@flora131
flora131 deleted the fix/workflow-resume-completed branch August 14, 2026 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show completed workflows in /workflow resume

2 participants