feat: add oauth with remote server#1178
Conversation
|
WalkthroughAdds a complete authentication subsystem (types, config, credential store, OAuth service, Supabase client, AuthManager), a CLI AuthCommand with subcommands and programmatic API, a configurable Logger package, storage auto-detection integrating auth, build-time dotenv/env injection, and minor test formatting changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CLI as CLI AuthCommand
participant AM as AuthManager
participant OAuth as OAuthService
participant Browser
participant Local as Local Callback Server
participant SB as SupabaseAuthClient
participant Store as CredentialStore
User->>CLI: tm auth login
CLI->>AM: authenticateWithOAuth(options, hooks)
AM->>OAuth: authenticate(options)
OAuth->>Local: start local server (ephemeral port)
OAuth->>CLI: onAuthUrl(url)
CLI->>Browser: open url (optional)
Browser-->>User: user signs in
Browser->>Local: GET /callback?...
Local->>OAuth: handle callback -> extract tokens
OAuth->>SB: getUser(token)
SB-->>OAuth: user info
OAuth->>Store: saveCredentials(credentials)
OAuth-->>AM: return AuthCredentials
AM-->>CLI: AuthResult(success)
CLI-->>User: print success/status
sequenceDiagram
autonumber
participant StorageFactory
participant ConfigMgr
participant AuthMgr
participant Storage as File/API Storage
StorageFactory->>ConfigMgr: getStorageConfig()
alt storage.type == 'api'
StorageFactory->>Storage: init API storage (use config apiEndpoint/apiAccessToken)
else storage.type == 'auto'
StorageFactory->>AuthMgr: isAuthenticated()
alt authenticated
StorageFactory->>AuthMgr: getCredentials()
StorageFactory->>Storage: init API storage (merge creds)
else
StorageFactory->>Storage: init File storage
end
else
StorageFactory->>Storage: init File storage
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 57
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (15)
package.json (1)
13-35: Add missing test:unit, test:integration, and test:ci scripts to package.jsonpackage.json (lines 13–35) is still missing definitions for these scripts. Please insert the following under
"test:coverage"without altering existing flows:"scripts": { … "test:coverage": "node --experimental-vm-modules node_modules/.bin/jest --coverage", + "test:unit": "node --experimental-vm-modules node_modules/.bin/jest --testPathPattern '(\\.unit\\.(test|spec)\\.[jt]sx?$|/__tests__/unit/)'", + "test:integration": "node --experimental-vm-modules node_modules/.bin/jest --testPathPattern '(\\.(int|integ)\\.(test|spec)\\.[jt]sx?$|/__tests__/integration/)'", "test:e2e": "./tests/e2e/run_e2e.sh", "test:e2e-report": "./tests/e2e/run_e2e.sh --analyze-log", + "test:ci": "node --experimental-vm-modules node_modules/.bin/jest --ci --coverage", … },tests/unit/prompt-manager.test.js (4)
75-92: Same issue: make this test async and await the callAligns with the change above.
- it('should handle missing variables with empty string', () => { + it('should handle missing variables with empty string', async () => { ... - const result = promptManager.loadPrompt('test-prompt', { name: 'John' }); + const result = await promptManager.loadPrompt('test-prompt', { name: 'John' });
114-133: Async misuse in array-variables testMake the test async and await loadPrompt.
- it('should handle array variables', () => { + it('should handle array variables', async () => { ... - const result = promptManager.loadPrompt('array-prompt', { + const result = await promptManager.loadPrompt('array-prompt', { items: ['one', 'two', 'three'] });
135-156: Async misuse in conditional-blocks testMake the test async and await both calls.
- it('should handle conditional blocks', () => { + it('should handle conditional blocks', async () => { ... - const withData = promptManager.loadPrompt('conditional-prompt', { + const withData = await promptManager.loadPrompt('conditional-prompt', { hasData: true }); ... - const withoutData = promptManager.loadPrompt('conditional-prompt', { + const withoutData = await promptManager.loadPrompt('conditional-prompt', { hasData: false });
35-38: Clear mocks between testsEnsure isolation per testing guidelines.
beforeEach(() => { promptManager = new PromptManager(); }); + + afterEach(() => { + jest.clearAllMocks(); + });packages/tm-core/package.json (1)
8-13: Incorrect exports mapping: types points at TS source; “require” targets ESM.
- "exports.types" should reference the built d.ts, not "./src/index.ts".
- With "type":"module" and no CJS build, the "require" condition should be removed or point to a CJS file.
Apply this minimal fix:
"exports": { ".": { - "types": "./src/index.ts", - "import": "./dist/index.js", - "require": "./dist/index.js" + "types": "./dist/index.d.ts", + "import": "./dist/index.js" } }If you intend to support CommonJS consumers, emit CJS and point "require" to it.
packages/tm-core/src/interfaces/configuration.interface.ts (1)
86-87: Do not persist tokens in config; rely on the credential store.Storing
apiAccessTokenin IConfiguration risks writing secrets to disk. Prefer retrieving credentials via the auth subsystem at runtime and remove/deprecate this field.Proposed change (conceptual):
- Remove
apiAccessTokenfrom StorageSettings (or mark @deprecated and ignore when saving).- Ensure ConfigManager redacts or drops this field on save.
packages/tm-core/src/config/config-manager.ts (1)
188-190: Update isUsingApiStorage to account for ‘auto’.With ‘auto’, this method will incorrectly return false even when API will be selected. Consider the config hint above.
- isUsingApiStorage(): boolean { - return this.getStorageConfig().type === 'api'; - } + isUsingApiStorage(): boolean { + const s = this.getStorageConfig(); + return s.type === 'api' || (s.type === 'auto' && s.apiConfigured); + }packages/tm-core/src/services/task-service.ts (3)
218-254: getNextTask builds completedIds from a pre-filtered list — dependencies never resolve.You filter to pending/in-progress before computing completedIds, so the set is always empty.
Outside selected lines, update as follows:
// Fetch all tasks first const all = await this.getTaskList({ tag }); // Use all tasks for completed set const completedIds = new Set(all.tasks.filter(t => t.status === 'done').map(t => t.id)); // Now derive available tasks from all.tasks const availableTasks = all.tasks.filter(task => { if (task.status !== 'pending' && task.status !== 'in-progress') return false; if (!task.dependencies?.length) return true; return task.dependencies.every(depId => completedIds.has(depId.toString())); });
309-316: Defensive checks to avoid potential undefined string access in search filter.If any of title/description/details could be missing, add safe fallbacks.
Outside selected lines:
const inTitle = (task.title ?? '').toLowerCase().includes(searchLower); const inDescription = (task.description ?? '').toLowerCase().includes(searchLower); const inDetails = (task.details ?? '').toLowerCase().includes(searchLower);
323-326: Guard subtasks length access.Avoid potential undefined access.
Outside selected lines:
const hasSubtasks = (task.subtasks?.length ?? 0) > 0;packages/tm-core/src/storage/storage-factory.ts (3)
153-161: detectOptimalStorage ignores 'auto' — align naming or behavior.Either document this as a helper for api/file only or expand to consider AuthManager for parity with create().
166-199: validateStorageConfig: add support for 'auto'.Currently flags 'auto' as unknown.
Apply this diff:
switch (storageType) { case 'api': if (!config.storage?.apiEndpoint) { errors.push('API endpoint is required for API storage'); } if (!config.storage?.apiAccessToken) { errors.push('API access token is required for API storage'); } break; case 'file': // File storage doesn't require additional config break; + case 'auto': + // Auto mode is valid; no extra fields required. + break; default: errors.push(`Unknown storage type: ${storageType}`); }
121-147: Fix API storage projectId usage
createApiStorage readsprojectIdfromconfig.projectPath, butStorageFactory.createpassesprojectPathas a separate argument and never setsconfig.projectPath. As a result,projectIdwill beundefinedwhen no override is provided. ModifycreateApiStorageto accept aprojectPath: stringparameter (in addition toconfig), passprojectPathfrom thecreatemethod, and use that value instead ofconfig.projectPath.packages/tm-core/src/logger/factory.ts (1)
1-60: Solid logger factory; minimal, predictable behavior.LGTM. One minor nit: document that subsequent getLogger(name, config) calls ignore config once cached.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpackages/tm-core/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (28)
apps/cli/src/commands/auth.command.ts(1 hunks)apps/cli/src/index.ts(1 hunks)package.json(1 hunks)packages/tm-core/package.json(1 hunks)packages/tm-core/src/auth/auth-manager.ts(1 hunks)packages/tm-core/src/auth/config.ts(1 hunks)packages/tm-core/src/auth/credential-store.ts(1 hunks)packages/tm-core/src/auth/index.ts(1 hunks)packages/tm-core/src/auth/oauth-service.ts(1 hunks)packages/tm-core/src/auth/templates.ts(1 hunks)packages/tm-core/src/auth/types.ts(1 hunks)packages/tm-core/src/clients/index.ts(1 hunks)packages/tm-core/src/clients/supabase-client.ts(1 hunks)packages/tm-core/src/config/config-manager.ts(1 hunks)packages/tm-core/src/index.ts(1 hunks)packages/tm-core/src/interfaces/configuration.interface.ts(1 hunks)packages/tm-core/src/logger/factory.ts(1 hunks)packages/tm-core/src/logger/index.ts(1 hunks)packages/tm-core/src/logger/logger.ts(1 hunks)packages/tm-core/src/services/task-service.ts(3 hunks)packages/tm-core/src/storage/storage-factory.ts(3 hunks)packages/tm-core/src/task-master-core.ts(1 hunks)packages/tm-core/tsup.config.ts(2 hunks)scripts/modules/commands.js(2 hunks)tests/integration/profiles/roo-files-inclusion.test.js(1 hunks)tests/integration/profiles/rules-files-inclusion.test.js(1 hunks)tests/unit/prompt-manager.test.js(9 hunks)tsup.config.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (16)
tests/{unit,integration,e2e,fixtures}/**/*.js
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Test files must be organized as follows: unit tests in tests/unit/, integration tests in tests/integration/, end-to-end tests in tests/e2e/, and test fixtures in tests/fixtures/.
Files:
tests/integration/profiles/roo-files-inclusion.test.jstests/integration/profiles/rules-files-inclusion.test.jstests/unit/prompt-manager.test.js
**/*.{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:
tests/integration/profiles/roo-files-inclusion.test.jstests/integration/profiles/rules-files-inclusion.test.jstests/unit/prompt-manager.test.js
**/*.test.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
**/*.test.js: Never use asynchronous operations in tests. Make all mocks return synchronous values when possible.
Always mock tests properly based on the way the tested functions are defined and used.
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.
Use fixtures from tests/fixtures/ for consistent sample data across tests.
Always declare mocks before importing the modules being tested in Jest test files.
Use jest.spyOn() after imports to create spies on mock functions and reference these spies in test assertions.
When testing functions with callbacks, get the callback from your mock's call arguments, execute it directly with test inputs, and verify the results.
For ES modules, use jest.mock() before static imports and jest.unstable_mockModule() before dynamic imports to mock dependencies.
Reset mock functions (mockFn.mockReset()) before dynamic imports if they might have been called previously.
When verifying console assertions, assert against the actual arguments passed (single formatted string), not multiple arguments.
Use mock-fs to mock file system operations in tests, and restore the file system after each test.
Mock API calls (e.g., Anthropic/Claude) by mocking the entire module and providing predictable responses.
Set mock environment variables in test setup and restore them after each test.
Maintain test fixtures separate from test logic.
Follow the mock-first-then-import pattern for all Jest mocks.
Do not define mock variables before jest.mock() calls (they won't be accessible due to hoisting).
Use test-specific file paths (e.g., 'test-tasks.json') for all file operations in tests.
Mock readJSON and writeJSON to avoid real file system interactions in tests.
Verify file operations use the correct paths in expect statements.
Use different file paths for each test to avoid test interdependence.
Verify modifications on the in-memory task objects passed to w...
Files:
tests/integration/profiles/roo-files-inclusion.test.jstests/integration/profiles/rules-files-inclusion.test.jstests/unit/prompt-manager.test.js
tests/integration/**/*.test.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
Integration tests must be located in tests/integration/, test interactions between modules, and focus on component interfaces rather than implementation details.
Files:
tests/integration/profiles/roo-files-inclusion.test.jstests/integration/profiles/rules-files-inclusion.test.js
tests/{unit,integration,e2e}/**/*.test.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
tests/{unit,integration,e2e}/**/*.test.js: When testing CLI commands built with Commander.js, test the command action handlers directly rather than trying to mock the entire Commander.js chain.
When mocking the Commander.js chain, mock ALL chainable methods (option, argument, action, on, etc.) and return this (or the mock object) from all chainable method mocks.
Explicitly handle all options, including defaults and shorthand flags (e.g., -p for --prompt), and include null/undefined checks in test implementations for parameters that might be optional.
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.
Mock the action handlers for CLI commands and verify they're called with correct arguments.
Use sample task fixtures for consistent test data, mock file system operations, and test both success and error paths for task operations.
Mock console output and verify correct formatting in UI function tests. Use flexible assertions like toContain() or toMatch() for formatted output.
Mock chalk functions to return the input text to make testing easier while still verifying correct function calls.
Files:
tests/integration/profiles/roo-files-inclusion.test.jstests/integration/profiles/rules-files-inclusion.test.jstests/unit/prompt-manager.test.js
tests/{integration,e2e}/**/*.test.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
Properly mock session objects when required by functions, and reset environment variables between tests if modified.
Files:
tests/integration/profiles/roo-files-inclusion.test.jstests/integration/profiles/rules-files-inclusion.test.js
**/*.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
**/*.js: Declare and initialize global variables at the top of modules to avoid hoisting issues.
Use proper function declarations to avoid hoisting issues and initialize variables before they are referenced.
Do not reference variables before their declaration in module scope.
Use dynamic imports (import()) to avoid initialization order issues in modules.
Files:
tests/integration/profiles/roo-files-inclusion.test.jstests/integration/profiles/rules-files-inclusion.test.jsscripts/modules/commands.jstests/unit/prompt-manager.test.js
**/*.{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:
tests/integration/profiles/roo-files-inclusion.test.jstests/integration/profiles/rules-files-inclusion.test.jstests/unit/prompt-manager.test.js
tests/{unit,integration,e2e}/**
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
Organize test directories by test type (unit, integration, e2e) and mirror source structure where possible
Files:
tests/integration/profiles/roo-files-inclusion.test.jstests/integration/profiles/rules-files-inclusion.test.jstests/unit/prompt-manager.test.js
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
scripts/modules/commands.js
📄 CodeRabbit inference engine (.cursor/rules/ai_services.mdc)
scripts/modules/commands.js: Centralize all LLM calls throughgenerateTextServiceorgenerateObjectService.
Do not import or call anything from the oldai-services.js,ai-client-factory.js, orai-client-utils.jsfiles.
Do not fetch AI-specific parameters (model ID, max tokens, temp) usingconfig-manager.jsgetters for the AI call. Pass theroleinstead.
Do not implement fallback or retry logic outsideai-services-unified.js.
Do not handle API key resolution outside the service layer (it usesutils.jsinternally).
Determine the appropriaterole(main,research,fallback) in your core logic and pass it to the service.
Pass thesessionobject (received in thecontextparameter, especially from direct function wrappers) to the service call when in MCP context.
UsegenerateTextServiceand implement robust manual JSON parsing (with Zod validation after parsing) when structured output is needed, asgenerateObjectServicehas shown unreliability with some providers/schemas.
Be aware of potential reliability issues withgenerateObjectServiceacross different providers and complex schemas. PrefergenerateTextService+ manual parsing as a more robust alternative for structured data needs.
scripts/modules/commands.js: All new user-facing commands should be added to 'scripts/modules/commands.js'.
Use consistent patterns for option naming and help text in CLI commands.
Follow the Commander.js model for subcommand structure in CLI commands.
When using callbacks (like in Commander.js commands), define them separately to allow testing the callback logic independently.
Add help text to the command definition and update 'dev_workflow.mdc' with command reference when adding a new feature.
Follow the established pattern in 'commands.js' for CLI command implementation, using Commander.js for argument parsing, including comprehensive help text and examples, and supporting tagged task context awareness.
Provide clear error messages for common failu...
Files:
scripts/modules/commands.js
scripts/modules/*.js
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Each module in scripts/modules/ should be focused on a single responsibility, following the modular architecture (e.g., commands.js for CLI command handling, task-manager.js for task data and core logic, dependency-manager.js for dependency management, ui.js for CLI output formatting, ai-services-unified.js for AI service integration, config-manager.js for configuration management, utils.js for utility functions).
scripts/modules/*.js: Export all core functions, helper functions, and utility methods needed by your new function or command from their respective modules. Explicitly review the module's export block to ensure every required dependency is included.
Pass all required parameters to functions you call within your implementation and verify that direct function parameters match their core function counterparts.
Use consistent file naming conventions: 'task_${id.toString().padStart(3, '0')}.txt', use path.join for composing file paths, and use appropriate file extensions (.txt for tasks, .json for data).
Use structured error objects with code and message properties, include clear error messages, and handle both function-specific and file system errors.
Import all silent mode utilities together from 'scripts/modules/utils.js' and always use isSilentMode() to check global silent mode status. Wrap core function calls within direct functions using enableSilentMode() and disableSilentMode() in a try/finally block if the core function might produce console output.
Core functions should check outputFormat === 'text' before displaying UI elements and use internal logging that respects silent mode.
Design functions to accept dependencies as parameters (dependency injection) and avoid hard-coded dependencies that are difficult to mock.
Keep pure logic separate from I/O operations or UI rendering to allow testing the logic without mocking complex dependencies.
When implementing core logic for new features, do so in 'scripts/modules/' before CLI or MCP interfaces, and d...
Files:
scripts/modules/commands.js
scripts/modules/**
📄 CodeRabbit inference engine (.cursor/rules/dev_workflow.mdc)
When using the MCP server, restart it if core logic in
scripts/modulesor MCP tool/direct function definitions change.
Files:
scripts/modules/commands.js
scripts/modules/*
📄 CodeRabbit inference engine (.cursor/rules/tags.mdc)
scripts/modules/*: Every command that reads or writes tasks.json must be tag-aware
All command files must import getCurrentTag from utils.js
Every CLI command that operates on tasks must include the --tag CLI option
All commands must resolve the tag using the pattern: options.tag || getCurrentTag(projectRoot) || 'master'
All commands must find projectRoot with error handling before proceeding
All commands must pass { projectRoot, tag } as context to core functions
MCP direct functions must accept and use a context object containing projectRoot and tag, and pass them to core functions
Do not hard-code tag resolution (e.g., const tag = options.tag || 'master';); always use getCurrentTag
Do not omit the --tag CLI option in commands that operate on tasks
Do not omit the context parameter when calling core functions from commands
Do not call readJSON or writeJSON without passing projectRoot and tag
Files:
scripts/modules/commands.js
tests/unit/*.js
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Each module should have a corresponding unit test file in tests/unit/ that reflects the module structure (one test file per module).
Files:
tests/unit/prompt-manager.test.js
tests/unit/**/*.test.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
tests/unit/**/*.test.js: Unit tests must be located in tests/unit/, test individual functions and utilities in isolation, mock all external dependencies, and keep tests small, focused, and fast.
Do not include actual command execution in unit tests.
Files:
tests/unit/prompt-manager.test.js
🧠 Learnings (27)
📚 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 : Verify file operations use the correct paths in expect statements.
Applied to files:
tests/integration/profiles/roo-files-inclusion.test.jstests/integration/profiles/rules-files-inclusion.test.jstests/unit/prompt-manager.test.js
📚 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:
tests/integration/profiles/roo-files-inclusion.test.jstests/integration/profiles/rules-files-inclusion.test.js
📚 Learning: 2025-08-12T01:22:48.873Z
Learnt from: olssonsten
PR: eyaltoledano/claude-task-master#1112
File: src/profiles/roo.js:0-0
Timestamp: 2025-08-12T01:22:48.873Z
Learning: In src/profiles/roo.js, the enhanceRooMCPConfiguration function was simplified to only set server.timeout = 300, removing problematic overwrites of server.env and alwaysAllow configurations that could interfere with user settings.
Applied to files:
tests/integration/profiles/roo-files-inclusion.test.js
📚 Learning: 2025-07-18T17:12:57.903Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-07-18T17:12:57.903Z
Learning: Applies to scripts/modules/**/*.test.js : Test core logic independently with both data formats, mock file system operations, test tag resolution behavior, and verify migration compatibility in unit tests.
Applied to files:
tests/integration/profiles/roo-files-inclusion.test.jstests/integration/profiles/rules-files-inclusion.test.js
📚 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 : Mock console output and verify correct formatting in UI function tests. Use flexible assertions like toContain() or toMatch() for formatted output.
Applied to files:
tests/integration/profiles/roo-files-inclusion.test.jstests/unit/prompt-manager.test.js
📚 Learning: 2025-07-18T17:07:39.336Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/architecture.mdc:0-0
Timestamp: 2025-07-18T17:07:39.336Z
Learning: Applies to tests/{unit,integration,e2e,fixtures}/**/*.js : Test files must be organized as follows: unit tests in tests/unit/, integration tests in tests/integration/, end-to-end tests in tests/e2e/, and test fixtures in tests/fixtures/.
Applied to files:
tests/integration/profiles/roo-files-inclusion.test.jstests/integration/profiles/rules-files-inclusion.test.js
📚 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 : Use different file paths for each test to avoid test interdependence.
Applied to files:
tests/integration/profiles/roo-files-inclusion.test.jstests/integration/profiles/rules-files-inclusion.test.js
📚 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 : Verify modifications on the in-memory task objects passed to writeJSON.
Applied to files:
tests/integration/profiles/roo-files-inclusion.test.js
📚 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 : Maintain test fixtures separate from test logic.
Applied to files:
tests/integration/profiles/roo-files-inclusion.test.js
📚 Learning: 2025-08-03T12:13:33.875Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-08-03T12:13:33.875Z
Learning: Applies to {tests/setup.ts,tests/setup/integration.ts,tests/teardown.ts} : Test setup files should be created at tests/setup.ts, tests/setup/integration.ts, and tests/teardown.ts
Applied to files:
tests/integration/profiles/roo-files-inclusion.test.js
📚 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:
apps/cli/src/index.tsscripts/modules/commands.js
📚 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 : Place utilities used primarily by the core `task-master` CLI logic and command modules (`scripts/modules/*`) into `scripts/modules/utils.js`.
Applied to files:
apps/cli/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/commands.js : All new user-facing commands should be added to 'scripts/modules/commands.js'.
Applied to files:
apps/cli/src/index.tsscripts/modules/commands.js
📚 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 : Follow the established pattern in 'commands.js' for CLI command implementation, using Commander.js for argument parsing, including comprehensive help text and examples, and supporting tagged task context awareness.
Applied to files:
apps/cli/src/index.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: Guidelines for implementing CLI commands using Commander.js (commands.mdc).
Applied to files:
apps/cli/src/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 : Follow the provided command template structure for all CLI commands using Commander.js, including descriptive command names, option definitions, and async action handlers.
Applied to files:
apps/cli/src/index.ts
📚 Learning: 2025-07-18T17:19:27.365Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/.windsurfrules:0-0
Timestamp: 2025-07-18T17:19:27.365Z
Learning: Use the global `task-master` CLI command instead of directly invoking `node scripts/dev.js` for all task management operations.
Applied to files:
apps/cli/src/index.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 scripts/modules/*.js : Each module in scripts/modules/ should be focused on a single responsibility, following the modular architecture (e.g., commands.js for CLI command handling, task-manager.js for task data and core logic, dependency-manager.js for dependency management, ui.js for CLI output formatting, ai-services-unified.js for AI service integration, config-manager.js for configuration management, utils.js for utility functions).
Applied to files:
apps/cli/src/index.ts
📚 Learning: 2025-07-18T17:19:27.365Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/.windsurfrules:0-0
Timestamp: 2025-07-18T17:19:27.365Z
Learning: Applies to assets/**/.windsurfrules : Update Windsurf rules when new patterns emerge, add examples from the actual codebase, remove outdated patterns, and cross-reference related rules.
Applied to files:
tests/integration/profiles/rules-files-inclusion.test.js
📚 Learning: 2025-07-18T17:10:12.881Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dev_workflow.mdc:0-0
Timestamp: 2025-07-18T17:10:12.881Z
Learning: Applies to tasks.json : Use the `tasks.json` file (generated by Taskmaster) to store the project's task list, including tags and task structures.
Applied to files:
packages/tm-core/src/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 : Provide functions for updating task status within the current tag context, handling both individual tasks and subtasks, and considering subtask status when updating parent tasks.
Applied to files:
packages/tm-core/src/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 : Allow filtering tasks by status within the current tag context, handle subtask display in lists, and use consistent table formats.
Applied to files:
packages/tm-core/src/services/task-service.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:
scripts/modules/commands.js
📚 Learning: 2025-07-18T17:19:27.365Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/.windsurfrules:0-0
Timestamp: 2025-07-18T17:19:27.365Z
Learning: Start new projects by running `task-master init` or `node scripts/dev.js parse-prd --input=<prd-file.txt>` to generate the initial tasks.json.
Applied to files:
scripts/modules/commands.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : 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:
scripts/modules/commands.js
📚 Learning: 2025-07-20T01:35:05.831Z
Learnt from: rtmcrc
PR: eyaltoledano/claude-task-master#933
File: scripts/modules/task-manager/parse-prd.js:226-226
Timestamp: 2025-07-20T01:35:05.831Z
Learning: The parsePRD function in scripts/modules/task-manager/parse-prd.js has a different parameter structure than other task-manager functions - it uses `options` parameter instead of `context` parameter because it generates tasks from PRD documents rather than operating on existing tasks.
Applied to files:
scripts/modules/commands.js
📚 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 : Explicitly handle all options, including defaults and shorthand flags (e.g., -p for --prompt), and include null/undefined checks in test implementations for parameters that might be optional.
Applied to files:
tests/unit/prompt-manager.test.js
🧬 Code graph analysis (13)
packages/tm-core/src/auth/config.ts (1)
packages/tm-core/src/auth/types.ts (1)
AuthConfig(44-49)
packages/tm-core/src/logger/factory.ts (2)
packages/tm-core/src/logger/logger.ts (2)
Logger(25-242)LoggerConfig(15-23)packages/tm-core/src/logger/index.ts (5)
Logger(6-6)createLogger(8-8)LoggerConfig(7-7)getLogger(8-8)setGlobalLogger(8-8)
packages/tm-core/src/auth/templates.ts (1)
packages/tm-core/src/auth/index.ts (3)
getSuccessHtml(21-21)getErrorHtml(22-22)getSecurityErrorHtml(23-23)
packages/tm-core/src/auth/auth-manager.ts (4)
packages/tm-core/src/auth/credential-store.ts (1)
CredentialStore(10-121)packages/tm-core/src/auth/oauth-service.ts (1)
OAuthService(23-336)packages/tm-core/src/clients/supabase-client.ts (1)
SupabaseAuthClient(9-154)packages/tm-core/src/auth/types.ts (4)
AuthConfig(44-49)AuthCredentials(5-13)OAuthFlowOptions(29-42)AuthenticationError(65-73)
packages/tm-core/src/auth/oauth-service.ts (6)
packages/tm-core/src/logger/factory.ts (1)
getLogger(23-44)packages/tm-core/src/auth/credential-store.ts (1)
CredentialStore(10-121)packages/tm-core/src/clients/supabase-client.ts (1)
SupabaseAuthClient(9-154)packages/tm-core/src/auth/types.ts (5)
AuthConfig(44-49)OAuthFlowOptions(29-42)AuthCredentials(5-13)AuthenticationError(65-73)CliData(51-60)packages/tm-core/src/auth/config.ts (1)
getAuthConfig(36-41)packages/tm-core/src/auth/auth-manager.ts (1)
refreshToken(86-116)
packages/tm-core/src/auth/types.ts (2)
packages/tm-core/src/auth/index.ts (7)
AuthCredentials(10-10)AuthOptions(11-11)AuthResponse(12-12)OAuthFlowOptions(13-13)AuthenticationError(18-18)AuthConfig(14-14)CliData(15-15)packages/tm-core/src/index.ts (6)
AuthCredentials(51-51)AuthOptions(52-52)AuthResponse(53-53)OAuthFlowOptions(54-54)AuthenticationError(50-50)AuthConfig(55-55)
packages/tm-core/src/auth/credential-store.ts (3)
packages/tm-core/src/logger/factory.ts (1)
getLogger(23-44)packages/tm-core/src/auth/types.ts (3)
AuthConfig(44-49)AuthCredentials(5-13)AuthenticationError(65-73)packages/tm-core/src/auth/config.ts (1)
getAuthConfig(36-41)
packages/tm-core/src/clients/supabase-client.ts (4)
packages/tm-core/src/clients/index.ts (1)
SupabaseAuthClient(5-5)packages/tm-core/src/auth/index.ts (1)
AuthenticationError(18-18)apps/cli/src/commands/auth.command.ts (1)
refreshToken(260-296)packages/tm-core/src/auth/auth-manager.ts (1)
refreshToken(86-116)
packages/tm-core/src/logger/logger.ts (1)
packages/tm-core/src/logger/index.ts (3)
LogLevel(6-6)LoggerConfig(7-7)Logger(6-6)
apps/cli/src/commands/auth.command.ts (2)
packages/tm-core/src/auth/types.ts (2)
AuthCredentials(5-13)AuthenticationError(65-73)packages/tm-core/src/auth/auth-manager.ts (1)
AuthManager(18-149)
scripts/modules/commands.js (1)
apps/cli/src/commands/auth.command.ts (1)
AuthCommand(31-510)
tests/unit/prompt-manager.test.js (4)
tests/manual/prompts/prompt-test.js (5)
result(1122-1126)result(1305-1309)result(1410-1414)promptManager(28-28)promptManager(1408-1408)scripts/modules/prompt-manager.js (1)
promptManager(523-523)scripts/modules/task-manager/add-task.js (2)
promptManager(417-417)promptManager(418-436)scripts/modules/task-manager/research.js (2)
promptManager(197-197)promptManager(211-214)
packages/tm-core/src/storage/storage-factory.ts (2)
packages/tm-core/src/logger/factory.ts (1)
getLogger(23-44)packages/tm-core/src/auth/auth-manager.ts (1)
AuthManager(18-149)
🪛 Gitleaks (8.27.2)
packages/tm-core/src/clients/supabase-client.ts
22-22: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🔇 Additional comments (31)
tests/integration/profiles/rules-files-inclusion.test.js (2)
92-95: LGTM: formatting-only change; path and expectation remain correct.Using fs.existsSync with path.join(process.cwd(), ...) aligns with our sync-test guideline.
97-100: LGTM: same here; no semantic change.Assertion and target path are correct.
tests/integration/profiles/roo-files-inclusion.test.js (2)
106-109: LGTM: readability tweak only.The path and sync exists check remain correct for integration tests.
111-114: LGTM: formatting-only; assertion unchanged.No behavior impact.
packages/tm-core/tsup.config.ts (2)
2-4: Good: load monorepo envs early.dotenv-mono here is appropriate for build-time. Ensure the package keeps dotenv-mono as a devDependency (it is in tm-core).
37-43: Externalization regex approved – no@tm/*imports found inpackages/tm-core/src, only in test files, so bundling won’t externalize any internal aliases.tsup.config.ts (2)
2-6: Load envs at build-time for CLI — OK.Matches the tm-core approach; keep dotenv-mono as a devDependency at repo root (see package.json comment).
50-50: Nice DX touch.The onSuccess message is harmless and helps in CI logs.
tests/unit/prompt-manager.test.js (1)
12-14: Remove FS‐mocking suggestion: PromptManager uses static JSON imports, not runtime file reads
The unit tests import JSON modules at load time and don’t perform any dynamic FS operations, so neither moving this to integration nor mocking the filesystem is needed.Likely an incorrect or invalid review comment.
packages/tm-core/src/clients/supabase-client.ts (1)
140-153: Revise signOut signature and implementation
In packages/tm-core/src/clients/supabase-client.ts, change signOut to accept accessToken and refreshToken, call client.auth.setSession({access_token, refresh_token}) then client.auth.signOut({scope:'global'}) instead of admin.signOut(token). No internal call sites detected—manually verify and update all external callers to pass both tokens.packages/tm-core/src/logger/logger.ts (1)
56-66: Fix TM_LOG_LEVEL parsing: numeric values currently break logging (enum reverse mapping returns strings).If users set TM_LOG_LEVEL=2,
envConfig.levelbecomes'WARN'(string), making comparisons likelevel <= this.config.levelunreliable and effectively silencing logs.Apply:
-// Check for log level -if (process.env.TASK_MASTER_LOG_LEVEL || process.env.TM_LOG_LEVEL) { - const levelStr = ( - process.env.TASK_MASTER_LOG_LEVEL || - process.env.TM_LOG_LEVEL || - '' - ).toUpperCase(); - if (levelStr in LogLevel) { - envConfig.level = LogLevel[levelStr as keyof typeof LogLevel]; - } -} +// Check for log level (accept names or numbers) +{ + const rawLevel = + (process.env.TASK_MASTER_LOG_LEVEL ?? process.env.TM_LOG_LEVEL ?? '').trim(); + if (rawLevel) { + const byName = (LogLevel as any)[rawLevel.toUpperCase()]; + if (typeof byName === 'number') { + envConfig.level = byName as LogLevel; + } else { + const n = Number(rawLevel); + if ( + Number.isInteger(n) && + n >= LogLevel.SILENT && + n <= LogLevel.DEBUG + ) { + envConfig.level = n as LogLevel; + } + } + } +}⛔ Skipped due to learnings
Learnt from: CR PR: eyaltoledano/claude-task-master#0 File: .cursor/rules/dev_workflow.mdc:0-0 Timestamp: 2025-07-18T17:10:02.683Z Learning: Applies to .taskmaster/config.json : Store Taskmaster configuration settings (AI model selections, parameters, logging level, default subtasks/priority, project name, tag management) in `.taskmaster/config.json` in the project root. Do not configure these via environment variables.apps/cli/src/index.ts (1)
8-8: LGTM: AuthCommand export and registration verified
AuthCommand is implemented in apps/cli/src/commands/auth.command.ts, exported in apps/cli/src/index.ts, and registered via AuthCommand.registerOn(programInstance) in scripts/modules/commands.js.packages/tm-core/package.json (2)
38-38: Ignore relocation suggestion: dotenv-mono is imported inpackages/tm-core/tsup.config.ts(line 2), so it belongs as a devDependency here.Likely an incorrect or invalid review comment.
29-31: Keep chalk in tm-core
Chalk is directly imported in packages/tm-core/src/logger/logger.ts, so it’s a required core dependency.Likely an incorrect or invalid review comment.
packages/tm-core/src/clients/index.ts (1)
1-6: Publicly exporting SupabaseAuthClient—confirm API stability.If this class is internal, re-export an interface or factory instead to avoid locking the class into semver. Otherwise, add an “experimental” note in docs.
packages/tm-core/src/task-master-core.ts (1)
155-155: Return type widened to include ‘auto’
Update the method docs/CLI help to mention that ‘auto’ may be returned, and audit all callers to ensure they correctly handle the new ‘auto’ case.packages/tm-core/src/index.ts (1)
58-59: Re-export of logger is justified
Logger imports chalk at packages/tm-core/src/logger/logger.ts:5, so retaining styling in tm-core and re-exporting here is appropriate—no changes required.packages/tm-core/src/auth/config.ts (3)
21-31: Central defaults look good; tiny consistency tweak optional.If you adopt the BASE_DOMAIN runtime envs above, this block will correctly reflect them. No functional issues spotted.
33-41: LGTM: Simple, predictable override merging.
getAuthConfigcleanly merges overrides atopDEFAULT_AUTH_CONFIG.
9-16: Tsup env injection covers TM_PUBLIC_BASE_DOMAIN
Thetsup.config.ts’senv: getBuildTimeEnvs()automatically includes allTM_PUBLIC_*variables (includingTM_PUBLIC_BASE_DOMAIN), so build-time replacement is correctly configured.scripts/modules/commands.js (2)
18-20: Import wiring for CLI commands looks correct.Importing
ListTasksCommandandAuthCommandfrom@tm/climatches the re-export pattern and keeps this module thin.
1744-1747: Add auth command to dev_workflow and unify CLI name
- Include the new
authcommand and its subcommands (e.g.login) in.cursor/rules/dev_workflow.mdc.- Docs reference both
tm auth loginandtask-master auth login, butsetupCLI()registers the binary asdev. Standardize examples on one invocation or add aliases fortask-master/tm→dev.packages/tm-core/src/logger/index.ts (1)
1-8: LGTM: Public logger surface is clean and minimal.Re-exports are ESM-friendly (
.jsspecifiers) and keep types separate withexport type.packages/tm-core/src/storage/storage-factory.ts (1)
224-229: Good fallback logging.Switch to logger.warn with context is an improvement.
packages/tm-core/src/auth/index.ts (1)
5-29: Public surface aggregator looks goodThe exports are coherent and bounded to auth concerns. No runtime logic; tree-shakeable.
packages/tm-core/src/auth/credential-store.ts (1)
60-78: Windows fs.chmodSync limited support
On Windows, fs.chmodSync only toggles the file’s read/write attribute (owner write) and does not enforce POSIX ACLs; the enclosing try/catch prevents crashes, but don’t rely on chmodSync for security on Windows.packages/tm-core/src/auth/oauth-service.ts (1)
307-308: No action needed: the project’senginesfield enforces Node ≥ 18, which already supportstoString('base64url').apps/cli/src/commands/auth.command.ts (4)
147-150: Remove process.exit() from status flowReturn control on error; let the outer runner decide exit behavior.
- } catch (error: any) { - this.handleError(error); - process.exit(1); - } + } catch (error: any) { + this.handleError(error); + return; + }⛔ Skipped due to learnings
Learnt from: joedanz PR: eyaltoledano/claude-task-master#748 File: scripts/modules/task-manager/parse-prd.js:726-733 Timestamp: 2025-07-21T17:51:07.239Z Learning: In CLI contexts within task-manager modules like scripts/modules/task-manager/parse-prd.js, using process.exit(1) for validation failures and error conditions is correct and preferred over throwing errors, as it provides immediate termination with appropriate exit codes for scripting. The code should distinguish between MCP contexts (throw errors) and CLI contexts (use process.exit).
131-137: Remove process.exit() from logout flowSame rationale as above; avoid terminating the process from within a command handler.
- if (!result.success) { - process.exit(1); - } + if (!result.success) { + return; + }⛔ Skipped due to learnings
Learnt from: joedanz PR: eyaltoledano/claude-task-master#748 File: scripts/modules/task-manager/parse-prd.js:726-733 Timestamp: 2025-07-21T17:51:07.239Z Learning: In CLI contexts within task-manager modules like scripts/modules/task-manager/parse-prd.js, using process.exit(1) for validation failures and error conditions is correct and preferred over throwing errors, as it provides immediate termination with appropriate exit codes for scripting. The code should distinguish between MCP contexts (throw errors) and CLI contexts (use process.exit).
108-117: Do not call process.exit() inside library command handlersDirect exits make the command non-embeddable and hard to test. Return control to the CLI entrypoint and let it decide exit codes.
- if (!result.success) { - process.exit(1); - } - - // Exit cleanly after successful authentication - // Small delay to ensure all output is flushed - setTimeout(() => { - process.exit(0); - }, 100); + if (!result.success) { + // let the caller decide how to exit + return; + }Also applies to: 117-120
⛔ Skipped due to learnings
Learnt from: joedanz PR: eyaltoledano/claude-task-master#748 File: scripts/modules/task-manager/parse-prd.js:726-733 Timestamp: 2025-07-21T17:51:07.239Z Learning: In CLI contexts within task-manager modules like scripts/modules/task-manager/parse-prd.js, using process.exit(1) for validation failures and error conditions is correct and preferred over throwing errors, as it provides immediate termination with appropriate exit codes for scripting. The code should distinguish between MCP contexts (throw errors) and CLI contexts (use process.exit).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 : Check that required parameters are provided, provide clear error messages when parameters are missing, and use early returns with process.exit(1) for validation failures.Learnt from: CR PR: eyaltoledano/claude-task-master#0 File: .cursor/rules/commands.mdc:0-0 Timestamp: 2025-07-18T17:08:48.695Z Learning: Applies to scripts/modules/commands.js : Check that required parameters are provided, provide clear error messages when parameters are missing, and use early returns with process.exit(1) for validation failures.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 : Provide clear error messages for common failures, handle tagged system migration errors gracefully, include suggestions for resolution, and exit with appropriate codes for scripting in CLI commands.
161-167: Remove process.exit() from refresh flowSame issue as other subcommands.
- if (!result.success) { - process.exit(1); - } + if (!result.success) { + return; + } @@ - } catch (error: any) { - this.handleError(error); - process.exit(1); - } + } catch (error: any) { + this.handleError(error); + return; + }⛔ Skipped due to learnings
Learnt from: joedanz PR: eyaltoledano/claude-task-master#748 File: scripts/modules/task-manager/parse-prd.js:726-733 Timestamp: 2025-07-21T17:51:07.239Z Learning: In CLI contexts within task-manager modules like scripts/modules/task-manager/parse-prd.js, using process.exit(1) for validation failures and error conditions is correct and preferred over throwing errors, as it provides immediate termination with appropriate exit codes for scripting. The code should distinguish between MCP contexts (throw errors) and CLI contexts (use process.exit).
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (10)
packages/tm-core/src/auth/types.ts (3)
10-13: Standardize time fields to epoch milliseconds (number).Use numbers for
expiresAt/savedAtto simplify math and avoid TZ parsing; update downstream usages accordingly.export interface AuthCredentials { token: string; refreshToken?: string; userId: string; email?: string; - expiresAt?: string; + expiresAt?: number; // epoch ms tokenType?: 'standard' | 'api_key'; - savedAt: string; + savedAt: number; // epoch ms } export interface AuthResponse { token: string; refreshToken?: string; userId: string; email?: string; - expiresAt?: string; + expiresAt?: number; // epoch ms }Follow-ups (separate changes): oauth-service.ts (calc/store numbers), credential-store.ts (read/write numbers with migration), supabase-client.ts (return numbers), auth-manager.ts (use Date.now()).
Also applies to: 26-27
15-19: Mark password as sensitive.Inline reminder helps prevent accidental logging/persistence.
export interface AuthOptions { email?: string; - password?: string; + /** SENSITIVE: never log or persist in plaintext */ + password?: string; apiKey?: string; }
61-71: Strongly type error codes and capture root causes.Typed codes improve handling; keep optional
cause.+export type AuthErrorCode = + | 'URL_GENERATION_FAILED' + | 'OAUTH_FAILED' + | 'OAUTH_ERROR' + | 'AUTH_TIMEOUT' + | 'INVALID_STATE' + | 'NO_TOKEN' + | 'NO_REFRESH_TOKEN' + | 'CONFIG_MISSING' + | 'REFRESH_FAILED' + | 'INVALID_RESPONSE' + | 'SAVE_FAILED' + | 'CLEAR_FAILED' + | 'NOT_SUPPORTED' + | 'STORAGE_ERROR' + | 'NETWORK_ERROR'; + export class AuthenticationError extends Error { constructor( - message: string, - public code: string + message: string, + public code: AuthErrorCode, + public cause?: unknown ) { - super(message); + super(message, cause ? { cause } : undefined); this.name = 'AuthenticationError'; } }packages/tm-core/src/auth/credential-store.ts (2)
21-21: Allow fetching expired creds for refresh flows; migrate to numeric timestamps safely.Avoids blocking refresh when access token expired but refresh token exists.
- getCredentials(): AuthCredentials | null { + getCredentials(options?: { allowExpired?: boolean }): AuthCredentials | null { @@ - // Check if token is expired - if (authData.expiresAt && new Date(authData.expiresAt) < new Date()) { + // Normalize/migrate timestamps and check expiry + const expiresAtMs = + typeof authData.expiresAt === 'number' + ? authData.expiresAt + : authData.expiresAt + ? Date.parse(authData.expiresAt as unknown as string) + : undefined; + if ( + expiresAtMs && + expiresAtMs < Date.now() && + !options?.allowExpired + ) { this.logger.warn('Authentication token has expired'); return null; }Update callers (e.g., AuthManager.refreshToken) to use
{ allowExpired: true }.Also applies to: 31-35
52-67: Make writes atomic; set secure perms on dir/file; stamp numeric savedAt.Prevents partial files/TOCTOU and locks down credentials.
- if (!fs.existsSync(this.config.configDir)) { - fs.mkdirSync(this.config.configDir, { recursive: true }); - } + if (!fs.existsSync(this.config.configDir)) { + fs.mkdirSync(this.config.configDir, { recursive: true, mode: 0o700 }); + } + try { fs.chmodSync(this.config.configDir, 0o700); } catch {} @@ - authData.savedAt = new Date().toISOString(); + authData.savedAt = Date.now(); // epoch ms @@ - // Save credentials - fs.writeFileSync( - this.config.configFile, - JSON.stringify(authData, null, 2) - ); - - // Set file permissions to read/write for owner only - fs.chmodSync(this.config.configFile, 0o600); + // Save credentials atomically with owner-only perms + const tmp = `${this.config.configFile}.tmp-${process.pid}-${Date.now()}`; + fs.writeFileSync(tmp, JSON.stringify(authData, null, 2), { mode: 0o600 }); + fs.renameSync(tmp, this.config.configFile);Note: keep a small cleanup on errors if a temp file remains.
packages/tm-core/src/auth/oauth-service.ts (5)
58-60: Replace brittle 100ms sleep with a readiness signal.Avoid race where
authorizationUrlisn't ready yet.- // Wait for server to start - await new Promise((resolve) => setTimeout(resolve, 100)); + // Wait for server to start and URL to be ready + if (this.authorizationReady) { + await this.authorizationReady; + }Additions outside this hunk:
// class fields private authorizationReady: Promise<void> | null = null; private resolveAuthorizationReady: (() => void) | null = null;Resolve it after
this.authorizationUrlis assigned in the listen callback:this.authorizationUrl = authUrl.toString(); this.resolveAuthorizationReady?.();Initialize before starting the server:
this.authorizationReady = new Promise((res) => (this.resolveAuthorizationReady = res));
116-121: Eliminate port TOCTOU; bind first, then derive callback URL; use 127.0.0.1 consistently.Prevents races and IPv6 localhost mismatch.
- private async startFlow(timeout: number = 300000): Promise<AuthCredentials> { - const state = this.generateState(); - const port = await this.getRandomPort(); - const callbackUrl = `http://localhost:${port}/callback`; + private async startFlow(timeout: number = 300000): Promise<AuthCredentials> { + const state = this.generateState(); + this.authorizationReady = new Promise((res) => (this.resolveAuthorizationReady = res)); @@ - // Start server on localhost only - server.listen(port, '127.0.0.1', () => { - // Build authorization URL for web app sign-in page - const authUrl = new URL(`${this.baseUrl}/auth/sign-in`); + // Start server on loopback only; let OS pick port + server.listen(0, '127.0.0.1', () => { + const actualPort = (server.address() as any).port; + const callbackUrl = `http://127.0.0.1:${actualPort}/callback`; + // Build authorization URL for web app sign-in page + const authUrl = new URL(`${this.baseUrl}/auth/sign-in`); @@ - // Encode CLI data as base64 - const cliParam = Buffer.from(JSON.stringify(cliData)).toString( + // Prepare and encode CLI data as base64 + const cliData: CliData = { + callback: callbackUrl, + state, + name: 'Task Master CLI', + version: this.getCliVersion(), + device: os.hostname(), + user: os.userInfo().username, + platform: os.platform(), + timestamp: Date.now() + }; + const cliParam = Buffer.from(JSON.stringify(cliData)).toString( 'base64' ); @@ - // Store auth URL for browser opening - this.authorizationUrl = authUrl.toString(); + // Store auth URL for browser opening + this.authorizationUrl = authUrl.toString(); + this.resolveAuthorizationReady?.(); @@ - this.logger.debug('CLI data:', cliData); + const { state: _redacted, ...safeCliData } = cliData; + this.logger.debug('CLI data:', safeCliData); });Then delete
getRandomPort()and its usages.Also applies to: 160-181, 313-327
136-153: Remove ineffectiveserverClosedflag; guard withserver.listeningand clear timeout.Prevents double close and stray timers.
- let serverClosed = false; @@ - await this.handleCallback( + await this.handleCallback( url, res, server, - serverClosed, resolve, reject ); - serverClosed = true; @@ -private async handleCallback( +private async handleCallback( url: URL, res: http.ServerResponse, server: http.Server, - serverClosed: boolean, resolve: (value: AuthCredentials) => void, reject: (error: any) => void ): Promise<void> { @@ - if (error) { - if (!serverClosed) { - server.close(); + if (error) { + if (server.listening) { + server.close(); + } + if (this.authTimeout) { + clearTimeout(this.authTimeout); + this.authTimeout = null; + } reject( @@ - if (returnedState !== this.originalState) { - if (!serverClosed) { - server.close(); + if (returnedState !== this.originalState) { + if (server.listening) server.close(); + if (this.authTimeout) { + clearTimeout(this.authTimeout); + this.authTimeout = null; + } reject( @@ - if (!serverClosed) { - server.close(); + if (server.listening) server.close(); + if (this.authTimeout) { + clearTimeout(this.authTimeout); + this.authTimeout = null; + } resolve(authData); @@ - if (!serverClosed) { - server.close(); + if (server.listening) server.close(); + if (this.authTimeout) { + clearTimeout(this.authTimeout); + this.authTimeout = null; + } reject(new AuthenticationError('No access token received', 'NO_TOKEN'));Add to class:
private authTimeout: NodeJS.Timeout | null = null;Set/clear timeout in startFlow (see next comment).
Also applies to: 198-205, 219-229, 232-241, 272-287
182-191: Store timeout id and clear it on success/error.Avoid stray timer firing after the flow ends.
- // Set timeout for authentication - setTimeout(() => { + // Set timeout for authentication + this.authTimeout = setTimeout(() => { if (!serverClosed) { - serverClosed = true; server.close(); reject( new AuthenticationError('Authentication timeout', 'AUTH_TIMEOUT') ); } - }, timeout); + }, timeout);Timeout is cleared in
handleCallback(see previous comment).
176-180: Redact sensitive fields in logs.Avoid logging
state/PII.- this.logger.info( + this.logger.info( `OAuth session started - ${cliData.name} v${cliData.version} on port ${port}` ); - this.logger.debug('CLI data:', cliData); + const { state: _redacted, ...safeCliData } = cliData; + this.logger.debug('CLI data:', safeCliData);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/tm-core/src/auth/config.ts(1 hunks)packages/tm-core/src/auth/credential-store.ts(1 hunks)packages/tm-core/src/auth/index.ts(1 hunks)packages/tm-core/src/auth/oauth-service.ts(1 hunks)packages/tm-core/src/auth/types.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.899Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
📚 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/*.js : Use consistent file naming conventions: 'task_${id.toString().padStart(3, '0')}.txt', use path.join for composing file paths, and use appropriate file extensions (.txt for tasks, .json for data).
Applied to files:
packages/tm-core/src/auth/config.ts
📚 Learning: 2025-09-02T21:51:41.338Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:0-0
Timestamp: 2025-09-02T21:51:41.338Z
Learning: In packages/tm-core/src/auth/config.ts, the BASE_DOMAIN configuration intentionally does not include runtime environment variable fallbacks like TM_BASE_DOMAIN or HAMSTER_BASE_URL. The maintainers prefer to keep these capabilities "hush hush" and undocumented, using only the build-time TM_PUBLIC_BASE_DOMAIN and the default value.
Applied to files:
packages/tm-core/src/auth/config.ts
📚 Learning: 2025-09-02T21:49:28.120Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/oauth-service.ts:215-218
Timestamp: 2025-09-02T21:49:28.120Z
Learning: In the OAuth service callback handler, the callback page is not actually shown to users, so returning HTML templates (getSuccessHtml/getErrorHtml) is not necessary - a blank 200 response is sufficient.
Applied to files:
packages/tm-core/src/auth/oauth-service.ts
📚 Learning: 2025-09-02T21:51:27.899Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.899Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
Applied to files:
packages/tm-core/src/auth/oauth-service.ts
📚 Learning: 2025-08-02T15:33:22.656Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1069
File: .changeset/fix-tag-complexity-detection.md:0-0
Timestamp: 2025-08-02T15:33:22.656Z
Learning: For changeset files (.changeset/*.md), Crunchyman-ralph prefers to ignore formatting nitpicks about blank lines between frontmatter and descriptions, as he doesn't mind having them and wants to avoid such comments in future reviews.
Applied to files:
packages/tm-core/src/auth/oauth-service.ts
🧬 Code graph analysis (4)
packages/tm-core/src/auth/credential-store.ts (4)
packages/tm-core/src/logger/factory.ts (1)
getLogger(23-44)packages/tm-core/src/auth/types.ts (3)
AuthConfig(44-48)AuthCredentials(5-13)AuthenticationError(64-72)packages/tm-core/src/auth/config.ts (1)
getAuthConfig(32-37)packages/tm-core/src/logger/logger.ts (1)
error(163-166)
packages/tm-core/src/auth/types.ts (2)
packages/tm-core/src/auth/index.ts (7)
AuthCredentials(10-10)AuthOptions(11-11)AuthResponse(12-12)OAuthFlowOptions(13-13)AuthenticationError(18-18)AuthConfig(14-14)CliData(15-15)packages/tm-core/src/index.ts (6)
AuthCredentials(51-51)AuthOptions(52-52)AuthResponse(53-53)OAuthFlowOptions(54-54)AuthenticationError(50-50)AuthConfig(55-55)
packages/tm-core/src/auth/config.ts (1)
packages/tm-core/src/auth/types.ts (1)
AuthConfig(44-48)
packages/tm-core/src/auth/oauth-service.ts (6)
packages/tm-core/src/logger/factory.ts (1)
getLogger(23-44)packages/tm-core/src/auth/credential-store.ts (1)
CredentialStore(10-105)packages/tm-core/src/clients/supabase-client.ts (1)
SupabaseAuthClient(9-154)packages/tm-core/src/auth/types.ts (5)
AuthConfig(44-48)OAuthFlowOptions(29-42)AuthCredentials(5-13)AuthenticationError(64-72)CliData(50-59)packages/tm-core/src/auth/config.ts (1)
getAuthConfig(32-37)packages/tm-core/src/auth/auth-manager.ts (1)
refreshToken(86-116)
🔇 Additional comments (1)
packages/tm-core/src/auth/index.ts (1)
5-23: Re-export surface looks good.Clear, tree-shakeable exports. No issues spotted.
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/tm-core/src/config/services/environment-config-provider.service.ts (1)
73-80: Treat empty string consistently across helpersloadConfig skips empty strings with
if (!value), while hasEnvVar treats empty string as set. Align by checkingvalue === undefined.- const value = process.env[mapping.env]; - if (!value) continue; + const value = process.env[mapping.env]; + if (value === undefined) continue;packages/tm-core/src/config/config-manager.spec.ts (3)
40-45: Mocks and expectations disagree on default storage typeYou expect
{ type: 'auto', apiConfigured: false }but the mocked sources return{ type: 'file' }. Update mocks and earlier expectation to 'auto'.- getDefaultConfig: vi.fn().mockReturnValue({ - models: { main: 'default-model', fallback: 'fallback-model' }, - storage: { type: 'file' }, - version: '1.0.0' - }), + getDefaultConfig: vi.fn().mockReturnValue({ + models: { main: 'default-model', fallback: 'fallback-model' }, + storage: { type: 'auto' }, + version: '1.0.0' + }), @@ - merge: vi.fn().mockReturnValue({ - models: { main: 'merged-model', fallback: 'fallback-model' }, - storage: { type: 'file' } - }), + merge: vi.fn().mockReturnValue({ + models: { main: 'merged-model', fallback: 'fallback-model' }, + storage: { type: 'auto' } + }), @@ - expect(config).toEqual({ - models: { main: 'merged-model', fallback: 'fallback-model' }, - storage: { type: 'file' } - }); + expect(config).toEqual({ + models: { main: 'merged-model', fallback: 'fallback-model' }, + storage: { type: 'auto' } + }); @@ - const storage = manager.getStorageConfig(); - expect(storage).toEqual({ type: 'auto', apiConfigured: false }); + const storage = manager.getStorageConfig(); + expect(storage).toEqual({ type: 'auto', apiConfigured: false });Also applies to: 57-61, 170-176, 178-181
154-164: Avoid asserting raw numeric precedenceAsserting
precedence: 0is brittle. Preferexpect.objectContaining({ name: 'defaults' })or import the enum/const.
67-77: MockgetCurrentTagon theRuntimeStateManager
Inpackages/tm-core/src/config/config-manager.spec.ts, theRuntimeStateManagermock definesgetActiveTag, butConfigManager.getActiveTag()actually callsstateManager.getCurrentTag(). Add:getCurrentTag: vi.fn().mockReturnValue('master'),to the mock implementation so the test exercises the real API.
♻️ Duplicate comments (16)
packages/tm-core/tsup.config.ts (1)
5-15: Allowlist TM_PUBLIC_ keys and omit unset vars (defense-in-depth).*Prevents accidental embedding of unexpected envs and preserves undefined semantics.
Apply:
-// Get all TM_PUBLIC_* env variables for build-time injection -const getBuildTimeEnvs = () => { - const envs: Record<string, string> = {}; - for (const [key, value] of Object.entries(process.env)) { - if (key.startsWith('TM_PUBLIC_')) { - // Return the actual value, not JSON.stringify'd - envs[key] = value || ''; - } - } - return envs; -}; +// Allowlist TM_PUBLIC_* keys for build-time injection +const ALLOWED_PUBLIC_ENVS = [ + 'TM_PUBLIC_BASE_DOMAIN', + 'TM_PUBLIC_AUTH_BASE_URL' +] as const; +type AllowedPublicEnv = (typeof ALLOWED_PUBLIC_ENVS)[number]; +const getBuildTimeEnvs = () => { + const envs: Record<AllowedPublicEnv, string> = {} as any; + for (const key of ALLOWED_PUBLIC_ENVS) { + const val = process.env[key]; + if (val !== undefined) { + envs[key] = val; + } + } + return envs; +};packages/tm-core/src/index.ts (1)
47-55: Add a subpath export for '@tm-core/auth' to aid tree-shaking and optional consumption.Keep the root re-exports, but also expose a dedicated subpath in package.json ("exports": { "./auth": "./dist/auth/index.js", ... }) so consumers can avoid pulling auth unintentionally.
Run to verify subpath exports exist:
#!/bin/bash rg -n '"exports"' -n packages/tm-core/package.json -C6 rg -nP '"\\./auth"\\s*:' packages/tm-core/package.jsonpackages/tm-core/src/auth/credential-store.ts (2)
56-58: Enforce secure perms even when the directory already exists.Harden existing dirs to 0700 after ensuring they exist.
- if (!fs.existsSync(this.config.configDir)) { - fs.mkdirSync(this.config.configDir, { recursive: true, mode: 0o700 }); - } + if (!fs.existsSync(this.config.configDir)) { + fs.mkdirSync(this.config.configDir, { recursive: true, mode: 0o700 }); + } else { + try { fs.chmodSync(this.config.configDir, 0o700); } catch {} + }
60-69: Avoid mutating inputs; make atomic write robust and unique; clean up on failure.Use a copy with savedAt, write to a unique temp, and remove leftovers on error.
- // Add timestamp - authData.savedAt = new Date().toISOString(); - - // Save credentials atomically with secure permissions - const tempFile = `${this.config.configFile}.tmp`; - fs.writeFileSync(tempFile, JSON.stringify(authData, null, 2), { - mode: 0o600 - }); - fs.renameSync(tempFile, this.config.configFile); + // Prepare payload (avoid mutating caller's object) + const payload: AuthCredentials = { + ...authData, + savedAt: new Date().toISOString() + }; + // Save credentials atomically with secure permissions + const tempFile = `${this.config.configFile}.tmp-${process.pid}-${Date.now()}`; + try { + fs.writeFileSync(tempFile, JSON.stringify(payload, null, 2), { mode: 0o600 }); + fs.renameSync(tempFile, this.config.configFile); + } catch (e) { + try { if (fs.existsSync(tempFile)) fs.unlinkSync(tempFile); } catch {} + throw e; + }packages/tm-core/src/auth/oauth-service.ts (3)
247-249: Return 204 and disable caching on callback responsePrevents intermediaries from caching any sensitive data.
- // Server handles displaying success/failure, just close connection - res.writeHead(200); - res.end(); + // Blank, non-cacheable response + res.writeHead(204, { 'Cache-Control': 'no-store' }); + res.end();
20-20: Fix JSON import assertion for broad Node/TS compatibilityUse ESM “assert” syntax; current “with” form may break depending on runtime/bundler.
-import packageJson from '../../../../package.json' with { type: 'json' }; +import packageJson from '../../../../package.json' assert { type: 'json' };
287-289: Specify radix in parseInt to avoid edge casesSafer parsing when expires_in includes leading zeros.
- const expiresAt = expiresIn - ? new Date(Date.now() + parseInt(expiresIn) * 1000).toISOString() - : undefined; + const expiresAt = expiresIn + ? new Date(Date.now() + parseInt(expiresIn, 10) * 1000).toISOString() + : undefined;packages/tm-core/src/auth/auth-manager.ts (2)
33-38: Warn when getInstance is called with config after initializationAvoid silent surprises; log once if a later config is ignored.
static getInstance(config?: Partial<AuthConfig>): AuthManager { if (!AuthManager.instance) { AuthManager.instance = new AuthManager(config); - } + } else if (config) { + // Subsequent configs are ignored + // eslint-disable-next-line no-console + console.warn('[AuthManager] getInstance called with config after initialization; config is ignored.'); + } return AuthManager.instance; }
149-162: Honor tokenType for headers (API key vs Bearer)Branch header construction to match token type or confirm server expects Bearer for both.
- return { - Authorization: `Bearer ${authData.token}` - }; + if (authData.tokenType === 'api_key') { + return { 'x-api-key': authData.token }; + } + return { Authorization: `Bearer ${authData.token}` };apps/cli/src/commands/auth.command.ts (6)
109-117: Replace raw process.exit with ui.exit for consistent CLI terminationUse the central exit wrapper to keep behavior consistent and testable. Also remove the setTimeout shim.
if (!result.success) { - process.exit(1); + await ui.exit(1); } - // Exit cleanly after successful authentication - // Small delay to ensure all output is flushed - setTimeout(() => { - process.exit(0); - }, 100); + await ui.exit(0); @@ } catch (error: any) { this.handleError(error); - process.exit(1); + await ui.exit(1); } @@ if (!result.success) { - process.exit(1); + await ui.exit(1); } } catch (error: any) { this.handleError(error); - process.exit(1); + await ui.exit(1); } @@ } catch (error: any) { this.handleError(error); - process.exit(1); + await ui.exit(1); } @@ if (!result.success) { - process.exit(1); + await ui.exit(1); } } catch (error: any) { this.handleError(error); - process.exit(1); + await ui.exit(1); }Run to confirm no raw exits remain:
rg -nP 'process\.exit\s*\(' apps/cliAlso applies to: 120-121, 132-135, 136-138, 148-151, 162-165, 166-168
43-45: Avoid hard-coding service domain in UX stringsMake the text neutral or resolve from config to prevent drift.
- this.description('Manage authentication with tryhamster.com'); + this.description('Manage authentication'); @@ - .description('Authenticate with tryhamster.com') + .description('Authenticate in your browser')Also applies to: 61-63
187-205: Fix near-expiry display (tokens with <1h remaining show as expired)Compute precise remaining minutes/hours.
- if (credentials.expiresAt) { - const expiresAt = new Date(credentials.expiresAt); - const now = new Date(); - const hoursRemaining = Math.floor( - (expiresAt.getTime() - now.getTime()) / (1000 * 60 * 60) - ); - - if (hoursRemaining > 0) { - console.log( - chalk.gray( - ` Expires: ${expiresAt.toLocaleString()} (${hoursRemaining} hours remaining)` - ) - ); - } else { - console.log( - chalk.yellow(` Token expired at: ${expiresAt.toLocaleString()}`) - ); - } - } else { + if (credentials.expiresAt) { + const expiresAt = new Date(credentials.expiresAt); + const ms = expiresAt.getTime() - Date.now(); + if (ms > 0) { + const mins = Math.ceil(ms / 60000); + const pretty = + mins >= 60 ? `${Math.floor(mins / 60)}h ${mins % 60}m` : `${mins}m`; + console.log( + chalk.gray( + ` Expires: ${expiresAt.toLocaleString()} (~${pretty} remaining)` + ) + ); + } else { + console.log(chalk.yellow(` Token expired at: ${expiresAt.toLocaleString()}`)); + } + } else { console.log(chalk.gray(' Expires: Never (API key)')); }
221-223: Correct CLI hint to use “tm auth login”Matches project docs/binary name.
- chalk.gray('\n Run "task-master auth login" to authenticate') + chalk.gray('\n Run "tm auth login" to authenticate')
499-503: Return the concrete AuthCommand from registerOn for better typing/chainingImproves API ergonomics for callers.
-static registerOn(program: Command): Command { +static registerOn(program: Command): AuthCommand { const authCommand = new AuthCommand(); program.addCommand(authCommand); return authCommand; }
61-66: Offer a --json flag for automation-friendly outputEmit machine-readable AuthResult alongside human logs.
Example sketch:
- this.command('status') + this.command('status') + .option('--json', 'output machine-readable JSON') .description('Display authentication status') .action(async (opts) => { - await this.executeStatus(); + await this.executeStatus(opts); }); @@ -private async executeStatus(): Promise<void> { +private async executeStatus(opts?: { json?: boolean }): Promise<void> { try { const result = this.displayStatus(); this.setLastResult(result); + if (opts?.json) { + console.log(JSON.stringify(result)); + } } catch (error: any) {I can apply this pattern to login/refresh as well if you want.
Also applies to: 82-88, 94-99, 174-231, 262-279
packages/tm-core/src/auth/types.ts (1)
10-13: Standardize time fields to epoch millisecondsNumbers simplify math and avoid timezone/string parsing pitfalls. Add migration in store to parse legacy ISO strings.
- expiresAt?: string; + expiresAt?: number; // epoch ms tokenType?: 'standard' | 'api_key'; - savedAt: string; + savedAt: number; // epoch msFollow-ups:
- Supabase client: return expiresAt as number (session.expires_at * 1000).
- AuthManager/OAuthService: set savedAt = Date.now().
- CLI: still works via new Date(number).
Verify all usages compile:
rg -n -t ts '\b(expiresAt|savedAt)\b'
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (20)
apps/cli/package.json(1 hunks)apps/cli/src/commands/auth.command.ts(1 hunks)packages/tm-core/package.json(2 hunks)packages/tm-core/src/auth/auth-manager.ts(1 hunks)packages/tm-core/src/auth/credential-store.ts(1 hunks)packages/tm-core/src/auth/index.ts(1 hunks)packages/tm-core/src/auth/oauth-service.ts(1 hunks)packages/tm-core/src/auth/types.ts(1 hunks)packages/tm-core/src/clients/supabase-client.ts(1 hunks)packages/tm-core/src/config/config-manager.spec.ts(2 hunks)packages/tm-core/src/config/config-manager.ts(1 hunks)packages/tm-core/src/config/services/environment-config-provider.service.spec.ts(1 hunks)packages/tm-core/src/config/services/environment-config-provider.service.ts(1 hunks)packages/tm-core/src/index.ts(1 hunks)packages/tm-core/src/interfaces/configuration.interface.ts(2 hunks)packages/tm-core/src/services/index.ts(1 hunks)packages/tm-core/src/subpath-exports.test.ts(1 hunks)packages/tm-core/src/utils/index.ts(1 hunks)packages/tm-core/tsconfig.json(1 hunks)packages/tm-core/tsup.config.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/config/config-manager.spec.tspackages/tm-core/src/subpath-exports.test.tspackages/tm-core/src/config/services/environment-config-provider.service.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/config/config-manager.spec.tspackages/tm-core/src/subpath-exports.test.tspackages/tm-core/src/config/services/environment-config-provider.service.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/config/config-manager.spec.tspackages/tm-core/src/subpath-exports.test.tspackages/tm-core/src/config/services/environment-config-provider.service.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/subpath-exports.test.ts
🧠 Learnings (31)
📓 Common learnings
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.899Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
📚 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 : Centralize all LLM calls through `generateTextService` or `generateObjectService`.
Applied to files:
packages/tm-core/src/services/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/services/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/task-manager.js : Features that create, read, update, or delete tasks belong in 'scripts/modules/task-manager.js'.
Applied to files:
packages/tm-core/src/services/index.ts
📚 Learning: 2025-07-18T17:14:54.131Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/telemetry.mdc:0-0
Timestamp: 2025-07-18T17:14:54.131Z
Learning: Applies to scripts/modules/task-manager/**/*.js : Core logic functions in scripts/modules/task-manager/ must return an object that includes aiServiceResponse.telemetryData.
Applied to files:
packages/tm-core/src/services/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 **/*.test.js : Verify file operations use the correct paths in expect statements.
Applied to files:
packages/tm-core/src/subpath-exports.test.tsapps/cli/src/commands/auth.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/**/*.test.js : Test core logic independently with both data formats, mock file system operations, test tag resolution behavior, and verify migration compatibility in unit tests.
Applied to files:
packages/tm-core/src/subpath-exports.test.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 : Use different file paths for each test to avoid test interdependence.
Applied to files:
packages/tm-core/src/subpath-exports.test.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/subpath-exports.test.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/fixtures/**/* : Test fixtures must be located in tests/fixtures/, provide reusable test data, and be exported as named exports for reuse.
Applied to files:
packages/tm-core/src/subpath-exports.test.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 tests/{unit,integration,e2e,fixtures}/**/*.js : Test files must be organized as follows: unit tests in tests/unit/, integration tests in tests/integration/, end-to-end tests in tests/e2e/, and test fixtures in tests/fixtures/.
Applied to files:
packages/tm-core/src/subpath-exports.test.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 : Create self-contained test implementations rather than using real implementations when testing modules with initialization issues.
Applied to files:
packages/tm-core/src/subpath-exports.test.ts
📚 Learning: 2025-08-03T12:13:33.875Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-08-03T12:13:33.875Z
Learning: Applies to {tests/setup.ts,tests/setup/integration.ts,tests/teardown.ts} : Test setup files should be created at tests/setup.ts, tests/setup/integration.ts, and tests/teardown.ts
Applied to files:
packages/tm-core/src/subpath-exports.test.ts
📚 Learning: 2025-08-03T12:13:33.875Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-08-03T12:13:33.875Z
Learning: Applies to **/*.test.ts : Test all code paths, including edge cases and error scenarios, in test files
Applied to files:
packages/tm-core/src/subpath-exports.test.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 : Use test-specific file paths (e.g., 'test-tasks.json') for all file operations in tests.
Applied to files:
packages/tm-core/src/subpath-exports.test.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: Each module should have well-defined exports that can be mocked in tests.
Applied to files:
packages/tm-core/src/subpath-exports.test.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 : For ES modules, use jest.mock() before static imports and jest.unstable_mockModule() before dynamic imports to mock dependencies.
Applied to files:
packages/tm-core/src/subpath-exports.test.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 : Create utilities for consistent task ID handling and support different ID formats (numeric, string, dot notation); do not duplicate formatting logic across modules.
Applied to files:
packages/tm-core/src/utils/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,mcp-server/src/core/utils/path-utils.js,mcp-server/src/tools/utils.js} : Export all utility functions explicitly, group related functions logically, and include new tagged system utilities.
Applied to files:
packages/tm-core/src/utils/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 : 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:
packages/tm-core/src/utils/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,mcp-server/src/core/utils/path-utils.js,mcp-server/src/tools/utils.js} : Keep utilities relevant to their location, export all utility functions in a single statement per file, group related exports together, export configuration constants, do not use default exports, and do not create circular dependencies.
Applied to files:
packages/tm-core/src/utils/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 : Use consistent formatting for task files, include all task properties in text files, and format dependencies with status indicators.
Applied to files:
packages/tm-core/src/utils/index.ts
📚 Learning: 2025-09-02T21:51:27.899Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.899Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
Applied to files:
packages/tm-core/src/auth/oauth-service.ts
📚 Learning: 2025-08-02T15:33:22.656Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1069
File: .changeset/fix-tag-complexity-detection.md:0-0
Timestamp: 2025-08-02T15:33:22.656Z
Learning: For changeset files (.changeset/*.md), Crunchyman-ralph prefers to ignore formatting nitpicks about blank lines between frontmatter and descriptions, as he doesn't mind having them and wants to avoid such comments in future reviews.
Applied to files:
packages/tm-core/src/auth/oauth-service.ts
📚 Learning: 2025-09-02T21:49:28.120Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/oauth-service.ts:215-218
Timestamp: 2025-09-02T21:49:28.120Z
Learning: In the OAuth service callback handler, the callback page is not actually shown to users, so returning HTML templates (getSuccessHtml/getErrorHtml) is not necessary - a blank 200 response is sufficient.
Applied to files:
packages/tm-core/src/auth/oauth-service.ts
📚 Learning: 2025-09-03T09:46:31.829Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/oauth-service.ts:293-300
Timestamp: 2025-09-03T09:46:31.829Z
Learning: In OAuth flows where URLs are opened in browsers and shown to users, logging the full authorization URL is acceptable since it's meant to be user-visible and typically contains non-sensitive client metadata rather than actual tokens or secrets.
Applied to files:
packages/tm-core/src/auth/oauth-service.ts
📚 Learning: 2025-07-18T17:19:27.365Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/.windsurfrules:0-0
Timestamp: 2025-07-18T17:19:27.365Z
Learning: Use the global `task-master` CLI command instead of directly invoking `node scripts/dev.js` for all task management operations.
Applied to files:
apps/cli/src/commands/auth.command.ts
📚 Learning: 2025-07-18T17:10:12.881Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dev_workflow.mdc:0-0
Timestamp: 2025-07-18T17:10:12.881Z
Learning: For CLI usage, install Taskmaster globally with `npm install -g task-master-ai` or use locally via `npx task-master-ai ...`.
Applied to files:
apps/cli/src/commands/auth.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 : Use consistent patterns for option naming and help text in CLI commands.
Applied to files:
apps/cli/src/commands/auth.command.ts
📚 Learning: 2025-09-02T21:51:41.338Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:0-0
Timestamp: 2025-09-02T21:51:41.338Z
Learning: In packages/tm-core/src/auth/config.ts, the BASE_DOMAIN configuration intentionally does not include runtime environment variable fallbacks like TM_BASE_DOMAIN or HAMSTER_BASE_URL. The maintainers prefer to keep these capabilities "hush hush" and undocumented, using only the build-time TM_PUBLIC_BASE_DOMAIN and the default value.
Applied to files:
packages/tm-core/tsup.config.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 : Do not rely on environment variables for API keys in tests; set mock environment variables in test setup.
Applied to files:
packages/tm-core/tsup.config.ts
🧬 Code graph analysis (8)
packages/tm-core/src/config/config-manager.spec.ts (2)
packages/tm-core/src/config/config-manager.ts (1)
ConfigManager(29-273)tests/integration/mcp-server/direct-functions.test.js (1)
testProjectRoot(14-14)
packages/tm-core/src/config/services/environment-config-provider.service.spec.ts (1)
packages/tm-core/src/config/services/environment-config-provider.service.ts (1)
EnvironmentConfigProvider(26-166)
packages/tm-core/src/auth/auth-manager.ts (4)
packages/tm-core/src/auth/credential-store.ts (1)
CredentialStore(10-109)packages/tm-core/src/auth/oauth-service.ts (1)
OAuthService(22-346)packages/tm-core/src/clients/supabase-client.ts (1)
SupabaseAuthClient(9-154)packages/tm-core/src/auth/types.ts (4)
AuthConfig(30-34)AuthCredentials(5-13)OAuthFlowOptions(15-28)AuthenticationError(73-85)
packages/tm-core/src/clients/supabase-client.ts (3)
packages/tm-core/src/auth/types.ts (1)
AuthenticationError(73-85)packages/tm-core/src/auth/auth-manager.ts (1)
refreshToken(89-121)packages/tm-core/src/logger/logger.ts (1)
error(163-166)
packages/tm-core/src/auth/oauth-service.ts (6)
packages/tm-core/src/logger/factory.ts (1)
getLogger(23-44)packages/tm-core/src/auth/credential-store.ts (1)
CredentialStore(10-109)packages/tm-core/src/clients/supabase-client.ts (1)
SupabaseAuthClient(9-154)packages/tm-core/src/auth/types.ts (5)
AuthConfig(30-34)OAuthFlowOptions(15-28)AuthCredentials(5-13)AuthenticationError(73-85)CliData(36-45)packages/tm-core/src/auth/config.ts (1)
getAuthConfig(32-37)packages/tm-core/src/auth/auth-manager.ts (1)
refreshToken(89-121)
apps/cli/src/commands/auth.command.ts (2)
packages/tm-core/src/auth/types.ts (2)
AuthCredentials(5-13)AuthenticationError(73-85)packages/tm-core/src/auth/auth-manager.ts (1)
AuthManager(18-163)
packages/tm-core/src/auth/types.ts (2)
packages/tm-core/src/auth/index.ts (5)
AuthCredentials(10-10)OAuthFlowOptions(11-11)AuthenticationError(16-16)AuthConfig(12-12)CliData(13-13)packages/tm-core/src/index.ts (4)
AuthCredentials(51-51)OAuthFlowOptions(52-52)AuthenticationError(50-50)AuthConfig(53-53)
packages/tm-core/src/auth/credential-store.ts (4)
packages/tm-core/src/logger/factory.ts (1)
getLogger(23-44)packages/tm-core/src/auth/types.ts (3)
AuthConfig(30-34)AuthCredentials(5-13)AuthenticationError(73-85)packages/tm-core/src/auth/config.ts (1)
getAuthConfig(32-37)packages/tm-core/src/logger/logger.ts (1)
error(163-166)
🔇 Additional comments (17)
apps/cli/package.json (1)
32-32: LGTM: dependency addition is correct for CLI OAuth flow.open is an appropriate runtime dep here and aligns with Node >=18 and ESM usage in this package.
packages/tm-core/src/config/services/environment-config-provider.service.spec.ts (1)
88-92: LGTM: adds coverage for 'auto' storage type.Test resets provider and validates acceptance; consistent with validator changes.
packages/tm-core/tsup.config.ts (2)
45-48: Confirm externalized imports and dependency declarations
- Run the build (e.g.
pnpm build/tsup) to populatepackages/tm-core/distand verify no unintended modules are bundled.- Ensure every externalized import (
@supabase/supabase-js,chalk,zod, etc.) is correctly listed underdependenciesorpeerDependenciesinpackages/tm-core/package.jsonas intended.
42-43: Only TM_PUBLIC_BASE_DOMAIN, TM_PUBLIC_SUPABASE_URL, and TM_PUBLIC_SUPABASE_ANON_KEY are referenced — no other TM_PUBLIC_ keys detected.*packages/tm-core/src/services/index.ts (1)
1-6: LGTM: clean public surface re-exportCentralized export of TaskService is clear and minimal.
packages/tm-core/src/config/services/environment-config-provider.service.ts (1)
34-35: LGTM: allow 'auto' for storage.typeValidation now accepts 'auto' as intended.
packages/tm-core/src/subpath-exports.test.ts (1)
1-4: Ensure subpath‐exports test runs against built distribution
Tests currently import source files (e.g.import('./auth')); to validate yourexportsmap you must import via package subpaths (@tm/core/auth, etc.) and execute the test against the compileddistoutput.packages/tm-core/src/auth/index.ts (1)
5-21: Barrel looks clean and type-safe.Value vs. type-only separation is correct; no further action.
packages/tm-core/src/utils/index.ts (2)
6-14: Good centralization of ID utilities; aligns with prior guidance.Re-exporting from id-generator reduces duplication and meets our utilities consistency rules.
16-16: No prod imports detected; please manually verify guarded utilities
Search yielded no references to formatDate or deepClone in non-test code—ensure these @deprecated placeholders remain hidden behind an internal entrypoint or feature flag.packages/tm-core/src/auth/credential-store.ts (1)
98-101: Outdated suggestion:refreshTokenalready callsgetCredentials({ allowExpired: true }).Likely an incorrect or invalid review comment.
packages/tm-core/src/clients/supabase-client.ts (3)
62-101: Refresh flow looks correct with supabase-js v2 session shapeInputs, error handling, and ISO conversion align with expected API. LGTM.
116-133: Graceful user lookup handling is appropriateNull on error with warning is fine for non-critical enrichment. LGTM.
140-153: Sign-out best-effort is acceptableGiven persistSession: false, treating errors as non-fatal makes sense. LGTM.
packages/tm-core/src/auth/auth-manager.ts (1)
126-137: All logout call sites correctly await async logout()
CLI invocation in apps/cli/src/commands/auth.command.ts usesawait this.authManager.logout()and no other unawaited usages were found.apps/cli/src/commands/auth.command.ts (1)
367-407: OAuth browser flow wiring looks solidGood use of callbacks, spinner lifecycle, and fallback URL print.
packages/tm-core/src/auth/types.ts (1)
47-69: Typed AuthErrorCode and enriched AuthenticationError are goodStrong codes and optional cause improve handling/debugging.
Also applies to: 73-85
1 participant
What type of PR is this?
Description
Related Issues
How to Test This
# Example commands or stepsExpected result:
Contributor Checklist
npm run changesetnpm testnpm run format-check(ornpm run formatto fix)Changelog Entry
For Maintainers
Summary by CodeRabbit
New Features
Improvements
Chores