From 0e52253d072e265e9c279a857b7dd7bba1022664 Mon Sep 17 00:00:00 2001 From: Ian Gulliver Date: Fri, 16 Oct 2026 12:43:54 -0700 Subject: [PATCH] Keep starting byre when one session fails to resume --- TODO.md | 2 -- docs/operations.md | 2 +- internal/agents/manager.go | 51 ++++++++++++++++++++++++-------------- 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/TODO.md b/TODO.md index 216bebf..2f9d6dc 100644 --- a/TODO.md +++ b/TODO.md @@ -6,8 +6,6 @@ `internal/agents/mergein.go:303–324` runs push → notes → fast-forward → `RemoveTask` → `MergeTask`. A crash or error after the push leaves an active task with no worktree; a retried Merge fails in `Snapshot`, or squashes to no change and closes with an empty `merge_commit`. Have the `finalizing` marker record the squash commit; on startup, a recorded commit that is an ancestor of origin/ finishes notes, fast-forward, removal and MergeTask idempotently, otherwise the marker is cleared and an error is filed. - A failed auto-commit still advances the workflow from the old HEAD A `CommitAll` error is only logged (`internal/agents/manager.go:838–841`) and `FinishStage` still runs (`:885–889`), so `RecordStage` stores the old `git.Head` (`internal/agents/workflows.go:349–356`), the next stage branches without the turn's work, and `discardStages` later deletes the worktree that held it. On commit failure, treat the turn as unfinished: skip FinishStage, the handoff and the summary, and file the error in the task's inbox. -- One bad session at startup stops byre from starting - `Recover` returns on a `taskFor` or `message` error (`internal/agents/manager.go:300–303`, `:338–341`) and `server.New` treats it as fatal (`internal/server/server.go:72–76`), so one unreadable worktree keeps the service down. Handle a failed `message` like a failed `ensureLive` (logUndelivered, DropStageOutput, EndTurn with the error, discard and merge follow-up) and continue; a `taskFor` error skips only that session. - Codex mid-turn delivery deletes input before the steer is accepted Queued inputs are deleted or marked delivered and recorded as `user.message` (`internal/agents/queue.go:119`, `:158`; `internal/agents/inbox.go:404`) before `Steer`, whose error is only logged (`internal/agents/codex.go:167–174`). Return the consumed ids from `deliverMidTurn`; when Steer fails, re-insert the inputs in order and clear `delivered_at` so the turn-end Deliver sends them. - Inbox answer validation isn't atomic with resolving the question diff --git a/docs/operations.md b/docs/operations.md index 6172b9a..1c9ea5e 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -46,7 +46,7 @@ Agents find closed tasks with the `search_closed_tasks` MCP tool, which task age - **Request.** `RequestDiscard` records the requester in `discarding_by`, sets `held`, clears any held merge and emits `task.discard_started` (a transcript note). The request returns the still-active task with 202. It is refused (409) while the task is merging or its discard is `held` or `running`; on a `closing` task it retries the cleanup. Merge and holding a merge are refused while `discard_state` is set. - **Admission.** While `discard_state` is set, no ordinary turn starts in the task: the store's idle-to-running transition (`transitionSession`) refuses it, so messages, inbox answers, Sync, Compact, queued messages and workflow stage starts all get a conflict and stay queued or held. Queued messages and answers aren't handed over mid-turn either (`deliverMidTurn`). The one way in is `StartDiscardTurn`, which needs `held` and an idle implementer session, and in one transaction sets the session `running`, the state `running` and `discard_session`. byre calls it (`startDiscard` in `internal/agents/summary.go`) right after the request, and again whenever a turn of the task ends with the state `held`; whichever call comes first wins, and a busy session simply leaves the state `held` until its turn ends. It uses the task's live implementer session, or a new one created by the task's creator when there is none, and sends `discardPrompt` (actor `byre`): the task is being discarded, change no files, call `set_task_summary` saying also why it is being dropped, then end the turn. Open inbox questions don't hold it back. If starting the turn fails, byre gives up on the summary: `CloseDiscard` sets `closing` and the task is removed. -- **Turn end.** Every implementer turn end, in `turnEnded`, on an agent process that exits mid-turn (`pump`), and on a session that can't be resumed at startup, marks the session idle through `EndTurn`, which in the same transaction reports what the discard needs: `closing` when the session is `discard_session` and the state was `running` (it sets `closing`), `held` when a discard is waiting. A turn of the task that ends while a discard is pending takes no snapshot. On `held`, the ending turn's handoff continuation and inbox delivery are skipped and the summary turn starts instead, in the same session, so a turn that was writing a handoff hands the summary turn its full context. On `closing`, `finishDiscard` stops the session, cancels reviews, deletes the snapshot refs, removes the worktree and branch (`git.RemoveWorktree`, which skips whatever is already gone so the cleanup can be repeated), and only then closes the task with `discarding_by` as the actor. The summary turn closes the task however it ends (finished, interrupted, an error), so interrupting it discards without a summary; a summary turn whose process dies discards without a summary too, while an ordinary turn whose process dies while a discard is `held` gets the summary turn started on a new process. A failed cleanup is filed in the task's inbox and leaves the state `closing`. +- **Turn end.** Every implementer turn end, in `turnEnded`, on an agent process that exits mid-turn (`pump`), and on a session that can't be resumed or sent its restart notice at startup (`failResume`, after which startup carries on with the other sessions; a session whose task can't be loaded is logged as `resume session: load task` and left `running`), marks the session idle through `EndTurn`, which in the same transaction reports what the discard needs: `closing` when the session is `discard_session` and the state was `running` (it sets `closing`), `held` when a discard is waiting. A turn of the task that ends while a discard is pending takes no snapshot. On `held`, the ending turn's handoff continuation and inbox delivery are skipped and the summary turn starts instead, in the same session, so a turn that was writing a handoff hands the summary turn its full context. On `closing`, `finishDiscard` stops the session, cancels reviews, deletes the snapshot refs, removes the worktree and branch (`git.RemoveWorktree`, which skips whatever is already gone so the cleanup can be repeated), and only then closes the task with `discarding_by` as the actor. The summary turn closes the task however it ends (finished, interrupted, an error), so interrupting it discards without a summary; a summary turn whose process dies discards without a summary too, while an ordinary turn whose process dies while a discard is `held` gets the summary turn started on a new process. A failed cleanup is filed in the task's inbox and leaves the state `closing`. - **Workflows.** `FinishStage` refuses to mark a stage finished while `discard_state` is set, so a stage whose turn ends after the discard was requested drops its output and doesn't advance. A stage that finished before the request advances as usual (`advanceStage` runs before the summary turn starts, since it reads the worktree's HEAD), and a `closing` task's stage never advances (`nextStage` clears it). An existing stage task being discarded isn't restarted by `advanceTo`. - **Restarts.** On startup, a `running` summary turn's session is resumed with `discardRestartPrompt` (the discard prompt again, after a line saying byre restarted) instead of the usual restart notice; `recoverDiscards` then starts the summary turn for each `held` task whose session isn't running, closes each `running` one whose session isn't running, and finishes each `closing` one. - **Handoffs.** A handoff continuation (`handoff` in `internal/agents/compact.go`) doesn't start while a discard is pending; it starts the summary turn in the old session instead. It retires the old session only through an idle-to-completed transition, so it can't retire a session the summary turn has just taken, and if a discard arrives between retiring and starting the continuation, the summary turn runs in the new session, without the handoff. diff --git a/internal/agents/manager.go b/internal/agents/manager.go index e005620..ebb2591 100644 --- a/internal/agents/manager.go +++ b/internal/agents/manager.go @@ -299,28 +299,15 @@ func (m *Manager) Recover(ctx context.Context) error { } task, err := m.taskFor(ctx, sess) if err != nil { - return err + slog.Error("resume session: load task", "session", sess.Id, "task", sess.TaskId, "err", err) + continue } l, err := m.ensureLive(ctx, sess, task) if err != nil { - slog.Error("resume session", "session", sess.Id, "err", err) - m.logUndelivered(ctx, sess.Id, "resume failed") - derr := m.store.DropStageOutput(ctx, sess.Id) - if derr != nil { - return derr - } - next, serr := m.store.EndTurn(ctx, sess.Id, "byre", map[string]any{"error": err.Error()}, nil, false) - if serr != nil { - return serr - } - switch { - case next == store.DiscardClosing: - go m.finishDiscard(task.Id) - case next == store.DiscardHeld: - go m.startDiscard(task.Id) - case sess.Role == api.Implementer: - m.abandonMerge(task.Id, task.ProjectId) + ferr := m.failResume(ctx, sess, task, err) + if ferr != nil { + return ferr } continue } @@ -337,7 +324,11 @@ func (m *Manager) Recover(ctx context.Context) error { } err = m.message(ctx, l, "byre", notice, nil) if err != nil { - return err + ferr := m.failResume(ctx, sess, task, err) + if ferr != nil { + return ferr + } + continue } slog.Info("resumed session", "session", sess.Id, "task", task.Id, "owner", sess.Owner) m.logUndelivered(ctx, sess.Id, "resumed with restart notice") @@ -367,6 +358,28 @@ func (m *Manager) Recover(ctx context.Context) error { return nil } +func (m *Manager) failResume(ctx context.Context, sess api.AgentSession, task api.Task, err error) error { + slog.Error("resume session", "session", sess.Id, "err", err) + m.logUndelivered(ctx, sess.Id, "resume failed") + derr := m.store.DropStageOutput(ctx, sess.Id) + if derr != nil { + return derr + } + next, serr := m.store.EndTurn(ctx, sess.Id, "byre", map[string]any{"error": err.Error()}, nil, false) + if serr != nil { + return serr + } + switch { + case next == store.DiscardClosing: + go m.finishDiscard(task.Id) + case next == store.DiscardHeld: + go m.startDiscard(task.Id) + case sess.Role == api.Implementer: + m.abandonMerge(task.Id, task.ProjectId) + } + return nil +} + func (m *Manager) logUndelivered(ctx context.Context, sessionID, reason string) { pending, err := m.store.Undelivered(ctx, sessionID) if err != nil {