Skip to content

fix(search): follow daily-note settings in orphan defaults - #642

Merged
aliasunder merged 16 commits into
mainfrom
fix/orphan-daily-folder-defaults
Oct 6, 2026
Merged

aliasunder merged 16 commits into
mainfrom
fix/orphan-daily-folder-defaults

Conversation

@aliasunder

@aliasunder aliasunder commented Oct 5, 2026 •

Copy link
Copy Markdown
Owner

What does this PR do?

Daily notes configured only in .obsidian/daily-notes.json currently appear as orphans. Resolve default exclusions on each tool or orientation-prompt invocation, so Obsidian settings changes apply without a restart.

Explicit request and environment lists still replace the defaults.

A shared folder-config module uses tolerant settings reads for orphan results and strict reads for delete/move protection. Protection preserves folder-name spaces and removes trailing separators before matching descendants.

Type of change

  • Bug fix

Validation

  • Full suite after rebase and review fixes: 4,515 passed, two skipped across 109 files.
  • Regression tests cover both adapters, descendants and sibling decoys, request/env overrides, live settings transitions, malformed-settings fallback, and result limits.
  • Build, lint, Prettier, Markdown lint, knip, and snapshot drift checks pass.
  • Only vault_find_orphans changes in captured tool surfaces. Against current main, the default list grows from 125,249 to 125,687 characters and remains 406 characters under the cap.
  • Oversized exclusion lists return too many excluded folders, verified over real HTTP; the description gives the caller a remedy. Internal logs retain the diagnostic.
  • CLI env blocks and Docker Hub documentation regenerated.
  • Empty values retain defaults; the documented comma-only value explicitly excludes nothing.
  • Daily-note forward references accept trailing separators without hiding broken links in sibling folders. The wiki map includes the extracted folder-config module and current config type.
  • Orientation summaries show one trailing slash, and local settings guidance describes bind-mount reads. CLI env blocks were regenerated after correcting both templates' restart instructions.

Live validation

Validated the user-deployed :test build at 5e2aafa2 after confirming its image revision and healthy container. Deployment run.

  • Public MCP defaults returned the exact expected 182 paths from 240 unfiltered orphans, excluding all 58 daily-folder paths. An explicit exclusion list restored those daily-folder paths, confirming replacement semantics.
  • Public delete returned the unchanged confirmation for two pruned folders. Persisted logs used prunedFolderCount and trashOption; all three scratch probes, their directory, and scoped index rows were cleaned up.
  • Sixteen checks against deployed compiled modules, real temporary filesystem/SQLite, and SDK in-memory transport passed: custom folder transitions, descendants and sibling decoys, limits, overrides, malformed fallback and repair, protection, capacity errors, orientation labels, and move/prune wire and log fields.
  • The move fixture returned pruned_empty_folders: 2 with exact rewritten links and unchanged target bytes; internal log keys were camelCase. SDK sessions closed and the fixture directory was removed.

The live vault uses Daily Notes, so custom-folder changes were exercised in the isolated image fixture. Move testing also stayed in that fixture because the real vault's automatic audit backlinks would cause the move to rewrite the audit note. No shared settings were changed.

Checklist

  • npm test passes
  • npm run lint passes
  • npm run prettier:check passes
  • npm run build succeeds
  • README and ARCHITECTURE.md updated

🔍 ship-check · ship-check · gpt-6.1-sol

@umm-actually

umm-actually Bot commented Oct 5, 2026

Copy link
Copy Markdown

Update ORPHAN_EXCLUDE_FOLDERS's stale registry description
Low severity · conventions · high confidence

server.json:166 — beyond the diff's line ranges, in code the changes touch or depend on.

server.json's ORPHAN_EXCLUDE_FOLDERS entry still states the default daily-notes folder comes from DAILY_NOTES_FOLDER alone (else Daily Notes), omitting the .obsidian/daily-notes.json read this PR makes live — the claim README, DEPLOY.md, DOCKERHUB.md, the .env.example files, the CLI env blocks, and the compose comment now all state. The entry ships to the MCP registry and GHCR listing, and its PROTECTED_PATHS sibling already names the file read, so the omission reads as a deliberate claim that the vault's own settings don't drive the orphan default.

Failure scenario: A user reads the environmentVariables table published from server.json (MCP registry or the GHCR package page), sees 'Default: DAILY_NOTES_FOLDER (else Daily Notes)', concludes the vault's synced .obsidian/daily-notes.json never affects vault_find_orphans, and either sets DAILY_NOTES_FOLDER redundantly or reports the exact orphan-default gap this PR fixes as still broken.

Suggested fix
Rewrite the entry to match the other surfaces: "Comma-separated vault folder names excluded from vault_find_orphans. Default: the daily notes folder (read on each query from DAILY_NOTES_FOLDER or .obsidian/daily-notes.json, default \"Daily Notes\"), \"Templates\", MEMORY_DIR. When set, overrides the default entirely."

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

@umm-actually

umm-actually Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

umm-actually re-reviewed at 6038119

No new findings (9 tracked finding(s) across all runs).

Context notes
  • Priority docs already in context: README.md, ARCHITECTURE.md, .env.example
  • Priority docs not included: 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
aliasunder force-pushed the fix/orphan-daily-folder-defaults branch from a1a88a3 to 439eb03 Compare October 6, 2026 00:29
@aliasunder

Copy link
Copy Markdown
Owner Author

Fixed the ORPHAN_EXCLUDE_FOLDERS description in server.json. It now states the environment-folder precedence, per-query settings-file read, fallback, and whole-list replacement behavior.

The repo-wide search found 77 references across 39 files, including hidden templates and workflows. The other default descriptions agree with the resolver; the remaining references forward settings, test overrides, or appear in generated tool snapshots.


🔍 ship-check · pr-monitor · gpt-6.1-sol

Comment thread .env.example
@umm-actually

umm-actually Bot commented Oct 6, 2026

Copy link
Copy Markdown

Document the env value that excludes nothing from orphan defaults
Low severity · conventions · medium confidence

.env.example:235 — at or near a changed line, posted here because GitHub rejected the inline review.

ORPHAN_EXCLUDE_FOLDERS has no documented env value that turns the default exclusions off: an empty or whitespace-only value is treated as unset, so the only off state is the comma-only form (e.g. ', ,') that the unit tests pin but no user-facing surface mentions. The conventions require default-on settings to carry a greppable sentinel off switch (SYNC_CONFIGS=none, TRASH_RETENTION_DAYS=none), and this PR rewrote the comment without adding one — the empty-means-unset semantics predate the PR, but this rewritten block is the natural place to close the gap.

Failure scenario: A user who wants vault_find_orphans to surface daily notes and templates sets ORPHAN_EXCLUDE_FOLDERS= (empty) in .env expecting to clear the defaults — loadConfig treats it as unset, so the daily notes folder, Templates, and the memory dir stay excluded, and the README, .env.example files, DEPLOY.md, and server.json offer no documented way to express 'exclude nothing' short of reading the source for the comma-only trick.

Suggested fix
Either add a none-style sentinel in config.ts (ORPHAN_EXCLUDE_FOLDERS=none → empty override) or extend the new comment on every env surface (root + deploy/local + deploy/remote .env.example, cli/src/env.ts blocks, README/DEPLOY rows): 'Set to ", ," (any comma-only value) to exclude nothing; an empty value means the defaults.'

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

@aliasunder
aliasunder force-pushed the fix/orphan-daily-folder-defaults branch from 439eb03 to 75e2e58 Compare October 6, 2026 00:59
Comment thread src/vault-mcp/mcp-core/tools/search-tools.ts Outdated
Comment thread src/vault-mcp/mcp-core/tools/search-tools.ts Outdated
Comment thread src/vault-mcp/vault-operations/vault-folder-config.ts
@umm-actually

umm-actually Bot commented Oct 6, 2026

Copy link
Copy Markdown

Strip trailing separators before building the daily-notes forward-ref prefix
Medium severity · correctness · high confidence

src/vault-mcp/search/search-queries.ts:1460 — beyond the diff's line ranges, in code the changes touch or depend on.

Pre-existing: the daily-note forward-ref exclusion in getOutgoingLinks and brokenLinkCount builds its prefix as <folder>/ from the folder exactly as read from .obsidian/daily-notes.json, while every other consumer of that spelling in this PR normalizes it (resolveEffectiveProtectedPaths strips trailing separators; findOrphans strips them per entry). A trailing-separator folder setting therefore yields a doubled-slash prefix like Journal// that no stored link target matches, so expected daily-note forward references are reported as genuinely broken and inflate the orientation prompt's broken-link count.

Failure scenario: A vault's .obsidian/daily-notes.json contains {“folder”:“Journal/”} and a Journal note links [[Journal/2026-10-07]] before that daily note is created. vault_get_outgoing_links returns daily_note_forward_ref: false for the link, and vault-orientation's stats count it under 'broken links' instead of excluding it as a forward reference, because the exclusion prefix 'Journal//' matches no stored link target.

Suggested fix
In both consumers, strip trailing separators before forming the prefix — getOutgoingLinks: `const dailyNotesFolderPrefix = params.dailyNotesFolder ? `${params.dailyNotesFolder.replace(/\/+$/, "")}/` : null`; brokenLinkCount: `const excludedFolderPrefix = `${excludedFolder.replace(/\/+$/, "")}/`` — mirroring the normalization resolveEffectiveProtectedPaths already applies.

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

@umm-actually

umm-actually Bot commented Oct 6, 2026

Copy link
Copy Markdown

Update .devin/wiki.json's stale ServerConfig type name
Low severity · conventions · high confidence

.devin/wiki.json:25 — beyond the diff's line ranges, in code the changes touch or depend on.

Pre-existing: the Configuration Reference page's purpose still names 'the ServerConfig type', but config.ts declares VaultConfig — the same stale name this PR cleans out of the AGENTS.md tree and ARCHITECTURE.md's Object.freeze row while missing this third surface. The Vault Operations page's module list likewise predates the vault-folder-config.ts extraction the PR documents in AGENTS.md.

Failure scenario: DeepWiki re-indexes the repo and the Configuration Reference page describes a 'ServerConfig' type that no longer exists, so a contributor searching for it finds nothing; the Vault Operations page omits the module that now owns protected-path and orphan-default resolution.

Suggested fix
Change 'the ServerConfig type' to 'the VaultConfig type' on the Configuration Reference page, and add vault-folder-config.ts (protected folders + live orphan exclusion defaults) to the Vault Operations page's module list.

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

@aliasunder

Copy link
Copy Markdown
Owner Author

Addressed both issue-comment findings:

  • #6007773233: both daily-note prefix builders now strip trailing separators. Six real-SQLite cases cover plain, single-slash, and repeated-slash folder settings; a sibling-folder decoy remains broken. Restoring the old code makes the four slash cases fail.
  • #6007773328: the wiki config page now names VaultConfig, and both vault-operations inventories include vault-folder-config. The repo-wide sweep found no remaining ServerConfig reference.

The full suite passed 4,513 tests with two skipped. Build, lint, formatting, knip, and diff checks passed; tool definitions and captures are unchanged by these fixes.


🔍 ship-check · pr-monitor · gpt-6.1-sol

Comment thread deploy/local/.env.example Outdated
@umm-actually

umm-actually Bot commented Oct 6, 2026

Copy link
Copy Markdown

Strip trailing separators from the excluded folder in the stats segment
Low severity · subtle bugs · medium confidence

src/vault-mcp/mcp-core/prompts/vault-orientation-prompt.ts:110 — beyond the diff's line ranges, in code the changes touch or depend on.

brokenLinkCount strips trailing separators when matching forward-refs but echoes the raw folder spelling in its result (pinned by the search-index test for the 'Daily Notes///' inputs), and formatBrokenLinkSegment appends another '/' when rendering the stats segment — so a daily-notes.json folder set as 'Journal/' renders 'excludes N forward-refs in Journal//'. Every other consumer this PR touches normalizes the spelling; this segment is the one place the raw value reaches users.

Failure scenario: A vault's .obsidian/daily-notes.json sets {“folder”: “Journal/”} and a Journal note links [[Journal/2026-10-10]] before that daily note exists: brokenLinkCount correctly excludes the forward-ref from its count but returns the raw spelling, and vault-orientation renders the stats line as '… 1 broken link (excludes 1 forward-ref in Journal//).' with a doubled separator.

Suggested fix
Normalize before interpolating in formatBrokenLinkSegment: const excludedFolderLabel = excludedFolder.replace(/\/+$/, "") and render `in ${excludedFolderLabel}/`. (Alternatively strip the separator in brokenLinkCount's returned excludedFolder and update the it.each expectation in search-index.test.ts that pins the raw echo.)

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

@aliasunder

Copy link
Copy Markdown
Owner Author

Fixed the stats display reported in issue comment #6008120332: it now shows exactly one trailing slash for the excluded folder. The query's returned metadata remains unchanged, and real prompt tests assert the full stats line for single and repeated suffixes.

Removing display normalization makes both regression cases fail. The restored fix passed the full suite of 4,515 tests with two skipped, plus independent bug and wording reviews.


🔍 ship-check · pr-monitor · gpt-6.1-sol

Ship-Check: pr-review · gpt-6.1-sol
Ship-Check: fresh-eyes · gpt-6.1-sol
Ship-Check: fresh-eyes · gpt-6.1-sol
Ship-Check: code-quality · gpt-6.1-sol
Ship-Check: code-quality · gpt-6.1-sol
Ship-Check: test-audit · gpt-6.1-sol
Ship-Check: bug-check · gpt-6.1-sol
Ship-Check: pr-monitor · gpt-6.1-sol
Ship-Check: pr-monitor · gpt-6.1-sol
Ship-Check: bug-check · gpt-6.1-sol
Ship-Check: pr-monitor · gpt-6.1-sol
Ship-Check: pr-monitor · gpt-6.1-sol
Ship-Check: code-quality · gpt-6.1-sol
@aliasunder
aliasunder force-pushed the fix/orphan-daily-folder-defaults branch from c391b53 to 6038119 Compare October 6, 2026 03:10
@aliasunder

aliasunder commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner Author

Rebased onto current main with the merged naming changes and stronger note-mover assertions. Range-diff shows all sixteen patches unchanged, and regenerated captures are byte-identical.

Combined verification: 4,515 tests passed, two skipped; build, lint, knip, and 21 snapshot checks passed. Default tool size remains 125,687 characters, 406 below the cap.


🔍 ship-check · pr-monitor · gpt-6.1-sol

@aliasunder
aliasunder merged commit 5e2aafa into main Oct 6, 2026
18 checks passed
@aliasunder
aliasunder deleted the fix/orphan-daily-folder-defaults branch October 6, 2026 03:27
@aliasunder

Copy link
Copy Markdown
Owner Author

Live validation passed on the deployed 5e2aafa2 test image after its revision and container health were verified.

  • Public MCP defaults returned the expected 182 of 240 orphan paths, excluding 58 daily-folder paths. Explicit exclusions replaced the defaults.
  • Public delete/prune confirmation and persisted camelCase count/trash fields passed. Three scratch notes, the probe directory, and scoped index rows were cleaned up.
  • Sixteen isolated SDK checks using the deployed modules and real temporary filesystem/SQLite passed, including file-configured folder changes, fallback/repair, overrides, protection, capacity, orientation labels, and exact move/prune wire/log results.

Custom-folder and move checks used the isolated image fixture, with all SDK sessions and temporary files cleaned up. The public transport checks remained separate; shared settings were unchanged. The PR body now contains the results and scope.


🔍 ship-check · verify · gpt-6.1-sol

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