mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 10:13:11 +02:00
## Thinking Path > - Paperclip lets people manage AI agents and their work. > - The issue page uses an external-store adapter for comments. > - The adapter must keep one identity during an unrelated render. > - The adapter must also call the newest send and cancel functions. > - Passive effects update those functions too late for a synchronous runtime call. > - This pull request updates the function refs during render and adds a regression test. > - The benefit is a stable comment thread with no stale comment action. ## Linked Issues or Issue Description Refs #3678 **What happened?** The issue comment runtime can call the prior send function after a render. The callback refs do not update until passive effects run. **Expected behavior** The stable runtime adapter must call the newest function as soon as the render supplies it. **Steps to reproduce** 1. Render the issue runtime with one send function. 2. Render it again with a new send function and the same thread data. 3. Call the stable adapter before passive effects run. 4. Observe that the prior send function runs. **Paperclip version or commit** `6395cae072` **Deployment mode** Local development from source. **Agent adapter(s) involved** This is a core UI bug. It is not adapter-specific. ## What Changed - Update the latest send and cancel refs during render. - Keep the external-store adapter stable across callback-only renders. - Add a test for a runtime callback before passive effects run. ## Verification - `pnpm --filter @paperclipai/ui exec vitest run src/hooks/usePaperclipIssueRuntime.test.tsx` - `pnpm --filter @paperclipai/ui typecheck` - `pnpm check:token-gates` - `pnpm -r typecheck` - `pnpm test:run` - `pnpm build` ## Risks - Low risk. The change only updates two refs earlier in the same render. - The adapter identity and its data dependencies do not change. > This bug fix does not add or overlap with a roadmap feature. ## Model Used - OpenAI Codex with GPT-5. The exact deployed model ID and context window are not exposed. Reasoning, tool use, and local code execution were enabled. ## 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 <noreply@paperclip.ing>
306 lines
8.4 KiB
TypeScript
306 lines
8.4 KiB
TypeScript
// @vitest-environment jsdom
|
|
|
|
import { act, useLayoutEffect } from "react";
|
|
import { createRoot } from "react-dom/client";
|
|
import { afterEach, describe, expect, it, vi } from "vitest";
|
|
import type {
|
|
AppendMessage,
|
|
ExternalStoreAdapter,
|
|
ThreadMessage,
|
|
} from "@assistant-ui/react";
|
|
import { usePaperclipIssueRuntime } from "./usePaperclipIssueRuntime";
|
|
|
|
const { externalStoreRuntimeState, useExternalStoreRuntimeMock } = vi.hoisted(
|
|
() => {
|
|
const state: {
|
|
adapter: ExternalStoreAdapter<ThreadMessage> | undefined;
|
|
} = { adapter: undefined };
|
|
return {
|
|
externalStoreRuntimeState: state,
|
|
useExternalStoreRuntimeMock: vi.fn(
|
|
(adapter: ExternalStoreAdapter<ThreadMessage>) => {
|
|
state.adapter = adapter;
|
|
return { kind: "runtime" };
|
|
},
|
|
),
|
|
};
|
|
},
|
|
);
|
|
|
|
vi.mock("@assistant-ui/react", () => ({
|
|
useExternalStoreRuntime: useExternalStoreRuntimeMock,
|
|
}));
|
|
|
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true;
|
|
|
|
function HookHarness({
|
|
messages,
|
|
isRunning,
|
|
onSend,
|
|
onCancel,
|
|
onLayout,
|
|
}: {
|
|
messages: readonly ThreadMessage[];
|
|
isRunning: boolean;
|
|
onSend: (options: {
|
|
body: string;
|
|
reopen?: boolean;
|
|
reassignment?: {
|
|
assigneeAgentId: string | null;
|
|
assigneeUserId: string | null;
|
|
};
|
|
}) => Promise<void>;
|
|
onCancel?: (() => Promise<void>) | undefined;
|
|
onLayout?: ((adapter: ExternalStoreAdapter<ThreadMessage>) => void) | undefined;
|
|
}) {
|
|
usePaperclipIssueRuntime({
|
|
messages,
|
|
isRunning,
|
|
onSend,
|
|
onCancel,
|
|
});
|
|
useLayoutEffect(() => {
|
|
if (externalStoreRuntimeState.adapter && onLayout) {
|
|
onLayout(externalStoreRuntimeState.adapter);
|
|
}
|
|
}, [onLayout]);
|
|
return null;
|
|
}
|
|
|
|
function createAppendMessage(body: string): AppendMessage {
|
|
return {
|
|
createdAt: new Date("2026-04-11T14:00:02.000Z"),
|
|
parentId: null,
|
|
role: "user",
|
|
sourceId: null,
|
|
content: [{ type: "text", text: body }],
|
|
metadata: { custom: {} },
|
|
attachments: [],
|
|
runConfig: undefined,
|
|
};
|
|
}
|
|
|
|
function createUserMessage(id: string, text: string): ThreadMessage {
|
|
return {
|
|
id,
|
|
role: "user",
|
|
content: [{ type: "text", text }],
|
|
metadata: { custom: {} },
|
|
attachments: [],
|
|
createdAt: new Date("2026-04-11T14:00:00.000Z"),
|
|
} as unknown as ThreadMessage;
|
|
}
|
|
|
|
function createAssistantMessage(id: string, text: string): ThreadMessage {
|
|
return {
|
|
id,
|
|
role: "assistant",
|
|
content: [{ type: "text", text }],
|
|
metadata: { custom: {} },
|
|
status: { type: "complete", reason: "stop" },
|
|
createdAt: new Date("2026-04-11T14:00:01.000Z"),
|
|
} as unknown as ThreadMessage;
|
|
}
|
|
|
|
describe("usePaperclipIssueRuntime", () => {
|
|
afterEach(() => {
|
|
useExternalStoreRuntimeMock.mockReset();
|
|
externalStoreRuntimeState.adapter = undefined;
|
|
});
|
|
|
|
it("forwards explicit upload selection separately from Markdown content", async () => {
|
|
const container = document.createElement("div");
|
|
const root = createRoot(container);
|
|
const onSend = vi.fn(async () => {});
|
|
act(() =>
|
|
root.render(
|
|
<HookHarness messages={[]} isRunning={false} onSend={onSend} />,
|
|
),
|
|
);
|
|
const calls = useExternalStoreRuntimeMock.mock.calls as unknown as Array<
|
|
[ExternalStoreAdapter<ThreadMessage>]
|
|
>;
|
|
const adapter = calls.at(-1)![0];
|
|
const id = "9af8228f-0be7-45ae-a104-6fbe0af6f1d3";
|
|
const message = createAppendMessage("Inspect the uploaded file");
|
|
message.runConfig = { custom: { attachmentIds: [id], reopen: true } };
|
|
await adapter.onNew?.(message);
|
|
expect(onSend).toHaveBeenLastCalledWith({
|
|
body: "Inspect the uploaded file",
|
|
reopen: true,
|
|
reassignment: undefined,
|
|
attachmentIds: [id],
|
|
});
|
|
await adapter.onNew?.(
|
|
createAppendMessage(`[old](/api/attachments/${id}/content)`),
|
|
);
|
|
expect(onSend).toHaveBeenLastCalledWith({
|
|
body: `[old](/api/attachments/${id}/content)`,
|
|
reopen: undefined,
|
|
reassignment: undefined,
|
|
});
|
|
act(() => root.unmount());
|
|
});
|
|
|
|
it("keeps the external-store adapter stable across unrelated rerenders", async () => {
|
|
const container = document.createElement("div");
|
|
document.body.appendChild(container);
|
|
const root = createRoot(container);
|
|
const messages: ThreadMessage[] = [createUserMessage("message-1", "hello")];
|
|
const firstOnSend = vi.fn(async () => {});
|
|
const secondOnSend = vi.fn(async () => {});
|
|
|
|
act(() => {
|
|
root.render(
|
|
<HookHarness
|
|
messages={messages}
|
|
isRunning={false}
|
|
onSend={firstOnSend}
|
|
/>,
|
|
);
|
|
});
|
|
|
|
const runtimeCalls = useExternalStoreRuntimeMock.mock
|
|
.calls as unknown as Array<[ExternalStoreAdapter<ThreadMessage>]>;
|
|
expect(runtimeCalls.length).toBeGreaterThanOrEqual(1);
|
|
const firstAdapter = runtimeCalls[0]![0];
|
|
expect(firstAdapter).toBeTruthy();
|
|
|
|
act(() => {
|
|
root.render(
|
|
<HookHarness
|
|
messages={messages}
|
|
isRunning={false}
|
|
onSend={secondOnSend}
|
|
/>,
|
|
);
|
|
});
|
|
|
|
expect(runtimeCalls.length).toBeGreaterThanOrEqual(2);
|
|
const secondAdapter = runtimeCalls[1]![0];
|
|
expect(secondAdapter).toBe(firstAdapter);
|
|
|
|
await act(async () => {
|
|
await secondAdapter.onNew?.(createAppendMessage("latest callback"));
|
|
});
|
|
|
|
expect(firstOnSend).not.toHaveBeenCalled();
|
|
expect(secondOnSend).toHaveBeenCalledWith({
|
|
body: "latest callback",
|
|
reopen: undefined,
|
|
reassignment: undefined,
|
|
});
|
|
|
|
act(() => {
|
|
root.unmount();
|
|
});
|
|
container.remove();
|
|
});
|
|
|
|
it("uses the latest callbacks before passive effects run", async () => {
|
|
const container = document.createElement("div");
|
|
document.body.appendChild(container);
|
|
const root = createRoot(container);
|
|
const messages: ThreadMessage[] = [createUserMessage("message-1", "hello")];
|
|
const firstOnSend = vi.fn(async () => {});
|
|
const secondOnSend = vi.fn(async () => {});
|
|
const firstOnCancel = vi.fn(async () => {});
|
|
const secondOnCancel = vi.fn(async () => {});
|
|
|
|
act(() => {
|
|
root.render(
|
|
<HookHarness
|
|
messages={messages}
|
|
isRunning={false}
|
|
onSend={firstOnSend}
|
|
onCancel={firstOnCancel}
|
|
/>,
|
|
);
|
|
});
|
|
|
|
let immediateSend: Promise<void> | undefined;
|
|
let immediateCancel: Promise<void> | undefined;
|
|
|
|
act(() => {
|
|
root.render(
|
|
<HookHarness
|
|
messages={messages}
|
|
isRunning={false}
|
|
onSend={secondOnSend}
|
|
onCancel={secondOnCancel}
|
|
onLayout={(adapter) => {
|
|
immediateSend = adapter.onNew?.(
|
|
createAppendMessage("layout runtime callback"),
|
|
);
|
|
immediateCancel = adapter.onCancel?.();
|
|
}}
|
|
/>,
|
|
);
|
|
});
|
|
await Promise.all([immediateSend, immediateCancel]);
|
|
|
|
expect(firstOnSend).not.toHaveBeenCalled();
|
|
expect(firstOnCancel).not.toHaveBeenCalled();
|
|
expect(secondOnSend).toHaveBeenCalledWith({
|
|
body: "layout runtime callback",
|
|
reopen: undefined,
|
|
reassignment: undefined,
|
|
});
|
|
expect(secondOnCancel).toHaveBeenCalledOnce();
|
|
|
|
act(() => {
|
|
root.unmount();
|
|
});
|
|
container.remove();
|
|
});
|
|
|
|
it("rebuilds the adapter when thread data changes", () => {
|
|
const container = document.createElement("div");
|
|
document.body.appendChild(container);
|
|
const root = createRoot(container);
|
|
const onSend = vi.fn(async () => {});
|
|
const firstMessages: ThreadMessage[] = [
|
|
createUserMessage("message-1", "hello"),
|
|
];
|
|
const secondMessages: ThreadMessage[] = [
|
|
...firstMessages,
|
|
createAssistantMessage("message-2", "world"),
|
|
];
|
|
|
|
act(() => {
|
|
root.render(
|
|
<HookHarness
|
|
messages={firstMessages}
|
|
isRunning={false}
|
|
onSend={onSend}
|
|
/>,
|
|
);
|
|
});
|
|
|
|
const runtimeCalls = useExternalStoreRuntimeMock.mock
|
|
.calls as unknown as Array<[ExternalStoreAdapter<ThreadMessage>]>;
|
|
expect(runtimeCalls.length).toBeGreaterThanOrEqual(1);
|
|
const firstAdapter = runtimeCalls[0]![0];
|
|
|
|
act(() => {
|
|
root.render(
|
|
<HookHarness
|
|
messages={secondMessages}
|
|
isRunning={false}
|
|
onSend={onSend}
|
|
/>,
|
|
);
|
|
});
|
|
|
|
expect(runtimeCalls.length).toBeGreaterThanOrEqual(2);
|
|
const secondAdapter = runtimeCalls[1]![0];
|
|
expect(secondAdapter).not.toBe(firstAdapter);
|
|
|
|
act(() => {
|
|
root.unmount();
|
|
});
|
|
container.remove();
|
|
});
|
|
});
|