Support CLAUDE_CONFIG_DIR for path resolution#7
Support CLAUDE_CONFIG_DIR for path resolution#7halindrome wants to merge 5 commits intoli195111:mainfrom
Conversation
Add CLAUDE_CONFIG_DIR as a fallback between plugin mode and $HOME/.claude in all three resolve functions (db_path, projects_dir, archive_dir). This ensures CTA respects the user's configured Claude config directory rather than always falling back to ~/.claude/ in standalone mode. Resolution priority is now four-mode: 1. Explicit CTA env var override 2. Plugin mode (CLAUDE_PLUGIN_ROOT) 3. Config dir mode (CLAUDE_CONFIG_DIR) 4. Standalone mode ($HOME/.claude) Fixes li195111#6 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Address QA round 1 findings: - CLAUDE.md: Update "Three-mode" to "Four-mode" path resolution - mcp.rs: Update sync_db tool description to mention CLAUDE_CONFIG_DIR Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
QA Report — PR #7 (Support CLAUDE_CONFIG_DIR for path resolution) — Round 1Model used: Claude Opus 4.6 (1M context) What Was Tested
Findings1. CLAUDE.md not updated — "Three-mode" is now four-mode (Low severity, confirmed)File: Fixed in: commit 7690680 2. MCP tool description for
|
Address QA round 2 finding: README Configuration section still described three-mode resolution. Updated priority chain and defaults table to include CLAUDE_CONFIG_DIR. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
QA Report — PR #7 Round 2 (Support CLAUDE_CONFIG_DIR for path resolution)Model used: Claude Opus 4.6 (1M context) Round 1 Fix VerificationFix 1: CLAUDE.md "Four-mode path resolution" — PASS Both fixes from commit 7690680 landed correctly. What Was Tested
Findings1. README.md path resolution description is outdated (Low severity, confirmed)File: Fixed in: commit 8fb9037 2. Pre-existing clippy warnings in format.rs (Info, pre-existing)Not introduced by this PR. Identical on 3. analyze_project parameter doc uses
|
Address QA round 3 findings: - Module table: "three deployment modes" → "four deployment modes" - Test count: 98 → 103 (5 new CLAUDE_CONFIG_DIR tests) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
QA Report — PR #7 Round 3 (Support CLAUDE_CONFIG_DIR for path resolution)Model used: Claude Opus 4.6 (1M context) Prior Fix Verification
What Was Tested
Findings1. CLAUDE.md module table still says "three deployment modes" (Low, confirmed)Line 62: Fixed in: commit 5550c6c 2. CLAUDE.md test count says 98, now 103 (Info, confirmed)Line 10 comment. PR added 5 new tests. Fixed in: commit 5550c6c 3. README CTA_DB_PATH default inconsistent with other rows (Info, pre-existing)Not introduced by this PR. 4. Empty CLAUDE_CONFIG_DIR edge case (Info, hypothetical)Pre-existing pattern across all env vars. Not a regression. SecurityNo concerns. Same trust model as all other env vars in the module. Overall RecommendationPASS with minor notes. Both findings fixed in commit 5550c6c. The code implementation is clean, correct, well-tested, and follows existing patterns. |
Address QA round 4 finding. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
QA Report — PR #7 Round 4 (Support CLAUDE_CONFIG_DIR for path resolution)Model used: Claude Opus 4.6 (1M context) Prior Fix Verification
All prior fixes verified. What Was Tested
Findings1. README.md line 96 still says "98 tests" (Low, confirmed)Build instructions section not updated when CLAUDE.md was fixed in R3. Fixed in: commit e9cb143 Overall RecommendationPASS with minor note. Fix committed. |
QA Report — PR #7 Round 5 (Support CLAUDE_CONFIG_DIR for path resolution)Model used: Claude Opus 4.6 (1M context) Prior Fix Verification
What Was Tested
FindingsNo new issues found. Pre-existing: clippy Overall RecommendationPASS — Merge-ready. All prior fixes verified, 103/103 tests green, no stale documentation. Clean round after 4 iterative fix rounds. |
li195111
left a comment
There was a problem hiding this comment.
Thanks for this PR, Shane! The design is sound — CLAUDE_CONFIG_DIR belongs exactly in this priority slot between CLAUDE_PLUGIN_ROOT and $HOME/.claude. The claude_config_dir_or_home() extraction is clean, the priority chain is correct, and the asymmetry (PLUGIN_ROOT for db/archive only, CONFIG_DIR for all three including projects) correctly models the two ownership domains.
Blocking: test coverage for resolve_archive_dir
resolve_archive_dir has four priority levels but only one test. Please add:
test_resolve_archive_dir_from_env— CTA_ARCHIVE_DIR overridetest_resolve_archive_dir_plugin_mode— CLAUDE_PLUGIN_ROOT pathtest_resolve_archive_dir_standalone— HOME/.claude fallback (clear CONFIG_DIR)test_plugin_root_takes_priority_over_config_dir_for_archive— both PLUGIN_ROOT and CONFIG_DIR set, archive should resolve under PLUGIN_ROOT
The last one is especially important: test_plugin_root_takes_priority_over_config_dir currently only covers resolve_db_path. The same priority invariant needs verification for archive.
Blocking: coexistence test
Please add a test verifying the split behavior when BOTH CLAUDE_PLUGIN_ROOT and CLAUDE_CONFIG_DIR are set simultaneously:
- db → CLAUDE_PLUGIN_ROOT/data/token-analyzer.db
- archive → CLAUDE_PLUGIN_ROOT/data/token-analyzer-archive
- projects → CLAUDE_CONFIG_DIR/projects (not PLUGIN_ROOT)
This makes the architectural asymmetry executable documentation and prevents future "simplification" that breaks the design.
Non-blocking suggestions
Unrelated file changes: docs/promotion/metrics-log.md and .gitignore appear to be base-branch divergence artifacts. Your branch removes the detailed baseline table from metrics-log.md and the gitignore entries for automation fork directories. Please restore both: git checkout origin/main -- docs/promotion/metrics-log.md .gitignore
Env isolation in existing tests: test_resolve_db_path_plugin_mode doesn't clear CLAUDE_CONFIG_DIR. Functionally safe (PLUGIN_ROOT is checked first), but adding ("CLAUDE_CONFIG_DIR", None) makes isolation explicit.
CLAUDE_CONFIG_DIR caveat: Worth noting that Claude Code's own support for this env var is still incomplete (see anthropics/claude-code#3833, #4739). A brief comment in the doc header acknowledging this would be helpful for future maintainers.
Summary
CLAUDE_CONFIG_DIRas a fallback inconfig.rspath resolution, between plugin mode (CLAUDE_PLUGIN_ROOT) and standalone mode ($HOME/.claude)claude_config_dir_or_home()helper used by all three resolve functions (resolve_db_path,resolve_projects_dir,resolve_archive_dir)Fixes #6
Test plan
cargo test --all-targets~/.local/bin/cta-mcp-serverCLAUDE_PLUGIN_ROOTstill takes priority overCLAUDE_CONFIG_DIR$HOME/.claudefallback still works when neither env var is setformat.rsare unrelated to this change🤖 Generated with Claude Code