Skip to content

fix(windows): remove findstr from scheduled-task restart probe (Fixes #84600)#91273

Closed
deepujain wants to merge 1 commit into
openclaw:mainfrom
deepujain:fix/84600-hide-heartbeat-findstr-window
Closed

fix(windows): remove findstr from scheduled-task restart probe (Fixes #84600)#91273
deepujain wants to merge 1 commit into
openclaw:mainfrom
deepujain:fix/84600-hide-heartbeat-findstr-window

Conversation

@deepujain

@deepujain deepujain commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #84600.

The Windows scheduled-task restart helper still probed task state with a PowerShell command piped into findstr /I /C:"Running". That leaves a visible cmd.exe -> findstr.exe child process shape on some Windows hosts, which matches the black heartbeat/restart window reported in the issue.

This change keeps the same retry semantics but replaces the findstr pipeline with a pure PowerShell exit-code check:

  • exit 0 when the scheduled task is already running
  • exit 1 otherwise

Non-goals:

  • no scheduled-task retry timing changes
  • no service install/startup behavior changes
  • no fallback-launcher changes

Tests and validation

node scripts/run-vitest.mjs src/infra/windows-task-restart.test.ts
git diff --check

Real behavior proof

Behavior addressed:

  • the generated Windows scheduled-task restart helper should not shell task-state detection through findstr

Environment tested:

  • local source checkout on current main-based branch

Evidence after the fix:

  • src/infra/windows-task-restart.test.ts now verifies the generated helper script uses a PowerShell-only task-state probe and explicitly does not contain findstr
  • the quoting case with an apostrophe in the task name stays covered

Observed result:

  • the helper script now probes Get-ScheduledTask(...).State directly and redirects PowerShell output, removing the findstr subprocess from the helper path

What was not tested:

  • a live Windows scheduled-task restart on a real host

Proof limitations:

  • I do not have a live Windows Task Scheduler environment in this workspace, so the meaningful proof here is the generated-script regression test over the exact helper content

Risk

Low. This keeps the existing retry loop and scheduled-task commands intact. The only behavior change is how the helper decides whether the task is already running before attempting /Run.

@deepujain
fix(windows): remove findstr from scheduled-task restart probe
a1aa159 
Use a pure PowerShell task-state exit-code check so the scheduled-task restart helper no longer shells through findstr.

Fixes openclaw#84600
@deepujain ChatGPT Codex Connector

deepujain commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Focused fix for the exact helper path from #84600 is up.

Validation on the branch:

  • node scripts/run-vitest.mjs src/infra/windows-task-restart.test.ts
  • git diff --check

I kept the change narrow to the scheduled-task restart probe, preserved the existing retry loop, and added coverage that the generated helper script no longer shells through findstr.

@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 8, 2026
@clawsweeper

clawsweeper Bot commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 30, 2026, 2:41 PM ET / 18:41 UTC.

Summary
The branch replaces the Windows scheduled-task restart helper's findstr pipeline with a PowerShell-only task-state exit-code check and updates generated-script tests.

PR surface: Source +2, Tests +2. Total +4 across 2 files.

Reproducibility: yes. for source-level reproduction: current main and v2026.6.11 still generate the exact PowerShell-to-findstr helper line reported. I did not run a live Windows Task Scheduler reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

  • Windows restart probe: 1 runtime task-state probe changed. This is the user-visible Windows restart decision point, so generated-script assertions do not settle live windowing and restart behavior.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #84600
Summary: This PR is the focused candidate fix for the canonical open issue about the visible or hanging Windows scheduled-task restart findstr probe; broader Windows launcher and handoff items overlap but do not supersede it.

Members:

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

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦞 diamond lobster
Result: blocked until real behavior proof from a real setup 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 Windows Task Scheduler, terminal, or desktop proof showing no cmd/findstr window and a successful Gateway restart.
  • Update the PR body after adding proof so ClawSweeper can re-review automatically; if it does not, ask a maintainer to comment @clawsweeper re-review.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR provides generated-script tests and source inspection only, and explicitly says no live Windows Task Scheduler environment was tested. 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.

Proof path suggestion
A real Windows desktop or terminal proof would directly show the no-window restart behavior that unit tests cannot prove. Mantis is currently scoped to Telegram, Discord, and web UI chat proof, so it is not the right proof path for this surface.
Use maintainer screenshot/manual proof, browser or Playwright proof, Crabbox where appropriate, or normal local artifact proof instead.

Risk before merge

  • [P1] No live Windows Scheduled Task proof is present yet, so the no-window behavior and successful Gateway restart remain unproven outside generated-script tests.
  • [P1] The PR changes the helper's task-state exit-code decision point; if PowerShell quoting or Task Scheduler state behavior differs on a real host, Windows restart handoff availability could regress.

Maintainer options:

  1. Require Windows restart proof (recommended)
    Ask for redacted Windows Task Scheduler, terminal, or desktop proof showing the helper completes, no cmd/findstr window appears, and the Gateway comes back up.
  2. Accept source-only risk
    Maintainers can intentionally land the narrow source/test change without live proof, but they would own any unproven Windows restart or windowing behavior.

Next step before merge

  • [P1] The remaining blocker is contributor or maintainer live Windows proof rather than an automated code repair.

Security
Cleared: The diff only changes existing Windows command generation and tests, removes a pipeline, and adds no dependencies, workflows, secrets, lockfiles, or supply-chain surface.

Review details

Best possible solution:

Land the focused PowerShell-only probe after redacted Windows proof shows no cmd/findstr window and a successful Gateway restart.

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

Yes for source-level reproduction: current main and v2026.6.11 still generate the exact PowerShell-to-findstr helper line reported. I did not run a live Windows Task Scheduler reproduction in this read-only review.

Is this the best way to solve the issue?

Yes, this is the best narrow fix if live proof passes: it preserves retry, cleanup, and fallback shape while removing findstr. The broader rewrite at #92726 is closed unmerged, and #95480 fixed an adjacent gateway.cmd launch path rather than this exact probe.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a normal-priority Windows Gateway restart bug with a narrow platform-specific surface and limited blast radius.
  • merge-risk: 🚨 availability: The PR changes the Windows restart helper's live task-state decision point, and a bad command or exit-code result could leave restart handoff unreliable.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦞 diamond lobster.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR provides generated-script tests and source inspection only, and explicitly says no live Windows Task Scheduler environment was tested. 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 +2, Tests +2. Total +4 across 2 files.

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

What I checked:

Likely related people:

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.
Review history (1 earlier review cycle)
  • reviewed 2026-06-21T16:32:40.949Z sha a1aa159 :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. 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: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels Jun 8, 2026
@BradGroux BradGroux mentioned this pull request Jun 8, 2026
Draft
This was referenced Jun 13, 2026
Closed
Closed
This was referenced Jun 21, 2026
Merged
Closed
This was referenced Jun 28, 2026
Closed
Closed
@steipete steipete self-assigned this Jul 7, 2026
@openclaw-barnacle

openclaw-barnacle Bot commented Jul 7, 2026

Copy link
Copy Markdown

This assigned pull request has been automatically marked as stale after being open for 27 days.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added stale Marked as stale due to inactivity triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 7, 2026
@steipete steipete mentioned this pull request Jul 7, 2026
Merged
@steipete

steipete commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Landed the maintained replacement in #101366 as 3a482a7, which also closed #84600.

The replacement keeps the PowerShell-only task-state fix, applies it on top of current Windows restart hardening, adds exact command and apostrophe-quoting regressions, and passed full hosted CI plus native Windows Server 2025 validation. @deepujain's contribution is preserved with a Co-authored-by trailer. The original branch was too far behind current main to refresh without unrelated ancestry, so I am closing this superseded PR.

Thank you, @deepujain!

@steipete steipete closed this Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: XS stale Marked as stale due to inactivity status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. 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.

Bug: Windows heartbeat cmd window not hidden - 'findstr /I /C:"Running"' stays visible

2 participants

@deepujain @steipete