Skip to content

Commit 07ba1ef

Browse files
authored
feat(assistant): host-injected in-place connect for proposal requirements (#172)
* feat(assistant): host-injected in-place connect for proposal requirements * fix(assistant): make proposal connect resilient to null connectUrl and failing handlers
1 parent cfb49f9 commit 07ba1ef

10 files changed

Lines changed: 665 additions & 10 deletions

src/assistant/AssistantDock.tsx

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@ import type { ToolDetailRenderers } from "../web-react";
2121
import { AssistantPanel } from "./AssistantPanel";
2222
import { useAssistantLauncher } from "./launcher";
2323
import { ResizeHandle } from "./ResizeHandle";
24-
import type { AssistantTranscriptView } from "./types";
24+
import type {
25+
AssistantTranscriptView,
26+
ConnectionRequirement,
27+
ConnectRequirementResult,
28+
} from "./types";
2529
import { useAssistantChat } from "./useAssistantChat";
2630
import { useIsDesktop, usePanelWidth } from "./usePanelPrefs";
2731

@@ -36,6 +40,14 @@ export interface AssistantDockProps {
3640
renderGraph?: (yaml: string) => ReactNode;
3741
/** Called after a workflow-mutating tool is confirmed (host re-fetches its list). */
3842
onWorkflowMutation?: () => void;
43+
/** In-place connect handler for a proposal's integration requirements. The host
44+
* runs its own connect flow (OAuth popup, api-key modal, app install) and
45+
* resolves whether the requirement is now satisfied; the proposal card then
46+
* flips it to connected. Host-agnostic — when omitted, the card falls back to
47+
* navigating the requirement's connect target via `navigate`. */
48+
onConnectRequirement?: (
49+
requirement: ConnectionRequirement,
50+
) => Promise<ConnectRequirementResult>;
3951
/** Markdown renderer for assistant message content (plain text when absent). */
4052
renderMarkdown?: (content: string) => ReactNode;
4153
/** Per-tool custom detail renderers for expanded tool cards in the transcript. */
@@ -64,12 +76,16 @@ export function AssistantDock({
6476
formatMoney,
6577
renderGraph,
6678
onWorkflowMutation,
79+
onConnectRequirement,
6780
renderMarkdown,
6881
toolRenderers,
6982
renderTranscript,
7083
}: AssistantDockProps) {
7184
const { open, openAssistant, closeAssistant } = useAssistantLauncher();
72-
const chat = useAssistantChat(userId, { onWorkflowMutation });
85+
const chat = useAssistantChat(userId, {
86+
onWorkflowMutation,
87+
onConnectRequirement,
88+
});
7389

7490
const isDesktop = useIsDesktop();
7591
const { width, maxWidth, setWidth, previewWidth, nudgeWidth } =

src/assistant/AssistantPanel.test.tsx

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ function makeChat(over: Partial<AssistantState> = {}): AssistantChat {
4646
stop: vi.fn(),
4747
confirm: vi.fn(async () => {}),
4848
cancel: vi.fn(),
49+
canConnectRequirement: false,
50+
connectRequirement: vi.fn(async () => {}),
4951
reset: vi.fn(),
5052
switchThread: vi.fn(),
5153
restoring: false,
@@ -117,6 +119,83 @@ describe("AssistantPanel transcript seam", () => {
117119
expect(screen.getByRole("button", { name: "Cancel" })).toBeTruthy();
118120
});
119121

122+
it("wires a requirement's in-place connect to chat.connectRequirement when the host can connect", () => {
123+
const requirement = {
124+
provider: "slack",
125+
kind: "integration" as const,
126+
connected: false,
127+
};
128+
const proposalWithReq: PendingProposal = {
129+
proposalId: "p1",
130+
callId: "c1",
131+
name: "create_workflow",
132+
args: { yaml: "name: demo" },
133+
requirements: [requirement],
134+
};
135+
const chat = makeChat({
136+
status: "awaiting_confirm",
137+
pendingProposals: [proposalWithReq],
138+
});
139+
chat.canConnectRequirement = true;
140+
141+
renderPanel(chat, (view) => (
142+
<div>
143+
{view.pendingProposals.map((p) => (
144+
<div key={p.callId}>{view.renderProposal(p)}</div>
145+
))}
146+
</div>
147+
));
148+
149+
fireEvent.click(screen.getByRole("button", { name: /Connect/ }));
150+
expect(chat.connectRequirement).toHaveBeenCalledWith(
151+
proposalWithReq,
152+
requirement,
153+
);
154+
});
155+
156+
it("does not offer in-place connect when the host cannot connect (navigate fallback)", () => {
157+
const openSpy = vi.spyOn(window, "open").mockReturnValue(null);
158+
// An absolute connect target so the fallback opens a new tab (spied) rather
159+
// than assigning window.location (which jsdom can't navigate).
160+
const proposalWithReq: PendingProposal = {
161+
proposalId: "p1",
162+
callId: "c1",
163+
name: "create_workflow",
164+
args: { yaml: "name: demo" },
165+
requirements: [
166+
{
167+
provider: "slack",
168+
connected: false,
169+
connectUrl: "https://example.com/connect/slack",
170+
},
171+
],
172+
};
173+
const chat = makeChat({
174+
status: "awaiting_confirm",
175+
pendingProposals: [proposalWithReq],
176+
});
177+
// canConnectRequirement stays false (the makeChat default).
178+
179+
renderPanel(chat, (view) => (
180+
<div>
181+
{view.pendingProposals.map((p) => (
182+
<div key={p.callId}>{view.renderProposal(p)}</div>
183+
))}
184+
</div>
185+
));
186+
187+
fireEvent.click(screen.getByRole("button", { name: /Connect/ }));
188+
// The panel passed no onConnect, so the card keeps its navigate/open fallback
189+
// and never calls the host connect handler.
190+
expect(chat.connectRequirement).not.toHaveBeenCalled();
191+
expect(openSpy).toHaveBeenCalledWith(
192+
"https://example.com/connect/slack",
193+
"_blank",
194+
"noopener,noreferrer",
195+
);
196+
openSpy.mockRestore();
197+
});
198+
120199
it("reflects a live streaming turn in the view's isStreaming/isThinking flags", () => {
121200
let captured: AssistantTranscriptView | null = null;
122201
// A turn that has started but emitted no answer text yet reads as thinking.

src/assistant/AssistantPanel.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,13 @@ export function AssistantPanel({
282282
onConfirm={() => chat.confirm(proposal)}
283283
onCancel={() => chat.cancel(proposal)}
284284
navigate={navigate}
285+
// Offer in-place connect only when the host wired a handler; otherwise the
286+
// card keeps its navigate-to-connect-target fallback.
287+
onConnect={
288+
chat.canConnectRequirement
289+
? (requirement) => chat.connectRequirement(proposal, requirement)
290+
: undefined
291+
}
285292
renderGraph={renderGraph}
286293
/>
287294
);

src/assistant/ProposalCard.test.tsx

Lines changed: 137 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// @vitest-environment jsdom
2-
import { fireEvent, render, screen } from "@testing-library/react";
2+
import { act, fireEvent, render, screen, waitFor } from "@testing-library/react";
33
import { afterEach, beforeEach, describe, expect, it, type MockInstance, vi } from "vitest";
44
import { ProposalCard } from "./ProposalCard";
55
import type { ConnectionRequirement, PendingProposal } from "./types";
@@ -193,4 +193,140 @@ describe("ProposalCard", () => {
193193
expect(screen.getByText("GitHub App")).toBeTruthy();
194194
expect(screen.getByText("installed")).toBeTruthy();
195195
});
196+
197+
it("calls the in-place connect handler instead of navigating when onConnect is set", () => {
198+
const navigate = vi.fn();
199+
const onConnect = vi.fn(() => Promise.resolve());
200+
const req: ConnectionRequirement = { provider: "slack", connected: false };
201+
render(
202+
<ProposalCard
203+
proposal={withReq(req)}
204+
confirming={false}
205+
onConfirm={noop}
206+
onCancel={noop}
207+
navigate={navigate}
208+
onConnect={onConnect}
209+
/>,
210+
);
211+
fireEvent.click(screen.getByRole("button", { name: /Connect/ }));
212+
// In-place connect runs; the navigate/new-tab fallbacks stay untouched.
213+
expect(onConnect).toHaveBeenCalledWith(req);
214+
expect(navigate).not.toHaveBeenCalled();
215+
expect(openSpy).not.toHaveBeenCalled();
216+
});
217+
218+
it("shows a disabled busy state while the in-place connect is in flight", async () => {
219+
let resolveConnect!: () => void;
220+
const onConnect = vi.fn(
221+
() =>
222+
new Promise<void>((resolve) => {
223+
resolveConnect = resolve;
224+
}),
225+
);
226+
render(
227+
<ProposalCard
228+
proposal={withReq({ provider: "slack", connected: false })}
229+
confirming={false}
230+
onConfirm={noop}
231+
onCancel={noop}
232+
onConnect={onConnect}
233+
/>,
234+
);
235+
fireEvent.click(screen.getByRole("button", { name: /Connect/ }));
236+
const busy = screen.getByRole("button", { name: /Connecting/ });
237+
expect((busy as HTMLButtonElement).disabled).toBe(true);
238+
// Once the host resolves, the row leaves the busy state (the parent flips the
239+
// requirement to connected in the real flow; here the isolated card just
240+
// restores the actionable button).
241+
await act(async () => {
242+
resolveConnect();
243+
});
244+
expect(screen.queryByRole("button", { name: /Connecting/ })).toBeNull();
245+
expect(
246+
(screen.getByRole("button", { name: /Connect/ }) as HTMLButtonElement)
247+
.disabled,
248+
).toBe(false);
249+
});
250+
251+
it("falls back to navigation when no onConnect handler is provided", () => {
252+
const navigate = vi.fn();
253+
render(
254+
<ProposalCard
255+
proposal={withReq({ provider: "slack", connected: false })}
256+
confirming={false}
257+
onConfirm={noop}
258+
onCancel={noop}
259+
navigate={navigate}
260+
/>,
261+
);
262+
fireEvent.click(screen.getByRole("button", { name: /Connect/ }));
263+
expect(navigate).toHaveBeenCalledWith("/app/integrations");
264+
});
265+
266+
it("offers in-place connect even when connectUrl is null (host owns the flow)", () => {
267+
const onConnect = vi.fn(() => Promise.resolve());
268+
render(
269+
<ProposalCard
270+
proposal={withReq({
271+
provider: "github",
272+
kind: "github_app",
273+
connected: false,
274+
connectUrl: null,
275+
})}
276+
confirming={false}
277+
onConfirm={noop}
278+
onCancel={noop}
279+
onConnect={onConnect}
280+
/>,
281+
);
282+
// With a host handler the row is actionable even with no connect URL.
283+
fireEvent.click(screen.getByRole("button", { name: /Install/ }));
284+
expect(onConnect).toHaveBeenCalled();
285+
});
286+
287+
it("clears the busy state when the in-place handler rejects (no unhandled rejection)", async () => {
288+
const onConnect = vi.fn(() => Promise.reject(new Error("connect failed")));
289+
render(
290+
<ProposalCard
291+
proposal={withReq({ provider: "slack", connected: false })}
292+
confirming={false}
293+
onConfirm={noop}
294+
onCancel={noop}
295+
onConnect={onConnect}
296+
/>,
297+
);
298+
fireEvent.click(screen.getByRole("button", { name: /Connect/ }));
299+
// The rejection is contained; the row leaves the busy state and stays usable.
300+
await waitFor(() =>
301+
expect(screen.queryByRole("button", { name: /Connecting/ })).toBeNull(),
302+
);
303+
expect(
304+
(screen.getByRole("button", { name: /Connect/ }) as HTMLButtonElement)
305+
.disabled,
306+
).toBe(false);
307+
});
308+
309+
it("clears the busy state when the in-place handler throws synchronously", async () => {
310+
const onConnect = vi.fn(() => {
311+
throw new Error("sync boom");
312+
});
313+
render(
314+
<ProposalCard
315+
proposal={withReq({ provider: "slack", connected: false })}
316+
confirming={false}
317+
onConfirm={noop}
318+
onCancel={noop}
319+
onConnect={onConnect}
320+
/>,
321+
);
322+
fireEvent.click(screen.getByRole("button", { name: /Connect/ }));
323+
await waitFor(() =>
324+
expect(screen.queryByRole("button", { name: /Connecting/ })).toBeNull(),
325+
);
326+
expect(onConnect).toHaveBeenCalled();
327+
expect(
328+
(screen.getByRole("button", { name: /Connect/ }) as HTMLButtonElement)
329+
.disabled,
330+
).toBe(false);
331+
});
196332
});

0 commit comments

Comments
 (0)