diff --git a/.claude/skills/verify-berthd/features/README.md b/.claude/skills/verify-berthd/features/README.md index 145887a52..19b517832 100644 --- a/.claude/skills/verify-berthd/features/README.md +++ b/.claude/skills/verify-berthd/features/README.md @@ -22,6 +22,7 @@ The maintained list of what a person can do in the app, and how to prove each on | Chat's foot and reply box: a live turn's spinner, timer, Stop and Queue; a turn that ended while its work runs on shows an hourglass and Send | [chat-turn-state.md](chat-turn-state.md) | `chat-turn-state`; `chat-turn-state-live` (a real box, `--agent`); `app/e2e/chat.spec.ts` asserts it | | Chat and board: no question card or answers but the options the agent's screen shows; the mode menu while turn check waits; a structured agent's form while its helpers report back, and a reply box that answers a card with nothing to pick | [chat-ask-card.md](chat-ask-card.md) | `chat-ask-card`, `chat-structured-ask-live` (a real box, `--agent`); `app/e2e/ask-card.spec.ts` on a stand-in box | | The board's cards for agents in chat: what they ask with its answers, their reply's last lines, the prompt they started with; never the host's frames or "stdio" | [chat-board-cards.md](chat-board-cards.md) | `chat-board-cards`; `app/e2e/dashboard-chat-cards.spec.ts` asserts it | +| A session's own variables: `--env KEY=VALUE` on `task new` and `session new`, `env` on `berth_task_new`; over the box's and the project's, through the agent host; berthd's own names and malformed ones refused | [session-env.md](session-env.md) | `scripts/session-env-live.sh` (a real box, real Claude), then `session-env-live` (`--agent`) | | Turn check's "working": an hourglass with why in the sidebar, the tab and the rail's card, not a live turn's spinner; a handoff reads as done | [turn-check-indicator.md](turn-check-indicator.md) | `turn-check-indicator`; `app/e2e/sidebar-turn-check.spec.ts` asserts it, `TestTurnCheckLive` asks the real provider | ## Proof and skip reporting diff --git a/.claude/skills/verify-berthd/features/session-env.md b/.claude/skills/verify-berthd/features/session-env.md new file mode 100644 index 000000000..5cbf15fde --- /dev/null +++ b/.claude/skills/verify-berthd/features/session-env.md @@ -0,0 +1,28 @@ +# A session's own variables: `--env KEY=VALUE` + +A task or session can be started with variables for it alone: `--env KEY=VALUE` (repeatable) on `berthd task new` and `berthd session new` (and `berth` for a box), `env` in the `POST /v1/tasks` and `POST /v1/sessions` bodies, and `env` (an object) on the `berth_task_new` MCP tool. They reach the agent's process in its terminal and through the agent host (a structured agent), and come over the same names from the box's `env.json` and the project's `env`. They last through a switch between the terminal and the chat: `@berth_env` names them on the tmux session, and `respawnAgent` leaves those names out of what it passes again (the worktree's values and the browser timeout's default), so the session's tmux environment supplies them. berthd's own names (`BERTH_SESSION`, `BERTH_PORT`, `BERTH_PORT_`, the worktree's `BERTH_*`, `BERTH_AGENT`, `BERTH_DRIVER`, and `BERTH_HOME`, `BERTH_USER_DIR`, `BERTH_OP`, which the secret wrapper trusts, …) and malformed ones (no `=`, no name, a name twice, a NUL in a value) are refused before anything is made (`internal/box/sessionenv.go`, `boxcmd` `envFlag`, `mcpserver` `envArg`). + +## Sub-features + +- `task-env`: `task new --env` sets them for the new worktree's agent. +- `session-env`: `session new --env` sets them for a session in an existing worktree, agent or command. +- `mcp-env`: `berth_task_new` with `env: {…}` does the same from an agent. +- `precedence`: session over project over box; names only one level sets come through. +- `refused`: a name berthd sets, or a malformed `--env`, is an error with a message, and no worktree or session is made. + +## How to get to it (user POV) + +- From a terminal or an agent on a box: `berthd task new acme/fix --agent claude --prompt … --env TICKET=T-7`. +- The app doesn't set them; it shows the agents started with them like any other. + +## Driving it + +- Go: `internal/box/sessionenv_test.go` (precedence over a real tmux, refusals with nothing made, a structured agent's pane keeping them across a respawn), `internal/boxcmd` `TestEnvFlags…`/`TestAMalformedEnvFlag…`, `internal/mcpserver` `TestATasksAgentGetsTheVariablesItWasGiven`. +- On a real box, with real Claude Code: `scripts/session-env-live.sh [OUT]` on a dev box of this checkout (`VERIFY_BOX_DIR=/tmp/vb-N scripts/dev-box.sh up`, every `BERTH_*` unset when you are an agent). It sets `ACME_*` in the box's env, the project's and each session's, starts Claude four ways (task new, session new, `berth_task_new` through `berthd mcp`, all structured; session new in its terminal) and has each one's Bash tool write `env | grep ^ACME_` to `OUT/.txt`, which it compares; then the two refusals, checking no session or worktree appeared. Evidence: the four files, each session's transcript, the terminal's screen, `refused-*.txt`. +- The chats in the app: then `node scripts/drive.mjs session-env-live --port $PORT --agent http://127.0.0.1:9378` opens `env-mcp`'s and `env-task`'s chats and asserts the Bash tool's output lines on screen (`scenarios/session-env-live.mjs`). + +## Gotchas + +- The terminal run needs Claude's folder-trust prompt answered: the script presses Down until `❯ … Yes, I trust` (or `Yes, proceed`) shows, then Enter, and runs Claude with `--dangerously-skip-permissions` so its Bash tool doesn't ask. +- The structured runs need the box's agent default on (`PUT agents/defaults {"structured":true,"mode":"bypassPermissions"}`); the script sets it, so `dev-box.sh`'s terminal default no longer holds after it. +- `env-task`'s worktree has two sessions (its chat and `env-term`'s terminal), so the scenario picks the chat's tab if the terminal opens first. diff --git a/.claude/skills/verify-berthd/scripts/scenarios/session-env-live.mjs b/.claude/skills/verify-berthd/scripts/scenarios/session-env-live.mjs new file mode 100644 index 000000000..7eb569f38 --- /dev/null +++ b/.claude/skills/verify-berthd/scripts/scenarios/session-env-live.mjs @@ -0,0 +1,38 @@ +// session-env-live: the chats of agents started with variables of their own +// (features/session-env.md), on the dev box ../session-env-live.sh left +// them on. Each chat shows Claude Code's Bash tool reading its own +// environment: the session's ACME_REGION and ACME_RUN over the project's +// and the box's, ACME_TICKET the project's over the box's, ACME_ACCOUNT the +// box's. +// +// node drive.mjs session-env-live --port $PORT --agent http://127.0.0.1:9378 +// +// The sessions are found on the box by where they run: env-mcp's worktree +// (berth_task_new) and env-task's (berthd task new). + +export const live = true; +export const params = { view: "conversation", sidebar: "list" }; + +const BOX = (process.env.VERIFY_WORKTREE ?? "devq/acme-shop").split("/")[0]; +const SAW = { + "env-mcp": ["ACME_ACCOUNT=box", "ACME_REGION=ap-south", "ACME_RUN=mcp-43", "ACME_TICKET=repo"], + "env-task": ["ACME_ACCOUNT=box", "ACME_REGION=eu-north", "ACME_RUN=task run 41", "ACME_TICKET=repo"], +}; + +export async function run({ page, shot, step, expect, call }) { + const sessions = await call(`/v1/boxes/${BOX}/api/sessions`); + const pane = page.locator("[data-testid=pane]:visible"); + const chat = pane.locator("[data-testid=chat]"); + for (const [wt, lines] of Object.entries(SAW)) { + const sess = sessions.find((s) => s.location === `acme-shop/${wt}` && s.driver === "claude"); + if (!sess) throw new Error(`no chat session in acme-shop/${wt}: ${sessions.map((s) => `${s.name} ${s.location} ${s.driver}`).join(", ")}`); + step(`${wt}: open ${sess.name}'s chat`); + await page.locator(`[data-testid=worktree-row][data-worktree="${BOX}/${wt}"]`).click({ position: { x: 32, y: 12 } }); + const tab = pane.getByRole("tab").filter({ hasText: /claude/i }); + if (await chat.count() === 0 && await tab.count() > 0) await tab.first().click(); + await expect(chat).toBeVisible({ timeout: 30_000 }); + for (const line of lines) await expect(chat.getByText(line, { exact: false }).first()).toBeVisible({ timeout: 30_000 }); + step(`${wt}: the chat shows ${lines.join(", ")}`); + await shot(`${wt}-chat`); + } +} diff --git a/.claude/skills/verify-berthd/scripts/session-env-live.sh b/.claude/skills/verify-berthd/scripts/session-env-live.sh new file mode 100755 index 000000000..33483bbbc --- /dev/null +++ b/.claude/skills/verify-berthd/scripts/session-env-live.sh @@ -0,0 +1,102 @@ +#!/usr/bin/env bash +# session-env-live.sh [OUT]: a session's own variables (`--env KEY=VALUE`, +# features/session-env.md) on a dev box of this checkout (dev-box.sh up), +# read back from inside Claude Code's Bash tool. +# +# It sets ACME_* at three levels: the box's env.json, the project's box-local +# env and the session's own. Then it starts Claude Code four ways, each +# with its own variables: +# env-task berthd task new --env, structured (the agent host) +# env-session berthd session new --env, structured +# env-mcp the berth_task_new MCP tool with env, structured +# env-term berthd session new --env -- claude …, in its terminal +# and each one's Bash tool writes `env | grep ^ACME_` to OUT/.txt, +# which must read: ACME_ACCOUNT=box (the box's), ACME_TICKET=repo (the +# project's over the box's), ACME_REGION=, ACME_RUN=. Then the refusals: a name berthd sets and a malformed --env, +# neither of which may start anything. +# +# Run it with VERIFY_BOX_DIR as dev-box.sh had it, and every BERTH_* +# variable unset when you are an agent yourself. Its sessions stay for the +# app scenario (session-env-live.mjs) and go with `dev-box.sh down`. +set -euo pipefail +root=$(git -C "$(dirname "$0")" rev-parse --show-toplevel) +dir=${VERIFY_BOX_DIR:-/tmp/verify-berthd-box} +out=${1:-/tmp/verify-berthd/session-env-$(date +%Y%m%d-%H%M%S)} +mkdir -p "$out" +[[ -f $dir/berthd.pid ]] || { echo "no dev box in $dir: dev-box.sh up first" >&2; exit 1; } +box() { env BERTH_HOME="$dir/box-home" BERTH_USER_DIR="$dir/box-user" TMUX_TMPDIR="$dir/tmux" "$root/bin/berthd" "$@"; } +tm() { env TMUX_TMPDIR="$dir/tmux" tmux -L berth "$@"; } +eval "$("$(dirname "$0")/dev-box.sh" env)" +api() { curl -sf -X "$1" -H "Authorization: Bearer $VERIFY_BERTH_TOKEN" -H 'content-type: application/json' "http://127.0.0.1:9378/v1/boxes/devq/api/$2" ${3:+-d "$3"}; } +fail() { echo "FAIL: $*" >&2; exit 1; } + +echo "== the box's and the project's variables" +api PUT env '{"env":{"ACME_ACCOUNT":"box","ACME_TICKET":"box","ACME_REGION":"box"}}' >/dev/null +api PUT locations/acme-shop/config '{"local":{"env":{"ACME_TICKET":"repo","ACME_REGION":"repo"}}}' >/dev/null +# The app's default: agents in chat, allowed what they ask. +api PUT agents/defaults '{"structured":true,"mode":"bypassPermissions"}' >/dev/null + +prompt() { + echo "Run exactly this one command with your Bash tool, nothing else: env | grep '^ACME_' | sort > $out/$1.txt && cat $out/$1.txt -- then reply with its output, verbatim." +} + +echo "== berthd task new --env (structured)" +box task new acme-shop/env-task --agent claude --no-notify --env ACME_REGION=eu-north --env "ACME_RUN=task run 41" --prompt "$(prompt env-task)" | tee "$out/cli.txt" + +echo "== berthd session new --env (structured)" +box session new acme-shop --name env-session --agent claude --env ACME_REGION=us-east --env ACME_RUN=session-42 --prompt "$(prompt env-session)" | tee -a "$out/cli.txt" + +echo "== berth_task_new with env (structured)" +args=$(jq -nc --arg p "$(prompt env-mcp)" '{location:"acme-shop",name:"env-mcp",agent:"claude",prompt:$p,notify:false,env:{ACME_REGION:"ap-south",ACME_RUN:"mcp-43"}}') +printf '%s\n' \ + '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-06-18"}}' \ + "{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"tools/call\",\"params\":{\"name\":\"berth_task_new\",\"arguments\":$args}}" | + box mcp | tail -1 | tee "$out/mcp.json" +jq -e '.result.isError != true' "$out/mcp.json" >/dev/null || fail "berth_task_new: $(cat "$out/mcp.json")" +echo + +echo "== berthd session new --env -- claude (its terminal)" +box session new acme-shop/env-task --name env-term --env ACME_REGION=eu-west --env ACME_RUN=term-44 -- claude --dangerously-skip-permissions "$(prompt env-term)" | tee -a "$out/cli.txt" +# Claude Code asks to trust a new folder before it reads the prompt. +for _ in $(seq 90); do + [[ -s $out/env-term.txt ]] && break + screen=$(box session screen env-term 2>/dev/null || true) + if grep -q '❯ .*Yes, I trust\|❯ .*Yes, proceed\|❯ .*Yes, I accept' <<<"$screen"; then + tm send-keys -t env-term Enter + elif grep -qi 'trust this folder\|Bypass Permissions mode' <<<"$screen"; then + tm send-keys -t env-term Down + fi + sleep 2 +done + +echo "== what each agent's Bash tool saw" +want() { printf 'ACME_ACCOUNT=box\nACME_REGION=%s\nACME_RUN=%s\nACME_TICKET=repo\n' "$1" "$2"; } +check() { + local name=$1 region=$2 run=$3 + for _ in $(seq 150); do [[ -s $out/$name.txt ]] && break; sleep 2; done + [[ -s $out/$name.txt ]] || fail "$name wrote nothing to $out/$name.txt in 5 minutes" + diff <(want "$region" "$run") "$out/$name.txt" || fail "$name saw the above" + echo "ok $name: $(tr '\n' ' ' <"$out/$name.txt")" +} +check env-task eu-north "task run 41" +check env-session us-east session-42 +check env-mcp ap-south mcp-43 +check env-term eu-west term-44 +box session screen env-term --history 60 >"$out/env-term.screen.txt" || true +for s in $(box sessions --json | jq -r '.[] | select(.name | test("env-(task|session|mcp)")) | .name'); do + api GET "sessions/$s/transcript" >"$out/$s.transcript.json" || true +done +box sessions --json | jq -r '.[] | select(.name | test("env-")) | "\(.name)\t\(.driver // "terminal")\t\(.location)"' | tee "$out/sessions.tsv" + +echo "== refusals: nothing starts" +before=$(box sessions --json | jq length) +if box task new acme-shop/env-refused --agent claude --env BERTH_SESSION=someone-else >"$out/refused-reserved.txt" 2>&1; then fail "BERTH_SESSION was accepted"; fi +grep -q "BERTH_SESSION is set by berthd" "$out/refused-reserved.txt" || fail "refusal said: $(cat "$out/refused-reserved.txt")" +if box session new acme-shop --agent claude --env ACME_REGION >"$out/refused-malformed.txt" 2>&1; then fail "--env with no = was accepted"; fi +grep -q -- '--env takes KEY=VALUE, got "ACME_REGION" with no =' "$out/refused-malformed.txt" || fail "refusal said: $(cat "$out/refused-malformed.txt")" +if box locations --json | jq -e '.[] | select(.name == "acme-shop") | .worktrees[]? | select(.name == "env-refused")' >/dev/null; then fail "a refused task made its worktree"; fi +[[ $(box sessions --json | jq length) == "$before" ]] || fail "a refused request started a session" +cat "$out/refused-reserved.txt" "$out/refused-malformed.txt" +echo "ok: refused, nothing started" +echo "evidence: $out" diff --git a/docs/changelog.mdx b/docs/changelog.mdx index c8ecb8a05..999cbfb9d 100644 --- a/docs/changelog.mdx +++ b/docs/changelog.mdx @@ -9,6 +9,18 @@ upgrades with `berth upgrade BOX`, or by running the [install command](/getting-started/add-a-box#with-the-install-command) again. +## Unreleased + +- **Give one agent variables of its own.** `berthd task new` and + `berthd session new` (and `berth` for a box) take `--env KEY=VALUE`, as + many as you like, and the `berth_task_new` tool for agents takes `env`. + The variables reach that session alone, in its terminal or its chat, and + win over the same names from the box's `env.json` or the project's + `env`: a ticket for its hooks, a second Claude Code account + (`CLAUDE_CONFIG_DIR`), a different config for each agent you try. Names + Shipyard sets itself, such as `BERTH_SESSION` or `BERTH_PORT`, and + malformed ones are refused with a message, and nothing is started. + ## v0.3.31 - **Worktrees with submodules can be removed.** git refuses to remove any diff --git a/docs/guides/project-config.mdx b/docs/guides/project-config.mdx index f45e5693a..22add3bb1 100644 --- a/docs/guides/project-config.mdx +++ b/docs/guides/project-config.mdx @@ -214,6 +214,8 @@ berth location scripts devl/shop --clear A box can also give every worktree on it some environment, in `~/.berth/env.json` (`{"env": {"KEY": "value"}}`), under the project's own. +One agent or terminal can have [variables of its own](/reference/files#a-sessions-own-variables) +over both: `berthd task new … --env KEY=VALUE`. ## Next diff --git a/docs/guides/runs.mdx b/docs/guides/runs.mdx index 4b8cd8d3c..b62f23965 100644 --- a/docs/guides/runs.mdx +++ b/docs/guides/runs.mdx @@ -215,7 +215,7 @@ JSON: | --- | --- | | `berth_sessions` | name, agent, state, turn, where | | `berth_screen` | the last lines (at most 60) | -| `berth_task_new` | the new session and worktree | +| `berth_task_new` | the new session and worktree; `env` gives its agent [variables of its own](/reference/files#a-sessions-own-variables), such as a different config per agent you start | | `berth_send` | the turn (held until the agent is idle, by default) | | `berth_wait_turn` | the state after at most 90 s; if not ended, a cursor to call again with (for short waits) | | `berth_exec` | a detached run; with `wait` (at most 90 s) its exit code and failing tail | diff --git a/docs/reference/berthd.mdx b/docs/reference/berthd.mdx index 7ed1cb934..be1a414bb 100644 --- a/docs/reference/berthd.mdx +++ b/docs/reference/berthd.mdx @@ -71,8 +71,8 @@ asks the app to open the page as a browser tab beside the agent; the | --- | --- | | `berthd sessions [--json]` | List sessions | | `berthd agents [--json]` | Agent CLIs this box can start | -| `berthd task new LOC/NAME [--agent ID] [--prompt TEXT] [--open split\|tab] [--branch B] [--base REF] [--no-notify] [-- COMMAND...]` | A worktree with an agent (or a command) running in it; from an agent's session, that agent [hears back](/guides/orchestration#hearing-back) when its first turn ends, and the new worktree nests under that agent's | -| `berthd session new LOC[/WORKTREE] [--name N] [--agent ID [--prompt TEXT]] [--open split\|tab] [-- COMMAND...]` | Start an agent or a command (default: a shell) there | +| `berthd task new LOC/NAME [--agent ID] [--prompt TEXT] [--env KEY=VALUE]... [--open split\|tab] [--branch B] [--base REF] [--no-notify] [-- COMMAND...]` | A worktree with an agent (or a command) running in it; from an agent's session, that agent [hears back](/guides/orchestration#hearing-back) when its first turn ends, and the new worktree nests under that agent's. `--env` sets a variable [for this agent only](/reference/files#a-sessions-own-variables) | +| `berthd session new LOC[/WORKTREE] [--name N] [--agent ID [--prompt TEXT]] [--env KEY=VALUE]... [--open split\|tab] [-- COMMAND...]` | Start an agent or a command (default: a shell) there, `--env` as for `task new` | | `berthd session screen NAME [--history N]` | Print what the session shows | | `berthd session send NAME TEXT [--when now\|idle] [--force] [--idem KEY] [--no-enter] [--wait [--timeout 30m]] [--no-notify]` | Type a prompt into a session (or hold it until the agent is idle), and wait for its turn; without `--wait`, an agent sending it hears back when the turn ends | | `berthd session wait NAME [--turn ID] [--for finished,waiting] [--timeout 30m]` | Wait for a turn (from `session send`), or its agent's current one, to end | diff --git a/docs/reference/cli.mdx b/docs/reference/cli.mdx index 302ea2ed9..1a952934a 100644 --- a/docs/reference/cli.mdx +++ b/docs/reference/cli.mdx @@ -147,8 +147,8 @@ Who may be reviewed is read from GitHub with this laptop's `gh`, as you. See | --- | --- | | `berth sessions BOX [--json]` | List sessions | | `berth agents BOX [--json]` | Agent CLIs this box can start | -| `berth task new BOX/LOC/NAME [--agent ID] [--prompt TEXT] [--open split\|tab] [--branch B] [--base REF] [-- COMMAND...]` | A worktree with an agent (or a command) running in it | -| `berth session new BOX/LOC[/WORKTREE] [--name N] [--agent ID [--prompt TEXT]] [--open split\|tab] [-- COMMAND...]` | Start an agent or a command (default: a shell) there | +| `berth task new BOX/LOC/NAME [--agent ID] [--prompt TEXT] [--env KEY=VALUE]... [--open split\|tab] [--branch B] [--base REF] [-- COMMAND...]` | A worktree with an agent (or a command) running in it; `--env` sets a variable [for this agent only](/reference/files#a-sessions-own-variables) | +| `berth session new BOX/LOC[/WORKTREE] [--name N] [--agent ID [--prompt TEXT]] [--env KEY=VALUE]... [--open split\|tab] [-- COMMAND...]` | Start an agent or a command (default: a shell) there, `--env` as for `task new` | | `berth session screen BOX/NAME [--history N]` | Print what the session shows | | `berth session send BOX/NAME TEXT [--when now\|idle] [--force] [--idem KEY] [--no-enter] [--wait [--timeout 30m]] [--queue]` | Type a prompt into a session (or hold it until the agent is idle), and wait for its turn | | `berth session wait BOX/NAME [--turn ID] [--for finished,waiting] [--timeout 30m]` | Wait for a turn (from `session send`), or its agent's current one, to end | diff --git a/docs/reference/files.mdx b/docs/reference/files.mdx index 7d558d832..95519ab73 100644 --- a/docs/reference/files.mdx +++ b/docs/reference/files.mdx @@ -112,11 +112,44 @@ agents, `exec`, repository hooks, flow steps, services): | `BERTH_KIT_DIR` | Its kit's files, when it has a kit | | `BERTH_REVIEW`, `BERTH_REVIEW_REPO`, `BERTH_REVIEW_SHA`, `BERTH_WITHHELD` | In a [review](/guides/review-a-pr): the PR number, its repository, the commit checked out, and the names of the reviewer's own keys it does not get | | then | the box's `env.json`, the project's `env`, and resolved secrets (in a review, without the reviewer's own keys) | +| then | Terminals and agents: the session's [own variables](#a-sessions-own-variables), over all of these | | `PORT` | `$BERTH_PORT`, for servers that read `PORT`, unless the box or `env` sets it | | `AGENT_BROWSER_IDLE_TIMEOUT_MS` | Terminals and agents: `300000`, so an [agent-browser](/guides/agent-browser#agent-browser) left idle closes after 5 minutes, unless the box, `env` or berthd's environment sets it | | `ORCA_ROOT_PATH`, `ORCA_WORKTREE_PATH`, `ORCA_WORKSPACE_NAME` | Setup and archive scripts, for scripts written for Orca | | what a helper answers | Services only: before berthd starts one, each running [plugin's helper](/guides/plugins#attach-what-a-helper-adds-to-a-service) that asks to be told may set the variables its manifest names (`OTEL_*`, a `DATABASE_URL` through its proxy). Each helper is shown the project's own values, not another helper's answer. Never a `BERTH_` variable, and never one that names a secret | +### A session's own variables + +A terminal or agent can be started with variables of its own, for it alone: +`--env KEY=VALUE` on `task new` and `session new` (repeat it for more), `env` +in a `POST /v1/tasks` or `POST /v1/sessions` body, or `env` on the +`berth_task_new` [MCP tool](/guides/runs#from-an-agent-mcp), an object such +as `{"TICKET": "T-7"}`. + +```sh +# a ticket for the agent's hooks to report against +berthd task new acme/fix-login --agent claude --prompt "fix the login redirect" --env TICKET=T-7 + +# a second Claude Code account, for this agent only +berthd session new acme --agent claude --env CLAUDE_CONFIG_DIR=$HOME/.claude-second +``` + +- They come over the box's `env.json` and the project's `env`: a variable + set both ways has the session's value. +- They are taken as given: `$VAR` in one is not expanded, and a value + shaped like a secret reference is not resolved. Like the rest of a + session's environment, they are passed to tmux on its command line, so + keep secrets in `env` as [references](/guides/secrets). +- They last as long as the session, through a switch between its terminal + and the chat. A conversation reopened from history, a fork, or a run's + agent starts without them. +- A name berthd sets itself (`BERTH_SESSION`, `BERTH_PORT`, `BERTH_PORT_1`, + the rest of the table above, `BERTH_AGENT`, `BERTH_DRIVER`, + `BERTH_SECRET_VARS`), or one that says where berthd keeps its state or + which `op` it runs (`BERTH_HOME`, `BERTH_USER_DIR`, `BERTH_OP`), is refused, and so is a malformed one (`--env KEY` + with no `=`, an empty name, the same name twice): nothing is started. + Other names, `BERTH_` ones included, are the session's to use. + ### What a helper gets A [plugin's helper](/guides/plugins#give-a-plugin-a-helper) runs with a small diff --git a/internal/box/api.go b/internal/box/api.go index 208ab006c..154bb5a53 100644 --- a/internal/box/api.go +++ b/internal/box/api.go @@ -702,6 +702,9 @@ type SessionRequest struct { // what it asks. Driver string `json:"driver,omitempty"` Mode string `json:"mode,omitempty"` + // Env is the session's own variables, over the box's and the + // project's (sessionenv.go). berthd's own can't be among them. + Env map[string]string `json:"env,omitempty"` } func (b *Box) addSession(w http.ResponseWriter, r *http.Request) error { @@ -715,6 +718,9 @@ func (b *Box) addSession(w http.ResponseWriter, r *http.Request) error { if req.Open != "" && req.Open != "split" && req.Open != "tab" { return badRequest("open must be split or tab") } + if err := CheckSessionEnv(req.Env); err != nil { + return err + } if req.Home { return b.addHomeSession(w, r, req) } @@ -746,7 +752,7 @@ func (b *Box) addSession(w http.ResponseWriter, r *http.Request) error { if req.Name == "" { req.Name = defaultSessionName(req.Location, p.Command) } - sess, err := b.startStructuredSession(r, req.Name, req.Location, dir, req.Agent, l, req.Prompt) + sess, err := b.startStructuredSession(r, req.Name, req.Location, dir, req.Agent, l, req.Prompt, req.Env) if err != nil { return err } @@ -767,7 +773,7 @@ func (b *Box) addSession(w http.ResponseWriter, r *http.Request) error { req.Name = defaultSessionName(req.Location, req.Command) } preset := req.Agent - sess, err := b.startSession(r, req.Name, req.Location, dir, req.Command, preset, preset != "" && req.Prompt != "") + sess, err := b.startSession(r, req.Name, req.Location, dir, req.Command, preset, preset != "" && req.Prompt != "", req.Env) if err != nil { return err } @@ -795,7 +801,7 @@ func (b *Box) addHomeSession(w http.ResponseWriter, r *http.Request, req Session if req.Name == "" { req.Name = defaultSessionName("home", req.Command) } - sess, err := b.startSession(r, req.Name, "", dir, req.Command, "", false) + sess, err := b.startSession(r, req.Name, "", dir, req.Command, "", false, req.Env) if err != nil { return err } @@ -815,14 +821,14 @@ func (b *Box) announceOpen(r *http.Request, sess Session, open string) { } // startSession runs command in dir once the hooks allow it; preset is the -// agent preset it runs, if any, and prompted says its command carries a -// first prompt. -func (b *Box) startSession(r *http.Request, name, location, dir, command, preset string, prompted bool) (Session, error) { +// agent preset it runs, if any, prompted says its command carries a first +// prompt, and own is the session's own variables. +func (b *Box) startSession(r *http.Request, name, location, dir, command, preset string, prompted bool, own map[string]string) (Session, error) { data := map[string]any{"name": name, "location": location, "path": dir, "command": command} if err := b.before(r, "session.start", data); err != nil { return Session{}, err } - sess, err := b.createAgentSession(r.Context(), name, location, dir, command, preset) + sess, err := b.createAgentSessionWith(r.Context(), name, location, dir, command, preset, nil, nil, own) if err != nil { return Session{}, err } @@ -881,12 +887,12 @@ func (b *Box) launchStructured(kind string, p AgentPreset, model, effort, mode s // startStructuredSession starts a structured agent (claudedriver.go) and // gives it its first prompt once its host listens. -func (b *Box) startStructuredSession(r *http.Request, name, location, dir, preset string, l structuredLaunch, prompt string) (Session, error) { +func (b *Box) startStructuredSession(r *http.Request, name, location, dir, preset string, l structuredLaunch, prompt string, own map[string]string) (Session, error) { data := map[string]any{"name": name, "location": location, "path": dir, "command": l.command} if err := b.before(r, "session.start", data); err != nil { return Session{}, err } - sess, err := b.createAgentSessionWith(r.Context(), name, location, dir, l.command, preset, l.env, l.labels) + sess, err := b.createAgentSessionWith(r.Context(), name, location, dir, l.command, preset, l.env, l.labels, own) if err != nil { return Session{}, err } diff --git a/internal/box/conversations.go b/internal/box/conversations.go index b9a627ada..e896e4304 100644 --- a/internal/box/conversations.go +++ b/internal/box/conversations.go @@ -343,7 +343,7 @@ func (b *Box) resumeConversation(w http.ResponseWriter, r *http.Request) error { return err } name := defaultSessionName(firstNonEmpty(conv.Location, filepath.Base(conv.Dir)), firstNonEmpty(conv.Agent, "claude")) - sess, err := b.startStructuredSession(r, name, conv.Location, conv.Dir, firstNonEmpty(conv.Agent, "claude"), l, "") + sess, err := b.startStructuredSession(r, name, conv.Location, conv.Dir, firstNonEmpty(conv.Agent, "claude"), l, "", nil) if err != nil { return err } diff --git a/internal/box/history.go b/internal/box/history.go index 638b8583a..f093b8040 100644 --- a/internal/box/history.go +++ b/internal/box/history.go @@ -194,7 +194,7 @@ func (b *Box) forkSession(w http.ResponseWriter, r *http.Request) error { if err != nil { return err } - ns, err := b.startSession(r, defaultSessionName(sess.Location, cmd), sess.Location, sess.Dir, cmd, "claude", strings.TrimSpace(req.Text) != "") + ns, err := b.startSession(r, defaultSessionName(sess.Location, cmd), sess.Location, sess.Dir, cmd, "claude", strings.TrimSpace(req.Text) != "", nil) if err != nil { return err } diff --git a/internal/box/repoconfig_test.go b/internal/box/repoconfig_test.go index ecd312ccc..46b2b96bd 100644 --- a/internal/box/repoconfig_test.go +++ b/internal/box/repoconfig_test.go @@ -206,7 +206,7 @@ func TestAWorktreesTerminalsGetItsPortAsPORT(t *testing.T) { t.Fatal(err) } envOf := func() map[string]string { - env, _ := b.sessionEnv(ctx, wt.Path) + env, _ := b.sessionEnv(ctx, wt.Path, nil) m := map[string]string{} for _, kv := range env { k, v, _ := strings.Cut(kv, "=") diff --git a/internal/box/runhost.go b/internal/box/runhost.go index abd4a0849..7fb69cb64 100644 --- a/internal/box/runhost.go +++ b/internal/box/runhost.go @@ -469,7 +469,7 @@ func (h *runHost) startStructuredAgent(ctx context.Context, x *runs.StepCtx, st if err := b.beforeAs(ctx, origOf(x), "session.start", map[string]any{"location": st.where, "path": st.dir, "command": l.command, "agent": st.agent}); err != nil { return fail(err) } - sess, err := b.createAgentSessionWith(ctx, st.name, st.where, st.dir, l.command, st.agent, l.env, l.labels) + sess, err := b.createAgentSessionWith(ctx, st.name, st.where, st.dir, l.command, st.agent, l.env, l.labels, nil) if err != nil { return fail(err) } diff --git a/internal/box/secrets.go b/internal/box/secrets.go index 68975d66f..e65248293 100644 --- a/internal/box/secrets.go +++ b/internal/box/secrets.go @@ -5,11 +5,13 @@ import ( "context" "errors" "fmt" + "maps" "net/http" "os" "os/exec" "path/filepath" "regexp" + "slices" "sort" "strings" "sync" @@ -623,17 +625,32 @@ func (b *Box) createSession(ctx context.Context, name, location, dir, command st // createAgentSession starts a session for the agent preset agent ("" for // a plain command). func (b *Box) createAgentSession(ctx context.Context, name, location, dir, command, agent string) (Session, error) { - return b.createAgentSessionWith(ctx, name, location, dir, command, agent, nil, nil) + return b.createAgentSessionWith(ctx, name, location, dir, command, agent, nil, nil, nil) } // createAgentSessionWith is createAgentSession with more environment and -// tmux options (a structured agent's: claudedriver.go). -func (b *Box) createAgentSessionWith(ctx context.Context, name, location, dir, command, agent string, extraEnv []string, labels map[string]string) (Session, error) { - env, wrap := b.sessionEnv(ctx, dir) +// tmux options (a structured agent's: claudedriver.go), and the session's +// own variables (sessionenv.go) over its worktree's. +func (b *Box) createAgentSessionWith(ctx context.Context, name, location, dir, command, agent string, extraEnv []string, labels map[string]string, own map[string]string) (Session, error) { + names := slices.Sorted(maps.Keys(own)) + env, wrap := b.sessionEnv(ctx, dir, names) + if len(own) > 0 { + labels = maps.Clone(labels) + if labels == nil { + labels = map[string]string{} + } + labels[labelOwnEnv] = strings.Join(names, ",") + for _, k := range names { + env = append(env, k+"="+own[k]) + } + } return b.Sessions.create(ctx, name, location, dir, command, agent, append(env, extraEnv...), wrap, labels) } -func (b *Box) sessionEnv(ctx context.Context, dir string) (env, wrap []string) { +// sessionEnv is the environment a session in dir starts with, its +// worktree's, and the wrapper that resolves its secrets, if it has any; +// without the variables named in own, which the session has of its own. +func (b *Box) sessionEnv(ctx context.Context, dir string, own []string) (env, wrap []string) { loc, wt, ok := b.worktreeAt(ctx, dir) if !ok { return nil, nil @@ -642,6 +659,10 @@ func (b *Box) sessionEnv(ctx context.Context, dir string) (env, wrap []string) { if err != nil { return nil, nil } + for _, k := range own { + delete(p.refs, k) + } + p.env = withoutVars(p.env, own) if len(p.refs) == 0 { return p.env, nil } diff --git a/internal/box/sessionenv.go b/internal/box/sessionenv.go new file mode 100644 index 000000000..45638d0f1 --- /dev/null +++ b/internal/box/sessionenv.go @@ -0,0 +1,77 @@ +package box + +import ( + "context" + "maps" + "regexp" + "slices" + "strings" +) + +// A session's own environment: variables one task or session is started +// with (`berthd task new --env K=V`), over the box's (env.json) and the +// project's (.berth/config.json). They are taken as given, never expanded, +// and last as long as the session, through a switch between its terminal +// and the chat. + +// labelOwnEnv names the variables a session was started with of its own, +// so running it again in its pane (switch.go) keeps them over the +// project's: only their names, as the values are in the session's tmux +// environment. +const labelOwnEnv = "@berth_env" + +// berthdVars are the variables berthd sets on a session to say where it is +// and who it is (repoconfig.go, sessions.go, the drivers), and those that +// say where berthd keeps its state and which op it trusts with the box's +// 1Password session (statefile, secrets.go), which `berthd secret exec` +// reads in the session: none a session can set itself. BERTH_PORT_1, _2, … +// are berthd's too. +var berthdVars = map[string]bool{ + "BERTH_HOME": true, "BERTH_USER_DIR": true, "BERTH_OP": true, + "BERTH_BOX": true, "BERTH_LOCATION": true, "BERTH_ROOT_PATH": true, + "BERTH_WORKTREE_PATH": true, "BERTH_WORKTREE_NAME": true, "BERTH_WORKTREE_SLUG": true, + "BERTH_BRANCH": true, "BERTH_KIT_DIR": true, "BERTH_URL": true, "BERTH_PORT": true, + "BERTH_BROWSER_PROXY": true, "PLAYWRIGHT_MCP_PROXY_SERVER": true, "PLAYWRIGHT_MCP_PROXY_BYPASS": true, + "BERTH_REVIEW": true, "BERTH_REVIEW_REPO": true, "BERTH_REVIEW_SHA": true, "BERTH_WITHHELD": true, + "BERTH_SESSION": true, "BERTH_AGENT": true, "BERTH_DRIVER": true, SecretVarsEnv: true, +} + +var berthdPort = regexp.MustCompile(`^BERTH_PORT_[0-9]+$`) + +// CheckSessionEnv says what is wrong with a session's own variables, if +// anything: each needs a name, a name berthd doesn't set, and a value an +// environment can hold. +func CheckSessionEnv(env map[string]string) error { + for _, k := range slices.Sorted(maps.Keys(env)) { + switch { + case k == "": + return badRequest("an environment variable needs a name: give KEY=VALUE") + case !envName.MatchString(k): + return badRequest("%q is not an environment variable name: use letters, digits and _, not starting with a digit", k) + case berthdVars[k] || berthdPort.MatchString(k): + return badRequest("%s is set by berthd for every session and can't be set for one", k) + case strings.ContainsRune(env[k], 0): + return badRequest("%s's value holds a NUL byte, which no environment can", k) + } + } + return nil +} + +// withoutVars is env without the variables named in names. +func withoutVars(env, names []string) []string { + return slices.DeleteFunc(env, func(kv string) bool { + k, _, _ := strings.Cut(kv, "=") + return slices.Contains(names, k) + }) +} + +// ownEnv names the variables session sess was started with of its own. +// Their values are in its tmux environment, which a pane run again there +// starts with. +func (b *Box) ownEnv(ctx context.Context, sess Session) []string { + v, err := b.Sessions.tmux(ctx, "show-options", "-v", "-q", "-t", "="+sess.Name+":", labelOwnEnv) + if names := strings.TrimSpace(string(v)); err == nil && names != "" { + return strings.Split(names, ",") + } + return nil +} diff --git a/internal/box/sessionenv_test.go b/internal/box/sessionenv_test.go new file mode 100644 index 000000000..65b27ee1a --- /dev/null +++ b/internal/box/sessionenv_test.go @@ -0,0 +1,193 @@ +package box + +import ( + "os" + "path/filepath" + "runtime" + "strconv" + "strings" + "testing" +) + +// A session's own variables come over the project's, which come over the +// box's; the rest of each still reaches it. +func TestASessionsOwnEnvironmentComesOverTheProjectsAndTheBoxs(t *testing.T) { + var bx *Box + envFile := filepath.Join(t.TempDir(), "env.json") + c, _ := servedBox(t, func(b *Box) { b.EnvFile = envFile; bx = b }) + call(t, c, "POST", "/v1/locations", "", map[string]string{"name": "acme", "path": gitRepo(t)}, nil) + if err := saveBoxEnv(envFile, BoxEnv{Env: map[string]string{"ACCOUNT": "box", "TICKET": "box", "REGION": "box"}}); err != nil { + t.Fatal(err) + } + if err := bx.Locations.SetLocalConfig("acme", RepoConfig{Env: map[string]string{"TICKET": "repo", "REGION": "repo"}}); err != nil { + t.Fatal(err) + } + stop := func(name string) { t.Cleanup(func() { call(t, c, "DELETE", "/v1/sessions/"+name, "", nil, nil) }) } + want := map[string]string{"ACCOUNT": "box", "TICKET": "repo", "REGION": "eu-north", "RUN": "r-41 with spaces"} + check := func(what string, sess Session) { + t.Helper() + for k, v := range want { + if got := bx.Sessions.EnvVar(t.Context(), sess, k); got != v { + t.Errorf("%s: %s = %q, want %q", what, k, got, v) + } + } + } + own := map[string]string{"REGION": "eu-north", "RUN": "r-41 with spaces"} + + var task Task + if st := call(t, c, "POST", "/v1/tasks", "", TaskRequest{Location: "acme", Name: "billing", Command: "sleep 30", Env: own}, &task); st != 200 { + t.Fatalf("task: %d", st) + } + stop(task.Session.Name) + check("task new", task.Session) + + var sess Session + if st := call(t, c, "POST", "/v1/sessions", "", SessionRequest{Name: "own", Location: "acme/billing", Command: "sleep 30", Env: own}, &sess); st != 200 { + t.Fatalf("session: %d", st) + } + stop(sess.Name) + check("session new", sess) + + // Without any, a session starts as it always did. + var plain Session + if st := call(t, c, "POST", "/v1/sessions", "", SessionRequest{Name: "plain", Location: "acme/billing", Command: "sleep 30"}, &plain); st != 200 { + t.Fatalf("plain session: %d", st) + } + stop(plain.Name) + if got := bx.Sessions.EnvVar(t.Context(), plain, "REGION"); got != "repo" { + t.Fatalf("a session with no variables of its own has REGION=%q", got) + } +} + +// berthd's own variables say where a session is and who it is; a session +// can't claim others. Nothing is made for a request refused. +func TestASessionCantSetBerthdsOwnVariablesOrMalformedOnes(t *testing.T) { + c, _ := servedBox(t) + repo := gitRepo(t) + call(t, c, "POST", "/v1/locations", "", map[string]string{"name": "acme", "path": repo}, nil) + for _, tc := range []struct { + env map[string]string + says string + }{ + {map[string]string{"BERTH_SESSION": "someone-else"}, "BERTH_SESSION is set by berthd"}, + {map[string]string{"BERTH_PORT": "1"}, "BERTH_PORT is set by berthd"}, + {map[string]string{"BERTH_PORT_2": "1"}, "BERTH_PORT_2 is set by berthd"}, + {map[string]string{"BERTH_WORKTREE_PATH": "/"}, "BERTH_WORKTREE_PATH is set by berthd"}, + {map[string]string{"BERTH_DRIVER": ""}, "BERTH_DRIVER is set by berthd"}, + {map[string]string{"BERTH_OP": "/tmp/not-op"}, "BERTH_OP is set by berthd"}, + {map[string]string{"BERTH_HOME": "/tmp/elsewhere"}, "BERTH_HOME is set by berthd"}, + {map[string]string{"": "x"}, "an environment variable needs a name"}, + {map[string]string{"NOT A NAME": "x"}, `"NOT A NAME" is not an environment variable name`}, + {map[string]string{"NUL": "a\x00b"}, "NUL"}, + } { + var resp struct{ Error string } + if st := call(t, c, "POST", "/v1/tasks", "", TaskRequest{Location: "acme", Name: "refused", Command: "sleep 30", Env: tc.env}, &resp); st != 400 || !strings.Contains(resp.Error, tc.says) { + t.Errorf("task with %q: %d %q, want 400 saying %q", tc.env, st, resp.Error, tc.says) + } + resp.Error = "" + if st := call(t, c, "POST", "/v1/sessions", "", SessionRequest{Location: "acme", Command: "sleep 30", Env: tc.env}, &resp); st != 400 || !strings.Contains(resp.Error, tc.says) { + t.Errorf("session with %q: %d %q, want 400 saying %q", tc.env, st, resp.Error, tc.says) + } + } + if _, err := os.Stat(filepath.Join(filepath.Dir(repo), filepath.Base(repo)+"-refused")); err == nil { + t.Fatal("a refused task made its worktree") + } + var all []Session + call(t, c, "GET", "/v1/sessions", "", nil, &all) + if len(all) != 0 { + t.Fatalf("refused requests started %v", all) + } + // A name berthd doesn't set is the session's to use. + var sess Session + if st := call(t, c, "POST", "/v1/sessions", "", SessionRequest{Location: "acme", Command: "sleep 30", Env: map[string]string{"BERTH_TICKET": "T-7"}}, &sess); st != 200 { + t.Fatalf("BERTH_TICKET: %d", st) + } + call(t, c, "DELETE", "/v1/sessions/"+sess.Name, "", nil, nil) +} + +// Every BERTH_ variable berthd gives a worktree's sessions is one a +// session can't set: the list of berthd's own keeps up with the box. +func TestEveryVariableBerthdSetsIsRefusedForASession(t *testing.T) { + ctx := t.Context() + repo := gitRepo(t) + dir := t.TempDir() + b := &Box{Name: "devbox", Locations: NewLocations(filepath.Join(dir, "locations.json")), Sessions: testSessions(t)} + b.Locations.Add(ctx, "acme", repo) + if err := b.Locations.SetLocalConfig("acme", RepoConfig{Ports: 3}); err != nil { + t.Fatal(err) + } + wt, _ := b.Locations.CreateWorktree(ctx, "acme", "billing", "", "") + sess, err := b.createAgentSessionWith(ctx, "probe", "acme/billing", wt.Path, "sleep 30", "claude", []string{"BERTH_DRIVER=structured"}, nil, nil) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { b.Sessions.Kill(ctx, sess.Name) }) + out, err := b.Sessions.tmux(ctx, "show-environment", "-t", "="+sess.Name) + if err != nil { + t.Fatal(err) + } + seen := 0 + for _, line := range strings.Split(string(out), "\n") { + k, _, ok := strings.Cut(line, "=") + if !ok || !strings.HasPrefix(k, "BERTH_") { + continue + } + seen++ + if CheckSessionEnv(map[string]string{k: "x"}) == nil { + t.Errorf("berthd sets %s, yet a session may set it too", k) + } + } + if seen < 10 { + t.Fatalf("the session had %d BERTH_ variables: %s", seen, out) + } +} + +// A structured agent's session has them too, and a switch between its +// terminal and the chat, which runs it again in the same pane, keeps them +// over the project's. +func TestAStructuredAgentKeepsItsOwnVariablesAcrossASwitch(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("reads the pane's environment from /proc") + } + // berthd's own environment sets no browser timeout, so a switch has a + // default to give, which the session's own must win over. + t.Setenv(AgentBrowserIdleEnv, "") + os.Unsetenv(AgentBrowserIdleEnv) + c, bx := structuredBox(t) + if err := bx.Locations.SetLocalConfig("acme", RepoConfig{Env: map[string]string{"REGION": "repo"}}); err != nil { + t.Fatal(err) + } + var task Task + if st := call(t, c, "POST", "/v1/tasks", "", TaskRequest{Location: "acme", Name: "chat", Agent: "claude", Driver: DriverClaude, Prompt: "hello", Env: map[string]string{"REGION": "eu-north", "RUN": "r-41", AgentBrowserIdleEnv: "0"}}, &task); st != 200 { + t.Fatalf("task: %d", st) + } + t.Cleanup(func() { call(t, c, "DELETE", "/v1/sessions/"+task.Session.Name, "", nil, nil) }) + if task.Session.Driver != DriverClaude { + t.Fatalf("the agent ran %q", task.Session.Driver) + } + paneEnv := func() map[string]string { + t.Helper() + live, err := bx.Sessions.Get(t.Context(), task.Session.Name) + if err != nil { + t.Fatal(err) + } + raw, err := os.ReadFile("/proc/" + strconv.Itoa(live.panePID) + "/environ") + if err != nil { + t.Fatal(err) + } + return parseEnv(splitNul(raw)) + } + if env := paneEnv(); env["REGION"] != "eu-north" || env["RUN"] != "r-41" || env["BERTH_DRIVER"] != "structured" { + t.Fatalf("the chat's pane has REGION=%q RUN=%q BERTH_DRIVER=%q", env["REGION"], env["RUN"], env["BERTH_DRIVER"]) + } + live, err := bx.Sessions.Get(t.Context(), task.Session.Name) + if err != nil { + t.Fatal(err) + } + if err := bx.respawnAgent(t.Context(), live, "sleep 30", []string{"BERTH_DRIVER="}, nil); err != nil { + t.Fatal(err) + } + if env := paneEnv(); env["REGION"] != "eu-north" || env["RUN"] != "r-41" || env[AgentBrowserIdleEnv] != "0" { + t.Fatalf("after the switch the pane has REGION=%q RUN=%q %s=%q", env["REGION"], env["RUN"], AgentBrowserIdleEnv, env[AgentBrowserIdleEnv]) + } +} diff --git a/internal/box/switch.go b/internal/box/switch.go index e1ee84058..f2e61dd32 100644 --- a/internal/box/switch.go +++ b/internal/box/switch.go @@ -276,11 +276,13 @@ func lastSessionID(path string) string { // respawnAgent runs command in the session's pane in place of what ran // there, with env added to the session's own and labels set, as create // starts one (sessions.go): with its agent's CLI on PATH, which a login -// shell may otherwise lose (Debian's resets it). +// shell may otherwise lose (Debian's resets it). The variables the session +// was started with of its own stay over its worktree's. func (b *Box) respawnAgent(ctx context.Context, sess Session, command string, env []string, labels map[string]string) error { agent := firstNonEmpty(agentFor(sess), "claude") s := b.Sessions - extra, wrap := b.sessionEnv(ctx, sess.Dir) + own := b.ownEnv(ctx, sess) + extra, wrap := b.sessionEnv(ctx, sess.Dir, own) file, err := s.writeCommand(sess.Name, command) if err != nil { return err @@ -289,7 +291,8 @@ func (b *Box) respawnAgent(ctx context.Context, sess Session, command string, en argv := []string{shell, "-lc", withPATH(shell, launchPATH(command, agent), sourceCommand(shell, file))} target := "=" + sess.Name + ":" args := []string{"respawn-pane", "-k", "-t", target, "-c", sess.Dir} - for _, kv := range append(append(withAgentBrowserIdle(extra), "BERTH_SESSION="+sess.Name, "BERTH_AGENT="+agent), env...) { + // A default for one of the session's own would hide its value. + for _, kv := range append(append(withoutVars(withAgentBrowserIdle(extra), own), "BERTH_SESSION="+sess.Name, "BERTH_AGENT="+agent), env...) { args = append(args, "-e", kv) } args = append(append(args, "--"), groups.Wrap(append(append([]string(nil), wrap...), argv...))...) diff --git a/internal/box/tasks.go b/internal/box/tasks.go index 4488189f6..9cfe8e08e 100644 --- a/internal/box/tasks.go +++ b/internal/box/tasks.go @@ -279,6 +279,8 @@ type TaskRequest struct { Open string `json:"open,omitempty"` // Title names the work; without one, the prompt's first line does. Title string `json:"title,omitempty"` + // Env is the agent's own variables: see SessionRequest. + Env map[string]string `json:"env,omitempty"` } type Task struct { @@ -294,6 +296,9 @@ func (b *Box) addTask(w http.ResponseWriter, r *http.Request) error { if req.Location == "" || req.Name == "" { return badRequest("a task needs a location and a name") } + if err := CheckSessionEnv(req.Env); err != nil { + return err + } // Without tmux there is nothing to run the agent in: say so before // making a worktree that would only be removed again. if _, err := tmuxPath(); err != nil { @@ -349,9 +354,9 @@ func (b *Box) addTask(w http.ResponseWriter, r *http.Request) error { } var sess Session if launch != nil { - sess, err = b.startStructuredSession(r, defaultSessionName(where, command), where, wt.Path, preset, *launch, req.Prompt) + sess, err = b.startStructuredSession(r, defaultSessionName(where, command), where, wt.Path, preset, *launch, req.Prompt, req.Env) } else { - sess, err = b.startSession(r, defaultSessionName(where, command), where, wt.Path, command, preset, preset != "" && req.Prompt != "") + sess, err = b.startSession(r, defaultSessionName(where, command), where, wt.Path, command, preset, preset != "" && req.Prompt != "", req.Env) } if err != nil { // A task is a worktree with an agent in it: without the agent, the diff --git a/internal/boxcmd/boxcmd.go b/internal/boxcmd/boxcmd.go index b47c1e95e..2118d0d3a 100644 --- a/internal/boxcmd/boxcmd.go +++ b/internal/boxcmd/boxcmd.go @@ -53,8 +53,8 @@ var usageSections = []struct { {"Agent sessions", [][2]string{ {"%[1]s sessions%[3]s [--json]", "List sessions"}, {"%[1]s agents%[3]s [--json]", "Agent CLIs this box can start"}, - {"%[1]s task new %[2]sLOC/NAME [--agent ID] [--prompt TEXT] [--title T] [--open split|tab] [--branch B] [--base REF] [--no-notify] [-- COMMAND...]", "A worktree with an agent (or COMMAND) running in it; run from\nan agent, it hears back when the first turn ends"}, - {"%[1]s session new %[2]sLOC[/WORKTREE] [--name N] [--agent ID [--prompt TEXT]] [--title T] [--open split|tab] [-- COMMAND...]", "Start an agent or COMMAND (default: a shell) there"}, + {"%[1]s task new %[2]sLOC/NAME [--agent ID] [--prompt TEXT] [--title T] [--env KEY=VALUE]... [--open split|tab] [--branch B] [--base REF] [--no-notify] [-- COMMAND...]", "A worktree with an agent (or COMMAND) running in it; run from\nan agent, it hears back when the first turn ends. --env sets a\nvariable for this agent only, over the box's and the repo's"}, + {"%[1]s session new %[2]sLOC[/WORKTREE] [--name N] [--agent ID [--prompt TEXT]] [--title T] [--env KEY=VALUE]... [--open split|tab] [-- COMMAND...]", "Start an agent or COMMAND (default: a shell) there, --env as\nfor task new"}, {"%[1]s session rename %[2]sNAME [TITLE]", "Name a session's work (no TITLE clears it; a prompt names an untitled one)"}, {"%[1]s session screen %[2]sNAME [--history N]", "Print what the session shows"}, {"%[1]s session send %[2]sNAME TEXT [--when now|idle] [--force] [--idem KEY] [--no-enter] [--wait [--timeout 30m]] [--no-notify]%[4]s", "Type a prompt into a session (or hold it until the agent is idle), and wait for its turn"}, @@ -1033,9 +1033,13 @@ func sessionNew(ctx context.Context, c *box.Client, args []string, out io.Writer fs.StringVar(&req.Prompt, "prompt", "", "the agent's first prompt") fs.StringVar(&req.Open, "open", "", "show it in the Shipyard app: split (beside the current terminal) or tab") fs.StringVar(&req.Title, "title", "", "name the work (default: the prompt's first line)") + fs.Var((*envFlag)(&req.Env), "env", "set KEY=VALUE in the session's environment, over the box's and the project's (repeatable)") pos, err := parse(fs, args) + if err != nil && strings.Contains(err.Error(), envFlagErr) { + return err + } if err != nil || len(pos) != 1 { - return usageErr("session new LOC[/WORKTREE] [--name N] [--agent ID [--prompt TEXT]] [--title T] [--open split|tab] [-- COMMAND...]") + return usageErr("session new LOC[/WORKTREE] [--name N] [--agent ID [--prompt TEXT]] [--title T] [--env KEY=VALUE]... [--open split|tab] [-- COMMAND...]") } req.Location, req.Command = pos[0], commandLine(command) sess, err := c.StartSession(ctx, req) @@ -1047,6 +1051,32 @@ func sessionNew(ctx context.Context, c *box.Client, args []string, out io.Writer }) } +// envFlag is a repeatable --env KEY=VALUE. A malformed one is an error, +// never dropped; the box refuses the names it sets itself. +type envFlag map[string]string + +const envFlagErr = "--env takes KEY=VALUE" + +func (e *envFlag) String() string { return "" } + +func (e *envFlag) Set(kv string) error { + k, v, ok := strings.Cut(kv, "=") + switch { + case !ok: + return fmt.Errorf("%s, got %q with no =", envFlagErr, kv) + case k == "": + return fmt.Errorf("%s, got %q with no KEY", envFlagErr, kv) + } + if *e == nil { + *e = map[string]string{} + } + if _, dup := (*e)[k]; dup { + return fmt.Errorf("%s once per KEY, got %s twice", envFlagErr, k) + } + (*e)[k] = v + return nil +} + // commandLine turns the words after -- back into one shell command. A // single word is taken as a command line already, so `-- "a && b"` works; // several are quoted one by one, so `-- claude "fix it, don't stop"` gives @@ -1252,9 +1282,13 @@ func taskNew(ctx context.Context, c *box.Client, args []string, out io.Writer) e fs.StringVar(&req.Branch, "branch", "", "branch to create (default: the worktree name)") fs.StringVar(&req.Base, "base", "", "ref to branch from") fs.StringVar(&req.Title, "title", "", "name the work (default: the prompt's first line)") + fs.Var((*envFlag)(&req.Env), "env", "set KEY=VALUE in the agent's environment, over the box's and the project's (repeatable)") noNotify := fs.Bool("no-notify", false, "don't tell this agent (BERTH_SESSION) when the new agent's first turn ends") pos, err := parse(fs, args) - usage := "task new LOC/NAME [--agent ID] [--prompt TEXT] [--title T] [--open split|tab] [--branch B] [--base REF] [--no-notify] [-- COMMAND...]" + usage := "task new LOC/NAME [--agent ID] [--prompt TEXT] [--title T] [--env KEY=VALUE]... [--open split|tab] [--branch B] [--base REF] [--no-notify] [-- COMMAND...]" + if err != nil && strings.Contains(err.Error(), envFlagErr) { + return err + } if err != nil || len(pos) != 1 { return usageErr(usage) } diff --git a/internal/boxcmd/boxcmd_test.go b/internal/boxcmd/boxcmd_test.go index 223a5eaa8..1b2907386 100644 --- a/internal/boxcmd/boxcmd_test.go +++ b/internal/boxcmd/boxcmd_test.go @@ -70,6 +70,42 @@ func TestSessionNewPassesTheCommandAfterDoubleDash(t *testing.T) { } } +func TestEnvFlagsSetTheNewSessionsOwnVariables(t *testing.T) { + r, _ := run(t, `{"worktree":{"name":"fix"},"session":{"name":"s"}}`, "task", "new", "acme/fix", "--agent", "claude", "--env", "TICKET=T-7", "--env", "NOTE=a=b c", "--env", "EMPTY=", "--no-notify") + if env, _ := r.body["env"].(map[string]any); len(env) != 3 || env["TICKET"] != "T-7" || env["NOTE"] != "a=b c" || env["EMPTY"] != "" { + t.Fatalf("task body env = %v", r.body["env"]) + } + r, _ = run(t, `{"name":"s","dir":"/w"}`, "session", "new", "acme/fix", "--env", "CLAUDE_CONFIG_DIR=/home/acme/.claude-two", "--agent", "claude") + if env, _ := r.body["env"].(map[string]any); len(env) != 1 || env["CLAUDE_CONFIG_DIR"] != "/home/acme/.claude-two" { + t.Fatalf("session body env = %v", r.body["env"]) + } + r, _ = run(t, `{"name":"s","dir":"/w"}`, "session", "new", "acme/fix", "--", "make") + if _, ok := r.body["env"]; ok { + t.Fatalf("no --env sent %v", r.body["env"]) + } +} + +func TestAMalformedEnvFlagIsRefusedNeverDropped(t *testing.T) { + for _, tc := range []struct { + args []string + says string + }{ + {[]string{"task", "new", "acme/fix", "--env", "TICKET"}, `--env takes KEY=VALUE, got "TICKET" with no =`}, + {[]string{"task", "new", "acme/fix", "--env", "=T-7"}, `--env takes KEY=VALUE, got "=T-7" with no KEY`}, + {[]string{"task", "new", "acme/fix", "--env", "A=1", "--env", "A=2"}, "--env takes KEY=VALUE once per KEY, got A twice"}, + {[]string{"session", "new", "acme/fix", "--env", "TICKET"}, `--env takes KEY=VALUE, got "TICKET" with no =`}, + } { + r := &recorder{reply: "{}"} + err := Run(context.Background(), &box.Client{Doer: r}, tc.args, io.Discard) + if err == nil || !strings.Contains(err.Error(), tc.says) { + t.Errorf("%v: %v, want %q", tc.args, err, tc.says) + } + if r.method != "" { + t.Errorf("%v reached the box", tc.args) + } + } +} + func TestLocationAddAndRemoveWorktree(t *testing.T) { r, out := run(t, `{"name":"cal","path":"/home/alex/work/cal","repo":true,"worktrees":[{"name":"cal","main":true}]}`, "location", "add", "cal", "~/work/cal") if r.body["name"] != "cal" || r.body["path"] != "~/work/cal" || !strings.Contains(out, "git repository") { diff --git a/internal/integrations/skills/berth/SKILL.md b/internal/integrations/skills/berth/SKILL.md index 5a2770db5..6528c1ceb 100644 --- a/internal/integrations/skills/berth/SKILL.md +++ b/internal/integrations/skills/berth/SKILL.md @@ -68,6 +68,9 @@ berthd session screen shop-fix-login-claude-1a2b --history 200 it in the user's Shipyard app when they are looking at that worktree, and offers it otherwise. Use it whenever the user asks for an agent "beside me", "in a split" or "in a new tab". +- `--env KEY=VALUE` (repeatable) gives that one agent a variable, over the + box's and the repository's: `--env TICKET=T-7`. Names berth sets itself + (`BERTH_SESSION`, `BERTH_PORT`, …) are refused. - A new agent may stop at a question before it is ready, such as "do you trust this folder?". Wait with `berthd session wait NAME --for idle,waiting`: `idle` means it is at its prompt; `waiting` means it needs the user, so diff --git a/internal/mcpserver/mcp.go b/internal/mcpserver/mcp.go index 1e3b1fff8..729569500 100644 --- a/internal/mcpserver/mcp.go +++ b/internal/mcpserver/mcp.go @@ -135,7 +135,7 @@ const told = " Shipyard tells you when it ends, needs a person or hits a gate: a var Tools = []Tool{ {Name: "berth_sessions", Description: "List agent sessions on this box: name, agent, state, current turn.", InputSchema: obj(map[string]any{}), call: sessions}, {Name: "berth_screen", Description: "Read the last lines of a session's screen. Use only when an agent waits for someone.", InputSchema: obj(map[string]any{"session": str("session name"), "lines": num("lines of history, at most 60 (default 20)")}, "session"), call: screen}, - {Name: "berth_task_new", Description: "Make a worktree with an agent in it, working on prompt. Returns the session, worktree path and branch." + told, InputSchema: obj(with(map[string]any{"location": str("repository (location) name"), "name": str("new worktree name"), "agent": str("agent preset, e.g. claude or codex"), "prompt": str("its first prompt"), "base": str("base branch (optional)")}), "location", "name", "agent"), call: taskNew}, + {Name: "berth_task_new", Description: "Make a worktree with an agent in it, working on prompt. Returns the session, worktree path and branch." + told, InputSchema: obj(with(map[string]any{"location": str("repository (location) name"), "name": str("new worktree name"), "agent": str("agent preset, e.g. claude or codex"), "prompt": str("its first prompt"), "base": str("base branch (optional)"), "env": map[string]any{"type": "object", "additionalProperties": map[string]any{"type": "string"}, "description": "variables to set in the agent's environment, over the box's and the repository's (optional)"}}), "location", "name", "agent"), call: taskNew}, {Name: "berth_send", Description: "Send a prompt to a session (held until it is idle by default). Returns its turn." + told, InputSchema: obj(with(map[string]any{"session": str("session name"), "text": str("the prompt"), "when": str("idle (default) or now"), "idem": str("a key that makes a retry return the same turn")}), "session", "text"), call: send}, {Name: "berth_wait_turn", Description: "Wait up to timeout seconds (at most 90) for a turn to end. For short waits: work you start reports back by itself (a ), so rather than calling this in a loop, end your turn. If ended is false, call again with the cursor.", InputSchema: obj(map[string]any{"turn": str("turn ID, or the cursor from a previous call"), "timeout": num("seconds, at most 90 (default 60)")}, "turn"), call: waitTurn}, {Name: "berth_exec", Description: "Run a command in a worktree as a detached run; with wait (at most 90 s) return its result if done, else the run, which reports back (a ) when it ends.", InputSchema: obj(with(map[string]any{"location": str("location or location/worktree"), "command": str("shell command"), "wait": num("seconds to wait, at most 90 (default 0)")}), "location", "command"), call: execTool}, @@ -249,6 +249,31 @@ func seconds(a map[string]any, k string, def, max float64) time.Duration { return time.Duration(v * float64(time.Second)) } +// envArg is a[k] as environment variables: an object of names to string +// values. Anything else is an error, never dropped. +func envArg(a map[string]any, k string) (map[string]string, error) { + raw, ok := a[k] + if !ok || raw == nil { + return nil, nil + } + m, ok := raw.(map[string]any) + if !ok { + return nil, fmt.Errorf("%s must be an object of names to values, like {\"TICKET\": \"T-7\"}", k) + } + env := make(map[string]string, len(m)) + for name, v := range m { + s, ok := v.(string) + switch { + case name == "": + return nil, fmt.Errorf("%s: a variable needs a name", k) + case !ok: + return nil, fmt.Errorf("%s: %s's value must be a string", k, name) + } + env[name] = s + } + return env, nil +} + func need(a map[string]any, keys ...string) error { for _, k := range keys { if strings.TrimSpace(arg(a, k)) == "" { @@ -307,8 +332,12 @@ func taskNew(ctx context.Context, s *Server, a map[string]any) (any, error) { if err := need(a, "location", "name", "agent"); err != nil { return nil, err } + env, err := envArg(a, "env") + if err != nil { + return nil, err + } c, reports := s.forWork(ctx, a) - t, err := c.AddTask(ctx, box.TaskRequest{Location: arg(a, "location"), Name: arg(a, "name"), Agent: arg(a, "agent"), Prompt: arg(a, "prompt"), Base: arg(a, "base"), FromSession: s.Caller}) + t, err := c.AddTask(ctx, box.TaskRequest{Location: arg(a, "location"), Name: arg(a, "name"), Agent: arg(a, "agent"), Prompt: arg(a, "prompt"), Base: arg(a, "base"), FromSession: s.Caller, Env: env}) if err != nil { return nil, err } diff --git a/internal/mcpserver/mcp_test.go b/internal/mcpserver/mcp_test.go index d1f45276b..6d14881ea 100644 --- a/internal/mcpserver/mcp_test.go +++ b/internal/mcpserver/mcp_test.go @@ -102,3 +102,49 @@ func TestToolsAreShortAndNonBlocking(t *testing.T) { t.Fatal("an unknown method answered") } } + +// taskBox answers a task and keeps what it was asked for. +type taskBox struct { + tasks []box.TaskRequest +} + +func (f *taskBox) DoWithHeader(ctx context.Context, method, path string, body io.Reader, h http.Header) (*http.Response, error) { + w := httptest.NewRecorder() + if path != "/v1/tasks" { + w.WriteHeader(404) + w.WriteString(`{"error":"no"}`) + return w.Result(), nil + } + var req box.TaskRequest + json.NewDecoder(body).Decode(&req) + f.tasks = append(f.tasks, req) + w.WriteString(`{"worktree":{"name":"fix","path":"/w/fix","branch":"fix"},"session":{"name":"acme-fix-claude"}}`) + return w.Result(), nil +} + +func TestATasksAgentGetsTheVariablesItWasGiven(t *testing.T) { + f := &taskBox{} + res := rpc(t, &Server{Box: box.NewClient(f)}, + `{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"berth_task_new","arguments":{"location":"acme","name":"fix","agent":"claude","env":{"TICKET":"T-7","CLAUDE_CONFIG_DIR":"/home/acme/.claude-two"}}}}`, + `{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"berth_task_new","arguments":{"location":"acme","name":"fix","agent":"claude"}}}`, + `{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"berth_task_new","arguments":{"location":"acme","name":"fix","agent":"claude","env":["TICKET=T-7"]}}}`, + `{"jsonrpc":"2.0","id":4,"method":"tools/call","params":{"name":"berth_task_new","arguments":{"location":"acme","name":"fix","agent":"claude","env":{"TICKET":7}}}}`, + `{"jsonrpc":"2.0","id":5,"method":"tools/call","params":{"name":"berth_task_new","arguments":{"location":"acme","name":"fix","agent":"claude","env":{"":"T-7"}}}}`, + ) + if len(f.tasks) != 2 { + t.Fatalf("%d tasks reached the box, want 2: %+v", len(f.tasks), f.tasks) + } + if env := f.tasks[0].Env; len(env) != 2 || env["TICKET"] != "T-7" || env["CLAUDE_CONFIG_DIR"] != "/home/acme/.claude-two" { + t.Fatalf("env = %v", env) + } + if f.tasks[1].Env != nil { + t.Fatalf("no env sent %v", f.tasks[1].Env) + } + for i, says := range map[int]string{2: "env must be an object", 3: "TICKET's value must be a string", 4: "a variable needs a name"} { + r := res[i]["result"].(map[string]any) + text := r["content"].([]any)[0].(map[string]any)["text"].(string) + if r["isError"] != true || !strings.Contains(text, says) { + t.Errorf("call %d: %v %q, want an error saying %q", i+1, r["isError"], text, says) + } + } +}