Added configurable labels to ticket refiner #13

Merged
ztimson merged 3 commits from ticket-duplicates into master 2026-01-14 13:22:33 -05:00
2 changed files with 33 additions and 27 deletions
Showing only changes of commit decd533e4e - Show all commits

View File

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

View File

@@ -6,8 +6,8 @@ import * as dotenv from 'dotenv';
import * as fs from 'node:fs';
import * as path from 'node:path';
dotenv.config({quiet: true});
dotenv.config({path: '.env.local', override: true, quiet: true});
dotenv.config({quiet: true, debug: false});
dotenv.config({path: '.env.local', override: true, quiet: true, debug: false});
(async () => {
let p = process.argv[process.argv.length - 1];
@@ -21,6 +21,10 @@ dotenv.config({path: '.env.local', override: true, quiet: true});
ticket = process.env['TICKET'],
host = process.env['AI_HOST'],
model = process.env['AI_MODEL'],
labelDupe = process.env['LABELS_DUPE'] || 'Review/Duplicate',
labelEnabled = process.env['LABEL_ENABLED'] || 'Review/AI',
labelsReq = process.env['LABELS_REQ'] || 'Kind/Aesthetic,Kind/Bug,Kind/DevOps,Kind/Document,Kind/Enhancement,Kind/Refactor,Kind/Security',
labelsOpt = process.env['LABELS_OPT'] || 'Breaking,Priority,QA',
token = process.env['AI_TOKEN'];
console.log(`Processing issue #${ticket}`);
@@ -32,13 +36,13 @@ dotenv.config({path: '.env.local', override: true, quiet: true});
if(resp.ok) return resp.json();
else throw new Error(`${resp.status} ${await resp.text()}`);
});
if(issueData.labels?.[0] !== 1 || issueData.labels?.[0]?.name !== 'Review/AI') {
if(issueData.labels?.length !== 1 || issueData.labels[0]?.name !== labelEnabled) {
ztimson marked this conversation as resolved
Review

Array access without null safety: The code accesses issueData.labels[0]?.name but doesn't check if the array exists before checking length. While the optional chaining on labels?.[0] is safe, consider using issueData.labels?.[0]?.name for consistency with the length check pattern.

Array access without null safety: The code accesses `issueData.labels[0]?.name` but doesn't check if the array exists before checking length. While the optional chaining on `labels?.[0]` is safe, consider using `issueData.labels?.[0]?.name` for consistency with the length check pattern.
console.log('Skipping');
return process.exit();
}
// Gather readme & template
let title = '', type = '', readme = '', readmeP = path.join(process.cwd(), 'README.md');
let title = '', type = '', labels = [], readme = '', readmeP = path.join(process.cwd(), 'README.md');
if(fs.existsSync(readmeP)) readme = fs.readFileSync(readmeP, 'utf-8');
ztimson marked this conversation as resolved
Review

Unused variable 'type' is still declared but never used after refactoring. It should be removed from the declaration on line 46.

Unused variable 'type' is still declared but never used after refactoring. It should be removed from the declaration on line 46.
Review

Unused variable 'type' is still declared but never used after refactoring. This variable should be removed from the declaration since the type-based labeling was replaced with the add_label tool.

Unused variable 'type' is still declared but never used after refactoring. This variable should be removed from the declaration since the type-based labeling was replaced with the add_label tool.
const template = p ? fs.readFileSync(p, 'utf-8') : `## Description
@@ -96,18 +100,27 @@ Implementation details, constraints, dependencies, design decisions
args: {title: {type: 'string', description: 'Ticket title, must match format: Module - Verb noun', required: true}},
fn: (args) => title = args.title
}, {
name: 'type',
description: 'Set the ticket type, must be called EXACTLY ONCE',
args: {type: {type: 'string', description: 'Ticket type', enum: ['Bug', 'DevOps', 'Document', 'Enhancement', 'Refactor', 'Security'], required: true}},
fn: (args) => type = args.type
name: 'add_label',
description: 'Add a label to the ticket',
args: {label: {type: 'string', description: 'Label name', required: true}},
fn: async (args) => {
labels.push(args.label);
ztimson marked this conversation as resolved
Review

The add_label function doesn't await the fetch call, which means errors may not be properly handled. The function is declared as async but doesn't await the promise, potentially causing race conditions or silent failures.

The add_label function doesn't await the fetch call, which means errors may not be properly handled. The function is declared as async but doesn't await the promise, potentially causing race conditions or silent failures.
Review

The add_label function doesn't await the fetch call. While the function is declared as async, the fetch promise is not awaited, which means errors may not be properly handled and could cause race conditions. Consider using 'await' before the fetch call to ensure proper error handling and sequencing.

The add_label function doesn't await the fetch call. While the function is declared as async, the fetch promise is not awaited, which means errors may not be properly handled and could cause race conditions. Consider using 'await' before the fetch call to ensure proper error handling and sequencing.
Review

Race condition risk: The add_label function is called multiple times in the AI tool execution flow, but there's no guarantee of execution order or completion. If the AI calls add_label multiple times rapidly, the labels array may not be properly synchronized with the actual API calls. Consider awaiting all label operations or using a queue.

Race condition risk: The add_label function is called multiple times in the AI tool execution flow, but there's no guarantee of execution order or completion. If the AI calls add_label multiple times rapidly, the labels array may not be properly synchronized with the actual API calls. Consider awaiting all label operations or using a queue.
fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}/labels`, {
method: 'POST',
ztimson marked this conversation as resolved
Review

The fetch call inside add_label is not awaited before returning. While the function is async and returns the promise, this could cause issues with error handling and sequencing. Consider using await fetch(...) to ensure proper error propagation.

The fetch call inside add_label is not awaited before returning. While the function is async and returns the promise, this could cause issues with error handling and sequencing. Consider using `await fetch(...)` to ensure proper error propagation.
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},
ztimson marked this conversation as resolved
Review

Security concern: The label value from args.label is directly interpolated into a JSON string without escaping. If the label contains special characters like quotes, it could break the JSON structure. Use JSON.stringify() instead: body: JSON.stringify({labels: [args.label]})

Security concern: The label value from args.label is directly interpolated into a JSON string without escaping. If the label contains special characters like quotes, it could break the JSON structure. Use JSON.stringify() instead: `body: JSON.stringify({labels: [args.label]})`
Review

Security concern: The label value from args.label is directly interpolated into a JSON string without escaping. If the label contains special characters like quotes, it could break the JSON structure or cause injection issues. Use JSON.stringify({labels: [args.label]}) instead of template literals.

Security concern: The label value from args.label is directly interpolated into a JSON string without escaping. If the label contains special characters like quotes, it could break the JSON structure or cause injection issues. Use `JSON.stringify({labels: [args.label]})` instead of template literals.
body: `{"labels":["${args.label}"]}`
}).then(async resp => { if(!resp.ok) throw new Error(`${resp.status} ${await resp.text()}`); });
}
}],
system: `Transform raw tickets into structured markdown following the template EXACTLY.
**MANDATORY STEPS:**
1. Identify ticket type: Bug, DevOps, Document, Enhancement, Refactor, or Security
2. Call \`type\` tool EXACTLY ONCE with the type from step 1
3. Call \`title\` tool EXACTLY ONCE in format: "[Module] - [Verb] [subject]" (example: Storage - fix file uploads)
4. Output formatted markdown matching template structure below
1. Call \`title\` tool EXACTLY ONCE in format: "[Module] - [Verb] [subject]" (example: Storage - fix file uploads)
2. Identify one label from each group which best applies to the ticket: ${labelsReq.replace(',', ', ')}
ztimson marked this conversation as resolved Outdated

String replacement issue: Using .replace(',', ', ') only replaces the first comma. This should be .replace(/,/g, ', ') to replace all commas with comma-space for proper formatting.

String replacement issue: Using `.replace(',', ', ')` only replaces the first comma. This should be `.replace(/,/g, ', ')` to replace all commas with comma-space for proper formatting.

String replacement issue: Using .replace(',', ', ') only replaces the first comma. This should be .replace(/,/g, ', ') to replace all commas with comma-space for proper formatting of the labels list in the prompt.

String replacement issue: Using `.replace(',', ', ')` only replaces the first comma. This should be `.replace(/,/g, ', ')` to replace all commas with comma-space for proper formatting of the labels list in the prompt.
Review

String replacement issue: Using .replaceAll(',', ', ') is correct, but note this requires Node.js 15+. If supporting older versions, use .replace(/,/g, ', ') instead for compatibility.

String replacement issue: Using `.replaceAll(',', ', ')` is correct, but note this requires Node.js 15+. If supporting older versions, use `.replace(/,/g, ', ')` instead for compatibility.
3. Call the \`add_label\` tool ONCE FOR EVERY LABEL identified in the previous step
4. Filter the following labels to any that apply to this ticket: ${labelsOpt.replace(',', ', ')}
5. Call the \`add_label\` tool ONCE FOR EVERY LABEL identified in the previous step
ztimson marked this conversation as resolved
Review

Same string replacement issue: .replace(',', ', ') only replaces the first comma. Should use .replace(/,/g, ', ') to replace all commas.

Same string replacement issue: `.replace(',', ', ')` only replaces the first comma. Should use `.replace(/,/g, ', ')` to replace all commas.
Review

Same string replacement issue: .replace(',', ', ') only replaces the first comma. Should use .replace(/,/g, ', ') to replace all commas for proper formatting of the optional labels list.

Same string replacement issue: `.replace(',', ', ')` only replaces the first comma. Should use `.replace(/,/g, ', ')` to replace all commas for proper formatting of the optional labels list.
6. Output the new ticket description in formated markdown matching the following rules:
ztimson marked this conversation as resolved Outdated

Typo in prompt: "formated markdown" should be "formatted markdown".

Typo in prompt: "formated markdown" should be "formatted markdown".

Typo in prompt: "formated markdown" should be "formatted markdown" (missing 't').

Typo in prompt: "formated markdown" should be "formatted markdown" (missing 't').
**TEMPLATE RULES:**
- Use ## headers (match template exactly)
@@ -128,11 +141,11 @@ Implementation details, constraints, dependencies, design decisions
| **Total** | **0-15** |
**SCORING:**
- Size: # of modules/layers/files changed
- Size: # of modules/layers/files affected
- Complexity: Technical difficulty
- Unknowns: Research/uncertainty needed
**README:**
**PROJECT README:**
\`\`\`markdown
${readme.trim() || 'No README available'}
\`\`\`
@@ -165,11 +178,11 @@ Output ONLY markdown. No explanations, labels, or extra formatting.`});
let dupeId = null;
const dupeIds = search.map(t => t.id);
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}`
const hasDuplicates = (await ai.language.ask(`ID: ${issueData.id}\nTitle: ${title}\n\`\`\`markdown\n${body}\n\`\`\``, {
Review

The duplicate detection prompt now includes the current issue's ID in the input, which could confuse the AI if it returns its own ID as a duplicate. Consider adding explicit instructions to the system prompt to ignore the first ID or exclude the current issue from the search results.

The duplicate detection prompt now includes the current issue's ID in the input, which could confuse the AI if it returns its own ID as a duplicate. Consider adding explicit instructions to the system prompt to ignore the first ID or exclude the current issue from the search results.
system: `Your job is to identify duplicates. Respond ONLY with the duplicate's ID number or "NONE" if no match exists\n\n${dupes}`
}))?.pop()?.content;
// Handle duplicates
if(!!hasDuplicates && (dupeId = dupeIds.find(id => new RegExp(`\\b${id}\\b`, 'm').test(hasDuplicates)))) {
if(hasDuplicates && hasDuplicates !== 'NONE' && (dupeId = dupeIds.find(id => id === hasDuplicates.trim()))) {
ztimson marked this conversation as resolved Outdated

Potential type mismatch in duplicate detection: The code compares id === hasDuplicates.trim() but issue IDs are typically numbers while hasDuplicates is a string. This comparison may fail if strict type checking is involved. Consider parsing the ID or ensuring type consistency.

Potential type mismatch in duplicate detection: The code compares `id === hasDuplicates.trim()` but issue IDs are typically numbers while hasDuplicates is a string. This comparison may fail if strict type checking is involved. Consider parsing the ID or ensuring type consistency.

The duplicate detection logic could fail if the AI returns the ID with additional text. Consider using a more robust parsing approach like parseInt(hasDuplicates) or regex extraction to handle edge cases.

The duplicate detection logic could fail if the AI returns the ID with additional text. Consider using a more robust parsing approach like `parseInt(hasDuplicates)` or regex extraction to handle edge cases.

Potential type mismatch in duplicate detection: The code compares id === hasDuplicates.trim() where issue IDs are typically numbers while hasDuplicates is a string. This comparison may fail with strict type checking. Consider using id.toString() === hasDuplicates.trim() or id === parseInt(hasDuplicates) to ensure type consistency.

Potential type mismatch in duplicate detection: The code compares `id === hasDuplicates.trim()` where issue IDs are typically numbers while hasDuplicates is a string. This comparison may fail with strict type checking. Consider using `id.toString() === hasDuplicates.trim()` or `id === parseInt(hasDuplicates)` to ensure type consistency.
Review

Type coercion in duplicate detection: Using loose equality id == hasDuplicates.trim() is intentional for type coercion, but this could mask bugs. Consider being explicit with id === parseInt(hasDuplicates.trim(), 10) or id.toString() === hasDuplicates.trim() to make the type conversion clear and handle NaN cases.

Type coercion in duplicate detection: Using loose equality `id == hasDuplicates.trim()` is intentional for type coercion, but this could mask bugs. Consider being explicit with `id === parseInt(hasDuplicates.trim(), 10)` or `id.toString() === hasDuplicates.trim()` to make the type conversion clear and handle NaN cases.
await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}/comments`, {
method: 'POST',
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},
@@ -178,7 +191,7 @@ Output ONLY markdown. No explanations, labels, or extra formatting.`});
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"]}'
body: `{"labels":["${labelDupe}"]}`
ztimson marked this conversation as resolved
Review

Inconsistent JSON construction: This line correctly uses template literal for the label variable, but it has the same security issue as line 110. For consistency and safety, use JSON.stringify({labels: [labelDupe]}) throughout the codebase.

Inconsistent JSON construction: This line correctly uses template literal for the label variable, but it has the same security issue as line 110. For consistency and safety, use `JSON.stringify({labels: [labelDupe]})` throughout the codebase.
}).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',
@@ -195,15 +208,8 @@ Output ONLY markdown. No explanations, labels, or extra formatting.`});
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`, {
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}`);
console.log(`Title: ${title}\nLabels: ${labels.join(', ')}\nBody:\n${body}`);
})().catch(err => {
console.error(`Error: ${err.message || err.toString()}`);
process.exit(1);