diff --git a/server/src/__tests__/chat-channels.integration.test.ts b/server/src/__tests__/chat-channels.integration.test.ts index 7549a64537..71545b2992 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -140,6 +140,7 @@ import { getExternalChannelBindingSummary } from "../services/chat-channel-bindi import { PaperclipRunnerToolAuthority } from "../services/native-runtime/paperclip-runner-tool-authority.js"; import { NativeChatAttachmentReadScope } from "../services/native-runtime/chat-attachment-read.js"; import { logActivity } from "../services/activity-log.js"; +import * as activityLogService from "../services/activity-log.js"; import { subscribeCompanyLiveEvents } from "../services/live-events.js"; import { issueThreadInteractionService } from "../services/issue-thread-interactions.js"; import { questionResponseDeliveryService } from "../services/question-response-delivery.js"; @@ -4044,6 +4045,26 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { for (const canary of [configToken, signingSecret, clientSecret, f.token]) expect(publicState).not.toContain(canary); await request(f.app).patch(`/api/chat-endpoints/${f.endpoint.id}`).send({ slackApp: appDetails }).expect(409); }); + it("preserves a saved app when its creation activity fails without exposing the failure", async () => { + const f = await fixture(); + const originalLogActivity = activityLogService.logActivity; + const activity = vi.spyOn(activityLogService, "logActivity").mockImplementation(async (database, input) => { + if (input.action === "chat_slack.app_created") throw new Error(`Audit outage ${clientSecret}`); + return originalLogActivity(database, input); + }); + const warning = vi.spyOn(chatAttachmentLogger, "warn").mockImplementation(() => {}); + try { + await f.create(); + await f.create(); + expect((await f.service.get(f.endpoint.id)).setup.slackRegistration).toMatchObject({ status: "install", appId: f.appId, errorCode: null }); + const row = await f.service.slackRegistration.registration(f.endpoint.id); + expect(Object.keys(row!.secretIds).sort()).toEqual(["clientSecret", "signingSecret"]); + expect(f.provider.mock.calls.filter(([url]) => String(url).endsWith("apps.manifest.create"))).toHaveLength(1); + expect(warning).toHaveBeenCalledWith({ endpointId: f.endpoint.id, appId: f.appId }, "Slack app creation activity could not be recorded"); + expect(JSON.stringify(warning.mock.calls)).not.toContain(clientSecret); + expect(activity.mock.calls.some(([, input]) => input.action === "chat_slack.creation_failed")).toBe(false); + } finally { activity.mockRestore(); warning.mockRestore(); } + }); it("installs, requires signed URL verification, reauthorizes the same app, and never links the installing user", async () => { const f = await fixture(); await f.create(); diff --git a/server/src/services/chat-slack-registration.ts b/server/src/services/chat-slack-registration.ts index fca36a7959..87c74a6200 100644 --- a/server/src/services/chat-slack-registration.ts +++ b/server/src/services/chat-slack-registration.ts @@ -6,6 +6,7 @@ import { buildSlackAppManifest, slackAppConfigurationSchema, slackRegistrationEr import { badRequest, conflict, forbidden, notFound, unprocessable } from "../errors.js"; import { accessService } from "./access.js"; import { logActivity } from "./activity-log.js"; +import { logger } from "../middleware/logger.js"; import { instanceSettingsService } from "./instance-settings.js"; import { secretService } from "./secrets.js"; import { writeConnectionCredential } from "./connection-credentials.js"; @@ -148,6 +149,7 @@ export function slackChatRegistrationService(db: Db, options: { const [row] = await db.insert(chatSlackRegistrations).values({ endpointId, ...values }) .onConflictDoUpdate({ target: chatSlackRegistrations.endpointId, set: values }).returning(); await audit(row, actor, "creation_started"); + let createdAppId: string; try { const result = await api("apps.manifest.create", { manifest: JSON.stringify(manifest) }, input.credentials.configurationToken); const credentials = object(result.credentials); @@ -159,14 +161,18 @@ export function slackChatRegistrationService(db: Db, options: { await endpoint(endpointId, actor); await saveSecrets(row, { signingSecret: credentials.signing_secret, clientSecret: credentials.client_secret }, actor, lease, { appId: result.app_id, clientId: credentials.client_id, status: "install", errorCode: null }); - await audit({ ...row, appId: result.app_id }, actor, "app_created"); + createdAppId = result.app_id; } catch (error) { // Only documented rejection responses prove that creation did not happen. const code = object(object(error).details).code; const rejected = typeof code === "string" && Object.values(knownProviderErrors).includes(code); await setFailure(row, rejected ? "failed" : "uncertain", rejected ? code : "slack_creation_uncertain", lease); await audit(row, actor, "creation_failed", rejected ? code : "slack_creation_uncertain"); + return; } + // The app and credentials are durable. An audit outage cannot change that outcome. + try { await audit({ ...row, appId: createdAppId }, actor, "app_created"); } + catch { logger.warn({ endpointId, appId: createdAppId }, "Slack app creation activity could not be recorded"); } }); } async function install(endpointId: string, actor: SlackSetupActor) { diff --git a/tests/e2e/chat-adapters-ui-providers.spec.ts b/tests/e2e/chat-adapters-ui-providers.spec.ts index f6c41e627f..f86e65a231 100644 --- a/tests/e2e/chat-adapters-ui-providers.spec.ts +++ b/tests/e2e/chat-adapters-ui-providers.spec.ts @@ -149,6 +149,24 @@ test.describe.serial("native chat adapter UI", () => { test("Slack: automatic creation survives refresh, consent, and verification without copying durable secrets", async ({ page }) => { const slack = PROVIDERS.find(provider => provider.provider === "slack")!; const mock = await installChatControlPlaneMock(page, slack, seed, { enableChatConnectors: true, automaticSlack: true }); + async function holdSetupRequest(action: "registration" | "install") { + let release!: () => void; + let received!: () => void; + const pending = new Promise(resolve => { release = resolve; }); + const requested = new Promise(resolve => { received = resolve; }); + await page.route(`**/api/chat-endpoints/endpoint-slack/slack/${action}`, async route => { + received(); + await pending; + await route.fallback(); + }, { times: 1 }); + return { release, requested }; + } + async function expectNavigationLocked() { + const steps = page.getByRole("navigation", { name: "Connection setup progress" }).getByRole("button"); + await expect(steps).toHaveCount(7); + for (const step of await steps.all()) await expect(step).toBeDisabled(); + await expect(page.getByRole("button", { name: "Save & exit", exact: true })).toBeDisabled(); + } const rootUrl = new URL(`/${seed.prefix}/apps/chat/connect?provider=slack&purpose=chat&resume=endpoint-slack`, test.info().project.use.baseURL).href; const callbackUrl = new URL("/api/chat-slack/oauth/callback?state=fixture-state&code=fixture-code", rootUrl).href; let callbackRequests = 0; @@ -169,8 +187,16 @@ test.describe.serial("native chat adapter UI", () => { await page.goto(`/${seed.prefix}/apps/chat/connect?provider=slack&purpose=chat&agentId=${seed.agentId}`); await page.getByRole("button", { name: "Continue", exact: true }).click(); await page.getByLabel("App configuration token", { exact: true }).fill("fixture-config-token"); - await page.getByRole("button", { name: "Create Slack app", exact: true }).click(); + const creation = await holdSetupRequest("registration"); + try { + await page.getByRole("button", { name: "Create Slack app", exact: true }).click(); + await creation.requested; + await expectNavigationLocked(); + await expect(page.getByLabel("App configuration token", { exact: true })).toHaveValue(""); + await expect(page.getByLabel("Slack app name", { exact: true })).toHaveAttribute("readonly", ""); + } finally { creation.release(); } await expect(page.getByRole("heading", { name: "Install Slack app", exact: true })).toBeVisible(); + await expect(page.getByRole("navigation", { name: "Connection setup progress" }).getByRole("button", { name: /Choose agent/ })).toBeEnabled(); expect(mock.slackCreations).toBe(1); await expect(page.getByLabel("Bot User OAuth Token", { exact: true })).toHaveCount(0); await expect(page.getByLabel("Signing Secret", { exact: true })).toHaveCount(0); @@ -178,7 +204,12 @@ test.describe.serial("native chat adapter UI", () => { await expect(page.getByRole("heading", { name: "Install Slack app", exact: true })).toBeVisible(); await page.getByRole("button", { name: "Save & exit", exact: true }).click(); await page.goto(rootUrl); - await page.getByRole("button", { name: "Install in Slack", exact: true }).click(); + const installation = await holdSetupRequest("install"); + try { + await page.getByRole("button", { name: "Install in Slack", exact: true }).click(); + await installation.requested; + await expectNavigationLocked(); + } finally { installation.release(); } await expect(page.getByRole("heading", { name: "Fixture Slack consent" })).toBeVisible(); await page.getByRole("link", { name: "Approve installation" }).click(); await expect(page.getByRole("heading", { name: "Verify Slack connection", exact: true })).toBeVisible(); diff --git a/ui/src/pages/apps/chat/ChatEndpointSetup.tsx b/ui/src/pages/apps/chat/ChatEndpointSetup.tsx index d5a2bb26c3..3877ff0011 100644 --- a/ui/src/pages/apps/chat/ChatEndpointSetup.tsx +++ b/ui/src/pages/apps/chat/ChatEndpointSetup.tsx @@ -184,6 +184,7 @@ function ChatSdkEndpointSetup() { const [slackCredentialsReady, setSlackCredentialsReady] = useState(params.get("stage") === "credentials"); const [viewedStep, setViewedStep] = useState(null); const [slackIdentityReady, setSlackIdentityReady] = useState(false); + const [automaticBusy, setAutomaticBusy] = useState(false); const [endpoint, setEndpoint] = useState(null); const [credentials, setCredentials] = useState>({}); const [generatedWebhookSecret, setGeneratedWebhookSecret] = useState(""); @@ -484,7 +485,7 @@ function ChatSdkEndpointSetup() { labels={isSlack ? ["Choose agent", "Create Slack app", automaticSlack ? "Install Slack app" : "Add credentials", "Verify Slack connection", "Add avatar", "Connect your Slack account", "Try it"] : undefined} step={step} availableStep={availableStep} - disabled={createEndpoint.isPending || setupAction.isPending || generateSetupSecret.isPending || testConnection.isPending} + disabled={automaticBusy || createEndpoint.isPending || setupAction.isPending || generateSetupSecret.isPending || testConnection.isPending} onSelect={setViewedStep} />
@@ -556,6 +557,8 @@ function ChatSdkEndpointSetup() { setCredentials={setCredentials} repairing={repairing} pending={setupAction.isPending} + automaticBusy={automaticBusy} + onAutomaticBusy={setAutomaticBusy} generatedWebhookSecret={generatedWebhookSecret} generatingSetupSecret={generateSetupSecret.isPending} onGenerateSetupSecret={() => generateSetupSecret.mutate()} @@ -641,6 +644,8 @@ function ProviderConnectStep({ setCredentials, repairing, pending, + automaticBusy, + onAutomaticBusy, onEndpointSaved, generatedWebhookSecret, generatingSetupSecret, @@ -659,6 +664,8 @@ function ProviderConnectStep({ setCredentials: Dispatch>>; repairing: boolean; pending: boolean; + automaticBusy: boolean; + onAutomaticBusy: (busy: boolean) => void; onEndpointSaved: (endpoint: ChatEndpoint) => void; generatedWebhookSecret: string; generatingSetupSecret: boolean; @@ -795,7 +802,6 @@ function ProviderConnectStep({ command: defaultSlackCommand, }, ); - const [automaticBusy, setAutomaticBusy] = useState(false); const automaticSlack = endpoint.setup?.slackSetupMethod === "automatic"; const registrationLocked = Boolean(endpoint.setup?.slackRegistration && endpoint.setup.slackRegistration.status !== "failed"); const slackDetailsEditable = endpoint.status === "draft" && !endpoint.botExternalId && !registrationLocked && !automaticBusy; @@ -1667,7 +1673,7 @@ function ProviderConnectStep({ key={`${endpoint.id}:${slackStage}`} endpoint={endpoint} stage={slackStage} disabled={!endpoint.setup?.webhookUrl || !endpoint.setup?.slackOAuthCallbackUri?.startsWith("https://") || !slackValidation.success || saveSlackApp.isPending} saveDetails={ensureSlackAppSaved} - onBusy={setAutomaticBusy} onSaved={onEndpointSaved} onContinue={onSlackAppCreated} + onBusy={onAutomaticBusy} onSaved={onEndpointSaved} onContinue={onSlackAppCreated} onManual={async existing => { if (!registrationLocked && slackValidation.success) await saveSlackApp.mutateAsync(slackValidation.data); onEndpointSaved(await chatEndpointsApi.update(endpoint.id, { slackSetupMethod: existing ? "existing" : "manual" }));