From d9c22ffe50a93d9c496e40649b5be576bd1da552 Mon Sep 17 00:00:00 2001 From: Vlad Mocanu Date: Thu, 26 Feb 2026 09:55:57 +0200 Subject: [PATCH] =?UTF-8?q?docs(prd-6):=20mark=20M2=20complete=20=E2=80=94?= =?UTF-8?q?=20PR=20review=20comment=20trigger=20validated?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit E2E test confirmed: Claude correctly responds to @claude mentions in PR review comments on Forgejo (run #12075, PR #15). Three bugs were found and fixed during validation: 1. Missing comment object in Forgejo webhook payload 2. Event name mismatch (pull_request_comment vs pull_request_review_comment) 3. MCP server routing updates to wrong API (critical — silently dropped responses) Progress: 3/8 milestones complete (37.5%) Co-Authored-By: Claude Opus 4.6 --- prds/6-e2e-test-forgejo-triggers.md | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/prds/6-e2e-test-forgejo-triggers.md b/prds/6-e2e-test-forgejo-triggers.md index a390675..c415102 100644 --- a/prds/6-e2e-test-forgejo-triggers.md +++ b/prds/6-e2e-test-forgejo-triggers.md @@ -41,13 +41,18 @@ Systematically exercise each untested scenario on the live Forgejo instance (git - **Key learning**: Never use `mock.module()` in Bun test files — it poisons the module registry for all files in the process. `spyOn` on ESM namespace exports also fails to restore correctly across files on Linux Bun. Best approach: avoid mocking entirely when the real function is safe to call. - **Validation**: CI passed (all 3 jobs: test, prettier, typecheck) on commit `49eedd5` -- [ ] **M2: Test PR review comment trigger** +- [x] **M2: Test PR review comment trigger** - - Create a test PR on the repo - - Post a review comment with `@claude` mention - - Verify: platform detection, context fetching (PR diff, review comment body), tracking comment, Claude response - - Document any issues and fix them - - **Validation**: Claude responds correctly to `@claude` in a PR review comment + - Create a test PR on the repo ✓ (PR #15) + - Post a review comment with `@claude` mention ✓ (review comments on `test/e2e-prd6-m2.txt`) + - Verify: platform detection, context fetching (PR diff, review comment body), tracking comment, Claude response ✓ + - Document any issues and fix them ✓ (3 bugs found and fixed, see below) + - **Validation**: Claude responds correctly to `@claude` in a PR review comment ✓ (run #12075, 32s, full PR summary posted) + - **Bugs found & fixed**: + 1. `9d8c96f`–`ac8e22e`: Forgejo `pull_request_review_comment` payload lacks the top-level `comment` object — added fallback to issue comment creation and 404 fallback for review comment fetch + 2. `e73c881`: Forgejo sends event name `pull_request_comment` (not `pull_request_review_comment`) — skipped `commentBody` validation for this event + 3. `c3c3eb5`: **Critical** — MCP comment server set `isPullRequestReviewComment=true` on Forgejo, causing every Claude update to try `pulls.updateReviewComment` (404) and silently fail. Fix: detect Forgejo via `GITHUB_API_URL` and route to issue comment API. Also hardened `update-claude-comment.ts` fallback to catch 422 alongside 404, and skipped the wasted `pulls.getReviewComment` call in `update-comment-link.ts` on Forgejo. + - **Key learning**: On Forgejo, the tracking comment for review comment events is always an issue comment (not a PR review comment reply) because the payload lacks `comment.id`. All update paths must account for this. - [ ] **M3: Test PR review trigger** @@ -100,6 +105,7 @@ Systematically exercise each untested scenario on the live Forgejo instance (git | Trigger type not supported by Forgejo Actions | Document as known limitation, skip gracefully | | Action creates unwanted branches/comments during testing | Use a dedicated test repo or clean up after each test | | Bun mock.module breaks mock tracking on Linux | RESOLVED: use spyOn on module namespace, never mock.module | +| MCP comment updates silently fail on Forgejo | RESOLVED: route to issue comment API when platform=forgejo | ## Success Criteria