Skip to content

fix(hooks): reject nested hook arrays - #11051

Merged
jdx merged 2 commits into
jdx:mainfrom
risu729:agent/fix-hooks-reject-nested-arrays
Jul 16, 2026
Merged

jdx merged 2 commits into
jdx:mainfrom
risu729:agent/fix-hooks-reject-nested-arrays

Conversation

@risu729

@risu729 risu729 commented Jul 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • separate top-level hook definitions from individual hook items
  • reject recursively nested hook arrays during configuration deserialization
  • preserve strings, tables, and flat mixed hook arrays
  • add regression coverage for accepted flat arrays and rejected nested arrays

Why

The JSON schema permits either one hook item or one array of hook items, but the runtime represented arrays as Vec<HookDef>. Since HookDef itself included the array variant, runtime deserialization accepted arbitrarily nested arrays and recursively flattened them. This made configurations executable by mise even though schema-aware editors and validators rejected them.

The runtime now models an outer definition separately from a non-array item, matching the schema's single-array-level contract.

Example

Nested arrays are now rejected:

[hooks]
enter = [["echo first"], [{ task = "setup" }]]

Use one flat array instead:

[hooks]
enter = ["echo first", { task = "setup" }]

Validation

  • mise x cargo -- cargo test hooks::tests --all-features
  • mise run lint-fix
  • mise run test:e2e e2e/config/test_hooks e2e/config/test_hooks_task_ref e2e/config/test_hooks_script_arrays

Summary by CodeRabbit

  • Bug Fixes
    • Improved validation for hook configuration entries.
    • Invalid or unexpected fields in hook definitions are now rejected.
    • Nested hook arrays in top-level hook configurations are no longer accepted.
    • Hook actions continue to support commands, scripts, and task references with more consistent configuration handling.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026 •

Copy link
Copy Markdown
Contributor

Caution

Review failed

An error occurred during the review process. Please try again later.


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.

@risu729
risu729 force-pushed the agent/fix-hooks-reject-nested-arrays branch from 503632e to 582c87c Compare July 16, 2026 14:30
@greptile-apps

greptile-apps Bot commented Jul 16, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes the hook configuration deserializer to reject arbitrarily nested arrays that are invalid per the JSON schema but were silently accepted by the runtime. It does so by introducing a new HookDefItem type for individual hook entries, restricting the HookDef array variant to Vec<HookDefItem> so nesting is impossible structurally.

  • HookDef is now a thin wrapper (One(HookDefItem) | Array(Vec<HookDefItem>)) while HookDefItem holds all the concrete hook shapes with deny_unknown_fields.
  • HookDef::into_hooks and the new HookDefItem::into_hook mirror the old match arms cleanly, producing identical runtime behavior for valid configurations.
  • A new unit test covers both the accepted flat-mixed case and two rejected nested-array cases.

Confidence Score: 5/5

The change is a structural refactoring of a deserialization enum with no behavioral change for valid configs and a correct new rejection for nested arrays.

The refactoring is minimal and self-contained within src/hooks.rs. The public into_hooks contract is unchanged, all existing tests are updated to the new constructor shape, and the new test exercises both the accepted flat case and two rejected nested cases. No external callers pattern-match on HookDef variants directly.

No files require special attention.

Important Files Changed

Filename Overview
src/hooks.rs Splits HookDef into a top-level HookDef (One/Array) and a non-recursive HookDefItem to prevent nested arrays during deserialization; moves deny_unknown_fields to HookDefItem; adds regression tests.

Reviews (2): Last reviewed commit: "test(hooks): clarify flat array assertio..." | Re-trigger Greptile

Comment thread src/hooks.rs Resolved
@risu729
risu729 force-pushed the agent/fix-hooks-reject-nested-arrays branch from a284b11 to 465b827 Compare July 16, 2026 14:45
@risu729
risu729 marked this pull request as ready for review July 16, 2026 14:51
@jdx
jdx merged commit 7f0a28e into jdx:main Jul 16, 2026
33 checks passed
@risu729
risu729 deleted the agent/fix-hooks-reject-nested-arrays branch July 16, 2026 20:13
This was referenced Jul 16, 2026
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