Skip to content

refactor: introduce client-side VaultFs boundary#37

Draft
enieuwy wants to merge 1 commit into
kavinsood:mainfrom
enieuwy:refactor/vaultfs-client-boundary
Draft

refactor: introduce client-side VaultFs boundary#37
enieuwy wants to merge 1 commit into
kavinsood:mainfrom
enieuwy:refactor/vaultfs-client-boundary

Conversation

@enieuwy
Copy link
Copy Markdown

@enieuwy enieuwy commented May 2, 2026

Goal

Introduce the client-side VaultFs storage boundary required before rebasing the headless CLI work. This PR migrates DiskMirror off direct app.vault.* file operations and onto a small filesystem interface, with an Obsidian adapter for the plugin and an in-memory adapter for tests.

Opening as a draft intentionally: please confirm this scoped PR-A boundary before detailed review.

Scope

Included:

  • VaultFs contract, errors, stats/list/read/rename/write/delete operations.
  • Shared vault path normalization (NFC, slash normalization, no leading/trailing slash).
  • ObsidianVaultFs preserving Obsidian semantics (Vault.delete, FileManager.renameFile).
  • FakeVaultFs and targeted DiskMirror tests for create/update/delete/rename, suppression fingerprints, frontmatter-blocked writes, target-exists rename, and missing-source rename.
  • DiskMirror now uses VaultFs for disk reads/writes/deletes/renames.

Explicitly not included in this PR:

  • The remaining raw app.vault.* callsites in src/main.ts for reconcile scans, vault event handlers, direct reads, and disk-index stat calls.
  • CLI NodeVaultFs / NodeDiskMirror refactor. That is the PR-B rebase follow-up after this boundary lands.
  • Orchestration unification into a shared DiskMirrorCore.

Follow-up after this merges

Rebase PR #16 (feat/headless-cli-fresh) on top of main, drop its duplicate normalizer, add NodeVaultFs, and move the CLI filesystem hardening behind that adapter while preserving the PR #16 hardening checklist.

Verification

  • node --import jiti/register --test tests/sync/diskMirror-vaultFs.test.ts
  • npm run build
  • npm run test:regressions
  • git diff --check
  • git log --oneline upstream/main..HEAD shows one refactor commit

Not run locally:

  • npm run test:e2e:obsidian failed before launching tests because wdio is not installed in this workspace (sh: wdio: command not found, exit 127). The two-peer manual Obsidian smoke was not run in this headless workspace for the same missing local harness reason.

@enieuwy enieuwy force-pushed the refactor/vaultfs-client-boundary branch from 3236105 to d925e74 Compare May 3, 2026 04:03
@enieuwy enieuwy force-pushed the refactor/vaultfs-client-boundary branch from d925e74 to aea8823 Compare May 3, 2026 04:46
@kavinsood kavinsood force-pushed the main branch 4 times, most recently from fc810fe to 3837c05 Compare May 18, 2026 15:24
@enieuwy
Copy link
Copy Markdown
Author

enieuwy commented May 23, 2026

Quick check before I rebase — upstream/main has moved ~120 commits since this branch was opened (runtime extraction into src/runtime/*, INV-SAFETY-02 in 48cfe67 + af61c03, the DiskMirror rewrite for RemoteDeleteDecision / PreservedUnresolvedRegistry / per-path write locks / setDiskWriteCallback, and the new canonical origin set in src/sync/origins.ts). Rebasing the existing DiskMirror migration on top of all that is doable but non-trivial, and the answer to two questions changes the shape:

1. Do you still want the client-side VaultFs seam as proposed? Text-only interface (normalize, readText, writeText, delete, rename, stat, listMarkdown) with ObsidianVaultFs / FakeVaultFs adapters, shared normalizeVaultPath (NFC + slash normalization), and exclude / frontmatter-guard behaviour staying where it currently lives. Yes / no / different shape?

2. If yes, do you want this PR shrunk to interface + adapters + tests only, with the DiskMirror migration as a focused follow-up? That makes the review trivial against current main and lets the recent DiskMirror rework settle. The alternative is to keep the current "boundary + DiskMirror migrated through it" shape, which is a more involved rebase against the new RemoteDeleteDecision / PreservedUnresolvedRegistry code paths.

Happy to push either shape on your call. I'll leave the draft as-is until I hear back so we don't churn on a direction you don't want.

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