[NoQA] Fix reviewer using absolute paths for compiler check script#86082
[NoQA] Fix reviewer using absolute paths for compiler check script#86082kacper-mikolajczak wants to merge 1 commit intoExpensify:mainfrom
Conversation
The allowedTools pattern Bash(check-compiler.sh:*) only matches when the command starts with the bare script name. The reviewer LLM resolves to absolute paths (/home/runner/.../check-compiler.sh), causing permission denials. Add an explicit instruction to the rule to use the script by name only since it is already on $PATH.
|
Verified the fix works on a fork test run: https://github.com/kacper-mikolajczak/ExpensifyApp/actions/runs/23434474868 The reviewer now invokes Fallback if this doesn't work reliably: Instead of having the reviewer call |
|
@eVoloshchak 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] |
Explanation of Change
Background: PR #85067 added
check-compiler.shto the reviewer'sallowedToolsso theCLEAN-REACT-PATTERNS-0rule could verify React Compiler compliance before flagging manual memoization. The script is on$PATH(via$GITHUB_PATH) and the allowed pattern isBash(check-compiler.sh:*).Problem: After analyzing post-merge reviewer runs (e.g. PRs #85636, #85629), the compiler check is never executing successfully. The reviewer LLM resolves the script to its absolute path (
/home/runner/_work/App/App/.claude/scripts/check-compiler.sh ...) or prefixes it withbash. SinceBash(check-compiler.sh:*)is prefix-matched against the literal command string, these invocations don't match and are permission-denied. The reviewer retries up to 4+ times with different path variants before giving up.This is the same class of issue that previously affected
createInlineComment.shandaddPrReaction.sh(resolved in #83651 by moving script execution to the workflow level via structured JSON output).Solution: Add an explicit instruction in the
CLEAN-REACT-PATTERNS-0rule telling the reviewer to invokecheck-compiler.shby bare name only - no absolute path, nobashprefix. This is the lightweight fix; if it proves insufficient, a follow-up would pre-compute compiler results at the workflow level.Fixed Issues
$ #85070
PROPOSAL:
Tests
.claude/skills/coding-standards/rules/clean-react-0-compiler.mdcheck-compiler.shexactly as shown above - by name only, without an absolute path orbashprefix. The script is already on$PATH.useMemo/useCallback/React.memocheck-compiler.shinvocations - verify the command uses the bare name (not an absolute path) and is no longer inpermission_denialsOffline tests
N/A - changes are to reviewer instructions only, no runtime code affected.
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
N/A - this is a CI/tooling change with no user-facing impact. Verification is that the reviewer agent can successfully run the compliance check on future PRs.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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