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 +7 -4
1 changed files with 7 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 7447204351 - Show all commits

11
src/review.mjs
View File

@@ -27,8 +27,11 @@ dotenv.config({path: '.env.local', override: true, quiet: true, debug: false});
console.log(`Reviewing: ${root}\n`);
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()}`); });
if(info.labels?.length > 0 && !info.labels.some(l => l.name === labelEnabled)) {
.then(async resp => {
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(resp.ok) return resp.json();
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.
throw new Error(`${resp.status} ${await resp.text()}`);
});
if(!info.labels.some(l => l.name === labelEnabled)) {
console.log('Skipping');
return process.exit();
}
@@ -95,10 +98,10 @@ dotenv.config({path: '.env.local', override: true, quiet: true, debug: false});
}]
ztimson marked this conversation as resolved Outdated
assistant commented 2026-01-14 13:52:35 -05:00
Outdated
Review
Copy Link

Potential null/undefined error: If info.title or info.body is null/undefined, this will insert "null" or "undefined" as text in the prompt. Add fallback values: Title: ${info.title || 'No title'} and ${info.body || 'No description'}.

Potential null/undefined error: If `info.title` or `info.body` is null/undefined, this will insert "null" or "undefined" as text in the prompt. Add fallback values: `Title: ${info.title || 'No title'}` and `${info.body || 'No description'}`.
});
const messages = await ai.language.ask(`Title: ${info.title}
const messages = await ai.language.ask(`Title: ${info.title || 'None'}
Description:
\`\`\`md
${info.body}
${info.body || 'None'}
\`\`\`
Git Diff: