diff --git a/.github/scripts/gate_summary.py b/.github/scripts/gate_summary.py index 6017a1789..07f68e25c 100644 --- a/.github/scripts/gate_summary.py +++ b/.github/scripts/gate_summary.py @@ -21,6 +21,7 @@ class SummaryContext: output_path: Path | None python_required: bool = True docs_guard_result: str = "success" + test_quality_result: str = "skipped" @dataclass(slots=True) @@ -243,6 +244,7 @@ def _append_job_table( job_results: Mapping[str, Iterable[str]], docs_guard_result: str, docker_result: str, + test_quality_result: str, ) -> None: lines.append("") lines.append("| Job | Result |") @@ -252,6 +254,7 @@ def _append_job_table( lines.append(f"| {job_name} | {_friendly(result)} |") lines.append(f"| docs-guard | {_friendly(docs_guard_result)} |") lines.append(f"| docker-smoke | {_friendly(docker_result)} |") + lines.append(f"| test-quality | {_friendly(test_quality_result)} |") def _active_lines( @@ -264,9 +267,10 @@ def _active_lines( job_results: Mapping[str, list[str]], docs_guard_result: str = "success", docker_result: str = "skipped", + test_quality_result: str = "skipped", ) -> list[str]: lines = ["### Gate status", *table] - _append_job_table(lines, job_results, docs_guard_result, docker_result) + _append_job_table(lines, job_results, docs_guard_result, docker_result, test_quality_result) lint_status, lint_detail = _aggregate(lint_entries) type_status, type_detail = _aggregate(type_entries) @@ -334,6 +338,7 @@ def summarize(context: SummaryContext) -> SummaryResult: job_results, docs_guard_result, context.docker_result, + context.test_quality_result, ) state = "success" @@ -341,6 +346,7 @@ def summarize(context: SummaryContext) -> SummaryResult: python_result = _normalize(context.python_result or "success") docker_result_norm = _normalize(context.docker_result or "skipped") + test_quality_result = _normalize(context.test_quality_result or "skipped") cosmetic_failure = False failure_checks: tuple[str, ...] = () format_failure = False @@ -378,10 +384,20 @@ def summarize(context: SummaryContext) -> SummaryResult: elif not context.docker_changed: lines.append("- Docker smoke skipped: no Docker-related changes detected.") + if state == "success": + if test_quality_result == "cancelled": + state = "pending" + description = "Test-quality cancelled; waiting for rerun." + elif test_quality_result not in ("success", "skipped"): + state = "failure" + description = f"Test-quality result: {test_quality_result}." + adjusted_lines = [] for line in lines: if line.startswith("| docker-smoke"): adjusted_lines.append(f"| docker-smoke | {_friendly(docker_result_norm)} |") + elif line.startswith("| test-quality"): + adjusted_lines.append(f"| test-quality | {_friendly(test_quality_result)} |") else: adjusted_lines.append(line) @@ -409,6 +425,7 @@ def build_context() -> SummaryContext: python_result = os.environ.get("PYTHON_RESULT") or "skipped" docs_guard_result = os.environ.get("DOCS_GUARD_RESULT") or "success" docker_result = os.environ.get("DOCKER_RESULT") or "skipped" + test_quality_result = os.environ.get("TEST_QUALITY_RESULT") or "skipped" docker_changed = _normalize(os.environ.get("DOCKER_CHANGED"), "false") == "true" python_required = _normalize(os.environ.get("PYTHON_REQUIRED"), "true") == "true" artifacts_root = Path(os.environ.get("GATE_ARTIFACTS_ROOT", "gate_artifacts")) @@ -422,6 +439,7 @@ def build_context() -> SummaryContext: python_result=python_result, docs_guard_result=docs_guard_result, docker_result=docker_result, + test_quality_result=test_quality_result, docker_changed=docker_changed, artifacts_root=artifacts_root, summary_path=summary_path, diff --git a/.github/scripts/runtime_ac_merge_guard.js b/.github/scripts/runtime_ac_merge_guard.js new file mode 100644 index 000000000..7e2c0472d --- /dev/null +++ b/.github/scripts/runtime_ac_merge_guard.js @@ -0,0 +1,137 @@ +'use strict'; + +const RUNTIME_AC_REQUIRED_LABELS = new Set([ + 'runtime-ac', + 'runtime-verification', + 'acceptance-criteria', + 'verification-spec', + 'verification-plan', + 'ac-checks', + 'runtime-checks', +]); + +function labelName(label) { + if (typeof label === 'string') { + return label; + } + if (label && typeof label.name === 'string') { + return label.name; + } + return ''; +} + +function normalizeLabelName(label) { + return labelName(label).trim().toLowerCase(); +} + +function runtimeAcRequirement(labels = []) { + const matched = []; + const seen = new Set(); + + for (const label of labels || []) { + const normalized = normalizeLabelName(label); + if (!normalized) { + continue; + } + const colonIndex = normalized.indexOf(':'); + const suffix = + colonIndex >= 0 && colonIndex < normalized.length - 1 + ? normalized.slice(colonIndex + 1).trim() + : ''; + if ( + RUNTIME_AC_REQUIRED_LABELS.has(normalized) || + (suffix && RUNTIME_AC_REQUIRED_LABELS.has(suffix)) + ) { + if (!seen.has(normalized)) { + matched.push(normalized); + seen.add(normalized); + } + } + } + + return { + required: matched.length > 0, + labels: matched, + }; +} + +function hasRuntimeAcRequirement(labels = []) { + return runtimeAcRequirement(labels).required; +} + +// Workflow callers should pass the withRetry function produced by createTokenAwareRetry. +async function fetchPullRequestLabels({ github, owner, repo, prNumber, withRetry }) { + if (!github || !github.rest || !github.rest.issues) { + throw new Error('GitHub client is required to evaluate runtime AC merge labels.'); + } + const call = (client = github) => + client.rest.issues.listLabelsOnIssue({ + owner, + repo, + issue_number: prNumber, + per_page: 100, + }); + + try { + const response = withRetry ? await withRetry(call) : await call(); + return Array.isArray(response && response.data) ? response.data : []; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + throw new Error( + `Unable to evaluate runtime AC merge labels for PR #${prNumber}: ${message}`, + ); + } +} + +async function assertRuntimeAcMergeAllowed({ + github, + core, + owner, + repo, + prNumber, + labels, + withRetry, + source = 'external merge lane', +} = {}) { + if (!owner || !repo || !prNumber) { + throw new Error('owner, repo, and prNumber are required for runtime AC merge guard.'); + } + + const labelItems = Array.isArray(labels) + ? labels + : await fetchPullRequestLabels({ github, owner, repo, prNumber, withRetry }); + const requirement = runtimeAcRequirement(labelItems); + + if (!requirement.required) { + if (core && typeof core.info === 'function') { + core.info(`Runtime AC merge guard passed for PR #${prNumber}.`); + } + return { + allowed: true, + labels: [], + }; + } + + const labelList = requirement.labels.join(', '); + const message = + `Runtime AC merge guard blocked ${source} for PR #${prNumber}: ` + + `label(s) ${labelList} require local Orchestrator runtime acceptance checks. ` + + 'Merge through Code/Orchestrator/merge_guard.py after the runtime AC spec passes.'; + + if (core && typeof core.warning === 'function') { + core.warning(message); + } + + const error = new Error(message); + error.code = 'runtime_ac_merge_blocked'; + error.labels = requirement.labels; + throw error; +} + +module.exports = { + RUNTIME_AC_REQUIRED_LABELS, + assertRuntimeAcMergeAllowed, + hasRuntimeAcRequirement, + normalizeLabelName, + runtimeAcRequirement, +}; diff --git a/.github/workflows/agents-73-codex-belt-conveyor.yml b/.github/workflows/agents-73-codex-belt-conveyor.yml index f2e3a3540..43a4427c0 100644 --- a/.github/workflows/agents-73-codex-belt-conveyor.yml +++ b/.github/workflows/agents-73-codex-belt-conveyor.yml @@ -192,6 +192,7 @@ jobs: .github/scripts/agent_registry.js .github/scripts/error_classifier.js .github/scripts/github-api-with-retry.js + .github/scripts/runtime_ac_merge_guard.js .github/scripts/token_load_balancer.js sparse-checkout-cone-mode: false @@ -440,6 +441,7 @@ jobs: script: | const fs = require('fs'); const retryHelperPath = './.github/scripts/github-api-with-retry.js'; + const { assertRuntimeAcMergeAllowed } = require('./.github/scripts/runtime_ac_merge_guard.js'); const retryHelpers = fs.existsSync(retryHelperPath) ? require(retryHelperPath) : { @@ -451,6 +453,15 @@ jobs: const prNumber = Number('${{ inputs.pr_number }}'); const { owner, repo } = context.repo; try { + await assertRuntimeAcMergeAllowed({ + github, + core, + owner, + repo, + prNumber, + withRetry, + source: 'agents-73-codex-belt-conveyor', + }); await withRetry(() => github.rest.pulls.merge({ owner, repo, pull_number: prNumber, merge_method: 'squash' })); core.setOutput('merged', 'true'); } catch (error) { diff --git a/.github/workflows/agents-81-gate-followups.yml b/.github/workflows/agents-81-gate-followups.yml index ce45bb64c..491632590 100644 --- a/.github/workflows/agents-81-gate-followups.yml +++ b/.github/workflows/agents-81-gate-followups.yml @@ -1552,6 +1552,7 @@ jobs: with: sparse-checkout: | .github/scripts/github-api-with-retry.js + .github/scripts/runtime_ac_merge_guard.js .github/scripts/token_load_balancer.js sparse-checkout-cone-mode: false - name: Merge labelled agent PRs (guarded) @@ -1563,6 +1564,7 @@ jobs: const fs = require('fs'); const label = 'automerge'; const retryPath = './.github/scripts/github-api-with-retry.js'; + const { assertRuntimeAcMergeAllowed } = require('./.github/scripts/runtime_ac_merge_guard.js'); const { createTokenAwareRetry } = fs.existsSync(retryPath) ? require(retryPath) : { @@ -1731,6 +1733,15 @@ jobs: note = 'Refusing auto-merge: linked issue/PR has unchecked tasks.'; } else { try { + await assertRuntimeAcMergeAllowed({ + github, + core, + owner, + repo, + prNumber, + withRetry, + source: 'agents-81-gate-followups guarded merge', + }); const response = await withRetry((client) => client.rest.pulls.merge({ owner, repo, pull_number: prNumber, merge_method: 'squash' })); if (response && response.data && response.data.merged) { status = 'merged'; diff --git a/.github/workflows/agents-guard.yml b/.github/workflows/agents-guard.yml index 933f0e414..a58c5ccaf 100644 --- a/.github/workflows/agents-guard.yml +++ b/.github/workflows/agents-guard.yml @@ -111,7 +111,7 @@ jobs: github.event_name == 'pull_request_target' && steps.eligibility.outputs.should-run == 'true' && steps.api_client_base.outputs.available != 'true' - uses: "stranske/Workflows/.github/actions/setup-api-client@6deed4d3937adab2370b4ddf96046ed295efe68f" # v1 + uses: "stranske/Workflows/.github/actions/setup-api-client@d68de1904bcdbe16bfe2462b73aa18f41f8a0a47" # v1 with: secrets: ${{ toJSON(secrets) }} github_token: ${{ github.token }} @@ -180,7 +180,7 @@ jobs: steps.eligibility.outputs.should-run == 'true' && github.event_name == 'pull_request' && steps.api_client_head.outputs.available != 'true' - uses: "stranske/Workflows/.github/actions/setup-api-client@6deed4d3937adab2370b4ddf96046ed295efe68f" # v1 + uses: "stranske/Workflows/.github/actions/setup-api-client@d68de1904bcdbe16bfe2462b73aa18f41f8a0a47" # v1 with: secrets: ${{ toJSON(secrets) }} github_token: ${{ github.token }} diff --git a/AGENTS.md b/AGENTS.md index 133e6da04..9559a8d18 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,6 +2,10 @@ > Read this before changing workflows, prompts, or synced automation files. +## Working Stance — Critical Evaluator (read first) + +Your job is correct judgment, not agreement. Evaluate claims, designs, and instructions on the merits before agreeing — including the orchestrator's and the user's. When something is wrong, weaker than an alternative, or missing, say so plainly and lead with the strongest objection. Separate "this is correct" from "I'll do as asked." State your confidence and what would change your mind; flag what you are unsure of. Do not soften a real problem to be agreeable, and do not manufacture disagreement to seem rigorous — calibrated dissent, not maximal. + ## This Is A Consumer Repo Most workflow logic for this repository lives in `stranske/Workflows`. The consumer repo should only carry repo-specific configuration unless it has an explicitly documented exception. diff --git a/CLAUDE.md b/CLAUDE.md index 8a6ed6518..9b8426bfe 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2,6 +2,10 @@ > Read this before changing workflows, prompts, or synced automation files. +## Working Stance — Critical Evaluator (read first) + +Your job is correct judgment, not agreement. Evaluate claims, designs, and instructions on the merits before agreeing — including the orchestrator's and the user's. When something is wrong, weaker than an alternative, or missing, say so plainly and lead with the strongest objection. Separate "this is correct" from "I'll do as asked." State your confidence and what would change your mind; flag what you are unsure of. Do not soften a real problem to be agreeable, and do not manufacture disagreement to seem rigorous — calibrated dissent, not maximal. + ## This Is A Consumer Repo Most workflow logic for this repository lives in `stranske/Workflows`. The consumer repo should only carry repo-specific configuration unless it has an explicitly documented exception. diff --git a/scripts/check_deliberate_break.py b/scripts/check_deliberate_break.py new file mode 100644 index 000000000..27ff39d6e --- /dev/null +++ b/scripts/check_deliberate_break.py @@ -0,0 +1,347 @@ +#!/usr/bin/env python3 +"""Opt-in execution check for deliberate-break acceptance criteria.""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import shlex +import shutil +import subprocess +import sys +import tarfile +import tempfile +from collections.abc import Iterator +from dataclasses import dataclass +from io import BytesIO +from pathlib import Path + +VERDICT_PASS = "PASS" +VERDICT_HOLLOW = "FAIL_HOLLOW" +VERDICT_BROKEN = "FAIL_BROKEN" +VERDICT_SKIPPED = "SKIPPED" + +MARKER_RE = re.compile( + r"(?:)?$", + re.IGNORECASE, +) +SECTION_HEADER_RE = re.compile(r"^#{2,6}\s+(.+?)\s*$", re.MULTILINE) +ASSERTION_DIFF_RE = re.compile( + r"\b(assert|expect\(|pytest\.raises\(|assert\.)\b", +) +DEFAULT_TIMEOUT_SECONDS = 120 + + +@dataclass(frozen=True) +class DeliberateBreakSpec: + test_id: str + test_file: str + break_file: str + command: tuple[str, ...] + + +def _json_result(verdict: str, **fields: object) -> dict[str, object]: + return {"verdict": verdict, **fields} + + +def _write_github_output(**fields: str) -> None: + output_path = os.environ.get("GITHUB_OUTPUT") + if not output_path: + return + with Path(output_path).open("a", encoding="utf-8") as handle: + for key, value in fields.items(): + handle.write(f"{key}={value}\n") + + +def _acceptance_criteria(markdown: str) -> str: + headers = list(SECTION_HEADER_RE.finditer(markdown)) + for index, match in enumerate(headers): + if match.group(1).strip().lower() != "acceptance criteria": + continue + start = match.end() + end = headers[index + 1].start() if index + 1 < len(headers) else len(markdown) + return markdown[start:end].strip() + return markdown + + +def _parse_key_values(text: str) -> dict[str, str]: + values: dict[str, str] = {} + for token in shlex.split(text): + if "=" not in token: + continue + key, value = token.split("=", 1) + values[key.strip().replace("_", "-").lower()] = value.strip() + return values + + +def _explicit_marker(section: str) -> DeliberateBreakSpec | None: + for line in section.splitlines(): + match = MARKER_RE.search(line.strip()) + if not match: + continue + values = _parse_key_values(match.group("body")) + test_id = values.get("test") or values.get("test-id") + test_file = values.get("test-file") or values.get("file") + break_file = values.get("break-file") or values.get("revert-file") + command_text = values.get("command") + if not test_id or not test_file or not break_file: + raise ValueError("deliberate-break marker requires test, test-file, and break-file") + command = tuple(shlex.split(command_text)) if command_text else _pytest_command(test_id) + return DeliberateBreakSpec(test_id, test_file, break_file, command) + return None + + +def _fallback_marker(section: str) -> DeliberateBreakSpec | None: + named_line = next( + (line for line in section.splitlines() if "named test:" in line.lower()), + "", + ) + break_line = next( + ( + line + for line in section.splitlines() + if "deliberate-break" in line.lower() or "deliberate break" in line.lower() + ), + "", + ) + if not named_line or not break_line: + return None + + test_file_match = re.search(r"`([^`]*(?:test|tests)[^`]*\.py)`", named_line) + test_name_match = re.search(r"\bwith\s+`?([A-Za-z_][A-Za-z0-9_]*)`?", named_line) + break_file_match = re.search(r"`([^`]+)`", break_line) + if not test_file_match or not test_name_match or not break_file_match: + return None + + test_file = test_file_match.group(1) + test_id = f"{test_file}::{test_name_match.group(1)}" + break_file = break_file_match.group(1) + return DeliberateBreakSpec(test_id, test_file, break_file, _pytest_command(test_id)) + + +def parse_deliberate_break_spec(markdown: str) -> DeliberateBreakSpec | None: + section = _acceptance_criteria(markdown) + return _explicit_marker(section) or _fallback_marker(section) + + +def _pytest_command(test_id: str) -> tuple[str, ...]: + return (sys.executable, "-m", "pytest", test_id, "-q") + + +def _run( + command: tuple[str, ...], + cwd: Path, + *, + timeout: int = DEFAULT_TIMEOUT_SECONDS, +) -> subprocess.CompletedProcess[str]: + env = os.environ.copy() + pythonpath = str(cwd) + if env.get("PYTHONPATH"): + pythonpath = pythonpath + os.pathsep + env["PYTHONPATH"] + env["PYTHONPATH"] = pythonpath + return subprocess.run( + list(command), + cwd=cwd, + text=True, + capture_output=True, + env=env, + timeout=timeout, + ) + + +def _git( + args: list[str], + cwd: Path, + *, + timeout: int = DEFAULT_TIMEOUT_SECONDS, +) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["git", *args], + cwd=cwd, + check=True, + text=True, + capture_output=True, + timeout=timeout, + ) + + +def _assertion_diff_lines(diff_text: str) -> Iterator[str]: + """Yield removed assertion lines; adding a new assertion is valid test growth.""" + for line in diff_text.splitlines(): + if not line.startswith("-") or line.startswith("---"): + continue + if ASSERTION_DIFF_RE.search(line): + yield line[:240] + + +def _changed_assertions(base: str, head: str, test_file: str, cwd: Path) -> list[str]: + status = _git(["diff", "--name-status", f"{base}...{head}", "--", test_file], cwd) + if any(line.split("\t", 1)[0] == "A" for line in status.stdout.splitlines()): + return [] + completed = _git( + ["diff", "--no-ext-diff", "--unified=0", f"{base}...{head}", "--", test_file], + cwd, + ) + return list(_assertion_diff_lines(completed.stdout)) + + +def _archive_ref(base: str, target: Path, cwd: Path) -> None: + archive = subprocess.run( + ["git", "archive", "--format=tar", base], + cwd=cwd, + check=True, + capture_output=True, + timeout=DEFAULT_TIMEOUT_SECONDS, + ) + target_root = target.resolve() + with tarfile.open(fileobj=BytesIO(archive.stdout), mode="r:") as tar: + for member in tar: + member_path = target_root / member.name + resolved = member_path.resolve() + if not resolved.is_relative_to(target_root): + raise ValueError(f"unsafe archive path: {member.name}") + if member.isdir(): + resolved.mkdir(parents=True, exist_ok=True) + continue + if not member.isfile(): + continue + resolved.parent.mkdir(parents=True, exist_ok=True) + source = tar.extractfile(member) + if source is None: + continue + with source, resolved.open("wb") as destination: + shutil.copyfileobj(source, destination) + resolved.chmod(member.mode & 0o777) + + +def verify_spec( + spec: DeliberateBreakSpec, + *, + base: str, + head: str = "HEAD", + cwd: Path | None = None, + enforce_tamper: bool = True, +) -> dict[str, object]: + repo = cwd or Path.cwd() + test_path = repo / spec.test_file + if not test_path.is_file(): + return _json_result( + VERDICT_BROKEN, + reason="test-file-missing", + test_file=spec.test_file, + ) + + try: + if enforce_tamper: + tampered = _changed_assertions(base, head, spec.test_file, repo) + if tampered: + return _json_result( + VERDICT_BROKEN, + reason="test-assertion-tamper", + test_file=spec.test_file, + changed_assertions=tampered, + ) + + head_run = _run(spec.command, repo) + except subprocess.TimeoutExpired as exc: + return _json_result( + VERDICT_BROKEN, + reason="command-timeout", + command=list(exc.cmd) if isinstance(exc.cmd, (tuple, list)) else str(exc.cmd), + timeout=exc.timeout, + ) + + if head_run.returncode != 0: + return _json_result( + VERDICT_BROKEN, + reason="head-test-failed", + test_id=spec.test_id, + command=list(spec.command), + stdout=head_run.stdout, + stderr=head_run.stderr, + ) + + try: + with tempfile.TemporaryDirectory(prefix="deliberate-break-base-") as tmp: + base_dir = Path(tmp) + _archive_ref(base, base_dir, repo) + base_test = base_dir / spec.test_file + base_test.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(test_path, base_test) + base_run = _run(spec.command, base_dir) + except subprocess.TimeoutExpired as exc: + return _json_result( + VERDICT_BROKEN, + reason="command-timeout", + command=list(exc.cmd) if isinstance(exc.cmd, (tuple, list)) else str(exc.cmd), + timeout=exc.timeout, + ) + except ValueError as exc: + return _json_result( + VERDICT_BROKEN, + reason="archive-extract-failed", + detail=str(exc), + ) + + if base_run.returncode == 0: + return _json_result( + VERDICT_HOLLOW, + reason="test-passed-on-base-with-candidate-test", + test_id=spec.test_id, + test_file=spec.test_file, + break_file=spec.break_file, + command=list(spec.command), + stdout=base_run.stdout, + stderr=base_run.stderr, + ) + + return _json_result( + VERDICT_PASS, + reason="head-passed-base-failed", + test_id=spec.test_id, + test_file=spec.test_file, + break_file=spec.break_file, + command=list(spec.command), + base_stdout=base_run.stdout, + base_stderr=base_run.stderr, + ) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--base", default="origin/main") + parser.add_argument("--head", default="HEAD") + parser.add_argument("--pr-body-file") + parser.add_argument("--pr-body-env", default="PR_BODY") + parser.add_argument("--no-tamper-check", action="store_true") + args = parser.parse_args(argv) + + body = "" + if args.pr_body_file: + body = Path(args.pr_body_file).read_text(encoding="utf-8") + else: + body = os.environ.get(args.pr_body_env, "") + + spec = parse_deliberate_break_spec(body) + if spec is None: + _write_github_output(has_marker="false", verdict=VERDICT_SKIPPED) + print(json.dumps(_json_result(VERDICT_SKIPPED, reason="no deliberate-break marker"))) + print("skipped: no deliberate-break marker") + return 0 + + _write_github_output(has_marker="true") + result = verify_spec( + spec, + base=args.base, + head=args.head, + enforce_tamper=not args.no_tamper_check, + ) + _write_github_output(verdict=str(result["verdict"])) + print(json.dumps(result, sort_keys=True)) + return 0 if result["verdict"] == VERDICT_PASS else 1 + + +if __name__ == "__main__": + raise SystemExit(main())