feat(operations)!: migrate vault operations off Obsidian CLI to direct disk access#58
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Passes the already-constructed VaultReader through IVaultEntryDeps.providerFactory so FsVaultProvider can receive it. Pure plumbing: no method behavior changes. FsVaultProvider gains a requireReader() guard for Task 4's disk-direct listTags/listProperties implementation to use. Co-Authored-By: Claude Opus 4.7 <[email protected]>
…rovider
Aggregate {name, count} from a vault-relative reader.scan() +
reader.readNotes() frontmatter pass instead of delegating to the CLI.
Revives get_vault_overview in headless mode, since it calls these two
provider methods with no try/catch.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Resolves today's daily-note path from .obsidian/daily-notes.json via readDailyNotesConfig + formatDailyDate and reads it straight from disk, removing the last daily-leg delegation to the Obsidian CLI. Missing config surfaces DAILY_NOTES_NOT_CONFIGURED; missing today-note surfaces NOT_FOUND with the resolved path. Co-Authored-By: Claude Opus 4.7 <[email protected]>
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Writes notes directly to disk (mkdir + writeFile with wx/w flags) instead of delegating to the Obsidian CLI. Content is written verbatim (no template expansion — the create_note tool already tells callers to render templates themselves). name-based paths resolve a folder prefix from .obsidian/app.json (newFileLocation=folder + newFileFolderPath), matching Obsidian's own new-note placement. Parent directories are now created automatically, which the CLI could not do. NOTE_EXISTS error wording is unchanged for tool-layer behavior parity. Co-Authored-By: Claude Opus 4.7 <[email protected]>
setProperty/removeProperty now read-mutate-write the frontmatter YAML block directly via yaml's parseDocument, preserving comments/formatting of untouched keys and the note body byte-for-byte. Removing the last key strips the block; removing an absent key is a no-op write. All 6 VaultProvider methods are now disk-direct; the cli field and ObsidianCLIProvider import are kept unused pending Task 9's removal PR. Co-Authored-By: Claude Opus 4.7 <[email protected]>
…dian-cli Co-Authored-By: Claude Opus 4.7 <[email protected]>
…flag Delete the dead ObsidianCLIProvider path now that all six VaultProvider methods are disk-direct: the provider file and its test, the `cli` field on FsVaultProvider, the `--obsidian-cli` config option, and the CLI_NOT_FOUND/CLI_UNAVAILABLE/CLI_TIMEOUT error codes. FsVaultProviderOptions is now standalone with required `vaultRoot` and `reader`, and the server instructions describe disk-direct runtime requirements instead of CLI availability. BREAKING CHANGE: the server no longer shells out to obsidian-cli. The --obsidian-cli option is removed (launch commands passing it fail to start) and the CLI_NOT_FOUND/CLI_UNAVAILABLE/CLI_TIMEOUT error codes no longer occur. Co-Authored-By: Claude Opus 4.7 <[email protected]>
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Vault operations (create_note, read_daily, set_property, remove_property, list_tags, list_properties) now read and write the vault directory directly via FsVaultProvider — no external process, no Obsidian CLI/runtime dependency. Mints ADR-0009 recording that decision and its accepted divergences (frontmatter-only tag counts, no types.json registration, no template expansion), marks ADR-0007 superseded, rewrites cli-write-defenses.md as disk-write-path.md, and sweeps every obsidian-cli / --obsidian-cli / CLI_NOT_FOUND / CLI_UNAVAILABLE reference out of the architecture and user-facing docs. Co-Authored-By: Claude Opus 4.7 <[email protected]>
Co-Authored-By: Claude Opus 4.7 <[email protected]>
…an-cli Co-Authored-By: Claude Opus 4.7 <[email protected]>
…n-cli Co-Authored-By: Claude Opus 4.7 <[email protected]>
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Co-Authored-By: Claude Opus 4.7 <[email protected]>
…ult-operations spec Co-Authored-By: Claude Opus 4.7 <[email protected]>
Remove the duplicated fence-slicing logic that lived inline in splitFrontmatter and again as a private sliceYamlBody in FsVaultProvider; both now call the single exported helper. Adds direct unit tests. Co-Authored-By: Claude Opus 4.7 <[email protected]>
… coverage Break the monolithic test/operations/fs-vault-provider.test.ts into one file per method under test/operations/fs-vault-provider/, sharing a _helpers.ts (makeVault/makeProvider/makeMockGraph/todayBasename/byPath/byName). Expand edge-case coverage on every method: 22 -> 75 tests. Co-Authored-By: Claude Opus 4.7 <[email protected]>
- set_property/remove_property reject ambiguous names with
AMBIGUOUS_MATCH via a resolver shared with edit_note (was: silent
alphabetically-first write)
- create_note name traversal ('../x') fails INVALID_ARGUMENT instead
of an uncoded error
- read_daily reads daily-notes.json once (provider-side only)
- drop the dead vaultProviderFactory hook from the operations module
- rewrite the stale CLI rationale comment in daily-notes-config
- fill the headless-vault-operations spec Purpose; add the ambiguity
scenario
- pin edge cases: YYYYY / unclosed '[' daily formats, yaml
trailing-comment association on property writes
Co-Authored-By: Claude Opus 4.8 <[email protected]>
…review 2) - readDaily: normalize the resolved daily-note path so a folder/format that escapes the vault (e.g. folder '../outside') is refused with DAILY_NOTES_NOT_CONFIGURED instead of reading outside the vault [P1 security] - setProperty/removeProperty: map write failures to WRITE_FAILED (new contract code) instead of leaking a bare fs error - createNote: wrap mkdir so a directory-creation failure (parent path is a file) surfaces as CREATE_FAILED, not raw ENOTDIR/EEXIST - editFrontmatter: reject syntactically-valid but non-map YAML roots (scalar/sequence frontmatter) with READ_FAILED before doc.set() throws a plain Error - formatDailyDate: reject long year runs (YYYYYY, YYYYYYYY) instead of silently rendering 202626 / 20262026 Adds regression tests for each and spec scenarios for the traversal guard and the ambiguity behavior. Co-Authored-By: Claude Opus 4.8 <[email protected]>
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
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
Migrates all six vault-operation methods off the Obsidian CLI to direct disk access, so neuro-vault runs fully headless — no
obsidianbinary, no running Obsidian app. This unblocks headless VPS deployment (Bro v0). Done as a strangler-fig migration:FsVaultProviderfirst delegated toObsidianCLIProvider, then each method was flipped to a disk implementation one at a time, and finally the CLI path was deleted entirely.OpenSpec change:
migrate-off-obsidian-cli(archived in this PR underopenspec/changes/archive/2026-07-17-migrate-off-obsidian-cli/; new capability specopenspec/specs/headless-vault-operations/).--obsidian-cliconfig option is removed — launch commands passing it now fail to start (yargs strict).CLI_NOT_FOUND/CLI_UNAVAILABLE/CLI_TIMEOUTerror codes no longer occur (nothing shells out).What changed
FsVaultProviderimplements all 6VaultProvidermethods on disk:createNote(wxwrite,NOTE_EXISTS,name→folder via.obsidian/app.json),readDaily(resolves today's path from.obsidian/daily-notes.jsonvia a minimal moment-format renderer),setProperty/removeProperty(YAMLparseDocumentround-trip — body byte-identical, comments preserved, idempotent absent-key remove, last-key strips block),listTags/listProperties(aggregate over the frontmatter scan).ObsidianCLIProvider, the--obsidian-cliflag, theCLI_*error codes, and the CLI error-mapping/write-defenses docs. Agrep 'CLI_' src/sweep test keeps it gone.cli-write-defenses.md→disk-write-path.md; guide/README swept of CLI prerequisites.Accepted divergences from the old CLI (documented in ADR-0009)
listTagscounts frontmatter tags only (not inline#tags) — consistent withquery_notes.setPropertyno longer registers new property types in.obsidian/types.json.createNoteapplies no Obsidian templates (callers pass fully-formed content).Test plan
npm test— 748 passing;npm run lint,npx tsc --noEmit,npm run buildall clean.get_vault_overviewtest runs against a rejecting (dead) CLI.obsidianoffPATH, drovecreate_note→read_daily→set_property→list_tags→get_vault_overviewagainst a temp vault via the production factory wiring — all succeeded, disk side-effects verified, noCLI_*leak.Non-blocking follow-ups are recorded in the change's
retrospective.md§5.Release note: merge to
main, thennpm run releaseonmain(major bump; theBREAKING CHANGEfooter drives the changelog).🤖 Generated with Claude Code