Repository navigation
🤖 tests: list bug-bash sandbox crash leftovers and add a recover target - #6072
Conversation
|
@codex review |
Codex Review SummaryThis 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. |
🛡️ 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. |
![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: cc326ab1bf
ℹ️ 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".
cc326ab to
fa63d84
Compare
|
@codex review |
🛡️ 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62c1a95133
ℹ️ 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".
62c1a95 to
57e3608
Compare
|
@codex review |
🛡️ 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57e3608887
ℹ️ 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 review |
|
Codex Review: Didn't find any major issues. Hooray! 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. |
…er target PR C2 of the #5714 sandbox plan (#5882, plan item 11), stacked on PR C. - Every launch lists the job containers of its checkout (by the checkout label) after `docker info`, with the owner state dead, alive or cannot tell. It removes nothing. - `make bug-bash-sandbox-recover` (`launch.ts --recover`) prints the Docker endpoint, then removes only containers whose name matches this checkout's job pattern and whose owner is dead: same boot and PID namespace, and a PID that is gone or has another start time. Removal is by ID, after the same name and label check as cleanup. Everything else is listed and left. Exit 3 when a removal cannot be proved. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$46.10`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=46.10 -->
- A launch lists the leftovers right after the Docker preflight, before it prepares the image, so a launch whose pull fails still shows them. - listJobs() ignores a container only when inspect says "No such container"; any other inspect failure refuses, so recover cannot exit 0 past an unseen container. - recover() checks the stop before each removal: a removal that started finishes, and no new one starts. - ownerState() counts a zombie launcher (state Z or X) as dead. - recover() returns 3 when the session cleanup state is unknown. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$52.06`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=52.06 -->
--- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$52.06`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=52.06 -->
Codex review round 2: a child that exits at once can be reaped by the shell before the shell execs `sleep 30`, and the test then waited forever. The child now exits 300 ms later, after the exec, and the wait is bounded with a clear assertion. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$52.06`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=52.06 -->
- runner.test.ts: the zombie-launcher test now releases the child through a FIFO only after the parent shell became `sleep 30`, instead of a fixed `sleep 0.3` that a descheduled shell could lose (Codex r3). - runner.ts: #endJob waits at most 10 s for each onStop hook, then logs and closes the lifeline anyway, so a hung hook (B1's proxy close) cannot block cleanup. Test: a never-settling hook still lets stdin close and cleanup finish. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
d9faf63 to
2bd7b63
Compare
|
@codex review |
🛡️ 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. |
|
Merge note: the maintainer approved a narrow exception to the "Codex approval on the final head" rule for this PR only.
The exception covers this unchanged patch only. It is not a general rule. Generated with |
1 participant
Summary
Lists the job containers that crashed launches left behind, and adds
make bug-bash-sandbox-recoverto remove the ones whose launcher is gone. This is PR C2 of the plan to run the model-driven bug bash in the sandbox, stacked on #6069 (C). It must land before B1, which activates model-driven jobs.Refs #5714
Fixes #5882
What changes
xum.bugbash.checkoutlabel, right afterdocker info. Each line shows the owner state:dead,aliveorcannot tell. The launch removes nothing.make bug-bash-sandbox-recover(launch.ts --recover) prints the Docker endpoint first. It removes a container only when both hold:^xbb-<checkout6>-[0-9a-f]{6}$.Real-Docker dogfood (Docker 27.5.1)
I created four test containers with names that only this check uses, then ran a launch (contract case (a) of #6069) and the recover target twice.
xbb-0b6a51-c2deadowner deadxbb-0b6a51-c2a11esleepowner alivexbb-0b6a51-c2livesleepowner alivexbbprobe-c2-otherThe launch still ended
cleanup: removedfor its own job container, and both recover runs exited 0. I removed the two remaining test containers by their exact IDs afterwards.1-launch.log
2-recover.log
4-recover-again.log
Validation
launch.tshad noownerStateorrecoverexport.ownerState(dead, alive, a reused PID, another boot, another PID namespace, malformed labels); a launch lists every leftover and removes nothing; recover removes only dead owners with a job name, by ID, and pulls or runs nothing; recover exits 3 when a removal cannot be proved.tests/bugbash/sandbox/pass on Bun 1.3.12.listJobs(), survived because it was redundant: thedocker pslabel filter already selects the checkout, andremoveJob()checks the name and both labels again before any removal. I removed it.Risks
Test tooling only. A wrong owner state could remove a container whose launcher still runs. That needs a container with this checkout's label and job name, on this boot and PID namespace, whose recorded PID start time no longer matches a running process.
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$51.47