Project conventions for AI-assisted development on vault-cortex — for Claude Code and other AI agents.
Remote MCP server exposing an Obsidian vault over HTTPS. One two-target
Dockerfile builds the ghcr.io/aliasunder/vault-cortex image: the local
target (:latest, the default stage) is tini + the MCP server alone; the
remote target (:remote) adds s6-overlay supervising both obsidian-sync
(bidirectional Obsidian Sync via the obsidian-headless npm CLI) and the MCP
server in a single container — s6 service definitions live in rootfs/, and
the init chain registers the initial Sync device under the DEVICE_NAME env
var, or the container hostname when unset. Both
processes run as UID 1000 (PUID/PGID-adjustable). Production runs the
:remote image on Lightsail as a single Compose service, fronted by API
Gateway with a Lambda authorizer (path-aware: OAuth endpoints pass
through, /mcp validates static token or JWT). IaC via SST v4.
The server provides vault CRUD, hybrid search (FTS5 keyword and sqlite-vec
vector results fused by Reciprocal Rank Fusion, then reranked by a
cross-encoder; the vector and rerank stages are optional via
EMBEDDING_ENABLED and RERANK_MODE; see ARCHITECTURE.md → Hybrid Search),
and an optional memory layer (dated-entry notes in the vault's About Me/
folder by default, set by MEMORY_DIR). The Docker image uses Debian
slim (node:24-trixie-slim) because onnxruntime-node requires glibc,
and specifically trixie because better-sqlite3 v13's bundled linux-arm64
prebuild needs glibc >= 2.38 (bookworm's 2.36 crash-loops arm64 images).
All solutions must be portable — they can't rely on one-off manual fixes, hardcoded paths, or user-specific configuration. If it works only on the author's machine, it's not done.
Design for the Obsidian user. The end user is always an Obsidian user, so
anything that mirrors an Obsidian concept — backlinks, outgoing links, orphans,
the graph, tags, properties, daily notes — must match what Obsidian itself does.
At minimum, recognize every form Obsidian recognizes; behavior that is a strict
subset of Obsidian's is a bug, not a limitation. For link resolution
specifically, that means all of Obsidian's link styles ([[wikilink]],
[[wikilink|alias]], [[wikilink#heading]], ![[embed]], [md](path.md),
),
links in frontmatter properties (e.g. related:), and all three "New link
format" modes — shortest path, path from vault folder, and path from current
file (including relative ../ paths).
See ARCHITECTURE.md for the full design.
The tree lists the main folders, a few root files, the loose files in src/
and src/vault-mcp/, and single-file folders. Where the names help, a
folder's comment names its main modules, without the .ts extension. Inside
a folder, a file gets its own line only when its comment states a constraint
the name does not show. Test folders appear only when their harness needs
explaining. Routine files (READMEs and other top-level docs, compose files,
tool configs such as package.json, tsconfig.json, and eslint.config.ts,
and the .github/, .devin/, and .husky/ folders) are left out; run ls
on a folder for the full list.
server.json # MCP server registry manifest
render.yaml # Render Blueprint; stays at the repo root, the only place Render reads it
Dockerfile # Two-target build: local (default) + remote
.claude/ # Committed Claude Code hook setup only (the rest is gitignored): settings.json registers the hook below
hooks/install-deps.sh # Runs at session start and worktree entry: loads nvm, installs dependencies when needed (npm ci, sst install), and at session start puts the checkout's Node (the .nvmrc version when nvm has it, else nvm's default alias) first on PATH for the session's later Bash commands
obsidian-headless/ # Lockfile-pinned obsidian-headless Sync CLI for the :remote image
rootfs/ # Container filesystem overlay for the :remote image: s6 init chain and services in etc/s6-overlay/, and usr/local/bin/get-sync-token, an in-container terminal sign-in to Obsidian that prints the Sync token for .env
templates/memory/ # About Me/ memory file templates for new vaults
deploy/ # End-user quickstarts (no clone needed): local/ and remote/ (guide, compose file, .env.example), render/ and railway/ (one-click guides; the Railway template definition lives in CONTRIBUTING.md)
assets/ # Static assets (not shipped in Docker)
scripts/ # Dev and ops helpers in TypeScript (not shipped in Docker); most open with a header comment saying what they do
deployment-env.ts # Loads ~/.config/vault-cortex/.env; shell variables override its values
instance-env.ts # Derives PUBLIC_URL for manual deploys (npm run lightsail:up) the same way sst.config.ts and deploy.yml do, or the Lambda authorizer rejects tokens
tool-surface-capture.ts # Boots one server per settings combination for the tool-surface snapshot test, size report, and LobeHub manifest; an on/off setting that gates tools goes in its SURFACE_AXES
cli/src/ # npx vault-cortex CLI, run on the host: bin (entry) → main (wiring) → program (Commander), one module per command (init, configure, upgrade, get-sync-token; lifecycle holds start, restart, logs, and down), and the shared modules they use (prompts, docker, env, scaffold, and others)
__tests__/integration/ # Interactive flows through node-pty in a real PTY: pty-harness, the cli-pty tests, and a fake docker binary in fixtures/
src/
logger.ts # Root logger (structured JSON, source location)
auth.ts # Auth helpers shared by the Lambda authorizer and vault-mcp/
jwt.ts # Minimal HS256 JWT sign and verify for the Lambda authorizer and the OAuth server
utils/ # Generic helpers with no domain logic (admission rules under Module layering)
functions/authorizer.ts # Lambda: path-aware auth (OAuth pass-through, JWT + static)
__tests__/integration/ # Server tests over real HTTP through the SDK Client; files listed under "Integration tests — when to add"
__tests__/docker/ # :remote image boot tests (npm run test:remote-boot): docker-harness, and fixtures/ob, a stub of the obsidian-headless CLI
vault-mcp/
server.ts, config.ts # Entry point and env-var loader, the only loose files here
obsidian-markdown/ # Pure parsers and transforms with no I/O (pdf-engine excepted): lines, frontmatter, callouts, headings, links, tags, tasks, recurrence, memory-entries, canvas, plaintext, pdf + pdf-engine, moment-format
vault-operations/ # Vault read, write, and patch: vault-filesystem (base I/O), vault-patcher, note-mover, memory-store, daily-notes, vault-folder-config, task-mutations, task-format-config, trash-config, trash-sweeper, asset-operations
mcp-core/ # MCP protocol surface: mcp-router, tool-registry, tool-availability, tool-definitions, prompt-definitions
tools/ # One <domain>-tools module per data-layer domain (vault-crud, search, task, memory, daily-note, asset), plus tool-helpers
prompts/ # One <name>-prompt module per prompt (vault-orientation, memory-review, daily-review), plus prompt-helpers
search/ # SQLite FTS5, hybrid search, file watching, embedding: search-index, search-queries, fts-query, hybrid-search, search-helpers, rrf, embedder, reranker, chunker, file-watcher
oauth/ # OAuth 2.1: oauth-provider, oauth-routes, consent-page
setup/ # Setup mode (browser sign-in to Obsidian Sync while the :remote image has no working token): setup-server (entry), setup-routes, setup-page, obsidian-api, vault-key, sync-token-store
The vault-mcp/ tree is organized in dependency layers — leaf layers
(obsidian-markdown/ parsers, plus src/utils/) → I/O and use-cases
(vault-operations/, search/) → protocol (mcp-core/, oauth/, setup/)
→ wiring (server.ts).
A module's folder is decided by what it depends on, not just its topic:
obsidian-markdown/— pure parsers/transforms over Obsidian's file formats (frontmatter, lines, headings, callouts, links). No fs, no SQLite, no MCP; they take strings/lines and return data or transformed strings, so they're trivially unit-testable. The folder's contract is the dependency profile, not the syntax family:canvas.tsparses JSON (JSON Canvas 1.0), but its text nodes and its linearized output are markdown, and it's the same pure leaf layer — Obsidian format parsers belong here regardless of whether the format is markdown, JSON, or YAML.lines.tsis the single home of the CommonMark §4.5 fence state machine (advanceFence) — every function that walks lines and tracks code fences calls it, so they can't disagree about where a fence opens. PDF engine exception:pdf-engine.tsis the one module in this folder that performs side effects — it resolvespdfjs-distpackage paths from disk viacreateRequireand mutatesglobalThis(canvas polyfill injection). It lives here because it is PDF domain logic: the bootstrap that configures pdfjs sopdf.ts's extraction pipeline works. The two PDF modules are a unit and belong together in the parser layer.pdf.tsimports it as a sibling for theextractPdfTextpipeline. Dual-format task mutations:tasks.tsreads and writes both emoji signifiers (✅,📅,⏫) and Dataview inline fields ([completion:: date],[priority:: high]). Mutation functions must strip both formats when removing a field (a Dataview-formatted task must not get fields orphaned).tasks.tstakes the format as a parameter;vault-operations/task-format-config.tsdecides it from the user's Tasks plugin settings (taskFormatin.obsidian/plugins/obsidian-tasks-plugin/data.json, emoji by default), and itssetDoneDate/setCancelledDateswitches control whether completion dates are stamped at all. When.obsidian/is not synced to the server,task-format-config.tsfalls back to emoji.vault-operations/— everything that reads/writes the vault.vault-filesystem.tsis the base I/O primitive (atomic writes, path-safety, read/list/delete);vault-patcher,note-mover,memory-store, anddaily-notesare use-cases composing it with the parsers. The line betweenvault-filesystem.tsandutils/fs.tsis policy vs. adapter:utils/fs.tsholds only policy-freenode:fswrappers (readFileOrNull,readdirOrNull,fileExists), while anything that encodes how the vault is written or guarded — the atomic-write strategy, vault-root path-safety, thevaultFsdata API — stays invault-filesystem.ts. "Mechanically generic" (an atomic write works on any file) isn't enough to demote something toutils/if it's load-bearing vault-I/O policy.mcp-core/— the MCP protocol surface.tool-registry.tsis the declarative registry: every tool's wire name, feature group, and MCP annotations in one leaf module with zero imports (config validation and tests consume it without loading the tool layer).tool-definitions.tsis the orchestrator: it computes the enabled tool set by filtering the registry through one AND-chain of predicates — group flags (MEMORY_ENABLED,FILE_TOOLS_ENABLED), read-only mode via each tool's ownreadOnlyHintannotation, and the subtractiveDISABLED_TOOLSlist — then invokes each group's register function with a gatedregisterToolwrapper that skips disabled names and injects the registry's annotations (the wrapper's config type has noannotationskey, so restating them inline is a compile error). A new gating axis is one new predicate, never new branching in group modules.tool-availability.tsis where "given the enabled set, how do you talk about tools" lives —isToolEnabled,whenToolEnabledText, andformatEnabledToolList(which narrows a list of tool names to the served ones and renders it as prose, empty when none survives). All three text surfaces — tool descriptions, prompt steps, and the router's server metadata — build on that one view, so a cross-reference disappears whenever its target does. It sits atmcp-core/root precisely becausetools/andprompts/both need it and cannot import each other. Each group module is self-contained: one register function and its data-layer imports, with tool names imported from the registry. Shared helpers (createSafeHandlers,formatNoteMetadata,ToolRegistrationContexttype) live intool-helpers.ts. Group modules takesafeHandlerandsafeHandlerContentfrom the registration context;createSafeHandlersbuilds them per server from the enabled tools and the vault path. Tool handlers stay thin: schema, wire mapping (snake_case ↔ camelCase), one data-layer call, and content-block/JSON formatting. Multi-step composition — filtering, counting, pagination, dispatching across parsers and I/O — is a use-case and belongs invault-operations/(asset-operations.tsis the worked example). The smell: a handler importing a parser to orchestrate between two data-layer calls; the fix is a use-case module, not a bigger handler.prompt-definitions.tsis the orchestrator that composesPROMPT_NAMESfrom three group modules undermcp-core/prompts/(vault-orientation, memory-review, daily-review) — mirroring thetools/pattern. Shared helpers (PromptRegistrationContexttype,textResult, andwrapWithDataMarkers, which wraps vault content in<vault-content>tags so the model treats it as data) live inprompt-helpers.ts.search/— SQLite FTS5 + sqlite-vec index, embedding pipeline, file watcher.oauth/— the OAuth 2.1 server (distinct from the sharedsrc/auth.tstoken utilities).setup/— the:remoteimage's setup mode. Contains the sign-in page served while the container has no working Obsidian Sync token, the Obsidian account API client, and the token store. A sibling surface tooauth/, not a layer below it: builds onconfig.ts,src/auth.ts, andutils/; nothing lower imports it (lint-enforced: each lower layer'sno-restricted-importsbanssetup/andoauth/).rootfs/etc/s6-overlay/s6-rc.d/svc-vault-mcp/runstartssetup-server.tsinstead ofserver.tswhen the init chain setsSETUP_MODE:init-check-authwhen no token exists,init-obsidian-loginwhen the saved token's login is rejected.utils/(atsrc/) — generic cross-cutting helpers.
Three rules keep this honest:
- Dependency direction.
obsidian-markdown/andutils/depend on nothing internal (leaf layers);vault-operations/andsearch/depend on those;mcp-core/and the top-level wiring depend on everything. A search module importing a parser should read as "uses the shared parser," never as reaching sideways intovault-operations/. - Group operations by shared dependency layer, not by topic. A domain's
operations live together only when they share a layer: asset read + browse
are both filesystem work, so
asset-operations.tsholds both. Task list (a SQL query — lives with the queries insearch/) and task update (a file mutation —task-mutations.ts) stay apart, and so do note search and note mutations. A topic-symmetric "one module per domain" grouping that crosses layers is the smell, not the goal — each file answers for one layer's view of its domain. - Top level is wiring only. Folders are domains; the only loose files at
vault-mcp/are the entry point (server.ts) and itsconfig.ts— the setup-mode entry point stays insidesetup/.
The dependency-direction rule is lint-enforced: eslint.config.ts bans runtime
cross-layer imports per folder via @typescript-eslint/no-restricted-imports
(type-only imports allowed — erased at compile time; tests exempt).
Tool-surface rules are lint-enforced too — the registry owns tool identity, and gating is derived once rather than re-decided per call site:
config.readOnlyModeis banned throughoutmcp-core/excepttool-definitions.ts, which is where the predicate lives. Everything downstream — descriptions, prompt steps, router metadata — keys on the enabled set throughisToolEnabled/whenToolEnabledText. The flag knows nothing aboutDISABLED_TOOLSor any axis added later, so branching on it is how a prompt ends up naming a tool the server never registered. Banned as a member access and as a destructured binding.- A local
TOOL_NAMESinmcp-core/tools/ormcp-core/prompts/is an error — import it fromtool-registry.ts. A local copy compiles and passes tests while drifting. prompts/andtools/cannot import each other at runtime. They are sibling surfaces, not a layer stack; a helper both need is either generic enough forutils/or belongs in that group's own helpers module.
Before editing these lint rules, read the comments in eslint.config.ts:
a narrower config block's rule options replace the wider block's instead of
merging, and import patterns match the specifier as written. Confirm any new
rule fires on a deliberately added violation.
utils/ admission: a helper belongs here only if it is generic with zero
domain knowledge (no vault, Markdown, or MCP concepts) and clears one of two
bars:
- (1) It removes real duplication — already called from more than one place
(
describeError,readFileOrNull). - (2) It's a complete, standalone primitive — you could name, describe, and
test it without mentioning any caller or the vault, and it would look at home in
a standard library (
mapWithConcurrency— a bounded-concurrency async map). This bar admits a single-caller helper; bar (1) does not.
Premature-abstraction guard: if the only way to explain the helper is "the part of
someFunction that does X," it fails bar (2) — it's a fragment, not a primitive,
so keep it private until a second caller appears. Markdown logic is domain — it
goes in obsidian-markdown/, never utils/.
import type from infrastructure modules (Logger, config types) is fine in
utils/ — type-only imports are erased at compile time and don't create runtime
coupling. Don't reinvent a type with a structural stand-in when the real type
exists.
Export style depends on what kind of module it is:
- Operation / data-layer modules — anything that performs vault or index
operations — export a single namespace object so call sites self-document
which module an operation belongs to:
vaultFs.readNote(…),vaultPatcher.patchNote(…),noteMover.moveNote(…),assetOperations.readAssetContent(…). Function count is irrelevant —noteMoveris essentially a single-operation module and still exports a namespace; "it only has one function" is not the named-export test. Stateful ones use a factory-closure returning that object (createSearchIndex,createMemoryStore), so prepared statements / caches live in the closure. - Parser, small-helper, and config-reader modules — the
obsidian-markdown/parsers (frontmatter,headings,callouts,lines),utils/, and the config readers (daily-notes,task-format-config,trash-config) — export named functions. The shape tracks whether a module performs operations (→ namespace) or parses/reads configuration (→ named), not whether it does I/O: the parsers are pure, while the config readers do light I/O, yet both use named exports because neither is an operation surface. links.tsis the deliberate edge case — a pure parser that nonetheless exports a singlelinksnamespace, not for the service-grouping reason above but to wall off its/ggrammar regexes (sharedlastIndexfootgun) behind position-safe methods.
vault-filesystem.ts illustrates the nuance within one module: its high-level
data API is grouped under vaultFs, but its low-level shared primitives
(resolveSafePath, atomicWriteFile, pruneEmptyParents, …) are named
exports — infrastructure consumed à la carte by other modules, not part of the
vault data API.
Root logger at src/logger.ts. Structured JSON to stdout/stderr.
Log format:
{
"timestamp": "...",
"level": "info",
"name": "vault-cortex",
"message": "read note",
"source": "vault-filesystem.ts:67",
"requestId": "1",
"sessionId": "abc",
"tool": "vault_read_note",
"clientIp": "203.0.113.42",
"path": "About Me/Principles.md"
}timestamp— ISO 8601source—filename.ts:line(auto-captured via V8prepareStackTraceon info/warn/error; skipped at debug level for performance)- Contextual properties (
requestId,sessionId,tool,clientIp) are carried by child loggers, not passed per call
Logger chain — context flows via .child():
root logger (src/logger.ts)
→ session logger: logger.child({ sessionId, clientIp })
→ request logger: sessionLogger.child({ requestId, tool })
→ passed to data-layer functions as required `logger` param
mcp-core/mcp-router.tscreates a session logger when a new MCP session initializes, addingsessionId+clientIp- Child props may be function-valued — resolved fresh on every emit.
Use
() => valuefor context that doesn't exist at child-creation time (the session logger'ssessionIdis generated by the SDK transport only while it handles the initialize request) - Each tool group module (
mcp-core/tools/*.ts) creates a request logger per tool call, addingrequestId(from the MCP SDK'sRequestHandlerExtra) +toolname (from the sharedTOOL_NAMESconstant intool-registry.ts) - Data-layer functions (
vault-filesystem,vault-patcher,note-mover,memory-store,search-index) take the logger as a required second argument (two-arg pattern:(params, logger)) - Background callers (file-watcher, startup) use the root logger directly — no request context available
Two-arg (params, logger) pattern:
All data-layer functions use named params + required logger:
vaultFs.readNote({ vaultPath, path }, reqLogger)
memoryStore.getMemory({ vaultPath, file, section }, reqLogger)
search.fullTextSearch({ query, filters }, reqLogger) // search: the SearchIndex on ToolRegistrationContextLog levels:
| Level | Meaning | Alert-worthy? |
|---|---|---|
error |
Something is broken — needs investigation | Yes |
warn |
Unexpected but not broken (bad client input) | No |
info |
Normal operations (tool calls, reads, writes) | No |
debug |
Verbose tracing (file watcher, dev only) | No |
info vs debug boundary: info is for per-request summaries —
one log per tool call or lifecycle event (startup, shutdown, rebuild).
debug is for per-file background events — fired by the file
watcher or during bulk indexing, with no user-initiated action. If the
log would produce N lines during a vault rebuild (one per note), it's
debug; if it produces one summary line, it's info.
Log content and security:
- Never log PII, credentials, tokens, or secrets — not in messages,
not in structured fields, not in tests (fake fixtures only). Log
identifiers (
sessionId), never identity payloads such as an account email or name. - Redact by destructuring the sensitive keys out:
const { password, token, ...safe } = payload. Keep the payload typed (noany), and neverdeletekeys from a copy. - If a child-process command contains sensitive values, catch a failure
at the call site and log a sanitized description. A rethrow must be a
new error without
{ cause }: the original message, and any cause chain carrying it, expose the full command. - Layer-appropriate messages: internal/data-layer functions describe what went wrong in their own domain and never name API surfaces (tool names, routes) or prescribe caller-level remediation.
- Log full detail internally, return generic messages externally —
error responses to clients never include the server's own paths,
stack traces, or implementation state. A Node error's paths reach the
client vault-relative, because the tool and prompt boundaries rewrite
them (
describeErrorRelativeTo), so a wrapper passes the Node error ascauseand never copies its message into its own. Errors the server throws itself keep their text, since they can quote the caller's input (a refused absolute path comes back as sent). /healthzneeds no sign-in, so never add deployment settings or host details to its response. The full server returns{ ok: true }, and setup mode addsmode: "setup"so the setup page can tell when the full server is back.
The server runs in Linux Docker — even on Windows and macOS, Docker
Desktop runs a Linux VM internally. Path operations (relative,
join, basename) produce POSIX separators (/) in all deployment
paths; WINDOWS_MODE=true handles Docker Desktop's WSL2 bind mount
limitations (no hard links, polling for file watching) but does not
change path separator behavior. Native Windows execution (without
Docker) is not a supported deployment and would break path handling
throughout the codebase.
These rules are authoring guidance, not a review checklist — apply them
while writing, not after. eslint.config.ts enforces this subset:
- No
functiondeclarations, andtypeoverinterface. - No
any,ascasts other thanas const, or!non-null assertions (warnings only in tests). - No nested ternaries, and no
elseafterreturn. - No single-character identifiers (lint allows
i,a/b,k,_). - No empty
catch {}blocks (lint does not flag.catch(() => {})). - A blank line between a declaration and the guard that consumes it.
- In
src/only: noconsole; Luxon overDateand noprocess.envmember access outsideconfig.ts(tests exempt). The rule misses a destructuredimport { env } from "node:process", whichlogger.tsuses. - The Module layering bans (layer imports,
config.readOnlyMode, a localTOOL_NAMES).
The rest are the author's responsibility at write time. The list below covers both kinds, with reasons:
- Functional over OOP. Arrow functions over
functiondeclarations. - Factory/closure pattern for stateful modules (see search-index.ts).
typeoverinterfaceunlessinterfaceis specifically required.- TypeScript strict mode.
node:prefix for built-ins. - Explicit return types on exports. Zod for MCP tool schemas.
- No
any,ascasts, or!(as constis allowed); use runtime guards or schema validation instead. When a library method returnsT | nullbut the null case is unreachable (e.g.DateTime.now().toISO()), throw on null — never fall back to?? ""or another sentinel, which passes the type checker and hides the bug downstream. - Blank lines separate logical sections — a declaration never runs straight into the guard that consumes it.
- Model states in the type system — reach for a discriminated union, a
user-defined type predicate
(
value is Type), ornever-exhaustiveness before reshaping an API to route around the checker. Optional fields doc-commented "present only in mode X" are the cue for a discriminated union; a callback param with a closed set of instantiations becomes a discriminated field naming the domain choice (every caller passes one of two formatters →format: "date" | "datetime"). Reuse type predicates in filters over union members. Keepx is Tguard bodies simple — predicates are compiler-trusted, not verified. A short&&chain is fine; when checks need a negatedinor||branches, early returns read clearer. - One discriminant represents one outcome. When two result shapes encode
the same statuses and need a converter between them (a boolean
okresult beside astatusresult for the same outcomes), use one status-discriminated union instead. - Prefer
async/awaitover.then()/.catch(). When.then()or.finally()is the natural idiom (e.g. promise-chain serialization queues), use it with a comment explaining the pattern. - Per-operation try/catch — each catch encloses one operation with
one failure meaning. Broad catch-alls are banned; bare
try/finallyfor resource scoping is fine. - Every catch logs or re-throws —
.catch(() => {})and empty catch blocks are banned; a swallowed error hides the failure. - Three tiers at input boundaries: invalid input → reject with a clear error; valid input with a surprising structural consequence → non-blocking advisory; machine-derived values (the stored value byte-exactly matches the tool's own earlier output) → auto-correct and report. User-authored values never auto-corrected.
- Required inputs enforced at every entry point — fail fast at boot/load, not only the friendliest launcher. Making an already-expected value mandatory is a bug fix, not a breaking change.
- Luxon
DateTimeover the nativeDateAPI, because Luxon is immutable and avoids manual millisecond arithmetic andsetDate()mutation: useDateTime.now(),.toISO(),.toISODate(),.toUnixInteger(). - Platform built-ins over manual string parsing — before hand-rolling
split/slice/regex over structured data (URLs, paths, headers, dates), check Node 24's stdlib (e.g. staticURL.parsereturningURL | null). A regex is the floor only when no built-in parser exists for the format, and then it gets a doc comment. - Standalone scripts are TypeScript (
.ts, tsx-compatible) — never.mjs/.js;scripts/meets the same type-safety and readability bar assrc/. - Default-on features get an explicit sentinel off switch
(
SYNC_CONFIGS=none), never empty/unset semantics — the sentinel is greppable and keeps${VAR:-default}compose interpolation working. - Public config switches are named for the user's mental model
(
READONLY_MODE— the term users search for), not internal family symmetry (WRITE_TOOLS_ENABLED); consistency governs internal code, not the public surface. - Immutable by default. Avoid
let— carry state in a reduce that returns a new accumulator each step, use early returns, or destructure conditional results. A bit of duplication is acceptable to keep code immutable and clear. Whenletis necessary (caching, parser state), add a comment justifying why mutation is needed here. Readability is the deciding gate — never refactor working, readable code into a more "functional" shape for its own sake. - Helpers do not mutate their inputs. Type collection parameters as
ReadonlyArray,ReadonlySet, orReadonly<T>views and return new values. Aconstlocal collection may be mutated in a plainfor…ofloop. - Don't disguise mutation as a fold. A
reducethat mutates its accumulator (acc.push(...),acc.count += …, thenreturn acc) is the worst of both worlds — it reads as declarative but isn't, so a reader has to mentally run it to see what it builds. Pick one and be honest: a genuine immutable fold (return a new value each step) for a real reduction to a single value, or a plainfor…ofloop with a justifying comment when the state is inherently sequential (a parser threading line-by-line state). A map-plus-sum is not a reduce —items.map(rewrite)then a separate, named count sum reads on its own. - Explicit names over abbreviations. Variable names should describe
what the value is, not use shorthand (
availableHeadingsnotavailable,searchTextnotneedle,fileContentnotraw,filesIndexednotcount). This applies everywhere: function params, callback params (rownotr,entrynote,orphannoto), SQL aliases (elementnotje), destructured bindings, and loop variables. For constants representing a category, name them for the domain contrast they represent —CONCRETE_STATUSEScommunicates the virtual/concrete distinction;ALL_REAL_STATUSESdoesn't (what makes a status "real"?). When a value flows through a multi-step pipeline (input → normalized → expanded → deduplicated), keep a consistent prefix so every variable clearly belongs to the same chain:statusInput→statusValues→statusValuesWithExpansions→expandedStatusValues, notinput→values→withExpansions→expanded. A reader scanning the function should see the domain noun on every intermediate, not just the first and last. - Alias generic destructured keys with their source when use sites do
not show the destructure:
{ on: modifiedOn }fromfilters.modified. Keep bare keys when they are self-describing or every use stays nearby. - Lean toward named records over positional tuples, and named locals over
inline expressions, where it helps a line read on its own —
{ start, end }over[start, end] as constdestructured as[spanStart, spanEnd];const linkText = match[0]over an inlinematch[0].length. Judgment, not a hard rule: an inline expression that's obvious in its context is fine. Optimize for readability in context, not mechanical extraction. - Break nested functional composition into named intermediate steps.
[...new Set(items.flatMap(transform))]nests three operations (spread, Set, flatMap) — a reader has to unpack inside-out. Split intoconst withExpansions = items.flatMap(transform)thenconst deduplicated = [...new Set(withExpansions)]so each line reads top-to-bottom. The threshold is ~2 nesting levels; a singleitems.map(f)or[...new Set(items)]is fine inline. - Extract multi-step callbacks into named functions when a
.map()/.reduce()callback builds multiple intermediates or nests chains — the parent becomesitems.map(formatItem).join("\n"). Conditional spreads (...(cond ? [item] : [])) and.filter(Boolean)assembly are both fine — pick whichever reads clearer; don't convert one to the other mechanically. Name non-trivial.filter()predicates. - A boolean mode param means the function does two things — split it; the caller owns the gating.
- Block bodies
{}for any multiline function response (guards, multi-clause booleans, multiline returns); expression bodies only for trivial one-liners. - Use named params when a positional value hides its role at the call site, even for one literal. Always use them above two args or for adjacent same-typed args that could transpose silently. Keep a clear one- or two-arg call positional.
- Scope constants to where they're used — module level overstates visibility.
- Function and helper names state what they do, specifically — a reader
should know what a function does without reading its body
(
collectWikilinksFromnotcollect,convertFrontmatterDatesToIsoStringsnotnormalizeDates). Value-returning functions name what they return (getPdfEngine, notensurePdfEngine— "ensure" reads side-effect when the return is the point). A docstring complements a self-documenting name; it never excuses a vague one. - No planning-session coinages in identifiers — a term invented during design means nothing to a stranger reading one file; sweep new identifiers before landing.
- Comment decision at write time (use
/** */; only when earned):- Can a reader understand this function from its name, params, and return type? → No comment. This is most functions.
- Something non-obvious? → One-line JSDoc stating the constraint or behavior the signature doesn't convey.
- Does the JSDoc restate the function name in different words? →
Delete it.
/** Gets the PDF engine. */ongetPdfEngineis noise. - Does it explain what the code does instead of why? → Rewrite to the constraint: why this shape, what breaks if changed.
- More than 2 lines? → Pick the format the reader absorbs quickest — bullets for parallel items, numbered steps for a sequence — never multi-paragraph prose that "breaks down" the blob into smaller blobs. If no structure fits, the comment covers too many concerns — trim to the one why.
- Inline comments at the relevant line, not everything in the docstring. The docstring states the outward contract; line-level concerns (why this guard, why this ordering) go as inline comments directly above the line they explain. When code moves into a helper, inline why-comments stay beside the call.
- SQL with branching logic (CASE, EXISTS subqueries) needs a comment explaining the overall strategy before the query.
- Regex constants get doc comments explaining what they match.
- Durable rationale only — never transition history, decision narrative, or the maintainer's own infrastructure details. OSS boundary: issue/PR numbers, incident dates, deployment names, task-board IDs, remediation narration, and investigation chronology never enter any public artifact — committed files, PR descriptions, or comments.
- No nested ternaries — a chained
a ? b : c ? d : eforces the reader to simulate branches. Decompose into named const steps or an early-return guard with a trailing ternary. - Early returns over nested
if/else— reduces indentation depth and cognitive load. Preferif (done) returnover wrapping 15 lines inif (!done) { ... }. In loops, preferif (cond) { …; continue }overif/else ifchains — each branch is self-contained and the reader doesn't have to track mutual exclusivity across the chain. - Extract multi-clause conditionals into a named boolean when the
ifcondition spans more than one line or combines unrelated checks. A reader should understand the guard's intent from the variable name without parsing the expression:if (hasDeletedNotes) {overif (deletedPaths.length > 0 && deleteVectorsForNoteStmt && ...) {. - Name booleans (params, flags, locals) for the affirmative state, and
let the value carry the negation:
hardLinksSupported: falsereads clearer thanhardLinksUnsupported: true, and a double negative likeif (!notReady)is a smell. This extends to guard booleans: name them for the action the guard controls —needsStatusFilterover!isUnfiltered, because the guard's intent is "do we need a filter?" not "is it not unfiltered?". A positively-named flag also keeps the guard's condition positive (if (hardLinksSupported) { … return }), so it pairs naturally with the early-return rule above — the common path returns, the fallback flows beneath it, noelse. - Prefer truthy/falsy checks over verbose comparisons for optional
values.
if (!taskLine)overif (taskLine === undefined);if (!finding.suggestion)overif (finding.suggestion === null || finding.suggestion === ""). Leverage optional chaining and nullish coalescing to flatten guard chains:heading?.text ?? "(none)"over a nestedif (heading) { heading.text } else { "(none)" }. Only use explicit comparisons when the falsy set is wrong (e.g.0orfalseare valid values). - TS ≥5.5 infers
.filter()predicate types from bare comparisons —xs.filter((x) => x !== null)narrows without(x): x is T.filter(Boolean)still does not narrow.Boolean(x)over!!x. - A param consumed only for truthiness is a boolean — a value-or-
""sentinel whose content is never read is a boolean wearing a string costume; type itbooleanand drop the dead value. - Don't use a thunk or callback when a plain value suffices. A
function accepting
() => TwhereTwould do adds indirection without benefit — the caller has to reason about evaluation timing, and the function body gains nothing from deferred computation. If the value is already available at the call site, pass it directly. - Data-layer helpers should not truncate or lossy-transform their output — the consumer decides presentation limits. A function that silently caps its return to N characters loses information the caller may need; if truncation is wanted, apply it at the call site. Extraction and presentation are separate concerns.
- Don't return observability data computed only for logging — the function has the logger; log at the site.
- Prepared statements at factory scope — compile SQL once, not per call.
- Multiplying config axes get a declarative registry + predicate chain, never per-flag branching. Declare per-item metadata once (reuse metadata items already carry) and compute the enabled set through AND-composed predicates — a new axis is one predicate; generated cross-references key on the enabled set, not on flags. Don't preserve alias/indirection layers to dodge migration churn — price the actual cost (usually mechanical import edits).
- Lint-enforce the mechanically-checkable conventions — layering via
per-layer
no-restricted-imports(allowTypeImports: true; tests exempt), style via core/typescript-eslint rules. Trial candidates against the codebase first; curated exceptions over blanket bans; never adopt a rule that fights an established idiom; a justifiedeslint-disable+ why-comment beats weakening the rule. - knip (
knip.json, pre-commit + CI) flags unused exports, types, dependencies, and files. Remove a flaggedexport(nothing imports it) rather than adding an ignore comment. - markdownlint (
.markdownlint-cli2.jsonc, lint-staged with--fix+ CI) enforces markdown structure: blank lines around fences, a language on every fenced block, no bare URLs. Ignores are documented in the config. - ShellCheck (
npm run lint:shell, lint-staged + CI) checks only the scriptsSHELL_SCRIPT_GLOBSinscripts/lint-shell.tslists, so add a new shell script's path there. ShellCheck cannot read#!/command/with-contenv sh, so s6 scripts that start with it carry# shellcheck shell=sh. A disable directive gets a comment saying why. - Simple code over clever code when the same outcome is achievable. A person should be able to read and follow the code without unnecessary cognitive overload. Working is the floor, not the bar — if "it passes" were enough, code review wouldn't matter. The first structure that compiles is rarely the simplest: before settling, ask whether it can be done with fewer moving parts and in fewer lines, and whether this is the shape that makes the most sense or just the first that came to mind. Each line should say what it does on its own — a reader shouldn't have to simulate the code to follow it.
These rules cover the two parts of a tool's definition that carry prose,
which are the description and the input schema with each parameter's Zod
.describe() text.
- MCP tool descriptions include
Example:,When to use:, andReturns:sections. IncludeErrors:whenever the tool has failure modes (with remediation guidance) or a no-match / empty-result contract worth clarifying (e.g. "returns an empty array, not an error"); omit it only for tools that cannot meaningfully fail. IncludeObsidian syntax:on write tools. Errors:gets one bullet per distinct remedy, and every bullet keeps its remedy. Messages that share a remedy share one bullet, which lists only the messages that tool can raise: for example"absolute path blocked","path traversal blocked", and"hidden path blocked".- Split parameter facts between
.describe()and the tool description. The calling model receives each parameter's Zod.describe()text inside the input schema, so the tool description earns its space (and its score from graders such as Glama's Tool Definition Quality Score, TDQS) only with what that text doesn't say:.describe()gives the parameter's plain meaning: what it is, its format, and its default.- The description gives what
.describe()can't: interactions between parameters, consequences, and non-obvious semantics. - When a semantic fact appears in both, keep it in the description
and cut
.describe()back to the plain meaning. Format and default stay in.describe(). Never drop a fact only.describe()carries.
- Tool input schemas stay at
.min(1)— no.refine. Rich validation (format, date validity, mutual exclusivity) lives in the data layer or handler, where failures flow throughsafeHandleras structured tool errors with remediation text and get logged astool_error. Zod schema failures surface as protocol-level invalid-params errors that bypass both, and a.refinepredicate can't be serialized into the JSON schema clients see anyway — so it adds no discoverability, only a second copy of a guard the data layer must enforce regardless (drift risk)..min(1)is the floor because it does serialize (minLength) and its default failure message is self-explanatory. - Edit only the tools a change is meant for. A grader re-scores every definition whose text changes, so apply a shared wording change tool by tool and leave every other tool's text as it is.
tool-surface-snapshot.test.tscaps the tool list's size, because clients such as claude.ai load every definition into each conversation. Each checked combo's total must stay withinCHARS_PER_TOOL_ALLOWANCEtimes its tool count.npm run report:tool-surface-sizeprints per-tool sizes; run it before and after editing a description.
- Registry entry — add the name to
TOOL_NAMES, then an entry toTOOL_REGISTRYintool-registry.ts: name, group, and annotations. The registry is a leaf module with zero imports. - Handler — add the tool in the appropriate
tools/*.tsgroup module. TheregisterToolwrapper auto-injects annotations from the registry and enforces the enabled-tool gate. Write the description and input schema to the rules in "MCP tool definitions" above. - Tests — unit tests in
tools/__tests__/, plus a happy-path case inserver-integration.test.ts(see "Integration tests — when to add"). Cover the handler's behavior, not just the schema. - Availability keying — if the tool's description names other
tools, use
whenToolEnabledTextso references disappear when their target is disabled. - Snapshot baseline — run
npm run snapshot:updateand commit the regenerated__snapshots__/tool-surface/files; any change to the tool surface fails the drift test until the baseline matches. The size cap intool-surface-snapshot.test.tsfails if the new tool is much larger than the per-tool average; trim the text, or raiseCHARS_PER_TOOL_ALLOWANCEand give the reason in the PR. - Feature-surface docs — see the "Files that track feature surface" list below for which files to update (README tools table, ARCHITECTURE.md, DOCKERHUB regen, etc.).
- Group module — create or extend a module in
prompts/. ExportPROMPT_NAMESand aregister*Promptfunction takingPromptRegistrationContext. - Registration — add the register call in
prompt-definitions.ts. If the prompt depends on a specific tool, gate it oncontext.isToolEnabled(TOOL_NAMES.*). - Tests — co-located at
prompts/__tests__/. Use the sharedprompt-test-harness.tsin that folder for registration capture. - Availability keying — use
whenToolEnabledText,isToolEnabled, andformatEnabledToolListfrom the context for any tool references in the prompt text or fallback paths. - Snapshot baseline — run
npm run snapshot:updateand commit the regenerated__snapshots__/tool-surface/files (prompts are part of the captured surface). - Feature-surface docs — update the README prompts table and regenerate DOCKERHUB.md.
Prompts (mcp-core/prompts/) are user-initiated workflows, distinct
from tools:
- Kebab-case names (
vault-orientation,memory-review), exported viaPROMPT_NAMES— mirroringTOOL_NAMES. - Short, picker-facing
title/description— they render in slash command / +-menu pickers, so noExample:/Returns:scaffolding; one or two sentences. - A prompt earns its place only through live content — assembled at
invocation time from the data layer — plus thin, durable instruction.
Never re-encode a procedure that can drift. Zero-arg prompts omit
argsSchemaso the SDK calls back as(extra) =>. - Handlers degrade, never throw — wrap data gathering so a failure returns a valid fallback message; a prompt must not hard-fail the client.
memory-reviewis append-only by design — it reads the memory layer as a dated evolution (never "newest supersedes older"), proposes only append updates, and never prunes "stale" entries. The one exception: a memory file whose frontmatter declaresentry-policy: livingis a current-state snapshot, and the review may propose pruning its expired entries (the policy is surfaced byvault_list_memory_files; absent means append-only).
Two naming layers — MCP (JSON wire format) and TypeScript (internal):
- MCP inputs and outputs use
snake_case— this is the JSON shape clients see. Examples:old_text,heading_level,snippet_tokens,additional_properties,sample_values,outgoing_links,exclude_folders,sort_by. - Internal TypeScript uses
camelCase— function params, local variables, internal types that never reach the wire. Examples:oldText,headingLevel,snippetTokens. - The mapping happens in each tool group module's handlers
(
mcp-core/tools/*.ts):replaceAllOccurrences: replace_all_occurrences. - Types that ARE the JSON response shape (
PropertyKeyInfo,SearchResult,NoteMetadata) usesnake_casefor any multi-word fields to match the wire format. Single-word fields (path,title,count) are the same in both conventions.
- Note-path tool inputs must end in
.md. Inputs naming a single markdown note —pathon read/write/patch/replace/delete/update_properties, the anchor tools (delete_span/replace_span/insert_at_anchor), and the task tools,new_pathon move — require the full filename with extension; a bareProjects/Planis rejected. Enforced by the genericassertPathHasExtension(path, ".md")util (src/utils/assert-path-has-extension.ts), called in the data-layer function each tool routes through (one rule, every layer). Folder, glob, and memory-file (file) inputs are exempt. Backlinks/outgoing_links accept.mdor.canvas— both note and canvas paths are valid graph queries (assertPathHasExtension(path, [".md", ".canvas"])).vault_move_note'sold_pathalso accepts.mdor.canvasat the handler boundary — the handler runs a backlinks lookup before the data-layer move, soold_pathis validated by the wider extension set; the data-layer.md-only check runs second. vault_read_fileis the deliberate inverse. Itspathnames any non-markdown file and must not end in.md—vaultFs.readAssetrejects notes so the.mdboundary stays a single rule with two sides (notes →vault_read_note, everything else →vault_read_file). Error messages never name tools; the routing guidance lives in each tool's description.
-
A test that denies file access with
chmodand expects the access to fail can't pass as root, because root ignores file permissions. Mark itit.skipIf(runsAsRoot), importingrunsAsRootfromsrc/__tests__/runs-as-root.ts. CI runs it as a normal user. To run these tests from a root shell, such as a cloud session, usenpm run test:as-nobody; it takes the same arguments asvitest run. -
Tests read as a behavioral spec. One focused
it()per behavior — a failing test name should identify which behavior regressed without reading the body. -
constper test overletinbeforeEach— this is the strong default, not a soft preference. When setup is cheap (in-memory DB, small fixtures), use a factory helper andconst index = createTestIndex()at the top of each test.beforeEachis only justified when per-test creation is genuinely impractical (expensive resources, complex multi-step setup that would obscure the test body). -
Every test must actually verify the behavior it claims to test. A folder-filter test must include data both inside and outside the folder to confirm exclusion works — not just data inside.
-
Tests must fail when the verified behavior breaks. If a test claims "body is unchanged", it must assert the full body — not a substring that would still match if the body were modified.
-
A test must pass only for the intended reason — not for an unintended or coincidental one. These are two separate bars, and both must hold: (1) the test fails when the behavior is wrong (above), and (2) it passes because the intended behavior occurred, not because of a no-op or a different code path that happens to leave the asserted state. Guard against the second:
- Silent no-op. A test asserting "the folder is retained / state X is preserved" passes even if the triggering operation never ran. Also assert the trigger happened — that the note was actually deleted / actually moved — so retention can't be satisfied by doing nothing.
- Wrong-error pass.
rejects.toThrow()with no argument matches any error, so a test meant to verify "rejected for reason A" can pass on an unrelated failure (e.g. a missing fixture throwing ENOENT). Assert the specific message, and set up the fixture so the intended rejection is the only one possible. - Early-return pass. A returned
0/empty/falsecan come from the guard you're testing or from the function bailing out before reaching it. Assert a side effect that only the intended path produces (e.g. the expectedwarnwas logged) so the happy-accident return can't pass. - Wrong-item pass. Seeding multiple items, querying one, then asserting only "something came back" — assert the specific expected item (path/id/content).
- Coincidental-equality hazard. When production and test read
the same source (e.g.
err.stack), both beingundefinedpasses trivially — set a predictable value and assert it exactly.
For any of these, when in doubt, mutate the code (break the specific behavior) and confirm the test fails for that reason — not a compile error or an unrelated assertion.
-
Exact assertions (
toHaveLength(2),toBe("value")) over loose matchers (toBeGreaterThanOrEqual(1),toBeDefined()) when the expected value is known. -
Prefer an exact assertion on the whole value (
toBeon the full output/object) overcontains/substring checks when the output is deterministic and you control the inputs —containssilently tolerates ordering changes, formatting drift, and accidental duplication. Reservecontainsfor when only a fragment is genuinely under test (one branch among many, or an excerpt of large output). -
Never decompose:
toHaveLength(1)+ index property checks is weaker thanexpect(results.map(result => result.path)).toEqual(["foo"])— the decomposed form misses extra items, ordering, and unexpected properties. -
Deterministic error messages get exact assertions — if the message is a hardcoded literal in production code, no
stringContaining. -
Explicit callback parameter names —
orphannoto,entrynote,linknotl. Same naming rules as production code. -
Test names match what they assert. If the test asserts 1 result, don't name it "returns multiple results."
-
Use vitest helpers (
onTestFinished,vi.mocked,vi.each) before hand-rolling test plumbing. -
No cleanup after assertions — trailing
rm/close()at the end of a test body is skipped when an assertion throws; register cleanup inafterEach/onTestFinishedat creation time. -
Every test file maps to a real source module and lives in the
__tests__/folder next to the module it tests — not in a centralized test directory higher up the tree. Don't spawn a standalone test file just to mock differently; usevi.mock(path, { spy: true })to keep the real implementation. Exception — shell scripts outside vitest's include paths. The remote image's s6 init scripts underrootfs/etc/s6-overlay/scripts/are thevault-mcpserver's boot chain for the:remotetarget, and the Claude Code hook sits in.claude/hooks/. Neither is a TypeScript module, and vitest's include paths (src/,cli/src/,scripts/) reach neither folder. The init-script tests live insrc/vault-mcp/__tests__/(init-check-auth.test.ts,init-obsidian-login.test.ts,init-first-sync.test.ts,init-setup-user.test.ts,init-setup-vault.test.ts,print-derived-env.test.ts, which covers the derivation thatinit-derive-envpublishes), and the hook's test insrc/__tests__/(install-deps.test.ts). These script tests run the real script under its own shell (shfor the init scripts,bashfor the hook) with stub binaries onPATH, and name the script they cover — don't move them next to the scripts or widen vitest's include for them. Whole-image behaviour (the init chain's ordering, thecontainer_environmentfiles the chain publishes, the volume layout, and the checks that stop the container) belongs in the remote-boot test suite (src/__tests__/docker/, see "Remote image boot tests — when to add"); the branches inside one script stay in that script's test file. -
Separate
it()blocks over callback-patternit.eachwhen assertions are structurally different —it.eachis for genuinely identical assertion shapes (input → expected). Use labeled case objects and$labelin the title when values alone do not name the scenario. -
Error paths and boundaries are covered, not just the happy path. Zero/one/empty inputs expose the special-case bugs.
-
Prefer a controllable seam over fake timers for retry/polling logic — an injected timeout/deadline param, or a named wrapper around the retrying operation mocked with controlled outcomes. Fake timers (
vi.useFakeTimers) are legitimate when timing is itself the contract (debounce, TTL, deadline expiry) or when a wrapper seam would exist only for tests — but assert outcomes after advancing the clock, never the tick-by-tick schedule. -
Production type rules apply in tests: no
!(guard or restructure instead) — but?.and?? fallbackare legitimate narrowing. -
Test-owned expected values for drift-catching tests — when a test verifies the content of a production constant (prompt text, config defaults, rendered output), define the expected value in the test file; importing the constant means both sides drift together and the test passes trivially on any change.
-
Assert mock interactions through the matcher API —
toHaveBeenCalledTimes(1)+toHaveBeenCalledWith(exactArgs)— never positional call-log readback (mock.calls[0][0]+toEqual). When an expected value needs derivation (resolved paths, URLs), derive it test-side with the same mechanism production uses — production must not be its own oracle. -
Production code never carries test-serving structure — no cache keying, extra branches, or widened semantics whose only job is isolating tests from shared state. Tests own their isolation (
vi.resetModules, factory-created instances per test); review findings proposing production changes for test-only scenarios get declined. -
Mocks must preserve the dimension under test. Give a stand-in varied order, timing, ranking, or size with decoys when the behavior depends on that dimension. If the stand-in cannot model that variation, test against the real component.
-
CI shell snippets are tested under
bash -ebefore committing — Actions runsrun:steps with errexit, so a failing[ test ] && cmdshort-circuit aborts the job; useif/then/fi.
Integration tests (src/__tests__/integration/) boot a real server
and call tools over real HTTP via the MCP SDK Client. They catch what
unit tests structurally can't — handler miscalls, config-gated surface
mismatches, transport-layer bugs. The deciding question: would this
bug survive unit tests but break in production? If the unit test
already uses real I/O (temp dirs, real SQLite), skip the integration
test.
Always add:
- New tool → happy-path test in
server-integration.test.ts+ fixture data if needed. - New error path in a tool's
Errors:section → test inserver-error-contracts.test.tsasserting the distinctive message prefix (include test-controlled variable parts like paths and section names). - New config gating axis → config matrix test in
server-integration.test.ts(tool count + key behavior), and add the axis toSURFACE_AXESinscripts/tool-surface-capture.tsso the snapshot combos cover it. For a setting that changes only one tool's text, add a single combo toSURFACE_COMBOSinstead. - New prompt → assembly test verifying live vault data, not just the instruction wrapper.
Never add:
- Parser changes (pure, no I/O).
- Search query changes (
search-index.test.tsuses a real SQLite DB). - Config validation (
config.test.tstestsloadConfigdirectly). - Tool registration/annotation (
tool-definitions.test.tscovers the registration layer via a mock server).
File structure:
server-integration.test.ts— happy paths + config gating (one server per config combo).server-error-contracts.test.ts— error paths, default config only (errors are config-independent).server-oauth-integration.test.ts— OAuth flows: token rotation, client sweep, reuse detection, scope widening.test-harness.ts— shared server lifecycle + client factory.fixtures/vault/— committed fixture vault; a PR that adds a tool also adds fixture data and a test case.
When to split a test file: when navigating the file requires scrolling past unrelated test groups to find what you need — the file has multiple distinct concerns that don't share setup or context. Split along concern boundaries (happy path vs error contracts, by tool group, by config combo), not arbitrary size.
CLI PTY tests (cli/src/__tests__/integration/) drive the real CLI
binary in a pseudo-terminal via node-pty. They catch what unit tests
(which use createScriptedPrompts DI) structurally can't — real
keystroke processing, terminal rendering, and entry-point wiring.
Run via npm run test:cli-pty (separate vitest config, excluded
from npm test).
Always add:
- New interactive command → happy-path test driving the full prompt sequence.
- New prompt in an existing command → extend or add a scenario.
- Docker start/health-check changes → test with the fake
dockerbinary's/healthzserver.
Never add:
- Output/message changes — unit tests pin via
createScriptedPrompts. - Flag parsing —
program.test.tscovers Commander. - Prompt branching logic — unit tests cover via scripted answers.
- Docker interaction — unit tests inject
DockerRunnerstubs.
Remote image boot tests (src/__tests__/docker/) boot the built
:remote image with the Sync CLI replaced by the fixtures/ob stub.
They catch what a single script's test file cannot: the ordering of
init scripts, the container_environment files the init chain
writes, the volume layout, and the guards that stop the container on bad
state (such as the deletion-storm guard). Run via npm run test:remote-boot
(builds the image, then runs a separate vitest config excluded from
npm test).
Tests in a describe block share one booted container. Two places boot
more often, because each scenario sets a different environment or
expects a different exit outcome: every guard scenario boots inside its
own it(), and the failing-sync block boots once per sub-describe
(memory on, memory off).
Always add:
- New init script or ordering change → extend the expected
obcall sequence. - New variable that
init-derive-envwrites → assert itscontainer_environmentfile. - New safety check that stops the container → a guard-scenario test asserting the ERROR line.
- New Sync failure mode → a new stub switch (like
OB_STUB_SYNC_FAIL=1).
Never add:
- Branch logic inside one script — that script's test file covers it.
- Server tool behaviour — the integration test suite covers it.
- Anything needing real Obsidian Sync — that stays a manual test deploy.
- Secrets via
sst.Secret, PascalCase names. Never hardcode. - Check the SST v4 docs before writing config you haven't used in this repo (linking, secrets, outputs) — don't infer from other IaC tools or older SST versions.
- Plain configuration a Lambda needs (
PUBLIC_URL) goes in the function'senvironment:, read withenv-var;link:is for SST resources a handler reads throughResource.*(theMcpAuthTokensecret). Type-generation timing is in "Build pipeline gotcha" below. $interpolateforOutput<string>composition.- Raw Pulumi
aws.*for Lightsail (no SST component exists). sst.aws.ApiGatewayV2+routeUrl()for HTTP proxy.- SST bundles Lambda handlers with esbuild from entry file.
Resource.McpAuthToken (used by src/functions/authorizer.ts) is
typed via sst-env.d.ts at the project root, which SST writes when
it runs the resource graph. The file is committed but auto-generated
— on a fresh clone it may be stale, and npm run build can fail with
Property 'McpAuthToken' does not exist on type 'Resource' until
you've run npm run deploy once for your stage.
npm run deploy requires ~/.config/vault-cortex/.env; create it from
.env.example as shown in DEPLOY.md one-time setup step 2.
If you add or rename a secret in sst.config.ts, re-run npm run deploy
to regenerate sst-env.d.ts.
sst.config.ts is typechecked by npm run build:sst (part of
npm run build) through its own tsconfig.sst.json. It cannot share
tsconfig.json: its globals come from .sst/platform/config.d.ts, which
pulls SST's platform source into the program, and that source does not
compile under the repo's stricter checks. .sst/platform exists only after
npx sst install — CI runs it before the build; run it once locally on a
fresh clone.
recurrence.ts transliterates the Tasks plugin's recurrence logic
(Recurrence.ts + Occurrence.ts) and pins rrule to the exact version
the plugin declares, so rule parsing and next-hit computation match the
plugin by construction. The pin is deliberately excluded from Dependabot.
On any rrule bump — or when re-verifying against a newer Tasks plugin
release — re-check each of these against the plugin source before merging:
- The rule grammar split (
/^([a-zA-Z0-9, !]+?)( when done)?$/i) and theparseTexttry/null handling. - Reference-date priority (due → scheduled → start; flipped to due →
start → scheduled under
removeScheduledDateOnRecurrence). - The month/year overflow walk-back: the rule's start date (rrule's
dtstart) moves with each step, and a rule whose text contains" on "(an explicit day, "every month on the 31st") skips the walk-back — month rules only. - The fields
createNextOccurrencesets on the next occurrence: block link, 🆔, and ⛔ cleared; created date replaced persetCreatedDate, never carried forward. - The
recurrence.test.tsvectors lifted from the plugin's ownRecurrence.test.ts— refresh them from the new release's tests.
Two deliberate divergences are pinned by tests, each contracted where it
lives: exhausted finite rules complete without spawning (recurrence.ts's
docstring), and calendar-invalid dates are nulled at parse time
(calendarValidOrNull in tasks.ts). A behavior change in either is a
decision, not a drift fix.
The Sync CLI's documentation
covers usage, not internals, so the :remote init chain depends on
behaviour observed in the pinned cli.js. Treat every bump
of obsidian-headless/package.json as a potential regression and
re-verify each contract against the new source before merging:
-
Verbs and flags the scripts call:
login,sync-config(including--modeand--json),sync,sync --continuous, andsync-setup --vault --device-name.sync-config --jsonprintsvaultIdandsyncMode(bidirectionalwhen unset), and an unknown--modevalue exits non-zero, whichinit-setup-vaulttreats as fatal. -
sync-setuprewrites the whole config on every boot without the filter fields orsyncMode.sync-config --file-types/--excluded-folders/--configsthen queue only the recorded server files that the new value allows and the reset defaults block, so some changes queue nothing. -
ob synccreates<vault>/.obsidian/and a.obsidian/.sync.lockdirectory before transferring anything.vault_has_contentininit-first-synctherefore treats an.obsidian/folder that holds nothing but.sync.lockas an empty vault. -
The device's file record is
obsidian-headless/sync/<vaultId>/state.dbunder$XDG_CONFIG_HOME, tablelocal_files. The engine loads this record at startup and compares it against the files on disk. The deletion-storm guard ininit-first-syncreads the same table: it stops the container when the record lists files but the vault is empty, because the engine would push each missing file as a deletion. -
Files delivered by
sync --continuousare recorded in that same table as they arrive, and a file deleted locally has its row removed at once. The stub'ssync-recordandsync-forgetverbs mirror the two. -
init-first-syncqueues every file the engine knows about but this device has no local record of (its header explains why). That step relies on five engine behaviours. The remote-boot oracle (fixtures/sync-engine-oracle.ts) runs the image's own engine on the first; re-check the other four by hand in the newcli.js:- The deletion scan pushes a deletion for a
server_filesrow with nolocal_filesrow, and only two-way sync runs it:pull-onlyandmirror-remotereturn first. The pending loop runs before the scan, and a one-shotsyncstops at the first failed download, before it. server_filesandpending_filesrows share one record shape (addPendingFilewritesuid, path, data), and the three tables keep the columns the script's schema check lists.sync --continuousbacks off a failed download and still runs the scan.- Skipping a superseded queued entry deletes every queued row for that
path (
DELETE FROM pending_files WHERE path = ?). - The oracle's minified names are unchanged: the closing
x.parse();call, andisas the engine class's name.
- The deletion scan pushes a deletion for a
-
The setup page's pre-flight (
src/vault-mcp/setup/) mirrors two callsob sync-setupmakes on the next boot:/vault/list(an end-to-end encrypted vault comes back withpassword: "") and/vault/accesswith the key hash — scrypt over the NFKC-normalized password and salt (N 32768, r 8, p 1, 32 bytes), then HKDF-SHA256 with infoObsidianKeyHashforencryption_version2 and 3, SHA-256 for 0. The vectors invault-key.test.tswere produced by the pinned CLI's own functions; recompute them on a bump.
The remote-boot scenarios never run the real CLI — they run the stub
(src/__tests__/docker/fixtures/ob), which imitates the behaviour listed
above. So if a new CLI version behaves differently, those tests still pass,
because the stub still behaves the old way. Only the sync-engine oracle runs
the image's real engine. After updating the pinned version:
- Update the stub to match the new behaviour, and the oracle's minified names if they changed.
- Run the remote-boot tests (the oracle among them) and the init-script tests.
- Boot the new image once against real Obsidian Sync and confirm the first sync, the guard's file count, and continuous sync in the logs.
The README is the front door — humans land there first. The full AWS/SST
deployment walkthrough lives in DEPLOY.md; the local and
Obsidian-Sync quickstarts live under deploy/. Keep this
file focused on conventions; don't duplicate procedure here.
Write for the Obsidian user, not the developer. User-facing docs (README, deploy guides, cli/README) describe what a feature does in terms the end user thinks in — "complete a task," "move between lanes," "filter by due date" — not implementation vocabulary ("mutate," "atomic write cycle," "composable mutations"). Internal docs (AGENTS.md, ARCHITECTURE.md, code comments) use precise engineering language. The test: would an Obsidian user with no programming background understand the sentence? If not, rewrite it.
Write-time format decision — before committing any prose (code comments, JSDoc, README sections, PR descriptions, env-file comments): information gets structured format (table for lookups, bullets for parallel items, numbered list for steps, one sentence for a single constraint); narrative goes in the PR description, not committed files. More than 3 sentences of prose → probably the wrong format. Sections match their siblings' length and shape.
Doc quality rules:
- Before calling a doc done, pick each reader the doc serves (the Obsidian user, a self-hoster deploying it, a contributor) and walk the entire document start to finish — line-level fact-checking cannot validate a doc; each sentence can be true while the doc as a whole misleads.
- When a doc offers multiple paths (install methods, tools, runtimes), every operational section (update, restart, verify, troubleshoot) serves every offered path — or explicitly scopes itself.
- Structural self-references ("shown below", "the section above") are claims about the document — they must resolve against the current document after any restructuring.
- Sibling docs (local/remote guides, per-OS variants) are authored as a set — same lead method and section skeleton, diverging only where the variant genuinely differs.
- Factual claims match the implementation — capability lists and data-flow descriptions are verified against the code; conditional capabilities are stated conditionally.
- Add an authoritative specification link when it helps a reader verify
a specific requirement or implement a behavior. Do not require a link
for every first mention, a bare protocol name, or a broad capability
summary. Use a Markdown link in docs and a raw URL in explanatory code
or env-file comments. Keep citation URLs out of MCP tool descriptions
and test names (
it/describetitles). - Pair a destructive-outcome claim with its actual recovery path at the same point in the doc.
- Mechanism language is earned — "caches", "batches", "switches automatically" only when the code implements that mechanism.
- Corrections leave no residue — fixing an over-claim states the current design directly, never a walk-back parenthetical explaining what "actually" handles it; in inventory-style sections, one mechanism per bullet.
- Before cutting copy that survived earlier reviews as redundant, check why it exists. Preserve a useful explanation or replace it with a plainer one that carries the same information.
- Concrete referents at the point of use — when a specific name exists (a UI toggle, a filename, a section title), state it where the reader is; a "see below" names its target. Vague referents force backwards sentences — lead with the reader's action as the conditional's subject (cause → effect).
- Opt-outs surface at the point of decision — a section describing opinionated behavior (auto-created files, background writes) states its disable switch inline in that section, not only in the config reference.
Contributor and release conventions live in
CONTRIBUTING.md. Flag a breaking change with a
BREAKING CHANGE: footer in the PR description, so the generated release
notes list it (other accepted flags are in CONTRIBUTING.md).
Pick a commit's conventional-commit type by what it changes in what this
repo ships: the server's Docker images and the vault-cortex npm CLI.
Claude Code hooks, dev scripts and other local tooling
are never feat or fix: use chore, or ci or build where they fit,
so they stay out of the changelog's Features and Bug Fixes. A squash merge
lands the PR title, so the title takes the type of the PR's main purpose.
No hardcoded tool or prompt counts in user-facing surfaces. Never
write a specific number of tools or prompts in README, server.json,
the social preview, .devin/wiki.json, the image descriptions (in the
Dockerfile, deploy.yml, and dockerhub-description.yml), or any other
surface an end user or registry sees. Use category names or capability
descriptions instead. Counts go stale on every tool addition and the drift compounds
across surfaces. The README's Tools and Prompts tables are the source of
truth; a reader counts from those. Internal docs (AGENTS.md, code
comments) may include counts where they help agents gauge
module size — these are agent-facing and not propagated externally.
Several files outside src/ reflect the project's feature surface and
need updating alongside code changes. What to check depends on what
changed:
README.md: New deployment mode, new feature worth mentioning in the value propARCHITECTURE.md: New component, requirement, or design decision; component diagram changes. Write for scannability: bullet lists and numbered pipelines over dense prose — a reader landing on this page should grasp the flow at a glance, not parse nested parentheticals.ROADMAP.md: Direction genuinely changes — a priority shifts between Planned and Exploring, or a non-goal is decided. Refreshed in periodic passes (roughly once or twice a year) that remove completed Planned entries; not updated per feature PR, and shipped items are never promoted into Delivered — that section is a static foundations sketch, not a changelogserver.json: Description changes.descriptionhas a 100-character limit per the MCP registry schema — counted in code points, not bytes, so em dashes are safe here (CI guards it).Dockerfile: OCIimage.descriptionlabel — keep in sync withserver.jsonanddeploy.ymldescriptionsassets/social-preview.svg+.png: Feature category changes (rendered in the image); regenerate PNG after SVG edits (runnpm run render:social-preview).devin/wiki.json: New architectural area (new page), module renamed/moved (updaterepo_notesorpurposereferences). Purposes stay structural — what the page covers and which modules — never capability narratives, counts, or tuning values; those live in README/ARCHITECTURE and DeepWiki derives them at index time.src/vault-mcp/mcp-core/__tests__/__snapshots__/tool-surface/: Any change to tool schemas, descriptions, annotations, prompts, or server instructions — runnpm run snapshot:updateand commit the regenerated baseline in the same PR (the drift test fails until it matches). A dependency bump that changes the MCP SDK's schema serialization needs the same regen commit: that CI failure is the gate working, not flake.deploy/local/+deploy/remote/: New env var, changed default, new deployment step, or Docker Compose service change — update.env.exampleandREADME.mdin the affected directoryrender.yaml+deploy/render/+deploy/railway/: A variable the image needs at boot is added or renamed, a shipped default changes (plan, disk size, hop count, health path, port), or the image tag changes.templates.test.tspinsrender.yaml; the Railway template is re-published by hand from the definition table inCONTRIBUTING.md— existing deployments keep their settings until their owners redeploy.env.example(root): New env var or changed default for the Lightsail reference deploymentcli/README.md: Feature description or search capability changes — this is the npmjs.com landing pagecli/src/env.ts: Auto-synced optional blocks fromdeploy/*/.env.examplevianpm run sync:cli-env-blocks— run the script after editing deploy/ env filesCONTRIBUTING.md: CI pipeline, repo settings, or release conventions changeDEPLOY.md: Infrastructure, env vars, or deployment procedure changesGOVERNANCE.md: Access to a sensitive resource changes — a registry, bot, or workflow credential is added or dropped, or a collaborator gains rights. The Access list and Continuity paragraph name concrete credentials, so they must track the workflows' secretsSECURITY.md: The attack surface or its protections change — a new endpoint, auth layer, guard, scanner, or credential kind. The scope, hardening, setup-mode, and Secrets Management sections state what the code and CI actually do, and a stale claim here misleads vulnerability reportersDOCKERHUB.md: Auto-generated — regenerate vianpm run generate:dockerhub-readmewhen README.md changes tool/prompt tables, feature descriptions, env var table, or deployment options. Do not edit manually..github/workflows/dockerhub-description.yml: Description changes. Reads fromDOCKERHUB.md. Docker Hub limits short descriptions to 100 UTF-8 bytes, not characters — an em dash costs 3 (CI guards the byte length).lhm.plugin.json: Generated and gitignored — never edit or commit it.npm run publish:lobehubregenerates it from the live tool/prompt registry and publishes the LobeHub listing; that command is the only thing that needs running when tools, prompts, theserver.jsondescription, or thepackage.jsonkeywords change (keywords become the listing's tags).
Env var update checklist — when adding, removing, or changing an
env var that the server reads (defined in config.ts, server.ts,
setup/setup-server.ts, or logger.ts), update every downstream surface.
Not every var goes in every file — container-internal vars (HOST, INDEX_DB_PATH) are hardcoded
in compose and skip .env.example; remote-only vars (OBSIDIAN_AUTH_TOKEN,
PUID, etc.) only go in the remote surfaces. Use existing entries as a
pattern:
- Server source (
config.ts,server.ts,setup/setup-server.ts, orlogger.ts) — authoritative definition via theenv-varpackage (logger.tsreadsLOG_*fromnode:processenvdirectly) - Deploy compose (
deploy/local/docker-compose.ymland/ordeploy/remote/docker-compose.yml) — add${VAR:-default}passthrough inenvironment: - Deploy .env.example (
deploy/local/.env.exampleand/ordeploy/remote/.env.example) — document for users with comment + default. These are the source of truth for optional var documentation. - Run
npm run sync:cli-env-blocks— syncs the optional sections from step 3 intocli/src/env.ts(LOCAL_OPTIONAL_BLOCK/REMOTE_OPTIONAL_BLOCK). Always run after editing deploy/ env files. - Root .env.example — Lightsail reference deployment (if applicable)
- Root compose files (
docker-compose.yml,docker-compose.local.yml) — maintainer/contributor surfaces (if applicable) - Deploy workflows (
.github/workflows/deploy.yml,.github/workflows/test_deploy.yml) — write the Lightsail.envfrom repo Variables. A var the workflows never write can never reach the instance. Required vars go in the always-written block; optional vars go in the conditional block (unset Variable = no line written). - Hosted templates (
render.yamland the Railway definition table inCONTRIBUTING.md) — only for a var the image needs at boot or that has no derivation; optional vars are documented in both guides' Configuration sections by reference to the remote guide's table. Re-publish the Railway template after editing its definition.
CI drift tests in cli/src/__tests__/templates.test.ts catch omissions across
steps 2–4 after the fact and pin the committed hosted templates (step 8);
following the checklist avoids the omissions in the first place.
Not every PR touches these. A new tool in an existing category updates
the README tools table, DOCKERHUB.md (regenerated), and the tool-surface
snapshot. Update the
server.json description only when the category description changes. A
module rename updates .devin/wiki.json and ARCHITECTURE.md. Use the
list as a checklist, not a mandate to touch every file.