From 95c3a3e49b34aea03a983bf0029ea59026f07aed Mon Sep 17 00:00:00 2001 From: Laurin-Notemann <47634664+Laurin-Notemann@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:47:15 +0000 Subject: [PATCH] fix(otter): defer SSH launches to the background service Co-Authored-By: Claude Opus 5.5 --- docs/internals/server-updates.md | 8 ++ packages/ssh/src/tunnel.test.ts | 221 ++++++++++++++++++++++++++++++- packages/ssh/src/tunnel.ts | 73 ++++++---- 3 files changed, 270 insertions(+), 32 deletions(-) diff --git a/docs/internals/server-updates.md b/docs/internals/server-updates.md index 51bc16775d08..8b75cab1b7ad 100644 --- a/docs/internals/server-updates.md +++ b/docs/internals/server-updates.md @@ -7,6 +7,14 @@ their service definition or select their own replacement. Local service commands may replace the launcher and state while the service is stopped. Foreground CLI processes do not self-update. +SSH launch defers to service ownership: durable `runtime/service-state.json` +(including pending updates or invalid state), an installed Otter systemd unit or +launchd plist, or an enabled `otter-code.service` prevents an SSH-managed spawn. +It waits up to 120 seconds for the default home's runtime metadata and HTTP +readiness, then connects as external. A service restart gap must never create a +second writer on the same database. If the service stays down, SSH fails with +service recovery commands instead of taking over the home. + Exact-version installs keep restarts independent of npm cache eviction or a moving release tag. Installation and preflight happen in staging before publishing an immutable runtime. Preflight checks the launcher protocol because a target that diff --git a/packages/ssh/src/tunnel.test.ts b/packages/ssh/src/tunnel.test.ts index 449a71b43d6b..0f2b48bd0518 100644 --- a/packages/ssh/src/tunnel.test.ts +++ b/packages/ssh/src/tunnel.test.ts @@ -16,6 +16,7 @@ import { HttpClient, HttpClientResponse } from "effect/http"; import { ChildProcess, ChildProcessSpawner } from "effect/process"; import * as SshAuth from "./auth.ts"; +import { collectProcessOutput } from "./command.ts"; import { SshCommandError } from "./errors.ts"; import * as SshTunnel from "./tunnel.ts"; @@ -339,7 +340,7 @@ describe("ssh tunnel scripts", () => { }).pipe(Effect.provide(layerProcess)); }); - it.effect("allows cold remote launches to exceed the default SSH command timeout", () => { + it.effect("allows node-script service restarts to exceed the default SSH command timeout", () => { const target = { alias: "devbox", hostname: "devbox.example.com", @@ -347,7 +348,7 @@ describe("ssh tunnel scripts", () => { port: 2222, } as const; const spawner = ChildProcessSpawner.make(() => - Effect.succeed(makeDelayedSuccessfulProcess('{"remotePort":3774}\n', 75_000)), + Effect.succeed(makeDelayedSuccessfulProcess('{"remotePort":3774}\n', 125_000)), ); const layerSpawner = Layer.succeed(ChildProcessSpawner.ChildProcessSpawner, spawner); const layerProcess = Layer.mergeAll(NodeServices.layer, layerSpawner, TestClock.layer()); @@ -357,7 +358,7 @@ describe("ssh tunnel scripts", () => { SshTunnel.launchOrReuseRemoteServer(target, undefined, NODE_SCRIPT), ); yield* Effect.yieldNow; - yield* TestClock.adjust(Duration.seconds(75)); + yield* TestClock.adjust(Duration.seconds(125)); const result = yield* Fiber.join(fiber); assert.equal(result.remotePort, 3774); @@ -696,6 +697,220 @@ describe("ssh tunnel scripts", () => { ); }); +// Exercise the generated shell with fake helpers and a deterministic clock. +// No fixture starts a server or signals a real PID. +describe.skipIf(HostProcess.Platform.defaultValue() === "win32")( + "remote launch service ownership", + () => { + const cases = [ + { name: "active service with a ready server", marker: "active", server: "up" }, + { + name: "pending trial publishes runtime before readiness", + marker: "pending", + server: "restart", + }, + { name: "service never starts", marker: "active", server: "down" }, + { name: "enabled systemd service restarts", marker: "enabled", server: "restart" }, + { name: "installed systemd unit with no service state", marker: "unit", server: "up" }, + { name: "installed launchd plist with no service state", marker: "plist", server: "up" }, + { name: "invalid service state fails closed", marker: "invalid", server: "down" }, + { name: "host without a service spawns managed", marker: "none", server: "down" }, + { name: "host without a service reuses external", marker: "none", server: "up" }, + { + name: "ready service stops stale managed PID", + marker: "active", + server: "up", + stale: "other", + }, + { + name: "stale ownership cannot kill the service PID", + marker: "active", + server: "up", + stale: "same", + }, + { + name: "down service never reuses stale managed server", + marker: "active", + server: "down", + stale: "other", + }, + ] as const; + + it.effect.each(cases)("$name", (testCase) => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; + const root = yield* fs.makeTempDirectoryScoped({ + directory: "/tmp", + prefix: "otter-ssh-service-", + }); + const home = `${root}/home`; + const serverHome = `${home}/.otter-code`; + const stateDir = `${serverHome}/ssh-launch/fixture`; + const runtimeDir = `${serverHome}/runtime/versions/${ARCHIVE.archiveVersion}`; + const runtimeFile = `${serverHome}/userdata/server-runtime.json`; + for (const dir of [stateDir, runtimeDir, `${serverHome}/userdata`, `${root}/bin`]) { + yield* fs.makeDirectory(dir, { recursive: true }); + } + yield* fs.writeFileString(`${home}/ticks`, "0"); + yield* fs.writeFileString(`${runtimeDir}/.install-complete`, ARCHIVE.archiveVersion); + const runtime = JSON.stringify({ + pid: process.pid, + port: 4773, + origin: "http://127.0.0.1:4773", + }); + if (testCase.server === "up") { + yield* fs.writeFileString(runtimeFile, runtime); + yield* fs.writeFileString(`${home}/ready`, "4773\n"); + } else if (testCase.server === "restart") { + // A dead child's metadata must be reread, and readiness may lag it. + yield* fs.writeFileString(runtimeFile, JSON.stringify({ pid: 0, port: 4772 })); + yield* fs.writeFileString(`${home}/next-runtime`, runtime); + } + if (["active", "pending", "invalid"].includes(testCase.marker)) { + const state = + testCase.marker === "active" + ? { activeVersion: "1.2.3" } + : { update: { status: "pending" } }; + yield* fs.writeFileString( + `${serverHome}/runtime/service-state.json`, + testCase.marker === "invalid" ? "{" : JSON.stringify(state), + ); + } else if (testCase.marker === "unit" || testCase.marker === "plist") { + const unitDir = + testCase.marker === "unit" + ? `${home}/.config/systemd/user` + : `${home}/Library/LaunchAgents`; + yield* fs.makeDirectory(unitDir, { recursive: true }); + yield* fs.writeFileString( + `${unitDir}/${testCase.marker === "unit" ? "otter-code.service" : "dev.otterware.code.service.plist"}`, + "fixture", + ); + } else if (testCase.marker === "enabled") { + yield* fs.writeFileString(`${home}/enabled`, ""); + } + const stale = "stale" in testCase ? testCase.stale : undefined; + if (stale) { + yield* fs.writeFileString( + `${stateDir}/pid`, + String(stale === "same" ? process.pid : 424242), + ); + yield* fs.writeFileString(`${stateDir}/port`, "4774"); + yield* fs.writeFileString(`${stateDir}/managed`, "managed"); + } + const executable = `${runtimeDir}/t3`; + yield* fs.writeFileString( + executable, + `#!/bin/sh +set -eu +case "$1" in + --version) exit 0 ;; + __ssh-helper) + shift + case "$1" in + runtime-port) + "$T3_TEST_NODE" -e 'try { const r = JSON.parse(require("node:fs").readFileSync(process.argv[1], "utf8")); if (!r.pid) process.exit(1); process.kill(r.pid, 0); process.stdout.write(r.pid + " " + r.port); } catch { process.exit(1); }' "$2" ;; + wait-ready) + if [ "$2" = "4774" ]; then exit 0; fi + [ -f "$HOME/ready" ] && [ "$(cat "$HOME/ready")" = "$2" ] ;; + pick-port) printf '4774' ;; + *) exit 99 ;; + esac ;; + *) exit 99 ;; +esac +`, + ); + yield* fs.chmod(executable, 0o700); + // Shadow systemctl unconditionally so tests cannot query the live unit. + yield* fs.writeFileString( + `${root}/bin/systemctl`, + `#!/bin/sh +printf '%s\\n' "$*" > "$HOME/systemctl-call" +[ "$*" = "--user is-enabled otter-code.service" ] && [ -f "$HOME/enabled" ] +`, + ); + yield* fs.chmod(`${root}/bin/systemctl`, 0o700); + const fakeCommands = ` +kill() { + if [ "$1" = "-0" ]; then [ ! -f "$HOME/killed" ]; return; fi + printf '%s' "$1" > "$HOME/killed" +} +date() { cat "$HOME/ticks"; } +sleep() { + TICKS=$(($(cat "$HOME/ticks") + 1)) + printf '%s' "$TICKS" > "$HOME/ticks" + if [ "$TICKS" -eq 2 ] && [ -f "$HOME/next-runtime" ]; then + mv "$HOME/next-runtime" "$HOME/.otter-code/userdata/server-runtime.json" + fi + if [ "$TICKS" -eq 3 ] && [ "${testCase.server}" = "restart" ]; then printf '4773' > "$HOME/ready"; fi +} +nohup() { printf '%s\\n' "$*" > "$HOME/spawned"; } +`; + const script = `${root}/launch.sh`; + yield* fs.writeFileString( + script, + fakeCommands + SshTunnel.buildRemoteLaunchScript(ARCHIVE), + ); + const child = yield* spawner.spawn( + ChildProcess.make("sh", [script, "fixture"], { + env: { + HOME: home, + PATH: `${root}/bin:${process.env.PATH ?? ""}`, + T3_TEST_NODE: process.execPath, + }, + extendEnv: false, + }), + ); + const [stdout, stderr, exitCode] = yield* Effect.all( + [ + collectProcessOutput(child.stdout), + collectProcessOutput(child.stderr), + child.exitCode.pipe(Effect.map(Number)), + ], + { concurrency: "unbounded" }, + ); + const failure = testCase.marker !== "none" && testCase.server === "down"; + assert.equal(exitCode, failure ? 1 : 0, stderr); + const managed = testCase.marker === "none" && testCase.server === "down"; + assert.equal(yield* fs.exists(`${home}/spawned`), managed); + if (failure) { + assert.equal(stdout, ""); + assert.include(stderr, "This host's Otter Code service is not running"); + assert.include(stderr, "otter-code service restart"); + assert.include(stderr, "systemctl --user start otter-code.service"); + assert.equal(Number(yield* fs.readFileString(`${home}/ticks`)), 120); + } else { + assert.deepEqual(JSON.parse(stdout), { + remotePort: managed ? 4774 : 4773, + serverKind: managed ? "managed" : "external", + }); + assert.equal( + (yield* fs.readFileString(`${stateDir}/managed`)).trim(), + managed ? "managed" : "external", + ); + assert.equal(yield* fs.exists(`${stateDir}/pid`), managed); + if (testCase.server === "restart") + assert.equal(Number(yield* fs.readFileString(`${home}/ticks`)), 3); + if (managed) + assert.include( + yield* fs.readFileString(`${home}/spawned`), + `serve --host 127.0.0.1 --port 4774 --base-dir ${serverHome}`, + ); + } + assert.equal(yield* fs.exists(`${home}/killed`), stale === "other" && !failure); + if (stale === "other" && !failure) + assert.equal(yield* fs.readFileString(`${home}/killed`), "424242"); + if (testCase.marker === "enabled" || testCase.marker === "none") { + assert.equal( + (yield* fs.readFileString(`${home}/systemctl-call`)).trim(), + "--user is-enabled otter-code.service", + ); + } + }).pipe(Effect.provide(NodeServices.layer)), + ); + }, +); + // The archive runner is generated shell; string assertions cannot prove the // lock excludes concurrent installers. Run the real script against a tiny // fake archive served from a file:// mirror. diff --git a/packages/ssh/src/tunnel.ts b/packages/ssh/src/tunnel.ts index 9fb00d0bb04b..6915701196da 100644 --- a/packages/ssh/src/tunnel.ts +++ b/packages/ssh/src/tunnel.ts @@ -52,7 +52,8 @@ const SSH_READY_TIMEOUT_MS = 20_000; const SSH_READY_PROBE_TIMEOUT_MS = 1_000; const TUNNEL_SHUTDOWN_TIMEOUT_MS = 2_000; const REMOTE_READY_TIMEOUT_MS = 60_000; -const REMOTE_LAUNCH_TIMEOUT_MS = 90_000; +const REMOTE_LAUNCH_TIMEOUT_MS = 150_000; +const REMOTE_SERVICE_WAIT_SECONDS = 120; // A cold archive launch also downloads and unpacks a ~70 MB release archive // and may wait on another installer's lock. The budgets nest: the checksum // file is tiny and the archive download is bounded; a waiter outlasts both @@ -631,39 +632,52 @@ NODE REMOTE_PID="$(cat "$PID_FILE" 2>/dev/null || true)" REMOTE_PORT="$(cat "$PORT_FILE" 2>/dev/null || true)" REMOTE_MANAGED="$(cat "$MANAGED_FILE" 2>/dev/null || true)" -DEFAULT_RUNTIME_INFO="$(resolve_default_runtime_port 2>/dev/null || true)" -DEFAULT_RUNTIME_PID="" -DEFAULT_REMOTE_PORT="" -if [ -n "$DEFAULT_RUNTIME_INFO" ]; then +service_manages_default_home() { + # Durable service state reserves this home even while its child is down. + # Fail closed on unreadable/malformed state rather than risking two writers. + [ -f "$DEFAULT_SERVER_HOME/runtime/service-state.json" ] || + [ -f "$HOME/.config/systemd/user/otter-code.service" ] || + [ -f "$HOME/Library/LaunchAgents/dev.otterware.code.service.plist" ] || + { command -v systemctl >/dev/null 2>&1 && systemctl --user is-enabled otter-code.service >/dev/null 2>&1; } +} +connect_default_runtime() { + DEFAULT_RUNTIME_INFO="$(resolve_default_runtime_port 2>/dev/null || true)" + [ -n "$DEFAULT_RUNTIME_INFO" ] || return 1 DEFAULT_RUNTIME_PID="\${DEFAULT_RUNTIME_INFO%% *}" DEFAULT_REMOTE_PORT="\${DEFAULT_RUNTIME_INFO#* }" -fi -if [ -n "$DEFAULT_REMOTE_PORT" ]; then REMOTE_PORT="$DEFAULT_REMOTE_PORT" - if wait_ready "@@T3_REUSE_READY_TIMEOUT_MS@@"; then - if [ "$REMOTE_MANAGED" = "managed" ]; then - PID_TO_STOP="\${REMOTE_PID:-$DEFAULT_RUNTIME_PID}" - if [ -n "$PID_TO_STOP" ] && kill -0 "$PID_TO_STOP" 2>/dev/null; then - kill "$PID_TO_STOP" 2>/dev/null || true - wait_for_pid_exit "$PID_TO_STOP" - fi - REMOTE_PID="" - REMOTE_PORT="$DEFAULT_REMOTE_PORT" - REMOTE_MANAGED="external" - rm -f "$PID_FILE" - printf '%s\\n' "$REMOTE_PORT" >"$PORT_FILE" - printf 'external\\n' >"$MANAGED_FILE" - else - printf '%s\\n' "$REMOTE_PORT" >"$PORT_FILE" - printf 'external\\n' >"$MANAGED_FILE" - REMOTE_PID="" - REMOTE_MANAGED="external" + wait_ready "@@T3_REUSE_READY_TIMEOUT_MS@@" || return 1 + if [ "$REMOTE_MANAGED" = "managed" ]; then + PID_TO_STOP="\${REMOTE_PID:-$DEFAULT_RUNTIME_PID}" + # A leftover ownership file must never kill the service's current child. + if [ -n "$PID_TO_STOP" ] && [ "$PID_TO_STOP" != "$DEFAULT_RUNTIME_PID" ] && kill -0 "$PID_TO_STOP" 2>/dev/null; then + kill "$PID_TO_STOP" 2>/dev/null || true + wait_for_pid_exit "$PID_TO_STOP" fi - else - REMOTE_PID="$(cat "$PID_FILE" 2>/dev/null || true)" - REMOTE_PORT="$(cat "$PORT_FILE" 2>/dev/null || true)" - REMOTE_MANAGED="$(cat "$MANAGED_FILE" 2>/dev/null || true)" fi + REMOTE_PID="" + REMOTE_PORT="$DEFAULT_REMOTE_PORT" + REMOTE_MANAGED="external" + rm -f "$PID_FILE" + printf '%s\\n' "$REMOTE_PORT" >"$PORT_FILE" + printf 'external\\n' >"$MANAGED_FILE" +} +if service_manages_default_home; then + SERVICE_WAIT_DEADLINE=$(($(date +%s) + @@T3_SERVICE_WAIT_SECONDS@@)) + while ! connect_default_runtime; do + if [ "$(date +%s)" -ge "$SERVICE_WAIT_DEADLINE" ]; then + printf "This host's Otter Code service is not running; start it with otter-code service restart (or systemctl --user start otter-code.service on Linux).\\n" >&2 + exit 1 + fi + sleep 1 + done + printf '{"remotePort":%s,"serverKind":"external"}\\n' "$REMOTE_PORT" + exit 0 +fi +if ! connect_default_runtime; then + REMOTE_PID="$(cat "$PID_FILE" 2>/dev/null || true)" + REMOTE_PORT="$(cat "$PORT_FILE" 2>/dev/null || true)" + REMOTE_MANAGED="$(cat "$MANAGED_FILE" 2>/dev/null || true)" fi if [ "$REMOTE_MANAGED" = "external" ]; then if [ -z "$REMOTE_PORT" ] || ! wait_ready "@@T3_REUSE_READY_TIMEOUT_MS@@"; then @@ -836,6 +850,7 @@ export function buildRemoteLaunchScript(input?: RemoteT3RunnerOptions): string { T3_REMOTE_PORT_SCAN_WINDOW: String(REMOTE_PORT_SCAN_WINDOW), T3_READY_TIMEOUT_MS: String(REMOTE_READY_TIMEOUT_MS), T3_REUSE_READY_TIMEOUT_MS: String(REMOTE_REUSE_READY_TIMEOUT_MS), + T3_SERVICE_WAIT_SECONDS: String(REMOTE_SERVICE_WAIT_SECONDS), T3_READY_PROBE_TIMEOUT_MS: String(SSH_READY_PROBE_TIMEOUT_MS), }); }