Fix schema-clean JSON hook installs#50
Merged
Merged
Conversation
- Drop obsolete managed markers from all hook templates. - Keep the snippets aligned with the current config shape. - Avoid carrying metadata that no longer affects behavior.
- Generalize config detection from static markers to detectors - Preserve legacy JSON _acd_managed detection for migration paths - Support JSON command signature matching without relying on spacing
- Keep per-path install detection consistent across both callers. - Replace marker-slice checks with the resolved detector field.
- Stop treating top-level `_acd_managed` as an install detector input. - Report the legacy Codex marker in doctor output with migration guidance. - Keep schema-breaking Codex configs visible without accepting them as installs.
- Expand detection tests to cover both signature-based and legacy JSON markers. - Add a reusable helper for asserting top-level JSON shape in setup tests. - Keep test fixtures aligned with the current hook schema while preserving legacy compatibility coverage.
- Ensure setup JSON no longer emits the legacy _acd_managed marker. - Tighten canonical hook schema checks before unmarshaling the JSON block. - Keep coverage aligned across Claude Code, Cursor, and Codex setup output.
- Tighten setup tests to expect the canonical hook JSON shape - Remove legacy top-level markers from the schema assertions - Keep invalid-JSON checks aligned with the current templates
- Drop the redundant top-level marker assertion from the Claude Code stop-hook test. - Update the Codex fixture to use the canonical hook JSON shape so the test covers real setup output.
- Use canonical snippets for setup fixtures instead of inline JSON. - Treat schema-clean hooks.json separately from legacy _acd_managed cases. - Add regression coverage for legacy Codex JSON marker handling.
- Reuse the canonical hooks.json snippet in repo-local and home-based tests - Match doctor expectations to the newer hook-stdin-extract signature - Fix a misleading JSON unmarshal failure message in the Codex marker test
- Refresh hook setup and doctor expectations for the new Cursor JSON shape - Verify raw setup output stays valid JSON and doctor drift checks use the updated hook body/snippet format
- Clarify doctor diagnostics for alternate config-path installs - Use consistent ACD wording in notes and duplicate-hook warnings - Improve readability of hook conflict messages without changing logic
- Explain that raw setup now emits strict hooks.json with only hooks. - Warn that old _acd_managed markers are legacy-only fallback data. - Clarify uninstall and doctor guidance for mixed or duplicated installs.
- Remove obsolete `_acd_managed` references from hook install docs - Clarify uninstall steps for older installs and merged custom hooks - Keep the guidance aligned across Claude Code and Cursor templates
- Codex 0.141+ rejects unknown top-level hook fields. - Keep the changelog aligned with the new JSON hook template behavior. - Preserve legacy `_acd_managed` handling for migration and doctor flow.
- Limit legacy `_acd_managed` checks to the emitted JSON block. - Avoid false positives from surrounding shell/setup output. - Keep Claude Code, Cursor, and Codex assertions aligned.
- Keep the marker declarations aligned for readability - No behavior changes; this is a formatting-only cleanup
- Keep doctor diagnostics aligned with alternate config locations. - Preserve the legacy _acd_managed warning for repo-local Codex hooks. - Extend coverage so the warning is verified from a repo-local install.
- Match only acd-managed hook objects by command/harness. - Preserve merged custom hooks by removing empty event keys only. - Update uninstall guidance for Claude Code, Codex, and Cursor.
- Simplifies the legacy _acd_managed guidance to favor schema-clean regeneration and manual preservation of custom hooks. - Keeps doctor warnings and tests aligned with the revised remediation text. - Updates the Codex template docs to match the new recommendation.
- Treat Claude Code and Codex JSON hooks like other active-hook harnesses - Count missing PreToolUse/PostToolUse entries as drift for these configs - Add regression coverage for JSON-backed harnesses missing active hooks
- Align uninstall instructions with the current hook command forms. - Replace outdated harness-based matching with the commands now used. - Keep removal guidance consistent across Claude Code, Codex, and Cursor.
- Extend setup test coverage for uninstall documentation - Verify each harness uninstall snippet lists its lifecycle commands - Catch regressions where docs drift from the supported command flow
- Reflects the new doctor behavior for Codex and Claude Code installs when required hooks are missing. - Notes the uninstall documentation fix so JSON harness guidance covers all lifecycle hooks, including commands without `--harness`. - Keeps the changelog aligned with the actual drift-detection and docs behavior changes in this branch.
- Retry fixture cleanup after macOS runner unlink races - Use an explicit temp directory so teardown can wait for git I/O - Keep bootstrap assertions focused on shadow behavior
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.
Bug Description
JSON hook installs needed to be schema-clean while still being detectable and diagnosable by ACD.
Expected:
_acd_managed.acd doctorreports missing active hooks as drift.Actual:
SessionStart-only Codex/Claude install could be treated as healthy.--harness.Root Cause
JSON install detection was tied to the legacy top-level
_acd_managedmarker. After removing that marker, ACD needed format-aware command-signature detection and drift checks that count missing active JSON hook events, not just stale commands that still exist.Type of Change
Fix Description
_acd_managedfrom JSON hook templates.acd doctorfor legacy JSON markers and missing Codex/Claude active hooks.How to Verify
acd setup codex --rawemits JSON with onlyhooksat the top level.acd doctor --jsonreports drift for schema-clean Codex/Claude installs missingPreToolUseandPostToolUse.acd touch,acd flush --logical, andacd stop.Testing
git diff --check 30c4c38455d81c71e7a3c2a3a9a86f659bb19f43...HEADcleanenv make lintcleanenv make testImpact Assessment
Regression Risk
acd doctordrift reportingChecklist
Related Issues
None found in branch name or commit messages.