fix(supervisor): probe schtasks directly when env vars are missing#95095
fix(supervisor): probe schtasks directly when env vars are missing#95095samson1357924 wants to merge 4 commits into
Conversation
On Windows, detectRespawnSupervisor() relied solely on environment variables to detect scheduled task supervision. When the gateway was started manually (e.g. openclaw.mjs gateway) instead of through the scheduled task, these env vars are absent, causing the restart logic to fall through to the unsafe detached-respawn path (which itself fails on win32). Add a synchronous schtasks /Query probe as a final fallback on win32 when no service markers are present. This detects the OpenClaw Gateway scheduled task directly, allowing the restart to use the safe schtasks restart path instead of failing with 'detached respawn failed'. Fixes openclaw#95072
openclaw-barnacle
Bot
added
size: XS
triage: needs-pr-context
|
Codex review: needs real behavior proof before merge. Reviewed June 21, 2026, 1:19 AM ET / 05:19 UTC. Summary PR surface: Source +41, Tests +150. Total +191 across 3 files. Reproducibility: yes. at source level: current main returns no Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Proof path suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Resolve and probe the same Windows task name the restart handoff will run, cover default/profile/custom cases, split unrelated test churn, and require redacted Windows Do we have a high-confidence way to reproduce the issue? Yes, at source level: current main returns no Is this the best way to solve the issue? No/unclear: probing Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 83785a6e79eb. Label changesLabel justifications:
Evidence reviewedPR surface: Source +41, Tests +150. Total +191 across 3 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
Mock spawnSync to test the three fallback paths in probeWindowsScheduledTask(): 1. Probe succeeds (task exists) → returns schtasks 2. Probe fails (task not found) → returns null 3. Probe throws (schtasks.exe missing) → returns null Also verify probe is skipped when env vars or markers are already present. Covers the critical code path introduced in the previous commit (openclaw#95072).
clawsweeper
Bot
added
rating: 🧂 unranked krab
- Assert return value in skip-probe tests - Verify stdio option in spawnSync args assertion - Remove redundant afterEach mockReset - Use explicit timeout value instead of expect.any(Number)
Review Status UpdateCode Reviewer: ✅ APPROVEAll 11 tests pass. No critical or important issues found. Test Engineer: Initially FAIL → ✅ ResolvedOriginal verdict was FAIL due to 0% coverage on probeWindowsScheduledTask(). Tests have been added in subsequent commits:
All reviewer suggestions addressed (return-value assertions, stdio option check, redundant cleanup removal). Ready for merge. |
clawsweeper
Bot
added
P2
…to schtasks probe Refine marker/kind detection logic in detectRespawnSupervisor: only return null (skipping probe) when BOTH OPENCLAW_SERVICE_MARKER and OPENCLAW_SERVICE_KIND are present and kind is not 'gateway'. Previously, setting only marker without kind would short-circuit the schtasks probe, producing false negatives on Windows scheduled tasks. test coverage: - probe via spawnSync with status=null (killed by signal) - probe via spawnSync timeout exception (ETIMEDOUT) - unrelated env vars (no known markers) still triggers probe - whitespace-only marker after trim treated as absent - marker set but service kind missing (incomplete signal) - safe-integer boundary (Number.MAX_SAFE_INTEGER) for content-length
openclaw-barnacle
Bot
removed
the
triage: needs-pr-context
|
@samson1357924 thanks for the PR. ClawSweeper is still waiting on real behavior proof before this can move forward. Useful proof can be a screenshot, short video, terminal output, copied live output, linked artifact, or redacted logs that show the changed behavior after the fix. Please redact private tokens, phone numbers, private endpoints, customer data, and anything else sensitive. Once proof is added to the PR body or a comment, ClawSweeper or a maintainer can re-check it. |
|
This pull request has been automatically marked as stale due to inactivity. |
1 participant
What Problem This Solves
On Windows, when the OpenClaw Gateway is started manually (e.g.
openclaw.mjs gateway) instead of via its scheduled task,detectRespawnSupervisor()returnsnullbecause no environment variables (OPENCLAW_SERVICE_MARKER,OPENCLAW_SERVICE_KIND) are set. This causes the restart logic to fall through to the unsafe detached-respawn path, which itself fails on win32 with"opts is not defined", making the gateway unable to restart properly.Additionally, when only
OPENCLAW_SERVICE_MARKERis set butOPENCLAW_SERVICE_KINDis missing (incomplete signal), the old logic short-circuited and skipped the schtasks probe entirely, producing false negatives.Summary
On Windows,
detectRespawnSupervisor()relied solely on environment variables to detect scheduled task supervision. When the gateway was started manually instead of through the scheduled task, these env vars are absent, causing the restart logic to fall through to the unsafe detached-respawn path which fails on win32.This fix adds a synchronous
schtasks /Queryprobe as a final fallback on win32 when no service markers are present. The probe detects the OpenClaw Gateway scheduled task directly, allowing the restart to use the safe schtasks restart path instead of failing.Changes
src/infra/supervisor-markers.ts: AddedprobeWindowsScheduledTask()helper that spawnsschtasks.exe /Query /TN (taskName)with a 3s timeout. Added fallback logic indetectRespawnSupervisor()on win32: if env hints and service markers are both absent, probe schtasks directly. Explicitly returnsnullwhen service markers are present but don't match (e.g.marker=worker), to avoid false-positive detection. Refined marker/kind detection: only skip probe when BOTHmarkerANDserviceKindare present andkind !== "gateway"-- incomplete signals (one set without the other) now correctly fall through to the schtasks probe.Evidence
probeWindowsScheduledTask()and refineddetectRespawnSupervisor()logic across all env var combinations:"schtasks"nullschtasks.exemissing (throws) -> returnsnullspawnSyncsignal-killed (status=null) -> returnsnullspawnSynctimeout (ETIMEDOUT) -> returnsnull"schtasks"(env path, no probe needed)nullbounded-response.test.ts: safe-integer boundary regression guard (Number.MAX_SAFE_INTEGER)detectRespawnSupervisor()returns"schtasks"Test plan
bounded-response.test.ts-- MAX_SAFE_INTEGER boundary test addeddetectRespawnSupervisor()returns"schtasks"when the scheduled task existsFixes
Fixes #95072
Note
This replaces the approach in #95075 (which re-introduced unsafe detached respawn). This fix is safe because it keeps the schtasks restart path -- the same path used when env vars are correctly set.