From a6ec1d0915106b127affbd6b5fcc9efec48ef712 Mon Sep 17 00:00:00 2001 From: amankumarsingh77 Date: Tue, 28 Jul 2026 20:29:29 +0530 Subject: refactor(profile): strip comment noise Removes module banners and comments restating the code. Keeps one-line notes for the ProcessSingleton user-data-dir rule, the sentinel-URL and async-spawn-error quirks, the replicated getAgentDir, pi's tilde-expansion mirroring, the Windows wmic removal, the macOS/Linux process-identification quirks, the picker's undefined-means-cancel contract, and the deliberately unpersisted browserContextId. Co-Authored-By: Claude Opus 5 (1M context) --- src/profile/bind.ts | 38 +------------ src/profile/browser-process.ts | 119 +++-------------------------------------- src/profile/command.ts | 24 --------- src/profile/launch.ts | 44 ++------------- src/profile/list.ts | 48 +---------------- src/profile/paths.ts | 65 +--------------------- src/profile/picker.ts | 25 +-------- src/profile/store.ts | 32 +---------- 8 files changed, 17 insertions(+), 378 deletions(-) (limited to 'src') diff --git a/src/profile/bind.ts b/src/profile/bind.ts index abdc5cc..0909d09 100644 --- a/src/profile/bind.ts +++ b/src/profile/bind.ts @@ -1,22 +1,3 @@ -/** - * Binding the harness session to a pinned profile. - * - * Sequence, all of it verified against a live browser: - * 1. resolve the browser binary and any explicit --user-data-dir - * 2. launch a window into the profile, marked with a file:// sentinel - * 3. poll Target.getTargets for the sentinel page — its browserContextId IS - * the profile's context, which is otherwise undiscoverable (CDP exposes no - * profile identity) - * 4. adopt that page as the harness window, blank it, drop the sentinel file - * - * If the sentinel never appears the bind FAILS LOUDLY. Falling through to a - * normal createTarget would put the agent in the focus-derived default profile - * — the exact bug this feature exists to remove — so the caller gets an - * actionable error instead. - * - * The resolved context id is session-scoped state, never persisted: Chrome - * mints new context ids on every browser run. - */ import type { BrowserClient } from "../client"; import { attachTo } from "../cdp/attach"; @@ -28,7 +9,6 @@ import { detectRunningBrowser, resolveBrowserExecutable } from "./browser-proces import { openProfileWindow } from "./launch"; import type { ProfilePin } from "./store"; -/** The launch delegates through ProcessSingleton, so allow for a cold profile. */ const SEED_DEADLINE_MS = 15_000; const SEED_POLL_MS = 500; @@ -44,16 +24,10 @@ const manualFallbackHint = (pin: ProfilePin): string => `open a window for "${pin.label}" yourself (browser profile menu → ${pin.profileDir}), then retry. ` + `Run /browser-profile to pick a different profile, or clear the selection to use whichever window is focused.`; -/** - * Open (and adopt) a harness window inside the pinned profile. Returns the - * seed tab's target id. - */ export const seedProfileWindow = async ( client: BrowserClient, pin: ProfilePin, ): Promise> => { - // A pin belongs to one user-data-dir. If the harness is now attached to a - // different browser, silently seeding the "same" profile name would be wrong. const connectedDir = client.userDataDir(); if (connectedDir && connectedDir !== pin.userDataDir) { return err(cdpError( @@ -63,9 +37,6 @@ export const seedProfileWindow = async ( )); } - // Identify the browser by the dir we're connected to. With several Chromium - // browsers running, launching into the wrong one would open a window the - // harness cannot see — in a browser the user never pointed us at. const targetUserDataDir = connectedDir ?? pin.userDataDir; const running = await detectRunningBrowser(targetUserDataDir); const exePath = await resolveBrowserExecutable(running); @@ -79,11 +50,7 @@ export const seedProfileWindow = async ( const launched = await openProfileWindow({ exePath, profileDir: pin.profileDir, - // Forward the user-data-dir ONLY when the running browser advertises one. - // ProcessSingleton keys on this path: passing the value the target process - // itself reports guarantees delegation, whereas supplying a default dir we - // reconstructed could differ by case or normalisation and start a SECOND - // browser instead. + // ProcessSingleton keys on the user-data-dir path, so forward only the value the running browser reports: a reconstructed one may differ by case and start a SECOND browser. ...(running.explicitUserDataDir ? { explicitUserDataDir: running.explicitUserDataDir } : {}), }); if (!launched.success) { @@ -116,9 +83,6 @@ export const seedProfileWindow = async ( const win = await getWindowId(client.session(), seed.targetId); if (win.success) ownership.setHarnessWindowId(win.data); - // Blank the handshake page so the seed tab behaves like any other fresh tab, - // then remove the sentinel file. Best-effort: a failure here costs nothing - // beyond the placeholder page staying visible. const attached = await attachTo(client.session(), seed.targetId); if (attached.success) { await client.session().callOnTarget("Page.navigate", { url: "about:blank" }, attached.data); diff --git a/src/profile/browser-process.ts b/src/profile/browser-process.ts index 5eb9b58..cbf4d70 100644 --- a/src/profile/browser-process.ts +++ b/src/profile/browser-process.ts @@ -1,34 +1,5 @@ -/** - * Identifying the browser process the harness is attached to. - * - * Two questions, deliberately kept apart: - * - * 1. Is a Chromium-family browser RUNNING? (`detectRunningBrowser().running`) - * Only live-process evidence may answer this — an installed-but-closed - * browser must read as "not running", which is why the registry lookup - * below is never consulted here. - * - * 2. Which binary do we invoke to open a window for a chosen profile? - * (`resolveBrowserExecutable()`) Chromium's ProcessSingleton hands a second - * launch's argv to the already-running browser instead of starting another - * one — Windows via WM_COPYDATA (process_singleton_win.cc), Linux and macOS - * via the socket in the user-data-dir (process_singleton_posix.cc, which is - * compiled on macOS too). Here an installed path is a fine fallback. - * - * Whether the running browser carries an explicit `--user-data-dir` matters: - * ProcessSingleton keys on that directory, so passing a path that differs from - * the running browser's by case or normalisation would start a SECOND browser - * rather than delegating. The flag is forwarded only when the running process - * already has one. - * - * Every probe is best-effort; when nothing can be determined, profile launching - * degrades to a clear message rather than guessing. - * - * Windows note: `wmic` is deliberately unused — Microsoft removed it by default - * in Windows 11 24H2 and entirely in 25H2. PowerShell CIM is the supported - * replacement, with `tasklist` for liveness and the documented App Paths - * registry key for install location. - */ +// `running` is derived only from live-process evidence: an installed-but-closed browser must read as "not running". +// `wmic` is deliberately unused — Microsoft removed it by default in Windows 11 24H2 and entirely in 25H2, so PowerShell CIM plus tasklist replace it. import { execFile } from "node:child_process"; import { access, readFile, readlink } from "node:fs/promises"; @@ -39,20 +10,15 @@ import { Compile } from "typebox/compile"; import { parseJson } from "../schemas/parse"; import { browserNameForUserDataDir } from "./paths"; -// execFile, not exec: arguments are passed as an array, with no shell involved. const run = promisify(execFile); const EXEC_TIMEOUT_MS = 5_000; export type RunningBrowser = { - /** True when a Chromium-family browser process is live. */ readonly running: boolean; - /** Absolute path to the running browser's binary, when determinable. */ readonly exePath?: string; - /** Value of an explicit --user-data-dir on the running process, if any. */ readonly explicitUserDataDir?: string; }; -/** Chromium-family process names, lowercase, as they appear per platform. */ const LINUX_COMMS: ReadonlyArray = [ "chrome", "chromium", "chromium-browser", "msedge", "microsoft-edge", "google-chrome", "brave", "brave-browser", @@ -62,22 +28,13 @@ const MAC_BROWSER_NAMES: ReadonlyArray = [ ]; const WIN_IMAGE_NAMES: ReadonlyArray = ["chrome.exe", "msedge.exe", "brave.exe"]; -/** - * Chromium tags every child process with `--type=`; only the browser - * process omits it or carries `--type=browser`. Matching the flag (rather than - * words like "renderer" anywhere in the string) keeps a user whose install path - * contains such a word from being filtered out. - */ +// Match the `--type=` flag, not words like "renderer" anywhere in the string, or an install path containing one is filtered out. const hasChildProcessType = (commandLine: string): boolean => { const lower = commandLine.toLowerCase(); return lower.includes("--type=") && !lower.includes("--type=browser"); }; -/** - * macOS names its child processes in the executable path itself ("Google Chrome - * Helper (Renderer)", crashpad, updaters), so the process NAME must be filtered - * too — the same exclusions the previous setup.ts check applied. - */ +// macOS names its child processes in the executable path itself, so the process NAME must be filtered too. const isMacChildProcessName = (comm: string): boolean => { const lower = comm.toLowerCase(); return ( @@ -89,25 +46,14 @@ const isMacChildProcessName = (comm: string): boolean => { ); }; -/** One running browser process, as much as the platform will tell us. */ export type BrowserCandidate = { readonly exePath?: string; readonly explicitUserDataDir?: string; }; -/** - * Linux marks a replaced binary in `/proc//exe`: once the file behind a - * running process is unlinked — which is exactly what a `google-chrome` package - * upgrade does while the browser keeps running — the symlink reads - * "/opt/google/chrome/chrome (deleted)". Spawning that string fails with - * ENOENT, so the marker has to come off. - * - * It is stripped only as a fallback, and only when the stripped path exists: a - * binary genuinely named "… (deleted)" must still win. - */ +// A package upgrade under a running browser makes `/proc//exe` read "… (deleted)", which fails to spawn with ENOENT unless the marker comes off. const DELETED_SUFFIX = " (deleted)"; -/** The path without Linux's deleted-binary marker, or undefined if unmarked. */ export const stripDeletedSuffix = (path: string): string | undefined => path.endsWith(DELETED_SUFFIX) && path.length > DELETED_SUFFIX.length ? path.slice(0, -DELETED_SUFFIX.length) @@ -122,12 +68,6 @@ const exists = async (path: string): Promise => { } }; -/** - * A spawnable executable path for a raw readlink/argv value: the value itself - * when it exists, else the same value with the deleted-binary marker removed. - * Undefined when neither form is on disk — a path we cannot spawn must never be - * handed on as if it were usable. - */ export const spawnableExePath = async (raw: string): Promise => { if (raw.length === 0) return undefined; if (await exists(raw)) return raw; @@ -136,7 +76,6 @@ export const spawnableExePath = async (raw: string): Promise return undefined; }; -/** Path comparison that tolerates separator and case differences per platform. */ const samePath = (a: string, b: string): boolean => { const norm = (p: string): string => { const unified = p.replace(/\\/g, "/").replace(/\/+$/, ""); @@ -145,22 +84,11 @@ const samePath = (a: string, b: string): boolean => { return norm(a) === norm(b); }; -/** - * Rank a candidate against the user-data-dir the harness is actually connected - * to. Several Chromium browsers can run at once (Chrome plus Brave, or a second - * instance on a custom dir), and launching a profile window into the wrong one - * would open a window the harness cannot see — while disturbing a browser the - * user never pointed us at. - */ export const rankBrowserCandidate = (candidate: BrowserCandidate, preferUserDataDir?: string): number => { if (!preferUserDataDir) return 1; if (candidate.explicitUserDataDir) { - // An explicit dir is decisive in both directions: exact match wins, and a - // different one means this is definitively another browser instance. return samePath(candidate.explicitUserDataDir, preferUserDataDir) ? 4 : 0; } - // No flag: the process uses its default dir. Prefer the one whose executable - // belongs to the same browser family as the target directory. const family = browserNameForUserDataDir(preferUserDataDir).toLowerCase(); const exe = (candidate.exePath ?? "").toLowerCase(); if (!exe) return 1; @@ -171,7 +99,6 @@ export const rankBrowserCandidate = (candidate: BrowserCandidate, preferUserData return 1; }; -/** Extract `--user-data-dir=` from a command line, quoted or bare. */ export const parseUserDataDirFlag = (commandLine: string): string | undefined => { const match = /--user-data-dir=(?:"([^"]*)"|'([^']*)'|(\S+))/.exec(commandLine); if (!match) return undefined; @@ -179,7 +106,6 @@ export const parseUserDataDirFlag = (commandLine: string): string | undefined => return value && value.length > 0 ? value : undefined; }; -// ── Linux ────────────────────────────────────────────────────────────────── const collectLinux = async (): Promise> => { let pids: string[]; @@ -201,7 +127,6 @@ const collectLinux = async (): Promise> => { for (const pid of pids) { let cmdline: string; try { - // /proc//cmdline is NUL-separated argv. cmdline = (await readFile(`/proc/${pid}/cmdline`, "utf8")).replace(/\0/g, " ").trim(); } catch { continue; @@ -211,12 +136,8 @@ const collectLinux = async (): Promise> => { try { link = await readlink(`/proc/${pid}/exe`); } catch { - // /proc//exe is unreadable for processes we don't own. } - // argv[0] backs up the symlink in both directions: unreadable for a process - // we don't own, and pointing at an unlinked binary after an in-place - // upgrade. Chrome's browser process rewrites its argv to the bare - // executable path, so on Linux argv[0] is the live install location. + // Chrome rewrites its argv to the bare executable path, so argv[0] is the live install location when /proc//exe is unreadable or stale. const exePath = (link ? await spawnableExePath(link) : undefined) ?? (await spawnableExePath(cmdline.split(" ")[0] ?? "")); @@ -229,7 +150,6 @@ const collectLinux = async (): Promise> => { return candidates; }; -// ── macOS ────────────────────────────────────────────────────────────────── const collectDarwin = async (): Promise> => { let lines: string[]; @@ -256,7 +176,6 @@ const collectDarwin = async (): Promise> => { const { stdout } = await run("ps", ["-ww", "-o", "args=", "-p", pid], { timeout: EXEC_TIMEOUT_MS }); args = stdout.trim(); } catch { - // args are optional — only the --user-data-dir probe needs them } if (args && hasChildProcessType(args)) continue; const explicit = args ? parseUserDataDirFlag(args) : undefined; @@ -268,7 +187,6 @@ const collectDarwin = async (): Promise> => { return candidates; }; -// ── Windows ──────────────────────────────────────────────────────────────── const POWERSHELL_QUERY = [ "$ErrorActionPreference='SilentlyContinue';", @@ -315,22 +233,17 @@ const collectWin32 = async (): Promise> => { } } } catch { - // PowerShell missing or blocked by policy — fall through to tasklist. } - // Liveness-only fallback: tasklist still ships on every supported Windows. - // It reveals no path, so the candidate is a bare "something is running". try { const { stdout } = await run("tasklist.exe", [], { timeout: EXEC_TIMEOUT_MS, windowsHide: true }); const lower = stdout.toLowerCase(); if (WIN_IMAGE_NAMES.some((n) => lower.includes(n))) return [{}]; } catch { - // best-effort } return []; }; -/** Installed-browser locations to try when the live process reveals no path. */ const winInstallCandidates = (): ReadonlyArray => { const roots = [ process.env["PROGRAMFILES"], @@ -348,7 +261,6 @@ const winInstallCandidates = (): ReadonlyArray => { return roots.flatMap((root) => suffixes.map((suffix) => join(root, suffix))); }; -/** Query the Shell's App Paths registration for a browser's install location. */ const winRegistryExecutable = async (): Promise => { for (const image of WIN_IMAGE_NAMES) { for (const hive of ["HKLM", "HKCU"]) { @@ -362,22 +274,13 @@ const winRegistryExecutable = async (): Promise => { const path = match?.[1]?.trim(); if (path) return path; } catch { - // key absent for this browser/hive — try the next } } } return undefined; }; -// ── Public API ───────────────────────────────────────────────────────────── -/** - * Live-process facts about the running Chromium-family browser. `running` is - * derived only from process evidence, never from an install location. - * - * `preferUserDataDir` should be the dir the harness is connected to; with more - * than one browser running it decides which process is described. - */ export const detectRunningBrowser = async (preferUserDataDir?: string): Promise => { const candidates = process.platform === "darwin" @@ -406,19 +309,10 @@ export const detectRunningBrowser = async (preferUserDataDir?: string): Promise< }; }; -/** True when a Chromium-family browser process is running. */ export const isBrowserRunning = async (): Promise => (await detectRunningBrowser()).running; -/** - * The binary to invoke when opening a window for a chosen profile: the running - * browser's own executable when known, otherwise a documented install location. - * Undefined when nothing usable is found. - */ export const resolveBrowserExecutable = async (running?: RunningBrowser): Promise => { const detected = running ?? (await detectRunningBrowser()); - // Re-check spawnability here too: this function is the single gate every - // launch passes through, and a caller may hand us a RunningBrowser collected - // before the binary was replaced on disk. const usable = detected.exePath ? await spawnableExePath(detected.exePath) : undefined; if (usable) return usable; if (process.platform !== "win32") return undefined; @@ -430,7 +324,6 @@ export const resolveBrowserExecutable = async (running?: RunningBrowser): Promis await access(candidate); return candidate; } catch { - // not installed here } } return undefined; diff --git a/src/profile/command.ts b/src/profile/command.ts index a49e35d..d0c36d6 100644 --- a/src/profile/command.ts +++ b/src/profile/command.ts @@ -1,11 +1,3 @@ -/** - * /browser-profile — choose which browser profile the agent works in. - * - * The selection is durable (see store.ts) and applies immediately: the current - * harness tabs are closed and a new window is opened in the chosen profile, so - * the agent's next action happens where the user just asked for it rather than - * one session later. - */ import type { ExtensionAPI, ExtensionContext } from "@mariozechner/pi-coding-agent"; import type { BrowserClient } from "../client"; @@ -16,11 +8,6 @@ import { browserNameForUserDataDir, userDataDirCandidates } from "./paths"; import { pinFor, promptForProfile } from "./picker"; import { clearPin, readPin, writePin } from "./store"; -/** - * Which user-data-dir the picker should enumerate: the connected browser's - * when known, otherwise a running browser's, otherwise the only installed one - * that actually has profiles. - */ export const resolveUserDataDir = async (client: BrowserClient): Promise> => { const connected = client.userDataDir(); if (connected) return ok(connected); @@ -43,7 +30,6 @@ export const resolveUserDataDir = async (client: BrowserClient): Promise }, string>> => { @@ -56,11 +42,6 @@ const loadProfiles = async ( return ok({ userDataDir: dir.data, profiles }); }; -/** - * Re-seed the live session so the new profile takes effect now. Closing the - * owned tabs and restarting the client makes start() open a window in the - * pinned profile and attach to it. - */ const applyToLiveSession = async (client: BrowserClient): Promise> => { if (!client.status().alive) return ok(undefined); await client.closeOwnedTabs(); @@ -127,11 +108,6 @@ export function registerProfileCommand(pi: ExtensionAPI, client: BrowserClient): }); } -/** - * First-run prompt used by setup: pick a profile when none is pinned yet. - * Returns a note to append to the setup output; the caller always continues, - * because a cancelled or unavailable picker must leave existing behavior intact. - */ export const promptForProfileIfUnset = async ( client: BrowserClient, ctx: ExtensionContext | undefined, diff --git a/src/profile/launch.ts b/src/profile/launch.ts index 0088b6d..a54906d 100644 --- a/src/profile/launch.ts +++ b/src/profile/launch.ts @@ -1,19 +1,5 @@ -/** - * Opening a browser window for a chosen profile. - * - * CDP cannot do this: `Target.createTarget` rejects another profile's - * browserContextId ("Failed to find browser context with id"), and a bare - * createTarget lands in `defaultBrowserContextId`, which follows window focus. - * The only way in is the browser's own command line — Chromium's - * ProcessSingleton hands a second launch's argv to the running browser, which - * "does what it would have done", i.e. opens a window in `--profile-directory`. - * - * The window is then identified by a unique `file://` sentinel page. That - * matters: a bare `about:blank#token` argument is discarded by Chrome (it opens - * chrome://newtab instead), so the token has to ride on a real URL. Polling for - * the sentinel URL — rather than diffing the target list — also keeps - * identification correct when the user opens tabs at the same moment. - */ +// CDP cannot open a window in another profile, so this goes through the command line: ProcessSingleton hands the argv to the running browser. +// The sentinel must ride on a real `file://` URL — Chrome discards a bare `about:blank#token` argument and opens chrome://newtab instead. import { spawn, type ChildProcess } from "node:child_process"; import { randomUUID } from "node:crypto"; @@ -24,23 +10,13 @@ import { pathToFileURL } from "node:url"; import { type Result, err, ok } from "../util/result"; export type ProfileWindowRequest = { - /** Browser binary to invoke. */ readonly exePath: string; - /** Profile subdirectory, e.g. "Profile 1". */ readonly profileDir: string; - /** - * Only set when the RUNNING browser was launched with an explicit - * --user-data-dir. ProcessSingleton keys on this path, so passing one that - * differs by case or normalisation would start a second browser instead of - * delegating to the running one. - */ readonly explicitUserDataDir?: string; }; export type ProfileWindowHandle = { - /** file:// URL that will appear as the new window's page target. */ readonly sentinelUrl: string; - /** Remove the sentinel file. Safe to call more than once. */ readonly cleanup: () => Promise; readonly kill: () => void; }; @@ -49,10 +25,6 @@ const SENTINEL_HTML = (token: string): string => `${token}` + `pi browser harness — opening this profile…`; -/** - * Ask the running browser to open a window in `profileDir`, marked with a - * unique sentinel page the caller can then find over CDP. - */ export const openProfileWindow = async ( req: ProfileWindowRequest, ): Promise> => { @@ -71,26 +43,18 @@ export const openProfileWindow = async ( let child: ChildProcess; try { - // No shell: arguments are passed as an array, so profile names and paths - // containing spaces survive intact on every platform. child = spawn(req.exePath, args, { detached: true, stdio: "ignore" }); } catch (e) { await rm(file, { force: true }).catch(() => {}); return err(`could not launch ${req.exePath}: ${e instanceof Error ? e.message : String(e)}`); } - // spawn() reports a missing or non-executable binary ASYNCHRONOUSLY, as an - // 'error' event — it does not throw. Ignoring that event made an ENOENT - // indistinguishable from a browser that simply never opened the window, so - // the caller's 15s sentinel poll reported a timeout for what was really a bad - // executable path. Node guarantees exactly one of 'spawn' or 'error', so - // waiting for it costs nothing and turns the failure into a named cause. + // spawn() reports a bad binary asynchronously as an 'error' event rather than throwing; unhandled, an ENOENT looked like the 15s sentinel timeout. const spawnFailure = await new Promise((resolve) => { child.once("spawn", () => resolve(undefined)); child.once("error", (e: Error) => resolve(e.message)); }); - // A later 'error' event with no listener would throw; keep one attached for - // the lifetime of the detached launcher. + // A later 'error' event with no listener would throw, so keep one attached for the detached launcher's lifetime. child.on("error", () => {}); child.unref(); diff --git a/src/profile/list.ts b/src/profile/list.ts index 3915a98..6429b91 100644 --- a/src/profile/list.ts +++ b/src/profile/list.ts @@ -1,23 +1,3 @@ -/** - * Browser profile enumeration. - * - * A Chromium user-data-dir registers its profiles in `Local State` under the - * JSON key `profile.info_cache`, with `profile.profiles_order` giving the - * user's own ordering and `profile.last_used` naming the active one. Those - * keys are declared in Chromium's pref_names.h with no platform BUILDFLAG, so - * the layout is identical on Linux, macOS, and Windows (the C++ constant is - * now `kProfileAttributes`; the on-disk key never changed). - * - * Enumeration is two-tier so a Chromium fork or a future schema change cannot - * blank the list: - * tier 1 — `Local State` → `profile.info_cache` (authoritative: display - * names, accounts, ordering) - * tier 2 — scan subdirectories holding a `Preferences` file and read - * `profile.name` + `account_info[0].email` - * - * Nothing here throws: an unreadable or malformed file yields an empty list so - * callers degrade to the harness's pre-existing behavior. - */ import { readdir, readFile } from "node:fs/promises"; import { join } from "node:path"; @@ -26,28 +6,18 @@ import { Compile } from "typebox/compile"; import { parseJson } from "../schemas/parse"; export type BrowserProfile = { - /** Profile subdirectory name, e.g. "Default" or "Profile 1". */ readonly dir: string; - /** Display name as shown in the browser's profile menu. */ readonly name: string; - /** Signed-in account address, when the profile has one. */ readonly email?: string; - /** Picker label — "Name (email)", or "Name (dir)" with no account. */ readonly label: string; - /** True for the profile the browser reports as last used. */ readonly lastUsed: boolean; }; -/** Directories inside a user-data-dir that are not user profiles. */ const NON_PROFILE_DIRS: ReadonlySet = new Set([ "System Profile", "Guest Profile", ]); -/** - * Picker label. The bracketed half is the account address when there is one, - * and the profile directory otherwise, so every row keeps the same shape. - */ export const formatProfileLabel = (name: string, email: string | undefined, dir: string): string => `${name} (${email && email.length > 0 ? email : dir})`; @@ -72,10 +42,6 @@ const asRecord = (v: unknown): Record | null => const asString = (v: unknown): string | undefined => typeof v === "string" && v.length > 0 ? v : undefined; -/** - * Order profiles the way the browser does: `profiles_order` first (in its own - * sequence), then anything unlisted, alphabetically. - */ const orderDirs = (dirs: ReadonlyArray, order: ReadonlyArray): ReadonlyArray => { const known = order.filter((d) => dirs.includes(d)); const rest = dirs.filter((d) => !known.includes(d)).sort((a, b) => a.localeCompare(b)); @@ -98,7 +64,6 @@ const buildProfile = ( }; }; -/** Tier 1 — `Local State` → `profile.info_cache`. */ const listFromLocalState = async (userDataDir: string): Promise> => { const localState = await readJsonFile(join(userDataDir, "Local State")); if (!localState) return []; @@ -116,8 +81,7 @@ const listFromLocalState = async (userDataDir: string): Promise { const entry = byDir.get(dir); if (!entry) return []; - // `name` is the user-visible profile name; `gaia_name` is the Google - // account's own display name and is the better fallback when a profile - // still carries an auto-generated name like "Person 1". const name = asString(entry["name"]) ?? asString(entry["gaia_name"]); const email = asString(entry["user_name"]); return [buildProfile(dir, name, email, dir === lastUsed)]; }); }; -/** Tier 2 — directory scan, reading each profile's own `Preferences`. */ const listFromPreferencesScan = async (userDataDir: string): Promise> => { let dirents; try { @@ -154,7 +114,7 @@ const listFromPreferencesScan = async (userDataDir: string): Promise> => { const fromLocalState = await listFromLocalState(userDataDir); if (fromLocalState.length > 0) return fromLocalState; diff --git a/src/profile/paths.ts b/src/profile/paths.ts index 095687e..35cbc1f 100644 --- a/src/profile/paths.ts +++ b/src/profile/paths.ts @@ -1,92 +1,39 @@ -/** - * Filesystem locations for browser profile discovery and pin persistence. - * - * Two independent path families live here: - * - * 1. Chromium user-data-dir candidates, per platform and channel. A - * user-data-dir holds `Local State` (the profile registry) and - * `DevToolsActivePort`; individual profiles are subdirectories of it - * (`Default`, `Profile 1`, …). - * Reference: https://chromium.googlesource.com/chromium/src/+/HEAD/docs/user_data_dir.md - * - * 2. pi's agent config dir, where the profile pin is stored. - * - * On the agent dir: pi exports `getAgentDir()` from - * `@mariozechner/pi-coding-agent`, but importing it would be a RUNTIME import - * of the host package. Extensions installed via `pi install npm:...` do not - * get pi resolvable from their own node_modules (verified: the installed - * tree's `@mariozechner/` directory is empty), which is why every other import - * in this codebase is `import type` and erased at build time. So the three - * lines of `getAgentDir()` are replicated here instead — env var first, then - * `~/.pi/agent` (pi's dist/config.js: `PI_CODING_AGENT_DIR`, CONFIG_DIR_NAME - * `.pi`). - */ +// pi's `getAgentDir()` is replicated rather than imported: a runtime import of the host package does not resolve for extensions installed via `pi install npm:...`. import { homedir } from "node:os"; import { join } from "node:path"; -// ── pi agent config dir ──────────────────────────────────────────────────── -/** pi's env override for the agent dir (APP_NAME.toUpperCase() + "_CODING_AGENT_DIR"). */ const ENV_AGENT_DIR = "PI_CODING_AGENT_DIR"; -/** - * Expand a leading `~` exactly as pi's expandTildePath does — `~` and `~/…` - * only, never `~\…`, on every platform. This deliberately mirrors pi rather - * than improving on it: the pin has to land in the directory pi itself resolves - * from the same variable, so being more lenient here would put the file - * somewhere pi does not look. - */ +// Deliberately mirrors pi's expandTildePath (never `~\…`) rather than improving on it, or the pin lands somewhere pi does not look. const expandTildePiCompatible = (path: string): string => { if (path === "~") return homedir(); if (path.startsWith("~/")) return homedir() + path.slice(1); return path; }; -/** - * Tilde expansion for values this package interprets on its own. Windows users - * write `~\dir` as naturally as `~/dir`, and no compatibility constraint - * applies, so both are accepted. - */ const expandTildeLenient = (path: string): string => { if (path === "~") return homedir(); if (path.startsWith("~/") || path.startsWith("~\\")) return join(homedir(), path.slice(2)); return path; }; -/** pi's agent config directory — `$PI_CODING_AGENT_DIR` or `~/.pi/agent`. */ export const agentDir = (): string => { const fromEnv = process.env[ENV_AGENT_DIR]; if (fromEnv) return expandTildePiCompatible(fromEnv); return join(homedir(), ".pi", "agent"); }; -/** Where the selected-profile pin is persisted. */ export const pinFilePath = (): string => join(agentDir(), "browser-harness.json"); -// ── Chromium user-data-dir candidates ────────────────────────────────────── -/** - * Windows resolves its per-user data root from %LOCALAPPDATA%, which can be - * redirected on roaming / managed accounts. Fall back to the conventional - * layout only when the variable is absent. - */ const localAppData = (): string => process.env["LOCALAPPDATA"] ?? join(homedir(), "AppData", "Local"); -/** - * Every known default user-data-dir for Chromium-family browsers on the - * current platform. Order is stable but carries no priority — callers probe - * all of them. - * - * Includes paths the harness previously missed: snap Chromium (the default - * Chromium on Ubuntu) and the Linux-only `$CHROME_USER_DATA_DIR` override - * documented in user_data_dir.md. - */ export const userDataDirCandidates = (): ReadonlyArray => { const home = homedir(); const dirs: string[] = []; - // A user-set override wins a spot at the front regardless of platform. const envOverride = process.env["CHROME_USER_DATA_DIR"]; if (envOverride) dirs.push(expandTildeLenient(envOverride)); @@ -135,9 +82,7 @@ export const userDataDirCandidates = (): ReadonlyArray => { join(home, ".config/microsoft-edge"), join(home, ".config/microsoft-edge-beta"), join(home, ".config/microsoft-edge-dev"), - // snap (Ubuntu's default Chromium) — confined to ~/snap//common join(home, "snap/chromium/common/chromium"), - // flatpak join(home, ".var/app/org.chromium.Chromium/config/chromium"), join(home, ".var/app/com.google.Chrome/config/google-chrome"), join(home, ".var/app/com.brave.Browser/config/BraveSoftware/Brave-Browser"), @@ -145,15 +90,9 @@ export const userDataDirCandidates = (): ReadonlyArray => { ); } - // De-duplicate while preserving order — an env override may repeat a default. return Array.from(new Set(dirs)); }; -/** - * Human-readable browser name for a user-data-dir, used to disambiguate the - * picker when more than one browser is installed. Falls back to the directory - * name, which is already descriptive for every path we generate. - */ export const browserNameForUserDataDir = (userDataDir: string): string => { const p = userDataDir.replace(/\\/g, "/"); const has = (needle: string): boolean => p.toLowerCase().includes(needle); diff --git a/src/profile/picker.ts b/src/profile/picker.ts index b4df059..d2a057b 100644 --- a/src/profile/picker.ts +++ b/src/profile/picker.ts @@ -1,17 +1,4 @@ -/** - * The profile picker, shared by /browser-profile and first-run setup so both - * show the user exactly the same list. - * - * Rendering rules: - * - one row per profile, labelled "Name (email)" — or "Name (Profile 3)" when - * the profile has no signed-in account, so every row keeps its shape - * - the pinned profile is marked with a check - * - a final row clears the selection, since a pin must be undoable - * - * `ctx.ui.select` resolves to undefined when the user presses Escape AND in - * non-interactive modes (pi's print/RPC UI context returns undefined for every - * dialog), so callers must treat "no answer" as "change nothing". - */ +// `ctx.ui.select` resolves to undefined both on Escape and in non-interactive modes, so "no answer" must mean "change nothing". import type { ExtensionContext } from "@mariozechner/pi-coding-agent"; import type { BrowserProfile } from "./list"; @@ -24,21 +11,12 @@ export type ProfileChoice = | { readonly kind: "clear" } | { readonly kind: "cancelled" }; -/** - * Two profiles can legitimately share a name and account (the same person - * signed into two profiles). Labels must stay distinct or the selection cannot - * be mapped back, so a repeated label gains its directory. - */ const disambiguate = (profiles: ReadonlyArray): ReadonlyArray => { const counts = new Map(); for (const p of profiles) counts.set(p.label, (counts.get(p.label) ?? 0) + 1); return profiles.map((p) => ((counts.get(p.label) ?? 0) > 1 ? `${p.label} [${p.dir}]` : p.label)); }; -/** - * Show the picker and report what the user chose. Returns "cancelled" when - * there is no UI or the user dismissed the dialog. - */ export const promptForProfile = async ( ctx: ExtensionContext, profiles: ReadonlyArray, @@ -64,7 +42,6 @@ export const promptForProfile = async ( return { kind: "profile", profile }; }; -/** Build the durable pin for a chosen profile. */ export const pinFor = (userDataDir: string, profile: BrowserProfile): ProfilePin => ({ userDataDir, profileDir: profile.dir, diff --git a/src/profile/store.ts b/src/profile/store.ts index e38ff2e..6e1b342 100644 --- a/src/profile/store.ts +++ b/src/profile/store.ts @@ -1,19 +1,4 @@ -/** - * Durable persistence for the selected browser profile. - * - * The pin must outlive session termination, so it cannot live in pi's session - * entries (`pi.appendEntry`, see src/state.ts) — those die with the session. - * It is a small JSON file in pi's agent config dir instead, shared by every - * project on the machine. - * - * The pin deliberately stores only `{ userDataDir, profileDir }`. A - * `browserContextId` is NOT persisted: Chrome mints fresh context ids on every - * browser run, so a stored one would be a stale pointer at best and a pointer - * into the wrong profile at worst. The context is re-resolved on each connect. - * - * Every read failure degrades to "no pin" rather than throwing — a corrupt - * file must never break browser control. - */ +// A `browserContextId` is deliberately NOT persisted: Chrome mints fresh context ids per browser run, so a stored one could point into the wrong profile. import { mkdir, readFile, rename, unlink, writeFile } from "node:fs/promises"; import { randomUUID } from "node:crypto"; @@ -25,13 +10,9 @@ import { type Result, err, ok } from "../util/result"; import { pinFilePath } from "./paths"; export type ProfilePin = { - /** User-data-dir the profile belongs to — guards against browser switches. */ readonly userDataDir: string; - /** Profile subdirectory, e.g. "Profile 1". */ readonly profileDir: string; - /** Label captured at selection time, for display without a disk re-read. */ readonly label: string; - /** ISO timestamp of the selection. */ readonly savedAt: string; }; @@ -63,11 +44,6 @@ const PinFileSchema = Type.Object( const pinFileValidator = Compile(PinFileSchema); -/** - * The persisted pin, or null when nothing is pinned, the file is missing, or - * it cannot be parsed. A file written by a newer version is treated as "no - * pin" rather than guessed at. - */ export const readPin = async (): Promise => { let raw: string; try { @@ -88,10 +64,6 @@ export const readPin = async (): Promise => { }; }; -/** - * Replace the pin atomically: a sibling temp file plus rename, so a crash - * mid-write can never leave a half-written pin behind. - */ const writePinFile = async (profile: ProfilePin | null): Promise> => { const target = pinFilePath(); const tmp = `${target}.${randomUUID()}.tmp`; @@ -107,8 +79,6 @@ const writePinFile = async (profile: ProfilePin | null): Promise> => writePinFile(pin); -/** Clear the selection, restoring the harness's unpinned behavior. */ export const clearPin = (): Promise> => writePinFile(null); -- cgit v1.2.3