Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions docs/plans/2026-09-27-001-feat-ci-repair-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ The outside reference is OpenAI's account of Codex babysitting a PR until CI is
- R3. A round sends the failing checks to the `run` phase as its input envelope, then runs every phase after `run` to the end of the chain, with every `if:` guard, gate and `on_fail` edge live. Phases before `run` do not re-run; their envelope fields carry over.
- R4. After the chain, acceptance and `publish.hold_when` are judged again over the merged field view. If acceptance fails or the hold now holds, nothing is pushed.
- R5. A fix is pushed to the same attempt-scoped branch without force, and the push is fenced and recorded as a numbered round on the control plane before and after the side effect.
- R6. The control plane enforces the round budget against the frozen snapshot, and refuses `accepted` unless the `ci` record was written after the last recorded round and names none of the heads any round repaired (each of those was judged red). It does not require the green head to equal the last round's `head_after`: a person may push a fix after the rounds run out, and the publish-only retry must still be able to accept it.
- R6. The control plane enforces the round budget against the frozen snapshot, refuses any round once `ci` is recorded green (so a `ci` record always postdates every round), and refuses `accepted` when the `ci` record names any head a round repaired (each of those was judged red). It does not require the green head to equal the last round's `head_after`: a person may push a fix after the rounds run out, and the publish-only retry must still be able to accept it.
- R7. Engine wall-clock and prompt sends are shared with the attempt: rounds draw from the same `AttemptCeiling` and `MaxAttemptSends`, so a repair cannot buy an attempt more budget than it was admitted with.
- R8. Cancellation stops a round at the next phase boundary. The engine part of a round is never inside the publish critical section, which runs under `context.WithoutCancel` (`internal/worker/publish.go:337`).
- R9. If the branch head on the remote is not the head jig last pushed, a person has taken over the branch: no round runs, and the result says so.
Expand All @@ -54,7 +54,7 @@ The outside reference is OpenAI's account of Codex babysitting a PR until CI is

- **KTD6 — Every failure degrades to today's red-CI outcome.** A round that fails acceptance, trips the hold, makes no change, exhausts the budget, breaches a boundary, loses its lease, or finds the head moved pushes nothing further and ends `accepted_unpublished` with the pull request at the last pushed head. The worst case of opting in is therefore today's behaviour plus fix commits that each passed the full downstream chain before they were pushed.

- **KTD7 — A person's push ends automation for that attempt.** Before a round, the worker compares the red head with the head it last pushed (the `push` record or the last round's `head_after`). If they differ, a person has pushed, and jig does not stack an agent's commits on top. The existing retry path already re-judges whatever a person pushed. The non-force push is the second line of defence if a person pushes mid-round.
- **KTD7 — A person's push ends automation for that attempt.** Before a round, the worker compares the red head with the head it last pushed (the `push` record or the last round's `head_after`). If they differ, a person has pushed, and jig does not stack an agent's commits on top. The existing retry path already re-judges whatever a person pushed. The control plane enforces the same rule on the ledger: a round's `head_before` must be the head jig last pushed (the proof ref for round 1, the previous round's `head_after` after that), so a worker that skipped the check is refused before it pushes. The non-force push is the third line if a person pushes mid-round.

---

Expand Down Expand Up @@ -111,8 +111,8 @@ Round state lives in the worker's `PublishingRunner.Run` loop. The engine owns c
### U2. Control plane: round ledger and the accepted rule

- **Files:** `internal/protocol/publish.go` (round record, authorization and record request types), `migrations/005_publish_ci_repairs.sql`, `internal/controlplane/publish_ledger.go`, `internal/controlplane/store.go`, `internal/controlplane/http.go`, `internal/worker/client.go`, tests beside each.
- **Approach:** authorize round N only when the lease is valid, the snapshot declares `on_fail`, N = last round + 1, N ≤ budget, and `proof` is recorded. Record with idempotent replay (same values return the stored row; different values conflict). Extend `CompleteAttempt` (`store.go:555`) so that when rounds exist, `accepted` needs a `ci` record completed after the last round whose `remote_ref` equals no round's `head_before` (R6).
- **Tests:** over-budget authorization refused; zombie lease refused; replay idempotence; `accepted` refused when `ci` predates the last round; migration from 004 keeps existing rows.
- **Approach:** authorize and record round N only when the lease is valid, the branch is the attempt's own, the frozen snapshot declares `on_fail`, `proof` is recorded, no `ci` record exists yet, N = last round + 1, N ≤ budget, and `head_before` is the head jig last pushed (KTD7). Record with idempotent replay (same values return the stored row; different values conflict). `head_after` must differ from `head_before`, and at most `MaxCIRepairFailedChecks` (20) check names of at most 200 bytes each are stored. `RecordPublishStep` refuses a `ci` record whose `remote_ref` equals any round's `head_before` (R6). That has to happen when the row is written, because a `ci` row is permanent and no round is allowed once one exists, so a bad row would leave the job unable ever to be accepted. `CompleteAttempt` (`store.go:555`) repeats the rule as a backstop. The snapshot reader `attemptWaitsForCI` becomes `attemptDefinition`, returning the parsed spec, since both rules read it.
- **Tests:** the chain from proof through two rounds; out-of-order, head-mismatch and over-budget rounds refused at both halves; foreign and expired leases refused; replay idempotence and conflicts; no rounds without `on_fail` or after green; malformed rounds refused; `accepted` refused on a repaired head but allowed on the pushed fix or a person's later fix; the client methods over the real HTTP handler.

### U3. Engine: continuation and `RepairCI`

Expand All @@ -129,7 +129,7 @@ Round state lives in the worker's `PublishingRunner.Run` loop. The engine owns c
### U5. Worker: the round loop

- **Files:** `internal/worker/publish.go`, new `internal/worker/publish_repair.go`, `internal/worker/publish_repair_test.go`
- **Approach:** in `PublishingRunner.Run`, when `publish` returns `ci_failed` and the continuation is present with budget left: the head-moved check (KTD7), authorize the round, `RepairCI` under the attempt's own cancellable context, then commit and non-force push in the critical section, record the round, and call `awaitCI` again as a fresh `ci` step. `PublishSummary` gains `CIRepairs []{Round, HeadBefore, HeadAfter, FailedChecks, Outcome}`. Events: `ci_repair_round_start`, `ci_repair_round_end`, `ci_repair_exhausted`.
- **Approach:** in `PublishingRunner.Run`, when `publish` returns `ci_failed` and the continuation is present with budget left: the head-moved check (KTD7), authorize the round, `RepairCI` under the attempt's own cancellable context, then commit and non-force push in the critical section, record the round, and call `awaitCI` again as a fresh `ci` step. `PublishSummary` gains `CIRepairs []{Round, HeadBefore, HeadAfter, FailedChecks, Outcome}`. Events: `ci_repair_round_start`, `ci_repair_round_end`, `ci_repair_exhausted`. Two traps from the U2 review: the head jig last pushed comes from `AttemptCIRepairs` (the last round's `head_after`, else the proof ref), because `RetryPublish` returns only `publish_records`; and an authorization's `Completed` is returned without comparing `head_before`, so the worker must check that the recorded round repaired the head it is holding before treating the round as done.
- **Tests (fake gateway plus scripted runtime):** red → fix → green ends `accepted` with one round recorded; red twice at budget 1 ends `accepted_unpublished` naming both heads; a person's push before round 1 means no round; non-force push rejected mid-round ends cleanly; hold tripped in a round pushes nothing; cancel mid-round leaves the pushed head intact.

### U6. Docs and the stock factory
Expand Down
Loading
Loading