Added bot to refine existing tickets if they have the Review/AI label
Issues
Checklist
Linked issues
Reviewed changes
## Description
<!-- Addition information & context -->
Added bot to refine existing tickets if they have the Review/AI label
## Issues
<!-- Issues this PR will address -->
## Checklist
<!-- Complete after creating PR -->
- [X] Linked issues
- [X] Reviewed changes
Path mismatch: Workflow references .github/issue_templates/ but file is at .github/issue_template/ (singular)
Command mismatch: Workflow calls format but package.json defines binary as refine
Invalid AI model: claude-sonnet-4-5 is not a valid Anthropic model identifier
Missing environment variable validation: Script will fail with cryptic errors if required env vars are missing
Bugs 🐛
Wrong script name check: refine.mjs checks for 'review' instead of 'refine'
Unsafe array operations: messages.pop() and type[0] can throw if empty
Incorrect exit call: return process.exit() is redundant syntax
Label replacement: New label replaces all existing labels instead of appending
Design Issues ⚠️
Hardcoded Gitea API: /api/v1/repos/ path won't work with GitHub/GitLab
No error handling: AI service calls lack try-catch blocks
Duplicate workflow runs: May trigger twice when issue opened with label already attached
Token inconsistency: Uses github.token for clone but ASSISTANT_TOKEN for API calls
Minor Issues 📝
Confusing template: Checkbox example shows completed item in new ticket template
## Code Review Summary
### Critical Issues 🔴
- **Path mismatch**: Workflow references `.github/issue_templates/` but file is at `.github/issue_template/` (singular)
- **Command mismatch**: Workflow calls `format` but package.json defines binary as `refine`
- **Invalid AI model**: `claude-sonnet-4-5` is not a valid Anthropic model identifier
- **Missing environment variable validation**: Script will fail with cryptic errors if required env vars are missing
### Bugs 🐛
- **Wrong script name check**: refine.mjs checks for 'review' instead of 'refine'
- **Unsafe array operations**: `messages.pop()` and `type[0]` can throw if empty
- **Incorrect exit call**: `return process.exit()` is redundant syntax
- **Label replacement**: New label replaces all existing labels instead of appending
### Design Issues ⚠️
- **Hardcoded Gitea API**: `/api/v1/repos/` path won't work with GitHub/GitLab
- **No error handling**: AI service calls lack try-catch blocks
- **Duplicate workflow runs**: May trigger twice when issue opened with label already attached
- **Token inconsistency**: Uses `github.token` for clone but `ASSISTANT_TOKEN` for API calls
### Minor Issues 📝
- **Confusing template**: Checkbox example shows completed item in new ticket template
Confusing checkbox example: The template shows - [X] Completed requirement which 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.
Confusing checkbox example: The template shows `- [X] Completed requirement` which 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.
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 if condition to check the label state.
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 `if` condition to check the label state.
Security concern: Using github.token for git clone authentication may not have sufficient permissions. The workflow later uses secrets.ASSISTANT_TOKEN for API calls. Consider using the same token for consistency and proper permissions, or document why different tokens are needed.
Security concern: Using `github.token` for git clone authentication may not have sufficient permissions. The workflow later uses `secrets.ASSISTANT_TOKEN` for API calls. Consider using the same token for consistency and proper permissions, or document why different tokens are needed.
Path mismatch: The workflow references .github/issue_templates/ai-refinement.md but 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.
Path mismatch: The workflow references `.github/issue_templates/ai-refinement.md` but 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 format but according to package.json, the binary is named "refine", not "format". This should be npx -y @ztimson/ai-agents@latest refine.
Command name mismatch: The workflow runs `npx -y @ztimson/ai-agents@latest format` but according to package.json, the binary is named "refine", not "format". This should be `npx -y @ztimson/ai-agents@latest refine`.
Invalid model name: claude-sonnet-4-5 is not a valid Anthropic model name. It should likely be claude-sonnet-4 or claude-3-5-sonnet-20241022 (or similar valid Anthropic model identifier).
Invalid model name: `claude-sonnet-4-5` is not a valid Anthropic model name. It should likely be `claude-sonnet-4` or `claude-3-5-sonnet-20241022` (or similar valid Anthropic model identifier).
Wrong script name in condition: The check if(p === 'review' || p.endsWith('review.mjs')) should be if(p === 'refine' || p.endsWith('refine.mjs')) since this is the refine.mjs script, not review.mjs.
Wrong script name in condition: The check `if(p === 'review' || p.endsWith('review.mjs'))` should be `if(p === 'refine' || p.endsWith('refine.mjs'))` since this is the refine.mjs script, not review.mjs.
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.
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.
Hardcoded 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.
Hardcoded 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.
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.
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.
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.
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.
Potential runtime error: type[0].toUpperCase() will throw if type is 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 > 0 before accessing type[0].
Potential runtime error: `type[0].toUpperCase()` will throw if `type` is 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 > 0` before accessing `type[0]`.
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.
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.
Workflow command mismatch: The workflow calls format subcommand but package.json defines refine binary - this will cause immediate failure
Syntax error: Double semicolon on line 86 of refine.mjs
API compatibility: Hardcoded Gitea/Forgejo API paths won't work with GitHub Actions
Major Concerns:
Missing environment variable validation will cause cryptic errors
Label replacement logic removes all existing labels instead of preserving them
Token usage inconsistency between git clone and API calls in workflow
Moderate Issues:
Path detection regex doesn't handle relative paths like ./ or ../ correctly
Type extraction logic works but variable naming is confusing
Early exit for label check happens after API fetch (inefficient)
The 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.
## Code Review Summary
**Critical Issues:**
- **Workflow command mismatch**: The workflow calls `format` subcommand but package.json defines `refine` binary - this will cause immediate failure
- **Syntax error**: Double semicolon on line 86 of refine.mjs
- **API compatibility**: Hardcoded Gitea/Forgejo API paths won't work with GitHub Actions
**Major Concerns:**
- Missing environment variable validation will cause cryptic errors
- Label replacement logic removes all existing labels instead of preserving them
- Token usage inconsistency between git clone and API calls in workflow
**Moderate Issues:**
- Path detection regex doesn't handle relative paths like `./` or `../` correctly
- Type extraction logic works but variable naming is confusing
- Early exit for label check happens after API fetch (inefficient)
The 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.
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.
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.
The 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.
The 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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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/')), ...]
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 🔴
- **Incorrect GitHub directory path**: Issue template directory should be `ISSUE_TEMPLATE` (uppercase), not `issue_template`
- **Invalid AI model name**: "claude-sonnet-4-5" is not a valid Anthropic model name
- **Broken Ollama configuration**: The options object incorrectly passes parameters to Ollama
- **Missing environment variable validation**: Script will fail with unclear errors if required env vars are missing
### Bugs 🐛
- **Label replacement issue**: PATCH request replaces all labels instead of appending, removing the trigger label
- **Silent error swallowing**: AI request errors are caught and ignored without logging
- **Duplicate review comments**: Removal of deduplication logic will cause repeated comments on re-runs
### Improvements 💡
- **Workflow trigger optimization**: Add label filter to prevent running on every label addition
- **Platform-specific API**: Document Gitea-specific API endpoints or make configurable
- **Inconsistent exit pattern**: Use either `return` or `process.exit()`, not both
- **Type/label format mismatch**: Inconsistent label naming conventions between checking and setting
- **Insecure git clone**: Replace shell parameter expansion with GitHub Actions' checkout action
The directory name should be ISSUE_TEMPLATE (uppercase) not issue_template (lowercase) to be recognized by GitHub. The current path won't work as an issue template.
The directory name should be `ISSUE_TEMPLATE` (uppercase) not `issue_template` (lowercase) to be recognized by GitHub. The current path won't work as an issue template.
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').
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')`.
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.
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`.
Hardcoded 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.
Hardcoded 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.
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.
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.
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.
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.
The early exit on line 38 happens after the async IIFE has already started. Using return process.exit() is redundant - either use return or process.exit(), not both.
The early exit on line 38 happens after the async IIFE has already started. Using `return process.exit()` is redundant - either use `return` or `process.exit()`, not both.
The Ollama configuration incorrectly uses host as the model parameter: {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.
The Ollama configuration incorrectly uses `host` as the `model` parameter: `{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.
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.
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.
The type parsing logic type?.split('/')[0]?.trim() assumes a specific format but then constructs a label with Kind/${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.
The type parsing logic `type?.split('/')[0]?.trim()` assumes a specific format but then constructs a label with `Kind/${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.
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 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.
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.
Missing environment variable validation - Script will fail with cryptic errors if required env vars are undefined
Hardcoded Gitea API path - Won't work with GitHub/GitLab; needs to be configurable or auto-detected
Labels completely replaced - Removes "Review/AI" and all existing labels instead of appending
No top-level error handling - Async IIFE lacks catch block for unhandled promise rejections
Bugs 🐛
Type extraction logic broken - Regex will parse "[Bug" instead of "Bug" due to incorrect split logic
Type information lost - Body processing removes Type header but doesn't preserve it in output
Invalid label check - type?.length check is always truthy for strings; should check for non-empty value
Silent error swallowing - .catch(() => []) hides all AI errors, making debugging impossible
Workflow Issues ⚙️
Invalid Claude model name - 'claude-sonnet-4-5' doesn't match Claude's naming convention
Duplicate workflow runs - Triggers on both 'opened' and 'labeled' could cause double execution
Insecure git clone - Token injection via sed is fragile and exposes secrets; use actions/checkout instead
Inconsistent token usage - Uses both github.token and ASSISTANT_TOKEN without clear reasoning
Minor Issues ⚠️
Incorrect regex flags - Multiline flag 'm' unnecessary for path validation
Template format mismatch - Template shows [Bug/DevOps/...] but parser expects Bug format
Misleading exit code - process.exit() without code defaults to success when skipping
Inefficient default options - Creates unused ollama config object for other providers
## Code Review Summary
### Critical Issues 🔴
- **Missing environment variable validation** - Script will fail with cryptic errors if required env vars are undefined
- **Hardcoded Gitea API path** - Won't work with GitHub/GitLab; needs to be configurable or auto-detected
- **Labels completely replaced** - Removes "Review/AI" and all existing labels instead of appending
- **No top-level error handling** - Async IIFE lacks catch block for unhandled promise rejections
### Bugs 🐛
- **Type extraction logic broken** - Regex will parse "[Bug" instead of "Bug" due to incorrect split logic
- **Type information lost** - Body processing removes Type header but doesn't preserve it in output
- **Invalid label check** - `type?.length` check is always truthy for strings; should check for non-empty value
- **Silent error swallowing** - `.catch(() => [])` hides all AI errors, making debugging impossible
### Workflow Issues ⚙️
- **Invalid Claude model name** - 'claude-sonnet-4-5' doesn't match Claude's naming convention
- **Duplicate workflow runs** - Triggers on both 'opened' and 'labeled' could cause double execution
- **Insecure git clone** - Token injection via sed is fragile and exposes secrets; use actions/checkout instead
- **Inconsistent token usage** - Uses both github.token and ASSISTANT_TOKEN without clear reasoning
### Minor Issues ⚠️
- **Incorrect regex flags** - Multiline flag 'm' unnecessary for path validation
- **Template format mismatch** - Template shows `[Bug/DevOps/...]` but parser expects `Bug` format
- **Misleading exit code** - process.exit() without code defaults to success when skipping
- **Inefficient default options** - Creates unused ollama config object for other providers
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.
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.
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.
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.
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.
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.
The 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.
The 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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
ztimson
merged commit d1c0b7a872 into master2025-12-30 15:38:49 -05:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
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