Skip to content

fix: allow detached respawn on Windows for /restart#95075

Closed
samson910022 wants to merge 1 commit into
openclaw:mainfrom
samson910022:fix/windows-restart-detached-respawn
Closed

fix: allow detached respawn on Windows for /restart#95075
samson910022 wants to merge 1 commit into
openclaw:mainfrom
samson910022:fix/windows-restart-detached-respawn

Conversation

@samson910022

@samson910022 samson910022 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Summary

On Windows (without Scheduled Task supervisor), /restart completes successfully but the gateway process is never actually restarted — the PID and uptime remain unchanged.

Root Cause

restartGatewayProcessWithFreshPid() in src/infra/process-respawn.ts returns mode: "disabled" on Windows when no OPENCLAW_WINDOWS_TASK_NAME env var is detected, forcing the run-loop into in-process restart (HTTP server close+reopen), which doesn't actually restart the process.

The update path (respawnGatewayProcessForUpdate()) already attempts detached spawn on Windows — the normal restart path should do the same.

Fix

Replace the mode: "disabled" return with a detached spawn attempt. On failure, return mode: "failed" so the run-loop can fallback to supervisor or in-process restart.

Test

  1. Run OpenClaw gateway on Windows (no Scheduled Task)
  2. Send /restart from Telegram/Discord
  3. Verify PID changes and uptime resets via /status

Closes #95072

@samson1357924
fix: allow detached respawn on Windows for /restart
63e8e65 
restartGatewayProcessWithFreshPid() previously returned mode:disabled on
Windows when no Scheduled Task markers were detected, causing /restart to
fall back to in-process restart (HTTP server close+reopen) without actually
restarting the Node.js process. The PID and uptime remained unchanged.

The update restart path (respawnGatewayProcessForUpdate()) already attempts
detached spawn on Windows. Apply the same behavior to the normal restart
path so /restart works on unmanaged Windows installs.
@openclaw-barnacle openclaw-barnacle Bot added size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 19, 2026
@clawsweeper

clawsweeper Bot commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 19, 2026, 3:47 PM ET / 19:47 UTC.

Summary
The PR changes restartGatewayProcessWithFreshPid() so unmanaged Windows /restart attempts a detached respawn instead of returning the in-process fallback.

PR surface: Source +8. Total +8 across 1 file.

Reproducibility: yes. at source level: current main returns disabled for win32 without Scheduled Task markers, and the run loop then performs the in-process restart fallback. I did not establish a live Windows gateway reproduction in this read-only review.

Review metrics: 2 noteworthy metrics.

  • Restart fallback branch: 1 changed. The only runtime branch changed flips unmanaged Windows from in-process fallback to detached full-process exit, which is upgrade and availability sensitive.
  • Opposing focused test: 1 unchanged. The PR head still contains a focused test expecting Windows without Scheduled Task markers to return disabled and avoid spawning.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #95072
Summary: This PR is the candidate fix for the open unmanaged Windows restart issue, while older restart PRs provide adjacent provenance rather than a replacement landing path.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Post redacted real Windows terminal/log/screenshot or recording proof for unmanaged /restart and /status before and after.
  • Resolve the unmanaged Windows contract by preserving the fallback or adding a health-checked handoff with focused tests.
  • Remove the unused child binding and align the focused respawn test after the contract is settled.

Proof guidance:

  • [P1] Needs real behavior proof before merge: Missing: add redacted real unmanaged-Windows proof showing /restart then /status PID/uptime before and after; updating the PR body should trigger review, or a maintainer can comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P2] Merging as-is changes shipped unmanaged Windows behavior from in-process fallback to detached child even though current source and tests call out orphan/PID-tracking safety.
  • [P1] The ordinary restart path exits on mode: "spawned" without the update path's health check, so a child that spawns but fails to bind or acquire the lock can leave the gateway down.
  • [P1] The external PR has manual test steps but no redacted after-fix Windows output, recording, screenshot, or log proving PID and uptime actually changed.

Maintainer options:

  1. Pause for Windows restart contract (recommended)
    Decide whether unmanaged Windows should keep in-process restarts or gain a full-process handoff before this branch changes the shipped safety fallback.
  2. Implement a health-checked handoff
    If maintainers want fresh PID semantics, add a Windows handoff that waits for the child to become healthy or falls back, then update focused tests and real proof.
  3. Accept detached orphan risk explicitly
    Maintainers can intentionally accept bare detached respawn risk, but that should be explicit and backed by Windows proof.

Next step before merge

  • [P2] A maintainer decision is needed before reversing the shipped unmanaged Windows fallback, and the external contributor still needs real Windows proof.

Security
Cleared: The diff changes gateway respawn control flow only and adds no dependencies, workflow permissions, downloads, publishing changes, or secret-handling paths.

Review findings

  • [P1] Health-check spawned Windows restarts before exiting — src/infra/process-respawn.ts:97
  • [P2] Remove the unused child binding — src/infra/process-respawn.ts:96
Review details

Best possible solution:

Maintainers should decide the unmanaged Windows restart contract; merge only a health-checked, tested Windows handoff with real proof, otherwise keep the shipped in-process fallback and document/status the PID semantics.

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

Yes, at source level: current main returns disabled for win32 without Scheduled Task markers, and the run loop then performs the in-process restart fallback. I did not establish a live Windows gateway reproduction in this read-only review.

Is this the best way to solve the issue?

No. A bare detached spawn is not the best fix because ordinary restarts exit without the update path's health check; if maintainers want this contract, the safer path is a health-checked Windows handoff with tests and real proof.

Full review comments:

  • [P1] Health-check spawned Windows restarts before exiting — src/infra/process-respawn.ts:97
    This branch returns mode: "spawned" for unmanaged Windows, but runGatewayLoop treats any ordinary spawned restart as a successful full-process handoff and exits without the update path's child health check. A successful spawn() is not proof the replacement gateway bound the port, so a lock, port, or startup failure can leave unmanaged Windows users with no running gateway instead of the current in-process fallback.
    Confidence: 0.88
  • [P2] Remove the unused child binding — src/infra/process-respawn.ts:96
    The new branch destructures child but never uses it. tsconfig.core.json enables noUnusedLocals, so the core type lane will reject this patch before runtime tests.
    Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against bb1043b14c46.

Label changes

Label changes:

  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Missing: add redacted real unmanaged-Windows proof showing /restart then /status PID/uptime before and after; updating the PR body should trigger review, or a maintainer can comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: This is a normal-priority platform-specific gateway lifecycle fix with limited scope but meaningful restart reliability impact.
  • merge-risk: 🚨 compatibility: The PR changes a shipped unmanaged Windows fallback and can break users relying on in-process restarts or custom PID tracking.
  • merge-risk: 🚨 availability: The new spawned path can make the parent exit without proving the replacement gateway is healthy.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Missing: add redacted real unmanaged-Windows proof showing /restart then /status PID/uptime before and after; updating the PR body should trigger review, or a maintainer can comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +8. Total +8 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 1 14 6 +8
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 14 6 +8

What I checked:

Likely related people:

  • mjamiv: Authored the merged PR that changed ordinary unmanaged gateway restarts to use the in-process fallback while preserving update respawn. (role: introduced current behavior; confidence: high; commits: 8c82df6c776f, c93d6d8daa37; files: src/infra/process-respawn.ts, src/infra/process-respawn.test.ts)
  • obviyus: Authored and landed the Scheduled Task Windows restart work that owns the managed Windows branch adjacent to this PR. (role: adjacent Windows restart contributor; confidence: high; commits: a55cd45b1d42, 05c240fad6fe; files: src/infra/process-respawn.ts, src/infra/process-respawn.test.ts, src/infra/supervisor-markers.ts)
  • Takhoffman: Posted the automerge command for the PR that established the current unmanaged in-process restart contract. (role: automerge trigger; confidence: medium; commits: c93d6d8daa37; files: src/infra/process-respawn.ts, src/infra/process-respawn.test.ts)
  • Vincent Koc: Current shallow blame points to a recent repository carry-forward commit for the respawn files, though deeper provenance points to the merged restart PRs above. (role: recent area contributor; confidence: low; commits: b470b1e21a8f; files: src/infra/process-respawn.ts, src/infra/process-respawn.test.ts, docs/cli/gateway.md)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works
  • 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.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 19, 2026
@samson1357924

samson1357924 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Closing — hotpatch testing confirmed detached respawn on unmanaged Windows causes orphaned child (parent exits, child dies). This approach is not viable. Will investigate sentinel file or schtasks-based alternative instead.

@samson1357924

samson1357924 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Closing to investigate sentinel file or schtasks-based alternative instead.

@samson1357924

samson1357924 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

⚠️ This PR is being closed in favor of a sentinel file or schtasks-based approach. The detached respawn fix here re-introduces a known unsafe behavior on Windows. Please see issue #95072 for the correct fix direction.

This was referenced Jun 19, 2026
Open
Open
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

merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fix: Windows /restart falls back to in-process restart without changing PID

2 participants

@samson910022 @samson1357924