Skip to content

Fix review feedback MCP tools: SQL bug, ownership, naming, and list tool#742

Merged
selfcontained merged 2 commits into
mainfrom
agt_ea20b4945b1f/agent-945b1f
Jul 9, 2026
Merged

Fix review feedback MCP tools: SQL bug, ownership, naming, and list tool#742
selfcontained merged 2 commits into
mainfrom
agt_ea20b4945b1f/agent-945b1f

Conversation

@selfcontained

Copy link
Copy Markdown
Owner

Summary

  • Fix ambiguous SQL column in resolveReviewFeedbackItem — the RETURNING clause reused unqualified column names (id, status, etc.) that are present on both review_feedback_items and reviews, causing PostgreSQL to throw "column reference 'id' is ambiguous" on every call. Fixed by inlining fi.-qualified columns.
  • Broaden ownership checks to accept assigned_agent_id in addition to agent_id across resolve, thread message, and list queries — so an agent assigned to handle a review can actually use the tools.
  • Rename tools to dispatch_review_* prefix for consistency: dispatch_resolve_review_feedbackdispatch_review_resolve, dispatch_add_review_messagedispatch_review_add_message.
  • Add dispatch_review_list_feedback tool so agents can discover review feedback item IDs instead of guessing (previously only exposed via REST, not MCP).
  • Add 15 integration tests covering create, resolve (fixed/ignored/wont_fix), status progression, ownership enforcement, assigned_agent_id access, thread messages, and listing.
  • Fix incomplete context type for listReviewFeedback — added missing fields (diffSnapshot, baseRef, resolvedBy, resolvedAt, updatedAt) to match the actual return type.

Test plan

  • pnpm run check — types pass
  • pnpm run test — 2236 unit tests pass (125 files)
  • pnpm run test:e2e — 172 e2e tests pass
  • New test file apps/server/test/review-feedback.test.ts — 15 tests covering all review feedback operations
  • Backend security review persona approved (round 1 + round 2)

🤖 Generated with Claude Code

selfcontained and others added 2 commits July 9, 2026 13:09
The McpRequestContext type for listReviewFeedback was missing diffSnapshot,
baseRef, resolvedBy, resolvedAt, and updatedAt — fields that
listFeedbackItemsForAgent actually returns. Align the type with reality.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Fix ambiguous column reference in resolveReviewFeedbackItem RETURNING
  clause by qualifying all columns with fi. prefix
- Broaden ownership checks to accept assigned_agent_id in addition to
  agent_id across resolve, thread message, and list queries
- Rename dispatch_resolve_review_feedback → dispatch_review_resolve and
  dispatch_add_review_message → dispatch_review_add_message
- Add dispatch_review_list_feedback tool so agents can discover item IDs
- Add 15 integration tests covering create, resolve, thread messages,
  ownership enforcement, and listing
- Update notification prompt with new tool names and list-first workflow

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@selfcontained
selfcontained merged commit 62dc3f2 into main Jul 9, 2026
1 check passed
@selfcontained
selfcontained deleted the agt_ea20b4945b1f/agent-945b1f branch July 9, 2026 19:23
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.

1 participant