diff --git a/DESIGN.md b/DESIGN.md index 7c805f885b..77261a6a2d 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1207,7 +1207,17 @@ Alert rows of the table above from the `--droplet-*` roles in `src/styles.css`. note is dim, red for an expired sign-in or a failed request. ### New workspace dialog -- Agent select comes from `GET /api/agents`; shell-only is always available. Directory defaults to +- Agent select comes from `GET /api/agents`; shell-only is always available. The bridge offers the + agents it types into the pane's shell itself (omo, gjc) exactly where it can start them: its own + PATH first, then on Unix the PATH its user's login shell sets (`server/shell-agent.ts` + ShellAgentLocator), and the start types the same absolute path the offer found. The login shell + runs only on demand: when `/api/agents` or a shell-agent start asks for an agent its own PATH + misses and the last answer is more than a minute old, never from a timer, and concurrent askers + share one run. Each run is bounded (2 s, 64 KiB of output, a short wait after a kill or the + shell's exit); a run that finds no PATH is a miss, and the server logs why once each time the + reason changes. A server run as a service with a bare PATH therefore still offers an agent + installed under `~/.local/bin` or a version manager. The agents herdr starts keep their own + environment's answer. The picker itself is unchanged. Directory defaults to the selected pane cwd and name is an optional workspace label. **Browse** updates Directory as each folder opens (`onFolder`), so **Start** uses the path shown in the browser even when **Use this folder** was not pressed; Directory keeps the `~/…` form for a folder under home, as diff --git a/changelog.d/service-agent-path.md b/changelog.d/service-agent-path.md new file mode 100644 index 0000000000..e91f638f91 --- /dev/null +++ b/changelog.d/service-agent-path.md @@ -0,0 +1,5 @@ +### Fixed +- A server run as a service now offers and starts OmO and Gajae Code when they are installed only on + its user's login-shell PATH (such as `~/.local/bin`), and starts the same executable it offered. + The login shell runs only when the agent list or a start needs it, at most once a minute, and + the server logs why when it gives no PATH. diff --git a/server/agent-launch.ts b/server/agent-launch.ts index 1f949f52c2..7d85ab9950 100644 --- a/server/agent-launch.ts +++ b/server/agent-launch.ts @@ -5,7 +5,7 @@ */ import { freeAgentName } from "./agent-name.ts"; import { launchedAgents } from "./launched-agents.ts"; -import { isShellAgentKind, startShellAgent } from "./shell-agent.ts"; +import { isShellAgentKind, startShellAgent, type ShellAgentLocator } from "./shell-agent.ts"; import { agentStart, HerdrError, sessionSnapshot } from "./herdr/client.ts"; import { launchArgsFault } from "../shared/launch-rules.ts"; @@ -26,14 +26,15 @@ export function agentFault(agent: AgentPayload | undefined): string | null { /** * Starts a checked `agent` in a pane herdr just made. A failure is answered, not thrown: the * workspace, tab or worktree around the pane is there either way. `launched` is told the pid of - * the process a shell-started agent came up as, when one is seen in front of the pane. + * the process a shell-started agent came up as, when one is seen in front of the pane; `locator` + * is the one `/api/agents` offered it from. */ -export async function agentLaunch(paneId: string, agent: AgentPayload, launched?: (pid: number) => void): Promise<{ agent_started: true } | { agent_started: false; error: { code: string; message: string } }> { +export async function agentLaunch(paneId: string, agent: AgentPayload, launched?: (pid: number) => void, locator?: ShellAgentLocator): Promise<{ agent_started: true } | { agent_started: false; error: { code: string; message: string } }> { try { const kind = agent.kind as string; const args = agent.args as string[] | undefined; if (isShellAgentKind(kind)) { - const pid = await startShellAgent(kind, paneId, args); + const pid = await startShellAgent(kind, paneId, args, locator ? { locator } : {}); if (pid !== null) launched?.(pid); } else { diff --git a/server/api.contract.test.ts b/server/api.contract.test.ts index 412f699de1..c9fb87476a 100644 --- a/server/api.contract.test.ts +++ b/server/api.contract.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it, beforeAll, afterAll, spyOn } from "bun:test"; import { Database } from "bun:sqlite"; -import { appendFileSync, chmodSync, copyFileSync, mkdirSync, mkdtempSync, existsSync, readFileSync, readdirSync, realpathSync, rmSync, statSync, symlinkSync, unlinkSync, utimesSync, writeFileSync } from "node:fs"; +import { appendFileSync, chmodSync, closeSync, constants as fsConstants, copyFileSync, mkdirSync, mkdtempSync, existsSync, readFileSync, readdirSync, realpathSync, openSync, rmSync, statSync, symlinkSync, unlinkSync, utimesSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { basename, dirname, join } from "node:path"; import { ConversationMonitor } from "./conversation-monitor.ts"; @@ -25,6 +25,7 @@ import { TailnetIdentitySource } from "./tailscale.ts"; import { forgetTranscriptState } from "./conversation.ts"; import type { RepoDiffFileList, RepoDiffFilePatch, SessionHistoryResponse } from "../shared/protocol.ts"; import { PendingInputError, PendingInputs } from "./pending-input.ts"; +import { loginPathProbe, ShellAgentLocator, type LoginPathProbe } from "./shell-agent.ts"; /** * Contract test for Ovis's HTTP + WS surface. @@ -1337,6 +1338,104 @@ describe("workspace and discovery endpoints", () => { } }); + it.skipIf(process.platform === "win32")("offers a shell agent found only on its login shell's PATH, starts that same executable, and keeps hits and misses until the next run", async () => { + // a service PATH without gjc, and a test-owned login shell whose -l run puts it on PATH: no dotfile is read + const root = mkdtempSync(join(tmpdir(), "ovis-login-path-")); + const home = join(root, "home"), serviceBin = join(root, "service bin"), loginBin = join(root, "login bin"); + for (const dir of [home, serviceBin, loginBin]) mkdirSync(dir); + const gjc = join(loginBin, "gjc"); + const alive = join(root, "alive"), release = join(root, "release"); + await Bun.$`mkfifo ${alive} ${release}`; + // the fixture agent says who it is on `alive`, then stays in front of the pane under its own path, + // reading `release` until the test closes it; it holds `alive` open, so that ends when it does + const writeGjc = () => writeFileSync(gjc, `#!/bin/bash\nexec 3>'${alive}'\necho "$$" >&3\nexec -a "$0" cat '${release}'\n`, { mode: 0o755 }); + writeGjc(); + const shell = join(root, "login-shell"); + writeFileSync(shell, `#!/bin/sh\nif [ "$1" = "-l" ]; then PATH='${loginBin}:/usr/bin:/bin'; export PATH; shift; echo 'fixture motd'; fi\n[ "$1" = "-c" ] && exec /bin/sh -c "$2"\nexit 64\n`, { mode: 0o755 }); + const real = loginPathProbe({ shell, env: { HOME: home, PATH: serviceBin } }); + let runs = 0; + const probe: LoginPathProbe = (signal) => { runs += 1; return real(signal); }; + // the answer's age is read from this clock: a minute passes only when the test says + let clock = 0; + const shellAgents = new ShellAgentLocator({ servicePath: () => serviceBin, probe, now: () => clock }); + const state = mkdtempSync(join(tmpdir(), "ovis-login-path-state-")); + const app = createServer({ port: 0, hostname: "127.0.0.1", token: "", stateDir: state, shellAgents }); + const url = `http://127.0.0.1:${app.port}`; + const kinds = async () => ((await (await fetch(`${url}/api/agents`)).json()) as { agents: AgentKind[] }).agents; + const owned = await workspaceCreate({ cwd: tmpdir(), label: "ovis-test-login-path" }); + const within = (promise: Promise, ms: number, what: string): Promise => { + let timer: ReturnType | undefined; + return Promise.race([promise, new Promise((_, reject) => { timer = setTimeout(() => reject(new Error(what)), ms); })]).finally(() => clearTimeout(timer)); + }; + // subscribed before the launch: the fixture's pid line says it is up, the end of the stream that it is gone + const watcher = Bun.spawn(["cat", alive], { stdout: "pipe" }); + const fixtureExited = watcher.exited; + const firstLine = (async () => { + const reader = watcher.stdout.getReader(), decoder = new TextDecoder(); + let text = ""; + while (!text.includes("\n")) { + const { value, done } = await reader.read(); + if (done) throw new Error("the fixture ended before it said it was up"); + text += decoder.decode(value, { stream: true }); + } + return text.split("\n")[0]!; + })(); + // closing the only writer of `release` ends the fixture's read, and its exit ends `alive` + const releaseFixture = () => { try { closeSync(openSync(release, fsConstants.O_WRONLY | fsConstants.O_NONBLOCK)); } catch { /* no reader: it never started or is gone */ } }; + let launchedPid: number | null = null; + try { + // no login shell runs until something asks + expect(runs).toBe(0); + // concurrent first reads share one run + const reads = await Promise.all(Array.from({ length: 6 }, kinds)); + for (const agents of reads) { + expect(agents).toContainEqual({ kind: "gjc", label: "Gajae Code", installed: true }); + expect(agents.map((agent) => agent.kind)).not.toContain("omo"); + // the wire shape is unchanged: kind, label and installed, nothing of where it was found + for (const agent of agents) expect(Object.keys(agent).sort()).toEqual(["installed", "kind", "label"]); + } + expect(JSON.stringify(reads)).not.toContain(root); + expect(runs).toBe(1); + + // the start types the absolute path the offer found, quoted: the fixture runs under it + const res = await fetch(`${url}/api/tab/create`, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ workspace_id: owned.workspace.workspace_id, agent: { kind: "gjc" } }) }); + expect(res.status).toBe(200); + const tab = await res.json() as WorkspaceCreated; + expect(tab).toMatchObject({ agent_started: true }); + launchedPid = Number(await within(firstLine, 10_000, "the fixture never said it was up")); + const info = await herdrRpc<{ process_info: { foreground_processes: { pid: number; argv?: string[] }[] } }>("pane.process_info", { pane_id: tab.pane_id }); + expect(info.process_info.foreground_processes).toContainEqual(expect.objectContaining({ pid: launchedPid, argv: [gjc, release] })); + // neither the reads nor the start ran a login shell of their own + expect(runs).toBe(1); + + // a removed executable is offered no more, and nothing else takes its place before the next run + rmSync(gjc); + expect((await kinds()).map((agent) => agent.kind)).not.toContain("gjc"); + clock += 60_000; + expect((await kinds()).map((agent) => agent.kind)).not.toContain("gjc"); + // back again: the kept miss holds until a read after the answer is a minute old runs the next one + writeGjc(); + expect((await kinds()).map((agent) => agent.kind)).not.toContain("gjc"); + clock += 60_000; + expect(await kinds()).toContainEqual({ kind: "gjc", label: "Gajae Code", installed: true }); + expect(runs).toBe(3); + + // released, the fixture agent ends: nothing of it outlives the test + releaseFixture(); + expect(await within(fixtureExited, 10_000, "the released fixture kept running")).toBe(0); + } finally { + releaseFixture(); + const released = await within(fixtureExited, 5000, "fixture still running").then(() => true, () => false); + if (!released && launchedPid !== null) try { process.kill(launchedPid, "SIGKILL"); } catch { /* gone with its pane */ } + watcher.kill(); + await fixtureExited; + app.stop(); + await workspaceClose(owned.workspace.workspace_id); + rmSync(root, { recursive: true, force: true }); + rmSync(state, { recursive: true, force: true }); + } + }, 60_000); + it("makes one folder for the folder browser, and refuses names, folders and origins it must not", async () => { const root = mkdtempSync(join(tmpdir(), "ovis-mkdir-")); const make = (body: unknown, headers: Record = {}) => fetch(`${base()}/api/workspace/directories`, { diff --git a/server/index.ts b/server/index.ts index 559fd1a7be..d1108c59d4 100644 --- a/server/index.ts +++ b/server/index.ts @@ -27,7 +27,7 @@ import { claudePaneSession } from "./conversation.ts"; import { processAlive } from "./omo-status.ts"; import { omoRuns, omoTasks } from "./omo-tasks.ts"; import { claudeSubagents, within } from "./claude-subagents.ts"; -import { SHELL_AGENTS, shellAgentExecutable } from "./shell-agent.ts"; +import { SHELL_AGENTS, ShellAgentLocator } from "./shell-agent.ts"; import { claudeSessionTempDir } from "./claude-store.ts"; import { fileResponse, findFile, crossSiteFileRead, forgetPageMedia, pageMediaResponse, pageMediaSource, PAGE_MEDIA_PATH } from "./file-view.ts"; import { findProjectFavicon } from "./project-favicon.ts"; @@ -225,6 +225,8 @@ export function createServer( terminalAttach?: boolean | (() => Promise); /** whether this runtime can run the PTY sidecar; unset, server/pty/sidecar.ts says. Tests give a runtime without Node or node-pty, while herdr keeps its own answer, or a probe to run in place of that one. */ sidecar?: boolean | (() => boolean); + /** where omo and gjc are found for the offer and the start (server/shell-agent.ts); unset, this server's PATH alone. The real entrypoints (this file's and remote-entry.ts) add their user's login shell; tests pass a fixture one. */ + shellAgents?: ShellAgentLocator; } = {}, ): ServerInstance { /** @@ -257,6 +259,7 @@ export function createServer( /** what lets Portal open an address: this server's own values, never a request's */ const portalGuards = () => ({ tokenSet: token !== "", serveOnly }); + const shellAgents = options.shellAgents ?? new ShellAgentLocator({ probe: null }); const openCodeState = new OpenCodePromptState(); const draftTimer = setInterval(() => openCodeState.prune(), 60_000); draftTimer.unref(); @@ -529,10 +532,13 @@ export function createServer( kinds.add("omp"); kinds.add("claude"); // not herdr kinds: offered where this server can run them (see shell-agent.ts) - for (const kind of Object.keys(SHELL_AGENTS)) if (shellAgentExecutable(kind)) kinds.add(kind); + // the locator the start asks too, so what is offered is what starts + const shellStarted = new Set(); + for (const kind of Object.keys(SHELL_AGENTS)) if (await shellAgents.executable(kind)) shellStarted.add(kind); + for (const kind of shellStarted) kinds.add(kind); const agents: AgentKind[] = [...kinds] - // an agent's command is its kind; the pane's shell may carry another PATH, as for omo and gjc above - .map((kind) => ({ kind, label: AGENT_LABELS[kind] ?? kind, installed: Bun.which(kind, { PATH: process.env["PATH"] ?? "" }) !== null })) + // an agent's command is its kind; herdr starts the others in its own environment, so only this server's PATH speaks for them + .map((kind) => ({ kind, label: AGENT_LABELS[kind] ?? kind, installed: shellStarted.has(kind) || Bun.which(kind, { PATH: process.env["PATH"] ?? "" }) !== null })) .sort((left, right) => left.label.localeCompare(right.label) || left.kind.localeCompare(right.kind)); return jsonResponse({ agents }); } catch (error) { @@ -584,7 +590,7 @@ export function createServer( const receipt = orderSequence.moved(); void machines?.refreshLocal(); return receipt; - }); + }, shellAgents); if (workspaceResponse) return workspaceResponse; if (pathname === "/api/plugins/actions") { @@ -771,6 +777,7 @@ export function createServer( registration?.close(); closeCodexDaemonSources(); }); + void shellAgents.stop(); server.stop(true); requestLog.close(); }, @@ -785,7 +792,7 @@ if (import.meta.main) { const portal = new PortalService({ stateDir: defaultStateDir() }); let instance: ReturnType; try { - instance = createServer({ updates, herdrUpdate: new HerdrUpdater(), portal, telemetry, registerBridge: true, herdrProfiles: readHerdrProfiles }); + instance = createServer({ updates, herdrUpdate: new HerdrUpdater(), portal, telemetry, registerBridge: true, herdrProfiles: readHerdrProfiles, shellAgents: new ShellAgentLocator() }); } catch (err) { const { code, syscall } = err as { code?: string; syscall?: string }; // only the bind itself: a permission error reading a state file keeps its own message and path diff --git a/server/remote-entry.contract.test.ts b/server/remote-entry.contract.test.ts new file mode 100644 index 0000000000..086e251761 --- /dev/null +++ b/server/remote-entry.contract.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, it } from "bun:test"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, watch, writeFileSync } from "node:fs"; +import { createHash } from "node:crypto"; +import { join } from "node:path"; + +describe("remote entry shell agents", () => { + it.skipIf(process.platform === "win32")("offers an agent installed only on the login shell's PATH", async () => { + // regression (fails on main): the bridge a remote PC runs asked its own bare PATH alone + const herdr = Bun.which("herdr"); + expect(herdr).not.toBeNull(); + const root = mkdtempSync("/tmp/hre-"); + const home = join(root, "home"); + const xdg = join(root, "xdg"); + const login = join(root, "login bin"); + const bridges = join(home, ".config/herdr-web-ui/bridges"); + mkdirSync(login); + mkdirSync(bridges, { recursive: true, mode: 0o700 }); + writeFileSync(join(login, "gjc"), "#!/bin/sh\nexit 0\n", { mode: 0o755 }); + // the user's login shell: its profile adds `login bin`; any other use is a plain shell + const shell = join(root, "login-shell"); + writeFileSync(shell, `#!/bin/sh\nif [ "$1" = "-l" ] && [ "$2" = "-c" ]; then PATH='${login}:/usr/bin:/bin'; export PATH; exec /bin/sh -c "$3"; fi\nexec /bin/sh "$@"\n`, { mode: 0o755 }); + const servicePath = "/usr/bin:/bin"; + expect(Bun.which("gjc", { PATH: servicePath })).toBeNull(); + const session = "t51"; + const env = { PATH: servicePath, HOME: home, SHELL: shell, XDG_CONFIG_HOME: xdg, XDG_STATE_HOME: join(root, "state"), HERDR_REMOTE_SESSION: session, OVIS_HERDR_BIN: herdr! }; + const socket = join(xdg, "herdr/sessions", session, "herdr.sock"); + const descriptor = join(bridges, createHash("sha256").update(socket).digest("hex") + ".json"); + // subscribed before the bridge starts, so its registration cannot be missed + const watcher = watch(bridges); + let child: ReturnType | undefined; + try { + const registered = new Promise<{ port: number; token: string }>((resolve, reject) => { + const timer = setTimeout(() => reject(new Error("the bridge did not register within 60 s")), 60_000); + const check = () => { + if (!existsSync(descriptor)) return; + clearTimeout(timer); + resolve(JSON.parse(readFileSync(descriptor, "utf8"))); + }; + watcher.on("change", check); + child = Bun.spawn([process.execPath, join(import.meta.dir, "remote-entry.ts")], { env, stdin: "ignore", stdout: "inherit", stderr: "inherit" }); + void child.exited.then((code) => { clearTimeout(timer); reject(new Error(`the bridge exited with ${code} before it registered`)); }); + check(); + }); + const { port, token } = await registered; + const response = await fetch(`http://127.0.0.1:${port}/api/agents`, { headers: { authorization: `Bearer ${token}` } }); + expect(response.status).toBe(200); + const { agents } = await response.json() as { agents: { kind: string; label: string; installed: boolean }[] }; + expect(agents).toContainEqual({ kind: "gjc", label: "Gajae Code", installed: true }); + } finally { + watcher.close(); + if (child) { child.kill(); await child.exited; } + Bun.spawnSync([herdr!, "--session", session, "server", "stop"], { env }); + rmSync(root, { recursive: true, force: true }); + } + }, 90_000); +}); diff --git a/server/remote-entry.ts b/server/remote-entry.ts index 1cbd75694f..f4f066461a 100644 --- a/server/remote-entry.ts +++ b/server/remote-entry.ts @@ -6,6 +6,7 @@ import { bridgeIdentity, descriptorLive, descriptorPath, registerBridge } from " import { acquireOwnerDirectory } from "./owner-guard.ts"; import { parseProcessLine, refusedSocket, staleMarker } from "./herdr-marker.ts"; import { psQuote } from "./powershell.ts"; +import { ShellAgentLocator } from "./shell-agent.ts"; import { existsSync, mkdirSync, readFileSync } from "node:fs"; import { delimiter, dirname, join } from "node:path"; import { randomBytes } from "node:crypto"; @@ -86,7 +87,8 @@ try { const info = await bridgeIdentity(); if (info.herdr.protocol < 22) throw new Error("herdr 0.9+ is required; update it explicitly before connecting"); const token = randomBytes(32).toString("hex"); - const server = createServer({ port: 0, hostname: "127.0.0.1", token, machines: false, stateDir: descriptor + ".state" }); + // started over SSH with a bare PATH: omo and gjc are often only on the login shell's + const server = createServer({ port: 0, hostname: "127.0.0.1", token, machines: false, stateDir: descriptor + ".state", shellAgents: new ShellAgentLocator() }); const registration = registerBridge(server.port, token); if (!registration.registered) { // the app finds a bridge only through its descriptor: an unregistered one serves nobody, and diff --git a/server/shell-agent.test.ts b/server/shell-agent.test.ts index dc4bff8faa..fe08d2b44c 100644 --- a/server/shell-agent.test.ts +++ b/server/shell-agent.test.ts @@ -1,6 +1,10 @@ -import { expect, it, spyOn } from "bun:test"; +import { afterEach, expect, it, jest, mock, spyOn } from "bun:test"; +import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, unlinkSync, watch, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import * as client from "./herdr/client.ts"; import { isGjcProcess } from "./gjc-runtime.ts"; -import { paneShell, shellAgentExecutable, shellCommandLine } from "./shell-agent.ts"; +import { loginPathProbe, paneShell, parseLoginPath, shellAgentExecutable, ShellAgentLocator, shellCommandLine, startShellAgent, type LoginPathProbe, type LoginPathResult } from "./shell-agent.ts"; import { descendantArgv, descendantProcesses, parseProcessTable, windowsArgv, windowsProcessTable } from "./windows-processes.ts"; it("tells a pane's shell from the program herdr reports in front", () => { @@ -63,6 +67,411 @@ it("gives up on a process-table query that stalls, and kills it", async () => { expect(await windowsProcessTable(5000, ["printf", '[{"ProcessId":2,"ParentProcessId":1,"ExecutablePath":null,"CommandLine":"x"}]'])).toEqual([{ pid: 2, parent: 1, path: null, commandLine: "x" }]); }); +const roots: string[] = []; +afterEach(() => { for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); }); + +/** + * A Unix HOME of its own, a service PATH without gjc, and a login PATH with it: `shell` is a + * test-owned stand-in for the user's login shell that sets that PATH only when run with `-l`, + * after `profile` (its stand-in for a profile's own output and children). No dotfile is read. + */ +function fixture(profile = "echo 'Welcome to the fixture shell'") { + const root = mkdtempSync(join(tmpdir(), "ovis-shell-agent-path-")); + roots.push(root); + const home = join(root, "home"), serviceBin = join(root, "service bin"), loginBin = join(root, "login bin"); + for (const dir of [home, serviceBin, loginBin]) mkdirSync(dir); + const gjc = join(loginBin, "gjc"); + writeFileSync(gjc, "#!/bin/sh\nexit 0\n", { mode: 0o755 }); + const shell = join(root, "login-shell"); + writeFileSync(shell, [ + "#!/bin/sh", + `if [ "$1" = "-l" ]; then PATH='${loginBin}:/usr/bin:/bin'; export PATH; shift; ${profile}; fi`, + '[ "$1" = "-c" ] && exec /bin/sh -c "$2"', + "exit 64", + "", + ].join("\n"), { mode: 0o755 }); + const env = { HOME: home, PATH: serviceBin }; + const servicePath = () => `${serviceBin}:/nonexistent`; + return { root, home, serviceBin, loginBin, gjc, shell, env, servicePath }; +} + +/** A probe that counts its runs. */ +function counted(probe: LoginPathProbe): { probe: LoginPathProbe; runs: () => number } { + let runs = 0; + return { probe: (signal) => { runs += 1; return probe(signal); }, runs: () => runs }; +} + +/** A probe whose runs the test answers by hand, each with a PATH or a miss's reason. */ +function manual() { + let answer: (result: LoginPathResult) => void = () => { throw new Error("no run is waiting"); }; + const { probe, runs } = counted(() => new Promise((resolve) => { answer = resolve; })); + return { probe, runs, found: (path: string) => answer({ path }), missed: (reason: string) => answer({ path: null, reason }) }; +} + +const once = (probe: LoginPathProbe): Promise => probe(new AbortController().signal); + +/** Whether `pid` still runs; a zombie waiting for its new parent to reap it does not. */ +async function running(pid: number): Promise { + try { process.kill(pid, 0); } catch { return false; } + const state = (await Bun.$`ps -o stat= -p ${pid}`.nothrow().text()).trim(); + return state !== "" && !state.startsWith("Z"); +} + +/** Resolves once `file` exists with content; bounded, so a probe that never got there fails the test. */ +function written(file: string, timeoutMs = 5000): Promise { + return new Promise((resolve, reject) => { + const check = (): boolean => { + if (!existsSync(file)) return false; + const text = readFileSync(file, "utf8").trim(); + if (text === "") return false; + watcher.close(); + clearTimeout(timer); + resolve(text); + return true; + }; + const watcher = watch(join(file, ".."), () => { check(); }); + const timer = setTimeout(() => { watcher.close(); reject(new Error(`${file} was not written`)); }, timeoutMs); + check(); + }); +} + +/** `promise`, or a failure after `ms`: a probe that hangs fails the test instead of stalling it. */ +function within(promise: Promise, ms: number, what: string): Promise { + let timer: ReturnType | undefined; + return Promise.race([promise, new Promise((_, reject) => { timer = setTimeout(() => reject(new Error(what)), ms); })]).finally(() => clearTimeout(timer)); +} + +it("reads the login PATH between its markers, whatever the profile prints around them", () => { + expect(parseLoginPath("motd\n__OVIS_LOGIN_PATH__\n/a:/b\n__OVIS_LOGIN_PATH_END__\nbye\n")).toBe("/a:/b"); + expect(parseLoginPath("/a:/b\n")).toBeNull(); + expect(parseLoginPath("__OVIS_LOGIN_PATH__\n/a\nno end\n")).toBeNull(); + expect(parseLoginPath("__OVIS_LOGIN_PATH__\n\n__OVIS_LOGIN_PATH_END__\n")).toBeNull(); +}); + +it("service miss resolves the same executable for offer and start", async () => { + const f = fixture(); + // today's lookup: the service PATH misses gjc + expect(shellAgentExecutable("gjc", "linux", f.servicePath())).toBeNull(); + const { probe, runs } = counted(loginPathProbe({ shell: f.shell, env: f.env })); + const locator = new ShellAgentLocator({ platform: "linux", servicePath: f.servicePath, probe }); + // without the login fallback the start misses it too + const serviceOnly = new ShellAgentLocator({ platform: "linux", servicePath: f.servicePath, probe: null }); + await expect(startShellAgent("gjc", "w1-1", [], { locator: serviceOnly })).rejects.toThrow("not on this server's PATH"); + // the offer runs the login shell it needs + expect(await locator.executable("gjc")).toBe(f.gjc); + expect(await locator.executable("omo")).toBeNull(); + expect(await locator.executable("claude")).toBeNull(); + const sent: string[] = []; + const rpc = spyOn(client, "herdrRpc").mockImplementation((async (method: string, params: { text?: string }) => { + if (method === "pane.send_text") { sent.push(params.text ?? ""); return {}; } + if (method === "pane.process_info") return { process_info: { shell_pid: 7, foreground_processes: sent.length === 0 ? [] : [{ pid: 4242, argv: [f.gjc, "it's"] }] } }; + throw new Error(`unexpected ${method}`); + }) as typeof client.herdrRpc); + try { + expect(await startShellAgent("gjc", "w1-1", ["it's"], { locator, timeoutMs: 5000 })).toBe(4242); + } finally { + rpc.mockRestore(); + } + // the start typed the absolute path the offer found, quoted as every shell-started command is + expect(sent).toEqual([`${shellCommandLine("posix", f.gjc, ["it's"])}\n`]); + // one login shell for the first read; the later reads and the start ran none + expect(runs()).toBe(1); +}); + +it("runs no login shell until asked, and again only once its answer is older than the TTL", async () => { + const f = fixture(); + const { probe, runs } = counted(async () => ({ path: `${f.loginBin}:/usr/bin` })); + jest.useFakeTimers(); + try { + // the default clock: Date.now, which the fake timers move + const locator = new ShellAgentLocator({ platform: "linux", servicePath: f.servicePath, probe }); + // a day with nobody asking runs nothing, from a timer or anything else + jest.advanceTimersByTime(24 * 60 * 60_000); + expect(runs()).toBe(0); + // nor does a kind the service PATH has, or one that is not shell-started + writeFileSync(join(f.serviceBin, "omo"), "#!/bin/sh\n", { mode: 0o755 }); + expect(await locator.executable("omo")).toBe(join(f.serviceBin, "omo")); + expect(await locator.executable("claude")).toBeNull(); + expect(runs()).toBe(0); + // the first read that needs the login PATH runs it + expect(await locator.executable("gjc")).toBe(f.gjc); + expect(runs()).toBe(1); + // its answer is kept for a minute + jest.advanceTimersByTime(59_999); + expect(await locator.executable("gjc")).toBe(f.gjc); + expect(runs()).toBe(1); + // and once stale, the next read runs it again; a stale answer alone runs nothing + jest.advanceTimersByTime(10 * 60_000); + expect(runs()).toBe(1); + expect(await locator.executable("gjc")).toBe(f.gjc); + expect(runs()).toBe(2); + } finally { + jest.useRealTimers(); + } +}); + +it("concurrent callers share one probe", async () => { + const f = fixture(); + const { probe, runs, found, missed } = manual(); + let clock = 0; + const locator = new ShellAgentLocator({ platform: "linux", servicePath: f.servicePath, probe, now: () => clock }); + const reads = Promise.all([...Array.from({ length: 8 }, () => locator.executable("gjc")), locator.executable("omo")]); + expect(runs()).toBe(1); + found(`${f.loginBin}:/usr/bin`); + expect(await reads).toEqual([...Array.from({ length: 8 }, () => f.gjc), null]); + // hits and misses are kept: later reads run nothing + expect(await locator.executable("gjc")).toBe(f.gjc); + expect(await locator.executable("omo")).toBeNull(); + expect(runs()).toBe(1); + // a stale answer's readers share its one new run too + clock += 60_000; + const stale = Promise.all(Array.from({ length: 5 }, () => locator.executable("gjc"))); + expect(runs()).toBe(2); + found(`${f.loginBin}:/usr/bin`); + expect(await stale).toEqual(Array.from({ length: 5 }, () => f.gjc)); + + // a removed executable is not replaced by anything, even before the next run + unlinkSync(f.gjc); + writeFileSync(join(f.root, "gjc"), "#!/bin/sh\n", { mode: 0o755 }); + expect(await locator.executable("gjc")).toBeNull(); + expect(runs()).toBe(2); + // the next run sees it gone, and the one after sees it again once it is back + clock += 60_000; + let read = locator.executable("gjc"); + found(`${f.loginBin}:/usr/bin`); + expect(await read).toBeNull(); + writeFileSync(f.gjc, "#!/bin/sh\n", { mode: 0o755 }); + expect(await locator.executable("gjc")).toBeNull(); + clock += 60_000; + read = locator.executable("gjc"); + found(`${f.loginBin}:/usr/bin`); + expect(await read).toBe(f.gjc); + // a file that is not executable, a relative PATH entry and a failed run are misses + chmodSync(f.gjc, 0o644); + clock += 60_000; + read = locator.executable("gjc"); + found(`${f.loginBin}:relative`); + expect(await read).toBeNull(); + chmodSync(f.gjc, 0o755); + clock += 60_000; + read = locator.executable("gjc"); + const warn = spyOn(console, "warn").mockImplementation(() => undefined); + try { + missed("fixture miss"); + expect(await read).toBeNull(); + } finally { + warn.mockRestore(); + } + expect(runs()).toBe(6); + // what the service PATH has needs no login shell at all + writeFileSync(join(f.serviceBin, "gjc"), "#!/bin/sh\n", { mode: 0o755 }); + writeFileSync(join(f.serviceBin, "omo"), "#!/bin/sh\n", { mode: 0o755 }); + clock += 60_000; + expect(await locator.executable("gjc")).toBe(join(f.serviceBin, "gjc")); + expect(runs()).toBe(6); +}); + +it("says why the login shell gave no PATH once each time the reason changes", async () => { + const f = fixture(); + const answers: (LoginPathResult | Error)[] = [ + { path: null, reason: "reason-a" }, { path: null, reason: "reason-a" }, { path: null, reason: "reason-a" }, + { path: null, reason: "reason-b" }, + { path: `${f.loginBin}:/usr/bin` }, { path: `${f.loginBin}:/usr/bin` }, + { path: null, reason: "reason-b" }, { path: null, reason: "reason-b" }, + new Error("reason-c"), new Error("reason-c"), + ]; + const probe: LoginPathProbe = async () => { + const next = answers.shift()!; + if (next instanceof Error) throw next; + return next; + }; + let clock = 0; + const locator = new ShellAgentLocator({ platform: "linux", servicePath: f.servicePath, probe, now: () => clock }); + const warn = spyOn(console, "warn").mockImplementation(() => undefined); + try { + const results: (string | null)[] = []; + // one read a minute apart for each answer: bounded, so a locator that stops probing fails instead of spinning + for (let round = 0; round < 10; round += 1) { + results.push(await locator.executable("gjc")); + clock += 60_000; + } + expect(answers).toHaveLength(0); + // a miss stays a miss: only the runs that found the PATH offer gjc + expect(results).toEqual([null, null, null, null, f.gjc, f.gjc, null, null, null, null]); + // one line per change of reason, none for a repeat, and again after the PATH was found + const said = warn.mock.calls.map(([line]) => ["reason-a", "reason-b", "reason-c"].filter((reason) => String(line).includes(reason))); + expect(said).toEqual([["reason-a"], ["reason-b"], ["reason-b"], ["reason-c"]]); + // the PATH itself is never logged + for (const [line] of warn.mock.calls) expect(String(line)).not.toContain(f.loginBin); + } finally { + warn.mockRestore(); + } +}); + +it("hung or oversized login probe is killed and reaped, and each miss says why", async () => { + // a profile that hangs, with a child of its own + const hung = fixture(`sleep 30 & echo $! > "${"$"}HOME/child.tmp"; mv "${"$"}HOME/child.tmp" "${"$"}HOME/child.pid"; echo $$ > "${"$"}HOME/shell.tmp"; mv "${"$"}HOME/shell.tmp" "${"$"}HOME/shell.pid"; wait`); + const started = Date.now(); + const timedOut = await once(loginPathProbe({ shell: hung.shell, env: hung.env, timeoutMs: 300 })); + expect(timedOut.path).toBeNull(); + expect(Date.now() - started).toBeLessThan(5000); + for (const pid of [await written(join(hung.home, "shell.pid")), await written(join(hung.home, "child.pid"))]) expect(await running(Number(pid))).toBe(false); + + // a profile that prints more than 64 KiB, and then a PATH + const noisy = fixture(`echo $$ > "${"$"}HOME/shell.pid"; head -c 200000 /dev/zero | tr '\\\\0' x`); + const flooded = await once(loginPathProbe({ shell: noisy.shell, env: noisy.env, timeoutMs: 5000 })); + expect(flooded.path).toBeNull(); + expect(await running(Number(await written(join(noisy.home, "shell.pid"))))).toBe(false); + // stderr counts against the same limit: a flood on stderr alone, and one split across both streams + const loud = fixture(`echo $$ > "${"$"}HOME/shell.pid"; head -c 200000 /dev/zero | tr '\\\\0' x >&2`); + expect((await once(loginPathProbe({ shell: loud.shell, env: loud.env, timeoutMs: 5000 }))).path).toBeNull(); + expect(await running(Number(await written(join(loud.home, "shell.pid"))))).toBe(false); + const mixed = fixture(`echo $$ > "${"$"}HOME/shell.pid"; head -c 40000 /dev/zero | tr '\\\\0' x; head -c 40000 /dev/zero | tr '\\\\0' y >&2`); + expect((await once(loginPathProbe({ shell: mixed.shell, env: mixed.env, timeoutMs: 5000 }))).path).toBeNull(); + expect(await running(Number(await written(join(mixed.home, "shell.pid"))))).toBe(false); + + // a chatty profile under the limit is fine; a shell that is not there or fails says nothing + const chatty = fixture("echo 'PATH=/not/this'; echo __OVIS_LOGIN_PATH_END__"); + expect(await once(loginPathProbe({ shell: chatty.shell, env: chatty.env }))).toEqual({ path: `${chatty.loginBin}:/usr/bin:/bin` }); + const missing = await once(loginPathProbe({ shell: join(chatty.root, "no-such-shell"), env: chatty.env })); + expect(missing.path).toBeNull(); + const failing = fixture("exit 3"); + const failed = await once(loginPathProbe({ shell: failing.shell, env: failing.env })); + expect(failed.path).toBeNull(); + // each miss keeps its reason: a timeout, a flood, a shell that cannot run and a non-zero exit are told apart + const reasons = [timedOut, flooded, missing, failed].map((result) => result.path === null ? result.reason : ""); + expect(new Set(reasons).size).toBe(4); + for (const reason of reasons) expect(reason).not.toBe(""); + expect(reasons[2]).toContain("ENOENT"); + expect(reasons[3]).toContain("exit 3"); + // and a failed run offers nothing + const locator = new ShellAgentLocator({ platform: "linux", servicePath: hung.servicePath, probe: loginPathProbe({ shell: hung.shell, env: hung.env, timeoutMs: 300 }) }); + const warn = spyOn(console, "warn").mockImplementation(() => undefined); + try { + expect(await locator.executable("gjc")).toBeNull(); + expect(warn).toHaveBeenCalledTimes(1); + } finally { + warn.mockRestore(); + } + await locator.stop(); +}); + +it("answers a shell SIGKILL does not end within the bound, and reaps it once it exits", async () => { + const f = fixture(`echo $$ > "${"$"}HOME/shell.tmp"; mv "${"$"}HOME/shell.tmp" "${"$"}HOME/shell.pid"; exec sleep 30`); + const realKill = process.kill.bind(process); + // the kills reach nothing while `stuck`, as for a shell blocked in the kernel; the exit's own group kill is seen + let stuck = true; + let reaped: (() => void) | null = null; + const groupKilled = new Promise((resolve) => { reaped = resolve; }); + let pid = 0; + const kill = spyOn(process, "kill").mockImplementation(((target: number, signal?: string | number) => { + if (stuck) return true; + if (target === -pid && signal === "SIGKILL") reaped?.(); + return realKill(target, signal); + }) as typeof process.kill); + try { + const answer = once(loginPathProbe({ shell: f.shell, env: f.env, timeoutMs: 300, graceMs: 200 })); + pid = Number(await written(join(f.home, "shell.pid"))); + const result = await within(answer, 5000, "the probe waited on a shell SIGKILL did not end"); + expect(result.path).toBeNull(); + // answered while the shell still runs + expect(await running(pid)).toBe(true); + // once it does end, the probe's exit handler still runs: it kills the group and the shell is reaped + stuck = false; + realKill(pid, "SIGKILL"); + await within(groupKilled, 5000, "the shell's exit was never handled"); + expect(() => realKill(pid, 0)).toThrow(); + } finally { + stuck = false; + kill.mockRestore(); + try { realKill(pid, "SIGKILL"); } catch { /* reaped */ } + } +}); + +it.skipIf(Bun.which("setsid") === null)("reads the PATH printed after the shell exited, before its output closed", async () => { + // the profile hands the script to a process in a session of its own (the shell exits only once it + // says, through a FIFO, that it left the group), which waits until the shell is reaped and only + // then prints the PATH: all of it arrives after the shell's exit + const afterExit = (before: string) => fixture(`mkfifo "${"$"}HOME/left"; setsid /bin/sh -c 'echo > "$HOME/left"; while kill -0 "$0" 2>/dev/null; do :; done; ${before} exec /bin/sh -c "$1"' "$$" "$2" & read _ < "${"$"}HOME/left"; exit 0`); + const f = afterExit(""); + expect(await once(loginPathProbe({ shell: f.shell, env: f.env, timeoutMs: 5000 }))).toEqual({ path: `${f.loginBin}:/usr/bin:/bin` }); + // what arrives after the exit still counts against the output limit + const flood = afterExit("yes x | head -c 200000;"); + expect((await once(loginPathProbe({ shell: flood.shell, env: flood.env, timeoutMs: 5000 }))).path).toBeNull(); +}); + +it.skipIf(Bun.which("setsid") === null)("answers with the PATH when a process outside the group keeps the output open", async () => { + // the profile leaves a process in a session of its own holding stdout and stderr; the shell exits + // only once it says, through a FIFO, that it left the group, so the group kill cannot end it + const f = fixture(`mkfifo "${"$"}HOME/left"; setsid /bin/sh -c 'echo > "$HOME/left"; exec sleep 30' & echo $! > "${"$"}HOME/held.pid"; read _ < "${"$"}HOME/left"`); + let held = 0; + try { + const result = await once(loginPathProbe({ shell: f.shell, env: f.env, timeoutMs: 2000, graceMs: 100 })); + held = Number(readFileSync(join(f.home, "held.pid"), "utf8")); + // the pipes never close: the PATH read by exit is the answer, not a timeout's miss + expect(result).toEqual({ path: `${f.loginBin}:/usr/bin:/bin` }); + expect(await running(held)).toBe(true); + } finally { + if (held === 0 && existsSync(join(f.home, "held.pid"))) held = Number(readFileSync(join(f.home, "held.pid"), "utf8")); + if (held > 0) try { process.kill(held, "SIGKILL"); } catch { /* gone */ } + } +}); + +it("clears every timer it set once it answered", async () => { + const f = fixture(); + const set = spyOn(globalThis, "setTimeout"); + const cleared = spyOn(globalThis, "clearTimeout"); + try { + expect(await once(loginPathProbe({ shell: f.shell, env: f.env, graceMs: 1000 }))).toEqual({ path: `${f.loginBin}:/usr/bin:/bin` }); + const timers = set.mock.results.map((result) => result.value as unknown); + // the timeout, and the wait for close after the exit + expect(timers.length).toBeGreaterThanOrEqual(2); + const clearedTimers = new Set(cleared.mock.calls.map(([timer]) => timer as unknown)); + for (const timer of timers) expect(clearedTimers.has(timer)).toBe(true); + } finally { + set.mockRestore(); + cleared.mockRestore(); + } +}); + +it("Windows does not invoke a Unix login shell", async () => { + const f = fixture(); + const probe = mock(async () => ({ path: f.loginBin })); + const locator = new ShellAgentLocator({ platform: "win32", servicePath: f.servicePath, probe }); + expect(await locator.executable("gjc")).toBeNull(); + writeFileSync(join(f.serviceBin, "gjc"), "#!/bin/sh\n", { mode: 0o755 }); + expect(await locator.executable("gjc")).toBe(join(f.serviceBin, "gjc")); + expect(probe).not.toHaveBeenCalled(); + // nor without a probe given: its own default asks no shell either + const plain = new ShellAgentLocator({ platform: "win32", servicePath: f.servicePath }); + expect(await plain.executable("omo")).toBeNull(); +}); + +it("stop cancels refresh without leaking a child", async () => { + const f = fixture(`echo $$ > "${"$"}HOME/shell.tmp"; mv "${"$"}HOME/shell.tmp" "${"$"}HOME/shell.pid"; exec sleep 30`); + const { probe, runs } = counted(loginPathProbe({ shell: f.shell, env: f.env, timeoutMs: 30_000 })); + let clock = 0; + const locator = new ShellAgentLocator({ platform: "linux", servicePath: f.servicePath, probe, now: () => clock }); + const warn = spyOn(console, "warn").mockImplementation(() => undefined); + try { + const read = locator.executable("gjc"); + const pid = Number(await written(join(f.home, "shell.pid"))); + expect(await running(pid)).toBe(true); + const started = Date.now(); + await locator.stop(); + expect(Date.now() - started).toBeLessThan(5000); + expect(await running(pid)).toBe(false); + expect(await read).toBeNull(); + // a stopped locator runs no more shells, whatever the clock says, and logs no miss of its own stop + clock += 60 * 60_000; + expect(await locator.executable("gjc")).toBeNull(); + expect(runs()).toBe(1); + expect(warn).not.toHaveBeenCalled(); + } finally { + warn.mockRestore(); + } +}); + it("offers OmO on Windows when its launcher is on PATH", () => { const which = spyOn(Bun, "which").mockReturnValue("C:\\tools\\omo.exe"); try { diff --git a/server/shell-agent.ts b/server/shell-agent.ts index e8e6cd7635..ce3c25fb39 100644 --- a/server/shell-agent.ts +++ b/server/shell-agent.ts @@ -9,6 +9,10 @@ * command is run by the Enter key; * - process_info names only the shell, so the shell's children say whether the agent runs. */ +import { spawn } from "node:child_process"; +import { accessSync, constants, statSync } from "node:fs"; +import { userInfo } from "node:os"; +import { isAbsolute } from "node:path"; import { herdrRpc } from "./herdr/client.ts"; import { isGjcProcess } from "./gjc-runtime.ts"; import { isOmoProcess, omoProcessIdentity } from "./omo.ts"; @@ -25,12 +29,220 @@ const WINDOWS_SHELL_AGENTS = new Set(["omo", "gjc"]); export function isShellAgentKind(kind: string): boolean { return Object.hasOwn(SHELL_AGENTS, kind); } -/** Where this server finds `kind`; the pane's shell is herdr's child and may carry another PATH. */ -export function shellAgentExecutable(kind: string, platform: string = process.platform): string | null { +/** Where this server finds `kind` on its own PATH; the pane's shell is herdr's child and may carry another PATH. */ +export function shellAgentExecutable(kind: string, platform: string = process.platform, path: string = process.env["PATH"] ?? ""): string | null { if (platform === "win32" && !WINDOWS_SHELL_AGENTS.has(kind)) return null; - return Bun.which(kind, { PATH: process.env["PATH"] ?? "" }); + return Bun.which(kind, { PATH: path }); } +const isExecutableFile = (path: string): boolean => { + try { + accessSync(path, constants.X_OK); + return statSync(path).isFile(); + } catch { return false; } +}; + +const LOGIN_PATH_START = "__OVIS_LOGIN_PATH__"; +const LOGIN_PATH_END = "__OVIS_LOGIN_PATH_END__"; +/** The fixed text the login shell runs: no request's kind or argument is ever part of it. printenv prints PATH joined by `:` in fish too. */ +const LOGIN_PATH_SCRIPT = `printf '\\n%s\\n' ${LOGIN_PATH_START}; printenv PATH; printf '%s\\n' ${LOGIN_PATH_END}`; + +/** The user's login shell as an absolute executable: $SHELL, else the account's; null when neither is one. */ +export function loginShell(env: Record = process.env): string | null { + let account: string | null = null; + try { account = userInfo().shell; } catch { /* no passwd entry */ } + for (const shell of [env["SHELL"], account]) if (shell && isAbsolute(shell) && isExecutableFile(shell)) return shell; + return null; +} + +/** The PATH between the markers of the probe's own output; the profile's chatter around them is ignored. */ +export function parseLoginPath(output: string): string | null { + const lines = output.split(/\r?\n/); + const start = lines.lastIndexOf(LOGIN_PATH_START); + if (start === -1 || lines[start + 2] !== LOGIN_PATH_END) return null; + const path = lines[start + 1]!.trim(); + return path === "" ? null : path; +} + +/** The login shell's PATH, or why it gave none. */ +export type LoginPathResult = { path: string } | { path: null; reason: string }; + +export interface LoginPathProbe { + /** the login shell's PATH; a miss says why: the shell failed, outran its time or said too much */ + (signal: AbortSignal): Promise; +} + +/** + * Runs `shell -l -c` once with the fixed script and reads the PATH its profile sets. The shell runs + * in a process group of its own, so a profile that leaves a child behind is killed with it; at + * `timeoutMs`, past `maxBytes` of output (stdout and stderr together) or on `signal` the group is + * killed and the shell reaped before this answers a miss. A shell SIGKILL cannot end (blocked in + * the kernel, say on a hung home) is answered for `graceMs` after the kill and reaped when it + * exits. The output is read until its pipes close, at most `graceMs` past the shell's exit. + */ +export function loginPathProbe(options: { shell: string; env?: Record; timeoutMs?: number; maxBytes?: number; graceMs?: number }): LoginPathProbe { + const { shell, env = process.env, timeoutMs = 2000, maxBytes = 64 * 1024, graceMs = 500 } = options; + return (signal) => new Promise((resolve) => { + if (signal.aborted) { resolve({ path: null, reason: "stopped" }); return; } + const child = spawn(shell, ["-l", "-c", LOGIN_PATH_SCRIPT], { env, stdio: ["ignore", "pipe", "pipe"], detached: true }); + const chunks: Buffer[] = []; + let bytes = 0; + /** why the group was killed; set once */ + let failure: string | null = null; + let exited = false; + let settled = false; + let grace: ReturnType | undefined; + const done = (result: LoginPathResult): void => { + if (settled) return; + settled = true; + clearTimeout(timer); + clearTimeout(grace); + signal.removeEventListener("abort", stopped); + // nothing more is read: a process outside the group that kept the pipes open is not waited on + child.stdout?.destroy(); + child.stderr?.destroy(); + resolve(result); + }; + const answer = (): void => { + const path = parseLoginPath(Buffer.concat(chunks).toString("utf8")); + done(path === null ? { path: null, reason: `${shell} -l printed no PATH between its markers` } : { path }); + }; + const kill = (reason: string): void => { + if (failure !== null) return; + failure = `${shell} -l ${reason}`; + // after the exit the group is already killed: only the output was still being read + if (exited) { done({ path: null, reason: failure }); return; } + try { process.kill(-child.pid!, "SIGKILL"); } catch { child.kill("SIGKILL"); } + // the answer never waits on an exit SIGKILL may not bring; the exit handler still reaps it + const killed = failure; + grace = setTimeout(() => done({ path: null, reason: `${killed}, and did not end after SIGKILL` }), graceMs); + }; + const stopped = (): void => kill("was stopped"); + const timer = setTimeout(() => kill(`timed out after ${timeoutMs} ms`), timeoutMs); + signal.addEventListener("abort", stopped, { once: true }); + // both streams share one budget; only stdout carries the PATH + const count = (chunk: Buffer): boolean => { + bytes += chunk.length; + if (bytes > maxBytes) kill(`printed over ${maxBytes} bytes`); + return failure === null; + }; + child.stdout!.on("data", (chunk: Buffer) => { if (count(chunk)) chunks.push(chunk); }); + child.stderr!.on("data", count); + // a shell that cannot be run never exits: its error is the answer + child.on("error", (error: NodeJS.ErrnoException) => done({ path: null, reason: `${shell} could not run: ${error.code ?? error.message}` })); + child.on("exit", (code, exitSignal) => { + exited = true; + // whatever the shell left in its group goes with it + try { process.kill(-child.pid!, "SIGKILL"); } catch { /* the group is gone */ } + if (failure !== null) { done({ path: null, reason: failure }); return; } + if (code !== 0) { done({ path: null, reason: `${shell} -l ended with ${code === null ? exitSignal : `exit ${code}`}` }); return; } + // stdout may still hold output at exit: it is read until close, bounded, as a process outside + // the group (one the profile moved to a session of its own) may keep the pipe open + grace = setTimeout(answer, graceMs); + }); + child.on("close", () => { if (exited && failure === null) answer(); }); + }); +} + +/** + * Where the shell-started agents are, for both the offer (`/api/agents`) and the start: this + * server's own PATH first, then, on Unix, the PATH its user's login shell sets, for a server run + * as a service with a bare PATH. One login shell is run for every kind missing from the service + * PATH at once, only when an offer or a start asks for a kind the service PATH misses and the last + * run is older than `ttlMs` (callers meanwhile share that run); its hits and misses are kept until + * then, and nothing runs while nobody asks. A run that finds no PATH says why on the console, once + * each time the reason changes. Only the fixed names of SHELL_AGENTS are looked up, and only + * absolute executable files are kept. + */ +export class ShellAgentLocator { + readonly #platform: string; + readonly #servicePath: () => string; + readonly #probe: LoginPathProbe | null; + readonly #ttlMs: number; + readonly #now: () => number; + #found = new Map(); + /** when the last run ended; null before the first */ + #checkedAt: number | null = null; + #running: Promise | null = null; + /** why the last run found no PATH, as last logged; null after one that found it */ + #failure: string | null = null; + #abort = new AbortController(); + #stopped = false; + + constructor(options: { + platform?: string; + /** this server's PATH, read at each lookup */ + servicePath?: () => string; + /** reads the login shell's PATH; null: this server's PATH alone (tests, and a server with no login shell) */ + probe?: LoginPathProbe | null; + /** how long a run's answer is kept */ + ttlMs?: number; + /** the clock the answer's age is read from */ + now?: () => number; + } = {}) { + this.#platform = options.platform ?? process.platform; + this.#servicePath = options.servicePath ?? (() => process.env["PATH"] ?? ""); + // Windows keeps its own PATH and allowlist: no Unix login shell is asked there + const shell = this.#platform === "win32" || options.probe !== undefined ? null : loginShell(); + this.#probe = this.#platform === "win32" ? null : options.probe !== undefined ? options.probe : shell === null ? null : loginPathProbe({ shell }); + this.#ttlMs = options.ttlMs ?? 60_000; + this.#now = options.now ?? Date.now; + } + + /** The kept answer while it is younger than ttlMs; otherwise one login-shell run, shared by every caller meanwhile. */ + #fresh(probe: LoginPathProbe): Promise { + if (this.#stopped) return Promise.resolve(); + if (this.#running !== null) return this.#running; + if (this.#checkedAt !== null && this.#now() - this.#checkedAt < this.#ttlMs) return Promise.resolve(); + this.#running = this.#lookup(probe).finally(() => { this.#running = null; }); + return this.#running; + } + + async #lookup(probe: LoginPathProbe): Promise { + const missing = Object.keys(SHELL_AGENTS).filter((kind) => shellAgentExecutable(kind, this.#platform, this.#servicePath()) === null); + const found = new Map(); + const result = missing.length === 0 ? null : await probe(this.#abort.signal).catch((error: unknown): LoginPathResult => ({ path: null, reason: `the login shell probe failed: ${error instanceof Error ? error.message : String(error)}` })); + if (this.#stopped) return; + if (result?.path) { + for (const kind of missing) { + const executable = Bun.which(kind, { PATH: result.path }); + if (executable !== null && isAbsolute(executable) && isExecutableFile(executable)) found.set(kind, executable); + } + } + // a failed run proves nothing is there: it answers as a miss until the next one + this.#found = found; + this.#checkedAt = this.#now(); + if (result === null) return; + const failure = result.path === null ? result.reason : null; + if (failure !== null && failure !== this.#failure) console.warn(`shell agents: no login-shell PATH (${failure}); omo and gjc are looked up on this server's PATH alone`); + this.#failure = failure; + } + + /** + * The absolute executable `kind` is started by, or null. A kind the service PATH misses waits + * for a login-shell run when the kept answer is missing or older than ttlMs; a path a run found + * that is no longer an executable is not replaced by anything else until the next run. + */ + async executable(kind: string): Promise { + if (!isShellAgentKind(kind)) return null; + const own = shellAgentExecutable(kind, this.#platform, this.#servicePath()); + if (own !== null || this.#probe === null) return own; + await this.#fresh(this.#probe); + const found = this.#found.get(kind); + return found !== undefined && isExecutableFile(found) ? found : null; + } + + /** Kills a login shell still running and runs no more; later lookups use this server's PATH and the last run's answer. */ + stop(): Promise { + this.#stopped = true; + this.#abort.abort(); + return this.#running ?? Promise.resolve(); + } +} + +/** This server's PATH alone: what a caller that names no locator gets. */ +const servicePathOnly = new ShellAgentLocator({ probe: null }); + const quote = (word: string) => `'${word.replaceAll("'", `'\\''`)}'`; /** cmd.exe has no escape for a quote inside quotes, and expands %NAME%. */ function cmdQuote(word: string): string { @@ -61,23 +273,24 @@ const processInfo = (paneId: string) => herdrRpc("pane.process_info /** * Types the agent's absolute path and args into the pane's shell and resolves once * `kind` is its foreground process, with that process's pid; null on Windows, where it is - * found among the shell's descendants and not in front. The path is the one `/api/agents` - * discovered, so the offer and the start agree whatever the pane shell's PATH says. + * found among the shell's descendants and not in front. The path comes from the same + * `locator` `/api/agents` asked, so the offer and the start agree whatever the pane shell's + * PATH says. */ -export async function startShellAgent(kind: string, paneId: string, args: string[] = [], options: { command?: string; timeoutMs?: number } = {}): Promise { +export async function startShellAgent(kind: string, paneId: string, args: string[] = [], options: { command?: string; timeoutMs?: number; locator?: ShellAgentLocator } = {}): Promise { const isProcess = SHELL_AGENTS[kind]; if (!isProcess) throw new Error(`${kind} is not a shell-started agent`); const windows = process.platform === "win32"; if (options.command !== undefined || !windows) { let command = options.command; if (command === undefined) { - const executable = shellAgentExecutable(kind); - if (!executable) throw new Error(`${kind} is not on this server's PATH`); - command = quote(executable); + const executable = await (options.locator ?? servicePathOnly).executable(kind); + if (!executable) throw new Error(`${kind} is not on this server's PATH or its login shell's`); + command = shellCommandLine("posix", executable, []); } await herdrRpc("pane.send_text", { pane_id: paneId, text: `${[command, ...args.map(quote)].join(" ")}\n` }); } else { - const executable = shellAgentExecutable(kind); + const executable = await (options.locator ?? servicePathOnly).executable(kind); if (!executable) throw new Error(`${kind} is not on this server's PATH`); const shell = paneShell((await processInfo(paneId))?.process_info?.foreground_processes?.[0]?.argv?.[0] ?? "powershell.exe"); await herdrRpc("pane.send_text", { pane_id: paneId, text: shellCommandLine(shell, executable, args) }); diff --git a/server/workspace-routes.ts b/server/workspace-routes.ts index b71fb80a48..4cb3a0b367 100644 --- a/server/workspace-routes.ts +++ b/server/workspace-routes.ts @@ -17,6 +17,7 @@ import { HerdrError, tabClose, tabCreate, tabMove, tabRename, workspaceClose, wo import { badRequest, conflictResponse, errorResponse, isJsonObject, jsonResponse, readJsonObject, setRequestRoute } from "./http.ts"; import { paneContext } from "./pane-context.ts"; import { repoDiffResponse } from "./repo-diff.ts"; +import type { ShellAgentLocator } from "./shell-agent.ts"; /** `value` as an absolute path (`~` expanded) when it is a directory this user can read; null otherwise. */ export function expandedDirectory(value: string): string | null { @@ -38,7 +39,7 @@ const notQueued = (error: unknown): { error: { code: string; message: string } } export async function handleWorkspaceRequest(request: Request, url: URL, pathname: string, bunServer: Pick, "timeout">, broadcastAll: (message: ServerMessage) => void, queueFirstMessage: (paneId: string, text: string, kind: string, requester: string | null, launchedPid: number | null) => Promise, /** a move herdr confirmed: its receipt in this server's order (server/order-sequence.ts), taken at once */ - workspaceMoved: () => MoveReceipt): Promise { + workspaceMoved: () => MoveReceipt, shellAgents?: ShellAgentLocator): Promise { if (pathname === "/api/workspace/directories" && request.method === "POST") { setRequestRoute(pathname); let payload: unknown; @@ -148,7 +149,7 @@ export async function handleWorkspaceRequest(request: Request, url: URL, pathnam } // the process the launch brought up, when it says: the first message is bound to it let launchedPid: number | null = null; - const launched = await agentLaunch(created.root_pane.pane_id, payload.agent, (pid) => { launchedPid = pid; }); + const launched = await agentLaunch(created.root_pane.pane_id, payload.agent, (pid) => { launchedPid = pid; }, shellAgents); // the request's first message is the user's send: queued for the agent's first ready turn through the // bridge's pending path (server/core/pending.ts), never typed here; an agent that did not start gets none const firstMessage = launched.agent_started && typeof payload.first_message === "string" @@ -259,7 +260,7 @@ export async function handleWorkspaceRequest(request: Request, url: URL, pathnam already_open: opened.already_open === true, path: opened.worktree.path, branch: opened.worktree.branch, - ...(agent ? await agentLaunch(opened.root_pane.pane_id, agent) : {}), + ...(agent ? await agentLaunch(opened.root_pane.pane_id, agent, undefined, shellAgents) : {}), }); } catch (error) { // git's own words for a branch that has no commit yet (a folder just `git init`ed): with no