diff --git a/src/ClaudeDo.Worker/CLAUDE.md b/src/ClaudeDo.Worker/CLAUDE.md index 7ad0549e..4d3fbca9 100644 --- a/src/ClaudeDo.Worker/CLAUDE.md +++ b/src/ClaudeDo.Worker/CLAUDE.md @@ -216,7 +216,9 @@ non-obvious, behaviour-changing. A fixed bug is git history, not a finding. - `online_inbox` — `enabled` (false by default; when false the entire `Online/` stack is not registered), `api_base_url` (must be HTTPS or loopback, validated at startup), `poll_interval_seconds` (60), `zitadel.authority`/`client_id`/`scopes`. The refresh token is **not** in this file — DPAPI-encrypted at `~/.todo-app/online-inbox.token`. Per-list config (`list_config` in DB) provides defaults for `model`, `system_prompt`, -`agent_path`, `max_turns`, `session_skills`, `verify_command`; tasks override each individually. +`agent_path`, `max_turns`, `session_skills`; tasks override each individually. `verify_command` is +list-only — there is no task-level override — and is written via `set_list_config`'s +`verifyCommand` parameter (`ConfigMcpTools`), the only writer besides the UI's list config editor. ## Notes diff --git a/src/ClaudeDo.Worker/External/ConfigMcpTools.cs b/src/ClaudeDo.Worker/External/ConfigMcpTools.cs index 6f2c207f..33684272 100644 --- a/src/ClaudeDo.Worker/External/ConfigMcpTools.cs +++ b/src/ClaudeDo.Worker/External/ConfigMcpTools.cs @@ -10,8 +10,10 @@ using ModelContextProtocol.Server; namespace ClaudeDo.Worker.External; public sealed record TaskConfigDto(string? Model, string? SystemPrompt, string? AgentPath, int? MaxTurns); +public sealed record ListConfigDto(string? Model, string? SystemPrompt, string? AgentPath, int? MaxTurns, string? VerifyCommand); public sealed record TaskConfigResult(bool Found, TaskConfigDto? Config); -public sealed record SetListConfigResult(bool Ok, string ListId, TaskConfigDto? Config); +public sealed record ListConfigResult(bool Found, ListConfigDto? Config); +public sealed record SetListConfigResult(bool Ok, string ListId, ListConfigDto? Config); public sealed record SetTaskConfigResult(bool Ok, string TaskId, TaskConfigDto? Config); public sealed record EffectiveModelDto(string Value, string Source); @@ -46,68 +48,68 @@ public sealed class ConfigMcpTools _dbFactory = dbFactory; } - [McpServerTool, Description("Read a list's default run config — the fallback used by tasks in this list that don't set their own overrides. Returns { found: false, config: null } if none is set.")] - public async Task GetListConfig(string listId, CancellationToken cancellationToken) + [McpServerTool, Description("Read a list's default run config, including its verify command — the fallback used by tasks in this list that don't set their own overrides. Returns { found: false, config: null } if none is set.")] + public async Task GetListConfig(string listId, CancellationToken cancellationToken) { var cfg = await _lists.GetConfigAsync(listId, cancellationToken); return cfg is null - ? new TaskConfigResult(false, null) - : new TaskConfigResult(true, new TaskConfigDto(cfg.Model, cfg.SystemPrompt, cfg.AgentPath, cfg.MaxTurns)); + ? new ListConfigResult(false, null) + : new ListConfigResult(true, new ListConfigDto(cfg.Model, cfg.SystemPrompt, cfg.AgentPath, cfg.MaxTurns, cfg.VerifyCommand)); } // clearFields (not a per-field sentinel like UpdateList's "" for workingDir) because maxTurns is an // int? — there's no blank value to overload as "clear" for a number the way there is for a string. private static readonly string[] ConfigFieldNames = { "model", "systemPrompt", "agentPath", "maxTurns" }; + private static readonly string[] ListConfigFieldNames = { "model", "systemPrompt", "agentPath", "maxTurns", "verifyCommand" }; - private static HashSet NormalizeClearFields(IReadOnlyList? clearFields) + private static HashSet NormalizeClearFields(IReadOnlyList? clearFields, string[] validFields) { var set = new HashSet(StringComparer.OrdinalIgnoreCase); foreach (var field in clearFields ?? Array.Empty()) { - if (!ConfigFieldNames.Contains(field, StringComparer.OrdinalIgnoreCase)) + if (!validFields.Contains(field, StringComparer.OrdinalIgnoreCase)) throw new InvalidOperationException( - $"Unknown clearFields entry '{field}'. Valid values: {string.Join(", ", ConfigFieldNames)}."); + $"Unknown clearFields entry '{field}'. Valid values: {string.Join(", ", validFields)}."); set.Add(field); } return set; } [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 " + - "unless it also carries settings this tool doesn't expose (session skills, verify command, file-scope " + - "serialization), which are always preserved.")] + "Set a list's default model/system prompt/agent path/max turns/verify command — the fallback for tasks " + + "in this list that don't override them (verify command is list-only; tasks cannot override it). Only " + + "the fields you pass are changed; omitted fields keep their current value. To clear a field instead, " + + "name it in clearFields; clearing all five deletes the list's config unless it also carries settings " + + "this tool doesn't expose (session skills, 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, - [Description("Field names to explicitly clear: 'model', 'systemPrompt', 'agentPath', 'maxTurns'. " + - "A field named here is cleared even if a value was also passed for it.")] + int? maxTurns = null, string? verifyCommand = null, + [Description("Field names to explicitly clear: 'model', 'systemPrompt', 'agentPath', 'maxTurns', " + + "'verifyCommand'. A field named here is cleared even if a value was also passed for it.")] IReadOnlyList? clearFields = null, CancellationToken cancellationToken = default) { _ = await _lists.GetByIdAsync(listId, cancellationToken) ?? throw new InvalidOperationException($"List {listId} not found."); - var clear = NormalizeClearFields(clearFields); + var clear = NormalizeClearFields(clearFields, ListConfigFieldNames); var existing = await _lists.GetConfigAsync(listId, cancellationToken); var m = clear.Contains("model") ? null : model.NullIfBlank() ?? existing?.Model; var sp = clear.Contains("systemPrompt") ? null : systemPrompt.NullIfBlank() ?? existing?.SystemPrompt; var ap = clear.Contains("agentPath") ? null : agentPath.NullIfBlank() ?? existing?.AgentPath; var mt = clear.Contains("maxTurns") ? null : maxTurns ?? existing?.MaxTurns; + var vc = clear.Contains("verifyCommand") ? null : verifyCommand.NullIfBlank() ?? existing?.VerifyCommand; // 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); + && (existing.SessionSkills is not null || existing.SerializeOnFileOverlap); - TaskConfigDto? config; - var allCleared = m is null && sp is null && ap is null && mt is null; + ListConfigDto? config; + var allCleared = m is null && sp is null && ap is null && mt is null && vc is null; if (allCleared && clear.Count > 0 && !hasUnrelatedSettings) { await _lists.DeleteConfigAsync(listId, cancellationToken); @@ -122,10 +124,10 @@ public sealed class ConfigMcpTools await _lists.SetConfigAsync(new ListConfigEntity { ListId = listId, Model = m, SystemPrompt = sp, AgentPath = ap, MaxTurns = mt, - SessionSkills = existing?.SessionSkills, VerifyCommand = existing?.VerifyCommand, + VerifyCommand = vc, SessionSkills = existing?.SessionSkills, SerializeOnFileOverlap = existing?.SerializeOnFileOverlap ?? false, }, cancellationToken); - config = allCleared ? null : new TaskConfigDto(m, sp, ap, mt); + config = allCleared ? null : new ListConfigDto(m, sp, ap, mt, vc); } await _broadcaster.ListUpdated(listId); @@ -147,7 +149,7 @@ public sealed class ConfigMcpTools var task = await _tasks.GetByIdAsync(taskId, cancellationToken) ?? throw new InvalidOperationException($"Task {taskId} not found."); - var clear = NormalizeClearFields(clearFields); + var clear = NormalizeClearFields(clearFields, ConfigFieldNames); var m = clear.Contains("model") ? null : model.NullIfBlank() ?? task.Model; var sp = clear.Contains("systemPrompt") ? null : systemPrompt.NullIfBlank() ?? task.SystemPrompt; diff --git a/tests/ClaudeDo.Worker.Tests/External/ConfigMcpToolsTests.cs b/tests/ClaudeDo.Worker.Tests/External/ConfigMcpToolsTests.cs index 1f804877..b252ecba 100644 --- a/tests/ClaudeDo.Worker.Tests/External/ConfigMcpToolsTests.cs +++ b/tests/ClaudeDo.Worker.Tests/External/ConfigMcpToolsTests.cs @@ -116,6 +116,85 @@ public sealed class ConfigMcpToolsTests : IDisposable _sut.SetListConfig(listId, clearFields: new[] { "bogus" }, cancellationToken: CancellationToken.None)); } + [Fact] + public async Task SetListConfig_SetsVerifyCommand() + { + var listId = await SeedListAsync(); + + var result = await _sut.SetListConfig(listId, verifyCommand: "dotnet test", cancellationToken: CancellationToken.None); + + Assert.Equal("dotnet test", result.Config!.VerifyCommand); + var cfg = await _sut.GetListConfig(listId, CancellationToken.None); + Assert.Equal("dotnet test", cfg.Config!.VerifyCommand); + } + + [Fact] + public async Task SetListConfig_OverwritesVerifyCommand() + { + var listId = await SeedListAsync(); + await _sut.SetListConfig(listId, verifyCommand: "dotnet test", cancellationToken: CancellationToken.None); + + var result = await _sut.SetListConfig(listId, verifyCommand: "dotnet build", cancellationToken: CancellationToken.None); + + Assert.Equal("dotnet build", result.Config!.VerifyCommand); + } + + [Fact] + public async Task SetListConfig_ClearVerifyCommand_ClearsOnlyThatField() + { + var listId = await SeedListAsync(); + await _sut.SetListConfig(listId, "opus", verifyCommand: "dotnet test", cancellationToken: CancellationToken.None); + + var result = await _sut.SetListConfig(listId, clearFields: new[] { "verifyCommand" }, cancellationToken: CancellationToken.None); + + Assert.Equal("opus", result.Config!.Model); + Assert.Null(result.Config.VerifyCommand); + } + + [Fact] + public async Task SetListConfig_ConfigWithOnlyVerifyCommand_IsCreated() + { + var listId = await SeedListAsync(); + + var result = await _sut.SetListConfig(listId, verifyCommand: "dotnet test", cancellationToken: CancellationToken.None); + + Assert.True(result.Ok); + Assert.NotNull(result.Config); + Assert.Equal("dotnet test", result.Config!.VerifyCommand); + Assert.Null(result.Config.Model); + } + + [Fact] + public async Task SetListConfig_ClearingEveryOtherField_KeepsConfigWhenVerifyCommandRemains() + { + var listId = await SeedListAsync(); + await _sut.SetListConfig(listId, "sonnet", "be terse", "agent.md", 30, "dotnet test", cancellationToken: CancellationToken.None); + + var result = await _sut.SetListConfig( + listId, clearFields: new[] { "model", "systemPrompt", "agentPath", "maxTurns" }, cancellationToken: CancellationToken.None); + + Assert.True(result.Ok); + Assert.NotNull(result.Config); + Assert.Equal("dotnet test", result.Config!.VerifyCommand); + Assert.Null(result.Config.Model); + } + + [Fact] + public async Task SetListConfig_ClearingEveryFieldIncludingVerifyCommand_DeletesConfig() + { + var listId = await SeedListAsync(); + await _sut.SetListConfig(listId, "sonnet", verifyCommand: "dotnet test", cancellationToken: CancellationToken.None); + + var result = await _sut.SetListConfig( + listId, + clearFields: new[] { "model", "systemPrompt", "agentPath", "maxTurns", "verifyCommand" }, + cancellationToken: CancellationToken.None); + + Assert.True(result.Ok); + Assert.Null(result.Config); + Assert.Null(await _lists.GetConfigAsync(listId)); + } + [Fact] public async Task SetTaskConfig_PersistsOverrides() { @@ -256,13 +335,12 @@ public sealed class ConfigMcpToolsTests : IDisposable { ListId = listId, Model = "sonnet", - VerifyCommand = "dotnet build", SerializeOnFileOverlap = true, }); var result = await _sut.SetListConfig( listId, - clearFields: new[] { "model", "systemPrompt", "agentPath", "maxTurns" }, + clearFields: new[] { "model", "systemPrompt", "agentPath", "maxTurns", "verifyCommand" }, cancellationToken: CancellationToken.None); Assert.True(result.Ok); @@ -271,7 +349,6 @@ public sealed class ConfigMcpToolsTests : IDisposable var cfg = await _lists.GetConfigAsync(listId); Assert.NotNull(cfg); Assert.Null(cfg!.Model); - Assert.Equal("dotnet build", cfg.VerifyCommand); Assert.True(cfg.SerializeOnFileOverlap); }