Skip to content

fix(hooks): deprecate spawned scripts form - #11043

Merged
jdx merged 1 commit into
jdx:mainfrom
risu729:agent/fix-hooks-deprecate-scripts
Jul 16, 2026
Merged

jdx merged 1 commit into
jdx:mainfrom
risu729:agent/fix-hooks-deprecate-scripts

Conversation

@risu729

@risu729 risu729 commented Jul 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • mark plural scripts hook tables as legacy when they execute as spawned commands
  • preserve supported current-shell scripts behavior for enter, leave, and cd
  • update the scheduled deprecation message and hook documentation to cover both aliases
  • add focused regression coverage for spawned and current-shell conversion

Why

Singular script and plural scripts tables have the same spawned-command compatibility behavior, but ScriptsTable passed legacy_script = false while ScriptTable passed true. As a result, plural scripts bypassed the deprecation scheduled to warn in mise 2026.9.0 and be removed in 2027.3.0.

This was discovered while reviewing #11041 but is independent and does not need to be stacked on it.

Validation

  • cargo test hooks::tests::scripts_tables_are_legacy_only_when_spawned --all-features
  • mise run test:e2e config/test_hooks_run_warnings
  • mise run lint-fix

Summary by CodeRabbit

  • Documentation

    • Clarified that preinstall and postinstall support both script and scripts as legacy aliases for run.
  • Bug Fixes

    • Updated deprecation warnings to accurately reference both legacy aliases.
    • Improved handling of spawned hook scripts while preserving current-shell behavior.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

ScriptsTable hooks now classify spawned commands as legacy, update their deprecation warning, test spawned and current-shell behavior, and document both script and scripts as aliases for run.

Changes

Legacy hook aliases

Layer / File(s) Summary
Runtime legacy handling
src/hooks.rs
Spawned script-table hooks now set legacy_script: true, use the updated deprecation identifier and message, and test spawned versus current-shell conversion.
Hook alias documentation
docs/hooks.md
Preinstall and postinstall documentation identifies both script and scripts as legacy aliases for run.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: jdx

Poem

A rabbit hopped through hooks with cheer,
“Both script names are legacy here!”
Spawned runs warn in proper tune,
Shell hooks keep their current room,
Docs now tell the tale quite clear.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: deprecating spawned scripts hook tables.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a gap in the deprecation path for plural scripts hook tables in mise: ScriptsTable was passing legacy_script = false to script_hook_action, silently bypassing the deprecation warning that ScriptTable (singular) already emits for spawned-command usage.

  • src/hooks.rs: Changes legacy_script from false to true in the ScriptsTable branch, making it consistent with ScriptTable; the current-shell path (Enter/Leave/Cd with a shell key) is unaffected because it routes to HookAction::CurrentShell before legacy_script is ever read; adds a regression test covering both the spawned and current-shell outcomes.
  • docs/hooks.md: Extends two prose passages to name scripts alongside script as a deprecated alias for install hooks.

Confidence Score: 5/5

Safe to merge — the change is a one-line fix to a flag value, the surrounding logic already handles both outcomes correctly, and a new test pins both branches.

The fix is minimal and surgical: a single boolean flipped in ScriptsTable makes it consistent with ScriptTable. The script_hook_action function already routes CurrentShell hooks before legacy_script is consumed, so the current-shell path for Enter/Leave/Cd is unaffected. The new test directly asserts both the spawned (legacy_script: true) and current-shell (HookAction::CurrentShell) outcomes for ScriptsTable, catching any regression. Documentation is updated in the two relevant places.

No files require special attention.

Important Files Changed

Filename Overview
src/hooks.rs Fixes ScriptsTable to pass legacy_script=true so spawned-command uses trigger the same deprecation path as ScriptTable; adds a focused regression test covering both the spawned and current-shell branches.
docs/hooks.md Documentation updated to mention both script and scripts as deprecated aliases for install hooks; consistent with the code change.

Reviews (1): Last reviewed commit: "fix(hooks): deprecate spawned scripts fo..." | Re-trigger Greptile

@risu729
risu729 marked this pull request as ready for review July 16, 2026 12:56
@jdx
jdx merged commit 6a69c2b into jdx:main Jul 16, 2026
34 checks passed
@risu729
risu729 deleted the agent/fix-hooks-deprecate-scripts branch July 16, 2026 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants