fix: 0.28.0 QA fixes — strict gates under the mod, clean port-in-use error, deferred pi discovery, Windows PATH

apps/hook/hooks/mod/controller.test.ts

@@ -278,6 +278,35 @@ describe('commands', () => {
     expect(host.store.get(STORE_LAUNCHES)).toEqual([])
   })
 
+  test('annotate refuses strict-gate and hook flags instead of launching a session whose decision nothing reads', async () => {
+    for (const [words, flag] of [
+      ['notes.md --gate --json --require-approval', '--require-approval'],
+      ['notes.md --gate --json --result-file out.json', '--result-file'],
+      ['notes.md --hook', '--hook'],
+    ] as const) {
+      const host = fakeHost()
+      serveOnLaunch(host)
+      const mod = new PlannotatorMod(host, SESSION)
+
+      const text = await mod.runCommand('annotate', words)
+
+      expect(text).toContain(flag)
+      expect(text).toContain('in a terminal')
+      expect(launches(host)).toEqual([])
+      expect(host.store.get(STORE_LAUNCHES)).toBeUndefined()
+    }
+  })
+
+  test('a word that merely mentions a strict flag inside a quoted path still opens', async () => {
+    const host = fakeHost()
+    serveOnLaunch(host)
+    const mod = new PlannotatorMod(host, SESSION)
+
+    await mod.runCommand('annotate', '"notes --hook.md" --gate')
+
+    expect(launches(host)[0]?.argv.slice(5)).toEqual(['plannotator', 'annotate', 'notes --hook.md', '--gate'])
+  })
+
   test('last sends the last assistant message on stdin', async () => {
     const host = fakeHost()
     serveOnLaunch(host)

apps/hook/hooks/mod/controller.ts

@@ -25,6 +25,7 @@ import {
   RECENT_MESSAGES_SUBJECT,
   recentAssistantTexts,
   subjectFor,
+  wordsOf,
 } from './launch'
 import {
   approvedPermissionDecision,
@@ -44,7 +45,13 @@ import {
   type OpenPlanReview,
   type PendingApproval,
 } from './plan'
-import { parsePlannotatorToolInput, plannotatorToolArgs, plannotatorToolOpenedText } from './tool'
+import {
+  parsePlannotatorToolInput,
+  plannotatorToolArgs,
+  plannotatorToolOpenedText,
+  scriptOnlyAnnotateFlag,
+  scriptOnlyAnnotateFlagText,
+} from './tool'
 import { TurnTracker } from './turns'
 
 /** Persisted in `$.store` so open reviews reattach after a restart or `--resume`. */
@@ -368,6 +375,12 @@ export class PlannotatorMod {
     | { state: 'starting'; subject: string }
     | { state: 'ready'; subject: string; url: string; extra?: string }
   > {
+    if (kind === 'annotate') {
+      // Strict gates and --hook answer on the CLI's exit code, stdout or result
+      // file, which nothing reads under a detached launch: refuse up front.
+      const flag = scriptOnlyAnnotateFlag(wordsOf(args))
+      if (flag) return { state: 'error', text: scriptOnlyAnnotateFlagText(flag) }
+    }
     let stdin = ''
     let extra: string | undefined
     const side: { messages?: string } = {}

apps/hook/hooks/mod/tool.ts

@@ -261,6 +261,32 @@ export function simpleShellCommandWords(command: string): string[] | null {
   return words
 }
 
+/**
+ * Annotate flags for a script that reads the CLI's own channels: the exit
+ * code and stdout record of a strict gate (`--require-approval`,
+ * `--result-file`) or the hook-shaped stdout (`--hook`). A host that starts
+ * the CLI detached and delivers the decision later as a message has no caller
+ * reading any of them, so the reviewer's decision would be lost. Such a host
+ * refuses annotate words carrying one (`scriptOnlyAnnotateFlag`), and the
+ * shell take-over below leaves a command carrying one to run as written.
+ */
+export const SCRIPT_ONLY_ANNOTATE_FLAGS: readonly string[] = ['--require-approval', '--result-file', '--hook']
+
+/** The first script-only annotate flag among the words, or null. */
+export function scriptOnlyAnnotateFlag(words: readonly string[]): string | null {
+  return words.find((word) => SCRIPT_ONLY_ANNOTATE_FLAGS.includes(word)) ?? null
+}
+
+/** What a detached host answers when the user's annotate words carry a script-only flag. */
+export function scriptOnlyAnnotateFlagText(flag: string): string {
+  return (
+    `Plannotator did not open: ${flag} is for scripts that read the CLI's exit code or result file, ` +
+    'and here your decision comes back as a message instead, so nothing would read it. ' +
+    `Run \`plannotator annotate <file> --gate --json ${flag === '--result-file' ? '--result-file <path>' : flag}\` in a terminal, ` +
+    `or drop ${SCRIPT_ONLY_ANNOTATE_FLAGS.join(' / ')} to annotate here.`
+  )
+}
+
 const COMMAND_ACTIONS: Record<string, PlannotatorToolAction> = {
   annotate: 'annotate',
   review: 'review',
@@ -306,6 +332,7 @@ export function plannotatorCommandToToolInput(command: string): PlannotatorToolI
   const call: Record<string, unknown> = { action }
   const options: Record<string, unknown> = {}
   const rest = words.slice(2)
+  if (scriptOnlyAnnotateFlag(rest)) return null
   for (let index = 0; index < rest.length; index += 1) {
     const word = rest[index] as string
     if (!word.startsWith('-')) {

apps/hook/server/annotate-cli.test.ts

@@ -251,3 +251,52 @@ describe("plannotator annotate: live app remote hard-off (CLI layer)", () => {
     }
   });
 });
+
+describe("annotate CLI on an occupied fixed port", () => {
+  // Remote mode binds one fixed port, so a second concurrent session finds it
+  // taken. That is a startup failure with one clean line, never a stack trace.
+  async function runOnOccupiedPort(extraArgs: string[]) {
+    const blocker = Bun.serve({ hostname: "0.0.0.0", port: 0, fetch: () => new Response("busy") });
+    try {
+      const child = Bun.spawn([process.execPath, cliEntry, "annotate", "notes.md", ...extraArgs], {
+        cwd: fixtureDir,
+        env: {
+          ...process.env,
+          PLANNOTATOR_CWD: fixtureDir,
+          PLANNOTATOR_DATA_DIR: dataDir,
+          PLANNOTATOR_REMOTE: "1",
+          PLANNOTATOR_PORT: String(blocker.port),
+          PLANNOTATOR_BROWSER: "none",
+        },
+        stdout: "pipe",
+        stderr: "pipe",
+      });
+      const exitCode = await child.exited;
+      return {
+        port: blocker.port,
+        exitCode,
+        stdout: await new Response(child.stdout).text(),
+        stderr: await new Response(child.stderr).text(),
+      };
+    } finally {
+      blocker.stop(true);
+    }
+  }
+
+  test("exits 1 with one line naming the port and PLANNOTATOR_PORT, no stack", async () => {
+    const result = await runOnOccupiedPort([]);
+    expect(result.exitCode).toBe(1);
+    expect(result.stderr).toContain(`port ${result.port} (PLANNOTATOR_PORT) is already in use`);
+    // Only the usual "Resolved:" note precedes it; the failure itself is one line.
+    expect(result.stderr.trim().split("\n").filter((line) => !line.startsWith("Resolved:"))).toHaveLength(1);
+    expect(result.stderr).not.toMatch(/\n\s+at /);
+    expect(result.stdout).toBe("");
+  }, 20_000);
+
+  test("is a gate startup failure (exit 2) under a strict annotate gate", async () => {
+    const result = await runOnOccupiedPort(["--gate", "--json", "--require-approval"]);
+    expect(result.exitCode).toBe(2);
+    expect(result.stderr).toContain("is already in use");
+    expect(result.stdout).toBe("");
+  }, 20_000);
+});

apps/hook/server/index.ts

@@ -77,21 +77,21 @@
  */
 
 import {
-  startPlannotatorServer,
+  startPlannotatorServer as startPlannotatorServerUnguarded,
   handleServerReady,
 } from "@plannotator/server";
 import {
-  startReviewServer,
+  startReviewServer as startReviewServerUnguarded,
   handleReviewServerReady,
 } from "@plannotator/server/review";
 import { runGuideCli } from "@plannotator/server/guide-cli";
 import {
-  startAnnotateServer,
+  startAnnotateServer as startAnnotateServerUnguarded,
   handleAnnotateServerReady,
   isRemoteSession,
 } from "@plannotator/server/annotate";
 import {
-  startGoalSetupServer,
+  startGoalSetupServer as startGoalSetupServerUnguarded,
   handleGoalSetupServerReady,
 } from "@plannotator/server/goal-setup";
 import { type DiffType, detectManagedVcs, prepareLocalReviewDiff, gitRuntime } from "@plannotator/server/vcs";
@@ -124,6 +124,7 @@ import {
   takeHostResultPath,
 } from "./host-result";
 import { readHostMessages, takeHostMessagesPath } from "./host-messages";
+import { exitOnPortInUse } from "./port-in-use";
 import { parseModPlanInput, readModPlanRevision, writeModPlanRevisionAck } from "./claude-mod-plan";
 import { LIVE_APP_REMOTE_MESSAGE } from "@plannotator/shared/live-probe";
 // Bridge sources for live app sessions: the CLI supplies them so
@@ -261,6 +262,18 @@ const resultFile = parsedStrictAnnotateOptions.resultFile
     )
   : undefined;
 
+// An occupied port (remote mode's fixed 19432, or PLANNOTATOR_PORT) is a
+// startup failure: one clean stderr line, no stack. Exit 2 under a strict
+// annotate gate, 1 otherwise, like every other startup failure.
+function exitPortInUse(message: string): never {
+  process.stderr.write(`${message}\n`);
+  process.exit(annotateStartupFailureExitCode({ requireApproval: requireApprovalFlag, resultFile }));
+}
+const startPlannotatorServer = exitOnPortInUse(startPlannotatorServerUnguarded, exitPortInUse);
+const startReviewServer = exitOnPortInUse(startReviewServerUnguarded, exitPortInUse);
+const startAnnotateServer = exitOnPortInUse(startAnnotateServerUnguarded, exitPortInUse);
+const startGoalSetupServer = exitOnPortInUse(startGoalSetupServerUnguarded, exitPortInUse);
+
 // Global flag: --browser <name>
 const browserIdx = args.indexOf("--browser");
 if (browserIdx !== -1 && args[browserIdx + 1]) {

apps/hook/server/port-in-use.ts

@@ -0,0 +1,22 @@
+import { PortInUseError } from "@plannotator/server/remote";
+
+/**
+ * Wrap a server start so an occupied port ends the CLI with one clean line
+ * instead of an uncaught error and its stack trace. In remote mode the port is
+ * fixed (19432 unless PLANNOTATOR_PORT says otherwise), so a second concurrent
+ * session — easy to hit now that the Claude Code mod's servers outlive the
+ * terminal — lands here. Any other error is rethrown untouched.
+ */
+export function exitOnPortInUse<A extends unknown[], R>(
+  start: (...args: A) => Promise<R>,
+  exit: (message: string) => never,
+): (...args: A) => Promise<R> {
+  return async (...args: A) => {
+    try {
+      return await start(...args);
+    } catch (error) {
+      if (error instanceof PortInUseError) exit(error.cliMessage);
+      throw error;
+    }
+  };
+}

apps/pi-extension/server/ai-runtime.test.ts

@@ -152,6 +152,71 @@ describe("createPiAIRuntime Codex discovery", () => {
 	}, 15_000);
 });
 
+describe("createPiAIRuntime Pi discovery", () => {
+	// Pi's model discovery spawns `pi` and waits up to 10s; it must not hold a
+	// plain capabilities answer, only an explicit ?activate= (or a session).
+	test("plain capabilities answers at once without spawning a slow pi; ?activate= spawns it", async () => {
+		if (process.platform === "win32") return;
+
+		const dir = mkdtempSync(join(tmpdir(), "plannotator-pi-lazy-pi-"));
+		tempDirs.push(dir);
+		const marker = join(dir, "pi-ran");
+		const pi = join(dir, "pi");
+		writeFileSync(pi, `#!/bin/sh\necho ran >> '${marker}'\nsleep 5\n`);
+		chmodSync(pi, 0o755);
+
+		const runner = join(dir, "runner.ts");
+		const runtimeUrl = pathToFileURL(join(import.meta.dir, "ai-runtime.ts")).href;
+		writeFileSync(runner, `
+			import { existsSync } from "node:fs";
+			import { createPiAIRuntime } from ${JSON.stringify(runtimeUrl)};
+			const runtime = await createPiAIRuntime({ cwd: ${JSON.stringify(dir)} });
+			if (!runtime) throw new Error("Pi AI runtime unavailable");
+			const startedAt = Date.now();
+			const capabilities = await runtime.endpoints["/api/ai/capabilities"](
+				new Request("http://localhost/api/ai/capabilities"),
+			);
+			const elapsedMs = Date.now() - startedAt;
+			const data = await capabilities.json();
+			const afterCapabilities = existsSync(${JSON.stringify(marker)});
+			void runtime.endpoints["/api/ai/capabilities"](
+				new Request("http://localhost/api/ai/capabilities?activate=pi-sdk"),
+			);
+			let afterActivate = false;
+			for (let i = 0; i < 60 && !afterActivate; i++) {
+				await new Promise((resolve) => setTimeout(resolve, 50));
+				afterActivate = existsSync(${JSON.stringify(marker)});
+			}
+			console.log(JSON.stringify({
+				hasPi: data.providers.some((provider) => provider.id === "pi-sdk"),
+				fast: elapsedMs < 2000,
+				afterCapabilities,
+				afterActivate,
+			}));
+			process.exit(0);
+		`);
+
+		const proc = Bun.spawn([process.execPath, runner], {
+			cwd: import.meta.dir,
+			env: { ...process.env, PATH: `${dir}:/usr/bin:/bin` },
+			stdout: "pipe",
+			stderr: "pipe",
+		});
+		const [stdout, stderr, exitCode] = await Promise.all([
+			new Response(proc.stdout).text(),
+			new Response(proc.stderr).text(),
+			proc.exited,
+		]);
+		expect(exitCode, stderr).toBe(0);
+		expect(JSON.parse(stdout.trim())).toEqual({
+			hasPi: true,
+			fast: true,
+			afterCapabilities: false,
+			afterActivate: true,
+		});
+	}, 15_000);
+});
+
 describe("createPiAIRuntime with a session bridge", () => {
 	// The Pi session that opened Plannotator answers Ask AI and nothing else:
 	// with a bridge no SDK provider is listed or reachable by id, even with

apps/pi-extension/server/ai-runtime.ts

@@ -58,7 +58,6 @@ export async function createPiAIRuntime(options: CreatePiAIRuntimeOptions = {}):
 		const cwd = options.cwd ?? process.cwd();
 		const registry = new ai.ProviderRegistry();
 		const sessionManager = new ai.SessionManager();
-		const modelDiscovery: Promise<void>[] = [];
 		// Model discovery spawns the provider's CLI, so it runs on first explicit
 		// activation (?activate= from a model picker) or the first session — never
 		// at startup.
@@ -107,14 +106,12 @@ export async function createPiAIRuntime(options: CreatePiAIRuntimeOptions = {}):
 						cwd,
 						piExecutablePath: piPath,
 					} as any);
-					if (provider && "fetchModels" in provider) {
-						modelDiscovery.push(
-							(provider as { fetchModels: () => Promise<void> })
-								.fetchModels()
-								.catch(() => {}),
-						);
-					}
-					registry.register(provider);
+					const providerId = registry.register(provider);
+					// Deferred like Codex: fetchModels spawns `pi` (up to 10s), and
+					// done eagerly it held every plain /api/ai/capabilities answer
+					// until it finished. A Pi session spawns its own `pi` and runs on
+					// Pi's default model when none is picked, so it never waits either.
+					deferModelDiscovery(providerId, provider, { blockSession: false });
 				}
 			} catch {
 				// Pi not available.
@@ -165,9 +162,6 @@ export async function createPiAIRuntime(options: CreatePiAIRuntimeOptions = {}):
 				...(catalogRegistry ? { catalogRegistry } : {}),
 				sessionManager,
 				getCwd: options.getCwd,
-				beforeCapabilities: async () => {
-					await Promise.allSettled(modelDiscovery);
-				},
 				beforeProviderSession: discovery.beforeProviderSession,
 				authorizeSessionBridgeRequest: (req: Request) =>
 					isLoopbackHostHeader(req.headers.get("host"), options.getServerPort?.()),