fix(env): improve hook-env watch_files tracking and early-exits

[rpendleton]

Summary

Fixes #8603 — env plugins (MiseEnv modules) can return watch_files, but those files were never tracked in the hook-env session or env cache. Modifying a watched file (e.g. a secrets config) wouldn't trigger re-evaluation until the next config change or directory switch.

Additionally fixes two related hook-env stability issues discovered while writing e2e tests for the original issue:

Plugin watch_files tracking (4 gaps)

• config.watch_files() now chains env_results.watch_files, so tools=false plugin watch_files reach the slow-path check and session.
• env_with_path_and_split() now returns tools=true plugin watch_files so hook-env can merge them into the session for the next prompt's fast-path check.
• Slow-path check now includes PREV_SESSION.watch_files to detect changes to tools=true plugin files before load_post_env runs. Uses a separate variable so stale entries don't persist indefinitely.
• final_env() merges NonToolsOnly watch_files into ToolsOnly env_results, so CachedEnv tracks all plugin watch_files and invalidates correctly.

Missing mise.lock destabilizes hook-env

Projects without a mise.lock file could fail to stabilize because the nonexistent lockfile was unconditionally added to the watch set and sometimes treated as perpetually deleted, preventing should_exit_early from returning true.

• Only add mise.lock to the watch set when it actually exists; parent directory mtime checks already detect later creation.
• Include mise.lock paths in the env-cache key so lockfile creation, deletion, or modification properly invalidates cached env state.

Session timestamp doesn't account for directory mtimes

should_exit_early_fast() checks config-search directory mtimes and the data-dir mtime, but build_session() didn't include those in latest_update. After a successful slow-path run, subsequent prompts would repeatedly fall back to the slow path because directory mtimes remained newer than the session timestamp.

• Extract config_search_dir_mtimes() helper shared by both should_exit_early_fast() and build_session().
• Include data-dir and config-search directory mtimes in latest_update so a full run stabilizes subsequent prompts.

Test plan

• E2E test (test_env_plugin_watch_files) covers all four plugin watch_files code paths:
  • tools=false + cache=off
  • tools=false + cache=on
  • tools=true + cache=off
  • tools=true + cache=on
• Each scenario verifies: initial value set, fast-path works when idle, fast-path bypassed on change, updated value picked up, fast-path restored after update
• E2E test (test_env_lockfile_caching) verifies missing lockfile doesn't prevent stabilization, and that creation/removal invalidates exactly once before re-stabilizing
• E2E test (test_hook_env_dir_mtime_stabilizes) verifies hook-env stabilizes after a directory mtime change
• All three e2e tests pass
• Manually verified the original problem is no longer reproducible

🤖 Generated with Claude Code

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical issue where environment plugin watch_files were not being properly tracked, leading to missed re-evaluations when these files changed. The changes ensure that all watch_files, regardless of whether they originate from tools=false or tools=true plugins, are correctly integrated into the session and environment cache. This comprehensive fix guarantees that mise will reliably detect modifications to watched files and trigger necessary environment updates, improving the responsiveness and correctness of the system.

Highlights

• Config Watch Files: The config.watch_files() function now includes env_results.watch_files, ensuring that tools=false plugin watch files are considered in the slow-path check and session.
• Env With Path And Split: The env_with_path_and_split() function now returns tools=true plugin watch files, allowing hook-env to merge them into the session for subsequent fast-path checks.
• Slow-Path Check Enhancement: The slow-path check now incorporates PREV_SESSION.watch_files to detect changes in tools=true plugin files before load_post_env executes, using a distinct variable to prevent stale entries.
• Final Environment Merging: The final_env() function now merges NonToolsOnly watch files into ToolsOnly env_results, ensuring that CachedEnv accurately tracks all plugin watch files for correct invalidation.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request effectively addresses the issue of watch_files from env plugins not being tracked in the session or env cache. The changes correctly integrate plugin-defined watch_files into the config.watch_files() chain and the HookEnvSession, ensuring that modifications to these files trigger re-evaluation. The addition of a comprehensive E2E test covering all four cache/tools combinations is excellent and provides strong validation for the fix. The code is clear, well-structured, and directly targets the identified gaps, leading to a robust solution.

[greptile-apps]

Greptile Summary

This PR fixes a family of related hook-env stability bugs: env plugin watch_files were silently dropped at four different points in the pipeline, missing mise.lock files could prevent hook-env from ever stabilising, and build_session omitted the data-dir and config-search directory mtimes from latest_update, causing the fast-path to fall back on every subsequent prompt after a full run.

Key changes:

• config.watch_files() now chains env_results.watch_files so tools=false plugin watch_files flow into the slow-path check.
• env_with_path_and_split() returns plugin watch_files as a fourth element; hook-env merges these into the session for the next fast-path check.
• The slow-path check now unions PREV_SESSION.watch_files with the current watch set (via a separate slow_path_watch_files variable) to catch changes to tools=true plugin files before load_post_env runs.
• final_env() extends env_results.watch_files with NonToolsOnly watch_files so the env cache tracks all plugin watch_files.
• mise.lock paths are only added to the watch set when they actually exist; their mtimes are included in the cache key so creation, deletion, and modification each correctly invalidate.
• A new config_search_dir_mtimes() helper is shared between should_exit_early_fast and build_session, and its output is folded into latest_update so a single full run fully stabilises all subsequent prompts.

Confidence Score: 5/5

• Safe to merge — all four watch_files gaps are closed, cache invalidation is correct in all lockfile scenarios, and the stabilisation fix is sound.
• All three previously-raised review concerns (misleading comment in final_env, asymmetric tool_watch_files semantics, vacuous stability assertion in test) have been resolved in this revision. The cache invalidation logic for lockfiles is correct: creation is caught by the changed cache key, deletion by PREV_SESSION.watch_files mtime check, and modification by both. The slow_path_watch_files union correctly avoids stale-entry accumulation by not writing PREV_SESSION entries into the new session. E2E coverage is comprehensive across all four code paths.
• No files require special attention.

Important Files Changed

Filename	Overview
src/cli/hook_env.rs	Adds slow_path_watch_files (merging PREV_SESSION.watch_files with current watch_files) for the slow-path exit check, returns 4-tuple from env_with_path_and_split, and merges env_watch_files into the session's watch set. Logic is correct and well-commented.
src/hook_env.rs	Extracts config_search_dir_mtimes() helper shared by should_exit_early_fast and build_session; adds data-dir and config-search dir mtimes to latest_update so a full run stabilises subsequent prompts; makes watch_files field public for PREV_SESSION access.
src/config/mod.rs	Adds env_results.watch_files to the watch_files() chain so tools=false plugin watch_files reach the slow-path check; conditionally adds mise.lock only when it exists to prevent instability from absent lock files.
src/toolset/toolset_env.rs	Extends env_with_path_and_split return type to include watch_files; merges NonToolsOnly watch_files into env_results in final_env; conditionally includes lockfiles in both the saved watch set and the cache key, ensuring correct invalidation on lockfile creation/deletion/modification.
e2e/env/test_env_plugin_watch_files	Comprehensive e2e test covering all four combinations of tools=true/false × cache=on/off; correctly uses sleep 1 to guarantee mtime advances; clears session and cache state between runs.
e2e/env/test_env_lockfile_caching	Tests stability without a lockfile, then validates that creation and removal each invalidate exactly once before re-stabilising; correctly uses sleep 1 and eval to maintain session state between steps.
e2e/cli/test_hook_env_dir_mtime_stabilizes	Verifies that a config-search path change triggers exactly one full hook-env run and then stabilises. The previous vacuous-assertion bug (redirecting stderr to /dev/null) was addressed before this revision.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[hook-env run] --> B{env_cache enabled?}
    B -- yes --> C{try_load_env_cache_full}
    C -- hit --> D[watch_files = cached.watch_files]
    C -- miss --> E[config.watch_files incl. tools=false plugin WF]
    B -- no --> E

    D & E --> F[slow_path_watch_files = watch_files ∪ PREV_SESSION.watch_files]
    F --> G{should_exit_early?}
    G -- yes --> H[return early]
    G -- no --> I[env_with_path_and_split]

    I --> J{cache hit?}
    J -- hit --> K[env_watch_files = cached.watch_files]
    J -- miss --> L[final_env: load_post_env tools=true\n+ merge NonToolsOnly WF]
    L --> M[env_watch_files = env_results.watch_files]

    K & M --> N[session watch_files = watch_files ∪ env_watch_files]
    N --> O[build_session: latest_update = max of\nall watch file mtimes\n+ data dir mtime\n+ config search dir mtimes]
    O --> P[__MISE_SESSION written]
    P --> Q[Next prompt: fast-path stable]

Loading

_{Reviews (5): Last reviewed commit: "fix(env): align hook-env session timesta..." | Re-trigger Greptile}

[jdx]

why is this marked as slow?

[rpendleton]

That's partially related to #8716 (comment), though even when considering the sleeps, I'm not sure why it's taking long enough to pass the 20 second warning. I'll take a look.

[jdx]

ideally we don't need to mark it as slow so the test will actually run on this PR

[rpendleton]

I think I can improve this a bit.

With debug builds, mise activate takes ~2.5s and mise hook-env takes ~1.3s. We also have the one second sleeps for file modification detection. Since we're testing four scenarios, (2.5 + 1.3 + 1.0) * 4 = 19.2s, and that's before we consider anything else (like macOS Gatekeeper randomly verifying the executable, since a build is performed right before the test).

If I configure both tools=true and tools=false plugins in the same config and test them simultaneously in each scenario, I can avoid running activate as many times and likely get it under the 20s. I'm also not sure if I even need to run activate more than once.

I'll work on that now.

[jdx]

ah hmm, I didn't realize things were so slow. I think maybe what we could do is loosen the rules on what is considered slow to more like 1 minute. It's really to avoid things that take multiple minutes like compiling ruby.

I think it would also be fine to just have a larger test that just activates once. We do that often.

[rpendleton]

I'm having some troubles with my test being a bit flaky, but I think it may be interacting with a pre-existing mise.lock issue that’s reproducible on main:

1. Temporarily initialize the logger before should_exit_early_fast() so fast-path trace logs are visible.
2. export MISE_TRACE=1
3. cd into a directory with a mise.toml but no mise.lock
4. Press Enter a few times to repeatedly trigger hook-env, and observe that should_exit_early() keeps running and returning false because mise.lock is treated as a watched file that was "deleted", even though it never existed.
5. touch mise.toml
6. Press Enter a few more times, and observe that hook-env now exits early as expected and mise.lock is no longer present in the resolved watch files.

I don’t yet understand why touching mise.toml changes whether mise.lock appears in the watch set, but the current behavior seems wrong regardless: a missing optional mise.lock should not keep hook-env from stabilizing.

My test seems to hit this sometimes as well, which may explain the flakiness. I haven’t pinned down exactly what makes it appear or disappear yet, but once mise.lock is no longer treated as a required existing watched file, I suspect the test will become stable too.

As a potential fix, I tried ignoring mise.lock if it didn't exist. That improved things so that when you cd into a directory, the fast check allows for an early exit on subsequent prompts. However, if I then touch mise.lock, it starts falling back to the slow check again.

I'm seeing if I can identify what's causing this to happen. It's maybe a bit out-of-scope at this point, but I don't want to introduce a flaky test.

[rpendleton]

Alright, I think I have that mise.lock issue resolved. I tried to fix it in a separate PR but then that e2e test ran into the same problem being fixed in this PR, so I figured I'd just group the fixes together in this PR. Let me know if there are any concerns about the size of the PR though.