diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | src/index.ts | 11 | ||||
| -rw-r--r-- | src/state.ts | 34 | ||||
| -rw-r--r-- | test/util/state-test.ts | 75 |
4 files changed, 115 insertions, 6 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 601d699..ac990d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ All notable changes to pi-browser-harness will be documented in this file. ### Fixed +- **Browser state is no longer appended to the session on every callback.** Every ownership change, every `session_tree` event (even one that merely restored the entry already on the branch) and every shutdown appended a `browser-harness-state` entry, so the append-only session file and Pi Web's `+N custom` chain grew without bound. A snapshot is now written only when it differs from the last one persisted or restored; tab order does not count as a difference. (#26) - **The daemon starts from a Bun-compiled pi binary.** Inside `pi.exe` (and any other `bun build --compile` host), `createRequire().resolve("tsx/cli")` is rooted at the binary's embedded virtual filesystem and cannot see the extension's real `node_modules`, so `browser_setup` failed instantly with `Could not start the browser daemon`. The resolver result is now checked on disk and, when it fails, the tsx CLI is found by walking up the `node_modules` chain the way Node does. Under Bun the daemon is launched with `node` from `PATH` instead of `process.execPath` (which is the pi binary itself); `PI_BROWSER_NODE` overrides the executable. (#27) ## 0.11.1 — 2026-08-12 diff --git a/src/index.ts b/src/index.ts index 0133f65..d69940a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -4,7 +4,7 @@ import { getBrowserSystemPrompt } from "./prompt"; import { registerSetupCommand } from "./setup"; import { registerProfileCommand } from "./profile/command"; import { readPin } from "./profile/store"; -import { type BrowserState, defaultState, persistState, restoreState } from "./state"; +import { type BrowserState, createStatePersister, defaultState, restoreState } from "./state"; import { registerAllTools } from "./registry"; import { cleanupTempDirs } from "./util/truncate"; import { createDaemonTransport } from "./daemon/transport"; @@ -16,6 +16,7 @@ export default function browserHarnessExtension(pi: ExtensionAPI): void { const namespace = flagNs ?? `pi-${Math.random().toString(36).slice(2, 10)}`; let state: BrowserState = defaultState(namespace); + const persister = createStatePersister(pi); let client: BrowserClient | null = null; let toolsRegistered = false; @@ -83,6 +84,7 @@ export default function browserHarnessExtension(pi: ExtensionAPI): void { pi.on("session_start", async (_event, ctx) => { state = restoreState(ctx, state.namespace); + persister.markPersisted(state); setDebugClicks(asBoolean(pi.getFlag("browser-debug-clicks")) ?? false); // Re-read the on-disk profile pin every session start: another pi session may have changed it since this client was created. @@ -112,7 +114,7 @@ export default function browserHarnessExtension(pi: ExtensionAPI): void { ? { harnessWindowId: snap.harnessWindowId } : {}), }; - try { persistState(pi, state); } catch {} + try { persister.persist(state); } catch {} }, }); } else { @@ -137,13 +139,14 @@ export default function browserHarnessExtension(pi: ExtensionAPI): void { } // Do NOT stop the client or null it out: the surviving daemon connection is what eliminates the per-session "Allow Remote Debugging" prompt. } - persistState(pi, state); + persister.persist(state); await cleanupTempDirs(); }); + // The branch already holds the restored snapshot: re-appending it only grows the session file. pi.on("session_tree", async (_event, ctx) => { state = restoreState(ctx, client?.namespace); - persistState(pi, state); + persister.markPersisted(state); }); pi.on("before_agent_start", async (event) => { diff --git a/src/state.ts b/src/state.ts index 8f25dc4..0620e11 100644 --- a/src/state.ts +++ b/src/state.ts @@ -13,8 +13,38 @@ export const defaultState = (namespace = "default"): BrowserState => ({ ownedTargetIds: [], }); -export const persistState = (pi: ExtensionAPI, state: BrowserState): void => { - pi.appendEntry<BrowserState>("browser-harness-state", state); +// Pi session JSONL is append-only and every custom entry shows up in the branch navigator, so a +// snapshot is only worth appending when it differs from the one the branch already holds (#26). +// Tab order is not a difference: ownership is a set. +const stateKey = (state: BrowserState): string => + JSON.stringify({ + namespace: state.namespace, + ownedTargetIds: [...(state.ownedTargetIds ?? [])].sort(), + harnessWindowTargetId: state.harnessWindowTargetId ?? null, + harnessWindowId: state.harnessWindowId ?? null, + }); + +export type StatePersister = { + // Appends the snapshot unless it equals the last one persisted or restored. Returns whether it wrote. + readonly persist: (state: BrowserState) => boolean; + // Records a snapshot that already lives in the session (restored from the branch) without writing it. + readonly markPersisted: (state: BrowserState) => void; +}; + +export const createStatePersister = (pi: ExtensionAPI): StatePersister => { + let last: string | undefined; + return { + persist: (state) => { + const key = stateKey(state); + if (key === last) return false; + pi.appendEntry<BrowserState>("browser-harness-state", state); + last = key; + return true; + }, + markPersisted: (state) => { + last = stateKey(state); + }, + }; }; const asPersistedState = (v: unknown): Partial<BrowserState> | undefined => { diff --git a/test/util/state-test.ts b/test/util/state-test.ts new file mode 100644 index 0000000..a354ab0 --- /dev/null +++ b/test/util/state-test.ts @@ -0,0 +1,75 @@ +import { test, describe } from "node:test"; +import assert from "node:assert/strict"; +import type { ExtensionAPI } from "@mariozechner/pi-coding-agent"; +import { type BrowserState, createStatePersister, defaultState } from "../../src/state"; + +// Pi session JSONL is append-only, so every appendEntry is permanent. Issue #26: identical +// snapshots were appended on every ownership callback, session_tree and shutdown, growing the +// session file and the branch navigator's "+N custom" chain without bound. +const fakePi = (): { readonly pi: ExtensionAPI; readonly entries: ReadonlyArray<unknown> } => { + const entries: unknown[] = []; + const pi = { + appendEntry: (_customType: string, data?: unknown) => { + entries.push(data); + }, + } as unknown as ExtensionAPI; + return { pi, entries }; +}; + +const owned = (...ids: string[]): BrowserState => ({ namespace: "default", ownedTargetIds: ids }); + +describe("state persister", () => { + test("repeated identical snapshots produce one entry", () => { + const { pi, entries } = fakePi(); + const persister = createStatePersister(pi); + assert.equal(persister.persist(owned("a")), true); + assert.equal(persister.persist(owned("a")), false); + assert.equal(persister.persist(owned("a")), false); + assert.equal(entries.length, 1); + }); + + test("a changed snapshot is appended, and the same change twice is not", () => { + const { pi, entries } = fakePi(); + const persister = createStatePersister(pi); + persister.persist(owned("a")); + persister.persist(owned("a", "b")); + persister.persist(owned("a", "b")); + persister.persist(owned("a")); + assert.deepEqual(entries.map((e) => (e as BrowserState).ownedTargetIds), [["a"], ["a", "b"], ["a"]]); + }); + + test("tab order does not count as a change", () => { + const { pi, entries } = fakePi(); + const persister = createStatePersister(pi); + persister.persist(owned("a", "b")); + persister.persist(owned("b", "a")); + assert.equal(entries.length, 1); + }); + + test("a state restored from the session is not re-appended until it changes", () => { + const { pi, entries } = fakePi(); + const persister = createStatePersister(pi); + persister.markPersisted(owned("a")); + assert.equal(persister.persist(owned("a")), false); + assert.equal(entries.length, 0); + assert.equal(persister.persist(owned("a", "b")), true); + assert.equal(entries.length, 1); + }); + + test("the window ids are part of the comparison", () => { + const { pi, entries } = fakePi(); + const persister = createStatePersister(pi); + persister.persist({ ...owned("a"), harnessWindowTargetId: "w1", harnessWindowId: 7 }); + persister.persist({ ...owned("a"), harnessWindowTargetId: "w1", harnessWindowId: 7 }); + persister.persist({ ...owned("a"), harnessWindowTargetId: "w2", harnessWindowId: 8 }); + assert.equal(entries.length, 2); + }); + + test("the default state is persisted once, then skipped", () => { + const { pi, entries } = fakePi(); + const persister = createStatePersister(pi); + assert.equal(persister.persist(defaultState()), true); + assert.equal(persister.persist(defaultState()), false); + assert.equal(entries.length, 1); + }); +}); |