Skip to content

Conversation

@dluc
Copy link
Collaborator

@dluc dluc commented Nov 28, 2025

Problem

km config was calling Initialize() which forced node selection, making it show only one node instead of the entire configuration.

Before:

$ km config
{
  "nodeId": "personal",    # Only showed selected node
  "access": "Full",
  ...
}

After:

$ km config
{
  "nodes": [               # Shows ALL nodes
    {
      "nodeId": "personal",
      ...
    }
  ],
  "cache": {               # Shows cache config
    "embeddingsCache": {...}
  }
}

Solution

  • ConfigCommand no longer calls Initialize() (no node selection needed)
  • Uses GetConfig() to access the injected config directly
  • Default behavior shows entire config (all nodes + cache)
  • --show-nodes and --show-cache flags still work

TDD Approach

  1. ✅ Test written first (RED) - verified it failed
  2. ✅ Code fixed (GREEN) - test now passes
  3. ✅ All 262 tests passing (185 Main + 77 Core)

Changes

  • Add ConfigCommandTests with multi-node config verification
  • Update ConfigCommand to skip Initialize() for config queries
  • Add BaseCommand.GetConfig() protected method

Test Results

  • ✅ 262 tests passing (185 Main + 77 Core)
  • ✅ Zero failures, zero skipped
  • ✅ Zero regressions

dluc added 2 commits November 28, 2025 15:00
Problem: ConfigCommand called Initialize() which forced node selection,
making it illogical to show only one node when querying the entire config.

Solution:
- ConfigCommand now uses GetConfig() directly (no node selection needed)
- Default behavior shows all nodes + cache configuration
- Added GetConfig() helper method in BaseCommand
- Added tests to verify entire config is displayed

Changes:
- Add ConfigCommandTests with multi-node config verification
- Update ConfigCommand to skip Initialize() for config queries
- Add BaseCommand.GetConfig() protected method

Test results:
- Test written first (RED) - verified it failed
- Code fixed (GREEN) - test now passes
- All 262 tests passing (185 Main + 77 Core)
- Zero regressions
@dluc dluc merged commit 5b70560 into microsoft:main Nov 28, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant