Skip to content

fix: read and edit notes with Windows line endings without a stray carriage return - #680

Merged
aliasunder merged 9 commits into
mainfrom
fix/crlf-notes-edit-corruption
Oct 16, 2026
Merged

aliasunder merged 9 commits into
mainfrom
fix/crlf-notes-edit-corruption

Conversation

@aliasunder

@aliasunder aliasunder commented Oct 16, 2026 •

Copy link
Copy Markdown
Owner

Summary

Notes saved with Windows line endings (CRLF), common in vaults checked out on Windows through git, now read and edit cleanly.

Before this change:

  • The last property gained a hidden carriage return. title: C read as "C\r", an empty due: read as "\r" instead of empty, and every edit saved the value back as title: "C\r".
  • A quoted or bracketed last property made the whole note unreadable to the server. title: "C", tags: [a, b], or a related: list of quoted links, the form Obsidian writes, was refused as "properties block is not valid YAML". Property reads, outline and section reads, every edit tool, task and memory tools, and moves all failed on such a note.
  • An empty CRLF properties block was refused on every edit ("holds a single value").
  • Text copied from a raw read didn't match. old_text with \r\n returned "text not found", and \r\n in new_text or content was written byte for byte into notes the server otherwise writes with LF.

Changes

  • Properties block (frontmatter.ts): the split closes the block at \n---, which left the \r of the last line's break at the end of the block. That \r is now dropped, so the block reads as the same note with LF endings does, which matches Obsidian's metadata cache. Note bodies keep their bytes, so vault_update_properties still keeps the body as it says.
  • Edit tools (vault-patcher.ts): vault_replace_in_note converts \r\n in old_text and new_text to \n. vault_patch_note, vault_replace_span and vault_insert_at_anchor split content the way the note body is split, so its Windows line breaks are written as LF and its blank lines count as empty lines. The leading-heading check no longer needs its own carriage-return strip.
  • vault_read_note description: only an unpaged full read returns the file's exact bytes; heading sections and paged reads use LF line endings (true before this change too; the old sentence said every unpaged read was byte-identical). It also lists two errors the tool already returns (ENAMETOOLONG, EACCES and other filesystem error codes), says properties_only returns {} for a note with no properties, and moves the one-mode-per-call rule out of the "Section boundaries" paragraph. Snapshots regenerated.

Upgrade notes

  • Values saved by earlier versions: an edit on a CRLF note saved the last property with a trailing carriage return (it reads as "C\r"). This change stops new ones; an existing one stays until it is rewritten. Fix an affected value with vault_update_properties (for example { "title": "C" }).
  • Search index: every note is re-parsed at startup, so titles and property values are corrected on the first boot. Notes whose title or properties changed re-embed once.

Not in this change

  • A text property whose value ends in blank lines (a |+ block, for example) loses those trailing line breaks on any rewrite. This is a pre-existing loss on LF notes too; on CRLF notes such a value was refused before and now behaves as on LF notes.
  • The properties block's opener rule is unchanged: a byte-order mark or spaces after the opening --- still split differently in the task tools.
  • Pre-existing partial-match (toContain) assertions in the three edited test files are left for a separate test-only change.

Tool definitions

Checked with the tool-definition reviewer against main's tool list, in every server configuration:

  • vault_read_note is the only tool whose text changed; no other tool, server instruction or prompt changed.
  • The review found two errors the tool already returns without listing them, ENAMETOOLONG and EACCES (or another filesystem error code). The code notes that each tool lists these when its description is next revised, so they are added here, with {} for a note with no properties.
  • The four edit tools' wording ("verbatim", "exact text", "as given") was checked against the new line-break handling and kept: each still describes what a caller gets, since every edit already rewrote the note with LF.
  • A re-review of the final text found no new defects. Its remaining notes are pre-existing: two wording gaps (the "start line past the end" entry and start_line's "full body") are left for the next description round, and the rest are kept as they are.

Size: vault_read_note 5,735 → 5,990 characters; the default tool list 139,437 → 139,692 (+255). CHARS_PER_TOOL_ALLOWANCE rises from 4,226 to 4,234 to fit the two error entries.

Test plan

  • npm test: 116 files, 5,317 passed, 2 skipped
  • npm run lint, npm run knip, npm run build (both typechecks)
  • New tests failed on the unchanged code, then passed; 12 deliberate code breaks were each caught by a test
  • Live test on the test image (below)

Live Test Results

Head: f786fda

  • Test image: Test Deploy run 38954501900 built org.opencontainers.image.revision=f786fdad5588b9549500f19d49d4dc38acbb8e52; the log shows the container Recreated, Started and Healthy. On the host, the running container is ghcr.io/aliasunder/vault-cortex:test, image sha256:a68479ec8adc…, revision label f786fdad5588b9549500f19d49d4dc38acbb8e52, started 2026-10-16 19:26:40 UTC.
  • Released image (before): ghcr.io/aliasunder/vault-cortex:remote v0.54.14, image sha256:d8c09778d068…, revision label aa361ed4540872cc997ed0c0416210f61a867f5b.
  • Client: the MCP SDK client against the deployed /mcp endpoint, so real carriage returns reach the server (a typed \r\n in a chat client arrives as four characters). Every probe wrote its own note per image, and a raw read confirmed the CRLF bytes before each operation. All probe notes were deleted afterwards.
Probe Released (v0.54.14) Test image (f786fda)
1. vault_write_note with a CRLF body ending its block in title: C, then properties_only {"title": "C\r"} {"title": "C"}
2. The same with a related: list of a quoted wikilink as the last property refused: "the note would open with a properties block the server cannot keep: properties block is not valid YAML at line 4, column 40: Unexpected scalar at node end" written; properties_only returns {"title": "C", "related": ["[[Scratch/crlf-live-test/target]]"]}
3. vault_replace_in_note with old_text "- [ ] task\r\n" copied from a raw read "text not found" replaced 1; file is ---\ntitle: C\n---\n# H\nA\n- [x] task\nB\n, properties still {"title": "C"}
4. vault_patch_note append with content "X1\r\nX2" file ends X1\r\nX2\n, block saved as title: "C\r" file ends X1\nX2\n, no carriage return anywhere
5. Outline and heading read on an untouched CRLF note (the description's sentence) root bytes 39 = the file; heading H returns # H\nA\nB, bytes 7 (the LF section, not the CRLF one) same as released

🤖 Generated with Claude Code

aliasunder and others added 9 commits October 16, 2026 14:12
…ut a carriage return

gray-matter closes the properties block at `\n---`, so in a note with
Windows line endings the block kept the `\r` of its last line. The YAML
parser read it into the last value (`title: "C\r"`), which every rewrite
then saved, or refused the whole block when that value was quoted or a
bracketed list. splitPropertiesBlock now drops that `\r`, so the block
reads as the same note with LF endings does. Note bodies keep their bytes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e's LF

The edit tools match against a note body split into LF lines, so old_text
copied from a raw read of a CRLF note never matched, and \r\n in new_text
or content was written byte for byte into LF output. replace_in_note now
converts old_text and new_text, and patch, replace_span and
insert_at_anchor split content with splitIntoLines, which also lets the
leading-heading check drop its own carriage-return strip.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… LF line endings

The description said unpaged reads stay byte-identical, but an unpaged
heading read of a note with Windows line endings returns LF lines. It now
limits the byte-for-byte promise to unpaged full reads, and drops
"verbatim" from the heading-mode pointer so the two statements agree.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- frontmatter.ts: say why only the last line's carriage return is dropped,
  what the block text keeps, why line numbers match the note, and that
  replacing a CRLF note's block leaves mixed line endings.
- vault-patcher.ts: comment each content split's CRLF conversion, the
  old_text conversion and its empty-string guard, the anchor and insert
  exceptions, and the LF-only offsets in listTextDeletionEdits; one
  spelling for empty-string guards; firstNonBlankLineIndex.
- vault_read_note: name what full reads match, move the line-ending
  sentence to Returns, and say "Kanban board". Snapshot regenerated.

Ship-Check: code-quality · claude-opus-5-5

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…'s docstring

Obsidian's Properties panel rewrites the whole note with LF; only its
processFrontMatter API leaves mixed line endings.

Ship-Check: triage · claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ot-found quote

- updateProperties on a CRLF note keeps the last property without a
  carriage return and the body's CRLF bytes
- readNoteSection returns a CRLF note's section with LF line breaks, as
  the vault_read_note description now states
- replaceInNote's not-found message quotes old_text as sent, CRLF included
- the CRLF old_text test sends an LF new_text, so it fails only when
  old_text matching breaks

Ship-Check: test-audit · claude-opus-5-5

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… line

The opener may carry trailing spaces or tabs, which stay at the start of the block text, so the text does not always start with the line break. Line numbers still match the note's.

Ship-Check: bug-check · claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…roperties result

The tool-definition review of this branch's vault_read_note edit found
gaps the tool already had:
- ENAMETOOLONG and EACCES (or another filesystem error code) reach the
  client, but no Errors entry named them; tool-helpers says each tool
  lists these when its description is next revised.
- properties_only returns {} for a note with no properties.
- The one-mode-per-call rule sat under "Section boundaries".
- The ".md" entry lost its remedy when vault_read_file is off.
The per-tool size allowance rises from 4,226 to 4,234 to fit them.

Ship-Check: tool-definitions · claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@umm-actually

umm-actually Bot commented Oct 16, 2026

Copy link
Copy Markdown

Drop the stale task_guide_test_anchor
Low severity · subtle bugs · medium confidence

src/vault-mcp/mcp-core/__tests__/tool-definitions.test.ts:2581 — beyond the diff's line ranges, in code the changes touch or depend on.

Pre-existing: vault_list_tasks' routing sentence still promises heading mode returns a byte-exact lane text, but readNoteSection normalizes a CRLF note's lines to LF, so the returned section is not byte-verbatim. This PR dropped the same word from vault_read_note's own routing sentence and its Returns section now states heading sections use LF line endings, leaving the sibling claim contradicting the tool it points at — for exactly the Windows-line-ending notes this PR targets.

Failure scenario: On a vault with CRLF notes (the scenario this PR fixes elsewhere), an agent following vault_list_tasks' routing reads a Kanban lane with vault_read_note's heading mode and receives LF-normalized text whose bytes differ from the note's CRLF bytes — the 'verbatim' promise is broken, and the same tool surface now says elsewhere that heading sections use LF line endings.

Suggested fix
In task-tools.ts, trim the sentence to "...only when you need a lane's Markdown or a task's state right after a write." to mirror vault_read_note's routing sentence, and update the pinned expectation in this test — it can ride the next vault_list_tasks description revision per the tool-by-tool editing rule.

umm-actually · z-ai/glm-5.3-flash

@umm-actually

umm-actually Bot commented Oct 16, 2026

Copy link
Copy Markdown

umm-actually reviewed at f786fda

1 new finding(s) posted (1 tracked finding(s) across all runs).

Context notes
  • Priority docs not included: ARCHITECTURE.md, deploy/remote/README.md (missing, unreadable, or over budget)
  • 18 changed file(s) excluded from review: src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/default.json (diff_exclude_paths input), src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/disabled-tools.json (diff_exclude_paths input), src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/embedding-off.json (diff_exclude_paths input), src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/file-tools-off+embedding-off.json (diff_exclude_paths input), src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/file-tools-off.json (diff_exclude_paths input), src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/memory-off+embedding-off.json (diff_exclude_paths input), src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/memory-off+file-tools-off+embedding-off.json (diff_exclude_paths input), src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/memory-off+file-tools-off.json (diff_exclude_paths input), src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/memory-off.json (diff_exclude_paths input), src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/obsidian-sync.json (diff_exclude_paths input), src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/readonly+embedding-off.json (diff_exclude_paths input), src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/readonly+file-tools-off+embedding-off.json (diff_exclude_paths input), src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/readonly+file-tools-off.json (diff_exclude_paths input), src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/readonly+memory-off+embedding-off.json (diff_exclude_paths input), src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/readonly+memory-off+file-tools-off+embedding-off.json (diff_exclude_paths input), src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/readonly+memory-off+file-tools-off.json (diff_exclude_paths input), src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/readonly+memory-off.json (diff_exclude_paths input), src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/readonly.json (diff_exclude_paths input)

umm-actually · z-ai/glm-5.3-flash

@aliasunder

Copy link
Copy Markdown
Owner Author

Re: issue comment 6222861364 (vault_list_tasks routing sentence still says "a lane's verbatim Markdown")

Valid. Heading mode has always returned LF lines for a note with Windows line endings, and vault_read_note's description now says so, which makes the sibling sentence in vault_list_tasks inexact for those notes.

The maintainer approved keeping vault_list_tasks's text as it is: any change to a tool's text re-scores that tool with tool-definition graders, and the inexact word does not lead an agent to a wrong action (lane text pasted back into vault_replace_in_note matches either way). The one-word fix, with its pinned test expectation, is recorded for that tool's next description revision.


🔍 ship-check · pr-monitor · claude-opus-5-5

@aliasunder
aliasunder merged commit 94b83ec into main Oct 16, 2026
19 checks passed
@aliasunder
aliasunder deleted the fix/crlf-notes-edit-corruption branch October 16, 2026 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant