Skip to content

Conversation

mafreud
Copy link
Contributor

@mafreud mafreud commented Oct 2, 2025

Summary by CodeRabbit

  • チョア
    • CI/CD を整理し、Node.js を .nvmrc で一元管理へ移行
    • Firebase デプロイ確認ワークフローを追加し、ワークフロー手順と依存インストールを明確化
    • ランタイム要件と開発依存を更新し、ビルド/エミュレーター実行フローを改善
    • ワークスペース辞書に nvmrc を追加
  • テスト
    • 複数バージョンのマトリクスを廃止し、単一バージョンでテスト実行へ移行
  • その他
    • TypeScript 設定やモジュール種別、型定義依存などのコンパイル周りを調整
    • 一部モジュールの参照先や開発用依存を更新

Copy link
Contributor

coderabbitai bot commented Oct 2, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

複数のGitHub Actionsワークフローを.nvmrc参照へ切替え、Firebase Functions周りのCIを再編成(依存インストール・ビルド・エミュレータ実行)。.nvmrc追加、いくつかのpackage.json/tsconfigとFirebase側のimportパスを更新。

Changes

Cohort / File(s) Summary
GitHub Actions: Check Firebase Functions CD
\.github/workflows/check-firebase-functions-cd.yml
新規ワークフロー追加。PRトリガーで working-directory: openci-runner/firebase/functionssetup-nodenode-version-file: .nvmrc で使用。github-appsfirebase-functions のインストール・ビルドを行い、google-github-actions/auth@v2 で認証後に firebase emulators:exec --only functions "echo ok" を実行。
GitHub Actions: Firebase Functions CI/CD
\.github/workflows/firebase-functions-ci-cd.yml
既存ワークフローのNodeマトリクスを削除。setup-nodenode-version-file: .nvmrc に変更し、openci-runner/github-apps の依存インストール/ビルドステップを追加。ハードコーディングされた node-version を除去。
GitHub Actions: Reusable GitHub Apps tests
\.github/workflows/reusable-github-apps-tests.yml
strategy.matrix の複数 Node バージョンを削除し、Set up Node.jsnode-version-file: .nvmrc に変更(単一バージョン化)。
Node version management
.nvmrc
新規追加(内容: 22)。
Firebase functions: package / module / config
openci-runner/firebase/functions/package.json, openci-runner/firebase/functions/src/index.ts, openci-runner/firebase/functions/tsconfig.json
package.json"type":"module" 追加、devDependency @types/node^24.6.2 に更新。src/index.ts の github-app import を ../../../github-apps/lib/index.js に変更。tsconfig.jsonesModuleInterop:falseallowSyntheticDefaultImports:true に変更、rootDir:"src" を追加。
GitHub Apps runner: package / tsconfig
openci-runner/github-apps/package.json, openci-runner/github-apps/tsconfig.json
package.jsonengines.node"22" に変更、devDependency に @types/ioredis を追加。tsconfig.jsonskipLibCheck:trueforceConsistentCasingInFileNames:true を追加。
VS Code workspace
openci-controller.code-workspace
cSpell.words に nvmrc を追加。

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as 開発者
  participant GH as GitHub Actions
  participant Node as setup-node\n(node-version-file: .nvmrc)
  participant GA as github-apps build
  participant FB as Firebase Emulators

  Dev->>GH: PR / push トリガー
  GH->>Node: Set up Node.js (from .nvmrc)
  Node-->>GH: Node v22
  GH->>GA: npm ci & npm run build\n(openci-runner/github-apps)
  GH->>GH: npm ci & npm run build\n(openci-runner/firebase/functions)
  alt Emulator check / deploy
    GH->>FB: firebase emulators:exec --only functions "echo ok"
    FB-->>GH: 実行結果 / exit code
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

ぴょんと置いた .nvmrc 一枚で
行き先は二十二、道は一本🌿
ビルドは積み、エミュは静かに返す
モジュール繋ぎ直し、ランナーは跳ねる🥕
また走るよ、CIの野原で

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed タイトルは @types/node のバージョン更新とNodeエンジン要件の変更という実際の変更を正しく示していますが、ワークフローの .nvmrc 統合などプルリク全体の主要な意図を網羅していません。
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between e8d80fb and 36df913.

📒 Files selected for processing (1)
  • .github/workflows/check-firebase-functions-cd.yml (1 hunks)

Comment @coderabbitai help to get the list of available commands and usage tips.

@mafreud mafreud changed the title fix: update @types/node version to 24.6.2 and set node engine require… fix: update @types/node version to 24.6.2 and set node engine requirement to 22 Oct 2, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.

📥 Commits

Reviewing files that changed from the base of the PR and between b297c39 and d69fdb8.

⛔ Files ignored due to path filters (1)
  • openci-runner/firebase/functions/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (7)
  • .github/workflows/check-firebase-functions-cd.yml (2 hunks)
  • .github/workflows/firebase-functions-ci-cd.yml (2 hunks)
  • .github/workflows/reusable-github-apps-tests.yml (1 hunks)
  • .nvmrc (1 hunks)
  • openci-controller.code-workspace (1 hunks)
  • openci-runner/firebase/functions/package.json (1 hunks)
  • openci-runner/github-apps/package.json (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/check-firebase-functions-cd.yml

22-22: could not parse as YAML: yaml: line 22: mapping values are not allowed in this context

(syntax-check)

🪛 YAMLlint (1.37.1)
.github/workflows/check-firebase-functions-cd.yml

[error] 22-22: syntax error: mapping values are not allowed here

(syntax)

🔇 Additional comments (9)
openci-controller.code-workspace (1)

66-66: LGTM!

新しく追加された.nvmrcファイルに対応するため、スペルチェック辞書に"nvmrc"を追加しています。適切な変更です。

.nvmrc (1)

1-1: LGTM!

Node.jsバージョン22を指定する.nvmrcファイルの追加は適切です。これにより、プロジェクト全体で一貫したNode.jsバージョン管理が可能になります。

openci-runner/github-apps/package.json (1)

29-29: LGTM!

engines.node">= 18"から"22"に更新し、.nvmrcファイルと一致させています。これによりNode.jsバージョンの要件が明確になります。

ただし、このファイルの@types/node(19行目)も^24.6.2を使用していることに注意してください。この依存関係のバージョンが有効かどうか確認することをお勧めします。

.github/workflows/check-firebase-functions-cd.yml (2)

1-1: LGTM!

ワークフロー名を「Check Firebase Functions Deploy」に変更し、より明確になりました。


19-19: LGTM!

ハードコードされたNode.jsバージョンからnode-version-file: .nvmrcへの移行は適切です。プロジェクト全体で一貫したバージョン管理が可能になります。

.github/workflows/firebase-functions-ci-cd.yml (2)

33-33: LGTM!

firebase-functions-testsジョブでハードコードされたNode.jsバージョンから.nvmrcファイル参照への移行は適切です。


55-55: LGTM!

deploy-firebase-functionsジョブでも.nvmrcファイル参照を使用するよう統一されています。プロジェクト全体で一貫性が保たれています。

.github/workflows/reusable-github-apps-tests.yml (1)

21-21: LGTM!

マトリックス戦略から.nvmrcファイル参照への移行は適切です。これにより、テストが単一のNode.jsバージョンで実行され、プロジェクト全体の一貫性が向上します。

openci-runner/firebase/functions/package.json (1)

24-24: @types/[email protected] の存在を確認しました
npm view の結果、このバージョンは公開済みで、依存インストールは正常に動作します。

mafreud added 19 commits October 2, 2025 18:43
…ring dependency installation in Firebase Functions CI workflow
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (2)
openci-runner/github-apps/package.json (1)

34-34: 厳密なNode.jsバージョン要件を確認してください。

engines.node"22"に設定されていますが、これはNode.js 22系のすべてのバージョンを許可する指定です(npm/yarnの解釈では22.x.xにマッチ)。ただし、より明示的な範囲指定(例:">=22.0.0 <23.0.0""^22.0.0")の方が意図が明確になります。

リポジトリ全体でNode.js 22への統一を図っているとのことなので、この変更は適切です。.nvmrcとCIワークフローとの整合性も取れています。

より明示的にしたい場合は、以下のような範囲指定も検討できます:

-		"node": "22"
+		"node": ">=22.0.0 <23.0.0"

または:

-		"node": "22"
+		"node": "^22.0.0"
.github/workflows/check-firebase-functions-cd.yml (1)

33-42: デバッグコマンドの削除を検討してください。

ビルドとエミュレータ実行のフローは正しく構成されていますが、35-39行目のデバッグコマンド(pwdls -a)が残っています。これらは開発中の検証には有用ですが、本番ワークフローでは不要です。

次のdiffを適用してデバッグコマンドを削除できます:

       - name: Dry-run check via Emulator on PRs
         run: |
           npm run build
           cd ../../github-apps && npm run build && cd -
-          pwd
-          ls -a
           cd ..
-          pwd
-          ls -a
           npx firebase-tools emulators:exec \
             --only functions \
             --project ${{ secrets.FIREBASE_PROJECT_ID }}
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between ebdb547 and efd164a.

⛔ Files ignored due to path filters (1)
  • openci-runner/github-apps/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (2)
  • .github/workflows/check-firebase-functions-cd.yml (2 hunks)
  • openci-runner/github-apps/package.json (2 hunks)
🔇 Additional comments (2)
.github/workflows/check-firebase-functions-cd.yml (2)

17-19: LGTM!

.nvmrcファイルを参照するように変更することで、プロジェクト全体でNode.jsバージョンの一貫性が保たれます。他のワークフローとも統一されており、適切な変更です。


21-25: 作業ディレクトリのコンテキストが失われます。

24行目のcd ../../github-apps && npm ciを実行すると、その後の作業ディレクトリがopenci-runner/github-appsに変わってしまいます。このジョブではdefaults.run.working-directoryopenci-runner/firebase/functionsに設定されていますが、明示的なcdコマンドでディレクトリを変更すると、この設定が上書きされます。

その結果、32行目以降の「Dry-run check via Emulator on PRs」ステップが誤ったディレクトリから実行される可能性があります。

次のdiffを適用して、元のディレクトリに戻るように修正してください:

       - name: Install dependencies
         run: |
           npm ci
-          cd ../../github-apps && npm ci
+          cd ../../github-apps && npm ci && cd -
           pwd

Likely an incorrect or invalid review comment.

"probot": "^14.0.2"
},
"devDependencies": {
"@types/ioredis": "^4.28.10",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

@types/ioredisの使用を確認してください。

新しいdevDependencyとして@types/ioredisが追加されていますが、ioredisパッケージ自体がdependenciesに含まれていません。このコードベースで実際にioredisが使用されているか確認してください。

以下のスクリプトを実行して、ioredisの使用状況を確認してください:


🏁 Script executed:

#!/bin/bash
# Description: ioredisの使用状況を確認

# ioredisパッケージが依存関係に含まれているか確認
echo "=== Checking for ioredis in dependencies ==="
fd -e json -x cat {} | jq 'select(.dependencies.ioredis or .devDependencies.ioredis)'

# ioredisのimportまたはrequire文を検索
echo "=== Checking for ioredis imports/requires ==="
rg -n --type ts --type js -C3 "from ['\"]ioredis['\"]|require\(['\"]ioredis['\"]\)"

# IORedisクラスの使用を検索
echo "=== Checking for IORedis class usage ==="
rg -n --type ts --type js -C3 "\bIORedis\b|\bredis\s*=\s*new\b"

Length of output: 190


以下のスクリプトで再度確認してください:openci-runner/github-apps/package.json@types/ioredis が正しく定義されているか、またコード内で ioredis が実際にインポート/使用されているかをチェックします。


🏁 Script executed:

#!/bin/bash
echo "=== devDependencies に @types/ioredis があるか ==="
jq '.devDependencies["@types/ioredis"] // "not defined"' openci-runner/github-apps/package.json
echo "=== dependencies に ioredis があるか ==="
jq '.dependencies["ioredis"] // "not defined"' openci-runner/github-apps/package.json
echo "=== コードベースでの ioredis の import/require ==="
rg -n --type js --type ts "(import .* from ['\"]ioredis['\"]|require\(['\"]ioredis['\"]\))" -C3 .

Length of output: 148


@types/ioredis は未使用のため削除
openci-runner/github-apps/package.json 行 23:@types/ioredis を削除してください。(ioredis 本体の依存もコード内で参照がなく不要です)

🤖 Prompt for AI Agents
In openci-runner/github-apps/package.json around line 23, remove the unused
dependency entry "@types/ioredis" (and if present elsewhere remove the "ioredis"
package as well since code does not reference it); after editing package.json
run the package manager (npm/yarn/pnpm) install to update the lockfile and
ensure no lingering references remain.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between efd164a and 825b6b7.

📒 Files selected for processing (5)
  • .github/workflows/check-firebase-functions-cd.yml (2 hunks)
  • openci-runner/firebase/functions/package.json (1 hunks)
  • openci-runner/firebase/functions/src/index.ts (1 hunks)
  • openci-runner/firebase/functions/tsconfig.json (1 hunks)
  • openci-runner/github-apps/tsconfig.json (1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Check Firebase Functions Deploy
openci-runner/firebase/functions/src/index.ts

[error] 6-6: tsc error: Cannot find module '../../../github-apps/lib/index.js' or its corresponding type declarations.

🪛 GitHub Actions: Firebase Functions Tests
openci-runner/firebase/functions/src/index.ts

[error] 6-6: Cannot find module '../../../github-apps/lib/index.js' imported from '/home/runner/work/openci/openci/openci-runner/firebase/functions/src/index.ts'. Does the file exist?

🪛 GitHub Check: firebase-functions-tests
openci-runner/firebase/functions/src/index.ts

[failure] 6-6: test/hello-world.test.ts
Error: Cannot find module '../../../github-apps/lib/index.js' imported from '/home/runner/work/openci/openci/openci-runner/firebase/functions/src/index.ts'
❯ src/index.ts:6:1

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'ERR_MODULE_NOT_FOUND' }
Caused by: Caused by: Error: 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?
❯ loadAndTransform node_modules/vite/dist/node/chunks/dep-Bu492Fnd.js:35728:17

🔇 Additional comments (3)
openci-runner/github-apps/tsconfig.json (1)

18-19: LGTM!

skipLibCheck: trueの追加は適切です。これにより、外部ライブラリの型定義ファイルの型チェックをスキップし、コンパイル時間を短縮できます。

openci-runner/firebase/functions/tsconfig.json (1)

4-4: LGTM!

esModuleInterop: falseallowSyntheticDefaultImports: trueの組み合わせは、NodeNextモジュールシステムで適切です。型チェック時にデフォルトインポートを許可しつつ、実行時のインターオペラビリティは無効化されています。

Also applies to: 13-13

openci-runner/firebase/functions/package.json (1)

30-30: LGTM!

"type": "module"の追加は適切です。これにより、パッケージがES Modulesとして扱われ、.jsファイルがESM構文で解釈されます。tsconfig.jsonmodule: "NodeNext"設定と整合性があります。

"devDependencies": {
"@types/supertest": "^6.0.3",
"@types/node": "^22.5.0",
"@types/node": "^24.6.2",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

@types/nodeのバージョンが不正です。

Node.js 22を使用している場合、@types/nodeのバージョンは^22.x.xである必要がありますが、^24.6.2が指定されています。これは以下の問題を引き起こす可能性があります:

  1. Node.js 24の型定義が存在しない(Node.js 24はまだリリースされていません)
  2. 型の不一致によるコンパイルエラーや実行時エラー

PRタイトルでは「update @types/node version to 24.6.2」となっていますが、おそらく22.6.2の誤記と思われます。

以下のdiffを適用して修正してください:

-		"@types/node": "^24.6.2",
+		"@types/node": "^22.6.2",

以下のスクリプトを実行して、@types/nodeの最新バージョンを確認してください:


🏁 Script executed:

#!/bin/bash
# @types/nodeの最新バージョンを確認
npm view @types/node versions --json | jq '.[-10:]'

# Node.js 22に対応する@types/nodeバージョンを確認
echo -e "\n=== Checking @types/node for Node.js 22 ==="
npm view @types/node@22 version

Length of output: 3993


@types/nodeのバージョン不整合

Firebase Functionsのpackage.jsonでエンジンにNode.js 22を指定していますが、@types/node^24.6.2となっており型定義と実行環境が一致していません。^22.x.x系(例:^22.18.8)に戻してください。

-  "@types/node": "^24.6.2",
+  "@types/node": "^22.18.8",
📝 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.

Suggested change
"@types/node": "^24.6.2",
"@types/node": "^22.18.8",
🤖 Prompt for AI Agents
In openci-runner/firebase/functions/package.json around line 24, @types/node is
set to ^24.6.2 which mismatches the Functions engine (Node.js 22); change the
devDependency to a Node 22-compatible version such as ^22.18.8 (or ^22.x.x) to
align type definitions with the runtime, then run npm install (or update
package-lock.json) to apply the version change.

"skipLibCheck": true,
"allowSyntheticDefaultImports": true
},
"rootDir": "src",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

rootDirの配置を修正してください。

rootDircompilerOptionsの中に配置する必要があります。現在はトップレベルに配置されているため、TypeScriptコンパイラに認識されません。

以下のdiffを適用して修正してください:

   "compilerOptions": {
     "module": "NodeNext",
     "esModuleInterop": false,
     "moduleResolution": "nodenext",
     "noImplicitReturns": true,
     "noUnusedLocals": true,
     "outDir": "lib",
     "sourceMap": true,
     "strict": true,
     "target": "es2022",
     "skipLibCheck": true,
-    "allowSyntheticDefaultImports": true
+    "allowSyntheticDefaultImports": true,
+    "rootDir": "src"
   },
-  "rootDir": "src",
   "compileOnSave": true,
📝 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.

Suggested change
"rootDir": "src",
"compilerOptions": {
"module": "NodeNext",
"esModuleInterop": false,
"moduleResolution": "nodenext",
"noImplicitReturns": true,
"noUnusedLocals": true,
"outDir": "lib",
"sourceMap": true,
"strict": true,
"target": "es2022",
"skipLibCheck": true,
"allowSyntheticDefaultImports": true,
"rootDir": "src"
},
"compileOnSave": true,
🤖 Prompt for AI Agents
openci-runner/firebase/functions/tsconfig.json around line 15: the "rootDir"
property is currently at the top level but must be nested inside
"compilerOptions"; move the existing "rootDir": "src" entry into the
"compilerOptions" object so the TypeScript compiler recognizes it, ensuring you
keep JSON syntax valid (commas/braces) after the move.

mafreud added 17 commits October 3, 2025 00:31
@mafreud mafreud merged commit 5ffa66c into develop Oct 2, 2025
6 of 7 checks passed
@mafreud mafreud deleted the update-node-related-packages branch October 2, 2025 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant