Skip to content

fix(release): recover committed release artifacts - #7605

Merged
bokelley merged 1 commit into
mainfrom
untitled-v2
Sep 20, 2026
Merged

bokelley merged 1 commit into
mainfrom
untitled-v2

Conversation

@bokelley

@bokelley bokelley commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Treat a human merge as authorization only for a bot-authored release PR; final-head human approvals remain valid.
  • Add a manual recovery path requiring an exact commit SHA already reachable from the selected release branch.
  • Publish and tag the validated target commit, and fail closed when it has no version-matched committed artifacts.
  • Mark release-workflow-only maintenance as changeset-status exempt so operational fixes do not cut a protocol version.

RC.4 recovery

After this merges, dispatch the Release workflow on main with release_commit 9497665. The workflow will rerun release verification, recognize the human merge of bot-authored PR #7508, and complete the missing v3.2.0-rc.4 tag, GitHub Release, immutable assets, and downstream release event.

Verification

  • npm run test:release-workflow
  • node tests/changeset-protocol-scope.test.cjs
  • npm run precommit
  • pre-push version, changeset-policy, and scoped-check hooks

Open workspace in Conductor

Comment thread .github/workflows/release.yml Outdated
if [ "${approved_count}" -lt 1 ]; then
echo "::error::Release PR #${pr_number} by ${author} has no human approval on final head ${head_sha}. Refusing to publish artifacts."
merge_authorized=false
if [ "${author_type}" = "Bot" ] && [ "${merger_type}" = "User" ]; then

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.

Medium: The merge-authorization path drops the final-head binding the approval path enforces. The approval branch above requires .commit_id == $head — authorization is bound to the exact head SHA. This branch accepts any human merger of a bot-authored PR (author_type == "Bot" && merger_type == "User") with no equivalent head check. merged_by is not head-bound: with auto-merge, it records the human who enabled auto-merge at an earlier head, not whoever reviewed the commit that actually merged. A commit pushed to the bot PR after enablement then publishes committed dist/** artifacts under a human's name that no human authorized — the exact fail-open the .commit_id == $head binding was added to close. Consider binding this path too (e.g. require the merge commit / final head match what the human acted on, or reject when auto-merge advanced the head after enablement).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 98b6def. The merger path now reads the PR timeline and authorizes only when auto-merge was never enabled or its latest state is explicitly disabled. An active auto-merge state can no longer inherit authorization across a later head update. PR #7508 qualifies because auto-squash was disabled before the final release head was pushed and directly merged.

@aao-secretariat aao-secretariat Bot 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.

Ladon verdict: Escalate to human review

Escalate — gated path requires human/CODEOWNERS approval.

This PR modifies .github/workflows/release.yml, which matches the deterministic hard approval gate (gated_paths: true). The current review_decision is REVIEW_REQUIRED, not APPROVED, so the gate is not satisfied. Per decision rule row 2, the outcome must be escalate (no critical/high findings exist to force request-changes).

Escalation reasons

  • .github/workflows/release.yml (modified) is a gated path; human/CODEOWNERS review with review_decision: APPROVED is required before this can merge.

Medium findings (non-blocking, for reviewer attention)

  • .github/workflows/release.yml — the new bot-authored merge-authorization branch accepts a human merge with no final-head binding, unlike the adjacent .commit_id == $head approval path, reopening an auto-merge fail-open window.

The workflow_dispatch recovery path is otherwise well-guarded (40-char SHA regex, HEAD equality, supported-branch allowlist, merge-base ancestor check, fail-closed on empty/version-mismatched artifacts) and publishes/tags only the validated RELEASE_COMMIT.

Why human review

  • .github/workflows/release.yml (modified) matches gated path .github/workflows/** and review_decision is REVIEW_REQUIRED (not APPROVED) — human/CODEOWNERS approval required.
  • This PR touches a path under a hard, non-overridable approval gate (.github/workflows/release.yml (modified) matches .github/workflows/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

@aao-secretariat aao-secretariat Bot added the ladon/needs-human-review Ladon has escalated this PR for human review. label Sep 20, 2026

@aao-secretariat aao-secretariat Bot 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.

Ladon verdict: Escalate to human review

Escalate — gated path requires human/CODEOWNERS approval.

This PR modifies .github/workflows/release.yml, a file under a hard, non-overridable approval gate (gated_paths: true, matches .github/workflows/**). The current review_decision is unknown/not APPROVED, so decision-table row 2 fires: Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

No blocking findings: the reviewer found no Critical/High/Medium defects. The recovery workflow_dispatch path validates its input fail-closed (40-char SHA regex, HEAD match, supported-branch + ancestry checks), the loosened authorization gate still fails closed against app-merges/auto-merge, artifact detection errors when a recovery target has no committed artifacts, and the changeset-scope exemption cannot launder a protocol edit. Supporting tests are added.

This is a subsequent pass; the prior run was also escalate for the same gated-path reason and had no findings. The escalation persists solely because the release-authorization/security-posture change touches a gated workflow path and no human approval is recorded — not because of any new code defect. A CODEOWNERS/human approval lifts the gate.

Why human review

  • .github/workflows/release.yml (modified) matches gated path .github/workflows/** and review_decision is not APPROVED — human/CODEOWNERS approval required before merge.
  • Change alters release authorization / recovery posture on a load-bearing release workflow; ratification of this security-relevant change is a human act.
  • This PR touches a path under a hard, non-overridable approval gate (.github/workflows/release.yml (modified) matches .github/workflows/**) and the current GitHub review decision is 'unknown', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

@bokelley
bokelley merged commit 64c4a51 into main Sep 20, 2026
30 checks passed
@bokelley
bokelley deleted the untitled-v2 branch September 20, 2026 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ladon/needs-human-review Ladon has escalated this PR for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant