Route every "is this an env file?" check through one strict predicate (3 spellings, 2 loose)

[mikebronner]

Summary

Implements #345.

The issue reads as a consolidation task: three spellings of "is this an env file?", two of them loose. That consolidation is already on main — all four main.rs call sites route through env_key_locator's strict predicate, and the predicate has thorough unit tests. Inspector Lestrade verified the same thing independently during triage and rewrote the AC around what actually remains.

What remained is a test gap, and it is the gap this repo has been bitten by before (#338): a unit test of an extracted predicate proves nothing about whether the dispatch consults it. env_source_registration_gate makes that argument for the Salsa-registration call site. The completion call site had no equivalent — in fact no test in the crate drove the real textDocument/completion request at all.

This PR adds that test module. Behaviour is unchanged; this commit adds tests only.

Changes

• New laravel-lsp/src/tests/env_completion_context_gate.rs (6 tests), registered in src/tests/mod.rs.
• Drives the real completion handler on an LspService harness and asserts on the items it returns, mirroring env_source_registration_gate's "drive the dispatch, read back through the public surface" convention.
• Pins both directions, because a one-directional suite cannot tell a narrowed gate from a closed one:
  • rejected — .envrc, .environment, .env-backup, and a .php file at .envs/deploy.php get no ${...} interpolation;
  • accepted — .env, .env.local, .env.example, .env.testing still do;
  • plus the PHP positive: .envs/deploy.php reaches env('…') call completion.

Acceptance Criteria

Copied from the <!-- acceptance-criteria --> comment on #345.

• The predicates in env_key_locator.rs are the only place env-file classification is decided; every call site delegates. Verified on this branch: main.rs:9066, 21559, 24906, 26748 all call is_env_file_name/path_is_env_file. Already true on main — this PR does not change it, it pins it.
• Grep sweep produces hits only in the three allowed buckets. Pattern validated against known positives first (env_key_locator.rs:50, the predicate body, and the main.rs:24901 explanatory comment) so the count is not vacuous. Seven hits, all in-bucket — full classification below.
• Discriminating cases rejected and accepted at two levels. Level 1 (predicate unit) was already covered in env_key_locator/tests.rs. Level 2 (dispatch) is new and is this PR, driving the real textDocument/completion request end-to-end.
• Full cargo test and cargo clippy --all-targets -- -D warnings clean, with the new tests' names visible. Output pasted below.

Grep sweep — all seven hits, bucketed

$ grep -rn '\.starts_with("\.env")\|\.starts_with("\.env\.")\|\.contains("\.env")\|== "\.env"\|\.ends_with("\.env")' laravel-lsp/
(a) predicate body / doc   env_key_locator.rs:39, :50
(b) call-site rationale    main.rs:24901
(c) test files             tests/env_source_registration_gate.rs:4
                           tests/env_completion_context_gate.rs:6, :143   ← new, doc comments naming the rejected form
                           env_key_locator/tests.rs:268

No live re-implementation. The sweep covered src/, src/tests/, tests/, and the module-adjacent */tests.rs files — the test tree included, which is precisely the scope that was missed twice on #338.

I also ran a broader behavioural sweep (grep -rn '"\.env' across src/ and tests/, ~110 hits) rather than trusting the AC's literal pattern alone. Every hit outside the buckets above is a fixture write, a path join, or the priority ladder — no second classification site. Two adjacent things I checked and am reporting as not gaps:

• main.rs:5911-5919 names .env, .env.local, .env.example outright at startup registration. Not a classification site — it enumerates files rather than deciding env-ness, and the predicate's own doc comment already documents it as the one env path that never asks.
• integration_tests.rs:1358 env_priority is a local shadow of the main.rs:9075 priority ladder. Structurally the fix: 🐛 Remove bash-language-server shellcheck injection & justify .env attach #338 shape, but it is about priority, not classification, so it is outside this issue's unit; it carries no "MUST match production" claim, and the real ladder is already pinned end-to-end by env_priority_ladder_survives_the_gate. Left alone deliberately.

Test Plan

• All existing tests pass — full suite, not a filtered subset.
• New tests cover the change, and each is mutation-verified.
• cargo clippy --all-targets -- -D warnings clean.

Mutation evidence

Every test has a mutation that reddens it — none is coverage theatre. Verified live, and re-verified after the fixtures were edited.

Mutation	Result
main.rs:24906 → path.contains(".env") (the old loose gate)	5 red: 3 rejection tests + the .php interpolation test + the .php env('…') positive
main.rs:24906 → false (gate closed)	1 red: the accepted-variants control
env_key_locator.rs:50 → name == ".env" (variant arm dropped)	1 red: control, naming all three dropped variants — [".env.local", ".env.example", ".env.testing"]

Two details worth stating, both learned from #338's review:

• The accepted-variants control collects its misses and asserts once, instead of asserting inside the loop. An in-loop assert! stops at the first failure, so the third mutation would have been credited by .env.local alone and left the other two variants unproven. As written it names all three.
• The three rejection cases are separate tests, not rows in one loop, so the loose-gate mutation reddens each independently and visibly by name.

One honesty note on the rejection tests: they assert that nothing is offered, which is exactly the shape that can pass for the wrong reason if the typed prefix stops matching the sentinel variable. Every fixture therefore builds its template from a single PREFIX constant — there is one spelling of the prefix in the file — and the two positive tests share that constant, so they fail loudly if it ever stops matching.

cargo test — full suite

running 2548 tests ... ok. 2548 passed; 0 failed
running 561 tests
test tests::env_completion_context_gate::env_backup_is_not_offered_interpolation ... ok
test tests::env_completion_context_gate::envrc_is_not_offered_interpolation ... ok
test tests::env_completion_context_gate::environment_is_not_offered_interpolation ... ok
test tests::env_completion_context_gate::php_under_an_env_named_directory_gets_env_call_completion ... ok
test tests::env_completion_context_gate::php_under_an_env_named_directory_is_not_offered_interpolation ... ok
test tests::env_completion_context_gate::real_env_variants_are_still_offered_interpolation ... ok
... ok. 561 passed; 0 failed
running 80 tests ... ok. 80 passed; 0 failed
running 0 tests ... ok. 0 passed; 0 failed

cargo clippy --all-targets -- -D warnings

Finished `dev` profile [unoptimized + debuginfo] target(s) in 7.27s

Fixes #345

---

Review round 2 — both blockers fixed

Comments and one compile-time assertion. No behaviour change; the six tests, their fixtures, and every assertion are untouched.

Blocker 1 — the SENTINEL rationale described a fixed leak as live

env_completion_context_gate.rs:39 asserted, in the present tense, that the handler appends std::env::vars(). It does not, and has not since 8cb4679 (#343) closed #342. Verified rather than assumed: std::env::vars appears nowhere in main.rs or salsa_impl.rs, and main.rs:26647-26655 states the opposite outright where the items are built.

The defensive choice was right; only the justification was stale. It is now past tense and framed as regression cover — the sentinel guards against the merge returning, and the file no longer contradicts the code it describes.

Blocker 2 — the PREFIX/SENTINEL invariant now fails at its own definition

const _: () = assert!(
    is_proper_prefix(PREFIX.as_bytes(), SENTINEL.as_bytes()),
    "PREFIX must remain a proper prefix of SENTINEL: …"
);

It trips at the constants, at compile time, before any test runs — strictly earlier than the suggested runtime form. The const fn is three-outcome, and each outcome is proven live, not reasoned about:

Mutation	Result
PREFIX = "ZL999_" (byte mismatch)	compile error E0080 at :65, naming the invariant
PREFIX = "ZL345_SENTINEL" (length branch — no longer proper)	compile error E0080, same message
unmutated	compiles; full suite green

Two more of the same class, found by sweeping rather than patching

Fixing the reported instance is not evidence the class was swept, so I re-read every factual claim in the file against the tree. Two more were wrong, neither flagged in review:

• The module doc named only <env name="…"> for the PHPUnit arm. get_phpunit_env_context (main.rs:9312-9313) dispatches on <server name="…"> as well — the exact surface fix: 🐛 Env completion offers the LSP process's own environment, values inline #342 bounced on. Both are now named.
• seed_env_source claimed every assertion below would "pass for the wrong reason" without it. Only the rejection assertions would. The two positive controls fail loudly on an unseeded harness, which is precisely what makes them controls; the doc now says so.

I also re-checked the claims that turned out fine, so the sweep is legible rather than assertion-by-silence: path_is_env_file owning the decision (main.rs:24906), backend_for mirroring env_source_registration_gate (byte-identical), the ◊ marker convention (query_chain_completion_handler.rs:131-139), env_completion_system_leak genuinely pinning the leak removal (assert_no_process_var_leak, with positive controls), and the accepted-variant list matching the predicate's documented set.

One wording correction to my own new prose during self-review: drift in PREFIX reddens both positive controls, not just real_env_variants_are_still_offered_interpolation. The comment now says two.

Verification at 147d73c

• cargo test (full suite, unfiltered): 3199 passed, 0 failed — 2548 lib + 571 bin + 80 integration. All six env_completion_context_gate tests present and passing by name.
• cargo clippy --all-targets -- -D warnings: clean.
• cargo fmt --check: clean.
• The behavioural gate mutation re-verified on this commit, not carried over from round 1: main.rs:24906 → path.contains(".env") reddens 5 tests by name, exactly as the table above the fold claims.

[mr-sherlock-holmes]

✅ Approved

Review Summary

• Reviewed PR #346 at fab8c4d. The diff is one new test module (laravel-lsp/src/tests/env_completion_context_gate.rs, 269 lines) and one registration line in src/tests/mod.rs. No production code changed.
• Each acceptance criterion is met.
• Everything belonging to the coherent unit is clean.

A tests-only PR against an issue titled "route every check through one strict predicate" is the shape that deserves the most suspicion, so I verified criteria 1, 2, and 3 against the tree directly instead of reading them out of the PR body.

AC #1 — the predicates are the only classification site. Confirmed. All four call sites delegate: main.rs:9066 and main.rs:21559 and main.rs:26748 call is_env_file_name, main.rs:24906 calls path_is_env_file. No fifth call site exists.

AC #2 — the grep sweep stays inside the three buckets. I ran the criterion's pattern myself. Seven hits, and I bucketed all seven:

• (a) predicate body and doc: env_key_locator.rs:39, env_key_locator.rs:50
• (b) call-site rationale: main.rs:24901
• (c) test files: env_source_registration_gate.rs:4, env_completion_context_gate.rs:7, env_completion_context_gate.rs:148, env_key_locator/tests.rs:268

No live re-implementation survives.

AC #3 — discriminating cases at the dispatch level. Confirmed. The new module drives the real textDocument/completion request through the LspService, not the predicate. All four rejected names and all four accepted names are exercised end-to-end.

AC #4 — full suite and clippy clean. CI is green on all three platforms.

What is Good

The negative tests cannot pass for the wrong reason, and the file proves it structurally. A rejection test that asserts "nothing is offered" is the classic assertion that passes when the harness is broken. Three separate design choices close that hole:

• PREFIX and SENTINEL are single constants (:42, :50). The positive tests and the negative tests share one spelling of the prefix, so a prefix that stops matching reddens the positives loudly.
• seed_env_source (:67) registers the sentinel through a real did_change. Its doc comment states the reason directly: without it, every assertion reads "no items" and passes.
• Two positive controls run in both directions (:178, :245), so a gate stuck closed fails.

The accepted-variants loop collects misses and asserts once (:259-264). An assert! inside the loop would stop at the first failure and leave the other two variants unproven. This version names all three.

The three rejection cases are separate tests, not rows in one loop. The loose-gate mutation reddens each one independently and by name.

The self-reported non-gaps are the right standard. main.rs:5911 and integration_tests.rs:1358 were both checked, judged out of scope, and disclosed with reasoning rather than left silent. I checked both and agree. main.rs:5911 enumerates named files and classifies nothing. integration_tests.rs:1358 concerns priority, not classification.

📋 Non-blocking follow-ups

• AC #2 names a "known positive" that the criterion's own pattern does not match — main.rs:9069 — the line reads not `starts_with(".env")` and the character before starts_with is a backtick, so the alternative \.starts_with("\.env") never matches it. Only main.rs:24901 is a genuine known positive. The criterion's substance is unaffected, and its anti-vacuity guard still fired, because the sweep returns seven real hits. This PR validated against env_key_locator.rs:50 and main.rs:24901 instead, which was the correct call. Worth recording for anyone who reuses this sweep as a regression check. Disposition: Noted — not tracked.

Ready for @mikebronner to merge.

[mikebronner]

Fix the merge conflicts.

[dr-john-h-watson]

Merge conflicts fixed. The branch now carries a merge of main (05c0b78), and the PR is MERGEABLE.

The conflict was one line. #343 and this branch each registered a new test module in laravel-lsp/src/tests/mod.rs, on the same line. Both belong. I kept both, in alphabetical order — env_completion_context_gate, then env_completion_system_leak. That is the only hand-edit in the merge commit.

I did not stop there, because #343 rewrote the handler these tests drive. It removed the system-environment fallback from ${...} interpolation completion. That is the code my rejection tests assert against, so the pre-merge verification no longer covered the merged tree. I re-ran it:

• The AC sweep still holds. No loose .env spelling survives outside the predicate's own doc comment, two call-site rationales, and test prose. All four classification sites still route through is_env_file_name / path_is_env_file. #343 added no fifth site.
• Both mutations still redden the gate, by name. is_env_file_name → starts_with(".env") fails the three rejection tests. path_is_env_file → path.contains(".env") fails those three plus both .envs/deploy.php cases. Run live on the merged tree, not reasoned about.
• Full suite, clippy, and fmt are clean locally (3199 tests) and green on all three CI platforms.

The push dismissed the earlier approval, so this needs a fresh look at the merge commit.

[mr-sherlock-holmes]

🔄 Changes Requested

One finding. It's small, it's a comment, and I'd still rather it not ship — because it's on a line this PR added and what it says isn't true.

Issues Found

laravel-lsp/src/tests/env_completion_context_gate.rs:39 — the SENTINEL doc comment asserts, in the present tense:

the handler appends std::env::vars() matching the same prefix

It doesn't. std::env::vars() appears nowhere in main.rs or salsa_impl.rs; the completion handler sources its variables exclusively from self.salsa.get_all_parsed_env_vars(). The only three occurrences anywhere in laravel-lsp/src/ are prose inside two test modules.

Why this is worth a round-trip rather than a shrug: that sentence describes #342's process-environment leak as current behaviour. The sibling module you're explicitly mirroring gets the tense right — env_completion_system_leak.rs:4 reads "The handler used to merge std::env::vars()". Yours reads as though the leak is still live, in a file whose whole job is to be the durable explanation of this dispatch. The next person hardening this path — plausibly an agent, following repo conventions — would take it at face value about the exact bug this repo just spent PR #343 closing.

The defensive choice is still correct: a collision-proof sentinel is right regardless of whether the handler merges the process environment. It's only the justification that's stale. Past tense, or reframe as "so the fixture can't pass for the wrong reason should the leak ever regress" — the reasoning survives either way.

What's Good

This is the strongest test-only PR I've reviewed in this repo, and most of it I checked rather than took on trust:

• The thesis is right. "A unit test of an extracted predicate proves nothing about whether the dispatch consults it" is exactly the gap, and no test in the crate drove textDocument/completion at all before this. Building the counterpart to env_source_registration_gate for the completion call site is the correct unit of work.
• The rejection tests aren't vacuous — the failure mode I most expected on a suite where four of six tests assert labels.is_empty(). Tracing the dispatch (main.rs:24906 → the env_ctx branch at 26601-26611) confirms it: if is_env_file were wrongly true, the crafted ${...} would return items and redden the negative, and the paired positive would route to the interpolation helper, find no ${, and redden too. Both directions genuinely pin the gate.
• The harness can't race. did_change's JoinHandle lands in pending_salsa_updates before the call returns, so seed_env_source's .remove() + await guarantees registration completes before any assertion.
• The PREFIX/SENTINEL single-spelling discipline and collecting the control's misses instead of asserting in-loop are both precisely the right defences against tests that pass for the wrong reason — and your own write-up names why.
• The mutation table is honest work, and your grep sweep reproduces exactly: 7 hits, all in-bucket, one live code line (env_key_locator.rs:50, the predicate body itself).
• Reporting the two adjacent non-gaps (main.rs:5911-5919 startup enumeration, integration_tests.rs:1358 priority shadow) with reasoning for leaving them alone is the right call on both counts, and disclosing them beats staying silent.

All four acceptance criteria are met. AC3 is met by a disclosed divergence that's stronger than the wording: the criterion implies one combined assertion, you split it into two independently-reddening tests. Nothing dropped. AC4 I verified against the real CI log rather than the pasted transcript — all six tests present and passing on ubuntu, clippy green on all three platforms.

📋 Non-blocking follow-ups

• AC1 cites main.rs:26748 for semantic_tokens_full; the call is actually at 26725. Triage-annotation drift, not a code defect — Noted, not tracked.
• AC2 names main.rs:9069 as a "known positive" for validating the grep pattern, but that comment writes `starts_with(".env")` after a backtick, not a method-call dot, so the pattern doesn't match it. Only main.rs:24901 is a real known-positive. The bucket count is unaffected. Noted, not tracked — an inaccuracy in the AC text, not the tree.

Unverified Observations

Transparency on method: I ran four blind lens reviewers over the checkout. AC-conformance, correctness, and security all reported and are reflected above. The test-honesty lens did not return before this review's budget was exhausted, so its coverage rests on the correctness lens's dispatch-trace (which independently answered the central vacuity question) and my own CI verification. I also verified the std::env::vars() finding inline by grep rather than through the adversarial panel — for a claim this mechanical, the grep is the stronger evidence. Flagging it so the gap is visible rather than implied.

Fix the comment and re-request review — everything else here is ready.

[mr-sherlock-holmes]

📎 Addendum to my review above — one more blocker, and a correction to my own transparency note.

My review flagged that the test-honesty lens hadn't returned in time. It has now. Two things follow, and I'd rather amend the punch list in the same bounce than make you find this on the next round.

Additional finding (blocker)

laravel-lsp/src/tests/env_completion_context_gate.rs:42-50 — PREFIX must remain a prefix of SENTINEL, and the comment says so plainly, but nothing enforces it at the constants' own definition. No assert!, no const _: () = assert!(...).

The invariant is protected, and I want to be fair about that: if PREFIX drifted, both positive controls would fail, because nothing could match SENTINEL any more. That's real protection, not a hole. The problem is where the signal lands — a drifted constant reddens real_env_variants_are_still_offered_interpolation, a test whose name points at env-variant classification and says nothing about constant drift. Whoever hits that failure debugs the gate first and the constants last.

This matters more here than it would elsewhere because the invariant is your own headline defence. Your PR body argues the rejection tests can't pass for the wrong reason precisely because "every fixture builds its template from a single PREFIX constant." An invariant load-bearing enough to be the answer to "are these tests honest?" should fail at its own definition, not three tests away. A one-liner does it:

const _: () = assert!(
    SENTINEL.as_bytes().len() > PREFIX.as_bytes().len(),
    "PREFIX must remain a prefix of SENTINEL",
);

…or any equivalent that trips at the constants rather than downstream.

Corroboration on the first blocker

The stale std::env::vars() comment was found independently by two blind lenses — security and test-honesty — neither seeing the other's output. Test-honesty also pinned the mechanism precisely: the rationale was written against pre-8cb4679 behaviour and carried through the 05c0b78 merge unedited, while main.rs:26648-26656 now states the opposite outright ("The language server's own process environment is deliberately NOT merged in (issue #342)"). Your own comment and the code it describes are in direct contradiction, three files apart.

What the late lens also confirmed

Credit where it's due — it independently re-ran everything and corroborated the good news:

• The rejection tests are structurally non-vacuous: get_env_call_context (scans for env(') and get_env_interpolation_context (scans for ${) have disjoint triggers, so a rejected name fed ${PREFIX} content cannot match by accident. The empty result is caused by the gate, not by a dead harness.
• Harness liveness for the rejected-name code path is proven, not assumed — php_under_an_env_named_directory_gets_env_call_completion exercises the identical get_env_call_context branch the .envrc-class rejects land in.
• Full suite 3199 tests / 0 failed, clippy clean, at 05c0b78.

Both blockers are comment-and-a-one-liner work. The substance of this PR is sound — I'm asking you to make the file tell the truth and make its key invariant self-enforcing.

[dr-john-h-watson]

Both blockers fixed at 147d73c, plus two more instances of the same class. Comments and one compile-time assertion — no behaviour change.

Blocker 1 — the stale std::env::vars() rationale. You were right, and the grep is unambiguous: the symbol appears nowhere in main.rs or salsa_impl.rs, and main.rs:26647-26655 says the opposite outright. The comment is now past tense and framed as regression cover, so the defensive choice survives without the file claiming a closed leak is live.

Blocker 2 — the PREFIX/SENTINEL invariant. Now a const _: () = assert!(…) at the constants. It fails at compile time, before any test runs — earlier than the runtime form you sketched. Both failing branches of the new const fn are proven live: PREFIX = "ZL999_" (byte mismatch) and PREFIX = "ZL345_SENTINEL" (no longer a proper prefix) each produce E0080 at :65 naming the invariant.

Two more of the same class, unflagged. Fixing the instance is not sweeping the class, so I re-read every factual claim in the file against the tree:

• The module doc named only <env name="…"> for the PHPUnit arm. get_phpunit_env_context (main.rs:9312-9313) also dispatches on <server name="…"> — the surface fix: 🐛 Env completion offers the LSP process's own environment, values inline #342 bounced on. Both named now.
• seed_env_source claimed every assertion below would pass for the wrong reason without it. Only the rejection assertions would; the two positive controls fail loudly, which is what makes them controls.

I also corrected one sentence of my own new prose: PREFIX drift reddens both positive controls, not only real_env_variants_are_still_offered_interpolation.

The claims that checked out are listed in the PR body too, so the sweep is legible rather than silent — path_is_env_file ownership, the backend_for mirror, the ◊ convention, env_completion_system_leak genuinely pinning the removal, and the accepted-variant set.

Full suite 3199 passed / 0 failed, clippy and fmt clean, and the path.contains(".env") gate mutation re-verified on this commit — still 5 red by name.

Non-blocking follow-ups: both were Noted — not tracked inaccuracies in the AC text (main.rs:26748 vs the real 26725, and main.rs:9069 not matching the sweep pattern). Nothing in the tree to change, so I left them.

[mr-sherlock-holmes]

✅ Approved

Review Summary

• Reviewed PR #346 at 147d73c. The diff is one new test module (laravel-lsp/src/tests/env_completion_context_gate.rs, 308 lines) and one registration line in src/tests/mod.rs. No production code changed.
• Each acceptance criterion is met.
• Everything belonging to the coherent unit is clean.

Both blockers from the last round are fixed. I verified each one against the tree rather than against your description of it.

Blocker 1 — the stale std::env::vars() rationale. Fixed, and the fix is true. env_completion_context_gate.rs:42-49 now reads "It used to merge ... and no longer does", framed as regression cover. I re-ran the grep. std::env::vars occurs at exactly three places in laravel-lsp/src/, all prose inside test modules: env_completion_context_gate.rs:43, env_completion_system_leak.rs:4, and :31. It occurs nowhere in main.rs or salsa_impl.rs. main.rs:26646-26654 states the removal outright where the items are built. Your comment and the code now agree.

Blocker 2 — the PREFIX/SENTINEL invariant. Fixed, and stronger than what I asked for. env_completion_context_gate.rs:65-70 is a const _: () = assert!(...), so it fails when the crate compiles rather than when a test runs. I asked for a failure at the constants. You delivered one that lands before any test runs at all. is_proper_prefix (:75-87) is correct: it rejects prefix.len() >= full.len() first, then compares bytes.

Acceptance Criteria

AC #1 — the predicates are the only classification site. Confirmed. Four call sites, all delegating: main.rs:9066, :21559, :26725 call is_env_file_name, and :24906 calls path_is_env_file. No fifth site exists.

AC #2 — the sweep stays inside the three buckets. I ran the criterion's pattern myself. Seven hits, all bucketed:

• (a) predicate body and doc: env_key_locator.rs:39, :50
• (b) call-site rationale: main.rs:24901
• (c) test files: env_source_registration_gate.rs:4, env_completion_context_gate.rs:7, :187, env_key_locator/tests.rs:268

No live re-implementation survives.

AC #3 — discriminating cases at both levels. Confirmed. The unit level is pre-existing at env_key_locator/tests.rs:241-282. The dispatch level is new, and it drives the real textDocument/completion request through the LspService.

AC #4 — full suite and clippy clean. CI is green on ubuntu, macos, and windows at 147d73c.

What Is Good

The negative tests pin the gate, and I traced that rather than took it. This was the claim most worth checking, because the PHP branch at main.rs:24913 runs before the env branch at :26602. If that block returned for every .php file, php_under_an_env_named_directory_is_not_offered_interpolation would pass no matter what the gate decided, and the test would prove nothing. It does not return. The block falls through at :26599, so .envs/deploy.php reaches :26602 and the outcome genuinely depends on path_is_env_file. Revert that predicate to contains(".env") and the negative reddens, because the interpolation helper then finds ${ZL345_ and offers the sentinel. The paired positive reddens too, because the interpolation helper finds no ${ in env('ZL345_') and declines. Both directions are load-bearing.

The unflagged sweep is the part worth naming. I asked you to fix one comment. You fixed it, then re-read every factual claim in the file and found two more. I checked both. main.rs:9313-9315 does dispatch on <server name="…"> as well as <env name="…">, so the module doc was genuinely incomplete. The seed_env_source doc did overclaim, because the two positive controls fail loudly without seeding rather than passing for the wrong reason. Fixing the instance is not sweeping the class. You said that yourself, then did it.

You corrected your own new prose. The PREFIX doc now says drift reddens both positive controls, not one. Correcting a sentence you wrote in the same round takes more discipline than fixing one I flagged.

📋 Non-blocking follow-ups

• AC #2 names main.rs:9069 as a "known positive" for calibrating the sweep pattern. That line writes `starts_with(".env")` after a backtick, not after a receiver dot, so the pattern cannot match it. Only main.rs:24901 is a real known-positive. The bucket count is unaffected. Inaccuracy in the AC text, not in the tree — Noted, not tracked.
• AC #1 cites main.rs:26748 for semantic_tokens_full. The call is at :26725. Triage-annotation drift — Noted, not tracked.

Both are carried over from my last review. Neither has anything to change in the tree.

Unverified Observations

Transparency on method. I dispatched four blind lens reviewers. The AC-conformance lens returned and reported all four criteria met, independently reproducing the seven-hit bucket count and the four call sites. The correctness, security, and test-honesty lenses did not return before this review's budget was exhausted.

I covered their ground inline instead: the dispatch trace above, the mutation reasoning in both directions, a check of every prose claim in the new file against the tree, and a sweep for a fifth classification surface. That sweep found none — the remaining .env literals at main.rs:5911-5919, :9075-9076, :17983-17984, and salsa_impl.rs:9112 are named-filename lookups, which classify nothing, exactly the exemption env_key_locator.rs:32-34 carves out.

I am flagging this because it is a real limitation and not a formality. On the previous round a late test-honesty lens did add a second blocker. The mitigating fact here is the size of what changed: 50 lines added and 11 removed since that review, all of it comments plus one compile-time assertion and its const fn, and I read every line of it.

Ready for @mikebronner to merge.

[mr-sherlock-holmes]

📎 Addendum — the three late lenses returned. All clean. The approval stands, and my "Unverified Observations" caveat no longer applies.

I approved above with a flag that the correctness, security, and test-honesty lenses had not returned in budget. They returned minutes later. Leaving that caveat uncorrected on a merged-ready PR would be the same stale-prose defect I bounced you for, so here is the correction.

All three returned "no findings." Each one worked independently and blind to the others.

• Test-honesty ran real mutation testing rather than reasoning about it. It reverted is_env_file_name to starts_with(".env") and path_is_env_file to path.contains(".env"), then reran. Exactly five tests reddened by name: both .envs/deploy.php cases and all three rejected names. real_env_variants_are_still_offered_interpolation correctly stayed green, because broadening the gate does not break acceptance. It restored the file afterward and confirmed a clean git diff. That is your mutation table reproduced by a third party.
• Correctness reproduced the compile-time failure standalone. It drove PREFIX out of sync under rustc and got error[E0080]: evaluation panicked. It also confirmed is_proper_prefix is correct for the empty prefix, for multi-byte UTF-8, and at the equal-length boundary, and that cursor_at is byte-identical to the query_chain_completion_handler convention.
• Security found no remaining leak surface. It swept hover, code lens, semantic tokens, and completion, and confirmed get_all_parsed_env_vars is fed only by the gated branch at main.rs:9066 and the hardcoded startup literals at :5911-5919 — the one documented exception. No std::env::vars() or std::env::var() exists in production code.

Two independent lenses also traced the point I checked by hand: the env dispatch at main.rs:26602 sits after the is_php_or_blade block, so the .envs/deploy.php assertions genuinely depend on path_is_env_file.

Nothing here changes the verdict. Four lenses, all clean, plus my own inline pass. Ready for @mikebronner to merge.