From 7ca62a9345ab2342bc075c93ac80849acf95d563 Mon Sep 17 00:00:00 2001 From: Dusty Date: Mon, 26 Oct 2026 16:41:40 -0700 Subject: [PATCH 01/10] Ship the routines worker in the server image The server image already carries the API and the built app. Add worker/src and its node_modules (the isolated install's link to @openbot/shared), so the same published image can run the worker with `bun ../worker/src/index.ts`. Nothing uses it yet; this is step 1 of running OpenBot Desktop's server, app and worker as containers instead of building them on the customer's machine. --- server/Dockerfile | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/server/Dockerfile b/server/Dockerfile index 0dda2d0968..f088a1c3f4 100644 --- a/server/Dockerfile +++ b/server/Dockerfile @@ -1,9 +1,14 @@ -# The API and the app it serves, in one image. +# The API, the app it serves, and the routines worker, in one image. # # ONE IMAGE ON PURPOSE. There is no CORS anywhere in the server, so the app must reach `/api` on its # own origin. Shipping them together means a deployment needs one service and no path rules, and the # two halves cannot end up disagreeing about which host they are on. # +# The worker rides along for a smaller version of the same reason: it is the server's own routine +# sweep (`server/src/routines/sweep.ts`) in a loop, importing the server's source rather than a copy +# of it, so a worker built from another commit would be sweeping with somebody else's rules. One +# image runs either process; the command decides which. +# # Multi-stage so the build's dependencies do not ship. Non-root because nothing here needs to be. FROM oven/bun:1.4.3 AS deps @@ -53,11 +58,20 @@ COPY --from=deps /app/package.json package.json COPY --from=deps /app/bun.lock bun.lock COPY --from=deps /app/server/node_modules server/node_modules COPY --from=deps /app/toolkit/node_modules toolkit/node_modules +# The worker's own `node_modules` holds one thing, the link to `@openbot/shared`, and without it the +# first import in `worker/src/env.ts` fails. The install is isolated: each workspace resolves through +# the folder beside its own manifest, so the server's copy of that link is no use to the worker. +COPY --from=deps /app/worker/node_modules worker/node_modules COPY server server COPY shared shared COPY toolkit toolkit COPY examples examples +# Its source and manifest, not its tests. Run it with `bun ../worker/src/index.ts` from the working +# directory below; it reaches the server's source by relative import, so it needs `server/` beside +# it, which is already here. +COPY worker/package.json worker/package.json +COPY worker/src worker/src # The built app, served by the process below. `APP_DIST_DIR` is what turns that on, so an image # without this layer is still a working API. From 1ca337befb34d7d8762c2d77c145eface43c993f Mon Sep 17 00:00:00 2001 From: Dusty Date: Mon, 26 Oct 2026 16:47:29 -0700 Subject: [PATCH 02/10] Add server and worker services to the compose file OpenBot's own server (the API and the built app it serves) and the routines worker, from the published server image, behind an `openbot` profile so scripts/start.sh and a plain `docker compose up -d` keep running them on the host as before. The server reads the same .env a host server does, with every address overridden to a service on the compose network, PORT and SERVER_PORT both pinned to 3001 (the server refuses to start when they disagree), NODE_ENV cleared so a local OpenBot may still reach private addresses, and the app and server host ports both mapped to it on loopback. The worker gets its three settings and sits on the data network only. Bots keep their host.docker.internal tool URL by default; whoever starts the server service points them at server:3001. --- docker-compose.yml | 147 +++++++++++++++++++++++++++++++++++++-- tests/compose.test.ts | 156 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 297 insertions(+), 6 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index a7c8614439..98f3d5e669 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -10,9 +10,9 @@ services: # says it plainly: isolation is the container's job, a shell can reach whatever the container # can reach. # - # `migrate` is the only service that reaches this by name, so the two of them are all that - # belongs here. Everything else that needs the database is outside this file: the API server - # runs on the host in local development and connects through the published port below. + # `migrate` reaches this by name, and so do `server` and `worker` when an installed OpenBot runs + # them; those are all that belong here. In local development the API server and the worker run + # on the host instead and connect through the published port below. environment: POSTGRES_DB: openbot POSTGRES_USER: openbot @@ -74,6 +74,138 @@ services: networks: - data + # OpenBot itself, the API and the app it serves, as an installed OpenBot runs it. + # + # BEHIND A PROFILE, BECAUSE A DEVELOPER'S SERVER IS ON THE HOST. `scripts/start.sh` runs the server + # and the worker as host processes for hot reload, names the services it wants from this file, and + # owns port 3001 while it does. Without the profile, a plain `docker compose up -d` would start a + # second server beside that one, fighting it for the port and the routines. Naming `server` on the + # command line still starts it, profile or not, which is how an installed OpenBot asks for it. + # + # The image `migrate` runs, because it is one image: the API, the built app at `APP_DIST_DIR`, and + # the worker below. A machine with that image needs Docker and nothing else, which is the point. + server: + profiles: ["openbot"] + build: + context: . + dockerfile: server/Dockerfile + # See migrate above for what pull_policy decides, and why an installed OpenBot overrides the + # `build` section away rather than trusting it. + image: ${SERVER_IMAGE:-openbot-server:latest} + pull_policy: ${IMAGE_PULL_POLICY:-build} + # The settings a host server reads with `--env-file=../.env`, so the two run the same deployment. + # Optional, because `docker compose config` and a first start both come before anything wrote it. + # + # Everything in `environment:` below wins over this file. That is what lets the addresses there + # name services on this network: the file says `127.0.0.1`, which inside this container is the + # container. + env_file: + - path: ./.env + required: false + environment: + # Both names, because `.env` carries `SERVER_PORT` for the host port, and the server refuses to + # start when the two disagree (`serverPort` in server/src/config.ts). Inside this container it + # is always 3001; the host port is the mapping's business, below. + PORT: "3001" + SERVER_PORT: "3001" + # Not production, as a host server is not. The image says production, and in production the + # server refuses `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS`, which a local OpenBot needs: its + # supervisor, computers and Bots are all at private addresses, on purpose. Empty rather than + # `development`, because unset is what a host server has, and libraries that look at this + # treat the two alike. + NODE_ENV: "" + # Every address below names a service on this network rather than the loopback port the host + # server reaches it on. + DATABASE_URL: postgres://openbot:openbot@postgres:5432/openbot + AGENT_COMPUTER_URL: http://agent-computer:4100 + # Per-Bot computers too, which is not automatic: the supervisor publishes each on a loopback + # port unless `COMPUTER_NETWORK` names this project's `default` network, and a loopback port on + # the host is not one this container can open. Whoever starts this service sets that as well. + COMPUTER_SUPERVISOR_URL: http://supervisor:4300 + # The managed Bot by name, when the deployment has one. `:+` keeps an empty setting empty, which + # is how a deployment without that Bot says so: the package drops its row while this is blank. + MANAGED_AGENT_AG_UI_URL: ${MANAGED_AGENT_AG_UI_URL:+http://agent-langgraph:4201/ag-ui} + # These three have a container address only the deployment knows, so they take the shape + # `OPENAI_CONTAINER_BASE_URL` already has on the Bots below: the container's own address when + # one is given, and otherwise the deployment's, which for anything not on this machine is + # already right. The picked harness's path, a model endpoint on the host, and Intelligence. + PICKED_HARNESS_URL: ${PICKED_HARNESS_CONTAINER_URL:-${PICKED_HARNESS_URL:-}} + OPENAI_BASE_URL: ${OPENAI_CONTAINER_BASE_URL:-${OPENAI_BASE_URL:-}} + INTELLIGENCE_API_URL: ${INTELLIGENCE_CONTAINER_API_URL:-${INTELLIGENCE_API_URL:-}} + INTELLIGENCE_GATEWAY_WS_URL: ${INTELLIGENCE_CONTAINER_GATEWAY_WS_URL:-${INTELLIGENCE_GATEWAY_WS_URL:-}} + # The deployment's secrets, from the environment Compose runs in and then from `.env`, which + # Compose reads for these `${...}` and the host server reads for itself. OpenBot Desktop keeps + # them out of that file and hands them to Compose in its environment, so they reach this + # container only by being named here. The defaults are the ones every other service in this + # file and `scripts/start.sh` agree on. + AGENT_TOOL_TOKEN: ${AGENT_TOOL_TOKEN:-} + COMPUTER_TOKEN: ${COMPUTER_TOKEN:-openbot-dev-computer-token} + KEY_ENCRYPTION_KEY: ${KEY_ENCRYPTION_KEY:-} + MANAGED_AGENT_TOKEN: ${MANAGED_AGENT_TOKEN:-} + SUPERVISOR_TOKEN: ${SUPERVISOR_TOKEN:-openbot-dev-supervisor-token} + WORKER_SHARED_SECRET: ${WORKER_SHARED_SECRET:-openbot-dev-worker-secret} + INTELLIGENCE_API_KEY: ${INTELLIGENCE_API_KEY:-} + OPENAI_API_KEY: ${OPENAI_API_KEY:-} + ANTHROPIC_API_KEY: ${ANTHROPIC_API_KEY:-} + CLAUDE_CODE_OAUTH_TOKEN: ${CLAUDE_CODE_OAUTH_TOKEN:-} + # The desktop's host-access broker proves itself with this. Minted per start and given to the + # server alone, so it is named here and nowhere else in this file. + OPENBOT_DESKTOP_HOST_TOKEN: ${OPENBOT_DESKTOP_HOST_TOKEN:-} + ports: + # Two host ports, one process. The app's is the address a window is given: the server serves + # the built app itself, so that one port reaches both halves on one origin. The server's is + # where everything else on the host already calls it, OpenBot Desktop's host-access broker + # among them. Loopback, for the reason every other port here is. + - "127.0.0.1:${APP_PORT:-3010}:3001" + - "[::1]:${APP_PORT:-3010}:3001" + - "127.0.0.1:${SERVER_PORT:-3001}:3001" + - "[::1]:${SERVER_PORT:-3001}:3001" + # For whatever stays on the host and is addressed as such: a model endpoint on this machine, and + # anything else the Bots below already reach the same way. + extra_hosts: + - "host.docker.internal:host-gateway" + depends_on: + migrate: + condition: service_completed_successfully + restart: unless-stopped + healthcheck: + test: ["CMD-SHELL", "bun -e \"const r = await fetch('http://127.0.0.1:3001/health'); process.exit(r.ok ? 0 : 1)\""] + interval: 10s + timeout: 5s + retries: 5 + # Both: `data` for the database, `default` for the Bots, the computers and the supervisor. This is + # the one service the note on `networks:` at the end of this file says may sit on both. + networks: + - data + - default + + # The routines worker, from the same image: the server's own sweep in a loop. See + # `worker/src/index.ts` for why a local deployment has this rather than a CronJob. + worker: + profiles: ["openbot"] + build: + context: . + dockerfile: server/Dockerfile + image: ${SERVER_IMAGE:-openbot-server:latest} + pull_policy: ${IMAGE_PULL_POLICY:-build} + command: ["bun", "../worker/src/index.ts"] + # Its three settings and nothing else. It is handed no `.env`: it does not read the deployment's + # configuration, and the less this process holds, the less a mistake in it can spend. + environment: + DATABASE_URL: postgres://openbot:openbot@postgres:5432/openbot + SERVER_INTERNAL_URL: http://server:3001 + WORKER_SHARED_SECRET: ${WORKER_SHARED_SECRET:-openbot-dev-worker-secret} + depends_on: + migrate: + condition: service_completed_successfully + server: + condition: service_started + restart: unless-stopped + # `data` only. It reaches the database there, and the server, which is on it too. Nothing a Bot + # can reach can reach this. + networks: + - data + # The Bot's computer is long-lived so browser sessions remain signed in across turns. agent-computer: build: @@ -382,6 +514,10 @@ services: BOT_REASONING_EFFORT: ${BOT_REASONING_EFFORT:-} # Where this Bot runs a tool: back through the deployment that granted it, never at the vendor. # `host.docker.internal` because the API server runs on the host, not in this network. + # + # Left as the default even though `server` above is now in this file, because the default is + # what `scripts/start.sh` relies on and that server is on the host. A deployment that starts + # `server` sets this to `http://server:3001/api/agent-tools/call`, for every Bot at once. OPENBOT_TOOL_URL: ${OPENBOT_TOOL_URL:-http://host.docker.internal:3001/api/agent-tools/call} AGENT_TOOL_TOKEN: ${AGENT_TOOL_TOKEN:-} extra_hosts: @@ -393,10 +529,9 @@ services: retries: 5 networks: - # The database and the one service that reaches it by name. Nothing a Bot can reach is on it. + # The database and the services that reach it by name. Nothing a Bot can reach is on it. # - # A deployment that runs the API server inside Compose rather than on the host puts that service - # on both this and `default`, which is the one place the two are meant to meet. + # `server` is on both this and `default`, which is the one place the two are meant to meet. data: # Everything else, which is where `default` already put it. Named here only so that adding a # service without a `networks:` key keeps landing beside the Bots rather than beside the database. diff --git a/tests/compose.test.ts b/tests/compose.test.ts index 8871209c26..6674ac68ae 100644 --- a/tests/compose.test.ts +++ b/tests/compose.test.ts @@ -9,6 +9,7 @@ import { } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { parse } from "yaml"; function composeFile() { return readFileSync( @@ -17,6 +18,35 @@ function composeFile() { ); } +/** The file as Compose reads it before interpolation: every `${...}` is still text. */ +function composeServices() { + return ( + parse(composeFile()) as { + services: Record< + string, + { + profiles?: string[]; + image?: string; + build?: { context: string; dockerfile: string }; + command?: string[]; + env_file?: Array<{ path: string; required: boolean }>; + environment?: Record; + depends_on?: Record; + restart?: string; + networks?: string[]; + } + >; + } + ).services; +} + +function serverDockerfile() { + return readFileSync( + join(import.meta.dir, "..", "server", "Dockerfile"), + "utf8", + ); +} + function rootDockerfile() { return readFileSync(join(import.meta.dir, "..", "Dockerfile"), "utf8"); } @@ -180,6 +210,8 @@ function runComposeConfig(env: Record) { "/dev/null", "--profile", "harness", + "--profile", + "openbot", "config", "--format", "json", @@ -239,6 +271,9 @@ test("publishes every service on a settable port with the documented default", ( ["SUPERVISOR_PORT", "4500", "4300"], ["BOT_PORT", "4200", "4200"], ["LANGGRAPH_PORT", "4201", "4201"], + // One process behind both: the server serves the built app on its own port. + ["APP_PORT", "3010", "3001"], + ["SERVER_PORT", "3001", "3001"], ] as const; for (const [name, host, container] of published) { @@ -269,6 +304,8 @@ test("publishes every service that holds a secret on loopback only", () => { "BOT_PORT", "LANGGRAPH_PORT", "PICKED_HARNESS_PORT", + "APP_PORT", + "SERVER_PORT", ]) { const published = [ ...compose.matchAll( @@ -563,3 +600,122 @@ test("gives the selected harness the same governed callback as the framework Bot ); } }); + +/** + * OpenBot's own server and worker, for an installed OpenBot, and out of a developer's way. + * + * `scripts/start.sh` runs both on the host, so a plain `docker compose up -d` that also started them + * would put a second server on port 3001 and a second worker on the routines. The profile is what + * keeps that from happening; naming the service is still enough to start it. + */ +test("keeps OpenBot's own server and worker behind a profile", () => { + const services = composeServices(); + + for (const name of ["server", "worker"]) { + expect(services[name].profiles).toEqual(["openbot"]); + } +}); + +test("runs the server and the worker from the image migrations already use", () => { + const services = composeServices(); + + for (const name of ["server", "worker"]) { + expect(services[name].image).toBe(services.migrate.image); + expect(services[name].build).toEqual(services.migrate.build); + expect(services[name].depends_on?.migrate).toEqual({ + condition: "service_completed_successfully", + }); + // Restarted by Docker, because nothing on the host is watching them any more. + expect(services[name].restart).toBe("unless-stopped"); + } + expect(services.worker.command).toEqual(["bun", "../worker/src/index.ts"]); +}); + +test("ships the worker in the server image", () => { + const dockerfile = serverDockerfile(); + + expect(dockerfile).toContain("COPY worker/src worker/src"); + // The isolated install's link to `@openbot/shared`. Without it the worker's first import fails. + expect(dockerfile).toContain( + "COPY --from=deps /app/worker/node_modules worker/node_modules", + ); +}); + +/** + * Every address the server is given names a service on its network. + * + * `.env` says `127.0.0.1` and `localhost`, which is right for a server on the host and means the + * container itself inside one. The settings in `environment:` win over that file, so these are what + * the server actually uses. + */ +test("gives the server container addresses, not the host's loopback", () => { + const server = composeServices().server; + const environment = server.environment ?? {}; + + expect(server.env_file).toEqual([{ path: "./.env", required: false }]); + expect(environment).toMatchObject({ + PORT: "3001", + SERVER_PORT: "3001", + DATABASE_URL: "postgres://openbot:openbot@postgres:5432/openbot", + AGENT_COMPUTER_URL: "http://agent-computer:4100", + COMPUTER_SUPERVISOR_URL: "http://supervisor:4300", + }); + for (const [name, value] of Object.entries(environment)) { + expect(`${name}=${value}`).not.toMatch(/localhost|127\.0\.0\.1|\[::1\]/); + } + // Production refuses `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS`, which a local OpenBot needs. + expect(environment.NODE_ENV).toBe(""); +}); + +test("puts the server beside the database and the Bots, and the worker beside the database only", () => { + const services = composeServices(); + + expect(services.server.networks).toEqual(["data", "default"]); + expect(services.worker.networks).toEqual(["data"]); + expect(Object.keys(services.worker.environment ?? {}).sort()).toEqual([ + "DATABASE_URL", + "SERVER_INTERNAL_URL", + "WORKER_SHARED_SECRET", + ]); + expect(services.worker.environment?.SERVER_INTERNAL_URL).toBe( + "http://server:3001", + ); +}); + +test("hands the desktop's host-access token to the server and nothing else", () => { + const holders = Object.entries(composeServices()) + .filter(([, service]) => + Object.keys(service.environment ?? {}).includes( + "OPENBOT_DESKTOP_HOST_TOKEN", + ), + ) + .map(([name]) => name); + + expect(holders).toEqual(["server"]); +}); + +test("addresses the server's Bots and endpoints from inside the network", () => { + const withBots = runComposeConfig({ + MANAGED_AGENT_AG_UI_URL: "http://127.0.0.1:4201/ag-ui", + PICKED_HARNESS_URL: "http://127.0.0.1:4202/run", + PICKED_HARNESS_CONTAINER_URL: "http://agent-harness:4202/run", + OPENAI_BASE_URL: "http://127.0.0.1:3001/api/model-provider/v1", + OPENAI_CONTAINER_BASE_URL: + "http://host.docker.internal:3001/api/model-provider/v1", + }); + expect(withBots.services.server.environment).toMatchObject({ + MANAGED_AGENT_AG_UI_URL: "http://agent-langgraph:4201/ag-ui", + PICKED_HARNESS_URL: "http://agent-harness:4202/run", + OPENAI_BASE_URL: "http://host.docker.internal:3001/api/model-provider/v1", + }); + + // No managed Bot stays no managed Bot, and a remote harness keeps its own address. + const without = runComposeConfig({ + MANAGED_AGENT_AG_UI_URL: "", + PICKED_HARNESS_URL: "https://agents.example/run", + }); + expect(without.services.server.environment).toMatchObject({ + MANAGED_AGENT_AG_UI_URL: "", + PICKED_HARNESS_URL: "https://agents.example/run", + }); +}); From 31f92aeb52c4dd4a7752eccf6a100b115d0e0a8e Mon Sep 17 00:00:00 2001 From: Dusty Date: Mon, 26 Oct 2026 16:54:27 -0700 Subject: [PATCH 03/10] Give the browser its own Intelligence gateway address The runtime sends the browser the gateway's /client socket in /info, and the browser opens it itself. A server in a container reaches local Intelligence at host.docker.internal, which the browser on the host cannot resolve, while the browser's localhost is the container from inside it. INTELLIGENCE_BROWSER_GATEWAY_WS_URL, when set, is the address /info (and a trajectory grant) gives the browser; the server's own runner and channel sockets stay on INTELLIGENCE_GATEWAY_WS_URL. The compose server service sets it to the deployment's own gateway URL and reads its container address from INTELLIGENCE_CONTAINER_GATEWAY_WS_URL. --- docker-compose.yml | 4 ++ server/src/config.ts | 23 ++++++- server/src/copilot.ts | 1 + server/tests/config.test.ts | 32 +++++++++ tests/compose.test.ts | 5 ++ toolkit/src/copilotkit/agents.ts | 40 ++++++++++- .../intelligence-browser-gateway.test.ts | 68 +++++++++++++++++++ 7 files changed, 171 insertions(+), 2 deletions(-) create mode 100644 toolkit/tests/intelligence-browser-gateway.test.ts diff --git a/docker-compose.yml b/docker-compose.yml index 98f3d5e669..26d6a4a526 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -133,6 +133,10 @@ services: OPENAI_BASE_URL: ${OPENAI_CONTAINER_BASE_URL:-${OPENAI_BASE_URL:-}} INTELLIGENCE_API_URL: ${INTELLIGENCE_CONTAINER_API_URL:-${INTELLIGENCE_API_URL:-}} INTELLIGENCE_GATEWAY_WS_URL: ${INTELLIGENCE_CONTAINER_GATEWAY_WS_URL:-${INTELLIGENCE_GATEWAY_WS_URL:-}} + # The browser opens the gateway too, at the address the server gives it in `/info`, and the + # browser is on the host. So it gets the deployment's own address, the one a host server would + # have used, whatever this container uses itself. + INTELLIGENCE_BROWSER_GATEWAY_WS_URL: ${INTELLIGENCE_GATEWAY_WS_URL:-} # The deployment's secrets, from the environment Compose runs in and then from `.env`, which # Compose reads for these `${...}` and the host server reads for itself. OpenBot Desktop keeps # them out of that file and hands them to Compose in its environment, so they reach this diff --git a/server/src/config.ts b/server/src/config.ts index 6188b79c1e..55c18e4e23 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -38,6 +38,11 @@ export type IntelligenceSettings = { gatewayWsUrl: string; apiKey: string; licenseToken?: string; + /** + * The gateway as the browser reaches it, when that is not where this server reaches it. See + * `INTELLIGENCE_BROWSER_GATEWAY_WS_URL` in `runtimeCapabilities`. + */ + browserGatewayWsUrl?: string; }; export type DockerComputerConfig = { @@ -990,10 +995,26 @@ function runtimeCapabilities(environment: Environment): RuntimeCapabilities { ); } + /* + * Optional, and only ever a second address for the same gateway. The browser opens the gateway + * itself, at the address `/info` gives it, so a server that reaches Intelligence somewhere the + * browser cannot follow names the browser's address here. The server in OpenBot Desktop's + * container does: it reaches local Intelligence at `host.docker.internal`, and the window on the + * host reaches the same gateway at `localhost`. Unset, the browser is given `gatewayWsUrl`, as + * before. + */ + const browserGatewayWsUrl = url( + environment, + "INTELLIGENCE_BROWSER_GATEWAY_WS_URL", + ); + return { mode: "intelligence", durableHistory: true, - intelligence: settings as IntelligenceSettings, + intelligence: { + ...(settings as IntelligenceSettings), + ...(browserGatewayWsUrl ? { browserGatewayWsUrl } : {}), + }, }; } diff --git a/server/src/copilot.ts b/server/src/copilot.ts index 5c5bca798a..9af6098ad1 100644 --- a/server/src/copilot.ts +++ b/server/src/copilot.ts @@ -1748,6 +1748,7 @@ export function mountCopilotRuntime( new IntelligenceKnowingANewThread({ apiUrl: intelligence.apiUrl, wsUrl: intelligence.gatewayWsUrl, + browserWsUrl: intelligence.browserGatewayWsUrl, apiKey: intelligence.apiKey, getLearningContainerId: async ({ agentId, diff --git a/server/tests/config.test.ts b/server/tests/config.test.ts index 9b271fb1e9..e3d6436a67 100644 --- a/server/tests/config.test.ts +++ b/server/tests/config.test.ts @@ -134,6 +134,38 @@ describe("deployment configuration", () => { ); }); + // The browser opens the gateway itself, at the address `/info` gives it. A server in a container + // reaches local Intelligence somewhere the browser on the host cannot, so it may name both. + test("keeps the browser's gateway address apart from the server's, when one is given", () => { + const config = loadConfig({ + ...baseEnvironment, + INTELLIGENCE_GATEWAY_WS_URL: "ws://host.docker.internal:14401", + INTELLIGENCE_BROWSER_GATEWAY_WS_URL: "ws://localhost:14401", + }); + if (config.runtime.mode !== "intelligence") { + throw new Error("expected the Intelligence runtime"); + } + expect(config.runtime.intelligence.gatewayWsUrl).toBe( + "ws://host.docker.internal:14401", + ); + expect(config.runtime.intelligence.browserGatewayWsUrl).toBe( + "ws://localhost:14401", + ); + + const without = loadConfig(baseEnvironment); + if (without.runtime.mode !== "intelligence") { + throw new Error("expected the Intelligence runtime"); + } + expect(without.runtime.intelligence.browserGatewayWsUrl).toBeUndefined(); + + expect(() => + loadConfig({ + ...baseEnvironment, + INTELLIGENCE_BROWSER_GATEWAY_WS_URL: "not a url", + }), + ).toThrow("INTELLIGENCE_BROWSER_GATEWAY_WS_URL must be a valid URL"); + }); + test("still forwards a licence token when a deployment sets one", () => { const config = loadConfig({ ...baseEnvironment, diff --git a/tests/compose.test.ts b/tests/compose.test.ts index 6674ac68ae..c3b0e22f43 100644 --- a/tests/compose.test.ts +++ b/tests/compose.test.ts @@ -702,11 +702,16 @@ test("addresses the server's Bots and endpoints from inside the network", () => OPENAI_BASE_URL: "http://127.0.0.1:3001/api/model-provider/v1", OPENAI_CONTAINER_BASE_URL: "http://host.docker.internal:3001/api/model-provider/v1", + INTELLIGENCE_GATEWAY_WS_URL: "ws://localhost:14401", + INTELLIGENCE_CONTAINER_GATEWAY_WS_URL: "ws://host.docker.internal:14401", }); expect(withBots.services.server.environment).toMatchObject({ MANAGED_AGENT_AG_UI_URL: "http://agent-langgraph:4201/ag-ui", PICKED_HARNESS_URL: "http://agent-harness:4202/run", OPENAI_BASE_URL: "http://host.docker.internal:3001/api/model-provider/v1", + // The server's own way to the gateway, and the browser's, which is on the host. + INTELLIGENCE_GATEWAY_WS_URL: "ws://host.docker.internal:14401", + INTELLIGENCE_BROWSER_GATEWAY_WS_URL: "ws://localhost:14401", }); // No managed Bot stays no managed Bot, and a remote harness keeps its own address. diff --git a/toolkit/src/copilotkit/agents.ts b/toolkit/src/copilotkit/agents.ts index 1486e5d7d7..46e6224941 100644 --- a/toolkit/src/copilotkit/agents.ts +++ b/toolkit/src/copilotkit/agents.ts @@ -337,11 +337,35 @@ export class UnavailableAgent extends AbstractAgent { * the instance that owns those fields. The authentication observer separately binds methods to * that original instance, so it preserves this override and its private-field receiver. * - * `getThreadMessages` is the only override. `handleGetThreadMessages` in the runtime calls it and + * `getThreadMessages` is one override. `handleGetThreadMessages` in the runtime calls it and * returns `Response.json` of whatever comes back, so an empty history here is the `{ messages: [] }` * the browser expects and a 200 instead of a 500. + * + * `ɵgetClientWsUrl` is the other, and only when `browserWsUrl` is given. That address is the one + * the runtime sends the browser in `/info`, and the browser opens it itself, so it has to be the + * gateway as the browser can reach it. A server in a container reaches local Intelligence at + * `host.docker.internal`, which a browser on the host cannot resolve, while the browser's + * `localhost` is, inside the container, the container. Everything the server opens itself, the + * runner and channel sockets, stays on `wsUrl`. */ export class IntelligenceKnowingANewThread extends CopilotKitIntelligence { + readonly #browserClientWsUrl: string | undefined; + + constructor( + config: ConstructorParameters[0] & { + browserWsUrl?: string; + }, + ) { + const { browserWsUrl, ...platform } = config; + super(platform); + const browser = browserWsUrl?.trim(); + this.#browserClientWsUrl = browser ? clientSocket(browser) : undefined; + } + + override ɵgetClientWsUrl(): string { + return this.#browserClientWsUrl ?? super.ɵgetClientWsUrl(); + } + override getThreadMessages( params: Parameters[0], ) { @@ -350,3 +374,17 @@ export class IntelligenceKnowingANewThread extends CopilotKitIntelligence { }); } } + +/** + * The gateway's client socket, from any of the three addresses the platform names it by. + * + * The rule the runtime applies to `wsUrl` (`deriveClientWsUrl`, which it does not export), so a + * browser address written the way the server's is lands on the same path. + */ +function clientSocket(wsUrl: string): string { + const base = wsUrl.replace(/\/$/, ""); + for (const socket of ["/client", "/runner", "/channels"]) { + if (base.endsWith(socket)) return `${base.slice(0, -socket.length)}/client`; + } + return `${base}/client`; +} diff --git a/toolkit/tests/intelligence-browser-gateway.test.ts b/toolkit/tests/intelligence-browser-gateway.test.ts new file mode 100644 index 0000000000..3cbe73838b --- /dev/null +++ b/toolkit/tests/intelligence-browser-gateway.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, test } from "bun:test"; +import { IntelligenceKnowingANewThread } from "../src/copilotkit/agents"; + +/** + * Two addresses for one gateway: the server's, and the browser's. + * + * The runtime sends the browser a websocket address in `/info` and the browser opens it itself. A + * server in a container reaches local Intelligence at `host.docker.internal`, a name the browser on + * the host cannot resolve; the browser reaches it at `localhost`, which inside the container is the + * container. One setting could not be right for both. + */ +describe("the gateway address a browser is given", () => { + const base = { + apiUrl: "http://host.docker.internal:18082", + wsUrl: "ws://host.docker.internal:14401", + apiKey: "synthetic-runtime-key", + }; + + test("is the server's own when no browser address is set", () => { + const client = new IntelligenceKnowingANewThread(base); + + expect(client.ɵgetClientWsUrl()).toBe( + "ws://host.docker.internal:14401/client", + ); + }); + + test("is the browser's when one is set, and the server keeps its own", () => { + const client = new IntelligenceKnowingANewThread({ + ...base, + browserWsUrl: "ws://localhost:14401", + }); + + expect(client.ɵgetClientWsUrl()).toBe("ws://localhost:14401/client"); + // Everything the server opens itself stays on the server's address. + expect(client.ɵgetRunnerWsUrl()).toBe( + "ws://host.docker.internal:14401/runner", + ); + expect(client.ɵgetChannelsWsUrl()).toBe( + "ws://host.docker.internal:14401/channels", + ); + }); + + test("names the client socket however the browser address was written", () => { + for (const browserWsUrl of [ + "ws://localhost:14401/", + "ws://localhost:14401/client", + "ws://localhost:14401/runner", + "ws://localhost:14401/channels", + ]) { + const client = new IntelligenceKnowingANewThread({ + ...base, + browserWsUrl, + }); + expect(client.ɵgetClientWsUrl()).toBe("ws://localhost:14401/client"); + } + }); + + test("ignores a blank browser address", () => { + const client = new IntelligenceKnowingANewThread({ + ...base, + browserWsUrl: " ", + }); + + expect(client.ɵgetClientWsUrl()).toBe( + "ws://host.docker.internal:14401/client", + ); + }); +}); From 316f6a96a7be16cdec459ab1c80e4ea4059f4217 Mon Sep 17 00:00:00 2001 From: Dusty Date: Mon, 26 Oct 2026 16:59:10 -0700 Subject: [PATCH 04/10] Mount the model sign-in folder into the server service A Google or xAI sign-in leaves .openbot/model-oauth.json on the host, and the server reads and renews it itself (temp file plus rename, and a .lock folder beside it), so the folder is bind-mounted rather than the file. Not created by Compose, which would make it root-owned on Linux. The user is settable so the Linux desktop can run the server as the host user who owns that 0600 file. --- docker-compose.yml | 18 ++++++++++++++++++ tests/compose.test.ts | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/docker-compose.yml b/docker-compose.yml index 26d6a4a526..16c2bf3f84 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -155,6 +155,24 @@ services: # The desktop's host-access broker proves itself with this. Minted per start and given to the # server alone, so it is named here and nowhere else in this file. OPENBOT_DESKTOP_HOST_TOKEN: ${OPENBOT_DESKTOP_HOST_TOKEN:-} + # A Google or xAI sign-in, which the server reads and renews itself. The file is on the host, + # at `.openbot/model-oauth.json` beside this one, and reaches the container through the mount + # below, so a host path here becomes the mount's. Empty stays empty: no sign-in, no file. + OPENBOT_MODEL_OAUTH_FILE: ${OPENBOT_MODEL_OAUTH_FILE:+/openbot/model-oauth.json} + # Who the server runs as. The image's own user by default. On Linux the desktop names the host's, + # because the sign-in file below is the host user's and readable by nobody else, and a bind mount + # there keeps the host's numbers; Docker Desktop's file sharing on macOS and Windows does not. + user: "${OPENBOT_SERVER_USER:-1001:1001}" + volumes: + # The directory, not the file: the server renews the token by writing a sibling and renaming it + # over the original, which a single-file mount cannot take. Not created here, because Docker + # would create it as root on Linux and the desktop could then not write its sign-in into it; + # whoever starts this service makes it first. + - type: bind + source: ./.openbot + target: /openbot + bind: + create_host_path: false ports: # Two host ports, one process. The app's is the address a window is given: the server serves # the built app itself, so that one port reaches both halves on one origin. The server's is diff --git a/tests/compose.test.ts b/tests/compose.test.ts index c3b0e22f43..478c103cca 100644 --- a/tests/compose.test.ts +++ b/tests/compose.test.ts @@ -34,6 +34,16 @@ function composeServices() { depends_on?: Record; restart?: string; networks?: string[]; + user?: string; + volumes?: Array< + | string + | { + type: string; + source: string; + target: string; + bind?: { create_host_path?: boolean }; + } + >; } >; } @@ -724,3 +734,29 @@ test("addresses the server's Bots and endpoints from inside the network", () => PICKED_HARNESS_URL: "https://agents.example/run", }); }); + +/** + * A Google or xAI sign-in is a file the server renews itself, so it reaches the container as the + * folder it sits in: the renewal writes a sibling and renames it over the original. + */ +test("mounts the model sign-in folder into the server, and only there", () => { + const services = composeServices(); + const server = services.server; + + expect(server.volumes).toContainEqual({ + type: "bind", + source: "./.openbot", + target: "/openbot", + bind: { create_host_path: false }, + }); + expect(server.environment?.OPENBOT_MODEL_OAUTH_FILE).toBe( + // biome-ignore lint/suspicious/noTemplateCurlyInString: the unexpanded placeholder is what is asserted. + "${OPENBOT_MODEL_OAUTH_FILE:+/openbot/model-oauth.json}", + ); + // biome-ignore lint/suspicious/noTemplateCurlyInString: the unexpanded placeholder is what is asserted. + expect(server.user).toBe("${OPENBOT_SERVER_USER:-1001:1001}"); + for (const [name, service] of Object.entries(services)) { + if (name === "server") continue; + expect(JSON.stringify(service.volumes ?? [])).not.toContain(".openbot"); + } +}); From c5fc5f43129674ceb3cab527f31d68e1e3a35b40 Mon Sep 17 00:00:00 2001 From: Dusty Date: Mon, 26 Oct 2026 17:02:29 -0700 Subject: [PATCH 05/10] desktop: Compose owns the server and worker; give containers their own addresses SERVICES now includes server and worker, and Stop names the openbot profile they sit behind. The settings point every Bot at server:3001, give the server host.docker.internal addresses for local Intelligence (the browser keeps the localhost ones), and give an installed harness its container address. The macOS Podman overlay publishes the server's two ports on IPv4 only. --- desktop/src-tauri/src/env.rs | 174 +++++++++++++++++-- desktop/src-tauri/src/macos-podman-ports.yml | 4 + desktop/src-tauri/src/main.rs | 2 +- desktop/src-tauri/src/stack.rs | 57 +++++- 4 files changed, 215 insertions(+), 22 deletions(-) diff --git a/desktop/src-tauri/src/env.rs b/desktop/src-tauri/src/env.rs index 2ee9a33410..4334375f19 100644 --- a/desktop/src-tauri/src/env.rs +++ b/desktop/src-tauri/src/env.rs @@ -204,6 +204,37 @@ fn bind_loopbacks(port: u16) -> std::io::Result> { Ok(held) } +/** +A loopback address as a container on this machine reaches it, or empty if it is not loopback. + +`localhost`, `*.localhost`, `127.x.y.z` and `[::1]` become `host.docker.internal`, keeping the +scheme, port and path. Anything else is already reachable from a container as written, and empty is +how `docker-compose.yml` is told to use it unchanged. +*/ +pub fn container_address(address: &str) -> String { + let address = address.trim(); + let Ok(mut url) = reqwest::Url::parse(address) else { + return String::new(); + }; + let host = url.host_str().unwrap_or_default(); + let loopback = host == "localhost" + || host.ends_with(".localhost") + || host + .trim_start_matches('[') + .trim_end_matches(']') + .parse::() + .is_ok_and(|ip| ip.is_loopback()); + if !loopback || url.set_host(Some("host.docker.internal")).is_err() { + return String::new(); + } + // The parser writes an empty path as `/`; given without one, it is returned without one. + let moved = url.to_string(); + match moved.strip_suffix('/') { + Some(bare) if url.path() == "/" && !address.ends_with('/') => bare.to_string(), + _ => moved, + } +} + #[cfg(test)] mod port_tests { use super::*; @@ -538,6 +569,23 @@ pub fn compose( "INTELLIGENCE_GATEWAY_WS_URL".into(), intelligence.gateway_ws_url.trim().to_string(), ); + /* + * The same two addresses as the server's container reaches them. + * + * Local Intelligence answers on this machine's loopback, which inside a container is the + * container. `host.docker.internal` is the machine. Only the server is moved: the browser opens + * the gateway too, from the address in `/info`, and the browser is on the host, so + * `docker-compose.yml` hands it the address above unchanged. Empty for an Intelligence somewhere + * else, which the container reaches at the address it was given. + */ + env.insert( + "INTELLIGENCE_CONTAINER_API_URL".into(), + container_address(&intelligence.api_url), + ); + env.insert( + "INTELLIGENCE_CONTAINER_GATEWAY_WS_URL".into(), + container_address(&intelligence.gateway_ws_url), + ); env.insert( "INTELLIGENCE_API_KEY".into(), intelligence.api_key.trim().to_string(), @@ -626,6 +674,9 @@ pub fn compose( } .into(), ); + // Empty unless a harness runs here, so the address a remote Bot was given is the one used and a + // harness picked on an earlier start does not leave its container address behind. + env.insert("PICKED_HARNESS_CONTAINER_URL".into(), String::new()); if let Some(picked) = harness { match picked { PickedHarness::Installed { @@ -641,15 +692,20 @@ pub fn compose( env.insert("PICKED_HARNESS_PORT".into(), port.to_string()); env.insert("PICKED_HARNESS_NAME".into(), name.clone()); let run_path = run_path.trim(); + let run_path = if run_path.is_empty() || run_path.starts_with('/') { + run_path.to_string() + } else { + format!("/{run_path}") + }; env.insert( "PICKED_HARNESS_URL".into(), - if run_path.is_empty() { - format!("http://127.0.0.1:{host_port}") - } else if run_path.starts_with('/') { - format!("http://127.0.0.1:{host_port}{run_path}") - } else { - format!("http://127.0.0.1:{host_port}/{run_path}") - }, + format!("http://127.0.0.1:{host_port}{run_path}"), + ); + // The server is a container on the harness's network, so it dials the service by + // name on the image's own port, not the host port published for everything else. + env.insert( + "PICKED_HARNESS_CONTAINER_URL".into(), + format!("http://agent-harness:{port}{run_path}"), ); /* * The kind, as the package spells it. @@ -698,12 +754,11 @@ pub fn compose( ); env.extend(ports.settings()); + // The server is a container on the Bots' network now, so they reach it by name and on its own + // port, whichever host port this start published it on. env.insert( "OPENBOT_TOOL_URL".into(), - format!( - "http://host.docker.internal:{}/api/agent-tools/call", - ports.server - ), + "http://server:3001/api/agent-tools/call".into(), ); // The whole deployment is on this machine, so the server must be allowed to talk to it. @@ -1882,8 +1937,8 @@ HTTPS_PROXY=http://proxy:8080 ); assert_eq!( value_of(&written, "OPENBOT_TOOL_URL"), - "http://host.docker.internal:3999/api/agent-tools/call", - "container tools must call the selected API port" + "http://server:3001/api/agent-tools/call", + "Bots call the server container by name, on its own port, whatever the host port" ); } @@ -1915,6 +1970,99 @@ HTTPS_PROXY=http://proxy:8080 settings["PICKED_HARNESS_URL"], "http://127.0.0.1:52106/ag-ui" ); + // The server container dials the service by name, on the image's port. + assert_eq!( + settings["PICKED_HARNESS_CONTAINER_URL"], + "http://agent-harness:4206/ag-ui" + ); + } + + #[test] + fn a_remote_harness_and_no_harness_leave_no_container_address() { + let remote = compose( + &intelligence(), + &Model::default(), + &engine_status(None), + &Ports::default(), + &pinned(), + Some(&PickedHarness::RemoteAgUi { + url: "https://agents.example/run".into(), + name: "Remote".into(), + remote_agent_id: String::new(), + }), + &BTreeMap::new(), + ); + assert_eq!(remote["PICKED_HARNESS_URL"], "https://agents.example/run"); + assert_eq!(remote["PICKED_HARNESS_CONTAINER_URL"], ""); + + let none = compose( + &intelligence(), + &Model::default(), + &engine_status(None), + &Ports::default(), + &pinned(), + None, + &BTreeMap::new(), + ); + assert_eq!(none["PICKED_HARNESS_CONTAINER_URL"], ""); + } + + /// Local Intelligence answers on loopback; the server's container reaches it through the host. + #[test] + fn local_intelligence_gets_a_container_address_and_hosted_does_not() { + for (given, expected) in [ + ( + "http://localhost:18082", + "http://host.docker.internal:18082", + ), + ("ws://localhost:14401", "ws://host.docker.internal:14401"), + ("ws://localhost:14401/", "ws://host.docker.internal:14401/"), + ( + "http://127.0.0.1:8082/api", + "http://host.docker.internal:8082/api", + ), + ("http://[::1]:8082", "http://host.docker.internal:8082"), + ( + "http://cpk.localhost:8080", + "http://host.docker.internal:8080", + ), + ( + " http://localhost:18082 ", + "http://host.docker.internal:18082", + ), + ("https://api.intelligence.copilotkit.ai", ""), + ("wss://realtime.intelligence.copilotkit.ai", ""), + ("http://localhost.evil.test:8082", ""), + ("not a url", ""), + ("", ""), + ] { + assert_eq!(container_address(given), expected, "{given}"); + } + + let env = compose( + &Intelligence { + api_url: "http://localhost:18082".into(), + gateway_ws_url: "ws://localhost:14401".into(), + api_key: "synthetic".into(), + }, + &Model::default(), + &engine_status(None), + &Ports::default(), + &pinned(), + None, + &BTreeMap::new(), + ); + // The host's own addresses stay as given: the browser reads the gateway's from these. + assert_eq!(env["INTELLIGENCE_API_URL"], "http://localhost:18082"); + assert_eq!(env["INTELLIGENCE_GATEWAY_WS_URL"], "ws://localhost:14401"); + assert_eq!( + env["INTELLIGENCE_CONTAINER_API_URL"], + "http://host.docker.internal:18082" + ); + assert_eq!( + env["INTELLIGENCE_CONTAINER_GATEWAY_WS_URL"], + "ws://host.docker.internal:14401" + ); } } diff --git a/desktop/src-tauri/src/macos-podman-ports.yml b/desktop/src-tauri/src/macos-podman-ports.yml index 20b0ae6804..f3f8a86f52 100644 --- a/desktop/src-tauri/src/macos-podman-ports.yml +++ b/desktop/src-tauri/src/macos-podman-ports.yml @@ -20,3 +20,7 @@ services: agent-harness: ports: !override - "127.0.0.1:${PICKED_HARNESS_PORT:-4202}:${PICKED_HARNESS_PORT:-4202}" + server: + ports: !override + - "127.0.0.1:${APP_PORT:-3010}:3001" + - "127.0.0.1:${SERVER_PORT:-3001}:3001" diff --git a/desktop/src-tauri/src/main.rs b/desktop/src-tauri/src/main.rs index 4b325622e4..05e14b9223 100644 --- a/desktop/src-tauri/src/main.rs +++ b/desktop/src-tauri/src/main.rs @@ -10586,7 +10586,7 @@ fn main() { ["compose","-f","docker-compose.yml","config","--format","json"] => println!("{{\"services\":{{\"supervisor\":{{\"environment\":{{\"COMPUTER_NAMESPACE\":\"fixture\"}}}}}}}}"), ["compose","-f","docker-compose.yml","stop","supervisor"] => (), ["ps","--quiet","--filter",_,"--filter",_] => (), - ["compose","-f","docker-compose.yml","--profile","harness","down"] => { + ["compose","-f","docker-compose.yml","--profile","harness","--profile","openbot","down"] => { if cwd.join("fail-down").exists() { eprintln!("synthetic down refusal");std::process::exit(73); } if fs::read_to_string(cwd.join("fixture-containers-running")).ok().as_deref()==Some(&identity) { fs::remove_file(cwd.join("fixture-containers-running")).unwrap(); diff --git a/desktop/src-tauri/src/stack.rs b/desktop/src-tauri/src/stack.rs index 22e01c8455..64788af15c 100644 --- a/desktop/src-tauri/src/stack.rs +++ b/desktop/src-tauri/src/stack.rs @@ -22,7 +22,24 @@ use crate::problem::Problem; /// The services Compose owns. `migrate` is deliberately absent: it is run once, to completion, /// rather than raised, and treating it as a long-lived service makes it look like a crash loop. -const SERVICES: [&str; 3] = ["postgres", "supervisor", "agent-computer"]; +/// +/// `server` and `worker` are OpenBot itself, from the published server image: the API, the app it +/// serves, and the routines sweep. They used to be Bun processes this app started on the host after +/// downloading Bun and running `bun install`, which on a fresh Windows machine took longer than +/// everything else in the install put together. Now they are containers like the rest, and the +/// restart policy that keeps them up is Docker's. +const SERVICES: [&str; 5] = [ + "postgres", + "supervisor", + "agent-computer", + "server", + "worker", +]; + +/// The Compose profile `server` and `worker` sit behind, so `scripts/start.sh`, whose server is a +/// host process, never starts a second one. Named on every command rather than only on `up`, so +/// `down` stops them and `config` describes them. +const OPENBOT_PROFILE: &str = "openbot"; /** The Bots that ship with OpenBot, which only run on an API key. @@ -828,9 +845,20 @@ pub fn down(engine: &Address, root: &Path) -> Result<(), String> { * * Named unconditionally rather than only when a harness was picked: this has to stop what an * earlier run started, and whether that run picked one is not something a Stop can know. + * + * `openbot` for the same reason: `server` and `worker` sit behind it, and a Stop that left them + * running would leave the app answering a window that says OpenBot is stopped. */ let output = compose_command(engine, root, &Secrets::new()) - .args(["-f", "docker-compose.yml", "--profile", "harness", "down"]) + .args([ + "-f", + "docker-compose.yml", + "--profile", + "harness", + "--profile", + OPENBOT_PROFILE, + "down", + ]) .output() .map_err(|error| format!("could not stop the stack: {error}"))?; @@ -4847,7 +4875,8 @@ fn main() { } } } - Some("compose") if actual == ["compose", "--profile", "harness", "down"] => { + Some("compose") + if actual == ["compose", "--profile", "harness", "--profile", "openbot", "down"] => { if race { std::fs::write(root.join("supervisor.stopped"), "").unwrap(); std::fs::write(root.join("stack.down"), "").unwrap(); @@ -6721,15 +6750,27 @@ fn main() { assert!(picked.contains(&"agent-harness")); } - /// Stop has to name the profile, or the one Bot the person picked keeps running. + /// Stop has to name both profiles, or what sits behind them keeps running: the one Bot the + /// person picked, and OpenBot's own server and worker. #[test] - fn stopping_names_the_harness_profile() { + fn stopping_names_the_harness_and_openbot_profiles() { let source = include_str!("stack.rs"); + let down = &source[source.find("pub fn down(").unwrap()..]; + let down = &down[..down.find("\n}\n").unwrap()]; + assert_eq!( + down.matches(r#""--profile","#).count(), + 2, + "compose down without a profile leaves that profile's services running" + ); assert!( - source - .contains(r#".args(["-f", "docker-compose.yml", "--profile", "harness", "down"])"#), - "compose down without the profile leaves agent-harness running" + down.contains(r#""harness","#), + "agent-harness keeps running" + ); + assert!( + down.contains("OPENBOT_PROFILE,"), + "server and worker keep running" ); + assert_eq!(OPENBOT_PROFILE, "openbot"); } #[test] From 289f4f4a9663b1f0091849f211417061fa1f4fe5 Mon Sep 17 00:00:00 2001 From: Dusty Date: Mon, 26 Oct 2026 17:15:34 -0700 Subject: [PATCH 06/10] desktop: wait for the server container instead of starting Bun on the host Start now hands everything to Compose and waits for the `server` container to answer `/health` on both the app and API host ports, for up to 180 s. Docker's restart policy supervises the server and worker, so the host launch, readiness, pid recording and restart loop are gone. - The desktop host token is minted before `up` and reaches Compose, which names it for the server alone; it is still never persisted. - COMPUTER_NETWORK is `_default` (new `stack::project_name`), so Bot computers join the server's network. On Linux OPENBOT_SERVER_USER is the deployment folder's uid:gid. - `.openbot/` is created before `up`, since its bind mount does not create it. - App and server ports count as ours when our container publishes them; the pre-spawn port check is gone. - Ownership for showing, restoring and asking OpenBot is now "this root's server container publishes the ports" (`stack::server_publishes`), not recorded host pids. - Status lists server and worker as containers; Restart runs `compose restart server worker`. - What is left of the supervisor polls the server for refused connections (`watch_connections`). - Start no longer requires the host Bun or packages. - Old Bun host processes from earlier installs are still stopped on Start and Stop, which is how an upgrade frees 3001/3010. Also brings tests left stale by the previous commit up to date (server/worker in the up list, `--profile openbot` on down). --- desktop/src-tauri/src/desktop_page.rs | 4 +- desktop/src-tauri/src/desktop_status.rs | 24 +- desktop/src-tauri/src/env.rs | 32 +- desktop/src-tauri/src/main.rs | 2074 ++++---------------- desktop/src-tauri/src/preparation.rs | 26 +- desktop/src-tauri/src/stack.rs | 296 +-- desktop/src-tauri/src/stop_ipc_tests.rs | 19 +- desktop/src-tauri/tests/fixtures/engine.rs | 11 +- desktop/src/StatusWindow.tsx | 6 +- 9 files changed, 609 insertions(+), 1883 deletions(-) diff --git a/desktop/src-tauri/src/desktop_page.rs b/desktop/src-tauri/src/desktop_page.rs index 8f4357766c..7b544ef459 100644 --- a/desktop/src-tauri/src/desktop_page.rs +++ b/desktop/src-tauri/src/desktop_page.rs @@ -49,8 +49,8 @@ async fn status( } #[tauri::command] -fn restart(app: tauri::AppHandle) -> Result<(), String> { - crate::restart_openbot(app) +async fn restart(app: tauri::AppHandle) -> Result<(), String> { + crate::restart_openbot(app).await } /// Stops everything, as the menu's Stop did. The page asking goes with it; the window shows setup. diff --git a/desktop/src-tauri/src/desktop_status.rs b/desktop/src-tauri/src/desktop_status.rs index 03c547d24d..b6d3b57d97 100644 --- a/desktop/src-tauri/src/desktop_status.rs +++ b/desktop/src-tauri/src/desktop_status.rs @@ -2,8 +2,8 @@ //! //! Containers are told apart by the names their owners give them: Compose names OpenBot's //! `openbot--` (its project is `openbot`), and k3d names an Intelligence stack's -//! `k3d--`. Memory is Docker's own figure for each container, and the resident size -//! of OpenBot's host processes, so the total is what the machine is actually spending. +//! `k3d--`. Memory is Docker's own figure for each container, OpenBot's own server +//! and worker among them, so the total is what the machine is actually spending. use serde::Serialize; @@ -62,18 +62,6 @@ pub fn containers(stats: &str, stack: Option<&str>) -> (Vec, Vec) { (openbot, intelligence) } -/// `ps -o pid=,rss= -p …`: each process's resident size, in bytes (ps reports kibibytes). -pub fn resident(ps: &str) -> Vec<(u32, u64)> { - ps.lines() - .filter_map(|line| { - let mut fields = line.split_whitespace(); - let pid = fields.next()?.parse().ok()?; - let kib: u64 = fields.next()?.parse().ok()?; - Some((pid, kib * 1024)) - }) - .collect() -} - #[cfg(test)] mod tests { use super::*; @@ -133,12 +121,4 @@ mod tests { ); assert_eq!(intelligence[0].memory_bytes, None); } - - #[test] - fn resident_sizes_are_kibibytes_per_pid() { - assert_eq!( - resident(" 123 2048\n 456 10\nnonsense\n"), - vec![(123, 2_097_152), (456, 10_240)] - ); - } } diff --git a/desktop/src-tauri/src/env.rs b/desktop/src-tauri/src/env.rs index 4334375f19..d610edb734 100644 --- a/desktop/src-tauri/src/env.rs +++ b/desktop/src-tauri/src/env.rs @@ -124,6 +124,8 @@ impl Ports { /// A connect probe misses Windows excluded ports: only a bind proves a port is usable. /// Hold the probes until every port is chosen so allocations cannot collide with each other. /// Existing containers from this exact Compose project are reusable, never foreign listeners. + /// The app and the API are among them: both are host ports of the one `server` container, so + /// a running OpenBot keeps its own ports across a Start rather than being moved off them. pub fn available( self, ours: &std::collections::HashSet, @@ -161,8 +163,8 @@ impl Ports { })) }; Ok(Self { - app: choose(self.app, false)?, - server: choose(self.server, false)?, + app: choose(self.app, true)?, + server: choose(self.server, true)?, postgres: choose(self.postgres, true)?, computer: choose(self.computer, true)?, bot: choose(self.bot, true)?, @@ -321,23 +323,43 @@ mod port_tests { } #[test] - fn own_published_container_ports_are_reused_but_do_not_authorize_host_port_reuse() { + fn own_published_container_ports_are_reused_but_a_foreign_listener_is_not() { let owned = TcpListener::bind("127.0.0.1:0").unwrap(); let port = owned.local_addr().unwrap().port(); let preferred = Ports { postgres: port, - server: port, ..Ports::default() }; let chosen = preferred.available(&HashSet::from([port]), None).unwrap(); assert_eq!(chosen.postgres, port); - assert_ne!(chosen.server, port); assert_ne!( preferred.available(&HashSet::new(), None).unwrap().postgres, port ); } + #[test] + fn the_app_and_api_ports_our_server_container_publishes_are_kept() { + // The one `server` container publishes both, so both are held while it runs. + let app = TcpListener::bind("127.0.0.1:0").unwrap(); + let server = TcpListener::bind("127.0.0.1:0").unwrap(); + let preferred = Ports { + app: app.local_addr().unwrap().port(), + server: server.local_addr().unwrap().port(), + ..Ports::default() + }; + let ours = HashSet::from([preferred.app, preferred.server]); + let chosen = preferred.available(&ours, None).unwrap(); + assert_eq!( + (chosen.app, chosen.server), + (preferred.app, preferred.server) + ); + // Held by somebody else, they are moved, as they always were. + let moved = preferred.available(&HashSet::new(), None).unwrap(); + assert_ne!(moved.app, preferred.app); + assert_ne!(moved.server, preferred.server); + } + #[test] fn invalid_saved_ports_fail_instead_of_probing_an_unrelated_default() { let root = crate::test_support::temp_root("invalid-local-ports"); diff --git a/desktop/src-tauri/src/main.rs b/desktop/src-tauri/src/main.rs index 05e14b9223..0ee33b6d86 100644 --- a/desktop/src-tauri/src/main.rs +++ b/desktop/src-tauri/src/main.rs @@ -17,7 +17,7 @@ mod test_support; use openbot_desktop_lib::{ acquire, deployment, deployment_release, engine, env as openbot_env, harness, host_access, install, local_eval, local_eval_state, preparation, problem::Problem, provider, pull_metrics, - quiet, stack, supervise, telemetry, tray, windows as win, + quiet, stack, telemetry, tray, windows as win, }; const QUIT_CLEANUP_NOTICE_FILE: &str = ".openbot-quit-cleanup-notice"; @@ -35,22 +35,20 @@ use tauri::{Emitter, Manager}; struct Shell { /// Session-only local folder authority; shutdown retires it before stopping the API server. host_access: Mutex>, - /// Named, because a restart policy that cannot say which process died cannot start it again. - children: Mutex>, /// Native Linux engine API owned by this desktop session, kept through Stop/Start. #[cfg(target_os = "linux")] podman_api: Mutex, /// Which run is the current one. /// - /// Stopping and starting again inside two seconds would otherwise leave the previous watcher - /// alive beside the new one, both answering the same death, and a process restarted twice is - /// one process and one orphan holding a port. + /// Stopping and starting again inside two seconds would otherwise leave the previous run's + /// connection watcher alive beside the new one, both sending the window back to setup for the + /// same refusal, and a recovery marked by the old run would outlive the run that cleared it. generation: std::sync::atomic::AtomicU64, /// Cancellation of pending Start is independent of the hosts still serving the previous run. start_generation: std::sync::atomic::AtomicU64, /// Stop serializes with synchronous startup side effects, never with an async wait. startup: Mutex<()>, - /// A cancelled attempt must finish returning its unpublished children before another starts. + /// A cancelled attempt must finish unwinding before another starts. starting: std::sync::atomic::AtomicBool, /// An explicit Stop must survive setup navigation; a new app session may resume again. stopped_in_session: std::sync::atomic::AtomicBool, @@ -1329,10 +1327,7 @@ fn reset_leftover_database_with( } let attempt = StartAttempt::begin(shell)?; let _startup = attempt.lock_current()?; - if shell.containers.lock().unwrap().is_some() - || shell.root.lock().unwrap().is_some() - || !shell.children.lock().unwrap().is_empty() - { + if shell.containers.lock().unwrap().is_some() || shell.root.lock().unwrap().is_some() { return Err(Problem::plain("OpenBot still owns running services. Choose Stop OpenBot before resetting a leftover database.")); } if shell @@ -1382,7 +1377,7 @@ async fn reset_leftover_database( ) } -/// Write the `.env`, raise the containers, migrate, then start the three host processes. +/// Write the `.env`, raise the containers, migrate, then wait for the server container to answer. #[tauri::command] #[allow( clippy::too_many_arguments, @@ -1521,7 +1516,15 @@ async fn start_stack_inner( ) })?; - let (logs, bun, mut secrets, ports) = { + // Minted before Compose, because the server is a container now and Compose is how it gets its + // environment. Never persisted: it goes into `secrets` only after they are written to the + // credential store, and the Compose file names it for the `server` service alone, so model + // workers, Bots and the worker cannot impersonate the native approval transport. + use base64::Engine as _; + let host_token = + base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(rand::random::<[u8; 32]>()); + + let (found, secrets, ports) = { let _startup = attempt.lock_current()?; // Belt and braces: a fetch that reported success and left something out is still not a @@ -1550,7 +1553,7 @@ async fn start_stack_inner( #[cfg(target_os = "linux")] let status = found.status(); acquire::prepare_for_compose(&found)?; - let bun = preparation::require(&root, Some(&requested_harness), &found)?; + preparation::require(&root, Some(&requested_harness), &found)?; // Checked here as well as in the health gate, because the gate only runs when an engine had to // be installed. A machine that already had Podman skips all of that and arrives at Compose, @@ -1585,7 +1588,14 @@ async fn start_stack_inner( }); })?; - // Only this deployment's recorded hosts are reclaimed; its existing containers are reusable. + /* + * Only this deployment's recorded hosts are reclaimed; its existing containers are reusable. + * + * THIS IS ALSO THE UPGRADE. An installation from before the server was a container still + * has its Bun server, app and worker on the host, recorded in `host-pids.json` and holding + * the very ports the `server` container is about to publish. Stopping them here, and waiting + * for their sockets to close, is what lets that `up` bind them. + */ let previous_ports = openbot_env::Ports::read(&root).map_err(|error| { Problem::with("OpenBot could not read its local ports.", error.to_string()) })?; @@ -1629,6 +1639,40 @@ async fn start_stack_inner( if let Some(authority) = organization_auth_url { settings.insert("OPENBOT_ORGANIZATION_AUTH_URL".into(), authority); } + /* + * Each Bot's computer on the network the server container is on. + * + * The supervisor publishes a computer on a loopback port of the host unless this names a + * network to attach it to instead, and a loopback port on the host is not one the server + * container can open. `default` is the network the server shares with the Bots, under the + * project name Compose is about to give it. See `stack::project_name`. + */ + settings.insert( + "COMPUTER_NETWORK".into(), + format!("{}_default", stack::project_name(&root)), + ); + /* + * On Linux the server runs as the person, not as the image's user. + * + * The model sign-in file is theirs and readable by nobody else, and a bind mount on Linux + * keeps the host's numbers, so the image's 1001 could not read it. The deployment folder is + * the one this app made for them, so its owner is who that is, with no call out to `id`. + * Docker Desktop on macOS and Windows maps ownership itself, and there the default stays. + */ + #[cfg(target_os = "linux")] + { + use std::os::unix::fs::MetadataExt as _; + let owner = std::fs::metadata(&root).map_err(|error| { + Problem::with( + "OpenBot could not read its installation folder.", + format!("{}: {error}", root.display()), + ) + })?; + settings.insert( + "OPENBOT_SERVER_USER".into(), + format!("{}:{}", owner.uid(), owner.gid()), + ); + } seed_learning_default( &root, &mut settings, @@ -1668,13 +1712,32 @@ async fn start_stack_inner( report(&app, "env", true, "settings written, credentials stored"); // Compose gives inherited environment precedence over .env. Pin this run's chosen ports. secrets.extend(ports.settings()); - for key in ["PICKED_HARNESS_PORT", "OPENBOT_TOOL_URL"] { + for key in [ + "PICKED_HARNESS_PORT", + "OPENBOT_TOOL_URL", + "COMPUTER_NETWORK", + "OPENBOT_SERVER_USER", + ] { if let Some(value) = settings.get(key) { secrets.insert(key.into(), value.clone()); } } - // Set before Bun imports the runtime, and retained for supervised restarts. + // Handed to Compose with the rest; a service receives only what the Compose file names. secrets.extend(desktop_telemetry::runtime_env(&app)); + // After the credential store has been written, so the token never is. See where it is + // minted above. + secrets.insert("OPENBOT_DESKTOP_HOST_TOKEN".into(), host_token.clone()); + + // Compose's bind mount of the sign-in folder does not create it, because Docker would make + // it as root on Linux and this app could then not write a sign-in into it. So it is made + // here, as the person, before anything asks for it. + let sign_in_folder = root.join(".openbot"); + std::fs::create_dir_all(&sign_in_folder).map_err(|error| { + Problem::with( + "OpenBot could not prepare its sign-in folder.", + format!("{}: {error}", sign_in_folder.display()), + ) + })?; // Installation already verified these images. Start only raises the local containers; // its no-pull policy sends missing assets back to the installation step. @@ -1714,50 +1777,55 @@ async fn start_stack_inner( report(&app, "services", false, detail); })?; - // Before spawning: if these are still held, whatever answers later is not ours. - if let Some(problem) = - stack::port_already_taken(&[("API server", ports.server), ("app", ports.app)]) - { - report(&app, "ports", false, problem.clone()); - return Err(problem.into()); - } - - let logs = root.join(".logs"); - (logs, bun, secrets, ports) + (found, secrets, ports) }; - // Never persisted or passed to Compose. Only the server process receives this credential; - // model workers and frontend processes cannot impersonate the native approval transport. - use base64::Engine as _; - let host_token = - base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(rand::random::<[u8; 32]>()); - secrets.insert("OPENBOT_DESKTOP_HOST_TOKEN".into(), host_token.clone()); - - let logs_for_wait = logs.clone(); - let generation = start_host_processes( - &attempt, - &root, - &logs, - &bun, - &secrets, - |name| report(&app, name, true, "started"), - move |started| { - stack::wait_until_answering( - started, - &logs_for_wait, - &stack::Ready { - api: ports.server, - app: ports.app, - }, - std::time::Duration::from_secs(180), - ) - }, - ) + /* + * The server container answering, on both of the host ports it was given. + * + * No port check before this, as there was before spawning host processes: these ports are now + * published by this deployment's own `server` container, which `Ports::available` already + * counts as ours, and anybody else on them was avoided when the ports were chosen. Docker's + * restart policy keeps it up from here, so there is nothing to supervise after it answers. + */ + report_running(&app, "answering", "waiting for OpenBot to answer", None); + let ticket = attempt.generation; + let waiting = app.clone(); + let ready = stack::Ready { + api: ports.server, + app: ports.app, + }; + let answered = tauri::async_runtime::spawn_blocking(move || { + stack::wait_until_healthy(&ready, std::time::Duration::from_secs(180), || { + waiting + .state::() + .start_generation + .load(std::sync::atomic::Ordering::SeqCst) + == ticket + }) + }) .await - .inspect_err(|problem| report(&app, "answering", false, problem.said.clone()))?; + .unwrap_or_else(|error| Err(format!("the wait did not run: {error}"))); + // A Stop during the wait is the reason it ended, whatever the wait itself says. + attempt.require_current()?; + if let Err(error) = answered { + // What the server said while it was not answering, from Compose's log, with every + // credential this run handed out taken out of it. + let log = stack::service_log(&found, &root, "server", 40); + let detail = if log.is_empty() { + "The server container wrote nothing to its log.".to_string() + } else { + format!("Last from the server container (up to 40 lines):\n{log}") + }; + let problem = stack::startup_problem(Problem::with(error, detail), &secrets); + report(&app, "answering", false, problem.said.clone()); + return Err(problem); + } // Stop must not finish between accepting readiness and reporting a successful Start. let _startup = attempt.lock_current()?; - // Only a stack that answered successfully acquires a restart policy. + *shell.root.lock().unwrap() = Some(root.clone()); + clear_recovery_required(&shell, &root); + let generation = shell.generation.load(std::sync::atomic::Ordering::SeqCst); let address = shell .containers .lock() @@ -1767,7 +1835,7 @@ async fn start_stack_inner( .ok_or_else(|| Problem::plain("The local container runtime is unavailable."))?; let config = host_access::HostAccessConfig::new( format!("http://127.0.0.1:{}", ports.server), - host_token, + host_token.clone(), address, deployment::reference(&root, "agent-computer")?, vec![root.clone()], @@ -1786,7 +1854,7 @@ async fn start_stack_inner( ) })?; preparation::save_selected_root(&config, &root)?; - supervise_host_processes(app.clone(), root, logs, bun, secrets, generation, ports); + watch_connections(app.clone(), root, host_token, generation, ports.server); report(&app, "answering", true, "the API and the app are answering"); Ok(()) @@ -1823,8 +1891,8 @@ fn shutdown_root(shell: &Shell, fallback_root: &Path) -> PathBuf { root } -/// Take the whole stack down: the host processes, anything left over, the containers, and the -/// local Intelligence stack if OpenBot created it. +/// Take the whole stack down: any host processes an installation from before the server was a +/// container left running, the containers, and the local Intelligence stack if OpenBot created it. /// /// One implementation, because there are three ways to ask for it (the button, the menu bar, and /// quitting) and a person who used one of them and got a different amount of stopping would be @@ -2233,102 +2301,16 @@ fn down_containers_on_quit(shell: &Shell, root: &Path) -> Result<(), String> { down_owned_containers(shell, root) } -/// Reclaim held replacements before consulting durable inventory. Keep the handles and pidfile -/// if any phase fails, so the next Stop or Start can retry with the same ownership evidence. -fn cleanup_host_children( - root: &Path, - children: &mut Vec<(&'static str, std::process::Child)>, - cleanup: C, -) -> Result -where - C: FnOnce(&Path) -> Result, -{ - let held = stack::stop_host_children(root, children)?; - stop_held_process_handles(children)?; - let recorded = cleanup(root)?; - Ok(held + recorded) -} - -fn stop_held_process_handles( - children: &mut Vec<(&'static str, std::process::Child)>, -) -> Result<(), Problem> { - for (name, child) in children.iter_mut() { - let failure = |error| { - Problem::with( - "OpenBot could not stop one of its host processes.", - format!("could not finish stopping held {name}: {error}"), - ) - }; - if child.try_wait().map_err(failure)?.is_none() { - child.kill().map_err(failure)?; - } - child.wait().map_err(failure)?; - } - children.clear(); - Ok(()) -} - -fn cleanup_after_host_recording_failure( - shell: &Shell, - root: &Path, - children: &mut Vec<(&'static str, std::process::Child)>, - recording: Problem, - cleanup: C, - force_handles: F, -) -> Result -where - C: FnOnce(&Path, &mut Vec<(&'static str, std::process::Child)>) -> Result, - F: FnOnce(&mut Vec<(&'static str, std::process::Child)>) -> Result<(), Problem>, -{ - shell - .generation - .fetch_add(1, std::sync::atomic::Ordering::SeqCst); - let cleanup = cleanup(root, children); - let failure = match cleanup { - Ok(_) => { - *shell.root.lock().unwrap() = None; - return Err(recording); - } - Err(cleanup) => cleanup, - }; - let forced = force_handles(children); - let mut detail = recording.detail.unwrap_or_default(); - if !detail.is_empty() { - detail.push('\n'); - } - detail.push_str(&problem_detail(failure)); - match forced { - Ok(()) => { - *shell.root.lock().unwrap() = None; - } - Err(forced) => { - detail.push('\n'); - detail.push_str(&problem_detail(forced)); - } - } - Err(Problem::with(recording.said, detail)) -} - -#[cfg(test)] -fn retire_host_processes(shell: &Shell, root: &Path, cleanup: C) -> Result -where - C: FnOnce(&Path) -> Result, -{ - // Invalidate before waiting for a restart that already owns the lock. That restart either - // observes retirement before spawning, or publishes its handle before cleanup can proceed. - shell - .generation - .fetch_add(1, std::sync::atomic::Ordering::SeqCst); - let _startup = shell.startup.lock().unwrap(); - cleanup_host_state(shell, root, cleanup) -} - +/// Stop what this session holds and then what the deployment's records name. The records are +/// how an installation from before the server was a container still has Bun processes on the +/// host to stop; a current one has none, and the cleanup finds nothing. Keep failed cleanup +/// owned for a later Stop. fn cleanup_host_state(shell: &Shell, root: &Path, cleanup: C) -> Result where C: FnOnce(&Path) -> Result, { // Also runs for startup replacement and failure recovery, so a retired authorization session - // cannot leave a job running beside a new server. Keep failed cleanup owned for a later Stop. + // cannot leave a job running beside a new server. let host_access_result = { let mut slot = shell.host_access.lock().unwrap(); let result = slot.as_ref().map_or(Ok(()), |broker| { @@ -2339,14 +2321,13 @@ where } result }; - let mut children = shell.children.lock().unwrap(); let selected = shell .root .lock() .unwrap() .clone() .unwrap_or_else(|| root.to_path_buf()); - let result = cleanup_host_children(&selected, &mut children, cleanup); + let result = cleanup(&selected); let result = match (host_access_result, result) { (Ok(()), result) => result, (Err(error), Ok(_)) => Err(Problem::with( @@ -2364,170 +2345,6 @@ where result } -/// The initial host launch, including ownership handoff on every outcome. -async fn start_host_processes( - attempt: &StartAttempt<'_>, - root: &Path, - logs: &Path, - bun: &Path, - secrets: &stack::Secrets, - mut report_started: R, - wait: W, -) -> Result -where - R: FnMut(&'static str), - W: FnOnce(&mut Vec<(&'static str, std::process::Child)>) -> Result<(), String> + Send + 'static, -{ - let started = { - let _startup = attempt.lock_current()?; - let mut started = Vec::new(); - for process in stack::HOST_PROCESSES.iter() { - if attempt.require_current().is_err() { - return finish_host_start_locked(attempt, root, started, Ok(())); - } - let child = match stack::spawn_host_process(process, root, logs, bun, secrets) { - Ok(child) => child, - Err(error) => { - return finish_host_start_locked( - attempt, - root, - started, - Err(format!("could not start {}: {error}", process.name)), - ); - } - }; - started.push((process.name, child)); - report_started(process.name); - } - // Stop and Quit need durable ownership while readiness is still waiting, especially on - // Windows where a deployment directory alone cannot authorize terminating a process. - if let Err(recording) = stack::record_host_processes( - root, - &started - .iter() - .map(|(name, child)| (*name, child.id())) - .collect::>(), - ) { - let shell = attempt.shell; - let mut children = shell.children.lock().unwrap(); - children.extend(started); - *shell.root.lock().unwrap() = Some(root.to_path_buf()); - return cleanup_after_host_recording_failure( - shell, - root, - &mut children, - recording, - |root, children| cleanup_host_children(root, children, stack::stop_processes_under), - stop_held_process_handles, - ); - } - if attempt.require_current().is_err() { - return finish_host_start_locked(attempt, root, started, Ok(())); - } - started - }; - // A failed blocking task must not drop the only handles either. The caller retains the - // vector while readiness borrows it; even a panic returns every child to the same cleanup. - let owned = std::sync::Arc::new(Mutex::new(started)); - let waiting = std::sync::Arc::clone(&owned); - let outcome = tauri::async_runtime::spawn_blocking(move || { - let mut started = waiting.lock().unwrap(); - wait(&mut started) - }) - .await - .unwrap_or_else(|error| Err(format!("the wait did not run: {error}"))); - let started = std::mem::take( - &mut *owned - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner), - ); - let readiness_failure = outcome.as_ref().err().cloned(); - finish_host_start(attempt, root, started, outcome).map_err(|problem| { - // Preserve cancellation and lifecycle failures. Only the actual readiness error gets - // the startup headline; cleanup details remain attached and are redacted with it. - if readiness_failure.as_deref() == Some(problem.said.as_str()) { - stack::startup_problem(problem, secrets) - } else { - problem - } - }) -} - -fn finish_host_start( - attempt: &StartAttempt<'_>, - root: &Path, - started: Vec<(&'static str, std::process::Child)>, - outcome: Result<(), String>, -) -> Result { - let _startup = attempt.shell.startup.lock().unwrap(); - finish_host_start_locked(attempt, root, started, outcome) -} - -fn finish_host_start_locked( - attempt: &StartAttempt<'_>, - root: &Path, - mut started: Vec<(&'static str, std::process::Child)>, - outcome: Result<(), String>, -) -> Result { - let shell = attempt.shell; - if let Err(cancelled) = attempt.require_current() { - // These handles were never published. Stop may already have finished, so this attempt - // must reap them itself. A concurrent initial Start is excluded until its ticket drops. - let cleaned = cleanup_host_children(root, &mut started, stack::stop_processes_under); - return match cleaned { - Ok(_) => Err(cancelled), - Err(cleanup) => { - let mut detail = problem_detail(cleanup); - if let Err(forced) = stop_held_process_handles(&mut started) { - detail.push('\n'); - detail.push_str(&problem_detail(forced)); - shell.children.lock().unwrap().extend(started); - *shell.root.lock().unwrap() = Some(root.to_path_buf()); - } - Err(Problem::with(cancelled.said, detail)) - } - }; - } - let mut children = shell.children.lock().unwrap(); - children.extend(started); - *shell.root.lock().unwrap() = Some(root.to_path_buf()); - if let Err(original) = outcome { - shell - .generation - .fetch_add(1, std::sync::atomic::Ordering::SeqCst); - // The initial failure is the reason Start failed, even if its cleanup also needs help. - return match cleanup_host_children(root, &mut children, stack::stop_processes_under) { - Ok(_) => { - *shell.root.lock().unwrap() = None; - Err(original.into()) - } - Err(cleanup) => Err(Problem::with(original, problem_detail(cleanup))), - }; - } - // Keep handles only while the recording failure is being cleaned up. Reporting Start failure - // while leaving the just-spawned host processes alive would recreate the orphan this ownership - // record exists to prevent. - if let Err(recording) = stack::record_host_processes( - root, - &children - .iter() - .map(|(name, child)| (*name, child.id())) - .collect::>(), - ) { - return cleanup_after_host_recording_failure( - shell, - root, - &mut children, - recording, - |root, children| cleanup_host_children(root, children, stack::stop_processes_under), - stop_held_process_handles, - ); - } - attempt.require_current()?; - clear_recovery_required(shell, root); - Ok(shell.generation.load(std::sync::atomic::Ordering::SeqCst)) -} - fn require_no_exited_compose_services( found: &engine::Address, root: &Path, @@ -2569,9 +2386,9 @@ where C: FnOnce(&Path) -> Result, { attempt.require_current()?; - // Preflight can fail while the previous run still owns live hosts. Retire that watcher only - // when replacement actually begins reclaiming them, before taking the children lock, so a - // restart already in progress hands its child to this cleanup and never adopts the new run. + // Preflight can fail while the previous run is still serving. Retire its connection watcher + // only when replacement actually begins, so a refusal it finds now cannot be published + // against the run that replaces it. attempt .shell .generation @@ -3031,7 +2848,7 @@ fn show_openbot_on( if recovery_required_or_pending_quit_notice(&shell, &root) && !organization_recovery { return Err("Part of OpenBot needs recovery. Try starting OpenBot once more.".into()); } - let url = owned_app_url(&root, ports).ok_or_else(|| { + let url = owned_app_url(&shell, &root, ports).ok_or_else(|| { format!("OpenBot could not verify its app on port {port} belongs to this installation. Try starting OpenBot again.") })?; let authority = @@ -3511,7 +3328,7 @@ fn show_status_in_app(app: &tauri::AppHandle) -> bool { } let Some(mut url) = ports_for_shell(app) .ok() - .and_then(|ports| owned_app_url(&root, &ports)) + .and_then(|ports| owned_app_url(&app.state::(), &root, &ports)) .and_then(|url| url.parse::().ok()) else { return false; @@ -3536,9 +3353,11 @@ fn show_status_in_app(app: &tauri::AppHandle) -> bool { pub(crate) struct DesktopStatus { /// OpenBot answers on its own address, whoever started it. openbot_running: bool, - /// This session of the desktop app holds OpenBot's processes, so its supervisor can restart - /// them. False for an OpenBot left running by an earlier session that ended without its Quit. + /// This session of the desktop app started the OpenBot that is running, so it can restart it. + /// False for an OpenBot left running by an earlier session that ended without its Quit. openbot_held: bool, + /// Always empty now: OpenBot's server and worker are containers and are listed with them. + /// Kept so a Status window from before that still reads the shape it expects. processes: Vec, containers: Vec, /// The local Intelligence stack recorded for this installation, if any. @@ -3574,14 +3393,7 @@ fn read_desktop_status( ) -> DesktopStatus { let shell = app.state::(); let root = cleanup_root(&shell, &stack::default_root()); - let pids: Vec<(&'static str, u32)> = shell - .children - .lock() - .unwrap() - .iter() - .map(|(name, child)| (*name, child.id())) - .collect(); - let processes = resident_parts(&pids); + let held = shell.root.lock().unwrap().is_some() && shell.containers.lock().unwrap().is_some(); let stack_id = local_eval_state::read(&root, None).map(|state| state.stack_id().as_str().to_owned()); let found = engine::detect(); @@ -3618,11 +3430,11 @@ fn read_desktop_status( let ports = ports_for_shell(app).ok(); let answering = ports .as_ref() - .is_some_and(|ports| owned_app_url(&root, ports).is_some()); + .is_some_and(|ports| owned_app_url(&shell, &root, ports).is_some()); DesktopStatus { - openbot_running: !pids.is_empty() || answering, - openbot_held: !pids.is_empty(), - processes, + openbot_running: held || answering, + openbot_held: held, + processes: Vec::new(), containers, intelligence_stack: stack_id, intelligence_containers, @@ -3638,57 +3450,34 @@ fn read_desktop_status( } } -/// Each host process by name, with its resident memory where `ps` could read it. -fn resident_parts(pids: &[(&'static str, u32)]) -> Vec { - let sizes = if pids.is_empty() { - Vec::new() - } else { - let list = pids - .iter() - .map(|(_, pid)| pid.to_string()) - .collect::>() - .join(","); - quiet::command("ps") - .args(["-o", "pid=,rss=", "-p", &list]) - .output() - .ok() - .map(|output| desktop_status::resident(&String::from_utf8_lossy(&output.stdout))) - .unwrap_or_default() - }; - pids.iter() - .map(|(name, pid)| desktop_status::Part { - name: (*name).to_owned(), - memory_bytes: sizes - .iter() - .find(|(found, _)| found == pid) - .map(|(_, bytes)| *bytes), - }) - .collect() -} - /** -Restart OpenBot's host processes: each is ended, and the supervisor that keeps them alive starts it -again, as it would after a crash. The containers, its database among them, keep running, so nothing -is lost and the restart takes seconds. +Restart OpenBot itself: the server and the worker containers, through Compose. The other +containers, its database among them, keep running, so nothing is lost and the restart takes seconds. +Only an OpenBot this session started, whose engine it knows. */ #[tauri::command] -fn restart_openbot(app: tauri::AppHandle) -> Result<(), String> { +async fn restart_openbot(app: tauri::AppHandle) -> Result<(), String> { + // Compose takes seconds to restart a container. Off Tauri's threads, as Stop is. + tauri::async_runtime::spawn_blocking(move || restart_openbot_now(&app)) + .await + .map_err(|error| format!("the restart did not run: {error}"))? +} + +fn restart_openbot_now(app: &tauri::AppHandle) -> Result<(), String> { let shell = app.state::(); - let mut children = shell.children.lock().unwrap(); - if children.is_empty() { + let root = shell.root.lock().unwrap().clone(); + let address = shell + .containers + .lock() + .unwrap() + .as_ref() + .filter(|owned| Some(&owned.root) == root.as_ref()) + .map(|owned| owned.address.clone()); + let (Some(root), Some(address)) = (root, address) else { return Err("OpenBot is not running.".into()); - } - for (name, child) in children.iter_mut() { - // SAFETY: the pid is this app's own child, held and not yet reaped, so it cannot be reused. - #[cfg(unix)] - let ended = unsafe { libc::kill(child.id() as i32, libc::SIGTERM) } == 0; - #[cfg(not(unix))] - let ended = child.kill().is_ok(); - if !ended { - eprintln!("[status] could not restart {name}"); - } - } - Ok(()) + }; + stack::restart_openbot(&address, &root) + .map_err(|error| format!("OpenBot could not restart: {error}")) } /// Stop, as the menu used to offer it: everything, Intelligence included if OpenBot started it. @@ -3761,23 +3550,64 @@ fn already_running(app: tauri::AppHandle, root: String) -> let shell = app.state::(); let _startup = shell.startup.lock().unwrap(); !recovery_required_or_pending_quit_notice(&shell, &root) - && openbot_env::Ports::read(&root).is_ok_and(|ports| already_running_at(&root, &ports)) + && openbot_env::Ports::read(&root) + .is_ok_and(|ports| already_running_at(&shell, &root, &ports)) } -fn already_running_at(root: &Path, ports: &openbot_env::Ports) -> bool { - owned_app_url(root, ports).is_some() +fn already_running_at(shell: &Shell, root: &Path, ports: &openbot_env::Ports) -> bool { + owned_app_url(shell, root, ports).is_some() } -/// Neither an owned API nor an answering app port alone authorizes showing a deployment. -fn owned_app_url(root: &Path, ports: &openbot_env::Ports) -> Option { - if !already_running_on(root, ports.server, stack::recorded_server_owns_port) - || !stack::recorded_process_owns_port(root, "app", ports.app).unwrap_or(false) - { +/// Neither an owned API nor an answering app port alone authorizes showing a deployment: this +/// root's own `server` container has to be the one publishing both. +fn owned_app_url(shell: &Shell, root: &Path, ports: &openbot_env::Ports) -> Option { + let owned = shell + .containers + .lock() + .unwrap() + .as_ref() + .filter(|owned| owned.root == root) + .map(|owned| owned.address.clone()); + if !already_running_on(root, ports.server, |root, server| { + server_container_publishes(owned.as_ref(), root, &[server, ports.app]) + }) { return None; } stack::app_url(ports.app) } +/// Whether `root`'s `server` container publishes `ports`, asked of the engine this session raised +/// it on when there is one, and otherwise of whichever engine answers: an OpenBot left running by +/// an earlier session is still this deployment's, and its restart policy kept it up. +#[cfg(not(test))] +fn server_container_publishes( + owned: Option<&engine::Address>, + root: &Path, + ports: &[u16], +) -> Result { + let address = match owned { + Some(address) => address.clone(), + None => { + let status = engine::detect(); + match status.address.filter(|_| status.responding) { + Some(address) => address, + None => return Ok(false), + } + } + }; + stack::server_publishes(&address, root, ports) +} + +/// Tests never ask a real engine. They say which roots' server containers publish which ports. +#[cfg(test)] +fn server_container_publishes( + _owned: Option<&engine::Address>, + root: &Path, + ports: &[u16], +) -> Result { + Ok(tests::published_by_server(root, ports)) +} + /// What stopped the stack, if anything did, and forget it once it has been read. /// /// Cleared on reading so a failure from an hour ago does not greet somebody who has since fixed it. @@ -3854,7 +3684,9 @@ async fn ask_the_bot_inner(root: PathBuf, question: String) -> Result Option { candidates.into_iter().find(|path| path.exists()) } -/// Watch the three host processes and start one again when it dies. +/// Mark this run as needing its connection refreshed, and send the window to setup to do it. /// -/// The policy is in `supervise.rs`; this is the loop that applies it. It ends when the stack is -/// stopped, which is what clearing the root means, so stopping does not race a restart. +/// False when the run it was found for is no longer the current one, which a Stop or a new Start +/// in the meantime makes it. fn publish_connection_failure( app: &tauri::AppHandle, root: &Path, @@ -5362,37 +5194,32 @@ fn publish_connection_failure( Ok(true) } -#[allow( - clippy::too_many_arguments, - reason = "Retain selected ports alongside the supervised run's identity and credentials." -)] -fn supervise_host_processes( +/** +Watch a running OpenBot for a connection it reports refused, and send the window back to setup to +refresh it. + +What is left of the host-process supervisor. Docker's restart policy keeps the server and the worker +up now, so nothing here starts anything. The server still knows, as only it can, when the model, the +Intelligence or the organization sign-in it was given has been refused, and it tells the desktop +through the token minted for this run. The watcher ends with the run it was started for: a Stop or +another Start moves the generation on, and clearing the root says there is no run at all. +*/ +fn watch_connections( app: tauri::AppHandle, root: PathBuf, - logs: PathBuf, - bun: PathBuf, - // Carried rather than fetched again on each restart. A restart happens when something is - // already wrong, and a credential prompt at that moment is the worst time to ask for one. - secrets: stack::Secrets, + // Carried rather than fetched again: it was minted for this run and is kept nowhere else. + token: String, generation: u64, - ports: openbot_env::Ports, + server_port: u16, ) -> std::thread::JoinHandle<()> { std::thread::spawn(move || { - eprintln!( - "[watch] supervising {} host processes", - stack::HOST_PROCESSES.len() - ); - let mut watches: Vec = stack::HOST_PROCESSES - .iter() - .map(|process| supervise::Watch::new(process.name)) - .collect(); - let connection_client = desktop_connection::client() - .map_err(|error| { - eprintln!("[watch] {error}"); - }) - .ok(); - let mut connection_notice_sent = false; - let mut connection_poll_failed = false; + let Ok(client) = desktop_connection::client().map_err(|error| { + eprintln!("[watch] {error}"); + }) else { + return; + }; + let mut notice_sent = false; + let mut poll_failed = false; loop { std::thread::sleep(std::time::Duration::from_secs(2)); @@ -5404,167 +5231,33 @@ fn supervise_host_processes( return; } - if connection_notice_sent && !recovery_required(&shell, &root) { - connection_notice_sent = false; - } - if !connection_notice_sent { - if let (Some(client), Some(token)) = ( - &connection_client, - secrets.get("OPENBOT_DESKTOP_HOST_TOKEN"), - ) { - match desktop_connection::poll(client, ports.server, token) { - Ok(Some(connection)) => { - match publish_connection_failure(&app, &root, generation, connection) { - Ok(published) => connection_notice_sent = published, - Err(error) => { - eprintln!("[watch] could not show connection refresh: {error}") - } - } - } - Ok(None) => connection_poll_failed = false, - Err(error) => { - if !connection_poll_failed { - eprintln!("[watch] {error}"); - } - connection_poll_failed = true; - } - } - } + if notice_sent && !recovery_required(&shell, &root) { + notice_sent = false; } - - // Which ones have died. Collected rather than acted on under the lock, because a - // restart waits, and waiting while holding the children is how Stop would block on a - // backoff nobody asked it to sit through. - let dead: Vec<&'static str> = { - let mut children = shell.children.lock().unwrap(); - let mut dead = Vec::new(); - for (name, child) in children.iter_mut() { - if let Ok(Some(_)) = child.try_wait() { - dead.push(*name); - } - } - dead - }; - - if !dead.is_empty() { - eprintln!("[watch] dead: {dead:?}"); + if notice_sent { + continue; } - for name in dead { - let Some(watch) = watches.iter_mut().find(|watch| watch.name == name) else { - continue; - }; - if !watch.should_restart(std::time::Instant::now()) { - // A restore already probing may finish first; publish recovery and its setup - // navigation together after it, so that late probe cannot undo this transition. - let _startup = shell.startup.lock().unwrap(); - // Let go of it. A dead child left in the list is found dead again two seconds - // later, and forever after: the count climbs past what actually happened, the - // window is sent back to the setup screen on a loop, and the giving up that was - // supposed to stop a hot laptop becomes one. - { - let mut children = shell.children.lock().unwrap(); - if shell.generation.load(std::sync::atomic::Ordering::SeqCst) != generation - || shell.root.lock().unwrap().as_deref() != Some(root.as_path()) - { - return; + match desktop_connection::poll(&client, server_port, &token) { + Ok(Some(connection)) => { + match publish_connection_failure(&app, &root, generation, connection) { + Ok(published) => notice_sent = published, + Err(error) => { + eprintln!("[watch] could not show connection refresh: {error}") } - children.retain(|(held, _)| *held != name); } - - let reason = watch.gave_up(); - mark_recovery_required(&shell, &root, generation); - report(&app, name, false, reason.clone()); - /* - * Both registers here too. `gave_up` names the process and quotes the tail of - * its log, which is the developer half; the person needs to know a piece of - * OpenBot stopped and that starting again is the thing to try. - */ - *shell.last_failure.lock().unwrap() = - Some(openbot_desktop_lib::problem::Problem::with( - format!( - "Part of OpenBot ({name}) stopped and could not be started again. \ - Try starting OpenBot once more." - ), - reason, - )); - // Back to the setup screen. By now the window is showing OpenBot, and OpenBot - // is not running: leaving it there is a window that lies. - let _ = show_setup(app.clone()); - continue; - } - report( - &app, - name, - false, - format!("{name} stopped. Starting it again."), - ); - std::thread::sleep(supervise::backoff(watch.restarts - 1)); - - // Asked again after the backoff: a stop, or another start, may have happened while - // this was waiting, and starting a process into either is how an orphan is made. - if shell.generation.load(std::sync::atomic::Ordering::SeqCst) != generation - || shell.root.lock().unwrap().is_none() - { - return; } - let Some(process) = stack::HOST_PROCESSES - .iter() - .find(|process| process.name == name) - else { - continue; - }; - match restart_host_process_with(&shell, &root, name, generation, || { - stack::spawn_host_process(process, &root, &logs, &bun, &secrets) - }) { - Ok(true) => report(&app, name, true, "started again"), - Ok(false) => return, - Err(problem) => { - report(&app, name, false, problem.said.clone()); - *shell.last_failure.lock().unwrap() = Some(problem); + Ok(None) => poll_failed = false, + Err(error) => { + if !poll_failed { + eprintln!("[watch] {error}"); } + poll_failed = true; } } } }) } -/// The same lock covers generation validation, launch, publication, and owned cleanup on every -/// platform. Stop can retire during spawn, but cannot finish before receiving that child handle. -fn restart_host_process_with( - shell: &Shell, - root: &Path, - name: &'static str, - generation: u64, - spawn: F, -) -> Result -where - F: FnOnce() -> std::io::Result, -{ - let mut children = shell.children.lock().unwrap(); - if shell.generation.load(std::sync::atomic::Ordering::SeqCst) != generation - || shell.root.lock().unwrap().as_deref() != Some(root) - { - return Ok(false); - } - let child = spawn().map_err(|error| { - Problem::with( - format!("OpenBot could not restart {name}."), - error.to_string(), - ) - })?; - #[cfg(unix)] - stack::replace_host_process(root, &mut children, name, child)?; - #[cfg(not(unix))] - stack::replace_windows_host_process_with( - root, - &mut children, - name, - child, - Path::new("powershell"), - )?; - Ok(shell.generation.load(std::sync::atomic::Ordering::SeqCst) == generation) -} - /// Point the window at OpenBot if it is up, and at the setup screen if it is not. /// /// Used by the tray and by a second launch, both of which happen at moments when the caller has no @@ -5591,7 +5284,7 @@ fn restore_window_on(app: &tauri::AppHandle, ports: &openb // Restore has the same deployment ownership requirement as the setup page's passive probe. // A successful app-port response alone may belong to another installation or application. let destination = if let Some(url) = (!recovery_required_or_pending_quit_notice(&shell, &root)) - .then(|| owned_app_url(&root, ports)) + .then(|| owned_app_url(&shell, &root, ports)) .flatten() { intelligence_bridge::allow(app, &url); @@ -5866,7 +5559,7 @@ fn install_menus( // The menu bar menu follows the system's appearance; its icons are drawn for each. let dark = system_is_dark(app); - let running = !app.state::().children.lock().unwrap().is_empty(); + let running = app.state::().root.lock().unwrap().is_some(); let dashboard = || { intelligence_menu_item( app.state::() @@ -11044,6 +10737,8 @@ fn main() { "docker-compose.yml", "--profile", "harness", + "--profile", + "openbot", "down", ], ); @@ -11506,33 +11201,33 @@ fn main() { let mut choice = serde_json::json!({"id":"byo-url", "agentUrl":remote}); let (expected_up, expected_image) = match case { "remote" | "remote-stale-image" => ( - "compose up -d --no-build --pull never postgres supervisor agent-computer agent-bot agent-langgraph", + "compose up -d --no-build --pull never postgres supervisor agent-computer server worker agent-bot agent-langgraph", None, ), "anthropic-api" => { model = serde_json::json!({"provider":"anthropic", "login":"api-key", "apiKey":"synthetic-anthropic-key"}); choice = serde_json::json!({"id":"langgraph"}); - ("compose --profile harness up -d --no-build --pull never postgres supervisor agent-computer agent-langgraph agent-harness", Some("agent-langgraph-agui")) + ("compose --profile harness up -d --no-build --pull never postgres supervisor agent-computer server worker agent-langgraph agent-harness", Some("agent-langgraph-agui")) } "compatible" => { model = serde_json::json!({"provider":"openai-compatible", "login":"endpoint", "baseUrl":"http://127.0.0.1:11434/v1", "model":"synthetic-model", "apiKey":""}); - ("compose up -d --no-build --pull never postgres supervisor agent-computer agent-bot agent-langgraph", None) + ("compose up -d --no-build --pull never postgres supervisor agent-computer server worker agent-bot agent-langgraph", None) } "installed" => { choice = serde_json::json!({"id":"langgraph"}); - ("compose --profile harness up -d --no-build --pull never postgres supervisor agent-computer agent-bot agent-langgraph agent-harness", Some("agent-langgraph-agui")) + ("compose --profile harness up -d --no-build --pull never postgres supervisor agent-computer server worker agent-bot agent-langgraph agent-harness", Some("agent-langgraph-agui")) } "none" => { choice = serde_json::Value::Null; - ("compose up -d --no-build --pull never postgres supervisor agent-computer agent-bot agent-langgraph", None) + ("compose up -d --no-build --pull never postgres supervisor agent-computer server worker agent-bot agent-langgraph", None) } "chatgpt-plan" => { model = serde_json::json!({"provider":"openai", "login":"plan", "token":"{\"refresh_token\":\"synthetic-plan\"}"}); - ("compose --profile harness up -d --no-build --pull never postgres supervisor agent-computer agent-harness", Some("agent-langgraph-agui")) + ("compose --profile harness up -d --no-build --pull never postgres supervisor agent-computer server worker agent-harness", Some("agent-langgraph-agui")) } "claude-plan" => { model = serde_json::json!({"provider":"anthropic", "login":"plan", "token":"synthetic-claude-plan"}); - ("compose --profile harness up -d --no-build --pull never postgres supervisor agent-computer agent-harness", Some("agent-claude-sdk")) + ("compose --profile harness up -d --no-build --pull never postgres supervisor agent-computer server worker agent-harness", Some("agent-claude-sdk")) } _ => panic!("unknown test case"), }; @@ -11613,6 +11308,24 @@ fn main() { ); assert!(commands.contains("\tcompose run --rm --pull never migrate\n")); assert!(!root.join(".logs").exists(), "no host runtime was launched"); + // The server container's own credential reaches Compose for `up`, and only Compose. + assert_eq!( + std::fs::read_to_string(root.join("host-token-at-up")).ok(), + Some("43".to_string()), + "case={case}: Compose must be handed the host token before it starts the server" + ); + assert!(!std::fs::read_to_string(root.join(".env")) + .unwrap() + .contains("OPENBOT_DESKTOP_HOST_TOKEN")); + // Compose's bind mount does not create the sign-in folder, so Start must have. + assert!(root.join(".openbot").is_dir()); + // Each Bot's computer goes on the network the server container is on. + assert_eq!( + openbot_env::read_already_set(&root.join(".env"), &["COMPUTER_NETWORK"]) + .unwrap() + .get("COMPUTER_NETWORK"), + Some(&format!("{}_default", stack::project_name(&root))) + ); let settings = openbot_env::read_already_set( &root.join(".env"), &[ @@ -11820,7 +11533,7 @@ fn main() { "{commands}" ); assert!(commands.contains("\tcompose version\n"), "{commands}"); - assert!(commands.contains("\tcompose up -d --no-build --pull never postgres supervisor agent-computer agent-bot agent-langgraph\n"), "{commands}"); + assert!(commands.contains("\tcompose up -d --no-build --pull never postgres supervisor agent-computer server worker agent-bot agent-langgraph\n"), "{commands}"); assert!( commands.contains("\tcompose run --rm --pull never migrate\n"), "{commands}" @@ -11887,13 +11600,13 @@ fn main() { let commands = std::fs::read_to_string(&record).expect("command record"); assert!( commands.contains( - "\tcompose up -d --no-build --pull never postgres supervisor agent-computer agent-langgraph\n" + "\tcompose up -d --no-build --pull never postgres supervisor agent-computer server worker agent-langgraph\n" ), "{commands}" ); assert!( !commands - .contains("compose up -d --no-build --pull never postgres supervisor agent-computer agent-bot"), + .contains("compose up -d --no-build --pull never postgres supervisor agent-computer server worker agent-bot"), "Anthropic Start must not target the OpenAI-only agent-bot: {commands}" ); assert_eq!(problem.said, "Part of OpenBot stopped during startup."); @@ -12589,568 +12302,28 @@ fn main() { std::fs::remove_dir_all(root).unwrap(); } - #[cfg(unix)] - struct InitialHostFixture { - base: PathBuf, - root: PathBuf, - bun: PathBuf, - pids: std::cell::RefCell>, - } - - #[cfg(unix)] - impl InitialHostFixture { - fn new(mode: &str) -> Self { - use std::os::unix::fs::PermissionsExt; - let base = temp_root("initial-host-launch"); - let root = base.join("deployment"); - std::fs::create_dir_all(&root).unwrap(); - for process in stack::HOST_PROCESSES { - if mode == "first-fails" - || ((mode == "second-fails" || mode == "cleanup-refuses") - && process.name != "server") - { - break; - } - std::fs::create_dir(root.join(process.cwd)).unwrap(); - } - let bun = base.join("bun"); - // The production spawn boundary supplies cwd/argv/log files. Only the executable is - // synthetic: one direct child with no network, engine, or credential access. - std::fs::write( - &bun, - "#!/bin/sh\nprintf '%s' \"$$\" > child.pid\nexec /bin/sleep 60\n", - ) - .unwrap(); - std::fs::set_permissions(&bun, std::fs::Permissions::from_mode(0o700)).unwrap(); - Self { - base, - root, - bun, - pids: std::cell::RefCell::new(Vec::new()), - } - } - - fn observe(&self, name: &str) { - let path = self.root.join(name).join("child.pid"); - let until = std::time::Instant::now() + std::time::Duration::from_secs(5); - let pid = loop { - if let Ok(text) = std::fs::read_to_string(&path) { - if let Ok(pid) = text.parse::() { - break pid; - } - } - assert!(std::time::Instant::now() < until, "child did not start"); - std::thread::sleep(std::time::Duration::from_millis(5)); - }; - self.pids.borrow_mut().push(pid); - } - - fn alive(&self) -> Vec { - self.pids - .borrow() - .iter() - .copied() - .filter(|pid| unsafe { libc::kill(*pid as i32, 0) } == 0) - .collect() - } - } - - #[cfg(unix)] - impl Drop for InitialHostFixture { - fn drop(&mut self) { - // Even an old-code regression failure must not orphan the fixture. waitpid first - // proves this is still our direct, unreaped child; ECHILD never authorizes a signal. - for pid in self.pids.get_mut() { - if unsafe { libc::waitpid(*pid as i32, std::ptr::null_mut(), libc::WNOHANG) } == 0 { - unsafe { - libc::kill(*pid as i32, libc::SIGKILL); - libc::waitpid(*pid as i32, std::ptr::null_mut(), 0); - } - } - } - if self.base.is_file() { - std::fs::remove_file(&self.base).unwrap(); - } else { - std::fs::remove_dir_all(&self.base).unwrap(); - } - } - } - - #[cfg(unix)] - struct SupervisionFixture { - host: InitialHostFixture, - app: tauri::App, - watcher: Option>, - } - - #[cfg(unix)] - impl Drop for SupervisionFixture { - fn drop(&mut self) { - let shell = self.app.state::(); - shell - .generation - .fetch_add(1, std::sync::atomic::Ordering::SeqCst); - stop_held_process_handles( - &mut shell - .children - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner), - ) - .unwrap(); - *shell - .root - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) = None; - if let Some(watcher) = self.watcher.take() { - watcher.join().unwrap(); - } - eprintln!( - "{}", - serde_json::json!({"supervisionCleanup": self.host.root, - "heldChildren": shell.children.lock().unwrap_or_else(std::sync::PoisonError::into_inner).len(), "watcherJoined": true}) - ); - } - } - - #[cfg(unix)] - fn failed_retry_keeps_survivor_supervised(failed_role: &str) { - let host = InitialHostFixture::new("success"); - std::fs::write( - &host.bun, - "#!/bin/sh\nprintf '%s' \"$$\" > child.pid\nif [ -f fail ]; then exit 71; fi\nexec /bin/sleep 300\n", - ).unwrap(); - let app = tauri::test::mock_builder() - .manage(Shell::default()) - .invoke_handler(tauri::generate_handler![start_stack]) - .build(tauri::test::mock_context(tauri::test::noop_assets())) - .unwrap(); - let window = tauri::WebviewWindowBuilder::new(&app, "main", Default::default()) - .build() - .unwrap(); - let mut fixture = SupervisionFixture { - host, - app, - watcher: None, - }; - let shell = fixture.app.state::(); - let setup = "tauri://localhost/failed-retry-setup"; - *shell.setup_url.lock().unwrap() = Some(setup.into()); - let attempt = StartAttempt::begin(&shell).unwrap(); - let generation = tauri::async_runtime::block_on(start_host_processes( - &attempt, - &fixture.host.root, - &fixture.host.root.join(".logs"), - &fixture.host.bun, - &stack::Secrets::new(), - |name| fixture.host.observe(name), - |_| Ok(()), - )) - .unwrap(); - drop(attempt); - fixture.watcher = Some(supervise_host_processes( - fixture.app.handle().clone(), - fixture.host.root.clone(), - fixture.host.root.join(".logs"), - fixture.host.bun.clone(), - stack::Secrets::new(), - generation, - openbot_env::Ports::default(), - )); - // The actual watcher exhausts its actual budget and backoffs after this role fails. - std::fs::write(fixture.host.root.join(failed_role).join("fail"), "").unwrap(); - { - let mut children = shell.children.lock().unwrap(); - children - .iter_mut() - .find(|(name, _)| *name == failed_role) - .unwrap() - .1 - .kill() - .unwrap(); - } - let deadline = std::time::Instant::now() + std::time::Duration::from_secs(70); - loop { - if shell - .last_failure - .lock() - .unwrap() - .as_ref() - .is_some_and(|p| p.said.contains("could not be started again")) - && window.url().unwrap().as_str() == setup - { - break; - } - assert!( - std::time::Instant::now() < deadline, - "watcher did not produce recovery setup" - ); - std::thread::sleep(std::time::Duration::from_millis(20)); - } - let survivor_pid = { - let mut children = shell.children.lock().unwrap(); - assert!(!children.iter().any(|(name, _)| *name == failed_role)); - let (_, survivor) = children - .iter_mut() - .find(|(name, _)| *name == "worker") - .unwrap(); - assert!(survivor.try_wait().unwrap().is_none()); - survivor.id() - }; - // Deliberate invalid input rejects before saved-secret/deployment/engine access. This is - // the generated production Start handler, after recovery has exposed ordinary Start. - let problem = tauri::test::get_ipc_response(&window, tauri::webview::InvokeRequest { - cmd: "start_stack".into(), callback: tauri::ipc::CallbackFn(0), error: tauri::ipc::CallbackFn(1), - url: "tauri://localhost".parse().unwrap(), - body: tauri::ipc::InvokeBody::Json(serde_json::json!({ - "root": fixture.host.root, "apiUrl": "https://intelligence.example.test", - "gatewayWsUrl": "wss://gateway.example.test", "apiKey": "synthetic-unused-key", - "model": {"provider": "synthetic-invalid-provider", "login": "api-key"}, "harness": null, - })), - headers: Default::default(), invoke_key: tauri::test::INVOKE_KEY.into(), - }).expect_err("credential preflight must reject the synthetic provider"); - assert!(problem["said"] - .as_str() - .unwrap() - .contains("synthetic-invalid-provider")); - assert_eq!( - shell.root.lock().unwrap().as_ref(), - Some(&fixture.host.root) - ); - assert!(!shell.starting.load(std::sync::atomic::Ordering::SeqCst)); - { - let mut children = shell.children.lock().unwrap(); - let (_, survivor) = children - .iter_mut() - .find(|(name, _)| *name == "worker") - .unwrap(); - assert_eq!(survivor.id(), survivor_pid); - assert!( - survivor.try_wait().unwrap().is_none(), - "preflight unexpectedly reclaimed survivor" - ); - survivor.kill().unwrap(); - } - // A subsequent real child death must still cause the existing watcher to restart it. - let deadline = std::time::Instant::now() + std::time::Duration::from_secs(8); - let replacement = loop { - let replacement = shell - .children - .lock() - .unwrap() - .iter_mut() - .find(|(name, child)| *name == "worker" && child.id() != survivor_pid) - .and_then(|(_, child)| child.try_wait().unwrap().is_none().then_some(child.id())); - if replacement.is_some() - || fixture.watcher.as_ref().unwrap().is_finished() - || std::time::Instant::now() >= deadline - { - break replacement; - } - std::thread::sleep(std::time::Duration::from_millis(20)); - }; - eprintln!( - "{}", - serde_json::json!({ - "failedRole": failed_role, "recoveryUrl": window.url().unwrap().as_str(), - "ipcError": problem, "survivorPid": survivor_pid, "replacementPid": replacement, - "watcherRetired": fixture.watcher.as_ref().unwrap().is_finished(), - "generationBefore": generation, - "generationAfterPreflight": shell.generation.load(std::sync::atomic::Ordering::SeqCst), - }) - ); - assert!( - replacement.is_some(), - "failed retry preflight abandoned the surviving host's watcher" - ); - } - - #[cfg(unix)] - #[test] - fn worker_exhaustion_does_not_adopt_healthy_survivors() { - if crate::test_support::isolated_process( - "tests::worker_exhaustion_does_not_adopt_healthy_survivors", - ) { - return; - } - let host = InitialHostFixture::new("success"); - write_installed_deployment(&host.root); - let source = host.base.join("worker-recovery-host.rs"); - std::fs::write(&source, r#" -use std::{fs,io::{Read,Write},net::TcpListener,time::Duration}; -fn main() { - let cwd=std::env::current_dir().unwrap(); - let role=cwd.file_name().unwrap().to_str().unwrap(); - fs::write("child.pid",std::process::id().to_string()).unwrap(); - let mut starts=fs::OpenOptions::new().create(true).append(true).open("starts.log").unwrap(); - writeln!(starts,"{}",std::process::id()).unwrap(); - if role=="worker" { - loop { - if cwd.join("fail").exists() { std::process::exit(71); } - std::thread::sleep(Duration::from_millis(20)); - } - } - let port=match role { "server"=>3001,"app"=>3010,_=>panic!("unexpected role") }; - let listener=TcpListener::bind(("127.0.0.1",port)).unwrap(); - for stream in listener.incoming() { - let mut stream=stream.unwrap(); - stream.set_read_timeout(Some(Duration::from_secs(2))).unwrap(); - let mut request=[0;2048]; - if stream.read(&mut request).unwrap_or(0)>0 { - let _=stream.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\n{}"); - } - } -} -"#).unwrap(); - crate::test_support::compile_fixture(&source, &host.bun); - let app = tauri::test::mock_builder() - .manage(Shell::default()) - .invoke_handler(tauri::generate_handler![ - already_running, - show_openbot, - last_failure - ]) - .build(tauri::test::mock_context(tauri::test::noop_assets())) - .unwrap(); - let window = tauri::WebviewWindowBuilder::new(&app, "main", Default::default()) - .build() - .unwrap(); - let mut fixture = SupervisionFixture { - host, - app, - watcher: None, - }; - let shell = fixture.app.state::(); - let setup = "tauri://localhost/worker-recovery-setup"; - *shell.setup_url.lock().unwrap() = Some(setup.into()); - let attempt = StartAttempt::begin(&shell).unwrap(); - let logs = fixture.host.root.join(".logs"); - let wait_logs = logs.clone(); - let generation = tauri::async_runtime::block_on(start_host_processes( - &attempt, - &fixture.host.root, - &logs, - &fixture.host.bun, - &stack::Secrets::new(), - |name| fixture.host.observe(name), - move |children| { - stack::wait_until_answering( - children, - &wait_logs, - &stack::Ready { - api: 3001, - app: 3010, - }, - std::time::Duration::from_secs(10), - ) - }, - )) - .unwrap(); - drop(attempt); - fixture.watcher = Some(supervise_host_processes( - fixture.app.handle().clone(), - fixture.host.root.clone(), - logs, - fixture.host.bun.clone(), - stack::Secrets::new(), - generation, - openbot_env::Ports::default(), - )); - std::fs::write(fixture.host.root.join("worker/fail"), "").unwrap(); - let deadline = std::time::Instant::now() + std::time::Duration::from_secs(70); - loop { - if shell - .last_failure - .lock() - .unwrap() - .as_ref() - .is_some_and(|p| { - p.said.contains("(worker)") && p.said.contains("could not be started again") - }) - && window.url().unwrap().as_str() == setup - { - break; - } - assert!( - std::time::Instant::now() < deadline, - "worker did not exhaust actual restart budget" - ); - std::thread::sleep(std::time::Duration::from_millis(20)); - } - assert!(server_capabilities_answer(3001)); - assert!(stack::recorded_server_owns_port(&fixture.host.root, 3001).unwrap()); - assert!(stack::recorded_process_owns_port(&fixture.host.root, "app", 3010).unwrap()); - let invoke = |command: &str, body: serde_json::Value| { - tauri::test::get_ipc_response( - &window, - tauri::webview::InvokeRequest { - cmd: command.into(), - callback: tauri::ipc::CallbackFn(0), - error: tauri::ipc::CallbackFn(1), - url: "tauri://localhost".parse().unwrap(), - body: tauri::ipc::InvokeBody::Json(body), - headers: Default::default(), - invoke_key: tauri::test::INVOKE_KEY.into(), - }, - ) - .map(|response| response.deserialize::().unwrap()) - }; - let result = invoke( - "already_running", - serde_json::json!({"root":fixture.host.root}), - ) - .unwrap() - .as_bool() - .unwrap(); - let children = shell.children.lock().unwrap(); - let survivors: Vec<_> = children - .iter() - .map(|(name, child)| (*name, child.id())) - .collect(); - drop(children); - let starts = std::fs::read_to_string(fixture.host.root.join("worker/starts.log")).unwrap(); - let failure = shell.last_failure.lock().unwrap().clone(); - println!( - "WORKER_RECOVERY_PROOF={}", - serde_json::json!({ - "root":fixture.host.root,"base":fixture.host.base,"generation":generation, - "setupUrl":window.url().unwrap().as_str(),"alreadyRunning":result, - "survivors":survivors,"workerStarts":starts,"failure":failure, - }) - ); - assert_eq!(starts.lines().count(), supervise::MAX_RESTARTS as usize + 1); - assert!( - !result, - "exhausted worker was automatically adopted through generated already_running IPC" - ); - let notification = invoke("last_failure", serde_json::json!({})).unwrap(); - assert!(notification["said"].as_str().unwrap().contains("(worker)")); - assert!(invoke("last_failure", serde_json::json!({})) - .unwrap() - .is_null()); - assert!(recovery_required(&shell, &fixture.host.root)); - let show_error = invoke("show_openbot", serde_json::json!({})).unwrap_err(); - assert!(show_error["said"] - .as_str() - .unwrap() - .contains("needs recovery")); - restore_window_on(fixture.app.handle(), &openbot_env::Ports::default()); - assert_eq!(window.url().unwrap().as_str(), setup); - println!( - "WORKER_RECOVERY_COMMANDS={}", - serde_json::json!({ - "notification":notification,"generatedShowError":show_error, - "recoveryAfterNoticeRead":true,"restoreUrl":window.url().unwrap().as_str(), - }) - ); - } - - #[cfg(unix)] - #[test] - fn recovery_transient_worker_restart_does_not_require_recovery() { - let host = InitialHostFixture::new("success"); - let app = tauri::test::mock_builder() - .manage(Shell::default()) - .build(tauri::test::mock_context(tauri::test::noop_assets())) - .unwrap(); - let mut fixture = SupervisionFixture { - host, - app, - watcher: None, - }; - let shell = fixture.app.state::(); - let attempt = StartAttempt::begin(&shell).unwrap(); - let generation = tauri::async_runtime::block_on(start_host_processes( - &attempt, - &fixture.host.root, - &fixture.host.root.join(".logs"), - &fixture.host.bun, - &stack::Secrets::new(), - |name| fixture.host.observe(name), - |_| Ok(()), - )) - .unwrap(); - drop(attempt); - fixture.watcher = Some(supervise_host_processes( - fixture.app.handle().clone(), - fixture.host.root.clone(), - fixture.host.root.join(".logs"), - fixture.host.bun.clone(), - stack::Secrets::new(), - generation, - openbot_env::Ports::default(), - )); - let original = { - let mut children = shell.children.lock().unwrap(); - let (_, worker) = children - .iter_mut() - .find(|(name, _)| *name == "worker") - .unwrap(); - worker.kill().unwrap(); - worker.id() - }; - let deadline = std::time::Instant::now() + std::time::Duration::from_secs(8); - let replacement = loop { - let replacement = shell - .children - .lock() - .unwrap() - .iter_mut() - .find(|(name, child)| *name == "worker" && child.id() != original) - .and_then(|(_, child)| child.try_wait().unwrap().is_none().then_some(child.id())); - if let Some(pid) = replacement { - break pid; - } - assert!( - std::time::Instant::now() < deadline, - "transient worker was not restarted" - ); - std::thread::sleep(std::time::Duration::from_millis(10)); - }; - assert!(!recovery_required(&shell, &fixture.host.root)); - assert!(shell.last_failure.lock().unwrap().is_none()); - println!( - "WORKER_TRANSIENT_PROOF={}", - serde_json::json!({"original":original,"replacement":replacement,"generation":generation,"recoveryRequired":false}) - ); - } - - #[cfg(unix)] #[test] - fn recovery_ready_start_and_completed_stop_resolve_condition() { - let host = InitialHostFixture::new("success"); + fn recovery_is_resolved_only_by_a_completed_stop() { + let root = temp_root("recovery-completed-stop"); let shell = Shell::default(); - mark_recovery_required(&shell, &host.root, 0); - *shell.last_failure.lock().unwrap() = Some(Problem::plain("worker exhausted")); - assert!(recovery_required(&shell, &host.root)); - assert!(!recovery_required(&shell, &host.base.join("other"))); - let attempt = StartAttempt::begin(&shell).unwrap(); - tauri::async_runtime::block_on(start_host_processes( - &attempt, - &host.root, - &host.root.join(".logs"), - &host.bun, - &stack::Secrets::new(), - |name| host.observe(name), - |_| Ok(()), - )) - .unwrap(); - drop(attempt); - assert!(!recovery_required(&shell, &host.root)); - assert!(shell.last_failure.lock().unwrap().is_none()); - mark_recovery_required(&shell, &host.root, 0); - assert!( - stop_everything_with(&shell, &host.root, stack::stop_processes_under, |_| Err( - "synthetic container cleanup refusal".into() - )) - .is_err() - ); + mark_recovery_required(&shell, &root, 0); + *shell.last_failure.lock().unwrap() = Some(Problem::plain("server refused")); + assert!(recovery_required(&shell, &root)); + assert!(!recovery_required(&shell, &root.join("other"))); + assert!(stop_everything_with( + &shell, + &root, + |_| Ok(0), + |_| Err("synthetic container cleanup refusal".into()) + ) + .is_err()); assert!( - recovery_required(&shell, &host.root), + recovery_required(&shell, &root), "failed Stop cannot resolve recovery" ); - stop_everything_with(&shell, &host.root, stack::stop_processes_under, |_| Ok(())).unwrap(); - assert!(!recovery_required(&shell, &host.root)); - assert!(host.alive().is_empty()); + stop_everything_with(&shell, &root, |_| Ok(0), |_| Ok(())).unwrap(); + assert!(!recovery_required(&shell, &root)); + assert!(shell.last_failure.lock().unwrap().is_none()); } #[test] @@ -13210,7 +12383,7 @@ fn main() { ); assert!(recovery_required(&shell, &f.owned)); assert!( - owned_app_url(&f.owned, &f.ports).is_some(), + owned_app_url(&shell, &f.owned, &f.ports).is_some(), "survivors must still answer" ); assert!(show_openbot_on(app.handle().clone(), &f.ports).is_err()); @@ -13240,7 +12413,7 @@ fn main() { let shell = app.state::(); let window = app.get_webview_window("main").unwrap(); assert!( - owned_app_url(&f.owned, &f.ports).is_some(), + owned_app_url(&shell, &f.owned, &f.ports).is_some(), "survivors must still answer before restore" ); assert!(!recovery_required(&shell, &f.owned)); @@ -13340,210 +12513,17 @@ fn main() { assert!(recovery_required(&app.state::(), &f.owned)); } - #[cfg(unix)] #[test] - fn failed_retry_after_server_exhaustion_keeps_survivor_supervised() { - failed_retry_keeps_survivor_supervised("server"); - } - - #[cfg(unix)] - #[test] - fn failed_retry_after_app_exhaustion_keeps_survivor_supervised() { - failed_retry_keeps_survivor_supervised("app"); - } - - #[cfg(unix)] - fn initial_host_case(mode: &'static str) { - let fixture = InitialHostFixture::new(mode); - let shell = Shell::default(); - let attempt = StartAttempt::begin(&shell).unwrap(); - let result = tauri::async_runtime::block_on(start_host_processes( - &attempt, - &fixture.root, - &fixture.root.join(".logs"), - &fixture.bun, - &stack::Secrets::new(), - |name| { - fixture.observe(name); - if mode == "cleanup-refuses" { - // A real filesystem failure prevents both the second spawn and verification - // of the first child's deployment. No cleanup failure is mocked away. - std::fs::remove_dir_all(&fixture.base).unwrap(); - std::fs::write(&fixture.base, b"blocked fixture parent").unwrap(); - } - }, - move |_| match mode { - "wait-fails" => Err("synthetic readiness failure".into()), - "wait-panics" => panic!("synthetic readiness task panic"), - "success" => Ok(()), - _ => panic!("readiness must not run after a spawn failure"), - }, - )); - let alive = fixture.alive(); - eprintln!( - "{}", - serde_json::json!({ - "initialHostCase": mode, - "spawned": fixture.pids.borrow().len(), - "aliveAfterStart": alive, - "heldAfterStart": shell.children.lock().unwrap().len(), - "error": result.as_ref().err().map(|problem| &problem.said), - }) - ); - if mode == "success" { - assert!(result.is_ok(), "{result:?}"); - assert_eq!(alive.len(), 3); - assert_eq!(stack::recorded_host_pids(&fixture.root).unwrap().len(), 3); - } else { - let problem = result.unwrap_err(); - let expected = match mode { - "first-fails" => "could not start server:", - "second-fails" | "cleanup-refuses" => "could not start app:", - "wait-fails" => "synthetic readiness failure", - "wait-panics" => "the wait did not run:", - _ => unreachable!(), - }; - if matches!(mode, "wait-fails" | "wait-panics") { - assert!( - problem.said.contains("could not finish starting"), - "{problem:?}" - ); - assert!( - problem - .detail - .as_deref() - .unwrap_or_default() - .starts_with(expected), - "{problem:?}" - ); - } else { - assert!(problem.said.starts_with(expected), "{problem:?}"); - } - if mode == "cleanup-refuses" { - assert_eq!(alive.len(), 1); - assert_eq!(shell.children.lock().unwrap().len(), 1); - assert_eq!(shell.root.lock().unwrap().as_ref(), Some(&fixture.root)); - assert!( - problem.detail.is_some(), - "cleanup failure must remain visible" - ); - std::fs::remove_file(&fixture.base).unwrap(); - std::fs::create_dir_all(&fixture.root).unwrap(); - } else { - assert!( - alive.is_empty(), - "failed Start left owned children alive: {alive:?}" - ); - assert!(shell.children.lock().unwrap().is_empty()); - assert!(shell.root.lock().unwrap().is_none()); - } - } - if mode == "success" || mode == "cleanup-refuses" { - retire_host_processes(&shell, &fixture.root, stack::stop_processes_under).unwrap(); - assert!(fixture.alive().is_empty()); - assert!(shell.children.lock().unwrap().is_empty()); - assert!(shell.root.lock().unwrap().is_none()); - } - } - - #[cfg(unix)] - #[test] - fn initial_start_stopped_during_readiness_cannot_publish_and_cleans_real_children() { - use std::sync::{atomic::Ordering::SeqCst, Arc, Barrier}; - let fixture = InitialHostFixture::new("success"); - let shell = Arc::new(Shell::default()); - let entered = Arc::new(Barrier::new(2)); - let release = Arc::new(Barrier::new(2)); - let starter = { - let (shell, root, bun, entered, release) = ( - shell.clone(), - fixture.root.clone(), - fixture.bun.clone(), - entered.clone(), - release.clone(), - ); - std::thread::spawn(move || { - let attempt = StartAttempt::begin(&shell).unwrap(); - tauri::async_runtime::block_on(start_host_processes( - &attempt, - &root, - &root.join(".logs"), - &bun, - &stack::Secrets::new(), - |_| {}, - move |_| { - entered.wait(); - release.wait(); - Ok(()) - }, - )) - }) - }; - entered.wait(); - for process in stack::HOST_PROCESSES { - fixture.observe(process.cwd); - } - assert_eq!( - stack::recorded_host_pids(&fixture.root).unwrap().len(), - stack::HOST_PROCESSES.len() - ); - stop_everything_with(&shell, &fixture.root, stack::stop_processes_under, |_| { - Ok(()) - }) - .unwrap(); - let stopped_generation = shell.generation.load(SeqCst); - assert!(shell.children.lock().unwrap().is_empty()); - assert!(shell.root.lock().unwrap().is_none()); - release.wait(); - let result = starter.join().unwrap(); - let published = shell.children.lock().unwrap().len(); - let active_root = shell.root.lock().unwrap().clone(); - let completed_generation = shell.generation.load(SeqCst); - let alive = fixture.alive(); - // The old-code run must also leave no fixture processes behind. - retire_host_processes(&shell, &fixture.root, stack::stop_processes_under).unwrap(); - eprintln!( - "{}", - serde_json::json!({ - "stoppedGeneration": stopped_generation, - "completedGeneration": completed_generation, - "publishedAfterStop": published, - "aliveAfterStartFinished": alive, - "result": result.as_ref().err().map(|problem| &problem.said), - }) - ); - assert!( - result.is_err(), - "Start succeeded after Stop completed: {result:?}" - ); - assert_eq!(completed_generation, stopped_generation); - assert_eq!(published, 0); - assert!(active_root.is_none()); - assert!( - alive.is_empty(), - "cancelled Start leaked its children: {alive:?}" - ); - } - - #[test] - fn initial_start_cancelled_during_preparation_never_launches_hosts() { + fn initial_start_cancelled_during_preparation_cannot_take_the_startup_lock() { let shell = Shell::default(); let attempt = StartAttempt::begin(&shell).unwrap(); // Deployment/download or dependency preparation returns after Stop has completed. stop_everything_with(&shell, Path::new("unused-root"), |_| Ok(0), |_| Ok(())).unwrap(); - let problem = tauri::async_runtime::block_on(start_host_processes( - &attempt, - Path::new("unused-root"), - Path::new("unused-logs"), - Path::new("must-not-be-launched"), - &stack::Secrets::new(), - |_| panic!("cancelled preparation launched a host"), - |_| panic!("cancelled preparation reached readiness"), - )) - .unwrap_err(); + let problem = attempt + .lock_current() + .expect_err("a cancelled Start resumed a side effect"); assert_eq!(problem.said, StartAttempt::cancelled().said); assert!(shell.root.lock().unwrap().is_none()); - assert!(shell.children.lock().unwrap().is_empty()); assert!( StartAttempt::begin(&shell).is_err(), "cancelled attempt is still unwinding" @@ -13611,283 +12591,6 @@ fn main() { } } - #[cfg(unix)] - #[test] - fn initial_host_second_spawn_failure_cleans_the_real_first_child() { - initial_host_case("second-fails"); - } - - #[cfg(unix)] - #[test] - fn initial_host_first_spawn_failure_never_waits_or_publishes() { - initial_host_case("first-fails"); - } - - #[cfg(unix)] - #[test] - fn initial_host_success_records_then_stops_all_children() { - initial_host_case("success"); - } - - #[cfg(unix)] - #[test] - fn initial_host_readiness_failure_preserves_error_and_cleans_children() { - initial_host_case("wait-fails"); - } - - #[cfg(unix)] - #[test] - fn initial_host_wait_panic_keeps_children_available_for_cleanup() { - initial_host_case("wait-panics"); - } - - #[cfg(unix)] - #[test] - fn initial_host_cleanup_refusal_keeps_original_error_and_stop_ownership() { - initial_host_case("cleanup-refuses"); - } - - #[cfg(unix)] - #[test] - fn failed_host_recording_retires_children_and_preserves_recording_failure() { - let root = temp_root("failed-recording-lifecycle"); - std::fs::create_dir_all(&root).unwrap(); - let worker = std::process::Command::new("/bin/sleep") - .arg("60") - .current_dir(&root) - .spawn() - .unwrap(); - let children = vec![("bogus", worker)]; - let shell = Shell::default(); - let attempt = StartAttempt::begin(&shell).unwrap(); - let result = finish_host_start(&attempt, &root, children, Ok(())).unwrap_err(); - assert_eq!( - result.said, - "OpenBot could not verify its host process ownership." - ); - assert!( - result - .detail - .as_deref() - .is_some_and(|detail| detail.contains("invalid host launch bogus")), - "{result:?}" - ); - assert!(shell.children.lock().unwrap().is_empty()); - assert!(shell.root.lock().unwrap().is_none()); - assert_eq!( - shell.generation.load(std::sync::atomic::Ordering::SeqCst), - 1 - ); - assert!( - !restart_host_process_with(&shell, &root, "worker", 0, || panic!( - "failed recording attempt restarted" - )) - .unwrap() - ); - std::fs::remove_dir_all(root).unwrap(); - } - - #[cfg(unix)] - #[test] - fn failed_host_recording_keeps_root_and_handles_when_forced_cleanup_fails() { - let root = temp_root("failed-recording-stubborn-child"); - std::fs::create_dir_all(&root).unwrap(); - let child = std::process::Command::new("/bin/sleep") - .arg("60") - .current_dir(&root) - .spawn() - .unwrap(); - let pid = child.id(); - let mut children = vec![("server", child)]; - let shell = Shell::default(); - *shell.root.lock().unwrap() = Some(root.clone()); - let result = cleanup_after_host_recording_failure( - &shell, - &root, - &mut children, - Problem::with("recording failed", "recording detail"), - |_, _| { - Err(Problem::with( - "normal cleanup failed", - "normal cleanup detail", - )) - }, - |_| { - Err(Problem::with( - "forced cleanup failed", - "forced cleanup detail", - )) - }, - ) - .unwrap_err(); - - assert_eq!(result.said, "recording failed"); - let detail = result.detail.as_deref().unwrap_or_default(); - assert!(detail.contains("recording detail"), "{detail}"); - assert!(detail.contains("normal cleanup detail"), "{detail}"); - assert!(detail.contains("forced cleanup detail"), "{detail}"); - assert_eq!(shell.root.lock().unwrap().as_deref(), Some(root.as_path())); - assert_eq!(children.len(), 1); - assert_eq!(children[0].1.id(), pid); - for (_, child) in children.iter_mut() { - let _ = child.kill(); - let _ = child.wait(); - } - std::fs::remove_dir_all(root).unwrap(); - } - - #[cfg(unix)] - #[test] - fn failed_host_recording_clears_root_and_handles_when_forced_cleanup_succeeds() { - let root = temp_root("failed-recording-forced-cleanup"); - std::fs::create_dir_all(&root).unwrap(); - let child = std::process::Command::new("/bin/sleep") - .arg("60") - .current_dir(&root) - .spawn() - .unwrap(); - let mut children = vec![("server", child)]; - let shell = Shell::default(); - *shell.root.lock().unwrap() = Some(root.clone()); - let result = cleanup_after_host_recording_failure( - &shell, - &root, - &mut children, - Problem::with("recording failed", "recording detail"), - |_, _| { - Err(Problem::with( - "normal cleanup failed", - "normal cleanup detail", - )) - }, - |children| { - for (_, child) in children.iter_mut() { - let _ = child.kill(); - let _ = child.wait(); - } - children.clear(); - Ok(()) - }, - ) - .unwrap_err(); - - assert_eq!(result.said, "recording failed"); - let detail = result.detail.as_deref().unwrap_or_default(); - assert!(detail.contains("recording detail"), "{detail}"); - assert!(detail.contains("normal cleanup detail"), "{detail}"); - assert!(!detail.contains("forced cleanup"), "{detail}"); - assert!(shell.root.lock().unwrap().is_none()); - assert!(children.is_empty()); - std::fs::remove_dir_all(root).unwrap(); - } - - #[cfg(unix)] - #[test] - fn failed_readiness_retires_children_and_preserves_original_failure() { - let root = temp_root("failed-readiness-lifecycle"); - std::fs::create_dir_all(&root).unwrap(); - let mut failed = std::process::Command::new("/bin/sh") - .args(["-c", "exit 71"]) - .spawn() - .unwrap(); - failed.wait().unwrap(); - let worker = std::process::Command::new("/bin/sleep") - .arg("60") - .current_dir(&root) - .spawn() - .unwrap(); - let mut children = vec![("server", failed), ("worker", worker)]; - let original = stack::wait_until_answering( - &mut children, - &root, - &stack::Ready { api: 0, app: 0 }, - std::time::Duration::from_secs(1), - ) - .unwrap_err(); - let shell = Shell::default(); - let attempt = StartAttempt::begin(&shell).unwrap(); - let result = - finish_host_start(&attempt, &root, children, Err(original.clone())).unwrap_err(); - assert_eq!(result.said, original); - assert!(result.detail.is_none()); - assert!(shell.children.lock().unwrap().is_empty()); - assert!(shell.root.lock().unwrap().is_none()); - assert_eq!( - shell.generation.load(std::sync::atomic::Ordering::SeqCst), - 1 - ); - assert!( - !restart_host_process_with(&shell, &root, "server", 0, || panic!( - "failed attempt restarted" - )) - .unwrap() - ); - std::fs::remove_dir_all(root).unwrap(); - } - - #[cfg(unix)] - #[test] - fn stop_serializes_with_a_restart_already_inside_spawn() { - use std::sync::{atomic::Ordering::SeqCst, Arc, Barrier}; - let root = temp_root("restart-stop-barrier"); - std::fs::create_dir_all(&root).unwrap(); - let shell = Arc::new(Shell::default()); - *shell.root.lock().unwrap() = Some(root.clone()); - shell.generation.store(1, SeqCst); - let entered = Arc::new(Barrier::new(2)); - let release = Arc::new(Barrier::new(2)); - let restart = { - let (shell, root, entered, release) = ( - shell.clone(), - root.clone(), - entered.clone(), - release.clone(), - ); - std::thread::spawn(move || { - restart_host_process_with(&shell, &root, "server", 1, || { - entered.wait(); - release.wait(); - std::process::Command::new("/bin/sleep") - .arg("60") - .current_dir(&root) - .spawn() - }) - }) - }; - entered.wait(); - let (sent, completed) = std::sync::mpsc::channel(); - let stop = { - let (shell, root) = (shell.clone(), root.clone()); - std::thread::spawn(move || { - let result = - stop_everything_with(&shell, &root, stack::stop_processes_under, |_| Ok(())); - sent.send(()).unwrap(); - result - }) - }; - let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); - while shell.generation.load(SeqCst) == 1 { - assert!(std::time::Instant::now() < deadline); - std::thread::yield_now(); - } - assert!(matches!( - completed.try_recv(), - Err(std::sync::mpsc::TryRecvError::Empty) - )); - release.wait(); - assert!(!restart.join().unwrap().unwrap()); - stop.join().unwrap().unwrap(); - assert!(shell.children.lock().unwrap().is_empty()); - assert!(shell.root.lock().unwrap().is_none()); - assert!( - !restart_host_process_with(&shell, &root, "server", 1, || panic!( - "retired restart spawned" - )) - .unwrap() - ); - std::fs::remove_dir_all(root).unwrap(); - } - #[test] fn failed_retry_cleanup_retires_generation_and_retains_selected_root() { let app = tauri::test::mock_builder() @@ -13930,12 +12633,31 @@ fn main() { 5 ); assert_eq!(shell.root.lock().unwrap().as_ref(), Some(&root)); - assert!( - !restart_host_process_with(&shell, &root, "server", 4, || panic!( - "old generation resumed" - )) - .unwrap() - ); + } + + /// Which roots' `server` containers publish which host ports, as far as the code under test + /// can tell. Keyed by root, and every test root is its own temporary folder, so tests running + /// side by side cannot see each other's. + static PUBLISHED_BY_SERVER: Mutex)>> = Mutex::new(Vec::new()); + + pub(super) fn published_by_server(root: &Path, ports: &[u16]) -> bool { + PUBLISHED_BY_SERVER + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .iter() + .any(|(published, by)| { + published == root && !ports.is_empty() && ports.iter().all(|port| by.contains(port)) + }) + } + + fn publish_from_server(root: &Path, ports: &[u16]) { + let mut published = PUBLISHED_BY_SERVER + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + published.retain(|(published, _)| published != root); + if !ports.is_empty() { + published.push((root.to_path_buf(), ports.to_vec())); + } } fn temp_root(name: &str) -> PathBuf { @@ -14061,30 +12783,26 @@ fn main() { assert!( lines.lines().any(|line| line == format!( - "{}\tcompose -f docker-compose.yml --profile harness down", + "{}\tcompose -f docker-compose.yml --profile harness --profile openbot down", root.display() )), "{lines}" ); } - /// Real loopback responder in a separate process, so root/PID ownership checks use the same - /// OS inventory as production. The Tauri mock replaces only the window, never the HTTP/PID path. + /// Real loopback responders in separate processes, so the HTTP probes are the production ones. + /// The Tauri mock replaces only the window, and `publish_from_server` only the engine's word on + /// which root's `server` container publishes the two ports. struct RestoreFixture { base: PathBuf, selected: PathBuf, owned: PathBuf, child: std::process::Child, app_child: std::process::Child, - app_descendant_pid: Option, ports: openbot_env::Ports, } impl RestoreFixture { fn new() -> Self { - Self::with_app_descendant(false) - } - - fn with_app_descendant(descendant: bool) -> Self { let base = temp_root("restore-owned-loopback"); let selected = base.join("selected"); let owned = base.join("owned"); @@ -14111,19 +12829,6 @@ fn serve(listener: TcpListener) { } } fn main() { - let args: Vec = std::env::args().collect(); - if args.get(1).map(String::as_str) == Some("--parent") { - let mut child = std::process::Command::new(std::env::current_exe().unwrap()) - .arg("--record-pid").arg(&args[2]).stdin(std::process::Stdio::null()).spawn().unwrap(); - let mut stop = [0; 1]; - let _ = std::io::stdin().read(&mut stop); - let _ = child.kill(); - let _ = child.wait(); - return; - } - if args.get(1).map(String::as_str) == Some("--record-pid") { - std::fs::write(&args[2], std::process::id().to_string()).unwrap(); - } let api = TcpListener::bind("127.0.0.1:0").unwrap(); let app = TcpListener::bind("127.0.0.1:0").unwrap(); println!("{} {}", api.local_addr().unwrap().port(), app.local_addr().unwrap().port()); @@ -14153,13 +12858,7 @@ fn main() { .split_whitespace() .map(|n| n.parse().unwrap()) .collect(); - let mut app_command = std::process::Command::new(&binary); - let descendant_file = base.join("app-listener.pid"); - if descendant { - app_command.arg("--parent").arg(&descendant_file); - } - let mut app_child = app_command - .stdin(std::process::Stdio::piped()) + let mut app_child = std::process::Command::new(&binary) .current_dir(&owned) .stdout(std::process::Stdio::piped()) .spawn() @@ -14174,33 +12873,19 @@ fn main() { .split_whitespace() .map(|n| n.parse().unwrap()) .collect(); - let app_descendant_pid = descendant.then(|| { - std::fs::read_to_string(descendant_file) - .unwrap() - .parse() - .unwrap() - }); let fixture = Self { base, selected, owned, child, app_child, - app_descendant_pid, ports: openbot_env::Ports { server: numbers[0], app: app_numbers[1], ..Default::default() }, }; - stack::record_host_processes( - &fixture.owned, - &[ - ("server", fixture.child.id()), - ("app", fixture.app_child.id()), - ], - ) - .unwrap(); + publish_from_server(&fixture.owned, &[fixture.ports.server, fixture.ports.app]); fixture } @@ -14227,13 +12912,9 @@ fn main() { fn drop(&mut self) { let _ = self.child.kill(); let _ = self.child.wait(); - if self.app_descendant_pid.is_some() { - // Dropping the pipe asks the fixture launcher to kill and reap its own child. - drop(self.app_child.stdin.take()); - } else { - let _ = self.app_child.kill(); - } + let _ = self.app_child.kill(); let _ = self.app_child.wait(); + publish_from_server(&self.owned, &[]); let _ = std::fs::remove_dir_all(&self.base); } } @@ -14485,8 +13166,8 @@ fn main() { let f = RestoreFixture::new(); assert!(server_capabilities_answer(f.ports.server)); assert!(stack::app_url(f.ports.app).is_some()); - assert!(!stack::recorded_server_owns_port(&f.selected, f.ports.server).unwrap()); - assert!(stack::recorded_server_owns_port(&f.owned, f.ports.server).unwrap()); + assert!(!published_by_server(&f.selected, &[f.ports.server])); + assert!(published_by_server(&f.owned, &[f.ports.server])); for setup in ["tauri://localhost/", "http://tauri.localhost/"] { let app = f.app(&f.selected, setup); restore_window_on(app.handle(), &f.ports); @@ -14527,7 +13208,7 @@ fn main() { app.state::().selected_root.lock().unwrap().as_ref(), Some(&f.owned) ); - assert!(stack::recorded_server_owns_port(&f.owned, f.ports.server).unwrap()); + assert!(published_by_server(&f.owned, &[f.ports.server])); } #[test] @@ -14573,7 +13254,8 @@ fn main() { #[test] fn restore_window_unproven_identity_shows_setup_without_losing_selected_root() { let f = RestoreFixture::new(); - std::fs::write(f.owned.join(".logs/host-pids.json"), "not-json").unwrap(); + // Answering on both ports, but the engine does not say this root's server publishes them. + publish_from_server(&f.owned, &[]); let app = f.app(&f.owned, "tauri://localhost/"); restore_window_on(app.handle(), &f.ports); assert_eq!( @@ -14596,16 +13278,16 @@ fn main() { #[test] fn app_adoption_and_restore_refuse_foreign_app_with_owned_api_still_running() { let f = RestoreFixture::new(); - stack::record_host_processes(&f.owned, &[("server", f.child.id())]).unwrap(); - stack::record_host_processes(&f.selected, &[("app", f.app_child.id())]).unwrap(); + // This root's server publishes the API port; something else has the app port. + publish_from_server(&f.owned, &[f.ports.server]); assert!(already_running_on( &f.owned, f.ports.server, - stack::recorded_server_owns_port + |root, port| { server_container_publishes(None, root, &[port]) } )); assert!(stack::app_url(f.ports.app).is_some()); - let initial_adoption = already_running_at(&f.owned, &f.ports); let app = f.app(&f.owned, "tauri://localhost/"); + let initial_adoption = already_running_at(&app.state::(), &f.owned, &f.ports); let shown = show_openbot_on(app.handle().clone(), &f.ports); restore_window_on(app.handle(), &f.ports); let destination = app.get_webview_window("main").unwrap().url().unwrap(); @@ -14614,25 +13296,23 @@ fn main() { } #[test] - fn app_adoption_and_restore_allow_owned_app_direct_and_launcher_descendant() { - for descendant in [false, true] { - let f = RestoreFixture::with_app_descendant(descendant); - assert_ne!(f.child.id(), f.app_child.id()); - if descendant { - assert_ne!(f.app_descendant_pid.unwrap(), f.app_child.id()); - } - assert!(already_running_at(&f.owned, &f.ports)); - let app = f.app(&f.owned, "tauri://localhost/"); - show_openbot_on(app.handle().clone(), &f.ports).unwrap(); - restore_window_on(app.handle(), &f.ports); - assert_eq!( - app.get_webview_window("main") - .unwrap() - .url() - .unwrap() - .as_str(), - format!("http://127.0.0.1:{}/", f.ports.app) - ); - } + fn app_adoption_and_restore_allow_the_app_its_own_server_container_publishes() { + let f = RestoreFixture::new(); + let app = f.app(&f.owned, "tauri://localhost/"); + assert!(already_running_at( + &app.state::(), + &f.owned, + &f.ports + )); + show_openbot_on(app.handle().clone(), &f.ports).unwrap(); + restore_window_on(app.handle(), &f.ports); + assert_eq!( + app.get_webview_window("main") + .unwrap() + .url() + .unwrap() + .as_str(), + format!("http://127.0.0.1:{}/", f.ports.app) + ); } } diff --git a/desktop/src-tauri/src/preparation.rs b/desktop/src-tauri/src/preparation.rs index 502af65741..922e8a3854 100644 --- a/desktop/src-tauri/src/preparation.rs +++ b/desktop/src-tauri/src/preparation.rs @@ -316,11 +316,16 @@ fn complete_with( } /// `selection=None` is the auth check, which has no authority to change the Bot selection. +/// +/// No runtime and no packages: OpenBot's server and worker are containers now, so what Start needs +/// from the installation is the deployment it laid out and the images it downloaded. Installation +/// still records the host packages, for the self-hosted Intelligence sign-in that runs on the host, +/// and that sign-in checks them itself. pub fn require( root: &Path, selection: Option<&Option>, address: &Address, -) -> Result { +) -> Result<(), Problem> { require_with(root, selection, |images| require_images(address, images)) } @@ -328,7 +333,7 @@ fn require_with( root: &Path, selection: Option<&Option>, verify: impl FnOnce(&[String]) -> Result<(), Problem>, -) -> Result { +) -> Result<(), Problem> { let record: Prepared = serde_json::from_slice( &std::fs::read(root.join(FILE)).map_err(|e| required(e.to_string()))?, ) @@ -352,9 +357,7 @@ fn require_with( "The deployment was laid out by an earlier OpenBot and is missing files this version runs.", )); } - let bun = dependencies_ready(root)?; - verify(&record.images)?; - Ok(bun) + verify(&record.images) } /// Public Compose overrides suffice to identify and download software before authentication. @@ -553,7 +556,7 @@ fn main() { } #[test] - fn changed_bot_lockfile_or_deleted_package_requires_installation() { + fn changed_bot_requires_installation_but_host_packages_do_not() { let f = Fixture::new(); let chosen = Some(HarnessChoice { id: "langgraph".into(), @@ -565,11 +568,14 @@ fn main() { "wrong selection must stop first" )) .is_err()); + // Start runs containers, so the host's packages and runtime are not its business. std::fs::remove_file(f.root.join("node_modules/synthetic-package/package.json")).unwrap(); - assert!(require_with(&f.root, Some(&chosen), |_| Ok(())).is_err()); - f.dependencies(); - f.prepared(chosen.as_ref()); - std::fs::write(f.root.join("bun.lock"), "changed-lock").unwrap(); + std::fs::write(f.root.join("runtime-version"), "1.2.15").unwrap(); + require_with(&f.root, Some(&chosen), |_| Ok(())).unwrap(); + // The images still are. + assert!(require_with(&f.root, Some(&chosen), |_| Err(required("missing image"))).is_err()); + // And the deployment Compose runs. + std::fs::write(f.root.join("docker-compose.yml"), "services: {x: {}}\n").unwrap(); assert!(require_with(&f.root, Some(&chosen), |_| Ok(())).is_err()); } diff --git a/desktop/src-tauri/src/stack.rs b/desktop/src-tauri/src/stack.rs index 64788af15c..5880e49692 100644 --- a/desktop/src-tauri/src/stack.rs +++ b/desktop/src-tauri/src/stack.rs @@ -2676,6 +2676,93 @@ pub fn ports_we_already_publish(engine: &Address, root: &Path) -> std::collectio published_in(&listing) } +/** +Whether this deployment's own `server` container publishes every one of `ports` on the host. + +THIS IS WHAT OWNERSHIP MEANS NOW. The server and the app used to be host processes, and a port +belonged to this deployment when a process recorded in its `host-pids.json` was listening on it. +They are one container now, and the listener on the host is the engine's port forwarder, which no +pid file names. What does name it is Compose: this project's `server` row lists the host ports it +was given. Something else answering on the same port, another installation's server or a stranger, +is not in that row. + +Every service rather than `server` alone, read row by row: naming a service on `ps` makes Compose +resolve it, and `server` sits behind a profile this command would otherwise have to repeat. +*/ +pub fn server_publishes(engine: &Address, root: &Path, ports: &[u16]) -> Result { + let operation = format!("{} compose ps", engine.engine.binary()); + let output = compose_command(engine, root, &Secrets::new()) + .args(["ps", "--format", "{{.Service}}\t{{.Ports}}"]) + .output() + .map_err(|error| { + Problem::with( + "OpenBot could not inspect its Compose services.", + format!("could not run {operation}: {error}"), + ) + })?; + if !output.status.success() { + return Err(Problem::with( + "OpenBot could not inspect its Compose services.", + format!("{operation}: {}", command_said(&output.stderr)), + )); + } + Ok(server_publishes_in( + &String::from_utf8_lossy(&output.stdout), + ports, + )) +} + +/// `server_publishes` over a `{{.Service}}\t{{.Ports}}` listing. Pure, for the reason +/// `published_in` is. +pub fn server_publishes_in(listing: &str, ports: &[u16]) -> bool { + let published: std::collections::HashSet = listing + .lines() + .filter_map(|row| row.split_once('\t')) + .filter(|(service, _)| service.trim() == "server") + .flat_map(|(_, mappings)| published_in(mappings)) + .collect(); + !ports.is_empty() && ports.iter().all(|port| published.contains(port)) +} + +/** +Restart OpenBot itself, the server and the worker, and leave everything else running. + +What the Status window's Restart did to the host processes, now done by the engine that runs them: +the database and the Bots stay up, so nothing is lost and it takes seconds. The profile is named +because both services sit behind it, and Compose refuses to restart a service it cannot see. +*/ +pub fn restart_openbot(engine: &Address, root: &Path) -> Result<(), String> { + let output = compose_command(engine, root, &Secrets::new()) + .args(["--profile", OPENBOT_PROFILE, "restart", "server", "worker"]) + .output() + .map_err(|error| format!("could not restart OpenBot: {error}"))?; + if output.status.success() { + return Ok(()); + } + Err(command_said(&output.stderr)) +} + +/** +The name Compose gives this deployment's project, and so the prefix of its networks. + +The desktop runs Compose from the deployment folder with no `-p` and the file names no `name:`, so +the project is the folder's own name as Compose normalises it: lower case, with everything but +`a-z`, `0-9`, `_` and `-` dropped, then any leading `_` or `-` trimmed. Worked out here rather than +asked of Compose, because the answer is wanted before `up`, to tell the supervisor which network to +put each Bot's computer on, and it has to match what `up` is about to create. +*/ +pub fn project_name(root: &Path) -> String { + let base = root + .file_name() + .map(|name| name.to_string_lossy().to_lowercase()) + .unwrap_or_default(); + base.chars() + .filter(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || *c == '_' || *c == '-') + .collect::() + .trim_start_matches(['_', '-']) + .to_string() +} + /** The published ports in a `compose ps` listing. @@ -2762,21 +2849,11 @@ pub fn port_already_taken_except( None } -/// Wait until the API answers, or say why it never did. -/// -/// Spawning is not starting. Each of these three can exit in the first second for a reason that has -/// nothing to do with the others, and a shell that reports "running" because it called `spawn` -/// three times is telling somebody the stack is up while nothing is listening. That is worse than -/// an error, because the next thing they do is open a page that will not load and go looking for -/// the fault in the wrong place. -/// -/// So: watch the child, and watch the port. Whichever fails first is what gets reported, with the -/// tail of the log that explains it. -/// The two things that have to answer before anybody is told the stack is up. +/// The two addresses that have to answer before anybody is told the stack is up. /// /// The API alone is not enough. The window navigates to the app, so a person told "running" who -/// then gets a blank window has been told something that is not true, and the API was answering the -/// whole time. +/// then gets a blank window has been told something that is not true. Both are the one `server` +/// container now, published on two host ports, and either mapping can be the one that is missing. pub struct Ready { pub api: u16, pub app: u16, @@ -2814,33 +2891,28 @@ pub fn app_url(port: u16) -> Option { answering_at(port, "/") } -/// Wait until the stack is genuinely usable, or say which part is not. +/// Wait until the server container answers its health check on both host ports, or say which +/// one never did. /// -/// Watches the children as well as the ports, because three processes that died leave a port -/// unanswered for the same length of time as three that are still starting, and only one of those -/// is worth waiting out. -pub fn wait_until_answering( - children: &mut [(&'static str, std::process::Child)], - logs: &Path, +/// `compose up` returns once it has asked for the container, not once the server inside it is +/// listening, and a server still running its first start leaves the port unanswered for the same +/// length of time as one that is crash-looping under its restart policy. Only an answer settles it. +/// +/// `keep_going` is asked between polls, so a Stop pressed during the wait ends it there rather than +/// after the full patience. What a stopped wait returns does not matter: the caller asks its own +/// ticket first, and the cancellation is what it reports. +pub fn wait_until_healthy( ready: &Ready, patience: std::time::Duration, + keep_going: impl Fn() -> bool, ) -> Result<(), String> { let deadline = std::time::Instant::now() + patience; let mut api_up = false; - while std::time::Instant::now() < deadline { - for (name, child) in children.iter_mut() { - if let Ok(Some(status)) = child.try_wait() { - return Err(format!( - "{name} stopped straight away ({status}). {}", - tail_of(logs, name) - )); - } - } - + while std::time::Instant::now() < deadline && keep_going() { // An earlier API success is no longer sufficient when the app becomes ready later. - api_up = answering_at(ready.api, "/api/capabilities").is_some(); - if api_up && app_url(ready.app).is_some() { + api_up = answering_at(ready.api, "/health").is_some(); + if api_up && answering_at(ready.app, "/health").is_some() { return Ok(()); } @@ -2849,56 +2921,11 @@ pub fn wait_until_answering( if api_up { return Err(format!( - "the API is answering, but the app is not answering on port {}. {}", - ready.app, - tail_of(logs, "app") + "the API is answering, but the app is not answering on port {}.", + ready.app )); } - Err(format!( - "the API is not answering on port {}. {}", - ready.api, - tail_of(logs, "server") - )) -} - -/// The last build error can precede Bun's wrapper stack, version, and exit message. -/// Bound both the file read and displayed lines while preserving that useful context. -fn tail_of(logs: &Path, name: &str) -> String { - use std::io::{Read, Seek, SeekFrom}; - const MAX_BYTES: u64 = 8 * 1024; - let read = || -> std::io::Result> { - let mut file = std::fs::File::open(logs.join(format!("{name}.log")))?; - let offset = file.metadata()?.len().saturating_sub(MAX_BYTES); - file.seek(SeekFrom::Start(offset))?; - let mut bytes = Vec::new(); - file.take(MAX_BYTES).read_to_end(&mut bytes)?; - // Do not show a truncated first line, which may include part of a credential. - if offset > 0 { - let first_line = bytes.iter().position(|byte| *byte == b'\n'); - bytes.drain(..first_line.map_or(bytes.len(), |index| index + 1)); - } - Ok(bytes) - }; - let bytes = match read() { - Ok(bytes) => bytes, - Err(error) => return format!("Could not read {name}.log: {error}"), - }; - let text = String::from_utf8_lossy(&bytes); - let tail: Vec<&str> = text - .lines() - .filter(|line| !line.trim().is_empty()) - .rev() - .take(40) - .collect(); - if tail.is_empty() { - return format!("No complete lines were available in the tail of {name}.log."); - } - let mut lines = tail; - lines.reverse(); - format!( - "Last from {name}.log (up to 40 lines, 8 KiB):\n{}", - lines.join("\n") - ) + Err(format!("the API is not answering on port {}.", ready.api)) } /// Keep a short startup headline and useful local diagnostics, with credentials removed. @@ -3041,29 +3068,6 @@ mod tests { use super::*; use crate::test_support::temp_root; - #[test] - fn startup_log_tail_retains_build_error_before_wrapper_without_loading_whole_log() { - let root = temp_root("startup-build-diagnostic"); - std::fs::create_dir_all(&root).unwrap(); - let mut log = "old output must be omitted\n".repeat(1_000); - log.push_str("error during build: Could not resolve imported module\n"); - log.push_str(&" at synthetic build frame\n".repeat(20)); - log.push_str(" at run (app/scripts/serve-or-build.ts:36:11)\n"); - log.push_str("Bun v1.2.15 (macOS arm64)\nerror: script serve exited with code 1\n"); - std::fs::write(root.join("app.log"), log).unwrap(); - - let detail = tail_of(&root, "app"); - assert!( - detail.contains("Could not resolve imported module"), - "{detail}" - ); - assert!(detail.contains("serve-or-build.ts:36:11"), "{detail}"); - assert!(!detail.contains(&"old output must be omitted\n".repeat(20))); - assert!(detail.len() < 8_500, "unbounded startup diagnostic"); - assert!(detail.lines().count() <= 42, "too many startup log lines"); - std::fs::remove_dir_all(root).unwrap(); - } - #[test] fn startup_problem_keeps_build_and_cleanup_details_but_removes_credentials() { let secrets = Secrets::from([ @@ -7128,7 +7132,14 @@ fn main() { let services = up(&podman, &root, true, BundledBots::none(), &Secrets::new()).unwrap(); assert_eq!( services, - ["postgres", "supervisor", "agent-computer", "agent-harness"] + [ + "postgres", + "supervisor", + "agent-computer", + "server", + "worker", + "agent-harness" + ] ); migrate(&podman, &root, &Secrets::new()).unwrap(); let log = std::fs::read_to_string(&record).unwrap(); @@ -7282,7 +7293,9 @@ fn main() { .unwrap(); let snapshot = log.find("ps --quiet").unwrap(); let computers = log.find("stop current late restarted").unwrap(); - let teardown = log.find("--profile harness down").unwrap(); + let teardown = log + .find("--profile harness --profile openbot down") + .unwrap(); assert!( config < supervisor && supervisor < snapshot @@ -7361,7 +7374,7 @@ fn main() { ); assert!( log.contains(&format!( - "{prefix}compose -f docker-compose.yml --profile harness down" + "{prefix}compose -f docker-compose.yml --profile harness --profile openbot down" )), "{log}" ); @@ -7579,7 +7592,7 @@ fn main() { } fn wait(&mut self, patience: std::time::Duration) -> Result<(), String> { - wait_until_answering(&mut self.children, &self.root, &self.ready, patience) + wait_until_healthy(&self.ready, patience, || true) } fn requests(&self, name: &str) -> String { @@ -7617,31 +7630,30 @@ fn main() { .children .iter_mut() .all(|(_, child)| child.try_wait().unwrap().is_none()); - let api_now = answering_at(fixture.ready.api, "/api/capabilities"); - let app_now = app_url(fixture.ready.app); + let api_now = answering_at(fixture.ready.api, "/health"); + let app_now = answering_at(fixture.ready.app, "/health"); eprintln!("current API readiness: result={result:?}; children_alive={live}; api_requests={api_requests:?}; api_now={api_now:?}; app_now={app_now:?}"); drop(fixture); assert!(live && api_now.is_none() && app_now.is_some()); let error = result.expect_err("a historical API success cannot satisfy readiness"); assert!(api_requests.lines().count() >= 2, "{api_requests}"); assert!(error.contains("the API is not answering"), "{error}"); - assert!(error.contains("synthetic server diagnostic"), "{error}"); } #[test] - fn readiness_accepts_currently_healthy_api_and_app() { + fn readiness_asks_the_server_health_check_on_both_ports() { let mut fixture = ReadinessFixture::new("200", "200"); let result = fixture.wait(std::time::Duration::from_secs(2)); let api_requests = fixture.requests("server"); let app_requests = fixture.requests("app"); drop(fixture); assert!(result.is_ok(), "{result:?}"); - assert!(api_requests.contains("200 GET /api/capabilities HTTP/1.1")); - assert!(app_requests.contains("200 GET / HTTP/1.1")); + assert!(api_requests.contains("200 GET /health HTTP/1.1")); + assert!(app_requests.contains("200 GET /health HTTP/1.1")); } #[test] - fn readiness_timeout_names_the_currently_unavailable_service() { + fn readiness_timeout_names_the_currently_unavailable_port() { for (api, app, missing) in [("503", "200", "server"), ("200", "503", "app")] { let mut fixture = ReadinessFixture::new(api, app); let result = fixture.wait(std::time::Duration::from_millis(100)); @@ -7657,26 +7669,52 @@ fn main() { error.contains(&format!("the {service} is not answering on port {port}")), "{error}" ); - assert!( - error.contains(&format!("synthetic {missing} diagnostic")), - "{error}" - ); } } #[test] - fn readiness_reports_child_exit_without_waiting_for_timeout() { - let mut fixture = ReadinessFixture::new("exit", "200"); + fn readiness_ends_at_once_when_the_start_is_stopped() { + // Port 1 needs privilege to bind, so nothing here will ever answer it. + let ready = Ready { api: 1, app: 1 }; let started = std::time::Instant::now(); - let result = fixture.wait(std::time::Duration::from_secs(30)); - let elapsed = started.elapsed(); - let status = fixture.children[0].1.try_wait().unwrap().unwrap(); - drop(fixture); - let error = result.unwrap_err(); - assert!(elapsed < std::time::Duration::from_secs(5), "{elapsed:?}"); - assert_eq!(status.code(), Some(17)); - assert!(error.contains("server stopped straight away"), "{error}"); - assert!(error.contains("synthetic server diagnostic"), "{error}"); + let result = wait_until_healthy(&ready, std::time::Duration::from_secs(30), || false); + assert!(result.is_err()); + assert!( + started.elapsed() < std::time::Duration::from_secs(5), + "a stopped Start sat out the readiness wait" + ); + } + + #[test] + fn project_name_is_the_folder_name_as_compose_normalises_it() { + assert_eq!(project_name(Path::new("/Users/someone/OpenBot")), "openbot"); + assert_eq!( + project_name(Path::new("/home/someone/My OpenBot!")), + "myopenbot" + ); + assert_eq!(project_name(Path::new("/srv/_x")), "x"); + assert_eq!(project_name(Path::new("/srv/-_open.bot_2-")), "openbot_2-"); + assert_eq!( + project_name(Path::new("C:/Users/someone/ÖpenBot")), + "penbot" + ); + } + + #[test] + fn only_the_server_row_can_own_the_app_and_api_ports() { + let listing = "postgres\t127.0.0.1:5432->5432/tcp\n\ + server\t127.0.0.1:3010->3001/tcp, [::1]:3010->3001/tcp, 127.0.0.1:3001->3001/tcp, [::1]:3001->3001/tcp\n\ + worker\t\n"; + assert!(server_publishes_in(listing, &[3001, 3010])); + assert!(server_publishes_in(listing, &[3001])); + // Published by the project, but not by the server: still not the app. + assert!(!server_publishes_in(listing, &[3001, 5432])); + assert!(!server_publishes_in(listing, &[3002])); + assert!(!server_publishes_in("", &[3001])); + assert!( + !server_publishes_in(listing, &[]), + "nothing asked is nothing proven" + ); } #[test] diff --git a/desktop/src-tauri/src/stop_ipc_tests.rs b/desktop/src-tauri/src/stop_ipc_tests.rs index 31bdeede89..9793454287 100644 --- a/desktop/src-tauri/src/stop_ipc_tests.rs +++ b/desktop/src-tauri/src/stop_ipc_tests.rs @@ -185,7 +185,8 @@ mod stop_ipc { let fixture = Fixture::new(); let compose = fixture.compose_barrier(); let shell = fixture.app.state::(); - let cleanup = shell.children.lock().unwrap(); + // The first lock host cleanup takes; held here, it keeps that cleanup waiting. + let cleanup = shell.host_access.lock().unwrap(); let dispatch = fixture.dispatch(); wait_until(|| shell.generation.load(SeqCst) == 1); // Always release both barriers before asserting against the old synchronous command. @@ -278,18 +279,10 @@ mod stop_ipc { pending, "Stop finished while Start still held the side-effect lock" ); - let error = tauri::async_runtime::block_on(start_host_processes( - &attempt, - &fixture.root, - &fixture.root.join(".logs"), - Path::new("must-not-launch"), - &stack::Secrets::new(), - |_| panic!("cancelled Start published a child"), - |_| panic!("cancelled Start reached readiness"), - )) - .unwrap_err(); + let error = attempt + .lock_current() + .expect_err("a cancelled Start resumed a side effect"); assert_eq!(error.said, StartAttempt::cancelled().said); - assert!(shell.children.lock().unwrap().is_empty()); assert!(shell.root.lock().unwrap().is_none()); } @@ -393,7 +386,7 @@ mod stop_ipc { let shell = fixture.app.state::(); // A poisoned cleanup lock is a real panic boundary; it must reject IPC, not abandon it. std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { - let _cleanup = shell.children.lock().unwrap(); + let _cleanup = shell.host_access.lock().unwrap(); panic!("synthetic cleanup lock poisoning"); })) .expect_err("poison the cleanup lock"); diff --git a/desktop/src-tauri/tests/fixtures/engine.rs b/desktop/src-tauri/tests/fixtures/engine.rs index 4f8bf8c90e..ab78c91875 100644 --- a/desktop/src-tauri/tests/fixtures/engine.rs +++ b/desktop/src-tauri/tests/fixtures/engine.rs @@ -94,7 +94,7 @@ fn main() { if SCENARIO == "stop-ipc" && (joined.starts_with("ps ") || joined.contains("stop supervisor")) { return; } - if SCENARIO == "stop-ipc" && joined == "compose -f docker-compose.yml --profile harness down" { + if SCENARIO == "stop-ipc" && joined == "compose -f docker-compose.yml --profile harness --profile openbot down" { let mut barrier = std::net::TcpStream::connect( std::fs::read_to_string("stop-barrier-address") .unwrap() @@ -137,7 +137,14 @@ fn main() { value if value.starts_with("compose up -d --no-build --pull never ") || value - .starts_with("compose --profile harness up -d --no-build --pull never ") => {} + .starts_with("compose --profile harness up -d --no-build --pull never ") => + { + // Whether Compose was handed the desktop's host token, by its length only: the + // token itself is never written anywhere. + if let Some(token) = std::env::var_os("OPENBOT_DESKTOP_HOST_TOKEN") { + std::fs::write("host-token-at-up", token.len().to_string()).unwrap(); + } + } "compose run --rm --pull never migrate" => { if SCENARIO == "harness" { eprintln!("synthetic migration barrier"); diff --git a/desktop/src/StatusWindow.tsx b/desktop/src/StatusWindow.tsx index f568d15a93..ed04bb3318 100644 --- a/desktop/src/StatusWindow.tsx +++ b/desktop/src/StatusWindow.tsx @@ -97,8 +97,8 @@ const BUSY_SAID: Record = { /** * What OpenBot is running, what each part costs, and Restart and Stop for each. * - * Memory is read every few seconds while the window shows; it is Docker's figure per container and - * the resident size of OpenBot's own processes. Intelligence's readiness is the CLI's, asked only on + * Memory is read every few seconds while the window shows; it is Docker's figure per container, + * OpenBot's own server and worker among them. Intelligence's readiness is the CLI's, asked only on * opening, on Refresh and after an action here: the main window's own Intelligence calls take the * same lock, and a status window polling it would make them fail as busy. */ @@ -211,7 +211,7 @@ export function StatusWindow({ readEveryMs = 5000 }: { readEveryMs?: number }) { {status.openbotRunning && (
- {/* Restart is the supervisor bringing back processes this session holds. */} + {/* Restart is Compose restarting the server and worker containers this session started. */} {status.openbotHeld && (