Skip to content

test(make): forbid bare 'uv run' in review-safe gates (#2296)#2299

Open
yastman wants to merge 1 commit into
devfrom
fix/2296-forbid-bare-uv-run-review-gates
Open

test(make): forbid bare 'uv run' in review-safe gates (#2296)#2299
yastman wants to merge 1 commit into
devfrom
fix/2296-forbid-bare-uv-run-review-gates

Conversation

@yastman
Copy link
Copy Markdown
Owner

@yastman yastman commented May 29, 2026

This pull request was created by @kiro-agent on behalf of @yastman 👻

Comment with /kiro fix to address specific feedback or /kiro all to address everything.
Learn about Kiro autonomous agent


Summary

Closes #2296 — guards the read-only review/candidate validation gates against silently regressing to a mutating uv run.

Background

make check (via lint / type-check) uses plain uv run, which auto-syncs the shared project .venv. In review/candidate worktrees this re-points the editable install at the temporary worktree and can switch package versions — the exact footgun observed during PR-queue validation (a later make candidate-check then failed as stale). #2290 added check-frozen / candidate-check (preflight uv sync --frozen --check + run tools via $(UV_RUN_NO_SYNC)), but nothing guarded that read-only guarantee against future regression.

Fix

Per the issue's proposed option, add a Makefile contract test (tests/contract/test_makefile_review_gate_no_autosync_contract.py) asserting:

  • check-frozen and candidate-check exist;
  • check-frozen preflights read-only with uv sync --frozen --check;
  • every uv run inside the review-safe gates uses --no-sync (directly or via $(UV_RUN_NO_SYNC)) — never a bare auto-syncing uv run;
  • check-frozen covers both ruff check and mypy via the no-sync runner;
  • the $(UV_RUN_NO_SYNC) macro itself resolves to --no-sync.

Developer-friendly make check is intentionally not constrained (auto-sync there is fine, per the issue). Detector self-checks pin the helper so the contract can't go vacuous.

Testing (TDD — verified red first)

  • Injecting a bare uv run into check-frozen makes test_review_safe_gates_never_use_bare_uv_run fail with the offending line; restored → green.
  • pytest tests/contract/ -k makefile → 61 passed. Ruff clean.
  • No production code changed (test-only; the Makefile already satisfies the contract thanks to fix: read-only candidate-check and bot no-sync for stale venv safety #2290).

verify:repo-only — no Docker/runtime validation required.

make check (via lint/type-check) uses plain 'uv run', which auto-syncs the
shared project .venv. In review/candidate worktrees this re-points the editable
install at the temp worktree and can switch package versions — observed footgun
during PR-queue validation. #2290 added check-frozen / candidate-check that
preflight with 'uv sync --frozen --check' and run tools via $(UV_RUN_NO_SYNC),
but nothing guarded that read-only guarantee against regression.

Per the issue's proposed fix option, add a Makefile contract test
(tests/contract/test_makefile_review_gate_no_autosync_contract.py) asserting:
- check-frozen and candidate-check exist;
- check-frozen preflights read-only via 'uv sync --frozen --check';
- every 'uv run' inside the review-safe gates uses --no-sync (directly or via
  $(UV_RUN_NO_SYNC)) — never a bare auto-syncing 'uv run';
- check-frozen covers both ruff and mypy via the no-sync runner;
- the $(UV_RUN_NO_SYNC) macro itself resolves to '--no-sync'.

Developer-friendly 'make check' is intentionally NOT constrained (auto-sync
there is fine, per the issue). Detector self-checks pin the helper.

Verified red first: injecting a bare 'uv run' into check-frozen makes the
contract fail with the offending line; restored -> green. 61 makefile-contract
tests pass; ruff clean. No production code changed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

make check mutates shared uv environment in review worktrees

2 participants