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
4 changes: 4 additions & 0 deletions cmd/jig/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,10 @@ func engineExecutor(
Cancelled: signalCtx.Done(),
FreshenLease: execution.FreshenLease,
})
// A direct run never publishes, so it never repairs CI either.
if outcome.Continuation != nil {
outcome.Continuation.Release()
}
return controlplane.DirectOutcome{
State: outcome.State,
Result: outcome.Result,
Expand Down
29 changes: 24 additions & 5 deletions cmd/jig/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,23 +272,37 @@ func workerAttemptRunner(
return worker.Outcome{State: protocol.AttemptFailed, Error: err.Error()}
}
// Belt to the engine's braces: the seeded credentials in the
// ephemeral HOME die with the attempt on every exit path (KTD11).
defer func() {
// ephemeral HOME die with the attempt on every exit path (KTD11) —
// unless a CI repair continuation still needs the scratch, in which
// case they die when it is released.
kept := false
removeScratch := func() {
if err := os.RemoveAll(filepath.Join(scratchRoot, attemptID)); err != nil {
logger.Warn("attempt_scratch_destroy_failed", "attempt_id", attemptID, "error", err)
}
}
defer func() {
if !kept {
removeScratch()
}
}()
trace, err := host().OpenTrace(prepared)
if err != nil {
return infraFail(err)
}
// The drain runs on a detached context: an interrupted attempt still
// has events worth delivering, and a dead context would drop the tail
// of exactly the run an operator most wants to read.
defer func() {
// of exactly the run an operator most wants to read. A continuation's
// repair rounds emit into the same stream, so it stays open for them.
closeTrace := func() {
if err := trace.Close(context.WithoutCancel(ctx)); err != nil {
logger.Warn("attempt_trace_close_failed", "attempt_id", attemptID, "error", err)
}
}
defer func() {
if !kept {
closeTrace()
}
}()

runner, err := engine.New(engine.Config{
Expand All @@ -308,14 +322,19 @@ func workerAttemptRunner(
// (which rides the heartbeat response) and the operator's Ctrl-C. The
// engine watches one channel, so they are merged rather than the
// second one being dropped.
return runner.Execute(context.WithoutCancel(ctx), engine.Attempt{
outcome := runner.Execute(context.WithoutCancel(ctx), engine.Attempt{
Claim: prepared.Claim,
WorktreePath: prepared.WorktreePath,
Branch: prepared.Branch,
BaseSHA: prepared.BaseSHA,
Cancelled: anyClosed(signalCtx.Done(), prepared.Cancelled()),
FreshenLease: prepared.FreshenLease,
})
kept = worker.DeferToContinuation(&outcome, func() {
closeTrace()
removeScratch()
})
return outcome
})
}

Expand Down
7 changes: 4 additions & 3 deletions docs/plans/2026-09-27-001-feat-ci-repair-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ The outside reference is OpenAI's account of Codex babysitting a PR until CI is

- **KTD1 — A dedicated `publish.ci.on_fail` block, not a new phase-edge `then:` value.** Publish is not a phase, and `RepairEdge.Then` accepts only `rerun-self` (`internal/protocol/definition.go:388`). A CI failure has no envelope to predicate on and no phase to re-run itself. Reusing the struct would force invalid combinations through validation, so this adds a small `CIRepairSpec{Run, Budget}` beside `CISpec`. It follows the edge vocabulary (`run`, `budget`) where it means the same thing. An earlier draft had a `resume_from` field; review of the U1 PR showed it let `run: build, resume_from: review` skip a `test` between them, carrying `test`'s verdict on the old code into acceptance of the new. The only safe value was the phase right after `run`, so the field was removed rather than constrained.

- **KTD2 — Rounds re-enter the engine through an in-process continuation, handed across the package seam as an interface.** The engine destroys its scratch directory and ephemeral HOME when `Execute` returns (`internal/engine/phase.go:185`), and `summaryJSON` caps each envelope (`maxPhaseEnvelopeBytes`), so the stored result is not a faithful field view to resume from. `engine` imports `worker`, never the reverse, and the boundary check enforces that. So `worker.Outcome` gains `Continuation worker.Continuation`, an interface with `RepairCI(ctx, CIFailure) Outcome` and `Release()`. The engine implements it by keeping its merged field view, `reportedBy`, results, gate reports, touched paths, send count and deadline. Each round builds a fresh scratch and HOME (KTD11 holds per round) and fresh agent sessions. Because the continuation exists only in the process that ran the chain, **the publish-only retry cannot repair (R11)**. That is deliberate: persisting full envelopes to resume across processes is a larger change with its own confidentiality questions, and retry already accepts a person's fix.
- **KTD2 — Rounds re-enter the engine through an in-process continuation, handed across the package seam as an interface.** The engine destroys its scratch directory and ephemeral HOME when `Execute` returns (`internal/engine/phase.go:185`), and `summaryJSON` caps each envelope (`maxPhaseEnvelopeBytes`), so the stored result is not a faithful field view to resume from. `engine` imports `worker`, never the reverse, and the boundary check enforces that. So `worker.Outcome` gains `Continuation worker.Continuation`, an interface with `RepairCI(ctx, CIFailure) Outcome` and `Release()`. The engine implements it by keeping its merged field view, `reportedBy`, results, gate reports, touched paths, send count and deadline. The ephemeral HOME is wiped when the chain ends and again when each round ends, and every role is re-seeded into it on first use in a round, so KTD11 holds per round. The handoff directory is kept, because it is agent notes outside the worktree that later phases read, not credentials. Each round's agent sessions use their own key namespace (`<attempt>-ci<N>-<role>`), since the old sessions' store lived in the wiped HOME. The engine returns a continuation only for accepted, unheld work under a definition that declares `on_fail`; otherwise the scratch dies with `Execute` as before. Whoever holds an outcome must `Release` its continuation (idempotent): the worker's attempt loop and the direct `jig run` path release unconditionally, and U5's publishing runner uses it first. Because the continuation exists only in the process that ran the chain, **the publish-only retry cannot repair (R11)**. That is deliberate: persisting full envelopes to resume across processes is a larger change with its own confidentiality questions, and retry already accepts a person's fix.

- **KTD3 — A round is split: engine (cancellable) → push (critical section) → CI wait (cancellable).** Today the whole publish runs under `WithoutCancel` because each step is a short `gh`/`git` command. A round can run agent phases for tens of minutes, so only the fenced push and record sit in the critical section. The engine part honours `lease.cancelled` like any attempt, and the CI wait already does (`publish.go:767`).

Expand Down Expand Up @@ -116,8 +116,8 @@ Round state lives in the worker's `PublishingRunner.Run` loop. The engine owns c

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

- **Files:** `internal/engine/phase.go`, new `internal/engine/resume.go`, `internal/engine/resume_test.go`, `internal/worker/registration.go` (`Outcome` and `AttemptRunner` live there, lines 88 and 101; `Continuation` joins them).
- **Approach:** on the accepted path only, `Execute` returns a continuation holding the execution state named in KTD2 minus scratch and sessions. `RepairCI` creates a fresh scratch, runs `run` with the synthesized envelope, runs every phase after `run` through the existing `runPhaseWithEdge`, then re-runs acceptance and `publishHold`. It returns an `Outcome` whose result lists only the round's touched paths, with `ci_repair_no_change` when there are none. The deadline and send counter carry over (R7). `Release` drops the state; `PublishingRunner` always calls it. Two semantics are fixed here, matching existing repair dispatch: the `run` phase runs regardless of its own `if:` guard, because guards are judged only in `runChain` (`phase.go:513`) and repair targets go through `runPhaseOnce`; and each round's resumed segment starts with fresh per-edge budgets (`edgeUses` is local to a `runChain` call), while the attempt-wide send count and ceiling stay shared.
- **Files:** `internal/engine/phase.go` (`run` split into `run` and `conclude`; `runChain` takes a start index; session keys take a prefix), new `internal/engine/resume.go`, `internal/engine/resume_test.go`, `internal/worker/registration.go` (`Continuation`, `CIFailure`, `Outcome.Continuation`), `internal/worker/claiming.go` and `cmd/jig/run.go` (release).
- **Approach:** on the accepted path only, `Execute` returns a continuation holding the execution state named in KTD2 minus scratch and sessions. `RepairCI` reuses the kept scratch with a wiped HOME (KTD2), runs `run` with the synthesized envelope, runs every phase after `run` through the existing `runPhaseWithEdge`, then re-runs acceptance and `publishHold`. It returns an `Outcome` whose result lists only the round's touched paths. An accepted round that touched nothing carries `ci_repair_no_change` in `Error` and spends the continuation, because pushing the same head cannot turn CI green. A HOME that cannot be wiped is never handed to a round: after the chain there is then no continuation, and after a round the continuation is spent. `Execute` keeps a panic-safe `defer` that destroys the scratch unless a continuation took ownership. Hosts that own per-attempt resources a round still needs (the `jig worker` scratch removal and trace stream in `cmd/jig/worker.go`) hand their cleanup to the continuation with `worker.DeferToContinuation`, which runs it once, after the engine's own `Release`. The deadline and send counter carry over (R7). `Release` drops the state; `PublishingRunner` always calls it. Two semantics are fixed here, matching existing repair dispatch: the `run` phase runs regardless of its own `if:` guard, because guards are judged only in `runChain` (`phase.go:513`) and repair targets go through `runPhaseOnce`; and each round's resumed segment starts with fresh per-edge budgets (`edgeUses` is local to a `runChain` call), while the attempt-wide send count and ceiling stay shared.
- **Tests (scripted runtime):** a round re-runs exactly `run` and the phases after it, and none before; a reviewer rejection inside a round loops the builder under its own edge; a classifier field now `high` trips the hold and yields no push; the send budget and ceiling are shared across rounds; cancellation between phases ends the round.

### U4. Gateway: failing-check detail
Expand All @@ -130,6 +130,7 @@ Round state lives in the worker's `PublishingRunner.Run` loop. The engine owns c

- **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`. 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.
- **Required end-to-end test:** one round must run through the real `jig worker` wiring (`cmd/jig/worker_test.go`'s scripted-runtime harness), asserting that the round's trace events reach the control plane and that the handoff notes were present during the round. U3 could only unit-test `DeferToContinuation`; this is the first place the host wiring in `cmd/jig/worker.go` can be proven.
- **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
48 changes: 37 additions & 11 deletions internal/engine/phase.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,16 @@ func (r *Runner) Execute(ctx context.Context, attempt Attempt) worker.Outcome {
return worker.Outcome{State: protocol.AttemptFailed, Error: err.Error()}
}
// The whole scratch family — ephemeral HOME included — dies with the
// attempt (KTD11). Anything an agent wrote "to its home" goes with it.
// attempt on every exit path, panics included (KTD11), unless a CI
// repair continuation takes ownership of it below.
kept := false
defer func() {
if err := scratch.destroy(); err != nil {
r.config.Logger.Warn("attempt_scratch_destroy_failed", "error", err)
if !kept {
if err := scratch.destroy(); err != nil {
r.config.Logger.Warn("attempt_scratch_destroy_failed", "error", err)
}
}
}()

e := &execution{
runner: r,
attempt: attempt,
Expand All @@ -206,8 +209,17 @@ func (r *Runner) Execute(ctx context.Context, attempt Attempt) worker.Outcome {
seededRoles: make(map[string]bool),
phaseEntries: make(map[string]int),
touchedPaths: make(map[string]bool),
sessionKey: attempt.Claim.Attempt.ID,
}
outcome := e.run(ctx)
// The chain stays alive for CI repair only when its HOME is provably
// empty: nothing an agent left "at home" (credentials included) may
// outlive the chain (KTD11). Only the handoff notes carry into a round.
if e.repairable(outcome) && e.wipeHome() == nil {
kept = true
outcome.Continuation = &ciContinuation{e: e}
}
return e.run(ctx)
return outcome
}

// exchange is one prompt/response pair kept for transcript digests (R7).
Expand Down Expand Up @@ -244,6 +256,11 @@ type execution struct {
seededRoles map[string]bool
phaseEntries map[string]int
touchedPaths map[string]bool
// sessionKey prefixes every runtime session key. It is the attempt id
// for the chain and gains a round suffix in each CI repair round, whose
// sessions start in a fresh HOME and must never collide with a key the
// runtime already saw.
sessionKey string
// sends counts every prompt send this attempt has issued, across phases,
// parse corrections, gate corrections, crash re-entries, and repair
// dispatches alike — the one number the whole ladder is bounded by.
Expand Down Expand Up @@ -280,7 +297,13 @@ func (e *execution) run(ctx context.Context) worker.Outcome {
"can_resume": e.capability.CanResume,
})

end := e.runChain(ctx)
return e.conclude(e.runChain(ctx, 0, nil))
}

// conclude maps how the chain ended onto the attempt's outcome: the terminal
// causes first, then acceptance, then the publish hold. A CI repair round
// ends through the same door, so a round is judged exactly as the chain was.
func (e *execution) conclude(end chainEnd) worker.Outcome {
switch end.kind {
case endCancelled:
e.emit.emit(protocol.EventLog, "", "attempt_cancelled", nil)
Expand Down Expand Up @@ -499,10 +522,13 @@ func (e *execution) freshenLease(ctx context.Context) {

// ---- the chain -------------------------------------------------------------

func (e *execution) runChain(ctx context.Context) chainEnd {
var previous *parsedEnvelope
// runChain runs the chain from phases[start] to the end, handing previous
// to the first phase run. A fresh edgeUses per call means each CI repair
// round gets fresh per-edge budgets; the attempt-wide sends and ceiling are
// what bound the rounds together.
func (e *execution) runChain(ctx context.Context, start int, previous *parsedEnvelope) chainEnd {
edgeUses := make(map[string]int)
for _, phase := range e.spec.Phases {
for _, phase := range e.spec.Phases[start:] {
if e.cancelled() {
return chainEnd{endCancelled, "cancelled between phases"}
}
Expand Down Expand Up @@ -1347,15 +1373,15 @@ func (e *execution) sessionFor(role string) *runtime.Session {
if session := e.sessions[role]; session != nil {
return session
}
session := &runtime.Session{Key: e.attempt.Claim.Attempt.ID + "-" + role}
session := &runtime.Session{Key: e.sessionKey + "-" + role}
e.sessions[role] = session
return session
}

func (e *execution) freshSession(role string) *runtime.Session {
e.sessionSeq++
session := &runtime.Session{
Key: fmt.Sprintf("%s-%s-r%d", e.attempt.Claim.Attempt.ID, role, e.sessionSeq),
Key: fmt.Sprintf("%s-%s-r%d", e.sessionKey, role, e.sessionSeq),
}
e.sessions[role] = session
return session
Expand Down
Loading
Loading