Skill-card worker leaves a Convex blob when attach fails

[SebTardif]

Summary

The skill-card worker stores generated skill-card.md in Convex, then attaches it to the skill version. If attach fails (stale lease or missing / soft-deleted version), the new storage id is never deleted. Retries store another orphan. skillCards.ts has no storage.delete.

This is a source-level resource leak in the worker action. I did not run the production skill-card worker.

Verification / reproduction

1. Read convex/skillCards.ts completeSkillCardJob (store, then attachCardAndSucceedJobInternal).
2. Follow lease mismatch / missing version throws.
3. Confirm the new _storage id is not deleted.

Actual

• A failed attach leaves an unreferenced markdown blob. Each retry adds another.

Expected

• If attach throws, the just-stored id is deleted.
• Successful attach still retains the card. Replacing an existing card on success may keep old fingerprints resolvable (that success-path retention is intentional).

Evidence

Source: convex/skillCards.ts. Proposed patch: PR #3614.

Security / privacy

Worker storage leak. No secret exposure.

Acceptance criteria

• Failed attach deletes the request-local card blob.
• Successful attach retains the new card.
• Success-path replacement of an older card does not break existing fingerprint resolution.

This tracker stays open if PR #3614 is closed without landing on main.

[SebTardif]

Proposed patch: #3614

Linked with Ref only. This issue stays open if that PR is closed without landing on main.

[clawsweeper]

Codex review: keeping this open for maintainer follow-up; there is still a little grit to resolve. Reviewed September 11, 2026, 10:13 AM ET / 14:13 UTC.

Summary
The storage leak remains on current main and in v0.23.3. The same-author linked PR is still open and unmerged, so this protected tracker remains necessary.

Reproducibility: yes. from source: valid worker credentials and markdown reach storage before a stale lease or deleted version causes attachment to throw, with no cleanup path. No live Convex reproduction was run.

Next step
Review #3614 as the existing repair candidate; the paired PR, protected security label, and bulk-filing policy preclude a new automated fix dispatch.

Security
None.

Review details

Best possible solution:

Keep cleanup local to failed completion: reclaim the newly stored blob, preserve the attachment error, and leave successful cards and historical fingerprints intact.

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

Yes, from source: valid worker credentials and markdown reach storage before a stale lease or deleted version causes attachment to throw, with no cleanup path. No live Convex reproduction was run.

Is this the best way to solve the issue?

Yes, request-local failure cleanup belongs in the completion action, which owns the storage ID; caller-side cleanup lacks that ID, and broad garbage collection would unnecessarily expand the repair.

AGENTS.md: found and applied where relevant.

Remaining risk / open question:

• Production frequency and accumulated orphan-storage volume are unknown; this review established the failure path from source without executing the worker.

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

Label changes

Label justifications:

• P2: Failed worker completions can accumulate unused storage, with no demonstrated outage or user-data loss.
• impact:other: The concrete impact is leaked blob storage and associated resource cost.

Evidence reviewed

What I checked:

• Current-main failure path: The checkout matches the supplied main SHA. completeSkillCardJob stores a blob at lines 529–531 and immediately awaits attachment without cleanup; attachment rejects mismatched leases and missing or soft-deleted versions at lines 459–462. (convex/skillCards.ts:529, cbfee7343ddc)
• Caller cannot reclaim the blob: processJob calls the completion action and reports failures through failSkillCardJob; it never receives the request-local storage ID. Its finally block removes only the local workspace. (scripts/skill-cards/run-skill-card-worker.ts:441, cbfee7343ddc)
• Successful replacement contract: Existing attachment coverage preserves source and prior generated-bundle fingerprints while installing the new card. The implementation explicitly preserves historical fingerprint records; failure cleanup must not broaden into success-path retention changes. (convex/skillCards.test.ts:741, cbfee7343ddc)
• Release comparison: The v0.23.3 worker source contains the same unguarded store-then-attach sequence; comparison against HEAD reports no changes in this file. (convex/skillCards.ts:529, 87ca030c30f3)
• Existing implementation candidate: GitHub REST confirms fix: delete skill-card blob when attach fails #3614 remains open and unmerged at head 2d28ce6. The supplied discussion explicitly pairs it with this tracker and preserves the tracker if that PR closes without landing. (2d28ce6bc845)
• History and routing limits: File history identifies repeated work by Patrick Erichsen and subsequent worker hardening by Jesse Merhi. Exact-line blame and patch inspection encountered unavailable historical blobs, so these are routing candidates rather than verified introducing authors. (convex/skillCards.ts, cbfee7343ddc)

Likely related people:

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

How this review workflow works

• 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.