Skip to content

shellper/launch loops: auto-restart should only trigger on unnatural exits — a deliberate quit should end cleanly #1241

Description

@waleedkadous

Problem

When the user deliberately exits the agent in a terminal — double Ctrl+C, /quit, or any clean exit — the restart machinery treats it like a crash: the launch-script loop prints Agent exited. Restarting in 2 seconds... and respawns, and the shellper's auto-restart does the same at its layer. The user made a choice; overriding it forces them to race another Ctrl+C into the 2-second window (and a mistimed one lands in the fresh agent instead). It also feeds the #1224 class: a killed claude respawned within ~2s can collide with its own dying predecessor's session lock.

Design (prescribed — keep BUGFIX-sized)

Exit code 0 = natural exit = the user's choice. Never restart on it.

Two surfaces enforce restart today; both should adopt the same rule:

  1. Builder launch scripts (the while true loops in spawn-worktree.ts-generated .builder-start.sh, incl. the Kimi provider-owned variants): on child exit 0, break the loop, clear the screen, print a one-liner ("Agent exited at your request. Press Enter to relaunch, or close this terminal.") — a keypress-gated relaunch keeps recovery one keystroke away without auto-respawning.
  2. Shellper auto-restart (session-manager.ts setupAutoRestart): exit code 0 → do not count it as a failure and do not respawn; leave the PTY open with the clean-exit message.

Nonzero exits and signal deaths keep today's restart behavior (that's what auto-recovery is for). No config knob needed.

Acceptance

Activity

  1. waleedkadous commented on Jul 25, 2026

    @waleedkadous
    ContributorAuthor

    On it! Working on a fix now.

  2. added 4 commits that reference this issue on Jul 25, 2026
  3. waleedkadous commented on Jul 25, 2026

    @waleedkadous
    ContributorAuthor

    Completion statistics

    Protocol: BUGFIX (investigate → fix → pr) · Builder: bugfix-1241 · PR: #1244, merged 2026-07-25 12:36Z as c3c3089

    Metric Value
    Files changed 11 (4 source, 5 test, 1 new test file, 1 thread)
    Diff +456 / −34
    Source LOC ~96 (rest is tests)
    Commits 8 (2 substantive + porch state)
    Phases 3, zero iterations — no phase re-run
    CMAP gemini=APPROVE, codex=APPROVE, claude=APPROVE (all HIGH, zero key issues)
    Gate pr, approved once, no rework
    Test suite 3655 passed / 0 failed
    CI 6/6 required checks green on first run
    Escalations / blockers None

    What changed

    Layer File Behavior on a deliberate quit
    Builder launch loops (5 sites) spawn-worktree.ts Clear screen, hint, relaunch gated on Enter
    Shellper auto-restart session-manager.ts Not counted, not respawned; session ends
    PTY notice + teardown timer pty-session.ts Honest notice, ends cleanly (no 10s false wait)
    Shared predicate shellper-protocol.ts isDeliberateExit()

    The non-obvious part

    code === 0 is not a sufficient test for "the user quit". node-pty reports a signal death as exit code 0 with the signal attached — probed directly during the investigation:

    normal exit  -> {"exitCode":0,"signal":0}
    SIGKILL      -> {"exitCode":0,"signal":9}
    exit 3       -> {"exitCode":3,"signal":0}
    

    A naive code-only check would have stopped auto-restarting killed agents — the exact inverse of the requirement. The predicate is therefore code 0 and no signal (null / '' / '0', since ShellperProcess stringifies the field).

    Two surfaces were named in the issue; a third had to move with them. pty-session.ts printed [Process exited — restarting...] and armed a 10s wait-for-respawn timer whenever restartOnExit was set — once the shellper layer stopped restarting clean exits, that notice was a lie and the timer tore the session down 10s later anyway.

    Deviation from the prescribed design

    The issue specified "leave the PTY open" at the shellper layer. For the architect that wedges relaunch: Tower gates architect creation on !entry.architects.has('main'), so a registered-but-dead terminal is unrelaunchable without a full workspace stop/start (which takes every builder with it). The architect terminal ends cleanly instead, clearing the architect row so afx workspace start brings it back; the shellper husk is not killed. Builders get the literal keypress-gated behavior, since their bash loop never exits on a clean quit.

    Scope notes

    Verification gap

    A live architect quit against a running Tower was not exercised — it requires a Tower restart, which would have killed the active builder sessions. The shellper layer is covered by a real-shellper integration test (/bin/sh -c 'exit 0' under an actual shellper process); the builder layer is covered end-to-end by tests that execute the generated bash script rather than pattern-matching it.

  4. added 4 commits that reference this issue on Jul 27, 2026
  5. added a commit that references this issue on Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/towerArea: Tower server / agent farm CLI

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions