feat(deepseek): add file upload support via --file flag#1093
feat(deepseek): add file upload support via --file flag#1093jackwener merged 5 commits intojackwener:mainfrom
Conversation
Benjamin-eecs
force-pushed
the
feat/deepseek-file-upload
branch
from
6376e06 to
a7943b1
Compare
There was a problem hiding this comment.
Pull request overview
Adds --file support to the DeepSeek browser adapter so opencli deepseek ask can attach a local file before sending a prompt (issue #1092).
Changes:
- Add
--fileargument todeepseek askand route to a newsendWithFile()upload+send flow. - Implement
sendWithFile()inclis/deepseek/utils.js(read file, base64-inject into page, wait for preview, send). - Update DeepSeek adapter documentation and CLI manifest to include the new option (plus additional manifest regeneration updates).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| docs/adapters/browser/deepseek.md | Documents new --file flag and notes base64/memory behavior. |
| clis/deepseek/utils.js | Adds sendWithFile() implementation for file injection + send sequence. |
| clis/deepseek/ask.js | Adds CLI arg and file-send branch for deepseek ask. |
| cli-manifest.json | Adds deepseek ask --file to manifest; also updates some unrelated command columns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - Default mode is Instant with DeepThink and Search disabled; each flag (`--model`, `--think`, `--search`) is synced on every invocation so omitting a flag resets it | ||
| - Long responses (code, essays) may need a higher `--timeout` | ||
| - File upload is not yet supported | ||
| - File upload reads the file into memory and passes it via base64 to the browser; files over 100 MB are rejected |
There was a problem hiding this comment.
Docs state files up to 100 MB are supported, but the implementation base64-encodes the entire file and injects it into page.evaluate(), which can hit memory/transport limits well below 100 MB in practice. Please align this documented limit with a safe, tested maximum (or add a warning about potential failures for large files).
| "author", | ||
| "publish_time", | ||
| "status", | ||
| "size" | ||
| "size", | ||
| "saved" |
There was a problem hiding this comment.
This PR’s description says the changes are limited to the DeepSeek adapter files/docs, but this manifest hunk also updates the web read command columns (adds saved). If this is intentional (e.g. a full manifest regeneration), please update the PR description to reflect the extra scope; otherwise consider reverting unrelated manifest changes to keep the PR focused.
| "author", | ||
| "publish_time", | ||
| "status", | ||
| "size" | ||
| "size", | ||
| "saved" |
There was a problem hiding this comment.
This manifest hunk updates the weixin download command columns (adds saved), which is outside the scope described in the PR summary (DeepSeek --file). If this comes from regenerating cli-manifest.json, please call it out in the PR description; otherwise revert unrelated manifest diffs to reduce review/merge risk.
| await page.wait(3); | ||
| const response = await waitForResponse(page, 0, prompt, timeoutMs); | ||
| if (!response) { |
There was a problem hiding this comment.
When sending with --file, waitForResponse is called with baselineCount set to 0. In an existing conversation this can cause waitForResponse to immediately treat a previous assistant message as the “new” response (since result.count > 0), returning stale output. Capture the current bubble count before calling sendWithFile (same as the non-file path) and pass that baseline into waitForResponse (or have sendWithFile return the baseline it used).
| const stats = fs.default.statSync(absPath); | ||
| if (stats.size > 100 * 1024 * 1024) { | ||
| return { ok: false, reason: `File too large (${(stats.size / 1024 / 1024).toFixed(1)} MB). Max: 100 MB` }; | ||
| } | ||
|
|
||
| const content = fs.default.readFileSync(absPath); | ||
| const base64 = content.toString('base64'); |
There was a problem hiding this comment.
The 100 MB cap is likely unsafe for this implementation: the file is read into memory, base64-encoded (~33% larger), then injected into a page.evaluate() string and decoded with atob, which can cause very large transient allocations and/or transport payload limits. The repo already documents OOM risk for this pattern (e.g. clis/yollomi/upload.js caps at 10–20 MB for the same reason). Consider lowering the maximum size to a safer limit and/or reworking the upload to avoid injecting huge base64 strings into page.evaluate().
Benjamin-eecs
force-pushed
the
feat/deepseek-file-upload
branch
from
a7943b1 to
407afb8
Compare
Benjamin-eecs
force-pushed
the
feat/deepseek-file-upload
branch
from
407afb8 to
09b547b
Compare
* WIP: deepseek file upload (blocked by 30s idle timeout) * feat(deepseek): add file upload support via --file flag Closes jackwener#1092 * fix(deepseek): use native file input path for --file --------- Co-authored-by: Benjamin Liu <beneecs@Benjamins-Mac-mini.local> Co-authored-by: jackwener <jakevingoo@gmail.com> (cherry picked from commit b9bb302)
Summary
Closes #1092.
Adds
--fileflag todeepseek askfor attaching files (PDF, image, text) with prompts.Changes
Only additive, no existing code modified:
utils.js: addedsendWithFile()function (attach file via browser UI, type prompt, click send)ask.js: added--filearg, branches tosendWithFilewhen file is provideddeepseek.md: updated docs with--fileoption and usage exampleTest plan
npx tsc --noEmitpassesopencli deepseek ask "read file just the code" --new --file ./test.txtreturns correct file contentCommandExecutionErrordeepseek askwithout--filestill works--thinkand--model expertcombinations work