fix(multiharness): wire fence evidence, sentinel, and adapter-failure packaging - #1049
fix(multiharness): wire fence evidence, sentinel, and adapter-failure packaging#1049johnjhughes wants to merge 1 commit into
Conversation
… packaging Five merge blockers from the adversarial review of stack #1042 (legalforecastbench-2ve1). CI was green on all five PRs; none of these is caught by the tests that already existed. The published native_tools_enabled / server_side_web_tools_disabled booleans were hardcoded True. Every parser already computed the observation; nothing read it. A Claude run whose init lists WebSearch would still publish server_side_web_tools_disabled: true and a scored LFB row. The summary now derives both booleans from the parser, and a row whose transcript shows provider-executed web retrieval is refused as sandbox_denial rather than scored. The release transcript also binds stdout_sha256 so a post-hoc edit of container-logs/*.stdout cannot survive repackaging. The workspace sentinel was tests-only while community-submissions.md claimed every run wrote the token. The probe now runs once per harness before scored rows, through the adapter's own container path, and the run refuses if the token does not come back. The docs sentence now describes that probe. An adapter-raised exception (proxy-bind timeout, missing evidence file) emitted a public_summary without adapter_version, so packaging died after the money was spent. Failure summaries now carry the adapter identity, and the packager takes harness identity from succeeded rows so one thin failure does not make a 100-row run unpackageable. The publication secret scan decoded text as strict UTF-8 and swallowed UnicodeDecodeError, so a .stdout identical but for one 0xff published ANTHROPIC_API_KEY unflagged. It now decodes with errors=replace. Host-path redaction knew /home and /Users and missed /work, where this repo actually lives. /work, /root, and /srv are rewritten to placeholders; the tree-wide absolute-path refusal is not applied, because it would also refuse /workspace/foo.
PR SummaryHigh Risk Overview Web fence and native-tools summary fields no longer ship as constants. Sentinel probe runs once per harness (under Packaging and guardrails: adapter-exception summaries include Reviewed by Cursor Bugbot for commit ba91702. Bugbot is set up for automated code reviews on this repo. Configure here. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_4df1733c-1ce0-4202-9ef5-46169e3df2f7) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba9170276b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "packet_sha256": request.task.task_sha256, | ||
| "prompt_sha256": prompt_sha256, | ||
| "response_sha256": output_sha256, | ||
| "stdout_sha256": release_bytes_sha256(stdout_bytes), |
There was a problem hiding this comment.
When a release-backed row's container log is changed after execution but before a package is built, this new field does not make the change detectable: _validate_transcript_binding() checks only request, packet, prompt, and response, while build_harness_lane_results_package() archives the current log without rehashing it against stdout_sha256. The altered stdout therefore produces a valid new package while the transcript still claims the old digest, so re-read the recorded stdout and compare it before accepting or packaging the row.
AGENTS.md reference: AGENTS.md:L24-L24
Useful? React with 👍 / 👎.
| record_path = run_root / "sentinel-probe.json" | ||
| if record_path.is_file(): | ||
| self._require_proven_sentinel(record_path) | ||
| return |
There was a problem hiding this comment.
In a MultiHarnessRunConfig containing multiple ContainerCliAdapter instances—or a fresh run that reuses an output directory—this single unkeyed record is created by the first invocation and every later adapter accepts it solely because its verdict is proven. _observed_native_tools() reads the same file, so one harness can inherit another harness's probe and publish native_tools_enabled: true without ever passing its own sentinel; store and validate the record against the adapter/image identity for the current run.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| # Adapter-raised failures and web-fence refusals may omit identity fields | ||
| # or carry a different observed posture. The harness identity is the | ||
| # succeeded rows'; failed rows still count in status and failure_class. | ||
| identity_rows = [row for row in rows if row.get("failure_class") is None] |
There was a problem hiding this comment.
When incomplete_run_policy="record_failure" catches an adapter exception, _failure_result() emits no failure_class, so this new predicate places the failed summary in identity_rows and the later aggregation counts it as "none". A mixed package can consequently report one failed canonical row while reporting two rows with no failure class; tag adapter exceptions with a non-null failure class or retain canonical row status when grouping.
Useful? React with 👍 / 👎.
|
Do not merge this PR. Do not patch it again. This stack is not mergeable. This is the tip of the six-PR harness-lane stack (base #1040 … this B1–B5 layer). Two independent reviews of this tip each found blockers the previous missed. The review-loop cap is 3; a seventh layer will not converge. Verified design-level failures that this layer did not (and cannot cheaply) fix:
B1–B5 on this layer are incomplete or bypassed (hardcoded measurement booleans, hostname redaction misses #1040 (base) conflicts with Converted to draft so it cannot merge. Do not close — keep as salvage reference (egress proxy resisted 25 hostile CONNECT hosts; per-run Tracked on |
1 participant
Fixes the five merge blockers in
legalforecastbench-2ve1so stack #1042 can merge.This is a sixth layer on top of #1045. The lower PRs stay as reviewed; the blockers live in the code they introduced, and this PR is the one that must land with them.
Blockers
B2 (the one that matters most).
adapter.pypublishednative_tools_enabled: Trueandserver_side_web_tools_disabled: Trueas literals. Every parser already computed the real observation; nothing read it. A Claude run whose init listsWebSearchwould still publishserver_side_web_tools_disabled: trueand a scored LFB row. These are real federal cases whose outcomes are one search away.The summary now derives both booleans from the parser. A row whose transcript shows provider-executed web retrieval is refused as
sandbox_denialrather than scored. The release transcript also bindsstdout_sha256, so editingcontainer-logs/*.stdoutafter the fact cannot survive repackaging.B3.
sentinel.pyhad no production caller.docs/community-submissions.mdsaid every run wrote the token. The probe now runs once per harness before scored rows, through the adapter's own container path, and the run refuses if the token does not come back. The docs sentence now describes that probe, not a ride-along on scored prompts (which would falsify the digest-bound prompt).B1. An adapter-raised exception (
_failure_result) omittedadapter_version. Packaging then died on_require_str(..., 'adapter_version')after a paid 100-row run. Failure summaries now carry the adapter identity, and the packager takes harness identity from succeeded rows so one thin failure does not make the run unpackageable.B4.
_scan_text_contentdecoded as strict UTF-8 and swallowedUnicodeDecodeError. A.stdoutidentical but for one0xffpublishedANTHROPIC_API_KEYunflagged. Decode now useserrors='replace'.B5. Host-path redaction knew
/homeand/Usersand missed/work/, where this repo actually lives./work,/root, and/srvare rewritten to placeholders. The tree-wide_ABSOLUTE_PATHcheck is not applied to the community tree — that would also refuse/workspace/foo.Tests
Focused (serial):
Also
uv run pyrightanduv run ruff checkon the touched files.Do not merge the stack until this layer is reviewed. The lower five were already green; these holes were not in those tests.