fix(workflow): align resume --json envelope and filter expired approvals

[stack72]

Summary

Fixes two manual-approval JSON/listing inconsistencies surfaced while writing UAT coverage (swamp-uat #470).

Issue 1 — workflow resume --json leaked _status-prefixed keys

run --json and history get --json map the domain WorkflowRun through libswamp's toRunData() into a plain WorkflowRunView, exposing canonical keys (status, startedAt, jobs, …). The resume command bypassed libswamp, consumed the domain service.resume() stream directly, and yielded the raw WorkflowRun class instance — so JSON.stringify serialized its private fields (_status, _startedAt, _jobs, _logFile, _tags, _workflowDataArtifacts).

Fix: Promote the internal mapEvent to an exported mapWorkflowExecutionEvent(event, runRepo, verbose?) (lighter signature — only ever needed the run repo + verbose flag), export it from libswamp/mod.ts, and route the resume stream through it. All three paths now emit the identical envelope.

Issue 2 — workflow approvals over-reported timed-out runs

The approval deadline check (now > suspendedAt + timeout) lived only inline in workflow approve, so workflow approvals kept listing expired gates as actionable.

Fix: Extract the single source of truth into a domain helper evaluateApprovalTimeout(startedAt, taskData, now) in src/domain/workflows/approval_timeout.ts. approve rejects expired gates with the same message; approvals now skips them (if (timeout?.expired) continue).

⚠️ UAT lockstep

swamp-uat #470 / PR #241 currently pins the old behavior. Both assertions must be updated in lockstep:

• resume-JSON: expect status (+ the toRunData envelope), not _status/_startedAt/…
• approvals-listing: expect an expired run to be absent

Test Plan

• New unit tests: src/domain/workflows/approval_timeout_test.ts (7 cases — expired, in-window, exactly-at-deadline, no timeout, never-started, non-approval task, absent task data).
• deno check, deno lint, deno fmt, full suite (6464 passed / 0 failed), deno run compile — all green.
• Manual repro against the compiled binary:
  • resume --json now reports status = succeeded with keys [duration, id, jobs, path, status, workflowId, workflowName] (no underscores).
  • After a 1s deadline lapses: approve rejects with Approval timed out…, and approvals --json returns { "approvals": [] }.
  • Positive regression: a gate with no timeout still lists normally.

🤖 Generated with Claude Code

[github-actions]

Code Review

Clean, well-structured fix for two real bugs. CI green on Linux and Windows.

Blocking Issues

None.

Suggestions

1. Verbose comments in approval_timeout.ts (lines 34–46): The multi-paragraph JSDoc on evaluateApprovalTimeout explains WHAT it does (clear from the signature) and references callers (workflow approve, workflow approvals). Per CLAUDE.md, prefer no comments or one-line-max, and avoid referencing callers since those references rot. The interface doc on ApprovalTimeout is fine. Consider trimming to just the non-obvious bit: the undefined return semantics.

2. Comment in run.ts (lines 373–380): Similar — the expanded JSDoc on mapWorkflowExecutionEvent references run and resume paths and explains the private-field leak. The WHY (preventing _status leaking via JSON.stringify) is worth keeping; the caller list is not.

Neither blocks merge — the logic, tests, DDD placement, and import boundaries are all correct.

[github-actions]

CLI UX Review

Blocking

None.

Suggestions

1. workflow approvals silently drops expired entries — when a run is suspended but its approval gate has timed out, the listing omits it without explanation. A user who knows a run is suspended and runs swamp workflow approvals to find it will see "No workflows awaiting approval" with no indication that an expired gate was filtered. A --verbose debug line (or a note like "1 run skipped (approval deadline elapsed)") would aid diagnostics, but the current behavior is at least accurate — showing entries that approve would immediately reject would be worse.

2. resume ignores --verbose when building JSON output — workflow run passes ctx.verbosity === "verbose" to mapWorkflowExecutionEvent; workflow resume passes nothing (defaults to false). So swamp workflow resume --verbose --json produces the same JSON as without --verbose. This gap pre-dates this PR (before, the raw domain object with _ fields was even more broken), so this PR doesn't regress anything — just worth a follow-up to align the two paths fully.

Verdict

PASS — both fixes are correct and improve UX. resume --json now emits the same canonical envelope as run --json (no more _status/_startedAt leakage), and workflow approvals no longer lists gates that approve would immediately reject.

[github-actions]

Adversarial Review

Critical / High

None found.

Medium

1. src/cli/commands/workflow_resume.ts:184 — verbose always undefined in the resume path. mapWorkflowExecutionEvent(event, runRepo) is called without the third verbose argument, so toRunData never includes step artifacts in the completed/suspended envelope. The resume command has no --verbose flag, so this is consistent — but if --verbose is ever added to resume, someone will need to wire it through here. This is not a regression (the old code bypassed toRunData entirely), just a gap to be aware of.

2. src/cli/commands/workflow_resume.ts:176-185 — resume path does not accumulate report_completed/report_failed events onto the final run view. The workflowRun generator in run.ts:548-585 collects report events and attaches them to the completed/suspended run view via { ...mapped, run: { ...mapped.run, reports: reportResults } }. The resume generator does not. If service.resume() yields report events, they'll be emitted individually but not aggregated on the final run view. This is pre-existing behavior (the old code had the same omission), not introduced by this PR, but worth noting since the mapping is now shared.

Low

1. TOCTOU between timeout check and approval action in workflow_approve.ts:102-114. Time passes between evaluateApprovalTimeout(step.startedAt, wfStep?.task.data, new Date()) and step.recordApprovalDecision() at line 119. Theoretically, a timeout could lapse in that window. In practice the window is sub-millisecond in a sequential CLI command, and this is the same race the old inline code had. Not a concern in practice.

Verdict

PASS. Clean, well-structured extraction. The evaluateApprovalTimeout helper correctly mirrors the old inline logic (strict > comparison, identical guard conditions for undefined/non-approval/no-timeout), the mapWorkflowExecutionEvent refactor correctly narrows the signature from (event, deps, input) to (event, runRepo, verbose?), the exhaustive switch covers all WorkflowExecutionEvent kinds, and the timeout field's Zod schema (z.number().positive().optional()) ensures !taskData.timeout can never accidentally filter out 0. Test coverage is thorough with 7 cases covering the boundary at exactly-at-deadline. The libswamp barrel export is properly added. No correctness, security, or data integrity issues found.