fix(sandbox): bound web E2E evidence and service logs - #1287
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
39e30cb
into
codex/pr931-bounded-subprocess-core-20260824
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if _services_output_limited(services): | ||
| output_limited = True | ||
| if exit_code == 0: | ||
| exit_code = bounded_subprocess.OUTPUT_LIMIT_EXIT_CODE | ||
| if not service_limit_reported: | ||
| print( | ||
| "sandboxed-web-e2e: service output exceeded " | ||
| f"{args.service_log_limit_bytes} bytes", | ||
| file=sys.stderr, | ||
| ) |
There was a problem hiding this comment.
📝 Info: Late-overflow exit-code precedence holds
In main's finally, a late service overflow raises the code to 123 only when exit_code == 0 (scripts/ci/sandboxed_web_e2e.py:496), so a nonzero E2E, readiness, or timeout result stays authoritative while output_limited still records the overflow. Consistent with the documented contract and the tests.
Was this helpful? React with 👍 or 👎 to provide feedback.
| @@ -135,41 +237,59 @@ def wait_for_url(url: str, timeout: int, service: Service) -> bool: | |||
| return False | |||
There was a problem hiding this comment.
📝 Info: 5xx readiness responses re-poll without backoff
A 5xx response in wait_for_url neither returns ready nor sleeps, so it re-polls immediately until the deadline (scripts/ci/sandboxed_web_e2e.py:231-236). Each pass still issues a real 2-second request, so it is not a tight spin. Pre-existing behavior; the PR only added the overflow guard to this loop.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| def service_output_limited(service: Service) -> bool: | ||
| """Return whether one service exceeded its declared combined log budget.""" | ||
| if service.capture is not None: | ||
| return service.capture.output_limited | ||
| return ( | ||
| service.log_path.exists() | ||
| and service.log_path.stat().st_size > service.log_limit_bytes | ||
| ) |
There was a problem hiding this comment.
📝 Info: Service log file materializes only at stream close
The bounded service log is written only when the stream closes (_write_destination in the drain thread, scripts/ci/bounded_subprocess.py:222-234), so it is absent while the service runs. Overflow detection uses in-memory capture.output_limited and tail_text runs after stop_service joins the capture, so the read ordering is sound.
Was this helpful? React with 👍 or 👎 to provide feedback.
| except CommandExecutableNotFoundError: | ||
| print( | ||
| "sandboxed-web-e2e: install each executable or correct command PATH", | ||
| file=sys.stderr, | ||
| ) | ||
| exit_code = sandboxed_verify.COMMAND_NOT_FOUND_EXIT_CODE | ||
| except CommandNotExecutableError: | ||
| print( | ||
| "sandboxed-web-e2e: select executable files or correct their permissions", | ||
| file=sys.stderr, | ||
| ) | ||
| exit_code = sandboxed_verify.COMMAND_NOT_EXECUTABLE_EXIT_CODE | ||
| except bounded_subprocess.OutputLimitUnsupportedError: | ||
| output_limit_unsupported = True | ||
| print( | ||
| "sandboxed-web-e2e: bounded child output is unavailable on this platform", | ||
| file=sys.stderr, | ||
| ) | ||
| exit_code = bounded_subprocess.OUTPUT_LIMIT_EXIT_CODE | ||
| except sandboxed_verify.RepositoryPathBoundaryError: | ||
| path_boundary_rejected = True | ||
| copied_repo = Path("(not-created)") | ||
| print( | ||
| "sandboxed-web-e2e: repository path boundary rejected", | ||
| file=sys.stderr, | ||
| ) | ||
| exit_code = sandboxed_verify.PATH_BOUNDARY_EXIT_CODE | ||
| except sandboxed_verify.RepositoryRootError: | ||
| copied_repo = Path("(not-created)") | ||
| print( | ||
| "sandboxed-web-e2e: repository root is not a directory", | ||
| file=sys.stderr, | ||
| ) | ||
| exit_code = 1 | ||
| except RuntimeError: | ||
| print( | ||
| "sandboxed-web-e2e: bounded output capture failed", | ||
| file=sys.stderr, | ||
| ) | ||
| exit_code = bounded_subprocess.OUTPUT_LIMIT_EXIT_CODE |
There was a problem hiding this comment.
📝 Info: RuntimeError handler ordering is safe
The specific CommandExecutableNotFoundError, CommandNotExecutableError, and OutputLimitUnsupportedError handlers (all RuntimeError subclasses) precede the catch-all except RuntimeError at scripts/ci/sandboxed_web_e2e.py:467, so each maps to its intended exit code and the catch-all only absorbs the generic missing-pipe/capture failures as 123.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Recreated from the preserved #1282 work after its deleted stacked base was merged into #1280. This layer adds bounded backend/frontend service capture, bounded E2E output, readiness URL validation, stable launch errors, cleanup evidence, and regression coverage.\n\nStack: #1280 -> this PR. Local evidence on the merge-result tree: 59 focused tests passed; full suite 1,478 passed, 1 skipped, 16 subtests; interrogate 100%; compileall and git diff --check passed.