Synthesizes #133's intent (machine-usable default for a machine protocol)
with the project instinct (concise formatted tool results, but unaltered
content for editing). Empirically grounded: default vault.read is lossy
(fragments + flattens newlines → broken read→edit round-trip), the raw
envelope double-encodes the body (~2x), and returnFullFile-without-raw
crashes the formatter.
Decision (two-bucket contract): content reads faithful/verbatim by default
(fragmentation = explicit opt-in via existing strategy/maxFragments);
action/result tools stay concise-formatted by default; stop double-encoding
the body; verbatim path must not route through the broken formatter.
#133's global-raw-flip remedy explicitly declined; its intent satisfied by
bucket 1. Status: Proposed (design of record; implementation tracked
separately, not in this ADR).
splitContent/sortFiles don't touch RouterContext — the uniform ctx-first
param was a mechanical-extraction artifact. Removed from both defs and
their two call sites. Indentation nit deliberately NOT addressed: the
retained class-method indentation is what makes the extraction a
byte-verifiable behaviour-preserving move (per the review's normalized
diff); reflowing would destroy that property for zero functional gain.
tsc clean, lint baseline, 235/235.
router.ts was a 2,228-line monolith (~2.8x the 800-line threshold). Extract
the largest unit — executeVaultOperation + its helpers — into
src/semantic/operations/vault.ts behind a RouterContext interface.
- operations/router-context.ts: RouterContext (api/app/fragmentRetriever/
validator); SemanticRouter implements it and passes itself, so shared
state propagates with no indirection
- operations/shared.ts: Params, SearchResultItem, paramStr/Num/Bool
(router-private → shared by all handlers)
- operations/vault.ts: executeVaultOperation + splitContent/sortFiles/
copyFile/copyDirectoryRecursive; mechanical this.→ctx. (tsc is the
safety net — a missed rewrite fails compile)
- router fields api/app/fragmentRetriever/validator → public readonly
- removed a fully unreachable pre-existing dead subtree (combineSearchResults,
isDirectory, performFileBasedSearch, indexVaultFiles, getSearchWorkflowHints,
extractContext, getFileType — zero call sites anywhere; eslint doesn't flag
unused class methods, which hid them). ~290 dead lines gone.
router.ts 2228→988; vault.ts 948. Behaviour-preserving: tsc clean, lint at
the exact pre-existing baseline (0 errors, 5 unrelated warnings), 235/235
tests unchanged.
Staged per the issue's "extract vault first in isolation" guidance: #199
stays open for the remaining handlers (edit/view/graph/system/bases →
router thin dispatcher <300 lines). ADR-202.
Parallel edit.window/append/patch/at_line/from_buffer calls against the
same file (the recommended batched-tool pattern for MCP clients) ran
overlapping read-modify-write cycles: every call returned "Edit
successful" but only one edit survived, with no error surfaced — silent
data loss.
A new SemanticRouter is constructed per request, so a per-instance lock
cannot serialize concurrent requests. Add a process-wide FileLockManager
singleton (same pattern as ContentBufferManager) that serializes
operations on the *same* file path in arrival order while leaving
different paths fully concurrent. executeEditOperation runs its whole
action switch inside withLock(params.path, …).
- src/utils/file-lock.ts: promise-chain-per-path lock; chains drain so
the map stays bounded; a rejected holder doesn't break later waiters.
- router.ts: wrap executeEditOperation in the per-file lock; also move
the window-edit dynamic import into the two cases that use it (window,
from_buffer) instead of eagerly on every edit action — append/patch/
at_line no longer load it, and the path is unit-testable.
- tests/file-lock-edit-serialization.test.ts: FileLockManager unit
behaviour + a router-level #139 repro (3 concurrent appends; without
the lock the read-modify-write MockAPI loses 2 of 3 — with it all
land) + a different-files-stay-concurrent assertion.
Scope: edit.* actions (the #139 report). No router extraction (that is
#199's own PR). make check green (build + lint 0 errors + 231/231).
Stale/evicted MCP sessions were unrecoverable without a client restart
(#128): a non-initialize request bearing an evicted Mcp-Session-Id hit a
server-side synthetic `initialize` that cannot drive SDK 1.29's
web-standard transport to an initialized state (proven in #190 — non-stream
compatReq, hono RequestError→400, post-hoc accept header lost→406), "failed
open", and returned a non-spec 400 -32000 "Server not initialized" that no
client treats as session-expiry → infinite 400 loop.
Emit the Streamable HTTP spec's session-lifecycle signal and let the client
re-initialize itself:
- evicted session + non-initialize → HTTP 404 + Mcp-Session-Id (spec §3);
compliant client/bridge re-inits per §4, next request opens a fresh
session — no restart (fixes#128)
- no session id + non-initialize → HTTP 400 (spec §2)
- initialize (fresh or recreate) → unchanged, never short-circuited
Delete the synthetic compat-initialize block, requireInitializeNotice
machinery, and the createNullRes/NullResponse shim. New
sendSessionTerminated() helper centralises the spec response.
ADR-106 records the decision, the restated acceptance (404-signal +
2xx-reinit pair), and the known client caveat (spec §4 is the client's
obligation; mcp-remote reaches it via connectToRemoteServer recursion).
Regression harness tests/mcp-session-reinit.test.ts drives the request
pair through handleMCPRequest deterministically (paths return before any
SDK transport — the thing #190 showed cannot be driven synthetically).
ADR numbered 106 (not 105) to avoid collision with the #197 branch's
ADR-105; contiguous once both merge. make check green (build + lint 0
errors + 229/229 tests).
- eslint.config.mts: remove now-deleted build-worker.js from ignores (review nit)
- ADR-105: correct overreaching claim — the ConnectionPool pipeline itself
was also dormant (submitRequest never called; live path is MCPServerPool →
SDK setRequestHandler → tool.handler). Scope explicitly limited to the
worker-offload path; unused pool scaffolding left to a separate decision.
The worker-thread pool ratified/extended by ADR-104 has never executed a
single request since introduction (v0.5.8b, 22742bd): ConnectionPool's
action-level workerOps gate (tool.vault.search, …) never matched the
operation-level method strings the pool emits (tool.vault, tool.edit, …),
so processWithWorker was unreachable. Independently, the worker built to
dist/workers/workers/semantic-worker.js while WorkerManager loaded
dist/workers/semantic-worker.js — load would have failed anyway.
Verified dead against current main; no CPU-starvation field reports.
Fixing forward was rejected: it would ship an untested concurrency path
that widens the #139 parallel-edit race via a new TOCTOU window for an
unproven perf need (see #197 analysis).
- delete src/workers/semantic-worker.ts, src/utils/worker-manager.ts,
build-worker.js; drop build:worker + the build-worker.js build step
- strip worker wiring from ConnectionPool (retained as a main-thread
bounded queue — the only behaviour that ever ran) and the dead
prepareWorkerContext/workerScript from mcp-server.ts
- ADR-105 records the removal as a partial reversal of ADR-104; ADR-104
stays Accepted (SSE-route deconfliction half, shipped #196, still
governs) with a banner pointing to ADR-105
Behaviour-preserving by construction: the deleted path never ran.
make check green (build + lint 0 errors + 225/225 tests).
#192 replaced the Claude Code copy/paste command with a hand-edit-this-JSON
blob plus a warning against `claude mcp add --header`. That was a real UX
regression, and the security premise was wrong for THIS transport:
- The argv / macOS-unified-log exposure applies to **stdio** transports
(a spawned `npx mcp-remote --header ...` child), NOT native **HTTP**
transport — there is no spawned child carrying the header in argv. #143's
threat model was conflated with the mcp-remote setup it was also removing.
- Cleartext-at-rest in ~/.claude.json / .mcp.json is identical whether the
entry is added via the CLI or hand-edited, so "edit the file instead"
bought no at-rest improvement either.
(Confirmed against current Claude Code docs via claude-code-guide.)
Restores the single `claude mcp add --transport http ...` command as the
Claude Code path in the Settings UI, README, and the issue-32 template.
Removes the inaccurate warning and its now-dead `.mcp-security-warning` CSS
and the eslint-disable it required.
Kept from #192 (those parts were legit): native HTTP transport over the
deprecated mcp-remote, and the self-signed-cert / NODE_EXTRA_CA_CERTS
trust docs.
ADR-104's context has a passing line calling the `--header` form
deprecated; left as-is (accepted ADRs are immutable records of what we
believed then) — this commit is the corrective record.
Reimplemented from PR #126 (djsplice, who also reported #125), reduced to
the parts that are verified-clean. The worker-offload half of #126 is split
out to a tracking issue (inert as wired + correctness divergences — see PR
discussion / the follow-up issue).
SSE route deconfliction
In @modelcontextprotocol/sdk@1.29.0 (the pinned version), `GET /mcp` is
the standalone server->client SSE stream for server-initiated messages —
verified in webStandardStreamableHttp.js:handleGetRequest (opens
`_GET_stream`, Content-Type text/event-stream, gated on
Accept: text/event-stream + session + protocol version). A debug route
returning application/json on `GET /mcp` shadowed it, so any client that
opens the standalone stream (mcp-remote-class bridges — #128's logs show
one in use here) received JSON instead of an SSE stream and treated it as
failed. Moved debug to `GET /mcp-info`; `GET /mcp` and `POST /mcp` both
reach handleMCPRequest. Unlike #126's `app.all('/mcp')`, GET/POST are
registered individually so the existing explicit `app.delete('/mcp')`
session-close handler is preserved.
Scope note: this restores the server-initiated-notification channel (a
real latent defect on its own). It is NOT claimed to fix#128 — per the
#190 investigation that is a separate SDK-1.29 compat-init problem — and
#125's "SSE reconnection loop" is the reporter's un-reproduced diagnosis.
Two-row Levenshtein
fuzzy-match.ts rewritten from a full (m+1)x(n+1) matrix to a two-row
formulation (O(n) memory, no per-line array-of-arrays allocation) plus
length heuristics and a 0.95 early-exit. Pure CPU/memory win on the
main-thread edit.window path; behaviour-preserving. Adds the first
fuzzy-match tests (classic Levenshtein distances, symmetry,
heuristic-preservation).
build.yml: PR-status-comment step set continue-on-error so it cannot red an
otherwise-green build on fork/limited-token PRs.
Co-authored-by: djsplice <barrows.jeff@gmail.com>
Code review (PR #193) caught that the inline branch returned
`sourceFiles: paths` (caller input order) while `content` is built from
the local sourceFiles array *after* sortFiles() mutates it in place. With
sortBy set — exactly the sorted multi-file retrieval case this feature
targets — a consumer mapping content sections back to files got the wrong
order. Return the post-sort order instead.
Adds a router-level regression test (the gap the reviewer flagged: the
formatter-only tests could not have caught this).
Co-authored-by: Earl Plak <5597016+laplaque@users.noreply.github.com>
`vault combine` without a `destination` now returns the combined content
inline in the response instead of erroring. Read-only consumers can use
combine for multi-file retrieval with no side effects.
- router.ts: no-destination path returns { inline, content, ... }; the
destination-exists guard is skipped when there is no destination.
- formatters/vault.ts: FileCombineResponse gains optional destination +
inline/content; formatFileCombine renders an inline block.
- semantic-tools.ts: combine counts as a write-op (blocked in read-only
mode) only when a destination is given; param description updated.
- tests: formatter inline/destination/fallback coverage.
Reimplemented from PR #146 against current `main` (code only); supersedes
and closes out #146. Dropped the original PR's
`path: destination || '_inline_'` validator workaround — the batch.combine
validators (BatchLimitValidator, PathArrayValidator) never inspect `path`,
so the hack was unnecessary. The README/troubleshooting cert-trust docs
that #146 also carried are covered by the #143 reimplementation instead, to
avoid duplicate divergent copies.
Co-authored-by: Earl Plak <5597016+laplaque@users.noreply.github.com>
`claude mcp add --header "Authorization: Bearer <token>"` defeats
secret-at-rest protection: the CLI resolves and echoes the header value to
stdout (captured by any parent process, incl. AI agents) and on macOS the
spawned MCP child argv is written to the unified log. Editing the MCP config
file directly avoids both vectors.
- Settings UI: new shared `renderClaudeCodeConnection()` renders a
ready-to-paste JSON config + a security warning for the authenticated
path, and the safe plain CLI command only when auth is disabled. Used by
both the initial render and the live-refresh handler so they cannot drift
(the previous two copies had already diverged).
- README / SECURITY.md / troubleshooting.md / issue-32 response template:
drop `mcp-remote` + `NODE_TLS_REJECT_UNAUTHORIZED=0`, document native HTTP
transport and proper self-signed-cert trust (macOS Keychain +
NODE_EXTRA_CA_CERTS for Bun-based runtimes), add the `--header` warning.
- styles.css: `.mcp-security-warning`.
Reimplemented from PR #143 against current `main` (the settings UI had been
restructured since the PR was opened); supersedes and closes out #143.
ADR-100 left untouched — accepted ADRs are immutable records; the `--header`
deprecation is recorded in ADR-104's context instead.
Co-authored-by: Earl Plak <5597016+laplaque@users.noreply.github.com>
Ratifies the existing (undocumented, v0.5.8b) worker-thread pool as the
sanctioned execution model for CPU-bound semantic operations, and records
the decision to extend it to edit.window fuzzy matching plus deconflict the
GET /mcp SSE route. This ADR is the architectural gate for PR #126 (#125).
Status: Draft — pending decider review.
Refs: #125, #126
Advisor follow-up. Asymmetry by design:
- scorecard-gate.mjs TOLERATES a STALE portal (comparing an old scan
is harmless) but now prints a non-gating note so a pass/fail is read
in context.
- scorecard-baseline.mjs REFUSES on STALE (exit 4) the way it refuses
on drift — anchoring the baseline to an outdated scan would freeze a
stale truth and mis-calibrate every future gate run.
Validated: gate exit 0 + STALE note when manifest>portal; baseline
exit 4 (refused). Refs #165
Standalone weekly watch (+ workflow_dispatch) that FAILS only on a real
portal regression vs scripts/scorecard-baseline.json:
- Health/Review score-ratio downgrade
- automated issue count increase
- a new behaviour/permission ("**Title**:") finding
Never inspects advisory wording — structured deltas only. Scraper drift
→ exit 3, reported distinctly (fix scorecard.mjs, not a regression).
Portal unreachable → exit 0 (inconclusive, never a false fail). Gates
nothing in the release path; no PR/release triggers; no dependency
install (Node builtins only).
`make scorecard-gate` runs it; `make scorecard-baseline` re-snapshots
deliberately (refuses on drift) — a named act, since an accidental
re-baseline silently disarms the gate. Baseline captured for 0.11.25
(Health Excellent 4/4, Review Satisfactory 3/4, 5 issues, 12 findings)
— still shows Dynamic Code Execution; it clears (an improvement, not a
fail) when a post-ADR-201 release is scanned.
Validated: happy path exit 0, simulated regression exit 1 (all 3
classes), drift refusal, idempotent re-baseline.
Closes#165
Records the decision to keep the per-instance, non-synced, auto-generated
self-signed cert/key and NOT move the private key into the synced vault,
and to dissolve the recurring "store the key in the vault?" question by
documenting the HTTPS scope (opt-in, loopback-only, asserts no identity;
zero-config already delivered by the HTTP-on-localhost default).
Rationale: localhost self-signed cert is low-sensitivity; the API key
(#135) is the real secret and already syncs; vault-shared key is a
defense-in-depth regression for shared/team vaults for marginal gain.
ADR only — no code change; certificate-manager.ts behaviour unchanged.
Independent of the accepted "Direct Filesystem Access" finding.
Refs #7
The 2026 portal redesign replaced the old DOM/RSC structure with a card
UI: Health/Review render as a coloured grade word + a segmented bar
meter, and findings are RSC JSX tuples. The old parser drifted (SCRAPER
DRIFT, null anchors).
- decodeDoc/toText split: keep tags for the structural bar meter, strip
for prose fields.
- gradeAndScore(): grade = coloured span after the label; numeric score =
filled/total bar-meter segments (#183 — the portal's visible trust
signal), window bounded to the next label so Health can't bleed into
Review.
- Grade vocab no longer enum-pinned (the hard-coded Excellent|Good|… list
is exactly what drifted) — capture whatever word renders; drift guard
catches a null.
- Findings re-extracted from ["$","div|details","<text>",{...}] tuples in
the scorecard region; SIGNATURE re-anchored to the redesign wording
(**Title**: + neutral scan/attestation sentences).
- Drift guard: numeric scores are now first-class anchors; findingsDrift
= non-zero issue count with zero findings. Exit 2 on drift preserved.
Validated live: Health Excellent (4/4), Review Satisfactory (3/4), 5
issues, 12 findings, freshness current, integrity ok. Correctly surfaces
the residual "Dynamic Code Execution" finding on 0.11.25 (clears when a
post-ADR-201 release is scanned — the CLAUDE.md caveat).
Refs #183
Advisor review follow-up on #186:
- Add SECURITY cases note[("cons"+"tructor")] / file[("__pro"+"to__")]:
runtime-computed member names our static pre-walk intentionally does NOT
catch (property is BinaryExpression, not Identifier/Literal) — they must
fail closed via expression-eval's own access guard. Test-grounds the
defense-in-depth claim instead of asserting it. 60/60 green.
- CLAUDE.md 'Known accepted review findings': the Bases new Function is
REMOVED (ADR-201, #180/#185/#186), not pending #175. Documents the
honest caveat — grep 'new Function' main.js != 0 by design: residual is
transitive ajv@6.14.0 validator codegen + a deprecation shim, a
pre-existing library-internal class not reachable from vault content,
present on the reviewed 0.11.25, non-gating. Scanner finding was
attributed to the Bases path; this closes that path.
Refs #180, ADR-201
PR2 of 2 for #180 (ADR-201). Closes the arbitrary-code-execution vector
reachable through a synced/shared `.base` file.
- Drop `new Function('context', 'with(context){return ${expr}}')` (and its
no-implied-eval eslint-disable) from expression-evaluator.ts.
- Parse with expression-eval@5.0.1 (jsep grammar: no eval/Function/new, no
global scope; member denylist built in). Pinned exact; jsep 0.3.5 is its
audited transitive parser. Both >7d old; npm audit 0 vulns.
- Defense-in-depth AST pre-walk (assertNoForbiddenAccess): rejects
constructor/__proto__/prototype member access (Identifier AND computed
["constructor"] forms) and ThisExpression, independent of the library's
internal list — ADR-201's explicit, tested no-globals property lives in
our code, not a transitive dep.
- createEvalContext and all helpers untouched → behavioural parity.
Differential corpus (tests/bases-expression-evaluator.test.ts, from PR1):
57 EVAL_CASES still green = expression-eval == old new Function for every
realistic Bases construct; the formerly-executing escapes (2/42/99 under
new Function) now fail closed; SECURITY set never reaches a function, the
real global, or a value.
No behavioural change for legitimate `.base` expressions. Breaking only by
design: the RCE vectors, `this`, and prototype-chain access are now
refused. Clears the Obsidian "Dynamic Code Execution" finding.
209/209 tests; build + lint clean. Refs #180, ADR-201
PR1 of 2 for #180 (ADR-201). Lands the regression net independently of
library choice, against the *current* `new Function` evaluator as the
behavioural oracle.
- Expand tests/fixtures/base-corpus.ts: EVAL_CASES (~57 expr/expected
pairs spanning every operator class + every fn/property the evaluator
exposes), makeNoteContext() canonical fixed context, SECURITY_EXPRESSIONS
(sandbox-escape set). FILTER_EXPRESSIONS kept as a derived export.
- tests/bases-expression-evaluator.test.ts: locks current behaviour over
the corpus AND proves the live RCE — constructor.constructor reaches the
Function constructor through the `with` scope chain (returns 2/42/99
today). PR2 inverts the SECURITY block.
Time-dependent primitives (now/today) appear only inside relations stable
for all wall-clock times, so expectations never rot.
209/209 tests pass; build + lint clean.
Refs #180, ADR-201
Code review found the date-safety doc cited the wrong line (date() global,
not the real reconciliation). Correct it: (1) Obsidian metadata cache is the
primary frontmatter source, this parser is only a fallback; (2) when the
fallback runs, expression-evaluator auto-coerces date-like keys via
new Date(); (3) .base docs have no date scalars.
Surface the masked divergences instead of hiding them behind toBeFalsy:
- empty doc: yaml→null vs js-yaml→undefined — asserted concretely, proven
neutralized by parseFrontmatter's typeof/!==null guard.
- merge keys: js-yaml resolves <<:*anchor, yaml keeps literal — asserted as
a known, accepted boundary (.base/frontmatter never use merge keys).
- input anchors/aliases: confirmed both resolve structurally.
Documented all in yaml-bridge.ts. 149/149 green.
Closes#178. updateStatusBar() previously remove()'d and re-addStatusBarItem()'d
on every call. It fires multiple times during async startup (port-conflict
paths, server-start callbacks, settings); concurrent calls could each add an
element while only the last was tracked, orphaning a transient 'Mcp: error'
element that persisted until the next Obsidian reload.
Create the element exactly once and mutate it thereafter (setText + class
swap). showConnectionStatus=false now hides the single element via the
existing mcp-hidden utility instead of leaving/removing DOM. Structurally
eliminates duplicate/orphaned indicators regardless of call ordering.
Closes#174. Obsidian source-code review flagged js-yaml (unmaintained) for
replacement. Rewire bases-api.ts's 5 call sites onto yaml-bridge; drop
js-yaml + @types/js-yaml. yaml pinned to 2.8.4 (published 2026-05-02, clears
the project's 7-day supply-chain hold; 2.9.0 was 5 days old). Existing
overrides.yaml reconciled to $yaml so the direct dep governs transitive
resolution. Behaviour-equivalence proven by the differential corpus.
