diff --git a/.github/workflows/cicd-main.yml b/.github/workflows/cicd-main.yml index 9253bd9358b..fb12a30efcf 100644 --- a/.github/workflows/cicd-main.yml +++ b/.github/workflows/cicd-main.yml @@ -96,9 +96,9 @@ jobs: with: base-ref: ${{ needs.pre-flight.outputs.base_ref }} - # P0 is published separately; requiring that check is a repository rules decision. + # Conformance is published separately as an optional check. harness_conformance: - name: Harness conformance + name: Harness conformance (non-blocking) needs: [pre-flight] uses: ./.github/workflows/harness-conformance.yml permissions: @@ -106,7 +106,9 @@ jobs: actions: read checks: write with: - base-ref: ${{ needs.pre-flight.outputs.base_ref }} + # Queue groups and main pushes can contain more than one commit. + # PR mirrors retain the full branch diff against the PR's base branch. + base-ref: ${{ github.event.merge_group.base_sha || (github.ref == 'refs/heads/main' && github.event.before) || needs.pre-flight.outputs.base_ref }} container_build: name: Build Gym container diff --git a/.github/workflows/harness-conformance.yml b/.github/workflows/harness-conformance.yml index f941ea4d057..f6e901ff0cd 100644 --- a/.github/workflows/harness-conformance.yml +++ b/.github/workflows/harness-conformance.yml @@ -128,7 +128,7 @@ jobs: echo "[Download conformance report and logs]($ARTIFACT_URL) — open probes/conformance_report.md in the artifact." >> "$GITHUB_STEP_SUMMARY" report: - name: Publish P0 check + name: Publish check needs: [select, probe] if: always() runs-on: ubuntu-latest @@ -141,7 +141,7 @@ jobs: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 with: persist-credentials: false - - name: Publish P0 result from this attempt's evidence + - name: Publish result from this attempt's evidence env: GH_TOKEN: ${{ github.token }} SELECTED_HARNESSES: ${{ needs.select.outputs.harnesses }} diff --git a/scripts/harness_conformance/README.md b/scripts/harness_conformance/README.md index 99340f089f8..ee611f7fd98 100644 --- a/scripts/harness_conformance/README.md +++ b/scripts/harness_conformance/README.md @@ -3,17 +3,24 @@ ## Pull-request feedback Gym CI runs a harness probe matrix on approved PR mirrors and merge-queue -revisions. Changes to a registered adapter select that harness; shared code, -dependency files, CI configuration, and other server code conservatively select -all registered harnesses. Changes limited to `fern/`, `docs/`, or root Markdown -files are skipped. Scheduled and manual CI runs exercise all harnesses. +revisions and on main pushes. Changes to a registered adapter select that harness. +Shared Gym code, conformance tooling and tests, the `legacy_agent`, +`single_agent_turn`, and `single_agent_turn_legacy` environments, dependency files, +and CI configuration select all registered harnesses. Unrelated servers, CLI/task +ingestion, other tests, benchmarks, and documentation are skipped. Unknown paths +conservatively select all harnesses. Scheduled and manual CI runs exercise all +harnesses. + +PR mirrors compare the full branch against its base branch. Main pushes compare +against the push's previous commit; merge-queue groups compare against their base +SHA, including every commit in the group. An unavailable diff runs all harnesses. Each job installs the adapter's pinned runtime, validates the runner, checker, and adapter tests, then runs every scenario. Execution errors fail the probe job; completed measurements can succeed as jobs even when they find conformance failures. Warnings and a detailed table remain available in the Actions summary. -After the probes finish, a separate reporting job publishes **Harness conformance P0** +After the probes finish, a separate reporting job publishes **Harness conformance (non-blocking)** in the PR checks list. It succeeds only if every selected harness passes the current P0 suite, or no harnesses are affected. Conformance failures and unavailable measurements both produce a failed check, with the first failure and number of @@ -25,13 +32,13 @@ Download the `harness-conformance-*` artifacts for test/setup logs and probe evi The reporter only uses artifacts from the current run attempt; rerun all jobs to refresh results for all selected harnesses. -After publishing the result, **Publish P0 check** also fails if P0 fails or any -probe job fails or is cancelled, making the result visible in the workflow job list. +After publishing the result, **Harness conformance (non-blocking) / Publish check** +also fails if P0 fails or any probe job fails or is cancelled, making the result +visible in the workflow job list. -To enforce P0 before merging, add the fixed **Harness conformance P0** check name to -the repository's required checks. Until configured, it remains optional. It is -independent of the existing `Nemo_CICD_Test` gate. Completed scheduled/manual main -runs trigger the Fern publisher, which consumes the same retained measurements. +The check is optional and independent of the existing `Nemo_CICD_Test` gate. +Completed scheduled/manual main runs trigger the Fern publisher, which consumes +the same retained measurements. Use the diagnostic command below to reproduce a warning at the reported commit. When registering another harness in `registry.HARNESSES`, provide its pinned runtime diff --git a/scripts/harness_conformance/checks.py b/scripts/harness_conformance/checks.py index e32d4091565..91e1ae232f8 100644 --- a/scripts/harness_conformance/checks.py +++ b/scripts/harness_conformance/checks.py @@ -14,6 +14,9 @@ from .registry import HARNESSES +CHECK_NAME = "Harness conformance (non-blocking)" + + class CheckOutput(TypedDict): title: str summary: str @@ -87,7 +90,7 @@ def p0_check( summary += reproduction(rerun) summary += f"\n\nTested commit: `{sha}`." return { - "name": "Harness conformance P0", + "name": CHECK_NAME, "head_sha": sha, "status": "completed", "conclusion": "failure" if harnesses is None or counts["failed"] or counts["unavailable"] else "success", @@ -139,7 +142,7 @@ def main() -> int: setup_url = f"{os.environ['GITHUB_SERVER_URL']}/{repo}/blob/{os.environ['GITHUB_SHA']}/scripts/harness_conformance/README.md#run-diagnostic-probes" output["summary"] += f" [Local setup instructions]({setup_url})." with Path(os.environ["GITHUB_STEP_SUMMARY"]).open("a") as handle: - handle.write(f"## Harness conformance P0\n\n{output['title']}\n\n{output['summary']}\n") + handle.write(f"## {CHECK_NAME}\n\n{output['title']}\n\n{output['summary']}\n") # Publish the detailed check before making the workflow job reflect its result. subprocess.run( ["gh", "api", "--method", "POST", f"repos/{repo}/check-runs", "--input", "-"], @@ -151,7 +154,7 @@ def main() -> int: probe_result = os.environ.get("PROBE_JOB_RESULT", "success") if payload["conclusion"] != "success" or probe_result not in ("success", "skipped"): print( - f"::error title=Harness conformance P0::{output['title']}. " + f"::error title={CHECK_NAME}::{output['title']}. " f"Probe jobs: {probe_result}. See the job summary for failures, rerun commands, and artifact links." ) return 1 diff --git a/scripts/harness_conformance/ci.py b/scripts/harness_conformance/ci.py index 5b31ddb962c..31865883079 100644 --- a/scripts/harness_conformance/ci.py +++ b/scripts/harness_conformance/ci.py @@ -18,17 +18,60 @@ def select_harnesses(paths: list[str]) -> list[str]: - """Narrow adapter-only changes; conservatively cover shared/dynamic dependencies.""" + """Select adapters and shared probe dependencies, skipping unrelated components.""" selected = set() for path in paths: adapter = next((name for name in HARNESSES if path.startswith(f"responses_api_agents/{name}_agent/")), None) if adapter: selected.add(adapter) - elif path.startswith(("fern/", "docs/")) or ("/" not in path and path.endswith(".md")) or path == "LICENSE": + elif path.startswith(("nemo_gym/cli/", "nemo_gym/tasks/")) and path not in ( + "nemo_gym/cli/__init__.py", + "nemo_gym/cli/_compat.py", + "nemo_gym/cli/utils.py", + ): + # Probes construct their own rows and call the collector directly. + # Core modules still import the shared CLI compatibility helpers. + continue + elif path.startswith( + ( + "nemo_gym/", + "responses_api_agents/shared/", + "environment_servers/legacy_agent/", + "environment_servers/single_agent_turn/", + "environment_servers/single_agent_turn_legacy/", + "scripts/harness_conformance/", + "tests/unit_tests/harness_capabilities/", + "tests/unit_tests/test_harness_conformance", + ) + ): + return list(HARNESSES) + elif path in ( + "tests/__init__.py", + "tests/conftest.py", + "tests/unit_tests/__init__.py", + "tests/unit_tests/conftest.py", + ): + # Checker tests inherit these fixtures and import through these packages. + return list(HARNESSES) + elif ( + path.startswith( + ( + "resources_servers/", + "responses_api_agents/", + "responses_api_models/", + "environment_servers/", + "tests/", + "benchmarks/", + "fern/", + "docs/", + ) + ) + or ("/" not in path and path.endswith(".md")) + or path in ("LICENSE", ".gitignore") + ): continue else: - # Includes other servers: adapters/tests can import them dynamically. - # Unknown code and dependency/configuration changes must not be skipped. + # Keep unknown code, dependency files, and CI setup conservative. return list(HARNESSES) return [name for name in HARNESSES if name in selected] diff --git a/tests/unit_tests/test_ci_environment.py b/tests/unit_tests/test_ci_environment.py index f248835709a..40fb6634bb5 100644 --- a/tests/unit_tests/test_ci_environment.py +++ b/tests/unit_tests/test_ci_environment.py @@ -249,8 +249,11 @@ def test_cicd_main_wires_preflight_cpu_and_gpu_workflows() -> None: assert "uses: ./.github/workflows/unit-tests.yml" in workflow jobs = _cicd_main_jobs() base_ref = "${{ needs.pre-flight.outputs.base_ref }}" - for job in ("unit_tests", "harness_conformance"): - assert jobs[job]["with"]["base-ref"] == base_ref + assert jobs["unit_tests"]["with"]["base-ref"] == base_ref + assert jobs["harness_conformance"]["with"]["base-ref"] == ( + "${{ github.event.merge_group.base_sha || " + "(github.ref == 'refs/heads/main' && github.event.before) || needs.pre-flight.outputs.base_ref }}" + ) assert [ step["with"]["base-ref"] for step in jobs["classify_changes"]["steps"] diff --git a/tests/unit_tests/test_harness_conformance_checks.py b/tests/unit_tests/test_harness_conformance_checks.py index b3afbfbc23b..9cce0821b3e 100644 --- a/tests/unit_tests/test_harness_conformance_checks.py +++ b/tests/unit_tests/test_harness_conformance_checks.py @@ -43,7 +43,7 @@ def test_mixed_probe_results_fail_p0_with_per_harness_explanation(tmp_path, monk for harness, code in (("pi", 0), ("codex", 1), ("hermes", 2)): write_probe_result(tmp_path, harness, code) check = checks.p0_check(["pi", "codex", "hermes", "opencode"], artifacts=tmp_path, sha="tested-sha") - assert check["name"] == "Harness conformance P0" + assert check["name"] == "Harness conformance (non-blocking)" assert check["head_sha"] == "tested-sha" assert check["conclusion"] == "failure" assert check["output"]["title"] == "1 passed, 1 failed, 2 unavailable" @@ -136,7 +136,7 @@ def artifact_listing(*args, **kwargs): assert posted[0]["external_id"] == "harness-conformance-p0-123-2" if not listing_failure: assert "https://github.com/example/Gym/actions/runs/123/artifacts/456" in posted[0]["output"]["summary"] - assert "Harness conformance P0" in (tmp_path / "summary.md").read_text() + assert "Harness conformance (non-blocking)" in (tmp_path / "summary.md").read_text() def test_actionable_summary_counts_scenarios_and_includes_rerun(tmp_path, monkeypatch): diff --git a/tests/unit_tests/test_harness_conformance_ci.py b/tests/unit_tests/test_harness_conformance_ci.py index bfe35cdc9bc..009ccd2a86e 100644 --- a/tests/unit_tests/test_harness_conformance_ci.py +++ b/tests/unit_tests/test_harness_conformance_ci.py @@ -22,13 +22,27 @@ def test_adapter_changes_select_only_that_harness(harness): "path", [ "nemo_gym/rollout_collection.py", + "nemo_gym/telemetry/gym_metrics.py", + "nemo_gym/agent_utils/sandbox_session.py", + "nemo_gym/sandbox/local.py", + "nemo_gym/cli/__init__.py", + "nemo_gym/cli/_compat.py", + "nemo_gym/cli/utils.py", "nemo_gym/prompts/system.md", "responses_api_agents/shared/prompts.md", "environment_servers/legacy_agent/app.py", - "responses_api_agents/opencode_sandboxed_agent/app.py", - "resources_servers/example_mcp_weather/app.py", + "environment_servers/single_agent_turn/app.py", + "environment_servers/single_agent_turn_legacy/app.py", "scripts/harness_conformance/registry.py", + "scripts/run_harness_conformance.py", + "scripts/inspect_harness_conformance.py", + "scripts/ci/setup_dev.sh", + "tests/__init__.py", + "tests/conftest.py", + "tests/unit_tests/__init__.py", + "tests/unit_tests/conftest.py", "tests/unit_tests/harness_capabilities/test_checker.py", + "tests/unit_tests/test_harness_conformance_checks.py", "pyproject.toml", "uv.lock", ".python-version", @@ -37,7 +51,33 @@ def test_adapter_changes_select_only_that_harness(harness): ], ) def test_shared_or_unknown_dependencies_run_all(path): + assert select_harnesses([path]) == list(HARNESSES) assert select_harnesses(["responses_api_agents/pi_agent/app.py", path]) == list(HARNESSES) + assert select_harnesses([path, "responses_api_agents/pi_agent/app.py"]) == list(HARNESSES) + + +@pytest.mark.parametrize( + "paths", + [ + ["resources_servers/ether0/.gitignore", "tests/unit_tests/test_global_config.py"], + ["resources_servers/sav_tracks/app.py", "resources_servers/sav_tracks/README.md"], + ["resources_servers/aviary/hypotest_app.py", "responses_api_agents/aviary_agent/app.py"], + [ + ".gitignore", + "nemo_gym/cli/main.py", + "nemo_gym/tasks/harbor/hub.py", + "tests/unit_tests/test_harbor_tasks.py", + ], + ["responses_api_agents/opencode_sandboxed_agent/app.py"], + ["responses_api_models/vllm_model/app.py"], + ["environment_servers/unrelated/app.py"], + ["benchmarks/example/config.yaml"], + ], +) +def test_unrelated_components_do_not_expand_adapter_selection(paths): + assert select_harnesses(paths) == [] + assert select_harnesses([*paths, "responses_api_agents/pi_agent/app.py"]) == ["pi"] + assert select_harnesses(["responses_api_agents/pi_agent/app.py", *paths]) == ["pi"] def test_documentation_only_and_multiple_adapters(): @@ -128,6 +168,7 @@ def test_custom_check_is_separate_from_required_ci_and_probe_permissions(): main = yaml.safe_load((root / ".github/workflows/cicd-main.yml").read_text())["jobs"] workflow = yaml.safe_load((root / ".github/workflows/harness-conformance.yml").read_text()) assert main["harness_conformance"]["needs"] == ["pre-flight"] + assert main["harness_conformance"]["name"] == "Harness conformance (non-blocking)" assert "harness_conformance" not in main["Nemo_CICD_Test"]["needs"] assert "harness_conformance" not in main["notify-failure"]["needs"] assert workflow["permissions"] == {"contents": "read"} @@ -141,4 +182,5 @@ def test_custom_check_is_separate_from_required_ci_and_probe_permissions(): == {"contents": "read", "actions": "read", "checks": "write"} ) assert workflow["jobs"]["report"]["if"] == "always()" + assert workflow["jobs"]["report"]["name"] == "Publish check" assert set(workflow["jobs"]["report"]["needs"]) == {"select", "probe"}