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

20
src/refine.mjs
View File

@@ -26,11 +26,13 @@ dotenv.config({path: '.env.local', override: true, quiet: true});
console.log(`Processing issue #${ticket}`);
// Fetch issue
const issueRes = await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}`, {
const issueData = await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}`, {
headers: {'Authorization': `token ${auth}`}
}).then(async resp => !resp.ok ? throw new Error(`${updateRes.status} ${await updateRes.text()}`) : null);
const issueData = await issueRes.json();
if(issueData.labels[0] !== 'Review/AI' || issueData.labels.length !== 1) {
}).then(async resp => {
ztimson marked this conversation as resolved Outdated
assistant commented 2025-12-30 23:35:49 -05:00
Outdated
Review
Copy Link

Critical bug: updateRes is referenced before it's defined. This variable is only defined later on line 189. The error message should reference resp.status and resp.text() instead of updateRes.status and updateRes.text().

Critical bug: `updateRes` is referenced before it's defined. This variable is only defined later on line 189. The error message should reference `resp.status` and `resp.text()` instead of `updateRes.status` and `updateRes.text()`.
assistant commented 2025-12-30 23:35:49 -05:00
Outdated
Review
Copy Link

Syntax error: Cannot use throw in a ternary expression without wrapping it in a function or using proper syntax. Should be throw new Error(...) wrapped in parentheses or use a different approach like .then(async resp => { if (!resp.ok) throw new Error(...); return resp; }).

Syntax error: Cannot use `throw` in a ternary expression without wrapping it in a function or using proper syntax. Should be `throw new Error(...)` wrapped in parentheses or use a different approach like `.then(async resp => { if (!resp.ok) throw new Error(...); return resp; })`.
assistant commented 2025-12-30 23:35:49 -05:00
Outdated
Review
Copy Link

Logic error: After throwing an error or returning null, the code tries to call issueRes.json() on line 32. If the response is not ok, issueRes will be null, causing a runtime error when trying to call .json() on it.

Logic error: After throwing an error or returning null, the code tries to call `issueRes.json()` on line 32. If the response is not ok, `issueRes` will be null, causing a runtime error when trying to call `.json()` on it.
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.length !== 1 || issueData.labels?.[0] !== '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();
}
@@ -170,17 +172,17 @@ Output ONLY markdown. No explanations, labels, or extra formatting.`});
method: 'POST',
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.
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},
body: `{"body": "Duplicate of #${dupeId}"}`
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}\`})`
}).then(async resp => !resp.ok ? throw new Error(`${updateRes.status} ${await updateRes.text()}`) : null);
}).then(async resp => { if(!resp.ok) throw new Error(`${resp.status} ${await resp.text()}`); });
await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}/labels`, {
ztimson marked this conversation as resolved
assistant commented 2025-12-30 23:35:49 -05:00
Review
Copy Link

Same bugs as line 172: updateRes referenced before definition and invalid throw syntax in ternary expression.

Same bugs as line 172: `updateRes` referenced before definition and invalid throw syntax in ternary expression.
method: 'POST',
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},
body: '{"labels":["Reviewed/Duplicate"]}'
}).then(async resp => !resp.ok ? throw new Error(`${updateRes.status} ${await updateRes.text()}`) : null);
}).then(async resp => { if(!resp.ok) throw new Error(`${resp.status} ${await resp.text()}`); });
await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}`, {
ztimson marked this conversation as resolved
assistant commented 2025-12-30 23:35:49 -05:00
Review
Copy Link

Same bugs as line 172: updateRes referenced before definition and invalid throw syntax in ternary expression.

Same bugs as line 172: `updateRes` referenced before definition and invalid throw syntax in ternary expression.
method: 'PATCH',
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},
body: '{"state": "closed"}'
}).then(async resp => !resp.ok ? throw new Error(`${updateRes.status} ${await updateRes.text()}`) : null);
}).then(async resp => { if(!resp.ok) throw new Error(`${resp.status} ${await resp.text()}`); });
console.log('Duplicate');
return process.exit();
}
@@ -190,13 +192,13 @@ Output ONLY markdown. No explanations, labels, or extra formatting.`});
method: 'PATCH',
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},
ztimson marked this conversation as resolved Outdated
assistant commented 2025-12-30 23:35:49 -05:00
Outdated
Review
Copy Link

The label name changed from Kind/${type} to Reviewed/${type} without explanation. This could break existing label filtering logic. Verify this is intentional and consistent with the system's labeling scheme.

The label name changed from `Kind/${type}` to `Reviewed/${type}` without explanation. This could break existing label filtering logic. Verify this is intentional and consistent with the system's labeling scheme.
body: JSON.stringify({title, body})
ztimson marked this conversation as resolved Outdated
assistant commented 2025-12-30 23:35:49 -05:00
Outdated
Review
Copy Link

Same bugs as above: updateRes referenced in error handler instead of resp, and invalid throw syntax.

Same bugs as above: `updateRes` referenced in error handler instead of `resp`, and invalid throw syntax.
}).then(async resp => !resp.ok ? throw new Error(`${updateRes.status} ${await updateRes.text()}`) : null);
}).then(async resp => { if(!resp.ok) throw new Error(`${resp.status} ${await resp.text()}`); });
ztimson marked this conversation as resolved Outdated
assistant commented 2025-12-30 23:43:54 -05:00
Outdated
Review
Copy Link

Missing error handling: If type is an empty string or undefined, the type[0].toUpperCase() will throw a runtime error. Add a check to ensure type is valid before attempting to transform it.

Missing error handling: If `type` is an empty string or undefined, the `type[0].toUpperCase()` will throw a runtime error. Add a check to ensure `type` is valid before attempting to transform it.
if(type) { // Label
await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}/labels`, {
method: 'POST',
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},
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 with template literal instead of using JSON.stringify(). This is error-prone and could break if type contains special characters. Should be body: JSON.stringify({labels: [\Reviewed/${type[0].toUpperCase() + type.slice(1).toLowerCase()}`]})`.

Bug: JSON string is manually constructed with template literal instead of using `JSON.stringify()`. This is error-prone and could break if `type` contains special characters. Should be `body: JSON.stringify({labels: [\`Reviewed/${type[0].toUpperCase() + type.slice(1).toLowerCase()}\`]})`.
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. If type contains special characters, this could produce invalid JSON. While type is constrained by the enum in the tool definition, it's still better practice to use JSON.stringify(): body: JSON.stringify({labels: [\Reviewed/${type[0].toUpperCase() + type.slice(1).toLowerCase()}`]})`

JSON injection vulnerability: The JSON body is manually constructed using a template literal. If `type` contains special characters, this could produce invalid JSON. While `type` is constrained by the enum in the tool definition, it's still better practice to use JSON.stringify(): `body: JSON.stringify({labels: [\`Reviewed/${type[0].toUpperCase() + type.slice(1).toLowerCase()}\`]})`
assistant commented 2025-12-30 23:56:24 -05:00
Review
Copy Link

Potential runtime error: If type is an empty string or undefined (e.g., if the AI fails to call the type tool), then type[0].toUpperCase() will throw a TypeError. Add validation before this line: if(!type) throw new Error('AI failed to set type');

Potential runtime error: If `type` is an empty string or undefined (e.g., if the AI fails to call the type tool), then `type[0].toUpperCase()` will throw a TypeError. Add validation before this line: `if(!type) throw new Error('AI failed to set type');`
assistant commented 2025-12-30 23:56:24 -05:00
Review
Copy Link

Breaking change: Label prefix changed from Kind/${type} to Reviewed/${type} without explanation. This could break existing workflows, filters, or automation that rely on the Kind/ label prefix. Verify this is intentional and update any dependent systems accordingly.

Breaking change: Label prefix changed from `Kind/${type}` to `Reviewed/${type}` without explanation. This could break existing workflows, filters, or automation that rely on the `Kind/` label prefix. Verify this is intentional and update any dependent systems accordingly.
body: `{"labels":["Reviewed/${type[0].toUpperCase() + type.slice(1).toLowerCase()}"]}`
}).then(async resp => !resp.ok ? throw new Error(`${updateRes.status} ${await updateRes.text()}`) : null);
}).then(async resp => { if(!resp.ok) throw new Error(`${resp.status} ${await resp.text()}`); });
}
console.log(`Title: ${title}\nType: ${type}\nBody:\n${body}`);