Two findings from testing 0.11.37 live against Dataview 0.5.68 in the app —
neither reachable through the pre-existing mock fixtures, both real:
1. Implicit `LIST ... GROUP BY` returned bare group keys, no rows. Dataview's
programmatic query() API drops grouped rows when nothing in the query
references `rows` (unlike rendered markdown), so `LIST FROM "x" GROUP BY
file.folder` came back as just the folder names. normalizeListGroupByQuery
injects a default `rows.file.link` output expression for exactly that shape
— a LIST with GROUP BY and no explicit output expression — so proper
{key, rows} groups come back. Queries that already name an output
expression, and non-LIST / non-grouped queries, pass through untouched; the
user-facing echoed query stays the original input.
2. `TASK ... GROUP BY status` groups incomplete tasks under a space-character
key, which rendered as an empty bold header (`**** (n)`). groupKeyLabel now
treats whitespace-only keys the same as empty → `(no group)`.
The test mock is made faithful to the live behavior: a grouped LIST that
references `rows` returns list-pair wrappers, one that doesn't collapses to bare
keys — so the injection is actually observable rather than passing vacuously.
Adds normalization unit tests, an end-to-end implicit-grouped-LIST recovery
test, and a whitespace-key label test. build + lint + test green, 355/355.
Incorporates the open dependabot dev-dependency PRs as one coherent, tightly
verified upgrade rather than five separate rebase/CI cycles, since they interact
across the build/lint toolchain:
- eslint 9.39.2 → 10.5.0 and @eslint/js 9.39.2 → 10.0.1 (#235 + #236 — these are
coupled: @eslint/js@10 peer-requires eslint@10, so #236 could only ever resolve
alongside #235; neither lands alone)
- typescript 5.9.3 → 6.0.3 (#237)
- typescript-eslint / @typescript-eslint/utils 8.60.1 → 8.62.0, esbuild
0.28.0 → 0.28.1, globals 17.6.0 → 17.7.0, obsidian 1.13.0 → 1.13.1 (#246,
which also subsumes the standalone esbuild bump in #233)
- actions/checkout v6 → v7 across all workflows (#240)
TypeScript 6 turns two tsconfig deprecations into errors. Fixed by modernizing
the config, not suppressing — Obsidian's community-plugin source review forbids
`ignoreDeprecations`-style escape hatches and rewards tight configs:
- dropped `baseUrl: "."` (TS5101) — unused; every local import is relative
- `moduleResolution: "node"` → `"bundler"` (TS5107) — the correct modern value
for an esbuild-bundled project with extensionless imports (node16/nodenext
would force `.js` extensions and break every relative import)
Also ran `npm audit fix` (security fixes are hold-exempt) clearing two dev/test
transitive advisories (@babel/core file-read, js-yaml merge-key DoS) that were
pre-existing on main via the jest/istanbul chain — `npm audit` now reports 0.
Verified tight: `npm run build && npm run lint && npm test` all green, 352/352
tests pass, no rule suppressions added.
Maintainer review-fix commit on top of the contributor's work in PR #249,
addressing three low-severity robustness findings from a security/quality
review of the formatter changes:
- F1: the group branches switched all-or-nothing on `groups.every(...)`, so a
single unrecognized group in a GROUP BY result dropped every group back to
the flat renderer (re-surfacing the #220 raw-wrapper symptom). Render grouped
when *any* element is a group and emit a stray non-group element as a plain
row, so one bad group can't collapse the whole result.
- F2: `asGroup` (formatter) accepted only plain-array inner collections while
the producer's `unwrapGroup` also accepted `.array()`-able DataArrays. They
reconcile today only because the producer flattens first; align `asGroup`'s
predicate and flatten so an unflattened wrapper reaching the formatter still
renders instead of leaking.
- F5: guard `toISO()`'s return with a `typeof === 'string'` check so a
non-string return can't propagate into `truncate`.
Happy-path GROUP BY behavior unchanged; all 352 tests pass. The mixed-group
and unflattened-group paths are defensive (the current producer can't emit
those shapes), so they are hardening rather than newly-reachable behavior.
Per the maintainer's #220 comment: TABLE cells that are Dataview Link objects rendered as raw JSON ({"path":...}) and Luxon DateTime values as quoted ISO strings. Adds a coerceCell helper (Link -> display/path, DateTime -> toISO(), native Date -> toISOString(), primitives as-is) and routes formatDataviewTable cells through it. Adds a TABLE regression test with a Link cell and a DateTime cell (DataArray-wrapped row).
GROUP BY nests rows under a group wrapper — {key, rows}, or a list-pair {$widget:'dataview:list-pair', key, value} for LIST. The producer's list branch passed wrappers through (the formatter then rendered them raw) and the task branch mangled each group into an empty task, so grouped rows never showed.
formatQueryResult now unwraps groups (flattening inner DataArrays to plain arrays) in the list and task branches, and the list/task formatters render each group's key and rows. Adds DataArray-wrapped GROUP BY fixtures + tests through executeQuery -> formatResponse (mock-faithful).
LIST GROUP BY verified live on Dataview 0.5.68; TASK GROUP BY covered by synthetic fixtures (no grouped-task data in the test vault). TABLE GROUP BY is outside #220's stated scope.
dataview.list and dataview.metadata non-raw output always rendered "No results found": the dispatcher force-cast their {success,count,pages} and {success,path,metadata} producer shapes into the {values} query formatter (formatDataviewQuery reads response.values), which always hit the empty branch. raw:true was unaffected.
A page list and a single page's metadata aren't query results, so add dedicated formatDataviewPages and formatDataviewMetadata and route the two dispatcher cases to them instead of formatDataviewQuery. Both surface producer-side failures (bad source, page-not-found) instead of a misleading empty result.
Adds mock-faithful regression tests through formatResponse() covering both happy paths and failure paths (the #216 mock-faithfulness lesson). Verified live against Dataview 0.5.68.
The .mcpb bridge ships inside the GitHub release bundle, and BRAT updates
only the Obsidian plugin — never the Claude Desktop connector. Users had no
signal that a plugin update doesn't deliver bridge-side fixes (#243/#247).
Add a standing "Claude Desktop (.mcpb connector)" section to the release
workflow's notes template so every release tells .mcpb users to re-import,
rather than depending on per-release notes remembering to.
Claude-Session: https://claude.ai/code/session_011yowKCMFCBp61DRBu5xQm4
Replace the JEST_WORKER_ID bootstrap guard with a bridge-owned
MCP_BRIDGE_NO_AUTOSTART opt-out. JEST_WORKER_ID is a jest-worker
artifact that isn't reliably set under --runInBand, so an in-band run
could let bridge-self-heal's in-process require() fire main() and
process.exit(1) — the same boot-trap class this fix exists to kill.
The bridge now declares its own boot contract: autostart unless a
caller explicitly opts out. bridge-self-heal sets the var before
requiring; bridge-bootstrap strips it from spawned child env (in case
a sibling test set it in the worker) so real launches still autostart.
Test robustness (review nits): parse only a complete newline-
terminated stdout line (a mid-line chunk split would throw on partial
JSON and flake); route every terminal path through a `finish` helper
that clears the timer and kills the child, so no failure mode orphans
a process. Expand the guard comment to note the manifest launches
`node server.js` (where require.main would be true) while Desktop's
built-in-Node path is the one that breaks.
Refs #243.
Claude-Session: https://claude.ai/code/session_011yowKCMFCBp61DRBu5xQm4
0.11.34 shipped a broken bridge: #243 gated startup on
`require.main === module`, but Claude Desktop runs the .mcpb with its
built-in Node ("Using built-in Node.js for MCP server") via a loader
that does NOT make server.js the main module. So main() never ran,
stdin was never read, the client's `initialize` went unanswered, and
Desktop timed out after 60s ("Request timed out", -32001) — the
extension "couldn't connect at all". (Local `node mcpb/server.js`
worked, which is why it passed review: that path makes
require.main === module true.)
Gate on the absence of Jest's worker env instead. main() then runs in
every real launch — system `node server.js` and Desktop's built-in
Node alike — while the require()-based test seam stays inert under
Jest. Reproduced via `node -e "require('./mcpb/server.js')"` (hangs
before, responds after).
Add tests/bridge-bootstrap.test.ts: spawns the bridge against a stub
HTTP server and asserts it answers `initialize` in BOTH launch shapes
(`node server.js` AND non-main require). The existing unit tests
`require()` the module — which by design skips main() — so they could
never catch a boot failure; this is that missing coverage. Verified
non-vacuous: reverting the guard fails the non-main case.
Refs #243. The GitHub 0.11.34 release .mcpb has this bug and must not
be promoted; ship the fix in 0.11.35.
Claude-Session: https://claude.ai/code/session_011yowKCMFCBp61DRBu5xQm4
Code review refinements (no behavior change):
- Make the SSE socket-exemption trade-off explicit: with setTimeout(0)
there is no socket-layer idle reap, so a truly-dead SSE socket is
bounded by the SessionManager's 1h idle eviction (which calls
transport.close()) and the 32-session cap, not leaked unbounded. Note
the finite-timeout alternative is deferred to live validation.
- Clarify the comment: the exemption matches any GET, but non-SSE GETs
are short-lived so it's a harmless no-op for them.
- Add afterEach teardown disposing the SessionManager + connection-pool
intervals (server.stop() short-circuits when never started), removing
the test's leaked-timer warning.
- Strengthen the missing-socket test to assert the request still routes
to the spec §2 400, not just that it doesn't throw.
Claude-Session: https://claude.ai/code/session_011yowKCMFCBp61DRBu5xQm4
Two transport/session-lifecycle hygiene fixes surfaced while tracing
the #221/#238 session cluster.
SSE notify-stream churn: `GET /mcp` opens the standalone SSE
notification stream, which is long-lived and idle by design (this
server pushes no server-initiated notifications). The server-wide
120s idle socket timeout set in start() reaped it every ~2 min,
surfacing as the "stream terminated" reconnect churn and noisy logs
in bridge reports (#221). handleMCPRequest now exempts only the GET
(SSE) socket from the idle timeout via req.socket.setTimeout(0); POST
request sockets keep the server-wide default.
Dead code: removed attachTransportHandlers, which wired
transport.on('close'|'error'). SDK 1.29's StreamableHTTPServerTransport
is not an EventEmitter and has no `.on`, so it never fired — dead and
misleading. Transport cleanup is already covered by the SessionManager
`session-evicted` handler (close + map delete; every transport maps to
a manager-tracked session that is eventually idle-evicted, so none
leaks) and the explicit DELETE /mcp handler. Replaced the helper with a
comment documenting those two paths.
Adds tests/sse-socket-timeout.test.ts (GET exempts its socket, POST
does not, missing-socket tolerated).
Refs #221. Behavioral SSE change; verify on a live Obsidian instance
before release.
Claude-Session: https://claude.ai/code/session_011yowKCMFCBp61DRBu5xQm4
Code review of the self-heal path surfaced a concurrency correctness
gap and some observability refinements.
Correctness: the 404 self-heal was gated on `response.status === 404
&& sessionId`. When two tool calls 404 concurrently on a dead
session, the first call's reinitialize() synchronously nulls
sessionId before the second call's 404 is handled, so the sibling
failed the `&& sessionId` guard, skipped self-heal, and surfaced
-32603 "Bridge: HTTP 404" instead of recovering. Drop the guard: a
404 on /mcp always means a session id was presented (the server
answers 400 when none is), so any non-initialize 404 is a self-heal
trigger regardless of current bridge state. Both concurrent calls now
coalesce onto the single in-flight reinit and replay under the fresh
session. Added a concurrent-dispatch regression test proving one
reinit + both replays under the new session id.
Observability: log a non-2xx (or thrown) notifications/initialized
during reinit so a required-but-failing handshake notification is
diagnosable from stderr; soften the comment to match its best-effort
reality; note the startNotifyStream fire-and-forget intent. Align the
test's __state() type with the seam.
Claude-Session: https://claude.ai/code/session_011yowKCMFCBp61DRBu5xQm4
The .mcpb stdio bridge cleared its session on a 404 and emitted
-32000 "please reinitialize", relying on the client to send a fresh
initialize. Clients that ignore that signal (Claude Desktop) then
sent the next tool call with no session → HTTP 400, and the
connection dead-ended until a manual restart. This is the #221/#238
"worked yesterday, broke this morning, tools time out" symptom: a
server restart (Obsidian reload / plugin update) or the 1-hour idle
session GC orphans the bridge's in-memory session.
The bridge now caches the initialize handshake and, on a 404 for a
non-initialize call, transparently re-initializes (fresh synthetic
id, result discarded) + completes the handshake with
notifications/initialized + replays the original message once, then
emits that real result. Recovery is invisible to the client and no
client-side fix is required. An isReplay flag caps it at a single
attempt so a session that dies again can't spin a reinit loop; the
existing `initializing` gate serializes concurrent 404s onto one
re-init.
Made the bridge require()-able (bootstrap guarded by
require.main === module, dispatch/state exported) so the self-heal
path is unit-testable without spawning a process. Adds
tests/bridge-self-heal.test.ts covering reinit+replay success, the
give-up path when re-init fails (no loop), and initialize caching.
Closes#238. Addresses #221 for users on the bundled .mcpb bridge
(third-party bridges like supergateway are unaffected; the server's
404 signal remains spec-compliant per ADR-106).
Claude-Session: https://claude.ai/code/session_011yowKCMFCBp61DRBu5xQm4
Move createProtocolInfoSection to render first, ahead of Connection
status and the config sections, so the one-click MCPB bundle, Claude
Code config, and advanced client config are the first thing a user
sees instead of being buried at the bottom of the tab. Rename its
heading "Mcp protocol information" -> "Getting started — connect a
client" to match the content.
Stays on the sanctioned display()->render() fallback (minAppVersion
1.6.6); declarative-API migration remains tracked in #224.
Claude-Session: https://claude.ai/code/session_011yowKCMFCBp61DRBu5xQm4
The portal review forbids disabling no-deprecated / prefer-window-timers /
no-global-this (an inline eslint-disable for these is a gating Error) and
errors on undescribed directive comments. The reflex to suppress fails the
review — it cost a round trip this session (the setWarning 1037 suppression).
Records the rule, the fail-fast behavior, and the four fix patterns that
worked across 0.11.32→0.11.33 (display→render, setWarning→setClass, global
timers→window.* + test polyfill, Server→McpServer.server), so the next session
reaches for the fix instead of the disable.
The Obsidian review flags @typescript-eslint/no-deprecated on the low-level
Server class (mcp-server.ts:457, mcp-server-pool.ts) and forbids disabling the
rule — so suppression isn't an option. We can't adopt McpServer's high-level
registerTool API either (it wants Zod; we use raw JSON Schema via
setRequestHandler).
Resolution: construct McpServer (non-deprecated) and register our existing raw
handlers on its underlying .server — the advanced low-level handle it
deliberately exposes ("useful for advanced operations"). The deprecated Server
symbol never appears in our source, so no-deprecated has nothing to flag, while
every setRequestHandler call and the session-pool architecture stay identical.
Verified McpServer installs its own tools/list handler lazily (only via
registerTool, which we never call), so it can't shadow ours — no empty-tools
regression (cf. #221). Removed the now-unneeded no-deprecated eslint exemption
for these files; lint passing with it enforced proves the symbol is gone.
Lint clean, build green, 336 tests pass.
Obsidian's source-code review forbids disabling @typescript-eslint/no-deprecated
(and requires descriptions on directive comments), so the scoped eslint-disable
added for setWarning() in #225 became a gating Error at main.ts:1037 — failing
the 0.11.33 review.
setWarning() is deprecated (1.13.0) and its replacement setDestructive() requires
1.13.0 > our minAppVersion 1.6.6 (no-unsupported-api). Both dead ends. Apply the
'mod-warning' class directly via setClass() (non-deprecated, @since 0.9.7) — same
destructive styling, no suppression, works on all supported versions.
Full 1.13.0 API adoption (incl. setDestructive + getSettingDefinitions) tracked in #224.
The blanket 7-day hold delayed exactly the vuln patches Obsidian's plugin
review and npm audit flag — leaving us knowingly exposed to a named CVE to
guard against a hypothetical malicious republish. Split the rule: routine
version bumps still age 7 days; security fixes (dependabot security PRs,
npm audit advisories, Obsidian-review dependency warnings) land immediately
once green.
Obsidian's source-code review forbids disabling obsidianmd/prefer-window-timers
and obsidianmd/no-global-this (and flags undescribed directive comments). The
0.11.32 review failed on five such disables.
- router.ts, obsidian-api.ts, session-manager.ts: global set/clearInterval and
setTimeout → window.* equivalents.
- debug.ts: drop the globalThis console fallback → window.console.
- tests/setup.ts: alias window to globalThis in the node test env so the
window.* timers/console resolve under Jest (Obsidian always provides window
at runtime).
- session-manager cleanupInterval typed number (window.setInterval's return).
All five eslint-disable directives removed. Lint clean, build green, 336 tests pass.
The dev-dependency bump tightened lint and exposed pre-existing debt.
Resolved without blanket rule suppression:
- no-unnecessary-type-assertion (15): removed redundant assertions
(eslint --fix) across formatters, mcp-server, security, utils, validation.
- no-base-to-string (6): the removed assertions un-masked String(unknown)
calls. All were already guarded (object→JSON.stringify, primitive→String),
so not bugs — narrowed via a typed primitive local, which also satisfies
no-unnecessary-type-assertion (whose receiver String() accepts anything).
- no-deprecated (15): obsidian 1.13.0 deprecates PluginSettingTab.display()
in favor of getSettingDefinitions(). Kept display() as the supported
cross-version fallback and routed internal re-renders through a new
render() method, so no deprecated symbol is referenced and no future
deprecation in main.ts is masked. Full declarative migration: #224.
- no-deprecated (1, ButtonComponent.setWarning): the replacement
setDestructive() requires 1.13.0 > our minAppVersion 1.6.6, so it's kept
with one scoped eslint-disable + comment. Revisit with #224.
- no-unused-vars (2): dropped now-unused http type imports in mcp-server.
- prefer-active-doc (5): document → activeDocument for popout-window
compatibility (main settings DOM queries, image-handler canvas).
Lint clean, build green, 336 tests pass.
Supersedes #217 (dependabot). Brings the dev toolchain current:
@types/node 25.3→25.9, @typescript-eslint/utils + typescript-eslint
8.56→8.60, esbuild 0.27→0.28, globals 17.3→17.6, jest 30.2→30.4,
jiti 2.6→2.7, obsidian 1.12.2→1.13.0, ts-jest 29.1→29.4.
The typescript-eslint 8.60 and obsidian 1.13.0 type bumps surface new
lint findings; cleared in the follow-up commit.
The '/' traverse summary reported Math.min(10, getFiles().length) using the
unfiltered file count, while the traversal actually seeds from the
.mcpignore-filtered set (graph-traversal.ts). With exclusions enabled this
overcounted and leaked that hidden files exist via an inflated number. Count
the same filtered set the seed logic uses.
Move Quick Start above the conceptual sections so install is the first
thing readers hit, and relocate 'Why Semantic MCP?' down to before Core
Tools. Add an inline-emoji hero banner (box → robot) for the .mcpb
drag-into-Claude-Desktop flow — glyphs render on the Obsidian listing
where embedded images get stripped, making the one-click path obvious
to non-power-users.
Graph operations (neighbors/backlinks/forwardlinks/traverse/path/
statistics) read metadataCache.resolvedLinks and vault.getFiles()
directly and never consulted the MCPIgnoreManager. With path
exclusions enabled, .mcpignore-ignored paths surfaced as neighbors/
edges of non-ignored queries, and querying an ignored note directly
disclosed its relationships.
- GraphSearchTool rejects an excluded query root (treated as
"File not found", matching ObsidianAPI's read-path behavior).
- GraphTraversal filters excluded paths out of the link primitives and
every file enumeration (neighbors, root traversal, tag connections,
vault statistics, all-nodes).
- Add ObsidianAPI.getIgnoreManager() accessor + regression tests.
No behavior change when path exclusions are disabled.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Dataview's `query()` resolves to a `Result` monad —
`{ successful, value: { type, values, headers }, error }` — where
`value.values` is a plain array. The tool modelled the return as a flat
`{ type, values }` object with a wrapped `DataArray`, so:
- `formatQueryResult()` switched on `result.type` (undefined → `result.value.type`),
collapsing every query to the `unknown` branch → `# Dataview: UNKNOWN`.
- it called `result.values?.array()` (undefined → `result.value.values`,
already a plain array) → `[]` → "No results found".
`dataview.status` had a separate shape mismatch: the detector returns
`{ installed, enabled, apiReady, version }` but `formatDataviewStatus`
reads `available`, so it always rendered "not available".
Fixes:
- Re-model `DataviewQueryResult` as the real monad; read payload from `.value`.
- Add `toPlainArray()` helper (accepts plain arrays and wrapped DataArrays).
- Add a `dataview.status` normalizer mapping apiReady → available.
- Correct the test mock, which encoded the same wrong flat+wrapped shape —
the reason CI stayed green while real Dataview v0.5.68 failed.
- Add formatted-output regression tests through formatResponse() for
query (LIST/TABLE) and status.
