From 679f0e89ae853686a77055bdc5c49d88a78d1a2f Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Mon, 6 Jul 2026 10:48:13 -0700 Subject: [PATCH 1/6] Green gate Stage A: config-driven format autofix before slice PR open When the per-slice green gate (#3398) finds a configured check red at the slice tip and the check carries an optional fix command in repositories.yaml (e.g. lint: make lint-fix), the runner executes the fix in its worktree and re-runs the check. If every failed check re-ran green, the orchestrator stages the fix from the shared hostPath worktree, commits it as egg-green-gate, and pushes it to the slice integration branch through the launcher-authed gateway push route before any close side effect, then lets the slice close. Dependent slices fork from the integration branch remote tip when they start, so they fork after the format commit. - validate_checks (shared + both fallbacks) accepts an optional fix key - runner reports a fix sub-object (re-run verdict, changed files) in the check verdict; the check ok stays false at the pushed tip - orchestrator autofix applies only in on mode; log mode logs that a fix was available (soak signal) and never commits or pushes - checks without fix, or whose re-run stays red, block exactly like Stage B Closes #3409 --- config/repo_config.py | 28 +- config/repositories.yaml.example | 8 + orchestrator/routes/pipelines/__init__.py | 14 +- orchestrator/slice_green_gate.py | 294 +++++++++++++++-- orchestrator/tests/test_slice_green_gate.py | 339 +++++++++++++++++++- shared/egg_config/validators.py | 24 +- tests/egg_config/test_validators.py | 56 ++++ 7 files changed, 720 insertions(+), 43 deletions(-) diff --git a/config/repo_config.py b/config/repo_config.py index c68b4dff96..5343a1a704 100644 --- a/config/repo_config.py +++ b/config/repo_config.py @@ -364,21 +364,27 @@ def validate_checks(checks: list[Any]) -> list[dict[str, str]]: """Validate and normalize a list of check command entries. Filters out malformed entries and coerces values to strings. + Mirrors ``egg_config.validators.validate_checks``, including + the optional ``fix`` auto-remediation command (#3409). Args: checks: Raw list of check entries (e.g. from YAML or JSON). Returns: - List of {"name": "...", "command": "..."} dicts with only - valid entries retained. + List of {"name": "...", "command": "..."} dicts (plus + "fix" when configured) with only valid entries retained. """ if not isinstance(checks, list): return [] - return [ - {"name": str(c["name"]), "command": str(c["command"])} - for c in checks - if isinstance(c, dict) and "name" in c and "command" in c - ] + result = [] + for c in checks: + if not (isinstance(c, dict) and "name" in c and "command" in c): + continue + entry = {"name": str(c["name"]), "command": str(c["command"])} + if c.get("fix"): + entry["fix"] = str(c["fix"]) + result.append(entry) + return result def reload_config() -> None: @@ -572,14 +578,16 @@ def get_repo_checks(repo: str) -> list[dict[str, str]]: These are the commands to run during the SDLC pipeline implement phase checker step. Each check has a "name" (display label) and "command" - (shell command to execute). They run sequentially. + (shell command to execute). They run sequentially. A check may also + carry an optional "fix" command — an auto-remediation the per-slice + green gate runs when the check is red at the slice tip (#3409). Args: repo: Repository in "owner/repo" format Returns: - List of {"name": "...", "command": "..."} dicts, - or empty list if no checks configured. + List of {"name": "...", "command": "..."} dicts (plus "fix" + when configured), or empty list if no checks configured. """ checks = get_repo_setting(repo, "checks", []) result: list[dict[str, str]] = validate_checks(checks) diff --git a/config/repositories.yaml.example b/config/repositories.yaml.example index ada795bac0..bd0afcc0c3 100644 --- a/config/repositories.yaml.example +++ b/config/repositories.yaml.example @@ -76,6 +76,13 @@ readable_repos: # - checks: List of check commands for the SDLC pipeline implement phase # Each entry has "name" (display label) and "command" (shell command) # These run sequentially during the checker step +# An entry may also set "fix" (shell command): when the per-slice green +# gate finds that check red at the slice tip, it runs the fix command, +# re-runs the check, and — if the fix turned it green — commits and +# pushes the result to the slice integration branch as the orchestrator +# (#3409). Only configure deterministic auto-remediations here, e.g. +# "make lint-fix" for a format/lint check. Checks without "fix" route +# red verdicts back to the slice team unchanged. # - build_commands: Commands to run during Docker image build to install # project-specific dependencies. Results are baked into the image so # containers start with dependencies pre-installed (critical for private @@ -137,6 +144,7 @@ repo_settings: # checks: # - name: lint # command: make lint + # fix: make lint-fix # optional green-gate auto-remediation (#3409) # - name: test # command: make test # some-org/external-repo: diff --git a/orchestrator/routes/pipelines/__init__.py b/orchestrator/routes/pipelines/__init__.py index f8d332d211..50b6469495 100644 --- a/orchestrator/routes/pipelines/__init__.py +++ b/orchestrator/routes/pipelines/__init__.py @@ -463,11 +463,15 @@ def get_repo_checks(repo: str) -> list[dict[str, str]]: # type: ignore[misc] def validate_checks(checks: list) -> list[dict[str, str]]: # type: ignore[misc] if not isinstance(checks, list): return [] - return [ - {"name": str(c["name"]), "command": str(c["command"])} - for c in checks - if isinstance(c, dict) and "name" in c and "command" in c - ] + result = [] + for c in checks: + if not (isinstance(c, dict) and "name" in c and "command" in c): + continue + entry = {"name": str(c["name"]), "command": str(c["command"])} + if c.get("fix"): + entry["fix"] = str(c["fix"]) + result.append(entry) + return result pipelines_bp = Blueprint("pipelines", __name__, url_prefix="/api/v1/pipelines") diff --git a/orchestrator/slice_green_gate.py b/orchestrator/slice_green_gate.py index 5bf41e1bd2..cc8cf5a87f 100644 --- a/orchestrator/slice_green_gate.py +++ b/orchestrator/slice_green_gate.py @@ -56,6 +56,21 @@ contract snapshot on the slice tip can red contract-hygiene tests for reasons unrelated to the slice's code) → ``on`` (block). +Stage A (#3409) adds config-driven auto-remediation on top of the Stage +B block: a check in ``repositories.yaml`` may carry an optional ``fix`` +command (e.g. egg's ``lint`` check gets ``fix: make lint-fix``). When +the gate finds such a check red, the runner executes the fix inside its +worktree and re-runs the check. If every failed check re-ran green, the +orchestrator stages the fix from the shared hostPath worktree, commits +it as ``egg-green-gate``, and pushes it to the slice integration branch +through the launcher-authed gateway push route before any close side +effect, then lets the slice close: the runner already re-validated the +identical tree, so no second runner pass is needed, and dependent +slices fork from the remote tip after the fix commit. In ``log`` mode +the fix still runs in the runner (soak signal) but nothing is committed +or pushed. Checks without a ``fix``, or whose re-run stays red, block +exactly like Stage B. + The check toolchain is the **repo-defined** one, not the sandbox image's: ``repositories.yaml::build_commands`` builds the repo's pinned dev environment at image build (e.g. egg's ``make sandbox-deps`` → @@ -78,6 +93,7 @@ import json import os +import subprocess import time import uuid from typing import TYPE_CHECKING, Any, Literal @@ -137,6 +153,23 @@ # runner pod by this selector (mirrors ``egg.io/probe-id``). _GATE_ID_LABEL = "egg.io/green-gate-id" +# Cap on the informational ``changed_files`` list a fix result carries +# in the verdict; a repo-wide format sweep can touch hundreds of files +# and the verdict must stay a single parseable log line. +_FIX_CHANGED_FILES_CAP = 100 + +# Identity for the orchestrator-authored autofix commit (#3409). +# Precedent: agent_salvage's ``egg-salvage`` system identity and the +# git-route orchestrator attribution from #2919; the commit must read +# as pipeline infrastructure in the history, not as a phantom coder. +_AUTOFIX_COMMIT_NAME = "egg-green-gate" +_AUTOFIX_COMMIT_EMAIL = "egg-green-gate@localhost" + +# Per-git-invocation ceiling for the autofix stage/commit sequence. The +# operations are local (no network); a format sweep staging hundreds of +# files finishes in single-digit seconds. +_AUTOFIX_GIT_TIMEOUT_SECONDS = 120 + # The runner program, executed as ``python3 -c`` in the pod. Restores # the repo's prebuilt build_commands artifacts (its pinned ``.venv``) # into the worktree, then reads the check list (JSON) and repo dir from @@ -148,12 +181,22 @@ # when the repo config requires one is exactly such an infra failure — # proceeding would red every check with "command not found" and block # the slice for a toolchain-packaging problem that is not its fault. +# +# #3409 Stage A: when a check fails and carries a configured ``fix`` +# command, the runner executes the fix in the worktree and re-runs the +# check, reporting a ``fix`` sub-object in that check's verdict entry. +# The check's ``ok`` stays false: the slice tip as pushed is still red; +# only the orchestrator (the sanctioned writer) may turn the fixed tree +# into a commit on the integration branch. The runner itself never +# pushes; the fix mutates the hostPath-mounted worktree, which the +# orchestrator stages and commits after the pod exits. _RUNNER_PROGRAM = """ import json, os, shutil, subprocess, sys, time checks = json.loads(os.environ["EGG_GREEN_GATE_CHECKS"]) repo_dir = os.environ["EGG_GREEN_GATE_REPO_DIR"] tail = int(os.environ.get("EGG_GREEN_GATE_OUTPUT_TAIL", "4000")) +changed_files_cap = int(os.environ.get("EGG_GREEN_GATE_CHANGED_FILES_CAP", "100")) def restore_prebuilt(target_dir): @@ -202,30 +245,74 @@ def copy_if_missing(src, dst, **kwargs): ) sys.exit(1) -results = [] -for check in checks: - started = time.monotonic() +def run_cmd(command): try: proc = subprocess.run( - ["bash", "-c", check["command"]], + ["bash", "-c", command], cwd=repo_dir, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, errors="replace", ) - rc, out = proc.returncode, proc.stdout or "" + return proc.returncode, proc.stdout or "" except Exception as exc: - rc, out = -1, f"runner failed to execute check: {exc}" - results.append( - { - "name": check["name"], - "ok": rc == 0, - "exit_code": rc, - "duration_seconds": round(time.monotonic() - started, 1), - "output_tail": out[-tail:], + return -1, f"runner failed to execute command: {exc}" + + +def tracked_changed_files(): + # Informational only (#3409): tracked modifications the fix left in + # the worktree, via the sandbox's gateway-routed git. The + # orchestrator stages from the shared worktree itself, so a failure + # here (git wrapper hiccup) degrades to an unreported file list, + # never a wrong commit. + try: + proc = subprocess.run( + ["git", "diff", "--name-only"], + cwd=repo_dir, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + errors="replace", + ) + except Exception: + return None + if proc.returncode != 0: + return None + return [line for line in (proc.stdout or "").splitlines() if line.strip()] + + +results = [] +for check in checks: + started = time.monotonic() + rc, out = run_cmd(check["command"]) + entry = { + "name": check["name"], + "ok": rc == 0, + "exit_code": rc, + "output_tail": out[-tail:], + } + fix_cmd = check.get("fix") + if rc != 0 and fix_cmd: + # #3409: attempt the configured auto-remediation, then re-run + # the check against the fixed tree. The re-run verdict, not the + # fix command's exit code, decides success: a fixer may exit + # non-zero while still having repaired everything the check + # tests (or exit zero while leaving unfixable findings). + fix_rc, fix_out = run_cmd(fix_cmd) + rerun_rc, rerun_out = run_cmd(check["command"]) + changed = tracked_changed_files() + entry["fix"] = { + "command": fix_cmd, + "exit_code": fix_rc, + "check_ok_after_fix": rerun_rc == 0, + "changed_files": (changed[:changed_files_cap] if changed is not None else None), + "changed_file_count": (len(changed) if changed is not None else None), + "output_tail": fix_out[-tail:], + "recheck_output_tail": rerun_out[-tail:], } - ) + entry["duration_seconds"] = round(time.monotonic() - started, 1) + results.append(entry) print("EGG_GREEN_GATE_VERDICT:" + json.dumps({"checks": results}), flush=True) """ @@ -341,6 +428,7 @@ def _build_runner_job_manifest( full_env["EGG_GREEN_GATE_CHECKS"] = json.dumps(checks) full_env["EGG_GREEN_GATE_REPO_DIR"] = repo_dir full_env["EGG_GREEN_GATE_OUTPUT_TAIL"] = str(_VERDICT_OUTPUT_TAIL_CHARS) + full_env["EGG_GREEN_GATE_CHANGED_FILES_CAP"] = str(_FIX_CHANGED_FILES_CAP) volumes = [] volume_mounts = [] @@ -579,6 +667,121 @@ def _format_failed_checks(failed: list[dict[str, Any]]) -> str: return "\n\n".join(parts) +def _commit_and_push_autofix( + gateway: Any, + *, + pipeline_id: str, + slice_id: str, + worktree_path: str, + integration_branch: str, + gateway_mode: Literal["public", "private"], + fixed_checks: list[dict[str, Any]], +) -> str | None: + """Commit the runner's fix output and push it to the integration branch (#3409). + + The runner executed each failed check's configured ``fix`` command + inside the hostPath-mounted gateway worktree and re-ran the checks + green, so the tree at ``worktree_path`` is exactly the tree the + checks validated. This stages the tracked modifications + (``git add -u``: untracked check droppings such as caches and + selection JSON are never picked up), commits them under the + orchestrator's green-gate identity with ``--no-verify`` (state-store + precedent; the sandbox commit path suppresses hooks the same way), + and pushes via the launcher-authed gateway push route, the + sanctioned writer: the runner never pushes. Because the check + re-run happened against this identical tree inside the runner's + pinned toolchain, the commit needs no second runner pass to be + trusted green. + + The push lands before any slice-close side effect, so dependent + slices (which fork from the integration branch's remote tip when + they start) fork after the format commit and cannot inherit + unformatted code that would re-trip the gate. + + Returns ``None`` on success, or a human-readable error string; the + caller then blocks the slice exactly like an unfixed red (Stage B + behavior) with the error appended to the failure message. + """ + + def _git(*args: str, identity: bool = False) -> subprocess.CompletedProcess[str]: + cmd = ["git", "-C", worktree_path] + if identity: + cmd += [ + "-c", + f"user.name={_AUTOFIX_COMMIT_NAME}", + "-c", + f"user.email={_AUTOFIX_COMMIT_EMAIL}", + ] + cmd += list(args) + return subprocess.run( + cmd, + capture_output=True, + text=True, + check=False, + timeout=_AUTOFIX_GIT_TIMEOUT_SECONDS, + ) + + fixed_names = ", ".join(str(c.get("name")) for c in fixed_checks) + try: + add = _git("add", "-u") + if add.returncode != 0: + return f"git add -u failed: {(add.stderr or add.stdout or '').strip()}" + + staged = _git("diff", "--cached", "--name-only") + if staged.returncode != 0: + return ( + "git diff --cached --name-only failed: " + f"{(staged.stderr or staged.stdout or '').strip()}" + ) + staged_files = [line for line in (staged.stdout or "").splitlines() if line.strip()] + if not staged_files: + # The re-run went green without the fix modifying any + # tracked file (a flaky first run). Nothing committable can + # make the remote tip green, so refuse rather than pass a + # tip whose red verdict stands as pushed. + return ( + "fix commands re-ran the checks green but left no tracked modifications to commit" + ) + + message = ( + f"Apply configured check autofix at the green gate: {fixed_names}\n\n" + f"Automated commit for pipeline {pipeline_id}, slice {slice_id} " + f"(#3409). The green-gate runner found the named checks red at " + f"the {integration_branch} tip, ran their configured fix " + f"commands, and re-ran the checks green." + ) + commit = _git("commit", "--no-verify", "-m", message, identity=True) + if commit.returncode != 0: + return f"git commit failed: {(commit.stderr or commit.stdout or '').strip()}" + except (OSError, subprocess.SubprocessError) as exc: + return f"autofix git operation raised: {exc}" + + try: + push = gateway.push_worktree_branch( + pipeline_id, + repo_path=worktree_path, + branch=integration_branch, + mode=gateway_mode, + ) + except Exception as exc: # noqa: BLE001 - push failure blocks like an unfixed red + return f"autofix push to {integration_branch} raised: {exc}" + if not getattr(push, "ok", False): + return ( + f"autofix push to {integration_branch} failed " + f"({getattr(push, 'category', 'unknown')}): {getattr(push, 'detail', '')}" + ) + + logger.info( + "Green gate autofix committed and pushed (#3409)", + pipeline_id=pipeline_id, + slice_id=slice_id, + integration_branch=integration_branch, + fixed_checks=fixed_names, + staged_file_count=len(staged_files), + ) + return None + + def run_slice_green_gate( pipeline_id: str, spawner: "KubernetesSpawner", # noqa: UP037 @@ -592,11 +795,11 @@ def run_slice_green_gate( Runs after slice consensus and the #3125 evidence gate, before any close side effect. Returns ``None`` when the slice may close (checks - green, gate off/log-mode, or an infrastructure failure — fail-open), - or a human-readable failure string naming the red checks — the - caller records the slice failure with it, routing through the - existing cascade + OVERSEER_ALERT machinery instead of opening a - red PR. + green, gate off/log-mode, an infrastructure failure — fail-open — + or a red verdict fully remediated by the #3409 autofix commit), or + a human-readable failure string naming the red checks — the caller + records the slice failure with it, routing through the existing + cascade + OVERSEER_ALERT machinery instead of opening a red PR. The runner gets its own gateway worktree forked from ``origin/`` (both ``base_branch`` and @@ -691,10 +894,14 @@ def run_slice_green_gate( repo_mounts: dict[str, str] = {} repo_dir = "" + repo_host_dir = "" for host_path in wt_result.worktrees.values(): container_path = f"/home/egg/repos/{os.path.basename(host_path)}" repo_mounts[container_path] = host_path repo_dir = container_path + # Orchestrator-side path of the same worktree: the #3409 + # autofix stages/commits here after the runner pod exits. + repo_host_dir = host_path from kubernetes_spawner import GATEWAY_K8S_URL @@ -790,6 +997,15 @@ def run_slice_green_gate( return None failed_names = ", ".join(str(c.get("name")) for c in failed) + # #3409 Stage A: the gate can self-heal when EVERY failed check + # carries a fix result whose re-run went green. A partially + # fixable verdict (some failed check has no fix, or its re-run + # stayed red) routes to the slice team unchanged: committing a + # partial fix would re-run the gate against a tip that is still + # red by construction. + autofix_ready = all( + isinstance(c.get("fix"), dict) and c["fix"].get("check_ok_after_fix") for c in failed + ) logger.error( "Green gate red: configured checks failed at the slice tip (#3398)", pipeline_id=pipeline_id, @@ -797,14 +1013,52 @@ def run_slice_green_gate( gate_id=gate_id, integration_branch=integration_branch, failed_checks=failed_names, + autofix_ready=autofix_ready, mode=mode, ) + autofix_note = "" + if autofix_ready and mode == "on" and repo_host_dir: + autofix_error = _commit_and_push_autofix( + spawner.gateway, + pipeline_id=pipeline_id, + slice_id=slice_id, + worktree_path=repo_host_dir, + integration_branch=integration_branch, + gateway_mode=gateway_mode, + fixed_checks=failed, + ) + if autofix_error is None: + # The fixed tree the runner re-validated green is now + # the integration-branch tip; the slice may close. + return None + logger.warning( + "Green gate autofix failed; blocking slice like an unfixed red (#3409)", + pipeline_id=pipeline_id, + slice_id=slice_id, + gate_id=gate_id, + integration_branch=integration_branch, + error=autofix_error, + ) + autofix_note = ( + f"\n\nThe configured fix commands turned the checks green in " + f"the runner, but committing/pushing the fix failed: " + f"{autofix_error}" + ) + elif autofix_ready and mode == "log": + logger.info( + "Green gate log mode: autofix available but not applied (#3409)", + pipeline_id=pipeline_id, + slice_id=slice_id, + gate_id=gate_id, + failed_checks=failed_names, + ) if mode == "log": return None return ( f"slice {slice_id}: green gate failed — configured checks are red " f"at integration branch {integration_branch} tip: {failed_names}.\n\n" - f"{_format_failed_checks(failed)}\n\n" + f"{_format_failed_checks(failed)}" + f"{autofix_note}\n\n" f"Fix the failures on {integration_branch} and restart the slice; " f"set {GREEN_GATE_ENV_VAR}=off to bypass." ) diff --git a/orchestrator/tests/test_slice_green_gate.py b/orchestrator/tests/test_slice_green_gate.py index 42dd0983cf..1518806260 100644 --- a/orchestrator/tests/test_slice_green_gate.py +++ b/orchestrator/tests/test_slice_green_gate.py @@ -10,7 +10,11 @@ * ``parse_verdict`` — sentinel-line extraction from noisy pod logs. * ``_RUNNER_PROGRAM`` — executed for real in a subprocess: check execution + verdict shape, output tails, the prebuilt-deps restore - (copy-if-missing), and the required-but-missing infra exit. + (copy-if-missing), the required-but-missing infra exit, and the + #3409 fix flow (fix executed only on a red check, re-run verdict, + changed-files reporting + cap, no-git degrade). +* ``_commit_and_push_autofix`` — real-git stage/commit + gateway push + wiring, the no-tracked-changes refusal, and push-failure reporting. * ``_build_runner_job_manifest`` — labels (NetworkPolicy component label present; monitor/agent-supervision labels absent), env, mounts, deadline. @@ -214,6 +218,7 @@ def _run_runner( *, require_prebuilt: str = "0", prebuilt_base: Path | None = None, + extra_env: dict[str, str] | None = None, ) -> subprocess.CompletedProcess[str]: repo_dir = tmp_path / "egg" repo_dir.mkdir(exist_ok=True) @@ -229,6 +234,7 @@ def _run_runner( ), } ) + env.update(extra_env or {}) return subprocess.run( [sys.executable, "-c", sgg._RUNNER_PROGRAM], capture_output=True, @@ -321,6 +327,126 @@ def test_optional_prebuilt_missing_proceeds(self, tmp_path: Path) -> None: assert verdict["checks"][0]["ok"] is True +def _git(repo_dir: Path, *args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["git", "-c", "user.name=t", "-c", "user.email=t@t", *args], + cwd=repo_dir, + capture_output=True, + text=True, + check=True, + ) + + +def _init_git_repo(repo_dir: Path) -> None: + repo_dir.mkdir(exist_ok=True) + _git(repo_dir, "init", "-q", ".") + (repo_dir / "file.txt").write_text("bad\n") + _git(repo_dir, "add", "file.txt") + _git(repo_dir, "commit", "-q", "-m", "init") + + +class TestRunnerFixFlow: + """#3409 — the runner's fix execution + re-run reporting.""" + + FIXABLE_CHECK = { + "name": "lint", + "command": "grep -q good file.txt", + "fix": "printf 'good\\n' > file.txt", + } + + def test_red_check_with_fix_reports_fix_result(self, tmp_path: Path) -> None: + _init_git_repo(tmp_path / "egg") + (tmp_path / "egg" / "junk.log").write_text("untracked check dropping") + proc = _run_runner(tmp_path, [dict(self.FIXABLE_CHECK)]) + assert proc.returncode == 0 + verdict = sgg.parse_verdict(proc.stdout) + assert verdict is not None + entry = verdict["checks"][0] + # The tip as pushed is still red; only the orchestrator commit + # may turn the verdict green. + assert entry["ok"] is False + fix = entry["fix"] + assert fix["command"] == self.FIXABLE_CHECK["fix"] + assert fix["exit_code"] == 0 + assert fix["check_ok_after_fix"] is True + # Tracked modification reported; untracked droppings are not. + assert fix["changed_files"] == ["file.txt"] + assert fix["changed_file_count"] == 1 + + def test_fix_that_does_not_repair_reports_red_rerun(self, tmp_path: Path) -> None: + _init_git_repo(tmp_path / "egg") + proc = _run_runner( + tmp_path, + [{"name": "lint", "command": "grep -q good file.txt", "fix": "true"}], + ) + verdict = sgg.parse_verdict(proc.stdout) + assert verdict is not None + fix = verdict["checks"][0]["fix"] + assert fix["check_ok_after_fix"] is False + assert fix["changed_files"] == [] + + def test_green_check_never_runs_fix(self, tmp_path: Path) -> None: + _init_git_repo(tmp_path / "egg") + proc = _run_runner( + tmp_path, + [{"name": "lint", "command": "true", "fix": "touch fix-ran.marker"}], + ) + verdict = sgg.parse_verdict(proc.stdout) + assert verdict is not None + assert "fix" not in verdict["checks"][0] + assert not (tmp_path / "egg" / "fix-ran.marker").exists() + + def test_red_check_without_fix_reports_plain_red(self, tmp_path: Path) -> None: + _init_git_repo(tmp_path / "egg") + proc = _run_runner(tmp_path, [{"name": "lint", "command": "false"}]) + verdict = sgg.parse_verdict(proc.stdout) + assert verdict is not None + entry = verdict["checks"][0] + assert entry["ok"] is False + assert "fix" not in entry + + def test_changed_files_capped_but_count_exact(self, tmp_path: Path) -> None: + repo_dir = tmp_path / "egg" + _init_git_repo(repo_dir) + for i in range(3): + (repo_dir / f"extra{i}.txt").write_text("bad\n") + _git(repo_dir, "add", f"extra{i}.txt") + _git(repo_dir, "commit", "-q", "-m", "more files") + proc = _run_runner( + tmp_path, + [ + { + "name": "lint", + "command": "grep -q good file.txt", + "fix": "for f in file.txt extra0.txt extra1.txt extra2.txt; " + "do printf 'good\\n' > \"$f\"; done", + } + ], + extra_env={"EGG_GREEN_GATE_CHANGED_FILES_CAP": "2"}, + ) + verdict = sgg.parse_verdict(proc.stdout) + assert verdict is not None + fix = verdict["checks"][0]["fix"] + assert fix["check_ok_after_fix"] is True + assert len(fix["changed_files"]) == 2 + assert fix["changed_file_count"] == 4 + + def test_no_git_repo_degrades_changed_files_to_none(self, tmp_path: Path) -> None: + # No git init: the gateway-routed git diff is best-effort and a + # failure must degrade to an unreported list, not a crash. + repo_dir = tmp_path / "egg" + repo_dir.mkdir(exist_ok=True) + (repo_dir / "file.txt").write_text("bad\n") + proc = _run_runner(tmp_path, [dict(self.FIXABLE_CHECK)]) + assert proc.returncode == 0 + verdict = sgg.parse_verdict(proc.stdout) + assert verdict is not None + fix = verdict["checks"][0]["fix"] + assert fix["check_ok_after_fix"] is True + assert fix["changed_files"] is None + assert fix["changed_file_count"] is None + + # ---------------------------------------------------------------------- # _build_runner_job_manifest # ---------------------------------------------------------------------- @@ -674,4 +800,215 @@ def test_manifest_env_marks_prebuilt_required( } assert env["EGG_GREEN_GATE_REQUIRE_PREBUILT"] == "1" assert env["EGG_SESSION_TOKEN"] == "tok-123" + assert env["EGG_GREEN_GATE_CHANGED_FILES_CAP"] == str(sgg._FIX_CHANGED_FILES_CAP) assert json.loads(env["EGG_GREEN_GATE_CHECKS"]) == CHECKS + + +# ---------------------------------------------------------------------- +# _commit_and_push_autofix (#3409) +# ---------------------------------------------------------------------- + + +def _autofix_repo(tmp_path: Path) -> Path: + repo_dir = tmp_path / "egg" + _init_git_repo(repo_dir) + return repo_dir + + +def _run_autofix(repo_dir: Path, gateway: MagicMock) -> str | None: + return sgg._commit_and_push_autofix( + gateway, + pipeline_id=PIPELINE_ID, + slice_id=SLICE_ID, + worktree_path=str(repo_dir), + integration_branch=INTEGRATION_BRANCH, + gateway_mode="public", + fixed_checks=[{"name": "lint", "fix": {"check_ok_after_fix": True}}], + ) + + +class TestCommitAndPushAutofix: + def test_stages_commits_and_pushes(self, tmp_path: Path) -> None: + repo_dir = _autofix_repo(tmp_path) + (repo_dir / "file.txt").write_text("good\n") + (repo_dir / "junk.log").write_text("untracked check dropping") + gateway = MagicMock() + gateway.push_worktree_branch.return_value = SimpleNamespace(ok=True) + + assert _run_autofix(repo_dir, gateway) is None + + head = _git(repo_dir, "log", "-1", "--format=%an|%ae|%s") + author, email, subject = head.stdout.strip().split("|") + assert author == "egg-green-gate" + assert email == "egg-green-gate@localhost" + assert "lint" in subject + # Untracked droppings never enter the commit. + shown = _git(repo_dir, "show", "--name-only", "--format=", "HEAD") + assert shown.stdout.split() == ["file.txt"] + gateway.push_worktree_branch.assert_called_once_with( + PIPELINE_ID, + repo_path=str(repo_dir), + branch=INTEGRATION_BRANCH, + mode="public", + ) + + def test_no_tracked_changes_refuses_without_push(self, tmp_path: Path) -> None: + repo_dir = _autofix_repo(tmp_path) + gateway = MagicMock() + error = _run_autofix(repo_dir, gateway) + assert error is not None + assert "no tracked" in error + gateway.push_worktree_branch.assert_not_called() + + def test_push_failure_is_reported(self, tmp_path: Path) -> None: + repo_dir = _autofix_repo(tmp_path) + (repo_dir / "file.txt").write_text("good\n") + gateway = MagicMock() + gateway.push_worktree_branch.return_value = SimpleNamespace( + ok=False, category="auth_failed", detail="denied" + ) + error = _run_autofix(repo_dir, gateway) + assert error is not None + assert "auth_failed" in error + assert "denied" in error + + def test_push_raising_is_reported(self, tmp_path: Path) -> None: + repo_dir = _autofix_repo(tmp_path) + (repo_dir / "file.txt").write_text("good\n") + gateway = MagicMock() + gateway.push_worktree_branch.side_effect = RuntimeError("gateway down") + error = _run_autofix(repo_dir, gateway) + assert error is not None + assert "gateway down" in error + + def test_not_a_git_repo_is_reported(self, tmp_path: Path) -> None: + repo_dir = tmp_path / "not-a-repo" + repo_dir.mkdir() + gateway = MagicMock() + error = _run_autofix(repo_dir, gateway) + assert error is not None + gateway.push_worktree_branch.assert_not_called() + + +# ---------------------------------------------------------------------- +# run_slice_green_gate — #3409 autofix wiring +# ---------------------------------------------------------------------- + + +def _fixed(ok: bool = True) -> dict[str, Any]: + return { + "command": "make lint-fix", + "exit_code": 0, + "check_ok_after_fix": ok, + "changed_files": ["a.py"], + "changed_file_count": 1, + "output_tail": "", + "recheck_output_tail": "", + } + + +def _red_lint_verdict(*, fix: dict[str, Any] | None) -> str: + entry: dict[str, Any] = { + "name": "lint", + "ok": False, + "exit_code": 1, + "output_tail": "would reformat a.py", + } + if fix is not None: + entry["fix"] = fix + return _verdict_line([entry, {"name": "test", "ok": True, "exit_code": 0, "output_tail": ""}]) + + +class TestGreenGateAutofixWiring: + def test_fixed_red_verdict_pushes_and_passes( + self, enabled_gate: pytest.MonkeyPatch, configured_checks: None + ) -> None: + spawner = _spawner() + with ( + patch.object(sgg, "_submit_runner_job"), + patch.object(sgg, "_wait_for_runner_pod", return_value=_terminal_pod()), + patch.object(sgg, "_read_runner_log", return_value=_red_lint_verdict(fix=_fixed())), + patch.object(sgg, "_delete_runner_job"), + patch.object(sgg, "_commit_and_push_autofix", return_value=None) as autofix, + ): + assert _run_gate(spawner) is None + autofix.assert_called_once() + kwargs = autofix.call_args.kwargs + # The autofix stages the SAME hostPath worktree the runner + # mutated, and pushes to the slice integration branch. + assert kwargs["worktree_path"] == "/home/host/.egg-worktrees/runner/egg" + assert kwargs["integration_branch"] == INTEGRATION_BRANCH + assert kwargs["gateway_mode"] == "public" + assert [c["name"] for c in kwargs["fixed_checks"]] == ["lint"] + # Cleanup still runs after the autofix path. + spawner.gateway.delete_session_by_container.assert_called_once() + spawner.gateway.delete_worktrees.assert_called_once() + + def test_autofix_failure_blocks_with_note( + self, enabled_gate: pytest.MonkeyPatch, configured_checks: None + ) -> None: + spawner = _spawner() + with ( + patch.object(sgg, "_submit_runner_job"), + patch.object(sgg, "_wait_for_runner_pod", return_value=_terminal_pod()), + patch.object(sgg, "_read_runner_log", return_value=_red_lint_verdict(fix=_fixed())), + patch.object(sgg, "_delete_runner_job"), + patch.object(sgg, "_commit_and_push_autofix", return_value="push exploded"), + ): + failure = _run_gate(spawner) + assert failure is not None + assert "lint" in failure + assert "push exploded" in failure + + def test_rerun_still_red_blocks_without_autofix( + self, enabled_gate: pytest.MonkeyPatch, configured_checks: None + ) -> None: + spawner = _spawner() + with ( + patch.object(sgg, "_submit_runner_job"), + patch.object(sgg, "_wait_for_runner_pod", return_value=_terminal_pod()), + patch.object( + sgg, "_read_runner_log", return_value=_red_lint_verdict(fix=_fixed(ok=False)) + ), + patch.object(sgg, "_delete_runner_job"), + patch.object(sgg, "_commit_and_push_autofix") as autofix, + ): + failure = _run_gate(spawner) + assert failure is not None + autofix.assert_not_called() + + def test_partially_fixable_verdict_blocks_without_autofix( + self, enabled_gate: pytest.MonkeyPatch, configured_checks: None + ) -> None: + spawner = _spawner() + log = _verdict_line( + [ + {"name": "lint", "ok": False, "exit_code": 1, "output_tail": "", "fix": _fixed()}, + {"name": "test", "ok": False, "exit_code": 2, "output_tail": "FAILED"}, + ] + ) + with ( + patch.object(sgg, "_submit_runner_job"), + patch.object(sgg, "_wait_for_runner_pod", return_value=_terminal_pod()), + patch.object(sgg, "_read_runner_log", return_value=log), + patch.object(sgg, "_delete_runner_job"), + patch.object(sgg, "_commit_and_push_autofix") as autofix, + ): + failure = _run_gate(spawner) + assert failure is not None + autofix.assert_not_called() + + def test_log_mode_never_pushes_a_fix( + self, gate_env: pytest.MonkeyPatch, configured_checks: None + ) -> None: + gate_env.setenv(sgg.GREEN_GATE_ENV_VAR, "log") + spawner = _spawner() + with ( + patch.object(sgg, "_submit_runner_job"), + patch.object(sgg, "_wait_for_runner_pod", return_value=_terminal_pod()), + patch.object(sgg, "_read_runner_log", return_value=_red_lint_verdict(fix=_fixed())), + patch.object(sgg, "_delete_runner_job"), + patch.object(sgg, "_commit_and_push_autofix") as autofix, + ): + assert _run_gate(spawner) is None + autofix.assert_not_called() diff --git a/shared/egg_config/validators.py b/shared/egg_config/validators.py index c9b94009a2..60e52173cb 100644 --- a/shared/egg_config/validators.py +++ b/shared/egg_config/validators.py @@ -168,20 +168,30 @@ def validate_checks(checks: list[Any]) -> list[dict[str, str]]: Used by config, orchestrator, and compose to validate check definitions from YAML config or JSON env vars. + Each entry requires ``name`` and ``command``. An optional ``fix`` + key names a shell command that auto-remediates a failing check + (e.g. ``make lint-fix`` for a ``lint`` check); the per-slice green + gate runs it at the slice tip and commits the result (#3409). A + ``fix`` that is present but empty/None is dropped from the entry. + Args: checks: Raw list of check entries (e.g. from YAML or JSON). Returns: - List of {"name": "...", "command": "..."} dicts with only - valid entries retained. + List of {"name": "...", "command": "..."} dicts (plus "fix" + when configured) with only valid entries retained. """ if not isinstance(checks, list): return [] - return [ - {"name": str(c["name"]), "command": str(c["command"])} - for c in checks - if isinstance(c, dict) and "name" in c and "command" in c - ] + result = [] + for c in checks: + if not (isinstance(c, dict) and "name" in c and "command" in c): + continue + entry = {"name": str(c["name"]), "command": str(c["command"])} + if c.get("fix"): + entry["fix"] = str(c["fix"]) + result.append(entry) + return result def validate_port(port: int | str) -> tuple[bool, str | None]: diff --git a/tests/egg_config/test_validators.py b/tests/egg_config/test_validators.py index d54059e8e2..2bf5e03604 100644 --- a/tests/egg_config/test_validators.py +++ b/tests/egg_config/test_validators.py @@ -5,6 +5,7 @@ from egg_config.validators import ( mask_secret, validate_anthropic_key, + validate_checks, validate_email, validate_github_token, validate_non_empty, @@ -13,6 +14,61 @@ ) +class TestValidateChecks: + """Tests for validate_checks function.""" + + def test_valid_entries(self): + """Name and command are retained and coerced to strings.""" + result = validate_checks([{"name": "lint", "command": "make lint"}]) + assert result == [{"name": "lint", "command": "make lint"}] + + def test_non_list_input(self): + """Non-list input yields an empty list.""" + assert validate_checks({"name": "lint"}) == [] + assert validate_checks(None) == [] + + def test_malformed_entries_dropped(self): + """Entries missing name or command are filtered out.""" + result = validate_checks( + [ + {"name": "lint"}, + {"command": "make test"}, + "make lint", + {"name": "ok", "command": "true"}, + ] + ) + assert result == [{"name": "ok", "command": "true"}] + + def test_values_coerced_to_strings(self): + """Non-string values are coerced to strings.""" + result = validate_checks([{"name": 1, "command": 2, "fix": 3}]) + assert result == [{"name": "1", "command": "2", "fix": "3"}] + + def test_fix_key_preserved(self): + """The optional fix auto-remediation command survives (#3409).""" + result = validate_checks( + [ + {"name": "lint", "command": "make lint", "fix": "make lint-fix"}, + {"name": "test", "command": "make test"}, + ] + ) + assert result == [ + {"name": "lint", "command": "make lint", "fix": "make lint-fix"}, + {"name": "test", "command": "make test"}, + ] + + def test_empty_fix_dropped(self): + """A present-but-empty fix is dropped from the entry.""" + for empty in ("", None): + result = validate_checks([{"name": "lint", "command": "make lint", "fix": empty}]) + assert result == [{"name": "lint", "command": "make lint"}] + + def test_unknown_keys_dropped(self): + """Keys outside the schema never pass through.""" + result = validate_checks([{"name": "lint", "command": "make lint", "extra": "x"}]) + assert result == [{"name": "lint", "command": "make lint"}] + + class TestValidateUrl: """Tests for validate_url function.""" From d019091decece33ec8e3f53d5431388001c0571d Mon Sep 17 00:00:00 2001 From: jwbron <8340608+jwbron@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:05:30 +0000 Subject: [PATCH 2/6] Fix checks: point artifact-spec ratchet at pipelines package after #3312 split --- .../egg_contracts/tests/test_artifact_spec.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/shared/egg_contracts/tests/test_artifact_spec.py b/shared/egg_contracts/tests/test_artifact_spec.py index 3caec9f87e..d60af18c41 100644 --- a/shared/egg_contracts/tests/test_artifact_spec.py +++ b/shared/egg_contracts/tests/test_artifact_spec.py @@ -408,8 +408,14 @@ class TestConsistencyC_PromptDerivesFromSpec: (covered by Consistency-B above). """ + # ``orchestrator/routes/pipelines`` was decomposed from a single + # ``pipelines.py`` module into a package (#3312). The prompt-builder + # code that once lived in that one file is now spread across the + # package's submodules (``_prompt_*.py``, ``_drafts.py``, + # ``_populate.py``, …), so the ratchet reads the concatenation of + # every ``.py`` file in the package rather than one file. PIPELINES_PATH = ( - Path(__file__).resolve().parents[3] / "orchestrator" / "routes" / "pipelines.py" + Path(__file__).resolve().parents[3] / "orchestrator" / "routes" / "pipelines" ) # Ratchet against a regression: forbid raw @@ -426,10 +432,17 @@ class TestConsistencyC_PromptDerivesFromSpec: @pytest.fixture(scope="class") def pipelines_text(self) -> str: - return self.PIPELINES_PATH.read_text() + return "\n".join( + p.read_text() for p in sorted(self.PIPELINES_PATH.glob("*.py")) + ) def test_pipelines_py_is_readable(self) -> None: - assert self.PIPELINES_PATH.exists(), f"missing: {self.PIPELINES_PATH} — has the file moved?" + assert self.PIPELINES_PATH.is_dir(), ( + f"missing: {self.PIPELINES_PATH} — has the package moved?" + ) + assert any(self.PIPELINES_PATH.glob("*.py")), ( + f"no Python sources under {self.PIPELINES_PATH} — has the package moved?" + ) def test_no_raw_agent_output_literals_remain(self, pipelines_text: str) -> None: # Slice-3 of #3077 removed every From 803187831cae3b8808b539cb92388ed957ecb63f Mon Sep 17 00:00:00 2001 From: egg Date: Mon, 6 Jul 2026 18:07:00 +0000 Subject: [PATCH 3/6] Fix checks: apply automated formatting fixes --- shared/egg_contracts/tests/test_artifact_spec.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/shared/egg_contracts/tests/test_artifact_spec.py b/shared/egg_contracts/tests/test_artifact_spec.py index d60af18c41..ae372ef59a 100644 --- a/shared/egg_contracts/tests/test_artifact_spec.py +++ b/shared/egg_contracts/tests/test_artifact_spec.py @@ -414,9 +414,7 @@ class TestConsistencyC_PromptDerivesFromSpec: # package's submodules (``_prompt_*.py``, ``_drafts.py``, # ``_populate.py``, …), so the ratchet reads the concatenation of # every ``.py`` file in the package rather than one file. - PIPELINES_PATH = ( - Path(__file__).resolve().parents[3] / "orchestrator" / "routes" / "pipelines" - ) + PIPELINES_PATH = Path(__file__).resolve().parents[3] / "orchestrator" / "routes" / "pipelines" # Ratchet against a regression: forbid raw # ``.egg-state/agent-outputs/{_identifier}-…`` f-string literals @@ -432,9 +430,7 @@ class TestConsistencyC_PromptDerivesFromSpec: @pytest.fixture(scope="class") def pipelines_text(self) -> str: - return "\n".join( - p.read_text() for p in sorted(self.PIPELINES_PATH.glob("*.py")) - ) + return "\n".join(p.read_text() for p in sorted(self.PIPELINES_PATH.glob("*.py"))) def test_pipelines_py_is_readable(self) -> None: assert self.PIPELINES_PATH.is_dir(), ( From 980d1fb152c83919af93a35f264a9eba211d4f7c Mon Sep 17 00:00:00 2001 From: "egg-reviewer[bot]" <261018737+egg-reviewer[bot]@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:06:57 +0000 Subject: [PATCH 4/6] Green gate autofix: disable hooks + gate on final full re-run Address review feedback on the #3409 Stage A autofix path: - Disable git hooks on the orchestrator's autofix commit. The _git helper in _commit_and_push_autofix now runs every git invocation with -c core.hooksPath=/dev/null, matching the state-store precedent (StateStore._run_git / agent_salvage._run_git). --no-verify alone left post-commit and git add hooks free to execute agent-controlled worktree hooks on the orchestrator host. - Make 'the committed tip is validated green' true in general. The runner now does one final full re-run of every configured check against the tree with all fixes applied, and reports any fix-created untracked files (git ls-files --others --exclude-standard). The orchestrator's new _autofix_ready gate self-heals only when that final re-run is green AND no new untracked files were created (git add -u would drop them). Unknown/degraded verdicts fail safe to a blocked slice. Also documents the tmp_path 'not nested in a git repo' assumption in test_no_git_repo_degrades_changed_files_to_none. --- orchestrator/slice_green_gate.py | 166 +++++++++++++++-- orchestrator/tests/test_slice_green_gate.py | 191 +++++++++++++++++++- 2 files changed, 335 insertions(+), 22 deletions(-) diff --git a/orchestrator/slice_green_gate.py b/orchestrator/slice_green_gate.py index cc8cf5a87f..8e8af79454 100644 --- a/orchestrator/slice_green_gate.py +++ b/orchestrator/slice_green_gate.py @@ -282,6 +282,34 @@ def tracked_changed_files(): return [line for line in (proc.stdout or "").splitlines() if line.strip()] +def untracked_files(): + # Non-ignored untracked paths (#3409). ``--exclude-standard`` honours + # .gitignore, so check droppings (caches, selection JSON) that live + # in .gitignore are excluded; a fix that *creates* a new source file + # (codegen, a formatter that splits a module) shows up here. The + # orchestrator stages with ``git add -u``, which never picks up + # untracked files, so a fix that produces one would push a tree the + # final re-run never validated as committed — the caller refuses + # autofix when this set grew. ``None`` on failure degrades to unsafe. + try: + proc = subprocess.run( + ["git", "ls-files", "--others", "--exclude-standard"], + cwd=repo_dir, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + errors="replace", + ) + except Exception: + return None + if proc.returncode != 0: + return None + return sorted(line for line in (proc.stdout or "").splitlines() if line.strip()) + + +untracked_before = untracked_files() +any_fix_applied = False + results = [] for check in checks: started = time.monotonic() @@ -299,6 +327,7 @@ def tracked_changed_files(): # fix command's exit code, decides success: a fixer may exit # non-zero while still having repaired everything the check # tests (or exit zero while leaving unfixable findings). + any_fix_applied = True fix_rc, fix_out = run_cmd(fix_cmd) rerun_rc, rerun_out = run_cmd(check["command"]) changed = tracked_changed_files() @@ -314,7 +343,46 @@ def tracked_changed_files(): entry["duration_seconds"] = round(time.monotonic() - started, 1) results.append(entry) -print("EGG_GREEN_GATE_VERDICT:" + json.dumps({"checks": results}), flush=True) +# #3409: once any fix has run, the per-check re-runs above each validated +# an *intermediate* tree — check i re-ran before check i+1's fix, and +# originally-green checks were validated pre-fix and never re-run. The +# orchestrator, however, commits one tree with *all* fixes applied. To +# make "the committed tip is validated green" true in general (not just +# for a single-check config), re-run every check once more against the +# final fixed tree and report the aggregate. The orchestrator gates +# autofix on this pass, not on the per-check re-runs. +verdict = {"checks": results} +if any_fix_applied: + final_checks = [] + for check in checks: + rc, out = run_cmd(check["command"]) + final_checks.append( + { + "name": check["name"], + "ok": rc == 0, + "exit_code": rc, + "output_tail": out[-tail:], + } + ) + untracked_after = untracked_files() + if untracked_before is None or untracked_after is None: + # Best-effort git failed on either side: we cannot prove the fix + # created no new untracked file, so report the count as unknown + # (``None``) and let the orchestrator refuse autofix. + new_untracked = None + else: + new_untracked = sorted(set(untracked_after) - set(untracked_before)) + verdict["final_verification"] = { + "ran": True, + "all_ok": all(c["ok"] for c in final_checks), + "failed": [c["name"] for c in final_checks if not c["ok"]], + "new_untracked_files": ( + new_untracked[:changed_files_cap] if new_untracked is not None else None + ), + "new_untracked_count": (len(new_untracked) if new_untracked is not None else None), + } + +print("EGG_GREEN_GATE_VERDICT:" + json.dumps(verdict), flush=True) """ @@ -667,6 +735,55 @@ def _format_failed_checks(failed: list[dict[str, Any]]) -> str: return "\n\n".join(parts) +def _autofix_ready(verdict: dict[str, Any], failed: list[dict[str, Any]]) -> tuple[bool, str]: + """Decide whether the runner's fixed tree is safe to commit + push (#3409). + + Returns ``(ready, reason)``. ``reason`` is empty when ready and a + short human-readable explanation of the block otherwise (logged, so + an operator can see *why* a fixable-looking verdict routed to the + slice team instead of self-healing). + + The bar is deliberately stronger than "every failed check's own + re-run went green": that only validates intermediate trees (each + check re-ran before later fixes, and originally-green checks were + never re-run), whereas the orchestrator commits one tree with all + fixes applied. Autofix is safe only when: + + * every failed check carries a fix whose re-run went green (a partial + fix would push a tree that is red by construction); **and** + * the runner's ``final_verification`` — one full re-run of *every* + configured check against the final fixed tree — went green; **and** + * that final tree created no new untracked files. ``git add -u`` (the + orchestrator's stage step) never picks up untracked files, so a + fix that emits a new source file would push a tree the final + re-run never validated as committed. Unknown (``None``) counts — + a best-effort git failure in the runner — are treated as unsafe. + """ + if not all( + isinstance(c.get("fix"), dict) and c["fix"].get("check_ok_after_fix") for c in failed + ): + return False, "a failed check has no fix or its re-run stayed red" + + final = verdict.get("final_verification") + if not isinstance(final, dict) or not final.get("ran"): + return False, "runner reported no final full re-run verdict" + if not final.get("all_ok"): + blocked = ", ".join(str(n) for n in (final.get("failed") or [])) or "unknown" + return False, f"final full re-run of all checks was not green (red: {blocked})" + + new_untracked = final.get("new_untracked_count") + if new_untracked is None: + return False, "runner could not determine whether the fix created untracked files" + if new_untracked: + sample = ", ".join(str(p) for p in (final.get("new_untracked_files") or [])[:5]) + return False, ( + f"fix created {new_untracked} untracked file(s) that git add -u would drop" + f"{f' (e.g. {sample})' if sample else ''}" + ) + + return True, "" + + def _commit_and_push_autofix( gateway: Any, *, @@ -685,13 +802,21 @@ def _commit_and_push_autofix( checks validated. This stages the tracked modifications (``git add -u``: untracked check droppings such as caches and selection JSON are never picked up), commits them under the - orchestrator's green-gate identity with ``--no-verify`` (state-store - precedent; the sandbox commit path suppresses hooks the same way), - and pushes via the launcher-authed gateway push route, the - sanctioned writer: the runner never pushes. Because the check - re-run happened against this identical tree inside the runner's - pinned toolchain, the commit needs no second runner pass to be - trusted green. + orchestrator's green-gate identity. Every git invocation disables + hooks with ``-c core.hooksPath=/dev/null`` — the state-store + precedent (``StateStore._run_git`` / ``agent_salvage._run_git``) for + running git on an agent-controlled worktree — and the commit adds + ``--no-verify`` on top; ``--no-verify`` alone would leave + ``post-commit`` and ``git add``'s hooks free to run. The push goes + via the launcher-authed gateway push route, the sanctioned writer: + the runner never pushes. + + The commit needs no second runner pass because the runner's final + full-check re-run (``final_verification``) validated *this* tree — + every configured check, after all fixes were applied — and the caller + only reaches this function when that re-run went green with no + fix-created untracked files, so the ``git add -u`` tip is exactly the + validated tree. The push lands before any slice-close side effect, so dependent slices (which fork from the integration branch's remote tip when @@ -704,7 +829,14 @@ def _commit_and_push_autofix( """ def _git(*args: str, identity: bool = False) -> subprocess.CompletedProcess[str]: - cmd = ["git", "-C", worktree_path] + # ``core.hooksPath=/dev/null`` neutralizes every hook (state-store + # precedent, ``StateStore._run_git`` / ``agent_salvage._run_git``): + # the worktree tree is agent-produced integration-branch code, and + # the orchestrator must never execute its hooks. ``--no-verify`` on + # the commit is not enough — it suppresses only ``pre-commit`` / + # ``commit-msg``, leaving ``post-commit`` (and ``git add``'s hooks) + # free to run whatever the tree's ``core.hooksPath`` resolves to. + cmd = ["git", "-c", "core.hooksPath=/dev/null", "-C", worktree_path] if identity: cmd += [ "-c", @@ -997,15 +1129,12 @@ def run_slice_green_gate( return None failed_names = ", ".join(str(c.get("name")) for c in failed) - # #3409 Stage A: the gate can self-heal when EVERY failed check - # carries a fix result whose re-run went green. A partially - # fixable verdict (some failed check has no fix, or its re-run - # stayed red) routes to the slice team unchanged: committing a - # partial fix would re-run the gate against a tip that is still - # red by construction. - autofix_ready = all( - isinstance(c.get("fix"), dict) and c["fix"].get("check_ok_after_fix") for c in failed - ) + # #3409 Stage A: the gate can self-heal only when it can prove the + # tree the orchestrator will commit (all fixes applied) is green. + # ``_autofix_ready`` gates on the runner's final full re-run of + # every check plus a no-new-untracked-files check, not just each + # failed check's own intermediate re-run — see its docstring. + autofix_ready, autofix_block_reason = _autofix_ready(verdict, failed) logger.error( "Green gate red: configured checks failed at the slice tip (#3398)", pipeline_id=pipeline_id, @@ -1014,6 +1143,7 @@ def run_slice_green_gate( integration_branch=integration_branch, failed_checks=failed_names, autofix_ready=autofix_ready, + autofix_block_reason=autofix_block_reason or None, mode=mode, ) autofix_note = "" diff --git a/orchestrator/tests/test_slice_green_gate.py b/orchestrator/tests/test_slice_green_gate.py index 1518806260..eac4cfc57c 100644 --- a/orchestrator/tests/test_slice_green_gate.py +++ b/orchestrator/tests/test_slice_green_gate.py @@ -173,8 +173,8 @@ def test_invalid_falls_back(self, gate_env: pytest.MonkeyPatch, value: str) -> N # ---------------------------------------------------------------------- -def _verdict_line(checks: list[dict[str, Any]]) -> str: - return sgg.VERDICT_SENTINEL + json.dumps({"checks": checks}) +def _verdict_line(checks: list[dict[str, Any]], **extra: Any) -> str: + return sgg.VERDICT_SENTINEL + json.dumps({"checks": checks, **extra}) class TestParseVerdict: @@ -434,6 +434,13 @@ def test_changed_files_capped_but_count_exact(self, tmp_path: Path) -> None: def test_no_git_repo_degrades_changed_files_to_none(self, tmp_path: Path) -> None: # No git init: the gateway-routed git diff is best-effort and a # failure must degrade to an unreported list, not a crash. + # + # Assumption: pytest's ``tmp_path`` (under the system temp root, + # e.g. ``/tmp``) is NOT nested inside any git repository, so the + # runner's ``git diff`` / ``git ls-files`` genuinely fail. If the + # tmp root ever moves under a checkout, git would succeed against + # the enclosing repo and these ``is None`` assertions would flip — + # a confusing failure that this note is here to explain. repo_dir = tmp_path / "egg" repo_dir.mkdir(exist_ok=True) (repo_dir / "file.txt").write_text("bad\n") @@ -445,6 +452,54 @@ def test_no_git_repo_degrades_changed_files_to_none(self, tmp_path: Path) -> Non assert fix["check_ok_after_fix"] is True assert fix["changed_files"] is None assert fix["changed_file_count"] is None + # Best-effort git failed, so the untracked delta is unknown and + # the orchestrator will refuse autofix (fail-safe). + final = verdict["final_verification"] + assert final["new_untracked_count"] is None + + def test_final_verification_green_for_fixed_tree(self, tmp_path: Path) -> None: + _init_git_repo(tmp_path / "egg") + proc = _run_runner(tmp_path, [dict(self.FIXABLE_CHECK)]) + assert proc.returncode == 0 + verdict = sgg.parse_verdict(proc.stdout) + assert verdict is not None + final = verdict["final_verification"] + assert final["ran"] is True + # The final full re-run of every check against the fixed tree is + # green, and the fix only touched a tracked file. + assert final["all_ok"] is True + assert final["failed"] == [] + assert final["new_untracked_count"] == 0 + + def test_final_verification_flags_fix_created_untracked_file(self, tmp_path: Path) -> None: + _init_git_repo(tmp_path / "egg") + # The fix repairs file.txt AND emits a new, non-ignored source + # file that git add -u would never stage. + proc = _run_runner( + tmp_path, + [ + { + "name": "lint", + "command": "grep -q good file.txt", + "fix": "printf 'good\\n' > file.txt; printf 'x\\n' > generated.py", + } + ], + ) + assert proc.returncode == 0 + verdict = sgg.parse_verdict(proc.stdout) + assert verdict is not None + final = verdict["final_verification"] + assert final["all_ok"] is True + assert final["new_untracked_count"] == 1 + assert final["new_untracked_files"] == ["generated.py"] + + def test_no_fix_applied_omits_final_verification(self, tmp_path: Path) -> None: + _init_git_repo(tmp_path / "egg") + proc = _run_runner(tmp_path, [{"name": "lint", "command": "true"}]) + verdict = sgg.parse_verdict(proc.stdout) + assert verdict is not None + # No fix ran, so there is no combined tree to re-validate. + assert "final_verification" not in verdict # ---------------------------------------------------------------------- @@ -907,7 +962,23 @@ def _fixed(ok: bool = True) -> dict[str, Any]: } -def _red_lint_verdict(*, fix: dict[str, Any] | None) -> str: +def _final_verification( + *, + all_ok: bool = True, + failed: list[str] | None = None, + new_untracked_count: int | None = 0, + new_untracked_files: list[str] | None = None, +) -> dict[str, Any]: + return { + "ran": True, + "all_ok": all_ok, + "failed": failed or [], + "new_untracked_files": (new_untracked_files if new_untracked_files is not None else []), + "new_untracked_count": new_untracked_count, + } + + +def _red_lint_verdict(*, fix: dict[str, Any] | None, final: Any = "default") -> str: entry: dict[str, Any] = { "name": "lint", "ok": False, @@ -916,7 +987,14 @@ def _red_lint_verdict(*, fix: dict[str, Any] | None) -> str: } if fix is not None: entry["fix"] = fix - return _verdict_line([entry, {"name": "test", "ok": True, "exit_code": 0, "output_tail": ""}]) + checks = [entry, {"name": "test", "ok": True, "exit_code": 0, "output_tail": ""}] + # A fixable verdict carries the runner's final full re-run by default; + # pass ``final=None`` to model an old/degraded runner that omitted it. + if final == "default": + final = _final_verification() if fix is not None else None + if final is not None: + return _verdict_line(checks, final_verification=final) + return _verdict_line(checks) class TestGreenGateAutofixWiring: @@ -1012,3 +1090,108 @@ def test_log_mode_never_pushes_a_fix( ): assert _run_gate(spawner) is None autofix.assert_not_called() + + def _assert_blocks_without_autofix(self, spawner: MagicMock, log: str) -> None: + with ( + patch.object(sgg, "_submit_runner_job"), + patch.object(sgg, "_wait_for_runner_pod", return_value=_terminal_pod()), + patch.object(sgg, "_read_runner_log", return_value=log), + patch.object(sgg, "_delete_runner_job"), + patch.object(sgg, "_commit_and_push_autofix") as autofix, + ): + failure = _run_gate(spawner) + assert failure is not None + autofix.assert_not_called() + + def test_final_rerun_red_blocks_without_autofix( + self, enabled_gate: pytest.MonkeyPatch, configured_checks: None + ) -> None: + # #3409: every failed check's own re-run went green, but the final + # full re-run of all checks against the combined tree is red (a + # fix broke another check) — the committed tip would be red. + log = _red_lint_verdict( + fix=_fixed(), final=_final_verification(all_ok=False, failed=["test"]) + ) + self._assert_blocks_without_autofix(_spawner(), log) + + def test_fix_created_untracked_files_blocks_without_autofix( + self, enabled_gate: pytest.MonkeyPatch, configured_checks: None + ) -> None: + # #3409: the fix emitted a new source file. ``git add -u`` would + # drop it, so the pushed tip omits it and the check is red as + # pushed even though the runner's on-disk re-run was green. + log = _red_lint_verdict( + fix=_fixed(), + final=_final_verification(new_untracked_count=1, new_untracked_files=["gen/new.py"]), + ) + self._assert_blocks_without_autofix(_spawner(), log) + + def test_unknown_untracked_count_blocks_without_autofix( + self, enabled_gate: pytest.MonkeyPatch, configured_checks: None + ) -> None: + # #3409: a best-effort git failure left the untracked delta + # unknown; the gate refuses rather than risk a red pushed tip. + log = _red_lint_verdict(fix=_fixed(), final=_final_verification(new_untracked_count=None)) + self._assert_blocks_without_autofix(_spawner(), log) + + def test_missing_final_verification_blocks_without_autofix( + self, enabled_gate: pytest.MonkeyPatch, configured_checks: None + ) -> None: + # #3409: an old/degraded runner that omitted final_verification + # cannot prove the committed tree is green — refuse autofix. + log = _red_lint_verdict(fix=_fixed(), final=None) + self._assert_blocks_without_autofix(_spawner(), log) + + +class TestAutofixReady: + """#3409 — ``_autofix_ready`` gating on the final full re-run.""" + + LINT_FAILED = [{"name": "lint", "fix": {"check_ok_after_fix": True}}] + + def test_ready_when_final_green_and_no_new_untracked(self) -> None: + verdict = {"checks": [], "final_verification": _final_verification()} + ready, reason = sgg._autofix_ready(verdict, self.LINT_FAILED) + assert ready is True + assert reason == "" + + def test_not_ready_when_a_failed_check_is_unfixable(self) -> None: + failed = [{"name": "lint", "fix": {"check_ok_after_fix": True}}, {"name": "test"}] + verdict = {"checks": [], "final_verification": _final_verification()} + ready, reason = sgg._autofix_ready(verdict, failed) + assert ready is False + assert "no fix" in reason + + def test_not_ready_when_final_missing(self) -> None: + ready, reason = sgg._autofix_ready({"checks": []}, self.LINT_FAILED) + assert ready is False + assert "final full re-run" in reason + + def test_not_ready_when_final_red(self) -> None: + verdict = { + "checks": [], + "final_verification": _final_verification(all_ok=False, failed=["test"]), + } + ready, reason = sgg._autofix_ready(verdict, self.LINT_FAILED) + assert ready is False + assert "test" in reason + + def test_not_ready_when_untracked_created(self) -> None: + verdict = { + "checks": [], + "final_verification": _final_verification( + new_untracked_count=2, new_untracked_files=["a.py", "b.py"] + ), + } + ready, reason = sgg._autofix_ready(verdict, self.LINT_FAILED) + assert ready is False + assert "untracked" in reason + assert "a.py" in reason + + def test_not_ready_when_untracked_count_unknown(self) -> None: + verdict = { + "checks": [], + "final_verification": _final_verification(new_untracked_count=None), + } + ready, reason = sgg._autofix_ready(verdict, self.LINT_FAILED) + assert ready is False + assert "untracked" in reason From 1ab60ee7390e43c81b93357da64932db8a772fc5 Mon Sep 17 00:00:00 2001 From: "egg-reviewer[bot]" <261018737+egg-reviewer[bot]@users.noreply.github.com> Date: Sat, 25 Jul 2026 17:26:42 +0000 Subject: [PATCH 5/6] Green gate: test infra/autofix composition, explain autofix refusals Addresses the non-blocking review items on #3517. - Add TestInfraFailOpenAutofixComposition: the merge that composed #3409's autofix with #3417's infra fail-open threaded genuine_failed into _autofix_ready / fixed_checks, and that line existed on neither parent and had no coverage. Pins the push (infra red + fixable genuine red + green final re-run pushes only the genuine fix), the block (a persistent infra red in the final re-run refuses the push), and the narrowing itself as what enables the push (switch off => no push). - Explain autofix refusals in the operator-facing message. When every red the operator is shown had a working fix but the gate still refused, append the block reason; otherwise they re-run the fix command, watch it succeed, and see no reason for the block. Factor _autofix_ready's first gate into _all_failed_checks_fixed so the no-fix case stays silent and the two cannot drift. - Correct the comment above the infra filter: an infra-tagged red *can* carry a fix that cleared it (infra is tagged from the first run and never recomputed), so the old rationale was wrong. State what the filter actually does and why it is safe in the push direction. - Document the gate's autofix push in docs/architecture/slice-dag.md: the orchestrator authoring commits on a slice branch is the kind of thing that section's readers need to know about. - Realign test_artifact_spec.py with main (comment prose and one assert message only; logic was already byte-identical) so it stops re-conflicting on every merge for no benefit. --- docs/architecture/slice-dag.md | 15 +- orchestrator/slice_green_gate.py | 51 ++++++- orchestrator/tests/test_slice_green_gate.py | 137 ++++++++++++++++++ .../egg_contracts/tests/test_artifact_spec.py | 13 +- 4 files changed, 202 insertions(+), 14 deletions(-) diff --git a/docs/architecture/slice-dag.md b/docs/architecture/slice-dag.md index 8094c92560..9b88596e66 100644 --- a/docs/architecture/slice-dag.md +++ b/docs/architecture/slice-dag.md @@ -554,7 +554,20 @@ shape: repo's configured checks at the integration-branch tip and blocks PR-open on a red verdict; staged rollout via `EGG_SLICE_GREEN_GATE`, fail-open on infra errors, including - infra-signature-tagged reds inside check execution, #3417) — calls + infra-signature-tagged reds inside check execution, #3417. + **The gate can also write to the integration branch**: when every + genuine red carries an optional `fix:` command in + `repositories.yaml` (e.g. `lint: {fix: make lint-fix}`), the + runner applies the fixes in its worktree and the orchestrator + commits them as `egg-green-gate` and pushes to the integration + branch via the launcher-authed gateway push route, #3409. This is + the one place the orchestrator authors commits on a slice branch; + it fires only in `on` mode, and only when the runner proves the + exact tree `git add -u` will stage is green — one full re-run of + *every* configured check against the all-fixes-applied tree + (`final_verification.all_ok`) plus a no-new-untracked-files + check. Any failure to commit or push blocks the slice exactly + like an unfixed red) — calls `GatewayClient.create_slice_pr` with `base` resolved from the slice's DAG parent (root → latest completed chain tip, else the pipeline branch (#3541); child → parent's diff --git a/orchestrator/slice_green_gate.py b/orchestrator/slice_green_gate.py index 19f37d5125..daaf1c317f 100644 --- a/orchestrator/slice_green_gate.py +++ b/orchestrator/slice_green_gate.py @@ -870,6 +870,21 @@ def _format_failed_checks(failed: list[dict[str, Any]]) -> str: return "\n\n".join(parts) +def _all_failed_checks_fixed(failed: list[dict[str, Any]]) -> bool: + """True when every failed check carries a fix whose re-run went green (#3409). + + This is the first gate inside :func:`_autofix_ready`, factored out so + the caller can distinguish "no fix was available" — where the reds in + the failure message are genuinely the operator's to fix and the + message is self-explanatory — from "every red *was* fixed and the + gate still refused", where the message needs to say why or it + contradicts what the operator sees when they re-run the fix. + """ + return all( + isinstance(c.get("fix"), dict) and c["fix"].get("check_ok_after_fix") for c in failed + ) + + def _autofix_ready(verdict: dict[str, Any], failed: list[dict[str, Any]]) -> tuple[bool, str]: """Decide whether the runner's fixed tree is safe to commit + push (#3409). @@ -894,9 +909,7 @@ def _autofix_ready(verdict: dict[str, Any], failed: list[dict[str, Any]]) -> tup re-run never validated as committed. Unknown (``None``) counts — a best-effort git failure in the runner — are treated as unsafe. """ - if not all( - isinstance(c.get("fix"), dict) and c["fix"].get("check_ok_after_fix") for c in failed - ): + if not _all_failed_checks_fixed(failed): return False, "a failed check has no fix or its re-run stayed red" final = verdict.get("final_verification") @@ -1270,9 +1283,23 @@ def run_slice_green_gate( # execution, not a verdict on the slice. Fail open when every red # is infra-tagged; when genuine and infra reds mix, block on the # genuine reds only so the failure routed to the cascade doesn't - # send anyone chasing an infra ghost. This runs *before* the - # #3409 autofix decision so autofix is judged on the genuine - # reds: an infra red carries no fix that could clear it. + # send anyone chasing an infra ghost. + # + # This runs *before* the #3409 autofix decision, so autofix is + # judged on the genuine reds only. Note an infra-tagged red *can* + # carry a fix that cleared it: ``infra`` is tagged from the check's + # first run and never recomputed, while the runner applies a + # configured ``fix`` to any red, so a check that was SIGKILLed and + # then re-ran clean after its fix has both ``infra`` set and + # ``check_ok_after_fix: True``. Dropping it here deliberately + # discards that evidence: the fail-open path returns before any + # push, so the proven fix is thrown away with the worktree rather + # than committed on the strength of a run we already classified as + # untrustworthy. The narrowing is safe in the push direction — + # ``_autofix_ready`` gates on ``final_verification.all_ok``, which + # is computed over *every* configured check including infra-tagged + # ones, so removing reds from this list can only remove reasons to + # push, never add one. genuine_failed = failed if _infra_fail_open_enabled(): infra_failed = [c for c in failed if c.get("infra")] @@ -1350,6 +1377,18 @@ def run_slice_green_gate( gate_id=gate_id, failed_checks=failed_names, ) + elif not autofix_ready and _all_failed_checks_fixed(genuine_failed): + # #3409: every listed red had a fix that worked, yet the gate + # still refuses to self-heal — the real blocker is something + # the failure message alone doesn't name (a fix that regressed + # a sibling check; an infra red filtered out above; a fix that + # emitted an untracked file). Without this line the operator + # re-runs the fix command, watches it succeed, and has no way + # to see why the gate said no. + autofix_note = ( + f"\n\nThe configured fix commands cleared the checks above in " + f"the runner, but the gate did not self-heal: {autofix_block_reason}." + ) if mode == "log": return None return ( diff --git a/orchestrator/tests/test_slice_green_gate.py b/orchestrator/tests/test_slice_green_gate.py index 00c9e45085..d1ce9b4f4c 100644 --- a/orchestrator/tests/test_slice_green_gate.py +++ b/orchestrator/tests/test_slice_green_gate.py @@ -1418,6 +1418,143 @@ def test_missing_final_verification_blocks_without_autofix( self._assert_blocks_without_autofix(_spawner(), log) +def _infra_plus_fixable_verdict(*, final: dict[str, Any]) -> str: + """An infra-tagged red (#3417) co-occurring with a fixable red (#3409).""" + return _verdict_line( + [ + { + "name": "test", + "ok": False, + "exit_code": 137, + "output_tail": "GATEWAY SIDECAR NOT AVAILABLE", + "infra": "GATEWAY SIDECAR NOT AVAILABLE", + }, + { + "name": "lint", + "ok": False, + "exit_code": 1, + "output_tail": "would reformat a.py", + "infra": None, + "fix": _fixed(), + }, + ], + final_verification=final, + ) + + +class TestInfraFailOpenAutofixComposition: + """#3417 infra fail-open composed with #3409 autofix. + + The gate narrows ``failed`` to ``genuine_failed`` *before* the autofix + decision, so ``_autofix_ready`` and ``_commit_and_push_autofix`` both + see only the non-infra reds. Neither #3417 nor #3409 alone exercises + this: the infra tests build verdicts with no ``fix`` block and the + autofix tests build verdicts with no ``infra`` field. + """ + + def test_infra_red_alongside_fixable_red_pushes_only_the_genuine_fix( + self, enabled_gate: pytest.MonkeyPatch, configured_checks: None + ) -> None: + # The infra-tagged red is filtered out, the genuine red's fix went + # green, and the final full re-run — which covers *every* check, + # infra-tagged ones included — is green, so the tip is provably + # green and the autofix pushes. + log = _infra_plus_fixable_verdict(final=_final_verification()) + spawner = _spawner() + with ( + patch.object(sgg, "_submit_runner_job"), + patch.object(sgg, "_wait_for_runner_pod", return_value=_terminal_pod()), + patch.object(sgg, "_read_runner_log", return_value=log), + patch.object(sgg, "_delete_runner_job"), + patch.object(sgg, "_commit_and_push_autofix", return_value=None) as autofix, + ): + assert _run_gate(spawner) is None + autofix.assert_called_once() + # Only the genuine red is reported as fixed — the infra red never + # reaches the commit path even though it was red at the tip. + assert [c["name"] for c in autofix.call_args.kwargs["fixed_checks"]] == ["lint"] + + def test_infra_red_still_red_in_final_rerun_blocks_the_fixable_red( + self, enabled_gate: pytest.MonkeyPatch, configured_checks: None + ) -> None: + # The infra red is filtered from the *presented* failures, but the + # final full re-run still covers it — so a persistent infra fault + # blocks the push rather than letting a tree only partly proven + # green reach the integration branch. + log = _infra_plus_fixable_verdict(final=_final_verification(all_ok=False, failed=["test"])) + spawner = _spawner() + with ( + patch.object(sgg, "_submit_runner_job"), + patch.object(sgg, "_wait_for_runner_pod", return_value=_terminal_pod()), + patch.object(sgg, "_read_runner_log", return_value=log), + patch.object(sgg, "_delete_runner_job"), + patch.object(sgg, "_commit_and_push_autofix") as autofix, + ): + failure = _run_gate(spawner) + autofix.assert_not_called() + assert failure is not None + # #3409: every red the operator is shown had a working fix, so the + # message must say why the gate refused to self-heal anyway — + # otherwise they re-run `make lint-fix`, watch it succeed, and see + # no reason for the block. The hidden check is named as the cause + # of the *autofix refusal*, not routed as a slice failure: its + # output tail stays out of the presented failure list (#3417). + assert "did not self-heal" in failure + assert "final full re-run of all checks was not green (red: test)" in failure + assert "GATEWAY SIDECAR NOT AVAILABLE" not in failure + + def test_no_note_when_the_genuine_red_had_no_fix( + self, enabled_gate: pytest.MonkeyPatch, configured_checks: None + ) -> None: + # Contrast: the genuine red carries no fix at all, so the reds in + # the message are the operator's own to fix and the failure text + # is self-explanatory. No autofix explanation is appended. + log = _verdict_line( + [ + { + "name": "lint", + "ok": False, + "exit_code": 1, + "output_tail": "infra", + "infra": "ENOSPC", + }, + {"name": "test", "ok": False, "exit_code": 2, "output_tail": "FAILED"}, + ] + ) + spawner = _spawner() + with ( + patch.object(sgg, "_submit_runner_job"), + patch.object(sgg, "_wait_for_runner_pod", return_value=_terminal_pod()), + patch.object(sgg, "_read_runner_log", return_value=log), + patch.object(sgg, "_delete_runner_job"), + ): + failure = _run_gate(spawner) + assert failure is not None + assert "did not self-heal" not in failure + + def test_fail_open_switch_off_makes_the_infra_red_block_the_autofix( + self, enabled_gate: pytest.MonkeyPatch, configured_checks: None + ) -> None: + # With the #3417 switch off there is no narrowing, so the + # unfixable infra-tagged red stays in the set `_autofix_ready` + # judges and the verdict is only partially fixable — no push. + # This pins the narrowing itself as what enables the push above. + enabled_gate.setenv(sgg.GREEN_GATE_INFRA_FAIL_OPEN_ENV_VAR, "off") + log = _infra_plus_fixable_verdict(final=_final_verification()) + spawner = _spawner() + with ( + patch.object(sgg, "_submit_runner_job"), + patch.object(sgg, "_wait_for_runner_pod", return_value=_terminal_pod()), + patch.object(sgg, "_read_runner_log", return_value=log), + patch.object(sgg, "_delete_runner_job"), + patch.object(sgg, "_commit_and_push_autofix") as autofix, + ): + failure = _run_gate(spawner) + autofix.assert_not_called() + assert failure is not None + assert "test" in failure + + class TestAutofixReady: """#3409 — ``_autofix_ready`` gating on the final full re-run.""" diff --git a/shared/egg_contracts/tests/test_artifact_spec.py b/shared/egg_contracts/tests/test_artifact_spec.py index 8c5f6aaa4e..91dd8e39c6 100644 --- a/shared/egg_contracts/tests/test_artifact_spec.py +++ b/shared/egg_contracts/tests/test_artifact_spec.py @@ -411,12 +411,11 @@ class TestConsistencyC_PromptDerivesFromSpec: (covered by Consistency-B above). """ - # ``orchestrator/routes/pipelines`` was decomposed from a single - # ``pipelines.py`` module into a package (#3312). The prompt-builder - # code that once lived in that one file is now spread across the - # package's submodules (``_prompt_*.py``, ``_drafts.py``, - # ``_populate.py``, …), so the ratchet reads the concatenation of - # every ``.py`` file in the package rather than one file. + # ``pipelines.py`` was decomposed into the ``pipelines/`` package + # (the prompt-construction / ``resolve_artifact_path`` calls now live + # across ``_prompt_agent.py``, ``_populate.py``, ``_drafts.py``, …), + # so this invariant reads the concatenation of every module in the + # package rather than a single file. PIPELINES_PATH = Path(__file__).resolve().parents[3] / "orchestrator" / "routes" / "pipelines" # Ratchet against a regression: forbid raw @@ -444,7 +443,7 @@ def test_pipelines_package_is_readable(self) -> None: f"missing: {self.PIPELINES_PATH} — has the package moved?" ) assert any(self.PIPELINES_PATH.glob("*.py")), ( - f"no Python sources under {self.PIPELINES_PATH} — has the package moved?" + f"no modules under {self.PIPELINES_PATH} — has the package moved?" ) def test_no_raw_agent_output_literals_remain(self, pipelines_text: str) -> None: From 9a160bf131510edd2b14d881c2cd4eb443706ff8 Mon Sep 17 00:00:00 2001 From: jwbron <8340608+jwbron@users.noreply.github.com> Date: Sat, 25 Jul 2026 22:36:57 +0000 Subject: [PATCH 6/6] Allowlist slice_green_gate.py: the #3609 x #3409 merge crosses the size cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Neither parent violated the 1500-line hard cap — main sat at 1165 lines and the Stage-A autofix branch at 1421 — but their union is 1594. Most of the growth is module-docstring prose added independently on both sides (the rollout rationale for the 'on' default, and the autofix self-heal contract). Decomposition is tracked in #3627; allowlisting keeps that refactor out of a merge commit. --- scripts/file-size-allowlist.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scripts/file-size-allowlist.yaml b/scripts/file-size-allowlist.yaml index 865d85777b..ee54979e8d 100644 --- a/scripts/file-size-allowlist.yaml +++ b/scripts/file-size-allowlist.yaml @@ -32,3 +32,9 @@ files: issue: "3587" orchestrator/routes/pipelines/_slice_state.py: issue: "3586" + # Went over the cap only on the #3609 x #3409 merge: main (1165) and + # the Stage-A autofix branch (1421) each sat under it, the union + # (1594) does not. Most of the growth is module-docstring prose from + # both sides. + orchestrator/slice_green_gate.py: + issue: "3627"