feat: add FILE_TOOLS_ENABLED env var to conditionally hide file tools#363
Merged
Conversation
When set to false, vault_read_file and vault_list_files are hidden from the tool list entirely — useful for remote deployments where Obsidian Sync has asset syncing disabled and no files exist on disk. Follows the exact MEMORY_ENABLED pattern: config parsing, conditional tool registration, cross-reference stripping from other tool descriptions and server metadata, startup logging, and full deploy surface coverage (compose files, .env.examples, CI workflows, server.json, README, DEPLOY, ARCHITECTURE, deploy guides, DOCKERHUB). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PR Code Suggestions ✨Explore these optional code suggestions:
|
![umm-actually[bot]](https://avatars.githubusercontent.com/in/4233716?s=60&v=4){kind=small}
README.md and DOCKERHUB.md claimed "File config vars (MAX_FILE_BYTES, etc.) are ignored when false" but config.ts unconditionally parses and validates them — an invalid value crashes at startup regardless of FILE_TOOLS_ENABLED. ARCHITECTURE.md correctly says "still parsed when disabled." Remove the misleading sentence rather than replace it; the core description is sufficient without it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HXbBfT61iG6EwV7ce42xX1
Replace the IIFE in mcp-router.ts instructions construction with plain const intermediates (matching the existing searchDescription pattern). Extract two inline ternaries from the outgoing-links tool description into named clauses (fileReadableClause, fileBytesClause). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HXbBfT61iG6EwV7ce42xX1
Both deploy/local/README.md and deploy/remote/README.md add a ## File Tools section but the Contents navigation line at the top of each doc was missing the [File Tools](#file-tools) link. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HXbBfT61iG6EwV7ce42xX1
Owner
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughChangesFile tools feature flag
Possibly related PRs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@deploy/local/.env.example`:
- Around line 56-58: Reorder the FILE_TOOLS_ENABLED block before MEMORY_ENABLED
in both deploy/local/.env.example lines 56-58 and deploy/remote/.env.example
lines 70-72, keeping each block’s content unchanged and maintaining consistent
ordering across the deployment templates.
In `@src/vault-mcp/mcp-core/__tests__/mcp-router.test.ts`:
- Around line 297-299: Replace non-null assertions with explicit precondition
checks: in src/vault-mcp/mcp-core/__tests__/mcp-router.test.ts at lines 297-299
and 316-318, verify the first mocked McpServer constructor call exists before
accessing its options; in
src/vault-mcp/mcp-core/__tests__/tool-definitions.test.ts at lines 760-764,
explicitly fail when a non-file tool lacks a description before scanning its
contents.
In `@src/vault-mcp/mcp-core/__tests__/tool-definitions.test.ts`:
- Around line 733-758: The non-file registration test currently checks only the
count, allowing omissions and duplicates to go unnoticed. Update the test around
registerWithDisabledFileTools and NON_FILE_TOOL_COUNT to compare the registered
tool names against the exact ALL_TOOL_NAMES set excluding FILE_TOOLS, and add a
separate assertion that registered names contain no duplicates.
🪄 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: af7f91ab-a429-4d51-9eca-1792fe02c75f
📒 Files selected for processing (29)
.env.example.github/workflows/deploy.yml.github/workflows/test_deploy.ymlAGENTS.mdARCHITECTURE.mdDEPLOY.mdDOCKERHUB.mdREADME.mdcli/src/env.tsdeploy/local/.env.exampledeploy/local/README.mddeploy/local/docker-compose.ymldeploy/remote/.env.exampledeploy/remote/README.mddeploy/remote/docker-compose.ymldocker-compose.local.ymldocker-compose.ymlserver.jsonsrc/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/search-tools.tssrc/vault-mcp/mcp-core/tools/vault-crud-tools.tssrc/vault-mcp/server.ts
…ertion - mcp-router.test.ts: replace `!` with `?.` + `toHaveLength(1)` precondition - tool-definitions.test.ts: replace count-only assertion with exact set comparison (catches omission+duplication); add `toBeDefined()` guard replacing `!` on description access Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HXbBfT61iG6EwV7ce42xX1
Obsidian calls non-markdown files "attachments" — "asset" is internal code vocabulary that doesn't match what users see in their Sync settings. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HXbBfT61iG6EwV7ce42xX1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FILE_TOOLS_ENABLEDenv var (defaulttrue) that hidesvault_read_fileandvault_list_fileswhen set tofalseMEMORY_ENABLEDpattern: config parsing, conditional tool registration, cross-reference stripping from other tool descriptions and server metadata, startup logging.env.examplefiles, CLI env sync, 2 CI workflows,server.json, README, DEPLOY, ARCHITECTURE, deploy guides, DOCKERHUBUse case: Remote deployments where Obsidian Sync has asset syncing disabled — no files on disk, so the file tools just produce "not found" noise and waste tool-description tokens in the MCP handshake.
What stays unchanged:
vault_get_outgoing_linksstill reports file links (kind: "file") — it indexes from the links table, not the file toolsMAX_FILE_BYTES,MAX_IMAGE_OUTPUT_BYTES,MAX_PDF_RENDER_PAGES) are still parsed when disabled (same asMEMORY_DIRwhenMEMORY_ENABLED=false)registerAssetTools,assetOperations)Test plan
npm test— 2065 tests pass (11 new: 6 config, 3 tool registration, 2 router metadata)npm run lint— 0 errorsnpm run build— compiles cleanlytemplates.test.ts— CI drift guard confirms compose ↔.env.example↔ CLI consistencyFILE_TOOLS_ENABLED=falsehides both file tools from the tool list🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
FILE_TOOLS_ENABLEDsetting, enabled by default.FILE_TOOLS_ENABLED=false.Documentation