Repository navigation
Conversation
…mit hash Port of the RN Expo change (YPE-6067). The signoff comment named the PR head SHA, so every push voided it: on Expo's first real major that cost four rounds, three of them lost to a lint fix and review feedback that changed nothing about what was approved. It is also stricter than this repo's own review policy, where `dismiss_stale_reviews_on_push` is false, so an ordinary code review survives a push while a breaking-change acknowledgment did not. React has not hit this only because no major has been through its gate yet. The signoff now names a token derived from the release: every changeset at head by git blob id, plus the resulting version. Ordinary pushes leave it standing; adding or editing a changeset, or moving the version, requires a fresh one. `isChangeset` is hoisted to module scope so the token and the major detector read the same rule rather than drifting apart.
🦋 Changeset detectedLatest commit: 62e73f4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen 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 |
Kyleasmth
marked this pull request as ready for review
October 5, 2026 20:32
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
jhampton
requested changes
Oct 5, 2026
Greptile found that `signoffToken` was defined but never called here, so the preview emitted no `signoff_token`, the gate's guard rejected the empty value, and every major PR would have been blocked behind a signoff that could never pass. The earlier edit missed because it matched on a six-space indent where this file uses four, and a no-op `String.replace` reports nothing. The four assertions that were supposed to catch this only read the workflow's text, so they all passed while the key was never written. Replaced with checks that bind the two sides together. - emit `signoff_token`, and assert the preview writes the key the workflow's jq reads - move `signoffToken` into its own module so the suite can drive it against real git trees - split the ls-tree line at the first tab only; a tab in a changeset path was truncating it out of the digest, and a file that is absent cannot be seen to change - restore the module from main with the rest of the tooling, or a PR could ship its own token logic and mint one matching an older signoff Eight behavioural cases plus the contract and restore-list assertions, each mutation-tested: removing the emit, dropping the module from the restore list, and reverting the tab fix each turn a test red.
camrun91
reviewed
Oct 8, 2026
camrun91
left a comment
camrun91
left a comment
Collaborator
There was a problem hiding this comment.
Four notes on the release token.
… the script tests in CI
jhampton
approved these changes
Oct 9, 2026
camrun91
self-requested a review
October 9, 2026 18:41
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes YPE-6068. Port of youversion/platform-sdk-reactnative-expo#230.
Problem
The signoff comment had to name the PR's head SHA, so every push voided it. Expo hit this on its first real major (#215, v2.0.0): four rounds of signoff, three lost to a lint fix and review feedback that changed nothing about what was approved.
It is also stricter than this repo's own policy.
Stable Mainsetsdismiss_stale_reviews_on_push: false, so an ordinary code review survives a push while a breaking-change acknowledgment did not.React has not hit this yet only because no major has gone through its gate. The defect is already here.
Change
The signoff now names a release token: every changeset at head by git blob id, plus the resulting version.
isChangesetis hoisted to module scope so the token and the major detector read the same rule instead of keeping two copies that can drift.Two decisions worth a reviewer's attention
1. Scope is every changeset, not only the ones declaring major. Any changeset edit can move the release, so erring toward asking again is the safe direction. The cost is that adding an unrelated patch changeset re-triggers a signoff.
2. 64 bits, not a short prefix. The old comment argued a 7-char SHA prefix was brute-forceable; the same reasoning applies to a digest, so the token is 16 hex characters.
An absent or malformed token now fails the job closed rather than degenerating the matcher into one that matches almost any comment.
Verification
Review Expo #230 first; this is the same change with React's inline changeset predicate.
The PR appears safe to merge; no new actionable issues remain.Summary
This PR replaces commit-based breaking-change approval with a release token.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart TD Head[PR head] --> Diff[Changesets changed since merge base] Head --> Config[Config at head] Preview[Release preview] --> Version[Next version] Diff --> Token[Release token] Config --> Token Version --> Token Token --> Match[Check collaborator comment] Match --> Status[Post status on PR head]Reviews (7) · Last reviewed commit: "ci: key the signoff token on this PR's c..." · Reviewed by Greptile