Uh oh!
There was an error while loading. Please reload this page.
🤖 feat: support podman as the docker runtime engine - #3931
Conversation
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
ibetitsmike
commented
Aug 23, 2026
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:a453d5445b
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…dently for devcontainers
ibetitsmike
commented
Aug 23, 2026
@codex review |
Codex Review: Didn't find any major issues. Bravo. 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". |
This comment has been minimized.
This comment has been minimized.
ibetitsmike
commented
Aug 23, 2026
@codex review |
Codex Review: Didn't find any major issues. Keep them coming! 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". |
This comment has been minimized.
This comment has been minimized.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…ration oversubscription) (coder#3939) ## Summary Fixes the three mechanisms that have been kicking fully validated PRs out of the merge queue: a unit-job Bun segfault caused by a desynced test-file exclusion list, integration-job cancellation caused by the job timeout colliding with jest's own test timeout, and integration test flakes caused by jest worker memory oversubscription on CI runners. ## Background Recent merge-queue evidence (all on green PRs whose diffs never touched the failing areas): 1. **Test / Unit segfault** (mq runs 32666269009, 32667158072, both kicked PR coder#3937): every test passes, then Bun 1.3.5 panics with `Segmentation fault` (exit 132). The logs show the crash is not teardown noise: it happens mid-`SandboxHostService` suite inside the monolithic coverage run. `src/node/services/sandbox/sandboxHostService.test.ts` was added to the `isolated_unit_tests` list in coder#3865 (QuickJS-heavy suites are known to crash Bun under coverage in a shared process) but was never added to the hand-maintained duplicate `find` exclusion list, so the suite ran a **second time** inside the shared process, where it segfaults intermittently. 2. **Test / Integration cancelled at the job cap** (mq runs 32632804824, 32639270365, 32640298486, kicking PRs coder#3931/coder#3930): the check-run annotation reads "The job has exceeded the maximum execution time of 10m0s". Healthy runs finish in ~4-5 min, but jest's integration `testTimeout` is 10 minutes (`jest.config.js`), so any single hung test makes jest wait right through the 10-minute job cap; the runner then kills the job and destroys all diagnostics. 3. **Recurring integration flakes** (`focus.test.ts` in run 32715740782 kicking PR coder#3932, `sendModeDropdown.test.ts` in run 32666269009, `anthropicCacheStrategy.test.ts` in run 32665306160, plus `analyticsEscape.test.ts` at 66s of its 60s budget in run 32719185534 kicking PR coder#3932's attempt 5, plus earlier `undo.test.ts`/`analyticsHeader.test.ts` kicks): all share one mechanism. CI runs jest with `--maxWorkers=100%`: 16 forked workers on the 16-core/64GB runner at ~4.7GiB peak each (per `scripts/lib/worker_budget.js`) oversubscribes memory (~75GiB peak) and thrashes. In the failing logs, suites that normally finish in seconds *pass* at 100-135s wall time while the only tests that *fail* are the ones with tight explicit timeouts (30s/45s), timing out in setup. The `--maxWorkers=100%` flag predates the repo's memory-aware worker budget (coder#1326 vs coder#3760), so CI never got the sizing fix that local runs already use. ## Implementation - **Unit job**: derive the `find` exclusions from the `isolated_unit_tests` array instead of maintaining a duplicate list, so the two can never drift again. This removes the double-run of the crashing suite (and fixes the whole desync class rather than one instance). No exit-code masking anywhere: a genuinely failing test still fails the job exactly as before. - **Integration job**: dropped the CLI `--maxWorkers` override so `jest.config.js`'s memory- and cgroup-aware `workerBudgetFor("jest")` sizes the pool (per Codex review; CLI flags take precedence over config), and `timeout-minutes: 10` → `20` so a hung test hits jest's 10-minute `testTimeout` and produces a real failure with logs instead of a log-destroying job cancellation. - **Worker recycling**: the first PR CI run under the bounded budget surfaced the complementary failure mode: integration suites leak memory across test files, so with fewer workers each one accumulates heap until it dies at V8's ~4GB cap (“Jest worker ran out of memory and crashed”, run 32720490232). `workerIdleMemoryLimit: "2GB"` recycles leaky workers between files. - **Tightest observed-killed test timeouts**: `focus.test.ts` 30s → 120s (aligned with its sibling `undo.test.ts`) `anthropicCacheStrategy.test.ts` 45s → 120s (peer provider tests already budget 45-150s per live call), and `analyticsEscape.test.ts` 60s → 120s after it died at 66s in run 32719185534. Other flagged tests already use the harness's 30s windows and should recover via the worker fix; they were left untouched to avoid churn. Live-API tests (e.g. `anthropicCacheStrategy`) are deliberately **not** excluded from `merge_group`: the mq run is the last gate that executes tests for the merged combination (main-push runs skip test jobs for the merge-queue bot), so quarantining them there would remove real coverage. The single observed kick was timeout-tuning, not provider outage. ## Validation - Red-green proof of the file-selection change: reproduced the old `find` invocation and the new derived one side by side; the old list contains `sandboxHostService.test.ts` (the double-run), the new list differs by exactly that one file (759 → 758, no other adds/removes). - `TEST_INTEGRATION=1 bun x jest tests/ui/review/focus.test.ts tests/ipc/providers/anthropicCacheStrategy.test.ts` passes locally (2/2, including the live Anthropic call). - `make lint-actions` (actionlint + zizmor) and `make static-check` pass. ## Risks CI-config only plus two test-timeout constants; no production code. The main tradeoff is failure-detection latency: a genuinely hung integration run is now killed at 20 min instead of 10 (but now with jest diagnostics at the 10-minute testTimeout), and the two touched tests take up to 120s before reporting a real hang. Suite wall time at 8 workers may shift slightly in either direction (less thrash, fewer lanes); healthy runs have ample headroom against the 20-minute cap either way. --- _Generated with `xum` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- xum-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
Summary
Lets the Docker runtime work with Podman: xum now resolves a Docker-compatible container CLI (
dockerfirst, thenpodman, or an explicitXUM_CONTAINER_CLIoverride) and uses it for every container operation instead of a hardcodeddockerbinary.Fixes#3077
Background
The Docker runtime shelled out to a literal
dockerinDockerRuntime.ts(~40 call sites), the runtime availability probe, and terminal sessions (embedded PTY and native). Hosts using Podman, a daemonless engine whose CLI is drop-in compatible with every subcommand we use (run/exec/inspect/start/stop/rm/cp), could not use the runtime at all.Implementation
src/node/runtime/containerCli.tsowns resolution:XUM_CONTAINER_CLIoverride wins (returned unprobed so failures surface from real commands); otherwisedockerthenpodmanare probed with<cli> info(5s timeout). A successful detection is cached for the process lifetime so one engine is used consistently; failures are not cached, so an engine started later is picked up. Docker-first ordering keeps dual-engine hosts stable.runDockerCommandnow prepends the resolved CLI; call sites pass arguments without the binary, so no invocation can hardcodedockeranymore. The CLI token is quoted for the actual host shell (cmd.exevs POSIX) sinceshell: trueruns cmd.exe on Windows, where POSIX single quotes are literal. Two host-side commands (git bundle create, local bundle cleanup) moved off that helper accordingly.TerminalService.openNative) exec into containers through the same resolver.checkRuntimeAvailabilityreports the docker runtime as available when either engine responds. Dev Containers get an independentdockerprobe because the devcontainer CLI shells out to the literaldockerbinary regardless of what the Docker runtime resolved (e.g. under anXUM_CONTAINER_CLIoverride).docker inspect'sno such objectstderr marker (used to distinguish missing containers from engine errors) was verified to hold on Podman:Error: error inspecting object: no such object: "x"(Podman 3.4.4).Out of scope: devcontainer Podman support (
--docker-paththreading), VS Code attached-container deep links for Podman, and persisting the chosen engine per workspace. An engine switching between sessions won't see the other engine's containers, the same class of behavior as stopping the Docker daemon today.Validation
containerCli.test.ts): docker preference, podman fallback, override handling, cache semantics.XUM_CONTAINER_CLI): fullDockerRuntimelifecycle passed, including create (collision check exercises theno such objectpath), provisioning, git-bundle sync, exec,ensureReadyrestart after an external stop, and delete with dirty checks. Re-run after each review-round change.podman ps, absent fromdocker ps; agent commands and terminal ran inside it). Podman cannot run in CI, so coverage there is unit-level only.Risks
Low-to-moderate: every Docker-runtime command path now resolves the CLI through one async helper. On docker-only hosts behavior is unchanged (detection picks
docker, then caches it). The main regression surface is the first command issued before any engine responds, which falls back todockerand fails with the same errors as before.Generated with
xum• Model:anthropic:claude-fable-5• Thinking:xhigh