diff --git a/src/ClaudeDo.Worker/External/ConfigMcpTools.cs b/src/ClaudeDo.Worker/External/ConfigMcpTools.cs index c65eda59..6f2c207f 100644 --- a/src/ClaudeDo.Worker/External/ConfigMcpTools.cs +++ b/src/ClaudeDo.Worker/External/ConfigMcpTools.cs @@ -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 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); diff --git a/tests/ClaudeDo.Worker.Tests/External/ConfigMcpToolsTests.cs b/tests/ClaudeDo.Worker.Tests/External/ConfigMcpToolsTests.cs index 0bc84534..1f804877 100644 --- a/tests/ClaudeDo.Worker.Tests/External/ConfigMcpToolsTests.cs +++ b/tests/ClaudeDo.Worker.Tests/External/ConfigMcpToolsTests.cs @@ -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() {