Deepen Harness process completion to retain interpretation after transport failures #461

What to build

A failed Harness session should still report the usage and warnings recoverable from its Session log and retained records. Today, if prompt delivery fails after stdout has been consumed successfully, the Run keeps the transport error but loses that interpretation: no retained export, usage summary, missing-skill warning, or session-ended line. A log-reader failure recovers these facts through a separate channel in the Session module, so equivalent failures receive different reporting.

Deepen the existing Harness process module so its streaming operation owns consumer-state recovery as well as the child and I/O workers. The Session module receives execution outcome and recovered interpretation together, then applies its existing completion rules. The Run still fails with the original transport error.

Acceptance criteria

• Replace the streaming operation with the state-carrying interface settled below, migrate its production caller and tests, and remove the Session module's recovery channel and error-dependent state matching.
• Recover consumer state after any normal consumer return, whether successful or failed, including when prompt writing or waiting for the child fails. A consumer failure retains the state accumulated before the failure. A successful consumer also retains its state when another worker fails.
• Return only after normal process completion or whole-tree cleanup and after every started I/O worker is joined. Keep concurrent prompt/output handling, environment overrides, stderr policy, Harness stop signals, and ordinary background-work behavior.
• Keep captured checks' output interface and behavior. Streaming still preserves nonzero exit status for interpretation to judge. Original transport errors retain their causes and take precedence over later reader, retained-record, or turn errors.
• Complete recovered interpretation exactly once and report its warnings, elapsed time and available usage once on ordinary failures. Recovery never turns an execution failure into success or starts a Resume from that failure; successful final-message and Resume behavior stays intact.
• Recorded interruption returns the canonical interrupted error, performs owned cleanup, and suppresses retained-record reads and session completion reporting. A spawn/startup failure or consumer panic with no recoverable state returns its original error without inventing interpretation.
• Cover successful stdout consumption followed by BrokenPipe, partial consumption followed by log failure with a blocked prompt writer, consumer panic and startup failure, and interruption. Assert recovered facts, original errors, exact owned-PID cleanup and absence of duplicate reporting through production interfaces. Keep the existing byte-preservation, EOF/exit ordering, captured-stderr, detached-descendant, and Resume regressions passing.
• Run formatting, linting and the repository's required test suite. Use the existing Rust and shell test fixtures; add no Python or live Harness requirement to the permanent tests.

Blocked by

None (can start immediately).

Decisions

The design tree is: preserve failed-session reporting → scope and process seam; process seam → returned state, state availability, ownership and failure rules; those settled rules → Session integration, testing, documentation and Ticket size. The following rounds settle every branch; no human decision remains.

Round 1 — constraints and seam

1. Scope: recover reporting facts lost on ordinary transport failures while keeping execution failure, Resume eligibility and interruption policy. Reason: interpretation already supports reporting alongside failed execution; the friction is loss of its input. This ranks first because the loss is reproduced on the recent Harness execution path and its design requires no new product decision.
2. Seam: deepen the existing Harness process streaming operation and keep its state opaque. Reason: that module already owns the child, cleanup and worker joins, which determine when state can be recovered. Existing Harness adapters and local shell fixtures make this a real seam; no additional adapter abstraction is needed. The deletion test passes: removing the caller recovery channel concentrates complexity in this implementation; removing the deep process implementation would spread ownership, joining and recovery back across callers.

Round 2 — interface and ownership

3. Returned shape: accept initial owned state and a consumer that receives stdout plus mutable access to that state. Return an outer result whose success carries both the state and an independent execution result. Reason: ordinary execution failure can accompany useful state, while successful return always supplies state. The throwaway prototype verified this decision-rich shape:

   struct Streamed<T> {
       execution: Result<ExitStatus>,
       state: T,
   }
   // streaming(..., state: T, consume: FnOnce(ChildStdout, &mut T) -> Result<()>)
   //     -> Result<Streamed<T>>

4. State availability: preserve state whenever the consumer returns normally, including an error return; otherwise return the execution error without state. Reason: a reader error leaves useful accumulated interpretation, whereas a panic or failure before the worker starts cannot promise usable state.

5. Ownership: retain the existing child/worker owner and place the single-value state handoff inside its streaming operation. Recover it after that owner has finished or cleaned up and joined the workers. Reason: the owner already guarantees this ordering, and a bounded, single-send handoff does not block cleanup. Callers and tests do not depend on that private mechanism.

6. Failure precedence: keep the owner's first transport failure and its error chain, even if cleanup later makes the consumer fail; pass it into interpretation unchanged. Reason: the cause of the Failed run must stay accurate, and retained facts are reporting rather than evidence of execution success.

7. Interruption: check recorded interruption after execution/cleanup and before exposing recovery; keep interpretation's own interruption checks. Reason: an interrupt must suppress optional retained reads and reporting even when usable state happens to exist.

8. Captured checks: keep their existing output operation and shared execution owner. Reason: checks require captured stdout/stderr bytes and status, and this deepening adds no new policy for them.

Round 3 — integration and verification

9. Session integration: remove the caller-owned channel; hand the returned interpretation its execution result, then emit the existing completion report. Keep raw logging, protocol decoding and retained-record reconciliation in their current modules. Reason: the process module owns transport locality while interpretation owns Harness-specific meaning; no protocol knowledge needs to cross their seam.
10. Test surface: test state retention through the streaming interface and reporting through the Session execution interface, using local CLI fixtures. Keep existing process ownership and black-box Run regressions. Reason: the interface is the test surface, and these tests expose the actual interaction that isolated interpretation tests missed. Replace obsolete success-only streaming assertions during migration; do not add tests of private channels or worker fields.
11. Domain docs: no domain glossary or ADR change is required. Reason: this deepens an existing Harness module under existing names and preserves ADR 0012/0013's execution policies; the returned-state form is a reversible implementation decision.
12. Plan size: one standalone Ticket with no blockers. Reason: the shared process implementation has one production streaming caller; migrating that interface and adding its reporting regressions fit one fresh context window and land together.

Validation: the throwaway fixture emitted a session id, closed stdout, and closed unread stdin only after its record reached the Session log, while a 1 MiB prompt writer was blocked. On main, the original BrokenPipe remained and the exact child PID was reaped, but retained export, usage and warning were absent. The same test passes with the state-carrying prototype. All 622 in-process tests and 31 existing logging, process-tree and Resume integration tests passed, as did formatting and binary linting (cargo clippy --bin thirdshift -- -D warnings). Both Mermaid diagrams passed parser validation. All 46 open issues were read with their bodies and comments and rechecked before publication; none covers this deepening.

Architecture review

Strength: Strong · Dependencies: local-substitutable

Files: src/harness/process.rs, src/harness/process/tests.rs, src/session.rs, tests/progress_lines.rs, tests/process_tree.rs, tests/background_work.rs

Before

flowchart LR
  S["Session module"] --> P["Harness process module"]
  P --> W["I/O workers"]
  W -. "consumer failure only" .-> S
  S --> I["Harness interpretation module"]
  classDef leak stroke:#dc2626,stroke-width:2px;
  class S,W leak

After

flowchart LR
  S["Session module"] --> P["Harness process module"]
  P --> W["I/O workers"]
  P -- "execution and recovered state" --> S
  S --> I["Harness interpretation module"]
  classDef deep stroke-width:3px;
  class P deep

Problem: the streaming completion interface is shallow where it discards interpretation after a transport failure and leaves recovery choreography in the Session module.

Solution: deepen the Harness process module by returning execution and consumer state together behind its existing seam.

Wins

• Depth: outcome and state together
• Locality: process recovery stays internal
• Leverage: remove Session recovery choreography
• Tests cross the production seam
• Retain the existing Harness adapters

Covered by: none; open issue titles, bodies and comments checked.