diff --git a/.gitattributes b/.gitattributes index 6cd6285821..27961f90ea 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,2 +1,6 @@ # Preserve the immutable archive's bytes even with core.autocrlf=true. benchmark/deepswe-gptxhigh-v1/** text eol=lf + +# Published study archives bind sha256 of their own LF bytes in index.json, so +# a CRLF checkout must not rewrite them either. +benchmark/edgebench/studies/**/data/** text eol=lf diff --git a/benchmark/edgebench/export_report.py b/benchmark/edgebench/export_report.py index d8be072b56..75138263ae 100644 --- a/benchmark/edgebench/export_report.py +++ b/benchmark/edgebench/export_report.py @@ -39,7 +39,8 @@ def read(path: Path): def write(path: Path, value): path.write_text( - json.dumps(value, indent=2, ensure_ascii=False, allow_nan=False) + "\n" + json.dumps(value, indent=2, ensure_ascii=False, allow_nan=False) + "\n", + newline="\n", ) diff --git a/benchmark/edgebench/feedback_hook.py b/benchmark/edgebench/feedback_hook.py index 99e656f665..cbbe1b76a0 100644 --- a/benchmark/edgebench/feedback_hook.py +++ b/benchmark/edgebench/feedback_hook.py @@ -5,13 +5,19 @@ """ from __future__ import annotations -import fcntl import hashlib +import importlib import json import re import shlex import sys from pathlib import Path +from typing import Any + +try: # POSIX-only: the installed hook serializes delivery with flock. + fcntl: Any = importlib.import_module("fcntl") +except ImportError: # imported on hosts that only read the report adapter chain + fcntl = None ROOT = Path('/opt/edgebench-feedback') EVENTS = ('PostToolUse', 'SessionStart', 'UserPromptSubmit') @@ -20,6 +26,8 @@ def deliver(event, root=ROOT, receipts=Path('/logs/agent/best-feedback-delivery'), *, output=None): + if fcntl is None: + raise OSError('best-only delivery has no supported session-lock backend on this host') output = output or sys.stdout name, session = event.get('hook_event_name'), event.get('session_id') if name not in EVENTS or not isinstance(session, str) or not session: diff --git a/benchmark/edgebench/studies/portfolio-ten-arm-20261006/README.md b/benchmark/edgebench/studies/portfolio-ten-arm-20261006/README.md index ff278804e3..101e04f7ac 100644 --- a/benchmark/edgebench/studies/portfolio-ten-arm-20261006/README.md +++ b/benchmark/edgebench/studies/portfolio-ten-arm-20261006/README.md @@ -153,6 +153,19 @@ uv run --extra test python -m benchmark.edgebench.export_report \ --verify benchmark/edgebench/studies/portfolio-ten-arm-20261006/data ``` +`.gitattributes` pins this archive to LF line endings, so the recorded digests +verify even when Git checks out other text files with `core.autocrlf=true`. The +exporter writes LF bytes on every platform, so a new report binds the same bytes +Git stores. Git does not rewrite files already materialized in an existing +checkout and a plain checkout skips them, so delete the stale directory once — +`rm -rf` below, or `Remove-Item -Recurse -Force` on PowerShell — and restore it +before verifying: + +```sh +rm -rf benchmark/edgebench/studies/portfolio-ten-arm-20261006/data +git checkout -- benchmark/edgebench/studies/portfolio-ten-arm-20261006/data +``` + For another terminal collection, supply an explicit JSON list of `{"row": , "settings": }` and run the same module with `--runs-root`, `--selection`, `--output` and diff --git a/benchmark/tests/test_edgebench_feedback_hook.py b/benchmark/tests/test_edgebench_feedback_hook.py index 94f764e01f..3ca7470f0e 100644 --- a/benchmark/tests/test_edgebench_feedback_hook.py +++ b/benchmark/tests/test_edgebench_feedback_hook.py @@ -11,6 +11,14 @@ from benchmark.edgebench.feedback_hook import deliver, install from benchmark.edgebench import feedback_hook +requires_posix_flock = pytest.mark.skipif( + feedback_hook.fcntl is None, + reason="delivery serializes its session reader with POSIX flock", +) +requires_posix_modes = pytest.mark.skipif( + os.name != 'posix', reason="the installer asserts POSIX file modes" +) + def packet(root, n=2): root.mkdir(exist_ok=True) @@ -33,6 +41,7 @@ def packet(root, n=2): return value +@requires_posix_flock def test_parallel_tools_resume_and_new_session(tmp_path): root, receipts = tmp_path / 'feedback', tmp_path / 'receipts' expected = packet(root) @@ -56,6 +65,7 @@ def call(name='PostToolUse', session='s1'): assert call('UserPromptSubmit') and not call() +@requires_posix_flock @pytest.mark.parametrize('event_name', ['PostToolUse', 'SessionStart', 'UserPromptSubmit']) def test_waiting_reader_cannot_rewind_newer_delivery(tmp_path, monkeypatch, event_name): root, receipts = tmp_path / 'feedback', tmp_path / 'receipts' @@ -92,6 +102,7 @@ def call(): assert not call() # A later hook must not replay the newest result either. +@requires_posix_flock def test_empty_and_malformed_do_not_disclose(tmp_path): value = packet(tmp_path) event = {'session_id': 'test', 'hook_event_name': 'PostToolUse'} @@ -106,6 +117,7 @@ def test_empty_and_malformed_do_not_disclose(tmp_path): assert not output.getvalue() +@requires_posix_flock @pytest.mark.parametrize('fault', ['digest', 'task', 'submission', 'epoch', 'revision', 'invalid', 'schema']) def test_misbound_result_never_advances_session_cursor(tmp_path, fault): value = packet(tmp_path) @@ -136,6 +148,7 @@ def test_misbound_result_never_advances_session_cursor(tmp_path, fault): assert stream.getvalue() and len(list(receipts.glob('*.json'))) == 1 +@requires_posix_modes def test_installer_preserves_official_stop_and_is_idempotent(tmp_path): settings = {'hooks': {'Stop': [{'hooks': [{'type': 'command', 'command': '/official-stop'}]}]}} (tmp_path / 'hooks.json').write_text(json.dumps(settings)) @@ -159,3 +172,11 @@ def test_installer_preserves_official_stop_and_is_idempotent(tmp_path): assert tmp_path.stat().st_mode & 0o777 == 0o755 assert script.stat().st_mode & 0o777 == 0o644 assert (tmp_path / 'hooks.json').stat().st_mode & 0o777 == 0o644 + + +def test_delivery_refuses_a_host_without_the_posix_session_lock(tmp_path, monkeypatch): + monkeypatch.setattr(feedback_hook, 'fcntl', None) + receipts = tmp_path / 'receipts' + with pytest.raises(OSError, match='session-lock backend'): + deliver({'session_id': 'test', 'hook_event_name': 'PostToolUse'}, tmp_path, receipts) + assert not receipts.exists() diff --git a/benchmark/tests/test_edgebench_report.py b/benchmark/tests/test_edgebench_report.py index d0b7993210..984995ae9f 100644 --- a/benchmark/tests/test_edgebench_report.py +++ b/benchmark/tests/test_edgebench_report.py @@ -3,14 +3,20 @@ import copy import csv import json +import shutil import subprocess import sys +from pathlib import Path import pytest from benchmark.edgebench.export_report import digest, export, verify, write +ROOT = Path(__file__).resolve().parents[2] +PUBLISHED_DATA = Path("benchmark/edgebench/studies/portfolio-ten-arm-20261006/data") + + @pytest.mark.parametrize("strategy", [None, "off", "evidence_conditioned_v1"]) def test_report_preserves_observed_stride_version_and_absence(trial, strategy): runs, selection, output, task = trial @@ -394,3 +400,79 @@ def test_cli_rejects_inconsistent_reports(trial, operation): assert '"verified": true' not in result.stdout if operation == "export": assert not trial[2].exists() + + +def _windows_text_writes(monkeypatch): + """Mirror the os.linesep translation CPython applies to Windows text writes.""" + real_open = Path.open + + def windows_text_write(self, mode="r", *args, **kwargs): + if "b" not in mode and any(flag in mode for flag in "wax"): + if kwargs.get("newline") is None: + kwargs["newline"] = "\r\n" + return real_open(self, mode, *args, **kwargs) + + monkeypatch.setattr(Path, "open", windows_text_write) + + +def _publish_and_recheck(tmp_path, build_data): + """Publish files through Git with text conversion forced on, then verify.""" + repository = tmp_path / "repository" + repository.mkdir() + subprocess.run(["git", "init", "-q", str(repository)], check=True) + shutil.copyfile(ROOT / ".gitattributes", repository / ".gitattributes") + data = build_data(repository) + (repository / "control.txt").write_bytes(b"control\n") + + def git(*args): + return subprocess.run( + ["git", "-C", str(repository), "-c", "core.autocrlf=true", *args], + check=True, capture_output=True, text=True, + ) + + relative = data.relative_to(repository).as_posix() + pin = git("check-attr", "eol", f"{relative}/index.json").stdout + assert pin.strip().endswith("eol: lf"), "the LF pin must apply to this path" + git("add", "--", ".gitattributes", relative, "control.txt") + checkout = tmp_path / "checkout" + checkout.mkdir() + git("checkout-index", "--all", f"--prefix={checkout.as_posix()}/") + assert (checkout / "control.txt").read_bytes() == b"control\r\n", "conversion must be active" + restored = checkout / relative + assert verify(restored)["verified"] is True + for path in data.rglob("*"): + if path.is_file(): + assert (restored / path.relative_to(data)).read_bytes() == path.read_bytes() + return restored + + +def test_json_writer_ignores_host_text_newline_translation(tmp_path, monkeypatch): + _windows_text_writes(monkeypatch) + value = {"best_score": 2, "note": "first line\nsecond line"} + path = tmp_path / "value.json" + write(path, value) + expected = json.dumps(value, indent=2, ensure_ascii=False, allow_nan=False) + "\n" + assert path.read_bytes() == expected.encode("utf-8") + + +def test_published_archive_materializes_byte_exact_under_autocrlf(tmp_path): + def build(repository): + data = repository / PUBLISHED_DATA + shutil.copytree(ROOT / PUBLISHED_DATA, data) + return data + + _publish_and_recheck(tmp_path, build) + + +def test_exported_report_survives_autocrlf_publication(tmp_path, trial, monkeypatch): + """A Windows host writes CRLF for default text writes, but this export binds + its own bytes, so Git must store and restore exactly what was written.""" + _windows_text_writes(monkeypatch) + + def build(repository): + data = repository / "benchmark/edgebench/studies/review-roundtrip/data" + runs, selection, _unused, _task = trial + export(runs, selection, data, observed_at="2026-01-02T00:00:00+00:00") + return data + + _publish_and_recheck(tmp_path, build)