fix(hook-env): suppress repeated untrusted config warnings#10589
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds per-directory gating for the untrusted config warning in ChangesUntrusted config warning once per directory
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR suppresses the per-prompt untrusted-config warning in
Confidence Score: 5/5Safe to merge; the change is limited to the hook-env warning path and does not affect any trust enforcement or env-loading correctness. The suppression logic is keyed on an exact (cwd + config path + mtime) hash, so it cannot fire for a different directory or a stale file. Both fast- and slow-path early-exit checks are guarded so the marker is always re-evaluated on every prompt. Explicit commands (mise run, etc.) never consult the marker. The new e2e covers first-warn, suppression, mtime-reset, parent-clear, and re-entry. No correctness regressions are visible in the changed paths. No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "fix(hook-env): key untrusted warning by ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| fn hook_env_shell_arg() -> Option<ShellType> { | ||
| let args = env::ARGS.read().unwrap(); | ||
| let mut iter = args.iter(); | ||
| while let Some(arg) = iter.next() { | ||
| if arg == "-s" || arg == "--shell" { | ||
| return iter.next().and_then(|s| s.parse().ok()); | ||
| } | ||
| if let Some(shell) = arg.strip_prefix("--shell=") { | ||
| return shell.parse().ok(); | ||
| } | ||
| if let Some(shell) = arg.strip_prefix("-s") | ||
| && !shell.is_empty() | ||
| { | ||
| return shell.parse().ok(); | ||
| } | ||
| } | ||
| None | ||
| } |
There was a problem hiding this comment.
hook_env_shell_arg re-implements the -s/--shell argument parsing by scanning ARGS directly, duplicating logic already handled by the CLI framework. If a new short-flag form is added to the hook-env subcommand (or if parsing order changes), this function will silently diverge. Consider exposing the parsed ShellType through the existing CLI structures rather than re-parsing raw args here.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
jdx
force-pushed
the
codex/hook-env-untrusted-once-per-dir
branch
2 times, most recently
from
fe52195 to
972a350
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 972a350. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/cli/test_hook_env_untrusted_once_per_dir`:
- Around line 7-9: The contains_trust_warning function is checking for "Config
files in" but the actual untrusted task error message from src/task/task_list.rs
uses "Config file(s) in ... are not trusted" format. Update the grep pattern in
the contains_trust_warning function to match the actual error text being
generated, ensuring it can match both singular and plural forms of the file
reference or use a pattern that correctly matches the error message as it
appears in the code.
In `@e2e/cli/test_zsh_precmd_runs_once`:
- Around line 86-88: The current check using contains_trust_warning as a proxy
to detect if chpwd hooks already ran is unreliable because trust warnings can be
suppressed, leading to the run_hooks_to_file function potentially executing
hooks twice. Replace the contains_trust_warning check on chpwd_file with a
direct signal check that looks for a hook signal variable like
__MISE_ZSH_CHPWD_RAN instead of relying on the presence or absence of warning
text, ensuring the fallback only runs when hooks genuinely haven't been invoked
by zsh.
In `@src/hook_env.rs`:
- Around line 74-80: The deduplication mechanism for untrusted config warnings
is using only the directory as the key via current_untrusted_warning_dir(),
which is too coarse and allows configuration changes within the same directory
to remain incorrectly suppressed. Replace the directory-only marker with a
composite config-state key that includes cwd, trust root, config path, and
mtime. Update the LAST_UNTRUSTED_CONFIG_WARNING_DIR_ENV environment variable to
store this composite key instead, and ensure all related functions
(should_show_untrusted_config_warning, the mark functions at lines 92-94, and
the clear functions at lines 99-102 and 109-117) consistently use the same
composite key for storing and comparing configuration state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 1e4b9360-2c4e-4147-a474-01b67bda6b21
📒 Files selected for processing (5)
e2e/cli/test_hook_env_untrusted_once_per_dire2e/cli/test_zsh_precmd_runs_oncesrc/cli/hook_env.rssrc/hook_env.rssrc/main.rs
jdx
force-pushed
the
codex/hook-env-untrusted-once-per-dir
branch
from
972a350 to
c4b7c16
Compare
jdx
force-pushed
the
codex/hook-env-untrusted-once-per-dir
branch
from
c4b7c16 to
c8de934
Compare
Hyperfine Performance
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.6.13 x -- echo |
23.7 ± 1.8 | 19.3 | 30.3 | 1.11 ± 0.14 |
mise x -- echo |
21.5 ± 2.2 | 18.1 | 42.7 | 1.00 |
✅ Performance improvement for x -- echo is 11% |
mise env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.6.13 env |
19.3 ± 1.2 | 17.4 | 24.3 | 1.00 |
mise env |
20.2 ± 1.5 | 17.8 | 26.4 | 1.05 ± 0.10 |
mise hook-env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.6.13 hook-env |
19.6 ± 1.2 | 17.7 | 26.4 | 1.00 |
mise hook-env |
21.3 ± 1.4 | 18.4 | 28.7 | 1.09 ± 0.10 |
mise ls
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.6.13 ls |
17.6 ± 1.2 | 14.9 | 22.4 | 1.00 |
mise ls |
18.2 ± 1.9 | 15.1 | 26.3 | 1.03 ± 0.13 |
xtasks/test/perf
| Command | mise-2026.6.13 | mise | Variance |
|---|---|---|---|
| install (cached) | 136ms | 138ms | -1% |
| ls (cached) | 59ms | 63ms | -6% |
| bin-paths (cached) | 65ms | 65ms | +0% |
| task-ls (cached) | 133ms | 133ms | +0% |
1 participant
Summary
Tests
Note
Medium Risk
Touches shell activation trust handling and early-exit logic; mistakes could hide security warnings or break zsh hook timing, but scope is limited to hook-env and covered by new e2e tests.
Overview
hook-envno longer spams the same untrusted-config message on every shell prompt in one directory. The first failure still prints the full trust error and exports__MISE_LAST_UNTRUSTED_CONFIG_WARNING_KEY(hashed from cwd, trust root, config path, and file mtime). Laterhook-envcalls with the same key exit quietly with code 1 instead of repeating stderr.A successful
hook-envrun removes that marker so leaving and re-entering the project warns again; editing.mise.tomlchanges mtime and re-enables the warning in place. Early-exit fast paths are skipped while the marker is set so suppression stays consistent. Explicit commands likemise runare unchanged and still surface the full trust error.E2e coverage adds
test_hook_env_untrusted_once_per_dirand tightens the zshchpwd/precmdtest (trust-warning grep, realcdwhen hooks fire).Reviewed by Cursor Bugbot for commit 5fb16f1. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
cd, avoiding duplicate side effects.Tests