From 19573c984d56c927669d1179471febb6f512a4ee Mon Sep 17 00:00:00 2001 From: justinTM <9123665+justinTM@users.noreply.github.com> Date: Tue, 28 Jul 2026 11:25:45 -0700 Subject: [PATCH 1/2] fix: isolate ProofShot session lifecycle --- PROOFSHOT.md | 3 + README.md | 9 + content/docs/concepts/how-it-works.mdx | 19 +- content/docs/faq.mdx | 7 +- content/docs/reference/cli.mdx | 22 +- proofshot-spec.md | 37 +- skills/claude/SKILL.md | 2 +- skills/codex/SKILL.md | 2 +- skills/cursor/proofshot.mdc | 2 +- skills/generic/PROOFSHOT.md | 2 +- skills/opencode/SKILL.md | 2 +- src/artifacts/viewer.ts | 23 +- src/browser/discovery.test.ts | 57 +++ src/browser/discovery.ts | 142 ++++++ src/browser/runtime.test.ts | 57 +++ src/browser/runtime.ts | 89 ++++ src/cli.ts | 3 +- src/commands/clean.test.ts | 45 ++ src/commands/clean.ts | 12 + src/commands/doctor.test.ts | 5 +- src/commands/doctor.ts | 10 +- src/commands/exec.ts | 36 +- src/commands/lifecycle.integration.test.ts | 493 +++++++++++++++++++++ src/commands/start.test.ts | 75 +++- src/commands/start.ts | 217 +++++---- src/commands/stop.test.ts | 221 +++++++++ src/commands/stop.ts | 229 ++++++++-- src/server/start.test.ts | 96 ++++ src/server/start.ts | 148 ++++--- src/session/lifecycle.test.ts | 85 ++++ src/session/lifecycle.ts | 78 ++++ src/session/state.test.ts | 25 +- src/session/state.ts | 65 ++- src/utils/exec.ts | 15 +- src/utils/process.test.ts | 71 +++ src/utils/process.ts | 238 +++++++++- 36 files changed, 2360 insertions(+), 282 deletions(-) create mode 100644 src/browser/discovery.test.ts create mode 100644 src/browser/discovery.ts create mode 100644 src/browser/runtime.test.ts create mode 100644 src/browser/runtime.ts create mode 100644 src/commands/clean.test.ts create mode 100644 src/commands/lifecycle.integration.test.ts create mode 100644 src/commands/stop.test.ts create mode 100644 src/server/start.test.ts create mode 100644 src/session/lifecycle.test.ts create mode 100644 src/session/lifecycle.ts diff --git a/PROOFSHOT.md b/PROOFSHOT.md index 905ca26..a4d6746 100644 --- a/PROOFSHOT.md +++ b/PROOFSHOT.md @@ -11,6 +11,8 @@ After building or modifying UI features, verify with this workflow: ProofShot keeps all `proofshot exec` commands inside the same isolated `agent-browser` session that was created by `proofshot start`, so recording, screenshots, and browser actions stay aligned. +Use `--url` on `start` when verification must begin on a specific target. In an isolated HOME, ProofShot discovers executable-only Chrome/Chromium installs from system/account locations; use `--browser-executable /absolute/path/to/chrome` to select one explicitly. + Key proofshot exec commands: - `proofshot exec snapshot -i` — see interactive elements - `proofshot exec click @e3` — click an element @@ -18,6 +20,7 @@ Key proofshot exec commands: - `proofshot exec screenshot step.png` — capture a moment Artifacts saved to ./proofshot-artifacts/ including video, screenshots, errors, and summary. +Custom `--output` paths do not move active control state, so a separate `proofshot stop` still finds the session. `stop` is idempotent; after `stop --no-close`, run a later plain `stop` to close that exact retained browser without rebundling. You can customize browser launch behavior in `proofshot.config.json`, including HTTPS error ignoring, a custom browser executable path, and a project-specific `agent-browser` config path. Use `proofshot doctor` when the local setup looks wrong. It prints the current config path, browser mode, viewport, installed binaries, and any active ProofShot session. diff --git a/README.md b/README.md index d157a65..fa4be6c 100644 --- a/README.md +++ b/README.md @@ -140,6 +140,7 @@ proofshot start # Server already running proofshot start --run "npm run dev" --port 3000 # Start and capture server proofshot start --description "Verify checkout flow" # Add description to report proofshot start --url http://localhost:3000/login # Open specific URL +proofshot start --browser-executable /path/to/chrome # Reuse an exact browser binary proofshot start --headed # Show browser (debugging) proofshot start --force # Override a stale session from a previous crash ``` @@ -158,6 +159,10 @@ You can also configure browser launch behavior in `proofshot.config.json`: Set `browser.configPath` when you need ProofShot to run `agent-browser` against a project-specific config instead of inheriting `~/.agent-browser/config.json`. Relative paths are resolved from the directory that contains `proofshot.config.json`. +ProofShot discovers system and account-level Chrome/Chromium installs even when the command runs with an isolated `HOME`. If no runnable browser is found, `start` prints the exact `agent-browser install` action. An explicit `--browser-executable` takes precedence for one run. + +`--output` changes only where evidence is written. Active control state stays in the configured/default output directory, so later `proofshot exec` and `proofshot stop` processes can find the same session. + ### `proofshot stop` Stop recording, collect errors, generate proof artifacts. @@ -167,6 +172,8 @@ proofshot stop # Stop session and close browser proofshot stop --no-close # Stop but keep browser open ``` +`stop` is idempotent. With `--no-close`, ProofShot retains exact ownership metadata after bundling; run a later plain `proofshot stop` to close that browser without rebuilding the artifacts. + ### `proofshot exec` Pass-through to agent-browser with automatic session logging. Captures timestamps, element data, and resolves screenshot paths. @@ -211,6 +218,8 @@ Remove the `./proofshot-artifacts/` directory. proofshot clean ``` +`clean` refuses while active or retained session control state exists. Run `proofshot stop` first so ProofShot does not discard exact process ownership metadata. + ### `proofshot doctor` Print the current ProofShot environment, including config path, browser mode, viewport, installed binaries, and any active session. diff --git a/content/docs/concepts/how-it-works.mdx b/content/docs/concepts/how-it-works.mdx index 6bdac70..1494300 100644 --- a/content/docs/concepts/how-it-works.mdx +++ b/content/docs/concepts/how-it-works.mdx @@ -45,11 +45,11 @@ ProofShot uses a three-phase model. `proofshot start` initializes the session: 1. Check if the port is available (fail fast on conflicts) -2. Spawn the dev server if `--run` is provided, pipe output to `server.log` +2. Spawn an isolated dev-server process session if `--run` is provided, pipe timestamped output to `server.log`, and persist its immutable PID/process-group identity 3. Wait for the port to respond (polls every 500ms, 30s timeout) -4. Open headless Chromium +4. Open the requested URL in a short, collision-safe agent-browser session and persist its daemon identity 5. Start video recording -6. Write `.session.json` (active session state) and `metadata.json` (git branch/commit, persists after stop) +6. Write `.session.json` to the configured/default control directory and `metadata.json` beside the evidence (git branch/commit, persists after stop) Recording is mandatory. If it fails after 3 retries, the session aborts. @@ -69,11 +69,12 @@ Each `proofshot exec` call: 1. Collects browser console errors and output (point-in-time snapshot) 2. Stops video recording -3. Closes the browser -4. Trims video dead time using ffmpeg (5s buffer before first action, 3s after last). Adjusts all `session-log.json` timestamps by the trim offset. -5. Scans `server.log` with multi-language regex patterns for errors -6. Generates `SUMMARY.md` and `viewer.html` -7. Clears `.session.json` +3. Closes the exact owned browser session +4. Stops only the dev-server process session created by this run +5. Trims video dead time using ffmpeg (5s buffer before first action, 3s after last). Adjusts all `session-log.json` timestamps by the trim offset. +6. Scans `server.log` with multi-language regex patterns for errors +7. Generates `SUMMARY.md` and `viewer.html` +8. Clears `.session.json` (or retains exact browser ownership after `--no-close`) ## Design principles @@ -81,6 +82,6 @@ Each `proofshot exec` call: **Minimal dependencies.** Three production dependencies: `commander`, `chalk`, `detect-port`. agent-browser is an optional peer dependency. Small install, small supply chain. -**Session isolation.** `.session.json` lives in the output directory, not globally. This supports parallel sessions in different projects. +**Session isolation.** Each project keeps control state in its configured/default output directory. A CLI-only custom evidence path cannot hide the session from a later process, while different projects still run independently. **ESM-only.** All imports use explicit `.js` extensions for correct resolution after TypeScript compilation. diff --git a/content/docs/faq.mdx b/content/docs/faq.mdx index cd0799d..9dd71cd 100644 --- a/content/docs/faq.mdx +++ b/content/docs/faq.mdx @@ -28,7 +28,7 @@ The skill file installed by `proofshot install` teaches your agent the three-ste Stable handles to interactive elements on a page. When your agent runs `agent-browser snapshot -i`, it gets a list like `@e1: button "Submit"`, `@e2: input "Email"`. These references persist across commands within a session, so the agent can target elements reliably without CSS selectors. **Can I run multiple sessions at the same time?** -Yes. Session state (`.session.json`) lives in the output directory, not globally. Different projects with different output directories can run sessions concurrently. +Yes. Session state (`.session.json`) lives in each project's configured/default output directory, not globally. Different projects can run concurrently. A one-run `--output` override moves evidence without changing where that project finds active control state. **What languages does error detection support?** JavaScript/Node.js, Python, Ruby/Rails, Go, Java/Kotlin, Rust, PHP, C#/.NET, Elixir/Phoenix, plus generic patterns for `FATAL`, `CRITICAL`, and segfaults. See [How to add error patterns](/docs/guides/add-error-pattern) to extend support. @@ -39,7 +39,10 @@ When ffmpeg is available, `proofshot stop` cuts dead time from the video — kee ## Troubleshooting **"No active session" when running exec or stop** -You need to run `proofshot start` first. Each session writes `.session.json` — if it's missing, there's no active session to operate on. +You need to run `proofshot start` before `exec`. `stop` is idempotent, so it succeeds without changing artifacts when the session is already stopped. + +**Chrome is installed, but an isolated HOME cannot find it** +ProofShot checks system paths and executable-only browser caches under the real account home without reusing a browser profile or storage. You can also pass one exact path with `proofshot start --browser-executable /absolute/path/to/chrome`. If nothing is runnable, run the `agent-browser install` command printed by `proofshot start`. **Server errors aren't being detected** Server log capture only works when ProofShot starts the server itself via `--run`. If your server was already running on the port, ProofShot skips spawning and gets no logs. diff --git a/content/docs/reference/cli.mdx b/content/docs/reference/cli.mdx index a2355f0..7f6a8ae 100644 --- a/content/docs/reference/cli.mdx +++ b/content/docs/reference/cli.mdx @@ -48,6 +48,8 @@ proofshot start [options] | `--description ` | Description of what you're verifying (appears in reports) | — | | `--headed` | Show the browser window (visible Chromium) | `false` | | `--output ` | Custom output directory for artifacts | `./proofshot-artifacts` | +| `--browser-executable ` | Use an exact Chrome/Chromium executable | auto-discovered | +| `--force` | Clean up and replace an active session | `false` | **Examples:** @@ -55,16 +57,17 @@ proofshot start [options] proofshot start # Server already running on port 3000 proofshot start --run "npm run dev" --port 3000 # Start server, capture logs proofshot start --url http://localhost:3000/login # Open a specific page +proofshot start --browser-executable /path/to/chrome # Use an exact browser binary proofshot start --description "Verify checkout flow" # Add description to report proofshot start --headed # Show the browser window ``` **What happens:** -1. If `--run` is provided: starts the dev server, pipes output to `server.log`, waits for the port +1. If `--run` is provided: fails without killing anything when the port is occupied; otherwise starts an owned dev-server process session, pipes timestamped output to `server.log`, and waits for the port 2. Opens headless Chromium via agent-browser 3. Navigates to `--url` (or `http://localhost:`) 4. Starts video recording (retries up to 3 times) -5. Writes `.session.json` and `metadata.json` (git branch and commit SHA) +5. Writes control `.session.json` to the configured/default output and durable `metadata.json` beside the evidence. A CLI-only `--output` changes evidence placement, not control discovery. --- @@ -83,11 +86,14 @@ proofshot stop [options] **What happens:** 1. Collects console errors and output from the browser 2. Stops video recording -3. Closes the browser (unless `--no-close`) -4. Trims video dead time (requires ffmpeg): 5s buffer before first action, 3s after last -5. Scans `server.log` for errors across 10+ languages -6. Generates `SUMMARY.md` and `viewer.html` -7. Clears `.session.json` +3. Closes the exact owned browser session (unless `--no-close`) +4. Stops only the dev-server process session created by this ProofShot start +5. Trims video dead time (requires ffmpeg): 5s buffer before first action, 3s after last +6. Scans `server.log` for errors across 10+ languages +7. Generates `SUMMARY.md` and `viewer.html` +8. Clears `.session.json`, or retains it after `--no-close` until a later plain `stop` closes that exact browser + +Repeated `stop` calls are successful no-ops. If bundling fails, control state remains retryable; a later `stop` reuses already-collected artifacts instead of widening process cleanup. --- @@ -187,4 +193,4 @@ Remove the entire artifacts directory. proofshot clean ``` -Deletes `./proofshot-artifacts/` (or the configured output directory). No flags. +Deletes `./proofshot-artifacts/` (or the configured output directory). No flags. If active or retained control state exists, `clean` refuses and asks you to run `proofshot stop` first so exact process ownership metadata is not discarded. diff --git a/proofshot-spec.md b/proofshot-spec.md index 93a55f3..a50619e 100644 --- a/proofshot-spec.md +++ b/proofshot-spec.md @@ -303,6 +303,8 @@ proofshot clean # Removes ./proofshot-artifacts/ ``` +If `.session.json` exists, `clean` refuses and directs the user to `proofshot stop`; it never discards exact process ownership metadata or performs implicit broad cleanup. + ### `proofshot pr` Format artifacts for inclusion in a PR description. @@ -362,22 +364,26 @@ async function ensureDevServer(config, errorLogPath: string) { ## 6. Session State -ProofShot uses a `.session.json` file in the output directory to track the active session: +ProofShot uses a `.session.json` file in the configured/default output directory to track the active session. A CLI-only `--output` override moves evidence but not this discoverable control file: ```json { "startedAt": "2026-02-25T14:32:00.000Z", "description": "Login form: fill credentials, submit, verify redirect", - "outputDir": "./proofshot-artifacts", - "videoPath": "./proofshot-artifacts/session-2026-02-25.webm", - "serverErrorLog": "./proofshot-artifacts/server-errors.log", + "outputDir": "/audit/custom-evidence", + "sessionDir": "/audit/custom-evidence/2026-02-25_login-form", + "sessionName": "ps-2026-02-a1b2c3d4e5f6", + "targetUrl": "http://localhost:5173/login", + "agentBrowserSocketDir": "/run/user/1000/proofshot/agent-browser", + "videoPath": "/audit/custom-evidence/2026-02-25_login-form/session.webm", + "serverErrorLog": "/audit/custom-evidence/2026-02-25_login-form/server.log", "port": 5173, - "framework": "Vite", - "pid": 12345 + "serverProcess": { "pid": 12345, "processGroupId": 12345, "sessionId": 12345, "startTime": "987654" }, + "browserProcess": { "pid": 12367, "processGroupId": 12367, "sessionId": 12367, "startTime": "987699" } } ``` -`proofshot stop` reads this file to know where to find artifacts and what metadata to include in the summary. +`proofshot exec` and `proofshot stop` read this file from separate CLI processes. Cleanup verifies the immutable identities and signals only process groups inside the recorded process sessions; it never kills by command name or occupied port. --- @@ -568,8 +574,8 @@ function ab(command: string): string { proofshot start: 1. Load config 2. Ensure output dir exists - 3. Start dev server (if needed), piping stderr to server-errors.log - 4. Open browser via agent-browser + 3. Fail actionably if the requested port is occupied; otherwise start an owned dev-server process session and timestamp output in server.log + 4. Open the requested URL in a short, collision-safe agent-browser session and persist its daemon identity 5. Start recording via agent-browser 6. Write .session.json with metadata 7. Print instructions for the agent @@ -581,12 +587,13 @@ proofshot stop: 2. Collect console errors via agent-browser errors 3. Collect console output via agent-browser console 4. Stop recording via agent-browser record stop - 5. Close browser via agent-browser close - 6. Read server-errors.log - 7. List all screenshots in output dir - 8. Generate SUMMARY.md - 9. Delete .session.json - 10. Print summary to stdout + 5. Close the exact browser session via agent-browser close + 6. Stop only the owned dev-server process session + 7. Read server.log + 8. List all screenshots in the evidence session dir + 9. Generate SUMMARY.md and viewer.html + 10. Delete .session.json (or retain exact browser ownership after --no-close) + 11. Print summary to stdout ``` ### Server Error Capture diff --git a/skills/claude/SKILL.md b/skills/claude/SKILL.md index 395178f..14c59fc 100644 --- a/skills/claude/SKILL.md +++ b/skills/claude/SKILL.md @@ -27,7 +27,7 @@ Use ProofShot after: proofshot start --run "your-dev-command" --port PORT --description "what you are about to verify" ``` -This opens a browser and begins recording. If the port is already in use, proofshot will kill the existing process automatically. +This opens a browser and begins recording. If the port is already in use, ProofShot leaves that unowned listener alone and asks you to choose another port or stop it explicitly. **Always use `--run`** to let proofshot start and capture your dev server output (server logs appear in the proof report). Only omit `--run` if the server was explicitly started by the user or another process — without it, no server logs are captured. diff --git a/skills/codex/SKILL.md b/skills/codex/SKILL.md index 2feda3d..07048f2 100644 --- a/skills/codex/SKILL.md +++ b/skills/codex/SKILL.md @@ -26,7 +26,7 @@ Use ProofShot after: proofshot start --run "your-dev-command" --port PORT --description "what you are about to verify" ``` -This opens a browser and begins recording. If the port is already in use, proofshot will kill the existing process automatically. +This opens a browser and begins recording. If the port is already in use, ProofShot leaves that unowned listener alone and asks you to choose another port or stop it explicitly. **Always use `--run`** to let proofshot start and capture your dev server output (server logs appear in the proof report). Only omit `--run` if the server was explicitly started by the user or another process — without it, no server logs are captured. diff --git a/skills/cursor/proofshot.mdc b/skills/cursor/proofshot.mdc index 49ca396..5cb308e 100644 --- a/skills/cursor/proofshot.mdc +++ b/skills/cursor/proofshot.mdc @@ -9,7 +9,7 @@ After modifying UI files, visually verify changes with this workflow: 1. Start session: `proofshot start --run "your-dev-command" --port PORT --description "what you are verifying"` Always use --run so proofshot captures server logs. Only omit --run if the server was started by the user. - If the port is in use, proofshot kills the existing process. Add --force if a stale session blocks startup. + If the port is in use, ProofShot leaves that unowned listener alone; choose another port or stop it explicitly. Add --force only if a stale ProofShot session blocks startup. 2. Drive browser: Use `proofshot exec` commands to navigate, click, fill forms, and take screenshots 3. Stop session: `proofshot stop` to bundle video + screenshots + error report diff --git a/skills/generic/PROOFSHOT.md b/skills/generic/PROOFSHOT.md index c8da9e8..43798b3 100644 --- a/skills/generic/PROOFSHOT.md +++ b/skills/generic/PROOFSHOT.md @@ -6,7 +6,7 @@ After building or modifying UI features, verify with this workflow: 1. Start: `proofshot start --run "your-dev-command" --port PORT --description "what you are verifying"` Always use --run so proofshot captures server logs. Only omit --run if the server was started by the user. - If the port is in use, proofshot kills the existing process. Add --force if a stale session blocks startup. + If the port is in use, ProofShot leaves that unowned listener alone; choose another port or stop it explicitly. Add --force only if a stale ProofShot session blocks startup. 2. Test: Use `proofshot exec` to navigate, click, fill forms, take screenshots 3. Stop: `proofshot stop` — bundles video, screenshots, and error report diff --git a/skills/opencode/SKILL.md b/skills/opencode/SKILL.md index 6d98489..b5b9eff 100644 --- a/skills/opencode/SKILL.md +++ b/skills/opencode/SKILL.md @@ -27,7 +27,7 @@ Use ProofShot after: proofshot start --run "your-dev-command" --port PORT --description "what you are about to verify" ``` -This opens a browser and begins recording. If the port is already in use, proofshot will kill the existing process automatically. +This opens a browser and begins recording. If the port is already in use, ProofShot leaves that unowned listener alone and asks you to choose another port or stop it explicitly. **Always use `--run`** to let proofshot start and capture your dev server output (server logs appear in the proof report). Only omit `--run` if the server was explicitly started by the user or another process - without it, no server logs are captured. diff --git a/src/artifacts/viewer.ts b/src/artifacts/viewer.ts index 13e7c53..7df6b72 100644 --- a/src/artifacts/viewer.ts +++ b/src/artifacts/viewer.ts @@ -14,6 +14,7 @@ interface ViewerData { videoFilename: string | null; entries: SessionLogEntry[]; consoleErrorCount: number; + consoleEvidenceAvailable?: boolean; serverErrorCount: number; consoleOutput?: string; serverLog?: string; @@ -157,9 +158,15 @@ export function generateViewer(data: ViewerData): string { ? `

${escapeHtml(data.description)}

` : ''; - const consoleBadgeClass = data.consoleErrorCount === 0 ? 'clean' : 'has-errors'; - const consoleBadgeText = - data.consoleErrorCount === 0 + const consoleEvidenceAvailable = data.consoleEvidenceAvailable !== false; + const consoleBadgeClass = !consoleEvidenceAvailable + ? 'unavailable' + : data.consoleErrorCount === 0 + ? 'clean' + : 'has-errors'; + const consoleBadgeText = !consoleEvidenceAvailable + ? 'Console: unavailable' + : data.consoleErrorCount === 0 ? 'Console: clean' : `Console: ${data.consoleErrorCount} error(s)`; @@ -458,6 +465,12 @@ export function generateViewer(data: ViewerData): string { border: 1px solid rgba(248, 81, 73, 0.25); } + .error-badge.unavailable { + background: rgba(210, 153, 34, 0.12); + color: #d29922; + border: 1px solid rgba(210, 153, 34, 0.25); + } + .error-badge .badge-dot { width: 6px; height: 6px; @@ -472,6 +485,10 @@ export function generateViewer(data: ViewerData): string { background: #f85149; } + .error-badge.unavailable .badge-dot { + background: #d29922; + } + .viewer { display: flex; height: calc(100vh - 180px); diff --git a/src/browser/discovery.test.ts b/src/browser/discovery.test.ts new file mode 100644 index 0000000..4ba5434 --- /dev/null +++ b/src/browser/discovery.test.ts @@ -0,0 +1,57 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { discoverBrowserExecutable } from './discovery.js'; + +const createdRoots: string[] = []; + +function createRoot(): string { + const cache = path.join(os.userInfo().homedir, '.cache'); + fs.mkdirSync(cache, { recursive: true }); + const root = fs.mkdtempSync(path.join(cache, 'proofshot-browser-test-')); + createdRoots.push(root); + return root; +} + +afterEach(() => { + for (const root of createdRoots.splice(0)) { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +describe('discoverBrowserExecutable', () => { + it('finds an executable in the real account home when HOME is isolated', () => { + const accountHome = createRoot(); + const chrome = path.join( + accountHome, + '.agent-browser', + 'browsers', + 'chrome-151.0.0', + 'chrome', + ); + fs.mkdirSync(path.dirname(chrome), { recursive: true }); + fs.writeFileSync(chrome, '#!/bin/sh\nexit 0\n'); + fs.chmodSync(chrome, 0o700); + + expect( + discoverBrowserExecutable({ + env: { HOME: path.join(accountHome, 'isolated-home') }, + accountHome, + platform: 'linux', + findExecutable: () => null, + }), + ).toBe(chrome); + }); + + it('returns one exact retry flag when an explicit browser path is invalid', () => { + const missing = path.join(createRoot(), 'missing-chrome'); + + expect(() => + discoverBrowserExecutable({ + configuredPath: missing, + findExecutable: () => null, + }), + ).toThrow(`proofshot start --browser-executable ${JSON.stringify(missing)}`); + }); +}); diff --git a/src/browser/discovery.ts b/src/browser/discovery.ts new file mode 100644 index 0000000..1722df2 --- /dev/null +++ b/src/browser/discovery.ts @@ -0,0 +1,142 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { findExecutablePath } from '../utils/process.js'; + +export interface BrowserDiscoveryOptions { + configuredPath?: string; + env?: NodeJS.ProcessEnv; + accountHome?: string; + platform?: NodeJS.Platform; + findExecutable?: typeof findExecutablePath; +} + +function isExecutable(filePath: string): boolean { + try { + const stat = fs.statSync(filePath); + if (!stat.isFile()) return false; + fs.accessSync(filePath, fs.constants.R_OK | fs.constants.X_OK); + return true; + } catch { + return false; + } +} + +function sortedDirectories(root: string): string[] { + try { + return fs + .readdirSync(root, { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name) + .sort((a, b) => b.localeCompare(a, undefined, { numeric: true })); + } catch { + return []; + } +} + +function cachedBrowserCandidates(home: string): string[] { + const candidates: string[] = []; + const agentBrowserRoot = path.join(home, '.agent-browser', 'browsers'); + for (const directory of sortedDirectories(agentBrowserRoot)) { + candidates.push( + path.join(agentBrowserRoot, directory, 'chrome'), + path.join(agentBrowserRoot, directory, 'chrome-linux64', 'chrome'), + path.join(agentBrowserRoot, directory, 'chrome-linux', 'chrome'), + ); + } + + const playwrightRoot = path.join(home, '.cache', 'ms-playwright'); + for (const directory of sortedDirectories(playwrightRoot)) { + if (!directory.startsWith('chromium')) continue; + candidates.push( + path.join(playwrightRoot, directory, 'chrome-linux64', 'chrome'), + path.join(playwrightRoot, directory, 'chrome-linux', 'chrome'), + path.join(playwrightRoot, directory, 'chrome-headless-shell-linux64', 'chrome-headless-shell'), + ); + } + + const puppeteerRoot = path.join(home, '.cache', 'puppeteer', 'chrome'); + for (const directory of sortedDirectories(puppeteerRoot)) { + candidates.push( + path.join(puppeteerRoot, directory, 'chrome-linux64', 'chrome'), + path.join(puppeteerRoot, directory, 'chrome-linux', 'chrome'), + ); + } + return candidates; +} + +function accountHomeDirectory(): string | undefined { + try { + return os.userInfo().homedir; + } catch { + return undefined; + } +} + +/** + * Find a Chrome/Chromium executable without assuming that `$HOME` is the + * account's real home directory. No profile, cookies, or storage are reused. + */ +export function discoverBrowserExecutable( + options: BrowserDiscoveryOptions = {}, +): string | null { + const env = options.env ?? process.env; + const platform = options.platform ?? process.platform; + const executableLookup = options.findExecutable ?? findExecutablePath; + const explicit = options.configuredPath || env.AGENT_BROWSER_EXECUTABLE_PATH; + + if (explicit) { + const resolved = path.resolve(explicit); + if (!isExecutable(resolved)) { + throw new Error( + `Browser executable is not runnable: ${resolved}\n` + + `Retry with: proofshot start --browser-executable ${JSON.stringify(resolved)}`, + ); + } + return resolved; + } + + const commandNames = + platform === 'darwin' + ? ['google-chrome', 'chromium'] + : platform === 'win32' + ? ['chrome', 'msedge'] + : ['google-chrome-stable', 'google-chrome', 'chromium', 'chromium-browser']; + for (const command of commandNames) { + const executable = executableLookup(command, platform); + if (executable && isExecutable(executable)) return executable; + } + + const homes = new Set(); + if (env.HOME) homes.add(path.resolve(env.HOME)); + const accountHome = options.accountHome ?? accountHomeDirectory(); + if (accountHome) homes.add(path.resolve(accountHome)); + + const candidates: string[] = []; + if (platform === 'darwin') { + candidates.push( + '/Applications/Google Chrome.app/Contents/MacOS/Google Chrome', + '/Applications/Chromium.app/Contents/MacOS/Chromium', + ); + } else if (platform === 'win32') { + for (const root of [env.PROGRAMFILES, env['PROGRAMFILES(X86)'], env.LOCALAPPDATA]) { + if (!root) continue; + candidates.push( + path.join(root, 'Google', 'Chrome', 'Application', 'chrome.exe'), + path.join(root, 'Microsoft', 'Edge', 'Application', 'msedge.exe'), + ); + } + } else { + candidates.push('/usr/bin/google-chrome', '/usr/bin/chromium', '/usr/bin/chromium-browser'); + for (const home of homes) candidates.push(...cachedBrowserCandidates(home)); + } + + return candidates.find(isExecutable) ?? null; +} + +export function browserSetupError(): Error { + return new Error( + 'No runnable Chrome/Chromium executable was found for this environment.\n' + + 'Run `agent-browser install` in this environment, then retry `proofshot start`.', + ); +} diff --git a/src/browser/runtime.test.ts b/src/browser/runtime.test.ts new file mode 100644 index 0000000..663bf72 --- /dev/null +++ b/src/browser/runtime.test.ts @@ -0,0 +1,57 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { + prepareAgentBrowserSocketDir, + UNIX_SOCKET_PATH_MAX_BYTES, +} from './runtime.js'; + +const createdRoots: string[] = []; + +function createAccountHome(): string { + const cache = path.join(os.userInfo().homedir, '.cache'); + fs.mkdirSync(cache, { recursive: true }); + const root = fs.mkdtempSync(path.join(cache, 'proofshot-runtime-test-')); + createdRoots.push(root); + return root; +} + +afterEach(() => { + for (const root of createdRoots.splice(0)) { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +describe('prepareAgentBrowserSocketDir', () => { + it('stays short and independent of a long isolated HOME', () => { + const accountHome = createAccountHome(); + const isolatedHome = path.join(accountHome, 'isolated', 'x'.repeat(180)); + const sessionName = 'ps-audit-123456789abc'; + + const socketDir = prepareAgentBrowserSocketDir( + sessionName, + { HOME: isolatedHome }, + accountHome, + ); + + expect(socketDir).not.toContain(isolatedHome); + expect(Buffer.byteLength(path.join(socketDir, `${sessionName}.sock`))).toBeLessThanOrEqual( + UNIX_SOCKET_PATH_MAX_BYTES, + ); + expect(fs.statSync(socketDir).mode & 0o777).toBe(0o700); + }); + + it('rejects an explicitly configured socket path before agent-browser starts', () => { + const accountHome = createAccountHome(); + const longSocketDir = path.join(accountHome, 'x'.repeat(90)); + + expect(() => + prepareAgentBrowserSocketDir( + 'ps-audit-123456789abc', + { AGENT_BROWSER_SOCKET_DIR: longSocketDir }, + accountHome, + ), + ).toThrow(/max 103/); + }); +}); diff --git a/src/browser/runtime.ts b/src/browser/runtime.ts new file mode 100644 index 0000000..b106d3f --- /dev/null +++ b/src/browser/runtime.ts @@ -0,0 +1,89 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { + captureProcessIdentity, + type ProcessIdentity, +} from '../utils/process.js'; + +export const UNIX_SOCKET_PATH_MAX_BYTES = 103; + +function assertOwnedDirectory(directory: string): void { + const stat = fs.lstatSync(directory); + if (!stat.isDirectory() || stat.isSymbolicLink()) { + throw new Error(`Agent-browser socket path is not a real directory: ${directory}`); + } + + const uid = process.getuid?.(); + if (uid !== undefined && stat.uid !== uid) { + throw new Error( + `Agent-browser socket directory is owned by uid ${stat.uid}, expected ${uid}: ${directory}`, + ); + } + + fs.accessSync(directory, fs.constants.R_OK | fs.constants.W_OK | fs.constants.X_OK); + if (uid !== undefined) fs.chmodSync(directory, 0o700); +} + +/** + * Prepare a short, user-owned socket directory that is stable across the + * separate `start`, `exec`, and `stop` CLI processes in one environment. + */ +export function prepareAgentBrowserSocketDir( + sessionName: string, + env: NodeJS.ProcessEnv = process.env, + accountHome = os.userInfo().homedir, +): string { + const uid = process.getuid?.() ?? process.pid; + const explicit = env.AGENT_BROWSER_SOCKET_DIR; + const systemRuntime = `/run/user/${uid}`; + let runtimeRoot = accountHome; + if (!explicit && env.XDG_RUNTIME_DIR && path.isAbsolute(env.XDG_RUNTIME_DIR)) { + runtimeRoot = env.XDG_RUNTIME_DIR; + } else if (!explicit && fs.existsSync(systemRuntime)) { + try { + assertOwnedDirectory(systemRuntime); + runtimeRoot = systemRuntime; + } catch { + // Fall back to the real account home, independently of isolated $HOME. + } + } + const directory = explicit + ? path.resolve(explicit) + : runtimeRoot === systemRuntime || runtimeRoot === env.XDG_RUNTIME_DIR + ? path.join(runtimeRoot, 'proofshot', 'agent-browser') + : path.join(runtimeRoot, '.cache', 'proofshot', 'run', 'agent-browser'); + + fs.mkdirSync(directory, { recursive: true, mode: 0o700 }); + assertOwnedDirectory(directory); + + const socketPath = path.join(directory, `${sessionName}.sock`); + const byteLength = Buffer.byteLength(socketPath); + if (byteLength > UNIX_SOCKET_PATH_MAX_BYTES) { + throw new Error( + `Agent-browser socket path is ${byteLength} bytes (max ${UNIX_SOCKET_PATH_MAX_BYTES}): ${socketPath}\n` + + 'Set AGENT_BROWSER_SOCKET_DIR to a shorter user-owned directory and retry.', + ); + } + + return directory; +} + +/** Read the exact daemon PID written for this isolated agent-browser session. */ +export function captureAgentBrowserProcessIdentity( + socketDir: string, + sessionName: string, +): ProcessIdentity | null { + if (!/^[a-zA-Z0-9_-]+$/.test(sessionName)) return null; + + try { + assertOwnedDirectory(socketDir); + const pidPath = path.join(socketDir, `${sessionName}.pid`); + const pid = Number(fs.readFileSync(pidPath, 'utf-8').trim()); + const identity = captureProcessIdentity(pid); + if (!identity || identity.sessionId !== identity.pid) return null; + return identity; + } catch { + return null; + } +} diff --git a/src/cli.ts b/src/cli.ts index 5be1c3f..e3bccea 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -36,6 +36,7 @@ export function createCLI(): Command { .option('--headed', 'Show browser window for debugging') .option('--output ', 'Custom output directory') .option('--url ', 'Open this URL instead of the root') + .option('--browser-executable ', 'Use this Chrome/Chromium executable') .option('--force', 'Override a stale session without running stop first') .action(async (options) => { await startCommand(options); @@ -46,7 +47,7 @@ export function createCLI(): Command { .description('Stop session: stop recording, collect errors, bundle proof artifacts') .option('--no-close', 'Don\'t close the browser (keep it open for further use)') .action(async (options) => { - await stopCommand(options); + await stopCommand({ noClose: options.close === false }); }); program diff --git a/src/commands/clean.test.ts b/src/commands/clean.test.ts new file mode 100644 index 0000000..9355030 --- /dev/null +++ b/src/commands/clean.test.ts @@ -0,0 +1,45 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const mocks = vi.hoisted(() => ({ loadConfig: vi.fn() })); +vi.mock('../utils/config.js', () => ({ loadConfig: mocks.loadConfig })); + +import { cleanCommand } from './clean.js'; + +let root: string; + +beforeEach(() => { + const cache = path.join(os.userInfo().homedir, '.cache'); + fs.mkdirSync(cache, { recursive: true }); + root = fs.mkdtempSync(path.join(cache, 'proofshot-clean-test-')); + mocks.loadConfig.mockReturnValue({ output: root }); + vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.spyOn(process, 'exit').mockImplementation(((code?: number) => { + throw new Error(`process.exit:${code ?? 0}`); + }) as never); +}); + +afterEach(() => { + vi.restoreAllMocks(); + mocks.loadConfig.mockReset(); + fs.rmSync(root, { recursive: true, force: true }); +}); + +describe('cleanCommand', () => { + it('refuses to discard active exact-process ownership metadata', async () => { + const controlPath = path.join(root, '.session.json'); + const evidencePath = path.join(root, 'evidence.txt'); + fs.writeFileSync(controlPath, JSON.stringify({ browserRetained: true })); + fs.writeFileSync(evidencePath, 'keep'); + + await expect(cleanCommand()).rejects.toThrow('process.exit:1'); + + expect(fs.readFileSync(controlPath, 'utf-8')).toContain('browserRetained'); + expect(fs.readFileSync(evidencePath, 'utf-8')).toBe('keep'); + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining('Run "proofshot stop" first'), + ); + }); +}); diff --git a/src/commands/clean.ts b/src/commands/clean.ts index 8eeb088..72f1676 100644 --- a/src/commands/clean.ts +++ b/src/commands/clean.ts @@ -2,11 +2,23 @@ import * as fs from 'fs'; import * as path from 'path'; import chalk from 'chalk'; import { loadConfig } from '../utils/config.js'; +import { hasActiveSession, resolveSessionControlDir } from '../session/state.js'; export async function cleanCommand(): Promise { const config = loadConfig(); + const controlDir = resolveSessionControlDir(config.output); const outputDir = path.resolve(config.output); + if (hasActiveSession(controlDir)) { + console.error( + chalk.red('✗') + + ' Cannot clean while a ProofShot session owns browser or server processes.\n' + + chalk.dim('Run "proofshot stop" first so exact cleanup metadata is preserved.'), + ); + process.exit(1); + return; + } + if (!fs.existsSync(outputDir)) { console.log(chalk.dim('Nothing to clean — no artifacts directory found.')); return; diff --git a/src/commands/doctor.test.ts b/src/commands/doctor.test.ts index e7d2d15..76943f1 100644 --- a/src/commands/doctor.test.ts +++ b/src/commands/doctor.test.ts @@ -1,10 +1,11 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -const { findConfigPathMock, loadConfigMock, loadSessionMock, findExecutablePathMock, readCommandVersionMock } = +const { findConfigPathMock, loadConfigMock, loadSessionMock, resolveSessionControlDirMock, findExecutablePathMock, readCommandVersionMock } = vi.hoisted(() => ({ findConfigPathMock: vi.fn(), loadConfigMock: vi.fn(), loadSessionMock: vi.fn(), + resolveSessionControlDirMock: vi.fn(), findExecutablePathMock: vi.fn(), readCommandVersionMock: vi.fn(), })); @@ -16,6 +17,7 @@ vi.mock('../utils/config.js', () => ({ vi.mock('../session/state.js', () => ({ loadSession: loadSessionMock, + resolveSessionControlDir: resolveSessionControlDirMock, })); vi.mock('../utils/process.js', () => ({ @@ -37,6 +39,7 @@ describe('doctorCommand', () => { defaultPages: ['/'], }); loadSessionMock.mockReturnValue(null); + resolveSessionControlDirMock.mockReturnValue('/workspace/proofshot-artifacts'); findExecutablePathMock.mockImplementation((name: string) => name === 'agent-browser' ? '/usr/local/bin/agent-browser' : '/opt/homebrew/bin/ffmpeg', ); diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 6904317..6340dc2 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -2,7 +2,7 @@ import chalk from 'chalk'; import { PROOFSHOT_VERSION } from '../version.js'; import { findConfigPath, loadConfig } from '../utils/config.js'; import { findExecutablePath, readCommandVersion } from '../utils/process.js'; -import { loadSession } from '../session/state.js'; +import { loadSession, resolveSessionControlDir } from '../session/state.js'; function statusLabel(ok: boolean, text: string): string { return ok ? `${chalk.green('✓')} ${text}` : `${chalk.yellow('⚠')} ${text}`; @@ -15,8 +15,8 @@ function printLine(label: string, value: string): void { export async function doctorCommand(): Promise { const configPath = findConfigPath(); const config = loadConfig(); - const outputDir = config.output; - const session = loadSession(outputDir); + const controlDir = resolveSessionControlDir(config.output); + const session = loadSession(controlDir); const agentBrowserPath = findExecutablePath('agent-browser'); const ffmpegPath = findExecutablePath('ffmpeg'); @@ -28,7 +28,8 @@ export async function doctorCommand(): Promise { printLine('ProofShot', PROOFSHOT_VERSION); printLine('Config', configPath || chalk.dim('not found')); - printLine('Output', outputDir); + printLine('Output', config.output); + printLine('Control state', controlDir); printLine('Browser mode', config.headless ? 'headless' : 'headed'); printLine('Viewport', `${config.viewport.width}x${config.viewport.height}`); console.log(''); @@ -48,6 +49,7 @@ export async function doctorCommand(): Promise { printLine('Session dir', session.sessionDir); printLine('Recording', session.recordingActive ? 'active' : 'stopped'); printLine('Port', String(session.port)); + if (session.targetUrl) printLine('Target', session.targetUrl); } else { printLine('Session dir', chalk.dim('none')); } diff --git a/src/commands/exec.ts b/src/commands/exec.ts index 921d8e9..6933f97 100644 --- a/src/commands/exec.ts +++ b/src/commands/exec.ts @@ -2,8 +2,19 @@ import * as fs from 'fs'; import * as path from 'path'; import { execSync } from 'child_process'; import { loadConfig } from '../utils/config.js'; -import { ab, buildAgentBrowserCommand, setAgentBrowserDefaults } from '../utils/exec.js'; -import { loadSession, saveSession, type SessionState } from '../session/state.js'; +import { + ab, + buildAgentBrowserCommand, + getAgentBrowserEnvironment, + setAgentBrowserDefaults, +} from '../utils/exec.js'; +import { + loadSession, + resolveSessionControlDir, + saveSession, + type SessionState, +} from '../session/state.js'; +import { canAddressOwnedBrowserSession } from '../session/lifecycle.js'; const SESSION_LOG_FILENAME = 'session-log.json'; @@ -180,9 +191,12 @@ export async function execCommand(args: string[]): Promise { // Load session state const config = loadConfig(); - setAgentBrowserDefaults({ configPath: config.browser.configPath }); - const outputDir = path.resolve(config.output); - const session = loadSession(outputDir); + const controlDir = resolveSessionControlDir(config.output); + const session = loadSession(controlDir); + setAgentBrowserDefaults({ + configPath: session?.agentBrowserConfigPath || config.browser.configPath, + socketDir: session?.agentBrowserSocketDir, + }); if (session && !session.recordingActive) { console.error( @@ -192,6 +206,15 @@ export async function execCommand(args: string[]): Promise { process.exit(1); } + if (session && !canAddressOwnedBrowserSession(session)) { + console.error( + 'Error: Browser ownership no longer matches this ProofShot session.\n' + + 'Refusing to address a possibly reused agent-browser session name.', + ); + process.exit(1); + return; + } + // Resolve args (screenshot path rewriting) let resolvedArgs = args; if (session) { @@ -237,6 +260,7 @@ export async function execCommand(args: string[]): Promise { encoding: 'utf-8', timeout: 60000, stdio: ['pipe', 'pipe', 'pipe'], + env: getAgentBrowserEnvironment(), }); if (result.trim()) { process.stdout.write(result); @@ -262,7 +286,7 @@ export async function execCommand(args: string[]): Promise { }); const vp = JSON.parse(vpJson); session.viewport = { width: vp.width, height: vp.height }; - saveSession(session); + saveSession(session, controlDir); } catch { // Non-critical — viewport cache stays stale } diff --git a/src/commands/lifecycle.integration.test.ts b/src/commands/lifecycle.integration.test.ts new file mode 100644 index 0000000..2ee54f8 --- /dev/null +++ b/src/commands/lifecycle.integration.test.ts @@ -0,0 +1,493 @@ +import * as fs from 'fs'; +import * as net from 'net'; +import * as os from 'os'; +import * as path from 'path'; +import { execFileSync, spawn, spawnSync } from 'child_process'; +import { fileURLToPath } from 'url'; +import { afterEach, beforeAll, describe, expect, it } from 'vitest'; +import { isPortOpen } from '../utils/port.js'; +import { + captureProcessIdentity, + terminateOwnedProcessTree, + type ProcessIdentity, +} from '../utils/process.js'; + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..'); +const cliPath = path.join(repoRoot, 'dist', 'bin', 'proofshot.js'); +const createdRoots: string[] = []; +const cleanupProcesses: ProcessIdentity[] = []; + +function cacheRoot(): string { + const cache = path.join(os.userInfo().homedir, '.cache'); + fs.mkdirSync(cache, { recursive: true }); + return cache; +} + +function createAuditRoot(): { base: string; audit: string } { + const base = fs.mkdtempSync(path.join(cacheRoot(), 'proofshot-lifecycle-test-')); + const audit = path.join( + base, + `generated-audit-${'x'.repeat(64)}`, + `consumer-evidence-${'y'.repeat(48)}`, + ); + fs.mkdirSync(audit, { recursive: true }); + createdRoots.push(base); + return { base, audit }; +} + +function shellQuote(value: string): string { + return `'${value.replace(/'/g, `'\\''`)}'`; +} + +async function freePort(): Promise { + const server = net.createServer(); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const address = server.address(); + if (!address || typeof address === 'string') throw new Error('missing free port'); + await new Promise((resolve) => server.close(() => resolve())); + return address.port; +} + +function processIsAlive(pid: number): boolean { + return captureProcessIdentity(pid) !== null; +} + +async function waitForProcessExit(pid: number, timeoutMs = 5000): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (!processIsAlive(pid)) return; + await new Promise((resolve) => setTimeout(resolve, 25)); + } + throw new Error(`process ${pid} did not exit`); +} + +function writeFixtureTools(base: string): { + binDir: string; + browserPath: string; + browserLog: string; + serverScript: string; +} { + const binDir = path.join(base, 'bin'); + fs.mkdirSync(binDir, { recursive: true }); + const browserLog = path.join(base, 'agent-browser.jsonl'); + const fakeAgentBrowser = path.join(binDir, 'agent-browser'); + fs.writeFileSync( + fakeAgentBrowser, + `#!/usr/bin/env node +const fs = require('fs'); +const path = require('path'); +const { spawn } = require('child_process'); +let args = process.argv.slice(2); +let session = 'default'; +const sessionIndex = args.indexOf('--session'); +if (sessionIndex >= 0) { + session = args[sessionIndex + 1]; + args.splice(sessionIndex, 2); +} +const configIndex = args.indexOf('--config'); +if (configIndex >= 0) args.splice(configIndex, 2); +const socketDir = process.env.AGENT_BROWSER_SOCKET_DIR; +if (!socketDir) { + process.stderr.write('missing AGENT_BROWSER_SOCKET_DIR\\n'); + process.exit(2); +} +fs.mkdirSync(socketDir, { recursive: true }); +const pidPath = path.join(socketDir, session + '.pid'); +const statePath = path.join(socketDir, session + '.fake.json'); +const command = args[0] || ''; +const detail = args.slice(1); +fs.appendFileSync(process.env.FAKE_AGENT_BROWSER_LOG, JSON.stringify({ + pid: process.pid, + session, + socketDir, + home: process.env.HOME, + command, + detail, +}) + '\\n'); +if (Buffer.byteLength(path.join(socketDir, session + '.sock')) > 103) { + process.stderr.write('socket path too long\\n'); + process.exit(3); +} +if (command === 'open') { + const daemon = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { + detached: true, + stdio: 'ignore', + }); + daemon.unref(); + fs.writeFileSync(pidPath, String(daemon.pid)); + fs.writeFileSync(statePath, JSON.stringify({ url: detail[0] })); + fs.appendFileSync(process.env.FAKE_AGENT_BROWSER_LOG, JSON.stringify({ + session, + socketDir, + command: 'daemon', + daemonPid: daemon.pid, + }) + '\\n'); + if (process.env.FAKE_AGENT_BROWSER_FAIL_OPEN === '1') { + process.stderr.write('simulated browser open failure\\n'); + process.exit(9); + } + process.exit(0); +} +if (command === 'get' && detail[0] === 'url') { + const state = JSON.parse(fs.readFileSync(statePath, 'utf8')); + process.stdout.write(state.url + '\\n'); + process.exit(0); +} +if (command === 'console' && detail.includes('--json')) { + process.stdout.write(JSON.stringify({ success: true, data: { messages: [] } }) + '\\n'); + process.exit(0); +} +if (command === 'console') { + process.stdout.write('No console output\\n'); + process.exit(0); +} +if (command === 'errors') { + process.stdout.write('No errors\\n'); + process.exit(0); +} +if (command === 'close') { + try { + const pid = Number(fs.readFileSync(pidPath, 'utf8')); + process.kill(-pid, 'SIGTERM'); + } catch {} + try { fs.unlinkSync(pidPath); } catch {} + try { fs.unlinkSync(statePath); } catch {} + process.exit(0); +} +process.exit(0); +`, + ); + fs.chmodSync(fakeAgentBrowser, 0o700); + + const browserPath = path.join(binDir, 'fake-chrome'); + fs.writeFileSync(browserPath, '#!/bin/sh\nexit 0\n'); + fs.chmodSync(browserPath, 0o700); + + const serverScript = path.join(base, 'server.mjs'); + fs.writeFileSync( + serverScript, + [ + "import fs from 'node:fs';", + "import http from 'node:http';", + 'const port = Number(process.argv[2]);', + 'const pidFile = process.argv[3];', + "fs.writeFileSync(pidFile, String(process.pid));", + "const server = http.createServer((request, response) => response.end(request.url || '/'));", + "server.listen(port, '127.0.0.1', () => console.log('server-ready'));", + ].join('\n'), + ); + return { binDir, browserPath, browserLog, serverScript }; +} + +function isolatedEnvironment( + audit: string, + tools: ReturnType, + overrides: NodeJS.ProcessEnv = {}, +): NodeJS.ProcessEnv { + const env: NodeJS.ProcessEnv = { + ...process.env, + HOME: path.join(audit, 'isolated-home'), + XDG_CACHE_HOME: path.join(audit, 'isolated-cache'), + PATH: `${tools.binDir}${path.delimiter}${process.env.PATH || ''}`, + FAKE_AGENT_BROWSER_LOG: tools.browserLog, + ...overrides, + }; + delete env.AGENT_BROWSER_SOCKET_DIR; + delete env.XDG_RUNTIME_DIR; + return env; +} + +function runCli( + cwd: string, + env: NodeJS.ProcessEnv, + args: string[], +): ReturnType { + return spawnSync(process.execPath, [cliPath, ...args], { + cwd, + env, + encoding: 'utf-8', + timeout: 15000, + }); +} + +beforeAll(() => { + execFileSync('npm', ['run', 'build'], { cwd: repoRoot, stdio: 'ignore' }); +}, 30000); + +afterEach(async () => { + for (const identity of cleanupProcesses.splice(0)) { + await terminateOwnedProcessTree(identity, { graceMs: 300 }); + } + for (const root of createdRoots.splice(0)) { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +describe('isolated CLI lifecycle', () => { + it('shares custom-output control across processes and stops idempotently', async () => { + const { base, audit } = createAuditRoot(); + const tools = writeFixtureTools(base); + const env = isolatedEnvironment(audit, tools); + fs.mkdirSync(env.HOME!, { recursive: true }); + fs.writeFileSync( + path.join(audit, 'proofshot.config.json'), + JSON.stringify({ output: './proofshot-artifacts' }), + ); + + const unrelated = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { + detached: true, + stdio: 'ignore', + }); + unrelated.unref(); + const unrelatedIdentity = captureProcessIdentity(unrelated.pid!); + if (!unrelatedIdentity) throw new Error('failed to capture unrelated process'); + cleanupProcesses.push(unrelatedIdentity); + + const port = await freePort(); + const serverPidFile = path.join(base, 'owned-server.pid'); + const customOutput = path.join(audit, 'custom-evidence'); + const intendedUrl = `http://127.0.0.1:${port}/intended-target`; + const serverCommand = [ + shellQuote(process.execPath), + shellQuote(tools.serverScript), + String(port), + shellQuote(serverPidFile), + ].join(' '); + + const start = runCli(audit, env, [ + 'start', + '--run', + serverCommand, + '--port', + String(port), + '--output', + customOutput, + '--url', + intendedUrl, + '--browser-executable', + tools.browserPath, + '--description', + 'isolated lifecycle integration', + ]); + expect(start.status, `${start.stdout}\n${start.stderr}`).toBe(0); + expect(start.stdout).toContain(`Target: ${intendedUrl}`); + + const controlPath = path.join(audit, 'proofshot-artifacts', '.session.json'); + expect(fs.existsSync(controlPath)).toBe(true); + expect(fs.existsSync(path.join(customOutput, '.session.json'))).toBe(false); + const state = JSON.parse(fs.readFileSync(controlPath, 'utf-8')); + expect(state).toMatchObject({ + outputDir: customOutput, + targetUrl: intendedUrl, + recordingActive: true, + }); + expect(Buffer.byteLength(path.join(state.agentBrowserSocketDir, `${state.sessionName}.sock`))).toBeLessThanOrEqual(103); + expect(state.agentBrowserSocketDir).not.toContain(env.HOME); + expect(state.serverProcess).toMatchObject({ pid: expect.any(Number), startTime: expect.any(String) }); + expect(state.browserProcess).toMatchObject({ pid: expect.any(Number), startTime: expect.any(String) }); + cleanupProcesses.push(state.serverProcess, state.browserProcess); + + const ownedServerPid = Number(fs.readFileSync(serverPidFile, 'utf-8')); + expect(processIsAlive(ownedServerPid)).toBe(true); + expect(processIsAlive(unrelated.pid!)).toBe(true); + + const execResult = runCli(audit, env, ['exec', 'get', 'url']); + expect(execResult.status, `${execResult.stdout}\n${execResult.stderr}`).toBe(0); + expect(execResult.stdout.trim()).toBe(intendedUrl); + expect(execResult.stdout).not.toContain('about:blank'); + const browserCalls = fs + .readFileSync(tools.browserLog, 'utf-8') + .trim() + .split('\n') + .map((line) => JSON.parse(line)); + expect(browserCalls.at(-1)).toMatchObject({ + session: state.sessionName, + socketDir: state.agentBrowserSocketDir, + command: 'get', + detail: ['url'], + }); + + const browserLogBeforeMismatchedExec = fs.readFileSync(tools.browserLog, 'utf-8'); + const mismatchedState = { + ...state, + browserProcess: { + ...state.browserProcess, + startTime: `${state.browserProcess.startTime}-recycled`, + }, + }; + fs.writeFileSync(controlPath, JSON.stringify(mismatchedState, null, 2) + '\n'); + const mismatchedExec = runCli(audit, env, ['exec', 'get', 'url']); + expect(mismatchedExec.status).toBe(1); + expect(mismatchedExec.stderr).toContain( + 'Browser ownership no longer matches this ProofShot session', + ); + expect(fs.readFileSync(tools.browserLog, 'utf-8')).toBe( + browserLogBeforeMismatchedExec, + ); + fs.writeFileSync(controlPath, JSON.stringify(state, null, 2) + '\n'); + + const stop = runCli(audit, env, ['stop']); + expect(stop.status, `${stop.stdout}\n${stop.stderr}`).toBe(0); + expect(fs.existsSync(controlPath)).toBe(false); + await waitForProcessExit(ownedServerPid); + await waitForProcessExit(state.serverProcess.pid); + await waitForProcessExit(state.browserProcess.pid); + expect(processIsAlive(unrelated.pid!)).toBe(true); + cleanupProcesses.splice(cleanupProcesses.indexOf(state.serverProcess), 1); + cleanupProcesses.splice(cleanupProcesses.indexOf(state.browserProcess), 1); + + const summaryPath = path.join(state.sessionDir, 'SUMMARY.md'); + const summaryBefore = fs.readFileSync(summaryPath, 'utf-8'); + const summaryMtimeBefore = fs.statSync(summaryPath).mtimeMs; + const browserLogBefore = fs.readFileSync(tools.browserLog, 'utf-8'); + + const secondStop = runCli(audit, env, ['stop']); + expect(secondStop.status, `${secondStop.stdout}\n${secondStop.stderr}`).toBe(0); + expect(secondStop.stdout).toContain('already stopped'); + expect(fs.readFileSync(summaryPath, 'utf-8')).toBe(summaryBefore); + expect(fs.statSync(summaryPath).mtimeMs).toBe(summaryMtimeBefore); + expect(fs.readFileSync(tools.browserLog, 'utf-8')).toBe(browserLogBefore); + }, 30000); + + it('preserves unrelated listeners and cleans partial browser/server starts', async () => { + const { base, audit } = createAuditRoot(); + const tools = writeFixtureTools(base); + const env = isolatedEnvironment(audit, tools); + fs.mkdirSync(env.HOME!, { recursive: true }); + fs.writeFileSync( + path.join(audit, 'proofshot.config.json'), + JSON.stringify({ output: './proofshot-artifacts' }), + ); + + const occupiedPort = await freePort(); + const unrelatedPidFile = path.join(base, 'unrelated-listener.pid'); + const unrelated = spawn( + process.execPath, + [tools.serverScript, String(occupiedPort), unrelatedPidFile], + { detached: true, stdio: 'ignore' }, + ); + unrelated.unref(); + const unrelatedIdentity = captureProcessIdentity(unrelated.pid!); + if (!unrelatedIdentity) throw new Error('failed to capture unrelated listener'); + cleanupProcesses.push(unrelatedIdentity); + for (let attempt = 0; attempt < 80 && !fs.existsSync(unrelatedPidFile); attempt++) { + await new Promise((resolve) => setTimeout(resolve, 25)); + } + for (let attempt = 0; attempt < 80 && !(await isPortOpen(occupiedPort)); attempt++) { + await new Promise((resolve) => setTimeout(resolve, 25)); + } + expect(await isPortOpen(occupiedPort)).toBe(true); + + const occupiedStart = runCli(audit, env, [ + 'start', + '--run', + `${shellQuote(process.execPath)} -e ${shellQuote('setInterval(() => {}, 1000)')}`, + '--port', + String(occupiedPort), + '--browser-executable', + tools.browserPath, + ]); + expect(occupiedStart.status).toBe(1); + expect(occupiedStart.stderr).toContain('already in use by a process ProofShot did not start'); + expect(processIsAlive(unrelated.pid!)).toBe(true); + expect(fs.existsSync(path.join(audit, 'proofshot-artifacts', '.session.json'))).toBe(false); + const stopAfterOccupiedFailure = runCli(audit, env, ['stop']); + expect(stopAfterOccupiedFailure.status).toBe(0); + expect(stopAfterOccupiedFailure.stdout).toContain('already stopped'); + expect(processIsAlive(unrelated.pid!)).toBe(true); + + const failedPort = await freePort(); + const failedServerPidFile = path.join(base, 'failed-server.pid'); + const failedServerCommand = [ + shellQuote(process.execPath), + shellQuote(tools.serverScript), + String(failedPort), + shellQuote(failedServerPidFile), + ].join(' '); + const failedStart = runCli( + audit, + isolatedEnvironment(audit, tools, { FAKE_AGENT_BROWSER_FAIL_OPEN: '1' }), + [ + 'start', + '--run', + failedServerCommand, + '--port', + String(failedPort), + '--browser-executable', + tools.browserPath, + ], + ); + expect(failedStart.status).toBe(1); + expect(failedStart.stderr).toContain('simulated browser open failure'); + const failedServerPid = Number(fs.readFileSync(failedServerPidFile, 'utf-8')); + await waitForProcessExit(failedServerPid); + expect(processIsAlive(unrelated.pid!)).toBe(true); + expect(fs.existsSync(path.join(audit, 'proofshot-artifacts', '.session.json'))).toBe(false); + + const calls = fs + .readFileSync(tools.browserLog, 'utf-8') + .trim() + .split('\n') + .map((line) => JSON.parse(line)); + const failedOpen = [...calls].reverse().find((call) => call.command === 'open'); + const failedDaemon = [...calls] + .reverse() + .find((call) => call.command === 'daemon' && call.session === failedOpen.session); + const failedSessionCalls = calls.filter((call) => call.session === failedOpen.session); + expect(failedSessionCalls.map((call) => call.command)).toEqual( + expect.arrayContaining(['open', 'record', 'close']), + ); + await waitForProcessExit(failedDaemon.daemonPid); + const failedBrowserPidPath = path.join( + failedOpen.socketDir, + `${failedOpen.session}.pid`, + ); + expect(fs.existsSync(failedBrowserPidPath)).toBe(false); + }, 30000); + + it('retains exact browser ownership across stop --no-close', async () => { + const { base, audit } = createAuditRoot(); + const tools = writeFixtureTools(base); + const env = isolatedEnvironment(audit, tools); + fs.mkdirSync(env.HOME!, { recursive: true }); + fs.writeFileSync( + path.join(audit, 'proofshot.config.json'), + JSON.stringify({ output: './proofshot-artifacts' }), + ); + + const start = runCli(audit, env, [ + 'start', + '--url', + 'https://example.invalid/retained-browser', + '--browser-executable', + tools.browserPath, + ]); + expect(start.status, `${start.stdout}\n${start.stderr}`).toBe(0); + const controlPath = path.join(audit, 'proofshot-artifacts', '.session.json'); + const initialState = JSON.parse(fs.readFileSync(controlPath, 'utf-8')); + cleanupProcesses.push(initialState.browserProcess); + + const retainedStop = runCli(audit, env, ['stop', '--no-close']); + expect(retainedStop.status, `${retainedStop.stdout}\n${retainedStop.stderr}`).toBe(0); + const retainedState = JSON.parse(fs.readFileSync(controlPath, 'utf-8')); + expect(retainedState).toMatchObject({ + recordingActive: false, + bundleComplete: true, + browserRetained: true, + browserProcess: initialState.browserProcess, + }); + expect(processIsAlive(initialState.browserProcess.pid)).toBe(true); + const summaryPath = path.join(initialState.sessionDir, 'SUMMARY.md'); + const summaryBefore = fs.readFileSync(summaryPath, 'utf-8'); + const summaryMtimeBefore = fs.statSync(summaryPath).mtimeMs; + + const finalStop = runCli(audit, env, ['stop']); + expect(finalStop.status, `${finalStop.stdout}\n${finalStop.stderr}`).toBe(0); + expect(finalStop.stdout).toContain('Retained browser closed'); + await waitForProcessExit(initialState.browserProcess.pid); + expect(fs.existsSync(controlPath)).toBe(false); + expect(fs.readFileSync(summaryPath, 'utf-8')).toBe(summaryBefore); + expect(fs.statSync(summaryPath).mtimeMs).toBe(summaryMtimeBefore); + cleanupProcesses.splice(cleanupProcesses.indexOf(initialState.browserProcess), 1); + }, 30000); +}); diff --git a/src/commands/start.test.ts b/src/commands/start.test.ts index 96632ae..2fd8a1b 100644 --- a/src/commands/start.test.ts +++ b/src/commands/start.test.ts @@ -11,10 +11,17 @@ const mocks = vi.hoisted(() => ({ generateTimestamp: vi.fn(), generateSessionDirName: vi.fn(), saveSession: vi.fn(), + loadSession: vi.fn(), hasActiveSession: vi.fn(), clearSession: vi.fn(), generateAgentBrowserSessionName: vi.fn(), + resolveSessionControlDir: vi.fn(), writeMetadata: vi.fn(), + discoverBrowserExecutable: vi.fn(), + browserSetupError: vi.fn(), + prepareAgentBrowserSocketDir: vi.fn(), + captureAgentBrowserProcessIdentity: vi.fn(), + cleanupFailedStart: vi.fn(), execSync: vi.fn(), })); @@ -35,6 +42,16 @@ vi.mock('../browser/capture.js', () => ({ startRecording: mocks.startRecording, })); +vi.mock('../browser/discovery.js', () => ({ + discoverBrowserExecutable: mocks.discoverBrowserExecutable, + browserSetupError: mocks.browserSetupError, +})); + +vi.mock('../browser/runtime.js', () => ({ + prepareAgentBrowserSocketDir: mocks.prepareAgentBrowserSocketDir, + captureAgentBrowserProcessIdentity: mocks.captureAgentBrowserProcessIdentity, +})); + vi.mock('../artifacts/bundle.js', () => ({ ensureOutputDir: mocks.ensureOutputDir, generateTimestamp: mocks.generateTimestamp, @@ -43,9 +60,15 @@ vi.mock('../artifacts/bundle.js', () => ({ vi.mock('../session/state.js', () => ({ saveSession: mocks.saveSession, + loadSession: mocks.loadSession, hasActiveSession: mocks.hasActiveSession, clearSession: mocks.clearSession, generateAgentBrowserSessionName: mocks.generateAgentBrowserSessionName, + resolveSessionControlDir: mocks.resolveSessionControlDir, +})); + +vi.mock('../session/lifecycle.js', () => ({ + cleanupFailedStart: mocks.cleanupFailedStart, })); vi.mock('../session/metadata.js', () => ({ @@ -76,9 +99,20 @@ describe('startCommand', () => { }, }); mocks.hasActiveSession.mockReturnValue(false); + mocks.loadSession.mockReturnValue(null); + mocks.resolveSessionControlDir.mockReturnValue('/project/proofshot-artifacts'); mocks.generateTimestamp.mockReturnValue('2026-04-08_07-28-00'); mocks.generateSessionDirName.mockReturnValue('2026-04-08_07-28-00_test'); - mocks.generateAgentBrowserSessionName.mockReturnValue('proofshot-2026-04-08_07-28-00'); + mocks.generateAgentBrowserSessionName.mockReturnValue('ps-2026-04-deadbeef1234'); + mocks.prepareAgentBrowserSocketDir.mockReturnValue('/run/user/1000/proofshot'); + mocks.discoverBrowserExecutable.mockReturnValue('/usr/bin/chromium'); + mocks.captureAgentBrowserProcessIdentity.mockReturnValue({ + pid: 4001, + processGroupId: 4001, + sessionId: 4001, + startTime: '12345', + }); + mocks.cleanupFailedStart.mockResolvedValue(undefined); mocks.execSync.mockImplementation((command: string) => { if (command === 'git branch --show-current') return 'main'; if (command === 'git rev-parse HEAD') return 'deadbeef'; @@ -92,7 +126,7 @@ describe('startCommand', () => { Object.values(mocks).forEach((mock) => mock.mockReset()); }); - it('closes the browser when recording never starts after all retries', async () => { + it('cleans the owned session when recording never starts after all retries', async () => { mocks.startRecording.mockImplementation(() => { throw new Error('Recording session could not be initialized'); }); @@ -102,11 +136,12 @@ describe('startCommand', () => { await expect(commandPromise).resolves.toMatchObject({ message: 'process.exit:1' }); expect(mocks.startRecording).toHaveBeenCalledTimes(3); - expect(mocks.closeBrowser).toHaveBeenCalledTimes(1); - expect(mocks.saveSession).not.toHaveBeenCalled(); + expect(mocks.cleanupFailedStart).toHaveBeenCalledTimes(1); + expect(mocks.saveSession).toHaveBeenCalled(); + expect(mocks.clearSession).toHaveBeenCalledWith('/project/proofshot-artifacts'); }); - it('does not try to stop recording when recording never started', async () => { + it('clears discoverable control state when recording never starts', async () => { mocks.startRecording.mockImplementation(() => { throw new Error('Recording already active'); }); @@ -116,7 +151,8 @@ describe('startCommand', () => { await expect(commandPromise).resolves.toMatchObject({ message: 'process.exit:1' }); expect(mocks.startRecording).toHaveBeenCalledTimes(3); - expect(mocks.closeBrowser).toHaveBeenCalledTimes(1); + expect(mocks.cleanupFailedStart).toHaveBeenCalledTimes(1); + expect(mocks.clearSession).toHaveBeenCalledWith('/project/proofshot-artifacts'); }); it('closes the session-scoped browser when browser open fails', async () => { @@ -127,8 +163,31 @@ describe('startCommand', () => { const commandPromise = startCommand({}).catch((error) => error); await expect(commandPromise).resolves.toMatchObject({ message: 'process.exit:1' }); - expect(mocks.closeBrowser).toHaveBeenCalledTimes(1); + expect(mocks.cleanupFailedStart).toHaveBeenCalledTimes(1); expect(mocks.startRecording).not.toHaveBeenCalled(); - expect(mocks.saveSession).not.toHaveBeenCalled(); + expect(mocks.clearSession).toHaveBeenCalledWith('/project/proofshot-artifacts'); + }); + + it('persists the intended target and stable control path with custom evidence output', async () => { + await startCommand({ + output: '/audit/custom-evidence', + url: 'http://127.0.0.1:43171/getting-started', + }); + + expect(mocks.openBrowser).toHaveBeenCalledWith( + 'http://127.0.0.1:43171/getting-started', + { width: 1280, height: 720 }, + true, + 'ps-2026-04-deadbeef1234', + expect.objectContaining({ executablePath: '/usr/bin/chromium' }), + ); + const finalState = mocks.saveSession.mock.calls.at(-1)?.[0]; + expect(finalState).toMatchObject({ + outputDir: '/audit/custom-evidence', + targetUrl: 'http://127.0.0.1:43171/getting-started', + recordingActive: true, + agentBrowserSocketDir: '/run/user/1000/proofshot', + }); + expect(mocks.saveSession.mock.calls.every((call) => call[1] === '/project/proofshot-artifacts')).toBe(true); }); }); diff --git a/src/commands/start.ts b/src/commands/start.ts index f9632ac..5cb1dba 100644 --- a/src/commands/start.ts +++ b/src/commands/start.ts @@ -4,15 +4,24 @@ import { execSync } from 'child_process'; import { loadConfig } from '../utils/config.js'; import { setAgentBrowserDefaults } from '../utils/exec.js'; import { ensureDevServer } from '../server/start.js'; -import { closeBrowser, openBrowser } from '../browser/session.js'; +import { openBrowser } from '../browser/session.js'; import { startRecording } from '../browser/capture.js'; +import { discoverBrowserExecutable, browserSetupError } from '../browser/discovery.js'; +import { + captureAgentBrowserProcessIdentity, + prepareAgentBrowserSocketDir, +} from '../browser/runtime.js'; import { ensureOutputDir, generateTimestamp, generateSessionDirName } from '../artifacts/bundle.js'; import { saveSession, + loadSession, hasActiveSession, clearSession, generateAgentBrowserSessionName, + resolveSessionControlDir, + type SessionState, } from '../session/state.js'; +import { cleanupFailedStart } from '../session/lifecycle.js'; import { writeMetadata } from '../session/metadata.js'; interface StartOptions { @@ -22,23 +31,26 @@ interface StartOptions { headed?: boolean; output?: string; url?: string; + browserExecutable?: string; force?: boolean; } export async function startCommand(options: StartOptions): Promise { const config = loadConfig(); - setAgentBrowserDefaults({ configPath: config.browser.configPath }); - if (options.port) config.devServer.port = options.port; - if (options.output) config.output = options.output; - if (options.headed !== undefined) config.headless = !options.headed; + const controlDir = resolveSessionControlDir(config.output); - const outputDir = path.resolve(config.output); - const timestamp = generateTimestamp(); - - if (hasActiveSession(outputDir)) { + if (hasActiveSession(controlDir)) { if (options.force) { - clearSession(outputDir); - console.log(chalk.yellow('⚠') + chalk.dim(' Cleared stale session')); + const existingSession = loadSession(controlDir); + if (existingSession) { + setAgentBrowserDefaults({ + configPath: existingSession.agentBrowserConfigPath || config.browser.configPath, + socketDir: existingSession.agentBrowserSocketDir, + }); + await cleanupFailedStart(existingSession); + } + clearSession(controlDir); + console.log(chalk.yellow('⚠') + chalk.dim(' Cleaned up the previous session')); } else { console.log( chalk.yellow('⚠ A session is already active.') + @@ -48,11 +60,40 @@ export async function startCommand(options: StartOptions): Promise { } } - ensureOutputDir(outputDir); + if (options.port) config.devServer.port = options.port; + if (options.output) config.output = options.output; + if (options.headed !== undefined) config.headless = !options.headed; + const outputDir = path.resolve(config.output); + const timestamp = generateTimestamp(); const sessionDirName = generateSessionDirName(timestamp, options.description || null); const sessionDir = path.join(outputDir, sessionDirName); const sessionName = generateAgentBrowserSessionName(timestamp); + let socketDir: string; + let browserExecutable: string | null; + + try { + socketDir = prepareAgentBrowserSocketDir(sessionName); + browserExecutable = discoverBrowserExecutable({ + configuredPath: options.browserExecutable || config.browser.executablePath, + }); + if ( + !browserExecutable && + !process.env.AGENT_BROWSER_PROVIDER && + !process.env.AGENT_BROWSER_CDP + ) { + throw browserSetupError(); + } + } catch (error: any) { + console.error(chalk.red('✗') + ` Browser preflight failed: ${error.message}`); + process.exit(1); + return; + } + + if (browserExecutable) config.browser.executablePath = browserExecutable; + setAgentBrowserDefaults({ configPath: config.browser.configPath, socketDir }); + + ensureOutputDir(outputDir); ensureOutputDir(sessionDir); const videoPath = path.join(sessionDir, 'session.webm'); @@ -84,96 +125,111 @@ export async function startCommand(options: StartOptions): Promise { description: options.description || null, }); - let serverAlreadyRunning = true; + const baseUrl = `http://localhost:${config.devServer.port}`; + const openUrl = options.url || baseUrl; + const session: SessionState = { + startedAt: new Date().toISOString(), + description: options.description || null, + outputDir, + sessionDir, + sessionName, + videoPath, + serverErrorLog, + port: config.devServer.port, + serverCommand: options.run || null, + serverAlreadyRunning: !options.run, + recordingActive: false, + bundleComplete: false, + browserRetained: false, + videoTrimComplete: false, + trimOffsetSec: 0, + sessionLogAdjusted: false, + consoleEvidenceAvailable: false, + consoleErrorCount: 0, + targetUrl: openUrl, + agentBrowserSocketDir: socketDir, + agentBrowserConfigPath: config.browser.configPath, + serverProcess: null, + browserProcess: null, + viewport: { width: config.viewport.width, height: config.viewport.height }, + }; + saveSession(session, controlDir); - if (options.run) { - console.log(chalk.dim(`Starting: ${options.run}`)); - try { - await ensureDevServer( + let failureContext = 'start the session'; + try { + if (options.run) { + failureContext = 'start dev server'; + console.log(chalk.dim(`Starting: ${options.run}`)); + const server = await ensureDevServer( options.run, config.devServer.port, config.devServer.startupTimeout, serverErrorLog, ); - serverAlreadyRunning = false; + session.serverAlreadyRunning = false; + session.serverProcess = server.process; + saveSession(session, controlDir); console.log(chalk.green('✓') + ` Dev server started on :${config.devServer.port}`); console.log(chalk.dim(` Server logs → ${serverErrorLog}`)); - } catch (error: any) { - console.error(chalk.red('✗') + ` Failed to start dev server: ${error.message}`); - process.exit(1); + } else { + console.log(chalk.dim('No --run provided, assuming server is already running')); } - } else { - console.log(chalk.dim('No --run provided, assuming server is already running')); - } - const baseUrl = `http://localhost:${config.devServer.port}`; - const openUrl = options.url || baseUrl; - - console.log(chalk.dim('Opening browser...')); - try { + failureContext = 'open browser'; + console.log(chalk.dim('Opening browser...')); openBrowser(openUrl, config.viewport, config.headless, sessionName, config.browser); + session.browserProcess = captureAgentBrowserProcessIdentity(socketDir, sessionName); + if (!session.browserProcess) { + throw new Error( + `Could not record the exact agent-browser daemon identity for session ${sessionName}.`, + ); + } + saveSession(session, controlDir); console.log(chalk.green('✓') + ' Browser ready'); - } catch (error: any) { - closeBrowser(); - console.error( - chalk.red('✗') + - ` Failed to open browser: ${error.message}\n` + - chalk.dim('Make sure agent-browser is installed: npm install -g agent-browser'), - ); - process.exit(1); - } - const RECORDING_RETRIES = 3; - const RETRY_DELAY_MS = 2000; - let recordingStarted = false; - let lastError: any; - - for (let attempt = 1; attempt <= RECORDING_RETRIES; attempt++) { - try { - startRecording(videoPath, sessionName); - recordingStarted = true; - console.log(chalk.green('✓') + ' Recording started'); - break; - } catch (error: any) { - lastError = error; - if (attempt < RECORDING_RETRIES) { - console.log( - chalk.yellow('⚠') + - ` Recording failed (attempt ${attempt}/${RECORDING_RETRIES}), retrying in ${RETRY_DELAY_MS / 1000}s...`, - ); - await new Promise((resolve) => setTimeout(resolve, RETRY_DELAY_MS)); + failureContext = 'initialize recording'; + const RECORDING_RETRIES = 3; + const RETRY_DELAY_MS = 2000; + let recordingStarted = false; + let lastError: any; + + for (let attempt = 1; attempt <= RECORDING_RETRIES; attempt++) { + try { + startRecording(videoPath, sessionName); + recordingStarted = true; + console.log(chalk.green('✓') + ' Recording started'); + break; + } catch (error: any) { + lastError = error; + if (attempt < RECORDING_RETRIES) { + console.log( + chalk.yellow('⚠') + + ` Recording failed (attempt ${attempt}/${RECORDING_RETRIES}), retrying in ${RETRY_DELAY_MS / 1000}s...`, + ); + await new Promise((resolve) => setTimeout(resolve, RETRY_DELAY_MS)); + } } } - } - if (!recordingStarted) { - closeBrowser(); + if (!recordingStarted) { + throw new Error( + `Recording did not start after ${RECORDING_RETRIES} attempts: ${lastError?.message}`, + ); + } + } catch (error: any) { + await cleanupFailedStart(session); + clearSession(controlDir); console.error( chalk.red('✗') + - ` Failed to initialize recording after ${RECORDING_RETRIES} attempts: ${lastError?.message}\n` + - chalk.dim('Recording is required — ProofShot cannot proceed without video capture.\n') + - chalk.dim('Troubleshooting:\n') + - chalk.dim(' 1. Make sure agent-browser is installed and running\n') + - chalk.dim(' 2. Try "proofshot clean" then re-run "proofshot start"\n') + - chalk.dim(' 3. If the port was already in use, stop the old server first'), + ` Failed to ${failureContext}: ${error.message}\n` + + chalk.dim('All processes started by this ProofShot attempt were cleaned up.'), ); process.exit(1); + return; } - saveSession({ - startedAt: new Date().toISOString(), - description: options.description || null, - outputDir, - sessionDir, - sessionName, - videoPath, - serverErrorLog, - port: config.devServer.port, - serverCommand: options.run || null, - serverAlreadyRunning, - recordingActive: true, - viewport: { width: config.viewport.width, height: config.viewport.height }, - }); + session.recordingActive = true; + saveSession(session, controlDir); console.log(''); console.log(chalk.green.bold('✅ ProofShot session started')); @@ -181,6 +237,7 @@ export async function startCommand(options: StartOptions): Promise { console.log(`Server: ${options.run ? chalk.cyan(options.run) : chalk.dim('external')} on :${config.devServer.port}`); console.log(`Browser: Chromium (${config.headless ? 'headless' : 'headed'})`); console.log(`Session: ${chalk.dim(sessionName)}`); + console.log(`Target: ${chalk.dim(openUrl)}`); console.log(`Recording: ${chalk.dim(videoPath)}`); console.log(`Errors log: ${chalk.dim(serverErrorLog)}`); diff --git a/src/commands/stop.test.ts b/src/commands/stop.test.ts new file mode 100644 index 0000000..b0e68d7 --- /dev/null +++ b/src/commands/stop.test.ts @@ -0,0 +1,221 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const mocks = vi.hoisted(() => ({ + loadConfig: vi.fn(), + loadSession: vi.fn(), + clearSession: vi.fn(), + resolveSessionControlDir: vi.fn(), + saveSession: vi.fn(), + stopRecording: vi.fn(), + getConsoleErrors: vi.fn(), + getConsoleOutput: vi.fn(), + getConsoleOutputJson: vi.fn(), + stopOwnedBrowser: vi.fn(), + stopOwnedServer: vi.fn(), + canAddressOwnedBrowserSession: vi.fn(), + writeViewer: vi.fn(), + extractServerErrors: vi.fn(), + loadSessionLog: vi.fn(), + estimateTokenUsage: vi.fn(), + execSync: vi.fn(), +})); + +vi.mock('../utils/config.js', () => ({ loadConfig: mocks.loadConfig })); +vi.mock('../session/state.js', () => ({ + loadSession: mocks.loadSession, + clearSession: mocks.clearSession, + resolveSessionControlDir: mocks.resolveSessionControlDir, + saveSession: mocks.saveSession, +})); +vi.mock('../browser/capture.js', () => ({ stopRecording: mocks.stopRecording })); +vi.mock('../browser/session.js', () => ({ + getConsoleErrors: mocks.getConsoleErrors, + getConsoleOutput: mocks.getConsoleOutput, + getConsoleOutputJson: mocks.getConsoleOutputJson, +})); +vi.mock('../session/lifecycle.js', () => ({ + canAddressOwnedBrowserSession: mocks.canAddressOwnedBrowserSession, + stopOwnedBrowser: mocks.stopOwnedBrowser, + stopOwnedServer: mocks.stopOwnedServer, +})); +vi.mock('../artifacts/viewer.js', () => ({ writeViewer: mocks.writeViewer })); +vi.mock('../utils/error-patterns.js', () => ({ extractServerErrors: mocks.extractServerErrors })); +vi.mock('./exec.js', () => ({ loadSessionLog: mocks.loadSessionLog })); +vi.mock('../utils/token-usage.js', () => ({ estimateTokenUsage: mocks.estimateTokenUsage })); +vi.mock('child_process', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, execSync: mocks.execSync }; +}); + +import { stopCommand } from './stop.js'; + +let root: string; +let session: any; + +beforeEach(() => { + const cache = path.join(os.userInfo().homedir, '.cache'); + fs.mkdirSync(cache, { recursive: true }); + root = fs.mkdtempSync(path.join(cache, 'proofshot-stop-test-')); + const sessionDir = path.join(root, 'custom-evidence', 'session'); + fs.mkdirSync(sessionDir, { recursive: true }); + session = { + startedAt: new Date(Date.now() - 1000).toISOString(), + description: 'retry bundle', + outputDir: path.join(root, 'custom-evidence'), + sessionDir, + sessionName: 'ps-retry-deadbeef1234', + videoPath: path.join(sessionDir, 'session.webm'), + serverErrorLog: path.join(sessionDir, 'server.log'), + port: 3000, + serverCommand: 'npm run dev', + serverAlreadyRunning: false, + recordingActive: true, + bundleComplete: false, + browserRetained: false, + videoTrimComplete: false, + trimOffsetSec: 0, + sessionLogAdjusted: false, + consoleEvidenceAvailable: false, + consoleErrorCount: 0, + serverProcess: { pid: 1001, processGroupId: 1001, sessionId: 1001, startTime: '1' }, + browserProcess: { pid: 1002, processGroupId: 1002, sessionId: 1002, startTime: '2' }, + }; + fs.writeFileSync(session.serverErrorLog, `${Date.now()}\tserver ready\n`); + + mocks.loadConfig.mockReturnValue({ output: './proofshot-artifacts', browser: {} }); + mocks.resolveSessionControlDir.mockReturnValue(path.join(root, 'proofshot-artifacts')); + mocks.loadSession.mockImplementation(() => session); + mocks.getConsoleErrors.mockReturnValue('No errors'); + mocks.getConsoleOutput.mockReturnValue('console evidence'); + mocks.getConsoleOutputJson.mockReturnValue([]); + mocks.extractServerErrors.mockReturnValue([]); + mocks.loadSessionLog.mockReturnValue([]); + mocks.estimateTokenUsage.mockReturnValue(null); + mocks.execSync.mockReturnValue(''); + mocks.stopOwnedBrowser.mockResolvedValue(undefined); + mocks.stopOwnedServer.mockResolvedValue(undefined); + mocks.canAddressOwnedBrowserSession.mockReturnValue(true); + vi.spyOn(console, 'log').mockImplementation(() => {}); +}); + +afterEach(() => { + vi.restoreAllMocks(); + Object.values(mocks).forEach((mock) => mock.mockReset()); + fs.rmSync(root, { recursive: true, force: true }); +}); + +describe('stopCommand retryability', () => { + it('keeps state after a bundle failure and retries without replacing a valid summary', async () => { + fs.writeFileSync(session.videoPath, 'nonempty-original-video'); + const sessionLogPath = path.join(session.sessionDir, 'session-log.json'); + fs.writeFileSync( + sessionLogPath, + JSON.stringify([ + { action: 'open target', relativeTimeSec: 10, timestamp: session.startedAt }, + { action: 'screenshot proof.png', relativeTimeSec: 20, timestamp: session.startedAt }, + ]), + ); + mocks.loadSessionLog.mockImplementation(() => + JSON.parse(fs.readFileSync(sessionLogPath, 'utf-8')), + ); + let trimCalls = 0; + mocks.execSync.mockImplementation((command: string) => { + if (command === 'ffmpeg -version') return ''; + if (command.startsWith('ffmpeg -i ')) { + trimCalls += 1; + fs.writeFileSync(session.videoPath, `trimmed-video-${trimCalls}`); + return ''; + } + throw new Error(`unexpected command: ${command}`); + }); + mocks.writeViewer.mockImplementationOnce(() => { + throw new Error('simulated viewer write failure'); + }); + + await expect(stopCommand({})).rejects.toThrow('simulated viewer write failure'); + expect(mocks.stopOwnedBrowser).toHaveBeenCalledWith(session); + expect(mocks.stopOwnedServer).toHaveBeenCalledWith(session); + expect(mocks.clearSession).not.toHaveBeenCalled(); + expect(mocks.saveSession).toHaveBeenCalledWith( + expect.objectContaining({ + recordingActive: false, + bundleComplete: false, + videoTrimComplete: true, + trimOffsetSec: 5, + sessionLogAdjusted: true, + }), + path.join(root, 'proofshot-artifacts'), + ); + expect(trimCalls).toBe(1); + expect(fs.readFileSync(session.videoPath, 'utf-8')).toBe('trimmed-video-1'); + expect(JSON.parse(fs.readFileSync(sessionLogPath, 'utf-8')).map((entry: any) => entry.relativeTimeSec)).toEqual([5, 15]); + + const summaryPath = path.join(session.sessionDir, 'SUMMARY.md'); + const summaryBefore = fs.readFileSync(summaryPath, 'utf-8'); + const summaryMtimeBefore = fs.statSync(summaryPath).mtimeMs; + mocks.writeViewer.mockReturnValue(path.join(session.sessionDir, 'viewer.html')); + mocks.canAddressOwnedBrowserSession.mockReturnValue(false); + + await stopCommand({}); + + expect(mocks.writeViewer).toHaveBeenCalledTimes(2); + expect(trimCalls).toBe(1); + expect(fs.readFileSync(session.videoPath, 'utf-8')).toBe('trimmed-video-1'); + expect(JSON.parse(fs.readFileSync(sessionLogPath, 'utf-8')).map((entry: any) => entry.relativeTimeSec)).toEqual([5, 15]); + expect(mocks.writeViewer.mock.calls.at(-1)?.[1].entries.map((entry: any) => entry.relativeTimeSec)).toEqual([5, 15]); + expect(mocks.writeViewer.mock.calls.at(-1)?.[1]).toMatchObject({ + consoleEvidenceAvailable: true, + consoleErrorCount: 0, + consoleOutput: 'console evidence', + }); + expect(mocks.clearSession).toHaveBeenCalledWith(path.join(root, 'proofshot-artifacts')); + expect(fs.readFileSync(summaryPath, 'utf-8')).toBe(summaryBefore); + expect(fs.statSync(summaryPath).mtimeMs).toBe(summaryMtimeBefore); + }); + + it('skips every session-addressed browser command when identity is mismatched', async () => { + mocks.canAddressOwnedBrowserSession.mockReturnValue(false); + mocks.writeViewer.mockReturnValue(path.join(session.sessionDir, 'viewer.html')); + + await stopCommand({}); + + expect(mocks.getConsoleErrors).not.toHaveBeenCalled(); + expect(mocks.getConsoleOutput).not.toHaveBeenCalled(); + expect(mocks.getConsoleOutputJson).not.toHaveBeenCalled(); + expect(mocks.stopRecording).not.toHaveBeenCalled(); + expect(mocks.stopOwnedBrowser).toHaveBeenCalledWith(session); + expect(mocks.stopOwnedServer).toHaveBeenCalledWith(session); + expect(mocks.clearSession).toHaveBeenCalled(); + expect(mocks.writeViewer).toHaveBeenCalledWith( + session.sessionDir, + expect.objectContaining({ consoleEvidenceAvailable: false }), + ); + const summary = fs.readFileSync(path.join(session.sessionDir, 'SUMMARY.md'), 'utf-8'); + expect(summary).toContain('console evidence was unavailable'); + expect(summary).not.toContain('No console errors detected'); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('Browser ownership could not be verified'), + ); + }); + + it('does not claim a retained browser was closed when its identity mismatches', async () => { + session.bundleComplete = true; + session.browserRetained = true; + session.recordingActive = false; + mocks.canAddressOwnedBrowserSession.mockReturnValue(false); + + await stopCommand({}); + + expect(mocks.stopOwnedBrowser).toHaveBeenCalledWith(session); + expect(mocks.clearSession).toHaveBeenCalled(); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('skipped session-name close'), + ); + expect(console.log).not.toHaveBeenCalledWith( + expect.stringContaining('Retained browser closed'), + ); + }); +}); diff --git a/src/commands/stop.ts b/src/commands/stop.ts index 2600ed3..a32261e 100644 --- a/src/commands/stop.ts +++ b/src/commands/stop.ts @@ -1,12 +1,23 @@ import * as fs from 'fs'; import * as path from 'path'; +import { randomUUID } from 'crypto'; import { execSync } from 'child_process'; import chalk from 'chalk'; import { loadConfig } from '../utils/config.js'; import { setAgentBrowserDefaults } from '../utils/exec.js'; -import { closeBrowser, getConsoleErrors, getConsoleOutput, getConsoleOutputJson } from '../browser/session.js'; +import { getConsoleErrors, getConsoleOutput, getConsoleOutputJson } from '../browser/session.js'; import { stopRecording } from '../browser/capture.js'; -import { loadSession, clearSession } from '../session/state.js'; +import { + loadSession, + clearSession, + resolveSessionControlDir, + saveSession, +} from '../session/state.js'; +import { + canAddressOwnedBrowserSession, + stopOwnedBrowser, + stopOwnedServer, +} from '../session/lifecycle.js'; import { writeViewer, type TimestampedLogEntry } from '../artifacts/viewer.js'; import { extractServerErrors } from '../utils/error-patterns.js'; import { loadSessionLog } from './exec.js'; @@ -55,57 +66,127 @@ interface StopOptions { export async function stopCommand(options: StopOptions): Promise { const config = loadConfig(); - setAgentBrowserDefaults({ configPath: config.browser.configPath }); - const outputDir = path.resolve(config.output); + const controlDir = resolveSessionControlDir(config.output); // Load session state - const session = loadSession(outputDir); + const session = loadSession(controlDir); if (!session) { - console.error( - chalk.red('✗') + - ' No active session found.\n' + - chalk.dim('Run "proofshot start" first.'), + console.log( + chalk.dim('No active session found; all owned processes are already stopped.'), ); - process.exit(1); + return; } + setAgentBrowserDefaults({ + configPath: session.agentBrowserConfigPath || config.browser.configPath, + socketDir: session.agentBrowserSocketDir, + }); + if (session.bundleComplete) { + if (session.browserRetained && !options.noClose) { + console.log(chalk.dim('Closing retained browser...')); + const browserSessionAddressable = canAddressOwnedBrowserSession(session); + await stopOwnedBrowser(session); + session.browserRetained = false; + clearSession(controlDir); + if (browserSessionAddressable) { + console.log(chalk.green('✓') + ' Retained browser closed; proof artifacts were already bundled.'); + } else { + console.log( + chalk.yellow('⚠') + + ' Retained browser ownership was no longer current; skipped session-name close and cleared control state after exact recorded-tree cleanup.', + ); + } + } else if (session.browserRetained) { + console.log( + chalk.dim('Proof artifacts are already bundled; the owned browser remains intentionally open.'), + ); + } else { + clearSession(controlDir); + console.log(chalk.dim('Proof artifacts are already bundled and all owned processes are stopped.')); + } + return; + } + + const retryingStoppedSession = !session.recordingActive; + const recordingWasActive = session.recordingActive; const startTime = new Date(session.startedAt).getTime(); const durationMs = Date.now() - startTime; const durationSec = Math.round(durationMs / 1000); + const browserSessionAvailable = canAddressOwnedBrowserSession(session); + + const priorConsoleEvidenceAvailable = session.consoleEvidenceAvailable === true; + if (!browserSessionAvailable && priorConsoleEvidenceAvailable) { + console.log( + chalk.dim('Browser already stopped; reusing console evidence collected before cleanup.'), + ); + } else if (!browserSessionAvailable) { + console.log( + chalk.yellow('⚠') + + ' Browser ownership could not be verified; skipping console and recording commands.\n' + + chalk.dim(' Browser evidence may be incomplete; exact recorded-process cleanup will still run.'), + ); + } // Step 1: Collect console errors and output console.log(chalk.dim('Collecting errors...')); let consoleErrors = ''; let consoleOutput = ''; let consoleEntries: TimestampedLogEntry[] = []; - try { - consoleErrors = getConsoleErrors(session.sessionName); - consoleOutput = getConsoleOutput(session.sessionName); - // Get timestamped console messages for viewer sync - const consoleMessages = getConsoleOutputJson(session.sessionName); - consoleEntries = consoleMessages.map((msg) => ({ - text: `[${msg.type}] ${msg.text}`, - relativeTimeSec: Math.max(0, parseFloat(((msg.timestamp - startTime) / 1000).toFixed(1))), - })); - } catch { - // Browser may already be closed + if (browserSessionAvailable) { + try { + consoleErrors = getConsoleErrors(session.sessionName); + consoleOutput = getConsoleOutput(session.sessionName); + // Get timestamped console messages for viewer sync + const consoleMessages = getConsoleOutputJson(session.sessionName); + consoleEntries = consoleMessages.map((msg) => ({ + text: `[${msg.type}] ${msg.text}`, + relativeTimeSec: Math.max(0, parseFloat(((msg.timestamp - startTime) / 1000).toFixed(1))), + })); + } catch { + // Browser may already be closed + } } // Write console output to file (before closing browser) if (consoleOutput.trim()) { fs.writeFileSync(path.join(session.sessionDir, 'console-output.log'), consoleOutput); + } else if (priorConsoleEvidenceAvailable) { + const savedConsoleOutput = path.join(session.sessionDir, 'console-output.log'); + if (fs.existsSync(savedConsoleOutput)) { + consoleOutput = fs.readFileSync(savedConsoleOutput, 'utf-8'); + } } // Step 2: Stop recording console.log(chalk.dim('Stopping recording...')); - stopRecording(session.sessionName); + if (browserSessionAvailable) { + stopRecording(session.sessionName); + } + session.recordingActive = false; + saveSession(session, controlDir); // Step 3: Close browser (unless --no-close) + let cleanupError: unknown; if (!options.noClose) { console.log(chalk.dim('Closing browser...')); - closeBrowser(session.sessionName); + try { + await stopOwnedBrowser(session); + } catch (error) { + cleanupError = error; + } } + // Step 3.5: Stop only the detached process session created by this start. + if (session.serverProcess) { + console.log(chalk.dim('Stopping dev server...')); + try { + await stopOwnedServer(session); + } catch (error) { + cleanupError ||= error; + } + } + if (cleanupError) throw cleanupError; + // Step 4: Read server log (with timestamp parsing) let serverLog = ''; let serverEntries: TimestampedLogEntry[] = []; @@ -126,22 +207,40 @@ export async function stopCommand(options: StopOptions): Promise { // Step 5.5: Trim video dead time const sessionLog = loadSessionLog(sessionDir); - let trimOffsetSec = 0; - if (fs.existsSync(session.videoPath)) { - trimOffsetSec = trimVideo(session.videoPath, screenshots, sessionDir, startTime, sessionLog); - } else if (session.recordingActive) { - console.log( - chalk.yellow('⚠') + - ' Recording was active but no video file was produced.\n' + - chalk.dim(' The screencast may have been interrupted. Screenshots and logs are still saved.'), - ); + let trimOffsetSec = session.trimOffsetSec ?? 0; + if (!session.videoTrimComplete) { + if (fs.existsSync(session.videoPath)) { + trimOffsetSec = trimVideo(session.videoPath, screenshots, sessionDir, startTime, sessionLog); + } else if (recordingWasActive) { + console.log( + chalk.yellow('⚠') + + ' Recording was active but no video file was produced.\n' + + chalk.dim(' The screencast may have been interrupted. Screenshots and logs are still saved.'), + ); + } + session.videoTrimComplete = true; + session.trimOffsetSec = trimOffsetSec; + saveSession(session, controlDir); } // Step 6: Count errors const consoleErrorLines = consoleErrors .split('\n') .filter((l) => l.trim() && l.trim() !== 'No errors'); - const consoleErrorCount = consoleErrorLines.length > 0 && consoleErrors.trim() !== '' ? consoleErrorLines.length : 0; + const observedConsoleErrorCount = + consoleErrorLines.length > 0 && consoleErrors.trim() !== '' + ? consoleErrorLines.length + : 0; + const consoleEvidenceAvailable = + browserSessionAvailable || priorConsoleEvidenceAvailable; + const consoleErrorCount = browserSessionAvailable + ? observedConsoleErrorCount + : session.consoleErrorCount ?? 0; + if (browserSessionAvailable) { + session.consoleEvidenceAvailable = true; + session.consoleErrorCount = consoleErrorCount; + saveSession(session, controlDir); + } // Extract errors from server log using multi-language patterns const serverErrorLines = extractServerErrors(serverLog); @@ -160,28 +259,35 @@ export async function stopCommand(options: StopOptions): Promise { screenshots, consoleErrors, consoleErrorCount, + consoleEvidenceAvailable, serverLog, serverErrorCount, tokenUsage, durationSec, outputDir: sessionDir, }); - fs.writeFileSync(summaryPath, summary); + if (!retryingStoppedSession || !fs.existsSync(summaryPath)) { + writeTextFileAtomically(summaryPath, summary); + } // Step 7.5: Generate interactive viewer (if session log exists) // Adjust session log timestamps to match the trimmed video - const viewerEntries = - trimOffsetSec > 0 - ? sessionLog.map((e) => ({ - ...e, - relativeTimeSec: parseFloat((e.relativeTimeSec - trimOffsetSec).toFixed(1)), - })) - : sessionLog; + let viewerEntries = sessionLog; + if (trimOffsetSec > 0 && !session.sessionLogAdjusted) { + viewerEntries = sessionLog.map((e) => ({ + ...e, + relativeTimeSec: parseFloat((e.relativeTimeSec - trimOffsetSec).toFixed(1)), + })); + } // Write adjusted log back to disk so timestamps match the trimmed video - if (trimOffsetSec > 0 && viewerEntries.length > 0) { + if (trimOffsetSec > 0 && !session.sessionLogAdjusted && viewerEntries.length > 0) { const logPath = path.join(sessionDir, 'session-log.json'); - fs.writeFileSync(logPath, JSON.stringify(viewerEntries, null, 2) + '\n'); + writeTextFileAtomically(logPath, JSON.stringify(viewerEntries, null, 2) + '\n'); + } + if (!session.sessionLogAdjusted) { + session.sessionLogAdjusted = true; + saveSession(session, controlDir); } // Apply trimOffsetSec to log entries (same adjustment as session log) @@ -199,6 +305,7 @@ export async function stopCommand(options: StopOptions): Promise { durationSec, videoFilename: fs.existsSync(session.videoPath) ? path.basename(session.videoPath) : null, consoleErrorCount, + consoleEvidenceAvailable, serverErrorCount, consoleOutput, serverLog, @@ -208,8 +315,14 @@ export async function stopCommand(options: StopOptions): Promise { tokenUsage, }); - // Step 8: Clear session state - clearSession(outputDir); + // Step 8: Retain exact browser ownership only when explicitly requested. + session.bundleComplete = true; + session.browserRetained = Boolean(options.noClose); + if (session.browserRetained) { + saveSession(session, controlDir); + } else { + clearSession(controlDir); + } // Step 9: Print results console.log(''); @@ -228,7 +341,13 @@ export async function stopCommand(options: StopOptions): Promise { } console.log(''); console.log( - `Console errors: ${consoleErrorCount === 0 ? chalk.green('0') : chalk.red(String(consoleErrorCount))}`, + `Console errors: ${ + !consoleEvidenceAvailable + ? chalk.yellow('unavailable') + : consoleErrorCount === 0 + ? chalk.green('0') + : chalk.red(String(consoleErrorCount)) + }`, ); console.log( `Server errors: ${serverErrorCount === 0 ? chalk.green('0') : chalk.red(String(serverErrorCount))}`, @@ -236,6 +355,9 @@ export async function stopCommand(options: StopOptions): Promise { console.log(`Duration: ${durationSec} seconds`); console.log(''); console.log(`Proof artifacts saved to ${chalk.dim(sessionDir)}`); + if (session.browserRetained) { + console.log(chalk.dim('Browser retained. Run "proofshot stop" later to close this exact session.')); + } // If errors were found, print them for immediate feedback if (consoleErrorCount > 0) { @@ -261,6 +383,16 @@ export async function stopCommand(options: StopOptions): Promise { } } +function writeTextFileAtomically(filePath: string, contents: string): void { + const temporaryPath = `${filePath}.${process.pid}.${randomUUID()}.tmp`; + try { + fs.writeFileSync(temporaryPath, contents); + fs.renameSync(temporaryPath, filePath); + } finally { + if (fs.existsSync(temporaryPath)) fs.unlinkSync(temporaryPath); + } +} + interface SummaryData { description: string | null; serverCommand: string | null; @@ -269,6 +401,7 @@ interface SummaryData { screenshots: string[]; consoleErrors: string; consoleErrorCount: number; + consoleEvidenceAvailable: boolean; serverLog: string; serverErrorCount: number; tokenUsage?: TokenUsage | null; @@ -318,7 +451,9 @@ Full session recording: [${relativeVideo}](./${relativeVideo}) (${data.durationS md += `## Console Errors `; - if (data.consoleErrorCount === 0) { + if (!data.consoleEvidenceAvailable) { + md += `Browser ownership could not be verified, so console evidence was unavailable.\n\n`; + } else if (data.consoleErrorCount === 0) { md += `No console errors detected.\n\n`; } else { md += `${data.consoleErrorCount} error(s) detected:\n\n\`\`\`\n${data.consoleErrors}\n\`\`\`\n\n`; diff --git a/src/server/start.test.ts b/src/server/start.test.ts new file mode 100644 index 0000000..206567c --- /dev/null +++ b/src/server/start.test.ts @@ -0,0 +1,96 @@ +import * as fs from 'fs'; +import * as http from 'http'; +import * as os from 'os'; +import * as path from 'path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { isPortOpen } from '../utils/port.js'; +import { terminateOwnedProcessTree, type ProcessIdentity } from '../utils/process.js'; +import { ensureDevServer } from './start.js'; + +const roots: string[] = []; +const ownedProcesses: ProcessIdentity[] = []; + +function createRoot(): string { + const cache = path.join(os.userInfo().homedir, '.cache'); + fs.mkdirSync(cache, { recursive: true }); + const root = fs.mkdtempSync(path.join(cache, 'proofshot-server-test-')); + roots.push(root); + return root; +} + +function shellQuote(value: string): string { + return `'${value.replace(/'/g, `'\\''`)}'`; +} + +afterEach(async () => { + for (const identity of ownedProcesses.splice(0)) { + await terminateOwnedProcessTree(identity, { graceMs: 200 }); + } + for (const root of roots.splice(0)) { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +describe('ensureDevServer', () => { + it('fails actionably without killing an unrelated occupied listener', async () => { + const listener = http.createServer((_request, response) => response.end('unrelated')); + await new Promise((resolve) => listener.listen(0, '127.0.0.1', resolve)); + const address = listener.address(); + if (!address || typeof address === 'string') throw new Error('missing listener port'); + + try { + await expect( + ensureDevServer( + `${shellQuote(process.execPath)} -e ${shellQuote('process.exit(99)')}`, + address.port, + 250, + path.join(createRoot(), 'server.log'), + ), + ).rejects.toThrow(/already in use by a process ProofShot did not start/); + expect(listener.listening).toBe(true); + } finally { + await new Promise((resolve) => listener.close(() => resolve())); + } + }); + + it('persists an exact supervisor identity and keeps epoch-tab server logs', async () => { + const root = createRoot(); + const scriptPath = path.join(root, 'server.mjs'); + const logPath = path.join(root, 'server.log'); + fs.writeFileSync( + scriptPath, + [ + "import http from 'node:http';", + 'const port = Number(process.argv[2]);', + "const server = http.createServer((_req, res) => res.end('ok'));", + "server.listen(port, '127.0.0.1', () => console.log('server-ready'));", + ].join('\n'), + ); + + const probe = http.createServer(); + await new Promise((resolve) => probe.listen(0, '127.0.0.1', resolve)); + const address = probe.address(); + if (!address || typeof address === 'string') throw new Error('missing probe port'); + const port = address.port; + await new Promise((resolve) => probe.close(() => resolve())); + + const result = await ensureDevServer( + `${shellQuote(process.execPath)} ${shellQuote(scriptPath)} ${port}`, + port, + 3000, + logPath, + ); + ownedProcesses.push(result.process); + + expect(result.process.processGroupId).toBe(result.process.pid); + expect(result.process.sessionId).toBe(result.process.pid); + expect(fs.readFileSync(logPath, 'utf-8')).toMatch(/^\d{13}\tserver-ready$/m); + + await terminateOwnedProcessTree(result.process, { graceMs: 300 }); + ownedProcesses.pop(); + for (let attempt = 0; attempt < 40 && (await isPortOpen(port)); attempt++) { + await new Promise((resolve) => setTimeout(resolve, 25)); + } + expect(await isPortOpen(port)).toBe(false); + }); +}); diff --git a/src/server/start.ts b/src/server/start.ts index 1ad1f61..499ce7f 100644 --- a/src/server/start.ts +++ b/src/server/start.ts @@ -1,60 +1,59 @@ import * as fs from 'fs'; -import { Transform } from 'stream'; +import { spawn } from 'child_process'; import { isPortOpen, waitForPort } from '../utils/port.js'; import { - findPidsListeningOnPort, - killPids, - spawnShellCommand, + captureProcessIdentity, + getShellExecutable, + terminateOwnedProcessTree, terminateProcessTree, + type ProcessIdentity, } from '../utils/process.js'; export interface ServerStartResult { alreadyRunning: boolean; port: number; + process: ProcessIdentity; } -/** - * Kill whatever process is listening on the given port. - * Retries up to 3 times to ensure the port is actually freed. - * Returns true if something was killed. - */ -async function killPort(port: number): Promise { - let killed = false; - for (let attempt = 0; attempt < 3; attempt++) { - const pids = findPidsListeningOnPort(port); - if (pids.length > 0) { - killed = killPids(pids) || killed; - } - - // Wait for the OS to release the port - await new Promise((r) => setTimeout(r, 1000)); - if (!(await isPortOpen(port))) return killed; - } - return killed; -} - -/** - * Create a Transform stream that prepends an epoch-ms timestamp to each line. - * Format: "1720612345678\toriginal line\n" - */ -function createTimestampTransform(): Transform { +// A detached supervisor keeps timestamping server output after the short-lived +// `proofshot start` process exits. It and the server share one new process +// session, whose immutable identity is persisted for exact later cleanup. +const SERVER_RUNNER_SOURCE = String.raw` +const fs = require('fs'); +const { spawn } = require('child_process'); +const [command, cwd, logPath, shell] = process.argv.slice(1); +const fd = fs.openSync(logPath, 'a'); +let closed = false; +const write = (text) => { + if (!closed) fs.writeSync(fd, Date.now() + '\t' + text + '\n'); +}; +const child = spawn(command, { + cwd, + shell, + stdio: ['ignore', 'pipe', 'pipe'], +}); +const attach = (stream) => { let buffer = ''; - return new Transform({ - transform(chunk, _encoding, callback) { - buffer += chunk.toString(); - const lines = buffer.split('\n'); - buffer = lines.pop()!; - for (const line of lines) { - this.push(`${Date.now()}\t${line}\n`); - } - callback(); - }, - flush(callback) { - if (buffer) this.push(`${Date.now()}\t${buffer}\n`); - callback(); - }, + stream.on('data', (chunk) => { + buffer += chunk.toString(); + const lines = buffer.split('\n'); + buffer = lines.pop(); + for (const line of lines) write(line); }); -} + stream.on('end', () => { + if (buffer) write(buffer); + buffer = ''; + }); +}; +attach(child.stdout); +attach(child.stderr); +child.on('error', (error) => write(error.stack || error.message || String(error))); +child.on('close', (code) => { + closed = true; + fs.closeSync(fd); + process.exit(code == null ? 1 : code); +}); +`; /** * Start a dev server command and wait for it to be ready. @@ -67,45 +66,50 @@ export async function ensureDevServer( startupTimeout: number, logPath: string, ): Promise { - // If port is occupied, kill the existing process — the user explicitly - // asked proofshot to own the server via --run. + // Port ownership is not session ownership. Never kill an unrelated listener. if (await isPortOpen(port)) { - const killed = await killPort(port); - if (killed) { - process.stderr.write(`Port ${port} was in use — killed existing process\n`); - } - // Final check — if still occupied, fail fast with a clear message - if (await isPortOpen(port)) { - throw new Error( - `Port ${port} is still in use after attempting to kill the process.\n` + - `Manually stop whatever is running on port ${port} and retry.`, - ); - } + throw new Error( + `Port ${port} is already in use by a process ProofShot did not start.\n` + + 'Choose another port or stop that process explicitly, then retry.', + ); } - const proc = spawnShellCommand(command, { - cwd: process.cwd(), - stdio: ['ignore', 'pipe', 'pipe'], + // Ensure log creation errors surface before launching the detached runner. + const logFd = fs.openSync(logPath, 'a'); + fs.closeSync(logFd); + const proc = spawn(process.execPath, [ + '-e', + SERVER_RUNNER_SOURCE, + command, + process.cwd(), + logPath, + getShellExecutable(), + ], { + stdio: 'ignore', detached: true, }); - const logStream = fs.createWriteStream(logPath, { flags: 'a' }); - const tsOut = createTimestampTransform(); - const tsErr = createTimestampTransform(); - proc.stdout?.pipe(tsOut).pipe(logStream, { end: false }); - proc.stderr?.pipe(tsErr).pipe(logStream, { end: false }); - proc.unref(); + let processIdentity = proc.pid ? captureProcessIdentity(proc.pid) : null; + for (let attempt = 0; !processIdentity && attempt < 5; attempt++) { + await new Promise((resolve) => setTimeout(resolve, 10)); + processIdentity = proc.pid ? captureProcessIdentity(proc.pid) : null; + } - try { - await waitForPort(port, startupTimeout); - } catch (error) { - // Clean up the spawned process if it failed to start on the expected port + if (!processIdentity || processIdentity.sessionId !== processIdentity.pid) { try { if (proc.pid) terminateProcessTree(proc.pid); } catch { - // Already exited + // The child may already have exited. } + throw new Error('ProofShot could not record an exact identity for the dev server process.'); + } + + try { + await waitForPort(port, startupTimeout); + } catch (error) { + // Clean up the spawned process if it failed to start on the expected port + await terminateOwnedProcessTree(processIdentity); throw new Error( `Failed to start dev server with "${command}" on port ${port}.\n` + `Make sure the command is correct and the port is available.\n` + @@ -116,5 +120,5 @@ export async function ensureDevServer( // Small delay for stability await new Promise((resolve) => setTimeout(resolve, 1000)); - return { alreadyRunning: false, port }; + return { alreadyRunning: false, port, process: processIdentity }; } diff --git a/src/session/lifecycle.test.ts b/src/session/lifecycle.test.ts new file mode 100644 index 0000000..21b65fa --- /dev/null +++ b/src/session/lifecycle.test.ts @@ -0,0 +1,85 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const mocks = vi.hoisted(() => ({ + captureAgentBrowserProcessIdentity: vi.fn(), + closeBrowser: vi.fn(), + stopRecording: vi.fn(), + ownedProcessTreeIsAlive: vi.fn(), + processIdentityMatches: vi.fn(), + terminateOwnedProcessTree: vi.fn(), +})); + +vi.mock('../browser/runtime.js', () => ({ + captureAgentBrowserProcessIdentity: mocks.captureAgentBrowserProcessIdentity, +})); +vi.mock('../browser/session.js', () => ({ closeBrowser: mocks.closeBrowser })); +vi.mock('../browser/capture.js', () => ({ stopRecording: mocks.stopRecording })); +vi.mock('../utils/process.js', () => ({ + ownedProcessTreeIsAlive: mocks.ownedProcessTreeIsAlive, + processIdentityMatches: mocks.processIdentityMatches, + terminateOwnedProcessTree: mocks.terminateOwnedProcessTree, +})); + +import { + canAddressOwnedBrowserSession, + cleanupFailedStart, + stopOwnedBrowser, +} from './lifecycle.js'; + +const persistedIdentity = { + pid: 12001, + processGroupId: 12001, + sessionId: 12001, + startTime: 'original-start', +}; + +function session(browserProcess: typeof persistedIdentity | null = persistedIdentity): any { + return { + sessionName: 'ps-owned-session', + agentBrowserSocketDir: '/run/user/1000/proofshot/agent-browser', + browserProcess, + serverProcess: null, + }; +} + +beforeEach(() => { + vi.clearAllMocks(); + mocks.processIdentityMatches.mockReturnValue(true); + mocks.ownedProcessTreeIsAlive.mockReturnValue(false); + mocks.terminateOwnedProcessTree.mockResolvedValue(true); +}); + +describe('owned browser lifecycle', () => { + it('does not address a recycled session name when the persisted identity mismatches', async () => { + mocks.processIdentityMatches.mockReturnValue(false); + const state = session(); + + expect(canAddressOwnedBrowserSession(state)).toBe(false); + await stopOwnedBrowser(state); + await cleanupFailedStart(state); + + expect(mocks.captureAgentBrowserProcessIdentity).not.toHaveBeenCalled(); + expect(mocks.closeBrowser).not.toHaveBeenCalled(); + expect(mocks.stopRecording).not.toHaveBeenCalled(); + expect(mocks.terminateOwnedProcessTree).toHaveBeenCalledWith(persistedIdentity); + }); + + it('allows a matching persisted identity and legacy state captured from its PID file', async () => { + const legacyIdentity = { + ...persistedIdentity, + pid: 12002, + processGroupId: 12002, + sessionId: 12002, + }; + mocks.captureAgentBrowserProcessIdentity.mockReturnValue(legacyIdentity); + + expect(canAddressOwnedBrowserSession(session())).toBe(true); + await stopOwnedBrowser(session()); + await stopOwnedBrowser(session(null)); + + expect(mocks.closeBrowser).toHaveBeenNthCalledWith(1, 'ps-owned-session'); + expect(mocks.closeBrowser).toHaveBeenNthCalledWith(2, 'ps-owned-session'); + expect(mocks.terminateOwnedProcessTree).toHaveBeenNthCalledWith(1, persistedIdentity); + expect(mocks.terminateOwnedProcessTree).toHaveBeenNthCalledWith(2, legacyIdentity); + }); +}); diff --git a/src/session/lifecycle.ts b/src/session/lifecycle.ts new file mode 100644 index 0000000..ad137b7 --- /dev/null +++ b/src/session/lifecycle.ts @@ -0,0 +1,78 @@ +import { stopRecording } from '../browser/capture.js'; +import { + captureAgentBrowserProcessIdentity, +} from '../browser/runtime.js'; +import { closeBrowser } from '../browser/session.js'; +import { + ownedProcessTreeIsAlive, + processIdentityMatches, + terminateOwnedProcessTree, + type ProcessIdentity, +} from '../utils/process.js'; +import type { SessionState } from './state.js'; + +function resolveOwnedBrowserIdentity(session: SessionState): ProcessIdentity | null { + return ( + session.browserProcess || + (session.agentBrowserSocketDir + ? captureAgentBrowserProcessIdentity( + session.agentBrowserSocketDir, + session.sessionName, + ) + : null) + ); +} + +/** + * Whether it is safe to address this agent-browser session by socket/name. + * Persisted immutable identity always wins: a mismatched PID must never fall + * back to a possibly reused session-name PID file. Legacy state without an + * identity may adopt the exact current identity from that file. + */ +export function canAddressOwnedBrowserSession(session: SessionState): boolean { + const identity = resolveOwnedBrowserIdentity(session); + return Boolean(identity && processIdentityMatches(identity)); +} + +export async function stopOwnedBrowser(session: SessionState): Promise { + const identity = resolveOwnedBrowserIdentity(session); + + // The graceful CLI command is name/socket addressed, so issue it only while + // the persisted immutable identity still matches. Exact tree termination + // below remains safe when the leader has exited or its PID was recycled. + if (identity && processIdentityMatches(identity)) { + closeBrowser(session.sessionName); + } + await terminateOwnedProcessTree(identity); + if (identity && ownedProcessTreeIsAlive(identity)) { + throw new Error(`Owned browser process session ${identity.sessionId} did not stop.`); + } +} + +export async function stopOwnedServer(session: SessionState): Promise { + await terminateOwnedProcessTree(session.serverProcess); + if (session.serverProcess && ownedProcessTreeIsAlive(session.serverProcess)) { + throw new Error(`Owned server process session ${session.serverProcess.sessionId} did not stop.`); + } +} + +export async function cleanupFailedStart(session: SessionState): Promise { + // Recording may have started even when its CLI call returned an error. Both + // operations are session-scoped and best effort. Never address a session + // name unless its daemon still has the identity captured by this start. + if (canAddressOwnedBrowserSession(session)) { + stopRecording(session.sessionName); + } + let cleanupError: unknown; + try { + await stopOwnedBrowser(session); + } catch (error) { + cleanupError = error; + } + try { + await stopOwnedServer(session); + } catch (error) { + cleanupError ||= error; + } + if (cleanupError) throw cleanupError; +} diff --git a/src/session/state.test.ts b/src/session/state.test.ts index 9a3409b..1108f99 100644 --- a/src/session/state.test.ts +++ b/src/session/state.test.ts @@ -2,15 +2,28 @@ import { describe, expect, it } from 'vitest'; import { generateAgentBrowserSessionName } from './state.js'; describe('generateAgentBrowserSessionName', () => { - it('prefixes ProofShot session names consistently', () => { - expect(generateAgentBrowserSessionName('2026-04-07_22-30-00')).toBe( - 'proofshot-2026-04-07_22-30-00', + it('creates a short, deterministic name when a nonce is supplied', () => { + const name = generateAgentBrowserSessionName('2026-04-07_22-30-00', 'test-nonce'); + expect(name).toMatch(/^ps-2026-04-[a-f0-9]{12}$/); + expect(name).toBe( + generateAgentBrowserSessionName('2026-04-07_22-30-00', 'test-nonce'), ); + expect(name.length).toBeLessThanOrEqual(24); }); - it('normalizes unsafe characters', () => { - expect(generateAgentBrowserSessionName("April 7 review / O'Connor")).toBe( - 'proofshot-april-7-review-o-connor', + it('normalizes unsafe characters and keeps concurrent runs collision-safe', () => { + const first = generateAgentBrowserSessionName("April 7 review / O'Connor", 'one'); + const second = generateAgentBrowserSessionName("April 7 review / O'Connor", 'two'); + expect(first).toMatch(/^ps-april-7-[a-f0-9]{12}$/); + expect(second).not.toBe(first); + expect(first).not.toMatch(/[^a-z0-9_-]/); + }); + + it('does not expose a long seed in the socket-facing name', () => { + expect( + generateAgentBrowserSessionName('x'.repeat(500), 'bounded'), + ).toMatch( + /^ps-x{8}-[a-f0-9]{12}$/, ); }); }); diff --git a/src/session/state.ts b/src/session/state.ts index 7cbccf9..be91c22 100644 --- a/src/session/state.ts +++ b/src/session/state.ts @@ -1,5 +1,7 @@ import * as fs from 'fs'; import * as path from 'path'; +import { createHash, randomUUID } from 'crypto'; +import type { ProcessIdentity } from '../utils/process.js'; const SESSION_FILENAME = '.session.json'; @@ -15,23 +17,54 @@ export interface SessionState { serverCommand: string | null; serverAlreadyRunning: boolean; recordingActive: boolean; + bundleComplete?: boolean; + browserRetained?: boolean; + videoTrimComplete?: boolean; + trimOffsetSec?: number; + sessionLogAdjusted?: boolean; + consoleEvidenceAvailable?: boolean; + consoleErrorCount?: number; + targetUrl?: string; + agentBrowserSocketDir?: string; + agentBrowserConfigPath?: string; + serverProcess?: ProcessIdentity | null; + browserProcess?: ProcessIdentity | null; viewport?: { width: number; height: number }; } +/** + * Resolve the stable control directory for a project. + * + * CLI-only `--output` overrides choose where evidence is written, but active + * control state remains in the configured/default output directory so a later + * `proofshot exec` or `proofshot stop` process can always find it. + */ +export function resolveSessionControlDir( + configuredOutput: string, + cwd = process.cwd(), +): string { + return path.resolve(cwd, configuredOutput); +} + /** * Write session state to disk. */ -export function saveSession(state: SessionState): void { - const sessionPath = path.join(state.outputDir, SESSION_FILENAME); - fs.writeFileSync(sessionPath, JSON.stringify(state, null, 2) + '\n'); +export function saveSession(state: SessionState, controlDir = state.outputDir): void { + fs.mkdirSync(controlDir, { recursive: true }); + const sessionPath = path.join(controlDir, SESSION_FILENAME); + const temporaryPath = `${sessionPath}.${process.pid}.${randomUUID()}.tmp`; + fs.writeFileSync(temporaryPath, JSON.stringify(state, null, 2) + '\n', { + mode: 0o600, + }); + fs.renameSync(temporaryPath, sessionPath); } /** * Read session state from disk. * Returns null if no active session. */ -export function loadSession(outputDir: string): SessionState | null { - const sessionPath = path.join(outputDir, SESSION_FILENAME); +export function loadSession(controlDir: string): SessionState | null { + const sessionPath = path.join(controlDir, SESSION_FILENAME); if (!fs.existsSync(sessionPath)) return null; try { return JSON.parse(fs.readFileSync(sessionPath, 'utf-8')); @@ -43,15 +76,15 @@ export function loadSession(outputDir: string): SessionState | null { /** * Check if a session is currently active. */ -export function hasActiveSession(outputDir: string): boolean { - return fs.existsSync(path.join(outputDir, SESSION_FILENAME)); +export function hasActiveSession(controlDir: string): boolean { + return fs.existsSync(path.join(controlDir, SESSION_FILENAME)); } /** * Delete the session state file (called after stop). */ -export function clearSession(outputDir: string): void { - const sessionPath = path.join(outputDir, SESSION_FILENAME); +export function clearSession(controlDir: string): void { + const sessionPath = path.join(controlDir, SESSION_FILENAME); if (fs.existsSync(sessionPath)) { fs.unlinkSync(sessionPath); } @@ -60,12 +93,20 @@ export function clearSession(outputDir: string): void { /** * Generate a deterministic agent-browser session name for a ProofShot run. */ -export function generateAgentBrowserSessionName(seed: string): string { +export function generateAgentBrowserSessionName( + seed: string, + nonce = randomUUID(), +): string { const normalized = seed .toLowerCase() .replace(/[^a-z0-9-_]+/g, '-') .replace(/^-+|-+$/g, '') - .slice(0, 48); + .slice(0, 8) + .replace(/-+$/g, ''); + const digest = createHash('sha256') + .update(`${seed}\0${nonce}`) + .digest('hex') + .slice(0, 12); - return normalized ? `proofshot-${normalized}` : 'proofshot'; + return normalized ? `ps-${normalized}-${digest}` : `ps-${digest}`; } diff --git a/src/utils/exec.ts b/src/utils/exec.ts index a864017..0dfac1b 100644 --- a/src/utils/exec.ts +++ b/src/utils/exec.ts @@ -14,17 +14,27 @@ export class ProofShotError extends Error { export interface AgentBrowserCommandOptions { configPath?: string; session?: string; + socketDir?: string; timeoutMs?: number; } -let defaultAgentBrowserOptions: Pick = {}; +let defaultAgentBrowserOptions: Pick = {}; export function setAgentBrowserDefaults( - options: Pick, + options: Pick, ): void { defaultAgentBrowserOptions = { ...options }; } +export function getAgentBrowserEnvironment( + options: Pick = {}, +): NodeJS.ProcessEnv { + const socketDir = options.socketDir ?? defaultAgentBrowserOptions.socketDir; + return socketDir + ? { ...process.env, AGENT_BROWSER_SOCKET_DIR: socketDir } + : { ...process.env }; +} + function shellQuote(value: string): string { const escaped = value.replace(/'/g, "'\\''"); return `'${escaped}'`; @@ -62,6 +72,7 @@ export function ab( encoding: 'utf-8', timeout: options.timeoutMs ?? 30000, stdio: ['pipe', 'pipe', 'pipe'], + env: getAgentBrowserEnvironment(options), }).trim(); } catch (error: any) { const stderr = error?.stderr?.toString?.() || ''; diff --git a/src/utils/process.test.ts b/src/utils/process.test.ts index 8d73db5..d88e169 100644 --- a/src/utils/process.test.ts +++ b/src/utils/process.test.ts @@ -1,11 +1,32 @@ +import * as fs from 'fs'; +import { spawn } from 'child_process'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { + captureProcessIdentity, findExecutablePath, getShellExecutable, + parseLinuxProcStat, parseWindowsNetstatOutput, readCommandVersion, + terminateOwnedProcessTree, } from './process.js'; +function waitForExit(pid: number, timeoutMs = 3000): Promise { + const deadline = Date.now() + timeoutMs; + return new Promise((resolve, reject) => { + const poll = () => { + if (!captureProcessIdentity(pid)) { + resolve(); + } else if (Date.now() >= deadline) { + reject(new Error(`process ${pid} did not exit`)); + } else { + setTimeout(poll, 25); + } + }; + poll(); + }); +} + describe('getShellExecutable', () => { it('uses cmd.exe on Windows when ComSpec is missing', () => { expect(getShellExecutable('win32', {})).toBe('cmd.exe'); @@ -62,3 +83,53 @@ describe('readCommandVersion', () => { expect(execSpy).toHaveBeenCalledWith('ffmpeg --version', expect.any(Object)); }); }); + +describe('process ownership', () => { + it('parses immutable Linux ownership fields', () => { + if (process.platform !== 'linux') return; + const stat = fs.readFileSync(`/proc/${process.pid}/stat`, 'utf-8'); + const identity = parseLinuxProcStat(stat); + expect(identity).toMatchObject({ pid: process.pid }); + expect(identity?.processGroupId).toBeGreaterThan(0); + expect(identity?.sessionId).toBeGreaterThan(0); + expect(identity?.startTime).toMatch(/^\d+$/); + }); + + it('terminates only the exact detached process session it owns', async () => { + if (process.platform === 'win32') return; + const owned = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { + detached: true, + stdio: 'ignore', + }); + const unrelated = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { + detached: true, + stdio: 'ignore', + }); + owned.unref(); + unrelated.unref(); + + const ownedIdentity = captureProcessIdentity(owned.pid!); + const unrelatedIdentity = captureProcessIdentity(unrelated.pid!); + expect(ownedIdentity?.sessionId).toBe(owned.pid); + expect(unrelatedIdentity?.sessionId).toBe(unrelated.pid); + + try { + await expect( + terminateOwnedProcessTree(ownedIdentity, { graceMs: 200 }), + ).resolves.toBe(true); + await waitForExit(owned.pid!); + expect(captureProcessIdentity(unrelated.pid!)).not.toBeNull(); + + await expect( + terminateOwnedProcessTree( + ownedIdentity && { ...ownedIdentity, startTime: `${ownedIdentity.startTime}-reused` }, + { graceMs: 20 }, + ), + ).resolves.toBe(false); + expect(captureProcessIdentity(unrelated.pid!)).not.toBeNull(); + } finally { + await terminateOwnedProcessTree(unrelatedIdentity, { graceMs: 200 }); + await waitForExit(unrelated.pid!); + } + }); +}); diff --git a/src/utils/process.ts b/src/utils/process.ts index 7d13cb5..a4a709d 100644 --- a/src/utils/process.ts +++ b/src/utils/process.ts @@ -1,7 +1,34 @@ -import { execSync, spawn, type ChildProcess, type SpawnOptions } from 'child_process'; +import * as fs from 'fs'; +import { + execFileSync, + execSync, + spawn, + type ChildProcess, + type SpawnOptions, +} from 'child_process'; type ExecSyncLike = typeof execSync; +/** + * Immutable identity for a process which started an isolated process session. + * + * A PID alone is not sufficient ownership proof because the operating system + * can reuse it. `startTime` lets cleanup reject a recycled PID, while the + * process/session group ids let ProofShot terminate only descendants created + * by the detached process it started. + */ +export interface ProcessIdentity { + pid: number; + processGroupId: number; + sessionId: number; + startTime: string; +} + +export interface TerminateProcessTreeOptions { + graceMs?: number; + pollIntervalMs?: number; +} + export function getShellExecutable( platform = process.platform, env: NodeJS.ProcessEnv = process.env, @@ -23,6 +50,215 @@ export function spawnShellCommand( }); } +/** Parse the ownership fields from Linux `/proc//stat`. */ +export function parseLinuxProcStat(stat: string): ProcessIdentity | null { + const closeParen = stat.lastIndexOf(')'); + if (closeParen < 0) return null; + + const pid = Number(stat.slice(0, stat.indexOf(' '))); + const fields = stat.slice(closeParen + 2).trim().split(/\s+/); + const processGroupId = Number(fields[2]); + const sessionId = Number(fields[3]); + const startTime = fields[19]; + + if ( + !Number.isInteger(pid) || + !Number.isInteger(processGroupId) || + !Number.isInteger(sessionId) || + !startTime + ) { + return null; + } + + return { pid, processGroupId, sessionId, startTime }; +} + +/** + * Capture the current immutable identity for a process. + * Returns null when the process is already gone or cannot be inspected. + */ +export function captureProcessIdentity(pid: number): ProcessIdentity | null { + if (!Number.isInteger(pid) || pid <= 0) return null; + + if (process.platform === 'linux') { + try { + return parseLinuxProcStat(fs.readFileSync(`/proc/${pid}/stat`, 'utf-8')); + } catch { + return null; + } + } + + if (process.platform !== 'win32') { + try { + const output = execFileSync( + 'ps', + ['-o', 'pgid=', '-o', 'sid=', '-o', 'lstart=', '-p', String(pid)], + { encoding: 'utf-8', stdio: ['ignore', 'pipe', 'pipe'] }, + ).trim(); + const match = output.match(/^(\d+)\s+(\d+)\s+(.+)$/); + if (!match) return null; + return { + pid, + processGroupId: Number(match[1]), + sessionId: Number(match[2]), + startTime: match[3], + }; + } catch { + return null; + } + } + + // Windows has no /proc-style start token available through Node. Keep the + // identity scoped to the exact PID; taskkill below still targets only it and + // its descendants rather than using a command-name match. + try { + process.kill(pid, 0); + return { pid, processGroupId: pid, sessionId: pid, startTime: `pid:${pid}` }; + } catch { + return null; + } +} + +export function processIdentityMatches(identity: ProcessIdentity): boolean { + const current = captureProcessIdentity(identity.pid); + return Boolean(current && identitiesMatch(current, identity)); +} + +function identitiesMatch(left: ProcessIdentity, right: ProcessIdentity): boolean { + return ( + left.pid === right.pid && + left.processGroupId === right.processGroupId && + left.sessionId === right.sessionId && + left.startTime === right.startTime + ); +} + +function listProcessGroupsInSession(sessionId: number): number[] { + const groups = new Set(); + + if (process.platform === 'linux') { + let entries: string[] = []; + try { + entries = fs.readdirSync('/proc'); + } catch { + return []; + } + + for (const entry of entries) { + if (!/^\d+$/.test(entry)) continue; + try { + const identity = parseLinuxProcStat( + fs.readFileSync(`/proc/${entry}/stat`, 'utf-8'), + ); + if (identity?.sessionId === sessionId) { + groups.add(identity.processGroupId); + } + } catch { + // The process may exit while /proc is being scanned. + } + } + return [...groups]; + } + + if (process.platform !== 'win32') { + try { + const output = execFileSync('ps', ['-axo', 'pgid=,sid='], { + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + }); + for (const line of output.split(/\r?\n/)) { + const match = line.trim().match(/^(\d+)\s+(\d+)$/); + if (match && Number(match[2]) === sessionId) { + groups.add(Number(match[1])); + } + } + } catch { + return []; + } + } + + return [...groups]; +} + +export function ownedProcessTreeIsAlive(identity: ProcessIdentity): boolean { + if (process.platform === 'win32') return processIdentityMatches(identity); + + const current = captureProcessIdentity(identity.pid); + if (current && !identitiesMatch(current, identity)) return false; + + return listProcessGroupsInSession(identity.sessionId).length > 0; +} + +function signalOwnedTree(identity: ProcessIdentity, signal: NodeJS.Signals): boolean { + if (process.platform === 'win32') return false; + + const current = captureProcessIdentity(identity.pid); + if (current && !identitiesMatch(current, identity)) return false; + + // Detached children created by ProofShot are session leaders. If that leader + // has already exited, its session id cannot be reused while descendants from + // that session remain, so scanning the recorded session stays ownership-safe. + if (identity.sessionId !== identity.pid) return false; + const groups = listProcessGroupsInSession(identity.sessionId); + if (groups.length === 0) return false; + + let signalled = false; + for (const groupId of groups) { + if (!Number.isInteger(groupId) || groupId <= 0) continue; + try { + process.kill(-groupId, signal); + signalled = true; + } catch { + // A group can exit between discovery and signalling. + } + } + return signalled; +} + +/** + * Terminate only the detached process session represented by `identity`. + * Missing/already-dead processes are an idempotent no-op. A recycled PID is + * rejected rather than widening cleanup to a name or port match. + */ +export async function terminateOwnedProcessTree( + identity: ProcessIdentity | null | undefined, + options: TerminateProcessTreeOptions = {}, +): Promise { + if (!identity) return false; + + if (process.platform === 'win32') { + if (!processIdentityMatches(identity)) return false; + try { + execFileSync('taskkill', ['/F', '/T', '/PID', String(identity.pid)], { + stdio: 'pipe', + }); + return true; + } catch { + return false; + } + } + + if (!ownedProcessTreeIsAlive(identity)) return false; + const signalled = signalOwnedTree(identity, 'SIGTERM'); + if (!signalled) return false; + + const graceMs = options.graceMs ?? 1500; + const pollIntervalMs = options.pollIntervalMs ?? 50; + const deadline = Date.now() + graceMs; + while (Date.now() < deadline && ownedProcessTreeIsAlive(identity)) { + await new Promise((resolve) => setTimeout(resolve, pollIntervalMs)); + } + + if (ownedProcessTreeIsAlive(identity)) { + signalOwnedTree(identity, 'SIGKILL'); + const killDeadline = Date.now() + 500; + while (Date.now() < killDeadline && ownedProcessTreeIsAlive(identity)) { + await new Promise((resolve) => setTimeout(resolve, pollIntervalMs)); + } + } + return true; +} + export function parseWindowsNetstatOutput(output: string, port: number): number[] { const pids = new Set(); From f71514bf0dbe6a7c754a740ce7d09ba7b181b95a Mon Sep 17 00:00:00 2001 From: justinTM <9123665+justinTM@users.noreply.github.com> Date: Tue, 28 Jul 2026 11:47:14 -0700 Subject: [PATCH 2/2] fix: close isolated session review gaps --- src/commands/lifecycle.integration.test.ts | 10 +++-- src/commands/stop.test.ts | 43 ++++++++++++++++++++++ src/commands/stop.ts | 41 +++++++++++++++++---- src/utils/config.test.ts | 12 ++++++ src/utils/config.ts | 4 ++ src/utils/process.ts | 17 ++++++--- 6 files changed, 110 insertions(+), 17 deletions(-) diff --git a/src/commands/lifecycle.integration.test.ts b/src/commands/lifecycle.integration.test.ts index 2ee54f8..657756a 100644 --- a/src/commands/lifecycle.integration.test.ts +++ b/src/commands/lifecycle.integration.test.ts @@ -291,7 +291,9 @@ describe('isolated CLI lifecycle', () => { expect(processIsAlive(ownedServerPid)).toBe(true); expect(processIsAlive(unrelated.pid!)).toBe(true); - const execResult = runCli(audit, env, ['exec', 'get', 'url']); + const nestedCwd = path.join(audit, 'nested', 'consumer'); + fs.mkdirSync(nestedCwd, { recursive: true }); + const execResult = runCli(nestedCwd, env, ['exec', 'get', 'url']); expect(execResult.status, `${execResult.stdout}\n${execResult.stderr}`).toBe(0); expect(execResult.stdout.trim()).toBe(intendedUrl); expect(execResult.stdout).not.toContain('about:blank'); @@ -316,7 +318,7 @@ describe('isolated CLI lifecycle', () => { }, }; fs.writeFileSync(controlPath, JSON.stringify(mismatchedState, null, 2) + '\n'); - const mismatchedExec = runCli(audit, env, ['exec', 'get', 'url']); + const mismatchedExec = runCli(nestedCwd, env, ['exec', 'get', 'url']); expect(mismatchedExec.status).toBe(1); expect(mismatchedExec.stderr).toContain( 'Browser ownership no longer matches this ProofShot session', @@ -326,7 +328,7 @@ describe('isolated CLI lifecycle', () => { ); fs.writeFileSync(controlPath, JSON.stringify(state, null, 2) + '\n'); - const stop = runCli(audit, env, ['stop']); + const stop = runCli(nestedCwd, env, ['stop']); expect(stop.status, `${stop.stdout}\n${stop.stderr}`).toBe(0); expect(fs.existsSync(controlPath)).toBe(false); await waitForProcessExit(ownedServerPid); @@ -341,7 +343,7 @@ describe('isolated CLI lifecycle', () => { const summaryMtimeBefore = fs.statSync(summaryPath).mtimeMs; const browserLogBefore = fs.readFileSync(tools.browserLog, 'utf-8'); - const secondStop = runCli(audit, env, ['stop']); + const secondStop = runCli(nestedCwd, env, ['stop']); expect(secondStop.status, `${secondStop.stdout}\n${secondStop.stderr}`).toBe(0); expect(secondStop.stdout).toContain('already stopped'); expect(fs.readFileSync(summaryPath, 'utf-8')).toBe(summaryBefore); diff --git a/src/commands/stop.test.ts b/src/commands/stop.test.ts index b0e68d7..56dedfc 100644 --- a/src/commands/stop.test.ts +++ b/src/commands/stop.test.ts @@ -176,6 +176,49 @@ describe('stopCommand retryability', () => { expect(fs.statSync(summaryPath).mtimeMs).toBe(summaryMtimeBefore); }); + it('persists collected console evidence before cleanup failure and reuses it', async () => { + mocks.getConsoleErrors.mockReturnValue('synthetic console failure'); + mocks.getConsoleOutput.mockReturnValue('captured before cleanup'); + mocks.getConsoleOutputJson.mockReturnValue([ + { type: 'error', text: 'synthetic console failure', timestamp: Date.now() }, + ]); + mocks.stopOwnedServer.mockRejectedValueOnce(new Error('simulated server cleanup failure')); + + await expect(stopCommand({})).rejects.toThrow('simulated server cleanup failure'); + + expect(session).toMatchObject({ + recordingActive: false, + consoleEvidenceAvailable: true, + consoleErrorCount: 1, + }); + expect(fs.readFileSync(path.join(session.sessionDir, 'console-errors.log'), 'utf-8')).toBe( + 'synthetic console failure', + ); + expect(fs.readFileSync(path.join(session.sessionDir, 'console-output.log'), 'utf-8')).toBe( + 'captured before cleanup', + ); + + mocks.canAddressOwnedBrowserSession.mockReturnValue(false); + mocks.stopOwnedServer.mockResolvedValue(undefined); + mocks.writeViewer.mockReturnValue(path.join(session.sessionDir, 'viewer.html')); + await stopCommand({}); + + expect(mocks.writeViewer).toHaveBeenCalledWith( + session.sessionDir, + expect.objectContaining({ + consoleEvidenceAvailable: true, + consoleErrorCount: 1, + consoleOutput: 'captured before cleanup', + consoleEntries: [ + expect.objectContaining({ text: '[error] synthetic console failure' }), + ], + }), + ); + const summary = fs.readFileSync(path.join(session.sessionDir, 'SUMMARY.md'), 'utf-8'); + expect(summary).toContain('1 error(s) detected'); + expect(summary).toContain('synthetic console failure'); + }); + it('skips every session-addressed browser command when identity is mismatched', async () => { mocks.canAddressOwnedBrowserSession.mockReturnValue(false); mocks.writeViewer.mockReturnValue(path.join(session.sessionDir, 'viewer.html')); diff --git a/src/commands/stop.ts b/src/commands/stop.ts index a32261e..a407a91 100644 --- a/src/commands/stop.ts +++ b/src/commands/stop.ts @@ -132,6 +132,9 @@ export async function stopCommand(options: StopOptions): Promise { let consoleErrors = ''; let consoleOutput = ''; let consoleEntries: TimestampedLogEntry[] = []; + const consoleErrorsPath = path.join(session.sessionDir, 'console-errors.log'); + const consoleOutputPath = path.join(session.sessionDir, 'console-output.log'); + const consoleEntriesPath = path.join(session.sessionDir, 'console-entries.json'); if (browserSessionAvailable) { try { consoleErrors = getConsoleErrors(session.sessionName); @@ -145,15 +148,37 @@ export async function stopCommand(options: StopOptions): Promise { } catch { // Browser may already be closed } - } - - // Write console output to file (before closing browser) - if (consoleOutput.trim()) { - fs.writeFileSync(path.join(session.sessionDir, 'console-output.log'), consoleOutput); + writeTextFileAtomically(consoleErrorsPath, consoleErrors); + writeTextFileAtomically(consoleOutputPath, consoleOutput); + writeTextFileAtomically( + consoleEntriesPath, + JSON.stringify(consoleEntries, null, 2) + '\n', + ); + const capturedErrorLines = consoleErrors + .split('\n') + .filter((line) => line.trim() && line.trim() !== 'No errors'); + session.consoleEvidenceAvailable = true; + session.consoleErrorCount = + capturedErrorLines.length > 0 && consoleErrors.trim() !== '' + ? capturedErrorLines.length + : 0; + // Persist evidence before any cleanup step can fail. A retry must not turn + // successfully collected browser facts into an "unavailable" claim. + saveSession(session, controlDir); } else if (priorConsoleEvidenceAvailable) { - const savedConsoleOutput = path.join(session.sessionDir, 'console-output.log'); - if (fs.existsSync(savedConsoleOutput)) { - consoleOutput = fs.readFileSync(savedConsoleOutput, 'utf-8'); + if (fs.existsSync(consoleErrorsPath)) { + consoleErrors = fs.readFileSync(consoleErrorsPath, 'utf-8'); + } + if (fs.existsSync(consoleOutputPath)) { + consoleOutput = fs.readFileSync(consoleOutputPath, 'utf-8'); + } + if (fs.existsSync(consoleEntriesPath)) { + try { + const savedEntries = JSON.parse(fs.readFileSync(consoleEntriesPath, 'utf-8')); + if (Array.isArray(savedEntries)) consoleEntries = savedEntries; + } catch { + // Keep the persisted availability/count; only the optional timeline is absent. + } } } diff --git a/src/utils/config.test.ts b/src/utils/config.test.ts index 5bd1bfd..4ed95a4 100644 --- a/src/utils/config.test.ts +++ b/src/utils/config.test.ts @@ -40,4 +40,16 @@ describe('loadConfig', () => { ignoreHttpsErrors: false, }); }); + + it('resolves control output against the ancestor config from a subdirectory', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'proofshot-output-path-')); + const nested = path.join(tempDir, 'nested', 'consumer'); + fs.mkdirSync(nested, { recursive: true }); + fs.writeFileSync( + path.join(tempDir, 'proofshot.config.json'), + JSON.stringify({ output: './project-proof' }), + ); + + expect(loadConfig(nested).output).toBe(path.join(tempDir, 'project-proof')); + }); }); diff --git a/src/utils/config.ts b/src/utils/config.ts index 4209ceb..b0d4a37 100644 --- a/src/utils/config.ts +++ b/src/utils/config.ts @@ -77,6 +77,10 @@ export function loadConfig(startDir?: string): ProofShotConfig { return { ...DEFAULT_CONFIG, ...parsed, + output: path.resolve( + configDir, + typeof parsed.output === 'string' ? parsed.output : DEFAULT_CONFIG.output, + ), devServer: { ...DEFAULT_CONFIG.devServer, ...parsed.devServer }, viewport: { ...DEFAULT_CONFIG.viewport, ...parsed.viewport }, browser: resolvedBrowser, diff --git a/src/utils/process.ts b/src/utils/process.ts index a4a709d..2a9f5e6 100644 --- a/src/utils/process.ts +++ b/src/utils/process.ts @@ -108,12 +108,19 @@ export function captureProcessIdentity(pid: number): ProcessIdentity | null { } } - // Windows has no /proc-style start token available through Node. Keep the - // identity scoped to the exact PID; taskkill below still targets only it and - // its descendants rather than using a command-name match. + // PowerShell exposes the process creation timestamp. If that immutable token + // cannot be read, refuse ownership instead of treating a reusable PID as + // sufficient proof for taskkill /T. try { - process.kill(pid, 0); - return { pid, processGroupId: pid, sessionId: pid, startTime: `pid:${pid}` }; + const script = + `(Get-Process -Id ${pid} -ErrorAction Stop).StartTime.ToUniversalTime().Ticks`; + const startTime = execFileSync( + 'powershell.exe', + ['-NoLogo', '-NoProfile', '-NonInteractive', '-Command', script], + { encoding: 'utf-8', stdio: ['ignore', 'pipe', 'pipe'] }, + ).trim(); + if (!/^\d+$/.test(startTime)) return null; + return { pid, processGroupId: pid, sessionId: pid, startTime }; } catch { return null; }