feat: add MCP server support — local stdio server in the CLI + remote worker on mcp.uploads.sh#19
Merged
Conversation
Documentation for the new `uploads mcp` subcommand landing in the next commit: README MCP section + client registration examples, uploads-cli skill note, AGENTS.md layout mention, and roadmap update (local stdio server ships in the CLI; hosted McpAgent variant stays future work). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rw7dmzhGRndB3mBhTVKK8D
New `uploads mcp` subcommand serves the Model Context Protocol over stdio (newline-delimited JSON-RPC 2.0), hand-rolled with zero new runtime dependencies like the rest of the CLI. - src/mcp/server.ts: transport/runtime-agnostic protocol core (initialize version negotiation, ping, tools/list, tools/call; tool failures become isError results, never JSON-RPC errors), published as the @buildinternet/uploads/mcp subpath for reuse by a future remote worker - src/mcp/stdio.ts: Node stdio transport (responses only on stdout) - src/mcp/tools.ts: seven tools mirroring the CLI commands — put, attach, list, delete, comment, health, doctor — with the same config resolution, defaults, and mutual-exclusion rules; per-call workspace argument mirrors --workspace; token errors surface per call so the server starts without credentials - commands.ts: syncAttachmentsComment exported and doctor report extracted to buildDoctorReport so CLI and MCP share one code path - 19 new protocol/tool tests; pack check covers the new dist files - test/tsconfig.json: tests were in no TS project, so type-aware oxlint lacked node types for them (latent for existing tests); package typecheck now covers tests too Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rw7dmzhGRndB3mBhTVKK8D
Standalone Cloudflare Worker (apps/mcp) serving stateless MCP Streamable HTTP at POST /:workspace/mcp, authenticated per workspace exactly like REST: it shares uploads-api's REGISTRY KV, D1, and R2 bindings and reuses its workspaceAuth/storage modules via TS-source subpath exports (the @uploads/storage sharing pattern). The JSON-RPC protocol core comes from the @buildinternet/uploads/mcp subpath — one implementation for both the stdio and hosted servers. Tools mirror the roadmap's remote surface: put (base64 upload → public URL + GitHub-ready markdown, keys via badKey/buildScreenshotKey with no git inference), list, delete, and health, with per-scope enforcement (files:write/read/delete) from the token's D1 scopes. gh-comment/filesystem tools stay CLI-only. - wrangler.jsonc: custom domain mcp.uploads.sh, nodejs_compat, bindings copied from uploads-api (first deploy needs one-time Cloudflare setup; Workers Builds only auto-deploys api/web today) - root scripts dev:mcp / deploy:mcp / upload:mcp; CI test job and type generation cover the new app - 12 tests: auth-before-MCP, handshake, tool set, put/list/delete round-trip against a fake R2 bucket, scope denial, invalid key, notification→202, GET/DELETE→405, batch rejection Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rw7dmzhGRndB3mBhTVKK8D
Simplification pass over the branch (4-angle review: reuse, simplification, efficiency, altitude): - apps/api/src/files-core.ts: put/list/delete object ops extracted from the REST route bodies (with badKey + UPLOAD_CACHE_CONTROL); routes/files.ts and the uploads-mcp tools are now thin adapters over one code path, so future key-governance rules land once - @buildinternet/uploads/mcp now exports the tool arg-validation helpers (args.ts); the worker imports them instead of copies - single fake-r2 test double exported from @uploads/storage - shared makeGhTarget, client.listAll, io.ts writeStdout/writeJson, and a gh-target schema-props factory replace per-surface copies - doctor report carries its workspace-mismatch warning as a field instead of being recomputed by the printer - worker version read from package.json; FileScope re-exported from @uploads/api/workspace instead of an indexed-access derivation - scope denials stay plain errors (no USAGE code suffix) All suites pass (19+76+10+12), typecheck/lint/pack-check/dry-run bundle green; stdio smoke test unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rw7dmzhGRndB3mBhTVKK8D
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (2)
🚫 Excluded labels (none allowed) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Zach Dunn (zachdunn)
changed the title
apps/mcp imports the package's dist, so type-aware oxlint fails on a fresh checkout without building it first (the test job already builds via pretest hooks). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rw7dmzhGRndB3mBhTVKK8D
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
packages/storage/test/fake-r2.ts (1)
91-95: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant copy before the cast.
new Uint8Array(data)copiesdatainto a new buffer, but sincedatais already aUint8Array, the copy provides no typing benefit — theas unknown as ArrayBuffercast is what bypasses the checker either way. This is test-only code so impact is minimal.♻️ Proposed simplification
async blob() { // Cast keeps this typechecking under every consumer's lib config — // apps/mcp has no BlobPart (Workers types own globals there). - return new Blob([new Uint8Array(data) as unknown as ArrayBuffer]); + return new Blob([data as unknown as ArrayBuffer]); },🤖 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 `@packages/storage/test/fake-r2.ts` around lines 91 - 95, Remove the unnecessary `new Uint8Array(data)` allocation in the `blob()` method and cast the existing `data` value directly to satisfy the `Blob` constructor typing. Preserve the existing cross-consumer type-checking behavior and returned blob contents.
🤖 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 `@apps/api/src/files-core.ts`:
- Line 56: Clamp the limit in the shared listObjects function to a positive
minimum as well as the existing maximum, ensuring zero or negative caller values
cannot reach the storage backend. Update the limit calculation to enforce the
intended lower bound while preserving the default and 1000-item cap.
In `@apps/api/src/routes/files.ts`:
- Around line 15-30: Validate the route key with badKey immediately after
entering the PUT handler and before calling c.req.arrayBuffer(), returning the
mapped file-operation error for invalid keys. Then proceed with buffering and
putObject using the validated key, preserving the existing success and error
handling.
In `@apps/mcp/src/tools.ts`:
- Around line 20-28: Enforce a maximum allowed contentBase64 length before
calling atob in decodeBase64, using the existing usage error mechanism to reject
oversized payloads with a clear message. Apply the same validation to the put
request handling covering the referenced range, and ensure the cap accounts for
the base64-to-decoded size relationship so oversized payloads are never
materialized in memory.
In `@apps/mcp/test/mcp.test.ts`:
- Around line 40-69: Update the authentication fakes in the test environment so
REGISTRY.get only returns the record for the test-ws key, and the
DB.prepare/first fake validates both the bound workspace and token hash. Add a
cross-workspace authentication assertion, including default, confirming lookups
do not succeed outside test-ws and that default is handled as a normal tenant.
In `@packages/uploads/test/mcp.test.ts`:
- Line 42: The fake delete client in the MCP test does not record invocations,
so dry-run and key forwarding behavior are unverified. Update the mock used by
the test setup to track delete calls, then assert in the dry-run case that no
delete call occurs and in the normal deletion case that exactly one call
forwards the key “a/b.png”; apply the same assertions to the related test at the
referenced section.
- Around line 30-37: Update the fake client's put() implementation to compute
the effective key before recording the upload, and store that key in puts
instead of opts.key. Keep the returned object and recorded entry consistent when
callers omit a key; use the existing key fallback logic in put().
In `@skills/uploads-cli/SKILL.md`:
- Around line 271-274: Update the MCP server documentation near the tool list
and config-resolution guidance to explicitly state that each MCP tool supports a
per-call workspace override, including how agents should provide the workspace
argument to target the intended tenant.
---
Nitpick comments:
In `@packages/storage/test/fake-r2.ts`:
- Around line 91-95: Remove the unnecessary `new Uint8Array(data)` allocation in
the `blob()` method and cast the existing `data` value directly to satisfy the
`Blob` constructor typing. Preserve the existing cross-consumer type-checking
behavior and returned blob contents.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f34bd0ef-ba0a-4c4b-95d3-00fe36841d96
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (34)
.github/workflows/ci.ymlAGENTS.mdapps/api/package.jsonapps/api/src/files-core.tsapps/api/src/routes/files.tsapps/api/src/workspace.tsapps/mcp/README.mdapps/mcp/package.jsonapps/mcp/src/index.tsapps/mcp/src/tools.tsapps/mcp/test/mcp.test.tsapps/mcp/tsconfig.jsonapps/mcp/wrangler.jsoncdocs/roadmap.mdpackage.jsonpackages/storage/package.jsonpackages/storage/test/fake-r2.tspackages/uploads/README.mdpackages/uploads/package.jsonpackages/uploads/scripts/check-pack.mjspackages/uploads/src/cli.tspackages/uploads/src/client.tspackages/uploads/src/commands.tspackages/uploads/src/commands/mcp.tspackages/uploads/src/io.tspackages/uploads/src/mcp/args.tspackages/uploads/src/mcp/server.tspackages/uploads/src/mcp/stdio.tspackages/uploads/src/mcp/tools.tspackages/uploads/test/commands-attach.test.tspackages/uploads/test/commands-comment.test.tspackages/uploads/test/mcp.test.tspackages/uploads/test/tsconfig.jsonskills/uploads-cli/SKILL.md
| ws: WorkspaceRecord, | ||
| opts: { prefix?: string; limit?: number; cursor?: string } = {}, | ||
| ) { | ||
| const limit = Math.min(opts.limit ?? 100, 1000); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clamp limit to a positive value.
Only the upper bound is enforced. A caller-supplied limit of 0 or negative (e.g. from routes/files.ts's Number(query) || 100, which lets negative numbers through since they're truthy) flows through unclamped to the storage backend, since this is the single shared listObjects used by REST, CLI, and MCP.
🛡️ Proposed fix
- const limit = Math.min(opts.limit ?? 100, 1000);
+ const limit = Math.min(Math.max(opts.limit ?? 100, 1), 1000);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const limit = Math.min(opts.limit ?? 100, 1000); | |
| const limit = Math.min(Math.max(opts.limit ?? 100, 1), 1000); |
🤖 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 `@apps/api/src/files-core.ts` at line 56, Clamp the limit in the shared
listObjects function to a positive minimum as well as the existing maximum,
ensuring zero or negative caller values cannot reach the storage backend. Update
the limit calculation to enforce the intended lower bound while preserving the
default and 1000-item cap.
Comment on lines
15
to
30
| .put("/:key{.+}", requireScope("files:write"), async (c) => { | ||
| const key = c.req.param("key"); | ||
| if (badKey(key)) return c.json({ error: "invalid key" }, 400); | ||
| const body = await c.req.arrayBuffer(); | ||
| if (body.byteLength === 0) return c.json({ error: "empty body" }, 400); | ||
|
|
||
| const ws = c.get("workspace"); | ||
| const contentType = c.req.header("Content-Type") ?? "application/octet-stream"; | ||
| await storage(c.env, ws).upload(key, new Uint8Array(body), { | ||
| contentType, | ||
| cacheControl: UPLOAD_CACHE_CONTROL, | ||
| }); | ||
|
|
||
| const url = publicUrl(storageConfig(c.env, ws), key); | ||
| return c.json( | ||
| { workspace: c.get("workspaceName"), key, url, size: body.byteLength, contentType }, | ||
| 201, | ||
| ); | ||
| try { | ||
| const result = await putObject( | ||
| c.env, | ||
| c.get("workspace"), | ||
| c.req.param("key"), | ||
| new Uint8Array(body), | ||
| contentType, | ||
| ); | ||
| return c.json({ workspace: c.get("workspaceName"), ...result }, 201); | ||
| } catch (err) { | ||
| return mapFileOpError(c, err); | ||
| } | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate the key before buffering the request body.
c.req.arrayBuffer() runs before putObject's internal badKey check, so an invalid key still causes the full body to be read into memory first. The key is available for free from the route param — check it first to fail fast on malformed keys without unnecessary allocation.
🛡️ Proposed fix
.put("/:key{.+}", requireScope("files:write"), async (c) => {
- const body = await c.req.arrayBuffer();
+ const key = c.req.param("key");
+ if (badKey(key)) return c.json({ error: "invalid key" }, 400);
+ const body = await c.req.arrayBuffer();
const contentType = c.req.header("Content-Type") ?? "application/octet-stream";
try {
const result = await putObject(
c.env,
c.get("workspace"),
- c.req.param("key"),
+ key,
new Uint8Array(body),
contentType,
);As per path instructions, **apps/api/src/routes/files.ts**: "Validate object keys using badKey before handling file operations."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| .put("/:key{.+}", requireScope("files:write"), async (c) => { | |
| const key = c.req.param("key"); | |
| if (badKey(key)) return c.json({ error: "invalid key" }, 400); | |
| const body = await c.req.arrayBuffer(); | |
| if (body.byteLength === 0) return c.json({ error: "empty body" }, 400); | |
| const ws = c.get("workspace"); | |
| const contentType = c.req.header("Content-Type") ?? "application/octet-stream"; | |
| await storage(c.env, ws).upload(key, new Uint8Array(body), { | |
| contentType, | |
| cacheControl: UPLOAD_CACHE_CONTROL, | |
| }); | |
| const url = publicUrl(storageConfig(c.env, ws), key); | |
| return c.json( | |
| { workspace: c.get("workspaceName"), key, url, size: body.byteLength, contentType }, | |
| 201, | |
| ); | |
| try { | |
| const result = await putObject( | |
| c.env, | |
| c.get("workspace"), | |
| c.req.param("key"), | |
| new Uint8Array(body), | |
| contentType, | |
| ); | |
| return c.json({ workspace: c.get("workspaceName"), ...result }, 201); | |
| } catch (err) { | |
| return mapFileOpError(c, err); | |
| } | |
| }) | |
| .put("/:key{.+}", requireScope("files:write"), async (c) => { | |
| const key = c.req.param("key"); | |
| if (badKey(key)) return c.json({ error: "invalid key" }, 400); | |
| const body = await c.req.arrayBuffer(); | |
| const contentType = c.req.header("Content-Type") ?? "application/octet-stream"; | |
| try { | |
| const result = await putObject( | |
| c.env, | |
| c.get("workspace"), | |
| key, | |
| new Uint8Array(body), | |
| contentType, | |
| ); | |
| return c.json({ workspace: c.get("workspaceName"), ...result }, 201); | |
| } catch (err) { | |
| return mapFileOpError(c, err); | |
| } | |
| }) |
🤖 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 `@apps/api/src/routes/files.ts` around lines 15 - 30, Validate the route key
with badKey immediately after entering the PUT handler and before calling
c.req.arrayBuffer(), returning the mapped file-operation error for invalid keys.
Then proceed with buffering and putObject using the validated key, preserving
the existing success and error handling.
Source: Path instructions
Comment on lines
+20
to
+28
| function decodeBase64(value: string): Uint8Array { | ||
| let binary: string; | ||
| try { | ||
| binary = atob(value); | ||
| } catch { | ||
| usage("contentBase64 must be valid base64"); | ||
| } | ||
| return Uint8Array.from(binary, (ch) => ch.charCodeAt(0)); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
No size cap on contentBase64 before buffering into memory.
decodeBase64 fully materializes the decoded payload (atob + Uint8Array.from) with no upper bound on contentBase64 length. Workers isolates are capped at 128 MB of memory, and Cloudflare's own guidance is to avoid buffering large payloads and stream instead, since "Stream request and response bodies — Use TransformStream or node:stream instead of buffering entire payloads in memory" and "Avoid large in-memory objects — Store large data in KV, R2, or D1 instead of holding it in Worker memory." A large put call (base64 + decoded bytes + JSON-RPC envelope all resident at once) risks hitting the 128 MB isolate limit and getting the whole isolate killed for concurrent requests, not just the one call.
🛡️ Proposed fix: cap payload size before decoding
+const MAX_CONTENT_BASE64_LENGTH = 20_000_000; // ~15MB decoded, tune as needed
+
function decodeBase64(value: string): Uint8Array {
+ if (value.length > MAX_CONTENT_BASE64_LENGTH) {
+ usage(`contentBase64 too large (max ${MAX_CONTENT_BASE64_LENGTH} chars)`);
+ }
let binary: string;
try {
binary = atob(value);
} catch {
usage("contentBase64 must be valid base64");
}
return Uint8Array.from(binary, (ch) => ch.charCodeAt(0));
}Also applies to: 84-124
🤖 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 `@apps/mcp/src/tools.ts` around lines 20 - 28, Enforce a maximum allowed
contentBase64 length before calling atob in decodeBase64, using the existing
usage error mechanism to reject oversized payloads with a clear message. Apply
the same validation to the put request handling covering the referenced range,
and ensure the cap accounts for the base64-to-decoded size relationship so
oversized payloads are never materialized in memory.
Comment on lines
+40
to
+69
| const env = { | ||
| REGISTRY: { | ||
| get: async () => record, | ||
| put: async () => undefined, | ||
| }, | ||
| DB: { | ||
| prepare: () => { | ||
| let values: unknown[] = []; | ||
| return { | ||
| bind(...next: unknown[]) { | ||
| values = next; | ||
| return this; | ||
| }, | ||
| async first() { | ||
| const [, hash] = values as string[]; | ||
| const token = options.d1; | ||
| if (token && token.tokenHash === hash) { | ||
| return { | ||
| id: "token-id", | ||
| workspace: "test-ws", | ||
| token_hash: token.tokenHash, | ||
| label: null, | ||
| scopes: token.scopes, | ||
| created_at: "2026-07-10T00:00:00.000Z", | ||
| expires_at: null, | ||
| revoked_at: null, | ||
| }; | ||
| } | ||
| return null; | ||
| }, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Make the authentication fakes enforce workspace identity.
REGISTRY.get accepts any key, while the D1 fake ignores the workspace binding and matches only the token hash. The suite could therefore pass if the worker looks up the wrong workspace, omits workspace filtering, or treats default specially. Restrict both fakes to test-ws and add a cross-workspace assertion.
As per path instructions, every workspace—including default—must be treated as a registered tenant. Based on learnings, workspace isolation must not be bypassed or special-cased.
Also applies to: 200-221
🤖 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 `@apps/mcp/test/mcp.test.ts` around lines 40 - 69, Update the authentication
fakes in the test environment so REGISTRY.get only returns the record for the
test-ws key, and the DB.prepare/first fake validates both the bound workspace
and token hash. Add a cross-workspace authentication assertion, including
default, confirming lookups do not succeed outside test-ws and that default is
handled as a normal tenant.
Sources: Coding guidelines, Learnings
Comment on lines
+30
to
+37
| puts.push({ key: opts.key, filename: opts.filename, contentType: opts.contentType }); | ||
| const key = opts.key ?? "generated/key.png"; | ||
| return { | ||
| workspace: config.workspace, | ||
| key, | ||
| url: `https://x.test/${key}`, | ||
| size: body.length, | ||
| contentType: opts.contentType ?? "image/png", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Store the effective generated key in the fake client.
When no key is supplied, put() returns generated/key.png, but puts records undefined. Subsequent list or listAll calls can therefore observe an impossible object.
Proposed fake-client fix
) => {
- puts.push({ key: opts.key, filename: opts.filename, contentType: opts.contentType });
const key = opts.key ?? "generated/key.png";
+ puts.push({ key, filename: opts.filename, contentType: opts.contentType });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| puts.push({ key: opts.key, filename: opts.filename, contentType: opts.contentType }); | |
| const key = opts.key ?? "generated/key.png"; | |
| return { | |
| workspace: config.workspace, | |
| key, | |
| url: `https://x.test/${key}`, | |
| size: body.length, | |
| contentType: opts.contentType ?? "image/png", | |
| const key = opts.key ?? "generated/key.png"; | |
| puts.push({ key, filename: opts.filename, contentType: opts.contentType }); | |
| return { | |
| workspace: config.workspace, | |
| key, | |
| url: `https://x.test/${key}`, | |
| size: body.length, | |
| contentType: opts.contentType ?? "image/png", |
🤖 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 `@packages/uploads/test/mcp.test.ts` around lines 30 - 37, Update the fake
client's put() implementation to compute the effective key before recording the
upload, and store that key in puts instead of opts.key. Keep the returned object
and recorded entry consistent when callers omit a key; use the existing key
fallback logic in put().
| }, | ||
| list, | ||
| listAll: async (opts: { prefix?: string } = {}) => (await list(opts)).items, | ||
| delete: async (key: string) => ({ key, deleted: true }), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the delete test observe client calls.
The fake delete method always returns success and records no invocation, so the test passes even if dryRun calls the backend or the wrong key is forwarded. Track delete calls and assert that dry-run performs none while the normal deletion forwards a/b.png.
Also applies to: 321-330
🤖 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 `@packages/uploads/test/mcp.test.ts` at line 42, The fake delete client in the
MCP test does not record invocations, so dry-run and key forwarding behavior are
unverified. Update the mock used by the test setup to track delete calls, then
assert in the dry-run case that no delete call occurs and in the normal deletion
case that exactly one call forwards the key “a/b.png”; apply the same assertions
to the related test at the referenced section.
Comment on lines
+271
to
+274
| - **MCP server:** the CLI can also be exposed to agents as a local stdio MCP server — | ||
| `uploads mcp` — with tools mirroring the commands described here (`put`, `attach`, | ||
| `list`, `delete`, `comment`, `health`, `doctor`) under the same config resolution. | ||
| E.g. `claude mcp add uploads -- uploads --env-file /path/to/.env mcp`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the MCP tool-level workspace override.
The README specifies a per-call workspace argument, but this skill only documents config resolution. Agents using the skill may otherwise send operations to the wrong tenant when multiple workspaces are configured.
🤖 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 `@skills/uploads-cli/SKILL.md` around lines 271 - 274, Update the MCP server
documentation near the tool list and config-resolution guidance to explicitly
state that each MCP tool supports a per-call workspace override, including how
agents should provide the workspace argument to target the intended tenant.
Main's #18 added size caps, magic-byte content sniffing, and a per-workspace write rate limit to the REST routes — the same routes this branch extracted into files-core. Resolution moves enforcement into the shared core so REST and the MCP worker apply one policy: - putObject now runs resolveUploadPolicy + inspectUpload (stored content type sniffed from bytes, never caller-supplied) and FileOpError carries the REST status/body (400/413/415) - routes/files.ts stays a thin adapter: pre-buffer Content-Length check and badKey fail-fast, writeRateLimit middleware on PUT/DELETE - guards.ts gains allowWrite() so the MCP worker's put/delete tools enforce the same write budget; apps/mcp gets its own WRITE_LIMITER binding (60/min per workspace) - MCP put drops the contentType argument (sniffed server-side) and rejects oversized contentBase64 before decoding Also applies CodeRabbit review fixes: positive lower clamp on list limit, key validation before body buffering, workspace-strict auth fakes with a cross-workspace 401 test, delete/put call tracking in the stdio test fakes, fake-r2 blob simplification, and docs for the per-call workspace override. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rw7dmzhGRndB3mBhTVKK8D
This was referenced Jul 10, 2026
Merged
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
2 participants
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.
Adds MCP (Model Context Protocol) support in both forms from the roadmap, plus a simplification pass consolidating shared logic across the CLI, REST API, and MCP surfaces.
Local:
uploads mcp(stdio)New CLI subcommand serving MCP over stdio (newline-delimited JSON-RPC 2.0), hand-rolled with zero new runtime dependencies like the rest of the CLI:
packages/uploads/src/mcp/server.ts— transport/runtime-agnostic protocol core (initialize version negotiation across 2024-11-05…2025-06-18,ping,tools/list,tools/call; tool failures becomeisErrorresults, never JSON-RPC errors). Published as the@buildinternet/uploads/mcpsubpath.src/mcp/tools.ts— seven tools mirroring the CLI commands one-to-one:put,attach,list,delete,comment,health,doctor, with the same config resolution (flags → env →--env-file→ user config), defaults, and mutual-exclusion rules. A per-callworkspaceargument mirrors--workspace. The server starts without credentials; token errors surface per tool call.claude mcp add uploads -- uploads --env-file /path/.env mcp.setup,login,admin,config) are deliberately not exposed as tools.Remote:
uploads-mcpworker (mcp.uploads.sh)Standalone Cloudflare Worker (
apps/mcp) serving stateless MCP Streamable HTTP atPOST /:workspace/mcp, authenticated per workspace exactly like REST — it shares uploads-api's REGISTRY KV / D1 / R2 bindings and reuses itsworkspaceAuth+ storage modules via TS-source workspace exports (the@uploads/storagepattern). The JSON-RPC core is the same@buildinternet/uploads/mcpmodule as the stdio server.Tools are the roadmap's remote surface:
put(base64 upload → public URL + GitHub-ready markdown),list,delete,health, withfiles:write/read/deletescope enforcement from the token. gh-comment/filesystem tools stay CLI-only. Register withclaude mcp add --transport http uploads https://mcp.uploads.sh/<ws>/mcp --header "Authorization: Bearer <token>".pnpm run deploy:mcpis wired up.Simplification pass
A 4-angle review (reuse / simplification / efficiency / altitude) was applied on top:
apps/api/src/files-core.ts: put/list/delete object ops extracted from the REST route bodies — REST and the MCP worker now share one code path (incl.badKey+ cache-control), so future key-governance rules land oncefake-r2test double,makeGhTarget,client.listAll, io helpers, schema-prop factory; doctor report carries its mismatch warning instead of recomputing itDeliberately deferred: extracting a full
packages/workspaceshared package (right long-term seam once a third consumer appears), and a shared CLI↔MCP put orchestrator (judged indirection without real dedup).Testing
wrangler deploy --dry-runbundle all greenpackages/uploads/test/tsconfig.json, now covered by the package typecheck)🤖 Generated with Claude Code
https://claude.ai/code/session_01Rw7dmzhGRndB3mBhTVKK8D
Generated by Claude Code
Summary by CodeRabbit
New Features
uploads mcp, providing local MCP access to upload, attach, list, delete, comment, health, and diagnostic tools.mcp.uploads.shwith workspace authentication and scoped permissions.Documentation
Tests