From 1748a2c3dfe06004dca0736500e41d12d672aced Mon Sep 17 00:00:00 2001 From: Vlad Mocanu Date: Thu, 26 Feb 2026 10:09:37 +0200 Subject: [PATCH] =?UTF-8?q?docs(prd-6):=20mark=20M3=20complete=20=E2=80=94?= =?UTF-8?q?=20PR=20review=20trigger=20validated?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.6 --- prds/6-e2e-test-forgejo-triggers.md | 30 ++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/prds/6-e2e-test-forgejo-triggers.md b/prds/6-e2e-test-forgejo-triggers.md index c415102..f017e6a 100644 --- a/prds/6-e2e-test-forgejo-triggers.md +++ b/prds/6-e2e-test-forgejo-triggers.md @@ -54,13 +54,16 @@ Systematically exercise each untested scenario on the live Forgejo instance (git 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** +- [x] **M3: Test PR review trigger** - - Create a test PR on the repo - - Submit a review with `@claude` mention in the review body - - Verify: platform detection, context fetching, tracking comment, Claude response - - Document any issues and fix them - - **Validation**: Claude responds correctly to `@claude` in a PR review + - Reused test PR #15 from M2 ✓ + - Submitted reviews with `@claude` mention in the review body ✓ (review IDs 20, 21, 22) + - Verify: platform detection, context fetching, tracking comment, Claude response ✓ + - Document any issues and fix them ✓ (1 finding, 1 fix, see below) + - **Validation**: Claude responds correctly to `@claude` in a PR review ✓ (runs 12080, 12087) + - **Finding**: Forgejo sends `pull_request_comment` instead of `pull_request_review` for review submissions — same pattern as M2. The `pull_request_review` trigger and workflow condition (line 21) are dead code on Forgejo. + - **Fix** (`e514bf5`→`b21dbd6`): Replaced blind Forgejo auto-trigger in `trigger.ts` with payload-aware `@claude` check that inspects `comment.body` and `review.body` with fallback. Attempted workflow `if:` gating but reverted — Forgejo's `pull_request_comment` payload does not populate `github.event.comment.body` or `github.event.review.body` (run 12084 was skipped). + - **Key learning**: On Forgejo, the workflow `if:` condition cannot reliably gate on payload body fields for `pull_request_comment` events. The `@claude` check must happen in application code (`trigger.ts`), not in the workflow condition. - [ ] **M4: Test label trigger** @@ -99,13 +102,14 @@ Systematically exercise each untested scenario on the live Forgejo instance (git ## Risks -| Risk | Mitigation | -| -------------------------------------------------------- | ----------------------------------------------------------- | -| Forgejo webhook payloads differ from expected | Log raw payloads, compare with GitHub format, adapt parsing | -| 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 | +| Risk | Mitigation | +| -------------------------------------------------------- | ------------------------------------------------------------ | +| Forgejo webhook payloads differ from expected | Log raw payloads, compare with GitHub format, adapt parsing | +| 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 | +| Workflow if: cannot gate on payload body for Forgejo | RESOLVED: @claude check moved to trigger.ts application code | ## Success Criteria