[aliasunder]

Summary

Vault changes that landed while the server was starting could stay wrong in search until that file changed again or the server restarted: a deleted note stayed searchable, an edited note kept its old text, and a new note was missing. The startup rebuild read the vault first and the file watcher started only afterwards, so anything that changed in between produced no event. With Obsidian Sync running beside the server, that window opens on every restart.

The file watcher now starts before the rebuild and holds what it reports. When the rebuild returns, the watcher goes live and one batch checks each held path against the disk and applies it.

What changed

• watchVaultAndBuildIndex (src/vault-mcp/search/file-watcher.ts) replaces the separate rebuild and startFileWatcher calls in server.ts:
  1. The watcher starts holding, and startup waits for chokidar's initial scan. Every change chokidar reports from then on is held.
  2. rebuildFromVault runs. The watcher is the only other writer of the index, so nothing writes while the rebuild reads, and its older snapshot can no longer replace a newer change.
  3. The watcher goes live. In the same synchronous step it removes each held path whose latest event was a deletion, then reserves an event token for every held path. A live change to a held path replaces its token, so the batch then skips that path's removal or drops what it read. Removing the held deletions first means a note Sync moves during startup still takes the links that pointed at its old path.
  4. One batch checks each held path against the disk: paths gone from disk are removed first, then the rest are read again, 16 at a time. PDFs whose text is extracted are read after the others, 4 at a time, the same bounds the rebuild uses. Every folder on the path must list the next name exactly, so a case-only rename of a file or a folder on a case-insensitive filesystem does not index the old path again. A read that finds the file gone leaves it to the deletion event chokidar sends for it.
• The server starts listening once the batch has finished writing to the index, or after 10 s if a Sync backlog makes it longer; the batch then finishes in the background. Index writes are awaited; embeddings are not.
• If the rebuild fails, the watcher is closed before the error reaches the caller, and a rescan or retry it had already scheduled neither watches nor indexes anything. The server exits on that failure, so this matters only to a caller that keeps running, and a live test cannot reach it.
• The handlers keep their live behaviour. handleChange's body is split so the batch can read under its reserved token and wait for a file's index write without waiting for its embedding.
• rebuildFromVault's docstring states that no source write may run during it, and that a new runtime writer must go through the watcher's hold. Its orphan-vector sweep, second links clear and returned embedding promise gain comments, and two version checks become named booleans, with no behaviour change.
• The rebuild and the batch share one exported read bound, INDEXING_FILE_READ_CONCURRENCY, and one PDF bound, PDF_EXTRACTION_CONCURRENCY.
• ARCHITECTURE.md: the Indexing section gains a Startup order list, and the container startup sentence states the new order.
• deploy/local/README.md and deploy/remote/README.md: the Restart sections list the startup tasks in the new order.
• ARCHITECTURE.md, the first-start list in deploy/local/README.md and a comment in search-index.ts now say a note is found by keyword only until its own vectors exist. They said search stayed keyword-only until the whole embedding pass finished, but vectors kept from earlier runs are reused, and semantic matches appear as each note is embedded.

Known limits, unchanged by this PR:

• A file created after the watcher is ready and deleted within chokidar's 2 s write-stability window emits no event at all, so if the rebuild read it, it stays searchable until the next restart. Sync writes files in place, so this needs a short-lived file during startup.
• A file created between chokidar's listing of a folder and that folder's watch registration is not tracked by chokidar (chokidar issue 1471, fixed in the unreleased chokidar 6.0.0).
• Incremental link resolution after a rename keeps today's behaviour: links re-resolve only when the linking note is indexed again.
• A rename that changes only letter case leaves incoming links on the old spelling, at startup and live, because link resolution compares paths case-sensitively.

Behaviour this PR introduces:

• Once the 10 s deadline passes, the server answers while the batch is still running. Until it finishes, search misses or shows older text for the paths it has not reached, and vault_move_note's backlink rewrite skips links in notes not yet indexed. Changes after the watcher goes live, including the server's own writes, are indexed as they arrive.
• When a live change takes over a held path during the batch, startup does not wait for that live read. For about one file read after the server starts (one text extraction for a PDF), search keeps the path's earlier state: its text from before startup, or no entry for a path created during startup.
• A note deleted and created again during startup is absent from search from the moment the watcher goes live until the batch reads it again.
• The batch reads each held path without waiting for its writes to settle. If Sync is rewriting a held file at that moment, the batch can index part of it, and search shows that partial text, also after the server starts answering, until chokidar's event for the write arrives about 2 s after the writes stop and indexes the whole file.

Tests

• file-watcher.test.ts: a controlled-startup suite (a deferred rebuild and an emitter the test fires) covers:

  • held changes and deletions, and the disk check over the event kind;
  • held deletions removed when the watcher goes live, including one whose file came back and one held after a change to the same path;
  • a rename whose new path arrives live during the batch, and rename ordering when both paths are held, including an old path whose latest held event is a change, which the batch removes before its reads;
  • a live add during a held path's check, and the token reserved when the watcher goes live rather than at the check;
  • deletions between the check and the read: the batch's read changes nothing, and the deletion event that follows removes the rows;
  • live events during the batch, so the latest state on disk wins;
  • contained read, stat and embedding failures, and an unexpected batch failure;
  • the read and PDF bounds, and the deadline and its 10 s default;
  • case-only renames of a file and of its folder, including a queued read of the old spelling;
  • folders that are missing or unlistable;
  • a rebuild failure that closes the watcher.

  A held change wins over the rebuild's older read of the same note. A real-chokidar test repeats the original reproduction (edit, create and delete during the rebuild's reads) end to end.

• The existing watcher tests run through a helper that stubs the rebuild. Their loose length and call-log assertions became exact matcher assertions in test-only commits.

• server-integration.test.ts: a note written through vault_write_note after startup becomes searchable, which fails if the server never stops holding.

• Each new behaviour was checked against a mutation of the code that turns it off, and each mutation fails its test on an index-state assertion:

  • dropping the token check before a batch removal, or reading before the removals;
  • removing a path when the batch's read finds it gone;
  • reading under a fresh token instead of the reserved one;
  • skipping, or keeping the earliest event kind for, the deletion removal when going live;
  • reserving tokens before that removal, or leaving the removed paths out of the batch;
  • checking presence with stat alone;
  • going live only after the batch;
  • reserving each token at the path's check.

• npm test: 116 files, 5,402 passed, 2 skipped. npm run build, lint (0 errors) and knip pass.

Live Test Results

Head: 5c14538

The Test Deploy workflow (run 39334401147, skip_build: false) built this head and restarted a live :remote deployment: 2,405 notes, with Obsidian Sync running. The run log shows org.opencontainers.image.revision=5c145387aa854eff62dd23ae2d8de3937e750c49 and the container Recreated, Started and Healthy. docker image inspect on the running container reports the same revision.

Two probe notes were written before the restart. A script then waited for the new container's file watcher started line and, 0.5 s into the rebuild, edited one note, created a second and deleted the first:

Check	Expected	Result
Changes land while the rebuild reads	after file watcher started, before rebuilt index	written at 22:24:54.4 UTC; the rebuild ran from 22:24:53.8 to 22:25:42.5
Watcher holds them	batch line shows the held paths	applied the changes held during startup indexing with heldPaths: 3
Edited note	new word found, old word gone	heldedit679new finds the note, and its indexed text no longer contains heldedit679old
Added note	found	heldadd679 finds it
Deleted note	gone from search and disk	helddelete679 returns no match for it; reading the note returns "note not found"
Health	/healthz answers within 10 s of rebuilt index	first 200 at 22:25:43.7, 1.3 s after rebuilt index and 0.5 s after server started; container healthy

• No change arrived while the batch itself ran, which took under a second, so a live change taking over a held path was not exercised live. The unit tests cover it.
• From file watcher started to server started took 49.4 s, of which the rebuild took 48.7 s.
• The probe notes were removed afterwards.
• The commits after this head change only comments and documentation.

The earlier design was live-tested at afb456c20bb8bf46d89b45beac868ee942fe531b before this rework; that run does not cover this head.

Local before and after

The released v0.54.15 image and this branch's local image ran against the same generated 4,000-note vault. A script edited, deleted and created probe notes from the server's "config loaded" line until it listened, then checked each probe with vault_search:

Image	Probe changes	Wrong in search afterwards
v0.54.15 (released)	316	305
this branch	282	0

• Startup time, from "config loaded" to "server started", with one write every 100 ms during startup on the same vault:

  Mode	v0.54.15 (released)	this branch
  native file events	8.7 s	7.4 s
  polling (WINDOWS_MODE=true)	8.0 s	7.4 s

  In the window run above, in native mode, startup took 9.8 s on the released image and 8.7 s on the branch.

• A burst of 1,500 new notes on a 10,000-note vault tripped the 10 s deadline as designed: /healthz answered 10.2 s after the rebuild, and all 1,500 notes were searchable once the batch finished.

🤖 Generated with Claude Code

[umm-actually]

Update the startup-task order in the Restart section
Low severity · conventions · high confidence

deploy/local/README.md:209 — beyond the diff's line ranges, in code the changes touch or depend on.

The Restart section still lists the boot tasks as rebuild-then-watcher, the order this PR inverts: the watcher now starts before the rebuild, holds changes during it, and applies them afterwards, as ARCHITECTURE.md's new Startup order paragraph states. An operator reading the guide mis-models when watching begins.

Failure scenario: An operator reading the Restart section concludes the watcher only starts after the index build, so changes landing during a rebuild are lost — the bug this PR fixes — and mis-diagnoses post-restart search staleness instead of the held-change application.

Suggested fix

Reorder the list to: 1. create memory templates; 2. start the file watcher; 3. rebuild the search index while the watcher holds changes; 4. apply the held changes — matching ARCHITECTURE.md → Startup order.

---

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

[umm-actually]

Update the startup-task order in the Restart section
Low severity · conventions · high confidence

deploy/remote/README.md:434 — beyond the diff's line ranges, in code the changes touch or depend on.

The Restart section still lists the boot tasks as rebuild-then-watcher, the order this PR inverts: the watcher now starts before the rebuild, holds changes during it, and applies them afterwards, as ARCHITECTURE.md's new Startup order paragraph states. An operator reading the guide mis-models when watching begins.

Failure scenario: An operator reading the Restart section concludes the watcher only starts after the index build, so changes landing during a rebuild are lost — the bug this PR fixes — and mis-diagnoses post-restart search staleness instead of the held-change application.

Suggested fix

Reorder the list to: 1. create memory templates; 2. start the file watcher; 3. rebuild the search index while the watcher holds changes; 4. apply the held changes — matching ARCHITECTURE.md → Startup order.

---

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

[umm-actually]

umm-actually re-reviewed at bc4aeea

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

Context notes

• Priority docs already in context: ARCHITECTURE.md, deploy/local/README.md, deploy/remote/README.md
• 5 related file(s) excluded by max_related_files cap: src/vault-mcp/search/__tests__/memory-recall.test.ts, src/vault-mcp/search/__tests__/search-helpers.test.ts, src/vault-mcp/search/__tests__/search-index.test.ts, src/vault-mcp/search/__tests__/task-queries.test.ts, src/vault-mcp/vault-operations/__tests__/trash-sweeper.test.ts

---

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

[aliasunder]

Both Restart-section findings (issue comments 6222746376 for deploy/local/README.md and 6222746619 for deploy/remote/README.md) are valid and fixed in be480ea. Each guide now lists the startup tasks in the new order: create memory templates, start the file watcher (which holds the changes it sees), rebuild the search index, then apply the held changes. ARCHITECTURE.md's Startup order step 4 was updated in 77c3c72 to match the bounded replay.

---

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

[umm-actually]

Pin the default held-changes deadline in a test
Low severity · tests · high confidence

src/vault-mcp/search/__tests__/file-watcher.test.ts:2578 — beyond the diff's line ranges, in code the changes touch or depend on.

DEFAULT_HELD_CHANGES_DEADLINE_MS (10 s) is the only watcher default this suite leaves unpinned: the stability threshold, poll interval, polling interval, and the rescan-delay default each have a drift-catching test, and the 10 s value carries a documented health-check-budget rationale in file-watcher.ts. A silent bump to the constant would outlast the deployment health-check start period it was sized against with nothing failing.

Failure scenario: DEFAULT_HELD_CHANGES_DEADLINE_MS is raised (e.g. to 60_000): npm test stays green, but on a deployment whose health-check start period the constant's doc comment budgets for (the 20 s local compose start period it cites), a Sync backlog during startup keeps the server from listening past the start period, so the container flaps unhealthy and restarts with no test catching the regression.

Suggested fix

Add a fake-timer pin beside the rescan-delay default test: stub the rebuild to resolve, hold one batch read open via gateFirstRead and never release it, vi.useFakeTimers({ toFake: ["setTimeout"] }), then advance 9_999 ms and assert the "startup continues while the changes held during it are still applied" info line has not fired, and the final 1 ms makes it fire — the same 399/400 pattern the rescan-delay default test already uses.

---

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

[aliasunder]

Issue comment 6223429953 (pin the default held-changes deadline) is valid and fixed in 74aa8cf. A new test, "returns 10 seconds after the rebuild by default while held changes are still applied", holds the batch's one read open and fakes only setTimeout. It checks that the deadline log has not fired at 9,999 ms and has fired at 10,000 ms, with { heldPaths: 1, heldChangesDeadline: 10_000 }. Raising the constant to 12 s makes it fail on that assertion.

---

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

[umm-actually]

Assert the asset upsert's arguments, not just its call paths
Medium severity · tests · medium confidence

src/vault-mcp/search/__tests__/file-watcher.test.ts:184 — beyond the diff's line ranges, in code the changes touch or depend on.

Pre-existing: the vanished-non-md test reads only the first argument out of the recorded call log, so the byte count it passes nowhere is unpinned — the test passes even when upsertNonMdFile receives a wrong size. AGENTS.md bans positional call-log readback in favour of the matcher API, and the same file's other assertions were converted to exact matcher form in this PR. This is the same-pattern residue of that sweep.

Failure scenario: A regression that records the asset row with a wrong size (upsertNonMdFile('kept.png', 0)) still passes the test, because the assertion compares only mock.calls[i][0] against the surviving path list.

Suggested fix

Replace the readback with a matcher assertion that pins both arguments: expect(upsertNonMdFileSpy).toHaveBeenCalledExactlyOnceWith("kept.png", Buffer.byteLength("here")) — and drop the separate calls.length wait, or keep it as a bare precondition.

---

umm-actually · deepseek/deepseek-v4.1-flash

[aliasunder]

Issue comment 6225996333 (assert the asset upsert's arguments) is valid and fixed in 1443fa1. The vanished-file test now asserts toHaveBeenCalledExactlyOnceWith("kept.png", Buffer.byteLength("here")), and recording the row with size 0 fails it. The same commit fixes the other call-log readback in this file: the chokidar option tests assert the whole options object through toHaveBeenCalledExactlyOnceWith, so an interval passed without polling fails three of them. The remaining mock.calls reads in this file are wait conditions, not assertions.

---

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

[aliasunder]

The startup batch was reworked into one mechanism (a29150c through c0fb410, now rebased as 5c723f8). The watcher goes live before the batch, so the replay of changes from during the batch no longer exists. Here is where each earlier review finding stands on the current head:

Finding	Where it stands
Bound the startup replay's file reads (thread on file-watcher.ts)	The replay is gone. The batch's own reads keep the bounds of 16, and PDFs whose text is extracted 4 at a time. Changes after the watcher goes live are applied as they arrive, the same as every live change; the PR body lists that under "Behaviour this PR introduces". The 40-change replay test went with the replay.
Replay held deletions with live tokens (thread on file-watcher.ts)	The replay is gone. Held deletions are now removed in the same synchronous step that makes the watcher live, before any token is reserved, and a live change to a held path replaces its reserved token. "points incoming links at a renamed note whose new path arrives live during the batch, after $label was held" (a held deletion alone, and a held change then a deletion) and "removes a held deletion of a re-created note when going live, then reads it again" cover the orders.
Release the watcher's hold when the rebuild fails	Unchanged: a failed rebuild closes the watcher before the error reaches the caller.
Garbled module docstring sentence	Rewritten. The watcher holds every change until applyHeldChanges makes it live.
Restore the readFile mock in the two replay tests	One test was removed with the replay. The other became "drops a queued read of the old spelling when a case-only rename lands during the batch", which restores stat and readFile in its own cleanup.
State why the rebuild clears links a second time	Unchanged.
Restart sections in both deploy guides (issue comments)	Unchanged and still accurate.
Pin the default held-changes deadline (issue comment)	Unchanged: "returns 10 seconds after the rebuild by default while held changes are still applied".
Assert the asset upsert's arguments (issue comment)	Unchanged.

---

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