Check for duplicates before adding tickets #12
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "@ztimson/ai-agents",
|
||||
"version": "0.0.8",
|
||||
"version": "0.1.0",
|
||||
"description": "AI agents",
|
||||
"keywords": ["ai", "review"],
|
||||
"author": "ztimson",
|
||||
|
||||
@@ -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
|
||||
|
||||
// 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
assistant
commented
Critical bug: The search API endpoint is missing the repository path segment. This should be 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
assistant
commented
Bug: The 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
assistant
commented
Error handling issue: If the search API returns an error, the code defaults to an empty array 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
Type safety issue: The fallback 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
Race condition: The duplicate search uses the 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
assistant
commented
Potential null reference: If 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
Silent failure risk: When the search API fails (line 153 returns []), the 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
assistant
commented
Race condition risk: The duplicate check uses 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
assistant
commented
Logic issue: The duplicate detection uses 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
Logic issue: The regex pattern 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
assistant
commented
Bug: JSON string is manually constructed instead of using 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
JSON injection vulnerability: The JSON body is manually constructed using a template literal instead of JSON.stringify(). If 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
assistant
commented
Same bugs as line 172: 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
assistant
commented
Same bugs as line 172: 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
assistant
commented
Unused variable: 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
assistant
commented
Bug: JSON string is manually constructed with template literal instead of using 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
JSON injection vulnerability: The JSON body is manually constructed using a template literal. If 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
Potential runtime error: If 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
Breaking change: Label prefix changed from 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);
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
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.