feat(pi-extension): add remote session support for SSH/devcontainer (#299)

* feat(pi-extension): add remote session support for SSH/devcontainer - Detect remote sessions via PLANNOTATOR_REMOTE env var or SSH_TTY/SSH_CONNECTION - Support PLANNOTATOR_PORT for fixed port (default 19432) in remote sessions - openBrowser() returns { isRemote, url } to notify users to open manually - Extract runBrowserReview helper to consolidate browser-open-wait-stop pattern * fix: handle EADDRINUSE retry and honor browser overrides in remote sessions - listenOnPort now retries up to 5 times on EADDRINUSE (matching packages/server/ canonical pattern) instead of crashing on fixed-port collisions in remote mode - openBrowser honors PLANNOTATOR_BROWSER/BROWSER env vars in remote sessions before early-returning (matching shared-handlers.ts pattern) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add async to server starters, rethrow non-EADDRINUSE errors, catch startup failures - Add missing async keyword to startPlanReviewServer, startReviewServer, startAnnotateServer - Only wrap EADDRINUSE in retry message; rethrow other errors as-is - Catch server startup failures in index.ts and surface via ctx.ui.notify instead of crashing the extension Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Michael Ramos <mdramos8@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-09-14 14:17:26 +08:00 · 2026-03-16 04:12:39 +01:00
parent 789f5eacf6
commit 3bbefa9585
2 changed files with 176 additions and 53 deletions
@@ -28,8 +28,11 @@ import type { ExtensionAPI, ExtensionContext } from "@mariozechner/pi-coding-age
 import { Key } from "@mariozechner/pi-tui";
 import { markCompletedSteps, parseChecklist, type ChecklistItem } from "./utils.js";
 import {
+  type AnnotateServerResult,
  startPlanReviewServer,
+  type PlanServerResult,
  startReviewServer,
+  type ReviewServerResult,
  startAnnotateServer,
  getGitContext,
  runGitDiff,
@@ -37,6 +40,15 @@ import {
 } from "./server.js";
 import { planDenyFeedback } from "./feedback-templates.js";

+// ── Types ──────────────────────────────────────────────────────────────
+
+/** Common interface for servers that can wait for a user decision in a browser. */
+interface DecisionServer<T> {
+  url: string;
+  stop: () => void;
+  waitForDecision: () => Promise<T>;
+}
+
 // Load HTML at runtime (jiti doesn't support import attributes)
 const __dirname = dirname(fileURLToPath(import.meta.url));
 let planHtmlContent = "";
@@ -68,6 +80,29 @@ function getTextContent(message: AssistantMessage): string {
    .join("\n");
 }

+/**
+ * Open browser for user review, wait for decision, then stop server.
+ * Handles remote session notification automatically.
+ */
+async function runBrowserReview<T>(
+  server: DecisionServer<T>,
+  ctx: ExtensionContext,
+): Promise<T> {
+  const browserResult = openBrowser(server.url);
+  if (browserResult.isRemote) {
+    ctx.ui.notify(`Remote session. Open manually: ${browserResult.url}`, "info");
+  }
+
+  const result = await server.waitForDecision();
+  await new Promise((r) => setTimeout(r, 1500));
+  server.stop();
+  return result;
+}
+
+function getStartupErrorMessage(err: unknown): string {
+  return err instanceof Error ? err.message : "Unknown error";
+}
+
 export default function plannotator(pi: ExtensionAPI): void {
  let phase: Phase = "idle";
  let planFilePath = "PLAN.md";
@@ -240,20 +275,22 @@ export default function plannotator(pi: ExtensionAPI): void {
      const gitCtx = getGitContext();
      const { patch: rawPatch, label: gitRef } = runGitDiff("uncommitted", gitCtx.defaultBranch);

-      const server = startReviewServer({
-        rawPatch,
-        gitRef,
-        origin: "pi",
-        diffType: "uncommitted",
-        gitContext: gitCtx,
-        htmlContent: reviewHtmlContent,
-      });
+      let server: ReviewServerResult;
+      try {
+        server = await startReviewServer({
+          rawPatch,
+          gitRef,
+          origin: "pi",
+          diffType: "uncommitted",
+          gitContext: gitCtx,
+          htmlContent: reviewHtmlContent,
+        });
+      } catch (err) {
+        ctx.ui.notify(`Failed to start code review UI: ${getStartupErrorMessage(err)}`, "error");
+        return;
+      }

-      openBrowser(server.url);
-
-      const result = await server.waitForDecision();
-      await new Promise((r) => setTimeout(r, 1500));
-      server.stop();
+      const result = await runBrowserReview(server, ctx);

      if (result.feedback) {
        if (result.approved) {
@@ -289,18 +326,20 @@ export default function plannotator(pi: ExtensionAPI): void {
      ctx.ui.notify(`Opening annotation UI for ${filePath}...`, "info");

      const markdown = readFileSync(absolutePath, "utf-8");
-      const server = startAnnotateServer({
-        markdown,
-        filePath: absolutePath,
-        origin: "pi",
-        htmlContent: planHtmlContent,
-      });
+      let server: AnnotateServerResult;
+      try {
+        server = await startAnnotateServer({
+          markdown,
+          filePath: absolutePath,
+          origin: "pi",
+          htmlContent: planHtmlContent,
+        });
+      } catch (err) {
+        ctx.ui.notify(`Failed to start annotation UI: ${getStartupErrorMessage(err)}`, "error");
+        return;
+      }

-      openBrowser(server.url);
-
-      const result = await server.waitForDecision();
-      await new Promise((r) => setTimeout(r, 1500));
-      server.stop();
+      const result = await runBrowserReview(server, ctx);

      if (result.feedback) {
        pi.sendUserMessage(
@@ -391,18 +430,23 @@ export default function plannotator(pi: ExtensionAPI): void {
      }

      // Start browser-based plan review server
-      const server = startPlanReviewServer({
-        plan: planContent,
-        htmlContent: planHtmlContent,
-        origin: "pi",
-      });
+      let server: PlanServerResult;
+      try {
+        server = await startPlanReviewServer({
+          plan: planContent,
+          htmlContent: planHtmlContent,
+          origin: "pi",
+        });
+      } catch (err) {
+        const message = `Failed to start plan review UI: ${getStartupErrorMessage(err)}`;
+        ctx.ui.notify(message, "error");
+        return {
+          content: [{ type: "text", text: message }],
+          details: { approved: false },
+        };
+      }

-      openBrowser(server.url);
-
-      // Wait for user decision in the browser
-      const result = await server.waitForDecision();
-      await new Promise((r) => setTimeout(r, 1500));
-      server.stop();
+      const result = await runBrowserReview(server, ctx);

      if (result.approved) {
        phase = "executing";
@@ -38,19 +38,93 @@ function html(res: import("node:http").ServerResponse, content: string): void {
  res.end(content);
 }

-function listenOnRandomPort(server: Server): number {
-  server.listen(0);
-  const addr = server.address() as { port: number };
-  return addr.port;
+const DEFAULT_REMOTE_PORT = 19432;
+
+/**
+ * Check if running in a remote session (SSH, devcontainer, etc.)
+ * Honors PLANNOTATOR_REMOTE env var, or detects SSH_TTY/SSH_CONNECTION.
+ */
+function isRemoteSession(): boolean {
+  const remote = process.env.PLANNOTATOR_REMOTE;
+  if (remote === "1" || remote?.toLowerCase() === "true") {
+    return true;
+  }
+  // Legacy SSH detection
+  if (process.env.SSH_TTY || process.env.SSH_CONNECTION) {
+    return true;
+  }
+  return false;
+}
+
+/**
+ * Get the server port to use.
+ * - PLANNOTATOR_PORT env var takes precedence
+ * - Remote sessions default to 19432 (for port forwarding)
+ * - Local sessions use random port
+ * Returns { port, portSource } so caller can notify user if needed.
+ */
+function getServerPort(): { port: number; portSource: "env" | "remote-default" | "random" } {
+  const envPort = process.env.PLANNOTATOR_PORT;
+  if (envPort) {
+    const parsed = parseInt(envPort, 10);
+    if (!isNaN(parsed) && parsed > 0 && parsed < 65536) {
+      return { port: parsed, portSource: "env" };
+    }
+    // Invalid port - fall back silently, caller can check env var themselves
+  }
+  if (isRemoteSession()) {
+    return { port: DEFAULT_REMOTE_PORT, portSource: "remote-default" };
+  }
+  return { port: 0, portSource: "random" };
+}
+
+const MAX_RETRIES = 5;
+const RETRY_DELAY_MS = 500;
+
+async function listenOnPort(server: Server): Promise<{ port: number; portSource: "env" | "remote-default" | "random" }> {
+  const result = getServerPort();
+
+  for (let attempt = 1; attempt <= MAX_RETRIES; attempt++) {
+    try {
+      await new Promise<void>((resolve, reject) => {
+        server.once("error", reject);
+        server.listen(result.port, () => {
+          server.removeListener("error", reject);
+          resolve();
+        });
+      });
+      const addr = server.address() as { port: number };
+      return { port: addr.port, portSource: result.portSource };
+    } catch (err: unknown) {
+      const isAddressInUse = err instanceof Error && err.message.includes("EADDRINUSE");
+      if (isAddressInUse && attempt < MAX_RETRIES) {
+        await new Promise((r) => setTimeout(r, RETRY_DELAY_MS));
+        continue;
+      }
+      if (isAddressInUse) {
+        const hint = isRemoteSession() ? " (set PLANNOTATOR_PORT to use a different port)" : "";
+        throw new Error(`Port ${result.port} in use after ${MAX_RETRIES} retries${hint}`);
+      }
+      throw err;
+    }
+  }
+
+  // Unreachable, but satisfies TypeScript
+  throw new Error("Failed to bind port");
 }

 /**
 * Open URL in system browser (Node-compatible, no Bun $ dependency).
 * Honors PLANNOTATOR_BROWSER and BROWSER env vars, matching packages/server/browser.ts.
+ * Returns { opened: true } if browser was opened, { opened: false, isRemote: true, url } if remote session.
 */
-export function openBrowser(url: string): void {
+export function openBrowser(url: string): { opened: boolean; isRemote?: boolean; url?: string } {
+  const browser = process.env.PLANNOTATOR_BROWSER || process.env.BROWSER;
+  if (isRemoteSession() && !browser) {
+    return { opened: false, isRemote: true, url };
+  }
+
  try {
-    const browser = process.env.PLANNOTATOR_BROWSER || process.env.BROWSER;
    const platform = process.platform;
    const wsl = platform === "linux" && os.release().toLowerCase().includes("microsoft");

@@ -82,8 +156,9 @@ export function openBrowser(url: string): void {
    const child = spawn(cmd, args, { detached: true, stdio: "ignore" });
    child.once("error", () => {});
    child.unref();
+    return { opened: true };
  } catch {
-    // Silently fail
+    return { opened: false };
  }
 }

@@ -266,16 +341,17 @@ function listProjectPlans(

 export interface PlanServerResult {
  port: number;
+  portSource: "env" | "remote-default" | "random";
  url: string;
  waitForDecision: () => Promise<{ approved: boolean; feedback?: string }>;
  stop: () => void;
 }

-export function startPlanReviewServer(options: {
+export async function startPlanReviewServer(options: {
  plan: string;
  htmlContent: string;
  origin?: string;
-}): PlanServerResult {
+}): Promise<PlanServerResult> {
  // Version history
  const slug = generateSlug(options.plan);
  const project = detectProjectName();
@@ -334,18 +410,17 @@ export function startPlanReviewServer(options: {
    }
  });

-  const port = listenOnRandomPort(server);
+  const { port, portSource } = await listenOnPort(server);

  return {
    port,
+    portSource,
    url: `http://localhost:${port}`,
    waitForDecision: () => decisionPromise,
    stop: () => server.close(),
  };
 }

-// ── Code Review Server ──────────────────────────────────────────────────
-
 export type DiffType = "uncommitted" | "staged" | "unstaged" | "last-commit" | "branch";

 export interface DiffOption {
@@ -361,6 +436,7 @@ export interface GitContext {

 export interface ReviewServerResult {
  port: number;
+  portSource: "env" | "remote-default" | "random";
  url: string;
  waitForDecision: () => Promise<{ approved: boolean; feedback: string }>;
  stop: () => void;
@@ -416,14 +492,14 @@ export function runGitDiff(diffType: DiffType, defaultBranch = "main"): { patch:
  }
 }

-export function startReviewServer(options: {
+export async function startReviewServer(options: {
  rawPatch: string;
  gitRef: string;
  htmlContent: string;
  origin?: string;
  diffType?: DiffType;
  gitContext?: GitContext;
-}): ReviewServerResult {
+}): Promise<ReviewServerResult> {
  let currentPatch = options.rawPatch;
  let currentGitRef = options.gitRef;
  let currentDiffType: DiffType = options.diffType || "uncommitted";
@@ -469,10 +545,11 @@ export function startReviewServer(options: {
    }
  });

-  const port = listenOnRandomPort(server);
+  const { port, portSource } = await listenOnPort(server);

  return {
    port,
+    portSource,
    url: `http://localhost:${port}`,
    waitForDecision: () => decisionPromise,
    stop: () => server.close(),
@@ -483,17 +560,18 @@ export function startReviewServer(options: {

 export interface AnnotateServerResult {
  port: number;
+  portSource: "env" | "remote-default" | "random";
  url: string;
  waitForDecision: () => Promise<{ feedback: string }>;
  stop: () => void;
 }

-export function startAnnotateServer(options: {
+export async function startAnnotateServer(options: {
  markdown: string;
  filePath: string;
  htmlContent: string;
  origin?: string;
-}): AnnotateServerResult {
+}): Promise<AnnotateServerResult> {
  let resolveDecision!: (result: { feedback: string }) => void;
  const decisionPromise = new Promise<{ feedback: string }>((r) => {
    resolveDecision = r;
@@ -518,10 +596,11 @@ export function startAnnotateServer(options: {
    }
  });

-  const port = listenOnRandomPort(server);
+  const { port, portSource } = await listenOnPort(server);

  return {
    port,
+    portSource,
    url: `http://localhost:${port}`,
    waitForDecision: () => decisionPromise,
    stop: () => server.close(),