Repository navigation
fix(github): connect existing app installations - #2000
jasonkuhrt wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Discovery contracts and persistence packages/domain/src/http/vcs.ts, packages/domain/src/http/integrations.ts, packages/domain/src/http/oauth-return.ts, packages/db/src/tables/*, packages/db/drizzle/20261030224401_github_installation_selection/migration.sql, packages/backend/src/services/integrations/vcs/VcsRepository.ts |
Adds schemas for installation choices and repository-discovery states. Persists eligible installation IDs and discovery state. Discovery updates only apply to the matching pending request. |
Existing installation authorization and selection packages/backend/src/services/integrations/vcs/vendor/github/GithubConnectService.ts, packages/api/src/routes/internal/integrations.http.ts, packages/api/src/routes/oauth-complete.ts, packages/api/src/routes/integrations-callback.http.ts, related route and service tests |
Adds existing-installation OAuth, eligible-installation discovery, and selection with organization-ownership checks. The API returns installation choices and accepts a selected installation. |
Repository discovery job lifecycle packages/backend/src/services/integrations/vcs/VcsSyncService.ts, packages/backend/src/services/integrations/vcs/__tests__/vcs.test.ts |
Installation-sync jobs mark discovery complete after listing repositories. They mark it failed when listing is blocked, the installation is gone, or retries are exhausted. |
Browser callback and installation selection apps/web/src/page/integrations/connect.ts, apps/web/src/page/integrations/oauth-return.ts, apps/web/src/page/integrations/message.ts, apps/web/src/page/integrations/subscription.ts, apps/web/src/page/integrations/update*.ts, apps/web/src/page/integrations/github.ts, apps/web/src/platform/oauth-return-stash.test.ts, apps/web/src/page/integrations/connect.test.ts, apps/web/src/page/sources/new/update.ts |
Carries the connection mode and installation selection through OAuth callbacks. Adds web commands and model updates for selecting an installation and handling its result. |
Discovery status and recovery interface apps/web/src/page/integrations/flow-github.ts, apps/web/src/page/integrations/view-github*.ts, apps/web/src/page/integrations/view-flow-github.ts, apps/web/src/page/integrations/github*.scene.test.ts, apps/web/src/page/integrations/subscription.test.ts, apps/web/src/page/errors/issue/attach.test.ts, apps/web/src/page/code-review/kit/test-fixtures.ts, apps/web/src/lab/security-fixture.ts, apps/landing/src/content/docs/integrations/github.md, docs/github-app-setup.md |
Displays installation choices and discovery progress, empty results, and failures. Polls while discovery or repository sync is pending, and provides an existing-installation connection action for recovery. |
Estimated code review effort: 4 (Complex) | ~60 minutes
Change: Feature · Severity of issue fixed: Medium
Sequence Diagram(s)
sequenceDiagram
participant WebIntegrationPage
participant InternalIntegrationsRoute
participant GithubConnectService
participant GitHub
participant OAuthCompleteRoute
participant VcsRepository
participant VcsSyncQueue
WebIntegrationPage->>InternalIntegrationsRoute: POST connect request with existing mode
InternalIntegrationsRoute->>GithubConnectService: startConnect
GithubConnectService-->>WebIntegrationPage: GitHub OAuth redirect URL
WebIntegrationPage->>GitHub: authorize account
GitHub->>OAuthCompleteRoute: callback with code and state
OAuthCompleteRoute->>GithubConnectService: discoverInstallations
GithubConnectService->>GitHub: exchange code and list accessible installations
GithubConnectService-->>OAuthCompleteRoute: eligible installation selection
OAuthCompleteRoute-->>WebIntegrationPage: return installation selection
WebIntegrationPage->>InternalIntegrationsRoute: POST selected installation and state
InternalIntegrationsRoute->>GithubConnectService: selectInstallation
GithubConnectService->>VcsRepository: persist installation and pending discovery
GithubConnectService->>VcsSyncQueue: enqueue installation sync
Possibly related issues
- Already-installed GitHub app leaves Sources waiting for a callback — the PR adds authenticated discovery and selection for GitHub installations created directly on GitHub.
Suggested labels: preview:web
Suggested reviewers: makisuo
Merge Risk: 
3ae2d
-
The change adds a recovery flow for connecting an existing GitHub App installation, with discovery status shown in the UI. No actionable merge-blocking issue was found in this review, so normal pre-merge checks and CI are sufficient.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: adding support to connect existing GitHub App installations. Linked Issues check [ #1998] requires an authenticated way to discover and connect an existing GitHub App installation to the current Maple organization, while keeping ownership checks and repository access. Evidence at t…Out of Scope Changes check Changes map to [ #1998]. Backend: existing-installation connect, discovery, selection, ownership helper, and VcsSyncService discovery settlement. Data: the two nullable JSONB columns and migration. Dom…Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Full details: Linked Issues check
-
Explanation
[
#1998] requires an authenticated way to discover and connect an existing GitHub App installation to the current Maple organization, while keeping ownership checks and repository access. Evidence at the reviewed head: (1) GithubConnectService.startConnect accepts mode "existing". It requires GITHUB_APP_CLIENT_ID, stores single-use state under provider "github-existing", and binds that state to orgId and initiatedByUserId. (2) The ownedInstallation helper keeps the cross-org refusal ("installation_cross_org_rejected"). (3) bindInstallation is the shared connect path. It upserts the installation, sets repositoryDiscovery to Pending with a requestId, and queues an installation-sync job with that requestId. (4) The migration adds nullable github_installation_ids to oauth_auth_states and repository_discovery to vcs_installations. Per the change summary, discoverInstallations exchanges the OAuth code, lists administered installations, and excludes installations owned by another Maple organization. selectInstallation consumes the state, rejects IDs outside the stored selection, checks ownership, and connects. The route, domain schema, and web picker are present per the summary. Limit: the file view of GithubConnectService.ts was truncated before the discoverInstallations and selectInstallation bodies. Their behavior rests on the change summary and the reported runtime regressions, not on a direct read of those bodies.
Full details: Out of Scope Changes check
-
Explanation
Changes map to [
#1998]. Backend: existing-installation connect, discovery, selection, ownership helper, and VcsSyncService discovery settlement. Data: the two nullable JSONB columns and migration. Domain: repository-discovery types, select request and response, and OAuth selection schema. API: /github/select and discovery in oauth-complete. Web: the installation picker, discovery pending and failed states, and selection flow. Docs: setup guidance and callback recovery text. Tests: coverage for these paths. The VcsSyncService change to settle exhausted retries for installation-sync requests supports the discovery requirement. Non-installation, non-commit job handling is kept as before. The SecureRandom and Base64URL state generation applies to the single-use state that the new flow depends on. Test-only reformatting in attach.test.ts and github.scene.test.ts is incidental. The change summary states no behavior change there. The large generated schema snapshot named in the PR description is not in the change summary and was not inspected. It is generator output for the migration and does not indicate unrelated work.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 35 files. (3 skipped: 3 unsupported.)
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
Maple review🟢 Confidence 9/10 · safe to merge. OAuth and cross-org installation ownership change, but single-use state binding, admin proof and stale-job rejection are each pinned by backend tests. Adds a "Connect existing installation" path: GitHub user OAuth discovers administered installations, the user picks one, and a
Before merge
Production impactOpen errors in the changed files (1)
After this merges, Maple checks whether they stop.
|
An app installed directly on GitHub has no Maple organization binding. The Sources flow currently sends users back to installation, where an already-installed account opens Configure and Maple waits indefinitely for a return.
Add Connect existing installation to GitHub source setup. GitHub user authorization discovers installations the user administers, then the user explicitly chooses the account to connect. The existing repository selection is retained.
The selection uses short-lived, single-use state bound to the initiating Maple user and organization. A nullable JSONB column on
oauth_auth_statesstores only verified installation IDs; the GitHub user token stays in the authorization request. Cross-organization ownership checks remain in place, and installation webhooks cannot invent a Maple tenant.A second nullable JSONB column tracks repository discovery on the installation. The page follows the sync worker's completion or failure for the current request, so an initial empty response or a GitHub rate-limit retry does not leave it stuck. An older job cannot acknowledge a newer request. The callback guidance and setup docs point already-installed users to the recovery flow.
Local validation passes: 164 backend/VCS tests, 60 UI tests, 17 callback/page tests, database migration parity, and scoped lint/formatting. The regressions include an actual GitHub-client 429 with
Retry-After: 20, stale status-read interleavings, one-use ownership proofs, and retrying authorization from Go live. Independent review findings are addressed.SDK/Alchemy/email builds and API, backend, domain, database and web production/e2e typechecks pass. Local Chrome verification passed on the actual Sources setup view using fixture accounts: the existing-installation recovery action, explicit organization/personal account choices, and disabled controls during authorization and selection. OAuth requests and callbacks are exercised by the runtime regressions above. Upstream CI currently requires maintainer approval before any workflow jobs run. The large added JSON file is the migration generator's schema snapshot.
Closes #1998.
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by CodeRabbit
New Features
Bug Fixes
Documentation