diff --git a/.github/workflows/agents-shipgate.yml b/.github/workflows/agents-shipgate.yml index b5223057..32de020b 100644 --- a/.github/workflows/agents-shipgate.yml +++ b/.github/workflows/agents-shipgate.yml @@ -20,6 +20,6 @@ jobs: config: shipgate.yaml ci_mode: advisory diff_base: target - fail_on_decisions: block + fail_on_merge_verdicts: blocked upload_artifact: "true" pr_comment: "false" diff --git a/src/agents_shipgate/cli/verify/command.py b/src/agents_shipgate/cli/verify/command.py index 94c5f424..1b2b2bc2 100644 --- a/src/agents_shipgate/cli/verify/command.py +++ b/src/agents_shipgate/cli/verify/command.py @@ -42,8 +42,9 @@ def verify( help=( "Local base ref/SHA for PR diff. Verify never fetches it. When " "omitted, verify auto-detects the default branch (origin/HEAD, " - "origin/main, origin/master, main, master) if it points at a " - "different commit than the head; --no-base disables that." + "origin/main, origin/master) if it points at a different commit " + "than the head. Local main/master are used only when passed " + "explicitly; --no-base disables auto-detection." ), ), no_base: bool = typer.Option( diff --git a/src/agents_shipgate/cli/verify/git.py b/src/agents_shipgate/cli/verify/git.py index 29d0b8fc..406a4194 100644 --- a/src/agents_shipgate/cli/verify/git.py +++ b/src/agents_shipgate/cli/verify/git.py @@ -3,6 +3,7 @@ import io import subprocess import tarfile +from dataclasses import dataclass from pathlib import Path from agents_shipgate.core.errors import ConfigError @@ -25,7 +26,14 @@ def ensure_git_workspace(workspace: Path) -> Path: return Path(root).resolve() -DEFAULT_BASE_CANDIDATES = ("origin/main", "origin/master", "main", "master") +REMOTE_BASE_CANDIDATES = ("origin/main", "origin/master") +LOCAL_BASE_CANDIDATES = ("main", "master") + + +@dataclass(frozen=True) +class DefaultBaseDetection: + base: str | None + notes: list[str] def commit_sha(workspace: Path, ref: str) -> str | None: @@ -42,16 +50,27 @@ def commit_sha(workspace: Path, ref: str) -> str | None: def detect_default_base(workspace: Path, head: str = "HEAD") -> str | None: """Best-effort default base ref for PR-style diff enrichment. - Tries the remote default branch (``origin/HEAD``) first, then the - conventional candidates. A candidate qualifies only when it exists - locally and points at a different commit than ``head`` — diffing a - branch against itself adds scan cost without diff signal. Never - fetches; this only reads refs that already exist in the checkout. + Tries the remote default branch (``origin/HEAD``) first, then remote + conventional candidates (``origin/main``, ``origin/master``). A + candidate qualifies only when it exists locally and points at a + different commit than ``head`` — diffing a branch against itself adds + scan cost without diff signal. Local ``main``/``master`` are never + selected implicitly because they are often stale in CI and worktrees; + pass ``--base main`` explicitly when that is intended. Never fetches; + this only reads refs that already exist in the checkout. """ + return detect_default_base_with_notes(workspace, head).base + + +def detect_default_base_with_notes( + workspace: Path, head: str = "HEAD" +) -> DefaultBaseDetection: + """Return the implicit base plus warnings for skipped local defaults.""" + head_sha = commit_sha(workspace, head) if head_sha is None: - return None + return DefaultBaseDetection(base=None, notes=[]) candidates: list[str] = [] origin_head = _run_git( workspace, ["rev-parse", "--abbrev-ref", "origin/HEAD"], check=False @@ -60,12 +79,42 @@ def detect_default_base(workspace: Path, head: str = "HEAD") -> str | None: name = origin_head.stdout.strip() if name and name != "origin/HEAD": candidates.append(name) - candidates.extend(c for c in DEFAULT_BASE_CANDIDATES if c not in candidates) + candidates.extend(c for c in REMOTE_BASE_CANDIDATES if c not in candidates) + notes = _skipped_local_base_notes(workspace, head_sha) for candidate in candidates: sha = commit_sha(workspace, candidate) if sha is not None and sha != head_sha: - return candidate - return None + return DefaultBaseDetection(base=candidate, notes=notes) + return DefaultBaseDetection(base=None, notes=notes) + + +def _skipped_local_base_notes(workspace: Path, head_sha: str) -> list[str]: + notes: list[str] = [] + for local in LOCAL_BASE_CANDIDATES: + local_sha = commit_sha(workspace, local) + if local_sha is None or local_sha == head_sha: + continue + remote = f"origin/{local}" + remote_sha = commit_sha(workspace, remote) + if remote_sha is not None and remote_sha != local_sha: + notes.append( + f"Skipped local base {local!r} for implicit auto-base because " + "only remote refs are auto-detected; " + f"{local!r} points at {_short_sha(local_sha)} while {remote!r} " + f"points at {_short_sha(remote_sha)}. Pass --base {local} " + "explicitly if that local branch is intended." + ) + continue + notes.append( + f"Skipped local base {local!r} for implicit auto-base because only " + "remote refs are auto-detected. Pass --base " + f"{local} explicitly if that local branch is intended." + ) + return notes + + +def _short_sha(sha: str) -> str: + return sha[:12] def ref_exists(workspace: Path, ref: str) -> bool: @@ -196,7 +245,9 @@ def _safe_extract(tar: tarfile.TarFile, destination: Path) -> None: __all__ = [ "archive_tree", "commit_sha", + "DefaultBaseDetection", "detect_default_base", + "detect_default_base_with_notes", "diff_context", "ensure_git_workspace", "git_path", diff --git a/src/agents_shipgate/cli/verify/orchestrator.py b/src/agents_shipgate/cli/verify/orchestrator.py index 64eb8f01..bae76ffc 100644 --- a/src/agents_shipgate/cli/verify/orchestrator.py +++ b/src/agents_shipgate/cli/verify/orchestrator.py @@ -56,7 +56,7 @@ from .fix_task import build_fix_task from .git import ( archive_tree, - detect_default_base, + detect_default_base_with_notes, diff_context, ensure_git_workspace, git_path, @@ -185,11 +185,12 @@ def run_verify( raise ConfigError(f"Head ref does not exist locally: {head}") if base is None and auto_base: - detected = detect_default_base(git_root, head) - if detected is not None: - base = detected + detection = detect_default_base_with_notes(git_root, head) + base_notes.extend(detection.notes) + if detection.base is not None: + base = detection.base base_notes.append( - f"Auto-detected base {detected!r} for diff context; " + f"Auto-detected base {detection.base!r} for diff context; " "pass --base to override or --no-base to disable." ) diff --git a/tests/test_action_metadata.py b/tests/test_action_metadata.py index c590be42..5c68e0e4 100644 --- a/tests/test_action_metadata.py +++ b/tests/test_action_metadata.py @@ -2,6 +2,35 @@ import yaml +WORKFLOW_DIR = Path(".github/workflows") + + +def _workflow_paths() -> list[Path]: + return sorted([*WORKFLOW_DIR.glob("*.yml"), *WORKFLOW_DIR.glob("*.yaml")]) + + +def _workflow_steps(workflow: dict) -> list[dict]: + steps: list[dict] = [] + jobs = workflow.get("jobs") or {} + for job in jobs.values(): + if not isinstance(job, dict): + continue + for step in job.get("steps") or []: + if isinstance(step, dict): + steps.append(step) + return steps + + +def _local_action_metadata_path(uses: str) -> Path | None: + if not uses.startswith("./"): + return None + action_path = Path(uses.split("@", 1)[0]) + candidates = [action_path / "action.yml", action_path / "action.yaml"] + for candidate in candidates: + if candidate.exists(): + return candidate + raise AssertionError(f"Local action {uses!r} has no action.yml or action.yaml") + def test_github_script_reads_output_dir_from_env(): text = Path("action.yml").read_text(encoding="utf-8") @@ -166,6 +195,34 @@ def test_action_preserves_reports_before_applying_exit_code(): assert "args+=(--no-plugins)" in text +def test_repo_workflows_use_declared_local_action_inputs(): + for workflow_path in _workflow_paths(): + workflow = yaml.safe_load(workflow_path.read_text(encoding="utf-8")) + assert isinstance(workflow, dict), f"{workflow_path} must parse as a mapping" + for step in _workflow_steps(workflow): + uses = step.get("uses") + if not isinstance(uses, str): + continue + metadata_path = _local_action_metadata_path(uses) + if metadata_path is None: + continue + metadata = yaml.safe_load(metadata_path.read_text(encoding="utf-8")) + declared = set((metadata.get("inputs") or {}).keys()) + supplied = set((step.get("with") or {}).keys()) + unknown = supplied - declared + assert not unknown, ( + f"{workflow_path}: step uses {uses!r} with undeclared local " + f"action inputs {sorted(unknown)}; update {metadata_path}" + ) + + +def test_agents_shipgate_workflow_uses_merge_verdict_policy_input(): + text = (WORKFLOW_DIR / "agents-shipgate.yml").read_text(encoding="utf-8") + + assert "fail_on_decisions" not in text + assert "fail_on_merge_verdicts: blocked" in text + + def test_action_step_summary_leads_with_verifier_merge_state(): text = Path("action.yml").read_text(encoding="utf-8") script = Path("scripts/github_action_outputs.py").read_text(encoding="utf-8") diff --git a/tests/test_adapter_static_only.py b/tests/test_adapter_static_only.py index e03b23e0..6adfad05 100644 --- a/tests/test_adapter_static_only.py +++ b/tests/test_adapter_static_only.py @@ -303,7 +303,7 @@ class AllowedException: AllowedException( relative_path="cli/verify/git.py", surface="attr_call:subprocess.run", - line=152, + line=201, snippet="subprocess.run(cmd, capture_output=True, check=check, text=text, timeout=60)", rationale=( "_run_git helper for verify: executes fixed git argv assembled " diff --git a/tests/test_verify_auto_base.py b/tests/test_verify_auto_base.py index dc9bbee4..b67dba63 100644 --- a/tests/test_verify_auto_base.py +++ b/tests/test_verify_auto_base.py @@ -2,9 +2,10 @@ When ``--base`` is omitted, verify auto-detects the default branch so the capability diff exists without the nine-flag canonical incantation. The -detection never fetches and only fires when the detected ref points at a -different commit than the head — diffing a branch against itself adds scan -cost without diff signal. ``--no-base`` restores the pure working-tree mode. +detection never fetches, considers only remote refs, and only fires when the +detected ref points at a different commit than the head — diffing a branch +against itself adds scan cost without diff signal. Local ``main``/``master`` +must be passed explicitly. ``--no-base`` restores the pure working-tree mode. """ from __future__ import annotations @@ -16,7 +17,10 @@ from typer.testing import CliRunner from agents_shipgate.cli.main import app -from agents_shipgate.cli.verify.git import detect_default_base +from agents_shipgate.cli.verify.git import ( + detect_default_base, + detect_default_base_with_notes, +) runner = CliRunner() @@ -68,6 +72,36 @@ def _docs_only_repo_with_origin_main(tmp_path: Path) -> Path: return repo +def _repo_with_stale_local_main_and_origin_main_head(tmp_path: Path) -> Path: + repo = _init_repo(tmp_path) + (repo / "shipgate.yaml").write_text( + """ +version: "0.1" +project: + name: test +agent: + name: test-agent + declared_purpose: + - test +environment: + target: local +tool_sources: + - id: tools + type: mcp + path: tools.json +""".lstrip(), + encoding="utf-8", + ) + (repo / "tools.json").write_text('{"tools":[]}\n', encoding="utf-8") + (repo / "README.md").write_text("base\n", encoding="utf-8") + _commit_all(repo, "base") + _git(repo, "checkout", "-q", "--detach", "main") + (repo / "README.md").write_text("remote main\n", encoding="utf-8") + _commit_all(repo, "remote main") + _git(repo, "update-ref", "refs/remotes/origin/main", "HEAD") + return repo + + # --- detect_default_base ------------------------------------------------------ @@ -94,14 +128,34 @@ def test_detects_origin_master_fallback(tmp_path: Path) -> None: assert detect_default_base(repo) == "origin/master" -def test_detects_local_main_from_feature_branch(tmp_path: Path) -> None: +def test_does_not_auto_detect_local_main_from_feature_branch(tmp_path: Path) -> None: repo = _init_repo(tmp_path) (repo / "README.md").write_text("base\n", encoding="utf-8") _commit_all(repo, "base") _git(repo, "checkout", "-q", "-b", "feature") (repo / "README.md").write_text("feature\n", encoding="utf-8") _commit_all(repo, "feature") - assert detect_default_base(repo) == "main" + assert detect_default_base(repo) is None + detection = detect_default_base_with_notes(repo) + assert detection.base is None + assert any( + "Skipped local base 'main'" in note and "--base main" in note + for note in detection.notes + ) + + +def test_warns_when_stale_local_main_is_skipped(tmp_path: Path) -> None: + repo = _repo_with_stale_local_main_and_origin_main_head(tmp_path) + + detection = detect_default_base_with_notes(repo) + + assert detection.base is None + assert any( + "Skipped local base 'main'" in note + and "origin/main" in note + and "--base main" in note + for note in detection.notes + ) def test_returns_none_in_empty_repo(tmp_path: Path) -> None: @@ -160,3 +214,40 @@ def test_explicit_base_wins_over_auto_detection(tmp_path: Path) -> None: payload = json.loads(result.output) assert payload["base_ref"] == "origin/master" assert not any("Auto-detected base" in note for note in payload["base_notes"]) + + +def test_zero_base_verify_warns_but_skips_stale_local_main(tmp_path: Path) -> None: + repo = _repo_with_stale_local_main_and_origin_main_head(tmp_path) + + result = _verify(repo) + + assert result.exit_code == 0, result.output + payload = json.loads(result.output) + assert payload["base_ref"] is None + assert payload["base_status"] == "not_requested" + assert any( + "Skipped local base 'main'" in note + and "origin/main" in note + and "--base main" in note + for note in payload["base_notes"] + ) + + +def test_explicit_local_base_main_remains_supported(tmp_path: Path) -> None: + repo = _repo_with_stale_local_main_and_origin_main_head(tmp_path) + + result = _verify(repo, "--base", "main") + + assert result.exit_code == 0, result.output + payload = json.loads(result.output) + assert payload["base_ref"] == "main" + assert not any("Skipped local base 'main'" in note for note in payload["base_notes"]) + + +def test_base_help_documents_remote_only_auto_detection() -> None: + result = runner.invoke(app, ["verify", "--help"]) + + assert result.exit_code == 0, result.output + assert "origin/HEAD, origin/main, origin/master" in result.output + assert "origin/main, origin/master, main, master" not in result.output + assert "Local main/master are used only" in result.output