Harden the subagent roster fold and close review gaps from #160

apps/server/src/diagnostics/ProcessDiagnostics.test.ts

@@ -467,6 +467,55 @@ describe("ProcessDiagnostics", () => {
     });
   });
 
+  it("never detects a live provider session's own child processes", () => {
+    const SERVER_PID = 100;
+    const processRow = (
+      pid: number,
+      ppid: number,
+      command: string,
+      elapsed = "05:00",
+    ): ProcessDiagnostics.ProcessRow => ({
+      pid,
+      ppid,
+      pgid: pid,
+      status: "S",
+      cpuPercent: 0,
+      rssBytes: 1024,
+      elapsed,
+      command,
+    });
+    const processRows = [
+      // The provider session and its transient tool children.
+      processRow(200, SERVER_PID, "claude --output-format stream-json"),
+      processRow(300, 200, "zsh -c source /Users/will/.claude/shell-snapshots/snapshot.sh"),
+      processRow(400, 300, "ugrep -G --ignore-files --hidden -I -- pattern ."),
+      // A genuinely orphaned dev process, reparented to launchd.
+      processRow(500, 1, "node scripts/dev-runner.ts dev --port 5990"),
+    ];
+
+    // Chat prose mentioned the wrapper and helper pids; hints matched them too.
+    const seeded = ProcessDiagnostics.resolveBackgroundRunsFromListeningPorts({
+      urls: [],
+      pids: [300, 400],
+      portRows: [],
+      processRows,
+      commandHints: ["zsh -c source snapshot", "ugrep -G --ignore-files"],
+      serverPid: SERVER_PID,
+    });
+    expect(seeded.runs).toEqual([]);
+
+    // The orphan is still found through its command hint.
+    const orphan = ProcessDiagnostics.resolveBackgroundRunsFromListeningPorts({
+      urls: [],
+      pids: [],
+      portRows: [],
+      processRows,
+      commandHints: ["node scripts/dev-runner.ts dev --port 5990"],
+      serverPid: SERVER_PID,
+    });
+    expect(orphan.runs.map((run) => run.pid)).toEqual([500]);
+  });
+
   it("uses command hints to resolve descendant-owned preview ports", () => {
     const result = ProcessDiagnostics.resolveBackgroundRunsFromListeningPorts({
       urls: ["http://localhost:6013", "http://localhost:14053"],

apps/server/src/diagnostics/ProcessDiagnostics.ts

@@ -1246,6 +1246,61 @@ function selectHintedProcessOnlyRows(input: {
   return best ? [best] : [];
 }
 
+/** Command shapes of the provider CLIs the server spawns for sessions. Path
+ *  segments count (`/opt/x/codex app-server`), dotted dirs (`.claude/…` in a
+ *  wrapper's args) do not. */
+const PROVIDER_SESSION_COMMAND_PATTERN =
+  /(?:^|[\s/])(?:claude|codex|cursor-agent|opencode)(?=$|[\s.])/i;
+
+/**
+ * Pids living under a live provider session's process subtree. A session's
+ * own children — shell wrappers around tool calls, search helpers, background
+ * commands — are tracked work (the task stream and terminal rows own them),
+ * not lost processes, so process-only detection must never resurface them as
+ * "detected" rows with stop buttons. Anything truly orphaned reparents to
+ * init/launchd, leaves this subtree, and stays detectable.
+ */
+export function providerSessionShieldedPids(
+  rows: ReadonlyArray<ProcessRow>,
+  serverPid: number,
+): Set<number> {
+  const childrenByParent = new Map<number, ProcessRow[]>();
+  for (const row of rows) {
+    const children = childrenByParent.get(row.ppid) ?? [];
+    children.push(row);
+    childrenByParent.set(row.ppid, children);
+  }
+
+  const collectSubtree = (rootPid: number, into: Set<number>) => {
+    const stack = [rootPid];
+    while (stack.length > 0) {
+      const pid = stack.pop();
+      if (pid === undefined || into.has(pid)) continue;
+      into.add(pid);
+      for (const child of childrenByParent.get(pid) ?? []) {
+        stack.push(child.pid);
+      }
+    }
+  };
+
+  const shielded = new Set<number>();
+  const serverStack = [...(childrenByParent.get(serverPid) ?? [])];
+  const visited = new Set<number>();
+  while (serverStack.length > 0) {
+    const row = serverStack.pop();
+    if (!row || visited.has(row.pid)) continue;
+    visited.add(row.pid);
+    if (PROVIDER_SESSION_COMMAND_PATTERN.test(row.command)) {
+      collectSubtree(row.pid, shielded);
+      continue;
+    }
+    for (const child of childrenByParent.get(row.pid) ?? []) {
+      serverStack.push(child);
+    }
+  }
+  return shielded;
+}
+
 export function resolveBackgroundRunsFromListeningPorts(input: {
   readonly urls: ReadonlyArray<string>;
   readonly pids?: ReadonlyArray<number> | undefined;
@@ -1299,8 +1354,11 @@ export function resolveBackgroundRunsFromListeningPorts(input: {
   const serverDescendantPids = new Set(
     buildDescendantEntries(input.processRows ?? [], serverPid).map((entry) => entry.pid),
   );
+  const shieldedPids = providerSessionShieldedPids(input.processRows ?? [], serverPid);
   const explicitProcessOnlyRows = uniquePositivePids(input.pids ?? [])
-    .filter((pid) => !portRunPids.has(pid) && serverDescendantPids.has(pid))
+    .filter(
+      (pid) => !portRunPids.has(pid) && serverDescendantPids.has(pid) && !shieldedPids.has(pid),
+    )
     .flatMap((pid) => {
       const row = processRowsByPid.get(pid);
       return row ? [row] : [];
@@ -1310,7 +1368,7 @@ export function resolveBackgroundRunsFromListeningPorts(input: {
       ? selectHintedProcessOnlyRows({
           processRows: input.processRows ?? [],
           commandHints: input.commandHints ?? [],
-        })
+        }).filter((row) => !shieldedPids.has(row.pid))
       : [];
   const processOnlyRows = [
     ...new Map(

apps/server/src/orchestration/subagentProjection.test.ts

@@ -259,6 +259,52 @@ describe("projectSubagentActivity", () => {
     expect(settled[0]?.status).toBe("interrupted");
   });
 
+  it("coalesces a pending spawn with an id-keyed row instead of duplicating", () => {
+    const codexItem = (item: Record<string, unknown>): OrchestrationThreadActivity =>
+      activity({
+        id: `codex-${String(item.id)}-${String(item.status)}`,
+        kind: "tool.updated",
+        turnId: TURN_ID,
+        payload: { itemType: "collab_agent_tool_call", data: { item } },
+      });
+
+    // Spawn starts before the provider names the agent: a pending placeholder.
+    const pending = projectSubagentActivity(
+      [],
+      codexItem({ id: "call-1", tool: "spawnAgent", status: "inProgress", prompt: "Review" }),
+    );
+    expect(pending).toHaveLength(1);
+    expect(pending[0]?.id).toBe("pending:call-1");
+
+    // A wait item reveals the agent id first, as its own row.
+    const revealed = projectSubagentActivity(
+      pending,
+      codexItem({
+        id: "call-2",
+        tool: "wait",
+        status: "inProgress",
+        receiverThreadIds: ["agent-x"],
+      }),
+    );
+    expect(revealed).toHaveLength(2);
+
+    // The spawn completion carries both keys. The two rows are the same agent;
+    // leaving both behind would violate the roster table's unique constraints.
+    const settled = projectSubagentActivity(
+      revealed,
+      codexItem({
+        id: "call-1",
+        tool: "spawnAgent",
+        status: "completed",
+        agentThreadId: "agent-x",
+      }),
+    );
+    expect(settled).toHaveLength(1);
+    expect(settled[0]?.agentThreadId).toBe("agent-x");
+    expect(settled[0]?.spawnCallId).toBe("call-1");
+    expect(settled[0]?.objective).toBe("Review");
+  });
+
   it("still folds Codex-shaped collab items", () => {
     const roster = projectSubagentActivity(
       [],

apps/server/src/orchestration/subagentProjection.ts

@@ -326,15 +326,70 @@ export function projectSubagentActivity(
   if (patches.length === 0) return current;
   const next = [...current];
   for (const patch of patches) {
-    const index = next.findIndex(
-      (entry) =>
-        (patch.agentThreadId !== null && patch.agentThreadId === entry.agentThreadId) ||
-        (patch.spawnCallId !== null && patch.spawnCallId === entry.spawnCallId) ||
-        patch.id === entry.id,
-    );
-    const merged = mergeSubagent(index >= 0 ? next[index] : undefined, patch, activity);
-    if (index >= 0) next[index] = merged;
+    // A patch can match more than one row: a spawn that started as a
+    // `pending:` placeholder and an id-keyed row learned from a later item
+    // are the same agent once a patch carries both keys. All matches merge
+    // into one row — the persisted table is unique on id, agentThreadId and
+    // spawnCallId per thread, so leaving both rows behind is not a cosmetic
+    // duplicate but a constraint violation that fails the write.
+    const matches: number[] = [];
+    for (let index = 0; index < next.length; index += 1) {
+      const entry = next[index];
+      if (
+        entry &&
+        ((patch.agentThreadId !== null && patch.agentThreadId === entry.agentThreadId) ||
+          (patch.spawnCallId !== null && patch.spawnCallId === entry.spawnCallId) ||
+          patch.id === entry.id)
+      ) {
+        matches.push(index);
+      }
+    }
+    const [primary, ...absorbed] = matches;
+    let base = primary !== undefined ? next[primary] : undefined;
+    for (const index of absorbed) {
+      const duplicate = next[index];
+      if (base && duplicate) {
+        base = mergeSubagent(
+          base,
+          { ...duplicatePatchFrom(duplicate), id: duplicate.id },
+          activity,
+        );
+      }
+    }
+    const merged = mergeSubagent(base, patch, activity);
+    if (primary !== undefined) next[primary] = merged;
     else next.push(merged);
+    for (let cursor = absorbed.length - 1; cursor >= 0; cursor -= 1) {
+      const index = absorbed[cursor];
+      if (index !== undefined) next.splice(index, 1);
+    }
   }
   return next.toSorted((left, right) => left.createdAt.localeCompare(right.createdAt));
 }
+
+/** Reshapes an absorbed duplicate row into a patch so its learned fields fold
+ *  into the surviving row through the same merge path patches use. */
+function duplicatePatchFrom(duplicate: OrchestrationSubagent): SubagentPatch {
+  return {
+    id: duplicate.id,
+    agentThreadId: duplicate.agentThreadId,
+    parentAgentThreadId: duplicate.parentAgentThreadId,
+    spawnCallId: duplicate.spawnCallId,
+    transcriptAgentId: duplicate.transcriptAgentId,
+    turnId: duplicate.turnId,
+    agentPath: duplicate.agentPath,
+    parentAgentPath: duplicate.parentAgentPath,
+    treeDepth: duplicate.treeDepth,
+    nickname: duplicate.nickname,
+    role: duplicate.role,
+    objective: duplicate.objective,
+    status: duplicate.status,
+    requestedModel: duplicate.requestedModel,
+    resolvedModel: duplicate.resolvedModel,
+    reasoningEffort: duplicate.reasoningEffort,
+    modelProvenance: duplicate.modelProvenance,
+    reasoningEffortProvenance: duplicate.reasoningEffortProvenance,
+    resultBody: duplicate.resultBody,
+    resultCreatedAt: duplicate.resultCreatedAt,
+  };
+}