Conversation
Removes `"version": ""` from internal workspace packages to fix changesets validation errors. These private packages don't need explicit version fields as they use workspace protocol. Fixes validation errors: - Package "task-master-ai" must depend on the current version of "@tm/ai-sdk-provider-grok-cli": "" vs "*" - Package "task-master-ai" must depend on the current version of "@tm/cli": "" vs "*" - Package "@tm/cli" must depend on the current version of "@tm/core": "" vs "*" - Package "extension" must depend on the current version of "@tm/core": "" vs "*" - Package "@tm/mcp" must depend on the current version of "@tm/core": "" vs "*" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 7776728 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Crunchyman-ralph
temporarily deployed
to
extension-release
GitHub Actions
Inactive
WalkthroughReplaces the legacy TaskMasterCore facade with a new TmCore facade and per-domain facades (tasks, auth, workflow, git, config, integration); reorganizes tm-core exports and many import paths; adds a temporary tm-bridge package and remote-update flow; updates CLI/MCP/extension scripts to use createTmCore/TmCore; adjusts CI, changelog, and pre-release config. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI / Script
participant TmCore as createTmCore / TmCore
participant Tasks as TasksDomain
participant Storage as Storage Adapter
participant Remote as Hamster API
CLI->>TmCore: createTmCore({ projectPath })
TmCore-->>CLI: TmCore instance
CLI->>TmCore: tmCore.tasks.get(taskId)
TmCore->>Tasks: get(taskId)
Tasks->>Storage: get task
Storage-->>Tasks: task (+ isSubtask)
Tasks-->>CLI: task data
CLI->>TmCore: tmCore.tasks.updateWithPrompt(taskId, prompt)
TmCore->>Tasks: updateWithPrompt(...)
Tasks->>Storage: updateTaskWithPrompt(...)
alt API storage
Storage->>Remote: PATCH /ai/api/v1/tasks/{taskId}/prompt
Remote-->>Storage: success
Storage-->>Tasks: success
else file storage
Storage-->>Tasks: throws / not supported
Tasks-->>CLI: fallback / instruct client-side update
end
Tasks-->>CLI: operation complete
sequenceDiagram
participant Script as legacy script
participant Bridge as tm-bridge.tryUpdateViaRemote
participant TmCore as createTmCore
participant Storage as tmCore.tasks.getStorageType
participant Remote as Hamster API
Script->>Bridge: tryUpdateViaRemote({taskId,prompt,projectRoot})
Bridge->>TmCore: createTmCore({projectPath})
TmCore-->>Bridge: TmCore
Bridge->>Storage: tmCore.tasks.getStorageType()
Storage-->>Bridge: 'api' or 'file'
alt api
Bridge->>Remote: delegate update (PATCH)
Remote-->>Bridge: result
Bridge-->>Script: RemoteUpdateResult (short-circuit)
else file
Bridge-->>Script: null (fallback)
Script->>File: perform file-based update
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Areas requiring special attention:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2).changeset/*.md📄 CodeRabbit inference engine (.cursor/rules/changeset.mdc)
Files:
.changeset/*📄 CodeRabbit inference engine (.cursor/rules/new_features.mdc)
Files:
🪛 LanguageTool.changeset/four-bugs-occur.md[style] ~7-~7: Consider a more concise word here. (IN_ORDER_TO_PREMIUM) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
apps/extension/src/services/terminal-manager.ts (1)
72-75: Shell injection risk when passing executionOutput to terminal.Quote-safety is not guaranteed with raw interpolation. Use JSON.stringify to safely pass arbitrary content as a single CLI arg.
- const command = `claude "${startResult.executionOutput}"`; + const safeArg = JSON.stringify(startResult.executionOutput); + const command = `claude ${safeArg}`; terminal.sendText(command);packages/tm-core/src/modules/storage/adapters/file-storage/file-operations.ts (1)
40-56: Locking is racy with multiple waiters; concurrent writes can interleave.Two or more callers arriving after the first write completes can each see no active lock and start parallel writes. Chain the lock promise to serialize writes safely.
- // Use file locking to prevent concurrent writes - const lockKey = filePath; - const existingLock = this.fileLocks.get(lockKey); - - if (existingLock) { - await existingLock; - } - - const lockPromise = this.performAtomicWrite(filePath, data); - this.fileLocks.set(lockKey, lockPromise); - - try { - await lockPromise; - } finally { - this.fileLocks.delete(lockKey); - } + // Serialize writes by chaining on the previous promise + const lockKey = filePath; + const prev = this.fileLocks.get(lockKey) ?? Promise.resolve(); + const next = prev.then(() => this.performAtomicWrite(filePath, data)); + this.fileLocks.set(lockKey, next); + try { + await next; + } finally { + // Only clear if we're still the tip of the chain + if (this.fileLocks.get(lockKey) === next) { + this.fileLocks.delete(lockKey); + } + }mcp-server/src/core/direct-functions/update-task-by-id.js (1)
141-156: Use standardized error codes across MCP direct functionsCodes should match the shared set: INPUT_VALIDATION_ERROR, FILE_NOT_FOUND_ERROR, CORE_FUNCTION_ERROR, UNEXPECTED_ERROR. Replace UPDATE_TASK_CORE_ERROR and DIRECT_FUNCTION_SETUP_ERROR to maintain consistency.
- return { - success: false, - error: { - code: 'UPDATE_TASK_CORE_ERROR', - message: error.message || 'Unknown error updating task' - } - }; + return { + success: false, + error: { + code: 'CORE_FUNCTION_ERROR', + message: error.message || 'Unknown error updating task' + } + };- return { - success: false, - error: { - code: 'DIRECT_FUNCTION_SETUP_ERROR', - message: error.message || 'Unknown setup error' - } - }; + return { + success: false, + error: { + code: 'UNEXPECTED_ERROR', + message: error.message || 'Unknown setup error' + } + };As per coding guidelines.
scripts/modules/commands.js (1)
1083-1107: Label-number task IDs are accepted by validation but rejected at runtimeThe validation at lines 1083-1107 accepts both numeric and label-number formats (e.g., ham-123), but
updateTaskByIdexplicitly rejects non-numeric IDs with: "For file storage, taskId must be a positive integer." This creates a false-positive UX where the command appears valid but fails during execution.Additionally, at line 2204 in commands.js,
parseInt(validatedParams.saveToId, 10)converts label-number IDs to NaN before passing toupdateTaskById, compounding the issue.Fixes required:
- Either remove label-number ID support from validation (lines 1083-1090) to prevent confusion, or
- Extend
updateTaskByIdto support label-number IDs throughout the update pipeline (currently requires numeric IDs only for file storage)- Add test coverage for attempted label-number ID usage
packages/tm-core/src/modules/config/services/runtime-state-manager.service.spec.ts (1)
6-17: Mocking the wrong module: switch tonode:fs/promises.You import from
node:fs/promisesbut mocknode:fs. This won’t intercept calls; tests may hit real FS.-import fs from 'node:fs/promises'; -vi.mock('node:fs', () => ({ - promises: { - readFile: vi.fn(), - writeFile: vi.fn(), - mkdir: vi.fn(), - unlink: vi.fn() - } -})); +import fs from 'node:fs/promises'; +vi.mock('node:fs/promises', () => ({ + default: { + readFile: vi.fn(), + writeFile: vi.fn(), + mkdir: vi.fn(), + unlink: vi.fn() + } +}));scripts/modules/task-manager/update-task-by-id.js (1)
443-446: Bug: Preserving ID may coerce it to string. UsenumericTaskId.
updatedTask.id = taskIdcan change type from number→string; downstream lookups expect numeric.- if (updatedTask.id !== taskId) { - report('warn', `AI changed task ID. Restoring original ID ${taskId}.`); - updatedTask.id = taskId; - } + if (updatedTask.id !== numericTaskId) { + report('warn', `AI changed task ID. Restoring original ID ${numericTaskId}.`); + updatedTask.id = numericTaskId; + }apps/cli/src/commands/set-status.command.ts (2)
16-24: Include 'completed' in allowed statuses (aligns with core types)CLI currently rejects 'completed' though it is used elsewhere. Add it.
const VALID_TASK_STATUSES: TaskStatus[] = [ 'pending', 'in-progress', 'done', 'deferred', 'cancelled', 'blocked', - 'review' + 'review', + 'completed' ];
111-115: Fix success flag, display results only on success, null-safe tmCore usage, and remove empty finally block
successis hardcoded true even whenhasErroris true. Use!hasErrorand skip displaying results on errors.- Capture tmCore in a local const to satisfy null-safety checks.
- Remove the empty finally block; add proper cleanup to call
close()on tmCore.- // Initialize TaskMaster core - this.tmCore = await createTmCore({ + // Initialize core + const tmCore = await createTmCore({ projectPath: options.project || process.cwd() }); + this.tmCore = tmCore; @@ - const result = await this.tmCore.tasks.updateStatus( + const result = await tmCore.tasks.updateStatus( taskId, options.status @@ - this.lastResult = { - success: true, - updatedTasks, - storageType: this.tmCore.tasks.getStorageType() - }; + this.lastResult = { + success: !hasError, + updatedTasks, + storageType: tmCore.tasks.getStorageType() + }; @@ - // Display results - this.displayResults(this.lastResult, options); + // Display results only on success + if (!hasError) { + this.displayResults(this.lastResult, options); + } @@ - // Clean up resources - if (this.tmCore) { - } + // Clean up resources + try { + await tmCore.close(); + } catch {}packages/tm-core/src/modules/tasks/services/task-loader.service.ts (1)
71-76: Add auto-initialization guard to TaskService.getTask() for consistencyTaskService.getTask() accesses storage without ensuring initialization, unlike updateTask, updateTaskWithPrompt, and updateTaskStatus which all include auto-initialization checks. This creates a null reference risk when TaskLoaderService calls getTask() without prior initialization.
Apply the same auto-initialization pattern used in other methods:
async getTask(taskId: string, tag?: string): Promise<Task | null> { // Use provided tag or get active tag const activeTag = tag || this.getActiveTag(); + // Auto-initialize if needed + if (!this.initialized) { + await this.initialize(); + } try { // Delegate to storage layer which handles the specific logic for tasks vs subtasks return await this.storage.loadTask(String(taskId), activeTag);apps/cli/src/commands/autopilot.command.ts (2)
155-159: Fix task ID validation: allow alphanumeric IDs and only a single subtask dotCurrent regex blocks IDs like HAM-123 and allows multi-depth IDs (e.g., 1.2.3) that downstream code doesn’t support. Align with TasksDomain.get semantics.
Apply this diff:
- // Task ID should be a number or number.number format (e.g., "1" or "1.2") - const taskIdPattern = /^\d+(\.\d+)*$/; + // Task ID: alphanumeric/hyphen/underscore; optional single ".sub" part (e.g., "1", "1.2", "HAM-123", "HAM-123.2") + const taskIdPattern = /^[A-Za-z0-9_-]+(?:\.[A-Za-z0-9_-]+)?$/;
173-184: Don’t swallow domain errors from tm-coreCatching and returning null masks actionable user-facing errors (e.g., NO_BRIEF_SELECTED). Let TaskMasterError bubble so handleError can present sanitized details.
Apply this diff:
private async loadTask(taskId: string): Promise<Task | null> { if (!this.tmCore) { throw new Error('TmCore not initialized'); } - - try { - const { task } = await this.tmCore.tasks.get(taskId); - return task; - } catch (error) { - return null; - } + const { task } = await this.tmCore.tasks.get(taskId); + return task; }
🧹 Nitpick comments (53)
packages/tm-core/src/modules/ui/index.ts (1)
1-8: Placeholder approach is appropriate, but expand TODO guidance for clarity.This placeholder aligns well with the modular TmCore refactoring strategy described in the AI summary. However, the TODO comment and commented export could provide more specific guidance for implementers.
Consider enhancing the TODO to specify what UI exports should eventually be included:
/** * @fileoverview UI domain - Placeholder for future migration * This module will handle UI components and rendering * when migrated from scripts/modules/ui.js */ -// TODO: Migrate ui.js from scripts/modules/ -// export * from './components/index.js'; +// TODO: Migrate UI components from scripts/modules/ui.js +// Expected exports: component utilities, formatting helpers, rendering logic +// export * from './components/index.js'; +// export * from './formatters/index.js';This expansion gives future developers clearer context about the scope of UI domain responsibilities.
update-task-migration-plan.md (3)
255-300: Add language identifier to package structure code block.The fenced code block starting at line 255 should specify a language for syntax highlighting. While this is a structural representation rather than executable code, marking it as
textor similar improves rendering and linting compliance.Apply this diff to add a language specifier:
-``` +```text packages/tm-core/ src/
887-934: Add language identifier to class diagram code block.The ASCII class diagram at line 887 lacks a language specifier. Mark it as
textorasciifor consistency and proper markdown linting.Apply this diff:
-``` +```text ┌─────────────────────────┐ │ UpdateTaskService │ ◄─── Main Orchestrator
1-1410: Comprehensive migration plan with sound architectural patterns.This is a well-structured, thorough migration plan that successfully unifies two disparate commands (
update-tasksandupdate-subtask-by-id) into a cohesive system. The design employs established patterns (Strategy, Factory, Template Method, Dependency Injection) appropriate for this domain.Strengths:
- Functionality checklist is exhaustive and covers both normal flows and edge cases
- Phased implementation approach (11 phases) provides clear, incremental milestones
- Architecture balances modularity (ContextBuilderService, PromptBuilderService, DataMergerService) with cohesion
- Decision to preserve existing integrations (ContextGatherer, FuzzyTaskSearch, PromptManager) reduces rework
- Strategy pattern elegantly handles the three update modes (bulk, single, subtask)
- Testing strategy and risk mitigation sections reflect operational maturity
Minor considerations:
- The success criteria checklist (lines 1398–1410) is primarily a task checklist; consider converting to a validation/verification checklist with measurable outcomes (e.g., "Zero regressions vs. legacy implementation in integration tests")
- Phase 11 (Cleanup) references removing old files "in future version" — ensure a deprecation period is enforced via versioning strategy and documented in release notes
packages/tm-core/src/modules/ai/index.ts (1)
5-6: Update the comment to match the actual export.The comment on line 5 states "Export all from AI module" but the export now sources from
./providers/index.js. The comment should be updated to accurately reflect what is being exported.Apply this diff to update the comment:
-// Export all from AI module +// Export all provider modules export * from './providers/index.js';packages/tm-bridge/package.json (3)
18-22: Use workspace protocol for internal dependency.Replace "@tm/core": "*" with a workspace protocol to avoid accidental semver drift and ensure local linking.
- "@tm/core": "*", + "@tm/core": "workspace:*",Based on learnings.
8-10: Expose package.json in exports for tooling.Add an export for "./package.json" to improve Node/tooling resolution.
"exports": { - ".": "./src/index.ts" + ".": "./src/index.ts", + "./package.json": "./package.json" },Based on learnings.
28-31: Optional: mark as side‑effect free.If this package only re-exports pure functions/types, set sideEffects to false for better tree-shaking.
"keywords": ["temporary", "bridge", "migration"], + "sideEffects": false,apps/extension/src/services/terminal-manager.ts (3)
107-114: Guard against concurrent double-initialization of tm-core.Add a single-flight initializer to prevent racing createTmCore() calls.
export class TerminalManager { private terminals = new Map<string, vscode.Terminal>(); private tmCore?: TmCore; + private initializingCore?: Promise<void>; @@ private async initializeCore(): Promise<void> { - if (!this.tmCore) { - const workspaceRoot = vscode.workspace.workspaceFolders?.[0]?.uri.fsPath; - if (!workspaceRoot) { - throw new Error('No workspace folder found'); - } - this.tmCore = await createTmCore({ projectPath: workspaceRoot }); - } + if (this.tmCore) return; + if (!this.initializingCore) { + const workspaceRoot = vscode.workspace.workspaceFolders?.[0]?.uri.fsPath; + if (!workspaceRoot) throw new Error('No workspace folder found'); + this.initializingCore = (async () => { + this.tmCore = await createTmCore({ projectPath: workspaceRoot }); + })().finally(() => { + this.initializingCore = undefined; + }); + } + await this.initializingCore; }
127-132: Dispose terminals when cleaning up by task.Avoid leaks by disposing before deleting.
cleanupTerminal(taskId: string): void { const terminal = this.terminals.get(taskId); if (terminal) { + try { + terminal.dispose(); + } catch (error) { + this.logger.error('Failed to dispose terminal:', error); + } this.terminals.delete(taskId); } }
147-150: Confirm tm-core lifecycle; optionally call close().If TmCore exposes close() or similar, call it to release watchers/resources.
// Clear tm-core reference (no explicit cleanup needed) if (this.tmCore) { - this.tmCore = undefined; + try { + // @ts-expect-error: optional lifecycle method depending on implementation + await (this.tmCore as any).close?.(); + } catch (e) { + this.logger.debug('tm-core close() not required/available'); + } finally { + this.tmCore = undefined; + } }Please verify whether close() is intentionally removed in the new API.
packages/tm-core/src/modules/auth/services/supabase-session-storage.ts (2)
27-41: Strongly type Supabase session to reduce parse/shape errors.Use Supabase’s Session type instead of any in build/parse helpers.
-import type { SupportedStorage } from '@supabase/supabase-js'; +import type { SupportedStorage, Session } from '@supabase/supabase-js'; @@ - private buildSessionFromCredentials(credentials: AuthCredentials): any { + private buildSessionFromCredentials(credentials: AuthCredentials): Session { @@ - const session = { + const session: Session = {
78-86: Be explicit about allowExpired to document intent.Calling getCredentials({ allowExpired: true }) makes the refresh behavior obvious and resilient if defaults ever change.
- const credentials = this.store.getCredentials(); + const credentials = this.store.getCredentials({ allowExpired: true });Based on learnings.
packages/tm-core/src/modules/ai/providers/base-provider.ts (1)
291-298: Improve retry/error classification using HTTP status if available.Leverage error.status/response.status to treat 5xx/408 as retryable and map codes precisely.
protected shouldRetry(error: unknown, attempt: number): boolean { if (attempt >= this.getMaxRetries()) { return false; } - - return this.isRetryableError(error); + return this.isRetryableError(error); } @@ protected getErrorCode(error: unknown): string { - if (this.isRateLimitError(error)) return ERROR_CODES.API_ERROR; - if (this.isTimeoutError(error)) return ERROR_CODES.NETWORK_ERROR; - if (this.isNetworkError(error)) return ERROR_CODES.NETWORK_ERROR; - - if (error instanceof Error && error.message.includes('401')) { - return ERROR_CODES.AUTHENTICATION_ERROR; - } - - return ERROR_CODES.PROVIDER_ERROR; + const status = this.getHttpStatus(error); + if (this.isRateLimitError(error) || status === 429) return ERROR_CODES.API_ERROR; + if (this.isTimeoutError(error) || status === 408) return ERROR_CODES.NETWORK_ERROR; + if (this.isNetworkError(error)) return ERROR_CODES.NETWORK_ERROR; + if (status === 401) return ERROR_CODES.AUTHENTICATION_ERROR; + if (status && status >= 500) return ERROR_CODES.API_ERROR; + return ERROR_CODES.PROVIDER_ERROR; } + + // Extract HTTP status from common error shapes without changing providers + protected getHttpStatus(error: unknown): number | undefined { + const anyErr = error as any; + return ( + anyErr?.status ?? + anyErr?.response?.status ?? + anyErr?.cause?.status ?? + undefined + ); + }Also applies to: 372-382
packages/tm-core/src/modules/storage/adapters/file-storage/format-handler.ts (1)
205-217: Preserve existing metadata tags when converting to standard format.Avoid dropping tags already present in metadata.
private convertToStandardFormat( tasks: Task[], metadata: TaskMetadata, tag?: string ): FileStorageData { - return { - tasks, - metadata: { - ...metadata, - tags: tag ? [tag] : [] - } - }; + const existing = Array.isArray(metadata.tags) ? metadata.tags : []; + const merged = tag ? [...existing, tag] : existing; + const tags = Array.from(new Set(merged)); + return { + tasks, + metadata: { + ...metadata, + tags + } + }; }packages/tm-core/src/modules/workflow/services/workflow-activity-logger.ts (2)
116-121: Be defensive on timestamp parsing.If
event.timestampis missing/invalid,toISOString()will throw. Fall back tonew Date().toISOString()to avoid dropping events.- const ts = - (event.timestamp as any) instanceof Date - ? (event.timestamp as Date).toISOString() - : new Date(event.timestamp as any).toISOString(); + const tsUnsafe = + (event.timestamp as any) instanceof Date + ? (event.timestamp as Date).toISOString() + : new Date(event.timestamp as any).toISOString(); + const ts = Number.isNaN(Date.parse(tsUnsafe)) + ? new Date().toISOString() + : tsUnsafe;
123-131: Don’t allow event.data to override reserved fields.Spreading
event.datalast can overwritetype,phase, etc. Ensure reserved fields win or nest payload under adatakey.- const activityEvent: Omit<ActivityEvent, 'timestamp'> = { - type: event.type, - phase: event.phase, - tddPhase: event.tddPhase, - subtaskId: event.subtaskId, - // Event timestamp kept as ISO for readability; storage layer adds its own "timestamp" - eventTimestamp: ts, - ...(event.data || {}) - }; + const { type: _t, phase: _p, tddPhase: _tp, subtaskId: _sid, eventTimestamp: _ets, ...rest } = + (event.data ?? {}) as Record<string, unknown>; + const activityEvent: Omit<ActivityEvent, 'timestamp'> = { + ...rest, // non-reserved fields first + type: event.type, + phase: event.phase, + tddPhase: event.tddPhase, + subtaskId: event.subtaskId, + eventTimestamp: ts + };packages/tm-core/src/modules/commands/index.ts (1)
1-8: Placeholder file for future command migration.This placeholder is appropriate for the ongoing architectural refactor. The TODO clearly documents the migration source.
Do you want me to create a tracking issue for the command migration from
scripts/modules/commands.jsto this module? Based on learnings, this should export aregisterCommandsfunction for clean CLI setup.mcp-server/src/core/direct-functions/update-subtask-by-id.js (1)
132-140: Consider parsing parentId to integer for consistency.The extracted
parentIdis currently a string fromsplit('.'). Other similar code in the codebase consistently parses parent IDs to integers usingparseInt(parentIdStr, 10)(seescripts/modules/task-manager/update-subtask-by-id.jsline 129,remove-subtask.jsline 38, andadd-subtask.jsline 94).Apply this diff for consistency:
-const parentId = subtaskIdStr.split('.')[0]; +const parentId = parseInt(subtaskIdStr.split('.')[0], 10);packages/tm-core/src/modules/auth/managers/auth-manager.ts (1)
31-31: Consider the visibility change from private to public.The
supabaseClientfield has been changed fromprivatetopublic, which exposes internal implementation details. While this may be intentional to support domain facade access patterns in the new TmCore architecture, it does break encapsulation.If external access is required, consider exposing only the necessary operations through dedicated public methods rather than exposing the entire client instance.
packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts (1)
375-390: Use a structured/typed error for unsupported prompt updatesThrowing a plain Error makes upstream handling brittle. Add a code/name for programmatic checks.
async updateTaskWithPrompt( _taskId: string, _prompt: string, _tag?: string, _options?: { useResearch?: boolean; mode?: 'append' | 'update' | 'rewrite' } ): Promise<void> { - throw new Error( - 'File storage does not support updateTaskWithPrompt. ' + - 'Client-side AI logic must process the prompt before calling updateTask().' - ); + const err = new Error( + 'File storage does not support updateTaskWithPrompt. Client-side AI must process the prompt before calling updateTask().' + ); + (err as any).code = 'UNSUPPORTED_OPERATION'; + err.name = 'UnsupportedOperationError'; + throw err; }packages/tm-core/src/modules/integration/integration-domain.ts (1)
31-33: Optional: allow AuthManager injection for testabilityAccept an optional authManager in ctor for easier unit testing/mocking.
- constructor(configManager: ConfigManager) { - const authManager = AuthManager.getInstance(); - this.exportService = new ExportService(configManager, authManager); - } + constructor(configManager: ConfigManager, authManager: AuthManager = AuthManager.getInstance()) { + this.exportService = new ExportService(configManager, authManager); + }apps/cli/src/commands/start.command.ts (2)
147-151: Avoid printing “Storage: undefined” — add a fallback.Minor UX: default to 'file' or 'unknown' if
getStorageType()is undefined.- const result: StartCommandResult = { - ...coreResult, - storageType: this.tmCore?.tasks.getStorageType() - }; + const result: StartCommandResult = { + ...coreResult, + storageType: this.tmCore?.tasks.getStorageType() ?? 'unknown' + };
293-301: Remove process signal listeners after child exits to prevent leaks.If users run multiple sessions in one process (e.g., programmatic API), listeners will accumulate.
- process.on('SIGINT', cleanup); - process.on('SIGTERM', cleanup); - process.on('exit', cleanup); + process.on('SIGINT', cleanup); + process.on('SIGTERM', cleanup); + process.on('exit', cleanup); + const removeHandlers = () => { + process.off('SIGINT', cleanup); + process.off('SIGTERM', cleanup); + process.off('exit', cleanup); + }; + childProcess.on('close', () => removeHandlers()); + childProcess.on('error', () => removeHandlers());packages/tm-core/src/modules/config/services/runtime-state-manager.service.spec.ts (2)
111-137: Persisted JSON assertion checks"activeTag"— confirm this is by design.The in-memory API uses
currentTag, but the save assertion expects"activeTag". If backward-compat persistence is intentional, add a brief comment in the spec to clarify; otherwise, update expectation to"currentTag".
161-175: Rename describe blocks to match new API names.
getActiveTag/setActiveTagblocks now exercisegetCurrentTag/setCurrentTag. Consider renaming for clarity.scripts/modules/task-manager/update-task-by-id.js (1)
352-364: Prefer generateTextService + manual JSON parsing for structured output.Per guidelines,
generateObjectServicecan be unreliable across providers/schemas. Consider switching to text generation plus robust parse (e.g., Zod), keeping the schema as validator.As per coding guidelines
packages/tm-core/src/modules/config/config-domain.ts (1)
37-39: Add explicit return types for public API stabilityAnnotate return types (e.g., getModelConfig(): ModelConfig, getConfigSources(): ConfigSourcesType) to prevent unintentional breaking changes from inference drift.
Also applies to: 111-116
packages/tm-core/src/modules/tasks/services/task-loader.service.ts (1)
168-189: Align CLI command names in suggestions with current CLI ('tm ...')Replace legacy 'task-master ...' strings with 'tm ...' to match the rest of the UI.
- suggestions.push(` task-master expand --id=${task.id}`); + suggestions.push(` tm expand --id=${task.id}`); @@ - suggestions.push(` task-master show ${task.id}`); + suggestions.push(` tm show ${task.id}`); @@ - suggestions.push(` task-master analyze-complexity --from=${task.id}`); + suggestions.push(` tm analyze-complexity --from=${task.id}`);packages/tm-core/src/modules/workflow/workflow-domain.ts (1)
8-13: Decouple type imports from service; import from types module instead.Avoid exposing service internals via public API types. Prefer central
./types.jsto reduce coupling and future refactor friction.-import type { - StartWorkflowOptions, - WorkflowStatus, - NextAction -} from './services/workflow.service.js'; -import type { TestResult, WorkflowContext } from './types.js'; +import type { + StartWorkflowOptions, + WorkflowStatus, + NextAction, + TestResult, + WorkflowContext +} from './types.js';Please confirm
StartWorkflowOptions,WorkflowStatus, andNextActionare exported frompackages/tm-core/src/modules/workflow/types.ts.packages/tm-core/src/modules/tasks/services/task-service.ts (4)
67-83: Make initialization single-flight to avoid races under concurrency.Concurrent callers can create and initialize storage twice. Use an
initializingpromise guard.Add a guard field and wrap initialize:
class TaskService { private configManager: ConfigManager; private storage: IStorage; private initialized = false; + private initializing?: Promise<void>; private logger = getLogger('TaskService'); @@ async initialize(): Promise<void> { - if (this.initialized) return; - - // Create storage based on configuration - const storageConfig = this.configManager.getStorageConfig(); - const projectRoot = this.configManager.getProjectRoot(); - - this.storage = StorageFactory.createFromStorageConfig( - storageConfig, - projectRoot - ); - - // Initialize storage - await this.storage.initialize(); - - this.initialized = true; + if (this.initialized) return; + if (this.initializing) return this.initializing; + this.initializing = (async () => { + const storageConfig = this.configManager.getStorageConfig(); + const projectRoot = this.configManager.getProjectRoot(); + this.storage = StorageFactory.createFromStorageConfig(storageConfig, projectRoot); + await this.storage.initialize(); + this.initialized = true; + })(); + try { + await this.initializing; + } finally { + this.initializing = undefined; + } }
486-492: Guard getStorageType against uninitialized usage (optional).If called pre-init,
this.storageis null. Either ensure init or document the precondition.Option A — ensure initialization:
-getStorageType(): 'file' | 'api' { - // Storage interface guarantees this method exists - return this.storage.getStorageType(); -} +getStorageType(): 'file' | 'api' { + if (!this.initialized) { + // Prefer throwing a clear error over null deref + throw new TaskMasterError('Storage not initialized', ERROR_CODES.STORAGE_ERROR); + } + return this.storage.getStorageType(); +}Option B — add
await this.ensureInitialized()and make this method async (API ripple). Choose per public API constraints.Confirm all call sites (e.g.,
tmCore.tasks.getStorageType()) are afterTasksDomain.initialize(); otherwise adopt Option A.
604-623: Error-wrapping policy differs between updateTask and updateTaskWithPrompt.
updateTaskWithPromptrethrows allTaskMasterErrors, whileupdateTaskonly lets user-facing ones through. Align behavior intentionally.If only user-facing codes should propagate:
- if (error instanceof TaskMasterError) { - throw error; - } + if (error instanceof TaskMasterError && error.is(ERROR_CODES.NO_BRIEF_SELECTED)) { + throw error; + }
188-219: Optional: ensure methods auto-init for robustness.
getTask,getTaskList, etc., assume prior init. If TaskService is used outsideTmCore, add a sharedensureInitialized()and call at entry.Would you like a small PR to add
private async ensureInitialized()and wire it into public getters?apps/cli/src/commands/next.command.ts (1)
231-235: Optional: add dispose hook when TmCore grows resources.Currently just clears the reference. If domains add watchers later, consider a
tmCore.dispose()in the future.tests/unit/scripts/modules/task-manager/list-tasks.test.js (2)
157-163: Prefer shared fixtures to avoid divergence.Reuse fixtures from
tests/fixtures/(or existing setup module) to keep sample tasks consistent across suites.
741-757: Scope console spies to each test to prevent leakage.You already restore after each; alternatively, use
mockRestore()inafterEachto centralize cleanup.packages/tm-core/src/modules/storage/utils/api-client.ts (2)
36-45: Session init + 401 refresh retry (idempotency-aware) and header hygieneInitialize session before getSession, don’t force Content-Type without a body, add Accept, and retry GET/HEAD once on 401 after refresh.
Apply:
- // Get auth session - const session = await authManager.supabaseClient.getSession(); + // Ensure Supabase session is initialized before use + await authManager.supabaseClient.initialize(); + // Get auth session + const session = await authManager.supabaseClient.getSession(); @@ - // Build headers - const headers: RequestInit['headers'] = { - 'Content-Type': 'application/json', - Authorization: `Bearer ${session.access_token}`, - ...(accountId ? { 'x-account-id': accountId } : {}), - ...options.headers - }; + const method = ((options.method || 'GET') as string).toUpperCase(); + const hasBody = options.body !== undefined; + // Build headers + const headers: RequestInit['headers'] = { + Accept: 'application/json', + ...(hasBody ? { 'Content-Type': 'application/json' } : {}), + Authorization: `Bearer ${session.access_token}`, + ...(accountId ? { 'x-account-id': accountId } : {}), + ...options.headers + }; @@ - const response = await fetch(url, { + let response = await fetch(url, { ...options, headers }); // Handle non-2xx responses - if (!response.ok) { + if (!response.ok) { + // If unauthorized, try one refresh-and-retry for idempotent requests + if (response.status === 401 && (method === 'GET' || method === 'HEAD')) { + await authManager.refreshToken(); + const refreshed = await authManager.supabaseClient.getSession(); + const retryHeaders = { + ...headers, + Authorization: `Bearer ${refreshed?.access_token || ''}` + }; + response = await fetch(url, { ...options, headers: retryHeaders }); + if (response.ok) { + // fall through to success parse below + } + } await this.handleErrorResponse(response, endpoint); }Also applies to: 49-55, 57-68
108-113: Map 401 to AUTHENTICATION_ERRORDifferentiate auth failures from generic API errors for better UX and handling.
Apply:
- throw new TaskMasterError(errorMessage, ERROR_CODES.API_ERROR, { + const code = + response.status === 401 + ? ERROR_CODES.AUTHENTICATION_ERROR + : ERROR_CODES.API_ERROR; + throw new TaskMasterError(errorMessage, code, { operation: 'api-request', endpoint, statusCode: response.status });packages/tm-core/src/modules/storage/adapters/api-storage.ts (2)
177-180: Use the class logger instead of console.debugConsistent logging helps traceability.
Apply:
- console.debug('No brief selected, starting with empty cache'); + this.logger.debug('No brief selected, starting with empty cache');
898-903: Clarify env var in error messageYou accept TM_BASE_DOMAIN or TM_PUBLIC_BASE_DOMAIN; the message mentions only one.
Apply:
- 'API endpoint not configured. Please set TM_PUBLIC_BASE_DOMAIN environment variable.', + 'API endpoint not configured. Please set TM_BASE_DOMAIN or TM_PUBLIC_BASE_DOMAIN.',packages/tm-bridge/src/update-bridge.ts (2)
18-24: Plumb useResearch through the bridgeExpose an optional useResearch flag and pass it to core.
Apply:
export interface UpdateBridgeParams { @@ appendMode?: boolean; + /** Enable research mode */ + useResearch?: boolean; @@ - await tmCore.tasks.updateWithPrompt(String(taskId), prompt, tag, { - mode - }); + await tmCore.tasks.updateWithPrompt(String(taskId), prompt, tag, { + mode, + ...(params.useResearch ? { useResearch: true } : {}) + });Also applies to: 136-141
123-134: Always clear the spinner (use finally)Guarantee cleanup even if a later step throws.
Apply:
let loadingIndicator: NodeJS.Timeout | null = null; if (!isMCP && outputFormat === 'text') { @@ try { @@ - if (loadingIndicator) { - clearInterval(loadingIndicator); - process.stdout.write('\r✓ Task updated successfully.\n'); - } + if (loadingIndicator) { + clearInterval(loadingIndicator); + loadingIndicator = null; + process.stdout.write('\r✓ Task updated successfully.\n'); + } @@ - } catch (updateError) { - if (loadingIndicator) { - clearInterval(loadingIndicator); - process.stdout.write('\r✗ Update failed.\n'); - } - // tm-core already formatted the error properly, just re-throw - throw updateError; - } + } catch (updateError) { + if (loadingIndicator) { + clearInterval(loadingIndicator); + loadingIndicator = null; + process.stdout.write('\r✗ Update failed.\n'); + } + throw updateError; + } finally { + if (loadingIndicator) { + clearInterval(loadingIndicator); + loadingIndicator = null; + if (!isMCP && outputFormat === 'text') process.stdout.write('\r'); + } + }Also applies to: 142-147, 175-182
apps/cli/src/commands/show.command.ts (1)
182-190: Parallelize multiple task fetchesAvoid N sequential awaits; fetch in parallel.
Apply:
- // Get each task individually - for (const taskId of taskIds) { - const result = await this.tmCore.tasks.get(taskId); - if (result.task) { - tasks.push(result.task); - } else { - notFound.push(taskId); - } - } + const results = await Promise.all(taskIds.map((id) => this.tmCore!.tasks.get(id))); + results.forEach((res, idx) => { + if (res.task) tasks.push(res.task); + else notFound.push(taskIds[idx]); + });packages/tm-core/src/modules/git/git-domain.ts (1)
237-239: Tighten return types for validator/parserEnsure consumers get strong types without importing internals.
Apply:
- validateCommitMessage(message: string) { + validateCommitMessage( + message: string + ): ReturnType<CommitMessageGenerator['validateConventionalCommit']> { return this.commitGenerator.validateConventionalCommit(message); } @@ - parseCommitMessage(message: string) { + parseCommitMessage( + message: string + ): ReturnType<CommitMessageGenerator['parseCommitMessage']> { return this.commitGenerator.parseCommitMessage(message); }Also applies to: 244-246
apps/cli/src/commands/autopilot.command.ts (2)
7-13: Normalize project path to absolute before creating TmCorePrevents surprises when invoked from different cwd.
Apply this diff:
import boxen from 'boxen'; import ora, { type Ora } from 'ora'; +import path from 'node:path'; import { createTmCore, type TmCore, type Task, type Subtask } from '@tm/core';private async initializeCore(projectRoot: string): Promise<void> { if (!this.tmCore) { - this.tmCore = await createTmCore({ projectPath: projectRoot }); + const absRoot = path.resolve(projectRoot); + this.tmCore = await createTmCore({ projectPath: absRoot }); } }Also applies to: 164-167
203-207: Minor: box width should be an integerAvoid passing a float to boxen width.
Apply this diff:
- width: process.stdout.columns ? process.stdout.columns * 0.95 : 100 + width: process.stdout.columns ? Math.floor(process.stdout.columns * 0.95) : 100packages/tm-core/src/modules/auth/auth-domain.ts (2)
38-41: Make dependency immutableThe manager is a singleton; mark it readonly.
Apply this diff:
-export class AuthDomain { - private authManager: AuthManager; +export class AuthDomain { + private readonly authManager: AuthManager;
159-181: Accept StorageType and handle 'auto' explicitlyPublic API is friendlier if it accepts StorageType and normalizes 'auto' (which can surface when callers pass config directly).
Apply this diff:
- getStorageDisplayInfo( - resolvedStorageType: 'file' | 'api' - ): StorageDisplayInfo { - if (resolvedStorageType === 'api') { + getStorageDisplayInfo( + resolvedStorageType: StorageType + ): StorageDisplayInfo { + const storage = resolvedStorageType === 'auto' ? 'file' : resolvedStorageType; + if (storage === 'api') { const context = this.getContext(); if (context?.briefId && context?.briefName) { return { storageType: 'api', briefInfo: { briefId: context.briefId, briefName: context.briefName, orgSlug: context.orgSlug, webAppUrl: this.getWebAppUrl() } }; } } // Default to file storage display return { storageType: 'file', filePath: path.join('.taskmaster', 'tasks', 'tasks.json') }; }packages/tm-core/src/modules/tasks/tasks-domain.ts (4)
172-181: Type the conflict check result explicitlyExpose the concrete type on the public surface for stability.
Apply this diff:
- async checkInProgressConflicts(taskId: string) { - return this.executionService.checkInProgressConflicts(taskId); - } + async checkInProgressConflicts(taskId: string): Promise<ConflictCheckResult> { + return this.executionService.checkInProgressConflicts(taskId); + }And extend the import above:
-import type { - StartTaskOptions, - StartTaskResult -} from './services/task-execution-service.js'; +import type { + StartTaskOptions, + StartTaskResult, + ConflictCheckResult +} from './services/task-execution-service.js';
200-204: Return type for getExecutionOrderHelp consumers with a precise type.
Apply this diff:
-import type { Task, TaskStatus } from '../../common/types/index.js'; +import type { Task, TaskStatus, Subtask } from '../../common/types/index.js'; ... - getExecutionOrder(task: Task) { + getExecutionOrder(task: Task): Subtask[] { return this.loaderService.getExecutionOrder(task); }
218-241: Annotate preflight helper returnsPublic methods should declare
Promise<CheckResult>to avoid any leakage ofany.Apply this diff:
-import type { - PreflightResult -} from './services/preflight-checker.service.js'; +import type { + PreflightResult, + CheckResult +} from './services/preflight-checker.service.js'; ... - async detectTestCommand() { + async detectTestCommand(): Promise<CheckResult> { return this.preflightChecker.detectTestCommand(); } ... - async checkGitWorkingTree() { + async checkGitWorkingTree(): Promise<CheckResult> { return this.preflightChecker.checkGitWorkingTree(); } ... - async validateRequiredTools() { + async validateRequiredTools(): Promise<CheckResult> { return this.preflightChecker.validateRequiredTools(); } ... - async detectDefaultBranch() { + async detectDefaultBranch(): Promise<CheckResult> { return this.preflightChecker.detectDefaultBranch(); }
63-89: ClarifyisSubtasksemantics or return the found subtaskAs implemented,
isSubtaskis true only when the subtask part exists and is found, not merely when the ID uses dot notation. Consider:
- Rename to
subtaskFound(orisSubtaskIdif you intend to reflect notation).- Or return
{ task, subtask, subtaskId }like the execution service.
| "provider": "anthropic", | ||
| "modelId": "claude-sonnet-4-5-20250929", |
There was a problem hiding this comment.
🧩 Analysis chain
Configuration changes must use CLI/MCP tools, not direct file edits.
Per coding guidelines, .taskmaster/config.json should not be manually edited. Use the task-master models command or models MCP tool for all configuration changes instead.
Additionally, verify the following:
- Model IDs: Confirm that
claude-sonnet-4-5-20250929,sonar-pro, andclaude-sonnet-4-20250514are valid and available through their respective providers (Anthropic and Perplexity). - Token limit reduction: The fallback model's
maxTokenswas reduced from 120000 to 64000 (45% reduction). Verify that this reduction does not impact critical workflows or cause token budget issues.
Please reapply these configuration changes using the proper CLI or MCP tool:
Also applies to: 11-11, 17-18
🏁 Script executed:
# Example: Use the CLI to set main model
task-master models set --model main --provider anthropic --modelId claude-sonnet-4-5-20250929
# Example: Use the CLI to set research model
task-master models set --model research --provider perplexity --modelId sonar-pro
# Example: Use the CLI to set fallback model with new token limit
task-master models set --model fallback --provider anthropic --modelId claude-sonnet-4-20250514 --maxTokens 64000Length of output: 149
🏁 Script executed:
#!/bin/bash
# Check how .taskmaster/config.json is used in the codebase
rg -n "\.taskmaster/config\.json" --type-list | head -20
fd ".taskmaster" -type f | head -20Length of output: 560
🏁 Script executed:
#!/bin/bash
# Look for documentation about config or model setup
fd -e md -e txt | xargs rg -l "config\.json|model.*config" 2>/dev/null | head -10Length of output: 380
🏁 Script executed:
#!/bin/bash
# Check if there's any guidance about manual vs tool-based config
rg -n "manually edit|task-master models" -g "*.md" -g "*.mdc" -C2 | head -30Length of output: 1796
🌐 Web query:
claude-sonnet-4-5-20250929 anthropic model API
💡 Result:
Short summary — Claude Sonnet 4.5 (API details)
- Official model name / API model id: claude-sonnet-4-5-20250929. [1][2]
- Release / announcement date: September 29, 2025. [1][6]
- Where available: Anthropic Claude API (via api.anthropic.com), Amazon Bedrock, Google Vertex AI, and Claude web/apps. [1][2][4]
- Context window / max output: primary published context ~200k (1M token beta available); max output ~64k tokens. [4][5][6]
- Pricing (published): input $3 / 1M tokens, output $15 / 1M tokens. [2][4]
- Notes / compatibility: use the model id exactly when calling Anthropic’s v1/messages endpoint; Sonnet 4.5 disallows specifying both temperature and top_p simultaneously. Also supports tool-calling/memory features described in the release notes. [1][2][3]
Sources
- Anthropic Claude release notes (Sept 29, 2025). [Claude Docs release notes].
- Anthropic — “What’s new” / model page for Claude Sonnet 4.5 (availability, pricing, migration notes). [Claude Docs model page].
- Example integration / endpoint usage (third‑party guide). [TypingMind integration guide].
- Helicone model listing (pricing, context, tokens).
- Vals.ai model card (benchmarks / context window).
If you want, I can show a short example cURL or Python request using the model id and Anthropic’s API.
🌐 Web query:
sonar-pro perplexity model API
💡 Result:
Short answer: Perplexity’s Sonar Pro is available via their API (OpenAI-compatible chat/completions endpoint). Use model name "sonar-pro" at https://api.perplexity.ai/chat/completions, with OpenAI-compatible client libraries (pass your Perplexity API key and change base_url). Sonar Pro features: 200K context, advanced search/citation retrieval, higher-depth answers; pricing is per-input/output tokens (docs list input ≈ $3/1M, output ≈ $15/1M) and per-request search fees. See Perplexity docs and announcements for full usage, sample cURL/SDK calls, and recent billing/changelog notes. [1][2][3][4]
Sources:
- Sonar Pro model card (features, context, pricing, examples). [1]
- OpenAI-compatibility / chat completions guide (endpoint, model name, SDK examples). [2]
- TechCrunch coverage summarizing Sonar & Sonar Pro pricing and capabilities. [3]
- Perplexity changelog (API notes / citation & deprecation changes). [4]
If you want, I can: show a ready-to-run cURL or Python OpenAI-compatible snippet using your API key, or fetch the exact current token/pricing table. Which would you like?
🌐 Web query:
claude-sonnet-4-20250514 anthropic model
💡 Result:
Claude Sonnet 4 (Anthropic) — quick summary
- Official model name / API ID: claude-sonnet-4-20250514. [1][2]
- Release / snapshot date: May 2025 (snapshot 20250514). [1][3]
- Capabilities: multimodal (text + image input, text output), up to 200k context window, large max output (tens of thousands of tokens), optimized for high-throughput/efficient reasoning and coding use cases. [1][2][4]
- Availability / platforms: offered by Anthropic API as claude-sonnet-4-20250514, available through cloud partners (e.g., Google Vertex AI and AWS Bedrock partner listings). [1][2][3]
- Typical uses: fast, cost‑efficient high-volume tasks, coding assistance, reasoning, multimodal applications; positioned as a balanced/high-performance variant alongside the more capable Opus model. [1][4]
Sources: Anthropic model pages and docs, Google Cloud Vertex AI partner page, reporting on the Claude 4 launch. [1][2][3][4]
---
🏁 Script executed:
```shell
#!/bin/bash
# Find and display the .taskmaster/config.json file
fd "config\.json" -path "*taskmaster*"
Length of output: 168
🏁 Script executed:
#!/bin/bash
# Check migration-guide.md for config-related changes
cat docs/migration-guide.md 2>/dev/null | head -100Length of output: 2617
🏁 Script executed:
#!/bin/bash
# Look for any comments or documentation about the token limit change
rg -n "maxTokens|120000|64000" -g "*.json" -g "*.md" -C2 | head -40Length of output: 4147
🏁 Script executed:
#!/bin/bash
# Check if there's a .taskmaster directory and its contents
find . -type d -name ".taskmaster" -o -type d -name "taskmaster" | head -5
cat .taskmaster/config.json 2>/dev/null || cat .taskmasterconfig 2>/dev/null || echo "File not found in root"Length of output: 1012
Configuration must use CLI/MCP tools, not direct file edits.
The coding guidelines require all .taskmaster/config.json changes to use the task-master models command or models MCP tool. Direct file edits bypass the validation and migration systems these tools provide.
Regarding the changes:
- All model IDs are valid:
claude-sonnet-4-5-20250929(Sept 2025),sonar-pro(Perplexity), andclaude-sonnet-4-20250514(May 2025) - The fallback model's token limit reduction from 120000 to 64000 is within supported limits, but represents a 45% decrease in capacity for the fallback model—verify this intentional constraint does not negatively impact critical workflows.
Please reapply these configuration changes using the proper CLI tool:
task-master models --set-main anthropic/claude-sonnet-4-5-20250929
task-master models --set-research perplexity/sonar-pro
task-master models --set-fallback anthropic/claude-sonnet-4-20250514 --max-tokens 64000🤖 Prompt for AI Agents
In .taskmaster/config.json around lines 4-5 the model entries were edited
directly; reapply these changes using the task-master CLI/MCP tools instead of
editing the file: run the three commands to set main, research, and fallback
models with the specified IDs and set the fallback max-tokens to 64000
(task-master models --set-main anthropic/claude-sonnet-4-5-20250929; task-master
models --set-research perplexity/sonar-pro; task-master models --set-fallback
anthropic/claude-sonnet-4-20250514 --max-tokens 64000), remove the direct file
modification, and then run the tool's verification/migration step to confirm the
change applied successfully.
| type Task, | ||
| type Subtask | ||
| } from '@tm/core'; | ||
| import { createTmCore, type TmCore, type Task, type Subtask } from '@tm/core'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Use TmCore facade for preflight and remove duplicate local types
Unify on core types and APIs. Avoid direct PreflightChecker construction and local type drift.
Apply this diff:
-import { createTmCore, type TmCore, type Task, type Subtask } from '@tm/core';
+import { createTmCore, type TmCore, type Task, type Subtask, type PreflightResult } from '@tm/core';-export interface PreflightCheckResult {
- success: boolean;
- message?: string;
-}
+// Use PreflightResult from @tm/core-export interface PreflightResult {
- success: boolean;
- testCommand: PreflightCheckResult;
- gitWorkingTree: PreflightCheckResult;
- requiredTools: PreflightCheckResult;
- defaultBranch: PreflightCheckResult;
-}
+// Use PreflightResult from @tm/core- private async runPreflightChecks(
- options: AutopilotCommandOptions
- ): Promise<PreflightResult> {
- const { PreflightChecker } = await import('@tm/core');
-
- console.log();
- console.log(chalk.cyan.bold('Running preflight checks...'));
-
- const preflightChecker = new PreflightChecker(
- options.project || process.cwd()
- );
- const result = await preflightChecker.runAllChecks();
+ private async runPreflightChecks(
+ _options: AutopilotCommandOptions
+ ): Promise<PreflightResult> {
+ if (!this.tmCore) throw new Error('TmCore not initialized');
+ console.log();
+ console.log(chalk.cyan.bold('Running preflight checks...'));
+ const result = await this.tmCore.tasks.runPreflightChecks();- private displayPreflightResults(result: PreflightResult): void {
+ private displayPreflightResults(result: PreflightResult): void {Run impact check: ensure no other references to local Preflight* interfaces remain.
Also applies to: 26-40, 259-275, 371-389
🏁 Script executed:
rg -n "interface Preflight(CheckResult|Result)|PreflightChecker" apps/cli/src/commands/autopilot.command.tsLength of output: 268
I'll verify the broader impact and confirm the types exist in @tm/core before finalizing.The web search didn't give me information about the specific @tm/core package. Let me search the codebase directly to verify if the types and methods exist, and to check for all usages of these local types.
🏁 Script executed:
#!/bin/bash
# Search for PreflightResult and PreflightCheckResult imports across the codebase
rg -n "from.*autopilot.*command|import.*PreflightResult|import.*PreflightCheckResult" -g "!node_modules" --type ts --type jsLength of output: 189
🏁 Script executed:
#!/bin/bash
# Check @tm/core exports - look for index or main files
find . -path "*/node_modules" -prune -o -path "*/@tm/core*" -type f \( -name "index.ts" -o -name "index.d.ts" -o -name "*.d.ts" \) -print 2>/dev/null | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search the whole file to see the complete structure
cat apps/cli/src/commands/autopilot.command.ts | head -100Length of output: 2549
The web search isn't finding the relevant npm package @tm/core. Let me search the codebase more directly to understand the structure and what @tm/core provides.
🏁 Script executed:
#!/bin/bash
# Look for more context in the file - get the entire runPreflightChecks and how it's used
sed -n '250,290p' apps/cli/src/commands/autopilot.command.tsLength of output: 972
🏁 Script executed:
#!/bin/bash
# Search for how tmCore is initialized or created
rg -n "tmCore\s*=" apps/cli/src/commands/autopilot.command.ts -A 2 -B 2Length of output: 296
🏁 Script executed:
#!/bin/bash
# Look for package.json to understand dependencies
cat package.json 2>/dev/null | head -50Length of output: 2130
Let me search for the @tm/core package definition to verify the TmCore interface and available methods:
🏁 Script executed:
#!/bin/bash
# Find @tm/core package location
find . -path "*/node_modules" -prune -o -type d -name "@tm" -print 2>/dev/nullLength of output: 0
🏁 Script executed:
#!/bin/bash
# Look for TmCore type/interface definition
rg -n "type TmCore|interface TmCore" -A 10 --type ts -g "!node_modules"Length of output: 8691
🏁 Script executed:
#!/bin/bash
# Search for PreflightResult type definition in @tm/core
rg -n "PreflightResult|runPreflightChecks" --type ts -g "!node_modules" | head -30Length of output: 1024
Now let me verify the TmCore interface structure to confirm the .tasks property exists:
🏁 Script executed:
#!/bin/bash
# Get TmCore class/interface definition with tasks property
sed -n '24,80p' packages/tm-core/src/tm-core.tsLength of output: 1420
🏁 Script executed:
#!/bin/bash
# Verify PreflightResult export from @tm/core
sed -n '55,65p' packages/tm-core/src/index.tsLength of output: 315
Consolidate to @tm/core types and use TmCore facade for preflight checks
The refactor is valid. PreflightResult is exported from @tm/core, and TasksDomain provides a runPreflightChecks() method accessible via this.tmCore.tasks. Remove the local PreflightResult and PreflightCheckResult interfaces, import PreflightResult from @tm/core, and replace the direct PreflightChecker construction with a call to this.tmCore.tasks.runPreflightChecks().
Apply the suggested diffs across lines 11, 26–40, 259–275, and 371–389.
🤖 Prompt for AI Agents
In apps/cli/src/commands/autopilot.command.ts (lines 11, 26–40, 259–275,
371–389): remove the local PreflightResult and PreflightCheckResult interfaces,
add PreflightResult to the import from '@tm/core' on line 11, and replace any
direct construction/use of a PreflightChecker with a call to
this.tmCore.tasks.runPreflightChecks(); update code that referenced the removed
local types to use the imported PreflightResult and use this.tmCore (TmCore)
facade for running preflight checks and handling results.
| TmCore | ||
| } from '@tm/core'; |
There was a problem hiding this comment.
🧩 Analysis chain
Public type re-export switched to TmCore — looks good.
Matches the migration to the new core facade. Consider scanning for lingering TaskMasterCore uses.
Based on learnings
🏁 Script executed:
#!/bin/bash
# Find lingering references to TaskMasterCore and old import paths
rg -nP -C2 '\bTaskMasterCore\b|@tm/core/types' --glob '!**/dist/**' --glob '!**/node_modules/**'Length of output: 3541
🏁 Script executed:
#!/bin/bash
# Check what's exported from @tm/core index to determine export strategy
cat packages/tm-core/src/index.ts | head -100Length of output: 2345
🏁 Script executed:
#!/bin/bash
# Check if createTaskMasterCore function still exists in codebase
rg -nP '\bcreateTaskMasterCore\s*\(' packages/tm-core/src/ --type tsLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check if TaskMasterCore is exported anywhere from @tm/core
rg -nP 'export.*TaskMasterCore|TaskMasterCore.*as' packages/tm-core/src/ --type tsLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check the tm-core.js file to see TmCore definition and whether it aliases TaskMasterCore
cat packages/tm-core/src/tm-core.ts | head -60Length of output: 1938
| const resolvedStorageType = tmCore.tasks.getStorageType(); | ||
|
|
||
| // Get storage display info from tm-core (single source of truth) | ||
| const displayInfo = tmCore.auth.getStorageDisplayInfo(resolvedStorageType); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Prefer active tag from config and harden displayInfo retrieval
- Use tmCore.config.getActiveTag() when tag is not provided.
- Guard getStorageDisplayInfo with try/catch to fall back gracefully.
- Minor: confirm tasks.getStorageType() never returns 'auto' post-resolution.
Apply:
- // Get the resolved storage type from tasks domain
- const resolvedStorageType = tmCore.tasks.getStorageType();
-
- // Get storage display info from tm-core (single source of truth)
- const displayInfo = tmCore.auth.getStorageDisplayInfo(resolvedStorageType);
-
- // Display header with computed display info
- displayHeader({
- tag: options.tag || 'master',
- filePath: displayInfo.filePath,
- storageType: displayInfo.storageType,
- briefInfo: displayInfo.briefInfo
- });
+ // Resolve tag and storage details from core
+ const tag = options.tag ?? tmCore.config.getActiveTag() ?? 'master';
+ let displayInfo:
+ | { filePath?: string; storageType: StorageType; briefInfo?: any }
+ | undefined;
+ try {
+ const resolvedStorageType = tmCore.tasks.getStorageType();
+ displayInfo = tmCore.auth.getStorageDisplayInfo(resolvedStorageType);
+ } catch {
+ // fall back to minimal header
+ displayHeader({ tag, storageType: options.storageType });
+ return;
+ }
+ displayHeader({
+ tag,
+ filePath: displayInfo?.filePath,
+ storageType: displayInfo!.storageType,
+ briefInfo: displayInfo?.briefInfo
+ });Verify that getStorageType() cannot yield 'auto' at this point:
[Based on learnings]
Also applies to: 37-42
🏁 Script executed:
#!/bin/bash
rg -nP --type=ts -C2 '\bgetStorageType\s*\(' packages/tm-core | sed -n '1,200p'Length of output: 3876
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for getActiveTag and tag resolution in config
rg -nP --type=ts -C2 '\bgetActiveTag\s*\(' packages/tm-core
Length of output: 4756
🏁 Script executed:
#!/bin/bash
# Look at the full context of display-helpers.ts lines 25-45
cat apps/cli/src/utils/display-helpers.ts | sed -n '25,45p'Length of output: 524
🏁 Script executed:
#!/bin/bash
# Search for tag resolution patterns and getCurrentTag in the codebase
rg -nP --type=ts -C2 'getCurrentTag|getActiveTag' packages/tm-core/src/modules | sed -n '1,150p'Length of output: 9155
Replace hard-coded tag fallback and add defensive error handling
The current code hard-codes 'master' as a fallback tag instead of using the active tag from config, which diverges from the established codebase pattern and learnings. Use tmCore.config.getActiveTag() for consistency with task-service and export service implementations. Additionally, guard getStorageDisplayInfo() with try/catch for resilience.
- // Get the resolved storage type from tasks domain
- const resolvedStorageType = tmCore.tasks.getStorageType();
-
- // Get storage display info from tm-core (single source of truth)
- const displayInfo = tmCore.auth.getStorageDisplayInfo(resolvedStorageType);
-
- // Display header with computed display info
- displayHeader({
- tag: options.tag || 'master',
- filePath: displayInfo.filePath,
- storageType: displayInfo.storageType,
- briefInfo: displayInfo.briefInfo
- });
+ // Resolve tag and storage details from core
+ const tag = options.tag ?? tmCore.config.getActiveTag() ?? 'master';
+ let displayInfo:
+ | { filePath?: string; storageType: StorageType; briefInfo?: any }
+ | undefined;
+ try {
+ const resolvedStorageType = tmCore.tasks.getStorageType();
+ displayInfo = tmCore.auth.getStorageDisplayInfo(resolvedStorageType);
+ } catch {
+ // fall back to minimal header
+ displayHeader({ tag, storageType: options.storageType });
+ return;
+ }
+ displayHeader({
+ tag,
+ filePath: displayInfo?.filePath,
+ storageType: displayInfo!.storageType,
+ briefInfo: displayInfo?.briefInfo
+ });Also applies to: 37-42
🤖 Prompt for AI Agents
In apps/cli/src/utils/display-helpers.ts around lines 31-35 (and similarly for
lines 37-42), the code currently falls back to a hard-coded 'master' tag and
calls tmCore.auth.getStorageDisplayInfo() without protection; replace the
hard-coded fallback with tmCore.config.getActiveTag() to derive the active tag
consistently, and wrap the call to
tmCore.auth.getStorageDisplayInfo(resolvedStorageType, activeTag) in a
try/catch; on error, log or return a safe default displayInfo so the CLI remains
resilient and behavior matches task-service/export-service patterns.
| // Parse taskId - handle numeric, alphanumeric, and subtask IDs | ||
| let taskId; | ||
| if (typeof id === 'string') { | ||
| // Handle subtask IDs (e.g., "5.2") | ||
| if (id.includes('.')) { | ||
| taskId = id; // Keep as string for subtask IDs | ||
| } else { | ||
| // Parse as integer for main task IDs | ||
| taskId = parseInt(id, 10); | ||
| if (Number.isNaN(taskId)) { | ||
| const errorMessage = `Invalid task ID: ${id}. Task ID must be a positive integer or subtask ID (e.g., "5.2").`; | ||
| logWrapper.error(errorMessage); | ||
| return { | ||
| success: false, | ||
| error: { code: 'INVALID_TASK_ID', message: errorMessage } | ||
| }; | ||
| } | ||
| } | ||
| } else { | ||
| // Keep ID as string - supports numeric (1, 2), alphanumeric (TAS-49, JIRA-123), and subtask IDs (1.2, TAS-49.1) | ||
| taskId = id; | ||
| } else if (typeof id === 'number') { | ||
| // Convert number to string for consistency | ||
| taskId = String(id); | ||
| } else { | ||
| const errorMessage = `Invalid task ID type: ${typeof id}. Task ID must be a string or number.`; | ||
| logWrapper.error(errorMessage); | ||
| return { | ||
| success: false, | ||
| error: { code: 'INPUT_VALIDATION_ERROR', message: errorMessage } | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
ID handling is flexible; ensure core accepts strings
Accepting numeric, alphanumeric, and dotted IDs is useful. Verify scripts/modules/task-manager.updateTaskById supports string IDs to avoid downstream failures.
🏁 Script executed:
#!/bin/bash
# Verify core updateTaskById signature and ID handling
rg -nP -C3 'export default function updateTaskById|export async function updateTaskById' scripts/modules -S
rg -nP -C3 '\bparseInt\s*\(\s*task(Id|ID)' scripts/modules | sed -n '1,200p'Length of output: 9206
I need to locate the actual updateTaskById function that this direct function calls. Let me search the codebase more thoroughly.Let me search the codebase directly for the update-task-by-id function:
🏁 Script executed:
#!/bin/bash
# Find and examine the actual updateTaskById or update-task-by-id function
fd 'update-task-by-id' scripts/modules --type f
cat scripts/modules/task-manager/update-task-by-id.js | head -100
rg -nP 'export.*updateTaskById|function updateTaskById' scripts/modules -A 10 -B 2Length of output: 4587
I need to see the actual task lookup logic in the core function:
🏁 Script executed:
#!/bin/bash
# Find where taskId is used in the update-task-by-id.js to understand actual ID handling
rg -nP 'tasks\.find|tasks\.filter|parseInt.*taskId|taskId.*includes' scripts/modules/task-manager/update-task-by-id.js -C 3
# Also get lines 100-200 to see main logic
sed -n '100,250p' scripts/modules/task-manager/update-task-by-id.jsLength of output: 5764
Comments about supported ID formats are inaccurate and misleading
The direct function claims support for alphanumeric IDs (e.g., "TAS-49", "JIRA-123") and subtask IDs (e.g., "1.2"), but the core updateTaskById function strictly validates that taskId matches the regex /^\d+$/ (digits only). It explicitly rejects non-numeric IDs with the error: "For file storage, taskId must be a positive integer." Only numeric string IDs like "1" or "2" will work; alphanumeric and dot-notation IDs will fail at the core level.
Fix: Update the comment to accurately reflect that only numeric string IDs are supported by this direct function, or confirm with the team whether API storage fallback (via tryUpdateViaRemote) is intended as the path for display IDs, and document that limitation clearly.
🤖 Prompt for AI Agents
In mcp-server/src/core/direct-functions/update-task-by-id.js around lines 64 to
79, the inline comments incorrectly state support for alphanumeric and subtask
IDs but the core logic only accepts numeric taskIds; update the comment to state
clearly that this direct function only supports numeric task IDs (positive
integers or numeric strings) and remove examples like "TAS-49", "JIRA-123", and
"1.2"; alternatively, if non-numeric display IDs should be supported, confirm
with the team and implement or document that callers must use tryUpdateViaRemote
for alphanumeric/subtask IDs and adjust the comment to point to that fallback.
| // Parse successful response | ||
| return (await response.json()) as T; | ||
| } catch (error) { |
There was a problem hiding this comment.
Avoid JSON parsing on 204/empty responses
JSON.parse on 204 or empty body will throw. Handle no-content safely.
Apply:
- // Parse successful response
- return (await response.json()) as T;
+ // Parse successful response
+ if (response.status === 204) {
+ // No content
+ return undefined as unknown as T;
+ }
+ const ct = response.headers.get('content-type') || '';
+ if (!ct.includes('application/json')) {
+ const text = await response.text();
+ return (text ? (JSON.parse(JSON.stringify({ value: text })) as unknown) : undefined) as T;
+ }
+ return (await response.json()) as T;📝 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.
| // Parse successful response | |
| return (await response.json()) as T; | |
| } catch (error) { | |
| // Parse successful response | |
| if (response.status === 204) { | |
| // No content | |
| return undefined as unknown as T; | |
| } | |
| const ct = response.headers.get('content-type') || ''; | |
| if (!ct.includes('application/json')) { | |
| const text = await response.text(); | |
| return (text ? (JSON.parse(JSON.stringify({ value: text })) as unknown) : undefined) as T; | |
| } | |
| return (await response.json()) as T; | |
| } catch (error) { |
🤖 Prompt for AI Agents
In packages/tm-core/src/modules/storage/utils/api-client.ts around lines 69 to
71, the code calls response.json() unconditionally which will throw for 204 No
Content or empty bodies; update the logic to detect no-content (e.g.,
response.status === 204, Content-Length === '0', or empty response.text()) and
return a safe empty value (undefined or null / appropriate T) instead of
parsing, otherwise confirm Content-Type includes application/json before calling
response.json(); implement the early-return path so empty responses are handled
without JSON.parse.
| // Ensure we have storage | ||
| if (!this.storage) { | ||
| throw new TaskMasterError( | ||
| 'Storage not initialized', | ||
| ERROR_CODES.STORAGE_ERROR | ||
| ); | ||
| } | ||
|
|
||
| // Auto-initialize if needed | ||
| if (!this.initialized) { | ||
| await this.initialize(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Fix init-order bug: auto-init block is unreachable due to early throw.
if (!this.storage) throw ... happens before calling initialize(), so fresh instances will throw instead of auto-initializing.
Apply this diff to initialize before checking storage:
- // Ensure we have storage
- if (!this.storage) {
- throw new TaskMasterError(
- 'Storage not initialized',
- ERROR_CODES.STORAGE_ERROR
- );
- }
-
- // Auto-initialize if needed
- if (!this.initialized) {
- await this.initialize();
- }
+ // Auto-initialize if needed
+ if (!this.initialized) {
+ await this.initialize();
+ }
+ // Ensure we have storage after init
+ if (!this.storage) {
+ throw new TaskMasterError('Storage not initialized', ERROR_CODES.STORAGE_ERROR);
+ }📝 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.
| // Ensure we have storage | |
| if (!this.storage) { | |
| throw new TaskMasterError( | |
| 'Storage not initialized', | |
| ERROR_CODES.STORAGE_ERROR | |
| ); | |
| } | |
| // Auto-initialize if needed | |
| if (!this.initialized) { | |
| await this.initialize(); | |
| } | |
| // Auto-initialize if needed | |
| if (!this.initialized) { | |
| await this.initialize(); | |
| } | |
| // Ensure we have storage after init | |
| if (!this.storage) { | |
| throw new TaskMasterError( | |
| 'Storage not initialized', | |
| ERROR_CODES.STORAGE_ERROR | |
| ); | |
| } |
🤖 Prompt for AI Agents
In packages/tm-core/src/modules/tasks/services/task-service.ts around lines 519
to 531, the current code throws if this.storage is falsy before attempting
auto-initialization, making fresh instances fail; change the order so the
auto-initialize block runs first (call await this.initialize() when
!this.initialized), then verify this.storage and throw a TaskMasterError with
ERROR_CODES.STORAGE_ERROR only if storage is still missing after initialization.
| // Ensure we have storage | ||
| if (!this.storage) { | ||
| throw new TaskMasterError( | ||
| 'Storage not initialized', | ||
| ERROR_CODES.STORAGE_ERROR | ||
| ); | ||
| } | ||
|
|
||
| // Auto-initialize if needed | ||
| if (!this.initialized) { | ||
| await this.initialize(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Same init-order bug in updateTaskWithPrompt.
Mirror the fix from updateTask to prevent premature throw.
- // Ensure we have storage
- if (!this.storage) {
- throw new TaskMasterError(
- 'Storage not initialized',
- ERROR_CODES.STORAGE_ERROR
- );
- }
-
- // Auto-initialize if needed
- if (!this.initialized) {
- await this.initialize();
- }
+ // Auto-initialize if needed
+ if (!this.initialized) {
+ await this.initialize();
+ }
+ // Ensure we have storage after init
+ if (!this.storage) {
+ throw new TaskMasterError('Storage not initialized', ERROR_CODES.STORAGE_ERROR);
+ }🤖 Prompt for AI Agents
In packages/tm-core/src/modules/tasks/services/task-service.ts around lines 577
to 589, the code checks for this.storage before calling await this.initialize(),
which can throw prematurely; reorder the logic so you auto-initialize first when
!this.initialized (await this.initialize()), then verify this.storage and throw
TaskMasterError with ERROR_CODES.STORAGE_ERROR only if storage is still missing
after initialization.
| report('info', `Updating single task ${taskId} with prompt: "${prompt}"`); | ||
|
|
||
| // --- Input Validations (Keep existing) --- | ||
| if (!Number.isInteger(taskId) || taskId <= 0) | ||
| throw new Error( | ||
| `Invalid task ID: ${taskId}. Task ID must be a positive integer.` | ||
| ); | ||
| // --- Input Validations --- | ||
| // Note: taskId can be a number (1), string with dot (1.2), or display ID (HAM-123) | ||
| // So we don't validate it as strictly anymore | ||
| if (taskId === null || taskId === undefined || String(taskId).trim() === '') | ||
| throw new Error('Task ID cannot be empty.'); | ||
|
|
There was a problem hiding this comment.
Don’t log full prompts; redact or truncate to avoid leaking sensitive data.
Current log includes entire prompt, which may contain PII/secrets. Emulate the remote bridge’s masked preview.
- report('info', `Updating single task ${taskId} with prompt: "${prompt}"`);
+ const showDebug = process.env.TM_DEBUG === '1';
+ const promptPreview = showDebug
+ ? `${String(prompt).slice(0, 60)}${String(prompt).length > 60 ? '...' : ''}`
+ : '[hidden]';
+ report('info', `Updating single task ${taskId} with prompt: ${promptPreview}`);[Based on coding guidelines]
📝 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.
| report('info', `Updating single task ${taskId} with prompt: "${prompt}"`); | |
| // --- Input Validations (Keep existing) --- | |
| if (!Number.isInteger(taskId) || taskId <= 0) | |
| throw new Error( | |
| `Invalid task ID: ${taskId}. Task ID must be a positive integer.` | |
| ); | |
| // --- Input Validations --- | |
| // Note: taskId can be a number (1), string with dot (1.2), or display ID (HAM-123) | |
| // So we don't validate it as strictly anymore | |
| if (taskId === null || taskId === undefined || String(taskId).trim() === '') | |
| throw new Error('Task ID cannot be empty.'); | |
| const showDebug = process.env.TM_DEBUG === '1'; | |
| const promptPreview = showDebug | |
| ? `${String(prompt).slice(0, 60)}${String(prompt).length > 60 ? '...' : ''}` | |
| : '[hidden]'; | |
| report('info', `Updating single task ${taskId} with prompt: ${promptPreview}`); | |
| // --- Input Validations --- | |
| // Note: taskId can be a number (1), string with dot (1.2), or display ID (HAM-123) | |
| // So we don't validate it as strictly anymore | |
| if (taskId === null || taskId === undefined || String(taskId).trim() === '') | |
| throw new Error('Task ID cannot be empty.'); |
🤖 Prompt for AI Agents
In scripts/modules/task-manager/update-task-by-id.js around lines 77 to 84, the
current log prints the full prompt which can leak PII or secrets; change the
logging to redact or truncate the prompt (e.g., mask or replace sensitive
substrings and/or show only a short preview like first 100 chars with "…"
appended) or call the existing mask/preview helper used by the remote bridge to
produce a maskedPreview variable, then log that maskedPreview instead of the raw
prompt and keep the rest of the message format the same.
| // For file storage, ensure the tasks file exists | ||
| if (!fs.existsSync(tasksPath)) | ||
| throw new Error(`Tasks file not found: ${tasksPath}`); | ||
| // --- End Input Validations --- |
There was a problem hiding this comment.
File existence check should resolve against projectRoot.
fs.existsSync(tasksPath) may fail for relative paths that readJSON(tasksPath, projectRoot, tag) can resolve.
- if (!fs.existsSync(tasksPath))
- throw new Error(`Tasks file not found: ${tasksPath}`);
+ import path from 'path';
+ const resolvedTasksPath = path.isAbsolute(tasksPath)
+ ? tasksPath
+ : path.join(projectRoot, tasksPath);
+ if (!fs.existsSync(resolvedTasksPath))
+ throw new Error(`Tasks file not found: ${resolvedTasksPath}`);[Based on coding guidelines]
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/modules/task-manager/update-task-by-id.js around lines 126 to 129,
the existence check uses fs.existsSync(tasksPath) which can fail for relative
paths that readJSON resolves against projectRoot; resolve the tasksPath against
projectRoot first (e.g., compute a resolvedPath using projectRoot and the
original tasksPath via path.resolve or path.join), then call
fs.existsSync(resolvedPath) and use that resolvedPath for subsequent file
operations so the existence check matches how readJSON will locate the file.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
packages/tm-core/src/modules/tasks/services/task-service.ts (2)
522-568: Fix init-order bug: initialize before checking storage.Currently throws before auto-init; same issue flagged previously.
- // Ensure we have storage - if (!this.storage) { - throw new TaskMasterError( - 'Storage not initialized', - ERROR_CODES.STORAGE_ERROR - ); - } - - // Auto-initialize if needed - if (!this.initialized) { - await this.initialize(); - } + // Auto-initialize if needed + if (!this.initialized) { + await this.initialize(); + } + // Ensure we have storage after init + if (!this.storage) { + throw new TaskMasterError('Storage not initialized', ERROR_CODES.STORAGE_ERROR); + }
579-633: Same init-order bug in updateTaskWithPrompt. Mirror the fix.Reorder to initialize first, then verify storage.
- // Ensure we have storage - if (!this.storage) { - throw new TaskMasterError( - 'Storage not initialized', - ERROR_CODES.STORAGE_ERROR - ); - } - - // Auto-initialize if needed - if (!this.initialized) { - await this.initialize(); - } + // Auto-initialize if needed + if (!this.initialized) { + await this.initialize(); + } + // Ensure we have storage after init + if (!this.storage) { + throw new TaskMasterError('Storage not initialized', ERROR_CODES.STORAGE_ERROR); + }packages/tm-core/src/modules/storage/adapters/api-storage.ts (1)
552-556: SendtaganduseResearchto the backend.The method accepts
tagandoptions.useResearchparameters but doesn't include them in the request body. The backend likely needs these values to properly process the AI prompt.Apply this diff to include the missing parameters:
const result = await apiClient.patch<UpdateTaskWithPromptResponse>( `/ai/api/v1/tasks/${taskId}/prompt`, - { prompt, mode } + { + prompt, + mode, + ...(tag ? { tag } : {}), + ...(options?.useResearch ? { useResearch: true } : {}) + } );
🧹 Nitpick comments (12)
packages/tm-core/src/tm-core.ts (1)
185-197: Consider partial initialization state in error handler.If an error occurs during initialization, some private fields may be uninitialized (e.g., if ConfigManager creation fails, domain facades won't be created). While the code checks
if (this._logger)before logging, callers might still access uninitialized domain properties through getters after catching the error.Consider documenting that a TmCore instance that failed initialization should not be used, or add guards in the getters to throw clear errors if domains are uninitialized:
get tasks(): TasksDomain { + if (!this._tasks) { + throw new TaskMasterError( + 'TmCore not properly initialized', + ERROR_CODES.INTERNAL_ERROR + ); + } return this._tasks; }This would prevent cryptic "undefined" errors if someone tries to use a failed instance.
apps/mcp/src/shared/utils.ts (3)
130-151: Use fileURLToPath for robust file:// handling (Windows/UNC-safe).Current slice(7) logic is brittle for file:/// vs file://host and UNC paths. Replace with Node’s url utilities.
+import { fileURLToPath } from 'node:url'; @@ - // Strip file:// prefix - if (pathString.startsWith('file:///')) { - pathString = pathString.slice(7); - } else if (pathString.startsWith('file://')) { - pathString = pathString.slice(7); - } + // Convert file:// URI to path when applicable + if (pathString.startsWith('file://')) { + try { + pathString = fileURLToPath(pathString); + } catch { + // Fallback: leave as-is if not a valid file URL + } + }
231-244: Pass logger to handleApiResult on errors inside HOF.Currently errors aren’t logged because log isn’t forwarded.
- return handleApiResult({ + return handleApiResult({ result: { success: false, error: { message: 'Could not determine project root. Please provide projectRoot argument or ensure TASK_MASTER_PROJECT_ROOT environment variable is set.' } - } + }, + log }); @@ - return handleApiResult({ + return handleApiResult({ result: { success: false, error: { message: `Operation failed: ${error.message}` } - } + }, + log });Also applies to: 260-268
23-37: Defaulting to 'master' might be misleading.If no tag is set, consider returning null consistently to signal “no tag”, letting callers decide defaults.
apps/mcp/src/tools/tasks/get-task.tool.ts (3)
64-67: Trim, filter empties, and de‑dupe task IDs.Avoid calling tmCore with empty IDs when trailing commas/spaces are present.
- const taskIds = id.split(',').map((tid) => tid.trim()); + const taskIds = Array.from( + new Set( + id + .split(',') + .map((tid) => tid.trim()) + .filter(Boolean) + ) + );
87-99: Include tag in error responses for context.Pass tag so handleApiResult can echo it; helps users operating in multi‑tag environments.
return handleApiResult({ result: { success: false, error: { message: `No tasks found for ID(s): ${id}` } }, log: context.log, - projectRoot + projectRoot, + tag });
17-33: Validate id/status shape more strictly (optional).Consider
.min(1)for id and constraining status to known values to fail fast.- id: z.string().describe('Task ID(s) to get (can be comma-separated for multiple tasks)'), + id: z.string().min(1).describe('Task ID(s) to get (comma-separated allowed)'), - status: z.string().optional().describe("Filter subtasks by status (e.g., 'pending', 'done')"), + status: z + .string() + .optional() + .describe("Filter subtasks by status (e.g., 'pending', 'done'; comma-separated)"),apps/mcp/src/tools/tasks/get-tasks.tool.ts (1)
63-71: Normalize status filters to lowercase to match TaskStatus.Prevents mismatches like 'Pending' vs 'pending'.
- const filter = - status && status !== 'all' + const filter = + status && status.toLowerCase() !== 'all' ? { - status: status - .split(',') - .map((s: string) => s.trim() as TaskStatus) + status: status + .split(',') + .map((s: string) => s.trim().toLowerCase() as TaskStatus) } : undefined;packages/tm-core/src/modules/tasks/services/task-service.ts (3)
156-170: Guard getStorageType usage with initialization (optional).getTaskList calls this.getStorageType() which dereferences storage; if callers forget to initialize, this will throw.
Option A: call
await this.initialize()at start of public methods.
Option B: add a privateensureInitialized()and use it.
89-136: Consider pushing multi‑status to storage when supported (optional).If storage adapters can accept multiple statuses, pass the array to avoid in‑memory filtering.
124-136: Potential performance: avoid double load when status filter pushed (optional).You load rawTasks then possibly allTasks again; consider a single call returning both total and filtered if adapter can.
packages/tm-core/src/common/logger/logger.ts (1)
119-130: Consider honoringsilenteven with callbacks (optional).Current logic ignores
silentwhen a callback exists; ifsilentis meant as a hard kill switch, include it.- if (this.config.logCallback) { - return level <= this.config.level; - } + if (this.config.logCallback) { + if (this.config.silent) return false; + return level <= this.config.level; + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.changeset/four-bugs-occur.md(1 hunks)apps/mcp/src/index.ts(1 hunks)apps/mcp/src/shared/utils.ts(1 hunks)apps/mcp/src/tools/tasks/get-task.tool.ts(1 hunks)apps/mcp/src/tools/tasks/get-tasks.tool.ts(1 hunks)apps/mcp/src/tools/tasks/index.ts(1 hunks)mcp-server/src/core/direct-functions/list-tasks.js(0 hunks)mcp-server/src/core/direct-functions/show-task.js(0 hunks)mcp-server/src/core/task-master-core.js(0 hunks)mcp-server/src/tools/get-task.js(0 hunks)mcp-server/src/tools/get-tasks.js(0 hunks)mcp-server/src/tools/tool-registry.js(4 hunks)packages/tm-core/src/common/interfaces/storage.interface.ts(5 hunks)packages/tm-core/src/common/logger/index.ts(1 hunks)packages/tm-core/src/common/logger/logger.spec.ts(1 hunks)packages/tm-core/src/common/logger/logger.ts(5 hunks)packages/tm-core/src/modules/storage/adapters/api-storage.ts(6 hunks)packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts(3 hunks)packages/tm-core/src/modules/tasks/services/task-service.ts(5 hunks)packages/tm-core/src/tm-core.ts(1 hunks)
💤 Files with no reviewable changes (5)
- mcp-server/src/tools/get-task.js
- mcp-server/src/tools/get-tasks.js
- mcp-server/src/core/direct-functions/list-tasks.js
- mcp-server/src/core/direct-functions/show-task.js
- mcp-server/src/core/task-master-core.js
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/git_workflow.mdc)
**/*.{test,spec}.{js,ts,jsx,tsx}: Create a test file and ensure all tests pass when all subtasks are complete; commit tests if added or modified
When all subtasks are complete, run final testing using the appropriate test runner (e.g., npm test, jest, or manual testing)
Files:
packages/tm-core/src/common/logger/logger.spec.ts
**/*.{test,spec}.*
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
Test files should follow naming conventions: .test., .spec., or _test. depending on the language
Files:
packages/tm-core/src/common/logger/logger.spec.ts
**/?(*.)+(spec|test).ts
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
In JavaScript/TypeScript projects using Jest, test files should match *.test.ts and *.spec.ts patterns
Files:
packages/tm-core/src/common/logger/logger.spec.ts
{packages/*/src/**/*.spec.ts,apps/*/src/**/*.spec.ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Place package and app unit test files alongside source as *.spec.ts under src (packages//src//.spec.ts or apps//src//.spec.ts)
Files:
packages/tm-core/src/common/logger/logger.spec.ts
{packages/*/src/**/*.@(spec|test).ts,apps/*/src/**/*.@(spec|test).ts,packages/*/tests/**/*.@(spec|test).ts,apps/*/tests/**/*.@(spec|test).ts,tests/unit/packages/*/**/*.@(spec|test).ts}
📄 CodeRabbit inference engine (CLAUDE.md)
{packages/*/src/**/*.@(spec|test).ts,apps/*/src/**/*.@(spec|test).ts,packages/*/tests/**/*.@(spec|test).ts,apps/*/tests/**/*.@(spec|test).ts,tests/unit/packages/*/**/*.@(spec|test).ts}: Do not use async/await in test functions unless the test is explicitly exercising asynchronous behavior
Use synchronous top-level imports in tests; avoid dynamic await import()
Keep test bodies synchronous whenever possible
Files:
packages/tm-core/src/common/logger/logger.spec.ts
mcp-server/src/tools/*.js
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
MCP server tools in mcp-server/src/tools/*.js must have their execute methods wrapped with the withNormalizedProjectRoot higher-order function from tools/utils.js to ensure consistent path handling.
mcp-server/src/tools/*.js: MCP tools must follow a specific structure: use server.addTool with snake_case tool names, define parameters using Zod, and implement the execute function as an async function.
All MCP tool execute methods that require access to the project root MUST be wrapped with the withNormalizedProjectRoot Higher-Order Function (HOF) from mcp-server/src/tools/utils.js.
MCP tools should always call *Direct wrappers instead of executeTaskMasterCommand, except as a fallback if a direct function is not yet implemented.
Use camelCase with Tool suffix for tool registration functions in mcp-server/src/tools/.
Use snake_case for tool names exposed to MCP clients in server.addTool definitions.
Log the start of execution with arguments (sanitized if sensitive), log successful completion with result summary, log all error conditions with appropriate log levels, and include the cache status in result logs in MCP tool files.
Do not log entire large data structures or sensitive information in MCP tool files.
Use handleApiResult to format and return the response from MCP tools.
mcp-server/src/tools/*.js: Create tool definitions in 'mcp-server/src/tools/', use Zod for parameter validation, include optional tag parameter for multi-context support, and follow established naming conventions.
For long-running operations that should not block the client, use the AsyncOperationManager in MCP tools and implement progress reporting.
Files:
mcp-server/src/tools/tool-registry.js
mcp-server/src/{tools,core/direct-functions}/*.js
📄 CodeRabbit inference engine (.cursor/rules/mcp.mdc)
mcp-server/src/{tools,core/direct-functions}/*.js: Use kebab-case for all file names in mcp-server/src/tools/ and mcp-server/src/core/direct-functions/.
Use helpers from mcp-server/src/tools/utils.js, mcp-server/src/core/utils/path-utils.js, and mcp-server/src/core/utils/ai-client-utils.js for centralized utilities.
Files:
mcp-server/src/tools/tool-registry.js
mcp-server/src/tools/**/*.js
📄 CodeRabbit inference engine (.cursor/rules/telemetry.mdc)
MCP tool files in mcp-server/src/tools/ must call the corresponding direct function wrapper and pass the result to handleApiResult(result, log) from mcp-server/src/tools/utils.js, ensuring telemetryData is included in the final MCP response.
Files:
mcp-server/src/tools/tool-registry.js
**/*.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
**/*.js: Declare and initialize global variables at the top of modules to avoid hoisting issues.
Use proper function declarations to avoid hoisting issues and initialize variables before they are referenced.
Do not reference variables before their declaration in module scope.
Use dynamic imports (import()) to avoid initialization order issues in modules.
Files:
mcp-server/src/tools/tool-registry.js
mcp-server/src/{core/utils,tools}/**/*.js
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
Place utilities specifically designed to support the MCP server implementation into the appropriate subdirectories within
mcp-server/src/(e.g., path/core logic helpers inmcp-server/src/core/utils/, tool execution/response helpers inmcp-server/src/tools/utils.js).
Files:
mcp-server/src/tools/tool-registry.js
.changeset/*.md
📄 CodeRabbit inference engine (.cursor/rules/changeset.mdc)
.changeset/*.md: When runningnpm run changesetornpx changeset add, provide a concise summary of the changes for theCHANGELOG.mdin imperative mood, typically a single line, and not a detailed Git commit message.
The changeset summary should be user-facing, describing what changed in the released version that is relevant to users or consumers of the package.
Do not use your detailed Git commit message body as the changeset summary.Write changeset entries as user-facing summaries of what users get or what was fixed, not low-level implementation details
Files:
.changeset/four-bugs-occur.md
.changeset/*
📄 CodeRabbit inference engine (.cursor/rules/new_features.mdc)
Create appropriate changesets for new features, use semantic versioning, include tagged system information in release notes, and document breaking changes if any.
Files:
.changeset/four-bugs-occur.md
🧠 Learnings (11)
📚 Learning: 2025-09-26T19:05:47.555Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1252
File: packages/ai-sdk-provider-grok-cli/package.json:11-13
Timestamp: 2025-09-26T19:05:47.555Z
Learning: In the eyaltoledano/claude-task-master repository, internal tm/ packages use a specific export pattern where the "exports" field points to TypeScript source files (./src/index.ts) while "main" points to compiled output (./dist/index.js) and "types" points to source files (./src/index.ts). This pattern is used consistently across internal packages like tm/core and tm/ai-sdk-provider-grok-cli because they are consumed directly during build-time bundling with tsdown rather than being published as separate packages.
Applied to files:
apps/mcp/src/index.tspackages/tm-core/src/tm-core.ts
📚 Learning: 2025-07-18T17:12:57.903Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-07-18T17:12:57.903Z
Learning: Applies to mcp-server/src/tools/*.js : Create tool definitions in 'mcp-server/src/tools/', use Zod for parameter validation, include optional tag parameter for multi-context support, and follow established naming conventions.
Applied to files:
apps/mcp/src/tools/tasks/get-task.tool.tsapps/mcp/src/tools/tasks/get-tasks.tool.ts
📚 Learning: 2025-07-18T17:11:36.732Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-07-18T17:11:36.732Z
Learning: Applies to mcp-server/src/tools/*.js : MCP tools must follow a specific structure: use server.addTool with snake_case tool names, define parameters using Zod, and implement the execute function as an async function.
Applied to files:
apps/mcp/src/tools/tasks/get-task.tool.tsapps/mcp/src/tools/tasks/get-tasks.tool.tsmcp-server/src/tools/tool-registry.js
📚 Learning: 2025-07-18T17:10:12.881Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dev_workflow.mdc:0-0
Timestamp: 2025-07-18T17:10:12.881Z
Learning: Use the Taskmaster command set (`task-master` CLI or MCP tools) for all task management operations: listing, expanding, updating, tagging, and status changes.
Applied to files:
apps/mcp/src/tools/tasks/get-task.tool.ts
📚 Learning: 2025-07-18T17:11:36.732Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-07-18T17:11:36.732Z
Learning: Applies to mcp-server/src/tools/*.js : Use camelCase with Tool suffix for tool registration functions in mcp-server/src/tools/.
Applied to files:
apps/mcp/src/tools/tasks/get-task.tool.tsapps/mcp/src/tools/tasks/index.tsmcp-server/src/tools/tool-registry.js
📚 Learning: 2025-07-18T17:07:39.336Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/architecture.mdc:0-0
Timestamp: 2025-07-18T17:07:39.336Z
Learning: Applies to mcp-server/src/core/direct-functions/*.js : Always use the isSilentMode() function to check silent mode state in mcp-server/src/core/direct-functions/*.js; do not access global variables like silentMode directly.
Applied to files:
packages/tm-core/src/common/logger/logger.ts
📚 Learning: 2025-07-18T17:12:57.903Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-07-18T17:12:57.903Z
Learning: Applies to scripts/modules/**/*.test.js : Test CLI and MCP interfaces with real task data, verify end-to-end workflows across tag contexts, and test error scenarios and recovery in integration tests.
Applied to files:
packages/tm-core/src/common/logger/logger.spec.ts
📚 Learning: 2025-07-18T17:10:53.657Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/glossary.mdc:0-0
Timestamp: 2025-07-18T17:10:53.657Z
Learning: Comprehensive reference for Taskmaster MCP tools and CLI commands with tagged task lists information (taskmaster.mdc).
Applied to files:
apps/mcp/src/tools/tasks/get-tasks.tool.ts
📚 Learning: 2025-07-18T17:11:36.732Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-07-18T17:11:36.732Z
Learning: Applies to mcp-server/src/tools/*.js : Use snake_case for tool names exposed to MCP clients in server.addTool definitions.
Applied to files:
mcp-server/src/tools/tool-registry.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Provide functions for updating task status within the current tag context, handling both individual tasks and subtasks, and considering subtask status when updating parent tasks.
Applied to files:
packages/tm-core/src/modules/tasks/services/task-service.ts
📚 Learning: 2025-09-03T12:20:36.005Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/index.ts:56-57
Timestamp: 2025-09-03T12:20:36.005Z
Learning: The logger functionality in tm-core should only be available through the main package entry point (import { getLogger, createLogger, setGlobalLogger } from 'tm-core'), not as a separate subpath export like other modules such as auth or storage.
Applied to files:
packages/tm-core/src/tm-core.ts
🧬 Code graph analysis (7)
apps/mcp/src/tools/tasks/get-task.tool.ts (3)
apps/mcp/src/shared/utils.ts (2)
withNormalizedProjectRoot(183-270)handleApiResult(43-112)apps/mcp/src/shared/types.ts (1)
MCPContext(24-32)packages/tm-core/src/tm-core.ts (1)
createTmCore(215-217)
apps/mcp/src/shared/utils.ts (1)
mcp-server/src/tools/utils.js (1)
versionInfo(295-295)
packages/tm-core/src/modules/storage/adapters/api-storage.ts (2)
packages/tm-core/src/modules/storage/utils/api-client.ts (1)
ApiClient(24-146)packages/tm-core/src/modules/auth/managers/auth-manager.ts (1)
AuthManager(26-286)
packages/tm-core/src/common/logger/logger.spec.ts (2)
packages/tm-core/src/common/logger/index.ts (3)
Logger(6-6)LogLevel(6-6)LogCallback(7-7)packages/tm-core/src/common/logger/logger.ts (3)
Logger(44-354)child(343-353)LogCallback(28-30)
apps/mcp/src/tools/tasks/get-tasks.tool.ts (3)
apps/mcp/src/shared/utils.ts (2)
withNormalizedProjectRoot(183-270)handleApiResult(43-112)apps/mcp/src/shared/types.ts (1)
MCPContext(24-32)packages/tm-core/src/tm-core.ts (1)
createTmCore(215-217)
mcp-server/src/tools/tool-registry.js (2)
apps/mcp/src/tools/tasks/get-tasks.tool.ts (1)
registerGetTasksTool(39-166)apps/mcp/src/tools/tasks/get-task.tool.ts (1)
registerGetTaskTool(40-136)
packages/tm-core/src/tm-core.ts (6)
packages/tm-core/src/common/logger/logger.ts (3)
LoggerConfig(32-42)Logger(44-354)error(250-253)packages/tm-core/src/modules/tasks/tasks-domain.ts (1)
TasksDomain(29-251)packages/tm-core/src/modules/workflow/workflow-domain.ts (1)
WorkflowDomain(18-98)packages/tm-core/src/modules/git/git-domain.ts (1)
GitDomain(14-247)packages/tm-core/src/modules/config/config-domain.ts (1)
ConfigDomain(15-116)packages/tm-core/src/modules/integration/integration-domain.ts (1)
IntegrationDomain(17-34)
🪛 LanguageTool
.changeset/four-bugs-occur.md
[style] ~7-~7: Consider a more concise word here.
Context: ...from get-tasks and get-task mcp tool - In an effort to reduce complexity and context bloat for...
(IN_ORDER_TO_PREMIUM)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Build
- GitHub Check: setup
- GitHub Check: Format Check
- GitHub Check: Typecheck
- GitHub Check: Test
- GitHub Check: update-docs
🔇 Additional comments (28)
packages/tm-core/src/common/logger/logger.spec.ts (1)
1-389: Comprehensive test coverage for MCP logging integration.The test suite thoroughly covers the Logger's MCP integration features including callback-based logging, MCP mode behavior, child logger inheritance, dynamic configuration updates, formatting, and edge cases. The environment variable cleanup logic in
beforeEach/afterEachensures test isolation.packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts (2)
54-64: Storage interface methods implemented correctly for file storage.The renamed
getStorageType()method and newgetCurrentBriefName()method correctly reflect file storage characteristics. ReturningnullforgetCurrentBriefName()is appropriate since briefs are only applicable to API storage.
383-397: Clear error for unsupported operation.The
updateTaskWithPrompt()implementation correctly throws an error with a clear message explaining that file storage requires client-side AI processing before callingupdateTask(). This maintains a clean separation of concerns.packages/tm-core/src/common/interfaces/storage.interface.ts (2)
78-93: Well-documented new method for AI-powered task updates.The
updateTaskWithPrompt()method is thoroughly documented, clearly explaining the differences between file storage (client-side AI processing) and API storage (server-side processing with mode options). The parameter documentation is comprehensive and the options structure is well-designed.
185-195: Clean discriminator methods for storage type detection.The addition of
getStorageType()andgetCurrentBriefName()provides a clean runtime mechanism to distinguish between file and API storage implementations. The return types appropriately handle both cases.packages/tm-core/src/tm-core.ts (2)
142-147: Good validation for absolute path requirement.The validation that
projectPathmust be absolute prevents subtle bugs from relative path handling. The error message clearly indicates what was received and what is expected.
159-198: Robust initialization with proper error handling.The initialization flow correctly:
- Creates the logger first (before anything that might log)
- Initializes ConfigManager before domain facades
- Applies configuration overrides if provided
- Calls async initialization only for domains that need it (tasks)
- Wraps errors in TaskMasterError with operation context
- Logs errors when the logger is available
apps/mcp/src/index.ts (1)
7-7: Clean addition of tasks tools to public API.The new export statement properly exposes task-related tool registration functions, aligning with the MCP tooling reorganization.
packages/tm-core/src/common/logger/index.ts (1)
7-7: Appropriate type exports for MCP integration.Adding
LogCallbackandLogObjectto the public type surface enables external consumers (like MCP tools) to type their logging callbacks correctly. This aligns with the MCP logging integration tested in logger.spec.ts.apps/mcp/src/tools/tasks/index.ts (1)
1-7: Clean aggregator module following MCP conventions.The barrel export module correctly uses camelCase with "Tool" suffix for registration function names, as specified in the coding guidelines. This provides a clean entry point for task-related MCP tools.
Based on learnings
mcp-server/src/tools/tool-registry.js (2)
41-53: Successful migration to TypeScript tooling from @tm/mcp.The import changes correctly migrate from local JavaScript tool implementations to the new TypeScript implementations in the @tm/mcp package. The naming conventions follow the established guidelines: snake_case for tool names exposed to MCP clients, and camelCase with "Tool" suffix for registration functions.
Based on learnings
70-71: Tool registry bindings updated correctly.The registry correctly maps the tool names
get_tasksandget_taskto their new registration functions from @tm/mcp, maintaining backward compatibility at the tool name level while leveraging the new implementation.apps/mcp/src/shared/utils.ts (2)
48-57: Tag plumbing: good addition and naming.Destructuring to providedTag is clear; behavior is backward‑compatible.
61-69: Prefer provided tag; sensible fallback.Logic is correct and avoids unnecessary fs reads when tag is supplied.
apps/mcp/src/tools/tasks/get-task.tool.ts (1)
55-61: Logger wiring via logCallback looks correct.createTmCore loggerConfig uses MCP callback properly.
apps/mcp/src/tools/tasks/get-tasks.tool.ts (1)
145-146: Good: echoing result.tag ensures correct brief/tag in payload.This aligns with handleApiResult’s providedTag path.
packages/tm-core/src/modules/tasks/services/task-service.ts (7)
171-193: Good error wrapping with metadata; preserves NO_BRIEF_SELECTED.The structured TaskMasterError usage is clear.
496-500: Public getStorageType signature looks fine.Returning concrete type ('file' | 'api') improves downstream logic.
238-244: Leveraging getTaskList for status queries is clean.Keeps filtering logic centralized.
304-423: Next-task selection logic reads well; deterministic sort.Nice priority/deps/tie-break ordering.
1-18: Imports and error types organization are good.Consistent module boundaries.
47-62: Initialization path is clear.Storage create + initialize flows look correct.
522-633: Inconsistent auto-init pattern detected in updateTaskStatus.TaskService is pre-initialized by TmCore's construction sequence (TmCore → TasksDomain → taskService.initialize()), so defensive auto-init checks are not strictly necessary. However,
updateTaskStatus(line 638) lacks the auto-init guard thatupdateTask(line 522) andupdateTaskWithPrompt(line 579) have, despite all three performing storage operations. This inconsistency should be either addressed for consistency or confirmed as intentional.All other public methods (
getTaskList,getTask,getTasksByStatus,setActiveTag) do not have auto-init checks, which aligns with the assumption that initialization occurs before method access through TmCore.packages/tm-core/src/common/logger/logger.ts (3)
110-114: MCP mode + callback handling makes sense.Silence only when no callback is provided.
210-245: Callback routing and type guard look solid.Supports both function and LogObject shapes.
327-338: getConfig typing is acceptable.Intersection with LoggerConfig exposes logCallback while preserving concrete fields.
packages/tm-core/src/modules/storage/adapters/api-storage.ts (2)
148-160: LGTM! Clean accessor methods.Both
getStorageType()andgetCurrentBriefName()are well-implemented. The storage type correctly returns the'api'literal, and the brief name retrieval safely handles missing context.
557-601: Error handling and logging are well-structured.The success flag check, comprehensive error wrapping, and informative logging provide good observability and debugging support. The error context includes all relevant parameters for troubleshooting.
| remove file and complexity report parameter from get-tasks and get-task mcp tool | ||
|
|
||
| - In an effort to reduce complexity and context bloat for ai coding agents, we simplified the parameters of these tools |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Capitalize MCP and focus on user-facing impact.
The changeset should emphasize what users/consumers of the package get, not internal reasoning. Per coding guidelines, changeset entries should be concise, user-facing summaries in imperative mood, not implementation details.
Issues:
- Line 5: "mcp tool" should be "MCP tool"
- Line 7: Rationale about "ai coding agents" is internal detail; the user-facing benefit is parameter simplification
Apply this diff to improve clarity and align with changeset conventions:
- remove file and complexity report parameter from get-tasks and get-task mcp tool
+ Simplify MCP get-tasks and get-task tool parametersFor the detail line, consider removing the internal rationale or reframing it to focus on what users experience:
- - In an effort to reduce complexity and context bloat for ai coding agents, we simplified the parameters of these tools
+ - Removed file and complexity report fields to streamline tool parameters📝 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.
| remove file and complexity report parameter from get-tasks and get-task mcp tool | |
| - In an effort to reduce complexity and context bloat for ai coding agents, we simplified the parameters of these tools | |
| Simplify MCP get-tasks and get-task tool parameters | |
| - Removed file and complexity report fields to streamline tool parameters |
🧰 Tools
🪛 LanguageTool
[style] ~7-~7: Consider a more concise word here.
Context: ...from get-tasks and get-task mcp tool - In an effort to reduce complexity and context bloat for...
(IN_ORDER_TO_PREMIUM)
🤖 Prompt for AI Agents
In .changeset/four-bugs-occur.md around lines 5 to 7, capitalize "MCP" and
replace the internal rationale about "ai coding agents" with a concise,
user-facing imperative summary: state that the get-tasks and get-task MCP tools
have had their parameters simplified to reduce complexity for users and improve
usability; remove or rephrase any implementation/internal reasoning so the
changeset focuses on the consumer-facing impact.
| projectRoot: z | ||
| .string() | ||
| .describe( | ||
| 'Absolute path to the project root directory (Optional, usually from session)' | ||
| ), | ||
| tag: z.string().optional().describe('Tag context to operate on') | ||
| }); |
There was a problem hiding this comment.
Make projectRoot optional to match wrapper behavior.
Zod requires projectRoot, but withNormalizedProjectRoot expects it optional; requests will fail without it.
- projectRoot: z
- .string()
- .describe(
- 'Absolute path to the project root directory (Optional, usually from session)'
- ),
+ projectRoot: z
+ .string()
+ .optional()
+ .describe(
+ 'Absolute path to the project root directory (Optional, usually from session)'
+ ),Based on learnings.
📝 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.
| projectRoot: z | |
| .string() | |
| .describe( | |
| 'Absolute path to the project root directory (Optional, usually from session)' | |
| ), | |
| tag: z.string().optional().describe('Tag context to operate on') | |
| }); | |
| projectRoot: z | |
| .string() | |
| .optional() | |
| .describe( | |
| 'Absolute path to the project root directory (Optional, usually from session)' | |
| ), | |
| tag: z.string().optional().describe('Tag context to operate on') | |
| }); |
🤖 Prompt for AI Agents
In apps/mcp/src/tools/tasks/get-task.tool.ts around lines 27 to 33, the Zod
schema currently requires projectRoot but the wrapper withNormalizedProjectRoot
treats it as optional; change projectRoot to be optional (use
z.string().optional()) and keep the existing description so requests lacking
projectRoot won't fail when wrapped.
| projectRoot: z | ||
| .string() | ||
| .describe('The directory of the project. Must be an absolute path.'), | ||
| status: z |
There was a problem hiding this comment.
Make projectRoot optional (aligns with wrapper).
Required here contradicts withNormalizedProjectRoot and the description.
- projectRoot: z
- .string()
- .describe('The directory of the project. Must be an absolute path.'),
+ projectRoot: z
+ .string()
+ .optional()
+ .describe('The directory of the project (absolute path). Optional; usually sourced from session/env.'),Based on learnings.
📝 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.
| projectRoot: z | |
| .string() | |
| .describe('The directory of the project. Must be an absolute path.'), | |
| status: z | |
| projectRoot: z | |
| .string() | |
| .optional() | |
| .describe('The directory of the project (absolute path). Optional; usually sourced from session/env.'), | |
| status: z |
🤖 Prompt for AI Agents
In apps/mcp/src/tools/tasks/get-tasks.tool.ts around lines 18–21, the zod schema
currently requires projectRoot but it should be optional to match the
wrapper/withNormalizedProjectRoot; change the schema so projectRoot is optional
(e.g., use z.string().optional() or z.optional(z.string())), update the
description to remove the "Must be an absolute path" wording or note it's
optional, and ensure any local typings/usages handle projectRoot being possibly
undefined (rely on withNormalizedProjectRoot or add a guard where needed).
| /** | ||
| * Get or create API client instance with auth | ||
| */ | ||
| private getApiClient(): ApiClient { | ||
| if (!this.apiClient) { | ||
| const apiEndpoint = | ||
| process.env.TM_BASE_DOMAIN || process.env.TM_PUBLIC_BASE_DOMAIN; | ||
|
|
||
| if (!apiEndpoint) { | ||
| throw new TaskMasterError( | ||
| 'API endpoint not configured. Please set TM_PUBLIC_BASE_DOMAIN environment variable.', | ||
| ERROR_CODES.MISSING_CONFIGURATION, | ||
| { operation: 'getApiClient' } | ||
| ); | ||
| } | ||
|
|
||
| const context = this.ensureBriefSelected('getApiClient'); | ||
| const authManager = AuthManager.getInstance(); | ||
|
|
||
| this.apiClient = new ApiClient({ | ||
| baseUrl: apiEndpoint, | ||
| authManager, | ||
| accountId: context.orgId | ||
| }); | ||
| } | ||
|
|
||
| return this.apiClient; | ||
| } |
There was a problem hiding this comment.
Cached apiClient retains stale orgId when context changes.
The apiClient is cached with the accountId (orgId) from the current context at creation time. If a user switches contexts (e.g., changes organizations or briefs), the cached client continues using the original orgId in the x-account-id header, potentially sending requests to the wrong organization.
Consider one of these solutions:
Option 1 (Recommended): Don't cache the apiClient
private getApiClient(): ApiClient {
- if (!this.apiClient) {
- const apiEndpoint =
- process.env.TM_BASE_DOMAIN || process.env.TM_PUBLIC_BASE_DOMAIN;
+ const apiEndpoint =
+ process.env.TM_BASE_DOMAIN || process.env.TM_PUBLIC_BASE_DOMAIN;
- if (!apiEndpoint) {
- throw new TaskMasterError(
- 'API endpoint not configured. Please set TM_PUBLIC_BASE_DOMAIN environment variable.',
- ERROR_CODES.MISSING_CONFIGURATION,
- { operation: 'getApiClient' }
- );
- }
+ if (!apiEndpoint) {
+ throw new TaskMasterError(
+ 'API endpoint not configured. Please set TM_PUBLIC_BASE_DOMAIN environment variable.',
+ ERROR_CODES.MISSING_CONFIGURATION,
+ { operation: 'getApiClient' }
+ );
+ }
- const context = this.ensureBriefSelected('getApiClient');
- const authManager = AuthManager.getInstance();
+ const context = this.ensureBriefSelected('getApiClient');
+ const authManager = AuthManager.getInstance();
- this.apiClient = new ApiClient({
- baseUrl: apiEndpoint,
- authManager,
- accountId: context.orgId
- });
- }
+ return new ApiClient({
+ baseUrl: apiEndpoint,
+ authManager,
+ accountId: context.orgId
+ });
-
- return this.apiClient;
}Option 2: Clear apiClient when context might have changed
Add to the close() method:
async close(): Promise<void> {
this.initialized = false;
this.tagsCache.clear();
+ this.apiClient = undefined;
}And clear it in methods that depend on context after operations that might change context. However, Option 1 is cleaner since ApiClient construction is lightweight.
📝 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.
| /** | |
| * Get or create API client instance with auth | |
| */ | |
| private getApiClient(): ApiClient { | |
| if (!this.apiClient) { | |
| const apiEndpoint = | |
| process.env.TM_BASE_DOMAIN || process.env.TM_PUBLIC_BASE_DOMAIN; | |
| if (!apiEndpoint) { | |
| throw new TaskMasterError( | |
| 'API endpoint not configured. Please set TM_PUBLIC_BASE_DOMAIN environment variable.', | |
| ERROR_CODES.MISSING_CONFIGURATION, | |
| { operation: 'getApiClient' } | |
| ); | |
| } | |
| const context = this.ensureBriefSelected('getApiClient'); | |
| const authManager = AuthManager.getInstance(); | |
| this.apiClient = new ApiClient({ | |
| baseUrl: apiEndpoint, | |
| authManager, | |
| accountId: context.orgId | |
| }); | |
| } | |
| return this.apiClient; | |
| } | |
| /** | |
| * Get or create API client instance with auth | |
| */ | |
| private getApiClient(): ApiClient { | |
| const apiEndpoint = | |
| process.env.TM_BASE_DOMAIN || process.env.TM_PUBLIC_BASE_DOMAIN; | |
| if (!apiEndpoint) { | |
| throw new TaskMasterError( | |
| 'API endpoint not configured. Please set TM_PUBLIC_BASE_DOMAIN environment variable.', | |
| ERROR_CODES.MISSING_CONFIGURATION, | |
| { operation: 'getApiClient' } | |
| ); | |
| } | |
| const context = this.ensureBriefSelected('getApiClient'); | |
| const authManager = AuthManager.getInstance(); | |
| return new ApiClient({ | |
| baseUrl: apiEndpoint, | |
| authManager, | |
| accountId: context.orgId | |
| }); | |
| } |
🤖 Prompt for AI Agents
In packages/tm-core/src/modules/storage/adapters/api-storage.ts around lines 899
to 926, the code caches an ApiClient instance with the orgId from the current
context which becomes stale when the user switches contexts; to fix this, stop
caching the client: change getApiClient to always construct and return a new
ApiClient using the current context (ensure you call ensureBriefSelected to get
up-to-date context and pass context.orgId as accountId), remove or stop
referencing this.apiClient in this method (and optionally remove the field if
unused elsewhere), and if you prefer the alternative approach instead, clear
this.apiClient in close() and at any points where context changes so subsequent
calls rebuild the client.
2 participants
What type of PR is this?
Description
Related Issues
How to Test This
# Example commands or stepsExpected result:
Contributor Checklist
npm run changesetnpm testnpm run format-check(ornpm run formatto fix)Changelog Entry
For Maintainers
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
Documentation