feat(api): repository_dispatch + AI analysis callback (#805) - #814
Merged
Conversation
Wire up Epic #616 Phase 2: when the Sentry webhook upserts a brand-new sentry_issue_id, fire-and-forget a GitHub `repository_dispatch` (`event_type: analyze-error`) so a downstream Actions workflow can run AI analysis on the error. Add a callback endpoint `PUT /api/webhooks/github/ai-result/:id` for the workflow to write back ai_summary, ai_suspected_files, ai_root_cause, ai_suggested_fix, and severity, authenticated via the GitHub App installation token. - New `lib/githubAppAuth.ts`: App JWT (RS256) → installation token with in-memory caching, repository_dispatch trigger, installation-token verification. - New `routes/webhooks/githubAiCallback.ts`: PUT endpoint mounted outside the admin gate; validates Bearer installation tokens via the GitHub API and rejects non-matching installation IDs. - `services/apiErrorService.ts`: `updateAiAnalysis` helper with boundary validation for severity / suspected-files shape. - `routes/webhooks/sentry.ts`: pre-upsert SELECT to detect first-sight, then fire dispatch only on isNew=true. Failures are logged, never thrown — issue #805 acceptance criterion: API stays functional even when the Actions workflow is not deployed yet. - Adds `GITHUB_APP_ID`, `GITHUB_APP_PRIVATE_KEY`, `GITHUB_APP_INSTALLATION_ID`, `GITHUB_DISPATCH_REPOSITORY` env vars. - 25 new Vitest cases covering token cache, dispatch flow, callback auth/validation, and isNew/recurrence branching. Closes #805
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds GitHub App configuration and auth utilities, a PUT webhook to accept AI analysis callbacks authenticated by installation tokens, repository_dispatch triggering for new Sentry issues, AI-analysis update service and validations, related tests, example env entries, and a gitleaks ignore entry. ChangesGitHub App Authentication & AI Analysis Integration
Sequence DiagramsequenceDiagram
participant Sentry as Sentry Webhook
participant API as Server API
participant DB as Database
participant GH as GitHub REST API
participant Actions as GitHub Actions
Sentry->>API: POST /api/webhooks/sentry (signed)
API->>API: verify signature
API->>DB: getApiErrorBySentryIssueId(summary.sentryIssueId)
DB-->>API: existing/null
alt isNew = true
API->>API: create App JWT
API->>GH: POST /app/installations/{id}/access_tokens (App JWT)
GH-->>API: 200 + installation token
API->>GH: POST /repos/{owner}/{repo}/dispatches (event_type: "analyze-error")
GH-->>API: 2xx
end
API->>DB: upsertFromSentrySummary(...)
DB-->>API: upsertedRow
API-->>Sentry: 200 {received: true}
Actions->>API: PUT /api/webhooks/github/ai-result/:id (Bearer installation token + payload)
API->>GH: GET /installation (Authorization: token) [5s timeout]
GH-->>API: 200 + installation id
API->>API: validate payload -> normalize
API->>DB: updateAiAnalysis(id, ...)
DB-->>API: updatedRow
API-->>Actions: 200 {data: updatedRow}
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly Related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements Phase 2 of the AI error analysis (Epic #616), enabling the system to trigger GitHub Actions workflows for error analysis and receive results via a new callback endpoint. Key changes include the addition of GitHub App authentication helpers, the githubAiCallback route, and updates to the Sentry webhook to initiate the analysis process. Feedback focuses on a security improvement for installation token verification, correcting a misleading response key in the callback route, and cleaning up an unused import.
Comment on lines
+245
to
+271
| const res = await fetch(`${GITHUB_API_BASE}/installation/repositories?per_page=1`, { | ||
| headers: { | ||
| Authorization: `Bearer ${token}`, | ||
| Accept: GITHUB_ACCEPT, | ||
| "X-GitHub-Api-Version": GITHUB_API_VERSION, | ||
| "User-Agent": USER_AGENT, | ||
| }, | ||
| signal: controller.signal, | ||
| }); | ||
| if (!res.ok) return false; | ||
| const json = (await res.json().catch(() => null)) as { | ||
| total_count?: unknown; | ||
| repositories?: unknown; | ||
| } | null; | ||
| if (!json) return false; | ||
| // GitHub returns the installation id only via the `installation` field on | ||
| // some endpoints. `/installation/repositories` does not echo it, so we | ||
| // additionally call `/installation` via the token to confirm the binding. | ||
| // Simpler: reconcile by header `x-github-installation-id` when provided, | ||
| // otherwise rely on the App-id-bound listing matching our installation. | ||
| const headerId = res.headers.get("x-github-installation-id"); | ||
| if (headerId !== null) { | ||
| return headerId === installationId; | ||
| } | ||
| // Fallback: the listing succeeded under our App private key's installation, | ||
| // so accept the token. (We've already verified our App config matches.) | ||
| return true; |
There was a problem hiding this comment.
The current implementation of verifyInstallationToken relies on the x-github-installation-id header or falls back to true if the header is missing. This is a security risk because res.ok only confirms the token is valid for some installation of the App, not necessarily ours. An attacker with a token from a different installation of the same App could bypass this check.
A more robust and secure approach is to use the GET /installation endpoint, which is specifically designed for an installation token to retrieve its own metadata, including its ID. This avoids relying on potentially missing headers and provides a definitive verification against our configured installationId.
Suggested change
| const res = await fetch(`${GITHUB_API_BASE}/installation/repositories?per_page=1`, { | |
| headers: { | |
| Authorization: `Bearer ${token}`, | |
| Accept: GITHUB_ACCEPT, | |
| "X-GitHub-Api-Version": GITHUB_API_VERSION, | |
| "User-Agent": USER_AGENT, | |
| }, | |
| signal: controller.signal, | |
| }); | |
| if (!res.ok) return false; | |
| const json = (await res.json().catch(() => null)) as { | |
| total_count?: unknown; | |
| repositories?: unknown; | |
| } | null; | |
| if (!json) return false; | |
| // GitHub returns the installation id only via the `installation` field on | |
| // some endpoints. `/installation/repositories` does not echo it, so we | |
| // additionally call `/installation` via the token to confirm the binding. | |
| // Simpler: reconcile by header `x-github-installation-id` when provided, | |
| // otherwise rely on the App-id-bound listing matching our installation. | |
| const headerId = res.headers.get("x-github-installation-id"); | |
| if (headerId !== null) { | |
| return headerId === installationId; | |
| } | |
| // Fallback: the listing succeeded under our App private key's installation, | |
| // so accept the token. (We've already verified our App config matches.) | |
| return true; | |
| const res = await fetch(`${GITHUB_API_BASE}/installation`, { | |
| headers: { | |
| Authorization: `Bearer ${token}`, | |
| Accept: GITHUB_ACCEPT, | |
| "X-GitHub-Api-Version": GITHUB_API_VERSION, | |
| "User-Agent": USER_AGENT, | |
| }, | |
| signal: controller.signal, | |
| }); | |
| if (!res.ok) return false; | |
| const json = (await res.json().catch(() => null)) as { | |
| id?: number; | |
| } | null; | |
| // Verify that the token belongs to our specific installation ID. | |
| // This prevents tokens from other installations of the same App from being used. | |
| return json?.id?.toString() === installationId; |
| import { HTTPException } from "hono/http-exception"; | ||
| import { | ||
| ApiErrorValidationError, | ||
| getApiErrorBySentryIssueId, |
There was a problem hiding this comment.
This import is no longer needed if the optimization to detect new issues via the upsert result is applied.
Suggested change
| getApiErrorBySentryIssueId, | |
| upsertFromSentrySummary, |
| console.log( | ||
| `[github-ai-callback] updated api_error=${updated.id} severity=${updated.severity}`, | ||
| ); | ||
| return c.json({ error: updated }); |
There was a problem hiding this comment.
Using the key error for a successful response is misleading and inconsistent with other routes in the codebase. It should be changed to a more standard key like data or result.
Suggested change
| return c.json({ error: updated }); | |
| return c.json({ data: updated }); |
Comment on lines
+216
to
+218
| const body = (await res.json()) as { error: { id: string; severity: string } }; | ||
| expect(body.error.id).toBe(VALID_UUID); | ||
| expect(body.error.severity).toBe("high"); |
There was a problem hiding this comment.
Updating the test expectation to match the improved response key in the route handler.
Suggested change
| const body = (await res.json()) as { error: { id: string; severity: string } }; | |
| expect(body.error.id).toBe(VALID_UUID); | |
| expect(body.error.severity).toBe("high"); | |
| const body = (await res.json()) as { data: { id: string; severity: string } }; | |
| expect(body.data.id).toBe(VALID_UUID); | |
| expect(body.data.severity).toBe("high"); |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d10d11f5fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+269
to
+271
| // Fallback: the listing succeeded under our App private key's installation, | ||
| // so accept the token. (We've already verified our App config matches.) | ||
| return true; |
There was a problem hiding this comment.
verifyInstallationToken returns true whenever GitHub does not include x-github-installation-id, so the callback accepts any token that can get a 2xx from /installation/repositories instead of requiring a match with GITHUB_APP_INSTALLATION_ID. This endpoint is the only auth gate for PUT /api/webhooks/github/ai-result/:id, so in environments where that header is omitted an unrelated installation token can write AI fields into arbitrary api_errors rows.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
server/api/src/__tests__/routes/webhooks/githubAiCallback.test.ts (2)
193-194: ⚡ Quick winKeep inline comments bilingual for guideline compliance.
These inline comments are English-only; add corresponding Japanese lines to stay consistent with the repository documentation policy.
As per coding guidelines
**/*.{ts,tsx,js,md}/**/*.{ts,tsx,js,jsx,json,md}:Comments and documentation should include both Japanese and English.Also applies to: 268-269
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/api/src/__tests__/routes/webhooks/githubAiCallback.test.ts` around lines 193 - 194, The inline comment describing "updateAiAnalysis issues a single update chain that resolves to [row]." is English-only; add a matching Japanese translation directly beneath or above it (same style) to comply with bilingual policy, and do the same for the other comment at lines referenced (around the same test block near createMockDb usage and the comment near lines 268-269). Locate the comments around the test that invokes createMockDb and the mention of updateAiAnalysis and insert equivalent Japanese sentences mirroring the English text.
36-46: ⚡ Quick winAdd explicit return types on test helpers for strict TypeScript consistency.
createAppandstubVerifyInstallationTokencurrently rely on inferred returns. Please annotate return types explicitly to align with the repo TS rule and keep test helpers self-documenting.Proposed change
-function createApp(dbResults: unknown[]) { +function createApp( + dbResults: unknown[], +): { app: Hono<AppEnv>; chains: ReturnType<typeof createMockDb>["chains"] } { const { db, chains } = createMockDb(dbResults); const app = new Hono<AppEnv>(); app.onError(errorHandler); @@ return { app, chains }; } -function stubVerifyInstallationToken(result: boolean | (() => Promise<boolean>)) { - return vi.doMock("../../../lib/githubAppAuth.js", async () => { +function stubVerifyInstallationToken(result: boolean | (() => Promise<boolean>)): void { + vi.doMock("../../../lib/githubAppAuth.js", async () => { const actual = await vi.importActual<typeof import("../../../lib/githubAppAuth.js")>( "../../../lib/githubAppAuth.js", ); @@ - }); + }); }As per coding guidelines
**/*.{ts,tsx}:TypeScript strict mode; any is forbidden, explicitly declare types.Also applies to: 54-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/api/src/__tests__/routes/webhooks/githubAiCallback.test.ts` around lines 36 - 46, createApp and stubVerifyInstallationToken lack explicit return types; add precise TypeScript return annotations to satisfy strict mode. For createApp, annotate the function to return an object with the Hono app and the chains type (e.g., { app: Hono<AppEnv>; chains: /* use ReturnType<typeof createMockDb>['chains'] or the concrete chains type */ }). For stubVerifyInstallationToken, add the concrete stub/mocking return type (e.g., SinonStub / JestMock / ReturnType<typeof vi.fn> as appropriate for your test runner). Update both function signatures to use those explicit return types and remove any use of unknown/any in their returns.server/api/src/lib/githubAppAuth.test.ts (1)
114-115: ⚡ Quick winMake these inline comments bilingual to match repo policy.
Please add Japanese counterparts for these English-only comments.
As per coding guidelines
**/*.{ts,tsx,js,md}/**/*.{ts,tsx,js,jsx,json,md}:Include both Japanese and English comments/documentation in code and documentation files.Also applies to: 142-143
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/api/src/lib/githubAppAuth.test.ts` around lines 114 - 115, Update the inline English-only comments near the assertion expect(fetchMock).toHaveBeenCalledTimes(1) in githubAppAuth.test.ts to include a Japanese translation immediately adjacent (e.g., add a Japanese sentence/comment that mirrors "Cached: only one network call, despite two callers."), and do the same for the other English-only comment referenced at lines 142-143; locate the comments by searching around the test function that uses fetchMock and the test names related to caching or network calls (symbols: fetchMock, toHaveBeenCalledTimes) and add corresponding Japanese comments following the repo's bilingual comment guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/api/src/lib/githubAppAuth.ts`:
- Around line 239-277: The current fallback in verifyInstallationToken returns
true when the x-github-installation-id header is missing, which is insecure;
replace that fallback by making a second fetch to GET
`${GITHUB_API_BASE}/installation` using the same Authorization, Accept,
"X-GitHub-Api-Version" and User-Agent headers (reuse controller.signal and
timeout logic), parse the JSON response to read the installation.id and compare
it to installationId from readAppConfig(), returning true only if they match
(handle non-OK responses and JSON parse failures by returning false), and ensure
the existing timer is cleared in finally as before.
In `@server/api/src/routes/webhooks/githubAiCallback.ts`:
- Around line 155-158: The response is returning the successful updated row
under the misleading key error; change the c.json call in the githubAiCallback
handler to return the updated record as a successful payload (e.g. c.json({
data: updated }) or simply c.json(updated)) instead of c.json({ error: updated
}) so consumers receive a correct success key; update the log or tests if they
expect the old shape.
---
Nitpick comments:
In `@server/api/src/__tests__/routes/webhooks/githubAiCallback.test.ts`:
- Around line 193-194: The inline comment describing "updateAiAnalysis issues a
single update chain that resolves to [row]." is English-only; add a matching
Japanese translation directly beneath or above it (same style) to comply with
bilingual policy, and do the same for the other comment at lines referenced
(around the same test block near createMockDb usage and the comment near lines
268-269). Locate the comments around the test that invokes createMockDb and the
mention of updateAiAnalysis and insert equivalent Japanese sentences mirroring
the English text.
- Around line 36-46: createApp and stubVerifyInstallationToken lack explicit
return types; add precise TypeScript return annotations to satisfy strict mode.
For createApp, annotate the function to return an object with the Hono app and
the chains type (e.g., { app: Hono<AppEnv>; chains: /* use ReturnType<typeof
createMockDb>['chains'] or the concrete chains type */ }). For
stubVerifyInstallationToken, add the concrete stub/mocking return type (e.g.,
SinonStub / JestMock / ReturnType<typeof vi.fn> as appropriate for your test
runner). Update both function signatures to use those explicit return types and
remove any use of unknown/any in their returns.
In `@server/api/src/lib/githubAppAuth.test.ts`:
- Around line 114-115: Update the inline English-only comments near the
assertion expect(fetchMock).toHaveBeenCalledTimes(1) in githubAppAuth.test.ts to
include a Japanese translation immediately adjacent (e.g., add a Japanese
sentence/comment that mirrors "Cached: only one network call, despite two
callers."), and do the same for the other English-only comment referenced at
lines 142-143; locate the comments by searching around the test function that
uses fetchMock and the test names related to caching or network calls (symbols:
fetchMock, toHaveBeenCalledTimes) and add corresponding Japanese comments
following the repo's bilingual comment guideline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aeda8b91-c093-40db-b077-6a52128c9308
📒 Files selected for processing (10)
server/api/.env.exampleserver/api/src/__tests__/routes/webhooks/githubAiCallback.test.tsserver/api/src/__tests__/routes/webhooks/sentry.test.tsserver/api/src/app.tsserver/api/src/lib/githubAppAuth.test.tsserver/api/src/lib/githubAppAuth.tsserver/api/src/routes/webhooks/githubAiCallback.tsserver/api/src/routes/webhooks/sentry.tsserver/api/src/services/apiErrorService.test.tsserver/api/src/services/apiErrorService.ts
Address review feedback on PR #814: 1. P1 security: `verifyInstallationToken` previously fell back to `true` whenever `x-github-installation-id` was missing — and that header is not actually documented as a response of `GET /installation/repositories`, so the fallback was always taken. This let any installation token of the same App write AI fields into arbitrary `api_errors` rows. Switch to `GET /installation`, which returns the installation's own metadata (including `id`), and require the `id` to equal `GITHUB_APP_INSTALLATION_ID`. Tokens for other installations of the same App now fail closed. 2. Security CI (gitleaks): the test file embedded a real-looking RSA PEM to drive `createAppJWT`'s signing path, which gitleaks rightly flagged. Replace it with a `jose` module mock (a constructible stub class for `SignJWT` + a no-op `importPKCS8`) so the test no longer ships a key. Adds 6 new tests covering the new `verifyInstallationToken` branches (matching id, mismatched id, non-2xx, malformed body, missing id field, empty token).
The Security CI scans full git history, not just HEAD. Commit d10d11f (in PR #814) included a disposable RSA test key that f6825d7 later removed by switching to a `jose` module mock. The blob still exists in the branch's git history, so gitleaks keeps flagging it. Add the fingerprint to .gitleaksignore — same pattern the repo already uses for test-only mock secrets (see the invite.test.ts entry). The key was never used to sign anything outside the test process, so this is a true false-positive at this point. Verified locally: `gitleaks detect --log-opts="develop..HEAD"` reports 0 leaks after the suppression.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
server/api/src/lib/githubAppAuth.ts (1)
119-155: ⚡ Quick winThe cache still stampedes on concurrent misses.
The cache check happens before any
await, so two requests that arrive with an empty or near-expiry cache will both mint a JWT and call/access_tokens. That defeats the “avoid stampede” goal and can fan out token exchanges under bursts of new Sentry issues.🔁 Single-flight refresh pattern
let cachedInstallationToken: CachedInstallationToken | null = null; +let installationTokenInFlight: Promise<string> | null = null; export async function getInstallationToken(): Promise<string> { if ( cachedInstallationToken && cachedInstallationToken.expiresAt - Date.now() > REFRESH_MARGIN_MS ) { return cachedInstallationToken.token; } + if (installationTokenInFlight) { + return installationTokenInFlight; + } - const { installationId } = readAppConfig(); - const jwt = await createAppJWT(); - const res = await fetch( - `${GITHUB_API_BASE}/app/installations/${encodeURIComponent(installationId)}/access_tokens`, - { - method: "POST", - headers: { - Authorization: `Bearer ${jwt}`, - Accept: GITHUB_ACCEPT, - "X-GitHub-Api-Version": GITHUB_API_VERSION, - "User-Agent": USER_AGENT, - }, - }, - ); - if (!res.ok) { - const body = await res.text().catch(() => ""); - throw new Error(`GitHub installation token request failed: ${res.status} ${body}`); - } - const json = (await res.json()) as { token?: unknown; expires_at?: unknown }; - const token = typeof json.token === "string" ? json.token : null; - const expiresAtIso = typeof json.expires_at === "string" ? json.expires_at : null; - if (!token || !expiresAtIso) { - throw new Error("GitHub installation token response missing token/expires_at"); - } - const expiresAt = Date.parse(expiresAtIso); - if (!Number.isFinite(expiresAt)) { - throw new Error(`GitHub installation token returned unparseable expires_at: ${expiresAtIso}`); + installationTokenInFlight = (async () => { + const { installationId } = readAppConfig(); + const jwt = await createAppJWT(); + const res = await fetch( + `${GITHUB_API_BASE}/app/installations/${encodeURIComponent(installationId)}/access_tokens`, + { + method: "POST", + headers: { + Authorization: `Bearer ${jwt}`, + Accept: GITHUB_ACCEPT, + "X-GitHub-Api-Version": GITHUB_API_VERSION, + "User-Agent": USER_AGENT, + }, + }, + ); + if (!res.ok) { + const body = await res.text().catch(() => ""); + throw new Error(`GitHub installation token request failed: ${res.status} ${body}`); + } + const json = (await res.json()) as { token?: unknown; expires_at?: unknown }; + const token = typeof json.token === "string" ? json.token : null; + const expiresAtIso = typeof json.expires_at === "string" ? json.expires_at : null; + if (!token || !expiresAtIso) { + throw new Error("GitHub installation token response missing token/expires_at"); + } + const expiresAt = Date.parse(expiresAtIso); + if (!Number.isFinite(expiresAt)) { + throw new Error(`GitHub installation token returned unparseable expires_at: ${expiresAtIso}`); + } + cachedInstallationToken = { token, expiresAt }; + return token; + })(); + try { + return await installationTokenInFlight; + } finally { + installationTokenInFlight = null; } - cachedInstallationToken = { token, expiresAt }; - return token; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/api/src/lib/githubAppAuth.ts` around lines 119 - 155, getInstallationToken currently races on concurrent cache misses because the cache check happens before any await; fix by adding a single-flight in-flight promise (e.g., inflightInstallationTokenPromise) alongside cachedInstallationToken in the module and use it inside getInstallationToken: after checking cachedInstallationToken and before creating a JWT, if inflightInstallationTokenPromise exists return await it; otherwise create and assign a promise that performs the createAppJWT + fetch flow (the existing request/response parsing and validation), set cachedInstallationToken on success, and ensure the inflight promise is cleared on both success and error so subsequent callers can retry; reference getInstallationToken, cachedInstallationToken, createAppJWT and the POST to /app/installations/.../access_tokens.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/api/src/lib/githubAppAuth.ts`:
- Around line 128-139: Both outbound GitHub fetch calls (in getInstallationToken
and triggerRepositoryDispatch) use bare fetch and can hang; wrap each call with
an AbortController-based timeout: create an AbortController, pass its signal
into fetch, set a timer (e.g. 5–10s) to call controller.abort(), and clear the
timer when the response or error arrives. Ensure you handle AbortError the same
as other fetch errors so the .catch/log path runs, and attach the signal to both
fetch invocations referenced in getInstallationToken and
triggerRepositoryDispatch to avoid leaked pending promises.
- Around line 243-274: The verifyInstallationToken function currently returns
false for both invalid tokens and transient GitHub failures; change it so only
genuine auth failures return false and transient/5xx/network/timeout errors
surface as retryable errors. Concretely, in verifyInstallationToken inspect
fetch errors and response.status: if fetch throws due to AbortError or network
error, or if res.status is 500–599, rethrow or throw a specific error (e.g., new
Error('GitHubUnavailable')) instead of returning false; only return false for
401/403/404 or when the id is missing/mismatched. Use the existing symbols
verifyInstallationToken, installationId, GITHUB_API_BASE and the AbortController
to distinguish AbortError from auth failures. Ensure the caller of
verifyInstallationToken handles the thrown error and maps it to a 5xx retryable
response while keeping false -> 403 behavior.
---
Nitpick comments:
In `@server/api/src/lib/githubAppAuth.ts`:
- Around line 119-155: getInstallationToken currently races on concurrent cache
misses because the cache check happens before any await; fix by adding a
single-flight in-flight promise (e.g., inflightInstallationTokenPromise)
alongside cachedInstallationToken in the module and use it inside
getInstallationToken: after checking cachedInstallationToken and before creating
a JWT, if inflightInstallationTokenPromise exists return await it; otherwise
create and assign a promise that performs the createAppJWT + fetch flow (the
existing request/response parsing and validation), set cachedInstallationToken
on success, and ensure the inflight promise is cleared on both success and error
so subsequent callers can retry; reference getInstallationToken,
cachedInstallationToken, createAppJWT and the POST to
/app/installations/.../access_tokens.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6751ed49-a8b5-4a26-80df-c077104bab6e
📒 Files selected for processing (2)
server/api/src/lib/githubAppAuth.test.tsserver/api/src/lib/githubAppAuth.ts
Address two CodeRabbit review findings on PR #814: 1. Add AbortController-based timeouts (10s) to both outbound GitHub fetch calls in `getInstallationToken` and `triggerRepositoryDispatch` via a shared `fetchGitHubWithTimeout` helper. Without this, a stalled GitHub API leaves the fire-and-forget Sentry path with a forever-pending promise; under bursty webhooks those accumulate. 2. Make `verifyInstallationToken` differentiate auth failures from GitHub-side outages. Previously it returned `false` for everything — including 5xx, network errors, timeouts, and malformed JSON — so the callback's existing 503 path never fired. Now: - 401/403/404 + id mismatch + missing id → `false` → route 403 - 5xx, network, abort, malformed JSON → throws `GitHubInstallationVerificationError` → route 503 (retryable) This stops a transient GitHub outage from silently dropping a valid AI result as a permanent auth failure. Skipped the third (nitpick) finding on single-flight cache: the dispatch path runs at most once per new Sentry issue, so concurrent cache misses are extremely rare and benign (last write wins). Adding in-flight tracking is maintenance burden for a non-measurable win. Adds 4 new tests covering 5xx / network / malformed-JSON throws and a 503-from-route case via the callback.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
server/api/src/routes/webhooks/githubAiCallback.ts (1)
157-160:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSuccess response uses misleading
errorkey.Line 160 returns the updated row under the
errorkey, which is semantically incorrect for a success response. Consumers parsingerrorpresence to detect failures will misinterpret this.Proposed fix
- return c.json({ error: updated }); + return c.json({ data: updated });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/api/src/routes/webhooks/githubAiCallback.ts` around lines 157 - 160, The success response is returning the updated row under the misleading key "error"; in the route handler (githubAiCallback) replace the success payload so it doesn't use the "error" key — return the updated record under a clear success key (e.g., "updated" or "data") or use a shape like { error: null, data: updated } and keep the existing console.log that references updated.id/updated.severity; ensure the client-facing JSON uses the new key instead of "error".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@server/api/src/routes/webhooks/githubAiCallback.ts`:
- Around line 157-160: The success response is returning the updated row under
the misleading key "error"; in the route handler (githubAiCallback) replace the
success payload so it doesn't use the "error" key — return the updated record
under a clear success key (e.g., "updated" or "data") or use a shape like {
error: null, data: updated } and keep the existing console.log that references
updated.id/updated.severity; ensure the client-facing JSON uses the new key
instead of "error".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 991f17a7-8cc0-49ef-a5fa-3291d83e799f
📒 Files selected for processing (4)
server/api/src/__tests__/routes/webhooks/githubAiCallback.test.tsserver/api/src/lib/githubAppAuth.test.tsserver/api/src/lib/githubAppAuth.tsserver/api/src/routes/webhooks/githubAiCallback.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- server/api/src/lib/githubAppAuth.test.ts
- server/api/src/tests/routes/webhooks/githubAiCallback.test.ts
Switch the AI callback's success payload from `{ error: updated }` to
`{ data: updated }`. The admin api_errors routes use `{ error: row }`
because their consumer (admin/src/api/admin.ts) is internal and tightly
coupled to that shape, but this webhook is consumed by external GitHub
Actions workflows where reusing the `error` key for success makes
"presence-of-error means failure" no longer hold and is genuinely
confusing.
No backward-compat concern: the AI workflow that calls this endpoint
has not been written yet, so there is no existing consumer to break.
Test updated to expect the new shape.
This was referenced May 5, 2026
This was referenced May 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wire up Epic #616 Phase 2: when the Sentry webhook upserts a brand-new
sentry_issue_id, fire-and-forget a GitHub
repository_dispatch(
event_type: analyze-error) so a downstream Actions workflow can runAI analysis on the error. Add a callback endpoint
PUT /api/webhooks/github/ai-result/:idfor the workflow to write backai_summary, ai_suspected_files, ai_root_cause, ai_suggested_fix, and
severity, authenticated via the GitHub App installation token.
lib/githubAppAuth.ts: App JWT (RS256) → installation token within-memory caching, repository_dispatch trigger, installation-token
verification.
routes/webhooks/githubAiCallback.ts: PUT endpoint mountedoutside the admin gate; validates Bearer installation tokens via the
GitHub API and rejects non-matching installation IDs.
services/apiErrorService.ts:updateAiAnalysishelper withboundary validation for severity / suspected-files shape.
routes/webhooks/sentry.ts: pre-upsert SELECT to detect first-sight,then fire dispatch only on isNew=true. Failures are logged, never
thrown — issue feat(api): repository_dispatch 発火と AI 解析結果コールバック API (Epic #616 Phase 2) #805 acceptance criterion: API stays functional even
when the Actions workflow is not deployed yet.
GITHUB_APP_ID,GITHUB_APP_PRIVATE_KEY,GITHUB_APP_INSTALLATION_ID,GITHUB_DISPATCH_REPOSITORYenv vars.auth/validation, and isNew/recurrence branching.
Closes #805
Summary by CodeRabbit
New Features
Tests
Chores