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

Check for duplicates before adding tickets #12

Merged
ztimson merged 6 commits from ticket-duplicates into master 2025-12-31 00:02:18 -05:00
Conversation 5 Commits 6 Files Changed 3 +2 -2
1 changed files with 2 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 23cb66544e - Show all commits

4
src/refine.mjs
View File

@@ -32,7 +32,7 @@ dotenv.config({path: '.env.local', override: true, quiet: true});
if(resp.ok) return resp.json();
else throw new Error(`${resp.status} ${await resp.text()}`);
ztimson marked this conversation as resolved Outdated
assistant commented 2025-12-30 23:35:49 -05:00
Outdated
Review
Copy Link

Bug: Unsafe array access. The code assumes issueData.labels[0] exists and is a string, but labels is an array of objects (with properties like name). The original code correctly checked issueData.labels?.some(l => l.name === 'Review/AI'). This will always fail since labels[0] is an object, not a string.

Bug: Unsafe array access. The code assumes `issueData.labels[0]` exists and is a string, but labels is an array of objects (with properties like `name`). The original code correctly checked `issueData.labels?.some(l => l.name === 'Review/AI')`. This will always fail since `labels[0]` is an object, not a string.
});
if(issueData.labels?.[0]?.name !== 1 || issueData.labels?.[0] !== 'Review/AI') {
if(issueData.labels?.[0] !== 1 || issueData.labels?.[0]?.name !== 'Review/AI') {
ztimson marked this conversation as resolved Outdated
assistant commented 2025-12-30 23:43:54 -05:00
Outdated
Review
Copy Link

Logic error: The check issueData.labels?.[0] !== 'Review/AI' compares a label object to a string. Labels are objects with properties like name, not strings. This should be issueData.labels?.[0]?.name !== 'Review/AI' to correctly access the label's name property.

Logic error: The check `issueData.labels?.[0] !== 'Review/AI'` compares a label object to a string. Labels are objects with properties like `name`, not strings. This should be `issueData.labels?.[0]?.name !== 'Review/AI'` to correctly access the label's name property.
assistant commented 2025-12-30 23:43:54 -05:00
Outdated
Review
Copy Link

Logic error: The condition checks if labels.length !== 1 which will skip processing for issues with 0 or 2+ labels. However, the original code used .some() which would process issues that have the 'Review/AI' label among other labels. This change makes the filter more restrictive and may break existing workflows where issues have multiple labels.

Logic error: The condition checks if `labels.length !== 1` which will skip processing for issues with 0 or 2+ labels. However, the original code used `.some()` which would process issues that have the 'Review/AI' label among other labels. This change makes the filter more restrictive and may break existing workflows where issues have multiple labels.
assistant commented 2025-12-30 23:56:24 -05:00
Outdated
Review
Copy Link

Critical logic error: The condition issueData.labels?.[0]?.name !== 1 compares a label name (string) to the number 1, which will always be true. This appears to be a typo and should likely be checking the length: issueData.labels?.length !== 1. Additionally, the second part issueData.labels?.[0] !== 'Review/AI' compares a label object to a string, which will always be true. The correct check should be: issueData.labels?.length !== 1 || issueData.labels?.[0]?.name !== 'Review/AI'

Critical logic error: The condition `issueData.labels?.[0]?.name !== 1` compares a label name (string) to the number 1, which will always be true. This appears to be a typo and should likely be checking the length: `issueData.labels?.length !== 1`. Additionally, the second part `issueData.labels?.[0] !== 'Review/AI'` compares a label object to a string, which will always be true. The correct check should be: `issueData.labels?.length !== 1 || issueData.labels?.[0]?.name !== 'Review/AI'`
console.log('Skipping');
return process.exit();
}
@@ -169,7 +169,7 @@ Output ONLY markdown. No explanations, labels, or extra formatting.`});
system: `Your job is to identify duplicates. Respond with the ID number of the duplicate or nothing if there are no matches \n\n${dupes}`
}))?.pop()?.content;
ztimson marked this conversation as resolved
assistant commented 2025-12-30 23:43:54 -05:00
Review
Copy Link

Logic issue: The duplicate detection uses hasDuplicates.includes(id.toString()) which is a substring match. This could cause false positives (e.g., ID 123 would match content containing "1234"). Consider using a more precise matching strategy or parsing the AI response more carefully.

Logic issue: The duplicate detection uses `hasDuplicates.includes(id.toString())` which is a substring match. This could cause false positives (e.g., ID 123 would match content containing "1234"). Consider using a more precise matching strategy or parsing the AI response more carefully.
assistant commented 2025-12-30 23:56:24 -05:00
Review
Copy Link

Logic issue: The regex pattern (^| )${id}( |$) performs substring matching which could cause false positives. For example, if the AI response contains "123" it would match issue ID 12 or 23. Consider using word boundaries or more precise parsing of the AI response to extract the exact ID number.

Logic issue: The regex pattern `(^| )${id}( |$)` performs substring matching which could cause false positives. For example, if the AI response contains "123" it would match issue ID 12 or 23. Consider using word boundaries or more precise parsing of the AI response to extract the exact ID number.
// Handle duplicates
if(!!hasDuplicates && (dupeId = dupeIds.find(id => new RegExp(`(^| )${id}( |$)`, 'm').test(hasDuplicates)))) {
if(!!hasDuplicates && (dupeId = dupeIds.find(id => new RegExp(`\\b${id}\\b`, 'm').test(hasDuplicates)))) {
ztimson marked this conversation as resolved Outdated
assistant commented 2025-12-30 23:35:49 -05:00
Outdated
Review
Copy Link

Same critical bug as line 31: updateRes is referenced before it's defined. Should reference resp.status and resp.text() instead.

Same critical bug as line 31: `updateRes` is referenced before it's defined. Should reference `resp.status` and `resp.text()` instead.
assistant commented 2025-12-30 23:35:49 -05:00
Outdated
Review
Copy Link

Same syntax error as line 31: Cannot use throw in a ternary expression without proper syntax.

Same syntax error as line 31: Cannot use `throw` in a ternary expression without proper syntax.
await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}/comments`, {
method: 'POST',
ztimson marked this conversation as resolved
assistant commented 2025-12-30 23:43:54 -05:00
Review
Copy Link

Bug: JSON string is manually constructed instead of using JSON.stringify(). The body should be body: JSON.stringify({body: \Duplicate of #${dupeId}`})` to properly escape special characters and prevent JSON injection vulnerabilities.

Bug: JSON string is manually constructed instead of using `JSON.stringify()`. The body should be `body: JSON.stringify({body: \`Duplicate of #${dupeId}\`})` to properly escape special characters and prevent JSON injection vulnerabilities.
assistant commented 2025-12-30 23:56:24 -05:00
Review
Copy Link

JSON injection vulnerability: The JSON body is manually constructed using a template literal instead of JSON.stringify(). If dupeId contains special characters like quotes or backslashes, this will produce invalid JSON or allow injection. Change to: body: JSON.stringify({body: \Duplicate of #${dupeId}`})`

JSON injection vulnerability: The JSON body is manually constructed using a template literal instead of JSON.stringify(). If `dupeId` contains special characters like quotes or backslashes, this will produce invalid JSON or allow injection. Change to: `body: JSON.stringify({body: \`Duplicate of #${dupeId}\`})`
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},