[PR #33] [MERGED] feat(cursor-review): collect structured output through MCP tools #41

New issue

Closed

opened 2026-10-26 22:35:40 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:35:40 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/33
Author: @huntcsg
Created: 7/17/2026
Status: ✅ Merged
Merged: 8/17/2026
Merged by: @mattmillerai

Base: main ← Head: convert-cursor-review-to-structured-output

📝 Commits (5)

adf1ca4 feat(cursor-review): collect structured output through MCP tools
4268a26 fix(cursor-review): address structured output review feedback
5ed67e8 fix(cursor-review): integrate model preflight after rebase
fa71c41 fix(cursor-review): harden structured output failures
cc96018 fix(cursor-review): preserve prior-review repeats after rebase

📊 Changes

11 files changed (+723 additions, -713 deletions)

View changed files

📝 .github/cursor-review/README.md (+8 -5)
➖ .github/cursor-review/extract-findings.py (+0 -275)
📝 .github/cursor-review/post-review.py (+2 -2)
📝 .github/cursor-review/prompt-adversarial.md (+12 -15)
📝 .github/cursor-review/prompt-edge-case.md (+12 -15)
📝 .github/cursor-review/prompt-judge.md (+16 -19)
➕ .github/cursor-review/review-output-mcp.py (+297 -0)
➖ .github/cursor-review/tests/test_extract_findings.py (+0 -284)
➕ .github/cursor-review/tests/test_review_output_mcp.py (+208 -0)
📝 .github/workflows/cursor-review.yml (+162 -94)
📝 .github/workflows/test-cursor-review-scripts.yml (+6 -4)

📄 Description

Summary

Replace Cursor Review's free-form JSON response contract with a small, dependency-free stdio MCP server that gives reviewers and the judge typed output tools.

reviewers call cursor_review_record_finding once per issue and cursor_review_finish exactly once
the judge calls cursor_review_submit_final exactly once with the adjudicated findings
tool arguments are validated and atomically persisted as the normalized artifacts consumed by later jobs
model stdout remains diagnostic only and is no longer parsed for review results
remove the JSON recovery parser and the second judge call that existed only to reformat malformed output

Strategy

The workflow writes a trusted project-scoped .cursor/mcp.json that starts review-output-mcp.py in reviewer or judge mode. It also writes an exact .cursor/cli.json allowlist for only the output tools needed by that role, avoiding broad --force permissions.

The MCP server owns the result contract:

expose a narrow JSON Schema for findings or final review submission
validate paths, line numbers, sides, severities, body sizes, and exact fields
preserve an explicit non-success state until the agent completes its required submission
atomically replace the artifact only after validation succeeds
refuse subsequent calls from overwriting completed output

The consolidate job continues to consume the existing normalized findings shape, so posting and blocking-gate behavior remain compatible.

Why this should be more reliable

The previous path depended on probabilistic formatting compliance: each model had to place valid JSON in free-form stdout, after which the workflow searched prose and Markdown fences, coerced common wrapper objects, and retried the judge when parsing still failed. That history already includes multiple hardening changes for prose-wrapped, malformed, or otherwise unparseable responses.

This change moves correctness to a typed protocol boundary:

prose, Markdown fences, escaping, and truncated response text cannot corrupt results
schema-invalid findings are rejected before persistence
an empty review is successful only after an explicit finish/final-submit call
a reviewer that records findings and then crashes remains visibly incomplete
a judge that never submits cannot be mistaken for a clean review
missing matrix artifacts become synthetic error cells rather than reducing the apparent panel size or producing a false “no findings” result
judge failure still degrades honestly to raw panel findings

MCP or agent execution can still fail, but those failures now remain explicit operational errors. They are no longer ambiguous parsing failures and cannot silently become an empty successful review. In short, the result path changes from heuristic extraction of model-authored text to deterministic validation and persistence of tool arguments.

Security and compatibility

uses Cursor CLI's supported project-scoped stdio MCP configuration
grants only the three exact MCP tools through project CLI permissions
rejects a symlinked .cursor directory and atomically replaces config files
supports the recent MCP protocol versions used by Cursor
preserves the established findings artifact and post-review.py contracts
preserves raw-panel fallback when the judge does not submit

Verification

python3 -m unittest discover -s .github/cursor-review/tests -p 'test_*.py' -v — 11 tests pass
Python compilation passes
workflow YAML parses successfully
git diff --check passes
local Cursor CLI smoke test successfully loaded the stdio MCP server and listed both reviewer tools
a full model-driven tool-call smoke test was not possible in the development workspace because it has no Cursor authentication
adversarial implementation re-review found no remaining high-confidence blockers

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/github-workflows/pull/33 **Author:** [@huntcsg](https://github.com/huntcsg) **Created:** 7/17/2026 **Status:** ✅ Merged **Merged:** 8/17/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `convert-cursor-review-to-structured-output` --- ### 📝 Commits (5) - [`adf1ca4`](https://github.com/Comfy-Org/github-workflows/commit/adf1ca4aa452c54d106593a9622b90326440fb95) feat(cursor-review): collect structured output through MCP tools - [`4268a26`](https://github.com/Comfy-Org/github-workflows/commit/4268a262d2168525e6672d96368299750a63d007) fix(cursor-review): address structured output review feedback - [`5ed67e8`](https://github.com/Comfy-Org/github-workflows/commit/5ed67e89061f9321b1493bf48500d7352f63333d) fix(cursor-review): integrate model preflight after rebase - [`fa71c41`](https://github.com/Comfy-Org/github-workflows/commit/fa71c418548743c47a58ca1c87ed93d6e674ec89) fix(cursor-review): harden structured output failures - [`cc96018`](https://github.com/Comfy-Org/github-workflows/commit/cc960183e6c7fab2f5440cd5532074f90cd9e83a) fix(cursor-review): preserve prior-review repeats after rebase ### 📊 Changes **11 files changed** (+723 additions, -713 deletions) <details> <summary>View changed files</summary> 📝 `.github/cursor-review/README.md` (+8 -5) ➖ `.github/cursor-review/extract-findings.py` (+0 -275) 📝 `.github/cursor-review/post-review.py` (+2 -2) 📝 `.github/cursor-review/prompt-adversarial.md` (+12 -15) 📝 `.github/cursor-review/prompt-edge-case.md` (+12 -15) 📝 `.github/cursor-review/prompt-judge.md` (+16 -19) ➕ `.github/cursor-review/review-output-mcp.py` (+297 -0) ➖ `.github/cursor-review/tests/test_extract_findings.py` (+0 -284) ➕ `.github/cursor-review/tests/test_review_output_mcp.py` (+208 -0) 📝 `.github/workflows/cursor-review.yml` (+162 -94) 📝 `.github/workflows/test-cursor-review-scripts.yml` (+6 -4) </details> ### 📄 Description ## Summary Replace Cursor Review's free-form JSON response contract with a small, dependency-free stdio MCP server that gives reviewers and the judge typed output tools. - reviewers call `cursor_review_record_finding` once per issue and `cursor_review_finish` exactly once - the judge calls `cursor_review_submit_final` exactly once with the adjudicated findings - tool arguments are validated and atomically persisted as the normalized artifacts consumed by later jobs - model stdout remains diagnostic only and is no longer parsed for review results - remove the JSON recovery parser and the second judge call that existed only to reformat malformed output ## Strategy The workflow writes a trusted project-scoped `.cursor/mcp.json` that starts `review-output-mcp.py` in reviewer or judge mode. It also writes an exact `.cursor/cli.json` allowlist for only the output tools needed by that role, avoiding broad `--force` permissions. The MCP server owns the result contract: 1. expose a narrow JSON Schema for findings or final review submission 2. validate paths, line numbers, sides, severities, body sizes, and exact fields 3. preserve an explicit non-success state until the agent completes its required submission 4. atomically replace the artifact only after validation succeeds 5. refuse subsequent calls from overwriting completed output The consolidate job continues to consume the existing normalized findings shape, so posting and blocking-gate behavior remain compatible. ## Why this should be more reliable The previous path depended on probabilistic formatting compliance: each model had to place valid JSON in free-form stdout, after which the workflow searched prose and Markdown fences, coerced common wrapper objects, and retried the judge when parsing still failed. That history already includes multiple hardening changes for prose-wrapped, malformed, or otherwise unparseable responses. This change moves correctness to a typed protocol boundary: - prose, Markdown fences, escaping, and truncated response text cannot corrupt results - schema-invalid findings are rejected before persistence - an empty review is successful only after an explicit finish/final-submit call - a reviewer that records findings and then crashes remains visibly incomplete - a judge that never submits cannot be mistaken for a clean review - missing matrix artifacts become synthetic error cells rather than reducing the apparent panel size or producing a false “no findings” result - judge failure still degrades honestly to raw panel findings MCP or agent execution can still fail, but those failures now remain explicit operational errors. They are no longer ambiguous parsing failures and cannot silently become an empty successful review. In short, the result path changes from heuristic extraction of model-authored text to deterministic validation and persistence of tool arguments. ## Security and compatibility - uses Cursor CLI's supported project-scoped stdio MCP configuration - grants only the three exact MCP tools through project CLI permissions - rejects a symlinked `.cursor` directory and atomically replaces config files - supports the recent MCP protocol versions used by Cursor - preserves the established findings artifact and `post-review.py` contracts - preserves raw-panel fallback when the judge does not submit ## Verification - `python3 -m unittest discover -s .github/cursor-review/tests -p 'test_*.py' -v` — 11 tests pass - Python compilation passes - workflow YAML parses successfully - `git diff --check` passes - local Cursor CLI smoke test successfully loaded the stdio MCP server and listed both reviewer tools - a full model-driven tool-call smoke test was not possible in the development workspace because it has no Cursor authentication - adversarial implementation re-review found no remaining high-confidence blockers --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:35:40 +00:00