-
Notifications
You must be signed in to change notification settings - Fork 1
JIT configを作成 #558
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
JIT configを作成 #558
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughGitHub App 認証を導入し、 Changes
Sequence Diagram(s)sequenceDiagram
participant Webhook as Webhook Event
participant Handler as index.ts Handler
participant App as @octokit/app
participant InstOct as Installation Octokit
participant GH as GitHub API
Webhook->>Handler: workflow_job (queued)
Note right of Handler: payload から installationId を取得
alt installationId missing
Handler-->>Webhook: 400 Bad Request
else installationId present
Handler->>Handler: 環境変数 GH_APP_ID / GH_APP_PRIVATE_KEY を検証
alt env missing
Handler-->>Webhook: 500 Internal Server Error
else env present
Handler->>App: new App({ appId, privateKey, Octokit })
App->>InstOct: getInstallationOctokit(installationId)
InstOct->>GH: actions.generateRunnerJitconfigForRepo(params)
GH-->>InstOct: JIT config response
InstOct-->>Handler: response data
Handler-->>Webhook: 201 Created + message
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 注意して確認すべき箇所:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
Comment |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
openci-runner-dev | 4844053 | Commit Preview URL Branch Preview URL |
Nov 02 2025, 08:45 AM |
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 ignored due to path filters (1)
openci-runner/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
openci-runner/.gitignore(1 hunks)openci-runner/package.json(1 hunks)openci-runner/src/index.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
openci-runner/src/index.ts (2)
openci-runner/worker-configuration.d.ts (1)
env(9024-9024)openci-runner/test/index.spec.ts (1)
webhookSecret(100-119)
⏰ 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: Workers Builds: openci-runner-dev
openci-runner/src/index.ts
Outdated
| const { data } = | ||
| await octokit.rest.actions.generateRunnerJitconfigForRepo({ | ||
| labels: ["openci-runner-beta-dev"], | ||
| name: `OpenCIランナーβ(開発環境)-${Date.now()}`, | ||
| owner: payload.repository.owner.login, | ||
| repo: payload.repository.name, | ||
| runner_group_id: 1, | ||
| work_folder: "_work", | ||
| }); | ||
|
|
||
| return new Response(`Successfully created OpenCI runner: ${data}`, { | ||
| status: 201, |
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.
レスポンス本文が [object Object] になってしまいます
generateRunnerJitconfigForRepo が返す data はオブジェクトなので、テンプレートリテラルで連結すると実際のレスポンスは "Successfully created OpenCI runner: [object Object]" になります。現行テスト(openci-runner/test/index.spec.ts Line 111 付近)も "Successfully created OpenCI runner" を期待しており、この変更で確実に失敗しますし、クライアント側も生成された JIT 設定を取得できません。
- return new Response(`Successfully created OpenCI runner: ${data}`, {
- status: 201,
- });
+ return new Response(JSON.stringify(data), {
+ status: 201,
+ headers: { "content-type": "application/json" },
+ });必要なら成功メッセージを付けたい場合は { message: "...", data } のように JSON を組み立てるのが良さそうです。
📝 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.
| const { data } = | |
| await octokit.rest.actions.generateRunnerJitconfigForRepo({ | |
| labels: ["openci-runner-beta-dev"], | |
| name: `OpenCIランナーβ(開発環境)-${Date.now()}`, | |
| owner: payload.repository.owner.login, | |
| repo: payload.repository.name, | |
| runner_group_id: 1, | |
| work_folder: "_work", | |
| }); | |
| return new Response(`Successfully created OpenCI runner: ${data}`, { | |
| status: 201, | |
| const { data } = | |
| await octokit.rest.actions.generateRunnerJitconfigForRepo({ | |
| labels: ["openci-runner-beta-dev"], | |
| name: `OpenCIランナーβ(開発環境)-${Date.now()}`, | |
| owner: payload.repository.owner.login, | |
| repo: payload.repository.name, | |
| runner_group_id: 1, | |
| work_folder: "_work", | |
| }); | |
| return new Response(JSON.stringify(data), { | |
| status: 201, | |
| headers: { "content-type": "application/json" }, | |
| }); |
🤖 Prompt for AI Agents
In openci-runner/src/index.ts around lines 43 to 54, the code returns a template
string including the object `data`, which becomes "[object Object]" in the
response; change the response to return JSON: build an object like { message:
"Successfully created OpenCI runner", data } and return it as a JSON body
(serialize with JSON.stringify) and set Content-Type: application/json so the
client receives the actual JIT config object; update the response status to 201
as-is and adjust tests if they assert on plain string.
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/src/index.ts (1)
55-57: レスポンス本文が[object Object]になります過去のレビューコメントで指摘された通り、
dataオブジェクトをテンプレートリテラルで連結すると"Successfully created OpenCI runner: [object Object]"となり、実際の JIT 設定が取得できません。また、テスト (openci-runner/test/index.spec.ts:117) は"Successfully created OpenCI runner"を期待しているため、このままではテストが失敗します。- return new Response(`Successfully created OpenCI runner: ${data}`, { + return new Response(JSON.stringify({ + message: "Successfully created OpenCI runner", + data + }), { status: 201, + headers: { "content-type": "application/json" }, });テストも JSON レスポンスを期待するように更新する必要があります。
🧹 Nitpick comments (1)
openci-runner/src/index.ts (1)
35-39: 環境変数のバリデーションを追加してください
GH_APP_IDとGH_APP_PRIVATE_KEYが未設定の場合、より明確なエラーメッセージを返すための事前バリデーションを追加することを推奨します。+ if (!env.GH_APP_ID || !env.GH_APP_PRIVATE_KEY) { + return new Response("GitHub App credentials not configured", { + status: 500, + }); + } + const app = new App({ appId: env.GH_APP_ID, Octokit: Octokit, privateKey: env.GH_APP_PRIVATE_KEY, });
📜 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/src/index.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: open-ci-io/openci PR: 0
File: packages/openci-controller/AGENTS.md:0-0
Timestamp: 2025-09-04T06:33:41.049Z
Learning: Conventional Commits(feat(scope): msg / fix(scope): msg)を用い、Issue を参照(例: #391)
🧬 Code graph analysis (1)
openci-runner/src/index.ts (2)
openci-runner/worker-configuration.d.ts (1)
env(9024-9024)openci-runner/test/index.spec.ts (2)
webhookSecret(100-119)webhookSecret(121-140)
⏰ 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: Workers Builds: openci-runner-dev
🔇 Additional comments (2)
openci-runner/src/index.ts (2)
1-2: LGTM: 必要なインポートが追加されていますGitHub App 認証と REST API 呼び出しに必要な依存関係が正しくインポートされています。
28-34: LGTM: Installation ID のバリデーションが適切です
installationIdが存在しない場合に 400 エラーを返す実装は適切です。オプショナルチェーンも正しく使用されています。
| const runnerLabel = "openci-runner-beta-dev"; | ||
|
|
||
| const { data } = | ||
| await octokit.rest.actions.generateRunnerJitconfigForRepo({ | ||
| labels: [runnerLabel], | ||
| name: `OpenCIランナーβ(開発環境)-${Date.now()}`, | ||
| owner: payload.repository.owner.login, | ||
| repo: payload.repository.name, | ||
| runner_group_id: 1, | ||
| work_folder: "_work", | ||
| }); |
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.
ハードコードされた値を環境変数に移行し、エラーハンドリングを追加してください
以下の問題があります:
runner_group_id: 1がハードコードされています - すべての GitHub インストールで runner group ID 1 が存在するとは限らず、実行時エラーの原因になりますrunnerLabelとwork_folderもハードコードされています - 環境ごとに変更できるべきです- API 呼び出しのエラーハンドリングがありません - ネットワークエラーや API エラーが発生した場合、500 エラーとして適切に処理されません
const octokit = await app.getInstallationOctokit(installationId);
- const runnerLabel = "openci-runner-beta-dev";
+ const runnerLabel = env.RUNNER_LABEL || "openci-runner-beta-dev";
- const { data } =
- await octokit.rest.actions.generateRunnerJitconfigForRepo({
- labels: [runnerLabel],
- name: `OpenCIランナーβ(開発環境)-${Date.now()}`,
- owner: payload.repository.owner.login,
- repo: payload.repository.name,
- runner_group_id: 1,
- work_folder: "_work",
- });
+ try {
+ const { data } =
+ await octokit.rest.actions.generateRunnerJitconfigForRepo({
+ labels: [runnerLabel],
+ name: `OpenCIランナーβ(開発環境)-${Date.now()}`,
+ owner: payload.repository.owner.login,
+ repo: payload.repository.name,
+ runner_group_id: env.RUNNER_GROUP_ID ? Number(env.RUNNER_GROUP_ID) : 1,
+ work_folder: env.RUNNER_WORK_FOLDER || "_work",
+ });
+ } catch (error) {
+ console.error("Failed to generate runner JIT config:", error);
+ return new Response("Failed to generate runner JIT config", {
+ status: 500,
+ });
+ }📝 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.
| const runnerLabel = "openci-runner-beta-dev"; | |
| const { data } = | |
| await octokit.rest.actions.generateRunnerJitconfigForRepo({ | |
| labels: [runnerLabel], | |
| name: `OpenCIランナーβ(開発環境)-${Date.now()}`, | |
| owner: payload.repository.owner.login, | |
| repo: payload.repository.name, | |
| runner_group_id: 1, | |
| work_folder: "_work", | |
| }); | |
| const runnerLabel = env.RUNNER_LABEL || "openci-runner-beta-dev"; | |
| let data; | |
| try { | |
| const result = | |
| await octokit.rest.actions.generateRunnerJitconfigForRepo({ | |
| labels: [runnerLabel], | |
| name: `OpenCIランナーβ(開発環境)-${Date.now()}`, | |
| owner: payload.repository.owner.login, | |
| repo: payload.repository.name, | |
| runner_group_id: env.RUNNER_GROUP_ID ? Number(env.RUNNER_GROUP_ID) : 1, | |
| work_folder: env.RUNNER_WORK_FOLDER || "_work", | |
| }); | |
| data = result.data; | |
| } catch (error) { | |
| console.error("Failed to generate runner JIT config:", error); | |
| return new Response("Failed to generate runner JIT config", { | |
| status: 500, | |
| }); | |
| } |
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/src/index.ts (2)
41-54: エラーハンドリングの追加とハードコード値の環境変数化が必要です以下の問題があります:
- API 呼び出しのエラーハンドリングがありません -
getInstallationOctokitとgenerateRunnerJitconfigForRepoはネットワークエラーや API エラーで失敗する可能性がありますrunner_group_id: 1がハードコードされています - すべての GitHub installation で runner group ID 1 が存在するとは限りませんrunnerLabel、work_folderもハードコードされています - 環境ごとに変更可能にすべきです以下の diff を適用してください:
- const octokit = await app.getInstallationOctokit(installationId); - - const runnerLabel = "openci-runner-beta-dev"; - const runnerName = "OpenCIランナーβ(開発環境)"; - - const { data } = - await octokit.rest.actions.generateRunnerJitconfigForRepo({ - labels: [runnerLabel], - name: `${runnerName}-${Date.now()}`, - owner: payload.repository.owner.login, - repo: payload.repository.name, - runner_group_id: 1, - work_folder: "_work", - }); + const runnerLabel = env.RUNNER_LABEL || "openci-runner-beta-dev"; + const runnerName = env.RUNNER_NAME || "OpenCIランナーβ(開発環境)"; + + let octokit; + let data; + try { + octokit = await app.getInstallationOctokit(installationId); + const response = + await octokit.rest.actions.generateRunnerJitconfigForRepo({ + labels: [runnerLabel], + name: `${runnerName}-${Date.now()}`, + owner: payload.repository.owner.login, + repo: payload.repository.name, + runner_group_id: env.RUNNER_GROUP_ID + ? Number(env.RUNNER_GROUP_ID) + : 1, + work_folder: env.RUNNER_WORK_FOLDER || "_work", + }); + data = response.data; + } catch (error) { + console.error("Failed to generate runner JIT config:", error); + return new Response("Failed to generate runner JIT config", { + status: 500, + }); + }Based on learnings
56-61: JIT 設定データをレスポンスに含める必要があります
encoded_jit_configを抽出していますが、レスポンスに含めていないため、クライアントは実際のランナー設定を取得できません。これではランナーを起動できません。以下の diff を適用してレスポンスを JSON 形式にしてください:
- // biome-ignore lint/correctness/noUnusedVariables: <Use this later> - const { encoded_jit_config } = data; - - return new Response(`Successfully created OpenCI runner`, { + return new Response( + JSON.stringify({ + message: "Successfully created OpenCI runner", + encoded_jit_config: data.encoded_jit_config, + }), + { - status: 201, + status: 201, + headers: { "content-type": "application/json" }, + }, );注意: この変更により、テスト (
openci-runner/test/index.spec.tsの Line 117 付近) も更新が必要になります。
🧹 Nitpick comments (1)
openci-runner/src/index.ts (1)
29-34:installationIdの型チェックを厳密にしてください
payload.installation?.idはundefinedだけでなくnullや0の可能性もあります。より厳密なチェックを推奨します。以下の diff を適用してください:
- const installationId = payload.installation?.id; - if (installationId === undefined) { + const installationId = payload.installation?.id; + if (!installationId) { return new Response("Installation ID not found in payload", { status: 400, }); }
📜 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/src/index.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: open-ci-io/openci PR: 0
File: packages/openci-controller/AGENTS.md:0-0
Timestamp: 2025-09-04T06:33:41.049Z
Learning: Conventional Commits(feat(scope): msg / fix(scope): msg)を用い、Issue を参照(例: #391)
📚 Learning: 2025-09-04T06:33:41.049Z
Learnt from: CR
Repo: open-ci-io/openci PR: 0
File: packages/openci-controller/AGENTS.md:0-0
Timestamp: 2025-09-04T06:33:41.049Z
Learning: DATABASE_URL, OPENCI_PORT, RUST_LOG, GITHUB_WEBHOOK_SECRET を適切に設定する
Applied to files:
openci-runner/src/index.ts
🧬 Code graph analysis (1)
openci-runner/src/index.ts (2)
openci-runner/worker-configuration.d.ts (1)
env(9024-9024)openci-runner/test/index.spec.ts (2)
webhookSecret(100-119)webhookSecret(121-140)
⏰ 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: Workers Builds: openci-runner-dev
🔇 Additional comments (2)
openci-runner/src/index.ts (2)
1-2: LGTM!GitHub App 認証に必要な import が正しく追加されています。
77-79: LGTM!定数定義が適切で、
as constによる型安全性も確保されています。
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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 (4)
biome.jsonc(1 hunks)openci-runner/src/index.ts(2 hunks)openci-runner/test/index.spec.ts(3 hunks)openci-runner/wrangler.jsonc(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- openci-runner/src/index.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: open-ci-io/openci PR: 0
File: packages/openci-controller/AGENTS.md:0-0
Timestamp: 2025-09-04T06:33:41.049Z
Learning: Conventional Commits(feat(scope): msg / fix(scope): msg)を用い、Issue を参照(例: #391)
📚 Learning: 2025-10-12T04:41:44.311Z
Learnt from: mafreud
Repo: open-ci-io/openci PR: 505
File: biome.jsonc:12-17
Timestamp: 2025-10-12T04:41:44.311Z
Learning: In Biome configuration (biome.json or biome.jsonc), `recommended: true` can be placed under `linter.rules` to enable recommended rules for all groups. This is a valid configuration according to the official Biome documentation at https://biomejs.dev/ja/reference/configuration/#linterrulesrecommended
Applied to files:
biome.jsonc
🧬 Code graph analysis (1)
openci-runner/test/index.spec.ts (1)
openci-runner/worker-configuration.d.ts (1)
env(9024-9024)
⏰ 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: Workers Builds: openci-runner-dev
🔇 Additional comments (6)
biome.jsonc (1)
25-25: LGTM!
recommended: trueの配置が正しくなりました。Biome の公式ドキュメントに従い、linter.rules直下に配置することで、すべてのグループの推奨ルールが有効になります。Based on learnings
openci-runner/wrangler.jsonc (1)
11-12: 本番環境でのサンプリングレートを確認してください。
head_sampling_rate: 1は 100% サンプリングを意味し、開発環境には適していますが、本番環境では高コストになる可能性があります。本番環境用の設定では、より低い値(例: 0.1)の使用を検討してください。openci-runner/test/index.spec.ts (4)
7-7: LGTM!モック機能に必要な
viのインポートが正しく追加されています。
126-137: LGTM!テストペイロードが GitHub App の webhook ペイロード構造に正しく更新されています。
installation、repository、workflow_jobの必須フィールドが含まれています。
177-206: LGTM!
installationIdが欠落している場合の 400 エラーケースが適切にテストされています。エラーハンドリングのカバレッジが向上しました。
208-240: LGTM!GitHub API 障害時の 500 エラーケースが適切にテストされています。
mockRejectedValueOnceを使用することで、このテストのみに影響を限定しています。
Summary by CodeRabbit
新機能
Chores
Tests
Style