Skip to content

miner(attempt): guard every step of runAttempt's finally-block cleanup #9677

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

runAttempt in packages/loopover-miner/lib/attempt-cli.ts ends with one finally block (lines 1087-1128) performing the whole teardown sequence in this order:

  1. await cleanupWorktree(...) (line 1096) — spawns git worktree remove.
  2. claimLedger.releaseClaim(...) (line 1101) — the local soft-claim release.
  3. await submitClaim({ ...claimRecord, status: "released" }, { env }) (line 1107) — a network POST to the hosted discovery plane.
  4. allocator.release(attemptId) (line 1109) — returns the worktree pool slot.
  5. the DB-fork discard (lines 1115-1122).
  6. allocator?.close(); claimLedger?.close(); eventLedger?.close(); attemptLog?.close(); governorLedger?.close(); (lines 1123-1127).

Only step 5 is defensive, and its comment states the discipline the rest of the block violates: "A discard failure must never crash the rest of this cleanup sequence ... it leaks one Neon branch rather than losing the claim/ledger release that follows it".

Steps 1 and 3 are the two riskiest operations in the block (a git subprocess and an HTTP POST) and both sit before the claim release, the allocator release, and every close(). If cleanupWorktree rejects (a locked worktree, a git binary failure, a timeout) or submitClaim rejects (the discovery plane is down or returns non-2xx), the finally block throws and steps 2/4/6 never run. The soft-claim stays active — line 1098's own comment says "a claim that outlives its own attempt process would wrongly tell a sibling miner this issue is still in flight" — the worktree pool slot stays leased, and every open SQLite handle is left unclosed.

The claim is not recoverable in-band, so a single git worktree remove failure can block maxConcurrentClaims on that repo for the ledger's full 14-day expiry window (DEFAULT_MAX_CLAIM_AGE_MS, claim-ledger-expiry.ts:5).

Requirements

  • Wrap the cleanupWorktree call (lines 1094-1097) in its own try/catch, so a rejection cannot abort the remainder of the finally block.
  • Wrap the hosted submitClaim(... status: "released" ...) call (lines 1105-1108) in its own try/catch, likewise.
  • Each catch must report via captureMinerError(error, { kind: "<name>", ... }) — the exact reporting call the DB-fork discard's own catch at line 1120 uses — with a distinct kind per site (attempt_worktree_cleanup_failed, attempt_hosted_claim_release_failed) and the repoFullName / attemptId context fields the sibling captures already pass.
  • Neither catch may change runAttempt's return value or swallow the error silently (no bare catch {}).
  • The local claimLedger.releaseClaim call and the allocator.release / close() calls must remain in their current order and must now be reachable on every path through the finally block.

⚠️ Required pattern: mirror the DB-fork discard block at packages/loopover-miner/lib/attempt-cli.ts:1115-1122 exactly — a local try { ... } catch (error) { captureMinerError(error, { kind: "...", ... }); } around the single risky call. It does NOT satisfy this issue to wrap the whole finally body in one try/catch (that still lets an early failure skip later steps), to move the cleanup out of finally, to introduce a new generic "safeCleanup" helper, or to swallow the errors without a captureMinerError call.

Deliverables

  • The finally block wraps the cleanupAttemptWorktree call in try/catch with captureMinerError(error, { kind: "attempt_worktree_cleanup_failed", repoFullName, attemptId }).
  • The same finally block wraps the hosted submitSoftClaim(... "released" ...) call in try/catch with captureMinerError(error, { kind: "attempt_hosted_claim_release_failed", repoFullName, attemptId }).
  • A new test in test/unit/miner-attempt-cli.test.ts injects a cleanupAttemptWorktree that rejects and asserts the injected claim ledger still received releaseClaim, the injected allocator still received release() and close(), and runAttempt returns the same exit code it returns when cleanup succeeds.
  • A second new test injects a submitSoftClaim that rejects (with the discovery plane enabled in the injected env) and asserts the injected allocator still received release() and every injected store still received close().

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example guarding cleanupAttemptWorktree while leaving the hosted release unguarded, or adding the guards without the two injection-based regression tests — does not resolve this issue.

Test Coverage Requirements

packages/loopover-miner/lib/**/*.ts IS inside Codecov's coverage.include in vitest.config.ts (the config comments note there is deliberately no blanket exemption for this package — it "stays included and genuinely graded"), so the 99%+ branch-counted codecov/patch gate applies here exactly as it does to src/**. Both arms of each new try/catch must be covered: the success path and the throwing path. Each new test must be a named regression test that fails against the current unguarded code.

Expected Outcome

A failing git worktree remove or hosted soft-claim release can no longer strand an active claim, leak a worktree pool slot, or leave local SQLite stores unclosed; both failures are reported to the error sink instead of aborting teardown.

Links & Resources

packages/loopover-miner/lib/attempt-cli.ts:1087-1128, :1115-1122 (the pattern to mirror), packages/loopover-miner/lib/attempt-worktree.ts, packages/loopover-miner/lib/discovery-index-client.ts, packages/loopover-miner/lib/sentry.ts.

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions