From 58cdebabbb90430656a68f7861b815d068b7b3a2 Mon Sep 17 00:00:00 2001 From: n Date: Wed, 5 Aug 2026 21:29:47 -0500 Subject: [PATCH] Verify the pipeline container before the skills use it The skills open at `docker exec crypter-pipeline` and assume the container is running and belongs to this checkout. Neither holds by itself. The Compose mounts are relative paths, so they resolve against whichever checkout launched the container, and a second checkout finds a container by name whose /runs writes land somewhere it never looks and whose /host-git is a different history. The trap is that an exited container looks repairable. `docker start` reuses the mounts and image fixed at creation, so it brings the wrong container back and the run fails two steps later at workspace creation, reading as a missing executable rather than as the wrong container. crypter-review and crypter-triage-review gain the preflight crypter-change already had, and all three now also check the image carries crypter-workspace, which the mount checks alone let through. The checks are `test` rather than `command -v` because docker exec runs a binary and not a shell, so a builtin exits 127 either way. The remedy is `up -d --build`, not `--force-recreate`, which recreates against new mounts but keeps a stale image. It takes over the one crypter-pipeline on the machine, so the skills ask before running it. Co-Authored-By: Claude Opus 5 --- .claude/skills/crypter-change/SKILL.md | 26 +++++++++++++---- .claude/skills/crypter-review/SKILL.md | 29 +++++++++++++++++++ .claude/skills/crypter-triage-review/SKILL.md | 29 +++++++++++++++++++ .../Agentic Development Pipeline.md | 19 ++++++++++-- 4 files changed, 96 insertions(+), 7 deletions(-) diff --git a/.claude/skills/crypter-change/SKILL.md b/.claude/skills/crypter-change/SKILL.md index d491150f..ba547c09 100644 --- a/.claude/skills/crypter-change/SKILL.md +++ b/.claude/skills/crypter-change/SKILL.md @@ -36,16 +36,32 @@ and both are yours to read at any point. write into a directory that grants it. Creating them on this side also keeps you able to delete what they wrote — a directory the container creates is one you cannot remove. -The container needs both mounts, and the run directory has to be writable from inside it. -Confirm before starting: +The container needs both mounts, the run directory has to be writable from inside it, and the +image has to carry the current tooling. Confirm before starting: ```bash docker exec crypter-pipeline test -d /plans/{run-id} && \ - docker exec crypter-pipeline test -w /runs/{run-id}/findings + docker exec crypter-pipeline test -w /runs/{run-id}/findings && \ + docker exec crypter-pipeline test -x /usr/local/bin/crypter-workspace ``` -A container created before these existed picks them up on -`docker compose -f .devcontainer/docker-compose.yml up -d --force-recreate`. +The mount checks also settle which checkout the container belongs to: one created against a +different one reaches neither directory. The last check is separate because an older image +passes the first two and then fails at workspace creation with nothing but a missing executable +to go on. All three are `test` because `docker exec` runs a binary and not a shell, so a builtin +like `command -v` exits 127 whether or not the thing it was looking for is there. + +**Do not `docker start` an exited container to fix any of this.** Mounts and image are fixed +when a container is created, so starting one built from another checkout, or from an older +image, brings back the same wrong container. Bring it up from here instead: + +```bash +docker compose -f .devcontainer/docker-compose.yml up -d --build +``` + +That rebuilds the image and recreates the container against this checkout's mounts. It replaces +any container of the same name, so **ask the user before running it** — theirs may belong to +another checkout and hold work you cannot see. Then make the workspace the container builds in. It is a clone of your repository, taken from the read-only `/host-git` mount, and it lasts exactly as long as this run: diff --git a/.claude/skills/crypter-review/SKILL.md b/.claude/skills/crypter-review/SKILL.md index 2c61f5b2..30a25983 100644 --- a/.claude/skills/crypter-review/SKILL.md +++ b/.claude/skills/crypter-review/SKILL.md @@ -31,6 +31,35 @@ The container's `agent` is uid 1001 and your files are uid 1000, so the agents w directories this side creates and grants. Creating them here also keeps you able to delete what they wrote. +Then confirm the running container is the one this checkout describes, before anything depends +on it: + +```bash +docker exec crypter-pipeline test -w /runs/pr-{number}/findings && \ + docker exec crypter-pipeline test -x /usr/local/bin/crypter-workspace +``` + +The first proves the `/runs` mount reaches the directory you just made, which a container +created against a different checkout will not. The second proves the image carries the current +tooling. A container that fails either is not this checkout's, and every later step fails +against it in a way that reads like something else — a missing executable, findings written +somewhere you never look. + +Both are `test` because `docker exec` runs a binary and not a shell, so a builtin like +`command -v` exits 127 whether or not the thing it was looking for is there. + +**Do not `docker start` an exited container to fix this.** Mounts and image are fixed when a +container is created, so starting one built from another checkout, or from an older image, +brings back the same wrong container. Bring it up from here instead: + +```bash +docker compose -f .devcontainer/docker-compose.yml up -d --build +``` + +That rebuilds the image and recreates the container against this checkout's mounts. It replaces +any container of the same name, so **ask the user before running it** — theirs may belong to +another checkout and hold work you cannot see. + ## 1. Read the pull request Read its title, description and diff with whatever GitHub access this session has — the `gh` diff --git a/.claude/skills/crypter-triage-review/SKILL.md b/.claude/skills/crypter-triage-review/SKILL.md index 34420ee4..e4af386c 100644 --- a/.claude/skills/crypter-triage-review/SKILL.md +++ b/.claude/skills/crypter-triage-review/SKILL.md @@ -27,6 +27,35 @@ chmod 777 .claude/runs/pr-{number} .claude/runs/pr-{number}/verification The container's `agent` is uid 1001 and your files are uid 1000, so the agents write into directories this side creates and grants. +Then confirm the running container is the one this checkout describes, before anything depends +on it: + +```bash +docker exec crypter-pipeline test -w /runs/pr-{number}/verification && \ + docker exec crypter-pipeline test -x /usr/local/bin/crypter-workspace +``` + +The first proves the `/runs` mount reaches the directory you just made, which a container +created against a different checkout will not. The second proves the image carries the current +tooling. A container that fails either is not this checkout's, and every later step fails +against it in a way that reads like something else — a missing executable, verification written +somewhere you never look. + +Both are `test` because `docker exec` runs a binary and not a shell, so a builtin like +`command -v` exits 127 whether or not the thing it was looking for is there. + +**Do not `docker start` an exited container to fix this.** Mounts and image are fixed when a +container is created, so starting one built from another checkout, or from an older image, +brings back the same wrong container. Bring it up from here instead: + +```bash +docker compose -f .devcontainer/docker-compose.yml up -d --build +``` + +That rebuilds the image and recreates the container against this checkout's mounts. It replaces +any container of the same name, so **ask the user before running it** — theirs may belong to +another checkout and hold work you cannot see. + ## 1. Collect the findings Read the pull request with whatever GitHub access this session has — the `gh` CLI, or the GitHub diff --git a/Documentation/Development/Agentic Development Pipeline.md b/Documentation/Development/Agentic Development Pipeline.md index 6c5252fc..0a19a8d7 100644 --- a/Documentation/Development/Agentic Development Pipeline.md +++ b/Documentation/Development/Agentic Development Pipeline.md @@ -105,8 +105,23 @@ git -c protocol.ext.allow=user fetch \ `protocol.ext.allow` is passed per command, so it stays out of your git config. -A container created before these mounts existed picks them up on -`docker compose -f .devcontainer/docker-compose.yml up -d --force-recreate`. +Because the mounts are relative paths in the Compose file, they resolve against the checkout you +launch from, and a container is stuck with whatever they resolved to when it was created. Run +the pipeline from a second checkout and the container it finds by name is the first one's: +`/runs` writes land under a repository you are not looking at, and `/host-git` clones a history +that is not the one you are reviewing. + +Starting an exited container does not repair this, and neither does it pick up a newer image — +`docker start` reuses what the container was created with. Recreate it from the checkout you +mean to work in: + +```bash +docker compose -f .devcontainer/docker-compose.yml up -d --build +``` + +That rebuilds the image and recreates the container against the mounts as they resolve here. +There is one `crypter-pipeline` on the machine, so this takes it over from whichever checkout +held it. ## Running a change