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
Showing only changes of commit 5b9f8e0e13 - Show all commits

View File

@@ -42,7 +42,7 @@ dotenv.config({path: '.env.local', override: true, quiet: true, debug: false});
}
// Gather readme & template
let title = '', type = '', labels = [], readme = '', readmeP = path.join(process.cwd(), 'README.md');
let title = '', 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
@@ -105,7 +105,7 @@ Implementation details, constraints, dependencies, design decisions
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`, {
return await 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}"]}`
@@ -116,11 +116,11 @@ Implementation details, constraints, dependencies, design decisions
**MANDATORY STEPS:**
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(',', ', ')}
2. Identify one label from each group which best applies to the ticket: ${labelsReq.replaceAll(',', ', ')}
ztimson marked this conversation as resolved
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(',', ', ')}
4. Filter the following labels to any that apply to this ticket: ${labelsOpt.replaceAll(',', ', ')}
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:
6. Output the new ticket description in formatted markdown matching the following rules:
**TEMPLATE RULES:**
- Use ## headers (match template exactly)
@@ -182,7 +182,7 @@ Output ONLY markdown. No explanations, labels, or extra formatting.`});
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 && hasDuplicates !== 'NONE' && (dupeId = dupeIds.find(id => id === hasDuplicates.trim()))) {
if(hasDuplicates && hasDuplicates !== 'NONE' && (dupeId = dupeIds.find(id => id == hasDuplicates.trim()))) {
ztimson marked this conversation as resolved
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'},