feat(sessions): add session_id FK to link agent_sessions to session logs (Phase 1 of #1668)

.squawk.toml

@@ -22,12 +22,13 @@ pg_version = "16"
 #   by ensuring migrations only run once.
 #
 # require-concurrent-index-creation, disallowed-unique-constraint, constraint-missing-not-valid,
-# adding-not-nullable-field:
+# adding-not-nullable-field, adding-foreign-key-constraint:
 #   Drizzle's migrator runs ALL statements in a single transaction (see pg-core/dialect.js
-#   line 60). CREATE INDEX CONCURRENTLY cannot run inside a transaction, so these online-safe
-#   patterns are impossible with Drizzle. Tables are small (~700 rows) so brief locking is
-#   acceptable. If large-table migrations are needed in the future, use a separate raw SQL
-#   runner outside Drizzle.
+#   line 60). CREATE INDEX CONCURRENTLY cannot run inside a transaction, and the NOT VALID
+#   two-step pattern for FK constraints requires two separate transactions — both impossible
+#   with Drizzle. Tables are small (hundreds of rows) so brief locking is acceptable.
+#   If large-table migrations are needed in the future, use a separate raw SQL runner
+#   outside Drizzle (see .claude/rules/database-migrations.md for the manual migration pattern).
 excluded_rules = [
   "prefer-bigint-over-int",
   "prefer-identity",
@@ -38,4 +39,5 @@ excluded_rules = [
   "disallowed-unique-constraint",
   "constraint-missing-not-valid",
   "adding-not-nullable-field",
+  "adding-foreign-key-constraint",
 ]

apps/web/src/app/internal/agent-sessions/agent-sessions-content.tsx

@@ -49,18 +49,25 @@ async function loadFromApi(): Promise<FetchResult<AgentSessionRow[]>> {
     revalidate: 60,
   });
 
-  // Build a branch → session log map for enrichment
+  // Build lookup maps for enrichment:
+  // 1. session_id → session log (direct FK, preferred for newer records)
+  // 2. branch → session log (fallback heuristic for older records without session_id)
+  const logsById = new Map<number, SessionRow>();
   const logsByBranch = new Map<string, SessionRow>();
   if (logsResult.ok) {
     for (const log of logsResult.data.items) {
+      logsById.set(log.id, log);
       if (log.branch) {
         logsByBranch.set(log.branch, log);
       }
     }
   }
 
   const rows: AgentSessionRow[] = agentResult.data.sessions.map((s): AgentSessionRow => {
-    const log = logsByBranch.get(s.branch);
+    // Prefer FK-linked session log; only fall back to branch heuristic when sessionId is absent
+    const log = s.sessionId != null
+      ? logsById.get(s.sessionId)
+      : logsByBranch.get(s.branch);
     return {
       id: s.id,
       branch: s.branch,
@@ -72,7 +79,7 @@ async function loadFromApi(): Promise<FetchResult<AgentSessionRow[]>> {
       startedAt: s.startedAt,
       completedAt: s.completedAt,
       // Prefer prUrl from agent_sessions (set by `crux issues done --pr=URL`),
-      // fall back to session log join on branch for older records.
+      // fall back to session log for older records.
       prUrl: s.prUrl ?? log?.prUrl ?? null,
       prOutcome: s.prOutcome ?? null,
       fixesPrUrl: s.fixesPrUrl ?? null,

apps/wiki-server/drizzle/0063_add_session_id_to_agent_sessions.sql

@@ -0,0 +1,20 @@
+-- Add session_id FK to agent_sessions, linking live tracking records to their
+-- corresponding historical session log.
+--
+-- This replaces the fragile branch-name join currently used by the Agent Sessions
+-- dashboard to enrich agent_sessions with cost/model data from the sessions table.
+-- With this FK, the dashboard can use a direct JOIN instead of a heuristic match.
+--
+-- The column is nullable because:
+-- 1. Older records predate this migration and won't have a session_id
+-- 2. Some agent sessions may never produce a session log (e.g., abandoned sessions)
+-- 3. Session logs can be created after agent sessions (at PR time)
+--
+-- Note: adding-foreign-key-constraint is excluded in .squawk.toml because Drizzle's
+-- migrator runs in a single transaction, making the two-step NOT VALID / VALIDATE
+-- pattern impossible. agent_sessions is a small table (hundreds of rows), so the
+-- brief SHARE ROW EXCLUSIVE lock is acceptable.
+
+ALTER TABLE "agent_sessions" ADD COLUMN IF NOT EXISTS "session_id" bigint REFERENCES "sessions"("id") ON DELETE SET NULL;
+
+CREATE INDEX IF NOT EXISTS "idx_as_session_id" ON "agent_sessions" ("session_id");

apps/wiki-server/drizzle/meta/_journal.json

@@ -442,6 +442,13 @@
       "when": 1773993600000,
       "tag": "0062_add_numeric_cost_duration_to_sessions",
       "breakpoints": true
+    },
+    {
+      "idx": 63,
+      "version": "7",
+      "when": 1774080000000,
+      "tag": "0063_add_session_id_to_agent_sessions",
+      "breakpoints": true
     }
   ]
 }

apps/wiki-server/src/__tests__/agent-sessions.test.ts

@@ -16,6 +16,7 @@ let store: Array<{
   session_type: string;
   issue_number: number | null;
   checklist_md: string;
+  session_id: number | null;
   status: string;
   started_at: Date;
   completed_at: Date | null;
@@ -48,6 +49,7 @@ const dispatch: SqlDispatcher = (query, params) => {
       session_type: params[2] as string,
       issue_number: params[3] as number | null,
       checklist_md: params[4] as string,
+      session_id: null,
       status: "active",
       started_at: new Date(),
       completed_at: null,
@@ -88,6 +90,9 @@ const dispatch: SqlDispatcher = (query, params) => {
           case "checklist_md":
             store[idx].checklist_md = params[pIdx] as string;
             break;
+          case "session_id":
+            store[idx].session_id = params[pIdx] as number | null;
+            break;
           case "status":
             store[idx].status = params[pIdx] as string;
             break;
@@ -486,6 +491,50 @@ describe("Agent Sessions API", () => {
       });
       expect(res.status).toBe(400);
     });
+
+    it("sets sessionId FK link to session log", async () => {
+      await postJson(app, "/api/agent-sessions", sampleSession);
+
+      const res = await patchJson(app, "/api/agent-sessions/1", {
+        sessionId: 42,
+      });
+      expect(res.status).toBe(200);
+      const body = await res.json();
+      expect(body.sessionId).toBe(42);
+    });
+
+    it("clears sessionId with null", async () => {
+      await postJson(app, "/api/agent-sessions", sampleSession);
+
+      // Set it first
+      await patchJson(app, "/api/agent-sessions/1", { sessionId: 42 });
+
+      // Now clear it
+      const res = await patchJson(app, "/api/agent-sessions/1", {
+        sessionId: null,
+      });
+      expect(res.status).toBe(200);
+      const body = await res.json();
+      expect(body.sessionId).toBeNull();
+    });
+
+    it("rejects non-integer sessionId", async () => {
+      await postJson(app, "/api/agent-sessions", sampleSession);
+
+      const res = await patchJson(app, "/api/agent-sessions/1", {
+        sessionId: 1.5,
+      });
+      expect(res.status).toBe(400);
+    });
+
+    it("rejects zero sessionId", async () => {
+      await postJson(app, "/api/agent-sessions", sampleSession);
+
+      const res = await patchJson(app, "/api/agent-sessions/1", {
+        sessionId: 0,
+      });
+      expect(res.status).toBe(400);
+    });
   });
 
   // ================================================================

apps/wiki-server/src/api-types.ts

@@ -1174,6 +1174,8 @@ export const UpdateAgentSessionSchema = z.object({
   prUrl: z.string().url().max(1000).nullable().optional(),
   prOutcome: z.enum(PR_OUTCOMES).nullable().optional(),
   fixesPrUrl: z.string().url().max(1000).nullable().optional(),
+  /** FK to the sessions table — set when the session log is synced to the DB */
+  sessionId: z.number().int().positive().nullable().optional(),
 });
 export type UpdateAgentSession = z.infer<typeof UpdateAgentSessionSchema>;
 

apps/wiki-server/src/routes/agent-sessions.ts

@@ -115,15 +115,16 @@ const agentSessionsApp = new Hono()
     const parsed = UpdateAgentSessionSchema.safeParse(body);
     if (!parsed.success) return validationError(c, parsed.error.message);
 
-    const { checklistMd, status, prUrl, prOutcome, fixesPrUrl } = parsed.data;
+    const { checklistMd, status, prUrl, prOutcome, fixesPrUrl, sessionId } = parsed.data;
     if (
       checklistMd === undefined &&
       status === undefined &&
       prUrl === undefined &&
       prOutcome === undefined &&
-      fixesPrUrl === undefined
+      fixesPrUrl === undefined &&
+      sessionId === undefined
     ) {
-      return validationError(c, "At least one of checklistMd, status, prUrl, prOutcome, or fixesPrUrl must be provided");
+      return validationError(c, "At least one of checklistMd, status, prUrl, prOutcome, fixesPrUrl, or sessionId must be provided");
     }
 
     const updates: Record<string, unknown> = { updatedAt: new Date() };
@@ -137,13 +138,24 @@ const agentSessionsApp = new Hono()
     if (prUrl !== undefined) updates.prUrl = prUrl;
     if (prOutcome !== undefined) updates.prOutcome = prOutcome;
     if (fixesPrUrl !== undefined) updates.fixesPrUrl = fixesPrUrl;
+    if (sessionId !== undefined) updates.sessionId = sessionId;
 
     const db = getDrizzleDb();
-    const result = await db
-      .update(agentSessions)
-      .set(updates)
-      .where(eq(agentSessions.id, id))
-      .returning();
+    let result;
+    try {
+      result = await db
+        .update(agentSessions)
+        .set(updates)
+        .where(eq(agentSessions.id, id))
+        .returning();
+    } catch (e: unknown) {
+      // Translate FK violation into a 400 response
+      const msg = e instanceof Error ? e.message : String(e);
+      if (msg.includes("foreign key") || msg.includes("violates foreign key")) {
+        return c.json({ error: "invalid_reference", message: "sessionId references a non-existent session" }, 400);
+      }
+      throw e;
+    }
 
     if (result.length === 0) {
       return c.json({ error: "not_found", message: `No session with id: ${id}` }, 404);

apps/wiki-server/src/schema.ts

@@ -762,6 +762,7 @@ export const agentSessions = pgTable(
     prUrl: text("pr_url"), // PR URL recorded when crux issues done --pr=URL is called
     prOutcome: text("pr_outcome"), // Outcome: merged | merged_with_revisions | reverted | closed_without_merge
     fixesPrUrl: text("fixes_pr_url"), // URL of the PR this session is fixing (enables fix-chain tracking)
+    sessionId: bigint("session_id", { mode: "number" }).references(() => sessions.id, { onDelete: "set null" }), // FK to session log — set when session completes and log is synced
     status: text("status").notNull().default("active"),
     startedAt: timestamp("started_at", { withTimezone: true })
       .notNull()
@@ -779,6 +780,7 @@ export const agentSessions = pgTable(
     index("idx_as_status").on(table.status),
     index("idx_as_issue").on(table.issueNumber),
     index("idx_as_started_at").on(table.startedAt),
+    index("idx_as_session_id").on(table.sessionId),
   ]
 );
 

crux/pr-patrol/index.ts

@@ -28,6 +28,7 @@ export type PrIssueType =
   | 'bot-review-nitpick';
 
 interface BotComment {
+  threadId: string;
   path: string;
   line: number | null;
   startLine: number | null;
@@ -614,7 +615,7 @@ const PR_QUERY = `query($owner: String!, $name: String!) {
           }}
         }}}}
         reviewThreads(first: 50) { nodes {
-          isResolved isOutdated path line startLine
+          id isResolved isOutdated path line startLine
           comments(first: 3) { nodes {
             author { login }
             body
@@ -626,6 +627,7 @@ const PR_QUERY = `query($owner: String!, $name: String!) {
 }`;
 
 interface GqlReviewThread {
+  id: string;
   isResolved: boolean;
   isOutdated: boolean;
   path: string;
@@ -684,6 +686,7 @@ function extractBotComments(pr: GqlPrNode): BotComment[] {
     if (!KNOWN_BOT_LOGINS.has(firstComment.author.login)) continue;
 
     comments.push({
+      threadId: thread.id,
       path: thread.path,
       line: thread.line,
       startLine: thread.startLine,
@@ -757,7 +760,7 @@ const SINGLE_PR_QUERY = `query($owner: String!, $name: String!, $number: Int!) {
         }}
       }}}}
       reviewThreads(first: 50) { nodes {
-        isResolved isOutdated path line startLine
+        id isResolved isOutdated path line startLine
         comments(first: 3) { nodes {
           author { login }
           body
@@ -1202,6 +1205,27 @@ function spawnClaude(
   });
 }
 
+/** Best-effort: resolve CodeRabbit review threads that were addressed by the fix. */
+async function resolveBotReviewThreads(botComments: BotComment[]): Promise<void> {
+  if (botComments.length === 0) return;
+
+  let resolved = 0;
+  for (const comment of botComments) {
+    try {
+      await githubGraphQL(
+        `mutation($id: ID!) { resolveReviewThread(input: {threadId: $id}) { thread { isResolved } } }`,
+        { id: comment.threadId },
+      );
+      resolved++;
+    } catch (e) {
+      log(`  ${cl.dim}Warning: could not resolve thread on ${comment.path}: ${e instanceof Error ? e.message : String(e)}${cl.reset}`);
+    }
+  }
+  if (resolved > 0) {
+    log(`  Resolved ${resolved}/${botComments.length} bot review thread(s)`);
+  }
+}
+
 async function fixPr(pr: ScoredPr, config: PatrolConfig): Promise<void> {
   log(`${cl.bold}→${cl.reset} Fixing PR ${cl.cyan}#${pr.number}${cl.reset} (${pr.title})`);
   log(`  Issues: ${cl.yellow}${pr.issues.join(', ')}${cl.reset}`);
@@ -1254,6 +1278,11 @@ async function fixPr(pr: ScoredPr, config: PatrolConfig): Promise<void> {
           },
         }).catch(() => log('  Warning: could not post summary comment'));
       }
+
+      // Resolve bot review threads that were addressed by this fix
+      if (pr.botComments.length > 0) {
+        await resolveBotReviewThreads(pr.botComments);
+      }
     } else if (result.hitMaxTurns) {
       const failCount = recordMaxTurnsFailure(pr.number);
       outcome = 'max-turns';

crux/wiki-server/sync-session.ts

@@ -20,6 +20,7 @@ import { fileURLToPath } from 'url';
 import { parse as parseYaml } from 'yaml';
 import { parseCliArgs } from '../lib/cli.ts';
 import { createSession, type SessionApiEntry } from '../lib/wiki-server/sessions.ts';
+import { getAgentSessionByBranch, updateAgentSession } from '../lib/wiki-server/agent-sessions.ts';
 
 const PAGE_ID_RE = /^[a-z0-9][a-z0-9-]*$/;
 
@@ -112,13 +113,34 @@ export function parseSessionYaml(filePath: string): SessionApiEntry | null {
 /**
  * Sync a single session YAML file to the wiki-server.
  * Returns true if the POST succeeded, false otherwise.
+ *
+ * After successfully creating the session record, also updates the corresponding
+ * agent_session (matched by branch) to set session_id — establishing the FK link
+ * between the live tracking record and the historical session log.
  */
 export async function syncSessionFile(filePath: string): Promise<boolean> {
   const entry = parseSessionYaml(filePath);
   if (!entry) return false;
 
   const result = await createSession(entry);
-  return result.ok;
+  if (!result.ok) return false;
+
+  // Best-effort: link the agent_session to the newly-created session log via FK.
+  // This replaces the fragile branch-name join used by the Agent Sessions dashboard.
+  if (entry.branch) {
+    try {
+      const agentSessionResult = await getAgentSessionByBranch(entry.branch);
+      if (agentSessionResult.ok && agentSessionResult.data.sessionId == null) {
+        await updateAgentSession(agentSessionResult.data.id, {
+          sessionId: result.data.id,
+        });
+      }
+    } catch {
+      // Best-effort — local YAML is authoritative; FK linking is a nice-to-have
+    }
+  }
+
+  return true;
 }
 
 // ---------------------------------------------------------------------------
@@ -153,9 +175,9 @@ async function main() {
   console.log(`  Branch: ${entry.branch || '(none)'}`);
   console.log(`  Pages: ${entry.pages?.length || 0}`);
 
-  const result = await createSession(entry);
-  if (result.ok) {
-    console.log(`\u2713 Session synced to wiki-server (id: ${result.data.id})`);
+  const ok = await syncSessionFile(resolved);
+  if (ok) {
+    console.log(`\u2713 Session synced to wiki-server`);
   } else {
     console.log('Warning: could not sync session to wiki-server (server unavailable or error)');
     // Not a hard failure — YAML is authoritative