From 873d7c0fa1f1596958a3845c4bc7f2a5a6171009 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 16 Jul 2026 14:59:09 +0000 Subject: [PATCH 1/2] fix(cli): handle model migration validation errors --- docs/spec/SPEC-004-cli-contract.md | 2 + docs/spec/SPEC-011-daemon-architecture.md | 3 +- .../.system/files/netclaw-operations/SKILL.md | 2 +- .../references/diagnostics.md | 6 +- .../Doctor/DoctorFixServiceTests.cs | 80 +++++++++++++ .../Model/ModelCommandTests.cs | 110 ++++++++++++++++++ src/Netclaw.Cli/Config/ConfigFileHelper.cs | 9 +- src/Netclaw.Cli/Config/ModelEntryWriter.cs | 30 +++-- src/Netclaw.Cli/Doctor/DoctorFixService.cs | 9 -- src/Netclaw.Cli/Model/ModelCommand.cs | 24 ++-- src/Netclaw.Cli/Program.cs | 68 ++++++----- .../CrashLogWriterTests.cs | 66 +++++++++++ src/Netclaw.Configuration/CrashLogWriter.cs | 5 +- tests/smoke/scenarios/doctor.sh | 50 ++++++++ tests/smoke/scenarios/provider-model-cli.sh | 85 ++++++++++++++ 15 files changed, 482 insertions(+), 67 deletions(-) create mode 100644 src/Netclaw.Configuration.Tests/CrashLogWriterTests.cs diff --git a/docs/spec/SPEC-004-cli-contract.md b/docs/spec/SPEC-004-cli-contract.md index 259d6b219..ff05e9969 100644 --- a/docs/spec/SPEC-004-cli-contract.md +++ b/docs/spec/SPEC-004-cli-contract.md @@ -100,6 +100,8 @@ Behavior: - optional output: JSON for automation - exit code `0` for success - exit code `1` for validation, policy, or runtime failures +- expected model-configuration migration failures are validation failures: print actionable + output, return exit code `1`, and do not create crash logs or emit stack traces - exit code `2` for usage and argument errors ## Safety Rules diff --git a/docs/spec/SPEC-011-daemon-architecture.md b/docs/spec/SPEC-011-daemon-architecture.md index 3ca232b78..4218d7773 100644 --- a/docs/spec/SPEC-011-daemon-architecture.md +++ b/docs/spec/SPEC-011-daemon-architecture.md @@ -225,7 +225,8 @@ The daemon installs process-level exception handlers at startup for: - `AppDomain.CurrentDomain.UnhandledException` - `TaskScheduler.UnobservedTaskException` -On either path, Netclaw writes a crash log under `~/.netclaw/logs/crash-*.log` +On either path, Netclaw writes a crash log under `/logs/crash-*.log`; +`NETCLAW_HOME` defaults to `~/.netclaw` with process diagnostics and the latest known session/turn context. When DI is available, the daemon also emits an operational alert with type `daemon.crashing` (category `DaemonCrashed`) so configured webhook targets can diff --git a/feeds/skills/.system/files/netclaw-operations/SKILL.md b/feeds/skills/.system/files/netclaw-operations/SKILL.md index 6ff82b499..119bab907 100644 --- a/feeds/skills/.system/files/netclaw-operations/SKILL.md +++ b/feeds/skills/.system/files/netclaw-operations/SKILL.md @@ -3,7 +3,7 @@ name: netclaw-operations description: "REQUIRED when the user asks about scheduling, reminders, cron jobs, timers, background jobs, diagnostics, troubleshooting, MCP tools, daemon health, identity updates, or Netclaw capabilities and self-maintenance." metadata: author: netclaw - version: "2.31.0" + version: "2.32.0" --- # Netclaw Operations diff --git a/feeds/skills/.system/files/netclaw-operations/references/diagnostics.md b/feeds/skills/.system/files/netclaw-operations/references/diagnostics.md index ec9ffd12e..4439d4c74 100644 --- a/feeds/skills/.system/files/netclaw-operations/references/diagnostics.md +++ b/feeds/skills/.system/files/netclaw-operations/references/diagnostics.md @@ -11,8 +11,8 @@ When something seems wrong with Netclaw itself: 2. If doctor reports fixable issues, run `netclaw doctor --fix --dry-run` to preview auto-repairs (schema-driven: stale properties, enum coercion, missing defaults) 3. Run `netclaw status` via `shell_execute` — live runtime state from daemon -3. Check daemon logs at `~/.netclaw/logs/daemon-{yyyy-MM-dd}.log` -4. Check session logs at `~/.netclaw/logs/sessions/{sanitized-session-id}/session.log` +3. Check daemon logs at `/logs/daemon-{yyyy-MM-dd}.log` (`NETCLAW_HOME` defaults to `~/.netclaw`) +4. Check session logs at `/logs/sessions/{sanitized-session-id}/session.log` If `netclaw status` or `netclaw chat` prints `daemon not configured - please run netclaw init`, do not troubleshoot daemon reachability or model defaults. The @@ -75,7 +75,7 @@ debugging a daemon-wide problem → read `daemon.log`. | No LLM responses | `netclaw doctor`; verify provider credentials | | Missing tools | `netclaw mcp list`; check MCP connection state | | Memory recall degraded | `netclaw status` memory section | -| Daemon won't start | crash logs at `~/.netclaw/logs/crash-*.log` | +| Daemon won't start | crash logs at `/logs/crash-*.log` (`NETCLAW_HOME` defaults to `~/.netclaw`) | | Docker daemon cannot create `/home/netclaw/.netclaw/*` | Official image entrypoint repairs writable bind mounts to UID/GID `1654:1654`; if bypassed or read-only, run `sudo chown -R 1654:1654 ` or use a Docker named volume | | Discord/Slack channel offline | `netclaw status` shows the channel `disconnected` with a reason. Discord may also report `degraded` when Discord.Net says the socket is connected but the gateway is not ready, such as after a resumed session that Netclaw is replacing with a clean reconnect. A misconfigured channel (bad token, missing Discord Message Content intent) degrades only that channel — the daemon keeps running and other channels are unaffected. A transient network failure retries automatically; a config/permission failure stays offline until the operator fixes the config and restarts the daemon. | | `command not found` for `netclaw`/`dotnet`/a user tool from the shell tool when the daemon runs as a systemd service | The systemd `--user` service does not inherit your login-shell `PATH`; `netclaw daemon install` captures it into `~/.netclaw/config/daemon.env`. Run `netclaw doctor` (the **Systemd Unit PATH** check flags a missing/stale/legacy env file), then `netclaw doctor --fix` to rehydrate `PATH` from your current shell (or re-run `netclaw daemon install`), and finally `systemctl --user restart netclaw`. Installed a new tool after install? Its dir won't be seen until you re-run one of those and restart. Per-directory managers (`mise`/`asdf`/`direnv`) are not captured. | diff --git a/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs b/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs index 245bb3d22..98a3a96a0 100644 --- a/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs +++ b/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs @@ -5,12 +5,14 @@ // ----------------------------------------------------------------------- using Netclaw.Cli.Daemon; using System.Text.Json; +using Netclaw.Cli.Config; using Netclaw.Cli.Doctor; using Netclaw.Configuration; using Xunit; namespace Netclaw.Cli.Tests.Doctor; +[Collection(Netclaw.Cli.Tests.LegacyModelEnvironmentCollection.Name)] public sealed class DoctorFixServiceTests { // POSIX install dir: systemd units are always POSIX-style regardless of the host OS @@ -283,6 +285,84 @@ public async Task DoesNotThrow_WhenUnitEnvironmentFilePathIsMalformed() Assert.DoesNotContain(plan.Fixes, f => f.FilePath == paths.DaemonEnvironmentFilePath); } + [Fact] + public async Task LegacyEnvironmentOverride_BlocksMigrationWithoutChangingConfig() + { + var paths = NewPaths(); + const string config = + """ + { + "configVersion": 1, + "Models": { + "Main": { + "Provider": "vllm", + "ModelId": "qwen-vl" + } + } + } + """; + await File.WriteAllTextAsync(paths.NetclawConfigPath, config, TestContext.Current.CancellationToken); + const string envVar = "NETCLAW_Models__Main__ContextWindow"; + var previous = Environment.GetEnvironmentVariable(envVar); + + try + { + Environment.SetEnvironmentVariable(envVar, "65536"); + var service = ConfigOnlyService(paths); + + var exception = await Assert.ThrowsAsync( + () => service.BuildPlanAsync(TestContext.Current.CancellationToken)); + + Assert.Contains(envVar, exception.Message, StringComparison.Ordinal); + Assert.Equal(config, await File.ReadAllTextAsync( + paths.NetclawConfigPath, TestContext.Current.CancellationToken)); + } + finally + { + Environment.SetEnvironmentVariable(envVar, previous); + } + } + + [Fact] + public async Task LegacyEnvironmentOverride_DoesNotBlockAlreadyNamedModels() + { + var paths = NewPaths(); + const string config = + """ + { + "configVersion": 1, + "Models": { + "Definitions": { + "main": { + "Provider": "vllm", + "ModelId": "qwen-vl" + } + }, + "Roles": { + "Main": "main" + } + } + } + """; + await File.WriteAllTextAsync(paths.NetclawConfigPath, config, TestContext.Current.CancellationToken); + const string envVar = "NETCLAW_Models__Main__ContextWindow"; + var previous = Environment.GetEnvironmentVariable(envVar); + + try + { + Environment.SetEnvironmentVariable(envVar, "65536"); + var service = ConfigOnlyService(paths); + + var plan = await service.BuildPlanAsync(TestContext.Current.CancellationToken); + + Assert.DoesNotContain(plan.Fixes, fix => fix.FilePath == paths.NetclawConfigPath); + } + finally + { + Environment.SetEnvironmentVariable(envVar, previous); + } + } + private static NetclawPaths NewPaths() { var paths = new NetclawPaths(CreateTempBasePath()); diff --git a/src/Netclaw.Cli.Tests/Model/ModelCommandTests.cs b/src/Netclaw.Cli.Tests/Model/ModelCommandTests.cs index b541261d7..cd5b77244 100644 --- a/src/Netclaw.Cli.Tests/Model/ModelCommandTests.cs +++ b/src/Netclaw.Cli.Tests/Model/ModelCommandTests.cs @@ -12,6 +12,7 @@ namespace Netclaw.Cli.Tests.Model; +[Collection(Netclaw.Cli.Tests.LegacyModelEnvironmentCollection.Name)] public sealed class ModelCommandTests : IDisposable { private readonly DisposableTempDir _dir = new(); @@ -613,6 +614,115 @@ public async Task Set_CorruptModalityButValidWindow_PreservesWindowEndToEnd() Assert.False(main.TryGetProperty("InputModalities", out _)); // corrupt override dropped } + [Fact] + public async Task Set_LegacyEnvironmentOverride_ReturnsErrorWithoutChangingConfig() + { + var config = ProvidersOnly(); + config["Models"] = new Dictionary + { + ["Main"] = new Dictionary + { + ["Provider"] = "my-ollama", + ["ModelId"] = "qwen3:30b" + } + }; + WriteConfig(config); + var original = File.ReadAllText(_paths.NetclawConfigPath); + const string envVar = "NETCLAW_Models__Main__ContextWindow"; + var previous = Environment.GetEnvironmentVariable(envVar); + + try + { + Environment.SetEnvironmentVariable(envVar, "65536"); + + var exitCode = await ModelCommand.RunAsync( + ["model", "set", "main", "my-ollama", "qwen3:8b"], _paths, output: _output); + + Assert.Equal(1, exitCode); + Assert.Contains( + $"Error: Cannot migrate Models while legacy environment override '{envVar}' is set.", + _output.ToString(), StringComparison.Ordinal); + Assert.Equal(original, File.ReadAllText(_paths.NetclawConfigPath)); + } + finally + { + Environment.SetEnvironmentVariable(envVar, previous); + } + } + + [Fact] + public async Task Set_ConflictingLegacyRoles_ReturnsErrorWithoutChangingConfig() + { + var config = ProvidersOnly(); + config["Models"] = new Dictionary + { + ["Main"] = new Dictionary + { + ["Provider"] = "my-ollama", + ["ModelId"] = "qwen3:30b", + ["ContextWindow"] = 32768 + }, + ["Fallback"] = new Dictionary + { + ["Provider"] = "my-ollama", + ["ModelId"] = "qwen3:30b", + ["ContextWindow"] = 65536 + } + }; + WriteConfig(config); + var original = File.ReadAllText(_paths.NetclawConfigPath); + + var exitCode = await ModelCommand.RunAsync( + ["model", "set", "compaction", "my-ollama", "qwen3:30b"], _paths, output: _output); + + Assert.Equal(1, exitCode); + Assert.Contains( + "Error: Legacy model roles conflict for my-ollama/qwen3:30b; align their metadata before migration.", + _output.ToString(), StringComparison.Ordinal); + Assert.Equal(original, File.ReadAllText(_paths.NetclawConfigPath)); + } + + [Fact] + public async Task Clear_LegacyEnvironmentOverride_ReturnsErrorWithoutChangingConfig() + { + var config = ProvidersOnly(); + config["Models"] = new Dictionary + { + ["Main"] = new Dictionary + { + ["Provider"] = "my-ollama", + ["ModelId"] = "qwen3:30b" + }, + ["Fallback"] = new Dictionary + { + ["Provider"] = "my-ollama", + ["ModelId"] = "qwen3:8b" + } + }; + WriteConfig(config); + var original = File.ReadAllText(_paths.NetclawConfigPath); + const string envVar = "NETCLAW_Models__Fallback__ContextWindow"; + var previous = Environment.GetEnvironmentVariable(envVar); + + try + { + Environment.SetEnvironmentVariable(envVar, "65536"); + + var exitCode = await ModelCommand.RunAsync( + ["model", "clear", "fallback"], _paths, output: _output); + + Assert.Equal(1, exitCode); + Assert.Contains( + $"Error: Cannot migrate Models while legacy environment override '{envVar}' is set.", + _output.ToString(), StringComparison.Ordinal); + Assert.Equal(original, File.ReadAllText(_paths.NetclawConfigPath)); + } + finally + { + Environment.SetEnvironmentVariable(envVar, previous); + } + } + private static Dictionary WithMainEntry(Dictionary main) { var config = ProvidersOnly(); diff --git a/src/Netclaw.Cli/Config/ConfigFileHelper.cs b/src/Netclaw.Cli/Config/ConfigFileHelper.cs index 2fd3f3fed..5094f53ce 100644 --- a/src/Netclaw.Cli/Config/ConfigFileHelper.cs +++ b/src/Netclaw.Cli/Config/ConfigFileHelper.cs @@ -177,14 +177,7 @@ private static void PreserveLegacyModelsBackup(string path, Dictionary__* and " + - "NETCLAW_Models__Roles__* first."); - } + ModelEntryWriter.ThrowIfLegacyEnvironmentOverride(); var backupPath = path + ".legacy-models.bak"; if (!File.Exists(backupPath)) diff --git a/src/Netclaw.Cli/Config/ModelEntryWriter.cs b/src/Netclaw.Cli/Config/ModelEntryWriter.cs index 0c3f7e697..341de4f16 100644 --- a/src/Netclaw.Cli/Config/ModelEntryWriter.cs +++ b/src/Netclaw.Cli/Config/ModelEntryWriter.cs @@ -37,10 +37,24 @@ internal static bool MigrateLegacy(Dictionary modelsSection) return false; if (!modelsSection.Keys.Any(key => key is "Main" or "Fallback" or "Compaction")) return false; + + ThrowIfLegacyEnvironmentOverride(); EnsureNamedShape(modelsSection); return true; } + internal static void ThrowIfLegacyEnvironmentOverride() + { + var legacyEnvironmentOverride = FindLegacyEnvironmentOverride(); + if (legacyEnvironmentOverride is null) + return; + + throw new ModelConfigurationException( + $"Cannot migrate Models while legacy environment override '{legacyEnvironmentOverride}' is set. " + + "Move model overrides to NETCLAW_Models__Definitions____* and " + + "NETCLAW_Models__Roles__* first."); + } + internal static bool ClearRole(Dictionary modelsSection, string roleKey) { var (_, roles) = EnsureNamedShape(modelsSection); @@ -123,12 +137,12 @@ private static (Dictionary Definitions, Dictionary Definitions, Dictionary Definitions, Dictionary(raw) - ?? throw new InvalidOperationException($"Models:{role} could not be parsed."); + ?? throw new ModelConfigurationException($"Models:{role} could not be parsed."); } catch (JsonException) { model = ReadLegacyIdentity(raw) - ?? throw new InvalidOperationException($"Models:{role} could not be repaired."); + ?? throw new ModelConfigurationException($"Models:{role} could not be repaired."); } var existingName = FindDefinition(definitions, model.Provider, model.ModelId); if (existingName is not null) @@ -167,7 +181,7 @@ private static (Dictionary Definitions, Dictionary(definitions[existingName])!; if (!Equivalent(existing, model)) { - throw new InvalidOperationException( + throw new ModelConfigurationException( $"Legacy model roles conflict for {model.Provider}/{model.ModelId}; " + $"align their metadata before migration."); } @@ -219,7 +233,7 @@ private static Dictionary GetDictionary(Dictionary BuildModelEntry( } } +internal sealed class ModelConfigurationException(string message) : Exception(message); + /// /// An operator's intent for an overridable, operator-owned model attribute on model set — /// a modality set () or the context window (). A plain diff --git a/src/Netclaw.Cli/Doctor/DoctorFixService.cs b/src/Netclaw.Cli/Doctor/DoctorFixService.cs index 57e5e19ce..d5cf22b5f 100644 --- a/src/Netclaw.Cli/Doctor/DoctorFixService.cs +++ b/src/Netclaw.Cli/Doctor/DoctorFixService.cs @@ -67,15 +67,6 @@ public Task BuildPlanAsync(CancellationToken cancellationToken = if (obj["Models"] is JsonObject modelsNode) { - var legacyEnvironmentOverride = ModelEntryWriter.FindLegacyEnvironmentOverride(); - if (legacyEnvironmentOverride is not null) - { - throw new InvalidOperationException( - $"Cannot migrate Models while legacy environment override '{legacyEnvironmentOverride}' is set. " + - "Move model overrides to NETCLAW_Models__Definitions____* and " + - "NETCLAW_Models__Roles__* first."); - } - var models = JsonSerializer.Deserialize>(modelsNode.ToJsonString())!; if (ModelEntryWriter.MigrateLegacy(models)) { diff --git a/src/Netclaw.Cli/Model/ModelCommand.cs b/src/Netclaw.Cli/Model/ModelCommand.cs index 44021cc61..1ad6dee14 100644 --- a/src/Netclaw.Cli/Model/ModelCommand.cs +++ b/src/Netclaw.Cli/Model/ModelCommand.cs @@ -25,15 +25,23 @@ public static async Task RunAsync( var writer = output ?? Console.Out; var subcommand = args.Length > 1 ? args[1] : "help"; - return subcommand switch + try { - "list" => RunList(paths, writer), - "set" => await RunSetAsync(args, paths, probe, writer), - "discover" => await RunDiscoverAsync(args, paths, probe, writer), - "clear" => RunClear(args, paths, writer), - "help" or "-h" or "--help" => WriteHelp(writer), - _ => WriteHelp(writer) - }; + return subcommand switch + { + "list" => RunList(paths, writer), + "set" => await RunSetAsync(args, paths, probe, writer), + "discover" => await RunDiscoverAsync(args, paths, probe, writer), + "clear" => RunClear(args, paths, writer), + "help" or "-h" or "--help" => WriteHelp(writer), + _ => WriteHelp(writer) + }; + } + catch (ModelConfigurationException ex) + { + writer.WriteLine($"Error: {ex.Message}"); + return 1; + } } private static int RunList(NetclawPaths paths, TextWriter writer) diff --git a/src/Netclaw.Cli/Program.cs b/src/Netclaw.Cli/Program.cs index f5b953b37..453406616 100644 --- a/src/Netclaw.Cli/Program.cs +++ b/src/Netclaw.Cli/Program.cs @@ -228,40 +228,56 @@ static async Task RunAsync(string[] args) var runner = scope.ServiceProvider.GetRequiredService(); var fixService = scope.ServiceProvider.GetRequiredService(); - DoctorFixPlan? fixPlan = null; - if (doctorOptions!.Fix) + try { - fixPlan = await fixService.BuildPlanAsync(); - if (doctorOptions.Format is DoctorOutputFormat.Text) - WriteDoctorFixPlan(fixPlan, doctorOptions.DryRun); - - if (fixPlan.HasChanges && !doctorOptions.DryRun) + DoctorFixPlan? fixPlan = null; + if (doctorOptions!.Fix) { - var shouldApply = doctorOptions.Yes || PromptForDoctorFixApply(); - if (shouldApply) - await fixService.ApplyAsync(fixPlan); + fixPlan = await fixService.BuildPlanAsync(); + if (doctorOptions.Format is DoctorOutputFormat.Text) + WriteDoctorFixPlan(fixPlan, doctorOptions.DryRun); + + if (fixPlan.HasChanges && !doctorOptions.DryRun) + { + var shouldApply = doctorOptions.Yes || PromptForDoctorFixApply(); + if (shouldApply) + await fixService.ApplyAsync(fixPlan); + } } - } - var result = await runner.RunAsync(); + var result = await runner.RunAsync(); - if (doctorOptions.Format is DoctorOutputFormat.Json) - WriteDoctorJsonResult(result, fixPlan, doctorOptions); - else - WriteDoctorResult(result); + if (doctorOptions.Format is DoctorOutputFormat.Json) + WriteDoctorJsonResult(result, fixPlan, doctorOptions); + else + WriteDoctorResult(result); - // Hint about --fix when there are issues and fix wasn't requested - if (!doctorOptions.Fix - && result.ExitCode != 0 - && doctorOptions.Format is DoctorOutputFormat.Text) - { - fixPlan ??= await fixService.BuildPlanAsync(); - if (fixPlan.HasChanges) - Console.WriteLine("hint: Some issues may be auto-fixable. Run `netclaw doctor --fix --dry-run` to preview."); + // Hint about --fix when there are issues and fix wasn't requested + if (!doctorOptions.Fix + && result.ExitCode != 0 + && doctorOptions.Format is DoctorOutputFormat.Text) + { + fixPlan ??= await fixService.BuildPlanAsync(); + if (fixPlan.HasChanges) + Console.WriteLine("hint: Some issues may be auto-fixable. Run `netclaw doctor --fix --dry-run` to preview."); + } + + Environment.ExitCode = result.ExitCode; + return; } + catch (ModelConfigurationException ex) + { + var failure = new DoctorRunResult( + [DoctorCheckResult.Error("model-configuration", ex.Message)], + ExitCode: 1); + if (doctorOptions!.Format is DoctorOutputFormat.Json) + WriteDoctorJsonResult(failure, fixPlan: null, doctorOptions); + else + Console.WriteLine($"Error: {ex.Message}"); - Environment.ExitCode = result.ExitCode; - return; + Environment.ExitCode = 1; + return; + } } if (mode is "status") diff --git a/src/Netclaw.Configuration.Tests/CrashLogWriterTests.cs b/src/Netclaw.Configuration.Tests/CrashLogWriterTests.cs new file mode 100644 index 000000000..5736af6c7 --- /dev/null +++ b/src/Netclaw.Configuration.Tests/CrashLogWriterTests.cs @@ -0,0 +1,66 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using Xunit; + +namespace Netclaw.Configuration.Tests; + +[Collection(nameof(NetclawHomeEnvCollection))] +public sealed class CrashLogWriterTests : IDisposable +{ + private const string EnvVar = "NETCLAW_HOME"; + private readonly string? _originalValue = Environment.GetEnvironmentVariable(EnvVar); + private readonly List _tempDirectories = []; + + public void Dispose() + { + Environment.SetEnvironmentVariable(EnvVar, _originalValue); + foreach (var directory in _tempDirectories) + Directory.Delete(directory, recursive: true); + } + + [Fact] + public void TryWrite_DefaultDirectory_HonorsNetclawHome() + { + var home = NewTempDirectory(); + Environment.SetEnvironmentVariable(EnvVar, home); + using var errors = new StringWriter(); + + var crashPath = CrashLogWriter.TryWrite( + new InvalidOperationException("boom"), "CLI", errorWriter: errors); + + Assert.NotNull(crashPath); + Assert.Equal(Path.Combine(home, "logs"), Path.GetDirectoryName(crashPath)); + Assert.True(File.Exists(crashPath)); + Assert.Contains(crashPath, errors.ToString(), StringComparison.Ordinal); + } + + [Fact] + public void TryWrite_ExplicitDirectory_TakesPrecedenceOverNetclawHome() + { + var home = NewTempDirectory(); + var explicitLogs = NewTempDirectory(); + Environment.SetEnvironmentVariable(EnvVar, home); + using var errors = new StringWriter(); + + var crashPath = CrashLogWriter.TryWrite( + new InvalidOperationException("boom"), "CLI", + errorWriter: errors, logsDirectory: explicitLogs); + + Assert.NotNull(crashPath); + Assert.Equal(explicitLogs, Path.GetDirectoryName(crashPath)); + Assert.True(File.Exists(crashPath)); + Assert.False(Directory.Exists(Path.Combine(home, "logs"))); + Assert.Contains(crashPath, errors.ToString(), StringComparison.Ordinal); + } + + private string NewTempDirectory() + { + var path = Path.Combine(Path.GetTempPath(), "netclaw-crash-tests", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(path); + _tempDirectories.Add(path); + return path; + } +} diff --git a/src/Netclaw.Configuration/CrashLogWriter.cs b/src/Netclaw.Configuration/CrashLogWriter.cs index 44aa94cd5..7bfd60960 100644 --- a/src/Netclaw.Configuration/CrashLogWriter.cs +++ b/src/Netclaw.Configuration/CrashLogWriter.cs @@ -24,10 +24,7 @@ public static class CrashLogWriter try { - var effectiveLogsDirectory = logsDirectory - ?? Path.Combine( - Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), - ".netclaw", "logs"); + var effectiveLogsDirectory = logsDirectory ?? new NetclawPaths().LogsDirectory; Directory.CreateDirectory(effectiveLogsDirectory); diff --git a/tests/smoke/scenarios/doctor.sh b/tests/smoke/scenarios/doctor.sh index 3a1761bee..338375c7a 100755 --- a/tests/smoke/scenarios/doctor.sh +++ b/tests/smoke/scenarios/doctor.sh @@ -28,5 +28,55 @@ case "$doctor_status" in ;; esac +log "Testing 'doctor --fix' legacy migration guard..." +config_path="${NETCLAW_HOME}/config/netclaw.json" +cat >"$config_path" <<'JSON' +{ + "configVersion": 1, + "Models": { + "Main": { + "Provider": "local", + "ModelId": "qwen3:30b" + } + } +} +JSON +expected_config="${NETCLAW_HOME}/doctor-legacy.expected.json" +cp "$config_path" "$expected_config" +crash_count_before="$(find "${NETCLAW_HOME}/logs" -maxdepth 1 -name 'crash-*.log' 2>/dev/null | wc -l | tr -d ' ')" +fix_status=0 +fix_output="$( + NETCLAW_Models__Main__ContextWindow=65536 run_timed \ + "$STEP_TIMEOUT_SECONDS" "$NETCLAW_SMOKE_CLI" doctor --fix --yes 2>&1 +)" || fix_status=$? +echo "$fix_output" +crash_count_after="$(find "${NETCLAW_HOME}/logs" -maxdepth 1 -name 'crash-*.log' 2>/dev/null | wc -l | tr -d ' ')" +if [[ "$fix_status" -eq 1 \ + && "$fix_output" == Error:*"Cannot migrate Models"* \ + && "$fix_output" != *"Unhandled exception"* \ + && "$fix_output" != *"Fatal error"* \ + && "$crash_count_after" == "$crash_count_before" ]] \ + && cmp -s "$config_path" "$expected_config"; then + pass "doctor --fix: migration guard exits 1 without crash artefacts or config changes" +else + fail "doctor --fix: migration guard was not a clean validation failure" +fi + +json_status=0 +json_output="$( + NETCLAW_Models__Main__ContextWindow=65536 run_timed \ + "$STEP_TIMEOUT_SECONDS" "$NETCLAW_SMOKE_CLI" doctor --fix --yes --format json 2>&1 +)" || json_status=$? +echo "$json_output" +if [[ "$json_status" -eq 1 \ + && "$json_output" == *'"exitCode": 1'* \ + && "$json_output" == *'"name": "model-configuration"'* \ + && "$json_output" == *"Cannot migrate Models"* \ + && "$json_output" != *"Unhandled exception"* \ + && "$json_output" != *"Fatal error"* ]]; then + pass "doctor --fix --format json: migration guard preserves the JSON error envelope" +else + fail "doctor --fix --format json: migration guard did not return structured validation output" +fi summarize exit $? diff --git a/tests/smoke/scenarios/provider-model-cli.sh b/tests/smoke/scenarios/provider-model-cli.sh index 2c43196d1..04a061d32 100755 --- a/tests/smoke/scenarios/provider-model-cli.sh +++ b/tests/smoke/scenarios/provider-model-cli.sh @@ -93,5 +93,90 @@ else pass "model clear: $ALT_MODEL cleared from fallback" fi +log "Testing legacy migration failures are clean model errors..." +config_path="${NETCLAW_HOME}/config/netclaw.json" +cat >"$config_path" </dev/null | wc -l | tr -d ' ')" +migration_status=0 +migration_output="$( + NETCLAW_Models__Main__ContextWindow=65536 run_timed \ + "$STEP_TIMEOUT_SECONDS" "$NETCLAW_SMOKE_CLI" model set main local-ollama "$ALT_MODEL" 2>&1 +)" || migration_status=$? +echo "$migration_output" +crash_count_after="$(find "${NETCLAW_HOME}/logs" -maxdepth 1 -name 'crash-*.log' 2>/dev/null | wc -l | tr -d ' ')" +new_crash_log="$(find "${NETCLAW_HOME}/logs" -maxdepth 1 -name 'crash-*.log' -newer "$expected_config" -print -quit 2>/dev/null)" +if [[ "$migration_status" -eq 1 \ + && "$migration_output" == Error:*"Cannot migrate Models"* \ + && "$migration_output" != *"Unhandled exception"* \ + && "$migration_output" != *"Fatal error"* \ + && -z "$new_crash_log" \ + && "$crash_count_after" == "$crash_count_before" \ + && ! -e "${config_path}.legacy-models.bak" ]] \ + && cmp -s "$config_path" "$expected_config"; then + pass "model set: legacy environment guard exits 1 without crash artefacts or config changes" +else + fail "model set: legacy environment guard was not a clean validation failure" +fi + +cat >"$config_path" </dev/null | wc -l | tr -d ' ')" +conflict_status=0 +conflict_output="$( + run_timed "$STEP_TIMEOUT_SECONDS" "$NETCLAW_SMOKE_CLI" \ + model set compaction local-ollama "$SMOKE_MODEL" 2>&1 +)" || conflict_status=$? +echo "$conflict_output" +conflict_crash_count_after="$(find "${NETCLAW_HOME}/logs" -maxdepth 1 -name 'crash-*.log' 2>/dev/null | wc -l | tr -d ' ')" +if [[ "$conflict_status" -eq 1 \ + && "$conflict_output" == Error:*"Legacy model roles conflict"* \ + && "$conflict_output" != *"Unhandled exception"* \ + && "$conflict_output" != *"Fatal error"* \ + && "$conflict_crash_count_after" == "$conflict_crash_count_before" ]] \ + && cmp -s "$config_path" "$expected_config"; then + pass "model set: conflicting legacy roles exit 1 without changing config" +else + fail "model set: conflicting legacy roles were not a clean validation failure" +fi summarize exit $? From fa6db434d13e4d57f3c569fc043cd5dbb812dda3 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 16 Jul 2026 15:19:15 +0000 Subject: [PATCH 2/2] test(configuration): use non-resetting path joins --- src/Netclaw.Configuration.Tests/CrashLogWriterTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Netclaw.Configuration.Tests/CrashLogWriterTests.cs b/src/Netclaw.Configuration.Tests/CrashLogWriterTests.cs index 5736af6c7..fae069346 100644 --- a/src/Netclaw.Configuration.Tests/CrashLogWriterTests.cs +++ b/src/Netclaw.Configuration.Tests/CrashLogWriterTests.cs @@ -32,7 +32,7 @@ public void TryWrite_DefaultDirectory_HonorsNetclawHome() new InvalidOperationException("boom"), "CLI", errorWriter: errors); Assert.NotNull(crashPath); - Assert.Equal(Path.Combine(home, "logs"), Path.GetDirectoryName(crashPath)); + Assert.Equal(Path.Join(home, "logs"), Path.GetDirectoryName(crashPath)); Assert.True(File.Exists(crashPath)); Assert.Contains(crashPath, errors.ToString(), StringComparison.Ordinal); } @@ -52,13 +52,13 @@ public void TryWrite_ExplicitDirectory_TakesPrecedenceOverNetclawHome() Assert.NotNull(crashPath); Assert.Equal(explicitLogs, Path.GetDirectoryName(crashPath)); Assert.True(File.Exists(crashPath)); - Assert.False(Directory.Exists(Path.Combine(home, "logs"))); + Assert.False(Directory.Exists(Path.Join(home, "logs"))); Assert.Contains(crashPath, errors.ToString(), StringComparison.Ordinal); } private string NewTempDirectory() { - var path = Path.Combine(Path.GetTempPath(), "netclaw-crash-tests", Guid.NewGuid().ToString("N")); + var path = Path.Join(Path.GetTempPath(), "netclaw-crash-tests", Guid.NewGuid().ToString("N")); Directory.CreateDirectory(path); _tempDirectories.Add(path); return path;