Skip to content

feat(github): share installations with web Cloud Agents (2/4) - #5978

Open
pandemicsyn wants to merge 28 commits into
mainfrom
feat/github-connections-pr2
Open

feat(github): share installations with web Cloud Agents (2/4)#5978
pandemicsyn wants to merge 28 commits into
mainfrom
feat/github-connections-pr2

Conversation

@pandemicsyn

@pandemicsyn pandemicsyn commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Outcome

Default-off shared canonical GitHub installations for WEB Cloud Agents with exact tenant-association pinning. Slack shared use remains PR3; Code Reviewer shared routing remains PR4.

Reviewed base: current main at cf59d8662499749228164232615da5af0abe5134. Reviewed head: af1734d3ce0b8d11acad16622754be8618603cdb.

Exact Association Boundaries

Generic token and Chat SDK webhook paths require canonical sharing_mode = exclusive. Managed shared lookup requires an exact active association ID. GitHub bot-link signed state carries exact platformIntegrationId; callback validates association, installation, Standard/Lite identity, canonical lifecycle, local health, and current Kilo owner or organization membership.

Predeploy bot-link state without an association ID is accepted only while management and sharing admission remain disabled and only for one healthy canonical-exclusive or true-legacy literal-null app association. Explicit Standard rows require exact signed association identity. Keep features disabled for at least one signed-state TTL after deployment.

Simple Synchronous Lifecycle

Required canonical/local database cleanup runs first and fails non-2xx without recording completion. Standard bot unlink runs second as best-effort alerting. Completed-success receipts dedupe later deliveries; concurrent and crash-window repeats are idempotent. GitHub does not automatically retry failed webhooks, so manual redelivery after database failure is the accepted recovery. No lifecycle queue, cron, outbox, lease, fencing, Retry-After, or automatic retry worker exists.

Repository refresh follows lifecycle lock order and cannot revive terminal state.

Provider Reservation Boundary

Slack, Linear, and Discord OAuth attempts are durable, expiring, owner-row locked, purpose-bound, and single-use. PR3 owns provider-installation generation and lazy recovery.

Migration And Rollout

Generated migration: 0243_exotic_triton.sql. Replacement indexes build concurrently with required transaction boundaries. Before enabling sharing, stale eligible-unlinked must be zero. PR2 adds no canonical reconciliation DML.

Verification

  • Accepted pre-rebase bot-link/state/DB resolver suite: 99 passed
  • Accepted pre-rebase schema/migration suite: 101 passed; fresh bootstrap passed
  • Final-rebase Git token service: 633 passed
  • Final-rebase focused Cloud Agent suites: 429 passed
  • Final-rebase range-diff: all 28 PR2 commits exact
  • Changes-only typecheck and web/DB/token/Cloud Agent lint: passed
  • Format, whitespace, and conflict-marker checks: passed

The final-rebase PostgreSQL-backed Jest/bootstrap rerun was blocked before test execution because the local OrbStack Docker daemon was unresponsive; docker info, Compose health, and pnpm test:db timed out. This was not an assertion failure. The accepted pre-rebase head had the schema/bootstrap and focused web suites passing.

No production queries, repairs, associations, settings, flags, backfills, deployments, or external provider operations were performed.

Base automatically changed from feat/github-connections-pr1 to main September 8, 2026 21:08
@pandemicsyn
pandemicsyn force-pushed the feat/github-connections-pr2 branch 2 times, most recently from fa6817f to d70becc Compare September 9, 2026 16:36
@pandemicsyn
pandemicsyn marked this pull request as ready for review September 9, 2026 17:39
@kilo-code-bot

kilo-code-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

findGitHubBotLinkIntegrations applies .limit(2) before its JS candidate filter, so the bot-link "exactly one healthy association" ambiguity guard can fail open when three or more associations share an installation.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/web/src/lib/integrations/db/platform-integrations.ts 96 .limit(2) runs before the JS usability filter, so with three or more matching associations the arbitrary pair (no ORDER BY) can contain exactly one usable row and usableCandidates.length === 1 accepts an ambiguous bot link instead of failing closed. Move the candidate filters into SQL or drop the limit and count in JS.
Files Reviewed (5 files)
  • apps/web/src/lib/integrations/db/platform-integrations.ts - 1 issue
  • apps/web/src/lib/bot/github-link-state.ts
  • apps/web/src/app/api/integrations/github/callback/route.ts
  • apps/web/src/app/github/link/route.ts
  • apps/web/src/lib/integrations/github/multiple-installations.ts

Fix this issue in Kilo Cloud

Inline publication was blocked: this PR has an existing bot pending review (id 5158007287) that this session is not permitted to clear, so the finding is recorded here.

Previous Review Summaries (10 snapshots, latest commit fda919a)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit fda919a)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

findGitHubBotLinkIntegrations applies .limit(2) before its JS candidate filter, so the bot-link "exactly one healthy association" ambiguity guard can fail open when three or more associations share an installation.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/web/src/lib/integrations/db/platform-integrations.ts 96 .limit(2) runs before the JS usability filter, so an arbitrary pair (no ORDER BY) can contain exactly one usable row and the callback's usableCandidates.length === 1 guard accepts even though multiple healthy associations exist. Move the candidate filters into SQL or drop the limit and count in JS.
Files Reviewed (9 files)
  • apps/web/src/app/api/integrations/github/callback/route.ts
  • apps/web/src/app/api/integrations/github/callback/route.test.ts
  • apps/web/src/app/github/link/route.ts
  • apps/web/src/app/github/link/route.test.ts
  • apps/web/src/lib/bot/github-link-state.ts
  • apps/web/src/lib/bot/github-link-state.test.ts
  • apps/web/src/lib/integrations/db/platform-integrations.ts - 1 issue
  • apps/web/src/lib/integrations/db/platform-integrations.test.ts
  • apps/web/src/lib/integrations/github/multiple-installations.ts

Fix this issue in Kilo Cloud

Inline publication was blocked: this PR has an existing bot pending review (id 5158007287) that this session is not permitted to clear, so the finding is recorded here.

Previous review (commit 4c3d38b)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Rebase onto current main left GitHub-installation review scope unchanged; overlapping files differ only by already-landed main content, with high confidence.

Files Reviewed (3 files)
  • ENVIRONMENT.md
  • apps/web/src/lib/integrations/db/platform-integrations.test.ts
  • apps/web/src/lib/integrations/db/platform-integrations.ts

Previous review (commit 98af34e)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Lock-observation tests now match the exact advisory-lock SQL already used by sibling suites; the single-file incremental change looks correct with high confidence.

Files Reviewed (1 file)
  • apps/web/src/lib/integrations/db/github-installations.test.ts

Previous review (commit fbc73e0)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Lifecycle lock-overlap tests now wait for a blocked owner advisory lock before releasing the holder; the single-file incremental change looks correct with high confidence.

Files Reviewed (1 file)
  • apps/web/src/lib/integrations/db/github-installations.test.ts

Previous review (commit f0df914)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Repository refresh now follows the lifecycle lock order and skips deleted or inactive projections; the two-file incremental change looks correct with high confidence.

Files Reviewed (2 files)
  • apps/web/src/lib/integrations/db/github-installations.test.ts
  • apps/web/src/lib/integrations/db/platform-integrations.ts

Previous review (commit 622241e)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Synchronous GitHub lifecycle cleanup now records completed receipts only after required DB work; the incremental simplification looks correct with high confidence.

Files Reviewed (8 files)
  • apps/web/src/lib/integrations/db/github-installations.test.ts
  • apps/web/src/lib/integrations/db/github-installations.ts
  • apps/web/src/lib/integrations/platforms/github/webhook-handler.test.ts
  • apps/web/src/lib/integrations/platforms/github/webhook-handler.ts
  • apps/web/src/lib/integrations/platforms/github/webhook-handlers/installation-handler.test.ts
  • apps/web/src/lib/integrations/platforms/github/webhook-handlers/installation-handler.ts
  • packages/db/src/migrations/0243_exotic_triton.sql
  • packages/db/src/schema.ts

Previous review (commit cd4e7d7)

Status: No Issues Found | Recommendation: Merge

Executive Summary

In-lease GitHub lifecycle receipts now return retryable 503 with Retry-After; the five-file incremental change looks correct with high confidence.

Files Reviewed (5 files)
  • ENVIRONMENT.md
  • apps/web/src/lib/integrations/db/github-installations.test.ts
  • apps/web/src/lib/integrations/db/github-installations.ts
  • apps/web/src/lib/integrations/platforms/github/webhook-handler.test.ts
  • apps/web/src/lib/integrations/platforms/github/webhook-handler.ts

Previous review (commit f1bbbb4)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

Failed GitHub installation.deleted retries can still be acknowledged as duplicates while a pending receipt lease is held, so unlink/cleanup is never reclaimed.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0

Fix these issues in Kilo Cloud

Issue Details (click to expand)

WARNING

File Line Issue
apps/web/src/lib/integrations/platforms/github/webhook-handler.ts 184 In-lease pending receipts return HTTP 200 Duplicate, so GitHub stops retrying after unlink+release failures
Files Reviewed (5 files)
  • apps/web/src/lib/integrations/db/github-installations.ts
  • apps/web/src/lib/integrations/github/git-token-service.behavior.test.ts
  • apps/web/src/lib/integrations/platforms/github/webhook-handler.test.ts
  • apps/web/src/lib/integrations/platforms/github/webhook-handler.ts - 1 issue
  • services/git-token-service/src/installation-lookup-service.ts

Previous review (commit 458e94b)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

Failed GitHub installation.deleted retries can still be acknowledged as duplicates while a pending receipt lease is held, so unlink/cleanup is never reclaimed.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0

Fix these issues in Kilo Cloud

Issue Details (click to expand)

WARNING

File Line Issue
apps/web/src/lib/integrations/platforms/github/webhook-handler.ts 184 In-lease pending receipts return HTTP 200 Duplicate, so GitHub stops retrying after unlink+release failures
Files Reviewed (19 files)
  • ENVIRONMENT.md
  • apps/web/src/app/api/integrations/github/callback/route.ts
  • apps/web/src/app/api/integrations/linear/callback/route.test.ts
  • apps/web/src/lib/integrations/db/github-installations.test.ts
  • apps/web/src/lib/integrations/db/github-installations.ts
  • apps/web/src/lib/integrations/github/runtime-authorization.test.ts
  • apps/web/src/lib/integrations/github/runtime-authorization.ts
  • apps/web/src/lib/integrations/oauth-state.ts
  • apps/web/src/lib/integrations/platforms/github/webhook-handler.test.ts
  • apps/web/src/lib/integrations/platforms/github/webhook-handler.ts - 1 issue
  • apps/web/src/lib/integrations/provider-oauth-attempts.ts
  • apps/web/src/lib/organizations/organizations.ts
  • apps/web/src/lib/user/index.ts
  • packages/db/src/migrations/0243_gray_supernaut.sql
  • packages/db/src/migrations/meta/0243_snapshot.json
  • packages/db/src/migrations/meta/_journal.json
  • packages/db/src/schema.ts
  • services/git-token-service/src/installation-lookup-service.test.ts
  • services/git-token-service/src/installation-lookup-service.ts

Previous review (commit d70becc)

Status: 3 Issues Found | Recommendation: Address before merge

Executive Summary

Exclusive GitHub uninstall retries can skip cleanup after unlink failures, unpinned runtime auth stays broken after sharing demotion, and the migration rebuilds live platform_integrations unique indexes without CONCURRENTLY.

Overview

Severity Count
CRITICAL 0
WARNING 3
SUGGESTION 0

Fix these issues in Kilo Cloud

Issue Details (click to expand)

WARNING

File Line Issue
packages/db/src/migrations/0243_eager_taskmaster.sql 41 Blocking unique index rebuild on populated platform_integrations without CONCURRENTLY
apps/web/src/lib/integrations/github/runtime-authorization.ts 119 Unpinned runtime auth treats disconnected sibling rows as ambiguous after sharing demotion
apps/web/src/lib/integrations/platforms/github/webhook-handlers/installation-handler.ts 190 Rethrowing unlink failures skips exclusive uninstall cleanup on GitHub retry
Files Reviewed (50 files)
  • ENVIRONMENT.md
  • apps/web/src/app/api/integrations/bitbucket/connect/route.test.ts
  • apps/web/src/app/api/integrations/github/callback/route.ts
  • apps/web/src/app/api/integrations/linear/callback/route.test.ts
  • apps/web/src/app/api/integrations/linear/connect/route.test.ts
  • apps/web/src/components/cloud-agent-next/NewSessionPanel.tsx
  • apps/web/src/components/integrations/GitHubIntegrationDetails.tsx
  • apps/web/src/components/integrations/OrganizationGitHubInstallations.tsx
  • apps/web/src/components/shared/RepositoryCombobox.tsx
  • apps/web/src/lib/admin/github-installation-uninstall.test.ts
  • apps/web/src/lib/admin/github-installation-uninstall.ts
  • apps/web/src/lib/agent-config/db/agent-configs.ts
  • apps/web/src/lib/cloud-agent/github-integration-helpers.test.ts
  • apps/web/src/lib/cloud-agent/github-integration-helpers.ts
  • apps/web/src/lib/code-reviews/review-memory/settings.ts
  • apps/web/src/lib/integrations/db/github-installations.test.ts
  • apps/web/src/lib/integrations/db/github-installations.ts
  • apps/web/src/lib/integrations/db/platform-integrations.test.ts
  • apps/web/src/lib/integrations/db/platform-integrations.ts
  • apps/web/src/lib/integrations/github-apps-service.ts
  • apps/web/src/lib/integrations/github/multiple-installations.test.ts
  • apps/web/src/lib/integrations/github/multiple-installations.ts
  • apps/web/src/lib/integrations/github/runtime-authorization.test.ts
  • apps/web/src/lib/integrations/github/runtime-authorization.ts - 1 issue
  • apps/web/src/lib/integrations/github/sharing-compatibility.ts
  • apps/web/src/lib/integrations/oauth-state.test.ts
  • apps/web/src/lib/integrations/oauth-state.ts
  • apps/web/src/lib/integrations/oauth/common.ts
  • apps/web/src/lib/integrations/oauth/platforms/discord-callback.ts
  • apps/web/src/lib/integrations/oauth/platforms/linear-callback.ts
  • apps/web/src/lib/integrations/oauth/platforms/slack-callback.ts
  • apps/web/src/lib/integrations/platforms/github/adapter.ts
  • apps/web/src/lib/integrations/platforms/github/webhook-handler.test.ts
  • apps/web/src/lib/integrations/platforms/github/webhook-handler.ts
  • apps/web/src/lib/integrations/platforms/github/webhook-handlers/installation-handler.test.ts
  • apps/web/src/lib/integrations/platforms/github/webhook-handlers/installation-handler.ts - 1 issue
  • apps/web/src/lib/integrations/platforms/github/webhook-handlers/installation-target-handler.test.ts
  • apps/web/src/lib/integrations/platforms/github/webhook-handlers/installation-target-handler.ts
  • apps/web/src/lib/integrations/provider-oauth-attempts.test.ts
  • apps/web/src/lib/integrations/provider-oauth-attempts.ts
  • apps/web/src/lib/organizations/organizations.ts
  • apps/web/src/lib/security-agent/db/security-config.ts
  • apps/web/src/lib/user/index.test.ts
  • apps/web/src/lib/user/index.ts
  • apps/web/src/routers/github-apps-router.test.ts
  • apps/web/src/routers/github-apps-router.ts
  • packages/db/src/migrations/0243_eager_taskmaster.sql - 1 issue
  • packages/db/src/schema.test.ts
  • packages/db/src/schema.ts
  • services/git-token-service/src/installation-lookup-service.test.ts
  • services/git-token-service/src/installation-lookup-service.ts

Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

@pandemicsyn
pandemicsyn force-pushed the feat/github-connections-pr2 branch from 98af34e to 4c3d38b Compare September 10, 2026 15:26
@pandemicsyn
pandemicsyn force-pushed the feat/github-connections-pr2 branch from fda919a to af1734d Compare September 11, 2026 01:27
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