fix(packages): reclaim unadopted legacy ZIPs - #3622
Patrick-Erichsen merged 3 commits into
Conversation
Non-npm-pack publishes stored a legacy zip before version and reuse checks. A duplicate version or insert failure left the new Convex blob unreferenced. Store the zip only when insert is about to own it, and delete it if insert fails. Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
@SebTardif is attempting to deploy a commit to the OpenClaw Foundation Team on Vercel. A member of the Team first needs to authorize it. |
|
Codex review: needs maintainer review before merge. Reviewed September 16, 2026, 1:01 AM ET / 05:01 UTC (Revision 3). ClawSweeper reviewWhat this changesPackage publishing now delays generated ZIP storage until release insertion and deletes unadopted archives after rejection or release reuse, with regression tests and documented ownership rules. Merge readiness✅ Ready for maintainer review The fix remains necessary: current main and the latest release retain the early ZIP allocation. No blocking findings remain, and the updated real Convex results satisfy the previous proof request. Priority: P2 Review scores
Verification
How this fits togetherClawHub package publication turns authenticated uploads into release records and downloadable archives. This change controls when generated ZIPs become release-owned and which temporary archives can be deleted. flowchart TD
A[Authenticated package upload] --> B[Validate package and check retries]
B --> C[Return existing attempt or reject duplicate]
B --> D[Store generated ZIP when needed]
D --> E[Insert release]
E -->|Rejected or reused| F[Delete newly generated ZIP]
E -->|Created| G[Retain archive for scans and downloads]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Technical reviewBest possible solution: Keep cleanup at the action-to-release ownership boundary, preserving committed archives and caller uploads through retries and downstream failures. Do we have a high-confidence way to reproduce the issue? Yes, from source: a valid legacy package publication stores a ZIP before duplicate rejection or failed insertion on current main. The contributor supplies real before/after Convex results; this read-only review did not execute the path. Is this the best way to solve the issue? Yes. Cleanup beside archive creation and insertion is the narrowest repair; HTTP-wide cleanup could delete committed uploads, while pending-release cleanup cannot discover an unattached ZIP. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against f96495690dad. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
History |
Rejected package publications could leave generated ZIPs that no release owned. Trusted idempotent retries could also return an existing release successfully while leaking a newly generated ZIP. This change delays allocation until insertion and reclaims only generated ZIPs that the release does not adopt. Committed archives and caller-supplied artifacts remain available after later failures.
Closes #3677.
The insertion result identifies reuse internally; publication and legacy finalization remove that marker before returning their public result. Concurrent pending insertions reject safely, while valid staged retries retain the existing attempt path. The existing plugin plan documents the ownership boundary.
Before/after proof
Used anonymous local Convex 1.44.0 at
http://127.0.0.1:3460/:3461: real stored files, publisher/token/package/release rows, publication actions, insertion mutations, and HTTP downloads. No mocked storage or action context.The core baseline is main
a230dae1b7; the retry baseline is the original patch integrated with main0ef3e59e53. The downstream-error case deliberately injects a local-only error into the scan-enqueue mutation after the actual release commit. This is fault-injection evidence, not a production incident. All temporary fixture/injection changes were removed before validation.Validation
bun run ci:static,bun run ci:unit(6,951 passed, 3 skipped),bun run ci:packages, andbun run ci:types-buildpass.tsc --noEmitchecks pass; the clean function set pushes to the disposable runtime with typechecking enabled.Contributor history is preserved with a normal merge from main. Current-head hosted checks must pass before landing.