diff --git a/src/cli/index.ts b/src/cli/index.ts index 940201ab21..aea63f9af2 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -216,6 +216,17 @@ async function handleStart(options: { block?: boolean } = {}) { const requestedPort = parsePortOption(); const owner = await findProxyOwnerBeforeJournalRecovery(); if (owner.live) { + // Service-wrapper context (opencodex-service.cmd `:loop`): a healthy proxy from + // ANY source means the requested port is already served. Exit 0 so the wrapper's + // `if %ERRORLEVEL% NEQ 0` retry loop terminates instead of respawning every 5s + // against a listener it can never claim (observed as an endless + // "Proxy already running" service.log loop). + // Only the exact "1" sentinel takes this path — the same check syncCleanup + // uses — so an env value like "0" or "false" cannot bypass the conflict error. + if (process.env.OCX_SERVICE === "1") { + console.log(`Proxy already running (PID ${owner.live.pid ?? owner.pidSnapshot ?? "unknown"}, port ${owner.live.port}); service wrapper staying out of the way.`); + process.exit(0); + } console.error(`⚠️ Proxy already running (PID ${owner.live.pid ?? owner.pidSnapshot ?? "unknown"}, port ${owner.live.port}). Use 'ocx stop' first.`); process.exit(1); } diff --git a/src/service.ts b/src/service.ts index 668f165202..03cc8749ff 100644 --- a/src/service.ts +++ b/src/service.ts @@ -24,6 +24,7 @@ import { ELEVATION_REQUEST_TIMEOUT_MS, OCX_ELEVATED_PROTOCOL_FAILED, raceWithTimeout, + resolveTrustedWindowsPowerShellExe, resolveTrustedWindowsSchtasksExe, startElevatedSchtasksCreateAndRun, runWindowsElevated, @@ -2164,6 +2165,50 @@ export function stopWindows(): void { } function statusWindows(): string { try { return schtasks(["/query", "/tn", TASK]); } catch { return ""; } } function statusWindowsXml(): string { try { return schtasks(["/query", "/tn", TASK, "/xml"]); } catch { return ""; } } + +/** + * Best-effort termination of surviving Windows scheduler launcher/wrapper processes. + * `schtasks /end` ends the task instance but often leaves wscript/cmd running the + * `:loop` batch, which brings the proxy back during a stop or restart. Same killer + * the update job uses, so both teardown paths share the guarantee. + * + * Matching is scoped to the CANONICAL paths of THIS installation (opencodex-service.cmd + * and opencodex-service-launcher.vbs under the current config dir), never a bare + * filename: a wrapper from another OpenCodex home — or an unrelated process whose + * command line merely contains the filename — must not be force-terminated. + * The path must appear as a COMPLETE command-line token (wscript.exe spawns the + * .vbs as an argument; cmd.exe /c runs the .cmd), so a substring-only match is + * excluded. + */ +function killWindowsServiceWrapperProcesses(): void { + if (process.platform !== "win32") return; + try { + const script = windowsServiceScriptPath(); + const launcher = windowsLauncherVbsPath(); + // Quote for PowerShell: single-quote the value and double any embedded quote. + const quote = (value: string) => `'${value.replace(/'/g, "''")}'`; + const ps = [ + `$pats = @(${quote(script)}, ${quote(launcher)});`, + "Get-CimInstance Win32_Process | Where-Object {", + " if ($_.ProcessId -eq $PID) { return $false };", + " $c = $_.CommandLine; if (-not $c) { return $false };", + " foreach ($p in $pats) {", + " $i = $c.IndexOf($p, [System.StringComparison]::OrdinalIgnoreCase);", + " if ($i -lt 0) { continue };", + " $before = if ($i -gt 0) { $c.Substring($i - 1, 1) } else { ' ' };", + " $end = $i + $p.Length;", + " $after = if ($end -lt $c.Length) { $c.Substring($end, 1) } else { ' ' };", + " if ($before -match '[\\s\"'']' -and $after -match '[\\s\"'']') { return $true };", + " };", + " $false", + "} | ForEach-Object { Stop-Process -Id $_.ProcessId -Force -ErrorAction SilentlyContinue }", + ].join(" "); + spawnSync(resolveTrustedWindowsPowerShellExe(), [ + "-NoProfile", "-NoLogo", "-NonInteractive", "-WindowStyle", "Hidden", + "-Command", ps, + ], { stdio: "ignore", timeout: 5000, windowsHide: true }); + } catch { /* best-effort */ } +} function uninstallWindows(): void { const probe = probeWindowsSchedulerTask(TASK); if (probe.status === "present") { @@ -2692,6 +2737,10 @@ export function stopServiceIfInstalled(): boolean { if (statusWinswRaw() !== "nonexistent") { try { stopWinswService(); stopped = true; } catch { /* best-effort */ } } + // `schtasks /end` ends the task instance but the cmd `:loop` wrapper survives and + // respawns its child seconds later (issue #764), resurrecting the proxy during a + // stop or a tray restart. Kill the launcher/wrapper processes outright. + killWindowsServiceWrapperProcesses(); if (stopped) return true; } else if (process.platform === "linux" && isSystemd() && existsSync(unitPath())) { try { stopSystemd(); return true; } catch { return false; } diff --git a/tests/cli-ready.test.ts b/tests/cli-ready.test.ts index bb62320c54..9e02e5f6ac 100644 --- a/tests/cli-ready.test.ts +++ b/tests/cli-ready.test.ts @@ -835,3 +835,58 @@ describe("runReady production findLiveProxy deadline wiring (source-level)", () expect(readySource).toContain("verifyPidFn: () => null"); }); }); + +// ── handleStart service-wrapper exit guard (source-level) ───────────────────── +// #764 follow-up: in OCX_SERVICE context a healthy proxy from ANY source must +// end handleStart with exit 0, so the opencodex-service.cmd `:loop` wrapper +// (retry on non-zero) does not respawn every 5s against a listener it can never +// claim. Source-level pin so a future edit cannot drop the guard silently. +describe("handleStart OCX_SERVICE exit guard (source-level)", () => { + const cliSource = readFileSync(join(import.meta.dir, "../src/cli/index.ts"), "utf8"); + + test("an already-live proxy exits 0 in OCX_SERVICE context", () => { + expect(cliSource).toMatch(/process\.env\.OCX_SERVICE === "1"/); + expect(cliSource).toMatch(/process\.exit\(0\)/); + const guard = cliSource.match(/if\s*\(process\.env\.OCX_SERVICE === "1"\)\s*\{[\s\S]{0,400}?process\.exit\(0\)/); + expect(guard, "OCX_SERVICE guard must exit 0 when the port is already served").not.toBeNull(); + const nonService = cliSource.match(/Proxy already running[\s\S]{0,200}?process\.exit\(1\)/); + expect(nonService, "non-service path keeps the exit 1 conflict error").not.toBeNull(); + }); + + test("service.ts teardown kills surviving wrapper processes on stop", () => { + const serviceSource = readFileSync(join(import.meta.dir, "../src/service.ts"), "utf8"); + expect(serviceSource).toMatch(/killWindowsServiceWrapperProcesses/); + const callSite = serviceSource.match(/stopServiceIfInstalled[\s\S]{0,1200}?killWindowsServiceWrapperProcesses\(\)/); + expect(callSite, "wrapper kill must run during stopServiceIfInstalled").not.toBeNull(); + }); + + test("wrapper kill matches the canonical paths of THIS installation, not bare filenames", () => { + // Review follow-up: matching by bare filename would force-terminate a + // wrapper from another OpenCodex home (or any process whose command line + // merely contains the name). The kill must target the exact canonical + // paths windowsServiceScriptPath()/windowsLauncherVbsPath() produce. + const serviceSource = readFileSync(join(import.meta.dir, "../src/service.ts"), "utf8"); + expect(serviceSource).toMatch(/windowsServiceScriptPath\(\)/); + expect(serviceSource).toMatch(/windowsLauncherVbsPath\(\)/); + const killBody = serviceSource.match(/function killWindowsServiceWrapperProcesses\(\)[\s\S]*?\n}/); + expect(killBody, "killWindowsServiceWrapperProcesses body must exist").not.toBeNull(); + expect(killBody![0]).toContain("windowsServiceScriptPath()"); + expect(killBody![0]).toContain("windowsLauncherVbsPath()"); + // Bare wrapper filenames must NOT be the match target. + expect(killBody![0]).not.toMatch(/\$pats = @\('opencodex-service\.cmd'\)/); + }); + + test("wrapper kill requires the canonical path as a complete command-line token", () => { + // Review follow-up: a substring match could force-terminate an unrelated + // process whose command line merely contains the canonical path. The + // PowerShell filter must check token boundaries (whitespace/quote before + // and after the path), not a bare IndexOf. + const serviceSource = readFileSync(join(import.meta.dir, "../src/service.ts"), "utf8"); + const killBody = serviceSource.match(/function killWindowsServiceWrapperProcesses\(\)[\s\S]*?\n}/); + expect(killBody, "killWindowsServiceWrapperProcesses body must exist").not.toBeNull(); + expect(killBody![0]).not.toMatch(/IndexOf\(\$p, \[System\.StringComparison\]::OrdinalIgnoreCase\) -ge 0/); + expect(killBody![0]).toMatch(/Substring\(/); + expect(killBody![0]).toMatch(/before/); + expect(killBody![0]).toMatch(/after/); + }); +});