fix(snapshot): stop restores from restarting sibling gateways - #112
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
merge-risk: 🚨 availability
|
Codex review: needs maintainer review before merge. Reviewed August 26, 2026, 3:48 PM ET / 19:48 UTC. ClawSweeper reviewWhat this changesThe PR makes snapshot restore and rollback refresh only the restored environment in the shared supervisor, with regressions that preserve unrelated gateways. Regression provenancePossible regression — probable (reproduction; reviewed change; failure trace). No predecessor PR is attributed. Merge readinessKeep this member-authored PR open for normal merge review. It narrowly fixes the source-proven recovery isolation bug with real-daemon regressions for both direct restore and failed-upgrade rollback. Priority: P1 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherOCM restores one environment’s snapshot or upgrade state and then reconciles it with the shared gateway supervisor. The supervisor’s persisted child specifications control whether running gateway processes are retained or restarted. flowchart LR
A[Snapshot restore or failed upgrade] --> B[Restore target environment]
B --> C[Targeted supervisor sync]
C --> D{Supervisor daemon running?}
D -->|Yes| E[Update target child only]
D -->|No, later activation| F[Full fleet reconciliation]
E --> G[Sibling gateways retained]
F --> G
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Merge after the platform test jobs confirm the targeted-recovery regressions and existing stopped-daemon full-sync behavior. Do we have a high-confidence way to reproduce the issue? Yes—source and two daemon-backed regression scenarios establish a concrete path: restore one environment while a sibling has latent supervisor-spec drift and verify its runtime state remains unchanged. Is this the best way to solve the issue? Yes—the patch reuses the established target-only supervisor merge rather than adding a parallel mechanism, while retaining full synchronization for later stopped-daemon activation. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning high; reviewed against cd8decaf04f2. 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
|
hannesrudolph
changed the title
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: fix(snapshot): stop restores from restarting sibling gateways This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
|
Thanks for the review. A few factual clarifications for the current head
Please reflect those distinctions in the fresh review of |
1 participant
Closes #111
What Problem This Solves
Fixes an issue where restoring one OCM environment could restart unrelated
running gateways when their persisted supervisor specifications had latent
drift.
This affected both direct snapshot restores and automatic rollback after a
failed upgrade. A target-scoped recovery could therefore interrupt unrelated
agent conversations and channel delivery.
Why This Change Was Made
When the shared supervisor daemon is already running, snapshot restore now
reconciles only the restored environment. Unrelated persisted child
specifications, skip entries, restart requests, and running processes are
preserved.
The stopped-daemon safety boundary remains unchanged: starting a stopped shared
daemon still performs full-fleet reconciliation before activation, so stale
sibling definitions are not launched.
User Impact
Operators can restore or automatically roll back one environment without
restarting sibling gateways. The restored target still receives its snapshot
state and converges normally.
Evidence
Production failure
During a failed Main upgrade rollback on August 26, 2026:
reloading rescue.OCM did not retain the exact Rescue specification field that differed. The
causal rollback path and sibling reload are directly recorded.
Regression coverage
Real-daemon tests introduce latent sibling specification drift and verify that
both restore paths preserve the sibling PID and persisted specification:
The tests also verify that the selected environment restores and converges
correctly.
Validation
cargo fmt --all -- --checkcargo test --test daemon_runtime_tests --locked -- --test-threads=1cargo test --test env_snapshot_tests --locked -- --test-threads=1cargo test --test upgrade_command_tests --locked -- --test-threads=1cargo check --workspace --all-targets --lockedcargo build --release --lockedcargo test --all-targets --locked -- --test-threads=1 --skip bin_wrapper_runs_with_an_overridden_homeThe skipped wrapper test requires
rustup; this host uses Homebrew Rust anddoes not have
rustup. Every executed test passed, and all GitHub CI checks aregreen.
Worked on by