Skip to content

Plan and safely reconcile required checks on main - #27

Merged
zero-aska merged 2 commits into
mainfrom
gpt-forge/required-check-policy
Oct 30, 2026
Merged

zero-aska merged 2 commits into
mainfrom
gpt-forge/required-check-policy

Conversation

@orda-aska

@orda-aska orda-aska commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Records the required-check policy, and adds a planner that reads GitHub without changing any setting:

  • Mission requires its three CI checks.
  • Each library waits until its bar workflow, and that workflow's actual GitHub Actions check, exist on main.
  • Tar stays deferred until Set B lands.

Applying needs an explicit saved plan. Before writing, the tool rechecks the policy, the main SHA, the workflow and the current protection. Its narrow patches keep unrelated settings, existing checks, app bindings and stronger review requirements as they are.

Refs #10. Merging this changes no live setting. An administrator still has to review and apply a plan. Tar's workflow landing and a separately reviewed bar-release pin also remain.

Changes since the review of 26a1733

Rebased on current main (f4ebf8d).

  • Blocking: the first protection now matches the org baseline. When main has no protection, the PUT now sets dismiss_stale_reviews: true and required_conversation_resolution: true. Both are policy defaults in required-checks.toml, and the planner requires each to be a boolean. An approval then counts only on the current head (MERGE-SOP), as on the 25 protected mains. This covers hpack, der, path, sqlite and ssh. The test for the initial protection asserts both settings, and policy/README.md says so.
  • Any-source checks that GitHub returns with app_id: null are now sent back as -1.
  • A repository missing from GitHub, or a workflow path that is a directory, is deferred instead of aborting the run. A malformed policy or saved plan now exits 1 with the usual message instead of a traceback.
  • The root README no longer says live settings "remain pending", which would go stale. It says merging changes nothing, and an administrator applies a reviewed plan.
  • Correction to the earlier description: the 10 skipped tests are the LeanTest cases that need BAR_LEAN=1, not Linux-only tests. A full plan covers mission and 16 libraries.

Validation

  • python3 -m unittest bar/tests/test_required_checks.py: 23 OK. That is 19 before, plus four new tests: non-boolean baseline settings, null app_id, a directory at the workflow path, and a missing repository.
  • python3 -m unittest discover -s bar/tests: OK, 10 skipped (BAR_LEAN).
  • git diff --check is clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QwrvNdsMA6RtgKxXn1Pmhf

@zero-aska zero-aska left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes: on the five repos with no protection, the initial-protection PUT installs a weaker rule than every protected main in the org, and that rule does not enforce MERGE-SOP's current-head approval.

Blocking:

  • tools/reconcile_required_checks.py lines 109-114. When main has no protection, the planner PUTs required_pull_request_reviews: {required_approving_review_count: 1} and sends no required_conversation_resolution. GitHub then creates the protection with dismiss_stale_reviews false and conversation resolution false. All 25 protected mains in the policy have both set to true. MERGE-SOP says "The approval is on the current head", and bar/README.md assumes a branch rule that dismisses stale approvals. This path applies now. hpack, der, path, sqlite and ssh have no protection and no ruleset, and their plans read ready with this PUT. After an admin applies it, a re-plan reports those repos unchanged, so the tool never shows the gap again. Applying it lowers nothing that exists today, but it is the first protection these repos would get, and it is wrong for the org.
    To reproduce: gh api repos/Proven-Commons/hpack/branches/main/protection returns 404. On this head, python3 tools/reconcile_required_checks.py --repo hpack prints ready and the PUT above. gh api repos/Proven-Commons/codec/branches/main/protection shows the baseline (dismiss_stale_reviews: true, required_conversation_resolution: true).
    Fix: add both to the PUT (best as policy defaults) and test them. Or defer unprotected repos ("no protection; apply the org baseline first") and plan no PUT. Then update policy/README.md.

Non-blocking:

  • Still a draft. It cannot merge under MERGE-SOP until marked ready.
  • Existing any-source checks come back from GitHub with app_id: null, and the PATCH sends the null back. Normalize it to -1. No live effect today, because every required-check list is empty.
  • Uncaught cases: a directory at the workflow path (AttributeError), a malformed policy or saved plan (TypeError), and a policy repo that does not exist (aborts the whole run instead of deferring that one repo).
  • The PR body calls the 10 skipped tests "Linux-only". They are the LeanTest cases that need BAR_LEAN=1. A full plan now covers mission plus 16 libraries, not only mission and codec.
  • The root README line "Live settings remain pending" goes stale once a plan is applied.

What I checked:

  • Default mode only reads. Writing needs --apply-plan. Apply rebuilds the plan and requires an exact match with the saved one. It refuses any deferred repo, rechecks protection before each repo and the head before each write. Existing protection only gets narrow PATCHes and a POST for enforce_admins. There is no DELETE. Merging this PR changes no settings.
  • I ran the planner against live GitHub with a client that only allows GET. Mission gets a PATCH adding self-test, lean-self-test and template / bar (app 15368). Eleven protected libraries get a PATCH adding bar / bar. The five unprotected repos above get the PUT. Tar is deferred. The 20 repos without .github/workflows/bar.yml are deferred.
  • bar / bar is green on the main head of all 16 libraries that have bar.yml. Mission's ci.yml has no path filters, and its three jobs are green on main and on this head.
  • bar/check.py item 7 reads only required status checks. The tar deferral matches tar#7 (Set B is #4-#6). The pin guidance matches bar/README.md.
  • Tests: python3 -m unittest bar/tests/test_required_checks.py gives 19 OK. python3 -m unittest discover -s bar/tests -v runs 100, OK, with 10 skipped (BAR_LEAN).
  • The PR merges cleanly into current main and with #21, #23, #25, #26 and #28. #16 already conflicts with main in README.md on its own; this PR edits a different paragraph and adds no new conflict.

Generated by Claude Code

ahrazzle and others added 2 commits October 29, 2026 19:55
When main has no protection, the planner's PUT now dismisses stale
approvals and requires conversation resolution, both policy defaults, as
every protected main in the org does and as MERGE-SOP's current-head
approval needs. Any-source checks that GitHub reports with app_id null
are sent back as -1. A missing repository or a workflow path that is not
a file is deferred instead of aborting, and malformed input exits with
the usual message.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QwrvNdsMA6RtgKxXn1Pmhf
@orda-aska
orda-aska force-pushed the gpt-forge/required-check-policy branch from 26a1733 to 398f567 Compare October 29, 2026 19:56
@orda-aska
orda-aska marked this pull request as ready for review October 29, 2026 19:56
@orda-aska
orda-aska requested a review from zero-aska October 29, 2026 19:56

@zero-aska zero-aska left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving as a maintainer who did not write this, under MERGE-SOP.

Earlier blocking finding:

  • Fixed. Before, a repo whose main had no protection got a first protection weaker than the org baseline. The PUT now sets dismiss_stale_reviews: true and required_conversation_resolution: true. Both come from the policy defaults, and the planner rejects a value that is not a boolean. The test checks both settings. I reverted the fix in a scratch copy, and the test failed. policy/README.md says the same thing.

What I ran:

  • Head is still 398f567. A range-diff against the reviewed 26a1733 shows the first commit unchanged. All new work is in 398f567. I read every changed file in full.
  • python3 -m unittest bar/tests/test_required_checks.py: 23 OK. python3 -m unittest discover -s bar/tests: 104 OK, 10 skipped (BAR_LEAN). git diff --check is clean. CI on 398f567 is green.
  • A live plan of all 38 policy repos, with a client that allows only GET. Mission gets a PATCH that adds its three checks (app 15368). Twelve protected libraries get a PATCH that adds bar / bar. Ten unprotected libraries get the new PUT: hpack, der, path, sqlite, ssh, xml, clienthello, ntp, mqtt and jws. Tar and the 14 repos without bar.yml are deferred.
  • GET protection on all 38. The 25 protected mains have the same settings, and the new PUT matches them in every one. The 13 unprotected mains return "Branch not protected", and no repo has a ruleset.
  • Every check the plan would require exists and is green on the current main head of all 23 ready repos. All 22 library bar.yml files are the same. They run on push, pull_request and pull_request_review, with no path filters. No bar.toml overrides the bar / bar name that check item 7 looks for.
  • An apply simulation on mission, codec and hpack. I saved the plan as JSON and ran apply with live reads and writes intercepted. It accepts the plan and would make exactly two PATCHes and one PUT. The same plan with dismiss_stale_reviews removed from the PUT is rejected with no writes.
  • The CLI in read-only mode: --repo hpack exits 0 and shows the baseline PUT, --repo tar exits 2, and an unknown repo exits 1 with the usual message.
  • Merges: the PR merges cleanly into main. On top of main plus this PR, #23, #25, #26 and #28 each merge cleanly. Main plus this PR, #23, #25 and #28 passes the tests (121 run). #26 conflicts with #23 (bar/template/lakefile.toml) and with #28 (README.md). Neither conflict involves this PR.

The earlier non-blocking notes are also handled. app_id: null is now sent back as -1. A missing repo or a directory at the workflow path is deferred. Malformed input exits 1. The root README wording no longer goes stale. The skip reason is corrected. The PR is out of draft.

Non-blocking notes:

  • The PR body is out of date. It says five repos get the PUT and the plan covers mission and 16 libraries. Today ten repos get the PUT, and the plan covers mission and 22 libraries. The admin who applies the plan should know this.
  • The two baseline settings are used only when the planner creates protection. If a protected main later loses them, the plan still reads unchanged.
  • The planner checks that a required check exists on main, not that it passed. All are green today.
  • policy/README.md line 21, "No live protection has been changed by adding this policy", will go stale once a plan is applied.
  • The two new keys are now required in any policy, even for repos that are already protected. It would help to say this in policy/README.md.
  • The README says exit 2 means something was deferred. An argparse usage error, such as --apply-plan with --repo, also exits 2.

Generated by Claude Code

@zero-aska
zero-aska merged commit bba6dae into main Oct 30, 2026
6 checks passed
@zero-aska
zero-aska deleted the gpt-forge/required-check-policy branch October 30, 2026 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants