Skip to content

Commit a57b5ec

Browse files
namastex888claude
andcommitted
feat(spawn): collapse to single UUID row — wish G3
Spawn callers now resolve the durable identity UUID via findOrCreateAgent before writing runtime fields through register(). The bare-name shadow path (`register({ id: workerId })`) is gone; register() asserts UUID-or- `dir:` ids at the top so any rogue caller fails loudly with a useful message instead of an opaque CHECK violation from migration 061. Touched call sites: - agents.ts:registerSpawnWorker — drops `?? ctx.workerId` fallback; agentIdentityId is now required. - agents.ts launchTmuxSpawn / rollbackSpawn / launchSdkSpawn / launchInlineSpawn — registry.update / unregister target the UUID id, not the bare workerId. - protocol-router-spawn.ts:spawnWorkerFromTemplate — resolves findOrCreateAgent(agentName, team, role) before register(); workerId lands on custom_name. - session.ts:registerSessionInRegistry — same reshape; bare-name lives on custom_name. New test (spawn-single-row.test.ts) covers: 1. register() rejects bare-name ids with the new error message. 2. register() accepts dir: master-row ids. 3. End-to-end spawn lands ONE UUID-keyed agents row (no shadow twin). 4. Repeated register() with the same identity is idempotent. Wish: retire-session-names-id-only Group 3. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 65ded73 commit a57b5ec

5 files changed

Lines changed: 200 additions & 27 deletions

File tree

src/genie-commands/session.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,20 @@ async function registerSessionInRegistry(sessionName: string, windowName: string
126126
const sanitized = sanitizeTeamName(windowName);
127127
const leaderName = await resolveSessionLeaderName(windowName);
128128
const sanitizedLeader = sanitizeTeamName(leaderName);
129+
130+
// Wish retire-session-names-id-only Group 3: spawn writes ONE row.
131+
// Resolve the durable identity UUID before registering runtime fields so
132+
// the row's id matches `agents_id_shape_check` (migration 061). The
133+
// bare-name display label (`<team>-<leader>`) lands on `custom_name`.
134+
const agentIdentity = await registry.findOrCreateAgent(leaderName, sanitized, leaderName);
135+
129136
await registry.register({
130-
id: `${sanitized}-${sanitizedLeader}`,
137+
id: agentIdentity.id,
131138
paneId,
132139
session: sessionName,
133140
team: windowName,
134141
role: leaderName,
142+
customName: `${sanitized}-${sanitizedLeader}`,
135143
worktree: null,
136144
startedAt: now,
137145
state: 'working',
@@ -143,9 +151,6 @@ async function registerSessionInRegistry(sessionName: string, windowName: string
143151
nativeAgentId: `${sanitizedLeader}@${sanitized}`,
144152
});
145153

146-
// Executor model: create agent identity + executor for leader session
147-
const agentIdentity = await registry.findOrCreateAgent(leaderName, sanitized, leaderName);
148-
149154
let pid: number | null = null;
150155
try {
151156
const pidStr = (await tmux.executeTmux(`display -t ${shellQuote(target)} -p '#{pane_pid}'`)).trim();
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
/**
2+
* Wish retire-session-names-id-only Group 3 — Spawn writes ONE row.
3+
*
4+
* Asserts the post-G3 invariants:
5+
* 1. `register()` rejects bare-name ids loudly (UUID OR `dir:<name>` only).
6+
* 2. The legitimate spawn path (`findOrCreateAgent` → `register`) lands a
7+
* single UUID-keyed agents row — no bare-name shadow twin.
8+
* 3. `register()` accepts `dir:<name>` master-row ids.
9+
*
10+
* The bare-name shadow rejection is mirrored in migration 061's
11+
* `agents_id_shape_check`; this test covers the application-level guard so
12+
* the failure message is "loud throw at the call site" instead of a deep
13+
* SQL CHECK violation.
14+
*/
15+
16+
import { afterAll, afterEach, beforeAll, describe, expect, test } from 'bun:test';
17+
import { findOrCreateAgent, list, register, unregister } from '../agent-registry.js';
18+
import { getConnection } from '../db.js';
19+
import { DB_AVAILABLE, setupTestDatabase } from '../test-db.js';
20+
21+
describe.skipIf(!DB_AVAILABLE)('spawn-single-row — wish retire-session-names-id-only G3', () => {
22+
let cleanup: () => Promise<void>;
23+
24+
beforeAll(async () => {
25+
cleanup = await setupTestDatabase();
26+
});
27+
28+
afterAll(async () => {
29+
await cleanup();
30+
});
31+
32+
afterEach(async () => {
33+
const sql = await getConnection();
34+
await sql`DELETE FROM executors`;
35+
await sql`DELETE FROM agents`;
36+
});
37+
38+
function makeRuntimeAgent(id: string, customName: string, team: string) {
39+
return {
40+
id,
41+
paneId: '%17',
42+
session: team,
43+
worktree: null,
44+
customName,
45+
role: customName,
46+
team,
47+
startedAt: new Date().toISOString(),
48+
state: 'spawning' as const,
49+
lastStateChange: new Date().toISOString(),
50+
repoPath: '/tmp/test',
51+
provider: 'claude' as const,
52+
transport: 'tmux' as const,
53+
};
54+
}
55+
56+
test('register rejects bare-name id with a useful error', async () => {
57+
const bareName = makeRuntimeAgent('engineer-4d48', 'engineer-4d48', 'genie');
58+
let caught: Error | null = null;
59+
try {
60+
await register(bareName);
61+
} catch (err) {
62+
caught = err as Error;
63+
}
64+
expect(caught).not.toBeNull();
65+
expect(caught!.message).toContain('non-UUID/non-dir agent id');
66+
expect(caught!.message).toContain('findOrCreateAgent');
67+
});
68+
69+
test('register accepts dir: master-row id', async () => {
70+
const master = makeRuntimeAgent('dir:engineer', 'engineer', 'genie');
71+
await register(master);
72+
const sql = await getConnection();
73+
const rows = await sql<{ id: string }[]>`SELECT id FROM agents WHERE id = 'dir:engineer'`;
74+
expect(rows.length).toBe(1);
75+
await unregister('dir:engineer');
76+
});
77+
78+
test('full spawn path lands ONE UUID-keyed row (no bare-name shadow)', async () => {
79+
// Step 1 (mirroring agents.ts:resolveSpawnIdentity → findOrCreateAgent):
80+
// resolve the durable identity row keyed by (custom_name, team).
81+
const identity = await findOrCreateAgent('engineer', 'genie', 'engineer');
82+
expect(identity.id).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i);
83+
84+
// Step 2 (mirroring agents.ts:registerSpawnWorker): register runtime fields
85+
// under the SAME UUID. workerId carries the human-readable label only.
86+
const workerId = 'engineer-4d48';
87+
const runtime = makeRuntimeAgent(identity.id, workerId, 'genie');
88+
await register(runtime);
89+
90+
// Assertion 1: exactly ONE row exists for this (custom_name, team).
91+
const all = await list();
92+
const matching = all.filter((a) => a.team === 'genie' && (a.customName === 'engineer' || a.id === identity.id));
93+
expect(matching.length).toBe(1);
94+
95+
// Assertion 2: the row is UUID-keyed (no bare-name shadow twin).
96+
const sql = await getConnection();
97+
const shadowRows = await sql<{ id: string }[]>`
98+
SELECT id FROM agents
99+
WHERE id NOT LIKE 'dir:%'
100+
AND id !~ '^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$'
101+
`;
102+
expect(shadowRows.length).toBe(0);
103+
104+
// Assertion 3: runtime fields landed on the identity row (single-source).
105+
const refreshed = matching[0];
106+
expect(refreshed.paneId).toBe('%17');
107+
expect(refreshed.state).toBe('spawning');
108+
expect(refreshed.id).toBe(identity.id);
109+
});
110+
111+
test('repeated register() against same identity is idempotent (no shadow twin)', async () => {
112+
const identity = await findOrCreateAgent('reviewer', 'genie', 'reviewer');
113+
const runtime1 = makeRuntimeAgent(identity.id, 'reviewer-aaaa', 'genie');
114+
const runtime2 = makeRuntimeAgent(identity.id, 'reviewer-bbbb', 'genie');
115+
await register(runtime1);
116+
await register(runtime2); // ON CONFLICT (id) DO UPDATE merges runtime fields
117+
118+
const all = await list();
119+
const matching = all.filter((a) => a.team === 'genie' && a.customName === 'reviewer');
120+
// Single row — register's ON CONFLICT (id) DO UPDATE is the upsert.
121+
// custom_name stays 'reviewer' from findOrCreateAgent (COALESCE preserves
122+
// the existing non-null value).
123+
expect(matching.length).toBe(1);
124+
expect(matching[0].id).toBe(identity.id);
125+
});
126+
});

src/lib/agent-registry.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,30 @@ function buildLegacyTeamLeadEntryId(teamName: string): string {
230230
return `team-lead:${teamName}`;
231231
}
232232

233+
/**
234+
* Identity-shape gates enforced by migration 061's `agents_id_shape_check` —
235+
* `agents.id` must be a UUID or the `dir:<name>` master-row prefix. Bare-name
236+
* inserts get rejected at the database level and crash the spawn pipeline.
237+
*
238+
* This module-level constant is the application-side mirror so we can fail
239+
* fast (with a useful error) BEFORE the SQL roundtrip. Wish
240+
* retire-session-names-id-only Group 3 — the spawn primitive writes ONE row,
241+
* keyed by the UUID identity from `findOrCreateAgent`. Bare-name shadow rows
242+
* are gone.
243+
*/
244+
const REGISTER_ID_RE = /^([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}|dir:[A-Za-z0-9_-]+)$/i;
245+
246+
function assertRegisterableId(id: string): void {
247+
if (!REGISTER_ID_RE.test(id)) {
248+
throw new Error(
249+
`register: refusing to insert non-UUID/non-dir agent id ${JSON.stringify(id)}. Spawn callers must resolve the durable identity row via findOrCreateAgent first and pass that UUID — the bare-name shadow path was retired in wish retire-session-names-id-only Group 3 (migration 061 enforces the same shape at the DB).`,
250+
);
251+
}
252+
}
253+
233254
// biome-ignore lint/complexity/noExcessiveCognitiveComplexity: flat field mapping
234255
export async function register(agent: Agent): Promise<void> {
256+
assertRegisterableId(agent.id);
235257
const sql = await getConnection();
236258
const now = new Date().toISOString();
237259

src/lib/protocol-router-spawn.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,15 +347,22 @@ export async function spawnWorkerFromTemplate(
347347
const isClaude = template.provider === 'claude' || template.provider === 'claude-sdk';
348348
const effectiveSessionId = resumeSessionId ?? params.sessionId;
349349

350+
// Wish retire-session-names-id-only Group 3: spawn writes ONE row.
351+
// Resolve the durable identity UUID first so register() runs against the
352+
// same key the rest of the pipeline observes. The bare `workerId`
353+
// (e.g. team-role-<short>) lands on `custom_name` for human-display only.
354+
const autoSpawnIdentity = await registry.findOrCreateAgent(agentName, team, template.role);
355+
350356
const workerEntry: registry.Agent = {
351-
id: workerId,
357+
id: autoSpawnIdentity.id,
352358
paneId,
353359
session,
354360
provider: template.provider,
355361
transport: 'tmux',
356362
role: template.role,
357363
skill: template.skill,
358364
team,
365+
customName: workerId,
359366
worktree: null,
360367
startedAt: now,
361368
state: 'spawning',

src/term-commands/agents.ts

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -636,26 +636,27 @@ async function registerSpawnWorker(
636636
windowInfo?: { windowId: string; windowName: string } | null,
637637
): Promise<registry.Agent> {
638638
const nt = ctx.validated.nativeTeam;
639-
// Wish retire-session-names-id-only Group 3 (P0 spawn unblock):
640-
// `agents.id` MUST be UUID-or-`dir:<name>` per migration 061's
641-
// `agents_id_shape_check`. Pre-G3 spawn wrote `id: ctx.workerId` (a bare
642-
// name like "engineer-4d48"), which the constraint now rejects, breaking
643-
// every `genie agent spawn` on the live host.
639+
// Wish retire-session-names-id-only Group 3: spawn writes ONE row.
644640
//
645-
// Fix: route the durable identity UUID (`ctx.agentIdentityId`, set from
646-
// `findOrCreateAgent` upstream) into `id`, and stash the human-readable
647-
// workerId in `customName` for display/lookup. This collapses the prior
648-
// dual-row pattern (UUID identity row + bare-name runtime row) into the
649-
// single identity row that already exists. The `register()` ON CONFLICT
650-
// (id) DO UPDATE clause then merges runtime fields (pane/session/state)
651-
// into the same row instead of inserting a shadow.
641+
// `agents.id` is the durable identity UUID returned by `findOrCreateAgent`
642+
// (propagated as `ctx.agentIdentityId`). The bare `ctx.workerId`
643+
// ("engineer-4d48") is the human-readable label that lives on
644+
// `custom_name` for display/lookup — it never enters the id column.
652645
//
653-
// Fallback to `ctx.workerId` is intentional for the rare paths that don't
654-
// populate `agentIdentityId` yet — those paths land bare-name rows that
655-
// 061 will reject loudly, exactly as designed (we want them surfaced and
656-
// patched, not silently bypassed).
646+
// The previous fallback (`ctx.agentIdentityId ?? ctx.workerId`) used to
647+
// double-write the row keyed by bare-name whenever a caller forgot to
648+
// resolve the identity upstream — exactly the regression engine this wish
649+
// retires. Migration 061's `agents_id_shape_check` rejects those inserts
650+
// at the DB; we mirror the invariant in the application so the failure
651+
// mode is "loud throw at the call site that forgot the identity step"
652+
// instead of "opaque CHECK violation deeper in the SQL roundtrip."
653+
if (!ctx.agentIdentityId) {
654+
throw new Error(
655+
`registerSpawnWorker: missing agentIdentityId for workerId=${JSON.stringify(ctx.workerId)} (team=${JSON.stringify(ctx.validated.team)}). The spawn pipeline MUST call registry.findOrCreateAgent before register() — see wish retire-session-names-id-only Group 3.`,
656+
);
657+
}
657658
const workerEntry: registry.Agent = {
658-
id: ctx.agentIdentityId ?? ctx.workerId,
659+
id: ctx.agentIdentityId,
659660
paneId,
660661
session: ctx.validated.team,
661662
provider: ctx.validated.provider,
@@ -1207,7 +1208,12 @@ async function launchTmuxSpawn(ctx: SpawnCtx): Promise<string> {
12071208
if (ctx.executorId) {
12081209
await executorRegistry.updateExecutorState(ctx.executorId, 'running').catch(() => {});
12091210
}
1210-
await registry.update(ctx.workerId, { state: 'idle' }).catch(() => {});
1211+
// The row was inserted with id = ctx.agentIdentityId (UUID); updates must
1212+
// target that same key. Falling back to ctx.workerId would no-op silently
1213+
// because the bare-name row no longer exists.
1214+
if (ctx.agentIdentityId) {
1215+
await registry.update(ctx.agentIdentityId, { state: 'idle' }).catch(() => {});
1216+
}
12111217

12121218
// #1600 — emit terminal-state success event so `genie events list --type
12131219
// worker.spawn.ok` becomes a positive signal (existing `worker.spawn` is
@@ -1496,8 +1502,10 @@ async function runSdkQuery(
14961502
* then findDeadResumable routed it back through tmux-only resume → crash.
14971503
*/
14981504
async function rollbackSpawn(ctx: SpawnCtx, opts: { workerRegistered: boolean }): Promise<void> {
1499-
if (opts.workerRegistered) {
1500-
await registry.unregister(ctx.workerId).catch(() => {});
1505+
// Unregister by the same id the row was inserted under — the UUID identity,
1506+
// not the bare-name workerId (which now lives only on custom_name).
1507+
if (opts.workerRegistered && ctx.agentIdentityId) {
1508+
await registry.unregister(ctx.agentIdentityId).catch(() => {});
15011509
}
15021510
if (ctx.executorId) {
15031511
await executorRegistry.updateExecutorState(ctx.executorId, 'error').catch(() => {});
@@ -1544,7 +1552,10 @@ async function launchSdkSpawn(
15441552
if (ctx.executorId) {
15451553
await executorRegistry.updateExecutorState(ctx.executorId, 'done').catch(() => {});
15461554
}
1547-
await registry.unregister(ctx.workerId);
1555+
// The row was inserted under the UUID identity; unregister by the same key.
1556+
if (ctx.agentIdentityId) {
1557+
await registry.unregister(ctx.agentIdentityId);
1558+
}
15481559
console.log(`\nAgent "${ctx.workerId}" SDK session ended.`);
15491560
return ctx.workerId;
15501561
}
@@ -1608,7 +1619,9 @@ async function launchInlineSpawn(ctx: SpawnCtx): Promise<string> {
16081619
if (ctx.agentIdentityId && ctx.executorId) {
16091620
await executorRegistry.updateExecutorState(ctx.executorId, 'done').catch(() => {});
16101621
}
1611-
await registry.unregister(ctx.workerId);
1622+
if (ctx.agentIdentityId) {
1623+
await registry.unregister(ctx.agentIdentityId);
1624+
}
16121625
if (nt?.enabled && ctx.agentName) {
16131626
await nativeTeams.clearNativeInbox(ctx.validated.team, ctx.agentName).catch(() => {});
16141627
await nativeTeams.unregisterNativeMember(ctx.validated.team, ctx.agentName).catch(() => {});

0 commit comments

Comments
 (0)