Skip to content

Test the Ready issue rules in-process behind a seam for the search's reads #370

Closed

Description

@JacobStephens2

What to build

The Ready issue search is the one place that decides which issue a Pickup run takes, and whether an Architect run gives way to a Ready issue. Today its rules can only be checked by running the binary against a fake gh and a git origin. Change it so the search reads its outside facts through a seam inside the module. Its rules, their order, and every passed-over line can then be tested in-process, against an in-memory adapter.

Nothing a user sees changes. Every Pickup run and Architect run takes the same issue, skips for the same reasons, and prints the same passed-over lines, in the same order, as each issue is passed over. It also makes the same reads of GitHub and origin, and no more.

Acceptance criteria

  • The search's two outside reads sit behind one seam inside the Ready issue module: an issue's candidate read (parent, sub-issues, blockers, last shaping) and whether the issue was started (an Issue branch on origin or a pull request). The production adapter reads GitHub and the Launch directory, as the search does now. Its tests use an in-memory adapter.
  • The search takes the current time as a value, read once per pass. It never reads the clock itself.
  • The search hands each passed-over line to a caller-supplied sink as soon as the issue is passed over. Production prints each one at once, as now. Tests collect them.
  • The module's public interface is unchanged: its two callers, the Pickup run and the Architect run, change only if the compiler makes them.
  • Reads stay lazy, as now: no candidate read for an issue passed over by its labels, and no started check for an issue passed over for its parent, its Base fix label or its blockers. Unit tests assert this through the in-memory adapter's record of the reads made.
  • Unit tests in the Ready issue module cover, through the search:
    • every rule, and its passed-over line;
    • the first-reason-that-applies order;
    • a Ticket whose Spec is a Ready issue getting no line, and a Ticket whose Spec is not ready naming it, both above and below the Spec's number;
    • the settle window at, just under and just over ten minutes, for each kind of shaping;
    • a Spec whose Tickets are all closed, started and not started;
    • the Spec-or-not answer on the Ready issue it returns.
  • Replace, don't layer. Delete each end-to-end test in the Pickup run suite that only checks rule order or line wording, once the unit tests cover it and no kept test is left as the only one reaching a read it reached. Keep at least one end-to-end test for each read the production adapter makes:
    • the labels;
    • the parent;
    • the blockers;
    • the timeline, for both label and link shaping;
    • the sub-issues' states;
    • the Issue branch on origin;
    • the pull request.
  • cargo test, cargo clippy and cargo fmt --check pass.

Blocked by

None (can start immediately).

Decisions

  • Deepen the Ready issue module in place, rather than merge it with the Ticket board. Both check unready labels, sub-issues and open blockers. But they answer different questions: whether a Pickup run may take a top-level issue, and whether a Spec run may start a Ticket now. The facts differ too, since the Ticket board counts running blockers and never looks at Claims or settling. Merging them would tie two definitions in CONTEXT.md (Ready issue, Unready Ticket) together for little gain. They already share the label predicate, and that is enough.
  • The seam is internal, not on the public interface. Callers keep asking for the first Ready issue of a repository from its Launch directory. The seam serves only the module's own tests. Exposing it would widen the interface without giving any caller something new.
  • Two adapters justify the seam: GitHub plus git in production, in-memory in tests. GitHub is a true external dependency, so per the dependency categories it is injected and mocked in tests. A smaller fake gh would only keep the tests end to end.
  • One port with two reads, not one per rule. The rules need exactly two outside facts per issue, read lazily in their current order. A narrower port per rule would leak the rule order into the port. A single "all facts" read would lose the laziness that keeps a pass cheap.
  • Labels come in with the listing, not through the port. The listing is read once per pass, before the search, and already holds each issue's labels. Tests build the listing directly.
  • Time is a value, read once per pass. Today the clock is read once per candidate. Over one pass the difference is negligible, and a fixed now makes the settle window testable to the second.
  • A line sink, not a returned list of lines. Each line reaches stderr as its issue is passed over, so a pass slowed by many reads still shows progress. A returned list would hold every line back until the search ended.
  • The rules stay one function, applied in one fixed order. The first reason that applies is the one printed, and that order is documented behaviour that the tests now pin down. It is not to be made data-driven.
  • No CONTEXT.md or ADR change. No domain term is added or sharpened: the seam is an implementation detail inside the module. Nothing here is hard to reverse.

Testing decisions

  • The interface is the test surface: unit tests drive the search through its entry point inside the module, with the in-memory adapter, a fixed now and a collecting sink. They assert only on the Ready issue returned, the lines collected and the reads made, never on internal state.
  • The end-to-end Pickup run tests that are kept still check that the production adapter maps gh and git output into the same facts. The fake gh's state keys for timelines, blockers and sub-issues stay reached by at least one kept test.

Architecture review

Strength: Strong · Dependencies: mock (GitHub is a true external dependency; the Launch directory's origin read rides along in the same adapter)

Files: src/ready.rs, src/github.rs (candidate, Candidate), src/branch.rs (started), tests/pickup_run.rs

Before

flowchart LR
  P[Pickup run] --> S[Ready issue search]
  A[Architect run] --> S
  S -.-> GH[github candidate read]
  S -.-> B[branch started check]
  S -.-> C[clock]
  S -.-> O[progress stderr]
  T[pickup_run tests] --> FG[fake gh + git origin] --> S
  classDef leak stroke:#dc2626,stroke-width:2px;
  class GH,B,C,O leak
Loading

After

flowchart LR
  P[Pickup run] --> R
  A[Architect run] --> R
  subgraph R[Ready issue module]
    direction LR
    S[search rules] --> port[reads seam]
  end
  port --> GA[GitHub + git adapter]
  port --> MA[in-memory adapter]
  U[unit tests] --> S
  classDef deep stroke-width:3px;
  class R deep
Loading

Problem: the Ready issue rules, their order and their lines interleave with gh, git and clock reads. So each rule is tested only through the binary, with about 20 Pickup run tests each building fake gh state, and the module has no unit tests.

Solution: put the two outside reads behind a seam inside the module, and pass the time and a line sink in. The rules are then tested in-process against an in-memory adapter.

Wins

  • Tests hit one interface
  • Locality: rule order tested in one place
  • Two adapters make a real seam
  • Fewer fake-gh end-to-end tests

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