fix(packages): delete unpublished blobs when package publish fails

[SebTardif]

What Problem This Solves

Fixes an issue where users publishing a package through authenticated POST /api/v1/packages with multipart/form-data would leave Convex storage blobs behind when publish failed. The request stored the ClawPack tarball and extracted files, or the loose uploaded files, then validated name, version, account age, and owner. A 400 from that validation, or a later extracted-file store failure, never deleted those ids. The blobs were not attached to a package release, so they stayed billed and unreferenced.

Why This Change Was Made

Multipart package publish now follows the same cleanup contract as skill-scan upload. File and artifact storage ids are tracked. If parse fails after any store, or if publish throws after parse succeeds, those ids are deleted with Promise.allSettled and storage.delete. A successful publish keeps the blobs. The existing HTTP 400 mapping is unchanged; cleanup runs first, then the same error response is returned.

User Impact

Failed CLI or API package publishes no longer leave orphaned upload blobs in Convex storage. Successful publishes are unchanged.

Evidence

Live bun on Windows, worktree C:/Users/sebta/.grok/tmp/pr-gate-batch/clawhub-f007. The script calls production publishPackageV1Handler with an in-memory storage stand-in. The old loop stores two loose files and never deletes them. The patched handler stores the same two ids and deletes both before returning 400. A ClawPack path that stores the tarball then fails on the first extracted-file store deletes the tarball id.

$ bun /tmp/clawhub-f007-proof.mjs
before_error=GitHub account is too new to publish
before_stored=storage:before-1,storage:before-2 before_deleted=-
after_status=400 after_error=GitHub account is too new to publish
after_stored=storage:after-1,storage:after-2 after_deleted=storage:after-1,storage:after-2
store_count=2 delete_count=2
clawpack_status=400 clawpack_error=extracted store failed
clawpack_stored=storage:tarball clawpack_deleted=storage:tarball

The unpatched path leaves storage:before-1,storage:before-2 with before_deleted=-. The patched path prints after_deleted=storage:after-1,storage:after-2 and clawpack_deleted=storage:tarball.

Real behavior proof

• Behavior or issue addressed: Failed multipart package publish now deletes Convex blobs that were stored before the request returned 400.
• Real environment tested: Windows, bun 1.4.1, worktree C:/Users/sebta/.grok/tmp/pr-gate-batch/clawhub-f007 on fix/package-publish-blob-cleanup. Proof imported production publishPackageV1Handler from convex/httpApiV1/packagesV1.ts.
• Exact steps or command run after this patch: Ran bun /tmp/clawhub-f007-proof.mjs. The script posted a multipart package publish with two loose files, first through the old store-then-throw loop, then through production publishPackageV1Handler. It also posted a ClawPack where the tarball store succeeded and the next store failed.
• Evidence after fix: terminal output copied above. After the patch, after_stored=storage:after-1,storage:after-2 and after_deleted=storage:after-1,storage:after-2. The ClawPack path stored storage:tarball and deleted storage:tarball. Before the patch, before_stored=storage:before-1,storage:before-2 and before_deleted=-.
• Observed result after fix: An invalid multipart package publish now removes the blobs it stored. The old loop left those blobs unreferenced. A ClawPack whose extracted-file store fails now removes the tarball blob.
• What was not tested: A live clawhub.ai POST with a real API token. Successful publish retention of stored blobs on the hosted backend (local handler path keeps them when publish returns 200).

Notes

Same cleanup contract as skill multipart in #3549 and skill scan uploads in #2479. Multipart package store-without-cleanup landed in #2414 (2026-06-02, 95 days). The handler catch without delete dates to #1093 (2026-03-20, 169 days).

Allow edits from maintainers is enabled.

Tracker

Ref #3673

That issue stays open if this PR is closed without landing on main.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[vercel]

@SebTardif is attempting to deploy a commit to the OpenClaw Foundation Team on Vercel.

A member of the Team first needs to authorize it.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed September 11, 2026, 10:08 AM ET / 14:08 UTC (Revision 3).

ClawSweeper review

What this changes

Deletes uploaded package files and tarballs after multipart storage or publication failures, and adds three handler regression tests.

Regression provenance

Possible regression — suspected (reviewed change). No predecessor PR is attributed.

Merge readiness

⛔ Blocked before merge - 11 items remain

The cleanup remains necessary on current main, but both prior blocking findings remain: the patch can delete reused staged tarballs and blobs already owned by a release. The submitted trace uses in-memory storage and does not establish safe Convex deletion.

Priority: P2
Reviewed head: 700a723704de15785101e248619f7a86b7140c0c

Review scores

Measure	Result	What it means
Overall readiness	🧂 unranked krab (1/6)	Useful leak prevention is blocked by two unchanged artifact-deletion defects and mocked storage proof.
Proof confidence	🦪 silver shellfish (2/6)	Needs real behavior proof before merge: Authority-chain proof required: The Windows/Bun trace exercises the production handler with in-memory storage, proving cleanup calls only. Real local/dev Convex evidence must show failed request-local uploads are deleted while reused or committed artifacts survive retries, post-commit failures, and a former organization publisher's rejected request. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🧂 unranked krab (1/6)	Security review found an item that needs attention.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: Authority-chain proof required: The Windows/Bun trace exercises the production handler with in-memory storage, proving cleanup calls only. Real local/dev Convex evidence must show failed request-local uploads are deleted while reused or committed artifacts survive retries, post-commit failures, and a former organization publisher's rejected request. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	9 items	Pinned patch and continuity: The complete host-provided merge-base delta changes two files. Live REST metadata confirms this PR remains open at the previously reviewed head, so neither prior finding has been repaired. The stale test merge was not used to infer removals from main. Current main still lacks handler cleanup: Current main parses and stores the multipart payload, calls publication, and maps exceptions to responses without deleting request-local blobs. No merged fixing PR was established. Inspection of the v0.23.3 source failed because its required blob was unavailable; no shipped-fix claim is made. Staged artifacts are reusable: The staged path passes an existing tarball ID into the same builder used for newly stored tarballs. Ticket consumption explicitly permits the same user to reuse the same storage ID; convex/uploads.test.ts:201 tests that contract. Both new cleanup sites can therefore delete an artifact belonging to an earlier publication.
Findings	2 actionable findings	[P1] Exclude reused staged tarballs from request-local cleanup [P1] Preserve blobs after publication has committed
Security	Needs attention	Reusable upload tickets can authorize destructive cleanup: Within ticket expiry, a former organization publisher can reuse their own staged tarball ID; the publication membership check then rejects them, but the HTTP catch deletes the organization's retained artifact without checking its current owner.

How this fits together

ClawHub’s package publishing endpoint accepts loose files or a packaged tarball, stores them in Convex, and passes their storage IDs to the publication service. Releases and pending publication attempts then retain those blobs for scanning and downloads.

flowchart TD
  A[Authenticated package upload] --> B[Direct files or staged tarball]
  B --> C[Parse and store files]
  C --> D[Validate and publish]
  D --> E[Release owns blobs]
  C --> F[Failure cleanup]
  D --> F
  F --> G[Delete selected storage IDs]

Loading

Before merge

• Add real behavior proof - Needs real behavior proof before merge: Authority-chain proof required: The Windows/Bun trace exercises the production handler with in-memory storage, proving cleanup calls only. Real local/dev Convex evidence must show failed request-local uploads are deleted while reused or committed artifacts survive retries, post-commit failures, and a former organization publisher's rejected request. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Exclude reused staged tarballs from request-local cleanup (P1) - The staged upload path passes an existing storage ID into this builder, and consumePackagePublishUploadTicketInternal permits reuse of the same ticket and ID. If extracted-file storage fails, this catch deletes that existing tarball; the handler catch does likewise after publication rejection. The tarball may already belong to a pending or published release. A former organization publisher can also replay their unexpired user-bound ticket and trigger deletion when publication rejects their removed membership. Carry request-local ownership explicitly and exclude reused artifacts from both cleanup sites.
• Preserve blobs after publication has committed (P1) - publishPackageImpl can throw after insertReleaseInternal commits: inspector-warning insertion, token revocation, and audit writes are awaited afterward. The staged path also revokes a token after creating the release and attempt. Such errors reach this catch and delete the files and tarball still referenced by those rows. Parse success does not establish that ownership never transferred. Restrict cleanup to a proven pre-adoption failure or perform it within the publication lifecycle owner.
• Resolve security concern: Reusable upload tickets can authorize destructive cleanup - Within ticket expiry, a former organization publisher can reuse their own staged tarball ID; the publication membership check then rejects them, but the HTTP catch deletes the organization's retained artifact without checking its current owner.
• Resolve merge risk (P1) - Retrying an existing staged upload can destroy a retained or published tarball, including after its original uploader loses organization publish access.
• Resolve merge risk (P1) - A post-commit publication error can leave a release or pending attempt pointing to deleted files and archives.
• Resolve merge risk (P1) - The supplied in-memory trace does not establish real storage cleanup, successful-publication retention, or rejection of stale deletion authority.
• Complete next step (P2) - Fix both ownership findings and provide real Convex cleanup and retention proof, including rejection after organization membership removal. Redact credentials, private endpoints, IP addresses, and other private data. Update the PR body to trigger re-review; if it does not run, ask a maintainer to comment @clawsweeper re-review.
• Improve patch quality - Separate newly stored request blobs from reused staged artifacts and preserve adopted blobs after post-commit errors.
• Improve patch quality - Add regression coverage for staged replay, post-commit failure, and removed organization membership.
• Improve patch quality - Provide redacted real Convex evidence of allowed cleanup and forbidden deletion being rejected before storage effects.

Findings

• [P1] Exclude reused staged tarballs from request-local cleanup — convex/httpApiV1/packagesV1.ts:1530-1533
• [P1] Preserve blobs after publication has committed — convex/httpApiV1/packagesV1.ts:2599-2602
• [high] Reusable upload tickets can authorize destructive cleanup — convex/httpApiV1/packagesV1.ts:2601

Agent review details

Security

Needs attention: The new deletion path treats an old upload capability as authority to delete a release artifact after current publishing permission is denied.

Review metrics

Metric	Value	Why it matters
Diff size	+220/-18 across 2 files	The patch is bounded to package HTTP handling and its tests.
Production versus test growth	production +61 net, tests +141 net	Production growth implements failure cleanup, supported by three mocked handler cases.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #3673
Summary: This PR is the proposed implementation for the multipart upload tracker; legacy ZIP cleanup and retained-publication recovery have different ownership boundaries.

Members:

• canonical: Multipart package publish leaves Convex blobs after a failed publish #3673 - Its acceptance criteria explicitly describe this patch's intended cleanup and retention behavior.
• adjacent_distinct: fix(packages): reclaim unadopted legacy ZIPs #3622 - Cleans a generated legacy ZIP inside publication rather than HTTP multipart uploads.
• adjacent_distinct: fix: delete unpublished skill blobs when publish fails #3549 - Addresses skill multipart uploads through a separate publication surface.
• adjacent_distinct: fix(publish): recover failed staged release artifacts #3616 - Merged recovery work preserves retained publication artifacts rather than collecting request-local uploads.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

1. Make cleanup follow blob ownership (recommended)
   Exclude reused staged artifacts and prevent handler cleanup after release adoption, with regression coverage for retries and post-commit errors.
2. Limit the initial change to parsing failures
   Retain cleanup for newly created parse-time blobs and defer action-failure cleanup until publication exposes a reliable ownership boundary.

Technical review

Best possible solution:

Track request-created blobs separately from staged artifacts, and transfer cleanup responsibility to the publication owner once a release adopts them.

Do we have a high-confidence way to reproduce the issue?

Yes, source establishes the failure paths: replay a previously used staged ticket into a rejected publish, or throw after release insertion. These paths were not executed during this read-only review.

Is this the best way to solve the issue?

No. Waiting for concurrent stores to settle is appropriate, but the outer catch cannot infer that blobs are unpublished; ownership-aware cleanup is safer than deleting every payload ID.

Full review comments:

• [P1] Exclude reused staged tarballs from request-local cleanup — convex/httpApiV1/packagesV1.ts:1530-1533
  The staged upload path passes an existing storage ID into this builder, and consumePackagePublishUploadTicketInternal permits reuse of the same ticket and ID. If extracted-file storage fails, this catch deletes that existing tarball; the handler catch does likewise after publication rejection. The tarball may already belong to a pending or published release. A former organization publisher can also replay their unexpired user-bound ticket and trigger deletion when publication rejects their removed membership. Carry request-local ownership explicitly and exclude reused artifacts from both cleanup sites.
  Confidence: 0.99
• [P1] Preserve blobs after publication has committed — convex/httpApiV1/packagesV1.ts:2599-2602
publishPackageImpl can throw after insertReleaseInternal commits: inspector-warning insertion, token revocation, and audit writes are awaited afterward. The staged path also revokes a token after creating the release and attempt. Such errors reach this catch and delete the files and tarball still referenced by those rows. Parse success does not establish that ownership never transferred. Restrict cleanup to a proven pre-adoption failure or perform it within the publication lifecycle owner.
  Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against cbfee7343ddc.

Labels

Label justifications:

• P2: The underlying orphan-storage problem is a bounded package publishing defect.
• merge-risk: 🚨 compatibility: The introduced deletion can break existing staged retries and remove artifacts already attached to releases.
• merge-risk: 🚨 security-boundary: A reusable upload ticket can reach artifact deletion after current organization publish authorization is denied.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Authority-chain proof required: The Windows/Bun trace exercises the production handler with in-memory storage, proving cleanup calls only. Real local/dev Convex evidence must show failed request-local uploads are deleted while reused or committed artifacts survive retries, post-commit failures, and a former organization publisher's rejected request. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

Security concerns:

• [high] Reusable upload tickets can authorize destructive cleanup — convex/httpApiV1/packagesV1.ts:2601
  Within ticket expiry, a former organization publisher can reuse their own staged tarball ID; the publication membership check then rejects them, but the HTTP catch deletes the organization's retained artifact without checking its current owner.
  Confidence: 0.98

What I checked:

• Pinned patch and continuity: The complete host-provided merge-base delta changes two files. Live REST metadata confirms this PR remains open at the previously reviewed head, so neither prior finding has been repaired. The stale test merge was not used to infer removals from main. (convex/httpApiV1/packagesV1.ts:2598, 700a723704de)
• Current main still lacks handler cleanup: Current main parses and stores the multipart payload, calls publication, and maps exceptions to responses without deleting request-local blobs. No merged fixing PR was established. Inspection of the v0.23.3 source failed because its required blob was unavailable; no shipped-fix claim is made. (convex/httpApiV1/packagesV1.ts:2638, cbfee7343ddc)
• Staged artifacts are reusable: The staged path passes an existing tarball ID into the same builder used for newly stored tarballs. Ticket consumption explicitly permits the same user to reuse the same storage ID; convex/uploads.test.ts:201 tests that contract. Both new cleanup sites can therefore delete an artifact belonging to an earlier publication. (convex/uploads.ts:133, 700a723704de)
• Publication can fail after ownership transfers: The publication action commits insertReleaseInternal before awaiting inspector-warning insertion, token revocation, and audit operations. Pending publication also commits a release and attempt before token revocation. An exception in a later operation reaches the new unconditional handler cleanup despite existing release ownership. (convex/packages.ts:9281, 700a723704de)
• Revoked publisher boundary: Organization membership is checked during publication, after the HTTP parser accepts a reusable user-bound ticket. A former organization publisher can therefore replay their unexpired ticket, fail the membership check, and reach deletion of the organization's retained tarball. Existing pending-release cleanup instead checks release identity and pending status before deletion. (convex/publishers.ts:2241, 700a723704de)
• Proof and applicable runtime policy: The complete captured PR body describes a Windows/Bun invocation of the production handler with an in-memory storage stand-in. Its output demonstrates cleanup calls, not actual Convex storage effects or committed-artifact retention. The repository's Convex skill explicitly requires real Convex validation for storage and HTTP-action behavior. Captured context sourceRevision: c075fe74fdd2e2649269f7a86e13743d9a2f2d89fc20c9b71d6ab1b968dbbb3d. (.agents/skills/clawhub-convex/SKILL.md, 700a723704de)

Likely related people:

• unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)
• jesse-merhi: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (2 earlier review cycles)

• reviewed 2026-09-05T12:31:08.637Z sha cda1046 :: needs real behavior proof before merge. :: [P1] Exclude reused staged tarballs from request-local cleanup | [P1] Preserve blobs after publication has committed
• reviewed 2026-09-05T12:43:10.574Z sha 700a723 :: needs real behavior proof before merge. :: [P1] Exclude reused staged tarballs from request-local cleanup | [P1] Preserve blobs after publication has committed

[SebTardif]

Closing this PR to free a barnacle slot for a higher-severity GitHub import unzip-limit fix (issue #3678).

ClawSweeper left this change unranked with open deletion-safety P1s, so it is unlikely to merge as written.

The leak itself stays tracked on issue #3673. That issue stays open. A later PR can pick it up after the P1s are addressed.