fix: delete unpublished package zip on publish failure

[SebTardif]

What Problem This Solves

Fixes an issue where users publishing a non-npm-pack package would leave an unreferenced Convex zip in storage when the version already existed or release insert failed. Authenticated packages.publishRelease and GitHub Actions publish both hit this path when artifact.kind is not npm-pack.

The zip is stored only for the release row. A duplicate-version reject or insert failure never attached it, and discardPendingPackagePublicationInternal only deletes blobs already on a pending release.

Why This Change Was Made

Store the legacy zip immediately before insertReleaseInternal, after version and reuse checks. If insert throws, delete that new id. npm-pack publishes and caller-supplied artifact.storageId are unchanged. This is not the multipart packagesV1 cleanup in #3613.

User Impact

Failed package publishes no longer leave orphan zip blobs in Convex storage. Successful publishes still attach the zip to the new release.

Evidence

Live bun run of publishPackageForTrustedPublisherInternal against this branch.

Duplicate-version reject: no storage.store. Insert failure: store then delete of storage:legacy-zip. Leftover ids: none.

$ bun /tmp/proof-f010-legacy-zip.mts
{
  "duplicate": {
    "path": "duplicate-version",
    "events": [],
    "threw": "Version 1.0.0 already exists. Increment the version number and try again."
  },
  "insertFail": {
    "path": "insert-failure",
    "events": [
      "store:storage:legacy-zip",
      "delete:storage:legacy-zip"
    ],
    "threw": "Version 1.0.0 already exists. Increment the version number and try again."
  },
  "leftoverAfterStoreWithoutDelete": [],
  "elapsedMs": 13
}

On unfixed main the duplicate-version path stores storage:legacy-zip and never deletes it.

Real behavior proof

• Behavior or issue addressed: A failed non-npm-pack package publish no longer leaves an unreferenced Convex zip.
• Real environment tested: Windows 11, Bun 1.4.1, worktree fix/package-publish-legacy-zip-cleanup on origin/main d3bde70e, live bun calling publishPackageForTrustedPublisherInternal with a blocked attempt and with a throwing insert mutation.
• Exact steps or command run after this patch: bun /tmp/proof-f010-legacy-zip.mts from the patched worktree (proof helper not committed).
• Evidence after fix: terminal output copied below.

$ bun /tmp/proof-f010-legacy-zip.mts
{
  "duplicate": {
    "path": "duplicate-version",
    "events": [],
    "threw": "Version 1.0.0 already exists. Increment the version number and try again."
  },
  "insertFail": {
    "path": "insert-failure",
    "events": [
      "store:storage:legacy-zip",
      "delete:storage:legacy-zip"
    ],
    "threw": "Version 1.0.0 already exists. Increment the version number and try again."
  },
  "leftoverAfterStoreWithoutDelete": [],
  "elapsedMs": 13
}

• Observed result after fix: Duplicate-version reject does not store a zip. Insert failure stores storage:legacy-zip then deletes that same id. The version-exists error is rethrown.
• What was not tested: A live production Convex publish against a real existing package version.

Summary

Same cleanup class as #3612, #3613, and #3614, but in publishPackageImpl rather than multipart parse, GitHub import, or skill-card attach.

The store landed in #3090 (85a3fde608) on 2026-07-24.

Tracker

Ref #3677

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 2).

ClawSweeper review

What this changes

Defers package ZIP storage until release insertion, deletes the newly created ZIP when insertion fails, and adds two regression tests.

Merge readiness

⛔ Blocked before merge - 3 items remain

Keep open: current main and v0.23.3 retain the early ZIP write. This is a useful, focused repair with no blocking code findings, but the previous real-Convex proof requirement remains unmet.

Priority: P2
Reviewed head: d25c99fa4f403f2c0de001a08c4aa8386779b9df

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The focused implementation and regression tests are useful, but required backend behavior proof remains incomplete.
Proof confidence	🦪 silver shellfish (2/6)	Needs stronger real behavior proof before merge: The Windows/Bun trace exercises the production publish handler’s rejection branches, but does not establish execution against real Convex storage and release mutations or successful downloadable ZIP retention, as requested in the previous review. 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: The Windows/Bun trace exercises the production publish handler’s rejection branches, but does not establish execution against real Convex storage and release mutations or successful downloadable ZIP retention, as requested in the previous review. 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	8 items	Pinned patch and scope: The verified merge-base-to-head change touches two files: production +52/-32 lines and tests +179/-0. The checkout matches the original PR head; the supplied stale test merge was not used to infer merge behavior. Current main still needs the repair: Current main stores legacyZipStorageId before the staged duplicate/reuse checks. Its later changes do not introduce this PR’s deferred-storage or insertion-failure cleanup. Latest release retains early storage: Reading packages.ts at v0.23.3 through GitHub’s contents API confirmed the early ZIP allocation and fallback storage-ID assignment. No shipped fix was established.
Findings	None	None.
Security	None	None.

How this fits together

ClawHub’s package publishing backend accepts authenticated uploads and creates release records linked to downloadable artifacts. This change manages the generated ZIP between validation and release insertion.

flowchart TD
  A[Authenticated package upload] --> B[Validate package and check existing release]
  B --> C[Reject or reuse existing result]
  B --> D[Store generated ZIP if needed]
  D --> E[Insert release]
  E --> F[Retain ZIP for downloads]
  E --> G[On insertion failure delete new ZIP]

Loading

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: The Windows/Bun trace exercises the production publish handler’s rejection branches, but does not establish execution against real Convex storage and release mutations or successful downloadable ZIP retention, as requested in the previous review. 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.
• Resolve merge risk (P1) - Real Convex insertion rejection, ZIP deletion, and successful artifact retention remain unverified by the supplied evidence.
• Complete next step (P2) - Add after-fix evidence from real local or dev Convex showing duplicate rejection, cleanup after rejected insertion, and retained downloadable ZIP storage after success. Terminal output or redacted logs count; terminal screenshots or video are welcome where useful. Remove credentials, private endpoints, IP addresses, and other private details. Update the PR body to trigger re-review; if it does not run, ask a maintainer to comment @clawsweeper re-review.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test LOC	Production +52/-32 (net +20); tests +179/-0; 2 files	Production growth implements one shared storage-cleanup boundary, supported by two focused regression tests.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #3677
Summary: This PR is the proposed repair for the generated-ZIP leak tracked by the paired issue; multipart cleanup and retained-release recovery are separate paths.

Members:

• canonical: Package publish leaves a legacy ZIP after a rejected release #3677 - Tracks the same generated ZIP, rejection paths, and successful-retention requirement.
• adjacent_distinct: fix(packages): delete unpublished blobs when package publish fails #3613 - Cleans multipart-uploaded files and artifacts before or around the publish action.
• adjacent_distinct: fix(publish): recover failed staged release artifacts #3616 - Merged recovery work operates on retained releases and their already-attached artifacts.

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

Merge-risk options

Maintainer options:

1. Decide the mitigation before merge
   Keep generated-ZIP cleanup at the release-insertion boundary, preserving caller-owned artifacts and ZIPs attached to successful releases.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Keep generated-ZIP cleanup at the release-insertion boundary, preserving caller-owned artifacts and ZIPs attached to successful releases.

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

Yes, from source: current main stores a ZIP before staged duplicate rejection and has no cleanup for that unowned blob. No runtime reproduction was executed during this read-only review.

Is this the best way to solve the issue?

Yes, for the rejected-publish scope: deferring storage and wrapping insertion addresses the owning layer directly. HTTP-only cleanup would miss direct publish actions, and pending-release cleanup cannot find a ZIP never attached to a release.

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

• P2: Rejected package publications can leave unreferenced storage blobs; this is a bounded backend resource leak.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish 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: The Windows/Bun trace exercises the production publish handler’s rejection branches, but does not establish execution against real Convex storage and release mutations or successful downloadable ZIP retention, as requested in the previous review. 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:

• Pinned patch and scope: The verified merge-base-to-head change touches two files: production +52/-32 lines and tests +179/-0. The checkout matches the original PR head; the supplied stale test merge was not used to infer merge behavior. (convex/packages.ts:9013, d25c99fa4f40)
• Current main still needs the repair: Current main stores legacyZipStorageId before the staged duplicate/reuse checks. Its later changes do not introduce this PR’s deferred-storage or insertion-failure cleanup. (convex/packages.ts:9114, cbfee7343ddc)
• Latest release retains early storage: Reading packages.ts at v0.23.3 through GitHub’s contents API confirmed the early ZIP allocation and fallback storage-ID assignment. No shipped fix was established. (convex/packages.ts:8648, 87ca030c30f3)
• Storage ownership and downstream behavior: Both insertion branches use the wrapper. Cleanup receives only the newly stored ZIP ID, excluding caller-supplied artifacts. Release insertion persists clawpackStorageId; pending-release discard uses attached IDs, and downloads read the retained ZIP. Existing authentication and transactional publisher checks remain intact. (convex/packages.ts:9033, d25c99fa4f40)
• Proof and re-review continuity: The complete supplied body, captured under sourceRevision b7fb69d52e256d095c6e89ea36d7be5fcd7cb86227c54e940d20f647b5e25031, contains Windows/Bun handler traces for duplicate rejection and a throwing insertion. The uncommitted helper and traces do not establish real Convex storage or mutation execution, nor successful downloadable ZIP retention. The previous review requested these same backend observations; the reviewed head is unchanged. (d25c99fa4f40)
• Applicable runtime proof policy: Root AGENTS.md was read fully; no nested AGENTS.md applies to these files and no maintainer-notes directory exists. The ClawHub Convex skill specifically requires a real Convex path for storage and action-to-mutation behavior, which this patch directly exercises. (.agents/skills/clawhub-convex/SKILL.md:48, d25c99fa4f40)

Likely related people:

• Patrick-Erichsen: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• giodl73-repo: 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.

• Provide real local or dev Convex evidence showing duplicate rejection, deletion after rejected insertion, and a retained downloadable ZIP after successful insertion.

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 (1 earlier review cycle)

• reviewed 2026-09-07T22:13:34.063Z sha d25c99f :: needs real behavior proof before merge. :: none