Skip to content

fix(board): surface agent startup failures instead of silently doing nothing#3508

Merged
dgageot merged 2 commits into
docker:mainfrom
dgageot:fix/board-surface-startup-failures
Jul 7, 2026
Merged

fix(board): surface agent startup failures instead of silently doing nothing#3508
dgageot merged 2 commits into
docker:mainfrom
dgageot:fix/board-surface-startup-failures

Conversation

@dgageot

@dgageot dgageot commented Jul 7, 2026

Copy link
Copy Markdown
Member

On the Kanban board (docker agent board), an agent that crashed at startup — bad ref, missing API key, --listen bind 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 as StatusError rather than dropping them. relaunch() falls back to launching from the repo root with --worktree/--worktree-base when the card's worktree was never created. AttachCommand allows 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 new sessionManager.Exists check). 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.Exists method wraps tmux has-session and is additive.

dgageot added 2 commits July 7, 2026 13:55
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
@dgageot
dgageot requested a review from a team as a code owner July 7, 2026 12:09
@dgageot
dgageot merged commit 7292003 into docker:main Jul 7, 2026
14 of 15 checks passed

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread pkg/board/app.go
}
if !a.controller.Ready(card) {
if card.Status == StatusError {
if exists, err := a.sessions.Exists(card.Session); err == nil && !exists {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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")
}

Comment thread pkg/board/controller.go
delay := retryDelay
if alive, aerr := c.sessions.Alive(card.Session); aerr == nil && !alive {
_ = c.relaunch(card, "")
if c.launchFailed(cardID) >= maxLaunchFailures {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants