fix(board): surface agent startup failures instead of silently doing nothing#3508
Conversation
Failed relaunches set StatusError instead of being silently dropped, a crash loop is capped at maxLaunchFailures to preserve error output, relaunch recreates a missing worktree from the repo, and errored cards are attachable so the user can read the agent's last output. Assisted-By: Claude
Worktree-recreation fallback now only triggers on fs.ErrNotExist with a non-empty path (transient stat errors no longer reroute the launch). Crash-loop counter moved to per-card controller state so a prompt-bearing relaunch resets the cap. Capped cards poll at 5s instead of 500ms. Duplicated relaunch-failure handling extracted into resume(). AttachCommand returns a clear error when an errored card's session is gone entirely (new sessionManager.Exists via tmux has-session). Docs and tests updated. Assisted-By: Claude
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
This PR cleanly addresses a real UX gap — startup-crashing agents leaving cards stuck in "starting" forever. The overall design (crash-loop cap, dead-pane preservation, worktree fallback, errored-card attach) is sound and well-tested. Two low-severity observations on newly added code are left as inline comments.
| } | ||
| if !a.controller.Ready(card) { | ||
| if card.Status == StatusError { | ||
| if exists, err := a.sessions.Exists(card.Session); err == nil && !exists { |
There was a problem hiding this comment.
[LOW] Exists() error silently ignored for StatusError cards in AttachCommand
When card.Status == StatusError, the check is:
if exists, err := a.sessions.Exists(card.Session); err == nil && !exists {
return nil, errors.New("the agent's session is gone; ...")
}If Exists() returns (false, someErr) — for example, an unexpected tmux error string not covered by isNoSuchSession — the condition err == nil && !exists is false, so the code falls through and attempts to attach anyway. The user then receives tmux's raw error instead of the friendly message the PR intends to provide.
isNoSuchSession already handles the common "no server running" case (returning (false, nil)), so this only bites on unanticipated tmux error output. Consider returning the error explicitly when Exists() fails, e.g.:
exists, err := a.sessions.Exists(card.Session)
if err != nil || !exists {
return nil, errors.New("the agent's session is gone; move the card forward to relaunch it, or delete it")
}| delay := retryDelay | ||
| if alive, aerr := c.sessions.Alive(card.Session); aerr == nil && !alive { | ||
| _ = c.relaunch(card, "") | ||
| if c.launchFailed(cardID) >= maxLaunchFailures { |
There was a problem hiding this comment.
[LOW] Crash-loop counter grows unboundedly while card remains in StatusError
Once launchFailed(cardID) >= maxLaunchFailures and the card transitions to StatusError, the watcher continues to loop every cappedRetryDelay (5 s). On each iteration the pane is still dead (alive == false), so launchFailed() is called again — incrementing the counter to 4, 5, 6, … indefinitely.
The practical impact is minimal (int won't overflow in any realistic timeframe, and setStatus(StatusError) is idempotent). However, after the user triggers a prompt-bearing relaunch, resetLaunchFailures correctly clears the counter, so recovery still works. The simplest fix is to skip the launchFailed() increment when the card is already in StatusError:
if card.Status != StatusError {
if c.launchFailed(cardID) >= maxLaunchFailures {
c.setStatus(cardID, StatusError)
delay = cappedRetryDelay
} else {
c.resume(card)
}
} else {
delay = cappedRetryDelay
}Or alternatively read the card's status after the store fetch (it's already available) and bail out of the Alive/launchFailed branch early.
On the Kanban board (
docker agent board), an agent that crashed at startup — bad ref, missing API key,--listenbind failure — left its card stuck in "starting" forever. Relaunch errors were silently discarded, the controller relaunched the crashing agent every 500ms (destroying the dead pane's error output each time), a first-launch failure before the worktree existed caused every subsequent relaunch to fail on a nonexistent working directory, and the user could not even attach to investigate because attach is gated on the control plane answering.The fix caps consecutive startup deaths at three before giving up: the card transitions to an error state, the dead tmux pane is preserved so its output can be read, and polling slows to 5 s instead of 500 ms.
watch()now surfaces relaunch failures asStatusErrorrather than dropping them.relaunch()falls back to launching from the repo root with--worktree/--worktree-basewhen the card's worktree was never created.AttachCommandallows attaching to an errored card even when the control plane is down, and returns a clear error when the session is gone entirely (via a newsessionManager.Existscheck). The crash-loop counter is per-card controller state; a user-initiated relaunch (one that carries a prompt) resets it so a red card gets fresh attempts when moved forward.No breaking changes. The new
sessionManager.Existsmethod wrapstmux has-sessionand is additive.