From 623d7a6f3c276cb7505d5ecc0f530573be87b010 Mon Sep 17 00:00:00 2001 From: JasonBuildAI Date: Tue, 13 Oct 2026 15:39:42 +0800 Subject: [PATCH 1/6] fix(benchmark): run the EdgeBench report adapter and suite on native Windows On native Windows `uv run --extra test python -m pytest -q`, the command CONTRIBUTING documents as the full local check, aborts with "Interrupted: 2 errors during collection" and runs none of the 21851 collected tests. Both errors come from `benchmark/edgebench/feedback_hook.py`, which imported POSIX-only `fcntl` at module scope, so the report adapter's suite and the hook's own suite could not be imported at all. The same import failure makes the documented verifier `python -m benchmark.edgebench.export_report --verify ` unusable on that host before it reads any evidence. Delivery keeps its flock-based session lock; guard the import the way `loopx/file_lock.py` guards `fcntl`/`msvcrt`, refuse delivery with a typed error where no supported lock backend exists, and skip the delivery suite itself where POSIX flock is absent. The report adapter has no lock requirement, so it imports and its tests run there again. The published study archive binds sha256 of its own files in `index.json`, so the checkout line-ending translation of the same host made all 13 bindings mismatch and the verifier report the published evidence as corrupted. Pin study archives to LF, as the immutable v1 archive already is. Signed-off-by: JasonBuildAI --- .gitattributes | 4 ++++ benchmark/edgebench/feedback_hook.py | 10 +++++++++- .../studies/portfolio-ten-arm-20261006/README.md | 3 +++ benchmark/tests/test_edgebench_feedback_hook.py | 10 ++++++++++ 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/.gitattributes b/.gitattributes index 6cd6285821..be4a97d47a 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 files in index.json, so a +# CRLF checkout must not rewrite them either. +benchmark/edgebench/studies/**/data/** text eol=lf 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..0bf036eca9 100644 --- a/benchmark/edgebench/studies/portfolio-ten-arm-20261006/README.md +++ b/benchmark/edgebench/studies/portfolio-ten-arm-20261006/README.md @@ -153,6 +153,9 @@ 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`. + 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..861bf94aa2 100644 --- a/benchmark/tests/test_edgebench_feedback_hook.py +++ b/benchmark/tests/test_edgebench_feedback_hook.py @@ -8,6 +8,8 @@ import pytest +pytest.importorskip("fcntl", reason="delivery serializes its session reader with POSIX flock") + from benchmark.edgebench.feedback_hook import deliver, install from benchmark.edgebench import feedback_hook @@ -159,3 +161,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() From ce28ae0f192da3ef03e392cf43f084af0fc043e7 Mon Sep 17 00:00:00 2001 From: JasonBuildAI Date: Tue, 13 Oct 2026 16:51:26 +0800 Subject: [PATCH 2/6] test(benchmark): exercise the no-lock refusal on non-POSIX hosts The module-level fcntl importorskip skipped the whole delivery suite on hosts without POSIX flock, including the refusal test added for exactly that condition, so the platform this fix targets never ran it. Keep the delivery tests behind a flock marker and leave the refusal case unmarked; it now runs on Windows against a real missing backend. Signed-off-by: JasonBuildAI --- benchmark/tests/test_edgebench_feedback_hook.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/benchmark/tests/test_edgebench_feedback_hook.py b/benchmark/tests/test_edgebench_feedback_hook.py index 861bf94aa2..3ca7470f0e 100644 --- a/benchmark/tests/test_edgebench_feedback_hook.py +++ b/benchmark/tests/test_edgebench_feedback_hook.py @@ -8,11 +8,17 @@ import pytest -pytest.importorskip("fcntl", reason="delivery serializes its session reader with POSIX flock") - 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) @@ -35,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) @@ -58,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' @@ -94,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'} @@ -108,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) @@ -138,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)) From bfeda8f910c3906b89383ae4d6e7f610c40a92ab Mon Sep 17 00:00:00 2001 From: JasonBuildAI Date: Tue, 13 Oct 2026 16:51:26 +0800 Subject: [PATCH 3/6] docs(benchmark): note the one-time re-materialization for the pinned archive Git does not rewrite files already on disk when .gitattributes changes, so an existing checkout keeps reporting digest mismatches until the pinned data directory is restored once. Signed-off-by: JasonBuildAI --- .../edgebench/studies/portfolio-ten-arm-20261006/README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/benchmark/edgebench/studies/portfolio-ten-arm-20261006/README.md b/benchmark/edgebench/studies/portfolio-ten-arm-20261006/README.md index 0bf036eca9..cc050c8873 100644 --- a/benchmark/edgebench/studies/portfolio-ten-arm-20261006/README.md +++ b/benchmark/edgebench/studies/portfolio-ten-arm-20261006/README.md @@ -154,7 +154,10 @@ uv run --extra test python -m benchmark.edgebench.export_report \ ``` `.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`. +verify even when Git checks out other text files with `core.autocrlf=true`. Git +does not rewrite files already materialized in an existing checkout, so run +`git checkout -- benchmark/edgebench/studies/portfolio-ten-arm-20261006/data` +once there before verifying. For another terminal collection, supply an explicit JSON list of `{"row": , "settings": }` From 1357ca22d1699720bb2bdf123323b81984080674 Mon Sep 17 00:00:00 2001 From: JasonBuildAI Date: Tue, 13 Oct 2026 17:21:32 +0800 Subject: [PATCH 4/6] docs(benchmark): give the archive recovery step that actually replaces stale files A plain `git checkout -- ` is a no-op for files that already exist in the worktree: with `text eol=lf` their cleaned content matches the index, so Git considers them unchanged and skips the write. Prescribing it left an existing checkout still failing verification. Delete the stale directory first and restore it from the index; re-ran it here: no CR bytes remain and --verify returns runs=10, files=13, verified=true. Signed-off-by: JasonBuildAI --- .../studies/portfolio-ten-arm-20261006/README.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/benchmark/edgebench/studies/portfolio-ten-arm-20261006/README.md b/benchmark/edgebench/studies/portfolio-ten-arm-20261006/README.md index cc050c8873..4d9ce84f3c 100644 --- a/benchmark/edgebench/studies/portfolio-ten-arm-20261006/README.md +++ b/benchmark/edgebench/studies/portfolio-ten-arm-20261006/README.md @@ -155,9 +155,14 @@ uv run --extra test python -m benchmark.edgebench.export_report \ `.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`. Git -does not rewrite files already materialized in an existing checkout, so run -`git checkout -- benchmark/edgebench/studies/portfolio-ten-arm-20261006/data` -once there before verifying. +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": }` From 8c6601d48acd44de37f97cc75d492a176b6a0603 Mon Sep 17 00:00:00 2001 From: JasonBuildAI Date: Wed, 14 Oct 2026 07:46:09 +0800 Subject: [PATCH 5/6] fix(benchmark): bind exported report bytes before Git text conversion export_report.write serialized JSON through the host text default, so a native Windows export wrote CRLF bytes and index.json bound the CRLF digests. The study archive pins text eol=lf, which normalizes the file to LF at git add, so a fresh materialization held different bytes than the ones hashed and --verify failed with File binding failed: run-rows.json. Write JSON with explicit LF bytes so the export is byte-identical on every platform, matching csv_write. State the LF-byte invariant in the archive note and the study README. Signed-off-by: JasonBuildAI --- .gitattributes | 4 ++-- benchmark/edgebench/export_report.py | 3 ++- .../studies/portfolio-ten-arm-20261006/README.md | 10 ++++++---- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/.gitattributes b/.gitattributes index be4a97d47a..27961f90ea 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,6 +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 files in index.json, so a -# CRLF checkout must not rewrite them either. +# 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/studies/portfolio-ten-arm-20261006/README.md b/benchmark/edgebench/studies/portfolio-ten-arm-20261006/README.md index 4d9ce84f3c..101e04f7ac 100644 --- a/benchmark/edgebench/studies/portfolio-ten-arm-20261006/README.md +++ b/benchmark/edgebench/studies/portfolio-ten-arm-20261006/README.md @@ -154,10 +154,12 @@ uv run --extra test python -m benchmark.edgebench.export_report \ ``` `.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`. 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: +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 From 8e6614ca16b9236d70126411e07e40adac558533 Mon Sep 17 00:00:00 2001 From: JasonBuildAI Date: Wed, 14 Oct 2026 07:46:09 +0800 Subject: [PATCH 6/6] test(benchmark): pin report bytes across a converting Git publication Walk a new export through Git publication with core.autocrlf=true and a fresh materialization, checking the bound digests and the materialized bytes. The export case fails when the writer leaks host newlines, the published-archive case fails when the LF pin stops applying, and a control file proves checkout conversion stayed active. The Windows text layer is modeled by forcing newline="\r\n" on default text writes, so the regression also fails on POSIX test hosts. Signed-off-by: JasonBuildAI --- benchmark/tests/test_edgebench_report.py | 82 ++++++++++++++++++++++++ 1 file changed, 82 insertions(+) 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)