Repository navigation
fix(hooks): type untyped flat command handlers for Codex - #3385
Open
parth (Parth-Vasave) wants to merge 1 commit into
Open
parth (Parth-Vasave) wants to merge 1 commit into
parth (Parth-Vasave) wants to merge 1 commit into
Conversation
Codex's HookHandlerConfig is an internally tagged enum (#[serde(tag = "type")]) with no default, so the nested handler APM generated from a flat command entry without `type` matched no variant. Share the flat-command type default introduced for Claude in microsoft#3153 with the Codex renderer: untyped flat command entries now render as `"type": "command"` handlers in .codex/hooks.json, including when they join existing groups. Explicit types, non-command entries and handlers already nested in the source are unchanged, as are other targets. Ownership matching on reinstall and update now recognises both earlier Codex shapes APM wrote (the original flat entry and the nested untyped group), so proven APM-owned output converges to one typed entry. Manual and unowned hooks are not rewritten or removed. Fixes microsoft#3249
parth (Parth-Vasave)
requested review from
Daniel Meppiel (danielmeppiel) and
Sergio Sisternes (sergio-sisternes-epam)
as code owners
October 25, 2026 16:26
This branch has not been deployed
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.
Description
A flat hook entry may omit
type(Cursor defaults it tocommand). APM already wraps such entries in Codex's{ "matcher", "hooks": [...] }groups, but the generated handler had notype. In the pinned Codex source (hook_config.rsat36512a34),HookHandlerConfigis#[serde(tag = "type")]with no default, so an untyped handler matches no variant.This is the Codex counterpart of #3130 / #3153. It reuses the renderer and ownership code merged there; there is no second implementation.
_with_default_command_type) also runs in_to_codex_hook_entries. Untyped flat command entries render as"type": "command"in.codex/hooks.jsonand its sidecar, including when they join groups from a Codex-shaped hook file in the same package.command,prompt,mcp_tool), untyped non-command entries, handlers already nested in the source, matchers, timeouts, and other targets (Cursor and Gemini are checked by a guard test)._legacy_hook_entriesfor Codex now returns both earlier shapes APM wrote: the original flat entry (before fix(hooks): render Codex hooks in native shape #3060) and the nested untyped group. Proven APM-owned output, including stale root-package sources healed by content match, therefore converges to one typed entry on reinstall andapm update. Hooks with no ownership record are never rewritten or removed, even when they are byte-identical to old APM output.Before (from the issue):
{ "matcher": "Write", "hooks": [ { "command": "node \".codex/hooks/demo-pkg/.apm/hooks/scripts/gate.mjs\"", "timeout": 10 } ] }After:
{ "matcher": "Write", "hooks": [ { "type": "command", "command": "node \".codex/hooks/demo-pkg/.apm/hooks/scripts/gate.mjs\"", "timeout": 10 } ] }Evidence and its limits: this establishes that the generated output matches Codex's source schema. I have not observed a Codex runtime rejecting the old output or accepting the new output.
Issue and approved scope
Issue: #3249
Human scope-approval comment: #3249 (comment) (clarified in #3249 (comment))
This PR completes the approved scope. Out of scope, as agreed: defaults inside already-nested source handlers, camelCase event-name mapping, a shared default across all targets, guessing non-command types, and #2111.
Fixes #3249
Type of change
Testing
New and updated tests:
tests/unit/integration/test_hook_integrator_issue3249.py(14 cases):9 of these fail without the fix. If only the new nested-untyped legacy matching is removed, the stale-root case fails.
test_required_codex_untyped_hook_upgrade_preserves_ownership_and_convergesruns the installed CLI with{standalone, merged} × {nested-untyped, flat}legacy output. It covers first install, two reinstalls andapm updateto a new revision, and checks that:.apmsources are unchanged;All 4 variants fail without the fix.
test_codex_native_edge_types_untyped_flat_commandscovers the renderer and the legacy shapes.In the [BUG] Flat hook entries without
typereach.claude/settings.jsonwithout the requiredtype: command#3130 suite, the guard asserting Codex did not get the default now targets Cursor and Gemini.Run locally, mirroring
ci.yml:relative_tochecks, pylint R0801, and the auth-signal and architecture-boundary linters.apm audit --ci.Not run locally: the Windows jobs and the PyInstaller-built binary (I used the dev install's
apm).Docs:
hooks-and-commands.mdand the apm-guidepackage-authoring.mdnow describe the Claude and Codex input/output contract. The CHANGELOG has an entry under Unreleased.Spec conformance (OpenAPM v0.1)