Skip to content

One Spec PR module owns the Spec PR's draft, opening and Tickets checklist in a Spec run #367

Closed

Description

@JacobStephens2

What to build

Deepen the Spec PR into one Spec PR module that owns its whole lifecycle in a Spec run: resuming it as a draft while the Tickets run, opening it as a draft once the first Ticket lands, keeping its Tickets checklist up to date between the markers, putting the checklist back after the Spec review, and saying its URL to a Failed spec run. The Spec run's loop stops carrying the Spec PR as an optional pull request it decides about in five places. It tells the Spec PR module what happened: "show this checklist" and "a Ticket landed".

Nothing a user sees changes. The progress lines, the Spec PR's title and body, when it opens, the checklist text and markers, and the warning when GitHub refuses a checklist update all stay exactly as they are today.

Acceptance criteria

  • A Spec PR module, a sibling of the Ticket board inside the Spec run, is the only code that reads or writes the Spec PR during a Spec run's Ticket phase and around the Spec review: finding it, converting it to a draft, opening it, reading and rewriting its body.
  • Its interface is these five operations (see Decisions):
    • resume
    • show a checklist
    • a Ticket landed
    • put the checklist back
    • its URL
  • The Tickets checklist markers, the replace-between-markers rule and the Spec PR's opening body ("The work on Spec #N…", "Closes #N", the checklist) live only in the Spec PR module. The Ticket board's checklist is the ## Tickets heading and its lines, with no markers.
  • The Spec run's functions take the Spec PR module in place of an optional pull request plus the Base branch they only needed for opening it.
  • The existing unit tests for replacing the checklist between the markers move to the Spec PR module unchanged in substance. The Ticket board's checklist test expects the checklist without markers.
  • Every existing end-to-end Spec run test passes unchanged, in particular:
    • the Spec PR opens as a draft once the first Ticket lands, with the checklist
    • the Spec review rewrites the Spec PR body, and the checklist is put back after it
    • a Spec review that keeps the markers has the checklist replaced between them
    • a failed Spec review that rewrote the body has the checklist put back in the draft
    • a checklist update GitHub refuses is only a warning
    • a Ticket that fails once the Spec PR is open is shown failed in its checklist
  • cargo clippy --all-targets -- -D warnings and cargo test pass.
  • No CONTEXT.md or ADR change: Spec PR and Tickets checklist are already defined there, and this decision isn't worth an ADR.

Blocked by

None (can start immediately)

Decisions

  1. The deepened module is the Spec PR, a private sibling of the Ticket board inside the Spec run. It is named after the existing Spec PR term. Only the Spec run uses it, as only the Spec run uses the Ticket board.

  2. Interface: five operations.

    • resume(Spec, Spec branch, Base branch) finds the open Spec PR and converts it back to a draft if it was ready. This is today's behaviour at the start of a Spec run.
    • show(checklist) rewrites the checklist if the Spec PR is open and does nothing if not. It only warns on failure, with today's line. It is used after Tickets start, after a Ticket ends without landing, and after a failed Delivery.
    • landed(checklist) → URL opens the Spec PR as a draft if it isn't open, and otherwise shows the checklist. A failure to open is an error, as today. It returns the Spec PR's URL. It is used when a Ticket lands, and before the Spec review, which needs the URL for its Session prompt.
    • put_back(checklist) → Result is the strict rewrite run in Delivery's before-ready step. A failure there fails the Delivery, as today.
    • url gives the Spec PR's URL if it is open, for a Failed spec run.

    Why: those are the only decisions the Spec run takes about the Spec PR. Two named operations ("show" and "landed") replace a landed: bool flag and the three-arm match on the optional pull request.

  3. "landed" before the Spec review also shows the checklist when the Spec PR is already open. Today that point skips the write. The checklist is unchanged since the last write, so the only cost is one extra body read, and no write happens because the body already matches. Why: one rule for "a Ticket landed" instead of a special case. The prototype confirmed that every end-to-end test passes with it.

  4. The Spec PR module owns the Spec branch name as its own copy. It borrows the Spec and the Base branch. Why: the Spec branch is borrowed from the worktree, which moves into the Delivery while the Spec PR is still needed afterwards. The prototype hit this borrow error.

  5. The checklist markers move from the Ticket board into the Spec PR module. The Ticket board says what each line is. The Spec PR module says where the checklist sits in the body. Why: locality. The markers, wrapping the checklist in them and replacing between them are one piece of knowledge, today split across two modules through a shared constant.

  6. No new seam: GitHub stays behind the existing github functions, and tests stay on the fake gh. Why: the codebase has no trait seams anywhere. Its one real seam is the fake gh binary on PATH, and a trait with one adapter would be a hypothetical seam. Only the pure body edits get unit tests inside the module. The open, draft and warn rules stay covered by the end-to-end Spec run tests, which survive unchanged and are the regression guard.

  7. Out of scope: the Failed run path's own "find the open PR and draft it" step for a Run's pull request. Why: it has a different rule (keep it ready when only the Self-merge failed) and belongs to a deepening of the pull request handle, not of the Spec PR. That keeps this Ticket to one session.

  8. No CONTEXT.md or ADR change. Why: the module is named for an existing term, and the decision is easy to reverse and unsurprising.

Prototype. The design was prototyped on main at b108c41 and thrown away. The build and clippy were clean, and every unit and end-to-end test passed, including all 75 Spec run tests. These are the decision-rich parts of the module from the prototype:

pub(super) struct SpecPr<'a> {
    spec: &'a IssueUrl,
    branch: String,
    base: &'a str,
    pr: Option<PullRequest>,
}

impl<'a> SpecPr<'a> {
    pub(super) fn resume(spec: &'a IssueUrl, branch: &str, base: &'a str) -> Result<Self>;
    pub(super) fn show(&self, checklist: &str);              // warns on failure
    pub(super) fn landed(&mut self, checklist: &str) -> Result<&str>; // opens if none
    pub(super) fn put_back(&self, checklist: &str) -> Result<()>;
    pub(super) fn url(&self) -> Option<&str>;
}

In the Spec run's loop, the three-arm match becomes:

if landed {
    if let Err(open_error) = spec_pr.landed(&list) {
        error.get_or_insert(open_error);
    }
} else {
    spec_pr.show(&list);
}

Architecture review

Strength: Strong · Dependencies: local-substitutable (fake gh on PATH)

Files: src/spec_run.rs, src/spec_run/ticket_board.rs, src/spec_run/spec_pr.rs (new)

Before

flowchart LR
  run[Spec run: run] -->|Option PR| land[land_tickets]
  land -->|Option PR, base| loop[run_ready_tickets]
  run -->|Option PR| review[review_and_deliver]
  run -.draft_spec_pr.-> gh[github]
  loop -.open_spec_pr / write_checklist_or_warn.-> gh
  review -.open_spec_pr / write_checklist / _or_warn.-> gh
  board[Ticket board] -.markers.-> run
  classDef leak stroke:#dc2626,stroke-width:2px;
  class loop,review,board leak
Loading

After

flowchart LR
  run[Spec run: run] --> land[land_tickets]
  land --> loop[run_ready_tickets]
  run --> review[review_and_deliver]
  loop -->|show / landed| SpecPr
  review -->|landed / put_back / show| SpecPr
  run -->|resume / url| SpecPr
  subgraph SpecPr[Spec PR module]
    direction LR
    draft --> open --> checklist[markers + body]
  end
  SpecPr --> gh[github]
  board[Ticket board] -->|checklist lines| loop
  classDef deep stroke-width:3px;
  class SpecPr deep
Loading

Problem: the Spec PR's lifecycle is spread across the Spec run. An optional pull request is threaded through three functions, five places decide whether to open, draft or rewrite it, and its markers are shared with the Ticket board.

Solution: one Spec PR module, a sibling of the Ticket board, owns resuming, opening, the checklist markers and rewriting the body, behind five small operations.

Wins

  • Locality: Spec PR rules in one module
  • Spec run loop loses three-arm match
  • Markers stop leaking from Ticket board
  • Interface shrinks: no Option threading
  • Tests unchanged; pure edits unit-tested

Activity

  1. added
    needs-triageMaintainer needs to evaluate this issue
    ready-for-agentFully specified, ready for an AFK agent
    architect-planAn Architect plan: the Spec or Ticket an Architecture review published
    in-progressA Claim: thirdshift has taken this issue
    and removed
    needs-triageMaintainer needs to evaluate this issue
    ready-for-agentFully specified, ready for an AFK agent
    on Oct 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    architect-planAn Architect plan: the Spec or Ticket an Architecture review published

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions