Repository navigation
Description
What to build
An Architecture review ends on the issue the last line of its final message names. The Architect run then concludes on it: a plan is checked (in this repository, open, created since the run started, no Unready label but needs-triage) and marked ready with architect-plan; an idea, or the open issue that already covers it, is labelled an Architect idea. Today that conclusion calls GitHub directly, reading the plan twice (once for its checks, once more for its label Edit), so its rules are tested only end to end, by running the binary against a fake gh.
Change it so the conclusion reads and changes GitHub through the Pass seam the Architect run already holds for its gates, and becomes one function over that seam that returns how the review ended, tested in-process against the shared in-memory adapter. #374 anticipated this: "The Architect plan's checks after the review stay out of scope … They can later sit behind the same seam."
Nothing a user sees changes. Every Architect run ends the same way, with the same progress lines, causes, labels, stdout, exit codes and Run notifications, and makes no more GitHub requests than now: one fewer for a plan.
Acceptance criteria
- The Pass seam gains exactly two operations:
- view one issue by its Issue URL: its state, labels and creation time, as the GitHub module's issue read gives them now;
- ask whether an interrupt was requested.
The production adapter does each through the code that does it now. The in-memory adapter holds the issues it can view (an issue it holds none for fails to view, asghwould), and whether an interrupt was requested. It records each view in its call record, in order with the other calls.
- The Architect run's conclusion is one function over the seam, given the final message, the origin, the time the run started and the Base branch. It returns how the review ended, as now, or fails with the same causes as now.
- A plan is viewed once. Its checks and its label Edit both use that one read: the Edit is built from the viewed labels, not read again. The checks keep their order: origin first (with no GitHub read when the plan is in another repository), then open, then created since the run started (to the second), then no Unready label once
needs-triageis taken off. - An idea, or an already-filed issue, is viewed once and its Edit built from the viewed labels. A failure to view or to edit it still fails with
could not label the Architect idea #<n>ahead of the cause. - An interrupt is still checked after a plan's checks and before its Edit, and before an idea is viewed, through the seam. An interrupted conclusion makes no Edit.
- The conclusion's progress lines (the plan published, the label swap, the idea labelling) go through the seam, in the same wording and order. The line that starts the review stays where it is.
- The label-edit module's constructor that reads an issue's labels itself is deleted, as the conclusion was its only caller. Every Edit is then built from labels its caller has read.
- Unit tests in the Architect run's module cover, through the conclusion function and the in-memory adapter:
- a plan marked ready:
needs-triageswapped forready-for-agent,architect-planadded, its other labels kept, in one Edit;needs-triagespelled in another case taken off; - each refusal of a plan, with its cause and no Edit: another repository (and no view), closed, created before the run started, each of
ready-for-human,needs-infoandwontfix; - an idea filed and an idea already filed each labelled
needs-triageandarchitect-idea, with its other labels kept,needs-triageput back on one that had been triaged; - an idea whose view or Edit fails, with its cause;
- an interrupt before a plan's Edit and before an idea's view;
- a final message with no line as the prompt asks for failing with no call made;
- the progress lines in order with the view and the Edit.
- a plan marked ready:
- Replace, don't layer. Delete these end-to-end tests in the Architect run suite, whose rules the unit tests now cover: the published Spec keeping its Tickets' and its other labels; an idea already filed and triaged going back to
needs-triage; a closed plan refused; a plan with another Unready label refused; a plan in another repository refused. Keep every other Architect run test, among them: the plan marked ready and its URL printed; each label created when the repository lacks it; an idea that cannot be labelled (it reaches the label adapter's writes); a plan older than the run refused (the only one reachingcreatedAtthroughghhere); a plan withneeds-triagein another case dispatched; and every notification, lock, preflight and dispatch test. - The Pass module's doc says the seam now also serves the Architect run's conclusion after its review.
CONTEXT.md's Pass entry is unchanged: the term names the command, not the seam's reach. -
cargo test,cargo clippyandcargo fmt --checkpass.
Blocked by
None (can start immediately).
Decisions
- The same Pass seam, not a new one. The Architect run already holds the Pass seam's production adapter for its gates, and its in-memory adapter already records Edits and progress lines. The conclusion needs only two more operations. A seam of its own would add a tenth seam trait and a ninth fake for no variation; the codebase already has seven traits re-wrapping the same issue reads, Edits, progress lines and interrupt checks.
- The seam views an issue whole. One operation gives state, labels and creation time, which a plan's checks need and an idea's Edit needs part of. One read covers both; a separate labels-only read would widen the interface for one call.
- A plan is read once, and its Edit is built from that read. Today the checks and the Edit read the plan separately, so the Unready label check could pass on one set of labels and the Edit be made against another. One read makes the checked labels the edited labels, and saves a request. The Edit still keeps the plan's other labels, as read.
- The interrupt crosses the seam. Without it, an interrupted conclusion can be tested only by signalling the process. It is a one-line operation in both adapters.
- Progress lines cross the seam, except the review's starting line. The conclusion's lines are its observable record, and tests assert their order with its reads and writes. The starting line belongs to the session's start, not the conclusion, and stays as it is.
- Deleting the reading Edit constructor. With the conclusion building its Edits from viewed labels, no caller is left. Deleting it takes I/O out of the label-edit value type's construction: the deletion test says it was a pass-through.
- The Pickup run is unaffected. Its adapter is the same production adapter and gains the two operations unused. No Pickup run code or test changes.
- No
CONTEXT.mdchange and no ADR. No new domain term is introduced. Nothing here is hard to reverse or a real trade-off beyond what Test the Ready issue rules in-process behind a seam for the search's reads #370, Test the Repair loop's rules in-process behind a seam for its outside effects #372 and Test the Architect run's and Pickup run's skip rules in-process behind a Pass seam #374 settled for in-process seams.
Testing decisions
- The interface is the test surface. Tests drive the conclusion through its one function with the in-memory adapter, asserting only on the returned ending or cause and on the adapter's call record.
- The kept end-to-end tests check that the production adapter maps
ghoutput and label writes to the same facts, and that a conclusion's failure still reaches stderr, the exit code and the Run notification.
Architecture review
Strength: Strong · Dependencies: mock (GitHub is a true external dependency)
Files: src/architect.rs (conclude, mark_plan_ready, label_idea, check_plan, review), src/pass.rs, src/labels.rs (Edit::read), src/github.rs (issue), tests/architect_run.rs
Before
flowchart LR
R[Architecture review] --> C[Conclusion]
G[Architect gates] --> X[Pass seam]
C -.-> GH[GitHub issue read]
C -.-> E[Edit read + apply]
E -.-> GH
C -.-> I[interrupt flag]
T[architect_run tests] --> FG[fake gh binary] --> C
classDef leak stroke:#dc2626,stroke-width:2px;
class GH,E,I leak
After
flowchart LR
R[Architecture review] --> C[Conclusion]
G[Architect gates] --> X
C --> X
subgraph X[Pass seam]
direction LR
GA[GitHub adapter] --> GH[GitHub issue read]
M[in-memory adapter]
end
U[unit tests] --> C
U --> M
classDef deep stroke-width:3px;
class C,X deep
Problem: the Architect plan's checks and the Architect idea's labelling call GitHub directly, reading the plan twice, so their rules are tested only end to end against a fake gh.
Solution: the conclusion reaches GitHub through the Pass seam the run already holds, gaining an issue view and an interrupt check, and is tested in-process.
Wins
- Leverage: one seam, gates and conclusion
- Plan checked and edited on one read
- Edit construction loses its I/O
- Delete five rule-only end-to-end tests
Activity