-
Notifications
You must be signed in to change notification settings - Fork 7
fix(remote-worker): make the leaf images usable as sandboxes (/workspace + advertised capabilities) #252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(remote-worker): make the leaf images usable as sandboxes (/workspace + advertised capabilities) #252
Changes from 7 commits
5d553ca
8ba6bee
187e7bf
72fae25
3b64c12
52a1c01
e47e70d
bb36151
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -15,6 +15,65 @@ COPY gen/go/ ./gen/go/ | |||||||||||||
| COPY remote-worker/ ./remote-worker/ | ||||||||||||||
| WORKDIR /src/remote-worker | ||||||||||||||
| RUN CGO_ENABLED=0 go build -trimpath -ldflags "-s -w" -o /out/remote-worker ./cmd/worker | ||||||||||||||
|
|
||||||||||||||
| # ripgrep is vendored because it is the IMPLEMENTATION of two registered tools, not a scheduling | ||||||||||||||
| # label. The Find/Glob tool runs `rg --files --hidden` (k8s-sandbox `createPodFindOps`.glob) and the | ||||||||||||||
| # Grep tool runs `rg` directly (`createPodGrepTool`), and both land in THIS container whenever a | ||||||||||||||
| # leased gRPC presence record supplies a relay transport: extension.ts's `opts?.transport ??` | ||||||||||||||
| # overrides the fast AND the stream transport, so every pod operation runs over the relay. Without | ||||||||||||||
| # `rg` on PATH the model's Glob returns "glob failed in pod (rg exited 127)" and its Grep "rg failed | ||||||||||||||
| # in pod (exit 127)" -- the same attached-and-healthy-yet-every-tool-fails signature as the | ||||||||||||||
| # /workspace bug below. Both other sandbox images in the repo install it (sandbox.Dockerfile, | ||||||||||||||
| # k8s-sandbox/deploy/sandbox.yaml). | ||||||||||||||
| # | ||||||||||||||
| # It is genuinely not in the UBI 9 repositories (verified: `microdnf install ripgrep` -> "No package | ||||||||||||||
| # matches"), and EPEL would add a third-party repository to an image that executes model-authored | ||||||||||||||
| # commands. So: the upstream static musl build, pinned by version and checked against the sha256 the | ||||||||||||||
| # release publishes. Static, so it needs no libc from this base. Fetched in its own stage, so the | ||||||||||||||
| # tar/gzip needed to unpack it never reach the runtime image. | ||||||||||||||
| # | ||||||||||||||
| # The version is not free to choose. `createPodFindOps`.glob leans on two documented .gitignore | ||||||||||||||
| # nuances -- gitignored DIRECTORIES stay pruned even when `-g` matches inside them, while an | ||||||||||||||
| # individually-gitignored FILE matching a positive `-g` IS re-included -- and those were originally | ||||||||||||||
| # characterised on rg 14.1.0, the version alpine:3.20 ships and therefore the one both other sandbox | ||||||||||||||
| # images and the k8s-sandbox SMOKE.md run used. 15.2.0 crosses a major boundary, so if it diverged | ||||||||||||||
| # there, Glob's ignore semantics would differ by which sandbox image an operator happened to build -- | ||||||||||||||
| # silently, and only for gitignored paths. Both nuances were re-verified against these exact pinned | ||||||||||||||
| # tarballs, on both arches, before pinning: they hold unchanged. operations.ts and the M3 spec's D5 | ||||||||||||||
| # note record that. | ||||||||||||||
| # | ||||||||||||||
| # Pinning back to 14.1.0 instead is NOT available: that release publishes no | ||||||||||||||
| # aarch64-unknown-linux-musl asset (only -gnu), so it would either break the arm64 build or give up | ||||||||||||||
| # the static property this stage exists for. | ||||||||||||||
| # | ||||||||||||||
| # To bump: change RG_VERSION and BOTH digests together, AND re-check those two nuances on the new | ||||||||||||||
| # version. The digests are per-arch and `sha256sum -c` fails the build closed if either is wrong. | ||||||||||||||
| # Arch comes from `uname -m` rather than TARGETARCH because buildah / `oc new-build | ||||||||||||||
| # --strategy=docker` do not populate BuildKit's automatic args. | ||||||||||||||
| # hadolint ignore=DL3007 | ||||||||||||||
| FROM registry.access.redhat.com/ubi9/ubi-minimal:latest AS rg | ||||||||||||||
| ARG RG_VERSION=15.2.0 | ||||||||||||||
| ARG RG_SHA256_X86_64=33e15bcf1624b25cdd2a55813a47a2f95dbe126268203e76aa6a585d1e7b149c | ||||||||||||||
| ARG RG_SHA256_AARCH64=800b1e7206afe799dfb5a6901f23147cfaabe0e52210538100f61e86e1740915 | ||||||||||||||
| # hadolint ignore=DL3041 | ||||||||||||||
| RUN microdnf install -y --nodocs tar gzip \ | ||||||||||||||
| && microdnf clean all | ||||||||||||||
| RUN set -eux; \ | ||||||||||||||
| case "$(uname -m)" in \ | ||||||||||||||
| x86_64) target=x86_64-unknown-linux-musl; sha="$RG_SHA256_X86_64" ;; \ | ||||||||||||||
| aarch64) target=aarch64-unknown-linux-musl; sha="$RG_SHA256_AARCH64" ;; \ | ||||||||||||||
| *) echo "no pinned ripgrep build for $(uname -m)" >&2; exit 1 ;; \ | ||||||||||||||
| esac; \ | ||||||||||||||
| tarball="ripgrep-${RG_VERSION}-${target}.tar.gz"; \ | ||||||||||||||
| curl -fsSL -o "/tmp/${tarball}" \ | ||||||||||||||
| "https://github.com/BurntSushi/ripgrep/releases/download/${RG_VERSION}/${tarball}"; \ | ||||||||||||||
| printf '%s %s\n' "${sha}" "/tmp/${tarball}" > /tmp/rg.sha256; \ | ||||||||||||||
| sha256sum -c /tmp/rg.sha256; \ | ||||||||||||||
| tar -xzf "/tmp/${tarball}" -C /tmp; \ | ||||||||||||||
| install -m 755 "/tmp/ripgrep-${RG_VERSION}-${target}/rg" /usr/local/bin/rg; \ | ||||||||||||||
| rm -rf "/tmp/${tarball}" /tmp/rg.sha256 "/tmp/ripgrep-${RG_VERSION}-${target}"; \ | ||||||||||||||
| /usr/local/bin/rg --version | ||||||||||||||
|
|
||||||||||||||
| # The worker execs `bash -c <command>`, so the runtime image needs a shell and the | ||||||||||||||
| # coreutils the harness's ops call: base64 (writeFile), file (image mime sniffing). | ||||||||||||||
| # distroless/static ships none of them — every exec would fail with ENOENT. | ||||||||||||||
|
|
@@ -26,10 +85,40 @@ RUN CGO_ENABLED=0 go build -trimpath -ldflags "-s -w" -o /out/remote-worker ./cm | |||||||||||||
| # bumps for the same reason .hadolint.yaml already ignores DL3018 for Alpine. | ||||||||||||||
| # hadolint ignore=DL3007 | ||||||||||||||
| FROM registry.access.redhat.com/ubi9/ubi-minimal:latest | ||||||||||||||
| # git and python3 are here because cmd/worker/main.go's `probed` list advertises | ||||||||||||||
| # bash, rg, base64, file, python3, git | ||||||||||||||
| # as this worker's Hello.capabilities, and its comment says "the pool will eventually match on | ||||||||||||||
| # these, so they must be true". Three of the six were not: on a real VM the leaf reported | ||||||||||||||
| # `caps=[bash base64 file]`, so it advertised a set it could not honour. All six are now installed | ||||||||||||||
| # (`rg` from the stage above), and dockerfile_parity_test.go fails if that stops being true. | ||||||||||||||
| # | ||||||||||||||
| # git also matters for measurement. Spec §2.3's duty bases were derived from workloads whose git | ||||||||||||||
| # operations cost ~470ms; without git in the sandbox, an E8 tool call can only be a ~0ms no-op, so | ||||||||||||||
| # the hands tier is exercised structurally but carries no load and the measured duty cycle describes | ||||||||||||||
| # a cheaper workload than the basis it is compared against. | ||||||||||||||
| # hadolint ignore=DL3041 | ||||||||||||||
| RUN microdnf install -y --nodocs bash coreutils-single findutils file \ | ||||||||||||||
| RUN microdnf install -y --nodocs bash coreutils-single findutils file git python3 \ | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion — nothing in CI builds either of these Dockerfiles, so the package-availability judgements this PR turns on are unverified, and the drift it fixes can recur silently. The whole diff rests on claims about what UBI 9 provides —
So a package name that is absent or renamed in a future UBI 9 minor surfaces to whoever next runs The cheap guard is a parity test, and the repo already has the pattern. Adding the two files to Keeping both Dockerfiles textually in step is a real maintenance cost now: they carry the same install line and the same three-command
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added as a Go test — I went with Go rather than the vitest parse you suggested, for one reason that I think makes it strictly better: in-package it can reference Three properties, and I negative-tested each rather than trusting a green run:
All three pass on the branch. Your framing was the useful part here — a test that could not fail would have been worse than none, and Two details worth calling out because they are where this kind of test usually rots:
On the two larger items you raised: I did not add these files to |
||||||||||||||
| && microdnf clean all | ||||||||||||||
| COPY --from=build /out/remote-worker /usr/local/bin/remote-worker | ||||||||||||||
| COPY --from=rg /usr/local/bin/rg /usr/local/bin/rg | ||||||||||||||
| # The harness execs every tool call from its sandbox working directory, which defaults to | ||||||||||||||
| # /workspace (KAGENTI_SANDBOX_CWD, select-sandbox.ts). Without this directory the worker's | ||||||||||||||
| # `bash -c` fails before running anything the caller asked for: | ||||||||||||||
| # bash: line 1: cd: /workspace: No such file or directory | ||||||||||||||
| # Command exited with code 1 | ||||||||||||||
| # and the harness reports a tool error for a sandbox that is otherwise healthy and attached -- | ||||||||||||||
| # observed on a real VM run, where the exec reached the leaf correctly and died on the cd. Owned by | ||||||||||||||
| # 1001:0 because that is the uid this image runs as (gid 0, matching the nonroot-v2 SCC below), so | ||||||||||||||
| # the agent can actually write in its own workspace. One `install -d` states owner, group and mode in | ||||||||||||||
| # one place, as build-swebench-sandbox.sh's generated Dockerfile already does. | ||||||||||||||
| # | ||||||||||||||
| # WORKDIR because BashRunner's "every command the harness sends is self-contained (`cd 'cwd' && | ||||||||||||||
| # ...`)" is today the only thing making the process cwd irrelevant. Any exec path that ever sends a | ||||||||||||||
| # bare command -- or an operator running `podman exec` to debug a sandbox -- would otherwise land in | ||||||||||||||
| # /, which is a second round of the bug documented directly above. Both sandbox images that came | ||||||||||||||
| # before this one set it. | ||||||||||||||
| RUN install -d -o 1001 -g 0 -m 775 /workspace | ||||||||||||||
| WORKDIR /workspace | ||||||||||||||
| # Matches the nonroot-v2 SCC deploy-incluster.sh applies. | ||||||||||||||
| USER 1001 | ||||||||||||||
| ENTRYPOINT ["/usr/local/bin/remote-worker"] | ||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion — version pinned here is two majors ahead of the one the dependent semantics were verified on.
This vendoring exists to serve Find/Glob, but those semantics were characterised against a different ripgrep major:
packages/k8s-sandbox/src/operations.ts:161— "rg --files --hiddenlists files under cwd, honouring .gitignore (verified on the pod's ripgrep 14.1.0)", followed by the gitignored-DIRECTORY-vs-FILE nuancedocs/specs/2026-06-17-m3-persistent-channel-design.md:28and:255— the same nuance, recorded as a "Verified nuance (rg 14.1.0)"15.0.0 was a major release, and after this PR the leaf is a sandbox that
rg --files --hiddenruns inside, so that nuance is unverified on the version actually being shipped. If it changed, Glob's ignore semantics differ by which sandbox image an operator built — silently, and only for gitignored paths.Either pin to the version the semantics were verified on, or re-verify on 15.2.0 and update both the comment and the spec note.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-verified on 15.2.0 and updated both the comment and the spec notes — e47e70d. I took your second option, because the first turned out not to be available.
Pinning back to 14.1.0 is not possible without giving something up. That release publishes no
aarch64-unknown-linux-muslasset — its only aarch64 Linux build is-gnu. So pinning to the version the semantics were characterised on would either break the arm64 path (which README-worker.md makes a real one, since these images are built for the host) or trade the static property this fetch stage exists for. That is worth recording, because "just pin to the verified version" is the obvious reading of your finding and it does not survive contact with the asset list.So I verified the version actually being shipped, against the exact pinned tarballs rather than a convenient local build:
-gmatches inside-gBoth nuances hold unchanged. Method, since a check like this is easy to do vacuously:
.gitignorecoveringnode_modules/andsecret.txt, thenrg --files --hidden -g '*.js'(directory case) and-g 'secret.txt'(file case)ubi9/ubi-minimalwith the tarball fetched andsha256sum -cverified exactly as the Dockerfile does it, on aarch64-unknown-linux-musl natively and x86_64-unknown-linux-musl emulated. Both digests matched the published.sha256.rg --versionruns.On 14.1.0 not being an arbitrary baseline — it is what
alpine:3.20ships, which is why both other sandbox images and the SMOKE.md run observed it. I left SMOKE.md alone: it is a dated result record, not a live claim.The Dockerfiles now say the version is not free to choose and that a bump has to re-check those two nuances, not just the digests — the failure mode you named (semantics differing by which image an operator built, silently, and only for gitignored paths) is not something a digest check would catch.