feat(db): add schema for multiple GitHub App installations (#283 PR 1/7)#288
feat(db): add schema for multiple GitHub App installations (#283 PR 1/7)#288
Conversation
shared DB: - github_app_links: PK を (organization_id, installation_id) に変更 - UNIQUE (github_account_id) を廃止、INDEX に変更 - suspended_at / github_account_type / membership_initialized_at カラム追加 - github_app_link_events 監査ログテーブル新規追加 tenant DB: - repositories.github_installation_id カラム追加 - repository_installation_memberships テーブル新規追加 本 PR は schema と型生成のみ。アプリケーションコードからの参照は PR 2 以降で追加する。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthrough複数のGitHub App インストレーションを組織ごとにサポートするために、データベーススキーマとTypeScript型定義を拡張しました。リポジトリテーブルに Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Actions performedReview triggered.
|
1 similar comment
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/services/type.ts`:
- Around line 52-74: The generated Kysely types in GithubAppLinkEvents and
GithubAppLinks (and corresponding entries in tenant-type.ts around the mentioned
lines) have been manually edited and are out of sync with codegen; revert any
hand changes and regenerate the types by running the codegen command (pnpm
db:generate), replace app/services/type.ts and app/services/tenant-type.ts with
the exact output produced, and commit those regenerated files so the Generated
DB types match CI expectations (look for the interfaces GithubAppLinkEvents and
GithubAppLinks to verify the corrected, codegen-produced definitions).
In `@db/shared.sql`:
- Around line 157-163: 現在のクエリ/サービスは org ごとに単一行を前提にしているため、複数 installation
を許可したスキーマ変更で取りこぼしや全件更新が発生します。対処は二択で揃えてください:1) 永続対応:batch/db/queries.ts の
getGithubAppLinkByOrgId() を installation-aware に改名/拡張して (例:
getGithubAppLinkByOrgAndInstallation(orgId, installationId)) あるいは
getAllGithubAppLinks() を org
毎に複数行返すように修正し、app/services/github-app-mutations.server.ts と
app/services/github-webhook-installation.server.ts の更新・削除ロジックを organizationId
単体ではなく installation_id をキーにして対象行を特定またはループで処理する(更新/削除は installation_id を WHERE
に含める)。2) 一時ガード:リリースまでの暫定対応として insert/update 前に既存行の存在チェックを追加して 1 org = 1 row
を維持する(新規挿入を拒否または既存行を更新する)。該当する識別子:getGithubAppLinkByOrgId(),
getAllGithubAppLinks(), app/services/github-app-mutations.server.ts,
app/services/github-webhook-installation.server.ts を修正対象として扱ってください。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ce73a542-af8b-4782-8295-5fe45b2d976e
⛔ Files ignored due to path filters (2)
db/migrations/shared/atlas.sumis excluded by!**/*.sumdb/migrations/tenant/atlas.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
app/services/tenant-type.tsapp/services/type.tsbatch/db/queries.tsdb/migrations/shared/20260407084907.sqldb/migrations/tenant/20260407084907.sqldb/shared.sqldb/tenant.sql
| export interface GithubAppLinkEvents { | ||
| createdAt: Generated<string>; | ||
| detailsJson: string | null; | ||
| eventType: string; | ||
| id: Generated<number>; | ||
| installationId: number; | ||
| organizationId: string; | ||
| source: string; | ||
| status: string; | ||
| } | ||
|
|
||
| export interface GithubAppLinks { | ||
| appRepositorySelection: "all" | "selected"; | ||
| createdAt: Generated<string>; | ||
| deletedAt: string | null; | ||
| githubAccountId: number; | ||
| githubAccountType: string | null; | ||
| githubOrg: string; | ||
| installationId: number; | ||
| membershipInitializedAt: string | null; | ||
| organizationId: string; | ||
| suspendedAt: string | null; | ||
| updatedAt: Generated<string>; |
There was a problem hiding this comment.
生成ファイルがまだ codegen と同期していません。
CI の Generated DB types are out of date が出ているので、このコミットの app/services/type.ts / app/services/tenant-type.ts はまだ再生成結果と一致していません。ここは手修正を積むのではなく、pnpm db:generate の出力をそのままコミットし直してください。
Based on learnings: In app/services/type.ts, do not manually modify generated kysely-codegen types. Rely on the codegen output for types and migrations for schema changes; manual edits can cause drift between codegen and database schema.
Also applies to: 173-174
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/services/type.ts` around lines 52 - 74, The generated Kysely types in
GithubAppLinkEvents and GithubAppLinks (and corresponding entries in
tenant-type.ts around the mentioned lines) have been manually edited and are out
of sync with codegen; revert any hand changes and regenerate the types by
running the codegen command (pnpm db:generate), replace app/services/type.ts and
app/services/tenant-type.ts with the exact output produced, and commit those
regenerated files so the Generated DB types match CI expectations (look for the
interfaces GithubAppLinkEvents and GithubAppLinks to verify the corrected,
codegen-produced definitions).
| PRIMARY KEY (`organization_id`, `installation_id`), | ||
| CONSTRAINT `github_app_links_organization_id_fkey` FOREIGN KEY (`organization_id`) REFERENCES `organizations` (`id`) ON UPDATE CASCADE ON DELETE CASCADE | ||
| ); | ||
| -- Create index "github_app_links_installation_id_key" to table: "github_app_links" | ||
| CREATE UNIQUE INDEX `github_app_links_installation_id_key` ON `github_app_links` (`installation_id`); | ||
| -- Create index "github_app_links_github_account_id_key" to table: "github_app_links" | ||
| CREATE UNIQUE INDEX `github_app_links_github_account_id_key` ON `github_app_links` (`github_account_id`); | ||
| -- Create index "github_app_links_github_account_id_idx" to table: "github_app_links" | ||
| CREATE INDEX `github_app_links_github_account_id_idx` ON `github_app_links` (`github_account_id`); |
There was a problem hiding this comment.
2件目の installation を許可する前に、既存の singleton 前提を外す対応が必要です。
organization_id ごとに複数行を持てるようにした一方で、現状の batch/db/queries.ts は getGithubAppLinkByOrgId() で 1 件だけ取得し、getAllGithubAppLinks() でも organizationId ごとに 1 件へ潰しています。さらに app/services/github-app-mutations.server.ts と app/services/github-webhook-installation.server.ts の更新系も organizationId だけで githubAppLinks を更新しているので、2件目の installation が入り始めると取りこぼしか全件更新/削除になります。installation-aware な read/write と同じリリース境界に揃えるか、それまでは 1 org = 1 row を保つガードが必要です。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@db/shared.sql` around lines 157 - 163, 現在のクエリ/サービスは org ごとに単一行を前提にしているため、複数
installation を許可したスキーマ変更で取りこぼしや全件更新が発生します。対処は二択で揃えてください:1)
永続対応:batch/db/queries.ts の getGithubAppLinkByOrgId() を installation-aware
に改名/拡張して (例: getGithubAppLinkByOrgAndInstallation(orgId, installationId)) あるいは
getAllGithubAppLinks() を org
毎に複数行返すように修正し、app/services/github-app-mutations.server.ts と
app/services/github-webhook-installation.server.ts の更新・削除ロジックを organizationId
単体ではなく installation_id をキーにして対象行を特定またはループで処理する(更新/削除は installation_id を WHERE
に含める)。2) 一時ガード:リリースまでの暫定対応として insert/update 前に既存行の存在チェックを追加して 1 org = 1 row
を維持する(新規挿入を拒否または既存行を更新する)。該当する識別子:getGithubAppLinkByOrgId(),
getAllGithubAppLinks(), app/services/github-app-mutations.server.ts,
app/services/github-webhook-installation.server.ts を修正対象として扱ってください。
|
@coderabbitai 指摘ありがとうございます。両方とも回答です。 1. 手動編集はありません。 2. クエリ層が "1 org = 1 row" 前提のままになっている件 これは stacked PR の構造による意図的な分離です。本 PR (#288) は schema 追加と型生成のみ をスコープとし、caller の追従は後続 PR で対応しています:
この分離理由は stack 全体のコンテキストで再レビューしていただくと、"1 org = 1 row" 前提は段階的に解消されているのが見えると思います。 |
|
Tip For best results, initiate chat on the files or code changes.
次の PR ( (∩`-´)⊃━☆゚.*・。゚ ✏️ Learnings added
🧠 Learnings used |
…queries (#283 PR 2/7) (#296) ## Summary Issue #283 の実装 stack **PR 2/7** — query / mutation / Octokit 解決層を複数 installation 対応にする。アプリケーションの動作はまだ変えない(fallback で互換維持)。 設計根拠: [`docs/rdd/issue-283-multiple-github-accounts.md`](./docs/rdd/issue-283-multiple-github-accounts.md) 作業計画: [`docs/rdd/issue-283-work-plan.md`](./docs/rdd/issue-283-work-plan.md) 依存: #288 (PR 1: schema) ## 変更内容 ### query 層 (`app/services/github-integration-queries.server.ts`) - `getGithubAppLinks(orgId)` 配列返却(ORDER BY createdAt ASC で決定的順序) - `getGithubAppLinkByInstallationId(installationId)` 追加 - `assertInstallationBelongsToOrg(orgId, installationId)` 追加 — クライアント由来の `installationId` をサーバ側で検証する境界 guard - `getGithubAppLink()` は最古の active link を返す互換 shim(`@deprecated`) ### mutation 層 (`app/services/github-app-mutations.server.ts`) - `disconnectGithubAppLink(orgId, installationId)` 追加 — 単一 installation の soft-delete + 最後の active を失った時のみ `method='token'` に戻す + audit log 書き込み - 1 transaction 内で完結(idempotent) - `disconnectGithubApp()` は legacy UI 互換 wrapper として 1 transaction で全 link 一括 soft-delete に書き換え(`@deprecated`) ### audit log writer (`app/services/github-app-link-events.server.ts`) 新規追加 - `logGithubAppLinkEvent()` helper - event_type / source / status の string union 型を export - `Kysely<DB> | Transaction<DB>` を受け取り、呼び出し側のトランザクションに乗せられる - PR 1 で追加した `github_app_link_events` table の **初回 writer**(disconnect 経由) ### Octokit 解決 (`app/services/github-octokit.server.ts`) - `resolveOctokitForInstallation(installationId)` 追加 - `resolveOctokitForRepository({ integration, githubAppLinks, repository })` 追加 — repository ごとの解決 - `repository.githubInstallationId` がセットされている場合は厳密にそれを使う(suspended は弾く) - `github_app` モードで未割当の repository に対する移行期間 fallback: - active link 1 件 → そのまま使う - 0 件 → エラー(**PAT 自動 fallback はしない**、RDD ルール) - 2+ 件 → エラー(曖昧、明示的な assignment が必要) - `token` モード: `privateToken` があれば PAT、無ければ未接続エラー - `IntegrationForOctokit.method` を `'token' | 'github_app' | (string & {})` のユニオンに型を絞る - `resolveOctokitFromOrg()` は legacy 互換 wrapper(`@deprecated`、PR 4 で削除予定) ### batch shape 更新 (`batch/db/queries.ts`) - `getGithubAppLinkByOrgId` → `getGithubAppLinksByOrgId`(配列返却) - `getAllGithubAppLinks` を `Map.groupBy` で書き換え - `getOrganization()` / `listAllOrganizations()` が `githubAppLinks: []` を返すよう変更 ### crawl / backfill ジョブ (`app/services/jobs/{crawl,backfill}.server.ts`) - 単一 Octokit 共有から per-repository 解決に変更 - `load-organization` step 内で `github_app + active 0` / `token + privateToken null` の早期エラー検出 - repository ループ内で `resolveOctokitForRepository()` を呼び、解決失敗時は warn ログ + skip(crawl 全体は止めない) ### tsconfig - `lib` を `ES2024` に bump(`Map.groupBy` を使うため) ### tests - `app/services/github-octokit.server.test.ts` に `resolveOctokitForRepository` の 11 ケース追加 - 明示 installation id (一致 / 不一致 / suspended) - 移行期間 fallback (active 1 / 0 / 2+ / suspended 除外) - token モード (PAT あり / なし) - integration null ## 満たす受入条件 - **#6**: `crawl.server.ts` / `backfill.server.ts` が repository ごとに対応 installation の Octokit を使う ## Stack 位置 ```text PR 1 (#288): schema └ [PR 2: query/octokit] ← this PR └ PR 3 (webhook/membership) └ PR 4 (UI) └ PR 5 (repo UI) └ PR 6 (backfill) └ PR 7 (strict) ``` ## 後続 PR への影響 - PR 3: webhook handler 群がここで追加した `getGithubAppLinkByInstallationId` / `logGithubAppLinkEvent` / canonical reassignment helper(PR 3 で実装)を使う - PR 4: UI loader / action から `getGithubAppLink()` を `getGithubAppLinks()` に移行 + `assertInstallationBelongsToOrg` を loader 境界で呼ぶ - PR 7: 移行期間 fallback (`activeLinks.length === 1` 分岐) を削除し、`github_installation_id IS NULL` を strict エラーにする ## テスト - [x] \`pnpm validate\` (lint / format / typecheck / build / test 全 331 tests) - [x] \`resolveOctokitForRepository\` の主要 11 ケースをユニットテストで検証 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## リリースノート * **新機能** * GitHub Appのリンク接続・切断イベントに対する監査ログ機能を追加しました。 * **バグ修正** * リポジトリごとのGitHub認証解決ロジックを改善し、複数のアクティブなリンク存在時のエラーハンドリングを強化しました。 * GitHub Appの削除状態をより厳密に追跡するようにしました。 * **改善** * GitHub統合に関する検証とエラー処理を堅牢化しました。 <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…rship initialization (#283 PR 3/7) (#290) ## Summary Issue #283 の実装 stack **PR 3/7** — webhook handler / canonical reassignment / membership 初期投入の中核実装。stack で最重要 PR。 設計根拠: [`docs/rdd/issue-283-multiple-github-accounts.md`](./docs/rdd/issue-283-multiple-github-accounts.md) 作業計画: [`docs/rdd/issue-283-work-plan.md`](./docs/rdd/issue-283-work-plan.md) 依存: #288 (PR 1: schema), #296 (PR 2: query/octokit) ## 変更内容 ### setup callback (`app/routes/api.github.setup.ts`) - `(organizationId, installationId)` 単位 upsert(複合主キー対応) - `github_account_type` を保存(personal / Organization の UI 分岐用) - membership 初期投入: `fetchInstallationRepositories` → `initializeMembershipsForInstallation` → `membership_initialized_at = now` - GitHub API 失敗時は link のみ保存し、`membership_initialized_at IS NULL` のまま auto-repair に委譲 - audit log: `link_created` / `membership_initialized` (success / failed) ### installation webhook (`app/services/github-webhook-installation.server.ts`) - `findActiveLinkByInstallationOrAccount` 削除 - すべての lookup を `installation_id` で行う - `installation.deleted`: - 該当 link のみ soft-delete - 最後の active link を失った時のみ `integrations.method = 'token'` に戻す - `link_deleted` 監査ログ - tenant 側で canonical reassignment を呼ぶ - `installation.suspend` / `unsuspend`: `github_app_links.suspended_at` を更新(旧 `integrations.app_suspended_at` から移行) - `installation_repositories.added/removed`: - membership upsert / soft-delete - canonical reassignment 呼び出し(removed 時) - bulk owner/repo 解決(1 query で N+1 解消) - `installation.created`: setup callback が正本のため、既存 link が無ければ no-op ### canonical reassignment (`app/services/github-app-membership.server.ts`) 新規 - `reassignCanonicalAfterLinkLoss(orgId, lostInstallationId, source)`: - tenant DB の `repository_installation_memberships` を正本とする - 候補は active / non-suspended / `membership_initialized_at IS NOT NULL` の link のみ - 候補数で判定: - 1 → 自動 reassign + `canonical_reassigned` - 0 → null + `canonical_cleared` (or `assignment_required` if 未初期化 link 残存) - 2+ → null + `assignment_required` - **未初期化 link ガード**: 未初期化 link が残っている org では、候補 0 でも `canonical_cleared` ではなく `assignment_required` に倒す - LEFT JOIN + bulk update で N+1 を回避 - tenant first / shared second の cross-store 順序 - `upsertRepositoryMembership` / `softDeleteRepositoryMembership` / `initializeMembershipsForInstallation` helpers ### installation repos fetcher (`app/services/github-installation-repos.server.ts`) 新規 - `fetchInstallationRepositories(installationId)`: GitHub API でその installation が見える repository を全ページ取得 ### audit log writer (`app/services/github-app-link-events.server.ts`) - `tryLogGithubAppLinkEvent` best-effort wrapper を追加(呼び出し側の `.catch(() => {})` ノイズを排除) ### auto repair (`app/services/jobs/crawl.server.ts`) - crawl 冒頭に独立 step `repair-membership:<installation_id>` を追加 - `membership_initialized_at IS NULL` の active link を検出 → `installation_repositories` を再 fetch → membership upsert → `membership_initialized_at = now` - per-link で独立 step、durably の中断・再開性を維持 - 失敗時は次回 crawl で再試行(idempotent) ### PR webhook (`app/services/github-webhook-pull.server.ts`) - `owner + repo + installation_id` で repository を引く - 移行期間中は `github_installation_id IS NULL` の repository も許可(PR 7 で strict 化) ### tests - **`app/services/github-app-membership.server.test.ts`** 新規 (8 ケース): - 1 候補 → reassign + `canonical_reassigned` - 0 候補 → null + `canonical_cleared` - 2+ 候補 → null + `assignment_required` - 未初期化 link 残存 + 候補 0 → `assignment_required` (cleared じゃない) - suspended link は除外 - 未初期化 link は除外 - soft-deleted membership は除外 - idempotency: 2 回実行しても結果が同じ - **`app/services/github-webhook.server.test.ts`** 既存 12 ケース更新: - 新 schema (`suspended_at`, `membership_initialized_at`, `github_account_type`, `github_app_link_events`) 対応 - tenant DB mock を chain proxy に変更 ## 満たす受入条件 - **#8**: `installation.suspend/unsuspend` が対象 installation row のみ更新 - **#9**: `installation_repositories` が対象 installation のみ更新 - **#10**: `installation.deleted` が対象 installation row のみ `deleted_at` セット - **#11**: 最後の active 切断時のみ method=token + private_token 有無で復帰先を分岐 - **#12**: canonical reassignment が候補 1 件で自動、0/複数で `null` + manual reselect - **#19**: cross-store 更新は tenant first / shared second + audit log - **#22**: setup callback で `membership_initialized_at` をセット、失敗時は repair に委譲 - **#23**: 未初期化 link 残存時は assignment_required に倒れる ## Stack 位置 \`\`\`text PR 1 (#288): schema └ PR 2 (#296): query/octokit └ [PR 3: webhook/membership] ← this PR └ PR 4 (UI) └ PR 5 (repo UI) └ PR 6 (backfill) └ PR 7 (strict) \`\`\` ## 後続 PR への影響 - **PR 4**: integration settings UI が `getGithubAppLinks()` に切替 + installation selector 追加 + `assertInstallationBelongsToOrg` を loader 境界で呼ぶ - **PR 5**: repository list/detail で `assignment required` バッジ + 個別再選択 mutation(同 helper を再利用) - **PR 7**: PR webhook の `OR github_installation_id IS NULL` 削除 + crawl/backfill の移行期間 fallback 削除 ## テスト - [x] \`pnpm validate\` (lint / format / typecheck / build / test 全 339 tests) - [x] canonical reassignment helper を 8 ケースのユニットテストでカバー (cross-store 整合性 / idempotency / 候補 0/1/2+ / 未初期化ガード / suspended 除外 / soft-deleted 除外) - [x] webhook integration test (12 ケース) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## リリースノート * **新機能** * GitHub Appインストール時にリポジトリメンバーシップを自動初期化・同期する仕組みを追加 * メンバーシップ情報に基づくリポジトリの正規割り当て(再割り当て)処理を導入 * インストールのリポジトリ取得機能を追加 * **バグ修正** * インストール/サスペンド/削除イベント処理のスコープと整合性を強化 * 監査ログ書き込み失敗を影響させない安全措置を追加 * **テスト** * 再割り当て挙動の包括的なテストスイートを追加 <!-- end of auto-generated comment: release notes by coderabbit.ai -->

Summary
Issue #283 の実装 stack PR 1/7 — schema と型生成のみ。アプリケーションコードからの新カラム参照は PR 2 以降で入れる。
設計根拠:
docs/rdd/issue-283-multiple-github-accounts.md作業計画:
docs/rdd/issue-283-work-plan.md変更内容
shared DB (
db/shared.sql)github_app_links:(organization_id)から(organization_id, installation_id)に変更 → 1 org に複数 installation を保持可能にUNIQUE (github_account_id)を削除し、non-uniqueINDEXに変更 → 同一 GitHub account を複数 Upflow org で使えるようにsuspended_at TEXT NULL追加 → installation 単位の suspend 状態を持つ (現状 org 単位 1 値のintegrations.app_suspended_atから PR 7 で移行予定)github_account_type TEXT NULL追加 → personal account / organization の UI 分岐用membership_initialized_at TEXT NULL追加 → canonical reassignment の安全ガード用github_app_link_events新規追加 → cross-store 更新の監査ログ。writer は PR 2 / PR 3 で追加されるtenant DB (
db/tenant.sql)repositories.github_installation_id INTEGER NULL追加 → repository ごとにどの installation で fetch するかを保持repository_installation_memberships新規追加 → installation ↔ repository の所属関係を永続化。canonical reassignment 判定の正本として使う型生成・ごく僅かな追従
app/services/type.ts,app/services/tenant-type.ts: kysely-codegen 再生成batch/db/queries.ts:getTenantData()の repositories SELECT にgithubInstallationIdを追加 (型エラー解消のための最小変更)満たす受入条件
github_app_linksは同一organization_idに複数 row を保持可能、installation_idunique、github_account_idnon-uniquerepositoriesがgithub_installation_idを保持Stack 位置
注意点
github_app_linksの migration は table 再作成方式 (PK 変更のため)。既存 row はorganization_id,installation_id,github_account_id,github_org,app_repository_selection,deleted_at,created_at,updated_atをコピー。新カラムはNULLで初期化されるgithub_app_link_eventsの writer は PR 2 (disconnectGithubAppLink) / PR 3 (canonical reassignment / repair / webhook) で追加される。本 PR 時点では未使用 table が main に入ることを許容DROP TABLEが含まれるが、直前のINSERT SELECT成功前提の Atlas 生成パターンテスト
pnpm validate(lint / format / typecheck / build / test 全 320 tests)pnpm db:setupで seed 含めて再現可能🤖 Generated with Claude Code
Summary by CodeRabbit
リリースノート