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
3 changed files with 68 additions and 30 deletions

View File

@@ -1,6 +1,6 @@
{
"name": "@ztimson/ai-agents",
"version": "0.0.8",
"version": "0.1.0",
"description": "AI agents",
"keywords": ["ai", "review"],
"author": "ztimson",

View File

@@ -26,16 +26,18 @@ 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 => {
if(resp.ok) return resp.json();
else throw new Error(`${resp.status} ${await resp.text()}`);
});
if(!issueRes.ok) throw new Error(`${issueRes.status} ${await issueRes.text()}`);
const issueData = await issueRes.json();
if(!issueData.labels?.some(l => l.name === 'Review/AI')) {
if(issueData.labels?.[0] !== 1 || issueData.labels?.[0]?.name !== 'Review/AI') {
console.log('Skipping');
return process.exit();
}
// 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 +82,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,35 +144,67 @@ ${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);
}
const updateRes = await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}`, {
method: 'PATCH',
headers: {
'Authorization': `token ${auth}`,
'Content-Type': 'application/json'
},
if(!body) throw new Error('Invalid response from AI');
ztimson marked this conversation as resolved
Review

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.
// Check for duplicates
const repoInfo = await fetch(`${git}/api/v1/repos/${owner}/${repo}`, {headers: {'Authorization': `token ${auth}`},}).then(resp => resp.ok ? resp.json() : null);
ztimson marked this conversation as resolved
Review

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.
const search = await fetch(`${git}/api/v1/repos/issues/search`, {
method: 'POST',
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},
body: JSON.stringify({
title,
body,
owner,
ztimson marked this conversation as resolved
Review

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.
priority_repo_id: repoInfo.id,
type: 'issues',
limit: 3,
q: title
ztimson marked this conversation as resolved
Review

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.
Review

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.
Review

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');`
})
});
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`, {
}).then(resp => resp.ok ? resp.json() : []);
ztimson marked this conversation as resolved
Review

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.
Review

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.
let dupeId = null;
const dupeIds = search.map(t => t.id);
ztimson marked this conversation as resolved
Review

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.
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\`\`\``, {
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
Review

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.
Review

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(`\\b${id}\\b`, 'm').test(hasDuplicates)))) {
await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}/comments`, {
method: 'POST',
ztimson marked this conversation as resolved
Review

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.
Review

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'
},
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: `{"body": "Duplicate of #${dupeId}"}`
ztimson marked this conversation as resolved
Review

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.
}).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`, {
method: 'POST',
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},
body: '{"labels":["Reviewed/Duplicate"]}'
ztimson marked this conversation as resolved
Review

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.
}).then(async resp => { if(!resp.ok) throw new Error(`${resp.status} ${await resp.text()}`); });
await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}`, {
method: 'PATCH',
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},
body: '{"state": "closed"}'
}).then(async resp => { if(!resp.ok) throw new Error(`${resp.status} ${await resp.text()}`); });
console.log('Duplicate');
return process.exit();
ztimson marked this conversation as resolved
Review

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.
}
// Update ticket
await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}`, {
method: 'PATCH',
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},
body: JSON.stringify({title, body})
}).then(async resp => { if(!resp.ok) throw new Error(`${resp.status} ${await resp.text()}`); });
if(type) { // Label
await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}/labels`, {
ztimson marked this conversation as resolved
Review

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()}\`]})`.
Review

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()}\`]})`
Review

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');`
Review

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.
method: 'POST',
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},
body: `{"labels":["Reviewed/${type[0].toUpperCase() + type.slice(1).toLowerCase()}"]}`
}).then(async resp => { if(!resp.ok) throw new Error(`${resp.status} ${await resp.text()}`); });
}
console.log(`Title: ${title}\nType: ${type}\nBody:\n${body}`);
})();
})().catch(err => {
console.error(`Error: ${err.message || err.toString()}`);
process.exit(1);
});

View File

@@ -106,4 +106,7 @@ dotenv.config({path: '.env.local', override: true, quiet: true, debug: false});
if(!res.ok) throw new Error(`${res.status} ${await res.text()}`);
}
console.log(comments.map(c => `${c.path}${c.new_position ? `:${c.new_position}` : ''}\n${c.body}`).join('\n\n') + '\n\n' + summary);
})();
})().catch(err => {
console.error(`Error: ${err.message || err.toString()}`);
process.exit(1);
});