Test the Base fix's rules in-process behind a seam for its outside effects #378

What to build

The Base fix module's rules get tested in-process, behind a seam for what they do outside. This follows the Repair loop, Pass, Ready issue and Issue branch selection seams, and changes no behaviour.

Today the Base fix module decides a lot:

• one Base fix per Run;
• whether the Run may start one;
• the advice after a cause;
• whether an open Base fix issue covers the Run's checks;
• restarting a Base fix that was left open;
• waiting on another Run's Base fix rather than starting one;
• what it reports.

It makes those decisions in between hard-wired effects: the look lock, the issue listing, the mark files, writing the issue, starting and waiting on the child Run, polling the issue and printing progress lines. None of its rules has a unit test. Every one is checked only end to end, through the fake gh and claude binaries.

After this Ticket:

• The Base fix module takes a private Outside seam with two adapters: the production one, and a scripted fake in the module's own tests.
• Each rule is a fast unit test.
• The end-to-end tests that only checked a rule or a line's wording are gone. The ones that need real processes, real locks or the whole Run stay.

Acceptance criteria

• The Base fix module's public interface is unchanged. Construction, sees_inherited_failures, ask_of_tickets, fix, into_advice, report, Advice and the label helpers all stay. The Delivery module and the Repair loop's Outside need no change.
• fix builds the production adapter over the Launch directory and the Run's issue, then hands off to a private function that holds every rule and reaches the outside world only through the seam.
• The private seam has the shape set out under Decisions. The production adapter makes the same gh calls, file locks, mark files, child Run and poll interval as today.
• Finding the open Base fix issue that covers the checks is a pure function over the listed issues: the oldest one, not the Run's own issue, whose title names the Base branch and every inherited check, including a check name that holds , .
• Unit tests against the scripted fake cover each of these:
  • Not allowed (Forbid): fails with the cause and links each Base branch check. Writes no issue and takes no lock.
  • Undecided: the same, plus the Retry with and Or set advice.
  • Allowed, with no covering issue: takes the look, writes the issue with the fixed title, body and labels, marks it running, releases the look, waits on the child Run, and on success says it merged and clears the mark.
  • A child Run that fails: the cause names the issue, and the session log line is printed when the child gave one.
  • A child Run that is interrupted.
  • A second Inherited failure after the one Base fix: the cause carries , even after Base fix <url> merged (or closed) and no advice. Nothing is looked up or written.
  • A covering issue found, with its mark showing it running from here: releases the look before waiting, then waits until the issue closes, or fails, naming the issue, when that Run ends with the issue still open.
  • A covering issue found with no mark (another clone or machine): waits on the issue alone.
  • A covering issue found with a Left mark: restarts that same issue and writes none.
  • A covering issue for another Base branch or other checks is not taken.
  • An interrupt during the wait.
  • report for started or waited-on, reached or not.
  • ask_of_tickets for each ask.
  • A Run that is itself a Base fix sees no Inherited failures.
• End-to-end Base fix tests whose rule is now unit-tested are deleted. That is: still-red checks after a merged Base fix, waiting on a found open Base fix, a found one that closes with checks still red, a comma in a check name, an open one for another Base branch or other checks, no Base fix writes no issue, no Base fix links checks with no offer, and a Base branch check with no URL.
• These end-to-end tests are kept, because they need real processes, real locks or the whole Run:
  • the happy path;
  • the binary replaced mid-run;
  • only the Run notifies;
  • the User config's base.fix in the child;
  • the Merge run self-merges after;
  • the Base branch whatever the Launch directory has checked out;
  • a failing Base fix (draft and notification);
  • a Left mark restarted across processes;
  • labels and Claim;
  • one advice test that reaches stderr and the notification;
  • the missing label is added;
  • help;
  • the Spec run's shared Base fix tests.
• No change to CONTEXT.md or any ADR is needed. Base fix and Inherited failure are already defined, and ADR-0008 stands.
• cargo test and cargo clippy pass.

Blocked by

• None (can start immediately)

Decisions

• Seam location: private, inside the Base fix module. This mirrors Issue branch selection (select builds the adapter and calls the _through function). The Delivery module's RunOutside::base_fix already stands in for the whole Base fix at the Repair loop's seam, so widening that seam would add nothing. Keeping the new seam private keeps the public interface unchanged.
• Two adapters: the production gh/Launch directory adapter, and a scripted fake. The second adapter is what makes this a real seam, not a hypothetical one.
• Seam shape. The snippet below is a design, not prototype output:

  trait Outside {
      type Look;    // the look lock, held until dropped
      type Running; // this Run's mark that it runs the Base fix it started
      type Watch;   // an opened mark of a Base fix found running from here
      fn look(&mut self) -> Result<Self::Look>;
      fn open_fixes(&mut self) -> Result<Vec<ListedIssue>>;     // open `base-fix` issues in the Run's repository
      fn mark(&mut self, number: u64) -> Result<Mark<Self::Watch>>; // Running(watch) | Left | None
      fn has_ended(&mut self, watch: &Self::Watch) -> bool;
      fn issue_is_open(&mut self, issue: &IssueUrl) -> Result<bool>;
      fn create_issue(&mut self, title: &str, body: &str, labels: &[Label]) -> Result<IssueUrl>;
      fn mark_running(&mut self, number: u64) -> Result<Self::Running>;
      fn merged(&mut self, running: Self::Running);            // removes the mark file
      fn run_base_fix(&mut self, fix: &IssueUrl, base: &str) -> Result<Ended>; // start the child Run and wait
      fn pause(&mut self) -> Result<()>;                       // one poll interval; fails `interrupted`
      fn step(&mut self, line: String);
  }

• Associated types for the lock, the running mark and the watched mark. Production holds real File locks, and a lock's lifetime is the rule ("held from the look until the issue is written and marked running"; "released before waiting on another's"). Guards that drop like the real ones let the fake record when they are released. The Watch keeps today's open file descriptor. Reopening the file by path on each probe would change how a merged fix, whose mark file was removed, is read.
• Left detection stays in the production adapter's mark. "The file exists and its lock is free when first opened" is a fact about file locks, not a Base fix rule. The rule, "Left means start that issue again", is in the core and unit-tested.
• The core owns the wait loop, and the adapter supplies pause. poll::until sleeps for the real poll interval and borrows its probe, so the core loops itself: read the mark before the issue (a Base fix closes its issue before it ends), stop when the issue is closed or the mark has ended, otherwise pause. pause gets a public one-interval sleep in the poll module, which fails as interrupted just as poll::until's sleep does. The fake records each pause and can be scripted to be interrupted.
• Labels and the issue's title and body stay in the core. They are Base fix rules (base-fix plus ready-for-agent, the fixed template), so the fake records them and the tests check them.
• Starting the child and waiting on it are one seam method. The waiting on Base fix #n line is printed by the core just before that call. The only difference from today: if starting the child fails, that line comes before the error rather than not at all. It is a progress line on a path that fails anyway, and one method avoids a fourth associated type.
• The fake follows the Repair loop's scripted style. Per-call answers come from VecDeques, and a Did log is kept in an Rc<RefCell<…>> so the Look and Running guards can record their own drops. Tests assert the Did sequence and the returned error text.
• Replace, don't layer. End-to-end tests that only checked a rule now covered in-process are deleted, as 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, Test the Architect run's and Pickup run's skip rules in-process behind a Pass seam #374 and Test Issue branch selection in-process behind a seam for its reads of origin, GitHub and the Launch directory #376 did. End-to-end tests of the production adapter's real effects stay.

Architecture review

Strength: Strong · Dependencies: local-substitutable (file locks and the child Run) plus remote (gh), behind ports & adapters

Files: src/base_fix.rs, src/poll.rs, tests/base_fix.rs

Before

flowchart LR
  Delivery[Delivery: RunOutside] --> Fix[Base fix module: fix]
  Fix -.-> Lock[look lock file]
  Fix -.-> GH[gh: list / create / view issue]
  Fix -.-> Marks[mark files]
  Fix -.-> Child[child Run]
  Fix -.-> Poll[poll::until]
  E2E[tests/base_fix.rs + fake gh/claude] --> Delivery
  classDef leak stroke:#dc2626,stroke-width:2px;
  class Lock,GH,Marks,Child,Poll leak

After

flowchart LR
  Delivery[Delivery: RunOutside] --> Fix
  subgraph Fix[Base fix module]
    direction LR
    rules[one Base fix, covering issue, restart Left, wait, advice, report] --> Seam[Outside seam]
  end
  Seam --> Prod[production adapter: gh, locks, marks, child Run, poll]
  Seam --> Fake[scripted fake]
  Unit[unit tests] --> Fix
  classDef deep stroke-width:3px;
  class Fix deep

Problem: The Base fix module mixes seven rules with hard-wired gh calls, file locks, the child Run and polling. Not one of its rules has a unit test, so each is checked only through slow end-to-end tests.

Solution: The rules reach the outside world only through a private Outside seam, with a production adapter and a scripted fake. The public interface and the behaviour stay as they are.

Wins

• Locality: Base fix rules tested in one place
• Interface is the test surface
• Two adapters: real seam
• Delete nine end-to-end rule tests
• A testable core for Report an Inherited failure as one issue when a Run may not start a Base fix #272