Add additional contraints for Claude when removing blocker labels - #81538
Conversation
|
@hungvu193 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
@hungvu193 No need for your review, this can be reviewed by an internal employee since it doesn't involve App changes. |
![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: ce6ea4c9e0
ℹ️ 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".
|
|
||
| **DO NOT:** | ||
| - Remove `DeployBlockerCash` if there's an App PR that caused or contributed to the issue | ||
| - Remove `DeployBlockerCash` if the issue is not reproducible on production |
There was a problem hiding this comment.
This new constraint conflicts with the decision tree that explicitly removes DeployBlockerCash for backend-caused issues, and it will force the agent to keep the App deploy blocked whenever an issue reproduces only on staging. In that scenario, the investigation can still confidently classify the bug as backend (per the tree), but this rule prevents the required label correction and can unnecessarily block App releases.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Yes, on purpose, this is to avoid deploying broken new features
| **DO NOT:** | ||
| - Remove `DeployBlockerCash` if there's an App PR that caused or contributed to the issue | ||
| - Remove `DeployBlockerCash` if the issue is not reproducible on production | ||
| - Remove any of the blocker labels if it has been added by an internal employee |
There was a problem hiding this comment.
This blanket prohibition overrides the label decision logic and prevents cleanup of incorrect blocker labels solely based on who added them. If an internal employee adds the wrong blocker label, the investigator is now instructed to keep it even after finding evidence to the contrary, which can leave unrelated deploys blocked and undermines the triage workflow described earlier in this file.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
That's the point of it 😄
| **DO NOT:** | ||
| - Remove `DeployBlockerCash` if there's an App PR that caused or contributed to the issue | ||
| - Remove `DeployBlockerCash` if the issue is not reproducible on production | ||
| - Remove any of the blocker labels if it has been added by an internal employee |
There was a problem hiding this comment.
Might have to check the runs from this because not sure if it has the permission to see who added the label
|
this shouldn't touch a remote build so ignoring |
|
@thienlnam looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
|
🚀 Deployed to staging by https://github.com/thienlnam in version: 9.3.13-1 🚀
|
|
🚀 Deployed to staging by https://github.com/thienlnam in version: 9.3.15-0 🚀
|
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.3.15-10 🚀
|
3 participants
Explanation of Change
This prevents Claude from removing the label if the issue is not in production, thereby avoiding the deployment of broken features.
Moreover, it prevents Claude from overwriting internal employee decisions to add the label, rather than constantly removing it.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/595830
PROPOSAL:
Tests
N/A
Offline tests
N/A
QA Steps
Note: This requires an internal employee to test it
DeployBlockerCashlabel. Melvin should NOT add it back.PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, 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.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari