Skip to content

feat(github-app): strict installation lookup, drop transitional fallbacks (#283 PR 7/7)#294

Open
coji wants to merge 1 commit intofeat/issue-283-backfillfrom
feat/issue-283-strict
Open

feat(github-app): strict installation lookup, drop transitional fallbacks (#283 PR 7/7)#294
coji wants to merge 1 commit intofeat/issue-283-backfillfrom
feat/issue-283-strict

Conversation

@coji
Copy link
Copy Markdown
Owner

@coji coji commented Apr 8, 2026

Summary

Issue #283 stack の 最終 PR (7/7) — 移行期間 fallback を全削除し、strict installation lookup に切替。これでシリーズ完結。

設計根拠: `docs/rdd/issue-283-multiple-github-accounts.md`
作業計画: `docs/rdd/issue-283-work-plan.md`

依存: #288, #296, #290, #291, #292, #293

⚠️ デプロイ順序

このマージは PR 6 (#293) の `backfill-installation-membership` を本番実行 + 検証完了後 にのみ実行してください。順序を守らないと既存リポジトリが strict lookup で弾かれてエラーになります。

詳細手順は #293 の Runbook 参照。

変更内容

schema (`db/shared.sql` + migration)

  • `integrations.app_suspended_at` カラム削除(PR 1 で `github_app_links.suspended_at` に移行済み)
  • Atlas が table-rebuild migration を生成 (`20260408001949.sql`)
  • `app/services/type.ts` 再生成

Octokit 解決 (`app/services/github-octokit.server.ts`)

  • `resolveOctokitForRepository` 内の transitional fallback を全削除:
    • `github_app + githubInstallationId IS NULL` → エラー "Repository has no canonical installation assigned. Run reassign-broken-repositories or reinstall."
    • "active link 1 件で代用" のロジック削除
  • 削除した legacy export:
    • `assertOrgGithubAuthResolvable`
    • `resolveOctokitFromOrg`
    • `OrgGithubAuthInput`

query 層 (`app/services/github-integration-queries.server.ts`)

  • `getGithubAppLink` (deprecated) を削除(caller ゼロ)

PR webhook (`app/services/github-webhook-pull.server.ts`)

  • 移行期間の `OR github_installation_id IS NULL` を削除し strict 化:
    ```sql
    WHERE owner = ? AND repo = ? AND github_installation_id = ?
    ```
  • broken state webhook を ops が検知できるよう、各 silent return に `debug()` ログを追加(`createDebug('app:github-webhook:pull')`)

route (`app/routes/$orgSlug/settings/repositories/$repository/$pull/`)

  • `index.tsx`: `resolveOctokitFromOrg` から `resolveOctokitForRepository` に migration
  • `queries.server.ts`: `getRepository` の select に `githubInstallationId` 追加

cleanup

  • `appSuspendedAt` への参照を全削除 (setup callback / webhook handler / mutation / batch query / test fixture)
  • `github-octokit.server.test.ts` から legacy 関数のテストと transitional fallback ケースを削除

満たす受入条件

デプロイ Rollback 戦略

  • migration は destructive (column drop)。本番デプロイ前に staging で apply → rollback → re-apply のリハーサル必須
  • デプロイ後 24 時間は `github_app_link_events` の異常パターン (連続 `canonical_reassigned`, `webhook_dropped` debug ログの急増) を監視
  • 異常検知時:
    1. PR 7 を revert
    2. `app_suspended_at` を復活させる down migration を当てる (Atlas で生成済み)
    3. 必要なら `reassign-broken-repositories` で個別 repo 救済

Stack 位置 (完成形)

```text
PR 1 (#288): schema
└ PR 2 (#296): query/octokit
└ PR 3 (#290): webhook/membership
└ PR 4 (#291): UI
└ PR 5 (#292): repo UI
└ PR 6 (#293): backfill
└ [PR 7: strict] ← this PR
```

テスト

  • `pnpm validate` (lint / format / typecheck / build / test 全 342 tests)
  • `resolveOctokitForRepository` の strict path テスト (canonical なし → throw)
  • migration `pnpm db:setup` で再現可能

🤖 Generated with Claude Code

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 8, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f960ed49-634f-447d-b20c-fe43c21c1f08

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/issue-283-strict

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.

❤️ Share

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

Copy link
Copy Markdown
Owner Author

coji commented Apr 8, 2026

@coji
Copy link
Copy Markdown
Owner Author

coji commented Apr 8, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 8, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coji coji marked this pull request as ready for review April 8, 2026 00:49
@coji
Copy link
Copy Markdown
Owner Author

coji commented Apr 8, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 8, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coji coji force-pushed the feat/issue-283-backfill branch from 8d629f8 to 95f0e0c Compare April 8, 2026 10:43
@coji coji force-pushed the feat/issue-283-strict branch from 6271744 to 7e96004 Compare April 8, 2026 10:44
@coji coji force-pushed the feat/issue-283-backfill branch from 95f0e0c to 44a5cda Compare April 8, 2026 11:04
@coji coji force-pushed the feat/issue-283-strict branch from 7e96004 to 6f00bea Compare April 8, 2026 11:04
@coji coji force-pushed the feat/issue-283-backfill branch from 44a5cda to aa36656 Compare April 8, 2026 13:21
@coji coji force-pushed the feat/issue-283-strict branch 2 times, most recently from d6b266b to 3365fac Compare April 8, 2026 14:21
@coji coji force-pushed the feat/issue-283-backfill branch from aa36656 to 3c83597 Compare April 8, 2026 14:22
@coji coji force-pushed the feat/issue-283-strict branch from 3365fac to 2e37380 Compare April 8, 2026 14:50
@coji coji force-pushed the feat/issue-283-backfill branch from 3c83597 to ea800f0 Compare April 8, 2026 14:50
@coji coji force-pushed the feat/issue-283-strict branch from 2e37380 to 39aaa36 Compare April 8, 2026 15:15
@coji coji force-pushed the feat/issue-283-backfill branch 2 times, most recently from 18a9df2 to 9ae43b8 Compare April 8, 2026 15:30
@coji coji force-pushed the feat/issue-283-strict branch 2 times, most recently from 85781db to 8d782a7 Compare April 8, 2026 15:32
@coji coji force-pushed the feat/issue-283-backfill branch 2 times, most recently from 1e0c6e8 to d0da8d7 Compare April 8, 2026 15:43
@coji coji force-pushed the feat/issue-283-strict branch from 8d782a7 to 81a57e7 Compare April 8, 2026 15:44
…acks (#283 PR 7/7)

Final PR in the multi-installation stack. After this lands the transitional
"active link 1 件" / "github_installation_id IS NULL OK" paths are gone and
every github_app repository must have a canonical installation assigned.

shared schema:
- db/shared.sql: drop integrations.app_suspended_at column (suspend state moved to github_app_links.suspended_at in PR 1)
- db/migrations/shared/20260408001949.sql: Atlas migration via table-rebuild
- regenerate app/services/type.ts

octokit resolution (app/services/github-octokit.server.ts):
- resolveOctokitForRepository: github_app + github_installation_id IS NULL now throws "Repository has no canonical installation assigned. Run reassign-broken-repositories or reinstall."
- delete deprecated resolveOctokitFromOrg / assertOrgGithubAuthResolvable / OrgGithubAuthInput

PR webhook lookup (github-webhook-pull.server.ts):
- drop the OR clause; require strict (owner, repo, installation_id) match
- broken state repos are silently skipped until operator runs reassign-broken-repositories

UI route migration:
- repositories/$repository/$pull/index.tsx: switch from resolveOctokitFromOrg to resolveOctokitForRepository (uses repository.githubInstallationId)
- queries.server.ts: select githubInstallationId

cleanup:
- remove appSuspendedAt references from setup callback, webhook handlers, mutations, batch queries
- update test fixtures (drop app_suspended_at column from in-memory SQLite schemas)
- update github-octokit.server.test.ts to drop legacy assertOrgGithubAuthResolvable / resolveOctokitFromOrg tests and the transitional fallback cases of resolveOctokitForRepository

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coji coji force-pushed the feat/issue-283-backfill branch from d0da8d7 to e6a4166 Compare April 8, 2026 15:57
@coji coji force-pushed the feat/issue-283-strict branch from 81a57e7 to 4e72683 Compare April 8, 2026 15:57
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