generated from ztimson/template
Added ticket refinement bot #4
assistant
Reference in New Issue
Block a user
Delete Branch "refinement"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Description
Added bot to refine existing tickets if they have the Review/AI label
Issues
Checklist
Code Review Summary
Critical Issues 🔴
.github/issue_templates/but file is at.github/issue_template/(singular)formatbut package.json defines binary asrefineclaude-sonnet-4-5is not a valid Anthropic model identifierBugs 🐛
messages.pop()andtype[0]can throw if emptyreturn process.exit()is redundant syntaxDesign Issues ⚠️
/api/v1/repos/path won't work with GitHub/GitLabgithub.tokenfor clone butASSISTANT_TOKENfor API callsMinor Issues 📝
@@ -0,0 +46,4 @@## Acceptance Criteria- [ ] Todo requirement- [X] Completed requirementConfusing checkbox example: The template shows
- [X] Completed requirementwhich suggests a completed item, but this is meant to be a template for new tickets. This might confuse users. Consider using only unchecked examples- [ ]or clarifying with a comment.@@ -0,0 +2,4 @@on:issues:types: [opened, labeled]Missing conditional check: The workflow triggers on both 'opened' and 'labeled' events, but there's no conditional to prevent it from running twice when an issue is opened with the 'Review/AI' label already attached. Consider adding an
ifcondition to check the label state.@@ -0,0 +9,4 @@container: node:22steps:- name: Fetch coderun: |Security concern: Using
github.tokenfor git clone authentication may not have sufficient permissions. The workflow later usessecrets.ASSISTANT_TOKENfor API calls. Consider using the same token for consistency and proper permissions, or document why different tokens are needed.@@ -0,0 +13,4 @@git clone "$(echo ${{github.server_url}}/${{github.repository}}.git | sed s%://%://${{github.token}}@% )" .git checkout ${{ github.event.repository.default_branch }}- name: Run AI FormatterPath mismatch: The workflow references
.github/issue_templates/ai-refinement.mdbut the actual file is located at.github/issue_template/ai-refinement.md(singular "template" not "templates"). This will cause the workflow to fail when trying to read the template file.Command name mismatch: The workflow runs
npx -y @ztimson/ai-agents@latest formatbut according to package.json, the binary is named "refine", not "format". This should benpx -y @ztimson/ai-agents@latest refine.@@ -0,0 +16,4 @@- name: Run AI Formatterrun: npx -y @ztimson/ai-agents@latest format .github/issue_templates/ai-refinement.mdenv:AI_HOST: anthropicInvalid model name:
claude-sonnet-4-5is not a valid Anthropic model name. It should likely beclaude-sonnet-4orclaude-3-5-sonnet-20241022(or similar valid Anthropic model identifier).@@ -0,0 +10,4 @@dotenv.config({path: '.env.local', override: true, quiet: true});(async () => {let p = process.argv[process.argv.length - 1];Wrong script name in condition: The check
if(p === 'review' || p.endsWith('review.mjs'))should beif(p === 'refine' || p.endsWith('refine.mjs'))since this is the refine.mjs script, not review.mjs.@@ -0,0 +24,4 @@host = process.env['AI_HOST'],model = process.env['AI_MODEL'],token = process.env['AI_TOKEN'];Missing environment variable validation: Required environment variables (GIT_HOST, GIT_OWNER, GIT_REPO, GIT_TOKEN, TICKET, AI_HOST, AI_MODEL, AI_TOKEN) are not validated before use. If any are undefined, the script will fail with cryptic errors. Add validation with helpful error messages.
@@ -0,0 +27,4 @@console.log(`Processing issue #${ticket}`);// Fetch issueHardcoded API path: The API path
/api/v1/repos/suggests this is Gitea-specific. This won't work with GitHub, GitLab, or other Git hosting platforms. Consider making the API structure configurable or detecting the platform type.@@ -0,0 +34,4 @@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')) {console.log('Skipping');Incorrect exit call:
return process.exit()is redundant -process.exit()doesn't return. Should be justprocess.exit(0)orreturn(not both).@@ -0,0 +83,4 @@Output ONLY the formatted ticket, no explanation.`});const messages = await ai.language.ask(`Title: ${issueData.title}\n\nDescription:\n${issueData.body || 'No description provided'}`);No error handling for AI call: The
ai.language.ask()call has no try-catch block. If the AI service fails, times out, or returns an unexpected format, the script will crash without helpful error messages.@@ -0,0 +84,4 @@});const messages = await ai.language.ask(`Title: ${issueData.title}\n\nDescription:\n${issueData.body || 'No description provided'}`);const content = messages.pop().content;Unsafe array access:
messages.pop()assumes the array is non-empty. If the AI returns an empty response, this will throw an error when accessing.content. Add a check to ensure messages exist.@@ -0,0 +86,4 @@const messages = await ai.language.ask(`Title: ${issueData.title}\n\nDescription:\n${issueData.body || 'No description provided'}`);const content = messages.pop().content;const title = /^# (.+)$/m.exec(content)?.[1] || issueData.title;const typeMatch = /^## Type:\s*(.+)$/m.exec(content);Potential runtime error:
type[0].toUpperCase()will throw iftypeis an empty string. The fallback 'Unassigned' is set earlier, but if the regex extracts an empty string, this will fail. Add a check:type && type.length > 0before accessingtype[0].@@ -0,0 +97,4 @@},body: JSON.stringify({title,body,Label replacement issue: The code sets labels to
[Type/${type}]which replaces all existing labels. This will remove the 'Review/AI' label and any other labels the issue had. Consider appending to existing labels or preserving important ones.Code Review Summary
Critical Issues:
formatsubcommand but package.json definesrefinebinary - this will cause immediate failureMajor Concerns:
Moderate Issues:
./or../correctlyThe new ticket refinement feature adds useful functionality but has several bugs that will prevent it from working correctly. The most critical is the command name mismatch between the workflow and package.json.
@@ -0,0 +9,4 @@container: node:22steps:- name: Fetch coderun: |Using 'github.token' in the git clone URL may not have sufficient permissions to push changes or may not be the intended token. The workflow uses 'secrets.ASSISTANT_TOKEN' for API calls (line 25) but 'github.token' for cloning. Consider using the same token for consistency, or document why different tokens are needed.
@@ -0,0 +13,4 @@git clone "$(echo ${{github.server_url}}/${{github.repository}}.git | sed s%://%://${{github.token}}@% )" .git checkout ${{ github.event.repository.default_branch }}- name: Run AI FormatterThe workflow calls 'format' subcommand but package.json defines 'refine' as the binary name. This mismatch will cause the workflow to fail. Either change the command to 'npx -y @ztimson/ai-agents@latest refine' or update package.json to use 'format' as the binary name.
@@ -0,0 +11,4 @@(async () => {let p = process.argv[process.argv.length - 1];if(p === 'refine' || p.endsWith('refine.mjs')) p = null;The path detection regex '/(\/|[A-Z]:)/.test(p)' checks if the path is absolute, but the logic is inverted - if it's NOT absolute, it joins with cwd. However, this doesn't handle edge cases like './' or '../' relative paths correctly, which would be treated as absolute due to the '/' character.
@@ -0,0 +23,4 @@ticket = process.env['TICKET'],host = process.env['AI_HOST'],model = process.env['AI_MODEL'],token = process.env['AI_TOKEN'];Missing validation for required environment variables. If any of git, owner, repo, auth, ticket, host, model, or token are undefined, the script will fail with cryptic errors. Add explicit validation and helpful error messages at the start.
@@ -0,0 +28,4 @@console.log(`Processing issue #${ticket}`);// Fetch issueconst issueRes = await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}`, {The API endpoint pattern '/api/v1/repos/' suggests this is for Gitea/Forgejo, not GitHub. The workflow uses github context variables which work, but the hardcoded API path is not compatible with GitHub's API (/repos/ without /api/v1/). This will fail if run on GitHub Actions.
@@ -0,0 +35,4 @@const issueData = await issueRes.json();if(!issueData.labels?.some(l => l.name === 'Review/AI')) {console.log('Skipping');return process.exit();The workflow checks for 'Review/AI' label but the issue template defines the label as 'Review/AI' in the YAML frontmatter. However, this early exit happens AFTER fetching the issue. Consider adding a condition in the workflow to only trigger when the correct label is present, rather than fetching and then skipping.
@@ -0,0 +83,4 @@Output ONLY the formatted ticket, no explanation.`})const messages = await ai.language.ask(`Title: ${issueData.title}\n\nDescription:\n${issueData.body || 'No description provided'}`).catch(() => []);;Double semicolon syntax error. Remove one semicolon from '.catch(() => []);;'
@@ -0,0 +91,4 @@}const title = /^# (.+)$/m.exec(content)?.[1] || issueData.title;const typeMatch = /^## Type:\s*(.+)$/m.exec(content);const type = typeMatch?.[1]?.split('/')[0]?.trim() || 'Unassigned';The type extraction logic 'type[0].toUpperCase() + type.slice(1).toLowerCase()' operates on a string but 'type' is already a string (not an array), so 'type[0]' gets the first character. While this works, the variable name is misleading. Also, this doesn't handle the 'Unassigned' case - it would create a 'Kind/Unassigned' label which may not be desired.
@@ -0,0 +102,4 @@body: JSON.stringify({title,body,labels: type?.length ? [`Kind/${type[0].toUpperCase() + type.slice(1).toLowerCase()}`] : []The label update logic replaces ALL labels with just the Kind label. This will remove the 'Review/AI' label and any other existing labels. Consider appending to existing labels or filtering more carefully:
labels: [...issueData.labels.map(l => l.name).filter(n => !n.startsWith('Kind/')), ...]Code Review Summary
Critical Issues 🔴
ISSUE_TEMPLATE(uppercase), notissue_templateBugs 🐛
Improvements 💡
returnorprocess.exit(), not both@@ -0,0 +1,53 @@---The directory name should be
ISSUE_TEMPLATE(uppercase) notissue_template(lowercase) to be recognized by GitHub. The current path won't work as an issue template.@@ -0,0 +2,4 @@on:issues:types: [opened, labeled]The workflow triggers on both 'opened' and 'labeled' events, but doesn't filter for the specific label. This means it will run for ANY label addition. Consider adding a condition to only run when the 'Review/AI' label is added:
if: contains(github.event.issue.labels.*.name, 'Review/AI').@@ -0,0 +10,4 @@steps:- name: Fetch coderun: |git clone "$(echo ${{github.server_url}}/${{github.repository}}.git | sed s%://%://${{github.token}}@% )" .The git clone command uses shell parameter expansion which could fail if github.token contains special characters. Consider using GitHub Actions' built-in checkout action instead:
actions/checkout@v4.@@ -0,0 +17,4 @@run: npx -y @ztimson/ai-agents@latest refine .github/issue_template/ai-refinement.mdenv:AI_HOST: anthropicAI_MODEL: claude-sonnet-4-5Hardcoded AI model "claude-sonnet-4-5" appears to be incorrect. The correct model name should be "claude-sonnet-4" or "claude-3-5-sonnet-20240620" based on Anthropic's naming conventions. This will cause API errors.
@@ -0,0 +15,4 @@if(!/^(\/|[A-Z]:)/m.test(p)) p = path.join(process.cwd(), p);if(!p || !fs.existsSync(p)) throw new Error('Please provide a template');Missing validation for required environment variables. If any of git, owner, repo, auth, ticket, host, model, or token are undefined, the script will fail with unclear error messages. Add validation and provide helpful error messages for missing configuration.
@@ -0,0 +28,4 @@console.log(`Processing issue #${ticket}`);// Fetch issueconst issueRes = await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}`, {The API endpoint path suggests this is Gitea-specific (api/v1). Consider documenting this requirement or making it configurable to support other Git platforms like GitHub or GitLab.
@@ -0,0 +35,4 @@const issueData = await issueRes.json();if(!issueData.labels?.some(l => l.name === 'Review/AI')) {console.log('Skipping');return process.exit();The early exit on line 38 happens after the async IIFE has already started. Using
return process.exit()is redundant - either usereturnorprocess.exit(), not both.@@ -0,0 +42,4 @@if(fs.existsSync(readmeP)) readme = fs.readFileSync(readmeP, 'utf-8');const template = fs.readFileSync(p, 'utf-8');let options = {ollama: {model, host}};The Ollama configuration incorrectly uses
hostas themodelparameter:{ollama: {model, host}}. Based on typical Ollama SDK usage, this should likely be{ollama: {model, endpoint: host}}or similar. The current configuration will not work correctly for Ollama.@@ -0,0 +83,4 @@Output ONLY the formatted ticket, no explanation.`})const messages = await ai.language.ask(`Title: ${issueData.title}\n\nDescription:\n${issueData.body || 'No description provided'}`).catch(() => []);Error handling with
.catch(() => [])silently swallows all errors from the AI request. This makes debugging difficult. Consider logging the error or providing more context about what went wrong.@@ -0,0 +91,4 @@}const title = /^# (.+)$/m.exec(content)?.[1] || issueData.title;const typeMatch = /^## Type:\s*(.+)$/m.exec(content);const type = typeMatch?.[1]?.split('/')[0]?.trim() || 'Unassigned';The type parsing logic
type?.split('/')[0]?.trim()assumes a specific format but then constructs a label withKind/${type}. This creates a mismatch - if the AI returns "Bug", the label becomes "Kind/Bug", but the check on line 36 looks for "Review/AI". Consider standardizing the label format.@@ -0,0 +101,4 @@},body: JSON.stringify({title,body,The PATCH request replaces all labels with just the type label, which will remove the "Review/AI" label that triggered the workflow. This could cause issues if the workflow is re-triggered or if other labels are needed. Consider appending to existing labels instead of replacing them.
The removal of the existing comments check eliminates protection against duplicate review comments. If the workflow runs multiple times, it will post the same comments repeatedly. Consider keeping this deduplication logic.
Code Review Summary
Critical Issues 🔴
Bugs 🐛
type?.lengthcheck is always truthy for strings; should check for non-empty value.catch(() => [])hides all AI errors, making debugging impossibleWorkflow Issues ⚙️
Minor Issues ⚠️
[Bug/DevOps/...]but parser expectsBugformat@@ -0,0 +9,4 @@---# [Module] - [Add/Change/Fix/Refactor/Remove] [Feature/Component]The Type field shows format '[Bug/DevOps/Enhancement/Refactor/Security]' which suggests selecting one option, but the parsing code expects 'Type: Bug' format. This mismatch between template and parsing logic will cause issues. Clarify the expected format.
@@ -0,0 +1,26 @@name: Ticket refinementon:issues:The workflow triggers on both 'opened' and 'labeled' events, which could cause the workflow to run twice if an issue is opened with the Review/AI label already applied. Consider adding a condition to check if the specific label was added.
@@ -0,0 +10,4 @@steps:- name: Fetch coderun: |git clone "$(echo ${{github.server_url}}/${{github.repository}}.git | sed s%://%://${{github.token}}@% )" .The git clone command uses sed to inject the token, but this approach is fragile and exposes the token in process lists. Consider using git credential helpers or the safer 'actions/checkout@v4' action instead.
@@ -0,0 +16,4 @@- name: Run AI Formatterrun: npx -y @ztimson/ai-agents@latest refine .github/issue_template/ai-refinement.mdenv:AI_HOST: anthropicThe AI_MODEL value 'claude-sonnet-4-5' appears incorrect. Claude model names typically use format like 'claude-sonnet-4-20250514' or 'claude-3-5-sonnet-20241022'. Verify this is a valid model identifier.
@@ -0,0 +21,4 @@AI_TOKEN: ${{ secrets.ANTHROPIC_TOKEN }}GIT_HOST: ${{ github.server_url }}GIT_OWNER: ${{ github.repository_owner }}GIT_REPO: ${{ github.event.repository.name }}Using github.token for git operations but secrets.ASSISTANT_TOKEN for API calls creates confusion. If ASSISTANT_TOKEN has the necessary permissions, use it consistently. Otherwise, document why two different tokens are needed.
@@ -0,0 +9,4 @@dotenv.config({quiet: true});dotenv.config({path: '.env.local', override: true, quiet: true});(async () => {The entire script is wrapped in an async IIFE without proper error handling. If any unhandled error occurs, it will cause an unhandled promise rejection. Add a .catch() block at the end to handle errors gracefully.
@@ -0,0 +11,4 @@(async () => {let p = process.argv[process.argv.length - 1];if(p === 'refine' || p.endsWith('refine.mjs')) p = null;Path validation regex uses 'm' flag unnecessarily. The multiline flag isn't needed for this single-line path check. Also, the regex doesn't handle relative paths starting with './' or '../' which are valid.
@@ -0,0 +25,4 @@model = process.env['AI_MODEL'],token = process.env['AI_TOKEN'];console.log(`Processing issue #${ticket}`);Missing validation for required environment variables. If any of git, owner, repo, auth, ticket, host, model, or token are undefined, the script will fail with cryptic errors. Add validation to check these values exist before proceeding.
@@ -0,0 +28,4 @@console.log(`Processing issue #${ticket}`);// Fetch issueconst issueRes = await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}`, {The fetch URL appears to be hardcoded for Gitea API (api/v1). This won't work with GitHub or GitLab. Consider making the API path configurable or detecting the git host type.
@@ -0,0 +35,4 @@const issueData = await issueRes.json();if(!issueData.labels?.some(l => l.name === 'Review/AI')) {console.log('Skipping');return process.exit();Using process.exit() without an exit code defaults to 0 (success), which is misleading when skipping. Use process.exit(0) explicitly or consider this might not be an error condition.
@@ -0,0 +42,4 @@if(fs.existsSync(readmeP)) readme = fs.readFileSync(readmeP, 'utf-8');const template = fs.readFileSync(p, 'utf-8');let options = {ollama: {model, host}};The default options object sets 'ollama' configuration even when using other providers. This creates an unnecessary object with model and host properties. Consider initializing options as an empty object or only setting the relevant provider.
@@ -0,0 +83,4 @@Output ONLY the formatted ticket, no explanation.`})const messages = await ai.language.ask(`Title: ${issueData.title}\n\nDescription:\n${issueData.body || 'No description provided'}`).catch(() => []);Error handling with .catch(() => []) silently swallows all errors, making debugging difficult. At minimum, log the error before returning an empty array, or let it propagate for better error visibility.
@@ -0,0 +90,4 @@return process.exit(1);}const title = /^# (.+)$/m.exec(content)?.[1] || issueData.title;const typeMatch = /^## Type:\s*(.+)$/m.exec(content);Type extraction logic is fragile. The split('/')[0] assumes the format is "Type/Subtype" but the template shows "Type: [Option1/Option2/...]". This will incorrectly parse "[Bug" instead of "Bug". Use a more robust regex or trim brackets.
@@ -0,0 +92,4 @@const title = /^# (.+)$/m.exec(content)?.[1] || issueData.title;const typeMatch = /^## Type:\s*(.+)$/m.exec(content);const type = typeMatch?.[1]?.split('/')[0]?.trim() || 'Unassigned';const body = content.replace(/^# .+$/m, '').replace(/^## Type:.+$/m, '').trim();The body processing removes the title and type lines, but doesn't preserve the Type line in the final output. The API update only sends the body without the Type header, which means the formatted ticket loses the Type information visually.
@@ -0,0 +102,4 @@body: JSON.stringify({title,body,labels: type?.length ? [`Kind/${type[0].toUpperCase() + type.slice(1).toLowerCase()}`] : []Label assignment logic has a bug: checking 'type?.length' on a string will always be truthy (even empty string has length property). This should check if type is truthy or has non-zero length. Also, the label format assumes "Kind/" prefix which may not match existing label conventions.
The labels array assignment completely replaces existing labels. This will remove the "Review/AI" label that triggered the workflow, and any other labels the user added. Consider appending to existing labels instead of replacing them.
Testing if I can link line numbers... ignore...
.github/issue_template/ai-refinement.md#2