Let the agent hook scenario inherit its environment; write down what mise does - #3016
Merged
Conversation
jeremy
force-pushed
the
prek-hook-inherit-env
branch
from
August 1, 2026 17:33
bf908f1 to
4166936
Compare
…mise does Two follow-ups to the toolchain-resolution fix. Neither changes the hook's behaviour: its 43 executable lines are byte-identical to master. The agent scenario rebuilt its environment under `env -i` and forwarded a hand-maintained list of the MISE_* variables that can relocate mise's install tree -- expected_ruby_path is derived in the invoking environment, so any variable missing from that list made the two disagree and failed a working hook. The list fell behind twice (MISE_INSTALLS_DIR, then MISE_SHARED_INSTALL_DIRS), and it could not have been completed even in principle: shared_install_dirs is a real mise setting, so a config FILE relocates the install tree too, and the list named no config-dir variable at all. Measured -- with MISE_CONFIG_DIR pointing at a config declaring shared_install_dirs, the old harness fails a working hook and this one passes. So delete the list. run_hook_inheriting() inherits the invoking environment and overrides only PATH, __MISE_ORIG_PATH and PROBE_ENV_KEYS, which is what an agent session actually does; the two now agree by construction under any store configuration. The [env] keys are unset so those assertions cannot pass on ambient state from a mise-activated shell. Both files described `mise exec` as substituting install dirs at the shims entry's position. Measured on 2026.7.10 (the floor) and 2026.7.16, on macOS and Arch, mise strips the install directories it recognises out of the inherited PATH, then inserts the resolved ones immediately BEFORE the shims entry -- keeping that entry -- or prepends them when there is none. Which is why a globally activated installs/ruby/4/bin at PATH #13 still loses to /usr/bin/ruby at #22. Recorded in both files with the measured rewrites. Adds a decoy scenario: an unrelated ruby ahead of /usr/bin. Not a different mise code path -- /usr/bin/ruby is already foreign and never stripped -- but a deterministic competitor that does not depend on what ruby the platform ships, failing unmistakably as DECOY_RUBY. Adds an in-suite negative control. The agent and decoy scenarios run under two activation states (mise's session internals retained, and fully scrubbed), and a mutated hook with the two shims-prepend lines removed must resolve the decoy, asserted by exact path and version rather than merely differing from expected -- this file has twice shipped assertions that could not fail. Failure to mutate is fatal rather than a skip.
jeremy
force-pushed
the
prek-hook-inherit-env
branch
from
August 1, 2026 17:38
4166936 to
3ebbb38
Compare
Copilot stopped reviewing on behalf of
jeremy due to an error
August 1, 2026 17:54
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two follow-ups to the toolchain-resolution fix that landed 2026-07-31. Neither changes the hook's behaviour — its 43 executable lines are byte-identical to master; the only hook edit is a comment.
1. The agent test scenario inherits its environment, and the forwarding list is deleted
The agent scenario rebuilt its environment under
env -iand forwarded a hand-maintained list of theMISE_*variables that can relocate mise's install tree.expected_ruby_pathis derived in the invoking environment while the probe ran underenv -i, so any variable missing from that list made the two disagree and failed a working hook.The list fell behind twice —
MISE_INSTALLS_DIR, thenMISE_SHARED_INSTALL_DIRS— and mise publishes no authoritative set to copy:mise settings --allreports none of the path settings. The shipped file admitted as much in a comment ("bounded by measurement, not enumerated"). That caveat was the smell: the list defended a boundary that need not exist.So
run_hook_inheriting()inherits the invoking environment and overrides onlyPATH,__MISE_ORIG_PATHandPROBE_ENV_KEYS— which is what an agent session actually does. The two now agree by construction under any store configuration, and both past bugs become structurally impossible rather than individually fixed.The
[env]keys are unset, because a mise-activated project shell already exports them and running the suite that way is normal — inheriting them would let those assertions pass on ambient state. That scrub is the one placeenv -i's hazard is deliberately reintroduced, so it gets its own negative control below.Both remaining lists are derived from the same source as the thing they serve, so neither can fall behind it:
scrub(-u $env_keys).mise.toml[env][env]assertions, read from.mise.tomlexpected_ruby_path, derived in that same environment_mise_dirsstays, unchanged, for the terminal scenario; its rationale is rewritten, since it was written around the agent scenario's path assertion.2. The mechanism comment understated what mise does
Both files said
mise exec"substitutes the install dirs at the shims entry's position". That is the visible consequence, not the mechanism. Asked directly (mise exec -- sh -c 'echo $PATH') rather than inferred from which binary won:/usr/bin : shims : /bin/usr/bin : «installs» : shims : /bin/usr/bin/rubyinstalls/ruby/4/bin : /usr/bin : shims/usr/bin : «installs» : shims/usr/bin/ruby/usr/bin : /bin«installs» : /usr/bin : /bindecoy : /usr/bin : shimsdecoy : /usr/bin : «installs» : shimsmise strips the install directories it recognises out of the inherited PATH, then inserts the resolved ones immediately before the shims entry — keeping that entry — or prepends them when there is no shims entry at all. Row 2 is the striking one: the leading
installs/ruby/4/binsimply vanishes. Row 4 shows foreign entries are never touched.Measured on mise 2026.7.10 (the project's
min_versionfloor) and 2026.7.16, on both platforms, identically. An earlier draft of this PR said mise removed the shims entry too; review caught it and direct measurement settled it.This is why a globally activated
installs/ruby/4/binat PATH #13, nine places ahead of/usr/binat #22, still lost to/usr/bin/ruby— the most counterintuitive fact in the investigation, and exactly the confusion behind "mise exec -- ruby -vprints 3.4.10 but the hook still fails". It was written down nowhere. Now it is, in both files.3. A fourth scenario: an unrelated ruby earlier on PATH
A decoy
rubyahead of/usr/bin, in a PATH otherwise the agent shape. Not a different mise code path — I claimed it was, and review was right that it isn't:/usr/bin/rubyis already foreign and mise never strips it, so the agent scenario always had a foreign competitor. What the decoy adds is determinism. The agent scenario's competitor is whatever ruby the platform happens to ship — 2.6.10 on macOS, 3.4.10 on Arch, and eventually nothing at all, since Apple is removing it; on a host without one that scenario would quietly have nothing left to beat and would pass whatever the hook did. The decoy always has a competitor, and fails asDECOY_RUBYrather than as a version string that might coincide with the project's.Kept as its own scenario rather than folded into the agent PATH: with a decoy winning, a broken hook would fail by version everywhere, and the suite would lose the Arch-specific evidence that it fails there by path alone (
/usr/bin/rubyis also 3.4.10 on Arch). Both properties are worth holding.4. Two activation states, and a permanent negative control
Review flagged that dropping
__MISE_DIFFwhile keeping__MISE_SESSION/MISE_SHELL/__MISE_EXEleaves a combination no real shell produces, built from undocumented mise internals. Rather than pick one interpretation,run_hook_inheritingtakes an activation mode and the agent and decoy scenarios each run under both —keep(drop__MISE_DIFFonly, the faithful model) andbare(drop all four). The property asserted is that the hook resolves correctly either way, which stays checked if a future mise changes what these mean.And the negative control is now in the suite, not a measurement someone took once by hand: it strips the two shims-prepend lines from the hook and requires the decoy to win — asserted positively, by exact path and exact version, in both activation modes. Positively, because "not the expected path" is also satisfied by an empty field from a mutant that never executed, which is the same liveness hole the control exists to close. Failure to mutate is fatal rather than a skip.
It earned its place twice over: it caught a real bug in itself on the first run (it was writing over its own input file), and review caught the
!=liveness hole before merge. Sensitivity verified both directions — pointed at the unmutated hook it goes red, and with the probe output forced empty it goes red.This matters because the file has form:
env -itwice strippedPWD, so two shipped assertions could never fail. Inheriting the environment trades that hazard for a new one — inherited mise state repairing a broken hook — and this control is what keeps it honest.Assertion count 17 → 28 (20 → 34 for launchpad, which has two
[env]keys).5. The forwarding list could not have been completed even in principle
Found while chasing my own review of this change, and it is the strongest argument for deleting the list rather than extending it a third time.
shared_install_dirsis a genuine mise setting, so a config file relocates the install tree exactly as an environment variable does — and the deleted list named no config-dir variable at all. So it had a third latent hole waiting.Demonstrated, same working hook, same environment, a fresh empty local store for each run, with
MISE_CONFIG_DIRpointing at a config declaringshared_install_dirs:…/sharedinstalls/ruby/3.4.10/bin/ruby, got…/storeX/installs/ruby/3.4.10/bin/ruby→ fails a working hook…/sharedinstalls/ruby/3.4.10/bin/ruby→ passesBy contrast
data_dir,shims_dirandinstalls_dirreportUnknown settingand a config file setting them is ignored with a warning — they are environment-only. (That is theUnknown settingvsSetting [x] is not setdistinction; misreading it once produced a confidently wrong rebuttal.)Verification
All on macOS and Arch, at this exact head; hook and test
sha256-identical across all five repos.shellcheckclean on both files, all five repos.Suite green: 28/28 (34/34 launchpad), both platforms.
Negative control — the gate for change 1. Strip only the two prepend lines. The new configuration is one that had never been run: broken hook, inherited environment, modelled PATH,
__MISE_ORIG_PATHsupplied,__MISE_DIFFremoved. The hazard was that inherited mise shell state might reconstitute install directories and mask the break. It does not — run from a fully mise-activated shell on both platforms (macOS:__MISE_DIFF/__MISE_SESSION/MISE_SHELLset and 9 install dirs ahead of/usr/bin; Arch likewise):2.6.10,/usr/bin/ruby); decoy fails by both./usr/bin/ruby, also 3.4.10, so the version assertion passes); decoy fails by both.All four inheriting scenarios (
agent,agent-bare,decoy,decoy-bare) fail correctly. This control is now also permanently in the suite — see section 4.Control that the
[env]scrub earns its place. A hook variant execing the probe bare, so nothing exports[env], run from a mise-activated shell wherePROMETHEUS_EXPORTER_URLis already set: (a) with the scrub →<UNSET>, the assertion fails; (b) without it → the ambient value, the assertion passes on state the hook never exported. Both platforms. The hazard is real and the scrub blocks it.Whole suite with
MISE_DATA_DIRat an empty store: green, with agent and decoy resolving out of that store because they inherit it. Repeated withMISE_SHARED_INSTALL_DIRSat a populated store, an empty local store and an emptyMISE_CONFIG_DIR— the fixture that exposed the second bug: green with no variable forwarding at all. Method kept honest with a made-up-name control:MISE_TOTALLY_MADE_UP_DIRSleavesmise which rubyunresolved.End-to-end in a real agent shell (shims below
/usr/bin): the hook with its finalexecswapped for a probe resolvesmise which ruby, not/usr/bin/ruby. Plus the real thing — a staged Ruby file sorubocopactually runs: Passed under the merged hook; under the stripped one it reproduces the original report exactly,bin/rubocop's#!/usr/bin/env rubylanding on 2.6.10 and dying in Bundler.Inert where already correct: the hook's executable content is byte-identical to master, and the GUI and terminal scenarios are unchanged and green on both platforms.
Note on adoption
Git runs
.githooks/pre-commitfrom the working tree, so a branch created before 2026-07-31 executes the pre-fix file no matter what master says. The remedy is to pick up master (git rebase origin/master), not--no-verify. The same lag applies to this PR — but it only affects the test suite, which you run from whatever branch you are on.