History

[codex] Fix Windows service install discovery (#1046)

Rhys Sullivan committed 4 months ago

Commit 2de18041 parent 4091f8b13 files · +294 −53Browse files

Showing the changes to apps/cli/src (3 of 13 files). Show all

Unified · Split

3 files
  1. apps/cli/src/main.ts +106 −22
  2. apps/cli/src/service.test.ts +11 −0
  3. apps/cli/src/service.ts +41 −0
apps/cli/src/main.ts+106−22
@@ -124,7 +124,12 @@ import { · 123 unmodified lines
124124 resolveExecutorDataDir,
125125 writeLocalServerManifest,
126126 } from "./local-server-manifest";
127−import { DEFAULT_SERVICE_PORT, getServiceBackend, SERVICE_LABEL } from "./service";
127+import {
128+ DEFAULT_SERVICE_PORT,
129+ getServiceBackend,
130+ SERVICE_LABEL,
131+ stopWindowsExecutorListenersOnPort,
132+} from "./service";
128133 import {
129134 defaultCliServerConnectionProfile,
130135 findCliServerConnectionProfile,
@@ -165,6 +170,7 @@ const DEFAULT_BASE_URL = `http://localhost:${DEFAULT_PORT}`; · 34 unmodified lines
165170 const DAEMON_BOOT_TIMEOUT_MS = 15_000;
166171 const DAEMON_BOOT_POLL_MS = 150;
167172 const DAEMON_STOP_TIMEOUT_MS = 10_000;
173+const SERVICE_BOOT_TIMEOUT_MS = 45_000;
168174
169175 // ---------------------------------------------------------------------------
170176 // Helpers
@@ -197,15 +203,15 @@ const readActiveLocalServerManifest = (): Effect.Effect< · 26 unmodified lines
197203 const manifest = yield* readLocalServerManifest();
198204 if (!manifest) return null;
199205
206+ if (yield* isServerReachable(manifest.connection.origin)) {
207+ return manifest;
208+ }
209+
200210 if (!isPidAlive(manifest.pid)) {
201211 yield* removeLocalServerManifestIfOwnedBy({ pid: manifest.pid }).pipe(Effect.ignore);
202212 return null;
203213 }
204214
205− if (yield* isServerReachable(manifest.connection.origin)) {
206− return manifest;
207− }
208−
209215 return yield* Effect.fail(
210216 new Error(
211217 [
@@ -2201,6 +2207,68 @@ const mcpCommand = Command.make( · 1989 unmodified lines
22012207
22022208 const supervisedServiceOrigin = (port: number): string => `http://127.0.0.1:${port}`;
22032209
2210+const LOCAL_SERVICE_HOSTS = new Set(["localhost", "127.0.0.1", "::1", "[::1]"]);
2211+
2212+const originPort = (url: URL): string => url.port || (url.protocol === "https:" ? "443" : "80");
2213+
2214+const sameServerOrigin = (left: string, right: string): boolean => {
2215+ const leftUrl = new URL(normalizeExecutorServerConnection({ origin: left }).origin);
2216+ const rightUrl = new URL(normalizeExecutorServerConnection({ origin: right }).origin);
2217+ if (leftUrl.toString() === rightUrl.toString()) return true;
2218+ return (
2219+ leftUrl.protocol === rightUrl.protocol &&
2220+ originPort(leftUrl) === originPort(rightUrl) &&
2221+ LOCAL_SERVICE_HOSTS.has(leftUrl.hostname.toLowerCase()) &&
2222+ LOCAL_SERVICE_HOSTS.has(rightUrl.hostname.toLowerCase())
2223+ );
2224+};
2225+
2226+const hasReachableCliDaemonManifest = (input: {
2227+ readonly origin: string;
2228+ readonly version: string;
2229+}): Effect.Effect<boolean, never, FileSystem.FileSystem | PlatformPath.Path> =>
2230+ Effect.gen(function* () {
2231+ const manifest = yield* readActiveLocalServerManifest().pipe(
2232+ Effect.catchCause(() => Effect.succeed(null)),
2233+ );
2234+ if (!manifest || manifest.kind !== "cli-daemon") return false;
2235+ if (!sameServerOrigin(manifest.connection.origin, input.origin)) return false;
2236+ if (manifest.owner.version !== input.version) return false;
2237+ return yield* isServerReachable(manifest.connection.origin);
2238+ });
2239+
2240+const clearUnmanifestedWindowsExecutorListener = (input: {
2241+ readonly port: number;
2242+ readonly origin: string;
2243+}): Effect.Effect<void, Error, FileSystem.FileSystem | PlatformPath.Path> =>
2244+ Effect.gen(function* () {
2245+ const active = yield* readActiveLocalServerManifest().pipe(
2246+ Effect.catchCause(() => Effect.succeed(null)),
2247+ );
2248+ if (active && sameServerOrigin(active.connection.origin, input.origin)) return;
2249+ if (!(yield* isServerReachable(input.origin))) return;
2250+
2251+ const stoppedPids = yield* stopWindowsExecutorListenersOnPort(input.port);
2252+ const stopped = yield* waitForUnreachable({
2253+ check: isServerReachable(input.origin),
2254+ timeoutMs: DAEMON_STOP_TIMEOUT_MS,
2255+ intervalMs: DAEMON_BOOT_POLL_MS,
2256+ });
2257+ if (stopped) return;
2258+
2259+ return yield* Effect.fail(
2260+ new Error(
2261+ [
2262+ `Executor is already reachable at ${input.origin}, but no live server manifest exists for it.`,
2263+ stoppedPids.length > 0
2264+ ? `Tried to stop orphaned Executor pid(s): ${stoppedPids.join(", ")}.`
2265+ : `No orphaned executor.exe listener could be stopped on port ${input.port}.`,
2266+ "Stop the process using that port and re-run the install.",
2267+ ].join("\n"),
2268+ ),
2269+ );
2270+ });
2271+
22042272 const portFromOrigin = (origin: string): number | null => {
22052273 try {
22062274 const url = new URL(origin);
@@ -2274,7 +2342,10 @@ const installService = (port: number, commandName: string) => · 67 unmodified lines
22742342 return;
22752343 }
22762344
2277− if (plan === "takeover-then-install") {
2345+ if (
2346+ plan === "takeover-then-install" ||
2347+ (backend.platform === "win32" && plan === "reinstall")
2348+ ) {
22782349 const replaced = yield* takeOverActiveLocalServer();
22792350 if (replaced) {
22802351 console.log(
@@ -2294,22 +2365,29 @@ const installService = (port: number, commandName: string) => · 13 unmodified lines
22942365 console.log("");
22952366 console.log("Writing the service definition and starting Executor...");
22962367
2368+ if (backend.platform === "win32") {
2369+ yield* clearUnmanifestedWindowsExecutorListener({ port, origin });
2370+ }
2371+
22972372 // The unit carries no secret: the supervised daemon mints/loads its bearer
22982373 // from auth.json (under EXECUTOR_DATA_DIR) on first boot, and clients read
22992374 // the same file — so reachability is the credential-free /api/health probe.
23002375 yield* backend.install({ executablePath: process.execPath, port, version: CLI_VERSION });
23012376
2302− console.log(`Waiting for Executor to become reachable at ${origin}...`);
2377+ console.log(`Waiting for Executor to publish its service manifest at ${origin}...`);
23032378 const reachable = yield* waitForReachable({
2304− check: isServerReachable(origin),
2305− timeoutMs: DAEMON_BOOT_TIMEOUT_MS,
2379+ check: hasReachableCliDaemonManifest({
2380+ origin,
2381+ version: CLI_VERSION,
2382+ }),
2383+ timeoutMs: SERVICE_BOOT_TIMEOUT_MS,
23062384 intervalMs: DAEMON_BOOT_POLL_MS,
23072385 });
23082386 if (!reachable) {
23092387 return yield* Effect.fail(
23102388 new Error(
23112389 [
2312− `Installed ${SERVICE_LABEL} but it did not become reachable at ${origin} within ${DAEMON_BOOT_TIMEOUT_MS / 1000}s.`,
2390+ `Installed ${SERVICE_LABEL} but it did not publish a reachable server manifest for ${origin} within ${SERVICE_BOOT_TIMEOUT_MS / 1000}s.`,
23132391 `Check ~/.executor/logs/daemon.error.log and \`${cliPrefix} service status\`.`,
23142392 ].join("\n"),
23152393 ),
@@ -2334,19 +2412,25 @@ const serviceInstallCommand = Command.make( · 18 unmodified lines
23342412 const serviceUninstallCommand = Command.make("uninstall", {}, () =>
23352413 Effect.gen(function* () {
23362414 const backend = getServiceBackend();
2337− const wasRunning = backend.automated
2338− ? yield* backend.status().pipe(
2339− Effect.map((status) => status.running),
2340− Effect.catchCause(() => Effect.succeed(false)),
2341− )
2342− : false;
2415+ const activeBefore = yield* readActiveLocalServerManifest().pipe(
2416+ Effect.catchCause(() => Effect.succeed(null)),
2417+ );
2418+ const servicePort = activeBefore
2419+ ? (portFromOrigin(activeBefore.connection.origin) ?? DEFAULT_SERVICE_PORT)
2420+ : DEFAULT_SERVICE_PORT;
2421+ const status = backend.automated
2422+ ? yield* backend.status().pipe(Effect.catchCause(() => Effect.succeed(null)))
2423+ : null;
23432424 yield* backend.uninstall();
2344− if (wasRunning) {
2345− const stopped = yield* takeOverActiveLocalServer({ onlyKind: "cli-daemon" });
2346− if (stopped) {
2347− console.log(
2348− `Stopped running Executor daemon at ${stopped.connection.origin} (pid ${stopped.pid}).`,
2349− );
2425+ const stopped = yield* takeOverActiveLocalServer({ onlyKind: "cli-daemon" });
2426+ if (stopped) {
2427+ console.log(
2428+ `Stopped running Executor daemon at ${stopped.connection.origin} (pid ${stopped.pid}).`,
2429+ );
2430+ } else if (backend.platform === "win32" && status?.registered) {
2431+ const stoppedPids = yield* stopWindowsExecutorListenersOnPort(servicePort);
2432+ if (stoppedPids.length > 0) {
2433+ console.log(`Stopped orphaned Executor daemon pid(s): ${stoppedPids.join(", ")}.`);
23502434 }
23512435 }
23522436 console.log("Executor background service uninstalled.");
apps/cli/src/service.test.ts+11−0
@@ -6,6 +6,7 @@ import { · 5 unmodified lines
66 generateSystemdUnit,
77 generateWindowsDaemonWrapper,
88 generateWindowsRegisterScript,
9+ generateWindowsStopExecutorListenersScript,
910 getServiceBackend,
1011 } from "./service";
1112
@@ -136,6 +137,16 @@ describe("service unit generation", () => { · 124 unmodified lines
136137 expect(script).toContain("Register-ScheduledTask -TaskName 'ExecutorDaemon'");
137138 expect(script).toContain("Start-ScheduledTask -TaskName 'ExecutorDaemon'");
138139 });
140+
141+ it("stops orphaned executor.exe listeners without shadowing PowerShell's $PID", () => {
142+ const script = generateWindowsStopExecutorListenersScript(4789);
143+
144+ expect(script).toContain("Get-NetTCPConnection -LocalPort 4789 -State Listen");
145+ expect(script).toContain("$listenerPid");
146+ expect(script).not.toContain("foreach ($pid");
147+ expect(script).toContain("[string]$process.Name -ieq 'executor.exe'");
148+ expect(script).toContain('Write-Output ("STOPPED=" + $listenerPid)');
149+ });
139150 });
140151
141152 describe("service backend dispatch", () => {
apps/cli/src/service.ts+41−0
@@ -579,6 +579,28 @@ export const generateWindowsRegisterScript = (options: { · 578 unmodified lines
579579 `Start-ScheduledTask -TaskName ${psSingleQuote(options.taskName)}`,
580580 ].join("\n");
581581
582+/**
583+ * Stop orphaned Windows `executor.exe` listeners on the service port. Task
584+ * Scheduler can report the task as stopped after terminating the `.cmd` action
585+ * while leaving the child `executor.exe` process bound to 4789. That process is
586+ * healthy enough to satisfy `/api/health`, but `executor web` cannot discover it
587+ * once its `server.json` is gone.
588+ */
589+export const generateWindowsStopExecutorListenersScript = (port: number): string =>
590+ [
591+ `$connections = @(Get-NetTCPConnection -LocalPort ${port} -State Listen -ErrorAction SilentlyContinue)`,
592+ `$listenerPids = @($connections | Select-Object -ExpandProperty OwningProcess -Unique)`,
593+ `foreach ($listenerPid in $listenerPids) {`,
594+ ` $process = Get-CimInstance Win32_Process -Filter "ProcessId=$listenerPid" -ErrorAction SilentlyContinue`,
595+ ` if ($null -eq $process) { continue }`,
596+ ` $pathName = if ($process.ExecutablePath) { [System.IO.Path]::GetFileName([string]$process.ExecutablePath) } else { '' }`,
597+ ` if ([string]$process.Name -ieq 'executor.exe' -or $pathName -ieq 'executor.exe') {`,
598+ ` Stop-Process -Id $listenerPid -Force -ErrorAction SilentlyContinue`,
599+ ` Write-Output ("STOPPED=" + $listenerPid)`,
600+ ` }`,
601+ `}`,
602+ ].join("\n");
603+
582604 /** Run a PowerShell script via -EncodedCommand (sidesteps all shell quoting). */
583605 const runPowerShell = (script: string): Effect.Effect<CommandResult, Error> =>
584606 runCommand("powershell.exe", [
@@ -590,6 +612,25 @@ const runPowerShell = (script: string): Effect.Effect<CommandResult, Error> => · 5 unmodified lines
590612 Buffer.from(script, "utf16le").toString("base64"),
591613 ]);
592614
615+export const stopWindowsExecutorListenersOnPort = (
616+ port: number,
617+): Effect.Effect<ReadonlyArray<number>, Error> =>
618+ Effect.gen(function* () {
619+ const result = yield* runPowerShell(generateWindowsStopExecutorListenersScript(port));
620+ if (result.code !== 0) {
621+ const detail = result.stderr.trim() || result.stdout.trim();
622+ return yield* Effect.fail(
623+ new Error(`Failed to inspect Executor listeners on port ${port}: ${detail}`),
624+ );
625+ }
626+ return result.stdout.split(/\r?\n/).flatMap((line) => {
627+ const match = /^STOPPED=(\d+)\s*$/.exec(line.trim());
628+ if (!match) return [];
629+ const pid = Number.parseInt(match[1], 10);
630+ return Number.isInteger(pid) && pid > 0 ? [pid] : [];
631+ });
632+ });
633+
593634 const makeWindowsBackend = (): ServiceBackend => ({
594635 platform: "win32",
595636 automated: true,