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

.claude/notes/reporting.md

@@ -422,13 +422,17 @@ where it is an unexplained low reward with no obvious cause.
 Filesystem and exit-code checks are portable — nothing about the verifier container changes
 what they need. `reference_comparison` needs the reference tree, which the export always
 places verifier-side when the task declares one, so that class never actually blocks.
-Trajectory-reading criteria cannot work in the export direction at all: the verifier is a
-separate process from the agent phase, and the agent may not even BE coder-eval, so there is
-no iterations list to read without ATIF ingestion. `cli_called` reads a log written by a
-recorder shim coder-eval's own sandbox installs, which the exported Dockerfile does not
-provision. Credential-needing criteria need a model reachable from inside the verifier
-container and a judge that does not follow the agent's route; they are refused with an
-explicit opt-in escape hatch for an operator who has provisioned that themselves.
+Trajectory-reading criteria once could not work in the export direction at all, but no
+longer block: the packager's `CoderEvalAgent` always runs `coder-eval execute --format
+harbor`, which writes `/logs/agent/trajectory.json` (ATIF) alongside `task.json`, and the
+generated `tests/test.sh` grades `/logs/agent` as a run directory — so the trajectory is
+always there by the time the verifier runs. `cli_called` reads a log written by a recorder
+shim coder-eval's own sandbox installs, which the exported Dockerfile does not provision —
+this is the one class that still hard-blocks. Credential-needing criteria need a model
+reachable from inside the verifier container and a judge that does not follow the agent's
+route; the export no longer gates them behind a flag — it exports them unconditionally on
+the assumption that an operator exporting one has already provisioned that access
+themselves (C2 does not do it for them).
 
 Coverage is registry-derived, so a new criterion type added to the union without a
 classification fails CLOSED rather than silently exporting as if it were portable.
@@ -508,12 +512,18 @@ letting a bare `OSError` escape, because the CLI catches only the export errors
 unreadable tree would otherwise abort a whole experiment export the docstring promises it
 will not abort.
 
-The generated shell script no longer interpolates a workdir value at all — it resolves its
-own cwd via `$(pwd)` at run time, a fixed literal in `_TEST_SH_TEMPLATE`. `docker exec`
-(with `-w` when the task set an explicit override, or none when it did not — see above)
-always lands the shell there, whether or not the same container's agent phase used an
-explicit override too, so `pwd` is authoritative and there is no longer an injection
-surface to `shlex.quote` against.
+The generated shell script no longer interpolates a workdir value, or guesses a cwd, at
+all — it is a fixed literal in `_TEST_SH_TEMPLATE` that calls `coder-eval evaluate
+/tests/task.yaml /logs/agent --in-place --run-dir /logs/verifier`, passing `/logs/agent`
+as an explicit RUN DIRECTORY rather than a workdir guess. `coder-eval execute --run-dir
+/logs/agent ...` (the agent phase) always finishes with `/logs/agent/task.json` and
+`/logs/agent/artifacts/<task_id>/`, so `coder-eval evaluate` locates the workspace from
+that `task.json`'s own recorded `sandbox_path` instead of a live `$(pwd)`. Passing the
+task file explicitly also keeps this off the untrusted-recorded-config path (which exists
+for a shared run directory whose config is not to be trusted without
+`--allow-recorded-commands`) — an explicit, operator-supplied task file always overrides
+the run's recorded config. Nothing task-controlled is interpolated into the template, so
+there is no injection surface to `shlex.quote` against.
 
 ## The ATIF trajectory bridge
 

.github/scripts/harbor_e2e.py

@@ -36,7 +36,6 @@ class Scenario:
 
     name: str
     task_file: Path
-    allow_credentials: bool = False
 
 
 SCENARIOS: list[Scenario] = [
@@ -45,15 +44,27 @@ class Scenario:
     # export -> CoderEvalAgent -> --workspace-dir -> verifier round trip.
     Scenario("baseline", REPO_ROOT / "tests/harbor_e2e/fixtures/docker_baseline.yaml"),
     # llm_judge: real model call inside the VERIFIER phase, not just the agent.
-    Scenario("llm_judge", REPO_ROOT / "tests/harbor_e2e/fixtures/llm_judge.yaml", allow_credentials=True),
+    Scenario("llm_judge", REPO_ROOT / "tests/harbor_e2e/fixtures/llm_judge.yaml"),
     # Custom (BYOD) Docker image via dockerfile_path -- reuses the in-tree
     # byod_smoke_test task/image rather than duplicating it.
     Scenario("docker_custom_image", REPO_ROOT / "tasks/byod_smoke_test.yaml"),
     # template_sources: TemplateDirSource copy-in + rewritten path, plus
     # sandbox.python.env_packages surviving the agent-phase task.yaml merge.
     Scenario("template_sources", REPO_ROOT / "tests/harbor_e2e/fixtures/template_sources.yaml"),
+    # command_executed: catches a regression where the verifier phase grades
+    # against a directory with no trajectory data -- reward could still land on
+    # 1.0 "by luck" from unrelated criteria while this one silently scores 0.0,
+    # so `assert_scenario_artifacts` checks its own criterion score directly
+    # rather than trusting the aggregate reward alone.
+    Scenario("trajectory_criteria", REPO_ROOT / "tests/harbor_e2e/fixtures/trajectory_criteria.yaml"),
 ]
 
+# Criteria types that can only score correctly if the verifier phase actually
+# hydrated the agent phase's trajectory (see portability.py's NEEDS_TRAJECTORY
+# class). Checked by name per scenario below rather than globally, since only
+# `trajectory_criteria` declares one.
+_TRAJECTORY_CRITERION_TYPES = frozenset({"command_executed", "commands_efficiency", "skill_triggered"})
+
 
 def _run(cmd: list[str]) -> subprocess.CompletedProcess[str]:
     print(f"+ {' '.join(cmd)}", flush=True)
@@ -64,8 +75,6 @@ def export_task(scenario: Scenario, out_dir: Path) -> None:
     if out_dir.exists():
         shutil.rmtree(out_dir)
     cmd = ["coder-eval", "export", str(scenario.task_file), "-o", str(out_dir)]
-    if scenario.allow_credentials:
-        cmd.append("--allow-credentials")
     result = _run(cmd)
     print(result.stdout)
     print(result.stderr, file=sys.stderr)
@@ -128,9 +137,27 @@ def assert_scenario_artifacts(scenario: Scenario, trial_dir: Path) -> None:
     if not verifier_task_json.is_file():
         raise RuntimeError(f"[{scenario.name}] missing {verifier_task_json}")
 
+    # An overall reward of 1.0 does not prove a trajectory criterion was
+    # actually graded -- it could pass "by luck" from unrelated criteria while
+    # this one silently scored 0.0 against an ungraded/empty trajectory. Check
+    # each trajectory-dependent criterion's OWN score directly.
+    verifier_result = json.loads(verifier_task_json.read_text(encoding="utf-8"))
+    trajectory_results = [
+        r
+        for r in verifier_result.get("success_criteria_results", [])
+        if r.get("criterion_type") in _TRAJECTORY_CRITERION_TYPES
+    ]
+    for r in trajectory_results:
+        if r.get("score") != 1.0:
+            raise RuntimeError(
+                f"[{scenario.name}] {r.get('criterion_type')} criterion did not score 1.0 "
+                + f"(got {r.get('score')!r}); trajectory hydration likely broken: {r.get('details')!r}"
+            )
+
     print(
         f"[{scenario.name}] OK: reward=1.0, trajectory.json present, "
         + f"{len(agent_task_jsons)} agent task.json + verifier/task.json present"
+        + (f", {len(trajectory_results)} trajectory criterion/criteria verified" if trajectory_results else "")
     )
 
 

src/coder_eval/cli/export_command.py

@@ -60,15 +60,6 @@ def export_command(
         "--format",
         help=f"Target format. Supported: {', '.join(_SUPPORTED_FORMATS)}.",
     ),
-    allow_credentials: bool = typer.Option(
-        False,
-        "--allow-credentials",
-        help=(
-            "Export criteria that need model credentials/network inside the verifier "
-            "(llm_judge / agent_judge / uipath_eval) anyway. Only pass this if you have "
-            "already provisioned that access yourself — the export does not do it for you."
-        ),
-    ),
 ) -> None:
     """Export a coder-eval task (or task x experiment.yaml variants) to another framework's directory format.
 
@@ -85,7 +76,7 @@ def export_command(
             console.print("[red]✗[/] Without --experiment, pass exactly one task YAML.")
             raise typer.Exit(1)
         try:
-            result = export_task(task_files[0], output_dir, allow_credentials=allow_credentials)
+            result = export_task(task_files[0], output_dir)
         except (TaskNotExportableError, CriteriaNotExportableError) as e:
             console.print(f"[red]✗[/] {e}")
             raise typer.Exit(1) from e
@@ -99,7 +90,6 @@ def export_command(
         all_task_files,
         experiment,
         output_dir,
-        allow_credentials=allow_credentials,
     )
 
     for exported in exp_result.exported:

src/coder_eval/cli/run_command.py

@@ -468,6 +468,12 @@ def run_pipeline(
     # --resume needs an explicit run dir to resume into (auto-generated dirs are always fresh).
     if resume and run_dir is None:
         raise typer.BadParameter("--resume requires --run-dir pointing at the run to continue.")
+    # --workspace-dir writes the finalized task.json flat at run_dir (batch.py's
+    # effective_run_dir); --resume's bookkeeping (clear_rerun_artifacts,
+    # _load_completed_result) reads/clears the nested per-task run_dir instead, so a
+    # resumed workspace-dir run would never recognize its own prior result.
+    if resume and workspace_dir is not None:
+        raise typer.BadParameter("--resume is not supported together with --workspace-dir.")
     # Without --resume this flag parsed, was accepted, and did nothing at all. Its
     # sibling mode-scoped flag (`evaluate --workspace`) hard-errors on exactly this.
     if allow_host_grading and not resume:

src/coder_eval/harbor/experiment_packager.py

@@ -119,8 +119,6 @@ def export_experiment(
     task_files: list[Path],
     experiment_file: Path,
     out_dir: Path,
-    *,
-    allow_credentials: bool = False,
 ) -> ExperimentExportResult:
     """Export every (task x variant x replicate[ x dataset row]) combination to Harbor directories.
 
@@ -183,7 +181,6 @@ def export_experiment(
                 resolved.task,
                 resolved.task_file,
                 dest,
-                allow_credentials=allow_credentials,
             )
         except (TaskNotExportableError, CriteriaNotExportableError) as e:
             skipped.append(

src/coder_eval/harbor/packager.py

@@ -61,13 +61,22 @@
 # always reach it.
 set -u
 
-# `$(pwd)` -- not a baked-in path -- so this script is agnostic of whatever
-# WORKDIR the agent's container actually used (task.toml's `environment.workdir`
-# when the task set one explicitly, or the image's own built-in WORKDIR
-# otherwise; see _write_environment). `docker exec` (or `-w`, when set) always
-# lands this shell's cwd there, so `pwd` is authoritative at run time -- no
-# export-time guess needed, and nothing to drift if the image changes later.
-coder-eval evaluate /tests/task.yaml "$(pwd)" --in-place --run-dir /logs/verifier || true
+# `/tests/task.yaml /logs/agent` -- an explicit task file over a RUN DIRECTORY,
+# not a plain workdir. CoderEvalAgent's `coder-eval execute --run-dir /logs/agent
+# ...` always finishes with `/logs/agent/task.json` (this task's own recorded
+# trajectory) and `/logs/agent/artifacts/<task_id>/` (the workspace it produced),
+# so `coder-eval evaluate` recognizes /logs/agent as a run directory and grades
+# against it directly -- no `$(pwd)` guess of the agent's WORKDIR needed (the
+# workspace is located from task.json's own recorded sandbox_path instead), and
+# no ATIF trajectory.json round-trip either (task.json already carries the same
+# trajectory natively). Passing the task file explicitly (rather than the bare
+# run directory alone) makes coder-eval grade with THIS file -- the exported
+# contract -- instead of rebuilding the task from the run's own recorded config,
+# which is also what keeps this off the untrusted-recorded-config path: that
+# path exists for a shared run directory whose config is not to be trusted
+# without --allow-recorded-commands, and does not apply once an explicit,
+# operator-supplied task file is in hand.
+coder-eval evaluate /tests/task.yaml /logs/agent --in-place --run-dir /logs/verifier || true
 coder-eval harbor reward /logs/verifier --out /logs/verifier/reward.json
 """
 
@@ -84,12 +93,7 @@ def __init__(self, issues: list[PortabilityIssue]) -> None:
         lines = "\n".join(
             f"  - {i.criterion_description!r} ({i.criterion_type}): {i.portability.value}" for i in issues
         )
-        super().__init__(
-            f"{len(issues)} criterion/criteria cannot export to Harbor v1:\n{lines}\n"
-            + "Remove them, or pass allow_credentials=True (--allow-credentials on the CLI) for the "
-            + "NEEDS_CREDENTIALS ones if you have already provisioned model access inside the verifier "
-            + "container yourself."
-        )
+        super().__init__(f"{len(issues)} criterion/criteria cannot export to Harbor v1:\n{lines}\nRemove them.")
 
 
 @dataclass(frozen=True)
@@ -104,8 +108,6 @@ class ExportResult:
 def export_task(
     task_file: Path,
     out_dir: Path,
-    *,
-    allow_credentials: bool = False,
 ) -> ExportResult:
     """Emit a Harbor task directory at ``out_dir`` from the coder-eval task at ``task_file``.
 
@@ -118,15 +120,13 @@ def export_task(
             portability audit.
     """
     task, _raw_yaml = load_task(task_file)
-    return export_resolved_task(task, task_file, out_dir, allow_credentials=allow_credentials)
+    return export_resolved_task(task, task_file, out_dir)
 
 
 def export_resolved_task(
     task: TaskDefinition,
     task_file: Path,
     out_dir: Path,
-    *,
-    allow_credentials: bool = False,
 ) -> ExportResult:
     """Emit a Harbor task directory from an already-loaded/resolved ``TaskDefinition``.
 
@@ -142,7 +142,7 @@ def export_resolved_task(
 
     Raises the same two errors as ``export_task``, for the same reasons.
     """
-    issues = audit_criteria(task.success_criteria, allow_credentials=allow_credentials)
+    issues = audit_criteria(task.success_criteria)
     if issues:
         raise CriteriaNotExportableError(issues)
 
@@ -244,18 +244,12 @@ def _write_environment(
     """Derive ``environment/`` and return ``(workdir, docker_image)``.
 
     ``workdir`` is an EXPLICIT override only — ``sandbox.docker.working_dir`` or a
-    Dockerfile's own ``WORKDIR`` line. ``None`` otherwise, so Harbor's ``docker exec``
-    gets no ``-w`` and lands wherever the image's OWN ``WORKDIR`` already puts it;
-    ``tests/test.sh`` resolves the real cwd itself at run time via ``$(pwd)``.
-
-    ``docker_image`` is set only when no ``environment/Dockerfile`` was written, so
-    ``task.toml``'s ``[environment].docker_image`` points at the pre-built image;
-    ``None`` when a Dockerfile was written and Harbor must build from it.
-
-    A ``dockerfile_path`` is the only shape that writes a Dockerfile, copied in
-    UNCHANGED — no ``WORKDIR`` appended even when it declares none.
-    ``environment/task.yaml`` is bind-mounted at :data:`AGENT_TASK_YAML_PATH`, never
-    ``COPY``'d.
+    Dockerfile's own ``WORKDIR`` line — ``None`` otherwise. ``docker_image`` is set only
+    when no ``environment/Dockerfile`` was written, so ``task.toml``'s
+    ``[environment].docker_image`` points at the pre-built image; ``None`` when a
+    Dockerfile was written and Harbor must build from it. A ``dockerfile_path`` is the
+    only shape that writes a Dockerfile, copied in UNCHANGED. ``environment/task.yaml``
+    is bind-mounted at :data:`AGENT_TASK_YAML_PATH`, never ``COPY``'d.
 
     Rationale: .claude/notes/reporting.md § What the export carries, and what it refuses to carry
     """
@@ -274,8 +268,8 @@ def _write_environment(
         shutil.copy2(source_dockerfile, dest_dockerfile)
         # No fabricated WORKDIR appended when the Dockerfile declares none --
         # the built image just inherits its base image's own default, and
-        # tests/test.sh finds it at run time via `$(pwd)` either way (see
-        # _TEST_SH_TEMPLATE and this function's docstring).
+        # CoderEvalAgent's own `--workspace-dir "$(pwd)"` (agent.py) finds it
+        # at run time either way (see this function's docstring).
         workdir = docker_cfg.working_dir or _find_workdir(dest_dockerfile)
         if not _from_line_mentions_coder_eval_agent(dest_dockerfile):
             warnings.append(_MISSING_CODER_EVAL_WARNING)
@@ -567,9 +561,9 @@ def _write_agent_phase_task_yaml(
 
 
 def _write_test_sh(out_dir: Path) -> None:
-    # No task-controlled value is interpolated into the template anymore --
-    # `$(pwd)` is a fixed literal (see _TEST_SH_TEMPLATE) -- so there is no
-    # longer an injection surface here to shlex.quote against.
+    # No task-controlled value is interpolated into the template anymore -- it
+    # is a fixed literal (see _TEST_SH_TEMPLATE) -- so there is no longer an
+    # injection surface here to shlex.quote against.
     path = out_dir / "tests" / "test.sh"
     path.write_text(_TEST_SH_TEMPLATE, encoding="utf-8")
     path.chmod(0o755)