fix(agents): share bounded login PATH discovery with shell launch

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

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.

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 {

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 = <T,>(promise: Promise<T>, ms: number, what: string): Promise<T> => {
+      let timer: ReturnType<typeof setTimeout> | undefined;
+      return Promise.race([promise, new Promise<never>((_, 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<string, string> = {}) => fetch(`${base()}/api/workspace/directories`, {

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<boolean>);
     /** 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<string>();
+          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<typeof createServer>;
   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

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<typeof Bun.spawn> | 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);
+});