fix(mcp): stop set_list_config from clearing settings it doesn't expose
This commit is contained in:
+17
-4
@@ -75,7 +75,9 @@ public sealed class ConfigMcpTools
|
||||
[McpServerTool, Description(
|
||||
"Set a list's default model/system prompt/agent path/max turns — the fallback for tasks in this list " +
|
||||
"that don't override them. Only the fields you pass are changed; omitted fields keep their current " +
|
||||
"value. To clear a field instead, name it in clearFields; clearing all four deletes the list's config.")]
|
||||
"value. To clear a field instead, name it in clearFields; clearing all four deletes the list's config " +
|
||||
"unless it also carries settings this tool doesn't expose (session skills, verify command, file-scope " +
|
||||
"serialization), which are always preserved.")]
|
||||
public async Task<SetListConfigResult> SetListConfig(
|
||||
string listId, string? model = null, string? systemPrompt = null, string? agentPath = null,
|
||||
int? maxTurns = null,
|
||||
@@ -95,13 +97,23 @@ public sealed class ConfigMcpTools
|
||||
var ap = clear.Contains("agentPath") ? null : agentPath.NullIfBlank() ?? existing?.AgentPath;
|
||||
var mt = clear.Contains("maxTurns") ? null : maxTurns ?? existing?.MaxTurns;
|
||||
|
||||
// Fields this tool doesn't expose but that live on the same row. They must survive every
|
||||
// write here — ListRepository.SetConfigAsync copies the entity verbatim, so anything left
|
||||
// at its default would silently reset (SerializeOnFileOverlap in particular has no UI
|
||||
// affordance at all, so a reset is invisible until tasks stop serializing).
|
||||
var hasUnrelatedSettings = existing is not null
|
||||
&& (existing.SessionSkills is not null
|
||||
|| existing.VerifyCommand is not null
|
||||
|| existing.SerializeOnFileOverlap);
|
||||
|
||||
TaskConfigDto? config;
|
||||
if (m is null && sp is null && ap is null && mt is null && clear.Count > 0)
|
||||
var allCleared = m is null && sp is null && ap is null && mt is null;
|
||||
if (allCleared && clear.Count > 0 && !hasUnrelatedSettings)
|
||||
{
|
||||
await _lists.DeleteConfigAsync(listId, cancellationToken);
|
||||
config = null;
|
||||
}
|
||||
else if (m is null && sp is null && ap is null && mt is null && existing is null)
|
||||
else if (allCleared && existing is null)
|
||||
{
|
||||
config = null;
|
||||
}
|
||||
@@ -111,8 +123,9 @@ public sealed class ConfigMcpTools
|
||||
{
|
||||
ListId = listId, Model = m, SystemPrompt = sp, AgentPath = ap, MaxTurns = mt,
|
||||
SessionSkills = existing?.SessionSkills, VerifyCommand = existing?.VerifyCommand,
|
||||
SerializeOnFileOverlap = existing?.SerializeOnFileOverlap ?? false,
|
||||
}, cancellationToken);
|
||||
config = new TaskConfigDto(m, sp, ap, mt);
|
||||
config = allCleared ? null : new TaskConfigDto(m, sp, ap, mt);
|
||||
}
|
||||
|
||||
await _broadcaster.ListUpdated(listId);
|
||||
|
||||
@@ -223,6 +223,72 @@ public sealed class ConfigMcpToolsTests : IDisposable
|
||||
_sut.SetTaskConfig(task.Id, clearFields: new[] { "bogus" }, cancellationToken: CancellationToken.None));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SetListConfig_PreservesSettingsItDoesNotExpose()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
await _lists.SetConfigAsync(new ListConfigEntity
|
||||
{
|
||||
ListId = listId,
|
||||
Model = "sonnet",
|
||||
SessionSkills = "[\"superpowers\"]",
|
||||
VerifyCommand = "dotnet build",
|
||||
SerializeOnFileOverlap = true,
|
||||
});
|
||||
|
||||
await _sut.SetListConfig(listId, model: "opus", cancellationToken: CancellationToken.None);
|
||||
|
||||
var cfg = await _lists.GetConfigAsync(listId);
|
||||
Assert.NotNull(cfg);
|
||||
Assert.Equal("opus", cfg!.Model);
|
||||
Assert.Equal("[\"superpowers\"]", cfg.SessionSkills);
|
||||
Assert.Equal("dotnet build", cfg.VerifyCommand);
|
||||
// SetConfigAsync copies the entity verbatim, so a field this tool doesn't expose would
|
||||
// reset to its default unless it is carried over explicitly.
|
||||
Assert.True(cfg.SerializeOnFileOverlap);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SetListConfig_ClearingEveryExposedField_KeepsRowWhenOtherSettingsRemain()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
await _lists.SetConfigAsync(new ListConfigEntity
|
||||
{
|
||||
ListId = listId,
|
||||
Model = "sonnet",
|
||||
VerifyCommand = "dotnet build",
|
||||
SerializeOnFileOverlap = true,
|
||||
});
|
||||
|
||||
var result = await _sut.SetListConfig(
|
||||
listId,
|
||||
clearFields: new[] { "model", "systemPrompt", "agentPath", "maxTurns" },
|
||||
cancellationToken: CancellationToken.None);
|
||||
|
||||
Assert.True(result.Ok);
|
||||
Assert.Null(result.Config);
|
||||
|
||||
var cfg = await _lists.GetConfigAsync(listId);
|
||||
Assert.NotNull(cfg);
|
||||
Assert.Null(cfg!.Model);
|
||||
Assert.Equal("dotnet build", cfg.VerifyCommand);
|
||||
Assert.True(cfg.SerializeOnFileOverlap);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SetListConfig_ClearingEveryField_DeletesRowWhenNothingElseIsSet()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
await _sut.SetListConfig(listId, "sonnet", cancellationToken: CancellationToken.None);
|
||||
|
||||
await _sut.SetListConfig(
|
||||
listId,
|
||||
clearFields: new[] { "model", "systemPrompt", "agentPath", "maxTurns" },
|
||||
cancellationToken: CancellationToken.None);
|
||||
|
||||
Assert.Null(await _lists.GetConfigAsync(listId));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetTaskConfig_NoOverrideSet_ReturnsNotFound()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user