[kixelated]

Problem

Concurrent Bun installs relink the root workspace beneath running media tests, producing EEXIST or invalidating an earlier invocation's cached dependencies.

Approach

Use sh/js/install.py as the single coordinated root workspace installer. It locks dependency setup and reuses an unchanged installation based on Bun, manifests, lockfile and postinstall inputs. Every caller chooses frozen validation or update lockfile behavior; an updating install stamps its resulting inputs. Playback remains concurrent.

Route checks/fixers, JS recipes, demos, benchmarks, media suites, tooling and workflow setup through it. Independent Worker/smoke dependency trees and disposable baseline checkouts retain separate installs.

Decided 2026-11-07 (maintainer, /quest-complete), installer scope: ✅ repo-wide. Promote the coordinated installer (test/lib/install.py at head 6e73247) to the single entry point for every root workspace bun install: check/fix, JS recipes, benchmarks, and remaining media suites, with explicit frozen versus updating modes. Not chosen: test entry points only with the broader finding refuted; or landing at test scope with a separate repo-wide quest. Reason: a concurrent uncoordinated install from any parallel session can invalidate a running test's cached install, so only the repo-wide fix removes the race at its root.

Decided 2026-11-06 (maintainer, /quest-complete): ✅ retain pulse-dbus.md until overlapping media runs pass. Two overlapping pause runs pass; the completed quest and its links are removed. Existing PulseAudio isolation is retained.

Impact

• Public API: none. Wire: none.
• Root workspace setup uses the existing uv toolchain and reuses installed dependencies until its inputs or libav output change. just js install frozen|update exposes both modes.

Alternatives

Locking only the install command still permits a later installer to relink dependencies beneath readers. Caching test entry points alone leaves normal development installers able to invalidate that cache. A shared repo-wide installer removes those bypasses.

Validation

• Round-5 validation on f47fcd364b0367ca7050c1cc2d7a2bdf02dbd83c: nix develop -c quest gate just ci passed (exit 0), including all 11,938 Rust tests. Installer, workspace and coverage fixtures passed; quest validation (600 documents) and git diff --check passed.

• Mutation proof: temporarily removed the installer's locking block. nix develop -c python sh/js/install.test.py failed (exit 1) in 0.212 seconds with ready instead of the contention notice. Restored locking and reran successfully. The mutation is not committed. Logs: .tmp/mutation.log, .tmp/overlap-restored.log, .tmp/ci.log, .tmp/review5.json.

• Iteration: merged origin/main, preserving its native-player push-to-talk build and routing setup through the coordinated installer. Added a flushed stderr notice only when the workspace install lock is contended. The existing overlap fixture asserts that real notice.

• nix develop -c quest gate just ci: exit 0 on the committed source (ccde7512afccd0d12abb625b2523296c57bc9221), including all 11,938 Rust tests. nix develop -c python sh/js/install.test.py and quest validation pass. Current logs: .tmp/ci.log, .tmp/review.json.

• Pullfrog api-diff finding refuted against the recorded repo-wide scope decision and existing Nix runner; lock-wait visibility finding fixed. Replies are on both threads.

• Two overlapping just test media --media-cases pause runs and a concurrent root just biome: all exit 0 on the uv-based implementation. The media runs resumed 84/89 frames and each presented 121 frames at 30.3fps, with tones 152/151dB above the floor. No PulseAudio or D-Bus collision.

• Deterministic root/test setup overlap: pass. The test observes the second installer blocked on the real lock before releasing the first; removing the lock makes the test fail. An updating install's changed lockfile is reused correctly.

• just js dry run retains its default check recipe. Coverage and selector fixtures pass. quest check --base origin/main and git diff --check pass.

• Final nix develop -c quest gate just ci: pass (exit 0). Its full Rust suite passed 11,933 tests. Earlier host memlock exhaustion also reproduced on unchanged main and is tracked in moq-dev/moq#8051.

• Native Windows execution is unavailable here. The native Windows path retains sequential recipe support and uses uv for Python; Unix coordinates parallel runs. Dependency/postinstall edits require stopping active consumers before reinstalling.

• Evidence in this checkout: .tmp/ci-clean.log, .tmp/overlap-approved.log, .tmp/media-{a,b}.log, .tmp/mutation.log, .tmp/review4.json.

Review

Claude Code, Claude Opus 5.5, medium effort reviewed f47fcd364b0367ca7050c1cc2d7a2bdf02dbd83c in round 5 of the existing session, with --responses: no findings. Round 4's low-severity finding is fixed: the overlap fixture reads combined stdout/stderr, so a second install's ready output fails the contention assertion instead of hanging.

Decided 2026-11-07 (maintainer, /quest-complete), fourth-round design checkpoint: ✅ continue with the current approach. Not chosen: merge as is, rethink, or drop. No design change; continue the branch's existing review session and fix the combined-stream overlap fixture.

Follow-ups

Host locked-memory capacity is already tracked by moq-dev/moq#8051.

(Written by GPT-6.1 Sol with medium effort)

[kixelated]

Design change: local Claude review identified that serializing installs alone still permits a later install to relink dependencies beneath an earlier run's readers. The harness now checks a dependency and postinstall-input fingerprint under the checkout lock and reuses an unchanged installation. Root installs in the test entry points share that helper. Playback remains concurrent. Two overlapping pause runs pass with this implementation.

(Written by GPT-6.1 Sol with medium effort)

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 5 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 879d3078-5c47-4d64-bc43-0c92ee3a46fe

Commits

Reviewing files that changed from the base of the PR and between fba4180 and f47fcd3.

Files selected for processing: 37

• .github/justfile
• .github/workflows/interop.yml
• .github/workflows/nightly.yml
• .github/workflows/release-js.yml
• bench/browser/run.sh
• demo/boy/justfile
• demo/carla/justfile
• demo/justfile
• demo/relay/justfile
• demo/web/justfile
• js/justfile
• justfile
• quest/m0/test-flakes-2/README.md
• quest/m0/test-flakes-2/pulse-dbus.md
• setup.bat
• sh/api-diff/run.sh
• sh/coverage.test.py
• sh/dispatch.sh
• sh/drafts/check.sh
• sh/js/README.md
• sh/js/coverage.py
• sh/js/install.py
• sh/js/install.test.py
• sh/js/workspace.py
• sh/js/workspace.test.py
• sh/size/collect.sh
• test/audio-quality/ptt.sh
• test/audio-quality/record.sh
• test/audio-quality/run.sh
• test/drain/run.sh
• test/ietf-conformance/run.sh
• test/interop/compat.sh
• test/interop/interop.sh
• test/json-properties.sh
• test/justfile
• test/portrait/run.sh
• test/resume-model.sh

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[kixelated]

Automated review by review (OpenAI)

Reviewed commit 9a2b720 against base e4b1aa0. No actionable findings in the coordinated-installer changes.

Inspected the lock/fingerprint/install/stamp sequence, postinstall inputs and libav output check, updating versus frozen callers, contention regression test, and root-workspace caller migrations. The post-update fingerprint is recorded only after a successful install. Caller paths resolve consistently, and the remaining bare installs found are independent Worker/smoke or disposable baseline workspaces. This assessment uses the documented contract: dependency inputs remain unchanged during concurrent consumers, and native Windows recipes run sequentially.

Verification limits: GitHub-only static review. I did not run the contention test, overlapping media suites, builds, or native Windows setup, or independently verify the reported playback results. Exact-head Check remains in progress: https://github.com/moq-dev/moq/actions/runs/42326904638. This is not runtime validation or merge approval.

(Written by review (OpenAI))

[pullfrog]

ℹ️ No blocking issues — two small suggestions inline.

Reviewed changes

• Coordinated installer — new sh/js/install.py fingerprints bun version, Nix presence, every workspace manifest, the lockfile, and the libav/patches inputs, then either reuses node_modules under a checkout flock or runs one bun install in frozen or update mode.
• Repo-wide routing — check/fix recipes, JS benches, demos, media/interop/audio-quality/drain/portrait/conformance suites, bench and size tooling, sh/dispatch.sh, and the interop, nightly, and release-js workflows all call the installer; separate trees (Smoke, infra/*, demo/pub, disposable baselines) keep their own installs.
• Toolchain wiring — uv declared on the biome, js, drafts, markdown, and gh dispatch modules, new just js install frozen|update recipe, setup.bat on the same path.
• Evidence — sh/js/install.test.py drives a stub bun plus a FIFO to prove contention and post-update reuse; sh/coverage.test.py and sh/js/workspace.test.py adapted.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[kixelated]

Automated review by review (OpenAI)

Incremental review of head f47fcd364b0367ca7050c1cc2d7a2bdf02dbd83c against base fba41806e110029f803818b32c6c7e2d375adcf0, following reviewed head 9a2b7208157125923d8e37f02d64a1c7d5aecfb9.

No new actionable findings. The installer now attempts a nonblocking exclusive lock, emits a flushed stderr notice only on contention, then waits for the same exclusive lock before fingerprinting or installing. The overlap fixture exercises that real notice rather than monkey-patching flock. Combining the second recipe's stderr/stdout means the stub installer's unexpected ready output fails the assertion when locking is removed, instead of leaving the fixture waiting on the wrong stream. Post-update reuse and the single-install assertion remain covered. The main merge preserves the native push-to-talk build while retaining coordinated dependency setup.

Direction: retain the contention notice and regression test; no changes requested in this incremental delta. The unchanged-input concurrency contract and sequential native Windows limitation remain applicable.

Verification limits: GitHub-only static comparison of commits, current base delta, installer and fixture. I did not run quest context, tests, the lock-removal mutation, overlapping playback, builds or Windows setup, and did not independently verify the reported CI results. Main's merged feature changes were not re-reviewed. This is not runtime validation or merge approval.

[pullfrog]

✅ No new issues found.

Reviewed changes

• Contention notice — sh/js/install.py now probes the install lock with LOCK_EX | LOCK_NB, prints waiting for another workspace dependency install to stderr when contended, then blocks on the same lock; uncontended reuse stays silent. Closes the earlier "silently looks hung" thread.
• Fixture reads the real notice — sh/js/install.test.py dropped its sitecustomize lock tracer and asserts the message produced by the actual recipe path, so removing the locking block fails the test.
• Unchanged since the prior review — the repo-wide routing, fingerprint inputs, uv tool wiring, and quest deletion were re-read in full and hold; the api-diff --ignore-scripts concern is withdrawn against the recorded scope decision (I verified api-diff.yml runs nix develop --command just api-diff).
• Scope check — no root-workspace bun install call site remains; the surviving ones are separate trees (test/smoke, infra/*, demo/pub, the workers.sh loop that skips the root lockfile) or disposable worktree baselines.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}