Repository navigation
quest: plan just check passing on macOS - #8410
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou'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 8 minutes. View limit details
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. |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head: f205e9d
[P2] Account for the missing Darwin release-plz dependency
In quest/m0/macos-check.md:43–50, the proposed diagnosis assumes a host release-plz shadows the dev shell's version. However, flake.nix:177–179 explicitly includes pkgs.release-plz only when !pkgs.stdenv.hostPlatform.isDarwin, with the comment that it is marked broken on Darwin. The macOS shell therefore supplies no release-plz to take precedence over a host install. On a clean host without that install, the version fixtures cannot invoke the tool at all; adding the proposed minimum-version error alone would still leave the quest's clean-shell goal unmet. Please record this known platform exclusion and plan a supported, pinned Darwin source for release-plz, then verify the fixtures without relying on an inherited host installation.
Direction: the focused m0 follow-up is appropriate, and no runtime/API/wire change is introduced here. Correct the dependency premise before using the plan to implement the macOS fix.
Verification limits: GitHub source review only, covering both changed documents and the relevant flake, test, documentation generator, bridge, and platform workflow context. I did not run quest checks, builds, or tests, or reproduce the failures on macOS.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — two plan corrections inline. The quest framing, milestone, and carla-bridge cause all check out against the tree.
Reviewed changes
- New quest plan:
quest/m0/macos-check.mdsets a greenjust checkon a macOS host as the outcome and lists the three failures seen in #8392 and #8394, separating the one confirmed cause from the two guesses. - m0 index: adds the quest to
quest/m0/README.md's## Requiredlist with a one-line summary.
Two factual details in the Plan are worth correcting before an implementer spends a macOS session on them; both are inline.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏
| ## Goal | ||
|
|
||
| `just check` on a clean `main` checkout passes on a macOS host inside the Nix | ||
| dev shell, as it already does on Linux. Three failures block it today, all |
There was a problem hiding this comment.
Three failures may be one short: #8392 also records a macOS-only clippy failure, an unfulfilled expect(dead_code) at rs/moq-bench/src/host.rs:43. That expectation is gated on not(target_os = "linux") and moq-bench's features never touch host.rs, so just check --all, or any selection that includes moq-bench (a default workspace member), should report it on macOS. If the Done condition is a fully green macOS just check, does this list need a fourth item?
| - `sh/version.test.py`, **guess**: the Linux dev shell provides release-plz | ||
| 0.4.1 (`pkgs.release-plz` in `flake.nix`), while the failing host ran | ||
| 0.3.149, so it likely picked up a non-Nix `release-plz` (e.g. from | ||
| `~/.cargo/bin`) ahead of the dev shell. The flake uses one nixpkgs for both | ||
| systems, so a different darwin build is less likely. If it is PATH, the | ||
| test should fail loud on a release-plz older than it supports instead of | ||
| asserting wrong versions; if the dev shell's darwin release-plz really is | ||
| 0.3.149, align it. |
There was a problem hiding this comment.
The PATH mechanism cannot be what happens on macOS: flake.nix:177-179 includes pkgs.release-plz only outside Darwin ("Marked broken on Darwin in nixpkgs"), so a macOS dev shell ships no release-plz to shadow. What it finds instead is just install's unpinned cargo install ... release-plz (justfile:98), which leaves ~/.cargo/bin on PATH and is the documented non-Nix path. That makes "align the dev shell's darwin release-plz" an override of a package nixpkgs marks broken rather than a one-line fix; the fail-loud option is the one that matches the tree.
Technical details
# release-plz cause on macOS
## Affected sites
- `quest/m0/macos-check.md:43-50` — the `sh/version.test.py` cause and its two options.
- `flake.nix:177-179` — `pkgs.release-plz` sits inside `pkgs.lib.optionals (!pkgs.stdenv.hostPlatform.isDarwin)`, commented "Marked broken on Darwin in nixpkgs, but builds fine on Linux."
- `justfile:98` — `cargo install --locked cargo-shear cargo-sort cargo-semver-checks release-plz`, unpinned, into the user's cargo bin.
- `doc/setup/index.md:34-36` — `just install` is the documented setup path without Nix.
## Required outcome
- The plan states that a macOS dev shell provides no release-plz at all, so the version on a macOS host is whatever the host installed.
- The remaining option matches that state: `sh/version.test.py` (or `just install`) pins or requires the release-plz version the tests need and fails loud on an older one, instead of treating a Darwin dev-shell build as an alignment target.
## Open questions for the human (optional)
- Should the flake override release-plz for Darwin so the dev shell pins it there too, or is an unpinned host install acceptable? The answer decides whether the quest also edits `flake.nix`.
Plans quest/m0/macos-check.md:
just checkpasses on macOS. It covers the three macOS-only failures reported in moq-dev/moq#8392 and moq-dev/moq#8394, and lists the quest under m0's## Required.Decisions
Causes (read from source on Linux): the carla bridge one is confirmed (a crate-level
cfg(linux)empties the crate, so its//!links don't resolve). The doc test cause (a/private/varsymlink mismatch) and the version test cause (a non-Nix release-plz 0.3.149 on PATH, where the dev shell has 0.4.1) are guesses.Impact
Public API: none. Wire: none.
Models
Size: S
Implementer: Claude Code, claude-opus-5-5, unknown effort
(Written by Claude Opus 5.5)
🤖 Generated with Claude Code
Review
Rewritten by
quest reviewafter each round.01a1c39c-1f5b-7253-92ad-1d308fff5f2f, round 1The change only adds a macOS follow-up plan and its index entry, changes no runtime behavior, and clearly distinguishes confirmed evidence from hypotheses while requiring platform verification.