CI: isolate container environment (thin wrapper + environment-independent test runner)

[hguillen]

Summary

Stacked on top of #25 (hguillen/ci-setup), a sibling to #33. Refactors the CI test path into three clean layers so the container run no longer depends on the runner's home directory, and the test logic lives in one environment-independent place.

• docker/run_docker.sh — isolated mode. Set ISAAC_AUTODATA_ISOLATED=1 to skip the dev-only host bind-mounts (shared $HOME/.cache and X11). Those couple the container to the runner's home directory, whose ownership collides with the recreated container user and broke warp's ~/.cache/warp. Local dev is unchanged (mounts on by default).
• scripts/ci/run_tests.sh — new, environment-independent test logic. Runs inside the container (or any env with the deps). Takes test paths positionally, reads PYTEST_MARK / ISAAC_AUTODATA_SUBPROCESS_TIMEOUT from the environment, and pins caches to a writable run-local dir. Single home for the pytest invocation.
• scripts/ci/run_e2e_tests.sh — now a thin wrapper. Owns only environment setup: picks the image (-c, -r on FORCE_REBUILD), runs isolated by default, and delegates to run_tests.sh. .github/workflows/ci.yml and nightly.yml call it exactly as before, so no workflow changes are needed.

This supersedes the minimal cache-redirect fix on #25 (the redirect now lives inside run_tests.sh) and additionally removes the host mount that caused the collision in the first place.

Test plan

• Premerge test_e2e runs the e2e/ + interfaces/ suites green with no ~/.cache/warp PermissionError.
• Nightly test_full (FORCE_REBUILD=true, no marker) runs the whole tree.
• Local ./scripts/ci/run_e2e_tests.sh reproduces the run; ISAAC_AUTODATA_ISOLATED=0 restores the shared-cache/X11 dev behavior.

Summary by CodeRabbit

• New Features

  • Added an isolated container mode (via ISAAC_AUTODATA_ISOLATED) that skips host cache and X11 coupling when enabled.
  • Introduced a container-first test runner that supports passing test paths, optional pytest markers, in-container cache location, and subprocess timeout settings.

• CI Improvements

  • Updated the host test entrypoint to delegate all pytest execution to the new in-container script, forwarding relevant environment overrides.
  • Simplified rebuild logic while keeping the existing FORCE_REBUILD behavior.

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes

Containerized CI tests

Layer / File(s)	Summary
CI wrapper delegation scripts/ci/run_tests.sh	The host wrapper resolves test paths, forwards environment overrides, applies isolated mode defaults, and delegates test execution to the container.
Docker isolated-mode handling docker/run_docker.sh	Cache and X11 mounts, along with xhost setup, are skipped when isolated mode is enabled.
In-container pytest runner scripts/ci/run_tests_in_container.sh	The new runner configures cache locations, forwards subprocess settings, assembles pytest arguments, and executes pytest inside the container.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CI
  participant run_tests.sh
  participant run_docker.sh
  participant run_tests_in_container.sh
  participant pytest
  CI->>run_tests.sh: provide test paths and environment overrides
  run_tests.sh->>run_docker.sh: start container with forwarded environment
  run_docker.sh->>run_tests_in_container.sh: execute container test runner
  run_tests_in_container.sh->>pytest: exec pytest with paths and marker

Loading

Suggested reviewers: peterd-nv, njawale42

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: isolating the CI container and refactoring test execution.
Description check	✅ Passed	The description covers the change, impact, and test plan, with only a minor mismatch from the template heading names.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch hguillen/isolate-ci-env

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docker/run_docker.sh`:
- Around line 133-139: Update the ISAAC_AUTODATA_ISOLATED parsing case around
ISOLATED so recognized false values such as 0 and false explicitly remain
non-isolated, recognized true values still enable isolation, and any other
non-empty value exits with an error instead of defaulting to ISOLATED=false.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 16c56607-87d5-472b-8d76-2c00e20811f5

📥 Commits

Reviewing files that changed from the base of the PR and between 8b60cba and 9b91b59.

📒 Files selected for processing (3)

• docker/run_docker.sh
• scripts/ci/run_tests.sh
• scripts/ci/run_tests_in_container.sh

[coderabbitai]

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Fail closed on invalid ISAAC_AUTODATA_ISOLATED values.

Unrecognized values currently leave ISOLATED=false, so a typo such as ture silently re-enables $HOME/.cache and X11 mounts. Reject invalid values while preserving explicit 0/false local-dev behavior.

Proposed validation

 ISOLATED=false
-case "${ISAAC_AUTODATA_ISOLATED:-}" in
-    1 | true | TRUE | yes) ISOLATED=true ;;
+value="${ISAAC_AUTODATA_ISOLATED:-}"
+case "${value,,}" in
+    1 | true | yes) ISOLATED=true ;;
+    "" | 0 | false | no) ;;
+    *) echo "Invalid ISAAC_AUTODATA_ISOLATED: ${value}" >&2; exit 2 ;;
 esac

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	# Isolated mode (ISAAC_AUTODATA_ISOLATED=1, e.g. CI): skip the dev-only host
	# mounts ($HOME/.cache, X11) whose ownership collides with the container user.
	ISOLATED=false
	case "${ISAAC_AUTODATA_ISOLATED:-}" in
	1 | true | TRUE | yes) ISOLATED=true ;;
	esac
	# Isolated mode (ISAAC_AUTODATA_ISOLATED=1, e.g. CI): skip the dev-only host
	# mounts ($HOME/.cache, X11) whose ownership collides with the container user.
	ISOLATED=false
	value="${ISAAC_AUTODATA_ISOLATED:-}"
	case "${value,,}" in
	1 | true | yes) ISOLATED=true ;;
	"" | 0 | false | no) ;;
	*) echo "Invalid ISAAC_AUTODATA_ISOLATED: ${value}" >&2; exit 2 ;;
	esac

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docker/run_docker.sh` around lines 133 - 139, Update the
ISAAC_AUTODATA_ISOLATED parsing case around ISOLATED so recognized false values
such as 0 and false explicitly remain non-isolated, recognized true values still
enable isolation, and any other non-empty value exits with an error instead of
defaulting to ISOLATED=false.