From 08e558753e8ef9b2aa96114981a5a1dd37357bad Mon Sep 17 00:00:00 2001 From: Igor Soarez Date: Wed, 29 Jul 2026 10:38:13 +0100 Subject: feat(cdp): /browser-target to switch browsers without leaving pi MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switching between the shared browser and this machine's own previously meant leaving pi and relaunching with BU_CDP_HTTP — and, less obviously, killing the daemon first, or the switch silently did not happen. /browser-target show the current target /browser-target local this machine's browser /browser-target default the built-in default /browser-target host:port somewhere else Precedence becomes env > stored > default. The environment still wins so a one-off `BU_CDP_HTTP=… pi` is never silently overridden by a choice made days ago; when it is set, the command says so rather than pretending to obey. Clearing falls back to the default rather than to local — "use the usual browser", not "go local". The daemon is the reason this needed more than a settings write. It is machine-wide, resolves its target once per process, and holds the WebSocket to the old browser, so re-reading the store would leave tool calls going to the old browser while /browser-status reported the new one. The command therefore replaces the process: the daemon now honours an inbound control/shutdown (the action already existed in the protocol, previously only sent daemon→client), and stopDaemon() asks for it and polls until the process is really gone before ensureDaemon() respawns it against the new target. /browser-status now reads the store too, so it cannot disagree with what the tools will do. Persistence mirrors the profile pin: atomic temp-file-plus-rename, and every read failure degrades to "nothing stored" so a corrupt file falls back to the default instead of breaking browser control. It is a separate file from the pin, so damage to one cannot cost you the other. The stored value stays in BU_CDP_HTTP's vocabulary so there is exactly one resolution path. 62 tests: the full precedence table including the stored tier, that a bad value is blamed on the source that set it rather than sending you hunting through shell config, that the command only accepts values resolution can actually use, and eight ways of damaging the store file. --- src/cdp/discovery.ts | 3 +- src/cdp/endpoint.ts | 76 +++++++++++++++++++++-------- src/cdp/target-store.ts | 87 +++++++++++++++++++++++++++++++++ src/daemon/index.ts | 10 ++++ src/daemon/spawn.ts | 70 +++++++++++++++++++++++++++ src/index.ts | 83 ++++++++++++++++++++++++++++++- src/profile/paths.ts | 7 +++ test/cdp/endpoint-test.ts | 60 ++++++++++++++++++++++- test/cdp/target-store-test.ts | 110 ++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 482 insertions(+), 24 deletions(-) create mode 100644 src/cdp/target-store.ts create mode 100644 test/cdp/target-store-test.ts diff --git a/src/cdp/discovery.ts b/src/cdp/discovery.ts index 53d4ad2..84da5f9 100644 --- a/src/cdp/discovery.ts +++ b/src/cdp/discovery.ts @@ -7,6 +7,7 @@ import { userDataDirCandidates } from "../profile/paths"; import { type CdpError, cdpError } from "./errors"; import { asString, errnoCode, isRecord } from "../util/guards"; import { describeCdpTarget, resolveCdpTarget } from "./endpoint"; +import { readTarget } from "./target-store"; const PORT_PROBE_DEADLINE_MS = 30_000; const PORT_PROBE_INTERVAL_MS = 1_000; @@ -127,7 +128,7 @@ export const discoverEndpoint = async (): Promise> // No userDataDir is returned: the profile registry lives on the remote host, // so profile enumeration degrades to the pre-profile behaviour rather than // reading the wrong machine's Local State. - const target = resolveCdpTarget(); + const target = resolveCdpTarget(process.env, await readTarget()); if (target.kind === "invalid") { return err(cdpError("discovery_failed", describeCdpTarget(target))); } diff --git a/src/cdp/endpoint.ts b/src/cdp/endpoint.ts index 1be5322..96cf2ea 100644 --- a/src/cdp/endpoint.ts +++ b/src/cdp/endpoint.ts @@ -15,31 +15,53 @@ export const DEFAULT_REMOTE_CDP = "10.88.0.25:9223"; /** Values of BU_CDP_HTTP that mean "ignore the default, use this machine". */ const LOCAL_ALIASES = new Set(["local", "localhost", "off", "none", "0", "no"]); +/** + * Where a target came from. "env" is BU_CDP_HTTP, "stored" is /browser-target, + * "default" is this fork's built-in. Only "default" is allowed to fall back. + */ +export type CdpTargetSource = "env" | "stored" | "default"; + export type CdpTarget = | { readonly kind: "remote"; readonly host: string; readonly port: number; - /** "env" was asked for explicitly; "default" is this fork's built-in. */ - readonly source: "env" | "default"; + readonly source: CdpTargetSource; } - | { readonly kind: "local"; readonly source: "env" } - | { readonly kind: "invalid"; readonly raw: string; readonly reason: string }; + | { readonly kind: "local"; readonly source: Exclude } + | { readonly kind: "invalid"; readonly raw: string; readonly reason: string; readonly source: CdpTargetSource }; /** - * Precedence: - * BU_CDP_HTTP="host:port" -> that remote, and a failure to reach it is fatal - * BU_CDP_HTTP=local|off|… -> upstream behaviour, this machine's own browser - * unset -> DEFAULT_REMOTE_CDP, falling back to local when - * it isn't reachable + * Precedence: BU_CDP_HTTP, then the /browser-target choice, then the built-in + * default. + * + * "host:port" -> that remote, and a failure to reach it is fatal + * local|off|… -> upstream behaviour, this machine's own browser + * neither set -> DEFAULT_REMOTE_CDP, falling back to local when unreachable + * + * The environment wins over the stored choice so that a one-off + * `BU_CDP_HTTP=… pi` is never silently overridden by a setting made days ago. + * + * The fallback asymmetry is deliberate. An explicit endpoint — from either + * source — is an instruction, so silently driving a different browser instead + * would be wrong: it would automate the operator's own signed-in Chrome rather + * than the shared one. The built-in default is only a preference, so a laptop + * away from the wire network still gets the local browser the extension was + * originally written for. * - * The asymmetry is deliberate. An explicit endpoint is an instruction, so - * silently driving a different browser instead would be wrong. The built-in - * default is only a preference, so a laptop away from the wire network still - * gets the local browser the extension was originally written for. + * `stored` is passed in rather than read here so this stays pure and + * synchronous: /browser-status can describe the target without I/O, and the + * daemon can resolve identically in its own process. */ -export const resolveCdpTarget = (env: NodeJS.ProcessEnv = process.env): CdpTarget => { - const raw = env["BU_CDP_HTTP"]?.trim(); +export const resolveCdpTarget = ( + env: NodeJS.ProcessEnv = process.env, + stored: string | null = null, +): CdpTarget => { + const fromEnv = env["BU_CDP_HTTP"]?.trim(); + const fromStore = stored?.trim(); + + const raw = fromEnv !== undefined && fromEnv !== "" ? fromEnv : fromStore; + const source: CdpTargetSource = fromEnv !== undefined && fromEnv !== "" ? "env" : "stored"; if (raw === undefined || raw === "") { const parsed = parseHostPort(DEFAULT_REMOTE_CDP); @@ -48,17 +70,28 @@ export const resolveCdpTarget = (env: NodeJS.ProcessEnv = process.env): CdpTarge return { kind: "remote", host: parsed.host, port: parsed.port, source: "default" }; } - if (LOCAL_ALIASES.has(raw.toLowerCase())) return { kind: "local", source: "env" }; + if (LOCAL_ALIASES.has(raw.toLowerCase())) return { kind: "local", source }; const parsed = parseHostPort(raw); if (!parsed) { return { kind: "invalid", raw, + source, reason: `expected "host:port" or one of ${[...LOCAL_ALIASES].join(", ")}`, }; } - return { kind: "remote", host: parsed.host, port: parsed.port, source: "env" }; + return { kind: "remote", host: parsed.host, port: parsed.port, source }; +}; + +/** The vocabulary /browser-target accepts, for help text and validation. */ +export const localAliases = (): ReadonlyArray => [...LOCAL_ALIASES]; + +/** True when `value` is something resolveCdpTarget can make sense of. */ +export const isValidTargetValue = (value: string): boolean => { + const v = value.trim(); + if (v === "") return false; + return LOCAL_ALIASES.has(v.toLowerCase()) || parseHostPort(v) !== null; }; const parseHostPort = (value: string): { host: string; port: number } | null => { @@ -71,14 +104,17 @@ const parseHostPort = (value: string): { host: string; port: number } | null => return { host, port }; }; +const sourceLabel = (source: CdpTargetSource): string => + source === "env" ? "BU_CDP_HTTP" : source === "stored" ? "/browser-target" : "built-in default"; + /** One-line description for /browser-status and error messages. */ export const describeCdpTarget = (target: CdpTarget): string => { switch (target.kind) { case "remote": - return `${target.host}:${target.port}` + (target.source === "default" ? " (built-in default)" : " (BU_CDP_HTTP)"); + return `${target.host}:${target.port} (${sourceLabel(target.source)})`; case "local": - return "this machine's own browser (BU_CDP_HTTP=local)"; + return `this machine's own browser (${sourceLabel(target.source)})`; case "invalid": - return `invalid BU_CDP_HTTP "${target.raw}" — ${target.reason}`; + return `invalid target "${target.raw}" from ${sourceLabel(target.source)} — ${target.reason}`; } }; diff --git a/src/cdp/target-store.ts b/src/cdp/target-store.ts new file mode 100644 index 0000000..3a35f0a --- /dev/null +++ b/src/cdp/target-store.ts @@ -0,0 +1,87 @@ +/** + * FORK: durable persistence for the chosen CDP target. + * + * `/browser-target` has to outlive the pi session that ran it, because the + * browser daemon is machine-wide and shared by every session — so the choice + * cannot live in pi's session entries. Same reasoning, and the same atomic + * write, as the profile pin in src/profile/store.ts; a separate file so a + * corrupt target cannot cost you your profile selection. + * + * The stored value is the raw string a user would have put in BU_CDP_HTTP + * ("local", "10.88.0.25:9223", …). It is deliberately NOT pre-parsed: keeping + * it in the same vocabulary as the environment variable means one resolution + * path in src/cdp/endpoint.ts, and a stored value can never mean something the + * variable could not. + * + * Every read failure degrades to "nothing stored" rather than throwing — a + * corrupt file must never break browser control, it should just fall back to + * the built-in default. + */ + +import { mkdir, readFile, rename, unlink, writeFile } from "node:fs/promises"; +import { randomUUID } from "node:crypto"; +import { dirname } from "node:path"; +import { type Result, err, ok } from "../util/result"; +import { isRecord } from "../util/guards"; +import { targetFilePath } from "../profile/paths"; + +const CURRENT_VERSION = 1; + +type TargetFile = { + readonly version: 1; + /** Raw BU_CDP_HTTP-style value, or null when cleared. */ + readonly target: string | null; + readonly savedAt: string; +}; + +/** + * The persisted target, or null when nothing is stored, the file is missing, or + * it cannot be parsed. A file written by a newer version is treated as "nothing + * stored" rather than guessed at. + */ +export const readTarget = async (): Promise => { + let raw: string; + try { + raw = await readFile(targetFilePath(), "utf8"); + } catch { + return null; + } + try { + const parsed: unknown = JSON.parse(raw); + if (!isRecord(parsed)) return null; + const file = parsed; + if (file["version"] !== CURRENT_VERSION) return null; + const target = file["target"]; + if (typeof target !== "string") return null; + const trimmed = target.trim(); + return trimmed.length > 0 ? trimmed : null; + } catch { + return null; + } +}; + +/** + * Replace the target atomically: a sibling temp file plus rename, so a crash + * mid-write cannot leave a half-written target behind — which would otherwise + * silently point the agent at the wrong browser. + */ +const writeTargetFile = async (target: string | null): Promise> => { + const path = targetFilePath(); + const tmp = `${path}.${randomUUID()}.tmp`; + const payload: TargetFile = { version: CURRENT_VERSION, target, savedAt: new Date().toISOString() }; + try { + await mkdir(dirname(path), { recursive: true }); + await writeFile(tmp, `${JSON.stringify(payload, null, 2)}\n`, "utf8"); + await rename(tmp, path); + return ok(undefined); + } catch (e) { + await unlink(tmp).catch(() => {}); + return err(`could not save browser target to ${path}: ${e instanceof Error ? e.message : String(e)}`); + } +}; + +/** Persist the chosen target. */ +export const writeTarget = (target: string): Promise> => writeTargetFile(target); + +/** Clear the choice, restoring the built-in default. */ +export const clearTarget = (): Promise> => writeTargetFile(null); diff --git a/src/daemon/index.ts b/src/daemon/index.ts index 226a895..916be6c 100644 --- a/src/daemon/index.ts +++ b/src/daemon/index.ts @@ -9,6 +9,16 @@ async function main() { const cdpBridge = createCdpBridge(); ipcServer.onMessage((msg, client) => { + // FORK: a client may ask the daemon to exit. /browser-target uses this to + // switch browsers: the target is resolved per daemon process, and this one + // is already holding a WebSocket to the old browser, so re-reading the + // stored target is not enough — the process has to go and be respawned. + if (msg.type === "control" && msg.action === "shutdown") { + console.log(`[pi-browser-daemon] Shutdown requested by ${client.id}: ${msg.reason ?? "no reason given"}`); + void shutdown(); + return; + } + if (msg.type !== "request") return; const send: SendToClient = (cid, resp) => { diff --git a/src/daemon/spawn.ts b/src/daemon/spawn.ts index cea8325..726cf84 100644 --- a/src/daemon/spawn.ts +++ b/src/daemon/spawn.ts @@ -32,6 +32,76 @@ export const daemonSocketMayExist = async ( } }; +/** + * FORK: ask a running daemon to exit, and wait until it has. + * + * Needed by /browser-target. The CDP target is resolved per daemon process and + * the daemon holds an open WebSocket to the old browser, so switching browsers + * means replacing the process — re-reading the stored target would not drop + * that connection. + * + * Resolves true once the daemon is gone (or was never running). Resolves false + * if it is still answering after the timeout, so callers can report the failure + * instead of respawning into a port clash. + */ +export const stopDaemon = async (timeoutMs = 5_000): Promise => { + if (!(await isDaemonRunning(1_500))) { + cleanupStaleSocket(); + return true; + } + + await new Promise((resolve) => { + let settled = false; + const done = (): void => { + if (settled) return; + settled = true; + resolve(); + }; + + const sock = createConnection(DAEMON_SOCKET_PATH); + const timer = setTimeout(() => { + sock.destroy(); + done(); + }, timeoutMs); + + sock.on("connect", () => { + // Registration is required before the daemon forwards anything to its + // message handler, so the shutdown has to come from a registered client. + const reg: WireControl = { type: "control", action: "register", clientId: "_target_switch" }; + sock.write(serialize(reg) + "\n"); + }); + + sock.on("data", (chunk: Buffer) => { + if (!chunk.toString().includes('"registered"')) return; + const bye: WireControl = { type: "control", action: "shutdown", reason: "browser target changed" }; + try { + sock.write(serialize(bye) + "\n"); + } catch { + // Daemon may already be going away; the poll below is the real check. + } + clearTimeout(timer); + sock.destroy(); + done(); + }); + + sock.on("error", () => { + clearTimeout(timer); + done(); + }); + }); + + // The daemon exits asynchronously, so confirm rather than assume. + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (!(await isDaemonRunning(1_000))) { + cleanupStaleSocket(); + return true; + } + await new Promise((r) => setTimeout(r, 200)); + } + return false; +}; + export const isDaemonRunning = async (timeoutMs = 2_000): Promise => { if (!(await daemonSocketMayExist())) return false; diff --git a/src/index.ts b/src/index.ts index 2b95697..152c94d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -11,7 +11,9 @@ import { restoreDownloadBehavior } from "./domains/files"; import { createDaemonTransport } from "./daemon/transport"; import { setDebugClicks } from "./util/debug"; import { asBoolean, asString } from "./util/guards"; -import { describeCdpTarget, resolveCdpTarget } from "./cdp/endpoint"; +import { describeCdpTarget, isValidTargetValue, localAliases, resolveCdpTarget } from "./cdp/endpoint"; +import { clearTarget, readTarget, writeTarget } from "./cdp/target-store"; +import { ensureDaemon, stopDaemon } from "./daemon/spawn"; import { queryLiveWsUrl } from "./cdp/discovery"; export default function browserHarnessExtension(pi: ExtensionAPI): void { @@ -47,7 +49,7 @@ export default function browserHarnessExtension(pi: ExtensionAPI): void { // first browser_* tool runs. A plain "Disconnected" reads as a fault // when it is really just "not needed yet", so say which it is, and show // what we would connect to. - const target = resolveCdpTarget(); + const target = resolveCdpTarget(process.env, await readTarget()); const reachable = target.kind === "remote" ? (await queryLiveWsUrl(target.port, 2_000, target.host)) !== null : null; @@ -91,6 +93,83 @@ export default function browserHarnessExtension(pi: ExtensionAPI): void { }, }); + // FORK: switch which browser the harness drives, without leaving pi. + // + // The target is resolved per daemon process and the daemon holds the browser + // connection, so a switch has to replace that process — writing the store + // alone would leave tool calls going to the old browser while /browser-status + // reported the new one. + pi.registerCommand("browser-target", { + description: "Show or change which browser to drive (local, default, or host:port)", + handler: async (args, ctx) => { + const wanted = (typeof args === "string" ? args : "").trim(); + + if (wanted === "") { + const current = resolveCdpTarget(process.env, await readTarget()); + ctx.ui.notify( + `Browser target: ${describeCdpTarget(current)}\n` + + `Change with: /browser-target local | default | host:port`, + "info", + ); + return; + } + + const clearing = wanted.toLowerCase() === "default"; + if (!clearing && !isValidTargetValue(wanted)) { + ctx.ui.notify( + `Not a usable target: "${wanted}"\n` + + `Expected "host:port", "default", or one of ${localAliases().join(", ")}.`, + "error", + ); + return; + } + + const saved = clearing ? await clearTarget() : await writeTarget(wanted); + if (!saved.success) { + ctx.ui.notify(saved.error, "error"); + return; + } + + const resolved = resolveCdpTarget(process.env, await readTarget()); + + // Warn rather than silently disobey: the environment deliberately wins, + // so a stored choice made now would not take effect this session. + const envOverride = (process.env["BU_CDP_HTTP"] ?? "").trim(); + if (envOverride !== "") { + ctx.ui.notify( + `Saved, but BU_CDP_HTTP=${envOverride} is set in this environment and takes precedence.\n` + + `Still using: ${describeCdpTarget(resolved)}`, + "warning", + ); + return; + } + + ctx.ui.notify(`Switching to ${describeCdpTarget(resolved)}...`, "info"); + + if (client) await client.stop(); + if (!(await stopDaemon())) { + ctx.ui.notify("Saved, but the browser daemon did not stop — run /browser-setup to pick it up.", "warning"); + return; + } + if (!(await ensureDaemon())) { + ctx.ui.notify("Saved, but the browser daemon did not come back. Run /browser-setup.", "error"); + return; + } + + if (!client) { + ctx.ui.notify(`Browser target set to ${describeCdpTarget(resolved)} ✓ — run /browser-setup to connect.`, "info"); + return; + } + const started = await client.start(); + ctx.ui.notify( + started.success + ? `Browser target set to ${describeCdpTarget(resolved)} ✓` + : `Target saved (${describeCdpTarget(resolved)}) but connecting failed: ${started.error.message}`, + started.success ? "info" : "error", + ); + }, + }); + pi.registerCommand("browser-reload-daemon", { description: "Restart the browser client", handler: async (_args, ctx) => { diff --git a/src/profile/paths.ts b/src/profile/paths.ts index 0a7c74e..4ff3f7d 100644 --- a/src/profile/paths.ts +++ b/src/profile/paths.ts @@ -26,6 +26,13 @@ export const agentDir = (): string => { export const pinFilePath = (): string => join(agentDir(), "browser-harness.json"); +/** + * FORK: where the chosen CDP target is persisted (see src/cdp/target-store.ts). + * A separate file from the profile pin: the two have different lifetimes, and a + * corrupt target must not cost you your profile selection or vice versa. + */ +export const targetFilePath = (): string => join(agentDir(), "browser-target.json"); + const localAppData = (): string => process.env["LOCALAPPDATA"] ?? join(homedir(), "AppData", "Local"); export const userDataDirCandidates = (): ReadonlyArray => { diff --git a/test/cdp/endpoint-test.ts b/test/cdp/endpoint-test.ts index d4b696f..57d6088 100644 --- a/test/cdp/endpoint-test.ts +++ b/test/cdp/endpoint-test.ts @@ -9,7 +9,12 @@ * * Run: npx tsx test/cdp/endpoint-test.ts */ -import { DEFAULT_REMOTE_CDP, describeCdpTarget, resolveCdpTarget } from "../../src/cdp/endpoint"; +import { + DEFAULT_REMOTE_CDP, + describeCdpTarget, + isValidTargetValue, + resolveCdpTarget, +} from "../../src/cdp/endpoint"; let passed = 0; let failed = 0; @@ -92,6 +97,59 @@ function main(): void { check(describeCdpTarget(resolveCdpTarget(env("local"))).includes("own browser"), "E8: local is described plainly"); } + // E9: the stored choice (/browser-target) sits between env and the default. + { + const t = resolveCdpTarget({}, "local"); + check(t.kind === "local" && t.source === "stored", "E9: a stored value is used when the env is unset"); + + const r = resolveCdpTarget({}, "10.2.3.4:9224"); + check(r.kind === "remote" && r.source === "stored", "E9: a stored host:port resolves as a remote target"); + } + + // E10: the environment wins, so a one-off `BU_CDP_HTTP=… pi` is never + // silently overridden by a setting made days ago. + { + const t = resolveCdpTarget(env("local"), "10.2.3.4:9224"); + check(t.kind === "local" && t.source === "env", "E10: env beats the stored value"); + + const r = resolveCdpTarget(env("10.9.9.9:1234"), "local"); + check(r.kind === "remote" && r.host === "10.9.9.9" && r.source === "env", "E10: an env endpoint beats stored local"); + } + + // E11: clearing the stored value falls back to the built-in default rather + // than to local — clearing is "use the usual browser", not "go local". + { + const t = resolveCdpTarget({}, null); + check(t.kind === "remote" && t.source === "default", "E11: no stored value falls back to the default"); + const blank = resolveCdpTarget({}, " "); + check(blank.kind === "remote" && blank.source === "default", "E11: a blank stored value is treated as cleared"); + } + + // E12: a corrupt stored value must be reported against the right source, or + // the operator goes hunting through their shell config for an env var that + // was never set. + { + const t = resolveCdpTarget({}, "garbage"); + check(t.kind === "invalid" && t.source === "stored", "E12: a bad stored value is blamed on /browser-target"); + if (t.kind === "invalid") { + check(describeCdpTarget(t).includes("/browser-target"), "E12: the description names the stored source"); + } + const e = resolveCdpTarget(env("garbage"), null); + check(e.kind === "invalid" && e.source === "env", "E12: a bad env value is blamed on BU_CDP_HTTP"); + } + + // E13: the values /browser-target accepts must match what resolution can + // actually use, or the command would happily store something unusable. + { + for (const good of ["local", "off", "1.2.3.4:9222", "castle.local:9223"]) { + check(isValidTargetValue(good), `E13: "${good}" is accepted by the command`); + } + for (const bad of ["", " ", "garbage", "host:", "host:99999"]) { + check(!isValidTargetValue(bad), `E13: "${bad}" is rejected by the command`); + } + check(!isValidTargetValue("default"), "E13: \"default\" is handled as clear, not stored verbatim"); + } + console.log(`\n${passed} passed, ${failed} failed`); if (failed > 0) process.exit(1); } diff --git a/test/cdp/target-store-test.ts b/test/cdp/target-store-test.ts new file mode 100644 index 0000000..56def62 --- /dev/null +++ b/test/cdp/target-store-test.ts @@ -0,0 +1,110 @@ +/** + * Unit tests for the persisted CDP target — no browser required. + * + * The store is what makes /browser-target outlive the pi session that ran it, + * because the browser daemon is machine-wide. Its central obligation is that a + * damaged file degrades to "nothing stored" — falling back to the built-in + * default — rather than throwing and taking browser control down with it. + * + * Run: npx tsx test/cdp/target-store-test.ts + */ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +let passed = 0; +let failed = 0; +const check = (cond: boolean, label: string): void => { + if (cond) { + passed++; + console.log(` ✓ ${label}`); + } else { + failed++; + console.error(` ✗ ${label}`); + } +}; + +async function main(): Promise { + // The store resolves its path from pi's agent dir, which paths.ts derives + // from the environment — so point that at a scratch dir before importing. + const root = mkdtempSync(join(tmpdir(), "pi-target-")); + process.env["PI_CODING_AGENT_DIR"] = root; + + const { readTarget, writeTarget, clearTarget } = await import("../../src/cdp/target-store"); + const { targetFilePath } = await import("../../src/profile/paths"); + const file = targetFilePath(); + + try { + // T1: nothing stored yet. + check((await readTarget()) === null, "T1: a missing file reads as nothing stored"); + + // T2: round-trip. + { + const w = await writeTarget("local"); + check(w.success, "T2: writing a target succeeds"); + check((await readTarget()) === "local", "T2: it reads back verbatim"); + } + + // T3: overwrite, not append — a switch must fully replace the last choice. + { + await writeTarget("10.4.5.6:9299"); + check((await readTarget()) === "10.4.5.6:9299", "T3: a second write replaces the first"); + } + + // T4: clearing restores "nothing stored", which resolution reads as the + // built-in default rather than as local. + { + const c = await clearTarget(); + check(c.success, "T4: clearing succeeds"); + check((await readTarget()) === null, "T4: a cleared target reads as nothing stored"); + } + + // T5: whitespace is normalised on read, so a hand-edited file with a + // trailing newline inside the JSON string still resolves. + { + writeFileSync(file, JSON.stringify({ version: 1, target: " local ", savedAt: "" })); + check((await readTarget()) === "local", "T5: surrounding whitespace is trimmed"); + } + + // T6: damage must degrade to null, never throw. Each of these is a real + // failure mode: truncated write, hand-edit, or a file from a future version. + { + const damaged: ReadonlyArray = [ + ["not json at all", "T6: unparseable JSON"], + ['{"version":1,"target":', "T6: truncated JSON"], + ['{"version":2,"target":"local"}', "T6: a newer file version"], + ['{"version":1,"target":42}', "T6: a non-string target"], + ['{"version":1,"target":null}', "T6: an explicitly null target"], + ['{"version":1}', "T6: a missing target field"], + ["[]", "T6: a JSON array instead of an object"], + ['{"version":1,"target":""}', "T6: an empty target string"], + ]; + for (const [contents, label] of damaged) { + writeFileSync(file, contents); + let threw = false; + let value: string | null = "unset"; + try { + value = await readTarget(); + } catch { + threw = true; + } + check(!threw && value === null, `${label} degrades to nothing stored`); + } + } + + // T7: recovery — a damaged file must not poison later writes. + { + writeFileSync(file, "garbage"); + const w = await writeTarget("local"); + check(w.success, "T7: writing over a damaged file succeeds"); + check((await readTarget()) === "local", "T7: the store recovers"); + } + } finally { + rmSync(root, { recursive: true, force: true }); + } + + console.log(`\n${passed} passed, ${failed} failed`); + if (failed > 0) process.exit(1); +} + +void main(); -- cgit v1.2.3