diff --git a/docs/a38-guard.md b/docs/a38-guard.md index 0259b6d..76fb78f 100644 --- a/docs/a38-guard.md +++ b/docs/a38-guard.md @@ -229,8 +229,10 @@ The latest author report-like comment, ordered by `updated_at` and numeric comme | Mode | Stable status context | Meaning | | --- | --- | --- | -| `enforce` | `A38 / report (develop)` for target branch `develop` | Success only for valid evidence; otherwise failure. | -| `observe` | `A38 / report (observe: develop)` | Advisory status only; do not require this context for merging. | +| `enforce` | `A38 / report (develop)` for target branch `develop` | On a draft PR: omit the **blocking** commit status (not failure, not pending, not a fabricated pass); see the `not_applicable` success-clear carve-out below. The guard process exits 0 so `dfx pr guard` is not red merely for a missing draft report. An author report is still required before Ready. Once Ready (`draft=false`): success only for valid evidence; otherwise failure. | +| `observe` | `A38 / report (observe: develop)` | Advisory status only; do not require this context for merging. Unchanged on drafts. | + +Configured `not_applicable` exclusions still publish success on the target enforce context to clear a wrong prior status, including on drafts; that success is not a test-pass claim. Contexts use the **target branch name**, not the moving base SHA. Thus branch protection can require a stable name while a head targeting different branches gets distinct contexts. Supported branch names are bounded to 75 ASCII letters/digits, dots, underscores, hyphens and slashes; unsupported names fail closed. The exact base SHA remains in the comment and approval binding. @@ -255,6 +257,8 @@ Issue-only events and the bot's own comments are ignored. The installed workflow Closed PRs return `status: closed` and process exit zero without reading policy, pr-guard configuration or publishing comments/statuses, including when a PR closes during an all-open scan. Ignored events and empty all-open scans are also successful no-ops. +On an open **draft** in `enforce` mode the guard still publishes or updates its educational comment, but it does **not** create or update a blocking `A38 / report ()` commit status (and does not post an invalidating `error` status on draft). Configured `not_applicable` exclusions may still write success on that context only to clear a wrong prior status; that is not a test-pass claim. Process exit is 0 so `dfx pr guard` is not red merely because a draft lacks an author report. Ready (`draft=false`) keeps success-only-for-valid-evidence and failure otherwise. + The bot marker is ``. Only comments owned by the numeric acting user may be updated. `/user` resolves normal tokens; fallback to the verified official Actions bot is allowed only when `GITHUB_ACTIONS=true`. Failed authentication outside Actions does not impersonate that bot. Existing identical comments/statuses are not reposted. Before publication, the guard re-fetches head/base/branch/state, the trusted pr-guard configuration revision and bytes, the latest author report and any active migration approval. It checks again immediately before a success status and reassesses if evidence changed. GitHub offers no atomic transaction across comments, reviews and statuses: an edit after the final read is corrected by the next event or scheduled reconciliation. diff --git a/docs/a38.md b/docs/a38.md index 01c01bb..72c30a1 100644 --- a/docs/a38.md +++ b/docs/a38.md @@ -109,7 +109,7 @@ The report must contain the exact repository, current head, visibility, full req Only the latest report-like comment by the authenticated PR author is considered. A newer failed or malformed report does not fall back to an older success. Editing or deleting reports, pushing commits and changing the target base require reassessment. Other contributors cannot satisfy the author's reporting requirement by posting a copied block. -The bot maintains one friendly comment with the rules, documentation and current problems. `observe` publishes an explicitly advisory status; `enforce` reports success only for valid evidence. Configure branch protection to require the appropriate enforced status after a successful rollout. A missing or invalid policy is a configuration failure, not permission to pass. A valid A38 report confirms the consistency of an author's declaration; it is not cryptographic proof that commands ran or proof that tests themselves are adequate. +The bot maintains one friendly comment with the rules, documentation and current problems. `observe` publishes an explicitly advisory status and is unchanged on drafts. `enforce` reports success only for valid evidence. On a **draft** pull request the guard publishes **no blocking** `A38 / report ()` commit status (not failure, not pending, and not a fabricated pass) and exits 0 so the GitHub Actions check `dfx pr guard` is not red merely because a draft has no author report yet. Configured `not_applicable` exclusions may still write success on that context to clear a wrong prior status; that is not a test-pass claim. An author report is still required before Ready for review. Once the pull request is Ready (`draft=false`), keep current behaviour: valid evidence → success; missing or invalid evidence → failure. Configure branch protection to require the appropriate enforced status after a successful rollout. A missing or invalid policy is a configuration failure, not permission to pass. A valid A38 report confirms the consistency of an author's declaration; it is not cryptographic proof that commands ran or proof that tests themselves are adequate. For the private opt-in equivalence, the final Ready join is a separate live read after the same SHA has been pushed and the unchanged generated report has been posted: diff --git a/docs/pull-request-lifecycle.md b/docs/pull-request-lifecycle.md index a064272..f832ee2 100644 --- a/docs/pull-request-lifecycle.md +++ b/docs/pull-request-lifecycle.md @@ -49,6 +49,8 @@ Hosted CI and other applicable checks may fail. There is no promise that CI neve Pending checks must be labeled **pending**. Do not fabricate a pass. +The blocking `A38 / report ()` commit status is **omitted** on drafts (not pending, not failure, and not a fabricated pass). `observe` stays advisory and unchanged. Configured `not_applicable` exclusions may still write success on that context only to clear a wrong prior status; that is not a test-pass claim. Real red hosted CI remains a blocker. Once the pull request is Ready for review, A38 publishes success only for valid evidence and failure otherwise. + Repositories can enable the [guard's continuous readiness reconciliation](a38-guard.md#optional-continuous-readiness). An open Ready PR returns to Draft with an explanatory comment when required CI is missing, queued, running, blocked or failed, or GitHub confirms merge conflicts. diff --git a/src/agent_cli/a38_guard.py b/src/agent_cli/a38_guard.py index 74e5483..3bb0d67 100644 --- a/src/agent_cli/a38_guard.py +++ b/src/agent_cli/a38_guard.py @@ -95,6 +95,8 @@ class PullSnapshot: # Used to locate configuration; never a built-in scope rule by itself. # Empty is allowed for closed no-ops; open assessments fail closed without it. default_branch: str = "" + # True only when GitHub reports draft is JSON true; missing/None/False → not draft. + draft: bool = False @dataclass(frozen=True) @@ -156,6 +158,7 @@ class Assessment: workflow_approvals: list[dict[str, Any]] = field(default_factory=list) lifecycle_enabled: bool = False lifecycle: dict[str, Any] = field(default_factory=dict) + draft: bool = False def to_json(self) -> dict[str, Any]: trusted = self.trusted_default_branch or self.default_branch @@ -185,6 +188,7 @@ def to_json(self) -> dict[str, Any]: "state": self.state_for_status, "description": self.description, "closed": self.closed, + "draft": self.draft, "skip_publish": self.skip_publish, "dry_run": self.dry_run, "writes": list(self.writes), @@ -640,6 +644,8 @@ def fetch_pull(api: GitHubApi, repo: str, number: int) -> PullSnapshot: default_branch = _parse_default_branch( base_repo.get("default_branch"), required=(state == "open") ) + # Fail closed for the draft exemption: only JSON true is draft. + draft = data.get("draft") is True return PullSnapshot( repo=repo, number=number, @@ -652,6 +658,7 @@ def fetch_pull(api: GitHubApi, repo: str, number: int) -> PullSnapshot: author_login=author_login, head_repo=head_repository, default_branch=default_branch, + draft=draft, ) @@ -985,22 +992,45 @@ def build_comment_body(assessment: Assessment) -> str: problems = "; ".join(assessment.reasons) if assessment.reasons else "none" if len(problems) > 800: problems = problems[:799] + "…" - en = ( - f"A38 {assessment.status}: " - + ( - "author local-CI report accepted for this head." - if assessment.ok and assessment.status == "pass" - else "author local-CI report missing or invalid for this head." + passing = assessment.ok and assessment.status == "pass" + if assessment.draft: + extra_en = ( + " An author local-CI report is accepted for this head." + if passing + else " An author local-CI report is still required before Ready." ) - ) - de = ( - f"A38 {assessment.status}: " - + ( - "Autor-Local-CI-Report für diesen Head akzeptiert." - if assessment.ok and assessment.status == "pass" - else "Autor-Local-CI-Report für diesen Head fehlt oder ist ungültig." + extra_de = ( + " Ein Autor-Local-CI-Report für diesen Head ist akzeptiert." + if passing + else " Ein Autor-Local-CI-Report ist vor Ready weiterhin erforderlich." + ) + en = ( + "A38: this pull request is a draft; " + "no blocking A38 report status is published until Ready for review." + + extra_en + ) + de = ( + "A38: dieser Pull Request ist ein Draft; " + "bis Ready for review wird kein blockierender A38-Report-Status veröffentlicht." + + extra_de + ) + else: + en = ( + f"A38 {assessment.status}: " + + ( + "author local-CI report accepted for this head." + if passing + else "author local-CI report missing or invalid for this head." + ) + ) + de = ( + f"A38 {assessment.status}: " + + ( + "Autor-Local-CI-Report für diesen Head akzeptiert." + if passing + else "Autor-Local-CI-Report für diesen Head fehlt oder ist ungültig." + ) ) - ) if assessment.mode == "observe": en = "Observe mode (advisory, not branch-required). " + en de = "Observe-Modus (Hinweis, nicht branch-pflichtig). " + de @@ -1050,6 +1080,13 @@ def _status_bits(assessment: Assessment) -> None: return assessment.context = status_context_enforce(base) assessment.observe_context = "" + if assessment.draft: + # Draft enforce: keep context for audit JSON; do not post success or failure. + assessment.state_for_status = "" + assessment.description = truncate_desc( + "draft: A38 status omitted until Ready" + ) + return if assessment.ok and assessment.status == "pass": assessment.state_for_status = "success" assessment.description = truncate_desc(f"pass for {assessment.head_sha[:7]}") @@ -1094,6 +1131,7 @@ def assess_from_parts( private=pull.private, closed=pull.state != "open", dry_run=dry_run, + draft=pull.draft, standard_url=blob_url(CENTRAL_REPO, trusted_runtime_revision, POLICY_DOCS), policy_url=blob_url(active_policy_repo, active_policy_sha, POLICY_PATH), guard_docs_url=blob_url(CENTRAL_REPO, trusted_runtime_revision, GUARD_DOCS), @@ -1344,6 +1382,7 @@ def _out_of_scope_assessment( comment_body="", skip_publish=False, dry_run=dry_run, + draft=snap.draft, ) _attach_trusted_config(assessment, trusted) return assessment @@ -1360,6 +1399,7 @@ def _snapshot_matches_assessment(fresh: PullSnapshot, assessment: Assessment) -> and fresh.head_repo == assessment.head_repo and fresh.private == assessment.private and fresh.state == expected_state + and fresh.draft == assessment.draft ) @@ -1400,7 +1440,7 @@ def assess_pull( default_branch=snap.default_branch, trusted_default_branch=snap.default_branch, head_repo=snap.head_repo, private=snap.private, - state_for_status="", dry_run=dry_run, + state_for_status="", dry_run=dry_run, draft=snap.draft, ) # Open PRs require trusted default_branch metadata to locate configuration. if not snap.default_branch: @@ -1636,6 +1676,8 @@ def _post_status( assessment.description, require_report=True, ) + elif assessment.draft: + assessment.writes.append("status:skipped:draft") else: _post_status( assessment.context or status_context_enforce(assessment.base_ref), @@ -1703,6 +1745,8 @@ def invalidate_status(api: GitHubApi, pull: PullSnapshot) -> None: """Best-effort error status on known head; never hide the original API failure.""" if pull.state != "open": return + if pull.draft: + return try: status, _, _ = api.request( "POST", f"/repos/{pull.repo}/statuses/{pull.head_sha}", @@ -1915,8 +1959,13 @@ def _load_event( def _assessment_exit_code(assessment: Assessment) -> int: - """A closed PR is a successful no-op; observe remains advisory.""" - return 0 if assessment.closed or assessment.ok or assessment.mode == "observe" else 1 + """Closed, observe, and draft enforce skips exit 0; Ready enforce failure exits 1.""" + return 0 if ( + assessment.closed + or assessment.ok + or assessment.mode == "observe" + or assessment.draft + ) else 1 def main(argv: Sequence[str] | None = None, *, env: MutableMapping[str, str] | None = None, diff --git a/tests/test_a38_guard.py b/tests/test_a38_guard.py index d34fe44..ad707eb 100644 --- a/tests/test_a38_guard.py +++ b/tests/test_a38_guard.py @@ -31,6 +31,7 @@ LOCAL_CI_END, assess_pull, event_should_ignore, + fetch_pull, looks_like_report, main, pick_latest_author_report, @@ -182,10 +183,13 @@ def set_pr_guard_config(self, config: dict | None, *, revision: str = DEFAULT_TI return self.files[key] = json.dumps(config).encode() - def _pull(self, head: str, base: str, *, state: str = "open") -> dict[str, Any]: + def _pull( + self, head: str, base: str, *, state: str = "open", draft: bool = False + ) -> dict[str, Any]: return { "number": 1, "state": state, + "draft": draft, "user": {"id": AUTHOR_ID, "login": "author"}, "head": {"sha": head, "repo": {"full_name": REPO, "default_branch": "feature"}}, "base": { @@ -405,6 +409,81 @@ def test_opened_no_report(self) -> None: result.policy_url, f"https://github.com/{REPO}/blob/{BASE}/.github/a38.json", ) + self.assertEqual(a38_guard._assessment_exit_code(result), 1) + + def test_draft_no_report_omits_status_and_exits_zero(self) -> None: + fake = FakeAPI() + fake.pull = fake._pull(HEAD, BASE, draft=True) + result = reconcile_pull(fake.api(), REPO, 1, dry_run=False, publish=True) + self.assertFalse(result.ok) + self.assertTrue(result.draft) + self.assertTrue(any("no author" in r for r in result.reasons)) + self.assertTrue(any(w.startswith("comment:") for w in result.writes)) + self.assertTrue( + any(w == "status:skipped:draft" for w in result.writes) + or not any(w.startswith("status:create:") for w in result.writes) + ) + self.assertNotIn( + status_context_enforce("develop"), + [s["context"] for s in fake.statuses], + ) + self.assertEqual(a38_guard._assessment_exit_code(result), 0) + body = result.comment_body + self.assertIn("EN:", body) + self.assertIn("DE:", body) + self.assertRegex(body, r"(?i)draft") + self.assertRegex(body, r"(?i)Ready") + self.assertIn( + "no blocking A38 report status is published until Ready for review", + body, + ) + self.assertIn("author local-CI report is still required before Ready", body) + self.assertNotIn("missing or invalid", body) + self.assertNotRegex(body, r"A38 fail:") + self.assertNotRegex(body, r"A38 pass:") + + def test_draft_valid_report_omits_enforce_status(self) -> None: + fake = FakeAPI() + fake.pull = fake._pull(HEAD, BASE, draft=True) + fake.add_author_report(_report_comment(), updated_at="2026-09-05T12:00:00Z", cid=21) + result = reconcile_pull(fake.api(), REPO, 1, publish=True) + self.assertTrue(result.ok) + self.assertTrue(result.draft) + self.assertEqual(result.status, "pass") + self.assertNotIn( + status_context_enforce("develop"), + [s["context"] for s in fake.statuses], + ) + self.assertTrue( + any(w == "status:skipped:draft" for w in result.writes) + or not any(w.startswith("status:create:") for w in result.writes) + ) + self.assertEqual(a38_guard._assessment_exit_code(result), 0) + body = result.comment_body + self.assertIn( + "no blocking A38 report status is published until Ready for review", + body, + ) + self.assertIn("author local-CI report is accepted for this head", body) + self.assertNotRegex(body, r"A38 fail:") + self.assertNotRegex(body, r"A38 pass:") + + def test_fetch_pull_draft_true_only_when_json_true(self) -> None: + fake = FakeAPI() + fake.pull = fake._pull(HEAD, BASE, draft=True) + snap = fetch_pull(fake.api(), REPO, 1) + self.assertTrue(snap.draft) + + fake.pull = fake._pull(HEAD, BASE, draft=False) + self.assertFalse(fetch_pull(fake.api(), REPO, 1).draft) + + fake.pull = fake._pull(HEAD, BASE) + del fake.pull["draft"] + self.assertFalse(fetch_pull(fake.api(), REPO, 1).draft) + + fake.pull = fake._pull(HEAD, BASE) + fake.pull["draft"] = None + self.assertFalse(fetch_pull(fake.api(), REPO, 1).draft) def test_author_valid_report(self) -> None: fake = FakeAPI() @@ -824,6 +903,43 @@ def test_excluded_target_is_not_applicable_without_policy_or_report(self) -> Non self.assertEqual(payload["config_revision"], DEFAULT_TIP) self.assertEqual(payload["scope_decision"], "exclude") + def test_excluded_target_on_draft_still_posts_not_applicable_success(self) -> None: + fake = FakeAPI() + fake.pull = fake._pull(HEAD, BASE, draft=True) + self._release_pull(fake) + del fake.files[(BASE, ".github/a38.json")] + result = reconcile_pull(fake.api(), REPO, 1, publish=True) + self.assertTrue(result.ok) + self.assertTrue(result.draft) + self.assertEqual(result.status, "not_applicable") + self.assertEqual(result.state_for_status, "success") + self.assertFalse(any(w == "status:skipped:draft" for w in result.writes)) + self.assertTrue( + any( + s["context"] == status_context_enforce("main") and s["state"] == "success" + for s in fake.statuses + ) + ) + self.assertEqual(a38_guard._assessment_exit_code(result), 0) + self.assertFalse(any(w.startswith("comment:") for w in result.writes)) + + def test_invalidate_status_skips_drafts(self) -> None: + fake = FakeAPI() + snap = fetch_pull(fake.api(), REPO, 1) + self.assertFalse(snap.draft) + a38_guard.invalidate_status(fake.api(), snap) + self.assertTrue( + any(s.get("state") == "error" for s in fake.statuses) + ) + fake_draft = FakeAPI() + fake_draft.pull = fake_draft._pull(HEAD, BASE, draft=True) + draft_snap = fetch_pull(fake_draft.api(), REPO, 1) + self.assertTrue(draft_snap.draft) + before = list(fake_draft.statuses) + a38_guard.invalidate_status(fake_draft.api(), draft_snap) + self.assertEqual(fake_draft.statuses, before) + self.assertFalse(any(s.get("state") == "error" for s in fake_draft.statuses)) + def test_configurable_enforce_exclude_default_and_exact_match(self) -> None: fake = FakeAPI() fake.set_pr_guard_config(