feat: add file_path parameter to bear-add-file#90
Conversation
Allow attaching files by local path instead of base64-encoded content. The server reads and encodes the file internally, saving the LLM from producing thousands of base64 output tokens. Created with Claude Code under the supervision of Serhii Vasylenko
The manifest.json still described base64-only input after the file_path feature was added. Synced the updated description to README and NPM docs. Created with Claude Code under the supervision of Serhii Vasylenko
…-file Add empty-file guard after readFileSync to prevent silently dropping the file parameter in Bear URL (empty base64 is falsy in buildBearUrl). Change id/title validation from throw to createToolResponse to match every other handler in the codebase. Created with Claude Code under the supervision of Serhii Vasylenko
|
Recreating via proper workflow |
There was a problem hiding this comment.
Pull request overview
Adds a file_path input mode to bear-add-file so clients can attach local files without generating large base64 payloads in-model.
Changes:
- Extend
bear-add-fileto acceptfile_path, inferfilenamefrombasename(file_path), and enforce mutual exclusivity withbase64_content. - Add system tests covering
file_pathsuccess, explicit filename override, and missing-file errors. - Update tool descriptions across
manifest.json,README.md, NPM docs, andCHANGELOG.md.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/main.ts |
Implements file_path support, filename inference, and new validation/error handling for bear-add-file. |
tests/system/attached-files.test.ts |
Adds system coverage for bear-add-file with file_path (including error case). |
README.md |
Updates tool list description for bear-add-file. |
manifest.json |
Updates bear-add-file tool description exposed to MCP clients. |
docs/NPM.md |
Updates published NPM docs tool list description for bear-add-file. |
CHANGELOG.md |
Documents the new file_path mode in Unreleased notes. |
| file_path: z | ||
| .string() | ||
| .trim() | ||
| .min(1) | ||
| .optional() | ||
| .describe( | ||
| 'Absolute path to a file on disk. Preferred over base64_content when the file already exists locally.' | ||
| ), |
There was a problem hiding this comment.
file_path is documented as an absolute path, but the handler currently accepts relative paths as-is. That can lead to attaching an unintended file depending on the server process CWD, and it weakens the safety/clarity of the new parameter. Consider validating that file_path is absolute (e.g., path.isAbsolute) and returning a soft error if not; optionally normalize/resolve before reading.
| file_path: z | ||
| .string() | ||
| .trim() | ||
| .min(1) | ||
| .optional() | ||
| .describe( | ||
| 'Absolute path to a file on disk. Preferred over base64_content when the file already exists locally.' | ||
| ), | ||
| base64_content: z | ||
| .string() | ||
| .trim() | ||
| .min(1, 'Base64 file content is required') | ||
| .describe('Base64-encoded file content'), | ||
| .min(1) | ||
| .optional() | ||
| .describe( | ||
| 'Base64-encoded file content. Use file_path instead when the file exists on disk.' | ||
| ), | ||
| filename: z | ||
| .string() | ||
| .trim() | ||
| .min(1, 'Filename is required') | ||
| .describe('Filename with extension (e.g., budget.xlsx, report.pdf)'), | ||
| .min(1) | ||
| .optional() | ||
| .describe( | ||
| 'Filename with extension (e.g., budget.xlsx, report.pdf). Required when using base64_content. Auto-inferred from file_path when omitted.' | ||
| ), |
There was a problem hiding this comment.
Most tool schemas in this file provide explicit Zod validation messages (e.g., bear-replace-text uses .min(1, 'Text content is required')), but the new bear-add-file params use .min(1) without messages. This will surface generic Zod errors (e.g., “String must contain at least 1 character(s)”) and makes the “clear errors” contract less consistent. Suggest adding parameter-specific .min(1, ...) messages for file_path, base64_content, and filename (especially since filename is only required in base64_content mode).
| if (file_path) { | ||
| // Read file from disk and encode — avoids the LLM producing thousands of base64 tokens | ||
| try { | ||
| const buffer = readFileSync(file_path); | ||
| if (buffer.length === 0) { | ||
| return createToolResponse(`File is empty: ${file_path}`); | ||
| } | ||
| fileData = buffer.toString('base64'); | ||
| } catch (err) { |
There was a problem hiding this comment.
readFileSync(file_path) introduces blocking disk I/O on the server’s main thread. For large attachments this can stall handling of other MCP requests and degrade responsiveness. Consider switching to async file reads (fs/promises) and (optionally) adding a file size check before base64-encoding to avoid excessive memory usage.
Summary
file_pathas an alternative tobase64_contentinbear-add-file— the server reads files from disk and encodes them internally, avoiding the LLM producing thousands of base64 output tokensfilenameis auto-inferred from the path viabasename()when not explicitly providedfile_pathandbase64_contentare mutually exclusive; empty files and missing files return clear errorstask docs:syncCloses #88
Test plan
bear-add-filewithfile_pathpointing to a real file attaches correctly in Bearbear-add-filewithbase64_contentstill works (regression)Created with Claude Code under the supervision of Serhii Vasylenko