ros/claude-task-master
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(config): Standardize env var access and config getters

Browse Source 
This commit focuses on standardizing configuration and API key access patterns across key modules as part of subtask 61.34.

Key changes include:

- Refactored `ai-services.js` to remove global AI clients and use `resolveEnvVariable` for API key checks. Client instantiation now relies on `getAnthropicClient`/`getPerplexityClient` accepting a session object.

- Refactored `task-manager.js` (`analyzeTaskComplexity` function) to use the unified `generateTextService` from `ai-services-unified.js`, removing direct AI client calls.

- Replaced direct `process.env` access for model parameters and other configurations (`PERPLEXITY_MODEL`, `CONFIG.*`) in `task-manager.js` with calls to the appropriate getters from `config-manager.js` (e.g., `getResearchModelId(session)`, `getMainMaxTokens(session)`).

- Ensured `utils.js` (`resolveEnvVariable`) correctly handles potentially undefined session objects.

- Updated function signatures where necessary to propagate the `session` object for correct context-aware configuration/key retrieval.

This moves towards the goal of using `ai-client-factory.js` and `ai-services-unified.js` as the standard pattern for AI interactions and centralizing configuration management through `config-manager.js`.
This commit is contained in:
Eyal Toledano
2025-04-21 17:48:30 -04:00
parent 538b874582
commit d46547a80f
24 changed files with 5987 additions and 5832 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
tasks/task_061.txt
View File

@@ -1497,7 +1497,39 @@ Implementation notes:
This separation ensures security best practices for credentials while centralizing application configuration for better maintainability.
</info added on 2025-04-20T03:50:25.632Z>
## 35. Review/Refactor MCP Direct Functions for Explicit Config Root Passing [pending]
<info added on 2025-04-20T06:58:36.731Z>
**Plan & Analysis (Added on 2023-05-15T14:32:18.421Z)**:
**Goal:**
1. **Standardize API Key Access**: Ensure all accesses to sensitive API keys (Anthropic, Perplexity, etc.) consistently use a standard function (like `resolveEnvVariable(key, session)`) that checks both `process.env` and `session.env`. Replace direct `process.env.API_KEY` access.
2. **Centralize App Configuration**: Ensure all non-sensitive configuration values (model names, temperature, logging levels, max tokens, etc.) are accessed *only* through `scripts/modules/config-manager.js` getters. Eliminate direct `process.env` access for these.
**Strategy: Inventory -> Analyze -> Target -> Refine**
1. **Inventory (`process.env` Usage):** Performed grep search (`rg "process\.env"`). Results indicate widespread usage across multiple files.
2. **Analysis (Categorization of Usage):**
* **API Keys (Credentials):** ANTHROPIC_API_KEY, PERPLEXITY_API_KEY, OPENAI_API_KEY, etc. found in `task-manager.js`, `ai-services.js`, `commands.js`, `dependency-manager.js`, `ai-client-utils.js`, test files. Needs replacement with `resolveEnvVariable(key, session)`.
* **App Configuration:** PERPLEXITY_MODEL, TEMPERATURE, MAX_TOKENS, MODEL, DEBUG, LOG_LEVEL, DEFAULT_*, PROJECT_*, TASK_MASTER_PROJECT_ROOT found in `task-manager.js`, `ai-services.js`, `scripts/init.js`, `mcp-server/src/logger.js`, `mcp-server/src/tools/utils.js`, test files. Needs replacement with `config-manager.js` getters.
* **System/Environment Info:** HOME, USERPROFILE, SHELL in `scripts/init.js`. Needs review (e.g., `os.homedir()` preference).
* **Test Code/Setup:** Extensive usage in test files. Acceptable for mocking, but code under test must use standard methods. May require test adjustments.
* **Helper Functions/Comments:** Definitions/comments about `resolveEnvVariable`. No action needed.
3. **Target (High-Impact Areas & Initial Focus):**
* High Impact: `task-manager.js` (~5800 lines), `ai-services.js` (~1500 lines).
* Medium Impact: `commands.js`, Test Files.
* Foundational: `ai-client-utils.js`, `config-manager.js`, `utils.js`.
* **Initial Target Command:** `task-master analyze-complexity` for a focused, end-to-end refactoring exercise.
4. **Refine (Plan for `analyze-complexity`):**
a. **Trace Code Path:** Identify functions involved in `analyze-complexity`.
b. **Refactor API Key Access:** Replace direct `process.env.PERPLEXITY_API_KEY` with `resolveEnvVariable(key, session)`.
c. **Refactor App Config Access:** Replace direct `process.env` for model name, temp, tokens with `config-manager.js` getters.
d. **Verify `resolveEnvVariable`:** Ensure robustness, especially handling potentially undefined `session`.
e. **Test:** Verify command works locally and via MCP context (if possible). Update tests.
This piecemeal approach aims to establish the refactoring pattern before tackling the entire codebase.
</info added on 2025-04-20T06:58:36.731Z>
## 35. Review/Refactor MCP Direct Functions for Explicit Config Root Passing [done]
### Dependencies: None
### Description: Review all functions in mcp-server/src/core/direct-functions/*.js. Ensure that any calls made from these functions to getters in scripts/modules/config-manager.js (e.g., getMainProvider, getDefaultPriority, getLogLevel, etc.) explicitly pass the projectRoot (obtained from the args object, which is derived from the session context) as the first argument to the getter. This prevents the getters from incorrectly falling back to using findProjectRoot() based on the server's cwd when running in an MCP context. This is crucial for loading the correct .taskmasterconfig settings based on the user's project.
### Details:

4
tasks/tasks.json
View File

@@ -3106,7 +3106,7 @@
"id": 34,
"title": "Audit and Standardize Env Variable Access",
"description": "Audit the entire codebase (core modules, provider modules, utilities) to ensure all accesses to environment variables (API keys, configuration flags) consistently use a standardized resolution function (like `resolveEnvVariable` or a new utility) that checks `process.env` first and then `session.env` if available. Refactor any direct `process.env` access where `session.env` should also be considered.",
"details": "\n\n<info added on 2025-04-20T03:50:25.632Z>\nThis audit should distinguish between two types of configuration:\n\n1. **Sensitive credentials (API keys)**: These should exclusively use the `resolveEnvVariable` pattern to check both `process.env` and `session.env`. Verify that no API keys are hardcoded or accessed through direct `process.env` references.\n\n2. **Application configuration**: All non-credential settings should be migrated to use the centralized `.taskmasterconfig` system via the `config-manager.js` getters. This includes:\n - Model selections and role assignments\n - Parameter settings (temperature, maxTokens, etc.)\n - Logging configuration\n - Default behaviors and fallbacks\n\nImplementation notes:\n- Create a comprehensive inventory of all environment variable accesses\n- Categorize each as either credential or application configuration\n- For credentials: standardize on `resolveEnvVariable` pattern\n- For app config: migrate to appropriate `config-manager.js` getter methods\n- Document any exceptions that require special handling\n- Add validation to prevent regression (e.g., ESLint rules against direct `process.env` access)\n\nThis separation ensures security best practices for credentials while centralizing application configuration for better maintainability.\n</info added on 2025-04-20T03:50:25.632Z>",
"details": "\n\n<info added on 2025-04-20T03:50:25.632Z>\nThis audit should distinguish between two types of configuration:\n\n1. **Sensitive credentials (API keys)**: These should exclusively use the `resolveEnvVariable` pattern to check both `process.env` and `session.env`. Verify that no API keys are hardcoded or accessed through direct `process.env` references.\n\n2. **Application configuration**: All non-credential settings should be migrated to use the centralized `.taskmasterconfig` system via the `config-manager.js` getters. This includes:\n - Model selections and role assignments\n - Parameter settings (temperature, maxTokens, etc.)\n - Logging configuration\n - Default behaviors and fallbacks\n\nImplementation notes:\n- Create a comprehensive inventory of all environment variable accesses\n- Categorize each as either credential or application configuration\n- For credentials: standardize on `resolveEnvVariable` pattern\n- For app config: migrate to appropriate `config-manager.js` getter methods\n- Document any exceptions that require special handling\n- Add validation to prevent regression (e.g., ESLint rules against direct `process.env` access)\n\nThis separation ensures security best practices for credentials while centralizing application configuration for better maintainability.\n</info added on 2025-04-20T03:50:25.632Z>\n\n<info added on 2025-04-20T06:58:36.731Z>\n**Plan & Analysis (Added on 2023-05-15T14:32:18.421Z)**:\n\n**Goal:**\n1. **Standardize API Key Access**: Ensure all accesses to sensitive API keys (Anthropic, Perplexity, etc.) consistently use a standard function (like `resolveEnvVariable(key, session)`) that checks both `process.env` and `session.env`. Replace direct `process.env.API_KEY` access.\n2. **Centralize App Configuration**: Ensure all non-sensitive configuration values (model names, temperature, logging levels, max tokens, etc.) are accessed *only* through `scripts/modules/config-manager.js` getters. Eliminate direct `process.env` access for these.\n\n**Strategy: Inventory -> Analyze -> Target -> Refine**\n\n1. **Inventory (`process.env` Usage):** Performed grep search (`rg \"process\\.env\"`). Results indicate widespread usage across multiple files.\n2. **Analysis (Categorization of Usage):**\n * **API Keys (Credentials):** ANTHROPIC_API_KEY, PERPLEXITY_API_KEY, OPENAI_API_KEY, etc. found in `task-manager.js`, `ai-services.js`, `commands.js`, `dependency-manager.js`, `ai-client-utils.js`, test files. Needs replacement with `resolveEnvVariable(key, session)`.\n * **App Configuration:** PERPLEXITY_MODEL, TEMPERATURE, MAX_TOKENS, MODEL, DEBUG, LOG_LEVEL, DEFAULT_*, PROJECT_*, TASK_MASTER_PROJECT_ROOT found in `task-manager.js`, `ai-services.js`, `scripts/init.js`, `mcp-server/src/logger.js`, `mcp-server/src/tools/utils.js`, test files. Needs replacement with `config-manager.js` getters.\n * **System/Environment Info:** HOME, USERPROFILE, SHELL in `scripts/init.js`. Needs review (e.g., `os.homedir()` preference).\n * **Test Code/Setup:** Extensive usage in test files. Acceptable for mocking, but code under test must use standard methods. May require test adjustments.\n * **Helper Functions/Comments:** Definitions/comments about `resolveEnvVariable`. No action needed.\n3. **Target (High-Impact Areas & Initial Focus):**\n * High Impact: `task-manager.js` (~5800 lines), `ai-services.js` (~1500 lines).\n * Medium Impact: `commands.js`, Test Files.\n * Foundational: `ai-client-utils.js`, `config-manager.js`, `utils.js`.\n * **Initial Target Command:** `task-master analyze-complexity` for a focused, end-to-end refactoring exercise.\n\n4. **Refine (Plan for `analyze-complexity`):**\n a. **Trace Code Path:** Identify functions involved in `analyze-complexity`.\n b. **Refactor API Key Access:** Replace direct `process.env.PERPLEXITY_API_KEY` with `resolveEnvVariable(key, session)`.\n c. **Refactor App Config Access:** Replace direct `process.env` for model name, temp, tokens with `config-manager.js` getters.\n d. **Verify `resolveEnvVariable`:** Ensure robustness, especially handling potentially undefined `session`.\n e. **Test:** Verify command works locally and via MCP context (if possible). Update tests.\n\nThis piecemeal approach aims to establish the refactoring pattern before tackling the entire codebase.\n</info added on 2025-04-20T06:58:36.731Z>",
"status": "pending",
"dependencies": [],
"parentTaskId": 61
@@ -3116,7 +3116,7 @@
"title": "Review/Refactor MCP Direct Functions for Explicit Config Root Passing",
"description": "Review all functions in mcp-server/src/core/direct-functions/*.js. Ensure that any calls made from these functions to getters in scripts/modules/config-manager.js (e.g., getMainProvider, getDefaultPriority, getLogLevel, etc.) explicitly pass the projectRoot (obtained from the args object, which is derived from the session context) as the first argument to the getter. This prevents the getters from incorrectly falling back to using findProjectRoot() based on the server's cwd when running in an MCP context. This is crucial for loading the correct .taskmasterconfig settings based on the user's project.",
"details": "",
"status": "pending",
"status": "done",
"dependencies": [],
"parentTaskId": 61
}