feat(assets): vault_read_asset + vault_list_assets — read and browse non-markdown vault files#351
feat(assets): vault_read_asset + vault_list_assets — read and browse non-markdown vault files#351aliasunder wants to merge 23 commits into
Conversation
…ough gate Registers a throwaway tool in mcp-router (not a tool group, so registration-count tests stay green) returning a 48x48 magenta PNG as an MCP image content block. Used to verify the claude.ai remote connector passes image blocks through to the model before investing in the image pipeline. Revert this commit before merge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
…-passthrough gate" This reverts commit 5f63dcd.
…lumn Foundations for vault_read_asset/vault_list_assets: binary+stat fs utils, vaultFs.readAsset (path safety, .md rejection, stat-first size cap), listAssets folder scoping, statAssets; links.getExtension beside stripExtension; canvas.ts linearizer (JSON Canvas 1.0 — spatial group containment, reading order, id-resolved edges); fit-image-to-byte-budget (sharp: autoOrient, quality ladder, WebP-for-alpha, dimension descent); MAX_ASSET_BYTES + MAX_IMAGE_OUTPUT_BYTES config; non_md_files bytes column (guarded migration, statted at rebuild + watcher) surfaced via COALESCE in getOutgoingLinks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
…Content safeHandlerContent becomes the error-contract core (safeHandler delegates as the single-text-block case); new asset-tools group registers vault_read_asset (dispatch: images via fit-to-budget pipeline as image blocks + metadata line, .canvas linearized, text passthrough set verbatim, .pdf/unknown structured errors, fixed 100KB text cap) and vault_list_assets (filesystem-backed listing with folder/extension filters, page-statted bytes, full-set counts). Cross-references added to read_note (.md error routing) and outgoing_links (bytes now live for assets) descriptions; READ_ONLY_TOOLS + pinned cross-ref + handler + data-layer tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
MAX_ASSET_BYTES + MAX_IMAGE_OUTPUT_BYTES through all four compose files, both deploy .env.examples (+ root), cli env blocks (sync script), and server.json environmentVariables. Docs: README Assets category + feature bullet, ARCHITECTURE Assets section + outgoing-links bytes note, AGENTS.md structure tree + obsidian-markdown JSON-format layering note + read_asset path-convention inverse, wiki.json (3 pages), social-preview SVG feature text (PNG re-render deferred — local Chrome headless screenshot hangs, unrelated to this change), DOCKERHUB regen, cli/README feature line, server instructions string, vault-orientation go-deeper list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
… error contract Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
There was a problem hiding this comment.
Sorry @aliasunder, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
PR Code Suggestions ✨Explore these optional code suggestions:
|
… category Line overflowed the canvas at font-size 22 after adding 'assets'; 19 fits with margin. Render script unchanged — the earlier screenshot timeouts were machine-load contention (concurrent Docker builds + test suites), not a script defect; verified by a 4-cell diagnostic matrix passing and the unmodified script rendering on an idle machine. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
- Replace reduce-with-spread counting in vault_list_assets with a plain for...of loop — eliminates a double extensionOf call per item and reads on its own without tracing an accumulator. - Add blank comment line in root .env.example between WINDOWS_MODE and MAX_ASSET_BYTES for visual group separation (matches deploy/ examples). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
… config fields Ship-check Phase 3 coverage gap analysis found three utility-level exports and two config fields without direct tests despite being exercised indirectly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
…e 3 soft gap) The file-watcher test suite already spy-mocks utils/fs.js, so the race is deterministic: statOrNull forced to null for one path, real for the rest — no timing sensitivity. Without the guard, the watcher callback crashes (null.size) and vitest fails the run with the error attributed to this test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
…iption The description unconditionally stated "Animated GIFs deliver their first frame" but the passthrough path (GIF already within budget and ≤1568px) returns the full animated buffer unchanged. Qualify to "when recompressed to fit the budget" which is accurate — sharp extracts the first frame only during the resize/encode path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
…tness, O(n) file filter, doc direction - canvas.ts: identical-rect groups each claimed the other as parent, so neither was top-level and both vanished from the rendition; containment now gets one deterministic direction (higher id contains lower) for group pairs. Content nodes exempt (never parents). - asset-tools.ts: text passthrough and canvas decode UTF-8 strictly — invalid bytes now error instead of silently becoming U+FFFD (the description promises verbatim content). New Errors entry + test. - search-index.ts: visibleFilesOfKind spread-reduce → filter/map chain (O(n), no per-entry array copies). - Review direction: search-coverage phrasing stated positively in both tool descriptions + README bullet; new README Assets section (bullet anchors to it); cli/README feature line reworded; WINDOWS_MODE added to server.json environmentVariables; DOCKERHUB regenerated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
…cost vault_list_assets and vault_get_outgoing_links report the on-disk file size; vault_read_asset downscales images to fit response limits, so a large listed image is still cheap to read. Both Returns sections now say so explicitly (text formats deliver verbatim, so there the listed size is the read cost). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
|
@CodeRabbit review |
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughChangesAsset support
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (1)
src/utils/__tests__/fs.test.ts (1)
65-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the non-null assertions from these tests.
The assertions can validate both presence and value without bypassing strict narrowing.
Proposed fix
const result = await readBinaryFileOrNull(path) - expect(result).not.toBeNull() - expect(result!.equals(bytes)).toBe(true) + expect(result).toEqual(bytes)const stats = await statOrNull(path) - expect(stats).not.toBeNull() - expect(stats!.isFile()).toBe(true) - expect(stats!.size).toBe(5) + expect(stats?.isFile()).toBe(true) + expect(stats?.size).toBe(5)As per coding guidelines,
src/**/*.tsmust not use non-null assertions.Also applies to: 86-88
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/__tests__/fs.test.ts` around lines 65 - 66, Remove the non-null assertions from the assertions around the result checks in the filesystem tests, including the additional occurrence around lines 86–88. Structure the expectations so they first narrow or validate result presence and then compare its value to bytes without using the `!` operator, preserving the existing test behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli/README.md`:
- Around line 10-13: Update the introductory description of Vault Cortex to
explicitly state that images, canvases, and data files are available for
read-only access through vault_read_asset and vault_list_assets, while
preserving the existing write-access claim for supported vault content.
In `@README.md`:
- Line 223: Update both documented occurrences of “per-type counts” in README.md
to “per-extension counts,” including the description associated with
vault_list_assets, while leaving the rest of each sentence unchanged.
In `@src/utils/fit-image-to-byte-budget.ts`:
- Line 31: Update the retry and resize logic around MAX_ENCODE_ATTEMPTS and the
loop covering the image-fitting flow so descent can reach and encode the
documented 64px minimum; ensure the termination condition at the 64px floor
performs that final encode instead of exiting first. Preserve the existing
quality and budget checks while allowing small positive budgets to succeed when
the 64px output fits.
In `@src/vault-mcp/__tests__/config.test.ts`:
- Around line 315-318: Add boundary-case tests in the config validation suite
for both MAX_ASSET_BYTES and MAX_IMAGE_OUTPUT_BYTES, asserting that "0", "-1",
and "1.5" are rejected. Keep the existing non-numeric rejection tests and use
loadConfig in each case to verify both environment variables require positive
integers.
In `@src/vault-mcp/mcp-core/__tests__/tool-definitions.test.ts`:
- Around line 1145-1156: Update the test case “filters by extension
case-insensitively with or without the leading dot” to run with both accepted
input spellings, “PNG” and “.PNG”, using parameterization or equivalent focused
coverage. Keep the existing expected result and fixture setup unchanged so the
test verifies both case-insensitive and leading-dot handling.
In `@src/vault-mcp/mcp-core/tools/asset-tools.ts`:
- Line 241: Update the return-description text near the asset listing to remove
the claim that every listed asset is readable via vault_read_asset. Clarify that
only supported asset types can be read, while PDFs, unknown extensions, and
other unsupported formats are rejected; retain the existing vault_search
guidance for markdown notes.
- Around line 256-259: Update the limit schema in the asset tool definition to
require an integer with a minimum value of 1, while preserving its optional
default-50 behavior and description.
In `@src/vault-mcp/obsidian-markdown/canvas.ts`:
- Around line 120-129: Update displayName’s heading cleanup so it removes
leading hashes only when they represent an ATX heading: the hash sequence must
be followed by whitespace or the end of the line. Preserve hashes in Obsidian
tags such as “#project” while continuing to format headings like “# Title” as
“Title.”
- Around line 94-111: Update the containing-group selection in the visible
reduce logic so equal-area candidates are resolved by choosing the lower group
ID, making content-node placement independent of JSON order while preserving the
existing smallest-area behavior. Add a regression test that reverses the canvas
input and verifies the same lower-ID group is selected.
In `@src/vault-mcp/search/search-index.ts`:
- Around line 1403-1414: The asset snapshot rebuild around
visibleFilesOfKind("asset") is not synchronized with watcher updates, allowing
deletions or size changes to produce stale inserted rows or byte counts. Update
the rebuild flow to queue watcher events while statting and inserting assets,
then replay them or perform a final synchronized reconciliation immediately
before commit; apply the same protection to the related logic at the noted
secondary section.
In `@src/vault-mcp/vault-operations/vault-filesystem.ts`:
- Around line 530-532: Update the Markdown checks in the asset-handling flow,
including both the guard near the existing “not an asset” error and the
corresponding logic around the second referenced location, to compare a
normalized extension case-insensitively. Compute the normalized extension once
and reuse that value when constructing the response, preserving existing
behavior for non-Markdown assets.
- Around line 533-544: Update the asset-reading flow around statOrNull and
readBinaryFileOrNull to read through a file handle with a buffer limited to
maxBytes + 1, rather than loading the entire file. Reject the asset when the
extra byte is present, while preserving the existing not-found, non-file, and
pre-read size validation behavior.
---
Nitpick comments:
In `@src/utils/__tests__/fs.test.ts`:
- Around line 65-66: Remove the non-null assertions from the assertions around
the result checks in the filesystem tests, including the additional occurrence
around lines 86–88. Structure the expectations so they first narrow or validate
result presence and then compare its value to bytes without using the `!`
operator, preserving the existing test behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 29f568ea-a068-4861-9dcc-252f2ce58604
⛔ Files ignored due to path filters (3)
assets/social-preview.pngis excluded by!**/*.pngassets/social-preview.svgis excluded by!**/*.svgpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (43)
.devin/wiki.json.env.exampleAGENTS.mdARCHITECTURE.mdDOCKERHUB.mdREADME.mdcli/README.mdcli/src/env.tsdeploy/local/.env.exampledeploy/local/docker-compose.ymldeploy/remote/.env.exampledeploy/remote/docker-compose.ymldocker-compose.local.ymldocker-compose.ymlpackage.jsonserver.jsonsrc/utils/__tests__/fit-image-to-byte-budget.test.tssrc/utils/__tests__/fs.test.tssrc/utils/fit-image-to-byte-budget.tssrc/utils/fs.tssrc/vault-mcp/__tests__/config.test.tssrc/vault-mcp/config.tssrc/vault-mcp/mcp-core/__tests__/mcp-router.test.tssrc/vault-mcp/mcp-core/__tests__/tool-definitions.test.tssrc/vault-mcp/mcp-core/__tests__/vault-orientation-prompt.test.tssrc/vault-mcp/mcp-core/mcp-router.tssrc/vault-mcp/mcp-core/prompts/vault-orientation-prompt.tssrc/vault-mcp/mcp-core/tool-definitions.tssrc/vault-mcp/mcp-core/tools/asset-tools.tssrc/vault-mcp/mcp-core/tools/search-tools.tssrc/vault-mcp/mcp-core/tools/tool-helpers.tssrc/vault-mcp/mcp-core/tools/vault-crud-tools.tssrc/vault-mcp/obsidian-markdown/__tests__/canvas.test.tssrc/vault-mcp/obsidian-markdown/__tests__/links.test.tssrc/vault-mcp/obsidian-markdown/canvas.tssrc/vault-mcp/obsidian-markdown/links.tssrc/vault-mcp/search/__tests__/file-watcher.test.tssrc/vault-mcp/search/__tests__/search-index.test.tssrc/vault-mcp/search/file-watcher.tssrc/vault-mcp/search/search-index.tssrc/vault-mcp/search/search-queries.tssrc/vault-mcp/vault-operations/__tests__/vault-filesystem.test.tssrc/vault-mcp/vault-operations/vault-filesystem.ts
…y names, capped read, zero-cap rejection - fit-image-to-byte-budget: descent now clamps to and encodes at the documented 64px floor before giving up (previously broke without ever trying it). - canvas: displayName strips only ATX headings (hashes + whitespace) so Obsidian tags keep their hash; equal-area group ties break on lower id so node ownership is JSON-order-independent (regression test with reversed declaration order). - readAsset: reads through a file handle into a stat-bounded buffer with a one-byte sentinel — a file growing between stat and read is rejected instead of ballooning memory; readBinaryFileOrNull removed as dead code. - config: reject 0 for MAX_ASSET_BYTES / MAX_IMAGE_OUTPUT_BYTES (env-var's asIntPositive admits 0, which would fail every read at runtime) — real gap surfaced by the suggested boundary tests. - vault_list_assets: limit schema int ≥ 1; Returns no longer overclaims that every listed asset is readable; README per-extension wording; cli/README read-only asset phrasing; fs.test.ts non-null assertions removed; extension-filter test covers .PNG spelling. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
|
CodeRabbit review round handled in 9859214 — 10 findings fixed, 1 false positive (rebuild/watcher race: the watcher is created only after the startup rebuild completes — evidence on the thread), 1 intentional with a design question flagged (.MD case-sensitivity is uniform system-wide; a vault-wide change is a separate decision). The review-body nitpick (non-null assertions in fs.test.ts) is also resolved: the readBinaryFileOrNull tests were removed with the now-dead util, and the statOrNull assertions use optional chaining. 🔍 ship-check · pr-monitor · claude-fable-5 |
…rst arc Opens with the agent's pain (an inert embed filename it can see but never look at) and pivots to the layer as resolution, matching Hybrid Search/Memory/Tasks. Ties the image bullet to the README's phone/remote through-line, verb-led Browse label, and adds the missing Assets entry to the Contents nav. DOCKERHUB regenerated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
…link graph The Assets opening implied agents can generally see assets exist and learn sizes — those are vault-cortex link-graph capabilities, not an ecosystem baseline (verified against 11 Obsidian MCP servers: none resolve assets inside a link graph). Reframed as the integrated stack — link graph resolves and sizes, asset layer reads — stating capability without exclusivity claims (which the research also shows would be partly false for image blocks: obsidian-mcp-pro ships them). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
raw: true returns the file's text source instead of any rendered form — for .canvas that's the JSON Canvas source (geometry, ids, colors); text formats already return source so raw is a no-op there; images error (binary has no text source). Restores source fidelity for canvases while keeping the readable outline as the default. Mirrors vault_read_note's representation-mode flags. 3 tests (mutation-checked), README/ ARCHITECTURE/DOCKERHUB updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
Each constant states its own rationale (model downscale ceiling, legibility floor, deterministic ladder, mid-ladder handoff, provable termination) so the file reads top-to-bottom without hopping back to the module docstring. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
The image tool_result line now carries originalBytes plus delivered and original dimensions alongside deliveredBytes/recompressed — one self-contained line for tuning MAX_IMAGE_OUTPUT_BYTES from logs alone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
'(downscaled to fit)' implied every image is downscaled — the pipeline passes small images through untouched and sometimes only recompresses. All six surfaces now state the conditionality: README marquee bullet (shrunk to fit when needed) + Assets section (when they exceed), tool description (delivered untouched otherwise), list_assets bytes note, server.json, both deploy .env.examples (+ CLI blocks resync), DOCKERHUB regenerated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
The Bases bullet linked its docs; Canvas didn't — README's canvas bullet now links Obsidian's Canvas help, and ARCHITECTURE links the JSON Canvas 1.0 spec (jsoncanvas.org). Both URLs verified live. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
What
Two new MCP tools (new Assets category) make the vault's non-markdown files readable and browsable:
vault_read_asset— one dispatcher, most useful form per type:imagecontent block + one-line metadata text block. A shared fit-to-byte-budget pipeline (utils/fit-image-to-byte-budget.ts, sharp) auto-orients (EXIF), resizes to ≤1568px, walks a quality ladder (JPEG/mozjpeg opaque, WebP for alpha), and shrinks dimensions by √(budget/actual) if needed — deterministic, bounded, decompression-bomb-guarded.obsidian-markdown/canvas.ts, JSON Canvas 1.0): spatial group containment (innermost wins), reading-order sort, id-resolvedA → B (label)edge list, lenient shape validation.vault_list_assets— filesystem-backed discovery (vaultFs.listAssets, deliberately not the index): folder + case-insensitive extension filters, per-extension counts over the full filtered set, bytes statted only for the returned page.Also folded in: bytes for assets in
vault_get_outgoing_links—non_md_filesgains a nullablebytescolumn (guarded ALTER migration; startup rebuild backfills), statted at rebuild + watcher write sites, surfaced viaCOALESCE(n.bytes, f.bytes). Completes the flow: read note → outgoing links (assets with sizes) → read asset.Plumbing
safeHandlerContentis now the error-contract core (safeHandlerdelegates as the single-text-block case) — first non-text content blocks in the codebase; nooutputSchema(SDK requiresstructuredContentwith it, incompatible with image blocks).vaultFs.readAsset(sameresolveSafePathguard as notes,.mdrejection, stat-firstMAX_ASSET_BYTEScap),listAssetsfolder param,statAssets;readBinaryFileOrNull/statOrNullinutils/fs.ts;links.getExtensionbesidestripExtension(one home for extension semantics).MAX_ASSET_BYTES(default 50 MiB) andMAX_IMAGE_OUTPUT_BYTES(default 48 KiB binary, pre-base64 — sized for Claude Code's 25k-token MCP output cap) through the full deploy surface: 4 compose files, both deploy .env.examples + root, CLI env blocks (sync script), server.json.sharpadded as a direct dependency. The existingoverridesstub (scoped under@huggingface/transformers) stays; verified real sharp resolves top-level locally and in the built image, and the lockfile carries all@img/*platform variants. sharp ≥0.33 has no install script, so the Dockernpm ci --ignore-scriptspath needs no rebuild step.vault_read_assetas the deliberate.md-rule inverse), wiki.json (3 pages), DOCKERHUB regen, cli/README, server instructions string, vault-orientation prompt. Social-preview SVG updated; PNG re-render deferred (local Chrome headless screenshot hangs — environment issue, unrelated to this change).Verification
:testdeploy, reverted).:testdeploy: image read of a 200 KB PNG → 1568×697 JPEG, 42,960 bytes ≈ 16.4k tokens (under the 25k cap); real .canvas (33 nodes) and .base render correctly; SVG verbatim; PDF/unknown/missing/.md/traversal error arms; folder + extension filters; asset bytes live in outgoing links (backfilled by startup rebuild).🤖 Generated with Claude Code
https://claude.ai/code/session_01NGA157nZouoSUoYVX1WvFe
Summary by CodeRabbit