diff --git a/apps/coder/src/routes/messages.ts b/apps/coder/src/routes/messages.ts index abc988f..e620ec9 100644 --- a/apps/coder/src/routes/messages.ts +++ b/apps/coder/src/routes/messages.ts @@ -224,8 +224,8 @@ export function registerMessageRoutes( // External provider: create a task for the dispatcher const projectId = sessionRows[0]!.project_id; const [task] = await sql<{ id: string; state: string }[]>` - INSERT INTO tasks (project_id, input, agent, model, mode_id, thinking_option_id, session_id) - VALUES (${projectId}, ${content}, ${provider}, ${model ?? null}, ${mode_id ?? null}, ${thinking_option_id ?? null}, ${sessionId}) + INSERT INTO tasks (project_id, input, agent, model, mode_id, thinking_option_id, session_id, chat_id) + VALUES (${projectId}, ${content}, ${provider}, ${model ?? null}, ${mode_id ?? null}, ${thinking_option_id ?? null}, ${sessionId}, ${chatId}) RETURNING id, state `; reply.code(202); diff --git a/apps/coder/src/routes/skills.ts b/apps/coder/src/routes/skills.ts index aa3519d..40b7fca 100644 --- a/apps/coder/src/routes/skills.ts +++ b/apps/coder/src/routes/skills.ts @@ -91,8 +91,8 @@ export function registerSkillRoutes( const taskInput = `${body}\n\n---\n\n${userText}`; const [task] = await sql<{ id: string; state: string }[]>` - INSERT INTO tasks (project_id, input, agent, model, mode_id, thinking_option_id, session_id) - VALUES (${sessionRows[0]!.project_id}, ${taskInput}, ${provider}, ${model ?? null}, ${mode_id ?? null}, ${thinking_option_id ?? null}, ${sessionId}) + INSERT INTO tasks (project_id, input, agent, model, mode_id, thinking_option_id, session_id, chat_id) + VALUES (${sessionRows[0]!.project_id}, ${taskInput}, ${provider}, ${model ?? null}, ${mode_id ?? null}, ${thinking_option_id ?? null}, ${sessionId}, ${chatId}) RETURNING id, state `; await sql`UPDATE chats SET updated_at = clock_timestamp() WHERE id = ${chatId}`; diff --git a/apps/coder/src/routes/worktree-safety.ts b/apps/coder/src/routes/worktree-safety.ts index 830ead6..0eb4525 100644 --- a/apps/coder/src/routes/worktree-safety.ts +++ b/apps/coder/src/routes/worktree-safety.ts @@ -18,7 +18,7 @@ export function registerWorktreeSafetyRoutes(app: FastifyInstance, sql: Sql): vo '/api/sessions/:sessionId/worktree-risk', async (req) => { const rows = await sql<{ worktree_path: string }[]>` - SELECT worktree_path FROM session_worktrees WHERE session_id = ${req.params.sessionId} + SELECT path AS worktree_path FROM worktrees WHERE session_id = ${req.params.sessionId} `; const reports = []; for (const row of rows) { @@ -33,7 +33,7 @@ export function registerWorktreeSafetyRoutes(app: FastifyInstance, sql: Sql): vo '/api/sessions/:sessionId/worktree-stash', async (req) => { const rows = await sql<{ worktree_path: string }[]>` - SELECT worktree_path FROM session_worktrees WHERE session_id = ${req.params.sessionId} + SELECT path AS worktree_path FROM worktrees WHERE session_id = ${req.params.sessionId} `; const results = []; for (const row of rows) { diff --git a/apps/coder/src/schema.sql b/apps/coder/src/schema.sql index 78752da..ffa5621 100644 --- a/apps/coder/src/schema.sql +++ b/apps/coder/src/schema.sql @@ -83,16 +83,20 @@ CREATE TABLE IF NOT EXISTS session_worktrees ( base_commit TEXT, created_at TIMESTAMPTZ NOT NULL DEFAULT clock_timestamp() ); --- Migrate existing FK to CASCADE (idempotent: drops the old constraint if present). +-- P1.5-b: DEFANG the CASCADE — a session delete must no longer wipe its worktree +-- row. This table is SUPERSEDED by `worktrees` below; all readers are repointed +-- this phase, so the row just persists (dead) on session delete until a later +-- cleanup drops the table. session_id is this table's PRIMARY KEY, so it cannot be +-- nullable → SET NULL is invalid and NO ACTION/RESTRICT would block deletes; the +-- only valid defang is to drop the FK with no replacement. Idempotent: only fires +-- while the FK is still ON DELETE CASCADE ('c'). DO $$ BEGIN IF EXISTS ( SELECT 1 FROM pg_constraint WHERE conname = 'session_worktrees_session_id_fkey' - AND confdeltype <> 'c' + AND confdeltype = 'c' ) THEN ALTER TABLE session_worktrees DROP CONSTRAINT session_worktrees_session_id_fkey; - ALTER TABLE session_worktrees ADD CONSTRAINT session_worktrees_session_id_fkey - FOREIGN KEY (session_id) REFERENCES sessions(id) ON DELETE CASCADE; END IF; END $$; @@ -127,6 +131,80 @@ END $$; -- v2.6: config fingerprint for stale-session detection (auto-recover on model change). ALTER TABLE agent_sessions ADD COLUMN IF NOT EXISTS config_hash TEXT; +-- ─── P1.5-b (corrected): worktrees entity + re-key agent_sessions to (chat_id, agent) ─── +-- The TAB (a chat) is the context unit: two opencode tabs in one session = two +-- independent contexts sharing one worktree. So agent_sessions keys on +-- (chat_id, agent), NOT (worktree_id, agent) or (session_id, agent). The +-- `worktrees` table is one-per-session (selectable later) and only referenced +-- informationally by agent_sessions.worktree_id (SET NULL); chat_id is the key. +-- +-- PREREQUISITE: the unmigratable test session (35 chats, 1 agent_sessions row that +-- maps to no single chat) is DELETED before this runs, so agent_sessions is empty +-- and the chat_id backfill is N/A. If a row with NULL chat_id remains, the verify +-- gate below RAISEs and aborts — delete the offending session first. + +-- worktree as a first-class entity; survives session delete (session_id SET NULL). +CREATE TABLE IF NOT EXISTS worktrees ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + session_id UUID REFERENCES sessions(id) ON DELETE SET NULL, + project_id UUID, + path TEXT NOT NULL, + branch TEXT, + base_commit TEXT, + slug TEXT, + status TEXT NOT NULL DEFAULT 'active' CHECK (status IN ('active','archived')), + created_at TIMESTAMPTZ NOT NULL DEFAULT clock_timestamp() +); +CREATE UNIQUE INDEX IF NOT EXISTS worktrees_active_path_uidx ON worktrees(path) WHERE status='active'; + +-- Migrate any surviving session_worktrees rows → worktrees (idempotent; 0 rows +-- after the test-session delete, kept for generality / fresh-DB safety). +INSERT INTO worktrees (session_id, path, branch, base_commit, status) +SELECT sw.session_id, sw.worktree_path, 'session-' || sw.session_id, sw.base_commit, 'active' +FROM session_worktrees sw +WHERE NOT EXISTS (SELECT 1 FROM worktrees w WHERE w.session_id = sw.session_id AND w.status='active'); + +-- Dispatch hint: which chat (tab) a task belongs to. The coder message route and +-- skills route set it from the frontend tab; session-less creators (arena, MCP, +-- new_task, generic /api/tasks) leave it NULL and the dispatcher creates a chat. +ALTER TABLE tasks ADD COLUMN IF NOT EXISTS chat_id UUID REFERENCES chats(id) ON DELETE SET NULL; + +-- Re-key columns on agent_sessions. +ALTER TABLE agent_sessions ADD COLUMN IF NOT EXISTS chat_id UUID; +ALTER TABLE agent_sessions ADD COLUMN IF NOT EXISTS worktree_id UUID; + +-- BACKFILL-VERIFY GATE: the new PK is (chat_id, agent), so chat_id must be +-- non-null on every row before the swap. With the test session deleted this is a +-- 0-row assertion; if any row has NULL chat_id (an unmigratable pre-existing row), +-- abort loudly rather than create a degenerate (NULL, agent) key. +DO $$ +DECLARE n int; +BEGIN + SELECT count(*) INTO n FROM agent_sessions WHERE chat_id IS NULL; + IF n > 0 THEN + RAISE EXCEPTION 'P1.5-b: % agent_sessions row(s) have NULL chat_id — delete the unmigratable session(s) before applying', n; + END IF; +END $$; + +-- Swap PK (session_id,agent) → (chat_id,agent) + FKs (run-once, guarded on the new +-- FK's absence). chat_id CASCADEs from chats (closing a tab ends its context); +-- worktree_id is informational SET NULL; session_id defanged to nullable SET NULL. +DO $$ BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'agent_sessions_chat_id_fkey') THEN + ALTER TABLE agent_sessions DROP CONSTRAINT IF EXISTS agent_sessions_pkey; + ALTER TABLE agent_sessions DROP CONSTRAINT IF EXISTS agent_sessions_session_id_fkey; + ALTER TABLE agent_sessions ALTER COLUMN session_id DROP NOT NULL; + ALTER TABLE agent_sessions ALTER COLUMN chat_id SET NOT NULL; + ALTER TABLE agent_sessions ADD CONSTRAINT agent_sessions_pkey PRIMARY KEY (chat_id, agent); + ALTER TABLE agent_sessions ADD CONSTRAINT agent_sessions_chat_id_fkey + FOREIGN KEY (chat_id) REFERENCES chats(id) ON DELETE CASCADE; + ALTER TABLE agent_sessions ADD CONSTRAINT agent_sessions_session_id_fkey + FOREIGN KEY (session_id) REFERENCES sessions(id) ON DELETE SET NULL; + ALTER TABLE agent_sessions ADD CONSTRAINT agent_sessions_worktree_id_fkey + FOREIGN KEY (worktree_id) REFERENCES worktrees(id) ON DELETE SET NULL; + END IF; +END $$; + -- v2.6: attribution for DiffPanel badges (Phase 1 UX reads this). ALTER TABLE pending_changes ADD COLUMN IF NOT EXISTS agent TEXT; diff --git a/apps/coder/src/services/agent-backend.ts b/apps/coder/src/services/agent-backend.ts index 4884cad..b0d1d59 100644 --- a/apps/coder/src/services/agent-backend.ts +++ b/apps/coder/src/services/agent-backend.ts @@ -37,8 +37,15 @@ export interface EnsureSessionOpts { agent: string; /** Resolved model id. */ model: string; + /** P1.5-b: the chat (tab) this turn belongs to. agent_sessions is keyed + * (chat_id, agent) — the tab/chat is the context unit. Always non-null: + * the dispatcher creates a chat for session-less tasks before calling. */ + chatId: string; /** Shared per-session worktree (one per `sessions.id`, not per pane). */ worktreePath: string; + /** P1.5-b: the `worktrees.id` for this session's worktree — stored on the + * agent_sessions row informationally (NOT the key). */ + worktreeId: string; projectId: string; } @@ -47,6 +54,10 @@ export interface AgentSessionHandle { sessionId: string; agent: string; backend: AgentBackendKind; + /** P1.5-b: the chat (tab) this session is keyed on (with agent). */ + chatId: string; + /** P1.5-b: the worktree this session's chat runs in (informational link). */ + worktreeId: string; /** Provider's own session id (resume token); null until the backend assigns one. */ agentSessionId: string | null; /** opencode HTTP server port; null for ACP backends. */ diff --git a/apps/coder/src/services/backends/opencode-server.ts b/apps/coder/src/services/backends/opencode-server.ts index f056941..a3d7d8c 100644 --- a/apps/coder/src/services/backends/opencode-server.ts +++ b/apps/coder/src/services/backends/opencode-server.ts @@ -423,9 +423,12 @@ export class OpenCodeServerBackend implements AgentBackend { if (!this.client) throw new Error('opencode-server: client not ready after ensureServer'); const configHash = sessionConfigHash(opts.model); + // P1.5-b: agent_sessions is keyed (chat_id, agent) — the tab/chat is the + // context unit (two tabs in one session = two contexts sharing one worktree). + // session_id + worktree_id are retained as informational (SET NULL) columns. const [row] = await this.sql<{ agent_session_id: string | null; status: string; config_hash: string | null }[]>` SELECT agent_session_id, status, config_hash FROM agent_sessions - WHERE session_id = ${sessionId} AND agent = ${opts.agent} + WHERE chat_id = ${opts.chatId} AND agent = ${opts.agent} `; let agentSessionId = row?.agent_session_id ?? null; @@ -447,10 +450,12 @@ export class OpenCodeServerBackend implements AgentBackend { agentSessionId = created.data.id; await this.sql` INSERT INTO agent_sessions - (session_id, agent, backend, agent_session_id, server_port, status, last_active_at, config_hash) + (chat_id, session_id, worktree_id, agent, backend, agent_session_id, server_port, status, last_active_at, config_hash) VALUES - (${sessionId}, ${opts.agent}, 'opencode_server', ${agentSessionId}, ${this.port}, 'active', clock_timestamp(), ${configHash}) - ON CONFLICT (session_id, agent) DO UPDATE SET + (${opts.chatId}, ${sessionId}, ${opts.worktreeId}, ${opts.agent}, 'opencode_server', ${agentSessionId}, ${this.port}, 'active', clock_timestamp(), ${configHash}) + ON CONFLICT (chat_id, agent) DO UPDATE SET + session_id = EXCLUDED.session_id, + worktree_id = EXCLUDED.worktree_id, backend = 'opencode_server', agent_session_id = EXCLUDED.agent_session_id, server_port = EXCLUDED.server_port, @@ -462,7 +467,7 @@ export class OpenCodeServerBackend implements AgentBackend { await this.sql` UPDATE agent_sessions SET status = 'active', last_active_at = clock_timestamp(), server_port = ${this.port}, config_hash = ${configHash} - WHERE session_id = ${sessionId} AND agent = ${opts.agent} + WHERE chat_id = ${opts.chatId} AND agent = ${opts.agent} `; } @@ -498,6 +503,8 @@ export class OpenCodeServerBackend implements AgentBackend { sessionId, agent: opts.agent, backend: 'opencode_server', + chatId: opts.chatId, + worktreeId: opts.worktreeId, agentSessionId: ocSessionId, serverPort: this.port, }; @@ -593,7 +600,7 @@ export class OpenCodeServerBackend implements AgentBackend { } await this.sql` UPDATE agent_sessions SET status = 'closed' - WHERE session_id = ${handle.sessionId} AND agent = ${handle.agent} + WHERE chat_id = ${handle.chatId} AND agent = ${handle.agent} `.catch(() => {}); } diff --git a/apps/coder/src/services/dispatcher.ts b/apps/coder/src/services/dispatcher.ts index fdbf716..94816e6 100644 --- a/apps/coder/src/services/dispatcher.ts +++ b/apps/coder/src/services/dispatcher.ts @@ -78,8 +78,9 @@ export function createDispatcher(deps: Deps): { start(): void; stop(): Promise` - SELECT id, project_id, input, agent, model, mode_id, thinking_option_id, session_id + SELECT id, project_id, input, agent, model, mode_id, thinking_option_id, session_id, chat_id FROM tasks WHERE state = 'pending' ORDER BY created_at @@ -110,6 +111,7 @@ export function createDispatcher(deps: Deps): { start(): void; stop(): Promise { const taskId = task.id; @@ -511,6 +513,7 @@ export function createDispatcher(deps: Deps): { start(): void; stop(): Promise { @@ -543,10 +546,18 @@ export function createDispatcher(deps: Deps): { start(): void; stop(): Promise` SELECT id FROM chats WHERE session_id = ${sessionId} AND status = 'open' ORDER BY created_at DESC LIMIT 1 @@ -587,7 +598,7 @@ export function createDispatcher(deps: Deps): { start(): void; stop(): Promise` branch, `/sess-` dir) so it never * collides with the per-task worktrees that arena/new_task/MCP still use. @@ -139,11 +141,13 @@ export async function ensureSessionWorktree( sessionId: string, opts?: { signal?: AbortSignal }, ): Promise { - const [existing] = await sql<{ worktree_path: string; base_commit: string | null }[]>` - SELECT worktree_path, base_commit FROM session_worktrees WHERE session_id = ${sessionId} + const [existing] = await sql<{ id: string; path: string; base_commit: string | null }[]>` + SELECT id, path, base_commit FROM worktrees + WHERE session_id = ${sessionId} AND status = 'active' + LIMIT 1 `; if (existing) { - return { worktreePath: existing.worktree_path, baseCommit: existing.base_commit }; + return { worktreeId: existing.id, worktreePath: existing.path, baseCommit: existing.base_commit }; } const worktreePath = `${WORKTREE_BASE}/sess-${sessionId}`; @@ -167,17 +171,28 @@ export async function ensureSessionWorktree( throw new Error(`Failed to create session worktree: ${result.stderr.trim() || result.stdout.trim()}`); } - // Persist. ON CONFLICT keeps the first writer's row if two turns race the create. - await sql` - INSERT INTO session_worktrees (session_id, worktree_path, base_commit) - VALUES (${sessionId}, ${worktreePath}, ${baseCommit}) - ON CONFLICT (session_id) DO NOTHING + // Insert-or-get: WHERE NOT EXISTS keeps the first writer's row if two turns race + // the create (the partial unique on active path also backstops it). + const [inserted] = await sql<{ id: string; path: string; base_commit: string | null }[]>` + INSERT INTO worktrees (session_id, path, branch, base_commit, status) + SELECT ${sessionId}, ${worktreePath}, ${branchName}, ${baseCommit}, 'active' + WHERE NOT EXISTS ( + SELECT 1 FROM worktrees WHERE session_id = ${sessionId} AND status = 'active' + ) + RETURNING id, path, base_commit `; - const [row] = await sql<{ worktree_path: string; base_commit: string | null }[]>` - SELECT worktree_path, base_commit FROM session_worktrees WHERE session_id = ${sessionId} + if (inserted) { + return { worktreeId: inserted.id, worktreePath: inserted.path, baseCommit: inserted.base_commit }; + } + // Lost the race — another turn inserted first; read its row. + const [row] = await sql<{ id: string; path: string; base_commit: string | null }[]>` + SELECT id, path, base_commit FROM worktrees + WHERE session_id = ${sessionId} AND status = 'active' + LIMIT 1 `; return { - worktreePath: row?.worktree_path ?? worktreePath, + worktreeId: row!.id, + worktreePath: row?.path ?? worktreePath, baseCommit: row?.base_commit ?? baseCommit, }; } diff --git a/apps/server/src/routes/sessions.ts b/apps/server/src/routes/sessions.ts index 288cc28..a2fc66a 100644 --- a/apps/server/src/routes/sessions.ts +++ b/apps/server/src/routes/sessions.ts @@ -432,16 +432,18 @@ export function registerSessionRoutes( const id = req.params.id; const force = req.query.force === 'true' || req.query.force === '1'; - // Session-delete work-loss guard. CASCADE on session_worktrees means the - // DELETE below auto-wipes the worktree row, so the safety check MUST run - // BEFORE it (paths read while the row still exists, pre-CASCADE). + // Session-delete work-loss guard. The check MUST run BEFORE the DELETE: + // worktrees.session_id is ON DELETE SET NULL (P1.5-b), so once the session + // is gone the worktree rows no longer point back to it — read them while + // the link still exists. // - // Optimization: read session_worktrees from our own (shared) DB first. - // No row => chat-only session => nothing on disk => delete immediately, - // zero round-trip. Only worktree-backed sessions pay the host git check. + // Optimization: read worktrees (P1.5-b — was session_worktrees) from our + // own (shared) DB first. No row => chat-only session => nothing on disk => + // delete immediately, zero round-trip. Only worktree-backed sessions pay + // the host git check. if (!force) { - const worktrees = await sql<{ worktree_path: string }[]>` - SELECT worktree_path FROM session_worktrees WHERE session_id = ${id} + const worktrees = await sql<{ path: string }[]>` + SELECT path FROM worktrees WHERE session_id = ${id} `; if (worktrees.length > 0) { // Worktree dirs live on the host; only BooCoder can run git on them.