fix(notes): cap note path length and harden folder creation (#22, #87)#192
Merged
Conversation
Closed
Deploying podnotes with
|
| Latest commit: |
6e47b57
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b1a63169.podnotes.pages.dev |
| Branch Preview URL: | https://chhoumann-22-max-path-length.podnotes.pages.dev |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 604a3e498f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ); | ||
|
|
||
| const filePathDotMd = addExtension(filePath, "md"); | ||
| return enforceMaxPathLength(addExtension(filePath, "md")); |
There was a problem hiding this comment.
When users already have episode notes created on APFS/NTFS whose filename is valid there but exceeds 255 UTF-8 bytes (for example, 100 CJK characters plus .md is over the new byte cap but under those filesystems' character limits), this resolver now always rewrites the path to the truncated 255-byte version. getPodcastNote and the context-menu existence check therefore miss the existing file and createPodcastNote can create a duplicate truncated note instead of opening it; consider checking the uncapped path as a fallback before creating.
Useful? React with 👍 / 👎.
Long episode titles produced a note path whose filename component exceeded the OS per-component limit, so app.vault.create failed with ENAMETOOLONG and note creation silently failed (#22). - Add enforceMaxPathLength: caps every path component at 255 BYTES (the most restrictive mainstream limit — ext4/Android count bytes, NTFS/APFS count 255 chars; a byte budget satisfies all), truncating the title on code-point boundaries (never splitting a surrogate pair) while preserving the .md extension and folders, trimming trailing dots/spaces a cut exposes, and substituting "Untitled" when a title sanitizes to nothing (the empty-name edge flagged when #99's illegal-char handling landed). - Apply it through a single shared path resolver so the existence check, open, and create all agree on the capped path. - Route folder creation through ensureFolderExists and make it tolerant of "Folder already exists" (case-insensitive FS / races) — folds in #87's spurious "Failed to create note". This also transitively hardens the feed note and download paths, which already call ensureFolderExists. Verified in a real Obsidian vault: a 382-char ASCII title, a 600-byte CJK title, and a surrogate-pair emoji title all create valid <=255-byte notes; an all-illegal title creates Untitled.md; a simulated folder-lookup miss no longer fails note creation. Tests cover the helper (byte budget, surrogate safety, fallback, collisions) and the folder-tolerance regression.
…wnload Per-component filename limits differ by unit, not just value: NTFS (Windows) and APFS/HFS+ (macOS, iOS) cap at 255 UTF-16 code units, while ext4/F2FS (Linux, Android) cap at 255 bytes. Detect the OS via Obsidian's native Platform API and budget in the matching unit, so a long non-ASCII title uses the full legal length on each platform instead of being needlessly truncated (a byte cap shortened a CJK title to 84 chars on macOS, where APFS accepts 255 units / ~750 bytes — verified empirically). - enforceMaxPathLength now takes a platform-derived FilenameLimit (injectable for tests); truncation stays code-point-safe in both units. - Extend the same cap to the other note/file creators that build paths from a title: feed notes (createFeedNote), transcripts (TranscriptionService), and downloads (downloadEpisode, via a shared safeDownloadFilePath used by both the existence check and the write). - Replace TranscriptionService's hand-rolled createFolder loop with the hardened ensureFolderExists, removing the last #87-class folder bug. Verified in a real Obsidian vault on macOS: a 300-char CJK title and an emoji title now create full 255-UTF-16-unit notes (valid UTF-8, no split surrogate).
…nsion, vault, folder trim
Adversarial review (round 2) found real follow-on gaps from extending the cap:
- {{podcastlink}} built its wikilink from the UNCAPPED feed path, so a long feed
title produced a link that didn't match the capped feed note (broken link).
getFeedNoteWikilink now mirrors createFeedNote's enforceMaxPathLength(addExtension)
derivation.
- getTranscriptPath force-appended .md, silently changing custom non-.md transcript
paths (orphaning existing transcripts, '.txt' -> '.txt.md'). It now preserves the
configured extension via lastSegmentExtension.
- ensureFolderExists gained an optional vault param so TranscriptionService creates
folders and writes the file through the same (injected) vault, not a global/instance
split.
- capFolderName now trims trailing dots/spaces even on short folder segments (illegal
on Windows), matching the documented behavior.
Adds tests for each (incl. createFeedNote/integration caps) and runtime-verified in a
real vault that {{podcastlink}} for a 400-char feed title emits a 252-char capped link.
…tension Round-3 review found lastSegmentExtension naively took everything after the last dot as the extension. A no-extension transcript template with a dotted title (e.g. "Episode.Part.<400 chars>") made the 400-char tail the 'extension', which capFileName then reattached past the cap — still tripping ENAMETOOLONG. Restrict it to a short alphanumeric suffix so a dotted title (or a leftover template token) is treated as part of the name and the whole segment is capped.
chhoumann
force-pushed
the
chhoumann/22-max-path-length
branch
from
d19e1b4 to
6e47b57
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Creating notes for podcasts with very long titles silently failed: the generated path's filename component exceeded the OS limit, so
app.vault.createthrewENAMETOOLONGand the user only saw "Failed to create note" (#22). Separately, note creation could fail spuriously with a consoleFolder already exists(#87).This PR caps generated paths safely (platform-aware) and hardens folder creation, across every path the plugin builds from a title.
Closes #22.
Closes #87.
What changed
New
src/utility/enforceMaxPathLength.ts— caps each path component at the platform's real per-component limit, detected via Obsidian's nativePlatform:It truncates only the title (the base name) on code-point boundaries (never splitting a surrogate pair), preserves the extension, substitutes
Untitledwhen a title sanitizes to nothing, trims trailing dots/spaces a cut exposes, and drops empty path segments. The limit is injectable for testing.Applied everywhere a path is built from a title (each via a single shared resolver, so the existence-check and the write always agree on the capped path):
createPodcastNote)createFeedNote) and the{{podcastlink}}wikilink that targets them (getFeedNoteWikilink)TranscriptionService, extension taken from the user's template — not forced to.md)downloadEpisode, shared by the pre-download check and the write)Folder creation hardened —
ensureFolderExistsnow toleratesFolder already exists(thrown when a case-sensitive lookup misses an existing folder on a case-insensitive FS, or on a concurrent create) while still rethrowing genuine errors. This is #87's root cause.TranscriptionService's hand-rolled loop was replaced with it (passing its injected vault), andcreateFeedNote/downloadEpisodeare transitively hardened.Runtime verification (real Obsidian vault on macOS)
Driven through the actual commands:
app.vault.createENAMETOOLONG(baseline)Untitled.md✓createFolderon existing folder / lookup miss{{podcastlink}}for a 400-char feed titledev:errorsclean throughout.Tests & gates
New
enforceMaxPathLength.test.ts(both platform modes, multibyte, surrogate safety, fallback, edge-trim, deterministic collision,lastSegmentExtensionguard) and extended tests forensureFolderExists(already-exists tolerance, injected vault),createFeedNote(cap),downloadEpisode(safeDownloadFilePath), andgetFeedNoteWikilink(cap). Gates:lint✓format:check✓typecheck✓build✓test✓ (404 passing).docs:buildnot run (mkdocs not installed locally).Review
Three rounds of an ultracode multi-lens workflow + opposite-model (Codex) adversarial reviewers. Round 1 → byte-vs-char cap (made platform-aware). Round 2 →
{{podcastlink}}mismatch, transcript extension, vault-instance split, short-folder trim (all fixed). Round 3 → a dotted-title-as-pseudo-extension edge (fixed); workflow 0 findings, architect PASS.Design notes / out of scope
MAX_PATH) is not bounded (it needs the absolute vault root); a long title trips the per-component limit, which is what this fixes.CON,PRN, …) remain unhandled by the sanitizer; downloads keep their existingepisodefallback (Default download.path is empty, breaking the Download command (writes ".mp3" to vault root, 2nd download fails) #183). Deterministic collisions (two titles sharing the capped prefix) resolve to the same note and open the existing one (no data loss) — matching the issue's deterministic-naming goal.