fix: route deep-review findings via structured output so empty model fails loudly#1010
fix: route deep-review findings via structured output so empty model fails loudly#1010ital0 wants to merge 1 commit into
Conversation
…fails loudly
- read-only model step had no Write tool, so the file-write output contract
produced nothing; the post phase ENOENT'd and the check went green-but-no-op
- emit findings via claude-code-action's --json-schema structured_output channel
- a non-model shell step materializes structured_output to THUNDER_FINDINGS_FILE
and exits non-zero on empty output (broken model = red check), while a genuine
{"findings":[]} passes
- model stays read-only (no Write/Bash)
Semgrep Security ScanNo security issues found. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a24ae3f. Configure here.
| # ───────────────────────────────────────────────────────────────────── | ||
| - name: Compute diff to a bounded file | ||
| run: | | ||
| git diff "$PR_BASE_SHA".."$PR_HEAD_SHA" > "$THUNDER_DIFF_FILE" || true |
There was a problem hiding this comment.
PR diff uses two-dot range
High Severity
The diff step uses a two-dot git diff between PR_BASE_SHA and PR_HEAD_SHA, which compares the two branch tips directly. GitHub PR diffs use merge-base-to-head (three-dot) semantics, so when the base branch moves ahead of the PR branch the patch can include unrelated base changes and misrepresent what the PR actually changes.
Reviewed by Cursor Bugbot for commit a24ae3f. Configure here.
|
Superseded: this fix belongs on PR #1003 (branch deep-review-tooling) and was folded there. This branch was created in error by a tooling mishap. Closing. |
|
Preview environment deployed 🚀
Stack: Auto-destroys on PR close/merge. Login via the bundled Keycloak realm — |
PR Metrics
Updated Fri, 19 Jun 2026 13:03:01 GMT · run #1970 |
1 participant
Why
The
thunder-deep-reviewworkflow's review check was passing green but doing nothing.The model step runs read-only (
--allowedTools Read,Grep,Glob— noWrite/Bash), yet the prompt's OUTPUT CONTRACT told it to write the findings JSON toTHUNDER_FINDINGS_FILE. With noWritetool, the file was never created → the post phase'sreadModelFindings()hitENOENT→ the orchestrator fail-softexit(0)→ a green check that never posted a review.Fix
claude-code-action's--json-schemastructured output channel instead of a model-written file. The model returns its findings as its final result; the action constrains it to the schema and exposes it onsteps.model.outputs.structured_output. No write tool needed — model stays read-only.Materialize structured findings) bridgesstructured_output→THUNDER_FINDINGS_FILEfor the orchestrator.structured_output(model auth failure,--max-turnsexhaustion, schema reject, etc. → red check, not a silent green no-op). A genuine empty review ({"findings":[]}) is non-empty JSON and passes the gate; the post step then no-ops cleanly.No change to
review-orchestrator.mjs— the structured output is{"findings":[...]}, which already matches what the post phase parses.Note
Low Risk
CI workflow wiring only; no app runtime or auth logic changes, with clearer failure signaling when the model step breaks.
Overview
Fixes
thunder-deep-reviewsilently passing when the model never produced findings. The read-only model step (Read,Grep,Globonly) was still instructed to writeTHUNDER_FINDINGS_FILE, so the file was missing, the post orchestrator fail-soft exited 0, and the check went green with no comment.The model step now returns findings through
claude-code-action--json-schemastructured output (steps.model.outputs.structured_output) with an inline findings schema, and the prompt OUTPUT CONTRACT tells the model to return structured JSON only (no file writes). A newMaterialize structured findingsshell step copies that output intoTHUNDER_FINDINGS_FILEfor the existing post phase.That materialize step fails the job if
structured_outputis empty or whitespace (broken auth, max turns, schema reject), while a real no-op review ({"findings":[]}) still passes and lets post no-op cleanly.Reviewed by Cursor Bugbot for commit a24ae3f. Bugbot is set up for automated code reviews on this repo. Configure here.