feat: make a computer's PID limit optional - #4
Merged
Conversation
Found running supervisor against a rootless-Docker enclave (bit-mind CopilotKit#20): container creation for every computer fails outright on a daemon whose systemd cgroup driver cannot register a scope for a PID limit — an OCI runtime error ("systemd error: Interactive authentication required"), not a soft degradation. Reproduced with both runc and runsc (gVisor), and with COMPUTER_MEMORY_BYTES unset, isolating it to the unconditional `PidsLimit: options.pidsLimit ?? 512` in hostConfig(). `pidsLimit` on EnsureOptions is now `number | null`. Unset keeps today's behavior (512 default). Explicit `null` omits the field from the container's HostConfig entirely, so creation no longer depends on that cgroup path at all. Wired to a new COMPUTER_PIDS_LIMIT env var: unset leaves the default, set to the empty string disables the limit. This is a real reduction in defense-in-depth on a host that needs it, not a free choice — documented on the EnsureOptions field and the env var comment in index.ts that a deployment disabling it should compensate elsewhere (run timeouts, per-run concurrency ceilings, a watchdog killing leaked containers, host-level process-count alerting). Root cause (systemd cgroup driver + rootless Docker + D-Bus scope registration) is a separate, host-level infrastructure question, not fixed here — this unblocks the product path while that gets investigated. Two new integration tests in docker.integration.test.ts (against a real Docker daemon, following the file's existing pattern) cover both the unchanged default and the new omission. Co-Authored-By: Claude Sonnet 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
2 participants
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.
What
EnsureOptions.pidsLimitis nownumber | nullinstead ofnumber | undefined. Unset keeps the existing 512 default. ExplicitnullomitsPidsLimitfrom the container'sHostConfigentirely, wired to a newCOMPUTER_PIDS_LIMITenv var (unset = default, empty string = disabled).Why
Found running
supervisoragainst a rootless-Docker enclave (bit-mind CopilotKit#20, standing up the execution plane this repo's gateway relays into).POST /computers/{botId}/ensurefails outright — an OCI runtime error, not a soft degradation:Reproduced with both
runcandrunsc(gVisor) — identical error, so not gVisor-specific. Reproduced withCOMPUTER_MEMORY_BYTESunset too, isolating it to the unconditionalPidsLimit: options.pidsLimit ?? 512inhostConfig()— every computer this supervisor creates asks the daemon to register a PID-limited cgroup scope via its systemd driver, and that registration is what's failing. Confirmed the session D-Bus bus itself works fine when tested directly (busctl --user list,systemd-run --user --scopeboth succeed as the same user) — this is specifically the containerd-shim/runc subprocess's own D-Bus interaction in a rootless context, not a broken session.Switching to
cgroupdriver=cgroupfswas tried and rejected: it fails differently (cgroup.subtree_control: permission denied— the daemon isn't resolving its own delegated cgroup subtree), and rootless Docker's cgroup v2 support is designed around the systemd driver, so cgroupfs isn't the pragmatic fix.Scope
This unblocks the product path (a deployment on an affected host can disable the limit and proceed) without touching the actual root cause, which is host-level systemd/cgroup/D-Bus configuration — a separate, out-of-repo infrastructure question. Disabling this is a real reduction in defense-in-depth, documented on the field and the env var: a deployment that disables it should compensate with run timeouts, per-run concurrency ceilings, a watchdog for leaked containers, and host-level process-count alerting.
Tests
Two new integration tests in
supervisor/tests/docker.integration.test.ts(real Docker daemon, following the file's existing pattern): the unchanged 512 default, and explicitnullproducing no limit. Both pass against this host's Docker daemon.bunx biome format/lintclean,bun run typecheck(supervisor) clean,bun test supervisor/tests/docker.integration.test.ts→ 7 pass / 0 fail.🤖 Generated with Claude Code