diff --git a/.changeset/ffmpeg-detect-missing-cache.md b/.changeset/ffmpeg-detect-missing-cache.md new file mode 100644 index 0000000..d8158b7 --- /dev/null +++ b/.changeset/ffmpeg-detect-missing-cache.md @@ -0,0 +1,5 @@ +--- +"@tangle-network/browser-agent-driver": patch +--- + +Fix the ffmpeg probe so a **missing** Playwright browsers cache disables video recording instead of keeping it on. The graceful degrade added in 0.35.1 only disabled `recordVideo` when the cache dir existed but had no ffmpeg; in the Tangle agent-thin sandbox `PLAYWRIGHT_BROWSERS_PATH` (`/opt/cache/npm/_playwright`) is never populated — Chromium is launched from Nix via executablePath — so the directory does not exist, and the previous "bias toward true on a missing dir" kept `recordVideo` and still crashed the run at `context.newPage()`. The probe now biases toward disabling video on any uncertainty (missing cache dir, cache without ffmpeg, or a probe error), because a wrong "available" hard-crashes the run while a wrong "unavailable" only skips the replay video; it returns true only when it positively finds an ffmpeg binary, or for the package-bundled `PLAYWRIGHT_BROWSERS_PATH=0` mode. Normal dev/CI (where `playwright install` provides ffmpeg) is unchanged. The disabled-video warning now names the resolved cache path and the driver version. diff --git a/src/cli/commands/run.ts b/src/cli/commands/run.ts index 6ab9058..f9181f5 100644 --- a/src/cli/commands/run.ts +++ b/src/cli/commands/run.ts @@ -3,7 +3,7 @@ import * as path from 'node:path'; import type { BrowserContext } from 'playwright'; import { toAgentConfig } from '../../config.js'; import { buildBrowserLaunchPlan } from '../../browser-launch.js'; -import { isPlaywrightFfmpegAvailable } from '../../ffmpeg-availability.js'; +import { isPlaywrightFfmpegAvailable, playwrightBrowsersPath } from '../../ffmpeg-availability.js'; import { runWalletPreflight, startWalletAutoApprover } from '../../wallet/automation.js'; import { isPersonaId, listPersonaIds, withPersonaDirective } from '../../personas.js'; import { resolveProviderApiKey, resolveProviderModelName, resolveDefaultProvider } from '../../provider-defaults.js'; @@ -359,8 +359,10 @@ export async function runRunCommand(values: CliValues): Promise { // skipped. In normal dev/CI (ffmpeg present) recording is unchanged. const recordVideo = isPlaywrightFfmpegAvailable(); if (!recordVideo) { + const cacheDir = playwrightBrowsersPath() ?? '(bundled in package)'; cliWarn( - 'ffmpeg not found in the Playwright browser cache; recording video disabled for this run ' + + `ffmpeg not found in the Playwright browser cache (${cacheDir}); recording video disabled ` + + `for this run — driver v${readCliVersion()} ` + '(report, screenshots, and trace are still captured).', ); } diff --git a/src/ffmpeg-availability.ts b/src/ffmpeg-availability.ts index a69dd49..49339d9 100644 --- a/src/ffmpeg-availability.ts +++ b/src/ffmpeg-availability.ts @@ -38,24 +38,41 @@ function resolveBrowsersPath(env: NodeJS.ProcessEnv): string | null { * Whether Playwright/patchright's video-recording ffmpeg binary is present. * * patchright resolves ffmpeg from `/ffmpeg-/` — the - * same `PLAYWRIGHT_BROWSERS_PATH` root as the browsers. The Tangle sandbox's - * agent-thin runtime seeds that warm cache with Chromium but NOT ffmpeg, so a + * same `PLAYWRIGHT_BROWSERS_PATH` root as the browsers. When it is missing, a * context opened with `recordVideo` throws at page creation * ("Executable doesn't exist at .../ffmpeg-/ffmpeg-linux") and kills the * whole run. Detecting the absence lets the caller drop `recordVideo` and still * produce report + screenshots + trace. * - * Bias toward `true`: we only return `false` when we can POSITIVELY confirm the - * browsers directory exists but contains no ffmpeg (the exact sandbox case). On - * any uncertainty — path unset/unresolvable/missing, or a probe error — we - * return `true` so normal dev/CI, where `playwright install` provides ffmpeg, - * keeps recording video unchanged. + * Bias toward `false` (disable video) on uncertainty. The failure is + * asymmetric: a wrong "available" HARD-CRASHES the run at page creation, while a + * wrong "unavailable" only skips the replay video. So we return `true` ONLY when + * we can positively confirm an ffmpeg binary exists (or the browsers are bundled + * inside the package via `PLAYWRIGHT_BROWSERS_PATH=0`, whose layout we can't + * cheaply probe). Every other path — cache dir absent, cache dir present without + * ffmpeg, or a probe error — returns `false`. + * + * This is the exact Tangle agent-thin case: Chromium is launched from Nix via + * executablePath and `PLAYWRIGHT_BROWSERS_PATH` points at a warm npm cache + * volume that never had `playwright install` run against it, so the browsers + * directory does not exist. An earlier "bias toward true on a missing dir" + * kept `recordVideo` there and crashed the run. + * + * Normal dev/CI is unaffected: `playwright`/`patchright install` populates the + * cache with `ffmpeg-/`, which the positive check finds → `true`. */ export function isPlaywrightFfmpegAvailable(env: NodeJS.ProcessEnv = process.env): boolean { try { + // Browsers (and ffmpeg) are bundled inside the installed package; we can't + // cheaply probe that layout, so assume ffmpeg shipped alongside them. + if (env.PLAYWRIGHT_BROWSERS_PATH === '0') return true; + const base = resolveBrowsersPath(env); - // Uncertain root → don't disable video. - if (!base || !fs.existsSync(base)) return true; + if (!base) return true; + + // No browsers cache directory → ffmpeg was never downloaded there. Recording + // would crash at newPage(), so report it unavailable (agent-thin case). + if (!fs.existsSync(base)) return false; const binary = ffmpegBinaryName(); for (const entry of fs.readdirSync(base)) { @@ -63,10 +80,20 @@ export function isPlaywrightFfmpegAvailable(env: NodeJS.ProcessEnv = process.env return true; } } - // Browsers dir present but no ffmpeg — the agent-thin sandbox case. + // Cache dir exists but holds no ffmpeg binary. return false; } catch { - // Never let detection failure break browser launch. - return true; + // Probe failure → disable video (crash-avoidance bias, see above). + return false; } } + +/** + * The Playwright browsers cache directory this process would probe for ffmpeg, + * or `null` for the package-internal (`PLAYWRIGHT_BROWSERS_PATH=0`) mode. Exposed + * so callers can surface the resolved path in a diagnostic when video is dropped. + */ +export function playwrightBrowsersPath(env: NodeJS.ProcessEnv = process.env): string | null { + if (env.PLAYWRIGHT_BROWSERS_PATH === '0') return null; + return resolveBrowsersPath(env); +} diff --git a/tests/ffmpeg-availability.test.ts b/tests/ffmpeg-availability.test.ts index a391a98..3efae51 100644 --- a/tests/ffmpeg-availability.test.ts +++ b/tests/ffmpeg-availability.test.ts @@ -2,7 +2,7 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { isPlaywrightFfmpegAvailable } from '../src/ffmpeg-availability.js'; +import { isPlaywrightFfmpegAvailable, playwrightBrowsersPath } from '../src/ffmpeg-availability.js'; describe('isPlaywrightFfmpegAvailable', () => { let tmp: string; @@ -42,13 +42,28 @@ describe('isPlaywrightFfmpegAvailable', () => { expect(isPlaywrightFfmpegAvailable({ PLAYWRIGHT_BROWSERS_PATH: tmp })).toBe(false); }); - it('returns true (uncertain) when the configured browsers path does not exist', () => { + it('returns false when the configured browsers path does not exist (agent-thin case)', () => { + // PLAYWRIGHT_BROWSERS_PATH is set but never populated (Chromium comes from + // Nix via executablePath), so the directory is absent. Recording would crash + // at newPage(), so the probe must report unavailable rather than optimistic. expect( isPlaywrightFfmpegAvailable({ PLAYWRIGHT_BROWSERS_PATH: path.join(tmp, 'does-not-exist') }), - ).toBe(true); + ).toBe(false); }); - it('returns true (uncertain) for the special PLAYWRIGHT_BROWSERS_PATH=0 mode', () => { + it('returns false when the browsers path is a file, not a directory (probe error)', () => { + const asFile = path.join(tmp, 'not-a-dir'); + fs.writeFileSync(asFile, ''); + // readdirSync throws ENOTDIR — the catch must bias toward disabling video. + expect(isPlaywrightFfmpegAvailable({ PLAYWRIGHT_BROWSERS_PATH: asFile })).toBe(false); + }); + + it('returns true for the special PLAYWRIGHT_BROWSERS_PATH=0 (package-bundled) mode', () => { expect(isPlaywrightFfmpegAvailable({ PLAYWRIGHT_BROWSERS_PATH: '0' })).toBe(true); }); + + it('playwrightBrowsersPath echoes the configured path, or null for =0', () => { + expect(playwrightBrowsersPath({ PLAYWRIGHT_BROWSERS_PATH: tmp })).toBe(tmp); + expect(playwrightBrowsersPath({ PLAYWRIGHT_BROWSERS_PATH: '0' })).toBe(null); + }); });