Merge pull request 'fix(prd-7): Senior developer review of Forgejo platform support' (#24) from feature/prd-7-senior-review-forgejo into main
This commit is contained in:
Vlad Mocanu committed
commit
01c9c503c3
29 files changed
+1459
-374
No files matched your search
@@ -40,7 +40,7 @@ Single entrypoint: `src/entrypoints/run.ts` orchestrates everything — prepare
|
||||
|
||||
This fork adds Forgejo support via a lean adapter pattern — only 4 genuinely different areas have Forgejo-specific code:
|
||||
|
||||
- **`src/platform/detector.ts`** — Three-tier platform detection (explicit input > `FORGEJO` env > `GITHUB_SERVER_URL` heuristic)
|
||||
- **`src/platform/detector.ts`** — Three-tier platform detection (explicit input > `FORGEJO` env > `GITHUB_SERVER_URL` heuristic) with cross-validation that rejects Forgejo override on github.com
|
||||
- **`src/forgejo/token.ts`** — Direct token handling (skips OIDC exchange)
|
||||
- **`src/forgejo/data/fetcher.ts`** — REST-based data fetching (replaces 3 GraphQL queries with 5-8 parallel REST calls)
|
||||
- **`src/forgejo/comment-logic.ts`** — Forgejo URL patterns for tracking comments
|
||||
@@ -54,9 +54,10 @@ All other `src/github/` modules work on Forgejo via Octokit REST as-is (permissi
|
||||
- `src/entrypoints/run.ts` — Platform branch at top for token setup
|
||||
- `src/modes/tag/index.ts` — Data fetcher swap (`fetchForgejoData` vs `fetchGitHubData`)
|
||||
- `src/github/operations/comments/create-initial.ts` — Platform-aware spinner/URL format
|
||||
- `src/github/operations/comments/update-with-branch.ts` — Forgejo PR compare URL
|
||||
- `src/github/operations/comments/update-with-branch.ts` — Forgejo branch link in tracking comment
|
||||
- `src/github/operations/branch.ts` — Uses `repos.getBranch()` on Forgejo (Forgejo has `/git/refs` not `/git/ref`)
|
||||
- `src/github/validation/actor.ts` — Skipped on Forgejo (no bot accounts)
|
||||
- `src/github/validation/actor.ts` — Bot-type check skipped on Forgejo; self-trigger guard active
|
||||
- `src/entrypoints/update-comment-link.ts` — Forgejo PR compare URL (`expand=1` instead of `quick_pull=1`)
|
||||
- `src/mcp/install-mcp-server.ts` — Skips GitHub public MCP server on Forgejo
|
||||
|
||||
**MCP server names stay the same** (`github_comment`, `github_ci`, etc.) even on Forgejo — changing them would require updating hardcoded tool name lists in `src/modes/tag/index.ts`.
|
||||
|
||||
+19
-16
@@ -88,6 +88,10 @@ The action detects the platform using a three-tier strategy:
|
||||
|
||||
**Recommendation**: Always set `platform: "forgejo"` explicitly. Auto-detection works but the explicit input eliminates ambiguity.
|
||||
|
||||
> **Security note**: Setting `platform: "forgejo"` on a `github.com` runner is rejected — the action cross-validates the `GITHUB_SERVER_URL` and falls through to standard detection. This prevents bypassing GitHub-specific security checks (e.g., actor validation).
|
||||
|
||||
> **Note for GitHub Enterprise Server (GHES) users**: The tier-3 heuristic classifies any non-`github.com` URL as Forgejo. If you run on GHES (e.g., `https://github.mycompany.com`), you **must** set `platform: "github"` explicitly to avoid misdetection.
|
||||
|
||||
## Differences from GitHub
|
||||
|
||||
### Authentication
|
||||
@@ -103,15 +107,15 @@ On Forgejo, there is no OIDC token exchange with Anthropic. The action uses the
|
||||
|
||||
### API Differences
|
||||
|
||||
| Feature | GitHub | Forgejo |
|
||||
| --------------------- | ------------------------------------------- | ------------------------------------------------------------------- |
|
||||
| **GraphQL API** | Used for PR/issue data fetching | Not available — REST API only |
|
||||
| **Data fetching** | Single GraphQL query per entity | 5-8 parallel REST API calls |
|
||||
| **Actor validation** | Checks for bot/app accounts | Skipped — no bot accounts on Forgejo |
|
||||
| **GitHub MCP server** | `ghcr.io/github/github-mcp-server` included | Skipped — GitHub-specific |
|
||||
| **CI/Actions MCP** | Full GitHub Actions API | Works via Octokit REST + `GITHUB_API_URL` |
|
||||
| **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) |
|
||||
| Feature | GitHub | Forgejo |
|
||||
| --------------------- | ------------------------------------------- | --------------------------------------------------------------------- |
|
||||
| **GraphQL API** | Used for PR/issue data fetching | Not available — REST API only |
|
||||
| **Data fetching** | Single GraphQL query per entity | 5-8 parallel REST API calls |
|
||||
| **Actor validation** | Checks for bot/app accounts | Bot-type check skipped; self-trigger guard active |
|
||||
| **GitHub MCP server** | `ghcr.io/github/github-mcp-server` included | Skipped — GitHub-specific |
|
||||
| **CI/Actions MCP** | Full GitHub Actions API | Works via Octokit REST + `GITHUB_API_URL` |
|
||||
| **Permission check** | `getCollaboratorPermissionLevel()` | `getCollaboratorPermissionLevel()` with `repos.get()` fallback on 403 |
|
||||
| **Git ref lookup** | `git.getRef()` (`/git/ref/heads/...`) | `repos.getBranch()` (`/git/refs` is plural on Forgejo) |
|
||||
|
||||
### Webhook & Event Differences
|
||||
|
||||
@@ -255,12 +259,10 @@ Runs on push to `main` and pull requests. Three parallel jobs:
|
||||
|
||||
### `test-action.yml` — Action Self-Test
|
||||
|
||||
Tests the action itself on Forgejo in agent mode. Runs weekly and on PRs that modify `src/`, `action.yml`, or `.forgejo/`. Validates:
|
||||
Tests the action itself on Forgejo in agent mode. Runs weekly and on PRs that modify `src/`, `action.yml`, `.forgejo/`, or `base-action/`. Validates:
|
||||
|
||||
- Platform detection returns "forgejo"
|
||||
- Token authentication works
|
||||
- Claude Code executes successfully
|
||||
- Output file is produced
|
||||
- Claude Code executes successfully with `platform: "forgejo"`
|
||||
- Output file is produced with expected content
|
||||
|
||||
### `upstream-sync.yml` — Upstream Sync Check
|
||||
|
||||
@@ -340,13 +342,14 @@ All execution modes and trigger types have been validated end-to-end on a Forgej
|
||||
|
||||
## Known Limitations
|
||||
|
||||
- **GitHub Enterprise Server (GHES) misdetection**: The tier-3 platform detection heuristic treats any `GITHUB_SERVER_URL` that isn't `github.com` as Forgejo. GHES instances (e.g., `https://github.mycompany.com`) will be misidentified. Set `platform: "github"` explicitly when running on GHES.
|
||||
- **Git ref API**: Forgejo uses `/git/refs` (plural) instead of GitHub's `/git/ref` (singular). The action handles this by using `repos.getBranch()` on Forgejo for branch SHA lookups.
|
||||
- **User display name fetch**: `GET /users/{username}` returns 403 without `read:user` scope. Non-fatal — falls back to login name.
|
||||
- **`pull_request_target` event**: May not exist in Forgejo. The action degrades gracefully (maps to `pull_request`).
|
||||
- **GitHub MCP server**: The `ghcr.io/github/github-mcp-server` Docker image is GitHub-specific and is skipped on Forgejo.
|
||||
- **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.
|
||||
- **Bot filtering**: `allowed_bots` checks won't match `claude[bot]` since Forgejo has no bot accounts. Bot-type actor validation is skipped on Forgejo, but self-trigger prevention is active — the action blocks events where the actor matches the configured `bot_name` to prevent infinite loops.
|
||||
- **`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.
|
||||
@@ -379,7 +382,7 @@ Forgejo doesn't populate `github.event.comment.body`, `github.event.review.body`
|
||||
|
||||
### 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.
|
||||
Forgejo's expression parser may reject `#` characters as invalid tokens when using `|` (literal block) scalars, since each line is preserved and `#` starts a comment that extends to end-of-line. With `>-` (folded scalar), newlines become spaces, and `#` comments may be tolerated by some Forgejo versions — but for maximum compatibility, move comments outside the `if:` block entirely.
|
||||
|
||||
### Label trigger fires twice when creating issue with label
|
||||
|
||||
|
||||
@@ -58,7 +58,7 @@ Perform a thorough, documented code review of all Forgejo-related changes. The r
|
||||
|
||||
## Milestones
|
||||
|
||||
- [ ] **M1: Architecture & design review**
|
||||
- [x] **M1: Architecture & design review**
|
||||
|
||||
- Review the lean adapter pattern: is it the right abstraction level?
|
||||
- Assess merge-conflict surface with upstream: are platform branches minimal?
|
||||
@@ -67,7 +67,7 @@ Perform a thorough, documented code review of all Forgejo-related changes. The r
|
||||
- Evaluate whether the `src/forgejo/` directory structure scales
|
||||
- **Deliverable**: Architecture findings document with severity ratings
|
||||
|
||||
- [ ] **M2: Error handling & edge case review**
|
||||
- [x] **M2: Error handling & edge case review**
|
||||
|
||||
- Review every `try/catch` block in Forgejo code paths
|
||||
- Check for unhandled promise rejections
|
||||
@@ -76,7 +76,7 @@ Perform a thorough, documented code review of all Forgejo-related changes. The r
|
||||
- Review fallback behavior (e.g., `repos.get()` fallback for permissions)
|
||||
- **Deliverable**: List of error handling gaps with fixes
|
||||
|
||||
- [ ] **M3: Security review**
|
||||
- [x] **M3: Security review**
|
||||
|
||||
- Token handling: is the token properly scoped and never logged?
|
||||
- Input sanitization: can webhook payloads inject content?
|
||||
@@ -84,7 +84,7 @@ Perform a thorough, documented code review of all Forgejo-related changes. The r
|
||||
- Secrets in logs: review all `core.info()` / `core.debug()` calls
|
||||
- **Deliverable**: Security findings with severity ratings
|
||||
|
||||
- [ ] **M4: Test coverage review & improvements**
|
||||
- [x] **M4: Test coverage review & improvements**
|
||||
|
||||
- Assess existing unit test coverage for Forgejo code paths
|
||||
- Identify critical paths without test coverage
|
||||
@@ -92,7 +92,7 @@ Perform a thorough, documented code review of all Forgejo-related changes. The r
|
||||
- Verify mocks accurately represent Forgejo API responses
|
||||
- **Deliverable**: Test coverage improved, gaps documented
|
||||
|
||||
- [ ] **M5: Code quality & maintainability review**
|
||||
- [x] **M5: Code quality & maintainability review**
|
||||
|
||||
- TypeScript strictness: are types accurate or using `any` / type assertions?
|
||||
- Naming conventions: consistent with the rest of the codebase?
|
||||
@@ -101,7 +101,7 @@ Perform a thorough, documented code review of all Forgejo-related changes. The r
|
||||
- Code comments: are complex decisions documented?
|
||||
- **Deliverable**: Code quality fixes applied
|
||||
|
||||
- [ ] **M6: Documentation accuracy review**
|
||||
- [x] **M6: Documentation accuracy review**
|
||||
- Cross-check `docs/forgejo.md` against actual code behavior
|
||||
- Verify all "Known Limitations" are still accurate
|
||||
- Check that code examples and configuration snippets work
|
||||
@@ -1006,7 +1006,6 @@ export async function createPrompt(
|
||||
core.exportVariable("ALLOWED_TOOLS", allAllowedTools);
|
||||
core.exportVariable("DISALLOWED_TOOLS", allDisallowedTools);
|
||||
} catch (error) {
|
||||
core.setFailed(`Create prompt failed with error: ${error}`);
|
||||
process.exit(1);
|
||||
throw new Error(`Create prompt failed with error: ${error}`);
|
||||
}
|
||||
}
|
||||
@@ -8,7 +8,6 @@ import {
|
||||
type CommentUpdateInput,
|
||||
} from "../github/operations/comment-logic";
|
||||
import {
|
||||
updateForgejoCommentBody,
|
||||
createForgejoPrCompareUrl,
|
||||
getForgejoRunIndex,
|
||||
} from "../forgejo/comment-logic";
|
||||
@@ -19,7 +18,7 @@ import {
|
||||
isEntityContext,
|
||||
} from "../github/context";
|
||||
import type { ParsedGitHubContext } from "../github/context";
|
||||
import { GITHUB_SERVER_URL } from "../github/api/config";
|
||||
import { getServerUrl } from "../github/api/config";
|
||||
import { checkAndCommitOrDeleteBranch } from "../github/operations/branch-cleanup";
|
||||
import { updateClaudeComment } from "../github/operations/comments/update-claude-comment";
|
||||
|
||||
@@ -54,7 +53,7 @@ export async function updateCommentLink(
|
||||
const { owner, repo } = context.repository;
|
||||
const platform = detectPlatform();
|
||||
|
||||
const serverUrl = GITHUB_SERVER_URL;
|
||||
const serverUrl = getServerUrl();
|
||||
const runIdentifier =
|
||||
platform === "forgejo" ? getForgejoRunIndex() : process.env.GITHUB_RUN_ID;
|
||||
const jobUrl = `${serverUrl}/${owner}/${repo}/actions/runs/${runIdentifier}`;
|
||||
@@ -250,10 +249,7 @@ export async function updateCommentLink(
|
||||
errorDetails,
|
||||
};
|
||||
|
||||
const updatedBody =
|
||||
platform === "forgejo"
|
||||
? updateForgejoCommentBody(commentInput)
|
||||
: updateCommentBody(commentInput);
|
||||
const updatedBody = updateCommentBody(commentInput);
|
||||
|
||||
try {
|
||||
await updateClaudeComment(octokit.rest, {
|
||||
|
||||
+13
-215
@@ -1,38 +1,16 @@
|
||||
/**
|
||||
* Forgejo-specific comment URL formatting and body construction.
|
||||
*
|
||||
* Mirrors the GitHub versions in:
|
||||
* - src/github/operations/comments/common.ts (spinner, link builders, initial comment)
|
||||
* - src/github/operations/comment-logic.ts (updateCommentBody for final comment)
|
||||
*
|
||||
* Differences from GitHub:
|
||||
* Contains Forgejo-specific helpers for:
|
||||
* - Unicode spinner instead of GitHub CDN image (CDN URLs don't resolve on Forgejo)
|
||||
* - PR compare URL omits `quick_pull=1` (GitHub-specific parameter)
|
||||
* - Dynamic server URL extraction instead of hardcoded github.com regex
|
||||
*/
|
||||
|
||||
import { ensureProperlyEncodedUrl } from "../github/operations/comment-logic";
|
||||
import type { CommentUpdateInput } from "../github/operations/comment-logic";
|
||||
|
||||
// Re-export types so consumers don't need to import from two places
|
||||
export type {
|
||||
CommentUpdateInput,
|
||||
ExecutionDetails,
|
||||
} from "../github/operations/comment-logic";
|
||||
export { ensureProperlyEncodedUrl } from "../github/operations/comment-logic";
|
||||
|
||||
/**
|
||||
* Read the server URL at call time (not module load time).
|
||||
* - PR compare URL with `expand=1` instead of GitHub's `quick_pull=1`
|
||||
* - Forgejo run index (GITHUB_RUN_NUMBER vs GITHUB_RUN_ID)
|
||||
*
|
||||
* The GitHub module uses a top-level constant from config.ts, which is fine
|
||||
* when GITHUB_SERVER_URL doesn't change. For Forgejo we read from env at
|
||||
* call time so that:
|
||||
* 1. Tests can override the env var and see the effect immediately.
|
||||
* 2. The same module works regardless of import order.
|
||||
* The final comment body logic (updateCommentBody) is unified in
|
||||
* src/github/operations/comment-logic.ts and works for both platforms.
|
||||
*/
|
||||
function getServerUrl(): string {
|
||||
return process.env.GITHUB_SERVER_URL || "https://github.com";
|
||||
}
|
||||
|
||||
import { getServerUrl } from "../github/api/config";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Run index helper
|
||||
@@ -66,6 +44,12 @@ export const FORGEJO_SPINNER_HTML = "⏳";
|
||||
// URL Builders
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
// Note: owner, repo, runId, and branchName are NOT URI-encoded in path
|
||||
// segments because: (1) owner/repo are validated by Forgejo (alphanumeric +
|
||||
// .-_), (2) branchName is validated by validateBranchName() which restricts
|
||||
// to [a-zA-Z0-9._/-], and (3) encoding "/" in branch names breaks Forgejo
|
||||
// URLs (e.g., "claude/fix-bug" → "claude%2Ffix-bug" is invalid).
|
||||
|
||||
export function createForgejoJobRunLink(
|
||||
owner: string,
|
||||
repo: string,
|
||||
@@ -118,189 +102,3 @@ I'll analyze this and get back to you.
|
||||
|
||||
${jobRunLink}${branchLink}`;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Owner/repo extraction helper
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Extract owner/repo from a URL using the configured server URL.
|
||||
*
|
||||
* Unlike the GitHub version (which hardcodes `github.com`), this dynamically
|
||||
* matches against any server URL — works for both GitHub and Forgejo.
|
||||
*/
|
||||
export function extractOwnerRepoFromUrl(
|
||||
url: string,
|
||||
serverUrl: string = getServerUrl(),
|
||||
): { owner: string; repo: string } | null {
|
||||
try {
|
||||
const parsedServer = new URL(serverUrl);
|
||||
const parsedUrl = new URL(url);
|
||||
|
||||
// Host must match (handles both github.com and git.wxs.ro etc.)
|
||||
if (parsedUrl.host !== parsedServer.host) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Extract first two path segments as owner/repo
|
||||
const segments = parsedUrl.pathname.split("/").filter(Boolean);
|
||||
const owner = segments[0];
|
||||
const repo = segments[1];
|
||||
if (owner && repo) {
|
||||
return { owner, repo };
|
||||
}
|
||||
} catch {
|
||||
// Invalid URLs — fall through to null
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Final comment body (shown after Claude finishes)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Forgejo version of updateCommentBody.
|
||||
*
|
||||
* Identical logic to src/github/operations/comment-logic.ts:updateCommentBody
|
||||
* with one critical fix: uses extractOwnerRepoFromUrl() for dynamic server URL
|
||||
* extraction instead of the hardcoded `github.com` regex.
|
||||
*/
|
||||
export function updateForgejoCommentBody(input: CommentUpdateInput): string {
|
||||
const originalBody = input.currentBody;
|
||||
const {
|
||||
executionDetails,
|
||||
jobUrl,
|
||||
branchLink,
|
||||
prLink,
|
||||
actionFailed,
|
||||
branchName,
|
||||
triggerUsername,
|
||||
errorDetails,
|
||||
} = input;
|
||||
|
||||
// Extract content from the original comment body
|
||||
// First, remove the "Claude Code is working…" or "Claude Code is working..." message
|
||||
const workingPattern = /Claude Code is working[…\.]{1,3}(?:\s*<img[^>]*>)?/i;
|
||||
let bodyContent = originalBody.replace(workingPattern, "").trim();
|
||||
|
||||
// Also remove the Unicode spinner used on Forgejo
|
||||
bodyContent = bodyContent.replace(/⏳/g, "").trim();
|
||||
|
||||
// Check if there's a PR link in the content
|
||||
let prLinkFromContent = "";
|
||||
|
||||
// Match the entire markdown link structure
|
||||
const prLinkPattern = /\[Create .* PR\]\((.*)\)$/m;
|
||||
const prLinkMatch = bodyContent.match(prLinkPattern);
|
||||
|
||||
if (prLinkMatch && prLinkMatch[1]) {
|
||||
const encodedUrl = ensureProperlyEncodedUrl(prLinkMatch[1]);
|
||||
if (encodedUrl) {
|
||||
prLinkFromContent = encodedUrl;
|
||||
// Remove the PR link from the content
|
||||
bodyContent = bodyContent.replace(prLinkMatch[0], "").trim();
|
||||
}
|
||||
}
|
||||
|
||||
// Calculate duration string if available
|
||||
let durationStr = "";
|
||||
if (executionDetails?.duration_ms !== undefined) {
|
||||
const totalSeconds = Math.round(executionDetails.duration_ms / 1000);
|
||||
const minutes = Math.floor(totalSeconds / 60);
|
||||
const seconds = totalSeconds % 60;
|
||||
durationStr = minutes > 0 ? `${minutes}m ${seconds}s` : `${seconds}s`;
|
||||
}
|
||||
|
||||
// Build the header
|
||||
let header = "";
|
||||
|
||||
if (actionFailed) {
|
||||
header = "**Claude encountered an error";
|
||||
if (durationStr) {
|
||||
header += ` after ${durationStr}`;
|
||||
}
|
||||
header += "**";
|
||||
} else {
|
||||
// Get the username from triggerUsername or extract from content
|
||||
const usernameMatch = bodyContent.match(/@([a-zA-Z0-9-]+)/);
|
||||
const username =
|
||||
triggerUsername || (usernameMatch ? usernameMatch[1] : "user");
|
||||
|
||||
header = `**Claude finished @${username}'s task`;
|
||||
if (durationStr) {
|
||||
header += ` in ${durationStr}`;
|
||||
}
|
||||
header += "**";
|
||||
}
|
||||
|
||||
// Add links section
|
||||
let links = ` —— [View job](${jobUrl})`;
|
||||
|
||||
// Add branch name with link
|
||||
if (branchName || branchLink) {
|
||||
let finalBranchName = branchName;
|
||||
let branchUrl = "";
|
||||
|
||||
if (branchLink) {
|
||||
// Extract the branch URL from the link
|
||||
const urlMatch = branchLink.match(/\((https?:\/\/.*)\)/);
|
||||
if (urlMatch && urlMatch[1]) {
|
||||
branchUrl = urlMatch[1];
|
||||
}
|
||||
|
||||
// Extract branch name from link if not provided
|
||||
if (!finalBranchName) {
|
||||
const branchNameMatch = branchLink.match(/tree\/([^"'\)]+)/);
|
||||
if (branchNameMatch) {
|
||||
finalBranchName = branchNameMatch[1];
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// If we don't have a URL yet but have a branch name, construct it
|
||||
// FIX: Uses extractOwnerRepoFromUrl instead of hardcoded github.com regex
|
||||
if (!branchUrl && finalBranchName) {
|
||||
const extracted = extractOwnerRepoFromUrl(jobUrl);
|
||||
if (extracted) {
|
||||
branchUrl = `${getServerUrl()}/${extracted.owner}/${extracted.repo}/tree/${finalBranchName}`;
|
||||
}
|
||||
}
|
||||
|
||||
if (finalBranchName && branchUrl) {
|
||||
links += ` • [\`${finalBranchName}\`](${branchUrl})`;
|
||||
} else if (finalBranchName) {
|
||||
links += ` • \`${finalBranchName}\``;
|
||||
}
|
||||
}
|
||||
|
||||
// Add PR link (either from content or provided)
|
||||
const prUrl =
|
||||
prLinkFromContent || (prLink ? prLink.match(/\(([^)]+)\)/)?.[1] : "");
|
||||
if (prUrl) {
|
||||
links += ` • [Create PR ➔](${prUrl})`;
|
||||
}
|
||||
|
||||
// Build the new body with blank line between header and separator
|
||||
let newBody = `${header}${links}`;
|
||||
|
||||
// Add error details if available
|
||||
if (actionFailed && errorDetails) {
|
||||
newBody += `\n\n\`\`\`\n${errorDetails}\n\`\`\``;
|
||||
}
|
||||
|
||||
newBody += `\n\n---\n`;
|
||||
|
||||
// Clean up the body content
|
||||
// Remove any existing View job run, branch links from the bottom
|
||||
bodyContent = bodyContent.replace(/\n?\[View job run\]\([^\)]+\)/g, "");
|
||||
bodyContent = bodyContent.replace(/\n?\[View branch\]\([^\)]+\)/g, "");
|
||||
|
||||
// Remove any existing duration info at the bottom
|
||||
bodyContent = bodyContent.replace(/\n*---\n*Duration: [0-9]+m? [0-9]+s/g, "");
|
||||
|
||||
// Add the cleaned body content
|
||||
newBody += bodyContent;
|
||||
|
||||
return newBody.trim();
|
||||
}
|
||||
@@ -129,6 +129,7 @@ export function mapRestReviews(
|
||||
user?: { login?: string } | null;
|
||||
state: string;
|
||||
submitted_at?: string;
|
||||
updated_at?: string;
|
||||
}>,
|
||||
inlineComments: (GitHubReviewComment & { reviewId: number | null })[],
|
||||
): GitHubReview[] {
|
||||
@@ -151,7 +152,7 @@ export function mapRestReviews(
|
||||
body: r.body ?? "",
|
||||
state: r.state.toUpperCase(), // Normalize to uppercase for consistency
|
||||
submittedAt: r.submitted_at ?? new Date().toISOString(),
|
||||
updatedAt: r.submitted_at, // Forgejo has no separate updatedAt for reviews
|
||||
updatedAt: r.updated_at ?? r.submitted_at, // Use updated_at for TOCTOU filtering if available
|
||||
lastEditedAt: undefined,
|
||||
comments: {
|
||||
nodes: commentsByReview.get(r.id) ?? [],
|
||||
|
||||
@@ -2,3 +2,13 @@ export const GITHUB_API_URL =
|
||||
process.env.GITHUB_API_URL || "https://api.github.com";
|
||||
export const GITHUB_SERVER_URL =
|
||||
process.env.GITHUB_SERVER_URL || "https://github.com";
|
||||
|
||||
/**
|
||||
* Read the server URL at call time (not module load time).
|
||||
*
|
||||
* Use this instead of GITHUB_SERVER_URL in code paths where tests may
|
||||
* override process.env.GITHUB_SERVER_URL after module import.
|
||||
*/
|
||||
export function getServerUrl(): string {
|
||||
return process.env.GITHUB_SERVER_URL || "https://github.com";
|
||||
}
|
||||
@@ -1,5 +1,5 @@
|
||||
import type { Octokits } from "../api/client";
|
||||
import { GITHUB_SERVER_URL } from "../api/config";
|
||||
import { getServerUrl } from "../api/config";
|
||||
import { $ } from "bun";
|
||||
|
||||
export async function checkAndCommitOrDeleteBranch(
|
||||
@@ -80,7 +80,7 @@ export async function checkAndCommitOrDeleteBranch(
|
||||
);
|
||||
|
||||
// Set branch link since we now have commits
|
||||
const branchUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/tree/${claudeBranch}`;
|
||||
const branchUrl = `${getServerUrl()}/${owner}/${repo}/tree/${claudeBranch}`;
|
||||
branchLink = `\n[View branch](${branchUrl})`;
|
||||
} else {
|
||||
console.log(
|
||||
@@ -91,7 +91,7 @@ export async function checkAndCommitOrDeleteBranch(
|
||||
} catch (gitError) {
|
||||
console.error("Error checking/committing changes:", gitError);
|
||||
// If we can't check git status, assume the branch might have changes
|
||||
const branchUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/tree/${claudeBranch}`;
|
||||
const branchUrl = `${getServerUrl()}/${owner}/${repo}/tree/${claudeBranch}`;
|
||||
branchLink = `\n[View branch](${branchUrl})`;
|
||||
}
|
||||
} else {
|
||||
@@ -102,13 +102,13 @@ export async function checkAndCommitOrDeleteBranch(
|
||||
}
|
||||
} else {
|
||||
// Only add branch link if there are commits
|
||||
const branchUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/tree/${claudeBranch}`;
|
||||
const branchUrl = `${getServerUrl()}/${owner}/${repo}/tree/${claudeBranch}`;
|
||||
branchLink = `\n[View branch](${branchUrl})`;
|
||||
}
|
||||
} catch (error) {
|
||||
console.error("Error comparing commits on Claude branch:", error);
|
||||
// If we can't compare but the branch exists remotely, include the branch link
|
||||
const branchUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/tree/${claudeBranch}`;
|
||||
const branchUrl = `${getServerUrl()}/${owner}/${repo}/tree/${claudeBranch}`;
|
||||
branchLink = `\n[View branch](${branchUrl})`;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -309,6 +309,6 @@ export async function setupBranch(
|
||||
};
|
||||
} catch (error) {
|
||||
console.error("Error in branch setup:", error);
|
||||
process.exit(1);
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
@@ -1,4 +1,4 @@
|
||||
import { GITHUB_SERVER_URL } from "../api/config";
|
||||
import { getServerUrl } from "../api/config";
|
||||
|
||||
export type ExecutionDetails = {
|
||||
total_cost_usd?: number;
|
||||
@@ -66,6 +66,37 @@ export function ensureProperlyEncodedUrl(url: string): string | null {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract owner/repo from a URL using the configured server URL.
|
||||
*
|
||||
* Dynamically matches against any server URL — works for both GitHub and Forgejo.
|
||||
*/
|
||||
export function extractOwnerRepoFromUrl(
|
||||
url: string,
|
||||
serverUrl: string = getServerUrl(),
|
||||
): { owner: string; repo: string } | null {
|
||||
try {
|
||||
const parsedServer = new URL(serverUrl);
|
||||
const parsedUrl = new URL(url);
|
||||
|
||||
// Host must match (handles both github.com and custom domains)
|
||||
if (parsedUrl.host !== parsedServer.host) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Extract first two path segments as owner/repo
|
||||
const segments = parsedUrl.pathname.split("/").filter(Boolean);
|
||||
const owner = segments[0];
|
||||
const repo = segments[1];
|
||||
if (owner && repo) {
|
||||
return { owner, repo };
|
||||
}
|
||||
} catch {
|
||||
// Invalid URLs — fall through to null
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
export function updateCommentBody(input: CommentUpdateInput): string {
|
||||
const originalBody = input.currentBody;
|
||||
const {
|
||||
@@ -84,6 +115,9 @@ export function updateCommentBody(input: CommentUpdateInput): string {
|
||||
const workingPattern = /Claude Code is working[…\.]{1,3}(?:\s*<img[^>]*>)?/i;
|
||||
let bodyContent = originalBody.replace(workingPattern, "").trim();
|
||||
|
||||
// Also remove the Unicode spinner used on Forgejo (safe no-op on GitHub)
|
||||
bodyContent = bodyContent.replace(/⏳/g, "").trim();
|
||||
|
||||
// Check if there's a PR link in the content
|
||||
let prLinkFromContent = "";
|
||||
|
||||
@@ -141,7 +175,7 @@ export function updateCommentBody(input: CommentUpdateInput): string {
|
||||
|
||||
if (branchLink) {
|
||||
// Extract the branch URL from the link
|
||||
const urlMatch = branchLink.match(/\((https:\/\/.*)\)/);
|
||||
const urlMatch = branchLink.match(/\((https?:\/\/.*)\)/);
|
||||
if (urlMatch && urlMatch[1]) {
|
||||
branchUrl = urlMatch[1];
|
||||
}
|
||||
@@ -156,11 +190,11 @@ export function updateCommentBody(input: CommentUpdateInput): string {
|
||||
}
|
||||
|
||||
// If we don't have a URL yet but have a branch name, construct it
|
||||
// Uses dynamic URL extraction that works for both GitHub and Forgejo
|
||||
if (!branchUrl && finalBranchName) {
|
||||
// Extract owner/repo from jobUrl
|
||||
const repoMatch = jobUrl.match(/github\.com\/([^\/]+)\/([^\/]+)\//);
|
||||
if (repoMatch) {
|
||||
branchUrl = `${GITHUB_SERVER_URL}/${repoMatch[1]}/${repoMatch[2]}/tree/${finalBranchName}`;
|
||||
const extracted = extractOwnerRepoFromUrl(jobUrl);
|
||||
if (extracted) {
|
||||
branchUrl = `${getServerUrl()}/${extracted.owner}/${extracted.repo}/tree/${finalBranchName}`;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -56,9 +56,14 @@ export async function createInitialComment(
|
||||
const botNameMatch =
|
||||
comment.user?.type === "Bot" &&
|
||||
comment.user?.login.toLowerCase().includes("claude");
|
||||
// On Forgejo, the bot is a regular user account identified by bot_name input
|
||||
const forgejoBotMatch =
|
||||
platform === "forgejo" &&
|
||||
context.inputs.botName &&
|
||||
comment.user?.login === context.inputs.botName;
|
||||
const bodyMatch = comment.body === initialBody;
|
||||
|
||||
return idMatch || botNameMatch || bodyMatch;
|
||||
return idMatch || botNameMatch || forgejoBotMatch || bodyMatch;
|
||||
});
|
||||
if (existingComment) {
|
||||
response = await octokit.rest.issues.updateComment({
|
||||
|
||||
@@ -14,6 +14,7 @@ import {
|
||||
createForgejoJobRunLink,
|
||||
createForgejoBranchLink,
|
||||
createForgejoCommentBody,
|
||||
getForgejoRunIndex,
|
||||
} from "../../../forgejo/comment-logic";
|
||||
import { detectPlatform } from "../../../platform/detector";
|
||||
import { type Octokits } from "../../api/client";
|
||||
@@ -34,7 +35,7 @@ export async function updateTrackingComment(
|
||||
|
||||
const jobRunLink =
|
||||
platform === "forgejo"
|
||||
? createForgejoJobRunLink(owner, repo, context.runId)
|
||||
? createForgejoJobRunLink(owner, repo, getForgejoRunIndex())
|
||||
: createJobRunLink(owner, repo, context.runId);
|
||||
|
||||
// Add branch link for issues (not PRs)
|
||||
|
||||
@@ -14,10 +14,20 @@ export async function checkHumanActor(
|
||||
githubContext: GitHubContext,
|
||||
) {
|
||||
// Forgejo doesn't have GitHub-style bot/app accounts, and the user API
|
||||
// requires read:user scope which may not be available. Skip entirely.
|
||||
// requires read:user scope which may not be available. Skip the bot-type
|
||||
// check but still guard against self-triggering (infinite loop prevention).
|
||||
if (detectPlatform() === "forgejo") {
|
||||
const botName = githubContext.inputs.botName.replace(/\[bot\]$/, "");
|
||||
if (
|
||||
githubContext.actor === botName ||
|
||||
githubContext.actor === githubContext.inputs.botName
|
||||
) {
|
||||
throw new Error(
|
||||
`Blocking self-trigger on Forgejo: actor '${githubContext.actor}' matches bot_name '${githubContext.inputs.botName}'. This prevents infinite loops.`,
|
||||
);
|
||||
}
|
||||
console.log(
|
||||
`Skipping human actor check on Forgejo for: ${githubContext.actor}`,
|
||||
`Forgejo actor check passed for: ${githubContext.actor} (bot_name: ${githubContext.inputs.botName})`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -83,16 +83,51 @@ export async function checkWritePermissions(
|
||||
}
|
||||
|
||||
/**
|
||||
* Forgejo permission check using the repo endpoint.
|
||||
* GET /repos/{owner}/{repo} returns a `permissions` object for the
|
||||
* authenticated token with { admin, push, pull } booleans. We check
|
||||
* that the token has push (write) access.
|
||||
* Forgejo permission check using the collaborator permission endpoint.
|
||||
*
|
||||
* Primary: GET /repos/{owner}/{repo}/collaborators/{username}/permission
|
||||
* Returns { "permission": "admin"|"write"|"read"|"none" } — checks the
|
||||
* actor's permission, not the token's.
|
||||
*
|
||||
* Fallback: GET /repos/{owner}/{repo} — checks the token's permission
|
||||
* (less precise but works if the collaborator endpoint returns 403).
|
||||
*/
|
||||
async function checkForgejoPermissions(
|
||||
octokit: Octokit,
|
||||
repository: { owner: string; repo: string },
|
||||
actor: string,
|
||||
): Promise<boolean> {
|
||||
// Try actor-level permission check first (requires repo admin or token with
|
||||
// sufficient scope to read collaborators).
|
||||
try {
|
||||
const response = await octokit.repos.getCollaboratorPermissionLevel({
|
||||
owner: repository.owner,
|
||||
repo: repository.repo,
|
||||
username: actor,
|
||||
});
|
||||
|
||||
const permissionLevel = response.data.permission;
|
||||
core.info(`Forgejo actor '${actor}' permission level: ${permissionLevel}`);
|
||||
|
||||
if (permissionLevel === "admin" || permissionLevel === "write") {
|
||||
return true;
|
||||
}
|
||||
|
||||
core.warning(
|
||||
`Forgejo actor '${actor}' has insufficient permissions: ${permissionLevel}`,
|
||||
);
|
||||
return false;
|
||||
} catch (error) {
|
||||
// Forgejo may return 403 if the token lacks admin scope. Fall back to
|
||||
// token-level check and log a warning about the reduced security.
|
||||
core.warning(
|
||||
`Forgejo collaborator permission check failed for '${actor}' (${error instanceof Error ? error.message : String(error)}). ` +
|
||||
`Falling back to token-level permission check. ` +
|
||||
`For actor-level checks, use a token with read:organization scope.`,
|
||||
);
|
||||
}
|
||||
|
||||
// Fallback: check the token's own permission on the repo
|
||||
const response = await octokit.repos.get({
|
||||
owner: repository.owner,
|
||||
repo: repository.repo,
|
||||
@@ -111,7 +146,7 @@ async function checkForgejoPermissions(
|
||||
|
||||
if (permissions.admin || permissions.push) {
|
||||
core.info(
|
||||
`Forgejo token has write access (admin=${permissions.admin}, push=${permissions.push}) for actor: ${actor}`,
|
||||
`Forgejo token has write access (admin=${permissions.admin}, push=${permissions.push}) for actor: ${actor} (token-level fallback)`,
|
||||
);
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -145,18 +145,20 @@ export function checkContainsTrigger(context: ParsedGitHubContext): boolean {
|
||||
return true;
|
||||
}
|
||||
|
||||
// On Forgejo, if no body was found in the payload at all, still trigger —
|
||||
// the actual comment data will be fetched via REST in the data fetcher.
|
||||
// This handles edge cases where Forgejo strips body from the webhook payload.
|
||||
// On Forgejo, pull_request_review_comment payloads may arrive with an
|
||||
// empty body. Rather than triggering unconditionally (which bypasses the
|
||||
// trigger phrase check), we skip. The user can re-trigger with a comment
|
||||
// that includes the trigger phrase. This prevents Claude from responding
|
||||
// to comments not directed at it.
|
||||
if (
|
||||
isPullRequestReviewCommentEvent(context) &&
|
||||
detectPlatform() === "forgejo" &&
|
||||
!commentBody
|
||||
) {
|
||||
console.log(
|
||||
"Forgejo pull_request_review_comment: no body in payload, triggering (data fetched via REST)",
|
||||
"Forgejo pull_request_review_comment: no body in payload, skipping (cannot verify trigger phrase)",
|
||||
);
|
||||
return true;
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -241,7 +241,6 @@ export async function prepareMcpConfig(
|
||||
// User's config will be passed as separate --mcp-config flags
|
||||
return JSON.stringify(baseMcpConfig, null, 2);
|
||||
} catch (error) {
|
||||
core.setFailed(`Install MCP server failed with error: ${error}`);
|
||||
process.exit(1);
|
||||
throw new Error(`Install MCP server failed with error: ${error}`);
|
||||
}
|
||||
}
|
||||
@@ -6,24 +6,57 @@
|
||||
* 2. FORGEJO env var (set by Forgejo runner v7.0+)
|
||||
* 3. GITHUB_SERVER_URL heuristic (non-github.com = Forgejo)
|
||||
* 4. Default to GitHub
|
||||
*
|
||||
* The result is memoized after first detection — platform doesn't change
|
||||
* at runtime and this avoids redundant env reads across 12+ call sites.
|
||||
*/
|
||||
|
||||
export type Platform = "github" | "forgejo";
|
||||
|
||||
let _cachedPlatform: Platform | undefined;
|
||||
|
||||
export function detectPlatform(): Platform {
|
||||
if (_cachedPlatform !== undefined) return _cachedPlatform;
|
||||
|
||||
let result: Platform = "github";
|
||||
|
||||
// 1. Explicit platform from action.yml detect step or user input
|
||||
const platformEnv = process.env.PLATFORM;
|
||||
if (platformEnv === "forgejo") return "forgejo";
|
||||
if (platformEnv === "github") return "github";
|
||||
|
||||
if (platformEnv === "forgejo") {
|
||||
// Cross-validate: prevent forcing Forgejo mode on GitHub.com, which would
|
||||
// skip actor validation and use a weaker permission model.
|
||||
const serverUrl = process.env.GITHUB_SERVER_URL || "";
|
||||
if (serverUrl.includes("github.com")) {
|
||||
console.warn(
|
||||
`Platform override 'forgejo' ignored: GITHUB_SERVER_URL (${serverUrl}) indicates GitHub.com. ` +
|
||||
`Falling through to standard detection.`,
|
||||
);
|
||||
} else {
|
||||
result = "forgejo";
|
||||
}
|
||||
} else if (platformEnv === "github") {
|
||||
result = "github";
|
||||
}
|
||||
// 2. FORGEJO env var (set by Forgejo runner v7.0+)
|
||||
if (process.env.FORGEJO === "true") return "forgejo";
|
||||
|
||||
else if (process.env.FORGEJO === "true") {
|
||||
result = "forgejo";
|
||||
}
|
||||
// 3. GITHUB_SERVER_URL heuristic — if not github.com, likely Forgejo/Gitea
|
||||
const serverUrl = process.env.GITHUB_SERVER_URL || "";
|
||||
if (serverUrl && !serverUrl.includes("github.com")) {
|
||||
return "forgejo";
|
||||
else {
|
||||
const serverUrl = process.env.GITHUB_SERVER_URL || "";
|
||||
if (serverUrl && !serverUrl.includes("github.com")) {
|
||||
result = "forgejo";
|
||||
}
|
||||
}
|
||||
|
||||
return "github";
|
||||
_cachedPlatform = result;
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Reset the memoized platform detection result.
|
||||
* Only needed in tests that change platform-related env vars between cases.
|
||||
*/
|
||||
export function _resetPlatformCache(): void {
|
||||
_cachedPlatform = undefined;
|
||||
}
|
||||
@@ -0,0 +1,51 @@
|
||||
# Test Coverage Notes — Forgejo Platform Support
|
||||
|
||||
## Well Covered (unit tested)
|
||||
|
||||
| Module | Tests | File |
|
||||
| -------------------------------- | --------------- | ------------------------------------ |
|
||||
| `src/forgejo/token.ts` | 8 tests | `test/forgejo-token.test.ts` |
|
||||
| `src/platform/detector.ts` | 17 tests | `test/platform-detector.test.ts` |
|
||||
| `src/forgejo/comment-logic.ts` | 30+ tests | `test/forgejo-comment-logic.test.ts` |
|
||||
| `src/forgejo/data/fetcher.ts` | 50+ tests | `test/forgejo-data-fetcher.test.ts` |
|
||||
| `src/github/validation/actor.ts` | 6 Forgejo tests | `test/actor.test.ts` |
|
||||
| `src/mcp/install-mcp-server.ts` | 8 Forgejo tests | `test/install-mcp-server.test.ts` |
|
||||
|
||||
## Added in M4
|
||||
|
||||
| Module | Tests | File |
|
||||
| ------------------------------------------------------ | -------- | -------------------------------------- |
|
||||
| `src/github/operations/comments/create-initial.ts` | 15 tests | `test/create-initial-comment.test.ts` |
|
||||
| `src/github/operations/comments/update-with-branch.ts` | 11 tests | `test/update-tracking-comment.test.ts` |
|
||||
| `src/github/operations/branch.ts` (API divergence) | 2 tests | `test/setup-branch-forgejo.test.ts` |
|
||||
|
||||
## Accepted Gaps (orchestrators)
|
||||
|
||||
### `src/modes/tag/index.ts` — `prepareTagMode()`
|
||||
|
||||
**Why not unit tested**: Calls 8+ modules (checkHumanActor, createInitialComment,
|
||||
fetchForgejoData/fetchGitHubData, setupBranch, configureGitAuth, createPrompt,
|
||||
prepareMcpConfig). Each is individually unit tested.
|
||||
|
||||
**Risk**: Low. The Forgejo branch is a simple `if (platform === "forgejo")`
|
||||
choosing `fetchForgejoData` vs `fetchGitHubData`. Both are thoroughly tested.
|
||||
|
||||
**Coverage path**: Integration tests in the separate `install-test` repo.
|
||||
|
||||
### `src/entrypoints/run.ts` — main orchestrator
|
||||
|
||||
**Why not unit tested**: Token setup path is a trivial conditional (1 line). Testing
|
||||
requires mocking the entire execution environment (GitHub Actions context, CLI
|
||||
installation, process lifecycle).
|
||||
|
||||
**Risk**: Very low. The Forgejo token path calls `setupForgejoToken()` which
|
||||
has 8 dedicated tests.
|
||||
|
||||
**Coverage path**: Integration tests in the separate `install-test` repo.
|
||||
|
||||
## Known Issue: CLAUDE_APP_BOT_ID Discrepancy
|
||||
|
||||
`src/github/operations/comments/create-initial.ts` (line 23) defines a local
|
||||
`CLAUDE_APP_BOT_ID = 209825114`, while `src/github/constants.ts` exports
|
||||
`CLAUDE_APP_BOT_ID = 41898282`. These are different values with the same name.
|
||||
This should be investigated in M5 (code quality review).
|
||||
+40
-2
@@ -2,6 +2,7 @@
|
||||
|
||||
import { describe, test, expect, beforeEach, afterEach } from "bun:test";
|
||||
import { checkHumanActor } from "../src/github/validation/actor";
|
||||
import { _resetPlatformCache } from "../src/platform/detector";
|
||||
import type { Octokit } from "@octokit/rest";
|
||||
import { createMockContext } from "./mockContext";
|
||||
|
||||
@@ -116,9 +117,12 @@ describe("checkHumanActor", () => {
|
||||
let savedServerUrl: string | undefined;
|
||||
|
||||
beforeEach(() => {
|
||||
_resetPlatformCache();
|
||||
savedPlatform = process.env.PLATFORM;
|
||||
savedServerUrl = process.env.GITHUB_SERVER_URL;
|
||||
process.env.PLATFORM = "forgejo";
|
||||
// Ensure cross-validation doesn't reject PLATFORM=forgejo
|
||||
process.env.GITHUB_SERVER_URL = "https://git.example.com";
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -127,9 +131,10 @@ describe("checkHumanActor", () => {
|
||||
if (savedServerUrl !== undefined)
|
||||
process.env.GITHUB_SERVER_URL = savedServerUrl;
|
||||
else delete process.env.GITHUB_SERVER_URL;
|
||||
_resetPlatformCache();
|
||||
});
|
||||
|
||||
test("should skip actor check entirely on Forgejo", async () => {
|
||||
test("should skip GitHub API actor check on Forgejo for regular users", async () => {
|
||||
let apiCalled = false;
|
||||
const mockOctokit = {
|
||||
users: {
|
||||
@@ -148,7 +153,7 @@ describe("checkHumanActor", () => {
|
||||
expect(apiCalled).toBe(false);
|
||||
});
|
||||
|
||||
test("should skip actor check for bot-like names on Forgejo", async () => {
|
||||
test("should skip actor check for bot-like names that don't match bot_name", async () => {
|
||||
let apiCalled = false;
|
||||
const mockOctokit = {
|
||||
users: {
|
||||
@@ -167,5 +172,38 @@ describe("checkHumanActor", () => {
|
||||
).resolves.toBeUndefined();
|
||||
expect(apiCalled).toBe(false);
|
||||
});
|
||||
|
||||
test("should block self-trigger when actor matches bot_name (with [bot] suffix)", async () => {
|
||||
const mockOctokit = createMockOctokit("User");
|
||||
const context = createMockContext();
|
||||
// Default botName is "claude[bot]" from CLAUDE_BOT_LOGIN
|
||||
context.actor = "claude[bot]";
|
||||
|
||||
await expect(checkHumanActor(mockOctokit, context)).rejects.toThrow(
|
||||
"Blocking self-trigger on Forgejo",
|
||||
);
|
||||
});
|
||||
|
||||
test("should block self-trigger when actor matches bot_name without [bot] suffix", async () => {
|
||||
const mockOctokit = createMockOctokit("User");
|
||||
const context = createMockContext();
|
||||
context.inputs.botName = "claude[bot]";
|
||||
context.actor = "claude";
|
||||
|
||||
await expect(checkHumanActor(mockOctokit, context)).rejects.toThrow(
|
||||
"Blocking self-trigger on Forgejo",
|
||||
);
|
||||
});
|
||||
|
||||
test("should block self-trigger with custom bot_name", async () => {
|
||||
const mockOctokit = createMockOctokit("User");
|
||||
const context = createMockContext();
|
||||
context.inputs.botName = "my-assistant";
|
||||
context.actor = "my-assistant";
|
||||
|
||||
await expect(checkHumanActor(mockOctokit, context)).rejects.toThrow(
|
||||
"Blocking self-trigger on Forgejo",
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,527 @@
|
||||
#!/usr/bin/env bun
|
||||
|
||||
import {
|
||||
describe,
|
||||
test,
|
||||
expect,
|
||||
beforeEach,
|
||||
afterEach,
|
||||
spyOn,
|
||||
type Mock,
|
||||
} from "bun:test";
|
||||
import { tmpdir } from "os";
|
||||
import { join } from "path";
|
||||
import { readFileSync, unlinkSync, writeFileSync } from "fs";
|
||||
import { createInitialComment } from "../src/github/operations/comments/create-initial";
|
||||
import { _resetPlatformCache } from "../src/platform/detector";
|
||||
import { createMockContext } from "./mockContext";
|
||||
import type { Octokit } from "@octokit/rest";
|
||||
|
||||
// Note: create-initial.ts defines its own CLAUDE_APP_BOT_ID = 209825114 (line 23),
|
||||
// which differs from src/github/constants.ts CLAUDE_APP_BOT_ID = 41898282.
|
||||
// Tests use 209825114 to match the actual code in create-initial.ts.
|
||||
const LOCAL_CLAUDE_APP_BOT_ID = 209825114;
|
||||
|
||||
function createMockOctokit(overrides: Record<string, any> = {}) {
|
||||
// createInitialComment receives an Octokit instance and accesses octokit.rest.issues.*
|
||||
return {
|
||||
rest: {
|
||||
issues: {
|
||||
listComments: overrides.listComments ?? (async () => ({ data: [] })),
|
||||
createComment:
|
||||
overrides.createComment ?? (async () => ({ data: { id: 100 } })),
|
||||
updateComment:
|
||||
overrides.updateComment ?? (async () => ({ data: { id: 200 } })),
|
||||
},
|
||||
pulls: {
|
||||
createReplyForReviewComment:
|
||||
overrides.createReplyForReviewComment ??
|
||||
(async () => ({ data: { id: 300 } })),
|
||||
},
|
||||
},
|
||||
} as unknown as Octokit;
|
||||
}
|
||||
|
||||
describe("createInitialComment", () => {
|
||||
let savedPlatform: string | undefined;
|
||||
let savedServerUrl: string | undefined;
|
||||
let savedRunNumber: string | undefined;
|
||||
let savedRunId: string | undefined;
|
||||
let savedGithubOutput: string | undefined;
|
||||
let tempOutputFile: string;
|
||||
let consoleLogSpy: Mock<(...args: any[]) => void>;
|
||||
let consoleErrorSpy: Mock<(...args: any[]) => void>;
|
||||
|
||||
beforeEach(() => {
|
||||
_resetPlatformCache();
|
||||
savedPlatform = process.env.PLATFORM;
|
||||
savedServerUrl = process.env.GITHUB_SERVER_URL;
|
||||
savedRunNumber = process.env.GITHUB_RUN_NUMBER;
|
||||
savedRunId = process.env.GITHUB_RUN_ID;
|
||||
savedGithubOutput = process.env.GITHUB_OUTPUT;
|
||||
|
||||
// Create a real temp file for GITHUB_OUTPUT
|
||||
tempOutputFile = join(
|
||||
tmpdir(),
|
||||
`test-gh-output-${Date.now()}-${Math.random().toString(36).slice(2)}`,
|
||||
);
|
||||
writeFileSync(tempOutputFile, "");
|
||||
process.env.GITHUB_OUTPUT = tempOutputFile;
|
||||
|
||||
// Default to GitHub platform
|
||||
delete process.env.PLATFORM;
|
||||
process.env.GITHUB_SERVER_URL = "https://github.com";
|
||||
process.env.GITHUB_RUN_ID = "9876543";
|
||||
|
||||
consoleLogSpy = spyOn(console, "log").mockImplementation(() => {});
|
||||
consoleErrorSpy = spyOn(console, "error").mockImplementation(() => {});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (savedPlatform !== undefined) process.env.PLATFORM = savedPlatform;
|
||||
else delete process.env.PLATFORM;
|
||||
if (savedServerUrl !== undefined)
|
||||
process.env.GITHUB_SERVER_URL = savedServerUrl;
|
||||
else delete process.env.GITHUB_SERVER_URL;
|
||||
if (savedRunNumber !== undefined)
|
||||
process.env.GITHUB_RUN_NUMBER = savedRunNumber;
|
||||
else delete process.env.GITHUB_RUN_NUMBER;
|
||||
if (savedRunId !== undefined) process.env.GITHUB_RUN_ID = savedRunId;
|
||||
else delete process.env.GITHUB_RUN_ID;
|
||||
if (savedGithubOutput !== undefined)
|
||||
process.env.GITHUB_OUTPUT = savedGithubOutput;
|
||||
else delete process.env.GITHUB_OUTPUT;
|
||||
|
||||
try {
|
||||
unlinkSync(tempOutputFile);
|
||||
} catch {}
|
||||
|
||||
consoleLogSpy.mockRestore();
|
||||
consoleErrorSpy.mockRestore();
|
||||
_resetPlatformCache();
|
||||
});
|
||||
|
||||
describe("GitHub platform — basic comment creation", () => {
|
||||
test("creates issue comment for issue_comment event", async () => {
|
||||
const createComment = async () => ({ data: { id: 42 } });
|
||||
const createCommentSpy = spyOn({ createComment }, "createComment");
|
||||
const mockOctokit = createMockOctokit({
|
||||
createComment: createCommentSpy,
|
||||
});
|
||||
|
||||
const context = createMockContext({ entityNumber: 10 });
|
||||
const result = await createInitialComment(mockOctokit, context);
|
||||
|
||||
expect(result.id).toBe(42);
|
||||
expect(createCommentSpy).toHaveBeenCalled();
|
||||
const callArgs = (createCommentSpy.mock.calls as any[][])[0]![0];
|
||||
expect(callArgs.owner).toBe("test-owner");
|
||||
expect(callArgs.repo).toBe("test-repo");
|
||||
expect(callArgs.issue_number).toBe(10);
|
||||
// GitHub body uses CDN spinner image
|
||||
expect(callArgs.body).toContain("user-attachments/assets");
|
||||
expect(callArgs.body).toContain("Claude Code is working");
|
||||
});
|
||||
|
||||
test("writes comment ID to GITHUB_OUTPUT file", async () => {
|
||||
const mockOctokit = createMockOctokit({
|
||||
createComment: async () => ({ data: { id: 777 } }),
|
||||
});
|
||||
const context = createMockContext();
|
||||
|
||||
await createInitialComment(mockOctokit, context);
|
||||
|
||||
const output = readFileSync(tempOutputFile, "utf-8");
|
||||
expect(output).toContain("claude_comment_id=777");
|
||||
});
|
||||
|
||||
test("uses GitHub CDN spinner and actions/runs URL in body", async () => {
|
||||
let capturedBody = "";
|
||||
const mockOctokit = createMockOctokit({
|
||||
createComment: async (args: any) => {
|
||||
capturedBody = args.body;
|
||||
return { data: { id: 1 } };
|
||||
},
|
||||
});
|
||||
const context = createMockContext({ runId: "12345" });
|
||||
|
||||
await createInitialComment(mockOctokit, context);
|
||||
|
||||
// GitHub spinner is a CDN image
|
||||
expect(capturedBody).toContain("user-attachments/assets");
|
||||
expect(capturedBody).not.toContain("⏳");
|
||||
// GitHub URL uses GITHUB_SERVER_URL + actions/runs/<runId>
|
||||
expect(capturedBody).toContain(
|
||||
"https://github.com/test-owner/test-repo/actions/runs/12345",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("GitHub platform — sticky comment on PR", () => {
|
||||
test("creates new comment when no existing comment matches", async () => {
|
||||
const createComment = async () => ({ data: { id: 50 } });
|
||||
const updateComment = async () => ({ data: { id: 99 } });
|
||||
const createSpy = spyOn({ createComment }, "createComment");
|
||||
const updateSpy = spyOn({ updateComment }, "updateComment");
|
||||
const mockOctokit = createMockOctokit({
|
||||
listComments: async () => ({ data: [] }),
|
||||
createComment: createSpy,
|
||||
updateComment: updateSpy,
|
||||
});
|
||||
|
||||
const context = createMockContext({
|
||||
eventName: "pull_request",
|
||||
isPR: true,
|
||||
entityNumber: 5,
|
||||
inputs: { useStickyComment: true },
|
||||
payload: {
|
||||
action: "opened",
|
||||
number: 5,
|
||||
pull_request: { number: 5 },
|
||||
} as any,
|
||||
});
|
||||
|
||||
const result = await createInitialComment(mockOctokit, context);
|
||||
expect(result.id).toBe(50);
|
||||
expect(createSpy).toHaveBeenCalled();
|
||||
expect(updateSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test("updates existing comment matched by CLAUDE_APP_BOT_ID", async () => {
|
||||
const updateComment = async () => ({ data: { id: 200 } });
|
||||
const updateSpy = spyOn({ updateComment }, "updateComment");
|
||||
const createComment = async () => ({ data: { id: 100 } });
|
||||
const createSpy = spyOn({ createComment }, "createComment");
|
||||
const mockOctokit = createMockOctokit({
|
||||
listComments: async () => ({
|
||||
data: [
|
||||
{
|
||||
id: 200,
|
||||
user: {
|
||||
id: LOCAL_CLAUDE_APP_BOT_ID,
|
||||
type: "User",
|
||||
login: "someone",
|
||||
},
|
||||
body: "old body",
|
||||
},
|
||||
],
|
||||
}),
|
||||
createComment: createSpy,
|
||||
updateComment: updateSpy,
|
||||
});
|
||||
|
||||
const context = createMockContext({
|
||||
eventName: "pull_request",
|
||||
isPR: true,
|
||||
entityNumber: 5,
|
||||
inputs: { useStickyComment: true },
|
||||
payload: {
|
||||
action: "opened",
|
||||
number: 5,
|
||||
pull_request: { number: 5 },
|
||||
} as any,
|
||||
});
|
||||
|
||||
const result = await createInitialComment(mockOctokit, context);
|
||||
expect(result.id).toBe(200);
|
||||
expect(updateSpy).toHaveBeenCalled();
|
||||
expect(createSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test("updates existing comment matched by Bot type with 'claude' in login", async () => {
|
||||
const updateComment = async () => ({ data: { id: 201 } });
|
||||
const updateSpy = spyOn({ updateComment }, "updateComment");
|
||||
const mockOctokit = createMockOctokit({
|
||||
listComments: async () => ({
|
||||
data: [
|
||||
{
|
||||
id: 201,
|
||||
user: { id: 999, type: "Bot", login: "claude[bot]" },
|
||||
body: "old",
|
||||
},
|
||||
],
|
||||
}),
|
||||
updateComment: updateSpy,
|
||||
});
|
||||
|
||||
const context = createMockContext({
|
||||
eventName: "pull_request",
|
||||
isPR: true,
|
||||
inputs: { useStickyComment: true },
|
||||
payload: {
|
||||
action: "opened",
|
||||
number: 1,
|
||||
pull_request: { number: 1 },
|
||||
} as any,
|
||||
});
|
||||
|
||||
const result = await createInitialComment(mockOctokit, context);
|
||||
expect(result.id).toBe(201);
|
||||
expect(updateSpy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test("updates existing comment matched by exact body content", async () => {
|
||||
// To test body matching, we need the mock comment's body to exactly equal
|
||||
// the generated initialBody. Import the GitHub comment builder to construct
|
||||
// the matching body with the same runId as the mock context.
|
||||
const { createCommentBody, createJobRunLink } = await import(
|
||||
"../src/github/operations/comments/common"
|
||||
);
|
||||
const expectedBody = createCommentBody(
|
||||
createJobRunLink("test-owner", "test-repo", "1234567890"),
|
||||
);
|
||||
|
||||
const updateComment = async () => ({ data: { id: 202 } });
|
||||
const updateSpy = spyOn({ updateComment }, "updateComment");
|
||||
const mockOctokit = createMockOctokit({
|
||||
listComments: async () => ({
|
||||
data: [
|
||||
{
|
||||
id: 202,
|
||||
user: { id: 1, type: "User", login: "random-user" },
|
||||
body: expectedBody,
|
||||
},
|
||||
],
|
||||
}),
|
||||
updateComment: updateSpy,
|
||||
});
|
||||
|
||||
const context = createMockContext({
|
||||
eventName: "pull_request",
|
||||
isPR: true,
|
||||
inputs: { useStickyComment: true },
|
||||
payload: {
|
||||
action: "opened",
|
||||
number: 1,
|
||||
pull_request: { number: 1 },
|
||||
} as any,
|
||||
});
|
||||
|
||||
const result = await createInitialComment(mockOctokit, context);
|
||||
expect(result.id).toBe(202);
|
||||
expect(updateSpy).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("GitHub platform — PR review comment reply", () => {
|
||||
test("creates reply via createReplyForReviewComment when payload has comment.id", async () => {
|
||||
const createReply = async () => ({ data: { id: 301 } });
|
||||
const replySpy = spyOn({ createReply }, "createReply");
|
||||
const mockOctokit = createMockOctokit({
|
||||
createReplyForReviewComment: replySpy,
|
||||
});
|
||||
|
||||
const context = createMockContext({
|
||||
eventName: "pull_request_review_comment",
|
||||
isPR: true,
|
||||
entityNumber: 10,
|
||||
payload: {
|
||||
action: "created",
|
||||
comment: { id: 555 },
|
||||
pull_request: { number: 10 },
|
||||
} as any,
|
||||
});
|
||||
|
||||
const result = await createInitialComment(mockOctokit, context);
|
||||
expect(result.id).toBe(301);
|
||||
expect(replySpy).toHaveBeenCalled();
|
||||
const callArgs = (replySpy.mock.calls as any[][])[0]![0];
|
||||
expect(callArgs.comment_id).toBe(555);
|
||||
expect(callArgs.pull_number).toBe(10);
|
||||
});
|
||||
|
||||
test("falls back to createComment when payload lacks comment.id", async () => {
|
||||
const createComment = async () => ({ data: { id: 102 } });
|
||||
const createSpy = spyOn({ createComment }, "createComment");
|
||||
const createReply = async () => ({ data: { id: 301 } });
|
||||
const replySpy = spyOn({ createReply }, "createReply");
|
||||
const mockOctokit = createMockOctokit({
|
||||
createComment: createSpy,
|
||||
createReplyForReviewComment: replySpy,
|
||||
});
|
||||
|
||||
const context = createMockContext({
|
||||
eventName: "pull_request_review_comment",
|
||||
isPR: true,
|
||||
entityNumber: 10,
|
||||
// No comment.id in payload (Forgejo behavior)
|
||||
payload: {
|
||||
action: "created",
|
||||
pull_request: { number: 10 },
|
||||
} as any,
|
||||
});
|
||||
|
||||
const result = await createInitialComment(mockOctokit, context);
|
||||
expect(result.id).toBe(102);
|
||||
expect(createSpy).toHaveBeenCalled();
|
||||
expect(replySpy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("Forgejo platform", () => {
|
||||
beforeEach(() => {
|
||||
_resetPlatformCache();
|
||||
process.env.PLATFORM = "forgejo";
|
||||
process.env.GITHUB_SERVER_URL = "https://git.example.com";
|
||||
process.env.GITHUB_RUN_NUMBER = "42";
|
||||
});
|
||||
|
||||
test("uses Unicode spinner and Forgejo URL format in body", async () => {
|
||||
let capturedBody = "";
|
||||
const mockOctokit = createMockOctokit({
|
||||
createComment: async (args: any) => {
|
||||
capturedBody = args.body;
|
||||
return { data: { id: 1 } };
|
||||
},
|
||||
});
|
||||
const context = createMockContext();
|
||||
|
||||
await createInitialComment(mockOctokit, context);
|
||||
|
||||
// Forgejo uses Unicode hourglass, not CDN image
|
||||
expect(capturedBody).toContain("⏳");
|
||||
expect(capturedBody).not.toContain("user-attachments/assets");
|
||||
// Forgejo URL uses GITHUB_RUN_NUMBER
|
||||
expect(capturedBody).toContain(
|
||||
"https://git.example.com/test-owner/test-repo/actions/runs/42",
|
||||
);
|
||||
});
|
||||
|
||||
test("sticky comment matches by botName input on Forgejo", async () => {
|
||||
const updateComment = async () => ({ data: { id: 203 } });
|
||||
const updateSpy = spyOn({ updateComment }, "updateComment");
|
||||
const mockOctokit = createMockOctokit({
|
||||
listComments: async () => ({
|
||||
data: [
|
||||
{
|
||||
id: 203,
|
||||
user: { id: 999, type: "User", login: "claude" },
|
||||
body: "some old body",
|
||||
},
|
||||
],
|
||||
}),
|
||||
updateComment: updateSpy,
|
||||
});
|
||||
|
||||
const context = createMockContext({
|
||||
eventName: "pull_request",
|
||||
isPR: true,
|
||||
inputs: { useStickyComment: true, botName: "claude" },
|
||||
payload: {
|
||||
action: "opened",
|
||||
number: 1,
|
||||
pull_request: { number: 1 },
|
||||
} as any,
|
||||
});
|
||||
|
||||
const result = await createInitialComment(mockOctokit, context);
|
||||
expect(result.id).toBe(203);
|
||||
expect(updateSpy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test("review comment event falls back to createComment (no comment.id in Forgejo payload)", async () => {
|
||||
const createComment = async () => ({ data: { id: 103 } });
|
||||
const createSpy = spyOn({ createComment }, "createComment");
|
||||
const mockOctokit = createMockOctokit({
|
||||
createComment: createSpy,
|
||||
});
|
||||
|
||||
const context = createMockContext({
|
||||
eventName: "pull_request_review_comment",
|
||||
isPR: true,
|
||||
entityNumber: 7,
|
||||
payload: {
|
||||
action: "created",
|
||||
pull_request: { number: 7 },
|
||||
// Forgejo payload lacks top-level comment object
|
||||
} as any,
|
||||
});
|
||||
|
||||
const result = await createInitialComment(mockOctokit, context);
|
||||
expect(result.id).toBe(103);
|
||||
expect(createSpy).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("error handling", () => {
|
||||
test("falls back to issues.createComment when primary path throws", async () => {
|
||||
let fallbackCalled = false;
|
||||
const mockOctokit = createMockOctokit({
|
||||
listComments: async () => {
|
||||
throw new Error("API error");
|
||||
},
|
||||
createComment: async () => {
|
||||
fallbackCalled = true;
|
||||
return { data: { id: 999 } };
|
||||
},
|
||||
});
|
||||
|
||||
const context = createMockContext({
|
||||
eventName: "pull_request",
|
||||
isPR: true,
|
||||
inputs: { useStickyComment: true },
|
||||
payload: {
|
||||
action: "opened",
|
||||
number: 1,
|
||||
pull_request: { number: 1 },
|
||||
} as any,
|
||||
});
|
||||
|
||||
const result = await createInitialComment(mockOctokit, context);
|
||||
expect(result.id).toBe(999);
|
||||
expect(fallbackCalled).toBe(true);
|
||||
expect(consoleErrorSpy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test("throws when both primary and fallback fail", async () => {
|
||||
const mockOctokit = createMockOctokit({
|
||||
listComments: async () => {
|
||||
throw new Error("Primary error");
|
||||
},
|
||||
createComment: async () => {
|
||||
throw new Error("Fallback error");
|
||||
},
|
||||
});
|
||||
|
||||
const context = createMockContext({
|
||||
eventName: "pull_request",
|
||||
isPR: true,
|
||||
inputs: { useStickyComment: true },
|
||||
payload: {
|
||||
action: "opened",
|
||||
number: 1,
|
||||
pull_request: { number: 1 },
|
||||
} as any,
|
||||
});
|
||||
|
||||
await expect(createInitialComment(mockOctokit, context)).rejects.toThrow(
|
||||
"Fallback error",
|
||||
);
|
||||
});
|
||||
|
||||
test("writes comment ID from fallback to GITHUB_OUTPUT", async () => {
|
||||
const mockOctokit = createMockOctokit({
|
||||
listComments: async () => {
|
||||
throw new Error("API error");
|
||||
},
|
||||
createComment: async () => ({ data: { id: 888 } }),
|
||||
});
|
||||
|
||||
const context = createMockContext({
|
||||
eventName: "pull_request",
|
||||
isPR: true,
|
||||
inputs: { useStickyComment: true },
|
||||
payload: {
|
||||
action: "opened",
|
||||
number: 1,
|
||||
pull_request: { number: 1 },
|
||||
} as any,
|
||||
});
|
||||
|
||||
await createInitialComment(mockOctokit, context);
|
||||
|
||||
const output = readFileSync(tempOutputFile, "utf-8");
|
||||
expect(output).toContain("claude_comment_id=888");
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -6,10 +6,12 @@ import {
|
||||
createForgejoBranchLink,
|
||||
createForgejoPrCompareUrl,
|
||||
createForgejoCommentBody,
|
||||
extractOwnerRepoFromUrl,
|
||||
updateForgejoCommentBody,
|
||||
type CommentUpdateInput,
|
||||
} from "../src/forgejo/comment-logic";
|
||||
import {
|
||||
extractOwnerRepoFromUrl,
|
||||
updateCommentBody,
|
||||
type CommentUpdateInput,
|
||||
} from "../src/github/operations/comment-logic";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Test helpers
|
||||
@@ -307,10 +309,10 @@ describe("extractOwnerRepoFromUrl", () => {
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// updateForgejoCommentBody
|
||||
// updateCommentBody
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("updateForgejoCommentBody", () => {
|
||||
describe("updateCommentBody", () => {
|
||||
const forgejoBaseInput: CommentUpdateInput = {
|
||||
currentBody: "Initial comment body",
|
||||
actionFailed: false,
|
||||
@@ -332,7 +334,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
triggerUsername: "testuser",
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
expect(result).toContain("**Claude finished @testuser's task**");
|
||||
expect(result).not.toContain("Claude Code is working");
|
||||
expect(result).not.toContain("<img");
|
||||
@@ -345,7 +347,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
triggerUsername: "testuser",
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
expect(result).toContain("**Claude finished @testuser's task**");
|
||||
expect(result).not.toContain("Claude Code is working");
|
||||
expect(result).not.toContain("⏳");
|
||||
@@ -359,7 +361,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
triggerUsername: "trigger-user",
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
expect(result).toContain(
|
||||
"**Claude finished @trigger-user's task in 1m 14s**",
|
||||
);
|
||||
@@ -373,7 +375,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
executionDetails: { duration_ms: 45000 },
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
expect(result).toContain("**Claude encountered an error after 45s**");
|
||||
});
|
||||
|
||||
@@ -386,7 +388,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
errorDetails: "Failed to fetch issue data",
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
expect(result).toContain("**Claude encountered an error after 45s**");
|
||||
expect(result).toContain("[View job]");
|
||||
expect(result).toContain("```\nFailed to fetch issue data\n```");
|
||||
@@ -400,7 +402,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
currentBody: "Some comment",
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
expect(result).toContain(`—— [View job](${forgejoBaseInput.jobUrl})`);
|
||||
});
|
||||
});
|
||||
@@ -412,7 +414,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
branchName: "claude/issue-5-20240101-1200",
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
// This is the critical fix: GitHub's version uses a hardcoded github.com regex
|
||||
// which fails for Forgejo URLs. Our version uses extractOwnerRepoFromUrl.
|
||||
expect(result).toContain(
|
||||
@@ -427,7 +429,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
branchName: "feature-branch",
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
expect(result).toContain("• `feature-branch`");
|
||||
expect(result).not.toContain("[`feature-branch`]");
|
||||
});
|
||||
@@ -440,7 +442,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
"\n[View branch](https://git.wxs.ro/wxs/myrepo/tree/feature-branch)",
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
expect(result).toContain(
|
||||
"• [`feature-branch`](https://git.wxs.ro/wxs/myrepo/tree/feature-branch)",
|
||||
);
|
||||
@@ -453,7 +455,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
"\n[View branch](https://git.wxs.ro/wxs/myrepo/tree/auto-detected-branch)",
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
expect(result).toContain(
|
||||
"• [`auto-detected-branch`](https://git.wxs.ro/wxs/myrepo/tree/auto-detected-branch)",
|
||||
);
|
||||
@@ -468,7 +470,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
"\n[Create a PR](https://git.wxs.ro/wxs/myrepo/compare/main...feature)",
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
expect(result).toContain(
|
||||
"• [Create PR ➔](https://git.wxs.ro/wxs/myrepo/compare/main...feature)",
|
||||
);
|
||||
@@ -481,7 +483,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
"Some comment with [Create a PR](https://git.wxs.ro/wxs/myrepo/compare/main...fix)",
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
expect(result).toContain(
|
||||
"• [Create PR ➔](https://git.wxs.ro/wxs/myrepo/compare/main...fix)",
|
||||
);
|
||||
@@ -496,7 +498,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
currentBody: `Done!\n\n[Create a PR](${forgejoUrl})`,
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
expect(result).toContain(`• [Create PR ➔](${forgejoUrl})`);
|
||||
});
|
||||
});
|
||||
@@ -509,7 +511,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
triggerUsername: "testuser",
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
expect(result).toContain(
|
||||
"**Claude finished @testuser's task in 1m 15s**",
|
||||
);
|
||||
@@ -522,7 +524,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
triggerUsername: "testuser",
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
expect(result).toContain("**Claude finished @testuser's task in 31s**");
|
||||
});
|
||||
|
||||
@@ -533,7 +535,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
triggerUsername: "testuser",
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
expect(result).toContain("**Claude finished @testuser's task**");
|
||||
expect(result).not.toContain(" in ");
|
||||
});
|
||||
@@ -553,7 +555,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
triggerUsername: "trigger-user",
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
|
||||
// Check header
|
||||
expect(result).toContain(
|
||||
@@ -594,7 +596,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
prLink: "",
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
|
||||
expect(result).toContain("Claude finished @claude's task in 1m 30s");
|
||||
expect(result).toContain(`[View job](${forgejoBaseInput.jobUrl})`);
|
||||
@@ -612,7 +614,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
triggerUsername: "user",
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
expect(result).not.toContain("[View job run]");
|
||||
expect(result).toContain("[View job]");
|
||||
});
|
||||
@@ -626,7 +628,7 @@ describe("updateForgejoCommentBody", () => {
|
||||
triggerUsername: "user",
|
||||
};
|
||||
|
||||
const result = updateForgejoCommentBody(input);
|
||||
const result = updateCommentBody(input);
|
||||
// Old link in body removed
|
||||
expect(result).not.toContain("[View branch]");
|
||||
// New branch in header
|
||||
|
||||
@@ -3,6 +3,7 @@ import { prepareMcpConfig } from "../src/mcp/install-mcp-server";
|
||||
import * as core from "@actions/core";
|
||||
import type { ParsedGitHubContext } from "../src/github/context";
|
||||
import { CLAUDE_APP_BOT_ID, CLAUDE_BOT_LOGIN } from "../src/github/constants";
|
||||
import { _resetPlatformCache } from "../src/platform/detector";
|
||||
|
||||
describe("prepareMcpConfig", () => {
|
||||
let consoleInfoSpy: any;
|
||||
@@ -321,11 +322,14 @@ describe("prepareMcpConfig", () => {
|
||||
let savedServerUrl: string | undefined;
|
||||
|
||||
beforeEach(() => {
|
||||
_resetPlatformCache();
|
||||
savedPlatform = process.env.PLATFORM;
|
||||
savedForgejo = process.env.FORGEJO;
|
||||
savedServerUrl = process.env.GITHUB_SERVER_URL;
|
||||
// Set platform to forgejo for all tests in this block
|
||||
process.env.PLATFORM = "forgejo";
|
||||
// Ensure cross-validation doesn't reject PLATFORM=forgejo
|
||||
process.env.GITHUB_SERVER_URL = "https://git.example.com";
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -344,6 +348,7 @@ describe("prepareMcpConfig", () => {
|
||||
} else {
|
||||
delete process.env.GITHUB_SERVER_URL;
|
||||
}
|
||||
_resetPlatformCache();
|
||||
});
|
||||
|
||||
test("should skip GitHub public MCP server on Forgejo", async () => {
|
||||
|
||||
@@ -9,7 +9,7 @@
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from "bun:test";
|
||||
import { detectPlatform } from "../src/platform/detector";
|
||||
import { detectPlatform, _resetPlatformCache } from "../src/platform/detector";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Helpers
|
||||
@@ -40,16 +40,19 @@ function restoreEnv() {
|
||||
|
||||
describe("M7: Platform routing for mode handlers", () => {
|
||||
beforeEach(() => {
|
||||
_resetPlatformCache();
|
||||
saveEnv("PLATFORM", "FORGEJO", "GITHUB_SERVER_URL");
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
restoreEnv();
|
||||
_resetPlatformCache();
|
||||
});
|
||||
|
||||
describe("detectPlatform() — used by tag mode and update-comment-link", () => {
|
||||
it('returns "forgejo" when PLATFORM=forgejo', () => {
|
||||
process.env.PLATFORM = "forgejo";
|
||||
process.env.GITHUB_SERVER_URL = "https://git.example.com";
|
||||
expect(detectPlatform()).toBe("forgejo");
|
||||
});
|
||||
|
||||
@@ -156,13 +159,13 @@ describe("M7: Forgejo PR compare URL format", () => {
|
||||
// update-comment-link: platform-aware final comment body
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("M7: updateForgejoCommentBody vs updateCommentBody", () => {
|
||||
it("Forgejo version strips Unicode spinner", async () => {
|
||||
const { updateForgejoCommentBody } = await import(
|
||||
"../src/forgejo/comment-logic"
|
||||
describe("M7: unified updateCommentBody handles both platforms", () => {
|
||||
it("strips Unicode spinner (Forgejo format)", async () => {
|
||||
const { updateCommentBody } = await import(
|
||||
"../src/github/operations/comment-logic"
|
||||
);
|
||||
|
||||
const result = updateForgejoCommentBody({
|
||||
const result = updateCommentBody({
|
||||
currentBody:
|
||||
"Claude Code is working… ⏳\n\nI'll analyze this and get back to you.\n\n[View job run](https://git.wxs.ro/owner/repo/actions/runs/123)",
|
||||
actionFailed: false,
|
||||
|
||||
+72
-51
@@ -3,6 +3,7 @@ import * as core from "@actions/core";
|
||||
import { checkWritePermissions } from "../src/github/validation/permissions";
|
||||
import type { ParsedGitHubContext } from "../src/github/context";
|
||||
import { CLAUDE_APP_BOT_ID, CLAUDE_BOT_LOGIN } from "../src/github/constants";
|
||||
import { _resetPlatformCache } from "../src/platform/detector";
|
||||
|
||||
describe("checkWritePermissions", () => {
|
||||
let coreInfoSpy: any;
|
||||
@@ -310,10 +311,13 @@ describe("checkWritePermissions", () => {
|
||||
let savedServerUrl: string | undefined;
|
||||
|
||||
beforeEach(() => {
|
||||
_resetPlatformCache();
|
||||
savedPlatform = process.env.PLATFORM;
|
||||
savedForgejo = process.env.FORGEJO;
|
||||
savedServerUrl = process.env.GITHUB_SERVER_URL;
|
||||
process.env.PLATFORM = "forgejo";
|
||||
// Ensure cross-validation doesn't reject PLATFORM=forgejo
|
||||
process.env.GITHUB_SERVER_URL = "https://git.example.com";
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -324,27 +328,78 @@ describe("checkWritePermissions", () => {
|
||||
if (savedServerUrl !== undefined)
|
||||
process.env.GITHUB_SERVER_URL = savedServerUrl;
|
||||
else delete process.env.GITHUB_SERVER_URL;
|
||||
_resetPlatformCache();
|
||||
});
|
||||
|
||||
const createForgejoOctokit = (permissions: {
|
||||
// Helper: Forgejo octokit where collaborator API works (actor-level check)
|
||||
const createForgejoActorOctokit = (permission: string) => {
|
||||
return {
|
||||
repos: {
|
||||
getCollaboratorPermissionLevel: async () => ({
|
||||
data: { permission },
|
||||
}),
|
||||
get: async () => {
|
||||
throw new Error("Should not fall back to repos.get");
|
||||
},
|
||||
},
|
||||
} as any;
|
||||
};
|
||||
|
||||
// Helper: Forgejo octokit where collaborator API fails (falls back to token-level)
|
||||
const createForgejoFallbackOctokit = (permissions: {
|
||||
admin?: boolean;
|
||||
push?: boolean;
|
||||
pull?: boolean;
|
||||
}) => {
|
||||
return {
|
||||
repos: {
|
||||
getCollaboratorPermissionLevel: async () => {
|
||||
throw new Error("403 Forbidden");
|
||||
},
|
||||
get: async () => ({
|
||||
data: { permissions },
|
||||
}),
|
||||
getCollaboratorPermissionLevel: async () => {
|
||||
throw new Error("Should not be called on Forgejo");
|
||||
},
|
||||
},
|
||||
} as any;
|
||||
};
|
||||
|
||||
test("should return true when Forgejo token has push access", async () => {
|
||||
const mockOctokit = createForgejoOctokit({
|
||||
test("should use actor-level permission check on Forgejo (write)", async () => {
|
||||
const mockOctokit = createForgejoActorOctokit("write");
|
||||
const context = createContext();
|
||||
|
||||
const result = await checkWritePermissions(mockOctokit, context);
|
||||
|
||||
expect(result).toBe(true);
|
||||
expect(coreInfoSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining(
|
||||
"Forgejo actor 'test-user' permission level: write",
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
test("should use actor-level permission check on Forgejo (admin)", async () => {
|
||||
const mockOctokit = createForgejoActorOctokit("admin");
|
||||
const context = createContext();
|
||||
|
||||
const result = await checkWritePermissions(mockOctokit, context);
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
test("should reject actor with read-only permission on Forgejo", async () => {
|
||||
const mockOctokit = createForgejoActorOctokit("read");
|
||||
const context = createContext();
|
||||
|
||||
const result = await checkWritePermissions(mockOctokit, context);
|
||||
|
||||
expect(result).toBe(false);
|
||||
expect(coreWarningSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("insufficient permissions: read"),
|
||||
);
|
||||
});
|
||||
|
||||
test("should fall back to token-level check when collaborator API fails", async () => {
|
||||
const mockOctokit = createForgejoFallbackOctokit({
|
||||
admin: false,
|
||||
push: true,
|
||||
pull: true,
|
||||
@@ -354,26 +409,16 @@ describe("checkWritePermissions", () => {
|
||||
const result = await checkWritePermissions(mockOctokit, context);
|
||||
|
||||
expect(result).toBe(true);
|
||||
expect(coreWarningSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("Falling back to token-level permission check"),
|
||||
);
|
||||
expect(coreInfoSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("Forgejo token has write access"),
|
||||
expect.stringContaining("token-level fallback"),
|
||||
);
|
||||
});
|
||||
|
||||
test("should return true when Forgejo token has admin access", async () => {
|
||||
const mockOctokit = createForgejoOctokit({
|
||||
admin: true,
|
||||
push: false,
|
||||
pull: true,
|
||||
});
|
||||
const context = createContext();
|
||||
|
||||
const result = await checkWritePermissions(mockOctokit, context);
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
test("should return false when Forgejo token has only pull access", async () => {
|
||||
const mockOctokit = createForgejoOctokit({
|
||||
test("should return false on fallback when token has only pull access", async () => {
|
||||
const mockOctokit = createForgejoFallbackOctokit({
|
||||
admin: false,
|
||||
push: false,
|
||||
pull: true,
|
||||
@@ -383,14 +428,14 @@ describe("checkWritePermissions", () => {
|
||||
const result = await checkWritePermissions(mockOctokit, context);
|
||||
|
||||
expect(result).toBe(false);
|
||||
expect(coreWarningSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("insufficient permissions"),
|
||||
);
|
||||
});
|
||||
|
||||
test("should return false when permissions object is missing", async () => {
|
||||
test("should return false on fallback when permissions object is missing", async () => {
|
||||
const mockOctokit = {
|
||||
repos: {
|
||||
getCollaboratorPermissionLevel: async () => {
|
||||
throw new Error("403 Forbidden");
|
||||
},
|
||||
get: async () => ({ data: {} }),
|
||||
},
|
||||
} as any;
|
||||
@@ -404,32 +449,8 @@ describe("checkWritePermissions", () => {
|
||||
);
|
||||
});
|
||||
|
||||
test("should not call getCollaboratorPermissionLevel on Forgejo", async () => {
|
||||
let collaboratorCalled = false;
|
||||
const mockOctokit = {
|
||||
repos: {
|
||||
get: async () => ({
|
||||
data: { permissions: { admin: false, push: true, pull: true } },
|
||||
}),
|
||||
getCollaboratorPermissionLevel: async () => {
|
||||
collaboratorCalled = true;
|
||||
return { data: { permission: "write" } };
|
||||
},
|
||||
},
|
||||
} as any;
|
||||
const context = createContext();
|
||||
|
||||
await checkWritePermissions(mockOctokit, context);
|
||||
|
||||
expect(collaboratorCalled).toBe(false);
|
||||
});
|
||||
|
||||
test("should still bypass for allowed_non_write_users on Forgejo", async () => {
|
||||
const mockOctokit = createForgejoOctokit({
|
||||
admin: false,
|
||||
push: false,
|
||||
pull: true,
|
||||
});
|
||||
const mockOctokit = createForgejoActorOctokit("read");
|
||||
const context = createContext();
|
||||
|
||||
const result = await checkWritePermissions(
|
||||
|
||||
@@ -1,9 +1,10 @@
|
||||
import { detectPlatform } from "../src/platform/detector";
|
||||
import { detectPlatform, _resetPlatformCache } from "../src/platform/detector";
|
||||
import { describe, it, expect, beforeEach } from "bun:test";
|
||||
|
||||
describe("detectPlatform", () => {
|
||||
beforeEach(() => {
|
||||
// Reset relevant env vars before each test
|
||||
// Reset memoized result and env vars before each test
|
||||
_resetPlatformCache();
|
||||
delete process.env.PLATFORM;
|
||||
delete process.env.FORGEJO;
|
||||
delete process.env.GITHUB_SERVER_URL;
|
||||
@@ -40,6 +41,19 @@ describe("detectPlatform", () => {
|
||||
// No other env vars set, should default to github
|
||||
expect(detectPlatform()).toBe("github");
|
||||
});
|
||||
|
||||
it("should reject PLATFORM=forgejo when GITHUB_SERVER_URL is github.com", () => {
|
||||
process.env.PLATFORM = "forgejo";
|
||||
process.env.GITHUB_SERVER_URL = "https://github.com";
|
||||
// Cross-validation guard: prevent forcing Forgejo mode on GitHub.com
|
||||
expect(detectPlatform()).toBe("github");
|
||||
});
|
||||
|
||||
it("should accept PLATFORM=forgejo when GITHUB_SERVER_URL is not github.com", () => {
|
||||
process.env.PLATFORM = "forgejo";
|
||||
process.env.GITHUB_SERVER_URL = "https://git.example.com";
|
||||
expect(detectPlatform()).toBe("forgejo");
|
||||
});
|
||||
});
|
||||
|
||||
describe("FORGEJO env var", () => {
|
||||
|
||||
@@ -0,0 +1,150 @@
|
||||
#!/usr/bin/env bun
|
||||
|
||||
/**
|
||||
* Tests the Forgejo API divergence in setupBranch().
|
||||
*
|
||||
* Forgejo uses repos.getBranch() to get source branch SHA because Forgejo's
|
||||
* /git/refs (plural) API differs from GitHub's /git/ref (singular), causing
|
||||
* git.getRef() to 404 on Forgejo.
|
||||
*
|
||||
* Approach: Call setupBranch() with mocked Octokit. The platform-specific
|
||||
* API call records that it was called, then throws a sentinel error to stop
|
||||
* execution before git shell commands run (avoiding real repo side effects).
|
||||
*/
|
||||
|
||||
import {
|
||||
describe,
|
||||
test,
|
||||
expect,
|
||||
beforeEach,
|
||||
afterEach,
|
||||
spyOn,
|
||||
type Mock,
|
||||
} from "bun:test";
|
||||
import { setupBranch } from "../src/github/operations/branch";
|
||||
import { _resetPlatformCache } from "../src/platform/detector";
|
||||
import { createMockContext } from "./mockContext";
|
||||
import type { Octokits } from "../src/github/api/client";
|
||||
import type { FetchDataResult } from "../src/github/data/fetcher";
|
||||
|
||||
const mockGithubData: FetchDataResult = {
|
||||
contextData: {
|
||||
title: "Test issue",
|
||||
labels: { nodes: [{ name: "bug" }] },
|
||||
},
|
||||
comments: [],
|
||||
changedFiles: [],
|
||||
changedFilesWithSHA: [],
|
||||
reviewData: null,
|
||||
imageUrlMap: new Map(),
|
||||
} as any;
|
||||
|
||||
function createBranchMockOctokits(
|
||||
overrides: Record<string, any> = {},
|
||||
): Octokits {
|
||||
return {
|
||||
rest: {
|
||||
repos: {
|
||||
get:
|
||||
overrides.reposGet ??
|
||||
(async () => ({ data: { default_branch: "main" } })),
|
||||
getBranch:
|
||||
overrides.getBranch ??
|
||||
(async () => ({ data: { commit: { sha: "abc123" } } })),
|
||||
},
|
||||
git: {
|
||||
getRef:
|
||||
overrides.getRef ??
|
||||
(async () => ({ data: { object: { sha: "abc123" } } })),
|
||||
},
|
||||
},
|
||||
graphql: {} as any,
|
||||
} as unknown as Octokits;
|
||||
}
|
||||
|
||||
describe("setupBranch — Forgejo API divergence", () => {
|
||||
let savedPlatform: string | undefined;
|
||||
let savedServerUrl: string | undefined;
|
||||
let consoleLogSpy: Mock<(...args: any[]) => void>;
|
||||
let consoleErrorSpy: Mock<(...args: any[]) => void>;
|
||||
|
||||
beforeEach(() => {
|
||||
_resetPlatformCache();
|
||||
savedPlatform = process.env.PLATFORM;
|
||||
savedServerUrl = process.env.GITHUB_SERVER_URL;
|
||||
|
||||
consoleLogSpy = spyOn(console, "log").mockImplementation(() => {});
|
||||
consoleErrorSpy = spyOn(console, "error").mockImplementation(() => {});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (savedPlatform !== undefined) process.env.PLATFORM = savedPlatform;
|
||||
else delete process.env.PLATFORM;
|
||||
if (savedServerUrl !== undefined)
|
||||
process.env.GITHUB_SERVER_URL = savedServerUrl;
|
||||
else delete process.env.GITHUB_SERVER_URL;
|
||||
|
||||
consoleLogSpy.mockRestore();
|
||||
consoleErrorSpy.mockRestore();
|
||||
_resetPlatformCache();
|
||||
});
|
||||
|
||||
test("on Forgejo: calls repos.getBranch() instead of git.getRef()", async () => {
|
||||
process.env.PLATFORM = "forgejo";
|
||||
process.env.GITHUB_SERVER_URL = "https://git.example.com";
|
||||
_resetPlatformCache();
|
||||
|
||||
let getBranchCalled = false;
|
||||
let getRefCalled = false;
|
||||
|
||||
const mockOctokits = createBranchMockOctokits({
|
||||
getBranch: async () => {
|
||||
getBranchCalled = true;
|
||||
// Throw sentinel error to stop before git commands execute
|
||||
throw new Error("STOP_BEFORE_GIT_COMMANDS");
|
||||
},
|
||||
getRef: async () => {
|
||||
getRefCalled = true;
|
||||
throw new Error("STOP_BEFORE_GIT_COMMANDS");
|
||||
},
|
||||
});
|
||||
|
||||
const context = createMockContext({ isPR: false, entityNumber: 1 });
|
||||
|
||||
await expect(
|
||||
setupBranch(mockOctokits, mockGithubData, context),
|
||||
).rejects.toThrow("STOP_BEFORE_GIT_COMMANDS");
|
||||
|
||||
expect(getBranchCalled).toBe(true);
|
||||
expect(getRefCalled).toBe(false);
|
||||
});
|
||||
|
||||
test("on GitHub: calls git.getRef() instead of repos.getBranch()", async () => {
|
||||
delete process.env.PLATFORM;
|
||||
process.env.GITHUB_SERVER_URL = "https://github.com";
|
||||
_resetPlatformCache();
|
||||
|
||||
let getBranchCalled = false;
|
||||
let getRefCalled = false;
|
||||
|
||||
const mockOctokits = createBranchMockOctokits({
|
||||
getBranch: async () => {
|
||||
getBranchCalled = true;
|
||||
throw new Error("STOP_BEFORE_GIT_COMMANDS");
|
||||
},
|
||||
getRef: async () => {
|
||||
getRefCalled = true;
|
||||
throw new Error("STOP_BEFORE_GIT_COMMANDS");
|
||||
},
|
||||
});
|
||||
|
||||
const context = createMockContext({ isPR: false, entityNumber: 1 });
|
||||
|
||||
await expect(
|
||||
setupBranch(mockOctokits, mockGithubData, context),
|
||||
).rejects.toThrow("STOP_BEFORE_GIT_COMMANDS");
|
||||
|
||||
expect(getRefCalled).toBe(true);
|
||||
expect(getBranchCalled).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -2,7 +2,8 @@ import {
|
||||
checkContainsTrigger,
|
||||
escapeRegExp,
|
||||
} from "../src/github/validation/trigger";
|
||||
import { describe, it, expect } from "bun:test";
|
||||
import { describe, it, expect, beforeEach, afterEach } from "bun:test";
|
||||
import { _resetPlatformCache } from "../src/platform/detector";
|
||||
import {
|
||||
createMockContext,
|
||||
mockIssueAssignedContext,
|
||||
@@ -451,6 +452,75 @@ describe("checkContainsTrigger", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("Forgejo empty-body trigger bypass", () => {
|
||||
let savedPlatform: string | undefined;
|
||||
let savedServerUrl: string | undefined;
|
||||
|
||||
beforeEach(() => {
|
||||
_resetPlatformCache();
|
||||
savedPlatform = process.env.PLATFORM;
|
||||
savedServerUrl = process.env.GITHUB_SERVER_URL;
|
||||
process.env.PLATFORM = "forgejo";
|
||||
process.env.GITHUB_SERVER_URL = "https://git.example.com";
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (savedPlatform !== undefined) process.env.PLATFORM = savedPlatform;
|
||||
else delete process.env.PLATFORM;
|
||||
if (savedServerUrl !== undefined)
|
||||
process.env.GITHUB_SERVER_URL = savedServerUrl;
|
||||
else delete process.env.GITHUB_SERVER_URL;
|
||||
_resetPlatformCache();
|
||||
});
|
||||
|
||||
it("should NOT trigger when Forgejo PR review comment has no body", () => {
|
||||
// Previously this returned true unconditionally, bypassing trigger check
|
||||
const context = {
|
||||
...mockPullRequestReviewCommentContext,
|
||||
payload: {
|
||||
...mockPullRequestReviewCommentContext.payload,
|
||||
comment: {
|
||||
...(mockPullRequestReviewCommentContext.payload as any).comment,
|
||||
body: "",
|
||||
},
|
||||
},
|
||||
} as ParsedGitHubContext;
|
||||
expect(checkContainsTrigger(context)).toBe(false);
|
||||
});
|
||||
|
||||
it("should NOT trigger when Forgejo PR review comment body is missing", () => {
|
||||
const context = {
|
||||
...mockPullRequestReviewCommentContext,
|
||||
payload: {
|
||||
...mockPullRequestReviewCommentContext.payload,
|
||||
comment: {
|
||||
...(mockPullRequestReviewCommentContext.payload as any).comment,
|
||||
body: undefined,
|
||||
},
|
||||
},
|
||||
} as ParsedGitHubContext;
|
||||
expect(checkContainsTrigger(context)).toBe(false);
|
||||
});
|
||||
|
||||
it("should still trigger when Forgejo PR review comment contains trigger phrase", () => {
|
||||
const context = {
|
||||
...mockPullRequestReviewCommentContext,
|
||||
inputs: {
|
||||
...mockPullRequestReviewCommentContext.inputs,
|
||||
triggerPhrase: "@claude",
|
||||
},
|
||||
payload: {
|
||||
...mockPullRequestReviewCommentContext.payload,
|
||||
comment: {
|
||||
...(mockPullRequestReviewCommentContext.payload as any).comment,
|
||||
body: "@claude please review this line",
|
||||
},
|
||||
},
|
||||
} as ParsedGitHubContext;
|
||||
expect(checkContainsTrigger(context)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("escapeRegExp", () => {
|
||||
it("should escape special regex characters", () => {
|
||||
expect(escapeRegExp(".*+?^${}()|[]\\")).toBe(
|
||||
|
||||
@@ -0,0 +1,277 @@
|
||||
#!/usr/bin/env bun
|
||||
|
||||
import {
|
||||
describe,
|
||||
test,
|
||||
expect,
|
||||
beforeEach,
|
||||
afterEach,
|
||||
spyOn,
|
||||
type Mock,
|
||||
} from "bun:test";
|
||||
import { updateTrackingComment } from "../src/github/operations/comments/update-with-branch";
|
||||
import { _resetPlatformCache } from "../src/platform/detector";
|
||||
import { createMockContext } from "./mockContext";
|
||||
import type { Octokits } from "../src/github/api/client";
|
||||
|
||||
function createMockOctokits(overrides: Record<string, any> = {}): Octokits {
|
||||
// updateTrackingComment calls updateClaudeComment(octokit.rest, ...),
|
||||
// which accesses octokit.rest.issues.updateComment / octokit.rest.pulls.updateReviewComment
|
||||
return {
|
||||
rest: {
|
||||
rest: {
|
||||
issues: {
|
||||
updateComment:
|
||||
overrides.updateComment ??
|
||||
(async () => ({ data: { id: 1, html_url: "", updated_at: "" } })),
|
||||
},
|
||||
pulls: {
|
||||
updateReviewComment:
|
||||
overrides.updateReviewComment ??
|
||||
(async () => ({ data: { id: 1, html_url: "", updated_at: "" } })),
|
||||
},
|
||||
},
|
||||
},
|
||||
graphql: {} as any,
|
||||
} as unknown as Octokits;
|
||||
}
|
||||
|
||||
describe("updateTrackingComment", () => {
|
||||
let savedPlatform: string | undefined;
|
||||
let savedServerUrl: string | undefined;
|
||||
let savedRunNumber: string | undefined;
|
||||
let savedRunId: string | undefined;
|
||||
let consoleLogSpy: Mock<(...args: any[]) => void>;
|
||||
let consoleErrorSpy: Mock<(...args: any[]) => void>;
|
||||
|
||||
beforeEach(() => {
|
||||
_resetPlatformCache();
|
||||
savedPlatform = process.env.PLATFORM;
|
||||
savedServerUrl = process.env.GITHUB_SERVER_URL;
|
||||
savedRunNumber = process.env.GITHUB_RUN_NUMBER;
|
||||
savedRunId = process.env.GITHUB_RUN_ID;
|
||||
|
||||
// Default to GitHub platform
|
||||
delete process.env.PLATFORM;
|
||||
process.env.GITHUB_SERVER_URL = "https://github.com";
|
||||
|
||||
consoleLogSpy = spyOn(console, "log").mockImplementation(() => {});
|
||||
consoleErrorSpy = spyOn(console, "error").mockImplementation(() => {});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (savedPlatform !== undefined) process.env.PLATFORM = savedPlatform;
|
||||
else delete process.env.PLATFORM;
|
||||
if (savedServerUrl !== undefined)
|
||||
process.env.GITHUB_SERVER_URL = savedServerUrl;
|
||||
else delete process.env.GITHUB_SERVER_URL;
|
||||
if (savedRunNumber !== undefined)
|
||||
process.env.GITHUB_RUN_NUMBER = savedRunNumber;
|
||||
else delete process.env.GITHUB_RUN_NUMBER;
|
||||
if (savedRunId !== undefined) process.env.GITHUB_RUN_ID = savedRunId;
|
||||
else delete process.env.GITHUB_RUN_ID;
|
||||
|
||||
consoleLogSpy.mockRestore();
|
||||
consoleErrorSpy.mockRestore();
|
||||
_resetPlatformCache();
|
||||
});
|
||||
|
||||
describe("GitHub platform — body construction", () => {
|
||||
test("includes GitHub job run link using context.runId", async () => {
|
||||
let capturedBody = "";
|
||||
const mockOctokits = createMockOctokits({
|
||||
updateComment: async (args: any) => {
|
||||
capturedBody = args.body;
|
||||
return { data: { id: 1, html_url: "", updated_at: "" } };
|
||||
},
|
||||
});
|
||||
|
||||
const context = createMockContext({ runId: "55555" });
|
||||
await updateTrackingComment(mockOctokits, context, 1);
|
||||
|
||||
expect(capturedBody).toContain("actions/runs/55555");
|
||||
expect(capturedBody).toContain("https://github.com/test-owner/test-repo");
|
||||
});
|
||||
|
||||
test("includes branch link for issue context", async () => {
|
||||
let capturedBody = "";
|
||||
const mockOctokits = createMockOctokits({
|
||||
updateComment: async (args: any) => {
|
||||
capturedBody = args.body;
|
||||
return { data: { id: 1, html_url: "", updated_at: "" } };
|
||||
},
|
||||
});
|
||||
|
||||
const context = createMockContext({ isPR: false });
|
||||
await updateTrackingComment(mockOctokits, context, 1, "claude/fix-123");
|
||||
|
||||
expect(capturedBody).toContain("View branch");
|
||||
expect(capturedBody).toContain("tree/claude/fix-123");
|
||||
});
|
||||
|
||||
test("omits branch link for PR context even when branch provided", async () => {
|
||||
let capturedBody = "";
|
||||
const mockOctokits = createMockOctokits({
|
||||
updateComment: async (args: any) => {
|
||||
capturedBody = args.body;
|
||||
return { data: { id: 1, html_url: "", updated_at: "" } };
|
||||
},
|
||||
});
|
||||
|
||||
const context = createMockContext({ isPR: true });
|
||||
await updateTrackingComment(mockOctokits, context, 1, "claude/fix-123");
|
||||
|
||||
expect(capturedBody).not.toContain("View branch");
|
||||
expect(capturedBody).not.toContain("tree/");
|
||||
});
|
||||
|
||||
test("omits branch link when branch is undefined", async () => {
|
||||
let capturedBody = "";
|
||||
const mockOctokits = createMockOctokits({
|
||||
updateComment: async (args: any) => {
|
||||
capturedBody = args.body;
|
||||
return { data: { id: 1, html_url: "", updated_at: "" } };
|
||||
},
|
||||
});
|
||||
|
||||
const context = createMockContext({ isPR: false });
|
||||
await updateTrackingComment(mockOctokits, context, 1);
|
||||
|
||||
expect(capturedBody).not.toContain("View branch");
|
||||
});
|
||||
});
|
||||
|
||||
describe("Forgejo platform — body construction", () => {
|
||||
beforeEach(() => {
|
||||
_resetPlatformCache();
|
||||
process.env.PLATFORM = "forgejo";
|
||||
process.env.GITHUB_SERVER_URL = "https://git.example.com";
|
||||
process.env.GITHUB_RUN_NUMBER = "77";
|
||||
});
|
||||
|
||||
test("uses Forgejo job run link with GITHUB_RUN_NUMBER", async () => {
|
||||
let capturedBody = "";
|
||||
const mockOctokits = createMockOctokits({
|
||||
updateComment: async (args: any) => {
|
||||
capturedBody = args.body;
|
||||
return { data: { id: 1, html_url: "", updated_at: "" } };
|
||||
},
|
||||
});
|
||||
|
||||
const context = createMockContext();
|
||||
await updateTrackingComment(mockOctokits, context, 1);
|
||||
|
||||
expect(capturedBody).toContain(
|
||||
"https://git.example.com/test-owner/test-repo/actions/runs/77",
|
||||
);
|
||||
});
|
||||
|
||||
test("uses Forgejo branch link for issues", async () => {
|
||||
let capturedBody = "";
|
||||
const mockOctokits = createMockOctokits({
|
||||
updateComment: async (args: any) => {
|
||||
capturedBody = args.body;
|
||||
return { data: { id: 1, html_url: "", updated_at: "" } };
|
||||
},
|
||||
});
|
||||
|
||||
const context = createMockContext({ isPR: false });
|
||||
await updateTrackingComment(mockOctokits, context, 1, "claude/fix-42");
|
||||
|
||||
expect(capturedBody).toContain(
|
||||
"https://git.example.com/test-owner/test-repo/tree/claude/fix-42",
|
||||
);
|
||||
});
|
||||
|
||||
test("uses Forgejo comment body with Unicode spinner", async () => {
|
||||
let capturedBody = "";
|
||||
const mockOctokits = createMockOctokits({
|
||||
updateComment: async (args: any) => {
|
||||
capturedBody = args.body;
|
||||
return { data: { id: 1, html_url: "", updated_at: "" } };
|
||||
},
|
||||
});
|
||||
|
||||
const context = createMockContext();
|
||||
await updateTrackingComment(mockOctokits, context, 1);
|
||||
|
||||
expect(capturedBody).toContain("⏳");
|
||||
expect(capturedBody).not.toContain("user-attachments/assets");
|
||||
});
|
||||
});
|
||||
|
||||
describe("comment type routing", () => {
|
||||
test("uses issue comment update for non-review events", async () => {
|
||||
let issueUpdateCalled = false;
|
||||
let reviewUpdateCalled = false;
|
||||
const mockOctokits = createMockOctokits({
|
||||
updateComment: async () => {
|
||||
issueUpdateCalled = true;
|
||||
return { data: { id: 1, html_url: "", updated_at: "" } };
|
||||
},
|
||||
updateReviewComment: async () => {
|
||||
reviewUpdateCalled = true;
|
||||
return { data: { id: 1, html_url: "", updated_at: "" } };
|
||||
},
|
||||
});
|
||||
|
||||
const context = createMockContext({ eventName: "issue_comment" });
|
||||
await updateTrackingComment(mockOctokits, context, 1);
|
||||
|
||||
expect(issueUpdateCalled).toBe(true);
|
||||
expect(reviewUpdateCalled).toBe(false);
|
||||
});
|
||||
|
||||
test("uses PR review comment update for review comment events", async () => {
|
||||
let issueUpdateCalled = false;
|
||||
let reviewUpdateCalled = false;
|
||||
const mockOctokits = createMockOctokits({
|
||||
updateComment: async () => {
|
||||
issueUpdateCalled = true;
|
||||
return { data: { id: 1, html_url: "", updated_at: "" } };
|
||||
},
|
||||
updateReviewComment: async () => {
|
||||
reviewUpdateCalled = true;
|
||||
return { data: { id: 1, html_url: "", updated_at: "" } };
|
||||
},
|
||||
});
|
||||
|
||||
const context = createMockContext({
|
||||
eventName: "pull_request_review_comment",
|
||||
isPR: true,
|
||||
});
|
||||
await updateTrackingComment(mockOctokits, context, 1);
|
||||
|
||||
expect(reviewUpdateCalled).toBe(true);
|
||||
expect(issueUpdateCalled).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("error handling", () => {
|
||||
test("throws and logs error when update fails", async () => {
|
||||
const mockOctokits = createMockOctokits({
|
||||
updateComment: async () => {
|
||||
throw new Error("Update failed");
|
||||
},
|
||||
});
|
||||
|
||||
const context = createMockContext();
|
||||
await expect(
|
||||
updateTrackingComment(mockOctokits, context, 1),
|
||||
).rejects.toThrow("Update failed");
|
||||
expect(consoleErrorSpy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test("logs correct comment type label on success", async () => {
|
||||
const mockOctokits = createMockOctokits();
|
||||
|
||||
const issueContext = createMockContext({ eventName: "issue_comment" });
|
||||
await updateTrackingComment(mockOctokits, issueContext, 42);
|
||||
|
||||
expect(consoleLogSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("issue"),
|
||||
);
|
||||
expect(consoleLogSpy).toHaveBeenCalledWith(expect.stringContaining("42"));
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user