Repository navigation
fix(update): clear source leases from the rehearsal state copy - #140004
Merged
Merged
Conversation
Contributor
|
🦞👀 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. |
steipete
force-pushed
the
steipete/fix-update-rehearsal-copy-leases
branch
from
September 6, 2026 09:58
cc6e27d to
5df208c
Compare
Contributor
|
Codex review: needs maintainer review before merge. Reviewed September 6, 2026, 6:12 AM ET / 10:12 UTC. ClawSweeper reviewWhat this changesClear process and coordination leases from disposable update-rehearsal database copies so candidate validation can proceed while the serving Gateway remains running. Merge readiness✅ Ready for maintainer review This remains a useful, focused repair: inspected main still copies live ownership claims into rehearsal state. No actionable correctness or security findings remain, and the related update work explicitly treats this PR as a separate prerequisite. Priority: P1 Review scores
Verification
How this fits togetherThe updater copies live databases into a private temporary directory before testing a candidate installation. Doctor acquires maintenance ownership of those copies, and successful rehearsal permits the updater to proceed toward activation. flowchart TD
A[Serving Gateway databases] --> B[Private database snapshots]
B --> C[Clear copied ownership claims]
C --> D[Rebind copied database paths]
D --> E[Doctor acquires fresh maintenance ownership]
E --> F[Candidate validation result]
Before mergeNone. Agent review detailsSecurityNone. PR surfaceSource +9, Tests +42. Total +51 across 2 files. View PR surface stats
Review metricsNone. Technical reviewBest possible solution: Keep rehearsal ownership independent of serving-process ownership while preserving live maintenance refusal and the copied data used for candidate validation. Do we have a high-confidence way to reproduce the issue? Yes: source shows copied live claims reaching unchanged maintenance refusal, and the supplied discussion records that failure in an installed update trial. This review did not execute a reproduction. Is this the best way to solve the issue? Yes: clearing invalid ownership claims at the private-copy producer repairs the boundary without weakening Doctor or applying unrelated archive sanitization. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against d7217198496e. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
Contributor
Author
|
Merged via squash.
|
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Sep 7, 2026
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
1 participant
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.
Related: #139701, #138839
What Problem This Solves
Fixes an issue where users updating a running Gateway would fail candidate migration rehearsal because the copied state still claimed that the serving Gateway owned its agent databases. The live Gateway correctly keeps serving through candidate validation, so its process remains alive and Doctor correctly refuses those copied claims.
Why This Change Was Made
Clear source agent-database and state-coordination leases in the private shared-database snapshot before publishing the rehearsal copy. Candidate processes then acquire their own ownership through the existing maintenance and runtime paths.
This is separate from concern E in the #135868 split because the repair belongs to the candidate-state snapshot owner. This is an implementation repair under the existing ownership-routing carve-out: there is no schema, schema-version, table-definition, migration, stored-representation, retention-rule, or public-contract change. It only clears lease rows in the throwaway rehearsal copy created by the canary; the live store and its lease semantics are unchanged. The material SQLite/persistent-store review checkpoint does not apply to this copy-only repair.
User Impact
Updates can validate a candidate while the previous Gateway remains running. Doctor continues to refuse maintenance against databases actually held by a live process.
Evidence
A packaged launchd A→B trial for fix(update): preserve restart outcomes and update-history ownership #139701 reproduced the refusal before activation. The old Gateway stayed healthy on the same process through 200 post-failure samples over 206 seconds. An independent installed snapshot-worker probe confirmed that the copy retained the serving process's two live agent-database leases.
The existing DELETE/WAL snapshot cases now initialize the canonical shared schema, seed a live PID/start identity and an unexpired maintenance claim, run the real snapshot worker, and require Doctor maintenance admission on the copy. Both copied lease tables must be empty; source database bytes and directory contents remain unchanged. Registry rebinding, durable data, schema versions, missing tables, and external-path coverage remain.
Focused candidate-state tests: 6 passed. Snapshot/canary/lease-owner/heartbeat coverage: 75 passed, 2 existing platform skips, including built-candidate startup/readiness and process-group teardown. Local autoreview found no actionable P0–P2 findings. The full changed-file gate passed twice, including after rebase. Branch autoreview is clean at P0–P2. The lockfile is unchanged.
Counterproof from committed checkpoint c535fab8e59: removing only the production repair makes both DELETE/WAL cases fail with the existing maintenance-lease timeout. Restoring the exact reviewed bytes makes both pass. The unchanged live-owner maintenance-refusal regression also passes.
Fresh launchd A→B and cold-C trials follow when fix(update): preserve restart outcomes and update-history ownership #139701 is rebased onto this repair, per the maintainer-requested sequence. They are not claimed as completed proof for this PR.
Exact-head CI passed on
5df208c0cd5d7c6d24bb76df73faa13628eccceb: run 34026108855. ClawSweeper’s review of that head found no actionable or security findings and listed no before-merge or rank-up requests.Production growth is limited to enforcing ownership in the private copy; there is no new abstraction or fallback. The broad archive sanitizer is intentionally unused because it also removes unrelated queued data and changes approval information.