ci: require human signoff before a major release by Kyleasmth · Pull Request #387 · youversion/platform-sdk-react

Kyleasmth · 2026-09-08T19:24:18Z

Closes YPE-2523 — pending a scope correction, see below.

Ports the Swift SDK's major-release signoff gate (YPE-2521) to this repo. A PR that declares a breaking change cannot merge until a write-access collaborator other than the author confirms it, by name, with the exact version and a 🚀.

The ticket's stated mechanism does not exist here

YPE-2523 says "semantic-release (current branch) detects a breaking change". This repo has no semantic-release: releases run on Changesets, and the bump level is declared in .changeset/*.md frontmatter rather than inferred from commit footers.

That is not a new discovery. docs/release-hardening-decisions.md (Decision 1, jhampton 2026-07-01) already records it:

In the Swift SDK, conventional commits drive changelog + version math. This repo does not work that way … Version bump = the level declared in each changeset … No semantic-release / conventional-changelog tooling.

So the gate is the same; only the detector changes. scripts/preview-release.mjs keeps the Swift name and output contract, but asks changeset status instead of @semantic-release/commit-analyzer.

One deliberate difference from Swift

Swift gates on "this release is a major". Here that would be wrong. Changesets accumulates changesets until the Version Packages PR ships them, so a major pending on main would make every unrelated PR report major and get blocked until the release went out.

The script therefore reports two separate things, and only the second gates:

Field	Means
`is_major`	the pending release would be a major (informational)
`introduced_major`	this PR added a changeset declaring `major` (gates)

Verified behaviour

Run against real commits on this branch, not reasoned about:

Scenario	Gate	`next`	`is_major`	`introduced_major`
CI-only PR (empty changeset)	pass	—	false	false
Adds a `patch` changeset	pass	2.12.2	false	false
Adds a `major` changeset	BLOCKS	3.0.0	true	true
Unrelated patch, major already pending	pass	3.0.0	true	false

The last row is the one a straight port would get wrong.

Also confirmed: the three packages are a fixed group in .changeset/config.json, so one major changeset bumps all three to the same 3.0.0. That keeps Swift's single-version signoff format valid — worth knowing, because if fixed were ever removed the signoff comment would become ambiguous about which version is approved. The script fails loudly rather than picking one if it sees more than one version.

Carried over from Swift unchanged

The parts that are release-tool-agnostic and were the hard-won bits there:

Rejects bot comments and the PR author, so an author with write access cannot self-clear the gate.
Verifies write/maintain/admin permission, with a per-login cache.
Re-runs on comment deleted, so a cached success cannot outlive its evidence.
Normalises > quote markers and whitespace, so GitHub's "Quote reply" matches.
Upserts one blocking comment rather than posting a new one per push.
Fails loudly if a major is detected but no version could be computed, rather than letting a degenerate regex pass the gate.

Not done, needs someone with admin

The workflow only blocks if major-release-signoff is added as a required status check on main. I cannot read branch protection (404 without admin). This is what actually made the Swift gate bite, so it should not be skipped.

Scope

Asking on the ticket for the description to be corrected before this merges, since it currently specifies a mechanism this repo does not have.

Greptile Summary

This PR adds a Changesets-based major-release approval gate that binds collaborator signoff to the precise release version and full PR head SHA.

Computes whether the PR introduced a major changeset while distinguishing majors already pending on main.
Splits unprivileged release-preview execution from the job that writes statuses and comments.
Requires the exact version, full commit hash, acknowledgment phrase, and rocket marker from a write-access non-author.
Adds the Changesets parser dependency and an intentional empty changeset.

Confidence Score: 3/5

This PR is not safe to merge until the status-writing gate executes from a trusted workflow definition that the evaluated PR cannot modify.

A same-repository PR can alter the pull_request-triggered gate job while retaining its status-write token, allowing it to publish a successful major-release-signoff result without an independent collaborator's approval.

Files Needing Attention: .github/workflows/major-release-signoff.yml

Security Review

The privileged gate remains defined by the pull request it evaluates. A same-repository PR can modify the status-writing commands and clear its own required signoff check without independent approval.

How this was verified: The pull_request-triggered workflow grants status-write access to its inline gate job, which directly publishes the protected status context.

Important Files Changed

Filename	Overview
.github/workflows/major-release-signoff.yml	Adds the complete signoff workflow and fixes the previously reported execution, pinning, and stale-approval paths, but its PR-controlled privileged gate can still forge the protected status.
scripts/preview-release.mjs	Implements Changesets-native release detection and now compares added, modified, and renamed changesets per package against the merge base.
package.json	Adds the Changesets parser used to interpret changeset frontmatter consistently with release tooling.
pnpm-lock.yaml	Locks the parser dependency and its resolved transitive dependency graph.
.changeset/odd-towns-unite.md	Records the CI-only change as an intentional empty changeset.

Sequence Diagram

sequenceDiagram
    participant A as PR author
    participant W as PR-defined workflow
    participant G as Privileged gate job
    participant S as GitHub status API
    A->>W: Open internal PR modifying gate commands
    W->>G: Run on pull_request
    Note over G: statuses: write
    G->>S: "Publish major-release-signoff = success"
    S-->>A: Required gate appears satisfied

Prompt To Fix All With AI

### Issue 1
.github/workflows/major-release-signoff.yml:173-175
**PR workflow can forge signoff**

When a same-repository PR modifies this workflow, its `pull_request` run executes the modified gate job with `statuses: write`, allowing the PR to publish a successful `major-release-signoff` status without a qualifying collaborator approval. This bypasses the independent human signoff required for a breaking release.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (5): Last reviewed commit: "fix(ci): match the full commit hash so a..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

changeset-bot · 2026-09-08T19:24:23Z

🦋 Changeset detected

Latest commit: 9e25007

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

… branch

Kyleasmth · 2026-09-09T18:15:00Z

@jhampton @camrun91 — out of draft and green. Two different asks, so splitting them:

@jhampton — the open question is on YPE-2523, not in this diff. The ticket specifies semantic-release detecting the breaking change; this repo has none, so the detector here is changeset status instead. That is not a new finding — your Decision 1 on YPE-2486 already records that this repo does not work the Swift way. The code is correct either way; the ticket wording is what needs your call.

The one thing that would change this diff: I gate on "this PR added a major changeset", not "the pending release is a major". Gating on the latter would block every unrelated PR while a major sits pending on main. If you want the Version Packages PR gated as a second line of defence, that is a different detector and I would rather add it deliberately than assume it.

@camrun91 — normal review welcome, particularly on whether the workflow fits how this repo runs CI.

Worth knowing, since it is the interesting bit: the gate failed on its own PR when I first pushed it, and the reason was hidden because the script swallowed stderr. changeset status resolves the configured baseBranch as a local ref, and a PR checkout only has origin/main — it worked locally purely because my clone had a real main branch. Fixed by fetching main:refs/heads/main in the workflow, and the script now surfaces Changesets' own message rather than exiting silently. It is passing its own gate now, which is a real test rather than a synthetic one: this PR carries an empty changeset, so introduced_major is false and the check goes green on its own merits.

Still needs someone with admin: the gate only actually blocks once major-release-signoff is a required status check on main. I get a 404 reading branch protection.

camrun91 · 2026-09-09T18:18:43Z

@jhampton @camrun91 — out of draft and green. Two different asks, so splitting them:

@jhampton — the open question is on YPE-2523, not in this diff. The ticket specifies semantic-release detecting the breaking change; this repo has none, so the detector here is changeset status instead. That is not a new finding — your Decision 1 on YPE-2486 already records that this repo does not work the Swift way. The code is correct either way; the ticket wording is what needs your call.

The one thing that would change this diff: I gate on "this PR added a major changeset", not "the pending release is a major". Gating on the latter would block every unrelated PR while a major sits pending on main. If you want the Version Packages PR gated as a second line of defence, that is a different detector and I would rather add it deliberately than assume it.

@camrun91 — normal review welcome, particularly on whether the workflow fits how this repo runs CI.

Worth knowing, since it is the interesting bit: the gate failed on its own PR when I first pushed it, and the reason was hidden because the script swallowed stderr. changeset status resolves the configured baseBranch as a local ref, and a PR checkout only has origin/main — it worked locally purely because my clone had a real main branch. Fixed by fetching main:refs/heads/main in the workflow, and the script now surfaces Changesets' own message rather than exiting silently. It is passing its own gate now, which is a real test rather than a synthetic one: this PR carries an empty changeset, so introduced_major is false and the check goes green on its own merits.

Still needs someone with admin: the gate only actually blocks once major-release-signoff is a required status check on main. I get a 404 reading branch protection.

Did you mean to tag @cameronapak?

cameronapak

Review

Jira: YPE-2523. Spec is the PR body.

Summary

Standards: 2 must-fix findings. Spec: 2 must-fix findings. Worst: the comment-triggered path executes PR-controlled install code with write-capable credentials.

For Agents: review basis

CI line: major-release-signoff
Greptile/bot: prior findings were independently verified.
Event: issue_comment
HEAD: fb94946456e826775b6fca1d75aeb2f50d94d829

By Code Reviewer bot, sent on behalf of Cam.

cameronapak · 2026-09-10T15:29:18Z

suggestion: Prefer GitHub-native approvals over a custom comment-driven release gate.

For Agents: simpler release control

A simpler approach would be to assign release maintainers as CODEOWNERS for .changeset/*, require code-owner approval on main, and dismiss stale approvals when new commits are pushed. This makes every release-bearing PR—including a major changeset—require explicit review from someone responsible for release impact, while automatically covering modified changesets.

That removes the issue_comment workflow, exact phrase/version/emoji parsing, PR-head installation, custom status management, and privileged mutable action execution. If approval must happen only at publication time, a protected GitHub Environment with required reviewers on the npm publish job is the next-simplest native option.

The tradeoff is that CODEOWNERS requires approval for every release-bearing changeset rather than only majors, but it directly protects the stated goal with substantially less custom security-sensitive code.

By Code Reviewer bot, sent on behalf of Cam.

Kyleasmth · 2026-09-10T15:38:04Z

@jhampton @camrun91 — out of draft and green. Two different asks, so splitting them:
@jhampton — the open question is on YPE-2523, not in this diff. The ticket specifies semantic-release detecting the breaking change; this repo has none, so the detector here is changeset status instead. That is not a new finding — your Decision 1 on YPE-2486 already records that this repo does not work the Swift way. The code is correct either way; the ticket wording is what needs your call.
The one thing that would change this diff: I gate on "this PR added a major changeset", not "the pending release is a major". Gating on the latter would block every unrelated PR while a major sits pending on main. If you want the Version Packages PR gated as a second line of defence, that is a different detector and I would rather add it deliberately than assume it.
@camrun91 — normal review welcome, particularly on whether the workflow fits how this repo runs CI.
Worth knowing, since it is the interesting bit: the gate failed on its own PR when I first pushed it, and the reason was hidden because the script swallowed stderr. changeset status resolves the configured baseBranch as a local ref, and a PR checkout only has origin/main — it worked locally purely because my clone had a real main branch. Fixed by fetching main:refs/heads/main in the workflow, and the script now surfaces Changesets' own message rather than exiting silently. It is passing its own gate now, which is a real test rather than a synthetic one: this PR carries an empty changeset, so introduced_major is false and the check goes green on its own merits.
Still needs someone with admin: the gate only actually blocks once major-release-signoff is a required status check on main. I get a 404 reading branch protection.

Did you mean to tag @cameronapak?

Yes, thank you.

…red parsing

…nd filename gaps

github-actions · 2026-09-10T20:21:55Z

🚨 Breaking change detected — signoff required for `v`

This PR adds a changeset declaring a major bump, which would ship all three packages as v (they are a fixed group in .changeset/config.json, so they version together). Merging is blocked until a repo collaborator with write access comments on this PR with all three (3) of the following:

The verbatim acknowledgment phrase below,
The precise next version (v or ``), and
A 🚀 (:rocket:) emoji.

I confirm that this is an intentional breaking change, and I have read the release procedures. I understand and have documented its impact upon release.

Copy-paste-ready reply:

I confirm that this is an intentional breaking change, and I have read the release procedures. I understand and have documented its impact upon release.

v 🚀

The check re-runs automatically when a qualifying comment is posted or edited.

Kyleasmth · 2026-09-10T20:22:52Z

All three addressed in 5ba79fc and c5fac06. Thanks both — chasing these turned up more than the original three, so worth writing down what changed.

Privileged workflow executing PR code

Split into three jobs. The one that runs PR code holds no write scopes; the one that can write statuses and comments never checks that code out.

Job	Permissions	Runs PR code
`context`	`pull-requests: read`	no
`preview`	`contents: read`	yes
`gate`	`pull-requests: write`, `statuses: write`	no

Workflow-level is now permissions: {}. Fork PRs skip preview entirely and the gate fails closed rather than trusting a preview computed by untrusted code.

That alone was not enough, though. A same-repo author could still edit scripts/preview-release.mjs to return introduced_major: false and the trusted job would post green. So the preview job now restores the detector from main before anything installs: scripts/preview-release.mjs, package.json, pnpm-lock.yaml, pnpm-workspace.yaml, .changeset/config.json, and .npmrc / .pnpmfile.cjs restored-or-deleted. The PR supplies changeset data only.

--ignore-scripts also was not sufficient on its own: pnpm hooks are a separate mechanism, so a PR-authored .pnpmfile.cjs could rewrite the detector after restore. Install is now --ignore-scripts --ignore-pnpmfile.

Modified changesets bypassing the gate

Confirmed exactly as described, against real commits. Editing a pending changeset from patch to major gave:

	`introduced_major`	`is_major`
before	false	true
after	true	true

So the workflow posted "No breaking changeset added; signoff not required" while Changesets computed a real major.

Now --diff-filter=AMR with rename following, and the base comparison is per package so an existing major on one package cannot mask a different one being raised. Three further bypasses fell out while testing, each proved with real commits:

Renaming a changeset while raising it to major reported as R and was skipped entirely.
Filenames git C-quotes (a literal tab, say) failed the changeset name test and dropped out of the scan. Diff parsing is now -z NUL-delimited.
YAML forms the line regex missed: {"pkg": major}, anchors/aliases, block scalars, !!str, quoted levels. The hand-written parser is gone; @changesets/parse is a real dependency now, pinned to 0.4.1, the exact version CLI 2.29.7 resolves, so it cannot drift from what changeset status itself computes.

Mutable action tags

All three pinned to full SHAs with the tag in a trailing comment.

One more, not in the original review

A signoff was not bound to the head it was given on. Approve v3.0.0 on commit A, push commit B with additional breaking changes, version is still 3.0.0, and the old comment satisfied the gate. This one can happen by accident rather than by malice. Comments older than the commit under review no longer count.

Also fixed: on a preview failure the job used to abort before reaching the step that posts a red status, so a previously-green status could outlive the signoff that earned it. The decision step now records why it cannot evaluate and the red-status and fail steps run under always().

Known residuals

If the context lookup itself fails, gate is skipped and a stale green can remain on the SHA. Cannot be fixed by the job condition alone, since an issue_comment payload carries no head SHA. The mitigation would be setting the status to pending in the trusted phase first; happy to add it if you want it.
A same-repo collaborator can still edit the workflow file itself. That is inherent to the gate living in the repo it guards.

…accept

…t timestamp

…nherit a signoff Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gnt93sVjs6ApXSzkDC2efU

greptile-apps · 2026-09-11T14:15:54Z

+    permissions:
+      pull-requests: write
+      statuses: write


PR workflow can forge signoff

When a same-repository PR modifies this workflow, its pull_request run executes the modified gate job with statuses: write, allowing the PR to publish a successful major-release-signoff status without a qualifying collaborator approval. This bypasses the independent human signoff required for a breaking release.

Prompt To Fix With AI

This is a comment left during a code review. Path: .github/workflows/major-release-signoff.yml Line: 173-175 Comment: **PR workflow can forge signoff** When a same-repository PR modifies this workflow, its `pull_request` run executes the modified gate job with `statuses: write`, allowing the PR to publish a successful `major-release-signoff` status without a qualifying collaborator approval. This bypasses the independent human signoff required for a breaking release. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

jhampton · 2026-09-11T16:05:14Z

+  context:
+    # issue_comment fires for issues AND PRs across the repo. Skip non-PR
+    # issues so we don't waste a run when someone comments on a plain issue.
+    if: ${{ github.event_name != 'issue_comment' || github.event.issue.pull_request != null }}


jhampton · 2026-09-11T16:05:30Z

+      pull-requests: read
+    outputs:
+      head_sha: ${{ steps.pr.outputs.head_sha }}
+      is_fork: ${{ steps.pr.outputs.is_fork }}


Good thinking.

jhampton · 2026-09-11T16:55:20Z

+          BASE=$(git merge-base main "$HEAD_SHA")
+          node scripts/preview-release.mjs --base "$BASE" --head "$HEAD_SHA" > preview.json
+          cat preview.json
+          # `introduced_major`, not `is_major`: a major already pending on main


jhampton · 2026-09-11T16:55:32Z

+          ref: ${{ needs.context.outputs.head_sha }}
+          fetch-depth: 0
+
+      - name: Restore release tooling from the base branch


Security, nice.

jhampton · 2026-09-11T16:57:19Z

+    runs-on: ubuntu-latest
+    permissions:
+      pull-requests: write
+      statuses: write


I get this, but I'm not sure it's something we must guard against. That said, it could happen accidentally, so we need some kind of answer. How about we check the file modifications against the workflows folder and establish a rule that breaking change PRs cannot introduce or change CI behavior, i.e. That would need to be a separate PR. What do you think?

Worth having, but one catch: on pull_request GitHub runs the workflow file from the PR head, so a PR could delete the new rule in the same commit. Any guard in this file is editable by the PR it guards.

What a PR cannot touch is on your side:

make major-release-signoff a required check

"Require review from Code Owners" + a CODEOWNERS entry for .github/workflows/**

No CODEOWNERS in the repo today. Who should own that path?

ci: require human signoff before a major release

4e13206

fix: create a local main ref so changeset status can resolve the base…

fb94946

… branch

Kyleasmth marked this pull request as ready for review September 9, 2026 18:09

greptile-apps Bot reviewed Sep 9, 2026

View reviewed changes

Comment thread .github/workflows/major-release-signoff.yml Outdated

Comment thread scripts/preview-release.mjs Outdated

Comment thread .github/workflows/major-release-signoff.yml Outdated

Kyleasmth requested review from camrun91 and jhampton September 9, 2026 18:14

cameronapak reviewed Sep 10, 2026

View reviewed changes

Comment thread .github/workflows/major-release-signoff.yml Outdated

Comment thread scripts/preview-release.mjs Outdated

Comment thread .github/workflows/major-release-signoff.yml Outdated

Kyleasmth removed the request for review from camrun91 September 10, 2026 18:56

Kyleasmth added 2 commits September 10, 2026 12:38

fix(ci): stop the signoff gate trusting PR-authored code and PR-autho…

5ba79fc

…red parsing

fix(ci): bind a signoff to the reviewed head and close the pnpmfile a…

c5fac06

…nd filename gaps

greptile-apps Bot reviewed Sep 10, 2026

View reviewed changes

Comment thread .github/workflows/major-release-signoff.yml Outdated

Kyleasmth added 4 commits September 10, 2026 13:46

fix(ci): let the gate bootstrap itself and post a status github will …

c4fbcd6

…accept

fix(ci): take all of main's release tooling or none of it

2041528

fix(ci): render an absent release type as no bump instead of null

15e1a84

fix(ci): bind a signoff to the commit hash rather than a committer-se…

5d1d291

…t timestamp

greptile-apps Bot reviewed Sep 10, 2026

View reviewed changes

Comment thread .github/workflows/major-release-signoff.yml Outdated

fix(ci): match the full commit hash so a forged short prefix cannot i…

9e25007

…nherit a signoff Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gnt93sVjs6ApXSzkDC2efU

greptile-apps Bot reviewed Sep 11, 2026

View reviewed changes

jhampton reviewed Sep 11, 2026

View reviewed changes

Uh oh!

Conversation

Kyleasmth commented Sep 8, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The ticket's stated mechanism does not exist here

One deliberate difference from Swift

Verified behaviour

Carried over from Swift unchanged

Not done, needs someone with admin

Scope

Greptile Summary

Confidence Score: 3/5

Security Review

Important Files Changed

Sequence Diagram

Uh oh!

changeset-bot Bot commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Kyleasmth commented Sep 9, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

camrun91 commented Sep 9, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cameronapak left a comment

Choose a reason for hiding this comment

Review

Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cameronapak commented Sep 10, 2026

Uh oh!

Kyleasmth commented Sep 10, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 10, 2026

🚨 Breaking change detected — signoff required for v

Uh oh!

Kyleasmth commented Sep 10, 2026

Privileged workflow executing PR code

Modified changesets bypassing the gate

Mutable action tags

One more, not in the original review

Known residuals

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

jhampton Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

jhampton Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

jhampton Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

jhampton Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

jhampton Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

Kyleasmth Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Kyleasmth commented Sep 8, 2026 •

edited by greptile-apps Bot

Loading

changeset-bot Bot commented Sep 8, 2026 •

edited

Loading

Kyleasmth commented Sep 9, 2026 •

edited by atlassian Bot

Loading

camrun91 commented Sep 9, 2026 •

edited by atlassian Bot

Loading

Kyleasmth commented Sep 10, 2026 •

edited by atlassian Bot

Loading

🚨 Breaking change detected — signoff required for `v`