fix: harden gateway restart, cleanup, and child temp dirs - #103
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
Codex review: needs maintainer review before merge. Reviewed August 21, 2026, 3:57 PM ET / 19:57 UTC. ClawSweeper reviewWhat this changesThe PR makes gateway restarts recovery-aware, scopes cleanup to the selected environment, reaps child process groups, and creates fresh private temporary directories for supervised children. Merge readinessThe re-review resolves the prior activated-shell blocker: readiness is skipped only for the target gateway identity, while an activated shell still waits for replacement and health. No new actionable patch defect was found; this member-authored PR should remain open for normal maintainer review. Priority: P1 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherOCM manages OpenClaw gateway processes for named environments. Lifecycle commands update the supervisor, which replaces gateway children and returns readiness information to operators. flowchart LR
Operator[Operator lifecycle command] --> Service[OCM service lifecycle]
Service --> Mode{Restart mode}
Mode -->|Protocol handoff| Gateway[OpenClaw gateway]
Mode -->|Legacy or forced| Supervisor[OCM supervisor]
Gateway --> Child[Replacement gateway child]
Supervisor --> Child
Child --> Ready[Readiness result]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land the recovery-aware restart path with its legacy fallback and explicit force escape hatch, while retaining the activated-shell and process-cleanup regression coverage. Do we have a high-confidence way to reproduce the issue? Yes: the checked-in daemon integration test starts a local supervisor, restarts with only OCM_ACTIVE_ENV set, and requires gateway readiness after replacement; the self-restart fixture covers the gateway-marker branch. Is this the best way to solve the issue? Yes: reusing the existing protocol-v1 gateway marker is the narrowest fix because normal activation removes that marker, so only an actual supervised gateway bypasses its own readiness wait. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning high; reviewed against 768714ae3491. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (2 earlier review cycles) |
|
@clawsweeper re-review I accepted the activated-shell finding and pushed Change selectedThe self-restart exemption now requires both:
I used the existing service-kind marker instead of adding another OCM-only variable. Coverage
I did not decline any part of the actionable review advice; the only implementation choice was to reuse the existing protocol-v1 gateway identity instead of creating a new marker. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
7f924cb to
1c1a177
Compare
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: fix: harden gateway restart, cleanup, and child temp dirs 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. |
Related: #60
Additional instructions
MUST: Keep Allow edits from maintainers enabled for this PR so maintainers
can help update the branch when needed.
What Problem This Solves
ocm service restartdirectly replaces a supervised gateway even when OpenClaw advertises restart-handoff support, so eligible active sessions and subagents can be interrupted without receiving OpenClaw's recovery marker. A restart issued from inside the target gateway can also wait on its own replacement.Environment removal and service uninstall currently reconcile the complete supervisor registry. If sibling runtime metadata or inherited process environment has drifted, deleting one environment can rewrite or restart unrelated gateways. Exited children can leave descendants or an unreaped process-group leader behind, and persisted macOS per-login
TMPDIRvalues can become invalid before a later gateway spawn.Why This Change Was Made
Use OpenClaw restart-handoff protocol v1 for normal service restarts, return immediately for a self-restart, retain a warned direct-restart fallback for legacy bindings, and expose
--forceas the explicit recovery bypass.Make uninstall, guarded destroy, direct removal, and prune reconcile only the affected environment. Clean the complete child process group with TERM, bounded polling, KILL fallback, and an unconditional leader wait. Stop persisting
TMPDIR; each child spawn receives a private daemon-scoped mode-0700directory derived from the daemon's current temp base, with a safe fallback when that base is unavailable.Local companion packaging and named Tailscale Service ingress changes are intentionally outside this PR.
User Impact
Gateway restarts preserve OpenClaw's eligible recovery behavior instead of blindly interrupting active work. Removing or stopping one environment leaves sibling gateway definitions and PIDs unchanged despite unrelated drift. Gateway descendants are cleaned up reliably, and future child spawns no longer inherit stale temporary-directory paths after login, reboot, or daemon replacement.
Legacy gateways remain restartable through the existing direct supervisor path and receive an explicit warning; operators can deliberately select that path with
--force.Evidence
main; it contains no companion-packaging or Tailscale ingress changes.cargo fmt --all -- --check: passed.git diff --check origin/main..HEAD: passed.cargo test --locked --test daemon_runtime_tests --test env_destroy_tests --test service_command_tests --test upgrade_command_tests: 126 passed, 0 failed.