feat(sandbox): driver-independent protected mock service for CLI fixtures

[dmorosanu]

What

Adds a driver-independent protected_mocks fixture service: task fixtures are loaded by a small host-side per-run server, and the agent's sandbox only ever contains a thin client shim. Extracted from #87 (codex/uid-gid-agent-isolation) and made portable; supersedes the protected_mock portion of that PR, which will rebase onto this and re-add the Docker UID/GID wiring.

• src/coder_eval/protected_mock/{protocol,server,client,runtime}.py - JSON-over-socket request/response service with per-tool request budgets, bounded payload sizes, and prefix-limited passthrough (unchanged from the donor where it worked).
• Per-run endpoint: AF_UNIX socket in a run-scoped scratch dir when the platform supports it (probed with a real bind), else TCP on 127.0.0.1 with an ephemeral port. Every request carries a per-run random token the generated shim bakes in - same-user hygiene against other local processes casually querying the service, not a security boundary (documented as such).
• match_mode: subset added alongside exact/normalized: all rule tokens must appear in the invocation's normalized token set; rules scan in fixture-file order, first match wins; exact/normalized take precedence; duplicates allowed; empty subset argv rejected at load.
• Driver gate inverted: supported under driver: tempdir; under driver: docker validation fails closed ("requires the UID/GID isolation layer; not yet available") until Add UID/GID isolation for evaluated agents #87 lands.
• Shims: one protected_mocks/<tool> shim (+ .cmd twin) per entry, PATH-prepended like mock_path_dirs; each shim carries its endpoint/token/call-log path itself and does not rely on the agent process environment.
• Fixture paths resolve against the task YAML's directory host-side (like uipath_eval.eval_set); fixture bytes are never copied into the sandbox.
• Call log: cli_called-schema JSONL written to protected_mock_calls.jsonl next to task.json in the run dir, outside the sandbox. Diagnostic-only for now: cli_called resolves its log field sandbox-relative and cannot read the host-side file (documented).
• Orchestrator wiring: server starts after sandbox setup / before agent start; teardown is unconditional in _cleanup, which runs on every exit path. environment_info records the endpoint kind and a SHA-256 digest of the fixture contents.

Why

Eval fixtures are grading material and must not be readable in the agent workspace. The current approach ships them into the sandbox behind a reversible encoding, and evaluated agents have read them to pass tests they should have failed - an encoding is not a boundary. Host-side loading removes the data from the workspace entirely, and the docker-only version in #87 could not run on the tempdir driver the test suites actually use (or on Windows).

Validation

• ruff format --check, ruff check, pyright: clean.
• Custom lint (tests/test_custom_lint.py): 171 passed.
• Full suite: 3948 passed, 103 skipped, plus the 2 pre-existing Windows symlink failures in tests/test_sandbox.py (WinError 1314 without Developer Mode, unrelated).
• Coverage: 89.89% (gate 80%).
• tests/test_protected_mock.py (28 tests): validation gates (tempdir OK, docker fails closed, name uniqueness, record_cli overlap), exact/normalized/subset matching (order, first-wins, precedence, budget exhaustion, duplicate rules), passthrough prefix+cache, TCP loopback end-to-end with token (mismatch rejected), AF_UNIX end-to-end where available, per-run endpoint isolation (two servers), runtime teardown (terminate, scratch dir removed), fixture resolution against task dir, shim generation (endpoint baked in, no fixture bytes anywhere in the sandbox tree), self-sufficient shim execution against a live service, and a full Orchestrator.run() lifecycle test (audit record, seeded call log, server stopped, preserved sandbox carries only the shim).

[github-actions]

Claude finished @dmorosanu's task in 1m 21s —— View job

---

Code Review

Todo List

• Read review guidelines in .github/code_review.md
• Read CLAUDE.md for project conventions
• Get full PR diff
• Review each changed file with full context
• Check cross-file consistency
• Analyze what's missing
• Provide design-level scrutiny
• Submit final review

Starting review...

[uipreliga]

Review: coder_eval — pr:90

Scope: pr:90 · branch feat/protected-mock-portable · 92e6e11 · 2026-08-08T14:55Z · workflow variant

Change class: complex — introduces a new host-side socket service, a per-run auth token, a new fixture match mode, orchestrator lifecycle wiring, and generated shim scripts; correctness requires reasoning about concurrency, teardown, and trust boundaries

Architecture, API surface, and security posture are strong (9.4/9.6/9.3) and no critical defects survived verification, but the risk is concentrated in the new protected_mock package, where an untyped hand-parsed fixture schema, a 5s-client-vs-60s-server timeout mismatch, and a failure-poisoning passthrough cache can each flip a task's score for byte-identical agent output, and a PR-introduced pyright error currently breaks make typecheck for everyone — bottom line: an 8.6 codebase that is sound in design but needs a short, targeted correctness-and-typing fix list on this package before it can be trusted as a grading input.

Summary

Axis	Score	🔴	🟠	🟡	🔵	Top Issue
1. Code Quality & Style	8.8 / 10	0	0	2	2	Unreachable peer-credential machinery: allowed_peer_uids is hardcoded to None and _peer_uid() is never callable
2. Type Safety	6.9 / 10	0	2	2	1	Fixture JSON schema is hand-parsed as an untyped dict with no unknown-key rejection: a typo'd match_mode/exit_code silently changes behaviour
3. Test Health	7 / 10	0	1	4	0	Unhappy-path branches of the new protected_mock package are untested (wire-protocol errors, fixture validation, force-kill/rmtree-retry/startup-timeout teardown)
4. Security	9.3 / 10	0	0	1	2	Generated shim bakes the runtime-dir socket path into the agent workspace; that dir's mock-config.json holds absolute fixture paths, so a same-user agent reaches the grading material in two hops
5. Architecture & Design	9.4 / 10	0	0	1	1	generate_protected_mock_shims duplicates ~30-38 lines of _generate_cli_recorders (collision scan, wipe+mkdir, Windows .cmd twin)
6. Error Handling & Resilience	8.8 / 10	0	0	2	2	Orchestrator assigns self._protected_mock_runtime only after await asyncio.to_thread(runtime.start) (orchestrator.py:1149→1152), so a cancellation at that await orphans the spawned server process and its cepm-* scratch dir
7. API Surface & Maintainability	9.6 / 10	0	0	0	4	fixture: path and fixture schema are validated only inside the spawned server, so coder-eval plan reports a broken task as valid
8. Evaluation Harness Quality	9 / 10	0	1	0	0	Client socket timeout (5s) is 12x shorter than the server's 60s passthrough budget, so slow passthrough calls return a spurious exit 125

Overall Score: 8.6 / 10 · Weakest Axis: Type Safety at 6.9 / 10
Totals: 🔴 0 · 🟠 4 · 🟡 12 · 🔵 12 across 8 axes.

Blockers

1. [Axis 2] Fixture JSON schema is hand-parsed as an untyped dict with no unknown-key rejection: a typo'd match_mode/exit_code silently changes behaviour (src/coder_eval/protected_mock/server.py:135) — The fixture file is a user-authored config surface documented in docs/TASK_DEFINITION_GUIDE.md, yet unlike every other YAML/JSON-consuming surface in this repo it has no Pydantic model and no extra="forbid" equivalent — it is parsed with bare dict.get defaults, so unknown/typo'd keys are silently dropped:

• server.py:135 — match_mode = entry.get("match_mode", "exact")
• server.py:98-100 — exit_code = raw.get("exit_code", 0) / stdout = raw.get("stdout", "") / stderr = raw.get("stderr", "")
• server.py:151 — raw.get("default", {...})

Concrete failure: this same feature's call log uses the key "exit" (client.py:35: entry = {"ts": ..., "tool": tool, "argv": argv, "exit": exit_code}), so a fixture author writing "exit": 3 instead of "exit_code": 3 gets a silently-successful exit 0 response; likewise "match-mode": "subset" silently degrades to exact matching, so uip rpa get-errors --output json falls through to the default (exit 2) and a correctly-behaving agent is graded as failing. Nothing raises. This directly contradicts the new doc line "Malformed responses, oversized output, request-budget exhaustion, and service startup failures all fail loudly."

Fix: define the fixture schema as Pydantic models (FixtureFile / FixtureResponse / FixtureDefault) with model_config = ConfigDict(extra="forbid"), match_mode: Literal["exact", "normalized", "subset"] = "exact", exit_code: int = Field(default=0, ge=0, le=255) (Pydantic also rejects true for an int, which the current isinstance(exit_code, int) check accepts because isinstance(True, int) is True — "exit_code": true currently yields shim exit 1), and validate with model_validate_json. Add a test asserting an unknown key in a fixture response raises at load.
2. [Axis 2] ProtectedMockConfig.tool drops the pattern allowlist its parallel RecordedCli.tool field carries, so a name containing a quote renders a syntactically broken shim (src/coder_eval/models/sandbox.py:420) — ProtectedMockConfig.tool (line 420) is declared with no constraint — tool: str = Field(description="Bare executable name presented to the agent (for example, 'uip')") — and is only guarded by a partial denylist in validate_tool_name (line 440): if not value or value != value.strip() or value in {".", ".."} or "/" in value or "\\" in value:. The directly parallel field in the same file, RecordedCli.tool (line 348), uses an allowlist with exactly the rationale that applies here: pattern=r"^[A-Za-z0-9._+-]+$" … "the value is interpolated into generated shim source, so a quote or newline would emit a broken script".

The protected-mock value IS interpolated raw (not via !r) into generated source in two places:

• src/coder_eval/protected_mock/runtime.py:231 — """Protected mock shim for {tool} - generated by coder_eval ... (module docstring of the generated shim)
• src/coder_eval/sandbox.py:646 — f'"{interpreter}" "%~dp0{spec.tool}" %*' (generated .cmd twin)

Failure scenario: tool: 'uip"""' passes validation and renders a shim whose docstring terminates early — a syntax error at best, arbitrary trailing code executed under the harness interpreter at worst; tool: 'uip x' (interior space) passes validation and writes a shim file that PATH lookup can never resolve, with no error. Fix: add pattern=r"^[A-Za-z0-9._+-]+$" to the field (matching RecordedCli.tool) and keep the suffix/reserved checks in the validator; add a test that a tool name containing a quote or a space is rejected at load.
3. [Axis 3] Unhappy-path branches of the new protected_mock package are untested (wire-protocol errors, fixture validation, force-kill/rmtree-retry/startup-timeout teardown) (src/coder_eval/protected_mock/server.py:302) — protected_mock/server.py is the weakest module in the PR at 72.63%, and the uncovered part is exactly its error surface — the branches that exist for a client that is NOT the trusted shim. Nothing in the 764-line test file drives the real handler with anything other than a well-formed request (happy path) or a wrong token. Untested branches, verified by reading each line:

(a) Request handling (ProtectedMockHandler.handle, no test at all):

• server.py:302 self._write(CommandResponse(64, "", "protected mock: invalid request size\n")) — oversized line AND the truncated/half-closed-connection case (not line.endswith(b"\n")), both folded into one untested guard at :301.
• server.py:307, 311, 313, 315 — non-JSON bytes, wrong version, non-dict envelope, non-string tool/argv all funnel to self._write(CommandResponse(64, "", "protected mock: invalid request\n")) at :315.
• server.py:348 — the oversized-response fallback in _write (reachable: _response caps stdout+stderr at MAX_RESPONSE_BYTES // 2 before JSON escaping, and ensure_ascii=True expands non-ASCII up to 3x).

(b) Fixture/config schema validation — 16 raise ValueError sites, only two of which (duplicate argv, non-empty for subset) have a test: server.py:97, 102, 104, 107 (response exceeds the configured size limit), 119, 122, 131, 134, 137, 159, 175, 178, 182, 188, 190, 195 (invalid passthrough prefixes). docs/TASK_DEFINITION_GUIDE.md asserts "Malformed responses, oversized output, request-budget exhaustion, and service startup failures all fail loudly" — only the last two are tested.

(c) Passthrough failure handling (the one path that shells out to a real tool mid-eval): server.py:272 CommandResponse(70, "", "protected mock: passthrough failed\n") and :275 "passthrough response exceeds size limit". Note server.py:257 (passthrough is unavailable, exit 69) is unreachable, not merely untested: _load_tool raises at :159 whenever prefixes are set and shutil.which returns None, so passthrough_executable is None implies empty passthrough_prefixes, which makes dispatch's any(...) at :246 false — either delete it or make it reachable.

(d) Same theme on the client (82.61%): client.py:40 (call-log OSError), :53 (response exceeded size limit), :64, :95 (ENDPOINT_ENV is not set), :101/:106/:108 (invalid response envelope / exit_code / streams).

Add a ProtectedMockHandler test that opens a raw socket to a _LiveServer and sends: a 65 KB line, a line with no trailing newline (then closes), b'{bad json}\n', {"version": 999,...}, and {"argv": [1]} — asserting exit 64 and that the server survives and still answers a subsequent valid request. Add a table-driven pytest.mark.parametrize over the malformed-fixture cases asserting load_config raises with the specific message (these are the only diagnostic a task author gets — the runtime surfaces them as a generic RuntimeError("...exited during startup...")). Add passthrough tests with subprocess.run monkeypatched to raise OSError and to return an oversized stdout.
4. [Axis 8] Client socket timeout (5s) is 12x shorter than the server's 60s passthrough budget, so slow passthrough calls return a spurious exit 125 (src/coder_eval/protected_mock/protocol.py:19) — protocol.py:19 sets CLIENT_TIMEOUT_SECONDS = 5.0, and client.py:67/client.py:74 apply it as the socket timeout (connection.settimeout(CLIENT_TIMEOUT_SECONDS) / socket.create_connection((host, port), timeout=CLIENT_TIMEOUT_SECONDS)), so it bounds recv in _receive_line, not just connect. Server-side, server.py:59 sets PASSTHROUGH_TIMEOUT_SECONDS = 60 and server.py:259-268 runs the real tool under that budget while holding self.passthrough_lock (server.py:252) for the whole subprocess. Consequences, all invisible to the harness: (1) any passthrough_argv_prefixes invocation taking >5s (uip docsai ask is a live LLM query — routinely >5s) makes the client raise socket.timeout, caught by except (OSError, ...) at client.py:109, printing protected mock client: service unavailable or invalid response and returning 125; (2) the budget was already spent at server.py:228-231 before dispatch, so the failed call is charged; (3) the server still completes and caches the result (server.py:276), so an immediate retry of the identical argv returns exit 0 instantly — meaning the SAME agent invocation scores differently depending on whether the live tool crossed the 5s line, which is exactly the Axis-8 non-determinism class; (4) because passthrough_lock is held for up to 60s, a second concurrent passthrough call is guaranteed to time out client-side. Fix: derive the client deadline from the server's worst case (e.g. CLIENT_TIMEOUT_SECONDS >= PASSTHROUGH_TIMEOUT_SECONDS + slack, or send a fast 'in progress' ack and keep the short timeout only for fixture dispatch), release passthrough_lock around subprocess.run (lock only the cache read/write), and refund the budget decrement when the response is not delivered. No test exercises passthrough through the socket — tests/test_protected_mock.py:328 (test_passthrough_is_prefix_limited_and_cached) calls ProtectedMockServer.dispatch directly with a mocked subprocess.run, so this mismatch is entirely uncovered; add a live-server test with a passthrough that sleeps past 5s.

Non-blocking, but please consider before merge

1. [Axis 1] Unreachable peer-credential machinery: allowed_peer_uids is hardcoded to None and _peer_uid() is never callable (src/coder_eval/protected_mock/server.py:222) — _init_state is the ONLY writer of the field and it writes a constant: self.allowed_peer_uids = None (server.py:222). grep -rn "allowed_peer_uids" src tests returns only the declaration (215), that assignment (222), and the guard that reads it (320) — no caller in src/ or tests/ ever sets it to a non-None value. So the guard if server.allowed_peer_uids is not None and self._peer_uid() not in server.allowed_peer_uids: (320) is statically dead, and the whole _peer_uid() helper (325-331, including the struct.calcsize("3i") / SO_PEERCRED unpack) is unreachable. The class docstring says "the Docker isolation layer will set it", but SandboxConfig.validate_configuration in models/sandbox.py rejects protected_mocks under driver: docker outright, so the only stated future consumer is currently blocked by a hard validation error. This is speculative 'just in case' machinery that reads as a live defense-in-depth layer. Delete the field, the guard, and _peer_uid() (plus the now-inaccurate 'peer-credential checks ... run only where available AND configured' sentence in the module docstring at server.py:7-10) and reintroduce them with the Docker isolation layer that actually needs them.
2. [Axis 1] load_config (server.py:172-197, radon C(20)) hand-re-validates a wire config the harness itself generates from already-validated ProtectedMockConfig instances (src/coder_eval/protected_mock/server.py:172) — load_config (server.py:172, radon C(20)) is ~25 lines of manual isinstance/range checks over a file that is written 60 lines earlier by trusted harness code: ProtectedMockRuntime.start serializes {"tool": mock.tool, "fixture": str(path), "max_requests": mock.max_requests, "passthrough_argv_prefixes": mock.passthrough_argv_prefixes} (runtime.py:114-122) from a list of already-validated ProtectedMockConfig instances. Every rule re-asserted here is already enforced on the model — e.g. if not isinstance(tool, str) or not tool or tool in loaded: raise ValueError("mock config tools must have unique non-empty names") (187-188) restates validate_tool_name plus the sandbox.protected_mocks tool names must be unique check in SandboxConfig.validate_configuration, and not isinstance(max_requests, int) or max_requests < 1 (189) restates ge=1, le=10_000. That is a second source of truth for four fields that must now be kept in sync by hand, and it is the bulk of the routed CC-20. Define one Pydantic model for the wire config (server.py can import coder_eval.models — it is launched as python -m coder_eval.protected_mock.server with coder_eval on the path, per runtime.py:132-143) and model_validate_json it, keeping hand-rolled parsing only for _load_tool (111, also C(20)), whose input is genuinely user-authored fixture JSON.
3. [Axis 2] pyright error introduced: implicit string concatenation at src/coder_eval/protected_mock/runtime.py:170 fails make typecheck (reportImplicitStringConcatenation is set at pyproject.toml:258) (src/coder_eval/protected_mock/runtime.py:170) — pyproject.toml:257 sets reportImplicitStringConcatenation = "error", and uv run pyright src/coder_eval/protected_mock/ on the PR HEAD reports exactly:
   runtime.py:170:13 - error: Implicit string concatenation not allowed (reportImplicitStringConcatenation) — the offending statement is

raise RuntimeError(
    f"protected mock server did not publish its endpoint within {waited:.1f}s "
    f"(deadline {STARTUP_TIMEOUT_SECONDS}s)" + self._server_stderr_suffix()
)

This breaks the repo's type gate (make typecheck / make verify) for everyone on main, and contradicts the PR description's "pyright: clean" claim. Fix by joining with + like the neighbouring code already does (e.g. f"...within {waited:.1f}s " + f"(deadline {STARTUP_TIMEOUT_SECONDS}s)" + self._server_stderr_suffix()), or build the message in a local variable.
4. [Axis 2] Any-typed socketserver base class erases the server class hierarchy and forces three unjustified # pyright: ignore suppressions (src/coder_eval/protected_mock/server.py:200) — server.py:200 — _UnixStreamServer: Any = getattr(socketserver, "UnixStreamServer", object) — makes _UnixMockServer's base unknown to the type checker, and create_server declares its return as ProtectedMockServer, a class that does not declare any of the socketserver API its callers use. The result is three suppressions with no justification comment (the repo's five pre-existing pyright: ignores are all self-evident optional-extra imports):

• server.py:294 — super().__init__(path, ProtectedMockHandler) # pyright: ignore[reportCallIssue]
• server.py:412 — server.serve_forever(poll_interval=0.2) # pyright: ignore[reportAttributeAccessIssue]
• server.py:414 — server.server_close() # pyright: ignore[reportAttributeAccessIssue]

So neither the constructor arity of the Unix transport nor the two lifecycle calls in serve() are type-checked at all — a refactor of create_server's return type or of serve_forever/server_close usage will not be caught. Fix: gate the import on sys.platform (if sys.platform != "win32": from socketserver import UnixStreamServer) so the base class stays typed, and type create_server's return as the socketserver base (or a small Protocol declaring serve_forever / server_close / server_address) so the three ignores can be deleted; if any must stay, append the reason inline.
5. [Axis 3] AF_UNIX skip guard probes the over-long pytest tmp_path (test skips on macOS while production binds fine), and transport="auto" selection is never asserted by any test (tests/test_protected_mock.py:436) — Two related gaps around the preferred endpoint:

1. The skip guard at tests/test_protected_mock.py:436-437 (if not _unix_transport_usable(tmp_path): pytest.skip("AF_UNIX stream sockets not usable on this platform")) probes by binding tmp_path / "probe.sock". I verified on this macOS box that a pytest-style tmp_path probe path is 130 chars and fails with OSError: AF_UNIX path too long, while the path production actually uses — tempfile.mkdtemp(prefix="cepm-") in runtime.py:105 — is 71 chars and binds fine: running ProtectedMockRuntime here reports endpoint kind: unix. So the reason printed is false (AF_UNIX is usable) and the transport the run will really use is silently unexercised on the developer platform. Probe in a short tempfile.mkdtemp() dir, and bind _LiveServer's socket there too, so the guard measures the condition production faces.

2. Nothing tests the auto-selection contract itself. create_server(..., transport="auto") (server.py:377-390, coverage-missed 378-385) is never called in-process — _LiveServer at tests/test_protected_mock.py:379 always passes an explicit "tcp"/"unix" — and the only assertion on what auto picked is tests/test_protected_mock.py:486 assert runtime.endpoint_kind in {"unix", "tcp"}, which passes under either outcome. A regression to always-TCP, or a broken except (OSError, ValueError) fallback at server.py:383-385, would fail no test. Add a test asserting create_server(transport="auto") returns a unix: endpoint in a short-path dir, plus one that monkeypatches _UnixMockServer to raise OSError and asserts the returned endpoint starts with tcp:127.0.0.1:.

3. [Axis 3] New SandboxConfig protected-mock fields are untested: passthrough_argv_prefixes validator success path, model-to-server plumbing, and config-merge/-D override coverage (src/coder_eval/models/sandbox.py:461) — Coverage reports models/sandbox.py:455 and :461 as missed — :455 is raise ValueError("protected mock passthrough prefix tokens must be non-empty strings up to 256 chars") and :461 is return normalized, the validator's success return. Both are uncovered because the only two tests that touch the field (tests/test_protected_mock.py:136-144) pass invalid values that raise, and pydantic does not validate the default_factory=list default. The one passthrough behaviour test (tests/test_protected_mock.py:328) hand-writes the server config JSON with "passthrough_argv_prefixes": [["docsai", "ask"]] rather than going through ProtectedMockConfig, so the chain ProtectedMockConfig.passthrough_argv_prefixes (models/sandbox.py:517) -> runtime.py:119 "passthrough_argv_prefixes": mock.passthrough_argv_prefixes -> server.py:186 entry.get("passthrough_argv_prefixes", []) is never exercised end-to-end. A key-name drift on either side would silently disable passthrough with no failing test. Add a test that builds ProtectedMockConfig(tool=..., fixture=..., passthrough_argv_prefixes=[["docsai", "ask"]]), asserts the normalized value round-trips, and drives it through ProtectedMockRuntime so the generated mock-config.json carries the prefixes.

4. [Axis 3] generate_protected_mock_shims is under-tested: stale-shim wipe and Windows .cmd twin content are both unasserted (src/coder_eval/sandbox.py:620) — sandbox.py:618-621 carries the comment "Wipe rather than reuse for the same DIRECT_WRITE reason as record_cli: a reused --run-dir must not keep stale shims pointing at a dead endpoint" followed by shutil.rmtree(shim_dir, ignore_errors=True). The feature it mirrors has two dedicated regression tests for exactly this (tests/test_sandbox_record_cli.py:82 test_reused_target_dir_does_not_carry_a_prior_runs_log and :98 test_stale_shim_for_an_undeclared_tool_is_removed); grep -rln protected_mock tests/ returns only tests/test_protected_mock.py, and none of its shim tests pre-seeds the directory. This is the stale-artifact class the harness treats as a scoring-correctness blocker: a re-run into the same DIRECT_WRITE target with a renamed tool would leave the prior run's shim on PATH pointing at a dead per-run endpoint, so identical agent output scores differently. Mirror test_stale_shim_for_an_undeclared_tool_is_removed: create <sandbox>/protected_mocks/olddtool, call generate_protected_mock_shims, assert the stale file is gone and the declared shim exists. The same applies to the orchestrator's call_log.write_text("", ...) seeding at orchestrator.py:1156 — tests/test_protected_mock.py:753 only asserts .is_file(), not that a pre-existing log is truncated.

5. [Axis 3] Threading server has no concurrency test, and all 15 dispatch assertions run against an unspecced MagicMock instead of a real server (tests/test_protected_mock.py:70) — _TcpMockServer/_UnixMockServer are ThreadingMixIn servers with daemon_threads = True (server.py:281, 290) sharing mutable ToolState.remaining and ToolState.passthrough_cache across handler threads, guarded only by budget_lock/passthrough_lock. Every matching/budget test calls the method unbound on a MagicMock built by _fake_server at tests/test_protected_mock.py:70-75 (fake = MagicMock(); fake.tools = tools; ...) from a single thread, so neither the locking nor the real server's state wiring is exercised. Two consequences: (1) a budget race — N concurrent shim invocations against max_requests=1 — is unverified, and the budget is what bounds a fixture's exposure; (2) the mock has no spec=ProtectedMockServer, so if dispatch later reads another attribute (e.g. self.allowed_peer_uids, declared at server.py:215 and today only read in the handler), MagicMock returns a truthy stub and the tests keep passing on nonsense — the exact shape rubric item 17 warns about. Use MagicMock(spec=ProtectedMockServer) (or instantiate a real _TcpMockServer), and add a test that fires ~20 concurrent client.invoke calls at a _LiveServer with max_requests=5 and asserts exactly 5 zeros and 15 exit-75s.

6. [Axis 4] Generated shim bakes the runtime-dir socket path into the agent workspace; that dir's mock-config.json holds absolute fixture paths, so a same-user agent reaches the grading material in two hops (src/coder_eval/protected_mock/runtime.py:109) — start() puts the fixture manifest in the same directory as the socket that is published into the sandbox:

self._runtime_dir = Path(tempfile.mkdtemp(prefix="cepm-"))   # runtime.py:105
...
config_path = self._runtime_dir / "mock-config.json"          # runtime.py:109
...
    "fixture": str(path),                                     # runtime.py:117 - absolute, resolved

and create_server binds the socket in that same dir (socket_path = runtime_dir / SOCKET_FILE_NAME, server.py:378), so the endpoint string is unix:/<runtime_dir>/mock.sock. That string is then baked verbatim into a file inside the agent's own workspace — I rendered the template to confirm:

SERVICE_ENV = {
    'CODER_EVAL_PROTECTED_MOCK_ENDPOINT': 'unix:/var/folders/ab/cepm-xyz/mock.sock',

Under the only supported driver (driver: tempdir; docker fails closed per models/sandbox.py) the agent runs as the same OS user that created the 0700 mkdtemp, so the walk is: cat protected_mocks/uip → runtime dir → cat /var/folders/ab/cepm-xyz/mock-config.json → absolute fixture path → cat <fixture> → every argv/response pair. This is not the documented limitation: the PR documents the token as non-boundary and claims "fixture bytes stay host-side and are never copied into its workspace", which stays literally true while the shim hands the agent the exact trail to them — defeating the stated motivation ("evaluated agents read them to pass tests they should have failed"). The same shim also leaks the harness checkout via INTERPRETER = '/repo/.venv/bin/python', giving a second route to tasks/**/fixtures/*.json. Fix: split the directories — bind the socket in its own dir containing nothing but mock.sock/endpoint, and keep mock-config.json + token in a separate scratch dir whose path never enters the sandbox (or pass the config to the child on an inherited fd/stdin instead of a file). Consider a test asserting no rendered shim string is a parent of any fixture path. CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:C/C:H/I:N/A:N
10. [Axis 5] generate_protected_mock_shims duplicates ~30-38 lines of _generate_cli_recorders (collision scan, wipe+mkdir, Windows .cmd twin) (src/coder_eval/sandbox.py:594) — generate_protected_mock_shims (sandbox.py:571-656) is a line-for-line copy of _generate_cli_recorders (sandbox.py:480-566) for three blocks: the mock_path_dirs collision scan (594-615 vs 499-522, identical for name in (spec.tool, f"{spec.tool}.cmd", f"{spec.tool}.bat", f"{spec.tool}.exe") probe and message shape), the wipe+mkdir (617-622 vs 524-531), and the Windows .cmd twin (641-652 vs 553-564, byte-identical except the REM text). Sandbox is now 1385 lines (was 1288) with three PATH-shim producers plus mock_path_dirs. The copies have already drifted with contradictory rationale: line 539 interpreter = os.path.realpath(sys.executable) vs line 627 interpreter = sys.executable with the comment 'sys.executable verbatim, NOT realpath: ... resolving it would drop the venv'. Extract one private helper (_write_path_shims(dir_const, specs, render, feature_name)) covering collision-scan → wipe → write → chmod → .cmd twin, and settle the interpreter question once so both generators share the same answer.
11. [Axis 6] Orchestrator assigns self._protected_mock_runtime only after await asyncio.to_thread(runtime.start) (orchestrator.py:1149→1152), so a cancellation at that await orphans the spawned server process and its cepm-* scratch dir (src/coder_eval/orchestrator.py:1149) — orchestrator.py:1148-1152 reads runtime = ProtectedMockRuntime(mocks, task_dir=task_dir) / await asyncio.to_thread(runtime.start) / self._protected_mock_runtime = runtime. asyncio.to_thread does not propagate cancellation into the worker thread: if the orchestrator task is cancelled while that await is pending (Ctrl-C during a batch — asyncio.gather in orchestration/batch.py:214 cancels its children — or any caller-side cancel), the await raises CancelledError, the thread still completes start() and spawns sys.executable -m coder_eval.protected_mock.server, but self._protected_mock_runtime is never assigned, so _cleanup's guard at orchestrator.py:2331 (if self._protected_mock_runtime is not None:) is False and stop() is never called. The orphan runs serve_forever(poll_interval=0.2) (server.py:412) forever with no parent-death detection, holding its cepm-* mkdtemp scratch dir — a subprocess + tempdir leak that accumulates per occurrence. This also diverges from the established pattern two blocks up: orchestrator.py:1028 assigns self.sandbox = Sandbox(...) before await asyncio.to_thread(self.sandbox.setup, direct_target) at orchestrator.py:1068, precisely so cleanup stays reachable across a cancelled await. Fix: assign self._protected_mock_runtime = runtime immediately after construction, before the await (stop() is already safe on a never-started runtime — runtime.py:189/runtime.py:201 both null-guard). The same gap leaves an orphan when the harness itself is SIGKILLed; the repo already has a precedent guard for that shape (the host-heartbeat watchdog in cli/run_task_internal_command.py).
12. [Axis 6] _passthrough caches transient failure responses for the life of the run, permanently poisoning that argv (src/coder_eval/protected_mock/server.py:276) — server.py:271-276 builds the failure response and then caches it unconditionally: except (OSError, subprocess.SubprocessError): response = CommandResponse(70, "", "protected mock: passthrough failed\n") … state.passthrough_cache[key] = response. Every failure mode reaches this line — a subprocess.TimeoutExpired from the 60 s cap, a transient OSError, and the oversize substitution at server.py:275. Failure scenario: the first uip docsai ask "..." hits a network blip or exceeds 60 s, the server caches exit 70, and every subsequent identical invocation for the rest of the run returns the cached failure from server.py:253-255 without ever retrying the real tool — so one transient error becomes a permanent, silent degradation that lowers the task's score. Fix: only populate passthrough_cache on a successful (non-synthesized) result, i.e. move the assignment inside the success branch, or keep a negative-cache entry with a short TTL / retry-once policy.

Nits

1. [Axis 1] transport knob is over-engineered: plumbed through three layers with no non-default caller and typed as a bare str for a closed three-value set (src/coder_eval/protected_mock/runtime.py:83) — transport: str = "auto" on ProtectedMockRuntime.__init__ (runtime.py:83) is forwarded to the subprocess as "--transport", self._transport (runtime.py:141-142) and parsed by parser.add_argument("--transport", default="auto", choices=["auto", "unix", "tcp"]) (server.py:423). Nothing ever passes a non-default value: the single production construction is ProtectedMockRuntime(mocks, task_dir=task_dir) (orchestrator.py:1148), and the transport tests bypass the runtime entirely, calling create_server(tools, token, tmp_path, transport) directly (tests/test_protected_mock.py:379, used at 402/423/438). There is also no YAML or CLI surface for it — TASK_DEFINITION_GUIDE.md documents only the automatic probe. Drop the transport parameter from ProtectedMockRuntime and the --transport argparse option, keeping the argument on create_server where the tests actually use it.
2. [Axis 1] The cli_called JSONL record schema now has two independent writers (src/coder_eval/protected_mock/client.py:35) — _record builds the record inline — entry = {"ts": round(time.time(), 3), "tool": tool, "argv": argv, "exit": exit_code} (client.py:35) — which is key-for-key the record the record_cli shim template emits in invocation_log.py (_TEMPLATE's record(): "ts": round(time.time(), 3), "tool": TOOL, "argv": list(argv), "exit": exit_code). invocation_log.py's module docstring claims it is where "both sides live" for this artifact, and its parse_log is the sole reader; that claim is now false. The record_cli copy is legitimately standalone (its shim runs inside the sandbox where coder_eval is not importable), but this client is launched as python -m coder_eval.protected_mock.client (runtime.py:258) and can import freely. Move the record construction into invocation_log.py (e.g. def build_record(tool, argv, exit_code) -> dict) and call it from _record, so a future field added for cli_called cannot land on only one writer.
3. [Axis 2] Wire request/response messages have no typed definition; key names are re-spelled as string literals independently on both ends (src/coder_eval/protected_mock/protocol.py:16) — protocol.py (the module whose docstring calls itself the shared contract) declares only constants and endpoint helpers — no request/response types. The message shape is therefore spelled out three separate times as raw dict literals/.get() calls: client.py:82 builds {"version": ..., "token": token, "tool": tool, "argv": argv}; server.py:306-312 re-reads those keys via request.get("tool") / request.get("argv") / request.get("token"); server.py:336-341 builds {"version", "exit_code", "stdout", "stderr"} (and again in the oversize fallback at server.py:350-355) which client.py:100-108 re-reads. Both ends validate defensively today, so there is no live bug, but a renamed key drifts silently. Add TypedDicts (MockRequest / MockResponse) in protocol.py and build/read the payloads through them so both ends share one definition.
4. [Axis 4] Server fails open when the token file is absent, leaving the TCP-loopback fallback unauthenticated to any local user (src/coder_eval/protected_mock/server.py:404) — serve() treats a missing token file as "no authentication required" rather than as an error:

token = token_file.read_text(encoding="utf-8").strip() if token_file.is_file() else None   # server.py:404

and the handler's check is conditional on that value being present: if server.token is not None and request.get("token") != server.token: (server.py:317). So a serve() invocation whose token file is missing, unreadable, or empty serves every request unauthenticated. On the AF_UNIX path the 0700 mkdtemp still contains the exposure, but on the TCP fallback (_TcpMockServer.__init__ binds ("127.0.0.1", 0), server.py:286) the socket is reachable by every local UID on the box, and the token is then the only barrier — which is a stronger role than the "same-user hygiene" framing in protocol.py:6-8 accounts for. The comparison is also a plain != rather than hmac.compare_digest. ProtectedMockRuntime.start() always writes the token before spawning, so the shipped path is unaffected today; server.main() is a public python -m entry point, and the fail-open default is one refactor away from mattering. Fix: make the token mandatory in serve() (raise if the file is missing or the stripped value is empty), keep token: str | None only for the in-process test constructor, and compare with hmac.compare_digest. CVSS:3.1/AV:L/AC:H/PR:L/UI:N/S:U/C:L/I:N/A:N
5. [Axis 4] Call-log destination is taken from an environment variable the agent controls, so run-dir invocation records are forgeable and the path is redirectable (src/coder_eval/protected_mock/client.py:32) — _record resolves its destination purely from the environment at invocation time:

raw_path = os.environ.get(CALL_LOG_ENV)   # client.py:32
if not raw_path:
    return
...
with Path(raw_path).open("a", encoding="utf-8", newline="\n") as stream:   # client.py:37

The shim sets CODER_EVAL_MOCK_CALL_LOG for its own child, but the shim also publishes INTERPRETER in cleartext, so the agent can bypass it entirely by running <INTERPRETER> -m coder_eval.protected_mock.client uip ... with its own environment: setting the var to an arbitrary host path turns this into a JSON-line append anywhere the harness user can write, and unsetting it silently suppresses recording while the request still reaches the server. The forged/suppressed entries land in protected_mock_calls.jsonl, which orchestrator.py:1153 places in the run directory next to task.json — the trusted run-record surface consumed downstream. The PR documents the log as diagnostic-only because cli_called resolves log sandbox-relative, but it does not document that the record is agent-writable, which is the property that must hold before it can ever be promoted to a grading input. Fix: have the server write the call log (it already sees every dispatched tool/argv/exit_code at server.py:224-248) instead of the sandbox-side client, so the record is produced by a process outside the agent's control; keep the client-side write only as an optional debug aid. CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:U/C:N/I:L/A:N
6. [Axis 5] Harness's own post-task uip --version probe is routed through the protected mock, consuming the request budget and polluting the call log (src/coder_eval/sandbox.py:470) — resolved_mock_path_dirs now appends the shim dir (sandbox.py:470-473), that list becomes the agent's env_path_prepend (orchestrator.py:1098), and it is echoed back into Sandbox.set_command_base_path from the agent's SDK PATH (orchestrator.py:1239), which is what uip_search_path prepends (sandbox.py:895-898). In run()'s finally block, self._refresh_runtime_tool_versions() (orchestrator.py:608) calls runtime_uip_versions(..., self.sandbox.uip_search_path) → _uip_version → shutil.which("uip", path=search_path) then subprocess.run([uip, "--version"], ...) (utils.py:254-259). With the documented example tool: uip, that probe resolves to the protected-mock shim: it burns one unit of max_requests (documented as 'Per-run request budget for this tool', models/sandbox.py:428) and appends a harness-originated {"tool":"uip","argv":["--version"]} record to protected_mock_calls.jsonl (client.py:35), which reads as an agent invocation to anything that later consumes that log. Either skip protected-mock shim dirs when building uip_search_path, or have the client tag harness-originated calls so the diagnostic log distinguishes them.
7. [Axis 6] Protected mock request handler sets no socket timeout, so a connection that never sends a newline wedges a handler thread and fd for the life of the server (src/coder_eval/protected_mock/server.py:300) — ProtectedMockHandler (server.py:297) subclasses socketserver.StreamRequestHandler without overriding the class attribute timeout, which defaults to None, so setup() never calls settimeout on the connection. server.py:300 (line = self.rfile.readline(MAX_REQUEST_BYTES + 1)) therefore blocks indefinitely on a peer that connects, sends bytes without a \n, and stays alive. Both transports use ThreadingMixIn (server.py:280, server.py:289), so each such peer permanently consumes one thread plus one fd in the server process. This is most reachable on the TCP fallback (server.py:388-390, 127.0.0.1 ephemeral port), where unrelated local processes — port scanners, endpoint-security agents on shared CI hosts — connect without speaking the protocol. daemon_threads = True keeps it from blocking process exit and teardown kills the process, so the blast radius is bounded to one run. Fix: add timeout = CLIENT_TIMEOUT_SECONDS (or a dedicated server-side read deadline) as a class attribute on ProtectedMockHandler, and handle the resulting socket.timeout in handle() the same way as the invalid-request case at server.py:302.
8. [Axis 6] Call-log write failures in the client are reported on stderr, contaminating the tool output stream the agent and criteria read (src/coder_eval/protected_mock/client.py:40) — client.py:39-40 handles a failed diagnostic append with except OSError as exc: sys.stderr.write(f"protected mock client: invocation log failed: {exc!r}\n"). Degrading rather than failing is the right call for a diagnostic-only surface, but the channel is wrong: _record is invoked at client.py:118 after sys.stderr.write(stderr) at client.py:117, so the harness's own diagnostic is appended to the mocked tool's stderr, which the agent reads and which file_contains / run_command criteria may match on. The {exc!r} payload also embeds the host-side call-log path (outside the sandbox) into agent-visible output. Fix: route this to the harness log rather than the shim's stderr — e.g. append the failure to the run-dir log, or suppress it entirely and surface log-write failures host-side.
9. [Axis 7] fixture: path and fixture schema are validated only inside the spawned server, so coder-eval plan reports a broken task as valid (src/coder_eval/models/sandbox.py:421) — fixture: str = Field(...) (models/sandbox.py:421) is only touched at run time: resolve_fixture_path (protected_mock/runtime.py:63 raise RuntimeError(f"protected mock fixture not found: {path}")) runs from Orchestrator._start_protected_mocks, and the fixture JSON is parsed in the child process (runtime.py:132-146 spawns sys.executable -m coder_eval.protected_mock.server). cli/plan_command.py only calls load_task + resolve_task_for_variant + validate_early_stop (lines 105-138), so a typo'd fixture path prints ✓ <task>.yaml and "All tasks are valid!". At run time the operator instead gets the stringified child stderr tail (runtime.py:159-162: "protected mock server exited during startup with code {code}" + "; server stderr (tail): "). Recommendation: add a fixture existence + schema pre-check to the plan surface (e.g. call resolve_fixture_path + the fixture loader from plan_command, alongside validate_early_stop), so a bad fixture fails at plan time with the parser's own message instead of a subprocess traceback mid-batch.
10. [Axis 7] Sandbox.resolved_mock_path_dirs docstring still describes the pre-PR two-entry order and omits the protected-mock dir it now returns (src/coder_eval/sandbox.py:445) — The property now returns three kinds of entry (sandbox.py:466-473 appends PROTECTED_MOCK_DIR between the record_cli dir and mock_path_dirs), but its docstring at sandbox.py:445-446 still reads "The generated record_cli directory comes first when configured, then the entries in SandboxConfig.mock_path_dirs in order;". This is the documented contract of the single input to the agent's PATH (orchestrator.py:1099 env_path_prepend = [str(p) for p in self.sandbox.resolved_mock_path_dirs]). Update it to name the protected-mock shim dir and its position, and to note that it is absent until generate_protected_mock_shims has run.
11. [Axis 7] SandboxConfig.validate_template_sources now also enforces the protected-mock rules; the method name no longer describes what it does (src/coder_eval/models/sandbox.py:562) — @model_validator(mode="after") / def validate_template_sources(self) -> SandboxConfig: with docstring """Validate template sources configuration.""" (models/sandbox.py:561-563) now carries the docker fail-closed gate, the tool-name uniqueness check and the record_cli overlap check (lines 566-578). Rename to something neutral (e.g. validate_sandbox_config) and update the docstring, so a future reader looking for where protected_mocks is validated can find it by name.
12. [Axis 7] New "Protected Fixture-Backed CLIs" section is missing from the guide's Table of Contents (docs/TASK_DEFINITION_GUIDE.md:19) — The guide's TOC lists the sibling sub-section (docs/TASK_DEFINITION_GUIDE.md:19: " - Recording CLI Invocations") under Sandbox Configuration, but the new ### Protected Fixture-Backed CLIs heading added at line 544 has no TOC entry, so the feature is undiscoverable from the top of the page. Add - [Protected Fixture-Backed CLIs](#protected-fixture-backed-clis) after line 19. Note nothing enforces this mechanically: CE028 governs the mkdocs nav:/extra.docs_index page index, not intra-page TOCs, and CE030 (tests/lint/doc_schema_parity.py:44-49) tracks only TaskDefinition, RunLimits, Dataset, SimulationConfig — SandboxConfig is explicitly excluded ("nested models (AgentConfig, SandboxConfig, criteria, …) are NOT walked").

What's Missing

Parallel paths:

• 🟠 🟠 Sandbox._plugin_discovery_path (sandbox.py:926-941) strips only RECORD_CLI_DIR from uip_search_path — added precisely because "a recording shim is not the real CLI" and letting it win the uip lookup silently drops the PLUGIN_TOOLS_DIR pin for every run_command criterion — but the new PROTECTED_MOCK_DIR (added to resolved_mock_path_dirs at sandbox.py:466-473, and to the agent PATH at orchestrator.py:1099) was not added to the same exclusion, so the documented protected_mocks: [{tool: uip}] example reintroduces exactly the bug that exclusion exists to prevent. (trigger: src/coder_eval/sandbox.py) (restates: Axis 5: Harness's own post-task uip --version probe is routed through the protected mock)
• 🟠 🟠 The evaluate-only re-grade path was not updated: Orchestrator._setup returns early when a sandbox is pre-supplied (orchestrator.py:1005-1017, used by cli/evaluate_command.py:105-114) — before await self._start_protected_mocks() at orchestrator.py:1090 — so coder-eval evaluate on a protected_mocks task runs criteria with no fixture service (and, when the work_dir is a preserved prior-run sandbox, with a stale shim baked against a dead endpoint → exit 125), and writes none of the protected_mock_* audit keys into environment_info; nothing errors. (trigger: src/coder_eval/orchestrator.py)
• 🟡 🟡 The recorder-failure protocol of the feature this mirrors was not carried over: invocation_log.py:47,79 writes a <log>.error sentinel when the shim cannot append, and criteria/cli_called.py:229-241 fails the criterion loudly on it so an incomplete log is never scored vacuously; the protected-mock client instead writes to stderr and drops the record (client.py:39-40), so protected_mock_calls.jsonl can be silently incomplete with no sentinel and no test. (trigger: src/coder_eval/protected_mock/client.py) (restates: Axis 6: Call-log write failures in the client are reported on stderr, contaminating the tool output stream)

Downstream consumers:

• 🟡 🟡 The new call log is written in the cli_called JSON Lines schema (client.py:35) into the run dir (orchestrator.py:1153-1156), but the consumer half was not updated: criteria/cli_called.py:219-244 resolves log sandbox-relative via sandbox.file_exists/get_file_content, so no criterion can read the host-side file and protected-mock invocations are ungradable — the PR calls this "diagnostic-only", which documents the gap rather than closing it. (trigger: src/coder_eval/orchestrator.py)
• 🔵 🔵 Artifact preservation was not extended: the generated shim lives inside the sandbox and is copied into the preserved run artifacts (the PR's own test asserts this at tests/test_protected_mock.py:757-759), carrying the per-run token, the runtime-dir socket path, the host interpreter path and the host call-log path into the archived run record — no entry was added to _WORKSPACE_CAPTURE_IGNORE (sandbox.py:44-56, which already scrubs credential stores) or to the preservation copy. (trigger: src/coder_eval/sandbox.py) (restates: Axis 4: Generated shim bakes the runtime-dir socket path into the agent workspace)

Tests:

• 🟡 🟡 No test pins the PATH-prepend order contract for the new third entry: resolved_mock_path_dirs (sandbox.py:440-476) now returns record_cli dir → protected-mock dir → mock_path_dirs, and that list is the sole input to the agent's PATH (orchestrator.py:1099), yet grep -rn resolved_mock_path_dirs tests/ hits only test_sandbox.py / test_sandbox_record_cli.py — tests/test_protected_mock.py never asserts on the property at all, so a reordering or a filtered-out entry would fail nothing. (trigger: src/coder_eval/sandbox.py)
• 🟡 🟡 The feature ships with zero in-repo adoption and no end-to-end exercise: grep -rn protected_mocks tasks/ experiments/ is empty, so the only coverage is unit-level plus one NoOpAgent orchestrator test (tests/test_protected_mock.py:709), and no real task ever drives the shim from an agent's PATH — the recall signal for "agent can't reach the fixture" is untested against a live agent. (trigger: src/coder_eval/models/sandbox.py)

Daily/nightly:

• 🟡 🟡 Nightly blast radius is unstated: SandboxConfig.validate_configuration (models/sandbox.py:566-571) hard-fails protected_mocks under driver: docker, so no containerized/nightly-isolated task can adopt the feature until the UID/GID layer lands; the PR does not say this, and docs/DOCKER_ISOLATION.md gained no unsupported-feature note (its text has no mention of protected_mocks or record_cli). (trigger: src/coder_eval/models/sandbox.py)
• 🔵 🔵 New run-record surface not stated for the cross-repo consumer: protected_mock_calls.jsonl is a new per-task file next to task.json (orchestrator.py:1153) and two new environment_info keys are written (orchestrator.py:1172-1174); docs/REPORT_SCHEMA.md's "Who writes what" table (lines 21-28) was not extended, and the PR doesn't state whether the eval-runner/blob sync picks the new file up. (Verified the env keys are safe — reports.py:486, reports_html.py:1043 and evalboard/lib/runs.ts:415 all treat environment_info as an open key/value map.) (trigger: src/coder_eval/orchestrator.py)

Harness & Lint Improvements

Static checks (lint / type):

• [ce-lint] CE032 — user-authored config files must be parsed through a Pydantic model. Whole-tree registry rule (CE031/CE028 style: a dedicated @pytest.mark.lint test class, not a BaseRule). Registry maps each task-YAML-reachable file surface to its loader (protected_mocks.fixture → protected_mock/server.py::_load_tool, the mock wire config → load_config, dataset JSONL → task_loader._load_jsonl, record_cli.log → invocation_log.parse_log); each registered loader must reach a model_validate* / TypeAdapter(...).validate_* call, and the model must carry ConfigDict(extra="forbid") (reusing the existing yaml_models_forbid_extras predicate). A second AST half keeps the registry honest: any new str field on a config model in models/sandbox.py / models/tasks.py named fixture / *_path / *_file (or whose description says the value is a path to a file the harness reads) must appear in the registry. NOTE on scoping, measured: a blanket "every json.load must be model-validated" rule is NOT adoptable — I counted 24 unvalidated json/yaml.load sites in src/, and most are genuinely untyped third-party artifacts (Codex rollout JSONL, npm ls output, tool version manifests, LiteLLM cost records); the registry scoping is what makes this zero-noise. Prevents: A2-high fixture JSON hand-parsed with bare dict.get and no unknown-key rejection ("exit": 3 / "match-mode": "subset" silently ignored → agent graded as failing; "exit_code": true accepted because isinstance(True, int)), server.py:98-100/135/151. Also A1-medium load_config (C(20)) hand-re-validating a wire config the harness itself generates from already-validated ProtectedMockConfig instances, and A2-low wire request/response keys respelled independently on both ends.
• [ce-lint] CE033 — raw (non-!r) replacement fields in a generated-source template must be paired with a pattern-constrained model field. BaseRule in tests/lint/rules/ce033_shim_template_interpolation.py, wired into ALL_RULES in tests/lint/runner.py. For every module-level string constant whose name matches *TEMPLATE* and that is later .format(...)ed into a generated .py/.cmd artifact, walk the literal with string.Formatter().parse and flag any replacement field whose conversion is not !r unless it is listed in the rule's allowlist together with the sanitizer that makes it safe (a pattern= constrained model field, or a harness-controlled value like sys.executable). Measured: exactly two such templates exist in src/ — invocation_log._TEMPLATE (raw fields interpreter, tool, log_filename) and protected_mock/runtime._SHIM_TEMPLATE (raw fields interpreter, tool); every other field already uses !r. RecordedCli.tool supplies the pattern=r"^[A-Za-z0-9._+-]+$" sanitizer for the first; ProtectedMockConfig.tool supplies none, so runtime.py:231 is the single violation today. The allowlist form also forces a reviewer decision the next time someone interpolates into shim source (a shebang #!{interpreter} legitimately cannot be repr'd). Prevents: A2-high ProtectedMockConfig.tool dropping the pattern allowlist its parallel RecordedCli.tool field carries — tool: 'uip\"\"\"' is accepted at load and renders a shim whose docstring terminates early (verified SyntaxError), and tool: 'uip x' writes a shim PATH can never resolve, silently. Pairs with the same finding's second parity gap (no RECORD_CLI_RESERVED_TOOLS-equivalent check, so a mock may shadow python/sh), which the same registry entry can assert.
• [ce-lint] CE034 — a resource must be stored on self before the first await that starts it. BaseRule in tests/lint/rules/ce034_own_before_await.py, wired into ALL_RULES. Within an async def, flag self.<attr> = <local> when <local> was bound from a constructor call earlier in the same function and an await occurs between the two statements — i.e. the construct → await → own ordering that makes teardown unreachable across a CancelledError. I ran this exact check over all of src/: it produces one hit tree-wide, orchestrator.py:1152 (self._protected_mock_runtime = runtime, ctor at 1148, await asyncio.to_thread(runtime.start) at 1149) — zero false positives, and orchestrator.py:1028 (self.sandbox = Sandbox(...) before its await ... setup at 1068) is the compliant precedent the rule codifies. Prevents: A6-medium: a cancellation at await asyncio.to_thread(runtime.start) leaves the spawned python -m coder_eval.protected_mock.server running serve_forever with no parent-death detection plus its cepm-* mkdtemp dir, because _cleanup's if self._protected_mock_runtime is not None: guard at orchestrator.py:2331 is False.
• [ce-lint] CE035 — dead sentinel guard. Whole-tree @pytest.mark.lint class (CE031 "dead config" generalized from Pydantic fields to instance attributes): flag an attribute whose ONLY assignment package-wide is the literal None (or False) while a conditional tests it with is not None (or truthiness) — the guard's other branch, and everything it protects, is statically unreachable. Annotation-only class declarations (allowed_peer_uids: frozenset[int] | None) must NOT count as writers; that distinction is what keeps the rule quiet. Measured: a naive version produces 6 candidates in src/, of which 5 are killed by the None/is not None shape restriction plus counting class-level AnnAssign initializers as writers, leaving protected_mock/server.py:222 as the only violation. Optionally extend the same whole-tree pass to dead knobs — an optional parameter on an internal class that no in-src/ caller ever passes a non-default value for (ProtectedMockRuntime.__init__(transport="auto"), runtime.py:83) — but measure that half's noise on the existing public-ish constructors before wiring it, since it is likely noisier than the attribute half. Prevents: A1-medium unreachable peer-credential machinery: allowed_peer_uids written only as None, so the guard at server.py:320 never fires and _peer_uid() (325-331, plus the socket/struct imports that exist only for it) is dead, while the docstrings advertise it as a live defense-in-depth layer. The dead-knob half prevents A1/A2-low (transport plumbed through three layers with no non-default caller).
• [ce-lint] CE036 — every # pyright: ignore[...] must carry an inline justification. Simple line-regex whole-tree @pytest.mark.lint class: a suppression comment must be followed by — <reason> (or be preceded by a comment line explaining it). Measured baseline: origin/main carries exactly 5 suppressions in src/, all optional-extra imports (utils.py:494, codex_agent.py:1268, antigravity_agent.py:322-324) — a one-clause reason each, so adoption is a 5-line diff. Prevents: A2-medium: the three bare suppressions at server.py:294/412/414 hide the fact that the Unix transport's constructor arity and both serve() lifecycle calls are entirely unchecked (verified: stripping the comments yields 3 real pyright errors). A required reason forces the author to notice they are suppressing a hierarchy problem rather than an optional-import problem.
• [pyright] Set reportUnnecessaryTypeIgnoreComment = "error" in [tool.pyright] (pyproject.toml, next to the existing reportImplicitStringConcatenation = "error"). Verified pyright 1.1.408 recognizes the setting and it is currently unset (default none in standard mode). Pairs with CE036: CE036 makes suppressions explain themselves, this makes them expire — once the Any base class is replaced by a sys.platform guard, the three server.py ignores become hard errors until deleted rather than lingering as permanent unchecked call sites. Prevents: The regression half of A2-medium (unjustified pyright: ignore suppressions). Note it would NOT have caught the original three (they suppress real errors today), which is exactly why CE036 is the primary gate and this is the follow-through.
• [ce-lint] CE037 — no X: Any = getattr(<module>, "Name", <fallback>) for platform-conditional symbols; use a sys.platform guard. BaseRule in tests/lint/rules/ce037_no_any_getattr_import.py, wired into ALL_RULES: flag a module-level AnnAssign annotated Any whose value is a getattr(...) call on an imported module. Measured: exactly one occurrence in all of src/ (protected_mock/server.py:200) → zero noise. I empirically validated the prescribed fix: replacing it with if sys.platform != "win32": from socketserver import UnixStreamServer, defining _UnixMockServer under that guard, and typing create_server as tuple[socketserver.BaseServer, str] gives 0 pyright errors with all three ignores deleted, and tests/test_protected_mock.py still passes (30 passed, 1 skipped). Also note I checked the pyright-only alternative and it does not exist: reportUntypedBaseClass = "error" does not fire on an Any base (verified by repro), and reportSubclassOfAny/reportExplicitAny are basedpyright-only settings this pyright rejects — so a CE rule is the only reachable gate. Prevents: A2-medium: the Any base erases _UnixMockServer's hierarchy, forcing the three suppressions and leaving create_server's declared return type (ProtectedMockServer) unable to describe the socketserver API its own serve() calls.
• [ce-lint] CE038 — secret comparison must use hmac.compare_digest. BaseRule in tests/lint/rules/ce038_constant_time_secret_compare.py, wired into ALL_RULES: flag == / != where either operand is a Name/Attribute/.get("...") whose identifier or key matches token|secret|password|api_key|apikey|signature|hmac. Cheap, AST-local, and the tree has essentially one such comparison today. Static-check-first note: this is genuinely not reachable by the security tooling already wired up — bandit's credential checks (B105/B106/B107) only detect hardcoded credentials, and the CodeQL security-and-quality pack configured at .github/workflows/codeql.yml:33 did not surface it on this PR — so the CE rule is the gate, not a duplicate. Prevents: A4-low: if server.token is not None and request.get("token") != server.token: (server.py:317) compares the shared secret with a plain !=. Pair the rule with a unit test for the other half of that finding (the fail-open token = ... if token_file.is_file() else None default at server.py:404), which is a policy decision no lint rule can infer.
• [ce-lint] CE039 — blocking socket reads must be bounded by a deadline. BaseRule in tests/lint/rules/ce039_socket_read_deadline.py, wired into ALL_RULES: a class deriving from socketserver.*RequestHandler must declare a class-level timeout, and any socket.socket(...) / accept()-derived connection must have settimeout(...) called before a read. socketserver defaults timeout = None, so the omission is invisible at the call site — exactly the shape a lint rule is for. One handler class in the tree today, so adoption is one attribute. Prevents: A6-low: ProtectedMockHandler (server.py:297) never sets timeout, so self.rfile.readline(...) at server.py:300 blocks forever on a peer that connects and never sends a newline, permanently consuming one thread + one fd per such peer under ThreadingMixIn — most reachable on the TCP-loopback fallback where unrelated local processes connect.
• [ce-lint] CE041 — a Mock passed as self to an unbound method call must be built with spec=/spec_set=. Whole-tree @pytest.mark.lint class over tests/: when a call has the shape SomeClass.some_method(x, ...) (unbound, first arg is the receiver) and x traces back to a MagicMock()/Mock()/AsyncMock() built without spec, flag it. Implementation must follow one level of helper/fixture indirection (the protected_mock case builds its mock inside a _fake_server() helper and receives it as a fixture param). Measured with the intraprocedural half only: 8 existing sites, all in tests/test_token_usage.py — a bounded, mechanical cleanup, not a suite-wide churn. This is the shape rubric item 17 already asks reviewers to catch by hand. Prevents: A3-medium: all 15 dispatch/budget/passthrough assertions run ProtectedMockServer.dispatch(fake, ...) against an unspecced MagicMock (tests/test_protected_mock.py:70-75), so any new self.<attr> read inside dispatch (e.g. allowed_peer_uids) returns a truthy stub and the tests keep passing on nonsense — evidenced in-file by the mock having to be hand-rewired around a real method at tests/test_protected_mock.py:350-353.
• [ce-lint] CE040 — hand-maintained intra-page TOCs must match their headings. Doc-surface @pytest.mark.lint class (CE028's sibling, in the same family as tests/lint/doc_schema_parity.py): for each docs/*.md that opens with a bulleted TOC, assert every ##/### heading below it has a matching TOC link and vice-versa, with an EXEMPT escape for deliberately-unlisted headings. Cheap Markdown parse, no AST. Prevents: A7-low: the new ### Protected Fixture-Backed CLIs section (docs/TASK_DEFINITION_GUIDE.md:544) has no TOC entry, so the feature is undiscoverable from the top of the page. As the finding notes, nothing mechanical covers this today — CE028 governs the mkdocs nav:/extra.docs_index page index, and CE030 tracks only TaskDefinition/RunLimits/Dataset/SimulationConfig (SandboxConfig is explicitly out of scope).

Harness improvements (not statically reachable):

• Promote the pre-commit pyright hook from stages: [manual] to pre-push (.pre-commit-config.yaml:37-45, entry: uv run pyright, pass_filenames: false), so the type gate runs before a PR exists rather than only when someone remembers pre-commit run --hook-stage manual pyright. Why not static: The static check already exists and CI already enforces it (pr-checks.yml:88 runs .venv/bin/pyright) — the gap is purely when it executes locally. I verified no ruff configuration can substitute: ISC001,ISC002 report 0 findings on this tree, and enabling ISC002 with flake8-implicit-str-concat.allow-multiline=false yields 518 pre-existing violations, because pyright only flags implicit concatenation when it is mixed with an explicit + (confirmed by minimal repro) — a distinction ruff has no rule for. Prevents: A2-medium: runtime.py:170 shipped a reportImplicitStringConcatenation error that breaks make typecheck/make verify for everyone on main, alongside a PR description claiming "pyright: clean".
• Add a changed-files coverage floor to make verify / pr-checks.yml (e.g. diff-cover against the base ref, or a per-new-package --cov-fail-under), so a new package cannot land far below the repo's 80% while the global number stays green. Pair it with a convention for new wire/subprocess modules: a required parametrized "malformed input" table that drives the real handler over a real socket (oversized line, no trailing newline then close, non-JSON bytes, wrong version, non-string argv) asserting the error exit code AND that the server survives to answer a subsequent valid request. Why not static: Coverage is a runtime measurement, and "does the error branch behave correctly and leave the server usable" needs a live socket and a second request — no AST rule can assert it. Prevents: A3-high: protected_mock/server.py at 72.63% with the entire non-shim-client error surface uncovered (16 raise ValueError config/fixture sites with only 2 tested; request-envelope guards at 302/307/311/313/315; the oversize-response fallback at 348), and client.py at 82.61%. Also A3-medium (models/sandbox.py:455/:461 — the passthrough-prefix validator's failure and success returns are both unexecuted).
• Add a timeout-nesting invariant test asserting every client-side deadline exceeds the server-side worst case it waits on — concretely CLIENT_TIMEOUT_SECONDS >= PASSTHROUGH_TIMEOUT_SECONDS + slack — plus a live-server test with a passthrough that sleeps past the client deadline, asserting the shim does not return 125 while the server is still working. Why not static: Which constant bounds which is a call-graph/semantic relation between two modules (protocol.py:19 vs server.py:59); a lint rule cannot know that the client's socket timeout brackets the server's subprocess.run budget. Prevents: A8-high: the 5s client socket timeout is 12x shorter than the 60s server passthrough budget, so a slow uip docsai ask returns a spurious exit 125 (reproduced live at 5.05s) while the same argv returns exit 0 instantly on the immediate retry from passthrough_cache.
• Add a scoring-determinism guard for mock-backed tasks: assert the same argv invoked twice yields the same exit code and the same criterion score irrespective of latency or of a prior transient failure — i.e. no negative caching, and no request-budget unit charged for a response that was never delivered. Why not static: Requires runtime timing and cross-invocation server state; determinism is a property of an execution, not of the source. Prevents: A8-high (identical agent behaviour scoring differently based on whether the live tool crossed the 5s line, plus the budget charged for the undelivered call) and A6-medium (_passthrough caching a TimeoutExpired/OSError/oversize failure at server.py:276 for the life of the run, so one transient blip permanently degrades every later identical invocation — note the success path caches transient non-zero exits too, since subprocess.run(check=False) does not raise).
• Add a concurrency test for shared per-run state: fire ~20 parallel client.invoke calls at a _LiveServer configured with max_requests=5 and assert exactly 5 zeros and 15 exit-75s (and, once passthrough lock scope is fixed, that two concurrent passthroughs both complete). Why not static: Requires real threads racing on ToolState.remaining under budget_lock in a ThreadingMixIn server. Prevents: A3-medium: the budget decrement — the thing that bounds a fixture's exposure — is only ever exercised single-threaded through an unspecced mock; the three real-server tests each issue one sequential request.
• Add skip-guard hygiene to CI: run pytest with -rs and fail the Linux quality-gate job when a test in a declared platform-gated allowlist skips, and require that platform probes bind/exercise the production path (tempfile.mkdtemp(prefix=...)) rather than a pytest tmp_path. Why not static: Whether a skip guard measures the condition production actually faces is a runtime property of the host (here: a 122-char pytest tmp_path socket exceeds the AF_UNIX limit while the 72-char production mkdtemp binds fine) — no static rule can see path lengths. Prevents: A3-medium: test_unix_socket_end_to_end skips on macOS with the false reason "AF_UNIX stream sockets not usable on this platform", so the transport production actually selects is unexercised on the developer platform; combined with _LiveServer always passing an explicit transport, create_server(transport="auto") and its except (OSError, ValueError) TCP fallback (server.py:375-390) are uncovered on every platform.
• Make coder-eval plan the single resolution-time validation seam: have plan_command call resolve_fixture_path + the fixture loader alongside validate_early_stop, and add a test asserting every sandbox sub-config that references a file on disk is validated at plan time. Why not static: Requires resolving task-relative paths against the filesystem and parsing the referenced file — inherently runtime. Prevents: A7-low: a typo'd fixture: path prints ✓ <task>.yaml / "All tasks are valid!" at plan time and only fails mid-batch as a stringified child-process stderr tail (runtime.py:159-162).
• Parametrize the existing stale-artifact regression tests over every PATH-shim generator rather than just record_cli: pre-seed <sandbox>/protected_mocks/oldtool, run generate_protected_mock_shims, assert the stale file is gone and the declared shim (+ its .cmd twin, with asserted content) exists; likewise assert the orchestrator truncates a pre-existing protected_mock_calls.jsonl. Why not static: Needs a reused DIRECT_WRITE target directory populated by a prior run — a filesystem-state invariant, not a source pattern. (The generator-level duplication that motivates this is real but not lint-detectable; a shared _write_path_shims helper plus a shared parametrized test is the durable fix, keeping the interpreter a per-caller argument since record_cli's realpath and the mock shim's venv-preserving sys.executable are a deliberate, necessary divergence.) Prevents: A3-medium: shutil.rmtree(shim_dir, ...) at sandbox.py:621 is reported MISSED by coverage and no test pre-seeds the directory, so a re-run into the same --run-dir with a renamed tool could leave a stale shim on PATH pointing at a dead per-run endpoint — the scoring-correctness class the mirrored record_cli tests exist to protect.
• Add a sandbox host-path-leak assertion covering all generated in-sandbox artifacts: render each one and assert it names no host path outside the sandbox other than an explicitly allowlisted set, and that no directory it does name is a parent of any fixture/grading file. Why not static: Requires rendering the templates with real runtime values and resolving the resulting strings against a live sandbox + run-dir layout; the leak only exists in the rendered output, not in the source literal. Prevents: A4-medium (CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:C/C:H/I:N/A:N): the shim bakes unix:/<runtime_dir>/mock.sock into the agent's workspace, and that same runtime dir holds mock-config.json with absolute, resolved fixture paths — a two-hop walk to the grading material under the only supported driver, plus the INTERPRETER (harness checkout) and absolute call-log paths as secondary routes.
• Add a teardown-under-cancellation harness test: cancel Orchestrator.run() at each await boundary in the setup path and assert no child processes and no scratch directories survive. Separately, treat "grading-relevant run-dir artifacts must be produced by a process outside the agent's control" as a documented harness invariant with a test — the protected-mock server already sees every dispatched tool/argv/exit_code and is the natural writer for the call log. Why not static: Process and tempdir survival is observable only at runtime; and whether a record is agent-forgeable depends on which process writes it and what environment that process trusts (CALL_LOG_ENV read from the agent-controlled environment at client.py:32), which no AST rule can determine. CE034 catches the specific ordering bug — this catches the semantically equivalent shapes it misses. Prevents: A6-medium (orphaned server + cepm-* dir on a cancelled start) and A4-low (call-log destination taken from an agent-controllable env var, making protected_mock_calls.jsonl — a run-directory artifact sitting next to task.json — forgeable, suppressible, and redirectable before it could ever be promoted to a grading input).

Top 5 Priority Actions

1. Fix the harness-nondeterminism at src/coder_eval/protected_mock/protocol.py:19 — CLIENT_TIMEOUT_SECONDS = 5.0 bounds recv, not just connect, while the server allows 60s under passthrough_lock, so a uip docsai ask crossing 5s returns exit 125 while an immediate identical retry returns exit 0 from cache (reproduced live: 125 @5.05s, then 0 @0.06s); derive the client deadline from PASSTHROUGH_TIMEOUT_SECONDS, hold the lock only around the cache, and refund the budget decremented at server.py:228-231 when no response is delivered.
2. Replace the hand-rolled fixture parsing at src/coder_eval/protected_mock/server.py:98-100/135/151 with Pydantic models using extra="forbid", match_mode: Literal[...] and exit_code: int = Field(ge=0, le=255) — today a typo'd "exit" (the key this feature's own call log uses) or "match-mode" is silently dropped, so a correctly-behaving agent falls through to the exit-2 default and is graded as failing with nothing raised.
3. Stop caching failure responses at src/coder_eval/protected_mock/server.py:276 — one transient OSError/60s timeout (and, since subprocess.run(check=False) never raises, any transient nonzero exit from the real tool) is cached for the whole run, so a single network blip permanently degrades every subsequent identical invocation and lowers the task score; cache only on a defined success predicate, or use a short-TTL/retry-once negative cache.
4. Split the protected-mock scratch directory at src/coder_eval/protected_mock/runtime.py:105-117 so the socket the shim publishes into the sandbox no longer sits next to mock-config.json — under the only supported driver the same-user agent walks protected_mocks/<tool> → runtime dir → absolute fixture paths → the grading material in two hops, which defeats the feature's stated motivation even though the documented "fixtures are never copied into the workspace" claim stays literally true.
5. Unblock the type gate and then close the error surface: fix the implicit string concatenation at src/coder_eval/protected_mock/runtime.py:170 (a real, reproducible reportImplicitStringConcatenation error that fails make typecheck/make verify and contradicts the PR's "pyright: clean" claim, per pyproject.toml:258), and add the missing unhappy-path tests behind server.py's 72.63% coverage — malformed wire requests, the 16 uncovered fixture ValueError sites, passthrough failures, and the stale-shim wipe at sandbox.py:621 that is the record_cli-equivalent scoring-correctness regression guard.

---

Stats: 0 🔴 · 4 🟠 · 12 🟡 · 12 🔵 across 8 axes reviewed.