Conversation
|
WalkthroughAdds MFA detection and routed verification into the OAuth and CLI auth flows, expands invitation statuses to include "already_invited" and updates CLI export invite/display flow, and extracts a shared org-selection utility used by multiple CLI commands. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Auth Command
participant OAuth as OAuth Service
participant Supabase as Supabase Client
participant MFA as MFA Handler
CLI->>OAuth: authenticateWithBrowser() / authenticateWithToken(token)
OAuth->>Supabase: obtain/reconstruct session
Supabase-->>OAuth: session (may include mfa state)
OAuth->>OAuth: checkAndThrowIfMFARequired()
alt MFA required with factor info
OAuth-->>CLI: throw MFA_REQUIRED (mfaChallenge)
CLI->>MFA: handleMFAVerification(mfaChallenge)
MFA->>Supabase: verify factor / complete flow
Supabase-->>CLI: authenticated session
else MFA required but incomplete
OAuth-->>CLI: throw MFA_REQUIRED_INCOMPLETE
CLI->>CLI: show error / abort MFA flow
else No MFA
OAuth-->>CLI: authenticated session
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 (4)
🧰 Additional context used📓 Path-based instructions (7)**/*.ts📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
Files:
**/*.{js,ts}📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
apps/cli/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/{utils,utilities,helpers}/**/*.{js,ts}📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
Files:
**/{utils,utilities}/*.{js,ts}📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
Files:
**/{utils,utilities}/**/*.{js,ts}📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
Files:
🧠 Learnings (12)📓 Common learnings📚 Learning: 2025-11-24T17:58:47.030ZApplied to files:
📚 Learning: 2025-07-18T17:08:48.695ZApplied to files:
📚 Learning: 2025-11-24T18:04:01.629ZApplied to files:
📚 Learning: 2025-11-24T18:01:44.169ZApplied to files:
📚 Learning: 2025-11-24T17:59:00.056ZApplied to files:
📚 Learning: 2025-11-24T18:01:44.169ZApplied to files:
📚 Learning: 2025-11-24T18:04:43.972ZApplied to files:
📚 Learning: 2025-11-24T22:09:45.455ZApplied to files:
📚 Learning: 2025-11-24T18:04:43.972ZApplied to files:
📚 Learning: 2025-11-26T20:50:40.810ZApplied to files:
📚 Learning: 2025-10-08T19:57:00.982ZApplied to files:
🧬 Code graph analysis (3)apps/cli/src/commands/context.command.ts (2)
apps/cli/src/commands/export.command.ts (1)
apps/cli/src/commands/briefs.command.ts (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). (3)
🔇 Additional comments (18)
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)
scripts/modules/commands.js (1)
483-499: Align invitation status handling with ExportCommand for consistencyAdding explicit handling for
already_invitedhere is good, but this block now diverges fromExportCommand.displayInvitationResults(different color/text and noerrorstatus branch). Consider mirroring that helper’s mapping (includingerror) so both CLI paths present invitation outcomes consistently.apps/cli/src/commands/export.command.ts (1)
921-941: Newalready_invitedstatus handling is correct; consider cross-CLI consistencyThe new
already_invitedbranch cleanly covers the expanded invitation status and the “pending” wording is helpful. To avoid confusing users, you may want to align the text/color with the similar handling inscripts/modules/commands.jsso both flows describe this status the same way.packages/tm-core/src/modules/auth/services/oauth-service.ts (1)
129-129: MFA gating is integrated cleanly; verify partial-session semantics are safeCentralizing MFA checks in
checkAndThrowIfMFARequiredand invoking it in both the PKCE and direct token paths is a good way to ensure OAuth flows honor MFA requirements and surface structuredAuthenticationErrors (withMFA_REQUIRED/MFA_REQUIRED_INCOMPLETEplusMFAChallenge).One thing to double‑check: in both flows you (a) establish the Supabase session and (b) save basic user context before calling
checkAndThrowIfMFARequired. IfcheckMFARequired()reports MFA is required and you throw, you’ll end up with stored session/identity but an error at the call site. That’s fine as long as the rest of the system consistently treats these MFA errors as “not fully authenticated” and never relies solely on the stored session/context to consider the user logged in. If that guarantee isn’t already enforced elsewhere, consider either:
- Deferring any durable session/context persistence until after the MFA check passes, or
- Explicitly clearing/signing out the session and rolling back related context before throwing on
MFA_REQUIRED/MFA_REQUIRED_INCOMPLETE.This would avoid any chance of a “half‑authenticated” state being accidentally treated as fully authenticated.
Also applies to: 285-320, 351-397, 441-478
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/cli/src/commands/auth.command.ts(5 hunks)apps/cli/src/commands/export.command.ts(1 hunks)packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts(1 hunks)packages/tm-core/src/modules/auth/services/oauth-service.ts(5 hunks)packages/tm-core/src/modules/auth/types.ts(1 hunks)packages/tm-core/src/modules/integration/services/export.service.ts(3 hunks)scripts/modules/commands.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
scripts/modules/commands.js
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
commands.js should parse command-line arguments and options, invoke appropriate core logic functions from scripts/modules/, handle user input/output for CLI, and implement CLI-specific validation
scripts/modules/commands.js: Follow the basic command template structure when implementing CLI commands: use.command(),.description(),.option(), and.action()in Commander.js with concise action handlers that extract functionality to core modules
Keep command action handlers concise and focused; extract core functionality to appropriate modules like task-manager.js or init.js rather than implementing business logic in handlers
Use kebab-case for command names (e.g., 'analyze-complexity', 'remove-task') and action-oriented descriptions
Use kebab-case for long-form option names (e.g., --output-format) with single-letter shortcuts when appropriate (e.g., -f, --file), and access them in code as camelCase properties (e.g., options.numTasks)
Use positive flags with --skip- prefix for disabling behavior instead of --no- prefix negated flags; use clear variable naming likeconst generateFiles = !options.skipGenerateto avoid double negatives
Include confirmation prompts by default for destructive operations (delete/remove commands) with a --yes or -y flag to skip confirmation; display what will be deleted in the confirmation message
For file path handling in destructive operations: use path.join() to construct paths, follow naming conventions (e.g., task_001.txt), check file existence before deletion, and handle errors gracefully without string concatenation
Clean up references to deleted items in other parts of the data after destructive operations, handling both direct and indirect references with explanatory console logging
Regenerate task files after destructive operations by explicitly passing all required parameters to generation functions; provide a --skip-generate option if needed
Suggest non-destructive alternatives (like status changes instead of deletion)...
Files:
scripts/modules/commands.js
**/*.js
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
**/*.js: Always use isSilentMode() function to check current silent mode status instead of directly accessing the global silentMode variable or global.silentMode
Use try/finally block pattern when wrapping core function calls with enableSilentMode/disableSilentMode to ensure silent mode is always restored, even if errors occur
For functions that need to handle both a passed silentMode parameter and check global state, check both the function parameter and global state: const isSilent = options.silentMode || (typeof options.silentMode === 'undefined' && isSilentMode())
Functions should accept their dependencies as parameters rather than using globals to promote testability and explicit dependency injection
Define callbacks as separate functions for easier testing rather than inline functions
Files:
scripts/modules/commands.js
scripts/**/*.js
📄 CodeRabbit inference engine (.cursor/rules/context_gathering.mdc)
scripts/**/*.js: Use theContextGathererclass fromscripts/modules/utils/contextGatherer.jsto extract context from multiple sources (tasks, files, custom text, project tree) with token counting usinggpt-tokenslibrary
InitializeContextGathererwith project root and tasks path, then callgather()method with tasks array, files array, customContext, includeProjectTree, format ('research', 'chat', or 'system-prompt'), and includeTokenCounts options
Use theFuzzyTaskSearchclass fromscripts/modules/utils/fuzzyTaskSearch.jsfor intelligent task discovery with semantic matching, purpose categorization, and relevance scoring using Fuse.js
Implement a three-step initialization pattern for context-aware commands: (1) validate and parse parameters, (2) initialize context gatherer and find project root, (3) auto-discover relevant tasks using fuzzy search if task IDs not specified
Display token breakdown usingboxenlibrary with sections for tasks, files, and prompts, showing formatted token counts and file sizes in a clean bordered box with title
Process AI result responses usingcli-highlightlibrary to apply syntax highlighting to code blocks with language detection in the formatlanguage\ncode
Set reasonable file size limits (50KB default) and project tree depth limits (3-5 levels) when gathering context to maintain performance
Implement graceful error handling for context gathering: handle missing files with warnings, validate task IDs with helpful messages, continue processing if some context sources fail, and provide fallback behavior
Files:
scripts/modules/commands.js
scripts/modules/**/*
📄 CodeRabbit inference engine (.cursor/rules/dev_workflow.mdc)
Restart the MCP server if core logic in
scripts/modulesor MCP tool definitions change
Files:
scripts/modules/commands.js
scripts/modules/*.js
📄 CodeRabbit inference engine (.cursor/rules/mcp.mdc)
When implementing MCP support for a command, ensure the core logic function can suppress console output via an outputFormat parameter or other mechanism
scripts/modules/*.js: Use consistent file naming conventions:task_${id.toString().padStart(3, '0')}.txtfor task files; usepath.join()for composing paths; use appropriate extensions (.txt for tasks, .json for data)
Export all core functions, helper functions, and utility methods needed by dependent code from their respective modules; explicitly verify module export blocks at the bottom of files
Use structured error objects with code and message properties; include clear error messages; handle both function-specific and file system errors; log errors at appropriate severity levels
UseisSilentMode()function to check global silent mode status; wrap core function calls within direct functions usingenableSilentMode()anddisableSilentMode()in try/finally blocks if the core function produces console output not reliably controlled by outputFormat parameter
Ensure AI calls correctly handle and propagatetelemetryDataas described intelemetry.mdc
Import context gathering utilities (ContextGatherer,FuzzyTaskSearch) for AI-powered commands; support multiple context types (tasks, files, custom text, project tree); implement detailed token breakdown display
PrefergenerateTextServicefor calls sending large context (like stringified JSON) where incremental display is not needed; import necessary service functions fromai-services-unified.jsand prepare parameters (role, session, systemPrompt, prompt)
Create a clear unidirectional flow of dependencies between modules; separate business logic from UI rendering to avoid circular dependencies
Design functions to accept dependencies as parameters; avoid hard-coded dependencies that are difficult to mock
Keep pure logic separate from I/O operations or UI rendering to allow testing logic without mocking complex dependencies
Design core logic to work wi...
Files:
scripts/modules/commands.js
**/*.{js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
JavaScript test files using Jest must follow the same testing patterns as TypeScript files, include proper mocking of external dependencies, and achieve the same coverage thresholds
Files:
scripts/modules/commands.js
**/*.{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:
scripts/modules/commands.jsapps/cli/src/commands/export.command.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/auth/types.tspackages/tm-core/src/modules/integration/services/export.service.tsapps/cli/src/commands/auth.command.tspackages/tm-core/src/modules/auth/services/oauth-service.ts
scripts/modules/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
scripts/modules/**/*.{js,ts}: Implement silent migration for tasks.json files that transforms old format to tagged format, marking global flag and performing complete migration
Implement tag resolution functions (getTasksForTag, setTasksForTag, getCurrentTag) that provide backward compatibility with legacy format and default to master tag
Implement complete migration functions for tagged task lists that handle configuration, state file creation, and migration status tracking
When a logger object is passed as a parameter to core functions, ensure the receiving function can call methods like .info, .warn, .error on that object
Files:
scripts/modules/commands.js
**/*.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:
apps/cli/src/commands/export.command.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/auth/types.tspackages/tm-core/src/modules/integration/services/export.service.tsapps/cli/src/commands/auth.command.tspackages/tm-core/src/modules/auth/services/oauth-service.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Import modules with
.jsextension even in TypeScript source files for ESM compatibility
Files:
apps/cli/src/commands/export.command.tspackages/tm-core/src/modules/auth/managers/auth-manager.spec.tspackages/tm-core/src/modules/auth/types.tspackages/tm-core/src/modules/integration/services/export.service.tsapps/cli/src/commands/auth.command.tspackages/tm-core/src/modules/auth/services/oauth-service.ts
apps/cli/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
CLI (@tm/cli) should be a thin presentation layer that calls tm-core methods and displays results; handle only CLI-specific concerns like argument parsing, output formatting, and user prompts
Files:
apps/cli/src/commands/export.command.tsapps/cli/src/commands/auth.command.ts
**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place package and app test files in
packages/<package-name>/src/<module>/<file>.spec.tsorapps/<app-name>/src/<module>/<file>.spec.tsalongside source files
Files:
packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts
**/*.{spec,test}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{spec,test}.ts: 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()
Files:
packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts
packages/tm-core/**/*.{spec,test}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
In unit tests for @tm/core, mock only external I/O (Supabase, APIs, filesystem) and use real internal services
Files:
packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts
🧠 Learnings (13)
📓 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.032Z
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: 1444
File: apps/cli/src/utils/auto-update/changelog.ts:103-111
Timestamp: 2025-11-25T18:32:29.828Z
Learning: The claude-task-master project uses a custom changelog format with PR numbers and author acknowledgements in the pattern `- [#PR](...) Thanks [author]! - Description`, which is parsed by the regex in apps/cli/src/utils/auto-update/changelog.ts.
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: 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: 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: 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: 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: 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:01:44.169Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-11-24T18:01:44.169Z
Learning: Applies to scripts/modules/commands.js : All new user-facing commands should be added to `commands.js` following Commander.js patterns for subcommand structure and consistent option naming
Applied to files:
scripts/modules/commands.js
📚 Learning: 2025-07-18T17:08:48.695Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-18T17:08:48.695Z
Learning: Applies to scripts/modules/commands.js : Suggest non-destructive alternatives when appropriate, explain the difference between deletion and status changes, and include examples of alternative commands.
Applied to files:
scripts/modules/commands.js
📚 Learning: 2025-07-20T01:35:05.831Z
Learnt from: rtmcrc
Repo: eyaltoledano/claude-task-master PR: 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-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
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/managers/auth-manager.spec.ts
📚 Learning: 2025-11-24T22:09:45.455Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T22:09:45.455Z
Learning: Applies to packages/tm-core/**/*.{spec,test}.ts : In unit tests for tm/core, mock only external I/O (Supabase, APIs, filesystem) and use real internal services
Applied to files:
packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts
📚 Learning: 2025-11-24T18:03:46.713Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.713Z
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/managers/auth-manager.spec.ts
📚 Learning: 2025-11-24T22:09:45.455Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T22:09:45.455Z
Learning: Applies to **/tests/integration/**/*.{spec,test}.ts : In integration tests, use real tm-core and mock only external boundaries (APIs, DB, filesystem)
Applied to files:
packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts
📚 Learning: 2025-11-24T18:03:13.456Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.456Z
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.ts
📚 Learning: 2025-11-24T22:09:45.455Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T22:09:45.455Z
Learning: Applies to apps/mcp/**/*.{spec,test}.ts : In unit tests for apps/mcp, mock tm-core responses but use real MCP framework to test response formatting
Applied to files:
packages/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/managers/auth-manager.spec.ts
📚 Learning: 2025-11-24T18:03:46.713Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.713Z
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/auth/managers/auth-manager.spec.ts
📚 Learning: 2025-10-15T14:43:40.410Z
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.
Applied to files:
packages/tm-core/src/modules/auth/services/oauth-service.ts
🧬 Code graph analysis (2)
apps/cli/src/commands/auth.command.ts (1)
packages/tm-core/src/modules/auth/types.ts (1)
AuthenticationError(126-143)
packages/tm-core/src/modules/auth/services/oauth-service.ts (1)
packages/tm-core/src/modules/auth/types.ts (3)
AuthCredentials(5-14)AuthenticationError(126-143)MFAChallenge(67-72)
⏰ 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). (2)
- GitHub Check: Test
- GitHub Check: Test
🔇 Additional comments (13)
packages/tm-core/src/modules/auth/managers/auth-manager.spec.ts (1)
75-87: SupabaseAuthClient singleton mock matches production patternThe module-scoped
instanceplus staticgetInstance/resetInstanceis a clean way to mirror the real singleton behavior in tests without side effects. Looks good and keeps the mock aligned with the implementation.packages/tm-core/src/modules/auth/types.ts (1)
91-121: MFA_REQUIRED_INCOMPLETE code cleanly extends AuthErrorCode unionAdding
MFA_REQUIRED_INCOMPLETEhere is consistent with the new OAuth MFA handling and keeps error typing centralized; no issues from a typing or compatibility standpoint.packages/tm-core/src/modules/auth/services/oauth-service.ts (1)
9-22: Type-only imports for Supabase and auth types are a good refinementSwitching Session, SupabaseAuthClient, ContextStore, and the various Auth* interfaces to
import typekeeps the runtime module surface minimal without changing behavior, which is a solid TS/ESM hygiene improvement.packages/tm-core/src/modules/integration/services/export.service.ts (3)
102-106: LGTM! Newalready_invitedstatus properly added to the base type.The
InvitationResulttype is correctly extended to distinguish between users who are already team members (already_member) and users who have a pending invitation (already_invited). This type is reused byGenerateBriefFromPrdResponse,GenerateBriefFromPrdResult,GenerateBriefResponse, andGenerateBriefResult.
236-242: LGTM! Status union consistent with API contract.The
SendTeamInvitationsResponsetype correctly includesalready_invitedfor handling pending invitation scenarios in the API response.
247-257: LGTM! Result type aligned with response type.The
SendTeamInvitationsResultstatus union is now consistent withSendTeamInvitationsResponse, enabling proper propagation of thealready_invitedstatus through the service layer.apps/cli/src/commands/auth.command.ts (7)
155-159: LGTM! Parameter default is appropriate.The
showHeader = truedefault maintains backward compatibility while allowing callers to suppress the banner when needed.
394-397: LGTM! Signature consistent withexecuteLogin.
498-502: LGTM! Documentation accurately reflects the MFA capability.
569-600: LGTM! Token auth MFA handling mirrors browser auth pattern.The MFA handling in
authenticateWithTokenfollows the same defensive pattern asauthenticateWithBrowser:
- Early check for
MFA_REQUIREDbefore showing failure- Validation of
mfaChallenge.factorId- Delegation to shared
handleMFAVerificationUsing
spinner.stop()(no argument) is correct for ora spinners, unlike the countdown timer.
602-620: LGTM! Shared MFA verification handler with proper context binding.The
handleMFAVerificationmethod correctly:
- Provides defensive validation (useful if called from additional locations in future)
- Binds
verifyMFAWithRetrytoauthManagerto preservethiscontext- Delegates to the shared
handleMFAFlowutility fromauth-ui.ts
625-629: LGTM! Parameter default consistent across auth methods.
538-563:AuthCountdownTimer.stop('mfa')is valid and well-supported.The MFA detection and delegation logic is correctly implemented:
- Checks for
MFA_REQUIREDbefore generic failure handling- Validates
factorIdpresence defensively- Correctly calls
countdownTimer.stop('mfa')— this is an explicitly supported state in theAuthCountdownTimerclass with proper handling (stops spinner without success/failure message)- Uses shared
handleMFAVerificationfor consistency with token auth flowfinallyblock ensurescountdownTimer.cleanup()always runs
…1472) Co-authored-by: Ralph Khreish <35776126+Crunchyman-ralph@users.noreply.github.com>
3 participants
What type of PR is this?
Description
Related Issues
How to Test This
# Example commands or stepsExpected result:
Contributor Checklist
npm run changesetnpm testnpm run format-check(ornpm run formatto fix)Changelog Entry
For Maintainers
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.