Release: version packages#130
Conversation
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved GitHub Actions PR — db5e7a1f
This PR was opened by trusted GitHub Actions automation.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
tangletools · auto-approval · reason: github_actions_author · 2026-07-03T13:42:09Z
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Concerns | 1 (1 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 51.2s (2 bridge agents) |
| Total | 51.2s |
💰 Value — sound
Version-bump release PR that publishes the 0.35.2 fix flipping the ffmpeg probe's failure bias from optimistic to pessimistic — a correct, well-reasoned asymmetry fix.
- What it does: This PR is the automated Changesets release vehicle: it bumps package.json from 0.35.1 to 0.35.2 (single line, package.json:4) so merging triggers OIDC npm publish. The substantive change it releases is in src/ffmpeg-availability.ts (commit c85666e): isPlaywrightFfmpegAvailable() now returns false (disable video) whenever ffmpeg cannot be positively confirmed — missing browsers cache dir, cache wi
- Goals it achieves: Stop the Tangle agent-thin sandbox from hard-crashing at context.newPage(). The sandbox launches Chromium from Nix via executablePath and never runs
playwright installagainst PLAYWRIGHT_BROWSERS_PATH, so that cache dir does not exist; the 0.35.1 'bias toward true on a missing dir' kept recordVideo on and Playwright then threw 'Executable doesn't exist at .../ffmpeg-/ffmpeg-linux' killing t - Assessment: Correct and in the codebase's grain. The 0.35.1 commit (fa9630c) introduced graceful degrade but had a real blind spot for the missing-dir case; this fix closes exactly that hole rather than reworking the mechanism. The asymmetry argument is sound and stated in the JSDoc (src/ffmpeg-availability.ts:38-58): a wrong 'available' hard-crashes the run, a wrong 'unavailable' only loses the replay video,
- Better / existing approach: none — this is the right approach. I verified the probe lives in the single dedicated module (src/ffmpeg-availability.ts) consumed only by src/cli/commands/run.ts:362, and grep for 'recordVideo' / 'ffmpeg' shows no parallel/duplicate detection logic elsewhere that this reinvents. It extends the existing 0.35.1 graceful-degrade seam rather than introducing a new one. The bundled-mode (=0) carve-out
- 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
Release PR publishes the ffmpeg-cache fix (#129), which is correctly wired into both video-recording paths and soundly inverts the probe bias toward crash-avoidance.
- Integration: Fully reachable: isPlaywrightFfmpegAvailable is called from src/cli/commands/run.ts:360 (bad run) and src/showcase/index.ts:48 (bad showcase); the new playwrightBrowsersPath diagnostic helper is consumed at run.ts:362. Both are live on the real browser-launch path, not speculative.
- Fit with existing patterns: Extends the existing 0.35.1 graceful-degrade pattern (commit fa9630c) rather than competing with it — same probe, same call sites, just corrected bias. No parallel mechanism introduced.
- Real-world viability: Robust across error paths: missing cache dir -> false, cache-without-ffmpeg -> false, ENOTDIR (path is a file) caught -> false, probe exception -> false. Only positively-found ffmpeg or PLAYWRIGHT_BROWSERS_PATH=0 (package-bundled) yield true. The asymmetry rationale (wrong-available hard-crashes vs wrong-unavailable skips video) is correct and documented in-code.
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
🎯 Usefulness Audit
🟡 showcase warning not updated with cache-path/version detail [ergonomics] ``
run.ts:361-366 now names the resolved cache path and driver version in its disabled-video warning, but the parallel warning in src/showcase/index.ts:50-52 still uses the older generic message. Cosmetic inconsistency only — both paths disable video correctly; the showcase one is just less self-verifying. Optional: lift the richer message into the shared ffmpeg-availability module so both callers print the same diagnostic.
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.
✅ No Blockers —
|
| glm | deepseek | aggregate | |
|---|---|---|---|
| Readiness | 95 | 95 | 95 |
| Confidence | 70 | 70 | 70 |
| Correctness | 95 | 95 | 95 |
| Security | 95 | 95 | 95 |
| Testing | 95 | 95 | 95 |
| Architecture | 95 | 95 | 95 |
Reviewer score is advisory once the run is complete and the verdict has no blockers.
Full multi-shot audit completed 2/2 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 2 changed files. Global verifier still owns final merge decision.
No findings.
tangletools · 2026-07-03T13:45:41Z · trace
tangletools
left a comment
There was a problem hiding this comment.
✅ Clean — db5e7a1f
Full multi-shot audit completed 2/2 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 2 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-03T13:45:41Z · immutable trace
This PR was opened by the Changesets release GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated.
Releases
@tangle-network/browser-agent-driver@0.35.2
Patch Changes
c85666eThanks @AtelyPham! - 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 disabledrecordVideowhen the cache dir existed but had no ffmpeg; in the Tangle agent-thin sandboxPLAYWRIGHT_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" keptrecordVideoand still crashed the run atcontext.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-bundledPLAYWRIGHT_BROWSERS_PATH=0mode. Normal dev/CI (whereplaywright installprovides ffmpeg) is unchanged. The disabled-video warning now names the resolved cache path and the driver version.