From 82a6c5b7bbb6380fb6007bb85178ac11611333f9 Mon Sep 17 00:00:00 2001 From: Dotta Date: Wed, 7 Oct 2026 01:07:10 -0500 Subject: [PATCH] fix(github): initialize and resume fresh manual App imports Co-Authored-By: Paperclip --- doc/plans/2026-10-06-github-app-wizard.md | 6 ++- packages/shared/src/types/chat-channels.ts | 1 + .../chat-channels.integration.test.ts | 51 +++++++++++++++++-- server/src/services/chat-channels.ts | 13 ++++- server/src/services/chat-github-wizard.ts | 10 ++-- 5 files changed, 70 insertions(+), 11 deletions(-) diff --git a/doc/plans/2026-10-06-github-app-wizard.md b/doc/plans/2026-10-06-github-app-wizard.md index 0b5a4a4d386..3b89c566cb2 100644 --- a/doc/plans/2026-10-06-github-app-wizard.md +++ b/doc/plans/2026-10-06-github-app-wizard.md @@ -106,7 +106,7 @@ enrollment handoff passed. The test did not authorize Cloud enrollment or create a provider App. The focused wizard and retained clipboard/recovery UI suites pass (37 tests). -GitHub backend coverage passes (189 cases), including both owner types, +GitHub backend coverage passes (192 cases), including both owner types, interrupted registration and vault storage, empty saved repository selections, localhost configuration, concurrent identity attempts, enrolled-origin recovery, tenant binding, and legacy callbacks. The complete OpenAPI suite passes (13 @@ -121,7 +121,9 @@ a claim of a green full suite. CI initially identified outdated wizard expectations and missing API documentation, both now covered by the updated focused checks. The full GitHub fixture also verifies installation permissions and preserved legacy tool authority. The browser -report attaches screenshots of both wizard screens. +report attaches screenshots of both wizard screens. Fresh manual App imports +initialize repository access and tools without a registration row; interrupted +imports resume those defaults while explicit restrictions stay preserved. Cloud's final standard `npm test` passes: 2,561 tests and 73 environment-dependent skips. Dedicated-App regressions cover normalized concurrent starts, stale diff --git a/packages/shared/src/types/chat-channels.ts b/packages/shared/src/types/chat-channels.ts index fd01c9f9b11..27fbd39566e 100644 --- a/packages/shared/src/types/chat-channels.ts +++ b/packages/shared/src/types/chat-channels.ts @@ -201,6 +201,7 @@ export interface ChatEndpointSetupState { initialRepositoriesImported?: boolean; repositorySelectionSaved?: boolean; initialRepositoryImportPending?: boolean; + initialSetupPending?: boolean; }; step: "choose_agent" | "provider_setup" | "test" | "complete"; /** Server-generated boundary; only provider events at or after this time can complete setup. */ diff --git a/server/src/__tests__/chat-channels.integration.test.ts b/server/src/__tests__/chat-channels.integration.test.ts index 0dfe8757166..14a67f82a26 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -1876,6 +1876,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { > & { wakeup?: ChatChannelServiceOptions["heartbeat"]["wakeup"] } = {}, useVerifiedAppId = false, registerWithManifest = false, + connectExistingApp = false, ) { let setupComplete = false; const deferredSchedule = overrides.scheduleDeferredWork; @@ -2051,6 +2052,10 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { ); if (registerWithManifest) await context.service.startGitHubRegistration(endpoint.id, "owner-user", "Maya"); + if (connectExistingApp) + await context.service.storeGitHubApp(endpoint.id, "owner-user", { + appId: String(appRegistrationId), privateKey, webhookSecret, + }); await context.service.configure( endpoint.id, { @@ -2069,7 +2074,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { await db.transaction((tx) => syncGitHubBotTools(tx, configured!, "owner-user", true)); } const resources = await context.service.listResources(endpoint.id); - if (!registerWithManifest) + if (!registerWithManifest && !connectExistingApp) await context.service.replaceResources(endpoint.id, [ { id: resources[0]!.id, enabled: true }, ]); @@ -2873,11 +2878,13 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { expect(session!.handoff?.identityLeaseId).toBeUndefined(); expect(writes).toBe(2); }); - it("imports initial GitHub access without a second picker and leaves later additions disabled", async () => { + it.each(["manifest", "existing"] as const)("imports initial GitHub access for a %s App without a second picker and leaves later additions disabled", async (method) => { const f = await seedCompany(); - const context = await configuredGitHubEndpoint(f, {}, true, true); + const context = await configuredGitHubEndpoint(f, {}, true, method === "manifest", method === "existing"); const current = await context.service.get(context.endpoint.id); expect(current.setup.github?.initialRepositoryImportPending).toBe(true); + const registrations = await db.select().from(chatGitHubRegistrations).where(eq(chatGitHubRegistrations.endpointId, current.id)); + expect(registrations).toHaveLength(method === "manifest" ? 1 : 0); await db.update(chatGitHubRegistrations).set({ status: "completed" }) .where(eq(chatGitHubRegistrations.endpointId, current.id)); const result = await context.service.githubWizard.advance( @@ -2929,6 +2936,44 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { )?.enabled, ).toBe(false); }); + it("manual App setup resumes tool initialization after an interrupted repository import", async () => { + const f = await seedCompany(); + const context = await configuredGitHubEndpoint(f, {}, true, false, true); + const wizard = githubChatWizardService(db, { + origin: () => "https://reviews.example.test", + fetch: context.providerFetch, + startDirect: context.service.startGitHubRegistration, + storeApp: context.service.storeGitHubApp, + storeCredentials: async () => {}, + refreshRepositories: context.service.refreshGitHubRepositories, + resources: context.service.listResources, + replaceResources: async (...args) => { + await context.service.replaceResources(...args); + throw new Error("Interrupted after repository import"); + }, + configure: async () => {}, + finish: async () => {}, + }); + await expect(wizard.advance(context.endpoint.id, "owner-user")).rejects.toThrow("Interrupted after repository import"); + expect((await context.service.get(context.endpoint.id)).setup.github).toMatchObject({ initialSetupPending: true, initialRepositoryImportPending: false }); + await context.service.githubWizard.advance(context.endpoint.id, "owner-user"); + expect((await githubChatManagementService(db, context.providerFetch).configuration(context.endpoint.id, "owner-user")).configuration.toolsEnabled).toBe(true); + expect((await context.service.get(context.endpoint.id)).setup.github).toMatchObject({ initialRepositoriesImported: true, initialSetupPending: false }); + }); + it("manual App setup preserves explicitly disabled repositories and tools", async () => { + const f = await seedCompany(); + const context = await configuredGitHubEndpoint(f, {}, true, false, true); + const management = githubChatManagementService(db, context.providerFetch); + const saved = await management.configuration(context.endpoint.id, "owner-user"); + await management.saveConfiguration(context.endpoint.id, { + expectedRevision: saved.revision, + configuration: { ...saved.configuration, toolsEnabled: false }, + }, "owner-user"); + await context.service.replaceResources(context.endpoint.id, [], "owner-user"); + await context.service.githubWizard.advance(context.endpoint.id, "owner-user"); + expect((await context.service.listResources(context.endpoint.id)).every(resource => !resource.enabled)).toBe(true); + expect(await management.configuration(context.endpoint.id, "owner-user")).toMatchObject({ revision: 1, configuration: { toolsEnabled: false } }); + }); it("Cloud bot delivery cannot enter another company's endpoint or a different agent binding", async () => { const f = await reviewBotFixture(); const event = { diff --git a/server/src/services/chat-channels.ts b/server/src/services/chat-channels.ts index dacea199ce3..824be5ceb50 100644 --- a/server/src/services/chat-channels.ts +++ b/server/src/services/chat-channels.ts @@ -38224,7 +38224,14 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) { try { await db.transaction(async tx => { await credentialLease.assertOwned(tx); - const claimed = await tx.update(chatEndpoints).set({ status: "attention", botExternalId: identity.botExternalId, botUsername: identity.botUsername, providerAccountId: identity.providerAccountId, providerAccountLabel: identity.providerAccountLabel, healthMessage: "Complete GitHub App setup", updatedAt: new Date() }).where(and(eq(chatEndpoints.id, endpointId), isNull(chatEndpoints.botExternalId))).returning({ id: chatEndpoints.id }); + const [current] = await tx.select({ setup: chatEndpoints.setup }).from(chatEndpoints).where(eq(chatEndpoints.id, endpointId)).for("update"); + const [resource] = await tx.select({ id: chatEndpointResources.id }).from(chatEndpointResources).where(eq(chatEndpointResources.endpointId, endpointId)).limit(1); + // A fresh manual App has no registration session. Record its first + // import before vault writes so interrupted credential storage can resume. + const initialImport = !current?.setup.github?.repositorySelectionSaved && !resource; + const claimed = await tx.update(chatEndpoints).set({ status: "attention", botExternalId: identity.botExternalId, botUsername: identity.botUsername, providerAccountId: identity.providerAccountId, providerAccountLabel: identity.providerAccountLabel, + setup: sql`jsonb_set(${chatEndpoints.setup}, '{github}', coalesce(${chatEndpoints.setup}->'github', '{}'::jsonb) || ${JSON.stringify({ initialSetupPending: true, ...(initialImport ? { initialRepositoryImportPending: true } : {}) })}::jsonb)`, + healthMessage: "Complete GitHub App setup", updatedAt: new Date() }).where(and(eq(chatEndpoints.id, endpointId), isNull(chatEndpoints.botExternalId))).returning({ id: chatEndpoints.id }); if (claimed.length !== 1) throw conflict("The bot changed during registration"); }); } catch (error) { @@ -38237,10 +38244,12 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) { if (!slug || !/^[a-z0-9-]+$/.test(slug)) throw unprocessable("GitHub did not identify this App"); await db.transaction(async tx => { await credentialLease.assertOwned(tx); + const [current] = await tx.select({ setup: chatEndpoints.setup, status: chatEndpoints.status }).from(chatEndpoints).where(eq(chatEndpoints.id, endpointId)).for("update"); + if (!current || ["archived", "active", "paused"].includes(current.status)) throw conflict("Use the reconnect flow for an existing active bot"); await tx.update(chatEndpoints).set({ status: "attention", botExternalId: identity.botExternalId, botUsername: identity.botUsername, botDisplayName: identity.botLabel, providerAccountId: identity.providerAccountId, providerAccountLabel: identity.providerAccountLabel, - setup: { ...record.endpoint.setup, github: { ...record.endpoint.setup.github, stage: "install", appSlug: slug, installationUrl: `https://github.com/apps/${slug}/installations/new`, registrationStatus: "completed" } }, + setup: { ...current.setup, github: { ...current.setup.github, stage: "install", appSlug: slug, installationUrl: `https://github.com/apps/${slug}/installations/new`, registrationStatus: "completed" } }, healthMessage: "Install the App on GitHub to continue", updatedAt: new Date(), }).where(eq(chatEndpoints.id, endpointId)); await logActivity(tx as unknown as Db, { companyId: record.endpoint.companyId, actorType: "user", actorId: userId, action: "chat_github.app_connected", entityType: "tool_connection", entityId: record.endpoint.connectionId, details: { endpointId, appId: identity.botExternalId } }); diff --git a/server/src/services/chat-github-wizard.ts b/server/src/services/chat-github-wizard.ts index 869515acbaa..d0072b6a93c 100644 --- a/server/src/services/chat-github-wizard.ts +++ b/server/src/services/chat-github-wizard.ts @@ -720,9 +720,11 @@ export function githubChatWizardService( const saved = await management.configuration(id, userId); if (!bot.setup.github?.initialRepositoriesImported) { const resources = await options.resources(id); - // Existing saved selections/behavior belong to the user. Only a fresh registration imports GitHub's choices. + // Fresh manifest and manual Apps import GitHub's initial choices. Saved + // selections and configuration still belong to the user on recovery. + const initializeApp = !!session || bot.setup.github?.initialSetupPending === true; if ( - session && + initializeApp && bot.status !== "active" && saved.revision === 0 && bot.setup.github?.initialRepositoryImportPending === true && @@ -738,7 +740,7 @@ export function githubChatWizardService( { initialGitHubImport: true }, ); } - if (session && saved.revision === 0 && bot.status !== "active") + if (initializeApp && saved.revision === 0 && bot.status !== "active") await management.saveConfiguration( id, { @@ -755,7 +757,7 @@ export function githubChatWizardService( }, userId, ); - await setup(bot, { initialRepositoriesImported: true }); + await setup(bot, { initialRepositoriesImported: true, initialSetupPending: false }); } if (bot.status !== "active" && bot.status !== "verifying") await options.configure(id, userId);