Repository navigation
Description
What to build
A Run that another thirdshift started, a Ticket's Run in a Spec run or a Base fix, tells what started it how it ended through the same lines a standalone Run prints: what became of its Base fix, then either PR <url> is <outcome> or its cause followed by its session log, with its pull request's URL on stdout and its exit code. Today the writing and the reading of those lines live apart. The command's entry point writes them, in an order only a comment explains; the child Run module reads them back by keeping the last two lines of stderr; and the two line prefixes they agree on are constants owned by the Failed run path and the Base fix module. Nothing tests the two halves against each other except whole Spec runs and Base fixes driven end to end.
Put both halves in one module: how a Run's ending shows, and how a Run reads back the ending of a child Run it started. Its interface is small:
- Show how a Run or a Spec run ended: its lines on stderr, its pull request's URL on stdout, and the exit code. A failure on its own, as an Architect run whose Architecture review failed has, shows through the same module.
- Read a child Run's ending back: fed each line of the child's stderr as it is relayed, then the child's stdout and whether it exited 0, it gives what the child reached (its pull request's URL and what became of its Base fix) or how it failed (its cause and its session log).
The prefixes, the order of the lines and which of them carries what become private to that module. What a Run prints does not change.
Reading gets one fix that owning both halves makes possible. A cause can span several lines: a failed git or gh command's error carries the last lines of its output. Read back as "the last line before the session log", such a cause comes out as the tail of that output, so a Ticket's line in the Tickets checklist, in the Spec run's summary and in the Run notification, and the cause a Run gives when its Base fix fails, name a trailing hint instead of the command that failed. Read the cause as its first line instead, as the Failed run path already does for the failure commit's subject.
Acceptance criteria
- One module holds both what a Run or a Spec run shows as it ends and how a child Run's ending is read back. No other module names the session-log or Base fix line prefixes, or depends on the order the ending's lines are printed in.
- A Run, a Spec run and an Architect run print what they print today on stderr and stdout, with the same exit codes. The existing end-to-end tests pass without being edited.
- The child Run module still starts the child, relays its stderr under the issue's number, passes an interrupt on and reports an interrupted child; it reads how the child ended only through the new module.
- A child Run that fails with a cause of several lines is read back with the cause's first line, and with its session log when it showed one. Every line is still relayed as it is today.
- Only the child's own progress lines are read as its ending. The lines it relayed from a Run it started itself, such as a Ticket's Run relaying its Base fix, are never taken for its cause, its session log or its Base fix.
- A child that exits non-zero having shown nothing still fails with its exit status as the cause.
- Unit tests in the module take each kind of ending through showing and reading and compare what comes back: ready for review; merged; reached after a Base fix; failed with a session log; failed with none; failed with a pull request left open; failed with a cause of several lines; failed after a Base fix that did not merge; and an ending shown after lines relayed from a child of its own. They go through the same formatting and parsing a progress line gets, and start no process.
- The progress module tells a relayed line that is one of thirdshift's own progress lines from one that continues the line before it, with a unit test.
Blocked by
None (can start immediately).
Decisions
- The lines a Run prints stay the transport; a child Run gets no private channel (no result file, no extra file descriptor, no machine-readable mode). ADR-0006 chose child processes so that "a child process keeps each Run exactly what a standalone Run is"; a second channel would make a child Run a different thing from a standalone one and give the ending two renderings to keep in step.
- Both halves in one new module, not in the child Run module. A standalone Run shows its ending too, so the showing half is not about child Runs; the child Run module keeps what is about processes (start, relay, interrupt, wait). The reading half moves next to the showing half because they change together: that is the locality the refactor is for.
- The interface is show and read. Showing takes how the Run ended and returns the exit code; showing a failure alone is exposed for the Architect run. Reading is fed lines one at a time, because the child's stderr is relayed as it arrives, and finished with stdout and the exit status. Interruption stays with the child Run module, which is what knows an interrupt was passed on.
- The prefixes move in and become private. The Failed run path and the Base fix module export them today only so that two other modules can agree on them. The other places that print a session log or a Base fix, such as a Ticket's summary line and the Run notification's body, are different texts and keep their own wording.
- What is printed does not change, including the Base fix line coming before the outcome. The end-to-end tests pin it, and people read it. The order stops being something the reader relies on.
- A cause of several lines is read back as its first line. The texts a read-back cause goes into are one line each: a checklist item, a summary line, a notification line, the cause of the Run that waited on a Base fix. The first line names what failed; the last is whatever the command printed last. The Failed run path already makes the same choice for the failure commit's subject. The full text is still relayed to stderr and is in the child's session log.
- A line counts toward the ending only if it is one of the child's own progress lines. The progress module already strips the prefix every such line starts with when it relays one; it now also says whether the prefix was there, which is how a continuation line is told from a new one. Lines a child relayed from its own child carry that Run's number first, so they match no prefix, and a test holds that.
- What became of the Base fix is read back whichever way the child ended, since it is shown either way. Callers keep using it only for a Ticket that landed, so no line changes.
- A child Run's arguments are out of scope. What a child Run is told is typed at both ends already, the argument parser is its one reader, and it has shown no defect. The four types that describe an ending (the Run's, the child Run's, a Ticket's standing, the notification's) are left as they are too: this Ticket moves where the wire format lives, not who consumes it.
- Tests: unit tests at the new module's interface, in-process. The module's dependencies are in-process once showing builds its lines as values before printing them, so it needs no adapter and no seam beyond its interface. The round trip is the test: show an ending, read it back, compare. There are no old unit tests to replace: neither the entry point nor the child Run module has any. The end-to-end Spec run and Base fix tests stay as the check that real processes are wired to it. Prior art: the argument parser's and the Run notification body's unit tests.
- No CONTEXT.md change and no ADR. The module is named for what it does and adds no domain term; the glossary already says how a Run ends. Nothing here is hard to reverse, and ADR-0006 stands as written.
Architecture review
Strength: Strong · Dependencies: in-process
Files: src/main.rs, src/child_run.rs, src/failed_run.rs, src/base_fix.rs, src/progress.rs, src/spec_run.rs
Before
flowchart LR
main["main: shows the ending"] --> wire[("child's stderr and stdout")]
wire --> child["child Run: reads the last two lines"]
failed["Failed run path: session-log prefix"] -.leak.-> main
failed -.leak.-> child
basefix["Base fix: report prefix"] -.leak.-> main
basefix -.leak.-> child
child --> spec["Spec run: Ticket's standing"]
child --> basefix
classDef leak stroke:#dc2626,stroke-width:2px;
class failed,main,child leak
After
flowchart LR
main["main"] --> show
subgraph ending["Run ending"]
direction LR
show["show"] --> lines["lines, prefixes, order"]
lines --> read["read"]
end
child["child Run: start, relay, interrupt"] --> read
child --> spec["Spec run: Ticket's standing"]
child --> basefix["Base fix"]
failed["Failed run path"]
classDef deep stroke-width:3px;
class ending deep
Problem: A Run's ending is written in one module and read back in another, joined by two prefixes that two more modules own and an ordering rule kept in a comment, so the reader guesses from the last two lines and loses a cause of several lines.
Solution: One module owns the ending's lines both ways, showing and reading, with the prefixes and order private to it.
Wins
- Locality: wire format in one module
- Interface: show and read
- Round trip tested in-process
- Prefix constants stop leaking
- Several-line causes name what failed
Activity