fix(ai-services): Prevent TTY errors during AI streaming output
The function used terminal manipulation functions (like , ) for the CLI streaming progress indicator. This caused errors when Task Master commands involving AI streaming were run in non-interactive terminals (e.g., via output redirection, some CI environments, or integrated terminals). This commit adds a check for to the condition that controls the display of the CLI progress indicator, ensuring these functions are only called when standard output is a fully interactive TTY.
This commit is contained in:
Eyal Toledano
@@ -34,8 +34,8 @@ While this document details the implementation of Task Master's **CLI commands**
|
||||
- **Command Handler Organization**:
|
||||
- ✅ DO: Keep action handlers concise and focused
|
||||
- ✅ DO: Extract core functionality to appropriate modules
|
||||
- ✅ DO: Have the action handler import and call the relevant function(s) from core modules (e.g., `task-manager.js`, `init.js`), passing the parsed `options`.
|
||||
- ✅ DO: Perform basic parameter validation (e.g., checking for required options) within the action handler or at the start of the called core function.
|
||||
- ✅ DO: Have the action handler import and call the relevant functions from core modules, like `task-manager.js` or `init.js`, passing the parsed `options`.
|
||||
- ✅ DO: Perform basic parameter validation, such as checking for required options, within the action handler or at the start of the called core function.
|
||||
- ❌ DON'T: Implement business logic in command handlers
|
||||
|
||||
## Best Practices for Removal/Delete Commands
|
||||
@@ -44,7 +44,7 @@ When implementing commands that delete or remove data (like `remove-task` or `re
|
||||
|
||||
- **Confirmation Prompts**:
|
||||
- ✅ **DO**: Include a confirmation prompt by default for destructive operations
|
||||
- ✅ **DO**: Provide a `--yes` or `-y` flag to skip confirmation for scripting/automation
|
||||
- ✅ **DO**: Provide a `--yes` or `-y` flag to skip confirmation, useful for scripting or automation
|
||||
- ✅ **DO**: Show what will be deleted in the confirmation message
|
||||
- ❌ **DON'T**: Perform destructive operations without user confirmation unless explicitly overridden
|
||||
|
||||
@@ -78,7 +78,7 @@ When implementing commands that delete or remove data (like `remove-task` or `re
|
||||
|
||||
- **File Path Handling**:
|
||||
- ✅ **DO**: Use `path.join()` to construct file paths
|
||||
- ✅ **DO**: Follow established naming conventions for tasks (e.g., `task_001.txt`)
|
||||
- ✅ **DO**: Follow established naming conventions for tasks, like `task_001.txt`
|
||||
- ✅ **DO**: Check if files exist before attempting to delete them
|
||||
- ✅ **DO**: Handle file deletion errors gracefully
|
||||
- ❌ **DON'T**: Construct paths with string concatenation
|
||||
@@ -166,10 +166,10 @@ When implementing commands that delete or remove data (like `remove-task` or `re
|
||||
- ✅ DO: Use descriptive, action-oriented names
|
||||
|
||||
- **Option Names**:
|
||||
- ✅ DO: Use kebab-case for long-form option names (`--output-format`)
|
||||
- ✅ DO: Provide single-letter shortcuts when appropriate (`-f, --file`)
|
||||
- ✅ DO: Use kebab-case for long-form option names, like `--output-format`
|
||||
- ✅ DO: Provide single-letter shortcuts when appropriate, like `-f, --file`
|
||||
- ✅ DO: Use consistent option names across similar commands
|
||||
- ❌ DON'T: Use different names for the same concept (`--file` in one command, `--path` in another)
|
||||
- ❌ DON'T: Use different names for the same concept, such as `--file` in one command and `--path` in another
|
||||
|
||||
```javascript
|
||||
// ✅ DO: Use consistent option naming
|
||||
@@ -181,7 +181,7 @@ When implementing commands that delete or remove data (like `remove-task` or `re
|
||||
.option('-p, --path <dir>', 'Output directory') // Should be --output
|
||||
```
|
||||
|
||||
> **Note**: Although options are defined with kebab-case (`--num-tasks`), Commander.js stores them internally as camelCase properties. Access them in code as `options.numTasks`, not `options['num-tasks']`.
|
||||
> **Note**: Although options are defined with kebab-case, like `--num-tasks`, Commander.js stores them internally as camelCase properties. Access them in code as `options.numTasks`, not `options['num-tasks']`.
|
||||
|
||||
- **Boolean Flag Conventions**:
|
||||
- ✅ DO: Use positive flags with `--skip-` prefix for disabling behavior
|
||||
@@ -210,7 +210,7 @@ When implementing commands that delete or remove data (like `remove-task` or `re
|
||||
- **Required Parameters**:
|
||||
- ✅ DO: Check that required parameters are provided
|
||||
- ✅ DO: Provide clear error messages when parameters are missing
|
||||
- ✅ DO: Use early returns with process.exit(1) for validation failures
|
||||
- ✅ DO: Use early returns with `process.exit(1)` for validation failures
|
||||
|
||||
```javascript
|
||||
// ✅ DO: Validate required parameters early
|
||||
@@ -221,7 +221,7 @@ When implementing commands that delete or remove data (like `remove-task` or `re
|
||||
```
|
||||
|
||||
- **Parameter Type Conversion**:
|
||||
- ✅ DO: Convert string inputs to appropriate types (numbers, booleans)
|
||||
- ✅ DO: Convert string inputs to appropriate types, such as numbers or booleans
|
||||
- ✅ DO: Handle conversion errors gracefully
|
||||
|
||||
```javascript
|
||||
@@ -254,7 +254,7 @@ When implementing commands that delete or remove data (like `remove-task` or `re
|
||||
const taskId = parseInt(options.id, 10);
|
||||
if (isNaN(taskId) || taskId <= 0) {
|
||||
console.error(chalk.red(`Error: Invalid task ID: ${options.id}. Task ID must be a positive integer.`));
|
||||
console.log(chalk.yellow('Usage example: task-master update-task --id=\'23\' --prompt=\'Update with new information.\nEnsure proper error handling.\''));
|
||||
console.log(chalk.yellow("Usage example: task-master update-task --id='23' --prompt='Update with new information.\\nEnsure proper error handling.'"));
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
@@ -392,9 +392,9 @@ When implementing commands that delete or remove data (like `remove-task` or `re
|
||||
process.on('uncaughtException', (err) => {
|
||||
// Handle Commander-specific errors
|
||||
if (err.code === 'commander.unknownOption') {
|
||||
const option = err.message.match(/'([^']+)'/)?.[1];
|
||||
const option = err.message.match(/'([^']+)'/)?.[1]; // Safely extract option name
|
||||
console.error(chalk.red(`Error: Unknown option '${option}'`));
|
||||
console.error(chalk.yellow(`Run 'task-master <command> --help' to see available options`));
|
||||
console.error(chalk.yellow("Run 'task-master <command> --help' to see available options"));
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
@@ -464,9 +464,9 @@ When implementing commands that delete or remove data (like `remove-task` or `re
|
||||
.option('-f, --file <path>', 'Path to the tasks file', 'tasks/tasks.json')
|
||||
.option('-p, --parent <id>', 'ID of the parent task (required)')
|
||||
.option('-i, --task-id <id>', 'Existing task ID to convert to subtask')
|
||||
.option('-t, --title <title>', 'Title for the new subtask (when not converting)')
|
||||
.option('-d, --description <description>', 'Description for the new subtask (when not converting)')
|
||||
.option('--details <details>', 'Implementation details for the new subtask (when not converting)')
|
||||
.option('-t, --title <title>', 'Title for the new subtask, required if not converting')
|
||||
.option('-d, --description <description>', 'Description for the new subtask, optional')
|
||||
.option('--details <details>', 'Implementation details for the new subtask, optional')
|
||||
.option('--dependencies <ids>', 'Comma-separated list of subtask IDs this subtask depends on')
|
||||
.option('--status <status>', 'Initial status for the subtask', 'pending')
|
||||
.option('--skip-generate', 'Skip regenerating task files')
|
||||
@@ -489,8 +489,8 @@ When implementing commands that delete or remove data (like `remove-task` or `re
|
||||
.command('remove-subtask')
|
||||
.description('Remove a subtask from its parent task, optionally converting it to a standalone task')
|
||||
.option('-f, --file <path>', 'Path to the tasks file', 'tasks/tasks.json')
|
||||
.option('-i, --id <id>', 'ID of the subtask to remove in format "parentId.subtaskId" (required)')
|
||||
.option('-c, --convert', 'Convert the subtask to a standalone task')
|
||||
.option('-i, --id <id>', 'ID of the subtask to remove in format parentId.subtaskId, required')
|
||||
.option('-c, --convert', 'Convert the subtask to a standalone task instead of deleting')
|
||||
.option('--skip-generate', 'Skip regenerating task files')
|
||||
.action(async (options) => {
|
||||
// Implementation with detailed error handling
|
||||
@@ -513,7 +513,8 @@ When implementing commands that delete or remove data (like `remove-task` or `re
|
||||
// ✅ DO: Implement version checking function
|
||||
async function checkForUpdate() {
|
||||
// Implementation details...
|
||||
return { currentVersion, latestVersion, needsUpdate };
|
||||
// Example return structure:
|
||||
return { currentVersion, latestVersion, updateAvailable };
|
||||
}
|
||||
|
||||
// ✅ DO: Implement semantic version comparison
|
||||
@@ -553,7 +554,7 @@ When implementing commands that delete or remove data (like `remove-task` or `re
|
||||
|
||||
// After command execution, check if an update is available
|
||||
const updateInfo = await updateCheckPromise;
|
||||
if (updateInfo.needsUpdate) {
|
||||
if (updateInfo.updateAvailable) {
|
||||
displayUpgradeNotification(updateInfo.currentVersion, updateInfo.latestVersion);
|
||||
}
|
||||
} catch (error) {
|
||||
|
||||
Reference in New Issue
Block a user