From f1fb1bcb8ad0c066aea87971de59360e65ae96c6 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 3 Nov 2026 17:20:07 -0600 Subject: [PATCH] fix(ui): let Done close completed Slack setup (#18878) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Slack connections let people talk to their assigned agents. > - Setup saves a completed state after the connection is ready. > - A saved setup link can still open Success after completion. > - Done submits completion again, which the server rejects. > - This pull request opens settings when the Slack setup is already complete. > - Users can close the saved setup without changing the bot's lifecycle. ## Linked Issues or Issue Description **What happened?** Reopening a completed Slack setup shows Success. Done then shows “Couldn't finish setup: This connection is not waiting for a setup test.” The user stays in the wizard. **Expected behavior** Done opens the saved connection settings. A paused bot stays paused. A setup that is still awaiting completion must use the existing verification request. **Steps to reproduce** 1. Finish a Slack connection. 2. Reopen its saved chat/connect link with provider=slack and its resume parameter. 3. Click Done on Success. 4. Observe the completion error instead of the settings page. **Paperclip version or commit** Reproduced live on Core da77c1bcfac0c1685392abfb61f63a7d8f3d8b5d. The handler is unchanged on this branch's base 51d1d4ce07abee9807d546b865e3c74fd600d31a. Related: #18837 and #18722. Also inspected #15828; it changes required tests for other providers and explicitly excludes Slack. This fix covers reopening a completed Slack setup. ## What Changed - Done opens settings for a Slack setup whose saved step is complete. Both the provisioned Success view and manual Try view use the same action. Unfinished setups retain the existing completion request. - Add active and paused completion cases for managed, automatic and manual setup. Keep positive cases for unfinished setup. - Extend the managed and automatic browser journeys to reopen the saved link, reject duplicate completion requests, click Done, and reload settings. - Add completed-state previews and make the Slack fixture reject duplicate completion, as the real server does. Update the provider guide. Maps are unchanged. ## Verification - Six component regressions failed on the original handler because it submitted completion again. - All 38 wizard/clipboard component cases passed after the handler change. All 284 related chat UI tests across 19 files passed. The final fixtures add the required welcome-status field. - Repository typecheck and build passed. UI token gates and Storybook catalog checks passed (3076 stories; structural only). All 9 Slack browser cases passed in 1.5 minutes on Node 24, including completed-link exit and settings reload for managed and automatic setup. Their provider responses are mocked. Fresh deployed acceptance remains pending. Full CI [41734669956](https://github.com/paperclipai/paperclip/actions/runs/41734669956) passed on attempt 2 for exact head `4a3bf180929df57350389f45e4c6acfa822e8ddc`. Greptile and Ponytail both give this head 5/5 with no actionable findings. - Completed-state production previews passed for managed and automatic setup. These previews check the rendered state and Slack handoff; exact Done navigation is covered by the browser journeys. One preview text-locator wait timed out; fresh accessibility state and the saved image then showed Story status Pass with all three assertions passed. - CI attempt 1 lost its typecheck runner while unpacking the Rust dependency cache. Typechecking had not started. The log is retained. All test jobs passed in attempt 1; its verify summary failed only because typecheck had failed. The failed-only retry passed on the same commit. - The initial typecheck found two new fixtures missing welcomeStatus. Those fixtures were corrected; the failure log is retained. - The live returning-user attempt is preserved as failed. The Slack bot answered the fresh probe, but Done returned the error above. The original toast expired before its journey screenshot; a separate ordinary-UI reproduction captured the visible toast. This is not a live pass of the new fix. - The full local server Vitest suite was not repeated for this UI-only change. Full CI passed before merge. ## Risks The action uses the server-reported completed setup state to leave the wizard. It does not activate, unpause or repair a bot. Settings load the saved connection through the existing access checks. Unfinished setups still use server verification. API contracts, credentials and provider permissions do not change. Concurrent completion from another tab is unchanged: a stale test-step tab can still need a reload after a conflict. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, repository editing, browser use and command execution. The runtime does not expose a more precise model ID or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip --- doc/connections/slack-setup.md | 4 ++ tests/e2e/chat-adapters-ui-providers.spec.ts | 20 +++++++++ .../apps/chat/ChatEndpoint.clipboard.test.tsx | 44 ++++++++++++++++++- ui/src/pages/apps/chat/ChatEndpointSetup.tsx | 11 ++++- ui/storybook/fixtures/SlackSetupFixture.tsx | 12 +++-- .../slack-automatic-setup.stories.tsx | 1 + .../slack-managed-setup.stories.tsx | 1 + 7 files changed, 86 insertions(+), 7 deletions(-) diff --git a/doc/connections/slack-setup.md b/doc/connections/slack-setup.md index 18c8544ce6c..e167017e84f 100644 --- a/doc/connections/slack-setup.md +++ b/doc/connections/slack-setup.md @@ -24,6 +24,10 @@ Creation, installation, message waiting, and success use the same production components and server evidence as the own-app flow. A click or OAuth return does not constitute successful configuration. +Reopening a completed Slack setup retains its Slack handoff. **Done** opens the +saved connection settings without repeating activation or reactivating a paused +bot. Unfinished setups still complete their normal server verification first. + ## Cloud deployment prerequisites Managed setup is disabled by default. Qualification requires all of: diff --git a/tests/e2e/chat-adapters-ui-providers.spec.ts b/tests/e2e/chat-adapters-ui-providers.spec.ts index 56f515e0e61..5b4679b6ec4 100644 --- a/tests/e2e/chat-adapters-ui-providers.spec.ts +++ b/tests/e2e/chat-adapters-ui-providers.spec.ts @@ -20,6 +20,22 @@ import { GITHUB_PRIVATE_KEY_PASTE_FIXTURE, } from "./chat-adapters-ui.shared"; +async function expectCompletedSlackSetupCloses(page: Page, seed: Seed) { + let finishRequests = 0; + await page.route("**/api/chat-endpoints/endpoint-slack/finish", async route => { + finishRequests++; + await route.fulfill({ status: 409, json: { error: "This connection is not waiting for a setup test" } }); + }); + await page.goto(`/${seed.prefix}/apps/chat/connect?provider=slack&toolHref=%2Fapps%2Fconnect%3Fsource%3Dslack&resume=endpoint-slack`); + await expect(page.getByRole("heading", { name: "Success", exact: true })).toBeVisible(); + await expect(page.getByRole("link", { name: "Open Slack", exact: true })).toBeVisible(); + await page.getByRole("button", { name: "Done", exact: true }).click(); + await expect(page).toHaveURL(/\/apps\/chat\/endpoint-slack\/settings$/); + await page.reload(); + await expect(page.getByRole("link", { name: "Open Slack", exact: true })).toBeVisible(); + expect(finishRequests).toBe(0); +} + async function findConnectorBySearch(page: Page, query: string, slug: string) { const search = page.getByRole("searchbox", { name: "Search connectors" }); await expect(search).toBeVisible(); @@ -187,6 +203,8 @@ test.describe.serial("native chat adapter UI", () => { await expect(page.getByRole("navigation", { name: "Connection setup progress" }).getByRole("button")).toHaveCount(2); await page.getByRole("button", { name: "Done", exact: true }).click(); await expect(page).toHaveURL(/\/apps\/chat\/endpoint-slack\/settings$/); + await expectCompletedSlackSetupCloses(page, seed); + expect(mock.slackCreations).toBe(1); expect(mock.slackInstallations).toBe(1); }); test("Slack: a failed manager refresh reloads workspace authorization before retry", async ({ page }) => { const slack = PROVIDERS.find(provider => provider.provider === "slack")!; @@ -379,6 +397,8 @@ test.describe.serial("native chat adapter UI", () => { await page.screenshot({ path: test.info().outputPath("automatic-slack-success.png"), fullPage: true }); await page.getByRole("button", { name: "Done", exact: true }).click(); await expect(page).toHaveURL(/\/apps\/chat\/endpoint-slack\/settings$/); + await expectCompletedSlackSetupCloses(page, seed); + expect(mock.slackCreations).toBe(1); expect(mock.slackInstallations).toBe(2); const stored = await page.evaluate(() => JSON.stringify({ local: { ...localStorage }, session: { ...sessionStorage } })); expect(stored).not.toContain("fixture-config-token"); }); diff --git a/ui/src/pages/apps/chat/ChatEndpoint.clipboard.test.tsx b/ui/src/pages/apps/chat/ChatEndpoint.clipboard.test.tsx index dc6c52282e3..e2bae6a68e9 100644 --- a/ui/src/pages/apps/chat/ChatEndpoint.clipboard.test.tsx +++ b/ui/src/pages/apps/chat/ChatEndpoint.clipboard.test.tsx @@ -30,6 +30,7 @@ const mocks = vi.hoisted(() => ({ createLinkIntent: vi.fn(), confirmIdentityLink: vi.fn(), pushToast: vi.fn(), + navigate: vi.fn(), setBreadcrumbs: vi.fn(), search: "", setParams: vi.fn(), @@ -57,7 +58,7 @@ vi.mock("@/context/SidebarContext", () => ({ useSidebar: () => ({ isMobile: false }), })); vi.mock("@/lib/router", () => ({ - useNavigate: () => vi.fn(), + useNavigate: () => mocks.navigate, useLocation: () => ({ state: null }), useParams: () => ({ endpointId: "endpoint-a", tab: mocks.tab }), useSearchParams: () => [new URLSearchParams(mocks.search), mocks.setParams], @@ -505,6 +506,47 @@ describe("chat setup and identity-link clipboard actions", () => { expect(container.querySelector('aside button[aria-current="step"]')?.textContent).toBe("4Send a message"); }); + it.each( + (["managed", "automatic", "manual"] as const).flatMap(method => + (["active", "paused"] as const).map(status => ({ method, status }))), + )("opens settings when Done resumes a completed $method Slack connection that is $status", async ({ method, status }) => { + const endpoint = await render("slack"); + const saved: ChatEndpoint = { ...endpoint, status, providerAccountId: "workspace-a", setup: { + ...endpoint.setup!, step: "complete", slackSetupMethod: method, + slackRegistration: { status: "configured", appId: "ATEST" }, + slackAccount: { externalUserId: "UPERSON", paperclipUserId: "owner-user", status: "linked", welcomeStatus: "sent", dmChannelId: "DTEST" }, + } }; + mocks.get.mockResolvedValue(saved); + mocks.finishSlackSetup.mockRejectedValue(new Error("This connection is not waiting for a setup test")); + flushSync(() => client.setQueryData(["chat-endpoint-setup-resume", endpoint.id], saved)); + await settle(); + + await click("Done"); + + expect(mocks.finishSlackSetup).not.toHaveBeenCalled(); + expect(mocks.navigate).toHaveBeenCalledWith("/apps/chat/endpoint-a/settings"); + expect(mocks.pushToast).not.toHaveBeenCalled(); + }); + + it.each(["managed", "automatic"] as const)("still finalizes an unfinished %s Slack setup before opening settings", async method => { + const endpoint = await render("slack"); + const saved: ChatEndpoint = { ...endpoint, status: "verifying", providerAccountId: "workspace-a", setup: { + ...endpoint.setup!, step: "test", slackSetupMethod: method, + webhookVerifiedAt: "2026-01-01T00:00:00Z", + slackRegistration: { status: "configured", appId: "ATEST" }, + slackAccount: { externalUserId: "UPERSON", paperclipUserId: "owner-user", status: "linked", welcomeStatus: "sent", dmChannelId: "DTEST" }, + } }; + mocks.get.mockResolvedValue(saved); + mocks.finishSlackSetup.mockResolvedValue({ ...saved, status: "active", setup: { ...saved.setup, step: "complete" } }); + flushSync(() => client.setQueryData(["chat-endpoint-setup-resume", endpoint.id], saved)); + await settle(); + + await click("Done"); + + expect(mocks.finishSlackSetup).toHaveBeenCalledExactlyOnceWith("endpoint-a"); + expect(mocks.navigate).toHaveBeenCalledWith("/apps/chat/endpoint-a/settings"); + }); + it("keeps settings focused on channel access and the test-message action", async () => { mocks.tab = "settings"; const endpoint = await render("slack", true); diff --git a/ui/src/pages/apps/chat/ChatEndpointSetup.tsx b/ui/src/pages/apps/chat/ChatEndpointSetup.tsx index 3932cee316e..727c2e0c1e1 100644 --- a/ui/src/pages/apps/chat/ChatEndpointSetup.tsx +++ b/ui/src/pages/apps/chat/ChatEndpointSetup.tsx @@ -475,6 +475,13 @@ function ChatSdkEndpointSetup() { tone: "error", }), }); + const finishSetup = () => { + if (provider === "slack" && endpoint?.setup?.step === "complete") { + navigate(`/apps/chat/${endpoint.id}/settings`); + return; + } + testConnection.mutate(); + }; const repairing = isChatEndpointRepairing( endpoint, @@ -671,7 +678,7 @@ function ChatSdkEndpointSetup() {

- : navigate(`/apps/chat/${endpoint.id}/access`)} - onTest={() => testConnection.mutate()} + onTest={finishSetup} onSaveExit={() => navigate("/apps")} /> )} diff --git a/ui/storybook/fixtures/SlackSetupFixture.tsx b/ui/storybook/fixtures/SlackSetupFixture.tsx index 668923c6b68..461e974c7af 100644 --- a/ui/storybook/fixtures/SlackSetupFixture.tsx +++ b/ui/storybook/fixtures/SlackSetupFixture.tsx @@ -7,7 +7,7 @@ import { type ChatEndpoint } from "@/api/chatEndpoints"; import { storybookAgents, storybookAuthSession } from "./paperclipData"; import { defaultSlackAppConfiguration } from "@paperclipai/shared"; -export type SlackSetupScenario = "choose" | "create" | "manual" | "install" | "declined" | "approval_denied" | "manager_reauthorize" | "manager_authorization_failed" | "uncertain" | "manifest_pending" | "recovery" | "verify" | "avatar_failed" | "welcome_failed" | "success"; +export type SlackSetupScenario = "choose" | "create" | "manual" | "install" | "declined" | "approval_denied" | "manager_reauthorize" | "manager_authorization_failed" | "uncertain" | "manifest_pending" | "recovery" | "verify" | "avatar_failed" | "welcome_failed" | "success" | "complete"; /** Production wizard over a local provider fixture. No requests go to Slack. */ export function SlackSetupFixture({ scenario = "create", managed = false, managedAvailable = true, workspaceCount = 1, optionsDelayMs = 0, optionsError = false }: { scenario?: SlackSetupScenario; managed?: boolean; managedAvailable?: boolean; workspaceCount?: number; optionsDelayMs?: number; optionsError?: boolean }) { @@ -48,12 +48,13 @@ export function SlackSetupFixture({ scenario = "create", managed = false, manage if (scenario === "approval_denied") endpoint.setup!.slackRegistration!.errorCode = "slack_approval_denied"; if (scenario === "recovery") endpoint.setup!.slackRegistration = { ...endpoint.setup!.slackRegistration!, status: "credentials_saved", errorCode: "slack_configuration_incomplete" }; if (scenario === "uncertain") endpoint.setup!.slackRegistration = { status: "uncertain", ...(managed ? { managerGrantId: grantId } : {}), errorCode: "slack_creation_uncertain", managementUrl: "https://api.slack.com/apps" }; - if (["verify", "avatar_failed", "welcome_failed", "success"].includes(scenario)) installed(); - if (["avatar_failed", "welcome_failed", "success"].includes(scenario)) { + if (["verify", "avatar_failed", "welcome_failed", "success", "complete"].includes(scenario)) installed(); + if (["avatar_failed", "welcome_failed", "success", "complete"].includes(scenario)) { endpoint.setup!.webhookVerifiedAt = new Date().toISOString(); endpoint.setup!.step = "test"; endpoint.setup!.testStartedAt = new Date().toISOString(); } + if (scenario === "complete") { endpoint.status = "active"; endpoint.setup!.step = "complete"; } setSignalVerification(() => () => { endpoint.setup!.webhookVerifiedAt = new Date().toISOString(); }); window.fetch = async (input, init) => { const url = new URL(typeof input === "string" ? input : input instanceof URL ? input.href : input.url, window.location.origin); @@ -113,7 +114,10 @@ export function SlackSetupFixture({ scenario = "create", managed = false, manage if (path.endsWith("/slack-person/link-intent")) return Response.json({ confirmationUrl: "https://board.example/confirm?token=fixture" }); if (path === "/api/chat-identity-links/confirm") { linked = true; return Response.json({ ok: true }); } if (path.endsWith("/test-status")) return Response.json({ messageReceivedAt: null }); - if (path.endsWith("/finish")) { endpoint.status = "active"; endpoint.setup!.step = "complete"; return Response.json(endpoint); } + if (path.endsWith("/finish")) { + if (endpoint.status !== "verifying" || endpoint.setup!.step !== "test") return Response.json({ error: "This connection is not waiting for a setup test" }, { status: 409 }); + endpoint.status = "active"; endpoint.setup!.step = "complete"; return Response.json(endpoint); + } return original(input, init); }; setReady(true); diff --git a/ui/storybook/stories/chat-channels/slack-automatic-setup.stories.tsx b/ui/storybook/stories/chat-channels/slack-automatic-setup.stories.tsx index 117444cae60..f4f6495a6aa 100644 --- a/ui/storybook/stories/chat-channels/slack-automatic-setup.stories.tsx +++ b/ui/storybook/stories/chat-channels/slack-automatic-setup.stories.tsx @@ -63,6 +63,7 @@ export const AvatarUploadFailed: Story = { name: "Recovery · Avatar upload fail export const WelcomeFailed: Story = { name: "Recovery · Welcome DM failed", args: { scenario: "welcome_failed" } }; export const Mobile: Story = { name: "Mobile · App configuration access token", args: { scenario: "create" }, globals: { viewport: { value: "mobile1", isRotated: false } } }; +export const AlreadyConnected: Story = { ...Success, name: "04 · Already connected", args: { scenario: "complete" } }; export const SuccessMobile: Story = { name: "Mobile · Success", args: { scenario: "success" }, globals: { viewport: { value: "mobile1", isRotated: false } } }; export const ChooseConnectionMobile: Story = { diff --git a/ui/storybook/stories/chat-channels/slack-managed-setup.stories.tsx b/ui/storybook/stories/chat-channels/slack-managed-setup.stories.tsx index 84937acceee..b35b00062ce 100644 --- a/ui/storybook/stories/chat-channels/slack-managed-setup.stories.tsx +++ b/ui/storybook/stories/chat-channels/slack-managed-setup.stories.tsx @@ -62,6 +62,7 @@ export const Success: Story = { name: "02 · Connect · Success", args: { scenar await expect(await canvas.findByRole("link", { name: /^Open Slack$/ })).toHaveAttribute("href", "https://app.slack.com/client/TSTORY/DSTORY"); } }; export const ApprovalPending: Story = { name: "Recovery · Slack approval pending", args: { scenario: "declined" } }; +export const AlreadyConnected: Story = { ...Success, name: "02 · Connect · Already connected", args: { scenario: "complete" } }; export const ApprovalDeclined: Story = { name: "Recovery · Slack approval declined", args: { scenario: "approval_denied" } }; export const WorkspaceReconnect: Story = { name: "Recovery · Reconnect workspace before app creation",