diff --git a/scripts/review/validate-agent-output.sh b/scripts/review/validate-agent-output.sh index fedab55..2929522 100755 --- a/scripts/review/validate-agent-output.sh +++ b/scripts/review/validate-agent-output.sh @@ -14,13 +14,19 @@ while IFS= read -r line || [[ -n "$line" ]]; do done < "$review_dir/agent.ndjson" jq -Rse 'split("\n") | map(fromjson?) | + def recoverable_comment_location_failure: + (.part.state.metadata.exit // -1) == 1 and + ((.part.state.output // .part.state.error // "") | + test("comment[[:space:]]+(position|line)[[:space:]]+(is|was)[[:space:]]+(invalid|unresolvable|not[[:space:]]+part[[:space:]]+of[[:space:]]+the[[:space:]]+diff)"; "i") or + (test("422|unprocessable[[:space:]]+entity"; "i") and + test("comment|review|pull[[:space:]]+request"; "i") and + test("position|line|side|diff[[:space:]]+hunk"; "i"))) + ; any(.[]; .type == "text" and (.part.text | type == "string" and test("\\S"))) and any(.[]; .type == "step_finish" and .part.reason == "stop") and all(.[]; .type != "error" and (.type != "tool_use" or (.part.state.status == "completed" and - ((.part.state.metadata.exit // -1) == 0 or - ((.part.state.metadata.exit // -1) == 1 and - ((.part.state.output // .part.state.error // "") | test("comment.*(position|line)|(position|line).*comment"; "i")))))) and + ((.part.state.metadata.exit // -1) == 0 or recoverable_comment_location_failure))) and (.type != "step_finish" or .part.reason == "stop" or .part.reason == "tool-calls")) ' "$review_dir/agent.ndjson" >/dev/null diff --git a/test/pr_review_test.go b/test/pr_review_test.go index a1d0d62..be64bb3 100644 --- a/test/pr_review_test.go +++ b/test/pr_review_test.go @@ -17,7 +17,7 @@ func TestPRReview(t *testing.T) { if err != nil { t.Fatal(err) } - for _, scenario := range []string{"success", "unlabeled", "stale", "oversized", "tampered", "agent-failure", "provider-failure", "cleanup-failure", "sandbox-gone", "cancel", "truncated", "malformed-trailing", "incomplete", "empty", "error", "tool_use", "tool_exit", "tool_missing_exit", "unrelated-422", "comment-position"} { + for _, scenario := range []string{"success", "unlabeled", "stale", "oversized", "tampered", "agent-failure", "provider-failure", "cleanup-failure", "sandbox-gone", "cancel", "truncated", "malformed-trailing", "incomplete", "empty", "error", "tool_use", "tool_exit", "tool_missing_exit", "unrelated-422", "unrelated-422-line", "unrelated-422-comment", "unrelated-comment", "comment-position"} { t.Run(scenario, func(t *testing.T) { root := t.TempDir() stepSummary := filepath.Join(root, "step-summary") @@ -171,6 +171,9 @@ case "$1 ${2:-}" in tool_exit) printf '%s\n' '{"type":"tool_use","part":{"state":{"status":"completed","metadata":{"exit":7},"output":"ordinary command failed"}}}' ;; tool_missing_exit) printf '%s\n' '{"type":"tool_use","part":{"state":{"status":"completed","metadata":{},"output":"missing exit"}}}' ;; unrelated-422) printf '%s\n' '{"type":"tool_use","part":{"state":{"status":"completed","metadata":{"exit":1},"output":"unrelated build failed at record 422"}}}'; printf '%s\n' '{"type":"step_finish","part":{"reason":"stop"}}' ;; + unrelated-422-line) printf '%s\n' '{"type":"tool_use","part":{"state":{"status":"completed","metadata":{"exit":1},"output":"build failed at line 422"}}}'; printf '%s\n' '{"type":"step_finish","part":{"reason":"stop"}}' ;; + unrelated-422-comment) printf '%s\n' '{"type":"tool_use","part":{"state":{"status":"completed","metadata":{"exit":1},"output":"comment delivery failed with status 422"}}}'; printf '%s\n' '{"type":"step_finish","part":{"reason":"stop"}}' ;; + unrelated-comment) printf '%s\n' '{"type":"tool_use","part":{"state":{"status":"completed","metadata":{"exit":1},"output":"comment formatting failed"}}}'; printf '%s\n' '{"type":"step_finish","part":{"reason":"stop"}}' ;; comment-position) printf '%s\n' '{"type":"tool_use","part":{"state":{"status":"completed","metadata":{"exit":1},"output":"comment position is invalid"}}}'; printf '%s\n' '{"type":"step_finish","part":{"reason":"stop"}}' ;; *) printf '%s\n' '{"type":"step_finish","part":{"reason":"stop"}}' ;; esac ;;