fix: disable video when the Playwright browsers cache is missing (agent-thin follow-up to #127)

[AtelyPham]

Why

Follow-up to #127 / 0.35.1. A live re-test on browser.tangle.tools (run `3245c43a`) still failed with the identical crash:

```
error: browserContext.newPage: Executable doesn't exist at /opt/cache/npm/_playwright/ffmpeg-1011/ffmpeg-linux
```

0.35.1's probe only disabled `recordVideo` when the browsers cache dir existed but had no ffmpeg. But in the Tangle agent-thin sandbox, bad-app's executor sets PLAYWRIGHT_BROWSERS_PATH=/opt/cache/npm/_playwright while Chromium is launched from Nix via executablePath — so playwright install never runs against that path and the directory does not exist. 0.35.1's !fs.existsSync(base) → return true ("bias toward true on uncertainty") therefore kept recordVideo on and still crashed. The absence of the 0.35.1 cliWarn line in the run's stderr.log confirmed the probe returned true.

Fix

The failure is asymmetric: a wrong "available" hard-crashes the whole run at page creation, while a wrong "unavailable" only skips the replay video. So the bias is inverted — disable video on any uncertainty:

case	0.35.1	now
ffmpeg binary positively found	true	true
PLAYWRIGHT_BROWSERS_PATH=0 (package-bundled)	true	true
cache dir missing (agent-thin)	true → crash	false
cache dir present, no ffmpeg	false	false
probe error (e.g. path is a file)	true	false

Normal dev/CI is unchanged: playwright/patchright install populates the cache with ffmpeg-<rev>/<binary>, which the positive check finds → true.

The disabled-video warning now names the resolved cache path and the driver version, so the next live run is self-verifying (its presence proves the new code ran; its absence + a crash would point at version resolution instead of logic).

Validation

• pnpm lint, pnpm check:boundaries (269 files), pnpm build — clean
• tests/ffmpeg-availability.test.ts — 7/7 (added missing-dir→false, probe-error→false, playwrightBrowsersPath)
• Compiled-dist smoke against the exact sandbox env (/opt/cache/npm/_playwright absent) → false

Ships as 0.35.2; bad-app resolves the driver at run time, so prod picks it up on the next run with no redeploy.

[tangletools]

🟡 Value Audit — sound-with-nits

Verdict	sound-with-nits
Concerns	1 (1 weak-concern)
Heuristic	0.0s
Duplication	0.0s
Interrogation	59.5s (2 bridge agents)
Total	59.5s

💰 Value — sound-with-nits

Inverts the ffmpeg-availability probe's uncertainty bias from optimistic (crashes runs) to pessimistic (skips video) — correct asymmetry reasoning, well-tested, in-grain.

• What it does: Flips isPlaywrightFfmpegAvailable() so that any uncertainty (missing cache dir, probe error) returns false (disable video) instead of true. Previously a missing PLAYWRIGHT_BROWSERS_PATH directory returned true and crashed the run at newPage(). Also exposes playwrightBrowsersPath() and enriches the cliWarn diagnostic with the resolved cache path + driver version for self-verifying live runs.
• Goals it achieves: Stop the agent-thin sandbox (Chromium from Nix via executablePath, PLAYWRIGHT_BROWSERS_PATH set but never populated by playwright install) from hard-crashing on recordVideo. The goal is graceful degradation: report + screenshots + trace still produced, only replay video skipped.
• Assessment: Sound. The failure-asymmetry reasoning is correct and well-documented: a wrong 'available' hard-crashes page creation while a wrong 'unavailable' only drops the replay video, so pessimism is the safe bias. The fix is minimal (3 files), reuses the existing helper at both call sites (run.ts:360 and showcase/index.ts:48), adds the two missing test cases (missing dir, ENOTDIR probe error), and the dia
• Better / existing approach: none — this is the right approach. Verified showcase/index.ts already reuses the same probe (no duplication), and the ffmpeg-static helper in showcase/assemble.ts is a distinct npm media-conversion binary, not Playwright's recorder ffmpeg. Cheaper alternatives (e.g. try-spawn ffmpeg) would be slower and less reliable than the filesystem probe for the known layout.
• Model: opencode/zai-coding-plan/glm-5.2
• Bridge attempts: 2
• Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content

🎯 Usefulness — sound

Correctly inverts an optimistic probe to a crash-avoidance bias for an asymmetric failure mode, wired into both existing call sites with no competing pattern.

• Integration: Fully reachable. isPlaywrightFfmpegAvailable() is called from src/cli/commands/run.ts:360 and src/showcase/index.ts:48; its boolean result feeds the recordVideo option of Playwright's launch context at run.ts:444 and run.ts:575. The new playwrightBrowsersPath() helper is consumed immediately in the diagnostic warning at run.ts:362. readCliVersion was already imported (run.ts:17). No dead surface.
• Fit with existing patterns: Fits the grain exactly: it edits the existing probe in place rather than introducing a competing mechanism. The asymmetry rationale (wrong-available hard-crashes at newPage; wrong-unavailable only skips video) is the right frame and is consistent with how the rest of run.ts already degrades gracefully (report + screenshots + trace still captured). The showcase path inherits the fix for free becaus
• Real-world viability: Holds up across the realistic input space: PLAYWRIGHT_BROWSERS_PATH=0 (bundled) → true; unset → default home-cache lookup then probe; set-but-missing → false (the agent-thin crash case); set-to-file (ENOTDIR) → catch → false; set-with-ffmpeg → true. Synchronous single-call probe at launch — no concurrency surface. One minor note (not a finding): the if (!base) return true; at line 71 is effectiv
• Model: opencode/zai-coding-plan/glm-5.2
• Bridge attempts: 1

💰 Value Audit

🟡 Dead contradictory branch: if (!base) return true; at line 71 [maintenance] ``

resolveBrowsersPath() returns null ONLY when PLAYWRIGHT_BROWSERS_PATH === '0' (ffmpeg-availability.ts:23), which is already handled at line 68. So the !base branch at line 71 is unreachable — and its return true contradicts the new 'bias toward false on uncertainty' doctrine the doc comment establishes. Recommend deleting line 71 (and letting the missing-dir check at line 75 cover it) or returning false for consistency. Harmless because unreachable, but it's a doctrinal inconsistency a futur

---

What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass	What it asks
Heuristic	Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication	Do added function/class names already exist elsewhere in the repo?
Value Audit	What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit	Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

_{value-audit · 20260703T114930Z}

[tangletools]

✅ No Blockers — 6374b073

Review health 100/100 · Reviewer score 89/100 · Confidence 80/100 · 3 findings (3 low)

	glm	deepseek	aggregate
Readiness	92	89	89
Confidence	80	80	80
Correctness	92	89	89
Security	92	89	89
Testing	92	89	89
Architecture	92	89	89

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 4/4 planned shots over 4 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 4/4 planned shots over 4 changed files. Global verifier still owns final merge decision.

🟡 LOW playwrightBrowsersPath can throw uncaught on homedir failure, escaping the !recordVideo warning block — src/cli/commands/run.ts

isPlaywrightFfmpegAvailable() (line 360) wraps resolveBrowsersPath()→os.homedir() in try/catch and returns false on error. playwrightBrowsersPath() (line 362) re-calls resolveBrowsersPath()→os.homedir() without a catch. If os.homedir() throws (no $HOME + no /etc/passwd entry, an improbable but possible Docker/no-user case), isPlaywrightFfmpegAvailable catches it → recordVideo=false, then playwrightBrowsersPath throws uncaught → CLI crashes while formatting a warning message. Fix: wrap [line 362](https://github.com/tangle-network

🟡 LOW Dead code: !base check on line 71 is unreachable — src/ffmpeg-availability.ts

After the early return for PLAYWRIGHT_BROWSERS_PATH=0 on line 68, resolveBrowsersPath can never return null — it only returns null for =0 (handled above) and never returns null in the platform-default branches. Line 71's if (!base) return true is dead code. Harmless in practice (same dead branch existed in the old optimistic-bias version), but confusing to readers. Replace with a non-null assertion or remove the check entirely.

🟡 LOW Redundant resolveBrowsersPath call when both exports used together — src/ffmpeg-availability.ts

In run.ts:360-362 isPlaywrightFfmpegAvailable() and playwrightBrowsersPath() each call resolveBrowsersPath(env) independently. The function is cheap (no fs I/O until later), so impact is negligible, but a single shared resolve would avoid the double computation and keep the diagnostic path guaranteed consistent with the probe decision. Optional: have isPlaywrightFfmpegAvailable return {available, path} or cache the resolve. Not blocking.

---

_{tangletools · 2026-07-03T11:53:34Z · trace}

[tangletools]

✅ Approved — 3 non-blocking findings — 6374b073

Full multi-shot audit completed 4/4 planned shots over 4 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 4/4 planned shots over 4 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary

---

_{tangletools · 2026-07-03T11:53:34Z · immutable trace}