From 309156f0f60fdcd77f28f3fe5bb8aab468996f8a Mon Sep 17 00:00:00 2001 From: TaprootFreakAI <315477232+TaprootFreakAI@users.noreply.github.com> Date: Fri, 11 Sep 2026 23:51:16 +0200 Subject: [PATCH 1/6] Draft Ready pull requests that have merge conflicts. Write-hold still covers red or missing CI only. Scheduled all-open sweeps use a separate concurrency group so pull events cannot starve them. --- docs/a38-guard.md | 25 +++++--- docs/a38.md | 2 +- docs/pull-request-lifecycle.md | 2 +- examples/a38-guard.yml | 3 +- src/agent_cli/pr_lifecycle.py | 44 +++++++++++-- tests/test_pr_lifecycle.py | 113 ++++++++++++++++++++++++++++++++- 6 files changed, 169 insertions(+), 20 deletions(-) diff --git a/docs/a38-guard.md b/docs/a38-guard.md index fa2c383..2817c0a 100644 --- a/docs/a38-guard.md +++ b/docs/a38-guard.md @@ -29,7 +29,7 @@ agent pr-guard --repo OWNER/NAME --all-open --dry-run agent pr-guard --repo OWNER/NAME --all-open ``` -Schedule that command externally when Actions are unavailable; no daemon is installed. The example workflow also reconciles all open PRs at minutes 17 and 47 of every hour and serializes all bot runs for the repository. Its manual dispatch accepts either a PR number or `all_open=true`. GitHub Actions does not guarantee delivery of every pending concurrency event, so scheduled reconciliation recovers missed events, base changes and permission changes. Immutable SHA-addressed contents and trees are cached within the API client, up to 128 entries; comments, reviews, permissions and PR snapshots are never cached. +Schedule that command externally when Actions are unavailable; no daemon is installed. The example workflow also reconciles all open PRs at minutes 17 and 47 of every hour and uses two concurrency groups (`event` versus `sweep`) so PR events cannot starve all-open reconciliation. Its manual dispatch accepts either a PR number or `all_open=true`. GitHub Actions does not guarantee delivery of every pending concurrency event, so scheduled reconciliation recovers missed events, base changes and permission changes. Immutable SHA-addressed contents and trees are cached within the API client, up to 128 entries; comments, reviews, permissions and PR snapshots are never cached. ## Trust and policy @@ -195,12 +195,15 @@ the pull request files. For **every open Ready PR targeting an A38-enforced branch**, confirmed merge conflicts or CI that is missing, queued, waiting, running, blocked, cancelled -or failed cause a Draft transition, **except** while a write collaborator holds -Ready: the PR author currently has `write`/`maintain`/`admin` on the target, or -the latest human `ready_for_review` timeline actor does. That hold skips -auto-draft only; it does not waive policy, workflow inventory, or migration -failures, and it does not skip auto-ready when A38 is already a fresh enforce -`pass`. Lifecycle Draft/Ready writes run only on A38-enforced targets; excluded +or failed cause a Draft transition. A write collaborator holds Ready through +missing or red CI only: the PR author currently has `write`/`maintain`/`admin` +on the target, or the latest human `ready_for_review` timeline actor does. +Confirmed merge conflicts always return Ready to Draft, including while that +write hold would otherwise apply. That hold skips auto-draft for CI only; it +does not waive policy, workflow inventory, or migration failures, and it does +not skip auto-ready when A38 is already a fresh enforce `pass`. Restore after +an auto-draft requires GitHub `mergeable` true and no conflicts (CI may still +be red). Lifecycle Draft/Ready writes run only on A38-enforced targets; excluded bases (for example a develop→main release PR) are left untouched. Missing required workflows are not an empty green result. Only completed, successful required workflows satisfy CI. Optional workflows @@ -241,8 +244,10 @@ The adopting workflow owns runner routing, `contents: write` for `pull-requests`/`issues`/`statuses` write access, and `actions: write` for initial workflow approval. The guard authorizes waiting allowlisted fork runs **before** it mutates Ready or Draft, so a failed convert-to-draft cannot skip approval. -Serialize **all** event and scheduled invocations with one -repository-wide concurrency group and `cancel-in-progress: false`. Run trusted +Serialize event-driven single-PR runs and scheduled all-open sweeps in two +repository-wide concurrency groups (`event` versus `sweep`) with +`cancel-in-progress: false`, so PR events cannot starve all-open reconciliation. +Sweep jobs may use a 45-minute timeout. Run trusted `--all-open` reconciliation on a repository-configured schedule (for example every five minutes). GitHub may delay scheduled execution; this is not a real-time SLA. Privileged runs must never check out PR code. Bot readiness must not be wired to @@ -277,7 +282,7 @@ For the private opt-in process, the checkout must be clean at the final reposito The latest author report-like comment, ordered by `updated_at` and numeric comment ID, is authoritative. A newer malformed or failed report never falls back to an older success. Other authors' reports cannot satisfy the requirement. Matching repository, head, visibility, full job set, names, commands, timeouts and successful measured results are mandatory, including for public repositories. -**Author-report Ready waivers:** When the PR author is a GitHub `User` who currently has `write`, `maintain`, or `admin` on the target repository, or when a GitHub `User` with one of those roles is the latest `ready_for_review` timeline actor **or** the `ready_for_review` webhook sender, the author-report gate is waived and enforce status may succeed with an explicit waiver description. Independently confirmed markdown-only change sets (every changed path ends with `.md`, fail-closed) also waive the author-report gate with reason `markdown-only change set`, reusing the same status-pass path so fork workflow approval and auto-ready can proceed without a local suite. Draft does not skip the Ready-actor path. A valid passing author report still takes the normal “report accepted” path when present. Only `User` actors can grant the write waiver (allowlist); bots and apps cannot, even with write/admin. `MEMBER` / association is not write; 404 or denied permission lookups do not grant it; timeline pagination 401/403/404 yields no waiver and does not crash assessment. Invalid policy, unclassified workflows, and pr-guard migration failures are not waived by write access or by markdown-only detection. No-write authors who mark Ready without a valid report still fail and are still auto-drafted by lifecycle unless the markdown-only waiver applies; lifecycle does not override Ready back to Draft while the write hold applies, and it restores Ready after an auto-draft if the Ready actor still has write. +**Author-report Ready waivers:** When the PR author is a GitHub `User` who currently has `write`, `maintain`, or `admin` on the target repository, or when a GitHub `User` with one of those roles is the latest `ready_for_review` timeline actor **or** the `ready_for_review` webhook sender, the author-report gate is waived and enforce status may succeed with an explicit waiver description. Independently confirmed markdown-only change sets (every changed path ends with `.md`, fail-closed) also waive the author-report gate with reason `markdown-only change set`, reusing the same status-pass path so fork workflow approval and auto-ready can proceed without a local suite. Draft does not skip the Ready-actor path. A valid passing author report still takes the normal “report accepted” path when present. Only `User` actors can grant the write waiver (allowlist); bots and apps cannot, even with write/admin. `MEMBER` / association is not write; 404 or denied permission lookups do not grant it; timeline pagination 401/403/404 yields no waiver and does not crash assessment. Invalid policy, unclassified workflows, and pr-guard migration failures are not waived by write access or by markdown-only detection. No-write authors who mark Ready without a valid report still fail and are still auto-drafted by lifecycle unless the markdown-only waiver applies; lifecycle does not override Ready back to Draft while the write hold applies for missing or red CI, confirmed merge conflicts still draft, and it restores Ready after an auto-draft if the Ready actor still has write, GitHub `mergeable` is true, and there are no conflicts. ## Statuses and events diff --git a/docs/a38.md b/docs/a38.md index 7d19a18..7a72514 100644 --- a/docs/a38.md +++ b/docs/a38.md @@ -120,7 +120,7 @@ Only the latest report-like comment by the authenticated PR author is considered On an open **draft** the bot comment is a friendly greeting that uses a Markdown link whose text is the rules name and whose target is `standard_url`, not a raw URL in the visible sentence. It does not lecture about draft status, missing reports, write waivers, markdown-only waivers, or tool-attribution, and it does not include the details block (jobs, problems, run command). Current problems and run instructions appear in the comment once the pull request is Ready (`draft=false`). `observe` publishes an explicitly advisory status and is unchanged on drafts. `enforce` reports success only for valid evidence, with the author-report waivers below (write collaborator, or independently confirmed markdown-only). On a **draft** pull request without `hard_fail` 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. Separately, the guard scans the live PR title, PR body, and every PR commit message plus author/committer identity (not the diff, not comments). A truncated GitHub pull-commits list (the endpoint caps at 250) is fail-closed. Tool-attribution markers — a `generated-with` banner, an AI `Co-Authored-By` trailer, an AI session header, or an AI author identity — fail the assessment with `hard_fail`. An unscannable or missing commit message is also fail-closed (`hard_fail`, exit 1 even on draft/observe), same as attribution markers. `dfx pr guard` then exits 1 even on a draft and even in observe mode. On Ready the comment tells authors to remove attribution markers, supply a non-empty commit message, or both. On `hard_fail` the guard posts `failure` on that context against the PR head even on a draft, so the pull request Checks box is red. The draft exemption (omit the blocking status, exit 0) applies only when there is no `hard_fail`. A leftover `failure` whose description starts with `hard_fail:` is success-cleared on the next draft reconcile that is no longer `hard_fail`; that is not a test-pass claim. Other enforce failures on the same head stay. -The **author local-CI report** is required for Ready when the PR author lacks `write`, `maintain`, or `admin` on the **target** repository **and** the change set is not independently markdown-only. If the author currently has one of those roles **and** is a GitHub `User`, the report is not required; enforce status succeeds with an explicit waiver description. The same write waiver applies when a GitHub `User` collaborator with `write`/`maintain`/`admin` is the latest `ready_for_review` timeline actor **or** the `ready_for_review` webhook sender (so a lagged timeline cannot drop the click). Independently confirmed markdown-only change sets waive the report (and the local suite) without granting the write hold: Ready still auto-drafts if required CI is red or pending. Draft does not hide that Ready actor: after an auto-draft, the next reconcile still sees them and lifecycle restores Ready even if CI is red. Only `User` actors can grant the write waiver (allowlist); bots and apps cannot, even with write/admin. `MEMBER` / `author_association` is not write. A 404 or denied collaborator-permission lookup does not grant it. Timeline pagination 401/403/404 yields no waiver and does not crash assessment. Invalid policy, unclassified workflows, and pr-guard migration failures are **not** waived. No-write authors (forks, triage-only, outside collaborators) who mark Ready without a valid author report still fail unless markdown-only applies; lifecycle still auto-drafts them. Lifecycle must not convert a Ready PR back to Draft while that write hold applies. +The **author local-CI report** is required for Ready when the PR author lacks `write`, `maintain`, or `admin` on the **target** repository **and** the change set is not independently markdown-only. If the author currently has one of those roles **and** is a GitHub `User`, the report is not required; enforce status succeeds with an explicit waiver description. The same write waiver applies when a GitHub `User` collaborator with `write`/`maintain`/`admin` is the latest `ready_for_review` timeline actor **or** the `ready_for_review` webhook sender (so a lagged timeline cannot drop the click). Independently confirmed markdown-only change sets waive the report (and the local suite) without granting the write hold: Ready still auto-drafts if required CI is red or pending. Draft does not hide that Ready actor: after an auto-draft, the next reconcile still sees them and lifecycle restores Ready even if CI is red when GitHub `mergeable` is true and there are no conflicts. Only `User` actors can grant the write waiver (allowlist); bots and apps cannot, even with write/admin. `MEMBER` / `author_association` is not write. A 404 or denied collaborator-permission lookup does not grant it. Timeline pagination 401/403/404 yields no waiver and does not crash assessment. Invalid policy, unclassified workflows, and pr-guard migration failures are **not** waived. No-write authors (forks, triage-only, outside collaborators) who mark Ready without a valid author report still fail unless markdown-only applies; lifecycle still auto-drafts them. That write hold does not apply to confirmed merge conflicts: those always return Ready to Draft. Lifecycle must not convert a Ready PR back to Draft while that write hold applies for missing or red CI. Once the pull request is Ready (`draft=false`) without a report waiver: 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. diff --git a/docs/pull-request-lifecycle.md b/docs/pull-request-lifecycle.md index d4165f3..5e6375b 100644 --- a/docs/pull-request-lifecycle.md +++ b/docs/pull-request-lifecycle.md @@ -58,7 +58,7 @@ Lifecycle Draft/Ready writes run only on A38-enforced targets; excluded bases (f An open Ready PR on an enforced target returns to Draft with an explanatory comment when required CI is missing, queued, running, blocked or failed, or GitHub confirms merge conflicts. The visible comment names the actual blocker(s), not a canned "CI or merge conflicts" alternative. -That return is skipped while a write collaborator holds Ready (author is a GitHub `User` with write on a Ready PR, or the latest `User` `ready_for_review` actor — or the matching webhook sender — has write). In that hold, lifecycle leaves the PR Ready (`action: unchanged`) and still records the CI reasons for audit; it does not post a draft-intent comment or call the draft transition. If a lagged timeline caused an auto-draft, the next reconcile still sees the Ready actor on draft and restores Ready even when CI is red. No-write authors who mark Ready without a valid author report still fail A38 and are still auto-drafted. +A write collaborator holds Ready through missing or red CI only (author is a GitHub `User` with write on a Ready PR, or the latest `User` `ready_for_review` actor — or the matching webhook sender — has write). In that hold, lifecycle leaves the PR Ready (`action: unchanged`) and still records the CI reasons for audit; it does not post a draft-intent comment or call the draft transition. Confirmed merge conflicts always return Ready to Draft, including while that write hold would otherwise apply. If a lagged timeline caused an auto-draft, the next reconcile still sees the Ready actor on draft and restores Ready only when GitHub `mergeable` is true and there are no conflicts, even when CI is red. No-write authors who mark Ready without a valid author report still fail A38 and are still auto-drafted. After the CI authorized by the bot succeeds, it can restore Ready only with current A38 evidence and confirmed mergeability (including a write-collaborator enforce `pass` without a report: author or latest `User` Ready actor). Required workflows, conditional CI scope, control-workflow exclusions and the polling schedule belong to the diff --git a/examples/a38-guard.yml b/examples/a38-guard.yml index cff960a..69faa55 100644 --- a/examples/a38-guard.yml +++ b/examples/a38-guard.yml @@ -24,7 +24,7 @@ on: default: false concurrency: - group: a38-pr-guard-${{ github.repository }} + group: a38-pr-guard-${{ github.repository }}-${{ (github.event_name == 'schedule' || (github.event_name == 'workflow_dispatch' && inputs.all_open)) && 'sweep' || 'event' }} cancel-in-progress: false permissions: @@ -38,6 +38,7 @@ permissions: jobs: guard: runs-on: ubuntu-latest + timeout-minutes: ${{ (github.event_name == 'schedule' || (github.event_name == 'workflow_dispatch' && inputs.all_open)) && 45 || 10 }} steps: - name: dfx pr guard uses: DFXswiss/agent/.github/actions/a38-guard@USES_REF_PIN_ME diff --git a/src/agent_cli/pr_lifecycle.py b/src/agent_cli/pr_lifecycle.py index fb19293..3145ec6 100644 --- a/src/agent_cli/pr_lifecycle.py +++ b/src/agent_cli/pr_lifecycle.py @@ -411,6 +411,27 @@ def _transition(api: Any, node: str, draft: bool) -> Mapping: return pull +def _has_merge_conflicts(api: Any, pull: Mapping) -> bool: + from .a38_guard import GuardError + mergeable = pull.get("mergeable") + if mergeable is False: + return True + if mergeable is True: + return False + node = pull.get("node_id") + if not isinstance(node, str): + raise GuardError("PR lifecycle mergeable query failed: node_id missing") + query = "query($id: ID!) { node(id: $id) { ... on PullRequest { mergeable } } }" + status, data, _ = api.request("POST", "/graphql", body={"query": query, "variables": {"id": node}}, retry=False) + errors = _field(data, "errors") + detail = "" + if isinstance(errors, list) and errors and isinstance(errors[0], Mapping): + detail = f": {errors[0].get('message') or 'graphql error'}" + if status != 200 or errors: + raise GuardError(f"PR lifecycle mergeable query failed (HTTP {status}){detail}") + return _field(data, "data", "node", "mergeable") == "CONFLICTING" + + def reconcile_lifecycle(api: Any, assessment: Any, *, dry_run: bool = False) -> dict: from .a38_guard import GuardError, assess_pull, fetch_pull, resolve_trusted_guard_config if assessment.closed or not assessment.lifecycle_enabled: @@ -438,7 +459,7 @@ def reconcile_lifecycle(api: Any, assessment: Any, *, dry_run: bool = False) -> and previous.get("state") == ("draft" if pull["draft"] else "ready") and not dry_run): _complete_transition_comment(api, assessment, previous) reasons, latest = ci_state(api, assessment, config, pull) - if pull.get("mergeable") is False: + if _has_merge_conflicts(api, pull): reasons.insert(0, "Merge conflicts") target = None we_drafted = ( @@ -451,11 +472,16 @@ def reconcile_lifecycle(api: Any, assessment: Any, *, dry_run: bool = False) -> "author has write", "ready by write collaborator", } - restore_write_ready = bool(pull["draft"] and write_hold and we_drafted) - # Write collaborator Ready hold: do not auto-draft while author or the latest - # ready_for_review actor has write/maintain/admin on the target repository. + restore_write_ready = bool( + pull["draft"] and write_hold and we_drafted + and pull.get("mergeable") is True + and "Merge conflicts" not in reasons + ) + # Write collaborator Ready hold: do not auto-draft for red/missing CI while + # author or the latest ready_for_review actor has write/maintain/admin. + # Confirmed merge conflicts always return Ready to Draft, including in a hold. # Markdown-only is a report waiver only — it does not hold Ready through red CI. - if not pull["draft"] and reasons and write_hold: + if not pull["draft"] and reasons and write_hold and "Merge conflicts" not in reasons: hold = { "repo": assessment.repo, "pr": assessment.pr, @@ -520,8 +546,12 @@ def reconcile_lifecycle(api: Any, assessment: Any, *, dry_run: bool = False) -> or final_pull.get("draft") != pull["draft"]): raise GuardError("pull or configuration changed before lifecycle transition") final_reasons, _ = ci_state(api, assessment, config, final_pull) - if final_pull.get("mergeable") is False: + if _has_merge_conflicts(api, final_pull): final_reasons.insert(0, "Merge conflicts") + if target == "ready" and ( + "Merge conflicts" in final_reasons or final_pull.get("mergeable") is False + ): + return {"action": "unchanged", "reasons": final_reasons, "dry_run": False} if ( target == "ready" and not restore_write_ready @@ -530,6 +560,8 @@ def reconcile_lifecycle(api: Any, assessment: Any, *, dry_run: bool = False) -> return {"action": "unchanged", "reasons": final_reasons, "dry_run": False} if target == "draft" and not final_reasons: return {"action": "unchanged", "reasons": [], "dry_run": False} + if target == "draft" and write_hold and "Merge conflicts" not in final_reasons: + return {"action": "unchanged", "reasons": final_reasons, "dry_run": False} record = {"repo": assessment.repo, "pr": assessment.pr, "head": snap.head_sha, "base": snap.base_sha, "state": target, "reasons": final_reasons, "phase": "planned"} _save_record(api, assessment, STATE_MARKER, record, diff --git a/tests/test_pr_lifecycle.py b/tests/test_pr_lifecycle.py index 0863e94..2a44fdd 100644 --- a/tests/test_pr_lifecycle.py +++ b/tests/test_pr_lifecycle.py @@ -5,6 +5,7 @@ import pytest +from agent_cli import pr_lifecycle from agent_cli.a38_guard import GuardError, reconcile_pull from agent_cli.pr_guard_config import load_pr_guard_config, PrGuardConfigError from agent_cli.pr_lifecycle import AUTH_MARKER, STATE_MARKER, visible_transition_sentences @@ -228,6 +229,7 @@ def __init__(self): self.graphql_ready_without_rest = False self.graphql_noop_ready = False self.graphql_noop_ready_null = False + self.graphql_mergeable = "MERGEABLE" self.mutate_during_transition = False self.fail_comment_once = False @@ -239,7 +241,14 @@ def request_fn(self, method, url, body=None): payload = json.loads(body) assert payload["variables"] == {"id": "PR_example"} assert "mergePullRequest" not in payload["query"] - operation = "convertPullRequestToDraft" if "convertPullRequestToDraft" in payload["query"] else "markPullRequestReadyForReview" + query = payload["query"] + if ( + "mergeable" in query + and "convertPullRequestToDraft" not in query + and "markPullRequestReadyForReview" not in query + ): + return 200, {"data": {"node": {"mergeable": self.graphql_mergeable}}}, {} + operation = "convertPullRequestToDraft" if "convertPullRequestToDraft" in query else "markPullRequestReadyForReview" if self.graphql_error: return 200, {"errors": [{"message": "denied"}]}, {} if self.graphql_noop_draft and operation == "convertPullRequestToDraft": @@ -933,6 +942,108 @@ def test_write_author_ready_holds_against_red_or_pending_ci(status, conclusion): assert not fake.pull["draft"] +def test_write_author_ready_conflicts_return_to_draft_and_do_not_restore(): + fake = LifecycleAPI() + fake.comments.clear() + fake.permissions["author"] = { + "permission": "write", + "user": {"id": AUTHOR_ID, "login": "author", "type": "User"}, + } + fake.pull["mergeable"] = False + result = reconcile_pull(fake.api(), REPO, 1) + assert result.lifecycle["action"] == "draft" + assert fake.transitions == [True] + assert fake.pull["draft"] + follow = reconcile_pull(fake.api(), REPO, 1) + assert follow.lifecycle["action"] == "unchanged" + assert fake.transitions == [True] + assert fake.pull["draft"] + + +def test_write_author_ready_graphql_conflicting_returns_to_draft(): + fake = LifecycleAPI() + fake.comments.clear() + fake.permissions["author"] = { + "permission": "write", + "user": {"id": AUTHOR_ID, "login": "author", "type": "User"}, + } + fake.pull["mergeable"] = None + fake.graphql_mergeable = "CONFLICTING" + result = reconcile_pull(fake.api(), REPO, 1) + assert result.lifecycle["action"] == "draft" + assert fake.transitions == [True] + assert fake.pull["draft"] + + +def test_graphql_unknown_does_not_invent_conflicts_on_write_ready(): + fake = LifecycleAPI() + fake.comments.clear() + fake.permissions["author"] = { + "permission": "write", + "user": {"id": AUTHOR_ID, "login": "author", "type": "User"}, + } + fake.pull["mergeable"] = None + fake.graphql_mergeable = "UNKNOWN" + result = reconcile_pull(fake.api(), REPO, 1) + assert fake.transitions == [] + assert not fake.pull["draft"] + assert "Merge conflicts" not in result.lifecycle["reasons"] + + +@pytest.mark.parametrize("status,conclusion", [ + ("in_progress", None), + ("completed", "failure"), +]) +def test_write_hold_after_conflicts_disappear_on_reread_keeps_ready(status, conclusion, monkeypatch): + fake = LifecycleAPI() + fake.comments.clear() + fake.permissions["author"] = { + "permission": "write", + "user": {"id": AUTHOR_ID, "login": "author", "type": "User"}, + } + fake.runs[0].update(status=status, conclusion=conclusion) + fake.pull["mergeable"] = False + conflict_reads = [] + real = pr_lifecycle._has_merge_conflicts + + def wrapped(api, pull): + found = real(api, pull) + conflict_reads.append(found) + if found: + fake.pull["mergeable"] = True + return found + + monkeypatch.setattr(pr_lifecycle, "_has_merge_conflicts", wrapped) + result = reconcile_pull(fake.api(), REPO, 1) + assert result.lifecycle["action"] == "unchanged" + assert fake.transitions == [] + assert not fake.pull["draft"] + assert conflict_reads == [True, False] + + +@pytest.mark.parametrize("status,conclusion", [ + ("in_progress", None), + ("completed", "failure"), +]) +def test_write_author_ready_conflicts_with_red_ci_still_draft(status, conclusion): + fake = LifecycleAPI() + fake.comments.clear() + fake.permissions["author"] = { + "permission": "write", + "user": {"id": AUTHOR_ID, "login": "author", "type": "User"}, + } + fake.runs[0].update(status=status, conclusion=conclusion) + fake.pull["mergeable"] = False + result = reconcile_pull(fake.api(), REPO, 1) + assert result.lifecycle["action"] == "draft" + assert fake.transitions == [True] + assert fake.pull["draft"] + follow = reconcile_pull(fake.api(), REPO, 1) + assert follow.lifecycle["action"] == "unchanged" + assert fake.transitions == [True] + assert fake.pull["draft"] + + def test_auto_draft_then_timeline_restores_write_ready(): fake = LifecycleAPI() fake.comments.clear() From 47bc96df6e3b3905e41599aef071311c2b32431f Mon Sep 17 00:00:00 2001 From: TaprootFreakAI <315477232+TaprootFreakAI@users.noreply.github.com> Date: Sat, 12 Sep 2026 00:07:00 +0200 Subject: [PATCH 2/6] Clarify event versus sweep serialization in Guard docs. --- docs/a38-guard.md | 3 ++- src/agent_cli/pr_lifecycle.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/a38-guard.md b/docs/a38-guard.md index 2817c0a..0afeec3 100644 --- a/docs/a38-guard.md +++ b/docs/a38-guard.md @@ -244,9 +244,10 @@ The adopting workflow owns runner routing, `contents: write` for `pull-requests`/`issues`/`statuses` write access, and `actions: write` for initial workflow approval. The guard authorizes waiting allowlisted fork runs **before** it mutates Ready or Draft, so a failed convert-to-draft cannot skip approval. -Serialize event-driven single-PR runs and scheduled all-open sweeps in two +Serialize event-driven single-PR runs and all-open sweeps in two repository-wide concurrency groups (`event` versus `sweep`) with `cancel-in-progress: false`, so PR events cannot starve all-open reconciliation. +The `sweep` group is `schedule` or `workflow_dispatch` with `all_open`. Sweep jobs may use a 45-minute timeout. Run trusted `--all-open` reconciliation on a repository-configured schedule (for example every five minutes). GitHub may delay scheduled execution; this is not a real-time SLA. diff --git a/src/agent_cli/pr_lifecycle.py b/src/agent_cli/pr_lifecycle.py index 3145ec6..f3170b8 100644 --- a/src/agent_cli/pr_lifecycle.py +++ b/src/agent_cli/pr_lifecycle.py @@ -1,7 +1,7 @@ """Configured PR readiness, based on live CI rather than a cached green rollup. This reconciler never runs tests, submits reviews, or merges pull requests. -The caller must serialize all guard invocations for the repository. +The caller must serialize guard invocations within the `event` group and within the `sweep` group, not across both groups. """ from __future__ import annotations From 45c9ab4e0659958128d368ac636ac1258dea83c8 Mon Sep 17 00:00:00 2001 From: TaprootFreakAI <315477232+TaprootFreakAI@users.noreply.github.com> Date: Sat, 12 Sep 2026 08:14:47 +0200 Subject: [PATCH 3/6] Document workflow_dispatch all-open as a supported Guard event. --- docs/a38-guard.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/a38-guard.md b/docs/a38-guard.md index 0afeec3..941bc74 100644 --- a/docs/a38-guard.md +++ b/docs/a38-guard.md @@ -309,7 +309,7 @@ Supported events: - `pull_request_target`: opened, reopened, synchronize, edited, ready_for_review. - `issue_comment`: created, edited, deleted, for PRs only. - Scheduled all-open reconciliation on the trusted default branch; cadence is repository configuration. -- `workflow_dispatch`: an explicit repository and PR number. +- `workflow_dispatch`: an explicit repository and PR number, or `all_open=true` reconciliation. Issue-only events and the bot's own comments are ignored. The installed workflow deliberately has no `pull_request_review` trigger because that event loads workflow code from PR context. After approving or dismissing a policy review, post a normal PR comment such as `A38 recheck` for immediate reassessment, or dispatch the default-branch workflow. Scheduled reconciliation catches other review/base changes. The CLI can consume submitted/edited/dismissed review events supplied by an external trusted event handler, but never grant elevated credentials to PR-context workflow code. Never check out the PR head in a privileged bot job. From 4adf98ff273fcd951e75f1fc6189f91bd54a1ad9 Mon Sep 17 00:00:00 2001 From: TaprootFreakAI <315477232+TaprootFreakAI@users.noreply.github.com> Date: Sat, 12 Sep 2026 08:36:24 +0200 Subject: [PATCH 4/6] Persist the write-hold record when conflicts clear on re-read. --- src/agent_cli/pr_lifecycle.py | 25 ++++++++++++++++++++++++- tests/test_pr_lifecycle.py | 3 +++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/agent_cli/pr_lifecycle.py b/src/agent_cli/pr_lifecycle.py index f3170b8..529a296 100644 --- a/src/agent_cli/pr_lifecycle.py +++ b/src/agent_cli/pr_lifecycle.py @@ -549,7 +549,7 @@ def reconcile_lifecycle(api: Any, assessment: Any, *, dry_run: bool = False) -> if _has_merge_conflicts(api, final_pull): final_reasons.insert(0, "Merge conflicts") if target == "ready" and ( - "Merge conflicts" in final_reasons or final_pull.get("mergeable") is False + "Merge conflicts" in final_reasons or final_pull.get("mergeable") is not True ): return {"action": "unchanged", "reasons": final_reasons, "dry_run": False} if ( @@ -561,6 +561,29 @@ def reconcile_lifecycle(api: Any, assessment: Any, *, dry_run: bool = False) -> if target == "draft" and not final_reasons: return {"action": "unchanged", "reasons": [], "dry_run": False} if target == "draft" and write_hold and "Merge conflicts" not in final_reasons: + hold = { + "repo": assessment.repo, + "pr": assessment.pr, + "head": snap.head_sha, + "base": snap.base_sha, + "state": "ready", + "reasons": final_reasons, + "phase": "applied", + } + if not dry_run and ( + previous.get("phase") != "applied" + or previous.get("state") != "ready" + or previous.get("head") != snap.head_sha + or previous.get("base") != snap.base_sha + ): + _save_record( + api, + assessment, + STATE_MARKER, + hold, + "A write collaborator holds Ready; this pull request stays ready for review.", + "Ein Write-Collaborator hält Ready; dieser Pull Request bleibt bereit zum Review.", + ) return {"action": "unchanged", "reasons": final_reasons, "dry_run": False} record = {"repo": assessment.repo, "pr": assessment.pr, "head": snap.head_sha, "base": snap.base_sha, "state": target, "reasons": final_reasons, "phase": "planned"} diff --git a/tests/test_pr_lifecycle.py b/tests/test_pr_lifecycle.py index 2a44fdd..a921248 100644 --- a/tests/test_pr_lifecycle.py +++ b/tests/test_pr_lifecycle.py @@ -1019,6 +1019,9 @@ def wrapped(api, pull): assert fake.transitions == [] assert not fake.pull["draft"] assert conflict_reads == [True, False] + states = [c for c in fake.comments if c["body"].startswith(STATE_MARKER)] + assert states and '"phase": "applied"' in states[-1]["body"] + assert '"state": "ready"' in states[-1]["body"] @pytest.mark.parametrize("status,conclusion", [ From 19d77498f5a1a18b04813b013e593cbdde4ca42b Mon Sep 17 00:00:00 2001 From: TaprootFreakAI <315477232+TaprootFreakAI@users.noreply.github.com> Date: Sat, 12 Sep 2026 09:05:01 +0200 Subject: [PATCH 5/6] Write the hold record before the empty-reasons draft abort. --- src/agent_cli/pr_lifecycle.py | 4 ++-- tests/test_pr_lifecycle.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/agent_cli/pr_lifecycle.py b/src/agent_cli/pr_lifecycle.py index 529a296..adec682 100644 --- a/src/agent_cli/pr_lifecycle.py +++ b/src/agent_cli/pr_lifecycle.py @@ -558,8 +558,6 @@ def reconcile_lifecycle(api: Any, assessment: Any, *, dry_run: bool = False) -> and (final_reasons or final_pull.get("mergeable") is not True) ): return {"action": "unchanged", "reasons": final_reasons, "dry_run": False} - if target == "draft" and not final_reasons: - return {"action": "unchanged", "reasons": [], "dry_run": False} if target == "draft" and write_hold and "Merge conflicts" not in final_reasons: hold = { "repo": assessment.repo, @@ -585,6 +583,8 @@ def reconcile_lifecycle(api: Any, assessment: Any, *, dry_run: bool = False) -> "Ein Write-Collaborator hält Ready; dieser Pull Request bleibt bereit zum Review.", ) return {"action": "unchanged", "reasons": final_reasons, "dry_run": False} + if target == "draft" and not final_reasons: + return {"action": "unchanged", "reasons": [], "dry_run": False} record = {"repo": assessment.repo, "pr": assessment.pr, "head": snap.head_sha, "base": snap.base_sha, "state": target, "reasons": final_reasons, "phase": "planned"} _save_record(api, assessment, STATE_MARKER, record, diff --git a/tests/test_pr_lifecycle.py b/tests/test_pr_lifecycle.py index a921248..7935028 100644 --- a/tests/test_pr_lifecycle.py +++ b/tests/test_pr_lifecycle.py @@ -993,6 +993,7 @@ def test_graphql_unknown_does_not_invent_conflicts_on_write_ready(): @pytest.mark.parametrize("status,conclusion", [ ("in_progress", None), ("completed", "failure"), + ("completed", "success"), ]) def test_write_hold_after_conflicts_disappear_on_reread_keeps_ready(status, conclusion, monkeypatch): fake = LifecycleAPI() From ec2e795990caefabb5416b7cffc3fdbde7f7ff24 Mon Sep 17 00:00:00 2001 From: TaprootFreakAI <315477232+TaprootFreakAI@users.noreply.github.com> Date: Sat, 12 Sep 2026 09:16:43 +0200 Subject: [PATCH 6/6] Document the event-job timeout next to the sweep timeout. --- docs/a38-guard.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/a38-guard.md b/docs/a38-guard.md index 941bc74..64a4a42 100644 --- a/docs/a38-guard.md +++ b/docs/a38-guard.md @@ -248,7 +248,7 @@ Serialize event-driven single-PR runs and all-open sweeps in two repository-wide concurrency groups (`event` versus `sweep`) with `cancel-in-progress: false`, so PR events cannot starve all-open reconciliation. The `sweep` group is `schedule` or `workflow_dispatch` with `all_open`. -Sweep jobs may use a 45-minute timeout. Run trusted +Sweep jobs may use a 45-minute timeout; event-driven single-PR jobs may use 10 minutes. Run trusted `--all-open` reconciliation on a repository-configured schedule (for example every five minutes). GitHub may delay scheduled execution; this is not a real-time SLA. Privileged runs must never check out PR code. Bot readiness must not be wired to