From 60a2250f58f7668accc8f9c60f97972b69e54f3e Mon Sep 17 00:00:00 2001 From: Codex Automation Date: Sat, 1 Aug 2026 11:06:11 -0500 Subject: [PATCH 1/4] fix(sync): classify covered consumer drift --- .../consumer-sync-drift-issue-body.test.js | 4 + .../scripts/consumer_sync_drift_issue_body.js | 16 ++- .../health-68-consumer-sync-drift.yml | 2 +- docs/ops/CONSUMER_REPO_MAINTENANCE.md | 9 ++ scripts/check_consumer_sync_drift.py | 127 +++++++++++++++++- .../scripts/test_check_consumer_sync_drift.py | 50 ++----- 6 files changed, 165 insertions(+), 43 deletions(-) diff --git a/.github/scripts/__tests__/consumer-sync-drift-issue-body.test.js b/.github/scripts/__tests__/consumer-sync-drift-issue-body.test.js index 40eb0417d..dd23d0132 100644 --- a/.github/scripts/__tests__/consumer-sync-drift-issue-body.test.js +++ b/.github/scripts/__tests__/consumer-sync-drift-issue-body.test.js @@ -105,6 +105,10 @@ test('compactMarkerPayload exposes the current drift checkpoint', () => { assert.equal(payload.follow_up.workflow, 'maint-68-sync-consumer-repos.yml'); }); +test('formatIssueComment suppresses covered-state noise', () => { + assert.equal(formatIssueComment({ ...report, status: 'covered' }), ''); +}); + test('mergeIssueBody refreshes generated issue bodies', () => { const oldBody = formatIssueBody({ counts: { drift: 1, missing: 0, errors: 0, obsolete: 0 }, diff --git a/.github/scripts/consumer_sync_drift_issue_body.js b/.github/scripts/consumer_sync_drift_issue_body.js index d5dc40e62..eaf2a4bc1 100644 --- a/.github/scripts/consumer_sync_drift_issue_body.js +++ b/.github/scripts/consumer_sync_drift_issue_body.js @@ -122,12 +122,16 @@ function formatIssueBody(report, options = {}) { const runLink = runUrl && runNumber ? `[Run #${runNumber}](${runUrl})` : runUrl || 'current run'; const openSyncPrs = formatOpenSyncPrs(report); + const remediation = report && report.sync_remediation ? report.sync_remediation : {}; + const isCovered = report && report.status === 'covered'; const lines = [ '## Consumer Repo Drift Detected', '', '> **Durable tracker** — see [`docs/ops/DURABLE_TRACKING_ISSUES.md`](https://github.com/stranske/Workflows/blob/main/docs/ops/DURABLE_TRACKING_ISSUES.md). The body below is regenerated each cycle by `health-68-consumer-sync-drift.yml`; auto-resolves on the next clean run.', '', - 'One or more consumer repos have drifted from the Workflows templates or manifest entries.', + isCovered + ? 'Detected drift is covered by current, unexpired compiler-plan sync PRs; no tracker comment is needed.' + : 'One or more consumer repos have actionable drift from the Workflows templates or manifest entries.', '', `**Check Details:** ${runLink}`, `**Counts:** ${countsLine(report)}`, @@ -144,6 +148,13 @@ function formatIssueBody(report, options = {}) { '- Close this issue when Health 68 passes.', '', ]; + if (remediation.expected_branch) { + lines.splice(lines.indexOf('### Required Actions'), 0, + '### Remediation state', + `- Current plan branch: \`${remediation.expected_branch}\``, + `- Coverage lease: ${remediation.coverage_lease_hours || 0} hours`, + ''); + } if (openSyncPrs.length > 0) { lines.push( '### Open sync PRs', @@ -175,6 +186,9 @@ function mergeIssueBody(existingBody, report, options = {}) { } function formatIssueComment(report, options = {}) { + if (report && report.status === 'covered') { + return ''; + } const runUrl = options.runUrl || ''; const runNumber = options.runNumber || ''; const runLink = runUrl && runNumber ? `[run #${runNumber}](${runUrl})` : runUrl || 'latest run'; diff --git a/.github/workflows/health-68-consumer-sync-drift.yml b/.github/workflows/health-68-consumer-sync-drift.yml index b8b15df48..61b84da0c 100644 --- a/.github/workflows/health-68-consumer-sync-drift.yml +++ b/.github/workflows/health-68-consumer-sync-drift.yml @@ -13,7 +13,7 @@ on: - 'scripts/**' - 'tools/**' schedule: - - cron: '10 5 * * *' # Daily at 5:10 UTC + - cron: '40 5 * * *' # Daily after Maint 68 and the 05:30 UTC janitor workflow_dispatch: inputs: repos: diff --git a/docs/ops/CONSUMER_REPO_MAINTENANCE.md b/docs/ops/CONSUMER_REPO_MAINTENANCE.md index 3a98adee0..26dda0efc 100644 --- a/docs/ops/CONSUMER_REPO_MAINTENANCE.md +++ b/docs/ops/CONSUMER_REPO_MAINTENANCE.md @@ -13,6 +13,15 @@ The list of registered repos lives in that workflow (env var `REGISTERED_CONSUMER_REPOS`). Avoid duplicating the list here; it changes over time and the workflow is the source of truth. +### Drift coverage states + +Health 68 evaluates only after Maint 68 and the Maint 71 janitor. It classifies each +consumer as `converged`, `covered`, `blocked`, `untracked_drift`, or `stale`. An open +sync PR covers drift only when its `sync/workflows-` branch matches the +current compiled plan and it is within the 36-hour coverage lease. Fully covered drift +exits zero and does not append a durable-tracker comment; stale, blocked, and untracked +states remain actionable failures. + ### Adding a New Consumer Repo 1. Add the repo to `REGISTERED_CONSUMER_REPOS` in `maint-68-sync-consumer-repos.yml`. diff --git a/scripts/check_consumer_sync_drift.py b/scripts/check_consumer_sync_drift.py index 108eb4960..1a77ed126 100755 --- a/scripts/check_consumer_sync_drift.py +++ b/scripts/check_consumer_sync_drift.py @@ -8,6 +8,7 @@ import hashlib import json import os +from datetime import UTC, datetime, timedelta from pathlib import Path import requests @@ -35,6 +36,7 @@ SUMMARY_ITEM_LIMIT = 50 CONTENT_ERROR_THRESHOLD = 5 SYNC_BRANCH_PREFIX = "sync/workflows-" +SYNC_COVERAGE_MAX_AGE = timedelta(hours=36) TOKEN_ENV_ORDER = ( "DRIFT_TOKEN", "SERVICE_BOT_PAT", @@ -394,6 +396,95 @@ def fetch_open_sync_prs( return prs, None +def parse_github_timestamp(value: object) -> datetime | None: + """Parse GitHub's UTC timestamp shape without making malformed data current.""" + if not isinstance(value, str) or not value: + return None + try: + return datetime.fromisoformat(value.replace("Z", "+00:00")).astimezone(UTC) + except ValueError: + return None + + +def build_remediation_states( + *, + repos: list[str], + drift: set[str], + missing: set[str], + errors: set[str], + obsolete: set[str], + open_sync_prs: list[dict[str, object]], + sync_pr_lookup_errors: list[str], + expected_branch: str, + now: datetime | None = None, +) -> dict[str, dict[str, object]]: + """Classify drift using the current compiler-plan branch, not PR presence alone.""" + current_time = now or datetime.now(UTC) + gaps_by_repo: dict[str, list[str]] = {repo: [] for repo in repos} + for category, items in ( + ("drift", drift), + ("missing", missing), + ("errors", errors), + ("obsolete", obsolete), + ): + for item in items: + repo, _detail = split_report_item(item) + if repo in gaps_by_repo: + gaps_by_repo[repo].append(category) + + lookup_failed = {item.split(":", 1)[0] for item in sync_pr_lookup_errors} + prs_by_repo: dict[str, list[dict[str, object]]] = {repo: [] for repo in repos} + for pr in open_sync_prs: + repo = str(pr.get("repo", "")) + if repo in prs_by_repo: + prs_by_repo[repo].append(pr) + + states: dict[str, dict[str, object]] = {} + for repo in repos: + categories = sorted(set(gaps_by_repo[repo])) + if not categories: + states[repo] = {"state": "converged", "reason": "no drift detected"} + continue + if repo in lookup_failed or "errors" in categories: + states[repo] = { + "state": "blocked", + "reason": "lookup or content error", + "categories": categories, + } + continue + + current = [pr for pr in prs_by_repo[repo] if pr.get("branch") == expected_branch] + fresh = [] + for pr in current: + updated = parse_github_timestamp(pr.get("updated_at")) + if updated is not None and current_time - updated <= SYNC_COVERAGE_MAX_AGE: + fresh.append(pr) + if fresh: + states[repo] = { + "state": "covered", + "reason": "current compiler-plan sync PR is open", + "expected_branch": expected_branch, + "pr": fresh[0], + "categories": categories, + } + elif current: + states[repo] = { + "state": "stale", + "reason": "current compiler-plan sync PR exceeded coverage lease", + "expected_branch": expected_branch, + "pr": current[0], + "categories": categories, + } + else: + states[repo] = { + "state": "untracked_drift", + "reason": "no open sync PR matches the current compiler plan", + "expected_branch": expected_branch, + "categories": categories, + } + return states + + def record_content_error( *, errors: set[str], @@ -428,6 +519,8 @@ def build_report( open_sync_prs: list[dict[str, object]] | None = None, sync_pr_lookup_errors: list[str] | None = None, token_diagnostics: dict[str, object] | None = None, + current_plan_id: str = "", + now: datetime | None = None, ) -> dict[str, object]: skipped = skipped or set() open_sync_prs = open_sync_prs or [] @@ -438,7 +531,27 @@ def build_report( "errors": len(errors), "obsolete": len(obsolete), } - status = "pass" if all(value == 0 for value in counts.values()) else "drift" + expected_branch = "" + if current_plan_id.startswith("sha256:"): + expected_branch = f"{SYNC_BRANCH_PREFIX}{current_plan_id.split(':', 1)[1][:12]}" + remediation_states = build_remediation_states( + repos=repos, + drift=drift, + missing=missing, + errors=errors, + obsolete=obsolete, + open_sync_prs=open_sync_prs, + sync_pr_lookup_errors=sync_pr_lookup_errors, + expected_branch=expected_branch, + now=now, + ) + state_values = {str(item["state"]) for item in remediation_states.values()} + if state_values <= {"converged"}: + status = "converged" + elif state_values <= {"converged", "covered"}: + status = "covered" + else: + status = "drift" repo_summaries = build_repo_summaries( repos=repos, drift=drift, @@ -450,9 +563,7 @@ def build_report( targeted_repos = [str(item["repo"]) for item in top_repo_gaps] open_sync_repo_count = len({str(item.get("repo", "")) for item in open_sync_prs if item}) latest_open_sync_pr = open_sync_prs[0] if open_sync_prs else None - remediation_state = "pass" - if status != "pass": - remediation_state = "pending_sync_prs" if open_sync_prs else "needs_sync" + remediation_state = status report: dict[str, object] = { "schema": REPORT_SCHEMA, "status": status, @@ -487,6 +598,10 @@ def build_report( }, "sync_remediation": { "state": remediation_state, + "plan_id": current_plan_id, + "expected_branch": expected_branch, + "coverage_lease_hours": int(SYNC_COVERAGE_MAX_AGE.total_seconds() // 3600), + "repo_states": remediation_states, "open_pr_count": len(open_sync_prs), "repo_count": open_sync_repo_count, "latest_open_pr": latest_open_sync_pr, @@ -738,6 +853,7 @@ def main() -> int: return 1 sections = list(COPY_SYNCED_SECTIONS) + current_plan_id = str(compiled.to_plan()["plan_id"]) session, token_diagnostics = select_read_token( candidates=candidates, @@ -848,11 +964,12 @@ def _check_file(local_file: Path, remote_target: str, repo: str) -> None: open_sync_prs=open_sync_prs, sync_pr_lookup_errors=sync_pr_lookup_errors, token_diagnostics=token_diagnostics, + current_plan_id=current_plan_id, ) write_report_json(args.report_json, report) write_summary_markdown(args.summary, report) - if report["status"] != "pass": + if report["status"] not in {"converged", "covered"}: print("::warning::Consumer repo drift detected") return 1 diff --git a/tests/scripts/test_check_consumer_sync_drift.py b/tests/scripts/test_check_consumer_sync_drift.py index 45e700143..9c892936d 100644 --- a/tests/scripts/test_check_consumer_sync_drift.py +++ b/tests/scripts/test_check_consumer_sync_drift.py @@ -1,4 +1,5 @@ import json +from datetime import UTC, datetime from scripts import check_consumer_sync_drift from scripts.sync_manifest_compiler import ( @@ -79,15 +80,8 @@ def test_build_report_returns_machine_readable_counts() -> None: ), } assert report["summary_limits"]["content_error_threshold_per_repo"] == 5 - assert report["sync_remediation"] == { - "state": "needs_sync", - "open_pr_count": 0, - "repo_count": 0, - "latest_open_pr": None, - "stale_open_pr_count": 0, - "open_prs": [], - "lookup_errors": [], - } + assert report["sync_remediation"]["state"] == "drift" + assert report["sync_remediation"]["repo_states"]["owner/a"]["state"] == "untracked_drift" assert report["drift"] == ["owner/b: .github/workflows/a.yml"] assert report["token_diagnostics"] == token_diagnostics @@ -176,14 +170,14 @@ def test_build_report_surfaces_manifest_skips_without_failing() -> None: skipped={"owner/custom: AGENTS.md (Uses historical Agents.md casing)"}, ) - assert report["status"] == "pass" + assert report["status"] == "converged" assert report["counts"] == {"drift": 0, "missing": 0, "errors": 0, "obsolete": 0} assert report["skip_count"] == 1 assert report["skipped"] == ["owner/custom: AGENTS.md (Uses historical Agents.md casing)"] - assert report["sync_remediation"]["state"] == "pass" + assert report["sync_remediation"]["state"] == "converged" -def test_build_report_surfaces_pending_sync_prs() -> None: +def test_build_report_marks_current_sync_pr_as_covered() -> None: report = check_consumer_sync_drift.build_report( repos=["owner/repo"], drift={"owner/repo: .github/workflows/a.yml"}, @@ -195,33 +189,17 @@ def test_build_report_surfaces_pending_sync_prs() -> None: "repo": "owner/repo", "number": 12, "url": "https://github.com/owner/repo/pull/12", - "branch": "sync/workflows-abc123", + "branch": "sync/workflows-aaaaaaaaaaaa", + "updated_at": "2026-04-26T01:00:00Z", } ], - sync_pr_lookup_errors=["owner/other: sync PR lookup failed (HTTP 403)"], + current_plan_id="sha256:" + "a" * 64, + now=datetime(2026, 4, 26, 2, 0, tzinfo=UTC), ) - assert report["sync_remediation"] == { - "state": "pending_sync_prs", - "open_pr_count": 1, - "repo_count": 1, - "latest_open_pr": { - "repo": "owner/repo", - "number": 12, - "url": "https://github.com/owner/repo/pull/12", - "branch": "sync/workflows-abc123", - }, - "stale_open_pr_count": 0, - "open_prs": [ - { - "repo": "owner/repo", - "number": 12, - "url": "https://github.com/owner/repo/pull/12", - "branch": "sync/workflows-abc123", - } - ], - "lookup_errors": ["owner/other: sync PR lookup failed (HTTP 403)"], - } + assert report["status"] == "covered" + assert report["sync_remediation"]["expected_branch"] == "sync/workflows-aaaaaaaaaaaa" + assert report["sync_remediation"]["repo_states"]["owner/repo"]["state"] == "covered" def test_fetch_open_sync_prs_filters_to_workflows_sync_branches() -> None: @@ -437,7 +415,7 @@ def test_write_report_json_creates_parent_directory(tmp_path) -> None: loaded = json.loads(output.read_text(encoding="utf-8")) assert loaded["schema"] == "workflows-consumer-sync-drift/v1" - assert loaded["status"] == "pass" + assert loaded["status"] == "converged" def test_write_summary_markdown_groups_and_bounds_items(tmp_path) -> None: From 4651719d58c5e7db2927619dcc5bc3d3c7ae43cf Mon Sep 17 00:00:00 2001 From: Codex Automation Date: Sat, 1 Aug 2026 11:25:56 -0500 Subject: [PATCH 2/4] fix(sync): harden covered drift review findings --- .../consumer-sync-drift-issue-body.test.js | 19 +++++ .../health-68-consumer-sync-drift.yml | 23 ++++-- .github/workflows/maint-71-merge-sync-prs.yml | 4 +- docs/ops/CONSUMER_REPO_MAINTENANCE.md | 8 +- scripts/check_consumer_sync_drift.py | 37 +++++++++- .../scripts/test_check_consumer_sync_drift.py | 73 ++++++++++++++++++- 6 files changed, 146 insertions(+), 18 deletions(-) diff --git a/.github/scripts/__tests__/consumer-sync-drift-issue-body.test.js b/.github/scripts/__tests__/consumer-sync-drift-issue-body.test.js index dd23d0132..15afe09bd 100644 --- a/.github/scripts/__tests__/consumer-sync-drift-issue-body.test.js +++ b/.github/scripts/__tests__/consumer-sync-drift-issue-body.test.js @@ -109,6 +109,25 @@ test('formatIssueComment suppresses covered-state noise', () => { assert.equal(formatIssueComment({ ...report, status: 'covered' }), ''); }); +test('formatIssueBody renders coverage details while preserving actionable output', () => { + const covered = formatIssueBody({ + ...report, + status: 'covered', + sync_remediation: { + ...report.sync_remediation, + expected_branch: 'sync/workflows-aaaaaaaaaaaa', + coverage_lease_hours: 36, + }, + }); + assert.match(covered, /covered by current, unexpired compiler-plan sync PRs/); + assert.match(covered, /Current plan branch: `sync\/workflows-aaaaaaaaaaaa`/); + assert.match(covered, /Coverage lease: 36 hours/); + + const actionable = formatIssueBody(report); + assert.match(actionable, /One or more consumer repos have actionable drift/); + assert.doesNotMatch(actionable, /Current plan branch:/); +}); + test('mergeIssueBody refreshes generated issue bodies', () => { const oldBody = formatIssueBody({ counts: { drift: 1, missing: 0, errors: 0, obsolete: 0 }, diff --git a/.github/workflows/health-68-consumer-sync-drift.yml b/.github/workflows/health-68-consumer-sync-drift.yml index 61b84da0c..642ed81a9 100644 --- a/.github/workflows/health-68-consumer-sync-drift.yml +++ b/.github/workflows/health-68-consumer-sync-drift.yml @@ -12,8 +12,9 @@ on: - '.github/scripts/**' - 'scripts/**' - 'tools/**' - schedule: - - cron: '40 5 * * *' # Daily after Maint 68 and the 05:30 UTC janitor + workflow_run: + workflows: [Merge Sync PRs] + types: [completed] workflow_dispatch: inputs: repos: @@ -32,6 +33,9 @@ concurrency: jobs: check-drift: name: Validate consumer repo drift + if: >- + github.event_name != 'workflow_run' || + (github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.head_branch == 'main') runs-on: ubuntu-latest steps: - name: Checkout Workflows repo @@ -183,15 +187,20 @@ jobs: return; } console.log(`Refreshed drift issue body for #${tracker.number}`); + const comment = formatIssueComment(report, { + runUrl, + runId: process.env.GITHUB_RUN_ID, + runNumber: process.env.GITHUB_RUN_NUMBER, + }); + if (!comment.trim()) { + console.log(`Skipped empty comment for existing issue #${tracker.number}`); + return; + } const commentResult = await skipOnRateLimit('comment', () => appendTrackerComment({ github, context, tracker, - comment: formatIssueComment(report, { - runUrl, - runId: process.env.GITHUB_RUN_ID, - runNumber: process.env.GITHUB_RUN_NUMBER, - }), + comment, core, withRetry, })); diff --git a/.github/workflows/maint-71-merge-sync-prs.yml b/.github/workflows/maint-71-merge-sync-prs.yml index 80c5fb418..505a478d8 100644 --- a/.github/workflows/maint-71-merge-sync-prs.yml +++ b/.github/workflows/maint-71-merge-sync-prs.yml @@ -20,8 +20,8 @@ name: Merge Sync PRs on: schedule: # Daily janitor pass: auto-merge ready sync PRs, close superseded ones, and prune - # leftover branches without waiting for a manual dispatch. Runs just after Health 68's - # drift scan (cron '10 5') so it acts on a fresh picture. A schedule event carries no + # leftover branches without waiting for a manual dispatch. Health 68 runs from this + # workflow's successful completion, so it evaluates the post-janitor state. A schedule event carries no # inputs, so the script's parseBooleanInput defaults apply unchanged: # auto_merge=true, dry_run=false, cleanup_branches=true, repos=all. - cron: '30 5 * * *' diff --git a/docs/ops/CONSUMER_REPO_MAINTENANCE.md b/docs/ops/CONSUMER_REPO_MAINTENANCE.md index 26dda0efc..72be1d314 100644 --- a/docs/ops/CONSUMER_REPO_MAINTENANCE.md +++ b/docs/ops/CONSUMER_REPO_MAINTENANCE.md @@ -15,12 +15,14 @@ the workflow is the source of truth. ### Drift coverage states -Health 68 evaluates only after Maint 68 and the Maint 71 janitor. It classifies each +Scheduled Health 68 runs are triggered only after a successful Maint 71 janitor; push and +manual runs are intentionally immediate. It classifies each consumer as `converged`, `covered`, `blocked`, `untracked_drift`, or `stale`. An open sync PR covers drift only when its `sync/workflows-` branch matches the current compiled plan and it is within the 36-hour coverage lease. Fully covered drift -exits zero and does not append a durable-tracker comment; stale, blocked, and untracked -states remain actionable failures. +exits zero and does not append a durable-tracker comment; stale (including expired +coverage), blocked (including global/lookup failures), and untracked states remain +actionable failures. ### Adding a New Consumer Repo diff --git a/scripts/check_consumer_sync_drift.py b/scripts/check_consumer_sync_drift.py index 1a77ed126..9d66eebec 100755 --- a/scripts/check_consumer_sync_drift.py +++ b/scripts/check_consumer_sync_drift.py @@ -373,6 +373,7 @@ def fetch_open_sync_prs( branch = str(head.get("ref", "")).strip() if not branch.startswith(SYNC_BRANCH_PREFIX): continue + head_repo = head.get("repo") if isinstance(head.get("repo"), dict) else {} prs.append( { "repo": repo, @@ -381,6 +382,7 @@ def fetch_open_sync_prs( "url": item.get("html_url", ""), "branch": branch, "head_sha": head.get("sha", ""), + "head_repo": head_repo.get("full_name", ""), "created_at": item.get("created_at", ""), "updated_at": item.get("updated_at", ""), } @@ -401,9 +403,12 @@ def parse_github_timestamp(value: object) -> datetime | None: if not isinstance(value, str) or not value: return None try: - return datetime.fromisoformat(value.replace("Z", "+00:00")).astimezone(UTC) + parsed = datetime.fromisoformat(value.replace("Z", "+00:00")) except ValueError: return None + if parsed.tzinfo is None: + return None + return parsed.astimezone(UTC) def build_remediation_states( @@ -416,6 +421,7 @@ def build_remediation_states( open_sync_prs: list[dict[str, object]], sync_pr_lookup_errors: list[str], expected_branch: str, + global_errors: list[str] | None = None, now: datetime | None = None, ) -> dict[str, dict[str, object]]: """Classify drift using the current compiler-plan branch, not PR presence alone.""" @@ -432,6 +438,7 @@ def build_remediation_states( if repo in gaps_by_repo: gaps_by_repo[repo].append(category) + global_errors = global_errors or [] lookup_failed = {item.split(":", 1)[0] for item in sync_pr_lookup_errors} prs_by_repo: dict[str, list[dict[str, object]]] = {repo: [] for repo in repos} for pr in open_sync_prs: @@ -442,6 +449,14 @@ def build_remediation_states( states: dict[str, dict[str, object]] = {} for repo in repos: categories = sorted(set(gaps_by_repo[repo])) + if global_errors: + states[repo] = { + "state": "blocked", + "reason": "global comparison error", + "categories": categories, + "global_errors": global_errors, + } + continue if not categories: states[repo] = {"state": "converged", "reason": "no drift detected"} continue @@ -453,7 +468,11 @@ def build_remediation_states( } continue - current = [pr for pr in prs_by_repo[repo] if pr.get("branch") == expected_branch] + current = [ + pr + for pr in prs_by_repo[repo] + if pr.get("branch") == expected_branch and pr.get("head_repo") == repo + ] fresh = [] for pr in current: updated = parse_github_timestamp(pr.get("updated_at")) @@ -534,6 +553,14 @@ def build_report( expected_branch = "" if current_plan_id.startswith("sha256:"): expected_branch = f"{SYNC_BRANCH_PREFIX}{current_plan_id.split(':', 1)[1][:12]}" + known_repos = set(repos) + global_errors = sorted( + item + for item in [*errors, *sync_pr_lookup_errors] + if split_report_item(item)[0] not in known_repos + ) + if not repos: + global_errors.append("no registered consumer repositories supplied") remediation_states = build_remediation_states( repos=repos, drift=drift, @@ -543,10 +570,13 @@ def build_report( open_sync_prs=open_sync_prs, sync_pr_lookup_errors=sync_pr_lookup_errors, expected_branch=expected_branch, + global_errors=global_errors, now=now, ) state_values = {str(item["state"]) for item in remediation_states.values()} - if state_values <= {"converged"}: + if global_errors: + status = "drift" + elif state_values <= {"converged"}: status = "converged" elif state_values <= {"converged", "covered"}: status = "covered" @@ -602,6 +632,7 @@ def build_report( "expected_branch": expected_branch, "coverage_lease_hours": int(SYNC_COVERAGE_MAX_AGE.total_seconds() // 3600), "repo_states": remediation_states, + "global_errors": global_errors, "open_pr_count": len(open_sync_prs), "repo_count": open_sync_repo_count, "latest_open_pr": latest_open_sync_pr, diff --git a/tests/scripts/test_check_consumer_sync_drift.py b/tests/scripts/test_check_consumer_sync_drift.py index 9c892936d..13e892fc0 100644 --- a/tests/scripts/test_check_consumer_sync_drift.py +++ b/tests/scripts/test_check_consumer_sync_drift.py @@ -190,6 +190,7 @@ def test_build_report_marks_current_sync_pr_as_covered() -> None: "number": 12, "url": "https://github.com/owner/repo/pull/12", "branch": "sync/workflows-aaaaaaaaaaaa", + "head_repo": "owner/repo", "updated_at": "2026-04-26T01:00:00Z", } ], @@ -202,6 +203,70 @@ def test_build_report_marks_current_sync_pr_as_covered() -> None: assert report["sync_remediation"]["repo_states"]["owner/repo"]["state"] == "covered" +def test_build_report_blocks_unattributed_errors_and_empty_repo_sets() -> None: + report = check_consumer_sync_drift.build_report( + repos=["owner/repo"], + drift={"owner/repo: .github/workflows/a.yml"}, + missing=set(), + errors={"sync-manifest.yml not found"}, + obsolete=set(), + ) + + assert report["status"] == "drift" + assert report["sync_remediation"]["repo_states"]["owner/repo"]["state"] == "blocked" + assert report["sync_remediation"]["global_errors"] == ["sync-manifest.yml not found"] + + empty = check_consumer_sync_drift.build_report( + repos=[], drift=set(), missing=set(), errors=set(), obsolete=set() + ) + assert empty["status"] == "drift" + assert empty["sync_remediation"]["global_errors"] == [ + "no registered consumer repositories supplied" + ] + + +def test_build_report_rejects_stale_or_untrusted_coverage() -> None: + base_pr = { + "repo": "owner/repo", + "number": 12, + "branch": "sync/workflows-aaaaaaaaaaaa", + "head_repo": "owner/repo", + } + report = check_consumer_sync_drift.build_report( + repos=["owner/repo"], + drift={"owner/repo: .github/workflows/a.yml"}, + missing=set(), + errors=set(), + obsolete=set(), + open_sync_prs=[{**base_pr, "updated_at": "2026-04-24T01:00:00Z"}], + current_plan_id="sha256:" + "a" * 64, + now=datetime(2026, 4, 26, 2, 0, tzinfo=UTC), + ) + assert report["sync_remediation"]["repo_states"]["owner/repo"]["state"] == "stale" + + untrusted = check_consumer_sync_drift.build_report( + repos=["owner/repo"], + drift={"owner/repo: .github/workflows/a.yml"}, + missing=set(), + errors=set(), + obsolete=set(), + open_sync_prs=[ + { + **base_pr, + "head_repo": "fork/repo", + "updated_at": "2026-04-26T01:00:00Z", + } + ], + current_plan_id="sha256:" + "a" * 64, + now=datetime(2026, 4, 26, 2, 0, tzinfo=UTC), + ) + assert untrusted["sync_remediation"]["repo_states"]["owner/repo"]["state"] == "untracked_drift" + + +def test_parse_github_timestamp_rejects_naive_values() -> None: + assert check_consumer_sync_drift.parse_github_timestamp("2026-04-26T01:00:00") is None + + def test_fetch_open_sync_prs_filters_to_workflows_sync_branches() -> None: class Response: status_code = 200 @@ -212,13 +277,13 @@ def json(self) -> list[dict[str, object]]: "number": 5, "title": "ordinary", "html_url": "https://github.com/owner/repo/pull/5", - "head": {"ref": "feature/example", "sha": "bad"}, + "head": {"ref": "feature/example", "sha": "bad", "repo": {"full_name": "owner/repo"}}, }, { "number": 6, "title": "sync", "html_url": "https://github.com/owner/repo/pull/6", - "head": {"ref": "sync/workflows-abc123", "sha": "good"}, + "head": {"ref": "sync/workflows-abc123", "sha": "good", "repo": {"full_name": "owner/repo"}}, "created_at": "2026-04-26T01:00:00Z", "updated_at": "2026-04-26T02:00:00Z", }, @@ -226,7 +291,7 @@ def json(self) -> list[dict[str, object]]: "number": 7, "title": "newer sync", "html_url": "https://github.com/owner/repo/pull/7", - "head": {"ref": "sync/workflows-def456", "sha": "newer"}, + "head": {"ref": "sync/workflows-def456", "sha": "newer", "repo": {"full_name": "owner/repo"}}, "created_at": "2026-04-26T03:00:00Z", "updated_at": "2026-04-26T04:00:00Z", }, @@ -255,6 +320,7 @@ def get(self, url: str) -> Response: "url": "https://github.com/owner/repo/pull/7", "branch": "sync/workflows-def456", "head_sha": "newer", + "head_repo": "owner/repo", "created_at": "2026-04-26T03:00:00Z", "updated_at": "2026-04-26T04:00:00Z", }, @@ -265,6 +331,7 @@ def get(self, url: str) -> Response: "url": "https://github.com/owner/repo/pull/6", "branch": "sync/workflows-abc123", "head_sha": "good", + "head_repo": "owner/repo", "created_at": "2026-04-26T01:00:00Z", "updated_at": "2026-04-26T02:00:00Z", }, From 5a9ddc74a9fdcd868f72593b4bc003e165537ff6 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sat, 1 Aug 2026 16:31:02 +0000 Subject: [PATCH 3/4] chore(autofix): formatting/lint --- .../scripts/test_check_consumer_sync_drift.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/tests/scripts/test_check_consumer_sync_drift.py b/tests/scripts/test_check_consumer_sync_drift.py index 13e892fc0..bf9748222 100644 --- a/tests/scripts/test_check_consumer_sync_drift.py +++ b/tests/scripts/test_check_consumer_sync_drift.py @@ -277,13 +277,21 @@ def json(self) -> list[dict[str, object]]: "number": 5, "title": "ordinary", "html_url": "https://github.com/owner/repo/pull/5", - "head": {"ref": "feature/example", "sha": "bad", "repo": {"full_name": "owner/repo"}}, + "head": { + "ref": "feature/example", + "sha": "bad", + "repo": {"full_name": "owner/repo"}, + }, }, { "number": 6, "title": "sync", "html_url": "https://github.com/owner/repo/pull/6", - "head": {"ref": "sync/workflows-abc123", "sha": "good", "repo": {"full_name": "owner/repo"}}, + "head": { + "ref": "sync/workflows-abc123", + "sha": "good", + "repo": {"full_name": "owner/repo"}, + }, "created_at": "2026-04-26T01:00:00Z", "updated_at": "2026-04-26T02:00:00Z", }, @@ -291,7 +299,11 @@ def json(self) -> list[dict[str, object]]: "number": 7, "title": "newer sync", "html_url": "https://github.com/owner/repo/pull/7", - "head": {"ref": "sync/workflows-def456", "sha": "newer", "repo": {"full_name": "owner/repo"}}, + "head": { + "ref": "sync/workflows-def456", + "sha": "newer", + "repo": {"full_name": "owner/repo"}, + }, "created_at": "2026-04-26T03:00:00Z", "updated_at": "2026-04-26T04:00:00Z", }, From 5efffeb675c5faa8323f750467cd13276679af2e Mon Sep 17 00:00:00 2001 From: Codex Automation Date: Sat, 1 Aug 2026 11:41:53 -0500 Subject: [PATCH 4/4] test(sync): pin global-error precedence over converged state A global comparison error must outrank the "no attributed gaps" branch in build_remediation_states; without a case that has no local drift, an unattributable failure could silently read as converged. Co-authored-by: Cursor --- tests/scripts/test_check_consumer_sync_drift.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/scripts/test_check_consumer_sync_drift.py b/tests/scripts/test_check_consumer_sync_drift.py index bf9748222..2ce2b8654 100644 --- a/tests/scripts/test_check_consumer_sync_drift.py +++ b/tests/scripts/test_check_consumer_sync_drift.py @@ -216,6 +216,17 @@ def test_build_report_blocks_unattributed_errors_and_empty_repo_sets() -> None: assert report["sync_remediation"]["repo_states"]["owner/repo"]["state"] == "blocked" assert report["sync_remediation"]["global_errors"] == ["sync-manifest.yml not found"] + # A global error must outrank the "no attributed gaps -> converged" branch, + # otherwise an unattributable comparison failure reads as a clean repo. + no_local_drift = check_consumer_sync_drift.build_report( + repos=["owner/repo"], + drift=set(), + missing=set(), + errors={"sync-manifest.yml not found"}, + obsolete=set(), + ) + assert no_local_drift["sync_remediation"]["repo_states"]["owner/repo"]["state"] == "blocked" + empty = check_consumer_sync_drift.build_report( repos=[], drift=set(), missing=set(), errors=set(), obsolete=set() )