-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(frontend): quote Claude CLI paths containing spaces on Windows #841
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 22 commits
5b30ee3
d2e4a79
b2b0b91
c1afe4a
8ba43b7
8d52e3e
651690c
f3f3042
e17d97a
2a5cd10
ad97057
45efdf9
aea1e17
01618d7
12f7a35
5ed4d27
2fea794
f7ea823
23e9713
ff1e0c0
2cd6501
6c81ec2
efcf1c0
af2e196
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,13 @@ | ||
| import type { | ||
| ChangelogGenerationRequest, | ||
| TaskSpecContent, | ||
| GitCommit | ||
| } from '../../shared/types'; | ||
| import { extractSpecOverview } from './parser'; | ||
| import type { ChangelogGenerationRequest, TaskSpecContent, GitCommit } from "../../shared/types"; | ||
| import { extractSpecOverview } from "./parser"; | ||
| import { preparePythonSubprocessCommand } from "../env-utils"; | ||
| import { isSecurePath } from "../utils/windows-paths"; | ||
|
|
||
| /** | ||
| * Format instructions for different changelog styles | ||
| */ | ||
| const FORMAT_TEMPLATES = { | ||
| 'keep-a-changelog': (version: string, date: string) => `## [${version}] - ${date} | ||
| "keep-a-changelog": (version: string, date: string) => `## [${version}] - ${date} | ||
|
|
||
| ### Added | ||
| - [New features] | ||
|
|
@@ -20,7 +18,7 @@ const FORMAT_TEMPLATES = { | |
| ### Fixed | ||
| - [Bug fixes]`, | ||
|
|
||
| 'simple-list': (version: string, date: string) => `# Release v${version} (${date}) | ||
| "simple-list": (version: string, date: string) => `# Release v${version} (${date}) | ||
|
|
||
| **New Features:** | ||
| - [List features] | ||
|
|
@@ -31,7 +29,7 @@ const FORMAT_TEMPLATES = { | |
| **Bug Fixes:** | ||
| - [List fixes]`, | ||
|
|
||
| 'github-release': (version: string, date: string) => `## ${version} - ${date} | ||
| "github-release": (version: string, date: string) => `## ${version} - ${date} | ||
|
|
||
| ### New Features | ||
|
|
||
|
|
@@ -53,84 +51,87 @@ const FORMAT_TEMPLATES = { | |
|
|
||
| ## Thanks to all contributors | ||
|
|
||
| @contributor1, @contributor2` | ||
| @contributor1, @contributor2`, | ||
| }; | ||
|
|
||
| /** | ||
| * Audience-specific writing instructions | ||
| */ | ||
| const AUDIENCE_INSTRUCTIONS = { | ||
| 'technical': 'You are a technical documentation specialist creating a changelog for developers. Use precise technical language.', | ||
| 'user-facing': 'You are a product manager writing release notes for end users. Use clear, non-technical language focusing on user benefits.', | ||
| 'marketing': 'You are a marketing specialist writing release notes. Focus on outcomes and user impact with compelling language.' | ||
| technical: | ||
| "You are a technical documentation specialist creating a changelog for developers. Use precise technical language.", | ||
| "user-facing": | ||
| "You are a product manager writing release notes for end users. Use clear, non-technical language focusing on user benefits.", | ||
| marketing: | ||
| "You are a marketing specialist writing release notes. Focus on outcomes and user impact with compelling language.", | ||
| }; | ||
|
|
||
| /** | ||
| * Get emoji usage instructions based on level and format | ||
| */ | ||
| function getEmojiInstructions(emojiLevel?: string, format?: string): string { | ||
| if (!emojiLevel || emojiLevel === 'none') { | ||
| return ''; | ||
| if (!emojiLevel || emojiLevel === "none") { | ||
| return ""; | ||
| } | ||
|
|
||
| // GitHub Release format uses specific emoji style matching Gemini CLI pattern | ||
| if (format === 'github-release') { | ||
| if (format === "github-release") { | ||
| const githubInstructions: Record<string, string> = { | ||
| 'little': `Add emojis ONLY to section headings. Use these specific emoji-heading pairs: | ||
| little: `Add emojis ONLY to section headings. Use these specific emoji-heading pairs: | ||
| - "### ✨ New Features" | ||
| - "### 🛠️ Improvements" | ||
| - "### 🐛 Bug Fixes" | ||
| - "### 📚 Documentation" | ||
| - "### 🔧 Other Changes" | ||
| Do NOT add emojis to individual line items.`, | ||
| 'medium': `Add emojis to section headings AND to notable/important items only. | ||
| medium: `Add emojis to section headings AND to notable/important items only. | ||
| Section headings MUST use these specific emoji-heading pairs: | ||
| - "### ✨ New Features" | ||
| - "### 🛠️ Improvements" | ||
| - "### 🐛 Bug Fixes" | ||
| - "### 📚 Documentation" | ||
| - "### 🔧 Other Changes" | ||
| Add emojis to 2-3 highlighted items per section that are particularly significant.`, | ||
| 'high': `Add emojis to section headings AND every line item. | ||
| high: `Add emojis to section headings AND every line item. | ||
| Section headings MUST use these specific emoji-heading pairs: | ||
| - "### ✨ New Features" | ||
| - "### 🛠️ Improvements" | ||
| - "### 🐛 Bug Fixes" | ||
| - "### 📚 Documentation" | ||
| - "### 🔧 Other Changes" | ||
| Every line item should start with a contextual emoji.` | ||
| Every line item should start with a contextual emoji.`, | ||
| }; | ||
| return githubInstructions[emojiLevel] || ''; | ||
| return githubInstructions[emojiLevel] || ""; | ||
| } | ||
|
|
||
| // Default instructions for other formats | ||
| const instructions: Record<string, string> = { | ||
| 'little': `Add emojis ONLY to section headings. Each heading should have one contextual emoji at the start. | ||
| little: `Add emojis ONLY to section headings. Each heading should have one contextual emoji at the start. | ||
| Examples: | ||
| - "### ✨ New Features" or "### 🚀 New Features" | ||
| - "### 🐛 Bug Fixes" | ||
| - "### 🔧 Improvements" or "### ⚡ Improvements" | ||
| - "### 📚 Documentation" | ||
| Do NOT add emojis to individual line items.`, | ||
| 'medium': `Add emojis to section headings AND to notable/important items only. | ||
| medium: `Add emojis to section headings AND to notable/important items only. | ||
| Section headings should have one emoji (e.g., "### ✨ New Features", "### 🐛 Bug Fixes"). | ||
| Add emojis to 2-3 highlighted items per section that are particularly significant. | ||
| Examples of highlighted items: | ||
| - "- 🎉 **Major Feature**: Description" | ||
| - "- 🔒 **Security Fix**: Description" | ||
| Most regular line items should NOT have emojis.`, | ||
| 'high': `Add emojis to section headings AND every line item for maximum visual appeal. | ||
| high: `Add emojis to section headings AND every line item for maximum visual appeal. | ||
| Section headings: "### ✨ New Features", "### 🐛 Bug Fixes", "### ⚡ Improvements" | ||
| Every line item should start with a contextual emoji: | ||
| - "- ✨ Added new feature..." | ||
| - "- 🐛 Fixed bug where..." | ||
| - "- 🔧 Improved performance of..." | ||
| - "- 📝 Updated documentation for..." | ||
| - "- 🎨 Refined UI styling..." | ||
| Use diverse, contextually appropriate emojis for each item.` | ||
| Use diverse, contextually appropriate emojis for each item.`, | ||
| }; | ||
|
|
||
| return instructions[emojiLevel] || ''; | ||
| return instructions[emojiLevel] || ""; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -145,28 +146,30 @@ export function buildChangelogPrompt( | |
| const emojiInstruction = getEmojiInstructions(request.emojiLevel, request.format); | ||
|
|
||
| // Build CONCISE task summaries (key to avoiding timeout) | ||
| const taskSummaries = specs.map(spec => { | ||
| const parts: string[] = [`- **${spec.specId}**`]; | ||
| const taskSummaries = specs | ||
| .map((spec) => { | ||
| const parts: string[] = [`- **${spec.specId}**`]; | ||
|
|
||
| // Get workflow type if available | ||
| if (spec.implementationPlan?.workflow_type) { | ||
| parts.push(`(${spec.implementationPlan.workflow_type})`); | ||
| } | ||
| // Get workflow type if available | ||
| if (spec.implementationPlan?.workflow_type) { | ||
| parts.push(`(${spec.implementationPlan.workflow_type})`); | ||
| } | ||
|
|
||
| // Extract just the overview/purpose | ||
| if (spec.spec) { | ||
| const overview = extractSpecOverview(spec.spec); | ||
| if (overview) { | ||
| parts.push(`: ${overview}`); | ||
| // Extract just the overview/purpose | ||
| if (spec.spec) { | ||
| const overview = extractSpecOverview(spec.spec); | ||
| if (overview) { | ||
| parts.push(`: ${overview}`); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return parts.join(''); | ||
| }).join('\n'); | ||
| return parts.join(""); | ||
| }) | ||
| .join("\n"); | ||
|
|
||
| // Format-specific instructions for tasks mode | ||
| let formatSpecificInstructions = ''; | ||
| if (request.format === 'github-release') { | ||
| let formatSpecificInstructions = ""; | ||
| if (request.format === "github-release") { | ||
| formatSpecificInstructions = ` | ||
| For GitHub Release format: | ||
|
|
||
|
|
@@ -188,65 +191,64 @@ RELEASE TITLE (CRITICAL): | |
|
|
||
| Format: | ||
| ${formatInstruction} | ||
| ${emojiInstruction ? `\nEmoji Usage:\n${emojiInstruction}` : ''} | ||
| ${emojiInstruction ? `\nEmoji Usage:\n${emojiInstruction}` : ""} | ||
| ${formatSpecificInstructions} | ||
|
|
||
| Completed tasks: | ||
| ${taskSummaries} | ||
|
|
||
| ${request.customInstructions ? `Note: ${request.customInstructions}` : ''} | ||
| ${request.customInstructions ? `Note: ${request.customInstructions}` : ""} | ||
|
|
||
| CRITICAL: Output ONLY the raw changelog content. Do NOT include ANY introductory text, analysis, or explanation. Start directly with the changelog heading (## or #). No "Here's the changelog" or similar phrases.`; | ||
| } | ||
|
|
||
| /** | ||
| * Build changelog prompt from git commits | ||
| */ | ||
| export function buildGitPrompt( | ||
| request: ChangelogGenerationRequest, | ||
| commits: GitCommit[] | ||
| ): string { | ||
| export function buildGitPrompt(request: ChangelogGenerationRequest, commits: GitCommit[]): string { | ||
| const audienceInstruction = AUDIENCE_INSTRUCTIONS[request.audience]; | ||
| const formatInstruction = FORMAT_TEMPLATES[request.format](request.version, request.date); | ||
| const emojiInstruction = getEmojiInstructions(request.emojiLevel, request.format); | ||
|
|
||
| // Format commits for the prompt | ||
| // Include author info for github-release format | ||
| const commitLines = commits.map(commit => { | ||
| const hash = commit.hash; | ||
| const subject = commit.subject; | ||
| const author = commit.author; | ||
|
|
||
| // Detect conventional commit format: type(scope): message | ||
| const conventionalMatch = subject.match(/^(\w+)(?:\(([^)]+)\))?:\s*(.+)$/); | ||
| if (conventionalMatch) { | ||
| const [, type, scope, message] = conventionalMatch; | ||
| return `- ${hash} | ${type}${scope ? `(${scope})` : ''}: ${message} | by ${author}`; | ||
| } | ||
| return `- ${hash} | ${subject} | by ${author}`; | ||
| }).join('\n'); | ||
| const commitLines = commits | ||
| .map((commit) => { | ||
| const hash = commit.hash; | ||
| const subject = commit.subject; | ||
| const author = commit.author; | ||
|
|
||
| // Detect conventional commit format: type(scope): message | ||
| const conventionalMatch = subject.match(/^(\w+)(?:\(([^)]+)\))?:\s*(.+)$/); | ||
| if (conventionalMatch) { | ||
| const [, type, scope, message] = conventionalMatch; | ||
| return `- ${hash} | ${type}${scope ? `(${scope})` : ""}: ${message} | by ${author}`; | ||
| } | ||
| return `- ${hash} | ${subject} | by ${author}`; | ||
| }) | ||
| .join("\n"); | ||
|
|
||
| // Add context about branch/range if available | ||
| let sourceContext = ''; | ||
| let sourceContext = ""; | ||
| if (request.branchDiff) { | ||
| sourceContext = `These commits are from branch "${request.branchDiff.compareBranch}" that are not in "${request.branchDiff.baseBranch}".`; | ||
| } else if (request.gitHistory) { | ||
| switch (request.gitHistory.type) { | ||
| case 'recent': | ||
| case "recent": | ||
| sourceContext = `These are the ${commits.length} most recent commits.`; | ||
| break; | ||
| case 'since-date': | ||
| case "since-date": | ||
| sourceContext = `These are commits since ${request.gitHistory.sinceDate}.`; | ||
| break; | ||
| case 'tag-range': | ||
| sourceContext = `These are commits between tag "${request.gitHistory.fromTag}" and "${request.gitHistory.toTag || 'HEAD'}".`; | ||
| case "tag-range": | ||
| sourceContext = `These are commits between tag "${request.gitHistory.fromTag}" and "${request.gitHistory.toTag || "HEAD"}".`; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // Format-specific instructions | ||
| let formatSpecificInstructions = ''; | ||
| if (request.format === 'github-release') { | ||
| let formatSpecificInstructions = ""; | ||
| if (request.format === "github-release") { | ||
| formatSpecificInstructions = ` | ||
| For GitHub Release format, you MUST follow this structure: | ||
|
|
||
|
|
@@ -306,12 +308,12 @@ ${formatSpecificInstructions} | |
|
|
||
| Format: | ||
| ${formatInstruction} | ||
| ${emojiInstruction ? `\nEmoji Usage:\n${emojiInstruction}` : ''} | ||
| ${emojiInstruction ? `\nEmoji Usage:\n${emojiInstruction}` : ""} | ||
|
|
||
| Git commits (${commits.length} total): | ||
| ${commitLines} | ||
|
|
||
| ${request.customInstructions ? `Note: ${request.customInstructions}` : ''} | ||
| ${request.customInstructions ? `Note: ${request.customInstructions}` : ""} | ||
|
|
||
| CRITICAL: Output ONLY the raw changelog content. Do NOT include ANY introductory text, analysis, or explanation. Start directly with the changelog heading (## or #). No "Here's the changelog" or similar phrases. Intelligently group and summarize related commits - don't just list each commit individually. Only include sections that have actual changes.`; | ||
| } | ||
|
|
@@ -320,30 +322,53 @@ CRITICAL: Output ONLY the raw changelog content. Do NOT include ANY introductory | |
| * Create Python script for Claude generation | ||
| */ | ||
| export function createGenerationScript(prompt: string, claudePath: string): string { | ||
| // Validate claudePath is secure before using it in shell commands | ||
| if (!isSecurePath(claudePath)) { | ||
| throw new Error( | ||
| `Invalid Claude CLI path: path contains potentially dangerous characters. Path: ${claudePath}` | ||
| ); | ||
| } | ||
|
|
||
| // Convert prompt to base64 to avoid any string escaping issues in Python | ||
| const base64Prompt = Buffer.from(prompt, 'utf-8').toString('base64'); | ||
| const base64Prompt = Buffer.from(prompt, "utf-8").toString("base64"); | ||
|
|
||
| // Escape the claude path for Python string | ||
| const escapedClaudePath = claudePath.replace(/\\/g, '\\\\').replace(/'/g, "\\'"); | ||
| // Prepare the path for Python subprocess execution (handles Windows quoting rules) | ||
| const { commandPath, needsShell } = preparePythonSubprocessCommand(claudePath); | ||
|
|
||
| return ` | ||
| import subprocess | ||
| import sys | ||
| import base64 | ||
| import shlex | ||
|
|
||
| try: | ||
| # Decode the base64 prompt to avoid string escaping issues | ||
| prompt = base64.b64decode('${base64Prompt}').decode('utf-8') | ||
|
|
||
| # Use Claude Code CLI to generate | ||
| # stdin=DEVNULL prevents hanging when claude checks for interactive input | ||
| ${ | ||
| needsShell | ||
| ? `# On Windows with .cmd/.bat files, use shell=True with properly escaped prompt | ||
| # Use shlex.quote to safely escape the prompt for shell | ||
| escaped_prompt = shlex.quote(prompt) | ||
| command = ${commandPath} + ' -p ' + escaped_prompt + ' --output-format text --model haiku' | ||
| result = subprocess.run( | ||
| ['${escapedClaudePath}', '-p', prompt, '--output-format', 'text', '--model', 'haiku'], | ||
| command, | ||
| capture_output=True, | ||
| text=True, | ||
| stdin=subprocess.DEVNULL, | ||
| timeout=300, | ||
| shell=True | ||
| )` | ||
| : `result = subprocess.run( | ||
| ['${commandPath}', '-p', prompt, '--output-format', 'text', '--model', 'haiku'], | ||
| capture_output=True, | ||
| text=True, | ||
| stdin=subprocess.DEVNULL, | ||
| timeout=300 | ||
| ) | ||
| )` | ||
| } | ||
|
Comment on lines
+350
to
+371
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: Short answer: shlex.quote() produces POSIX-style single-quoted tokens (e.g. "'a b'") and is intended for Unix shells. cmd.exe does not treat single quotes as quoting characters (only double quotes are standard), so using shlex.quote() for commands executed by cmd.exe will leave the single-quote characters literal rather than grouping/escaping the argument. Use subprocess with a list (shell=False) or apply Windows/cmd quoting (double quotes and the cmd-specific escaping rules) instead. [1][2][3] Sources:
Replace On Windows with Use platform-aware escaping (e.g., double quotes with cmd-specific escaping rules) or avoid |
||
|
|
||
| if result.returncode == 0: | ||
| print(result.stdout) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Minor:
shleximported unconditionally but only used in shell mode.The
import shlexstatement at line 334 is in the base Python script, butshlex.quote()is only used whenneedsShellis true (line 346). This doesn't cause errors but adds a small unnecessary import in list mode.⚡ Optional optimization
Move the
import shlexinto the shell mode branch:🤖 Prompt for AI Agents