From a6d17794cac5724999d00045f0f20435109a6b91 Mon Sep 17 00:00:00 2001 From: Pengfei Hu Date: Wed, 17 Jun 2026 15:27:36 -0700 Subject: [PATCH 1/2] Harden PR workflow support --- .cursor/rules/agents-shipgate.mdc | 2 +- .well-known/agents-shipgate.json | 10 + README.md | 2 +- action.yml | 18 +- docs/agents/use-with-claude-code.md | 2 +- docs/agents/use-with-codex.md | 2 +- docs/agents/use-with-cursor.md | 2 +- .../ai-coding-workflow-verifier.md | 2 +- docs/shipgate-strategic-engineering-review.md | 5 + docs/target-repo-agent-snippets.md | 8 +- docs/use-cases/ai-generated-agent-prs.md | 6 +- .../10-check-run-annotations.yml | 8 +- examples/github-actions/README.md | 8 +- scripts/github_action_annotations.py | 123 +----- scripts/github_action_outputs.py | 25 ++ scripts/github_check_run.py | 103 ++++- .../agent_instructions/renderers/agents_md.py | 2 +- .../agent_instructions/renderers/claude_md.py | 2 +- .../agent_instructions/renderers/cursor.py | 2 +- .../cli/discovery/ci_workflow.py | 13 +- .../cli/verify/orchestrator.py | 2 +- src/agents_shipgate/report/pr_projection.py | 356 ++++++++++++++++++ src/agents_shipgate/schemas/contract.py | 5 +- src/agents_shipgate/triggers.py | 9 +- tests/test_action_metadata.py | 10 +- tests/test_adapter_static_only.py | 6 +- tests/test_github_action_annotations.py | 44 +++ tests/test_github_action_outputs.py | 41 ++ tests/test_github_check_run.py | 99 +++++ tests/test_init_ci.py | 20 +- tests/test_public_surface_contract.py | 12 + tests/test_verify.py | 2 +- 32 files changed, 783 insertions(+), 168 deletions(-) create mode 100644 src/agents_shipgate/report/pr_projection.py diff --git a/.cursor/rules/agents-shipgate.mdc b/.cursor/rules/agents-shipgate.mdc index 68e5514d..f2917d7b 100644 --- a/.cursor/rules/agents-shipgate.mdc +++ b/.cursor/rules/agents-shipgate.mdc @@ -58,7 +58,7 @@ plugin manifests, `.mcp.json`, `.app.json`, or `SKILL.md`, run `first_next_action.actor` is `human`, stop and route the change to a human. For committed PR/CI verification, run `agents-shipgate verify --base -origin/main --head HEAD --json` after making the base ref available; it never +origin/main --head HEAD --format json` after making the base ref available; it never fetches. Read `agents-shipgate-reports/agent-result.json` first, then `agents-shipgate-reports/verifier.json` for `merge_verdict` and `agents-shipgate-reports/report.json.release_decision.decision` for the release diff --git a/.well-known/agents-shipgate.json b/.well-known/agents-shipgate.json index bcd85fee..1b01843d 100644 --- a/.well-known/agents-shipgate.json +++ b/.well-known/agents-shipgate.json @@ -127,6 +127,16 @@ "external_integration_surfaces": ["preflight", "capability_lock", "capability_lock_diff", "capability_standard", "governance_benchmark_catalog", "governance_benchmark_result"], "gating_signal": "release_decision.decision", "merge_verdicts": ["mergeable", "human_review_required", "insufficient_evidence", "blocked", "unknown"], + "check_run_policies": ["advisory", "blocked-fails", "require-mergeable"], + "github_action_pr_workflow": { + "recommended_inputs": { + "ci_mode": "advisory", + "diff_base": "target", + "check_annotations": "true", + "pr_comment": "true" + }, + "branch_protection_check_run_policy": "require-mergeable" + }, "applicability_values": ["verified", "not_applicable", "unknown"], "release_decisions": ["passed", "review_required", "insufficient_evidence", "blocked"], "merge_verdict_labels": { diff --git a/README.md b/README.md index e7c5722a..90d49d3d 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,7 @@ the coding-agent diff that adds `stripe.create_refund` to a support agent > `safeguards.idempotency` for this financial write action. > 3. Replace wildcard/admin scopes with operation-specific scopes. > -> Then re-verify: `agents-shipgate verify --base origin/main --head HEAD --json` +> Then re-verify: `agents-shipgate verify --base origin/main --head HEAD --format json` The same `uvx agents-shipgate fixture run ai_generated_refund_pr` command above writes this comment verbatim to `reports/pr-comment.md`. diff --git a/action.yml b/action.yml index bf6e62e5..9ae972f1 100644 --- a/action.yml +++ b/action.yml @@ -98,8 +98,12 @@ inputs: Create/update a GitHub Check Run named by check_run_name with the merge verdict as the check conclusion (mergeable=success, blocked=failure, otherwise neutral) and up to 50 line-level - annotations from report.sarif. Requires `checks: write` permission - on the workflow. + annotations from the PR projection. Requires `checks: write` + permission on the workflow. + check_run_policy: + required: false + default: advisory + description: "Check Run conclusion policy: advisory, blocked-fails, or require-mergeable." check_run_name: required: false default: "Agents Shipgate" @@ -395,7 +399,14 @@ runs: return; } const STICKY = ""; - const body = fs.readFileSync(commentPath, "utf8").slice(0, 6000); + let body = fs.readFileSync(commentPath, "utf8").slice(0, 6000); + const serverUrl = process.env.GITHUB_SERVER_URL || "https://github.com"; + const runId = process.env.GITHUB_RUN_ID; + const runUrl = `${serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${runId}`; + const artifactLine = `\n\nWorkflow artifacts: ${runUrl}`; + if (body.length + artifactLine.length <= 6000) { + body = `${body}${artifactLine}`; + } // Upsert via sticky marker so re-runs update the existing comment instead of spamming the PR. // Paginate the lookup — single-page (per_page=100) misses the marker on PRs with >100 // earlier comments before Shipgate's first run, which would silently regress to append-only. @@ -428,6 +439,7 @@ runs: env: OUTPUT_DIR: ${{ inputs.output_dir }} CHECK_RUN_NAME: ${{ inputs.check_run_name }} + CHECK_RUN_POLICY: ${{ inputs.check_run_policy }} run: | python "${GITHUB_ACTION_PATH}/scripts/github_check_run.py" diff --git a/docs/agents/use-with-claude-code.md b/docs/agents/use-with-claude-code.md index 885cec5e..a1707433 100644 --- a/docs/agents/use-with-claude-code.md +++ b/docs/agents/use-with-claude-code.md @@ -128,7 +128,7 @@ reporting the change as complete, then run `verify` for PR/reviewer evidence: ```bash shipgate check --agent claude-code --workspace . --format agent-json agents-shipgate preflight --json -agents-shipgate verify --base origin/main --head HEAD --json +agents-shipgate verify --base origin/main --head HEAD --format json ``` If preflight returns `requires_human_review: true`, Claude Code must stop for a diff --git a/docs/agents/use-with-codex.md b/docs/agents/use-with-codex.md index 7bb9c146..17e8a0dd 100644 --- a/docs/agents/use-with-codex.md +++ b/docs/agents/use-with-codex.md @@ -211,7 +211,7 @@ verifier before claiming the work is done: ```bash agents-shipgate preflight --json -agents-shipgate verify --base origin/main --head HEAD --json +agents-shipgate verify --base origin/main --head HEAD --format json ``` If preflight returns `requires_human_review: true`, Codex must stop for a human diff --git a/docs/agents/use-with-cursor.md b/docs/agents/use-with-cursor.md index 58014b2c..1db0132f 100644 --- a/docs/agents/use-with-cursor.md +++ b/docs/agents/use-with-cursor.md @@ -85,7 +85,7 @@ treating the change as finished, then run `verify` for PR/reviewer evidence: ```bash shipgate check --agent cursor --workspace . --format agent-json agents-shipgate preflight --json -agents-shipgate verify --base origin/main --head HEAD --json +agents-shipgate verify --base origin/main --head HEAD --format json ``` If preflight returns `requires_human_review: true`, Cursor must stop for a human diff --git a/docs/engineering/ai-coding-workflow-verifier.md b/docs/engineering/ai-coding-workflow-verifier.md index 15911871..766cf585 100644 --- a/docs/engineering/ai-coding-workflow-verifier.md +++ b/docs/engineering/ai-coding-workflow-verifier.md @@ -628,7 +628,7 @@ Acceptance criteria: "Do not lower severity.", "Do not mark approval_policy: present without evidence." ], - "verification_command": "agents-shipgate verify --base origin/main --head HEAD --json" + "verification_command": "agents-shipgate verify --base origin/main --head HEAD --format json" } } ``` diff --git a/docs/shipgate-strategic-engineering-review.md b/docs/shipgate-strategic-engineering-review.md index 3833bcc3..2c286626 100644 --- a/docs/shipgate-strategic-engineering-review.md +++ b/docs/shipgate-strategic-engineering-review.md @@ -1,5 +1,10 @@ # Shipgate Strategic Engineering Review +> 历史说明：本文是 2026-06-09 基于 `0.11.0` / commit `ef58a57` +> 的战略审计快照。当前 `main` 已经实现 GitHub Check Run、Actions +> annotations、`verifier.json` 和 PR capability-review comment；文中将这些 +> 能力标为"缺失"的段落应按历史记录阅读，而不是当前实现状态。 + > 审计日期：2026-06-09 · 审计对象：`ThreeMoonsLab/agents-shipgate` · commit `ef58a57` · 版本 `0.11.0` > > 证据标注约定：**【仓库实证】** 直接观察到；**【意图明确但未完成】** 有代码/文档意图但未落地；**【缺失/未实现】** 经搜索确认不存在；**【战略建议】** 基于明确论点的建议。 diff --git a/docs/target-repo-agent-snippets.md b/docs/target-repo-agent-snippets.md index 230c6147..002ef104 100644 --- a/docs/target-repo-agent-snippets.md +++ b/docs/target-repo-agent-snippets.md @@ -76,7 +76,7 @@ repair and rerun the command. If `human_review.required=true` or `must_stop=true`, stop and surface the JSON result to a human. For committed PR/CI verification, run `agents-shipgate verify --base -origin/main --head HEAD --json` after making the base ref available; it never +origin/main --head HEAD --format json` after making the base ref available; it never fetches. Read `agents-shipgate-reports/agent-result.json` first, then `agents-shipgate-reports/verifier.json` for `merge_verdict` and `agents-shipgate-reports/report.json.release_decision.decision` for the release @@ -177,7 +177,7 @@ plugin manifests, `.mcp.json`, `.app.json`, or `SKILL.md`, run `first_next_action.actor` is `human`, stop and route the change to a human. For committed PR/CI verification, run `agents-shipgate verify --base -origin/main --head HEAD --json` after making the base ref available; it never +origin/main --head HEAD --format json` after making the base ref available; it never fetches. Read `agents-shipgate-reports/agent-result.json` first, then `agents-shipgate-reports/verifier.json` for `merge_verdict` and `agents-shipgate-reports/report.json.release_decision.decision` for the release @@ -255,7 +255,7 @@ plugin manifests, `.mcp.json`, `.app.json`, or `SKILL.md`, run `first_next_action.actor` is `human`, stop and route the change to a human. For committed PR/CI verification, run `agents-shipgate verify --base -origin/main --head HEAD --json` after making the base ref available; it never +origin/main --head HEAD --format json` after making the base ref available; it never fetches. Read `agents-shipgate-reports/agent-result.json` first, then `agents-shipgate-reports/verifier.json` for `merge_verdict` and `agents-shipgate-reports/report.json.release_decision.decision` for the release @@ -369,7 +369,7 @@ companion to the bootstrap snippets above: ```text Before claiming completion on any PR that changes agent tools, MCP exports, OpenAPI specs, prompts, permissions, policies, CI gates, or shipgate.yaml, run: -`agents-shipgate verify --base origin/main --head HEAD --json`. Read +`agents-shipgate verify --base origin/main --head HEAD --format json`. Read agents-shipgate-reports/verifier.json first: merge_verdict, can_merge_without_human, first_next_action, fix_task, and capability_review.top_changes. Then read report.json.release_decision.decision; diff --git a/docs/use-cases/ai-generated-agent-prs.md b/docs/use-cases/ai-generated-agent-prs.md index 88ffaa26..2c461a34 100644 --- a/docs/use-cases/ai-generated-agent-prs.md +++ b/docs/use-cases/ai-generated-agent-prs.md @@ -58,7 +58,7 @@ projection of it. There is no second verdict. A coding agent is asked to "add a support-agent feature that can issue Stripe refunds." It adds `stripe.create_refund` to the tool surface and opens a PR. -`agents-shipgate verify --base origin/main --head HEAD --json` produces: +`agents-shipgate verify --base origin/main --head HEAD --format json` produces: - a `capability_review.top_changes[]` row for `stripe.create_refund`, with an impact derived from the tool/action surface diff; @@ -101,7 +101,7 @@ block release through ordinary `SHIP-VERIFY-*` findings. pipx install agents-shipgate agents-shipgate verify --preview --json agents-shipgate init --workspace . --write --ci --agent-instructions=default --json -agents-shipgate verify --base origin/main --head HEAD --json +agents-shipgate verify --base origin/main --head HEAD --format json ``` - `verify --preview --json` is a lightweight relevance check — no scan, no @@ -113,7 +113,7 @@ agents-shipgate verify --base origin/main --head HEAD --json (`AGENTS.md`, the Cursor rule, the Claude `/shipgate` command, and `.shipgate/agent-contract.json`). Skill bundles stay explicit targets such as `codex-skill`. -- `verify --base origin/main --head HEAD --json` runs the authoritative head +- `verify --base origin/main --head HEAD --format json` runs the authoritative head scan with diff context and writes the verifier artifacts. `verify` never fetches, so make the base ref available first (`fetch-depth: 0` in CI, or `git fetch origin main` locally); if the requested diff cannot be inspected, diff --git a/examples/github-actions/10-check-run-annotations.yml b/examples/github-actions/10-check-run-annotations.yml index 8a8031ac..9dff65b8 100644 --- a/examples/github-actions/10-check-run-annotations.yml +++ b/examples/github-actions/10-check-run-annotations.yml @@ -1,10 +1,9 @@ # Surface the merge verdict as a native GitHub Check Run with line-level -# annotations from the SARIF report. Branch protection can then require +# annotations from the PR projection. Branch protection can then require # the "Agents Shipgate" check directly — no custom exit-code wiring. # -# Conclusion mapping: mergeable → success, blocked → failure, everything -# else (human_review_required / insufficient_evidence / unknown) → -# neutral, so human-routed verdicts never hard-fail the check. +# This recipe uses check_run_policy=require-mergeable, so only PRs with +# can_merge_without_human=true produce a successful Check Run. name: Agents Shipgate (check run) on: @@ -30,4 +29,5 @@ jobs: diff_base: target pr_comment: 'true' check_run: 'true' + check_run_policy: require-mergeable shipgate_version: '0.13.0' diff --git a/examples/github-actions/README.md b/examples/github-actions/README.md index a2111e61..5ff3efae 100644 --- a/examples/github-actions/README.md +++ b/examples/github-actions/README.md @@ -13,7 +13,7 @@ Copy-paste-ready workflows. Each one is a complete file — drop it into `.githu | [`07-block-on-blocked-verdict.yml`](07-block-on-blocked-verdict.yml) | Intermediate verifier policy: allow human-review PRs, but fail blocked verdicts. | | [`08-require-mergeable.yml`](08-require-mergeable.yml) | Strict verifier policy: fail unless no human authority gap remains. | | [`09-risk-labels-and-reviewers.yml`](09-risk-labels-and-reviewers.yml) | Label PRs by risk signal (`agent-capability-change`, `trust-root-touched`, `shipgate-blocked`) and request boundary owners as reviewers. | -| [`10-check-run-annotations.yml`](10-check-run-annotations.yml) | Native Check Run with line-level SARIF annotations; branch protection can require the "Agents Shipgate" check directly. Needs `checks: write`. | +| [`10-check-run-annotations.yml`](10-check-run-annotations.yml) | Native Check Run with merge-relevant line annotations; branch protection can require the "Agents Shipgate" check directly. Needs `checks: write`. | | [`11-fail-on-insufficient-evidence.yml`](11-fail-on-insufficient-evidence.yml) | Evidence policy: fail when static evidence is too weak to gate confidently. | | [`12-host-grant-drift.yml`](12-host-grant-drift.yml) | Scheduled drift gate: fail when current coding-agent host grants (MCP servers, permission rules, hooks, workflow scopes) no longer match the acknowledged `.agents-shipgate/host-grants.json` baseline. Catches authority changes that land outside PR review. | @@ -94,6 +94,12 @@ The Action also emits GitHub Actions job annotations by default for source-backed blockers and review items. Disable with `check_annotations: 'false'`, or tune the cap with `check_annotation_limit`. +When `check_run: 'true'` is enabled, the Check Run uses the same PR projection +as job annotations. `check_run_policy: advisory` preserves the default +mergeable/success, blocked/failure, human-routed/neutral behavior. For direct +branch protection, use `check_run_policy: require-mergeable`; only +`can_merge_without_human == true` succeeds. + `verify` writes static capability artifacts to the workflow artifact when available: `capabilities.lock.json`, `base.capabilities.lock.json`, and `capability-lock-diff.json`. These are review artifacts only; they do not diff --git a/scripts/github_action_annotations.py b/scripts/github_action_annotations.py index 70e3fd85..d58c2256 100644 --- a/scripts/github_action_annotations.py +++ b/scripts/github_action_annotations.py @@ -5,31 +5,38 @@ from pathlib import Path from typing import Any +from agents_shipgate.report.pr_projection import ( + PR_PROJECTION_SCHEMA_VERSION, + item_to_action_annotation, + select_pr_items, +) + ANNOTATION_SCHEMA_VERSION = "0.1" def build_annotations(output_dir: Path, *, limit: int = 50) -> dict[str, Any]: report_path = output_dir / "report.json" + verifier_path = output_dir / "verifier.json" payload = _load_json(report_path) - findings = payload.get("findings") or [] - release_decision = payload.get("release_decision") or {} - selected = _selected_findings(findings, release_decision) + verifier = _load_json(verifier_path) + selected = select_pr_items(payload, verifier, limit=10_000) normalized_limit = max(0, limit) annotations: list[dict[str, Any]] = [] omitted_no_source = 0 omitted_by_limit = 0 - for finding in selected: - source = _best_source(finding) - if source is None: + for item in selected: + if item.source_path is None: omitted_no_source += 1 continue if len(annotations) >= normalized_limit: omitted_by_limit += 1 continue - annotations.append(_annotation(finding, source)) + annotations.append(item_to_action_annotation(item)) return { "annotation_schema_version": ANNOTATION_SCHEMA_VERSION, + "pr_projection_schema_version": PR_PROJECTION_SCHEMA_VERSION, "source_report": str(report_path), + "source_verifier": str(verifier_path), "limit": normalized_limit, "annotations": annotations, "omitted": { @@ -64,104 +71,6 @@ def emit_github_annotations(payload: dict[str, Any]) -> None: print(f"::{level} {prop_text}::{message}") -def _selected_findings( - findings: list[Any], - release_decision: dict[str, Any], -) -> list[dict[str, Any]]: - active = [ - finding - for finding in findings - if isinstance(finding, dict) and not finding.get("suppressed") - ] - by_id = {finding.get("id"): finding for finding in active if finding.get("id")} - by_fingerprint = { - finding.get("fingerprint"): finding - for finding in active - if finding.get("fingerprint") - } - selected: list[dict[str, Any]] = [] - seen: set[int] = set() - for kind in ("blockers", "review_items"): - for item in release_decision.get(kind) or []: - if not isinstance(item, dict): - continue - finding = by_id.get(item.get("id")) or by_fingerprint.get( - item.get("fingerprint") - ) - if finding is not None and id(finding) not in seen: - selected.append(finding) - seen.add(id(finding)) - if selected: - return selected - return sorted(active, key=_finding_sort_key) - - -def _finding_sort_key(finding: dict[str, Any]) -> tuple[int, str, str]: - return ( - {"critical": 0, "high": 1, "medium": 2, "low": 3, "info": 4}.get( - str(finding.get("severity") or ""), - 9, - ), - str(finding.get("check_id") or ""), - str(finding.get("title") or ""), - ) - - -def _best_source(finding: dict[str, Any]) -> dict[str, Any] | None: - for key in ("source", "policy_evidence_source"): - source = finding.get(key) - if not isinstance(source, dict): - continue - if source.get("path"): - return source - return None - - -def _annotation(finding: dict[str, Any], source: dict[str, Any]) -> dict[str, Any]: - path = str(source.get("path") or "") - selector = _selector(source, path) - title = f"{finding.get('check_id')}: {finding.get('title')}" - recommendation = str(finding.get("recommendation") or "") - message = recommendation or str(finding.get("title") or finding.get("check_id") or "") - if selector: - message = f"{message} Source: {selector}" - return { - "level": _annotation_level(str(finding.get("severity") or "")), - "path": path, - "start_line": source.get("start_line"), - "end_line": source.get("end_line"), - "start_column": source.get("start_column"), - "selector": selector, - "title": _truncate(title, 160), - "message": _truncate(message, 1000), - "check_id": finding.get("check_id"), - "severity": finding.get("severity"), - "finding_id": finding.get("id"), - "fingerprint": finding.get("fingerprint"), - } - - -def _annotation_level(severity: str) -> str: - if severity in {"critical", "high"}: - return "error" - if severity == "medium": - return "warning" - return "notice" - - -def _selector(source: dict[str, Any], path: str) -> str: - pointer = source.get("pointer") - if pointer is not None: - return f"{path}#{pointer}" - location = source.get("location") - if location: - return str(location) - ref = source.get("ref") - if ref: - return str(ref) - return path - - def _escape_data(value: object) -> str: return ( str(value) @@ -179,10 +88,6 @@ def _escape_property(value: object) -> str: ) -def _truncate(value: str, limit: int) -> str: - return value if len(value) <= limit else value[: limit - 1] + "..." - - def _load_json(path: Path) -> dict[str, Any]: if not path.exists(): return {} diff --git a/scripts/github_action_outputs.py b/scripts/github_action_outputs.py index e17c4b0b..16164e17 100644 --- a/scripts/github_action_outputs.py +++ b/scripts/github_action_outputs.py @@ -157,6 +157,7 @@ def append_step_summary(output_dir: Path, values: dict[str, object]) -> None: return payload = _load_json(output_dir / "report.json") + verifier_payload = _load_json(output_dir / "verifier.json") agent_result = _load_json(output_dir / "agent-result.json") release_decision = payload.get("release_decision") or {} summary = payload.get("summary") or {} @@ -169,8 +170,23 @@ def append_step_summary(output_dir: Path, values: dict[str, object]) -> None: blocker_count = len(release_decision.get("blockers") or []) review_item_count = len(release_decision.get("review_items") or []) would_fail_ci = str(bool(fail_policy.get("would_fail_ci"))).lower() + first_next_action = verifier_payload.get("first_next_action") or {} with open(step_summary, "a", encoding="utf-8") as summary_file: summary_file.write("## Agents Shipgate\n\n") + if verifier_payload: + summary_file.write( + f"- Merge verdict: `{clean(verifier_payload.get('merge_verdict'))}`\n" + ) + summary_file.write( + "- Can merge without human: " + f"`{clean(values.get('can_merge_without_human'))}`\n" + ) + if isinstance(first_next_action, dict) and first_next_action: + actor = clean(first_next_action.get("actor")) + kind = clean(first_next_action.get("kind")) + action = "/".join(part for part in (actor, kind) if part) + if action: + summary_file.write(f"- First next action: `{action}`\n") if agent_result: summary_file.write( f"- Decision: `{clean(agent_result.get('decision'))}`\n" @@ -254,6 +270,15 @@ def append_step_summary(output_dir: Path, values: dict[str, object]) -> None: f"- Tool-surface diff: {clean(tool_surface_diff.get('notes')[0])}\n" ) summary_file.write(f"- Report JSON: `{clean(values.get('report_json'))}`\n") + if values.get("verifier_json"): + summary_file.write( + f"- Verifier JSON: `{clean(values.get('verifier_json'))}`\n" + ) + if values.get("pr_comment_markdown"): + summary_file.write( + "- PR comment Markdown: " + f"`{clean(values.get('pr_comment_markdown'))}`\n" + ) def write_github_outputs(values: dict[str, object]) -> None: diff --git a/scripts/github_check_run.py b/scripts/github_check_run.py index 6d4fe0ba..1b1897d6 100644 --- a/scripts/github_check_run.py +++ b/scripts/github_check_run.py @@ -8,14 +8,16 @@ Mapping contract: -- ``merge_verdict`` → check conclusion: ``mergeable`` → ``success``, - ``blocked`` → ``failure``, everything else (``human_review_required``, - ``insufficient_evidence``, ``unknown``, missing) → ``neutral``. The - conclusion mirrors — never replaces — the one decision engine: +- ``merge_verdict`` + ``check_run_policy`` → check conclusion. The default + ``advisory`` policy preserves the historical mapping + (``mergeable`` → ``success``, ``blocked`` → ``failure``, everything else + → ``neutral``). ``require-mergeable`` is the branch-protection policy: + only ``can_merge_without_human=true`` succeeds. The conclusion mirrors — + never replaces — the one decision engine: ``report.json.release_decision.decision`` stays the gate. -- SARIF results → up to ``MAX_ANNOTATIONS`` line-level annotations - (Checks API caps one request at 50). ``error`` → ``failure``, - ``warning`` → ``warning``, anything else → ``notice``. +- PR projection items → up to ``MAX_ANNOTATIONS`` line-level annotations + (Checks API caps one request at 50). SARIF remains the full finding export; + Check Runs intentionally annotate only merge-relevant PR items. - ``pr-comment.md`` → check summary (truncated to the API limit). """ @@ -26,11 +28,20 @@ from pathlib import Path from typing import Any +from agents_shipgate.report.pr_projection import ( + item_to_check_run_annotation, + select_pr_items, +) + MAX_ANNOTATIONS = 50 # Checks API hard limit is 65535 chars for output.summary; keep headroom. MAX_SUMMARY_CHARS = 60000 PAYLOAD_FILENAME = "check-run-payload.json" DEFAULT_CHECK_NAME = "Agents Shipgate" +DEFAULT_CHECK_RUN_POLICY = "advisory" +CHECK_RUN_POLICIES = frozenset( + {"advisory", "blocked-fails", "require-mergeable"} +) _CONCLUSIONS = { "mergeable": "success", @@ -43,18 +54,38 @@ } -def conclusion_for(merge_verdict: object) -> str: +def conclusion_for( + merge_verdict: object, + *, + policy: str = DEFAULT_CHECK_RUN_POLICY, + can_merge_without_human: object = None, +) -> str: """Map a merge verdict onto a Checks API conclusion. - Unknown / missing verdicts are ``neutral``: the check must never - claim success for a verdict it does not understand, and must not - fail a PR on a verdict that routes to humans rather than blocking. + The default policy is advisory for backward compatibility. Strict branch + protection should use ``require-mergeable``. """ + normalized_policy = normalize_check_run_policy(policy) + if normalized_policy == "require-mergeable": + return "success" if _truthy(can_merge_without_human) else "failure" if not isinstance(merge_verdict, str): return "neutral" + if normalized_policy == "blocked-fails": + if merge_verdict == "mergeable": + return "success" + if merge_verdict in {"blocked", "unknown"}: + return "failure" + return "neutral" return _CONCLUSIONS.get(merge_verdict, "neutral") +def normalize_check_run_policy(policy: object) -> str: + normalized = str(policy or DEFAULT_CHECK_RUN_POLICY).strip().lower() + if normalized in CHECK_RUN_POLICIES: + return normalized + return DEFAULT_CHECK_RUN_POLICY + + def title_for(merge_verdict: object, *, blocker_count: int = 0) -> str: verdict = merge_verdict if isinstance(merge_verdict, str) and merge_verdict else "unknown" if verdict == "blocked" and blocker_count: @@ -63,12 +94,26 @@ def title_for(merge_verdict: object, *, blocker_count: int = 0) -> str: return f"merge_verdict: {verdict}" +def annotations_from_pr_projection( + report: dict[str, Any] | None, + verifier: dict[str, Any] | None, + *, + limit: int = MAX_ANNOTATIONS, +) -> list[dict[str, Any]]: + annotations: list[dict[str, Any]] = [] + for item in select_pr_items(report, verifier, limit=max(0, limit)): + if not item.source_path: + continue + annotations.append(item_to_check_run_annotation(item)) + return annotations + + def annotations_from_sarif(sarif: dict[str, Any] | None) -> list[dict[str, Any]]: """Project SARIF results onto Checks API annotations. - Deterministic: results are taken in SARIF order, capped at - ``MAX_ANNOTATIONS``. Only results with a resolvable file location - become annotations; the rest stay visible in the summary/report. + Compatibility fallback only. Normal check runs use + :func:`annotations_from_pr_projection` so job annotations and Check Run + annotations show the same merge-relevant items. """ if not isinstance(sarif, dict): return [] @@ -117,26 +162,34 @@ def _first_location(result: dict[str, Any]) -> tuple[str, int] | None: def build_check_run_payload( *, verifier: dict[str, Any] | None, - sarif: dict[str, Any] | None, + report: dict[str, Any] | None = None, + sarif: dict[str, Any] | None = None, summary_markdown: str, name: str = DEFAULT_CHECK_NAME, + check_run_policy: str = DEFAULT_CHECK_RUN_POLICY, ) -> dict[str, Any]: verifier = verifier or {} merge_verdict = verifier.get("merge_verdict") release_decision = verifier.get("release_decision") or {} blockers = release_decision.get("blockers") or [] - annotations = annotations_from_sarif(sarif) + annotations = annotations_from_pr_projection(report, verifier) + if not annotations and report is None: + annotations = annotations_from_sarif(sarif) total_results = _sarif_result_count(sarif) summary = summary_markdown.strip() or "No Shipgate summary was produced." if total_results > len(annotations): summary += ( f"\n\n_{len(annotations)} of {total_results} findings shown as " - "line annotations; see the uploaded report artifact for the " - "full list._" + "merge-relevant line annotations; see the uploaded report " + "artifact for the full list._" ) return { "name": name, - "conclusion": conclusion_for(merge_verdict), + "conclusion": conclusion_for( + merge_verdict, + policy=check_run_policy, + can_merge_without_human=verifier.get("can_merge_without_human"), + ), "output": { "title": title_for(merge_verdict, blocker_count=len(blockers)), "summary": summary[:MAX_SUMMARY_CHARS], @@ -161,10 +214,20 @@ def _load_json(path: Path) -> dict[str, Any] | None: return loaded if isinstance(loaded, dict) else None +def _truthy(value: object) -> bool: + if isinstance(value, bool): + return value + if isinstance(value, str): + return value.strip().lower() == "true" + return False + + def main() -> int: output_dir = Path(os.environ.get("OUTPUT_DIR") or "agents-shipgate-reports") name = os.environ.get("CHECK_RUN_NAME") or DEFAULT_CHECK_NAME + check_run_policy = normalize_check_run_policy(os.environ.get("CHECK_RUN_POLICY")) verifier = _load_json(output_dir / "verifier.json") + report = _load_json(output_dir / "report.json") sarif = _load_json(output_dir / "report.sarif") comment_path = output_dir / "pr-comment.md" summary_markdown = "" @@ -175,9 +238,11 @@ def main() -> int: summary_markdown = "" payload = build_check_run_payload( verifier=verifier, + report=report, sarif=sarif, summary_markdown=summary_markdown, name=name, + check_run_policy=check_run_policy, ) out_path = output_dir / PAYLOAD_FILENAME out_path.parent.mkdir(parents=True, exist_ok=True) diff --git a/src/agents_shipgate/cli/discovery/agent_instructions/renderers/agents_md.py b/src/agents_shipgate/cli/discovery/agent_instructions/renderers/agents_md.py index 948cd528..31c7d9a8 100644 --- a/src/agents_shipgate/cli/discovery/agent_instructions/renderers/agents_md.py +++ b/src/agents_shipgate/cli/discovery/agent_instructions/renderers/agents_md.py @@ -57,7 +57,7 @@ def render_block() -> str: `must_stop=true`, stop and surface the JSON result to a human. For committed PR/CI verification, run `agents-shipgate verify --base -origin/main --head HEAD --json` after making the base ref available; it never +origin/main --head HEAD --format json` after making the base ref available; it never fetches. Read `agents-shipgate-reports/agent-result.json` first, then `agents-shipgate-reports/verifier.json` for `merge_verdict` and `agents-shipgate-reports/report.json.release_decision.decision` for the release diff --git a/src/agents_shipgate/cli/discovery/agent_instructions/renderers/claude_md.py b/src/agents_shipgate/cli/discovery/agent_instructions/renderers/claude_md.py index f143b300..49a6fd40 100644 --- a/src/agents_shipgate/cli/discovery/agent_instructions/renderers/claude_md.py +++ b/src/agents_shipgate/cli/discovery/agent_instructions/renderers/claude_md.py @@ -47,7 +47,7 @@ def render_block() -> str: `first_next_action.actor` is `human`, stop and route the change to a human. For committed PR/CI verification, run `agents-shipgate verify --base -origin/main --head HEAD --json` after making the base ref available; it never +origin/main --head HEAD --format json` after making the base ref available; it never fetches. Read `agents-shipgate-reports/agent-result.json` first, then `agents-shipgate-reports/verifier.json` for `merge_verdict` and `agents-shipgate-reports/report.json.release_decision.decision` for the release diff --git a/src/agents_shipgate/cli/discovery/agent_instructions/renderers/cursor.py b/src/agents_shipgate/cli/discovery/agent_instructions/renderers/cursor.py index 5ad7f3f9..6f14a82a 100644 --- a/src/agents_shipgate/cli/discovery/agent_instructions/renderers/cursor.py +++ b/src/agents_shipgate/cli/discovery/agent_instructions/renderers/cursor.py @@ -74,7 +74,7 @@ def render_file() -> str: `first_next_action.actor` is `human`, stop and route the change to a human. For committed PR/CI verification, run `agents-shipgate verify --base -origin/main --head HEAD --json` after making the base ref available; it never +origin/main --head HEAD --format json` after making the base ref available; it never fetches. Read `agents-shipgate-reports/agent-result.json` first, then `agents-shipgate-reports/verifier.json` for `merge_verdict` and `agents-shipgate-reports/report.json.release_decision.decision` for the release diff --git a/src/agents_shipgate/cli/discovery/ci_workflow.py b/src/agents_shipgate/cli/discovery/ci_workflow.py index 44f6f122..f67707fb 100644 --- a/src/agents_shipgate/cli/discovery/ci_workflow.py +++ b/src/agents_shipgate/cli/discovery/ci_workflow.py @@ -53,26 +53,31 @@ def _action_ref() -> str: on: pull_request: branches: [main] - push: - branches: [main] permissions: contents: read - pull-requests: write # only used when pr_comment: true; harmless otherwise + pull-requests: write # required for pr_comment: "true" + # checks: write # uncomment when enabling check_run: "true" jobs: shipgate: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 + with: + fetch-depth: 0 - name: Run Agents Shipgate uses: ThreeMoonsLab/agents-shipgate@{ref} with: config: shipgate.yaml ci_mode: advisory # change to "strict" once findings are clean + diff_base: target + check_annotations: "true" + pr_comment: "true" # fail_on: critical,high # baseline: .agents-shipgate/baseline.json - # pr_comment: "true" + # check_run: "true" + # check_run_policy: require-mergeable """ diff --git a/src/agents_shipgate/cli/verify/orchestrator.py b/src/agents_shipgate/cli/verify/orchestrator.py index 5ce9fac0..80e5eaf7 100644 --- a/src/agents_shipgate/cli/verify/orchestrator.py +++ b/src/agents_shipgate/cli/verify/orchestrator.py @@ -1168,7 +1168,7 @@ def _preview_verify_command( parts.extend(["--head", head]) if out is not None: parts.extend(["--out", str(out)]) - parts.extend(["--ci-mode", "advisory", "--json"]) + parts.extend(["--ci-mode", "advisory", "--format", "json"]) return _shell_join(parts) diff --git a/src/agents_shipgate/report/pr_projection.py b/src/agents_shipgate/report/pr_projection.py new file mode 100644 index 00000000..32e2a485 --- /dev/null +++ b/src/agents_shipgate/report/pr_projection.py @@ -0,0 +1,356 @@ +from __future__ import annotations + +from dataclasses import asdict, dataclass +from typing import Any + +PR_PROJECTION_SCHEMA_VERSION = "0.1" + +_SEVERITY_ORDER = { + "critical": 0, + "high": 1, + "medium": 2, + "low": 3, + "info": 4, +} +_IMPACT_TO_SEVERITY = { + "blocks_release": "critical", + "insufficient_evidence": "high", + "review_required": "high", + "informational": "low", + "none": "info", +} + + +@dataclass(frozen=True) +class PrReviewItem: + """One deterministic PR-facing item projected from report/verifier facts.""" + + check_id: str + title: str + severity: str + level: str + message: str + recommendation: str + source_path: str | None = None + source_start_line: int | None = None + source_end_line: int | None = None + source_start_column: int | None = None + selector: str | None = None + finding_id: str | None = None + fingerprint: str | None = None + merge_impact: str | None = None + related_finding_ids: tuple[str, ...] = () + capability_subject: str | None = None + + def to_dict(self) -> dict[str, Any]: + payload = asdict(self) + payload["related_finding_ids"] = list(self.related_finding_ids) + return payload + + +def select_pr_items( + report: dict[str, Any] | None, + verifier: dict[str, Any] | None = None, + *, + limit: int = 50, +) -> list[PrReviewItem]: + """Return prioritized PR review items from existing gate artifacts. + + Selection is intentionally a projection only: release-decision blockers and + review items come first, source-backed capability changes can add reviewer + context, and active critical/high findings are only a fallback for legacy or + malformed reports that lack a release-decision block. + """ + + report = report if isinstance(report, dict) else {} + verifier = verifier if isinstance(verifier, dict) else {} + normalized_limit = max(0, limit) + findings = _active_findings(report) + by_id, by_fingerprint = _finding_indexes(findings) + selected: list[PrReviewItem] = [] + seen = set[str]() + + release_decision = report.get("release_decision") or {} + for bucket in ("blockers", "review_items"): + for decision_item in release_decision.get(bucket) or []: + if not isinstance(decision_item, dict): + continue + finding = _matching_finding(decision_item, by_id, by_fingerprint) + item = _item_from_finding(finding or decision_item, decision_item) + _append_unique(selected, seen, item) + + for change in _top_capability_changes(verifier): + item = _item_from_capability_change(change) + if item.source_path: + _append_unique(selected, seen, item) + + if not selected: + for finding in sorted(_critical_high_findings(findings), key=_finding_sort_key): + _append_unique(selected, seen, _item_from_finding(finding)) + + return selected[:normalized_limit] + + +def item_to_action_annotation(item: PrReviewItem) -> dict[str, Any]: + """Project one PR item onto a GitHub Actions workflow annotation.""" + + return { + "level": item.level, + "path": item.source_path, + "start_line": item.source_start_line, + "end_line": item.source_end_line, + "start_column": item.source_start_column, + "selector": item.selector, + "title": _truncate(f"{item.check_id}: {item.title}", 160), + "message": _truncate(item.message, 1000), + "check_id": item.check_id, + "severity": item.severity, + "finding_id": item.finding_id, + "fingerprint": item.fingerprint, + "merge_impact": item.merge_impact, + "capability_subject": item.capability_subject, + "related_finding_ids": list(item.related_finding_ids), + } + + +def item_to_check_run_annotation(item: PrReviewItem) -> dict[str, Any]: + """Project one PR item onto a GitHub Checks API annotation.""" + + line = item.source_start_line if isinstance(item.source_start_line, int) else 1 + return { + "path": item.source_path, + "start_line": line, + "end_line": item.source_end_line or line, + "annotation_level": _check_run_level(item.level), + "message": _truncate(item.message, 1000), + "title": _truncate(item.check_id or "agents-shipgate", 255), + } + + +def _item_from_finding( + finding: dict[str, Any], + decision_item: dict[str, Any] | None = None, +) -> PrReviewItem: + decision_item = decision_item if isinstance(decision_item, dict) else {} + merged = {**decision_item, **finding} + source = _best_source(merged) + path = str(source.get("path")) if source and source.get("path") else None + selector = _selector(source, path) if source and path else None + severity = str(merged.get("severity") or "info") + recommendation = str(merged.get("recommendation") or "") + title = str(merged.get("title") or merged.get("check_id") or "Shipgate finding") + message = recommendation or title + if selector: + message = f"{message} Source: {selector}" + return PrReviewItem( + check_id=str(merged.get("check_id") or "agents-shipgate"), + title=title, + severity=severity, + level=_action_level(severity), + message=message, + recommendation=recommendation, + source_path=path, + source_start_line=_int_or_none(source.get("start_line") if source else None), + source_end_line=_int_or_none(source.get("end_line") if source else None), + source_start_column=_int_or_none( + source.get("start_column") if source else None + ), + selector=selector, + finding_id=_str_or_none(merged.get("id")), + fingerprint=_str_or_none(merged.get("fingerprint")), + merge_impact=( + "blocks_release" if bool(merged.get("blocks_release")) else None + ), + related_finding_ids=tuple(_string_list(merged.get("related_finding_ids"))), + capability_subject=_capability_subject(merged), + ) + + +def _item_from_capability_change(change: dict[str, Any]) -> PrReviewItem: + impact = str(change.get("impact") or "informational") + severity = _IMPACT_TO_SEVERITY.get(impact, "info") + subject_kind = str(change.get("subject_kind") or "capability") + subject = str(change.get("subject") or change.get("id") or "unknown") + change_type = str(change.get("change_type") or "changed") + title = f"{subject_kind} {change_type}: {subject}" + rationale = str(change.get("rationale") or title) + source_path = _str_or_none(change.get("source_path")) + line = _int_or_none(change.get("source_start_line")) + return PrReviewItem( + check_id="SHIP-CAPABILITY-CHANGE", + title=title, + severity=severity, + level=_action_level(severity), + message=_truncate(rationale, 1000), + recommendation=rationale, + source_path=source_path, + source_start_line=line, + source_end_line=line, + selector=source_path, + finding_id=None, + fingerprint=None, + merge_impact=impact, + related_finding_ids=tuple(_string_list(change.get("related_finding_ids"))), + capability_subject=f"{subject_kind}:{subject}", + ) + + +def _active_findings(report: dict[str, Any]) -> list[dict[str, Any]]: + return [ + finding + for finding in report.get("findings") or [] + if isinstance(finding, dict) and not finding.get("suppressed") + ] + + +def _finding_indexes( + findings: list[dict[str, Any]], +) -> tuple[dict[str, dict[str, Any]], dict[str, dict[str, Any]]]: + by_id = {str(finding.get("id")): finding for finding in findings if finding.get("id")} + by_fingerprint = { + str(finding.get("fingerprint")): finding + for finding in findings + if finding.get("fingerprint") + } + return by_id, by_fingerprint + + +def _matching_finding( + item: dict[str, Any], + by_id: dict[str, dict[str, Any]], + by_fingerprint: dict[str, dict[str, Any]], +) -> dict[str, Any] | None: + finding_id = item.get("id") + if finding_id is not None and str(finding_id) in by_id: + return by_id[str(finding_id)] + fingerprint = item.get("fingerprint") + if fingerprint is not None and str(fingerprint) in by_fingerprint: + return by_fingerprint[str(fingerprint)] + return None + + +def _top_capability_changes(verifier: dict[str, Any]) -> list[dict[str, Any]]: + review = verifier.get("capability_review") or {} + return [ + change + for change in review.get("top_changes") or [] + if isinstance(change, dict) + ] + + +def _critical_high_findings(findings: list[dict[str, Any]]) -> list[dict[str, Any]]: + return [ + finding + for finding in findings + if str(finding.get("severity") or "") in {"critical", "high"} + ] + + +def _append_unique( + selected: list[PrReviewItem], + seen: set[str], + item: PrReviewItem, +) -> None: + key = _item_key(item) + if key in seen: + return + selected.append(item) + seen.add(key) + + +def _item_key(item: PrReviewItem) -> str: + if item.finding_id: + return f"finding-id:{item.finding_id}" + if item.fingerprint: + return f"fingerprint:{item.fingerprint}" + if item.related_finding_ids: + return "related:" + ",".join(item.related_finding_ids) + return "|".join( + [ + item.check_id, + item.title, + item.source_path or "", + str(item.source_start_line or ""), + item.capability_subject or "", + ] + ) + + +def _finding_sort_key(finding: dict[str, Any]) -> tuple[int, str, str]: + return ( + _SEVERITY_ORDER.get(str(finding.get("severity") or ""), 99), + str(finding.get("check_id") or ""), + str(finding.get("title") or ""), + ) + + +def _best_source(item: dict[str, Any]) -> dict[str, Any] | None: + for key in ("source", "policy_evidence_source"): + source = item.get(key) + if isinstance(source, dict) and source.get("path"): + return source + return None + + +def _selector(source: dict[str, Any], path: str | None) -> str | None: + if path is None: + return None + pointer = source.get("pointer") + if pointer is not None: + return f"{path}#{pointer}" + location = source.get("location") + if location: + return str(location) + ref = source.get("ref") + if ref: + return str(ref) + return path + + +def _capability_subject(item: dict[str, Any]) -> str | None: + refs = item.get("capability_refs") + if isinstance(refs, list) and refs: + return ",".join(str(ref) for ref in refs if str(ref)) + tool_name = item.get("tool_name") + if tool_name: + return str(tool_name) + return None + + +def _action_level(severity: str) -> str: + if severity in {"critical", "high"}: + return "error" + if severity == "medium": + return "warning" + return "notice" + + +def _check_run_level(action_level: str) -> str: + if action_level == "error": + return "failure" + if action_level == "warning": + return "warning" + return "notice" + + +def _string_list(value: object) -> list[str]: + if not isinstance(value, list): + return [] + return [str(item) for item in value if str(item)] + + +def _str_or_none(value: object) -> str | None: + if value is None: + return None + text = str(value) + return text or None + + +def _int_or_none(value: object) -> int | None: + if isinstance(value, int): + return value + return None + + +def _truncate(value: str, limit: int) -> str: + return value if len(value) <= limit else value[: limit - 1] + "..." diff --git a/src/agents_shipgate/schemas/contract.py b/src/agents_shipgate/schemas/contract.py index 99cb366b..847e870c 100644 --- a/src/agents_shipgate/schemas/contract.py +++ b/src/agents_shipgate/schemas/contract.py @@ -105,11 +105,12 @@ "agents-shipgate init --workspace . --write --ci --agent-instructions=default --json" ), "verify_local": ( - "agents-shipgate verify --workspace . --config shipgate.yaml --ci-mode advisory --json" + "agents-shipgate verify --workspace . --config shipgate.yaml " + "--ci-mode advisory --format json" ), "verify_pr": ( "agents-shipgate verify --workspace . --config shipgate.yaml " - "--base origin/main --head HEAD --ci-mode advisory --json" + "--base origin/main --head HEAD --ci-mode advisory --format json" ), "contract": "agents-shipgate contract --json", } diff --git a/src/agents_shipgate/triggers.py b/src/agents_shipgate/triggers.py index 638c4d74..36692b2d 100644 --- a/src/agents_shipgate/triggers.py +++ b/src/agents_shipgate/triggers.py @@ -152,7 +152,10 @@ def _next_action( if manifest_present: return { "kind": "command", - "command": "agents-shipgate verify --base origin/main --head HEAD --json", + "command": ( + "agents-shipgate verify --workspace . --config shipgate.yaml " + "--base origin/main --head HEAD --ci-mode advisory --format json" + ), "why": ( "This change affects an agent tool or release-policy " "surface; verify whether the PR can merge." @@ -171,8 +174,8 @@ def _next_action( } if dry_run_recommended: command = ( - "agents-shipgate verify --base origin/main --head HEAD " - "--ci-mode advisory --json" + "agents-shipgate verify --workspace . --config shipgate.yaml " + "--base origin/main --head HEAD --ci-mode advisory --format json" if manifest_present else default_command ) diff --git a/tests/test_action_metadata.py b/tests/test_action_metadata.py index af753ae5..d78c0a08 100644 --- a/tests/test_action_metadata.py +++ b/tests/test_action_metadata.py @@ -69,6 +69,8 @@ def test_action_has_marketplace_metadata_and_outputs(): assert data["inputs"]["check_annotations"]["default"] == "true" assert data["inputs"]["check_annotation_limit"]["default"] == "50" assert data["inputs"]["check_run"]["default"] == "false" + assert data["inputs"]["check_run_policy"]["default"] == "advisory" + assert "require-mergeable" in data["inputs"]["check_run_policy"]["description"] assert data["inputs"]["check_run_name"]["default"] == "Agents Shipgate" assert data["inputs"]["pr_comment_style"]["default"] == "capability-review" assert "legacy v1 findings comment" in data["inputs"]["pr_comment_style"]["description"] @@ -125,6 +127,7 @@ def test_action_preserves_reports_before_applying_exit_code(): assert "agent-result.json did not expose an agent decision" in text assert "scripts/github_check_run.py" in text assert "check-run-payload.json" in text + assert "CHECK_RUN_POLICY: ${{ inputs.check_run_policy }}" in text assert "verify" in text assert "scan" in text assert "--workspace" in text @@ -141,13 +144,16 @@ def test_action_preserves_reports_before_applying_exit_code(): assert "args+=(--no-plugins)" in text -def test_action_step_summary_leads_with_release_decision(): +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") assert "scripts/github_action_outputs.py" in text assert "GITHUB_STEP_SUMMARY" in script assert "## Agents Shipgate" in script + assert "Merge verdict:" in script + assert "Can merge without human:" in script + assert "First next action:" in script assert "Decision:" in script assert "Risk:" in script assert "Audit ID:" in script @@ -162,6 +168,8 @@ def test_action_pr_comment_truncates_user_controlled_text(): assert "pr-comment.md" in text assert "fs.readFileSync(commentPath" in text assert ".slice(0, 6000)" in text + assert "Workflow artifacts:" in text + assert "body.length + artifactLine.length <= 6000" in text assert "preferredDiff" not in text diff --git a/tests/test_adapter_static_only.py b/tests/test_adapter_static_only.py index e03b23e0..1d81bb4c 100644 --- a/tests/test_adapter_static_only.py +++ b/tests/test_adapter_static_only.py @@ -237,7 +237,7 @@ class AllowedException: AllowedException( relative_path="triggers.py", surface="attr_call:subprocess.run", - line=480, + line=483, snippet="subprocess.run(names_cmd, **run_kwargs)", rationale=( "git-diff change-name pass: ``git diff --name-only " @@ -249,7 +249,7 @@ class AllowedException: AllowedException( relative_path="triggers.py", surface="attr_call:subprocess.run", - line=481, + line=484, snippet="subprocess.run(body_cmd, **run_kwargs)", rationale=( "git-diff body pass: ``git diff base...HEAD`` for full " @@ -260,7 +260,7 @@ class AllowedException: AllowedException( relative_path="triggers.py", surface="attr_call:subprocess.run", - line=486, + line=489, snippet=( "subprocess.run(['git', 'ls-files', '--others', " "'--exclude-standard'], **run_kwargs)" diff --git a/tests/test_github_action_annotations.py b/tests/test_github_action_annotations.py index b4040512..05051310 100644 --- a/tests/test_github_action_annotations.py +++ b/tests/test_github_action_annotations.py @@ -62,6 +62,8 @@ def test_annotations_select_source_backed_blockers_and_review_items( payload = build_annotations(output_dir, limit=10) + assert payload["pr_projection_schema_version"] == "0.1" + assert payload["source_verifier"].endswith("verifier.json") assert [item["check_id"] for item in payload["annotations"]] == [ "SHIP-ACTION-APPROVAL-REMOVED", "SHIP-POLICY-APPROVAL-MISSING", @@ -105,3 +107,45 @@ def test_annotations_respect_limit(tmp_path: Path) -> None: assert zero_payload["annotations"] == [] assert zero_payload["omitted"]["limit"] == 3 + + +def test_annotations_include_source_backed_capability_changes(tmp_path: Path) -> None: + output_dir = tmp_path / "agents-shipgate-reports" + output_dir.mkdir() + (output_dir / "report.json").write_text( + json.dumps({"release_decision": {}, "findings": []}), + encoding="utf-8", + ) + (output_dir / "verifier.json").write_text( + json.dumps( + { + "capability_review": { + "top_changes": [ + { + "id": "cap-refund", + "change_type": "action_added", + "subject_kind": "action", + "subject": "stripe.create_refund", + "impact": "blocks_release", + "rationale": "Action added: stripe.create_refund", + "source_path": "api.yaml", + "source_start_line": 42, + "related_finding_ids": ["F-refund"], + } + ] + } + } + ), + encoding="utf-8", + ) + + payload = build_annotations(output_dir, limit=10) + + assert len(payload["annotations"]) == 1 + annotation = payload["annotations"][0] + assert annotation["check_id"] == "SHIP-CAPABILITY-CHANGE" + assert annotation["path"] == "api.yaml" + assert annotation["start_line"] == 42 + assert annotation["level"] == "error" + assert annotation["merge_impact"] == "blocks_release" + assert annotation["capability_subject"] == "action:stripe.create_refund" diff --git a/tests/test_github_action_outputs.py b/tests/test_github_action_outputs.py index 09d13cb4..663a7588 100644 --- a/tests/test_github_action_outputs.py +++ b/tests/test_github_action_outputs.py @@ -6,6 +6,7 @@ import pytest from scripts.github_action_outputs import ( + append_step_summary, decision_policy_exit_code, extract_outputs, trigger_action, @@ -194,6 +195,46 @@ def test_action_outputs_include_agent_result_fields(tmp_path: Path) -> None: assert outputs["policy_snapshot_sha256"] == "b" * 64 +def test_step_summary_leads_with_verifier_merge_state( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + output_dir = tmp_path / "agents-shipgate-reports" + output_dir.mkdir() + summary_path = tmp_path / "summary.md" + monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary_path)) + _write_json( + output_dir / "report.json", + { + "summary": {"status": "release_blockers_detected"}, + "release_decision": { + "decision": "blocked", + "blockers": [{"id": "F1"}], + "review_items": [{"id": "F2"}], + "fail_policy": {"would_fail_ci": True, "exit_code": 20}, + }, + }, + ) + _write_json( + output_dir / "verifier.json", + { + "merge_verdict": "blocked", + "can_merge_without_human": False, + "first_next_action": {"actor": "human", "kind": "review"}, + }, + ) + values = extract_outputs(output_dir) + + append_step_summary(output_dir, values) + + text = summary_path.read_text(encoding="utf-8") + assert text.index("Merge verdict: `blocked`") < text.index("Release gate: `blocked`") + assert "Can merge without human: `false`" in text + assert "First next action: `human/review`" in text + assert f"Verifier JSON: `{output_dir / 'verifier.json'}`" in text + assert f"PR comment Markdown: `{output_dir / 'pr-comment.md'}`" in text + + def test_decision_policy_exit_code_is_opt_in() -> None: assert decision_policy_exit_code("block", "") == 0 assert decision_policy_exit_code("block", "block") == 20 diff --git a/tests/test_github_check_run.py b/tests/test_github_check_run.py index 4d93f7f8..932f3c25 100644 --- a/tests/test_github_check_run.py +++ b/tests/test_github_check_run.py @@ -6,6 +6,7 @@ from scripts.github_check_run import ( MAX_ANNOTATIONS, + annotations_from_pr_projection, annotations_from_sarif, build_check_run_payload, conclusion_for, @@ -30,6 +31,42 @@ def test_conclusion_mapping(verdict, expected): assert conclusion_for(verdict) == expected +@pytest.mark.parametrize( + ("verdict", "expected"), + [ + ("mergeable", "success"), + ("blocked", "failure"), + ("unknown", "failure"), + ("human_review_required", "neutral"), + ("insufficient_evidence", "neutral"), + ("future", "neutral"), + ], +) +def test_blocked_fails_conclusion_mapping(verdict, expected): + assert conclusion_for(verdict, policy="blocked-fails") == expected + + +@pytest.mark.parametrize( + ("can_merge_without_human", "expected"), + [ + (True, "success"), + ("true", "success"), + (False, "failure"), + ("false", "failure"), + (None, "failure"), + ], +) +def test_require_mergeable_conclusion_mapping(can_merge_without_human, expected): + assert ( + conclusion_for( + "mergeable", + policy="require-mergeable", + can_merge_without_human=can_merge_without_human, + ) + == expected + ) + + def test_title_includes_blocker_count_when_blocked(): assert title_for("blocked", blocker_count=2) == "merge_verdict: blocked (2 blockers)" assert title_for("blocked", blocker_count=1) == "merge_verdict: blocked (1 blocker)" @@ -126,6 +163,68 @@ def test_payload_defaults_safe_when_artifacts_missing(): assert payload["output"]["summary"] +def test_payload_uses_pr_projection_annotations_before_sarif(): + report = { + "release_decision": {"blockers": [{"id": "F1"}]}, + "findings": [ + { + "id": "F1", + "check_id": "SHIP-ACTION-APPROVAL-REMOVED", + "title": "Approval removed", + "severity": "critical", + "recommendation": "Restore approval.", + "source": {"path": "api.yaml", "start_line": 12}, + } + ], + } + verifier = { + "merge_verdict": "blocked", + "can_merge_without_human": False, + "release_decision": {"blockers": [{"id": "F1"}]}, + } + sarif = _sarif([_result(uri="other.yaml", line=99)]) + + payload = build_check_run_payload( + verifier=verifier, + report=report, + sarif=sarif, + summary_markdown="blocked", + ) + + assert payload["conclusion"] == "failure" + assert payload["output"]["annotations"][0]["path"] == "api.yaml" + assert payload["output"]["annotations"][0]["title"] == "SHIP-ACTION-APPROVAL-REMOVED" + + +def test_pr_projection_annotations_match_action_projection_shape(): + report = { + "release_decision": {"review_items": [{"id": "F2"}]}, + "findings": [ + { + "id": "F2", + "check_id": "SHIP-POLICY-APPROVAL-MISSING", + "title": "Approval missing", + "severity": "high", + "recommendation": "Declare approval.", + "policy_evidence_source": {"path": "shipgate.yaml", "start_line": 20}, + } + ], + } + + annotations = annotations_from_pr_projection(report, {"merge_verdict": "blocked"}) + + assert annotations == [ + { + "path": "shipgate.yaml", + "start_line": 20, + "end_line": 20, + "annotation_level": "failure", + "message": "Declare approval. Source: shipgate.yaml", + "title": "SHIP-POLICY-APPROVAL-MISSING", + } + ] + + def test_payload_is_json_serializable_and_deterministic(): sarif = _sarif([_result()]) verifier = {"merge_verdict": "mergeable", "release_decision": {"blockers": []}} diff --git a/tests/test_init_ci.py b/tests/test_init_ci.py index 8a3ec821..08d3a663 100644 --- a/tests/test_init_ci.py +++ b/tests/test_init_ci.py @@ -52,6 +52,16 @@ def test_write_ci_workflow_writes_to_fresh_workspace(tmp_path: Path) -> None: from agents_shipgate import __version__ assert f"ThreeMoonsLab/agents-shipgate@v{__version__}" in content assert "ci_mode: advisory" in content + assert "pull_request:" in content + assert "push:" not in content + assert "pull-requests: write" in content + assert "# checks: write" in content + assert "fetch-depth: 0" in content + assert "diff_base: target" in content + assert 'check_annotations: "true"' in content + assert 'pr_comment: "true"' in content + assert '# check_run: "true"' in content + assert "# check_run_policy: require-mergeable" in content def test_write_ci_workflow_refuses_overwrite(tmp_path: Path) -> None: @@ -217,5 +227,13 @@ def test_workflow_template_references_action_yml_input_names(tmp_path: Path) -> write_ci_workflow(tmp_path) content = (tmp_path / WORKFLOW_RELATIVE_PATH).read_text(encoding="utf-8") # Required inputs from action.yml that appear in the template: - for token in ("config:", "ci_mode:"): + for token in ( + "config:", + "ci_mode:", + "diff_base:", + "check_annotations:", + "pr_comment:", + ): assert token in content + for opt_in in ("fail_on:", "baseline:", "check_run:", "check_run_policy:"): + assert f"# {opt_in}" in content diff --git a/tests/test_public_surface_contract.py b/tests/test_public_surface_contract.py index 142a5935..9649d2e4 100644 --- a/tests/test_public_surface_contract.py +++ b/tests/test_public_surface_contract.py @@ -1360,6 +1360,18 @@ def test_well_known_seo_geo_positioning_fields_are_pinned(): assert commands.get("install_ai_coding_workflow") == ( "agents-shipgate init --workspace . --write --ci --agent-instructions=default --json" ) + assert commands.get("verify_pr", "").endswith("--format json") + assert data.get("check_run_policies") == [ + "advisory", + "blocked-fails", + "require-mergeable", + ] + assert ( + data.get("github_action_pr_workflow", {}) + .get("recommended_inputs", {}) + .get("diff_base") + == "target" + ) assert "feedback export" in commands.get("feedback_export", "") assert data.get("fixture_run") == "agents-shipgate fixture run ai_generated_refund_pr" assert data.get("static_scan_fixture_run") == ( diff --git a/tests/test_verify.py b/tests/test_verify.py index fed19a8d..c1d8c252 100644 --- a/tests/test_verify.py +++ b/tests/test_verify.py @@ -1243,7 +1243,7 @@ def test_verify_preview_configured_repo_preserves_exact_verify_args(tmp_path: Pa assert payload["mode"] == "preview" assert payload["first_next_action"]["command"] == ( f"agents-shipgate verify --workspace {repo} --config shipgate.yaml " - f"--base origin/main --head HEAD --out {out} --ci-mode advisory --json" + f"--base origin/main --head HEAD --out {out} --ci-mode advisory --format json" ) From 71c7460576e3bda5b57cc84c6f71b792a90150ac Mon Sep 17 00:00:00 2001 From: Pengfei Hu Date: Wed, 17 Jun 2026 15:51:02 -0700 Subject: [PATCH 2/2] Address PR workflow review feedback --- .cursor/rules/agents-shipgate.mdc | 2 +- README.md | 2 +- docs/agents/use-with-claude-code.md | 2 +- docs/agents/use-with-codex.md | 2 +- docs/agents/use-with-cursor.md | 2 +- .../ai-coding-workflow-verifier.md | 2 +- docs/target-repo-agent-snippets.md | 8 +-- docs/use-cases/ai-generated-agent-prs.md | 6 +-- .../10-check-run-annotations.yml | 8 ++- examples/github-actions/README.md | 8 ++- scripts/github_check_run.py | 8 +-- .../agent_instructions/renderers/agents_md.py | 2 +- .../agent_instructions/renderers/claude_md.py | 2 +- .../agent_instructions/renderers/cursor.py | 2 +- .../cli/verify/orchestrator.py | 2 +- src/agents_shipgate/report/pr_projection.py | 5 ++ src/agents_shipgate/schemas/contract.py | 5 +- src/agents_shipgate/triggers.py | 9 ++-- tests/test_adapter_static_only.py | 6 +-- tests/test_github_action_annotations.py | 53 +++++++++++++++++++ tests/test_public_surface_contract.py | 1 - tests/test_verify.py | 2 +- 22 files changed, 101 insertions(+), 38 deletions(-) diff --git a/.cursor/rules/agents-shipgate.mdc b/.cursor/rules/agents-shipgate.mdc index f2917d7b..68e5514d 100644 --- a/.cursor/rules/agents-shipgate.mdc +++ b/.cursor/rules/agents-shipgate.mdc @@ -58,7 +58,7 @@ plugin manifests, `.mcp.json`, `.app.json`, or `SKILL.md`, run `first_next_action.actor` is `human`, stop and route the change to a human. For committed PR/CI verification, run `agents-shipgate verify --base -origin/main --head HEAD --format json` after making the base ref available; it never +origin/main --head HEAD --json` after making the base ref available; it never fetches. Read `agents-shipgate-reports/agent-result.json` first, then `agents-shipgate-reports/verifier.json` for `merge_verdict` and `agents-shipgate-reports/report.json.release_decision.decision` for the release diff --git a/README.md b/README.md index 90d49d3d..e7c5722a 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,7 @@ the coding-agent diff that adds `stripe.create_refund` to a support agent > `safeguards.idempotency` for this financial write action. > 3. Replace wildcard/admin scopes with operation-specific scopes. > -> Then re-verify: `agents-shipgate verify --base origin/main --head HEAD --format json` +> Then re-verify: `agents-shipgate verify --base origin/main --head HEAD --json` The same `uvx agents-shipgate fixture run ai_generated_refund_pr` command above writes this comment verbatim to `reports/pr-comment.md`. diff --git a/docs/agents/use-with-claude-code.md b/docs/agents/use-with-claude-code.md index a1707433..885cec5e 100644 --- a/docs/agents/use-with-claude-code.md +++ b/docs/agents/use-with-claude-code.md @@ -128,7 +128,7 @@ reporting the change as complete, then run `verify` for PR/reviewer evidence: ```bash shipgate check --agent claude-code --workspace . --format agent-json agents-shipgate preflight --json -agents-shipgate verify --base origin/main --head HEAD --format json +agents-shipgate verify --base origin/main --head HEAD --json ``` If preflight returns `requires_human_review: true`, Claude Code must stop for a diff --git a/docs/agents/use-with-codex.md b/docs/agents/use-with-codex.md index 17e8a0dd..7bb9c146 100644 --- a/docs/agents/use-with-codex.md +++ b/docs/agents/use-with-codex.md @@ -211,7 +211,7 @@ verifier before claiming the work is done: ```bash agents-shipgate preflight --json -agents-shipgate verify --base origin/main --head HEAD --format json +agents-shipgate verify --base origin/main --head HEAD --json ``` If preflight returns `requires_human_review: true`, Codex must stop for a human diff --git a/docs/agents/use-with-cursor.md b/docs/agents/use-with-cursor.md index 1db0132f..58014b2c 100644 --- a/docs/agents/use-with-cursor.md +++ b/docs/agents/use-with-cursor.md @@ -85,7 +85,7 @@ treating the change as finished, then run `verify` for PR/reviewer evidence: ```bash shipgate check --agent cursor --workspace . --format agent-json agents-shipgate preflight --json -agents-shipgate verify --base origin/main --head HEAD --format json +agents-shipgate verify --base origin/main --head HEAD --json ``` If preflight returns `requires_human_review: true`, Cursor must stop for a human diff --git a/docs/engineering/ai-coding-workflow-verifier.md b/docs/engineering/ai-coding-workflow-verifier.md index 766cf585..15911871 100644 --- a/docs/engineering/ai-coding-workflow-verifier.md +++ b/docs/engineering/ai-coding-workflow-verifier.md @@ -628,7 +628,7 @@ Acceptance criteria: "Do not lower severity.", "Do not mark approval_policy: present without evidence." ], - "verification_command": "agents-shipgate verify --base origin/main --head HEAD --format json" + "verification_command": "agents-shipgate verify --base origin/main --head HEAD --json" } } ``` diff --git a/docs/target-repo-agent-snippets.md b/docs/target-repo-agent-snippets.md index 002ef104..230c6147 100644 --- a/docs/target-repo-agent-snippets.md +++ b/docs/target-repo-agent-snippets.md @@ -76,7 +76,7 @@ repair and rerun the command. If `human_review.required=true` or `must_stop=true`, stop and surface the JSON result to a human. For committed PR/CI verification, run `agents-shipgate verify --base -origin/main --head HEAD --format json` after making the base ref available; it never +origin/main --head HEAD --json` after making the base ref available; it never fetches. Read `agents-shipgate-reports/agent-result.json` first, then `agents-shipgate-reports/verifier.json` for `merge_verdict` and `agents-shipgate-reports/report.json.release_decision.decision` for the release @@ -177,7 +177,7 @@ plugin manifests, `.mcp.json`, `.app.json`, or `SKILL.md`, run `first_next_action.actor` is `human`, stop and route the change to a human. For committed PR/CI verification, run `agents-shipgate verify --base -origin/main --head HEAD --format json` after making the base ref available; it never +origin/main --head HEAD --json` after making the base ref available; it never fetches. Read `agents-shipgate-reports/agent-result.json` first, then `agents-shipgate-reports/verifier.json` for `merge_verdict` and `agents-shipgate-reports/report.json.release_decision.decision` for the release @@ -255,7 +255,7 @@ plugin manifests, `.mcp.json`, `.app.json`, or `SKILL.md`, run `first_next_action.actor` is `human`, stop and route the change to a human. For committed PR/CI verification, run `agents-shipgate verify --base -origin/main --head HEAD --format json` after making the base ref available; it never +origin/main --head HEAD --json` after making the base ref available; it never fetches. Read `agents-shipgate-reports/agent-result.json` first, then `agents-shipgate-reports/verifier.json` for `merge_verdict` and `agents-shipgate-reports/report.json.release_decision.decision` for the release @@ -369,7 +369,7 @@ companion to the bootstrap snippets above: ```text Before claiming completion on any PR that changes agent tools, MCP exports, OpenAPI specs, prompts, permissions, policies, CI gates, or shipgate.yaml, run: -`agents-shipgate verify --base origin/main --head HEAD --format json`. Read +`agents-shipgate verify --base origin/main --head HEAD --json`. Read agents-shipgate-reports/verifier.json first: merge_verdict, can_merge_without_human, first_next_action, fix_task, and capability_review.top_changes. Then read report.json.release_decision.decision; diff --git a/docs/use-cases/ai-generated-agent-prs.md b/docs/use-cases/ai-generated-agent-prs.md index 2c461a34..88ffaa26 100644 --- a/docs/use-cases/ai-generated-agent-prs.md +++ b/docs/use-cases/ai-generated-agent-prs.md @@ -58,7 +58,7 @@ projection of it. There is no second verdict. A coding agent is asked to "add a support-agent feature that can issue Stripe refunds." It adds `stripe.create_refund` to the tool surface and opens a PR. -`agents-shipgate verify --base origin/main --head HEAD --format json` produces: +`agents-shipgate verify --base origin/main --head HEAD --json` produces: - a `capability_review.top_changes[]` row for `stripe.create_refund`, with an impact derived from the tool/action surface diff; @@ -101,7 +101,7 @@ block release through ordinary `SHIP-VERIFY-*` findings. pipx install agents-shipgate agents-shipgate verify --preview --json agents-shipgate init --workspace . --write --ci --agent-instructions=default --json -agents-shipgate verify --base origin/main --head HEAD --format json +agents-shipgate verify --base origin/main --head HEAD --json ``` - `verify --preview --json` is a lightweight relevance check — no scan, no @@ -113,7 +113,7 @@ agents-shipgate verify --base origin/main --head HEAD --format json (`AGENTS.md`, the Cursor rule, the Claude `/shipgate` command, and `.shipgate/agent-contract.json`). Skill bundles stay explicit targets such as `codex-skill`. -- `verify --base origin/main --head HEAD --format json` runs the authoritative head +- `verify --base origin/main --head HEAD --json` runs the authoritative head scan with diff context and writes the verifier artifacts. `verify` never fetches, so make the base ref available first (`fetch-depth: 0` in CI, or `git fetch origin main` locally); if the requested diff cannot be inspected, diff --git a/examples/github-actions/10-check-run-annotations.yml b/examples/github-actions/10-check-run-annotations.yml index 9dff65b8..4833a8c2 100644 --- a/examples/github-actions/10-check-run-annotations.yml +++ b/examples/github-actions/10-check-run-annotations.yml @@ -4,6 +4,11 @@ # # This recipe uses check_run_policy=require-mergeable, so only PRs with # can_merge_without_human=true produce a successful Check Run. +# +# check_run_policy is newer than v0.13.0. Until the next release is tagged, +# this example targets main and intentionally omits shipgate_version so the +# action installs the CLI from the same ref. After release, pin both the action +# ref and shipgate_version to that release. name: Agents Shipgate (check run) on: @@ -22,7 +27,7 @@ jobs: - uses: actions/checkout@v4 with: fetch-depth: 0 - - uses: ThreeMoonsLab/agents-shipgate@v0.13.0 + - uses: ThreeMoonsLab/agents-shipgate@main with: config: shipgate.yaml ci_mode: advisory @@ -30,4 +35,3 @@ jobs: pr_comment: 'true' check_run: 'true' check_run_policy: require-mergeable - shipgate_version: '0.13.0' diff --git a/examples/github-actions/README.md b/examples/github-actions/README.md index 5ff3efae..ce86912f 100644 --- a/examples/github-actions/README.md +++ b/examples/github-actions/README.md @@ -96,9 +96,13 @@ source-backed blockers and review items. Disable with `check_annotations: When `check_run: 'true'` is enabled, the Check Run uses the same PR projection as job annotations. `check_run_policy: advisory` preserves the default -mergeable/success, blocked/failure, human-routed/neutral behavior. For direct +mergeable/success, blocked/failure, human-routed/neutral behavior. +`check_run_policy: blocked-fails` keeps human-routed verdicts neutral but fails +`blocked` and `unknown` so setup failures do not look successful. For direct branch protection, use `check_run_policy: require-mergeable`; only -`can_merge_without_human == true` succeeds. +`can_merge_without_human == true` succeeds. `check_run_policy` is newer than +v0.13.0; until the next release is tagged, the Check Run policy example targets +`main` and omits `shipgate_version` so the action installs from that ref. `verify` writes static capability artifacts to the workflow artifact when available: `capabilities.lock.json`, `base.capabilities.lock.json`, and diff --git a/scripts/github_check_run.py b/scripts/github_check_run.py index 1b1897d6..2bc62062 100644 --- a/scripts/github_check_run.py +++ b/scripts/github_check_run.py @@ -11,9 +11,11 @@ - ``merge_verdict`` + ``check_run_policy`` → check conclusion. The default ``advisory`` policy preserves the historical mapping (``mergeable`` → ``success``, ``blocked`` → ``failure``, everything else - → ``neutral``). ``require-mergeable`` is the branch-protection policy: - only ``can_merge_without_human=true`` succeeds. The conclusion mirrors — - never replaces — the one decision engine: + → ``neutral``). ``blocked-fails`` maps ``blocked`` and ``unknown`` to + ``failure`` while leaving human-routed verdicts neutral. ``require-mergeable`` + is the strict branch-protection policy: only + ``can_merge_without_human=true`` succeeds. The conclusion mirrors — never + replaces — the one decision engine: ``report.json.release_decision.decision`` stays the gate. - PR projection items → up to ``MAX_ANNOTATIONS`` line-level annotations (Checks API caps one request at 50). SARIF remains the full finding export; diff --git a/src/agents_shipgate/cli/discovery/agent_instructions/renderers/agents_md.py b/src/agents_shipgate/cli/discovery/agent_instructions/renderers/agents_md.py index 31c7d9a8..948cd528 100644 --- a/src/agents_shipgate/cli/discovery/agent_instructions/renderers/agents_md.py +++ b/src/agents_shipgate/cli/discovery/agent_instructions/renderers/agents_md.py @@ -57,7 +57,7 @@ def render_block() -> str: `must_stop=true`, stop and surface the JSON result to a human. For committed PR/CI verification, run `agents-shipgate verify --base -origin/main --head HEAD --format json` after making the base ref available; it never +origin/main --head HEAD --json` after making the base ref available; it never fetches. Read `agents-shipgate-reports/agent-result.json` first, then `agents-shipgate-reports/verifier.json` for `merge_verdict` and `agents-shipgate-reports/report.json.release_decision.decision` for the release diff --git a/src/agents_shipgate/cli/discovery/agent_instructions/renderers/claude_md.py b/src/agents_shipgate/cli/discovery/agent_instructions/renderers/claude_md.py index 49a6fd40..f143b300 100644 --- a/src/agents_shipgate/cli/discovery/agent_instructions/renderers/claude_md.py +++ b/src/agents_shipgate/cli/discovery/agent_instructions/renderers/claude_md.py @@ -47,7 +47,7 @@ def render_block() -> str: `first_next_action.actor` is `human`, stop and route the change to a human. For committed PR/CI verification, run `agents-shipgate verify --base -origin/main --head HEAD --format json` after making the base ref available; it never +origin/main --head HEAD --json` after making the base ref available; it never fetches. Read `agents-shipgate-reports/agent-result.json` first, then `agents-shipgate-reports/verifier.json` for `merge_verdict` and `agents-shipgate-reports/report.json.release_decision.decision` for the release diff --git a/src/agents_shipgate/cli/discovery/agent_instructions/renderers/cursor.py b/src/agents_shipgate/cli/discovery/agent_instructions/renderers/cursor.py index 6f14a82a..5ad7f3f9 100644 --- a/src/agents_shipgate/cli/discovery/agent_instructions/renderers/cursor.py +++ b/src/agents_shipgate/cli/discovery/agent_instructions/renderers/cursor.py @@ -74,7 +74,7 @@ def render_file() -> str: `first_next_action.actor` is `human`, stop and route the change to a human. For committed PR/CI verification, run `agents-shipgate verify --base -origin/main --head HEAD --format json` after making the base ref available; it never +origin/main --head HEAD --json` after making the base ref available; it never fetches. Read `agents-shipgate-reports/agent-result.json` first, then `agents-shipgate-reports/verifier.json` for `merge_verdict` and `agents-shipgate-reports/report.json.release_decision.decision` for the release diff --git a/src/agents_shipgate/cli/verify/orchestrator.py b/src/agents_shipgate/cli/verify/orchestrator.py index 80e5eaf7..5ce9fac0 100644 --- a/src/agents_shipgate/cli/verify/orchestrator.py +++ b/src/agents_shipgate/cli/verify/orchestrator.py @@ -1168,7 +1168,7 @@ def _preview_verify_command( parts.extend(["--head", head]) if out is not None: parts.extend(["--out", str(out)]) - parts.extend(["--ci-mode", "advisory", "--format", "json"]) + parts.extend(["--ci-mode", "advisory", "--json"]) return _shell_join(parts) diff --git a/src/agents_shipgate/report/pr_projection.py b/src/agents_shipgate/report/pr_projection.py index 32e2a485..06dfd69c 100644 --- a/src/agents_shipgate/report/pr_projection.py +++ b/src/agents_shipgate/report/pr_projection.py @@ -251,6 +251,11 @@ def _append_unique( seen: set[str], item: PrReviewItem, ) -> None: + if any( + f"finding-id:{finding_id}" in seen + for finding_id in item.related_finding_ids + ): + return key = _item_key(item) if key in seen: return diff --git a/src/agents_shipgate/schemas/contract.py b/src/agents_shipgate/schemas/contract.py index 847e870c..99cb366b 100644 --- a/src/agents_shipgate/schemas/contract.py +++ b/src/agents_shipgate/schemas/contract.py @@ -105,12 +105,11 @@ "agents-shipgate init --workspace . --write --ci --agent-instructions=default --json" ), "verify_local": ( - "agents-shipgate verify --workspace . --config shipgate.yaml " - "--ci-mode advisory --format json" + "agents-shipgate verify --workspace . --config shipgate.yaml --ci-mode advisory --json" ), "verify_pr": ( "agents-shipgate verify --workspace . --config shipgate.yaml " - "--base origin/main --head HEAD --ci-mode advisory --format json" + "--base origin/main --head HEAD --ci-mode advisory --json" ), "contract": "agents-shipgate contract --json", } diff --git a/src/agents_shipgate/triggers.py b/src/agents_shipgate/triggers.py index 36692b2d..638c4d74 100644 --- a/src/agents_shipgate/triggers.py +++ b/src/agents_shipgate/triggers.py @@ -152,10 +152,7 @@ def _next_action( if manifest_present: return { "kind": "command", - "command": ( - "agents-shipgate verify --workspace . --config shipgate.yaml " - "--base origin/main --head HEAD --ci-mode advisory --format json" - ), + "command": "agents-shipgate verify --base origin/main --head HEAD --json", "why": ( "This change affects an agent tool or release-policy " "surface; verify whether the PR can merge." @@ -174,8 +171,8 @@ def _next_action( } if dry_run_recommended: command = ( - "agents-shipgate verify --workspace . --config shipgate.yaml " - "--base origin/main --head HEAD --ci-mode advisory --format json" + "agents-shipgate verify --base origin/main --head HEAD " + "--ci-mode advisory --json" if manifest_present else default_command ) diff --git a/tests/test_adapter_static_only.py b/tests/test_adapter_static_only.py index 1d81bb4c..e03b23e0 100644 --- a/tests/test_adapter_static_only.py +++ b/tests/test_adapter_static_only.py @@ -237,7 +237,7 @@ class AllowedException: AllowedException( relative_path="triggers.py", surface="attr_call:subprocess.run", - line=483, + line=480, snippet="subprocess.run(names_cmd, **run_kwargs)", rationale=( "git-diff change-name pass: ``git diff --name-only " @@ -249,7 +249,7 @@ class AllowedException: AllowedException( relative_path="triggers.py", surface="attr_call:subprocess.run", - line=484, + line=481, snippet="subprocess.run(body_cmd, **run_kwargs)", rationale=( "git-diff body pass: ``git diff base...HEAD`` for full " @@ -260,7 +260,7 @@ class AllowedException: AllowedException( relative_path="triggers.py", surface="attr_call:subprocess.run", - line=489, + line=486, snippet=( "subprocess.run(['git', 'ls-files', '--others', " "'--exclude-standard'], **run_kwargs)" diff --git a/tests/test_github_action_annotations.py b/tests/test_github_action_annotations.py index 05051310..c0fd6a9a 100644 --- a/tests/test_github_action_annotations.py +++ b/tests/test_github_action_annotations.py @@ -149,3 +149,56 @@ def test_annotations_include_source_backed_capability_changes(tmp_path: Path) -> assert annotation["level"] == "error" assert annotation["merge_impact"] == "blocks_release" assert annotation["capability_subject"] == "action:stripe.create_refund" + + +def test_annotations_dedupe_capability_change_related_to_selected_finding( + tmp_path: Path, +) -> None: + output_dir = tmp_path / "agents-shipgate-reports" + output_dir.mkdir() + (output_dir / "report.json").write_text( + json.dumps( + { + "release_decision": {"blockers": [{"id": "F1"}]}, + "findings": [ + { + "id": "F1", + "check_id": "SHIP-ACTION-APPROVAL-REMOVED", + "title": "Approval removed", + "severity": "critical", + "recommendation": "Restore approval.", + "source": {"path": "api.yaml", "start_line": 42}, + } + ], + } + ), + encoding="utf-8", + ) + (output_dir / "verifier.json").write_text( + json.dumps( + { + "capability_review": { + "top_changes": [ + { + "id": "cap-refund", + "change_type": "action_added", + "subject_kind": "action", + "subject": "stripe.create_refund", + "impact": "blocks_release", + "rationale": "Action added: stripe.create_refund", + "source_path": "api.yaml", + "source_start_line": 42, + "related_finding_ids": ["F1"], + } + ] + } + } + ), + encoding="utf-8", + ) + + payload = build_annotations(output_dir, limit=10) + + assert [item["check_id"] for item in payload["annotations"]] == [ + "SHIP-ACTION-APPROVAL-REMOVED" + ] diff --git a/tests/test_public_surface_contract.py b/tests/test_public_surface_contract.py index 9649d2e4..8a93e60f 100644 --- a/tests/test_public_surface_contract.py +++ b/tests/test_public_surface_contract.py @@ -1360,7 +1360,6 @@ def test_well_known_seo_geo_positioning_fields_are_pinned(): assert commands.get("install_ai_coding_workflow") == ( "agents-shipgate init --workspace . --write --ci --agent-instructions=default --json" ) - assert commands.get("verify_pr", "").endswith("--format json") assert data.get("check_run_policies") == [ "advisory", "blocked-fails", diff --git a/tests/test_verify.py b/tests/test_verify.py index c1d8c252..fed19a8d 100644 --- a/tests/test_verify.py +++ b/tests/test_verify.py @@ -1243,7 +1243,7 @@ def test_verify_preview_configured_repo_preserves_exact_verify_args(tmp_path: Pa assert payload["mode"] == "preview" assert payload["first_next_action"]["command"] == ( f"agents-shipgate verify --workspace {repo} --config shipgate.yaml " - f"--base origin/main --head HEAD --out {out} --ci-mode advisory --format json" + f"--base origin/main --head HEAD --out {out} --ci-mode advisory --json" )