-
Notifications
You must be signed in to change notification settings - Fork 1
feat: integrate Probot middleware for GitHub webhooks #468
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughFirebase Functions の HTTP ハンドラを Probot ミドルウェアで初期化・公開するように変更し、GITHUB_APP_ID/GITHUB_PRIVATE_KEY/GITHUB_WEBHOOK_SECRET を secrets で参照。github-apps 側はデフォルト export を廃止して named export の appFn に変更し、関連ドキュメントや再利用 CI ワークフローを削除・再構成。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub → Webhook
participant FF as Firebase Function (githubWebhook)
participant S as Secrets (defineSecret)
participant PB as Probot Middleware
participant APP as Probot App (appFn)
participant API as GitHub API
GH->>FF: POST / (webhook)
FF->>S: GITHUB_APP_ID / PRIVATE_KEY / WEBHOOK_SECRET を取得
FF->>PB: createProbot(overrides) & createNodeMiddleware(appFn)
FF->>PB: delegate HTTP request
PB->>APP: deliver webhook event (e.g., issues.opened)
APP->>API: issues.createComment("Hello, World!")
API-->>APP: 201 Created
PB-->>FF: ミドルウェア完了
FF-->>GH: 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openci-runner/firebase/functions/src/index.ts (1)
18-19
: 未使用のコードを削除してください。
serviceAccountJson
が解析されていますが、実際には使用されていません。Line 19 の単独の変数参照は何も実行しません。このコードが一時的なプレースホルダーの場合は削除するか、実装を完成させてください。
以下の diff を適用して未使用のコードを削除してください:
- const serviceAccountJson = JSON.parse(firebaseServiceAccount.value()); - serviceAccountJson; logger.info("Hello logs!", { structuredData: true }); res.send("Hello from Firebase!");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
openci-runner/firebase/functions/src/index.ts
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
openci-runner/firebase/functions/src/index.ts (2)
openci-runner/github-apps/src/index.ts (1)
app
(3-11)openci-runner/github-apps/test/index.test.ts (4)
probot
(28-76)body
(60-63)nock
(32-45)mock
(47-70)
🪛 GitHub Actions: Deploy GitHub Apps
openci-runner/firebase/functions/src/index.ts
[error] 1-1: Command failed: npm test. Vitest failed with error: Failed to load url probot (resolved id: probot) in firebase/functions/src/index.ts. Does the file exist?
🔇 Additional comments (1)
openci-runner/firebase/functions/src/index.ts (1)
10-23
:probotApp
とgithubWebhook
のデプロイ意図を確認してください。
- 両者は Firebase Functions として独立してデプロイされます。
githubWebhook
がプレースホルダーのまま残っていますが、不要であれば削除を検討してください。
- Added Probot as a dependency in Firebase functions. - Updated Firebase functions to use Probot middleware for GitHub app integration. - Defined additional GitHub app secrets in Firebase functions. - Updated TypeScript configuration to support ES modules and added skipLibCheck. - Refactored GitHub app implementation to export as a function. - Updated tests to reflect changes in the app structure and dependencies. - Cleaned up package-lock.json by removing unnecessary nested dependencies. - Updated dependencies in GitHub apps to latest versions for better compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (1)
openci-runner/firebase/functions/src/index.ts (1)
14-23
:probotApp
をonRequest
でラップしてください
probotApp
が生のミドルウェアとしてエクスポートされていますが、Firebase Functions の HTTP 関数として適切に動作させるには、onRequest
でラップする必要があります。また、必要なシークレットをsecrets
オプションで指定してください。次の diff を適用してください:
-export const probotApp = createNodeMiddleware(appFn, { - probot: createProbot({ - overrides: { - appId: 123, - privateKey: githubPrivateKey.value(), - secret: githubWebhookSecret.value(), - }, - }), - webhooksPath: "/", -}); +export const probotApp = onRequest( + { + secrets: [githubAppId, githubPrivateKey, githubWebhookSecret], + }, + createNodeMiddleware(appFn, { + probot: createProbot({ + overrides: { + appId: Number.parseInt(githubAppId.value(), 10), + privateKey: githubPrivateKey.value(), + secret: githubWebhookSecret.value(), + }, + }), + webhooksPath: "/", + }), +);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
openci-runner/firebase/functions/package-lock.json
is excluded by!**/package-lock.json
openci-runner/github-apps/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (7)
openci-runner/firebase/functions/package.json
(2 hunks)openci-runner/firebase/functions/src/index.ts
(1 hunks)openci-runner/firebase/functions/test/hello-world.test.ts
(1 hunks)openci-runner/firebase/functions/tsconfig.json
(1 hunks)openci-runner/github-apps/src/index.ts
(1 hunks)openci-runner/github-apps/test/index.test.ts
(2 hunks)openci-runner/github-apps/tsconfig.json
(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
openci-runner/firebase/functions/src/index.ts (1)
openci-runner/github-apps/src/index.ts (1)
appFn
(3-11)
openci-runner/github-apps/test/index.test.ts (1)
openci-runner/github-apps/src/index.ts (1)
appFn
(3-11)
🪛 GitHub Actions: Deploy GitHub Apps
openci-runner/firebase/functions/src/index.ts
[error] 1-1: Failed to load url ../../../github-apps/lib/index.js (resolved id: ../../../github-apps/lib/index.js) in /home/runner/work/openci/openci/openci-runner/firebase/functions/src/index.ts. Does the file exist? (Command: npm test)
🔇 Additional comments (8)
openci-runner/firebase/functions/tsconfig.json (1)
12-13
: TODO コメントを Issue として追跡することを推奨
skipLibCheck: true
は一時的な回避策として追加されていますが、型チェックを抑制するため、将来的には削除すべきです。この TODO を Issue として追跡し、根本的な型の問題を解決することをお勧めします。openci-runner/firebase/functions/package.json (2)
19-20
: LGTM!Probot 依存関係の追加は適切です。バージョン
^14.0.2
は最新の安定版と思われます。
29-30
: LGTM!
"type": "module"
の追加により ESM が有効になり、Probot の統合に必要な設定です。openci-runner/firebase/functions/test/hello-world.test.ts (1)
8-8
: LGTM!モックシークレットに
appId: 123
が追加され、新しいシークレット構造と整合性が取れています。openci-runner/github-apps/src/index.ts (1)
1-3
: LGTM! ただし破壊的変更に注意デフォルトエクスポートから名前付きエクスポート
appFn
への変更は、公開 API の破壊的変更です。ただし、テストおよび Firebase Functions が既に更新されているため、整合性は保たれています。openci-runner/github-apps/test/index.test.ts (2)
13-13
: LGTM!名前付きエクスポート
appFn
のインポートに更新され、src/index.ts
の API 変更と整合性が取れています。
44-44
: LGTM!
probot.load(appFn)
への更新により、新しい名前付きエクスポートが正しく使用されています。openci-runner/firebase/functions/src/index.ts (1)
25-40
:githubWebhook
ハンドラの目的を明確にしてください
probotApp
が追加された後も既存のgithubWebhook
ハンドラが残っていますが、現在は "Hello from Firebase!" を返すだけです。このハンドラの目的が不明確です。
probotApp
が GitHub Webhook を処理する場合、このgithubWebhook
ハンドラは不要かもしれません- 両方が必要な場合は、それぞれの役割を明確にするコメントを追加してください
export const probotApp = createNodeMiddleware(appFn, { | ||
probot: createProbot({ | ||
overrides: { | ||
appId: 123, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ハードコードされた appId の代わりにシークレットを使用してください
appId: 123
がハードコードされていますが、Line 10 で定義された githubAppId
シークレットを使用すべきです。
次の diff を適用してください:
export const probotApp = createNodeMiddleware(appFn, {
probot: createProbot({
overrides: {
- appId: 123,
+ appId: Number.parseInt(githubAppId.value(), 10),
privateKey: githubPrivateKey.value(),
secret: githubWebhookSecret.value(),
},
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
appId: 123, | |
export const probotApp = createNodeMiddleware(appFn, { | |
probot: createProbot({ | |
overrides: { | |
appId: Number.parseInt(githubAppId.value(), 10), | |
privateKey: githubPrivateKey.value(), | |
secret: githubWebhookSecret.value(), | |
}, | |
}), | |
}); |
🤖 Prompt for AI Agents
In openci-runner/firebase/functions/src/index.ts around line 17, replace the
hardcoded appId: 123 with the githubAppId secret defined at line 10; set appId
to the githubAppId value (convert to a number with parseInt if the API expects a
numeric id) instead of the literal 123 so the secret is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/github-apps-cd.yml
(1 hunks)openci-runner/github-apps/tsconfig.json
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- openci-runner/github-apps/tsconfig.json
.github/workflows/github-apps-cd.yml
Outdated
- name: Install dependencies for GitHub apps | ||
working-directory: openci-runner/github-apps | ||
run: | | ||
npm ci | ||
npm run build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
本番デプロイにも同じビルド手順を追加してください。
DEV 用ジョブにだけ GitHub Apps の npm ci && npm run build
を追加すると、本番デプロイ (deploy-firebase-functions-production
) では未ビルドのまま Firebase Functions がデプロイされるリスクがあります。Probot 連携が Functions から openci-runner/github-apps
のビルド成果物に依存している場合、push 時の本番デプロイが失敗するか旧成果物のままになります。
以下のように production ジョブにも同じステップを追加することを提案します。
- uses: google-github-actions/auth@v2
with:
credentials_json: ${{ secrets.FIREBASE_SA_KEY }}
+ - name: Install dependencies for GitHub apps
+ working-directory: openci-runner/github-apps
+ run: |
+ npm ci
+ npm run build
+
- name: Install dependencies
working-directory: openci-runner/firebase/functions
run: npm ci
🤖 Prompt for AI Agents
In .github/workflows/github-apps-cd.yml around lines 34-38, the CI currently
runs npm ci && npm run build only in the DEV job which means the production
deploy job may push unbuilt Firebase Functions; add the same two-step block (npm
ci and npm run build with working-directory: openci-runner/github-apps) into the
production job (deploy-firebase-functions-production) before the deploy step so
the GitHub Apps artifacts are built for production as well and ensure it runs in
the same context/order as the DEV job.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
openci-runner/firebase/functions/src/index.ts (1)
41-41
:appId
を数値に変換してください。GitHub App の
appId
は通常数値型を期待しますが、githubAppId.value()
は文字列を返します。型エラーを防ぐため、Number.parseInt()
で数値に変換してください。次の diff を適用してください:
- appId: githubAppId.value(), + appId: Number.parseInt(githubAppId.value(), 10),
🧹 Nitpick comments (1)
openci-runner/firebase/functions/src/index.ts (1)
35-36
: 未使用の変数を削除してください。
serviceAccountJson
は解析されていますが使用されていません。この変数が不要な場合は削除してください。次の diff を適用してください:
- const serviceAccountJson = JSON.parse(firebaseServiceAccount.value()); - serviceAccountJson;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
openci-runner/firebase/functions/src/index.ts
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
openci-runner/firebase/functions/src/index.ts (2)
openci-runner/github-apps/src/index.ts (1)
appFn
(3-11)openci-runner/firebase/functions/test/hello-world.test.ts (1)
mod
(15-18)
…deployment workflow feat: add "FUNCS" to workspace extensions for improved development experience
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (1)
openci-runner/firebase/functions/src/index.ts (1)
36-48
: GitHub Webhook が処理されていません。関数名は
githubWebhook
ですが、実際には GitHub webhook を処理せず、常に "Hello from Firebase!" を返します。Lines 36-45 の Probot ミドルウェア作成コードもコメントアウトされています。これは過去のレビューコメントで指摘された問題と同じです。Probot 統合を完了させるには、以下のいずれかの対応が必要です:
- Lines 12-21 のコメントアウトされた
probotApp
パターンを使用する- Lines 36-45 のミドルウェアコードを有効化し、
req
とres
に対して呼び出す現在の実装では、webhook イベントが適切に処理されません。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
openci-runner/firebase/functions/src/index.ts
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
openci-runner/firebase/functions/src/index.ts (1)
openci-runner/firebase/functions/test/hello-world.test.ts (1)
mod
(15-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy-firebase-functions-dev
🔇 Additional comments (1)
openci-runner/firebase/functions/src/index.ts (1)
4-4
: LGTM!logger のインポートは適切で、Line 46 で使用されています。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
openci-runner/firebase/functions/src/index.ts
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
openci-runner/firebase/functions/src/index.ts (2)
openci-runner/firebase/functions/test/hello-world.test.ts (1)
mod
(15-18)openci-runner/github-apps/src/index.ts (1)
app
(3-11)
🔇 Additional comments (1)
openci-runner/firebase/functions/src/index.ts (1)
4-5
: ロガーのインポートが適切に追加されています。
logger
が正しくインポートされ、Line 47 で使用されています。
… Firebase function
…and remove unused variable in Firebase function
- Removed the existing GitHub app implementation from openci-runner/github-apps. - Added new implementation in openci-runner/firebase/functions/probot. - Updated test fixtures and test cases to reflect the new structure. - Deleted obsolete files related to the previous implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/github-apps-cd.yml (1)
27-37
: 本番・プレビューともに GitHub Apps のビルド工程が不足していますFunctions から利用する
openci-runner/github-apps
の成果物を生成していないため、未ビルド資産のままデプロイされるリスクがあります。DEV 側だけでなく本番ジョブでも同じビルド手順を追加してください。- uses: google-github-actions/auth@v2 with: credentials_json: ${{ secrets.GHA_SERVICE_ACCOUNT_PREVIEW_FUNCS }} + - name: Build GitHub Apps assets + working-directory: openci-runner/github-apps + run: | + npm ci + npm run build + - name: Install dependencies working-directory: openci-runner/firebase/functions run: npm ci @@ - uses: google-github-actions/auth@v2 with: credentials_json: ${{ secrets.FIREBASE_SA_KEY }} + - name: Build GitHub Apps assets + working-directory: openci-runner/github-apps + run: | + npm ci + npm run build + - name: Install dependencies working-directory: openci-runner/firebase/functions run: npm ciAlso applies to: 59-69
openci-runner/firebase/functions/src/index.ts (1)
39-50
: GitHub Webhook が処理されていません
createNodeMiddleware
で生成したミドルウェアをreq
/res
に適用せず破棄しているため、GitHub からのリクエストは Probot に届かず固定文言だけ返ってしまいます。これでは Webhook が一切動作しません。ミドルウェアを返す形に修正し、ハードコードされたレスポンスは削除してください。- createNodeMiddleware(appFn, { - probot: createProbot({ - overrides: { - appId: githubAppId.value(), - privateKey: githubPrivateKey.value(), - secret: githubWebhookSecret.value(), - }, - }), - webhooksPath: "/", -}); - logger.info("Hello logs!", { structuredData: true }); - res.send("Hello from Firebase!"); + const middleware = createNodeMiddleware(appFn, { + probot: createProbot({ + overrides: { + appId: githubAppId.value(), + privateKey: githubPrivateKey.value(), + secret: githubWebhookSecret.value(), + }, + }), + webhooksPath: "/", + }); + return middleware(req, res);
🧹 Nitpick comments (1)
openci-runner/firebase/functions/package.json (1)
18-22
: nock は開発依存に移動しましょう
nock
はテスト専用ライブラリなので、本番デプロイ対象の依存関係に含めると不要にバンドルが膨らみます。devDependencies
へ移した方が維持コストが下がります。"dependencies": { "firebase-admin": "^12.6.0", "firebase-functions": "^6.0.1", - "nock": "^14.0.10", "probot": "^14.0.2" }, "devDependencies": { + "nock": "^14.0.10", "@types/supertest": "^6.0.3",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
openci-runner/firebase/functions/package-lock.json
is excluded by!**/package-lock.json
openci-runner/firebase/functions/test/fixtures/mock-cert.pem
is excluded by!**/*.pem
openci-runner/github-apps/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (20)
.github/workflows/github-apps-cd.yml
(2 hunks).github/workflows/reusable-firebase-functions-ci.yml
(0 hunks).github/workflows/reusable-github-apps-ci.yml
(0 hunks)openci-runner/firebase/functions/package.json
(2 hunks)openci-runner/firebase/functions/probot/index.ts
(1 hunks)openci-runner/firebase/functions/src/index.ts
(1 hunks)openci-runner/firebase/functions/test/index.test.ts
(2 hunks)openci-runner/firebase/functions/tsconfig.json
(1 hunks)openci-runner/github-apps/.dockerignore
(0 hunks)openci-runner/github-apps/.env.example
(0 hunks)openci-runner/github-apps/.gitignore
(0 hunks)openci-runner/github-apps/CODE_OF_CONDUCT.md
(0 hunks)openci-runner/github-apps/CONTRIBUTING.md
(0 hunks)openci-runner/github-apps/Dockerfile
(0 hunks)openci-runner/github-apps/LICENSE
(0 hunks)openci-runner/github-apps/README.md
(0 hunks)openci-runner/github-apps/app.yml
(0 hunks)openci-runner/github-apps/package.json
(0 hunks)openci-runner/github-apps/tsconfig.json
(0 hunks)openci-runner/github-apps/vitest.config.ts
(0 hunks)
💤 Files with no reviewable changes (14)
- openci-runner/github-apps/CODE_OF_CONDUCT.md
- openci-runner/github-apps/package.json
- openci-runner/github-apps/README.md
- openci-runner/github-apps/.env.example
- openci-runner/github-apps/.gitignore
- openci-runner/github-apps/CONTRIBUTING.md
- openci-runner/github-apps/vitest.config.ts
- openci-runner/github-apps/app.yml
- openci-runner/github-apps/LICENSE
- .github/workflows/reusable-github-apps-ci.yml
- openci-runner/github-apps/.dockerignore
- openci-runner/github-apps/Dockerfile
- .github/workflows/reusable-firebase-functions-ci.yml
- openci-runner/github-apps/tsconfig.json
🧰 Additional context used
🧬 Code graph analysis (6)
openci-runner/firebase/functions/probot/index.ts (2)
openci-runner/github-apps/src/index.ts (2)
app
(3-11)context
(6-10)openci-runner/github-apps/test/index.test.ts (4)
probot
(28-76)mock
(47-70)body
(60-63)nock
(32-45)
openci-runner/firebase/functions/tsconfig.json (1)
openci-runner/firebase/functions/test/hello-world.test.ts (5)
mod
(15-18)it
(20-29)req
(23-23)app
(21-28)name
(6-9)
.github/workflows/github-apps-cd.yml (1)
openci-runner/firebase/functions/test/hello-world.test.ts (4)
mod
(15-18)name
(6-9)it
(20-29)app
(21-28)
openci-runner/firebase/functions/src/index.ts (2)
openci-runner/firebase/functions/probot/index.ts (1)
appFn
(3-11)openci-runner/firebase/functions/test/hello-world.test.ts (1)
mod
(15-18)
openci-runner/firebase/functions/package.json (2)
openci-runner/firebase/functions/test/hello-world.test.ts (5)
mod
(15-18)it
(20-29)req
(23-23)app
(21-28)name
(6-9)openci-runner/github-apps/test/index.test.ts (4)
probot
(28-76)body
(60-63)nock
(32-45)mock
(47-70)
openci-runner/firebase/functions/test/index.test.ts (2)
openci-runner/firebase/functions/probot/index.ts (1)
appFn
(3-11)openci-runner/github-apps/src/index.ts (1)
app
(3-11)
🪛 GitHub Actions: Deploy GitHub Apps
openci-runner/firebase/functions/test/index.test.ts
[error] 1-1: Failed to load url '../lib/probot/index.js' (resolved id: ../lib/probot/index.js) in /home/runner/work/openci/openci/openci-runner/firebase/functions/test/index.test.ts. Does the file exist? (npm test)
import { appFn } from "../lib/probot/index.js"; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ビルド済み lib への依存でテストが即失敗しています
npm test
実行時には tsc
が走らないため、../lib/probot/index.js
が存在せずパイプラインが落ちています(ログ参照)。テストではソースの TypeScript を直接解決するようにして、Vitest に任せた方が安全です。以下のように lib
プレフィックスを外せば、tsconfig で probot
ディレクトリを含めているので解決できます。
-import { appFn } from "../lib/probot/index.js";
+import { appFn } from "../probot/index.js";
同様に probot.load
側もそのまま動作します。もしビルド済みアーティファクトを参照したい場合は、test
スクリプトを npm run build && vitest run
に変えるなど事前ビルドを必須化してください。
Also applies to: 44-45
🤖 Prompt for AI Agents
In openci-runner/firebase/functions/test/index.test.ts around lines 13-14 (and
similarly lines 44-45), the tests import the built artifact
"../lib/probot/index.js" which fails when tsc hasn't run; remove the "lib/"
prefix so the test imports the source module (e.g., change
"../lib/probot/index.js" to "../probot/index" so Vitest/ts-node resolves the
TypeScript source via tsconfig), and if you instead want to keep importing built
files, update the test script to run a pre-build (e.g., "npm run build && vitest
run").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
openci-runner/firebase/functions/src/index.ts (2)
35-42
: Webhook が Probot ミドルウェアで処理されていません。
githubWebhook
関数は常に "Hello from Firebase!" を返すだけで、GitHub webhook を実際に処理していません。PR の目的は「Probot ミドルウェアを統合する」ことですが、作成されたprobotApp
ミドルウェアが使用されていません。以下のいずれかの対応が必要です:
オプション 1 (推奨):
probotApp
を webhook エンドポイントとしてデプロイし、githubWebhook
を削除する:-export const githubWebhook = onRequest( - { - secrets: [ - firebaseServiceAccount, - githubAppId, - githubPrivateKey, - githubWebhookSecret, - ], - }, - (req, res) => { - const serviceAccountJson = JSON.parse(firebaseServiceAccount.value()); - serviceAccountJson; - probotApp; - - logger.info("Hello logs!", { structuredData: true }); - res.send("Hello from Firebase!"); - }, -);オプション 2:
githubWebhook
内でprobotApp
ミドルウェアを呼び出す:+export const probotMiddleware = createNodeMiddleware(appFn, { -export const probotApp = createNodeMiddleware(appFn, { probot: createProbot({ overrides: { - appId: githubAppId.value(), + appId: Number.parseInt(githubAppId.value(), 10), privateKey: githubPrivateKey.value(), secret: githubWebhookSecret.value(), }, }), webhooksPath: "/", }); export const githubWebhook = onRequest( { secrets: [ firebaseServiceAccount, githubAppId, githubPrivateKey, githubWebhookSecret, ], }, (req, res) => { - const serviceAccountJson = JSON.parse(firebaseServiceAccount.value()); - serviceAccountJson; - probotApp; - - logger.info("Hello logs!", { structuredData: true }); - res.send("Hello from Firebase!"); + return probotMiddleware(req, res); }, );
18-18
: appId を数値型に変換してください。GitHub App ID は数値型ですが、
githubAppId.value()
は文字列を返します。型の不一致により実行時エラーが発生する可能性があります。次の diff を適用してください:
export const probotApp = createNodeMiddleware(appFn, { probot: createProbot({ overrides: { - appId: githubAppId.value(), + appId: Number.parseInt(githubAppId.value(), 10), privateKey: githubPrivateKey.value(), secret: githubWebhookSecret.value(), },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
openci-runner/firebase/functions/src/index.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: open-ci-io/openci#0
File: AGENTS.md:0-0
Timestamp: 2025-09-27T13:05:21.020Z
Learning: Applies to github-apps/src/**/*.{ts,tsx} : GitHub App のWebhookハンドラ実装は github-apps/src/ に置く
🧬 Code graph analysis (1)
openci-runner/firebase/functions/src/index.ts (2)
openci-runner/firebase/functions/probot/index.ts (1)
appFn
(3-11)openci-runner/firebase/functions/test/hello-world.test.ts (4)
mod
(15-18)app
(21-28)it
(20-29)name
(6-9)
🔇 Additional comments (2)
openci-runner/firebase/functions/src/index.ts (2)
4-7
: LGTM!インポートは正しく、コード内で適切に使用されています。
10-13
: LGTM!GitHub 認証用のシークレットが正しく定義されています。
…b webhook function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
openci-runner/firebase/functions/src/index.ts (1)
27-30
: エラーハンドリングの追加を検討してください。ミドルウェアの実行中にエラーが発生した場合の処理がありません。本番環境での堅牢性を高めるため、エラーハンドリングの追加を推奨します。
次のようなエラーハンドリングを追加できます:
+ try { probot(req, res, () => { res.writeHead(404); res.end(); }); + } catch (error) { + logger.error("Probot middleware error", error); + if (!res.headersSent) { + res.writeHead(500); + res.end(); + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
biome.jsonc
(0 hunks)openci-runner/firebase/functions/src/index.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- biome.jsonc
🧰 Additional context used
🧬 Code graph analysis (1)
openci-runner/firebase/functions/src/index.ts (2)
openci-runner/firebase/functions/probot/index.ts (1)
appFn
(3-11)openci-runner/firebase/functions/test/hello-world.test.ts (3)
mod
(15-18)app
(21-28)it
(20-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy-firebase-functions-dev
🔇 Additional comments (2)
openci-runner/firebase/functions/src/index.ts (2)
1-5
: インポートは正しく実装されています。Probot の必要な関数と
appFn
が適切にインポートされており、ESM 形式(.js
拡張子)も正しく使用されています。
7-9
: シークレット定義は適切です。GitHub App の認証に必要な3つのシークレット(App ID、秘密鍵、Webhook シークレット)が正しく定義されています。
const probot = await createNodeMiddleware(appFn, { | ||
probot: createProbot({ | ||
overrides: { | ||
appId: githubAppId.value(), | ||
privateKey: githubPrivateKey.value(), | ||
secret: githubWebhookSecret.value(), | ||
}, | ||
}), | ||
webhooksPath: "/", | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
リクエストごとのミドルウェア作成はパフォーマンス上の問題があります。
現在の実装では、リクエストごとに Probot ミドルウェアを作成しています。これは以下の問題を引き起こします:
- 接続プーリングやキャッシュが効かず、パフォーマンスが大幅に低下します
- GitHub API のレート制限に達しやすくなります
- 不要な初期化オーバーヘッドが発生します
Firebase Functions v2 では、シークレットはハンドラ内でのみ利用可能ですが、ミドルウェアをキャッシュして再利用する仕組みの実装を検討してください。
次のパターンでシングルトン実装を検討してください:
let cachedProbot: ReturnType<typeof createNodeMiddleware> | null = null;
export const githubWebhook = onRequest(
{
secrets: [githubAppId, githubPrivateKey, githubWebhookSecret],
},
async (req, res) => {
if (!cachedProbot) {
cachedProbot = createNodeMiddleware(appFn, {
probot: createProbot({
overrides: {
appId: Number.parseInt(githubAppId.value(), 10),
privateKey: githubPrivateKey.value(),
secret: githubWebhookSecret.value(),
},
}),
webhooksPath: "/",
});
}
cachedProbot(req, res, () => {
res.writeHead(404);
res.end();
});
},
);
🤖 Prompt for AI Agents
In openci-runner/firebase/functions/src/index.ts around lines 16 to 25, the code
creates a new Probot middleware on every request which causes performance and
rate-limit issues; change this to a cached/singleton pattern by declaring a
module-scoped variable to hold the created middleware, and inside the exported
onRequest handler (which has access to secrets) initialize the cached middleware
only once using the handler-scoped secret values (ensure appId is parsed to a
number), then invoke the cached middleware for each request and provide a next
callback that returns 404 if not handled.
const probot = await createNodeMiddleware(appFn, { | ||
probot: createProbot({ | ||
overrides: { | ||
appId: githubAppId.value(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
App ID を数値に変換してください。
GitHub App ID は文字列として保存されていますが、数値型として渡す必要があります。以前のレビューでも同様の指摘がありました。
次の diff を適用してください:
overrides: {
- appId: githubAppId.value(),
+ appId: Number.parseInt(githubAppId.value(), 10),
privateKey: githubPrivateKey.value(),
secret: githubWebhookSecret.value(),
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
appId: githubAppId.value(), | |
overrides: { | |
appId: Number.parseInt(githubAppId.value(), 10), | |
privateKey: githubPrivateKey.value(), | |
secret: githubWebhookSecret.value(), |
🤖 Prompt for AI Agents
In openci-runner/firebase/functions/src/index.ts around line 19, the GitHub App
ID is being passed as a string (appId: githubAppId.value()) but must be provided
as a number; update the assignment to convert the value to a numeric type before
passing it (e.g., use Number(githubAppId.value()) or parseInt with a radix) and
ensure you handle or surface invalid/NaN cases appropriately (throw or log) so a
non-numeric value doesn't get sent.
Summary by CodeRabbit