fix: preserve Slack creation results and pending setup guards

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip committed 2026-10-06 22:12:56 -05:00
1 parent 47e3ea3066
commit 7cfc23a318
4 files changed
+70 -6

No files matched your search

@@ -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();
@@ -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) {
+33 -2
View File
@@ -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<void>(resolve => { release = resolve; });
const requested = new Promise<void>(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();
+9 -3
View File
@@ -184,6 +184,7 @@ function ChatSdkEndpointSetup() {
const [slackCredentialsReady, setSlackCredentialsReady] = useState(params.get("stage") === "credentials");
const [viewedStep, setViewedStep] = useState<number | null>(null);
const [slackIdentityReady, setSlackIdentityReady] = useState(false);
const [automaticBusy, setAutomaticBusy] = useState(false);
const [endpoint, setEndpoint] = useState<ChatEndpoint | null>(null);
const [credentials, setCredentials] = useState<Record<string, string>>({});
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}
/>
<div className="min-w-0 space-y-6">
@@ -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<SetStateAction<Record<string, string>>>;
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" }));