Verify the pipeline container before the skills use it - #848
Merged
Conversation
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The pipeline skills open at
docker exec crypter-pipelineand assume the container is running and belongs to this checkout. The Compose mounts are relative paths, so they resolve against whichever checkout launched the container — run the pipeline from a second checkout and it finds a container by name whose/runswrites land under a repository you are not looking at.An exited container also looks repairable when it is not.
docker startreuses the mounts and image fixed at creation, so it brings the same wrong container back, and the run then fails at workspace creation reading as a missing executable rather than as the wrong container.crypter-reviewandcrypter-triage-reviewnow carry the preflightcrypter-changealready had, and all three additionally check the image carriescrypter-workspace, which the mount checks alone let through.Two things a reviewer would otherwise have to work out:
testrather thancommand -vbecausedocker execruns a binary and not a shell, so a builtin exits 127 whether or not the tool is present.testis checked because coreutils provides/usr/bin/test.up -d --force-recreatetoup -d --build. The former recreates against new mounts but keeps a stale image, which is half the failure. Since it takes over the singlecrypter-pipelineon the machine, the skills now ask before running it.Found by running
/crypter-reviewagainst a container left over from another checkout.