|
| 1 | +# Codex MCP CLI Enhancement - Analysis & Recommendation Report |
| 2 | + |
| 3 | +**Date**: December 15, 2025 |
| 4 | +**Author**: Augment Agent |
| 5 | +**Status**: Analysis Complete - Ready for Implementation |
| 6 | + |
| 7 | +--- |
| 8 | + |
| 9 | +## Executive Summary |
| 10 | + |
| 11 | +**Recommendation**: Add 6 new CLI arguments and reuse 4 existing arguments to support all 10 Codex-specific configuration fields. |
| 12 | + |
| 13 | +**Rationale**: This approach follows the existing codebase pattern of shared arguments across hosts, provides clear semantics, requires simple implementation, and introduces no breaking changes. |
| 14 | + |
| 15 | +**Implementation Complexity**: LOW (~100-150 lines of code) |
| 16 | +**Risk Level**: VERY LOW (purely additive changes) |
| 17 | + |
| 18 | +--- |
| 19 | + |
| 20 | +## 1. Existing CLI Architecture Analysis |
| 21 | + |
| 22 | +### 1.1 Current Structure |
| 23 | + |
| 24 | +**Main Command**: `hatch mcp configure` |
| 25 | + |
| 26 | +**Argument Flow**: |
| 27 | +1. Arguments parsed by argparse in `setup_mcp_configure_parser()` |
| 28 | +2. Passed to `handle_mcp_configure()` handler |
| 29 | +3. Mapped to `omni_config_data` dictionary |
| 30 | +4. Used to create `MCPServerConfigOmni` instance |
| 31 | +5. Converted to host-specific model via `from_omni()` |
| 32 | + |
| 33 | +**Location**: `hatch/cli_hatch.py` (lines 1571-1654 for parser, 783-850 for handler) |
| 34 | + |
| 35 | +### 1.2 Existing CLI Arguments by Category |
| 36 | + |
| 37 | +**Universal Arguments** (all hosts): |
| 38 | +- `--command` → command (str) |
| 39 | +- `--url` → url (str) |
| 40 | +- `--args` → args (List[str], action="append") |
| 41 | +- `--env-var` → env (Dict[str, str], KEY=VALUE format) |
| 42 | +- `--header` → headers (Dict[str, str], KEY=VALUE format) |
| 43 | + |
| 44 | +**Gemini-Specific Arguments**: |
| 45 | +- `--timeout` → timeout (int, milliseconds) |
| 46 | +- `--trust` → trust (bool) |
| 47 | +- `--cwd` → cwd (str) |
| 48 | +- `--http-url` → httpUrl (str) |
| 49 | +- `--include-tools` → includeTools (List[str]) |
| 50 | +- `--exclude-tools` → excludeTools (List[str]) |
| 51 | + |
| 52 | +**Cursor/VS Code/LM Studio Arguments**: |
| 53 | +- `--env-file` → envFile (str) |
| 54 | + |
| 55 | +**VS Code Arguments**: |
| 56 | +- `--input` → input (str) |
| 57 | + |
| 58 | +**Kiro Arguments**: |
| 59 | +- `--disabled` → disabled (bool) |
| 60 | +- `--auto-approve-tools` → autoApprove (List[str]) |
| 61 | +- `--disable-tools` → disabledTools (List[str]) |
| 62 | + |
| 63 | +### 1.3 Argument Patterns |
| 64 | + |
| 65 | +**List Arguments**: Use `action="append"` (e.g., `--args`, `--include-tools`) |
| 66 | +**Dict Arguments**: Use `action="append"` with KEY=VALUE parsing (e.g., `--env-var`, `--header`) |
| 67 | +**Boolean Flags**: Use `action="store_true"` (e.g., `--trust`, `--disabled`) |
| 68 | +**String/Int Arguments**: Use `type=str` or `type=int` (e.g., `--command`, `--timeout`) |
| 69 | + |
| 70 | +--- |
| 71 | + |
| 72 | +## 2. Codex Field Mapping Analysis |
| 73 | + |
| 74 | +### 2.1 Complete Field Inventory |
| 75 | + |
| 76 | +**Codex Fields in MCPServerConfigOmni** (lines 680-689): |
| 77 | +1. `env_vars` - List[str] - Environment variable names to whitelist |
| 78 | +2. `startup_timeout_sec` - int - Server startup timeout in seconds |
| 79 | +3. `tool_timeout_sec` - int - Tool execution timeout in seconds |
| 80 | +4. `enabled` - bool - Enable/disable server without deleting |
| 81 | +5. `enabled_tools` - List[str] - Tool allow-list |
| 82 | +6. `disabled_tools` - List[str] - Tool deny-list |
| 83 | +7. `bearer_token_env_var` - str - Env var name for bearer token |
| 84 | +8. `http_headers` - Dict[str, str] - Static HTTP headers |
| 85 | +9. `env_http_headers` - Dict[str, str] - Headers from env vars |
| 86 | +10. `cwd` - str - Working directory (already in Omni as Gemini field) |
| 87 | + |
| 88 | +### 2.2 Field-to-Argument Mapping Table |
| 89 | + |
| 90 | +| # | Codex Field | Type | Existing CLI Arg | New CLI Arg | Decision | Rationale | |
| 91 | +|---|-------------|------|------------------|-------------|----------|-----------| |
| 92 | +| 1 | cwd | str | --cwd | - | **REUSE** | Already exists for Gemini, exact same semantics | |
| 93 | +| 2 | env_vars | List[str] | - | --env-vars | **NEW** | Different from --env-var (names vs values) | |
| 94 | +| 3 | startup_timeout_sec | int | - | --startup-timeout | **NEW** | Different from --timeout (purpose & units) | |
| 95 | +| 4 | tool_timeout_sec | int | - | --tool-timeout | **NEW** | No existing equivalent | |
| 96 | +| 5 | enabled | bool | - | --enabled | **NEW** | Inverse of --disabled (different host) | |
| 97 | +| 6 | enabled_tools | List[str] | --include-tools | - | **REUSE** | Exact same semantics as Gemini | |
| 98 | +| 7 | disabled_tools | List[str] | --exclude-tools | - | **REUSE** | Exact same semantics as Gemini | |
| 99 | +| 8 | bearer_token_env_var | str | - | --bearer-token-env-var | **NEW** | No existing equivalent | |
| 100 | +| 9 | http_headers | Dict[str,str] | --header | - | **REUSE** | Exact same semantics (universal) | |
| 101 | +| 10 | env_http_headers | Dict[str,str] | - | --env-header | **NEW** | Different from --header (env vars) | |
| 102 | + |
| 103 | +**Summary**: 4 reused, 6 new arguments |
| 104 | + |
| 105 | + |
| 106 | + |
| 107 | + |
| 108 | +--- |
| 109 | + |
| 110 | +## 3. Semantic Analysis Details |
| 111 | + |
| 112 | +### 3.1 Fields Requiring New Arguments |
| 113 | + |
| 114 | +**1. env_vars (NEW: --env-vars)** |
| 115 | +- **Codex semantics**: List of environment variable NAMES to whitelist/forward |
| 116 | +- **Existing --env-var**: Sets KEY=VALUE pairs (Dict[str, str]) |
| 117 | +- **Why different**: env_vars whitelists which vars to forward, env-var sets values |
| 118 | +- **Example**: `--env-vars PATH --env-vars HOME` (forward PATH and HOME from parent env) |
| 119 | + |
| 120 | +**2. startup_timeout_sec (NEW: --startup-timeout)** |
| 121 | +- **Codex semantics**: Timeout in seconds for server startup (default: 10) |
| 122 | +- **Existing --timeout**: Gemini request timeout in milliseconds |
| 123 | +- **Why different**: Different purpose (startup vs request) and units (seconds vs milliseconds) |
| 124 | +- **Example**: `--startup-timeout 15` (wait 15 seconds for server to start) |
| 125 | + |
| 126 | +**3. tool_timeout_sec (NEW: --tool-timeout)** |
| 127 | +- **Codex semantics**: Timeout in seconds for tool execution (default: 60) |
| 128 | +- **No existing equivalent** |
| 129 | +- **Example**: `--tool-timeout 120` (allow 2 minutes for tool execution) |
| 130 | + |
| 131 | +**4. enabled (NEW: --enabled)** |
| 132 | +- **Codex semantics**: Boolean to enable/disable server without deleting config |
| 133 | +- **Existing --disabled**: Kiro-specific disable flag |
| 134 | +- **Why different**: Different hosts, inverse semantics (enabled vs disabled) |
| 135 | +- **Example**: `--enabled` (enable the server) |
| 136 | + |
| 137 | +**5. bearer_token_env_var (NEW: --bearer-token-env-var)** |
| 138 | +- **Codex semantics**: Name of env var containing bearer token for Authorization header |
| 139 | +- **No existing equivalent** |
| 140 | +- **Example**: `--bearer-token-env-var FIGMA_OAUTH_TOKEN` |
| 141 | + |
| 142 | +**6. env_http_headers (NEW: --env-header)** |
| 143 | +- **Codex semantics**: Map of header names to env var names (values pulled from env) |
| 144 | +- **Existing --header**: Sets static header values |
| 145 | +- **Why different**: env-header pulls values from environment, header uses static values |
| 146 | +- **Example**: `--env-header X-API-Key=API_KEY_VAR` (header value from $API_KEY_VAR) |
| 147 | + |
| 148 | +### 3.2 Fields Reusing Existing Arguments |
| 149 | + |
| 150 | +**1. cwd (REUSE: --cwd)** |
| 151 | +- **Shared by**: Gemini, Codex |
| 152 | +- **Semantics**: Working directory to launch server from |
| 153 | +- **Already supported**: Yes (line 1614 in cli_hatch.py) |
| 154 | + |
| 155 | +**2. enabled_tools (REUSE: --include-tools)** |
| 156 | +- **Shared by**: Gemini (includeTools), Codex (enabled_tools) |
| 157 | +- **Semantics**: Allow-list of tools to expose from server |
| 158 | +- **Already supported**: Yes (line 1618 in cli_hatch.py) |
| 159 | + |
| 160 | +**3. disabled_tools (REUSE: --exclude-tools)** |
| 161 | +- **Shared by**: Gemini (excludeTools), Codex (disabled_tools) |
| 162 | +- **Semantics**: Deny-list of tools to hide |
| 163 | +- **Already supported**: Yes (line 1622 in cli_hatch.py) |
| 164 | + |
| 165 | +**4. http_headers (REUSE: --header)** |
| 166 | +- **Shared by**: All hosts (universal) |
| 167 | +- **Semantics**: Static HTTP headers |
| 168 | +- **Already supported**: Yes (line 1590 in cli_hatch.py) |
| 169 | + |
| 170 | +--- |
| 171 | + |
| 172 | +## 4. Alias Feasibility Assessment |
| 173 | + |
| 174 | +### 4.1 Argparse Alias Capabilities |
| 175 | + |
| 176 | +**Native Support**: argparse does NOT support argument aliases natively |
| 177 | + |
| 178 | +**Available Options**: |
| 179 | +1. Multiple flags for same dest: `parser.add_argument('-v', '--verbose', dest='verbose')` |
| 180 | +2. Post-processing: Check multiple args and merge |
| 181 | +3. Custom action classes: Complex, not worth it |
| 182 | + |
| 183 | +### 4.2 Current Codebase Pattern |
| 184 | + |
| 185 | +**Pattern Used**: Shared arguments across hosts |
| 186 | +- Same CLI argument name used by multiple hosts |
| 187 | +- Example: `--include-tools` used by both Gemini and Codex |
| 188 | +- Mapping happens in omni_config_data dictionary |
| 189 | +- Host-specific models extract relevant fields via `from_omni()` |
| 190 | + |
| 191 | +**Why This Works**: |
| 192 | +- MCPServerConfigOmni contains ALL fields from ALL hosts |
| 193 | +- Each host's `from_omni()` extracts only supported fields |
| 194 | +- CLI doesn't need to know which host uses which field |
| 195 | +- Simple, clean, maintainable |
| 196 | + |
| 197 | +### 4.3 Conclusion |
| 198 | + |
| 199 | +**No aliases needed**. The existing shared argument pattern is simpler and more maintainable. |
| 200 | + |
| 201 | +--- |
| 202 | + |
| 203 | +## 5. Trade-off Analysis |
| 204 | + |
| 205 | +### 5.1 Chosen Approach: 6 New + 4 Reused Arguments |
| 206 | + |
| 207 | +**Benefits**: |
| 208 | +1. ✅ Clear semantics - each argument name matches its purpose |
| 209 | +2. ✅ Follows existing pattern - shared args across hosts |
| 210 | +3. ✅ Simple implementation - just add new arguments to parser |
| 211 | +4. ✅ No breaking changes - existing args continue to work |
| 212 | +5. ✅ Type safety - argparse handles validation |
| 213 | +6. ✅ Easy to document - clear help text for each arg |
| 214 | +7. ✅ Maintainable - no complex logic or workarounds |
| 215 | + |
| 216 | +**Costs**: |
| 217 | +1. ⚠️ More total CLI options (6 new args) |
| 218 | +2. ⚠️ Slightly longer help text |
| 219 | +3. ⚠️ Users need to learn new args for Codex features |
| 220 | + |
| 221 | +**Risk Assessment**: **VERY LOW** |
| 222 | +- Purely additive changes |
| 223 | +- No modifications to existing argument behavior |
| 224 | +- No complex logic or edge cases |
| 225 | +- Follows established patterns |
| 226 | + |
| 227 | +### 5.2 Alternative Considered: Argparse Aliases |
| 228 | + |
| 229 | +**Why Rejected**: |
| 230 | +- Not supported natively by argparse |
| 231 | +- Would require custom implementation |
| 232 | +- More complex to maintain |
| 233 | +- No clear benefit over shared arguments |
| 234 | +- Doesn't fit existing codebase pattern |
| 235 | + |
| 236 | + |
| 237 | +--- |
| 238 | + |
| 239 | +## 6. Edge Cases & Risks |
| 240 | + |
| 241 | +### 6.1 Identified Edge Cases |
| 242 | + |
| 243 | +**1. env_vars vs env-var Confusion** |
| 244 | +- **Risk**: Users might confuse `--env-vars` (names) with `--env-var` (values) |
| 245 | +- **Mitigation**: Very clear help text explaining the difference |
| 246 | +- **Example help text**: |
| 247 | + - `--env-var`: "Set environment variable (KEY=VALUE format)" |
| 248 | + - `--env-vars`: "Whitelist environment variable names to forward (Codex only)" |
| 249 | + |
| 250 | +**2. Timeout Field Confusion** |
| 251 | +- **Risk**: Users might use wrong timeout for wrong host |
| 252 | +- **Mitigation**: Help text specifies which host uses which timeout |
| 253 | +- **Example help text**: |
| 254 | + - `--timeout`: "Request timeout in milliseconds (Gemini only)" |
| 255 | + - `--startup-timeout`: "Server startup timeout in seconds (Codex only)" |
| 256 | + - `--tool-timeout`: "Tool execution timeout in seconds (Codex only)" |
| 257 | + |
| 258 | +**3. http_headers vs env_http_headers** |
| 259 | +- **Risk**: Users might confuse when to use which |
| 260 | +- **Mitigation**: Clear help text explaining the difference |
| 261 | +- **Example help text**: |
| 262 | + - `--header`: "Static HTTP header (KEY=VALUE format)" |
| 263 | + - `--env-header`: "HTTP header from env var (KEY=ENV_VAR_NAME, Codex only)" |
| 264 | + |
| 265 | +**4. enabled vs disabled** |
| 266 | +- **Risk**: Confusion about which to use |
| 267 | +- **Mitigation**: Help text clarifies which host uses which |
| 268 | +- **Example help text**: |
| 269 | + - `--enabled`: "Enable server (Codex only)" |
| 270 | + - `--disabled`: "Disable server (Kiro only)" |
| 271 | + |
| 272 | +### 6.2 Backward Compatibility |
| 273 | + |
| 274 | +**Impact**: NONE - All changes are purely additive |
| 275 | +- ✅ All existing arguments continue to work |
| 276 | +- ✅ No changes to existing argument behavior |
| 277 | +- ✅ New arguments are optional |
| 278 | +- ✅ Existing hosts (Claude, Cursor, Kiro, Gemini) unaffected |
| 279 | + |
| 280 | +### 6.3 Missing Field Investigation |
| 281 | + |
| 282 | +**Question**: Is `cwd` missing from MCPServerConfigOmni? |
| 283 | + |
| 284 | +**Answer**: NO - `cwd` already exists in Omni (line 654) as a Gemini-specific field |
| 285 | +- Gemini uses it: ✅ |
| 286 | +- Codex uses it: ✅ |
| 287 | +- Already in CLI: ✅ (--cwd, line 1614) |
| 288 | +- No changes needed: ✅ |
| 289 | + |
| 290 | +--- |
| 291 | + |
| 292 | +## 7. Final Recommendation |
| 293 | + |
| 294 | +### 7.1 Implementation Plan |
| 295 | + |
| 296 | +**Add 6 New CLI Arguments**: |
| 297 | +1. `--env-vars` - List of env var names (action="append") |
| 298 | +2. `--startup-timeout` - Startup timeout in seconds (type=int) |
| 299 | +3. `--tool-timeout` - Tool execution timeout in seconds (type=int) |
| 300 | +4. `--enabled` - Enable server flag (action="store_true") |
| 301 | +5. `--bearer-token-env-var` - Bearer token env var name (type=str) |
| 302 | +6. `--env-header` - Header from env var (action="append", KEY=ENV_VAR format) |
| 303 | + |
| 304 | +**Reuse 4 Existing Arguments**: |
| 305 | +1. `--cwd` - Working directory (already exists) |
| 306 | +2. `--include-tools` - Tool allow-list (already exists) |
| 307 | +3. `--exclude-tools` - Tool deny-list (already exists) |
| 308 | +4. `--header` - Static headers (already exists) |
| 309 | + |
| 310 | +### 7.2 Implementation Checklist |
| 311 | + |
| 312 | +**Code Changes**: |
| 313 | +- [ ] Add 6 argument definitions to `setup_mcp_configure_parser()` (~30 lines) |
| 314 | +- [ ] Add 6 mappings to `omni_config_data` in `handle_mcp_configure()` (~6 lines) |
| 315 | +- [ ] Update help text for clarity (~6 lines) |
| 316 | + |
| 317 | +**Testing**: |
| 318 | +- [ ] Add CLI tests for each new argument (~50-100 lines) |
| 319 | +- [ ] Test argument parsing and mapping |
| 320 | +- [ ] Test integration with Codex strategy |
| 321 | +- [ ] Verify existing hosts unaffected |
| 322 | + |
| 323 | +**Documentation**: |
| 324 | +- [ ] Update CLI help text |
| 325 | +- [ ] Update user documentation (if exists) |
| 326 | + |
| 327 | +**Estimated Effort**: 2-3 hours |
| 328 | +**Estimated LOC**: ~100-150 lines |
| 329 | + |
| 330 | +### 7.3 Success Criteria |
| 331 | + |
| 332 | +1. ✅ All 10 Codex fields configurable via CLI |
| 333 | +2. ✅ No breaking changes to existing functionality |
| 334 | +3. ✅ Clear, user-friendly help text |
| 335 | +4. ✅ All tests pass |
| 336 | +5. ✅ Consistent with existing CLI patterns |
| 337 | + |
| 338 | +--- |
| 339 | + |
| 340 | +## 8. Conclusion |
| 341 | + |
| 342 | +The analysis supports a straightforward implementation: **add 6 new CLI arguments and reuse 4 existing arguments**. This approach: |
| 343 | + |
| 344 | +- Follows existing codebase patterns |
| 345 | +- Provides clear semantics |
| 346 | +- Requires simple implementation |
| 347 | +- Introduces no breaking changes |
| 348 | +- Maintains type safety |
| 349 | +- Is easy to document and maintain |
| 350 | + |
| 351 | +**Status**: ✅ Analysis Complete - Ready to proceed with implementation (Task 2) |
| 352 | + |
| 353 | +--- |
| 354 | + |
| 355 | +## Appendix A: Argument Naming Conventions |
| 356 | + |
| 357 | +| Pattern | Example | Usage | |
| 358 | +|---------|---------|-------| |
| 359 | +| Kebab-case | --env-var | Standard for multi-word args | |
| 360 | +| Action append | --args | For list arguments | |
| 361 | +| KEY=VALUE | --env-var KEY=VALUE | For dict arguments | |
| 362 | +| store_true | --enabled | For boolean flags | |
| 363 | +| Type hints | type=int | For typed arguments | |
| 364 | + |
| 365 | +## Appendix B: References |
| 366 | + |
| 367 | +- CLI Parser: `hatch/cli_hatch.py` lines 1571-1654 |
| 368 | +- CLI Handler: `hatch/cli_hatch.py` lines 783-850 |
| 369 | +- Omni Model: `hatch/mcp_host_config/models.py` lines 632-700 |
| 370 | +- Codex Model: `hatch/mcp_host_config/models.py` lines 567-630 |
0 commit comments