Greptile Summary

This PR fixes a visible console window during gateway restart on Windows by replacing direct cmd.exe spawning with a VBScript wrapper that uses WScript.Shell.Run with intWindowStyle = 0. The approach is sound — windowsHide: true on a cmd.exe spawn does not suppress windows created by child processes, and the VBScript technique is the standard Windows workaround for this.

Key issues found:

• Incorrect backslash escaping (restart-helper.ts:174): VBScript strings do not treat \ as an escape character, so .replace(/\\/g, "\\\\") produces double backslashes in the generated .vbs file. This works for typical local temp paths due to Windows path normalization, but is semantically wrong and will break for UNC paths (e.g. \\server\share\...).
• Off-by-one in VBScript quote scheme (restart-helper.ts:177): The template uses 4 quotes before and 7 quotes after the path. After VBScript's "" decoding this gives cmd.exe the argument ""path""", and with /s /c stripping the outer pair, cmd.exe sees "path"" — a stray trailing ". For typical batch scripts without positional parameter usage this is harmless, but it is technically incorrect.
• No error handling around the new fs.writeFile call (restart-helper.ts:182–189): Previously runRestartScript could not throw. The new VBScript file write adds a failure mode; if the write fails, the entire restart silently fails with an unhandled rejection.
• The test suite drops the metacharacter quoting test without adding equivalent coverage for paths containing spaces or special characters via the VBScript path.

Confidence Score: 3/5

• The overall fix direction is correct, but there are two concrete quoting bugs and a new unhandled failure mode that should be resolved before merging.
• The VBScript approach is the right solution for hiding child-process console windows on Windows. However, the backslash doubling is wrong for VBScript (breaks UNC paths), the "" quote count is off by one pair (trailing stray " in the cmd.exe argument), and the new async file write introduces an unhandled failure mode that didn't exist before. These are concrete, fixable bugs rather than style nits, warranting a 3.
• src/cli/update-cli/restart-helper.ts — specifically the quotedBat escaping and the quote scheme in the ws.Run line.

This is a comment left during a code review.
Path: src/cli/update-cli/restart-helper.ts
Line: 174

Comment:
**Backslash doubling is wrong for VBScript**

VBScript strings do not use `\` as an escape character — backslashes are treated literally. The `.replace(/\\/g, "\\\\")` call writes `C:\\Users\\...\\script.bat` into the `.vbs` file instead of `C:\Users\...\script.bat`. While Windows path APIs often normalise double-backslashes, this is technically incorrect and **breaks for UNC paths** (e.g. `\\server\share\script.bat` becomes `\\\\server\\share\\script.bat`, which is a different UNC root).

Only the quote-doubling step is needed for VBScript string embedding:

```suggestion
    const quotedBat = scriptPath.replace(/"/g, '""');
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/cli/update-cli/restart-helper.ts
Line: 177

Comment:
**Off-by-one in VBScript quote scheme produces a trailing stray `"` in the cmd.exe argument**

The template literal has **4 `"`** before `${quotedBat}` and **7 `"`** after it. After VBScript's `""` → `"` decoding the `Run` argument becomes:

```
cmd.exe /d /s /c ""<path>"""
```

With `cmd.exe /d /s /c`, the `/s` flag strips the **first and last** `"` of the argument string, yielding `"<path>""`. The extra trailing `"` is passed as an empty argument to the batch script. While in practice most batch scripts ignore unexpected arguments, this is still incorrect.

The correct pattern to have cmd.exe receive `"<path>"` (i.e., one `""` pair on each side in the VBScript string) needs **4 before + 5 after**:

```suggestion
      `ws.Run "cmd.exe /d /s /c """"${quotedBat}""""", 0, False`,
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/cli/update-cli/restart-helper.ts
Line: 182-189

Comment:
**No error handling around VBScript file write**

Previously, `runRestartScript` only called `spawn` (synchronous, never throws). Adding `await fs.writeFile(...)` introduces a new failure mode: if the write fails (disk full, permissions), the function rejects and the gateway restart silently fails. Consider wrapping the Windows path in a try/catch to fall back gracefully, similar to how `prepareRestartScript` already does:

```typescript
try {
  await fs.writeFile(vbsPath, vbsContent, "utf8");
} catch {
  // Fall back: run the bat directly (console flash may be visible but restart succeeds)
  const child = spawn("cmd.exe", ["/d", "/s", "/c", scriptPath], {
    detached: true,
    stdio: "ignore",
    windowsHide: true,
  });
  child.unref();
  return;
}
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(windows): hide console window during..." | Re-trigger Greptile}

Closing this as implemented after Codex automated review.

Current main already removed the reported Windows update-restart netstat | findstr batch path and keeps the restart handoff detached/hidden with windowsHide: true. The current implementation uses a guarded .cmd wrapper with embedded PowerShell, bounded Scheduled Task stop/run calls, log capture, and tests that explicitly reject the old findstr/netstat -ano | script shape. PR #54120’s VBScript approach was useful against the older helper but is now obsolete on main.

Best possible solution:

Close PR #54120 as implemented on main and keep the current PowerShell-backed Windows restart helper. If users still reproduce a console flash after a release containing d1e5f4bd3c62f7b6d38717a2036a2fc56a199475, handle it as a fresh Windows restart bug with current logs and reproduction details rather than merging this older VBScript branch.

What I checked:

• Current Windows restart helper: Windows update restarts now generate an openclaw-restart-*.cmd wrapper that invokes embedded PowerShell, logs to the gateway restart log, uses bounded schtasks.exe calls via System.Diagnostics.ProcessStartInfo with UseShellExecute = false, and polls listeners with Get-NetTCPConnection plus a parsed netstat.exe -ano -p tcp fallback rather than the old batch findstr pipeline. (src/cli/update-cli/restart-helper.ts:152, 06d409dc2738)
• Detached hidden handoff remains covered: runRestartScript() still launches the prepared Windows restart script through cmd.exe /d /s /c with detached: true, stdio: "ignore", and windowsHide: true, using quoteCmdScriptArg() for metacharacter-safe paths. (src/cli/update-cli/restart-helper.ts:352, 06d409dc2738)
• Regression tests reject old visible-console path: The Windows restart-helper tests assert ordering for the guarded PowerShell/Scheduled Task path and explicitly assert the generated script does not contain findstr, netstat -ano |, or direct batch schtasks /End /TN commands. Separate tests verify the Windows spawn uses windowsHide: true and quotes metacharacter paths. (src/cli/update-cli/restart-helper.test.ts:71, 06d409dc2738)
• Fixing history: Commit d1e5f4bd3c62f7b6d38717a2036a2fc56a199475 replaced the older Windows batch restart script that used netstat -ano | findstr polling and direct schtasks batch commands with the current PowerShell-backed implementation and updated tests. (src/cli/update-cli/restart-helper.ts:142, d1e5f4bd3c62)
• PR review context: PR fix(windows): hide console window during gateway restart #54120 targeted the older runRestartScript()/batch behavior with a VBScript wrapper. The discussion usefully identified VBScript quoting, fallback, and error-handling concerns, but current main no longer has the findstr batch script that motivated the PR, so merging this branch would duplicate and partially bypass newer restart-helper hardening. (538f310fe4f7)

So I’m closing this as already implemented rather than keeping a duplicate issue open.

Codex Review notes: model gpt-5.5, reasoning high; reviewed against 06d409dc2738; fix evidence: commit d1e5f4bd3c62.