diff --git a/doc/connections/SLACK-CEO-M2.md b/doc/connections/SLACK-CEO-M2.md index c6e00b6e4e6..19c91f4310c 100644 --- a/doc/connections/SLACK-CEO-M2.md +++ b/doc/connections/SLACK-CEO-M2.md @@ -53,7 +53,12 @@ and suggest up to three small fixes, with source links and uncertainty. Specify discovery only: no editing, delegation, new tasks, PR or merge. Expect one Connect Slack card in Slack and the canonical Paperclip task. The -link hands off through the authenticated task to personal OAuth. After the user +link hands off automatically through the authenticated task to personal OAuth; +it must not require a second Connect Slack click in Paperclip. Sign-in is still +required if the browser has no session. The credential-free interaction suffix +must survive UUID-to-identifier and company-prefix redirects. Consume it once +before starting OAuth; settled requests, different addressees and ordinary task +visits must not start authorization. After the user approves, the existing durable intent delivery resumes the original request. The answer keeps its original audience. On legacy v1 tasks, the normal Paperclip composer stays internal and Reply via Slack opts in to that turn's Slack reply. @@ -96,6 +101,27 @@ Slack grant. Do not rotate/revoke the existing bot token to reset a read test. ## Verification +September 23 handoff correction: the task's canonical-URL redirect was dropping +`/connect-slack-read/:interactionId` from the actual UUID-based Slack link. The +redirect now preserves an unconsumed suffix and does not restore one already +consumed by the card. Regression tests reproduce the old failure and exercise +UUID/wrong-company resolution followed by the real card's automatic start, +remount/StrictMode deduplication, ineligible audiences/states and explicit retry +after failure. All 154 tests in IssueDetail and ConnectionIntentInteractionBody, +UI TypeScript, token gates and the UI production build pass. Provider scopes, +OAuth state/session binding and continuation handling are unchanged. A new live +OAuth run is not claimed by these tests; do not reset a working grant just to +exercise this fix without the user's approval. + +Deployed only to the isolated port-3210 pilot after rebuilding its UI. Live +browser smoke: TES-10's actual unprefixed UUID/interaction URL resolved to +`/TES/issues/TES-10/connect-slack-read/:interactionId` with the suffix intact. +Its already-accepted card stayed connected and did not start another OAuth flow. +The browser was returned to the ordinary TES-10 URL; no grant was reset. +Subsequently, at the user's explicit request, only the personal channel-read +connection was removed through Paperclip to prepare a fresh-consent retest. +The CEO chat connection remained active; the user-led fresh OAuth test is pending. + See [the implementation plan](../plans/2026-09-22-slack-ceo-read-discovery.md) for the current automated/live split and provider references. Run `slack-read-profile.test.ts`, `slack-read-discovery.test.ts`, connection-intent diff --git a/doc/plans/2026-09-22-slack-ceo-threaded-research.md b/doc/plans/2026-09-22-slack-ceo-threaded-research.md index bf53952f7ce..575a36508f1 100644 --- a/doc/plans/2026-09-22-slack-ceo-threaded-research.md +++ b/doc/plans/2026-09-22-slack-ceo-threaded-research.md @@ -8,6 +8,15 @@ commit: `91339784ce9a740c149b34f013a1e29e7e7febdc`. ## Current checkpoint: threaded-DM preview +September 23 direct-consent correction: preserve the credential-free Slack +handoff suffix during task UUID/company canonicalization. The existing addressed +card then automatically starts the session-bound personal OAuth request, without +a second Paperclip Connect click. Already-consumed handoffs are not restored; +ordinary visits and settled/non-addressed cards do not auto-start. This is a +routing-only fix, not a permission or research-scope expansion. The two relevant +UI suites pass all 154 tests; UI TypeScript, token gates and the production build +pass. Fresh live OAuth approval remains a separate test, not inferred from mocks. + The user's immediate request is to test a new Slack DM with a receipt, its own thread and editable status. The isolated port-3210 pilot now runs this narrower preview. Its database was backed up privately before applying migration 0284; diff --git a/ui/src/features/connections/ConnectionIntentInteractionBody.test.tsx b/ui/src/features/connections/ConnectionIntentInteractionBody.test.tsx index c5252122089..1bf35573bbe 100644 --- a/ui/src/features/connections/ConnectionIntentInteractionBody.test.tsx +++ b/ui/src/features/connections/ConnectionIntentInteractionBody.test.tsx @@ -1,6 +1,6 @@ // @vitest-environment jsdom -import { act as reactAct, useState, type ReactNode } from "react"; +import { act as reactAct, StrictMode, useState, type ReactNode } from "react"; import { createRoot, type Root } from "react-dom/client"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import type { @@ -212,6 +212,52 @@ describe("ConnectionIntentInteractionBody states and audience", () => { expect(slackReadMock).toHaveBeenCalledWith(interaction.id); expect(document.querySelector("[data-testid=shared-connection-setup]")).toBeNull(); }); + it("starts a pending Slack handoff only once under StrictMode", async () => { + const interaction = { ...pendingConnectionIntentInteraction, payload: { + ...pendingConnectionIntentInteraction.payload, capabilityProfile: "slack-public-read-v1" as const, + } }; + window.history.replaceState(null, "", `/TES/issues/TES-10/connect-slack-read/${interaction.id}`); + renderNode(); + await flush(); + expect(slackReadMock).toHaveBeenCalledTimes(1); + expect(window.location.pathname).toBe("/TES/issues/TES-10"); + }); + it.each([ + { user: null, status: "pending", matching: true }, + { user: "another-user", status: "pending", matching: true }, + { user: pendingConnectionIntentInteraction.addresseeUserId, status: "accepted", matching: true }, + { user: pendingConnectionIntentInteraction.addresseeUserId, status: "rejected", matching: true }, + { user: pendingConnectionIntentInteraction.addresseeUserId, status: "expired", matching: true }, + { user: pendingConnectionIntentInteraction.addresseeUserId, status: "pending", matching: false }, + ] as const)("does not auto-start an ineligible handoff: %j", async ({ user, status, matching }) => { + const interaction = { ...pendingConnectionIntentInteraction, status, payload: { + ...pendingConnectionIntentInteraction.payload, capabilityProfile: "slack-public-read-v1" as const, + } } as ConnectionIntentInteraction; + const pathname = `/TES/issues/TES-10/connect-slack-read/${matching ? interaction.id : "another-interaction"}`; + window.history.replaceState(null, "", pathname); + renderBody(interaction, user); + await flush(); + expect(slackReadMock).not.toHaveBeenCalled(); + expect(window.location.pathname).toBe(pathname); + }); + it("leaves a failed automatic handoff on the task for explicit retry without looping", async () => { + const interaction = { ...pendingConnectionIntentInteraction, payload: { + ...pendingConnectionIntentInteraction.payload, capabilityProfile: "slack-public-read-v1" as const, + } }; + slackReadMock.mockRejectedValue(new Error("Please sign in again")); + window.history.replaceState(null, "", `/TES/issues/TES-10/connect-slack-read/${interaction.id}`); + renderBody(interaction); + await waitForAssertion(() => expect(document.querySelector("[role=alert]")?.textContent).toContain("Please sign in again")); + expect(window.location.pathname).toBe("/TES/issues/TES-10"); + await act(() => root?.unmount()); host?.remove(); root = null; host = null; + renderBody(interaction); + await flush(); + expect(slackReadMock).toHaveBeenCalledTimes(1); + await act(async () => button("Connect Slack")!.click()); + await flush(); + expect(slackReadMock).toHaveBeenCalledTimes(2); + }); it("shows an actionable Slack authorization failure and permits declining", async () => { slackReadMock.mockRejectedValue(new Error("Source channel configuration changed")); const interaction = { ...pendingConnectionIntentInteraction, payload: { ...pendingConnectionIntentInteraction.payload, capabilityProfile: "slack-public-read-v1" as const, sourceChannelId: "CAPPROVED" } }; diff --git a/ui/src/pages/IssueDetail.test.tsx b/ui/src/pages/IssueDetail.test.tsx index e9159c1d627..4580c70d769 100644 --- a/ui/src/pages/IssueDetail.test.tsx +++ b/ui/src/pages/IssueDetail.test.tsx @@ -41,6 +41,9 @@ import { } from "../lib/issueDetailBreadcrumb"; import { getRecentTasksStorageKey, readRecentTasks } from "../lib/recent-tasks"; import { ApiError } from "../api/client"; +import { connectionIntentsApi } from "../api/connection-intents"; +import { ConnectionIntentInteractionBody } from "../features/connections/ConnectionIntentInteractionBody"; +import { pendingConnectionIntentInteraction } from "../fixtures/issueThreadInteractionFixtures"; import type { issuesApi } from "../api/issues"; const mockIssuesApi = vi.hoisted(() => ({ @@ -133,6 +136,7 @@ const mockPanelState = vi.hoisted(() => ({ panelVisible: true })); const mockRouteParams = vi.hoisted(() => ({ issueId: "PAP-1", companyPrefix: "PAP", + interactionId: undefined as string | undefined, })); const mockSidebarState = vi.hoisted(() => ({ isMobile: false })); const mockIssuePropertiesRender = vi.hoisted(() => vi.fn()); @@ -1407,6 +1411,8 @@ describe("IssueDetail", () => { mockLocation.state = null; mockRouteParams.issueId = "PAP-1"; mockRouteParams.companyPrefix = "PAP"; + mockRouteParams.interactionId = undefined; + window.history.replaceState(null, "", "/issues/PAP-1"); }); afterEach(async () => { @@ -1433,6 +1439,77 @@ describe("IssueDetail", () => { expect(mockIssuesApi.markRead).toHaveBeenCalledWith(canonical.id); }); + it.each(["PAP", "OTHER"])("preserves the Slack OAuth handoff when canonicalizing a UUID link under %s", async (prefix) => { + const issue = createIssue({ id: "7c2da70e-d1a2-491f-9f5d-f82e0f88f1d2" }); + const interaction = { + ...pendingConnectionIntentInteraction, + id: "06df12d7-86e8-44aa-af1f-6cb9bfb32af5", + issueId: issue.id, + payload: { + ...pendingConnectionIntentInteraction.payload, + capabilityProfile: "slack-public-read-v1" as const, + sourceChannelId: "CAPPROVED", + }, + }; + const handoff = `/connect-slack-read/${interaction.id}`; + // Slack publishes /issues//connect-slack-read/. The + // unprefixed board redirect adds the selected company's prefix unchanged. + window.history.replaceState(null, "", `/issues/${issue.id}${handoff}`); + mockLocation.pathname = `/${prefix}${window.location.pathname}`; + window.history.replaceState(null, "", mockLocation.pathname); + mockRouteParams.issueId = issue.id; + mockRouteParams.companyPrefix = prefix; + mockRouteParams.interactionId = interaction.id; + mockIssuesApi.get.mockResolvedValue(issue); + mockIssuesApi.listInteractions.mockResolvedValue([interaction]); + const start = vi.spyOn(connectionIntentsApi, "startSlackRead").mockResolvedValue({ + status: "CONNECTED", connectionId: "read-connection", + }); + await act(async () => { + root.render(); + }); + await waitForAssertion(() => expect(mockNavigate).toHaveBeenCalledWith( + { pathname: `/PAP/issues/PAP-1${handoff}`, search: "", hash: "" }, + { replace: true, state: null }, + )); + // No interactive card is exposed on the outgoing UUID/wrong-company view. + expect(start).not.toHaveBeenCalled(); + const destination = mockNavigate.mock.calls.at(-1)?.[0] as { pathname: string }; + window.history.replaceState(null, "", destination.pathname); + // Mount the real card on the destination. It must start without a second + // Connect click, consume the suffix, and stay consumed across a remount. + for (const key of ["first-mount", "remount"]) { + await act(async () => { + root.render( + + ); + }); + await flushReact(); + expect(start).toHaveBeenCalledTimes(1); + expect(start).toHaveBeenCalledWith(interaction.id); + expect(window.location.pathname).toBe("/PAP/issues/PAP-1"); + } + }); + + it("does not restore an already consumed Slack handoff during a later company redirect", async () => { + const interactionId = pendingConnectionIntentInteraction.id; + mockRouteParams.companyPrefix = "OTHER"; + mockRouteParams.interactionId = interactionId; + mockLocation.pathname = `/OTHER/issues/PAP-1/connect-slack-read/${interactionId}`; + // The card consumes the handoff synchronously in browser history. Router + // state can still contain the old suffix while company data finishes loading. + window.history.replaceState(null, "", "/OTHER/issues/PAP-1"); + mockIssuesApi.get.mockResolvedValue(createIssue()); + await act(async () => { + root.render(); + }); + await waitForAssertion(() => expect(mockNavigate).toHaveBeenCalledWith( + { pathname: "/PAP/issues/PAP-1", search: "", hash: "" }, + { replace: true, state: null }, + )); + }); + it.each(["message", "attachment"])("creates an unused conversation only for the first %s and updates its canonical cache", async (kind) => { mockIssuesApi.markRead.mockClear(); const agent = createAgent(); diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index bf42ee7910b..6c7530a4e17 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -2848,7 +2848,9 @@ export function IssueDetail({ tasksTab }: { tasksTab?: TaskSidePanelProps["tasks export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskSidePanelProps["tasksTab"]; conversation?: { agent: Agent; issue: Issue | null; ensureIssue: () => Promise; } }) { - const { issueId: routeIssueId, companyPrefix } = useParams<{ issueId: string; companyPrefix: string }>(); + const { issueId: routeIssueId, companyPrefix, interactionId: slackReadInteractionId } = useParams<{ + issueId: string; companyPrefix: string; interactionId?: string; + }>(); const issueId = conversation ? conversation.issue?.id : routeIssueId; const [draftWorkMode, setDraftWorkMode] = useState("standard"); const draftIssue = useMemo(() => conversation ? agentChatDraft(conversation.agent, draftWorkMode) : undefined, [conversation?.agent, draftWorkMode]); @@ -5473,7 +5475,17 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS nextState, location.search, ); - const taskPath = createIssueDetailPath(canonicalRef); + const handoffSuffix = slackReadInteractionId + ? `/connect-slack-read/${slackReadInteractionId}` + : ""; + // Slack publishes UUID task links. Keep its credential-free handoff when + // resolving the canonical task/company so the card can start OAuth without + // another click. Consult live history: the card may already have consumed + // the suffix while router state/company data is still catching up. + const pendingHandoff = handoffSuffix && window.location.pathname.endsWith(handoffSuffix) + ? handoffSuffix + : ""; + const taskPath = `${createIssueDetailPath(canonicalRef)}${pendingHandoff}`; navigate( { pathname: taskCompany @@ -5493,6 +5505,7 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS loadedIssue, loadedIssueCompany, companyPrefix, + slackReadInteractionId, issueId, navigate, location.state,