Skip to content

fix(hooks): PreCompact emits {} no-op (never empty) so Codex accepts the JSON (#66) — v6.3.1#67

Merged
intel352 merged 3 commits into
mainfrom
fix/precompact-codex-empty-json-66
Jun 1, 2026
Merged

fix(hooks): PreCompact emits {} no-op (never empty) so Codex accepts the JSON (#66) — v6.3.1#67
intel352 merged 3 commits into
mainfrom
fix/precompact-codex-empty-json-66

Conversation

@intel352
Copy link
Copy Markdown
Contributor

@intel352 intel352 commented Jun 1, 2026

Summary

Bug fix for #66 — PreCompact hook returned "invalid PreCompact hook JSON output" on Codex. systematic-debugging root-caused + TDD.

Closes #66.

Root cause

hooks/pre-compact-snapshot emitted empty stdout on its no-locked-plans path (the common case at compaction) + the disabled/TTY/no-jq/empty-input guards. Claude Code tolerates empty PreCompact output (no-op); Codex rejects empty as invalid JSON. The v6.3.0 wrapper (#41) recovers JSON behind diagnostics but still emitted nothing for empty output — so this path was uncovered. Most sessions at compaction have no autodev locked plan (incl. the reporter's workflow-compute session) → the empty path is the common case.

Fix (defense-in-depth, both invocation paths)

Invariant: every hook emits a valid JSON object on stdout on every exit path; empty → {} (a universal no-op on Claude Code, valid JSON for Codex).

  • hooks/pre-compact-snapshotnoop_json() { printf '{}\n'; exit 0; } on the 5 empty-exit paths (covers Codex invoking the hook directly, bypassing the wrapper — Regression: PreCompact hook still returns invalid JSON in Codex #66 asks for the installed-layout path).
  • hooks/run-hook.cmd — empty hook output → {} (covers the wrapper path for all hooks when jq is present).

Tests (the Codex-style regression #66 requested)

  • tests/hook-contracts.sh::test_pre_compact_snapshot_emits_json_when_no_locked_plans — runs the hook directly (no wrapper, Codex-style) on a no-docs/plans cwd + the disabled path; asserts stdout is valid JSON. (RED before fix: got: [].)
  • tests/hook-stdout-discipline.sh case (e) — empty fixture → wrapper emits {}.
  • 3 existing tests that asserted empty output updated to "no populated snapshot" (jq -e .hookSpecificOutput.additionalContext), robust to both {} and empty.
  • CI-gated by hooks-check.yml (added in v6.3.0). Adversarial design review (cycle 1, 0C/2I — incl. the 3 test-inventory catch — resolved) + 2-stage code review APPROVED.

Honesty note

I cannot reproduce on Codex from here. Verified on the source layout by the new direct-invocation regression (the hook now emits valid JSON on every path, removing the empty-stdout root cause). {} resolves the not-valid-JSON rejection class; if Codex enforces a typed schema that rejects unknown objects, that's a separate error needing a Codex session — captured here so the next investigator isn't misled.

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings June 1, 2026 14:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a cross-host hook contract issue where pre-compact-snapshot (and the run-hook.cmd wrapper) could emit empty stdout on no-op paths, which Codex rejects as invalid JSON. The change enforces an invariant that the PreCompact hook always emits a valid JSON object, using {} as a universal no-op, and adds regressions to cover both direct (Codex-style) and wrapper invocation paths.

Changes:

  • Update hooks/pre-compact-snapshot to emit {} on all previously-empty early-exit/no-op paths.
  • Update hooks/run-hook.cmd to emit {} when a hook produces empty stdout (jq-present path).
  • Add/adjust tests to assert valid JSON output (including {}) on no-op paths, plus update release notes and plugin versions to v6.3.1.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/hook-stdout-discipline.sh Adds wrapper regression ensuring empty hook stdout becomes {} (valid JSON).
tests/hook-contracts.sh Adds direct-invocation PreCompact regression and updates “no snapshot” assertions to be {}-tolerant.
RELEASE-NOTES.md Documents v6.3.1 fix for Codex invalid-JSON PreCompact behavior.
hooks/run-hook.cmd Emits {} instead of empty output when captured hook stdout is empty (jq-present wrapper path).
hooks/pre-compact-snapshot Introduces noop_json and uses it on all no-op/guard exit paths to guarantee valid JSON.
docs/plans/2026-06-01-precompact-codex-empty-json-design.md Adds design note documenting root cause, fix strategy, and tests.
.cursor-plugin/plugin.json Bumps plugin version to 6.3.1.
.claude-plugin/plugin.json Bumps plugin version to 6.3.1.
.claude-plugin/marketplace.json Bumps marketplace version to 6.3.1.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@intel352 intel352 merged commit 59438d7 into main Jun 1, 2026
8 checks passed
@intel352 intel352 deleted the fix/precompact-codex-empty-json-66 branch June 1, 2026 15:29
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.

Regression: PreCompact hook still returns invalid JSON in Codex

2 participants