From 440f9af56fa5ea021bd4c774f81128fbb35940e4 Mon Sep 17 00:00:00 2001 From: Darafei Praliaskouski Date: Thu, 23 Jul 2026 13:08:49 +0400 Subject: [PATCH 1/3] ci: sync needs rebase label with merge state --- .github/scripts/sync_codex_ok_labels.py | 62 +++++++++++++++++ openspec/specs/github-automation/spec.md | 28 +++++++- tests/unit/test_sync_codex_ok_labels.py | 86 ++++++++++++++++++++++++ 3 files changed, 174 insertions(+), 2 deletions(-) diff --git a/.github/scripts/sync_codex_ok_labels.py b/.github/scripts/sync_codex_ok_labels.py index 168300721a..0e650f5ace 100755 --- a/.github/scripts/sync_codex_ok_labels.py +++ b/.github/scripts/sync_codex_ok_labels.py @@ -16,6 +16,7 @@ CODEX_OK_LABEL = "🤖 codex: ok" CODEX_NEEDS_WORK_LABEL = "🤖 codex: needs work" +NEEDS_REBASE_LABEL = "needs rebase" LEGACY_CODEX_LABELS = {"🤖 codex-ok"} CODEX_REVIEW_AUTHORS = { "chatgpt-codex-connector", @@ -32,6 +33,8 @@ FAIL_CHECK_STATES = {"ACTION_REQUIRED", "CANCELLED", "ERROR", "FAILURE", "STALE", "TIMED_OUT"} PENDING_CHECK_STATES = {"EXPECTED", "IN_PROGRESS", "PENDING", "QUEUED", "REQUESTED", "WAITING"} UNMERGEABLE_STATES = {"DIRTY", "BLOCKED"} +NEEDS_REBASE_STATES = {"CONFLICTING", "DIRTY"} +NO_REBASE_STATES = {"BLOCKED", "CLEAN"} CODEX_LB_REQUIRED_CHECKS = frozenset( { "Frontend lint (eslint)", @@ -229,6 +232,9 @@ class SyncDecision: has_needs_work_label: bool wants_needs_work_label: bool needs_work_action: str + has_needs_rebase_label: bool + wants_needs_rebase_label: bool + needs_rebase_action: str legacy_labels: frozenset[str] reason: str review_url: str | None @@ -933,6 +939,16 @@ def pr_merge_state(repo: str, number: int) -> str: return merge_state or "UNKNOWN" +def needs_rebase_label_target(merge_state: str, *, has_label: bool) -> bool: + """Sync confirmed conflicts and preserve the label when GitHub is ambiguous.""" + + if merge_state in NEEDS_REBASE_STATES: + return True + if merge_state in NO_REBASE_STATES: + return False + return has_label + + def workflow_runs_requiring_approval(repo: str, head_sha: str) -> tuple[int, ...]: runs = paged_api(f"/repos/{repo}/actions/runs?event=pull_request&head_sha={head_sha}") run_ids: list[int] = [] @@ -1140,6 +1156,11 @@ def decide_pr( ) has_ok_label = CODEX_OK_LABEL in labels has_needs_work_label = CODEX_NEEDS_WORK_LABEL in labels + has_needs_rebase_label = NEEDS_REBASE_LABEL in labels + wants_needs_rebase_label = needs_rebase_label_target( + merge_state, + has_label=has_needs_rebase_label, + ) legacy_labels = frozenset(label for label in labels if label in LEGACY_CODEX_LABELS) reason_parts: list[str] = [] @@ -1204,6 +1225,12 @@ def decide_pr( needs_work_action = "remove" else: needs_work_action = "keep" + if wants_needs_rebase_label and not has_needs_rebase_label: + needs_rebase_action = "add" + elif not wants_needs_rebase_label and has_needs_rebase_label: + needs_rebase_action = "remove" + else: + needs_rebase_action = "keep" review_url = unresolved_finding_urls[0] if unresolved_finding_urls else None if review_url is None and isinstance(review_node, dict): @@ -1219,6 +1246,9 @@ def decide_pr( has_needs_work_label=has_needs_work_label, wants_needs_work_label=wants_needs_work_label, needs_work_action=needs_work_action, + has_needs_rebase_label=has_needs_rebase_label, + wants_needs_rebase_label=wants_needs_rebase_label, + needs_rebase_action=needs_rebase_action, legacy_labels=legacy_labels, reason="; ".join(reason_parts), review_url=review_url, @@ -1277,6 +1307,26 @@ def record(warning: str | None) -> None: action=f"remove {CODEX_NEEDS_WORK_LABEL} from {decision.repo}#{decision.number}", ) ) + if decision.needs_rebase_action == "add": + record( + gh_api_write( + f"/repos/{decision.repo}/issues/{decision.number}/labels", + method="POST", + input_json={"labels": [NEEDS_REBASE_LABEL]}, + tolerate_permission_errors=tolerate_permission_errors, + action=f"add {NEEDS_REBASE_LABEL} to {decision.repo}#{decision.number}", + ) + ) + elif decision.needs_rebase_action == "remove": + record( + gh_api_write( + f"/repos/{decision.repo}/issues/{decision.number}/labels/{quote(NEEDS_REBASE_LABEL, safe='')}", + method="DELETE", + tolerate_permission_errors=tolerate_permission_errors, + tolerate_missing=True, + action=f"remove {NEEDS_REBASE_LABEL} from {decision.repo}#{decision.number}", + ) + ) for label in decision.legacy_labels: record( gh_api_write( @@ -1414,6 +1464,16 @@ def main(argv: list[str] | None = None) -> int: tolerate_permission_errors=args.tolerate_write_permission_errors, ) ) + setup_warnings.extend( + ensure_label( + repo, + NEEDS_REBASE_LABEL, + color="fbca04", + description="Needs rebase or conflict repair against current main", + apply=args.apply, + tolerate_permission_errors=args.tolerate_write_permission_errors, + ) + ) for warning in setup_warnings: print(f"warning: {warning}", file=sys.stderr, flush=True) numbers = list_open_pr_numbers(repo) if args.all_open else list(args.pr or []) @@ -1476,6 +1536,8 @@ def main(argv: list[str] | None = None) -> int: f"ok={decision.has_ok_label}->{decision.wants_ok_label}/{decision.ok_action} " f"needs_work={decision.has_needs_work_label}->{decision.wants_needs_work_label}/" f"{decision.needs_work_action} " + f"needs_rebase={decision.has_needs_rebase_label}->{decision.wants_needs_rebase_label}/" + f"{decision.needs_rebase_action} " f"legacy={','.join(sorted(decision.legacy_labels)) or '-'} " f"approve_runs={','.join(str(run_id) for run_id in decision.approve_workflow_run_ids) or '-'} " f"trigger_codex={decision.trigger_codex_review and not args.no_trigger_missing_codex} " diff --git a/openspec/specs/github-automation/spec.md b/openspec/specs/github-automation/spec.md index f2a7f951dd..883db0bdb2 100644 --- a/openspec/specs/github-automation/spec.md +++ b/openspec/specs/github-automation/spec.md @@ -1,8 +1,33 @@ # github-automation Specification ## Purpose -Repository automation around the Codex review merge gate: the `Codex review labels` workflow and its synchronization script keep `🤖 codex: ok` / `🤖 codex: needs work` labels faithful to current-head CI state and Codex review evidence, with token sourcing that stays within GitHub API quotas and degrades safely when privileged credentials are unavailable. +Repository automation around the Codex review merge gate: the `Codex review labels` workflow and its synchronization script keep `🤖 codex: ok` / `🤖 codex: needs work` labels faithful to current-head CI state and Codex review evidence, keep `needs rebase` faithful to confirmed merge-conflict state, and use token sourcing that stays within GitHub API quotas and degrades safely when privileged credentials are unavailable. ## Requirements +### Requirement: Needs-rebase label sync + +The Codex label synchronization script MUST add `needs rebase` when GitHub +reports a confirmed merge conflict, MUST remove it when GitHub reports a known +non-conflict state, and MUST preserve its current value when merge state is +ambiguous. It MUST NOT infer a conflict from the pull request merely being +behind the base branch. + +#### Scenario: Confirmed conflict gains the label + +- **WHEN** GitHub reports the pull request as `CONFLICTING` or `DIRTY` +- **THEN** the synchronizer adds `needs rebase` + +#### Scenario: Review-blocked pull request loses a stale label + +- **GIVEN** a pull request has `needs rebase` +- **WHEN** GitHub reports it as `BLOCKED` by review or status requirements +- **THEN** the synchronizer removes `needs rebase` + +#### Scenario: Base lag alone does not create the label + +- **WHEN** GitHub reports a pull request as `BEHIND` without a confirmed conflict +- **THEN** the synchronizer preserves the current label state +- **AND** it does not add `needs rebase` to an unlabelled pull request + ### Requirement: Codex review label sync write-token fallback The `Codex review labels` workflow MUST execute the label synchronization script from the trusted default branch and MUST prefer a dedicated GitHub App installation token, then a repository-provided write token, before falling back to the default `github.token`. @@ -297,4 +322,3 @@ labels, so the override MUST NOT apply there: a change that would leave - **WHEN** a budget is exceeded - **THEN** no pull-request label set is resolved and the check fails regardless of any label on the originating pull request - diff --git a/tests/unit/test_sync_codex_ok_labels.py b/tests/unit/test_sync_codex_ok_labels.py index 5c9bfbe750..3d53ca7eca 100644 --- a/tests/unit/test_sync_codex_ok_labels.py +++ b/tests/unit/test_sync_codex_ok_labels.py @@ -31,6 +31,9 @@ def decision(module: ModuleType, **overrides: Any) -> Any: "has_needs_work_label": False, "wants_needs_work_label": False, "needs_work_action": "keep", + "has_needs_rebase_label": False, + "wants_needs_rebase_label": False, + "needs_rebase_action": "keep", "legacy_labels": frozenset(), "reason": "checks are pending", "review_url": None, @@ -44,6 +47,89 @@ def decision(module: ModuleType, **overrides: Any) -> Any: return module.SyncDecision(**values) +@pytest.mark.parametrize("merge_state", ["CONFLICTING", "DIRTY"]) +def test_needs_rebase_label_target_adds_for_confirmed_conflicts(merge_state: str) -> None: + module = load_sync_module() + + assert module.needs_rebase_label_target(merge_state, has_label=False) is True + + +@pytest.mark.parametrize("merge_state", ["BLOCKED", "CLEAN"]) +def test_needs_rebase_label_target_removes_for_known_non_conflict_states(merge_state: str) -> None: + module = load_sync_module() + + assert module.needs_rebase_label_target(merge_state, has_label=True) is False + + +@pytest.mark.parametrize("merge_state", ["BEHIND", "DRAFT", "HAS_HOOKS", "UNKNOWN", "UNSTABLE"]) +@pytest.mark.parametrize("has_label", [False, True]) +def test_needs_rebase_label_target_preserves_ambiguous_states( + merge_state: str, + has_label: bool, +) -> None: + module = load_sync_module() + + assert module.needs_rebase_label_target(merge_state, has_label=has_label) is has_label + + +def test_apply_decision_adds_needs_rebase_label(monkeypatch: pytest.MonkeyPatch) -> None: + module = load_sync_module() + calls: list[tuple[str, str, Any | None]] = [] + + def capture_write(path: str, *, method: str = "GET", input_json: Any | None = None) -> None: + calls.append((method, path, input_json)) + + monkeypatch.setattr(module, "gh_api", capture_write) + + warnings = module.apply_decision( + decision( + module, + ok_action="keep", + has_needs_rebase_label=False, + wants_needs_rebase_label=True, + needs_rebase_action="add", + ) + ) + + assert warnings == () + assert calls == [ + ( + "POST", + "/repos/Soju06/codex-lb/issues/714/labels", + {"labels": ["needs rebase"]}, + ) + ] + + +def test_apply_decision_removes_stale_needs_rebase_label(monkeypatch: pytest.MonkeyPatch) -> None: + module = load_sync_module() + calls: list[tuple[str, str, Any | None]] = [] + + def capture_write(path: str, *, method: str = "GET", input_json: Any | None = None) -> None: + calls.append((method, path, input_json)) + + monkeypatch.setattr(module, "gh_api", capture_write) + + warnings = module.apply_decision( + decision( + module, + ok_action="keep", + has_needs_rebase_label=True, + wants_needs_rebase_label=False, + needs_rebase_action="remove", + ) + ) + + assert warnings == () + assert calls == [ + ( + "DELETE", + "/repos/Soju06/codex-lb/issues/714/labels/needs%20rebase", + None, + ) + ] + + def test_classify_check_state_uses_latest_run_for_duplicate_check_names() -> None: module = load_sync_module() From 448af565ab9e5aa31dd17e78a6d4d2d6163af491 Mon Sep 17 00:00:00 2001 From: Darafei Praliaskouski Date: Thu, 23 Jul 2026 13:23:25 +0400 Subject: [PATCH 2/3] ci: remove stale rebase labels for mergeable states --- .github/scripts/sync_codex_ok_labels.py | 2 +- .../sync-needs-rebase-label/proposal.md | 33 +++++++++++++++++++ .../specs/github-automation/spec.md | 31 +++++++++++++++++ .../changes/sync-needs-rebase-label/tasks.md | 11 +++++++ openspec/specs/github-automation/spec.md | 6 ++-- tests/unit/test_sync_codex_ok_labels.py | 10 ++---- 6 files changed, 82 insertions(+), 11 deletions(-) create mode 100644 openspec/changes/sync-needs-rebase-label/proposal.md create mode 100644 openspec/changes/sync-needs-rebase-label/specs/github-automation/spec.md create mode 100644 openspec/changes/sync-needs-rebase-label/tasks.md diff --git a/.github/scripts/sync_codex_ok_labels.py b/.github/scripts/sync_codex_ok_labels.py index 0e650f5ace..fbfba84582 100755 --- a/.github/scripts/sync_codex_ok_labels.py +++ b/.github/scripts/sync_codex_ok_labels.py @@ -34,7 +34,7 @@ PENDING_CHECK_STATES = {"EXPECTED", "IN_PROGRESS", "PENDING", "QUEUED", "REQUESTED", "WAITING"} UNMERGEABLE_STATES = {"DIRTY", "BLOCKED"} NEEDS_REBASE_STATES = {"CONFLICTING", "DIRTY"} -NO_REBASE_STATES = {"BLOCKED", "CLEAN"} +NO_REBASE_STATES = {"BEHIND", "BLOCKED", "CLEAN", "DRAFT", "HAS_HOOKS", "UNSTABLE"} CODEX_LB_REQUIRED_CHECKS = frozenset( { "Frontend lint (eslint)", diff --git a/openspec/changes/sync-needs-rebase-label/proposal.md b/openspec/changes/sync-needs-rebase-label/proposal.md new file mode 100644 index 0000000000..87fa9c59dc --- /dev/null +++ b/openspec/changes/sync-needs-rebase-label/proposal.md @@ -0,0 +1,33 @@ +## Why + +The repository's `needs rebase` label is maintained manually even though the +existing Codex label workflow already resynchronizes every open pull request +every 15 minutes. A resolved conflict can therefore leave a stale blocker, and +base-branch lag or review policy can be mistaken for a conflict. + +## What Changes + +- The Codex label synchronizer adds `needs rebase` only for confirmed conflict + states. +- It removes the label for every known mergeable or non-conflict state, + including `BEHIND`, `BLOCKED`, `HAS_HOOKS`, and `UNSTABLE`. +- It preserves the current label only while GitHub reports an unknown merge + state. +- Focused tests cover the state mapping and exact add/remove API writes. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `github-automation`: the scheduled label sync also owns `needs rebase` + freshness. + +## Impact + +- Code: `.github/scripts/sync_codex_ok_labels.py` +- Tests: `tests/unit/test_sync_codex_ok_labels.py` +- Specs: `openspec/specs/github-automation/spec.md` diff --git a/openspec/changes/sync-needs-rebase-label/specs/github-automation/spec.md b/openspec/changes/sync-needs-rebase-label/specs/github-automation/spec.md new file mode 100644 index 0000000000..b729b2802f --- /dev/null +++ b/openspec/changes/sync-needs-rebase-label/specs/github-automation/spec.md @@ -0,0 +1,31 @@ +## ADDED Requirements + +### Requirement: Needs-rebase label sync + +The Codex label synchronization script MUST add `needs rebase` when GitHub +reports a confirmed merge conflict, MUST remove it when GitHub reports a known +mergeable or non-conflict state, and MUST preserve its current value only when +merge state is unknown. It MUST NOT infer a conflict from the pull request +merely being behind the base branch. + +#### Scenario: Confirmed conflict gains the label + +- **WHEN** GitHub reports the pull request as `CONFLICTING` or `DIRTY` +- **THEN** the synchronizer adds `needs rebase` + +#### Scenario: Review-blocked pull request loses a stale label + +- **GIVEN** a pull request has `needs rebase` +- **WHEN** GitHub reports it as `BLOCKED` by review or status requirements +- **THEN** the synchronizer removes `needs rebase` + +#### Scenario: Mergeable status loses a stale label + +- **GIVEN** a pull request has `needs rebase` +- **WHEN** GitHub reports `BEHIND`, `CLEAN`, `DRAFT`, `HAS_HOOKS`, or `UNSTABLE` +- **THEN** the synchronizer removes `needs rebase` + +#### Scenario: Unknown state preserves current evidence + +- **WHEN** GitHub reports an unknown merge state +- **THEN** the synchronizer preserves the current `needs rebase` value diff --git a/openspec/changes/sync-needs-rebase-label/tasks.md b/openspec/changes/sync-needs-rebase-label/tasks.md new file mode 100644 index 0000000000..1beefd78fc --- /dev/null +++ b/openspec/changes/sync-needs-rebase-label/tasks.md @@ -0,0 +1,11 @@ +## 1. Label synchronization + +- [x] 1.1 Classify confirmed conflict, known non-conflict, and unknown merge states. +- [x] 1.2 Add or remove `needs rebase` through the existing idempotent label write path. +- [x] 1.3 Ensure the label exists and expose its decision in dry-run output. + +## 2. Validation + +- [x] 2.1 Cover merge-state decisions and exact GitHub API writes with unit tests. +- [x] 2.2 Verify current live conflict, stale-label, and unknown-state examples in read-only dry-run mode. +- [x] 2.3 Validate the focused test suite, Ruff, and OpenSpec strictly. diff --git a/openspec/specs/github-automation/spec.md b/openspec/specs/github-automation/spec.md index 883db0bdb2..503e4c27db 100644 --- a/openspec/specs/github-automation/spec.md +++ b/openspec/specs/github-automation/spec.md @@ -22,11 +22,11 @@ behind the base branch. - **WHEN** GitHub reports it as `BLOCKED` by review or status requirements - **THEN** the synchronizer removes `needs rebase` -#### Scenario: Base lag alone does not create the label +#### Scenario: Base lag alone removes a stale label - **WHEN** GitHub reports a pull request as `BEHIND` without a confirmed conflict -- **THEN** the synchronizer preserves the current label state -- **AND** it does not add `needs rebase` to an unlabelled pull request +- **THEN** the synchronizer removes `needs rebase` when present +- **AND** it does not add the label to an unlabelled pull request ### Requirement: Codex review label sync write-token fallback diff --git a/tests/unit/test_sync_codex_ok_labels.py b/tests/unit/test_sync_codex_ok_labels.py index 3d53ca7eca..cd60a45a59 100644 --- a/tests/unit/test_sync_codex_ok_labels.py +++ b/tests/unit/test_sync_codex_ok_labels.py @@ -54,22 +54,18 @@ def test_needs_rebase_label_target_adds_for_confirmed_conflicts(merge_state: str assert module.needs_rebase_label_target(merge_state, has_label=False) is True -@pytest.mark.parametrize("merge_state", ["BLOCKED", "CLEAN"]) +@pytest.mark.parametrize("merge_state", ["BEHIND", "BLOCKED", "CLEAN", "DRAFT", "HAS_HOOKS", "UNSTABLE"]) def test_needs_rebase_label_target_removes_for_known_non_conflict_states(merge_state: str) -> None: module = load_sync_module() assert module.needs_rebase_label_target(merge_state, has_label=True) is False -@pytest.mark.parametrize("merge_state", ["BEHIND", "DRAFT", "HAS_HOOKS", "UNKNOWN", "UNSTABLE"]) @pytest.mark.parametrize("has_label", [False, True]) -def test_needs_rebase_label_target_preserves_ambiguous_states( - merge_state: str, - has_label: bool, -) -> None: +def test_needs_rebase_label_target_preserves_unknown_state(has_label: bool) -> None: module = load_sync_module() - assert module.needs_rebase_label_target(merge_state, has_label=has_label) is has_label + assert module.needs_rebase_label_target("UNKNOWN", has_label=has_label) is has_label def test_apply_decision_adds_needs_rebase_label(monkeypatch: pytest.MonkeyPatch) -> None: From fd11828ea139128e38af8f8e85c5f07664a58da4 Mon Sep 17 00:00:00 2001 From: Darafei Praliaskouski Date: Thu, 23 Jul 2026 13:31:00 +0400 Subject: [PATCH 3/3] docs: clarify rebase label state mapping --- openspec/specs/github-automation/spec.md | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/openspec/specs/github-automation/spec.md b/openspec/specs/github-automation/spec.md index 503e4c27db..ddec209a68 100644 --- a/openspec/specs/github-automation/spec.md +++ b/openspec/specs/github-automation/spec.md @@ -7,9 +7,9 @@ Repository automation around the Codex review merge gate: the `Codex review labe The Codex label synchronization script MUST add `needs rebase` when GitHub reports a confirmed merge conflict, MUST remove it when GitHub reports a known -non-conflict state, and MUST preserve its current value when merge state is -ambiguous. It MUST NOT infer a conflict from the pull request merely being -behind the base branch. +mergeable or non-conflict state, and MUST preserve its current value only when +GitHub reports `UNKNOWN`. It MUST NOT infer a conflict from the pull request +merely being behind the base branch. #### Scenario: Confirmed conflict gains the label @@ -28,6 +28,17 @@ behind the base branch. - **THEN** the synchronizer removes `needs rebase` when present - **AND** it does not add the label to an unlabelled pull request +#### Scenario: Other mergeable statuses remove a stale label + +- **GIVEN** a pull request has `needs rebase` +- **WHEN** GitHub reports `CLEAN`, `DRAFT`, `HAS_HOOKS`, or `UNSTABLE` +- **THEN** the synchronizer removes `needs rebase` + +#### Scenario: Unknown state preserves current evidence + +- **WHEN** GitHub reports `UNKNOWN` +- **THEN** the synchronizer preserves the current `needs rebase` value + ### Requirement: Codex review label sync write-token fallback The `Codex review labels` workflow MUST execute the label synchronization script from the trusted default branch and MUST prefer a dedicated GitHub App installation token, then a repository-provided write token, before falling back to the default `github.token`.