ztimson/ai-agents
1
0
Fork 0
generated from ztimson/template
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

review-labels #14

Merged
ztimson merged 5 commits from review-labels into master 2026-01-14 14:06:21 -05:00
Conversation 2 Commits 5 Files Changed 6 +1 -1
1 changed files with 1 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 3b01e1bfc1 - Show all commits

2
src/review.mjs
View File

@@ -28,7 +28,7 @@ dotenv.config({path: '.env.local', override: true, quiet: true, debug: false});
console.log(`Reviewing: ${root}\n`); console.log(`Reviewing: ${root}\n`);
const info = await fetch(`${git}/api/v1/repos/${owner}/${repo}/pulls/${pr}`) const info = await fetch(`${git}/api/v1/repos/${owner}/${repo}/pulls/${pr}`)
ztimson marked this conversation as resolved
assistant commented 2026-01-14 13:47:40 -05:00
Review
Copy Link

Missing error handling: The fetch call should handle errors. If the API call fails, info will be undefined and line 30 will throw an error when trying to access info.labels.

Missing error handling: The fetch call should handle errors. If the API call fails, `info` will be undefined and line 30 will throw an error when trying to access `info.labels`.
assistant commented 2026-01-14 13:52:35 -05:00
Review
Copy Link

Missing error handling: The fetch call should handle network errors with a .catch() handler. If the API call fails due to network issues, the promise will reject and crash the script before reaching line 30.

Missing error handling: The fetch call should handle network errors with a .catch() handler. If the API call fails due to network issues, the promise will reject and crash the script before reaching line 30.
.then(async resp => { return resp.ok ? resp.json() : throw new Error(`${resp.status} ${await resp.text()}`); }); .then(async resp => { return resp.ok ? resp.json() : throw new Error(`${resp.status} ${await resp.text()}`); });
ztimson marked this conversation as resolved Outdated
assistant commented 2026-01-14 13:47:40 -05:00
Outdated
Review
Copy Link

Logic error: The condition info.labels?.length > 0 || !info.labels.some(l => l.name === labelEnabled) will always be true when labels exist. This should use && instead of ||. Currently, if labels.length > 0, it skips even if the enabled label is present. The correct logic should be: !info.labels?.some(l => l.name === labelEnabled) or info.labels?.length === 0 || !info.labels.some(l => l.name === labelEnabled)

Logic error: The condition `info.labels?.length > 0 || !info.labels.some(l => l.name === labelEnabled)` will always be true when labels exist. This should use `&&` instead of `||`. Currently, if labels.length > 0, it skips even if the enabled label is present. The correct logic should be: `!info.labels?.some(l => l.name === labelEnabled)` or `info.labels?.length === 0 || !info.labels.some(l => l.name === labelEnabled)`
assistant commented 2026-01-14 13:52:35 -05:00
Outdated
Review
Copy Link

Syntax error: throw new Error(...) cannot be used directly in a ternary expression. This should be wrapped in a block: resp.ok ? resp.json() : (() => { throw new Error(...); })() or use an if-else statement instead.

Syntax error: `throw new Error(...)` cannot be used directly in a ternary expression. This should be wrapped in a block: `resp.ok ? resp.json() : (() => { throw new Error(...); })()` or use an if-else statement instead.
if(info.labels?.length > 0 || !info.labels.some(l => l.name === labelEnabled)) { if(info.labels?.length > 0 && !info.labels.some(l => l.name === labelEnabled)) {
ztimson marked this conversation as resolved Outdated
assistant commented 2026-01-14 13:52:35 -05:00
Outdated
Review
Copy Link

Logic error: The condition info.labels?.length > 0 && !info.labels.some(l => l.name === labelEnabled) will skip the review when labels exist but the enabled label is NOT present. This appears backwards - it should run when the label IS present. Consider: if(!info.labels?.some(l => l.name === labelEnabled)) to skip when the label is missing.

Logic error: The condition `info.labels?.length > 0 && !info.labels.some(l => l.name === labelEnabled)` will skip the review when labels exist but the enabled label is NOT present. This appears backwards - it should run when the label IS present. Consider: `if(!info.labels?.some(l => l.name === labelEnabled))` to skip when the label is missing.
console.log('Skipping'); console.log('Skipping');
return process.exit(); return process.exit();
} }