diff options
| author | Igor Soarez <igor@soarez.org> | 2026-07-29 10:38:13 +0100 |
|---|---|---|
| committer | Igor Soarez <igor@soarez.org> | 2026-10-14 12:19:26 +0100 |
| commit | 08e558753e8ef9b2aa96114981a5a1dd37357bad (patch) | |
| tree | 1d52d895f98d7880e7b1cd1ab6ed13321814dd86 /test | |
| parent | 6eda4b9cce4b2ae7003ccd7429e3f3aad7c04b66 (diff) | |
feat(cdp): /browser-target to switch browsers without leaving pi
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.
Diffstat (limited to 'test')
| -rw-r--r-- | test/cdp/endpoint-test.ts | 60 | ||||
| -rw-r--r-- | test/cdp/target-store-test.ts | 110 |
2 files changed, 169 insertions, 1 deletions
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<void> { + // 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<readonly [string, string]> = [ + ["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(); |
