feat(harbor): unblock trajectory-dependent criteria, always allow credentials at export

[akshaylive]

Summary

• Flatten the run_dir layout for --workspace-dir (Harbor single-task) runs so task.json/task.html/task.log/artifacts live directly under run_dir, next to trajectory.json (already written as task.json's sibling by emit_trajectories_for_run) — matching CoderEvalAgent's own run-directory shape.
• Unblock NEEDS_TRAJECTORY criteria (command_executed/commands_efficiency/skill_triggered) at export time: the verifier's tests/test.sh now grades via coder-eval evaluate /tests/task.yaml /logs/agent --run-dir /logs/verifier, hydrating the agent phase's native task.json trajectory directly (no ATIF round-trip), and without touching the untrusted-recorded-config gate since an explicit task file is passed.
• Remove --allow-credentials entirely: NEEDS_CREDENTIALS criteria (llm_judge/agent_judge/uipath_eval) now always export unconditionally, on the same assumption the flag previously encoded — the operator provisions model access inside the verifier container themselves.
• Extend the Harbor E2E CI script (.github/scripts/harbor_e2e.py) with a trajectory_criteria scenario (new fixture tests/harbor_e2e/fixtures/trajectory_criteria.yaml) that checks the trajectory-dependent criterion's own score directly, not just aggregate reward — so a silently ungraded trajectory can't hide behind an unrelated criterion passing.

Test plan

• Full coder_eval test suite passes (6423 passed, 8 skipped) and ruff check/ruff format clean.
• Verified end-to-end against real Docker: rebuilt coder-eval-agent:latest from this branch's source and re-ran 6 real skills-repo tasks exported via coder-eval export -e experiments/smoke.yaml through hb run. All 6 previously partial/zero rewards (caused by a stale pre-fix image, not these code changes) now score correctly — 5 hit 1.0, the 6th genuinely scores 0.5 (agent behavior gap, not a grading bug).
• Confirmed the fix directly: before, skill_triggered/llm_judge criteria saw an empty trajectory (observed='no', "no turn records available") despite the real trajectory recording the skill invocation and full agent output; after rebuilding the image with this branch's code, the same task scores 1.0.
• harbor-e2e.yml nightly/on-demand CI run (not triggered from this session).

🤖 Generated with Claude Code

[uipreliga]

Review: coder_eval — pr:186 (10 files) axis:1,2,3,4,5,6,7,8 — "feat(harbor): unblock trajectory-dependent criteria, always allow credentials at export" by @akshaylive

Scope: pr:186 (10 files) axis:1,2,3,4,5,6,7,8 — "feat(harbor): unblock trajectory-dependent criteria, always allow credentials at export" by @akshaylive · branch akshaya/update_harbor_conversion · c98929e · 2026-09-17T14:52Z · workflow variant

Change class: complex — changes the Harbor verifier grading path (test.sh now grades /logs/agent as a run dir), un-blocks two criterion portability classes, removes a public CLI flag, and changes run_batch run-dir layout in --workspace-dir mode

The core harness is in good shape (9.2/10: clean architecture, strict type safety, almost no security findings), but the new Harbor export path can give a wrong reward for the same agent output: it grades a filtered copy of the workspace, invents a score when the run record is missing, and silently scores judges 0 when credentials are missing, and pytest covers none of this; fix the Harbor verifier contract before anyone relies on exported rewards.

Summary

Axis	Score	🔴	🟠	🟡	🔵	Top Issue
1. Code Quality & Style	9.7 / 10	0	0	0	3	Generated test.sh carries a 15-line comment that records history
2. Type Safety	9.8 / 10	0	0	0	2	harbor_e2e trajectory-criterion check hardcodes the type set, reads an untyped dict, and passes vacuously
3. Test Health	8.4 / 10	0	1	1	1	Flat --workspace-dir run_dir layout is chosen only in run_batch; --resume and finished-check paths still read the nested run_dir, and no test covers it
4. Security	9.9 / 10	0	0	0	1	Harbor verifier now trusts an agent-writable /logs/agent/task.json as the grading evidence for trajectory criteria, and the test.sh comment says this path is safe
5. Architecture & Design	9.8 / 10	0	0	0	2	Verifier phase now rewrites the agent phase's /logs/agent records
6. Error Handling & Resilience	8 / 10	0	2	0	0	test.sh grades the agent log directory and writes a fabricated reward when /logs/agent has no task.json
7. API Surface & Maintainability	8.9 / 10	0	0	2	1	portability.py and test docstrings describe a trajectory.json / --format harbor (ATIF) grading path that the generated test.sh does not use
8. Evaluation Harness Quality	9 / 10	0	1	0	0	Harbor verifier now grades a filtered COPY of the workspace, so criteria that need node_modules/.venv/.cache/.config/.local get a different score for the same agent output

Overall Score: 9.2 / 10 · Weakest Axis: Error Handling & Resilience at 8 / 10
Totals: 🔴 0 · 🟠 4 · 🟡 3 · 🔵 10 across 8 axes.

Blockers

1. [Axis 3] Flat --workspace-dir run_dir layout is chosen only in run_batch; --resume and finished-check paths still read the nested run_dir, and no test covers it (src/coder_eval/orchestration/batch.py:159) — The new branch effective_run_dir = config.run_dir if config.workspace_dir is not None else rt.run_dir (line 159), passed as run_dir=effective_run_dir (line 199), changes where task.json, task.log and artifacts/ land. That location is now the contract the exported test.sh relies on (/logs/agent/task.json). No pytest checks it. A grep of tests/ for workspace_dir finds only Orchestrator-level capture tests (test_orchestrator.py:2433, test_cleanup_preservation_guard.py:153), the docker-refusal CLI error (test_detached_grading_guards.py:417) and a string check on the agent command (test_harbor_agent.py:89). None of them calls run_batch or execute --workspace-dir and then checks the layout. test_execute_format_harbor.py uses run_dir.glob("**/task.json"), so it passes for both the flat and the nested layout. Coverage reports this line as hit, but no assertion depends on its result. A missing test here also hides a parallel-path divergence: _load_completed_result (line 444, report_path = rt.run_dir / TASK_JSON_FILENAME) and clear_rerun_artifacts (line 435, artifacts = rt.run_dir / "artifacts" / rt.task.task_id) still use the NESTED path. Nothing rejects --resume together with --workspace-dir, so a resumed run never finds the finalized flat task.json, runs the agent again, and does not clear stale flat artifacts. Add a test that calls run_batch (or execute <task> --run-dir R --workspace-dir W) with one tempdir task. It must assert that R/task.json exists, that R/<variant>/<task_id>/00/ does not exist, and that --format harbor writes R/trajectory.json. Also add a test for the resume path, or make it an error to pass --workspace-dir with --resume.
2. [Axis 6] test.sh grades the agent log directory and writes a fabricated reward when /logs/agent has no task.json (src/coder_eval/harbor/packager.py:79) — Offending line: coder-eval evaluate /tests/task.yaml /logs/agent --in-place --run-dir /logs/verifier || true. resolve_evaluate_target (cli/evaluate_target.py) selects RUN_DIR only when /logs/agent/task.json exists. When it is missing, the mode silently becomes WORK_DIR. The same happens after as_work_dir when the file does not parse (evaluate_command.py logs only a warning: "grading it as a plain work directory"). The task.json goes missing in two cases. First, Harbor's agent timeout kills coder-eval execute before finalize. Second, execute fails before the Orchestrator runs (batch.py _create_error_task_result rows are never persisted). Harbor still runs the verifier after both: harbor/trial/single_step.py:84 is except (AgentTimeoutError, NonZeroAgentExitCodeError) as exc: self._record_exception(exc), and verification follows. The verifier then runs every criterion with --in-place against Harbor's log directory, not a workspace. File criteria score 0, and run_command criteria execute inside the log dir and can change it. The row finalizes as FAILURE with a real weighted_score, and coder-eval harbor reward writes reward.json. This breaks reward.py's documented contract ("An unmeasured row must not become reward=0.0"). The old $(pwd) form graded the agent's actual partial workspace. Fix: fail loud in test.sh, for example [ -f /logs/agent/task.json ] || { echo 'agent phase produced no task.json' >&2; exit 0; } before evaluate, so no reward file is written and Harbor records the missing reward. Alternatively, give evaluate a way to require RUN_DIR mode, so that it refuses the WORK_DIR fallback for this call. Add a packager/golden test for the missing-task.json case. Candidate lint/test: assert the emitted test.sh guards on the run record.
3. [Axis 6] Judge criteria now export with no warning that the verifier needs model credentials and network access (src/coder_eval/harbor/portability.py:73) — Offending code: _BLOCKING_IN_V1 = frozenset({CriterionPortability.NEEDS_CLI_RECORDER,}), which drops NEEDS_CREDENTIALS and the --allow-credentials opt-in. This removes the only loud failure for a required dependency. In task.toml, verifier env forwarding uses ${NAME:-} (packager.py _env_template_dict), so an unset key becomes an empty string. In that case llm_judge short-circuits (criteria/llm_judge.py: if route is None or (isinstance(route, DirectRoute) and route.judge_transport is None): ... return JudgeCriterionResult(score=0.0, details="(judge transport unconfigured)", error=...)). The checker records that as an ordinary failed criterion, not an ERROR status. The row finalizes FAILURE, and reward.py writes a numeric reward (0.0, or a partial one), not the "write nothing" infrastructure path. This is the failure mode that .claude/notes/reporting.md § 'Not every criterion can grade inside someone else's container' says the audit exists to prevent: "refuse ... AT EXPORT TIME ... rather than at verify time, where it is an unexplained low reward with no obvious cause". That note still describes the escape hatch, so it is now stale. Fix: keep exporting, but add a non-blocking ExportResult.warnings entry for each NEEDS_CREDENTIALS criterion that names the env vars the verifier must receive. Better: make a judge criterion with no usable transport finalize the row as ERROR, so reward.py skips the reward instead of scoring 0. Update the reporting.md rationale in the same change.
4. [Axis 8] Harbor verifier now grades a filtered COPY of the workspace, so criteria that need node_modules/.venv/.cache/.config/.local get a different score for the same agent output (src/coder_eval/harbor/packager.py:79) — test.sh changed from coder-eval evaluate /tests/task.yaml "$(pwd)" --in-place ... (it graded the live WORKDIR in place) to coder-eval evaluate /tests/task.yaml /logs/agent --in-place --run-dir /logs/verifier || true. Because /logs/agent now holds a flat task.json, evaluate_target.py picks RUN_DIR mode. regrade.default_workspace then grades prior.sandbox_path. In --workspace-dir mode, Orchestrator._cleanup sets that to the output of self.sandbox.capture_to(artifacts_dir) (orchestrator.py:3032-3034). capture_to runs shutil.copytree(..., ignore=shutil.ignore_patterns(*_WORKSPACE_CAPTURE_IGNORE)) (sandbox.py:1521-1527), and that ignore list drops ".venv", "node_modules", ".npm-prefix", ".cache", ".config", ".local" (sandbox.py:53-60). So a task where the agent runs npm install and a run_command criterion runs npm test/npx jest, or a criterion that calls .venv/bin/python or uses a uv/pip cache, passed under the old test.sh and now fails. The agent output is the same. regrade_in_place's own docstring names this defect: "the template-copy path filters out node_modules / dist / build / .venv, which would make a criterion reading those fail as a copying artifact rather than as a verdict" (regrade.py:917-920). This is the Axis 8 scoring-correctness class: the score changes for identical agent output. Fix: keep the run directory as the source of the trajectory, but grade the live workspace. In test.sh, pass --workspace "$(pwd)", which is an existing run-dir-mode option ("e.g. a verifier's /app"). This needs a change to default_workspace's containment rule or an explicit --workspace (the explicit flag already bypasses _contained). Add a round-trip unit test: execute --workspace-dir W --run-dir R, then evaluate task.yaml R --in-place, with a criterion that reads node_modules/ in W. Today only the GitHub e2e workflow covers this path, and no scenario uses node_modules. Nightly impact: none. This path is Harbor-export only; the DockerRunner and nightly run path never set workspace_dir. Previously exported task directories that are regenerated pick up the new behavior.

Non-blocking, but please consider before merge

1. [Axis 3] The verifier grading contract in test.sh is checked only as a string; the round trip with a trajectory criterion runs only in the nightly e2e job, not on PRs (src/coder_eval/harbor/packager.py:79) — The PR changes the verifier to coder-eval evaluate /tests/task.yaml /logs/agent --in-place --run-dir /logs/verifier || true (packager.py:79, golden test.sh:31). It also makes NEEDS_TRAJECTORY non-blocking because of that change. The only in-pytest checks are string matches (test_harbor_packager.py assert "coder-eval evaluate /tests/task.yaml /logs/agent --in-place --run-dir /logs/verifier" in content) and audit_criteria([criterion]) == []. No test grades an exported task: execute --workspace-dir into a flat run dir, then evaluate <task.yaml> <that run dir> --in-place --run-dir <other>, where command_executed or skill_triggered is hydrated from task.json and the workspace is found through the recorded sandbox_path. Only .github/scripts/harbor_e2e.py covers this, and .github/workflows/harbor-e2e.yml says it is 'Deliberately NOT triggered on pull_request'. If the flat layout, the run-dir detection or the task-file override breaks, command_executed silently scores 0.0 on every exported Harbor task, and || true hides the failure. Add a pytest integration test with a mock agent (like test_execute_evaluate_loop.py): run execute with --workspace-dir and --run-dir A, then evaluate task.yaml A --in-place --run-dir B. Assert that B/task.json has a command_executed result with score 1.0. Also add a negative case where the trajectory is emptied and the criterion scores 0.
2. [Axis 7] portability.py and test docstrings describe a trajectory.json / --format harbor (ATIF) grading path that the generated test.sh does not use (src/coder_eval/harbor/portability.py:13) — The module docstring (lines 13-15) says: "skill_triggered; never actually blocking, since the packager's CoderEvalAgent always runs coder-eval execute --format harbor and the generated tests/test.sh always grades against the resulting /logs/agent/trajectory.json (ATIF)." The _BLOCKING_IN_V1 comment (lines 69-70) repeats it: "NEEDS_TRAJECTORY, which C2's CoderEvalAgent + generated test.sh always wire up via /logs/agent/trajectory.json". The same PR's test.sh does the opposite. packager.py:71 says "no ATIF trajectory.json round-trip either (task.json already carries the same trajectory natively)", and packager.py:79 runs coder-eval evaluate /tests/task.yaml /logs/agent --in-place --run-dir /logs/verifier. It has no --format harbor or --trajectory. The tests repeat the wrong claim: tests/test_harbor_portability.py:97 ("/logs/agent/trajectory.json into the verifier's coder-eval evaluate --format harbor call") and tests/test_harbor_packager.py:73 ("test.sh always wires /logs/agent/trajectory.json"). This is the public contract for why trajectory criteria are safe to export, and it points readers at the wrong grading path. Rewrite all four places to name the real path: the verifier grades /logs/agent as a run directory from its task.json. Keep one statement of that mechanism and point the others at it. n/a
3. [Axis 7] --workspace-dir now also flattens the run-directory layout (task.json beside run.json), and neither run-layout.md nor the flag help text documents this (src/coder_eval/orchestration/batch.py:159) — effective_run_dir = config.run_dir if config.workspace_dir is not None else rt.run_dir writes task.json, task.log and artifacts/ at the run root, next to run.json. A flag named for where the agent's workspace lives now also changes the on-disk result layout. .claude/shared/run-layout.md (mirrored in plugins/coder-eval/reference/run-layout.md) still says the layout is runs/<run_id>/<variant_id>/<task_id>/<NN>/{task.json, task.log, artifacts/}. Its scope markers (run.json at the root means run scope, task.json directly in the path means task scope) now conflict on this directory. The --workspace-dir help text in execute_command.py:190-196 and run_command.py does not mention the layout change, and it still says the workspace is where "Harbor's verifier phase looks for them", which is no longer true now that test.sh grades /logs/agent. Make the flat layout explicit. Either add a documented entry for it to run-layout.md and the flag help text, or keep the standard nesting and have CoderEvalAgent read trajectory.json from the one known nested path. Also check that --resume and report --rebuild handle a flat directory where run.json and task.json share the root. n/a

Nits

1. [Axis 1] Generated test.sh carries a 15-line comment that records history (src/coder_eval/harbor/packager.py:64-78) — _TEST_SH_TEMPLATE now contains a 15-line shell comment that begins "# /tests/task.yaml /logs/agent -- an explicit task file over a RUN DIRECTORY,". Most of it is design history and rationale ("no $(pwd) guess of the agent's WORKDIR needed", "no ATIF trajectory.json round-trip either", the untrusted-recorded-config argument). The comment is inside a string literal, so make docs-budget does not see it, although it is over the 8-line comment-run cap in spirit. The same text is copied into tests/_fixtures/harbor_export_golden/expected/tests/test.sh:16-30 and ships in every exported task. Per CLAUDE.md ("A docstring states the contract, not the history"), reduce it to a 1-2 line contract, for example "# Grade /logs/agent (CoderEvalAgent's run dir) with the exported task.yaml.". Put the rationale in .claude/notes/reporting.md § Harbor export.
2. [Axis 1] Classification rationale restated three times in portability.py; _BLOCKING_IN_V1 comment is a hard-to-parse run-on sentence (src/coder_eval/harbor/portability.py:67-77) — The same statement ("NEEDS_REFERENCE / NEEDS_TRAJECTORY / NEEDS_CREDENTIALS are not blocking, and here is why") is in the module docstring (lines 10-21), in the comment above _BLOCKING_IN_V1 (lines 67-72), and in the audit_criteria docstring (lines 108-114). The comment is one nested sentence with em-dashes: "(vs. tolerating with a caveat, like NEEDS_REFERENCE — C2 always emits tests/reference/ when task.reference is set — NEEDS_TRAJECTORY, which C2's CoderEvalAgent + ... — none of these three classes is actually blocking)". Its NEEDS_TRAJECTORY clause is also factually stale (see the contradictory-contract finding). Keep the per-class explanation in the module docstring only. Replace the comment with one line, or with none: _BLOCKING_IN_V1 = frozenset({CriterionPortability.NEEDS_CLI_RECORDER}) explains itself. Reduce the audit_criteria docstring to its contract.
3. [Axis 1] Stale Scenario docstring after allow_credentials removal; rationale comment duplicated within the same script (.github/scripts/harbor_e2e.py:35) — The PR removed the only export flag (allow_credentials) from Scenario, but the docstring at line 35 still says """One (task.yaml, export flags) pair to round-trip through Harbor.""". Change it to "One named task.yaml to round-trip through Harbor.". Also, the new scenario comment at lines 54-58 ("reward could still land on 1.0 'by luck' from unrelated criteria while this one silently scores 0.0") says almost the same thing as the comment at lines 140-143 ("it could pass 'by luck' from unrelated criteria while this one silently scored 0.0"). Keep one of the two. The comment at the check site is the better place.
4. [Axis 2] harbor_e2e trajectory-criterion check hardcodes the type set, reads an untyped dict, and passes vacuously (.github/scripts/harbor_e2e.py:144) — The script reads the persisted record without a schema: verifier_result = json.loads(verifier_task_json.read_text(encoding="utf-8")) (line 144), then uses for r in verifier_result.get("success_criteria_results", []) if r.get("criterion_type") in _TRAJECTORY_CRITERION_TYPES (lines 146-149). The types are a hand-written string set: _TRAJECTORY_CRITERION_TYPES = frozenset({"command_executed", "commands_efficiency", "skill_triggered"}) (line 66). Nothing is checked against a type, so a renamed key or a missing success_criteria_results gives an empty trajectory_results. The trajectory_criteria scenario then passes without grading a trajectory criterion. The set can also drift from portability._PORTABILITY_BY_TYPE. The script already needs coder_eval installed. Load the record with EvaluationResult.model_validate_json(...), or at least check that success_criteria_results is a list. Derive the set from {t for t, p in _PORTABILITY_BY_TYPE.items() if p is CriterionPortability.NEEDS_TRAJECTORY}. Fail when a scenario that declares a trajectory criterion yields zero matches. This is a cold CI script, so the severity is Low.
5. [Axis 2] Parametrized criterion params are typed object and use unjustified # type: ignore[list-item], but SuccessCriterion is already imported (tests/test_harbor_portability.py:95) — The PR rewrote def test_trajectory_criteria_do_not_block_export(criterion: object) -> None: (line 95) with assert audit_criteria([criterion]) == [] # type: ignore[list-item] (line 98). It did the same in test_credentials_criteria_never_block_export(criterion: object) (lines 117/120). In the same change, test_cli_called_still_blocks_export was made fully typed and its ignore was dropped, so the file now uses two styles. Annotate the params as criterion: SuccessCriterion (already imported from coder_eval.models) and delete the ignores. Tests are outside pyright, so these ignores do nothing and only hide a real mismatch if tests are ever type-checked. The pre-existing line 78 has the same pattern. The code is test-only and has no runtime effect, so the severity is Low.
6. [Axis 3] The changed export CLI command body has no CLI-level test (17.54% coverage) (src/coder_eval/cli/export_command.py:79) — The changed calls result = export_task(task_files[0], output_dir) (line 79) and the export_experiment(all_task_files, experiment, output_dir) call are in the uncovered range 70-113. A grep finds no "export" CLI invocation in tests/. The removal of --allow-credentials is also not tested, so a stale reference would not fail pytest (tests/harbor_e2e/fixtures/llm_judge.yaml:4 still says (--allow-credentials)). Add one CliRunner smoke test for export <task> -o <dir> that asserts the directory is written and that an llm_judge task now exports with exit 0.
7. [Axis 4] Harbor verifier now trusts an agent-writable /logs/agent/task.json as the grading evidence for trajectory criteria, and the test.sh comment says this path is safe (src/coder_eval/harbor/packager.py:79) — This PR removes NEEDS_TRAJECTORY from _BLOCKING_IN_V1 (portability.py:73) and changes test.sh to coder-eval evaluate /tests/task.yaml /logs/agent --in-place --run-dir /logs/verifier || true (packager.py:79). Before this change, command_executed, commands_efficiency and skill_triggered could not export. Now they grade against prior, which load_prior_result reads from /logs/agent/task.json. Harbor runs the agent and the verifier in the same container, and the agent phase writes /logs/agent as the same user that the LLM's Bash tool runs as. So an agent can start a detached process (for example nohup sh -c 'sleep 30; cat > /logs/agent/task.json') that replaces the recorded trajectory after coder-eval execute exits and before /tests is mounted. That fakes a pass on a trajectory criterion without doing the work. Path traversal is NOT possible: default_workspace (regrade.py:406-421) checks the recorded sandbox_path and the artifacts children with _contained against run_dir, and _write_back and back_up_pre_grade_record refuse symlinks. The comment at packager.py:73-78 ("which is also what keeps this off the untrusted-recorded-config path ... does not apply once an explicit, operator-supplied task file is in hand") is only half true. The explicit task file makes the CONFIG trusted, but the trajectory and execution facts still come from an untrusted record. Change the comment so it says the trajectory evidence is agent-writable and outside the trust boundary. Add a matching caveat for NEEDS_TRAJECTORY in the portability.py docstring and in the Harbor export docs. That docstring also says test.sh grades /logs/agent/trajectory.json (ATIF), but test.sh grades task.json. Fix that mismatch in the same change. Optional hardening: make the agent-phase run dir read-only after execute (for example chmod -R a-w /logs/agent as the last step of CoderEvalAgent.run when it runs as a different user from the LLM), or record a digest of task.json that the host-side agent adapter keeps outside the container. CVSS:3.1/AV:L/AC:H/PR:L/UI:N/S:U/C:N/I:L/A:N
8. [Axis 5] Verifier phase now rewrites the agent phase's /logs/agent records (src/coder_eval/harbor/packager.py:79) — coder-eval evaluate /tests/task.yaml /logs/agent --in-place --run-dir /logs/verifier targets /logs/agent in RUN_DIR mode. In evaluate_command.py, if prior is not None and target.mode is EvaluateMode.RUN_DIR: ... _write_back(target.target, result, prepared_run_dir) (lines 582-587) replaces /logs/agent/task.json with the verdict, and back_up_pre_grade_record writes task.execute.json there. _refresh_run_summary then rebuilds /logs/agent/run.json. The verifier therefore mutates artifacts that belong to the agent phase. Harbor keeps a separate per-phase log directory, and agent.py's populate_context_post_run reads the agent-phase files. The golden comment says this avoids a 'plain workdir' guess, but it does not mention the write-back side effect. Fix: grade from a copy (--no-in-place) or pass the workspace explicitly, so that everything the verifier writes stays under /logs/verifier. Otherwise, document that the agent-phase record is overwritten on purpose.
9. [Axis 5] Portability enum/table keeps five classes, but only one now changes behaviour (src/coder_eval/harbor/portability.py:73) — After this PR, _BLOCKING_IN_V1 = frozenset({CriterionPortability.NEEDS_CLI_RECORDER}) (lines 73-77) is the only behavioural use of the classification. NEEDS_REFERENCE, NEEDS_TRAJECTORY and NEEDS_CREDENTIALS are now equal to PORTABLE for every caller (audit_criteria is the only consumer, packager.py:145). A long comment block (lines 67-72) and a docstring (lines 108-115) now explain why three of the four non-portable classes do nothing. Following 'Delete before you guard' and YAGNI, collapse the three inert classes into PORTABLE, or reduce the module to a registry-derived 'blocked types' check. Keep the fail-closed coverage of unknown criterion types.
10. [Axis 7] Stale references remain to the removed --allow-credentials flag and the NEEDS_TRAJECTORY hard error (src/coder_eval/harbor/portability.py:23) — portability.py:23 points to Rationale: .claude/notes/reporting.md § Not every criterion can grade inside someone else's container. That section still says trajectory criteria "cannot work in the export direction at all ... no iterations list to read without ATIF ingestion" and that credential criteria "are refused with an explicit opt-in escape hatch" (reporting.md:425-432). tests/harbor_e2e/fixtures/llm_judge.yaml:4 still says "llm_judge call inside the Harbor verifier phase (--allow-credentials)." Update the notes section and the fixture description so they match the new behavior. The flag was removed without an alias. That is acceptable under this repo's greenfield rule, the flag appears in no docs/ page, and old scripts fail loudly with 'No such option', so this finding is only about the leftover references. n/a

What's Missing

Parallel paths:

• 🟠 batch.py:159 moves task.json/task.log/artifacts to the flat run root under --workspace-dir, but the parallel readers of the same location were not changed: clear_rerun_artifacts (batch.py:435) and _load_completed_result (batch.py:444) still build rt.run_dir. Nothing rejects --resume together with --workspace-dir. Choose the location once (for example, set it on ResolvedTask) or reject the combination. (trigger: src/coder_eval/orchestration/batch.py) (restates: Axis 3: Flat --workspace-dir run_dir layout is chosen only in run_batch; --resume and finished-check paths still read the nested run_dir)
• 🟡 The export policy changed, but its prose copies did not: .claude/notes/reporting.md § 'Not every criterion can grade inside someone else's container' (lines 416-432) still describes trajectory criteria as impossible and credentials as an opt-in refusal. tests/harbor_e2e/fixtures/llm_judge.yaml:4 still names --allow-credentials. The portability.py docstring and comment, plus tests/test_harbor_portability.py:97 and tests/test_harbor_packager.py:73, still name a trajectory.json/--format harbor grading path. Update all of them in this change. (trigger: src/coder_eval/harbor/packager.py) (restates: Axis 7: Stale references remain to the removed --allow-credentials flag and the NEEDS_TRAJECTORY hard error)
• 🔵 The test.sh contract changed, but its consumer-side documentation did not. The CoderEvalAgent docstring (src/coder_eval/harbor/agent.py:83-84) still says --workspace-dir matters because otherwise the workspace lands 'somewhere Harbor's verifier never looks'. The verifier now finds the workspace through /logs/agent/task.json's recorded sandbox_path (a filtered capture_to copy). The same stale text is in .claude/shared/run-layout.md, plugins/coder-eval/reference/run-layout.md and the --workspace-dir help text in execute_command.py:187-198 and run_command.py:363-374. (trigger: src/coder_eval/harbor/packager.py) (restates: Axis 7: --workspace-dir now also flattens the run-directory layout (task.json beside run.json), and neither run-layout.md nor the flag help text documents this)

Tests:

• 🟠 The new flat-layout branch in run_batch (batch.py:159) has no test. Add a test that runs execute --workspace-dir W --run-dir R and asserts that R/task.json exists, that no R//<task_id>/00/ exists, and that --format harbor writes R/trajectory.json. tests/test_execute_format_harbor.py uses glob('**/task.json'), which passes for both layouts. (trigger: src/coder_eval/orchestration/batch.py) (restates: Axis 3: Flat --workspace-dir run_dir layout is chosen only in run_batch; --resume and finished-check paths still read the nested run_dir)
• 🟡 Add a pytest round trip that uses a mock agent: execute --workspace-dir W --run-dir A, then evaluate task.yaml A --in-place --run-dir B. Include a command_executed or skill_triggered criterion and assert score 1.0 in B/task.json. Add a negative case with an emptied trajectory. Today only the non-PR harbor-e2e.yml job exercises this path. (trigger: src/coder_eval/harbor/packager.py) (restates: Axis 3: The verifier grading contract in test.sh is checked only as a string; the round trip with a trajectory criterion runs only in the nightly e2e job, not on PRs)
• 🟡 No test covers the verifier when /logs/agent has no task.json or holds an unparsable one (agent timeout, or execute failing before the Orchestrator). In that case evaluate silently falls back to WORK_DIR mode and harbor reward writes a numeric reward. Add a test that asserts no reward.json is written. (trigger: src/coder_eval/harbor/packager.py) (restates: Axis 6: test.sh grades the agent log directory and writes a fabricated reward when /logs/agent has no task.json)
• 🟡 No test checks that grading an exported task sees the live workspace. Add a round-trip test with a run_command criterion that reads W/node_modules (or .venv) after execute --workspace-dir W. It fails today, because the verifier grades the capture_to copy, and capture_to drops those directories. (trigger: src/coder_eval/harbor/packager.py) (restates: Axis 8: Harbor verifier now grades a filtered COPY of the workspace, so criteria that need node_modules/.venv/.cache/.config/.local get a different score for the same agent output)
• 🔵 export_experiment removed allow_credentials, so experiments with llm_judge or trajectory criteria that were skipped before are now exported. tests/test_harbor_experiment_packager.py has no case that exports an experiment with an llm_judge or command_executed task and asserts skipped == []. It also has no case that shows cli_called is still skipped with a reason. (trigger: src/coder_eval/harbor/experiment_packager.py)
• 🔵 Add a CliRunner smoke test for export <task> -o <dir> with an llm_judge task (exit 0, directory written) and for --allow-credentials (it must be rejected as an unknown option), so the flag removal is pinned. (trigger: src/coder_eval/cli/export_command.py) (restates: Axis 3: The changed export CLI command body has no CLI-level test (17.54% coverage))

Downstream consumers:

• 🟠 The NEEDS_CREDENTIALS classification no longer blocks export, but its downstream consumers were not changed to match. ExportResult.warnings and the export CLI output add no line that names the env vars the verifier needs. reward.py still turns a Direct-route judge with no key (score 0.0, '(judge transport unconfigured)', not ERROR) into a numeric reward. Add a warning, or make an unconfigured judge finalize the row as ERROR so that reward.py writes no file. (trigger: src/coder_eval/harbor/portability.py) (restates: Axis 6: Judge criteria now export with no warning that the verifier needs model credentials and network access)
• 🟡 Grading /logs/agent with --in-place makes the verifier rewrite the agent phase's task.json and run.json and add task.execute.json. The PR does not check CoderEvalAgent.populate_context_post_run (agent.py) or Harbor's agent-phase log consumers against a record that the verifier later overwrites. Nor does it state whether reward/metrics should come from /logs/verifier or /logs/agent. (trigger: src/coder_eval/harbor/packager.py) (restates: Axis 5: Verifier phase now rewrites the agent phase's /logs/agent records)
• 🔵 After _BLOCKING_IN_V1 shrinks to NEEDS_CLI_RECORDER, no consumer reads NEEDS_REFERENCE, NEEDS_TRAJECTORY or NEEDS_CREDENTIALS any more (audit_criteria at packager.py:145 is the only caller). Either give those classes a new use (non-blocking export caveats) or collapse them into PORTABLE. (trigger: src/coder_eval/harbor/portability.py) (restates: Axis 5: Portability enum/table keeps five classes, but only one now changes behaviour)

Daily/nightly:

• 🔵 The PR changes run_batch, which is on the production run path, and changes the on-disk task.json location for run/execute --workspace-dir. The PR does not say what this means for the nightly or cross-repo pipeline. From the code: the change applies only when workspace_dir is set, and the DockerRunner (run_task_internal) goes to the Orchestrator directly, not through run_batch, so nightly runs are not affected. Previously exported Harbor task directories keep the old $(pwd) test.sh until they are regenerated. State this blast radius in the PR description. (trigger: src/coder_eval/orchestration/batch.py)

Harness & Lint Improvements

Static checks (lint / type):

• [ce-lint] CE068 (the next free number; CE067 is the highest in use): a CLI flag reference must resolve. Walk the Typer app (coder_eval.cli:app) and collect every registered --option name. Then scan .claude/notes/**/*.md, docs/**/*.md, tests/**/fixtures/**/*.yaml, tasks/**/*.yaml, plugins/coder-eval/** and action.yml for backticked or parenthesised --kebab-flag tokens. Fail on any token that no command defines, with a small allowlist for flags of external tools (harbor, docker, uv, npm). Wire it as a @pytest.mark.lint class in tests/test_custom_lint.py, because it reasons over Markdown and YAML (the same pattern as CE029 in tests/lint/doc_examples.py). Prevents: The stale --allow-credentials references left after the flag was removed (tests/harbor_e2e/fixtures/llm_judge.yaml:4 and the reporting.md escape-hatch text), and any later option rename or removal that leaves notes, fixtures or skills pointing at a flag that no longer exists.
• [ce-lint] CE069: every enum member must change behaviour, not only be classified. For each CriterionPortability member, and in general for each member of an enum in harbor/ that a _PORTABILITY_BY_TYPE-style table uses, require at least one reference OUTSIDE the classification table: a membership in a blocking set such as _BLOCKING_IN_V1, or a branch that emits an ExportResult.warnings entry. It is an AST check in tests/lint/rules/ce069_portability_class_has_effect.py, wired in tests/lint/runner.py. Blind spot: a reference that is a no-op branch. Prevents: The removal of NEEDS_CREDENTIALS from _BLOCKING_IN_V1 with no replacement warning, which lets llm_judge tasks export silently and score 0.0 when the verifier has no key (portability.py:73, the high-severity A6 finding). It also catches the three inert classes (NEEDS_REFERENCE / NEEDS_TRAJECTORY / NEEDS_CREDENTIALS) that are now the same as PORTABLE (the A5 finding at portability.py:73). The fix must either delete the class or give it an effect, such as a warning.
• [ce-lint] CE070: no hand-written sets of criterion type names. Forbid a set/frozenset/tuple/list literal that holds two or more string constants equal to registered criterion type discriminators (read from CriterionRegistry.list_types(), or statically from the Literal[...] type fields in models/criteria.py). Exempt models/criteria.py, criteria/ and harbor/portability.py (the SSOT table). The scope is src/, tests/ and .github/scripts/, which match LINT_PATHS. Put it in tests/lint/rules/ce070_no_criterion_type_string_sets.py, wired in tests/lint/runner.py. Prevents: _TRAJECTORY_CRITERION_TYPES = frozenset({"command_executed", "commands_efficiency", "skill_triggered"}) in .github/scripts/harbor_e2e.py:66, which can drift from portability._PORTABILITY_BY_TYPE (the grouped A1/A2/A3/A6/A8 finding). The author then has to derive the set from the portability table.
• [ce-lint] CE071: run_dir has one resolution seam. In src/coder_eval/orchestration/, forbid an IfExp or if assignment that selects between two different .run_dir attribute reads (for example config.run_dir if ... else rt.run_dir). Forbid reading rt.run_dir directly for task-record paths (TASK_JSON_FILENAME, artifacts) anywhere except ResolvedTask's own property. The flat or nested layout choice must live in ResolvedTask.run_dir, so that run_batch, clear_rerun_artifacts and _load_completed_result all read the same value. Put it in tests/lint/rules/ce071_run_dir_single_seam.py, wired in tests/lint/runner.py. Prevents: The divergence between batch.py:159 (effective_run_dir, flat) and batch.py:435/444 (rt.run_dir, nested) that makes --resume together with --workspace-dir never find the finalized task.json (the high-severity A3/A5/A6/A8 finding).
• [ce-lint] Widen make docs-budget (tests/lint/prose_budget.py) so the 8-line comment-run cap also covers emitted scripts. Scan (a) module-level str constants whose name ends in _TEMPLATE, _SH or _DOCKERFILE and count #-prefixed lines inside the literal, and (b) golden fixture scripts under tests/_fixtures/**/*.sh and templates/**/*.sh. Apply the same _COMMENT_RUN_LINES = 8 / _RUN_BLANK_BRIDGE = 1 rules. Prevents: The 15-line history comment in _TEST_SH_TEMPLATE (packager.py:64-78), which is copied into tests/_fixtures/harbor_export_golden/expected/tests/test.sh:16-30 and ships in every exported task. The budget cannot see it today because it is inside a string literal.
• [ce-lint] CE072 (a ratchet cap): no inert # type: ignore in files that pyright does not check. pyright include is only src/coder_eval, so an ignore under tests/ or .github/scripts/ does nothing. Count # type: ignore comments in those trees and fail when the count is above a cap set BELOW the current value (currently 159 under tests/, so cap at 158 and ratchet down). Also forbid the specific shape of a test parameter annotated object that is then passed with # type: ignore[...]. Put it in tests/lint/rules/ce072_no_inert_type_ignore.py. The alternative is to bring tests/ under pyright in basic mode, which is a larger change. Prevents: criterion: object together with # type: ignore[list-item] in tests/test_harbor_portability.py:95/98, 117/120 and 78, where SuccessCriterion was already imported. The file ended up with two styles in one change.
• [ce-lint] CE073: every Typer command has a CliRunner test. For each function registered with @app.command(name=...) under src/coder_eval/cli/, require at least one runner.invoke(app, ["<name>", ...]) (or the CliRunner().invoke equivalent) somewhere in tests/. Wire it as a @pytest.mark.lint class in tests/test_custom_lint.py, because it is a whole-tree cross-reference between src and tests. Blind spot: it proves the command is invoked, not that its new branches are asserted. Prevents: The export command body in src/coder_eval/cli/export_command.py:70-113, which had no CLI-level test (17.54% coverage), so the removal of --allow-credentials and the changed export_task/export_experiment calls were never exercised through the CLI.

Harness improvements (not statically reachable):

• Add a pytest round-trip integration test for the Harbor grading contract, using a mock agent as test_execute_evaluate_loop.py does. Run execute <task> --workspace-dir W --run-dir A, then evaluate task.yaml A --in-place --run-dir B. Assert: A/task.json exists and A//<task_id>/00/ does not; --format harbor writes A/trajectory.json; B/task.json has a command_executed result with score 1.0; a negative case with an emptied trajectory scores 0.0; and a criterion that reads node_modules/ (or .venv/) inside W passes. Also make --resume together with --workspace-dir either tested or rejected. Why not static: The defects are behavioural and only visible at runtime: which directory resolve_evaluate_target picks, what capture_to's ignore list drops from the graded copy, and whether a trajectory criterion hydrates from task.json. They need a real execute followed by an evaluate. Prevents: The untested flat run_dir layout (batch.py:159), the verifier round trip that only the nightly e2e covers (packager.py:79), and the filtered-copy score change where node_modules/.venv/.cache are dropped from the graded workspace (the high-severity A8 finding at packager.py:79).
• Add a golden test on the emitted test.sh for the unmeasured-row contract. Run the generated test.sh (or its evaluate line plus coder-eval harbor reward) against a /logs/agent that has (a) no task.json and (b) an unparseable task.json. Assert that no reward.json is written. Add an evaluate option that REQUIRES RUN_DIR mode, so that it refuses the silent WORK_DIR fallback, and assert that the packager uses it. Why not static: The failure is the runtime interaction of resolve_evaluate_target's fallback, || true, and reward.py's skip rule when a file is missing. A text match on test.sh cannot prove that the reward path stays unwritten. Prevents: The fabricated reward when Harbor's agent timeout or an early execute failure leaves /logs/agent without task.json (the high-severity A6/A8 finding at packager.py:79).
• Add a verify-time guard for judge criteria with no credentials. Make an llm_judge on the Direct route with no transport finalize the row as ERROR (so reward.py writes nothing), not as a scored 0.0. Add a contract test that runs a judge criterion with ANTHROPIC_API_KEY unset and asserts the final status category is error. Also add a harbor_e2e scenario that exports an llm_judge task and runs the verifier without the key. Why not static: Whether a missing key becomes a scored 0 or an ERROR depends on the route resolved at runtime and on env state. Only an execution with the key absent shows it. Prevents: The silent 0.0 reward for exported llm_judge tasks after the NEEDS_CREDENTIALS refusal was removed (the high-severity A6/A7 finding at portability.py:73).
• Trigger the Harbor e2e workflow (.github/workflows/harbor-e2e.yml) on pull_request with a path filter on src/coder_eval/harbor/**, src/coder_eval/cli/evaluate*, src/coder_eval/orchestration/batch.py, src/coder_eval/regrade.py and tests/_fixtures/harbor_export_golden/**. In harbor_e2e.py, load verifier records with EvaluationResult.model_validate_json and fail when a scenario that declares a trajectory criterion yields zero matching results. Why not static: The job needs Docker, a Harbor install and a live container round trip. A vacuous pass (an empty success_criteria_results) is only visible from the data the run produces. Prevents: Harbor contract regressions that reach main because the only round-trip coverage is nightly or manual, and the vacuous pass of the trajectory_criteria scenario (harbor_e2e.py:144-149).
• Add a phase-isolation assertion to the Harbor e2e job. Take a tree digest of /logs/agent (with path_utils's tree-digest helper) after the agent phase and again after the verifier phase, and assert they are equal. Also add an anti-tamper scenario: the agent starts a detached process that rewrites /logs/agent/task.json after execute exits, and the check asserts that the trajectory criterion does not pass. Why not static: Both properties are about what one container phase writes and when. That needs the real Harbor agent-then-verifier sequence and a background process racing it. Prevents: The verifier overwriting agent-phase records through --in-place RUN_DIR write-back (the A5 finding at packager.py:79), and the agent-writable task.json being trusted as grading evidence (the A4 security finding at packager.py:79).
• Add a CI co-change check: when a diff changes _BLOCKING_IN_V1, _PORTABILITY_BY_TYPE or _TEST_SH_TEMPLATE, require the same PR to touch the Rationale: target section (.claude/notes/reporting.md § Not every criterion can grade inside someone else's container) and .claude/shared/run-layout.md when batch.py run_dir resolution changes. A reviewer can waive it with a PR label. Why not static: Whether the prose still matches the behaviour is a semantic judgement. A diff-coupling heuristic can only force a human to re-read the section; it cannot check that the text is correct. The existing Rationale: resolver only proves the heading exists. Prevents: The stale ATIF trajectory.json grading claims in portability.py:13-15/69-70, tests/test_harbor_portability.py:97 and tests/test_harbor_packager.py:73 (the A7 medium finding), the stale reporting.md escape-hatch text, and the undocumented flat layout in run-layout.md (the A7 medium finding at batch.py:159).

Top 5 Priority Actions

1. Make the generated test.sh grade the live workspace, not the filtered capture_to copy that drops node_modules/.venv/.cache/.config/.local: pass --workspace "$(pwd)" to coder-eval evaluate at src/coder_eval/harbor/packager.py:79, so identical agent output scores the same as it did before this change.
2. Stop test.sh from writing a fabricated reward when /logs/agent/task.json is missing (agent timeout or an early execute failure): add a [ -f /logs/agent/task.json ] || exit 0 guard, or add a way to make evaluate require RUN_DIR mode, at src/coder_eval/harbor/packager.py:79, plus a golden test for that case.
3. Put back a loud signal for NEEDS_CREDENTIALS criteria that the change at src/coder_eval/harbor/portability.py:73 removed: add an ExportResult.warnings entry that names the env vars the verifier needs, and make a judge with no transport (criteria/llm_judge.py) finalize the row as ERROR, not score 0.0, so reward.py writes no reward.
4. Add pytest round-trip tests for the flat --workspace-dir layout at src/coder_eval/orchestration/batch.py:159 (assert R/task.json exists, R//<task_id>/00 does not, then evaluate task.yaml R --in-place --run-dir B grades command_executed at 1.0). Also reject --resume together with --workspace-dir, or make _load_completed_result (batch.py:444) and clear_rerun_artifacts (batch.py:435) use the flat path.
5. Make the Harbor contract docs match what the code does: fix the stale trajectory.json/ATIF grading claims in src/coder_eval/harbor/portability.py:13-15 and 67-72 and in the test docstrings, add the flat layout to .claude/shared/run-layout.md, update .claude/notes/reporting.md, and say in the test.sh comment that agent-writable task.json evidence is outside the trust boundary.

---

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

[uipreliga]

Fix what you agree with and 🚢

[akshaylive]

Thanks for the thorough review! Went through the three blockers:

Judge criteria failing silently (score 0.0 instead of ERROR) when creds are missing — agreed this is fine as-is. Not fixing.

Verifier grading a filtered capture_to copy instead of the live workspace — ran this in practice and it works fine for our current task set. Not fixing for now; if node_modules/.venv-dependent criteria show up later we can revisit.

Fabricated reward when /logs/agent/task.json is missing — traced this through resolve_evaluate_target → orchestrator._setup()'s evaluate-only branch. When task.json is absent, evaluate falls back to WORK_DIR mode with no prior result, so self.agent stays None and criteria grade directly against whatever's on disk with empty turn_records. File criteria score against whatever partial output the crashed agent left (same exposure the old $(pwd)-live-grading path already had — not new), and trajectory criteria (command_executed etc.) score 0.0 since there's no trajectory to hydrate. So this writes a real 0.0, not a fabricated 1.0 — there's no path here that invents a pass for work that didn't happen. The actual gap is narrower than "wrong score": the row finalizes as FAILURE rather than ERROR, so reward.py's skip-on-unmeasured check doesn't fire and a reward file gets written for what was an infra crash rather than a real attempt. That's a data-hygiene nit (mixing "agent failed" with "agent never ran" in aggregate stats), not a scoring bug, and we're OK with it for now.

Already pushed fixes for the --resume + --workspace-dir divergence you flagged (guard added in run_command.py, plus a batch-level test) and the stale doc/comment references (.claude/notes/reporting.md, the llm_judge.yaml fixture description).