Skip to content

fix(review): keep code-review output in tracking comment only#45

Open
varin-nair-factory wants to merge 3 commits intodevfrom
vn/remove-review-comments
Open

fix(review): keep code-review output in tracking comment only#45
varin-nair-factory wants to merge 3 commits intodevfrom
vn/remove-review-comments

Conversation

@varin-nair-factory
Copy link
Collaborator

@varin-nair-factory varin-nair-factory commented Feb 28, 2026

Summary

This PR consolidates code review output into the existing tracking comment to reduce PR timeline noise.

  • Removes github_pr___submit_review from code-review tool allowlists in both single-pass and validator paths.
  • Adds a guard so user-provided MCP tool overrides cannot re-enable github_pr___submit_review.
  • Updates review prompts to publish via inline comments + tracking comment updates only.
  • Updates no-findings behavior to only update the tracking comment (no separate review summary comment).
  • Adjusts prompt/command tests to match the new behavior.

Scope

Code review flow only (security review flow unchanged).

Validation

  • bun test
  • bun run typecheck
  • bun run format:check

@varin-nair-factory varin-nair-factory self-assigned this Feb 28, 2026
@varin-nair-factory varin-nair-factory marked this pull request as ready for review February 28, 2026 01:18
@Factory-AI Factory-AI deleted a comment from factory-droid bot Feb 28, 2026
@varin-nair-factory varin-nair-factory marked this pull request as draft February 28, 2026 01:37
@varin-nair-factory varin-nair-factory marked this pull request as ready for review February 28, 2026 01:37
@factory-droid
Copy link
Contributor

factory-droid bot commented Feb 28, 2026

Droid finished @varin-nair-factory's task —— View job


Code Review Summary

The PR correctly removes github_pr___submit_review from review flows and blocks user overrides. One finding:

  • [P1] src/create-prompt/templates/review-prompt.ts line 338 — "ID" is outside the ternary for droidCommentId, so the prompt reads "Update tracking comment ID for this run" when the ID is absent. The validator template handles this correctly; this one should match.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
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