Fix deploy checklist including cherry-picked PRs already in production - #94903
Fix deploy checklist including cherry-picked PRs already in production#94903mountiny wants to merge 3 commits into
Conversation
When a PR is cherry-picked directly to the staging branch and deployed to production, its original merge commit can re-enter the staging range after the post-deploy sync. Because that original commit predates the previous staging tag (it was merged to main before the previous cycle started), the existing duplicate-detection and previous-checklist filters both miss it, causing the PR to appear in the next cycle's deploy checklist even though it is already in production. Fix by capturing the committer date of the base tag's commit from the compareCommits API response (available at no extra cost as base_commit on the first page). Any PR whose commit date predates this baseline was brought into the range by a post-deploy sync, not by a genuine merge in the current cycle, and is filtered out. Fixes #94901 Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13050a0f1c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // | ||
| // We use this as the filter signal: any PR whose commit date is before the previous staging | ||
| // tag's commit date was already deployed to production via cherry-pick and must be excluded. | ||
| const staleEntries = baseCommitDate ? mergedPREntries.filter((pr) => pr.date < baseCommitDate) : []; |
There was a problem hiding this comment.
When the stale cherry-pick is an App commit that updates the Mobile-Expensify submodule, the compare range still includes the old Update Mobile-Expensify submodule... commit dated before baseCommitDate, but this filter only derives stale entries from mergedPREntries. Because the unfiltered submoduleUpdates array is later passed into buildChronologicalSection, that cherry-picked submodule-bump scenario still renders an already-deployed Mobile-Expensify update in the next checklist even though stale PRs were removed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 500cc78. is now filtered through the same baseCommitDate staleness check before being passed to buildChronologicalSection, and a dedicated test was added to cover this scenario.
…us staging tag Addresses reviewer feedback: submoduleUpdates was passed unfiltered into buildChronologicalSection, so a Mobile-Expensify submodule bump that was cherry-picked and already deployed to production could still appear in the new checklist's chronological section. Apply the same baseCommitDate staleness filter to submoduleUpdates so that commits predating the previous staging tag are excluded. Co-authored-by: Cursor <cursoragent@cursor.com>
Recompiled all compiled index.js action bundles to reflect the updated
return type of getCommitHistoryBetweenTags ({commits, baseCommitDate}).
Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Explanation of Change
When a PR (or a Mobile-Expensify submodule update) is cherry-picked to staging and deployed to production, its original merge commit re-enters the staging range via a post-deploy sync. Because that original commit predates the previous staging tag, neither the duplicate-detection filter nor the previous-checklist filter removes it — so it incorrectly appears in the next cycle's deploy checklist even though it is already in production.
Root cause signal: Any commit appearing in
compareCommits(previousStagingTag, newStagingTag)whose committer date is before the base tag's own commit date was brought into the range by a post-deploy sync, not by a genuine merge in the current cycle.Fix:
GithubUtils.getCommitHistoryBetweenTagsnow returns{commits, baseCommitDate}, capturingbase_commit.commit.committer.datefrom the firstcompareCommitspage at no extra API cost.GitUtils.getMergedPRsDeployedBetweenpropagatesbaseCommitDatethroughMergedPRsResult.createOrUpdateDeployChecklist.tsfilters bothmergedPREntriesandsubmoduleUpdates(the reviewer caught that submodule bumps had the same problem) using thebaseCommitDatesignal before building the chronological section and the main PR list.index.jsbundles rebuilt vianpm run gh-actions-build.Fixed Issues
$ #94901
PROPOSAL:
Tests
npx jest tests/unit/GithubUtilsTest.ts tests/unit/createOrUpdateDeployChecklistTest.ts'filters out stale PRs whose commit date predates the previous staging tag (cherry-picked in a prior cycle)''filters out stale Mobile-Expensify submodule updates predating the previous staging tag''should return empty baseCommitDate when base_commit is missing from API response'(graceful fallback)StagingDeployCashchecklist's chronological section.Offline tests
N/A — this change is a CI/CD script, not a user-facing feature.
QA Steps
N/A — this change affects the deploy checklist generation script only, not app behavior. Validation is done via the unit tests above and by observing the next staging deploy checklist after a cherry-pick.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
This PR only modifies CI/CD scripts and has no visual UI changes.
Android: Native
N/A — no UI changes
Android: mWeb Chrome
N/A — no UI changes
iOS: Native
N/A — no UI changes
iOS: mWeb Safari
N/A — no UI changes
MacOS: Chrome / Safari
N/A — no UI changes