Repository navigation
fix(dev): keep the dev shell out of the MOQ_* namespace - #7979
Conversation
#7710 exported MOQ_QUEST, which every moq binary rejects as an unknown variable. Rename it to DEV_SHELL_QUEST and assert the shell exports no MOQ_*. 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 22 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
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head: 830af86098c4bce10c3fae6b3826271271efbd8b.
Findings: No actionable defects found in the five-file diff. The flake export, launcher, and test fixtures consistently use DEV_SHELL_QUEST; the launcher preserves quoting, arguments, exit status, and existing fallback behavior. The Nix assertion checks the merged platform-specific env keys, and the new name avoids the binaries' strict MOQ_* validation.
Direction: Keep this tooling variable outside both reserved prefixes rather than weakening binary validation. Existing launcher tests cover pinned selection and failure propagation; no public API or wire changes are introduced.
Verification limits: GitHub-only static review of the diff, surrounding launcher/validation code, and tests. I did not run Nix evaluation, Python tests, or interop checks. The head's Check CI job is still running, so this is not a CI pass or merge-readiness claim.
There was a problem hiding this comment.
✅ No blocking issues found. The rename is complete and the guard sits at the right layer.
Verified against the base:
MOQ_QUESTon the base appears only in these five files; nothing else in the tree (docs, workflows,.claude, quest plans) referenced it, so there is no missed consumer. The working tree now has zeroMOQ_QUESThits and nineDEV_SHELL_QUESThits.- The guard's
MOQ_prefix filter matches what the binaries actually refuse:moq_tokio::cli::check_envfilters the process environment byPREFIX = "MOQ_"(rs/moq-tokio/src/cli/env.rs:9), soIN_MOQ_DEV_SHELLcorrectly stays outside the check. Theassert pkgs.lib.assertMsg (...)idiom also matches the repo's existing use innix/runner/config-test.nix. - The guard is exercised in CI: workflows enter the shell via
nix develop --command ...(.github/workflows/interop.yml,ts.yml), so a reintroducedMOQ_*key fails there rather than only in a local shell. - The tests are updated consistently (
sh/local.test.py:28/81/104,sh/dispatch-query.test.py:291,sh/js/workspace.test.py:82), and the stderr assertion atsh/local.test.py:109still matches the script's unchangedDev-shell quest:line.
Optional, non-blocking: the guard inspects only the env set. shellHook (flake.nix:666) is the shell's other export site, so an export MOQ_* added there would reproduce the original failure without tripping the assert. Today's shellHook exports only PATH and TMPDIR, so this is just a note on the comment's "the shell must export none" wording — not a defect.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

Summary
moq-dev/moq#7710 made the dev shell export
MOQ_QUEST. Everymoq-*binary rejects unknownMOQ_*variables, somoq-relay/moqexited 1 inside the dev shell andjust test interop --allfailed before any check.DEV_SHELL_QUEST(outside bothMOQ_*, which the binaries own, andQUEST_*, which quest reads); updatesh/quest.shand the three tests.flake.nixasserts the shell'senvcontains noMOQ_*key, so evaluating the shell fails loudly if one is added again. The binaries' refusal is unchanged.Public API / wire impact: none. Anyone scripting against
MOQ_QUESTshould useDEV_SHELL_QUEST.Testing
MOQ_QUEST=/x target/debug/moq-relay->unknown environment variables ... MOQ_QUEST, exit 1. After: the dev shell exports noMOQ_*; relay gets past the env check.MOQ_QUESTfails eval withdev shell must not export MOQ_*.quest gate just test interop --all: all checks passed.quest gate just check: pass.Review
quest review --base origin/main(codex gpt-6.1-sol, medium) at head 830af86: 0 findings.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code