diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 91db5b7..ae437f2 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -9,7 +9,7 @@ { "name": "autodev", "description": "Autonomous development workflow skills for coding agents", - "version": "6.3.0", + "version": "6.3.1", "source": "./", "author": { "name": "Jon Langevin", diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 940fae4..d22c713 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "autodev", "description": "Autonomous development workflow skills for coding agents: design, review, planning, execution, monitoring, and retrospectives", - "version": "6.3.0", + "version": "6.3.1", "author": { "name": "Jon Langevin", "email": "jon@gocodealone.com" diff --git a/.cursor-plugin/plugin.json b/.cursor-plugin/plugin.json index 47f5913..3ec3935 100644 --- a/.cursor-plugin/plugin.json +++ b/.cursor-plugin/plugin.json @@ -2,7 +2,7 @@ "name": "autodev", "displayName": "Autonomous Dev Kit", "description": "Autonomous development workflow skills for coding agents", - "version": "6.3.0", + "version": "6.3.1", "author": { "name": "Jon Langevin", "email": "jon@gocodealone.com" diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 2c3ee1a..4530d1e 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -1,5 +1,23 @@ # Autonomous Dev Kit Release Notes +## v6.3.1 — 2026-06-01 + +Bug fix for **#66** — PreCompact hook returned invalid JSON on **Codex**. + +- Root cause: `hooks/pre-compact-snapshot` emitted **empty stdout** on its no-locked-plans + path (the common case at compaction) and other guard paths. Claude Code tolerates empty + PreCompact output; Codex rejects it as "invalid PreCompact hook JSON output". The v6.3.0 + wrapper recovered JSON behind diagnostics but still emitted nothing for empty output. +- Fix (defense-in-depth, both invocation paths): every exit path now emits a valid JSON + object — `hooks/pre-compact-snapshot` emits a `{}` no-op instead of empty (covers Codex + invoking the hook directly), and `hooks/run-hook.cmd` emits `{}` for any empty hook output + (covers the wrapper path for every hook). `{}` is a universal no-op on Claude Code and + valid JSON for Codex. +- New regression `tests/hook-contracts.sh::test_pre_compact_snapshot_emits_json_when_no_locked_plans` + runs the installed hook the way Codex invokes it (directly, no wrapper) and asserts valid + JSON on the no-locked-plans + disabled paths; `tests/hook-stdout-discipline.sh` case (e) + asserts the wrapper emits `{}` for empty output. Both CI-gated by `hooks-check.yml`. + ## v6.3.0 — 2026-06-01 Pipeline-hardening release closing seven recurring gate-miss / context-waste issues diff --git a/docs/plans/2026-06-01-precompact-codex-empty-json-design.md b/docs/plans/2026-06-01-precompact-codex-empty-json-design.md new file mode 100644 index 0000000..4741e32 --- /dev/null +++ b/docs/plans/2026-06-01-precompact-codex-empty-json-design.md @@ -0,0 +1,124 @@ +# Fix: PreCompact invalid-JSON on Codex (#66) — Design + +**Status:** Approved (autonomous bug-fix; user-directed; v6.3.1 release after merge) +**Date:** 2026-06-01 +**Issue:** #66 (regression/uncovered-path of #41) +**Type:** Bug fix (systematic-debugging root-caused) +**Adversarial review:** design cycle 1 = FAIL (0C/2I/3m) → resolved this revision (I1 test-inventory + I2 jq-present scoping + m1 schema-vs-syntax + m2 TTY + m3 test-(b) note). + +## Root cause + +`hooks/pre-compact-snapshot` exits with **empty stdout** on five paths — the common one +being **no locked plan in `docs/plans/`** (`[ -z "$state_section" ] && exit 0`, line 132), +plus disabled / TTY-stdin / no-jq / empty-input. Claude Code tolerates empty PreCompact +output (no-op); **Codex rejects it** ("hook returned invalid PreCompact hook JSON output"). +The v6.3.0 wrapper (#41) routes *non-JSON* to stderr and recovers JSON behind warnings, but +its empty-output branch emits **nothing** (`: # nothing to emit`) — so the empty path is +still empty on the wrapper route too. Most sessions at compaction have no autodev locked +plan (incl. the reporter's workflow-compute session) → the empty path is the *common* case. + +Evidence constraint: cannot run Codex here. Conclusion derived from the hook's exit paths + +the exact error wording (empty ≠ valid JSON) + the common-case match. This is the +highest-confidence, verifiable-on-source-layout root cause. + +## Invariant (the fix) + +**Every hook emits a valid JSON object on stdout on every exit path; empty becomes `{}`.** +`{}` is the minimal valid JSON object and a universal **no-op** across all Claude Code hook +events (SessionStart=no context, PreToolUse/PostToolUse=no decision→proceed, +UserPromptSubmit=no context/no block, Stop/SubagentStop=no block, PreCompact=no-op) — so it +changes no Claude Code behavior — while satisfying Codex's "valid JSON" requirement. + +Fix at **two layers** (defense in depth, covers both invocation paths — #66 asks "not only +the Claude wrapper path", "the installed hook the way Codex invokes it"): + +1. **`hooks/pre-compact-snapshot`** — replace each empty `exit 0` (lines 18/20/21/24/132) + with `printf '{}\n'; exit 0` (via a `noop_json` helper; `printf` needs no jq, so the + no-jq path is covered). The content path (line ~163, after `emit_additional_context`) + is unchanged. Covers **direct invocation** (Codex bypassing the wrapper). (Line 20 is the + TTY guard — an interactive-only path no host triggers with TTY stdin; adding `noop_json` + there is harmless, m2.) +2. **`hooks/run-hook.cmd`** — the empty-hook-output branch emits `{}` instead of nothing. + Covers all hooks on the wrapper path **when jq is present** (the jq-absent branch is a + verbatim `exec bash` passthrough — that path is covered for pre-compact-snapshot only, + by its own `printf`-based fix; I2). Safe no-op on Claude Code for every event type. + +## Tests requiring update (I1 — these existing tests assert empty output) + +Three existing `hook-contracts.sh` tests run pre-compact-snapshot on a *no-snapshot* +scenario and assert `[ -n "$output" ]` is empty; after the fix `output="{}"`, so they must +change to "no *populated* snapshot" rather than "empty string": +- `test_scope_lock_complete*` (~line 654): completed (not locked) plan → no snapshot. +- `pre_compact_ignores_prose_mention_of_locked_status` (~line 1182): draft plan → no snapshot. +- `pre_compact_ignores_single_workspace_lock_when_session_has_no_lock` (~line 1279). + +New assertion (robust to both old-empty and new-`{}`): **fail only if the output contains a +populated snapshot** — +```bash +if printf '%s' "$output" | jq -e '.hookSpecificOutput.additionalContext' >/dev/null 2>&1; then + fail "... expected no lock snapshot, got a populated one: $output" +fi +``` +(`jq -e .hookSpecificOutput.additionalContext` is false for both `{}` and empty, true only +for a real snapshot.) The existing with-locked-plan test (asserts the populated +`hookSpecificOutput`) is unchanged. + +`tests/hook-stdout-discipline.sh` test (b) (noise-only fixture) is **unaffected** (noise +routes via the wrapper's `else` branch, not the empty branch, so stdout stays empty there); +the new wrapper test is a **distinct case (e): a fixture that emits nothing at all → wrapper +emits `{}`**. + +## Tests (the regression #66 explicitly requests) + +- `tests/hook-contracts.sh`: `test_pre_compact_snapshot_emits_json_when_no_locked_plans` — + run the hook **directly** (`run_hook`, NOT the wrapper — Codex-style) against a tmp cwd + with no `docs/plans/`, assert stdout parses as JSON (`jq -e .`) and is non-empty. Plus the + disabled-env path (`SUPERPOWERS_HOOKS_DISABLE=1`) also emits `{}`. The existing + with-locked-plan test continues to assert the `hookSpecificOutput` shape. +- `tests/hook-stdout-discipline.sh`: a fixture that emits empty → the wrapper emits `{}` + (valid JSON), not empty. +- CI: `hooks-check.yml` (added in v6.3.0) gates both on Linux. + +## Global Design Guidance + +`Guidance: README §Cross-LLM. The contract is host-neutral: stdout must be valid JSON for +the strictest host (Codex). `{}` is the lowest-common-denominator no-op.` + +## Security Review + +`{}` carries no directive — a PreToolUse/Stop hook emitting `{}` = no block = proceed, +identical to today's empty/exit-0 behavior. No block decision is weakened (the v6.3.0 +discipline still recovers an explicit `{"decision":"block"}` behind a warning). No secrets, +no network. + +## Infrastructure Impact + +None. Hook + wrapper + tests + version bump (v6.3.0 → v6.3.1, patch). Release via +`release-tag.yml`. + +## Multi-Component Validation + +The `test_pre_compact_snapshot_emits_json_when_no_locked_plans` runs the **real** hook the +way Codex invokes it (direct, no wrapper) — the actual boundary #66 names. The wrapper test +runs the **real** `run-hook.cmd` against an empty-output fixture. + +## Assumptions + +| id | assumption | challenge | fallback | +|---|---|---|---| +| A1 | Codex rejects empty PreCompact stdout but accepts a valid JSON object | Could reject `{}` specifically if it enforces a strict typed schema | `{}` is the minimal valid object; if Codex needs specific fields, that requires Codex-side repro (documented limitation — I cannot run Codex). Emitting valid JSON is strictly better than empty. **`{}` solves only the "not valid JSON" rejection class — NOT a "wrong schema shape" rejection.** If Codex enforces a typed PreCompact schema that rejects unknown objects, a *different* error will surface; the issue-close note must capture this distinction so the next investigator isn't misled (m1). | +| A2 | `{}` is a no-op for every Claude Code hook event | Some event might warn on `{}` | All events treat `{}` as "no directives"; the existing with-content tests confirm no regression to the populated path. | +| A3 | Codex invokes the installed hook (direct and/or via wrapper) | Unknown exact path | Fixed at BOTH layers so either path emits valid JSON. | + +## Rollback + +Revert the PR; the hook returns to empty-exit, the wrapper to empty-branch. No migration. +Do not tag v6.3.1 if reverted pre-merge. + +## Honesty note (verification-before-completion) + +I cannot reproduce on Codex from here. This fix is verified on the **source layout** by the +new direct-invocation regression (the hook now emits valid JSON on every path, including the +empty/no-locked-plans case that previously emitted nothing). That removes the +highest-confidence root cause (empty ≠ valid JSON). Final Codex confirmation belongs to a +Codex session; the issue close note will state this. diff --git a/hooks/pre-compact-snapshot b/hooks/pre-compact-snapshot index 5bc19ef..24178b8 100755 --- a/hooks/pre-compact-snapshot +++ b/hooks/pre-compact-snapshot @@ -15,13 +15,19 @@ set -euo pipefail -[ "${SUPERPOWERS_HOOKS_DISABLE:-}" = "1" ] && exit 0 +# #66: every exit path must emit a valid JSON object on stdout. Codex rejects empty +# PreCompact output ("invalid PreCompact hook JSON output"); Claude Code treats `{}` as a +# no-op. `printf` needs no jq, so this also covers the jq-absent path. Works whether the +# hook is invoked directly (Codex) or via run-hook.cmd. +noop_json() { printf '{}\n'; exit 0; } -[ -t 0 ] && exit 0 -command -v jq >/dev/null 2>&1 || exit 0 +[ "${SUPERPOWERS_HOOKS_DISABLE:-}" = "1" ] && noop_json + +[ -t 0 ] && noop_json +command -v jq >/dev/null 2>&1 || noop_json hook_input=$(cat || true) -[ -z "$hook_input" ] && exit 0 +[ -z "$hook_input" ] && noop_json cwd_dir=$(printf '%s' "$hook_input" | jq -r '.cwd // empty' 2>/dev/null || true) [ -z "$cwd_dir" ] && cwd_dir="${PWD}" @@ -128,8 +134,9 @@ if [ -d "$plans_dir" ]; then fi if [ -z "$state_section" ]; then - # No active locked plans found — nothing to snapshot. Exit silently. - exit 0 + # No active locked plans — nothing to snapshot. Emit a `{}` no-op (not empty) so + # Codex accepts the PreCompact output (#66). This is the common case. + noop_json fi # ── Append to autodev-state file ───────────────────────────────────────── diff --git a/hooks/run-hook.cmd b/hooks/run-hook.cmd index 73f5872..5edcfaf 100755 --- a/hooks/run-hook.cmd +++ b/hooks/run-hook.cmd @@ -63,7 +63,7 @@ if command -v jq >/dev/null 2>&1; then hook_out="$(bash "${SCRIPT_DIR}/${SCRIPT_NAME}" "$@")" hook_rc=$? if [ -z "$hook_out" ]; then - : # nothing to emit + printf '{}\n' # #66: emit a `{}` no-op (not empty) so strict hosts (Codex) accept it elif printf '%s' "$hook_out" | jq -e . >/dev/null 2>&1; then printf '%s\n' "$hook_out" # valid JSON as a whole else diff --git a/tests/hook-contracts.sh b/tests/hook-contracts.sh index 6defdf3..38c3e89 100755 --- a/tests/hook-contracts.sh +++ b/tests/hook-contracts.sh @@ -419,6 +419,24 @@ test_pretool_pr_review_json() { assert_hook_context_json "pretool-pr-review-reminder" "PreToolUse" "$output" } +test_pre_compact_snapshot_emits_json_when_no_locked_plans() { + # #66: Codex rejects empty PreCompact stdout. The no-locked-plans path (common case) + # must emit a valid JSON object, not empty. Run the hook DIRECTLY (Codex-style — no wrapper). + local tmp; tmp="$(mktemp -d)" + trap 'rm -rf "$tmp"' RETURN + local out; out="$(run_hook pre-compact-snapshot '{"cwd":"'"$tmp"'"}')" + if printf '%s' "$out" | jq -e . >/dev/null 2>&1; then + pass "pre-compact-snapshot: emits valid JSON (not empty) with no locked plans (#66)" + else + fail "pre-compact-snapshot: no-locked-plans must emit valid JSON, got: [$out]" + fi + # The disabled path must also emit valid JSON (the host still invokes the hook). + local outd; outd="$(export SUPERPOWERS_HOOKS_DISABLE=1; run_hook pre-compact-snapshot '{"cwd":"'"$tmp"'"}')" + printf '%s' "$outd" | jq -e . >/dev/null 2>&1 \ + && pass "pre-compact-snapshot: emits valid JSON when disabled (#66)" \ + || fail "pre-compact-snapshot: disabled path must emit valid JSON, got: [$outd]" +} + test_pr_reminder_dedup() { local tmp; tmp="$(mktemp -d)" trap 'rm -rf "$tmp"' RETURN @@ -650,7 +668,7 @@ PLAN return fi compact_output="$(run_hook pre-compact-snapshot '{"cwd":"'"$tmp"'","transcript_path":"'"$transcript"'"}')" - if [ -n "$compact_output" ]; then + if printf '%s' "$compact_output" | jq -e '.hookSpecificOutput.additionalContext' >/dev/null 2>&1; then fail "scope-lock-complete: expected completed plan to produce no pre-compact lock snapshot, got: ${compact_output}" return fi @@ -1176,7 +1194,7 @@ test_pre_compact_ignores_prose_mention_of_locked_status() { mkdir -p "$tmp/docs/plans" emit_draft_fixture "$tmp/docs/plans/draft.md" "draft" output="$(run_hook pre-compact-snapshot '{"cwd":"'"$tmp"'","transcript_path":"'"$transcript"'"}')" - if [ -n "$output" ]; then + if printf '%s' "$output" | jq -e '.hookSpecificOutput.additionalContext' >/dev/null 2>&1; then fail "pre-compact-snapshot: prose mention of Locked status triggered snapshot, output: ${output}" return fi @@ -1273,7 +1291,7 @@ test_pre_compact_ignores_single_workspace_lock_when_session_has_no_lock() { mkdir -p "$tmp/docs/plans" emit_locked_fixture "$tmp/docs/plans/active.md" "active" output="$(run_hook pre-compact-snapshot '{"cwd":"'"$tmp"'","transcript_path":"'"$transcript"'"}')" - if [ -n "$output" ]; then + if printf '%s' "$output" | jq -e '.hookSpecificOutput.additionalContext' >/dev/null 2>&1; then fail "pre-compact-snapshot: single workspace lock falsely triggered fallback, output: ${output}" return fi @@ -1811,6 +1829,7 @@ test_prompt_strict_uses_session_locked_plan_only test_prompt_strict_ignores_prose_mention_of_locked_status test_pretool_pr_review_json test_pr_reminder_dedup +test_pre_compact_snapshot_emits_json_when_no_locked_plans test_posttool_pr_created_json test_pre_compact_snapshot_json test_wrapper_suppresses_pre_compact_locale_noise diff --git a/tests/hook-stdout-discipline.sh b/tests/hook-stdout-discipline.sh index 196859b..0fcb075 100755 --- a/tests/hook-stdout-discipline.sh +++ b/tests/hook-stdout-discipline.sh @@ -10,7 +10,7 @@ command -v jq >/dev/null 2>&1 || { echo "SKIP: jq required for a/b/c"; exit 0; } tmp="$(mktemp -d)" # Cleanup trap set BEFORE any fixture is copied into hooks/ (rm -f on absent files is safe). -trap 'rm -f "$REPO_ROOT"/hooks/fix-warn-then-json "$REPO_ROOT"/hooks/fix-noise "$REPO_ROOT"/hooks/fix-clean; rm -rf "$tmp"' EXIT +trap 'rm -f "$REPO_ROOT"/hooks/fix-warn-then-json "$REPO_ROOT"/hooks/fix-noise "$REPO_ROOT"/hooks/fix-clean "$REPO_ROOT"/hooks/fix-empty; rm -rf "$tmp"' EXIT mkfix() { printf '%s\n' "$1" > "$tmp/$2"; chmod +x "$tmp/$2"; } # Fixture A: a warning leaks to stdout, then a block JSON on stdout. @@ -23,8 +23,11 @@ echo "just a diagnostic line"' fix-noise # Fixture C: clean single-line JSON. mkfix '#!/usr/bin/env bash printf "%s\n" "{\"hookSpecificOutput\":{\"hookEventName\":\"X\"}}"' fix-clean +# Fixture E: emits NOTHING at all (#66) — wrapper must emit a `{}` no-op, not empty. +mkfix '#!/usr/bin/env bash +exit 0' fix-empty -for f in fix-warn-then-json fix-noise fix-clean; do cp "$tmp/$f" "$REPO_ROOT/hooks/$f"; done +for f in fix-warn-then-json fix-noise fix-clean fix-empty; do cp "$tmp/$f" "$REPO_ROOT/hooks/$f"; done run() { OUT="$("$WRAPPER" "$1" 2>"$tmp/err")"; RC=$?; ERR="$(cat "$tmp/err")"; } @@ -58,4 +61,10 @@ OUTD="$(PATH="$nojq" "$WRAPPER" fix-warn-then-json 2>/dev/null)" && pass "(d) jq-absent → verbatim passthrough (no discipline applied)" \ || fail "(d) expected verbatim passthrough with jq absent, got: $OUTD" +# (e) empty hook output (#66) → wrapper emits a `{}` no-op (valid JSON), not empty. +run fix-empty +printf '%s' "$OUT" | jq -e '. == {}' >/dev/null 2>&1 \ + && pass "(e) empty hook output → wrapper emits {} no-op (valid JSON)" \ + || fail "(e) expected {} for empty hook output, got: [$OUT]" + echo ""; echo "Results: $failures failure(s)"; [ "$failures" -eq 0 ]