[Bugfix #1241] Auto-restart only on unnatural exits — a deliberate quit ends cleanly

codev/projects/bugfix-1241-shellper-launch-loops-auto-res/status.yaml

@@ -0,0 +1,17 @@
+id: bugfix-1241
+title: shellper-launch-loops-auto-res
+protocol: bugfix
+phase: pr
+plan_phases: []
+current_plan_phase: null
+gates:
+  pr:
+    status: approved
+    requested_at: '2026-07-25T09:59:32.448Z'
+    approved_at: '2026-07-25T12:32:57.528Z'
+iteration: 1
+build_complete: false
+history: []
+started_at: '2026-07-25T09:40:46.551Z'
+updated_at: '2026-07-25T12:32:57.529Z'
+pr_ready_for_human: false

codev/state/bugfix-1241_thread.md

@@ -0,0 +1,65 @@
+# bugfix-1241 — auto-restart should only trigger on unnatural exits
+
+## Investigate
+
+**Repro / mechanism** (read from code, plus a node-pty probe):
+
+- Builder terminals: `.builder-start.sh` is a `while true; do <agent>; echo "Agent exited.
+  Restarting in 2 seconds..."; sleep 2; done` loop, generated in 5 places in
+  `packages/codev/src/agent-farm/commands/spawn-worktree.ts`
+  (resume / role / no-role in `startBuilderSession`, role / no-role in
+  `buildWorktreeLaunchScript`). The loop is exit-code blind — a clean `/quit`
+  (exit 0) respawns exactly like a crash.
+- Architect terminals: launched directly by Tower (`tower-instances.ts`, no bash
+  loop) with `restartOnExit: true`, so the respawn comes from
+  `session-manager.ts` `setupAutoRestart`, which is also exit-code blind: it
+  increments `restartCount` and re-SPAWNs on *any* exit.
+- Builder sessions do NOT set `restartOnExit`, so the two layers are cleanly
+  split: layer 1 = builders, layer 2 = architects.
+
+**Key finding — the naive `code === 0` test is wrong.** node-pty reports signal
+deaths as `exitCode 0` plus a signal (probed here: SIGKILL →
+`{exitCode: 0, signal: 9}`; normal exit → `{exitCode: 0, signal: 0}`), and
+`shellper-process.ts` stringifies that field. So "deliberate quit" must be
+`code === 0 && signal in (null, '', '0')`, otherwise a SIGKILLed agent would
+stop restarting — the opposite of what the issue asks.
+
+**Note on the issue text**: it mentions "the Kimi provider-owned variants" of
+the launch script. There is no Kimi provider in this repo (`grep -ri kimi` is
+empty); all launch-loop generation lives in the 5 sites above, and they are all
+covered.
+
+**Third surface found (not named in the issue)**: `pty-session.ts`
+`attachShellper`'s exit handler prints `[Process exited — restarting...]` and
+arms a 10s "wait for the restart" timer whenever `restartOnExit` is set. With
+the layer-2 fix in place that restart never comes, so it must also branch on a
+deliberate exit — otherwise a clean architect quit shows a false "restarting"
+notice and then tears down 10 seconds later.
+
+Scope: 3 source files + tests, well under 300 LOC. BUGFIX-appropriate.
+
+## Fix
+
+- `shellper-protocol.ts`: `isDeliberateExit()` — the one predicate both layers use.
+- `session-manager.ts`: deliberate exit → log, emit `session-clean-exit`, drop
+  the dead session, do not count it, do not respawn.
+- `pty-session.ts`: deliberate exit → print the clean-exit line and end cleanly
+  (no false "restarting" notice, no 10s timer).
+- `spawn-worktree.ts`: all 5 loops share one `LAUNCH_LOOP_TAIL` — exit 0 clears
+  the screen and gates the relaunch on Enter; EOF on stdin exits instead of
+  spinning; nonzero/signal keeps the 2s auto-restart.
+
+Deviation from the issue's "leave the PTY open" for the architect: the session
+is dropped from SessionManager and PtySession emits `exit`, because Tower's
+`workspace start` is gated on `!entry.architects.has('main')` — keeping the
+registered-but-dead terminal would make the architect unrelaunchable without a
+full workspace stop/start. Ending cleanly clears the architect row, so
+`afx workspace start` brings it back. The shellper husk itself is not killed.
+
+## PR
+
+PR #1244 opened. CMAP 3-way: gemini=APPROVE, codex=APPROVE, claude=APPROVE — all
+HIGH confidence, zero key issues, nothing to address. Results posted as a PR
+comment. Architect notified. Waiting at the `pr` gate.
+
+Full suite green throughout: 3655 passed / 0 failures; build green.

packages/codev/src/agent-farm/__tests__/launch-loop-exit-code.test.ts

@@ -0,0 +1,95 @@
+/**
+ * Bugfix #1241 — the builder launch loop must only auto-restart on unnatural
+ * exits. A deliberate quit (exit 0: double Ctrl+C, `/quit`) ends the loop and
+ * waits for a keypress instead of respawning.
+ *
+ * These tests EXECUTE the generated script with bash rather than pattern-match
+ * it: the regression is a shell control-flow bug, and only running it proves
+ * the agent was launched once instead of in a loop.
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { spawnSync } from 'node:child_process';
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+import { buildWorktreeLaunchScript } from '../commands/spawn-worktree.js';
+
+let worktree: string;
+let counter: string;
+
+/** Build the real launch script with a fake agent that exits with `code`. */
+function writeLaunchScript(code: number): string {
+  // The fake agent records each launch, so the test can count respawns.
+  const baseCmd = `sh -c "echo run >> '${counter}'; exit ${code}"`;
+  const script = buildWorktreeLaunchScript(worktree, baseCmd, null, worktree);
+  const scriptPath = path.join(worktree, 'launch.sh');
+  fs.writeFileSync(scriptPath, script);
+  fs.chmodSync(scriptPath, 0o755);
+  return scriptPath;
+}
+
+function runCount(): number {
+  if (!fs.existsSync(counter)) return 0;
+  return fs.readFileSync(counter, 'utf-8').trim().split('\n').filter(Boolean).length;
+}
+
+beforeEach(() => {
+  worktree = fs.mkdtempSync(path.join(os.tmpdir(), 'codev-1241-'));
+  counter = path.join(worktree, 'runs.txt');
+});
+
+afterEach(() => {
+  fs.rmSync(worktree, { recursive: true, force: true });
+});
+
+describe('builder launch loop exit handling (Bugfix #1241)', () => {
+  it('does not respawn the agent after a deliberate exit (code 0)', () => {
+    const scriptPath = writeLaunchScript(0);
+
+    // stdin is closed, so the relaunch prompt reads EOF and the script ends.
+    // Without the fix this loops forever and the timeout below kills it.
+    const result = spawnSync('/bin/bash', [scriptPath], {
+      stdio: ['ignore', 'pipe', 'pipe'],
+      encoding: 'utf-8',
+      timeout: 10_000,
+    });
+
+    expect(result.signal).toBeNull(); // exited on its own, was not timed out
+    expect(result.status).toBe(0);
+    expect(runCount()).toBe(1);
+    expect(result.stdout).toContain('Agent exited at your request');
+    expect(result.stdout).not.toContain('Restarting in 2 seconds');
+  });
+
+  it('relaunches once per keypress after a deliberate exit', () => {
+    const scriptPath = writeLaunchScript(0);
+
+    // One Enter → one relaunch; then EOF ends the loop.
+    const result = spawnSync('/bin/bash', [scriptPath], {
+      input: '\n',
+      encoding: 'utf-8',
+      timeout: 10_000,
+    });
+
+    expect(result.status).toBe(0);
+    expect(runCount()).toBe(2);
+  });
+
+  it('still auto-restarts after a crash (nonzero exit)', () => {
+    const scriptPath = writeLaunchScript(7);
+
+    // No natural end for a crash loop — kill it after two restart delays.
+    const result = spawnSync('/bin/bash', [scriptPath], {
+      stdio: ['ignore', 'pipe', 'pipe'],
+      encoding: 'utf-8',
+      timeout: 5_000,
+      killSignal: 'SIGKILL',
+    });
+
+    expect(runCount()).toBeGreaterThan(1);
+    expect(result.stdout).toContain('Restarting in 2 seconds');
+    expect(result.stdout).toContain('code 7');
+    expect(result.stdout).not.toContain('Agent exited at your request');
+  }, 15_000);
+});

packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts

@@ -386,6 +386,29 @@ describe('spawn-worktree', () => {
       expect(script).not.toContain('--resume');
       expect(script).toContain('--append-system-prompt');
     });
+
+    // Bugfix #1241: every generated variant must gate the relaunch on exit
+    // code, not respawn blindly — this is the assertion that catches a new
+    // launch-script code path being added without the deliberate-quit branch.
+    it.each([
+      ['resume', { sessionId: 'abc', scriptFragment: "--resume 'abc'" }, 'ROLE'],
+      ['role', undefined, 'ROLE'],
+      ['no role', undefined, null],
+    ] as const)('%s → script does not auto-restart on exit 0', async (name, resume, role) => {
+      await startBuilderSession(
+        { workspaceRoot: '/tmp/ws' } as any,
+        `b-${name}`, '/tmp/worktree', 'claude',
+        'PROMPT', role, role ? 'codev' : null, resume,
+      );
+
+      const script = findScript()!;
+      expect(script).toContain('status=$?');
+      expect(script).toContain('if [ "$status" -eq 0 ]; then');
+      expect(script).toContain('Press Enter to relaunch');
+      expect(script).toContain('read -r || exit 0');
+      // The crash path is untouched.
+      expect(script).toContain('Restarting in 2 seconds');
+    });
   });
 
   // =========================================================================

packages/codev/src/agent-farm/commands/spawn-worktree.ts

@@ -732,6 +732,34 @@ function installHarnessWorktreeFiles(
   }
 }
 
+/**
+ * The tail shared by every builder launch loop, appended after the agent
+ * invocation inside `while true; do … done`.
+ *
+ * Issue #1241: exit code 0 is the user deliberately quitting (double Ctrl+C,
+ * `/quit`) — auto-respawning overrides that choice and forces them to race a
+ * second Ctrl+C into the sleep window, where a mistimed one lands in the fresh
+ * agent instead. It also feeds the #1224 class, where a respawn within ~2s
+ * collides with the dying predecessor's session lock. So a clean exit clears
+ * the screen and gates the relaunch on a keypress: recovery stays one keystroke
+ * away without anything happening on its own. Nonzero exits and signal deaths
+ * (bash reports those as 128+N) keep the historical auto-restart — that is what
+ * the loop is for.
+ *
+ * `read` failing means EOF on stdin, i.e. the terminal is gone; exit rather
+ * than spin the loop on an input that will never arrive.
+ */
+const LAUNCH_LOOP_TAIL = `  status=$?
+  if [ "$status" -eq 0 ]; then
+    clear
+    echo "Agent exited at your request. Press Enter to relaunch, or close this terminal."
+    read -r || exit 0
+    continue
+  fi
+  echo ""
+  echo "Agent exited (code $status). Restarting in 2 seconds... (Ctrl+C to quit)"
+  sleep 2`;
+
 /**
  * Start a terminal session for a builder.
  *
@@ -767,9 +795,7 @@ export async function startBuilderSession(
 cd "${worktreePath}"
 while true; do
   ${baseCmd} ${resume.scriptFragment}
-  echo ""
-  echo "Agent exited. Restarting in 2 seconds... (Ctrl+C to quit)"
-  sleep 2
+${LAUNCH_LOOP_TAIL}
 done
 `;
   } else if (roleContent) {
@@ -801,9 +827,7 @@ done
 cd "${worktreePath}"
 ${envBlock}while true; do
   ${baseCmd} ${fragment} "$(cat '${promptFile}')"
-  echo ""
-  echo "Agent exited. Restarting in 2 seconds... (Ctrl+C to quit)"
-  sleep 2
+${LAUNCH_LOOP_TAIL}
 done
 `;
   } else {
@@ -819,9 +843,7 @@ done
 cd "${worktreePath}"
 while true; do
   ${baseCmd} "$(cat '${promptFile}')"
-  echo ""
-  echo "Agent exited. Restarting in 2 seconds... (Ctrl+C to quit)"
-  sleep 2
+${LAUNCH_LOOP_TAIL}
 done
 `;
   }
@@ -894,9 +916,7 @@ export function buildWorktreeLaunchScript(
 cd "${worktreePath}"
 ${envBlock}while true; do
   ${baseCmd} ${fragment}
-  echo ""
-  echo "Agent exited. Restarting in 2 seconds... (Ctrl+C to quit)"
-  sleep 2
+${LAUNCH_LOOP_TAIL}
 done
 `;
   }
@@ -907,9 +927,7 @@ done
 cd "${worktreePath}"
 while true; do
   ${baseCmd}
-  echo ""
-  echo "Agent exited. Restarting in 2 seconds... (Ctrl+C to quit)"
-  sleep 2
+${LAUNCH_LOOP_TAIL}
 done
 `;
 }