Keep starting byre when one session fails to resume
This commit is contained in:
@@ -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/<default> 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
|
||||
|
||||
+1
-1
@@ -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.
|
||||
|
||||
+32
-19
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user