Skip to content

Deepen CI watch to own Delivery's per-head re-run policy #955

Closed

Description

@JacobStephens2

What to build

Deepen the CI watch module so one interface owns CI observation, Abandoned job re-runs and Check re-runs throughout a Delivery. Runs and Spec PRs must keep their existing outcomes, progress lines, stages and failure causes. The Repair loop should request an assessment and report a successfully completed CI-fix Repair, without choosing low-level CI operations or tracking their allowances.

Acceptance criteria

  • A CI watch instance belongs to one issue and one Delivery. It owns the pending CI-fix Repair's head and failed checks, consumed Check re-run heads, and Abandoned job re-run heads. The same instance survives every Repair loop round, Base fix and refused Self-merge; each new Delivery, including a Continuation, starts fresh.
  • The Repair loop's CI interface consists of assessment and notification of a completed CI-fix Repair. Remove its allowance state, abandoned-check predicate and separate watch / Check re-run / Abandoned job re-run operations; the corresponding raw CI operations become private implementation details.
  • Record a CI-fix Repair only after it and its post-session pull request check succeed. Compare its saved full head with the head after the next successful Base branch merge. Intervening conflict Repairs must not discard that pending fact.
  • Preserve ADR 0010 and ADR 0020 exactly: separate once-per-full-head allowances, consumed before their requests; no Abandoned job re-run after a Check re-run is consumed; fresh assessments on revisited heads; own failures on a consumed Check re-run head become the same Declined CI fix. Changed heads and separately started Deliveries have independent allowances.
  • Preserve current Base commit comparison, own/Inherited failure classification, non-Actions checks, new-attempt protection, shared workflow handling, partial/refused requests, missing attempts, read errors, interruption and actionability checks. In particular, a refused or unseen Abandoned job re-run retains the original red assessment, while a refused Check re-run follows its existing fresh-observation rules.
  • Enter CI watch immediately before assessment, including an assessment that fails as a Declined CI fix. Keep Repair and upstream-move budgets, Base fix selection, push/merge order, captured pull request checks, progress text and Failed run reporting unchanged.
  • Move the per-head policy matrix to tests through the CI watch assessment interface using actual scripted check/job observations and the existing in-memory adapter. Include returning to an earlier consumed head after another head, identical short prefixes on different full heads, current Base comparison, separate Delivery reset, both allowances on one head, and red/green/Inherited-only outcomes after consumption. Replace overlapping tests instead of retaining duplicate policy matrices.
  • Keep Repair loop tests for assessment/Repair notification ordering, failed Repairs, conflict Repairs before the next assessment, stages, budgets, Base fix routing and refused Self-merge reconciliation. Retain representative CLI tests proving the real wiring and pull request gates, including old/missing attempts, refusal, mixed own/Inherited failures and Abandoned jobs avoiding a Repair. Update the direct cancellation test to use CI watch assessment.
  • Add a glossary definition for CI watch: observing CI on a full Issue branch head against its selected Base branch commit, including eligible Abandoned job re-runs and Check re-runs, with allowances remembered across one Delivery; CI watch reports CI outcomes or a Declined CI fix and starts neither Repairs nor Base fixes. Keep implementation details out of the definition. Update the domain-to-code navigation pointers when ownership moves.
  • Focused CI watch, Repair loop, GitHub cancellation and CLI CI tests pass; formatting and the repository's required Rust checks pass. No user-visible configuration, storage format or CI policy changes are introduced.

Blocked by

None (can start immediately).

Decisions

The design tree was settled in three rounds: constraints and ownership first; interface, state transitions and effects next; tests, terminology and publication size last. Each round depends on the preceding decisions.

  1. Preserve behavior and the existing ADRs. This is a depth improvement, because the observed friction is split knowledge of established CI policy, not missing product behavior. ADR 0008's deterministic Inherited failure handling and ADRs 0010/0020 remain authoritative; none needs reopening.
  2. Make CI watch the owning module. Put allowance selection, head history, job classification, requests and attempt reconciliation behind its interface, because changing either re-run policy currently requires understanding both CI watch and the Repair loop. Removing the resulting module would put that complexity back into its caller: it passes the deletion test.
  3. Use one issue-bound CI watch instance per Delivery. The production Repair loop adapter constructs and owns it once when taking the pull request to its goal, because that adapter already survives all rounds and failed-merge reconciliation. Do not persist it or put it in command-global state: a child Run, another issue and a new Continuation need fresh allowances.
  4. Expose construction, assessment and completed-CI-fix notification. Assessment takes the current full head, optional selected Base commit and a temporarily borrowed actionability check, and returns the existing CI outcome or error. Notification takes the repaired head and the failed checks handed to that Repair. The interface hides the three raw CI operations; callers retain only facts the CI module cannot observe itself.
  5. Pass dependencies only while assessing. Keep the existing private effects seam, production GitHub adapter and in-memory test adapter; they justify a real seam. Build the production effects adapter for each assessment rather than retaining a mutable pull request borrow across agent sessions. No new public effects interface is needed.
  6. Keep pending Repair facts until the next assessment. Notify only after the successful CI-fix Repair and its actionability postcheck; do not notify for conflict/review Repairs or failures. Consume the pending fact when assessment compares against the head after Base merging, because an immediate post-session comparison would miss the existing ordering rule.
  7. Retain both allowances and their exact transition rules. Move the existing policy intact, keyed by full head commit and consumed before I/O, because ADR 0020 deliberately allows an Abandoned job re-run before the later Check re-run. Visiting another head, a Base fix or a refused merge never renews an allowance. Continue using fresh observations and the current Base commit even for a consumed head.
  8. Keep Delivery orchestration outside CI watch. The Repair loop enters its CI watch stage immediately before assessment and still chooses Repairs/Base fixes, counts budgets, reconciles upstream moves and finishes Self-merge. This gives locality to CI policy without making CI watch depend on Delivery stages or agent sessions. Error causes and interruption propagation retain their current meanings.
  9. Test policy together with actual attempt observation. Exercise the assessment implementation through the existing in-memory effects adapter, because the old Repair loop adapter can promise a final CI outcome without proving that the new attempt was observed. Migrate overlapping re-run and policy tests to that interface; retain transport/format tests only where they prove distinct behavior.
  10. Keep orchestration and CLI evidence at their existing seams. Repair loop tests verify when assessment and successful-Repair notification happen, not the CI module's internal choice of raw operations. Representative CLI tests verify that one instance is wired through the real Delivery and that captured pull request guards, stage reporting and failures survive. This replaces shallow policy scripts while preserving independent wiring evidence.
  11. Document the existing concept as CI watch. The glossary already uses this phrase without a dedicated definition; make its responsibility precise in this Ticket's implementation, because the deepened module needs a stable domain name. This is a reversible ownership change with no new policy trade-off, so no new ADR is warranted.
  12. Publish one standalone Ticket with no blockers. All production raw CI operation callers are in the same Delivery adapter, and the work is one coherent refactor with its tests and terminology. It fits one fresh context window; splitting it would leave the shallow interface or duplicate policy in place between slices.

Architecture review

Strength: Strong · Dependencies: mock

Files: src/ci.rs:68, src/delivery/repair_loop.rs:236, src/delivery.rs:569, src/delivery.rs:717, src/github/execution_tests.rs:977, tests/ci.rs, GLOSSARY.md, docs/code-navigation.md

Before

flowchart LR
  subgraph Delivery[Delivery]
    R[Repair loop] --> P[Per-head re-run policy]
  end
  subgraph CI[CI watch]
    O[Observation and attempt reconciliation]
  end
  P -. three CI operations .-> O
  O --> A[GitHub adapter]
  classDef leak stroke:#dc2626,stroke-width:2px;
  class P,O leak
Loading

After

flowchart LR
  subgraph Delivery[Delivery]
    R[Repair loop]
  end
  subgraph CI[CI watch]
    P[Per-head re-run policy] --> O[Observation and attempt reconciliation]
  end
  R -->|assess and report completed Repair| P
  O --> A[GitHub adapter]
  classDef deep stroke-width:3px;
  class CI deep
Loading

Problem: CI watch has a shallow interface for Delivery: its caller selects three operations and carries allowance state that must agree with the implementation's attempt protections.

Solution: Deepen the module by moving per-head policy behind the CI watch seam, using the existing production and in-memory adapters.

Wins

  • Locality: CI policy changes together
  • Leverage: one assessment interface
  • Depth: attempts and allowances tested together
  • Delete shallow forwarding operations

Covered by: None after reading all 110 open issue titles and bodies; #839 and #927 change cancellation interpretation, and #907 changes Self-merge policy, rather than this deepening.

Selection reason: This recently changed policy has concrete cross-module friction, a settled behavior-preserving design, existing adapters and a direct testability gain.

Review validation: On main at 65e3dcf007662b06993a23b7529a1f713f087fcd, all 44 existing CI module tests, 58 Repair loop tests and the GitHub check-read cancellation test passed (103 total). Both diagrams passed Mermaid parsing. The review made no repository changes; these are baseline checks, not validation of an implementation.

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
    and removed
    needs-triageMaintainer needs to evaluate this issue
    on Oct 21, 2026
  2. added
    enhancementNew feature or request
    in-progressA Claim: thirdshift has taken this issue
    and removed
    ready-for-agentFully specified, ready for an AFK agent
    on Oct 21, 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 publishedenhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions