Repository navigation
Conversation
The pinned provider forwards the guest engine socket through its vsock muxer. On macOS, when a host client closes a forwarded connection while the guest is still sending, the muxer re-arms the dead stream with write interest only and then fails to delete it: one kevent change list stops at the unregistered read filter. The socket reports a hang-up on every wait, each one queues a reset to the guest and restarts the stream's reaper delay, so the stream is never removed. Guest vCPUs and the shared vsock queue stay saturated and later agent and engine requests time out. The BuildKit client closes many connections before reading the whole response and the rest at exit, so most builds reached that state. The build client now reaches the engine through a private relay that always drains guest bytes (discarding them once the client is gone), forwards a client's end of stream only as a half-close, and closes a provider connection only after the guest ended it, within a bounded wait. Refs HACK-1924 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`runtime down` failed with agent_protocol_uncertain and `runtime recover` refused while the provider process was alive, so a guest whose agent stopped answering could only be stopped by signalling the provider by hand. When a guest step of `down` fails that way, or `recover` finds the recorded provider alive, both now ping the agent three times over about 13 seconds. If no ping starts a reply, they stop this home's recorded provider after verifying its PID, start time, user and executable, with no guest quiescence, and record the pool `recovered-unclean`, which the next start accepts after its usual disk checks. A guest that answers keeps the clean stop, and `recover` still refuses it. Refs HACK-1924 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffe2b3fd5f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A build whose process is killed cannot remove its private relay socket directory. Relay directories now carry the creating PID, and each new relay removes those whose process no longer exists, but only a mode-0700 directory owned by this user that holds nothing except the relay socket. Refs HACK-1924 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Under sustained backpressure, partial writes advanced the start offset but the delivered prefix stayed in the buffer until the queue drained completely, so the limits bounded only the unread suffix while storage grew with the whole transfer (128 MiB for a 1 MiB limit in the new test). The delivered prefix is now dropped once it reaches half the buffer, keeping storage within about twice the pending bytes plus one read at linear cost. Refs HACK-1924 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66d25b8381
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
A build releases the operation lease while its client runs and keeps only the shared build fence, so the lease alone did not prove that no build client was using the engine. `runtime recover` could stop the provider of a silent guest underneath a live build client. Both silent-guest paths now check the fence under the operation lease (`recover` before probing, and both again just before signalling) and refuse with provider_busy, telling the user to finish or cancel the build first; the next attempt then stops the silent guest. The fence is a flock released when the build's processes exit, including on SIGKILL, as a_killed_coordinator_leaves_its_client_fenced already proves. Also writes the abandoned-relay fixture's extra file before its socket, so a relay starting in a concurrent test cannot remove it first. Refs HACK-1924 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Its only caller is a macOS-only lifecycle test, so Linux test builds flagged the helper as dead code and failed the candidate-core lint. Refs HACK-1924 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…rent relays The abandoned-relay test built its fixtures in the shared temporary directory, where any concurrently starting relay's sweep could remove a fixture between its creation and its socket bind. Fixtures now live under a per-test root that no other relay sweeps, and the relay under test starts there (ClientRelay::start_in, which start uses with the temporary directory). The test also checks that the abandoned fixture existed before the sweep. The refused-provider test treats EINVAL from setting the read timeout as the closed connection it expects: macOS refuses socket options once the relay has already closed the connection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
, #612, #628, #629, #630) (#639) ## What this is Merge train batch 6 into `next`: the native image builds stack and its fixes, as one batch. Builds stay off by default: they run only with `HACK_NATIVE_BUILDS=1`, which native `up`, restart and doctor all check. Each component's reviewed head is merged with `git merge --no-ff`, in stack order. Integration conflicts are resolved in the batch's merge commits, described below. Hosted CI on this branch is the full-suite gate for the combined result. | PR | Title | Linear | Head included | Review | | --- | --- | --- | --- | --- | | #583 | feat(native): pin a BuildKit client and scan Dockerfile image references | HACK-1270 | `f21d5a04` | Codex code + security: no findings; 0 open threads | | #606 | feat(native): build Compose images in the guest engine (runtime build-image) | HACK-1270 | `460ddff2` | Codex code + security: no findings; 0 open threads | | #607 | feat(native): admit build storage, budget build cache and reclaim superseded build images | HACK-1270 | `9f33a1d0` | Codex code + security: no findings; 0 open threads | | #612 | feat(native): build Compose build services in native up behind HACK_NATIVE_BUILDS | HACK-1270 | `7f42e328` | Codex clean at `8ec7460e`; `7f42e328` adds #628's squash (re-review requested) | | #628 | fix(native): admit native build fields and name services in build refusals | HACK-1925 | `f07ea583` | Codex code + security clean; already squashed into #612's branch | | #629 | fix(native): keep the VM responsive during image builds and stop a silent guest | HACK-1924 | `a0e3e797` | Codex code + security: no findings; 0 open threads | | #630 | fix(native): restore a sourceless graph after its project files were touched | HACK-1914 | `a93db842` | Codex clean at `009989c7`; `a93db842` only merges #629 into it (re-review requested) | ## Security properties in the merged code - **Build arguments:** they reach the BuildKit client only through a private 0600 bake definition, never its environment. The client environment is cleared to an allowlist. Names must be plain identifiers, and BuildKit-interpreted names such as `BUILDKIT_*`, `BUILDX_*` and `SOURCE_DATE_EPOCH` are refused. A build argument equal to a managed secret value is refused without echoing the value. Build arguments are not secrets, matching Docker. - **Source policy:** a source policy from `build_policy::document`, set through `EXPERIMENTAL_BUILDKIT_SOURCE_POLICY` in a private file, applies to every build. It allows only the scanner's digest-pinned base images and denies other images, `oci-layout`, `http`, `https`, `git` and blob sources. Pinned bases whose config has `OnBuild` triggers are refused. - **Dockerfile scanner:** - remote `ADD` sources are refused, including the JSON form, multiple sources and variables; - JSON-form arguments are read as BuildKit reads them, and a lenient-decode differential fails closed; - heredocs are recognized as BuildKit's lexer does, including whitespace after `<<`; - a stage named `scratch` is refused; - `ARG` names must be plain identifiers; - platform arguments must resolve to `linux/arm64`; - expanded values are size-bounded. - **Build output:** forwarded output is sanitized and then redacted, including Go-quoted and JSON-escaped forms and values split across a line cut. It is bounded by size. ## Integration changes - **#606 with #605 / #609:** `resolve_build_base` runs on #605's guarded registry client, so the resolver and address policy, origin-bound authentication and the realm rules apply. It uses #609's native platform selection, so builds stay Docker Hub and linux/arm64 only. The older Docker Hub-only fetch helpers are dropped. - **#606 / #607 with #605 / #609 / #615:** the `build-image` and `reclaim-build-images` commands sit beside the image-policy and emulation options. The build error helpers take `impl Into<ErrorMessage>`. - **#607:** the native-build-retention TLA+ contract is registered beside the restart-policy, pool-emulation and multi-root contracts. Its model-coverage row now says an orphaned build client keeps the fence and per-key lock until it exits (`9f33a1d0`). - **#612 with batch 5:** - The builds-enabled classification flows through `classifyAdapted`, the Compose-backend refusal and doctor. - With the gate off, native `up` refuses a build with a message that names `HACK_NATIVE_BUILDS`, so doctor's basic-build advice names it too; one test expectation changed to match. - Startup and restart keep the dependency-file selection, host aliases, emulation policy and the inactive-profile skip alongside the build services. - **New (#609 x #612):** a build service whose adaptation selects amd64 emulation is refused before any effect, because native builds are linux/arm64 only. `prefer-native` passes, and a test covers it. - **#629:** the socket cleanup after a clean stop is kept inside #629's new guest stop. #629's two flaky relay tests are fixed on its branch (`a0e3e797`): each test gets its own fixture root, and EINVAL from `set_read_timeout` after the peer closes is accepted. - **`next` merge:** the last merge brings in Kit's CI shard and SIGKILL harness changes (#624, #627). The commit hook formatted `tests/native-compose-generation.test.ts` with Biome, a whitespace-only change to that Kit file. ## Validation on this branch (`6337e0ed`) - Clean CLI typecheck: pass. Biome on the 20 changed files: no errors. - `RUSTUP_TOOLCHAIN=1.97.1 bun run check:local`: pass. All-features clippy `-D warnings` (Zig 0.15.2): pass. - Cargo `--lib`: 1346 pass. `--features native-config-plan` `provider::graph`: 673 pass. Contracts: `project_contract` 66, `compose_fields_contract` 10, `interpolation_budget_contract` 1, `local_contract` 14. - `bun run test:models` (TLA+ 1.7.4): exit 0, 172 controls verified. - Focused bun tests, bounded runner, the changed test files: 242 pass, 0 fail. - `docs:prepare` + `docs:check`, `privacy:check`, gitleaks over `origin/next..HEAD`: pass. Host evidence: 11 of 11 builds passed on an M5 host with the relay (#629). Still open: the source-policy host check (a pinned build succeeds, and a base trigger that pulls an unpinned image or fetches https fails with "denied by policy") and the bake `--allow fs.read` / metadata shape on the pinned buildx 0.34.1. Both are listed in #606. After this merges, each component PR is closed with a link to the merge commit. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- codesmith:footer --> --- <a href="https://backend.blacksmith.sh/track/run-proof?expires=1796625833&installation_model_id=17053&pr_number=639&ref=codesmith_pr_footer&repository=hack-dance%2Fhack&return_to=https%3A%2F%2Fgithub.com%2Fhack-dance%2Fhack%2Fpull%2F639&variant=see_it_in_action&signature=c3ba624f6f34813ba26773c5b4c3caf941bd59515e8c3016c92df8f0e2a6048f"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/proof-see-it-in-action-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/proof-see-it-in-action-light.svg"><img alt="See it in action" src="https://pr-comments-assets.blacksmith.sh/codesmith/proof-see-it-in-action-dark-3x.png" width="132" height="28"></picture></a> <sup><a href="https://app.blacksmith.sh/hack-dance/codesmith/hack/pr/639?autoLogin=true&ref=codesmith_pr_footer">Open in [code]smith</a> · Need help on this PR? Tag <code>@codesmith-bot</code> with what you need.</sup> <!-- codesmith:autofix:disabled --> <!-- /codesmith:footer --> --------- Co-authored-by: Dimitri Kennedy <344842+roodboi@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
Merged into 🤖 Addressed by Claude Code |
![https://pr-comments-assets.blacksmith.sh/codesmith/proof-see-it-in-action-dark.svg"><source](https://pr-comments-assets.blacksmith.sh/codesmith/proof-see-it-in-action-dark.svg"><source){kind=link}
![https://pr-comments-assets.blacksmith.sh/codesmith/proof-see-it-in-action-light.svg"><img](https://pr-comments-assets.blacksmith.sh/codesmith/proof-see-it-in-action-light.svg"><img){kind=link}
Refs HACK-1924
Cause
The guest stops responding because of a busy loop in the pinned provider's host-side socket forwarding (SmolVM 1.14.3, its bundled libkrun vsock muxer on macOS). It is not a guest CPU or memory limit, and not a BuildKit or bake problem.
How it happens:
docker-buildx, docker driver) closes many engine connections before it has read the whole response, and closes the rest when it exits.kevent()change list, and that call stops at the first change that fails. Here the read filter is not registered, so the call fails withENOENTand the write filter is never deleted.EV_EOFon every wait. Each report queues another RST to the guest, signals an interrupt, and restarts the stream's 5-second reaper timer. The stream is never removed.no active session … context deadline exceeded,engine_protocol,agent_protocol_uncertain).Upstream libkrun fixed the delete in "utils/macos: delete each kqueue filter in its own kevent call (#198)". SmolVM v1.32.3 is the first release that ships that fix. This PR does not change the provider pin.
Fix
provider/image_build/client_relay.rs(new). The build client now reaches the engine through a private relay: a fresh mode-0700 socket directory under/private/tmp, removed when the build ends. The relay follows three rules:FINISH_GRACE(10 s) after the client exits.The relay works on any Docker client and does not inspect bytes. Build behaviour, receipts and the client environment are unchanged except for the
DOCKER_HOSTpath.Separate commit:
runtime downandruntime recovercan stop a guest that stopped responding. This applies when a guest step ofdownfails withagent_protocol_uncertain, or whenrecoverfinds the recorded provider alive. Both then ping the agent 3 times over about 13 s. If no ping gets a reply, they stop this home's recorded provider after verifying its PID, start time, user and executable at the signal. Nothing in the guest is quiesced, and the pool is recordedrecovered-unclean. A guest that answers keeps the clean stop, andrecoverstill refuses it.Third commit: relay directories carry their creating PID. A new relay removes a directory whose process no longer exists, but only when that directory is mode-0700, owned by this user, and holds nothing except the relay socket. Before this, a build process that was killed (for example by
a_killed_coordinator_leaves_its_client_fenced) left its relay directory behind.Fourth commit (review fix): a relay queue now drops bytes it has already delivered once they reach half the buffer, so its storage follows the pending bytes rather than the whole transfer. A regression test sends 128 MiB through a slow peer and checks that capacity stays at most 4×(limit + one read); the previous code grew to 128 MiB.
Fifth commit (review fix): a build drops the operation lease while its client runs and keeps only the shared build fence. So both silent-guest stop paths now check that fence while holding the lease:
recoverchecks before probing, and both paths check again just before signalling. If a build holds it, they refuse withprovider_busyand say to finish or cancel the build first; the next attempt then stops the silent guest. The fence is aflock, so it is released when the build's processes exit, including on SIGKILL. The existinga_killed_coordinator_leaves_its_client_fencedtest already proves that.Evidence (Apple Silicon host, load average 4–13, one VM at a time, isolated HOME/TMPDIR,
HACK_DAEMON_AUTOSTART_DISABLE=1)smolvm machine create --docker-socketVM with the guest Docker 29.5.2 engine. A 0-secondFROM scratchbuild succeeded, then VM CPU went to 240–310% and/_pingtimed out. With provider debug logging on, one forwarded stream logged about 3.57 millionprocess_event: HANG_UP/push_reset/deferring proxy removalevents in 94 s, starting right afterflush_tx: host send failed … Broken pipe. A hostsampleshows the vsock muxer thread busy inupdate_polling→Epoll::ctlandsignal_used_queue.kevent()call on an fd that has only the write filter registered and whose peer closed. The call returns-1 ENOENT, and the write filter keeps reportingEV_EOFon every wait. Deleting the write filter in its own call clears it.build:fixture,HACK_NATIVE_BUILDS=1 hack upto ready (cold build),hack down, thenruntime down. Result: 11 of 11 consecutive runs reached ready. Up took 23–28 s, and post-ready VM CPU was 0.5–14%.runtime downexited 0 in 2–3 s with phasestoppedand no provider process left, every time. No relay directories were left behind.runtime downalone stopped it: rc 0, phaserecovered-unclean, no provider left. It took 59 s, most of it waiting on the guest step that timed out.runtime recoveralone stopped it: rc 0 in 13 s,recovered-unclean.recoverrefused the responsive guest withrecovery_required.Validation
ffe2b3fd. The third commit,5643b8c9, changes only the cleanup of abandoned relay directories. A unit test covers it: an abandoned directory is removed, while a live process's directory and a directory with other files are kept. At5643b8c9,check:localpasses and the library tests pass (1188 passed, 105 ignored), with no relay directories left behind.RUSTUP_TOOLCHAIN=1.97.1 bun run check:local: pass (rustfmt check, plus Clippy on all targets with-D warnings).RUSTUP_TOOLCHAIN=1.97.1 bun run test:local -- --no-fail-fast, run from a short-path checkout of the head: exit 0. The library has 1187 passed and 105 ignored, and every integration binary passed, includingnode_contract(20 passed, 1 ignored). From the long scratch checkout, onlynode_contractfails, because its socket paths exceedsockaddr_un; that is a known path-length limit, not related to this change.gitleaks protect --stagedbefore each commit andgitleaks detect --log-opts=<base>..HEAD: no leaks.Remaining gaps
hack up, the nexthack upfor that project refuses withgraph_retained_startup(hack downreportsgraph_owner_recovery). The interrupted graph owner needs the existing graph recovery commands. The manual SIGTERM workaround leaves the same state. Tracked in HACK-1929.downpath can take about a minute, because it waits for the guest step that is running to time out before it probes.🤖 Generated with Claude Code