chore: apply requested changes#1442
Conversation
|
WalkthroughUpdates MFA handling in the CLI auth flow to extract and pass factorId explicitly, tightens maxAttempts validation and test expectations, relaxes a null check and improves warning logging in SessionManager, adds environment variable cleanup in Supabase integration tests, and documents an npm test invocation in CLAUDE.md. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Auth Command
participant User as User
participant AuthMgr as Auth Manager
CLI->>CLI: Receive MFA error (mfaError)
CLI->>CLI: factorId = mfaError.mfaChallenge?.factorId
alt factorId missing
CLI-->>User: Fail flow (MFA info missing)
else factorId present
CLI->>User: Prompt for MFA code
User-->>CLI: Provide code
CLI->>AuthMgr: verifyMFAWithRetry(factorId, code, maxAttempts)
AuthMgr->>AuthMgr: Validate maxAttempts (finite integer ≥ 1)
alt invalid maxAttempts
AuthMgr-->>CLI: Throw TypeError (Invalid maxAttempts: Must be a positive integer)
else valid
AuthMgr->>AuthMgr: Retry verification loop
alt verification success
AuthMgr-->>CLI: Return success
else verification fails
AuthMgr-->>CLI: Return failure/error
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
CLAUDE.md (1)
175-180: Align bullet style with surrounding guidelinesFor consistency with the preceding bullets, consider capitalizing and formatting the command as inline code, e.g.:
- Run `npm run test -w <package-name>` to test a packagepackages/tm-core/src/modules/auth/services/session-manager.spec.ts (1)
395-411: Using a plainErrorthrow to guard the happy path is fineThe explicit
throw new Error('Should have thrown MFA_REQUIRED error')correctly fails the test if no error is raised. If you ever want to simplify, this test could be rewritten withawait expect(...).rejects.toThrow(AuthenticationError), but the current structure is perfectly acceptable.packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts (1)
327-363: Tests now depend on the fullmaxAttemptserror messageThe updated expectations correctly reflect the new message, but they do couple the spec to the exact wording. If you want these tests to be a bit more resilient to future copy changes, you could switch to a substring/regex match like:
await expect(...).rejects.toThrow(/Invalid maxAttempts value: 0/);That said, the current assertions are valid if you prefer strict messaging checks.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CLAUDE.md(1 hunks)apps/cli/src/commands/auth.command.ts(2 hunks)packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts(2 hunks)packages/tm-core/src/modules/auth/managers/auth-manager.ts(1 hunks)packages/tm-core/src/modules/auth/services/session-manager.spec.ts(1 hunks)packages/tm-core/src/modules/auth/services/session-manager.ts(2 hunks)packages/tm-core/src/modules/integration/clients/supabase-client.spec.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{spec,test}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{spec,test}.ts: Place package tests inpackages/<package-name>/src/<module>/<file>.spec.tsor apps inapps/<app-name>/src/<module>/<file>.spec.tsalongside source, or usepackages/<package-name>/tests/integration/<module>/<file>.test.tsfor integration tests
Always use.tsfor TypeScript tests, never.js
NEVER use async/await in test functions unless testing actual asynchronous operations; use synchronous top-level imports instead of dynamicawait import()
Write tests for bug fixes (add regression test), business logic (complex calculations, validations, transformations), edge cases (boundary conditions, error handling), public APIs, and integration points; skip tests for simple getters/setters, trivial pass-through functions, pure configuration objects, and code that just delegates to another tested function
Files:
packages/tm-core/src/modules/auth/services/session-manager.spec.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/integration/clients/supabase-client.spec.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
TypeScript test files must achieve minimum code coverage thresholds: 80% lines/functions and 70% branches globally, 90% for utilities, and 85% for middleware; new features must meet or exceed these thresholds
Files:
packages/tm-core/src/modules/auth/services/session-manager.spec.tsapps/cli/src/commands/auth.command.tspackages/tm-core/src/modules/auth/managers/auth-manager.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/auth/services/session-manager.tspackages/tm-core/src/modules/integration/clients/supabase-client.spec.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/*.{js,ts}: Import and use specific getters from config-manager.js (e.g., getMainProvider(), getLogLevel(), getMainMaxTokens()) to access configuration values needed for application logic
Use isApiKeySet(providerName, session) from config-manager.js to check if a provider's key is available before potentially attempting an AI call
Do not add direct console.log calls outside the logging utility - use the central log function instead
Ensure silent mode is disabled in a finally block to prevent it from staying enabled
Do not access the global silentMode variable directly - use the exported silent mode control functions instead
Do not duplicate task ID formatting logic across modules - centralize formatting utilities
Use ContextGatherer class from utils/contextGatherer.js for AI-powered commands that need project context, supporting tasks, files, custom text, and project tree context
Use FuzzyTaskSearch class from utils/fuzzyTaskSearch.js for automatic task relevance detection with configurable search parameters
Use fuzzy search to supplement user-provided task IDs and display discovered task IDs to users for transparency
Do not replace explicit user task selections with fuzzy results - fuzzy search should supplement, not replace user selections
Use readJSON and writeJSON utilities for all JSON file operations instead of raw fs.readFileSync or fs.writeFileSync
Include error handling for JSON file operations and validate JSON structure after reading
Use path.join() for cross-platform path construction and path.resolve() for absolute paths, validating paths before file operations
Support both .env files and MCP session environment for environment variable resolution with fallbacks for missing values
Prefer updating the core function to accept an outputFormat parameter and check outputFormat === 'json' before displaying UI elements
Files:
packages/tm-core/src/modules/auth/services/session-manager.spec.tsapps/cli/src/commands/auth.command.tspackages/tm-core/src/modules/auth/managers/auth-manager.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/auth/services/session-manager.tspackages/tm-core/src/modules/integration/clients/supabase-client.spec.ts
**/*.md
📄 CodeRabbit inference engine (.cursor/rules/ai_providers.mdc)
Update relevant documentation (like
README.md) mentioning supported providers or configuration when adding a new AI provider
Files:
CLAUDE.md
🧠 Learnings (37)
📓 Common learnings
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/git_workflow.mdc:0-0
Timestamp: 2025-11-24T18:00:23.000Z
Learning: Pull Request descriptions must include: Task Overview, Subtasks Completed (checklist), Implementation Details, Testing approach, Breaking Changes (if any), and Related Tasks.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 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.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1232
File: packages/build-config/package.json:14-15
Timestamp: 2025-09-22T19:45:13.323Z
Learning: In the eyaltoledano/claude-task-master repository, Crunchyman-ralph intentionally omits version fields from internal packages (like tm/build-config) to prevent changesets from releasing new versions for these packages. This is the desired behavior for internal tooling packages that should not be published or versioned independently.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
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.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1132
File: .github/workflows/weekly-metrics-discord.yml:81-93
Timestamp: 2025-08-13T22:10:46.958Z
Learning: Crunchyman-ralph ignores YAML formatting nitpicks about trailing spaces when there's no project-specific YAML formatter configured, preferring to focus on functionality over cosmetic formatting issues.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1132
File: .github/workflows/weekly-metrics-discord.yml:81-93
Timestamp: 2025-08-13T22:10:46.958Z
Learning: Crunchyman-ralph ignores YAML formatting nitpicks about trailing spaces when there's no project-specific YAML formatter configured, preferring to focus on functionality over cosmetic formatting issues.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1200
File: src/ai-providers/custom-sdk/grok-cli/language-model.js:96-100
Timestamp: 2025-09-19T16:06:42.182Z
Learning: The user Crunchyman-ralph prefers to keep environment variable names explicit (like GROK_CLI_API_KEY) rather than supporting multiple aliases, to avoid overlap and ensure clear separation between different CLI implementations.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1105
File: scripts/modules/supported-models.json:242-254
Timestamp: 2025-08-08T11:33:15.297Z
Learning: Preference: In scripts/modules/supported-models.json, the "name" field is optional. For OpenAI entries (e.g., "gpt-5"), Crunchyman-ralph prefers omitting "name" when the id is explicit enough; avoid nitpicks requesting a "name" in such cases.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1178
File: packages/tm-core/src/subpath-exports.test.ts:6-9
Timestamp: 2025-09-03T12:45:30.724Z
Learning: The user Crunchyman-ralph prefers to avoid overly nitpicky or detailed suggestions in code reviews, especially for test coverage of minor import paths. Focus on more substantial issues rather than comprehensive coverage of all possible edge cases.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1217
File: apps/cli/src/index.ts:16-21
Timestamp: 2025-09-18T16:35:35.147Z
Learning: The user Crunchyman-ralph considers suggestions to export types for better ergonomics (like exporting UpdateInfo type alongside related functions) as nitpicky and prefers not to implement such suggestions.
📚 Learning: 2025-11-24T18:03:46.700Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.700Z
Learning: Applies to **/*.test.js : When testing exact error messages, don't assert on exact error message text that might change. Instead test for error presence and general properties using patterns like `toContain` or `toMatch`.
Applied to files:
packages/tm-core/src/modules/auth/services/session-manager.spec.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.ts
📚 Learning: 2025-11-24T18:03:13.408Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.408Z
Learning: Applies to **/*.test.ts : Unit tests must follow the describe/it pattern, use beforeEach for mock setup with jest.clearAllMocks(), include specific assertions with expect(), and test both success and error scenarios including edge cases
Applied to files:
packages/tm-core/src/modules/auth/services/session-manager.spec.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.ts
📚 Learning: 2025-11-24T18:03:46.700Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.700Z
Learning: Applies to **/*.test.js : Do not import or instantiate real AI service clients. Create fully mocked versions that return predictable responses. Mock the entire module with controlled behavior.
Applied to files:
packages/tm-core/src/modules/auth/services/session-manager.spec.tspackages/tm-core/src/modules/integration/clients/supabase-client.spec.ts
📚 Learning: 2025-11-24T18:03:13.408Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.408Z
Learning: Use established mocking patterns from auth.test.ts as templates: mock bcrypt with proper TypeScript typing, mock Prisma client with transaction support, and always clear mocks between tests
Applied to files:
packages/tm-core/src/modules/auth/services/session-manager.spec.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/integration/clients/supabase-client.spec.ts
📚 Learning: 2025-11-24T18:03:13.408Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.408Z
Learning: Applies to tests/e2e/**/*.test.ts : End-to-end tests must test complete user workflows across multiple API endpoints in sequence, verify state changes between workflow steps, use extended timeouts (30000ms), and validate final outcomes without mocking business logic
Applied to files:
packages/tm-core/src/modules/auth/services/session-manager.spec.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.ts
📚 Learning: 2025-11-12T20:05:10.138Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1389
File: packages/tm-core/src/modules/auth/config.ts:55-59
Timestamp: 2025-11-12T20:05:10.138Z
Learning: In packages/tm-core/src/modules/auth/config.ts, the DEFAULT_AUTH_CONFIG Proxy does not require enumeration support (has, ownKeys, getOwnPropertyDescriptor traps). The codebase only uses direct property access on this config object, so the basic get trap is sufficient.
Applied to files:
packages/tm-core/src/modules/auth/services/session-manager.spec.ts
📚 Learning: 2025-11-24T18:03:46.699Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.699Z
Learning: Applies to **/*.test.js : When testing console assertions: ensure assertion matches actual arguments passed; if code logs a single formatted string, assert against that single string using `expect.stringContaining` or exact match, not multiple arguments; verify exact behavior of the code being tested.
Applied to files:
packages/tm-core/src/modules/auth/services/session-manager.spec.ts
📚 Learning: 2025-11-24T18:03:13.408Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.408Z
Learning: Applies to tests/fixtures/**/*.ts : Test fixture files must export reusable test data creators and constants (createTestUser, adminUser, invalidUser, etc.) for use across unit, integration, and e2e tests to ensure consistency
Applied to files:
packages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/integration/clients/supabase-client.spec.ts
📚 Learning: 2025-11-24T18:03:13.408Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.408Z
Learning: Applies to src/**/*.ts : Source code must have corresponding test files either colocated (*.test.ts) or in tests/unit/ directory following established patterns from src/utils/auth.test.ts with proper mocking for external dependencies
Applied to files:
packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts
📚 Learning: 2025-11-24T17:56:37.308Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:56:37.308Z
Learning: Applies to **/*.{spec,test}.ts : Write tests for bug fixes (add regression test), business logic (complex calculations, validations, transformations), edge cases (boundary conditions, error handling), public APIs, and integration points; skip tests for simple getters/setters, trivial pass-through functions, pure configuration objects, and code that just delegates to another tested function
Applied to files:
packages/tm-core/src/modules/auth/managers/auth-manager.spec.tsCLAUDE.md
📚 Learning: 2025-11-24T18:03:13.408Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.408Z
Learning: Applies to **/*.integration.test.ts : Integration tests must use supertest for API endpoint testing, verify database state changes after operations, clean test data before each test, and include full request/response validation with expected HTTP status codes
Applied to files:
packages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/integration/clients/supabase-client.spec.ts
📚 Learning: 2025-11-24T17:57:14.728Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/ai_providers.mdc:0-0
Timestamp: 2025-11-24T17:57:14.728Z
Learning: Applies to tests/unit/ai-providers/*.test.js : Create unit tests in `tests/unit/ai-providers/<provider-name>.test.js` that mock the provider's AI SDK module and test each exported function for correct client instantiation, parameter passing, result handling, and error handling
Applied to files:
packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts
📚 Learning: 2025-11-24T18:03:13.408Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.408Z
Learning: Applies to tests/setup/integration.ts : Integration test setup file must connect to Prisma before all tests with prisma.$connect(), disconnect after all tests with prisma.$disconnect(), and provide database cleanup before each test
Applied to files:
packages/tm-core/src/modules/integration/clients/supabase-client.spec.ts
📚 Learning: 2025-11-24T18:03:13.408Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.408Z
Learning: Applies to tests/teardown.ts : Global teardown file must be created to handle cleanup after all tests complete and prevent worker process leaks
Applied to files:
packages/tm-core/src/modules/integration/clients/supabase-client.spec.ts
📚 Learning: 2025-11-24T18:03:46.700Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.700Z
Learning: Applies to **/*.test.js : Set mock environment variables for API keys in test setup (e.g., ANTHROPIC_API_KEY, PERPLEXITY_API_KEY) rather than relying on real environment variables.
Applied to files:
packages/tm-core/src/modules/integration/clients/supabase-client.spec.ts
📚 Learning: 2025-11-24T18:03:46.700Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.700Z
Learning: Applies to **/*.test.js : Create simplified test functions instead of complex setups: create simplified versions of complex functions focusing on core logic; remove file system operations, API calls, and external dependencies; pass all dependencies as parameters.
Applied to files:
packages/tm-core/src/modules/integration/clients/supabase-client.spec.ts
📚 Learning: 2025-11-24T18:03:46.699Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.699Z
Learning: Applies to tests/integration/**/*.test.js : Locate integration tests in `tests/integration/` directory. Test interactions between modules, focus on component interfaces rather than implementation details, use more realistic but controlled test environments.
Applied to files:
packages/tm-core/src/modules/integration/clients/supabase-client.spec.ts
📚 Learning: 2025-11-24T18:03:13.408Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.408Z
Learning: Applies to tests/setup.ts : Create global test setup file that configures jest.setTimeout(10000), clears all mocks after each test with jest.clearAllMocks(), and initializes global test configuration
Applied to files:
packages/tm-core/src/modules/integration/clients/supabase-client.spec.ts
📚 Learning: 2025-11-24T18:03:46.700Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.700Z
Learning: Applies to **/*.test.js : For modules with initialization-dependent functions in tests: create test-specific implementations that initialize all variables correctly; use factory functions in mocks to ensure proper initialization order; be careful with how you mock functions that depend on module state.
Applied to files:
packages/tm-core/src/modules/integration/clients/supabase-client.spec.ts
📚 Learning: 2025-11-24T18:03:46.699Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.699Z
Learning: Applies to **/*.test.js : Mock file system operations using `mock-fs` library. Mock API calls by providing jest.fn() implementations that return expected structures. Mock environment variables in test setup.
Applied to files:
packages/tm-core/src/modules/integration/clients/supabase-client.spec.ts
📚 Learning: 2025-11-24T18:01:44.137Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-11-24T18:01:44.137Z
Learning: Applies to **/*.test.{js,ts} : Follow the mock-first-then-import pattern for Jest mocking; use jest.spyOn() for spy functions; clear mocks between tests; verify mocks with the pattern described in `tests.mdc`
Applied to files:
packages/tm-core/src/modules/integration/clients/supabase-client.spec.ts
📚 Learning: 2025-11-24T18:03:46.700Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.700Z
Learning: Applies to **/*.test.js : Clear mocks between tests using `jest.clearAllMocks()` in `beforeEach`. Reset specific mock functions with `mockFn.mockReset()`. Prevent state leakage between tests.
Applied to files:
packages/tm-core/src/modules/integration/clients/supabase-client.spec.ts
📚 Learning: 2025-11-24T17:56:37.308Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:56:37.308Z
Learning: Add a changeset for code changes by running `npx changeset` after making code changes (not needed for docs-only PRs); changesets should be user-facing, describing what end-users get or what is fixed rather than implementation details
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T17:58:19.822Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/changeset.mdc:0-0
Timestamp: 2025-11-24T17:58:19.822Z
Learning: Run `npm run changeset` after staging a logical set of changes that should be communicated in the next release's `CHANGELOG.md` for new features, bug fixes, breaking changes, performance improvements, significant refactoring, user-facing documentation updates, dependency updates, or build/tooling changes
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T18:00:32.587Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/glossary.mdc:0-0
Timestamp: 2025-11-24T18:00:32.587Z
Learning: Refer to changeset.mdc for guidelines on using Changesets (npm run changeset) to manage versioning and changelogs
Applied to files:
CLAUDE.md
📚 Learning: 2025-08-02T14:54:52.216Z
Learnt from: eyaltoledano
Repo: eyaltoledano/claude-task-master PR: 1069
File: .changeset/floppy-news-buy.md:7-38
Timestamp: 2025-08-02T14:54:52.216Z
Learning: For major feature additions like new CLI commands, eyaltoledano prefers detailed changesets with comprehensive descriptions, usage examples, and feature explanations rather than minimal single-line summaries.
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T18:03:13.408Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.408Z
Learning: Applies to package.json : package.json scripts must include: 'test', 'test:watch', 'test:coverage', 'test:unit', 'test:integration', 'test:e2e', and 'test:ci' commands for testing framework integration
Applied to files:
CLAUDE.md
📚 Learning: 2025-10-08T12:21:14.455Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1285
File: .changeset/nice-ways-hope.md:5-17
Timestamp: 2025-10-08T12:21:14.455Z
Learning: For changeset files (.changeset/*.md): The first line should be concise and in imperative mood, but the body after the first line can include as many user-facing details as desired (bullets, explanations, links, etc.) to provide context for the CHANGELOG.md.
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T17:58:19.822Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/changeset.mdc:0-0
Timestamp: 2025-11-24T17:58:19.822Z
Learning: Provide a concise, single-line changeset summary in imperative mood (e.g., 'Add feature X', 'Fix bug Y') that describes what changed from a user/consumer perspective, not implementation details
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T18:03:46.699Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.699Z
Learning: Applies to **/*.test.js : When testing CLI commands built with Commander.js: test command action handlers directly rather than mocking the entire Commander chain; create simplified test-specific implementations of command handlers; explicitly handle all options including defaults and shorthand flags; include null/undefined checks for optional parameters; use fixtures for consistent sample data.
Applied to files:
CLAUDE.md
📚 Learning: 2025-08-06T21:14:23.071Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1091
File: .changeset/wide-actors-report.md:0-0
Timestamp: 2025-08-06T21:14:23.071Z
Learning: For changeset files (.changeset/*.md), the first line after the frontmatter must be a plain, unstyled summary line that gets integrated directly into the changelog. Do not add markdown headings or styling as this would interfere with the changelog generation process. Ignore markdownlint MD041 rule for these files.
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T18:03:13.408Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.408Z
Learning: Before committing code, verify all tests pass with coverage using npm run test:coverage, run unit tests with npm run test:unit, and integration tests with npm run test:integration where applicable
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T18:01:44.137Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-11-24T18:01:44.137Z
Learning: Run the full test suite with `npm test` when submitting new features to ensure nothing was broken
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T18:03:46.699Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.699Z
Learning: Applies to **/*.test.js : Test quality guidelines: Write tests before implementing features (TDD). Test edge cases and error conditions, not just happy paths. Keep tests independent and isolated. Use descriptive test names. Maintain test fixtures separately. Aim for 80%+ code coverage with critical paths at 100%.
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T18:03:13.408Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.408Z
Learning: Applies to **/*.ts : TypeScript test files must achieve minimum code coverage thresholds: 80% lines/functions and 70% branches globally, 90% for utilities, and 85% for middleware; new features must meet or exceed these thresholds
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T17:58:19.822Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/changeset.mdc:0-0
Timestamp: 2025-11-24T17:58:19.822Z
Learning: When running changeset, select the appropriate semantic version bump type: `Major` for breaking changes, `Minor` for new features, and `Patch` for bug fixes, performance improvements, user-facing documentation changes, significant refactoring, relevant dependency updates, or impactful build/tooling changes
Applied to files:
CLAUDE.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
🔇 Additional comments (5)
packages/tm-core/src/modules/auth/services/session-manager.ts (2)
39-49: Include error object in initialization debug logPassing the
errorobject into the debug log gives useful context while still keeping it at debug level so it won't spam users. No issues here.
104-111: Treat both null and undefined sessions as “no session”Changing the check to
session != nullensures bothnullandundefinedare treated as “no valid session,” which is safer than only checking againstnull. This matches the intended semantics ofhasValidSession().packages/tm-core/src/modules/auth/managers/auth-manager.ts (1)
162-174: Stronger validation formaxAttemptsinverifyMFAWithRetryThe combined
Number.isFinite/Number.isInteger/maxAttempts < 1guard cleanly rejects NaN, Infinity, non-integers, zero, and negatives, and the updated error message is clear and user-facing. This is a solid improvement.packages/tm-core/src/modules/integration/clients/supabase-client.spec.ts (1)
40-48: Good environment isolation for Supabase config in testsCaching
TM_SUPABASE_URL/TM_SUPABASE_ANON_KEYbefore each test and restoring or deleting them inafterEachkeeps this spec’s env mutations from leaking into other tests. This is a nice, low-friction improvement to test isolation.Also applies to: 78-91
apps/cli/src/commands/auth.command.ts (1)
551-639: Safer MFA factor handling before callingverifyMFAWithRetryAdding the explicit guard on
mfaError.mfaChallenge?.factorIdand then passing the extractedfactorIdintoverifyMFAWithRetryavoids ever calling core with an undefined factor ID and produces a clearMFA_VERIFICATION_FAILEDerror if the challenge data is missing. This makes the MFA flow more robust without changing the happy path.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/tm-core/src/modules/auth/services/session-manager.spec.ts (1)
400-414: Consolidate redundant invocations.The test invokes
authenticateWithCode('test-code')twice—once at line 401 to assert it throws, and again at line 406 to inspect error properties. This is inefficient and could confuse future maintainers.Option 1: Use only the try-catch block with all assertions.
- await expect( - sessionManager.authenticateWithCode('test-code') - ).rejects.toThrow(AuthenticationError); - - // Verify error details by catching it try { await sessionManager.authenticateWithCode('test-code'); + // Fail if no error is thrown + expect.fail('Expected AuthenticationError to be thrown'); } catch (error) { + expect(error).toBeInstanceOf(AuthenticationError); expect((error as AuthenticationError).code).toBe('MFA_REQUIRED'); expect((error as AuthenticationError).mfaChallenge).toEqual({ factorId: 'factor-123', factorType: 'totp' }); }Option 2: Use Vitest's
.rejectswith property matchers.- await expect( - sessionManager.authenticateWithCode('test-code') - ).rejects.toThrow(AuthenticationError); - - // Verify error details by catching it - try { - await sessionManager.authenticateWithCode('test-code'); - } catch (error) { - expect((error as AuthenticationError).code).toBe('MFA_REQUIRED'); - expect((error as AuthenticationError).mfaChallenge).toEqual({ - factorId: 'factor-123', - factorType: 'totp' - }); - } + const promise = sessionManager.authenticateWithCode('test-code'); + + await expect(promise).rejects.toThrow(AuthenticationError); + await expect(promise).rejects.toMatchObject({ + code: 'MFA_REQUIRED', + mfaChallenge: { + factorId: 'factor-123', + factorType: 'totp' + } + });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CLAUDE.md(1 hunks)packages/tm-core/src/modules/auth/services/session-manager.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- CLAUDE.md
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{spec,test}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{spec,test}.ts: Place package tests inpackages/<package-name>/src/<module>/<file>.spec.tsor apps inapps/<app-name>/src/<module>/<file>.spec.tsalongside source, or usepackages/<package-name>/tests/integration/<module>/<file>.test.tsfor integration tests
Always use.tsfor TypeScript tests, never.js
NEVER use async/await in test functions unless testing actual asynchronous operations; use synchronous top-level imports instead of dynamicawait import()
Write tests for bug fixes (add regression test), business logic (complex calculations, validations, transformations), edge cases (boundary conditions, error handling), public APIs, and integration points; skip tests for simple getters/setters, trivial pass-through functions, pure configuration objects, and code that just delegates to another tested function
Files:
packages/tm-core/src/modules/auth/services/session-manager.spec.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
TypeScript test files must achieve minimum code coverage thresholds: 80% lines/functions and 70% branches globally, 90% for utilities, and 85% for middleware; new features must meet or exceed these thresholds
Files:
packages/tm-core/src/modules/auth/services/session-manager.spec.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/*.{js,ts}: Import and use specific getters from config-manager.js (e.g., getMainProvider(), getLogLevel(), getMainMaxTokens()) to access configuration values needed for application logic
Use isApiKeySet(providerName, session) from config-manager.js to check if a provider's key is available before potentially attempting an AI call
Do not add direct console.log calls outside the logging utility - use the central log function instead
Ensure silent mode is disabled in a finally block to prevent it from staying enabled
Do not access the global silentMode variable directly - use the exported silent mode control functions instead
Do not duplicate task ID formatting logic across modules - centralize formatting utilities
Use ContextGatherer class from utils/contextGatherer.js for AI-powered commands that need project context, supporting tasks, files, custom text, and project tree context
Use FuzzyTaskSearch class from utils/fuzzyTaskSearch.js for automatic task relevance detection with configurable search parameters
Use fuzzy search to supplement user-provided task IDs and display discovered task IDs to users for transparency
Do not replace explicit user task selections with fuzzy results - fuzzy search should supplement, not replace user selections
Use readJSON and writeJSON utilities for all JSON file operations instead of raw fs.readFileSync or fs.writeFileSync
Include error handling for JSON file operations and validate JSON structure after reading
Use path.join() for cross-platform path construction and path.resolve() for absolute paths, validating paths before file operations
Support both .env files and MCP session environment for environment variable resolution with fallbacks for missing values
Prefer updating the core function to accept an outputFormat parameter and check outputFormat === 'json' before displaying UI elements
Files:
packages/tm-core/src/modules/auth/services/session-manager.spec.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/git_workflow.mdc:0-0
Timestamp: 2025-11-24T18:00:23.000Z
Learning: Pull Request descriptions must include: Task Overview, Subtasks Completed (checklist), Implementation Details, Testing approach, Breaking Changes (if any), and Related Tasks.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 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.
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.699Z
Learning: Applies to **/*.test.js : When testing CLI commands built with Commander.js: test command action handlers directly rather than mocking the entire Commander chain; create simplified test-specific implementations of command handlers; explicitly handle all options including defaults and shorthand flags; include null/undefined checks for optional parameters; use fixtures for consistent sample data.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1314
File: packages/tm-core/src/auth/credential-store.ts:92-94
Timestamp: 2025-10-15T14:43:40.410Z
Learning: In the claude-task-master codebase, `getCredentials()` in `credential-store.ts` defaults to `allowExpired: true` to enable Supabase refresh flows. The Supabase client automatically handles token refresh when credentials are expired but have a valid refresh token. The `SupabaseSessionStorage` updates the credentials after Supabase performs the refresh. This is an intentional design pattern where the credential store is passive storage and Supabase manages the active token lifecycle.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
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.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1132
File: .github/workflows/weekly-metrics-discord.yml:81-93
Timestamp: 2025-08-13T22:10:46.958Z
Learning: Crunchyman-ralph ignores YAML formatting nitpicks about trailing spaces when there's no project-specific YAML formatter configured, preferring to focus on functionality over cosmetic formatting issues.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1132
File: .github/workflows/weekly-metrics-discord.yml:81-93
Timestamp: 2025-08-13T22:10:46.958Z
Learning: Crunchyman-ralph ignores YAML formatting nitpicks about trailing spaces when there's no project-specific YAML formatter configured, preferring to focus on functionality over cosmetic formatting issues.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1200
File: src/ai-providers/custom-sdk/grok-cli/language-model.js:96-100
Timestamp: 2025-09-19T16:06:42.182Z
Learning: The user Crunchyman-ralph prefers to keep environment variable names explicit (like GROK_CLI_API_KEY) rather than supporting multiple aliases, to avoid overlap and ensure clear separation between different CLI implementations.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1105
File: scripts/modules/supported-models.json:242-254
Timestamp: 2025-08-08T11:33:15.297Z
Learning: Preference: In scripts/modules/supported-models.json, the "name" field is optional. For OpenAI entries (e.g., "gpt-5"), Crunchyman-ralph prefers omitting "name" when the id is explicit enough; avoid nitpicks requesting a "name" in such cases.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1178
File: packages/tm-core/src/subpath-exports.test.ts:6-9
Timestamp: 2025-09-03T12:45:30.724Z
Learning: The user Crunchyman-ralph prefers to avoid overly nitpicky or detailed suggestions in code reviews, especially for test coverage of minor import paths. Focus on more substantial issues rather than comprehensive coverage of all possible edge cases.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1217
File: apps/cli/src/index.ts:16-21
Timestamp: 2025-09-18T16:35:35.147Z
Learning: The user Crunchyman-ralph considers suggestions to export types for better ergonomics (like exporting UpdateInfo type alongside related functions) as nitpicky and prefers not to implement such suggestions.
📚 Learning: 2025-11-24T18:03:13.408Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.408Z
Learning: Use established mocking patterns from auth.test.ts as templates: mock bcrypt with proper TypeScript typing, mock Prisma client with transaction support, and always clear mocks between tests
Applied to files:
packages/tm-core/src/modules/auth/services/session-manager.spec.ts
📚 Learning: 2025-11-24T17:57:14.728Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/ai_providers.mdc:0-0
Timestamp: 2025-11-24T17:57:14.728Z
Learning: Applies to tests/unit/ai-providers/*.test.js : Create unit tests in `tests/unit/ai-providers/<provider-name>.test.js` that mock the provider's AI SDK module and test each exported function for correct client instantiation, parameter passing, result handling, and error handling
Applied to files:
packages/tm-core/src/modules/auth/services/session-manager.spec.ts
📚 Learning: 2025-11-24T18:03:46.700Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.700Z
Learning: Applies to **/*.test.js : Do not import or instantiate real AI service clients. Create fully mocked versions that return predictable responses. Mock the entire module with controlled behavior.
Applied to files:
packages/tm-core/src/modules/auth/services/session-manager.spec.ts
📚 Learning: 2025-11-24T18:03:13.408Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.408Z
Learning: Applies to **/*.test.ts : Unit tests must follow the describe/it pattern, use beforeEach for mock setup with jest.clearAllMocks(), include specific assertions with expect(), and test both success and error scenarios including edge cases
Applied to files:
packages/tm-core/src/modules/auth/services/session-manager.spec.ts
📚 Learning: 2025-11-24T18:03:46.700Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.700Z
Learning: Applies to **/*.test.js : When testing exact error messages, don't assert on exact error message text that might change. Instead test for error presence and general properties using patterns like `toContain` or `toMatch`.
Applied to files:
packages/tm-core/src/modules/auth/services/session-manager.spec.ts
📚 Learning: 2025-11-24T18:03:13.408Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.408Z
Learning: Applies to src/**/*.ts : Source code must have corresponding test files either colocated (*.test.ts) or in tests/unit/ directory following established patterns from src/utils/auth.test.ts with proper mocking for external dependencies
Applied to files:
packages/tm-core/src/modules/auth/services/session-manager.spec.ts
📚 Learning: 2025-11-24T18:03:13.408Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.408Z
Learning: Applies to tests/e2e/**/*.test.ts : End-to-end tests must test complete user workflows across multiple API endpoints in sequence, verify state changes between workflow steps, use extended timeouts (30000ms), and validate final outcomes without mocking business logic
Applied to files:
packages/tm-core/src/modules/auth/services/session-manager.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
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
Bug Fixes
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.