fix: deep-review hardening — auth probes, session-id, signal-kill, concurrency

.gitignore

@@ -11,3 +11,6 @@ dist/
 
 # Claude Code worktree state (agent isolation runs)
 .claude/
+
+# CodeGraph local index (not committed)
+.codegraph/

packages/polycli-runtime/src/claude.js

@@ -9,6 +9,10 @@ const CLAUDE_BIN = process.env.CLAUDE_CLI_BIN || "claude";
 const DEFAULT_TIMEOUT_MS = 900_000;
 const AUTH_CHECK_TIMEOUT_MS = 30_000;
 const PROMPT_STDIN_THRESHOLD = 100_000;
+const CLAUDE_EXPLICIT_AUTH_ERROR_RE = /\b(unauthenticated|unauthorized|not authenticated|not authorized|login required|log in|sign in|invalid api key|missing api key|api key required|token expired|invalid token|credential(?:s)? (?:missing|invalid|expired)|permission denied|access denied|forbidden|401|403)\b/i;
+export const TRANSIENT_PROBE_ERROR_PATTERNS = [
+  /\b(timed out|timeout|429|rate limit|no capacity available|temporar(?:y|ily)|service unavailable|overloaded|try again|econnreset|econnrefused|enotfound|network|socket hang up)\b/i,
+];
 
 function collectTextFromContent(content) {
   if (typeof content === "string") {
@@ -213,22 +217,31 @@ export function getClaudeAvailability(cwd) {
   return binaryAvailable(CLAUDE_BIN, ["--version"], { cwd });
 }
 
-export function getClaudeAuthStatus(cwd) {
-  const result = runClaudePrompt({
+export function getClaudeAuthStatus(cwd, { promptRunner = runClaudePrompt } = {}) {
+  const result = promptRunner({
     prompt: "ping",
     cwd,
     timeout: AUTH_CHECK_TIMEOUT_MS,
   });
 
-  if (!result.ok) {
-    return { loggedIn: false, detail: result.error };
+  if (result.ok) {
+    return {
+      loggedIn: true,
+      detail: "authenticated",
+      model: result.model ?? null,
+    };
   }
 
-  return {
-    loggedIn: true,
-    detail: "authenticated",
-    model: null,
-  };
+  // A timeout / 429 / transient probe failure must NOT regress to loggedIn:false
+  // (the probe is inconclusive, not proof of logout).
+  const detail = String(result.error ?? "").trim() || "claude auth probe failed";
+  if (CLAUDE_EXPLICIT_AUTH_ERROR_RE.test(detail)) {
+    return { loggedIn: false, detail };
+  }
+  if (TRANSIENT_PROBE_ERROR_PATTERNS.some((pattern) => pattern.test(detail))) {
+    return { loggedIn: true, detail: `auth probe inconclusive: ${detail}`, model: result.model ?? null };
+  }
+  return { loggedIn: false, detail };
 }
 
 export function runClaudePrompt({

packages/polycli-runtime/src/cmd.js

@@ -1,5 +1,4 @@
 import { binaryAvailable, runCommand } from "@bbingz/polycli-utils/process";
-import { resolveSessionId } from "@bbingz/polycli-utils/session-id";
 
 import { classifyProviderFailure, formatProviderExitError } from "./errors.js";
 import { spawnStreamingCommand } from "./spawn.js";
@@ -124,11 +123,6 @@ export function runCmdPrompt({
   }
 
   const parsed = parseCmdTextResult(result.stdout);
-  const resolvedSession = resolveSessionId({
-    stdout: result.stdout,
-    stderr: result.stderr,
-    priority: ["stdout", "stderr", "file"],
-  });
   const hasVisibleText = Boolean(parsed.response.trim());
 
   const error = result.status === 0
@@ -138,7 +132,9 @@ export function runCmdPrompt({
     ok: result.status === 0 && hasVisibleText,
     response: parsed.response,
     events: parsed.events,
-    sessionId: resolvedSession.sessionId,
+    // cmd stdout is pure assistant prose with no session-id field; never scan it for a
+    // UUID, which would fabricate a sessionId from any UUID in the answer (cf. agy v0.6.18).
+    sessionId: null,
     model: model ?? defaultModel ?? DEFAULT_CMD_MODEL,
     error,
     errorCode: classifyProviderFailure(error, { provider: "cmd" }),
@@ -180,19 +176,16 @@ export function runCmdPromptStreaming({
     },
   }).then((result) => {
     const parsed = parseCmdTextResult(result.stdout);
-    const resolvedSession = resolveSessionId({
-      stdout: result.stdout,
-      stderr: result.stderr,
-      priority: ["stdout", "stderr", "file"],
-    });
     const hasVisibleText = Boolean(parsed.response.trim());
     const error = result.ok
       ? (hasVisibleText ? null : "cmd produced no visible text")
       : result.error;
     return {
       ...result,
       ...parsed,
-      sessionId: resolvedSession.sessionId,
+      // cmd stdout is pure assistant prose with no session-id field; never scan it for a
+      // UUID, which would fabricate a sessionId from any UUID in the answer (cf. agy v0.6.18).
+      sessionId: null,
       model: model ?? defaultModel ?? DEFAULT_CMD_MODEL,
       ok: result.ok && hasVisibleText,
       error,

packages/polycli-runtime/src/copilot.js

@@ -7,6 +7,10 @@ import { spawnStreamingCommand } from "./spawn.js";
 const COPILOT_BIN = process.env.COPILOT_CLI_BIN || "copilot";
 const DEFAULT_TIMEOUT_MS = 900_000;
 const AUTH_CHECK_TIMEOUT_MS = 30_000;
+const COPILOT_EXPLICIT_AUTH_ERROR_RE = /\b(unauthenticated|unauthorized|not authenticated|not authorized|login required|log in|sign in|invalid api key|missing api key|api key required|token expired|invalid token|credential(?:s)? (?:missing|invalid|expired)|permission denied|access denied|forbidden|401|403)\b/i;
+export const TRANSIENT_PROBE_ERROR_PATTERNS = [
+  /\b(timed out|timeout|429|rate limit|no capacity available|temporar(?:y|ily)|service unavailable|overloaded|try again|econnreset|econnrefused|enotfound|network|socket hang up)\b/i,
+];
 
 function collectCopilotContentText(content) {
   if (typeof content === "string") {
@@ -179,22 +183,30 @@ export function getCopilotAvailability(cwd) {
   return binaryAvailable(COPILOT_BIN, ["--version"], { cwd });
 }
 
-export function getCopilotAuthStatus(cwd) {
-  const result = runCopilotPrompt({
+export function getCopilotAuthStatus(cwd, { promptRunner = runCopilotPrompt } = {}) {
+  const result = promptRunner({
     prompt: "ping",
     cwd,
     timeout: AUTH_CHECK_TIMEOUT_MS,
   });
 
-  if (!result.ok) {
-    return { loggedIn: false, detail: result.error };
+  if (result.ok) {
+    return {
+      loggedIn: true,
+      detail: "authenticated",
+      model: result.model ?? null,
+    };
   }
 
-  return {
-    loggedIn: true,
-    detail: "authenticated",
-    model: result.model ?? null,
-  };
+  // A timeout / 429 / transient probe failure must NOT regress to loggedIn:false.
+  const detail = String(result.error ?? "").trim() || "copilot auth probe failed";
+  if (COPILOT_EXPLICIT_AUTH_ERROR_RE.test(detail)) {
+    return { loggedIn: false, detail };
+  }
+  if (TRANSIENT_PROBE_ERROR_PATTERNS.some((pattern) => pattern.test(detail))) {
+    return { loggedIn: true, detail: `auth probe inconclusive: ${detail}`, model: result.model ?? null };
+  }
+  return { loggedIn: false, detail };
 }
 
 export function runCopilotPrompt({

packages/polycli-runtime/src/gemini.js

@@ -122,11 +122,6 @@ function parseGeminiJsonResult(stdout, stderr, status, { defaultModel = null } =
 
   try {
     const parsed = JSON.parse(text.slice(jsonStart));
-    const resolvedSession = resolveSessionId({
-      stdout,
-      stderr,
-      priority: ["stdout", "stderr", "file"],
-    });
     if (parsed.error) {
       return {
         ok: false,
@@ -135,6 +130,21 @@ function parseGeminiJsonResult(stdout, stderr, status, { defaultModel = null } =
         status,
       };
     }
+    if (status !== 0) {
+      return {
+        ok: false,
+        error: String(stderr ?? "").trim() || formatProviderExitError("gemini", status),
+        status,
+      };
+    }
+    // Session id is the structured parsed.session_id; the stderr/file fallback is allowed
+    // (gemini may print an id to stderr) but stdout is blanked so a UUID inside the answer
+    // prose (which lives in the same JSON) can never be promoted to a fabricated sessionId.
+    const resolvedSession = resolveSessionId({
+      stdout: "",
+      stderr,
+      priority: ["stdout", "stderr", "file"],
+    });
     return {
       ok: true,
       response: parsed.response ?? "",
@@ -270,7 +280,7 @@ export function runGeminiPromptStreaming({
   }).then((result) => {
     const parsed = parseGeminiStreamText(result.stdout);
     const resolvedSession = resolveSessionId({
-      stdout: result.stdout,
+      stdout: "",
       stderr: result.stderr,
       priority: ["stdout", "stderr", "file"],
     });
@@ -281,7 +291,8 @@ export function runGeminiPromptStreaming({
     return {
       ...result,
       ...parsed,
-      sessionId: parsed.sessionId ?? resolvedSession.sessionId,
+      // stdout blanked so a UUID in the answer prose is never promoted to a fabricated id.
+      sessionId: parsed.sessionId ?? resolvedSession.sessionId ?? null,
       model: parsed.model ?? model ?? defaultModel,
       ok: result.ok && !resultError && hasVisibleText,
       error: result.ok

packages/polycli-runtime/src/minimax.js

@@ -5,6 +5,10 @@ import { spawnStreamingCommand } from "./spawn.js";
 const MMX_BIN = process.env.MMX_CLI_BIN || process.env.MINIMAX_CLI_BIN || "mmx";
 const DEFAULT_TIMEOUT_MS = 120_000;
 const AUTH_CHECK_TIMEOUT_MS = 30_000;
+const MINIMAX_EXPLICIT_AUTH_ERROR_RE = /\b(unauthenticated|unauthorized|not authenticated|not authorized|login required|log in|sign in|invalid api key|missing api key|api key required|token expired|invalid token|credential(?:s)? (?:missing|invalid|expired)|permission denied|access denied|forbidden|401|403)\b/i;
+export const TRANSIENT_PROBE_ERROR_PATTERNS = [
+  /\b(timed out|timeout|429|rate limit|no capacity available|temporar(?:y|ily)|service unavailable|overloaded|try again|econnreset|econnrefused|enotfound|network|socket hang up)\b/i,
+];
 
 export function stripAnsiSgr(text) {
   return String(text ?? "").replace(/\x1b\[[0-9;]*m/g, "");
@@ -136,17 +140,30 @@ export function getMiniMaxAvailability(cwd) {
   return binaryAvailable(MMX_BIN, ["--version"], { cwd });
 }
 
-export async function getMiniMaxAuthStatus(cwd) {
-  const result = runCommand(MMX_BIN, ["auth", "status", "--output", "json", "--non-interactive"], {
+export async function getMiniMaxAuthStatus(cwd, { runner = runCommand } = {}) {
+  const result = runner(MMX_BIN, ["auth", "status", "--output", "json", "--non-interactive"], {
     cwd,
     timeout: AUTH_CHECK_TIMEOUT_MS,
   });
 
+  // A timeout / 429 / transient failure of the auth-status subcommand is inconclusive,
+  // not proof of logout — it must NOT regress to loggedIn:false.
   if (result.error) {
-    return { loggedIn: false, detail: result.error.message };
+    const detail = result.error.code === "ETIMEDOUT"
+      ? `mmx auth probe timed out after ${Math.round(AUTH_CHECK_TIMEOUT_MS / 1000)}s`
+      : result.error.message;
+    if (TRANSIENT_PROBE_ERROR_PATTERNS.some((pattern) => pattern.test(detail))) {
+      return { loggedIn: true, detail: `auth probe inconclusive: ${detail}`, model: null };
+    }
+    return { loggedIn: false, detail };
   }
   if (result.status !== 0) {
-    return { loggedIn: false, detail: result.stderr.trim() || `mmx auth status exited with code ${result.status}` };
+    const detail = result.stderr.trim() || `mmx auth status exited with code ${result.status}`;
+    if (!MINIMAX_EXPLICIT_AUTH_ERROR_RE.test(detail)
+      && TRANSIENT_PROBE_ERROR_PATTERNS.some((pattern) => pattern.test(detail))) {
+      return { loggedIn: true, detail: `auth probe inconclusive: ${detail}`, model: null };
+    }
+    return { loggedIn: false, detail };
   }
 
   const text = `${result.stdout ?? ""}\n${result.stderr ?? ""}`.trim();

packages/polycli-runtime/src/pi.js

@@ -232,7 +232,7 @@ export function runPiPrompt({
 
   const parsed = parsePiStreamText(result.stdout);
   const resolvedSession = resolveSessionId({
-    stdout: result.stdout,
+    stdout: "",
     stderr: result.stderr,
     priority: ["stdout", "stderr", "file"],
   });
@@ -246,7 +246,9 @@ export function runPiPrompt({
     ok: result.status === 0 && !resultError && !providerError && hasVisibleText,
     response: parsed.response,
     events: parsed.events,
-    sessionId: parsed.sessionId ?? resolvedSession.sessionId,
+    // pi's session id comes from its structured `session` event; stdout is blanked so a UUID
+    // in the answer prose can never be promoted to a fabricated id (stderr/file still allowed).
+    sessionId: parsed.sessionId ?? resolvedSession.sessionId ?? null,
     model: parsed.model ?? model ?? defaultModel ?? DEFAULT_PI_MODEL,
     error: result.status === 0
       ? (resultError || providerError || (hasVisibleText ? null : "pi produced no visible text"))
@@ -297,7 +299,7 @@ export function runPiPromptStreaming({
   }).then((result) => {
     const parsed = parsePiStreamText(result.stdout);
     const resolvedSession = resolveSessionId({
-      stdout: result.stdout,
+      stdout: "",
       stderr: result.stderr,
       priority: ["stdout", "stderr", "file"],
     });
@@ -309,7 +311,8 @@ export function runPiPromptStreaming({
     return {
       ...result,
       ...parsed,
-      sessionId: parsed.sessionId ?? resolvedSession.sessionId,
+      // stdout blanked so a UUID in the answer prose is never promoted to a fabricated id.
+      sessionId: parsed.sessionId ?? resolvedSession.sessionId ?? null,
       model: parsed.model ?? model ?? defaultModel ?? DEFAULT_PI_MODEL,
       ok: result.ok && !resultError && !providerError && hasVisibleText,
       error: result.ok

packages/polycli-runtime/src/qwen.js

@@ -288,7 +288,9 @@ export function runQwenPrompt({
   });
 
   if (result.error) {
-    const error = result.error.message;
+    const error = result.error.code === "ETIMEDOUT"
+      ? `qwen timed out after ${Math.round(timeout / 1000)}s`
+      : result.error.message;
     return { ok: false, error, errorCode: classifyProviderFailure(error, { provider: "qwen" }) };
   }
 

packages/polycli-runtime/test/claude.test.js

@@ -9,6 +9,7 @@ import { loadStreamFixture } from "./helpers/fixture-replay.mjs";
 import {
   buildClaudeInvocation,
   extractClaudeText,
+  getClaudeAuthStatus,
   parseClaudeJsonResult,
   parseClaudeStreamText,
   runClaudePrompt,
@@ -325,3 +326,21 @@ test("parseClaudeStreamText replays a captured real cli fixture", () => {
     "claude ask result must carry a non-empty model"
   );
 });
+
+test("getClaudeAuthStatus keeps loggedIn=true for a transient/timeout probe failure", () => {
+  const auth = getClaudeAuthStatus(process.cwd(), {
+    promptRunner: () => ({ ok: false, error: "claude timed out after 30s" }),
+  });
+
+  assert.equal(auth.loggedIn, true);
+  assert.match(auth.detail, /inconclusive/i);
+});
+
+test("getClaudeAuthStatus reports loggedIn=false only on an explicit auth error", () => {
+  const auth = getClaudeAuthStatus(process.cwd(), {
+    promptRunner: () => ({ ok: false, error: "401 Unauthorized: invalid api key" }),
+  });
+
+  assert.equal(auth.loggedIn, false);
+  assert.match(auth.detail, /unauthorized/i);
+});

packages/polycli-runtime/test/cmd.test.js

@@ -165,3 +165,45 @@ test("runCmdPromptStreaming emits text events for plain stdout lines", async ()
     { type: "text_delta", delta: "world" },
   ]);
 });
+
+test("runCmdPrompt never fabricates a sessionId from a UUID in prose stdout", () => {
+  withFakeCmdBin(
+    `#!/usr/bin/env node
+process.stdout.write("Sure, here is a uuid: 123e4567-e89b-42d3-a456-426614174000\\n");
+`,
+    ({ root, bin }) => {
+      const result = runCmdPrompt({ prompt: "give me a uuid", cwd: root, bin });
+
+      assert.equal(result.ok, true);
+      assert.match(result.response, /123e4567-e89b-42d3-a456-426614174000/);
+      assert.equal(result.sessionId, null);
+    }
+  );
+});
+
+test("runCmdPromptStreaming never fabricates a sessionId from prose", async () => {
+  const child = new EventEmitter();
+  child.stdout = new EventEmitter();
+  child.stderr = new EventEmitter();
+  child.stdin = { write() {}, end() {}, on() {} };
+  child.kill = () => {};
+  child.unref = () => {};
+
+  const result = await runCmdPromptStreaming({
+    prompt: "give me a uuid",
+    cwd: process.cwd(),
+    timeout: 5_000,
+    onEvent() {},
+    spawnImpl() {
+      queueMicrotask(() => {
+        child.stdout.emit("data", "here is one: 123e4567-e89b-42d3-a456-426614174000\n");
+        child.emit("close", 0, null);
+      });
+      return child;
+    },
+  });
+
+  assert.equal(result.ok, true);
+  assert.match(result.response, /123e4567-e89b-42d3-a456-426614174000/);
+  assert.equal(result.sessionId, null);
+});