public/claude-code-action
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(prd-6): archive completed PRD to prds/done/

Browse Source 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Vlad MocanuandClaude Opus 4.6 committed 2026-02-26 11:32:53 +02:00
1 parent 4a30b9819e
commit 57353f9eb9
1 file changed
Download Patch File Download Diff File Expand all files Collapse all files
prds/done/6-e2e-test-forgejo-triggers.md
+144
View File
@@ -0,0 +1,144 @@
# PRD #6: E2E Test All Untested Forgejo Trigger Scenarios
**Issue**: [#6](https://git.wxs.ro/actions/claude-code-action/issues/6)
**Priority**: High
**Created**: 2026-02-25
---
## Problem
The Forgejo platform support (PRD #1) was validated for the two most common scenarios (agent mode via `workflow_dispatch` and tag mode via `issue_comment`). However, 5 trigger scenarios remain untested in a live environment:
1. PR review comment triggers (`pull_request_review_comment`)
2. PR review triggers (`pull_request_review`)
3. Label triggers (`issues` with `labeled` action)
4. Assignee triggers (`issues` with `assigned` action)
5. Branch creation + code changes from tag mode (the initial test only asked a question, no code was generated)
These untested paths could have silent failures — the code may look correct but break on Forgejo's subtly different webhook payloads, API responses, or event structures.
## Solution
Systematically exercise each untested scenario on the live Forgejo instance (git.wxs.ro), document results, fix any issues discovered, and update the E2E status table in `docs/forgejo.md`.
## Milestones
- [x] **M1: Ensure workflow covers all trigger types**
- Review `.forgejo/workflows/claude.yml` to confirm all 5 trigger types are listed
- Add any missing triggers to the workflow file
- Ensure the workflow has the correct permissions for each trigger type
- **Validation**: Workflow YAML includes all required `on:` triggers
- [x] **M1.5: Fix image-downloader test failures blocking CI**
- **Problem**: All 18 `downloadCommentImages` tests failed on Forgejo CI (Linux Bun 1.2.21 in `harbor.wxs.ro/wxs/grunt-runner:v1.9.0`) while passing locally on macOS. This blocked all PRs.
- **Root cause**: `mock.module("../src/github/utils/image-downloader")` in `test/forgejo-data-fetcher.test.ts` replaced the image-downloader module **globally** in Bun's module registry for the entire process. When Bun's non-deterministic test file ordering ran forgejo-data-fetcher.test.ts BEFORE image-downloader.test.ts, the latter imported the stubbed module instead of the real one, breaking all mock tracking. The failure was intermittent because file ordering changed when other files were added/removed.
- **Fix (2 parts)**:
1. `c6ca8a7`: Added DI (`FsDeps` interface + optional `fsDeps` param) to `downloadCommentImages()` — removed `mock.module("fs/promises")` from the image-downloader test itself
2. `49eedd5`: Removed all image-downloader mocking from forgejo-data-fetcher.test.ts entirely — the real function is safe to call because test comments have no GitHub image URLs, so it returns an empty Map naturally. Both `mock.module` and `spyOn` caused cross-file pollution in Bun's single-process runner.
- **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`
- [x] **M2: Test PR review comment trigger**
- 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.
- [x] **M3: Test PR review trigger**
- 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.
- [x] **M4: Test label trigger**
- Created `claude` label (id: 125) on the repo ✓
- Applied label to test issues (#17, #18, #19) ✓
- Verify: trigger detection, context fetching (issue body/title), Claude response ✓
- Document any issues and fix them ✓ (3 bugs found and fixed, see below)
- **Validation**: Claude responds when the configured label is applied to an issue ✓ (run 52795, 20s, 2 turns, $0.12)
- Also verified: creating issue with label in one go fires exactly 1 run (issue #19)
- **Bugs found & fixed**:
1. `a864b16`: Workflow `if:` expression parser rejects `#` comments — Forgejo runner's expression lexer treats `#` as invalid token. Moved all comments outside the `if:` block, switched from `|` to `>-` scalar.
2. `414c705`→`a864b16`: Forgejo does not populate `github.event.action` in workflow `if:` context for `issues` events (run 12061 skipped despite action being `labeled`). Let all `issues` events through the gate; `trigger.ts` validates.
3. `baa007a`: Forgejo sends `label_updated` instead of `labeled` as `eventAction` for issue label events. Added `FORGEJO_EVENT_ACTION_MAP` in `context.ts` to normalize at parse time (same pattern as event name normalization).
- **Key learning**: Forgejo's `issues` event action names differ from GitHub — `label_updated` not `labeled`. Normalize in `context.ts` at parse time so all downstream code works unchanged. Creating an issue with a label fires both `opened` and `label_updated` as separate events — do NOT check labels on `opened` to avoid duplicate runs.
- [x] **M5: Test assignee trigger**
- Created test issue #20, assigned `claude` user via API ✓
- Verify: trigger detection, context fetching, Claude response ✓
- Document any issues and fix them ✓ (1 bug found and fixed, see below)
- **Validation**: Claude responds when assigned to an issue ✓ (run 52842, 2 turns, $0.09)
- **Bug found & fixed**:
1. `99624ef`: Forgejo's `issues/assigned` webhook payload lacks the top-level `assignee` object — `payload.assignee` is `undefined`. The assignee is only available at `payload.issue.assignee`. Added fallback in `trigger.ts` to check `issue.assignee.login` when `payload.assignee.login` is missing.
- **Also discovered**: Forgejo sends `assigned` as the event action (matching GitHub) — no normalization needed, unlike M4's `label_updated`.
- **Also discovered**: Forgejo's API `PATCH /issues/{index}` with `{"assignees": ["user"]}` (plural array) returns 404. Must use deprecated singular `{"assignee": "user"}` field instead.
- **Key learning**: On Forgejo, the `issues/assigned` webhook payload structure differs from GitHub — the assigned user is nested under `issue.assignee`, not at the top-level `assignee` field. Same pattern as M2's missing top-level `comment` object.
- [x] **M6: Test tag mode with code changes (branch creation)**
- Created test issue #21 requesting creation of `test/e2e-prd6-m6.txt` ✓
- Triggered Claude via `@claude` mention in issue comment ✓
- Verify: Claude creates a branch, pushes commits, links the branch in tracking comment ✓
- Branch: `claude/issue-21-20260226-0917`
- Commit: `59f7e4e` by `claude <claude@wxs.ro>` (bot identity correct)
- File content verified via API: matches requested content
- Verify the Forgejo-specific PR compare URL format is correct ✓ (after fix, see below)
- **Validation**: Claude successfully creates a branch with code changes on Forgejo ✓ (run #198, 41s)
- **Bug found & fixed**:
1. `f08b1b4`: Prompt template in `src/create-prompt/index.ts` (lines 604, 794) hardcoded `quick_pull=1` (GitHub-specific) in the "Create a PR" URL. On Forgejo, this param is ignored so the compare page doesn't auto-expand the PR creation form. Forgejo uses `expand=1`. Fix: made prompt template platform-aware via `detectPlatform()`. Note: `updateForgejoCommentBody()` extracts PR URLs from comment content (`prLinkFromContent`) with priority over the URL from `update-comment-link.ts`, so the prompt template is the correct fix location.
- **Key learning**: The PR compare URL appears in two places — the prompt template (used by Claude to write the URL into comment body) and `update-comment-link.ts` (post-execution update). Because `updateForgejoCommentBody()` prioritizes URLs found in existing content, both must use the correct platform-specific query parameter. Fix validated after merge to `main` (workflow runs from default branch for `issue_comment` events).
- [x] **M7: Update documentation with E2E results**
- Updated `docs/forgejo.md` E2E status table with all 5 validated trigger scenarios ✓
- Removed "Not Yet E2E Tested" section entirely — all items validated ✓
- Added new "Webhook & Event Differences" section with 12-row comparison table documenting all Forgejo payload/event divergences ✓
- Added 4 new Known Limitations entries (dead `pull_request_review` trigger, review comments as issue comments, unreliable workflow `if:`, assignees API) ✓
- Added 5 new Troubleshooting entries (review reply tracking, `if:` skipping, `#` parse errors, duplicate label fires, `mock.module()` pollution) ✓
- Expanded "Tag Mode Verified Capabilities" from 6 to 10 bullet points ✓
- **Validation**: `docs/forgejo.md` accurately reflects the current E2E validation status ✓
## Dependencies
- PRD #5 (Dedicated Forgejo User) is recommended but not strictly required — tests can run with the current token setup
- A test repository on git.wxs.ro with the action workflow configured
## 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 |
| Workflow if: cannot gate on payload body for Forgejo | RESOLVED: @claude check moved to trigger.ts application code |
| Workflow if: expression parser rejects # comments | RESOLVED: moved comments outside if: block, use >- scalar |
| Forgejo event action names differ from GitHub | RESOLVED: FORGEJO_EVENT_ACTION_MAP normalizes at parse time |
| Creating issue with label fires duplicate runs | RESOLVED: label check only on labeled event, not on opened |
| Forgejo assigned payload lacks top-level assignee | RESOLVED: fallback to issue.assignee.login in trigger.ts |
| Forgejo API assignees array field returns 404 | RESOLVED: use deprecated singular assignee field instead |
| Prompt template hardcodes quick_pull=1 in PR compare URL | RESOLVED: platform-aware expand=1 for Forgejo in prompt |
## Success Criteria
- All 5 previously untested scenarios have been exercised on a live Forgejo instance
- Any bugs discovered are fixed and tested
- `docs/forgejo.md` E2E status table is fully up-to-date
- "Not Yet E2E Tested" section is empty (all items moved to validated or documented as unsupported)