fix: repair orphaned pending skill versions stuck after publish - #3401
Open
trippyogi wants to merge 19 commits into
Open
fix: repair orphaned pending skill versions stuck after publish#3401trippyogi wants to merge 19 commits into
trippyogi wants to merge 19 commits into
Conversation
Contributor
|
@trippyogi is attempting to deploy a commit to the OpenClaw Foundation Team on Vercel. A member of the Team first needs to authorize it. |
trippyogi
added a commit
to trippyogi/clawhub
that referenced
this pull request
Aug 4, 2026
…air flow - releaseFinalizationClaimPatch (publishAttempts.ts) now caps consecutive finalization failures only for kind==="skill". Package attempts have no repair path yet, so they retry unbounded instead of becoming a permanent orphan. - isActiveAttemptLive (publishAttempts.ts) no longer abandons a non-terminal attempt just because finalizationFailureCount > 0. It now checks staleness against updatedAt (falling back to createdAt) using ACTIVE_ATTEMPT_RETRYABLE_STALE_MS for attempts with retry activity, or ACTIVE_ATTEMPT_UNCLAIMED_GRACE_MS for zero-failure attempts, so a below-cap dispatcher retry (or a long pending_checks phase that just transitioned cleanly) is not raced by manual repair. Added findActiveSkillPublishAttemptByIdInternal so the repair pre-check can look up a known publishAttemptId directly instead of only scanning by slug/version. - repairOrphanedPendingSkillVersionHandler (maintenance.ts) now publishes the pending version and closes the orphaned attempt atomically via a new publishPendingVersionAndCloseAttemptInternal mutation (skills.ts), closing before follow-ups are scheduled. This removes the publish-before-close race where a crash between publish and close left the version published but the attempt reclaimable. Also handles the already-published-but-still-open-attempt retry case as an attempt-cleanup path. - skillsGetRouterV1Handler exact-version path (skillsV1.ts) falls back to an owner-visible pending-publication diagnostic via getSkillBySlugInternal + describeOwnerVisibleSkillVersionState when the public getBySlug lookup misses entirely, so a brand-new hidden skill (pending its first publish) no longer 404s for its owner. Tests added/updated in publishAttempts.test.ts, maintenance.test.ts, and httpApiV1.handlers.test.ts for all four findings.
trippyogi
marked this pull request as ready for review
August 4, 2026 21:57
clawsweeper
Bot
added
rating: 🦪 silver shellfish
Contributor
|
Codex review: needs real behavior proof before merge. Reviewed August 20, 2026, 12:44 AM ET / 04:44 UTC. ClawSweeper reviewWhat this changesThe PR bounds failed skill finalization retries, adds admin recovery for orphaned pending skill versions, and gives authorized owners actionable publication-state diagnostics. Merge readiness⛔ Blocked until stronger real behavior proof is added - 3 items remain Keep this PR open: the current recovery design addresses the prior maintainer blockers, but it still needs inspectable real after-fix proof from the current head before merge. Priority: P1 Review scores
Verification
Live VerificationCommand: Result: PASS (completed) Assertions:
How this fits togetherClawHub stages a skill version before asynchronously projecting it into the public skill index, latest pointer, tags, and security follow-ups. This change governs how failed finalization is surfaced and safely recovered without exposing pending versions to other callers. flowchart LR
A[Skill publish request] --> B[Pending skill version]
B --> C[Recorded security checks]
C --> D[Async finalization]
D --> E[Public index and latest tags]
D --> F[Owner-only diagnostics]
D --> G[Admin orphan recovery]
Before merge
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. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land the bounded recovery path only after a current-head run proves that incomplete checks remain blocked and that a clean stranded version is atomically projected, audited, and made publicly readable. Do we have a high-confidence way to reproduce the issue? Yes for source reproduction: the current Convex-test fixture creates the pending-version and incomplete-check state, then verifies rejection before the clean-check repair path. This review did not execute the test. Is this the best way to solve the issue? Yes: reusing the existing finalizer from the stored pending payload, while requiring recorded clean checks and atomically closing the attempt, is the narrowest maintainable recovery path. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against d6a8c68501d4. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (24 earlier review cycles; latest 8 shown)
|
Patrick-Erichsen
requested changes
Aug 5, 2026
Patrick-Erichsen
left a comment
Patrick-Erichsen
left a comment
Collaborator
There was a problem hiding this comment.
This adds owner diagnostics and an operator repair for skill versions stranded in staged publication. The repair is not safe yet because a terminally failed publish attempt is not actually closed/cleared by the helper path.
LOC: +2087/-26 (9 files)
Finding: when the linked attempt is already failed, the close helper returns without clearing the persisted pendingPublication snapshot. The version can become public while its record remains internally staged. Add the cap-exhausted regression case and real Convex repair proof.
Best-fix verdict: too broad in its current form. Reusing the finalization contract is sound, but the atomic repair must explicitly handle terminal attempts before this operator path is safe.
Alternatives considered: a second cleanup mutation would restore the crash window; the better shape is one transaction that publishes and clears/closes every supported attempt state.
Code read: convex/maintenance.ts, convex/skills.ts, convex/lib/skillPublish.ts, publish-attempt helpers, HTTP owner diagnostics, and adjacent tests.
Remaining uncertainty: no after-fix staged publish/repair has been exercised on a real Convex deployment, and the branch is currently conflicted with main.
clawsweeper
Bot
added
rating: 🧂 unranked krab
Contributor
|
This pull request has been automatically marked as stale due to inactivity. |
Collaborator
|
Independent #3466 validation confirms this PR owns the same stranded staged-publication lifecycle as #3349, so I am not opening a duplicate fix PR. Validated PR head Runtime red/green proof used a real
Current main fails because no recovery action exists. This branch correctly refuses recovery while the original ClawScan check is incomplete, then publishes and projects Three blockers remain before this is safe to land:
Validation on the current-main merge:
The PR is also currently conflicting with |
…claw#3349) A staged skill publish inserts the skillVersion as pending up front and only projects it onto the skill's latest/tags/version-index once the async publishAttempts finalize step succeeds. If that worker never runs again (crashed action, exhausted retries), the version was left orphaned forever: files exist, the duplicate-version guard blocks a retry, but every metadata read 404s. - Cap consecutive transient finalization failures (finalizationFailureCount, publishAttempts.ts) so a stuck attempt surfaces as failed instead of looping forever. - Add an admin-gated repair path (maintenance.ts) that re-runs finalization for one orphaned pending version, or sweeps the whole table in age-bounded batches, from the exact insert args captured on the version at publish time. Refuses to act while a live attempt still owns the version, and force-closes the original attempt row afterward so the normal dispatcher can't re-claim it and re-run followups. - Surface pending/blocked publication state to the authenticated owner on version reads instead of a bare 404, without leaking it to other callers (skillsV1.ts).
- Authorize owner-visible pending/blocked reads via publisher scope - Return 409 when linked finalize attempt already failed - Do not treat checkFailureCount as abandoned (scanner retry gaps)
…air flow - releaseFinalizationClaimPatch (publishAttempts.ts) now caps consecutive finalization failures only for kind==="skill". Package attempts have no repair path yet, so they retry unbounded instead of becoming a permanent orphan. - isActiveAttemptLive (publishAttempts.ts) no longer abandons a non-terminal attempt just because finalizationFailureCount > 0. It now checks staleness against updatedAt (falling back to createdAt) using ACTIVE_ATTEMPT_RETRYABLE_STALE_MS for attempts with retry activity, or ACTIVE_ATTEMPT_UNCLAIMED_GRACE_MS for zero-failure attempts, so a below-cap dispatcher retry (or a long pending_checks phase that just transitioned cleanly) is not raced by manual repair. Added findActiveSkillPublishAttemptByIdInternal so the repair pre-check can look up a known publishAttemptId directly instead of only scanning by slug/version. - repairOrphanedPendingSkillVersionHandler (maintenance.ts) now publishes the pending version and closes the orphaned attempt atomically via a new publishPendingVersionAndCloseAttemptInternal mutation (skills.ts), closing before follow-ups are scheduled. This removes the publish-before-close race where a crash between publish and close left the version published but the attempt reclaimable. Also handles the already-published-but-still-open-attempt retry case as an attempt-cleanup path. - skillsGetRouterV1Handler exact-version path (skillsV1.ts) falls back to an owner-visible pending-publication diagnostic via getSkillBySlugInternal + describeOwnerVisibleSkillVersionState when the public getBySlug lookup misses entirely, so a brand-new hidden skill (pending its first publish) no longer 404s for its owner. Tests added/updated in publishAttempts.test.ts, maintenance.test.ts, and httpApiV1.handlers.test.ts for all four findings.
Cap-exhausted failed attempts keep a fresh updatedAt and failure count, so isActiveAttemptLive blocked orphan repair for ~20m even though the dispatcher cannot reclaim them. Gate the live check on active statuses only, matching the direct-ID precheck, and cover the failed+cap path.
Refresh stale take(10) comments to match the paginated fallback, and remove attemptCloseWarning from the successful repair path now that claim-active/version-mismatch throw and roll back in the same mutation.
Require clean TruffleHog+ClawScan before any repair-eligible status, write a transactional recovery audit with actor, and only claim operator repair when the failed attempt is actually eligible. Add openclaw#3466 CLEAN-but-pending runtime coverage and a throwaway-dev proof harness.
trippyogi
force-pushed
the
fix/3349-publish-version-projection
branch
from
August 20, 2026 04:23
778ecf6 to
8c312d8
Compare
Remove throwaway proof seed/script and restore Convex generated files to match origin/main so the openclaw#3401 tip stays publishable.
clawsweeper
Bot
removed
the
merge-risk: 🚨 availability
Author
|
@Patrick-Erichsen addressed the three blockers from your latest pass on cleaned head
Also added the #3466 CLEAN-but- Convex proofReal dev deployment:
No production deployment or repair was run. 1. Finalized + incomplete checks stay blocked{
"pendingChecks": {
"reason": "attempt-checks-incomplete",
"repaired": false,
"status": "pending_checks"
},
"finalizedWithIncompleteChecks": {
"repair": {
"reason": "attempt-checks-incomplete",
"repaired": false,
"status": "finalized"
},
"attempt": {
"status": "finalized",
"checks": {
"trufflehog": "clean",
"clawscan": "pending"
},
"repairEligible": false
},
"version": {
"version": "1.0.1",
"publicationStatus": "pending",
"hasPendingPublicationSnapshot": true
},
"recoveryAudits": []
}
}This directly covers the finalized-attempt bypass: no projection, no cleanup, and no recovery audit when both recorded checks are not clean. 2. Capped clean finalization failure → successful repairBefore: {
"skill": {
"latestVersion": "1.0.0",
"versionsStat": 1
},
"version": {
"version": "1.0.1",
"publicationStatus": "pending",
"hasPendingPublicationSnapshot": true
},
"attempt": {
"status": "failed",
"finalizationFailureCount": 5,
"checks": {
"trufflehog": "clean",
"clawscan": "clean"
},
"repairEligible": true,
"hasResult": false
}
}Repair: {
"repaired": true,
"slug": "orphan-repair-proof",
"version": "1.0.1",
"result": {
"versionId": "xs7eactfxz3e56geg1p0tvjpkn8ctp9g",
"publicationStatus": "published"
}
}After: {
"skill": {
"latestVersion": "1.0.1",
"latestVersionId": "xs7eactfxz3e56geg1p0tvjpkn8ctp9g",
"latestTagVersionId": "xs7eactfxz3e56geg1p0tvjpkn8ctp9g",
"versionsStat": 2
},
"version": {
"version": "1.0.1",
"publicationStatus": "published",
"hasPendingPublicationSnapshot": false
},
"attempt": {
"status": "finalized",
"hasResult": true,
"resultVersionId": "xs7eactfxz3e56geg1p0tvjpkn8ctp9g"
}
}A second repair/sweep did not republish the version. 3. Transactional recovery audit{
"actorUserId": "zx71adwhfnxytkvzvs6eyveaw98ctb36",
"action": "skill.orphaned_pending_version.repair",
"metadata": {
"skillId": "xx75gsf08g3k755rexfztjgerx8cvcwb",
"version": "1.0.1",
"publishAttemptId": "r97cyjhzzmhnj4xfrnwmep53918cv239",
"priorPublicationStatus": "pending",
"priorAttemptStatus": "failed",
"hadPendingPublication": true,
"resultVersionId": "xs7eactfxz3e56geg1p0tvjpkn8ctp9g",
"resultPublicationStatus": "published"
}
}Dry-run returned 4. Owner / anonymous HTTP behavior{
"incompleteChecks_owner": {
"status": 423
},
"incompleteChecks_anonymous": {
"status": 404
},
"failedIncompleteChecks_owner": {
"status": 409,
"message": "publication checks did not complete successfully; orphan repair is not available until recorded TruffleHog and ClawScan checks are both clean"
},
"failedIncompleteChecks_anonymous": {
"status": 404
},
"cappedFailedClean_owner": {
"status": 409,
"message": "publication finalization failed; an operator can repair it with maintenance:repairOrphanedPendingSkillVersion"
},
"cappedFailedClean_anonymous": {
"status": 404
},
"after_anonymousVersion": {
"status": 200,
"version": "1.0.1"
},
"after_skill": {
"tags.latest": "1.0.1",
"stats.versions": 2
}
}5. Security followups from real Convex schedulerRead-only {
"counts": {
"vt": 1,
"enqueueSkillVersionScanInternal": 3,
"total": 4
},
"jobs": [
{
"name": "securityScan.js:enqueueSkillVersionScanInternal",
"scheduledTime": 1787199263783,
"state": { "kind": "success" },
"args": {
"source": "publish",
"versionId": "xs7eactfxz3e56geg1p0tvjpkn8ctp9g"
}
},
{
"name": "vt.js:scanWithVirusTotal",
"scheduledTime": 1787199263783,
"state": { "kind": "success" },
"args": {
"versionId": "xs7eactfxz3e56geg1p0tvjpkn8ctp9g"
}
},
{
"name": "securityScan.js:enqueueSkillVersionScanInternal",
"scheduledTime": 1787199265783,
"state": { "kind": "success" },
"args": {
"source": "publish",
"preserveActiveJob": true,
"preserveExistingJob": true,
"versionId": "xs7eactfxz3e56geg1p0tvjpkn8ctp9g"
}
},
{
"name": "securityScan.js:enqueueSkillVersionScanInternal",
"scheduledTime": 1787199278783,
"state": { "kind": "success" },
"args": {
"source": "publish",
"preserveActiveJob": true,
"preserveExistingJob": true,
"versionId": "xs7eactfxz3e56geg1p0tvjpkn8ctp9g"
}
}
]
}That is exactly:
The normal-finalizer path remains covered against duplicate scheduling. Validation
Local Windows full-suite runs have unrelated environment/format noise, so the repository Linux workflows remain the final full-suite gate. No production deployment or repair was run. |
lechiffre-cfo
pushed a commit
to lechiffre-cfo/clawhub
that referenced
this pull request
Aug 22, 2026
…air flow - releaseFinalizationClaimPatch (publishAttempts.ts) now caps consecutive finalization failures only for kind==="skill". Package attempts have no repair path yet, so they retry unbounded instead of becoming a permanent orphan. - isActiveAttemptLive (publishAttempts.ts) no longer abandons a non-terminal attempt just because finalizationFailureCount > 0. It now checks staleness against updatedAt (falling back to createdAt) using ACTIVE_ATTEMPT_RETRYABLE_STALE_MS for attempts with retry activity, or ACTIVE_ATTEMPT_UNCLAIMED_GRACE_MS for zero-failure attempts, so a below-cap dispatcher retry (or a long pending_checks phase that just transitioned cleanly) is not raced by manual repair. Added findActiveSkillPublishAttemptByIdInternal so the repair pre-check can look up a known publishAttemptId directly instead of only scanning by slug/version. - repairOrphanedPendingSkillVersionHandler (maintenance.ts) now publishes the pending version and closes the orphaned attempt atomically via a new publishPendingVersionAndCloseAttemptInternal mutation (skills.ts), closing before follow-ups are scheduled. This removes the publish-before-close race where a crash between publish and close left the version published but the attempt reclaimable. Also handles the already-published-but-still-open-attempt retry case as an attempt-cleanup path. - skillsGetRouterV1Handler exact-version path (skillsV1.ts) falls back to an owner-visible pending-publication diagnostic via getSkillBySlugInternal + describeOwnerVisibleSkillVersionState when the public getBySlug lookup misses entirely, so a brand-new hidden skill (pending its first publish) no longer 404s for its owner. Tests added/updated in publishAttempts.test.ts, maintenance.test.ts, and httpApiV1.handlers.test.ts for all four findings.
lechiffre-cfo
pushed a commit
to lechiffre-cfo/clawhub
that referenced
this pull request
Aug 22, 2026
Require clean TruffleHog+ClawScan before any repair-eligible status, write a transactional recovery audit with actor, and only claim operator repair when the failed attempt is actually eligible. Add openclaw#3466 CLEAN-but-pending runtime coverage and a throwaway-dev proof harness.
lechiffre-cfo
pushed a commit
to lechiffre-cfo/clawhub
that referenced
this pull request
Aug 22, 2026
Remove throwaway proof seed/script and restore Convex generated files to match origin/main so the openclaw#3401 tip stays publishable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3349
Related: #3353 (scanner retry cap only — explicitly not this defect)
What Problem This Solves
Fixes an issue where
clawhub skill publishcould report success and reserve a version while that version never appeared inlatest, the version index, or tag pointers. Publishers then hit a dead end: metadata reads 404, files remain fetchable by explicit version, and republish is rejected as a duplicate.Why This Change Was Made
Staged skill publish inserts a pending
skillVersionfirst and only projects it onto the skill document during async finalization. If finalization never completes, the version stays orphaned forever.This change:
failedinstead of looping forevermaintenance:repairOrphanedPendingSkillVersion+ sweep) that re-runs finalization from the version's captured insert args, refuses while a live attempt still owns the version, and force-closes the original attempt afterwardNon-goals: does not change the normal happy-path publish projection; does not auto-run repair in production without an operator.
User Impact
Evidence
Focused tests (local):
bunx vitest run convex/httpApiV1.handlers.test.ts -t "3349"— 5 passedbunx vitest run convex/publishAttempts.test.ts -t "findActiveSkillPublishAttempt|3349|orphaned|finalizationFailure|scanner retries"— 11 passedBugbot (branch vs main): fixed findings before draft —
checkFailureCounttreated as abandoned → onlyfinalizationFailureCountabandons grace (scanner retries stay live)Note: #3353 merged earlier and bounds prepublication scanner retries; it does not close #3349 (called out in that PR body as a separate post-finalization consistency defect).