Repository navigation
Description
What to build
Test the Spec PR's rules in-process, behind a seam for their outside effects, and have the Delivery write the "Built with" line in the pull request's body through its own seam. This follows the same pattern as the Claim, Delivery, Run start, CI watch, Base fix, Spec run Ticket loop and Sessions seams.
The Spec PR's rules are:
- When a Spec run resumes and the Spec PR is open but not a draft, the Spec PR goes back to a draft while the Tickets run. A closed Spec PR counts as no Spec PR.
- The Spec PR opens as a draft once the first Ticket lands. Its title is the Spec's, and its body is the fixed text,
Closes #<n>and the Tickets checklist between its markers. - Showing the checklist is only a warning when it fails. It writes nothing if the body would not change, and does nothing when there is no Spec PR.
- Putting the checklist back after the Spec review fails the Delivery when it fails, and fails when there is no open Spec PR.
Today these rules call GitHub directly, so only the end-to-end tests in the Spec run's suite reach them, through the fake gh. The same goes for the Delivery's "Built with" line. Delivery hands its steps a closure that ignores the Outside it is given and calls GitHub straight from inside, which goes around the Delivery's own seam.
After this change:
- The Spec PR module keeps its rules and reaches GitHub and the progress lines only through one private seam. It has two adapters: the real one on GitHub, and an in-memory one in its tests.
- The Delivery writes the "Built with" line as one of its steps, through its existing
Outside.
Behaviour seen from outside stays the same: the same GitHub requests in the same order, the same progress lines and warnings, and the same pull request bodies.
Acceptance criteria
-
The Spec PR module declares a private
Outsidetrait in the style of the other seams (&mut self, progress lines handed on asString). It covers exactly what the Spec PR does outside itself:- read the pull request from the Spec branch, if there is one;
- convert it to a draft;
- read the Spec's title;
- create the draft pull request with a title and body;
- read and set the pull request's body;
- hand on a progress line.
-
The real adapter holds the Spec's Issue URL and the Spec branch, and calls the same GitHub functions the module calls today. The Spec run's Ticket loop adapter and the Spec run's review-and-deliver step use the Spec PR exactly as before. Its constructor for production keeps its current arguments, and a second constructor takes an adapter, for the tests.
-
The Spec PR's rules reach GitHub and the terminal only through that
Outside. The opening line, the line about updating the checklist and the warning when the checklist could not be updated are all handed on through it, with their text unchanged. -
The Delivery's
Outsidegains reading and setting the pull request's body. Its route writes the "Built with" line as a step after the push and before the caller's before-ready step and the PR check:- It writes nothing when there is no pull request, or when the body already has the line.
- Its progress line goes through
step. - A failure is a warning through
warn, with the same text as today, and the Delivery carries on.
-
The before-ready closure that the Delivery's real path hands its route no longer calls GitHub itself. It only runs the caller's before-ready step.
-
In-process tests drive the Spec PR through an in-memory adapter that holds the pull request (number, URL, draft or not, open or closed), its body and the Spec's title. The adapter records each call and can fail any one call. The tests cover at least:
- resuming with a ready open PR converts it; resuming with a draft one, a closed one or none converts nothing, and a closed one has no URL;
- landing with no PR opens one, with the opening line, the Spec's title and the body (text,
Closes #<n>, checklist between markers), and returns the URL read back; landing when the PR just opened can't be read back fails with "the Spec PR just opened is not found"; - landing with a PR open writes the checklist and creates nothing;
- showing with no PR makes no call, showing an unchanged checklist reads the body but does not set it, and a failing set is the warning line, not an error;
- putting back with no PR fails with "the Spec PR is not open", and a failing set fails it.
-
In-process tests of the Delivery's route, using its existing scripted
Outsideextended with a body, cover:- the "Built with" line written after the push and before the caller's before-ready step and the PR check;
- nothing set when the line is already there;
- nothing read when there is no PR, with the route then failing on "no PR found" as now;
- a failing body write is a warning, and the route goes on to mark the PR ready.
-
Replace, don't layer. Delete two end-to-end tests from the Spec run's suite, whose rules the in-process tests now cover:
- the one that a checklist update GitHub refuses is only a warning;
- the one that a Spec review keeping the markers has the checklist replaced between them.
Keep the end-to-end tests that check the wiring: the Spec PR opening as a draft once the first Ticket lands, the checklist put back after the Spec review (which also pins the "Built with" line before the checklist), and the checklist put back in the draft after a failed Spec review. Keep the "Built with" tests in the Harness choice and Codex suites, which check the Harness wiring.
-
The two pure body splicers, for the "Built with" line and for the Tickets checklist, stay as they are, with their tests.
-
cargo testandcargo clippypass. No change toCONTEXT.mdor an ADR is needed.
Blocked by
- None (can start immediately)
Decisions
- The Spec PR module stays separate from the Delivery. The Spec PR exists before the Delivery starts: it resumes as a draft, opens on the first Ticket and shows the checklist as each Ticket starts and ends, all in the Ticket loop. Folding it into the Delivery would teach the Delivery about the Ticket loop. The Delivery already serves a Run and a Spec run alike through its opening session and the caller's before-ready step, and that stays.
- The seam is private to the Spec PR module, named
Outside. It follows the repo's other in-process seams. Callers never see it. The Ticket loop's ownOutside, which stubsshowandlandedwhole, stays as it is. - The Spec PR is generic over its adapter, not boxed. Its one production user, the Ticket loop's adapter, names the concrete GitHub adapter, so the generic parameter goes no further. Methods that call out take
&mut self. The before-ready closure that puts the checklist back borrows the Spec PR mutably, and it is dropped before the failure path shows the checklist again, so the borrows don't overlap. - The real Spec PR adapter holds the Spec and the Spec branch. Every GitHub call the Spec PR makes is about that one pull request, so the seam's methods take only what changes between calls: a number, a title, a body.
- The "Built with" write moves inside the Delivery's route, through the Delivery's
Outside. It is a Delivery step, written for a Run and a Spec run alike. Today it is the one GitHub call in the Delivery's steps that goes around the seam, so no in-process test can see it or its warning. Adding body read and set to the existingOutsideis enough; no new seam is needed. - The caller's before-ready step stays a closure. For a Spec run it is the Spec PR putting its checklist back. Once the Spec PR has its own seam, that closure is tested where it lives.
- Order and requests are unchanged. "Built with" first, then the checklist put back, so the body still ends with the line followed by the checklist. The pull request is still read once for the line and once for the PR check. A shared read would save one
ghcall, but it would change the requests the end-to-end fakes see, for no gain in depth. - The two body splicers are not merged. The "Built with" line uses one end-of-line marker and the checklist uses a start and end pair. Both formats are already in the bodies of live pull requests. One splicer would need to keep reading the old format, which adds code rather than removing it.
- Tests: replace, don't layer. End-to-end tests of rules the in-process tests now cover go. Those that check the wiring between the Ticket loop, the Spec review, the Delivery and the Harness stay.
- No glossary or ADR change. Spec PR, Tickets checklist, Spec review and Delivery are already defined in
CONTEXT.md. Adding a seam the way the repo's other seams are added is neither surprising nor hard to reverse.
Architecture review
Strength: Strong · Dependencies: mock (GitHub through gh; an in-memory adapter in-process, the real one in production)
Files: src/spec_run/spec_pr.rs, src/spec_run.rs, src/delivery.rs, tests/spec_run.rs
Before
flowchart LR
Loop[Ticket loop] --> SpecPr[Spec PR module]
Review[review and deliver] --> SpecPr
Review --> Delivery
subgraph Delivery[Delivery]
direction LR
route[route steps] --> Outside{{Outside seam}}
route -.before-ready closure.-> builtwith[Built with write]
end
SpecPr -.direct.-> GH[GitHub via gh]
builtwith -.direct.-> GH
Outside --> GH
E2E[e2e tests: fake gh] -.only way in.-> GH
classDef leak stroke:#dc2626,stroke-width:2px;
class SpecPr,builtwith,E2E leak
After
flowchart LR
Loop[Ticket loop] --> SpecPr
Review[review and deliver] --> SpecPr
Review --> Delivery
subgraph SpecPr[Spec PR module]
direction LR
rules[Spec PR rules] --> SOut{{Outside seam}}
end
subgraph Delivery[Delivery]
direction LR
route[route steps + Built with] --> DOut{{Outside seam}}
end
SOut --> GH[GitHub via gh]
DOut --> GH
SOut --> Mem[in-memory adapter]
DOut --> Scripted[scripted adapter]
classDef deep stroke-width:3px;
class SpecPr,Delivery deep
Problem: the Spec PR's rules and the Delivery's "Built with" write call GitHub directly, so only the fake gh reaches them, and the "Built with" write goes around the Delivery's own seam.
Solution: give the Spec PR module a private seam with a GitHub adapter and an in-memory adapter, and route the "Built with" write through the Delivery's existing Outside.
Wins
- Tests hit Spec PR rules in-process
- Delivery's seam stops leaking
- Two adapters justify the seam
- Locality: Spec PR rules in one module
- Delete two slow end-to-end tests
Activity