feat: add progressive tree output for workflow run

[adamhjk]

Summary

• Adds a two-zone terminal UI for workflow run in TTY terminals: a scrollback zone for completed work and a live active zone with spinners, step expansion, and output peek
• Tree-structured display with terminal budget system that degrades gracefully across 5 tiers when the terminal is too small
• Active zone capped at 50% terminal height with branded separator line
• Event batching via microtask coalescing handles 50+ parallel jobs without hitting React update depth limits
• Falls back to flat log output for non-TTY environments (CI, pipes)
• New --log global flag to force flat log output in TTY
• Upgraded ink 5→6.8 and react 18→19
• Also fixes vault secrets leaking into report data files

Closes #596
Closes #623
Closes #664
Closes #700

Test plan

• 3545 tests pass (deno run test)
• deno check clean
• deno lint clean
• deno fmt --check clean
• Manual: swamp workflow run <name> in TTY shows tree output
• Manual: swamp --log workflow run <name> forces flat log
• Manual: swamp workflow run <name> | cat falls back to log (non-TTY)
• Manual: swamp --json workflow run <name> unchanged

🤖 Generated with Claude Code

[github-actions]

CLI UX Review

Blocking

None.

Suggestions

1. --log is global but only meaningful for workflow run — --log is registered as a .globalOption() so it appears in the help text of every command (e.g., swamp vault list --help, swamp model run --help). For every command other than workflow run, the flag is silently ignored — there is no interactive tree to suppress. A user who reads the flag in swamp data list --help and tries it will see no difference and get confused. Consider either: (a) making it a local option on the workflow run subcommand, or (b) updating the global help description to hint at its limited scope (e.g., "Force flat log output instead of interactive tree (workflow run only)").

2. --log / --log-level visual proximity — In the rendered help output, --log and --log-level sit next to each other. They're distinct flags and Cliffy handles them correctly, but the names read as a prefix group. Something like --no-tree or --plain would be unambiguous. Low-priority since the description is clear.

3. Separator line brand text is lowercase swamp — the separator renders as ─swamp──── my-workflow ──. If other branded strings in the CLI use title or consistent casing, align here. Minor cosmetic point.

Verdict

PASS — the new tree output is well-structured, degrades gracefully, falls back correctly in non-TTY environments, and JSON mode is unchanged. The --log escape hatch works as documented. No blocking issues.

[github-actions]

Code Review

This is a well-architected PR that adds a progressive tree output for workflow run. The implementation follows the project's conventions closely.

Blocking Issues

None.

Suggestions

1. useEffect missing dependency — In workflow_run_tree.tsx:161, the useEffect that calls onDone(state.failed) has [state.phase, state.failed] as deps but omits onDone and exit. Since onDone is stable in practice (created once in the renderer), this won't cause bugs, but adding it to the dep array would be more correct for React's rules of hooks.

2. WorkflowJobInfo / WorkflowRunJobInfo naming — The domain layer uses WorkflowJobInfo (execution_events.ts) and the libswamp layer uses WorkflowRunJobInfo (run.ts). These are structurally identical. The naming difference is fine for layer separation, but aligning them (e.g., both as WorkflowJobInfo) could reduce cognitive overhead since they carry the same fields.

3. Shallow Map clone in cloneJobs — cloneJobs does new Map(jobs) which is a shallow clone. This works because updateJob always spreads into a new object, but it's worth noting that any future direct mutation of a JobState inside the cloned map would break the reducer's immutability contract. A brief comment would help future contributors.

4. setTimeout cleanup race in renderer — In renderer.tsx:82, the completed handler uses setTimeout(() => this.cleanup?.(), 100) to allow a final render cycle. This is a common Ink pattern, but if the process exits before the timeout fires, cleanup may not run. Not a practical issue (Ink handles process exit), just noting it.

DDD Assessment

The architecture cleanly follows domain-driven design:

• Domain events (execution_events.ts) are extended with WorkflowJobInfo as a value object — correct choice for identity-free metadata.
• Libswamp layer maps domain events to its own WorkflowRunEvent type, maintaining the anti-corruption layer between domain and presentation.
• State management uses a pure reducer (treeReducer) — the state types (TreeState, JobState, StepState) are value objects modeling UI state, not domain entities, which is appropriate for the presentation layer.
• Import boundaries are respected — all presentation imports come from libswamp/mod.ts, never from internal paths.

Overall

Clean implementation with strong test coverage (state reducer, budget calculation, event bridge, component rendering). The event batching via microtask coalescing is a smart solution for high-parallelism workflows. The 5-tier degradation system handles terminal size constraints gracefully. LGTM.

[github-actions]

Adversarial Review

Critical / High

None found.

Medium

1. Data artifact hints are never generated in the tree renderer (state.ts:572-579, components/data_hints.tsx, components/scrollback_item.tsx:41)

   The ScrollbackItem union type includes a data_hints variant (state.ts:102-106), the DataHints component exists and renders artifact commands (data_hints.tsx:29-42), and ScrollbackEntry routes the data_hints case (scrollback_item.tsx:41-47). However, no code path ever creates a data_hints scrollback item. The completed reducer case (state.ts:572-579) sets finalRun (which contains dataArtifacts on steps) but never extracts artifact names or appends a data_hints entry to scrollback.

   The design doc explicitly specifies data hints should appear after workflow-scope reports. The LogWorkflowRunRenderer generates these hints in renderDataArtifactHints (workflow_run.ts:184-213). The tree renderer silently omits them.

   Breaking example: User runs swamp workflow run deploy which produces data artifacts. In log mode they see swamp data get --workflow deploy ec2-state hints. In tree mode (the new default for TTY), they see nothing — they have to know the swamp data commands already.

   Suggested fix: In the completed case of treeReducerSingle, extract artifact names from event.run.jobs[*].steps[*].dataArtifacts and append a data_hints scrollback item when any exist.

2. Double unmount race between renderer cleanup and React exit (renderer.tsx:85-91, workflow_run_tree.tsx:123-129)

   On completion, two independent cleanup paths fire:

   • renderer.tsx:90: setTimeout(() => this.cleanup?.(), 100) — calls instance.unmount() after 100ms
   • workflow_run_tree.tsx:127: setTimeout(() => exit(), 0) — calls Ink's exit() after 0ms (which also triggers unmount internally)

   These race against each other. In normal operation, the component's exit() fires first (0ms timeout), then the renderer's unmount() fires 100ms later on an already-unmounted instance. Depending on Ink 6's tolerance for double-unmount, this could produce TTY errors or silent failures.

   Breaking example: On a fast-completing workflow, both timeouts resolve, exit() tears down Ink, then instance.unmount() runs on the dead instance. If Ink 6 throws on double unmount (which it may, given the major version bump), this becomes a crash after apparently successful output.

   Suggested fix: Guard the cleanup: set this.cleanup = null after calling it, and in the onDone callback, avoid calling exit() since the renderer owns the lifecycle.

Low

1. O(n²) indexOf in active zone rendering (components/active_zone.tsx:51,111)

   visibleJobIds.indexOf(id) is called inside a loop over visibleJobIds, yielding O(n²) behavior. For the "50+ parallel jobs" scenario mentioned in the PR description, this creates ~2500 lookups per render at 15fps. Not a correctness issue, but easily fixed with a Map or using the loop index directly.

2. useElapsed hook returns stale value on first render after startedAt changes (hooks.ts:60-76)

   When startedAt transitions from null to a timestamp, the effect runs and calls setElapsed(Date.now() - startedAt). But the hook returns the elapsed state from before this effect runs (initial value 0). The component needs one more render cycle to pick up the correct elapsed time. At 100ms polling this means up to ~100ms of showing "0" elapsed. Cosmetic only.

3. Spinner comment says 80ms but constant is 120ms (hooks.ts:34,37)

   The JSDoc says "cycling through braille dot patterns at 80ms intervals" but SPINNER_INTERVAL_MS is 120. Misleading comment only.

Verdict

PASS — The code is well-structured with clean separation of concerns, proper React immutability patterns, and solid event batching. The missing data hints (#1 Medium) is a functional gap worth tracking but doesn't break existing behavior (it's a new renderer). The double-unmount race (#2 Medium) is unlikely to cause visible issues in practice given Ink's typical resilience. No security, data corruption, or crash-in-production-path issues found.