Merge pull request 'feat(prd-6): E2E test and fix all untested Forgejo trigger scenarios' (#15) from feature/prd-6-m2-test-pr-review-comment into main
This commit is contained in:
commit
eee75baa45
4 files changed
+90
-29
No files matched your search
@@ -26,7 +26,13 @@ jobs:
|
|||||||
# despite action being 'labeled'). Let all through; trigger.ts validates.
|
# despite action being 'labeled'). Let all through; trigger.ts validates.
|
||||||
if: >-
|
if: >-
|
||||||
(github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude')) ||
|
(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') ||
|
(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 == 'pull_request_review' && contains(github.event.review.body, '@claude')) ||
|
||||||
(github.event_name == 'issues')
|
(github.event_name == 'issues')
|
||||||
runs-on: grunt
|
runs-on: grunt
|
||||||
|
|||||||
+61
-15
@@ -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) |
|
| **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) |
|
| **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
|
### 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.
|
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
|
## 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 |
|
| Mode | Trigger | Status | Details |
|
||||||
| -------------- | --------------------------- | --------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
| -------------- | ------------------------------- | --------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||||
| **Agent mode** | `workflow_dispatch` | Validated | Platform detection, token auth, Claude execution, output file — all green |
|
| **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** | `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 <claude@wxs.ro>`) correct. PR compare URL uses `expand=1` (Forgejo) instead of `quick_pull=1` (GitHub) |
|
||||||
|
|
||||||
### Tag Mode Verified Capabilities
|
### Tag Mode Verified Capabilities
|
||||||
|
|
||||||
- `@claude` trigger phrase detected in issue comments
|
- `@claude` trigger phrase detected in issue comments, PR review comments, and PR reviews
|
||||||
- Issue body and title fully visible to Claude
|
- Issue body, title, PR diff, and review comment body fully visible to Claude
|
||||||
- Tracking comment created with progress spinner, then updated with final response
|
- Tracking comment created with progress spinner, then updated with final response
|
||||||
- Forgejo job run URL correctly formatted in tracking comment
|
- Forgejo job run URL correctly formatted in tracking comment
|
||||||
- Branch creation works via `repos.getBranch()` API (Forgejo uses `/git/refs` not `/git/ref`)
|
- Branch creation works via `repos.getBranch()` API (Forgejo uses `/git/refs` not `/git/ref`)
|
||||||
- Permission check passes via `repos.get()` fallback
|
- Permission check passes via `repos.get()` fallback
|
||||||
|
- Label trigger fires when configured label is applied to an issue
|
||||||
### Not Yet E2E Tested
|
- 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
|
||||||
- PR review comment triggers (`pull_request_review_comment`)
|
- Bot identity (commits attributed to `claude` user) verified via git log and API
|
||||||
- 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)
|
|
||||||
|
|
||||||
## Known Limitations
|
## 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.
|
- **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.
|
- **`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.
|
- **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
|
## 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.
|
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
|
### 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.
|
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.
|
||||||
@@ -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.
|
- **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.
|
- **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")
|
- Created test issue #21 requesting creation of `test/e2e-prd6-m6.txt` ✓
|
||||||
- Trigger Claude via `@claude` mention
|
- Triggered Claude via `@claude` mention in issue comment ✓
|
||||||
- Verify: Claude creates a branch, pushes commits, links the branch in tracking comment
|
- Verify: Claude creates a branch, pushes commits, links the branch in tracking comment ✓
|
||||||
- Verify the Forgejo-specific PR compare URL format is correct
|
- Branch: `claude/issue-21-20260226-0917`
|
||||||
- **Validation**: Claude successfully creates a branch with code changes on Forgejo
|
- 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).
|
||||||
|
|
||||||
- [ ] **M7: Update documentation with E2E results**
|
- [x] **M7: Update documentation with E2E results**
|
||||||
- Update `docs/forgejo.md` E2E status table with results for all 5 scenarios
|
- Updated `docs/forgejo.md` E2E status table with all 5 validated trigger scenarios ✓
|
||||||
- Move tested items from "Not Yet E2E Tested" to the validated table
|
- Removed "Not Yet E2E Tested" section entirely — all items validated ✓
|
||||||
- Document any Forgejo-specific quirks or limitations discovered
|
- Added new "Webhook & Event Differences" section with 12-row comparison table documenting all Forgejo payload/event divergences ✓
|
||||||
- Add troubleshooting entries for any issues encountered
|
- Added 4 new Known Limitations entries (dead `pull_request_review` trigger, review comments as issue comments, unreliable workflow `if:`, assignees API) ✓
|
||||||
- **Validation**: `docs/forgejo.md` accurately reflects the current E2E validation status
|
- 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
|
## 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 |
|
| 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 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 |
|
| 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
|
## Success Criteria
|
||||||
|
|
||||||
@@ -601,7 +601,7 @@ ${
|
|||||||
eventData.claudeBranch
|
eventData.claudeBranch
|
||||||
? `
|
? `
|
||||||
When done with changes, provide a PR link:
|
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=<url-encoded-title>&body=<url-encoded-body>)
|
[Create a PR](${GITHUB_SERVER_URL}/${context.repository}/compare/${eventData.baseBranch}...${eventData.claudeBranch}?${detectPlatform() === "forgejo" ? "expand=1" : "quick_pull=1"}&title=<url-encoded-title>&body=<url-encoded-body>)
|
||||||
Use THREE dots (...) between branches. URL-encode all parameters.`
|
Use THREE dots (...) between branches. URL-encode all parameters.`
|
||||||
: ""
|
: ""
|
||||||
}
|
}
|
||||||
@@ -791,7 +791,7 @@ ${eventData.eventName === "issue_comment" || eventData.eventName === "pull_reque
|
|||||||
${
|
${
|
||||||
eventData.claudeBranch
|
eventData.claudeBranch
|
||||||
? `- Provide a URL to create a PR manually in this format:
|
? `- Provide a URL to create a PR manually in this format:
|
||||||
[Create a PR](${GITHUB_SERVER_URL}/${context.repository}/compare/${eventData.baseBranch}...<branch-name>?quick_pull=1&title=<url-encoded-title>&body=<url-encoded-body>)
|
[Create a PR](${GITHUB_SERVER_URL}/${context.repository}/compare/${eventData.baseBranch}...<branch-name>?${detectPlatform() === "forgejo" ? "expand=1" : "quick_pull=1"}&title=<url-encoded-title>&body=<url-encoded-body>)
|
||||||
- IMPORTANT: Use THREE dots (...) between branch names, not two (..)
|
- IMPORTANT: Use THREE dots (...) between branch names, not two (..)
|
||||||
Example: ${GITHUB_SERVER_URL}/${context.repository}/compare/main...feature-branch (correct)
|
Example: ${GITHUB_SERVER_URL}/${context.repository}/compare/main...feature-branch (correct)
|
||||||
NOT: ${GITHUB_SERVER_URL}/${context.repository}/compare/main..feature-branch (incorrect)
|
NOT: ${GITHUB_SERVER_URL}/${context.repository}/compare/main..feature-branch (incorrect)
|
||||||
|
|||||||
Reference in new issue
Block a user