Skip to content

🤖 tests: follow-ups for the bug-bash sandbox runner library (#5875) #5877

Description

@ThomasK33

Follow-ups from the final readiness check of #5875 (merged in abfa4e32d5), the runner library of the bug-bash sandbox (#5714, B1 PR 2). None of them blocked that merge, because the library has no entry point yet. Items 1 and 2 must be fixed in B1 PR 3, before PR 3 adds the launch command.

  1. Process group. Session.#stop and the per-command timeout signal only the direct child (tests/bugbash/sandbox/runner.ts, #stop and #spawn). Grandchildren, such as the git and grep that build.sh --key starts, can outlive a stop and keep stdout open. Fix: spawn each child as a group leader and signal the group.
  2. cleanup(job) keeps the first call's job. cleanup() stores the promise of its first call, so an early cleanup() without a job makes a later cleanup(job) skip the container removal. Fix: register the job before the container starts, or assert that every call passes the same job.
  3. "removed" with no client. When a job is given but preflight never connected, cleanup returns "removed" without checking Docker. No container can exist then, but a separate state (for example "none") is more accurate.
  4. Private config folder leak. mkdtempSync in #connect can run after cleanup has finished, and that folder is then never removed. Fix: check the stop state before creating the folder, or create it inside cleanup's ownership.
  5. The name filter is a regex. --filter name=^/<name>$ treats . and other regex characters as patterns. The two label filters still limit the match. PR 3 must validate the container name format.

Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high • Cost: $115.92

Activity

  1. self-assigned this
    on Oct 8, 2026
  2. ThomasK33 commented on Oct 8, 2026

    @ThomasK33
    MemberAuthor

    Follow-ups from the #5878 final check (non-blocking):

    1. The item-4 test also passes without the new stop check before mkdtempSync: the check guards future changes, the test does not prove it.
    2. A group that outlives its leader is re-probed only at the next stop or cleanup; a PID wrap in between could signal a new same-user group (very low risk; pidfd would close it).
    3. EPERM counts as "gone": a group left with only another user's setuid processes would look empty (no setuid commands run here).
    4. Pre-existing, unreachable today: a setsid grandchild holding the pipe hangs await children; two concurrent ensureImage() calls leak one temp folder.
  3. added a commit that references this issue on Oct 8, 2026
    7ba0f93
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions