feat(skills): publish large bundles via staged upload - #3036
Conversation
Fixes the edge 413 Request Entity Too Large that blocks publishing any skill whose bundle exceeds the inline multipart body limit. `clawhub publish` posts each file as an individual multipart `files` part to POST /api/v1/skills. That request traverses the Vercel edge proxy (/api/** -> server/handlers/convexProxy.ts), whose serverless body limit (~4.5MB) rejects larger bundles with a raw 413 before the request reaches Convex -- even though the app-level skill limits (10MB per file / 50MB total) are far higher. Packages already bypass this with a staged upload-url + storage-ticket flow; skills had no equivalent. Add the same staged path for skills: - POST /api/v1/skills/-/upload-url mints a one-shot storage upload URL + ticket bound to the publishing user (skillPublishUploadTickets, 15m TTL, single-use, storage blob must post-date the ticket -- mirrors the package ticket model). - The CLI zips the bundle, PUTs it straight to storage (bypassing the edge body limit), then publishes with payload + bundleStorageId + bundleUploadTicket. Bundles under ~4MB keep the inline multipart path unchanged. - The server consumes the ticket, unzips, and runs every entry through the same per-file size gate, path sanitization, and storage persistence as the inline path, so validation is identical regardless of transport. Mirrors the existing package staged-upload flow rather than introducing a new mechanism. Adds CLI unit tests (staging threshold + zip round-trip) and a server unit test for the pure bundle-extraction logic; documents the new route in the OpenAPI spec and docs/http-api.md.
|
@autogame-17 is attempting to deploy a commit to the OpenClaw Foundation Team on Vercel. A member of the Team first needs to authorize it. |
knip flagged SkillBundleFile as an unused exported type; it is only used internally by buildSkillBundleZip. Make it module-private.
|
Codex review: needs real behavior proof before merge. Reviewed July 16, 2026, 10:32 AM ET / 14:32 UTC. Summary Reproducibility: yes. from source: publish an authenticated staged ZIP whose compressed bytes are small but whose entries expand far beyond the logical bundle limit; the synchronous unzip occurs before the per-file and total-size gates. The original edge 413 is also consistent with the documented 50 MB application limit versus the smaller proxy body limit. Review metrics: 3 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Keep the package-aligned staged transport, but reject excessive declared expansion, entry counts, and total uncompressed bytes before allocating archive contents, preserve the existing inline path, and demonstrate both a successful bundle above 4 MB and bounded rejection of an oversized expansion on a real deployment. Do we have a high-confidence way to reproduce the issue? Yes, from source: publish an authenticated staged ZIP whose compressed bytes are small but whose entries expand far beyond the logical bundle limit; the synchronous unzip occurs before the per-file and total-size gates. The original edge 413 is also consistent with the documented 50 MB application limit versus the smaller proxy body limit. Is this the best way to solve the issue? No, not yet. Reusing the established ticketed storage-upload pattern is maintainable, but synchronous unbounded expansion and the absence of real deployment proof prevent this implementation from being the safe final solution. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against bf9c3be4a7b0. Label changesLabel justifications:
Evidence reviewedSecurity concerns:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (3 earlier review cycles)
|
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: feat(skills): publish large bundles via staged upload This is item 1/1 in the current shard. Shard 12/24. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
What
Publish large skill bundles via a staged upload, fixing the edge
413 Request Entity Too Largethat blocks any skill whose bundle exceeds the inline multipart body limit.clawhub publishposts each skill file as an individual multipartfilespart toPOST /api/v1/skills. That request goes through the Vercel edge (/api/**→server/handlers/convexProxy.ts), whose serverless body limit (~4.5MB) rejects larger bundles with a raw413before the request ever reaches Convex — even though the app-level skill limits (MAX_PUBLISH_FILE_BYTES10MB /MAX_PUBLISH_TOTAL_BYTES50MB) are far higher. Packages already solve this with a staged upload-url + storage-ticket flow; skills had no equivalent.This PR adds the same staged path for skills:
POST /api/v1/skills/-/upload-urlmints a one-shot storage upload URL + ticket bound to the publishing user.PUTs it straight to Convex storage (bypassing the edge body limit), then publishes with a smallmultipart/form-datarequest carryingpayload+bundleStorageId+bundleUploadTicket(no inlinefiles).The CLI switches to the staged path automatically once a bundle exceeds ~4MB of content; smaller bundles keep the existing inline multipart path byte-for-byte unchanged.
Why this shape
I mirrored the existing package staged-upload flow rather than inventing a new one:
convex/uploads.tsconsumePackagePublishUploadTicketInternal): the ticket is bound to the issuing user, has a 15-minute TTL, is single-use (usedAt), and the referenced storage blob must have been created after the ticket (anti-replay). Skills only authenticate via a user API token (requireApiTokenUserOrResponse), so the skill ticket is user-scoped only — there is nogithub-actionskind.fflatezipSync/unzipSync), so the staged bundle reuses that format instead of introducing ClawPack tarballs.filesarray shape as the inline path, so the 50MB total-bundle cap and all publish validation inskillPublishapply unchanged.Changes
convex/schema.ts— newskillPublishUploadTicketstable (+ retention policy inconvex/lib/retentionPolicy.ts).convex/uploads.ts—createSkillPublishUploadForUserInternal/consumeSkillPublishUploadTicketInternal.convex/httpApiV1/skillsV1.ts— mint handlerpublishSkillUploadUrlV1Handler; publish handler routes multipart requests to inline vs staged based on thebundle*fields.convex/httpApiV1/shared.ts— extractedbuildPublishBody+parseMultipartPublishForm; addedparseStagedBundlePublishand a pure, unit-testableextractSkillBundleEntries.convex/http.ts,convex/httpApiV1.ts— register the new route/handler.packages/clawhub/src) —skills.tsgainsbuildSkillBundleZip+shouldStageSkillBundle+ threshold;cli/commands/publish.tsuses the staged path for large bundles.skillsUploadUrl(source + builtdist/routes.*), OpenAPI (public/api/v1/openapi.json), anddocs/http-api.md.Testing
tsc --noEmiton the app, schema, and CLI packages;oxfmt --check+oxlinton changed files; CLI package tests (bun run --cwd packages/clawhub test:src) including newskills.test.tscases for the staging threshold and zip round-trip.convex/httpApiV1/skillBundle.test.tscoversextractSkillBundleEntries(round-trip, mac-junk skip, per-file size cap, invalid-zip rejection). I couldn't run the rootconvex/vitest suite locally (the jsdom worker times out on my machine), so those run in CI.Notes
Per CONTRIBUTING, larger/architectural changes usually start with a Discord conversation — happy to move discussion there if preferred. I framed this as a bug fix (large valid skills currently can't publish at all) that deliberately reuses the established package staged-upload pattern rather than introducing a new mechanism. Feedback on the wire protocol (
bundleStorageId+bundleUploadTicketmultipart fields vs. a dedicated JSON shape) very welcome.