diff --git a/docs/a38-guard.md b/docs/a38-guard.md index fa2c383..64a4a42 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,11 @@ 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 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; 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 @@ -277,7 +283,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 @@ -303,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. 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..adec682 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 @@ -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,14 +546,43 @@ 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 not True + ): + return {"action": "unchanged", "reasons": final_reasons, "dry_run": False} if ( target == "ready" and not restore_write_ready and (final_reasons or final_pull.get("mergeable") is not True) ): return {"action": "unchanged", "reasons": final_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} 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, diff --git a/tests/test_pr_lifecycle.py b/tests/test_pr_lifecycle.py index 0863e94..7935028 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,112 @@ 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"), + ("completed", "success"), +]) +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] + 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", [ + ("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()