From c83a6a474dbcd19539131f7ac73cf07f912e7405 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Mon, 21 Sep 2026 02:39:00 +0200 Subject: [PATCH] fix(container-gateway): make the macOS path work end to end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects, found while doctoring a sandbox where `podman` worked by hand but not through the gateway. Backend discovery trusted `podman machine inspect` for the socket path. That command renders the path from the *caller's* TMPDIR, not the machine's, so a hook, agent, or launchd service whose TMPDIR differs is told a path that does not exist and discovery reports no backend at all. Only the basename is stable, so the finders now return candidate lists and `discover` takes the first that exists, with the per-user temp directory from `getconf DARWIN_USER_TEMP_DIR` as the second podman candidate. `_docker_socket` gets the same shape: the context result and the `~/.docker` fallback are now both candidates rather than either/or. `CONTAINER_HOST` / `DOCKER_HOST` were committed in the baseline with a project-relative `unix://./…` value. That can never have worked: a `unix://` URL's authority is parsed as a host component, so `unix://./x` dials `/.//x`, `unix://x` dials `/x/`, and `unix:x` dials `//`. Only `unix:///abs/path` connects, and an absolute path is per-machine, so both variables move out of the committed baseline and join the `allowUnixSockets` pair in the gitignored `.claude/settings.local.json`. The spec had recorded this as a resolved open question in favour of the relative form, concluded from URL parsing rather than a connection; it now records what actually happens. The TMPDIR troubleshooting entry prescribed `env.TMPDIR` in project settings to get a per-project scratch directory. Claude Code sets TMPDIR itself when it builds the sandbox and that assignment wins, so the setting is accepted and silently does nothing. The entry now says so, and the doctor's probe 4 reports a shared session root as a pass instead of sending the operator after a fix that cannot work. Probe 3 gains an explicit check for a non-absolute value, because stripping `unix://` leaves a relative path that stats fine and hides the real cause. Also fixes an unrelated blocker found on the way: the touch overlay polled `lsof -U -n` before the O(1) `signing_in_flight` check, and on a loaded host that enumeration outlasts the signature it is meant to observe, so the overlay never appeared and its test failed one run in three. Asking the cheap question first keeps `lsof` out of the hot loop on the wrapped path entirely. Specs are synced to main: the two pr-management-code-review fixes (#1299, #1300) are recorded in the family contract, the GitHub Copilot harness from #1287 gets copilot-runtime.md, and .last-sync moves to the current tip. Generated-by: Claude Opus 5 --- .claude/settings.json | 4 - docs/mode-economics.md | 10 +- docs/setup/sandbox-troubleshooting.md | 72 +++++---- docs/setup/secure-agent-setup.md | 39 ++--- .../skills/isolated-setup-doctor/SKILL.md | 43 +++-- .../skills/isolated-setup-install/SKILL.md | 16 +- .../skills/isolated-setup-verify/SKILL.md | 14 +- tools/agent-isolation/gpg-touch-overlay.sh | 10 +- tools/container-gateway/README.md | 7 +- .../src/container_gateway/backends.py | 38 +++-- .../container-gateway/tests/test_backends.py | 18 +++ tools/sandbox-lint/expected.json | 4 - tools/sandbox-lint/tests/test_validator.py | 27 ++-- tools/skill-evals/README.md | 2 +- .../setup-isolated-setup-doctor/README.md | 15 +- .../fixtures/case-10-gateway-pass/report.md | 2 +- .../case-11-gateway-not-running/report.md | 2 +- .../case-12-gateway-socket-denied/report.md | 4 +- .../report.md | 2 +- .../expected.json | 1 + .../report.md | 12 ++ .../expected.json | 1 + .../report.md | 12 ++ tools/spec-loop/.last-sync | 2 +- tools/spec-loop/specs/README.md | 1 + tools/spec-loop/specs/container-gateway.md | 52 +++--- tools/spec-loop/specs/copilot-runtime.md | 150 ++++++++++++++++++ tools/spec-loop/specs/overview.md | 1 + tools/spec-loop/specs/pr-management-family.md | 22 +++ 29 files changed, 440 insertions(+), 143 deletions(-) create mode 100644 tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-14-scratch-shared-session-root/expected.json create mode 100644 tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-14-scratch-shared-session-root/report.md create mode 100644 tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-15-gateway-relative-container-host/expected.json create mode 100644 tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-15-gateway-relative-container-host/report.md create mode 100644 tools/spec-loop/specs/copilot-runtime.md diff --git a/.claude/settings.json b/.claude/settings.json index cccfaf15..6e621752 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -1,9 +1,5 @@ { "$schema": "https://json.schemastore.org/claude-code-settings.json", - "env": { - "CONTAINER_HOST": "unix://./.apache-magpie-local/run/podman.sock", - "DOCKER_HOST": "unix://./.apache-magpie-local/run/docker.sock" - }, "sandbox": { "enabled": true, "excludedCommands": [ diff --git a/docs/mode-economics.md b/docs/mode-economics.md index e5500453..0d9a958d 100644 --- a/docs/mode-economics.md +++ b/docs/mode-economics.md @@ -84,7 +84,7 @@ history separately provides its publication revision and date. -Measured on (UTC): 2026-09-20. +Measured on (UTC): 2026-09-21. Tokenizer: **tiktoken 0.14.0, `cl100k_base`**. Method: full UTF-8 file, including frontmatter and comments; line endings normalized to LF; @@ -92,7 +92,7 @@ special-token spellings counted as ordinary text. Coverage: **75 of 75 local `skills/*/SKILL.md` files**. External `source.md` redirects and harness symlinks are excluded. -Measurement manifest SHA-256: `f44def0e67cfebd4dceadc4b3253f16215740b0f611736356b3f0f300bb3b2f3`. +Measurement manifest SHA-256: `025b473aca2cddcd3304dfc5adbda35c972e51c0039ad790437528e3e047060c`. | Skill file | Measured tokens | Source SHA-256 (first 16 characters) | |---|---:|---| @@ -159,10 +159,10 @@ Measurement manifest SHA-256: `f44def0e67cfebd4dceadc4b3253f16215740b0f611736356 | [security-model-verify](../skills/security-model-verify/SKILL.md) | 6,625 | `cde155672857b33c` | | [security-tracker-stats-dashboard](../skills/security-tracker-stats-dashboard/SKILL.md) | 4,897 | `b52154deb8557ba4` | | [setup](../skills/setup/SKILL.md) | 8,724 | `82788542bb240309` | -| [setup-isolated-setup-doctor](../skills/setup-isolated-setup-doctor/SKILL.md) | 7,651 | `7485a409d69de376` | -| [setup-isolated-setup-install](../skills/setup-isolated-setup-install/SKILL.md) | 10,059 | `d2a1e98a9f2c3b38` | +| [setup-isolated-setup-doctor](../skills/setup-isolated-setup-doctor/SKILL.md) | 7,950 | `3fa5d728fa080ed0` | +| [setup-isolated-setup-install](../skills/setup-isolated-setup-install/SKILL.md) | 10,110 | `c32091305674dd2b` | | [setup-isolated-setup-update](../skills/setup-isolated-setup-update/SKILL.md) | 4,843 | `60bab0e30ac2e25a` | -| [setup-isolated-setup-verify](../skills/setup-isolated-setup-verify/SKILL.md) | 7,410 | `e5c4cd00e97147ad` | +| [setup-isolated-setup-verify](../skills/setup-isolated-setup-verify/SKILL.md) | 7,489 | `d7fa414b358d90ad` | | [setup-override-upstream](../skills/setup-override-upstream/SKILL.md) | 4,012 | `fb583feb56b7f77c` | | [setup-privacy-llm](../skills/setup-privacy-llm/SKILL.md) | 2,145 | `0e27b542a1656846` | | [setup-shared-config-sync](../skills/setup-shared-config-sync/SKILL.md) | 4,357 | `d1dfcd7cdeb5f5a6` | diff --git a/docs/setup/sandbox-troubleshooting.md b/docs/setup/sandbox-troubleshooting.md index 1e2005d8..6fbf445f 100644 --- a/docs/setup/sandbox-troubleshooting.md +++ b/docs/setup/sandbox-troubleshooting.md @@ -614,20 +614,25 @@ See [Container gateway](secure-agent-setup.md#container-gateway) in the setup gu | Error line | Cause | Action | |---|---|---| -| `failed to read identity "…/machine/machine": operation not permitted` | `CONTAINER_HOST` / `DOCKER_HOST` are unset, so the CLI fell back to its default connection instead of the gateway | Add the reference `env` block below to `.claude/settings.json` / `settings.local.json` | -| `dial unix ./.apache-magpie-local/run/podman.sock: connect: no such file or directory` | The gateway is not running for this project | Run `~/.claude/scripts/container-gateway-hook.sh start` from a terminal, or check `/.apache-magpie-local/run/container-gateway.log` for why it did not start | -| `dial unix ./.apache-magpie-local/run/podman.sock: connect: operation not permitted` | The gateway is running but its socket is missing from `sandbox.network.allowUnixSockets` | Add both gateway sockets as absolute paths, per [Container gateway](secure-agent-setup.md#container-gateway) | +| `failed to read identity "…/machine/machine": operation not permitted` | `CONTAINER_HOST` / `DOCKER_HOST` are unset, so the CLI fell back to its default connection instead of the gateway | Add the reference `env` block below to `.claude/settings.local.json` | +| `dial unix /.//.apache-magpie-local/run/podman.sock` — note the leading `/.//` | `CONTAINER_HOST` / `DOCKER_HOST` use a project-relative `unix://./…` value, which the CLIs do **not** resolve against the cwd | Use the absolute `unix:////…` spelling in the `env` block below | +| `dial unix //.apache-magpie-local/run/podman.sock: connect: no such file or directory` | The gateway is not running for this project | Run `~/.claude/scripts/container-gateway-hook.sh start` from a terminal, or check `/.apache-magpie-local/run/container-gateway.log` for why it did not start | +| `dial unix //.apache-magpie-local/run/podman.sock: connect: operation not permitted` | The gateway is running but its socket is missing from `sandbox.network.allowUnixSockets` | Add both gateway sockets as absolute paths, per [Container gateway](secure-agent-setup.md#container-gateway) | +| `no podman or docker backend found; nothing to serve` in the gateway log, while `podman` works by hand | On macOS the gateway asked `podman machine inspect` for the socket path, and that command renders it from the **caller's** `TMPDIR` | Update the framework: discovery now also probes `getconf DARWIN_USER_TEMP_DIR`/`podman/`, so a hook whose `TMPDIR` differs from the machine's still finds the socket | ```jsonc -// .claude/settings.json (already the framework's committed default on this branch) +// .claude/settings.local.json (gitignored, per machine — NOT committed) { "env": { - "CONTAINER_HOST": "unix://./.apache-magpie-local/run/podman.sock", - "DOCKER_HOST": "unix://./.apache-magpie-local/run/docker.sock" + "CONTAINER_HOST": "unix:////.apache-magpie-local/run/podman.sock", + "DOCKER_HOST": "unix:////.apache-magpie-local/run/docker.sock" } } ``` +A `unix://` URL's authority is parsed as a host component, so every relative spelling misses the socket — `unix://./x` dials `/.//x`, `unix://x` dials `/x/`, and `unix:x` dials `//`. +`unix:///absolute/path` is the only form that connects (verified against podman 6.1.0), which is why this block is per-machine rather than committed. + #### `403 container-gateway: …` A request that reaches the gateway but fails its policy comes back as `403`, and the CLI prints the message verbatim, for example `container-gateway: bind-mount: /Users/you/.ssh is outside the allowed roots (…); see docs/setup/sandbox-troubleshooting.md#docker--podman-command-fails-with-a-socket-error`. @@ -696,43 +701,50 @@ directory and can collide on identical filenames. ### Fix -Point `TMPDIR` at a per-project directory inside the writable tree, -in the **project's** `.claude/settings.local.json` — that file is -per-project, so the value is per-project by construction: +For the **unset / overwritten** and **outside `allowWrite`** cases +above, point `TMPDIR` back at a directory inside the writable tree +for whatever cleared it — the `env -i` wrapper, the Makefile, the +login shell — at that call site. `/tmp/claude-/` is already +inside the sandbox's writable set, so nothing needs widening. + +For the **shared-session-root** case there is currently **no fix**. +Setting `env.TMPDIR` in the project's `.claude/settings.local.json` +— which this entry recommended until recently — does not work: ```jsonc // /.claude/settings.local.json { "env": { - // is your numeric uid; is the project's - // absolute path with "/" replaced by "-". + // Accepted, and silently without effect. Do not rely on it. "TMPDIR": "/tmp/claude-//shared" } } ``` -Per-entry rationale: - -- `/tmp/claude-/` is already inside the sandbox's writable - set, so no `allowWrite` widening is needed — this entry costs - nothing in sandbox surface. -- `` matches the convention Claude Code already uses for - its own scratch tree, so the directory sits alongside the - session's existing state instead of introducing a second - location. -- Scoping to `settings.local.json` rather than user-scope - `settings.json` is what makes the value per-project. A - user-scope `TMPDIR` would be shared by every repo and would - reintroduce the collision mode. - -Create the directory before first use — a `TMPDIR` naming a -non-existent path fails the same way. +Claude Code sets `TMPDIR` itself when it builds the sandbox, to the +shared session root `/tmp/claude-`, and that assignment wins +over the settings value. The override is specific to `TMPDIR`: +other `env` keys from the same file do take effect, so a session +can show a live `CONTAINER_HOST` from project settings and a +`TMPDIR` that ignores them. The symptom of having tried is a +directory that exists, is named exactly as configured, and stays +empty for the life of the setting. + +In practice the collision risk this case describes is mostly +absorbed elsewhere: each session also gets its own scratchpad +under `/tmp/claude-///`, which is +per-project and per-session by construction. Prefer that for +anything a skill or tool writes; treat a bare `$TMPDIR` as shared +with every other project on the machine, and make temp filenames +unique rather than assuming the directory is yours. ### Notes -- **`env` is applied at session start.** The change does not take - effect in the session that makes it; restart, then confirm with - the doctor skill's *project-scratch* probe. +- **`env` is applied at session start.** For the keys that are + honored, a change does not take effect in the session that makes + it; restart, then confirm with the doctor skill's + *project-scratch* probe. `TMPDIR` is not one of those keys — see + the Fix above. - The scratch directory **cannot** be remapped onto literal `/tmp` inside the sandbox. `sandbox.filesystem.*` accepts allow / deny path lists only — there is no bind-mount or path-remap key. diff --git a/docs/setup/secure-agent-setup.md b/docs/setup/secure-agent-setup.md index 590a34e5..51db8e43 100644 --- a/docs/setup/secure-agent-setup.md +++ b/docs/setup/secure-agent-setup.md @@ -462,14 +462,13 @@ below, annotated. ```jsonc { - // The container gateway (tools/container-gateway) is where sandboxed - // podman / docker calls go. Both CLIs honour these variables; the - // sockets are project-relative, so this block is the same for every - // adopter. The gateway is started by the SessionStart hook below. - "env": { - "CONTAINER_HOST": "unix://./.apache-magpie-local/run/podman.sock", - "DOCKER_HOST": "unix://./.apache-magpie-local/run/docker.sock" - }, + // No `env` block here. Sandboxed podman / docker calls go through the + // container gateway (tools/container-gateway), but CONTAINER_HOST / + // DOCKER_HOST have to name its sockets by ABSOLUTE path — the CLIs do + // not resolve a project-relative `unix://./…` value against the cwd — + // and an absolute path is per-machine. So they live in the gitignored + // `.claude/settings.local.json` alongside the matching + // `allowUnixSockets` entries, not here. See "Container gateway" below. "sandbox": { "enabled": true, // `excludedCommands` runs the listed commands OUTSIDE the sandbox. @@ -2369,25 +2368,17 @@ Wire it as a `SessionStart` / `SessionEnd` pair in `~/.claude/settings.json`, al } ``` -The framework's own `.claude/settings.json` already carries the `env` half of the project-settings block, using project-relative `unix://` URLs so the same file works in every worktree: - -```jsonc -// .claude/settings.json (committed, project-wide) -{ - "env": { - "CONTAINER_HOST": "unix://./.apache-magpie-local/run/podman.sock", - "DOCKER_HOST": "unix://./.apache-magpie-local/run/docker.sock" - } -} -``` - -`allowUnixSockets` entries need an absolute path, which is per-machine, so they belong in the gitignored `.claude/settings.local.json` instead. -Add the block by hand, substituting your own project's absolute path for `` — nothing writes it for you. +Every setting that points something at a gateway socket needs that socket's **absolute** path, which is per-machine, so the whole project-settings block belongs in the gitignored `.claude/settings.local.json` — nothing is committed. +Add it by hand, substituting your own project's absolute path for `` — nothing writes it for you. (`setup-isolated-setup-install` Step L proposes the same block as a settings diff; `/magpie-setup config` does **not** write it, and automating it there is a recorded follow-up.) ```jsonc // .claude/settings.local.json (gitignored, per machine) { + "env": { + "CONTAINER_HOST": "unix:////.apache-magpie-local/run/podman.sock", + "DOCKER_HOST": "unix:////.apache-magpie-local/run/docker.sock" + }, "sandbox": { "network": { "allowUnixSockets": [ @@ -2399,6 +2390,10 @@ Add the block by hand, substituting your own project's absolute path for `/.apache-magpie-local/run/$rt.sock)" + continue ;; + esac sock="${url#unix://}" case "$(gw_state "$rt")" in not-running) @@ -367,12 +378,17 @@ per the catalog entry below. ### Probe 4 — Per-project scratch directory (`TMPDIR`) -Tests whether the session has a **writable, per-project** scratch -directory. The sandbox mounts the host `/tmp` read-only and punches -only specific subpaths writable, so a session whose `TMPDIR` falls -back to `/tmp` gets no scratch area at all, and one whose `TMPDIR` -points at the shared session root gets an area that collides with -every other project on the machine. +Tests whether the session has a **writable** scratch directory. The +sandbox mounts the host `/tmp` read-only and punches only specific +subpaths writable, so a session whose `TMPDIR` falls back to `/tmp` +gets no scratch area at all. + +`TMPDIR` landing on the shared session root rather than a +per-project directory is **not** a finding. Claude Code sets +`TMPDIR` itself when it builds the sandbox and that assignment wins +over `env.TMPDIR` from any settings file, so the shared root is the +expected value and no configuration changes it. Each session still +gets a per-project, per-session scratchpad underneath it. **Command:** @@ -388,7 +404,7 @@ else slug=$(pwd | sed 's|/|-|g') case "$TMPDIR" in *"$slug"*) echo "PROBE: project-scratch → ✓ (per-project + writable: $TMPDIR)" ;; - *) echo "PROBE: project-scratch → ⚠ (writable but shared across projects: $TMPDIR)" ;; + *) echo "PROBE: project-scratch → ✓ (writable; shared session root, which is the harness default: $TMPDIR)" ;; esac fi ``` @@ -398,16 +414,19 @@ fi | Result | Status | Meaning | |---|---|---| | `✓ per-project + writable` | Pass | `TMPDIR` resolves under this project's path slug and accepts writes. | -| `⚠ writable but shared across projects` | Warn | Scratch works, but every project on this machine shares it; concurrent sessions can collide on identical temp filenames. | +| `✓ writable; shared session root` | Pass | The expected value on current Claude Code. Every project on the machine shares this directory, so write through the per-session scratchpad beneath it, or use unique filenames — but there is nothing to fix. | | `✗ TMPDIR not set` | Fail | Tooling falls back to `/tmp`, which is read-only inside the sandbox. | -| `✗ directory missing` | Fail | `env.TMPDIR` names a path nothing has created yet. | +| `✗ directory missing` | Fail | `TMPDIR` names a path nothing has created yet. | | `✗ not writable inside sandbox` | Fail | `TMPDIR` points outside `sandbox.filesystem.allowWrite`. | -**On ✗ / ⚠ → remediation:** +**On ✗ → remediation:** [`docs/setup/sandbox-troubleshooting.md` — Temp files fail with "Read-only file system" under `/tmp`](../../../../docs/setup/sandbox-troubleshooting.md#temp-files-fail-with-read-only-file-system-under-tmp). -Note that `env` is applied at session start, so a fix does not take -effect in the session that makes it — restart before re-probing. +Do **not** propose `env.TMPDIR` in a settings file as the fix. +Claude Code overrides it when it builds the sandbox, so the setting +is accepted and silently has no effect; the giveaway is a directory +that exists, is named exactly as configured, and stays empty. The +catalog entry above covers what is actually actionable. ### Probe 5 — Signing key readable (`gpg.format=ssh`) diff --git a/plugins/magpie-setup/skills/isolated-setup-install/SKILL.md b/plugins/magpie-setup/skills/isolated-setup-install/SKILL.md index 7d3d05c9..b57bae7c 100644 --- a/plugins/magpie-setup/skills/isolated-setup-install/SKILL.md +++ b/plugins/magpie-setup/skills/isolated-setup-install/SKILL.md @@ -735,12 +735,16 @@ populated needs neither, since the hook falls back to merging into existing arrays with a diff the operator approves, exactly as for K.2's touch-overlay hooks. -**L.3 — Project wiring.** Propose the project `env` block -(`CONTAINER_HOST` / `DOCKER_HOST`, project-relative `unix://` URLs, -committed in `.claude/settings.json`) and the `allowUnixSockets` -pair (absolute paths, per-machine, in the gitignored -`.claude/settings.local.json`) as a single settings diff — the same -two-file split the setup guide documents. Never propose the real +**L.3 — Project wiring.** Propose the `env` block +(`CONTAINER_HOST` / `DOCKER_HOST`) and the `allowUnixSockets` pair +as a single settings diff into the gitignored +`.claude/settings.local.json`. All four values are **absolute** +paths and therefore per-machine: the CLIs do not resolve a +project-relative `unix://./…` value against the cwd — the URL +authority is read as a host component, so `unix://./x` dials +`/.//x` — and `allowUnixSockets` has no relative form either. +Nothing gateway-related is committed to `.claude/settings.json`. +Never propose the real daemon socket under any name; `tools/sandbox-lint` rejects an `allowUnixSockets` entry named `docker.sock` / `podman.sock` / `*-api.sock` outside `.apache-magpie-local/run/`. diff --git a/plugins/magpie-setup/skills/isolated-setup-verify/SKILL.md b/plugins/magpie-setup/skills/isolated-setup-verify/SKILL.md index fff6aaba..4c5945d8 100644 --- a/plugins/magpie-setup/skills/isolated-setup-verify/SKILL.md +++ b/plugins/magpie-setup/skills/isolated-setup-verify/SKILL.md @@ -449,14 +449,18 @@ Walk each in order: ✗. - **12b — hook script present.** `~/.claude/scripts/container-gateway-hook.sh` exists and is executable. Missing or non-executable is ✗. - - **12c — project wiring.** The project `.claude/settings.json` - or `.claude/settings.local.json` (check both — which file - carries the gateway entries depends on which install variant - the adopter chose) has `env.CONTAINER_HOST` and + - **12c — project wiring.** The gitignored + `.claude/settings.local.json` has `env.CONTAINER_HOST` and `env.DOCKER_HOST`, and both gateway sockets appear in `sandbox.network.allowUnixSockets`. Either half missing (the `env` pair or the socket allow-list pair) is ✗; report - which half. + which half. All four values must be **absolute** paths: the + CLIs read a `unix://` URL's authority as a host component, so + a project-relative `unix://./…` value dials a path that does + not exist. A relative `env` value — including one left in the + committed `.claude/settings.json` by an older install — is ✗, + quoting it and pointing at + [Container gateway](../../../../docs/setup/secure-agent-setup.md#container-gateway). - **12d — no raw daemon socket in any scope's `allowUnixSockets`.** Scan project, project-local, and user scope (`.claude/settings.json`, `.claude/settings.local.json`, diff --git a/tools/agent-isolation/gpg-touch-overlay.sh b/tools/agent-isolation/gpg-touch-overlay.sh index 21304800..9588e49d 100755 --- a/tools/agent-isolation/gpg-touch-overlay.sh +++ b/tools/agent-isolation/gpg-touch-overlay.sh @@ -632,8 +632,14 @@ _watch() { if [[ -n ${MAGPIE_GPG_TOUCH_PARENT:-} ]] && ! kill -0 "$MAGPIE_GPG_TOUCH_PARENT" 2>/dev/null; then break fi - rows="$(agent_socket_rows "$sockets")" - if { signing_in_flight || (( rows > baseline )); } && ! pinentry_up; then + # `signing_in_flight` is O(1) under `wrap` -- the watcher's own + # existence is the answer -- while `agent_socket_rows` shells out to + # `lsof -U -n`, which enumerates every unix socket on the machine and + # on a loaded host can take longer than the signature it is meant to + # observe. Asking the cheap question first keeps the expensive one out + # of the hot loop entirely on the wrapped path; polling it first cost + # the overlay the whole window on a busy machine, so it never appeared. + if { signing_in_flight || { rows="$(agent_socket_rows "$sockets")"; (( rows > baseline )); }; } && ! pinentry_up; then blocked=$(( blocked + 1 )) (( blocked >= SHOW_DELAY )) && show_overlay else diff --git a/tools/container-gateway/README.md b/tools/container-gateway/README.md index 0a2717a4..675c4ca2 100644 --- a/tools/container-gateway/README.md +++ b/tools/container-gateway/README.md @@ -55,10 +55,13 @@ It exits on `SessionEnd`, on `SIGTERM`, or after an idle timeout (default 4h) as ## Point the CLIs at it ```bash -export CONTAINER_HOST=unix://./.apache-magpie-local/run/podman.sock -export DOCKER_HOST=unix://./.apache-magpie-local/run/docker.sock +export CONTAINER_HOST=unix://$PWD/.apache-magpie-local/run/podman.sock +export DOCKER_HOST=unix://$PWD/.apache-magpie-local/run/docker.sock ``` +The path must be absolute. +A `unix://` URL's authority is parsed as a host component, so a project-relative spelling never reaches the socket: `unix://./x` dials `/.//x`, `unix://x` dials `/x/`, and `unix:x` dials `//`. + The podman CLI needs the libpod API and therefore only ever talks to a podman backend. The docker CLI talks to a docker backend when one exists, otherwise to podman's compat API. Persist these per-machine in `.claude/settings.local.json`'s `env` block, and allow the two sockets in `sandbox.network.allowUnixSockets`, never the real daemon socket. diff --git a/tools/container-gateway/src/container_gateway/backends.py b/tools/container-gateway/src/container_gateway/backends.py index ce07d80a..82863d13 100644 --- a/tools/container-gateway/src/container_gateway/backends.py +++ b/tools/container-gateway/src/container_gateway/backends.py @@ -49,22 +49,39 @@ def host_alias(kind: str, platform: str) -> str: return "10.88.0.1" if kind == "podman" else "172.17.0.1" -def _podman_socket(platform: str, env: Mapping[str, str], run: Runner) -> Path | None: +def _podman_socket(platform: str, env: Mapping[str, str], run: Runner) -> list[Path]: if platform == "Darwin": out = run(["podman", "machine", "inspect", "--format", "{{.ConnectionInfo.PodmanSocket.Path}}"]) - return Path(out) if out else None + if not out: + return [] + reported = Path(out) + # `podman machine inspect` builds that path from the *caller's* TMPDIR, + # not from the machine's. A caller whose TMPDIR differs from the one the + # machine was started under -- an agent, a hook, a launchd service -- is + # told a path that does not exist, and discovery finds no backend on a + # host where podman works fine by hand. Only the basename is stable, so + # also look for it under the per-user temp directory, which `getconf` + # reports regardless of the environment. + candidates = [reported] + user_temp = run(["getconf", "DARWIN_USER_TEMP_DIR"]) + if user_temp: + candidates.append(Path(user_temp, "podman", reported.name)) + return candidates runtime_dir = env.get("XDG_RUNTIME_DIR") - return Path(runtime_dir, "podman", "podman.sock") if runtime_dir else None + return [Path(runtime_dir, "podman", "podman.sock")] if runtime_dir else [] -def _docker_socket(platform: str, env: Mapping[str, str], run: Runner) -> Path | None: +def _docker_socket(platform: str, env: Mapping[str, str], run: Runner) -> list[Path]: if platform == "Darwin": + candidates = [] out = run(["docker", "context", "inspect", "--format", '{{(index .Endpoints "docker").Host}}']) if out and out.startswith("unix://"): - return Path(out[len("unix://") :]) + candidates.append(Path(out[len("unix://") :])) home = env.get("HOME") - return Path(home, ".docker", "run", "docker.sock") if home else None - return Path("/var/run/docker.sock") + if home: + candidates.append(Path(home, ".docker", "run", "docker.sock")) + return candidates + return [Path("/var/run/docker.sock")] def discover( @@ -78,9 +95,10 @@ def discover( for kind, finder in (("podman", _podman_socket), ("docker", _docker_socket)): if kind not in wanted: continue - sock = finder(platform, env, run) - if sock is not None and exists(sock): - found.append(Backend(kind, sock, host_alias(kind, platform))) + for sock in finder(platform, env, run): + if exists(sock): + found.append(Backend(kind, sock, host_alias(kind, platform))) + break return found diff --git a/tools/container-gateway/tests/test_backends.py b/tools/container-gateway/tests/test_backends.py index 64936963..a8e5c3d3 100644 --- a/tools/container-gateway/tests/test_backends.py +++ b/tools/container-gateway/tests/test_backends.py @@ -38,6 +38,24 @@ def run(argv: list[str]) -> str | None: assert found == [Backend("podman", sock, "host.containers.internal")] +def test_podman_machine_path_reported_under_a_foreign_tmpdir() -> None: + # `podman machine inspect` renders the socket path from the caller's TMPDIR, + # so a hook or agent with a different TMPDIR is told a path that does not + # exist. Discovery must still find the socket the machine really listens on. + reported = Path("/tmp/podman/podman-machine-default-api.sock") + real = Path("/var/folders/sm/xxx/T/podman/podman-machine-default-api.sock") + + def run(argv: list[str]) -> str | None: + if argv[:3] == ["podman", "machine", "inspect"]: + return str(reported) + if argv == ["getconf", "DARWIN_USER_TEMP_DIR"]: + return "/var/folders/sm/xxx/T/" + return None + + found = discover("Darwin", {}, run, lambda p: p == real, ALL) + assert found == [Backend("podman", real, "host.containers.internal")] + + def test_machine_socket_missing_means_no_backend() -> None: run = lambda argv: "/nope.sock" if argv[:2] == ["podman", "machine"] else None # noqa: E731 assert discover("Darwin", {}, run, lambda p: False, ALL) == [] diff --git a/tools/sandbox-lint/expected.json b/tools/sandbox-lint/expected.json index cccfaf15..6e621752 100644 --- a/tools/sandbox-lint/expected.json +++ b/tools/sandbox-lint/expected.json @@ -1,9 +1,5 @@ { "$schema": "https://json.schemastore.org/claude-code-settings.json", - "env": { - "CONTAINER_HOST": "unix://./.apache-magpie-local/run/podman.sock", - "DOCKER_HOST": "unix://./.apache-magpie-local/run/docker.sock" - }, "sandbox": { "enabled": true, "excludedCommands": [ diff --git a/tools/sandbox-lint/tests/test_validator.py b/tools/sandbox-lint/tests/test_validator.py index d16f6b52..172efcd9 100644 --- a/tools/sandbox-lint/tests/test_validator.py +++ b/tools/sandbox-lint/tests/test_validator.py @@ -388,25 +388,32 @@ def test_cli_exits_when_top_level_value_is_not_object(tmp_path: Path, baseline: # --------------------------------------------------------------------------- -# Container gateway: env vars route CONTAINER_HOST / DOCKER_HOST through the -# gateway; absolute allowUnixSockets entries for the gateway sockets are -# per-project, local settings (RELATIVE_SOCKETS=no), not committed here. +# Container gateway: every setting that points a CLI or a sandboxed Bash at a +# gateway socket needs that socket's ABSOLUTE path, which is per-machine, so +# none of them belongs in this committed baseline. # --------------------------------------------------------------------------- -def test_baseline_routes_containers_through_the_gateway(baseline: dict[str, Any]) -> None: +def test_baseline_carries_no_gateway_env(baseline: dict[str, Any]) -> None: + # CONTAINER_HOST / DOCKER_HOST used to be committed here with a + # project-relative "unix://./.apache-magpie-local/run/podman.sock" value, + # on the assumption that the CLIs resolve it against the cwd. They do not: + # podman parses the URL authority as a host component, so "unix://./x" + # dials "/.//x" and "unix://x" dials "/x/" -- no relative spelling reaches + # the socket. Only "unix:///abs/path" works, and an absolute path is + # per-machine, so both vars live in .claude/settings.local.json next to + # the allowUnixSockets entries below (see + # docs/setup/secure-agent-setup.md#container-gateway). env = baseline.get("env", {}) - assert env.get("CONTAINER_HOST") == "unix://./.apache-magpie-local/run/podman.sock" - assert env.get("DOCKER_HOST") == "unix://./.apache-magpie-local/run/docker.sock" + assert "CONTAINER_HOST" not in env + assert "DOCKER_HOST" not in env def test_baseline_has_no_gateway_socket_entries(baseline: dict[str, Any]) -> None: - # The committed reference (RELATIVE_SOCKETS=no) routes podman/docker - # through the project-relative env vars above only. The absolute + # Same reason as above, for the other half of the wiring: the absolute # allowUnixSockets entries a sandboxed Bash needs to connect(2) to the # gateway sockets are per-project, local settings -- written into - # .claude/settings.local.json by `/magpie-setup config`, never into this - # committed baseline (see docs/setup/secure-agent-setup.md). + # .claude/settings.local.json, never into this committed baseline. sockets = baseline["sandbox"]["network"].get("allowUnixSockets", []) assert not any(s.endswith("podman.sock") or s.endswith("docker.sock") for s in sockets) diff --git a/tools/skill-evals/README.md b/tools/skill-evals/README.md index ec5c11a2..0fd07eee 100644 --- a/tools/skill-evals/README.md +++ b/tools/skill-evals/README.md @@ -37,7 +37,7 @@ Suites are currently implemented for: - **list-skills** — 8 cases across 2 steps (step-1-command, step-2-present) - **setup-isolated-setup-verify** — 14 cases across 3 steps (runtime-routing, step-1-classify, step-2-recommend) - **setup-isolated-setup-update** — 15 cases across 4 steps (runtime-routing, step-snapshot-drift, step-tool-freshness, step-after-report) -- **setup-isolated-setup-doctor** — 20 cases across 3 steps (runtime-routing, interpret-probes, after-report) +- **setup-isolated-setup-doctor** — 22 cases across 3 steps (runtime-routing, interpret-probes, after-report) - **contributor-activity-sweep** — 12 cases across 3 steps (step-0-resolve-inputs, step-1-classify-reviews, step-2-render) - **optimize-skill** — 5 cases across 1 step (step-diagnose) - **committer-onboarding** — 27 cases across 4 steps (step-0-validate-vote, step-1-icla-comms, step-2-checklist, step-3-completion-summary) diff --git a/tools/skill-evals/evals/setup-isolated-setup-doctor/README.md b/tools/skill-evals/evals/setup-isolated-setup-doctor/README.md index 1c6c9199..e501e6e2 100644 --- a/tools/skill-evals/evals/setup-isolated-setup-doctor/README.md +++ b/tools/skill-evals/evals/setup-isolated-setup-doctor/README.md @@ -5,12 +5,12 @@ Behavioral evals for the `setup-isolated-setup-doctor` skill. -## Suites (20 cases total) +## Suites (22 cases total) | Suite | Step | Cases | What it covers | |---|---|---|---| | `runtime-routing` | Runtime routing | 2 | Codex and Gemini route to their native adapters and never require Claude files | -| `interpret-probes` | Probe interpretation (`## The 6 probes`) | 13 | all-pass, ssh-fail, localhost-fail, docker-skipped, multiple-fail, ssh-skipped-no-env, injection-in-probe-output, signing-key-fail, gh-sandbox-fail, container-gateway pass/not-running/socket-denied/no-backend | +| `interpret-probes` | Probe interpretation (`## The 6 probes`) | 15 | all-pass, ssh-fail, localhost-fail, docker-skipped, multiple-fail, ssh-skipped-no-env, injection-in-probe-output, signing-key-fail, gh-sandbox-fail, container-gateway pass/not-running/socket-denied/no-backend/relative-CONTAINER_HOST, scratch-on-shared-session-root | | `after-report` | Report synthesis (`## After the report`) | 5 | all-clear-all-pass, all-clear-with-skips, ssh-fail-with-catalog-link, multiple-fail-two-catalog-links, injection-asks-autofix-rejected | ## Run @@ -37,7 +37,7 @@ Given raw bash output from the three probe commands, the model classifies each probe as `pass`, `fail`, or `skip` and reports whether any failures were found. -The thirteen cases span: +The fifteen cases span: - **case-1-all-pass**: All three probes return ✓ lines. - **case-2-ssh-fail-unreachable**: SSH probe returns ✗ (rc=2, agent unreachable); the other two pass. @@ -76,6 +76,15 @@ The thirteen cases span: but `status` reports `serving` without `podman` — the Podman machine is stopped); `docker-runtime` ✓. Expected `docker_status: "fail"`, `has_failures: true`. +- **case-14-scratch-shared-session-root**: `project-scratch` ✓ with + `TMPDIR` on the shared session root rather than a per-project + directory. Expected `scratch_status: "pass"`, `has_failures: false` — + Claude Code sets `TMPDIR` when it builds the sandbox and overrides + `env.TMPDIR`, so the shared root is the harness default, not a finding. +- **case-15-gateway-relative-container-host**: `podman-runtime` ✗ + because `CONTAINER_HOST` uses a project-relative `unix://./…` value, + which the CLIs do not resolve against the cwd. Expected + `docker_status: "fail"`, `has_failures: true`. ### after-report diff --git a/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-10-gateway-pass/report.md b/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-10-gateway-pass/report.md index de03798c..3d68eb77 100644 --- a/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-10-gateway-pass/report.md +++ b/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-10-gateway-pass/report.md @@ -5,5 +5,5 @@ Probe output collected after wiring the container gateway hooks. PROBE: ssh-agent → ✓ (2 identities listed) PROBE: localhost-bind → ✓ (bound + loopback GET → HTTP 200, body=b'ok') -PROBE: podman-runtime → ✓ (podman reaches the container gateway at ./.apache-magpie-local/run/podman.sock) +PROBE: podman-runtime → ✓ (podman reaches the container gateway at /Users/alice/tracker/.apache-magpie-local/run/podman.sock) PROBE: docker-runtime → ⊘ (docker not on PATH) diff --git a/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-11-gateway-not-running/report.md b/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-11-gateway-not-running/report.md index 2a3fe6f9..e56fed74 100644 --- a/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-11-gateway-not-running/report.md +++ b/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-11-gateway-not-running/report.md @@ -5,5 +5,5 @@ Probe output collected before the SessionStart hook had a chance to run. PROBE: ssh-agent → ✓ (2 identities listed) PROBE: localhost-bind → ✓ (bound + loopback GET → HTTP 200, body=b'ok') -PROBE: podman-runtime → ✗ (gateway socket missing at ./.apache-magpie-local/run/podman.sock — container gateway not running) +PROBE: podman-runtime → ✗ (gateway socket missing at /Users/alice/tracker/.apache-magpie-local/run/podman.sock — container gateway not running) PROBE: docker-runtime → ⊘ (docker not on PATH) diff --git a/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-12-gateway-socket-denied/report.md b/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-12-gateway-socket-denied/report.md index 3796964e..846dd398 100644 --- a/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-12-gateway-socket-denied/report.md +++ b/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-12-gateway-socket-denied/report.md @@ -5,5 +5,5 @@ Probe output collected after the operator forgot to add the gateway sockets to a PROBE: ssh-agent → ✓ (2 identities listed) PROBE: localhost-bind → ✓ (bound + loopback GET → HTTP 200, body=b'ok') -PROBE: podman-runtime → ✗ (connect to ./.apache-magpie-local/run/podman.sock denied — add it to sandbox.network.allowUnixSockets) -PROBE: docker-runtime → ✓ (docker reaches the container gateway at ./.apache-magpie-local/run/docker.sock) +PROBE: podman-runtime → ✗ (connect to /Users/alice/tracker/.apache-magpie-local/run/podman.sock denied — add it to sandbox.network.allowUnixSockets) +PROBE: docker-runtime → ✓ (docker reaches the container gateway at /Users/alice/tracker/.apache-magpie-local/run/docker.sock) diff --git a/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-13-gateway-no-podman-backend/report.md b/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-13-gateway-no-podman-backend/report.md index 8c5a117e..01335c6d 100644 --- a/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-13-gateway-no-podman-backend/report.md +++ b/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-13-gateway-no-podman-backend/report.md @@ -6,4 +6,4 @@ Probe output collected while the gateway is up but the Podman machine is stopped PROBE: ssh-agent → ✓ (2 identities listed) PROBE: localhost-bind → ✓ (bound + loopback GET → HTTP 200, body=b'ok') PROBE: podman-runtime → ✗ (gateway running without a podman backend — is the Podman machine started? start it, then restart the gateway) -PROBE: docker-runtime → ✓ (docker reaches the container gateway at ./.apache-magpie-local/run/docker.sock) +PROBE: docker-runtime → ✓ (docker reaches the container gateway at /Users/alice/tracker/.apache-magpie-local/run/docker.sock) diff --git a/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-14-scratch-shared-session-root/expected.json b/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-14-scratch-shared-session-root/expected.json new file mode 100644 index 00000000..94047cd9 --- /dev/null +++ b/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-14-scratch-shared-session-root/expected.json @@ -0,0 +1 @@ +{"ssh_status": "pass", "localhost_status": "pass", "docker_status": "skip", "scratch_status": "pass", "signing_key_status": "skip", "gh_sandbox_status": "pass", "has_failures": false} diff --git a/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-14-scratch-shared-session-root/report.md b/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-14-scratch-shared-session-root/report.md new file mode 100644 index 00000000..d2629bf7 --- /dev/null +++ b/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-14-scratch-shared-session-root/report.md @@ -0,0 +1,12 @@ + + +Every probe ran to completion. TMPDIR is the shared session root rather than a per-project directory, which is the harness default and not a finding: Claude Code sets TMPDIR itself when it builds the sandbox, overriding any env.TMPDIR from a settings file. + +PROBE: ssh-agent → ✓ (1 identities listed) +PROBE: localhost-bind → ✓ (bound + loopback GET → HTTP 200, body=b'ok') +PROBE: docker-runtime → ⊘ (docker not on PATH) +PROBE: podman-runtime → ⊘ (podman not on PATH) +PROBE: project-scratch → ✓ (writable; shared session root, which is the harness default: /tmp/claude-501) +PROBE: signing-key → ⊘ (gpg.format is not ssh) +PROBE: gh-sandbox → ✓ (gh works inside the sandbox; exclusion not needed on this platform) diff --git a/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-15-gateway-relative-container-host/expected.json b/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-15-gateway-relative-container-host/expected.json new file mode 100644 index 00000000..bc8c041c --- /dev/null +++ b/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-15-gateway-relative-container-host/expected.json @@ -0,0 +1 @@ +{"ssh_status": "pass", "localhost_status": "pass", "docker_status": "fail", "scratch_status": "pass", "signing_key_status": "skip", "gh_sandbox_status": "pass", "has_failures": true} diff --git a/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-15-gateway-relative-container-host/report.md b/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-15-gateway-relative-container-host/report.md new file mode 100644 index 00000000..01d02020 --- /dev/null +++ b/tools/skill-evals/evals/setup-isolated-setup-doctor/interpret-probes/fixtures/case-15-gateway-relative-container-host/report.md @@ -0,0 +1,12 @@ + + +An older install left a project-relative CONTAINER_HOST in the committed .claude/settings.json. The gateway is running and the socket exists, but the CLI never reaches it: a unix:// URL's authority is read as a host component. + +PROBE: ssh-agent → ✓ (1 identities listed) +PROBE: localhost-bind → ✓ (bound + loopback GET → HTTP 200, body=b'ok') +PROBE: podman-runtime → ✗ (CONTAINER_HOST=unix://./.apache-magpie-local/run/podman.sock is not absolute — the CLIs do not resolve a relative unix:// value against the cwd; use unix:////.apache-magpie-local/run/podman.sock) +PROBE: docker-runtime → ⊘ (docker not on PATH) +PROBE: project-scratch → ✓ (writable; shared session root, which is the harness default: /tmp/claude-501) +PROBE: signing-key → ⊘ (gpg.format is not ssh) +PROBE: gh-sandbox → ✓ (gh works inside the sandbox; exclusion not needed on this platform) diff --git a/tools/spec-loop/.last-sync b/tools/spec-loop/.last-sync index 7452720a..b762e770 100644 --- a/tools/spec-loop/.last-sync +++ b/tools/spec-loop/.last-sync @@ -1 +1 @@ -492b582699b605cfc97a7bc7654009647fe0fb19 +5a6e150981dd7f1660ad7178c409e2270f0d6b84 diff --git a/tools/spec-loop/specs/README.md b/tools/spec-loop/specs/README.md index ac405349..3bde6164 100644 --- a/tools/spec-loop/specs/README.md +++ b/tools/spec-loop/specs/README.md @@ -52,6 +52,7 @@ Start with [`overview.md`](overview.md), then: [`contributor-growth.md`](contributor-growth.md), [`good-first-issue-sweep.md`](good-first-issue-sweep.md), [`codex-runtime.md`](codex-runtime.md), + [`copilot-runtime.md`](copilot-runtime.md), [`maintainer-education.md`](maintainer-education.md), [`spec-gap-staleness.md`](spec-gap-staleness.md), [`vetted-command-surface.md`](vetted-command-surface.md), diff --git a/tools/spec-loop/specs/container-gateway.md b/tools/spec-loop/specs/container-gateway.md index 6f749327..5e6d4097 100644 --- a/tools/spec-loop/specs/container-gateway.md +++ b/tools/spec-loop/specs/container-gateway.md @@ -111,13 +111,15 @@ One gateway process per project, keyed by the project root. It listens on `/.apache-magpie-local/run/podman.sock` (libpod + compat API, for the podman CLI) and `/.apache-magpie-local/run/docker.sock` (compat API, for the docker CLI). Both files sit inside the project tree. -What shipped: `CONTAINER_HOST` / `DOCKER_HOST` do honour a -project-relative `unix://./…` value, so the committed reference -`env` block (below) names both sockets that way and needs no -per-project edit. `sandbox.network.allowUnixSockets` is a separate -setting with no such relative form in practice; the committed -baseline carries no gateway-socket entry in it at all. The operator -adds the two sockets' **absolute** paths to the gitignored, per-project +Every setting that names a gateway socket needs its **absolute** +path. `CONTAINER_HOST` / `DOCKER_HOST` do **not** honour a +project-relative `unix://./…` value: a `unix://` URL's authority is +parsed as a host component, so `unix://./x` dials `/.//x`, +`unix://x` dials `/x/`, and `unix:x` dials `//` (verified against +podman 6.1.0). `sandbox.network.allowUnixSockets` has no relative +form either. The committed baseline therefore carries **neither** — +no gateway `env` block and no gateway-socket entry. The operator +adds both, with absolute paths, to the gitignored, per-project `.claude/settings.local.json` by hand (the block is in [Container gateway](../../../docs/setup/secure-agent-setup.md#container-gateway) in the setup guide, and `setup-isolated-setup-install` Step L proposes @@ -330,14 +332,22 @@ CLI flags with environment-variable equivalents, no config file: outside the tree — each one is logged at start so it shows up in a `setup verify` run), `--idle-timeout`, `--log-level`, `--pid-file`. -Reference settings (committed, project-agnostic): +Reference settings. The `hooks` half is committed and +project-agnostic; the `env` half is per-machine and belongs in the +gitignored `.claude/settings.local.json`, because it needs absolute +socket paths: ```jsonc +// .claude/settings.local.json — per machine { "env": { - "CONTAINER_HOST": "unix://./.apache-magpie-local/run/podman.sock", - "DOCKER_HOST": "unix://./.apache-magpie-local/run/docker.sock" - }, + "CONTAINER_HOST": "unix:////.apache-magpie-local/run/podman.sock", + "DOCKER_HOST": "unix:////.apache-magpie-local/run/docker.sock" + } +} + +// committed, project-agnostic +{ "hooks": { "SessionStart": [{ "hooks": [{ "type": "command", "command": "~/.claude/scripts/container-gateway-hook.sh start" }] }], @@ -347,16 +357,16 @@ Reference settings (committed, project-agnostic): } ``` -Both open questions this section used to flag are resolved, and this is -what shipped: `podman` and `docker` both resolve a project-relative -`unix://./…` value in `CONTAINER_HOST` / `DOCKER_HOST` against the cwd, -so the committed `env` block above works unedited in every adopting -project and carries no `allowUnixSockets` entry at all. -`sandbox.network.allowUnixSockets` has no equivalent relative-path -support, so the two gateway sockets are added there as **absolute** -per-project paths — in the gitignored `.claude/settings.local.json`, -never in the committed baseline, added by the operator (see *Known -gaps*): +This section once recorded both open questions as resolved in favour +of a project-relative `unix://./…` value, on the strength of the URL +*parsing* rather than a connection. That was wrong, and the committed +`env` block it justified could never have worked: the CLIs read the +authority as a host component and dial a path that does not exist. +Neither `CONTAINER_HOST` / `DOCKER_HOST` nor +`sandbox.network.allowUnixSockets` has a usable relative form, so all +four values are **absolute** per-project paths in the gitignored +`.claude/settings.local.json`, never in the committed baseline, added +by the operator (see *Known gaps*): ```jsonc // /.claude/settings.local.json diff --git a/tools/spec-loop/specs/copilot-runtime.md b/tools/spec-loop/specs/copilot-runtime.md new file mode 100644 index 00000000..f0c9a1b9 --- /dev/null +++ b/tools/spec-loop/specs/copilot-runtime.md @@ -0,0 +1,150 @@ + + +--- +title: GitHub Copilot skill runtime +status: experimental +kind: feature +mode: infra +source: > + RFC-AI-0004 Principle 3 (vendor neutrality) and issue #318. + Implemented by the canonical .agents/skills tree, the .github/skills + relay, .github/copilot-instructions.md, docs/adapters/copilot.md, and + tools/agent-isolation. +acceptance: + - A maintainer with GitHub Copilot and no Claude Code can discover and + invoke Magpie SKILL.md workflows from .agents/skills or .github/skills. + - Both Copilot surfaces are covered - the interactive CLI and the + server-side Coding Agent - with the boundary each one runs under + stated separately. + - The Coding Agent terminates at a Draft Pull Request and is never + dispatched on private security-tracker material. + - Copilot CLI use on security-list or private-list content requires an + explicit adopter opt-in, because GitHub-hosted models are not in the + default-approved set. + - The isolation the wrapper does and does not provide for this harness + is recorded rather than implied. +--- + +# GitHub Copilot skill runtime + +## What it does + +Makes GitHub Copilot a native Magpie runtime rather than a delegation +target. Copilot reads the same workflow sources as every other harness +and drives the same deterministic `tools/` bridges, under GitHub's own +confirmation and review boundaries. + +Copilot is two operational surfaces, not one, and they carry different +risk. The **interactive CLI** (`copilot`) is terminal-native and +operator-driven, confirming each command. The **Coding Agent** is +server-side and autonomous: it is assigned an issue or mentioned on a +PR and authors a scoped Draft Pull Request. Every rule below that +distinguishes the two does so because one runs on the operator's +machine and the other on shared cloud infrastructure. + +## Where it lives + +- `.agents/skills/` - canonical cross-harness skill symlinks. +- `.github/skills/` - the relay path Copilot also discovers. +- `.github/copilot-instructions.md` - repository instructions, read + alongside `AGENTS.md`. +- `docs/adapters/copilot.md` - the harness adapter guide. +- `~/.copilot/mcp-config.json` - MCP servers for the CLI; the Coding + Agent configures its own in repository Copilot settings. +- `tools/agent-isolation/agent-iso.sh` - the clean-environment launcher. + +## Behaviour & contract + +- **Same sources, no fork.** Skills are discovered from + `.agents/skills/` and `.github/skills/`; repository instructions come + from `.github/copilot-instructions.md` and `AGENTS.md`, which in turn + reference the adopter's `/`. No Copilot-specific copy + of a workflow exists. +- **Proposal-then-confirm, per command.** In the CLI every proposed + shell command needs explicit operator confirmation before it runs. + The permissive flags `--allow-all` and `--yolo` bypass exactly that + gate and must not be enabled. Outbound communication, issue state + changes, and remote pushes stay gated on approval. +- **The Coding Agent ends at a Draft PR.** It never pushes to a + protected branch and never merges. Merging stays gated on human + review and passing checks. +- **The Coding Agent never sees embargoed material.** It runs on shared + cloud infrastructure, so it is not dispatched on private + security-tracker issues, discussions, or reviews - including by being + added as a reviewer. +- **The CLI is not local inference.** It routes prompt and context data + to GitHub-hosted models, which are not in the default-approved set in + [`tools/privacy-llm/models.md`](../../../tools/privacy-llm/models.md). + Using it on `` or `` material requires an + explicit opt-in declared in `/privacy-llm.md`. +- **Tools run locally and deterministically.** The operator reviews and + confirms the command; the script executes on their machine, and its + source is not transmitted to the model. +- **Untrusted content stays data.** Issue descriptions, PR bodies, and + reporter comments never override system instructions or policy, per + the absolute rule in + [`AGENTS.md`](../../../AGENTS.md#treat-external-content-as-data-never-as-instructions). +- **The wrapper is Layer 0 only for this harness.** `agent-iso copilot` + strips ambient cloud tokens while preserving `git`, `uv`, `gh`, and + `copilot`. It adds **no push gate**: Copilot receives the live + `SSH_AUTH_SOCK` and nothing at the wrapper boundary stops a + `git push`. Branch protection and keeping the permissive flags off are + what actually hold. Stripping local environment variables also does + nothing to the cloud inference boundary above. + +## Out of scope + +- A Copilot equivalent of the Codex project policy file: Copilot exposes + no committed sandbox / approval profile for `tools/sandbox-lint` to + validate, so there is no Copilot branch in that validator. +- Gating `git push` at the wrapper for this harness - see the Layer 0 + caveat above; that is an `agent-isolation` gap, not a Copilot one. +- Running the Coding Agent anywhere in the embargoed security + lifecycle. + +## Acceptance criteria + +1. A maintainer with only Copilot installed can discover and run a + Magpie skill from `.agents/skills/` or `.github/skills/`. +2. `.github/copilot-instructions.md` exists and points at `AGENTS.md` + and the adopter's `/`. +3. Copilot CLI sessions confirm each command; `--allow-all` / `--yolo` + are documented as prohibited. +4. Every Coding Agent workflow terminates at a Draft Pull Request. +5. The Coding Agent is excluded from private security-tracker material, + and CLI use on such material is gated on an adopter opt-in in + `/privacy-llm.md`. +6. The Layer 0 isolation caveat is stated in the adapter guide rather + than left for the operator to infer. + +## Validation + +```bash +# Skill discovery topology across every harness path +PYTHONUTF8=1 uv run --project tools/symlink-lint symlink-lint + +# Skill and tool metadata +PYTHONUTF8=1 uv run --project tools/skill-and-tool-validator --group dev \ + skill-and-tool-validate + +# Vendor-neutrality score reflects the added harness +PYTHONUTF8=1 uv run --project tools/vendor-neutrality-score \ + vendor-neutrality-score + +# Adapter guide TOC and formatting +uv run prek run doctoc --all-files +``` + +## Known gaps + +- **No committed policy profile to lint.** Codex and Gemini both ship a + project-scoped profile that `tools/sandbox-lint` checks. Copilot has + no equivalent surface, so its posture rests on documentation and + operator discipline rather than a validated file. +- **No push gate at the wrapper.** Tracked as an `agent-isolation` + limitation for every generic `agent-iso ` invocation, not + specific to Copilot. +- **The Coding Agent's MCP configuration is not in the repository.** It + lives in repository Copilot settings, so it cannot be reviewed in a + PR the way `.mcp.json` or the CLI's `~/.copilot/mcp-config.json` can. diff --git a/tools/spec-loop/specs/overview.md b/tools/spec-loop/specs/overview.md index 1ed0df2d..f3ab8162 100644 --- a/tools/spec-loop/specs/overview.md +++ b/tools/spec-loop/specs/overview.md @@ -69,6 +69,7 @@ Each mode is an independently toggleable set of skills. Maturity mirrors | Contributor-growth family | [contributor-growth.md](contributor-growth.md) | | Good-first-issue backlog sweep | [good-first-issue-sweep.md](good-first-issue-sweep.md) | | Codex first-class skill runtime | [codex-runtime.md](codex-runtime.md) | +| GitHub Copilot skill runtime | [copilot-runtime.md](copilot-runtime.md) | | Spec-gap staleness verification (proposed) | [spec-gap-staleness.md](spec-gap-staleness.md) | | Vetted command surface (fixed-shape forge ops) | [vetted-command-surface.md](vetted-command-surface.md) | diff --git a/tools/spec-loop/specs/pr-management-family.md b/tools/spec-loop/specs/pr-management-family.md index a257bd3c..c24ee64c 100644 --- a/tools/spec-loop/specs/pr-management-family.md +++ b/tools/spec-loop/specs/pr-management-family.md @@ -130,6 +130,28 @@ is listed here for navigability since its domain is PR threads. without either remains `unknown`. Every surfaced finding carries that constraint ledger, and its remediation follows the adopter's applicable `AGENTS.md` and dependency or release policy for all three classifications. +- **A green rollup is not evidence CI ran.** `statusCheckRollup.state == + SUCCESS` aggregates only completed check-runs, so a PR whose real + workflows sit in `action_required` — awaiting approval for a + first-time contributor — reports SUCCESS while nothing was built, + linted, or tested, and fast bot checks (Mergeable, WIP, DCO, + boring-cyborg) that succeed unconditionally are enough to carry it + there. Both `pr-management-triage` and `pr-management-code-review` + therefore run a mandatory Real-CI guard before any row classifies a + PR as passing. A PR whose real CI never ran is as ineligible for + APPROVE as one that fails, sorts below every PR with a real run when + a queue is ordered, and is reported as *CI unverified* rather than + having its outcome predicted. +- **A posted review is confirmed, never retried.** `gh pr review` prints + nothing on success, so an empty result must not be read as failure: a + zero exit means the review posted whatever it printed. Before any + retry the skill confirms the post-condition through the reviews API, + because a duplicate cannot be withdrawn — GitHub deletes only pending + reviews, and `DELETE /repos/{owner}/{repo}/pulls/{n}/reviews/{id}` + answers `422 Can not delete a non-pending pull request review` once + submitted, leaving the body edited down to a pointer as the only + repair. The same rule covers `gh pr comment` and the + `addPullRequestReview` mutation. - **Config-driven, not skill-edited.** Project-specific values (committers team handle, area-label prefix, comment-template wording, CI-check → doc-URL map, review criteria, quick-merge path globs) all