fix(native-eval): rebootstrap replacement leases - #58
Conversation
clawsweeper
Bot
added
rating: 🦐 gold shrimp
|
Codex review: needs maintainer review before merge. Reviewed August 22, 2026, 12:09 PM ET / 16:09 UTC. ClawSweeper reviewWhat this changesThe PR records the lease ID used to hydrate a native-evaluation machine and rehydrates a replacement lease before dispatching recovery work. Regression provenancePossible regression — probable (reviewed change; failure trace). No predecessor PR is attributed. Merge readinessKeep open: current main still uses timestamp-only hydration, while this MEMBER-authored PR is the focused candidate fix for the linked open recovery report. No concrete patch defect was found; maintainers need to accept the deliberate one-time hydration for legacy manifests. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherShellBench’s native-evaluation fleet controller resumes benchmark runs on remote Crabbox leases. It reads a persisted manifest, prepares the active machine when needed, dispatches the harness, and records verified results. flowchart LR
A[Persisted recovery manifest] --> B[Fleet controller]
B --> C[Resolve active Crabbox lease]
C --> D{Hydrated lease ID matches?}
D -->|No| E[Hydrate runner and tasks]
D -->|Yes| F[Reuse prepared lease]
E --> G[Dispatch native harness]
F --> G
G --> H[Verify and persist results]
Decision needed
Why: The patch intentionally changes persisted-manifest recovery behavior, so maintainers own the resulting provider-cost trade-off. 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: Accept the lease-ID invariant and merge the focused repair, retaining safe one-time hydration for legacy manifests. Do we have a high-confidence way to reproduce the issue? Yes—the linked report and focused test define a timestamp-only recovery manifest that obtains a replacement lease; this read-only review did not execute it. Is this the best way to solve the issue? Yes—the active lease ID is the missing identity needed to distinguish stale bootstrap metadata from a prepared active machine, and the change is confined to recovery. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning high; reviewed against 884dd1bb5511. 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 (68 earlier review cycles; latest 8 shown)
|
clawsweeper
Bot
added
rating: 🐚 platinum hermit
clawsweeper
Bot
added
rating: 🐚 platinum hermit
clawsweeper
Bot
removed
the
proof: sufficient
Integrate PR #58 onto current main while preserving active-harness hydration. Cover legacy manifests, mismatched and matching lease IDs, active-run preservation, and retry after a failed bootstrap. Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
|
Superseded by #72 (merged), a reviewed integration of this repair from current main with the legacy timestamp-only manifest case verified — thank you @vincentkoc, credit preserved in the landed commit and changelog. |
2 participants
What does this PR do?
Rehydrates replacement native-eval leases before dispatch.
Why?
The controller previously treated an old bootstrap timestamp as proof that a
new lease contained the runner. Recovery could then fail before the harness
started, producing a false benchmark failure.
Fixes #55
Changes
bootstrapped_lease_idLive proof
Recovered a legacy OpenClaw run whose manifest had an old bootstrap timestamp
but no
bootstrapped_lease_id.bootstrapped_lease_id4/4completed results0This is the exact recovery state the regression test covers; the controller no
longer trusts timestamp-only bootstrap state.
Tests