Skip to content

fix(release): verify all protected main policies - #1201

Merged
bokelley merged 3 commits into
mainfrom
claude/issue-1198-release-readiness
Sep 27, 2026
Merged

bokelley merged 3 commits into
mainfrom
claude/issue-1198-release-readiness

Conversation

@bokelley

@bokelley bokelley commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Release acceptance now checks both effective rulesets and branch-protection inventories, with explicit GitHub App bindings. Main CI runs read-only IPR and conventional-commit checks, and publication/recovery rejects substituted wheel or sdist bytes.

The commit policy validates integrations after published beta.15. Normal two-parent GitHub merges must contain a conventional subject and any required breaking footer in their committed body; a single-parent commit cannot impersonate that merge wrapper. Conventional merge subjects are also validated directly.

The only historical footer exceptions match both the commit and its exact committed conventional subject:

Every other breaking commit requires a footer, including a new commit reusing either title. The runbook and release notes preserve both migration requirements. There is no moving date or history cutoff; historical IPR validation is unchanged.

Validation: make lint, make typecheck-all, 282 focused release-policy/configuration tests, script mypy, Black/Ruff and normal commit hooks pass. The direct main-push policy passes all 28 current main integrations. The regression fixture uses those actual committed messages and parents so shallow CI checkouts exercise the same history offline; it failed on #1192 before the fix. Full local make test passed: 12,187 passed, 2,359 skipped, 9 deselected, 1 xfailed; coverage 81.58% against the unchanged 80% floor.

Keep this PR ready and merge after #1217, with green CI and current-head approval. Rebase only if GitHub reports a conflict. The final combined main must pass fresh CI before any release proposal; the coordinator will verify gates and arrange the human merge.

@bokelley
bokelley force-pushed the claude/issue-1198-release-readiness branch from d6e7c22 to 81610de Compare September 26, 2026 21:37
@bokelley
bokelley marked this pull request as ready for review September 26, 2026 23:04

Copy link
Copy Markdown
Contributor Author

The PR is ready and aao review is running; branch CI is green. A local run of the new main-push policy found a merge blocker that PR CI does not exercise: nine normal GitHub merge subjects are rejected, and the already-merged #1174 breaking commit has no footer.

I am correcting validation of normal two-parent merge messages without skipping their conventional message. The coordinator has been asked whether the stricter footer policy should apply prospectively or keep the historical footer as a blocker. Please hold merge until this is resolved.

aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Sep 26, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ladon verdict: Approve

Approve — release-gate hardening with no blocking findings.

Reviewed: adds native read-only IPR-signature and conventional-commit main-policy CI jobs, folds the classic branch-protection inventory into validate_checks (only ever adds required checks, tightens App-binding to a positive int), fail-closed throughout (unreadable inventory raises, bot exemption keyed on API account type rather than login, breaking footer checked in the actual commit body). No changes to the public adcp.* surface, no changeset impact, and comprehensive test coverage in tests/test_main_release_policies.py and tests/test_release_guard.py.

Decision-table walk: no critical/high/medium findings (row 1 n/a); gated_paths false (row 2 n/a); high_risk false (rows 3/5 n/a); no medium data-loss/schema/infra finding (row 4 n/a); no prior escalation (row 6 n/a); no no-auto-approve team match (row 7 n/a); fewer than three medium findings (row 8 n/a). Falls through to row 9 → approve. Note review_decision is REVIEW_REQUIRED, but with gated_paths: false that does not force a comment or escalate.

Copy link
Copy Markdown
Contributor Author

Exact-head technical verdict — 27ccea1239cdf221b4b45339269a7802002d0aa1

Holding approval. The correction fixes 9 of the 10 main-history rejections, but one still blocks: a push to main would fail Validate conventional commit format, and since that job is now in CI_FLOOR, the release gate would reject too.

Reproduction against current main (2bfbca4b9, includes #1225)

Ran the module itself — main_commits() + validate_commit() — not a reimplementation:

commits after floor: 28
REJECTED: 1
  34c8f6d92  breaking subject requires its BREAKING CHANGE footer in the actual commit

The blocking commit is a real two-parent merge:

parents: e16eb8cf3 decfb4eb5
subject: Merge pull request #1192 from adcontextprotocol/conductor/reporting-production-tier-capabilities-b24
body:    feat(reporting)!: enable production tier status and ownership
BREAKING footer present: 0

So the new unwrapping works correctly and that's exactly why it now reaches this: two parents ✓, GITHUB_MERGE matches ✓ → validates the committed body → sees ! → requires a footer → absent. LEGACY_BREAKING_COMMIT only covers 6c5ee7b29 (a single-parent squash), so 34c8f6d92 doesn't inherit it.

The rest of the correction is right

Worth saying explicitly, because the remaining failure is a gap in coverage, not a flaw in the approach:

  • len(parents.split()) == 2 gating the unwrap is the correct guard. A single-parent commit merely named Merge pull request #… gets no exemption, which keeps the push path consistent with the --no-merges reasoning already documented on the PR path.
  • Validating the committed merge body rather than the live PR title is the right choice — a PR title is mutable after merge; the body is bytes in the commit.
  • The historical exception is bound to SHA and subject, so no other commit inherits it, including one with an identical subject. That's the auditable shape this needs.
  • Empirically, the other 9 GitHub-default merge subjects that previously failed now pass.

Remedy

The consistent move is to extend the same narrow, enumerated exception to 34c8f6d92 — bound to its SHA and its body subject the way 6c5ee7b29 is bound to its subject, with the same "no other commit inherits this" comment. That keeps the prospective boundary strict (new breaking commits, merge or squash, still need footers) while acknowledging one more pre-guard commit.

Moving PUBLISHED_BASE would also clear it, but that contradicts the stated immutability and would skip re-checking the integrations in between.

Other checks at this head (all clean)

  • tests/test_main_release_policies.py + tests/test_release_guard.py — 271 passed
  • make lint — clean
  • CI_FLOOR ↔ ci.yml job names: all 14 resolve, including the four that come from the templated matrix name Test Python ${{ matrix.python-version }}. (My first pass reported these as missing; that was my own fixed-indent regex, not a defect.)
  • Policy inventory (validate_checks): reading classic branches/main protection alongside the ruleset endpoint is sound — requiring set(contexts) == {checks[].context} stops a requirement existing in one view without a binding in the other, type(app_id) is int and app_id > 0 is a real tightening over is not None, and the branch["commit"]["sha"] == context.target bind prevents reading protection from another state. CI_FLOOR remains satisfiable by the ruleset alone, so classic protection can only add bindings, never substitute for the floor.
  • Artifact byte binding: genuinely byte-level — digest compared against downloaded bytes, exact member set and count (catches duplicate zip names), MAX_UNPACKED bound, symlink rejection, no extractall. Unchanged by this PR; noting it verified, not as new work here.

Re-running the direct main-history check on the next head.

aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Sep 26, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ladon verdict: Approve

Approve — clean review, no blocking findings.

Checked: this PR hardens CI/release policy tooling. check_main_policies.py adds validate_commit unwrapping of GitHub two-parent merge bodies plus a SHA+subject-pinned historical footer exception for #1174, with matching tests and docs. Verified the exception is fail-closed (only the footer is waived; the load-bearing ! semver signal is preserved) and consistent across code, tests, and both docs. No touches to the public adcp.* surface, generated code, forward-compat/type-layering modules, or ctx_metadata. CI gates remain armed (no disabled tests, no dropped ruff rules, no blanket type-ignores). Changes are confined to CI workflow, release scripts, tests, and docs.

Decision-table walk: no critical/high/medium findings (row 1 n/a); gated_paths=false (row 2 n/a); high_risk=false, no deletions (rows 3–5 n/a); prior decision was approve, not escalate (row 6 n/a); no no-auto-approve team match (row 7 n/a); zero medium findings (row 8 n/a) → row 9 approve. Consistent with the prior clean approve pass; nothing new to surface.

Copy link
Copy Markdown
Contributor Author

Updated to 86adc6d01 for renewed exact-head review. The historical exceptions now require both the exact #1174/#1192 commit SHA and the exact committed conventional subject. All other breaking commits remain strict, including new commits with those subjects.

The new actual-history regression failed on #1192 before the fix. The corrected direct main-push check passes all 28 integrations; 282 focused tests, lint, all type checks and normal hooks pass. The migration boundaries are documented in the runbook and release notes.

Fresh CI and the full local make test are running. Please keep merge on hold until this head is green and approved.

Copy link
Copy Markdown
Contributor Author

Coordinator check at 86adc6d: ran the conventional main-push policy against current main 2bfbca4; all 28 first-parent integrations passed. The historical exceptions are bound to the exact SHA and subject, and 55 focused policy tests passed. Fresh CI and exact-head independent approval are still required before merge.

Copy link
Copy Markdown
Contributor Author

Exact-head technical verdict — 86adc6d0

No blocking findings. The main-push policy edge is closed. Technically good to merge once CI is green; formal exact-head APPROVE still needs the distinct aao-secretariat identity.

The blocker is gone — verified directly, not inferred

Ran the module itself against current main (2bfbca4b9):

28 commits after floor, REJECTED 0
MAIN-HISTORY CHECK: PASSES

That's the same probe that reported 34c8f6d92 at the previous head. All 28 now validate, including the nine GitHub-default merge subjects that the validate_commit unwrapping handles.

The exception still can't leak — the part that mattered most

Generalising one exception into a dict is where a fix like this usually goes wrong, so I probed the leak vectors independently before looking at the tests:

case result
exempt subject, different SHA rejected
exempt SHA, different subject rejected
brand-new breaking, no footer, no SHA rejected
brand-new breaking, random SHA rejected
exempt SHA + exempt subject accepted
new breaking with footer accepted

Both keys are still required, exact-match, with no pattern matching and no blanket "merges are exempt". The prospective boundary is unchanged: any new breaking commit — merge or squash — still needs its footer.

Your own test_historical_footer_exception_requires_exact_commit_and_subject covers the same eight vectors, including the cross-contamination case (34c8f6d9's SHA paired with the other exception's subject → rejected). Independent probe and the test agree.

The pinned history fixture is a good call

tests/fixtures/release_main_history.json captures real committed messages and parents, so the regression runs on a shallow CI checkout with no network and no dependence on a moving origin/main. That's the right way to keep this honest — the previous two heads were only caught by running against live main, which CI wouldn't have done.

Optional nit: assert len(result["commits"]) == 28 freezes the count alongside the fixture, so the two have to move together. Harmless, just a maintenance coupling worth knowing about.

Carried forward from the earlier heads, still clean

  • Policy inventory (validate_checks): classic branches/main protection read alongside the ruleset; set(contexts) == {checks[].context} prevents a requirement existing in one view without a binding in the other; type(app_id) is int and app_id > 0 is a real tightening over is not None; the branch["commit"]["sha"] == context.target bind prevents reading protection from another state; CI_FLOOR stays satisfiable by the ruleset alone, so classic protection can only add bindings.
  • CI_FLOOR ↔ ci.yml: all 14 entries resolve, including the four from the templated matrix name.
  • Artifact byte binding: byte-level — digest compared against downloaded content, exact member set and count (catches duplicate zip names), MAX_UNPACKED bound, symlink rejection, no extractall. Pre-existing and untouched here; verified, not credited to this PR.
  • Workflow hardening: pinned action SHA, persist-credentials: false, per-job permissions, one fewer third-party action, GITHUB_WORKFLOW_SHA bound to the target.

What I ran at 86adc6d0

  • tests/test_main_release_policies.py + tests/test_release_guard.py — 276 passed
  • make lint — clean
  • Direct main-history check via main_commits() + validate_commit() against 2bfbca4b9
  • Six independent exception-leak probes

CI at time of writing: 17 SUCCESS / 30 pending, no failures.

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ladon verdict: Approve

Approve — clean pass, no blocking or medium findings.

The diff symmetrically extends the historical breaking-footer exception in check_main_policies.py to a second already-merged commit (#1192, 34c8f6d) with a fail-closed SHA+subject guard, paired with matching docs in releasing.md and reporting-release-notes.md, plus a real-history regression fixture and test replaying the 28-commit first-parent chain.

No public-surface/semver, forward-compat, type-layering, generated-code, credential, or CI-gate concerns. No high-risk paths touched; gated_paths is false; no author-team gate. This is a net-positive follow-up to the prior clean approve.

Decision table: rows 1–8 do not fire (no findings, no gate triggers, high_risk false, no no-auto-approve team, fewer than three medium findings) → row 9 approve. Note review_decision is REVIEW_REQUIRED but gated_paths is false, so row 2 does not apply.

Copy link
Copy Markdown
Contributor Author

make test passed on 86adc6d0161779ba54195134746b1eb36a8dffa0 (exit 0): 12,187 passed, 2,359 skipped, 9 deselected, 1 xfailed, 553 warnings in 2478.76s. Coverage is 81.58%, above the unchanged 80% floor.

Lint, all type checks, 282 focused tests and the direct 28-commit main-history policy check also pass. Current-head aao approval is in place, with no open review threads. CI is down to the Python 3.12 coverage job; all other checks are green.

#1201 remains ready and held for #1217 to merge first. Rebase only if it conflicts; the final combined main will run fresh CI before any release proposal.

@bokelley
bokelley merged commit 06b16a0 into main Sep 27, 2026
52 checks passed
@bokelley
bokelley deleted the claude/issue-1198-release-readiness branch September 27, 2026 00:29
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.

1 participant