From ef92288e45f3bb636a711435f65ff6e65d5e4e3d Mon Sep 17 00:00:00 2001 From: "propr-dev[bot]" <1316198+propr-dev[bot]@users.noreply.github.com> Date: Sat, 26 Sep 2026 19:26:40 +0000 Subject: [PATCH 01/22] Implementation complete. Summary of what I built for #2547: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three-state mode - `packages/shared/src/agentTank.ts` (new): `AGENT_TANK_MODES`, `normalizeAgentTankMode`, `isAgentTankMode`, `agentTankModeFromLegacyEnabled` — one vocabulary for core/API/CLI/UI. - `configManagerAgents.ts`: `AgentTankSettings` gains `mode`; `enabled` stays as a derived read-only field so all existing call sites keep working. `loadAgentTankSettings` reads the persisted value as `unknown` and normalizes, so a legacy `{enabled:true,url}` loads as `{mode:'external',url}` and `{enabled:false}` as `disabled`. `AGENT_TANK_MODE` is an env fallback only when no record exists. Bundled runner - `agentTankBundledRunner.ts` (new): builds the config, runs `agent-tank --once --json --config …` in the configured agent image with each enabled agent's credential dir bind-mounted `:ro` at `CONTAINER_CONFIG_PATHS`, parses output, and owns the 60s TTL cache + in-flight coalescing. Every failure path returns `undefined` and leaves the prior snapshot intact. - I verified the config/output schema against `integry/agent-tank` rather than using the issue's speculative one. Upstream takes `agents: [{ provider, id, configPath }]` (not `name`/`home`), where `configPath` becomes `CLAUDE_CONFIG_DIR`/`CODEX_HOME`/`GEMINI_CLI_HOME`; `--once --json` prints a bare status map. Published version is 0.9.10, so that's the pin (not `1.4.0`). OpenCode/Vibe are excluded — upstream rejects unknown providers, which would fail the whole run. - `agentTankTypes.ts` (new): the provider-key mapping and `AgentStatusResponse` moved here so the runner and the transport router don't import each other in a cycle; `agentTankService` re-exports them unchanged. Surfaces `agentTankService` routes by mode (bundled `refreshAgent` only *schedules*; `getStatus` is cache-only bounded by a 90s delta-freshness window) and gains `getAllStatuses`. API routes read/write `mode` and still accept legacy `{enabled}` bodies, preserving a saved URL when none is sent. `propr tank bundled|external|off` with `on` as a deprecated alias for `external`. MCP `update_provider_policy` takes `mode`, requiring `url` only for external. Settings UI is a radio group; the URL field exists only in external mode and bundled gets its own "ready/unavailable" wording. Image `Dockerfile.agent` gets an `agent-tank-cli` stage pinned at `ARG AGENT_TANK_CLI_VERSION=0.9.10` (so it feeds the bundle content hash), a label, the bin link and a `--version` check. `agent-entrypoint.sh` dispatches `agent-tank` directly — skipping every per-agent ownership repair — and drops to `node` if root. `build-images.sh` passes the matching build arg. Verification - New tests: settings migration (9), bundled runner incl. 10-concurrent-refresh → 1 container (9), mode routing (9), API routes (2), CLI (5), UI section (6), plus supply-chain and bundle-tag-changes assertions. 58 pass across the agent-tank suites. - `npx tsc --noEmit` clean in root, api, cli, propr-ui; lint clean in root, core, api, cli, propr-ui. Full UI vitest suite: 1411 passed. - Docs rewritten: `agent-tank.md` restructured around the three modes, `configuration-reference.md` documents the new env vars, plus `propr-cli.md`, `observability.md`, `mcp-coverage.md`, `dockerhub/agent.md`. - I captured Playwright previews of the new settings section in all three states (a new `e2e/agent-tank-modes.pw.ts` follows the repo's existing `PROPR_CAPTURE_PREVIEWS` pattern). One thing I could not verify: `docker run --rm propr/agent: agent-tank --version` needs an actual image build, which isn't possible here. The Dockerfile `RUN` itself asserts `agent-tank --version` in both the install stage and the final stage, so a build failure would surface at build time. Also worth flagging: `test/ciRunnerRouting.test.mjs` fails on this branch, but it's unrelated to this work — it hard-codes two `apps/desktop/scripts/*electron*.test.mjs` files while a third (`published-preview-electron.test.mjs`, added in commit b59b10c6) now exists. My changes touch nothing under `apps/` or `.github/`. --- Dockerfile.agent | 23 +- dockerhub/agent.md | 5 + docs/docs/features/observability.md | 2 +- docs/docs/features/propr-cli.md | 2 +- docs/docs/operations/agent-tank.md | 113 +++++-- .../operations/configuration-reference.md | 7 +- docs/mcp-coverage.md | 2 +- packages/api/mcp/toolsConfiguration.ts | 20 +- packages/api/routes/configRoutesAgentTank.ts | 100 ++++-- .../api/test/configRoutesAgentTank.test.ts | 105 ++++++ .../api/test/mcpReviewerConcurrency.test.ts | 9 +- packages/cli/src/api/agentTank.ts | 21 +- .../cli/src/commands/tankCommands.test.ts | 73 ++++ packages/cli/src/commands/tankCommands.ts | 69 +++- .../agents/impl/utils/usageTrackingWrapper.ts | 13 +- packages/core/src/config/configManager.ts | 2 + .../core/src/config/configManagerAgents.ts | 74 +++- packages/core/src/index.ts | 12 +- .../src/services/agentTankBundledRunner.ts | 320 ++++++++++++++++++ .../core/src/services/agentTankService.ts | 130 ++++--- packages/core/src/services/agentTankTypes.ts | 62 ++++ packages/shared/src/agentTank.ts | 39 +++ packages/shared/src/index.ts | 11 + propr-ui/e2e/agent-tank-modes.pw.ts | 120 +++++++ propr-ui/src/api/revertApi.ts | 13 +- .../components/AgentTankDetectionBanner.tsx | 20 +- .../SettingsPage/AgentTankSection.test.tsx | 60 ++++ .../pages/SettingsPage/AgentTankSection.tsx | 124 ++++--- propr-ui/src/pages/SettingsPage/index.tsx | 2 +- .../src/pages/SettingsPage/parseLoadedData.ts | 10 +- .../pages/SettingsPage/useSettingsState.ts | 16 +- scripts/agent-entrypoint.sh | 16 +- scripts/build-images.sh | 5 + test/agentDockerfileSupplyChain.test.ts | 17 + test/agentTankBundledRunner.test.ts | 211 ++++++++++++ test/agentTankService.test.ts | 111 +++++- test/agentTankSettingsMigration.test.ts | 128 +++++++ test/agentVersionManagement.test.ts | 33 +- 38 files changed, 1860 insertions(+), 240 deletions(-) create mode 100644 packages/api/test/configRoutesAgentTank.test.ts create mode 100644 packages/cli/src/commands/tankCommands.test.ts create mode 100644 packages/core/src/services/agentTankBundledRunner.ts create mode 100644 packages/core/src/services/agentTankTypes.ts create mode 100644 packages/shared/src/agentTank.ts create mode 100644 propr-ui/e2e/agent-tank-modes.pw.ts create mode 100644 propr-ui/src/pages/SettingsPage/AgentTankSection.test.tsx create mode 100644 test/agentTankBundledRunner.test.ts create mode 100644 test/agentTankSettingsMigration.test.ts diff --git a/Dockerfile.agent b/Dockerfile.agent index 113a59899..dc0992988 100644 --- a/Dockerfile.agent +++ b/Dockerfile.agent @@ -154,6 +154,21 @@ RUN case "${VIBE_CLI_VERSION}" in \ && if [ ! -e /usr/local/bin/vibe-acp ]; then ln -s /usr/local/bin/vibe /usr/local/bin/vibe-acp; fi +FROM agent-base AS agent-tank-cli +# Pinned here, not in CI: this literal is part of the Dockerfile content that +# feeds the agent bundle content hash, so bumping it produces a new image tag. +# An env-only override in CI would NOT change the tag and would silently ship a +# different binary under an existing tag - do not do that. +ARG AGENT_TANK_CLI_VERSION=0.9.10 +USER root +# node-pty compiles against the toolchain already present in agent-base +# (build-essential + python3), which is why this needs no extra apt packages. +RUN npm install -g "agent-tank@${AGENT_TANK_CLI_VERSION}" \ + && npm cache clean --force \ + && rm -rf /root/.npm \ + && agent-tank --version + + FROM agent-base AS antigravity-cli ARG ANTIGRAVITY_CLI_VERSION=1.2.4 ARG ANTIGRAVITY_CLI_RELEASE_ID=6085322963025920 @@ -196,18 +211,21 @@ ARG CODEX_CLI_VERSION=0.154.0 ARG ANTIGRAVITY_CLI_VERSION=1.2.4 ARG OPENCODE_CLI_VERSION=1.18.31 ARG VIBE_CLI_VERSION=2.25.4 +ARG AGENT_TANK_CLI_VERSION=0.9.10 LABEL dev.propr.agent-bundle="true" \ dev.propr.agent.claude.version="${CLAUDE_CLI_VERSION}" \ dev.propr.agent.codex.version="${CODEX_CLI_VERSION}" \ dev.propr.agent.antigravity.version="${ANTIGRAVITY_CLI_VERSION}" \ dev.propr.agent.opencode.version="${OPENCODE_CLI_VERSION}" \ - dev.propr.agent.vibe.version="${VIBE_CLI_VERSION}" + dev.propr.agent.vibe.version="${VIBE_CLI_VERSION}" \ + dev.propr.agent-tank.version="${AGENT_TANK_CLI_VERSION}" USER root COPY --from=claude-cli /usr/local/lib/node_modules/@anthropic-ai /usr/local/lib/node_modules/@anthropic-ai COPY --from=codex-cli /usr/local/lib/node_modules/@openai /usr/local/lib/node_modules/@openai COPY --from=opencode-cli /usr/local/lib/node_modules/opencode-ai /usr/local/lib/node_modules/opencode-ai +COPY --from=agent-tank-cli /usr/local/lib/node_modules/agent-tank /usr/local/lib/node_modules/agent-tank COPY --from=vibe-cli /usr/local/bin/uv /usr/local/bin/uv COPY --from=vibe-cli /usr/local/bin/uvx /usr/local/bin/uvx COPY --from=vibe-cli /usr/local/bin/vibe /usr/local/bin/vibe @@ -255,7 +273,8 @@ RUN set -eu; \ && codex --version \ && opencode --version \ && vibe --version \ - && agy --version + && agy --version \ + && agent-tank --version USER node RUN git config --global user.name "ProPR Agent Bot" \ diff --git a/dockerhub/agent.md b/dockerhub/agent.md index a095cda24..2ff6cccf4 100644 --- a/dockerhub/agent.md +++ b/dockerhub/agent.md @@ -12,6 +12,11 @@ agent's credentials and task worktree. Version-specific bundle tags contain a complete CLI version matrix, so every agent instance can switch to the same image without another pull. +The image also bundles the [Agent Tank](https://github.com/integry/agent-tank) +CLI (`agent-tank`). ProPR's optional bundled usage-tracking mode runs it here +on demand, so operators do not have to install it — or a second copy of the +agent CLIs — on the host. It is inert unless that mode is enabled. + The common Debian runtime is an internal Dockerfile stage, not a separately published image. Custom installation-level packages create one derivative of the selected bundle. The base includes `build-essential` for native extension diff --git a/docs/docs/features/observability.md b/docs/docs/features/observability.md index 5a54d9dbf..406a4ac3a 100644 --- a/docs/docs/features/observability.md +++ b/docs/docs/features/observability.md @@ -29,7 +29,7 @@ The task detail view exposes progress during execution, including streamed outpu ## Provider Capacity -With the optional [Agent Tank](../operations/agent-tank.md) integration enabled, provider capacity becomes a visible signal too: the sidebar shows live usage bars per subscription provider, and each LLM log entry records the usage delta its call consumed. Turn it on from the dashboard banner ProPR shows when it detects a running instance, from **Settings → LLM Usage Tracking**, or with `propr tank on`. +With the optional [Agent Tank](../operations/agent-tank.md) integration enabled, provider capacity becomes a visible signal too: the sidebar shows live usage bars per subscription provider, and each LLM log entry records the usage delta its call consumed. Turn it on from the dashboard banner, from **Settings → LLM Usage Tracking**, or with `propr tank bundled` — bundled mode runs Agent Tank inside the ProPR agent image, so there is nothing to install. ## Recovery diff --git a/docs/docs/features/propr-cli.md b/docs/docs/features/propr-cli.md index 741e87d00..efbf0be5e 100644 --- a/docs/docs/features/propr-cli.md +++ b/docs/docs/features/propr-cli.md @@ -56,7 +56,7 @@ The full-screen wizard requires an interactive terminal. Over SSH or in shells w - `propr init stack [--root ]` creates `data/`, `logs/`, `repos/`, writes `.env` from the bundled template, and auto-detects agent credential directories on the host (`~/.claude`, `~/.codex`, `~/.gemini`, `~/.config/opencode`, `~/.vibe`). - `propr check` reports the detected [GitHub auth mode](../operations/github-auth.md) (own App, relay, or demo) and flags missing or placeholder configuration before anything starts. `--verify` additionally runs an image/CLI smoke test per agent. - `propr start --no-tui` starts without the interactive dashboard (for scripts/CI); `--no-pull` skips image pulls; `--restart` recreates running services. -- `propr tank [on|off] [--url ]` toggles [Agent Tank](../operations/agent-tank.md) LLM usage tracking on a running stack (omit the state to print the current setting). +- `propr tank [bundled|external|off] [--url ]` configures [Agent Tank](../operations/agent-tank.md) LLM usage tracking on a running stack (omit the mode to print the current one). `bundled` runs Agent Tank inside the agent image with nothing to install; `external` needs `--url` pointing at a daemon you run. `on` remains a deprecated alias for `external`. ### Agent Skill diff --git a/docs/docs/operations/agent-tank.md b/docs/docs/operations/agent-tank.md index b9cda9126..3f1fb2979 100644 --- a/docs/docs/operations/agent-tank.md +++ b/docs/docs/operations/agent-tank.md @@ -1,10 +1,10 @@ # Agent Tank Usage Tracking -[Agent Tank](https://agenttank.io) is a separate, optional local tool that monitors the usage limits of your AI coding agent CLIs. ProPR integrates with it to show live provider capacity in the Web UI and to record per-call usage deltas alongside every LLM log entry. +[Agent Tank](https://agenttank.io) monitors the usage limits of your AI coding agent CLIs. ProPR integrates with it to show live provider capacity in the Web UI and to record per-call usage deltas alongside every LLM log entry. -Agent Tank is open source ([github.com/integry/agent-tank](https://github.com/integry/agent-tank)) and runs entirely on your own machine. It is **not** part of the ProPR stack — you install and run it yourself, then point ProPR at it. If you never enable it, ProPR works exactly the same; you just don't get the capacity bars. +Agent Tank is open source ([github.com/integry/agent-tank](https://github.com/integry/agent-tank)) and runs entirely on your own machine — nothing is sent anywhere. The integration is **off by default**; if you never turn it on, ProPR works exactly the same, you just don't get the capacity bars. -This page covers what Agent Tank is, how to run it, how to connect ProPR to it (including the Docker networking that makes the connection work), and what you see once it is enabled. +This page covers what Agent Tank tracks, the three integration modes, and what you see once it is enabled. ## What Agent Tank Tracks @@ -18,9 +18,11 @@ It is **not** an API-spend tracker. For pay-as-you-go API key billing or per-req | Codex (`codex`) | JSON-RPC `account/rateLimits/read`, falling back to `/status` | 5-hour session limit and weekly limit | | Antigravity (`agy`) | Runs the CLI's `/usage` command | Per-model quota availability and reset windows | +OpenCode and Vibe are not tracked: neither CLI exposes a subscription usage endpoint for Agent Tank to read. + ### How It Gets The Data -Agent Tank reads usage directly from the CLI tools you already have installed. It launches each CLI locally in a pseudo-terminal, runs the tool's built-in usage command, and parses the output into a unified dashboard and JSON API. Nothing leaves your machine. Specifically, it does **not**: +Agent Tank reads usage directly from the CLI tools you already have installed. It launches each CLI in a pseudo-terminal, runs the tool's built-in usage command, and parses the output. Nothing leaves your machine. Specifically, it does **not**: - scrape provider websites - read browser cookies or depend on a logged-in browser session @@ -30,18 +32,49 @@ Agent Tank reads usage directly from the CLI tools you already have installed. I This matters for ProPR: the usage numbers in the sidebar come from the same `/usage` output you would see if you ran the CLI yourself; no estimation is involved. -## Run Agent Tank +## The Three Integration Modes + +The integration is a single setting with three states. Choose it in **Settings → LLM Usage Tracking**, with `propr tank`, or via the `AGENT_TANK_MODE` environment variable. + +| Mode | What it does | When to use it | +|---|---|---| +| `disabled` | **Default.** Nothing is contacted or started. No usage tracking at all. | You don't want capacity bars. | +| `bundled` | ProPR runs the Agent Tank CLI **inside the `propr/agent` image** on demand, against your configured agent credentials. | Almost everyone. No host install, no daemon, no networking. | +| `external` | ProPR talks HTTP to an Agent Tank daemon **you** run yourself. | You already run Agent Tank, want its web dashboard, or want to track credentials ProPR doesn't manage. | + +Upgrades are transparent: an installation that had the integration enabled before bundled mode existed loads as `external` with exactly the URL it had, and a disabled one stays disabled. + +### Bundled Mode (Recommended) + +The `propr/agent` image already contains `claude`, `codex`, and `agy`, and ProPR already knows where each configured agent's credentials live. Bundled mode uses both: for each refresh it starts a short-lived container from the same agent image your tasks run in, mounts every enabled agent's credential directory **read-only** at the path that agent's runtime uses, and runs `agent-tank --once --json`. + +That means: + +- **Nothing to install.** No `npm install -g agent-tank`, no daemon to keep alive, no second copy of the agent CLIs. +- **No networking.** There is no HTTP endpoint and therefore no `localhost` vs `host.docker.internal` mistake to make. +- **Same credentials as your runs.** Bundled Agent Tank inspects exactly the directories the agents themselves use, so the numbers describe the accounts doing the work. The mounts are read-only, so a usage probe can never modify or corrupt them. +- **A cached snapshot, not a live daemon.** Starting a container and driving `/usage` through a pseudo-terminal takes time, so ProPR caches the result and refreshes out of band. The per-LLM-call probes only ever read that cache; the sidebar's refresh button forces a fresh run. -Install and start it on the host that runs your agent CLIs (usually the same host as the ProPR stack): +Enable it with: + +```bash +propr tank bundled +``` + +Bundled mode reports the providers it can see. An agent with no credentials mounted, or a provider Agent Tank does not support, is simply left out. + +### External Mode + +Use this when you run Agent Tank yourself. Install and start it on the host that runs your agent CLIs: ```bash npm install -g agent-tank # or run it directly with: npx agent-tank agent-tank # auto-discovers installed CLIs, serves dashboard + API ``` -By default it serves the dashboard and HTTP API at `http://127.0.0.1:3456` and, when Docker is available, also binds the Docker bridge gateway addresses so containers on the same host can reach it (see [Networking](#networking-propr-to-agent-tank) below). Building it compiles the native `node-pty` module, so the host needs Node.js 18+, Python 3.8+, and C/C++ build tools — see the [Agent Tank README](https://github.com/integry/agent-tank#installation-notes) if the build fails. +By default it serves the dashboard and HTTP API at `http://127.0.0.1:3456` and, when Docker is available, also binds the Docker bridge gateway addresses so containers on the same host can reach it (see [Networking](#networking-propr-to-an-external-agent-tank) below). Building it compiles the native `node-pty` module, so the host needs Node.js 18+, Python 3.8+, and C/C++ build tools — see the [Agent Tank README](https://github.com/integry/agent-tank#installation-notes) if the build fails. -You need at least one supported CLI installed, authenticated, and on the `PATH`. For Claude, `/usage` requires Claude Code 2.0+. +You need at least one supported CLI installed, authenticated, and on the `PATH` **of the host running Agent Tank**. In a normal ProPR install those CLIs live inside `propr/agent` rather than on the host, which is the friction bundled mode removes. Common flags: @@ -53,28 +86,13 @@ agent-tank --no-docker # bind localhost only (skip Docker bridge bindin agent-tank --claude-api # use the Anthropic OAuth usage API for Claude (faster refresh) ``` -To keep Agent Tank running alongside the ProPR stack, start it with `--background` (or run it under your own process manager). See the [Agent Tank README](https://github.com/integry/agent-tank) for the full option, environment-variable, and config-file reference. - -## Connect ProPR To Agent Tank - -There are three ways to turn the integration on. All three write the same backend setting (`enabled` plus a service `url`). - -**Detection banner (easiest).** When ProPR detects a running Agent Tank instance at the Docker-internal default (`http://host.docker.internal:3456`) and the integration is off, the dashboard and LLM Log page show a dismissible banner offering to enable it in one click. - -**Settings → LLM Usage Tracking.** Toggle *Enable Agent Tank Integration* and set the service URL. The section shows a live connectivity indicator (green "connected" / red with the error) so you can confirm ProPR can reach the service before relying on it. - -**CLI (`propr tank`).** Toggle it on a running stack from the terminal: +Then point ProPR at it: ```bash -propr tank # show the current setting (on/off + URL) -propr tank on # enable using the saved/default URL -propr tank on --url http://127.0.0.1:3456 # enable with a specific URL -propr tank off # disable +propr tank external --url http://host.docker.internal:3456 ``` -Because Agent Tank is an external service rather than a stack container, `propr tank` talks to the running ProPR backend — start the stack first (`propr start`). - -### Networking: ProPR To Agent Tank +#### Networking: ProPR To An External Agent Tank ProPR's shipped default URL is `http://0.0.0.0:3456`; the `propr tank` CLI client defaults to `http://127.0.0.1:3456`. The default only reaches Agent Tank when the ProPR backend runs directly on the host (a source checkout running `npm run daemon`/`npm run worker`). In the standard install the backend runs in Docker, where `0.0.0.0` and `localhost` resolve to the container itself — set the URL to `http://host.docker.internal:3456` there, which is exactly what the detection banner offers to do for you. @@ -85,9 +103,35 @@ Change the URL in two situations: Agent Tank's own bind addresses support the container case: by default it listens on `127.0.0.1` plus, when Docker is available, the **private** Docker bridge gateway addresses, so same-host containers can reach it without it being exposed on a public interface. `--no-docker` restricts it to localhost, which Docker containers cannot reach. -Two environment variables tune the backend integration: +This whole section is why bundled mode exists — none of it applies there. + +## Choosing The Mode + +There are three ways to set it. All write the same backend setting. + +**Detection banner (easiest).** While tracking is off, the dashboard and LLM Log page show a dismissible banner offering to turn it on in one click. If a daemon is already answering at `http://host.docker.internal:3456` the banner offers `external` pointed at it; otherwise it offers `bundled`. + +**Settings → LLM Usage Tracking.** Pick one of the three modes. The **Daemon URL** field only appears for `external`, because an external URL means nothing in the other two. The section shows a live status indicator so you can confirm the mode works before relying on it. -- `AGENT_TANK_URL` — fallback service URL used when no URL is saved in settings. +**CLI (`propr tank`).** Configure it on a running stack from the terminal: + +```bash +propr tank # show the current mode (plus URL, for external) +propr tank bundled # run Agent Tank inside the agent image +propr tank external --url http://127.0.0.1:3456 # use your own daemon +propr tank off # disable +``` + +`propr tank on` still works as a deprecated alias for `propr tank external` — that is what it has always meant — and prints a note saying so. + +Because this is a backend setting rather than a stack container, `propr tank` talks to the running ProPR backend — start the stack first (`propr start`). + +## Environment Variables + +- `AGENT_TANK_MODE` — `disabled`, `bundled`, or `external`. Only used when **no** setting has been saved yet, so headless deployments can configure the stack entirely from `.env`. A saved setting always wins. +- `AGENT_TANK_URL` — fallback service URL when none is saved. Applies to `external` mode only. +- `AGENT_TANK_BUNDLED_TIMEOUT_MS` — how long a bundled refresh container may run before it is abandoned (default `120000`). +- `AGENT_TANK_BUNDLED_CACHE_TTL_MS` — how long a bundled snapshot stays fresh before the next refresh starts a container (default `60000`). - `ANALYSIS_AGENT_TANK_TIMEOUT_MS` — per-request timeout for the pre/post-call usage probes (kept short so tracking never slows a task). ## What You See Once Enabled @@ -96,22 +140,25 @@ Two environment variables tune the backend integration: - **Per-call usage deltas.** Around each agent run ProPR snapshots usage before and after the call, computes the delta per metric, and stores it next to the [LLM Log](./metrics.md) entry. The task detail context strip shows a compact session/weekly delta chip for the run. - **Capacity in your metrics.** Provider capacity pressure becomes a first-class signal alongside cost and cycle time — see [Metrics](./metrics.md). +In bundled mode the deltas come from cached snapshots, so a call that finishes between two refreshes records no delta rather than a guessed one. + {/* SCREENSHOT PLACEHOLDER (P3 — needs a running Agent Tank instance; interim: the site's ui-agent-tank.png): Capture the sidebar Usage section with Agent Tank enabled, showing provider rows (for example Claude and Codex) with colored usage bars and percentages, and one provider expanded to show its session and weekly metrics. Requires a running Agent Tank instance configured in Settings. */} ## Best-Effort By Design -The integration never blocks a task. If Agent Tank is disabled, unreachable, or slow: +The integration never blocks a task. If Agent Tank is disabled, unreachable, slow, or — in bundled mode — the agent image is missing or the container fails: - the pre/post-call usage probes are skipped or time out quietly, - the LLM call runs and completes normally with no usage delta recorded, and - the sidebar Usage section hides itself. -So a missing or stopped Agent Tank instance degrades to "no capacity bars," and the work itself completes normally. +So a missing Agent Tank degrades to "no capacity bars," and the work itself completes normally. ## Troubleshooting -- **Sidebar is empty / "not connected" in Settings.** Confirm Agent Tank is running (`http://127.0.0.1:3456` in a browser) and that the URL ProPR uses is reachable *from inside the container* — typically `http://host.docker.internal:3456`, since `localhost` there resolves to the container itself. Avoid `--no-docker` when ProPR runs in Docker. -- **No agents found by Agent Tank.** At least one supported CLI (`claude`, `agy`, or `codex`) must be installed, authenticated, and on the `PATH` of the host running Agent Tank. Check with `claude --version` etc. -- **`Timeout waiting for usage data`.** Make sure the CLI works and is authenticated on its own (no pending trust/auth/update prompts). For Claude, try `--claude-api`. +- **Sidebar is empty in bundled mode.** Confirm the agent image is built and at least one enabled agent is authenticated. `docker run --rm propr/agent:latest agent-tank --version` proves the image ships the CLI; a task that runs successfully proves the credentials are mounted. +- **Sidebar is empty / "unreachable" in external mode.** Confirm Agent Tank is running (`http://127.0.0.1:3456` in a browser) and that the URL ProPR uses is reachable *from inside the container* — typically `http://host.docker.internal:3456`, since `localhost` there resolves to the container itself. Avoid `--no-docker` when ProPR runs in Docker. Bundled mode sidesteps all of this. +- **No agents found by Agent Tank.** At least one supported CLI (`claude`, `agy`, or `codex`) must be installed and authenticated. In bundled mode that means an enabled ProPR agent of that type with a readable credential directory; in external mode, a CLI on the `PATH` of the host running Agent Tank. +- **`Timeout waiting for usage data`.** Make sure the CLI works and is authenticated on its own (no pending trust/auth/update prompts). For Claude, try `--claude-api` in external mode. For deeper operational context, see [Metrics](./metrics.md). diff --git a/docs/docs/operations/configuration-reference.md b/docs/docs/operations/configuration-reference.md index f55238809..41e08dcb2 100644 --- a/docs/docs/operations/configuration-reference.md +++ b/docs/docs/operations/configuration-reference.md @@ -140,11 +140,14 @@ Optional: expose a local stack's API to the hosted control plane at `https://app ## Agent Tank & Metrics -These two variables are read from code but are not in `.env.example` — Agent Tank is normally connected through the Web UI or `propr agent-tank`, which save the URL as a backend setting. See [Agent Tank](./agent-tank.md). +These variables are read from code but are not in `.env.example` — Agent Tank is normally configured through the Web UI or `propr tank`, which save the mode as a backend setting. The integration has three modes (`disabled`, `bundled`, `external`); see [Agent Tank](./agent-tank.md). | Variable | Default (shipped / code) | What it does | Required when | |---|---|---|---| -| `AGENT_TANK_URL` | Code falls back to `http://0.0.0.0:3456` when no saved setting exists | Fallback Agent Tank service URL used when no URL is saved in settings. Empty or `false` disables usage tracking for LLM calls. | Only when configuring Agent Tank via env instead of the UI/CLI. | +| `AGENT_TANK_MODE` | `disabled` | Integration mode used when **no** Agent Tank setting has been saved yet: `disabled`, `bundled` (run the CLI inside the agent image), or `external` (talk HTTP to your own daemon). A saved setting always wins, and an unrecognized value is treated as `disabled`. | Only when configuring Agent Tank via env instead of the UI/CLI. | +| `AGENT_TANK_URL` | Code falls back to `http://0.0.0.0:3456` when no saved setting exists | Fallback Agent Tank service URL used when no URL is saved in settings. **Applies to `external` mode only** — bundled mode contacts no URL. Empty or `false` disables usage tracking for LLM calls in external mode. | Only when configuring external Agent Tank via env instead of the UI/CLI. | +| `AGENT_TANK_BUNDLED_TIMEOUT_MS` | `120000` | How long a bundled-mode refresh container may run before it is abandoned. A timeout degrades to "no usage data"; it never fails a task. | Optional, `bundled` mode only. | +| `AGENT_TANK_BUNDLED_CACHE_TTL_MS` | `60000` | How long a bundled-mode usage snapshot stays fresh before the next refresh starts a container. Per-LLM-call probes only read this cache. | Optional, `bundled` mode only. | | `ANALYSIS_AGENT_TANK_TIMEOUT_MS` | `2000` | Timeout for the Agent Tank status fetch wrapped around each LLM call. | Optional. | ## Advanced diff --git a/docs/mcp-coverage.md b/docs/mcp-coverage.md index 2363da6c8..776adfcf7 100644 --- a/docs/mcp-coverage.md +++ b/docs/mcp-coverage.md @@ -54,7 +54,7 @@ remain separate gates. | Direct agent configuration | `get_agent_configuration`, `create_agent_configuration`, `update_agent_configuration`, `remove_agent_configuration`; actual types/models, alias, enablement, model labels/reasoning, CLI versions; new agents start disabled for secure login | | Synthetic-agent composition | `create_synthetic_agent`, `update_synthetic_agent`, `remove_synthetic_agent`; pool models/members, strategy, priority and usage thresholds; existing reference/default guards | | Advanced indexing policy | `get_indexing_configuration`, `update_indexing_configuration`; primary/fallback alias:model, prompt, enablement and runtime cooldown state | -| Provider policy | `get_provider_policy`, `update_provider_policy`, `get_provider_status`, `get_provider_usage`, `refresh_provider_usage`, `detect_provider_service`; Agent Tank service origin and enablement, no credential entry | +| Provider policy | `get_provider_policy`, `update_provider_policy`, `get_provider_status`, `get_provider_usage`, `refresh_provider_usage`, `detect_provider_service`; Agent Tank integration mode (`disabled`/`bundled`/`external`) and, for external, the service origin; no credential entry | | Execution/review/context | `get_execution_settings`, `update_execution_settings`; worker concurrency, analysis/planner models, review model/prompt/context enablement/model/budget, reasoning and bounded ultrafix defaults | | Workflow labels and keywords | `get_`/`update_` tools for `followup_keywords`, `followup_ignore_keywords`, `primary_processing_labels`, `pr_label`, `ai_primary_tag` | | Runtime package configuration/build | `get_runtime_configuration`, `update_runtime_configuration` | diff --git a/packages/api/mcp/toolsConfiguration.ts b/packages/api/mcp/toolsConfiguration.ts index 8195adf63..837720695 100644 --- a/packages/api/mcp/toolsConfiguration.ts +++ b/packages/api/mcp/toolsConfiguration.ts @@ -1,7 +1,7 @@ import { randomUUID } from 'node:crypto'; import { z } from 'zod'; import { loadAgents, loadSyntheticAgents, loadMonitoredReposRaw, AGENT_DEFAULTS, AGENT_TYPES } from '@propr/core'; -import { getManagedAgentConfigPath, isAgentLoginSupported, syntheticAgentConfigSchema, REASONING_LEVELS } from '@propr/shared'; +import { AGENT_TANK_MODES, getManagedAgentConfigPath, isAgentLoginSupported, syntheticAgentConfigSchema, REASONING_LEVELS } from '@propr/shared'; import type { createConfigRoutes } from '../routes/configRoutes.js'; import { configRevision } from '../routes/configRevision.js'; import { callWorkflow } from './adapter.js'; @@ -56,7 +56,23 @@ export function addConfigurationTools(tools: McpTool[], deps: ToolDeps, config: workflow(tools, { name: 'get_indexing_configuration', description: 'Read indexing model/fallback policy, prompt, cooldowns and degradation state.', scope: 'manage', permission: 'instance.manage_settings', readOnly: true, schema: z.object({}).strict() }, config.getSummarizationSettings, () => ({})); workflow(tools, { name: 'update_indexing_configuration', description: 'Replace indexing configuration with explicit primary/fallback alias:model and prompt. Existing validation and delayed reindex behavior apply.', scope: 'manage', permission: 'instance.manage_settings', schema: z.object({ ...mutationShape, enabled: z.boolean(), agent_alias: z.string().max(256), fallback_agent_alias: z.string().max(256), custom_prompt: z.string().max(65536) }).strict() }, config.postSummarizationSettings, args => ({ body: args })); workflow(tools, { name: 'get_provider_policy', description: 'Read the configured Agent Tank provider policy; does not return credentials.', scope: 'manage', permission: 'instance.manage_agents', readOnly: true, schema: z.object({}).strict() }, config.getAgentTankSettings, () => ({})); - workflow(tools, { name: 'update_provider_policy', description: 'Configure the existing Agent Tank provider service with a non-secret HTTP(S) base URL and explicit enabled state. Requires instance.manage_agents.', scope: 'manage', permission: 'instance.manage_agents', schema: z.object({ ...mutationShape, enabled: z.boolean(), url: z.url().max(2048).refine(value => { const url = new URL(value); return ['http:', 'https:'].includes(url.protocol) && !url.username && !url.password && !url.search && !url.hash && url.pathname === '/'; }, 'Use an HTTP(S) origin without credentials, path, query or fragment') }).strict() }, config.postAgentTankSettings, args => ({ body: { enabled: args.enabled, url: args.url.replace(/\/$/, '') } })); + workflow(tools, { + name: 'update_provider_policy', + description: 'Configure Agent Tank usage tracking. "bundled" runs it inside the ProPR agent image and needs no url; "external" targets an operator-run instance at a non-secret HTTP(S) base URL; "disabled" turns it off. Requires instance.manage_agents.', + scope: 'manage', + permission: 'instance.manage_agents', + schema: z.object({ + ...mutationShape, + mode: z.enum(AGENT_TANK_MODES), + // Optional because it is meaningless outside external mode; the refine + // below makes it required exactly when it matters, so the tool cannot be + // called into an inconsistent state. + url: z.url().max(2048).refine(value => { const url = new URL(value); return ['http:', 'https:'].includes(url.protocol) && !url.username && !url.password && !url.search && !url.hash && url.pathname === '/'; }, 'Use an HTTP(S) origin without credentials, path, query or fragment').optional(), + }).strict().refine( + args => args.mode !== 'external' || typeof args.url === 'string', + { message: 'url is required when mode is "external"' }, + ), + }, config.postAgentTankSettings, args => ({ body: { mode: args.mode, url: args.url?.replace(/\/$/, '') } })); for (const [name, handler] of [['get_provider_status', config.getAgentTankStatus], ['get_provider_usage', config.getAgentTankUsage], ['detect_provider_service', config.getAgentTankDetect]] as const) workflow(tools, { name, description: 'Read the existing configured Agent Tank provider service state.', scope: 'manage', permission: 'instance.manage_agents', readOnly: true, schema: z.object({}).strict() }, handler, () => ({})); workflow(tools, { name: 'refresh_provider_usage', description: 'Refresh usage from the existing configured Agent Tank service.', scope: 'manage', permission: 'instance.manage_agents', schema: z.object(mutationShape).strict() }, config.postAgentTankRefresh, () => ({})); for (const action of ['create', 'remove'] as const) tools.push({ name: `${action}_repository_configuration`, description: `${action} a repository configuration under instance administration and explicit repository grants. Missing consent returns browser continuation without changing configuration.`, scope: 'manage', permission: 'instance.manage_settings', schema: z.object({ ...mutationShape, repository: repositorySchema, ...(action === 'create' ? { baseBranch: idSchema, enabled: z.boolean().default(true), alias: idSchema.optional() } : {}) }).strict(), run: async ({ principal, args }) => { diff --git a/packages/api/routes/configRoutesAgentTank.ts b/packages/api/routes/configRoutesAgentTank.ts index 05e1093fc..2cc15af6e 100644 --- a/packages/api/routes/configRoutesAgentTank.ts +++ b/packages/api/routes/configRoutesAgentTank.ts @@ -1,6 +1,7 @@ import { Request, Response } from 'express'; import * as configManager from '@propr/core'; -import { normalizeAgentTankAgents, type AgentStatusResponse } from '@propr/core'; +import { canRunBundledAgentTank, getAgentTankStatuses, refreshBundledStatuses } from '@propr/core'; +import { AGENT_TANK_MODES, isAgentTankMode, normalizeAgentTankMode } from '@propr/shared'; export function createAgentTankRoutes() { async function getAgentTankSettings(_req: Request, res: Response): Promise { @@ -15,8 +16,26 @@ export function createAgentTankRoutes() { async function postAgentTankSettings(req: Request, res: Response): Promise { try { - const { enabled, url } = req.body; - await configManager.saveAgentTankSettings({ enabled: !!enabled, url: url || 'http://0.0.0.0:3456' }); + const { mode, enabled, url } = req.body ?? {}; + // Accept the legacy `{ enabled }` body so older CLI builds and any + // in-flight clients keep working during a rolling upgrade. + if (mode !== undefined && !isAgentTankMode(mode)) { + res.status(400).json({ error: `mode must be one of: ${AGENT_TANK_MODES.join(', ')}` }); + return; + } + const resolvedMode = mode === undefined + ? (enabled === true ? 'external' : 'disabled') + : normalizeAgentTankMode(mode); + if (resolvedMode === 'external' && typeof url === 'string' && url.trim() === '') { + res.status(400).json({ error: 'url is required when mode is "external"' }); + return; + } + // Keep a hand-tuned external URL when the caller omits one (bundled mode + // has no URL to send), so switching modes back and forth is lossless. + const resolvedUrl = typeof url === 'string' && url.trim() + ? url.trim() + : (await configManager.loadAgentTankSettings()).url; + await configManager.saveAgentTankSettings({ mode: resolvedMode, url: resolvedUrl }); res.json({ success: true }); } catch (error) { console.error('Error in /api/config/agent-tank POST:', error); @@ -27,23 +46,34 @@ export function createAgentTankRoutes() { async function getAgentTankStatus(_req: Request, res: Response): Promise { try { const settings = await configManager.loadAgentTankSettings(); - if (!settings.enabled) { + if (settings.mode === 'disabled') { res.json({ available: false, reason: 'disabled' }); return; } + if (settings.mode === 'bundled') { + // "Available" for bundled mode means "we can produce a snapshot", + // which is exactly what a (cached) refresh answers. Reusing the same + // call keeps the status indicator honest instead of asserting health + // from image presence alone. + const agents = await refreshBundledStatuses(); + res.json(agents + ? { available: true, mode: 'bundled' } + : { available: false, mode: 'bundled', reason: 'bundled_run_failed' }); + return; + } const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), 3000); try { const response = await fetch(`${settings.url}/status/claude`, { signal: controller.signal }); clearTimeout(timer); if (response.ok) { - res.json({ available: true }); + res.json({ available: true, mode: 'external' }); } else { - res.json({ available: false, reason: `HTTP ${response.status}` }); + res.json({ available: false, mode: 'external', reason: `HTTP ${response.status}` }); } } catch { clearTimeout(timer); - res.json({ available: false, reason: 'unreachable' }); + res.json({ available: false, mode: 'external', reason: 'unreachable' }); } } catch (error) { console.error('Error in /api/config/agent-tank/status GET:', error); @@ -54,25 +84,20 @@ export function createAgentTankRoutes() { async function getAgentTankUsage(_req: Request, res: Response): Promise { try { const settings = await configManager.loadAgentTankSettings(); - if (!settings.enabled) { + if (settings.mode === 'disabled') { res.json({ enabled: false }); return; } - const controller = new AbortController(); - const timer = setTimeout(() => controller.abort(), 5000); - try { - const response = await fetch(`${settings.url}/status`, { signal: controller.signal }); - clearTimeout(timer); - if (response.ok) { - const data = await response.json() as Record; - res.json({ enabled: true, agents: normalizeAgentTankAgents(data) }); - } else { - res.json({ enabled: true, error: `HTTP ${response.status}` }); - } - } catch { - clearTimeout(timer); - res.json({ enabled: true, error: 'unreachable' }); - } + // One transport-agnostic call: the UI response shape is unchanged, so + // AgentTankSidebar needs no modification for bundled mode. + const agents = await getAgentTankStatuses(); + res.json(agents + ? { enabled: true, mode: settings.mode, agents } + : { + enabled: true, + mode: settings.mode, + error: settings.mode === 'bundled' ? 'bundled_run_failed' : 'unreachable' + }); } catch (error) { console.error('Error in /api/config/agent-tank/usage GET:', error); res.status(500).json({ error: 'Failed to fetch Agent Tank usage' }); @@ -82,10 +107,17 @@ export function createAgentTankRoutes() { async function postAgentTankRefresh(_req: Request, res: Response): Promise { try { const settings = await configManager.loadAgentTankSettings(); - if (!settings.enabled) { + if (settings.mode === 'disabled') { res.json({ success: false, error: 'Agent Tank not enabled' }); return; } + if (settings.mode === 'bundled') { + // `force` because this is an explicit operator action: they pressed + // refresh precisely because they do not trust the cached snapshot. + const agents = await refreshBundledStatuses({ force: true }); + res.json(agents ? { success: true } : { success: false, error: 'bundled_run_failed' }); + return; + } const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), 10000); try { @@ -113,12 +145,14 @@ export function createAgentTankRoutes() { const DEFAULT_URL = 'http://host.docker.internal:3456'; try { const settings = await configManager.loadAgentTankSettings(); - // If already enabled, no need to detect - if (settings.enabled) { + // Only offer the banner when tracking is entirely off. + if (settings.mode !== 'disabled') { res.json({ detected: false, reason: 'already_enabled' }); return; } - // Try to detect Agent Tank at default URL + // An external instance, if one happens to be running, wins the offer so + // we point the operator at what they already set up. Otherwise bundled is + // suggested: it is the lower-friction option and needs nothing installed. const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), 2000); try { @@ -128,14 +162,18 @@ export function createAgentTankRoutes() { const data = await response.json(); // Check if we got valid agent data const hasAgents = data && typeof data === 'object' && Object.keys(data).length > 0; - res.json({ detected: hasAgents, url: DEFAULT_URL }); - } else { - res.json({ detected: false }); + if (hasAgents) { + res.json({ detected: true, mode: 'external', url: DEFAULT_URL }); + return; + } } } catch { clearTimeout(timer); - res.json({ detected: false }); } + // Only offer bundled when it would actually report something: a fresh + // install with no authenticated agent would just get an empty sidebar. + const bundledUsable = await canRunBundledAgentTank(); + res.json(bundledUsable ? { detected: true, mode: 'bundled' } : { detected: false }); } catch (error) { console.error('Error in /api/config/agent-tank/detect GET:', error); res.json({ detected: false }); diff --git a/packages/api/test/configRoutesAgentTank.test.ts b/packages/api/test/configRoutesAgentTank.test.ts new file mode 100644 index 000000000..8b20bbdda --- /dev/null +++ b/packages/api/test/configRoutesAgentTank.test.ts @@ -0,0 +1,105 @@ +/** + * Agent Tank config routes. + * + * These cover the two ways the mode can arrive at the backend: the current + * `{ mode }` body the UI/CLI/MCP send, and the legacy `{ enabled }` body an + * older client may still send during a rolling upgrade. + */ + +import { after, test } from 'node:test'; +import assert from 'node:assert/strict'; + +process.env.NODE_ENV = 'test'; + +// One shared connection for the file: closing it per test would leave the next +// test unable to reacquire one. +const configManager = await import('@propr/core'); +const { createAgentTankRoutes } = await import('../routes/configRoutesAgentTank.js'); + +after(async () => { + await configManager.db('system_configs').whereIn('key', ['agent_tank']).delete(); + await configManager.closeConnection(); +}); + +function responseSpy() { + return { + statusCode: 200, + body: undefined as Record | undefined, + status(code: number) { + this.statusCode = code; + return this; + }, + json(payload: Record) { + this.body = payload; + return this; + }, + }; +} + +test('agent tank settings routes persist the mode and reject an unknown one', async () => { + await configManager.runMigrations(); + await configManager.db('system_configs').whereIn('key', ['agent_tank']).delete(); + + const routes = createAgentTankRoutes(); + + // Bundled mode carries no URL; the previously saved one must survive. + await configManager.saveAgentTankSettings({ mode: 'external', url: 'http://saved:3456' }); + let res = responseSpy(); + await routes.postAgentTankSettings({ body: { mode: 'bundled' } } as never, res as never); + assert.equal(res.statusCode, 200); + let saved = await configManager.loadAgentTankSettings(); + assert.equal(saved.mode, 'bundled'); + assert.equal(saved.url, 'http://saved:3456'); + assert.equal(saved.enabled, true); + + // A legacy `{ enabled: true }` body means "my host install", i.e. external. + res = responseSpy(); + await routes.postAgentTankSettings({ body: { enabled: true, url: 'http://legacy:3456' } } as never, res as never); + saved = await configManager.loadAgentTankSettings(); + assert.equal(saved.mode, 'external'); + assert.equal(saved.url, 'http://legacy:3456'); + + res = responseSpy(); + await routes.postAgentTankSettings({ body: { enabled: false } } as never, res as never); + assert.equal((await configManager.loadAgentTankSettings()).mode, 'disabled'); + + res = responseSpy(); + await routes.postAgentTankSettings({ body: { mode: 'sideways' } } as never, res as never); + assert.equal(res.statusCode, 400); + // A rejected write must not change the stored mode. + assert.equal((await configManager.loadAgentTankSettings()).mode, 'disabled'); + + // GET returns the mode alongside the derived boolean older clients read. + res = responseSpy(); + await routes.getAgentTankSettings({} as never, res as never); + assert.equal(res.body?.mode, 'disabled'); + assert.equal(res.body?.enabled, false); +}); + +test('disabled mode short-circuits status, usage and refresh without any transport', async () => { + await configManager.runMigrations(); + await configManager.saveAgentTankSettings({ mode: 'disabled', url: 'http://0.0.0.0:3456' }); + + const routes = createAgentTankRoutes(); + const originalFetch = globalThis.fetch; + let fetches = 0; + globalThis.fetch = (async () => { fetches += 1; return new Response('{}'); }) as typeof fetch; + + try { + const status = responseSpy(); + await routes.getAgentTankStatus({} as never, status as never); + assert.deepEqual(status.body, { available: false, reason: 'disabled' }); + + const usage = responseSpy(); + await routes.getAgentTankUsage({} as never, usage as never); + assert.deepEqual(usage.body, { enabled: false }); + + const refresh = responseSpy(); + await routes.postAgentTankRefresh({} as never, refresh as never); + assert.equal(refresh.body?.success, false); + + assert.equal(fetches, 0); + } finally { + globalThis.fetch = originalFetch; + } +}); diff --git a/packages/api/test/mcpReviewerConcurrency.test.ts b/packages/api/test/mcpReviewerConcurrency.test.ts index 3b9aa1d6d..db4c02140 100644 --- a/packages/api/test/mcpReviewerConcurrency.test.ts +++ b/packages/api/test/mcpReviewerConcurrency.test.ts @@ -78,9 +78,14 @@ test('real MCP catalog and shared persistence reject stale repository and agent for (const [key, value] of Object.entries(settings)) assert.equal(read[key], value); assert.equal((await call('update_indexing_configuration', { enabled: false, agent_alias: '', fallback_agent_alias: '', custom_prompt: 'Bounded summaries' })).state, 'completed'); assert.equal((await call('get_indexing_configuration', {})).custom_prompt, 'Bounded summaries'); - assert.equal((await call('update_provider_policy', { enabled: false, url: 'http://localhost:3456' })).state, 'completed'); + assert.equal((await call('update_provider_policy', { mode: 'disabled', url: 'http://localhost:3456' })).state, 'completed'); + assert.equal((await call('get_provider_policy', {})).mode, 'disabled'); assert.equal((await call('get_provider_policy', {})).enabled, false); - await assert.rejects(call('update_provider_policy', { enabled: true, url: 'https://user:secret@example.com' })); + // Bundled mode is reachable without a url; external still requires one. + assert.equal((await call('update_provider_policy', { mode: 'bundled' })).state, 'completed'); + assert.equal((await call('get_provider_policy', {})).mode, 'bundled'); + await assert.rejects(call('update_provider_policy', { mode: 'external' })); + await assert.rejects(call('update_provider_policy', { mode: 'external', url: 'https://user:secret@example.com' })); principal.authorization.permissions = []; await assert.rejects(call('create_repository_configuration', { repository: 'acme/three', baseBranch: 'main' }), /instance.manage_settings/); } finally { diff --git a/packages/cli/src/api/agentTank.ts b/packages/cli/src/api/agentTank.ts index 9396068d0..90817ae8f 100644 --- a/packages/cli/src/api/agentTank.ts +++ b/packages/cli/src/api/agentTank.ts @@ -1,14 +1,18 @@ /** * Agent Tank API * - * Agent Tank tracks LLM subscription usage. It is an external service (not a - * stack container) — toggling it is a backend setting, so these helpers go - * through the running ProPR API (`/api/config/agent-tank`). + * Agent Tank tracks LLM subscription usage. It is a backend setting rather than + * a stack container — in `bundled` mode ProPR runs the Agent Tank CLI inside the + * agent image, and in `external` mode it talks to an instance the operator runs + * — so these helpers go through the running ProPR API + * (`/api/config/agent-tank`). */ import { ApiClient, createApiClient } from "./index.js"; +import type { AgentTankMode } from "@propr/shared"; export interface AgentTankSettings { + mode: AgentTankMode; enabled: boolean; url?: string; } @@ -22,21 +26,22 @@ export async function getAgentTank(client?: ApiClient): Promise { const apiClient = client ?? (await createApiClient()); - // Preserve the existing URL when the caller doesn't pass one. + // Preserve the existing URL when the caller doesn't pass one, so switching to + // bundled and back to external does not lose a hand-tuned endpoint. let resolvedUrl = url; if (!resolvedUrl) { const current = await getAgentTank(apiClient); resolvedUrl = current.url || DEFAULT_AGENT_TANK_URL; } - await apiClient.post("/api/config/agent-tank", { body: { enabled, url: resolvedUrl } }); - return { enabled, url: resolvedUrl }; + await apiClient.post("/api/config/agent-tank", { body: { mode, url: resolvedUrl } }); + return { mode, enabled: mode !== "disabled", url: resolvedUrl }; } diff --git a/packages/cli/src/commands/tankCommands.test.ts b/packages/cli/src/commands/tankCommands.test.ts new file mode 100644 index 000000000..e4aef024e --- /dev/null +++ b/packages/cli/src/commands/tankCommands.test.ts @@ -0,0 +1,73 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import type { ApiClient } from "../api/index.js"; +import { getAgentTank, setAgentTank } from "../api/agentTank.js"; +import { parseTankMode } from "./tankCommands.js"; + +test("parseTankMode accepts the three modes plus the off/on aliases", () => { + assert.equal(parseTankMode("bundled"), "bundled"); + assert.equal(parseTankMode("external"), "external"); + assert.equal(parseTankMode("disabled"), "disabled"); + assert.equal(parseTankMode(" OFF "), "disabled"); + // `on` has always meant "use my host install", so it maps to external and + // never silently repoints an existing user at a container. + assert.equal(parseTankMode("on"), "external"); + assert.equal(parseTankMode("bundle"), undefined); + assert.equal(parseTankMode(""), undefined); +}); + +function fakeClient( + recorded: Array<{ endpoint: string; body?: unknown }>, + current: Record = { mode: "external", enabled: true, url: "http://saved:3456" }, +): ApiClient { + return { + get: async (endpoint: string) => { + recorded.push({ endpoint }); + return { data: current }; + }, + post: async (endpoint: string, options?: { body?: unknown }) => { + recorded.push({ endpoint, body: options?.body }); + return { data: { success: true } }; + }, + } as unknown as ApiClient; +} + +test("setAgentTank sends the mode and preserves the saved URL when none is given", async () => { + const recorded: Array<{ endpoint: string; body?: unknown }> = []; + + const result = await setAgentTank("bundled", undefined, fakeClient(recorded)); + + assert.deepEqual(result, { mode: "bundled", enabled: true, url: "http://saved:3456" }); + assert.deepEqual(recorded.at(-1), { + endpoint: "/api/config/agent-tank", + body: { mode: "bundled", url: "http://saved:3456" }, + }); +}); + +test("setAgentTank sends an explicit URL without reading the current settings", async () => { + const recorded: Array<{ endpoint: string; body?: unknown }> = []; + + const result = await setAgentTank("external", "http://127.0.0.1:9999", fakeClient(recorded)); + + assert.equal(result.url, "http://127.0.0.1:9999"); + assert.deepEqual(recorded, [{ + endpoint: "/api/config/agent-tank", + body: { mode: "external", url: "http://127.0.0.1:9999" }, + }]); +}); + +test("setAgentTank derives enabled false only for the disabled mode", async () => { + const recorded: Array<{ endpoint: string; body?: unknown }> = []; + + assert.equal((await setAgentTank("disabled", "http://x:1", fakeClient(recorded))).enabled, false); + assert.equal((await setAgentTank("external", "http://x:1", fakeClient(recorded))).enabled, true); +}); + +test("getAgentTank returns the backend settings unchanged", async () => { + const recorded: Array<{ endpoint: string; body?: unknown }> = []; + + const settings = await getAgentTank(fakeClient(recorded, { mode: "bundled", enabled: true, url: "" })); + + assert.equal(settings.mode, "bundled"); + assert.deepEqual(recorded, [{ endpoint: "/api/config/agent-tank" }]); +}); diff --git a/packages/cli/src/commands/tankCommands.ts b/packages/cli/src/commands/tankCommands.ts index 687285de0..aafaaf17e 100644 --- a/packages/cli/src/commands/tankCommands.ts +++ b/packages/cli/src/commands/tankCommands.ts @@ -1,14 +1,14 @@ /** - * `propr tank on|off` — toggle Agent Tank LLM usage tracking. + * `propr tank bundled|external|off` — configure Agent Tank LLM usage tracking. * - * Agent Tank is an external service, not a stack container, so this is a backend + * Agent Tank is a backend setting rather than a stack container, so this is a * setting flip routed through the running ProPR API. */ import { Command } from "commander"; +import { AGENT_TANK_MODES, type AgentTankMode } from "@propr/shared"; import { getAgentTank, setAgentTank } from "../api/agentTank.js"; import { NetworkError, UnauthorizedError } from "../api/errors.js"; -import { parseOnOffState, ParseStateError } from "../utils/index.js"; function handleApiError(error: unknown): never { if (error instanceof NetworkError) { @@ -21,33 +21,66 @@ function handleApiError(error: unknown): never { process.exit(1); } +/** + * `on` is kept as a deprecated alias for `external` rather than for `bundled`: + * an existing user typing `propr tank on` today means "use my host install", + * and silently repointing them at a container would change behavior under them. + */ +export function parseTankMode(value: string): AgentTankMode | undefined { + const normalized = value.trim().toLowerCase(); + if (normalized === "off") return "disabled"; + if (normalized === "on") return "external"; + return (AGENT_TANK_MODES as readonly string[]).includes(normalized) + ? normalized as AgentTankMode + : undefined; +} + +/** Only `external` talks to a URL, so only `external` prints one. */ +function describeSettings(mode: AgentTankMode, url?: string): string { + return mode === "external" && url ? `${mode} (${url})` : mode; +} + export function createTankCommand(): Command { const tank = new Command("tank") - .description("Toggle Agent Tank LLM usage tracking (requires the stack running)") - .argument("[state]", "on or off (omit to show current setting)") - .option("--url ", "Agent Tank service URL") + .description("Configure Agent Tank LLM usage tracking (requires the stack running)") + .argument("[mode]", "bundled, external, or off (omit to show the current mode)") + .option("--url ", "Agent Tank service URL (external mode only)") .addHelpText("after", ` +Modes: + bundled ProPR runs the Agent Tank CLI inside the agent image (no host install) + external Talk to an Agent Tank daemon you run yourself + off No usage tracking at all + Examples: - $ propr tank # show current setting - $ propr tank on + $ propr tank # show current mode + $ propr tank bundled + $ propr tank external --url http://127.0.0.1:3456 $ propr tank off - $ propr tank on --url http://127.0.0.1:3456 `) - .action(async (state: string | undefined, options: { url?: string }) => { + .action(async (mode: string | undefined, options: { url?: string }) => { try { - if (!state) { + if (!mode) { const current = await getAgentTank(); - console.log(`Agent Tank: ${current.enabled ? "on" : "off"}${current.url ? ` (${current.url})` : ""}`); + console.log(`Agent Tank: ${describeSettings(current.mode, current.url)}`); return; } - const enable = parseOnOffState(state); - const result = await setAgentTank(enable, options.url); - console.log(`Agent Tank ${result.enabled ? "enabled" : "disabled"}${result.url ? ` (${result.url})` : ""}.`); - } catch (error) { - if (error instanceof ParseStateError) { - console.error(`Error: ${error.message}`); + + const parsed = parseTankMode(mode); + if (!parsed) { + console.error(`Error: invalid mode "${mode}". Use one of: ${AGENT_TANK_MODES.join(", ")}, off`); + process.exit(1); + } + if (options.url && parsed !== "external") { + console.error(`Error: --url only applies to external mode.`); process.exit(1); } + if (mode.trim().toLowerCase() === "on") { + console.warn(`Note: "propr tank on" is deprecated; use "propr tank external".`); + } + + const result = await setAgentTank(parsed, options.url); + console.log(`Agent Tank set to ${describeSettings(result.mode, result.url)}.`); + } catch (error) { handleApiError(error); } }); diff --git a/packages/core/src/agents/impl/utils/usageTrackingWrapper.ts b/packages/core/src/agents/impl/utils/usageTrackingWrapper.ts index 044c3fad1..49d71ce57 100644 --- a/packages/core/src/agents/impl/utils/usageTrackingWrapper.ts +++ b/packages/core/src/agents/impl/utils/usageTrackingWrapper.ts @@ -88,14 +88,19 @@ export interface UsageTrackingMetrics { /** * Returns true when Agent Tank tracking is enabled. * - * Checks the database settings for the Agent Tank configuration. - * Tracking is disabled when enabled is false or url is empty/invalid. + * Checks the database settings for the Agent Tank configuration. Tracking is + * off in `disabled` mode, and in `external` mode when the URL is empty or + * invalid. `bundled` mode needs no URL — it runs the CLI in the agent image. */ export async function isAgentTankEnabled(): Promise { try { const settings = await loadAgentTankSettings(); - const enabled = settings.enabled && !!settings.url && settings.url !== 'false' && settings.url !== '0'; - logger.info({ enabled, settings }, 'Agent Tank enabled check'); + // Bundled mode contacts no URL at all, so the URL sanity checks only + // apply to the external transport. + const enabled = settings.mode === 'bundled' + || (settings.mode === 'external' + && !!settings.url && settings.url !== 'false' && settings.url !== '0'); + logger.info({ enabled, mode: settings.mode }, 'Agent Tank enabled check'); return enabled; } catch (err) { logger.warn({ error: (err as Error).message }, 'Failed to load Agent Tank settings, assuming disabled'); diff --git a/packages/core/src/config/configManager.ts b/packages/core/src/config/configManager.ts index 17df3dc6e..182734113 100644 --- a/packages/core/src/config/configManager.ts +++ b/packages/core/src/config/configManager.ts @@ -405,6 +405,8 @@ export { saveAgents, migrateAgentConfigs, type AgentTankSettings, + DEFAULT_AGENT_TANK_URL, + normalizeAgentTankSettings, loadAgentTankSettings, saveAgentTankSettings } from './configManagerAgents.js'; diff --git a/packages/core/src/config/configManagerAgents.ts b/packages/core/src/config/configManagerAgents.ts index a127f8a37..e57048d68 100644 --- a/packages/core/src/config/configManagerAgents.ts +++ b/packages/core/src/config/configManagerAgents.ts @@ -1,7 +1,10 @@ import fs from 'node:fs'; import path from 'node:path'; import { + agentTankModeFromLegacyEnabled, getManagedAgentConfigRelativePath, + normalizeAgentTankMode, + type AgentTankMode, type AgentType, type ReasoningLevel } from '@propr/shared'; @@ -236,29 +239,82 @@ export async function migrateAgentConfigs(): Promise { * Settings for Agent Tank integration (LLM usage monitoring). */ export interface AgentTankSettings { + /** Authoritative integration mode. */ + mode: AgentTankMode; + /** + * Derived convenience flag (`mode !== 'disabled'`). + * + * Kept so the existing `settings.enabled` call sites keep working without a + * sweeping refactor. Treat it as read-only: `mode` is the source of truth, + * and `saveAgentTankSettings` ignores whatever is passed here. + */ enabled: boolean; + /** Only meaningful in `external` mode. */ url: string; } -const DEFAULT_AGENT_TANK_SETTINGS: AgentTankSettings = { - enabled: false, - url: 'http://0.0.0.0:3456' -}; +export const DEFAULT_AGENT_TANK_URL = 'http://0.0.0.0:3456'; + +/** + * Environment fallback for headless/automated deployments that configure the + * stack entirely through `.env` and never open the Settings UI. Database + * settings still win; this only fills in a missing record. + */ +function environmentModeFallback(): AgentTankMode | undefined { + const raw = process.env.AGENT_TANK_MODE?.trim(); + if (!raw) return undefined; + const normalized = normalizeAgentTankMode(raw); + // normalizeAgentTankMode is total, so an unrecognized value silently becomes + // 'disabled'. Log it instead of pretending the operator asked for that. + if (normalized === 'disabled' && raw !== 'disabled') { + logger.warn({ AGENT_TANK_MODE: raw }, 'Unrecognized AGENT_TANK_MODE; treating Agent Tank as disabled'); + } + return normalized; +} + +/** + * Accepts both the current `{ mode, url }` shape and the legacy + * `{ enabled, url }` shape written before bundled mode existed. + */ +export function normalizeAgentTankSettings(raw: unknown): AgentTankSettings { + const record = (raw && typeof raw === 'object') ? raw as Record : {}; + const mode = 'mode' in record + ? normalizeAgentTankMode(record.mode) + // No `mode` key at all means this record predates the feature (or is + // empty). Derive from the legacy boolean, then from the environment. + : ('enabled' in record + ? agentTankModeFromLegacyEnabled(record.enabled) + : environmentModeFallback() ?? 'disabled'); + const url = typeof record.url === 'string' && record.url.trim() + ? record.url.trim() + : (process.env.AGENT_TANK_URL?.trim() || DEFAULT_AGENT_TANK_URL); + return { mode, enabled: mode !== 'disabled', url }; +} /** * Loads Agent Tank settings from the database. */ export async function loadAgentTankSettings(): Promise { - const settings = await getConfig('agent_tank', DEFAULT_AGENT_TANK_SETTINGS); - logger.info({ agentTank: settings }, 'Successfully loaded Agent Tank settings'); + // Read as `unknown`: the persisted value may be the legacy shape, and the + // normalizer is what guarantees callers only ever see the current one. + const raw = await getConfig('agent_tank', {}); + const settings = normalizeAgentTankSettings(raw); + logger.info({ agentTank: { mode: settings.mode } }, 'Successfully loaded Agent Tank settings'); return settings; } /** * Saves Agent Tank settings to the database. */ -export async function saveAgentTankSettings(settings: AgentTankSettings): Promise { - await saveConfig('agent_tank', settings); - logger.info({ agentTank: settings }, 'Successfully saved Agent Tank settings'); +export async function saveAgentTankSettings( + settings: Pick & Partial +): Promise { + // Persist the canonical shape only. `enabled` is intentionally written too, + // so that a rollback to an older build still reads a sane boolean instead of + // defaulting Agent Tank on/off arbitrarily. + const mode = normalizeAgentTankMode(settings.mode); + const persisted = { mode, enabled: mode !== 'disabled', url: settings.url || DEFAULT_AGENT_TANK_URL }; + await saveConfig('agent_tank', persisted); + logger.info({ agentTank: { mode } }, 'Successfully saved Agent Tank settings'); return true; } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index d7472960d..5a37299da 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -364,9 +364,19 @@ export { toAgentTankAgent, toProprAgent, normalizeAgentTankStatus, - normalizeAgentTankAgents + normalizeAgentTankAgents, + getAllStatuses as getAgentTankStatuses } from './services/agentTankService.js'; export type { AgentStatusResponse } from './services/agentTankService.js'; +export { + buildBundledAgentTankConfig, + canRunBundledAgentTank, + parseBundledAgentTankOutput, + refreshBundledStatuses, + getCachedBundledStatuses, + getBundledStatusesForDelta, + clearBundledAgentTankCache +} from './services/agentTankBundledRunner.js'; export type { BuildOpenCodePromptOptions, OpenCodeDockerArgsParams, OpenCodeEvent, ParsedOpenCodeOutput } from './agents/impl/openCodeUtils.js'; export { VibeAgent, parseVibeConversationLog, parseVibeOutput } from './agents/impl/VibeAgent.js'; export type { diff --git a/packages/core/src/services/agentTankBundledRunner.ts b/packages/core/src/services/agentTankBundledRunner.ts new file mode 100644 index 000000000..2de7d8d24 --- /dev/null +++ b/packages/core/src/services/agentTankBundledRunner.ts @@ -0,0 +1,320 @@ +/** + * Bundled Agent Tank transport. + * + * Runs `agent-tank --once --json --config ` inside the unified + * `propr/agent` image, with each enabled agent's credential directory + * bind-mounted read-only at the same container path the agent runtime uses. + * No host install, no daemon, no container networking to get wrong. + * + * Everything Docker-specific (image resolution, mounts, timeouts) and every + * assumption about the Agent Tank config/output schema lives here, so the HTTP + * transport stays readable and an upstream schema change is a local edit. + */ + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { randomBytes } from 'node:crypto'; +import logger from '../utils/logger.js'; +import { executeDockerCommand } from '../claude/docker/dockerExecutor.js'; +import { loadAgents, resolveConfigPath, resolveCodexConfigPath } from '../config/configManager.js'; +import { CONTAINER_CONFIG_PATHS } from '../agents/types.js'; +import type { AgentConfig } from '../agents/types.js'; +import { toAgentTankAgent, type AgentStatusResponse } from './agentTankTypes.js'; + +/** + * A bundled refresh starts a container and drives interactive `/usage` calls + * through a PTY, so it is slow by nature. Callers never block on it: the hot + * path reads `getCachedBundledStatuses()` and a refresh happens out of band. + */ +const DEFAULT_REFRESH_TIMEOUT_MS = 120_000; +/** + * Provider usage windows move on the order of minutes, so a 60s snapshot is + * plenty fresh for a capacity gauge while keeping container churn near zero. + */ +const DEFAULT_CACHE_TTL_MS = 60_000; +/** + * Past this age a snapshot is too stale to subtract for a per-call delta: two + * LLM calls could both read the same snapshot and report a bogus zero delta, or + * a very old snapshot could attribute unrelated consumption to this call. We + * would rather record no delta than a wrong one. + */ +const DELTA_FRESHNESS_MS = 90_000; + +const CONTAINER_CONFIG_FILE = '/tmp/propr-agent-tank/config.json'; + +/** + * Agent Tank only knows these three providers (`SUPPORTED_PROVIDERS` upstream). + * OpenCode and Vibe have no usage endpoint to read, so including them would + * make the whole run exit non-zero on an "Unsupported agent provider" error. + */ +const BUNDLED_SUPPORTED_TANK_AGENTS = new Set(['claude', 'codex', 'agy']); + +interface CachedSnapshot { + agents: Record; + capturedAt: number; +} + +/** One entry of the generated Agent Tank `agents` array. */ +export interface BundledAgentTankEntry { + /** Agent Tank provider key (`claude`, `codex`, `agy`). */ + provider: string; + /** Container path holding that provider's credentials. */ + configPath: string; +} + +let cached: CachedSnapshot | undefined; +// Coalesces concurrent refresh requests onto a single container run. Without +// this, the sidebar poll and a task's post-call probe could each spawn one. +let inFlight: Promise | undefined> | undefined; + +function timeoutMs(): number { + const parsed = Number.parseInt(process.env.AGENT_TANK_BUNDLED_TIMEOUT_MS || '', 10); + return Number.isFinite(parsed) && parsed > 0 ? parsed : DEFAULT_REFRESH_TIMEOUT_MS; +} + +function cacheTtlMs(): number { + const parsed = Number.parseInt(process.env.AGENT_TANK_BUNDLED_CACHE_TTL_MS || '', 10); + return Number.isFinite(parsed) && parsed > 0 ? parsed : DEFAULT_CACHE_TTL_MS; +} + +/** + * Resolve the host path that holds this agent's credentials. + * + * Codex has its own resolver because its portable `~/.codex` default has to be + * mapped through the launcher's host mapping rather than expanded against the + * backend container's HOME. Reusing the agent runtime's own resolvers is what + * guarantees bundled Agent Tank inspects exactly the credentials the agent + * itself would use - not a lookalike directory. + */ +function hostCredentialPath(agent: AgentConfig): string | undefined { + try { + return agent.type === 'codex' + ? resolveCodexConfigPath(agent.configPath) + : resolveConfigPath(agent.configPath); + } catch (error) { + logger.debug({ agentAlias: agent.alias, error: (error as Error).message }, + 'Skipping agent for bundled Agent Tank: credential path is unavailable'); + return undefined; + } +} + +/** + * ONE OF TWO PLACES that know the Agent Tank config file schema (the other is + * `parseBundledAgentTankOutput`). Verified against integry/agent-tank + * `src/agent-config.js`: each entry takes `provider` (claude | codex | agy), an + * optional `id`, and a `configPath` that is handed to the CLI as its config + * home (`CLAUDE_CONFIG_DIR`, `CODEX_HOME`, `GEMINI_CLI_HOME`). If upstream + * renames keys, change only this function. + */ +export function buildBundledAgentTankConfig(entries: BundledAgentTankEntry[]): string { + return JSON.stringify({ + agents: entries.map(entry => ({ + provider: entry.provider, + // Pin the id to the provider key so the output map is keyed exactly + // like the HTTP `/status` response every downstream consumer parses. + id: entry.provider, + configPath: entry.configPath, + })), + // `--once` already skips the HTTP server; disabling Docker bridge + // detection stops Agent Tank from shelling out to a `docker` binary that + // deliberately does not exist inside the agent image. + dockerAccess: false, + }, null, 2); +} + +/** + * ONE OF TWO PLACES that know the Agent Tank output schema. Upstream + * `--once --json` prints `watcher.getStatus()`, a bare map keyed by agent id; + * we also accept an `{ agents: {...} }` envelope so a minor upstream wrapper + * change does not break the integration. + */ +export function parseBundledAgentTankOutput(stdout: string): Record { + const trimmed = stdout.trim(); + if (!trimmed) return {}; + // `--once --json` may be preceded by banner lines; start at the first brace + // rather than assuming the whole buffer parses. + const start = trimmed.indexOf('{'); + if (start < 0) return {}; + let parsed: Record; + try { + parsed = JSON.parse(trimmed.slice(start)) as Record; + } catch (error) { + logger.warn({ error: (error as Error).message }, 'Bundled Agent Tank produced unparseable JSON'); + return {}; + } + const source = (parsed.agents && typeof parsed.agents === 'object') + ? parsed.agents as Record + : parsed; + const agents: Record = {}; + for (const [key, value] of Object.entries(source)) { + if (!value || typeof value !== 'object' || Array.isArray(value)) continue; + const status = value as Partial; + agents[key] = { + name: typeof status.name === 'string' ? status.name : key, + usage: (status.usage && typeof status.usage === 'object') + ? status.usage as Record + : {}, + metadata: status.metadata, + lastUpdated: status.lastUpdated, + error: typeof status.error === 'string' ? status.error : null, + }; + } + return agents; +} + +/** + * Resolve the image to run Agent Tank in: the exact one the agent registry is + * already using, so bundled Agent Tank always matches the CLI versions the + * agents actually run with. + */ +async function resolveAgentImage(): Promise { + try { + const { AgentRegistry } = await import('../agents/AgentRegistry.js'); + const configured = AgentRegistry.getInstance().getAllAgents()[0]?.config.dockerImage; + if (configured) return configured; + } catch (error) { + logger.debug({ error: (error as Error).message }, + 'Agent registry unavailable for bundled Agent Tank; falling back to the configured image name'); + } + return process.env.AGENT_DOCKER_IMAGE || 'propr/agent:latest'; +} + +/** Build the mount list and config entries for every eligible enabled agent. */ +async function collectBundledAgents(): Promise<{ mounts: string[]; entries: BundledAgentTankEntry[] }> { + const agents = (await loadAgents()).filter(agent => agent.enabled); + const mounts: string[] = []; + const entries: BundledAgentTankEntry[] = []; + const seen = new Set(); + + for (const agent of agents) { + const provider = toAgentTankAgent(agent.type); + if (!BUNDLED_SUPPORTED_TANK_AGENTS.has(provider)) continue; + // Agent Tank tracks a provider, not a ProPR alias. If two aliases share a + // provider we can only report one; the first enabled one wins, matching + // how the sidebar already groups by provider. + if (seen.has(provider)) continue; + const hostPath = hostCredentialPath(agent); + const containerConfigPath = CONTAINER_CONFIG_PATHS[agent.type]; + if (!hostPath || !containerConfigPath || !fs.existsSync(hostPath)) continue; + seen.add(provider); + // Read-only: usage inspection must never be able to mutate or corrupt the + // credentials the real agent runs depend on. + mounts.push('-v', `${hostPath}:${containerConfigPath}:ro`); + entries.push({ provider, configPath: containerConfigPath }); + } + + return { mounts, entries }; +} + +/** + * True when at least one enabled agent is an Agent Tank provider with readable + * credentials, i.e. when bundled mode would actually report something. Used by + * the detection banner so a fresh install with no usable agent is not nagged to + * enable a feature that would show an empty sidebar. + */ +export async function canRunBundledAgentTank(): Promise { + try { + const { entries } = await collectBundledAgents(); + return entries.length > 0; + } catch (error) { + logger.debug({ error: (error as Error).message }, + 'Could not determine bundled Agent Tank eligibility'); + return false; + } +} + +async function runBundledAgentTank(): Promise | undefined> { + let configDir: string | undefined; + try { + const { mounts, entries } = await collectBundledAgents(); + if (entries.length === 0) { + logger.debug('Bundled Agent Tank skipped: no enabled agent has a readable credential directory'); + return {}; + } + + const image = await resolveAgentImage(); + + configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'propr-agent-tank-')); + const configFile = path.join(configDir, 'config.json'); + fs.writeFileSync(configFile, buildBundledAgentTankConfig(entries), { mode: 0o600 }); + + const result = await executeDockerCommand('docker', [ + 'run', '--rm', + // No inbound/outbound needs beyond the provider APIs the CLIs call; + // we do not add --network none because `/usage` for some providers + // hits the provider API. + '--name', `propr-agent-tank-${randomBytes(6).toString('hex')}`, + '-e', 'PROPR_AGENT_TYPE=agent-tank', + '-v', `${configFile}:${CONTAINER_CONFIG_FILE}:ro`, + ...mounts, + image, + 'agent-tank', '--once', '--json', '--config', CONTAINER_CONFIG_FILE, + ], { timeout: timeoutMs() }); + + if (result.exitCode !== 0) { + logger.warn({ exitCode: result.exitCode, stderr: (result.stderr || '').slice(0, 500) }, + 'Bundled Agent Tank run failed'); + return undefined; + } + return parseBundledAgentTankOutput(result.stdout || ''); + } catch (error) { + logger.warn({ error: (error as Error).message }, 'Bundled Agent Tank run threw'); + return undefined; + } finally { + // Best-effort: the temp file holds no secrets, but leaving one per probe + // would slowly fill the container's tmp. + if (configDir) { + try { fs.rmSync(configDir, { recursive: true, force: true }); } catch { /* ignore */ } + } + } +} + +/** Cache-only read. Safe on the hot path: never spawns a container. */ +export function getCachedBundledStatuses( + options: { maxAgeMs?: number } = {} +): Record | undefined { + if (!cached) return undefined; + const maxAge = options.maxAgeMs ?? cacheTtlMs(); + return Date.now() - cached.capturedAt <= maxAge ? cached.agents : undefined; +} + +/** Cache-only read bounded by the delta freshness window. */ +export function getBundledStatusesForDelta(): Record | undefined { + return getCachedBundledStatuses({ maxAgeMs: DELTA_FRESHNESS_MS }); +} + +/** + * Return a fresh snapshot, reusing the cache when it is young enough and + * coalescing concurrent callers onto one container run. + */ +export async function refreshBundledStatuses( + options: { force?: boolean } = {} +): Promise | undefined> { + if (!options.force) { + const fresh = getCachedBundledStatuses(); + if (fresh) return fresh; + } + if (inFlight) return inFlight; + + inFlight = runBundledAgentTank() + .then(agents => { + // Only replace the cache on success: a transient container failure + // should not blank out a perfectly good recent snapshot. + if (agents) cached = { agents, capturedAt: Date.now() }; + return agents; + }) + .finally(() => { inFlight = undefined; }); + return inFlight; +} + +/** Fire-and-forget refresh used by hot paths that must not await a container. */ +export function scheduleBundledRefresh(): void { + void refreshBundledStatuses().catch(() => { /* best-effort by design */ }); +} + +/** Test seam. */ +export function clearBundledAgentTankCache(): void { + cached = undefined; + inFlight = undefined; +} diff --git a/packages/core/src/services/agentTankService.ts b/packages/core/src/services/agentTankService.ts index ee8fbd362..872557905 100644 --- a/packages/core/src/services/agentTankService.ts +++ b/packages/core/src/services/agentTankService.ts @@ -1,5 +1,26 @@ import logger from '../utils/logger.js'; import { loadAgentTankSettings } from '../config/configManager.js'; +import { + getBundledStatusesForDelta, + refreshBundledStatuses, + scheduleBundledRefresh, +} from './agentTankBundledRunner.js'; +import { + normalizeAgentTankAgents, + normalizeAgentTankStatus, + toAgentTankAgent, + type AgentStatusResponse, +} from './agentTankTypes.js'; + +// The provider-key vocabulary and the status shape live in `agentTankTypes.ts` +// so the bundled runner can share them without importing this router back. +export { + normalizeAgentTankAgents, + normalizeAgentTankStatus, + toAgentTankAgent, + toProprAgent, +} from './agentTankTypes.js'; +export type { AgentStatusResponse } from './agentTankTypes.js'; // Refresh can take 15-20 seconds when CLI agent needs cold start const DEFAULT_TIMEOUT_MS = 25000; @@ -17,61 +38,6 @@ async function getAgentTankBaseUrl(): Promise { } } -const AGENT_TANK_AGENT_ALIASES: Record = { - antigravity: 'agy', -}; - -const PROPR_AGENT_ALIASES: Record = Object.fromEntries( - Object.entries(AGENT_TANK_AGENT_ALIASES).map(([proprAgent, tankAgent]) => [tankAgent, proprAgent]) -); - -/** - * Translate ProPR agent aliases to Agent Tank provider keys. - * - * ProPR exposes Google's agent as "antigravity", while Agent Tank tracks the - * same provider under the CLI key "agy". - */ -export function toAgentTankAgent(agent: string): string { - return AGENT_TANK_AGENT_ALIASES[agent] || agent; -} - -/** Translate Agent Tank provider keys back to ProPR agent aliases. */ -export function toProprAgent(agent: string): string { - return PROPR_AGENT_ALIASES[agent] || agent; -} - -/** - * Response shape from GET /status/:agent - * - * Example call: - * const status = await getStatus('claude'); - * // GET http://0.0.0.0:3456/status/claude - * // => { "name": "claude", "usage": { "session": { "percent": 42, ... }, ... }, ... } - */ -export interface AgentStatusResponse { - name: string; - usage: Record; - metadata?: Record; - lastUpdated?: string; - error?: string | null; - isRefreshing?: boolean; -} - -/** Normalize a single Agent Tank status object to ProPR-facing agent names. */ -export function normalizeAgentTankStatus(status: AgentStatusResponse): AgentStatusResponse { - return { ...status, name: toProprAgent(status.name) }; -} - -/** Normalize a GET /status response map to ProPR-facing agent keys and names. */ -export function normalizeAgentTankAgents(agents: Record): Record { - return Object.fromEntries( - Object.entries(agents).map(([agent, status]) => { - const proprAgent = toProprAgent(agent); - return [proprAgent, { ...status, name: toProprAgent(status.name || agent) }]; - }) - ); -} - /** * Trigger a refresh for the given agent on Agent Tank. * @@ -84,6 +50,17 @@ export function normalizeAgentTankAgents(agents: Record { + const settings = await loadAgentTankSettings(); + if (settings.mode === 'disabled') return; + if (settings.mode === 'bundled') { + // Bundled refresh means starting a container, which can take a minute. + // Callers of refreshAgent (notably the per-LLM-call usage wrapper) run + // on a short budget, so we only *schedule* the work here and let the + // next read pick up the newer snapshot. Explicit user-driven refreshes + // go through the API route, which awaits `refreshBundledStatuses`. + scheduleBundledRefresh(); + return; + } const baseUrl = await getAgentTankBaseUrl(); const tankAgent = toAgentTankAgent(agent); const url = `${baseUrl}/refresh/${encodeURIComponent(tankAgent)}`; @@ -115,6 +92,20 @@ export async function refreshAgent(agent: string, timeoutMs: number = DEFAULT_TI * const status = await getStatus('claude'); */ export async function getStatus(agent: string, timeoutMs: number = DEFAULT_TIMEOUT_MS): Promise { + const settings = await loadAgentTankSettings(); + if (settings.mode === 'disabled') { + throw new Error('Agent Tank is disabled'); + } + if (settings.mode === 'bundled') { + // Cache-only: bounded by the delta freshness window so a stale snapshot + // cannot be subtracted to produce a misleading per-call usage delta. + const agents = getBundledStatusesForDelta(); + const status = agents?.[toAgentTankAgent(agent)]; + if (!status) { + throw new Error(`No fresh bundled Agent Tank snapshot for ${agent}`); + } + return normalizeAgentTankStatus(status); + } const baseUrl = await getAgentTankBaseUrl(); const tankAgent = toAgentTankAgent(agent); const url = `${baseUrl}/status/${encodeURIComponent(tankAgent)}`; @@ -140,6 +131,35 @@ export async function getStatus(agent: string, timeoutMs: number = DEFAULT_TIMEO } } +/** + * Transport-agnostic "give me every provider's usage" used by the sidebar and + * the MCP usage tool. Returns `undefined` when tracking is disabled or no data + * is available, so callers can hide the UI rather than render an error. + */ +export async function getAllStatuses( + options: { refresh?: boolean } = {} +): Promise | undefined> { + const settings = await loadAgentTankSettings(); + if (settings.mode === 'disabled') return undefined; + if (settings.mode === 'bundled') { + const agents = await refreshBundledStatuses({ force: options.refresh === true }); + return agents ? normalizeAgentTankAgents(agents) : undefined; + } + + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), DEFAULT_TIMEOUT_MS); + try { + const response = await fetch(`${settings.url}/status`, { signal: controller.signal }); + if (!response.ok) return undefined; + const data = await response.json() as Record; + return normalizeAgentTankAgents(data); + } catch { + return undefined; + } finally { + clearTimeout(timer); + } +} + /** * Recursively compute the numeric delta between two nested usage objects. * diff --git a/packages/core/src/services/agentTankTypes.ts b/packages/core/src/services/agentTankTypes.ts new file mode 100644 index 000000000..e48458167 --- /dev/null +++ b/packages/core/src/services/agentTankTypes.ts @@ -0,0 +1,62 @@ +/** + * Agent Tank vocabulary shared by both transports (HTTP and bundled). + * + * This lives apart from `agentTankService.ts` so the bundled runner can reuse + * the provider-key mapping and the response shape without importing the + * transport router that imports it back. + */ + +const AGENT_TANK_AGENT_ALIASES: Record = { + antigravity: 'agy', +}; + +const PROPR_AGENT_ALIASES: Record = Object.fromEntries( + Object.entries(AGENT_TANK_AGENT_ALIASES).map(([proprAgent, tankAgent]) => [tankAgent, proprAgent]) +); + +/** + * Translate ProPR agent aliases to Agent Tank provider keys. + * + * ProPR exposes Google's agent as "antigravity", while Agent Tank tracks the + * same provider under the CLI key "agy". + */ +export function toAgentTankAgent(agent: string): string { + return AGENT_TANK_AGENT_ALIASES[agent] || agent; +} + +/** Translate Agent Tank provider keys back to ProPR agent aliases. */ +export function toProprAgent(agent: string): string { + return PROPR_AGENT_ALIASES[agent] || agent; +} + +/** + * Response shape from GET /status/:agent + * + * Example call: + * const status = await getStatus('claude'); + * // GET http://0.0.0.0:3456/status/claude + * // => { "name": "claude", "usage": { "session": { "percent": 42, ... }, ... }, ... } + */ +export interface AgentStatusResponse { + name: string; + usage: Record; + metadata?: Record; + lastUpdated?: string; + error?: string | null; + isRefreshing?: boolean; +} + +/** Normalize a single Agent Tank status object to ProPR-facing agent names. */ +export function normalizeAgentTankStatus(status: AgentStatusResponse): AgentStatusResponse { + return { ...status, name: toProprAgent(status.name) }; +} + +/** Normalize a GET /status response map to ProPR-facing agent keys and names. */ +export function normalizeAgentTankAgents(agents: Record): Record { + return Object.fromEntries( + Object.entries(agents).map(([agent, status]) => { + const proprAgent = toProprAgent(agent); + return [proprAgent, { ...status, name: toProprAgent(status.name || agent) }]; + }) + ); +} diff --git a/packages/shared/src/agentTank.ts b/packages/shared/src/agentTank.ts new file mode 100644 index 000000000..347ffde0e --- /dev/null +++ b/packages/shared/src/agentTank.ts @@ -0,0 +1,39 @@ +/** + * Agent Tank integration modes. + * + * `disabled` - no usage tracking at all (the default; nothing is contacted). + * `bundled` - ProPR runs the Agent Tank CLI inside the unified agent image. + * `external` - ProPR talks HTTP to an Agent Tank instance the operator runs. + */ +export const AGENT_TANK_MODES = ['disabled', 'bundled', 'external'] as const; +export type AgentTankMode = typeof AGENT_TANK_MODES[number]; + +export const DEFAULT_AGENT_TANK_MODE: AgentTankMode = 'disabled'; + +/** + * Coerce an unknown persisted/requested value into a valid mode. + * + * This is deliberately total (never throws): it is called on the read path for + * configuration that may predate this feature, and a corrupt value must degrade + * to "off" rather than break settings loading for the whole installation. + */ +export function normalizeAgentTankMode(value: unknown): AgentTankMode { + return typeof value === 'string' && (AGENT_TANK_MODES as readonly string[]).includes(value) + ? value as AgentTankMode + : DEFAULT_AGENT_TANK_MODE; +} + +/** True when `value` is already one of the three modes. */ +export function isAgentTankMode(value: unknown): value is AgentTankMode { + return typeof value === 'string' && (AGENT_TANK_MODES as readonly string[]).includes(value); +} + +/** + * Migrate the pre-mode persisted shape. Historically the only two states were + * "off" and "talk HTTP to a host install", so a legacy `enabled: true` means + * `external` and never `bundled` - we must not silently change what an existing + * installation is pointed at. + */ +export function agentTankModeFromLegacyEnabled(enabled: unknown): AgentTankMode { + return enabled === true ? 'external' : 'disabled'; +} diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 78d8bf3f7..8c3e54e97 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -464,3 +464,14 @@ export * from './visualPreviewCapacity.js'; export * from './previewStorage/v1.js'; export * from './publishedVisualPreviews.js'; + +// Export the Agent Tank integration mode vocabulary shared by core, the API, +// the CLI and the UI so the three states cannot drift between surfaces. +export { + AGENT_TANK_MODES, + DEFAULT_AGENT_TANK_MODE, + agentTankModeFromLegacyEnabled, + isAgentTankMode, + normalizeAgentTankMode, + type AgentTankMode, +} from './agentTank.js'; diff --git a/propr-ui/e2e/agent-tank-modes.pw.ts b/propr-ui/e2e/agent-tank-modes.pw.ts new file mode 100644 index 000000000..08412d756 --- /dev/null +++ b/propr-ui/e2e/agent-tank-modes.pw.ts @@ -0,0 +1,120 @@ +import { expect, test, type Page } from '@playwright/test'; +import { mkdir } from 'node:fs/promises'; +import path from 'node:path'; + +/** + * The three-state Agent Tank integration setting: a radio group where the + * Daemon URL field only exists in external mode. With PROPR_CAPTURE_PREVIEWS + * set it also captures the section in each state. + */ + +const agents = [ + { + id: 'claude', type: 'claude', alias: 'claude', enabled: true, dockerImage: 'propr/agent:latest', configPath: '~/.claude', + supportedModels: ['claude-opus-5-5'], defaultModel: 'claude-opus-5-5', + }, +]; + +async function installFixture( + page: Page, + agentTank: Record, +): Promise>> { + const saved: Array> = []; + let tank = { ...agentTank }; + await page.routeWebSocket('**/socket.io/**', socket => socket.close()); + await page.route('**/api/**', async route => { + const request = route.request(); + const pathname = new URL(request.url()).pathname; + if (pathname === '/api/config/agent-tank' && request.method() === 'POST') { + const body = request.postDataJSON() as Record; + saved.push(body); + tank = { ...tank, ...body, enabled: body.mode !== 'disabled' }; + return route.fulfill({ json: { success: true } }); + } + const responses: Record = { + '/api/auth/demo-mode': { demoMode: false }, + '/api/auth/user': { + id: 'preview-user', login: 'preview', username: 'preview', displayName: 'Preview User', + email: null, avatarUrl: null, role: 'admin', permissions: ['instance.manage_settings', 'instance.manage_agents'], + authorizationSource: 'local', + }, + '/api/config/settings': { + worker_concurrency: 2, auto_followup_score_threshold: 4, auto_resolve_merge_conflicts: false, + ultrafix_rating_goal: 7, ultrafix_max_cycles: 5, ultrafix_pause_seconds: 60, + default_agent_alias: 'claude', model_reasoning_level: '', planner_context_model: '', + planner_generation_model: '', pr_review_model: 'claude:claude-opus-5-5', analysis_model_fast: '', + pr_review_context_enabled: true, pr_review_context_model: '', github_user_whitelist: [], + }, + '/api/config/followup-keywords': { followup_keywords: [] }, + '/api/config/followup-ignore-keywords': { followup_ignore_keywords: [] }, + '/api/config/pr-label': { pr_label: 'propr' }, + '/api/config/primary-processing-labels': { primary_processing_labels: ['AI'] }, + '/api/config/agents': { agents }, + '/api/config/summarization': { enabled: false, agent_alias: '', fallback_agent_alias: '' }, + '/api/config/agent-tank': tank, + '/api/config/agent-tank/status': { available: true, mode: tank.mode }, + '/api/config/agent-tank/detect': { detected: false }, + '/api/instance/catalog': { + agents: agents.map(agent => ({ id: agent.id, kind: 'direct', alias: agent.alias, enabled: true, supportedModels: agent.supportedModels })), + repositories: [], + }, + '/api/notifications/config': { push: { configured: false, vapidPublicKey: null } }, + '/api/notifications/unread-count': { unreadCount: 0 }, + }; + if (pathname in responses) return route.fulfill({ json: responses[pathname] }); + return route.fulfill({ status: 503, json: { error: 'Unavailable in Agent Tank mode fixture' } }); + }); + return saved; +} + +async function capture(page: Page, name: string): Promise { + if (!process.env.PROPR_CAPTURE_PREVIEWS) return; + const directory = path.resolve('../.propr/previews'); + await mkdir(directory, { recursive: true }); + const section = page.getByRole('region', { name: 'LLM Usage Tracking' }); + await section.scrollIntoViewIfNeeded(); + const box = await section.boundingBox(); + if (!box) throw new Error('LLM Usage Tracking section is not visible'); + await page.screenshot({ + animations: 'disabled', + path: path.join(directory, `${name}.png`), + clip: { + x: Math.max(0, box.x - 24), + y: Math.max(0, box.y - 24), + width: box.width + 48, + height: box.height + 48, + }, + }); +} + +test('offers three modes and shows the daemon URL only for external', async ({ page }) => { + await page.setViewportSize({ width: 1280, height: 1000 }); + const saved = await installFixture(page, { mode: 'disabled', enabled: false, url: 'http://host.docker.internal:3456' }); + await page.goto('/settings?tab=integrations'); + + await expect(page.getByRole('radio', { name: /Disabled/ })).toBeChecked(); + await expect(page.getByLabel('Daemon URL')).toHaveCount(0); + await capture(page, 'agent-tank-disabled'); + + await page.getByRole('radio', { name: /Bundled/ }).check(); + await expect.poll(() => saved.at(-1)?.mode).toBe('bundled'); + await expect(page.getByLabel('Daemon URL')).toHaveCount(0); + const section = page.getByRole('region', { name: 'LLM Usage Tracking' }); + await expect(section.getByRole('status')).toContainText('Bundled Agent Tank ready'); + await capture(page, 'agent-tank-bundled'); + + await page.getByRole('radio', { name: /External/ }).check(); + await expect.poll(() => saved.at(-1)?.mode).toBe('external'); + await expect(page.getByLabel('Daemon URL')).toHaveValue('http://host.docker.internal:3456'); + await capture(page, 'agent-tank-external'); +}); + +test('loads a legacy enabled installation as external with its saved URL', async ({ page }) => { + await page.setViewportSize({ width: 1280, height: 1000 }); + // An older backend answers without `mode`; the UI must still select external. + await installFixture(page, { enabled: true, url: 'http://host.docker.internal:3456' }); + await page.goto('/settings?tab=integrations'); + + await expect(page.getByRole('radio', { name: /External/ })).toBeChecked(); + await expect(page.getByLabel('Daemon URL')).toHaveValue('http://host.docker.internal:3456'); +}); diff --git a/propr-ui/src/api/revertApi.ts b/propr-ui/src/api/revertApi.ts index 3b46ccbfb..e633911df 100644 --- a/propr-ui/src/api/revertApi.ts +++ b/propr-ui/src/api/revertApi.ts @@ -1,4 +1,5 @@ import { API_BASE_URL, apiFetch, handleApiResponse } from './apiClient'; +import type { AgentTankMode } from '@propr/shared'; import type { SummarizationSettings } from './proprTypes'; export type { SummarizationSettings }; @@ -78,8 +79,8 @@ export const triggerReindexAll = async (ignoreCooldown = false): Promise => { const response = await apiFetch(`${API_BASE_URL}/api/config/agent-tank`, { credentials: 'include' }); @@ -87,7 +88,7 @@ export const getAgentTankSettings = async (): Promise return response.json(); }; -export const updateAgentTankSettings = async (settings: { enabled: boolean; url: string }): Promise => { +export const updateAgentTankSettings = async (settings: { mode: AgentTankMode; url: string }): Promise => { const response = await apiFetch(`${API_BASE_URL}/api/config/agent-tank`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(settings), credentials: 'include' @@ -150,6 +151,8 @@ export const refreshAgentTank = async (): Promise<{ success: boolean; error?: st export interface AgentTankDetectResponse { detected: boolean; + /** Which mode the banner should offer: bundled needs no url. */ + mode?: AgentTankMode; url?: string; reason?: string; } @@ -160,11 +163,11 @@ export const detectAgentTank = async (): Promise => { return response.json(); }; -export const enableAgentTank = async (url: string): Promise<{ success: boolean }> => { +export const enableAgentTank = async (mode: AgentTankMode, url?: string): Promise<{ success: boolean }> => { const response = await apiFetch(`${API_BASE_URL}/api/config/agent-tank`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ enabled: true, url }), + body: JSON.stringify(url ? { mode, url } : { mode }), credentials: 'include' }); await handleApiResponse(response); diff --git a/propr-ui/src/components/AgentTankDetectionBanner.tsx b/propr-ui/src/components/AgentTankDetectionBanner.tsx index 4e04db6e2..fc3e28d33 100644 --- a/propr-ui/src/components/AgentTankDetectionBanner.tsx +++ b/propr-ui/src/components/AgentTankDetectionBanner.tsx @@ -1,11 +1,16 @@ import React, { useState, useEffect } from 'react'; import { X, Activity } from 'lucide-react'; import { detectAgentTank, enableAgentTank } from '../api/revertApi'; +import type { AgentTankMode } from '@propr/shared'; const DISMISSED_KEY = 'agent-tank-banner-dismissed'; const AgentTankDetectionBanner: React.FC = () => { const [detected, setDetected] = useState(false); + // Which mode the backend suggests: 'external' when a daemon answered at the + // default URL, 'bundled' when nothing is running but the agent image can do + // the job with no install at all. + const [offeredMode, setOfferedMode] = useState('bundled'); const [detectedUrl, setDetectedUrl] = useState(null); const [dismissed, setDismissed] = useState(false); const [enabling, setEnabling] = useState(false); @@ -21,10 +26,11 @@ const AgentTankDetectionBanner: React.FC = () => { // Detect Agent Tank detectAgentTank() .then(result => { - if (result.detected && result.url) { - setDetected(true); - setDetectedUrl(result.url); - } + if (!result.detected) return; + const mode = result.mode === 'external' && result.url ? 'external' : 'bundled'; + setOfferedMode(mode); + setDetectedUrl(mode === 'external' ? result.url ?? null : null); + setDetected(true); }) .catch(() => { // Silently fail - detection is optional @@ -32,10 +38,9 @@ const AgentTankDetectionBanner: React.FC = () => { }, []); const handleEnable = async () => { - if (!detectedUrl) return; setEnabling(true); try { - await enableAgentTank(detectedUrl); + await enableAgentTank(offeredMode, detectedUrl ?? undefined); setDetected(false); // Reload the page to show the sidebar window.location.reload(); @@ -60,10 +65,11 @@ const AgentTankDetectionBanner: React.FC = () => {

- Agent Tank Detected + {offeredMode === 'external' ? 'Agent Tank Detected' : 'Track Your LLM Usage Limits'}

Monitor AI subscription limits and see how much rate-limit capacity each task consumes. + {offeredMode === 'bundled' && ' Runs inside the ProPR agent image — nothing to install.'}

diff --git a/propr-ui/src/pages/SettingsPage/AgentTankSection.test.tsx b/propr-ui/src/pages/SettingsPage/AgentTankSection.test.tsx new file mode 100644 index 000000000..2226bd5c8 --- /dev/null +++ b/propr-ui/src/pages/SettingsPage/AgentTankSection.test.tsx @@ -0,0 +1,60 @@ +import { fireEvent, render, screen } from '@testing-library/react'; +import { describe, expect, test, vi } from 'vitest'; +import type { AgentTankMode } from '@propr/shared'; +import AgentTankSection, { type AgentTankSettings } from './AgentTankSection'; + +function settings(mode: AgentTankMode): AgentTankSettings { + return { mode, enabled: mode !== 'disabled', url: 'http://0.0.0.0:3456' }; +} + +describe('AgentTankSection', () => { + test('hides the daemon URL for disabled and bundled modes', () => { + const { rerender } = render( + + ); + expect(screen.queryByLabelText('Daemon URL')).toBeNull(); + + rerender(); + expect(screen.queryByLabelText('Daemon URL')).toBeNull(); + }); + + test('shows the daemon URL only for external mode', () => { + render(); + + expect(screen.getByLabelText('Daemon URL')).toHaveValue('http://0.0.0.0:3456'); + }); + + test('selecting a mode reports that mode with a consistent derived enabled flag', () => { + const onChange = vi.fn(); + render(); + + fireEvent.click(screen.getByLabelText(/Bundled/)); + + expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ mode: 'bundled', enabled: true })); + }); + + test('selecting disabled reports enabled false', () => { + const onChange = vi.fn(); + render(); + + fireEvent.click(screen.getByLabelText('Disabled')); + + expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ mode: 'disabled', enabled: false })); + }); + + test('bundled mode never claims a URL is unreachable', () => { + const { rerender } = render( + + ); + expect(screen.getByRole('status')).toHaveTextContent('Bundled Agent Tank unavailable'); + + rerender(); + expect(screen.getByRole('status')).toHaveTextContent('Agent Tank unreachable'); + }); + + test('disabled mode shows no connection status at all', () => { + render(); + + expect(screen.queryByRole('status')).toBeNull(); + }); +}); diff --git a/propr-ui/src/pages/SettingsPage/AgentTankSection.tsx b/propr-ui/src/pages/SettingsPage/AgentTankSection.tsx index 15cf96dee..9803a2d32 100644 --- a/propr-ui/src/pages/SettingsPage/AgentTankSection.tsx +++ b/propr-ui/src/pages/SettingsPage/AgentTankSection.tsx @@ -1,8 +1,10 @@ import React from 'react'; -import { SettingsCheckboxField, SettingsField, SettingsSection, SettingsStatus } from './SettingsLayout'; -import { SETTINGS_CONTROL } from './settingsStyles'; +import { SettingsField, SettingsSection, SettingsStatus } from './SettingsLayout'; +import { SETTINGS_CHECKBOX, SETTINGS_CONTROL, SETTINGS_HELPER, SETTINGS_LABEL } from './settingsStyles'; +import type { AgentTankMode } from '@propr/shared'; export interface AgentTankSettings { + mode: AgentTankMode; enabled: boolean; url: string; } @@ -16,6 +18,29 @@ interface AgentTankSectionProps { isCheckingStatus?: boolean; } +const MODE_OPTIONS: Array<{ value: AgentTankMode; label: string; description: string }> = [ + { + value: 'disabled', + label: 'Disabled', + description: 'No usage tracking. Nothing is contacted or started.', + }, + { + value: 'bundled', + label: 'Bundled (recommended)', + description: 'ProPR runs Agent Tank inside the agent image using your configured agent credentials. No host install and no networking required.', + }, + { + value: 'external', + label: 'External installation', + description: 'Talk to an Agent Tank daemon you run yourself over HTTP.', + }, +]; + +/** + * The three modes are mutually exclusive, so this is a radio group rather than + * a checkbox: two booleans could express `disabled + bundled`, which is not a + * real state. + */ const AgentTankSection: React.FC = ({ settings, onChange, @@ -24,11 +49,8 @@ const AgentTankSection: React.FC = ({ isAvailable, isCheckingStatus }) => { - const handleToggleEnabled = () => { - onChange({ - ...settings, - enabled: !settings.enabled - }); + const handleModeChange = (mode: AgentTankMode) => { + onChange({ ...settings, mode, enabled: mode !== 'disabled' }); }; const handleUrlChange = (e: React.ChangeEvent) => { @@ -38,10 +60,16 @@ const AgentTankSection: React.FC = ({ }); }; - const status = !settings.enabled ? null + // Bundled mode has no URL to be unreachable, so it gets its own wording: + // telling an operator their daemon is unreachable would be misleading when + // there is no daemon at all. + const readyLabel = settings.mode === 'bundled' ? 'Bundled Agent Tank ready' : 'Agent Tank connected'; + const failedLabel = settings.mode === 'bundled' ? 'Bundled Agent Tank unavailable' : 'Agent Tank unreachable'; + + const status = settings.mode === 'disabled' ? null : isCheckingStatus ? Checking connection… - : isAvailable === true ? Agent Tank connected - : isAvailable === false ? Agent Tank unreachable + : isAvailable === true ? {readyLabel} + : isAvailable === false ? {failedLabel} : null; return ( @@ -50,7 +78,7 @@ const AgentTankSection: React.FC = ({ status={status} description={ <> - Monitor LLM CLI usage limits via a local{' '} + Monitor LLM CLI usage limits with{' '} = ({ > Agent Tank {' '} - daemon — see the{' '} + — see the{' '} = ({ } className={className} > - +
+ Integration mode +
+ {MODE_OPTIONS.map(option => ( +
+ handleModeChange(option.value)} + className={SETTINGS_CHECKBOX} + /> +
+ +

{option.description}

+
+
+ ))} +
+
- - - + {settings.mode === 'external' && ( + + + + )} ); }; diff --git a/propr-ui/src/pages/SettingsPage/index.tsx b/propr-ui/src/pages/SettingsPage/index.tsx index 42e4a4feb..286951ad5 100644 --- a/propr-ui/src/pages/SettingsPage/index.tsx +++ b/propr-ui/src/pages/SettingsPage/index.tsx @@ -263,7 +263,7 @@ const AdminSettingsPage: React.FC = () => { { id: 'agent-tank', category: 'integrations', - searchText: 'LLM usage tracking Agent Tank daemon URL rate limit Claude Antigravity Codex CLI connection', + searchText: 'LLM usage tracking Agent Tank daemon URL rate limit Claude Antigravity Codex CLI connection mode bundled external disabled', content: ( ({ - enabled: false, - url: 'http://0.0.0.0:3456' - }); + const [agentTankSettings, setAgentTankSettings] = useState({ mode: 'disabled', enabled: false, url: '' }); const [agentTankAvailable, setAgentTankAvailable] = useState(null); const [agentTankCheckingStatus, setAgentTankCheckingStatus] = useState(false); @@ -241,7 +239,7 @@ export function useSettingsState() { try { const agentTankSettingsRequest = requireCompleteConfiguration ? getAgentTankSettings() - : getAgentTankSettings().catch(() => ({ enabled: false, url: 'http://0.0.0.0:3456' })); + : getAgentTankSettings().catch(() => ({ mode: 'disabled', enabled: false, url: 'http://0.0.0.0:3456' })); const [results, catalog] = await Promise.all([ Promise.all([ getSettings(), getFollowupKeywords(), getFollowupIgnoreKeywords(), @@ -263,7 +261,7 @@ export function useSettingsState() { setCatalogAgents(catalog.agents); setSummarizationSettings(parsed.summarizationSettings); setAgentTankSettings(parsed.agentTankSettings); - if (parsed.agentTankSettings.enabled) { + if (parsed.agentTankSettings.mode !== 'disabled') { setAgentTankCheckingStatus(true); getAgentTankStatus() .then(status => setAgentTankAvailable(status.available)) @@ -381,13 +379,13 @@ export function useSettingsState() { saveSettingsOnly(newSettings); }, [settings, saveSettingsOnly]); - const handleAgentTankChange = useCallback((newSettings: { enabled: boolean; url: string }) => { + const handleAgentTankChange = useCallback((newSettings: AgentTankSettingsState) => { setAgentTankSettings(newSettings); setAgentTankAvailable(null); - updateAgentTankSettings(newSettings).catch(err => { + updateAgentTankSettings({ mode: newSettings.mode, url: newSettings.url }).catch(err => { console.error('Failed to save Agent Tank settings:', err); }); - if (newSettings.enabled) { + if (newSettings.mode !== 'disabled') { setAgentTankCheckingStatus(true); setTimeout(() => { getAgentTankStatus() diff --git a/scripts/agent-entrypoint.sh b/scripts/agent-entrypoint.sh index 9ecf2f168..75ad61af4 100644 --- a/scripts/agent-entrypoint.sh +++ b/scripts/agent-entrypoint.sh @@ -15,6 +15,11 @@ if [ -z "$agent_type" ] && [ "$#" -gt 0 ]; then agy|antigravity) agent_type=antigravity ;; opencode|opencode-run|/usr/local/bin/opencode-run) agent_type=opencode ;; vibe) agent_type=vibe ;; + # Agent Tank inspects every provider's credentials read-only, so it owns + # no single agent type and must not run a per-agent entrypoint or its + # ownership repair. Its mounts are :ro by construction, so there is + # nothing to chown anyway. + agent-tank|/usr/local/bin/agent-tank) agent_type=agent-tank ;; esac if [ -z "$agent_type" ]; then case "$1" in @@ -29,8 +34,17 @@ case "$agent_type" in claude|codex|antigravity|opencode|vibe) exec "/home/node/${agent_type}-entrypoint.sh" "$@" ;; + agent-tank) + # Run the command as given, dropping privileges the same way the agent + # entrypoints do so Agent Tank never reads credentials as root. + if [ "$#" -eq 0 ]; then set -- agent-tank; fi + if [ "$(id -u)" = "0" ]; then + exec gosu node "$@" + fi + exec "$@" + ;; *) - echo "Set PROPR_AGENT_TYPE to claude, codex, antigravity, opencode, or vibe" >&2 + echo "Set PROPR_AGENT_TYPE to claude, codex, antigravity, opencode, vibe, or agent-tank" >&2 exit 64 ;; esac diff --git a/scripts/build-images.sh b/scripts/build-images.sh index 4996b3c24..36db1e95f 100755 --- a/scripts/build-images.sh +++ b/scripts/build-images.sh @@ -35,6 +35,10 @@ ANTIGRAVITY_CLI_RELEASE_ID="${ANTIGRAVITY_CLI_RELEASE_ID:-6085322963025920}" ANTIGRAVITY_CLI_SHA512="${ANTIGRAVITY_CLI_SHA512:-5811d39ec1bf96a82ed06de6b8ee2bb7f5be8d74423b8c52b6b975e8f0e2c84c6cc2fa0baf902aad942c7566509c3ba6ddb5ef076260c6a635c4616e6ae17897}" OPENCODE_CLI_VERSION="${OPENCODE_CLI_VERSION:-1.18.31}" VIBE_CLI_VERSION="${VIBE_CLI_VERSION:-2.25.4}" +# Keep this default identical to the ARG default in Dockerfile.agent: only the +# Dockerfile literal participates in the agent bundle content hash, so a +# mismatch here would ship a different Agent Tank build under an existing tag. +AGENT_TANK_CLI_VERSION="${AGENT_TANK_CLI_VERSION:-0.9.10}" PUSH_LATEST="${PUSH_LATEST:-true}" VERSION="$(node -p "require('./package.json').version")" @@ -555,6 +559,7 @@ build_image() { "--build-arg" "ANTIGRAVITY_CLI_SHA512=$ANTIGRAVITY_CLI_SHA512" "--build-arg" "OPENCODE_CLI_VERSION=$OPENCODE_CLI_VERSION" "--build-arg" "VIBE_CLI_VERSION=$VIBE_CLI_VERSION" + "--build-arg" "AGENT_TANK_CLI_VERSION=$AGENT_TANK_CLI_VERSION" ) ;; esac diff --git a/test/agentDockerfileSupplyChain.test.ts b/test/agentDockerfileSupplyChain.test.ts index d0adc2907..f456db8d8 100644 --- a/test/agentDockerfileSupplyChain.test.ts +++ b/test/agentDockerfileSupplyChain.test.ts @@ -65,3 +65,20 @@ test('the image build script uses the same pinned Antigravity version', () => { assert.match(buildScript, /"--build-arg" "ANTIGRAVITY_CLI_RELEASE_ID=\$ANTIGRAVITY_CLI_RELEASE_ID"/); assert.match(buildScript, /"--build-arg" "ANTIGRAVITY_CLI_SHA512=\$ANTIGRAVITY_CLI_SHA512"/); }); + +test('bundled Agent Tank is installed at a pinned version, never latest', () => { + assert.match(dockerfile, /ARG AGENT_TANK_CLI_VERSION=\d+\.\d+\.\d+/); + assert.match(dockerfile, /npm install -g "agent-tank@\$\{AGENT_TANK_CLI_VERSION\}"/); + assert.doesNotMatch(dockerfile, /agent-tank@latest/); + assert.doesNotMatch(dockerfile, /npm install -g agent-tank(\s|$)/m); + // The final stage must actually verify the binary it ships. + assert.match(dockerfile, /&& agent-tank --version/); +}); + +test('the image build script uses the same pinned Agent Tank version', () => { + const dockerVersion = dockerfile.match(/ARG AGENT_TANK_CLI_VERSION=(\d+\.\d+\.\d+)/)?.[1]; + const scriptVersion = buildScript.match(/AGENT_TANK_CLI_VERSION="\$\{AGENT_TANK_CLI_VERSION:-(\d+\.\d+\.\d+)\}"/)?.[1]; + assert.ok(dockerVersion); + assert.equal(scriptVersion, dockerVersion); + assert.match(buildScript, /"--build-arg" "AGENT_TANK_CLI_VERSION=\$AGENT_TANK_CLI_VERSION"/); +}); diff --git a/test/agentTankBundledRunner.test.ts b/test/agentTankBundledRunner.test.ts new file mode 100644 index 000000000..b9eaafede --- /dev/null +++ b/test/agentTankBundledRunner.test.ts @@ -0,0 +1,211 @@ +/** + * Bundled Agent Tank runner. + * + * The expensive failure this guards is container churn: `executeWithUsageTracking` + * probes usage around every LLM call, so without TTL caching and in-flight + * coalescing a burst of calls would each start their own container. + */ + +import { afterEach, beforeEach, mock, test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import type { AgentConfig } from '../packages/core/src/agents/types.js'; +import type { ExecutionResult } from '../packages/core/src/claude/docker/dockerExecutor.js'; + +await mock.module('../packages/core/src/utils/logger.js', { + defaultExport: { + trace: () => {}, + debug: () => {}, + info: () => {}, + warn: () => {}, + error: () => {}, + fatal: () => {}, + }, +}); + +let dockerRuns: string[][] = []; +let dockerResult: ExecutionResult = { + exitCode: 0, + stdout: '{}', + stderr: '', + messageTimestamps: new Map(), +}; +/** Held open so concurrent callers overlap and coalescing is actually exercised. */ +let dockerGate: Promise | undefined; + +await mock.module('../packages/core/src/claude/docker/dockerExecutor.js', { + namedExports: { + executeDockerCommand: async (_command: string, args: string[]): Promise => { + dockerRuns.push(args); + if (dockerGate) await dockerGate; + return dockerResult; + }, + }, +}); + +// Two host credential directories that actually exist, because the runner +// deliberately skips any agent whose credentials are not readable. +const credentialRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'propr-tank-test-')); +const claudeHome = path.join(credentialRoot, 'claude'); +const codexHome = path.join(credentialRoot, 'codex'); +fs.mkdirSync(claudeHome); +fs.mkdirSync(codexHome); + +let configuredAgents: AgentConfig[] = []; + +await mock.module('../packages/core/src/config/configManager.js', { + namedExports: { + loadAgents: async (): Promise => configuredAgents, + resolveConfigPath: (configPath: string): string => configPath, + resolveCodexConfigPath: (configPath: string): string => configPath, + }, +}); + +await mock.module('../packages/core/src/agents/AgentRegistry.js', { + namedExports: { + AgentRegistry: { + getInstance: () => ({ getAllAgents: () => [{ config: { dockerImage: 'propr/agent:test' } }] }), + }, + }, +}); + +const { + buildBundledAgentTankConfig, + clearBundledAgentTankCache, + parseBundledAgentTankOutput, + refreshBundledStatuses, +} = await import('../packages/core/src/services/agentTankBundledRunner.js'); + +function agent(overrides: Partial): AgentConfig { + return { + id: overrides.alias || 'agent', + type: 'claude', + alias: 'claude', + enabled: true, + dockerImage: 'propr/agent:test', + configPath: claudeHome, + supportedModels: [], + ...overrides, + } as AgentConfig; +} + +const SAMPLE_OUTPUT = JSON.stringify({ + claude: { name: 'claude', usage: { session: { percent: 42 } }, lastUpdated: '2026-09-26T00:00:00.000Z' }, + codex: { name: 'codex', usage: { fiveHour: { percentUsed: 3 } } }, +}); + +beforeEach(() => { + dockerRuns = []; + dockerGate = undefined; + dockerResult = { exitCode: 0, stdout: SAMPLE_OUTPUT, stderr: '', messageTimestamps: new Map() }; + configuredAgents = [ + agent({ alias: 'claude', type: 'claude', configPath: claudeHome }), + agent({ alias: 'codex', type: 'codex', configPath: codexHome }), + ]; + clearBundledAgentTankCache(); + delete process.env.AGENT_TANK_BUNDLED_CACHE_TTL_MS; +}); + +afterEach(() => { + clearBundledAgentTankCache(); +}); + +test('ten concurrent refreshes coalesce onto exactly one container run', async () => { + // Hold the fake docker call open so all ten requests are genuinely in flight. + let unblock: () => void = () => {}; + dockerGate = new Promise(resolve => { unblock = resolve; }); + + const pending = Array.from({ length: 10 }, () => refreshBundledStatuses()); + unblock(); + const results = await Promise.all(pending); + + assert.equal(dockerRuns.length, 1); + for (const result of results) { + assert.ok(result?.claude.usage.session); + } +}); + +test('a second refresh inside the TTL spawns no container at all', async () => { + await refreshBundledStatuses(); + assert.equal(dockerRuns.length, 1); + + await refreshBundledStatuses(); + assert.equal(dockerRuns.length, 1); +}); + +test('forcing a refresh bypasses the cache', async () => { + await refreshBundledStatuses(); + await refreshBundledStatuses({ force: true }); + + assert.equal(dockerRuns.length, 2); +}); + +test('a failed run returns undefined and leaves the previous snapshot intact', async () => { + const good = await refreshBundledStatuses(); + assert.ok(good?.claude); + + dockerResult = { exitCode: 1, stdout: '', stderr: 'boom', messageTimestamps: new Map() }; + const failed = await refreshBundledStatuses({ force: true }); + assert.equal(failed, undefined); + + // The good snapshot must survive: a transient container failure should not + // blank out usable data. + const cached = await refreshBundledStatuses(); + assert.ok(cached?.claude); +}); + +test('credential directories are mounted read-only at the agent runtime container paths', async () => { + await refreshBundledStatuses(); + + const args = dockerRuns[0]; + assert.ok(args.includes(`${claudeHome}:/home/node/.claude:ro`)); + assert.ok(args.includes(`${codexHome}:/home/node/.codex:ro`)); + assert.ok(args.includes('propr/agent:test')); + assert.deepEqual(args.slice(-5), ['agent-tank', '--once', '--json', '--config', '/tmp/propr-agent-tank/config.json']); +}); + +test('unsupported providers are left out rather than failing the whole run', async () => { + configuredAgents = [ + agent({ alias: 'opencode', type: 'opencode', configPath: claudeHome }), + agent({ alias: 'vibe', type: 'vibe', configPath: codexHome }), + ]; + clearBundledAgentTankCache(); + + const result = await refreshBundledStatuses(); + + // Nothing to inspect means no container, and an empty (not failed) result. + assert.deepEqual(result, {}); + assert.equal(dockerRuns.length, 0); +}); + +test('the generated config uses the upstream provider/configPath schema', () => { + const config = JSON.parse(buildBundledAgentTankConfig([ + { provider: 'claude', configPath: '/home/node/.claude' }, + { provider: 'agy', configPath: '/home/node/.gemini' }, + ])); + + assert.deepEqual(config.agents, [ + { provider: 'claude', id: 'claude', configPath: '/home/node/.claude' }, + { provider: 'agy', id: 'agy', configPath: '/home/node/.gemini' }, + ]); + assert.equal(config.dockerAccess, false); +}); + +test('output parsing accepts a bare status map, an agents envelope, and leading banner text', () => { + const bare = parseBundledAgentTankOutput('{"claude":{"name":"claude","usage":{"session":{"percent":7}}}}'); + assert.equal((bare.claude.usage.session as { percent: number }).percent, 7); + + const enveloped = parseBundledAgentTankOutput('{"agents":{"agy":{"name":"agy","usage":{}}}}'); + assert.equal(enveloped.agy.name, 'agy'); + + const withBanner = parseBundledAgentTankOutput('Agent Tank starting…\n{"codex":{"name":"codex","usage":{}}}'); + assert.equal(withBanner.codex.name, 'codex'); +}); + +test('output parsing degrades to an empty map instead of throwing', () => { + assert.deepEqual(parseBundledAgentTankOutput(''), {}); + assert.deepEqual(parseBundledAgentTankOutput('no json here'), {}); + assert.deepEqual(parseBundledAgentTankOutput('{ not json'), {}); +}); diff --git a/test/agentTankService.test.ts b/test/agentTankService.test.ts index dc7c978f1..99142f359 100644 --- a/test/agentTankService.test.ts +++ b/test/agentTankService.test.ts @@ -1,17 +1,71 @@ -import { after, test } from 'node:test'; +import { after, beforeEach, mock, test } from 'node:test'; import assert from 'node:assert/strict'; process.env.NODE_ENV = 'test'; -import { closeConnection } from '../packages/core/src/db/connection.js'; -import { +await mock.module('../packages/core/src/utils/logger.js', { + defaultExport: { + trace: () => {}, + debug: () => {}, + info: () => {}, + warn: () => {}, + error: () => {}, + fatal: () => {}, + }, +}); + +import type { AgentTankMode } from '@propr/shared'; +import type { AgentStatusResponse } from '../packages/core/src/services/agentTankTypes.js'; + +let mode: AgentTankMode = 'disabled'; +await mock.module('../packages/core/src/config/configManager.js', { + namedExports: { + loadAgentTankSettings: async () => ({ + mode, + enabled: mode !== 'disabled', + url: 'http://0.0.0.0:3456', + }), + }, +}); + +let scheduledRefreshes = 0; +let bundledSnapshot: Record | undefined; +await mock.module('../packages/core/src/services/agentTankBundledRunner.js', { + namedExports: { + getBundledStatusesForDelta: () => bundledSnapshot, + refreshBundledStatuses: async () => bundledSnapshot, + scheduleBundledRefresh: () => { scheduledRefreshes += 1; }, + }, +}); + +const { + getAllStatuses, + getStatus, normalizeAgentTankAgents, normalizeAgentTankStatus, + refreshAgent, toAgentTankAgent, - toProprAgent -} from '../packages/core/src/services/agentTankService.js'; + toProprAgent, +} = await import('../packages/core/src/services/agentTankService.js'); + +const { closeConnection } = await import('../packages/core/src/db/connection.js'); + +const originalFetch = globalThis.fetch; +let fetchCalls: string[] = []; + +beforeEach(() => { + mode = 'disabled'; + scheduledRefreshes = 0; + bundledSnapshot = undefined; + fetchCalls = []; + globalThis.fetch = (async (input: string | URL | Request) => { + fetchCalls.push(input.toString()); + return new Response('{}', { status: 200, headers: { 'content-type': 'application/json' } }); + }) as typeof fetch; +}); after(async () => { + globalThis.fetch = originalFetch; await closeConnection(); }); @@ -42,3 +96,50 @@ test('normalizes Agent Tank usage maps to ProPR keys and names', () => { assert.deepEqual(Object.keys(normalized).sort(), ['antigravity', 'claude']); assert.equal(normalized.antigravity.name, 'antigravity'); }); + +test('disabled mode contacts nothing at all', async () => { + await refreshAgent('claude'); + await assert.rejects(() => getStatus('claude'), /disabled/); + assert.equal(await getAllStatuses(), undefined); + + assert.deepEqual(fetchCalls, []); + assert.equal(scheduledRefreshes, 0); +}); + +test('bundled mode never issues an HTTP request', async () => { + mode = 'bundled'; + bundledSnapshot = { claude: { name: 'claude', usage: { session: { percent: 5 } } } }; + + // refreshAgent only schedules: the hot path must not wait on a container. + await refreshAgent('claude'); + assert.equal(scheduledRefreshes, 1); + + const status = await getStatus('claude'); + assert.equal(status.name, 'claude'); + + const all = await getAllStatuses(); + assert.deepEqual(Object.keys(all ?? {}), ['claude']); + assert.deepEqual(fetchCalls, []); +}); + +test('bundled mode with a cold cache throws so no usage delta is recorded', async () => { + mode = 'bundled'; + bundledSnapshot = undefined; + + await assert.rejects(() => getStatus('claude'), /No fresh bundled Agent Tank snapshot/); +}); + +test('external mode still talks HTTP to the configured daemon', async () => { + mode = 'external'; + + await refreshAgent('antigravity'); + await getStatus('antigravity'); + await getAllStatuses(); + + assert.deepEqual(fetchCalls, [ + 'http://0.0.0.0:3456/refresh/agy', + 'http://0.0.0.0:3456/status/agy', + 'http://0.0.0.0:3456/status', + ]); + assert.equal(scheduledRefreshes, 0); +}); diff --git a/test/agentTankSettingsMigration.test.ts b/test/agentTankSettingsMigration.test.ts new file mode 100644 index 000000000..aeb91ad0f --- /dev/null +++ b/test/agentTankSettingsMigration.test.ts @@ -0,0 +1,128 @@ +/** + * Agent Tank settings migration. + * + * Every existing installation has a persisted `{ enabled, url }` record written + * before bundled mode existed. These assertions are what guarantee those + * installations keep pointing at exactly the same Agent Tank after upgrading. + */ + +import { afterEach, beforeEach, mock, test } from 'node:test'; +import assert from 'node:assert/strict'; + +await mock.module('../packages/core/src/utils/logger.js', { + defaultExport: { + trace: () => {}, + debug: () => {}, + info: () => {}, + warn: () => {}, + error: () => {}, + fatal: () => {}, + }, +}); + +let storedConfig: unknown; +let storedKey: string | undefined; + +await mock.module('../packages/core/src/config/configStore.js', { + namedExports: { + getConfig: async (_key: string, fallback: T): Promise => + (storedConfig === undefined ? fallback : storedConfig as T), + saveConfig: async (key: string, value: unknown): Promise => { + storedKey = key; + storedConfig = value; + }, + }, +}); + +const { + loadAgentTankSettings, + normalizeAgentTankSettings, + saveAgentTankSettings, +} = await import('../packages/core/src/config/configManagerAgents.js'); + +const originalMode = process.env.AGENT_TANK_MODE; +const originalUrl = process.env.AGENT_TANK_URL; + +beforeEach(() => { + storedConfig = undefined; + storedKey = undefined; + delete process.env.AGENT_TANK_MODE; + delete process.env.AGENT_TANK_URL; +}); + +afterEach(() => { + if (originalMode === undefined) delete process.env.AGENT_TANK_MODE; + else process.env.AGENT_TANK_MODE = originalMode; + if (originalUrl === undefined) delete process.env.AGENT_TANK_URL; + else process.env.AGENT_TANK_URL = originalUrl; +}); + +test('a legacy enabled record migrates to external mode without changing its URL', async () => { + storedConfig = { enabled: true, url: 'http://host.docker.internal:3456' }; + + const settings = await loadAgentTankSettings(); + + assert.equal(settings.mode, 'external'); + assert.equal(settings.url, 'http://host.docker.internal:3456'); + assert.equal(settings.enabled, true); +}); + +test('a legacy disabled record migrates to disabled mode', async () => { + storedConfig = { enabled: false, url: 'http://0.0.0.0:3456' }; + + const settings = await loadAgentTankSettings(); + + assert.equal(settings.mode, 'disabled'); + assert.equal(settings.enabled, false); +}); + +test('a fresh install with no record defaults to disabled', async () => { + const settings = await loadAgentTankSettings(); + + assert.equal(settings.mode, 'disabled'); + assert.equal(settings.enabled, false); +}); + +test('a corrupt mode degrades to disabled rather than breaking settings loading', () => { + assert.equal(normalizeAgentTankSettings({ mode: 'sideways' }).mode, 'disabled'); + assert.equal(normalizeAgentTankSettings({ mode: 42 }).mode, 'disabled'); + assert.equal(normalizeAgentTankSettings(null).mode, 'disabled'); +}); + +test('AGENT_TANK_MODE only applies when no record exists at all', () => { + process.env.AGENT_TANK_MODE = 'bundled'; + + assert.equal(normalizeAgentTankSettings({}).mode, 'bundled'); + // A persisted record always wins over the environment fallback. + assert.equal(normalizeAgentTankSettings({ enabled: false }).mode, 'disabled'); + assert.equal(normalizeAgentTankSettings({ mode: 'external' }).mode, 'external'); +}); + +test('an unrecognized AGENT_TANK_MODE degrades to disabled', () => { + process.env.AGENT_TANK_MODE = 'sideways'; + + assert.equal(normalizeAgentTankSettings({}).mode, 'disabled'); +}); + +test('AGENT_TANK_URL supplies the URL when none is persisted', () => { + process.env.AGENT_TANK_URL = 'http://127.0.0.1:9999'; + + assert.equal(normalizeAgentTankSettings({ mode: 'external' }).url, 'http://127.0.0.1:9999'); +}); + +test('saving persists the canonical shape including a derived enabled boolean', async () => { + await saveAgentTankSettings({ mode: 'bundled', url: 'http://0.0.0.0:3456' }); + + assert.equal(storedKey, 'agent_tank'); + assert.deepEqual(storedConfig, { mode: 'bundled', enabled: true, url: 'http://0.0.0.0:3456' }); + + // A rollback to an older build must read a sane boolean, not a surprise. + await saveAgentTankSettings({ mode: 'disabled', url: 'http://0.0.0.0:3456' }); + assert.deepEqual(storedConfig, { mode: 'disabled', enabled: false, url: 'http://0.0.0.0:3456' }); +}); + +test('saving ignores a caller-supplied enabled flag that contradicts the mode', async () => { + await saveAgentTankSettings({ mode: 'disabled', enabled: true, url: 'http://0.0.0.0:3456' }); + + assert.deepEqual(storedConfig, { mode: 'disabled', enabled: false, url: 'http://0.0.0.0:3456' }); +}); diff --git a/test/agentVersionManagement.test.ts b/test/agentVersionManagement.test.ts index 504910a9e..58046874a 100644 --- a/test/agentVersionManagement.test.ts +++ b/test/agentVersionManagement.test.ts @@ -1,6 +1,8 @@ import { afterEach, describe, test } from 'node:test'; import assert from 'node:assert'; import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; import { AGENT_DEFAULTS } from '@propr/shared'; import { AGENT_IMAGE_NAME, @@ -10,7 +12,7 @@ import { } from '../packages/core/src/agents/constants.js'; import { CONTAINER_CONFIG_PATHS } from '../packages/core/src/agents/types.js'; import { AGENT_CLI_PACKAGES, AGENT_CLI_TAGS, AGENT_DEFAULT_VERSIONS } from '../packages/core/src/agents/version/types.js'; -import { findAgentCliVersionConflicts, generateAgentBundleImageTag, getAvailableVersions, getDefaultAgentCliVersionMatrix, resolveVersion } from '../packages/core/src/agents/version/versionService.js'; +import { computeContentHash, findAgentCliVersionConflicts, generateAgentBundleImageTag, getAvailableVersions, getDefaultAgentCliVersionMatrix, resolveVersion } from '../packages/core/src/agents/version/versionService.js'; import { clearNpmCache } from '../packages/core/src/agents/version/npmClient.js'; const originalFetch = globalThis.fetch; @@ -92,6 +94,35 @@ describe('agent version management', () => { assert.match(buildScript, new RegExp(`^CODEX_CLI_VERSION="\\$\\{CODEX_CLI_VERSION:-${AGENT_DEFAULT_VERSIONS.codex}\\}"$`, 'm')); }); + test('pins the bundled Agent Tank version identically in the Dockerfile and the build script', () => { + const agentDockerfile = fs.readFileSync('Dockerfile.agent', 'utf8'); + const buildScript = fs.readFileSync('scripts/build-images.sh', 'utf8'); + + const pinned = agentDockerfile.match(/^ARG AGENT_TANK_CLI_VERSION=(\d+\.\d+\.\d+)$/m)?.[1]; + assert.ok(pinned, 'Dockerfile.agent must pin ARG AGENT_TANK_CLI_VERSION'); + assert.match(buildScript, new RegExp(`^AGENT_TANK_CLI_VERSION="\\$\\{AGENT_TANK_CLI_VERSION:-${pinned}\\}"$`, 'm')); + }); + + test('changing the pinned Agent Tank version changes the generated bundle image tag', () => { + // Only the Dockerfile literal feeds the content hash, which is exactly + // why the build script default must never drift from it. + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'propr-bundle-tag-')); + const dockerfile = fs.readFileSync('Dockerfile.agent', 'utf8'); + const versions = getDefaultAgentCliVersionMatrix(); + + fs.writeFileSync(path.join(root, 'Dockerfile.agent'), dockerfile); + const before = generateAgentBundleImageTag(versions, computeContentHash(root)); + + fs.writeFileSync( + path.join(root, 'Dockerfile.agent'), + dockerfile.replace(/ARG AGENT_TANK_CLI_VERSION=\d+\.\d+\.\d+/g, 'ARG AGENT_TANK_CLI_VERSION=9.9.9') + ); + const after = generateAgentBundleImageTag(versions, computeContentHash(root)); + + assert.notStrictEqual(before, after); + fs.rmSync(root, { recursive: true, force: true }); + }); + test('defaults every coding agent task execution to 24 hours', () => { const envExample = fs.readFileSync('.env.example', 'utf8'); const timeoutEnvVars = [ From 2deac235b081dba84f18e70d0168cde69398ceac Mon Sep 17 00:00:00 2001 From: "propr-dev[bot]" <1316198+propr-dev[bot]@users.noreply.github.com> Date: Sat, 26 Sep 2026 19:59:31 +0000 Subject: [PATCH 02/22] feat(ai): **Cause of the shard 4 failure** MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Cause of the shard 4 failure** Shard 4 reported 138 passed / 1 failed; the only failing unit was `test/ciRunnerRouting.test.mjs` → *"requires real hosted native Electron assertions on both routes"*: ``` + 'apps/desktop/scripts/published-preview-electron.test.mjs' (actual) ``` The `native-electron` job in `.github/workflows/pr-test-on-label.yml:396` discovers its units dynamically (`grep -l 'prepareNativeElectronTest(' apps/desktop/scripts/*.test.mjs`, minus the harness's own unit), but the test pinned that discovery to a two-file literal. Commit `b59b10c56` (on `main`, merged into this branch's base) added a third native probe, `apps/desktop/scripts/published-preview-electron.test.mjs`, without updating the literal — so the assertion went stale. Nothing in this PR's Agent Tank work is involved; the job's own Electron run (`Full Test Suite Native Electron (hosted)`) passed with all three units. **Fix** (`test/ciRunnerRouting.test.mjs:533`) Derive the expectation from the repository the same way the workflow does, instead of a hand-maintained list: collect every `apps/desktop/scripts/*.test.mjs` that calls `prepareNativeElectronTest(`, assert the harness's own unit is discoverable and excluded from the hosted route, and compare the remainder against the workflow's `mapfile` output. The guard still fails if discovery stops picking up a native probe or starts pulling in the harness unit, but a newly added probe no longer breaks it. **Verification** - `test/ciRunnerRouting.test.mjs`: 44 tests, 44 pass (was failing on this commit). - Sibling CI-contract suites `ciFullSuiteSelection`, `ciWorkflowDeduplication`, `ciChangeClassification`, `testSuiteRunner`: 170 tests, all pass. - This branch's Agent Tank units (`agentTankBundledRunner`, `agentTankService`, `agentTankSettingsMigration`, `agentDockerfileSupplyChain`, `configRoutesAgentTank`, `tankCommands`): 6/6 units pass. Shards 1–3 and the docs job were still queued in that run, so only shard 4's result was available to diagnose; no other failures were reported. No preview files were created — this is a CI test-assertion fix with no user-visible surface. PR: #2555 Comment by: @propr-dev[bot] (ID: 5849359335) Model: claude-opus-5 --- test/ciRunnerRouting.test.mjs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/test/ciRunnerRouting.test.mjs b/test/ciRunnerRouting.test.mjs index 595169046..2f2cc491d 100644 --- a/test/ciRunnerRouting.test.mjs +++ b/test/ciRunnerRouting.test.mjs @@ -530,10 +530,21 @@ describe('PR check routing', () => { cwd: REPOSITORY, encoding: 'utf8', }).stdout.trim().split('\n'); - assert.deepEqual(units, [ - 'apps/desktop/scripts/electron-frame-semantics.test.mjs', - 'apps/desktop/scripts/electron-pairing-zstd.test.mjs', - ]); + // Every desktop unit that opts into the native harness must reach this job, + // so the expectation is discovered from the repository rather than pinned to + // a list that a new native probe would silently fall out of. + const nativeSetupUnit = 'apps/desktop/scripts/electron-native-test-setup.test.mjs'; + const nativeUnits = readdirSync(join(REPOSITORY, 'apps', 'desktop', 'scripts')) + .filter(name => name.endsWith('.test.mjs')) + .map(name => `apps/desktop/scripts/${name}`) + .filter(unit => readFileSync(join(REPOSITORY, unit), 'utf8').includes('prepareNativeElectronTest(')) + .sort(); + // The harness's own unit exercises prepareNativeElectronTest with injected + // platforms instead of launching Electron, so it stays on the shard route. + assert.ok(nativeUnits.includes(nativeSetupUnit), 'the native harness unit is discoverable'); + const expected = nativeUnits.filter(unit => unit !== nativeSetupUnit); + assert.ok(expected.length >= 2, 'the native Electron probes are discovered'); + assert.deepEqual([...units].sort(), expected); assert.match(run, /node scripts\/run-test-suite\.mjs "\$\{files\[@\]\}"/); // The workflow-level shard count must not reach this unsharded run. assert.match(electron, /PROPR_TEST_SHARD_COUNT: ''\n/); From db58429cfe13f3a5f6092834415ced0ecec1077c Mon Sep 17 00:00:00 2001 From: "propr-dev[bot]" <1316198+propr-dev[bot]@users.noreply.github.com> Date: Sat, 26 Sep 2026 22:35:56 +0000 Subject: [PATCH 03/22] feat(ai): ## What failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What failed Shard 4/4 passed **all 139 of its 139 test units** (`Shard 4/4: 139/139 passed in 459.8s`, every suite reporting `# fail 0`). The job failed in the *teardown* step afterwards: ``` Run ./scripts/ci-redis.sh stop Error response from daemon: cannot remove container "8dc535d37f1d…": could not kill container: container 8dc535d37f1d PID 2029746 is zombie and can not be killed. Use the --init option when creating containers… ##[error]Process completed with exit code 1. ``` Two defects, both fixed: **1. The zombie was manufactured by the container's own health check.** `ci-redis.sh` starts Redis with a `redis-cli ping` health check every 2s and no init process. Each health-check process is reparented to PID 1 in the container's namespace when its runc parent exits, and `redis-server` does not reap children — so a ~7.5 minute shard accumulated hundreds of zombies against a `--pids-limit 64`, ending with a PID 1 the rootless daemon could not kill. `scripts/ci-redis.sh:186` now passes `--init`, making tini PID 1 so it reaps them. `--init` is already used throughout this repo's Docker invocations, so runner support is established. **2. Teardown failing an otherwise-green shard.** `stop` runs after the tests have already decided the job result, and no step in the job can reap a zombie PID. `remove_container` now returns a distinct status (`UNREMOVABLE_STATUS=3`) when ownership verifies fully but the daemon still refuses removal, and a new `stop_for_teardown` turns only that case into a `::warning::` annotation and exit 0, leaving the container for host cleanup. The state file is kept so a later teardown of the same owner retries. Ownership violations, and removal failures during `start` (where reusing the name would collide), still fail as before. I also added an init-binary check to `scripts/ci-rootless-preflight.sh`, so a daemon that cannot run `--init` containers names that prerequisite instead of failing every shard's Redis start with an opaque OCI error. ## Verification - `node --test test/ciRunnerRouting.test.mjs test/testSuiteRunner.test.mjs` — 66/66 pass. - Negative-checked each new assertion by restoring the original scripts: the two new `ci-redis.sh` tests and the extended preflight case fail without the fix (`# fail 2` and `# fail 1` respectively), then pass with it. - `shellcheck --exclude=SC2016` (the same exclusion `pr-build-check.yml` uses) clean on both scripts; `eslint` clean on the test file. Three new/extended tests cover it, using the existing Docker CLI double: teardown tolerates an unkillable owned container and retries on the next stop; `start` and ownership violations still fail; and `--init` is asserted in the run arguments. `docs/ci-runners.md` documents the cause, the `--init` remedy and the precise scope of the teardown tolerance. The change is CI-only (shell scripts, tests, docs) with nothing visually perceptible, so I did not generate preview files. PR: #2555 Comment by: @propr-dev[bot] (ID: 5850397831) Model: claude-opus-5 --- docs/ci-runners.md | 20 ++++++++++-- scripts/ci-redis.sh | 41 +++++++++++++++++++++--- scripts/ci-rootless-preflight.sh | 6 ++++ test/ciRunnerRouting.test.mjs | 54 +++++++++++++++++++++++++++++++- 4 files changed, 114 insertions(+), 7 deletions(-) diff --git a/docs/ci-runners.md b/docs/ci-runners.md index 89b0deb05..2a1858b3d 100644 --- a/docs/ci-runners.md +++ b/docs/ci-runners.md @@ -73,7 +73,8 @@ expected contract, not a claim that the workers are configured or validated: `DOCKER_CONTEXT`, `DOCKER_TLS_VERIFY` and `DOCKER_CERT_PATH`. Job setup replaces HOME and Docker client config, so a saved HOME-based Docker context is not a reliable endpoint. `ci-rootless-preflight.sh` rejects default/remote/production - endpoints, checks the daemon reports rootless, and requires cgroup v2/systemd. + endpoints, checks the daemon reports rootless, requires cgroup v2/systemd, and + requires an init binary (the Redis helper starts `--init` containers). It does not prove socket ownership, host mount isolation or effective limits. - CI paths used as Docker bind sources must contain the same files at the same absolute path inside the runner and the daemon's host mount namespace. Map @@ -290,10 +291,25 @@ older attempts matching the exact owner; it preserves newer attempts and all other owners. An unexpected owner fails closed. Existing callers with no instance, including nightly, retain one container per job and attempt. +Each container runs with `--init`. The container's PID namespace reparents every +health-check process to PID 1 once its runc parent exits, and `redis-server` +does not reap them; one check every two seconds for the length of a shard +therefore filled `--pids-limit` with zombies and left a container the rootless +daemon could not kill, which failed the teardown step of a shard whose tests had +all passed. tini as PID 1 reaps them instead. + +Teardown (`stop`) is the only caller that tolerates a failed removal. It runs +after the tests have decided the job's result, and no step in the job can reap a +zombie PID, so a container whose ownership fully verifies but which the daemon +still refuses to remove is reported as a run warning and left for host cleanup. +Its state file is kept, so a later teardown of the same owner retries. Ownership +violations, and failed removals during `start`, still fail. + Regression tests prove `job=shard, instance=default` and `job=shard-default, instance=` coexist and either stop order preserves the other. They also cover foreign labels, tampered state, field-boundary -collisions, retries and resource limits using a Docker CLI double. +collisions, retries, resource limits, `--init` and the teardown tolerance using +a Docker CLI double. ## Coverage, required check and partial reruns diff --git a/scripts/ci-redis.sh b/scripts/ci-redis.sh index a85cc85fb..0c90f8dae 100755 --- a/scripts/ci-redis.sh +++ b/scripts/ci-redis.sh @@ -17,6 +17,10 @@ INSTANCE="${CI_REDIS_INSTANCE:-}" MEMORY_LIMIT="${CI_REDIS_MEMORY:-512m}" CPU_LIMIT="${CI_REDIS_CPUS:-1}" PIDS_LIMIT="${CI_REDIS_PIDS_LIMIT:-64}" +# remove_container returns this instead of 1 when ownership is fully verified +# but the daemon still refuses to remove the container. Distinct from 1 so an +# ownership violation and a stuck container never collapse into one outcome. +UNREMOVABLE_STATUS=3 if [[ -n "$INSTANCE" && ! "$INSTANCE" =~ ^[A-Za-z0-9][A-Za-z0-9_.-]{0,62}$ ]]; then # Rejected rather than sanitized: rewriting characters could map two @@ -102,12 +106,12 @@ remove_container() { echo "Refusing to remove $name: attempt label does not match" >&2 return 1 fi - docker rm --force "$id" >/dev/null || return 1 + docker rm --force "$id" >/dev/null || return "$UNREMOVABLE_STATUS" echo "Stopped Redis container $name" } stop_redis() { - local name="$CONTAINER_NAME" + local name="$CONTAINER_NAME" status=0 if [[ -f "$STATE_FILE" ]]; then name="$(<"$STATE_FILE")" @@ -117,10 +121,32 @@ stop_redis() { return 1 fi - remove_container "$name" || return 1 + remove_container "$name" || status=$? + # The state file keeps recording the container while it still exists, so a + # later teardown of the same owner retries the removal instead of skipping it. + (( status == 0 )) || return "$status" rm -f "$STATE_FILE" } +# The workflows' teardown step. It runs after the tests have already decided the +# job's result, so a container the daemon cannot kill -- a zombie PID under a +# rootless daemon, which no step in this job can reap -- is reported for host +# cleanup rather than failing an otherwise green shard. Ownership violations and +# every other stop failure still fail the step. +stop_for_teardown() { + local status=0 + + stop_redis || status=$? + if (( status == UNREMOVABLE_STATUS )); then + echo "Docker could not remove $CONTAINER_NAME; leaving it for host cleanup." >&2 + if [[ "${GITHUB_ACTIONS:-}" == "true" ]]; then + echo "::warning::Leaked CI Redis container $CONTAINER_NAME: the daemon could not remove it." + fi + return 0 + fi + return "$status" +} + # Recover only older attempts owned by this exact run, job and instance. # Filters narrow discovery; inspection by immutable ID authorizes removal. remove_previous_attempts() { @@ -147,10 +173,17 @@ start_redis() { # port candidates by owner and retry. A runner-local free-port probe cannot # see listeners in the host namespace, so Docker's bind is authoritative. local publish_port="" run_error run_status port_attempt + + # `--init` makes tini PID 1 so it reaps the health-check processes the + # container's PID namespace reparents to PID 1 once their runc parent exits: + # redis-server does not reap them, and one check every 2s for the length of a + # shard both fills --pids-limit with zombies and leaves behind a container the + # daemon cannot kill at teardown. for port_attempt in 1 2 3 4 5; do if run_error="$(docker run \ --detach \ --rm \ + --init \ --name "$CONTAINER_NAME" \ --label propr.ci.redis=true \ --label "$LABEL_RUN" \ @@ -225,7 +258,7 @@ start_redis() { case "$ACTION" in start) start_redis ;; - stop) stop_redis ;; + stop) stop_for_teardown ;; name) printf '%s\n' "$CONTAINER_NAME" ;; *) echo "Usage: $0 start|stop|name" >&2 diff --git a/scripts/ci-rootless-preflight.sh b/scripts/ci-rootless-preflight.sh index 3da288610..8cffa44cc 100755 --- a/scripts/ci-rootless-preflight.sh +++ b/scripts/ci-rootless-preflight.sh @@ -26,6 +26,12 @@ security="$(docker info --format '{{range .SecurityOptions}}{{println .}}{{end}} [[ "$security" == *name=rootless* ]] || fail 'Docker daemon does not report rootless mode' cgroups="$(docker info --format '{{.CgroupVersion}}/{{.CgroupDriver}}')" [[ "$cgroups" == '2/systemd' ]] || fail 'rootless resource limits require cgroup v2 with systemd' +# The Redis helper runs its container with --init so tini reaps the health-check +# processes the container's PID namespace reparents to PID 1. A daemon without an +# init binary would instead fail every shard's Redis start with an opaque OCI +# error, so name the missing prerequisite here. +init_binary="$(docker info --format '{{.InitBinary}}')" +[[ -n "$init_binary" ]] || fail 'rootless daemon reports no init binary; --init containers cannot start' # The socket remains explicit after HOME and DOCKER_CONFIG move to job state. # Only successful validation enables the always() Redis cleanup on this daemon. diff --git a/test/ciRunnerRouting.test.mjs b/test/ciRunnerRouting.test.mjs index 1715b1566..e9823f135 100644 --- a/test/ciRunnerRouting.test.mjs +++ b/test/ciRunnerRouting.test.mjs @@ -125,6 +125,10 @@ case "$command" in ;; rm) name="\${@: -1}"; [[ "$name" == id-* ]] || exit 9; name="\${name#id-}" + if [[ -n "\${FAKE_UNREMOVABLE:-}" ]]; then + echo "Error response from daemon: cannot remove container \"$name\": could not kill container: container PID 1 is zombie and can not be killed" >&2 + exit 1 + fi rm -f "$state/$name" echo "rm $name" >> "$state/.log" ;; @@ -359,6 +363,49 @@ describe('scripts/ci-redis.sh shared-host isolation', () => { assert.deepEqual(docker.containers(), [name, other].sort()); }); + test('reports an owned container the daemon cannot remove instead of failing teardown', () => { + const docker = createFakeDocker(); + const env = { CI_REDIS_INSTANCE: 'shard-4' }; + const name = startRedis(docker, env); + const result = runRedis(docker, 'stop', { ...env, FAKE_UNREMOVABLE: 'true', GITHUB_ACTIONS: 'true' }); + // The shard's tests have already decided the job result, and no step in + // the job can reap a zombie PID, so teardown reports and continues. + assert.equal(result.status, 0, result.stderr); + assert.match(result.stderr, /is zombie and can not be killed/); + assert.match(result.stderr, new RegExp(`Docker could not remove ${name}`)); + assert.match(result.stdout, new RegExp(`^::warning::Leaked CI Redis container ${name}:`, 'm')); + assert.deepEqual(docker.containers(), [name]); + assert.deepEqual(docker.removals(), []); + // The state file still records it, so a later teardown retries instead + // of reporting the name as already gone. + const retry = runRedis(docker, 'stop', env); + assert.equal(retry.status, 0, retry.stderr); + assert.deepEqual(docker.containers(), []); + assert.deepEqual(docker.removals(), [name]); + }); + + test('still fails start and ownership violations when a removal is refused', () => { + const docker = createFakeDocker(); + // A leftover container of this caller's own name blocks the retry, so a + // start that cannot remove it must not continue. + const blocked = runRedis(docker, 'start', { FAKE_RUN_FAILURES: '1', FAKE_UNREMOVABLE: 'true' }); + assert.notEqual(blocked.status, 0); + assert.match(blocked.stderr, /is zombie and can not be killed/); + assert.doesNotMatch(blocked.stderr, /leaving it for host cleanup/); + assert.deepEqual(docker.containers(), [redisName(docker, {})]); + + // Teardown tolerance covers only the daemon's refusal, never a + // container this caller does not own. + const other = createFakeDocker(); + const name = startRedis(other, {}); + const file = join(other.state, name); + writeFileSync(file, readFileSync(file, 'utf8').replace('propr.ci.redis=true', 'propr.ci.redis=false')); + const refused = runRedis(other, 'stop', { FAKE_UNREMOVABLE: 'true' }); + assert.equal(refused.status, 1, refused.stderr); + assert.match(refused.stderr, /Refusing to remove/); + assert.deepEqual(other.containers(), [name]); + }); + test('rejects invalid instances, attempts and Docker limits', () => { const docker = createFakeDocker(); for (const env of [ @@ -378,6 +425,9 @@ describe('scripts/ci-redis.sh shared-host isolation', () => { assert.equal(option('--cpus'), '1'); assert.equal(option('--pids-limit'), '64'); assert.equal(option('--publish'), '127.0.0.1::6379'); + // tini as PID 1 reaps the reparented health-check processes that would + // otherwise fill --pids-limit and leave an unkillable container. + assert.ok(args.includes('--init')); const overridden = docker.runArguments(startRedis(docker, { CI_REDIS_INSTANCE: 'limits', CI_REDIS_MEMORY: '1g', CI_REDIS_CPUS: '0.5' })); assert.equal(overridden[overridden.indexOf('--memory') + 1], '1g'); assert.equal(overridden[overridden.indexOf('--cpus') + 1], '0.5'); @@ -796,6 +846,7 @@ set -eu case "$*" in *SecurityOptions*) echo "\${FAKE_SECURITY-name=rootless}" ;; *CgroupVersion*) echo "\${FAKE_CGROUPS-2/systemd}" ;; + *InitBinary*) echo "\${FAKE_INIT_BINARY-docker-init}" ;; *) exit 90 ;; esac `); @@ -831,7 +882,8 @@ esac { DOCKER_HOST: 'unix:///run/docker.sock' }, { DOCKER_HOST: 'tcp://localhost:2375' }, { DOCKER_CONTEXT: 'production' }, { DOCKER_TLS_VERIFY: '1' }, { FAKE_SECURITY: 'name=seccomp' }, { FAKE_CGROUPS: '2/none' }, - { FAKE_CGROUPS: '1/systemd' }, { GITHUB_WORKSPACE: '/nonexistent-propr-workspace' }, + { FAKE_CGROUPS: '1/systemd' }, { FAKE_INIT_BINARY: '' }, + { GITHUB_WORKSPACE: '/nonexistent-propr-workspace' }, ]) { const result = preflight(overrides); assert.notEqual(result.status, 0, JSON.stringify(overrides)); From ea4599e5f3e50280bcf980edc6d01e304dd747fb Mon Sep 17 00:00:00 2001 From: "propr-dev[bot]" <1316198+propr-dev[bot]@users.noreply.github.com> Date: Sat, 26 Sep 2026 23:49:17 +0000 Subject: [PATCH 04/22] feat(ai): Both selected findings are fixed, each with a regression that fails without the fix. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both selected findings are fixed, each with a regression that fails without the fix. **F1 — `agent-tank` executable link (`Dockerfile.agent:262`)** Added `link_npm_bin agent-tank agent-tank` to the final stage's linking chain, before the `agent-tank --version` verification. Confirmed against the registry that `agent-tank@0.9.10` declares `bin: { "agent-tank": "bin/agent-tank.js" }`, so the helper resolves a valid target. Completeness sweep of the final stage: `claude`, `codex`, `opencode` use `link_npm_bin`, `vibe`/`vibe-acp`/`uv` are copied as `/usr/local/bin` binaries, `agy` is symlinked from `/home/node/.local/bin` — Agent Tank was the only verified command with no executable in that stage. **F2 — config readable by the container user (`packages/core/src/services/agentTankBundledRunner.ts:238`)** The generated `config.json` is now written `0o444` with an explicit `chmodSync` after the write (the `writeFileSync` mode alone is umask-dependent), so the `node` user inside the image can read it regardless of the backend process UID. The mount stays `:ro`, and the mode itself carries no write bits. The temp directory is also chmodded `0o755`, matching the sibling pattern in `writeVibePromptFile`, which matters for rootless/userns daemons that resolve the bind source as a non-root user. The file contains only provider keys and container paths — no secrets — and cleanup still removes it after every run. This runner is the only place in the PR's changed behavior that generates a bind-mounted file; `configRoutesAgentTank.ts`, `agentTankService.ts` and `tankCommands.ts` write nothing. **Tests** - `test/agentDockerfileSupplyChain.test.ts`: new test extracts every `--version` check in the final stage and asserts each command has a link/copy in that same stage — it reports `agent-tank is verified in the final stage but nothing links its executable there` when the new line is removed. - `test/agentTankBundledRunner.test.ts`: the docker mock now captures the config file's mode at run time (the runner deletes it afterwards); one test asserts world-readable and non-writable, another asserts the file and its directory are removed after the run. The permission test fails when the mode is reverted to `0o600`. Verification: `npx tsx --experimental-test-module-mocks --test test/agentTankBundledRunner.test.ts` → 11/11 pass; `npx tsx --test test/agentDockerfileSupplyChain.test.ts` → 11/11 pass; `tsc --noEmit -p packages/core/tsconfig.json` clean. I did not build the image (no Docker daemon here), so the `agent-tank --version` step in the final stage is verified statically, not by an actual build. No preview files: both changes are a Dockerfile link and file permission bits, with nothing visually perceptible. PR: #2555 Comment by: @propr-ultrafix (ID: 0) Model: claude-opus-5 --- Dockerfile.agent | 1 + .../src/services/agentTankBundledRunner.ts | 14 ++++++- test/agentDockerfileSupplyChain.test.ts | 16 ++++++++ test/agentTankBundledRunner.test.ts | 38 +++++++++++++++++++ 4 files changed, 68 insertions(+), 1 deletion(-) diff --git a/Dockerfile.agent b/Dockerfile.agent index dc0992988..8b63abf2a 100644 --- a/Dockerfile.agent +++ b/Dockerfile.agent @@ -259,6 +259,7 @@ RUN set -eu; \ link_npm_bin @anthropic-ai/claude-code claude \ && link_npm_bin @openai/codex codex \ && link_npm_bin opencode-ai opencode \ + && link_npm_bin agent-tank agent-tank \ && ln -sf /home/node/.local/bin/agy /usr/local/bin/agy \ && chmod +x \ /home/node/agent-entrypoint.sh \ diff --git a/packages/core/src/services/agentTankBundledRunner.ts b/packages/core/src/services/agentTankBundledRunner.ts index 2de7d8d24..538aef0f7 100644 --- a/packages/core/src/services/agentTankBundledRunner.ts +++ b/packages/core/src/services/agentTankBundledRunner.ts @@ -237,7 +237,19 @@ async function runBundledAgentTank(): Promise { assert.equal(scriptVersion, dockerVersion); assert.match(buildScript, /"--build-arg" "AGENT_TANK_CLI_VERSION=\$AGENT_TANK_CLI_VERSION"/); }); + +test('every CLI the final stage verifies is linked into PATH in that same stage', () => { + const finalStage = dockerfile.slice(dockerfile.indexOf('FROM agent-base AS final')); + // Stages are independent: copying a package's node_modules tree does not bring + // along the npm bin symlink created where it was installed. A command that is + // verified but never linked here fails the build with "command not found". + const verified = [...finalStage.matchAll(/&& ([a-z][a-z-]*) --version/g)].map(match => match[1]); + assert.ok(verified.includes('agent-tank'), 'the final stage should verify bundled Agent Tank'); + for (const command of verified) { + assert.match( + finalStage, + new RegExp(`link_npm_bin \\S+ ${command}\\b|ln -sf \\S+ /usr/local/bin/${command}\\b|COPY --from=\\S+ \\S+ /usr/local/bin/${command}\\b`), + `${command} is verified in the final stage but nothing links its executable there` + ); + } +}); diff --git a/test/agentTankBundledRunner.test.ts b/test/agentTankBundledRunner.test.ts index b9eaafede..2c05decb5 100644 --- a/test/agentTankBundledRunner.test.ts +++ b/test/agentTankBundledRunner.test.ts @@ -34,11 +34,24 @@ let dockerResult: ExecutionResult = { }; /** Held open so concurrent callers overlap and coalescing is actually exercised. */ let dockerGate: Promise | undefined; +/** + * Permission bits of the generated config as seen while the container would be + * running. Captured here because the runner deletes the file once the run ends. + */ +let configModes: number[] = []; + +function captureConfigMode(args: string[]): void { + const mount = args.find(arg => arg.endsWith(':/tmp/propr-agent-tank/config.json:ro')); + if (!mount) return; + const hostPath = mount.slice(0, mount.indexOf(':/tmp/propr-agent-tank/config.json:ro')); + configModes.push(fs.statSync(hostPath).mode & 0o777); +} await mock.module('../packages/core/src/claude/docker/dockerExecutor.js', { namedExports: { executeDockerCommand: async (_command: string, args: string[]): Promise => { dockerRuns.push(args); + captureConfigMode(args); if (dockerGate) await dockerGate; return dockerResult; }, @@ -98,6 +111,7 @@ const SAMPLE_OUTPUT = JSON.stringify({ beforeEach(() => { dockerRuns = []; + configModes = []; dockerGate = undefined; dockerResult = { exitCode: 0, stdout: SAMPLE_OUTPUT, stderr: '', messageTimestamps: new Map() }; configuredAgents = [ @@ -209,3 +223,27 @@ test('output parsing degrades to an empty map instead of throwing', () => { assert.deepEqual(parseBundledAgentTankOutput('no json here'), {}); assert.deepEqual(parseBundledAgentTankOutput('{ not json'), {}); }); + +test('the generated config is readable by the container user, not owner-only', async () => { + await refreshBundledStatuses(); + + // Docker bind-mounts the file with the host owner and mode intact, and the + // image runs Agent Tank as `node`. A 0600 file written by a differently + // owned backend process (root in most deployments) would be unreadable + // inside the container, so the refresh would produce nothing. + assert.equal(configModes.length, 1); + assert.equal(configModes[0] & 0o004, 0o004); + // Read-only by mode as well as by mount: nothing should be able to rewrite it. + assert.equal(configModes[0] & 0o222, 0); +}); + +test('the generated config is cleaned up after the run', async () => { + await refreshBundledStatuses(); + + const mount = dockerRuns[0].find(arg => arg.endsWith(':/tmp/propr-agent-tank/config.json:ro')); + assert.ok(mount); + const hostPath = mount.slice(0, mount.indexOf(':/tmp/propr-agent-tank/config.json:ro')); + // A world-readable file must not outlive the run it was written for. + assert.equal(fs.existsSync(hostPath), false); + assert.equal(fs.existsSync(path.dirname(hostPath)), false); +}); From 6309787db2050b62a2d37d9bf38c8e368a5e9abf Mon Sep 17 00:00:00 2001 From: "propr-dev[bot]" <1316198+propr-dev[bot]@users.noreply.github.com> Date: Sun, 27 Sep 2026 00:08:00 +0000 Subject: [PATCH 05/22] feat(ai): Applied F3 only. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applied F3 only. ## The defect In bundled mode, `collectBundledAgents` keeps one agent per provider (first enabled wins) and `buildBundledAgentTankConfig` pins the entry id to the provider key. The alias of the account whose credentials were actually mounted was then thrown away, so a snapshot produced from `claude-secondary`'s credentials came back labelled `claude` and satisfied the consumer's exact-name guard — handing alias `claude`'s capacity decisions another account's usage. ## Correction - `packages/core/src/services/agentTankBundledRunner.ts` — `BundledAgentTankEntry` now carries the selected `alias`; a run returns `{ agents, aliases }` and the cache stores provider→alias provenance in the same single assignment as the snapshots (so an alias read can never pair one run's numbers with another run's provenance). New `getBundledStatusForAlias(alias)` answers only when the requested alias is the one that produced the snapshot, still bounded by the delta freshness window. - `packages/core/src/services/agentTankService.ts` — new `getStatusForAlias(alias)`: bundled mode goes through the provenance-checked read and throws `No fresh bundled Agent Tank snapshot for alias …` otherwise; external mode keeps the daemon's per-name answer. - `packages/core/src/services/syntheticUsageSnapshotProvider.ts` — the alias-specific consumer now defaults to `getStatusForAlias`, so its name check is backed by provenance rather than by a provider-pinned label. - `packages/core/src/index.ts` exports both new readers; `docs/docs/operations/agent-tank.md` documents that a second account of the same provider reports no usage data in bundled mode. Sibling sweep: every `executeWithUsageTracking` call site passes a provider/runtime name (`'claude'`, `'codex'`, `getRuntimeName()`), never an alias, so provider-level `getStatus` and the provider-keyed `getAllStatuses` (sidebar) keep their existing semantics — the synthetic usage provider was the only alias-specific reader. ## Verification - New regressions in `test/agentTankBundledRunner.test.ts` cover the exact two-account ordering (secondary first → `claude` gets nothing, `claude-secondary` gets the snapshot), the reverse ordering, staleness, and cold cache; `test/agentTankService.test.ts` covers the same through `getStatusForAlias` and end-to-end through `AliasSpecificAgentTankSnapshotProvider` (withheld for the wrong account, reported for the inspected one), plus the external-mode path. - Reverting the provenance lookup fails the ordering regression; reverting the consumer's default fetcher fails both consumer regressions. - `npx tsc --noEmit` clean at the root and in `packages/core`; 49/49 tests pass across `agentTankBundledRunner`, `agentTankService`, `agentTankSettingsMigration`, and `syntheticRoutingService`. Backend capacity provenance only — nothing visually perceptible, so no preview files were produced. Reported separately, not implemented: the bundled cache is not invalidated when agent configuration changes, so re-pointing an alias at a different account can be answered from the pre-change snapshot for up to the 90s freshness window. That window is pre-existing for the whole cache (identity is by alias, not by credential fingerprint) and outside this finding. PR: #2555 Comment by: @propr-ultrafix (ID: 0) Model: claude-opus-5 --- docs/docs/operations/agent-tank.md | 2 + packages/core/src/index.ts | 2 + .../src/services/agentTankBundledRunner.ts | 87 +++++++++++++++---- .../core/src/services/agentTankService.ts | 32 +++++++ .../syntheticUsageSnapshotProvider.ts | 11 ++- test/agentTankBundledRunner.test.ts | 60 ++++++++++++- test/agentTankService.test.ts | 74 ++++++++++++++++ 7 files changed, 245 insertions(+), 23 deletions(-) diff --git a/docs/docs/operations/agent-tank.md b/docs/docs/operations/agent-tank.md index 3f1fb2979..b2e4eb296 100644 --- a/docs/docs/operations/agent-tank.md +++ b/docs/docs/operations/agent-tank.md @@ -63,6 +63,8 @@ propr tank bundled Bundled mode reports the providers it can see. An agent with no credentials mounted, or a provider Agent Tank does not support, is simply left out. +If two enabled agents share a provider — two Claude accounts, for example — one run can only inspect one of them, and the first enabled one wins. The snapshot then describes that account only: capacity-aware routing for the other alias reports "no usage data" rather than borrowing the inspected account's numbers. Use external mode if you need every account measured. + ### External Mode Use this when you run Agent Tank yourself. Install and start it on the host that runs your agent CLIs: diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 5a37299da..07c1d2edf 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -365,6 +365,7 @@ export { toProprAgent, normalizeAgentTankStatus, normalizeAgentTankAgents, + getStatusForAlias as getAgentTankStatusForAlias, getAllStatuses as getAgentTankStatuses } from './services/agentTankService.js'; export type { AgentStatusResponse } from './services/agentTankService.js'; @@ -375,6 +376,7 @@ export { refreshBundledStatuses, getCachedBundledStatuses, getBundledStatusesForDelta, + getBundledStatusForAlias, clearBundledAgentTankCache } from './services/agentTankBundledRunner.js'; export type { BuildOpenCodePromptOptions, OpenCodeDockerArgsParams, OpenCodeEvent, ParsedOpenCodeOutput } from './agents/impl/openCodeUtils.js'; diff --git a/packages/core/src/services/agentTankBundledRunner.ts b/packages/core/src/services/agentTankBundledRunner.ts index 538aef0f7..3a0e02d7e 100644 --- a/packages/core/src/services/agentTankBundledRunner.ts +++ b/packages/core/src/services/agentTankBundledRunner.ts @@ -50,8 +50,23 @@ const CONTAINER_CONFIG_FILE = '/tmp/propr-agent-tank/config.json'; */ const BUNDLED_SUPPORTED_TANK_AGENTS = new Set(['claude', 'codex', 'agy']); -interface CachedSnapshot { +/** + * One Agent Tank run: the per-provider snapshots plus which configured account + * each one actually describes. + */ +interface BundledRunResult { agents: Record; + /** + * Provider key -> the alias of the enabled agent whose credentials were + * mounted for that provider. Agent Tank knows only providers, so this is the + * ONLY record of which configured account the numbers belong to: the + * generated id is the provider key, so two accounts of the same provider are + * indistinguishable from the snapshot itself. + */ + aliases: Record; +} + +interface CachedSnapshot extends BundledRunResult { capturedAt: number; } @@ -59,6 +74,8 @@ interface CachedSnapshot { export interface BundledAgentTankEntry { /** Agent Tank provider key (`claude`, `codex`, `agy`). */ provider: string; + /** ProPR alias of the agent whose credentials are mounted for that provider. */ + alias: string; /** Container path holding that provider's credentials. */ configPath: string; } @@ -66,7 +83,7 @@ export interface BundledAgentTankEntry { let cached: CachedSnapshot | undefined; // Coalesces concurrent refresh requests onto a single container run. Without // this, the sidebar poll and a task's post-call probe could each spawn one. -let inFlight: Promise | undefined> | undefined; +let inFlight: Promise | undefined; function timeoutMs(): number { const parsed = Number.parseInt(process.env.AGENT_TANK_BUNDLED_TIMEOUT_MS || '', 10); @@ -106,6 +123,10 @@ function hostCredentialPath(agent: AgentConfig): string | undefined { * optional `id`, and a `configPath` that is handed to the CLI as its config * home (`CLAUDE_CONFIG_DIR`, `CODEX_HOME`, `GEMINI_CLI_HOME`). If upstream * renames keys, change only this function. + * + * There is no place in this schema for the ProPR alias, which is why the alias + * of the account each entry was built from is tracked separately (see + * `BundledRunResult.aliases`) instead of being recovered from the output. */ export function buildBundledAgentTankConfig(entries: BundledAgentTankEntry[]): string { return JSON.stringify({ @@ -192,7 +213,9 @@ async function collectBundledAgents(): Promise<{ mounts: string[]; entries: Bund if (!BUNDLED_SUPPORTED_TANK_AGENTS.has(provider)) continue; // Agent Tank tracks a provider, not a ProPR alias. If two aliases share a // provider we can only report one; the first enabled one wins, matching - // how the sidebar already groups by provider. + // how the sidebar already groups by provider. Which alias won is recorded + // on the entry so an alias-specific reader cannot mistake this account's + // usage for another account of the same provider. if (seen.has(provider)) continue; const hostPath = hostCredentialPath(agent); const containerConfigPath = CONTAINER_CONFIG_PATHS[agent.type]; @@ -201,7 +224,7 @@ async function collectBundledAgents(): Promise<{ mounts: string[]; entries: Bund // Read-only: usage inspection must never be able to mutate or corrupt the // credentials the real agent runs depend on. mounts.push('-v', `${hostPath}:${containerConfigPath}:ro`); - entries.push({ provider, configPath: containerConfigPath }); + entries.push({ provider, alias: agent.alias, configPath: containerConfigPath }); } return { mounts, entries }; @@ -224,14 +247,15 @@ export async function canRunBundledAgentTank(): Promise { } } -async function runBundledAgentTank(): Promise | undefined> { +async function runBundledAgentTank(): Promise { let configDir: string | undefined; try { const { mounts, entries } = await collectBundledAgents(); if (entries.length === 0) { logger.debug('Bundled Agent Tank skipped: no enabled agent has a readable credential directory'); - return {}; + return { agents: {}, aliases: {} }; } + const aliases = Object.fromEntries(entries.map(entry => [entry.provider, entry.alias])); const image = await resolveAgentImage(); @@ -269,7 +293,7 @@ async function runBundledAgentTank(): Promise maxAge) return undefined; + const provider = Object.entries(cached.aliases) + .find(([, snapshotAlias]) => snapshotAlias === alias)?.[0]; + if (!provider) { + logger.debug({ alias, inspectedAliases: Object.values(cached.aliases) }, + 'No bundled Agent Tank snapshot belongs to this alias'); + return undefined; + } + return cached.agents[provider]; +} + /** * Return a fresh snapshot, reusing the cache when it is young enough and * coalescing concurrent callers onto one container run. @@ -307,17 +358,17 @@ export async function refreshBundledStatuses( const fresh = getCachedBundledStatuses(); if (fresh) return fresh; } - if (inFlight) return inFlight; - - inFlight = runBundledAgentTank() - .then(agents => { - // Only replace the cache on success: a transient container failure - // should not blank out a perfectly good recent snapshot. - if (agents) cached = { agents, capturedAt: Date.now() }; - return agents; - }) - .finally(() => { inFlight = undefined; }); - return inFlight; + if (!inFlight) { + inFlight = runBundledAgentTank() + .then(result => { + // Only replace the cache on success: a transient container failure + // should not blank out a perfectly good recent snapshot. + if (result) cached = { ...result, capturedAt: Date.now() }; + return result; + }) + .finally(() => { inFlight = undefined; }); + } + return (await inFlight)?.agents; } /** Fire-and-forget refresh used by hot paths that must not await a container. */ diff --git a/packages/core/src/services/agentTankService.ts b/packages/core/src/services/agentTankService.ts index 872557905..53627dbb5 100644 --- a/packages/core/src/services/agentTankService.ts +++ b/packages/core/src/services/agentTankService.ts @@ -1,6 +1,7 @@ import logger from '../utils/logger.js'; import { loadAgentTankSettings } from '../config/configManager.js'; import { + getBundledStatusForAlias, getBundledStatusesForDelta, refreshBundledStatuses, scheduleBundledRefresh, @@ -131,6 +132,37 @@ export async function getStatus(agent: string, timeoutMs: number = DEFAULT_TIMEO } } +/** + * Fetch usage for one configured agent *alias*, for decisions that are specific + * to that account rather than to the provider as a whole (synthetic-agent + * capacity routing). + * + * Bundled mode inspects one account per provider, so its snapshot can only + * answer for the alias whose credentials produced it; every other alias of the + * same provider is reported as unavailable instead of being handed a stranger's + * numbers. External mode keeps the daemon's own per-name answer. + * + * @example + * const status = await getStatusForAlias('claude-secondary'); + */ +export async function getStatusForAlias( + alias: string, + timeoutMs: number = DEFAULT_TIMEOUT_MS, +): Promise { + const settings = await loadAgentTankSettings(); + if (settings.mode === 'disabled') { + throw new Error('Agent Tank is disabled'); + } + if (settings.mode === 'bundled') { + const status = getBundledStatusForAlias(alias); + if (!status) { + throw new Error(`No fresh bundled Agent Tank snapshot for alias ${alias}`); + } + return normalizeAgentTankStatus(status); + } + return getStatus(alias, timeoutMs); +} + /** * Transport-agnostic "give me every provider's usage" used by the sidebar and * the MCP usage tool. Returns `undefined` when tracking is disabled or no data diff --git a/packages/core/src/services/syntheticUsageSnapshotProvider.ts b/packages/core/src/services/syntheticUsageSnapshotProvider.ts index 6086b55cf..52308e4b0 100644 --- a/packages/core/src/services/syntheticUsageSnapshotProvider.ts +++ b/packages/core/src/services/syntheticUsageSnapshotProvider.ts @@ -1,6 +1,6 @@ import { loadAgentTankSettings } from '../config/configManager.js'; import logger from '../utils/logger.js'; -import { getStatus, type AgentStatusResponse } from './agentTankService.js'; +import { getStatusForAlias, type AgentStatusResponse } from './agentTankService.js'; import type { SyntheticUsageSnapshot, SyntheticUsageSnapshotProvider } from './syntheticRoutingTypes.js'; const DEFAULT_USAGE_FRESHNESS_MS = 5 * 60_000; @@ -23,12 +23,17 @@ function nestedPercent(usage: Record, names: string[]): number return undefined; } -/** Provides fresh usage data only when Agent Tank names the requested direct alias exactly. */ +/** + * Provides fresh usage data only when Agent Tank names the requested direct alias + * exactly. `getStatusForAlias` is what makes that name trustworthy in bundled + * mode, where only one account per provider is inspected: it refuses to answer + * for an alias whose credentials did not produce the snapshot. + */ export class AliasSpecificAgentTankSnapshotProvider implements SyntheticUsageSnapshotProvider { constructor( private readonly now: () => Date = () => new Date(), private readonly freshnessMs = Number(process.env.SYNTHETIC_USAGE_FRESHNESS_MS) || DEFAULT_USAGE_FRESHNESS_MS, - private readonly fetchStatus: (alias: string) => Promise = getStatus, + private readonly fetchStatus: (alias: string) => Promise = getStatusForAlias, ) {} async getSnapshot(directAgentAlias: string): Promise { diff --git a/test/agentTankBundledRunner.test.ts b/test/agentTankBundledRunner.test.ts index 2c05decb5..12ea2866c 100644 --- a/test/agentTankBundledRunner.test.ts +++ b/test/agentTankBundledRunner.test.ts @@ -63,8 +63,11 @@ await mock.module('../packages/core/src/claude/docker/dockerExecutor.js', { const credentialRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'propr-tank-test-')); const claudeHome = path.join(credentialRoot, 'claude'); const codexHome = path.join(credentialRoot, 'codex'); +// A second Claude account, for the case where two aliases share one provider. +const secondaryClaudeHome = path.join(credentialRoot, 'claude-secondary'); fs.mkdirSync(claudeHome); fs.mkdirSync(codexHome); +fs.mkdirSync(secondaryClaudeHome); let configuredAgents: AgentConfig[] = []; @@ -87,6 +90,7 @@ await mock.module('../packages/core/src/agents/AgentRegistry.js', { const { buildBundledAgentTankConfig, clearBundledAgentTankCache, + getBundledStatusForAlias, parseBundledAgentTankOutput, refreshBundledStatuses, } = await import('../packages/core/src/services/agentTankBundledRunner.js'); @@ -196,8 +200,8 @@ test('unsupported providers are left out rather than failing the whole run', asy test('the generated config uses the upstream provider/configPath schema', () => { const config = JSON.parse(buildBundledAgentTankConfig([ - { provider: 'claude', configPath: '/home/node/.claude' }, - { provider: 'agy', configPath: '/home/node/.gemini' }, + { provider: 'claude', alias: 'claude', configPath: '/home/node/.claude' }, + { provider: 'agy', alias: 'antigravity', configPath: '/home/node/.gemini' }, ])); assert.deepEqual(config.agents, [ @@ -247,3 +251,55 @@ test('the generated config is cleaned up after the run', async () => { assert.equal(fs.existsSync(hostPath), false); assert.equal(fs.existsSync(path.dirname(hostPath)), false); }); + +test('an alias-specific read only answers for the account whose credentials were inspected', async () => { + // Two Claude accounts, the secondary one first. Provider dedup keeps only + // `claude-secondary`'s credentials, but Agent Tank labels the result with the + // provider key `claude` - so without provenance the snapshot would be handed + // out as the capacity of alias `claude`, which is a different account. + configuredAgents = [ + agent({ alias: 'claude-secondary', type: 'claude', configPath: secondaryClaudeHome }), + agent({ alias: 'claude', type: 'claude', configPath: claudeHome }), + ]; + clearBundledAgentTankCache(); + + await refreshBundledStatuses(); + + // Only the first enabled Claude account was mounted, so it is the only + // account the snapshot can describe. + assert.ok(dockerRuns[0].includes(`${secondaryClaudeHome}:/home/node/.claude:ro`)); + assert.equal(dockerRuns[0].includes(`${claudeHome}:/home/node/.claude:ro`), false); + + assert.equal(getBundledStatusForAlias('claude'), undefined); + assert.equal(getBundledStatusForAlias('claude-secondary')?.name, 'claude'); +}); + +test('an alias-specific read follows the order the aliases are configured in', async () => { + // Same two accounts, opposite order: now the snapshot really is alias + // `claude`'s, and the secondary alias is the one that must get nothing. + configuredAgents = [ + agent({ alias: 'claude', type: 'claude', configPath: claudeHome }), + agent({ alias: 'claude-secondary', type: 'claude', configPath: secondaryClaudeHome }), + ]; + clearBundledAgentTankCache(); + + await refreshBundledStatuses(); + + assert.equal(getBundledStatusForAlias('claude')?.name, 'claude'); + assert.equal(getBundledStatusForAlias('claude-secondary'), undefined); +}); + +test('an alias-specific read reports nothing once the snapshot is too stale to trust', async () => { + await refreshBundledStatuses(); + assert.ok(getBundledStatusForAlias('claude')); + + // Past the delta freshness window the snapshot is no longer evidence about + // the account's current capacity. + assert.equal(getBundledStatusForAlias('claude', { maxAgeMs: -1 }), undefined); +}); + +test('an alias-specific read reports nothing when no run has succeeded', () => { + clearBundledAgentTankCache(); + + assert.equal(getBundledStatusForAlias('claude'), undefined); +}); diff --git a/test/agentTankService.test.ts b/test/agentTankService.test.ts index 99142f359..44b93ac2d 100644 --- a/test/agentTankService.test.ts +++ b/test/agentTankService.test.ts @@ -30,9 +30,16 @@ await mock.module('../packages/core/src/config/configManager.js', { let scheduledRefreshes = 0; let bundledSnapshot: Record | undefined; +/** + * Snapshots keyed by the alias whose credentials produced them - the runner only + * inspects one account per provider, so this is what an alias-specific read is + * allowed to see. + */ +let bundledSnapshotsByAlias: Record = {}; await mock.module('../packages/core/src/services/agentTankBundledRunner.js', { namedExports: { getBundledStatusesForDelta: () => bundledSnapshot, + getBundledStatusForAlias: (alias: string) => bundledSnapshotsByAlias[alias], refreshBundledStatuses: async () => bundledSnapshot, scheduleBundledRefresh: () => { scheduledRefreshes += 1; }, }, @@ -41,6 +48,7 @@ await mock.module('../packages/core/src/services/agentTankBundledRunner.js', { const { getAllStatuses, getStatus, + getStatusForAlias, normalizeAgentTankAgents, normalizeAgentTankStatus, refreshAgent, @@ -48,6 +56,9 @@ const { toProprAgent, } = await import('../packages/core/src/services/agentTankService.js'); +const { AliasSpecificAgentTankSnapshotProvider } = + await import('../packages/core/src/services/syntheticUsageSnapshotProvider.js'); + const { closeConnection } = await import('../packages/core/src/db/connection.js'); const originalFetch = globalThis.fetch; @@ -57,6 +68,7 @@ beforeEach(() => { mode = 'disabled'; scheduledRefreshes = 0; bundledSnapshot = undefined; + bundledSnapshotsByAlias = {}; fetchCalls = []; globalThis.fetch = (async (input: string | URL | Request) => { fetchCalls.push(input.toString()); @@ -143,3 +155,65 @@ test('external mode still talks HTTP to the configured daemon', async () => { ]); assert.equal(scheduledRefreshes, 0); }); + +test('bundled mode answers an alias-specific read only for the inspected account', async () => { + mode = 'bundled'; + // Two Claude accounts with `claude-secondary` configured first: provider + // dedup means only that account was inspected, even though Agent Tank labels + // the snapshot with the provider key `claude`. + const status: AgentStatusResponse = { + name: 'claude', + usage: { session: { percent: 5 } }, + lastUpdated: new Date().toISOString(), + }; + bundledSnapshot = { claude: status }; + bundledSnapshotsByAlias = { 'claude-secondary': status }; + + // Alias `claude` is a different account, so it gets nothing rather than the + // secondary account's capacity. + await assert.rejects(() => getStatusForAlias('claude'), /No fresh bundled Agent Tank snapshot for alias claude/); + assert.equal((await getStatusForAlias('claude-secondary')).name, 'claude'); + assert.deepEqual(fetchCalls, []); +}); + +test('alias-specific capacity is withheld when the snapshot belongs to another account of the same provider', async () => { + mode = 'bundled'; + const status: AgentStatusResponse = { + name: 'claude', + usage: { session: { percent: 5 }, weekly: { percent: 11 } }, + lastUpdated: new Date().toISOString(), + }; + bundledSnapshot = { claude: status }; + bundledSnapshotsByAlias = { 'claude-secondary': status }; + + const provider = new AliasSpecificAgentTankSnapshotProvider(); + + // Without provenance the name check alone would pass here, because the + // bundled id is pinned to the provider key. + assert.equal(await provider.getSnapshot('claude'), null); +}); + +test('alias-specific capacity is reported for the account that was actually inspected', async () => { + mode = 'bundled'; + bundledSnapshotsByAlias = { + claude: { + name: 'claude', + usage: { session: { percent: 5 }, weekly: { percent: 11 } }, + lastUpdated: new Date().toISOString(), + }, + }; + + const snapshot = await new AliasSpecificAgentTankSnapshotProvider().getSnapshot('claude'); + + assert.equal(snapshot?.directAgentAlias, 'claude'); + assert.equal(snapshot?.sessionPercent, 5); + assert.equal(snapshot?.weeklyPercent, 11); +}); + +test('an alias-specific read in external mode still asks the daemon by name', async () => { + mode = 'external'; + + await getStatusForAlias('antigravity'); + + assert.deepEqual(fetchCalls, ['http://0.0.0.0:3456/status/agy']); +}); From 8b74a88fb260fb7c9305f0acdc22f904ddc9d9d8 Mon Sep 17 00:00:00 2001 From: "propr-dev[bot]" <1316198+propr-dev[bot]@users.noreply.github.com> Date: Sun, 27 Sep 2026 00:21:49 +0000 Subject: [PATCH 06/22] feat(ai): Both selected findings are fixed, each with a regression that fails without the fix. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both selected findings are fixed, each with a regression that fails without the fix. **F4 — generated config was unreachable by the Docker daemon** (`packages/core/src/services/agentTankBundledRunner.ts`) The runner wrote its config under `os.tmpdir()` inside the backend container and passed that backend-local pathname as a bind source. With the host daemon resolving `-v` sources on the host, the agent container got an empty directory instead of the config. The config is now materialized inside the container: it travels in the `docker run` as `PROPR_AGENT_TANK_CONFIG` (provider keys and container paths only — no secrets), and a fixed bootstrap script writes it at `/tmp/propr-agent-tank/config.json` before `exec agent-tank …`. The config path is passed as `$1` rather than interpolated into the script. No temp file exists on the backend, so cleanup is inherent to `--rm` (the `mkdtemp`/`chmod`/`rmSync` block is gone). Sibling check: the only other bind sources in the run are the credential directories, which already go through the deployment host mapping (`resolveConfigPath` / `resolveCodexConfigPath`); the diff contains no other generated-file bind source. Regressions in `test/agentTankBundledRunner.test.ts`: one asserts the run's `-v` list contains exactly the two credential mounts and nothing referencing the config path; one executes the bootstrap script the way the container would (final `exec` swapped for a no-op) and asserts it reproduces the generated config byte for byte at a nested target path. **F5 — alias response kept the provider name** (`packages/core/src/services/agentTankService.ts`) `getStatusForAlias` verified bundled provenance and then returned a status still named `claude`, which `AliasSpecificAgentTankSnapshotProvider`'s exact-name contract rejected for a custom alias like `claude-secondary`. It now returns a copy renamed to the requested alias (the cached snapshot is left untouched; a test asserts that). Rejection for uninspected aliases is unchanged, and external mode still uses the daemon's own per-name answer. Regressions in `test/agentTankService.test.ts`: the existing bundled-alias test now expects `claude-secondary` and asserts no in-place mutation, plus a new synthetic-provider test covering a successfully inspected custom alias; the existing withheld-capacity test still covers the uninspected alias. Verification: `test/agentTankBundledRunner.test.ts` + `test/agentTankService.test.ts` — 29/29 pass; reverting either source file individually fails exactly the new tests (3 and 2 respectively). `packages/core/test/syntheticRoutingService.test.ts`, `packages/api/test/configRoutesAgentTank.test.ts`, `test/agentTankSettingsMigration.test.ts`, `packages/api/test/dockerCommandSafety.test.ts` and `test/agentDockerfileSupplyChain.test.ts` also pass; `tsc --noEmit` on `packages/core` is clean. The changes are backend-only and not visually perceptible, so no previews were generated. PR: #2555 Comment by: @propr-ultrafix (ID: 0) Model: claude-opus-5 --- .../src/services/agentTankBundledRunner.ts | 59 +++++++------ .../core/src/services/agentTankService.ts | 13 ++- .../syntheticUsageSnapshotProvider.ts | 4 +- test/agentTankBundledRunner.test.ts | 86 ++++++++++++------- test/agentTankService.test.ts | 27 +++++- 5 files changed, 129 insertions(+), 60 deletions(-) diff --git a/packages/core/src/services/agentTankBundledRunner.ts b/packages/core/src/services/agentTankBundledRunner.ts index 3a0e02d7e..77c3533ca 100644 --- a/packages/core/src/services/agentTankBundledRunner.ts +++ b/packages/core/src/services/agentTankBundledRunner.ts @@ -12,8 +12,6 @@ */ import fs from 'node:fs'; -import os from 'node:os'; -import path from 'node:path'; import { randomBytes } from 'node:crypto'; import logger from '../utils/logger.js'; import { executeDockerCommand } from '../claude/docker/dockerExecutor.js'; @@ -43,6 +41,33 @@ const DELTA_FRESHNESS_MS = 90_000; const CONTAINER_CONFIG_FILE = '/tmp/propr-agent-tank/config.json'; +/** Carries the generated config into the container (see `CONFIG_BOOTSTRAP`). */ +const CONFIG_ENV_VAR = 'PROPR_AGENT_TANK_CONFIG'; + +/** + * Materialize the generated config *inside* the container instead of + * bind-mounting it from this process's filesystem. + * + * The backend normally runs in its own container and drives the host Docker + * daemon, so a backend-local pathname is not a usable bind source: the daemon + * resolves `-v` sources on the host, where the generated file does not exist, + * and would hand Agent Tank an empty directory instead of its config. No file + * mode or directory permission can bridge two filesystem namespaces. Every + * other mount in the run is a credential directory whose path already went + * through the deployment's host mapping (`resolveConfigPath` / + * `resolveCodexConfigPath`); the generated config has no such mapping, so it + * travels in the run itself and the container writes it as the user that reads + * it. The config holds provider keys and container paths only - no secrets - so + * an environment variable is a safe carrier. The `--rm` container takes the + * file with it, so there is nothing host-side left to clean up. + */ +const CONFIG_BOOTSTRAP = [ + 'set -e', + 'mkdir -p "$(dirname "$1")"', + `printf %s "$${CONFIG_ENV_VAR}" > "$1"`, + 'exec agent-tank --once --json --config "$1"', +].join('; '); + /** * Agent Tank only knows these three providers (`SUPPORTED_PROVIDERS` upstream). * OpenCode and Vibe have no usage endpoint to read, so including them would @@ -248,7 +273,6 @@ export async function canRunBundledAgentTank(): Promise { } async function runBundledAgentTank(): Promise { - let configDir: string | undefined; try { const { mounts, entries } = await collectBundledAgents(); if (entries.length === 0) { @@ -259,22 +283,6 @@ async function runBundledAgentTank(): Promise { const image = await resolveAgentImage(); - configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'propr-agent-tank-')); - const configFile = path.join(configDir, 'config.json'); - // The container runs Agent Tank as `node`, while this backend process may - // be any other uid (root in most deployments). A bind mount preserves the - // host owner and mode, so an owner-only file would be unreadable inside - // the container. The config holds provider names and container paths - - // no secrets - so it is made world-readable; `chmod` after the write - // because `writeFileSync`'s mode is still subject to the umask. The - // mount stays `:ro`, which is what keeps Agent Tank from rewriting it. - fs.writeFileSync(configFile, buildBundledAgentTankConfig(entries), { mode: 0o444 }); - fs.chmodSync(configFile, 0o444); - // mkdtemp creates the directory 0700; the daemon resolves the bind source - // path itself, so this only matters for rootless/userns daemons that do - // it as a non-root user. - fs.chmodSync(configDir, 0o755); - const result = await executeDockerCommand('docker', [ 'run', '--rm', // No inbound/outbound needs beyond the provider APIs the CLIs call; @@ -282,10 +290,13 @@ async function runBundledAgentTank(): Promise { // hits the provider API. '--name', `propr-agent-tank-${randomBytes(6).toString('hex')}`, '-e', 'PROPR_AGENT_TYPE=agent-tank', - '-v', `${configFile}:${CONTAINER_CONFIG_FILE}:ro`, + '-e', `${CONFIG_ENV_VAR}=${buildBundledAgentTankConfig(entries)}`, ...mounts, image, - 'agent-tank', '--once', '--json', '--config', CONTAINER_CONFIG_FILE, + // `sh -c