Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 57 additions & 25 deletions action/post-review-comments.sh
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,20 @@ gh_api_safe() {
extract_review_json() {
local log_content="$1"

# Use Python for robust JSON extraction that handles nested braces correctly
# Use Python for robust JSON extraction that handles nested braces correctly.
# The log file contains stream-json output from Claude Code, with lines like:
# {"type":"user","message":{"content":[...]}} ← prompt (may contain PR diff!)
# {"type":"assistant","message":{"content":[...]}} ← Claude's response
# {"type":"result","result":"<Claude's final text>"}
# We must extract from the "result" event only, otherwise template JSON
# embedded in the PR diff (e.g. security-review.md examples) can be matched.
local json_block
json_block=$(echo "$log_content" | python3 -c "
import sys
import re
import json

content = sys.stdin.read()
raw_input = sys.stdin.read()

def find_json_objects(text):
'''Find all valid JSON objects in text using bracket matching.'''
Expand Down Expand Up @@ -168,35 +174,61 @@ def find_json_objects(text):
i += 1
return objects

# First, try to extract from markdown code blocks
code_block_pattern = r'\`\`\`(?:json)?\s*\n(.*?)\n\`\`\`'
code_blocks = re.findall(code_block_pattern, content, re.DOTALL)
def search_for_review(text):
'''Search text for a review JSON object. Returns it or None.'''
# First, try to extract from markdown code blocks
code_block_pattern = r'\`\`\`(?:json)?\s*\n(.*?)\n\`\`\`'
code_blocks = re.findall(code_block_pattern, text, re.DOTALL)

# Check code blocks first (in reverse order to get the final summary)
for block in reversed(code_blocks):
for obj in find_json_objects(block):
if 'summary' in obj and 'comments' in obj:
print(json.dumps(obj))
sys.exit(0)
# Check code blocks first (in reverse order to get the final summary)
for block in reversed(code_blocks):
for obj in find_json_objects(block):
if 'summary' in obj and 'comments' in obj:
return obj

# Then check the full content for bare JSON
all_objects = find_json_objects(content)
# Then check for bare JSON
all_objects = find_json_objects(text)

# Look for the summary object (should have summary, verdict, comments)
for obj in reversed(all_objects):
if 'summary' in obj and 'comments' in obj:
print(json.dumps(obj))
sys.exit(0)
# Look for the summary object (should have summary, verdict, comments)
for obj in reversed(all_objects):
if 'summary' in obj and 'comments' in obj:
return obj

# Fallback: try to find any object with comments array
for obj in reversed(all_objects):
if 'comments' in obj and isinstance(obj.get('comments'), list):
obj.setdefault('summary', 'Review completed.')
obj.setdefault('verdict', 'comment')
return obj

return None

# Step 1: Try to extract from stream-json result events.
# This narrows the search to Claude's actual output, avoiding prompt content.
result_text = None
for line in raw_input.splitlines():
line = line.strip()
if not line:
continue
try:
event = json.loads(line)
if isinstance(event, dict) and event.get('type') == 'result' and 'result' in event:
result_text = str(event['result'])
except (json.JSONDecodeError, TypeError):
continue

# Fallback: try to find any object with comments array
for obj in reversed(all_objects):
if 'comments' in obj and isinstance(obj.get('comments'), list):
# Add missing fields
obj.setdefault('summary', 'Review completed.')
obj.setdefault('verdict', 'comment')
print(json.dumps(obj))
if result_text is not None:
review = search_for_review(result_text)
if review is not None:
print(json.dumps(review))
sys.exit(0)

# Step 2: Fallback — scan full content (backward compat for non-stream-json logs)
review = search_for_review(raw_input)
if review is not None:
print(json.dumps(review))
sys.exit(0)

# No structured output found
print(json.dumps({
'summary': 'Review completed but no structured output found.',
Expand Down
158 changes: 158 additions & 0 deletions action/test_extract_review_json.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
#!/usr/bin/env bash
# test_extract_review_json.sh — Verify extract_review_json parses stream-json correctly
#
# Reproduces the PR #152 bug where template JSON in the PR diff (inside a
# stream-json "user" event) was matched instead of Claude's actual review
# output (inside the "result" event).
set -euo pipefail

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
PASS=0
FAIL=0

# Source the function by providing dummy env vars so the main body's
# required-var checks pass, then intercepting execution at the main section.
# We use a subshell trick: extract everything before "Main" section.
TMPFUNC=$(mktemp)
trap 'rm -f "$TMPFUNC"' EXIT

# Extract lines from the script: shebang + set options + all functions (up to the Main marker)
sed -n '1,/^# Main$/p' "$SCRIPT_DIR/post-review-comments.sh" | head -n -1 > "$TMPFUNC"

# Source just the functions (no main execution)
# shellcheck disable=SC1090
source "$TMPFUNC"

assert_eq() {
local test_name="$1" expected="$2" actual="$3"
if [[ "$expected" == "$actual" ]]; then
echo " PASS: $test_name"
PASS=$((PASS + 1))
else
echo " FAIL: $test_name"
echo " expected: $expected"
echo " actual: $actual"
FAIL=$((FAIL + 1))
fi
}

# ---------------------------------------------------------------------------
# Test 1: Stream-json log with poisoned prompt (PR #152 reproduction)
# The user event contains a PR diff that includes template JSON with
# summary+comments fields. The result event has Claude's real review.
# ---------------------------------------------------------------------------
echo "Test 1: Stream-json with poisoned prompt (PR #152 reproduction)"

# Template JSON that would appear in a PR diff of security-review.md
template_json='{"summary": "No security vulnerabilities found.", "verdict": "approve", "comments": []}'

# Claude's actual review with real findings
real_review='{"summary": "Found 3 critical issues in auth module.", "verdict": "request_changes", "comments": [{"file": "auth.py", "line": 42, "severity": "critical", "category": "security", "comment": "SQL injection via unsanitized input"}]}'

# Construct stream-json log lines using Python for reliable JSON encoding
log_content=$(python3 -c "
import json
user_event = json.dumps({
'type': 'user',
'message': {'content': [{'type': 'text', 'text': 'Review this PR.\n\nHere is security-review.md:\n\`\`\`json\n$template_json\n\`\`\`'}]}
})
assistant_event = json.dumps({
'type': 'assistant',
'message': {'content': [{'type': 'text', 'text': \"I'll review the changes.\"}]}
})
result_event = json.dumps({
'type': 'result',
'result': 'Here is my review:\n\n\`\`\`json\n$real_review\n\`\`\`'
})
print(user_event)
print(assistant_event)
print(result_event)
")

result=$(extract_review_json "$log_content")
actual_summary=$(echo "$result" | python3 -c "import sys,json; print(json.load(sys.stdin)['summary'])")
actual_verdict=$(echo "$result" | python3 -c "import sys,json; print(json.load(sys.stdin)['verdict'])")
actual_count=$(echo "$result" | python3 -c "import sys,json; print(len(json.load(sys.stdin)['comments']))")

assert_eq "extracts real review summary" "Found 3 critical issues in auth module." "$actual_summary"
assert_eq "extracts real review verdict" "request_changes" "$actual_verdict"
assert_eq "extracts real review comments" "1" "$actual_count"

# ---------------------------------------------------------------------------
# Test 2: Non-stream-json log (backward compatibility)
# Plain text output without stream-json envelope — should still work.
# ---------------------------------------------------------------------------
echo "Test 2: Plain text log (backward compatibility)"

plain_log="Some preamble output...

\`\`\`json
$real_review
\`\`\`

Done."

result=$(extract_review_json "$plain_log")
actual_summary=$(echo "$result" | python3 -c "import sys,json; print(json.load(sys.stdin)['summary'])")
assert_eq "backward compat extracts review" "Found 3 critical issues in auth module." "$actual_summary"

# ---------------------------------------------------------------------------
# Test 3: Stream-json log with bare JSON in result (no code block)
# ---------------------------------------------------------------------------
echo "Test 3: Stream-json with bare JSON in result"

log_bare=$(python3 -c "
import json
user_event = json.dumps({
'type': 'user',
'message': {'content': [{'type': 'text', 'text': 'Review.\n$template_json'}]}
})
result_event = json.dumps({
'type': 'result',
'result': '$real_review'
})
print(user_event)
print(result_event)
")

result=$(extract_review_json "$log_bare")
actual_summary=$(echo "$result" | python3 -c "import sys,json; print(json.load(sys.stdin)['summary'])")
assert_eq "bare JSON in result event" "Found 3 critical issues in auth module." "$actual_summary"

# ---------------------------------------------------------------------------
# Test 4: No review JSON at all
# ---------------------------------------------------------------------------
echo "Test 4: No review JSON found"

log_empty=$(python3 -c "
import json
print(json.dumps({'type': 'result', 'result': 'I reviewed the code and it looks fine.'}))
")
result=$(extract_review_json "$log_empty")
actual_summary=$(echo "$result" | python3 -c "import sys,json; print(json.load(sys.stdin)['summary'])")
assert_eq "fallback when no JSON" "Review completed but no structured output found." "$actual_summary"

# ---------------------------------------------------------------------------
# Test 5: Multiple result events — should use the last one
# ---------------------------------------------------------------------------
echo "Test 5: Multiple result events (uses last)"

log_multi=$(python3 -c "
import json
print(json.dumps({'type': 'result', 'result': json.dumps({'summary': 'First pass.', 'verdict': 'comment', 'comments': []})}))
print(json.dumps({'type': 'result', 'result': json.dumps({'summary': 'Final review.', 'verdict': 'approve', 'comments': []})}))
")

result=$(extract_review_json "$log_multi")
actual_summary=$(echo "$result" | python3 -c "import sys,json; print(json.load(sys.stdin)['summary'])")
assert_eq "uses last result event" "Final review." "$actual_summary"

# ---------------------------------------------------------------------------
# Summary
# ---------------------------------------------------------------------------
echo ""
echo "Results: $PASS passed, $FAIL failed"
if [[ "$FAIL" -gt 0 ]]; then
exit 1
fi
echo "All tests passed."
Loading