diff --git a/goal/governance/delivery.py b/goal/governance/delivery.py index 99c3f1f..80dd45c 100644 --- a/goal/governance/delivery.py +++ b/goal/governance/delivery.py @@ -404,7 +404,7 @@ def governance_diagnostic_guidance(root: Path, output: str) -> list[str]: return guidance -def _governance_gate(root: Path) -> None: +def _governance_gate(root: Path, *, base: str | None = None) -> None: missing = missing_governance_package_files(root) if missing: if is_new_project_source_hub(root): @@ -432,7 +432,10 @@ def _governance_gate(root: Path) -> None: raise click.ClickException( "governance delivery requires project/governance-check.sh" ) - result = _run([str(gate)], cwd=root) + arguments = [str(gate)] + if base is not None: + arguments.extend(["--base", base]) + result = _run(arguments, cwd=root) if result.returncode != 0: detail = ( "\n".join( @@ -529,11 +532,42 @@ def _legacy_clean_default_base(root: Path) -> bool: return True +def _pull_request_validation_base(policy: DeliveryPolicy, root: Path) -> str: + """Observe the actual target; stale local refs and ticket prose are not bases.""" + remote_ref = f"refs/heads/{policy.base_branch}" + observed = _run( + ["git", "ls-remote", "--heads", policy.remote, remote_ref], cwd=root + ) + rows = [line.split() for line in observed.stdout.splitlines() if line.strip()] + if ( + observed.returncode != 0 + or len(rows) != 1 + or len(rows[0]) != 2 + or rows[0][1] != remote_ref + or re.fullmatch(r"[0-9a-f]{40}", rows[0][0]) is None + ): + raise click.ClickException( + "pull-request preflight could not resolve exactly one authoritative " + f"{policy.remote}/{policy.base_branch} base" + ) + base = rows[0][0] + known = _run(["git", "cat-file", "-e", f"{base}^{{commit}}"], cwd=root) + if known.returncode != 0: + raise click.ClickException( + "pull-request preflight cannot verify the authoritative base locally; " + f"fetch {policy.remote}/{policy.base_branch} and retry" + ) + return base + + def validate_delivery_ready(policy: DeliveryPolicy, *, cwd: Path | None = None) -> None: """Fail before workflow side effects when delivery prerequisites are unmet.""" root = _repository_root(cwd) if policy.require_clean_governance: - _governance_gate(root) + if policy.mode == "pull-request": + _governance_gate(root, base=_pull_request_validation_base(policy, root)) + else: + _governance_gate(root) if policy.mode == "publish-only": status = _git_value("status", "--porcelain", cwd=root) diff --git a/project/ticket-096/README.md b/project/ticket-096/README.md new file mode 100644 index 0000000..1f63dbf --- /dev/null +++ b/project/ticket-096/README.md @@ -0,0 +1,12 @@ +# Ticket 096: Validate PR preflight against its authoritative target base + +- **Owner**: codex +- **Status**: IN_PROGRESS +- **Workflow state**: PUBLICATION + +SESSION_EXECUTION_AUTHORIZATION: continue the requested repair, tests and publication. Ticket-095 exposed a preflight that invoked governance without a base, letting a merged adoption ticket contribute unrelated paths. Keep the gate and all independent publication checks; pass the real remote target explicitly. + +- [x] AC-01: Prove stale adoption history is not included, while unavailable or invalid remote target observations remain rejected. +- [ ] AC-02: Publish under active server rules, validate both Python versions and resume ticket-095 with the installed fix. + +Validation: 35 focused and 743 full tests passed (2 existing full-suite skips). The actual ticket-095 preflight passes with this candidate against the unchanged managed governance gate and authoritative remote base. The same current gate, configured remote, independent Validator and server rules remain required. Publication runs this tested CLI candidate through its ordinary Goal workflow. diff --git a/project/ticket-096/intent.json b/project/ticket-096/intent.json new file mode 100644 index 0000000..792e344 --- /dev/null +++ b/project/ticket-096/intent.json @@ -0,0 +1,83 @@ +{ + "schema": "new-project.intent/v3", + "ticket": "ticket-096", + "summary": "Bind PR preflight to the authoritative target instead of a stale adoption ticket", + "workstream": "application", + "classification": { + "kind": "BUG", + "priority": "P1", + "origin": "regression" + }, + "allowedPaths": [ + "goal/governance/delivery.py", + "tests/test_governance_delivery.py", + "project/ticket-096/**" + ], + "forbiddenPaths": [ + "project/ticket-*/user-*.md" + ], + "stacks": [ + "python", + "docker" + ], + "dependsOn": [], + "conflictsWith": [], + "integrationTicket": null, + "delivery": { + "acceptedBaseSha": "bd43a30a3ce5122ebe551246bc8174900eae6174", + "targetBranch": "main", + "outcome": "A new PR after a standard adoption passes the unchanged governance gate with its actual remote target base; invalid or missing target observations fail closed.", + "nonGoals": [ + "Change policy checks, trusted identities or branch protection." + ], + "complexity": "M", + "estimatedMinutes": 30, + "budgets": { + "maxImplementationFiles": 5, + "maxAffectedComponents": 1, + "maxPublicInterfaceChanges": 0, + "maxRuntimeDependencies": 0 + }, + "architecture": { + "status": "accepted", + "decision": "Resolve the configured remote target to an immutable commit before PR preflight and pass it explicitly to the existing managed gate.", + "components": [ + { + "name": "delivery-preflight", + "paths": [ + "goal/governance/delivery.py", + "tests/test_governance_delivery.py" + ] + } + ], + "responsibilityChanges": false, + "interfaceChanges": [], + "dataChanges": [], + "ui": { + "impact": "none", + "states": [], + "evidence": [] + }, + "rollback": "Revert the bounded implementation through independently reviewed delivery." + }, + "runtimeDependencies": [], + "validation": [ + { + "criterion": "AC-01", + "commands": [ + "python3 -m pytest tests/test_governance_delivery.py -q" + ], + "evidence": "Real Git remote/base and failure-path regression coverage." + }, + { + "criterion": "AC-02", + "commands": [ + "python3 -m pytest tests/ -q", + "./project/governance-check.sh --base origin/main", + "docker compose config --quiet" + ], + "evidence": "Protected publication, both Python CI jobs, post-merge CI and successful real ticket-095 preflight." + } + ] + } +} diff --git a/tests/test_governance_delivery.py b/tests/test_governance_delivery.py index ee465bc..828de0f 100644 --- a/tests/test_governance_delivery.py +++ b/tests/test_governance_delivery.py @@ -1,6 +1,7 @@ """Contract tests for governed delivery policy and local hook handling.""" import json +from dataclasses import replace from pathlib import Path import subprocess @@ -10,6 +11,76 @@ from goal.governance import delivery +def test_pr_preflight_supplies_real_base_to_unchanged_gate(tmp_path, monkeypatch): + root = _publish_repository(tmp_path) + base = _git(root, "rev-parse", "HEAD").stdout.strip() + for relative in delivery.GOVERNANCE_PACKAGE_FILES.values(): + path = root / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("{}\n", encoding="utf-8") + gate = root / "project/governance-check.sh" + gate.parent.mkdir(parents=True) + gate.write_text( + '#!/bin/sh\n[ "$#" = 2 ] && [ "$1" = --base ] && ' + f'[ "$2" = "{base}" ] || exit 17\n', encoding="utf-8" + ) + gate.chmod(0o755) + _git(root, "add", ".") + _git(root, "commit", "--quiet", "-m", "candidate after adoption") + original_run = delivery._run + def run(arguments, **kwargs): + if arguments == ["gh", "auth", "status"]: + return subprocess.CompletedProcess(arguments, 0, "", "") + return original_run(arguments, **kwargs) + monkeypatch.setattr(delivery, "_run", run) + monkeypatch.setattr(delivery.shutil, "which", lambda command: "/usr/bin/gh") + policy = replace(_pull_request_policy(), require_clean_governance=True) + before = _git(root, "status", "--porcelain").stdout + delivery.validate_delivery_ready(policy, cwd=root) + assert _git(root, "status", "--porcelain").stdout == before + + +def test_pr_preflight_ignores_stale_tracking_ref_and_uses_configured_remote(tmp_path): + root = _publish_repository(tmp_path) + stale = _git(root, "rev-parse", "HEAD").stdout.strip() + _git(root, "remote", "rename", "origin", "upstream") + (root / "README.md").write_text("advanced target\n", encoding="utf-8") + _git(root, "commit", "--quiet", "-am", "target advance") + current = _git(root, "rev-parse", "HEAD").stdout.strip() + _git(root, "push", "--quiet", "upstream", "HEAD:release") + _git(root, "update-ref", "refs/remotes/upstream/release", stale) + policy = replace(_pull_request_policy(), remote="upstream", base_branch="release") + assert delivery._pull_request_validation_base(policy, root) == current + assert _git(root, "rev-parse", "upstream/release").stdout.strip() == stale + + +@pytest.mark.parametrize("output", ["", "invalid\trefs/heads/main\n", "a" * 40 + "\trefs/heads/other\n", ("a" * 40 + "\trefs/heads/main\n") * 2]) +def test_pr_preflight_rejects_invalid_remote_observation(tmp_path, monkeypatch, output): + root = _repository(tmp_path) + monkeypatch.setattr(delivery, "_run", lambda args, **kwargs: subprocess.CompletedProcess(args, 0, output, "")) + with pytest.raises(click.ClickException, match="exactly one authoritative"): + delivery._pull_request_validation_base(_pull_request_policy(), root) + + +def test_pr_preflight_does_not_fall_back_when_remote_is_unavailable(tmp_path): + root = _publish_repository(tmp_path) + _git(root, "remote", "set-url", "origin", str(tmp_path / "missing-remote")) + with pytest.raises(click.ClickException, match="exactly one authoritative"): + delivery._pull_request_validation_base(_pull_request_policy(), root) + + +def test_pr_preflight_requires_observed_commit_to_exist_locally(tmp_path, monkeypatch): + root = _repository(tmp_path) + original_run = delivery._run + def run(arguments, **kwargs): + if arguments[:2] == ["git", "ls-remote"]: + return subprocess.CompletedProcess(arguments, 0, "a" * 40 + "\trefs/heads/main\n", "") + return original_run(arguments, **kwargs) + monkeypatch.setattr(delivery, "_run", run) + with pytest.raises(click.ClickException, match="fetch origin/main"): + delivery._pull_request_validation_base(_pull_request_policy(), root) + + def _config(**delivery_values): return {"governance": {"delivery": delivery_values}}