From 8b33fe853c1c3f93610a5621c8fce3fb7d95bc3f Mon Sep 17 00:00:00 2001 From: Neil Date: Fri, 18 Sep 2026 20:27:00 -0700 Subject: [PATCH] fix(editor): retain CR line endings during save recovery --- .../editor/rich-markdown-save-reopen.test.ts | 46 ++++++++++++- .../editor/rich-markdown-source-reconcile.ts | 14 ++-- tests/e2e/markdown-link-label-save.spec.ts | 65 +++++++++++++++++++ 3 files changed, 119 insertions(+), 6 deletions(-) create mode 100644 tests/e2e/markdown-link-label-save.spec.ts diff --git a/src/renderer/src/components/editor/rich-markdown-save-reopen.test.ts b/src/renderer/src/components/editor/rich-markdown-save-reopen.test.ts index 93895fae0b1..d2eb48ff747 100644 --- a/src/renderer/src/components/editor/rich-markdown-save-reopen.test.ts +++ b/src/renderer/src/components/editor/rich-markdown-save-reopen.test.ts @@ -1,5 +1,5 @@ import { Editor } from '@tiptap/core' -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { encodeRawMarkdownHtmlForRichEditor } from './raw-markdown-html' import { createRichMarkdownExtensions } from './rich-markdown-extensions' import { commitRichMarkdownSerialization } from './rich-markdown-serialization-commit' @@ -144,3 +144,47 @@ for (const eol of ['\n', '\r\n', '\r']) { }) } } + +for (const eol of ['\n', '\r\n', '\r']) { + it.each(['null', 'throw'])( + `recovers after a %s reconciliation failure with EOL ${JSON.stringify(eol)}`, + (failure) => { + const source = '# target\n\n_unchanged_\n'.replace(/\n/g, eol) + const editor = openDocument(source) + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + try { + const refs = { + originalSourceRef: { current: source }, + baseCanonicalRef: { current: editor.getMarkdown() }, + lastCommittedMarkdownRef: { current: source } + } + editor.view.dispatch(editor.state.tr.insertText('changed', 1, 7)) + const saved = commitRichMarkdownSerialization(editor, refs, () => { + if (failure === 'throw') { + throw new Error('Injected round-trip failure') + } + return null + }) + expect(canonicalize(saved.markdown)).toBe(editor.getMarkdown()) + expect(saved.markdown.endsWith(eol)).toBe(true) + expect(refs.lastCommittedMarkdownRef.current).toBe(saved.markdown) + const serialize = vi.spyOn(editor, 'getMarkdown').mockImplementationOnce(() => { + throw new Error('Injected editor teardown') + }) + expect(commitRichMarkdownSerialization(editor, refs, canonicalize)).toEqual({ + markdown: saved.markdown, + didSerialize: false + }) + serialize.mockRestore() + editor.view.dispatch(editor.state.tr.insertText('recovered', 1, 8)) + const recovered = commitRichMarkdownSerialization(editor, refs, canonicalize) + expect(recovered.didSerialize).toBe(true) + expect(canonicalize(recovered.markdown)).toBe(editor.getMarkdown()) + expect(recovered.markdown).toContain('recovered') + } finally { + consoleError.mockRestore() + editor.destroy() + } + } + ) +} diff --git a/src/renderer/src/components/editor/rich-markdown-source-reconcile.ts b/src/renderer/src/components/editor/rich-markdown-source-reconcile.ts index 1377ecb8567..b56ba5f5648 100644 --- a/src/renderer/src/components/editor/rich-markdown-source-reconcile.ts +++ b/src/renderer/src/components/editor/rich-markdown-source-reconcile.ts @@ -133,25 +133,29 @@ function stripTrailingNewlines(lfText: string): string { return lfText.replace(/\n+$/, '') } -function detectDominantEol(text: string): '\n' | '\r\n' { +function detectDominantEol(text: string): '\n' | '\r\n' | '\r' { const totalLf = (text.match(/\n/g) ?? []).length const crlf = (text.match(/\r\n/g) ?? []).length const lfOnly = totalLf - crlf + const crOnly = (text.match(/\r/g) ?? []).length - crlf + if (crOnly > Math.max(lfOnly, crlf)) { + return '\r' + } return crlf > 0 && crlf >= lfOnly ? '\r\n' : '\n' } function toLf(text: string): string { - return text.replace(/\r\n/g, '\n') + return text.replace(/\r\n|\r/g, '\n') } -function restoreEol(lfText: string, eol: '\n' | '\r\n'): string { +function restoreEol(lfText: string, eol: '\n' | '\r\n' | '\r'): string { // lfText is pure LF, so a blind LF→CRLF replace produces no mixed endings. - return eol === '\r\n' ? lfText.replace(/\n/g, '\r\n') : lfText + return eol === '\n' ? lfText : lfText.replace(/\n/g, eol) } function normalizeForSafety(text: string): string { // Why: compare exactly (only CRLF-normalized) — a trailing `\n\n` empty paragraph is semantic, so a lenient trimEnd would mask the trailing-block drift branch 6 must catch. - return text.replace(/\r\n/g, '\n') + return text.replace(/\r\n|\r/g, '\n') } function getUtf8OffsetsAtCodeUnitIndices( diff --git a/tests/e2e/markdown-link-label-save.spec.ts b/tests/e2e/markdown-link-label-save.spec.ts new file mode 100644 index 00000000000..e2ffa5debaf --- /dev/null +++ b/tests/e2e/markdown-link-label-save.spec.ts @@ -0,0 +1,65 @@ +import { readFileSync } from 'node:fs' +import { test, expect } from './helpers/orca-app' +import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { + cleanupMarkdownFixture, + closeActiveEditorTab, + createMarkdownFixture, + getActiveWorktreeContext, + openMarkdownFixture, + waitForRichMarkdownEditor +} from './helpers/markdown-editor-fixture' + +const LABEL = '[label](path) $5' + +test('literal link labels survive editing and reopening', async ({ orcaPage }, testInfo) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + const context = await getActiveWorktreeContext(orcaPage) + const file = await createMarkdownFixture( + context, + 'markdown-links', + 'literal-label', + testInfo.workerIndex, + '# Link label safety\n\n[Original label](https://example.com)\n\nUnchanged tail.\n' + ) + try { + await openMarkdownFixture(orcaPage, context, file) + const editor = await waitForRichMarkdownEditor(orcaPage) + await expect(editor.locator('a')).toHaveText('Original label') + await testInfo.attach('before-edit', { + body: await orcaPage.screenshot({ path: testInfo.outputPath('before-edit.png') }), + contentType: 'image/png' + }) + await editor.locator('a').evaluate((link) => { + link.closest('[contenteditable]')?.focus() + const range = document.createRange() + range.selectNodeContents(link) + const selection = window.getSelection() + selection?.removeAllRanges() + selection?.addRange(range) + document.dispatchEvent(new Event('selectionchange')) + }) + await expect + .poll(() => orcaPage.evaluate(() => window.getSelection()?.toString())) + .toBe('Original label') + await orcaPage.keyboard.insertText(LABEL) + await expect(editor.locator('a')).toHaveText(LABEL) + await orcaPage.keyboard.press('ControlOrMeta+S') + await expect.poll(() => readFileSync(file, 'utf8')).not.toContain('Original label') + expect(readFileSync(file, 'utf8')).toContain('https://example.com') + await closeActiveEditorTab(orcaPage, file) + await openMarkdownFixture(orcaPage, context, file) + const reopened = await waitForRichMarkdownEditor(orcaPage) + await expect(reopened.locator('a')).toHaveCount(1) + await expect(reopened.locator('a')).toHaveText(LABEL) + await expect(reopened.locator('a')).toHaveAttribute('href', 'https://example.com') + await expect(reopened.locator('p').last()).toHaveText('Unchanged tail.') + await testInfo.attach('after-reopen', { + body: await orcaPage.screenshot({ path: testInfo.outputPath('after-reopen.png') }), + contentType: 'image/png' + }) + } finally { + await cleanupMarkdownFixture(file) + } +})