Deepen child Run execution ownership for Spec runs and Base fixes #463

What to build

A Spec run and a Base fix must receive a child Run's actual ending even when its Harness prints non-UTF-8 diagnostics. Deepen the child Run execution module so it owns the spawned process, both output readers, graceful stopping, and completion behind one interface. Spec runs should retain and poll owned child handles; Base fixes should continue to wait synchronously.

Acceptance criteria

• Starting a child Run installs ownership immediately after spawning and starts concurrent stdout capture and stderr relay before returning a handle. Any subsequent startup failure gracefully stops and reaps that child and joins every reader that started.
• The child Run interface supports starting, nonblocking completion polling, and synchronous waiting. A pending or stopping child produces no terminal result; a terminal result is delivered only after the child is reaped and both readers have finished and been joined.
• Non-UTF-8 stderr is displayed with replacement characters and does not lose the child's ending. Valid later lines still convey its cause, Session log, and Base fix account. LF, CRLF, and a final line without a newline retain their current line semantics.
• Both output pipes are consumed while the child runs. A finite fixture writing more than pipe capacity to stdout completes without deadlock, and the existing last-stdout-line ending contract is preserved.
• A reader failure or panic, a process-wait failure, or dropping a live handle initiates owned graceful shutdown. No child or reader is left running after the owning scope finishes, and a transport failure retains its original numbered diagnostic after cleanup.
• A parent interruption is forwarded once as SIGTERM to each live child. Completion waits for the child's Failed run path to save unfinished work. Shared process groups, separate child processes, and the existing absence of a shutdown timeout are preserved; no group-wide kill or Harness stop policy is introduced.
• The Spec run removes its additional waiter threads and completion channel, owns its running child handles, and polls all of them fairly. Parallelism, Ticket dependency order, checklist updates, and numbered landed/failed/interrupted progress lines retain their current behavior.
• A Spec run interrupted while several Tickets are active forwards the interruption to all of them before waiting for any one to finish, starts no dependent Ticket, and finishes only after its active children are cleaned up.
• Base fixes continue using the same synchronous child Run interface and keep their Running mark owned until completion. Hidden argument propagation, same-executable launching, child ending interpretation, and notification behavior remain compatible.
• Tests cross the child Run execution interface with finite local executable fixtures and cover byte-tolerant diagnostics, success and failure accounts, pipe pressure, pending polling, transport failures, reader panics, startup failures after spawn, abandoned handles, and interruption. Cleanup assertions use only exact fixture PIDs.
• Existing argument/ending tests and Spec run/Base fix integration tests pass, including preservation of half-finished work after parent-only interruption. Run formatting, clippy, and the full test suite before delivery.

Blocked by

None (can start immediately).

Decisions

The design tree starts with child Run execution ownership. Its branches are compatibility, lifecycle, byte handling, caller integration, testing, and plan size. Lifecycle branches into startup, polling, stopping, failure precedence, and abandonment. Caller integration branches into Spec runs and Base fixes. All branches are settled below; no decision requires a human.

Round 1: scope and constraints

1. Deepen child Run execution, keeping separate processes. The module owns execution from spawn through cleanup and ending interpretation. ADR 0006 requires Ticket Runs to remain separate processes, and ADR 0008 requires Base fixes to remain child Merge runs; concentrating ownership honors both.
2. Keep the domain language and existing decisions. Run, Ticket, Spec run, Base fix, Failed run, and Harness already name the concepts involved. No new glossary term, CONTEXT.md change, or ADR is needed: this is a reversible implementation change that preserves the recorded choices.
3. Publish one standalone Ticket with no blockers. The change is one coherent execution refactor with two existing callers and local tests, sized for one fresh context window. A Spec would add no useful independently deliverable slice. Its only label is needs-triage.

Round 2: the deepened interface and implementation

4. Retain start and wait, adding nonblocking try_wait on the owned handle. Starting returns an owned handle, try_wait reports pending or one completed result, and wait drives the same completion logic synchronously. The two real caller needs justify this small interface: a Spec run has several active children, while a Base fix has one to await. Callers must remove a handle once it returns a terminal result; neither callers nor tests manage its pipes or reader threads.
5. Install ownership before every fallible post-spawn operation. Pipe extraction and fallible reader startup happen under the same owner that will stop, reap, and join on failure. Start both readers before exposing the handle so pipe consumption does not depend on a caller remembering to wait promptly.
6. Polling never waits for unfinished work. It polls the child and completed readers, joins only readers already finished, and retains their results. A reader failure or panic moves the private execution into stopping; polling continues until cleanup completes before returning that failure. This lets a Spec run forward interruption to every child without one child's shutdown blocking supervision of its siblings. Use the existing roughly 100 ms process supervision cadence, not the GitHub polling helper, which returns early on interruption.
7. Keep graceful stopping specific to a child Run. Forward SIGTERM once to the still-owned child and allow its Failed run path to push unfinished work, then reap it and join its readers. Do not signal the shared process group or reuse the Harness adapter's whole-tree stop routine: those would stop the parent or bypass the child's cleanup. Keep the existing unbounded graceful wait rather than inventing a shutdown timeout or escalation policy.
8. Abandonment drives the same cleanup. Dropping an unfinished handle requests graceful stop and completes reaping and reader joining; dropping a completed handle does nothing. Signal only a still-owned unreaped child. This gives startup errors, unwinding, and early caller exits the same lifecycle guarantees as explicit waiting.
9. Treat stderr as bytes until displaying each line. Read newline-delimited bytes, strip line terminators as the current line reader does, and decode each displayed line lossily. The Harness inherits stderr and can supply arbitrary bytes, so malformed text must not replace the actual child outcome. Feed later valid lines to the existing Run ending reader, preserving its distinction between the child's own lines and relayed descendants.
10. Preserve stdout and ending semantics. Capture stdout concurrently, retain its existing UTF-8 error behavior, and use the existing Run ending reader after complete output and exit status are available. The last stdout line remains the PR URL; exit status, cause, Session log, and Base fix account remain authoritative. A recorded interruption forwarded to a live child yields Interrupted after cleanup; otherwise Reached/Failed follows the existing ending interpretation.
11. Preserve the first transport cause through cleanup. Reader startup, read, wait, and panic failures carry the child issue number and cause. Cleanup must not replace them with the child's resulting exit status or a secondary error. An observed parent interruption takes precedence for interruption reporting, matching the surrounding Command behavior.
12. Keep execution dependencies local and private. Real processes and finite executable fixtures satisfy the existing execution seam, making this local-substitutable. No public dependency trait or shared generic process module is needed. The Harness execution implementation retains its different stopping policy; its existing owned execution can inform the work without being coupled to child Run execution.

Round 3: integration and verification

13. Let the Spec run own handles directly. Store handles by Ticket number, scan all active handles on each completion pass, and remove one only after its terminal result. Replace the additional waiter threads/channel with this polling; keep the Ticket board and its existing Outside interface. Emit the existing numbered completion line when observing the result. This removes caller-owned execution ordering and detached waiter ownership without changing scheduling policy.
14. Keep the Base fix synchronous. Its existing start-then-wait use crosses the same seam and automatically gains the lifecycle guarantees. Its Running mark and sharing rules stay with the Base fix module, because they concern ownership of a Base fix issue rather than execution of its child process.
15. The interface is the test surface. Exercise start/try_wait/wait/drop with real finite local executables through the existing private executable selection seam. Assert observable endings, relayed diagnostics, prompt completion, and exact-PID cleanup, not private execution state. Where OS errors or thread-start failures cannot be induced reliably, use a private fault-injection seam around reader startup/consumption while retaining a real child and the same execution path. No test-only public interface is required.
16. Retain distinct behavior tests and replace duplicated transport tests. Hidden argument propagation and Run ending interpretation remain separate responsibilities, so their tests survive. New ownership tests belong at child Run execution; do not duplicate them across Spec run and Base fix tests. Integration tests verify caller behavior, especially concurrent Ticket interruption and saving unfinished work.
17. Keep fixtures isolated and bounded. Run interruption probes in isolated test processes because the interruption flag is process-wide. Fixture cleanup uses recorded owned PIDs and finite deadlines, including on assertion failure; no process-name searches. Test stdout pipe pressure as transport verification, not as evidence of an observed production deadlock: native child Run stdout currently contains a bounded PR URL.
18. Verify the completed implementation with repository checks. Run cargo fmt --check, cargo clippy --all-targets -- -D warnings, and cargo test. The Spec run and Base fix integration suites are prior art for the caller guarantees; child execution tests must also cover a transport failure while the child is still active and more than one active Ticket during interruption.

Validation during this Architecture review

At main head 0944f68, a throwaway execution-interface test launched a finite executable that wrote byte 0xff in a stderr diagnostic, printed a valid PR URL, and exited zero. The test failed with “could not read the Run for #248” caused by “stream did not contain valid UTF-8.” With only byte-tolerant stderr reading prototyped, that regression and all five existing child Run tests passed. The probe and prototype were removed afterward. The existing parent-only Spec interruption regression and the Base fix success-path integration test passed on the restored base code. Both report diagrams were checked with the Mermaid parser.

Architecture review

Strength: Strong · Dependencies: local-substitutable

Files: src/child_run.rs, src/spec_run.rs, src/base_fix.rs, src/run_ending.rs, src/harness/process.rs, tests/spec_run.rs, tests/base_fix.rs

Before

flowchart LR
  S[Spec run] --> W[Run completion]
  W -.execution ownership.-> E[Child Run execution]
  B[Base fix] --> E
  E --> P[Run process]
  E --> D[Stderr relay]
  E -.after exit.-> O[Stdout capture]
  D --> R[Run ending]
  O --> R
  linkStyle 1,5 stroke:#dc2626,stroke-width:2px

After

flowchart LR
  S[Spec run] --> E
  B[Base fix] --> E
  subgraph E[Child Run execution]
    direction LR
    W[Run completion] --> P[Run process]
    W --> D[Stderr relay]
    W --> O[Stdout capture]
  end
  D --> R[Run ending]
  O --> R
  classDef deep stroke-width:3px
  class E deep

Problem: The child Run module is shallow at its execution seam: callers own completion ordering, while its implementation can lose a successful ending on raw Harness diagnostics or leave child and reader work unowned on failure.

Solution: Put process ownership, concurrent readers, polling, graceful stopping, and completion behind one deep child Run interface, using the existing process adapter and local executable fixtures.

Deletion test: Removing this depth would return process supervision, pipe consumption, interruption, and cleanup to both callers; the implementation earns its interface.

Covered by: No open issue proposes this deepening; all open issue titles and bodies were checked.

Wins

• Locality: execution rules stay together
• Leverage: both callers gain ownership
• Tests cross one execution seam
• Delete caller-owned waiter threads
• Raw diagnostics preserve actual endings