Merge pull request #1693 from automagik-dev/fix/exterminate-name-flag

fix(agent): retire --name flag + distinguish auto_resume_disabled vs null_session

Author: namastex888

src/db/migrations/044_phase_b_flip_defaults.test.ts

@@ -42,7 +42,12 @@ describe.skipIf(!DB_AVAILABLE)('migration 044 — Phase B: flip auto_resume + re
     await cleanup();
   });
 
-  test('fresh DB: agents.auto_resume column default is false after migration 044', async () => {
+  test('fresh DB: agents.auto_resume column default is true after migration 055', async () => {
+    // Migration 055 (PR #1693) flipped the column default from `false` to `true`.
+    // Felipe directive 2026-05-07: default-off was a bug shape — 52/53 agents
+    // had auto_resume=false, meaning no agent could resume without first
+    // running `genie agent recover`. The migration runner applies every
+    // migration under the test schema, so we observe the post-055 state.
     const sql = await getConnection();
     const rows = await sql<{ column_default: string | null }[]>`
       SELECT column_default
@@ -53,10 +58,12 @@ describe.skipIf(!DB_AVAILABLE)('migration 044 — Phase B: flip auto_resume + re
     `;
     expect(rows.length).toBe(1);
     // Postgres reports boolean defaults as 'true' / 'false' literal strings.
-    expect(rows[0].column_default).toBe('false');
+    expect(rows[0].column_default).toBe('true');
   });
 
-  test('fresh-INSERT agent row inherits auto_resume=false', async () => {
+  test('fresh-INSERT agent row inherits auto_resume=true (post-055 default)', async () => {
+    // Migration 055 flipped the default to true. Fresh inserts that don't
+    // explicitly set auto_resume now inherit the safe value.
     const sql = await getConnection();
     const id = `test-fresh-${Date.now()}`;
     await sql`
@@ -66,7 +73,7 @@ describe.skipIf(!DB_AVAILABLE)('migration 044 — Phase B: flip auto_resume + re
     const rows = await sql<{ auto_resume: boolean | null }[]>`
       SELECT auto_resume FROM agents WHERE id = ${id}
     `;
-    expect(rows[0].auto_resume).toBe(false);
+    expect(rows[0].auto_resume).toBe(true);
   });
 
   test('live row (last_state_change < 1h, non-terminal state) preserves auto_resume=true', async () => {

src/db/migrations/055_default_auto_resume_true.sql

@@ -0,0 +1,39 @@
+-- 055_default_auto_resume_true.sql — Flip the agents.auto_resume default to
+-- TRUE and backfill every existing FALSE row to TRUE.
+--
+-- Why
+-- ---
+-- DB evidence (2026-05-07): 52 of 53 agents had auto_resume=false. The
+-- chokepoint `shouldResume()` treats false as "operator paused or scheduler
+-- exhausted retry budget" — but the column was effectively defaulting off,
+-- so no agent could be resumed without first running `genie agent recover`
+-- to flip the flag. Felipe directive (2026-05-07): default-on is the bug
+-- shape; flip every existing row to true and change the column DEFAULT so
+-- newly-spawned agents get the safe value out of the gate.
+--
+-- This migration also lands the trace-confirmed bug fix where
+-- `MissingResumeSessionError` reported reason='null_session' on rows whose
+-- session UUID was perfectly intact — auto_resume=false was the actual
+-- precondition. The protocol-router error enum now distinguishes the two
+-- (`auto_resume_disabled` vs `null_session`); this migration removes the
+-- silent default-off that triggered the misleading message in the first
+-- place.
+--
+-- Idempotent: re-running this migration is a no-op on rows that are already
+-- true and on a column whose default is already true.
+
+BEGIN;
+
+-- 1. Flip every existing false row. No WHERE narrowing — Felipe directive
+--    is to flip ALL of them so resume works out-of-the-box.
+UPDATE agents
+SET auto_resume = true
+WHERE auto_resume = false;
+
+-- 2. Change the column DEFAULT so future spawns inherit the safe value.
+--    NULLs (legacy rows that pre-date the column) become true — same as
+--    the safe-default the runtime treats them as.
+ALTER TABLE agents ALTER COLUMN auto_resume SET DEFAULT true;
+UPDATE agents SET auto_resume = true WHERE auto_resume IS NULL;
+
+COMMIT;

src/lib/agent-directory.ts

@@ -348,6 +348,27 @@ export async function resolve(name: string): Promise<ResolvedAgent | null> {
       if (templateTeam) entry.team = templateTeam;
       return { entry, builtin: false };
     }
+
+    // 1b. Fallback to custom_name when role didn't match. Spawned agents
+    // routinely have a custom_name (e.g. "felipe", "fix-resume-name-flag")
+    // that diverges from `role`; without this fallback, `genie agent
+    // recover felipe` failed at the resolver step even though the row was
+    // there (Felipe directive 2026-05-07).
+    const byCustomName = await sql`
+      SELECT role, custom_name, metadata, created_at FROM agents
+      WHERE custom_name = ${name}
+      ORDER BY (CASE WHEN position('dir:' in id) = 1 THEN 0 ELSE 1 END), started_at DESC
+      LIMIT 1
+    `;
+    if (byCustomName.length > 0) {
+      const row = byCustomName[0];
+      const meta = parseMetadata(row.metadata);
+      const createdAt =
+        row.created_at instanceof Date ? row.created_at.toISOString() : (row.created_at as string | undefined);
+      const entry = roleToEntry(typeof row.role === 'string' ? row.role : name, undefined, meta, createdAt);
+      if (templateTeam) entry.team = templateTeam;
+      return { entry, builtin: false };
+    }
   } catch {
     /* PG unavailable — fall through to built-ins */
   }

src/lib/executor-registry.ts

@@ -259,13 +259,19 @@ export async function updateClaudeSessionId(executorId: string, sessionId: strin
 
 /**
  * Identity passed to the JSONL fallback scanner. Mirrors the canonical
- * `(team, custom_name)` columns on `agents`. Both fields must be non-null
- * to attempt a match — the fallback refuses to return another agent's
- * transcript when ownership is unknown.
+ * `(team, custom_name)` columns on `agents`. `customName` is required (the
+ * scanner refuses to return another agent's transcript when role identity
+ * is unknown). `team` may be null for legacy / orphan rows where no team
+ * was ever recorded — in that case the scanner matches on `agentName` alone
+ * and additionally accepts JSONL bodies whose `teamName` is null.
  */
 export interface ResumeFallbackIdentity {
-  /** Agent's `team` column. Populated by `findOrCreateAgent`. */
-  team: string;
+  /**
+   * Agent's `team` column. Populated by `findOrCreateAgent`. May be null
+   * for legacy/orphan rows; when null, the scanner relaxes to a customName
+   * match only.
+   */
+  team: string | null;
   /** Agent's `custom_name` column. The role-or-name part of the identity. */
   customName: string;
 }
@@ -432,7 +438,12 @@ async function defaultScanForSession(cwd: string, identity: ResumeFallbackIdenti
 
   for (const candidate of sorted) {
     const { teamName, agentName } = await readJsonlIdentity(candidate.full);
-    if (teamName !== identity.team || agentName !== identity.customName) continue;
+    if (agentName !== identity.customName) continue;
+    // Identity.team is allowed to be null for orphan / legacy rows
+    // (Felipe directive 2026-05-07): in that case match on agentName alone.
+    // When identity.team is set, require strict teamName equality so we
+    // don't attach team A's runtime to team B's transcript.
+    if (identity.team !== null && teamName !== identity.team) continue;
     return candidate.name.replace(/\.jsonl$/, '');
   }
   return null;
@@ -467,21 +478,23 @@ type ResumeRow = {
 /**
  * Try the JSONL on-disk fallback for an agent whose DB resume read missed.
  * Returns the recovered session UUID if a matching JSONL is found, or null
- * if no cwd / no identity / no JSONL match. Emits `resume.recovered_via_jsonl`
- * on hit.
+ * if no cwd / no customName / no JSONL match. Emits
+ * `resume.recovered_via_jsonl` on hit.
  *
- * Identity is `(team, custom_name)` and BOTH must be present — a missing
- * identity makes ownership unverifiable, and we refuse to attach an agent's
- * runtime to another agent's transcript.
+ * Identity is `(team, custom_name)`. `custom_name` is required (the scanner
+ * refuses to attach without role identity), but `team` may be null —
+ * legacy/orphan rows that never got a team assigned still resume off-disk
+ * by matching on `agentName` alone (Felipe directive 2026-05-07: previous
+ * strict-both check stranded rows where team got dropped).
  */
 async function tryJsonlFallback(agentId: string, row: ResumeRow | null, actor: string): Promise<string | null> {
   const cwd = row?.repo_path ?? null;
   if (!cwd) return null;
 
   const team = row?.team ?? null;
   const customName = row?.custom_name ?? null;
-  const identity: ResumeFallbackIdentity | null = team && customName ? { team, customName } : null;
-  if (!identity) return null;
+  if (!customName) return null;
+  const identity: ResumeFallbackIdentity = { team, customName };
 
   const scanner = _resumeJsonlScannerDeps.scanForSession ?? defaultScanForSession;
   const recoveredSessionId = await scanner(cwd, identity);

src/lib/protocol-router.test.ts

@@ -526,6 +526,37 @@ describe.skipIf(!DB_AVAILABLE)('pg', () => {
       expect(legacyAlias.reason).toBe('no_session_id');
       expect(legacyAlias.message).toContain('no_session_id');
     });
+
+    // Felipe directive 2026-05-07: when an agent has auto_resume=false but
+    // its session UUID is intact, the error must NOT pretend the session is
+    // lost. Previously every "I can't resume" outcome collapsed onto
+    // null_session, and operators chased nonexistent missing-session bugs.
+    test('MissingResumeSessionError(reason=auto_resume_disabled) tells the operator the session is intact and to run recover', async () => {
+      const { MissingResumeSessionError } = await import('./protocol-router.js');
+
+      const paused = new MissingResumeSessionError('paused-agent', undefined, 'auto_resume_disabled');
+      expect(paused.reason).toBe('auto_resume_disabled');
+      expect(paused.entityId).toBe('paused-agent');
+      // Message MUST distinguish from the "session lost" case.
+      expect(paused.message).toContain('auto_resume is disabled');
+      expect(paused.message).toContain('session UUID is intact');
+      expect(paused.message).toContain('genie agent recover paused-agent');
+      // Must NOT mislead the operator into thinking the session is lost.
+      expect(paused.message).not.toContain('no claude_session_id recorded');
+      expect(paused.message).not.toContain('genie reset');
+    });
+
+    test('MissingResumeSessionError(reason=null_session) keeps the session-lost runbook hint', async () => {
+      const { MissingResumeSessionError } = await import('./protocol-router.js');
+
+      const lost = new MissingResumeSessionError('lost-agent', undefined, 'null_session');
+      expect(lost.reason).toBe('null_session');
+      // Genuine session-loss path keeps the legacy `genie reset` runbook hint.
+      expect(lost.message).toContain('no claude_session_id recorded');
+      expect(lost.message).toContain('genie reset lost-agent');
+      // And must NOT confuse itself with the auto_resume_disabled message.
+      expect(lost.message).not.toContain('auto_resume is disabled');
+    });
   });
 
   // ---------------------------------------------------------------------------

src/lib/protocol-router.ts

@@ -47,12 +47,28 @@ interface DeliveryResult {
  * of quietly proceeding with a fresh spawn.
  *
  * `reason` names which precondition failed:
- *   - `no_executor`   — agent has no `current_executor_id` (never spawned, or row pruned)
- *   - `null_session`  — executor row exists but `claude_session_id` is null
- *   - `no_session_id` — legacy alias kept for callers that read `agent.claudeSessionId`
- *                       directly before Group 1's single-reader helper lands.
+ *   - `no_executor`           — agent has no `current_executor_id` (never spawned, or row pruned)
+ *   - `null_session`          — executor row exists but `claude_session_id` is null (genuine session loss)
+ *   - `no_session_id`         — legacy alias kept for callers that read `agent.claudeSessionId`
+ *                                directly before Group 1's single-reader helper lands.
+ *   - `auto_resume_disabled`  — executor HAS a session_id, but `agents.auto_resume = false`
+ *                                (operator paused or scheduler exhausted retries). The session
+ *                                is intact — flip `auto_resume = true` via `genie agent recover`.
  */
-export type MissingResumeSessionReason = 'no_executor' | 'null_session' | 'no_session_id';
+export type MissingResumeSessionReason = 'no_executor' | 'null_session' | 'no_session_id' | 'auto_resume_disabled';
+
+function buildResumeErrorMessage(workerId: string, suffix: string, reason: MissingResumeSessionReason): string {
+  if (reason === 'auto_resume_disabled') {
+    return (
+      `Cannot resume worker "${workerId}"${suffix}: auto_resume is disabled, but the session UUID is intact ` +
+      `(reason: auto_resume_disabled). Run \`genie agent recover ${workerId} --yes\` to flip the flag and resume.`
+    );
+  }
+  return (
+    `Cannot resume worker "${workerId}"${suffix}: executor has no claude_session_id recorded (reason: ${reason}). ` +
+    `This usually means the worker predates the session-sync hook. Run \`genie reset ${workerId}\` or re-spawn the worker to recover.`
+  );
+}
 
 export class MissingResumeSessionError extends Error {
   readonly workerId: string;
@@ -62,9 +78,7 @@ export class MissingResumeSessionError extends Error {
 
   constructor(workerId: string, recipientId?: string, reason: MissingResumeSessionReason = 'null_session') {
     const suffix = recipientId ? ` (recipient "${recipientId}")` : '';
-    super(
-      `Cannot resume worker "${workerId}"${suffix}: executor has no claude_session_id recorded (reason: ${reason}). This usually means the worker predates the session-sync hook. Run \`genie reset ${workerId}\` or re-spawn the worker to recover.`,
-    );
+    super(buildResumeErrorMessage(workerId, suffix, reason));
     this.name = 'MissingResumeSessionError';
     this.workerId = workerId;
     this.entityId = workerId;
@@ -331,7 +345,21 @@ export async function resolveResumeSessionId(
   const agentIdToProbe = worker?.id ?? `dir:${recipientId}`;
   const decision = await shouldResume(agentIdToProbe);
   if (worker && (await isExecutorResumable(worker))) {
-    if (!decision.sessionId) throw new MissingResumeSessionError(worker.id, recipientId);
+    // CodeRabbit-flagged gap (PR #1693): the chokepoint may return
+    // `resume: false` with a sessionId still attached (e.g.
+    // `auto_resume_disabled` — operator explicitly paused). Without checking
+    // `decision.resume`, the spawn-side path would silently resume a paused
+    // worker. Propagate the chokepoint reason onto the error so operators
+    // see "auto_resume disabled — run recover" instead of "session lost".
+    if (!decision.resume || !decision.sessionId) {
+      const errReason: MissingResumeSessionReason =
+        decision.reason === 'unknown_agent'
+          ? 'no_executor'
+          : decision.reason === 'auto_resume_disabled'
+            ? 'auto_resume_disabled'
+            : 'null_session';
+      throw new MissingResumeSessionError(worker.id, recipientId, errReason);
+    }
   }
   return decision.sessionId;
 }

src/lib/provider-adapters.test.ts

@@ -141,6 +141,58 @@ describe('buildClaudeCommand', () => {
     expect(result.command).toContain("--model 'opus'");
   });
 
+  // Felipe directive 2026-05-07: --name was retired because CC stored it as
+  // `customTitle` in the JSONL header and then composed the resume hint as
+  // `claude --resume "<customTitle>"` instead of the canonical UUID, breaking
+  // session resume. Even when callers pass `name`, the launch command MUST
+  // NOT emit --name. Sessions are identified exclusively by --session-id /
+  // --resume <uuid>.
+  it('never emits --name even when params.name is set', () => {
+    const result = buildClaudeCommand({
+      provider: 'claude',
+      team: 'work',
+      role: 'implementor',
+      name: 'felipe',
+    });
+    expect(result.command).not.toContain('--name ');
+  });
+
+  it('never emits --name on a fresh-spawn command', () => {
+    const result = buildClaudeCommand({
+      provider: 'claude',
+      team: 'work',
+      role: 'implementor',
+    });
+    expect(result.command).not.toContain('--name ');
+  });
+
+  it('uses --session-id (not --name) for session identity on fresh spawn', () => {
+    const result = buildClaudeCommand({
+      provider: 'claude',
+      team: 'work',
+      role: 'implementor',
+      sessionId: '11111111-2222-3333-4444-555555555555',
+      name: 'should-be-ignored',
+    });
+    expect(result.command).toContain('--session-id');
+    expect(result.command).toContain("'11111111-2222-3333-4444-555555555555'");
+    expect(result.command).not.toContain('--name ');
+    expect(result.command).not.toContain("'should-be-ignored'");
+  });
+
+  it('uses --resume (not --name) for session identity on resume', () => {
+    const result = buildClaudeCommand({
+      provider: 'claude',
+      team: 'work',
+      role: 'implementor',
+      resume: 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee',
+      name: 'felipe',
+    });
+    expect(result.command).toContain('--resume');
+    expect(result.command).toContain("'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'");
+    expect(result.command).not.toContain('--name ');
+  });
+
   it('includes --append-system-prompt-file by default when systemPromptFile is set', () => {
     const result = buildClaudeCommand({
       provider: 'claude',

src/lib/provider-adapters.ts

@@ -105,7 +105,13 @@ export interface SpawnParams {
   model?: string;
   /** Initial prompt to send as the first user message (Claude Code positional [prompt] arg). */
   initialPrompt?: string;
-  /** Display name for the CC session (emits --name). Used in /resume and terminal title. */
+  /**
+   * @deprecated 2026-05-07 — `--name` was retired because CC stored it as
+   * `customTitle` in the JSONL header and then composed `claude --resume
+   * "<customTitle>"` instead of the canonical UUID, breaking session resume.
+   * The field remains in the schema for backward compatibility but is no
+   * longer emitted on the launch command. Always use `sessionId` / `resume`.
+   */
   name?: string;
   /** Claude Code permissions (allow/deny lists with Bash() patterns). Merged into --settings. */
   permissions?: { allow?: string[]; deny?: string[] };
@@ -389,7 +395,10 @@ function appendSessionFlags(parts: string[], params: SpawnParams): void {
   const claudeAgentFlag = params.agentTemplate ?? params.role;
   if (claudeAgentFlag) parts.push('--agent', escapeShellArg(claudeAgentFlag));
   if (params.model) parts.push('--model', escapeShellArg(params.model));
-  if (params.name) parts.push('--name', escapeShellArg(params.name));
+  // NOTE: --name intentionally NOT emitted. CC stores --name as `customTitle`
+  // in the JSONL header and then suggests `claude --resume "<customTitle>"`
+  // instead of the canonical UUID, clobbering session resume (Felipe directive
+  // 2026-05-07). The session UUID flows through --session-id / --resume above.
 }
 
 // Inject hook dispatch + permissions via --settings (deep-merges with existing settings).

src/lib/team-lead-command.test.ts

@@ -142,7 +142,9 @@ describe('buildTeamLeadCommand resume behavior', () => {
     const cmd = buildTeamLeadCommand('test-team', { promptMode: 'append' });
     expect(cmd).not.toContain('--resume');
     expect(cmd).not.toContain('--session-id');
-    expect(cmd).toContain("--name 'test-team'");
+    // --name retired (Felipe directive 2026-05-07): CC's customTitle
+    // clobbered the UUID resume hint. Always use --session-id / --resume.
+    expect(cmd).not.toContain('--name ');
   });
 
   test('sessionId without resume emits --session-id <uuid>', () => {

src/lib/team-lead-command.ts

@@ -67,8 +67,11 @@ export function buildTeamLeadCommand(teamName: string, options?: BuildTeamLeadCo
     '--permission-mode auto',
   ];
 
-  // Session name for CC's /resume and terminal title
-  parts.push(`--name ${shellQuote(sanitized)}`);
+  // NOTE: --name intentionally omitted. CC stores --name as `customTitle` in
+  // the JSONL header and then composes its resume hint as
+  // `claude --resume "<customTitle>"` instead of the canonical UUID, breaking
+  // session resumption (Felipe directive 2026-05-07). Always use --session-id
+  // / --resume <uuid>.
 
   if (options?.sessionId) {
     const flag = options.resume ? '--resume' : '--session-id';