From f5da343d9abd8d5c4359e10eebecf90e7d273dfb Mon Sep 17 00:00:00 2001 From: Lilly Luo Date: Sat, 19 Sep 2026 02:57:52 +0000 Subject: [PATCH] Bound managed Codex state snapshots and integration deadlines --- .github/workflows/integration.yml | 4 +- scripts/run_integration.py | 29 +++++- tests/README.md | 6 ++ tests/integration/README.md | 16 +++- .../test_ug_codex_managed_model_discovery.py | 21 ++--- tests/integration/utils/managed.py | 29 ++++++ tests/test_integration_runner.py | 24 +++++ tests/test_integration_snapshots.py | 94 +++++++++++++++++++ 8 files changed, 206 insertions(+), 17 deletions(-) create mode 100644 tests/test_integration_runner.py create mode 100644 tests/test_integration_snapshots.py diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index bd7e5d55..1a82b6b9 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -198,6 +198,7 @@ jobs: # runner-reachable ca-central, but leave these lanes for signal until the managed-config apply # path is proven stable. Then drop this and add `managed` to the required set. continue-on-error: true + # Test/suite deadlines below leave time for teardown and evidence upload before a job kill. timeout-minutes: 30 strategy: fail-fast: false @@ -234,9 +235,10 @@ jobs: if [[ -n "$DEPENDENCY" ]]; then args+=(--dependency "$DEPENDENCY") fi - uv run --no-project --python 3.12 python scripts/run_integration.py \ + /usr/bin/time -v uv run --no-project --python 3.12 python scripts/run_integration.py \ --python 3.12 --ug-version "$UG_VERSION" --entry-point "$ENTRY_POINT" \ --default-index "$PACKAGE_INDEX" --output "$RUNNER_TEMP/ug-integration" \ + --test-timeout 300 --suite-timeout 1200 \ "${args[@]}" -- -m "(managed or managed_fixture) and $AGENT" - name: Upload managed test evidence if: ${{ !cancelled() }} diff --git a/scripts/run_integration.py b/scripts/run_integration.py index b7b418e3..fb41d76f 100644 --- a/scripts/run_integration.py +++ b/scripts/run_integration.py @@ -28,6 +28,13 @@ AGENT_PACKAGES = {"claude": "@anthropic-ai/claude-code", "codex": "@openai/codex"} +def positive_seconds(value: str) -> int: + seconds = int(value) + if seconds <= 0: + raise argparse.ArgumentTypeError("Timeout must be greater than zero seconds.") + return seconds + + def mint_m2m_token(workspace: str, client_id: str, client_secret: str) -> str: """Mint a short-lived workspace token for a service principal via OAuth client credentials. @@ -134,6 +141,12 @@ def arguments(): parser.add_argument("--workspace", default=os.environ.get("UCODE_TEST_WORKSPACE")) parser.add_argument("--output", type=Path, help="New results directory; never reused.") parser.add_argument("--installation-only", action="store_true", help="No workspace calls.") + parser.add_argument( + "--suite-timeout", type=positive_seconds, default=3600, help="Pytest deadline in seconds." + ) + parser.add_argument( + "--test-timeout", type=positive_seconds, help="Optional per-case deadline in seconds." + ) parser.add_argument( "pytest_args", nargs=argparse.REMAINDER, help="After --, pass pytest filters." ) @@ -301,6 +314,7 @@ def run(command, *, cwd=output, env=base_env, timeout=600) -> str: }, "platform": platform.platform(), "installation_only": args.installation_only, + "timeouts": {"suite": args.suite_timeout, "test": args.test_timeout}, } manifest = output / "versions.json" exitcode = 1 @@ -519,6 +533,7 @@ def run(command, *, cwd=output, env=base_env, timeout=600) -> str: "--default-index", args.default_index, "pytest==9.0.3", + "pytest-timeout==2.4.0", "pexpect==4.9.0", "pyte==0.8.2", ] @@ -552,6 +567,16 @@ def run(command, *, cwd=output, env=base_env, timeout=600) -> str: report["suite_sha256"] = suite_hash.hexdigest() extra = args.pytest_args report["pytest_args"] = extra + timeout_args = ( + [ + f"--timeout={args.test_timeout}", + "--timeout-method=signal", + "-o", + f"faulthandler_timeout={max(1, args.test_timeout // 2)}", + ] + if args.test_timeout + else [] + ) manifest.write_text(redact(json.dumps(report, indent=2)) + "\n") print("Running integration tests against the installed package.", flush=True) with managed_process( @@ -567,6 +592,8 @@ def run(command, *, cwd=output, env=base_env, timeout=600) -> str: "-o", f"cache_dir={output / 'pytest-cache'}", f"--junitxml={output / 'junit.xml'}", + "--durations=10", + *timeout_args, *extra, ], env=runtime_env, @@ -574,7 +601,7 @@ def run(command, *, cwd=output, env=base_env, timeout=600) -> str: stdin=subprocess.DEVNULL, interrupt=True, ) as result: - result.wait(timeout=3600) + result.wait(timeout=args.suite_timeout) exitcode = result.returncode junit = output / "junit.xml" if junit.is_file(): diff --git a/tests/README.md b/tests/README.md index 5c671fdd..a0097cdd 100644 --- a/tests/README.md +++ b/tests/README.md @@ -79,6 +79,12 @@ argument spelling or routing mode, never hides the agent/provider in the test na are incorporated into the Databricks configuration TUI journeys. Generated-file cleanup and strict app-server stdout assertions remain enforced. +Codex source-override cases preserve persistent ug/agent files using streamed SHA-256 +fingerprints and symlink targets. Only the fetched managed-config cache and Codex's +disposable `tmp/arg0` helper links are excluded: even a version probe rotates those links. +`test_integration_snapshots.py` covers bounded memory, symlink handling, and detection of +persistent file additions, changes, and removals. No managed Codex cases are skipped. + ug no longer runs a post-configure agent probe; the deprecated `--skip-validate` flag is accepted as a no-op where older journeys still pass it. Tests retain `--skip-upgrade` as a deprecated no-op too; UG only upgrades agents below its diff --git a/tests/integration/README.md b/tests/integration/README.md index 8d4fc73e..4812a3a3 100644 --- a/tests/integration/README.md +++ b/tests/integration/README.md @@ -224,6 +224,13 @@ inspection; remove that run directory when finished. Agent versions are checked before and after the suite so an automatic upgrade cannot silently change the combination being tested. Model requests and subprocesses have deadlines, and the process group is cleaned up after each command. +Managed CI additionally sets `--test-timeout 300 --suite-timeout 1200`: pytest dumps +stacks halfway through the per-case budget, fails an over-budget case after five minutes, +and the runner stops the suite after twenty minutes. Both deadlines precede the +thirty-minute job limit so teardown and evidence upload can run. The runner records +these budgets and reports the ten slowest test phases. Other suites retain their +existing process deadlines and one-hour suite budget unless explicitly overridden. +Managed job logs also report peak resident memory through `/usr/bin/time -v`. Selection after `--` accepts `-k`, `-m`, `-x`, and `--maxfail`; configuration and report paths cannot be overridden. `--installation-only` always restricts the selection to installation checks, including when additional filters are used. @@ -299,7 +306,14 @@ module fetches the workspace's published config once, replaces Claude's static m across all configured/fresh scenarios. The Codex module does the same with `main.default.ci_e2e_openai_mps`. The tests verify Claude's admin header, native cache and real model picker, Codex's exact app-server catalog, and both agents' rejection of personal source -overrides. In addition, +overrides. The Codex override cases compare persistent files with streamed SHA-256 fingerprints and +record symlinks without following them. They exclude only ug's fetched managed-config cache +and `.codex/tmp/arg0`, where Codex 0.154.0 rotates executable helper links even during +`--version`. This keeps the configuration-preservation assertion without reading several +copies of the agent binary into memory or rendering binary contents in assertion failures. +All configured/fresh override cases remain enabled. + +In addition, `test_ug_configure_managed_codex_catalog_fallback` injects the intentionally nonexistent `system.ai.gpt-99`, keeping it out of the real workspace while launching Codex through that workspace on the valid default model `system.ai.gpt-5-6-sol`. With smart routing enabled, it opens diff --git a/tests/integration/test_ug_codex_managed_model_discovery.py b/tests/integration/test_ug_codex_managed_model_discovery.py index efa50080..e9e3709b 100644 --- a/tests/integration/test_ug_codex_managed_model_discovery.py +++ b/tests/integration/test_ug_codex_managed_model_discovery.py @@ -12,8 +12,8 @@ import pytest from utils.constants import MANAGED_CODEX_PROVIDER_SERVICE from utils.managed import ( + codex_state_snapshot, fetch_managed_config_stub, - is_managed_config_control_plane_cache, use_managed_config_stub, ) @@ -38,18 +38,7 @@ def _managed_codex_config(live_session, _managed_codex_config_stub): def _codex_state_and_agent_files(session): - paths = [] - for directory in (session.home / ".ucode", session.home / ".codex"): - if directory.exists(): - paths.extend(path for path in directory.rglob("*") if path.is_file()) - return { - str(path.relative_to(session.home)): path.read_bytes() - for path in paths - if path.is_file() - # A fresh launch must retrieve and cache the control-plane input before it can reject an - # override. Exclude only that expected cache; every agent-owned state/file stays compared. - and not is_managed_config_control_plane_cache(session.home, path) - } + return codex_state_snapshot(session.home) def _assert_rejected_before_codex_started(session, result, before=None): @@ -58,7 +47,11 @@ def _assert_rejected_before_codex_started(session, result, before=None): assert "`--provider` or `--model-location` is not allowed" in output assert "managed config exists for the workspace" in output if before is not None: - assert _codex_state_and_agent_files(session) == before + after = _codex_state_and_agent_files(session) + changed = sorted( + name for name in before.keys() | after.keys() if before.get(name) != after.get(name) + ) + assert not changed, f"Rejected override changed persistent files: {changed}" def _assert_managed_provider_catalog(session, models): diff --git a/tests/integration/utils/managed.py b/tests/integration/utils/managed.py index 423ebcf8..4a34c02c 100644 --- a/tests/integration/utils/managed.py +++ b/tests/integration/utils/managed.py @@ -6,6 +6,7 @@ enforces that boundary). """ +import hashlib import json import urllib.request from pathlib import Path @@ -78,6 +79,34 @@ def is_managed_config_control_plane_cache(home: Path, path: Path) -> bool: return path == home / ".ucode" / "managed-config.json" +def codex_state_snapshot(home: Path) -> dict[str, tuple[str, str]]: + """Fingerprint persistent ug/Codex files without retaining or following binary helpers. + + Even `codex --version` rotates tmp/arg0 links to its executable. Those disposable + bootstrap files are not configuration; following each link can retain hundreds of MB + and make pytest render a huge binary diff when the random paths change. + """ + snapshot = {} + + def visit(path: Path) -> None: + if path == home / ".codex/tmp/arg0" or is_managed_config_control_plane_cache(home, path): + return + name = str(path.relative_to(home)) + if path.is_symlink(): + # Include broken links and directory links, but never read their targets. + snapshot[name] = ("symlink", str(path.readlink())) + elif path.is_dir(): + for child in sorted(path.iterdir()): + visit(child) + elif path.is_file(): + with path.open("rb") as source: + snapshot[name] = ("sha256", hashlib.file_digest(source, "sha256").hexdigest()) + + for directory in (home / ".ucode", home / ".codex"): + visit(directory) + return snapshot + + def build_coding_agent_config( default_agent: str, *agents: dict, diff --git a/tests/test_integration_runner.py b/tests/test_integration_runner.py new file mode 100644 index 00000000..ddd37b46 --- /dev/null +++ b/tests/test_integration_runner.py @@ -0,0 +1,24 @@ +"""Local checks for integration-runner deadline configuration.""" + +import subprocess +import sys +from pathlib import Path + +import pytest + +RUNNER = Path(__file__).resolve().parents[1] / "scripts/run_integration.py" + + +@pytest.mark.parametrize("option", ["--test-timeout", "--suite-timeout"]) +@pytest.mark.parametrize("value", ["0", "-1", "not-a-duration"]) +def test_integration_runner_rejects_invalid_deadlines(option, value): + result = subprocess.run( + [sys.executable, str(RUNNER), option, value], + stdin=subprocess.DEVNULL, + capture_output=True, + text=True, + timeout=5, + ) + assert result.returncode == 2 + assert option in result.stderr + assert "error:" in result.stderr diff --git a/tests/test_integration_snapshots.py b/tests/test_integration_snapshots.py new file mode 100644 index 00000000..9e368001 --- /dev/null +++ b/tests/test_integration_snapshots.py @@ -0,0 +1,94 @@ +"""Unit coverage for integration evidence collection, without launching an agent.""" + +import hashlib +import tracemalloc + +import pytest + +from tests.integration.utils.managed import codex_state_snapshot + + +@pytest.fixture +def snapshot_home(tmp_path): + home = tmp_path / "home" + (home / ".codex").mkdir(parents=True) + (home / ".ucode").mkdir() + return home + + +def test_codex_snapshot_ignores_only_bootstrap_links_and_control_plane_cache(snapshot_home): + home = snapshot_home + config = home / ".codex/config.toml" + config.write_text('model = "example"\n') + before = codex_state_snapshot(home) + bootstrap = home / ".codex/tmp/arg0/random-launch" + bootstrap.mkdir(parents=True) + (bootstrap / "apply_patch").symlink_to("/unreadable/agent-binary") + (bootstrap / ".lock").touch() + (home / ".ucode/managed-config.json").write_text('{"fetched_at": 1}') + assert codex_state_snapshot(home) == before + + (bootstrap / "apply_patch").unlink() + (bootstrap / "new-helper").symlink_to("/another/agent-binary") + (home / ".ucode/managed-config.json").write_text('{"fetched_at": 2}') + assert codex_state_snapshot(home) == before + + # A blanket exclusion of .codex/tmp would hide unrelated persistent changes. + (home / ".codex/tmp/other-file").write_text("must remain covered") + assert ".codex/tmp/other-file" in codex_state_snapshot(home) + + +@pytest.mark.parametrize("name", [".ucode/state.json", ".codex/config.toml"]) +def test_codex_snapshot_detects_added_changed_and_removed_persistent_files(snapshot_home, name): + path = snapshot_home / name + empty = codex_state_snapshot(snapshot_home) + path.write_text("original") + before = codex_state_snapshot(snapshot_home) + assert before != empty + assert before[name] == ("sha256", hashlib.sha256(b"original").hexdigest()) + path.write_text("modified") + assert codex_state_snapshot(snapshot_home) != before + path.unlink() + assert codex_state_snapshot(snapshot_home) == empty + + +def test_codex_snapshot_records_links_without_following_files_or_directories( + snapshot_home, tmp_path +): + external = tmp_path / "external" + external.mkdir() + target = external / "large-binary" + target.write_bytes(b"original binary") + links = snapshot_home / ".codex" + (links / "file-link").symlink_to(target) + (links / "directory-link").symlink_to(external, target_is_directory=True) + (links / "broken-link").symlink_to("missing") + (links / "cycle").symlink_to(links, target_is_directory=True) + before = codex_state_snapshot(snapshot_home) + assert len(before) == 4 + assert before[".codex/file-link"] == ("symlink", str(target)) + assert before[".codex/directory-link"] == ("symlink", str(external)) + assert before[".codex/broken-link"] == ("symlink", "missing") + target.write_bytes(b"changed outside the test home") + assert codex_state_snapshot(snapshot_home) == before + (links / "broken-link").unlink() + (links / "broken-link").symlink_to("different-target") + assert codex_state_snapshot(snapshot_home) != before + + +def test_codex_snapshot_hashes_large_files_with_bounded_memory(snapshot_home): + path = snapshot_home / ".codex/large-file" + with path.open("wb") as output: + output.truncate(16 * 1024 * 1024) + tracemalloc.start() + try: + snapshot = codex_state_snapshot(snapshot_home) + _, peak = tracemalloc.get_traced_memory() + finally: + tracemalloc.stop() + assert peak < 1024 * 1024, f"Snapshot allocated {peak} bytes for a 16 MiB file" + assert len(snapshot[".codex/large-file"][1]) == 64 + + +def test_codex_snapshot_handles_a_fresh_home(tmp_path): + assert codex_state_snapshot(tmp_path / "not-created") == {}