From 73557622b70d7c4c394a290602a35ae5792557a2 Mon Sep 17 00:00:00 2001 From: charles chen Date: Sun, 4 Oct 2026 09:25:55 +0300 Subject: [PATCH] feat: build code --check runs the commands itself and fails the run when one fails; release 0.52.0 Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 10 +++ docs/adopting.md | 13 +++- docs/interface.md | 10 +++ medharness/cli/build.py | 14 ++-- medharness/results.py | 7 ++ medharness/services/checks.py | 24 ++++++ medharness/services/cr_generation.py | 45 +++++++++++- medharness/services/prompt_assembly.py | 19 ++++- pyproject.toml | 2 +- tests/fixtures/fake_claude.py | 1 + .../integration/test_ai_stages_end_to_end.py | 73 +++++++++++++++++++ uv.lock | 2 +- 12 files changed, 205 insertions(+), 15 deletions(-) create mode 100644 medharness/services/checks.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ba5a2f59..314d7c19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,16 @@ MedHarness follows [Semantic Versioning](https://semver.org/): ## [Unreleased] +## [0.52.0] — 2026-10-04 + +### Added + +- **`build code --check CMD`** (repeatable): commands the code must pass. The model is told which + they are and to run them; when it finishes, MedHarness runs them itself, sends a failure back to + the model for a fix (two attempts), and fails the run if one still exits non-zero. `--pr` pushes + nothing in that case. A command that cannot run (exit 127) is a failure, not a guess. The + answer carries `checks` (`command`, `exit_code`, `passed`). + ## [0.51.0] — 2026-10-03 ### Added diff --git a/docs/adopting.md b/docs/adopting.md index f4d53288..f56d88bd 100644 --- a/docs/adopting.md +++ b/docs/adopting.md @@ -337,7 +337,7 @@ jobs: - if: inputs.stage == 'plan' run: medharness build plan --cr "${{ inputs.cr }}" - if: inputs.stage == 'code' - run: medharness build code --cr "${{ inputs.cr }}" + run: medharness build code --cr "${{ inputs.cr }}" --check "pnpm typecheck" --check "pnpm test" - run: | # without --pr the files changed and nothing was committed git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" @@ -361,10 +361,19 @@ jobs: git config user.email "github-actions[bot]@users.noreply.github.com" case "${BRANCH%%/*}" in design) medharness build plan --cr "${BRANCH#*/}" --pr "$PR" ;; - develop) medharness build code --cr "${BRANCH#*/}" --pr "$PR" ;; + develop) medharness build code --cr "${BRANCH#*/}" --pr "$PR" --check "pnpm typecheck" --check "pnpm test" ;; esac ``` +**Make `build code` pass your tests.** `--check CMD` (repeatable) names the commands the +code must pass. The model is told to run them and fix until they exit 0; when it is done, +MedHarness runs them itself and sends a failure back for a fix (two attempts). If one still +fails, the run exits non-zero, `checks` in the answer says which, and `--pr` pushes nothing. +A command that cannot run counts as failed (exit 127), so the job needs the project's +toolchain installed: without it the model can only guess, and the run fails instead of +shipping the guess. Without `--check`, the model finds the commands in `AGENTS.md` or +`CLAUDE.md` and nothing enforces them. + **Check out full history** (`fetch-depth: 0`) wherever a job runs `build plan|code`: they compare the branch with where it left `origin/main`, and a shallow clone cannot reach that point (the run stops with git's own reason). A `pull_request_review` run diff --git a/docs/interface.md b/docs/interface.md index ec58436c..9766e746 100644 --- a/docs/interface.md +++ b/docs/interface.md @@ -271,6 +271,7 @@ The fields of what each command above answers with, generated from the models in | `stage` | `develop` | Always `develop` for `build code` | | `artifacts` | `CodeArtifacts` | What the run changed | | `code_review` | `Review` or null | The code review rounds, when one ran | +| `checks` | list of `CheckResult` or null | Each `--check` as the harness last ran it; absent without `--check` | `CodeArtifacts`: @@ -278,6 +279,14 @@ The fields of what each command above answers with, generated from the models in |---|---|---| | `files_changed` | `ChangeSet` or null | The code changed on the branch; null when the diff could not be read | +`CheckResult`: + +| Field | Type | Meaning | +|---|---|---| +| `command` | string | The `--check` command, as given | +| `exit_code` | integer | What it exited with in the last run; 124 when it timed out | +| `passed` | boolean | Whether it exited 0 | + `Timing`, `Inputs`, `Progress`, `Step`, `RunWarning`, `RunError`, `Review`, `ChangeSet`, `ReviewCycle` are as listed above. #### `build soup` @@ -548,6 +557,7 @@ Write the code and tests for a CR's approved design, with a model. |---|---| | `--cr CR_ID` | The CR whose approved design to implement. [required] | | `--pr N` | The PR this run belongs to, in CI: revise if a reviewer asked for changes, then commit and push the result to its branch. Without it, local files change and nothing is committed. | +| `--check CMD` | A command that must exit 0 (typecheck, tests). The model is told to pass it, the harness runs it when the model is done and sends a failure back for a fix, and a run whose check still fails exits non-zero and pushes nothing. Repeatable. | | `--prompt` | Print the stage's instructions, with this CR's DHF context, for an agent that is already running, instead of starting a model. | #### `medharness build release` diff --git a/medharness/cli/build.py b/medharness/cli/build.py index d5b8beaf..6e0e45be 100644 --- a/medharness/cli/build.py +++ b/medharness/cli/build.py @@ -56,10 +56,10 @@ def _raise_for_outcome_error(result: dict) -> None: raise click.exceptions.Exit(1) -def _print_prompt(assemble, cr_id: str, dhf: Path) -> None: +def _print_prompt(assemble, cr_id: str, dhf: Path, **options) -> None: """Markdown, not JSON: the reader is an agent following it, not a parser.""" warnings: list[dict] = [] - click.echo(assemble(cr_id, dhf_path=dhf, warnings=warnings)) + click.echo(assemble(cr_id, dhf_path=dhf, warnings=warnings, **options)) for w in warnings: click.echo(f"WARN [prompt] {w.get('message', w)}", err=True) @@ -138,12 +138,16 @@ def change_plan(ctx: click.Context, cr_id: str, pr_number: int | None, help="The PR this run belongs to, in CI: revise if a reviewer asked for changes, then " "commit and push the result to its branch. Without it, local files " "change and nothing is committed.") + @click.option("--check", "checks", multiple=True, metavar="CMD", + help="A command that must exit 0 (typecheck, tests). The model is told to pass it, " + "the harness runs it when the model is done and sends a failure back for a fix, and " + "a run whose check still fails exits non-zero and pushes nothing. Repeatable.") @click.option("--prompt", "print_prompt", is_flag=True, default=False, help="Print the stage's instructions, with this CR's DHF context, for an " "agent that is already running, instead of starting a model.") @click.pass_context def change_implement(ctx: click.Context, cr_id: str, pr_number: int | None, - print_prompt: bool) -> None: + checks: tuple[str, ...], print_prompt: bool) -> None: """Write the code and tests for a CR's approved design, with a model. Reads the CR's implementation_notes and the items it affects, and @@ -168,9 +172,9 @@ def change_implement(ctx: click.Context, cr_id: str, pr_number: int | None, pass # DHF config not loadable yet; generate_code will surface the error if print_prompt: from medharness.services.prompt_assembly import _assemble_develop_prompt - _print_prompt(_assemble_develop_prompt, cr_id, dhf) + _print_prompt(_assemble_develop_prompt, cr_id, dhf, checks=checks) return - result = generate_code(cr_id, dhf, pr_number=pr_number) + result = generate_code(cr_id, dhf, pr_number=pr_number, checks=checks) emit(result) click.echo(_format_summary("Implementation", "revised" if pr_number else "generated", cr_id, result), err=True) for error in result.get("errors") or []: diff --git a/medharness/results.py b/medharness/results.py index d34cc3a4..153321fb 100644 --- a/medharness/results.py +++ b/medharness/results.py @@ -132,12 +132,19 @@ class PlanReport(StageReport): design_review: Review | None = Field(default=None, description="The design review rounds, when one ran") +class CheckResult(Shape): + command: str = Field(description="The `--check` command, as given") + exit_code: int = Field(description="What it exited with in the last run; 124 when it timed out") + passed: bool = Field(description="Whether it exited 0") + + class CodeReport(StageReport): """`build code`.""" stage: Literal["develop"] = Field(description="Always `develop` for `build code`") artifacts: CodeArtifacts = Field(description="What the run changed") code_review: Review | None = Field(default=None, description="The code review rounds, when one ran") + checks: list[CheckResult] | None = Field(default=None, description="Each `--check` as the harness last ran it; absent without `--check`") # ── build soup / build release / init ──────────────────────────────────────── diff --git a/medharness/services/checks.py b/medharness/services/checks.py new file mode 100644 index 00000000..f1d28462 --- /dev/null +++ b/medharness/services/checks.py @@ -0,0 +1,24 @@ +"""Commands a run must pass, run by the harness and not left to the model.""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +TIMEOUT_SECONDS = 1800 +_OUTPUT_TAIL = 4000 + + +def run_checks(repo_root: Path, commands: tuple[str, ...]) -> list[dict]: + """Run each command from ``repo_root``; ``exit_code`` is 124 on a timeout.""" + results = [] + for command in commands: + try: + done = subprocess.run(command, shell=True, cwd=str(repo_root), check=False, capture_output=True, + text=True, timeout=TIMEOUT_SECONDS) + code, output = done.returncode, (done.stdout or "") + (done.stderr or "") + except subprocess.TimeoutExpired: + code, output = 124, f"Timed out after {TIMEOUT_SECONDS} s." + results.append({"command": command, "exit_code": code, "passed": code == 0, + "output": output.strip()[-_OUTPUT_TAIL:]}) + return results diff --git a/medharness/services/cr_generation.py b/medharness/services/cr_generation.py index 3b4d445e..f6bcb608 100644 --- a/medharness/services/cr_generation.py +++ b/medharness/services/cr_generation.py @@ -13,6 +13,7 @@ from datetime import datetime, timezone from pathlib import Path +from medharness.services import checks as check_runner from medharness.services import design_validation, git from medharness.services.cr_impact import _record_design_impact_in_cr from medharness.services.github_session import get_session, put_session @@ -25,6 +26,7 @@ _assemble_review_code_prompt, _assemble_review_design_prompt, _enrich_with_plan_context, + checks_section, ) __all__ = [ @@ -50,6 +52,7 @@ def _code_paths(dhf_path: Path, repo_root: Path) -> tuple[str, ...]: _MAX_DESIGN_REVIEW_CYCLES = 3 _MAX_CODE_REVIEW_CYCLES = 3 +_MAX_CHECK_FIX_CYCLES = 2 # ── Multi-provider LLM config ───────────────────────────────────────────────── @@ -1006,6 +1009,7 @@ def generate_code( cr_id: str, dhf_path: Path, pr_number: int | None = None, + checks: tuple[str, ...] = (), ) -> dict: """Generate or revise implementation code for a CR.""" started_at = _now_iso() @@ -1083,14 +1087,14 @@ def generate_code( f"Read the implementation on this branch related to {cr_id}, " f"then revise it based on the following pull request review feedback.\n\n" f"Review feedback:\n{feedback['prompt_text']}" - ) + ) + checks_section(checks) steps.append(_finish_step(prompt_step, prompt_perf, "ok")) else: prompt_step, prompt_perf = _begin_step( "prepare_prompt", {"prompt_kind": "develop_generation", "used_pr_feedback": False}, ) - prompt = _assemble_develop_prompt(cr_id, dhf_path=dhf_path, warnings=warnings) + prompt = _assemble_develop_prompt(cr_id, dhf_path=dhf_path, warnings=warnings, checks=checks) diff = git.compute_diff(repo_root, "origin/main", *_code_paths(dhf_path, repo_root)) if diff: truncated = len(diff) > MAX_DIFF_CHARS @@ -1168,6 +1172,32 @@ def generate_code( diagnostics["code_review_verdict"] = code_review_verdict diagnostics["code_review_cycles"] = review_cycle + check_results: list[dict] = [] + for attempt in range(_MAX_CHECK_FIX_CYCLES + 1): + if not checks: + break + check_step, check_perf = _begin_step("run_checks", {"attempt": attempt + 1}) + check_results = check_runner.run_checks(repo_root, checks) + failed = [c for c in check_results if not c["passed"]] + steps.append(_finish_step(check_step, check_perf, "failed" if failed else "ok", + {"failed": [c["command"] for c in failed]})) + if not failed or attempt == _MAX_CHECK_FIX_CYCLES: + break + report = "\n\n".join(f"`{c['command']}` exited {c['exit_code']}:\n{c['output']}" for c in failed) + _, _, fix_session_id = _run_claude_step( + name=f"run_check_fix_{attempt + 1}", + prompt=f"The checks for {cr_id} failed when the harness ran them. Fix the code so each exits 0; " + f"do not change the checks.\n\n{report}", + steps=steps, + warnings=warnings, + critical=False, + resume_session=session_id, + llm_config=develop_llm, + ) + if fix_session_id: + session_id = fix_session_id + diagnostics["session_id"] = session_id + if session_id and pr_number: put_session(pr_number, session_id) @@ -1180,8 +1210,13 @@ def generate_code( warnings.append(_warning("diff_unavailable", f"affected_items not updated: {unreadable[-1]}")) else: _record_design_impact_in_cr(cr_id, dhf_path, items_changed) - errors: list[dict] = [] - if pr_number: + errors: list[dict] = [ + {"field": "check", "issue": f"`{c['command']}` exited {c['exit_code']} after {_MAX_CHECK_FIX_CYCLES} " + f"fix attempts:\n{c['output'][-1000:]}", + "fix": "Make the check pass in the working tree, then run `build code` again; nothing was pushed."} + for c in check_results if not c["passed"] + ] + if pr_number and not errors: _push_to_pr(repo_root, pr_number, f"feat({cr_id}): build code", errors) artifact_step, artifact_perf = _begin_step("collect_artifacts", {"kind": "files_changed"}) try: @@ -1210,6 +1245,8 @@ def generate_code( critical_step_failed=critical_step_failed, ) result["code_review"] = _build_review_result("Implementation", code_review_log) + if checks: + result["checks"] = [{k: c[k] for k in ("command", "exit_code", "passed")} for c in check_results] if pr_number: token = os.environ.get("GH_TOKEN") or os.environ.get("GITHUB_TOKEN") or "" result["pr_comments"] = _auto_post_pr_feedback(pr_number, cr_id, result, token=token) diff --git a/medharness/services/prompt_assembly.py b/medharness/services/prompt_assembly.py index a79ab058..cf145fdb 100644 --- a/medharness/services/prompt_assembly.py +++ b/medharness/services/prompt_assembly.py @@ -225,13 +225,28 @@ def _enrich_with_plan_context(prompt: str, cr_id: str, dhf_path: Path, dhf_path, warnings) +def checks_section(checks: tuple[str, ...]) -> str: + """Tells the model which commands the harness will run, and that they must pass.""" + if not checks: + return "" + listed = "".join(f"- `{command}`\n" for command in checks) + return ( + "\n\n## Checks That Must Pass\n\n" + "When you finish, the harness runs these commands itself. The run fails if any exits non-zero, " + "and nothing is pushed. Use them as the build, typecheck and test commands for step 4, run them " + "yourself first, and fix the code until each exits 0. Do not edit, skip or work around a check; " + "if one cannot run here, say why in `implementation_notes`.\n\n" + listed + ) + + def _assemble_develop_prompt(cr_id: str, dhf_path: Path | None = None, - warnings: list[str] | None = None) -> str: + warnings: list[str] | None = None, + checks: tuple[str, ...] = ()) -> str: prompt = _load_prompt("cr_develop.md").replace("{{cr_id}}", cr_id) if dhf_path is not None: prompt = _enrich(prompt, lambda p: _render_code_context(_cr_context(p, cr_id)), dhf_path, warnings) - return prompt + return prompt + checks_section(checks) def _assemble_review_code_prompt(cr_id: str) -> str: diff --git a/pyproject.toml b/pyproject.toml index 429ddf06..f566c83c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "medharness" -version = "0.51.0" +version = "0.52.0" description = "Open-source design-controlled development infrastructure for medical device and SaMD teams — includes dhfkit" readme = "README.md" license = { text = "MIT" } diff --git a/tests/fixtures/fake_claude.py b/tests/fixtures/fake_claude.py index 5ec293d9..5f4a5561 100644 --- a/tests/fixtures/fake_claude.py +++ b/tests/fixtures/fake_claude.py @@ -22,6 +22,7 @@ else "code_review" if "CR Code Review" in prompt else "develop" if "CR Implementation Task" in prompt else "code_fix" if prompt.startswith("The code review for") + else "check_fix" if prompt.startswith("The checks for") else "design_fix" if prompt.startswith("The design review for") else "design" ) diff --git a/tests/integration/test_ai_stages_end_to_end.py b/tests/integration/test_ai_stages_end_to_end.py index 6ff6e2b4..2953a5b2 100644 --- a/tests/integration/test_ai_stages_end_to_end.py +++ b/tests/integration/test_ai_stages_end_to_end.py @@ -468,6 +468,79 @@ def _remote_files(project: Path, branch: str) -> list[str]: capture_output=True, text=True, check=True).stdout.split() +class TestBuildCodeChecks: + """`--check` is run by the harness, so the model cannot skip it or report a guess.""" + + APPROVED = [{"say": "**Verdict:** Approved"}] + + def _code(self, project: Path, tmp_path: Path, *extra: str, **stages) -> object: + _design_merged(project) + _plan(tmp_path, develop=[{"run": CODE}], code_review=self.APPROVED, **stages) + return _run("build", "code", "--cr", "CR-001", *extra) + + def test_the_model_is_told_which_commands_the_harness_will_run(self, project: Path, tmp_path: Path) -> None: + self._code(project, tmp_path, "--check", "true") + develop = next(c for c in _calls(project) if c["stage"] == "develop") + assert "## Checks That Must Pass" in develop["prompt"] and "- `true`" in develop["prompt"] + + def test_a_passing_check_is_in_the_report(self, project: Path, tmp_path: Path) -> None: + result = self._code(project, tmp_path, "--check", "true") + assert result.exit_code == 0, result.stderr + assert _report(result)["checks"] == [{"command": "true", "exit_code": 0, "passed": True}] + assert "check_fix" not in [c["stage"] for c in _calls(project)] + + def test_a_run_without_check_reports_none(self, project: Path, tmp_path: Path) -> None: + assert "checks" not in _report(self._code(project, tmp_path)) + develop = next(c for c in _calls(project) if c["stage"] == "develop") + assert "Checks That Must Pass" not in develop["prompt"] + + def test_a_failure_goes_back_to_the_model_and_the_run_passes_once_it_is_fixed( + self, project: Path, tmp_path: Path, + ) -> None: + result = self._code(project, tmp_path, "--check", "test -f src/fixed.txt", + check_fix=[{"run": ["touch src/fixed.txt"]}]) + assert result.exit_code == 0, result.stderr + assert [c["stage"] for c in _calls(project)] == ["develop", "code_review", "check_fix"] + fix = next(c for c in _calls(project) if c["stage"] == "check_fix") + assert "`test -f src/fixed.txt` exited 1" in fix["prompt"] + assert _report(result)["checks"][0]["passed"] is True + + def test_a_check_that_never_passes_fails_the_run_after_two_fix_attempts( + self, project: Path, tmp_path: Path, + ) -> None: + result = self._code(project, tmp_path, "--check", "echo broken >&2; exit 3", check_fix=[{"run": []}]) + report = _report(result) + assert result.exit_code == 1 + assert [c["stage"] for c in _calls(project)].count("check_fix") == 2 + assert report["checks"] == [{"command": "echo broken >&2; exit 3", "exit_code": 3, "passed": False}] + error = next(e for e in report["errors"] if e["field"] == "check") + assert "exited 3" in error["issue"] and "broken" in error["issue"] + + def test_a_command_that_cannot_run_is_a_failure_not_a_guess(self, project: Path, tmp_path: Path) -> None: + result = self._code(project, tmp_path, "--check", "no-such-tool-for-this-test", check_fix=[{"run": []}]) + assert result.exit_code == 1 + assert _report(result)["checks"][0]["exit_code"] == 127 + + def test_every_check_must_pass(self, project: Path, tmp_path: Path) -> None: + result = self._code(project, tmp_path, "--check", "true", "--check", "false", check_fix=[{"run": []}]) + assert result.exit_code == 1 + assert [c["passed"] for c in _report(result)["checks"]] == [True, False] + + def test_the_prompt_for_an_agent_already_running_lists_them_too(self, project: Path, tmp_path: Path) -> None: + _design_merged(project) + result = _run("build", "code", "--cr", "CR-001", "--check", "pnpm test", "--prompt") + assert "- `pnpm test`" in result.stdout + + def test_a_pull_request_gets_nothing_pushed_while_a_check_fails( + self, project: Path, tmp_path: Path, pr, + ) -> None: + before = _remote_files(project, "design/CR-001") + result = self._code(project, tmp_path, "--check", "false", "--pr", "7", check_fix=[{"run": []}]) + assert result.exit_code == 1 + assert not any(e["field"] == "pr_push" for e in _report(result)["errors"]) + assert _remote_files(project, "design/CR-001") == before + + class TestWithAPullRequest: def test_the_work_is_committed_and_pushed_to_the_prs_branch( self, project: Path, tmp_path: Path, pr, diff --git a/uv.lock b/uv.lock index 07acee7b..ac8ade01 100644 --- a/uv.lock +++ b/uv.lock @@ -508,7 +508,7 @@ wheels = [ [[package]] name = "medharness" -version = "0.51.0" +version = "0.52.0" source = { editable = "." } dependencies = [ { name = "click" },