91 lines
4.6 KiB
Markdown
91 lines
4.6 KiB
Markdown
# PR#123: The Codex Runner Checks Its Sandbox before It Reviews
|
|
|
|
Version: minor
|
|
|
|
> **WARNING:** This document describes only the change applied in this PR.
|
|
> It may already be outdated once the next PR is merged.
|
|
> Historic PR documentation is not maintained along with new PRs; treat it as a snapshot.
|
|
|
|
## Related Links
|
|
|
|
- [PR#88](2026-10-11-PR#88-codex-runner.md): the Codex runner and its `bwrap` jail.
|
|
|
|
## Problem
|
|
|
|
The Codex runner runs Codex in a `bwrap` jail, and Codex starts its own `bwrap` sandbox inside it.
|
|
On 2026-10-16 at 19:53 the AppArmor profile `bwrap` was unloaded on the workstation, while `bwrap-userns-restrict` was disabled.
|
|
Until it was loaded again on 2026-10-17 at 12:38, `bwrap` fell under `unprivileged_userns`
|
|
and failed with "bwrap: setting up uid map: Permission denied".
|
|
|
|
Each attempt for PR#115 and PR#116 failed this way and counted against `MAX_ATTEMPTS`; PR#115 waited 16.6 hours.
|
|
With `bwrap-userns-restrict` active, the nested `bwrap` fails too,
|
|
and Codex rates a PR critical because its shell sandbox failed.
|
|
A later update of the AppArmor package can bring either fault back.
|
|
|
|
## Non-Goals
|
|
|
|
- No change of the jail or of the AppArmor profiles of the workstation.
|
|
- No detection of a failed sandbox inside a Codex answer; the check before the run covers it.
|
|
|
|
## Specification
|
|
|
|
### Feature: the Codex runner reviews only with a working sandbox
|
|
|
|
#### Scenario#123.01: a broken sandbox stops the reviews of a run
|
|
|
|
… so that a fault of the workstation neither uses up the attempts of a PR nor produces a wrong verdict.
|
|
|
|
- **Given** a ready PR, and a nested `bwrap` that fails on the workstation
|
|
- **When** the runner runs
|
|
- **Then** the PR gets no review, its attempts stay unchanged, the log names the cause, and the run exits with 1
|
|
|
|
##### Verified by
|
|
|
|
- [test_codexrunner.CodexRunnerTest.test_a_broken_sandbox_stops_the_reviews](../../tests/test_codexrunner.py)
|
|
- [test_codexrunner.CodexRunnerTest.test_the_run_exits_with_1_after_a_sandbox_fault](../../tests/test_codexrunner.py)
|
|
- [test_codex.CodexTest.test_the_sandbox_check_runs_a_bwrap_inside_the_jail](../../tests/test_codex.py)
|
|
|
|
#### Scenario#123.02: a jail that fails during a review costs no attempt
|
|
|
|
… so that a fault that starts after the check does not count against the PR either.
|
|
|
|
- **Given** a ready PR, a check that passes,
|
|
and a review whose `bwrap` fails with "bwrap: setting up uid map: Permission denied"
|
|
- **When** the runner runs
|
|
- **Then** the attempts of the PR stay unchanged, and the run exits with 1
|
|
|
|
##### Verified by
|
|
|
|
- [test_codexrunner.CodexRunnerTest.test_a_jail_fault_in_a_review_costs_no_attempt](../../tests/test_codexrunner.py)
|
|
- [test_codex.CodexTest.test_only_a_failed_bwrap_is_a_jail_fault](../../tests/test_codex.py)
|
|
|
|
## Solution
|
|
|
|
- `codex.check_sandbox` runs `bwrap --ro-bind / / true` inside the jail that `codex.jail` builds for a review,
|
|
with the same environment; it returns the cause, or `None`.
|
|
- `Runner.run` calls the check first.
|
|
On a failure, it logs the cause with the next steps, reviews nothing in this run and returns `False`;
|
|
`tools/codex-runner` then exits with 1, and the systemd unit shows as failed.
|
|
- A review whose `bwrap` fails, i.e. a non-zero exit with an error that contains "bwrap: ",
|
|
counts no attempt and stops the reviews of the run in the same way.
|
|
- The runner still collects the PRs while the sandbox fails:
|
|
a PR with a Codex comment gets its gate status, the others keep `pending` "waiting for the Codex review".
|
|
- Tests inject the check; a `Runner` takes it as `sandbox_check`.
|
|
- README "Codex runner" and the plan describe the check.
|
|
- Version 0.51.0.
|
|
|
|
## Open Questions
|
|
|
|
- [x] Exit code 0 or 1 on a failed check?
|
|
1: the systemd unit then shows as failed, which `systemctl --user --failed` and the desktop show.
|
|
A failed oneshot service does not stop a timer with `OnUnitInactiveSec`;
|
|
a test with `systemd-run` fired 3 failed runs in 12 s.
|
|
- [x] Does a PR keep "waiting for the Codex review" while the sandbox fails?
|
|
Yes: the cause lies on the workstation, not in the PR, and the failed unit shows it to the human.
|
|
A gate PR stays held, as with a workstation that is off.
|
|
- [x] Does a review whose `bwrap` fails count as an attempt?
|
|
No: the jail fails before Codex starts, so the review uses no quota, and the PR is not at fault.
|
|
A PR cannot bypass the gate this way: a run that fails like this posts no comment, so the PR stays `pending`.
|
|
- [x] Does the check detect a failed sandbox inside a Codex answer, e.g. a verdict "critical" because the shell sandbox failed?
|
|
No; the check runs the nested `bwrap` before the review, which is the cause of such an answer.
|