Skip to content

Tower architect identity hardening: role_id || 'main' silent fallback, missing UNIQUE constraint, fresh-launch crash-loop fallback gap #1214

Description

@amrmelsayed

Problem

Hitting Ctrl-C twice in Claude Code terminates the CLI process (Claude Code's own behavior — two SIGINTs, it exits). The question is what Tower does in response, and the answer is the bug: Tower cannot tell a deliberate quit apart from a crash, and always tries to resume.

Confirmed directly in the shellper-backed exit handler (packages/codev/src/terminal/pty-session.ts:156-185):

client.on('exit', (exitInfo: { code: number; signal: string | null }) => {
  ...
  if (this._restartOnExit) {
    // unconditional — no branch on exitInfo.code or exitInfo.signal
    this.onPtyData('[Process exited — restarting...]');
    ... respawns ...

There is zero inspection of exitInfo.code/exitInfo.signal before deciding to restart. Whether Claude Code exits with code 0 (deliberate quit) or dies from an actual crash, Tower observes the identical event and reacts identically. restartOnExit is always true for architect sessions.

Worse: the restart is never a plain relaunch. It re-injects --resume <stored-session-id>, read from that architect's row in state.db (issue #832's design, meant to survive real crashes without losing conversation context). This resume-injection fires on every exit, deliberate or not — so whatever picks the wrong stored session isn't a rare edge case, it's exercised every time a user deliberately ends a session this way.

Reported symptom (Waleed, 2026-07-21, Tower UI)

Session restart when I do ctrl-c ctrl-c is picking random sessions to restart that do not match the architect I just restarted. I would rather that when I do ctrl-c ctrl-c it just restarts without any retry.

Structural causes found (grounded in source, not speculation)

  1. Identity-resolution fallback silently defaults to 'main'. dbSession.role_id || 'main' appears at packages/codev/src/agent-farm/servers/tower-terminals.ts:653, 909, 1030. The adjacent comment says a v13 migration "should have" backfilled every legacy row's role_id" — if any row's role_idis null when Tower rebuilds that session's restart options (at reconciliation — Tower startup, or a reconnect sweep), the rebuilt session resolves identity as **main** regardless of which architect it actually was, and bakes in main's stored--resume` id.
  2. role_id has no UNIQUE constraint anywhere in the schema. Explicitly called out in code comments: packages/codev/src/agent-farm/commands/workspace-recover.ts:121 ("has no UNIQUE constraint on role_id — the cautious read is 'alive'") and :343 ("role_id has no UNIQUE constraint in the schema, so collect every matching"). Duplicate/stale rows for the same architect name are structurally possible; any lookup expecting "the one row for this name" can land on the wrong one depending on insertion order — which looks like "random" to a user, since row order isn't observable.
  3. A dated, real incident matching this exact failure shape already occurred. The pir-1198 builder thread documents that on 2026-07-19, one session's reconnect glitch got misclassified as dead, and killOrphanedShellpers (packages/codev/src/terminal/session-manager.ts:599-643 — scoped to all shellpers on the Tower instance, not to one terminal) then killed two unrelated, healthy architect processes as collateral damage. That specific trigger was patched, but the mechanism — one terminal's problem causing an unrelated terminal's session to die or rebuild — is proven to fire in this exact subsystem within the last few days.

What's NOT yet confirmed

A clean, single-keystroke path where Ctrl-C-Ctrl-C in terminal A directly restarts terminal B in normal steady-state operation (no recent Tower restart/reconcile). The generic per-session restart handler (session-manager.ts:setupAutoRestart) is correctly scoped per in-memory session with no shared-state bug found there. The likely trigger for the specific cross-architect mismatch is a reconciliation event (Tower restart/redeploy) landing around the same time as the Ctrl-C-Ctrl-C, at which point causes 1 and 2 above kick in. Worth confirming with Waleed whether Tower was restarted/redeployed around when he observed the mismatch.

Why PIR, not BUGFIX

This touches core, shared Tower session-restart machinery (pty-session.ts, session-manager.ts, tower-terminals.ts, tower-instances.ts, the terminal_sessions/architect schema) with several real design decisions, not a mechanical fix:

  1. How should Codev distinguish "intentional quit" from "crash"? Recommended: fix the underlying cause (branch on exit code), not keystroke-listening. A precedent already exists in this exact file: session-manager.ts:905 gates the crash-loop counter with if (exit.code !== 0), and the adjacent comment states the semantic plainly — "Clean exits (code 0) never count: a user quitting a healthy session repeatedly must not trigger it." Codev already treats exit code 0 as "not a crash"; it just isn't wired to the restart-with-resume decision, only to the crash-loop-counting decision. Extending that same signal to gate resume-injection is the general, root-cause fix: it covers every voluntary-exit path uniformly (Ctrl-C-Ctrl-C, /exit, closing the terminal), not just the one specific keystroke sequence. Listening for the literal double-Ctrl-C pattern client-side would (a) miss every other intentional-quit mechanism, leaving them still wrongly auto-resumed, and (b) require adding keystroke-parsing to a layer (the shellper passthrough pipe) that is deliberately keystroke-agnostic today, to reconstruct information the exit code should already carry. Client-side keystroke detection should be treated as a fallback only, if empirical testing shows Claude Code's exit code does NOT reliably distinguish voluntary quit from every crash mode (segfault, OOM-kill, unhandled exception) — this needs verification (spawn a bare session, trigger each exit path, observe the actual code/signal) before committing to the exit-code branch as sufficient.
  2. Should role_id gain a real UNIQUE constraint (scoped by workspace_path + type), with a migration to reconcile any existing duplicate/legacy rows? Or is a code-level fix (never trust an ambiguous match, log loudly and fail closed instead of silently defaulting to 'main') sufficient?
  3. Should the 'main' fallback be removed entirely in favor of failing loudly (skip restart-with-resume, log an error) when an architect's role_id can't be resolved, rather than silently misattributing to main?
  4. Should "restart without resume" be the default behavior for ANY exit, a per-exit-reason branch, or a user-facing toggle? This is the crux of the design: Multi-architect conversation resume: disambiguate via per-architect session UUID #832 introduced resume-on-restart specifically to survive real crashes without losing context — an implementation must not regress that case while fixing the deliberate-quit case.

Origin

Reported by Waleed Kadous (2026-07-21) via architect relay, investigated by the main architect session same day. Related: issue #1198 (killOrphanedShellpers root-cause, same subsystem, same week), issue #832 (introduced resume-on-restart), issue #1149 (crash-loop fallback), Spec 755 (role_id / multi-architect identity), Spec 786 (architect identity preservation across restart).

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