feat: seed default templates from detected tools#24
Conversation
Co-authored-by: jellydn <870029+jellydn@users.noreply.github.com>
Co-authored-by: jellydn <870029+jellydn@users.noreply.github.com>
Co-authored-by: jellydn <870029+jellydn@users.noreply.github.com>
Co-authored-by: jellydn <870029+jellydn@users.noreply.github.com>
Change the codex prompt command from "codex" to "codex exec" to align with the correct CLI invocation for the OpenAI Codex tool.
- Add 8 pre-configured templates for common AI coding tasks - Include templates for code review, commit generation, architecture explanation, PR creation, type safety, testing, documentation, and code explanation - Templates support multiple aliases for easier access - Improve out-of-box experience with ready-to-use AI workflows
- Replace hardcoded templates with dynamic generation from detected tools - Add category-based prompt definitions with tool preferences - Implement intelligent tool selection with fallback logic - Support multiple AI CLI tools (opencode, claude, amp, codex, ccs profiles) - Update loadConfig to accept detected tools for template generation - Maintain backward compatibility with existing config structure This change allows the tool to automatically generate appropriate templates based on what AI tools are actually installed on the user's system, providing a better out-of-box experience without manual configuration.
|
WalkthroughReplaces static default templates with generated templates built from detected tools; Changes
Sequence DiagramsequenceDiagram
participant Index as Index Module
participant Config as Config Module
participant Builder as Template Builder
participant Validator as Validator
participant Runtime as Runtime / Launcher
Index->>Index: detect available tools
Index->>Config: loadConfig(detectedTools)
Config->>Builder: buildDefaultTemplates(detectedTools)
Builder->>Builder: normalizeName / pickTool / buildCommandForTool
Builder->>Validator: validateTemplate(s)
Validator-->>Builder: validation results
Builder-->>Config: return generated templates
Config->>Config: merge or persist templates into config
Config-->>Index: return merged config
Index->>Index: parseTemplateCommand(finalCommand) => cmd + args
Index->>Runtime: launchTool(cmd, args)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @jellydn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience by introducing support for the OpenAI Codex CLI and implementing a robust dynamic default template generation system. Instead of starting with an empty configuration, users will now automatically receive a set of intelligent, tool-aware templates tailored to their installed AI CLIs. This change aims to reduce friction for new users and leverage the specific strengths of various AI tools for common development tasks, providing instant value and progressive enhancement. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
jellydn
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Codex CLI and dynamically generates default templates, significantly improving the onboarding experience. However, a critical command injection vulnerability exists in the template construction, allowing user-supplied input to escape single-quoted prompt strings and lead to arbitrary code execution due to insufficient validation and shell execution. Robust shell argument escaping or avoiding shell execution is strongly recommended. Additionally, there are suggestions for src/config.ts to improve code clarity and consistency.
| preferredTools: ["ccs:mm", "claude", "opencode", "amp", "codex", "ccs:*"], | ||
| }, | ||
| { | ||
| name: "test", | ||
| description: "Generate tests", | ||
| aliases: ["spec", "tests"], | ||
| prompt: | ||
| "Write tests using Arrange-Act-Assert pattern. Focus on behavior, not implementation details for: $@", | ||
| requiresInput: true, | ||
| preferredTools: ["ccs:mm", "claude", "opencode", "amp", "codex", "ccs:*"], | ||
| }, | ||
| { | ||
| name: "docs", |
There was a problem hiding this comment.
Critical command injection vulnerability in buildCommandForTool (src/config.ts). The function constructs shell commands by wrapping the prompt in single quotes but does not escape single quotes within the prompt. User-supplied input can escape the quoted string and execute arbitrary shell commands, as launchTool uses a shell and isSafeCommand (src/template.ts) does not block shell metacharacters. This must be addressed immediately, for example, by robust shell argument escaping or avoiding shell execution. For future maintainability, consider refactoring the if statements to a switch statement after resolving this critical security flaw.
|
|
||
| function normalizeName(name: string): string { | ||
| return name.trim().toLowerCase(); | ||
| } |
There was a problem hiding this comment.
The logic here to strip : $@ for prompts that don't require input is a bit confusing because the prompts in CATEGORY_PROMPTS that have requiresInput: false don't contain this placeholder to begin with. This makes the .replace() call a no-op and potentially confusing.
To simplify and improve clarity, you could remove the .replace() call and rely on the convention that prompts for requiresInput: false categories don't contain the placeholder.
| } | |
| const prompt = category.prompt; |
jellydn
changed the title
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to fcac10a in 7 seconds. Click for details.
- Reviewed
214lines of code in2files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_n3tkVNhNNY3dixkf
You can customize 
Greptile OverviewGreptile SummaryIntroduced automatic template generation based on detected tools, improving first-run user experience by seeding useful defaults instead of starting with an empty config. Key Changes:
Issue Found:
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| src/config.ts | Added template generation logic with category prompts and tool detection - found regex issue in prompt trimming logic |
| src/index.ts | Reordered tool detection to run before config loading - clean and correct implementation |
Sequence Diagram
sequenceDiagram
participant User
participant main
participant detectInstalledTools
participant loadConfig
participant buildDefaultTemplates
participant pickTool
participant buildCommandForTool
participant createDefaultConfig
participant FS as File System
User->>main: Run ai-switcher
main->>detectInstalledTools: Detect installed tools
detectInstalledTools-->>main: Return Tool[]
main->>loadConfig: loadConfig(detectedTools)
alt Config file does not exist
loadConfig->>buildDefaultTemplates: buildDefaultTemplates(detectedTools)
loop For each CATEGORY_PROMPTS
buildDefaultTemplates->>pickTool: pickTool(detectedTools, preferredTools)
pickTool-->>buildDefaultTemplates: Return matching Tool or undefined
alt Tool found
buildDefaultTemplates->>buildCommandForTool: buildCommandForTool(tool, prompt)
buildCommandForTool-->>buildDefaultTemplates: Return command string or null
alt Command built successfully
buildDefaultTemplates->>buildDefaultTemplates: Add template to list
end
end
end
buildDefaultTemplates-->>loadConfig: Return Template[]
loadConfig->>createDefaultConfig: createDefaultConfig(templates)
createDefaultConfig->>FS: Write config.json with generated templates
loadConfig-->>main: Return Config with generated templates
else Config file exists
loadConfig->>FS: Read config.json
FS-->>loadConfig: Return config content
loadConfig-->>main: Return existing Config
end
main->>main: Continue with tool execution
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| continue; | ||
| } | ||
|
|
||
| const prompt = category.requiresInput ? category.prompt : category.prompt.replace(": $@", ""); |
There was a problem hiding this comment.
Line 210's regex will not match prompts that end with : $@, causing them to remain unchanged
| const prompt = category.requiresInput ? category.prompt : category.prompt.replace(": $@", ""); | |
| const prompt = category.requiresInput ? category.prompt : category.prompt.replace(/ $@$/, ""); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/config.ts
Line: 210:210
Comment:
Line 210's regex will not match prompts that end with `: $@`, causing them to remain unchanged
```suggestion
const prompt = category.requiresInput ? category.prompt : category.prompt.replace(/ $@$/, "");
```
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/config.ts`:
- Around line 176-195: The buildCommandForTool function interpolates raw prompt
text into single-quoted shell commands (variable prompt), which breaks when the
prompt contains single quotes; sanitize the prompt before interpolation by
replacing each single quote with the standard shell-safe sequence that closes
the single-quoted string, inserts an escaped single quote, and reopens it (i.e.,
perform the shell-safe single-quote escaping), then use that sanitizedPrompt in
every returned command branch (including the tool.promptCommand branch) so
generated commands remain valid.
There was a problem hiding this comment.
Pull request overview
This PR implements automatic template generation based on detected tools to improve the first-run experience. Instead of starting with empty configuration, new users get pre-configured templates tailored to the tools installed on their system.
Changes:
- Modified config loading to accept detected tools and generate default templates based on tool availability
- Added CATEGORY_PROMPTS data structure defining template categories with tool preferences
- Reordered initialization in main() to detect tools before loading config
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/index.ts | Reordered tool detection to occur before config loading, enabling template seeding |
| src/config.ts | Added template generation logic including CATEGORY_PROMPTS definitions, tool selection, and command building |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (name.startsWith("ccs:") && tool.promptCommand) { | ||
| return `${tool.promptCommand} '${prompt}'`; |
There was a problem hiding this comment.
The new template generation always uses "--permission-mode plan" for ccs: profiles, but the old DEFAULT_TEMPLATES used "--permission-mode acceptEdits" for templates like "types", "test", and "docs" (see old config.ts lines 40-57). This is a breaking behavioral change that will affect users upgrading from the old defaults. Templates that previously made automatic edits will now only suggest them. Consider either preserving the acceptEdits mode for appropriate templates or documenting this as a breaking change.
| function normalizeName(name: string): string { | ||
| return name.trim().toLowerCase(); | ||
| } | ||
|
|
||
| function pickTool(detectedTools: Tool[], preferredTools: string[]): Tool | undefined { | ||
| const toolsByName = new Map(detectedTools.map((tool) => [normalizeName(tool.name), tool])); | ||
|
|
||
| for (const preferred of preferredTools) { | ||
| const normalized = normalizeName(preferred); | ||
| if (normalized === "ccs:*") { | ||
| const ccsProfile = detectedTools.find((tool) => normalizeName(tool.name).startsWith("ccs:")); | ||
| if (ccsProfile) { | ||
| return ccsProfile; | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| const directMatch = toolsByName.get(normalized); | ||
| if (directMatch) { | ||
| return directMatch; | ||
| } | ||
| } | ||
|
|
||
| return undefined; | ||
| } | ||
|
|
||
| function buildCommandForTool(tool: Tool, prompt: string): string | null { | ||
| const name = normalizeName(tool.name); | ||
| if (name === "opencode") { | ||
| return `opencode run --model opencode/big-pickle --agent plan '${prompt}'`; | ||
| } | ||
| if (name === "claude") { | ||
| return `claude --permission-mode plan -p '${prompt}'`; | ||
| } | ||
| if (name === "amp") { | ||
| return `amp -x '${prompt}'`; | ||
| } | ||
| if (name === "codex") { | ||
| return `codex exec '${prompt}'`; | ||
| } | ||
| if (name.startsWith("ccs:") && tool.promptCommand) { | ||
| return `${tool.promptCommand} '${prompt}'`; | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| function buildDefaultTemplates(detectedTools: Tool[]): Template[] { | ||
| if (detectedTools.length === 0) { | ||
| return []; | ||
| } | ||
|
|
||
| const templates: Template[] = []; | ||
|
|
||
| for (const category of CATEGORY_PROMPTS) { | ||
| const tool = pickTool(detectedTools, category.preferredTools); | ||
| if (!tool) { | ||
| continue; | ||
| } | ||
|
|
||
| const prompt = category.requiresInput ? category.prompt : category.prompt.replace(": $@", ""); | ||
| const command = buildCommandForTool(tool, prompt); | ||
| if (!command) { | ||
| continue; | ||
| } | ||
|
|
||
| templates.push({ | ||
| name: category.name, | ||
| command, | ||
| description: `${category.description} with ${tool.name}`, | ||
| aliases: category.aliases, | ||
| }); | ||
| } | ||
|
|
||
| return templates; | ||
| } |
There was a problem hiding this comment.
The new functions buildDefaultTemplates, pickTool, buildCommandForTool, and normalizeName are not covered by tests. Given that other functions in this file have comprehensive test coverage (see config.test.ts), these new functions should also have tests to ensure they correctly generate templates from detected tools, handle edge cases like missing tools, and properly format commands.
| continue; | ||
| } | ||
|
|
||
| const prompt = category.requiresInput ? category.prompt : category.prompt.replace(": $@", ""); |
There was a problem hiding this comment.
The replace(": $@", "") logic is inconsistent with the actual prompt patterns. Some prompts that have requiresInput: false don't have ": $@" suffix to remove (e.g., "Explain this codebase architecture" on line 102, "Create draft pr with what why how by gh cli" on line 110). Consider using a more robust pattern like prompt.replace(/[:\s]*\$@\s*$/, "") or checking if the prompt ends with "$@" before attempting replacement.
| const prompt = category.requiresInput ? category.prompt : category.prompt.replace(": $@", ""); | |
| const prompt = category.requiresInput | |
| ? category.prompt | |
| : category.prompt.replace(/[:\s]*\$@\s*$/, ""); |
| description: "Generate commit message", | ||
| aliases: ["zen", "logical-commit"], | ||
| prompt: | ||
| "Review the following changes and generate a concise git commit message, group by logical changes with commitizen convention, do atomic commit message: $@", |
There was a problem hiding this comment.
The commit message prompt differs from the previous DEFAULT_TEMPLATES. The old version included "on git" ("Review the following changes on git and generate...") while the new version omits it ("Review the following changes and generate..."). This may affect the quality of generated commit messages as the tool may not understand the context is specifically about git changes. Consider adding "on git" back or making it more explicit that this is for git changes.
| "Review the following changes and generate a concise git commit message, group by logical changes with commitizen convention, do atomic commit message: $@", | |
| "Review the following changes on git and generate a concise git commit message, group by logical changes with commitizen convention, do atomic commit message: $@", |
| function buildCommandForTool(tool: Tool, prompt: string): string | null { | ||
| const name = normalizeName(tool.name); | ||
| if (name === "opencode") { | ||
| return `opencode run --model opencode/big-pickle --agent plan '${prompt}'`; | ||
| } | ||
| if (name === "claude") { | ||
| return `claude --permission-mode plan -p '${prompt}'`; | ||
| } | ||
| if (name === "amp") { | ||
| return `amp -x '${prompt}'`; | ||
| } | ||
| if (name === "codex") { | ||
| return `codex exec '${prompt}'`; | ||
| } |
There was a problem hiding this comment.
The hardcoded commands in buildCommandForTool don't match the tool definitions from detectInstalledTools. For example, opencode is detected with promptCommand "opencode run --model opencode/big-pickle" (see detect.ts:21), but buildCommandForTool adds "--agent plan" which isn't in the detected tool's promptCommand. This creates inconsistency between template-based usage and direct tool usage. Consider either using tool.promptCommand when available, or documenting why templates need different flags than regular tool invocation.
- Add escapeSingleQuotes function to safely handle single quotes in commands - Extract SAFE_COMMAND_PATTERN constant for consistent validation - Refactor validateTool function to use early returns for better readability - Extract countPlaceholders helper function for template validation - Add parseTemplateCommand function to centralize command parsing logic - Remove hardcoded DEFAULT_TEMPLATES and use empty array instead - Improve error handling in validateTemplate with early returns
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed a73cf1e in 7 seconds. Click for details.
- Reviewed
255lines of code in2files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_LyDKgkh4WVtjfRld
You can customize
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/config.ts (1)
337-338:⚠️ Potential issue | 🟡 MinorAdd try-catch for JSON parsing to provide helpful error messages.
If the config file contains malformed JSON,
JSON.parsewill throw aSyntaxErrorwith a generic message like "Unexpected token". Wrapping this in a try-catch would provide a more helpful error that includes the config file path.🛡️ Proposed fix
const rawContent = readFileSync(CONFIG_PATH, "utf-8"); - const parsed = JSON.parse(rawContent); + let parsed: unknown; + try { + parsed = JSON.parse(rawContent); + } catch (error) { + const message = error instanceof SyntaxError ? error.message : "Unknown parsing error"; + throw new Error(`Invalid JSON in config file ${CONFIG_PATH}: ${message}`); + }
🧹 Nitpick comments (1)
src/config.ts (1)
102-122: Consider handling duplicate tool names explicitly.If
detectedToolscontains tools with the same normalized name, theMapconstructor will silently keep only the last one. While this is unlikely withdetectInstalledTools, it could cause unexpected behavior with user-provided tools.♻️ Optional: Log warning on duplicates
function pickTool(detectedTools: Tool[], preferredTools: string[]): Tool | undefined { + const seenNames = new Set<string>(); + for (const tool of detectedTools) { + const normalized = normalizeName(tool.name); + if (seenNames.has(normalized)) { + console.warn(`Warning: Duplicate tool name detected: ${tool.name}`); + } + seenNames.add(normalized); + } const toolsByName = new Map(detectedTools.map((tool) => [normalizeName(tool.name), tool]));
Code Review: Issues Found🔴 Blocking Issues1. Bug: validateTool() no longer accumulates all validation errors The refactored function returns early on each validation failure instead of accumulating errors. Previously, users would see all issues at once; now they must run the tool multiple times to discover each error. 2. Potential double-escaping in template commands Template commands are escaped in buildCommandForTool(), then escaped again in launchToolWithPrompt(). This could cause improperly nested shell quotes. 🟡 Suggestions3. Missing test updates 4. Unused field in CATEGORY_PROMPTS ✅ OverallGood separation of concerns in the new functions. Please address the error accumulation bug before merging. |
| function buildCommandForTool(tool: Tool, prompt: string): string | null { | ||
| const name = normalizeName(tool.name); | ||
| if (name === "opencode") { | ||
| return `opencode run --model opencode/big-pickle --agent plan '${escapeSingleQuotes(prompt)}'`; |
There was a problem hiding this comment.
| return `opencode run --model opencode/big-pickle --agent plan '${escapeSingleQuotes(prompt)}'`; | |
| return `opencode run --model opencode/minimax-m2.5-free --agent plan '${escapeSingleQuotes(prompt)}'`; |
@copilot code review[agent] ❯ opencode models
opencode/big-pickle
opencode/claude-3-5-haiku
opencode/claude-haiku-4-5
opencode/claude-opus-4-1
opencode/claude-opus-4-5
opencode/claude-opus-4-6
opencode/claude-sonnet-4
opencode/claude-sonnet-4-5
opencode/claude-sonnet-4-6
opencode/gemini-3-flash
opencode/gemini-3-pro
opencode/gemini-3.1-pro
opencode/glm-4.6
opencode/glm-4.7
opencode/glm-5
opencode/gpt-5
opencode/gpt-5-codex
opencode/gpt-5-nano
opencode/gpt-5.1
opencode/gpt-5.1-codex
opencode/gpt-5.1-codex-max
opencode/gpt-5.1-codex-mini
opencode/gpt-5.2
opencode/gpt-5.2-codex
opencode/gpt-5.3-codex
opencode/kimi-k2.5
opencode/minimax-m2.1
opencode/minimax-m2.5
opencode/minimax-m2.5-free
opencode/trinity-large-preview-free
…40) * Initial plan * Use opencode/minimax-m2.5-free as default model for opencode template Co-authored-by: jellydn <870029+jellydn@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jellydn <870029+jellydn@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/config.ts (2)
4-5: Use@/path aliases forsrcimports.Line 5 (and the adjacent internal import on Line 4) still uses relative
./...imports insidesrc/, which breaks the alias rule.♻️ Proposed fix
-import { isSafeCommand } from "./template"; -import type { Config, ConfigValidationError, Template, Tool } from "./types"; +import { isSafeCommand } from "@/template"; +import type { Config, ConfigValidationError, Template, Tool } from "@/types";As per coding guidelines: "src/**/*.{ts,tsx}: Use path alias
@/*to resolve tosrc/*in imports".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.ts` around lines 4 - 5, Update the imports at the top of this module to use the project path alias instead of relative paths: replace the relative import of isSafeCommand from "./template" with "@/template" and replace the type import of Config, ConfigValidationError, Template, Tool from "./types" with "@/types" so the module imports use the "@/..." alias and resolve correctly under the src/* alias; ensure import specifiers remain unchanged (isSafeCommand, Config, ConfigValidationError, Template, Tool).
21-27:requiresInputis dead metadata right now.
requiresInputis defined in the category schema but not used when building templates, so it can drift frompromptplaceholder behavior.♻️ One way to enforce it
for (const category of CATEGORY_PROMPTS) { + const hasPlaceholder = category.prompt.includes("$@"); + if (category.requiresInput && !hasPlaceholder) { + throw new Error( + `Category "${category.name}" requires input but prompt is missing "$@"`, + ); + } + if (!category.requiresInput && hasPlaceholder) { + throw new Error( + `Category "${category.name}" does not require input but prompt includes "$@"`, + ); + } + const tool = pickTool(detectedTools, category.preferredTools); if (!tool) { continue; }As per coding guidelines: "Remove unused code -
noUnusedLocalsandnoUnusedParametersmust pass".Also applies to: 156-163
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.ts` around lines 21 - 27, The CATEGORY_PROMPTS schema currently declares a requiresInput field that isn’t used when building templates; remove this dead metadata by deleting the requiresInput property from the CATEGORY_PROMPTS type definition and from the duplicate schema occurrence later in the file, and then search for any remaining references to requiresInput and either remove those usages or wire them into the template-building logic (e.g., the function that composes prompts) if you intend to preserve the flag; ensure the file compiles with no unused locals/params after the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/config.ts`:
- Around line 199-223: The validator currently returns early on the first
failure for a tool (checks on t.name, t.command, SAFE_COMMAND_PATTERN,
t.description), so change the function (the block validating t.name / t.command
/ t.description and calling validateAliases) to accumulate errors in an array
(e.g., const errors: ValidationError[] = []), push each error object instead of
returning immediately, then at the end concat the result of
validateAliases(t.aliases, `${path}.aliases`) and return the full errors array;
ensure you still trim/check types as before and preserve the same error shape
and path strings.
- Line 10: SAFE_COMMAND_PATTERN currently uses \s which permits tabs and other
whitespace; update the regex to only allow the ASCII space character (e.g.,
replace \s in the character class with a literal space or explicit space range)
so commands validated by SAFE_COMMAND_PATTERN only accept spaces and not
tabs/newlines, then run/update tests that exercise the command validation to
ensure tab-containing strings are rejected; reference symbol:
SAFE_COMMAND_PATTERN.
---
Nitpick comments:
In `@src/config.ts`:
- Around line 4-5: Update the imports at the top of this module to use the
project path alias instead of relative paths: replace the relative import of
isSafeCommand from "./template" with "@/template" and replace the type import of
Config, ConfigValidationError, Template, Tool from "./types" with "@/types" so
the module imports use the "@/..." alias and resolve correctly under the src/*
alias; ensure import specifiers remain unchanged (isSafeCommand, Config,
ConfigValidationError, Template, Tool).
- Around line 21-27: The CATEGORY_PROMPTS schema currently declares a
requiresInput field that isn’t used when building templates; remove this dead
metadata by deleting the requiresInput property from the CATEGORY_PROMPTS type
definition and from the duplicate schema occurrence later in the file, and then
search for any remaining references to requiresInput and either remove those
usages or wire them into the template-building logic (e.g., the function that
composes prompts) if you intend to preserve the flag; ensure the file compiles
with no unused locals/params after the change.
| const CONFIG_PATH = join(CONFIG_DIR, "config.json"); | ||
|
|
||
| const DEFAULT_TEMPLATES: Template[] = [ | ||
| const SAFE_COMMAND_PATTERN = /^[a-zA-Z0-9._\s-]+$/; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -E "src/config\.(ts|js)" | head -5Repository: jellydn/ai-launcher
Length of output: 76
🏁 Script executed:
cat -n src/config.ts | head -30Repository: jellydn/ai-launcher
Length of output: 1078
🏁 Script executed:
cat -n src/config.ts | sed -n '200,220p'Repository: jellydn/ai-launcher
Length of output: 871
🏁 Script executed:
rg "SAFE_COMMAND_PATTERN" -nRepository: jellydn/ai-launcher
Length of output: 370
🏁 Script executed:
python3 << 'EOF'
import re
# Current pattern
current = re.compile(r'^[a-zA-Z0-9._\s-]+$')
# Proposed fix
fixed = re.compile(r'^[a-zA-Z0-9._ -]+$')
test_cases = [
("claude --model sonnet", "valid command with space"),
("claude\t--model sonnet", "command with tab (should FAIL)"),
("claude\nrm -rf /", "command with newline (should FAIL)"),
("my-tool_name", "valid with hyphen and underscore"),
("tool.name", "valid with period"),
("claude -c config.json", "valid multiword"),
]
print("CURRENT PATTERN (vulnerable):")
for cmd, desc in test_cases:
result = bool(current.fullmatch(cmd))
print(f" {result}: {desc!r}")
print("\nPROPOSED FIX (secure):")
for cmd, desc in test_cases:
result = bool(fixed.fullmatch(cmd))
print(f" {result}: {desc!r}")
EOFRepository: jellydn/ai-launcher
Length of output: 555
SAFE_COMMAND_PATTERN currently allows tabs and other whitespace characters.
Line 10 uses \s, which matches tabs, spaces, and other whitespace. This permits commands with embedded tabs (e.g., "command\ttab-separated") to bypass validation at line 214, creating a command injection vector.
🛡️ Proposed fix
-const SAFE_COMMAND_PATTERN = /^[a-zA-Z0-9._\s-]+$/;
+const SAFE_COMMAND_PATTERN = /^[a-zA-Z0-9._ -]+$/;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const SAFE_COMMAND_PATTERN = /^[a-zA-Z0-9._\s-]+$/; | |
| const SAFE_COMMAND_PATTERN = /^[a-zA-Z0-9._ -]+$/; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/config.ts` at line 10, SAFE_COMMAND_PATTERN currently uses \s which
permits tabs and other whitespace; update the regex to only allow the ASCII
space character (e.g., replace \s in the character class with a literal space or
explicit space range) so commands validated by SAFE_COMMAND_PATTERN only accept
spaces and not tabs/newlines, then run/update tests that exercise the command
validation to ensure tab-containing strings are rejected; reference symbol:
SAFE_COMMAND_PATTERN.
|
@copilot old have tools but not on this PR
|
… fix validateTool (#41) * Initial plan * feat: add missing templates, permissionMode/agent support, fix validateTool and commit-zen prompt Co-authored-by: jellydn <870029+jellydn@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jellydn <870029+jellydn@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/config.ts (2)
319-323:⚠️ Potential issue | 🔴 CriticalGuard
validateToolagainst non-object entries before property reads.Line 322 casts
toolwithout runtime guarding. Iftoolisnullor a primitive value, accessingt.name,t.command,t.description, ort.aliaseson lines 324–345 will throw aTypeErrorinstead of returning validation errors.Suggested fix
function validateTool(tool: unknown, index: number): ConfigValidationError[] { const errors: ConfigValidationError[] = []; const path = `tools[${index}]`; + if (typeof tool !== "object" || tool === null) { + return [{ path, message: "Tool must be an object" }]; + } const t = tool as Record<string, unknown>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.ts` around lines 319 - 323, The validateTool function currently casts tool to a Record and reads properties without checking for null/primitives; add a runtime guard at the start of validateTool (check typeof tool === "object" && tool !== null) and if it fails push a ConfigValidationError for the path (`tools[${index}]`) indicating an invalid non-object entry and return errors early; only after the guard, perform the cast to `t` and proceed to validate t.name, t.command, t.description, and t.aliases.
348-351:⚠️ Potential issue | 🔴 CriticalAdd null/object guard before casting in
validateTemplate.Line 350 casts
templatetoRecord<string, unknown>and immediately accessest.nameat line 352 without validating thattemplateis actually an object. If a null or non-object value is passed (which can happen when parsing malformed JSON), accessing properties will throw a runtime error.Suggested fix
export function validateTemplate(template: unknown, path: string): ConfigValidationError[] { const errors: ConfigValidationError[] = []; + if (typeof template !== "object" || template === null) { + return [{ path, message: "Template must be an object" }]; + } const t = template as Record<string, unknown>;This same issue exists in
validateToolat line 322.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.ts` around lines 348 - 351, validateTemplate currently casts template to Record<string, unknown> (t) and immediately reads properties like t.name without guarding for null/non-object inputs; update validateTemplate to first check that template is a non-null plain object (e.g. typeof template === 'object' && template !== null && !Array.isArray(template)) and if not, push a ConfigValidationError and return early or handle appropriately before casting/using t; make the same guard/failure path change in validateTool to prevent runtime property access on null/non-objects (referencing validateTemplate, validateTool, and the variable t/name access to locate the spots to change).
♻️ Duplicate comments (1)
src/config.ts (1)
10-10:⚠️ Potential issue | 🔴 CriticalRestrict whitespace in command allowlist regex.
Line 10 uses
\s, which accepts tabs/newlines in addition to spaces; that weakens command validation.Suggested fix
-const SAFE_COMMAND_PATTERN = /^[a-zA-Z0-9._\s-]+$/; +const SAFE_COMMAND_PATTERN = /^[a-zA-Z0-9._ -]+$/;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.ts` at line 10, The regex SAFE_COMMAND_PATTERN currently uses \s which permits tabs and newlines; change it to allow only the literal space character so commands can't include control whitespace. Locate the constant SAFE_COMMAND_PATTERN and replace /^[a-zA-Z0-9._\s-]+$/ with a pattern that uses a literal space (for example /^[a-zA-Z0-9._ \-]+$/ or /^[a-zA-Z0-9._ -]+$/), ensuring the hyphen is placed or escaped so it is treated literally; run related validation/tests after the change.
🧹 Nitpick comments (2)
src/config.ts (2)
5-5: Use the src path alias for type imports.Line 5 uses a relative import from inside
src/; repository rules require@/*aliasing.Suggested fix
-import type { Config, ConfigValidationError, Template, Tool } from "./types"; +import type { Config, ConfigValidationError, Template, Tool } from "@/types";As per coding guidelines, "Use path alias
@/*to resolve tosrc/*in imports."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.ts` at line 5, The import in src/config.ts currently uses a relative path for types; update the import that brings in Config, ConfigValidationError, Template, and Tool to use the repository path alias (`@/`*) that maps to src/* (e.g., import these types from "@/types") so all type imports follow the project's aliasing rules and resolve correctly; ensure you only change the module path, keeping the same named type imports.
21-30: Remove or enforcerequiresInput; it is currently dead metadata.
requiresInputis defined in category definitions but not used when generating templates/commands.As per coding guidelines, "Remove unused code -
noUnusedLocalsandnoUnusedParametersmust pass."Also applies to: 276-287
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.ts` around lines 21 - 30, The CATEGORY_PROMPTS type includes an unused property requiresInput; remove this dead metadata by deleting requiresInput from the type declaration and from every object inside the CATEGORY_PROMPTS array (and the duplicate definitions around the other occurrence), or alternatively implement consistent enforcement where templates/command generation checks CATEGORY_PROMPTS[].requiresInput; specifically update the type definition for CATEGORY_PROMPTS (remove requiresInput), remove the requiresInput key from each category object, and run the build to ensure no references remain (if any code references requiresInput, replace that logic with the intended behavior in the generator functions instead of leaving the property unused).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/config.ts`:
- Around line 319-323: The validateTool function currently casts tool to a
Record and reads properties without checking for null/primitives; add a runtime
guard at the start of validateTool (check typeof tool === "object" && tool !==
null) and if it fails push a ConfigValidationError for the path
(`tools[${index}]`) indicating an invalid non-object entry and return errors
early; only after the guard, perform the cast to `t` and proceed to validate
t.name, t.command, t.description, and t.aliases.
- Around line 348-351: validateTemplate currently casts template to
Record<string, unknown> (t) and immediately reads properties like t.name without
guarding for null/non-object inputs; update validateTemplate to first check that
template is a non-null plain object (e.g. typeof template === 'object' &&
template !== null && !Array.isArray(template)) and if not, push a
ConfigValidationError and return early or handle appropriately before
casting/using t; make the same guard/failure path change in validateTool to
prevent runtime property access on null/non-objects (referencing
validateTemplate, validateTool, and the variable t/name access to locate the
spots to change).
---
Duplicate comments:
In `@src/config.ts`:
- Line 10: The regex SAFE_COMMAND_PATTERN currently uses \s which permits tabs
and newlines; change it to allow only the literal space character so commands
can't include control whitespace. Locate the constant SAFE_COMMAND_PATTERN and
replace /^[a-zA-Z0-9._\s-]+$/ with a pattern that uses a literal space (for
example /^[a-zA-Z0-9._ \-]+$/ or /^[a-zA-Z0-9._ -]+$/), ensuring the hyphen is
placed or escaped so it is treated literally; run related validation/tests after
the change.
---
Nitpick comments:
In `@src/config.ts`:
- Line 5: The import in src/config.ts currently uses a relative path for types;
update the import that brings in Config, ConfigValidationError, Template, and
Tool to use the repository path alias (`@/`*) that maps to src/* (e.g., import
these types from "@/types") so all type imports follow the project's aliasing
rules and resolve correctly; ensure you only change the module path, keeping the
same named type imports.
- Around line 21-30: The CATEGORY_PROMPTS type includes an unused property
requiresInput; remove this dead metadata by deleting requiresInput from the type
declaration and from every object inside the CATEGORY_PROMPTS array (and the
duplicate definitions around the other occurrence), or alternatively implement
consistent enforcement where templates/command generation checks
CATEGORY_PROMPTS[].requiresInput; specifically update the type definition for
CATEGORY_PROMPTS (remove requiresInput), remove the requiresInput key from each
category object, and run the build to ensure no references remain (if any code
references requiresInput, replace that logic with the intended behavior in the
generator functions instead of leaving the property unused).
Summary
What changed
Why
New users currently start with an empty config, which adds setup friction. Seeding templates from detected tools gives immediate, useful workflows without manual configuration.
How
Test plan
Important
This PR adds dynamic template generation based on detected tools, enhancing initial configuration with pre-configured workflows.
buildDefaultTemplates()inconfig.ts.loadConfig()inconfig.tsnow seeds templates on first run by detecting installed tools.buildDefaultTemplates(),pickTool(), andbuildCommandForTool()inconfig.tsto support dynamic template generation.loadConfig()inconfig.tsto accept detected tools and create default config with templates.main()inindex.tsto pass detected tools toloadConfig()and handle tool selection and execution.This description was created by
for a73cf1e. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Improvements