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

78
src/refine.mjs
View File

@@ -28,14 +28,14 @@ dotenv.config({path: '.env.local', override: true, quiet: true});
// Fetch issue
const issueRes = await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}`, {
headers: {'Authorization': `token ${auth}`}
});
if(!issueRes.ok) throw new Error(`${issueRes.status} ${await issueRes.text()}`);
}).then(async resp => !resp.ok ? throw new Error(`${updateRes.status} ${await updateRes.text()}`) : null);
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.
const issueData = await issueRes.json();
if(!issueData.labels?.some(l => l.name === 'Review/AI')) {
if(issueData.labels[0] !== 'Review/AI' || issueData.labels.length !== 1) {
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.
console.log('Skipping');
return process.exit();
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'`
}
// Gather readme & template
let title = '', type = '', readme = '', readmeP = path.join(process.cwd(), 'README.md');
if(fs.existsSync(readmeP)) readme = fs.readFileSync(readmeP, 'utf-8');
const template = p ? fs.readFileSync(p, 'utf-8') : `## Description
@@ -80,6 +80,7 @@ Implementation details, constraints, dependencies, design decisions
| **Total** | **0-15** |
`;
// Create AI
let options = {ollama: {model, host}};
if(host === 'anthropic') options = {anthropic: {model, token}};
else if(host === 'openai') options = {openAi: {model, token}};
@@ -141,34 +142,61 @@ ${template.trim()}
Output ONLY markdown. No explanations, labels, or extra formatting.`});
// Format ticket with AI
const messages = await ai.language.ask(`Title: ${issueData.title}\n\nDescription:\n${issueData.body || 'No description provided'}`).catch(() => []);
const body = messages?.pop()?.content;
if(!body) {
console.log('Invalid response from AI');
return process.exit(1);
if(!body) throw new Error('Invalid response from AI');
// Check for duplicates
ztimson marked this conversation as resolved
assistant commented 2025-12-30 23:43:54 -05:00
Review
Copy Link

Error handling change: Changed from graceful exit with exit code 1 to throwing an error. This is a behavioral change that could affect how the script is used in CI/CD pipelines. Ensure this is intentional and that calling code can handle the exception.

Error handling change: Changed from graceful exit with exit code 1 to throwing an error. This is a behavioral change that could affect how the script is used in CI/CD pipelines. Ensure this is intentional and that calling code can handle the exception.
const search = await fetch(`${git}/api/v1/repos/issues/search`, {
method: 'POST',
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},
ztimson marked this conversation as resolved Outdated
assistant commented 2025-12-30 23:43:54 -05:00
Outdated
Review
Copy Link

Potential bug: The search API endpoint ${git}/api/v1/repos/issues/search is missing the ${owner}/${repo} path segment. This should likely be ${git}/api/v1/repos/${owner}/${repo}/issues/search to search within the specific repository, otherwise it may search globally or fail.

Potential bug: The search API endpoint `${git}/api/v1/repos/issues/search` is missing the `${owner}/${repo}` path segment. This should likely be `${git}/api/v1/repos/${owner}/${repo}/issues/search` to search within the specific repository, otherwise it may search globally or fail.
assistant commented 2025-12-30 23:56:24 -05:00
Review
Copy Link

Critical bug: The search API endpoint is missing the repository path segment. This should be ${git}/api/v1/repos/${owner}/${repo}/issues/search instead of ${git}/api/v1/repos/issues/search. Without the proper path, the API call will likely fail or search globally instead of within the specific repository.

Critical bug: The search API endpoint is missing the repository path segment. This should be `${git}/api/v1/repos/${owner}/${repo}/issues/search` instead of `${git}/api/v1/repos/issues/search`. Without the proper path, the API call will likely fail or search globally instead of within the specific repository.
body: JSON.stringify({
priority_repo_id: repo,
type: 'issues',
limit: 3,
q: title
ztimson marked this conversation as resolved Outdated
assistant commented 2025-12-30 23:43:54 -05:00
Outdated
Review
Copy Link

Bug: The priority_repo_id parameter expects a repository ID (numeric), but repo variable contains the repository name (string). This will likely cause the search to fail or return incorrect results.

Bug: The `priority_repo_id` parameter expects a repository ID (numeric), but `repo` variable contains the repository name (string). This will likely cause the search to fail or return incorrect results.
assistant commented 2025-12-30 23:56:24 -05:00
Review
Copy Link

Bug: The priority_repo_id parameter expects a numeric repository ID, but repoInfo.id could be null if the fetch on line 152 failed (returns null when !resp.ok). This will cause the search to fail. Add proper error handling: if(!repoInfo) throw new Error('Failed to fetch repository info'); before using repoInfo.id.

Bug: The `priority_repo_id` parameter expects a numeric repository ID, but `repoInfo.id` could be null if the fetch on line 152 failed (returns null when !resp.ok). This will cause the search to fail. Add proper error handling: `if(!repoInfo) throw new Error('Failed to fetch repository info');` before using repoInfo.id.
})
}).then(resp => resp.ok ? resp.json() : []);
let dupeId = null;
const dupeIds = search.map(t => t.id);
ztimson marked this conversation as resolved
assistant commented 2025-12-30 23:43:54 -05:00
Review
Copy Link

Error handling issue: If the search API returns an error, the code defaults to an empty array []. This silently fails and skips duplicate detection. Consider logging a warning or handling the error more explicitly to avoid missing duplicates due to API failures.

Error handling issue: If the search API returns an error, the code defaults to an empty array `[]`. This silently fails and skips duplicate detection. Consider logging a warning or handling the error more explicitly to avoid missing duplicates due to API failures.
assistant commented 2025-12-30 23:43:54 -05:00
Review
Copy Link

Type safety issue: The fallback [] when resp.ok is false means search could be an empty array or the actual response data. This inconsistency could cause issues if the API response structure is unexpected. Consider explicitly handling the error case.

Type safety issue: The fallback `[]` when `resp.ok` is false means `search` could be an empty array or the actual response data. This inconsistency could cause issues if the API response structure is unexpected. Consider explicitly handling the error case.
assistant commented 2025-12-30 23:56:24 -05:00
Review
Copy Link

Race condition: The duplicate search uses the title variable which is set by the AI tool call. If the AI hasn't called the title tool yet at this point in execution, title will be an empty string, causing the search query to be empty or invalid. Consider moving the duplicate check after ensuring the AI has completed and set the title, or add validation: if(!title) throw new Error('AI failed to set title');

Race condition: The duplicate search uses the `title` variable which is set by the AI tool call. If the AI hasn't called the `title` tool yet at this point in execution, `title` will be an empty string, causing the search query to be empty or invalid. Consider moving the duplicate check after ensuring the AI has completed and set the title, or add validation: `if(!title) throw new Error('AI failed to set title');`
const dupes = search.map(t => `ID: ${t.id}\nTitle: ${t.title}\n\`\`\`markdown\n${t.body}\n\`\`\``).join('\n\n');
const hasDuplicates = (await ai.language.ask(`${title}\n\`\`\`markdown\n${body}\n\`\`\``, {
ztimson marked this conversation as resolved
assistant commented 2025-12-30 23:43:54 -05:00
Review
Copy Link

Potential null reference: If search is an empty array (from error or no results), search.map() will work but dupes will be empty. The AI duplicate check on line 166 will then have no context to compare against, making the duplicate detection ineffective when it fails silently.

Potential null reference: If `search` is an empty array (from error or no results), `search.map()` will work but `dupes` will be empty. The AI duplicate check on line 166 will then have no context to compare against, making the duplicate detection ineffective when it fails silently.
assistant commented 2025-12-30 23:56:24 -05:00
Review
Copy Link

Silent failure risk: When the search API fails (line 153 returns []), the dupes variable becomes an empty string, and the AI duplicate check has no context to compare against. The duplicate detection becomes ineffective but continues silently. Consider logging a warning or throwing an error when search fails to ensure duplicate detection is working properly.

Silent failure risk: When the search API fails (line 153 returns []), the `dupes` variable becomes an empty string, and the AI duplicate check has no context to compare against. The duplicate detection becomes ineffective but continues silently. Consider logging a warning or throwing an error when search fails to ensure duplicate detection is working properly.
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

Race condition risk: The duplicate check uses title which is set by the AI tool call. If the AI hasn't called the title tool yet or it's still empty, the search query will be empty or invalid, potentially returning incorrect results or all issues.

Race condition risk: The duplicate check uses `title` which is set by the AI tool call. If the AI hasn't called the `title` tool yet or it's still empty, the search query will be empty or invalid, potentially returning incorrect results or all issues.
// Handle duplicates
if(!!hasDuplicates && (dupeId = dupeIds.find(id => hasDuplicates.includes(id.toString()))) != null) {
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

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.
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},
body: `{"body": "Duplicate of #${dupeId}"}`
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.
}).then(async resp => !resp.ok ? throw new Error(`${updateRes.status} ${await updateRes.text()}`) : null);
await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}/labels`, {
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}\`})`
method: 'POST',
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},
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.
body: '{"labels":["Reviewed/Duplicate"]}'
}).then(async resp => !resp.ok ? throw new Error(`${updateRes.status} ${await updateRes.text()}`) : null);
await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}`, {
method: 'PATCH',
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},
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.
body: '{"state": "closed"}'
}).then(async resp => !resp.ok ? throw new Error(`${updateRes.status} ${await updateRes.text()}`) : null);
console.log('Duplicate');
return process.exit();
}
// Update ticket
const updateRes = await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}`, {
ztimson marked this conversation as resolved Outdated
assistant commented 2025-12-30 23:35:49 -05:00
Outdated
Review
Copy Link

Same bugs: updateRes is used in its own error handler before being assigned, and invalid throw syntax in ternary expression. This creates a circular reference issue.

Same bugs: `updateRes` is used in its own error handler before being assigned, and invalid throw syntax in ternary expression. This creates a circular reference issue.
assistant commented 2025-12-30 23:43:54 -05:00
Review
Copy Link

Unused variable: updateRes is assigned but never used. The .then() chain doesn't return the response, so updateRes will be a Promise that resolves to undefined. If you need the response, remove the .then() or return the response from it.

Unused variable: `updateRes` is assigned but never used. The `.then()` chain doesn't return the response, so `updateRes` will be a Promise that resolves to undefined. If you need the response, remove the `.then()` or return the response from it.
method: 'PATCH',
headers: {
'Authorization': `token ${auth}`,
'Content-Type': 'application/json'
},
body: JSON.stringify({
title,
body,
})
});
if(!updateRes.ok) throw new Error(`${updateRes.status} ${await updateRes.text()}`);
if(type) {
const resp = await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}/labels`, {
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},
body: JSON.stringify({title, body})
}).then(async resp => !resp.ok ? throw new Error(`${updateRes.status} ${await updateRes.text()}`) : null);
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.
if(type) { // Label
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.
await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}/labels`, {
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.
method: 'POST',
headers: {
'Authorization': `token ${auth}`,
'Content-Type': 'application/json'
},
body: JSON.stringify({labels: [`Kind/${type[0].toUpperCase() + type.slice(1).toLowerCase()}`]})
});
if(!resp.ok) throw new Error(`${resp.status} ${await resp.text()}`);
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},
body: `{"labels":["Reviewed/${type[0].toUpperCase() + type.slice(1).toLowerCase()}"]}`
}).then(async resp => !resp.ok ? throw new Error(`${updateRes.status} ${await updateRes.text()}`) : null);
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.
}
console.log(`Title: ${title}\nType: ${type}\nBody:\n${body}`);