From c0660f0c6e065550acda0f4ae96b5be142cce654 Mon Sep 17 00:00:00 2001 From: Kenil Shah Date: Fri, 26 Jun 2026 18:20:57 +0530 Subject: [PATCH 1/3] feat: add GitHub App installation and support for dynamic installation ID --- README.md | 116 +++++++++++----- .../migration.sql | 5 + coverage-service/api/prisma/schema.prisma | 19 +-- coverage-service/api/query-redis.js | 53 ++++++++ .../api/src/github/github.module.ts | 3 + .../api/src/github/github.service.ts | 8 +- .../src/repositories/repositories.service.ts | 33 +++++ .../api/src/webhooks/webhooks.controller.ts | 6 + .../api/src/webhooks/webhooks.service.ts | 61 +++++++++ .../lib/src/providers/github-provider.ts | 68 ++++++---- coverage-service/worker/src/lib/providers.ts | 16 ++- package.json | 3 + pnpm-lock.yaml | 23 ++++ service/.env.example | 16 ++- service/package.json | 2 + service/src/config.ts | 18 ++- .../src/dispatch/coverage-webhook-forward.ts | 49 +++++++ service/src/github/auth.ts | 96 ++++++++++++- service/src/routes/webhook.ts | 127 +++++++++++------- service/src/web.ts | 4 +- service/src/worker.ts | 4 +- 21 files changed, 599 insertions(+), 131 deletions(-) create mode 100644 coverage-service/api/prisma/migrations/20250625000000_add_github_installation_id/migration.sql create mode 100644 coverage-service/api/query-redis.js create mode 100644 service/src/dispatch/coverage-webhook-forward.ts diff --git a/README.md b/README.md index 0199cfd..2258678 100644 --- a/README.md +++ b/README.md @@ -200,17 +200,19 @@ OpenReview then takes those generated files, **opens a stacked PR** against the ```text ┌────────────────────────────────────────────┐ -GitHub PR ─► webhook OR curl ─►│ OpenReview service │ - │ • POST /webhook (webhook path) │ - │ • POST /coverage-runs/trigger (curl path)│ - └─────────────┬──────────────────────────────┘ - │ POST /repositories - │ POST /repositories/:id/analyze - │ GET /pr-runs/:id (polled) - ▼ +GitHub App ──webhook─────────►│ OpenReview service (:3003) │ + pull_request │ • POST /webhook │ + installation* │ • POST /webhooks/github (alias) │ + │ • POST /coverage-runs/trigger (manual) │ + │ → review-fast + coverage-analysis jobs │ + └──────┬─────────────────────┬───────────────┘ + │ installation* │ HTTP (localhost) + │ forwarded │ POST /repositories/:id/analyze + ▼ ▼ ┌────────────────────────────────────────────┐ - │ coverage-service (NestJS API + BullMQ worker)│ - │ clone → install → c8 → diff-cover → │ + │ coverage-service (:3010) │ + │ Nest API + BullMQ worker │ + │ clone → install → c8 → diff-cover → │ │ LLM test gen → node --test → re-cover │ └─────────────┬──────────────────────────────┘ │ generatedTestFiles[] @@ -222,6 +224,10 @@ GitHub PR ─► webhook OR curl ─►│ OpenReview service │ • open stacked PR → feature branch │ │ • post coverage-delta comment on PR │ └────────────────────────────────────────────┘ + +* `installation` / `installation_repositories` events received by OpenReview + are forwarded to coverage-service so per-repo `githubInstallationId` values + are stored automatically. ``` ### Prerequisites @@ -233,10 +239,33 @@ GitHub PR ─► webhook OR curl ─►│ OpenReview service | **Postgres** | Coverage-service Prisma DB (`prcoverage`) | `createdb prcoverage` (or your DB UI) | | **Redis** | BullMQ for both services (queue names are disjoint, safe to share) | `brew services start redis` / `docker run redis` | | **Python 3** + **diff-cover** | The coverage worker shells out to it | `pnpm coverage:setup:worker-deps` (creates `coverage-service/worker/.venv-tools/`) | -| **GitHub PAT** | One token used by every service for clone + comment + PR-author | See **PAT scopes** below | +| **GitHub App** (recommended) | Posts reviews and coverage comments as your bot (`deuex-reviewer[bot]`), not a personal account | Install the app on target repos; set `GITHUB_AUTH_MODE=app` in `service/.env` and `coverage-service/.env` (see below) | +| **GitHub PAT** (fallback) | Local dev without a GitHub App installation | Classic `ghp_…` token with `repo` scope, or fine-grained with Contents + Pull requests read/write | | **OpenAI key** | LLM that writes the tests (Anthropic also supported) | Paste into `coverage-service/.env` → `OPENAI_API_KEY=` | -#### GitHub PAT scopes (critical — this is the most common failure) +#### GitHub App setup (recommended for self-hosted bot) + +Install your GitHub App on the repos you want to review. Configure **both** env files with the same app credentials: + +```bash +# service/.env — OpenReview web + worker (posts PR comments) +GITHUB_AUTH_MODE=app +GITHUB_APP_ID= +GITHUB_APP_PRIVATE_KEY="-----BEGIN RSA PRIVATE KEY-----\n...\n-----END RSA PRIVATE KEY-----\n" +# Leave blank to resolve per-repo via GitHub API; set only for single-org dev. +# GITHUB_APP_INSTALLATION_ID= + +# coverage-service/.env — clone + analyze PRs +GITHUB_AUTH_MODE=app +GITHUB_APP_ID= +GITHUB_APP_PRIVATE_KEY= +``` + +Webhook secret must match across the GitHub App settings, `service/.env` (`GITHUB_WEBHOOK_SECRET`), and `coverage-service/.env` (`WEBHOOK_SECRET`). + +When `GITHUB_AUTH_MODE=app`, comments and stacked PRs are attributed to the **app bot**, not your personal GitHub account. + +#### GitHub PAT scopes (when using `GITHUB_AUTH_MODE=pat`) OpenReview's PR-author path needs to create git blobs → trees → commits → refs. The review-only path (just commenting on a PR) only needs `pull-requests: write`, but **opening a stacked PR additionally needs `contents: write`**. @@ -246,7 +275,7 @@ OpenReview's PR-author path needs to create git blobs → trees → commits → - Metadata is auto-granted. - **Classic PAT** (`ghp_…`): the single `repo` scope covers everything we need. Easiest if you don't want to fiddle with fine-grained permissions. -After regenerating the token, **the same value must be in all three `.env` files** (the rotated token will 401 wherever it isn't updated): +After regenerating a PAT, **the same value must be in all three `.env` files** (the rotated token will 401 wherever it isn't updated). Skip this when using GitHub App auth (`GITHUB_AUTH_MODE=app`). ``` .env # root — used by @openreview/core (CLI / action) @@ -254,6 +283,8 @@ service/.env # OpenReview service (web + worker) coverage-service/.env # coverage-service (api + worker) ``` +Only needed when `GITHUB_AUTH_MODE=pat` in the service / coverage env files. + ### One-time setup ```bash @@ -262,9 +293,9 @@ pnpm install pnpm build # 2. Configure env files (copy templates, paste your secrets) -cp .env.example .env # GITHUB_PAT, OPENAI_API_KEY, ... -cp service/.env.example service/.env # add the same GITHUB_PAT -cp coverage-service/.env.example coverage-service/.env # add the same GITHUB_PAT + OPENAI_API_KEY +cp .env.example .env # OPENAI_API_KEY, ... +cp service/.env.example service/.env # GITHUB_WEBHOOK_SECRET, GITHUB_AUTH_MODE=app, app creds +cp coverage-service/.env.example coverage-service/.env # WEBHOOK_SECRET (same as above), GITHUB_AUTH_MODE=app, OPENAI_API_KEY # 3. Generate the Prisma client + run database migrations pnpm coverage:db:generate @@ -279,17 +310,19 @@ pnpm coverage:setup:worker-deps # COVERAGE_SERVICE_URL=http://localhost:3010 ``` -### Run the stack (5 terminals) +### Run the stack (4–5 terminals) | # | Command | Binds | Role | |---|---|---|---| -| 1 | `pnpm coverage:dev:api` | `:3010` | Coverage-service HTTP API (Nest) | -| 2 | `pnpm coverage:dev:worker` | (none) | Coverage-service worker — does the actual clone + coverage + LLM work | -| 3 | `pnpm --filter @openreview/service start:web` | `:3003` | OpenReview HTTP — webhooks + `/coverage-runs/trigger` | -| 4 | `pnpm --filter @openreview/service start:worker` | (none) | OpenReview worker — opens the stacked PR + posts the comment | -| 5 | `cd web && pnpm dev` | `:3000` | Next.js Coverage & Cost Monitoring Dashboard | +| 1 | `pnpm coverage:dev:api` | `:3010` | Coverage-service HTTP API (Nest) — **internal**, not the public webhook target | +| 2 | `pnpm coverage:dev:worker` | (none) | Coverage-service worker — clone + coverage + LLM test generation | +| 3 | `pnpm --filter @openreview/service start:web` | `:3003` | OpenReview HTTP — **GitHub App webhook entrypoint** + `/coverage-runs/trigger` | +| 4 | `pnpm --filter @openreview/service start:worker` | (none) | OpenReview worker — review, stacked PR, coverage comment | +| 5 | `cd web && pnpm dev` | `:3000` | Next.js Coverage & Cost Monitoring Dashboard (optional) | -> Ports are configurable. `:3010` is `API_PORT` in `coverage-service/.env`; `:3003` is `PORT` in `service/.env`. +> **Ports:** `:3010` = `API_PORT` in `coverage-service/.env`; `:3003` = `PORT` in `service/.env`. +> +> **Local tunnel:** point ngrok (or similar) at **`:3003`**, not `:3010`. Example: `ngrok http 3003` → set the GitHub App webhook URL to `https:///webhooks/github`. ### Verified end-to-end flow (curl path, no webhook needed) @@ -343,16 +376,27 @@ On GitHub: - A `Test file | Covers | Status` table mapping each generated `filePath` to its `targetFile` with ✅ / ❌ / — for the test execution result. - A `[INFO] OpenReview — Coverage` summary comment on the original PR. -### Webhook path (alternative — also supported) +### Webhook path (GitHub App — recommended) + +Point your **GitHub App** webhook at OpenReview on **port 3003** (not coverage-service on 3010): + +| Setting | Value | +| --- | --- | +| **Payload URL** | `https:///webhooks/github` or `https://:3003/webhook` | +| **Content type** | `application/json` | +| **Secret** | Same string in GitHub App settings, `service/.env` → `GITHUB_WEBHOOK_SECRET`, and `coverage-service/.env` → `WEBHOOK_SECRET` | +| **Events** | `Pull requests` (`opened`, `synchronize`, `reopened`, `ready_for_review`) and `Installation` (so coverage-service stores per-repo installation IDs) | + +Both `POST /webhook` and `POST /webhooks/github` are accepted on OpenReview — use whichever matches your GitHub App configuration. + +On each qualifying `pull_request` event, OpenReview enqueues: -If you'd rather skip the curl trigger and have the pipeline fire automatically on every PR, point a GitHub webhook at OpenReview's `/webhook`: +- `review-fast` — gitar-style summary + inline findings (cached when the reviewable diff matches a prior run; see `REVIEW_CACHE_ENABLED` in `service/.env.example`) +- `coverage-analysis` — calls coverage-service, then opens the stacked test PR and posts the coverage comment -- **Payload URL**: `https:///webhook` (or `http://localhost:3003/webhook` over a tunnel) -- **Content type**: `application/json` -- **Secret**: the value of `GITHUB_WEBHOOK_SECRET` in `service/.env` -- **Events**: `Pull requests` (subscribe to at least `opened`, `synchronize`, `reopened`, `ready_for_review`) +**Ignored by design:** `edited` (e.g. CodeRabbit updating the PR body), `closed`, and other non-code events return `200` with `ignored` — push a new commit or redeliver an `opened`/`synchronize` delivery to re-run. -OpenReview will then enqueue a `review-fast` job (posts the inline review) **and** a `coverage-analysis` job (runs the pipeline above) on every PR. The two paths are equivalent; the curl path just lets you trigger on repos that aren't webhook-configured. +The curl path in the previous section remains supported for repos without a webhook. ### `GET /pr-runs/:id` — response fields OpenReview consumes @@ -370,10 +414,14 @@ OpenReview will then enqueue a `review-fast` job (posts the inline review) **and | Symptom | Cause | Fix | | --- | --- | --- | +| Comments appear under your personal account, not the bot | OpenReview is using `GITHUB_AUTH_MODE=pat` (or `GITHUB_PAT` only) | Set `GITHUB_AUTH_MODE=app` with `GITHUB_APP_ID` + `GITHUB_APP_PRIVATE_KEY` in `service/.env`; restart the worker | +| Webhook delivery `200` but `action edited ignored` | PR description was edited (e.g. by another bot), not new code | Push a commit (`synchronize`) or redeliver the `opened` webhook; only `opened` / `synchronize` / `reopened` / `ready_for_review` trigger runs | +| Nothing hits OpenReview web logs; coverage API logs webhooks instead | Tunnel or GitHub App webhook points at `:3010` instead of `:3003` | `ngrok http 3003` and set Payload URL to `https:///webhooks/github` | | `Cannot POST /repositories/.../analyze` (HTML 404) | Hitting the wrong port — coverage-service is on `:3010`, OpenReview on `:3003`. | Use the right port. Confirm with `lsof -iTCP:3010 -sTCP:LISTEN`. | +| `No GitHub App installation ID found for owner/repo` on `POST .../analyze` | Repo not linked to an app installation in the coverage DB | Install the app on the repo; ensure `installation` events reach OpenReview (forwarded to coverage-service), or set `GITHUB_APP_INSTALLATION_ID` | | `Repository not found` on analyze | Wrong `repo-id`. `GET /repositories` returns every registered repo — pick the row with the matching `githubRepo`. | Pipe `POST /repositories` straight into the analyze call (see Step 1 above); avoids retyping the cuid. | -| `GitHub API access forbidden (403) for /git/blobs` | PAT has `pull-requests: write` but not `contents: write`. | Update the fine-grained PAT to **Contents: read & write** (or switch to a classic PAT with `repo`). | -| `GitHub API access denied for owner/repo` (401) on analyze | PAT was rotated and `coverage-service/.env` still has the old string. | Same token must be in all three `.env` files. `grep -H GITHUB_PAT .env service/.env coverage-service/.env`. | +| `GitHub API access forbidden (403) for /git/blobs` | PAT has `pull-requests: write` but not `contents: write`. | Update the fine-grained PAT to **Contents: read & write** (or switch to a classic PAT with `repo`, or use GitHub App auth). | +| `GitHub API access denied for owner/repo` (401) on analyze | PAT was rotated and `coverage-service/.env` still has the old string. | Same token must be in all three `.env` files when using PAT mode. With App auth, verify `GITHUB_APP_PRIVATE_KEY` is correct. | | `ModuleNotFoundError: No module named 'diff_cover'` in the coverage worker | The Python venv wasn't created. | `pnpm coverage:setup:worker-deps` (creates `coverage-service/worker/.venv-tools/`). | | `Custom Id cannot contain :` from BullMQ | Stale jobId format from a pre-fix build. | `pnpm build && pnpm --filter @openreview/service start:worker` to pick up the new code. | | `status: "duplicate"` on `/coverage-runs/trigger` | A job for the same head SHA was already enqueued (and possibly failed). | `redis-cli del 'bull:openreview:coverage-analysis~/#@'` to drop it, then re-trigger. Or push a new commit to bump the head SHA. | @@ -381,8 +429,8 @@ OpenReview will then enqueue a `review-fast` job (posts the inline review) **and ### Configuration reference -- All `COVERAGE_SERVICE_*` settings (timeouts, branch prefix, default coverage/test/install commands) live in [`service/.env.example`](service/.env.example). -- Coverage-service's own settings (DB, Redis, LLM provider, `TEST_THRESHOLD`, `MAX_GENERATION_ATTEMPTS`, `DIFF_COVER_BIN`, worker concurrency) live in [`coverage-service/.env.example`](coverage-service/.env.example). +- OpenReview service settings (webhook secret, **GitHub App auth**, review cache, `COVERAGE_SERVICE_*` timeouts and branch prefix) live in [`service/.env.example`](service/.env.example). +- Coverage-service settings (DB, Redis, LLM provider, `WEBHOOK_SECRET`, `GITHUB_AUTH_MODE`, `TEST_THRESHOLD`, `MAX_GENERATION_ATTEMPTS`, `DIFF_COVER_BIN`, worker concurrency) live in [`coverage-service/.env.example`](coverage-service/.env.example). ## Instruction Files diff --git a/coverage-service/api/prisma/migrations/20250625000000_add_github_installation_id/migration.sql b/coverage-service/api/prisma/migrations/20250625000000_add_github_installation_id/migration.sql new file mode 100644 index 0000000..d69b71a --- /dev/null +++ b/coverage-service/api/prisma/migrations/20250625000000_add_github_installation_id/migration.sql @@ -0,0 +1,5 @@ +-- AlterTable: add optional githubInstallationId to Repository +ALTER TABLE "Repository" ADD COLUMN "githubInstallationId" TEXT; + +-- CreateIndex: allows looking up a repo by its GitHub App installation +CREATE INDEX "Repository_githubInstallationId_idx" ON "Repository"("githubInstallationId"); diff --git a/coverage-service/api/prisma/schema.prisma b/coverage-service/api/prisma/schema.prisma index 2907f33..86376ee 100644 --- a/coverage-service/api/prisma/schema.prisma +++ b/coverage-service/api/prisma/schema.prisma @@ -8,15 +8,16 @@ datasource db { } model Repository { - id String @id @default(cuid()) - githubRepo String @unique - defaultBranch String @default("main") - coverageCommand String @default("npm test -- --coverage") - testCommand String @default("npm test") - installCommand String @default("") - createdAt DateTime @default(now()) - pullRequestRuns PullRequestRun[] - testGenerationRuns TestGenerationRun[] + id String @id @default(cuid()) + githubRepo String @unique + defaultBranch String @default("main") + coverageCommand String @default("npm test -- --coverage") + testCommand String @default("npm test") + installCommand String @default("") + githubInstallationId String? // set automatically when app is installed via webhook + createdAt DateTime @default(now()) + pullRequestRuns PullRequestRun[] + testGenerationRuns TestGenerationRun[] } enum PrRunStatus { diff --git a/coverage-service/api/query-redis.js b/coverage-service/api/query-redis.js new file mode 100644 index 0000000..68f71a7 --- /dev/null +++ b/coverage-service/api/query-redis.js @@ -0,0 +1,53 @@ +const path = require('path'); +const dotenv = require('dotenv'); +const envPath = path.resolve(__dirname, '../.env'); +dotenv.config({ path: envPath }); + +const { Queue } = require('bullmq'); + +async function main() { + const Redis = require('ioredis'); + const redis = new Redis(process.env.REDIS_URL ?? 'redis://localhost:6379'); + console.log('Connecting to Redis via ioredis...'); + + // Get all keys matching pr-analysis + const keys = await redis.keys('*pr-analysis*'); + console.log('Redis keys matching *pr-analysis*:', keys); + + // Get active jobs list in BullMQ structure + // BullMQ uses hashes and sets. Let's get the active jobs from the active zset/list. + // In BullMQ 5, the active list can be retrieved. Or we can just use BullMQ Queue but without the client.get call. + const { Queue } = require('bullmq'); + const connection = { + url: process.env.REDIS_URL ?? 'redis://localhost:6379', + }; + const prAnalysisQueue = new Queue('pr-analysis', { connection }); + + const waitingJobs = await prAnalysisQueue.getWaiting(); + const activeJobs = await prAnalysisQueue.getActive(); + const failedJobs = await prAnalysisQueue.getFailed(); + + console.log('Waiting jobs count:', waitingJobs.length); + console.log('Active jobs count:', activeJobs.length); + for (const j of activeJobs) { + const lockKey = `bull:pr-analysis:${j.id}:lock`; + const lockVal = await redis.get(lockKey); + const lockTtl = await redis.ttl(lockKey); + console.log(`- Job ${j.id}: PR run ${j.data.prRunId}, PR #${j.data.prNumber}`); + console.log(` Processed at: ${j.processedOn ? new Date(j.processedOn).toISOString() : 'N/A'}`); + console.log(` Lock exists: ${!!lockVal} (value: ${lockVal}, TTL: ${lockTtl}s)`); + console.log(` Progress: ${j.progress}`); + } + + console.log('\nFetching connected Redis clients...'); + const clients = await redis.client('list'); + console.log(clients); + + await prAnalysisQueue.close(); + await redis.disconnect(); +} + +main() + .catch((e) => { + console.error('Error querying Redis:', e); + }); diff --git a/coverage-service/api/src/github/github.module.ts b/coverage-service/api/src/github/github.module.ts index e189813..456b85c 100644 --- a/coverage-service/api/src/github/github.module.ts +++ b/coverage-service/api/src/github/github.module.ts @@ -1,9 +1,12 @@ import { Global, Module } from '@nestjs/common'; +import { RepositoriesModule } from '../repositories/repositories.module'; + import { GitHubService } from './github.service'; @Global() @Module({ + imports: [RepositoriesModule], providers: [GitHubService], exports: [GitHubService], }) diff --git a/coverage-service/api/src/github/github.service.ts b/coverage-service/api/src/github/github.service.ts index bb3bc00..abea765 100644 --- a/coverage-service/api/src/github/github.service.ts +++ b/coverage-service/api/src/github/github.service.ts @@ -6,18 +6,22 @@ import { import type { GitHubAuthMode} from '@openreview/coverage-lib'; import { GitHubProvider } from '@openreview/coverage-lib'; +import { RepositoriesService } from '../repositories/repositories.service'; + @Injectable() export class GitHubService { private readonly provider: GitHubProvider; - constructor() { + constructor(private readonly repositories: RepositoriesService) { const authMode = (process.env.GITHUB_AUTH_MODE ?? 'pat') as GitHubAuthMode; this.provider = new GitHubProvider({ authMode, pat: process.env.GITHUB_PAT, appId: process.env.GITHUB_APP_ID, privateKey: process.env.GITHUB_APP_PRIVATE_KEY, - installationId: process.env.GITHUB_APP_INSTALLATION_ID, + installationId: process.env.GITHUB_APP_INSTALLATION_ID || undefined, + resolveInstallationId: (githubRepo) => + this.repositories.resolveInstallationId(githubRepo), }); } diff --git a/coverage-service/api/src/repositories/repositories.service.ts b/coverage-service/api/src/repositories/repositories.service.ts index 853ada1..87141d9 100644 --- a/coverage-service/api/src/repositories/repositories.service.ts +++ b/coverage-service/api/src/repositories/repositories.service.ts @@ -56,4 +56,37 @@ export class RepositoriesService { where: { githubRepo }, }); } + + /** + * Called by the installation webhook handler to store (or clear) the + * GitHub App installation ID for a given repo. Creates the repo record + * automatically when the app is installed on a new repo that has not yet + * been manually registered. + */ + async upsertByGithubRepo( + githubRepo: string, + installationId: string | null, + ) { + return this.prisma.repository.upsert({ + where: { githubRepo }, + update: { githubInstallationId: installationId }, + create: { + githubRepo, + githubInstallationId: installationId, + }, + }); + } + + /** + * Resolves the installation ID for a given repo — used by the worker + * so each repo authenticates with its own installation token. + */ + async resolveInstallationId(githubRepo: string): Promise { + const repo = await this.prisma.repository.findFirst({ + where: { githubRepo }, + select: { githubInstallationId: true }, + }); + return repo?.githubInstallationId ?? null; + } } + diff --git a/coverage-service/api/src/webhooks/webhooks.controller.ts b/coverage-service/api/src/webhooks/webhooks.controller.ts index c5f7af4..9da0151 100644 --- a/coverage-service/api/src/webhooks/webhooks.controller.ts +++ b/coverage-service/api/src/webhooks/webhooks.controller.ts @@ -28,6 +28,12 @@ export class WebhooksController { return this.webhooksService.handlePullRequest(payload); } + // GitHub App installation events — fired when someone installs/uninstalls + // the app or grants/revokes access to individual repos. + if (event === 'installation' || event === 'installation_repositories') { + return this.webhooksService.handleInstallation(payload); + } + return { received: true, event }; } diff --git a/coverage-service/api/src/webhooks/webhooks.service.ts b/coverage-service/api/src/webhooks/webhooks.service.ts index 5270c49..c5f27cf 100644 --- a/coverage-service/api/src/webhooks/webhooks.service.ts +++ b/coverage-service/api/src/webhooks/webhooks.service.ts @@ -13,6 +13,16 @@ interface PullRequestPayload { repository: { full_name: string }; } +interface InstallationPayload { + action: string; // 'created' | 'deleted' | 'added' | 'removed' + installation: { id: number }; + /** Present on 'created' (full install) */ + repositories?: { full_name: string }[]; + /** Present on 'installation_repositories' events (selective add/remove) */ + repositories_added?: { full_name: string }[]; + repositories_removed?: { full_name: string }[]; +} + @Injectable() export class WebhooksService { private readonly logger = new Logger(WebhooksService.name); @@ -46,4 +56,55 @@ export class WebhooksService { baseBranch: pr.pull_request.base.ref, }); } + + /** + * Handles GitHub App installation/uninstallation events. + * + * - `installation` event with action `created`: the app was installed on one + * or more repos. We upsert each repo and store the installation ID. + * - `installation_repositories` event with action `added`: the app was granted + * access to additional repos within an existing installation. + * - `installation` with action `deleted` / `installation_repositories` with + * action `removed`: the app was removed — clear the installation ID. + */ + async handleInstallation(payload: Record) { + const data = payload as unknown as InstallationPayload; + const installationId = String(data.installation.id); + const action = data.action; + + this.logger.log( + `GitHub App installation event: action=${action} installationId=${installationId}`, + ); + + const added: string[] = [ + ...(data.repositories ?? []), + ...(data.repositories_added ?? []), + ].map((r) => r.full_name); + + const removed: string[] = (data.repositories_removed ?? []).map( + (r) => r.full_name, + ); + + if (action === 'deleted') { + // Entire installation removed — we don't know the exact repos from this + // payload variant, so clear by installation ID if we have it. + this.logger.warn( + `App uninstalled for installationId=${installationId}. ` + + `Repos will retain their row but lose the installation ID.`, + ); + // The repositories[] list IS present on 'deleted', re-use added[] logic. + } + + for (const repo of added) { + await this.repositories.upsertByGithubRepo(repo, installationId); + this.logger.log(`Linked installationId=${installationId} → ${repo}`); + } + + for (const repo of removed) { + await this.repositories.upsertByGithubRepo(repo, null); + this.logger.log(`Cleared installationId for ${repo}`); + } + + return { received: true, action, added, removed }; + } } diff --git a/coverage-service/lib/src/providers/github-provider.ts b/coverage-service/lib/src/providers/github-provider.ts index 318dab7..58cfc15 100644 --- a/coverage-service/lib/src/providers/github-provider.ts +++ b/coverage-service/lib/src/providers/github-provider.ts @@ -24,42 +24,54 @@ export interface GitHubProviderConfig { pat?: string; appId?: string; privateKey?: string; + /** Static installation ID (single-org / Option A). Takes priority over the resolver. */ installationId?: string; + /** + * Dynamic resolver for multi-tenant GitHub App (Option C). + * Called with the `owner/repo` string and must return the installation ID + * for that specific repo, or null if unknown. + */ + resolveInstallationId?: (githubRepo: string) => Promise; } export class GitHubProvider implements RepositoryProvider { readonly name = 'github'; - private octokit: Octokit | null = null; constructor(private readonly config: GitHubProviderConfig) {} - private async getOctokit(): Promise { - if (this.octokit) return this.octokit; + private async resolveId(githubRepo?: string): Promise { + // Static ID wins — keeps Option A working with zero changes. + if (this.config.installationId) return this.config.installationId; + if (this.config.resolveInstallationId && githubRepo) { + const id = await this.config.resolveInstallationId(githubRepo); + if (id) return id; + } + + throw new Error( + `No GitHub App installation ID found for ${githubRepo ?? 'unknown repo'}. ` + + 'Install the app on this repo or set GITHUB_APP_INSTALLATION_ID.', + ); + } + + private async getOctokit(githubRepo?: string): Promise { if (this.config.authMode === 'app') { - if ( - !this.config.appId || - !this.config.privateKey || - !this.config.installationId - ) { - throw new Error( - 'GitHub App auth requires appId, privateKey, and installationId', - ); + if (!this.config.appId || !this.config.privateKey) { + throw new Error('GitHub App auth requires appId and privateKey'); } + const installationId = parseInt(await this.resolveId(githubRepo), 10); const auth = createAppAuth({ appId: this.config.appId, privateKey: this.config.privateKey.replace(/\\n/g, '\n'), - installationId: parseInt(this.config.installationId, 10), + installationId, }); - this.octokit = new Octokit({ auth: (await auth({ type: 'installation' })).token }); - } else { - if (!this.config.pat) { - throw new Error('PAT auth requires GITHUB_PAT'); - } - this.octokit = new Octokit({ auth: this.config.pat }); + return new Octokit({ auth: (await auth({ type: 'installation' })).token }); } - return this.octokit; + if (!this.config.pat) { + throw new Error('PAT auth requires GITHUB_PAT'); + } + return new Octokit({ auth: this.config.pat }); } private async getCloneUrl(repoUrl: string): Promise { @@ -70,18 +82,22 @@ export class GitHubProvider implements RepositoryProvider { return url.toString(); } - const octokit = await this.getOctokit(); const match = repoUrl.match(/github\.com[:/](.+?)(?:\.git)?$/); if (!match) return repoUrl; - const [owner, repo] = match[1].split('/'); - const { data } = await octokit.rest.apps.createInstallationAccessToken({ - installation_id: parseInt(this.config.installationId!, 10), + const githubRepo = match[1]; // 'owner/repo' + const installationId = parseInt(await this.resolveId(githubRepo), 10); + + const auth = createAppAuth({ + appId: this.config.appId!, + privateKey: this.config.privateKey!.replace(/\\n/g, '\n'), + installationId, }); + const { token } = await auth({ type: 'installation' }); - const url = new URL(`https://github.com/${owner}/${repo}.git`); + const url = new URL(`https://github.com/${githubRepo}.git`); url.username = 'x-access-token'; - url.password = data.token; + url.password = token; return url.toString(); } @@ -96,7 +112,7 @@ export class GitHubProvider implements RepositoryProvider { baseBranch: string; }> { const [owner, repo] = githubRepo.split('/'); - const octokit = await this.getOctokit(); + const octokit = await this.getOctokit(githubRepo); const { data } = await octokit.rest.pulls.get({ owner, repo, diff --git a/coverage-service/worker/src/lib/providers.ts b/coverage-service/worker/src/lib/providers.ts index c7c7bb2..ee11b70 100644 --- a/coverage-service/worker/src/lib/providers.ts +++ b/coverage-service/worker/src/lib/providers.ts @@ -6,14 +6,28 @@ import { GitHubProvider } from '@openreview/coverage-lib'; +import { prisma } from './prisma'; + export function createRepositoryProvider() { const authMode = (process.env.GITHUB_AUTH_MODE ?? 'pat') as GitHubAuthMode; + return new GitHubProvider({ authMode, pat: process.env.GITHUB_PAT, appId: process.env.GITHUB_APP_ID, privateKey: process.env.GITHUB_APP_PRIVATE_KEY, - installationId: process.env.GITHUB_APP_INSTALLATION_ID, + // Static ID: set when running in single-org mode (Option A). + // Leave blank in multi-tenant mode (Option C) — the resolver below takes over. + installationId: process.env.GITHUB_APP_INSTALLATION_ID || undefined, + // Dynamic resolver: looks up the installation ID per-repo from the database. + // Only invoked when authMode === 'app' and no static installationId is set. + resolveInstallationId: async (githubRepo: string) => { + const repo = await prisma.repository.findFirst({ + where: { githubRepo }, + select: { githubInstallationId: true }, + }); + return repo?.githubInstallationId ?? null; + }, }); } diff --git a/package.json b/package.json index cdc3e4a..9fdf09d 100644 --- a/package.json +++ b/package.json @@ -56,5 +56,8 @@ "typescript-eslint": "^8.57.1", "vitest": "^4.1.0", "zod": "^4.3.6" + }, + "dependencies": { + "thread-stream": "^4.2.0" } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a030edf..ce0327f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -7,6 +7,10 @@ settings: importers: .: + dependencies: + thread-stream: + specifier: ^4.2.0 + version: 4.2.0 devDependencies: '@eslint/js': specifier: ^10.0.1 @@ -227,6 +231,12 @@ importers: service: dependencies: + '@octokit/auth-app': + specifier: ^7.1.4 + version: 7.2.2 + '@octokit/rest': + specifier: ^21.0.2 + version: 21.1.1 '@openreview/core': specifier: workspace:* version: link:../core @@ -3171,6 +3181,9 @@ packages: resolution: {integrity: sha512-57frrGM/OCTLqLOAh0mhVA9VBMHd+9U7Zb2THMGdBUoZVOtGbJzjxsYGDJ3A9AYYCP4hn6y1TVbaOfzWtm5GFg==} engines: {node: '>= 12.13.0'} + real-require@1.0.0: + resolution: {integrity: sha512-P4nbQYQfePJxRSmY+v/KINxVucm4NF3p3s7pJveMTtom52FR4YGltUQLB8idDXwDDWW+eYrWDFbuzUnjoWHF7g==} + redis-errors@1.2.0: resolution: {integrity: sha512-1qny3OExCf0UvUV/5wpYKf2YwPcOqXzkwKKSmKHiE6ZMQs5heeE/c8eXK+PNllPvmjgAbfnsbpkGZWy8cBpn9w==} engines: {node: '>=4'} @@ -3499,6 +3512,10 @@ packages: thread-stream@3.2.0: resolution: {integrity: sha512-zLBvqpwr4Esa0kRjcrzGU6zL25lePWaCLMx0RQFrmteozIfeNdaMLpG5U7PeHzvlFkAWaRKA9/KVW4F60iB+qw==} + thread-stream@4.2.0: + resolution: {integrity: sha512-e2zZ96wSChazBsbENf/Pcm/4swHt2cEKQ92rhUjkL9GCKiTDJIaTBenjE/m9DXi0QBmTMDkFDdOomUy20A1tDQ==} + engines: {node: '>=20'} + through@2.3.8: resolution: {integrity: sha512-w89qg7PI8wAdvX60bMDP+bFoD5Dvhm9oLheFp5O4a2QF0cSBGsBX4qZmadPMvVqlLJBBci+WqGGOAPvcDeNSVg==} @@ -6861,6 +6878,8 @@ snapshots: real-require@0.2.0: {} + real-require@1.0.0: {} + redis-errors@1.2.0: {} redis-parser@3.0.0: @@ -7205,6 +7224,10 @@ snapshots: dependencies: real-require: 0.2.0 + thread-stream@4.2.0: + dependencies: + real-require: 1.0.0 + through@2.3.8: {} tinybench@2.9.0: {} diff --git a/service/.env.example b/service/.env.example index 0aad8b2..4d9ac0c 100644 --- a/service/.env.example +++ b/service/.env.example @@ -10,13 +10,23 @@ # ----------------------------------------------------------------------------- # Secret used to verify GitHub webhook deliveries (X-Hub-Signature-256). +# Must match the GitHub App webhook secret and coverage-service WEBHOOK_SECRET. # Generate with: openssl rand -hex 32 GITHUB_WEBHOOK_SECRET= -# GitHub Personal Access Token used by the service to fetch PRs and post -# review comments. Reuses GITHUB_PAT from the root .env if not set here. +# GitHub auth — use `app` when running as a GitHub App (posts as the bot). +# Use `pat` only for local dev without an app installation. +GITHUB_AUTH_MODE=app + +# GitHub App credentials (required when GITHUB_AUTH_MODE=app). +# Use the same values as coverage-service/.env. +GITHUB_APP_ID= +GITHUB_APP_PRIVATE_KEY= +# Optional — leave blank to resolve per-repo from GitHub API. +# GITHUB_APP_INSTALLATION_ID= + +# GitHub Personal Access Token — only required when GITHUB_AUTH_MODE=pat. # Scopes needed for private repos: repo -# For public-repo-only operation, no scopes are required. # GITHUB_PAT= # Redis connection URL used by BullMQ. diff --git a/service/package.json b/service/package.json index 444ead0..259282a 100644 --- a/service/package.json +++ b/service/package.json @@ -14,6 +14,8 @@ "dev:worker": "node --watch --import tsx src/worker.ts" }, "dependencies": { + "@octokit/auth-app": "^7.1.4", + "@octokit/rest": "^21.0.2", "@openreview/core": "workspace:*", "bullmq": "^5.34.0", "dotenv": "^17.3.1", diff --git a/service/src/config.ts b/service/src/config.ts index 834a88d..e8ef335 100644 --- a/service/src/config.ts +++ b/service/src/config.ts @@ -52,7 +52,14 @@ const ConfigSchema = z.object({ // GitHub githubWebhookSecret: stringOrEmpty, + githubAuthMode: z + .enum(['pat', 'app']) + .optional() + .transform((raw) => raw ?? 'pat'), githubPat: stringOrEmpty, + githubAppId: stringOrEmpty, + githubAppPrivateKey: stringOrEmpty, + githubAppInstallationId: stringOrEmpty, // Redis / queue redisUrl: stringOrEmpty, @@ -111,7 +118,11 @@ export function loadServiceConfig(): ServiceConfig { maxPayloadBytes: process.env.MAX_PAYLOAD_BYTES, githubWebhookSecret: process.env.GITHUB_WEBHOOK_SECRET, + githubAuthMode: process.env.GITHUB_AUTH_MODE as 'pat' | 'app' | undefined, githubPat: process.env.GITHUB_PAT || process.env.GITHUB_TOKEN, + githubAppId: process.env.GITHUB_APP_ID, + githubAppPrivateKey: process.env.GITHUB_APP_PRIVATE_KEY, + githubAppInstallationId: process.env.GITHUB_APP_INSTALLATION_ID, redisUrl: process.env.REDIS_URL || 'redis://localhost:6379', workerConcurrency: process.env.WORKER_CONCURRENCY, @@ -151,7 +162,12 @@ export function assertConfigReady(cfg: ServiceConfig): void { const missing: string[] = []; if (!cfg.githubWebhookSecret) missing.push('GITHUB_WEBHOOK_SECRET'); - if (!cfg.githubPat) missing.push('GITHUB_PAT (or GITHUB_TOKEN)'); + if (cfg.githubAuthMode === 'app') { + if (!cfg.githubAppId) missing.push('GITHUB_APP_ID'); + if (!cfg.githubAppPrivateKey) missing.push('GITHUB_APP_PRIVATE_KEY'); + } else if (!cfg.githubPat) { + missing.push('GITHUB_PAT (or GITHUB_TOKEN)'); + } if (!cfg.redisUrl) missing.push('REDIS_URL'); if (cfg.coverageServiceEnabled) { diff --git a/service/src/dispatch/coverage-webhook-forward.ts b/service/src/dispatch/coverage-webhook-forward.ts new file mode 100644 index 0000000..ce72bee --- /dev/null +++ b/service/src/dispatch/coverage-webhook-forward.ts @@ -0,0 +1,49 @@ +import axios from 'axios'; + +import type { Logger } from '../logger.js'; + +export interface CoverageWebhookForwardHeaders { + event: string; + deliveryId: string; + signature?: string; +} + +/** + * Forwards GitHub App installation events to the coverage-service so it can + * store per-repo `githubInstallationId` values. OpenReview is the public + * webhook entrypoint; coverage-service stays on localhost. + */ +export async function forwardCoverageInstallationWebhook( + baseUrl: string, + rawBody: Buffer, + headers: CoverageWebhookForwardHeaders, + logger: Logger, +): Promise<{ status: number; body: unknown }> { + const url = `${baseUrl.replace(/\/$/, '')}/webhooks/github`; + + const response = await axios.post(url, rawBody, { + headers: { + 'content-type': 'application/json', + 'x-github-event': headers.event, + 'x-github-delivery': headers.deliveryId, + ...(headers.signature + ? { 'x-hub-signature-256': headers.signature } + : {}), + }, + maxBodyLength: Infinity, + maxContentLength: Infinity, + validateStatus: () => true, + }); + + logger.info( + { + deliveryId: headers.deliveryId, + event: headers.event, + status: response.status, + url, + }, + 'forwarded installation webhook to coverage-service', + ); + + return { status: response.status, body: response.data }; +} diff --git a/service/src/github/auth.ts b/service/src/github/auth.ts index 84d1ffe..e768bc4 100644 --- a/service/src/github/auth.ts +++ b/service/src/github/auth.ts @@ -1,19 +1,33 @@ +import { createAppAuth } from '@octokit/auth-app'; +import { Octokit } from '@octokit/rest'; + import type { ServiceConfig } from '../config.js'; /** * Resolve a GitHub token suitable for posting reviews and reading PRs. - * - * v1 uses a single shared PAT for every repository. The interface is shaped - * so that swapping in GitHub App installation tokens later is a one-file - * change: callers always pass (owner, repo) and receive a string token. + * Callers pass (owner, repo) and receive a string token. */ export interface GitHubAuth { getTokenFor(owner: string, repo: string): Promise; } +export type GitHubAuthMode = 'pat' | 'app'; + +interface CachedInstallationToken { + token: string; + expiresAtMs: number; +} + +export function createGitHubAuth(cfg: ServiceConfig): GitHubAuth { + if (cfg.githubAuthMode === 'app') { + return createAppInstallationAuth(cfg); + } + return createPatAuth(cfg); +} + export function createPatAuth(cfg: ServiceConfig): GitHubAuth { if (!cfg.githubPat) { - throw new Error('GITHUB_PAT is required for the PAT auth provider.'); + throw new Error('GITHUB_PAT is required when GITHUB_AUTH_MODE=pat.'); } return { @@ -22,3 +36,75 @@ export function createPatAuth(cfg: ServiceConfig): GitHubAuth { }, }; } + +/** + * GitHub App auth — posts comments as the bot, not the PAT owner. + * Resolves the installation ID per repo via the GitHub API unless + * GITHUB_APP_INSTALLATION_ID is set (single-org mode). + */ +export function createAppInstallationAuth(cfg: ServiceConfig): GitHubAuth { + if (!cfg.githubAppId || !cfg.githubAppPrivateKey) { + throw new Error( + 'GitHub App auth requires GITHUB_APP_ID and GITHUB_APP_PRIVATE_KEY.', + ); + } + + const privateKey = cfg.githubAppPrivateKey.replace(/\\n/g, '\n'); + const staticInstallationId = cfg.githubAppInstallationId + ? Number.parseInt(cfg.githubAppInstallationId, 10) + : undefined; + + const installationIdByRepo = new Map(); + const tokenByInstallation = new Map(); + + const appAuth = createAppAuth({ + appId: cfg.githubAppId, + privateKey, + }); + + async function resolveInstallationId(owner: string, repo: string): Promise { + if (staticInstallationId && !Number.isNaN(staticInstallationId)) { + return staticInstallationId; + } + + const key = `${owner}/${repo}`; + const cached = installationIdByRepo.get(key); + if (cached) return cached; + + const appOctokit = new Octokit({ + auth: (await appAuth({ type: 'app' })).token, + }); + const { data } = await appOctokit.rest.apps.getRepoInstallation({ + owner, + repo, + }); + installationIdByRepo.set(key, data.id); + return data.id; + } + + async function installationToken(installationId: number): Promise { + const cached = tokenByInstallation.get(installationId); + if (cached && cached.expiresAtMs > Date.now() + 60_000) { + return cached.token; + } + + const auth = createAppAuth({ + appId: cfg.githubAppId!, + privateKey, + installationId, + }); + const result = await auth({ type: 'installation' }); + tokenByInstallation.set(installationId, { + token: result.token, + expiresAtMs: new Date(result.expiresAt).getTime(), + }); + return result.token; + } + + return { + async getTokenFor(owner: string, repo: string): Promise { + const installationId = await resolveInstallationId(owner, repo); + return installationToken(installationId); + }, + }; +} diff --git a/service/src/routes/webhook.ts b/service/src/routes/webhook.ts index 05285c1..3451b0f 100644 --- a/service/src/routes/webhook.ts +++ b/service/src/routes/webhook.ts @@ -1,15 +1,24 @@ -import { Router, raw } from 'express'; +import { Router, raw, type Request, type Response } from 'express'; import type { ServiceConfig } from '../config.js'; +import { forwardCoverageInstallationWebhook } from '../dispatch/coverage-webhook-forward.js'; import type { Logger } from '../logger.js'; import type { WebhookRouterDeps } from '../webhook/router.js'; import { routeWebhook } from '../webhook/router.js'; import { verifySignature } from '../webhook/verify.js'; +const COVERAGE_INSTALLATION_EVENTS = new Set([ + 'installation', + 'installation_repositories', +]); + +/** Paths GitHub App webhooks may target — both hit the same handler. */ +const WEBHOOK_PATHS = ['/webhook', '/webhooks/github'] as const; + /** - * Mount POST /webhook. Uses `express.raw` so we can verify the HMAC - * against the EXACT bytes GitHub signed; `express.json` would re-stringify - * and break the signature. + * Mount POST /webhook and POST /webhooks/github. Uses `express.raw` so we can + * verify the HMAC against the EXACT bytes GitHub signed; `express.json` would + * re-stringify and break the signature. * * We always ACK fast (202) before doing any review work — GitHub's webhook * delivery times out at ~10s and a real review can take 60s+. Errors that @@ -21,53 +30,79 @@ export function createWebhookRouter( logger: Logger, ): Router { const router = Router(); + const rawParser = raw({ type: 'application/json', limit: cfg.maxPayloadBytes }); + + for (const path of WEBHOOK_PATHS) { + router.post(path, rawParser, (req, res) => { + void handleWebhook(req, res, cfg, deps, logger); + }); + } + + return router; +} - router.post( - '/webhook', - raw({ type: 'application/json', limit: cfg.maxPayloadBytes }), - async (req, res) => { - const event = req.header('x-github-event') || ''; - const deliveryId = req.header('x-github-delivery') || 'unknown'; - const signature = req.header('x-hub-signature-256'); +async function handleWebhook( + req: Request, + res: Response, + cfg: ServiceConfig, + deps: WebhookRouterDeps, + logger: Logger, +): Promise { + const event = req.header('x-github-event') || ''; + const deliveryId = req.header('x-github-delivery') || 'unknown'; + const signature = req.header('x-hub-signature-256'); - const rawBody = req.body as Buffer; - if (!Buffer.isBuffer(rawBody) || rawBody.length === 0) { - res.status(400).json({ error: 'empty body' }); - return; - } + const rawBody = req.body as Buffer; + if (!Buffer.isBuffer(rawBody) || rawBody.length === 0) { + res.status(400).json({ error: 'empty body' }); + return; + } - if (!verifySignature(rawBody, signature, cfg.githubWebhookSecret)) { - logger.warn({ deliveryId, event }, 'webhook signature verification failed'); - res.status(401).json({ error: 'invalid signature' }); - return; - } + if (!verifySignature(rawBody, signature, cfg.githubWebhookSecret)) { + logger.warn({ deliveryId, event }, 'webhook signature verification failed'); + res.status(401).json({ error: 'invalid signature' }); + return; + } - let payload: unknown; - try { - payload = JSON.parse(rawBody.toString('utf8')); - } catch { - res.status(400).json({ error: 'invalid JSON' }); - return; - } + let payload: unknown; + try { + payload = JSON.parse(rawBody.toString('utf8')); + } catch { + res.status(400).json({ error: 'invalid JSON' }); + return; + } - // Acknowledge immediately, do the actual work after the response is sent. - res.status(202).json({ status: 'accepted', deliveryId }); + // Acknowledge immediately, do the actual work after the response is sent. + res.status(202).json({ status: 'accepted', deliveryId }); - // Fire-and-forget. Errors are logged; BullMQ retries handle transient - // downstream failures for jobs that were successfully enqueued. - void routeWebhook(event, deliveryId, payload, deps) - .then((result) => { - logger.info( - { deliveryId, event, result }, - `webhook ${result.status}`, - ); - }) - .catch((err) => { - const msg = err instanceof Error ? err.message : String(err); - logger.error({ deliveryId, event, err: msg }, 'webhook handler crashed'); - }); - }, - ); + if ( + COVERAGE_INSTALLATION_EVENTS.has(event) && + cfg.coverageServiceEnabled && + cfg.coverageServiceUrl + ) { + void forwardCoverageInstallationWebhook( + cfg.coverageServiceUrl, + rawBody, + { event, deliveryId, signature }, + logger, + ).catch((err) => { + const msg = err instanceof Error ? err.message : String(err); + logger.error( + { deliveryId, event, err: msg }, + 'coverage-service installation forward failed', + ); + }); + return; + } - return router; + // Fire-and-forget. Errors are logged; BullMQ retries handle transient + // downstream failures for jobs that were successfully enqueued. + void routeWebhook(event, deliveryId, payload, deps) + .then((result) => { + logger.info({ deliveryId, event, result }, `webhook ${result.status}`); + }) + .catch((err) => { + const msg = err instanceof Error ? err.message : String(err); + logger.error({ deliveryId, event, err: msg }, 'webhook handler crashed'); + }); } diff --git a/service/src/web.ts b/service/src/web.ts index 3151a24..1df2d3e 100644 --- a/service/src/web.ts +++ b/service/src/web.ts @@ -1,7 +1,7 @@ import { createApp } from './app.js'; import { assertConfigReady, loadServiceConfig } from './config.js'; import { createNoopDispatcher } from './dispatch/downstream.js'; -import { createPatAuth } from './github/auth.js'; +import { createGitHubAuth } from './github/auth.js'; import { createRedisConnection } from './jobs/connection.js'; import { ReviewQueue } from './jobs/queue.js'; import { createLogger } from './logger.js'; @@ -24,7 +24,7 @@ async function main(): Promise { const redis = createRedisConnection(cfg); const queue = new ReviewQueue(redis, cfg, logger); const downstream = createNoopDispatcher(logger); - const auth = createPatAuth(cfg); + const auth = createGitHubAuth(cfg); const app = createApp({ cfg, logger, redis, queue, downstream, auth }); diff --git a/service/src/worker.ts b/service/src/worker.ts index 9878376..6ccae42 100644 --- a/service/src/worker.ts +++ b/service/src/worker.ts @@ -4,7 +4,7 @@ import type { Job } from 'bullmq'; import { assertConfigReady, loadServiceConfig } from './config.js'; -import { createPatAuth } from './github/auth.js'; +import { createGitHubAuth } from './github/auth.js'; import { createRedisConnection } from './jobs/connection.js'; import { processChat } from './jobs/processors/chat.js'; import { processCoverageAnalysis } from './jobs/processors/coverage-analysis.js'; @@ -37,7 +37,7 @@ async function main(): Promise { process.exit(1); } - const auth = createPatAuth(cfg); + const auth = createGitHubAuth(cfg); const connection = createRedisConnection(cfg); const reviewCache = new ReviewCache(connection, cfg, logger); From 0bffb39e6ffda35c629ccaea2059ef2b32aa0e9a Mon Sep 17 00:00:00 2001 From: Kenil Shah Date: Fri, 26 Jun 2026 18:26:29 +0530 Subject: [PATCH 2/3] nit --- coverage-service/api/query-redis.js | 53 ----------------------------- 1 file changed, 53 deletions(-) delete mode 100644 coverage-service/api/query-redis.js diff --git a/coverage-service/api/query-redis.js b/coverage-service/api/query-redis.js deleted file mode 100644 index 68f71a7..0000000 --- a/coverage-service/api/query-redis.js +++ /dev/null @@ -1,53 +0,0 @@ -const path = require('path'); -const dotenv = require('dotenv'); -const envPath = path.resolve(__dirname, '../.env'); -dotenv.config({ path: envPath }); - -const { Queue } = require('bullmq'); - -async function main() { - const Redis = require('ioredis'); - const redis = new Redis(process.env.REDIS_URL ?? 'redis://localhost:6379'); - console.log('Connecting to Redis via ioredis...'); - - // Get all keys matching pr-analysis - const keys = await redis.keys('*pr-analysis*'); - console.log('Redis keys matching *pr-analysis*:', keys); - - // Get active jobs list in BullMQ structure - // BullMQ uses hashes and sets. Let's get the active jobs from the active zset/list. - // In BullMQ 5, the active list can be retrieved. Or we can just use BullMQ Queue but without the client.get call. - const { Queue } = require('bullmq'); - const connection = { - url: process.env.REDIS_URL ?? 'redis://localhost:6379', - }; - const prAnalysisQueue = new Queue('pr-analysis', { connection }); - - const waitingJobs = await prAnalysisQueue.getWaiting(); - const activeJobs = await prAnalysisQueue.getActive(); - const failedJobs = await prAnalysisQueue.getFailed(); - - console.log('Waiting jobs count:', waitingJobs.length); - console.log('Active jobs count:', activeJobs.length); - for (const j of activeJobs) { - const lockKey = `bull:pr-analysis:${j.id}:lock`; - const lockVal = await redis.get(lockKey); - const lockTtl = await redis.ttl(lockKey); - console.log(`- Job ${j.id}: PR run ${j.data.prRunId}, PR #${j.data.prNumber}`); - console.log(` Processed at: ${j.processedOn ? new Date(j.processedOn).toISOString() : 'N/A'}`); - console.log(` Lock exists: ${!!lockVal} (value: ${lockVal}, TTL: ${lockTtl}s)`); - console.log(` Progress: ${j.progress}`); - } - - console.log('\nFetching connected Redis clients...'); - const clients = await redis.client('list'); - console.log(clients); - - await prAnalysisQueue.close(); - await redis.disconnect(); -} - -main() - .catch((e) => { - console.error('Error querying Redis:', e); - }); From 8730809114066831f7248beb76f00239366ee1f2 Mon Sep 17 00:00:00 2001 From: Kenil Shah Date: Fri, 26 Jun 2026 18:30:49 +0530 Subject: [PATCH 3/3] fox test --- service/src/jobs/processors/coverage-analysis.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/service/src/jobs/processors/coverage-analysis.test.ts b/service/src/jobs/processors/coverage-analysis.test.ts index 4d42e83..215ba17 100644 --- a/service/src/jobs/processors/coverage-analysis.test.ts +++ b/service/src/jobs/processors/coverage-analysis.test.ts @@ -184,8 +184,8 @@ describe('processCoverageAnalysis', () => { }), ); - // 1 ack + 1 coverage summary (updated in place on later pushes) - expect(poster.postAcknowledgement).toHaveBeenCalledTimes(1); + // Coverage summary only (no separate ack comment — keeps PR thread clean) + expect(poster.postAcknowledgement).not.toHaveBeenCalled(); expect(poster.postCoverageComment).toHaveBeenCalledTimes(1); const summary = (poster.postCoverageComment as unknown as ReturnType).mock.calls[0][1] as string; expect(summary).toContain('Diff coverage');