Skip to content

fix(gateway): preserve child turns during systemd drain - #140568

Merged
steipete merged 1 commit into
mainfrom
codex/restart-drain-codex
Sep 7, 2026
Merged

fix(gateway): preserve child turns during systemd drain#140568
steipete merged 1 commit into
mainfrom
codex/restart-drain-codex

Conversation

@steipete

@steipete steipete commented Sep 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where a Linux managed Gateway restart interrupted an active child runtime before its advertised drain window finished (#140479, reported by @DonnieFi). Also documents the missing offline-reader prerequisite for recovery cleanup (#140467, reported by @agentzerodao); that reporter confirms main-session recovery succeeded after upgrading to 2026.9.2.

Related: #140479, #140467.

Why This Change Was Made

The generated systemd unit gave the Gateway 330 seconds to stop, but KillMode=control-group immediately sent SIGTERM to its children as well. src/daemon/systemd-unit.ts now uses mixed: the Gateway owns cooperative drain, and systemd retains final cgroup cleanup after parent exit or timeout. src/daemon/service-audit.ts checks the effective manager setting, including drop-ins, and flags older or omitted modes. Existing process/none diagnostic codes remain intact; duplicate dependency parsing is consolidated.

Increasing the timeout cannot postpone that initial child signal. Process/none would abandon final descendant cleanup. Adding a separate Gateway-first stop protocol would duplicate lifecycle orchestration. The repair uses the supervisor's existing main-process-first contract instead.

Cleanup documentation explicitly requires session watchers and other database readers to stay stopped until cleanup exits, explains WAL/SHM changes, and gives the retry action. Unlike #140505, this does not relax destination verification or change persistence semantics. The issue's accepted documentation alternative preserves the existing deletion safeguards.

User Impact

New or explicitly reinstalled Linux units let child turns complete during Gateway drain. Existing installations need openclaw gateway install --force for their selected profile; ordinary update-mode Doctor preserves existing Linux service definitions. Operator-owned drop-ins remain separately editable and are checked through their effective setting. Cleanup users can identify and stop readers that invalidate offline verification.

Evidence

Reviewed head: b1d9cde423b9dd70292fdb189f257624b6c7e740.

  • Pre-fix regressions: generated-unit test rejected control-group; audit tests failed to flag an effective control-group drop-in, explicit control-group, and omitted KillMode. They pass after repair.
  • Focused daemon, Gateway run-loop, and real-process SIGTERM suite: 396 passed. Final audit-only rerun: 62 passed.
  • Main-session recovery state/store/marking and selected startup/repeated-restart harnesses: 70 passed. The original 9.1 operator abort interleaving was not reproduced or attributed to one historical commit; no recovery/admission guard was changed.
  • Existing cleanup destination mutation and successful retirement tests: 10 passed. In-place edits, truncation, and WAL commits retain originals across confirmation/publication/unlink phases.
  • pnpm check:changed passed all gates, including core/core-test typechecks, three dead-export scans, lint and repository guards; git diff --check passed.
  • Independent Codex review: no actionable P0–P2 findings on the final diff. An initial check caught string widening in a test table; using the exported audit constants corrected that type error.
  • Native systemd 255 Testbox proof: 6/6 passed with detached synthetic stdio children. Control-group interrupts active work; mixed permits completion during stop and actual restart, with a fresh successor MainPID. Timeout, parent crash, and ordinary stop still remove descendants. Zero test units/processes remain; lease tbx_01m1wpdp5yxrtq5zqf7gjp1sja stopped, wrapper exit 0. The synthetic timeout was shortened to two seconds; production remains 330 seconds. External Testbox cleanup may mark its backing Actions run cancelled; the wrapper records successful proof independently.

The sibling Codex source was personally inspected at dependency revision 64b482500d00b331189beefd173a5e08b4060ff2: codex-rs/app-server/src/lib.rs:742-744 disables graceful signal restart for stdio; :1040-1063 exits on stdio closure; :1203-1212 shuts down threads. codex-rs/app-server-transport/src/transport/stdio.rs:44-78 translates stdin EOF to connection closure. Native systemd proof uses synthetic piped child work, not an authenticated provider or a reproduction of the reporter's exact clean-exit code.

Production: +15/-18 (net -3). Tests: +23/-5 (net +18). Docs: +27/-7 (net +20). No new configuration, schema, persistence semantics, protocol, dependency, public SDK boundary, or changelog changes.

node scripts/run-vitest.mjs src/daemon/systemd-unit.test.ts src/daemon/service-audit.test.ts src/daemon/systemd.test.ts src/cli/gateway-cli/run-loop.test.ts src/cli/gateway-cli/run-loop.direct-stop-active-work.process.test.ts
node scripts/run-vitest.mjs src/daemon/service-audit.test.ts
node scripts/run-vitest.mjs src/agents/main-session-recovery/main-session-recovery-state.test.ts src/agents/main-session-recovery/main-session-recovery-store.test.ts src/agents/main-session-recovery/main-session-restart-recovery-marking.test.ts
node scripts/run-vitest.mjs src/agents/main-session-recovery/main-session-restart-recovery.test.ts -t 'marks startup-orphaned running main sessions before recovery|aborts an exact recovery accepted after the execution-start deadline|keeps restart safety across a second restart of the recovery turn|resumes an abort artifact persisted with the gateway restart reason'
node scripts/run-vitest.mjs src/agents/main-session-recovery/main-session-restart-recovery.test.ts -t 'restores an accepted recovery that fails before execution starts' --reporter=dot
node scripts/run-vitest.mjs src/commands/doctor-session-sqlite.test.ts -t 'retains originals after destination|retires.*original'
pnpm check:changed
git diff --check

CI: exact-head run 34072463845 completed successfully on b1d9cde423b9dd70292fdb189f257624b6c7e740, with no failed jobs. No contributor PR code was used; thanks to both issue reporters for the operational evidence.

Review follow-through

ClawSweeper found no actionable source or security defect. Its remaining risk concerns independent access to the native evidence and the exact reported clean-exit signature. The mitigation is the implemented ownership boundary: Gateway drain, systemd final cleanup, effective-mode diagnostics, and explicit reinstall/drop-in guidance. The exact native Codex exit signature remains an explicit proof gap; an authenticated-provider replay is deferred because this patch changes the supervisor signal target and neither the provider nor stdio implementation. Detached-process kernel proof plus the existing Gateway process/recovery harnesses establishes the repaired boundary.

For direct inspection, the public dependency revision is available at Codex stdio signal handling and stdin closure. Availability was verified through the upstream commit API.

Sanitized observations from the completed native systemd 255 run:

Policy and action Observed result
control-group stop during work Detached child received SIGTERM before completion; return 143; parent saw closed pipe.
mixed stop during work Parent received SIGTERM; child completed afterward with return 0 and no child SIGTERM.
mixed actual restart Original child completed and parent exited before a new MainPID started.
mixed stop timeout Uncooperative parent and child both removed after the shortened two-second fixture deadline (2.212 s).
mixed parent crash Parent and lingering detached child both removed (0.007 s).
mixed ordinary stop Parent exited and lingering detached child was removed (0.022 s).

The fixture asserted distinct parent/child session IDs and process groups. Final cleanup reported zero owned transient units and zero running observed process IDs, including the restart successor. The Testbox wrapper reported exitCode=0, runStatus=succeeded, then stopped its lease. Its later external-portal synchronization warning did not change the proof result. These observations are synthetic process evidence; no claim about the reporter's exact exit code is added.

@steipete
fix(gateway): preserve child turns during systemd drain
b1d9cde 
Keep child runtimes alive during cooperative drain while retaining final cgroup cleanup. Detect older effective unit settings and document explicit service repair. Clarify that recovery cleanup requires stopped database readers as well as writers. Addresses #140479 and #140467.
@clawsweeper

clawsweeper Bot commented Sep 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation gateway Gateway runtime app: linux plugin: linux-node size: S maintainer Maintainer-authored PR labels Sep 7, 2026
@steipete
steipete marked this pull request as ready for review September 7, 2026 01:15
@clawsweeper clawsweeper Bot added P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 7, 2026
@clawsweeper

clawsweeper Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Codex review: blocked before merge. Reviewed September 6, 2026, 9:21 PM ET / September 7, 2026, 01:21 UTC.

ClawSweeper review

What this changes

This PR lets Linux Gateway child processes finish during shutdown drain, reports incompatible service settings, and documents stopping database readers before recovery cleanup.

Merge readiness

⛔ Blocked before merge - 1 item remains

The fix remains necessary: current main and v2026.9.2 still generate control-group shutdown behavior. No blocking defect was found in the introduced changes.

Priority: P1
Reviewed head: b1d9cde423b9dd70292fdb189f257624b6c7e740

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused lifecycle repair with relevant native supervisor observations, regression coverage, and explicit upgrade guidance.
Proof confidence 🐚 platinum hermit (4/6) ✨ media proof bonus Sufficient (linked_artifact): The captured native systemd 255 observations exercise the changed supervisor policy with piped children: mixed permits completion during stop and restart while timeout, crash, and final cleanup still remove descendants. The backing artifact was unavailable to this reviewer; the evidence does not claim an authenticated Codex reproduction.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (linked_artifact): The captured native systemd 255 observations exercise the changed supervisor policy with piped children: mixed permits completion during stop and restart while timeout, crash, and final cleanup still remove descendants. The backing artifact was unavailable to this reviewer; the evidence does not claim an authenticated Codex reproduction.
Evidence reviewed 11 items Introduced scope: Host-verified introduction evidence pins the reviewed changes to 2413c5e..b1d9cde. Base-only changes were excluded from this review; the verified test merge retains that distinction.
Current-main necessity: Fetched main still renders KillMode=control-group alongside TimeoutStopSec=330; the proposed supervisor repair is not already present.
Latest-release check: The supplied latest release, v2026.9.2, also renders KillMode=control-group.
Findings None None.
Security None None.

How this fits together

Linux systemd services supervise the Gateway and its child runtimes. Stop and restart requests must let the Gateway drain active work before systemd removes remaining processes.

flowchart TD
  A[Stop or restart request] --> B[systemd service]
  B --> C[Signal Gateway first]
  C --> D[Drain active child work]
  D --> E[Gateway exits or deadline expires]
  E --> F[Clean up remaining processes]
  G[Doctor service audit] --> B
Loading

Before merge

  • Resolve merge risk (P1) - The reported native proof establishes supervisor behavior with synthetic children, not the reporter's exact Codex clean-exit symptom; the backing run and cited historical Codex revision were unavailable for independent inspection.
Agent review details

Security

None.

PR surface

Source -3, Tests +18, Docs +20. Total +35 across 10 files.

View PR surface stats
Area Files Added Removed Net
Source 2 15 18 -3
Tests 2 23 5 +18
Docs 6 27 7 +20
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 10 65 30 +35

Review metrics

Metric Value Why it matters
Supervisor default 1 stop-policy default changed; 330-second deadline retained The repair changes who receives the initial stop signal while preserving bounded final cleanup.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Keep Gateway-owned drain, systemd-owned final cleanup, effective-setting diagnostics, and explicit profile-specific reinstall guidance for existing units.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Keep Gateway-owned drain, systemd-owned final cleanup, effective-setting diagnostics, and explicit profile-specific reinstall guidance for existing units.

Do we have a high-confidence way to reproduce the issue?

Yes for the supervisor defect: current-main service settings and systemd's documented signal behavior establish immediate child signaling during stop. This review did not execute a reproduction or establish the reporter's exact Codex exit sequence.

Is this the best way to solve the issue?

Yes. Using the supervisor's existing mixed mode is a narrow repair that preserves final cleanup; the cleanup documentation follows the reporter's accepted alternative without changing storage safeguards.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 0cd1ca7552a0.

Labels

Label changes:

  • add P1: The linked current-release report describes managed restarts interrupting active agent work before the advertised drain window.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The captured native systemd 255 observations exercise the changed supervisor policy with piped children: mixed permits completion during stop and restart while timeout, crash, and final cleanup still remove descendants. The backing artifact was unavailable to this reviewer; the evidence does not claim an authenticated Codex reproduction.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): The captured native systemd 255 observations exercise the changed supervisor policy with piped children: mixed permits completion during stop and restart while timeout, crash, and final cleanup still remove descendants. The backing artifact was unavailable to this reviewer; the evidence does not claim an authenticated Codex reproduction.

Label justifications:

  • P1: The linked current-release report describes managed restarts interrupting active agent work before the advertised drain window.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): The captured native systemd 255 observations exercise the changed supervisor policy with piped children: mixed permits completion during stop and restart while timeout, crash, and final cleanup still remove descendants. The backing artifact was unavailable to this reviewer; the evidence does not claim an authenticated Codex reproduction.
  • proof: sufficient: Contributor real behavior proof is sufficient. The captured native systemd 255 observations exercise the changed supervisor policy with piped children: mixed permits completion during stop and restart while timeout, crash, and final cleanup still remove descendants. The backing artifact was unavailable to this reviewer; the evidence does not claim an authenticated Codex reproduction.

Evidence

What I checked:

  • Introduced scope: Host-verified introduction evidence pins the reviewed changes to 2413c5e..b1d9cde. Base-only changes were excluded from this review; the verified test merge retains that distinction. (b1d9cde423b9)
  • Current-main necessity: Fetched main still renders KillMode=control-group alongside TimeoutStopSec=330; the proposed supervisor repair is not already present. (src/daemon/systemd-unit.ts:97, 0cd1ca7552a0)
  • Latest-release check: The supplied latest release, v2026.9.2, also renders KillMode=control-group. (src/daemon/systemd-unit.ts:97, 3928bad9badf)
  • Supervisor contract: The official systemd v255 kill contract specifies that mixed signals the main process first, then kills remaining cgroup processes after main-process exit or the stop deadline. Omitted KillMode defaults to control-group. (man/systemd.kill.xml:57)
  • Gateway lifecycle ordering: The run loop waits for active work before calling server.close on both restart and ordinary stop paths. Changing the supervisor signal target therefore preserves the existing drain owner rather than adding competing shutdown orchestration. (src/cli/gateway-cli/run-loop.ts:802, b1d9cde423b9)
  • Effective settings and upgrade behavior: The audit uses systemctl show before falling back to the base unit. Added cases cover conflicting base/drop-in modes and omitted settings. Existing Doctor code preserves Linux service definitions during ordinary updates, while installation rewrites and activates the unit; the new documentation accurately requires explicit reinstall and separate drop-in inspection. (src/daemon/service-audit.test.ts:605, b1d9cde423b9)

Likely related people:

  • Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete
steipete merged commit 9ae24df into main Sep 7, 2026
259 of 260 checks passed
@steipete
steipete deleted the codex/restart-drain-codex branch September 7, 2026 01:26
@steipete

steipete commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

This was referenced Sep 7, 2026
Closed
Closed
@clawsweeper clawsweeper Bot mentioned this pull request Sep 7, 2026
Closed
@github-actions github-actions Bot mentioned this pull request Sep 7, 2026
Open
vincentkoc added a commit that referenced this pull request Sep 7, 2026
@vincentkoc
Merge commit '8a540c252002878f3c3e9d8dfb837b83c8602a78' into temp/pr-…
9c19c1e 
…137098

* commit '8a540c252002878f3c3e9d8dfb837b83c8602a78': (2210 commits)
  perf(plugins): keep bundled catalog lookups lightweight (#140597)
  refactor: streamline skills JSON serialization (#140581)
  fix(feishu): expand quoted merged-forward messages (#136231)
  fix(talk): keep opaque realtime routes out of public config (#129144)
  fix(ci): avoid Linux packaging on pull requests (#140589)
  fix(usage): release completed request payloads (#140588)
  fix(tooling): omit Trae machine squash credit (#140562)
  fix(ui): stop catalog paging in hidden tabs (#140583)
  fix(portals): reach IPv6 loopback with IPv4-only localhost records (#140580)
  fix(transcripts): preserve fixed and legacy capture history (#140563)
  fix(ui): prevent Talk E2E audio capture stalls (#138803)
  fix(media): release completed document source buffers (#140561)
  fix(anthropic): preserve resumed CLI caches across Git changes (#140566)
  chore(ui): refresh control ui locales (#140586)
  fix(ui): keep usage exports bound to their session instance (#140573)
  refactor(exa): test response readers through tool execution (#140572)
  fix(macos): omit incompatible worker binaries from app bundles (#140557)
  fix: avoid retaining discarded tool output (#140570)
  refactor(gateway): share placement move matching (#140569)
  fix(gateway): preserve child turns during systemd drain (#140568)
  ...

# Conflicts:
#	.agents/skills/release-openclaw-maintainer/SKILL.md
#	docs/reference/RELEASING.md
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Sep 7, 2026
@steipete
fix(gateway): preserve child turns during systemd drain (openclaw#140568
dde2d29 
)

Keep child runtimes alive during cooperative drain while retaining final cgroup cleanup. Detect older effective unit settings and document explicit service repair. Clarify that recovery cleanup requires stopped database readers as well as writers. Addresses openclaw#140479 and openclaw#140467.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

app: linux docs Improvements or additions to documentation gateway Gateway runtime maintainer Maintainer-authored PR P1 High-priority user-facing bug, regression, or broken workflow. plugin: linux-node proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@steipete