fix(github): fix connect/disconnect lifecycle bugs found in live org testing

ENVIRONMENT.md

@@ -107,7 +107,10 @@ Manage shared web env var additions and rotations with `pnpm web:env set <VARIAB
 - `GITHUB_LITE_APP_PRIVATE_KEY` - Private key for the lite GitHub App. `[SECRET]`
 - `GITHUB_LITE_APP_CLIENT_ID` - OAuth Client ID for the lite GitHub App install/login flow. [PUBLIC]
 - `GITHUB_MULTIPLE_INSTALLATION_ORGANIZATION_IDS` - Comma-separated Kilo organization UUIDs allowed to connect multiple GitHub App installations. Unset or empty disables multiple installations for all organizations. [SERVER]
+- `GITHUB_SHARED_INSTALLATION_ORGANIZATION_IDS` - Comma-separated destination Kilo organization UUIDs allowed to create an association to a GitHub App installation already associated elsewhere. Unset or empty disables new shared associations without revoking existing ones. [SERVER]
 - `GITHUB_CONNECTION_MANAGEMENT_ENABLED` - Set to exact `true` to admit new existing-installation connection management and local disconnect. Unset or any other value keeps new management admission disabled without changing incumbent GitHub integration workflows. [SERVER]
+  - Keep disabled for at least one OAuth state TTL (10 minutes) after deploying reservation-aware callbacks so purpose-less states issued by the previous version can complete.
+  - During the migration-to-app promotion window, old pending-install callbacks may fail against the replaced pending indexes. Keep the window brief, monitor deploy health, and retry the GitHub connection after promotion completes.
 - `PER_REPO_SETTINGS` - Set to exactly `true` to reveal the Repository Customizations UI (per-installation default AI model / PR review mode, plus per-repository overrides) on the GitHub integration settings pages, for both personal accounts and organizations. Defaults to disabled so the feature can ship dark. [SERVER]
 - `GITHUB_ADMIN_STATS_TOKEN` - Token for admin GitHub API stats lookups; used in `apps/web/src/scripts/backfill-pr-author-github-ids.ts`. `[SECRET]`
 - `GITHUB_CLI_PAT` - GitHub personal access token for `gh` CLI operations inside contractors; used in `services/gastown/container/src/process-manager.ts`. `[SECRET]`

apps/web/src/app/api/integrations/bitbucket/connect/route.test.ts

@@ -63,6 +63,7 @@ describe('GET /api/integrations/bitbucket/connect', () => {
       owner: `org_${ORGANIZATION_ID}`,
       userId: USER_ID,
       returnTo: `/organizations/${ORGANIZATION_ID}/integrations/bitbucket`,
+      issuedAt: expect.any(Number),
     });
     expect(mockedEnsureOrganizationAccess).toHaveBeenCalledWith(
       { user: expect.objectContaining({ id: USER_ID }) },

apps/web/src/app/api/integrations/github/callback/route.test.ts

@@ -11,6 +11,7 @@ import { failureResult } from '@/lib/maybe-result';
 import { consumeInstallState } from '@/lib/integrations/github/install-state';
 import {
   bindGitHubIntegrationToCanonicalInstallation,
+  connectVerifiedGitHubInstallation,
   observeGitHubInstallationLifecycle,
 } from '@/lib/integrations/db/github-installations';
 import type * as InstallStateModule from '@/lib/integrations/github/install-state';
@@ -24,6 +25,7 @@ import { eq } from 'drizzle-orm';
 import { randomUUID } from 'node:crypto';
 import type { Owner } from '@/lib/integrations/core/types';
 import {
+  findGitHubBotLinkIntegrations,
   findIntegrationByInstallationId,
   findIntegrationByInstallationIdForOwner,
   upsertPlatformIntegrationForOwner,
@@ -85,6 +87,7 @@ jest.mock('@/routers/organizations/utils', () => ({
 }));
 jest.mock('@/lib/integrations/db/platform-integrations', () => ({
   createPendingIntegration: jest.fn(),
+  findGitHubBotLinkIntegrations: jest.fn(),
   findIntegrationByInstallationId: jest.fn(),
   findIntegrationByInstallationIdForOwner: jest.fn(),
   findPendingInstallationByRequesterId: jest.fn(),
@@ -113,6 +116,7 @@ const mockedExchangeGitHubOAuthCode = jest.mocked(exchangeGitHubOAuthCode);
 const mockedLinkKiloUser = jest.mocked(linkKiloUser);
 const mockedBot = jest.mocked(bot);
 const mockedFindIntegrationByInstallationId = jest.mocked(findIntegrationByInstallationId);
+const mockedFindGitHubBotLinkIntegrations = jest.mocked(findGitHubBotLinkIntegrations);
 const mockedFindIntegrationByInstallationIdForOwner = jest.mocked(
   findIntegrationByInstallationIdForOwner
 );
@@ -129,6 +133,7 @@ const mockedCaptureMessage = jest.mocked(captureMessage);
 const mockedEnsureOrganizationAccess = jest.mocked(ensureOrganizationAccess);
 const mockedAssertUserAdministersInstallation = jest.mocked(assertUserAdministersInstallation);
 const mockedObserveGitHubInstallationLifecycle = jest.mocked(observeGitHubInstallationLifecycle);
+const mockedConnectVerifiedGitHubInstallation = jest.mocked(connectVerifiedGitHubInstallation);
 const mockedBindGitHubIntegrationToCanonicalInstallation = jest.mocked(
   bindGitHubIntegrationToCanonicalInstallation
 );
@@ -160,6 +165,18 @@ beforeEach(() => {
         github_app_type: 'standard',
       }) as never
   );
+  mockedFindGitHubBotLinkIntegrations.mockImplementation(
+    async () =>
+      [
+        {
+          id: '00000000-0000-4000-8000-000000000099',
+          owned_by_user_id: writtenOwner.type === 'user' ? writtenOwner.id : null,
+          owned_by_organization_id: writtenOwner.type === 'org' ? writtenOwner.id : null,
+          platform_installation_id: INSTALLATION_ID,
+          github_app_type: 'standard',
+        },
+      ] as never
+  );
   mockedFindIntegrationByInstallationIdForOwner.mockImplementation(
     async () =>
       ({
@@ -197,6 +214,8 @@ function expectRedirectLocation(response: Response, expectedPathWithQuery: strin
 describe('GET /api/integrations/github/callback bot link flow', () => {
   beforeEach(() => {
     jest.clearAllMocks();
+    process.env.GITHUB_CONNECTION_MANAGEMENT_ENABLED = 'false';
+    process.env.GITHUB_SHARED_INSTALLATION_ORGANIZATION_IDS = '';
 
     mockedGetUserFromAuth.mockResolvedValue({
       user: { id: USER_ID },
@@ -206,6 +225,7 @@ describe('GET /api/integrations/github/callback bot link flow', () => {
       userId: USER_ID,
       installationId: INSTALLATION_ID,
       callbackPath: '/github/link',
+      platformIntegrationId: '00000000-0000-4000-8000-000000000099',
     });
     mockedExchangeGitHubOAuthCode.mockResolvedValue({
       id: GITHUB_USER_ID,
@@ -218,6 +238,15 @@ describe('GET /api/integrations/github/callback bot link flow', () => {
       github_app_type: 'standard',
       metadata: null,
     } as never);
+    mockedFindGitHubBotLinkIntegrations.mockResolvedValue([
+      {
+        id: '00000000-0000-4000-8000-000000000099',
+        owned_by_organization_id: 'org_1',
+        owned_by_user_id: null,
+        platform_installation_id: INSTALLATION_ID,
+        github_app_type: 'standard',
+      } as never,
+    ]);
     mockedIsOrganizationMember.mockResolvedValue(true);
   });
 
@@ -281,11 +310,11 @@ describe('GET /api/integrations/github/callback bot link flow', () => {
     await expect(response.text()).resolves.toContain(
       'not a member of the organization that owns this GitHub integration'
     );
-    expect(mockedFindIntegrationByInstallationId).toHaveBeenCalledWith(
-      'github',
-      INSTALLATION_ID,
-      'standard'
-    );
+    expect(mockedFindGitHubBotLinkIntegrations).toHaveBeenCalledWith({
+      installationId: INSTALLATION_ID,
+      appType: 'standard',
+      platformIntegrationId: '00000000-0000-4000-8000-000000000099',
+    });
     expect(mockedExchangeGitHubOAuthCode).not.toHaveBeenCalled();
     expect(mockedLinkKiloUser).not.toHaveBeenCalled();
   });
@@ -299,11 +328,11 @@ describe('GET /api/integrations/github/callback bot link flow', () => {
     expect(response.status).toBe(200);
     await expect(response.text()).resolves.toContain('GitHub account octocat has been linked');
     expect(mockedExchangeGitHubOAuthCode).toHaveBeenCalledWith('abc', 'standard');
-    expect(mockedFindIntegrationByInstallationId).toHaveBeenCalledWith(
-      'github',
-      INSTALLATION_ID,
-      'standard'
-    );
+    expect(mockedFindGitHubBotLinkIntegrations).toHaveBeenCalledWith({
+      installationId: INSTALLATION_ID,
+      appType: 'standard',
+      platformIntegrationId: '00000000-0000-4000-8000-000000000099',
+    });
     expect(mockedIsOrganizationMember).toHaveBeenCalledWith('org_1', USER_ID);
     expect(mockedBot.initialize).toHaveBeenCalled();
     expect(mockedLinkKiloUser).toHaveBeenCalledWith(
@@ -318,27 +347,80 @@ describe('GET /api/integrations/github/callback bot link flow', () => {
     );
   });
 
+  test('allows one healthy legacy no-ID state while management and sharing are disabled', async () => {
+    mockedVerifyGitHubBotLinkState.mockReturnValue({
+      userId: USER_ID,
+      installationId: INSTALLATION_ID,
+      callbackPath: '/github/link',
+    });
+    const { GET } = await import('./route');
+    const response = await GET(
+      makeRequest('/api/integrations/github/callback?code=abc&state=signed') as never
+    );
+    expect(response.status).toBe(200);
+    expect(mockedFindGitHubBotLinkIntegrations).toHaveBeenCalledWith({
+      installationId: INSTALLATION_ID,
+      appType: 'standard',
+      platformIntegrationId: undefined,
+    });
+  });
+
+  test('rejects a no-ID state after connection management is enabled', async () => {
+    process.env.GITHUB_CONNECTION_MANAGEMENT_ENABLED = 'true';
+    mockedVerifyGitHubBotLinkState.mockReturnValue({
+      userId: USER_ID,
+      installationId: INSTALLATION_ID,
+      callbackPath: '/github/link',
+    });
+    const { GET } = await import('./route');
+    const response = await GET(
+      makeRequest('/api/integrations/github/callback?code=abc&state=signed') as never
+    );
+    expect(response.status).toBe(404);
+    expect(mockedExchangeGitHubOAuthCode).not.toHaveBeenCalled();
+  });
+
+  test('rejects a no-ID state after shared-installation admission is enabled', async () => {
+    process.env.GITHUB_SHARED_INSTALLATION_ORGANIZATION_IDS =
+      '00000000-0000-4000-8000-000000000001';
+    mockedVerifyGitHubBotLinkState.mockReturnValue({
+      userId: USER_ID,
+      installationId: INSTALLATION_ID,
+      callbackPath: '/github/link',
+    });
+    const { GET } = await import('./route');
+    const response = await GET(
+      makeRequest('/api/integrations/github/callback?code=abc&state=signed') as never
+    );
+    expect(response.status).toBe(404);
+    expect(mockedExchangeGitHubOAuthCode).not.toHaveBeenCalled();
+  });
+
   test('routes a Lite bot-link callback through the Lite app identity', async () => {
     mockedVerifyGitHubBotLinkState.mockReturnValue({
       userId: USER_ID,
       installationId: INSTALLATION_ID,
       callbackPath: '/github/link',
       githubAppType: 'lite',
+      platformIntegrationId: '00000000-0000-4000-8000-000000000099',
     });
-    mockedFindIntegrationByInstallationId.mockResolvedValue({
-      owned_by_organization_id: 'org_1',
-      github_app_type: 'lite',
-    } as never);
+    mockedFindGitHubBotLinkIntegrations.mockResolvedValue([
+      {
+        id: '00000000-0000-4000-8000-000000000099',
+        owned_by_organization_id: 'org_1',
+        github_app_type: 'lite',
+      } as never,
+    ]);
     const { GET } = await import('./route');
     const response = await GET(
       makeRequest('/api/integrations/github/callback?code=abc&state=signed') as never
     );
     expect(response.status).toBe(200);
-    expect(mockedFindIntegrationByInstallationId).toHaveBeenCalledWith(
-      'github',
-      INSTALLATION_ID,
-      'lite'
-    );
+    expect(mockedFindGitHubBotLinkIntegrations).toHaveBeenCalledWith({
+      installationId: INSTALLATION_ID,
+      appType: 'lite',
+      platformIntegrationId: '00000000-0000-4000-8000-000000000099',
+    });
     expect(mockedExchangeGitHubOAuthCode).toHaveBeenCalledWith('abc', 'lite');
     expect(mockedLinkKiloUser).toHaveBeenCalledWith(
       mockState,
@@ -347,18 +429,16 @@ describe('GET /api/integrations/github/callback bot link flow', () => {
     );
   });
 
-  test("exchanges the OAuth code against the integration's github_app_type", async () => {
-    mockedFindIntegrationByInstallationId.mockResolvedValue({
-      owned_by_organization_id: 'org_1',
-      owned_by_user_id: null,
-      github_app_type: 'lite',
-      metadata: null,
-    } as never);
+  test('rejects an integration whose app type does not match signed state', async () => {
+    mockedFindGitHubBotLinkIntegrations.mockResolvedValue([]);
 
     const { GET } = await import('./route');
-    await GET(makeRequest('/api/integrations/github/callback?code=abc&state=signed') as never);
+    const response = await GET(
+      makeRequest('/api/integrations/github/callback?code=abc&state=signed') as never
+    );
 
-    expect(mockedExchangeGitHubOAuthCode).toHaveBeenCalledWith('abc', 'lite');
+    expect(response.status).toBe(404);
+    expect(mockedExchangeGitHubOAuthCode).not.toHaveBeenCalled();
   });
 });
 
@@ -1139,6 +1219,10 @@ describe('GET /api/integrations/github/callback plaintext state rejection', () =
 describe('GET /api/integrations/github/callback admin proof', () => {
   beforeEach(() => {
     jest.clearAllMocks();
+    // Most tests in this describe exercise the legacy access-list admin
+    // check; the connection-management-enabled test below opts back in
+    // explicitly so it does not leak into the tests that follow it.
+    process.env.GITHUB_CONNECTION_MANAGEMENT_ENABLED = 'false';
 
     mockedGetUserFromAuth.mockResolvedValue({
       user: {
@@ -1229,6 +1313,43 @@ describe('GET /api/integrations/github/callback admin proof', () => {
       installationId: INSTALLATION_ID,
     });
     expect(mockedUpsertPlatformIntegrationForOwner).toHaveBeenCalled();
+    // Even on the legacy access-list admin check path, the OAuth code
+    // exchange resolved a real GitHub identity — it must still be recorded
+    // as authorization provenance rather than left null.
+    expect(mockedUpsertPlatformIntegrationForOwner).toHaveBeenCalledWith(
+      expect.anything(),
+      expect.objectContaining({
+        kiloUserId: USER_ID,
+        githubUserId: GITHUB_USER_ID,
+      })
+    );
+  });
+
+  test('routes a management-enabled install through the verified writer with both identities', async () => {
+    process.env.GITHUB_CONNECTION_MANAGEMENT_ENABLED = 'true';
+    mockedConnectVerifiedGitHubInstallation.mockResolvedValue({
+      ok: true,
+      integrationId: '00000000-0000-4000-8000-0000000000aa',
+    });
+
+    const { GET } = await import('./route');
+    const response = await GET(
+      makeRequest(
+        `/api/integrations/github/callback?installation_id=${INSTALLATION_ID}&setup_action=install&state=${INSTALL_STATE_TOKEN}&code=abc`
+      ) as never
+    );
+
+    expect(response.status).toBe(307);
+    expect(mockedVerifyGitHubInstallationAuthorization).toHaveBeenCalled();
+    expect(mockedAssertUserAdministersInstallation).not.toHaveBeenCalled();
+    expect(mockedUpsertPlatformIntegrationForOwner).not.toHaveBeenCalled();
+    expect(mockedConnectVerifiedGitHubInstallation).toHaveBeenCalledWith(
+      { type: 'user', id: USER_ID },
+      expect.objectContaining({
+        kiloUserId: USER_ID,
+        githubUserId: GITHUB_USER_ID,
+      })
+    );
   });
 
   test('rejects an install when admin check returns false', async () => {