chore: refactor tm-core to host more of our "core" commands#1331
chore: refactor tm-core to host more of our "core" commands#1331Crunchyman-ralph merged 9 commits intonextfrom
Conversation
|
WalkthroughIntroduces a new TmCore façade (createTmCore/TmCore), reorganizes @tm/core into domain facades and managers/services/adapters, renames storage methods (getType → getStorageType), rewrites many import/export paths, removes several old index barrels, and updates CLI/extension callers to the new public API. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI / Extension
participant Factory as createTmCore()
participant TmCore as TmCore (façade)
participant Config as ConfigManager
participant Tasks as TasksDomain
participant Auth as AuthDomain
participant Services as Underlying services
CLI->>Factory: createTmCore({ projectPath, configuration? })
Factory->>TmCore: new TmCore(options)
TmCore->>Config: instantiate & apply overrides
TmCore->>Tasks: construct TasksDomain(ConfigManager)
TmCore->>Auth: construct AuthDomain(ConfigManager)
Tasks->>Services: wire TaskService, TaskExecutionService, TaskLoaderService
Auth->>Services: wire AuthManager, CredentialStore, OAuthService
TmCore-->>CLI: return TmCore (tasks, auth, config, workflow, git, integration)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/tm-core/src/modules/tasks/entities/task.entity.ts (1)
202-210: Fix ID collision bug and type inconsistency in subtask ID generation.This method has two issues:
ID collision (critical): Using
this.subtasks.length + 1for ID generation can create collisions. If a subtask is removed from the middle of the array, the next generated ID may match an existing subtask's ID.Type inconsistency (major): The method creates a numeric ID, but the constructor (lines 54-58) normalizes all subtask IDs to strings. This means subtasks added via
addSubtaskhave a different ID type than subtasks loaded from storage, which can cause comparison failures or unexpected behavior.Apply this diff to fix both issues:
addSubtask(subtask: Omit<Subtask, 'id' | 'parentId'>): void { - const nextId = this.subtasks.length + 1; + const maxId = this.subtasks.reduce((max, s) => { + const numId = typeof s.id === 'string' ? parseInt(s.id, 10) : s.id; + return Math.max(max, isNaN(numId) ? 0 : numId); + }, 0); + const nextId = String(maxId + 1); this.subtasks.push({ ...subtask, id: nextId, parentId: this.id }); this.updatedAt = new Date().toISOString(); }packages/tm-core/src/modules/auth/services/credential-store.test.ts (1)
24-26: Fix the mock path to match the actual import.The mock path
'../logger/index.js'doesn't match the actual import incredential-store.ts(line 9), which is'../../../common/logger/index.js'. This mock won't be applied, potentially causing test failures.Apply this diff to fix the mock path:
-vi.mock('../logger/index.js', () => ({ +vi.mock('../../../common/logger/index.js', () => ({ getLogger: () => mockLogger }));packages/tm-core/src/modules/config/managers/config-manager.spec.ts (1)
16-20: Fix incorrect mock paths for services.The mock paths use
'./services/...'which would resolve tomanagers/services/...since the test file is in themanagers/directory. The services are actually at the config module level.Apply this fix:
-vi.mock('./services/config-loader.service.js'); -vi.mock('./services/config-merger.service.js'); -vi.mock('./services/runtime-state-manager.service.js'); -vi.mock('./services/config-persistence.service.js'); -vi.mock('./services/environment-config-provider.service.js'); +vi.mock('../services/config-loader.service.js'); +vi.mock('../services/config-merger.service.js'); +vi.mock('../services/runtime-state-manager.service.js'); +vi.mock('../services/config-persistence.service.js'); +vi.mock('../services/environment-config-provider.service.js');packages/tm-core/src/modules/storage/index.ts (1)
26-55: EnablestripInternalin tsconfig.json and add@internalJSDoc to placeholders.
StorageAdapterandPlaceholderStorageare publicly accessible via the"./storage"export in package.json and will leak into generated.d.tsfiles. WithstripInternalcurrently disabled, the@deprecatedannotation alone won't prevent emission.
- Add
"stripInternal": truetopackages/tm-core/tsconfig.json→compilerOptions- Add
@internalJSDoc tag to bothStorageAdapterinterface andPlaceholderStorageclass (in addition to existing@deprecatedon the class)This ensures they're stripped from published typings while remaining available for internal tests.
🧹 Nitpick comments (17)
packages/tm-core/src/modules/tasks/parser/index.ts (1)
9-10: Remove or clarify the status of commented-out parser exports.The commented parser implementations should either be removed or retained with a clear TODO/issue reference indicating the expected timeline for implementation.
Apply this diff to remove stale comments or convert to TODO:
-// Parser implementations will be defined here -// export * from './prd-parser.js'; -// export * from './task-parser.js'; -// export * from './markdown-parser.js';Alternatively, if these are intentional placeholders, convert to a tracked TODO:
-// Parser implementations will be defined here -// export * from './prd-parser.js'; -// export * from './task-parser.js'; -// export * from './markdown-parser.js'; +// TODO: Implement and export specific parsers in related tasks +// - PRD parser (prd-parser.js) +// - Task parser (task-parser.js) +// - Markdown parser (markdown-parser.js)packages/tm-core/src/modules/dependencies/index.ts (1)
1-8: Clarify the status and timeline for this placeholder module.This file establishes the Dependencies domain structure but provides no public exports or functionality. While placeholder modules are reasonable for documenting intended architecture, verify that:
- This module will be implemented during this PR or explicitly deferred to follow-up work.
- The TmCore facade (or domain-specific facade) will reference this module once implemented, so it's not left orphaned.
Based on learnings, the Dependencies domain should eventually expose cycle detection (DFS), self-dependency handling, duplicate prevention, and validation capabilities migrated from
scripts/modules/dependency-manager.js.Would you like me to help implement the migration from the existing dependency-manager.js, or should this placeholder be addressed in a follow-up PR after the refactoring is merged?
packages/tm-core/src/modules/storage/adapters/api-storage.ts (1)
66-70: Pre-existing TODO: Incomplete repository implementation.The TODO comment indicates that SupabaseTaskRepository doesn't fully implement all TaskRepository methods yet and requires a type cast. While this is a pre-existing issue (not introduced by this refactor), consider tracking this technical debt.
Would you like me to open a new issue to track completing the SupabaseTaskRepository implementation?
packages/tm-core/src/modules/config/config-domain.ts (2)
34-36: Tighten return types; avoidanyon public surface.
- getModelConfig currently returns
any. Expose a concrete type (e.g.,ModelConfig) orunknownif intentionally opaque.- getConfigSources also returns untyped data; consider a readonly, structured type for debuggability and stability.
This keeps the facade’s API self-documenting and reduces downstream
anyleakage.Also applies to: 110-112
52-57: Clarify "explicit vs computed" API config semantics.You expose both
isApiExplicitlyConfigured()andRuntimeStorageConfig.apiConfigured(computed). Add docstrings clarifying the difference or consolidate to one authoritative check to prevent ambiguity for consumers.Also applies to: 27-29
packages/tm-core/src/modules/tasks/tasks-domain.ts (1)
104-116: Make return types explicit for public methods.For a stable public API, annotate return types instead of relying on inference:
- checkInProgressConflicts(...): Promise
- getNextAvailable(): Promise<string | null>
- getExecutionOrder(...): Subtask[]
- detectTestCommand/checkGitWorkingTree/validateRequiredTools/detectDefaultBranch: concrete result types from PreflightChecker
This enhances discoverability and prevents accidental type regressions.
Also applies to: 120-123, 144-146, 160-169, 174-176, 181-183
packages/tm-core/src/modules/auth/auth-domain.ts (1)
20-23: Consider optional dependency injection for testability.Allow passing an
AuthManagerinstance to the constructor (defaulting to singleton) to simplify unit testing/mocking:- constructor() { - this.authManager = AuthManager.getInstance(); - } + constructor(manager: AuthManager = AuthManager.getInstance()) { + this.authManager = manager; + }Non-breaking and improves test ergonomics.
packages/tm-core/src/index.ts (3)
32-43: Export surface is broad; prefer minimal, domain-level exports to avoid lock-in.This file re-exports many internals (including service-layer types). It increases coupling and raises semver risk when services change. Recommend:
- Curate a minimal public surface (only what downstreams consume).
- Avoid re-exporting from service files; add domain-level types barrels (e.g., modules/tasks/types.ts) and export from there.
- Replace
export *with explicit named exports for stability and discoverability.Based on learnings.
Also applies to: 46-62, 70-102
86-89: Expose commit message validation/parse types alongside CommitMessageOptions.GitDomain exposes validateCommitMessage/parseCommitMessage; consumers will likely need ValidationResult and ParsedCommitMessage. Re-export them here for a coherent public API.
Apply:
export type { - CommitMessageOptions + CommitMessageOptions, + ValidationResult, + ParsedCommitMessage } from './modules/git/services/commit-message-generator.js';
46-62: Don’t leak service file paths in public API.Types are exported from task-service, task-execution-service, and preflight-checker.service. Suggest creating modules/tasks/public-types.ts that re-exports stable shapes; then re-export from that file here. It lets you refactor services without breaking API.
Example new file (modules/tasks/public-types.ts):
export type { TaskListResult, GetTaskListOptions } from './services/task-service.js'; export type { StartTaskOptions, StartTaskResult, ConflictCheckResult } from './services/task-execution-service.js'; export type { PreflightResult, CheckResult } from './services/preflight-checker.service.js';Then update index.ts:
-export type { TaskListResult, GetTaskListOptions } from './modules/tasks/services/task-service.js'; -export type { StartTaskOptions, StartTaskResult, ConflictCheckResult } from './modules/tasks/services/task-execution-service.js'; -export type { PreflightResult, CheckResult } from './modules/tasks/services/preflight-checker.service.js'; +export type * from './modules/tasks/public-types.js';packages/tm-core/src/modules/git/git-domain.ts (7)
6-10: Decouple public API from simple-git types.getStatus() returns simple-git’s StatusResult, coupling consumers to a vendor. Prefer a domain Status shape (e.g., GitStatus) and map in the adapter, keeping third-party details internal.
Apply:
-import type { StatusResult } from 'simple-git'; +// import domain type instead of vendor type +import type { GitStatus } from './types.js' // define this in modules/git/types.ts -async getStatus(): Promise<StatusResult> { +async getStatus(): Promise<GitStatus> { return this.gitAdapter.getStatus(); }Also applies to: 58-60
189-214: Avoidanyin public API for logs/remotes; define domain types.Returning
any/any[]weakens the contract. Define and return domain types, e.g., CommitLogEntry, CommitSummary, RemoteInfo.Apply:
-async getCommitLog(options?: { maxCount?: number }): Promise<any[]> { +async getCommitLog(options?: { maxCount?: number }): Promise<CommitLogEntry[]> { return this.gitAdapter.getCommitLog(options); } -async getLastCommit(): Promise<any> { +async getLastCommit(): Promise<CommitSummary> { return this.gitAdapter.getLastCommit(); } -async getRemotes(): Promise<any[]> { +async getRemotes(): Promise<RemoteInfo[]> { return this.gitAdapter.getRemotes(); }And export these types from modules/git/types.ts, then from the package index.
218-237: Add explicit return types for validator/parser methods.Annotate return types to lock the public contract and aid consumers.
Apply:
-import { CommitMessageGenerator } from './services/commit-message-generator.js'; -import type { CommitMessageOptions } from './services/commit-message-generator.js'; +import { CommitMessageGenerator } from './services/commit-message-generator.js'; +import type { CommitMessageOptions, ValidationResult, ParsedCommitMessage } from './services/commit-message-generator.js'; -validateCommitMessage(message: string) { +validateCommitMessage(message: string): ValidationResult { return this.commitGenerator.validateConventionalCommit(message); } -parseCommitMessage(message: string) { +parseCommitMessage(message: string): ParsedCommitMessage { return this.commitGenerator.parseCommitMessage(message); }Also re-export ValidationResult and ParsedCommitMessage from the package root (see related comment in index.ts).
18-21: Enable dependency injection for testability/customization.Constructor hard-wires GitAdapter and CommitMessageGenerator. Allow optional injection for testing and custom templates/scope mappings.
Apply:
-export class GitDomain { +export class GitDomain { private gitAdapter: GitAdapter; private commitGenerator: CommitMessageGenerator; - constructor(projectPath: string) { - this.gitAdapter = new GitAdapter(projectPath); - this.commitGenerator = new CommitMessageGenerator(); - } + constructor( + projectPath: string, + deps?: { + gitAdapter?: GitAdapter; + commitGenerator?: CommitMessageGenerator; + commitTemplates?: Record<string, string>; + scopeMappings?: Record<string, string>; + scopePriorities?: Record<string, number>; + } + ) { + this.gitAdapter = deps?.gitAdapter ?? new GitAdapter(projectPath); + this.commitGenerator = + deps?.commitGenerator ?? + new CommitMessageGenerator(deps?.commitTemplates, deps?.scopeMappings, deps?.scopePriorities); + }
65-74: Status summary shape: document and type it.getStatusSummary returns an inline object type. Extract a named type (e.g., GitStatusSummary) and export it so callers can import a stable type.
174-184: Commit options: document behavior and edge cases.Options like enforceNonDefaultBranch and force can be dangerous. Add JSDoc specifying precedence, defaults, and error semantics (e.g., what happens on default branch when enforceNonDefaultBranch = true).
202-207: Consider exposing remote names/URLs directly.hasRemote() is coarse. Returning RemoteInfo[] (even when empty) can eliminate the need for a separate boolean call and reduce round-trips.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (67)
package.json(2 hunks)packages/tm-core/src/common/interfaces/index.ts(1 hunks)packages/tm-core/src/executors/index.ts(0 hunks)packages/tm-core/src/index.ts(1 hunks)packages/tm-core/src/modules/ai/index.ts(1 hunks)packages/tm-core/src/modules/ai/providers/base-provider.ts(2 hunks)packages/tm-core/src/modules/auth/auth-domain.ts(1 hunks)packages/tm-core/src/modules/auth/index.ts(1 hunks)packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts(1 hunks)packages/tm-core/src/modules/auth/managers/auth-manager.ts(1 hunks)packages/tm-core/src/modules/auth/services/credential-store.test.ts(1 hunks)packages/tm-core/src/modules/auth/services/credential-store.ts(1 hunks)packages/tm-core/src/modules/auth/services/oauth-service.ts(1 hunks)packages/tm-core/src/modules/auth/services/organization.service.ts(1 hunks)packages/tm-core/src/modules/auth/services/supabase-session-storage.ts(1 hunks)packages/tm-core/src/modules/commands/index.ts(1 hunks)packages/tm-core/src/modules/config/config-domain.ts(1 hunks)packages/tm-core/src/modules/config/index.ts(2 hunks)packages/tm-core/src/modules/config/managers/config-manager.spec.ts(1 hunks)packages/tm-core/src/modules/config/managers/config-manager.ts(1 hunks)packages/tm-core/src/modules/config/services/config-loader.service.spec.ts(1 hunks)packages/tm-core/src/modules/config/services/config-loader.service.ts(1 hunks)packages/tm-core/src/modules/config/services/config-merger.service.ts(1 hunks)packages/tm-core/src/modules/config/services/config-persistence.service.ts(1 hunks)packages/tm-core/src/modules/config/services/environment-config-provider.service.ts(1 hunks)packages/tm-core/src/modules/config/services/runtime-state-manager.service.spec.ts(1 hunks)packages/tm-core/src/modules/config/services/runtime-state-manager.service.ts(1 hunks)packages/tm-core/src/modules/dependencies/index.ts(1 hunks)packages/tm-core/src/modules/execution/executors/base-executor.ts(1 hunks)packages/tm-core/src/modules/execution/executors/claude-executor.ts(1 hunks)packages/tm-core/src/modules/execution/executors/executor-factory.ts(1 hunks)packages/tm-core/src/modules/execution/index.ts(1 hunks)packages/tm-core/src/modules/execution/services/executor-service.ts(1 hunks)packages/tm-core/src/modules/execution/types.ts(1 hunks)packages/tm-core/src/modules/git/git-domain.ts(1 hunks)packages/tm-core/src/modules/git/index.ts(1 hunks)packages/tm-core/src/modules/git/services/branch-name-generator.spec.ts(1 hunks)packages/tm-core/src/modules/integration/clients/supabase-client.ts(1 hunks)packages/tm-core/src/modules/integration/services/export.service.ts(1 hunks)packages/tm-core/src/modules/reports/index.ts(1 hunks)packages/tm-core/src/modules/reports/managers/complexity-report-manager.ts(1 hunks)packages/tm-core/src/modules/storage/adapters/api-storage.ts(1 hunks)packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts(1 hunks)packages/tm-core/src/modules/storage/adapters/file-storage/format-handler.ts(1 hunks)packages/tm-core/src/modules/storage/index.ts(2 hunks)packages/tm-core/src/modules/storage/services/storage-factory.ts(1 hunks)packages/tm-core/src/modules/tasks/entities/task.entity.ts(1 hunks)packages/tm-core/src/modules/tasks/parser/index.ts(1 hunks)packages/tm-core/src/modules/tasks/repositories/supabase/dependency-fetcher.ts(1 hunks)packages/tm-core/src/modules/tasks/repositories/supabase/supabase-task-repository.ts(1 hunks)packages/tm-core/src/modules/tasks/repositories/task-repository.interface.ts(1 hunks)packages/tm-core/src/modules/tasks/services/preflight-checker.service.ts(1 hunks)packages/tm-core/src/modules/tasks/services/task-execution-service.ts(1 hunks)packages/tm-core/src/modules/tasks/services/task-loader.service.ts(1 hunks)packages/tm-core/src/modules/tasks/services/task-service.ts(1 hunks)packages/tm-core/src/modules/tasks/tasks-domain.ts(1 hunks)packages/tm-core/src/modules/ui/index.ts(1 hunks)packages/tm-core/src/modules/workflow/managers/workflow-state-manager.spec.ts(1 hunks)packages/tm-core/src/modules/workflow/managers/workflow-state-manager.ts(1 hunks)packages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.test.ts(1 hunks)packages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.ts(2 hunks)packages/tm-core/src/modules/workflow/services/workflow-activity-logger.ts(1 hunks)packages/tm-core/src/modules/workflow/services/workflow.service.ts(1 hunks)packages/tm-core/src/modules/workflow/workflow-domain.ts(1 hunks)packages/tm-core/src/services/index.ts(0 hunks)packages/tm-core/src/task-master-core.ts(2 hunks)packages/tm-core/src/tm-core.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/tm-core/src/services/index.ts
- packages/tm-core/src/executors/index.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/workflow/managers/workflow-state-manager.spec.tspackages/tm-core/src/modules/config/managers/config-manager.spec.tspackages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.test.tspackages/tm-core/src/modules/config/services/config-loader.service.spec.tspackages/tm-core/src/modules/config/services/runtime-state-manager.service.spec.tspackages/tm-core/src/modules/auth/services/credential-store.test.tspackages/tm-core/src/modules/git/services/branch-name-generator.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/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/workflow/managers/workflow-state-manager.spec.tspackages/tm-core/src/modules/config/managers/config-manager.spec.tspackages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.test.tspackages/tm-core/src/modules/config/services/config-loader.service.spec.tspackages/tm-core/src/modules/config/services/runtime-state-manager.service.spec.tspackages/tm-core/src/modules/auth/services/credential-store.test.tspackages/tm-core/src/modules/git/services/branch-name-generator.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/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/workflow/managers/workflow-state-manager.spec.tspackages/tm-core/src/modules/config/managers/config-manager.spec.tspackages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.test.tspackages/tm-core/src/modules/config/services/config-loader.service.spec.tspackages/tm-core/src/modules/config/services/runtime-state-manager.service.spec.tspackages/tm-core/src/modules/auth/services/credential-store.test.tspackages/tm-core/src/modules/git/services/branch-name-generator.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/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/workflow/managers/workflow-state-manager.spec.tspackages/tm-core/src/modules/config/managers/config-manager.spec.tspackages/tm-core/src/modules/config/services/config-loader.service.spec.tspackages/tm-core/src/modules/config/services/runtime-state-manager.service.spec.tspackages/tm-core/src/modules/git/services/branch-name-generator.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/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/workflow/managers/workflow-state-manager.spec.tspackages/tm-core/src/modules/config/managers/config-manager.spec.tspackages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.test.tspackages/tm-core/src/modules/config/services/config-loader.service.spec.tspackages/tm-core/src/modules/config/services/runtime-state-manager.service.spec.tspackages/tm-core/src/modules/auth/services/credential-store.test.tspackages/tm-core/src/modules/git/services/branch-name-generator.spec.ts
**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
**/*.test.ts: Use established mocking patterns for dependencies such as bcrypt and Prisma in test files
Test all code paths, including edge cases and error scenarios, in test files
Use descriptive names for test functions, such as should_return_error_for_invalid_input
Files:
packages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.test.tspackages/tm-core/src/modules/auth/services/credential-store.test.ts
package.json
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
Add and update test scripts in package.json to include test, test:watch, test:coverage, test:unit, test:integration, test:e2e, and test:ci
Files:
package.json
🧠 Learnings (25)
📚 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/dependency-manager.js : Features that handle task relationships belong in 'scripts/modules/dependency-manager.js'.
Applied to files:
packages/tm-core/src/modules/dependencies/index.ts
📚 Learning: 2025-07-18T17:09:40.548Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dependencies.mdc:0-0
Timestamp: 2025-07-18T17:09:40.548Z
Learning: Applies to scripts/modules/dependency-manager.js : Use graph traversal algorithms (DFS) to detect cycles
Applied to files:
packages/tm-core/src/modules/dependencies/index.ts
📚 Learning: 2025-07-18T17:18:17.759Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-07-18T17:18:17.759Z
Learning: Applies to scripts/modules/utils.js : Detect circular dependencies, validate dependency references, and support cross-tag dependency checking (future enhancement) in dependency analysis utilities.
Applied to files:
packages/tm-core/src/modules/dependencies/index.ts
📚 Learning: 2025-07-18T17:09:40.548Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dependencies.mdc:0-0
Timestamp: 2025-07-18T17:09:40.548Z
Learning: Applies to scripts/modules/dependency-manager.js : Handle both direct and indirect self-dependencies
Applied to files:
packages/tm-core/src/modules/dependencies/index.ts
📚 Learning: 2025-07-18T17:09:45.690Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dependencies.mdc:0-0
Timestamp: 2025-07-18T17:09:45.690Z
Learning: Applies to scripts/modules/dependency-manager.js : Check for existing dependencies to prevent duplicates
Applied to files:
packages/tm-core/src/modules/dependencies/index.ts
📚 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:
packages/tm-core/src/modules/execution/index.tspackages/tm-core/src/modules/tasks/services/task-execution-service.tspackages/tm-core/src/modules/tasks/services/task-loader.service.tspackages/tm-core/src/modules/execution/types.tspackages/tm-core/src/modules/execution/executors/claude-executor.tspackages/tm-core/src/common/interfaces/index.tspackages/tm-core/src/task-master-core.tspackages/tm-core/src/index.tspackages/tm-core/src/modules/integration/services/export.service.tspackage.json
📚 Learning: 2025-10-08T19:57:00.982Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1282
File: packages/tm-core/src/utils/index.ts:16-34
Timestamp: 2025-10-08T19:57:00.982Z
Learning: For the tm-core package in the eyaltoledano/claude-task-master repository, the team prefers a minimal, need-based export strategy in index files rather than exposing all internal utilities. Exports should only be added when functions are actually consumed by other packages in the monorepo.
Applied to files:
packages/tm-core/src/modules/execution/index.tspackages/tm-core/src/index.ts
📚 Learning: 2025-07-18T17:06:04.909Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_providers.mdc:0-0
Timestamp: 2025-07-18T17:06:04.909Z
Learning: Applies to src/ai-providers/*.js : Provider modules must import the provider's create<ProviderName> function from ai-sdk/<provider-name>, and import generateText, streamText, generateObject from the core ai package, as well as the log utility from ../../scripts/modules/utils.js.
Applied to files:
packages/tm-core/src/modules/ai/providers/base-provider.tspackages/tm-core/src/modules/ai/index.tspackages/tm-core/src/common/interfaces/index.ts
📚 Learning: 2025-07-18T17:06:04.909Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_providers.mdc:0-0
Timestamp: 2025-07-18T17:06:04.909Z
Learning: Applies to src/ai-providers/*.js : Implement generate<ProviderName>Text, stream<ProviderName>Text, and generate<ProviderName>Object functions in provider modules with basic validation and try/catch error handling.
Applied to files:
packages/tm-core/src/modules/ai/providers/base-provider.tspackages/tm-core/src/modules/ai/index.ts
📚 Learning: 2025-07-18T17:06:04.909Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_providers.mdc:0-0
Timestamp: 2025-07-18T17:06:04.909Z
Learning: Applies to src/ai-providers/*.js : Create a new provider module file in src/ai-providers/ named <provider-name>.js when adding a new AI provider.
Applied to files:
packages/tm-core/src/modules/ai/providers/base-provider.tspackages/tm-core/src/modules/ai/index.ts
📚 Learning: 2025-07-18T17:06:04.909Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_providers.mdc:0-0
Timestamp: 2025-07-18T17:06:04.909Z
Learning: Applies to tests/unit/ai-providers/*.test.js : Create unit tests for the new provider in tests/unit/ai-providers/<provider-name>.test.js, mocking ai-sdk/<provider-name> and core ai module functions, and testing all exported functions for correct behavior and error handling.
Applied to files:
packages/tm-core/src/modules/ai/providers/base-provider.tspackages/tm-core/src/modules/ai/index.ts
📚 Learning: 2025-07-18T17:06:57.833Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_services.mdc:0-0
Timestamp: 2025-07-18T17:06:57.833Z
Learning: Applies to scripts/modules/task-manager/*.js : Do not implement fallback or retry logic outside `ai-services-unified.js`.
Applied to files:
packages/tm-core/src/modules/ai/providers/base-provider.ts
📚 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 src/ai-providers/*.js : Provider-specific wrappers for Vercel AI SDK functions must be implemented in src/ai-providers/*.js, each file corresponding to a provider.
Applied to files:
packages/tm-core/src/modules/ai/providers/base-provider.tspackages/tm-core/src/modules/ai/index.ts
📚 Learning: 2025-09-26T19:10:32.906Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1252
File: tsconfig.json:22-28
Timestamp: 2025-09-26T19:10:32.906Z
Learning: In the eyaltoledano/claude-task-master repository, all internal tm/ package path mappings in tsconfig.json consistently point to TypeScript source files (e.g., "./packages/*/src/index.ts") rather than built JavaScript. This is intentional architecture because tsdown bundles internal packages directly from source during build time, eliminating the need for separate compilation of internal packages.
Applied to files:
packages/tm-core/src/modules/tasks/services/task-execution-service.tspackages/tm-core/src/modules/execution/types.tspackages/tm-core/src/modules/storage/adapters/file-storage/file-storage.tspackage.json
📚 Learning: 2025-07-18T17:06:57.833Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_services.mdc:0-0
Timestamp: 2025-07-18T17:06:57.833Z
Learning: Applies to scripts/modules/task-manager/*.js : Do not import or call anything from the old `ai-services.js`, `ai-client-factory.js`, or `ai-client-utils.js` files.
Applied to files:
packages/tm-core/src/modules/tasks/services/task-service.tspackages/tm-core/src/modules/tasks/services/task-loader.service.tspackages/tm-core/src/modules/config/services/runtime-state-manager.service.tspackages/tm-core/src/task-master-core.tspackages/tm-core/src/modules/integration/services/export.service.ts
📚 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: Module dependencies should be mocked before importing the test module, following Jest's hoisting behavior.
Applied to files:
packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts
📚 Learning: 2025-07-18T17:06:04.909Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_providers.mdc:0-0
Timestamp: 2025-07-18T17:06:04.909Z
Learning: Applies to src/ai-providers/*.js : Provider modules must export three functions: generate<ProviderName>Text, stream<ProviderName>Text, and generate<ProviderName>Object.
Applied to files:
packages/tm-core/src/modules/ai/index.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/ai-services.js : Features that use AI models belong in 'scripts/modules/ai-services.js'.
Applied to files:
packages/tm-core/src/modules/ai/index.ts
📚 Learning: 2025-07-18T17:06:04.909Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_providers.mdc:0-0
Timestamp: 2025-07-18T17:06:04.909Z
Learning: Applies to scripts/modules/ai-services-unified.js : Integrate the new provider module with scripts/modules/ai-services-unified.js by importing it and adding an entry to the PROVIDER_FUNCTIONS map.
Applied to files:
packages/tm-core/src/modules/ai/index.ts
📚 Learning: 2025-07-18T17:06:57.833Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_services.mdc:0-0
Timestamp: 2025-07-18T17:06:57.833Z
Learning: Applies to scripts/modules/commands.js : Do not import or call anything from the old `ai-services.js`, `ai-client-factory.js`, or `ai-client-utils.js` files.
Applied to files:
packages/tm-core/src/modules/ai/index.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/commands.js : All new user-facing commands should be added to 'scripts/modules/commands.js'.
Applied to files:
packages/tm-core/src/modules/commands/index.ts
📚 Learning: 2025-07-31T22:07:49.716Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-31T22:07:49.716Z
Learning: Applies to scripts/modules/commands.js : Export the registerCommands function and keep the CLI setup code clean and maintainable.
Applied to files:
packages/tm-core/src/modules/commands/index.ts
📚 Learning: 2025-07-18T17:16:13.793Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-07-18T17:16:13.793Z
Learning: Applies to tests/{unit,integration,e2e}/**/*.test.js : Use sample task fixtures for consistent test data, mock file system operations, and test both success and error paths for task operations.
Applied to files:
packages/tm-core/src/modules/config/services/config-loader.service.spec.ts
📚 Learning: 2025-10-15T14:43:40.410Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1314
File: packages/tm-core/src/auth/credential-store.ts:92-94
Timestamp: 2025-10-15T14:43:40.410Z
Learning: In the claude-task-master codebase, `getCredentials()` in `credential-store.ts` defaults to `allowExpired: true` to enable Supabase refresh flows. The Supabase client automatically handles token refresh when credentials are expired but have a valid refresh token. The `SupabaseSessionStorage` updates the credentials after Supabase performs the refresh. This is an intentional design pattern where the credential store is passive storage and Supabase manages the active token lifecycle.
Applied to files:
packages/tm-core/src/modules/auth/services/supabase-session-storage.ts
📚 Learning: 2025-09-26T19:03:33.225Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1252
File: package.json:130-132
Timestamp: 2025-09-26T19:03:33.225Z
Learning: In the eyaltoledano/claude-task-master repository, packages are bundled using tsdown during the build process, which means dependencies imported by the source code (including tm internal packages like tm/ai-sdk-provider-grok-cli) are included in the final bundle and don't need to be available as separate runtime dependencies, so they should remain as devDependencies rather than being moved to dependencies.
Applied to files:
package.json
🧬 Code graph analysis (5)
packages/tm-core/src/modules/workflow/workflow-domain.ts (1)
packages/tm-core/src/modules/workflow/services/workflow.service.ts (1)
WorkflowService(77-494)
packages/tm-core/src/modules/tasks/tasks-domain.ts (4)
packages/tm-core/src/modules/tasks/services/task-service.ts (1)
TaskService(51-565)packages/tm-core/src/modules/tasks/services/task-execution-service.ts (1)
TaskExecutionService(42-308)packages/tm-core/src/modules/tasks/services/task-loader.service.ts (2)
TaskLoaderService(59-401)TaskValidationResult(27-40)packages/tm-core/src/modules/tasks/services/preflight-checker.service.ts (1)
PreflightChecker(61-395)
packages/tm-core/src/tm-core.ts (5)
packages/tm-core/src/modules/tasks/tasks-domain.ts (1)
TasksDomain(32-184)packages/tm-core/src/modules/auth/auth-domain.ts (1)
AuthDomain(17-127)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-238)packages/tm-core/src/modules/config/config-domain.ts (1)
ConfigDomain(12-113)
packages/tm-core/src/modules/git/git-domain.ts (1)
packages/tm-core/src/modules/git/services/commit-message-generator.ts (1)
CommitMessageGenerator(53-205)
packages/tm-core/src/modules/config/config-domain.ts (1)
packages/tm-core/src/common/interfaces/configuration.interface.ts (1)
RuntimeStorageConfig(104-119)
packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts
Outdated
Show resolved
Hide resolved
packages/tm-core/src/modules/config/managers/config-manager.spec.ts
Outdated
Show resolved
Hide resolved
packages/tm-core/src/modules/git/services/branch-name-generator.spec.ts
Outdated
Show resolved
Hide resolved
| } from '../../../common/types/index.js'; | ||
| import type { IStorage } from '../../../common/interfaces/storage.interface.js'; | ||
| import { ConfigManager } from '../../config/managers/config-manager.js'; | ||
| import { StorageFactory } from '../../storage/services/storage-factory.js'; | ||
| import { TaskEntity } from '../entities/task.entity.js'; | ||
| import { ERROR_CODES, TaskMasterError } from '../errors/task-master-error.js'; | ||
| import { getLogger } from '../logger/factory.js'; | ||
| import { ERROR_CODES, TaskMasterError } from '../../../common/errors/task-master-error.js'; | ||
| import { getLogger } from '../../../common/logger/factory.js'; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the logger import path consistency.
Most of the import path updates follow the correct pattern for the centralized common module structure. However, line 17 imports logger from '../../../common/logger/factory.js' while other files in this refactor consistently use '../../../common/logger/index.js'.
Please verify whether factory.js is the correct import or if this should be index.js for consistency.
🏁 Script executed:
#!/bin/bash
# Check which logger export is used across the codebase
echo "=== Logger imports using factory.js ==="
rg -n "from ['\"].*logger/factory\.js['\"]" -g '*.ts' -g '*.js'
echo -e "\n=== Logger imports using index.js ==="
rg -n "from ['\"].*logger/index\.js['\"]" -g '*.ts' -g '*.js'
echo -e "\n=== Check logger module exports ==="
fd -t f "factory.js|index.js" packages/tm-core/src/common/logger/ --exec cat {}Length of output: 2646
packages/tm-core/src/modules/workflow/managers/workflow-state-manager.spec.ts
Outdated
Show resolved
Hide resolved
packages/tm-core/src/tm-core.ts
Outdated
| if (!options.projectPath) { | ||
| throw new TaskMasterError( | ||
| 'Project path is required', | ||
| ERROR_CODES.MISSING_CONFIGURATION | ||
| ); | ||
| } | ||
|
|
||
| this._projectPath = options.projectPath; |
There was a problem hiding this comment.
Validate/normalize projectPath.
Comment says “Absolute path”, but only a truthy check is enforced. Validate with path.isAbsolute() and normalize with path.resolve(). Emit a precise error (e.g., INVALID_ARGUMENT) including the received value.
Proposed snippet:
+ import path from 'node:path';
...
- if (!options.projectPath) {
+ if (!options.projectPath) {
throw new TaskMasterError(
'Project path is required',
ERROR_CODES.MISSING_CONFIGURATION
);
}
+ if (!path.isAbsolute(options.projectPath)) {
+ throw new TaskMasterError(
+ 'Project path must be absolute',
+ ERROR_CODES.MISSING_CONFIGURATION
+ );
+ }
+ options.projectPath = path.resolve(options.projectPath);📝 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.
| if (!options.projectPath) { | |
| throw new TaskMasterError( | |
| 'Project path is required', | |
| ERROR_CODES.MISSING_CONFIGURATION | |
| ); | |
| } | |
| this._projectPath = options.projectPath; | |
| if (!options.projectPath) { | |
| throw new TaskMasterError( | |
| 'Project path is required', | |
| ERROR_CODES.MISSING_CONFIGURATION | |
| ); | |
| } | |
| if (!path.isAbsolute(options.projectPath)) { | |
| throw new TaskMasterError( | |
| 'Project path must be absolute', | |
| ERROR_CODES.MISSING_CONFIGURATION | |
| ); | |
| } | |
| options.projectPath = path.resolve(options.projectPath); | |
| this._projectPath = options.projectPath; |
🤖 Prompt for AI Agents
In packages/tm-core/src/tm-core.ts around lines 73 to 80, the code only checks
for a truthy projectPath but should enforce an absolute path and normalize it;
update the validation to use path.isAbsolute(options.projectPath) and if it
fails throw a TaskMasterError with ERROR_CODES.INVALID_ARGUMENT and include the
received value in the message, then set this._projectPath to the normalized
absolute path using path.resolve(options.projectPath); also import Node's path
module if not already imported.
- update-task coming up next
Crunchyman-ralph
force-pushed
the
ralph/chore/refactor.tm.core
branch
from
a02aa7a to
4131976
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
packages/tm-core/src/modules/storage/adapters/api-storage.ts (2)
69-73: Type safety bypassed withunknowncast for incomplete implementation.The cast
as unknown as TaskRepositorycircumvents TypeScript's type checking for an incomplete implementation. This could lead to runtime exceptions if unimplemented methods are called.Consider one of these approaches:
- Complete the
SupabaseTaskRepositoryimplementation to fully satisfyTaskRepository- Create a typed subset interface that
SupabaseTaskRepositorycan safely implement- Add runtime guards that throw descriptive errors for unimplemented methods
Do you want me to open a tracking issue for completing the
SupabaseTaskRepositoryimplementation?
687-708: Backup method returns identifier without creating actual backup.The method fetches tasks and tags but doesn't store them, returning only a formatted string. Callers might assume a backup was created.
Consider adding a console warning or updating the return type to signal incomplete implementation:
async backup(): Promise<string> { await this.ensureInitialized(); try { + console.warn('ApiStorage.backup() is not fully implemented - no data is being stored'); + // Export all data await this.repository.getTasks(this.projectId); await this.repository.getTags(this.projectId);packages/tm-core/src/task-master-core.ts (1)
384-390: Add storage cleanup to theclose()method - missingtaskService.close()call.The
close()method only stops the ExecutorService but omits cleanup of TaskService's storage connection. TaskService initializes storage (file or API) in itsinitialize()method, and both storage adapters implementclose()methods:
- FileStorage.close() calls fileOps.cleanup() to release pending file locks
- ApiStorage.close() resets state and clears cache
Without calling
await this.taskService.close()(or directlyawait this.taskService.storage.close()), storage resources are never released. The comment "TaskService handles storage cleanup internally" is inaccurate—TaskService has no close method.async close(): Promise<void> { // Stop any running executors if (this.executorService) { await this.executorService.stopCurrentTask(); } // Close storage connections await this.taskService.close(); }Alternatively, add a close() method to TaskService that delegates to storage:
async close(): Promise<void> { if (this.storage) { await this.storage.close(); } }packages/tm-core/src/modules/integration/services/export.service.ts (2)
364-371: Add timeout, basic sanitization, and ID encoding for the export API call.Network call lacks timeout/backoff and doesn’t sanitize base URL or encode briefId. This can hang indefinitely, and malformed base domains (trailing slashes) can 404. Encode IDs and apply a short timeout; consider retries separately.
- // Use the new bulk import API endpoint - const apiUrl = `${apiEndpoint}/ai/api/v1/briefs/${briefId}/tasks`; + // Use the new bulk import API endpoint (sanitize base, encode IDs) + const base = apiEndpoint.replace(/\/+$/, ''); + const apiUrl = `${base}/ai/api/v1/briefs/${encodeURIComponent(briefId)}/tasks`; // Transform tasks to flat structure for API const flatTasks = this.transformTasksForBulkImport(tasks); // Prepare request body const requestBody = { source: 'task-master-cli', options: { dryRun: false, stopOnError: false }, accountId: orgId, tasks: flatTasks }; // Get auth token const credentials = await this.authManager.getCredentials(); if (!credentials || !credentials.token) { throw new Error('Not authenticated'); } // Make API request - const response = await fetch(apiUrl, { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 30_000); + const response = await fetch(apiUrl, { method: 'POST', headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${credentials.token}` }, - body: JSON.stringify(requestBody) + body: JSON.stringify(requestBody), + signal: controller.signal }); + clearTimeout(timeout);Optionally add retry/backoff (e.g., 3 attempts with jitter) around fetch for 5xx/429.
Also applies to: 391-405
460-477: Tighten brief ID extraction; current len≥8 fallback yields false positives (e.g., “overview”, “settings”).Only accept IDs that match known patterns; remove the permissive length checks.
- if (url) { - const qId = url.searchParams.get('id') || url.searchParams.get('briefId'); - const candidate = (qId || fromParts(url.pathname)) ?? null; - if (candidate) { - if (this.isLikelyId(candidate) || candidate.length >= 8) { - return candidate; - } - } - } + if (url) { + const qId = url.searchParams.get('id') || url.searchParams.get('briefId'); + const candidate = (qId || fromParts(url.pathname)) ?? null; + if (candidate && this.isLikelyId(candidate)) { + return candidate; + } + } // Check if it looks like a path without scheme if (raw.includes('/')) { const candidate = fromParts(raw); - if (candidate && (this.isLikelyId(candidate) || candidate.length >= 8)) { + if (candidate && this.isLikelyId(candidate)) { return candidate; } } // Return as-is if it looks like an ID - if (this.isLikelyId(raw) || raw.length >= 8) { + if (this.isLikelyId(raw)) { return raw; }Optionally widen slug acceptance but keep strict charset by relaxing the slug regex instead of raw length:
- const slugRegex = /^[A-Za-z0-9_-]{16,}$/; + const slugRegex = /^[A-Za-z0-9_-]{8,}$/;Also applies to: 481-486, 492-499
packages/tm-core/src/modules/git/adapters/git-adapter.ts (1)
258-261: Fix staged-changes detection;StatusResult.stagedisn’t stable across simple-git versions.Compute staged files from
status.files[].indexto avoid runtime undefined errors and ensure correctness.async hasStagedChanges(): Promise<boolean> { const status = await this.git.status(); - return status.staged.length > 0; + return status.files.some((f) => (f.index ?? '').trim() !== ''); } async getStatusSummary(): Promise<{ isClean: boolean; staged: number; modified: number; deleted: number; untracked: number; totalChanges: number; }> { const status = await this.git.status(); - const staged = status.staged.length; + const staged = status.files.filter((f) => (f.index ?? '').trim() !== '').length; const modified = status.modified.length; const deleted = status.deleted.length; const untracked = status.not_added.length; const totalChanges = staged + modified + deleted + untracked;Based on learnings.
Also applies to: 288-311
apps/cli/src/commands/set-status.command.ts (1)
266-276: Add 'completed' to VALID_TASK_STATUSES arrayThe TaskStatus type from @tm/core includes 'completed', and statusColors has a mapping for it, but VALID_TASK_STATUSES (line 16-24) is missing it. This creates a validation gap where users cannot set tasks to 'completed' status via CLI despite the type supporting it.
Add
'completed'to the VALID_TASK_STATUSES array at line 23 (before the closing bracket).apps/cli/src/commands/list.command.ts (1)
426-439: Fix comparator: Number(id) returns NaN for non‑numeric IDs → unstable sort.If IDs include strings (e.g., "HAM-123"), the comparator can return NaN, yielding nondeterministic ordering. Use numeric when both numeric; otherwise localeCompare with numeric option.
Apply:
- // ID (lower first) - return Number(a.id) - Number(b.id); + // ID (lower first): numeric if both numeric, else natural string compare + const aId = String(a.id); + const bId = String(b.id); + const aNum = Number(aId); + const bNum = Number(bId); + if (!Number.isNaN(aNum) && !Number.isNaN(bNum)) { + return aNum - bNum; + } + return aId.localeCompare(bId, undefined, { numeric: true });
♻️ Duplicate comments (5)
packages/tm-core/src/tm-core.ts (3)
33-44: Docs: update example to match facade API (authenticateWithOAuth, getModelConfig).- // Access any domain - await tmcore.auth.login({ ... }); + // Access any domain + await tmcore.auth.authenticateWithOAuth({ /* ... */ }); ... - const mainModel = tmcore.config.get('models.main'); + const mainModel = tmcore.config.getModelConfig();
78-87: Validate and normalizeprojectPath(absolute path required).+ import path from 'node:path'; ... private constructor(options: TmCoreOptions) { if (!options.projectPath) { throw new TaskMasterError( 'Project path is required', ERROR_CODES.MISSING_CONFIGURATION ); } + if (!path.isAbsolute(options.projectPath)) { + throw new TaskMasterError( + 'Project path must be absolute', + ERROR_CODES.MISSING_CONFIGURATION + ); + } - this._projectPath = options.projectPath; + this._projectPath = path.resolve(options.projectPath);
47-58: Don’t mutate public readonly fields via(this as any); use private fields + getters.Replace readonly public fields with private definite-assignment fields and getters; assign directly in initialize() without casts. This preserves type guarantees and avoids hidden init bugs.
Also applies to: 88-97, 103-123
packages/tm-core/src/index.ts (2)
22-27: Docs: align example with actual façade methods.- await tmcore.auth.login({ ... }); + await tmcore.auth.authenticateWithOAuth({ /* ... */ }); ... - const config = tmcore.config.get('models.main'); + const mainModel = tmcore.config.getModelConfig();
29-29: Ensure TypeScript preserves .js specifiers (verbatimModuleSyntax).Since exports/imports use .js specifiers, tsconfig must set
verbatimModuleSyntax: truewith NodeNext resolution. This was flagged already; just confirming for this entrypoint as well.#!/bin/bash # Verify tm-core tsconfig has the required flags jq '.compilerOptions | {module, moduleResolution, verbatimModuleSyntax}' packages/tm-core/tsconfig.json
🧹 Nitpick comments (27)
CLAUDE.md (1)
36-77: Architecture Guidelines strongly align with PR refactoring objectives.The new Business Logic Separation guidelines provide clear, prescriptive boundaries that directly support the domain-driven architecture refactor described in the PR. The separation of concerns (all business logic in
@tm/core, thin presentation layers in@tm/cliand@tm/mcp) mirrors the unified facade pattern and domain module organization introduced in this PR. The concrete examples (lines 73–76) showing howtmCore.tasks.get(taskId)intelligently handles multiple ID formats make the pattern actionable.Optional suggestion: Enumerate the domain modules for clarity.
Consider briefly listing the domain modules (auth, config, git, tasks, workflow) established by this refactor to give developers immediate context about what domain facades they should call. Example: "Provides clean facade APIs through domain objects (tasks, auth, workflow, git, config, integration)". This would help junior developers quickly discover the available boundaries.
packages/tm-core/src/task-master-core.ts (1)
95-98: Consider using optional properties instead ofnull as any.The
null as anytype assertions bypass TypeScript's type safety. While this pattern works with the factory method ensuring initialization, consider making these properties optional (?) instead, which would maintain type safety while still supporting the deferred initialization pattern.Apply this diff to improve type safety:
private constructor() { // Services will be initialized in the initialize() method - this.configManager = null as any; - this.taskService = null as any; - this.taskExecutionService = null as any; - this.exportService = null as any; + this.configManager = undefined!; + this.taskService = undefined!; + this.taskExecutionService = undefined!; + this.exportService = undefined!; }Or declare the fields as:
private configManager!: ConfigManager; private taskService!: TaskService; // etc.packages/tm-core/src/modules/config/config-domain.ts (2)
37-39: Add explicit return type annotation for consistency.The
getModelConfig()method lacks an explicit return type annotation. While TypeScript can infer it from the delegated call, adding an explicit return type improves API documentation and catches potential breaking changes.
113-115: Add explicit return type annotation for consistency.The
getConfigSources()method returnsany, which bypasses type safety. Consider adding an explicit return type (or at minimum documenting the shape) to improve type safety and API clarity.packages/tm-core/src/modules/integration/services/export.service.ts (2)
250-257: Replaceany[]with a concrete API DTO type for bulk import.Typed payload improves safety and evolvability.
+type ApiStatus = 'todo' | 'in_progress' | 'done'; +type ApiPriority = 'low' | 'medium' | 'high'; +interface BulkTaskDto { + externalId: string; + parentExternalId?: string; + title: string; + description: string; + status: ApiStatus; + priority: ApiPriority; + dependencies: string[]; + details?: string; + testStrategy?: string; + complexity?: unknown; + metadata?: Record<string, unknown>; +} - private transformTasksForBulkImport(tasks: Task[]): any[] { - const flatTasks: any[] = []; + private transformTasksForBulkImport(tasks: Task[]): BulkTaskDto[] { + const flatTasks: BulkTaskDto[] = [];Also applies to: 308-309
415-423: Avoidconsole.*in core service; inject and use a logger.Console noise from services makes headless/SDK use noisy and inconsistent. Prefer a logger abstraction (already present under common/logger).
apps/cli/src/commands/next.command.ts (1)
130-136: Type-narrowstorageTypeafter guard for stronger typing in the result.Minor TS polish: narrow after the guard to satisfy
Exclude<StorageType, 'auto'>without casts.- const storageType = - this.tmCore.config.getStorageConfig().type; - if (storageType === 'auto') { + const { type } = this.tmCore.config.getStorageConfig(); + if (type === 'auto') { throw new Error('Storage type must be resolved before use'); } - const activeTag = options.tag || this.tmCore.config.getActiveTag(); + const storageType = type; // now inferred as Exclude<..., 'auto'> + const activeTag = options.tag || this.tmCore.config.getActiveTag();Also applies to: 137-143
packages/tm-core/src/modules/git/adapters/git-adapter.ts (2)
121-125: Specify radix inparseIntto avoid edge cases.Small but safe.
- ? parseInt(versionResult.patch) + ? parseInt(versionResult.patch, 10) : versionResult.patch || 0,
507-512: Prefer simple-git’sdeleteLocalBranchhelper over rawbranch -d/-D.Clearer intent and safer across versions.
- const deleteOptions = options.force - ? ['-D', branchName] - : ['-d', branchName]; - await this.git.branch(deleteOptions); + await this.git.deleteLocalBranch(branchName, !!options.force);packages/tm-core/src/modules/integration/integration-domain.ts (1)
23-31: Expose exportFromBriefInput/validateContext on the facade to remove CLI duplicationCLI re-implements brief parsing and context checks. Make these pass-throughs to centralize logic in ExportService.
Apply this diff:
export class IntegrationDomain { private exportService: ExportService; constructor(configManager: ConfigManager) { // Get singleton AuthManager instance const authManager = AuthManager.getInstance(); this.exportService = new ExportService(configManager, authManager); } // ========== Export Operations ========== /** * Export tasks to external systems (e.g., Hamster briefs) */ async exportTasks(options: ExportTasksOptions): Promise<ExportResult> { return this.exportService.exportTasks(options); } + + /** + * Export by accepting a brief ID or URL (resolves org/brief internally) + */ + async exportFromBriefInput(briefInput: string): Promise<ExportResult> { + return this.exportService.exportFromBriefInput(briefInput); + } + + /** + * Validate whether org/brief context is present + */ + async validateContext() { + return this.exportService.validateContext(); + } }This enables CLI to call tmCore.integration without duplicating parsing/validation. Based on learnings.
apps/cli/src/utils/display-helpers.ts (1)
30-39: Coerce 'auto' storage type for display to avoid ambiguous headerWhen tm-core reports 'auto', pass a concrete type to the header for consistent UX.
Apply this diff:
- // Display header with computed display info - displayHeader({ - tag: options.tag || 'master', - filePath: displayInfo.filePath, - storageType: displayInfo.storageType, - briefInfo: displayInfo.briefInfo - }); + // Display header with computed display info + const { filePath, storageType, briefInfo } = displayInfo; + const safeType = + (storageType === 'auto' ? options.storageType : storageType) as Exclude<StorageType, 'auto'>; + displayHeader({ + tag: options.tag || 'master', + filePath, + storageType: safeType, + briefInfo + });apps/cli/src/commands/export.command.ts (5)
36-36: Rename to tmCore for consistencyAligns with other commands and the new facade terminology.
Apply this diff:
- private taskMasterCore?: TmCore; + private tmCore?: TmCore;Update references accordingly (initializeServices and export call sites).
76-81: Use the same project path option as other commandsOther commands accept -p/--project. Consider adding it here and use it instead of process.cwd() for consistency.
160-167: Delegate URL/ID handling to IntegrationDomain to avoid duplicationInstead of resolving IDs here and then calling exportTasks, call a facade method that accepts the raw input. Pair this with exposing exportFromBriefInput on the facade.
Example change after adding IntegrationDomain.exportFromBriefInput:
- // Use integration domain facade - const exportResult = await this.taskMasterCore!.integration.exportTasks({ - orgId, - briefId, - tag: options?.tag, - status: options?.status, - excludeSubtasks: options?.excludeSubtasks || false - }); + const exportResult = briefOrUrl && !options?.brief + ? await this.tmCore!.integration.exportFromBriefInput(briefOrUrl) + : await this.tmCore!.integration.exportTasks({ + orgId, + briefId, + tag: options?.tag, + status: options?.status, + excludeSubtasks: options?.excludeSubtasks || false + });Then you can remove resolveBriefInput/extractBriefId/isLikelyId here. Based on learnings.
210-299: Remove duplicated brief parsing and validation logicextractBriefId/isLikelyId here duplicate logic maintained in core. Prefer a single source of truth (IntegrationDomain/ExportService) to prevent divergence on URL patterns and ID shapes.
53-57: Validate --status against TaskStatusCurrently any string passes through. Validate against the TaskStatus union (and map friendly aliases if needed) to reduce surprises during export.
Would you like a small helper to import TaskStatus and validate/normalize it here?
apps/cli/src/commands/set-status.command.ts (2)
182-186: Remove no-op cleanup blockThis finally block does nothing. Either drop it or clear the instance.
Apply this diff:
- } finally { - // Clean up resources - if (this.tmCore) { - } - } + }
110-114: Initialize core only once per run (minor)If this command is chained programmatically, guard against re-creating tmCore repeatedly.
- this.tmCore = await createTmCore({ + if (!this.tmCore) this.tmCore = await createTmCore({ projectPath: options.project || process.cwd() });apps/cli/src/commands/start.command.ts (1)
300-303: Avoid accumulating process signal listenersUse once() so repeated executions don’t stack handlers.
Apply this diff:
- process.on('SIGINT', cleanup); - process.on('SIGTERM', cleanup); - process.on('exit', cleanup); + process.once('SIGINT', cleanup); + process.once('SIGTERM', cleanup); + process.once('exit', cleanup);If this can run multiple times in one process, also consider removing listeners in the 'close'/'error' handlers.
apps/cli/src/commands/list.command.ts (3)
92-95: Prefer return over process.exit for CLI validation failures.Improves testability and programmatic reuse (command runners can decide exit codes).
- if (!this.validateOptions(options)) { - process.exit(1); - } + if (!this.validateOptions(options)) { + return; + }
257-261: Use active tag from core config rather than defaulting to 'master'.Aligns with show.command.ts and avoids stale default.
- displayCommandHeader(this.tmCore, { - tag: tag || 'master', - storageType - }); + const activeTag = this.tmCore?.config.getActiveTag() || 'master'; + displayCommandHeader(this.tmCore, { + tag: tag ?? activeTag, + storageType + });
49-55: Import TaskListResult from @tm/core to prevent type drift.
TaskListResultis properly exported from@tm/coreand currently has identical shape to the localListTasksResult(both definestorageType: Exclude<StorageType, 'auto'>). Importing from the core package eliminates the need for the unsafe type cast on line 182 and ensures the CLI stays in sync with any future type changes in core.apps/cli/src/commands/show.command.ts (2)
183-191: Fetch multiple tasks in parallel to improve latency.Large ID lists currently serialize network/disk calls.
- 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((r, i) => (r.task ? tasks.push(r.task) : notFound.push(taskIds[i])));
83-96: Avoid hard exits in command flow.Return early instead of process.exit for better composability and tests.
- if (!this.validateOptions(options)) { - process.exit(1); - } + if (!this.validateOptions(options)) { + return; + } ... - if (!idArg) { - console.error(chalk.red('Error: Please provide a task ID')); - process.exit(1); - } + if (!idArg) { + console.error(chalk.red('Error: Please provide a task ID')); + return; + }packages/tm-core/src/modules/auth/auth-domain.ts (1)
163-182: Keep storageType accurate for API even when context is missing.Currently falls back to
file, which misleads UIs. ReturnstorageType: 'api'with optional briefInfo; only usefilewhen storage is actually file.- if (storageType === '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') - }; + if (storageType === 'api') { + const context = this.getContext(); + return { + storageType: 'api', + briefInfo: context?.briefId && context?.briefName + ? { + briefId: context.briefId, + briefName: context.briefName, + orgSlug: context.orgSlug, + webAppUrl: this.getWebAppUrl() + } + : undefined + }; + } + return { + storageType: 'file', + filePath: path.join(this.configManager.getProjectRoot(), '.taskmaster', 'tasks', 'tasks.json') + };packages/tm-core/src/modules/git/git-domain.ts (1)
164-173: Optionally expose clean/default-branch guards for parity with adapter.Keeps consumers on the façade only.
// ========== Commit Operations ========== + /** + * Ensure working tree is clean before destructive ops. + */ + async ensureCleanWorkingTree(): Promise<void> { + return this.gitAdapter.ensureCleanWorkingTree(); + } + + /** + * Ensure we are not on a default branch (main/master/develop). + */ + async ensureNotOnDefaultBranch(): Promise<void> { + return this.gitAdapter.ensureNotOnDefaultBranch(); + }packages/tm-core/src/index.ts (1)
107-127: Remove ExportService from advanced exports—unused outside tm-core package.ExportService has no usage across apps/cli, apps/mcp, or other packages. All other advanced exports (AuthManager, WorkflowOrchestrator, WorkflowStateManager, WorkflowService, SubtaskInfo, GitAdapter, CommitMessageGenerator, PreflightChecker, TaskLoaderService) are actively consumed by CLI and MCP as intended. Remove only the unused export.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
CLAUDE.md(1 hunks)apps/cli/src/commands/auth.command.ts(1 hunks)apps/cli/src/commands/autopilot.command.ts(4 hunks)apps/cli/src/commands/autopilot/start.command.ts(2 hunks)apps/cli/src/commands/context.command.ts(1 hunks)apps/cli/src/commands/export.command.ts(4 hunks)apps/cli/src/commands/list.command.ts(5 hunks)apps/cli/src/commands/next.command.ts(4 hunks)apps/cli/src/commands/set-status.command.ts(5 hunks)apps/cli/src/commands/show.command.ts(7 hunks)apps/cli/src/commands/start.command.ts(8 hunks)apps/cli/src/index.ts(1 hunks)apps/cli/src/ui/components/dashboard.component.ts(1 hunks)apps/cli/src/ui/components/next-task.component.ts(1 hunks)apps/cli/src/ui/components/task-detail.component.ts(1 hunks)apps/cli/src/utils/display-helpers.ts(1 hunks)apps/cli/src/utils/ui.ts(1 hunks)apps/mcp/src/tools/autopilot/start.tool.ts(2 hunks)package.json(1 hunks)packages/tm-core/src/index.ts(1 hunks)packages/tm-core/src/modules/ai/providers/base-provider.ts(2 hunks)packages/tm-core/src/modules/auth/auth-domain.ts(1 hunks)packages/tm-core/src/modules/auth/index.ts(1 hunks)packages/tm-core/src/modules/auth/services/organization.service.ts(1 hunks)packages/tm-core/src/modules/config/config-domain.ts(1 hunks)packages/tm-core/src/modules/git/adapters/git-adapter.ts(2 hunks)packages/tm-core/src/modules/git/git-domain.ts(1 hunks)packages/tm-core/src/modules/integration/integration-domain.ts(1 hunks)packages/tm-core/src/modules/integration/services/export.service.ts(1 hunks)packages/tm-core/src/modules/storage/adapters/api-storage.ts(1 hunks)packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts(1 hunks)packages/tm-core/src/modules/storage/services/storage-factory.ts(1 hunks)packages/tm-core/src/modules/tasks/tasks-domain.ts(1 hunks)packages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.ts(2 hunks)packages/tm-core/src/modules/workflow/services/workflow-activity-logger.ts(1 hunks)packages/tm-core/src/task-master-core.ts(2 hunks)packages/tm-core/src/tm-core.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/tm-core/src/modules/storage/services/storage-factory.ts
- packages/tm-core/src/modules/ai/providers/base-provider.ts
- packages/tm-core/src/modules/auth/services/organization.service.ts
- packages/tm-core/src/modules/auth/index.ts
- package.json
- packages/tm-core/src/modules/tasks/tasks-domain.ts
- packages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
Reference documentation at https://docs.task-master.dev rather than local file paths in content
Files:
CLAUDE.md
🧠 Learnings (18)
📚 Learning: 2025-07-18T17:16:32.622Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ui.mdc:0-0
Timestamp: 2025-07-18T17:16:32.622Z
Learning: Applies to scripts/modules/ui.js : Use chalk.blue for informational messages, chalk.green for success, chalk.yellow for warnings, chalk.red for errors, chalk.cyan for prompts/highlights, and chalk.magenta for subtask information
Applied to files:
apps/cli/src/ui/components/dashboard.component.tsapps/cli/src/ui/components/next-task.component.tsapps/cli/src/utils/ui.ts
📚 Learning: 2025-07-18T17:09:16.839Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/context_gathering.mdc:0-0
Timestamp: 2025-07-18T17:09:16.839Z
Learning: Display token breakdowns using a detailed, sectioned format, showing tokens per task, file, and prompt, and use formatting utilities like `boxen` and `chalk` for CLI output.
Applied to files:
apps/cli/src/ui/components/dashboard.component.tsapps/cli/src/ui/components/next-task.component.tsapps/cli/src/utils/ui.ts
📚 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/cli/src/index.tsapps/cli/src/commands/export.command.tsapps/cli/src/commands/list.command.tspackages/tm-core/src/tm-core.tsapps/cli/src/utils/ui.tspackages/tm-core/src/index.tspackages/tm-core/src/task-master-core.ts
📚 Learning: 2025-09-26T19:10:32.906Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1252
File: tsconfig.json:22-28
Timestamp: 2025-09-26T19:10:32.906Z
Learning: In the eyaltoledano/claude-task-master repository, all internal tm/ package path mappings in tsconfig.json consistently point to TypeScript source files (e.g., "./packages/*/src/index.ts") rather than built JavaScript. This is intentional architecture because tsdown bundles internal packages directly from source during build time, eliminating the need for separate compilation of internal packages.
Applied to files:
apps/cli/src/index.ts
📚 Learning: 2025-10-08T19:57:00.982Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1282
File: packages/tm-core/src/utils/index.ts:16-34
Timestamp: 2025-10-08T19:57:00.982Z
Learning: For the tm-core package in the eyaltoledano/claude-task-master repository, the team prefers a minimal, need-based export strategy in index files rather than exposing all internal utilities. Exports should only be added when functions are actually consumed by other packages in the monorepo.
Applied to files:
apps/cli/src/commands/export.command.tspackages/tm-core/src/index.ts
📚 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 : Generate task files from the current tag context, include tag information in generated files, and do not mix tasks from different tags in file generation.
Applied to files:
apps/cli/src/commands/autopilot/start.command.ts
📚 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 : Extract tasks from PRD documents using AI, create them in the current tag context (defaulting to 'master'), provide clear prompts to guide AI task generation, and validate/clean up AI-generated tasks.
Applied to files:
apps/cli/src/commands/autopilot/start.command.ts
📚 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 : Use AI to generate detailed subtasks within the current tag context, considering complexity analysis for subtask counts and ensuring proper IDs for newly created subtasks.
Applied to files:
apps/cli/src/commands/autopilot/start.command.ts
📚 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 : The default tag 'master' must be used for all existing and new tasks unless otherwise specified.
Applied to files:
apps/cli/src/commands/autopilot/start.command.ts
📚 Learning: 2025-07-18T17:18:17.759Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-07-18T17:18:17.759Z
Learning: Applies to scripts/modules/utils.js : Use tagged task system aware functions for task finding and manipulation, handle both task and subtask operations, and validate task IDs before operations.
Applied to files:
apps/cli/src/commands/autopilot/start.command.ts
📚 Learning: 2025-09-24T15:12:12.658Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/.windsurfrules:0-0
Timestamp: 2025-09-24T15:12:12.658Z
Learning: Use task-master next to select the next actionable task with all dependencies satisfied
Applied to files:
apps/cli/src/commands/next.command.tsapps/cli/src/commands/start.command.ts
📚 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:
apps/cli/src/commands/set-status.command.tsapps/mcp/src/tools/autopilot/start.tool.ts
📚 Learning: 2025-07-18T17:18:17.759Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-07-18T17:18:17.759Z
Learning: Applies to mcp-server/src/tools/utils.js : Use `normalizeProjectRoot(rawPath, log)`, `getRawProjectRootFromSession(session, log)`, and `withNormalizedProjectRoot(executeFn)` in `mcp-server/src/tools/utils.js` to ensure project root paths are normalized for MCP tools.
Applied to files:
packages/tm-core/src/tm-core.ts
📚 Learning: 2025-07-18T17:09:13.815Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/context_gathering.mdc:0-0
Timestamp: 2025-07-18T17:09:13.815Z
Learning: Display token breakdowns in CLI output using a detailed, sectioned format, including tasks, files, and prompt tokens, with clear formatting (e.g., using boxen and chalk).
Applied to files:
apps/cli/src/utils/ui.ts
📚 Learning: 2025-07-18T17:16:32.622Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ui.mdc:0-0
Timestamp: 2025-07-18T17:16:32.622Z
Learning: Applies to scripts/modules/ui.js : Use getStatusWithColor for status display, formatDependenciesWithStatus for dependency lists, and truncate for overflowing text
Applied to files:
apps/cli/src/utils/ui.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: Describes the high-level architecture of the Task Master CLI application, including the new tagged task lists system (architecture.mdc).
Applied to files:
CLAUDE.md
📚 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 : 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.
Applied to files:
apps/mcp/src/tools/autopilot/start.tool.ts
📚 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/tools/*.js : 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.
Applied to files:
apps/mcp/src/tools/autopilot/start.tool.ts
🧬 Code graph analysis (16)
packages/tm-core/src/modules/config/config-domain.ts (1)
packages/tm-core/src/common/interfaces/configuration.interface.ts (1)
RuntimeStorageConfig(104-119)
apps/cli/src/commands/export.command.ts (1)
packages/tm-core/src/tm-core.ts (2)
TmCore(46-141)createTmCore(150-152)
apps/cli/src/commands/autopilot.command.ts (2)
packages/tm-core/src/index.ts (2)
TmCore(29-29)createTmCore(29-29)packages/tm-core/src/tm-core.ts (2)
TmCore(46-141)createTmCore(150-152)
apps/cli/src/commands/autopilot/start.command.ts (2)
packages/tm-core/src/index.ts (1)
createTmCore(29-29)packages/tm-core/src/tm-core.ts (1)
createTmCore(150-152)
apps/cli/src/commands/next.command.ts (2)
packages/tm-core/src/index.ts (2)
TmCore(29-29)createTmCore(29-29)packages/tm-core/src/tm-core.ts (2)
TmCore(46-141)createTmCore(150-152)
apps/cli/src/commands/list.command.ts (1)
packages/tm-core/src/tm-core.ts (2)
TmCore(46-141)createTmCore(150-152)
apps/cli/src/commands/set-status.command.ts (3)
packages/tm-core/src/index.ts (2)
TmCore(29-29)createTmCore(29-29)packages/tm-core/src/tm-core.ts (2)
TmCore(46-141)createTmCore(150-152)packages/tm-core/src/common/types/index.ts (1)
StorageType(11-11)
packages/tm-core/src/modules/integration/integration-domain.ts (1)
packages/tm-core/src/modules/integration/services/export.service.ts (3)
ExportService(89-500)ExportTasksOptions(42-53)ExportResult(58-74)
packages/tm-core/src/modules/auth/auth-domain.ts (2)
packages/tm-core/src/common/types/index.ts (1)
StorageType(11-11)packages/tm-core/src/modules/auth/services/organization.service.ts (3)
Organization(17-21)Brief(26-39)RemoteTask(44-60)
apps/cli/src/commands/start.command.ts (1)
packages/tm-core/src/tm-core.ts (2)
TmCore(46-141)createTmCore(150-152)
apps/cli/src/commands/show.command.ts (2)
packages/tm-core/src/index.ts (2)
TmCore(29-29)createTmCore(29-29)packages/tm-core/src/tm-core.ts (2)
TmCore(46-141)createTmCore(150-152)
packages/tm-core/src/modules/git/git-domain.ts (2)
packages/tm-core/src/index.ts (3)
GitAdapter(119-119)CommitMessageGenerator(120-120)CommitMessageOptions(91-91)packages/tm-core/src/modules/git/adapters/git-adapter.ts (1)
GitAdapter(15-780)
packages/tm-core/src/tm-core.ts (6)
packages/tm-core/src/modules/tasks/tasks-domain.ts (1)
TasksDomain(29-210)packages/tm-core/src/modules/auth/auth-domain.ts (1)
AuthDomain(38-215)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(14-31)
apps/cli/src/utils/display-helpers.ts (3)
packages/tm-core/src/tm-core.ts (1)
TmCore(46-141)packages/tm-core/src/common/types/index.ts (1)
StorageType(11-11)apps/cli/src/ui/components/header.component.ts (1)
displayHeader(32-81)
packages/tm-core/src/index.ts (1)
packages/tm-core/src/modules/tasks/tasks-domain.ts (1)
TasksDomain(29-210)
apps/mcp/src/tools/autopilot/start.tool.ts (2)
packages/tm-core/src/index.ts (1)
createTmCore(29-29)packages/tm-core/src/tm-core.ts (1)
createTmCore(150-152)
🔇 Additional comments (25)
CLAUDE.md (2)
78-82: Documentation URL reference complies with coding guidelines.Line 81 correctly references https://docs.task-master.dev instead of local file paths, adhering to the guideline that .md/.mdx files must reference external documentation. This supports maintainability by centralizing documentation authority.
83-86: Changeset Guidelines are practical and user-focused.The guidance to emphasize user-facing changes over code specifics is a sensible standard for changesets and will improve the quality of user-facing release notes.
packages/tm-core/src/modules/workflow/services/workflow-activity-logger.ts (1)
8-14: All import paths verified and resolve correctly.The refactored imports all point to existing files in their new locations following the domain-driven architecture reorganization. The
.jsextensions in the import statements are properly handled by TypeScript's module resolution system, and the type-only imports are correctly marked.packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts (1)
5-19: Import paths verified and correctly resolved.All import paths have been verified to resolve correctly:
../../../../common/types/index.ts✓../../../../common/interfaces/storage.interface.ts✓../../../reports/managers/complexity-report-manager.ts✓The relative path navigation is correct, and the changes align with the PR's domain-driven architecture refactoring. No issues found.
packages/tm-core/src/modules/storage/adapters/api-storage.ts (1)
6-25: All import paths verified—reorganization is correct.Import path verification confirmed:
- ✓ All 6 source files exist
- ✓ All 16 imported symbols are properly exported from their sources
- ✓ Import structure aligns with domain-driven architecture
No issues identified.
packages/tm-core/src/task-master-core.ts (5)
5-63: LGTM! Clean import restructuring for domain-driven architecture.The import paths have been properly updated to reflect the new module organization, and the type re-exports provide a clean public API surface. The consistent use of
.jsextensions ensures ESM compatibility.
146-152: Good backward compatibility approach.The deprecated method properly redirects to the new implementation, maintaining compatibility while guiding users toward the new API.
181-191: Excellent backward compatibility handling.Removing
storageTypefrom the return value while maintaining it internally is a clean way to preserve the existing API contract while the internal implementation evolves.
396-399: LGTM! Clean factory function.The factory function provides a clean public API entry point as intended by the refactor, properly delegating to the static create method.
130-131: Singleton pattern is consistent with domain-driven design in this codebase.The code already practices dependency injection.
AuthManager.getInstance()is a boundary accessor pattern used consistently across the codebase, and the authManager instance is explicitly passed as a dependency toExportService(which accepts it as a constructor parameter at line 93 of export.service.ts). This same pattern is followed inintegration-domain.tsandauth-domain.ts. The singleton serves as a controlled entry point for a cross-cutting concern while actual services remain dependency-injectable. No changes needed.apps/cli/src/ui/components/next-task.component.ts (1)
8-8: LGTM!Import path updated to align with the new centralized public API surface. The change is mechanical and maintains type safety.
apps/cli/src/ui/components/dashboard.component.ts (1)
8-8: LGTM!Import path updated to align with the new centralized public API surface. The change is mechanical and maintains type safety.
apps/cli/src/commands/auth.command.ts (1)
11-15: LGTM!Import path updated to align with the new centralized public API surface. The change consolidates auth-related exports under
@tm/corewhile maintaining all runtime functionality.apps/cli/src/utils/ui.ts (1)
9-9: LGTM!Import path updated to align with the new centralized public API surface. The change is mechanical and maintains type safety.
apps/cli/src/commands/context.command.ts (1)
11-11: LGTM!Import path updated to align with the new centralized public API surface. The change consolidates auth-related exports under
@tm/corewhile maintaining all runtime functionality.apps/cli/src/ui/components/task-detail.component.ts (1)
11-11: LGTM!Import path updated to align with the new centralized public API surface. The change is mechanical and maintains type safety.
apps/cli/src/index.ts (1)
44-44: Breaking change confirmed as intentional; verify version strategy and monorepo migration consistency.The export rename is a deliberate API evolution. The core library's public API (packages/tm-core/src/index.ts) exports only
createTmCore, TmCore, type TmCoreOptions, establishing TmCore as the official interface. apps/cli aligns with this by exporting TmCore, and all internal CLI commands are already migrated to use the new API.However, migration across the monorepo is incomplete:
apps/extensionstill imports the legacyTaskMasterCore. Ensure the version bump reflects this breaking change for external consumers and verify that other packages are updated consistently.apps/cli/src/commands/next.command.ts (1)
106-114: Core init flow LGTM.Lazy init with resolved project path matches the new facade.
apps/cli/src/commands/autopilot/start.command.ts (1)
70-80: CLI start flow aligns with new TmCore; looks good.Core init, task fetch, and branch creation via adapter read cleanly.
Also applies to: 95-103, 135-144
apps/mcp/src/tools/autopilot/start.tool.ts (1)
85-96: MCP tool migration to TmCore is correct; wrapper usage is consistent.Uses withNormalizedProjectRoot, new API surfaces, and strict main-task guard. Good.
Also applies to: 122-131, 142-156
packages/tm-core/src/modules/integration/services/export.service.ts (1)
392-400: The review comment is incorrect—the project explicitly requires Node ≥18.0.0 wherefetchis natively available.The root
package.jsonspecifies"node": ">=18.0.0", and Node 18+ includes globalfetchby default. The codebase contains no fetch polyfills, confirming the team relies on native availability. The code is correct as-is.Likely an incorrect or invalid review comment.
packages/tm-core/src/modules/integration/integration-domain.ts (1)
17-21: AuthManager singleton capture can go stale across login/logoutIf AuthManager state (token/context) can change after this facade is constructed, capturing the singleton once might drift. Consider resolving the instance per call or subscribing to auth events to refresh internal services.
apps/cli/src/commands/autopilot.command.ts (1)
162-168: TmCore initialization pattern looks goodIdempotent factory usage with createTmCore and guarded assignment is clean and consistent with the new facade.
apps/cli/src/commands/start.command.ts (1)
148-152: Storage type propagation is correctUsing tmCore.config.getStorageConfig().type keeps the CLI aligned with core config.
packages/tm-core/src/modules/git/git-domain.ts (1)
6-21: GitDomain façade looks consistent and idiomatic.
Crunchyman-ralph
force-pushed
the
ralph/chore/refactor.tm.core
branch
from
4131976 to
ba36440
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
packages/tm-core/package.json (1)
8-20: Refactor incomplete: missing barrel files break all subpath exports.The package.json exports point to non-existent barrel files at src/ root level. While implementations have been moved to src/modules/ and src/common/ as part of the domain-driven refactor, the bridge files required to support the export structure were not created. All 11 exports will fail at runtime:
- Missing
src/auth/index.ts,src/storage/index.ts,src/config/index.ts,src/providers/index.ts,src/services/index.ts(should delegate tosrc/modules/{module}/)- Missing
src/errors/index.ts,src/logger/index.ts,src/types/index.ts,src/interfaces/index.ts,src/utils/index.ts(should delegate tosrc/common/{module}/)The subpath-exports.test.ts test file also expects these barrel files to exist for its relative imports (
./auth,./storage, etc.). Create these barrel files to re-export from their new locations in src/modules/ and src/common/.apps/extension/src/services/terminal-manager.ts (2)
149-149: Remove or replace theawait this.tmCore.close()call — TmCore class has noclose()method.The refactored
TmCoreclass (packages/tm-core/src/tm-core.ts) does not expose aclose()method. The call at line 149 of terminal-manager.ts will throw a runtime error. Either add aclose()method to TmCore that performs necessary cleanup, or remove this call if no cleanup is required.
52-56: Update API calls to use domain facades on refactored TmCore.The code makes two incorrect calls on
TmCore. The refactoredTmCoreclass (line 46, tm-core.ts) exposes only domain facades (tasks, auth, workflow, git, config, integration) and has no directstartTaskorclosemethods.
- Line 52: Change
this.tmCore!.startTask(taskId, options)tothis.tmCore!.tasks.startTask(taskId, options)- Line 149: Remove
await this.tmCore.close()— no close method exists on TmCorepackages/build-config/CHANGELOG.md (1)
3-10: Remove placeholder "null" version headers from changelog.These "## null" headers appear to be auto-generated placeholders that should be cleaned up before merging. Changelog entries should either have proper version numbers or be removed entirely if there are no changes to document.
Apply this diff to remove the placeholder headers:
# @tm/build-config -## null - -## null - -## null - -## null - ## 1.0.1packages/tm-core/src/modules/config/managers/config-manager.spec.ts (1)
16-20: Fix inconsistent mock paths - must match actual import paths.The mock paths use
'./services/*'but the actual imports (lines 9-13) use'../services/*'. Vitest requires mock paths to exactly match the import paths being mocked.Apply this fix:
// Mock all services -vi.mock('./services/config-loader.service.js'); -vi.mock('./services/config-merger.service.js'); -vi.mock('./services/runtime-state-manager.service.js'); -vi.mock('./services/config-persistence.service.js'); -vi.mock('./services/environment-config-provider.service.js'); +vi.mock('../services/config-loader.service.js'); +vi.mock('../services/config-merger.service.js'); +vi.mock('../services/runtime-state-manager.service.js'); +vi.mock('../services/config-persistence.service.js'); +vi.mock('../services/environment-config-provider.service.js');packages/tm-core/src/modules/storage/adapters/api-storage.ts (1)
69-74: Unsafe cast of SupabaseTaskRepository → TaskRepository will cause runtime failures when missing methods are called.Verification confirms SupabaseTaskRepository implements only 3 of 13 required methods (getTasks, getTask, updateTask). ApiStorage actively calls missing methods including createTask, deleteTask, getTag, createTag, updateTag, and bulkCreateTasks—each will fail at runtime.
Apply this fail-fast capability check:
constructor(config: ApiStorageConfig) { this.validateConfig(config); // Use provided repository or create Supabase repository if (config.repository) { this.repository = config.repository; } else if (config.supabaseClient) { - // TODO: SupabaseTaskRepository doesn't implement all TaskRepository methods yet - // Cast for now until full implementation is complete - this.repository = new SupabaseTaskRepository( - config.supabaseClient - ) as unknown as TaskRepository; + const repo = new SupabaseTaskRepository(config.supabaseClient) as any; + this.assertRepositoryCapabilities(repo); + this.repository = repo as TaskRepository; } else { throw new TaskMasterError( 'Either repository or supabaseClient must be provided', ERROR_CODES.MISSING_CONFIGURATION ); }Add helper at the class:
private assertRepositoryCapabilities(repo: any): void { const required = [ 'getTasks','getTask','createTask','updateTask','deleteTask','bulkCreateTasks', 'getTags','getTag','createTag','updateTag','deleteTag','bulkUpdateTasks','bulkDeleteTasks' ]; const missing = required.filter((m) => typeof repo[m] !== 'function'); if (missing.length) { throw new TaskMasterError( `SupabaseTaskRepository missing methods: ${missing.join(', ')}`, ERROR_CODES.NOT_IMPLEMENTED, { component: 'ApiStorage' } ); } }
♻️ Duplicate comments (4)
packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts (1)
76-76: Import path reverted to unnecessarily complex traversal.The import on line 76 uses
'../managers/auth-manager.js', which goes up one directory level then back intomanagers/. Since both the test file and the source file are in the samemanagers/directory, this should use a simple relative import'./auth-manager.js'for clarity and consistency with other same-directory imports in this file (see lines 21 and 46).The AI summary indicates this path was changed FROM the correct
./auth-manager.jsTO the current path, which appears to be a regression.Apply this diff:
-import { AuthManager } from '../managers/auth-manager.js'; +import { AuthManager } from './auth-manager.js';packages/tm-core/src/modules/tasks/services/task-service.ts (1)
17-17: Logger import path inconsistency (previously flagged).This import uses
factory.jswhile the codebase standard is to useindex.jsfor logger imports. This inconsistency has been previously identified and thoroughly analyzed in past review comments.packages/tm-core/src/modules/config/managers/config-manager.spec.ts (1)
7-7: Fix incorrect import path for ConfigManager.The test file is located at
packages/tm-core/src/modules/config/managers/config-manager.spec.ts, which is already inside themanagers/directory. The import path'./managers/config-manager.js'would incorrectly resolve tomanagers/managers/config-manager.js.Apply this fix:
-import { ConfigManager } from './managers/config-manager.js'; +import { ConfigManager } from './config-manager.js';apps/cli/src/commands/export.command.ts (1)
108-113: Note: async getContext usage differs from ApiStorage.Here
await authManager.getContext()is used; in ApiStorage it’s synchronous. Please resolve the inconsistency per earlier comment.
🧹 Nitpick comments (13)
packages/tm-core/src/modules/ui/index.ts (1)
1-8: Placeholder module looks good as scaffolding.This placeholder appropriately sets up the structure for the future UI domain migration. The TODO comment clearly indicates the migration source.
Would you like me to help create a tracking issue for the UI migration work from
scripts/modules/ui.js? I can generate an issue template with migration steps and acceptance criteria.packages/tm-core/src/modules/commands/index.ts (1)
1-8: Track the TODO with an issue.This placeholder file establishes the intended structure for the Commands domain migration. Consider creating a tracking issue for the TODO and referencing it in the comment to ensure the migration work isn't forgotten.
Based on learnings, the source file
scripts/modules/commands.jsshould export aregisterCommandsfunction and contain user-facing commands. Would you like me to:
- Open an issue to track this migration task?
- Generate a script to examine the current structure of
scripts/modules/commands.jsto help plan the migration?Apply this diff to reference a tracking issue once created:
-// TODO: Migrate commands.js from scripts/modules/ +// TODO: Migrate commands.js from scripts/modules/ (tracked in #ISSUE_NUMBER) // export * from './handlers/command-handler.js';packages/tm-core/src/modules/execution/executors/claude-executor.ts (1)
12-12: Simplify the import path for better clarity.The import path
../executors/base-executor.jsworks but is unnecessarily complex. Since bothclaude-executor.tsandbase-executor.tsare in the sameexecutors/folder, this should use a relative import from the same directory.Apply this diff to simplify:
-import { BaseExecutor } from '../executors/base-executor.js'; +import { BaseExecutor } from './base-executor.js';packages/tm-core/src/modules/execution/executors/executor-factory.ts (1)
6-6: Now let me verify the actual imports in the file to see if there are other concerns:Based on my verification, you shouldn't go back a folder from the same original folder (ex. being in a file in the validation folder and doing import {XyService} from "../validation";). The import path in question follows exactly this antipattern. While the file resolves correctly, this pattern can lead to errors of imports not being defined.
Fix the redundant import path in executor-factory.ts, line 6.
Change:
import { ClaudeExecutor } from '../executors/claude-executor.js';To:
import { ClaudeExecutor } from './claude-executor.js';Best practice is to use relative imports when importing from the same folder. Since both
executor-factory.tsandclaude-executor.tsare in the sameexecutors/directory, the direct relative import is appropriate.packages/tm-core/src/modules/tasks/tasks-domain.ts (2)
63-89: Consider returning the subtask object for better ergonomics.The method finds the subtask (line 80-82) but only returns
isSubtask: !!subtaskwithout the actual subtask. Callers must re-parse the ID and search again.Consider enhancing the return type:
async get( taskId: string, tag?: string -): Promise<{ task: Task | null; isSubtask: boolean }> { +): Promise<{ task: Task | null; isSubtask: boolean; subtask?: any }> { // Parse ID - check for dot notation (subtask) const parts = taskId.split('.'); const parentId = parts[0]; const subtaskIdPart = parts[1]; // Fetch the task const task = await this.taskService.getTask(parentId, tag); if (!task) { return { task: null, isSubtask: false }; } // Handle subtask notation (1.2, HAM-123.2) if (subtaskIdPart && task.subtasks) { const subtask = task.subtasks.find( (st) => String(st.id) === subtaskIdPart ); // Return parent task with isSubtask flag - return { task, isSubtask: !!subtask }; + return { task, isSubtask: !!subtask, subtask }; } // It's a regular task return { task, isSubtask: false }; }
108-110: Consider clarifying method names to reduce confusion.Both
getNext()(line 108) andgetNextAvailable()(line 147) appear to retrieve the next task, but:
getNext()returnsTask | nullgetNextAvailable()returnsstring | null(just the ID)The similar names may confuse callers about which method to use.
Consider renaming for clarity:
-async getNext(tag?: string): Promise<Task | null> { +async getNextTask(tag?: string): Promise<Task | null> { return this.taskService.getNextTask(tag); } -async getNextAvailable(): Promise<string | null> { +async getNextAvailableTaskId(): Promise<string | null> { return this.executionService.getNextAvailableTask(); }Also applies to: 147-149
packages/tm-core/src/modules/storage/adapters/api-storage.ts (6)
171-176: Brief scoping isn’t pushed to repository; potential cross-brief leakage.
getTasks(this.projectId, options)ignores the selected brief. PassbriefId(or equivalent) so the repo filters server-side.- const tasks = await this.retryOperation(() => - this.repository.getTasks(this.projectId, options) - ); + const tasks = await this.retryOperation(() => + this.repository.getTasks(this.projectId, { ...(options || {}), briefId: context.briefId } as any) + );Confirm
LoadTasksOptions(and repository) accept abriefIdfilter; otherwise add it to the interface.
594-613: renameTag is non-atomic and may orphan references.Creating new, then deleting old without migrating task references risks inconsistency. Prefer a repository-level atomic rename or a transactional op that updates tag references.
Does the backend offer a single
renameTag(projectId, oldTag, newTag)? If not, implement migration of all affected tasks between steps or wrap in a transaction (where supported).
626-641: copyTag may duplicate tasks without dedup semantics.Cloning tag metadata/tasks naïvely can lead to duplicates if tasks are keyed by tag. Ensure backend semantics are defined (deep copy vs reference) and guard against conflicts.
Document expected behavior and add idempotency (e.g., upsert with conflict handling).
802-816: Retry policy: add jitter and selective retry.Current exponential backoff lacks jitter and retries all errors. Add jitter and retry only on transient errors (5xx, network).
- const delay = Math.pow(2, attempt) * 1000; - await new Promise((resolve) => setTimeout(resolve, delay)); + const base = Math.pow(2, attempt) * 1000; + const jitter = Math.random() * 250; + await new Promise((r) => setTimeout(r, base + jitter));Consider classifying errors (e.g., by HTTP status on known repo errors) to avoid retrying 4xx.
258-273: Consider upsert to avoid read-before-write.You fetch
existingthen branch create/update. If the backend supports upsert, prefer it to reduce round trips and races.
654-674: getStats can be heavy; prefer backend aggregation.Pulling all tasks/tags to compute counts doesn’t scale. Provide a
repository.getStats(projectId)that returns pre-aggregated stats.packages/tm-core/src/modules/config/index.ts (1)
41-44: Barrel export is correctly implemented; direct manager imports are widespread but functional.The review concern is valid. The barrel correctly re-exports
ConfigManagerandDEFAULT_CONFIG_VALUES, but withinpackages/tm-core/src, multiple files bypass the barrel and import directly from the manager (e.g.,task-master-core.ts,tm-core.ts,task-service.ts,task-loader.service.ts,export.service.ts, and domain files). This won't cause breakage since the manager remains at the same path, but it's inconsistent with the barrel pattern.The re-exports themselves are correct and won't cause issues. Existing code will continue to work.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (113)
.changeset/dirty-hairs-know.md(0 hunks).changeset/fix-parent-directory-traversal.md(0 hunks).changeset/kind-lines-melt.md(0 hunks).changeset/light-owls-stay.md(0 hunks).changeset/metal-rocks-help.md(0 hunks).changeset/open-tips-notice.md(0 hunks).changeset/pre.json(0 hunks).changeset/some-dodos-wonder.md(0 hunks)CHANGELOG.md(1 hunks)CLAUDE.md(1 hunks)apps/cli/CHANGELOG.md(1 hunks)apps/cli/package.json(1 hunks)apps/cli/src/commands/auth.command.ts(1 hunks)apps/cli/src/commands/autopilot.command.ts(4 hunks)apps/cli/src/commands/autopilot/start.command.ts(2 hunks)apps/cli/src/commands/context.command.ts(1 hunks)apps/cli/src/commands/export.command.ts(4 hunks)apps/cli/src/commands/list.command.ts(5 hunks)apps/cli/src/commands/next.command.ts(4 hunks)apps/cli/src/commands/set-status.command.ts(5 hunks)apps/cli/src/commands/show.command.ts(7 hunks)apps/cli/src/commands/start.command.ts(8 hunks)apps/cli/src/index.ts(1 hunks)apps/cli/src/ui/components/dashboard.component.ts(1 hunks)apps/cli/src/ui/components/next-task.component.ts(1 hunks)apps/cli/src/ui/components/task-detail.component.ts(1 hunks)apps/cli/src/utils/display-helpers.ts(1 hunks)apps/cli/src/utils/ui.ts(1 hunks)apps/docs/CHANGELOG.md(1 hunks)apps/docs/package.json(1 hunks)apps/extension/package.json(1 hunks)apps/extension/src/services/terminal-manager.ts(3 hunks)apps/mcp/CHANGELOG.md(1 hunks)apps/mcp/package.json(0 hunks)apps/mcp/src/tools/autopilot/start.tool.ts(2 hunks)package.json(2 hunks)packages/ai-sdk-provider-grok-cli/CHANGELOG.md(1 hunks)packages/ai-sdk-provider-grok-cli/package.json(1 hunks)packages/build-config/CHANGELOG.md(1 hunks)packages/build-config/package.json(1 hunks)packages/claude-code-plugin/CHANGELOG.md(1 hunks)packages/tm-core/CHANGELOG.md(1 hunks)packages/tm-core/package.json(1 hunks)packages/tm-core/src/auth/index.ts(0 hunks)packages/tm-core/src/common/interfaces/index.ts(1 hunks)packages/tm-core/src/executors/index.ts(0 hunks)packages/tm-core/src/index.ts(1 hunks)packages/tm-core/src/modules/ai/index.ts(1 hunks)packages/tm-core/src/modules/ai/providers/base-provider.ts(2 hunks)packages/tm-core/src/modules/auth/auth-domain.ts(1 hunks)packages/tm-core/src/modules/auth/index.ts(1 hunks)packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts(1 hunks)packages/tm-core/src/modules/auth/managers/auth-manager.ts(1 hunks)packages/tm-core/src/modules/auth/services/credential-store.test.ts(1 hunks)packages/tm-core/src/modules/auth/services/credential-store.ts(1 hunks)packages/tm-core/src/modules/auth/services/oauth-service.ts(1 hunks)packages/tm-core/src/modules/auth/services/organization.service.ts(1 hunks)packages/tm-core/src/modules/auth/services/supabase-session-storage.ts(1 hunks)packages/tm-core/src/modules/commands/index.ts(1 hunks)packages/tm-core/src/modules/config/config-domain.ts(1 hunks)packages/tm-core/src/modules/config/index.ts(2 hunks)packages/tm-core/src/modules/config/managers/config-manager.spec.ts(1 hunks)packages/tm-core/src/modules/config/managers/config-manager.ts(1 hunks)packages/tm-core/src/modules/config/services/config-loader.service.spec.ts(1 hunks)packages/tm-core/src/modules/config/services/config-loader.service.ts(1 hunks)packages/tm-core/src/modules/config/services/config-merger.service.ts(1 hunks)packages/tm-core/src/modules/config/services/config-persistence.service.ts(1 hunks)packages/tm-core/src/modules/config/services/environment-config-provider.service.ts(1 hunks)packages/tm-core/src/modules/config/services/runtime-state-manager.service.spec.ts(1 hunks)packages/tm-core/src/modules/config/services/runtime-state-manager.service.ts(1 hunks)packages/tm-core/src/modules/dependencies/index.ts(1 hunks)packages/tm-core/src/modules/execution/executors/base-executor.ts(1 hunks)packages/tm-core/src/modules/execution/executors/claude-executor.ts(1 hunks)packages/tm-core/src/modules/execution/executors/executor-factory.ts(1 hunks)packages/tm-core/src/modules/execution/index.ts(1 hunks)packages/tm-core/src/modules/execution/services/executor-service.ts(1 hunks)packages/tm-core/src/modules/execution/types.ts(1 hunks)packages/tm-core/src/modules/git/adapters/git-adapter.ts(2 hunks)packages/tm-core/src/modules/git/git-domain.ts(1 hunks)packages/tm-core/src/modules/git/index.ts(1 hunks)packages/tm-core/src/modules/git/services/branch-name-generator.spec.ts(1 hunks)packages/tm-core/src/modules/integration/clients/supabase-client.ts(1 hunks)packages/tm-core/src/modules/integration/integration-domain.ts(1 hunks)packages/tm-core/src/modules/integration/services/export.service.ts(1 hunks)packages/tm-core/src/modules/reports/index.ts(1 hunks)packages/tm-core/src/modules/reports/managers/complexity-report-manager.ts(1 hunks)packages/tm-core/src/modules/storage/adapters/api-storage.ts(1 hunks)packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts(1 hunks)packages/tm-core/src/modules/storage/adapters/file-storage/format-handler.ts(1 hunks)packages/tm-core/src/modules/storage/index.ts(2 hunks)packages/tm-core/src/modules/storage/services/storage-factory.ts(1 hunks)packages/tm-core/src/modules/tasks/entities/task.entity.ts(1 hunks)packages/tm-core/src/modules/tasks/parser/index.ts(1 hunks)packages/tm-core/src/modules/tasks/repositories/supabase/dependency-fetcher.ts(1 hunks)packages/tm-core/src/modules/tasks/repositories/supabase/supabase-task-repository.ts(1 hunks)packages/tm-core/src/modules/tasks/repositories/task-repository.interface.ts(1 hunks)packages/tm-core/src/modules/tasks/services/preflight-checker.service.ts(1 hunks)packages/tm-core/src/modules/tasks/services/task-execution-service.ts(1 hunks)packages/tm-core/src/modules/tasks/services/task-loader.service.ts(1 hunks)packages/tm-core/src/modules/tasks/services/task-service.ts(1 hunks)packages/tm-core/src/modules/tasks/tasks-domain.ts(1 hunks)packages/tm-core/src/modules/ui/index.ts(1 hunks)packages/tm-core/src/modules/workflow/managers/workflow-state-manager.spec.ts(1 hunks)packages/tm-core/src/modules/workflow/managers/workflow-state-manager.ts(1 hunks)packages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.test.ts(1 hunks)packages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.ts(2 hunks)packages/tm-core/src/modules/workflow/services/workflow-activity-logger.ts(1 hunks)packages/tm-core/src/modules/workflow/services/workflow.service.ts(1 hunks)packages/tm-core/src/modules/workflow/workflow-domain.ts(1 hunks)packages/tm-core/src/services/index.ts(0 hunks)packages/tm-core/src/task-master-core.ts(2 hunks)packages/tm-core/src/tm-core.ts(1 hunks)update-task-migration-plan.md(1 hunks)
💤 Files with no reviewable changes (12)
- .changeset/dirty-hairs-know.md
- .changeset/some-dodos-wonder.md
- .changeset/open-tips-notice.md
- .changeset/light-owls-stay.md
- packages/tm-core/src/auth/index.ts
- packages/tm-core/src/services/index.ts
- packages/tm-core/src/executors/index.ts
- apps/mcp/package.json
- .changeset/kind-lines-melt.md
- .changeset/metal-rocks-help.md
- .changeset/fix-parent-directory-traversal.md
- .changeset/pre.json
✅ Files skipped from review due to trivial changes (8)
- packages/build-config/package.json
- packages/tm-core/CHANGELOG.md
- apps/docs/CHANGELOG.md
- apps/docs/package.json
- packages/tm-core/src/modules/reports/index.ts
- packages/tm-core/src/modules/workflow/managers/workflow-state-manager.spec.ts
- apps/mcp/CHANGELOG.md
- packages/tm-core/src/modules/git/services/branch-name-generator.spec.ts
🚧 Files skipped from review as they are similar to previous changes (32)
- packages/tm-core/src/modules/auth/services/credential-store.test.ts
- apps/cli/src/commands/context.command.ts
- packages/tm-core/src/modules/storage/adapters/file-storage/format-handler.ts
- packages/tm-core/src/modules/config/services/config-loader.service.spec.ts
- packages/tm-core/src/modules/execution/services/executor-service.ts
- packages/tm-core/src/modules/execution/index.ts
- packages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.test.ts
- packages/tm-core/src/tm-core.ts
- apps/cli/src/ui/components/task-detail.component.ts
- packages/tm-core/src/modules/tasks/entities/task.entity.ts
- packages/tm-core/src/modules/tasks/parser/index.ts
- packages/tm-core/src/modules/tasks/repositories/supabase/dependency-fetcher.ts
- packages/tm-core/src/modules/workflow/orchestrators/workflow-orchestrator.ts
- packages/tm-core/src/modules/config/services/config-loader.service.ts
- packages/tm-core/src/modules/git/adapters/git-adapter.ts
- apps/cli/src/ui/components/next-task.component.ts
- package.json
- packages/tm-core/src/modules/workflow/workflow-domain.ts
- packages/tm-core/src/modules/config/services/config-persistence.service.ts
- packages/tm-core/src/modules/auth/managers/auth-manager.ts
- packages/tm-core/src/modules/git/index.ts
- packages/tm-core/src/modules/auth/services/organization.service.ts
- packages/tm-core/src/modules/config/config-domain.ts
- apps/cli/src/commands/auth.command.ts
- packages/tm-core/src/modules/auth/index.ts
- packages/tm-core/src/modules/tasks/repositories/supabase/supabase-task-repository.ts
- apps/cli/src/utils/ui.ts
- apps/cli/src/ui/components/dashboard.component.ts
- packages/tm-core/src/modules/storage/services/storage-factory.ts
- packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts
- packages/tm-core/src/modules/integration/integration-domain.ts
- packages/tm-core/src/common/interfaces/index.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
Reference documentation at https://docs.task-master.dev rather than local file paths in content
Files:
CLAUDE.mdupdate-task-migration-plan.mdapps/cli/CHANGELOG.mdCHANGELOG.mdpackages/ai-sdk-provider-grok-cli/CHANGELOG.mdpackages/build-config/CHANGELOG.mdpackages/claude-code-plugin/CHANGELOG.md
**/*.{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/modules/config/services/runtime-state-manager.service.spec.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/config/managers/config-manager.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/modules/config/services/runtime-state-manager.service.spec.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/config/managers/config-manager.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/modules/config/services/runtime-state-manager.service.spec.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/config/managers/config-manager.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/modules/config/services/runtime-state-manager.service.spec.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/config/managers/config-manager.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/modules/config/services/runtime-state-manager.service.spec.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/config/managers/config-manager.spec.ts
🧠 Learnings (38)
📚 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/cli/src/index.tsapps/extension/package.jsonpackages/tm-core/src/modules/integration/services/export.service.tspackages/tm-core/src/modules/execution/executors/claude-executor.tspackages/tm-core/src/modules/tasks/services/task-loader.service.tspackages/tm-core/package.jsonapps/cli/src/commands/export.command.tspackages/tm-core/src/modules/tasks/services/task-execution-service.tspackages/tm-core/src/task-master-core.tsapps/cli/src/commands/list.command.tspackages/tm-core/src/modules/execution/types.tspackages/tm-core/src/index.ts
📚 Learning: 2025-09-26T19:10:32.906Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1252
File: tsconfig.json:22-28
Timestamp: 2025-09-26T19:10:32.906Z
Learning: In the eyaltoledano/claude-task-master repository, all internal tm/ package path mappings in tsconfig.json consistently point to TypeScript source files (e.g., "./packages/*/src/index.ts") rather than built JavaScript. This is intentional architecture because tsdown bundles internal packages directly from source during build time, eliminating the need for separate compilation of internal packages.
Applied to files:
apps/cli/src/index.tsapps/extension/package.jsonpackages/tm-core/src/modules/tasks/services/task-execution-service.tspackages/tm-core/src/modules/execution/types.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: Describes the high-level architecture of the Task Master CLI application, including the new tagged task lists system (architecture.mdc).
Applied to files:
CLAUDE.mdupdate-task-migration-plan.md
📚 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 : 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.
Applied to files:
apps/mcp/src/tools/autopilot/start.tool.ts
📚 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/tools/*.js : 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.
Applied to files:
apps/mcp/src/tools/autopilot/start.tool.ts
📚 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:
apps/mcp/src/tools/autopilot/start.tool.tsapps/cli/src/commands/set-status.command.ts
📚 Learning: 2025-10-15T14:43:40.410Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1314
File: packages/tm-core/src/auth/credential-store.ts:92-94
Timestamp: 2025-10-15T14:43:40.410Z
Learning: In the claude-task-master codebase, `getCredentials()` in `credential-store.ts` defaults to `allowExpired: true` to enable Supabase refresh flows. The Supabase client automatically handles token refresh when credentials are expired but have a valid refresh token. The `SupabaseSessionStorage` updates the credentials after Supabase performs the refresh. This is an intentional design pattern where the credential store is passive storage and Supabase manages the active token lifecycle.
Applied to files:
packages/tm-core/src/modules/auth/services/supabase-session-storage.ts
📚 Learning: 2025-09-26T19:03:33.225Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1252
File: package.json:130-132
Timestamp: 2025-09-26T19:03:33.225Z
Learning: In the eyaltoledano/claude-task-master repository, packages are bundled using tsdown during the build process, which means dependencies imported by the source code (including tm internal packages like tm/ai-sdk-provider-grok-cli) are included in the final bundle and don't need to be available as separate runtime dependencies, so they should remain as devDependencies rather than being moved to dependencies.
Applied to files:
apps/extension/package.json
📚 Learning: 2025-08-07T13:00:22.966Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1090
File: apps/extension/package.json:241-243
Timestamp: 2025-08-07T13:00:22.966Z
Learning: In monorepos, local packages should use "*" as the version constraint in package.json dependencies, as recommended by npm. This ensures the local version from within the same workspace is always used, rather than attempting to resolve from external registries. This applies to packages like task-master-ai within the eyaltoledano/claude-task-master monorepo.
Applied to files:
apps/extension/package.json
📚 Learning: 2025-09-26T19:07:10.485Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1252
File: packages/ai-sdk-provider-grok-cli/package.json:21-35
Timestamp: 2025-09-26T19:07:10.485Z
Learning: In the eyaltoledano/claude-task-master repository, the tsdown build configuration uses `noExternal: [/^tm\//]` which means internal tm/ packages are bundled into the final output while external npm dependencies remain external and are resolved from the root package.json dependencies at runtime. This eliminates the need for peer dependencies in internal packages since the root package.json already provides the required external dependencies.
Applied to files:
apps/extension/package.json
📚 Learning: 2025-07-18T17:06:04.909Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_providers.mdc:0-0
Timestamp: 2025-07-18T17:06:04.909Z
Learning: Applies to src/ai-providers/*.js : Provider modules must import the provider's create<ProviderName> function from ai-sdk/<provider-name>, and import generateText, streamText, generateObject from the core ai package, as well as the log utility from ../../scripts/modules/utils.js.
Applied to files:
packages/tm-core/src/modules/ai/index.tspackages/tm-core/src/modules/ai/providers/base-provider.ts
📚 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 src/ai-providers/*.js : Provider-specific wrappers for Vercel AI SDK functions must be implemented in src/ai-providers/*.js, each file corresponding to a provider.
Applied to files:
packages/tm-core/src/modules/ai/index.ts
📚 Learning: 2025-07-18T17:06:04.909Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_providers.mdc:0-0
Timestamp: 2025-07-18T17:06:04.909Z
Learning: Applies to src/ai-providers/*.js : Provider modules must export three functions: generate<ProviderName>Text, stream<ProviderName>Text, and generate<ProviderName>Object.
Applied to files:
packages/tm-core/src/modules/ai/index.ts
📚 Learning: 2025-07-18T17:06:04.909Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_providers.mdc:0-0
Timestamp: 2025-07-18T17:06:04.909Z
Learning: Applies to src/ai-providers/*.js : Create a new provider module file in src/ai-providers/ named <provider-name>.js when adding a new AI provider.
Applied to files:
packages/tm-core/src/modules/ai/index.ts
📚 Learning: 2025-07-18T17:06:04.909Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_providers.mdc:0-0
Timestamp: 2025-07-18T17:06:04.909Z
Learning: Applies to tests/unit/ai-providers/*.test.js : Create unit tests for the new provider in tests/unit/ai-providers/<provider-name>.test.js, mocking ai-sdk/<provider-name> and core ai module functions, and testing all exported functions for correct behavior and error handling.
Applied to files:
packages/tm-core/src/modules/ai/index.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/ai-services.js : Features that use AI models belong in 'scripts/modules/ai-services.js'.
Applied to files:
packages/tm-core/src/modules/ai/index.ts
📚 Learning: 2025-07-18T17:06:04.909Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_providers.mdc:0-0
Timestamp: 2025-07-18T17:06:04.909Z
Learning: Applies to scripts/modules/ai-services-unified.js : Integrate the new provider module with scripts/modules/ai-services-unified.js by importing it and adding an entry to the PROVIDER_FUNCTIONS map.
Applied to files:
packages/tm-core/src/modules/ai/index.ts
📚 Learning: 2025-07-18T17:06:04.909Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_providers.mdc:0-0
Timestamp: 2025-07-18T17:06:04.909Z
Learning: Applies to src/ai-providers/*.js : Implement generate<ProviderName>Text, stream<ProviderName>Text, and generate<ProviderName>Object functions in provider modules with basic validation and try/catch error handling.
Applied to files:
packages/tm-core/src/modules/ai/index.tspackages/tm-core/src/modules/ai/providers/base-provider.ts
📚 Learning: 2025-07-18T17:06:57.833Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_services.mdc:0-0
Timestamp: 2025-07-18T17:06:57.833Z
Learning: Applies to scripts/modules/commands.js : Do not import or call anything from the old `ai-services.js`, `ai-client-factory.js`, or `ai-client-utils.js` files.
Applied to files:
packages/tm-core/src/modules/ai/index.ts
📚 Learning: 2025-07-18T17:06:57.833Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/ai_services.mdc:0-0
Timestamp: 2025-07-18T17:06:57.833Z
Learning: Applies to scripts/modules/task-manager/*.js : Do not import or call anything from the old `ai-services.js`, `ai-client-factory.js`, or `ai-client-utils.js` files.
Applied to files:
packages/tm-core/src/modules/integration/services/export.service.tspackages/tm-core/src/modules/tasks/services/task-loader.service.tspackages/tm-core/src/modules/tasks/services/task-service.ts
📚 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 : Use consistent formatting for task files, include all task properties in text files, and format dependencies with status indicators.
Applied to files:
update-task-migration-plan.md
📚 Learning: 2025-10-20T16:44:58.151Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-20T16:44:58.151Z
Learning: Applies to {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} : Use synchronous top-level imports in tests; avoid dynamic await import()
Applied to files:
packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts
📚 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: Module dependencies should be mocked before importing the test module, following Jest's hoisting behavior.
Applied to files:
packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts
📚 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 : Generate task files from the current tag context, include tag information in generated files, and do not mix tasks from different tags in file generation.
Applied to files:
apps/cli/src/commands/autopilot/start.command.ts
📚 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 : Extract tasks from PRD documents using AI, create them in the current tag context (defaulting to 'master'), provide clear prompts to guide AI task generation, and validate/clean up AI-generated tasks.
Applied to files:
apps/cli/src/commands/autopilot/start.command.ts
📚 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 : Use AI to generate detailed subtasks within the current tag context, considering complexity analysis for subtask counts and ensuring proper IDs for newly created subtasks.
Applied to files:
apps/cli/src/commands/autopilot/start.command.ts
📚 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 : The default tag 'master' must be used for all existing and new tasks unless otherwise specified.
Applied to files:
apps/cli/src/commands/autopilot/start.command.ts
📚 Learning: 2025-07-18T17:18:17.759Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-07-18T17:18:17.759Z
Learning: Applies to scripts/modules/utils.js : Use tagged task system aware functions for task finding and manipulation, handle both task and subtask operations, and validate task IDs before operations.
Applied to files:
apps/cli/src/commands/autopilot/start.command.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/commands.js : All new user-facing commands should be added to 'scripts/modules/commands.js'.
Applied to files:
packages/tm-core/src/modules/commands/index.ts
📚 Learning: 2025-07-31T22:07:49.716Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-31T22:07:49.716Z
Learning: Applies to scripts/modules/commands.js : Export the registerCommands function and keep the CLI setup code clean and maintainable.
Applied to files:
packages/tm-core/src/modules/commands/index.ts
📚 Learning: 2025-09-22T19:45:04.337Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1232
File: packages/tm-core/package.json:50-51
Timestamp: 2025-09-22T19:45:04.337Z
Learning: In the eyaltoledano/claude-task-master project, Crunchyman-ralph intentionally omits version fields from internal/private packages in package.json files to prevent changesets from releasing new versions of these packages while still allowing them to be processed for dependency updates. The changesets warnings about missing versions are acceptable as they don't break the process and achieve the desired behavior of only releasing public packages.
Applied to files:
apps/cli/package.json
📚 Learning: 2025-09-24T15:12:12.658Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/.windsurfrules:0-0
Timestamp: 2025-09-24T15:12:12.658Z
Learning: Use task-master next to select the next actionable task with all dependencies satisfied
Applied to files:
apps/cli/src/commands/next.command.tsapps/cli/src/commands/start.command.ts
📚 Learning: 2025-10-08T19:57:00.982Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1282
File: packages/tm-core/src/utils/index.ts:16-34
Timestamp: 2025-10-08T19:57:00.982Z
Learning: For the tm-core package in the eyaltoledano/claude-task-master repository, the team prefers a minimal, need-based export strategy in index files rather than exposing all internal utilities. Exports should only be added when functions are actually consumed by other packages in the monorepo.
Applied to files:
packages/tm-core/src/index.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/dependency-manager.js : Features that handle task relationships belong in 'scripts/modules/dependency-manager.js'.
Applied to files:
packages/tm-core/src/modules/dependencies/index.ts
📚 Learning: 2025-07-18T17:09:40.548Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dependencies.mdc:0-0
Timestamp: 2025-07-18T17:09:40.548Z
Learning: Applies to scripts/modules/dependency-manager.js : Use graph traversal algorithms (DFS) to detect cycles
Applied to files:
packages/tm-core/src/modules/dependencies/index.ts
📚 Learning: 2025-07-18T17:18:17.759Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-07-18T17:18:17.759Z
Learning: Applies to scripts/modules/utils.js : Detect circular dependencies, validate dependency references, and support cross-tag dependency checking (future enhancement) in dependency analysis utilities.
Applied to files:
packages/tm-core/src/modules/dependencies/index.ts
📚 Learning: 2025-07-18T17:09:40.548Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dependencies.mdc:0-0
Timestamp: 2025-07-18T17:09:40.548Z
Learning: Applies to scripts/modules/dependency-manager.js : Handle both direct and indirect self-dependencies
Applied to files:
packages/tm-core/src/modules/dependencies/index.ts
📚 Learning: 2025-07-18T17:09:45.690Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dependencies.mdc:0-0
Timestamp: 2025-07-18T17:09:45.690Z
Learning: Applies to scripts/modules/dependency-manager.js : Check for existing dependencies to prevent duplicates
Applied to files:
packages/tm-core/src/modules/dependencies/index.ts
🧬 Code graph analysis (16)
apps/mcp/src/tools/autopilot/start.tool.ts (2)
packages/tm-core/src/index.ts (1)
createTmCore(29-29)packages/tm-core/src/tm-core.ts (1)
createTmCore(150-152)
apps/cli/src/commands/autopilot.command.ts (1)
packages/tm-core/src/tm-core.ts (2)
TmCore(46-141)createTmCore(150-152)
apps/cli/src/commands/set-status.command.ts (1)
packages/tm-core/src/tm-core.ts (2)
TmCore(46-141)createTmCore(150-152)
apps/extension/src/services/terminal-manager.ts (2)
packages/tm-core/src/index.ts (2)
TmCore(29-29)createTmCore(29-29)packages/tm-core/src/tm-core.ts (2)
TmCore(46-141)createTmCore(150-152)
apps/cli/src/commands/autopilot/start.command.ts (2)
packages/tm-core/src/index.ts (1)
createTmCore(29-29)packages/tm-core/src/tm-core.ts (1)
createTmCore(150-152)
apps/cli/src/commands/show.command.ts (1)
packages/tm-core/src/tm-core.ts (2)
TmCore(46-141)createTmCore(150-152)
packages/tm-core/src/modules/git/git-domain.ts (1)
packages/tm-core/src/modules/git/adapters/git-adapter.ts (1)
GitAdapter(15-780)
packages/tm-core/src/modules/auth/auth-domain.ts (2)
packages/tm-core/src/common/types/index.ts (1)
StorageType(11-11)packages/tm-core/src/modules/auth/services/organization.service.ts (3)
Organization(17-21)Brief(26-39)RemoteTask(44-60)
apps/cli/src/commands/export.command.ts (1)
packages/tm-core/src/tm-core.ts (2)
TmCore(46-141)createTmCore(150-152)
apps/cli/src/commands/list.command.ts (3)
apps/cli/src/index.ts (1)
TmCore(44-44)packages/tm-core/src/index.ts (2)
TmCore(29-29)createTmCore(29-29)packages/tm-core/src/tm-core.ts (2)
TmCore(46-141)createTmCore(150-152)
apps/cli/src/commands/next.command.ts (1)
packages/tm-core/src/tm-core.ts (2)
TmCore(46-141)createTmCore(150-152)
packages/tm-core/src/modules/tasks/tasks-domain.ts (4)
packages/tm-core/src/modules/tasks/services/task-service.ts (1)
TaskService(51-565)packages/tm-core/src/modules/tasks/services/task-execution-service.ts (1)
TaskExecutionService(42-308)packages/tm-core/src/modules/tasks/services/task-loader.service.ts (1)
TaskValidationResult(27-40)apps/cli/src/commands/autopilot.command.ts (1)
PreflightResult(34-40)
packages/tm-core/src/modules/ai/providers/base-provider.ts (1)
packages/tm-core/src/modules/ai/interfaces/ai-provider.interface.ts (3)
ProviderInfo(87-109)AIModel(63-82)ProviderUsageStats(213-240)
apps/cli/src/commands/start.command.ts (3)
apps/cli/src/index.ts (1)
TmCore(44-44)packages/tm-core/src/index.ts (2)
TmCore(29-29)createTmCore(29-29)packages/tm-core/src/tm-core.ts (2)
TmCore(46-141)createTmCore(150-152)
packages/tm-core/src/index.ts (1)
packages/tm-core/src/modules/tasks/tasks-domain.ts (1)
TasksDomain(29-210)
apps/cli/src/utils/display-helpers.ts (3)
packages/tm-core/src/tm-core.ts (1)
TmCore(46-141)packages/tm-core/src/common/types/index.ts (1)
StorageType(11-11)apps/cli/src/ui/components/header.component.ts (1)
displayHeader(32-81)
🪛 LanguageTool
update-task-migration-plan.md
[grammar] ~1111-~1111: Use a hyphen to join words.
Context: ...er, FuzzyTaskSearch` - Port context gathering logic from both old files 2. ...
(QB_NEW_EN_HYPHEN)
packages/build-config/CHANGELOG.md
[grammar] ~9-~9: Hier könnte ein Fehler sein.
Context: ...nfig ## null ## null ## null ## null ## 1.0.1
(QB_NEW_DE)
packages/claude-code-plugin/CHANGELOG.md
[grammar] ~3-~3: Hier könnte ein Fehler sein.
Context: # @tm/claude-code-plugin ## 0.0.3 ## 0.0.2
(QB_NEW_DE)
🪛 markdownlint-cli2 (0.18.1)
packages/ai-sdk-provider-grok-cli/CHANGELOG.md
7-7: Multiple headings with the same content
(MD024, no-duplicate-heading)
packages/build-config/CHANGELOG.md
9-9: Multiple headings with the same content
(MD024, no-duplicate-heading)
Crunchyman-ralph
changed the base branch from
ralph/feat/update.task.remote
to
next
Crunchyman-ralph
force-pushed
the
ralph/chore/refactor.tm.core
branch
from
1c9220e to
d710f57
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/tm-core/src/modules/tasks/services/task-service.ts (1)
372-385: Fix ID sorting: current numeric casts break for non‑numeric IDs (e.g., “HAM-123”).Number(“HAM-123”) yields NaN, leading to unstable/incorrect ordering in getNextTask. Use a natural/segmented comparator that handles dotted and mixed IDs.
- // Compare parent then subtask ID numerically - const [aPar, aSub] = String(a.id).split('.').map(Number); - const [bPar, bSub] = String(b.id).split('.').map(Number); - if (aPar !== bPar) return aPar - bPar; - return aSub - bSub; + return naturalCompareIds(String(a.id), String(b.id));- return Number(a.id) - Number(b.id); + return naturalCompareIds(String(a.id), String(b.id));Add this helper (outside the class or as a private static):
function naturalCompareIds(a: string, b: string): number { const segA = a.split('.'); const segB = b.split('.'); const len = Math.max(segA.length, segB.length); for (let i = 0; i < len; i++) { const sa = segA[i] ?? ''; const sb = segB[i] ?? ''; const na = /^\d+$/.test(sa) ? Number(sa) : NaN; const nb = /^\d+$/.test(sb) ? Number(sb) : NaN; if (!Number.isNaN(na) && !Number.isNaN(nb)) { if (na !== nb) return na - nb; } else { const cmp = sa.localeCompare(sb, undefined, { numeric: true, sensitivity: 'base' }); if (cmp !== 0) return cmp; } } return 0; }Also applies to: 401-412
♻️ Duplicate comments (5)
packages/tm-core/src/modules/tasks/services/task-service.ts (1)
17-17: Use the logger barrel import for consistency.-import { getLogger } from '../../../common/logger/factory.js'; +import { getLogger } from '../../../common/logger/index.js';packages/tm-core/src/tm-core.ts (3)
33-44: Docs show nonexistent APIs; update to façade methods.- * await tmcore.auth.login({ ... }); + * await tmcore.auth.authenticateWithOAuth({ ... }); ... - * const mainModel = tmcore.config.get('models.main'); + * const { main: mainModel } = tmcore.config.getModelConfig();
6-19: ESM/TypeScript config: ensure verbatimModuleSyntax is enabled.You’re using import type with .js extensions; add "verbatimModuleSyntax": true to packages/tm-core/tsconfig.json to preserve ESM import specifiers.
#!/bin/bash # Verify NodeNext + verbatimModuleSyntax in tm-core tsconfig jq '.compilerOptions | {module, moduleResolution, verbatimModuleSyntax}' packages/tm-core/tsconfig.json
98-108: Validate and normalize projectPath (absolute path required).+import path from 'node:path'; ... - if (!options.projectPath) { + if (!options.projectPath) { throw new TaskMasterError( 'Project path is required', ERROR_CODES.MISSING_CONFIGURATION ); } - - this._projectPath = options.projectPath; + if (!path.isAbsolute(options.projectPath)) { + throw new TaskMasterError( + 'Project path must be absolute', + ERROR_CODES.MISSING_CONFIGURATION + ); + } + this._projectPath = path.resolve(options.projectPath);packages/tm-core/src/modules/tasks/tasks-domain.ts (1)
35-40: Critical: Duplicate TaskService instance remains unresolved.The past review identified that
TaskLoaderServicecreates its own internalTaskServiceinstance at line 38, resulting in separate state and double initialization. This issue has not been addressed in the current changes.Please apply the fix suggested in the previous review: pass the shared
this.taskServicetoTaskLoaderServiceinstead ofconfigManager.getProjectRoot(), and update theTaskLoaderServiceconstructor accordingly.
🧹 Nitpick comments (5)
packages/tm-core/src/common/interfaces/storage.interface.ts (1)
167-173: Avoid duplicating the storage type literal; centralize the resolved type.Prefer a shared alias (e.g., type ResolvedStorageType = Exclude<StorageType,'auto'>) to keep types in sync across layers.
Example:
- getStorageType(): 'file' | 'api'; + getStorageType(): ResolvedStorageType;And in BaseStorage:
- abstract getStorageType(): 'file' | 'api'; + abstract getStorageType(): ResolvedStorageType;Define ResolvedStorageType once under common/types and re-use here and in adapters.
Also applies to: 250-251
packages/tm-core/src/modules/tasks/services/task-service.ts (3)
230-236: Type storage options instead of any; safer and clearer.-const storageOptions: any = {}; +import type { LoadTasksOptions } from '../../../common/interfaces/storage.interface.js'; +const storageOptions: LoadTasksOptions = {}; ... -const baseOptions: any = {}; +const baseOptions: LoadTasksOptions = {};Also applies to: 253-260
96-103: Guard optional filter props with optional chaining for TS soundness.-const canPushStatusFilter = - options.filter?.status && - !options.filter.priority && - !options.filter.tags && - !options.filter.assignee && - !options.filter.search && - options.filter.hasSubtasks === undefined; +const canPushStatusFilter = + !!options.filter?.status && + !options.filter?.priority && + !options.filter?.tags && + !options.filter?.assignee && + !options.filter?.search && + options.filter?.hasSubtasks === undefined;
463-477: Defensive null‑safety in filters.details and subtasks may be undefined on some entities; add fallbacks.
-const inDetails = task.details.toLowerCase().includes(searchLower); +const inDetails = (task.details ?? '').toLowerCase().includes(searchLower); ... -const hasSubtasks = task.subtasks.length > 0; +const hasSubtasks = (task.subtasks?.length ?? 0) > 0;apps/cli/src/utils/display-helpers.ts (1)
37-42: Use active tag from tmCore when available; avoid defaulting to “master”.If tmCore is provided, prefer tmCore.config.getActiveTag() to reflect real context.
-displayHeader({ - tag: options.tag || 'master', +displayHeader({ + tag: options.tag || tmCore.config.getActiveTag(), filePath: displayInfo.filePath, storageType: displayInfo.storageType, briefInfo: displayInfo.briefInfo });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/cli/src/utils/display-helpers.ts(1 hunks)packages/tm-core/src/common/interfaces/storage.interface.ts(2 hunks)packages/tm-core/src/modules/auth/auth-domain.ts(1 hunks)packages/tm-core/src/modules/config/managers/config-manager.spec.ts(1 hunks)packages/tm-core/src/modules/storage/adapters/api-storage.ts(2 hunks)packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts(2 hunks)packages/tm-core/src/modules/tasks/services/task-service.ts(2 hunks)packages/tm-core/src/modules/tasks/tasks-domain.ts(1 hunks)packages/tm-core/src/tm-core.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/tm-core/src/modules/storage/adapters/api-storage.ts
- packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/modules/config/managers/config-manager.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/modules/config/managers/config-manager.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/modules/config/managers/config-manager.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/modules/config/managers/config-manager.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/modules/config/managers/config-manager.spec.ts
🧠 Learnings (3)
📚 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:
packages/tm-core/src/tm-core.ts
📚 Learning: 2025-07-18T17:18:17.759Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-07-18T17:18:17.759Z
Learning: Applies to mcp-server/src/tools/utils.js : Use `normalizeProjectRoot(rawPath, log)`, `getRawProjectRootFromSession(session, log)`, and `withNormalizedProjectRoot(executeFn)` in `mcp-server/src/tools/utils.js` to ensure project root paths are normalized for MCP tools.
Applied to files:
packages/tm-core/src/tm-core.ts
📚 Learning: 2025-07-18T17:16:13.793Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-07-18T17:16:13.793Z
Learning: Applies to **/*.test.js : Set mock environment variables in test setup and restore them after each test.
Applied to files:
packages/tm-core/src/modules/config/managers/config-manager.spec.ts
🧬 Code graph analysis (4)
packages/tm-core/src/modules/tasks/tasks-domain.ts (4)
packages/tm-core/src/modules/tasks/services/task-service.ts (3)
TaskService(51-563)GetTaskListOptions(38-45)TaskListResult(22-33)packages/tm-core/src/modules/tasks/services/task-execution-service.ts (1)
TaskExecutionService(42-308)packages/tm-core/src/modules/tasks/services/task-loader.service.ts (1)
TaskValidationResult(27-40)apps/cli/src/commands/autopilot.command.ts (1)
PreflightResult(34-40)
packages/tm-core/src/tm-core.ts (6)
packages/tm-core/src/modules/tasks/tasks-domain.ts (1)
TasksDomain(29-219)packages/tm-core/src/modules/auth/auth-domain.ts (1)
AuthDomain(37-208)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)
packages/tm-core/src/modules/auth/auth-domain.ts (2)
packages/tm-core/src/common/types/index.ts (1)
StorageType(11-11)packages/tm-core/src/modules/auth/services/organization.service.ts (3)
Organization(17-21)Brief(26-39)RemoteTask(44-60)
apps/cli/src/utils/display-helpers.ts (3)
packages/tm-core/src/tm-core.ts (1)
TmCore(46-151)packages/tm-core/src/common/types/index.ts (1)
StorageType(11-11)apps/cli/src/ui/components/header.component.ts (1)
displayHeader(32-81)
⏰ 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)
- GitHub Check: typecheck
- GitHub Check: build
- GitHub Check: Test
🔇 Additional comments (3)
packages/tm-core/src/modules/tasks/tasks-domain.ts (1)
63-89: LGTM! Clean subtask ID handling.The
get()method correctly parses dotted task IDs (e.g., "1.2"), retrieves the parent task, and validates subtask membership. The return structure clearly indicates whether the ID represents a subtask.packages/tm-core/src/modules/auth/auth-domain.ts (2)
23-32: LGTM! Well-designed interface.The
StorageDisplayInfointerface correctly excludes'auto'fromStorageTypesince it represents resolved storage information. The optional fields appropriately handle different storage contexts.
188-207: LGTM! Robust URL construction.The
getWebAppUrl()helper correctly handles environment variable fallbacks, protocol detection, and local vs. remote domain differentiation. The logic is defensive and handles edge cases well.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/tm-core/src/modules/tasks/repositories/supabase/supabase-task-repository.ts (1)
50-58: Remove unnecessaryasyncfromgetBriefIdOrThrow()and update all callers.
AuthManager.getContext()is synchronous, sogetBriefIdOrThrow()no longer needs to be async. The method has noawaitoperations, yet is still declared asasyncwith aPromise<string>return type, forcing unnecessary awaits at all three call sites (lines 64, 117, 160).Apply these changes:
- private async getBriefIdOrThrow(): Promise<string> { + private getBriefIdOrThrow(): string { const context = this.authManager.getContext(); if (!context?.briefId) { throw new Error( 'No brief selected. Please select a brief first using: tm context brief' ); } return context.briefId; }Update callers:
- const briefId = await this.getBriefIdOrThrow(); + const briefId = this.getBriefIdOrThrow();
♻️ Duplicate comments (1)
packages/tm-core/src/tm-core.ts (1)
36-44: Fix doc example to match actual API.The example uses non-existent methods:
tmcore.auth.login(...)should betmcore.auth.authenticateWithOAuth(...)tmcore.config.get('models.main')should betmcore.config.getModelConfig()ortmcore.config.getConfig()Based on the AuthDomain and ConfigDomain facades in the codebase.
🧹 Nitpick comments (1)
packages/tm-core/src/modules/auth/services/supabase-session-storage.ts (1)
136-146: Consider removing the type cast to catch type mismatches.The
as AuthCredentialscast on line 146 may mask type errors. If all required fields ofAuthCredentialsare genuinely populated by this point, the cast should be unnecessary. Consider verifying that:
userIdis guaranteed to have a value (line 142 has a fallback chain but might not cover all cases)- All required fields in
AuthCredentialsare properly setIf you prefer to keep the cast for practical reasons, consider adding a runtime assertion or comment explaining why the cast is safe.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/cli/src/commands/autopilot.command.ts(7 hunks)packages/tm-core/src/modules/auth/services/supabase-session-storage.ts(5 hunks)packages/tm-core/src/modules/config/managers/config-manager.spec.ts(1 hunks)packages/tm-core/src/modules/tasks/repositories/supabase/supabase-task-repository.ts(2 hunks)packages/tm-core/src/modules/tasks/services/task-loader.service.ts(2 hunks)packages/tm-core/src/modules/tasks/tasks-domain.ts(1 hunks)packages/tm-core/src/tm-core.ts(1 hunks)packages/tm-core/tests/integration/auth-token-refresh.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/cli/src/commands/autopilot.command.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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/modules/config/managers/config-manager.spec.tspackages/tm-core/tests/integration/auth-token-refresh.test.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/modules/config/managers/config-manager.spec.tspackages/tm-core/tests/integration/auth-token-refresh.test.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/modules/config/managers/config-manager.spec.tspackages/tm-core/tests/integration/auth-token-refresh.test.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/modules/config/managers/config-manager.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/modules/config/managers/config-manager.spec.tspackages/tm-core/tests/integration/auth-token-refresh.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
**/*.test.ts: Use established mocking patterns for dependencies such as bcrypt and Prisma in test files
Test all code paths, including edge cases and error scenarios, in test files
Use descriptive names for test functions, such as should_return_error_for_invalid_input
Files:
packages/tm-core/tests/integration/auth-token-refresh.test.ts
{packages/*/tests/integration/**/*.test.ts,apps/*/tests/integration/**/*.test.ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Place integration tests under tests/integration as *.test.ts for packages and apps
Files:
packages/tm-core/tests/integration/auth-token-refresh.test.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1282
File: packages/tm-core/src/utils/index.ts:16-34
Timestamp: 2025-10-08T19:57:00.982Z
Learning: For the tm-core package in the eyaltoledano/claude-task-master repository, the team prefers a minimal, need-based export strategy in index files rather than exposing all internal utilities. Exports should only be added when functions are actually consumed by other packages in the monorepo.
📚 Learning: 2025-07-18T17:16:13.793Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-07-18T17:16:13.793Z
Learning: Applies to **/*.test.js : Do not import real AI service clients in tests; create fully mocked versions that return predictable responses.
Applied to files:
packages/tm-core/src/modules/config/managers/config-manager.spec.ts
📚 Learning: 2025-07-18T17:16:13.793Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-07-18T17:16:13.793Z
Learning: Applies to **/*.test.js : Troubleshoot mock functions not being called by ensuring mocks are defined before imports, referencing the correct mock instance, and verifying import paths.
Applied to files:
packages/tm-core/src/modules/config/managers/config-manager.spec.ts
📚 Learning: 2025-07-18T17:16:13.793Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-07-18T17:16:13.793Z
Learning: Applies to **/*.test.js : Follow the test file organization: mocks must be set up before importing modules under test, and spies on mocked modules should be set up after imports.
Applied to files:
packages/tm-core/src/modules/config/managers/config-manager.spec.ts
📚 Learning: 2025-07-18T17:16:13.793Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-07-18T17:16:13.793Z
Learning: Applies to **/*.test.js : Be careful with how you mock or stub functions that depend on module state; use factory functions in mocks to ensure proper initialization order.
Applied to files:
packages/tm-core/src/modules/config/managers/config-manager.spec.ts
📚 Learning: 2025-07-18T17:16:13.793Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-07-18T17:16:13.793Z
Learning: Applies to tests/{integration,e2e}/**/*.test.js : Properly mock session objects when required by functions, and reset environment variables between tests if modified.
Applied to files:
packages/tm-core/src/modules/config/managers/config-manager.spec.ts
📚 Learning: 2025-07-18T17:16:13.793Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-07-18T17:16:13.793Z
Learning: Applies to **/*.test.js : Always declare mocks before importing the modules being tested in Jest test files.
Applied to files:
packages/tm-core/src/modules/config/managers/config-manager.spec.ts
📚 Learning: 2025-07-18T17:16:13.793Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-07-18T17:16:13.793Z
Learning: Applies to tests/{unit,integration,e2e}/**/*.test.js : Do not try to use the real action implementation without proper mocking, and do not mock Commander partially—either mock it completely or test the action directly.
Applied to files:
packages/tm-core/src/modules/config/managers/config-manager.spec.ts
📚 Learning: 2025-07-18T17:16:13.793Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-07-18T17:16:13.793Z
Learning: Applies to **/*.test.js : Follow the mock-first-then-import pattern for all Jest mocks.
Applied to files:
packages/tm-core/src/modules/config/managers/config-manager.spec.ts
📚 Learning: 2025-07-18T17:18:17.759Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-07-18T17:18:17.759Z
Learning: Applies to mcp-server/src/tools/utils.js : Use `normalizeProjectRoot(rawPath, log)`, `getRawProjectRootFromSession(session, log)`, and `withNormalizedProjectRoot(executeFn)` in `mcp-server/src/tools/utils.js` to ensure project root paths are normalized for MCP tools.
Applied to files:
packages/tm-core/src/tm-core.ts
📚 Learning: 2025-10-15T14:43:40.410Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1314
File: packages/tm-core/src/auth/credential-store.ts:92-94
Timestamp: 2025-10-15T14:43:40.410Z
Learning: In the claude-task-master codebase, `getCredentials()` in `credential-store.ts` defaults to `allowExpired: true` to enable Supabase refresh flows. The Supabase client automatically handles token refresh when credentials are expired but have a valid refresh token. The `SupabaseSessionStorage` updates the credentials after Supabase performs the refresh. This is an intentional design pattern where the credential store is passive storage and Supabase manages the active token lifecycle.
Applied to files:
packages/tm-core/src/modules/auth/services/supabase-session-storage.ts
🧬 Code graph analysis (3)
packages/tm-core/src/modules/tasks/tasks-domain.ts (4)
packages/tm-core/src/modules/tasks/services/task-service.ts (3)
TaskService(51-563)GetTaskListOptions(38-45)TaskListResult(22-33)packages/tm-core/src/modules/tasks/services/task-execution-service.ts (1)
TaskExecutionService(42-308)packages/tm-core/src/modules/tasks/services/task-loader.service.ts (2)
TaskLoaderService(58-384)TaskValidationResult(26-39)apps/cli/src/commands/autopilot.command.ts (1)
PreflightResult(34-40)
packages/tm-core/src/modules/tasks/services/task-loader.service.ts (1)
packages/tm-core/src/modules/tasks/services/task-service.ts (1)
TaskService(51-563)
packages/tm-core/src/tm-core.ts (6)
packages/tm-core/src/modules/tasks/tasks-domain.ts (1)
TasksDomain(29-219)packages/tm-core/src/modules/auth/auth-domain.ts (1)
AuthDomain(37-208)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)
⏰ 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)
- GitHub Check: Test
- GitHub Check: build
- GitHub Check: typecheck
🔇 Additional comments (20)
packages/tm-core/tests/integration/auth-token-refresh.test.ts (1)
13-15: LGTM! Import paths correctly updated to new module structure.The import paths have been properly updated to reflect the new domain-driven architecture under
src/modules/auth/, and the.jsextensions follow ESM best practices. No test logic was modified, which is appropriate for this refactoring.packages/tm-core/src/modules/config/managers/config-manager.spec.ts (1)
8-20: LGTM! Import and mock paths correctly updated.The refactor properly updates all import paths to reflect the new domain structure:
- DEFAULT_CONFIG_VALUES correctly imports from the common interfaces module
- All service imports consistently use
../services/paths- Mock declarations perfectly match their corresponding import paths
The past review concerns have been fully addressed.
packages/tm-core/src/modules/auth/services/supabase-session-storage.ts (6)
10-12: LGTM! Import paths updated for domain structure.The import path changes align with the broader refactoring to a domain-driven architecture.
27-43: LGTM! Improved session building logic.Removing the hardcoded expiration default and conditionally including
expires_atonly when present is the correct approach. This allows Supabase to manage the refresh lifecycle without arbitrary values interfering.
48-69: LGTM! Defensive input handling.Accepting both string and object formats for
sessionDatais good defensive programming and handles variability in how Supabase may invoke this method.
74-96: LGTM! Stricter validation prevents partial sessions.The removal of the explicit
allowExpiredparameter is safe sincegetCredentials()defaults toallowExpired: true(based on learnings). The new validation requiring both access token and refresh token prevents partial session states while still allowing Supabase to handle token refresh for expired but complete sessions.Based on learnings.
114-125: LGTM! Critical validation for atomic token rotation.The validation ensuring both
tokenandrefreshTokenare present before saving is essential for preventing partial session states during Supabase's refresh token rotation. The warning log provides good visibility when incomplete updates occur.
127-157: Excellent atomic handling of refresh token rotation.The implementation correctly:
- Detects refresh token rotation for debugging visibility
- Atomically updates all credential fields together
- Adds comprehensive logging for troubleshooting
- Synchronously saves to ensure atomicity during refresh
This is exactly the right approach for handling Supabase's token rotation flow.
packages/tm-core/src/modules/tasks/repositories/supabase/supabase-task-repository.ts (1)
2-11: All refactored import paths resolve correctly and export the expected symbols.Verification confirms that all seven imports successfully resolve to their source files and all imported symbols are properly exported:
Taskfrom index.tsDatabaseandJsonfrom database.types.tsTaskMapperfrom TaskMapper.tsAuthManagerfrom auth-manager.tsDependencyFetcherfrom dependency-fetcher.tsTaskWithRelationsandTaskDatabaseUpdatefrom repository-types.tsLoadTasksOptionsfrom storage.interface.tsThe refactoring is complete and correct.
packages/tm-core/src/modules/tasks/services/task-loader.service.ts (3)
6-8: LGTM! Import paths align with refactoring.The updated import paths to
common/typesandcommon/loggerare consistent with the domain-driven restructuring. Usingimport typefor TaskService is correct since it's only referenced in type positions.
59-66: Excellent refactor to dependency injection.Accepting
TaskServiceas a constructor parameter enables sharing a single instance across services, eliminating the duplicate state issue from the previous architecture. The null check is good defensive programming.
121-128: LGTM! Simplified control flow.Direct usage of
this.taskServiceis safe since the constructor enforces a non-null instance. This removes the complexity of lazy initialization.packages/tm-core/src/tm-core.ts (4)
6-29: LGTM! Clean interface and imports.The TmCoreOptions interface is clear and well-documented. ESM imports with
.jsextensions are consistent with the NodeNext module settings.
49-78: LGTM! Type-safe initialization pattern.Using private definite-assignment fields with public getters avoids the type-casting issues from earlier iterations. The pattern is clean and maintains type safety.
107-113: Path validation is correct.Good use of
path.isAbsolute()andpath.resolve()to validate and normalize the project path. The error message helpfully includes the received value.
125-153: LGTM! Initialization sequence is well-structured.The initialization order (ConfigManager → domains → async setup) is logical. Error handling consistently wraps failures with operation context.
packages/tm-core/src/modules/tasks/tasks-domain.ts (4)
6-24: LGTM! Well-organized imports.Imports are logically grouped and use type-only imports where appropriate. Consistent ESM
.jsextensions.
35-40: Excellent! Shared TaskService instance eliminates state duplication.Line 38 correctly passes the shared
this.taskServicetoTaskLoaderService, ensuring all services operate on the same state. This directly addresses the critical issue from the past review about duplicate TaskService instances.
63-89: LGTM! Subtask ID parsing is well-implemented.The method cleanly handles both regular task IDs and subtask notation (e.g., "1.2"), returning the parent task with an
isSubtaskflag. The logic is straightforward and robust.
91-219: LGTM! Comprehensive and well-organized facade API.The TasksDomain provides a clean, unified interface for task operations. Methods are logically grouped, delegate appropriately to underlying services, and maintain consistent naming conventions.
Crunchyman-ralph
force-pushed
the
ralph/chore/refactor.tm.core
branch
from
3819146 to
a3825b7
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/tm-core/src/modules/auth/services/supabase-session-storage.ts (2)
100-101: “Atomic” here is process-local; add file-level atomicity if multi-process is possible.If multiple CLI instances can refresh simultaneously, consider atomic file writes (write-temp + fs.rename) or advisory locking in CredentialStore to prevent interleaving.
106-106: Reduce log level to avoid noise on every refresh call.Demote to debug; keep the success/info log below for visibility.
- this.logger.info('Supabase called setItem - storing refreshed session'); + this.logger.debug('Supabase called setItem - storing refreshed session');
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/tm-core/src/modules/auth/services/supabase-session-storage.ts(5 hunks)packages/tm-core/src/modules/tasks/repositories/supabase/supabase-task-repository.ts(5 hunks)packages/tm-core/src/tm-core.ts(1 hunks)packages/tm-core/tests/integration/auth-token-refresh.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/tm-core/tests/integration/auth-token-refresh.test.ts
- packages/tm-core/src/tm-core.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1282
File: packages/tm-core/src/utils/index.ts:16-34
Timestamp: 2025-10-08T19:57:00.982Z
Learning: For the tm-core package in the eyaltoledano/claude-task-master repository, the team prefers a minimal, need-based export strategy in index files rather than exposing all internal utilities. Exports should only be added when functions are actually consumed by other packages in the monorepo.
📚 Learning: 2025-10-15T14:43:40.410Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1314
File: packages/tm-core/src/auth/credential-store.ts:92-94
Timestamp: 2025-10-15T14:43:40.410Z
Learning: In the claude-task-master codebase, `getCredentials()` in `credential-store.ts` defaults to `allowExpired: true` to enable Supabase refresh flows. The Supabase client automatically handles token refresh when credentials are expired but have a valid refresh token. The `SupabaseSessionStorage` updates the credentials after Supabase performs the refresh. This is an intentional design pattern where the credential store is passive storage and Supabase manages the active token lifecycle.
Applied to files:
packages/tm-core/src/modules/auth/services/supabase-session-storage.ts
⏰ 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). (1)
- GitHub Check: Test
🔇 Additional comments (5)
packages/tm-core/src/modules/tasks/repositories/supabase/supabase-task-repository.ts (2)
2-11: Import path refactoring looks good.The updated import paths correctly reflect the new domain-driven architecture with centralized common modules. This aligns with the PR's stated objective.
50-58: ****
AuthManager.getContext()is a synchronous method that returnsUserContext | nulldirectly—not a Promise. The method is defined atpackages/tm-core/src/modules/auth/managers/auth-manager.ts:180-183with the signaturegetContext(): UserContext | null. The change fromawait this.authManager.getContext()tothis.authManager.getContext()is correct and actually fixes the buggy behavior of awaiting a non-Promise value.Likely an incorrect or invalid review comment.
packages/tm-core/src/modules/auth/services/supabase-session-storage.ts (3)
32-35: LGTM: defer expires_at to Supabase when absent.Only including expires_at when present is correct and avoids arbitrary defaults.
78-90: Session retrieval criteria are sound (require both tokens, let Supabase refresh).Requiring both access and refresh tokens and not forcing expiry checks aligns with our design where allowExpired defaults to true in the store.
Based on learnings.
127-135: Nice: rotation detection + debug log aids incident triage.
packages/tm-core/src/modules/auth/services/supabase-session-storage.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/extension/src/services/terminal-manager.ts (1)
71-75: Command injection risk: unescaped user content is interpolated into a shell command.executionOutput may contain quotes/newlines/shell metacharacters. Sending it directly via terminal.sendText is unsafe and shell-dependent.
Apply this minimal, cross‑platform quoting fix and use it when building the command:
- const command = `claude "${startResult.executionOutput}"`; + const command = `claude ${this.quoteForShell(startResult.executionOutput)}`; terminal.sendText(command);Add this helper to the class (POSIX single‑quote strategy; PowerShell single‑quote doubling; normalize newlines):
private quoteForShell(arg: string): string { const s = String(arg ?? '').replace(/\r?\n/g, ' '); if (process.platform === 'win32') { // Prefer PowerShell-safe quoting by default on Windows terminals return `'${s.replace(/'/g, "''")}'`; } // POSIX shells return `'${s.replace(/'/g, `'\\''`)}'`; }Alternatively (preferred), switch to vscode.tasks with ShellExecution to let VS Code handle quoting per shell.
🧹 Nitpick comments (6)
apps/extension/src/services/terminal-manager.ts (6)
52-56: Validate the tasks.start() contract and narrow types.You assume { started, executionOutput, error }. Please confirm this is the new stable return shape and, if available, import its type to avoid structural assumptions.
108-114: Memoize core initialization to avoid races and duplicate work.Concurrent executeTask calls can trigger multiple createTmCore() runs. Memoize with a Promise and have initializeCore return the instance.
Example:
- private tmCore?: TmCore; + private tmCore?: TmCore; + private tmCoreInit?: Promise<TmCore>; - private async initializeCore(): Promise<void> { - if (!this.tmCore) { + private async initializeCore(): Promise<TmCore> { + if (!this.tmCore && !this.tmCoreInit) { const workspaceRoot = vscode.workspace.workspaceFolders?.[0]?.uri.fsPath; if (!workspaceRoot) { throw new Error('No workspace folder found'); } - this.tmCore = await createTmCore({ projectPath: workspaceRoot }); + this.tmCoreInit = createTmCore({ projectPath: workspaceRoot }).then(core => { + this.tmCore = core; + this.tmCoreInit = undefined; + return core; + }).catch(err => { + this.tmCoreInit = undefined; + throw err; + }); } - } + return this.tmCore ?? (await this.tmCoreInit!); + }
127-131: Dispose terminals when cleaning up entries.Currently you only remove the map entry. Dispose to avoid leaked terminals if callers assume cleanupTerminal fully cleans resources.
cleanupTerminal(taskId: string): void { const terminal = this.terminals.get(taskId); if (terminal) { + try { terminal.dispose(); } catch {} this.terminals.delete(taskId); } }Also consider wiring vscode.window.onDidCloseTerminal to prune the map on user-initiated closes.
147-150: Confirm whether TmCore needs explicit shutdown.If tm-core wires watchers/IO, a dispose/shutdown might be required. If such API exists, call it; otherwise, clearing the reference is fine.
Example:
if (this.tmCore && typeof (this.tmCore as any).dispose === 'function') { try { await (this.tmCore as any).dispose(); } catch (e) { this.logger.error('tm-core dispose failed:', e); } } this.tmCore = undefined;
24-24: Optional: avoid non‑null assertions downstream.After memoized initializeCore returns TmCore, use a local const tmCore = await this.initializeCore() and call tmCore.tasks.start(...) to eliminate this.tmCore! usage.
43-46: Demote or remove debug-only log.“TaskId type: …” seems temporary. Consider removing or guarding behind a debug flag to reduce log noise.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/extension/package.json(1 hunks)apps/extension/src/services/terminal-manager.ts(5 hunks)packages/tm-core/src/task-master-core.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/tm-core/src/task-master-core.ts
✅ Files skipped from review due to trivial changes (1)
- apps/extension/package.json
🧰 Additional context used
🧬 Code graph analysis (1)
apps/extension/src/services/terminal-manager.ts (1)
packages/tm-core/src/tm-core.ts (2)
TmCore(47-161)createTmCore(170-172)
⏰ 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)
- GitHub Check: typecheck
- GitHub Check: build
- GitHub Check: Test
🔇 Additional comments (1)
apps/extension/src/services/terminal-manager.ts (1)
7-7: Migration to the new @tm/core surface looks correct.Import rename to createTmCore/TmCore is aligned with the unified facade.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/extension-ci.yml (1)
9-10: Minor: Consistent YAML string quoting.The path filter strings are now double-quoted, which is cosmetically more consistent with YAML best practices but functionally equivalent to the previous format.
Also applies to: 16-17
packages/tm-core/src/modules/auth/services/supabase-session-storage.ts (1)
78-89: Consider explicitly passingallowExpired: truefor consistency.While
getCredentials()defaults toallowExpired: true(per learnings),setItemat lines 110-111 explicitly passes this parameter. For consistency and clarity, consider making it explicit here as well.Apply this diff for consistency:
- const credentials = this.store.getCredentials(); + const credentials = this.store.getCredentials({ allowExpired: true });Based on learnings
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/extension-ci.yml(2 hunks).github/workflows/extension-release.yml(2 hunks)apps/extension/docs/extension-CI-setup.md(1 hunks)apps/extension/docs/extension-development-guide.md(1 hunks)packages/tm-core/src/modules/auth/services/supabase-session-storage.ts(5 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/extension/docs/extension-CI-setup.md
- apps/extension/docs/extension-development-guide.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-15T14:43:40.410Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1314
File: packages/tm-core/src/auth/credential-store.ts:92-94
Timestamp: 2025-10-15T14:43:40.410Z
Learning: In the claude-task-master codebase, `getCredentials()` in `credential-store.ts` defaults to `allowExpired: true` to enable Supabase refresh flows. The Supabase client automatically handles token refresh when credentials are expired but have a valid refresh token. The `SupabaseSessionStorage` updates the credentials after Supabase performs the refresh. This is an intentional design pattern where the credential store is passive storage and Supabase manages the active token lifecycle.
Applied to files:
packages/tm-core/src/modules/auth/services/supabase-session-storage.ts
⏰ 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)
- GitHub Check: typecheck
- GitHub Check: build
- GitHub Check: Test
🔇 Additional comments (7)
.github/workflows/extension-release.yml (2)
110-110: Minor: Add notification message to success output.This is a cosmetic addition to the success notification that informs users about the GitHub release. The change adds useful context to the workflow completion message.
40-40: npm scripttypecheckexists and is properly configured.Verification confirms that
apps/extension/package.jsoncontains"typecheck": "tsc --noEmit". The script name change fromcheck-typestotypecheckis valid and the workflow will execute successfully..github/workflows/extension-ci.yml (1)
74-74: No action required—thetypecheckscript is properly configured.Verification confirms the
typecheckscript exists inapps/extension/package.jsonwith value"tsc --noEmit", and the oldcheck-typesscript has been successfully removed. The workflow change tonpm run typecheckintroduces no breaking changes.packages/tm-core/src/modules/auth/services/supabase-session-storage.ts (4)
9-12: LGTM! Import issues from previous review have been resolved.The imports now correctly use type-only imports for
SupportedStorageandAuthCredentials, and the path toCredentialStorehas been fixed to reference the same directory.
32-35: Good defensive change: conditionally include expires_at.Removing the arbitrary 1-hour default and only including
expires_atwhen it exists allows Supabase to handle token refresh properly without incorrect expiry information.
52-59: Excellent fixes: flexible session parsing and proper userId handling.Two improvements here:
- Handling both string and object formats makes the parser more robust
- Line 59 no longer defaults
userIdto'unknown', which fixes the critical issue from the previous review where the truthy default would bypass downstream validation
110-167: Excellent refactoring! All critical issues from previous review have been resolved.The atomic update logic is well-implemented with:
- Safe spreading using
?? {}at line 148 (fixes past crash on null)- Proper nullish coalescing (
??) foruserIdand- Robust validation preventing partial session states
- Clear logging for rotation detection and debugging
The explicit
allowExpired: trueparameter and comprehensive error handling demonstrate good defensive programming.
1 participant
What type of PR is this?
Description
createTmCore({ projectPath })auth,config,git,tasks,workflowsrc/modules/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
Refactor
Chores