From 4a6ca578395525d56ea56114e061c641c32d46d3 Mon Sep 17 00:00:00 2001 From: Sollan Systems Date: Sun, 5 Jul 2026 18:27:37 -0400 Subject: [PATCH 1/2] fix(action): install schema extras on the gate surfaces so strict validation runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GitHub Action installed the bare package (no [schemas] extra) on both install paths and the pre-commit loop-doctor hook declared no deps, so both gate surfaces ran loop/contract.py's pure-stdlib structural fallback — a type-invalid contract passed the shipped gates. The CLI's structural default is deliberate and unchanged; strictness now lands by-install on the gates only. - action.yml: both paths install "loop-engineer[schemas,yaml]" (PyPI-pinned and action-checkout). Local-path-with-extras syntax proven in a scratch venv. - action.yml doctor step asserts the report's validation_mode is "jsonschema" so a future packaging regression that drops the extras fails loudly. - .pre-commit-hooks.yaml: additional_dependencies jsonschema>=4, pyyaml>=6. Co-Authored-By: Claude Fable 5 --- .pre-commit-hooks.yaml | 4 ++ action.yml | 21 ++++++++-- scripts/test_action_scorecard.py | 71 ++++++++++++++++++++++++++++++++ scripts/test_precommit_hook.py | 16 +++++++ 4 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 scripts/test_action_scorecard.py diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index a992d6b..fc1e0e8 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -3,5 +3,9 @@ description: "Validate the repo's .loop/ contract objects; fails on a dishonest or malformed contract." entry: loop doctor . language: python + # The CLI default is pure-stdlib structural validation; the gate surface runs + # the strict path. pre-commit installs these into the hook's isolated env so + # loop/contract.py takes the jsonschema branch and PyYAML parses the manifest. + additional_dependencies: ["jsonschema>=4", "pyyaml>=6"] pass_filenames: false always_run: true diff --git a/action.yml b/action.yml index 11de31e..9e92dea 100644 --- a/action.yml +++ b/action.yml @@ -38,17 +38,32 @@ runs: env: LOOP_VERSION: "${{ inputs.version }}" run: | + # Install WITH the schema extras on both paths: the gate surface must run + # real JSON-Schema validation, not the CLI's pure-stdlib structural default. if [ -n "$LOOP_VERSION" ]; then - python -m pip install --quiet "loop-engineer==$LOOP_VERSION" + python -m pip install --quiet "loop-engineer[schemas,yaml]==$LOOP_VERSION" else - python -m pip install --quiet "${{ github.action_path }}" + python -m pip install --quiet "${{ github.action_path }}[schemas,yaml]" fi - name: loop doctor (hard gate) shell: bash env: LOOP_PATH: "${{ inputs.path }}" - run: loop doctor "$LOOP_PATH" + run: | + # -eo pipefail (GitHub's bash default) makes a doctor failure fail the step + # despite the tee. Then assert the strict validation path actually ran, so a + # future packaging regression that drops the extras fails loudly here. The + # check reads one fixed field from doctor's own JSON — no fragile parsing. + loop doctor "$LOOP_PATH" | tee "${RUNNER_TEMP}/doctor.json" + python - "${RUNNER_TEMP}/doctor.json" <<'PY' + import json, sys + mode = json.load(open(sys.argv[1])).get("validation_mode") + if mode != "jsonschema": + print(f"::error::loop doctor ran in {mode!r} mode, not 'jsonschema' — " + "the gate is installed without its [schemas] extra") + raise SystemExit(1) + PY - name: loop inspect (scorecard) shell: bash diff --git a/scripts/test_action_scorecard.py b/scripts/test_action_scorecard.py new file mode 100644 index 0000000..708a262 --- /dev/null +++ b/scripts/test_action_scorecard.py @@ -0,0 +1,71 @@ +# scripts/test_action_scorecard.py +"""Gate-strictness acceptance for the shipped GitHub Action. + +F3 — the composite action must install loop-engineer WITH its schema extras on +both install paths, so `loop doctor` runs real JSON-Schema validation (not the +pure-stdlib structural fallback) and asserts as much. F8 — the scorecard logic +is extracted into scripts/action_scorecard.py so it validates fail-under as an +integer instead of tracebacking on non-numeric input, and the PR comment is +sticky (edit-in-place, never a fresh comment per run).""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent +ACTION_YML = REPO_ROOT / "action.yml" + + +def _action() -> dict: + yaml = pytest.importorskip("yaml") + return yaml.safe_load(ACTION_YML.read_text(encoding="utf-8")) + + +def _steps() -> list[dict]: + return _action()["runs"]["steps"] + + +def _step(name_fragment: str) -> dict: + for step in _steps(): + if name_fragment.lower() in str(step.get("name", "")).lower(): + return step + raise AssertionError(f"no action step whose name contains {name_fragment!r}") + + +def test_action_yml_is_valid_yaml(): + action = _action() + assert action["runs"]["using"] == "composite" + + +# --- F3: strict-by-install on the action gate surface ------------------------ + + +def test_both_install_paths_carry_the_schema_extras(): + run = _step("Install loop-engineer")["run"] + # PyPI-pinned path and the action-checkout path must both request the extras. + assert 'loop-engineer[schemas,yaml]==' in run, ( + "the versioned PyPI install must request the [schemas,yaml] extras" + ) + assert '}}[schemas,yaml]' in run or 'github.action_path }}[schemas,yaml]' in run, ( + "the action-checkout install must request the [schemas,yaml] extras" + ) + + +def test_bare_install_without_extras_is_gone(): + run = _step("Install loop-engineer")["run"] + assert '"loop-engineer==' not in run, "bare (extras-free) PyPI install path lingers" + # the action-path install must not appear without the extras suffix + assert '"${{ github.action_path }}"' not in run, ( + "bare (extras-free) action-checkout install path lingers" + ) + + +def test_doctor_step_asserts_jsonschema_validation_mode(): + run = _step("loop doctor")["run"] + assert "validation_mode" in run, ( + "the doctor step should assert the report's validation_mode is jsonschema " + "so a packaging regression that drops the extras fails loudly" + ) + assert "jsonschema" in run diff --git a/scripts/test_precommit_hook.py b/scripts/test_precommit_hook.py index 03533e3..c040e22 100644 --- a/scripts/test_precommit_hook.py +++ b/scripts/test_precommit_hook.py @@ -31,6 +31,22 @@ def test_hook_definition_is_sound(): assert hook["always_run"] is True +def test_hook_installs_the_schema_extras_so_it_runs_the_strict_gate(): + # The CLI's pure-stdlib structural default is deliberate, but the pre-commit + # gate must run real JSON-Schema validation — otherwise a type-invalid + # contract passes the shipped hook. pre-commit installs additional_dependencies + # into the hook's isolated env. + (hook,) = _hooks() + deps = hook.get("additional_dependencies", []) + joined = " ".join(deps).lower() + assert "jsonschema" in joined, ( + "loop-doctor hook must install jsonschema so validation runs in strict mode" + ) + assert "pyyaml" in joined or "yaml" in joined, ( + "loop-doctor hook must install pyyaml so the manifest parses via PyYAML" + ) + + def test_entry_command_matches_a_declared_console_script(): text = (REPO_ROOT / "pyproject.toml").read_text(encoding="utf-8") assert 'loop = "loop.__main__:main"' in text From 8e8b14c4cdd4b70e2b39e1a1dfc30e840380ad84 Mon Sep 17 00:00:00 2001 From: Sollan Systems Date: Sun, 5 Jul 2026 18:32:52 -0400 Subject: [PATCH 2/2] fix(action): robust extracted scorecard + sticky PR comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scorecard lived in an inline action heredoc that did int(sys.argv[2]) — a non-integer fail-under-score ("80.5"/"abc") tracebacked instead of failing cleanly — and the PR-comment step POSTed a fresh comment every run. - Extract the logic to scripts/action_scorecard.py (invoked from the action checkout at github.action_path). It validates fail-under as an integer 0-100 and emits a clear ::error with a distinct exit code (2) on bad input, separate from the fail-under-breach code (1); same summary/scorecard.md/warning/ fail-under behavior otherwise. TDD-covered in test_action_scorecard.py. - Sticky PR comment: the rendered body carries a marker; the comment step finds that comment and PATCHes it in place, else POSTs. Stays non-fatal on API failure. - Document honestly (fail-under-score input + README): the inspect score is an advisory heuristic and can be gamed; loop doctor is the hard gate. Co-Authored-By: Claude Fable 5 --- README.md | 11 ++- action.yml | 53 +++++------- scripts/action_scorecard.py | 114 ++++++++++++++++++++++++++ scripts/test_action_scorecard.py | 134 +++++++++++++++++++++++++++++++ 4 files changed, 278 insertions(+), 34 deletions(-) create mode 100644 scripts/action_scorecard.py diff --git a/README.md b/README.md index a9ec420..0926e7c 100644 --- a/README.md +++ b/README.md @@ -304,9 +304,14 @@ refuses an evidence-free `Succeeded` at write time. Recipe: path: "." ``` -`doctor` failure fails the job; the inspect verdict is warn-only unless you set -`fail-under-score`. Pre-commit users: hook id `loop-doctor`. This repo dogfoods -the same action in CI against its flagship example contract +`doctor` failure fails the job (and the action installs the `[schemas]` extra so +`doctor` runs real JSON-Schema validation, not the structural fallback). The +inspect **score is an advisory heuristic** — useful as a trend, but a determined +author can game it, so it is warn-only unless you set `fail-under-score`; +**`loop doctor` is the hard gate.** The optional PR comment is sticky — it edits +one scorecard comment in place across re-runs. Pre-commit users: hook id +`loop-doctor` (its `additional_dependencies` pin the schema extras too). This repo +dogfoods the same action in CI against its flagship example contract ([`examples/coverage-repair`](examples/coverage-repair)) — the gate gates its maker. --- diff --git a/action.yml b/action.yml index 9e92dea..535a7ae 100644 --- a/action.yml +++ b/action.yml @@ -14,7 +14,7 @@ inputs: required: false default: "" fail-under-score: - description: "Fail the job when the inspect score (0-100) is below this. 0 keeps inspect warn-only." + description: "Fail the job when the inspect score (0-100) is below this. 0 keeps inspect warn-only. NOTE: the inspect score is an advisory heuristic and a determined author can game it — loop doctor is the hard gate." required: false default: "0" python-version: @@ -71,43 +71,34 @@ runs: LOOP_PATH: "${{ inputs.path }}" LOOP_FAIL_UNDER: "${{ inputs.fail-under-score }}" run: | + # inspect is advisory; capture its JSON even on a non-zero verdict, then + # let the extracted (tested) scorecard render + apply fail-under. The + # script validates fail-under as an int instead of tracebacking on it. set +e loop inspect "$LOOP_PATH" > "${RUNNER_TEMP}/inspect.json" set -e - python - "${RUNNER_TEMP}/inspect.json" "$LOOP_FAIL_UNDER" <<'PY' - import json, os, sys - - report = json.load(open(sys.argv[1])) - fail_under = int(sys.argv[2]) - score, verdict = report.get("score", 0), report.get("verdict", "?") - lines = [ - "## loop-engineer scorecard", - "", - f"| metric | value |", - f"|---|---|", - f"| verdict | **{verdict}** |", - f"| score | {score}/100 |", - f"| gaps | {len(report.get('gaps', []))} |", - "", - ] - for gap in report.get("gaps", [])[:10]: - lines.append(f"- {gap}") - summary = "\n".join(lines) + "\n" - with open(os.environ["GITHUB_STEP_SUMMARY"], "a", encoding="utf-8") as fh: - fh.write(summary) - open(os.path.join(os.environ["RUNNER_TEMP"], "scorecard.md"), "w", encoding="utf-8").write(summary) - if verdict == "weak": - print(f"::warning::loop inspect verdict is weak (score {score}/100)") - if fail_under and score < fail_under: - print(f"::error::inspect score {score} < fail-under-score {fail_under}") - raise SystemExit(1) - PY + python "${{ github.action_path }}/scripts/action_scorecard.py" \ + "${RUNNER_TEMP}/inspect.json" "$LOOP_FAIL_UNDER" - name: PR scorecard comment (optional) if: ${{ inputs.github-token != '' && github.event_name == 'pull_request' }} shell: bash env: GH_TOKEN: ${{ inputs.github-token }} + GH_REPO: ${{ github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} run: | - gh api "repos/${GITHUB_REPOSITORY}/issues/${{ github.event.pull_request.number }}/comments" \ - -f body="$(cat "${RUNNER_TEMP}/scorecard.md")" || echo "::warning::PR comment failed (non-fatal)" + # Sticky comment: find our own marker-bearing comment and edit it in place, + # else post a fresh one — so re-runs update one comment instead of spamming. + # Non-fatal on any API failure. + set +e + body="$(cat "${RUNNER_TEMP}/scorecard.md")" + existing="$(gh api --paginate "repos/${GH_REPO}/issues/${PR_NUMBER}/comments" \ + --jq '.[] | select(.body | contains("")) | .id' | head -n1)" + if [ -n "$existing" ]; then + gh api --method PATCH "repos/${GH_REPO}/issues/comments/${existing}" \ + -f body="$body" || echo "::warning::PR comment update failed (non-fatal)" + else + gh api --method POST "repos/${GH_REPO}/issues/${PR_NUMBER}/comments" \ + -f body="$body" || echo "::warning::PR comment failed (non-fatal)" + fi diff --git a/scripts/action_scorecard.py b/scripts/action_scorecard.py new file mode 100644 index 0000000..64cdbeb --- /dev/null +++ b/scripts/action_scorecard.py @@ -0,0 +1,114 @@ +#!/usr/bin/env python3 +"""Scorecard renderer for the loop-engineer GitHub Action. + +Invoked by the composite action from its own checkout: + + python "${{ github.action_path }}/scripts/action_scorecard.py" + +The inspect score is an ADVISORY heuristic — a determined author can game it. +`loop doctor` is the hard gate; this only summarizes and optionally fails the job +when the score dips below an author-set floor. + +Exit codes: + 0 scorecard rendered (verdict may be weak → a ::warning, still a pass) + 1 the inspect score is below + 2 bad input (non-integer/out-of-range fail-under, missing/malformed inspect.json, + wrong argument count) — distinct from the fail-under-breach code +""" + +from __future__ import annotations + +import json +import os +import sys +from pathlib import Path +from typing import Any + +MARKER = "" + + +def parse_fail_under(raw: str) -> int: + """Parse the fail-under threshold as an integer in [0, 100]. + + Raises ValueError on anything else (``"80.5"``, ``"abc"``, ``""``, ``"101"``) + so the caller can emit one clear ::error instead of a raw traceback. + """ + try: + value = int(raw.strip()) + except (AttributeError, ValueError): + raise ValueError(f"fail-under-score must be an integer 0-100, got {raw!r}") + if not 0 <= value <= 100: + raise ValueError(f"fail-under-score must be in [0, 100], got {value}") + return value + + +def render_summary(report: dict[str, Any]) -> str: + """Render the Markdown scorecard. The sticky marker leads the body so the + PR-comment step can find-and-edit its own prior comment in place.""" + score = report.get("score", 0) + verdict = report.get("verdict", "?") + gaps = report.get("gaps", []) + lines = [ + MARKER, + "## loop-engineer scorecard", + "", + "| metric | value |", + "|---|---|", + f"| verdict | **{verdict}** |", + f"| score | {score}/100 |", + f"| gaps | {len(gaps)} |", + "", + ] + for gap in gaps[:10]: + lines.append(f"- {gap}") + return "\n".join(lines) + "\n" + + +def _publish(summary: str) -> None: + """Append to the job summary and drop scorecard.md for the PR-comment step. + Both destinations come from the runner env; absent (e.g. in tests) → skipped.""" + step_summary = os.environ.get("GITHUB_STEP_SUMMARY") + if step_summary: + with open(step_summary, "a", encoding="utf-8") as fh: + fh.write(summary) + runner_temp = os.environ.get("RUNNER_TEMP") + if runner_temp: + (Path(runner_temp) / "scorecard.md").write_text(summary, encoding="utf-8") + + +def main(argv: list[str] | None = None) -> int: + argv = list(sys.argv[1:] if argv is None else argv) + if len(argv) != 2: + print("::error::action_scorecard.py expects ") + return 2 + + inspect_path, fail_under_raw = argv + try: + fail_under = parse_fail_under(fail_under_raw) + except ValueError as exc: + print(f"::error::{exc}") + return 2 + + try: + report = json.loads(Path(inspect_path).read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError) as exc: + print(f"::error::could not read inspect report {inspect_path!r}: {exc}") + return 2 + if not isinstance(report, dict): + print(f"::error::inspect report {inspect_path!r} is not a JSON object") + return 2 + + score = report.get("score", 0) + verdict = report.get("verdict", "?") + _publish(render_summary(report)) + + if verdict == "weak": + print(f"::warning::loop inspect verdict is weak (score {score}/100)") + if fail_under and isinstance(score, (int, float)) and score < fail_under: + print(f"::error::inspect score {score} < fail-under-score {fail_under}") + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/test_action_scorecard.py b/scripts/test_action_scorecard.py index 708a262..c83060a 100644 --- a/scripts/test_action_scorecard.py +++ b/scripts/test_action_scorecard.py @@ -10,14 +10,26 @@ from __future__ import annotations +import json +import sys from pathlib import Path import pytest REPO_ROOT = Path(__file__).resolve().parent.parent +SCRIPTS_DIR = REPO_ROOT / "scripts" ACTION_YML = REPO_ROOT / "action.yml" +def _scorecard(): + """Import the extracted scorecard module lazily so the action-wiring tests + stay independent of it existing yet (TDD).""" + sys.path.insert(0, str(SCRIPTS_DIR)) + import action_scorecard # type: ignore + + return action_scorecard + + def _action() -> dict: yaml = pytest.importorskip("yaml") return yaml.safe_load(ACTION_YML.read_text(encoding="utf-8")) @@ -69,3 +81,125 @@ def test_doctor_step_asserts_jsonschema_validation_mode(): "so a packaging regression that drops the extras fails loudly" ) assert "jsonschema" in run + + +# --- F8: robust, tested scorecard logic -------------------------------------- + + +def test_parse_fail_under_accepts_valid_integers(): + sc = _scorecard() + assert sc.parse_fail_under("0") == 0 + assert sc.parse_fail_under("80") == 80 + assert sc.parse_fail_under("100") == 100 + + +@pytest.mark.parametrize("bad", ["80.5", "abc", "", "-1", "101", " "]) +def test_parse_fail_under_rejects_non_integers_and_out_of_range(bad): + sc = _scorecard() + with pytest.raises(ValueError): + sc.parse_fail_under(bad) + + +def test_render_summary_embeds_sticky_marker_and_scorecard_fields(): + sc = _scorecard() + out = sc.render_summary({"score": 72, "verdict": "strong", "gaps": ["g1", "g2"]}) + assert sc.MARKER in out + assert "loop-engineer scorecard" in out + assert "72/100" in out + assert "**strong**" in out + assert "- g1" in out and "- g2" in out + + +def test_render_summary_caps_gap_list_at_ten(): + sc = _scorecard() + out = sc.render_summary({"score": 0, "verdict": "weak", "gaps": [f"g{i}" for i in range(20)]}) + assert "- g9" in out + assert "- g10" not in out + + +def _run_main(sc, argv, tmp_path, monkeypatch): + monkeypatch.setenv("RUNNER_TEMP", str(tmp_path)) + monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(tmp_path / "summary.md")) + return sc.main(argv) + + +def test_main_non_numeric_fail_under_is_distinct_exit_and_clear_error(tmp_path, monkeypatch, capsys): + sc = _scorecard() + inspect = tmp_path / "inspect.json" + inspect.write_text(json.dumps({"score": 90, "verdict": "strong"}), encoding="utf-8") + rc = _run_main(sc, [str(inspect), "80.5"], tmp_path, monkeypatch) + out = capsys.readouterr().out + assert rc == 2, "bad fail-under must use a distinct exit code, not the fail-under-breach code" + assert "::error::" in out + assert "80.5" in out + + +def test_main_weak_verdict_warns_but_passes_when_fail_under_zero(tmp_path, monkeypatch, capsys): + sc = _scorecard() + inspect = tmp_path / "inspect.json" + inspect.write_text(json.dumps({"score": 40, "verdict": "weak", "gaps": []}), encoding="utf-8") + rc = _run_main(sc, [str(inspect), "0"], tmp_path, monkeypatch) + out = capsys.readouterr().out + assert rc == 0 + assert "::warning::" in out + + +def test_main_fail_under_breach_exits_one_with_error(tmp_path, monkeypatch, capsys): + sc = _scorecard() + inspect = tmp_path / "inspect.json" + inspect.write_text(json.dumps({"score": 50, "verdict": "strong", "gaps": []}), encoding="utf-8") + rc = _run_main(sc, [str(inspect), "80"], tmp_path, monkeypatch) + out = capsys.readouterr().out + assert rc == 1 + assert "::error::" in out + + +def test_main_writes_step_summary_and_scorecard_with_marker(tmp_path, monkeypatch): + sc = _scorecard() + inspect = tmp_path / "inspect.json" + inspect.write_text(json.dumps({"score": 88, "verdict": "strong", "gaps": ["x"]}), encoding="utf-8") + rc = _run_main(sc, [str(inspect), "0"], tmp_path, monkeypatch) + assert rc == 0 + scorecard_md = (tmp_path / "scorecard.md").read_text(encoding="utf-8") + summary_md = (tmp_path / "summary.md").read_text(encoding="utf-8") + assert sc.MARKER in scorecard_md + assert sc.MARKER in summary_md + assert "88/100" in scorecard_md + + +def test_main_wrong_argument_count_returns_two(tmp_path, monkeypatch, capsys): + sc = _scorecard() + rc = _run_main(sc, ["only-one-arg"], tmp_path, monkeypatch) + capsys.readouterr() + assert rc == 2 + + +def test_main_malformed_inspect_json_returns_two(tmp_path, monkeypatch, capsys): + sc = _scorecard() + inspect = tmp_path / "inspect.json" + inspect.write_text("not json", encoding="utf-8") + rc = _run_main(sc, [str(inspect), "0"], tmp_path, monkeypatch) + capsys.readouterr() + assert rc == 2 + + +# --- F8: action wiring ------------------------------------------------------- + + +def test_scorecard_step_invokes_the_extracted_script(): + run = _step("loop inspect")["run"] + assert "action_scorecard.py" in run, "the scorecard step must call the extracted script" + + +def test_no_fragile_inline_scorecard_python_remains(): + text = ACTION_YML.read_text(encoding="utf-8") + assert "int(sys.argv[2])" not in text, ( + "the fragile inline int(sys.argv[2]) scorecard heredoc must be gone" + ) + + +def test_pr_comment_step_is_sticky(): + run = _step("PR scorecard comment")["run"] + assert "loop-engineer-scorecard" in run, "the comment step must look up the marker comment" + assert "PATCH" in run, "the comment step must edit the existing marker comment in place" + assert "warning" in run.lower(), "the comment step must stay non-fatal on API failure"