chore: apply requested coderabbit changes

This commit is contained in:
Ralph Khreish
2025-10-16 21:47:00 +02:00
parent 0bbe7f7291
commit 1698ee168d
4 changed files with 88 additions and 96 deletions

View File

@@ -59,6 +59,7 @@ export class NextCommand extends Command {
* Execute the next command * Execute the next command
*/ */
private async executeCommand(options: NextCommandOptions): Promise<void> { private async executeCommand(options: NextCommandOptions): Promise<void> {
let hasError = false;
try { try {
// Validate options (throws on invalid options) // Validate options (throws on invalid options)
this.validateOptions(options); this.validateOptions(options);
@@ -77,11 +78,17 @@ export class NextCommand extends Command {
this.displayResults(result, options); this.displayResults(result, options);
} }
} catch (error: any) { } catch (error: any) {
displayError(error); hasError = true;
displayError(error, { skipExit: true });
} finally { } finally {
// Always clean up resources, even on error // Always clean up resources, even on error
await this.cleanup(); await this.cleanup();
} }
// Exit after cleanup completes
if (hasError) {
process.exit(1);
}
} }
/** /**

View File

@@ -86,6 +86,7 @@ export class SetStatusCommand extends Command {
private async executeCommand( private async executeCommand(
options: SetStatusCommandOptions options: SetStatusCommandOptions
): Promise<void> { ): Promise<void> {
let hasError = false;
try { try {
// Validate required options // Validate required options
if (!options.id) { if (!options.id) {
@@ -137,6 +138,7 @@ export class SetStatusCommand extends Command {
newStatus: result.newStatus newStatus: result.newStatus
}); });
} catch (error: any) { } catch (error: any) {
hasError = true;
if (options.format === 'json') { if (options.format === 'json') {
const errorMessage = error?.getSanitizedDetails const errorMessage = error?.getSanitizedDetails
? error.getSanitizedDetails().message ? error.getSanitizedDetails().message
@@ -152,14 +154,13 @@ export class SetStatusCommand extends Command {
timestamp: new Date().toISOString() timestamp: new Date().toISOString()
}) })
); );
process.exit(1);
} else if (!options.silent) { } else if (!options.silent) {
// Show which task failed with context // Show which task failed with context
console.error(chalk.red(`\nFailed to update task ${taskId}:`)); console.error(chalk.red(`\nFailed to update task ${taskId}:`));
displayError(error); displayError(error, { skipExit: true });
} else {
process.exit(1);
} }
// Don't exit here - let finally block clean up first
break;
} }
} }
@@ -176,15 +177,13 @@ export class SetStatusCommand extends Command {
// Display results // Display results
this.displayResults(this.lastResult, options); this.displayResults(this.lastResult, options);
} catch (error: any) { } catch (error: any) {
hasError = true;
if (options.format === 'json') { if (options.format === 'json') {
const errorMessage = const errorMessage =
error instanceof Error ? error.message : 'Unknown error occurred'; error instanceof Error ? error.message : 'Unknown error occurred';
console.log(JSON.stringify({ success: false, error: errorMessage })); console.log(JSON.stringify({ success: false, error: errorMessage }));
process.exit(1);
} else if (!options.silent) { } else if (!options.silent) {
displayError(error); displayError(error, { skipExit: true });
} else {
process.exit(1);
} }
} finally { } finally {
// Clean up resources // Clean up resources
@@ -192,6 +191,11 @@ export class SetStatusCommand extends Command {
await this.tmCore.close(); await this.tmCore.close();
} }
} }
// Exit after cleanup completes
if (hasError) {
process.exit(1);
}
} }
/** /**

View File

@@ -58,6 +58,8 @@ export function displayCommandHeader(
} }
// Get file path for display (only for file storage) // Get file path for display (only for file storage)
// Note: The file structure is fixed for file storage and won't change.
// This is a display-only relative path, not used for actual file operations.
const filePath = const filePath =
storageType === 'file' && tmCore storageType === 'file' && tmCore
? `.taskmaster/tasks/tasks.json` ? `.taskmaster/tasks/tasks.json`

View File

@@ -158,21 +158,7 @@ export class ApiStorage implements IStorage {
await this.ensureInitialized(); await this.ensureInitialized();
try { try {
const authManager = AuthManager.getInstance(); const context = this.ensureBriefSelected('loadTasks');
const context = authManager.getContext();
// If no brief is selected in context, throw an error
if (!context?.briefId) {
throw new TaskMasterError(
'No brief selected',
ERROR_CODES.NO_BRIEF_SELECTED,
{
operation: 'loadTasks',
userMessage:
'No brief selected. Please select a brief first using: tm context brief <brief-id> or tm context brief <brief-url>'
}
);
}
// Load tasks from the current brief context with filters pushed to repository // Load tasks from the current brief context with filters pushed to repository
const tasks = await this.retryOperation(() => const tasks = await this.retryOperation(() =>
@@ -187,20 +173,11 @@ export class ApiStorage implements IStorage {
return tasks; return tasks;
} catch (error) { } catch (error) {
// If it's already a NO_BRIEF_SELECTED error, don't wrap it this.wrapError(error, 'Failed to load tasks from API', {
if ( operation: 'loadTasks',
error instanceof TaskMasterError && tag,
error.is(ERROR_CODES.NO_BRIEF_SELECTED) context: 'brief-based loading'
) { });
throw error;
}
throw new TaskMasterError(
'Failed to load tasks from API',
ERROR_CODES.STORAGE_ERROR,
{ operation: 'loadTasks', tag, context: 'brief-based loading' },
error as Error
);
} }
} }
@@ -251,40 +228,17 @@ export class ApiStorage implements IStorage {
await this.ensureInitialized(); await this.ensureInitialized();
try { try {
const authManager = AuthManager.getInstance(); this.ensureBriefSelected('loadTask');
const context = authManager.getContext();
// If no brief is selected in context, throw an error
if (!context?.briefId) {
throw new TaskMasterError(
'No brief selected',
ERROR_CODES.NO_BRIEF_SELECTED,
{
operation: 'loadTask',
userMessage:
'No brief selected. Please select a brief first using: tm context brief <brief-id> or tm context brief <brief-url>'
}
);
}
return await this.retryOperation(() => return await this.retryOperation(() =>
this.repository.getTask(this.projectId, taskId) this.repository.getTask(this.projectId, taskId)
); );
} catch (error) { } catch (error) {
// If it's already a NO_BRIEF_SELECTED error, don't wrap it this.wrapError(error, 'Failed to load task from API', {
if ( operation: 'loadTask',
error instanceof TaskMasterError && taskId,
error.is(ERROR_CODES.NO_BRIEF_SELECTED) tag
) { });
throw error;
}
throw new TaskMasterError(
'Failed to load task from API',
ERROR_CODES.STORAGE_ERROR,
{ operation: 'loadTask', taskId, tag },
error as Error
);
} }
} }
@@ -548,21 +502,7 @@ export class ApiStorage implements IStorage {
await this.ensureInitialized(); await this.ensureInitialized();
try { try {
const authManager = AuthManager.getInstance(); this.ensureBriefSelected('updateTaskStatus');
const context = authManager.getContext();
// If no brief is selected in context, throw an error
if (!context?.briefId) {
throw new TaskMasterError(
'No brief selected',
ERROR_CODES.NO_BRIEF_SELECTED,
{
operation: 'updateTaskStatus',
userMessage:
'No brief selected. Please select a brief first using: tm context brief <brief-id> or tm context brief <brief-url>'
}
);
}
const existingTask = await this.retryOperation(() => const existingTask = await this.retryOperation(() =>
this.repository.getTask(this.projectId, taskId) this.repository.getTask(this.projectId, taskId)
@@ -600,20 +540,12 @@ export class ApiStorage implements IStorage {
taskId taskId
}; };
} catch (error) { } catch (error) {
// If it's already a NO_BRIEF_SELECTED error, don't wrap it this.wrapError(error, 'Failed to update task status via API', {
if ( operation: 'updateTaskStatus',
error instanceof TaskMasterError && taskId,
error.is(ERROR_CODES.NO_BRIEF_SELECTED) newStatus,
) { tag
throw error; });
}
throw new TaskMasterError(
'Failed to update task status via API',
ERROR_CODES.STORAGE_ERROR,
{ operation: 'updateTaskStatus', taskId, newStatus, tag },
error as Error
);
} }
} }
@@ -831,6 +763,29 @@ export class ApiStorage implements IStorage {
} }
} }
/**
* Ensure a brief is selected in the current context
* @returns The current auth context with a valid briefId
*/
private ensureBriefSelected(operation: string) {
const authManager = AuthManager.getInstance();
const context = authManager.getContext();
if (!context?.briefId) {
throw new TaskMasterError(
'No brief selected',
ERROR_CODES.NO_BRIEF_SELECTED,
{
operation,
userMessage:
'No brief selected. Please select a brief first using: tm context brief <brief-id> or tm context brief <brief-url>'
}
);
}
return context;
}
/** /**
* Retry an operation with exponential backoff * Retry an operation with exponential backoff
*/ */
@@ -849,4 +804,28 @@ export class ApiStorage implements IStorage {
throw error; throw error;
} }
} }
/**
* Wrap an error unless it's already a NO_BRIEF_SELECTED error
*/
private wrapError(
error: unknown,
message: string,
context: Record<string, unknown>
): never {
// If it's already a NO_BRIEF_SELECTED error, don't wrap it
if (
error instanceof TaskMasterError &&
error.is(ERROR_CODES.NO_BRIEF_SELECTED)
) {
throw error;
}
throw new TaskMasterError(
message,
ERROR_CODES.STORAGE_ERROR,
context,
error as Error
);
}
} }