diff --git a/.forgejo/workflows/claude.yml b/.forgejo/workflows/claude.yml index 7dadd18..4429059 100644 --- a/.forgejo/workflows/claude.yml +++ b/.forgejo/workflows/claude.yml @@ -26,7 +26,13 @@ jobs: # despite action being 'labeled'). Let all through; trigger.ts validates. if: >- (github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude')) || + # Forgejo collapses both pull_request_review_comment AND pull_request_review + # into pull_request_comment. We cannot gate on @claude here because Forgejo's + # webhook payload does not populate github.event.comment.body or + # github.event.review.body for these events (tested: run 12084 was skipped). + # The @claude check is handled in trigger.ts which reads the actual payload. (github.event_name == 'pull_request_review_comment' || github.event_name == 'pull_request_comment') || + # This condition only fires on GitHub (Forgejo never sends pull_request_review). (github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude')) || (github.event_name == 'issues') runs-on: grunt diff --git a/docs/forgejo.md b/docs/forgejo.md index 6d35ffc..f02dc1b 100644 --- a/docs/forgejo.md +++ b/docs/forgejo.md @@ -113,6 +113,27 @@ On Forgejo, there is no OIDC token exchange with Anthropic. The action uses the | **Permission check** | `getCollaboratorPermissionLevel()` | `repos.get()` fallback (workflow tokens lack collaborator endpoint) | | **Git ref lookup** | `git.getRef()` (`/git/ref/heads/...`) | `repos.getBranch()` (`/git/refs` is plural on Forgejo) | +### Webhook & Event Differences + +Forgejo's webhook payloads and event naming diverge from GitHub in several ways. These are handled automatically by the action but important to understand when debugging: + +| Behavior | GitHub | Forgejo | +| ------------------------------------ | -------------------------------------------------------- | -------------------------------------------------------------------------- | +| **PR review comment event name** | `pull_request_review_comment` | `pull_request_comment` (same event for both review comments and reviews) | +| **PR review event name** | `pull_request_review` | `pull_request_comment` (merged with review comments) | +| **Issue label event action** | `labeled` | `label_updated` (normalized by `FORGEJO_EVENT_ACTION_MAP` in `context.ts`) | +| **Issue assigned event action** | `assigned` | `assigned` (matches GitHub — no normalization needed) | +| **Review comment payload** | Top-level `comment` object present | No top-level `comment` — tracking comment falls back to issue comment | +| **Assigned payload** | Top-level `assignee` object present | No top-level `assignee` — must read from `issue.assignee` | +| **PR compare URL parameter** | `quick_pull=1` | `expand=1` | +| **Workflow `if:` body fields** | `github.event.comment.body` populated | Not populated for `pull_request_comment` events | +| **Workflow `if:` action field** | `github.event.action` populated for `issues` events | Not populated — `if:` cannot gate on action | +| **Workflow `if:` expression parser** | Allows `#` comments | Rejects `#` as invalid token — comments must go outside `if:` block | +| **Creating issue with label** | Fires `opened` only | Fires both `opened` and `label_updated` as separate events | +| **Assignees API** | `PATCH /issues/{id}` with `{"assignees": [...]}` (array) | Array form returns 404 — must use singular `{"assignee": "user"}` | + +**Practical impact**: The `@claude` mention check and event action validation happen in application code (`trigger.ts`, `context.ts`) rather than the workflow `if:` condition. The workflow lets all events through and the action decides what to act on. + ### Bun Installation The upstream action uses `oven-sh/setup-bun`. On Forgejo, this is replaced with a universal `curl` install because Forgejo runners pre-fetch all `uses:` action references before evaluating `if:` conditions, causing failures for GitHub-hosted actions that aren't mirrored. @@ -292,29 +313,30 @@ New upstream features that use GraphQL or GitHub-specific APIs won't automatical ## E2E Test Results -Both execution modes have been validated end-to-end on a Forgejo instance (git.wxs.ro): +All execution modes and trigger types have been validated end-to-end on a Forgejo instance (git.wxs.ro): -| Mode | Trigger | Status | Details | -| -------------- | --------------------------- | --------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| **Agent mode** | `workflow_dispatch` | Validated | Platform detection, token auth, Claude execution, output file — all green | -| **Tag mode** | `issue_comment` (`@claude`) | Validated | Issue context (title + body) read correctly, platform detected as `forgejo`, tracking comment created with spinner then updated with response (31s), job link URL correct | +| Mode | Trigger | Status | Details | +| -------------- | ------------------------------- | --------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Agent mode** | `workflow_dispatch` | Validated | Platform detection, token auth, Claude execution, output file — all green | +| **Tag mode** | `issue_comment` (`@claude`) | Validated | Issue context (title + body) read correctly, platform detected as `forgejo`, tracking comment created with spinner then updated with response (31s), job link URL correct | +| **Tag mode** | `pull_request_review_comment` | Validated | PR diff and review comment body visible to Claude. Tracking comment created as issue comment (not review reply — see [webhook quirks](#webhook--event-differences) below). 3 bugs fixed: missing top-level `comment` in payload, event name mismatch, MCP comment routing | +| **Tag mode** | `pull_request_review` | Validated | Claude responds to `@claude` in PR review body. Forgejo sends `pull_request_comment` instead of `pull_request_review` — the `@claude` check runs in `trigger.ts`, not the workflow `if:` condition | +| **Tag mode** | `issues` with `labeled` action | Validated | Claude responds when the configured label is applied. 3 bugs fixed: workflow `if:` expression parser rejects `#` comments, `github.event.action` not populated in `if:` context, event action is `label_updated` not `labeled` | +| **Tag mode** | `issues` with `assigned` action | Validated | Claude responds when assigned to an issue. 1 bug fixed: `assignee` field missing from top-level payload (nested under `issue.assignee` instead) | +| **Tag mode** | Code changes (branch + commits) | Validated | Claude creates branch, pushes commits, links branch in tracking comment. Bot identity (`claude `) correct. PR compare URL uses `expand=1` (Forgejo) instead of `quick_pull=1` (GitHub) | ### Tag Mode Verified Capabilities -- `@claude` trigger phrase detected in issue comments -- Issue body and title fully visible to Claude +- `@claude` trigger phrase detected in issue comments, PR review comments, and PR reviews +- Issue body, title, PR diff, and review comment body fully visible to Claude - Tracking comment created with progress spinner, then updated with final response - Forgejo job run URL correctly formatted in tracking comment - Branch creation works via `repos.getBranch()` API (Forgejo uses `/git/refs` not `/git/ref`) - Permission check passes via `repos.get()` fallback - -### Not Yet E2E Tested - -- PR review comment triggers (`pull_request_review_comment`) -- PR review triggers (`pull_request_review`) -- Label triggers (`issues` with `labeled` action) -- Assignee triggers (`issues` with `assigned` action) -- Branch creation + code changes from tag mode (issue asked a question, no code changes needed) +- Label trigger fires when configured label is applied to an issue +- Assignee trigger fires when the bot user is assigned to an issue +- Code changes: branch creation, file commits, and PR compare URL all work correctly +- Bot identity (commits attributed to `claude` user) verified via git log and API ## Known Limitations @@ -325,6 +347,10 @@ Both execution modes have been validated end-to-end on a Forgejo instance (git.w - **Commit signing**: SSH signing has not been tested on Forgejo. The `use_commit_signing` option (GitHub API signing) won't work since it uses GitHub's commit API. - **`downloadJobLogForWorkflowRun`**: Forgejo's Actions API may lack this endpoint. The CI MCP server attempts it via Octokit REST but may return limited data. - **Bot filtering**: `allowed_bots` checks won't match `claude[bot]` since Forgejo has no bot accounts. Actor validation is skipped entirely on Forgejo. +- **`pull_request_review` trigger is dead code**: Forgejo sends `pull_request_comment` for both review comments and review submissions. The `pull_request_review` trigger in the workflow never fires — all review-related events are handled through the `pull_request_comment` codepath. +- **Review comment tracking comments are always issue comments**: On Forgejo, review comment events lack the top-level `comment` object needed to create PR review comment replies. The tracking comment is always created as a regular issue comment instead. +- **Workflow `if:` conditions are unreliable for Forgejo events**: Forgejo does not populate `github.event.comment.body`, `github.event.review.body`, or `github.event.action` in the workflow expression context for several event types. All trigger validation happens in application code (`trigger.ts`), not in workflow conditions. +- **Assignees API (plural array form)**: `PATCH /repos/{owner}/{repo}/issues/{index}` with `{"assignees": ["user"]}` returns 404 on Forgejo. Use the deprecated singular `{"assignee": "user"}` field instead. ## Troubleshooting @@ -343,6 +369,26 @@ Unit tests assume GitHub context by default. When running on a Forgejo runner, s If `curl -fsSL https://bun.sh/install | bash` fails on your runner, pre-install Bun on the runner image or use the `path_to_bun_executable` input to point to a custom Bun binary. +### Claude's tracking comment doesn't appear as a review reply + +On Forgejo, review comment triggers (`pull_request_review_comment`) always create a regular issue comment — not a reply in the review thread. This is because Forgejo's webhook payload lacks the top-level `comment` object needed to create a review reply. The response still appears on the PR, just as a standalone comment. + +### Workflow `if:` condition skips events it shouldn't + +Forgejo doesn't populate `github.event.comment.body`, `github.event.review.body`, or `github.event.action` in the workflow expression context for `pull_request_comment` and `issues` events. If your workflow `if:` condition checks these fields, events will be silently skipped. The fix is to let all events through and handle validation in `trigger.ts` instead. + +### Workflow `if:` expression fails with "invalid token" error + +Forgejo's expression parser rejects `#` characters as invalid tokens. Move any comments outside the `if:` block and use `>-` scalar folding instead of `|` for multi-line expressions. + +### Label trigger fires twice when creating issue with label + +If you create an issue with a label in one operation, Forgejo fires both `opened` and `label_updated` as separate events. The action handles this correctly — it only checks labels on `label_updated` events, not on `opened`. + +### `mock.module()` breaks tests on Forgejo CI (Linux Bun) + +Bun's `mock.module()` replaces the module globally in the process module registry. With Bun's non-deterministic test file ordering on Linux, this causes cross-file pollution: one test file's mock leaks into another. Use dependency injection or `spyOn` on individual functions instead. If the real function is safe to call (e.g., returns empty results for test data), avoid mocking entirely. + ### Upstream workflows run and fail If `.github/workflows/` from upstream are executing on your Forgejo runner, configure the runner to only read from `.forgejo/workflows/`. This is a runner-level setting — see your Forgejo instance's runner configuration. diff --git a/prds/6-e2e-test-forgejo-triggers.md b/prds/done/6-e2e-test-forgejo-triggers.md similarity index 82% rename from prds/6-e2e-test-forgejo-triggers.md rename to prds/done/6-e2e-test-forgejo-triggers.md index 45e7186..7553e6c 100644 --- a/prds/6-e2e-test-forgejo-triggers.md +++ b/prds/done/6-e2e-test-forgejo-triggers.md @@ -91,20 +91,28 @@ Systematically exercise each untested scenario on the live Forgejo instance (git - **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. -- [ ] **M6: Test tag mode with code changes (branch creation)** +- [x] **M6: Test tag mode with code changes (branch creation)** - - Create an issue requesting a code change (e.g., "add a comment to file X") - - Trigger Claude via `@claude` mention - - Verify: Claude creates a branch, pushes commits, links the branch in tracking comment - - Verify the Forgejo-specific PR compare URL format is correct - - **Validation**: Claude successfully creates a branch with code changes on Forgejo + - 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 ` (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). -- [ ] **M7: Update documentation with E2E results** - - Update `docs/forgejo.md` E2E status table with results for all 5 scenarios - - Move tested items from "Not Yet E2E Tested" to the validated table - - Document any Forgejo-specific quirks or limitations discovered - - Add troubleshooting entries for any issues encountered - - **Validation**: `docs/forgejo.md` accurately reflects the current E2E validation status +- [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 @@ -126,6 +134,7 @@ Systematically exercise each untested scenario on the live Forgejo instance (git | 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 diff --git a/src/create-prompt/index.ts b/src/create-prompt/index.ts index e25683f..46c6d05 100644 --- a/src/create-prompt/index.ts +++ b/src/create-prompt/index.ts @@ -601,7 +601,7 @@ ${ eventData.claudeBranch ? ` When done with changes, provide a PR link: -[Create a PR](${GITHUB_SERVER_URL}/${context.repository}/compare/${eventData.baseBranch}...${eventData.claudeBranch}?quick_pull=1&title=&body=) +[Create a PR](${GITHUB_SERVER_URL}/${context.repository}/compare/${eventData.baseBranch}...${eventData.claudeBranch}?${detectPlatform() === "forgejo" ? "expand=1" : "quick_pull=1"}&title=&body=) Use THREE dots (...) between branches. URL-encode all parameters.` : "" } @@ -791,7 +791,7 @@ ${eventData.eventName === "issue_comment" || eventData.eventName === "pull_reque ${ eventData.claudeBranch ? `- Provide a URL to create a PR manually in this format: - [Create a PR](${GITHUB_SERVER_URL}/${context.repository}/compare/${eventData.baseBranch}...?quick_pull=1&title=&body=) + [Create a PR](${GITHUB_SERVER_URL}/${context.repository}/compare/${eventData.baseBranch}...?${detectPlatform() === "forgejo" ? "expand=1" : "quick_pull=1"}&title=&body=) - IMPORTANT: Use THREE dots (...) between branch names, not two (..) Example: ${GITHUB_SERVER_URL}/${context.repository}/compare/main...feature-branch (correct) NOT: ${GITHUB_SERVER_URL}/${context.repository}/compare/main..feature-branch (incorrect)