[BUG] Flat hook entries without type reach .codex/hooks.json without the required type: command #3249

Activity

1. github-project-automation

   added this to APM Roadmapon Oct 16, 2026
2. Parth-Vasave commented on Oct 16, 2026

   

   parth (Parth-Vasave)

   on Oct 16, 2026

   ContributorAuthor

   More actions

   Hi, I'd like to take this one. I fixed the Claude counterpart in #3153, so I know this render path.

   Proposed scope, for a maintainer to confirm or adjust:

   • Scope: In Codex-native rendering, supply type: "command" for recognized command handlers whose flat source entry omits type, mirroring the Claude-only approach in fix(hooks): type untyped flat command handlers for Claude #3153. Preserve explicitly supplied handler types, and leave untyped entries that aren't recognizable as commands unchanged.
   • Done when: Untyped flat command input produces schema-valid Codex command handlers in .codex/hooks.json. Explicit types stay intact. Reinstalling over hooks written by earlier versions converges to one typed entry without leaving untyped duplicates. Regression tests cover these cases, and the hooks guidance reflects the Codex input/output contract. Claude, Cursor and other targets stay unchanged.
   • Out of scope: Codex event-name mapping (camelCase preToolUse), changing the shared hook representation to impose a default on every target, guessing non-command handler types, and the broader [FEATURE] Define OpenAPM hook semantics and replace pairwise hook translation #2111 work.

   Two questions:

   1. fix(hooks): type untyped flat command handlers for Claude #3153 isn't merged yet, and this would build on the same renderer and cleanup code. Should I wait for it to merge before opening a PR?
   2. Should handlers already inside nested Codex groups also get the default, or only flat input as in fix(hooks): type untyped flat command handlers for Claude #3153? I'd keep it to flat input unless you say otherwise.

   As with #3130, I'll describe the result as schema-valid output rather than claim a confirmed Codex runtime rejection.
3. danielmeppiel commented on Oct 21, 2026

   

   Daniel Meppiel (danielmeppiel)

   on Oct 21, 2026

   Collaborator

   More actions

   Decision: approve
   Area: project
   Scope: In Codex-native rendering, supply type: command when converting recognized command handlers whose flat source entries omit type, including merging converted entries into existing groups. Reuse the existing native-rendering and ownership mechanisms; preserve explicit types, non-command input, matchers, timeouts, restrictions, consent, supporting resources and other targets. Reconcile proven APM-owned older output to the corrected form without stale untyped duplicates.
   Done when: Installed-CLI first-install, reinstall and update coverage demonstrates one correctly typed Codex command entry, with regression coverage for both older flat and currently generated nested-untyped APM-owned output. Explicit types, non-command and already-nested source handlers remain unchanged; manual, unowned or ambiguously owned hooks are not rewritten or removed. Verify the pinned native schema, preserve denied-consent behavior and source files, and do not execute hooks during installation. Update directly related hook documentation and usage guidance. Describe generated-schema correctness separately from any independently obtained native runtime evidence.
   Out of scope: Defaulting types inside already-nested source handlers, event-name mapping, a shared-representation default across targets, guessing non-command types, new targets, broader #2111 semantics, frontmatter-hook expansion, module-packaging changes, a competing contribution, assignment, technical PR approval, release commitment, automation dispatch or restart of blocked delivery.
   Review contact: Daniel Meppiel (@danielmeppiel)
4. danielmeppiel commented on Oct 21, 2026

   

   Daniel Meppiel (danielmeppiel)

   on Oct 21, 2026

   Collaborator

   More actions

   Accepted: correct the Codex output APM generates from simple flat command entries, and repair older output that APM can prove it owns.

   For example, when an author supplies {"command": "python check.py"}, APM already wraps it in Codex's nested hook structure. That generated handler also needs "type": "command". Reinstall/update must replace the earlier incomplete APM-owned entry, not leave both versions behind.

   parth (@Parth-Vasave), this preserves your existing claim and answers the two scope questions:

   • fix(hooks): type untyped flat command handlers for Claude #3153 merged on October 20 at 09:59:43 UTC, so waiting for that merge is no longer necessary. Use the merged renderer/ownership mechanisms rather than a competing implementation.
   • Keep this repair to flat source input, including when the converted output joins existing native groups. Do not add defaults inside already-nested source handlers. Those authors must supply the type explicitly. Explicit command/non-command types and unrelated native configuration remain untouched.

   The distinction is the input shape, not whether the final output is nested. Replacing an older nested entry generated by APM from flat input is included when ownership is established; rewriting a manually authored or ambiguously owned entry is not.

   The October 20 investigation ran the actual installed Python CLI for first and repeat local-package installs at 53ccf06d11dd2d675e8ea8c3e2523a6d321c1b79. Untyped flat input remained untyped for Codex but gained command for Claude; explicit types, a manual hook and source bytes were preserved, and a sentinel confirmed the fixture hook did not execute. The native renderer is byte-identical at the October 21 main snapshot 9ff06af17bdec985f07a5b8565344ac155a29490. Other install/resource-path code has advanced; the full lifecycle was not rerun at that newer revision.

   The inspected upstream Codex schema requires the type discriminator. This is generated-output and source-schema evidence, not an observed Codex runtime rejection or proof of repaired historical migration.

   The accepted tradeoff is deliberate: already-structured source with a missing type remains an authoring error rather than receiving automatic correction. No event-name conversion, broad hook semantics, frontmatter expansion or packaging change is included. #3130's Claude-only acceptance and the separate consent/module-metadata work remain unchanged.

   The adjacent strict scope defines completion. The issue remains open with labels and assignees unchanged; the existing contributor claim is preserved without a new assignment. This product decision does not approve a PR, promise a release or dispatch automated implementation.

   Prepared and published with AI assistance following the maintainer's explicit narrow-scope confirmation. Evidence and its limits are described above.
5. Parth-Vasave commented on Oct 25, 2026

   

   parth (Parth-Vasave)

   on Oct 25, 2026

   ContributorAuthor

   More actions

   Thanks for confirming the scope, Daniel Meppiel (@danielmeppiel). I'm starting on this now and will build on the renderer and ownership code merged in #3153:

   • Flat source command entries that omit type get "type": "command" in .codex/hooks.json, including when they merge into existing groups. Already-nested source handlers, explicit types and non-command entries stay unchanged.
   • Reinstall and update replace APM-owned output from older versions (both the original flat shape and the current nested-untyped shape) with one typed entry. Manual or ambiguously owned hooks are left alone.
   • I'll cover first install, reinstall and update with the installed CLI, update the hook docs and usage guidance, and report the generated-schema result separately from any runtime evidence.

   I've confirmed against the pinned Codex source (36512a34, hook_config.rs) that HookHandlerConfig is #[serde(tag = "type")] with no default.