Skip to content

fix(fp-check): use correct JSON response format in stop hooks#129

Open
jonathanreedmevs wants to merge 2 commits intotrailofbits:mainfrom
jonathanreedmevs:fix/fp-check-hook-json-response
Open

fix(fp-check): use correct JSON response format in stop hooks#129
jonathanreedmevs wants to merge 2 commits intotrailofbits:mainfrom
jonathanreedmevs:fix/fp-check-hook-json-response

Conversation

@jonathanreedmevs
Copy link
Copy Markdown

Summary

  • Stop and SubagentStop hook prompts were instructing Claude to return plain text ('block' or 'approve')
  • Claude Code expects prompt-type hooks to respond with JSON, causing Stop hook error: JSON validation failed on every session end
  • Updated both prompts to respond with {"decision": "block", "reason": "..."} to block, or {} to allow stopping

Root Cause

The hook prompts on line 10 and line 22 ended with:

return 'block' with the specific gaps.
return 'approve'.

Claude Code parses the hook response as JSON. Plain text fails JSON validation, producing the error even though the hook logic itself was correct.

Fix

Replace plain-text return instructions with explicit JSON response format matching the Claude Code stop hook schema (decision field). Bump version to 1.0.1 so installed clients receive the update.

- If ANY bug is missing ANY of these phases or the gate review, return 'block' with the specific gaps.
- If all bugs have complete verification with verdicts, return 'approve'.
- If the conversation is not about fp-check verification at all, return 'approve'.
+ If ANY bug is missing ANY of these phases or the gate review, respond with JSON: {"decision": "block", "reason": "<specific gaps>"}
+ If all bugs have complete verification with verdicts, respond with JSON: {}
+ If the conversation is not about fp-check verification at all, respond with JSON: {}

Same change applies to the SubagentStop prompt on line 22.

Discovery

Bug discovered by Claude while debugging the Stop hook error: JSON validation failed error during active use of the fp-check skill.

Bug Replication

  • Start a Claude Code session with fp-check installed
  • Prompt Claude with anything
  • Confirm Stop hook error: JSON validation failed appears
  • Apply fix, restart session, confirm error is gone

Prompt-type stop hooks must respond with JSON. The previous prompts
instructed Claude to return plain text ('block' or 'approve'), causing
'Stop hook error: JSON validation failed' on every session end.

Updated both Stop and SubagentStop hook prompts to respond with:
- {"decision": "block", "reason": "..."} to prevent stopping
- {} to allow stopping (omitting decision field per Claude Code docs)

Bug discovered by Claude while debugging the JSON validation error
during active use of the fp-check skill.
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 19, 2026

CLA assistant check
All committers have signed the CLA.

@dguido
Copy link
Copy Markdown
Member

dguido commented Mar 31, 2026

Automated Review — PR #129

Flagged for human review (touches hooks)

Review Findings

1. BLOCKER: marketplace.json deletes dimensional-analysis entry

This branch is based on 5c15f4f (before PR #132 merged). The marketplace.json diff removes the dimensional-analysis plugin entry that was added by PR #132 (9df4731). This is a rebase/merge artifact — the PR author likely resolved the file without realizing the new entry existed. The branch needs to be rebased onto current main before merging, or this deletion will silently remove dimensional-analysis from the marketplace.

2. Hook JSON response format is correct

The fix correctly addresses a real bug. Per Claude Code docs, prompt-type Stop hooks must return JSON. The old prompts instructed Claude to return plain text ('block' / 'approve'), which caused Stop hook error: JSON validation failed on every session end.

The new format uses:

  • {"decision": "block", "reason": "<specific gaps>"} to block — correct per docs
  • {} (empty JSON) to allow stopping — correct per docs (omitting decision allows the action)

3. Overlap with PR #137

PR #137 fixes the same bug with a slightly different approach:

Both are valid per the docs. PR #137 also includes the IMPORTANT: Return valid JSON only guardrail which is a useful addition — LLMs sometimes wrap JSON in markdown code fences. However, "decision": "approve" is not documented as a valid value (the docs say to omit the field or return {}), so PR #129's approach is arguably more correct.

These two PRs will conflict — they both modify the same lines in hooks.json. Only one should be merged; the other needs to be closed or rebased.

4. Version bump is correct

plugin.json and marketplace.json both bump to 1.0.1 and match. Appropriate for a bugfix.

5. No security concerns

The change is prompt text only. No executable code, no path changes, no new dependencies.

Validation

  • validate_codex_skills.py: PASS (60 plugin skills, 61 Codex entries)
  • validate_plugin_metadata.py: PASS (35 plugins in sync)

Action Items

  1. Rebase onto main to pick up the dimensional-analysis marketplace entry from PR add dimensional analysis plugin #132
  2. Coordinate with PR fix: fp-check Stop hooks return JSON instead of plain text #137 — decide which approach to merge and close the other

Changes Made

None — review only.


Reviewed by Claude Code

@dguido dguido requested review from ahpaleus and dguido as code owners March 31, 2026 22:14
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.

3 participants