Fix #2280: make lint/security auto-sync venv in fresh worktrees

[jwbron]

Closes #2280.

Summary

• Switch tool-resolution variables (RUFF, PYTEST, MYPY, YAMLLINT, BANDIT, PYTHON) from := (immediate) to = (recursive) so the wildcard $(VENV_BIN)/...) re-evaluates after a prereq has populated .venv. The old behavior locked the variable at parse time, which is why even adding a uv sync prereq wasn't enough on its own.
• Add sync-venv-if-uv as a prereq to lint-python, lint-yaml, lint-custom, lint-python-fix, lint-yaml-fix, and security — mirroring the pattern already used by test/test-all.

Why this is a two-part fix

I initially tried just (2). The prereq did run uv sync and populate .venv/bin/ruff, but make lint-python still failed with make: ruff: No such file or directory because RUFF := had already been resolved to bare ruff at Make parse time, before any recipe ran. Switching to = makes the wildcard re-evaluate at recipe-expansion time, after the prereq has done its work.

Sandbox compatibility

The sandbox container has tools installed globally and no uv:

• sync-venv-if-uv is a no-op when uv isn't on PATH (existing behavior).
• The recursive variable's wildcard returns empty (no .venv), so it falls through to the bare tool name — found on PATH because they're globally installed. Same end state as before.

Targets intentionally left alone

lint-shell, lint-docker, lint-actions, lint-shell-fix use system tools (shellcheck/shfmt/hadolint/actionlint), not venv-resolved tools, so the venv state is irrelevant.

Test plan

• rm -rf .venv && make lint-python — passes (was the reported failure)
• rm -rf .venv && make lint — passes end-to-end
• make security works without manual make deps first
• CI green
• Sandbox image builds and lints/tests still run there (no uv, globally-installed tools)

[egg-reviewer]

Approval

The fix correctly addresses #2280. The two-part diagnosis is right and the explanation in the PR description nails it: adding the sync-venv-if-uv prereq alone wouldn't have worked because RUFF := … was resolved to bare ruff at Make parse time, before any recipe ran. Switching to = defers wildcard evaluation to recipe-expansion time, after the prereq has populated .venv. I reproduced this locally to confirm.

What I checked

• Recursive evaluation timing: Verified with a minimal Makefile that RUFF = $(if $(wildcard …),…,fallback) re-evaluates after a prereq creates the file. Works as the PR claims.
• Phony prereq dedup with make lint: GNU Make deduplicates phony prereqs across sibling targets in a single invocation, so sync-venv-if-uv runs once per make lint even though six sub-targets list it. No multiplicative uv sync cost.
• Sandbox path: command -v uv fails in the sandbox → sync-venv-if-uv is a no-op → wildcard returns empty → falls through to bare tool name on global PATH. End state matches pre-PR behavior.
• CI: All lint.yml and test.yml jobs run uv sync --extra dev before invoking make. The new prereq is a fast no-op against an already-synced lockfile, so no double-sync cost.
• Target-specific PYTHONPATH propagation: lint-python and lint-custom set export PYTHONPATH and add the new prereq. Target-specific exports propagate to prereqs in GNU Make, so sync-venv-if-uv (and the sub-make inside it) runs with PYTHONPATH=shared:gateway:orchestrator. uv sync doesn't import project modules, so this is harmless. The same shape is already in test: and test-all:, so this matches the established pattern.
• Targets correctly left alone: lint-shell, lint-docker, lint-actions, lint-shell-fix use system tools (shellcheck/shfmt/hadolint/actionlint), not venv-resolved tools — no prereq needed. check-linters references $(RUFF) etc. but is a diagnostic; auto-syncing inside it would mask the very state the user wants to inspect, so excluding it is correct.

Side benefit worth noting

.github/scripts/checks/lint_check.py:41 invokes make lint directly without pre-syncing. Before this PR, that path would have hit the same "ruff: No such file or directory" failure on a fresh checkout. This PR makes that call self-bootstrapping.

Non-blocking observations

1. Pre-existing prereq inconsistency (out of scope; flagging only). smoketest-long-poll (line 369) and test-integration / test-e2e / test-security (lines 394/398/402) still depend on venv rather than sync-venv-if-uv, so they will fail in the sandbox the same way lint-python did before this PR. They require Docker/k3s and aren't typically run from the sandbox, so it's mostly moot — but a follow-up unifying the prereq pattern would be nice.

2. Test plan checkboxes unchecked. "CI green" and "Sandbox image builds and lints/tests still run there" are still unchecked. The reasoning in the PR description is sound and matches my analysis, but please confirm these green before merging — particularly that the sandbox image build still passes, since the recursive-variable change does subtly alter the timing of wildcard calls on every recipe invocation.

3. Stat amplification (negligible). Each $(RUFF) reference in a recipe now performs a fresh wildcard (filesystem stat) instead of a single parse-time evaluation. In hot recipes that reference the variable several times, this is a handful of extra stats per make invocation — well below the noise floor.

— Authored by egg

[egg-reviewer]

Re-Review: Approved (unchanged)

The delta since my previous review at 57a11c33 is a single merge commit (c5445f9 — Merge branch 'main' into egg/2280-lint-fresh-venv). No new author commits.

What I re-verified

• Makefile is byte-identical to the previously approved revision. The blob SHA of Makefile at 57a11c33 and at c5445f9 is the same (36e62b7). The merge from main did not touch this file, and there are no merge conflicts to inspect.
• PR diff vs origin/main is unchanged in scope. git diff origin/main..HEAD --stat still reports Makefile | 26 ++++++++++++++------------ (14 insertions, 12 deletions). This is exactly the surface I reviewed before.
• Main commits brought in by the merge are unrelated to this fix. ca4191d (docs update for tester scaffold-first, #2276) and 2f1c1e1 (ruff v0.15.12 sync + formatter sweep, #2297). Neither interacts with the recursive-variable / sync-venv-if-uv-prereq logic. In fact, #2297 makes this PR's bootstrapping behavior more valuable on fresh worktrees, since it pins a newer ruff that a stale .venv would not satisfy.

Status of prior non-blocking observations

Carried over from my previous review — none are blocking, restating only because the PR is open longer now:

1. Unchecked test-plan boxes ("CI green", "sandbox image still lints/tests"). Worth a final glance at the latest GitHub Actions run before merge, particularly because the recursive = change subtly changes when wildcard is evaluated on every recipe invocation.
2. Pre-existing venv-prereq inconsistency on smoketest-long-poll, test-integration, test-e2e, test-security — out of scope for this PR; flagged as future cleanup only.
3. Stat amplification from recursive expansion — negligible.

My previous approval stands. Ship it once CI is green.

— Authored by egg

[james-in-a-box]

Both reviews are APPROVED. No code changes pushed because the feedback contained no actionable items for this PR — only non-blocking observations:

1. Pre-existing venv-prereq inconsistency on smoketest-long-poll, test-integration, test-e2e, test-security — the reviewer explicitly flagged this as "out of scope for this PR; flagged as future cleanup only." Those targets require Docker/k3s and aren't run from the sandbox, so they don't share the failure mode this PR fixes. Worth a follow-up issue if we want a unified prereq pattern, but not appropriate to bundle here.

2. Unchecked test-plan boxes ("CI green", "sandbox image still lints/tests") — these are author/operator verification items, not code changes. They depend on observing CI runs and sandbox image builds.

3. Stat amplification from recursive expansion — reviewer explicitly described as "negligible, well below the noise floor."

The PR is ready to merge once the human verifies CI is green.

— Authored by egg

[james-in-a-box]

egg is addressing review feedback...

[james-in-a-box]

Re-review is APPROVED (unchanged from the prior approval at 57a11c33). No code changes pushed — the re-review carried forward the same three non-blocking observations the reviewer already marked as out-of-scope:

1. Pre-existing venv-prereq inconsistency on smoketest-long-poll, test-integration, test-e2e, test-security — reviewer: "out of scope for this PR; flagged as future cleanup only." Those targets require Docker/k3s, don't run from the sandbox, and don't share the failure mode this PR fixes. Worth a follow-up issue, not a bundled change.
2. Unchecked test-plan boxes ("CI green", "sandbox image still lints/tests") — operator verification items observed at merge time, not code changes.
3. Stat amplification from recursive expansion — reviewer: "negligible, well below the noise floor."

Reviewer's standing instruction: "Ship it once CI is green."

— Authored by egg

[james-in-a-box]

egg feedback addressed. View run logs

4 previous review(s) hidden.