mirror of
https://github.com/coder/coder.git
synced 2026-06-02 20:48:20 +00:00
fix: clear agents page draft from localStorage after chat creation (#22691)
## Problem When creating a new chat from the `/agents` page and navigating back, the initial prompt was still displayed. The draft text persisted in `localStorage` (`agents.empty-input`) even after the chat was successfully created. ### Root cause After `handleSend` synchronously clears the localStorage key, the React re-render (caused by `isCreating` flipping to `true`) triggers Lexical's `ContentChangePlugin`, which fires `handleContentChange` with the old editor content — re-writing the draft back to localStorage before navigation occurs. ## Fix - Extract the draft persistence logic into a `useCreatePageDraft` hook with a `sentRef` guard - Once `markSent()` is called, `handleContentChange` skips all localStorage writes - This prevents the Lexical editor's change events from re-persisting the draft during the async gap between send and navigation ## Testing Added 6 unit tests for `useCreatePageDraft`, including a regression test that reproduces the exact bug scenario (calling `handleContentChange` with old content after `markSent`).
This commit is contained in:
@@ -0,0 +1,165 @@
|
||||
import { act, renderHook } from "@testing-library/react";
|
||||
import { beforeEach, describe, expect, it } from "vitest";
|
||||
import { emptyInputStorageKey, useEmptyStateDraft } from "./AgentsPage";
|
||||
|
||||
describe("useEmptyStateDraft", () => {
|
||||
beforeEach(() => {
|
||||
localStorage.clear();
|
||||
});
|
||||
|
||||
const renderDraft = () => renderHook(() => useEmptyStateDraft());
|
||||
|
||||
it("reads the initial value from localStorage", () => {
|
||||
localStorage.setItem(emptyInputStorageKey, "saved draft");
|
||||
|
||||
const { result, unmount } = renderDraft();
|
||||
|
||||
expect(result.current.initialInputValue).toBe("saved draft");
|
||||
expect(result.current.getCurrentContent()).toBe("saved draft");
|
||||
unmount();
|
||||
});
|
||||
|
||||
it("returns empty string when localStorage has no draft", () => {
|
||||
const { result, unmount } = renderDraft();
|
||||
|
||||
expect(result.current.initialInputValue).toBe("");
|
||||
expect(result.current.getCurrentContent()).toBe("");
|
||||
unmount();
|
||||
});
|
||||
|
||||
it("writes content to localStorage via handleContentChange", () => {
|
||||
const { result, unmount } = renderDraft();
|
||||
|
||||
act(() => {
|
||||
result.current.handleContentChange("work in progress");
|
||||
});
|
||||
|
||||
expect(localStorage.getItem(emptyInputStorageKey)).toBe("work in progress");
|
||||
expect(result.current.getCurrentContent()).toBe("work in progress");
|
||||
unmount();
|
||||
});
|
||||
|
||||
it("removes the draft key when handleContentChange receives empty string", () => {
|
||||
localStorage.setItem(emptyInputStorageKey, "old draft");
|
||||
const { result, unmount } = renderDraft();
|
||||
|
||||
act(() => {
|
||||
result.current.handleContentChange("");
|
||||
});
|
||||
|
||||
expect(localStorage.getItem(emptyInputStorageKey)).toBeNull();
|
||||
unmount();
|
||||
});
|
||||
|
||||
it("clears the draft from localStorage when submitDraft is called", () => {
|
||||
localStorage.setItem(emptyInputStorageKey, "draft to clear");
|
||||
const { result, unmount } = renderDraft();
|
||||
|
||||
expect(localStorage.getItem(emptyInputStorageKey)).toBe("draft to clear");
|
||||
|
||||
act(() => {
|
||||
result.current.submitDraft();
|
||||
});
|
||||
|
||||
expect(localStorage.getItem(emptyInputStorageKey)).toBeNull();
|
||||
unmount();
|
||||
});
|
||||
|
||||
it("does not re-persist the draft after submitDraft is called", () => {
|
||||
localStorage.setItem(emptyInputStorageKey, "fix the bug");
|
||||
const { result, unmount } = renderDraft();
|
||||
|
||||
// Simulate handleSend: submitDraft clears the draft.
|
||||
act(() => {
|
||||
result.current.submitDraft();
|
||||
});
|
||||
expect(localStorage.getItem(emptyInputStorageKey)).toBeNull();
|
||||
|
||||
// Simulate the Lexical ContentChangePlugin firing during
|
||||
// the re-render with the old content. Without the sentRef
|
||||
// guard this would re-persist the draft.
|
||||
act(() => {
|
||||
result.current.handleContentChange("fix the bug");
|
||||
});
|
||||
|
||||
expect(localStorage.getItem(emptyInputStorageKey)).toBeNull();
|
||||
expect(result.current.getCurrentContent()).toBe("fix the bug");
|
||||
unmount();
|
||||
});
|
||||
|
||||
it("ignores all handleContentChange calls after submitDraft, even with new content", () => {
|
||||
const { result, unmount } = renderDraft();
|
||||
|
||||
act(() => {
|
||||
result.current.handleContentChange("original");
|
||||
});
|
||||
expect(localStorage.getItem(emptyInputStorageKey)).toBe("original");
|
||||
|
||||
act(() => {
|
||||
result.current.submitDraft();
|
||||
});
|
||||
|
||||
act(() => {
|
||||
result.current.handleContentChange("totally new content");
|
||||
});
|
||||
expect(localStorage.getItem(emptyInputStorageKey)).toBeNull();
|
||||
unmount();
|
||||
});
|
||||
|
||||
it("returns empty draft when remounting after submitDraft", () => {
|
||||
localStorage.setItem(emptyInputStorageKey, "draft before send");
|
||||
const { result, unmount } = renderDraft();
|
||||
|
||||
act(() => {
|
||||
result.current.submitDraft();
|
||||
});
|
||||
unmount();
|
||||
|
||||
// Simulate returning to the page.
|
||||
const { result: fresh, unmount: unmountFresh } = renderDraft();
|
||||
expect(fresh.current.initialInputValue).toBe("");
|
||||
expect(localStorage.getItem(emptyInputStorageKey)).toBeNull();
|
||||
unmountFresh();
|
||||
});
|
||||
|
||||
it("re-enables draft persistence after resetDraft", () => {
|
||||
const { result, unmount } = renderDraft();
|
||||
|
||||
act(() => {
|
||||
result.current.handleContentChange("attempt one");
|
||||
});
|
||||
expect(localStorage.getItem(emptyInputStorageKey)).toBe("attempt one");
|
||||
|
||||
act(() => {
|
||||
result.current.submitDraft();
|
||||
});
|
||||
expect(localStorage.getItem(emptyInputStorageKey)).toBeNull();
|
||||
|
||||
// Simulate error recovery — re-enable persistence.
|
||||
act(() => {
|
||||
result.current.resetDraft();
|
||||
});
|
||||
|
||||
act(() => {
|
||||
result.current.handleContentChange("attempt two");
|
||||
});
|
||||
expect(localStorage.getItem(emptyInputStorageKey)).toBe("attempt two");
|
||||
unmount();
|
||||
});
|
||||
|
||||
it("handles submitDraft being called twice without error", () => {
|
||||
localStorage.setItem(emptyInputStorageKey, "draft");
|
||||
const { result, unmount } = renderDraft();
|
||||
|
||||
act(() => {
|
||||
result.current.submitDraft();
|
||||
});
|
||||
expect(localStorage.getItem(emptyInputStorageKey)).toBeNull();
|
||||
|
||||
act(() => {
|
||||
result.current.submitDraft();
|
||||
});
|
||||
expect(localStorage.getItem(emptyInputStorageKey)).toBeNull();
|
||||
unmount();
|
||||
});
|
||||
});
|
||||
@@ -62,7 +62,7 @@ import { useAgentsPWA } from "./useAgentsPWA";
|
||||
import { WebPushButton } from "./WebPushButton";
|
||||
|
||||
/** @internal Exported for testing. */
|
||||
const emptyInputStorageKey = "agents.empty-input";
|
||||
export const emptyInputStorageKey = "agents.empty-input";
|
||||
const selectedWorkspaceIdStorageKey = "agents.selected-workspace-id";
|
||||
const lastModelConfigIDStorageKey = "agents.last-model-config-id";
|
||||
const systemPromptStorageKey = "agents.system-prompt";
|
||||
@@ -339,7 +339,6 @@ const AgentsPage: FC = () => {
|
||||
});
|
||||
|
||||
if (typeof window !== "undefined") {
|
||||
localStorage.removeItem(emptyInputStorageKey);
|
||||
if (modelConfigID !== nilUUID) {
|
||||
localStorage.setItem(lastModelConfigIDStorageKey, modelConfigID);
|
||||
} else {
|
||||
@@ -602,6 +601,60 @@ const AgentsPage: FC = () => {
|
||||
);
|
||||
};
|
||||
|
||||
/**
|
||||
* Hook that manages draft persistence for the empty-state chat input.
|
||||
* Persists the current input to localStorage so the user's draft
|
||||
* survives page reloads.
|
||||
*
|
||||
* Once `submitDraft` is called, the stored draft is removed and further
|
||||
* content changes are no longer persisted for the lifetime of the hook.
|
||||
* Call `resetDraft` to re-enable persistence (e.g. on mutation failure).
|
||||
*
|
||||
* @internal Exported for testing.
|
||||
*/
|
||||
export function useEmptyStateDraft() {
|
||||
const [initialInputValue] = useState(() => {
|
||||
if (typeof window === "undefined") {
|
||||
return "";
|
||||
}
|
||||
return localStorage.getItem(emptyInputStorageKey) ?? "";
|
||||
});
|
||||
const inputValueRef = useRef(initialInputValue);
|
||||
const sentRef = useRef(false);
|
||||
|
||||
const handleContentChange = useCallback((content: string) => {
|
||||
inputValueRef.current = content;
|
||||
if (typeof window !== "undefined" && !sentRef.current) {
|
||||
if (content) {
|
||||
localStorage.setItem(emptyInputStorageKey, content);
|
||||
} else {
|
||||
localStorage.removeItem(emptyInputStorageKey);
|
||||
}
|
||||
}
|
||||
}, []);
|
||||
|
||||
const submitDraft = useCallback(() => {
|
||||
// Mark as sent so that editor change events firing during
|
||||
// the async gap cannot re-persist the draft.
|
||||
sentRef.current = true;
|
||||
localStorage.removeItem(emptyInputStorageKey);
|
||||
}, []);
|
||||
|
||||
const resetDraft = useCallback(() => {
|
||||
sentRef.current = false;
|
||||
}, []);
|
||||
|
||||
const getCurrentContent = useCallback(() => inputValueRef.current, []);
|
||||
|
||||
return {
|
||||
initialInputValue,
|
||||
getCurrentContent,
|
||||
handleContentChange,
|
||||
submitDraft,
|
||||
resetDraft,
|
||||
};
|
||||
}
|
||||
|
||||
interface AgentsEmptyStateProps {
|
||||
onCreateChat: (options: CreateChatOptions) => Promise<void>;
|
||||
isCreating: boolean;
|
||||
@@ -633,13 +686,8 @@ export const AgentsEmptyState: FC<AgentsEmptyStateProps> = ({
|
||||
isConfigureAgentsDialogOpen,
|
||||
onConfigureAgentsDialogOpenChange,
|
||||
}) => {
|
||||
const [initialInputValue] = useState(() => {
|
||||
if (typeof window === "undefined") {
|
||||
return "";
|
||||
}
|
||||
return localStorage.getItem(emptyInputStorageKey) ?? "";
|
||||
});
|
||||
const inputValueRef = useRef(initialInputValue);
|
||||
const { initialInputValue, handleContentChange, submitDraft, resetDraft } =
|
||||
useEmptyStateDraft();
|
||||
const initialSystemPrompt = () => {
|
||||
if (typeof window === "undefined") {
|
||||
return "";
|
||||
@@ -782,16 +830,6 @@ export const AgentsEmptyState: FC<AgentsEmptyStateProps> = ({
|
||||
}
|
||||
};
|
||||
|
||||
const handleContentChange = useCallback((content: string) => {
|
||||
inputValueRef.current = content;
|
||||
if (typeof window !== "undefined") {
|
||||
if (content) {
|
||||
localStorage.setItem(emptyInputStorageKey, content);
|
||||
} else {
|
||||
localStorage.removeItem(emptyInputStorageKey);
|
||||
}
|
||||
}
|
||||
}, []);
|
||||
const handleModelChange = useCallback((value: string) => {
|
||||
setHasUserSelectedModel(true);
|
||||
setUserSelectedModel(value);
|
||||
@@ -818,17 +856,18 @@ export const AgentsEmptyState: FC<AgentsEmptyStateProps> = ({
|
||||
|
||||
const handleSend = useCallback(
|
||||
(message: string) => {
|
||||
// Clear the draft synchronously before the async
|
||||
// onCreateChat call so that editor change events
|
||||
// firing during the async gap cannot re-persist it.
|
||||
localStorage.removeItem(emptyInputStorageKey);
|
||||
submitDraft();
|
||||
void onCreateChat({
|
||||
message,
|
||||
workspaceId: selectedWorkspaceIdRef.current ?? undefined,
|
||||
model: selectedModelRef.current || undefined,
|
||||
}).catch(() => {
|
||||
// Re-enable draft persistence so the user can edit
|
||||
// and retry after a failed send attempt.
|
||||
resetDraft();
|
||||
});
|
||||
},
|
||||
[onCreateChat],
|
||||
[submitDraft, resetDraft, onCreateChat],
|
||||
);
|
||||
|
||||
const selectedWorkspace = selectedWorkspaceId
|
||||
|
||||
Reference in New Issue
Block a user