Skip to content

Commit 325b298

Browse files
dlucdluc
committed
Fix: km config now shows entire configuration, not just one node
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
1 parent 9002c28 commit 325b298

File tree

3 files changed

+223
-30
lines changed

3 files changed

+223
-30
lines changed

‎src/Main/CLI/Commands/BaseCommand.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,15 @@ protected BaseCommand(AppConfig config)
3131
this._config = config ?? throw new ArgumentNullException(nameof(config));
3232
}
3333

34+
/// <summary>
35+
/// Gets the injected application configuration.
36+
/// </summary>
37+
/// <returns>The application configuration.</returns>
38+
protected AppConfig GetConfig()
39+
{
40+
return this._config;
41+
}
42+
3443
/// <summary>
3544
/// Initializes command dependencies: node and formatter.
3645
/// Config is already injected via constructor (no file I/O).

‎src/Main/CLI/Commands/ConfigCommand.cs‎

Lines changed: 48 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,17 @@ public override async Task<int> ExecuteAsync(
4141
{
4242
try
4343
{
44-
var (config, node, formatter) = this.Initialize(settings);
44+
// ConfigCommand doesn't need node selection - it queries the entire configuration
45+
// So we skip Initialize() and just use the injected config directly
46+
var formatter = CLI.OutputFormatters.OutputFormatterFactory.Create(settings);
4547

4648
// Determine what to show
4749
object output;
4850

4951
if (settings.ShowNodes)
5052
{
51-
// Show all nodes
52-
output = config.Nodes.Select(kvp => new NodeSummaryDto
53+
// Show all nodes summary
54+
output = this.GetConfig().Nodes.Select(kvp => new NodeSummaryDto
5355
{
5456
Id = kvp.Key,
5557
Access = kvp.Value.Access.ToString(),
@@ -64,44 +66,60 @@ public override async Task<int> ExecuteAsync(
6466
// Show cache configuration
6567
output = new CacheInfoDto
6668
{
67-
EmbeddingsCache = config.EmbeddingsCache != null ? new CacheConfigDto
69+
EmbeddingsCache = this.GetConfig().EmbeddingsCache != null ? new CacheConfigDto
6870
{
69-
Type = config.EmbeddingsCache.Type.ToString(),
70-
Path = config.EmbeddingsCache.Path
71+
Type = this.GetConfig().EmbeddingsCache.Type.ToString(),
72+
Path = this.GetConfig().EmbeddingsCache.Path
7173
} : null,
72-
LlmCache = config.LLMCache != null ? new CacheConfigDto
74+
LlmCache = this.GetConfig().LLMCache != null ? new CacheConfigDto
7375
{
74-
Type = config.LLMCache.Type.ToString(),
75-
Path = config.LLMCache.Path
76+
Type = this.GetConfig().LLMCache.Type.ToString(),
77+
Path = this.GetConfig().LLMCache.Path
7678
} : null
7779
};
7880
}
7981
else
8082
{
81-
// Default: show current node details
82-
output = new NodeDetailsDto
83+
// Default: show entire configuration with all nodes
84+
output = new
8385
{
84-
NodeId = node.Id,
85-
Access = node.Access.ToString(),
86-
ContentIndex = new ContentIndexConfigDto
87-
{
88-
Type = node.ContentIndex.Type.ToString(),
89-
Path = node.ContentIndex is KernelMemory.Core.Config.ContentIndex.SqliteContentIndexConfig sqlite
90-
? sqlite.Path
91-
: null
92-
},
93-
FileStorage = node.FileStorage != null ? new StorageConfigDto
86+
Nodes = this.GetConfig().Nodes.Select(kvp => new NodeDetailsDto
9487
{
95-
Type = node.FileStorage.Type.ToString()
96-
} : null,
97-
RepoStorage = node.RepoStorage != null ? new StorageConfigDto
98-
{
99-
Type = node.RepoStorage.Type.ToString()
100-
} : null,
101-
SearchIndexes = node.SearchIndexes.Select(si => new SearchIndexDto
88+
NodeId = kvp.Key,
89+
Access = kvp.Value.Access.ToString(),
90+
ContentIndex = new ContentIndexConfigDto
91+
{
92+
Type = kvp.Value.ContentIndex.Type.ToString(),
93+
Path = kvp.Value.ContentIndex is KernelMemory.Core.Config.ContentIndex.SqliteContentIndexConfig sqlite
94+
? sqlite.Path
95+
: null
96+
},
97+
FileStorage = kvp.Value.FileStorage != null ? new StorageConfigDto
98+
{
99+
Type = kvp.Value.FileStorage.Type.ToString()
100+
} : null,
101+
RepoStorage = kvp.Value.RepoStorage != null ? new StorageConfigDto
102+
{
103+
Type = kvp.Value.RepoStorage.Type.ToString()
104+
} : null,
105+
SearchIndexes = kvp.Value.SearchIndexes.Select(si => new SearchIndexDto
106+
{
107+
Type = si.Type.ToString()
108+
}).ToList()
109+
}).ToList(),
110+
Cache = new CacheInfoDto
102111
{
103-
Type = si.Type.ToString()
104-
}).ToList()
112+
EmbeddingsCache = this.GetConfig().EmbeddingsCache != null ? new CacheConfigDto
113+
{
114+
Type = this.GetConfig().EmbeddingsCache.Type.ToString(),
115+
Path = this.GetConfig().EmbeddingsCache.Path
116+
} : null,
117+
LlmCache = this.GetConfig().LLMCache != null ? new CacheConfigDto
118+
{
119+
Type = this.GetConfig().LLMCache.Type.ToString(),
120+
Path = this.GetConfig().LLMCache.Path
121+
} : null
122+
}
105123
};
106124
}
107125

‎tests/Main.Tests/Integration/ConfigCommandTests.cs‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
// Copyright (c) Microsoft. All rights reserved.
2+
3+
using KernelMemory.Core.Config;
4+
using KernelMemory.Main.CLI.Commands;
5+
using Spectre.Console.Cli;
6+
using Xunit;
7+
8+
namespace KernelMemory.Main.Tests.Integration;
9+
10+
/// <summary>
11+
/// Tests for ConfigCommand behavior.
12+
/// These tests verify that config command shows the entire configuration,
13+
/// not just a single node.
14+
/// </summary>
15+
public sealed class ConfigCommandTests : IDisposable
16+
{
17+
private readonly string _tempDir;
18+
private readonly string _configPath;
19+
20+
public ConfigCommandTests()
21+
{
22+
this._tempDir = Path.Combine(Path.GetTempPath(), $"km-config-test-{Guid.NewGuid():N}");
23+
Directory.CreateDirectory(this._tempDir);
24+
25+
this._configPath = Path.Combine(this._tempDir, "config.json");
26+
27+
// Create test config with MULTIPLE nodes to verify entire config is shown
28+
var config = new AppConfig
29+
{
30+
Nodes = new Dictionary<string, NodeConfig>
31+
{
32+
["personal"] = NodeConfig.CreateDefaultPersonalNode(Path.Combine(this._tempDir, "nodes", "personal")),
33+
["work"] = NodeConfig.CreateDefaultPersonalNode(Path.Combine(this._tempDir, "nodes", "work")),
34+
["shared"] = NodeConfig.CreateDefaultPersonalNode(Path.Combine(this._tempDir, "nodes", "shared"))
35+
}
36+
};
37+
38+
var json = System.Text.Json.JsonSerializer.Serialize(config);
39+
File.WriteAllText(this._configPath, json);
40+
}
41+
42+
public void Dispose()
43+
{
44+
try
45+
{
46+
if (Directory.Exists(this._tempDir))
47+
{
48+
Directory.Delete(this._tempDir, true);
49+
}
50+
}
51+
catch (IOException)
52+
{
53+
// Ignore cleanup errors
54+
}
55+
catch (UnauthorizedAccessException)
56+
{
57+
// Ignore cleanup errors
58+
}
59+
}
60+
61+
[Fact]
62+
public void ConfigCommand_WithoutFlags_ShouldShowEntireConfiguration()
63+
{
64+
// This test verifies the bug: km config should show ALL nodes, not just the selected one
65+
66+
// Arrange
67+
var config = ConfigParser.LoadFromFile(this._configPath);
68+
69+
var settings = new ConfigCommandSettings
70+
{
71+
ConfigPath = this._configPath,
72+
Format = "json" // Use JSON format for easier assertion
73+
};
74+
75+
var command = new ConfigCommand(config);
76+
var context = new CommandContext(
77+
new[] { "--config", this._configPath },
78+
new EmptyRemainingArguments(),
79+
"config",
80+
null);
81+
82+
// Capture stdout to verify output
83+
using var outputCapture = new StringWriter();
84+
var originalOutput = Console.Out;
85+
Console.SetOut(outputCapture);
86+
87+
try
88+
{
89+
// Act
90+
var exitCode = command.ExecuteAsync(context, settings).GetAwaiter().GetResult();
91+
92+
// Assert
93+
Assert.Equal(Constants.ExitCodeSuccess, exitCode);
94+
95+
var output = outputCapture.ToString();
96+
97+
// The output should contain ALL THREE nodes, not just one
98+
Assert.Contains("personal", output);
99+
Assert.Contains("work", output);
100+
Assert.Contains("shared", output);
101+
102+
// Verify it's showing the entire config structure
103+
// Current bug: it only shows the first node's details
104+
// Expected: it should show all nodes
105+
}
106+
finally
107+
{
108+
Console.SetOut(originalOutput);
109+
}
110+
}
111+
112+
[Fact]
113+
public void ConfigCommand_WithShowNodesFlag_ShouldShowAllNodesSummary()
114+
{
115+
// Arrange
116+
var config = ConfigParser.LoadFromFile(this._configPath);
117+
118+
var settings = new ConfigCommandSettings
119+
{
120+
ConfigPath = this._configPath,
121+
Format = "json",
122+
ShowNodes = true
123+
};
124+
125+
var command = new ConfigCommand(config);
126+
var context = new CommandContext(
127+
new[] { "--config", this._configPath },
128+
new EmptyRemainingArguments(),
129+
"config",
130+
null);
131+
132+
// Capture stdout
133+
using var outputCapture = new StringWriter();
134+
var originalOutput = Console.Out;
135+
Console.SetOut(outputCapture);
136+
137+
try
138+
{
139+
// Act
140+
var exitCode = command.ExecuteAsync(context, settings).GetAwaiter().GetResult();
141+
142+
// Assert
143+
Assert.Equal(Constants.ExitCodeSuccess, exitCode);
144+
145+
var output = outputCapture.ToString();
146+
147+
// Should show all three nodes
148+
Assert.Contains("personal", output);
149+
Assert.Contains("work", output);
150+
Assert.Contains("shared", output);
151+
}
152+
finally
153+
{
154+
Console.SetOut(originalOutput);
155+
}
156+
}
157+
158+
/// <summary>
159+
/// Helper class to provide empty remaining arguments for CommandContext.
160+
/// </summary>
161+
private sealed class EmptyRemainingArguments : IRemainingArguments
162+
{
163+
public IReadOnlyList<string> Raw => Array.Empty<string>();
164+
public ILookup<string, string?> Parsed => Enumerable.Empty<string>().ToLookup(x => x, x => (string?)null);
165+
}
166+
}

0 commit comments

Comments
 (0)