fix(ui): ignore MDXEditor initial markdown normalization in MarkdownEditor (#13915)

## 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) <noreply@anthropic.com>
This commit is contained in:
Prompt-SurferandClaude Opus 5.5 authored and GitHub committed 2026-10-15 16:06:53 -07:00
1 parent 7c6d977929
commit fa062b202d
2 files changed
+41 -2

No files matched your search

+33 -1
View File
@@ -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(
<MarkdownEditor value={"- first\n- second"} onChange={handleChange} placeholder="Markdown body" />,
);
});
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 <name> to the real name.";
// What the real exporter emits for this document: `<` re-escaped, `<=` bare.
+8 -1
View File
@@ -1532,7 +1532,7 @@ export const MarkdownEditor = forwardRef<MarkdownEditorRef, MarkdownEditorProps>
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<MarkdownEditorRef, MarkdownEditorProps>
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