fix(ui): prevent false agent instruction saves (#12502)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Operators edit each agent instruction bundle in the agent detail page > - The rich Markdown editor can normalize content and emit an onChange event while it mounts > - Paperclip treated that editor event as a user edit and retained the dirty state after the tab unmounted > - This pull request accepts rich-editor changes only after real user interaction and clears shared edit state when the instructions tab closes > - The benefit is that opening instructions or moving between agent tabs no longer shows false save controls or navigation warnings ## Linked Issues or Issue Description **What happened?** Opening an agent Instructions page could mark the page as edited without user input. The page showed Save and Cancel controls and warned about unsaved changes during unrelated tab navigation. The shared edit callbacks could remain active after the Instructions tab unmounted. **Expected behavior** Opening an instruction file must not create a draft. Save controls and navigation warnings must appear only after a user changes content. Leaving the Instructions tab must clear its shared dirty, saving, save, and cancel state. **Steps to reproduce** 1. Open an agent Instructions tab with a Markdown entry file. 2. Do not edit the file. 3. Move to another agent tab or navigate away. 4. Observe false save controls or an unsaved-changes prompt. **Paperclip version or commit** Current `master` before this change. **Deployment mode** Self-hosted server and local development. ## What Changed - Ignore rich Markdown editor normalization events until keyboard, pointer, paste, input, drop, or before-input interaction occurs. - Reset the interaction guard when the selected file, agent, or persisted content changes. - Clear the parent dirty, saving, save, and cancel state when the Instructions tab unmounts. - Add regression tests for mount normalization and cross-tab state cleanup. ## Verification - `pnpm --filter @paperclipai/ui exec vitest run src/pages/AgentDetail.instructions.test.tsx src/pages/AgentDetail.liveRun.test.ts src/pages/AgentDetail.progress.test.ts src/components/MarkdownEditor.test.tsx` — 76 tests passed. - `pnpm -r typecheck` — passed. - `pnpm build` — passed. - `pnpm test:run` — the relevant UI tests passed. The local full runner reproduced unrelated workspace-runtime failures present on `master`; GitHub CI is the authoritative isolated full-suite gate. - `pnpm check:token-gates` — the changed files are clean. The command reports nine existing color literals in `ui/src/components/onboarding/PillGuy.tsx` from `master`. ## Risks - Low risk. The change affects only local instruction-editor dirty-state tracking. - The interaction guard covers keyboard, pointer, paste, input, drop, and before-input events. - There are no API, database, migration, or visual design changes. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex, GPT-5 family. The deployment model identifier and context-window size are not exposed in this session. The agent used reasoning, repository tools, shell execution, and test execution. ## 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>
This commit is contained in:
Dotta
GitHub
parent
cf6db7b523
commit
9e4be0e60c
|
|
@ -62,6 +62,7 @@ vi.mock("../components/MarkdownEditor", () => ({
|
|||
}) => {
|
||||
markdownEditorRenderMock({
|
||||
value,
|
||||
onChange,
|
||||
contentClassName,
|
||||
hasImageUploadHandler: Boolean(imageUploadHandler),
|
||||
});
|
||||
|
|
@ -301,6 +302,7 @@ describe("PromptsTab instruction editor", () => {
|
|||
}));
|
||||
|
||||
await act(async () => {
|
||||
editor.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true }));
|
||||
setNativeValue(editor, "# Updated");
|
||||
});
|
||||
await waitFor(() => {
|
||||
|
|
@ -321,6 +323,75 @@ describe("PromptsTab instruction editor", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("ignores rich-editor mount normalization until the user interacts", async () => {
|
||||
const summary = makeSummary("AGENTS.md", "AGENTS.md");
|
||||
const onDirtyChange = vi.fn();
|
||||
await renderPromptsTab(
|
||||
makeBundle("AGENTS.md", [summary]),
|
||||
{ "AGENTS.md": makeDetail(summary, "# Current") },
|
||||
{ onDirtyChange },
|
||||
);
|
||||
|
||||
const editorProps = await waitFor(() => {
|
||||
const latest = markdownEditorRenderMock.mock.calls.at(-1)?.[0] as
|
||||
| { onChange?: (value: string) => void }
|
||||
| undefined;
|
||||
expect(latest?.onChange).toEqual(expect.any(Function));
|
||||
return latest!;
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
editorProps.onChange?.("# Current\n");
|
||||
});
|
||||
await flushReact();
|
||||
|
||||
expect(onDirtyChange).not.toHaveBeenCalledWith(true);
|
||||
expect(saveAction).toBeNull();
|
||||
expect(mockAgentsApi.saveInstructionsFile).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("releases dirty state and save controls when the instructions tab unmounts", async () => {
|
||||
const summary = makeSummary("AGENTS.md", "AGENTS.md");
|
||||
const onDirtyChange = vi.fn();
|
||||
const onSavingChange = vi.fn();
|
||||
let cancelAction: (() => void) | null = null;
|
||||
await renderPromptsTab(
|
||||
makeBundle("AGENTS.md", [summary]),
|
||||
{ "AGENTS.md": makeDetail(summary, "# Current") },
|
||||
{
|
||||
onDirtyChange,
|
||||
onSavingChange,
|
||||
onSaveActionChange: (next) => { saveAction = next; },
|
||||
onCancelActionChange: (next) => { cancelAction = next; },
|
||||
},
|
||||
);
|
||||
|
||||
const editor = await waitFor(() => {
|
||||
const candidate = container.querySelector<HTMLTextAreaElement>('[data-testid="markdown-editor"]');
|
||||
expect(candidate).not.toBeNull();
|
||||
return candidate!;
|
||||
});
|
||||
await act(async () => {
|
||||
editor.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true }));
|
||||
setNativeValue(editor, "# Updated");
|
||||
});
|
||||
await waitFor(() => {
|
||||
expect(onDirtyChange).toHaveBeenLastCalledWith(true);
|
||||
expect(saveAction).toEqual(expect.any(Function));
|
||||
expect(cancelAction).toEqual(expect.any(Function));
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
root?.unmount();
|
||||
});
|
||||
root = null;
|
||||
|
||||
expect(onDirtyChange).toHaveBeenLastCalledWith(false);
|
||||
expect(onSavingChange).toHaveBeenLastCalledWith(false);
|
||||
expect(saveAction).toBeNull();
|
||||
expect(cancelAction).toBeNull();
|
||||
});
|
||||
|
||||
it("uses the Markdown editor for pending new .md files before server metadata exists", async () => {
|
||||
const summary = makeSummary("settings.json", "settings.json", {
|
||||
language: "json",
|
||||
|
|
|
|||
|
|
@ -2303,7 +2303,7 @@ export function PromptsTab({
|
|||
const queryClient = useQueryClient();
|
||||
const { selectedCompanyId } = useCompany();
|
||||
const { isMobile } = useSidebar();
|
||||
const [selectedFile, setSelectedFile] = useState<string>("AGENTS.md");
|
||||
const [selectedFile, setSelectedFileState] = useState<string>("AGENTS.md");
|
||||
const [showFilePanel, setShowFilePanel] = useState(false);
|
||||
const [draft, setDraft] = useState<string | null>(null);
|
||||
const [bundleDraft, setBundleDraft] = useState<{
|
||||
|
|
@ -2325,8 +2325,20 @@ export function PromptsTab({
|
|||
entryFile: string;
|
||||
selectedFile: string;
|
||||
} | null>(null);
|
||||
// MDXEditor can normalize markdown and emit onChange while it mounts. Only
|
||||
// treat editor output as a draft after a real interaction so merely opening
|
||||
// an instructions file cannot mark the agent dirty.
|
||||
const editorInteractedRef = useRef(false);
|
||||
const markEditorInteracted = useCallback(() => {
|
||||
editorInteractedRef.current = true;
|
||||
}, []);
|
||||
const setSelectedFile = useCallback((filePath: string) => {
|
||||
editorInteractedRef.current = false;
|
||||
setSelectedFileState(filePath);
|
||||
}, []);
|
||||
|
||||
useEffect(() => {
|
||||
editorInteractedRef.current = false;
|
||||
setSelectedFile("AGENTS.md");
|
||||
setShowFilePanel(false);
|
||||
setDraft(null);
|
||||
|
|
@ -2393,7 +2405,10 @@ export function PromptsTab({
|
|||
entryFile?: string;
|
||||
clearLegacyPromptTemplate?: boolean;
|
||||
}) => agentsApi.updateInstructionsBundle(agent.id, data, companyId),
|
||||
onMutate: () => setAwaitingRefresh(true),
|
||||
onMutate: () => {
|
||||
editorInteractedRef.current = false;
|
||||
setAwaitingRefresh(true);
|
||||
},
|
||||
onSuccess: () => {
|
||||
queryClient.invalidateQueries({ queryKey: queryKeys.agents.instructionsBundle(agent.id) });
|
||||
queryClient.invalidateQueries({ queryKey: queryKeys.agents.detail(agent.id) });
|
||||
|
|
@ -2405,7 +2420,10 @@ export function PromptsTab({
|
|||
const saveFile = useMutation({
|
||||
mutationFn: (data: { path: string; content: string; clearLegacyPromptTemplate?: boolean }) =>
|
||||
agentsApi.saveInstructionsFile(agent.id, data, companyId),
|
||||
onMutate: () => setAwaitingRefresh(true),
|
||||
onMutate: () => {
|
||||
editorInteractedRef.current = false;
|
||||
setAwaitingRefresh(true);
|
||||
},
|
||||
onSuccess: (_, variables) => {
|
||||
setPendingFiles((prev) => prev.filter((f) => f !== variables.path));
|
||||
queryClient.invalidateQueries({ queryKey: queryKeys.agents.instructionsBundle(agent.id) });
|
||||
|
|
@ -2418,7 +2436,10 @@ export function PromptsTab({
|
|||
|
||||
const deleteFile = useMutation({
|
||||
mutationFn: (relativePath: string) => agentsApi.deleteInstructionsFile(agent.id, relativePath, companyId),
|
||||
onMutate: () => setAwaitingRefresh(true),
|
||||
onMutate: () => {
|
||||
editorInteractedRef.current = false;
|
||||
setAwaitingRefresh(true);
|
||||
},
|
||||
onSuccess: (_, relativePath) => {
|
||||
queryClient.invalidateQueries({ queryKey: queryKeys.agents.instructionsBundle(agent.id) });
|
||||
queryClient.removeQueries({ queryKey: queryKeys.agents.instructionsFile(agent.id, relativePath) });
|
||||
|
|
@ -2545,6 +2566,13 @@ export function PromptsTab({
|
|||
useEffect(() => { onSavingChange(isSaving); }, [onSavingChange, isSaving]);
|
||||
useEffect(() => { onDirtyChange(isDirty); }, [onDirtyChange, isDirty]);
|
||||
|
||||
useEffect(() => () => {
|
||||
onSaveActionChange(null);
|
||||
onCancelActionChange(null);
|
||||
onDirtyChange(false);
|
||||
onSavingChange(false);
|
||||
}, [onCancelActionChange, onDirtyChange, onSaveActionChange, onSavingChange]);
|
||||
|
||||
useEffect(() => {
|
||||
onSaveActionChange(isDirty ? () => {
|
||||
const save = async () => {
|
||||
|
|
@ -3005,19 +3033,31 @@ export function PromptsTab({
|
|||
{selectedFileExists && fileLoading && !selectedFileDetail ? (
|
||||
<PromptEditorSkeleton />
|
||||
) : useMarkdownEditor ? (
|
||||
<MarkdownEditor
|
||||
key={selectedOrEntryFile}
|
||||
value={displayValue}
|
||||
onChange={(value) => setDraft(value ?? "")}
|
||||
placeholder="# Agent instructions"
|
||||
className="min-w-0 overflow-hidden"
|
||||
contentClassName="min-h-(--sz-420px) max-w-full break-words text-sm leading-7"
|
||||
imageUploadHandler={async (file) => {
|
||||
const namespace = `agents/${agent.id}/instructions/${selectedOrEntryFile.replaceAll("/", "-")}`;
|
||||
const asset = await uploadMarkdownImage.mutateAsync({ file, namespace });
|
||||
return asset.contentPath;
|
||||
}}
|
||||
/>
|
||||
<div
|
||||
onBeforeInputCapture={markEditorInteracted}
|
||||
onDropCapture={markEditorInteracted}
|
||||
onInput={markEditorInteracted}
|
||||
onKeyDownCapture={markEditorInteracted}
|
||||
onPasteCapture={markEditorInteracted}
|
||||
onPointerDownCapture={markEditorInteracted}
|
||||
>
|
||||
<MarkdownEditor
|
||||
key={selectedOrEntryFile}
|
||||
value={displayValue}
|
||||
onChange={(value) => {
|
||||
if (!editorInteractedRef.current) return;
|
||||
setDraft(value ?? "");
|
||||
}}
|
||||
placeholder="# Agent instructions"
|
||||
className="min-w-0 overflow-hidden"
|
||||
contentClassName="min-h-(--sz-420px) max-w-full break-words text-sm leading-7"
|
||||
imageUploadHandler={async (file) => {
|
||||
const namespace = `agents/${agent.id}/instructions/${selectedOrEntryFile.replaceAll("/", "-")}`;
|
||||
const asset = await uploadMarkdownImage.mutateAsync({ file, namespace });
|
||||
return asset.contentPath;
|
||||
}}
|
||||
/>
|
||||
</div>
|
||||
) : (
|
||||
<textarea
|
||||
value={displayValue}
|
||||
|
|
|
|||
Loading…
Reference in New Issue