Skip to content

Commit b0dee82

Browse files
committed
fix(service): drop wait keyed on schtasks /end failure
Owner review: lidge-jun#764 is an /end that succeeds while the wrapper respawns, so waiting only when /end errors cannot catch it. Keep orphan/PID cleanup and the immediate live-proxy refuse-success guard; restart-window verification stays on the separate stop-verification path.
1 parent 1df9c82 commit b0dee82

3 files changed

Lines changed: 23 additions & 17 deletions

File tree

src/service.ts

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1494,8 +1494,6 @@ async function installWindowsNative(): Promise<void> {
14941494
writeServiceInstallState("native");
14951495
}
14961496
function startWindows(): void { schtasks(["/run", "/tn", TASK]); }
1497-
/** Batch wrapper cooldown after a failed child exit (`ping -n 6` ≈ 5s). */
1498-
export const WINDOWS_SCHEDULER_WRAPPER_RESTART_MS = 6500;
14991497

15001498
export function isWindowsSchedulerEndBenign(error: unknown): boolean {
15011499
const detail = schtasksErrorDetail(error).toLowerCase();
@@ -1504,13 +1502,19 @@ export function isWindowsSchedulerEndBenign(error: unknown): boolean {
15041502
|| detail.includes("0x41330");
15051503
}
15061504

1507-
/** End the scheduler task; false when `/end` failed for a reason other than "not running". */
1508-
export function stopWindows(): boolean {
1505+
/**
1506+
* End the scheduler task. "Already stopped" is success; other `/end` failures are
1507+
* swallowed so callers can still run tracked-proxy + live-proxy cleanup.
1508+
*
1509+
* Do not key a restart-window wait on `/end` failure: the #764 case is an `/end`
1510+
* that *succeeds* while the wrapper survives and respawns. That verification lives
1511+
* on the stop-verification path (poll across the restart window), not here.
1512+
*/
1513+
export function stopWindows(): void {
15091514
try {
15101515
schtasks(["/end", "/tn", TASK]);
1511-
return true;
15121516
} catch (error) {
1513-
return isWindowsSchedulerEndBenign(error);
1517+
if (isWindowsSchedulerEndBenign(error)) return;
15141518
}
15151519
}
15161520
function statusWindows(): string { try { return schtasks(["/query", "/tn", TASK]); } catch { return ""; } }
@@ -2030,19 +2034,17 @@ export async function serviceCommand(...args: (string | undefined)[]): Promise<v
20302034
ops.start();
20312035
console.log("✅ service started.");
20322036
break;
2033-
case "stop":
2037+
case "stop": {
20342038
assertServiceEnvironmentMatchesInstall();
20352039
// Only stop what is actually installed. The unguarded call ran a real `launchctl unload`
20362040
// (and its Windows/Linux twins) even with nothing installed.
2037-
let schedulerEndOk = true;
20382041
if (ops.status() !== null || isServiceInstalled()) {
2039-
if (process.platform === "win32" && backend === "scheduler") schedulerEndOk = stopWindows();
2040-
else ops.stop();
2042+
ops.stop();
20412043
}
20422044
await stopTrackedProxyForServiceCommand();
2043-
if (process.platform === "win32" && backend === "scheduler" && !schedulerEndOk) {
2044-
await Bun.sleep(WINDOWS_SCHEDULER_WRAPPER_RESTART_MS);
2045-
}
2045+
// Refuse success while a proxy still answers. Restart-window polling for the
2046+
// succeed-then-respawn scheduler case is owned by the separate stop-verification
2047+
// work; this keeps the immediate-survivor guard for orphan/PID cleanup paths.
20462048
if (await findLiveProxy({ timeoutMs: 1500 })) {
20472049
console.error("❌ Service stop did not terminate the proxy — it is still running. Check `ocx service status` and the service log.");
20482050
process.exit(1);
@@ -2058,6 +2060,7 @@ export async function serviceCommand(...args: (string | undefined)[]): Promise<v
20582060
else if (!grok.ok) console.error(`⚠️ ${grok.message}`);
20592061
}
20602062
break;
2063+
}
20612064
case "status": {
20622065
if (process.platform === "win32" && backend === "scheduler") {
20632066
console.log(await inspectWindowsSchedulerServiceStatus());

tests/proxy-liveness.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ describe("findLiveProxy", () => {
140140
test("a runtime record whose healthz reports a different pid is rejected", async () => {
141141
const live = await findLiveProxy({
142142
readPidFn: () => 1111,
143+
verifyPidFn: () => null,
143144
readRuntimeFn: () => ({ port: 58195 }),
144145
configFn: () => ({ port: 58195 }),
145146
fetchFn: (async () => healthz({ ...OURS, pid: 9999 })) as typeof fetch,

tests/service.test.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -672,12 +672,14 @@ describe("service lifecycle cleanup ordering", () => {
672672
});
673673

674674

675-
test("Windows scheduler stop waits through wrapper restart when schtasks /end fails", async () => {
675+
test("Windows scheduler stop does not wait on schtasks /end failure", async () => {
676676
const service = await readText("src/service.ts");
677677
const stopCase = service.slice(service.indexOf('case "stop":'), service.indexOf('case "status":'));
678-
expect(stopCase).toContain("schedulerEndOk = stopWindows()");
679-
expect(stopCase).toContain("WINDOWS_SCHEDULER_WRAPPER_RESTART_MS");
680-
expect(stopCase).toContain("await Bun.sleep(WINDOWS_SCHEDULER_WRAPPER_RESTART_MS)");
678+
// #764 is an /end that succeeds while the wrapper respawns; waiting only when
679+
// /end errors cannot catch that path. Restart-window polling lives elsewhere.
680+
expect(stopCase).not.toContain("WINDOWS_SCHEDULER_WRAPPER_RESTART_MS");
681+
expect(stopCase).not.toContain("schedulerEndOk");
682+
expect(stopCase).not.toContain("await Bun.sleep(");
681683
});
682684

683685
test("tracked proxy cleanup verifies health-reported pids before stopProxy", async () => {

0 commit comments

Comments
 (0)