From 4fdcc47aadc9b00f89b7d6e1146a91380f330977 Mon Sep 17 00:00:00 2001 From: Michael Wu Date: Sun, 30 Aug 2026 19:01:18 +0900 Subject: [PATCH 1/6] feat(githubbot): support GitHub App authentication --- contrib/chart/Chart.yaml | 2 +- contrib/chart/templates/_helpers.tpl | 22 ++++++++- contrib/chart/templates/githubbot.yaml | 34 ++++++++++++-- contrib/chart/values.yaml | 23 ++++++--- services/githubbot/README.md | 34 ++++++++------ services/githubbot/src/index.ts | 52 ++++++++++++++++++++- services/githubbot/src/server.ts | 36 ++++++++++++-- services/githubbot/src/types.ts | 18 +++++-- services/githubbot/test/github-auth.test.ts | 51 ++++++++++++++++++++ 9 files changed, 234 insertions(+), 38 deletions(-) create mode 100644 services/githubbot/test/github-auth.test.ts diff --git a/contrib/chart/Chart.yaml b/contrib/chart/Chart.yaml index 292f4459f..e3c0a1e5e 100644 --- a/contrib/chart/Chart.yaml +++ b/contrib/chart/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 name: centaur description: Helm chart for the trusted Centaur control plane type: application -version: 0.1.131 +version: 0.1.134 appVersion: "0.1.0" dependencies: - name: connect diff --git a/contrib/chart/templates/_helpers.tpl b/contrib/chart/templates/_helpers.tpl index 85c8c2d96..7a40c2325 100644 --- a/contrib/chart/templates/_helpers.tpl +++ b/contrib/chart/templates/_helpers.tpl @@ -157,8 +157,9 @@ so the defaults are safe for repos that only carry some surfaces. {{- /* The GitHub App PEM is intentionally a separate secret rather than a key in the -shared infra env Secret: it is mounted read-only into only the Console worker, -not translated to an environment variable or exposed to Console web pods. +shared infra env Secret: it is mounted read-only only into components that mint +installation tokens, never translated to an environment variable, and never +exposed to Console web pods. */ -}} {{- define "centaur.githubAppInstallationChecksum" -}} {{- $console := include "centaur.consoleValues" . | fromYaml -}} @@ -170,6 +171,23 @@ not translated to an environment variable or exposed to Console web pods. {{- end -}} {{- end -}} +{{- /* +The githubbot can use the same App installation identity without placing its +PEM in an environment variable. Include both the mounted Secret generation and +the public identity in the checksum so key/client/installation rotation rolls +the controller immediately. +*/ -}} +{{- define "centaur.githubbotAppChecksum" -}} +{{- $app := .Values.githubbot.githubApp -}} +{{- if $app.enabled -}} +{{- $name := required "githubbot.githubApp.existingSecretName is required when GitHub App authentication is enabled" $app.existingSecretName -}} +{{- $payload := dict "secret" (include "centaur.secretResourceVersion" (dict "root" . "name" $name)) "clientId" (required "githubbot.githubApp.clientId is required when GitHub App authentication is enabled" $app.clientId) "installationId" (required "githubbot.githubApp.installationId is required when GitHub App authentication is enabled" $app.installationId) -}} +{{- toJson $payload | sha256sum | quote -}} +{{- else -}} +{{- "disabled" | quote -}} +{{- end -}} +{{- end -}} + {{- /* The upstream 1Password Connect subchart names its Service after `connect.applicationName` (default `onepassword-connect`) and exposes the diff --git a/contrib/chart/templates/githubbot.yaml b/contrib/chart/templates/githubbot.yaml index 458db3406..c0a6d4b15 100644 --- a/contrib/chart/templates/githubbot.yaml +++ b/contrib/chart/templates/githubbot.yaml @@ -42,6 +42,7 @@ spec: metadata: annotations: checksum/infra-secrets: {{ include "centaur.infraSecretsChecksum" . }} + checksum/github-app-private-key: {{ include "centaur.githubbotAppChecksum" . }} {{- if or .Values.githubbot.reviewPrompt .Values.githubbot.issuePrompt .Values.githubbot.managementPrompt }} checksum/prompts: {{ dict "review" .Values.githubbot.reviewPrompt "issue" .Values.githubbot.issuePrompt "management" .Values.githubbot.managementPrompt | toJson | sha256sum }} {{- end }} @@ -70,14 +71,22 @@ spec: # the deployment's default harness. - name: GITHUBBOT_DEFAULT_HARNESS value: {{ .Values.sandbox.harnessEngine | quote }} - # Personal access token for the bot's GitHub teammate account — kept - # distinct from the sandbox tool token so the bot acts as its own - # GitHub user (requestable as a reviewer, @-mentionable). +{{- if .Values.githubbot.githubApp.enabled }} + - name: GITHUB_APP_CLIENT_ID + value: {{ required "githubbot.githubApp.clientId is required when GitHub App authentication is enabled" .Values.githubbot.githubApp.clientId | quote }} + - name: GITHUB_INSTALLATION_ID + value: {{ required "githubbot.githubApp.installationId is required when GitHub App authentication is enabled" .Values.githubbot.githubApp.installationId | quote }} + - name: GITHUB_PRIVATE_KEY_FILE + value: {{ printf "%s/%s" (required "githubbot.githubApp.privateKeyMountPath is required when GitHub App authentication is enabled" .Values.githubbot.githubApp.privateKeyMountPath) (required "githubbot.githubApp.privateKeySecretKey is required when GitHub App authentication is enabled" .Values.githubbot.githubApp.privateKeySecretKey) | quote }} +{{- else }} + # Personal access token for the bot's GitHub teammate account. This + # compatibility mode remains available for deployments without an App. - name: GITHUB_TOKEN valueFrom: secretKeyRef: name: {{ include "centaur.secretEnvName" . }} key: {{ printf "%sGITHUBBOT_TOKEN" .Values.secretManager.envPrefix }} +{{- end }} # githubbot's own webhook signing secret (the GitHub repo/org webhook). - name: GITHUB_WEBHOOK_SECRET valueFrom: @@ -172,16 +181,33 @@ spec: {{ toYaml .Values.containerSecurityContext | nindent 12 }} resources: {{ toYaml .Values.githubbot.resources | nindent 12 }} -{{- if or .Values.githubbot.reviewPrompt .Values.githubbot.issuePrompt .Values.githubbot.managementPrompt }} +{{- if or .Values.githubbot.githubApp.enabled .Values.githubbot.reviewPrompt .Values.githubbot.issuePrompt .Values.githubbot.managementPrompt }} volumeMounts: +{{- if .Values.githubbot.githubApp.enabled }} + - name: github-app-private-key + mountPath: {{ required "githubbot.githubApp.privateKeyMountPath is required when GitHub App authentication is enabled" .Values.githubbot.githubApp.privateKeyMountPath | quote }} + readOnly: true +{{- end }} +{{- if or .Values.githubbot.reviewPrompt .Values.githubbot.issuePrompt .Values.githubbot.managementPrompt }} - name: prompts mountPath: /etc/githubbot/prompts readOnly: true +{{- end }} volumes: +{{- if .Values.githubbot.githubApp.enabled }} + - name: github-app-private-key + secret: + secretName: {{ required "githubbot.githubApp.existingSecretName is required when GitHub App authentication is enabled" .Values.githubbot.githubApp.existingSecretName | quote }} + items: + - key: {{ required "githubbot.githubApp.privateKeySecretKey is required when GitHub App authentication is enabled" .Values.githubbot.githubApp.privateKeySecretKey | quote }} + path: {{ required "githubbot.githubApp.privateKeySecretKey is required when GitHub App authentication is enabled" .Values.githubbot.githubApp.privateKeySecretKey | quote }} +{{- end }} +{{- if or .Values.githubbot.reviewPrompt .Values.githubbot.issuePrompt .Values.githubbot.managementPrompt }} - name: prompts configMap: name: {{ include "centaur.componentName" (dict "root" . "component" "githubbot") }}-prompts {{- end }} +{{- end }} --- apiVersion: v1 kind: Service diff --git a/contrib/chart/values.yaml b/contrib/chart/values.yaml index 369b8c90c..a8ddc14dc 100644 --- a/contrib/chart/values.yaml +++ b/contrib/chart/values.yaml @@ -655,13 +655,12 @@ linearbot: extraEnv: {} resources: {} -# Chat SDK GitHub bot — GitHub teammate (PAT) ingress. Receives issue/PR comment -# webhooks on /api/webhooks/github (the @chat-adapter/github adapter), answers in -# the comment thread, and runs a review when the bot account is requested as a -# reviewer. Forwards sessions to the api-rs control plane (:8080). Disabled by -# default: requires the GITHUBBOT_TOKEN (PAT) and GITHUBBOT_WEBHOOK_SECRET secrets -# (see contrib/scripts/bootstrap-k8s-secrets.sh). userName must be the bot -# account's GitHub login so @-mention and review-request matching work. +# Chat SDK GitHub ingress. Receives issue/PR comment webhooks on +# /api/webhooks/github (the @chat-adapter/github adapter), answers in the +# comment thread, and forwards sessions to the api-rs control plane (:8080). +# Disabled by default: requires GITHUBBOT_WEBHOOK_SECRET plus either the +# githubApp block or the legacy GITHUBBOT_TOKEN secret. userName must match the +# App bot or teammate login so @-mention matching works. githubbot: enabled: false # Singleton controller: per-PR admission and merge gates are process-serialized. @@ -672,6 +671,16 @@ githubbot: tag: latest pullPolicy: Always userName: "" + # Prefer a GitHub App installation for production. When enabled, the PAT + # secret key is not referenced; the PEM is mounted read-only and Octokit + # transparently rotates short-lived installation tokens. + githubApp: + enabled: false + clientId: "" + installationId: "" + existingSecretName: "" + privateKeySecretKey: private-key.pem + privateKeyMountPath: /var/run/centaur/github-app # v2 PR self-management (only acts on PRs assigned to the bot account). # Auto-merge respects branch protection and is paused per-PR by the hold label # / draft status. diff --git a/services/githubbot/README.md b/services/githubbot/README.md index 7c49e62f8..7086d9a94 100644 --- a/services/githubbot/README.md +++ b/services/githubbot/README.md @@ -153,17 +153,21 @@ the same shutdown drain rather than being treated as completed work. ## Auth -A personal access token for the bot's GitHub teammate account is required (`GITHUB_TOKEN`). As a -normal user account it is natively mentionable, assignable, and requestable as a reviewer, and the -token inherits that user's permissions. Scopes: **`repo`** (read PRs/issues, post and edit comments, -add reactions) — and, when the agent pushes branches or opens PRs from its sandbox, **`workflow`**. - -Keep this distinct from the `GITHUB_TOKEN` used by the repo-cache / sandbox tooling — that one is the -agent's git-operations token; this one is the bot's own identity. The chart wires githubbot's token -from a separate `GITHUBBOT_TOKEN` secret key to avoid collision. - -GitHub App auth is also supported by the adapter (`GITHUB_APP_ID` / `GITHUB_PRIVATE_KEY`), but the -PAT-teammate model is what we run. +Use exactly one controller identity: + +- Preferred for production: a fixed GitHub App installation. Set + `GITHUB_APP_CLIENT_ID`, `GITHUB_INSTALLATION_ID`, and either + `GITHUB_PRIVATE_KEY_FILE` or `GITHUB_PRIVATE_KEY`. The Client ID is passed as + the JWT issuer, and Octokit transparently mints and refreshes short-lived + installation tokens. The chart mounts the PEM from a dedicated Secret rather + than placing it in an environment variable. +- Compatibility mode: a personal access token for a bot teammate account in + `GITHUB_TOKEN`. Keep it distinct from any repo-cache or sandbox token. + +Do not configure both modes. The bot fails startup on missing, partial, or mixed +credentials. GitHub Apps are not normal user accounts, so assignment and +requested-review flows may require a teammate PAT; signed comment mentions and +PR/issue lifecycle management work with the App installation identity. Webhook events to subscribe: **Issue comments**, **Pull request review comments**, **Issues**, **Pull requests**, **Pull request reviews**, **Check runs**, **Check suites**, and **Workflow runs** @@ -173,9 +177,13 @@ requests**, **Pull request reviews**, **Check runs**, **Check suites**, and **Wo | Var | Required | Notes | |-----|----------|-------| -| `GITHUB_TOKEN` | ✅ | PAT for the bot's teammate account. | +| `GITHUB_TOKEN` | one auth mode | PAT for the bot's teammate account. | +| `GITHUB_APP_CLIENT_ID` | one auth mode | Recommended GitHub App JWT issuer (legacy `GITHUB_APP_ID` is accepted). | +| `GITHUB_INSTALLATION_ID` | App mode | Fixed positive installation ID. | +| `GITHUB_PRIVATE_KEY_FILE` | App mode | Preferred path to a mounted PEM; mutually exclusive with `GITHUB_PRIVATE_KEY`. | +| `GITHUB_PRIVATE_KEY` | App mode | Inline PEM compatibility input. | | `GITHUB_WEBHOOK_SECRET` | ✅ | Webhook signing secret (or `GITHUBBOT_WEBHOOK_SECRET`). | -| `GITHUB_BOT_USERNAME` | ✅ | The bot account's GitHub login — drives `@`-mention and requested-reviewer matching (or `GITHUBBOT_USER_NAME`). | +| `GITHUB_BOT_USERNAME` | ✅ | Mention name used by the bot. For an App, use its slug without the `[bot]` suffix; for a teammate PAT, use the account login (or `GITHUBBOT_USER_NAME`). | | `GITHUBBOT_DATABASE_URL` | ✅ | Postgres for chat-SDK state (falls back to `DATABASE_URL` / `POSTGRES_URL`). | | `GITHUBBOT_REPOSITORY_ALLOWLIST` | ✅ | Comma-separated exact `owner/repository` names. Empty/unset is rejected at startup; wildcards are not supported. Signed events for other repositories are acknowledged but ignored before chat state or agent work is created. | | `CENTAUR_API_URL` | — | api-rs control plane, default `http://127.0.0.1:8080`. | diff --git a/services/githubbot/src/index.ts b/services/githubbot/src/index.ts index 20b6ec999..62bb7ff8a 100644 --- a/services/githubbot/src/index.ts +++ b/services/githubbot/src/index.ts @@ -1,5 +1,10 @@ import { createHmac, timingSafeEqual } from "node:crypto"; -import { createGitHubAdapter, type GitHubAdapter } from "@chat-adapter/github"; +import { + createGitHubAdapter, + type GitHubAdapter, + type GitHubAdapterAppConfig, + type GitHubAdapterPATConfig, +} from "@chat-adapter/github"; import { createPostgresState } from "@chat-adapter/state-pg"; import { Chat, @@ -70,7 +75,7 @@ export function createGithubbot(options: GithubbotOptions): Githubbot { const userName = options.userName ?? "github-bot"; const logger = options.logger ?? noopLogger; const github = createGitHubAdapter({ - token: options.token, + ...resolveGithubAdapterAuth(options), webhookSecret: options.webhookSecret, userName, ...(options.botUserId ? { botUserId: Number(options.botUserId) } : {}), @@ -227,6 +232,49 @@ export function createGithubbot(options: GithubbotOptions): Githubbot { return { app, chat }; } +export function resolveGithubAdapterAuth( + options: Pick< + GithubbotOptions, + | "token" + | "githubAppClientId" + | "githubAppInstallationId" + | "githubAppPrivateKey" + >, +): GitHubAdapterPATConfig | GitHubAdapterAppConfig { + const token = options.token?.trim(); + const clientId = options.githubAppClientId?.trim(); + const installationId = options.githubAppInstallationId; + const privateKey = options.githubAppPrivateKey?.trim(); + const appFieldsPresent = [clientId, installationId, privateKey].filter( + (value) => value !== undefined && value !== "", + ).length; + + if (token && appFieldsPresent > 0) { + throw new Error( + "GitHub PAT and GitHub App authentication are mutually exclusive", + ); + } + if (token) return { token }; + + if ( + !clientId || + !privateKey || + appFieldsPresent !== 3 || + !Number.isSafeInteger(installationId) || + (installationId ?? 0) <= 0 + ) { + throw new Error( + "GitHub authentication requires GITHUB_TOKEN or a complete GitHub App Client ID, installation ID, and private key", + ); + } + + return { + appId: clientId, + installationId: installationId as number, + privateKey, + }; +} + type MessageHandlerInput = { adapter: GitHubAdapter; mode: "execute" | "append"; diff --git a/services/githubbot/src/server.ts b/services/githubbot/src/server.ts index 60b70d02e..e2becd2b1 100644 --- a/services/githubbot/src/server.ts +++ b/services/githubbot/src/server.ts @@ -6,9 +6,32 @@ import { createGithubbot, type GithubbotOptions } from "./index"; const port = numberEnv("PORT", 3001); const apiUrl = stringEnv("CENTAUR_API_URL", "http://127.0.0.1:8080"); -// Personal access token for the bot's GitHub teammate account (the bot acts as a -// real GitHub user — it can be requested as a reviewer, @-mentioned, assigned). -const token = requiredEnv("GITHUB_TOKEN"); +// Use either a teammate PAT or a fixed GitHub App installation. The App path +// reads its PEM from a mounted Secret and lets Octokit rotate the one-hour +// installation token transparently; the PEM never enters the pod environment. +const token = optionalEnv("GITHUB_TOKEN"); +const githubAppClientId = + optionalEnv("GITHUB_APP_CLIENT_ID") ?? optionalEnv("GITHUB_APP_ID"); +const githubAppInstallationId = optionalNumberEnv("GITHUB_INSTALLATION_ID"); +const githubAppPrivateKeyInline = optionalEnv("GITHUB_PRIVATE_KEY"); +const githubAppPrivateKeyFile = optionalEnv("GITHUB_PRIVATE_KEY_FILE"); +if (githubAppPrivateKeyInline && githubAppPrivateKeyFile) { + throw new Error( + "GITHUB_PRIVATE_KEY and GITHUB_PRIVATE_KEY_FILE are mutually exclusive", + ); +} +let githubAppPrivateKey = githubAppPrivateKeyInline; +if (!githubAppPrivateKey && githubAppPrivateKeyFile) { + try { + githubAppPrivateKey = readFileSync(githubAppPrivateKeyFile, "utf8"); + } catch (error) { + throw new Error( + `GITHUB_PRIVATE_KEY_FILE (${githubAppPrivateKeyFile}) could not be read: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } +} // Signing secret configured on the GitHub repo/org webhook. The adapter verifies // comment webhooks; githubbot verifies the pull_request (review-request) webhook. @@ -18,8 +41,8 @@ if (!webhookSecret) { throw new Error("GITHUB_WEBHOOK_SECRET (or GITHUBBOT_WEBHOOK_SECRET) is required"); } -// The bot account's GitHub login. Drives @-mention detection and matching the -// requested reviewer on review-request webhooks, so it must be the real login. +// The bot's mention name. For an App this is its slug without the `[bot]` +// suffix; for PAT mode it is the teammate account login. const userName = optionalEnv("GITHUB_BOT_USERNAME") ?? optionalEnv("GITHUBBOT_USER_NAME"); if (!userName) { @@ -150,6 +173,9 @@ const options: GithubbotOptions = { ), stateKeyPrefix: optionalEnv("GITHUBBOT_STATE_KEY_PREFIX"), token, + githubAppClientId, + githubAppInstallationId, + githubAppPrivateKey, userName, webhookSecret, logger: consoleLogger, diff --git a/services/githubbot/src/types.ts b/services/githubbot/src/types.ts index 901a7063d..575aec8bc 100644 --- a/services/githubbot/src/types.ts +++ b/services/githubbot/src/types.ts @@ -29,9 +29,10 @@ export type GithubbotApiAttachment = { width?: number; }; -// GitHub scopes by repository (owner/repo), resolved from the thread id; the bot -// authenticates as a single PAT/teammate, so sessions are keyed by thread id -// alone (no per-workspace token like Slack's teamId). +// GitHub scopes by repository (owner/repo), resolved from the thread id. The +// controller uses one fixed GitHub identity (a preferred App installation or a +// compatibility PAT), so sessions are keyed by thread id alone (no +// per-workspace token like Slack's teamId). export type GithubbotApiMessage = { attachments: GithubbotApiAttachment[]; author: GithubbotApiAuthor; @@ -171,7 +172,16 @@ export type GithubbotOptions = { /** Merge method for auto-merge: "merge" | "squash" | "rebase". Default "squash". */ mergeMethod?: "merge" | "squash" | "rebase"; /** Personal access token for the bot's GitHub teammate account. */ - token: string; + token?: string; + /** + * GitHub App Client ID used as the JWT issuer. GitHub and Octokit recommend + * the Client ID over the legacy numeric App ID. + */ + githubAppClientId?: string; + /** Fixed organization/repository installation used by this bot instance. */ + githubAppInstallationId?: number; + /** GitHub App PEM contents. Mount a Secret and read it at process startup. */ + githubAppPrivateKey?: string; userName?: string; /** * GitHub `author_association` values allowed to drive the conversational diff --git a/services/githubbot/test/github-auth.test.ts b/services/githubbot/test/github-auth.test.ts new file mode 100644 index 000000000..c08c18931 --- /dev/null +++ b/services/githubbot/test/github-auth.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, test } from "bun:test"; +import { resolveGithubAdapterAuth } from "../src/index"; + +describe("GitHub authentication", () => { + test("keeps the existing PAT mode", () => { + expect(resolveGithubAdapterAuth({ token: " token " })).toEqual({ + token: "token", + }); + }); + + test("accepts a fixed App installation using the recommended Client ID", () => { + expect( + resolveGithubAdapterAuth({ + githubAppClientId: "Iv1.example", + githubAppInstallationId: 123, + githubAppPrivateKey: " private-key ", + }), + ).toEqual({ + appId: "Iv1.example", + installationId: 123, + privateKey: "private-key", + }); + }); + + test("rejects mixed PAT and App credentials", () => { + expect(() => + resolveGithubAdapterAuth({ + token: "token", + githubAppClientId: "Iv1.example", + githubAppInstallationId: 123, + githubAppPrivateKey: "private-key", + }), + ).toThrow("mutually exclusive"); + }); + + test("fails closed for partial or invalid App credentials", () => { + expect(() => + resolveGithubAdapterAuth({ githubAppClientId: "Iv1.example" }), + ).toThrow("complete GitHub App"); + expect(() => + resolveGithubAdapterAuth({ + githubAppClientId: "Iv1.example", + githubAppInstallationId: 0, + githubAppPrivateKey: "private-key", + }), + ).toThrow("complete GitHub App"); + expect(() => resolveGithubAdapterAuth({})).toThrow( + "GitHub authentication requires", + ); + }); +}); From 68705d9af3e9820248b26f6a17f13b68eac48541 Mon Sep 17 00:00:00 2001 From: Michael Wu Date: Sun, 30 Aug 2026 19:21:02 +0900 Subject: [PATCH 2/6] fix(githubbot): validate installation IDs --- services/githubbot/README.md | 6 ++++++ services/githubbot/src/server.ts | 7 ++---- services/githubbot/src/utils.ts | 11 ++++++++++ services/githubbot/test/github-auth.test.ts | 24 +++++++++++++++++++++ 4 files changed, 43 insertions(+), 5 deletions(-) diff --git a/services/githubbot/README.md b/services/githubbot/README.md index 7086d9a94..8ede0fb48 100644 --- a/services/githubbot/README.md +++ b/services/githubbot/README.md @@ -187,6 +187,12 @@ requests**, **Pull request reviews**, **Check runs**, **Check suites**, and **Wo | `GITHUBBOT_DATABASE_URL` | ✅ | Postgres for chat-SDK state (falls back to `DATABASE_URL` / `POSTGRES_URL`). | | `GITHUBBOT_REPOSITORY_ALLOWLIST` | ✅ | Comma-separated exact `owner/repository` names. Empty/unset is rejected at startup; wildcards are not supported. Signed events for other repositories are acknowledged but ignored before chat state or agent work is created. | | `CENTAUR_API_URL` | — | api-rs control plane, default `http://127.0.0.1:8080`. | + +The chart checksum includes the mounted private-key Secret's live resource +version. Rotate the Secret through the same Helm reconcile used for deployment; +the resulting pod-template change restarts githubbot, which reads the PEM once +at process startup. Do not edit the Secret out of band without reconciling the +release. | `GITHUBBOT_API_KEY` | — | Dedicated bearer sent to api-rs. | | `GITHUBBOT_DEFAULT_HARNESS` | — | Harness for new threads without an inline flag, default `codex`. | | `GITHUBBOT_REVIEW_PROMPT` | — | Full review methodology, inline. Replaces the bundled default verbatim. | diff --git a/services/githubbot/src/server.ts b/services/githubbot/src/server.ts index e2becd2b1..27dee7182 100644 --- a/services/githubbot/src/server.ts +++ b/services/githubbot/src/server.ts @@ -2,6 +2,7 @@ import { readFileSync } from "node:fs"; import { requireRepositoryAllowlist } from "./authorization"; import { drainBackgroundWork } from "./context"; import { createGithubbot, type GithubbotOptions } from "./index"; +import { positiveIntegerValue } from "./utils"; const port = numberEnv("PORT", 3001); const apiUrl = stringEnv("CENTAUR_API_URL", "http://127.0.0.1:8080"); @@ -263,11 +264,7 @@ function mergeMethodEnv(): "merge" | "squash" | "rebase" | undefined { function optionalNumberEnv(name: string): number | undefined { const value = optionalEnv(name); if (!value) return undefined; - const parsed = Number.parseInt(value, 10); - if (!Number.isFinite(parsed) || parsed <= 0) { - throw new Error(`${name} must be a positive integer`); - } - return parsed; + return positiveIntegerValue(value, name); } function log( diff --git a/services/githubbot/src/utils.ts b/services/githubbot/src/utils.ts index 9e295ec16..97f67130e 100644 --- a/services/githubbot/src/utils.ts +++ b/services/githubbot/src/utils.ts @@ -48,6 +48,17 @@ export function stringValue(value: unknown): string | undefined { return typeof value === "string" && value.trim() ? value.trim() : undefined; } +export function positiveIntegerValue(value: string, name: string): number { + if (!/^[1-9]\d*$/.test(value)) { + throw new Error(`${name} must be a positive decimal integer`); + } + const parsed = Number(value); + if (!Number.isSafeInteger(parsed)) { + throw new Error(`${name} must be a safe positive decimal integer`); + } + return parsed; +} + export function isJsonObject(value: unknown): value is JsonObject { return Boolean(value && typeof value === "object" && !Array.isArray(value)); } diff --git a/services/githubbot/test/github-auth.test.ts b/services/githubbot/test/github-auth.test.ts index c08c18931..da608a904 100644 --- a/services/githubbot/test/github-auth.test.ts +++ b/services/githubbot/test/github-auth.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; import { resolveGithubAdapterAuth } from "../src/index"; +import { positiveIntegerValue } from "../src/utils"; describe("GitHub authentication", () => { test("keeps the existing PAT mode", () => { @@ -49,3 +50,26 @@ describe("GitHub authentication", () => { ); }); }); + +describe("GitHub installation ID parsing", () => { + test("accepts a complete safe positive decimal integer", () => { + expect(positiveIntegerValue("157611530", "GITHUB_INSTALLATION_ID")).toBe( + 157611530, + ); + }); + + test("rejects prefixes, fractions, scientific notation, and unsafe values", () => { + for (const value of [ + "123oops", + "123.5", + "1e3", + "0", + "-1", + "9007199254740992", + ]) { + expect(() => + positiveIntegerValue(value, "GITHUB_INSTALLATION_ID"), + ).toThrow("positive decimal integer"); + } + }); +}); From 37b1a72f9dc5f7bbd05ec609fe6685664328cd3c Mon Sep 17 00:00:00 2001 From: Michael Wu Date: Sun, 30 Aug 2026 19:31:38 +0900 Subject: [PATCH 3/6] docs(githubbot): restore environment table --- services/githubbot/README.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/services/githubbot/README.md b/services/githubbot/README.md index 8ede0fb48..07ec7068e 100644 --- a/services/githubbot/README.md +++ b/services/githubbot/README.md @@ -188,11 +188,6 @@ requests**, **Pull request reviews**, **Check runs**, **Check suites**, and **Wo | `GITHUBBOT_REPOSITORY_ALLOWLIST` | ✅ | Comma-separated exact `owner/repository` names. Empty/unset is rejected at startup; wildcards are not supported. Signed events for other repositories are acknowledged but ignored before chat state or agent work is created. | | `CENTAUR_API_URL` | — | api-rs control plane, default `http://127.0.0.1:8080`. | -The chart checksum includes the mounted private-key Secret's live resource -version. Rotate the Secret through the same Helm reconcile used for deployment; -the resulting pod-template change restarts githubbot, which reads the PEM once -at process startup. Do not edit the Secret out of band without reconciling the -release. | `GITHUBBOT_API_KEY` | — | Dedicated bearer sent to api-rs. | | `GITHUBBOT_DEFAULT_HARNESS` | — | Harness for new threads without an inline flag, default `codex`. | | `GITHUBBOT_REVIEW_PROMPT` | — | Full review methodology, inline. Replaces the bundled default verbatim. | @@ -223,6 +218,12 @@ release. | `SESSION_IDLE_TIMEOUT_MS` / `SESSION_MAX_DURATION_MS` | — | Forwarded to api-rs executes. | | `GITHUBBOT_SHUTDOWN_DRAIN_MS` | — | How long to let in-flight turns finish on `SIGTERM` before exiting. Default `25000`; the chart derives it from the pod's termination grace period. | +The chart checksum includes the mounted private-key Secret's live resource +version. Rotate the Secret through the same Helm reconcile used for deployment; +the resulting pod-template change restarts githubbot, which reads the PEM once +at process startup. Do not edit the Secret out of band without reconciling the +release. + ## Tests `bun test test` — unit tests for the override flag parser, the GitHub thread-key parsing / context From 041ffca7d40db3208edd4911f240928e61ff2e7e Mon Sep 17 00:00:00 2001 From: Michael Wu Date: Sun, 30 Aug 2026 19:54:34 +0900 Subject: [PATCH 4/6] fix(githubbot): support App-owned workflows --- contrib/chart/Chart.yaml | 2 +- contrib/chart/templates/githubbot.yaml | 8 +- contrib/chart/values.yaml | 11 +- services/githubbot/README.md | 50 +++--- services/githubbot/src/body-mention.ts | 8 +- services/githubbot/src/index.ts | 21 ++- services/githubbot/src/issue-manager.ts | 152 ++++++++++++++---- services/githubbot/src/issue-prompt.ts | 8 +- services/githubbot/src/pr-manager.ts | 82 +++++++--- services/githubbot/src/review.ts | 6 +- services/githubbot/src/server.ts | 29 +++- services/githubbot/src/types.ts | 10 ++ services/githubbot/test/body-mention.test.ts | 13 +- services/githubbot/test/github-auth.test.ts | 29 +++- services/githubbot/test/issue-manager.test.ts | 70 ++++++++ services/githubbot/test/pr-manager.test.ts | 43 +++++ services/githubbot/test/review.test.ts | 9 ++ 17 files changed, 458 insertions(+), 93 deletions(-) diff --git a/contrib/chart/Chart.yaml b/contrib/chart/Chart.yaml index e3c0a1e5e..f5a7ef597 100644 --- a/contrib/chart/Chart.yaml +++ b/contrib/chart/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 name: centaur description: Helm chart for the trusted Centaur control plane type: application -version: 0.1.134 +version: 0.1.135 appVersion: "0.1.0" dependencies: - name: connect diff --git a/contrib/chart/templates/githubbot.yaml b/contrib/chart/templates/githubbot.yaml index c0a6d4b15..a0607b330 100644 --- a/contrib/chart/templates/githubbot.yaml +++ b/contrib/chart/templates/githubbot.yaml @@ -104,7 +104,11 @@ spec: name: {{ include "centaur.secretEnvName" . }} key: {{ printf "%sDATABASE_URL" .Values.secretManager.envPrefix }} - name: GITHUB_BOT_USERNAME - value: {{ required "githubbot.userName is required (the bot account's GitHub login)" .Values.githubbot.userName | quote }} + value: {{ required "githubbot.userName is required (App mention slug or PAT account login)" .Values.githubbot.userName | quote }} +{{- if .Values.githubbot.botActorLogin }} + - name: GITHUB_BOT_ACTOR_LOGIN + value: {{ .Values.githubbot.botActorLogin | quote }} +{{- end }} - name: GITHUBBOT_REPOSITORY_ALLOWLIST value: {{ required "githubbot.repositoryAllowlist is required when githubbot is enabled" .Values.githubbot.repositoryAllowlist | quote }} # v2 PR self-management knobs. @@ -112,6 +116,8 @@ spec: value: {{ .Values.githubbot.autoMerge | quote }} - name: GITHUBBOT_MERGE_METHOD value: {{ .Values.githubbot.mergeMethod | quote }} + - name: GITHUBBOT_OWNERSHIP_LABEL + value: {{ required "githubbot.ownershipLabel is required when githubbot is enabled" .Values.githubbot.ownershipLabel | quote }} - name: GITHUBBOT_REVIEW_MAX_ROUNDS_PER_EPOCH value: {{ .Values.githubbot.reviewMaxRoundsPerEpoch | quote }} - name: GITHUBBOT_REVIEW_MAX_TOTAL_ROUNDS_PER_EPOCH diff --git a/contrib/chart/values.yaml b/contrib/chart/values.yaml index a8ddc14dc..8d8cfbe94 100644 --- a/contrib/chart/values.yaml +++ b/contrib/chart/values.yaml @@ -659,8 +659,8 @@ linearbot: # /api/webhooks/github (the @chat-adapter/github adapter), answers in the # comment thread, and forwards sessions to the api-rs control plane (:8080). # Disabled by default: requires GITHUBBOT_WEBHOOK_SECRET plus either the -# githubApp block or the legacy GITHUBBOT_TOKEN secret. userName must match the -# App bot or teammate login so @-mention matching works. +# githubApp block or the legacy GITHUBBOT_TOKEN secret. userName is the App +# mention slug (without [bot]) or teammate login so @-mention matching works. githubbot: enabled: false # Singleton controller: per-PR admission and merge gates are process-serialized. @@ -671,6 +671,9 @@ githubbot: tag: latest pullPolicy: Always userName: "" + # Optional exact login recorded on bot-authored events. App deployments use + # [bot]; when empty, githubbot derives it from userName in App mode. + botActorLogin: "" # Prefer a GitHub App installation for production. When enabled, the PAT # secret key is not referenced; the PEM is mounted read-only and Octokit # transparently rotates short-lived installation tokens. @@ -681,7 +684,9 @@ githubbot: existingSecretName: "" privateKeySecretKey: private-key.pem privateKeyMountPath: /var/run/centaur/github-app - # v2 PR self-management (only acts on PRs assigned to the bot account). + # v2 PR self-management acts on bot-authored PRs, PAT-account assignments, or + # subjects carrying this explicit App-compatible handoff label. + ownershipLabel: centaur-managed # Auto-merge respects branch protection and is paused per-PR by the hold label # / draft status. autoMerge: true diff --git a/services/githubbot/README.md b/services/githubbot/README.md index 07ec7068e..2e34400cf 100644 --- a/services/githubbot/README.md +++ b/services/githubbot/README.md @@ -7,9 +7,10 @@ and the bot answers *in the thread* with a comment. It's built on the official the session logic (`session-api.ts`) and rendering are the same as the other bots; the Rust `api-rs` control plane is unchanged (`github:…` thread keys flow through identically). -The bot acts as a **real GitHub teammate**: it authenticates with a personal access token on a -dedicated machine-user account, so it can be `@`-mentioned, assigned, and **requested as a -reviewer** like any other collaborator. +The bot authenticates as either a preferred GitHub App installation or a dedicated +machine-user PAT. Both identities can be `@`-mentioned. PAT accounts can also be assigned and +requested as reviewers; App deployments use the explicit `centaur-managed` ownership label because +Apps cannot be assignees or requested reviewers. ## Behavior @@ -26,7 +27,7 @@ reviewer** like any other collaborator. a turn — the agent runs in a write-capable sandbox and posts its transcript back, so untrusted commenters can't steer it. Widen or open it with `GITHUBBOT_ALLOWED_AUTHOR_ASSOCIATIONS` (`*` allows everyone, e.g. a fully-private repo). Every path also requires an exact match in - `GITHUBBOT_REPOSITORY_ALLOWLIST`; lifecycle triggers (assignment, review-request) are gated by the + `GITHUBBOT_REPOSITORY_ALLOWLIST`; lifecycle triggers (ownership handoff, review-request) are gated by the same repository boundary plus GitHub permissions. - **`@`-mentioning the bot in the body of a newly-opened issue or PR** (the description, not a comment) → the same conversational turn runs, keyed to that issue/PR thread, with the reply posted @@ -48,10 +49,11 @@ reviewer** like any other collaborator. deployment can **fully replace** via `GITHUBBOT_REVIEW_PROMPT` / `GITHUBBOT_REVIEW_PROMPT_FILE` (the override is used verbatim, so org conventions supersede ours wholesale; for Splits this is where the overlay supplies its review guide). Webhook redeliveries are de-duplicated by delivery id. -- **Assigning an issue to the bot** (`issues` / `assigned` to the bot account) → an autonomous work +- **Handing an issue to the bot** (assigning the PAT account, or applying the configured ownership + label in App or PAT mode) → an autonomous work turn runs on a **dedicated, isolated session thread** (`github-issue:{owner}/{repo}:{n}`): the agent - reads the issue, implements a fix in its sandbox, and opens a PR (self-assigning it so it then - manages that PR toward merge). Like reviews, this lifecycle event is handled directly (githubbot + reads the issue, implements a fix in its sandbox, and opens a PR carrying the same ownership label + so it then manages that PR toward merge. Like reviews, this lifecycle event is handled directly (githubbot verifies the signature) and de-duplicated by delivery id. The **issue-work methodology** is a bundled, standalone default (`src/issue-prompt.ts`) that a deployment can **fully replace** via `GITHUBBOT_ISSUE_PROMPT` / `GITHUBBOT_ISSUE_PROMPT_FILE` (used verbatim, like the review prompt). @@ -62,12 +64,14 @@ reviewer** like any other collaborator. ## PR self-management (v2) -For PRs the bot **owns** — i.e. **assigned to the bot account** — githubbot drives the PR toward merge -by reacting to lifecycle webhooks. Ownership is purely an assignment mechanism: assign a PR to the bot -to have it take over, and unassign to hand it back. It only ever acts on owned PRs, and on a dedicated -management thread (`github-manage:{owner}/{repo}:{n}`); the agent does its GitHub writes via `gh`. +For PRs the bot **owns**—authored by its exact actor login, assigned to its PAT account, or carrying +the configured ownership label—githubbot drives the PR toward merge by reacting to lifecycle +webhooks. Remove the ownership label (and PAT assignment, if present) to hand a human-authored PR +back. It only ever acts on owned PRs, and on a dedicated management thread +(`github-manage:{owner}/{repo}:{n}`); the agent does its GitHub writes via `gh`. -- **Take over on assign.** Being assigned a PR is the explicit signal to take it over, so the bot +- **Take over on handoff.** Assignment or application of the ownership label is an explicit signal, + so the bot immediately evaluates CI (fixing red or merging green) rather than waiting for the next lifecycle event. - **Fix CI.** When **all** checks for a head SHA are settled (not per failing job — interwoven jobs @@ -75,7 +79,7 @@ management thread (`github-manage:{owner}/{repo}:{n}`); the agent does its GitHu `GITHUBBOT_CI_FIX_MAX_ATTEMPTS` consecutive attempts (default 3, reset when CI goes green); on exhaustion the bot comments tagging a human and stops. On the steady-state CI path it backs off if the failing head commit was authored by a human (it won't step on someone mid-edit) — except right - after assignment, where being assigned is an explicit hand-off, so it fixes the PR regardless of who + after an explicit handoff, when it fixes the PR regardless of who pushed last. - **Address review.** A submitted review (`changes_requested` / `commented`) triggers one holistic turn that reads all the feedback, validates each finding against reachable code @@ -120,7 +124,7 @@ management thread (`github-manage:{owner}/{repo}:{n}`); the agent does its GitHu - **Owned-PR conversation.** An @-mention in an owned PR's conversation (or a review-comment thread) runs in that PR's management session too — so the bot answers with the context of the CI fixes and review work it's been doing on the PR — while the rendered reply still posts to the comment thread. - An @-mention in the conversation of an **issue assigned to the bot** likewise runs in that issue's + An @-mention in the conversation of an **issue owned by the bot** likewise runs in that issue's work session (`github-issue:…`), so the bot replies with the context of the work it's doing on it. > **Scope.** v2 targets **same-repo PRs on repos you control** (where you own the webhook). The @@ -165,13 +169,16 @@ Use exactly one controller identity: `GITHUB_TOKEN`. Keep it distinct from any repo-cache or sandbox token. Do not configure both modes. The bot fails startup on missing, partial, or mixed -credentials. GitHub Apps are not normal user accounts, so assignment and -requested-review flows may require a teammate PAT; signed comment mentions and -PR/issue lifecycle management work with the App installation identity. +credentials. In App mode, `GITHUB_BOT_USERNAME` is the mention slug and the +controller separately recognizes the event actor as `slug[bot]` (override with +`GITHUB_BOT_ACTOR_LOGIN` only when needed). Because Apps are not normal user +accounts, assignment and requested-review flows require a teammate PAT. App +deployments use `GITHUBBOT_OWNERSHIP_LABEL` for explicit PR/issue handoff, and +App-authored PRs are owned automatically. Webhook events to subscribe: **Issue comments**, **Pull request review comments**, **Issues**, **Pull requests**, **Pull request reviews**, **Check runs**, **Check suites**, and **Workflow runs** -(**Issues** drives issue-work-on-assignment; the last four drive v2 PR self-management). +(**Issues** drives assignment/ownership-label issue work; the last four drive v2 PR self-management). ## Environment @@ -184,10 +191,10 @@ requests**, **Pull request reviews**, **Check runs**, **Check suites**, and **Wo | `GITHUB_PRIVATE_KEY` | App mode | Inline PEM compatibility input. | | `GITHUB_WEBHOOK_SECRET` | ✅ | Webhook signing secret (or `GITHUBBOT_WEBHOOK_SECRET`). | | `GITHUB_BOT_USERNAME` | ✅ | Mention name used by the bot. For an App, use its slug without the `[bot]` suffix; for a teammate PAT, use the account login (or `GITHUBBOT_USER_NAME`). | +| `GITHUB_BOT_ACTOR_LOGIN` | — | Exact login on bot-authored events. Defaults to `GITHUB_BOT_USERNAME[bot]` in App mode and `GITHUB_BOT_USERNAME` in PAT mode. | | `GITHUBBOT_DATABASE_URL` | ✅ | Postgres for chat-SDK state (falls back to `DATABASE_URL` / `POSTGRES_URL`). | | `GITHUBBOT_REPOSITORY_ALLOWLIST` | ✅ | Comma-separated exact `owner/repository` names. Empty/unset is rejected at startup; wildcards are not supported. Signed events for other repositories are acknowledged but ignored before chat state or agent work is created. | | `CENTAUR_API_URL` | — | api-rs control plane, default `http://127.0.0.1:8080`. | - | `GITHUBBOT_API_KEY` | — | Dedicated bearer sent to api-rs. | | `GITHUBBOT_DEFAULT_HARNESS` | — | Harness for new threads without an inline flag, default `codex`. | | `GITHUBBOT_REVIEW_PROMPT` | — | Full review methodology, inline. Replaces the bundled default verbatim. | @@ -203,6 +210,7 @@ requests**, **Pull request reviews**, **Check runs**, **Check suites**, and **Wo | `GITHUBBOT_LOG_LEVEL` | — | `debug`/`info`/`warn`/`error`, default `info`. | | `GITHUBBOT_AUTO_MERGE` | — | Auto-merge owned PRs when mergeable. Default `true`. | | `GITHUBBOT_MERGE_METHOD` | — | `merge` / `squash` / `rebase`. Default `squash`. | +| `GITHUBBOT_OWNERSHIP_LABEL` | — | Exact App-compatible PR/issue handoff label. Default `centaur-managed`; must differ from the review-reset label. | | `GITHUBBOT_HOLD_LABEL` | — | Label that pauses auto-merge. Default `do-not-merge`. | | `GITHUBBOT_CI_FIX_MAX_ATTEMPTS` | — | Consecutive CI-fix attempts before escalating. Default 3. | | `GITHUBBOT_REVIEW_MAX_ROUNDS_PER_EPOCH` | — | Review heads handled per reviewer within one epoch. Default 3 (initial review plus two validations). | @@ -227,7 +235,7 @@ release. ## Tests `bun test test` — unit tests for the override flag parser, the GitHub thread-key parsing / context -preamble, the review-request trigger gating (incl. team requests), the issue-assignment gating, the -v2 PR-manager decision logic (CI evaluation, assignment-based ownership, merge gating, the CI-fix +preamble, the review-request trigger gating (incl. team requests), the issue-ownership handoff, the +v2 PR-manager decision logic (CI evaluation, actor/label/assignment ownership, merge gating, the CI-fix counter / escalation, and the merge-claim release-on-failure), the author-association gate, body mentions, and the per-session serialization queue. diff --git a/services/githubbot/src/body-mention.ts b/services/githubbot/src/body-mention.ts index 6ac5e8459..3f4dbc402 100644 --- a/services/githubbot/src/body-mention.ts +++ b/services/githubbot/src/body-mention.ts @@ -49,7 +49,13 @@ export function handleBodyMention( // Never act on the bot's own issue/PR (it opens PRs during issue work). const author = stringValue(isRecord(node.user) ? node.user.login : undefined); - if (author && author.toLowerCase() === ctx.userName.toLowerCase()) return null; + if ( + author && + author.toLowerCase() === + (ctx.botActorLogin ?? ctx.userName).toLowerCase() + ) { + return null; + } // Same trust gate as the comment path, read from the issue/PR author. const allowed = resolveAllowedAuthorAssociations( diff --git a/services/githubbot/src/index.ts b/services/githubbot/src/index.ts index 62bb7ff8a..479dadce5 100644 --- a/services/githubbot/src/index.ts +++ b/services/githubbot/src/index.ts @@ -26,7 +26,7 @@ import { handleBodyMention } from "./body-mention"; import { backgroundWaitUntil, requestContext, waitUntil } from "./context"; import { handleIssueEvent, - isIssueAssignedToBot, + isIssueOwnedByBot, issueWorkThreadKey, } from "./issue-manager"; import { extractMessageOverrides } from "./overrides"; @@ -71,8 +71,19 @@ const POSTGRES_CONNECT_INITIAL_DELAY_MS = 250; const POSTGRES_CONNECT_MAX_DELAY_MS = 10_000; const DEDUP_WINDOW = 200; +export function resolveBotActorLogin( + options: Pick, + userName: string, +): string { + return ( + options.botActorLogin?.trim() || + (options.githubAppClientId ? `${userName}[bot]` : userName) + ); +} + export function createGithubbot(options: GithubbotOptions): Githubbot { const userName = options.userName ?? "github-bot"; + const botActorLogin = resolveBotActorLogin(options, userName); const logger = options.logger ?? noopLogger; const github = createGitHubAdapter({ ...resolveGithubAdapterAuth(options), @@ -140,6 +151,7 @@ export function createGithubbot(options: GithubbotOptions): Githubbot { }; const prManagerCtx: PrManagerContext = { + botActorLogin, octokit: github.octokit, options, state, @@ -203,6 +215,7 @@ export function createGithubbot(options: GithubbotOptions): Githubbot { const handled = requestContext.run(context, () => Promise.all([ routeLifecycleEvent(eventType, rawBody, { + botActorLogin, botUserName: userName, deliveryId, options, @@ -444,7 +457,7 @@ function parseWebhookPayload(rawBody: string): unknown { * session key so the turn shares the sandbox/context the bot uses for it — while * replies still post to this thread. For an owned PR that's the management * session (`github-manage:…`, where it fixes CI and addresses reviews); for an - * issue assigned to the bot it's the issue-work session (`github-issue:…`). + * issue owned by the bot it's the issue-work session (`github-issue:…`). * Returns undefined when the thread maps to neither (the turn then runs on its * own conversation session) or on lookup failure. The resolved key is cached on * the thread so follow-ups skip the lookup. @@ -465,7 +478,7 @@ async function resolveManagementSession( sessionKey = managementThreadKey(ref.owner, ref.repo, ref.number); } } else if ( - await isIssueAssignedToBot( + await isIssueOwnedByBot( input.prManagerCtx, ref.owner, ref.repo, @@ -545,6 +558,7 @@ function routeLifecycleEvent( eventType: string, rawBody: string, input: { + botActorLogin: string; botUserName: string; deliveryId: string; options: GithubbotOptions; @@ -555,6 +569,7 @@ function routeLifecycleEvent( if (eventType === "pull_request") { if (pullRequestAction(rawBody) === "review_requested") { return handleReviewRequest(rawBody, { + botActorLogin: input.botActorLogin, botUserName: input.botUserName, deliveryId: input.deliveryId, octokit: input.prManagerCtx.octokit, diff --git a/services/githubbot/src/issue-manager.ts b/services/githubbot/src/issue-manager.ts index 4fdb7c814..26278ec89 100644 --- a/services/githubbot/src/issue-manager.ts +++ b/services/githubbot/src/issue-manager.ts @@ -1,6 +1,9 @@ import { backgroundWaitUntil } from "./context"; import { DEFAULT_ISSUE_PROMPT } from "./issue-prompt"; -import type { PrManagerContext } from "./pr-manager"; +import { + DEFAULT_OWNERSHIP_LABEL, + type PrManagerContext, +} from "./pr-manager"; import { reactWorkingOnSubject, settleSubjectReaction } from "./reactions"; import { runTurnStream } from "./turn"; import type { @@ -11,17 +14,17 @@ import type { import { errorMessage, noopLogger, nowMs, stringValue, traceLog } from "./utils"; /** - * Issues, like PRs, are worked on assignment: assigning an issue to the bot is - * the signal to pick it up. On the `issues` `assigned` webhook (when the bot is - * among the assignees), the bot runs an autonomous work turn — read the issue, - * implement a fix, and open a PR (self-assigning that PR so it then drives it to - * merge via the PR-management flow). The methodology is the bundled + * Issues, like PRs, are worked after an explicit ownership handoff: assignment + * to a PAT-backed teammate or the configured App-compatible ownership label. + * The bot runs an autonomous work turn — read the issue, implement a fix, and + * open a PR marked with that ownership label so PR management continues it. + * The methodology is the bundled * DEFAULT_ISSUE_PROMPT unless the deployment fully replaces it via * options.issuePrompt. * * The work runs on its own isolated session thread (`github-issue:{owner}/{repo}: * {n}`), kept separate from the issue's conversation thread so a work run never - * shares a sandbox with chit-chat — but persistent per issue, so a re-assignment + * shares a sandbox with chit-chat — but persistent per issue, so a fresh handoff * builds on the prior attempt. The agent does all GitHub I/O via `gh`, so the bot * does not post through the adapter. */ @@ -33,7 +36,7 @@ type IssueManagerContext = PrManagerContext; // Assignment webhooks are de-duplicated by delivery id for a week — long enough // to cover GitHub's redelivery window without growing state unboundedly. const ISSUE_WORK_DEDUP_TTL_MS = 7 * 24 * 60 * 60 * 1000; -const ASSIGNED_CACHE_TTL_MS = 10 * 60 * 1000; +const OWNED_CACHE_TTL_MS = 10 * 60 * 1000; export function issueWorkThreadKey( owner: string, @@ -43,7 +46,7 @@ export function issueWorkThreadKey( return `github-issue:${owner}/${repo}:${n}`; } -/** `issues` lifecycle: on `assigned` to the bot, run an autonomous work turn. */ +/** `issues` lifecycle: on an explicit ownership handoff, run an autonomous turn. */ export function handleIssueEvent( ctx: IssueManagerContext, rawBody: string, @@ -51,15 +54,29 @@ export function handleIssueEvent( ): Promise | null { const payload = parseJson(rawBody); if (!payload) return null; - if (stringValue(payload.action) !== "assigned") return null; + const action = stringValue(payload.action); const issue = isRecord(payload.issue) ? payload.issue : null; const repo = repoFromPayload(payload); if (!issue || !repo) return null; const number = numberValue(issue.number); if (number === undefined) return null; if (stringValue(issue.state) !== "open") return null; - if (!isAssignedToBot(assigneeLogins(issue.assignees), ctx.userName)) { - // A different assignee — not ours to act on. + const ownershipLabel = + ctx.options.ownershipLabel ?? DEFAULT_OWNERSHIP_LABEL; + const eventLabel = stringValue( + isRecord(payload.label) ? payload.label.name : undefined, + ); + if ( + !isIssueWorkSignal({ + action, + assignees: assigneeLogins(issue.assignees), + botActorLogin: ctx.botActorLogin, + eventLabel, + labels: labelNames(issue.labels), + ownershipLabel, + userName: ctx.userName, + }) + ) { return null; } @@ -68,7 +85,7 @@ export function handleIssueEvent( const url = stringValue(issue.html_url) ?? `https://github.com/${repo.owner}/${repo.repo}/issues/${number}`; - const assigner = + const requester = stringValue(isRecord(payload.sender) ? payload.sender.login : undefined) ?? "a teammate"; const threadKey = issueWorkThreadKey(repo.owner, repo.repo, number); @@ -105,11 +122,12 @@ export function handleIssueEvent( }); return; } - traceLog(options, "githubbot_issue_assigned", trace, { - assigner, + traceLog(options, "githubbot_issue_work_requested", trace, { issue: `${repo.owner}/${repo.repo}#${number}`, + requester, + signal: action, }); - // No triggering comment on an assignment, so ack on the issue itself — + // No triggering comment on a lifecycle handoff, so ack on the issue itself — // instant 👀, settled to 🚀/😕 when the work turn finishes. await reactWorkingOnSubject(ctx.octokit, repo.owner, repo.repo, number, logger); @@ -121,11 +139,12 @@ export function handleIssueEvent( contextPreamble: options.issuePrompt ?? DEFAULT_ISSUE_PROMPT, conversationName: `${repo.owner}/${repo.repo}#${number}: ${title}`, executeMessage: issueTriggerMessage({ - assigner, deliveryId, number, + ownershipLabel, owner: repo.owner, repo: repo.repo, + requester, threadKey, title, url, @@ -174,17 +193,17 @@ export function handleIssueEvent( } /** - * Whether an issue is assigned to the bot, cached briefly so the conversational + * Whether an issue is owned by the bot, cached briefly so the conversational * path doesn't hit the API on every comment. Mirrors the PR manager's isPrOwned; * a stale result only affects which session a reply shares context with. */ -export async function isIssueAssignedToBot( +export async function isIssueOwnedByBot( ctx: IssueManagerContext, owner: string, repo: string, number: number, ): Promise { - const cacheKey = `${ctx.options.stateKeyPrefix ?? "centaur-githubbot"}:issue-assigned-cache:${owner}/${repo}#${number}`; + const cacheKey = `${ctx.options.stateKeyPrefix ?? "centaur-githubbot"}:issue-owned-cache:${owner}/${repo}#${number}`; try { const cached = await ctx.state.get(cacheKey); if (cached === "1") return true; @@ -192,46 +211,51 @@ export async function isIssueAssignedToBot( } catch { // fall through to a live lookup } - let assigned = false; + let owned = false; try { const { data } = await ctx.octokit.rest.issues.get({ owner, repo, issue_number: number, }); - assigned = isAssignedToBot( - assigneeLogins(data.assignees), - ctx.userName, - ); + owned = isIssueOwned({ + assignees: assigneeLogins(data.assignees), + botActorLogin: ctx.botActorLogin, + labels: labelNames(data.labels), + ownershipLabel: ctx.options.ownershipLabel, + userName: ctx.userName, + }); } catch (error) { (ctx.options.logger ?? noopLogger).debug( - "githubbot_issue_assignment_lookup_failed", + "githubbot_issue_ownership_lookup_failed", { error: errorMessage(error) }, ); return false; } try { - await ctx.state.set(cacheKey, assigned ? "1" : "0", ASSIGNED_CACHE_TTL_MS); + await ctx.state.set(cacheKey, owned ? "1" : "0", OWNED_CACHE_TTL_MS); } catch { // best-effort cache } - return assigned; + return owned; } function issueTriggerMessage(input: { - assigner: string; deliveryId: string; number: number; + ownershipLabel: string; owner: string; repo: string; + requester: string; threadKey: string; title: string; url: string; }): GithubbotApiMessage { const text = - `You have been assigned GitHub issue ${input.owner}/${input.repo}#${input.number} — ` + - `"${input.title}" (${input.url}) by @${input.assigner}. Work it now, following ` + - `your guidance above, using the gh CLI and git in your sandbox.`; + `Centaur work was requested for GitHub issue ${input.owner}/${input.repo}#${input.number} — ` + + `"${input.title}" (${input.url}) by @${input.requester}. Work it now, following ` + + `your guidance above, using the gh CLI and git in your sandbox. Mark the resulting ` + + `pull request with the exact ownership label ${JSON.stringify(input.ownershipLabel)}.`; return { attachments: [], author: { @@ -241,8 +265,8 @@ function issueTriggerMessage(input: { userId: "github-issue", userName: "github-issue", }, - // Keyed by delivery id so a fresh assignment re-executes (the state claim - // dedupes a redelivery of the same assignment). + // Keyed by delivery id so a fresh handoff re-executes (the state claim + // dedupes a redelivery of the same lifecycle event). id: `issue-${input.threadKey}-${input.deliveryId}`, isMention: true, raw: { githubbotIssueWork: true, url: input.url }, @@ -263,6 +287,52 @@ export function isAssignedToBot(assignees: string[], userName: string): boolean return assignees.some((login) => login.toLowerCase() === target); } +export function isIssueOwned(input: { + assignees: string[]; + botActorLogin?: string; + labels: string[]; + ownershipLabel?: string; + userName: string; +}): boolean { + const ownershipLabel = ( + input.ownershipLabel ?? DEFAULT_OWNERSHIP_LABEL + ).toLowerCase(); + const assignmentSupported = + (input.botActorLogin ?? input.userName).toLowerCase() === + input.userName.toLowerCase(); + return ( + (assignmentSupported && + isAssignedToBot(input.assignees, input.userName)) || + input.labels.some((label) => label.toLowerCase() === ownershipLabel) + ); +} + +export function isIssueWorkSignal(input: { + action?: string; + assignees: string[]; + botActorLogin?: string; + eventLabel?: string; + labels: string[]; + ownershipLabel?: string; + userName: string; +}): boolean { + if ( + input.action === "assigned" && + (input.botActorLogin ?? input.userName).toLowerCase() === + input.userName.toLowerCase() && + isAssignedToBot(input.assignees, input.userName) + ) { + return true; + } + const ownershipLabel = + input.ownershipLabel ?? DEFAULT_OWNERSHIP_LABEL; + return ( + input.action === "labeled" && + input.eventLabel?.toLowerCase() === ownershipLabel.toLowerCase() && + isIssueOwned(input) + ); +} + export function assigneeLogins(value: unknown): string[] { if (!Array.isArray(value)) return []; const logins: string[] = []; @@ -273,6 +343,20 @@ export function assigneeLogins(value: unknown): string[] { return logins; } +export function labelNames(value: unknown): string[] { + if (!Array.isArray(value)) return []; + const labels: string[] = []; + for (const entry of value) { + if (typeof entry === "string" && entry) { + labels.push(entry); + continue; + } + const name = isRecord(entry) ? stringValue(entry.name) : undefined; + if (name) labels.push(name); + } + return labels; +} + function isRecord(value: unknown): value is JsonRecord { return Boolean(value && typeof value === "object" && !Array.isArray(value)); } diff --git a/services/githubbot/src/issue-prompt.ts b/services/githubbot/src/issue-prompt.ts index 948cd17e1..c2535bce5 100644 --- a/services/githubbot/src/issue-prompt.ts +++ b/services/githubbot/src/issue-prompt.ts @@ -1,5 +1,5 @@ /** - * The default methodology used when an issue is assigned to the bot. A full, + * The default methodology used when an issue is handed to the bot. A full, * standalone "issue-work system prompt": competent and safe out of the box, with * no org-specific assumptions. A deployment can fully replace it (e.g. the Splits * overlay points GITHUBBOT_ISSUE_PROMPT_FILE at its own playbook) — the override @@ -9,11 +9,11 @@ * This rides as the issue-work turn's context preamble; the specific issue being * worked is supplied separately as the turn's message. */ -export const DEFAULT_ISSUE_PROMPT = `You have been assigned a GitHub issue to work. Act as a careful, autonomous teammate, working entirely from your sandbox using the gh CLI and git. +export const DEFAULT_ISSUE_PROMPT = `A GitHub issue has been explicitly handed to you to work. Act as a careful, autonomous teammate, working entirely from your sandbox using the gh CLI and git. Understand the work before touching anything: - Read the issue: \`gh issue view \` for the body and \`gh issue view --comments\` for the discussion. Follow any links and read the referenced code in context. -- Decide what "done" means before you write code. If it's a bug, reproduce it first so you can prove your fix. If the ask is ambiguous, underspecified, or larger than one coherent change, do NOT guess: post a comment on the issue explaining what you'd need to proceed (or how you'd split it up), @-mention the person who assigned you, and stop there. +- Decide what "done" means before you write code. If it's a bug, reproduce it first so you can prove your fix. If the ask is ambiguous, underspecified, or larger than one coherent change, do NOT guess: post a comment on the issue explaining what you'd need to proceed (or how you'd split it up), @-mention the person who requested the work, and stop there. Implement the change: - Work on a new branch off the default branch. Make the smallest coherent change that resolves the issue, matching the conventions of the surrounding code. @@ -22,7 +22,7 @@ Implement the change: Open a pull request: - Push your branch and open a PR that closes the issue (e.g. "Closes #" in the body). Keep the description brief and in plain prose — what changed and how to verify, not a code walkthrough. -- Assign the PR to yourself, so you keep managing it through review and CI to merge. +- Add the exact Centaur ownership label named in the trigger message to the PR, so you keep managing it through review and CI to merge. If the label does not exist or cannot be applied, report that on the issue and stop rather than silently leaving an unmanaged PR. - Comment on the issue linking the PR. Do not merge the PR here — opening it hands off to your PR-management flow. If at any point you can't make confident progress, stop and ask on the issue rather than pushing a guess.`; diff --git a/services/githubbot/src/pr-manager.ts b/services/githubbot/src/pr-manager.ts index 1d16d8e4a..924ef54de 100644 --- a/services/githubbot/src/pr-manager.ts +++ b/services/githubbot/src/pr-manager.ts @@ -52,12 +52,16 @@ export type PrManagerContext = { octokit: Octokit; options: GithubbotOptions; state: StateAdapter; + /** App actor login (`slug[bot]`) or the PAT account login. */ + botActorLogin?: string; + /** Mention slug for App mode, or the PAT account login. */ userName: string; }; const STATE_TTL_MS = 90 * 24 * 60 * 60 * 1000; const CLAIM_TTL_MS = 7 * 24 * 60 * 60 * 1000; const DEFAULT_CI_FIX_MAX_ATTEMPTS = 3; +export const DEFAULT_OWNERSHIP_LABEL = "centaur-managed"; const REVIEW_STATE_RETRY_DELAYS_MS = [0, 100, 500, 1_000, 5_000, 10_000, 30_000]; // --------------------------------------------------------------------------- @@ -65,16 +69,31 @@ const REVIEW_STATE_RETRY_DELAYS_MS = [0, 100, 500, 1_000, 5_000, 10_000, 30_000] // --------------------------------------------------------------------------- /** - * A PR is bot-owned when the bot is one of its assignees. Ownership is purely an - * assignment mechanism: assign the PR to the bot to have it manage the PR toward - * merge (and unassign to hand it back). + * A PR is bot-owned when the bot authored it, is an assignee (PAT mode), or the + * configured ownership label is present (the explicit App-compatible handoff). */ export function isOwnedPr(input: { assignees: string[]; + author?: string; + botActorLogin?: string; + labels?: string[]; + ownershipLabel?: string; userName: string; }): boolean { - const target = input.userName.toLowerCase(); - return input.assignees.some((login) => login.toLowerCase() === target); + const mentionLogin = input.userName.toLowerCase(); + const actorLogin = (input.botActorLogin ?? input.userName).toLowerCase(); + const assignmentSupported = actorLogin === mentionLogin; + const ownershipLabel = ( + input.ownershipLabel ?? DEFAULT_OWNERSHIP_LABEL + ).toLowerCase(); + return ( + (assignmentSupported && + input.assignees.some((login) => login.toLowerCase() === mentionLogin)) || + input.author?.toLowerCase() === actorLogin || + (input.labels ?? []).some( + (label) => label.toLowerCase() === ownershipLabel, + ) + ); } export type MergeDecision = @@ -452,6 +471,7 @@ function logger(ctx: PrManagerContext) { type PullRequestSummary = { assignees: string[]; + author: string | null; draft: boolean; headRef: string; headRepoFullName: string | null; @@ -481,9 +501,11 @@ function summarizePr(pr: { state: string; title: string; assignees?: ({ login?: string } | null)[] | null; + user?: { login?: string | null } | null; }): PullRequestSummary { return { assignees: assigneeLogins(pr.assignees), + author: pr.user?.login ?? null, draft: pr.draft === true, headRef: pr.head.ref, headRepoFullName: pr.head.repo?.full_name ?? null, @@ -551,7 +573,14 @@ export async function isPrOwned( } function owns(ctx: PrManagerContext, pr: PullRequestSummary): boolean { - return isOwnedPr({ assignees: pr.assignees, userName: ctx.userName }); + return isOwnedPr({ + assignees: pr.assignees, + author: pr.author ?? undefined, + botActorLogin: ctx.botActorLogin, + labels: pr.labels, + ownershipLabel: ctx.options.ownershipLabel, + userName: ctx.userName, + }); } function reviewBudgetReviewerKey(user?: JsonRecord): string { @@ -583,6 +612,8 @@ export async function handlePullRequestEvent( const labelNode = payload.label; const label = isRecord(labelNode) ? stringValue(labelNode.name) : undefined; const resetLabel = ctx.options.reviewResetLabel ?? DEFAULT_REVIEW_RESET_LABEL; + const ownershipLabel = + ctx.options.ownershipLabel ?? DEFAULT_OWNERSHIP_LABEL; if ( action === "labeled" && label?.toLowerCase() === resetLabel.toLowerCase() @@ -637,12 +668,14 @@ export async function handlePullRequestEvent( ); return; } - // Being assigned the PR is the explicit signal to take it over: evaluate CI now - // (forcing past the human-commit back-off — the assignment is a human handing - // it to us) so an already-red or already-green PR is acted on immediately, - // rather than only on the next lifecycle event. processCi fixes red CI or merges - // when green. - if (action === "assigned") { + // Assignment (PAT mode) or the ownership label (App/PAT mode) is an explicit + // handoff. Evaluate CI now, forcing past human-commit back-off, so a PR that + // was already red or green does not wait for another lifecycle event. + if ( + action === "assigned" || + (action === "labeled" && + label?.toLowerCase() === ownershipLabel.toLowerCase()) + ) { await processCi(ctx, repo.owner, repo.repo, number, pr.headSha, true); return; } @@ -672,7 +705,13 @@ export async function handleReviewEvent( const reviewState = stringValue(reviewNode.state)?.toLowerCase(); // Submitted reviews on public repositories are not collaborator-only by // default. Gate before workflow emission, claims, or write-capable turns. - if (reviewer && reviewer.toLowerCase() === ctx.userName.toLowerCase()) return; + if ( + reviewer && + reviewer.toLowerCase() === + (ctx.botActorLogin ?? ctx.userName).toLowerCase() + ) { + return; + } if (!isReviewAuthorAllowed(payload, ctx.options)) { logger(ctx).warn("githubbot_review_author_denied", { pr: `${repo.owner}/${repo.repo}#${number}`, @@ -827,7 +866,8 @@ async function maybeRecordReviewResetApproval( if ( !sender || senderType !== "user" || - sender.toLowerCase() === ctx.userName.toLowerCase() + sender.toLowerCase() === + (ctx.botActorLogin ?? ctx.userName).toLowerCase() ) { logger(ctx).warn("githubbot_review_reset_denied", { pr: `${owner}/${repo}#${pr.number}`, @@ -995,7 +1035,9 @@ async function compareReviewChange( const totalCommits = typeof data.total_commits === "number" ? data.total_commits : commits.length; const kinds = new Set( - commits.map((commit) => commitActorKind(commit, ctx.userName)), + commits.map((commit) => + commitActorKind(commit, ctx.botActorLogin ?? ctx.userName), + ), ); let actor: ReviewChangeActor = "unknown"; if (totalCommits === commits.length && !kinds.has("unknown")) { @@ -1407,11 +1449,15 @@ async function processCi( } // Red: back off if a human pushed the failing commit (don't step on them) — - // unless this is a forced takeover (the PR was just assigned to us, so the - // human has explicitly handed it over and we fix it regardless of who pushed). + // unless this is a forced takeover (assignment or the ownership label is an + // explicit handoff, so we fix it regardless of who pushed). if (!force) { const headAuthor = await commitAuthor(ctx, owner, repo, headSha); - if (headAuthor && headAuthor.toLowerCase() !== ctx.userName.toLowerCase()) { + if ( + headAuthor && + headAuthor.toLowerCase() !== + (ctx.botActorLogin ?? ctx.userName).toLowerCase() + ) { traceLog(ctx.options, "githubbot_ci_human_commit_skipped", trace, { author: headAuthor, }); diff --git a/services/githubbot/src/review.ts b/services/githubbot/src/review.ts index 1e0d883d0..b42b1a6c1 100644 --- a/services/githubbot/src/review.ts +++ b/services/githubbot/src/review.ts @@ -13,6 +13,7 @@ import type { import { errorMessage, noopLogger, nowMs, stringValue, traceLog } from "./utils"; type ReviewHandlerInput = { + botActorLogin?: string; botUserName: string; deliveryId: string; octokit: GitHubAdapter["octokit"]; @@ -91,7 +92,10 @@ export function handleReviewRequest( // names a different individual reviewer and no team is not ours. const reviewer = stringValue(payload.requested_reviewer?.login); const directMatch = - !!reviewer && reviewer.toLowerCase() === input.botUserName.toLowerCase(); + !!reviewer && + [input.botUserName, input.botActorLogin ?? input.botUserName].some( + (login) => reviewer.toLowerCase() === login.toLowerCase(), + ); const teamSlug = stringValue(payload.requested_team?.slug); if (!directMatch && !teamSlug) return null; diff --git a/services/githubbot/src/server.ts b/services/githubbot/src/server.ts index 27dee7182..711133b6c 100644 --- a/services/githubbot/src/server.ts +++ b/services/githubbot/src/server.ts @@ -2,6 +2,8 @@ import { readFileSync } from "node:fs"; import { requireRepositoryAllowlist } from "./authorization"; import { drainBackgroundWork } from "./context"; import { createGithubbot, type GithubbotOptions } from "./index"; +import { DEFAULT_OWNERSHIP_LABEL } from "./pr-manager"; +import { DEFAULT_REVIEW_RESET_LABEL } from "./review-budget"; import { positiveIntegerValue } from "./utils"; const port = numberEnv("PORT", 3001); @@ -42,12 +44,29 @@ if (!webhookSecret) { throw new Error("GITHUB_WEBHOOK_SECRET (or GITHUBBOT_WEBHOOK_SECRET) is required"); } -// The bot's mention name. For an App this is its slug without the `[bot]` -// suffix; for PAT mode it is the teammate account login. +// Keep the human-facing mention slug separate from the actor login GitHub puts +// on App-authored events. Apps are mentioned as @slug but act as slug[bot]. const userName = optionalEnv("GITHUB_BOT_USERNAME") ?? optionalEnv("GITHUBBOT_USER_NAME"); if (!userName) { - throw new Error("GITHUB_BOT_USERNAME is required (the bot account's GitHub login)"); + throw new Error( + "GITHUB_BOT_USERNAME is required (App mention slug or PAT account login)", + ); +} +const botActorLogin = + optionalEnv("GITHUB_BOT_ACTOR_LOGIN") ?? + (githubAppClientId ? `${userName}[bot]` : userName); +if (githubAppClientId && !botActorLogin.toLowerCase().endsWith("[bot]")) { + throw new Error("GITHUB_BOT_ACTOR_LOGIN must end in [bot] for GitHub App auth"); +} +const ownershipLabel = + optionalEnv("GITHUBBOT_OWNERSHIP_LABEL") ?? DEFAULT_OWNERSHIP_LABEL; +const reviewResetLabel = + optionalEnv("GITHUBBOT_REVIEW_RESET_LABEL") ?? DEFAULT_REVIEW_RESET_LABEL; +if (ownershipLabel.toLowerCase() === reviewResetLabel.toLowerCase()) { + throw new Error( + "GITHUBBOT_OWNERSHIP_LABEL and GITHUBBOT_REVIEW_RESET_LABEL must differ", + ); } // Full review methodology override. Inline wins; otherwise a mounted file (the @@ -139,6 +158,7 @@ const options: GithubbotOptions = { allowedAuthorAssociations: listEnv("GITHUBBOT_ALLOWED_AUTHOR_ASSOCIATIONS"), apiKey: optionalEnv("GITHUBBOT_API_KEY"), autoMerge: boolEnv("GITHUBBOT_AUTO_MERGE", true), + botActorLogin, botUserId: optionalEnv("GITHUBBOT_USER_ID"), ciFixMaxAttempts: optionalNumberEnv("GITHUBBOT_CI_FIX_MAX_ATTEMPTS"), reviewMaxRoundsPerEpoch: optionalNumberEnv( @@ -155,7 +175,7 @@ const options: GithubbotOptions = { "GITHUBBOT_REVIEW_MATERIAL_CHANGE_FILES", ), reviewAuthorAllowlist: listEnv("GITHUBBOT_REVIEW_AUTHOR_ALLOWLIST"), - reviewResetLabel: optionalEnv("GITHUBBOT_REVIEW_RESET_LABEL"), + reviewResetLabel, workflowEvents: boolEnv("GITHUBBOT_WORKFLOW_EVENTS", false), deleteBranchOnMerge: boolEnv("GITHUBBOT_DELETE_BRANCH_ON_MERGE", true), escalationHandle: optionalEnv("GITHUBBOT_ESCALATION_HANDLE"), @@ -169,6 +189,7 @@ const options: GithubbotOptions = { reviewPrompt, issuePrompt, managementPrompt, + ownershipLabel, repositoryAllowlist: requireRepositoryAllowlist( listEnv("GITHUBBOT_REPOSITORY_ALLOWLIST"), ), diff --git a/services/githubbot/src/types.ts b/services/githubbot/src/types.ts index 575aec8bc..646b13937 100644 --- a/services/githubbot/src/types.ts +++ b/services/githubbot/src/types.ts @@ -182,6 +182,16 @@ export type GithubbotOptions = { githubAppInstallationId?: number; /** GitHub App PEM contents. Mount a Secret and read it at process startup. */ githubAppPrivateKey?: string; + /** + * Login GitHub records for actions by this identity. In App mode this is the + * App slug with `[bot]`; `userName` remains the mention slug. + */ + botActorLogin?: string; + /** + * Label that explicitly hands a PR or issue to lifecycle automation. Defaults + * to `centaur-managed`; useful for Apps, which cannot be assignees. + */ + ownershipLabel?: string; userName?: string; /** * GitHub `author_association` values allowed to drive the conversational diff --git a/services/githubbot/test/body-mention.test.ts b/services/githubbot/test/body-mention.test.ts index 81490dbfd..89ebba098 100644 --- a/services/githubbot/test/body-mention.test.ts +++ b/services/githubbot/test/body-mention.test.ts @@ -18,7 +18,7 @@ describe("mentionsBot", () => { type Spies = { reactions: number; comments: number }; -function makeCtx(spies: Spies): PrManagerContext { +function makeCtx(spies: Spies, botActorLogin?: string): PrManagerContext { const m = new Map(); const state = { get: async (k: string) => m.get(k), @@ -32,6 +32,7 @@ function makeCtx(spies: Spies): PrManagerContext { }, }; return { + botActorLogin, octokit: { rest: { reactions: { @@ -123,6 +124,16 @@ describe("handleBodyMention", () => { ).toBeNull(); }); + test("ignores an App-authored subject using the actor login", () => { + expect( + handleBodyMention( + makeCtx({ reactions: 0, comments: 0 }, "centaur-bot[bot]"), + "pull_request", + openedPr("@centaur-bot do it", "NONE", "centaur-bot[bot]"), + ), + ).toBeNull(); + }); + test("ignores an unauthorized author", () => { const spies = { reactions: 0, comments: 0 }; expect( diff --git a/services/githubbot/test/github-auth.test.ts b/services/githubbot/test/github-auth.test.ts index da608a904..8da52d49f 100644 --- a/services/githubbot/test/github-auth.test.ts +++ b/services/githubbot/test/github-auth.test.ts @@ -1,5 +1,8 @@ import { describe, expect, test } from "bun:test"; -import { resolveGithubAdapterAuth } from "../src/index"; +import { + resolveBotActorLogin, + resolveGithubAdapterAuth, +} from "../src/index"; import { positiveIntegerValue } from "../src/utils"; describe("GitHub authentication", () => { @@ -73,3 +76,27 @@ describe("GitHub installation ID parsing", () => { } }); }); + +describe("GitHub bot identity", () => { + test("keeps mention and actor logins separate for Apps", () => { + expect( + resolveBotActorLogin( + { githubAppClientId: "Iv1.example" }, + "centaur-bot", + ), + ).toBe("centaur-bot[bot]"); + }); + + test("uses the PAT login directly and honors an explicit actor", () => { + expect(resolveBotActorLogin({}, "centaur-bot")).toBe("centaur-bot"); + expect( + resolveBotActorLogin( + { + botActorLogin: "custom-app[bot]", + githubAppClientId: "Iv1.example", + }, + "centaur-bot", + ), + ).toBe("custom-app[bot]"); + }); +}); diff --git a/services/githubbot/test/issue-manager.test.ts b/services/githubbot/test/issue-manager.test.ts index 031577a52..36b956a27 100644 --- a/services/githubbot/test/issue-manager.test.ts +++ b/services/githubbot/test/issue-manager.test.ts @@ -2,7 +2,10 @@ import { describe, expect, test } from "bun:test"; import { assigneeLogins, isAssignedToBot, + isIssueOwned, + isIssueWorkSignal, issueWorkThreadKey, + labelNames, } from "../src/issue-manager"; describe("isAssignedToBot", () => { @@ -41,6 +44,73 @@ describe("assigneeLogins", () => { }); }); +describe("App-compatible issue ownership", () => { + test("recognizes the configured label case-insensitively", () => { + expect( + isIssueOwned({ + assignees: [], + labels: ["Centaur-Managed"], + ownershipLabel: "centaur-managed", + userName: "centaur-bot", + }), + ).toBe(true); + }); + + test("only a matching labeled event starts label-based work", () => { + const base = { + assignees: [] as string[], + labels: ["centaur-managed"], + ownershipLabel: "centaur-managed", + userName: "centaur-bot", + }; + expect( + isIssueWorkSignal({ + ...base, + action: "labeled", + eventLabel: "Centaur-Managed", + }), + ).toBe(true); + expect( + isIssueWorkSignal({ ...base, action: "opened", eventLabel: "centaur-managed" }), + ).toBe(false); + expect( + isIssueWorkSignal({ ...base, action: "labeled", eventLabel: "bug" }), + ).toBe(false); + }); + + test("retains PAT assignment as an explicit work signal", () => { + expect( + isIssueWorkSignal({ + action: "assigned", + assignees: ["Centaur-Bot"], + labels: [], + userName: "centaur-bot", + }), + ).toBe(true); + }); + + test("does not treat an App mention slug as an assignable account", () => { + expect( + isIssueWorkSignal({ + action: "assigned", + assignees: ["centaur-bot"], + botActorLogin: "centaur-bot[bot]", + labels: [], + userName: "centaur-bot", + }), + ).toBe(false); + }); +}); + +describe("labelNames", () => { + test("accepts GitHub's string and object label shapes", () => { + expect(labelNames(["bug", { name: "centaur-managed" }, null, {}])).toEqual([ + "bug", + "centaur-managed", + ]); + }); +}); + describe("issueWorkThreadKey", () => { test("builds the isolated work-session key", () => { expect(issueWorkThreadKey("0xSplits", "centaur", 7)).toBe( diff --git a/services/githubbot/test/pr-manager.test.ts b/services/githubbot/test/pr-manager.test.ts index a047eb01e..81014598f 100644 --- a/services/githubbot/test/pr-manager.test.ts +++ b/services/githubbot/test/pr-manager.test.ts @@ -126,6 +126,49 @@ describe("isOwnedPr", () => { test("not owned when there are no assignees", () => { expect(isOwnedPr({ assignees: [], userName: "centaur-bot" })).toBe(false); }); + + test("owned when an App-authored PR uses the separate bot actor login", () => { + expect( + isOwnedPr({ + assignees: [], + author: "centaur-bot[bot]", + botActorLogin: "centaur-bot[bot]", + userName: "centaur-bot", + }), + ).toBe(true); + }); + + test("does not confuse an App mention slug with its actor login", () => { + expect( + isOwnedPr({ + assignees: [], + author: "centaur-bot", + botActorLogin: "centaur-bot[bot]", + userName: "centaur-bot", + }), + ).toBe(false); + }); + + test("does not treat an App mention slug as an assignable account", () => { + expect( + isOwnedPr({ + assignees: ["centaur-bot"], + botActorLogin: "centaur-bot[bot]", + userName: "centaur-bot", + }), + ).toBe(false); + }); + + test("owned when the configured handoff label is present", () => { + expect( + isOwnedPr({ + assignees: [], + labels: ["bug", "Centaur-Managed"], + ownershipLabel: "centaur-managed", + userName: "centaur-bot", + }), + ).toBe(true); + }); }); describe("decideMerge", () => { diff --git a/services/githubbot/test/review.test.ts b/services/githubbot/test/review.test.ts index 069040754..4486dc483 100644 --- a/services/githubbot/test/review.test.ts +++ b/services/githubbot/test/review.test.ts @@ -80,6 +80,15 @@ describe("handleReviewRequest", () => { expect(result).not.toBeNull(); }); + test("recognizes a separately configured App actor login", () => { + const result = handleReviewRequest(reviewRequestedBody("review-bot[bot]"), { + ...input, + botActorLogin: "review-bot[bot]", + state: stubState(), + }); + expect(result).not.toBeNull(); + }); + test("de-duplicates a redelivered review request", async () => { const state = stubState(); // First delivery claims the dedup key; second (same id) finds it taken. From 62402a1b5f515d8284a560573c65ef0dc8bfe4ac Mon Sep 17 00:00:00 2001 From: Michael Wu Date: Sun, 30 Aug 2026 20:04:14 +0900 Subject: [PATCH 5/6] fix(chart): bootstrap GitHub App ingress secrets --- contrib/scripts/bootstrap-k8s-secrets.sh | 26 +++++++++++++++--------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/contrib/scripts/bootstrap-k8s-secrets.sh b/contrib/scripts/bootstrap-k8s-secrets.sh index b5f0a22ca..d97c53edc 100755 --- a/contrib/scripts/bootstrap-k8s-secrets.sh +++ b/contrib/scripts/bootstrap-k8s-secrets.sh @@ -39,11 +39,14 @@ Optional Linear bot bootstrap (consumed when linearbot.enabled=true): Optional GitHub ingress bootstrap (consumed when githubbot.enabled=true): GITHUBBOT_TOKEN personal access token for the bot's GitHub - teammate account; required together with the - webhook secret (partial config fails fast). Kept + teammate account in PAT mode; when set, the + webhook secret is also required. Omit for App + mode. Kept distinct from GITHUB_TOKEN (the repo-cache / sandbox tool token) so the bot acts as its own user. - GITHUBBOT_WEBHOOK_SECRET signing secret from the GitHub repo/org webhook + GITHUBBOT_WEBHOOK_SECRET signing secret from the GitHub repo/org webhook; + seeds the shared ingress keys for either PAT or + App authentication GITHUBBOT_API_KEY bearer the bot sends to api-rs; auto-generated when absent @@ -153,11 +156,10 @@ if [[ -n "${LINEAR_ACCESS_TOKEN:-}" || -n "${LINEARBOT_WEBHOOK_SECRET:-}" ]]; th require_env LINEARBOT_WEBHOOK_SECRET fi -# GitHub bot config is optional but must be complete: a PAT without the webhook -# secret (or vice versa) deploys a githubbot that boots and then rejects every -# delivery, which reads as silence. -if [[ -n "${GITHUBBOT_TOKEN:-}" || -n "${GITHUBBOT_WEBHOOK_SECRET:-}" ]]; then - require_env GITHUBBOT_TOKEN +# GitHub ingress shared secrets support PAT and App authentication. A PAT or an +# explicitly supplied api-rs bearer without a webhook secret is incomplete; +# the webhook secret alone is valid App-mode bootstrap input. +if [[ -n "${GITHUBBOT_TOKEN:-}" || -n "${GITHUBBOT_API_KEY:-}" ]]; then require_env GITHUBBOT_WEBHOOK_SECRET fi @@ -286,10 +288,12 @@ if secret_exists centaur-infra-env; then patch_data+=("\"LINEARBOT_API_KEY\":\"$(rand_hex | base64 | tr -d '\n')\"") fi fi - # GitHub bot credentials. The PAT + webhook secret are set whenever present so - # they can be rotated; the api-rs bearer is generated once and kept stable. + # GitHub ingress shared secrets are independent of controller authentication: + # App mode has no PAT. Rotate supplied values; generate the api-rs bearer once. if [[ -n "${GITHUBBOT_TOKEN:-}" ]]; then patch_data+=("\"GITHUBBOT_TOKEN\":\"$(printf '%s' "$GITHUBBOT_TOKEN" | base64 | tr -d '\n')\"") + fi + if [[ -n "${GITHUBBOT_WEBHOOK_SECRET:-}" ]]; then patch_data+=("\"GITHUBBOT_WEBHOOK_SECRET\":\"$(printf '%s' "$GITHUBBOT_WEBHOOK_SECRET" | base64 | tr -d '\n')\"") if [[ -n "${GITHUBBOT_API_KEY:-}" ]]; then patch_data+=("\"GITHUBBOT_API_KEY\":\"$(printf '%s' "$GITHUBBOT_API_KEY" | base64 | tr -d '\n')\"") @@ -366,6 +370,8 @@ else fi if [[ -n "${GITHUBBOT_TOKEN:-}" ]]; then secret_args+=(--from-literal=GITHUBBOT_TOKEN="$GITHUBBOT_TOKEN") + fi + if [[ -n "${GITHUBBOT_WEBHOOK_SECRET:-}" ]]; then secret_args+=(--from-literal=GITHUBBOT_WEBHOOK_SECRET="$GITHUBBOT_WEBHOOK_SECRET") secret_args+=(--from-literal=GITHUBBOT_API_KEY="${GITHUBBOT_API_KEY:-$(rand_hex)}") fi From d9874dcf41830911e054edbbbc76894cbbaabb1c Mon Sep 17 00:00:00 2001 From: Michael Wu Date: Sun, 30 Aug 2026 20:23:38 +0900 Subject: [PATCH 6/6] fix(githubbot): tighten App ownership events --- services/githubbot/README.md | 6 ++-- services/githubbot/src/index.ts | 20 +++++++++++-- services/githubbot/src/pr-manager.ts | 27 ++++++++++++++++- services/githubbot/src/review.ts | 8 +++-- services/githubbot/src/server.ts | 19 +++++++----- services/githubbot/test/body-mention.test.ts | 2 +- services/githubbot/test/github-auth.test.ts | 18 ++++++++++++ services/githubbot/test/pr-manager.test.ts | 31 ++++++++++++++++++++ services/githubbot/test/review.test.ts | 4 +-- 9 files changed, 115 insertions(+), 20 deletions(-) diff --git a/services/githubbot/README.md b/services/githubbot/README.md index 2e34400cf..20649b860 100644 --- a/services/githubbot/README.md +++ b/services/githubbot/README.md @@ -37,9 +37,9 @@ Apps cannot be assignees or requested reviewers. thread's session as append-only context — no execution, no reply — so a follow-up like "actually, hold off" is seen by the next turn. The bot's own comments are skipped (loop guard) and inactive threads are ignored. -- **Requesting the bot's review on a PR** (`pull_request` / `review_requested` targeting the bot - account — or a **team the bot belongs to**, whose membership is checked and briefly cached) → a - review turn runs on a **dedicated, isolated session thread** +- **Requesting the PAT teammate bot's review on a PR** (`pull_request` / `review_requested` + targeting the bot account — or a **team the bot belongs to**, whose membership is checked and + briefly cached) → a review turn runs on a **dedicated, isolated session thread** (`github-review:{owner}/{repo}:{prNumber}`) — kept separate from the PR conversation so reviews never share a sandbox with chit-chat, but persistent per PR so a re-request builds on the prior review. The chat adapter only surfaces comment threads, so this lifecycle event is handled diff --git a/services/githubbot/src/index.ts b/services/githubbot/src/index.ts index 479dadce5..f6b337be8 100644 --- a/services/githubbot/src/index.ts +++ b/services/githubbot/src/index.ts @@ -75,10 +75,24 @@ export function resolveBotActorLogin( options: Pick, userName: string, ): string { - return ( + if ( + options.githubAppClientId && + userName.trim().toLowerCase().endsWith("[bot]") + ) { + throw new Error( + "GitHub App userName must be the mention slug without the [bot] suffix", + ); + } + const actorLogin = options.botActorLogin?.trim() || - (options.githubAppClientId ? `${userName}[bot]` : userName) - ); + (options.githubAppClientId ? `${userName}[bot]` : userName); + if ( + options.githubAppClientId && + !actorLogin.toLowerCase().endsWith("[bot]") + ) { + throw new Error("GitHub App botActorLogin must end in [bot]"); + } + return actorLogin; } export function createGithubbot(options: GithubbotOptions): Githubbot { diff --git a/services/githubbot/src/pr-manager.ts b/services/githubbot/src/pr-manager.ts index 924ef54de..849bd57ce 100644 --- a/services/githubbot/src/pr-manager.ts +++ b/services/githubbot/src/pr-manager.ts @@ -96,6 +96,22 @@ export function isOwnedPr(input: { ); } +/** An assignment is a handoff only when GitHub assigned the PAT bot itself. */ +export function isBotAssignmentHandoff(input: { + action?: string; + assignee?: string; + botActorLogin?: string; + userName: string; +}): boolean { + const mentionLogin = input.userName.toLowerCase(); + const actorLogin = (input.botActorLogin ?? input.userName).toLowerCase(); + return ( + input.action === "assigned" && + actorLogin === mentionLogin && + input.assignee?.toLowerCase() === mentionLogin + ); +} + export type MergeDecision = | "merge" | "resolve_conflict" @@ -611,6 +627,10 @@ export async function handlePullRequestEvent( const labelNode = payload.label; const label = isRecord(labelNode) ? stringValue(labelNode.name) : undefined; + const assigneeNode = payload.assignee; + const assignee = isRecord(assigneeNode) + ? stringValue(assigneeNode.login) + : undefined; const resetLabel = ctx.options.reviewResetLabel ?? DEFAULT_REVIEW_RESET_LABEL; const ownershipLabel = ctx.options.ownershipLabel ?? DEFAULT_OWNERSHIP_LABEL; @@ -672,7 +692,12 @@ export async function handlePullRequestEvent( // handoff. Evaluate CI now, forcing past human-commit back-off, so a PR that // was already red or green does not wait for another lifecycle event. if ( - action === "assigned" || + isBotAssignmentHandoff({ + action, + assignee, + botActorLogin: ctx.botActorLogin, + userName: ctx.userName, + }) || (action === "labeled" && label?.toLowerCase() === ownershipLabel.toLowerCase()) ) { diff --git a/services/githubbot/src/review.ts b/services/githubbot/src/review.ts index b42b1a6c1..350a430b0 100644 --- a/services/githubbot/src/review.ts +++ b/services/githubbot/src/review.ts @@ -91,11 +91,13 @@ export function handleReviewRequest( // belongs to a requested team (resolved asynchronously below). A request that // names a different individual reviewer and no team is not ours. const reviewer = stringValue(payload.requested_reviewer?.login); + const directReviewSupported = + (input.botActorLogin ?? input.botUserName).toLowerCase() === + input.botUserName.toLowerCase(); const directMatch = + directReviewSupported && !!reviewer && - [input.botUserName, input.botActorLogin ?? input.botUserName].some( - (login) => reviewer.toLowerCase() === login.toLowerCase(), - ); + reviewer.toLowerCase() === input.botUserName.toLowerCase(); const teamSlug = stringValue(payload.requested_team?.slug); if (!directMatch && !teamSlug) return null; diff --git a/services/githubbot/src/server.ts b/services/githubbot/src/server.ts index 711133b6c..f527aacf6 100644 --- a/services/githubbot/src/server.ts +++ b/services/githubbot/src/server.ts @@ -1,7 +1,11 @@ import { readFileSync } from "node:fs"; import { requireRepositoryAllowlist } from "./authorization"; import { drainBackgroundWork } from "./context"; -import { createGithubbot, type GithubbotOptions } from "./index"; +import { + createGithubbot, + resolveBotActorLogin, + type GithubbotOptions, +} from "./index"; import { DEFAULT_OWNERSHIP_LABEL } from "./pr-manager"; import { DEFAULT_REVIEW_RESET_LABEL } from "./review-budget"; import { positiveIntegerValue } from "./utils"; @@ -53,12 +57,13 @@ if (!userName) { "GITHUB_BOT_USERNAME is required (App mention slug or PAT account login)", ); } -const botActorLogin = - optionalEnv("GITHUB_BOT_ACTOR_LOGIN") ?? - (githubAppClientId ? `${userName}[bot]` : userName); -if (githubAppClientId && !botActorLogin.toLowerCase().endsWith("[bot]")) { - throw new Error("GITHUB_BOT_ACTOR_LOGIN must end in [bot] for GitHub App auth"); -} +const botActorLogin = resolveBotActorLogin( + { + botActorLogin: optionalEnv("GITHUB_BOT_ACTOR_LOGIN"), + githubAppClientId, + }, + userName, +); const ownershipLabel = optionalEnv("GITHUBBOT_OWNERSHIP_LABEL") ?? DEFAULT_OWNERSHIP_LABEL; const reviewResetLabel = diff --git a/services/githubbot/test/body-mention.test.ts b/services/githubbot/test/body-mention.test.ts index 89ebba098..1451691be 100644 --- a/services/githubbot/test/body-mention.test.ts +++ b/services/githubbot/test/body-mention.test.ts @@ -129,7 +129,7 @@ describe("handleBodyMention", () => { handleBodyMention( makeCtx({ reactions: 0, comments: 0 }, "centaur-bot[bot]"), "pull_request", - openedPr("@centaur-bot do it", "NONE", "centaur-bot[bot]"), + openedPr("@centaur-bot do it", "MEMBER", "centaur-bot[bot]"), ), ).toBeNull(); }); diff --git a/services/githubbot/test/github-auth.test.ts b/services/githubbot/test/github-auth.test.ts index 8da52d49f..45ec3b93d 100644 --- a/services/githubbot/test/github-auth.test.ts +++ b/services/githubbot/test/github-auth.test.ts @@ -99,4 +99,22 @@ describe("GitHub bot identity", () => { ), ).toBe("custom-app[bot]"); }); + + test("rejects a suffixed App mention slug or unsuffixed actor login", () => { + expect(() => + resolveBotActorLogin( + { githubAppClientId: "Iv1.example" }, + "centaur-bot[bot]", + ), + ).toThrow("mention slug without the [bot] suffix"); + expect(() => + resolveBotActorLogin( + { + botActorLogin: "centaur-bot", + githubAppClientId: "Iv1.example", + }, + "centaur-bot", + ), + ).toThrow("botActorLogin must end in [bot]"); + }); }); diff --git a/services/githubbot/test/pr-manager.test.ts b/services/githubbot/test/pr-manager.test.ts index 81014598f..4ec48439b 100644 --- a/services/githubbot/test/pr-manager.test.ts +++ b/services/githubbot/test/pr-manager.test.ts @@ -5,6 +5,7 @@ import { handleCiEvent, handlePullRequestEvent, handleReviewEvent, + isBotAssignmentHandoff, isOwnedPr, type PrManagerContext, } from "../src/pr-manager"; @@ -171,6 +172,36 @@ describe("isOwnedPr", () => { }); }); +describe("isBotAssignmentHandoff", () => { + test("accepts only an assignment of the PAT bot itself", () => { + expect( + isBotAssignmentHandoff({ + action: "assigned", + assignee: "Centaur-Bot", + userName: "centaur-bot", + }), + ).toBe(true); + expect( + isBotAssignmentHandoff({ + action: "assigned", + assignee: "alice", + userName: "centaur-bot", + }), + ).toBe(false); + }); + + test("does not treat assignments as App-mode handoffs", () => { + expect( + isBotAssignmentHandoff({ + action: "assigned", + assignee: "centaur-bot", + botActorLogin: "centaur-bot[bot]", + userName: "centaur-bot", + }), + ).toBe(false); + }); +}); + describe("decideMerge", () => { const base = { autoMerge: true, diff --git a/services/githubbot/test/review.test.ts b/services/githubbot/test/review.test.ts index 4486dc483..26fc1042c 100644 --- a/services/githubbot/test/review.test.ts +++ b/services/githubbot/test/review.test.ts @@ -80,13 +80,13 @@ describe("handleReviewRequest", () => { expect(result).not.toBeNull(); }); - test("recognizes a separately configured App actor login", () => { + test("does not treat an App actor as a requestable reviewer", () => { const result = handleReviewRequest(reviewRequestedBody("review-bot[bot]"), { ...input, botActorLogin: "review-bot[bot]", state: stubState(), }); - expect(result).not.toBeNull(); + expect(result).toBeNull(); }); test("de-duplicates a redelivered review request", async () => {