diff --git a/.github/workflows/github-hourly-review-repair.yml b/.github/workflows/github-hourly-review-repair.yml new file mode 100644 index 000000000..97665aa5f --- /dev/null +++ b/.github/workflows/github-hourly-review-repair.yml @@ -0,0 +1,27 @@ +name: Central GitHub Hourly Review Repair + +on: + schedule: + # Keep the control-plane queue moving without colliding with minute-zero jobs. + - cron: "21 * * * *" + +concurrency: + group: github-hourly-review-repair + cancel-in-progress: false + +permissions: + contents: read + +jobs: + dispatch-review-repair: + uses: ./.github/workflows/pr-review-fix-scheduler.yml + with: + target_repository: ContextualWisdomLab/.github + base_branch: main + max_prs: "50" + max_dispatches: "1" + resolve_unreviewed_conflicts: true + retry_hours: "1" + secrets: + PR_REVIEW_MERGE_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN }} + OPENCODE_APPROVE_TOKEN: ${{ secrets.OPENCODE_APPROVE_TOKEN }} diff --git a/.github/workflows/hourly-nvidia-nim-review-repair.yml b/.github/workflows/hourly-nvidia-nim-review-repair.yml index 0cb5e33dc..de4a03314 100644 --- a/.github/workflows/hourly-nvidia-nim-review-repair.yml +++ b/.github/workflows/hourly-nvidia-nim-review-repair.yml @@ -10,6 +10,7 @@ on: - .github/workflows/clearfolio-hourly-review-repair.yml - .github/workflows/disksage-hourly-review-repair.yml - .github/workflows/fast-mlsirm-hourly-review-repair.yml + - .github/workflows/github-hourly-review-repair.yml - .github/workflows/governance-risk-compliance-hourly-review-repair.yml - .github/workflows/hourly-nvidia-nim-review-repair.yml - .github/workflows/originweave-hourly-review-repair.yml @@ -18,6 +19,7 @@ on: - tests/test_bandscope_hourly_review_caller.py - tests/test_disksage_hourly_review_caller.py - tests/test_fast_mlsirm_hourly_review_caller.py + - tests/test_github_hourly_conflict_repair.py - tests/test_governance_risk_compliance_hourly_review_caller.py - tests/test_hourly_scheduler_runtime_budget.py - tests/test_originweave_hourly_review_caller.py @@ -39,6 +41,7 @@ on: - docs/doctoring/conflict-control-evidence-isolation.md - docs/doctoring/disksage-hourly-review-caller.md - docs/doctoring/fast-mlsirm-hourly-review-caller.md + - docs/doctoring/github-hourly-conflict-repair.md - docs/doctoring/governance-risk-compliance-hourly-review-caller.md - docs/doctoring/hourly-nvidia-nim-autofix.md - docs/doctoring/originweave-hourly-review-caller.md @@ -51,6 +54,7 @@ on: - .github/workflows/clearfolio-hourly-review-repair.yml - .github/workflows/disksage-hourly-review-repair.yml - .github/workflows/fast-mlsirm-hourly-review-repair.yml + - .github/workflows/github-hourly-review-repair.yml - .github/workflows/governance-risk-compliance-hourly-review-repair.yml - .github/workflows/hourly-nvidia-nim-review-repair.yml - .github/workflows/originweave-hourly-review-repair.yml @@ -59,6 +63,7 @@ on: - tests/test_bandscope_hourly_review_caller.py - tests/test_disksage_hourly_review_caller.py - tests/test_fast_mlsirm_hourly_review_caller.py + - tests/test_github_hourly_conflict_repair.py - tests/test_governance_risk_compliance_hourly_review_caller.py - tests/test_hourly_scheduler_runtime_budget.py - tests/test_originweave_hourly_review_caller.py @@ -80,6 +85,7 @@ on: - docs/doctoring/conflict-control-evidence-isolation.md - docs/doctoring/disksage-hourly-review-caller.md - docs/doctoring/fast-mlsirm-hourly-review-caller.md + - docs/doctoring/github-hourly-conflict-repair.md - docs/doctoring/governance-risk-compliance-hourly-review-caller.md - docs/doctoring/hourly-nvidia-nim-autofix.md - docs/doctoring/originweave-hourly-review-caller.md @@ -133,6 +139,7 @@ jobs: tests/test_bandscope_hourly_review_caller.py \ tests/test_disksage_hourly_review_caller.py \ tests/test_fast_mlsirm_hourly_review_caller.py \ + tests/test_github_hourly_conflict_repair.py \ tests/test_governance_risk_compliance_hourly_review_caller.py \ tests/test_hourly_scheduler_runtime_budget.py \ tests/test_originweave_hourly_review_caller.py \ diff --git a/.github/workflows/pr-review-fix-scheduler.yml b/.github/workflows/pr-review-fix-scheduler.yml index 7eb0251d5..a3fdaa1aa 100644 --- a/.github/workflows/pr-review-fix-scheduler.yml +++ b/.github/workflows/pr-review-fix-scheduler.yml @@ -23,6 +23,11 @@ on: required: false default: "" type: string + resolve_unreviewed_conflicts: + description: Dispatch bounded conflict repair before the original head is reviewed + required: false + default: true + type: boolean retry_hours: description: Minimum hours before redispatching autofix for the same head required: false @@ -83,6 +88,7 @@ jobs: DRY_RUN: ${{ github.event.client_payload.dry_run == true || github.event.client_payload.dry_run == 'true' || inputs.dry_run == true }} MAX_PRS: ${{ github.event.client_payload.max_prs || inputs.max_prs || '50' }} MAX_DISPATCHES: ${{ github.event.client_payload.max_dispatches || inputs.max_dispatches || '1' }} + RESOLVE_UNREVIEWED_CONFLICTS: ${{ github.event.client_payload.resolve_unreviewed_conflicts == true || github.event.client_payload.resolve_unreviewed_conflicts == 'true' || inputs.resolve_unreviewed_conflicts == true }} RETRY_HOURS: ${{ github.event.client_payload.retry_hours || inputs.retry_hours || '1' }} AUTOFIX_WORKFLOW: pr-review-autofix.yml AUTOFIX_REPOSITORY: ContextualWisdomLab/.github @@ -104,20 +110,26 @@ jobs: "${TARGET_REPOSITORY:-}" exit 1 fi - if [ -z "$ALLOWED_TARGET_REPOSITORIES" ]; then - echo "::error::Scheduler target repository allowlist is not configured." - exit 1 - fi target_allowed=false - IFS=',' read -r -a allowed_targets <<<"$ALLOWED_TARGET_REPOSITORIES" - for candidate in "${allowed_targets[@]}"; do - candidate="${candidate//[[:space:]]/}" - if [ -n "$candidate" ] && [ "$candidate" = "$TARGET_REPOSITORY" ]; then - target_allowed=true - break + if [ -n "${GITHUB_REPOSITORY:-}" ] && + [ "$TARGET_REPOSITORY" = "$GITHUB_REPOSITORY" ]; then + echo "Self-targeted scheduler invocation uses the protected caller repository." + target_allowed=true + else + if [ -z "$ALLOWED_TARGET_REPOSITORIES" ]; then + echo "::error::Scheduler target repository allowlist is not configured." + exit 1 fi - done + IFS=',' read -r -a allowed_targets <<<"$ALLOWED_TARGET_REPOSITORIES" + for candidate in "${allowed_targets[@]}"; do + candidate="${candidate//[[:space:]]/}" + if [ -n "$candidate" ] && [ "$candidate" = "$TARGET_REPOSITORY" ]; then + target_allowed=true + break + fi + done + fi if [ "$target_allowed" != "true" ]; then printf '::error::Scheduler target repository is not allowlisted: %s.\n' \ "$TARGET_REPOSITORY" @@ -127,7 +139,7 @@ jobs: # A reusable workflow receives its caller's original event payload, # so the hourly callers arrive as `schedule`, not `workflow_call`. # Only the direct repository_dispatch surface needs sender binding; - # every invocation still passes the target allowlist above. + # cross-repository invocations still pass the configured allowlist. if [ "$EVENT_NAME" = "repository_dispatch" ]; then if [ -z "$ALLOWED_DISPATCH_ACTOR" ] || [ "$DISPATCH_ACTOR" != "$ALLOWED_DISPATCH_ACTOR" ] || @@ -308,6 +320,9 @@ jobs: --autofix-workflow "$AUTOFIX_WORKFLOW" --autofix-repository "$AUTOFIX_REPOSITORY" ) + if [ "$RESOLVE_UNREVIEWED_CONFLICTS" = "true" ]; then + args+=(--resolve-unreviewed-conflicts) + fi if [ "$DRY_RUN" = "true" ]; then args+=(--dry-run) fi diff --git a/docs/doctoring/github-hourly-conflict-repair.md b/docs/doctoring/github-hourly-conflict-repair.md new file mode 100644 index 000000000..2a3fc2a68 --- /dev/null +++ b/docs/doctoring/github-hourly-conflict-repair.md @@ -0,0 +1,119 @@ +# Central `.github` hourly OpenCode conflict repair + +## Decision + +The central repository scans its own open `main` pull requests once per hour and +dispatches the existing trusted OpenCode conflict worker for a same-repository +head reported by GitHub as `DIRTY` or `CONFLICTING`. + +A review is **not** a prerequisite for this bounded repair. Resolving the +conflict creates a new merge commit and therefore a new pull-request head; any +review of the old head cannot establish approval of the resulting combined +source. The repaired head must complete fresh review and required checks before +it can merge. + +Direct Python-library callers retain the historical approval prerequisite. The +trusted reusable workflow opts into unreviewed conflict repair explicitly with +`--resolve-unreviewed-conflicts`, making the privilege visible and testable. + +## Execution path + +```text +hourly protected-default-branch caller +→ exact open PR inventory +→ same-repository, non-draft, configured-base filter +→ GitHub DIRTY / CONFLICTING signal +→ head-scoped retry marker +→ repository_dispatch(pr-review-autofix, repair_mode=conflict) +→ exact live base/head revalidation +→ git merge --no-commit --no-ff +→ sealed NUL-delimited conflicted-path allowlist +→ whole-worktree snapshot outside the repository +→ OpenCode edits conflicted paths only +→ scope verification, conflict-marker rejection, syntax checks +→ live-head race check +→ merge commit push +→ fresh required reviews and checks +``` + +## Preserved security and governance boundaries + +- Draft pull requests remain ineligible. +- Fork and external-head pull requests remain read-only. +- The configured base branch must match. +- The worker refetches and validates the exact live base and head before writing. +- OpenCode receives no GitHub token, OIDC request token, shell permission, + external-directory permission, web access, task delegation, or arbitrary + JavaScript execution permission. +- The model may modify only paths Git reported as unmerged. +- Tracked, untracked, ignored, deleted, retargeted, and symbolic-link state is + included in the scope evidence. +- Unresolved conflict markers fail closed. +- A concurrent head movement prevents the push. +- Conflict repair never approves, merges, or releases the pull request; it only + produces a reviewable combined head. +- One repair is dispatched per scheduler pass, with a one-hour exact-head retry + interval and non-cancelling worker concurrency. +- `COPILOT_GITHUB_TOKEN` is not used. + +## Why approval-before-repair was removed from the scheduled path + +The previous selector required a current-head approval before conflict repair. +That created a circular dependency for PRs such as `.github#1098`: reviewers +could not assess a valid merge preview while the conflict prevented the safe +combined head from existing, and the conflict worker could not run until a +review approved the pre-resolution head. + +The correct evidence order is: + +```text +conflict detected +→ bounded mechanical/semantic repair +→ new exact head +→ review and checks on that exact head +→ guarded merge decision +``` + +This changes eligibility only. It does not weaken the worker's write boundary or +the repository's review, required-check, branch-protection, and merge gates. + +## Regression evidence + +`tests/test_github_hourly_conflict_repair.py` fixes the following contracts: + +1. An unreviewed `DIRTY` PR becomes eligible only when the trusted policy flag is + explicit. +2. Direct library use remains backward-compatible by default. +3. The CLI exposes the policy flag. +4. The reusable workflow enables the policy for hourly callers by default. +5. `.github` has its own hourly caller at minute 21. +6. A same-repository protected caller does not require a cross-repository target + allowlist entry, while cross-repository targets still do. +7. The focused NVIDIA NIM review-repair gate tracks the caller, regression test, + and this doctoring record. + +The pre-existing conflict-scope, control-file isolation, trusted Git executable, +ignored-path, symlink-target, exact-head, writer-security, and NVIDIA NIM +contract suites remain authoritative for the worker boundary. + +## Operator next action + +After this change reaches `main`, inspect the next `Central GitHub Hourly Review +Repair` run. A qualifying conflict should receive the head-scoped scheduler +marker, followed by a `PR Review Autofix` conflict-mode run. Confirm that the +new head has a merge commit whose parents are the previous PR head and the live +protected base, then require normal current-head reviews and checks before +merging. + +## References — APA 7th + +GitHub. (n.d.). *About protected branches*. GitHub Docs. +https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches + +GitHub. (n.d.). *Resolving a merge conflict using the command line*. GitHub Docs. +https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/addressing-merge-conflicts/resolving-a-merge-conflict-using-the-command-line + +National Institute of Standards and Technology. (2022). *Secure software +development framework (SSDF) version 1.1: Recommendations for mitigating the +risk of software vulnerabilities* (NIST Special Publication 800-218). +https://doi.org/10.6028/NIST.SP.800-218 diff --git a/scripts/ci/pr_review_fix_scheduler.py b/scripts/ci/pr_review_fix_scheduler.py index 0a4263e19..2c9745d09 100755 --- a/scripts/ci/pr_review_fix_scheduler.py +++ b/scripts/ci/pr_review_fix_scheduler.py @@ -174,20 +174,28 @@ def needs_rca_repair(pr: dict[str, Any]) -> tuple[bool, tuple[str, ...]]: CONFLICT_MERGE_STATES = frozenset({"DIRTY", "CONFLICTING"}) -def needs_conflict_resolution(pr: dict[str, Any]) -> tuple[bool, tuple[str, ...]]: - """Return whether an approved PR has a conflict safe to auto-resolve. - - Only a current-head-approved PR that GitHub reports as ``DIRTY`` or - ``CONFLICTING`` qualifies. The worker merges the base into the head and the - resulting head must be reviewed and checked again before merge. +def needs_conflict_resolution( + pr: dict[str, Any], + *, + allow_unreviewed: bool = False, +) -> tuple[bool, tuple[str, ...]]: + """Return whether a GitHub-reported conflict is safe to auto-resolve. + + Direct library callers retain the historical current-head approval + prerequisite unless ``allow_unreviewed`` is explicit. Trusted scheduled + callers enable it because conflict repair creates a new head and therefore + requires fresh reviews and checks regardless of the previous review state. """ merge_state = str(pr.get("mergeStateStatus") or "").upper() if merge_state not in CONFLICT_MERGE_STATES: return False, () - if not has_current_head_approval(pr): + approved = has_current_head_approval(pr) + if not approved and not allow_unreviewed: return False, () + review_state = "current-head approved" if approved else "unreviewed" return True, ( - f"current-head approved PR is {merge_state.lower()}; auto-resolving the merge conflict", + f"{review_state} PR is {merge_state.lower()}; auto-resolving the merge " + "conflict and requiring fresh review and checks on the resulting head", ) @@ -234,7 +242,7 @@ def dispatch_autofix( ``repair_mode=rca`` tells the trusted context collector to gather failed check evidence and widen the sealed edit scope only to current PR files. - ``resolve_conflict`` retains the separate approved-conflict path. + ``resolve_conflict`` retains the separately bounded conflict path. """ dispatch_repo = workflow_repository or repo if workflow != DEFAULT_AUTOFIX_WORKFLOW: @@ -303,7 +311,12 @@ def inspect_pr( repair_mode = "rca" reasons = rca_reasons else: - needs_resolve, resolve_reasons = needs_conflict_resolution(pr) + needs_resolve, resolve_reasons = needs_conflict_resolution( + pr, + allow_unreviewed=bool( + getattr(args, "resolve_unreviewed_conflicts", False) + ), + ) if not needs_resolve: return "skip", ( "no current-head autofixable review, failed-check RCA, or approved merge conflict", @@ -356,7 +369,12 @@ def process_queue(args: argparse.Namespace) -> int: continue needs_fix, _ = needs_autofix(pr) needs_rca, _ = needs_rca_repair(pr) - needs_resolve, _ = needs_conflict_resolution(pr) + needs_resolve, _ = needs_conflict_resolution( + pr, + allow_unreviewed=bool( + getattr(args, "resolve_unreviewed_conflicts", False) + ), + ) if needs_fix or needs_rca or needs_resolve: prs_needing_comments.append(pr) @@ -503,6 +521,12 @@ def self_test() -> int: {**approved_dirty_pr, "mergeStateStatus": "CLEAN"} ) == (False, ()) assert needs_conflict_resolution(dirty_pr) == (False, ()) + resolves, resolve_reasons = needs_conflict_resolution( + dirty_pr, + allow_unreviewed=True, + ) + assert resolves + assert "fresh review and checks" in resolve_reasons[0] model_exhausted_pr = { **pr, "reviews": { @@ -552,6 +576,7 @@ def parse_args(argv: list[str]) -> argparse.Namespace: parser.add_argument("--max-prs", type=int, default=50) parser.add_argument("--max-dispatches", type=int, default=1) parser.add_argument("--retry-hours", type=int, default=24) + parser.add_argument("--resolve-unreviewed-conflicts", action="store_true") parser.add_argument("--autofix-workflow", default="pr-review-autofix.yml") parser.add_argument( "--autofix-repository", diff --git a/tests/test_github_hourly_conflict_repair.py b/tests/test_github_hourly_conflict_repair.py new file mode 100644 index 000000000..e905bbce8 --- /dev/null +++ b/tests/test_github_hourly_conflict_repair.py @@ -0,0 +1,133 @@ +"""Regression contracts for unattended OpenCode merge-conflict repair.""" + +from pathlib import Path +from typing import Any + +import pytest + +from scripts.ci import pr_review_fix_scheduler as scheduler + + +_CALLER = Path(".github/workflows/github-hourly-review-repair.yml") +_REUSABLE_SCHEDULER = Path(".github/workflows/pr-review-fix-scheduler.yml") + + +def _unreviewed_conflict() -> dict[str, object]: + """Return a same-repository PR whose current head has no review yet.""" + return { + "number": 1098, + "isDraft": False, + "baseRefName": "main", + "baseRefOid": "b" * 40, + "headRefName": "feature/conflict", + "headRefOid": "a" * 40, + "headRepository": {"nameWithOwner": "ContextualWisdomLab/.github"}, + "mergeStateStatus": "DIRTY", + "reviews": {"nodes": []}, + "reviewThreads": {"nodes": []}, + } + + +def test_explicit_policy_dispatches_unreviewed_conflict() -> None: + """Conflict repair must not wait for an approval invalidated by its own commit.""" + needs_repair, reasons = scheduler.needs_conflict_resolution( + _unreviewed_conflict(), + allow_unreviewed=True, + ) + + assert needs_repair + assert "fresh review and checks" in reasons[0] + + +def test_scheduler_dispatches_conflict_mode_for_unreviewed_head( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The trusted queue must reach the existing bounded conflict worker.""" + arguments = scheduler.parse_args( + [ + "--repo", + "ContextualWisdomLab/.github", + "--base-branch", + "main", + "--resolve-unreviewed-conflicts", + "--dry-run", + ] + ) + captured: dict[str, Any] = {} + + def capture_dispatch(_repo: str, _pr: dict[str, Any], **kwargs: Any) -> None: + """Capture dispatch arguments without invoking GitHub.""" + captured.update(kwargs) + + monkeypatch.setattr(scheduler, "dispatch_autofix", capture_dispatch) + monkeypatch.setattr( + scheduler, + "create_fix_marker", + lambda *_args, **_kwargs: None, + ) + + action, reasons = scheduler.inspect_pr( + "ContextualWisdomLab/.github", + _unreviewed_conflict(), + arguments, + comments=[], + ) + + assert action == "dispatch" + assert "fresh review and checks" in reasons[0] + assert captured["resolve_conflict"] is True + + +def test_default_library_policy_remains_backward_compatible() -> None: + """Direct library callers retain the prior approval requirement unless opted in.""" + assert scheduler.needs_conflict_resolution(_unreviewed_conflict()) == (False, ()) + + +def test_cli_exposes_unreviewed_conflict_policy() -> None: + """The trusted workflow can opt into unreviewed conflict repair explicitly.""" + arguments = scheduler.parse_args( + [ + "--repo", + "ContextualWisdomLab/.github", + "--base-branch", + "main", + "--resolve-unreviewed-conflicts", + ] + ) + + assert arguments.resolve_unreviewed_conflicts is True + + +def test_reusable_scheduler_enables_policy_for_hourly_callers() -> None: + """Central callers receive conflict repair by default without duplicating logic.""" + workflow = _REUSABLE_SCHEDULER.read_text(encoding="utf-8") + + assert "resolve_unreviewed_conflicts:" in workflow + policy_block = workflow.split("resolve_unreviewed_conflicts:", maxsplit=1)[1].split( + "retry_hours:", maxsplit=1 + )[0] + assert "default: true" in policy_block + assert "--resolve-unreviewed-conflicts" in workflow + + +def test_central_repository_has_hourly_self_caller() -> None: + """The central repository itself is scanned instead of relying on product callers.""" + workflow = _CALLER.read_text(encoding="utf-8") + + assert 'cron: "21 * * * *"' in workflow + assert "uses: ./.github/workflows/pr-review-fix-scheduler.yml" in workflow + assert "target_repository: ContextualWisdomLab/.github" in workflow + assert "base_branch: main" in workflow + assert "resolve_unreviewed_conflicts: true" in workflow + assert 'max_dispatches: "1"' in workflow + assert 'retry_hours: "1"' in workflow + assert "COPILOT_GITHUB_TOKEN" not in workflow + + +def test_scheduled_self_target_does_not_require_cross_repository_allowlist() -> None: + """A protected same-repository schedule is valid even without cross-repo config.""" + workflow = _REUSABLE_SCHEDULER.read_text(encoding="utf-8") + + assert 'if [ -n "${GITHUB_REPOSITORY:-}" ] &&' in workflow + assert '[ "$TARGET_REPOSITORY" = "$GITHUB_REPOSITORY" ]; then' in workflow + assert "Self-targeted scheduler invocation uses the protected caller repository." in workflow diff --git a/tests/test_repository_branch_coverage_review_schedulers.py b/tests/test_repository_branch_coverage_review_schedulers.py index 8ee58db12..d50f94f05 100644 --- a/tests/test_repository_branch_coverage_review_schedulers.py +++ b/tests/test_repository_branch_coverage_review_schedulers.py @@ -138,7 +138,9 @@ def test_fix_scheduler_queue_includes_eligible_pr_without_fix_need( monkeypatch.setattr(fix_scheduler, "same_repository_head", lambda *_args: True) monkeypatch.setattr(fix_scheduler, "needs_autofix", lambda _pr: (False, ())) monkeypatch.setattr( - fix_scheduler, "needs_conflict_resolution", lambda _pr: (False, ()) + fix_scheduler, + "needs_conflict_resolution", + lambda _pr, **_kwargs: (False, ()), ) monkeypatch.setattr( fix_scheduler, "inspect_pr", lambda *_args, **_kwargs: ("skip", ("clean",))