From fa062b202d1be6628e66d4d4127848dc759c0aed Mon Sep 17 00:00:00 2001 From: Prompt-Surfer <196745532+Prompt-Surfer@users.noreply.github.com> Date: Fri, 16 Oct 2026 10:06:53 +1100 Subject: [PATCH] fix(ui): ignore MDXEditor initial markdown normalization in MarkdownEditor (#13915) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Operators edit agent instructions, routine descriptions, and other Markdown fields in the web UI > - All of these fields use the shared `MarkdownEditor`, which wraps MDXEditor > - MDXEditor re-serializes the initial Markdown when it loads, and it sends the result through `onChange` > - MDXEditor marks that event with the second `onChange` argument, `initialMarkdownNormalize`, but `MarkdownEditor` ignores the flag > - Thus each parent with dirty tracking shows unsaved changes before the user types, and each parent must add its own interaction gate (#12502, #13485) > - This pull request drops the flagged event inside `MarkdownEditor` > - The benefit is that no consumer reports unchanged content as modified on load, and new consumers do not need their own gate ## Linked Issues or Issue Description Refs #13472 Related: - #12502 (merged) adds an interaction gate for agent instructions in `AgentDetail.tsx`. - #13485 (open) adds the same gate to the two routine description editors. Its description names "a shared fix inside `MarkdownEditor`" as a follow-up. This PR is that follow-up. It does not conflict with #13485. The two changes can merge in any order. ## What Changed - `MarkdownEditor.tsx`: read the second `onChange` argument (`initialMarkdownNormalize`). When it is `true`, do not call the parent `onChange`. - The check runs after the empty-document mount guard. That guard still restores content when the first event is empty. - `latestValueRef` keeps the parent value. Thus the prop-sync effect does not push the original Markdown back into the editor. - `MarkdownEditor.test.tsx`: the MDXEditor mock can now send a flagged normalization event on mount. A new test makes sure that the parent `onChange` does not run and that the editor keeps its content. ## Verification - `pnpm --filter @paperclipai/ui exec vitest run src/components/MarkdownEditor.test.tsx`: 48 tests pass. - The new test fails without the change ("expected vi.fn() to not be called at all, but actually been called 1 times"). It passes with the change. - `pnpm --filter @paperclipai/ui exec vitest run` (full UI suite): 634 test files and 6653 tests pass. - `pnpm --filter @paperclipai/ui typecheck`: passes with 0 errors. - I read the MDXEditor 4.2.3 source (`dist/plugins/core/index.js`). The flag is `true` only for the first export after the initial Markdown is set. The editor sets it to `false` after that export. User edits and `setMarkdown` echoes get `false`. - Manual check: we tested the same guard in a real browser on an older Paperclip base (MDXEditor 3.55), together with two changes that `master` already has from #12502. We opened an agent Instructions tab with `-` bullets. The Save bar and the unsaved-changes prompt did not show until we typed. After an edit, one prompt showed for each navigation, and Save still stored the edit. Manual steps for a reviewer: 1. Put an instruction file with `-` bullets on an agent, for example `- first` and `- second`. 2. Open the agent Instructions tab. Do not type. 3. Make sure that the Save bar does not show. 4. Go to another tab. Make sure that no unsaved-changes prompt shows. 5. Do the same steps with a routine description that has `-` bullets. ## Risks - Low risk. The change affects only the first `onChange` event after the editor loads. - A consumer that uses the first event to learn the normalized Markdown no longer gets it. We found no such consumer in the 24 files that render `MarkdownEditor`, and the full UI suite passes. - There are no API, database, migration, or visual 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 - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), 1M-token context window, run as a Claude Code agent with tool use (shell, file edits, test runs). The model wrote the code, the test, and this description. A human operator approved them before submission. ## 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 - [ ] I have updated relevant documentation to reflect my changes (no docs describe this behavior) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (pending: CI runs after push) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (pending: runs after push) - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Claude Opus 5.5 (1M context) --- ui/src/components/MarkdownEditor.test.tsx | 34 ++++++++++++++++++++++- ui/src/components/MarkdownEditor.tsx | 9 +++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/ui/src/components/MarkdownEditor.test.tsx b/ui/src/components/MarkdownEditor.test.tsx index 1d442922983..a172ff8853e 100644 --- a/ui/src/components/MarkdownEditor.test.tsx +++ b/ui/src/components/MarkdownEditor.test.tsx @@ -26,6 +26,8 @@ const mdxEditorMockState = vi.hoisted(() => ({ throwOnRender: false, /** Markdown the mock emits through `onChange` once mounted, as the real editor would export it. */ emitMountChange: null as string | null, + /** Markdown the mock emits once mounted with MDXEditor's `initialMarkdownNormalize` flag set. */ + emitMountNormalizedMarkdown: null as string | null, /** Milliseconds the mock waits before painting its content, to model a slow import. */ populateDelayMs: 0, markdownValues: [] as string[], @@ -72,7 +74,7 @@ vi.mock("@mdxeditor/editor", async () => { }: { markdown: string; placeholder?: string; - onChange?: (value: string) => void; + onChange?: (value: string, initialMarkdownNormalize?: boolean) => void; onError?: (error: unknown) => void; suppressHtmlProcessing?: boolean; className?: string; @@ -150,6 +152,10 @@ vi.mock("@mdxeditor/editor", async () => { if (mdxEditorMockState.emitMountChange !== null) { onChange?.(mdxEditorMockState.emitMountChange); } + if (mdxEditorMockState.emitMountNormalizedMarkdown !== null) { + setContent(mdxEditorMockState.emitMountNormalizedMarkdown); + onChange?.(mdxEditorMockState.emitMountNormalizedMarkdown, true); + } }, 0); return () => { window.clearTimeout(timer); @@ -326,6 +332,7 @@ describe("MarkdownEditor", () => { mdxEditorMockState.emitMountSilentEmptyState = false; mdxEditorMockState.throwOnRender = false; mdxEditorMockState.emitMountChange = null; + mdxEditorMockState.emitMountNormalizedMarkdown = null; mdxEditorMockState.populateDelayMs = 0; mdxEditorMockState.markdownValues = []; mdxEditorMockState.insertedMarkdownValues = []; @@ -695,6 +702,31 @@ describe("MarkdownEditor", () => { }); }); + it("does not report MDXEditor's initial markdown normalization as a change", async () => { + // MDXEditor rewrites `-` bullets to `*` on load and flags it as normalization. + mdxEditorMockState.emitMountNormalizedMarkdown = "* first\n* second"; + const handleChange = vi.fn(); + const root = createRoot(container); + + await act(async () => { + root.render( + , + ); + }); + + await flush(); + await waitPastEmptyHeuristic(); + + expect(handleChange).not.toHaveBeenCalled(); + // The editor keeps its normalized document; nothing pushes the original back. + expect(container.textContent).toContain("* first"); + expect(container.querySelector("textarea")).toBeNull(); + + await act(async () => { + root.unmount(); + }); + }); + it("returns editor output to the parent with the transport escaping removed", async () => { const value = "Affected versions: <= v0.3.1\n\nRename to the real name."; // What the real exporter emits for this document: `<` re-escaped, `<=` bare. diff --git a/ui/src/components/MarkdownEditor.tsx b/ui/src/components/MarkdownEditor.tsx index 80d13c6b5a3..39a0f3d9f6b 100644 --- a/ui/src/components/MarkdownEditor.tsx +++ b/ui/src/components/MarkdownEditor.tsx @@ -1532,7 +1532,7 @@ export const MarkdownEditor = forwardRef suppressHtmlProcessing placeholder={placeholder} readOnly={readOnly} - onChange={(rawNext) => { + onChange={(rawNext, initialMarkdownNormalize) => { if (readOnly) return; // Reverse the editor-only rewrites: blockquotes the exporter escaped // as `\>` (so a `>`-prefixed line the user typed survives even when @@ -1560,6 +1560,13 @@ export const MarkdownEditor = forwardRef return; } } + // MDXEditor re-serialises the initial markdown on load (bullet + // markers, escapes, blank lines) and flags that emission. It is not + // a user edit: forwarding it marks every controlled parent with + // dirty tracking as unsaved before any keystroke. `latestValueRef` + // stays on the parent's value, so the prop-sync effect does not + // push the original back over the editor. + if (initialMarkdownNormalize) return; // `latestValueRef` is compared against `editorValue`, so it has to // hold editor space; storing `next` would make every edit containing // an escaped bracket look like a pending prop sync and trigger a