Conversation
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 22 minutes Limit details: You’ve used the included review currently available. Your 62 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
Comment |
| return [DST_ICON, DST_IOS_ICON, DST_ADAPTIVE_ICON].every((dst) => { | ||
| const file = path.join(projectRoot, dst); | ||
| return !fs.existsSync(file) || fs.statSync(file).size > 0; |
There was a problem hiding this comment.
Stale file predicate in isUpToDate in appIconBadge.plugin.js marks missing generated icons as valid because !fs.existsSync(file) returns true. Require every destination for the current jobs to exist and have fs.statSync(file).size > 0 before returning true, or .expo/app-icon-badge/*.png deletion after a successful run can leave config.icon, ios.icon, and adaptiveIcon.foregroundImage pointing to non-existent files and break the next Expo icon mods prebuild.
return jobs.every(({ dstPath }) => {
const file = path.join(projectRoot, dstPath);
return fs.existsSync(file) && fs.statSync(file).size > 0;
});Prompt for LLM
File appIconBadge.plugin.js:
Line 40 to 42:
Stale file predicate in `isUpToDate` in `appIconBadge.plugin.js` marks missing generated icons as valid because `!fs.existsSync(file)` returns true. Require every destination for the current `jobs` to exist and have `fs.statSync(file).size > 0` before returning true, or `.expo/app-icon-badge/*.png` deletion after a successful run can leave `config.icon`, `ios.icon`, and `adaptiveIcon.foregroundImage` pointing to non-existent files and break the next Expo icon mods prebuild.
Suggested Code:
return jobs.every(({ dstPath }) => {
const file = path.join(projectRoot, dstPath);
return fs.existsSync(file) && fs.statSync(file).size > 0;
});
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| }); | ||
| }; | ||
|
|
||
| const withAppIconBadge = (config, options = {}) => { |
There was a problem hiding this comment.
Invalid rule match in appIconBadge.plugin.js: the reported .bind() or inline arrow function usage in JSX props does not apply to const withAppIconBadge = (config, options = {}) => { because this file context does not show JSX props. Remove this suggestion unless a specific JSX prop location is identified.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File appIconBadge.plugin.js:
Line 46:
Invalid rule match in `appIconBadge.plugin.js`: the reported `.bind()` or inline arrow function usage in JSX props does not apply to `const withAppIconBadge = (config, options = {}) => {` because this file context does not show JSX props. Remove this suggestion unless a specific JSX prop location is identified.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| dstPath: path.join(projectRoot, job.dstPath), | ||
| })); | ||
|
|
||
| execFileSync(process.execPath, [path.join(projectRoot, 'scripts', 'generate-icon-badges.js'), JSON.stringify(absoluteJobs)], { |
There was a problem hiding this comment.
Unhandled external process failure in appIconBadge.plugin.js leaves execFileSync(process.execPath, [path.join(projectRoot, 'scripts', 'generate-icon-badges.js'), JSON.stringify(absoluteJobs)], { ... }) without contextual diagnostics when scripts are missing, paths are invalid, or the child exits non-zero. Wrap the call in try/catch and rethrow with projectRoot context, as also required at patches/app-icon-badge+0.1.2.patch:10-10, patches/app-icon-badge+0.1.2.patch:23-23, patches/app-icon-badge+0.1.2.patch:36-36, and scripts/generate-icon-badges.js:17-17.
Kody rule violation: Add try-catch blocks for external calls
try {
execFileSync(process.execPath, [path.join(projectRoot, 'scripts', 'generate-icon-badges.js'), JSON.stringify(absoluteJobs)], {
cwd: projectRoot,
stdio: 'inherit',
});
} catch (err) {
throw new Error(`generate-icon-badges failed for projectRoot=${projectRoot}: ${err instanceof Error ? err.message : String(err)}`);
}Prompt for LLM
File appIconBadge.plugin.js:
Line 91:
Unhandled external process failure in `appIconBadge.plugin.js` leaves `execFileSync(process.execPath, [path.join(projectRoot, 'scripts', 'generate-icon-badges.js'), JSON.stringify(absoluteJobs)], { ... })` without contextual diagnostics when scripts are missing, paths are invalid, or the child exits non-zero. Wrap the call in `try/catch` and rethrow with `projectRoot` context, as also required at `patches/app-icon-badge+0.1.2.patch:10-10`, `patches/app-icon-badge+0.1.2.patch:23-23`, `patches/app-icon-badge+0.1.2.patch:36-36`, and `scripts/generate-icon-badges.js:17-17`.
Suggested Code:
try {
execFileSync(process.execPath, [path.join(projectRoot, 'scripts', 'generate-icon-badges.js'), JSON.stringify(absoluteJobs)], {
cwd: projectRoot,
stdio: 'inherit',
});
} catch (err) {
throw new Error(`generate-icon-badges failed for projectRoot=${projectRoot}: ${err instanceof Error ? err.message : String(err)}`);
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| dstPath: path.join(projectRoot, job.dstPath), | ||
| })); | ||
|
|
||
| execFileSync(process.execPath, [path.join(projectRoot, 'scripts', 'generate-icon-badges.js'), JSON.stringify(absoluteJobs)], { |
There was a problem hiding this comment.
Mutable path risk in appIconBadge.plugin.js: path.join(projectRoot, 'scripts', 'generate-icon-badges.js') derives an executable target from projectRoot without validation. Constrain scriptPath to the expected allowlisted path.join(projectRoot, 'scripts') location before passing it to execFileSync, even though the call uses argv rather than a shell string.
Kody rule violation: Avoid building system commands from user input
const scriptPath = path.join(projectRoot, 'scripts', 'generate-icon-badges.js');
if (!scriptPath.startsWith(path.join(projectRoot, 'scripts'))) {
throw new Error('Invalid script path');
}
execFileSync(process.execPath, [scriptPath, JSON.stringify(absoluteJobs)], {
cwd: projectRoot,
stdio: 'inherit',
});Prompt for LLM
File appIconBadge.plugin.js:
Line 91:
Mutable path risk in `appIconBadge.plugin.js`: `path.join(projectRoot, 'scripts', 'generate-icon-badges.js')` derives an executable target from `projectRoot` without validation. Constrain `scriptPath` to the expected allowlisted `path.join(projectRoot, 'scripts')` location before passing it to `execFileSync`, even though the call uses argv rather than a shell string.
Suggested Code:
const scriptPath = path.join(projectRoot, 'scripts', 'generate-icon-badges.js');
if (!scriptPath.startsWith(path.join(projectRoot, 'scripts'))) {
throw new Error('Invalid script path');
}
execFileSync(process.execPath, [scriptPath, JSON.stringify(absoluteJobs)], {
cwd: projectRoot,
stdio: 'inherit',
});
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| icon | ||
| }); | ||
| - resultImage.writeAsync(resultFilename); | ||
| + yield resultImage.writeAsync(resultFilename); |
There was a problem hiding this comment.
Unhandled promise rejection in patches/app-icon-badge+0.1.2.patch leaves yield resultImage.writeAsync(resultFilename); without local failure diagnostics. Guard writeAsync with try/catch, log structured context including op: 'addBadge.writeAsync' and resultFilename, and rethrow, including at patches/app-icon-badge+0.1.2.patch:23-23 and patches/app-icon-badge+0.1.2.patch:36-36.
Kody rule violation: Handle async operations with proper error handling
try {
yield resultImage.writeAsync(resultFilename);
} catch (err) {
logger.error('writeAsync failed', { op: 'addBadge.writeAsync', resultFilename, err });
throw err;
}Prompt for LLM
File patches/app-icon-badge+0.1.2.patch:
Line 10:
Unhandled promise rejection in `patches/app-icon-badge+0.1.2.patch` leaves `yield resultImage.writeAsync(resultFilename);` without local failure diagnostics. Guard `writeAsync` with `try/catch`, log structured context including `op: 'addBadge.writeAsync'` and `resultFilename`, and rethrow, including at `patches/app-icon-badge+0.1.2.patch:23-23` and `patches/app-icon-badge+0.1.2.patch:36-36`.
Suggested Code:
try {
yield resultImage.writeAsync(resultFilename);
} catch (err) {
logger.error('writeAsync failed', { op: 'addBadge.writeAsync', resultFilename, err });
throw err;
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| }; | ||
|
|
||
| main().catch((error) => { | ||
| console.error(error); |
There was a problem hiding this comment.
Insufficient error context in scripts/generate-icon-badges.js because console.error(error); logs only the raw error object. Log structured metadata including the operation name and process.argv?.[2] so generate-icon-badges failures can be traced and diagnosed.
Kody rule violation: Include error context in structured logs
console.error('generate-icon-badges failed', {
op: 'generate-icon-badges',
inputPath: process.argv?.[2],
error,
});Prompt for LLM
File scripts/generate-icon-badges.js:
Line 27:
Insufficient error context in `scripts/generate-icon-badges.js` because `console.error(error);` logs only the raw error object. Log structured metadata including the operation name and `process.argv?.[2]` so `generate-icon-badges` failures can be traced and diagnosed.
Suggested Code:
console.error('generate-icon-badges failed', {
op: 'generate-icon-badges',
inputPath: process.argv?.[2],
error,
});
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
2 participants
Summary
This PR fixes the app icon badge build flow by replacing the direct
app-icon-badgeExpo plugin usage with a local plugin that generates badged icons before Expo prebuild consumes them.What changed
app.config.tsto use a localappIconBadge.plugin.jsinstead of the package-providedapp-icon-badgeplugin..expo/app-icon-badgescripts/generate-icon-badges.jshelper script to create the badged icon assets and validate that the output files were written correctly.app-icon-badgeso badge image writes are properly awaited before returning.Functional impact
This change prevents build/prebuild failures on clean environments such as EAS where Expo may try to read generated badged icon files before they exist or before they are fully written. It ensures the required icon assets are ready and non-empty before Expo continues processing them.