fix: delete unpublished skill blobs when publish fails

[SebTardif]

Failed multipart skill publishes now delete request-owned uploads, including partial stores and failures before publication. Once a pending or published version commits, the publishing helper transfers ownership immediately, preserving its files through later scheduler or compensation failures. This completes Sebastien Tardif's existing parser cleanup and fixes the post-commit deletion finding.

Fixes #3670.

Native local Convex proof: invalid metadata, license refusal and rejected insertion went from two orphan blobs to zero; partial storage failure went from one to zero. The previous PR deleted both files after a post-commit scheduler failure (download 404); the repaired version retained both files (download 200, 989 and 43 bytes). Pending success, successful compensation, failed compensation and normal publication were also exercised using actual storage/version mutations and explicit fault injection. No production data or hosted deployment was used.

Validation: 522 focused tests; 6,972 full-suite tests passed; static, full types/build, explicit schema/CLI/Convex typechecks passed; structured repair review returned no findings.

[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 16, 2026, 1:22 AM ET / 05:22 UTC (Revision 76).

ClawSweeper review

What this changes

Deletes unclaimed multipart skill uploads after failures while preserving files once a pending or published version owns them, with regression tests and a documented ownership invariant.

Merge readiness

⛔ Blocked before merge - 2 items remain

This PR remains necessary: main and the latest release still leak failed multipart uploads. The previous persistence-boundary finding and spec omission are resolved, with no remaining blocking code findings; inspectable runtime evidence remains the merge gate.

Priority: P2
Reviewed head: 9d2945bfeadf8a2c60568d3634fdf7a8100e7118

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The focused patch resolves the prior defect and adds useful coverage; the detailed native-run report still needs inspectable supporting output.
Proof confidence	🦐 gold shrimp (3/6)	Needs stronger real behavior proof before merge: Needs real behavior proof before merge: the body reports relevant native Convex cleanup and retained-download results for the parser, handler, and publication helper, but provides no inspectable transcript or artifact connecting those observations to the exercised entrypoint and fault injection. 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	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: Needs real behavior proof before merge: the body reports relevant native Convex cleanup and retained-download results for the parser, handler, and publication helper, but provides no inspectable transcript or artifact connecting those observations to the exercised entrypoint and fault injection. 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	Repository policy and scope: Read the full root AGENTS.md, ClawHub maintainer and Convex skills, and generated Convex guidelines. No nested AGENTS.md applies under convex or specs; no maintainer-notes directory exists. The checkout matches the pinned head and remains clean. Current main still needs the repair: The main parser stores each file before payload validation without a cleanup boundary. The multipart handler likewise has no cleanup for subsequent publication rejection. Latest release retains the leak: The v0.23.3 source also stores files before validating the publish body without deleting them on failure. The GitHub contents endpoint supplied the release source after local historical blob retrieval failed.
Findings	None	None.
Security	None	None.

How this fits together

ClawHub’s publishing API stores uploaded skill files in Convex and attaches them to a version. Scanners and downloads subsequently read those files, so cleanup must stop when the version commits.

flowchart TD
  A[Authenticated multipart upload] --> B[Check sizes and store files]
  B --> C[Validate publication]
  C --> D[Commit skill version]
  B -->|Failure| E[Delete request uploads]
  C -->|Failure before commit| E
  D --> F[Retain version files]
  F --> G[Scans and downloads]
  D --> H[Pending publication compensation]

Loading

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: Needs real behavior proof before merge: the body reports relevant native Convex cleanup and retained-download results for the parser, handler, and publication helper, but provides no inspectable transcript or artifact connecting those observations to the exercised entrypoint and fault injection. 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.
• Complete next step (P2) - Attach the existing native Convex run’s command and redacted output showing cleanup before persistence and retention after commit. Logs, copied terminal output, terminal screenshots, or recordings count; redact credentials, private endpoints, IP addresses, and other private details. Update the PR body to trigger re-review; if it does not, ask a maintainer to comment @clawsweeper re-review.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Diff size	+336/-57 across 7 files	The branch stays within multipart cleanup, publication ownership, tests, and the subsystem spec.
Production and test delta	Production +75/-40; tests +252/-17; specs +9/-0	Production growth supports the explicit ownership boundary, with most added lines devoted to regression coverage.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #3670
Summary: This PR implements the multipart skill-upload tracker; GitHub imports and package publishing have separate storage entrypoints.

Members:

• canonical: Multipart skill publish leaves Convex blobs after a failed publish #3670 - The linked open tracker specifies both failed-upload cleanup and committed-file retention.
• adjacent_distinct: fix(import): delete GitHub skill blobs when import fails #3612 - The open branch addresses GitHub import uploads, outside this multipart handler.
• adjacent_distinct: fix(packages): delete unpublished blobs when package publish fails #3613 - The closed-unmerged branch addresses package files and artifacts and is not a replacement landing path.

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

Technical review

Best possible solution:

Keep cleanup tied to request ownership until version persistence, with committed-file retention demonstrated by the existing native Convex run’s published evidence.

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

Yes, source establishes a focused path: upload a multipart file with invalid publish metadata, then observe validation fail after storage without cleanup on main. This review did not execute that path.

Is this the best way to solve the issue?

Yes, this is the best code-level repair: transfer cleanup ownership at persistence, not helper completion. Earlier metadata validation alone misses later failures, and direct-upload ticket expiry does not cover multipart uploads.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• remove rating: 🦪 silver shellfish: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.
• remove merge-risk: 🚨 other: Current PR review selected no merge-risk labels.

Label justifications:

• P2: This repairs a bounded authenticated-publishing storage leak without evidence of an urgent production incident.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Needs real behavior proof before merge: the body reports relevant native Convex cleanup and retained-download results for the parser, handler, and publication helper, but provides no inspectable transcript or artifact connecting those observations to the exercised entrypoint and fault injection. 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

What I checked:

• Repository policy and scope: Read the full root AGENTS.md, ClawHub maintainer and Convex skills, and generated Convex guidelines. No nested AGENTS.md applies under convex or specs; no maintainer-notes directory exists. The checkout matches the pinned head and remains clean. (AGENTS.md:1, 9d2945bfeadf)
• Current main still needs the repair: The main parser stores each file before payload validation without a cleanup boundary. The multipart handler likewise has no cleanup for subsequent publication rejection. (convex/httpApiV1/shared.ts:389, be87bfd9d3f8)
• Latest release retains the leak: The v0.23.3 source also stores files before validating the publish body without deleting them on failure. The GitHub contents endpoint supplied the release source after local historical blob retrieval failed. (convex/httpApiV1/shared.ts, 87ca030c30f3)
• Prior persistence finding resolved: Both immediate and staged publication now invoke the retention callback immediately after insertVersion returns, before scheduling or attempt creation. Failed pending-publication compensation therefore cannot reactivate request cleanup. (convex/lib/skillPublish.ts:588, 9d2945bfeadf)
• Durable ownership and compensation: insertVersion persists args.files in skillVersions. discardPendingPublicationInternal checks the version is pending and belongs to the requested skill before deleting that version and its files. The new callback leaves this existing compensation owner responsible after persistence. (convex/skills.ts:13223, 9d2945bfeadf)
• Existing alternatives and security boundary: Multipart cleanup uses only IDs returned by storage.store for this request; client-supplied payload file IDs are replaced by the parser's own list. Direct JSON uploads retain their separate upload-ticket validation and expiry cleanup. The scan parser already uses the same best-effort deletion pattern. (convex/httpApiV1/shared.ts:427, 9d2945bfeadf)

Likely related people:

• Patrick-Erichsen: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Attach the existing native Convex transcript or logs showing rejected-upload cleanup and committed-file retention, including the invoked entrypoint and fault scenarios.

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 (75 earlier review cycles; latest 8 shown)

• reviewed 2026-09-03T22:40:56.933Z sha b702bc1 :: needs real behavior proof before merge. :: [P1] Preserve blobs once version insertion commits | [P3] Record the multipart blob-ownership invariant
• reviewed 2026-09-04T04:52:30.000Z sha b702bc1 :: needs real behavior proof before merge. :: [P1] Do not delete blobs after version insertion commits | [P3] Record the multipart blob-ownership invariant
• reviewed 2026-09-04T21:01:22.732Z sha b702bc1 :: needs real behavior proof before merge. :: [P1] Do not delete blobs after a version has committed | [P3] Document the multipart blob-lifetime invariant
• reviewed 2026-09-04T23:01:52.031Z sha b702bc1 :: needs real behavior proof before merge. :: [P1] Preserve blobs after the version transaction commits
• reviewed 2026-09-05T01:59:09.645Z sha b702bc1 :: needs real behavior proof before merge. :: [P1] Preserve uploaded blobs once the version transaction commits
• reviewed 2026-09-05T03:10:32.473Z sha b702bc1 :: needs real behavior proof before merge. :: [P1] Preserve uploaded blobs once the version transaction commits
• reviewed 2026-09-07T19:16:16.416Z sha d4bcf53 :: needs real behavior proof before merge. :: [P1] Preserve uploaded blobs once the version transaction commits
• reviewed 2026-09-11T14:08:33.912Z sha d4bcf53 :: needs real behavior proof before merge. :: [P1] Preserve uploaded blobs once the version transaction commits

[SebTardif]

@clawsweeper re-review

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/34155038246
• Updated: 2026-09-07T19:18:06.486Z