diff --git a/eval/tests/test_comparator_reuse.py b/eval/tests/test_comparator_reuse.py index 8ee27183a..189426b79 100644 --- a/eval/tests/test_comparator_reuse.py +++ b/eval/tests/test_comparator_reuse.py @@ -21,6 +21,12 @@ from workflow_bench.proposer_sandbox import SandboxError from workflow_bench.runner_sessions import PARENT_EVENT_STREAM_SOURCE +requires_openat = pytest.mark.skipif( + os.open not in os.supports_dir_fd, + reason="comparator reuse resolves every artifact against a pinned directory descriptor", +) + + def _digest(text: str = "blob") -> str: return hashlib.sha256(text.encode()).hexdigest() @@ -148,6 +154,7 @@ def test_select_drops_conflicting_duplicates() -> None: assert ("review-pr-2718-defect", "review", 0) in same +@requires_openat def test_materialize_copies_transcript_and_review_artifacts(tmp_path: Path) -> None: payload = b'{"type":"result"}\n' source = tmp_path / "prior" @@ -177,6 +184,7 @@ def test_materialize_copies_transcript_and_review_artifacts(tmp_path: Path) -> N @pytest.mark.skipif(os.name == "nt", reason="symlink creation may require elevated Windows privileges") +@requires_openat def test_a_reused_artifact_is_copied_from_the_inode_that_was_checked(tmp_path: Path) -> None: """The reuse source is a directory another sweep wrote and may still write. @@ -186,24 +194,24 @@ def test_a_reused_artifact_is_copied_from_the_inode_that_was_checked(tmp_path: P same substitution, made deterministic. """ - source = tmp_path / "transcript.jsonl" - source.write_bytes(b"verified\n") + (tmp_path / "transcript.jsonl").write_bytes(b"verified\n") decoy = tmp_path / "decoy.jsonl" decoy.write_bytes(b"substituted\n") - destination = tmp_path / "copy.jsonl" - with comparator_reuse._open_regular(source, label="transcript") as descriptor: - source.unlink() - source.symlink_to(decoy) - comparator_reuse._copy_owner_only(descriptor, destination) + with comparator_reuse._open_real_directory(tmp_path, label="reuse source") as dir_fd: + with comparator_reuse._open_regular("transcript.jsonl", dir_fd=dir_fd, label="transcript") as descriptor: + (tmp_path / "transcript.jsonl").unlink() + (tmp_path / "transcript.jsonl").symlink_to(decoy) + comparator_reuse._copy_owner_only(descriptor, "copy.jsonl", dir_fd=dir_fd) - assert destination.read_bytes() == b"verified\n" - with pytest.raises(SandboxError, match="regular non-symlink"): - with comparator_reuse._open_regular(source, label="transcript"): - pass + assert (tmp_path / "copy.jsonl").read_bytes() == b"verified\n" + with pytest.raises(SandboxError, match="regular non-symlink"): + with comparator_reuse._open_regular("transcript.jsonl", dir_fd=dir_fd, label="transcript"): + pass @pytest.mark.skipif(os.name == "nt", reason="symlink creation may require elevated Windows privileges") +@requires_openat def test_a_symlinked_transcripts_directory_is_refused_on_both_sides(tmp_path: Path) -> None: """`O_NOFOLLOW` refuses the leaf, not the directory above it. @@ -237,6 +245,42 @@ def test_a_symlinked_transcripts_directory_is_refused_on_both_sides(tmp_path: Pa materialize_reused_row(row, source_dir=source, dest_dir=linked_dest) +@requires_openat +@pytest.mark.skipif(os.name == "nt", reason="symlink creation may require elevated Windows privileges") +def test_a_renamed_transcripts_directory_cannot_redirect_a_copy(tmp_path: Path) -> None: + """The directory is pinned, not re-walked from its name. + + An lstat that passed and a pathname used afterwards are two different + directories the moment a concurrent writer renames the first one away. This + performs exactly that substitution — rename, then leave a symlink in its + place — while the descriptor is held, which is what makes the race testable + without timing. + """ + + payload = b'{"type":"result"}\n' + results = tmp_path / "results" + transcripts = results / "transcripts" + transcripts.mkdir(parents=True) + (transcripts / "session-1.jsonl").write_bytes(payload) + outside = tmp_path / "outside" + outside.mkdir() + + with comparator_reuse._open_real_directory(results, label="reuse source") as root_fd: + with comparator_reuse._open_real_directory( + "transcripts", dir_fd=root_fd, label="transcript source" + ) as dir_fd: + transcripts.rename(results / "moved") + (results / "transcripts").symlink_to(outside, target_is_directory=True) + with comparator_reuse._open_regular( + "session-1.jsonl", dir_fd=dir_fd, label="transcript" + ) as artifact_fd: + comparator_reuse._copy_owner_only(artifact_fd, "copy.jsonl", dir_fd=dir_fd) + + assert (results / "moved" / "copy.jsonl").read_bytes() == payload + assert not (outside / "copy.jsonl").exists() + + +@requires_openat def test_materialize_rejects_same_directory_and_missing_transcript(tmp_path: Path) -> None: source = tmp_path / "prior" source.mkdir() @@ -249,6 +293,7 @@ def test_materialize_rejects_same_directory_and_missing_transcript(tmp_path: Pat materialize_reused_row(row, source_dir=source, dest_dir=dest) +@requires_openat def test_a_reused_row_ages_from_its_first_measurement_not_the_copy(tmp_path: Path): """Reuse chains must not refresh the clock. diff --git a/eval/tests/test_evolve.py b/eval/tests/test_evolve.py index 510946244..1c0dc7057 100644 --- a/eval/tests/test_evolve.py +++ b/eval/tests/test_evolve.py @@ -1237,6 +1237,124 @@ def test_parser_rejects_non_positive_max_runtime() -> None: ["--tasks", "t.yaml", "--model", "pinned", "--max-runtime-seconds", "7200"] ) assert args.max_runtime_seconds == 7200 + assert args.max_runtime_from_instance_window is False + + +def _task_file(tmp_path: Path) -> Path: + tasks = tmp_path / "tasks.yaml" + tasks.write_text( + """tasks: + - id: demo + class: test + repo: . + prompt: implement + verify: "true" + oracle: + command: "true" + files: + - source: hidden.test.ts + target: hidden.test.ts +""" + ) + return tasks + + +def _stub_main_preflight(monkeypatch, tmp_path) -> None: + """Everything main() shells out to before it reaches _run_generations.""" + + monkeypatch.setattr(evolve.runner, "selected_task_bindings", lambda _tasks: [{"id": "demo"}]) + monkeypatch.setattr(evolve, "preflight_bubblewrap", lambda: tmp_path / "bwrap") + monkeypatch.setattr(evolve, "require_claude_sandbox_helpers", lambda: None) + + +def test_the_runtime_cap_is_derived_where_its_clock_starts(monkeypatch, tmp_path, capsys) -> None: + """The budget and the clock it is measured against must be one instant. + + run-evolution.sh used to compute the budget in a separate `uv run python -c` + and pass a number, so the script's remaining provenance work and this + interpreter's startup were charged to the sweep — out of the upload reserve + the cap exists to protect. main() reads /proc/uptime itself now, next to its + own clock, so no interval exists to lose. + """ + + uptime = tmp_path / "uptime" + uptime.write_text("3600.00 8000.00\n") + monkeypatch.setenv("EVENTBRIDGE_INSTANCE_WINDOW_SECONDS", "20000") + monkeypatch.setenv("EVENTBRIDGE_STOP_RESERVE_SECONDS", "1000") + monkeypatch.setattr(evolve, "read_instance_uptime_seconds", lambda: 3600.0) + captured: dict[str, object] = {} + + def record(args, **kwargs): + captured["max_runtime_seconds"] = args.max_runtime_seconds + captured["started_monotonic"] = kwargs["started_monotonic"] + return 0 + + monkeypatch.setattr(evolve, "_run_generations", record) + _stub_main_preflight(monkeypatch, tmp_path) + monkeypatch.setattr( + sys, + "argv", + [ + "evolve", + "--tasks", + str(_task_file(tmp_path)), + "--model", + "pinned", + "--out-root", + str(tmp_path / "out"), + "--max-runtime-from-instance-window", + ], + ) + + assert evolve.main() == 0 + + assert captured["max_runtime_seconds"] == 20000 - 3600 - 1000 + # Derived here, not passed in: the clock handed to the sweep is the one + # taken beside the uptime read. + assert isinstance(captured["started_monotonic"], float) + assert "capping the sweep to 15400s" in capsys.readouterr().out + + +def test_the_runtime_cap_refuses_two_sources_of_truth(monkeypatch, tmp_path) -> None: + monkeypatch.setattr(evolve, "read_instance_uptime_seconds", lambda: 3600.0) + monkeypatch.setattr( + sys, + "argv", + [ + "evolve", + "--tasks", + str(_task_file(tmp_path)), + "--model", + "pinned", + "--max-runtime-from-instance-window", + "--max-runtime-seconds", + "7200", + ], + ) + with pytest.raises(SystemExit): + evolve.main() + + +def test_the_runtime_cap_fails_closed_without_a_readable_uptime(monkeypatch, tmp_path) -> None: + def unreadable(): + raise ValueError("cannot read instance uptime from /proc/uptime") + + monkeypatch.setattr(evolve, "read_instance_uptime_seconds", unreadable) + monkeypatch.setattr( + sys, + "argv", + [ + "evolve", + "--tasks", + str(_task_file(tmp_path)), + "--model", + "pinned", + "--max-runtime-from-instance-window", + ], + ) + # Better to refuse than to run a box-stopped sweep believing it is uncapped. + with pytest.raises(SystemExit): + evolve.main() @pytest.mark.skipif(sys.platform != "linux", reason="Bubblewrap PID namespaces require Linux") diff --git a/eval/workflow_bench/README.md b/eval/workflow_bench/README.md index 0f78069b8..a0008fa7d 100644 --- a/eval/workflow_bench/README.md +++ b/eval/workflow_bench/README.md @@ -170,9 +170,11 @@ router thresholds as an incumbent policy, not permanent truth. Candidate changes run offline in the same throwaway clones as the incumbent; production skills never rewrite themselves from a live task. -On the self-hosted evolution box, `run-evolution.sh` caps the sweep with -`--max-runtime-seconds` derived from `/proc/uptime` (24h EventBridge window -minus a 90-minute upload reserve). A `workflow_dispatch` that lands on an +On the self-hosted evolution box, `run-evolution.sh` passes +`--max-runtime-from-instance-window` and the CLI derives its own cap from +`/proc/uptime` at startup (24h EventBridge window minus a 90-minute upload +reserve), in the same breath as it starts the clock that cap is measured +against — a budget computed anywhere earlier is spent by the seconds between. A `workflow_dispatch` that lands on an already-running instance therefore exits in-process instead of vanishing when the box stops — a cancelled GitHub job skips even `if: always()`, which is how run 33962002890 lost 51 finished sessions. Local runs are uncapped. diff --git a/eval/workflow_bench/comparator_reuse.py b/eval/workflow_bench/comparator_reuse.py index 3be891d08..548e540b8 100644 --- a/eval/workflow_bench/comparator_reuse.py +++ b/eval/workflow_bench/comparator_reuse.py @@ -263,23 +263,31 @@ def materialize_reused_row( artifacts = row.get("transcript_artifacts") if not isinstance(artifacts, list) or not artifacts: raise SandboxError("reused row is missing transcript_artifacts") - copied_artifacts: list[dict[str, Any]] = [] - for artifact in artifacts: - copied_artifacts.append(_copy_transcript_artifact(source, dest, artifact)) - materialized["transcript_artifacts"] = copied_artifacts - review_name = row.get("review_artifact") - if isinstance(review_name, str) and review_name: - _copy_named_artifact(source, dest, review_name, label="review artifact") + # Every path below is resolved against a held descriptor, never re-walked + # from a name. Both roots are already symlink-free (_resolved_directory + # resolved them), and pinning them here means the components under them + # cannot be swapped out from under a check that already passed. + with ( + _open_real_directory(source, label="reuse source") as source_fd, + _open_real_directory(dest, label="reuse destination") as dest_fd, + ): + copied_artifacts: list[dict[str, Any]] = [] + for artifact in artifacts: + copied_artifacts.append(_copy_transcript_artifact(source_fd, dest_fd, artifact)) + materialized["transcript_artifacts"] = copied_artifacts - task = row.get("task") - arm = row.get("arm") - run = row.get("run") - if isinstance(task, str) and isinstance(arm, str) and isinstance(run, int) and not isinstance(run, bool): - patch_name = f"{task}-{arm}-run{run}.patch" - patch = source / patch_name - if patch.is_file() and not patch.is_symlink(): - _copy_named_artifact(source, dest, patch_name, label="patch artifact") + review_name = row.get("review_artifact") + if isinstance(review_name, str) and review_name: + _copy_named_artifact(source_fd, dest_fd, review_name, label="review artifact") + + task = row.get("task") + arm = row.get("arm") + run = row.get("run") + if isinstance(task, str) and isinstance(arm, str) and isinstance(run, int) and not isinstance(run, bool): + patch_name = f"{task}-{arm}-run{run}.patch" + if _is_regular_at(patch_name, dir_fd=source_fd): + _copy_named_artifact(source_fd, dest_fd, patch_name, label="patch artifact") return materialized @@ -359,102 +367,156 @@ def _resolved_directory(path: Path, *, label: str) -> Path: return resolved.resolve() -def _copy_transcript_artifact(source: Path, dest: Path, metadata: Mapping[str, Any]) -> dict[str, Any]: +def _copy_transcript_artifact(source_fd: int, dest_fd: int, metadata: Mapping[str, Any]) -> dict[str, Any]: relative, expected_digest, expected_size = _transcript_metadata(metadata) name = PurePosixPath(relative).name - dest_dir = _real_child_directory(dest, "transcripts", label="transcript destination", create=True) - dest_dir.chmod(0o700) - destination = dest_dir / name - source_dir = _real_child_directory(source, "transcripts", label="transcript source") - # One descriptor for the size check, the digest and the copy. Re-opening the - # path between them is what let a concurrent writer swap the checked file - # for a symlink and have the copy follow it. - with _open_regular(source_dir / name, label="transcript") as source_fd: - if os.fstat(source_fd).st_size != expected_size: - raise SandboxError(f"reused transcript size drifted: {relative}") - digest = _sha256_descriptor(source_fd) - if digest != expected_digest: - raise SandboxError(f"reused transcript digest drifted: {relative}") - _copy_owner_only(source_fd, destination) + # Both `transcripts` components are opened as descriptors, not checked as + # names. An lstat that passes and a pathname that is used afterwards are two + # different directories whenever a concurrent writer renames the first one + # away — which the reuse directory, written by a prior sweep, invites. + with ( + _open_real_directory("transcripts", dir_fd=dest_fd, label="transcript destination", create=True) as dest_dir_fd, + _open_real_directory("transcripts", dir_fd=source_fd, label="transcript source") as source_dir_fd, + ): + os.fchmod(dest_dir_fd, 0o700) + # One descriptor for the size check, the digest and the copy. Re-opening + # the name between them is what let a writer swap the checked file for a + # symlink and have the copy follow it. + with _open_regular(name, dir_fd=source_dir_fd, label="transcript") as artifact_fd: + if os.fstat(artifact_fd).st_size != expected_size: + raise SandboxError(f"reused transcript size drifted: {relative}") + digest = _sha256_descriptor(artifact_fd) + if digest != expected_digest: + raise SandboxError(f"reused transcript digest drifted: {relative}") + _copy_owner_only(artifact_fd, name, dir_fd=dest_dir_fd) return {"path": relative, "sha256": digest, "bytes": expected_size, "source": PARENT_EVENT_STREAM_SOURCE} -def _copy_named_artifact(source: Path, dest: Path, name: str, *, label: str) -> None: +def _copy_named_artifact(source_fd: int, dest_fd: int, name: str, *, label: str) -> None: relative = PurePosixPath(name) if relative.is_absolute() or len(relative.parts) != 1 or relative.parts[0] in {"", ".", ".."}: raise SandboxError(f"unsafe {label} path: {name!r}") - with _open_regular(source / name, label=label) as source_fd: - _copy_owner_only(source_fd, dest / name) + with _open_regular(name, dir_fd=source_fd, label=label) as artifact_fd: + _copy_owner_only(artifact_fd, name, dir_fd=dest_fd) -def _real_child_directory(parent: Path, name: str, *, label: str, create: bool = False) -> Path: - """One directory component below the reuse root, proven not to be a symlink. +def _require_openat() -> None: + """openat is what makes a checked directory and a used directory the same one. - ``_resolved_directory`` tolerates a symlinked ROOT because everything below - it is validated individually. ``transcripts`` is the component that argument - misses: it is neither a file read guarded by ``lstat`` nor a write guarded - by ``O_NOFOLLOW``, and ``O_NOFOLLOW`` refuses only the leaf, so a link here - redirects the read or the write out of the results directory entirely. - Checked per component, as ``evolution._require_directory_chain`` does. + Without it the only alternative is to re-walk the name after the check, + which is exactly the race this module is guarding. Refusing is safe: the + caller in runner treats a SandboxError from reuse as "run a paid cell", so + a platform without openat pays for the cells rather than copying through a + directory nobody verified. The sweep itself is Linux-only anyway (bwrap, + /proc/uptime); this is about the unit tests and about failing loudly. """ - child = parent / name + if os.open not in os.supports_dir_fd or os.lstat not in os.supports_dir_fd: + raise SandboxError("comparator reuse requires POSIX openat support (os.supports_dir_fd)") + + +def _is_regular_at(name: str, *, dir_fd: int) -> bool: + """True when `name` under the pinned directory is a regular non-symlink file.""" + try: - metadata = child.lstat() - except FileNotFoundError: - if not create: - raise SandboxError(f"{label} is missing: {child}") from None - child.mkdir(mode=0o700) - return child - except OSError as exc: - raise SandboxError(f"{label} is unavailable: {child}: {exc}") from exc - if stat.S_ISLNK(metadata.st_mode) or not stat.S_ISDIR(metadata.st_mode): - raise SandboxError(f"{label} must be a real directory: {child}") - return child + metadata = os.lstat(name, dir_fd=dir_fd) + except OSError: + return False + return stat.S_ISREG(metadata.st_mode) @contextmanager -def _open_regular(path: Path, *, label: str) -> Iterator[int]: - """Open a regular non-symlink file and hold it open for every later read. +def _open_real_directory( + path: Path | str, + *, + dir_fd: int | None = None, + label: str, + create: bool = False, +) -> Iterator[int]: + """Open one directory that is not a symlink, and hold it for every use below. - Checking the path and then re-opening it is a race the reuse directory is - exposed to: it is written by a previous sweep and read by this one, so a - concurrent writer can replace a validated file with a symlink in between. - O_NOFOLLOW refuses the leaf link and the fstat comparison proves the open - descriptor is the inode that was checked — the same guarantee - evolution._bounded_regular_bytes makes for evidence files. + ``O_DIRECTORY | O_NOFOLLOW`` makes the check and the open a single syscall, + so unlike an ``lstat`` followed by a path, there is no window in which the + directory can be replaced. ``_resolved_directory`` still tolerates a + symlinked reuse ROOT — it hands this function the already-resolved path — + but every component below it is pinned. """ + _require_openat() + if create: + try: + os.mkdir(path, 0o700, dir_fd=dir_fd) + except FileExistsError: + pass + except OSError as exc: + raise SandboxError(f"{label} cannot be created: {path}: {exc}") from exc try: - before = path.lstat() + descriptor = os.open( + path, + os.O_RDONLY | getattr(os, "O_DIRECTORY", 0) | getattr(os, "O_NOFOLLOW", 0), + dir_fd=dir_fd, + ) + except FileNotFoundError as exc: + # Absent is a different fact from present-but-not-a-real-directory, and + # the caller falls through to a paid cell on either. + raise SandboxError(f"{label} is missing: {path}") from exc except OSError as exc: - raise SandboxError(f"{label} is missing: {path}: {exc}") from exc - if stat.S_ISLNK(before.st_mode) or not stat.S_ISREG(before.st_mode): - raise SandboxError(f"{label} must be a regular non-symlink file: {path}") + raise SandboxError(f"{label} must be a real directory: {path}: {exc}") from exc try: - descriptor = os.open(path, os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0)) - except OSError as exc: - raise SandboxError(f"{label} is unreadable: {path}: {exc}") from exc - try: - opened = os.fstat(descriptor) - if not stat.S_ISREG(opened.st_mode) or (opened.st_dev, opened.st_ino) != (before.st_dev, before.st_ino): - raise SandboxError(f"{label} changed while opening: {path}") + # O_DIRECTORY is the check on Linux; the fstat covers a platform whose + # os module does not define it, where the flag degrades to 0. + if not stat.S_ISDIR(os.fstat(descriptor).st_mode): + raise SandboxError(f"{label} must be a real directory: {path}") yield descriptor finally: os.close(descriptor) -def _copy_owner_only(source: int, destination: Path) -> None: +@contextmanager +def _open_regular(name: str, *, dir_fd: int, label: str) -> Iterator[int]: + """Open a regular non-symlink file under a pinned directory, and hold it. + + Checking a name and then re-opening it is a race the reuse directory is + exposed to: it is written by a previous sweep and read by this one, so a + concurrent writer can replace a validated file with a symlink in between. + Resolving against ``dir_fd`` removes the directory half, ``O_NOFOLLOW`` + refuses the leaf link, and the fstat comparison proves the open descriptor + is the inode that was checked — the same guarantee + evolution._bounded_regular_bytes makes for evidence files. + """ + + _require_openat() + try: + before = os.lstat(name, dir_fd=dir_fd) + except OSError as exc: + raise SandboxError(f"{label} is missing: {name}: {exc}") from exc + if stat.S_ISLNK(before.st_mode) or not stat.S_ISREG(before.st_mode): + raise SandboxError(f"{label} must be a regular non-symlink file: {name}") + try: + descriptor = os.open(name, os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0), dir_fd=dir_fd) + except OSError as exc: + raise SandboxError(f"{label} is unreadable: {name}: {exc}") from exc + try: + opened = os.fstat(descriptor) + if not stat.S_ISREG(opened.st_mode) or (opened.st_dev, opened.st_ino) != (before.st_dev, before.st_ino): + raise SandboxError(f"{label} changed while opening: {name}") + yield descriptor + finally: + os.close(descriptor) + + +def _copy_owner_only(source: int, name: str, *, dir_fd: int) -> None: # O_CREAT|O_EXCL is the existence check, and unlike a stat beforehand it is # atomic: a file appearing between check and open cannot slip through. try: descriptor = os.open( - destination, + name, os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_NOFOLLOW", 0), 0o600, + dir_fd=dir_fd, ) except FileExistsError as exc: - raise SandboxError(f"reuse destination already exists: {destination}") from exc + raise SandboxError(f"reuse destination already exists: {name}") from exc try: os.fchmod(descriptor, 0o600) os.lseek(source, 0, os.SEEK_SET) diff --git a/eval/workflow_bench/evolve.py b/eval/workflow_bench/evolve.py index d2c261066..d765793e9 100644 --- a/eval/workflow_bench/evolve.py +++ b/eval/workflow_bench/evolve.py @@ -871,13 +871,42 @@ def instance_window_budget_seconds( return leftover -def instance_window_budget_from_proc( - uptime_path: Path = Path("/proc/uptime"), +def _instance_uptime_or_none() -> float | None: + """The uptime read main() takes before it knows whether it needs it. + + Deferring the read until after argument parsing would put the parse back + inside the interval the cap is supposed to cover, so it happens first and + an unreadable /proc/uptime is only an error if the flag turns out to be set. + """ + + try: + return read_instance_uptime_seconds() + except ValueError: + return None + + +def read_instance_uptime_seconds(uptime_path: Path = Path("/proc/uptime")) -> float: + """Host uptime, the clock the EventBridge stop is scheduled against.""" + + try: + return float(uptime_path.read_text().split()[0]) + except (OSError, IndexError, ValueError) as exc: + raise ValueError(f"cannot read instance uptime from {uptime_path}: {exc}") from exc + + +def instance_window_budget_from_uptime( + uptime_seconds: float, *, window_seconds: int | None = None, reserve_seconds: int | None = None, ) -> int: - """Read host uptime and apply the EventBridge window env overrides.""" + """Apply the EventBridge window env overrides to an already-read uptime. + + Separate from the read so ``main`` can take the uptime in the same breath + as its own clock: the budget and the clock it is measured against have to + describe one instant, or the interval between them is spent by nobody and + charged to the sweep. + """ window = ( window_seconds @@ -889,11 +918,22 @@ def instance_window_budget_from_proc( if reserve_seconds is not None else int(os.environ.get("EVENTBRIDGE_STOP_RESERVE_SECONDS", str(EVENTBRIDGE_STOP_RESERVE_SECONDS))) ) - try: - uptime = float(uptime_path.read_text().split()[0]) - except (OSError, IndexError, ValueError) as exc: - raise ValueError(f"cannot read instance uptime from {uptime_path}: {exc}") from exc - return instance_window_budget_seconds(uptime, window_seconds=window, reserve_seconds=reserve) + return instance_window_budget_seconds(uptime_seconds, window_seconds=window, reserve_seconds=reserve) + + +def instance_window_budget_from_proc( + uptime_path: Path = Path("/proc/uptime"), + *, + window_seconds: int | None = None, + reserve_seconds: int | None = None, +) -> int: + """Read host uptime and apply the EventBridge window env overrides.""" + + return instance_window_budget_from_uptime( + read_instance_uptime_seconds(uptime_path), + window_seconds=window_seconds, + reserve_seconds=reserve_seconds, + ) def remaining_runtime_seconds(*, max_runtime_seconds: int | None, started_monotonic: float) -> int | None: @@ -1388,9 +1428,15 @@ def build_parser() -> argparse.ArgumentParser: "--max-runtime-seconds", type=_positive_int, default=None, - help="wall-clock cap for the whole evolve process (CI sets this from " + help="wall-clock cap for the whole evolve process (CI derives this from " "instance uptime so the sweep exits before EventBridge stops the box)", ) + parser.add_argument( + "--max-runtime-from-instance-window", + action="store_true", + help="derive --max-runtime-seconds from /proc/uptime at startup, so the " + "budget and the clock it is measured against describe one instant", + ) parser.add_argument("--base-url", default=None) parser.add_argument( "--anthropic-api-key", @@ -1428,14 +1474,26 @@ def build_parser() -> argparse.ArgumentParser: def main() -> int: - # Before anything else: --max-runtime-seconds is measured from /proc/uptime - # before this process is even exec'd (run-evolution.sh), so every second - # spent parsing, reading tasks, running the sandbox preflight and starting - # the gateway would otherwise be handed back to the sweep and taken out of - # the upload reserve the cap exists to protect. + # These two lines are the cap, and they are adjacent on purpose: the clock + # the sweep is measured against, and the uptime the budget is derived from. + # run-evolution.sh used to compute the budget in its own `uv run python -c` + # and pass a number, so the script's remaining work and this interpreter's + # startup were spent by nobody and charged to the sweep — out of the upload + # reserve the cap exists to protect. Nothing can be spent between them now. started_monotonic = time.monotonic() + instance_uptime = _instance_uptime_or_none() parser = build_parser() args = parser.parse_args() + if args.max_runtime_from_instance_window: + if args.max_runtime_seconds is not None: + parser.error("--max-runtime-from-instance-window and --max-runtime-seconds are mutually exclusive") + if instance_uptime is None: + parser.error("--max-runtime-from-instance-window needs a readable /proc/uptime") + try: + args.max_runtime_seconds = instance_window_budget_from_uptime(instance_uptime) + except ValueError as exc: + parser.error(str(exc)) + print(f"capping the sweep to {args.max_runtime_seconds}s so the instance-window reserve can upload evidence") if args.generations < 1: parser.error("--generations must be positive") if args.runs < 1 or args.timeout < 1: diff --git a/eval/workflow_bench/run-evolution.sh b/eval/workflow_bench/run-evolution.sh index 396fdac28..066df111a 100755 --- a/eval/workflow_bench/run-evolution.sh +++ b/eval/workflow_bench/run-evolution.sh @@ -15,7 +15,7 @@ # MODEL PROPOSER_MODEL EFFORT GENERATIONS RUNS WORKERS PROVIDER # EVOLUTION_PROFILE CE_PLUGIN_DIR CE_PLUGIN_VERSION # INCLUDE_EXPENSIVE SEED_RESULTS CLAUDE_BIN OUT_ROOT -# CI (caps --max-runtime-seconds from /proc/uptime) +# CI (passes --max-runtime-from-instance-window; the CLI reads /proc/uptime) # EVENTBRIDGE_INSTANCE_WINDOW_SECONDS EVENTBRIDGE_STOP_RESERVE_SECONDS # UNSAFE_NO_BWRAP=1 (local review diagnostics only) # GITNEXUS_BENCH_ANTHROPIC_API_KEY (legacy GITNEXUS_BENCH_AUTH_TOKEN) @@ -195,26 +195,24 @@ if ((${#passthrough[@]})); then cmd+=("${passthrough[@]}") fi +# A cancelled GitHub job skips even `if: always()`, so evidence dies with the +# runner. The evolution box is EventBridge-stopped 24h after boot; a Friday +# dispatch inherits leftover uptime. Cap the sweep so it fails in-process and +# the upload step still runs (run 33962002890). The CLI reads /proc/uptime +# itself, in the same breath as it starts the clock the cap is measured +# against; computing a number here — in a separate interpreter, before the +# provenance work and the exec below — charged the sweep for every second +# this script spent afterwards. +if [[ -n "${CI:-}" && -r /proc/uptime ]]; then + cmd+=(--max-runtime-from-instance-window) +fi + if ((dry_run)); then printf '%q ' "${cmd[@]}" printf '\n' exit 0 fi -# A cancelled GitHub job skips even `if: always()`, so evidence dies with the -# runner. The evolution box is EventBridge-stopped 24h after boot; a Friday -# dispatch inherits leftover uptime. Cap the sweep so it fails in-process and -# the upload step still runs (run 33962002890). -if [[ -n "${CI:-}" && -r /proc/uptime ]]; then - remaining="$( - cd "${eval_dir}" - uv run --locked --extra dev python -c \ - 'from workflow_bench.evolve import instance_window_budget_from_proc; print(instance_window_budget_from_proc())' - )" - cmd+=(--max-runtime-seconds "${remaining}") - echo "Capping the sweep to ${remaining}s so the instance-window reserve can upload evidence." >&2 -fi - mkdir -p "${out_root}" source_sha="$(git -C "${eval_dir}/.." rev-parse HEAD)" runtime_digest="$( diff --git a/gitnexus/test/unit/skill-evolution-workflow.test.ts b/gitnexus/test/unit/skill-evolution-workflow.test.ts index 2bc4d1844..1d303f00f 100644 --- a/gitnexus/test/unit/skill-evolution-workflow.test.ts +++ b/gitnexus/test/unit/skill-evolution-workflow.test.ts @@ -580,8 +580,11 @@ exit 1`); path.join(REPO_ROOT, 'eval/workflow_bench/run-evolution.sh'), 'utf8', ); - expect(script).toContain('--max-runtime-seconds'); - expect(script).toContain('instance_window_budget_from_proc'); + // The flag, not a precomputed number: the CLI reads /proc/uptime in the + // same breath as it starts the clock the cap is measured against, so + // nothing between the two can be charged to the sweep. + expect(script).toContain('--max-runtime-from-instance-window'); + expect(script).not.toContain('instance_window_budget_from_proc'); expect(script).toContain('export RUNTIME_DIGEST'); // The workflow's half of that contract is calling the entrypoint, not // naming the flag: its only occurrence in the YAML is the explanatory