From 3bbefa9585aaba251eee8afcc2b5d2c6c8a8daa7 Mon Sep 17 00:00:00 2001 From: Andreas Fink <33727073+fink-andreas@users.noreply.github.com> Date: Mon, 16 Mar 2026 04:12:39 +0100 Subject: [PATCH] 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) * 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) --------- Co-authored-by: Michael Ramos Co-authored-by: Claude Opus 4.6 (1M context) --- apps/pi-extension/index.ts | 114 ++++++++++++++++++++++++----------- apps/pi-extension/server.ts | 115 ++++++++++++++++++++++++++++++------ 2 files changed, 176 insertions(+), 53 deletions(-) diff --git a/apps/pi-extension/index.ts b/apps/pi-extension/index.ts index 22843a18..44635354 100644 --- a/apps/pi-extension/index.ts +++ b/apps/pi-extension/index.ts @@ -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 { + url: string; + stop: () => void; + waitForDecision: () => Promise; +} + // 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( + server: DecisionServer, + ctx: ExtensionContext, +): Promise { + 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"; diff --git a/apps/pi-extension/server.ts b/apps/pi-extension/server.ts index fd02b524..03b0a5c5 100644 --- a/apps/pi-extension/server.ts +++ b/apps/pi-extension/server.ts @@ -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((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 { // 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 { 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 { 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(),