From 2127a80a2b26edc52079c1e8398a1ca10bd0effa Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 19 Jun 2026 19:25:47 +0000 Subject: [PATCH] feat(tui): show advertised skill count for remote skill servers The Skill Sources config screen now surfaces how many skills each remote server advertises, so operators can see what a feed offers without leaving the TUI. - The reachability probe parses the RFC agent-skills index it already fetches and returns the skill count; it shows in the add / Test-connection feedback and, after "Rescan all", as a "| N advertised" badge on the inventory row. - "advertised", not "loaded": it is the count the server publishes. The daemon may load fewer after content-scan / hash / minimum-version filtering, so the label is honest about what the number measures. - Rescan all refreshes counts off the loop (one probe per ENABLED remote server) via a shared probe helper, caches them by source name, and posts a completion summary ("Updated skill counts for K of N skill server(s); J did not respond") instead of a premature green success. - The count cache is invalidated on every config reload (rename / remove / URL change / disable), so a stale or misattributed count cannot survive a config change. - A stored bearer token that will not decrypt is skipped (no anonymous probe) and surfaced as a Rescan warning, never silently degraded to an unauthenticated request. - Interactive add / test probes stay capped at 10s for snappy feedback; the background Rescan refresh honors the feed's full configured timeout (matching the daemon). Tests cover advertised-count parsing, the disabled / undecryptable-token / unreachable paths, and cache invalidation on disable. --- .../SkillSourcesConfigViewModelTests.cs | 120 +++++++++++- .../Tui/Config/SkillSourcesConfigViewModel.cs | 182 ++++++++++++++++-- 2 files changed, 288 insertions(+), 14 deletions(-) diff --git a/src/Netclaw.Cli.Tests/Tui/Config/SkillSourcesConfigViewModelTests.cs b/src/Netclaw.Cli.Tests/Tui/Config/SkillSourcesConfigViewModelTests.cs index 9e3649921..f1b2be531 100644 --- a/src/Netclaw.Cli.Tests/Tui/Config/SkillSourcesConfigViewModelTests.cs +++ b/src/Netclaw.Cli.Tests/Tui/Config/SkillSourcesConfigViewModelTests.cs @@ -26,6 +26,110 @@ public SkillSourcesConfigViewModelTests() public void Dispose() => _dir.Dispose(); + [Theory] + [InlineData("{\"skills\":[{\"name\":\"a\"},{\"name\":\"b\"},{\"name\":\"c\"}]}", 3)] + [InlineData("{\"skills\":[]}", 0)] + [InlineData("{\"feedType\":\"system\"}", null)] // reachable index, but no skills array + [InlineData("not json at all", null)] // malformed body + [InlineData("[1,2,3]", null)] // non-object root + public void CountSkillsInIndexJson_returns_skill_count_or_null_when_unknown(string json, int? expected) + => Assert.Equal(expected, SkillFeedReachabilityProbe.CountSkillsInIndexJson(json)); + + [Fact] + public async Task Rescan_all_probes_remote_servers_and_shows_skill_counts_on_the_inventory_row() + { + File.WriteAllText( + _paths.NetclawConfigPath, + "{\"configVersion\":1,\"SkillFeeds\":{\"Feeds\":[{\"Name\":\"custom-feed\",\"Url\":\"https://feed.example.test\",\"Enabled\":true}]}}"); + using var vm = new SkillSourcesConfigViewModel(_paths, new FakeSkillFeedProbe(true, skillCount: 7)); + + TriggerRescanAll(vm); + await vm.PendingCountRefresh!; + + var remoteRow = vm.InventoryRows.Single(static row => row.SourceKind == SkillSourceKind.RemoteSkillServer); + Assert.Contains("7 advertised", remoteRow.Detail); + } + + [Fact] + public async Task Rescan_all_does_not_badge_disabled_remote_servers() + { + File.WriteAllText( + _paths.NetclawConfigPath, + "{\"configVersion\":1,\"SkillFeeds\":{\"Feeds\":[{\"Name\":\"custom-feed\",\"Url\":\"https://feed.example.test\",\"Enabled\":false}]}}"); + using var vm = new SkillSourcesConfigViewModel(_paths, new FakeSkillFeedProbe(true, skillCount: 7)); + + TriggerRescanAll(vm); + if (vm.PendingCountRefresh is { } refresh) + await refresh; + + var remoteRow = vm.InventoryRows.Single(static row => row.SourceKind == SkillSourceKind.RemoteSkillServer); + Assert.DoesNotContain("advertised", remoteRow.Detail, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task Rescan_all_invalidates_a_cached_count_when_the_server_is_disabled() + { + File.WriteAllText( + _paths.NetclawConfigPath, + "{\"configVersion\":1,\"SkillFeeds\":{\"Feeds\":[{\"Name\":\"custom-feed\",\"Url\":\"https://feed.example.test\",\"Enabled\":true}]}}"); + using var vm = new SkillSourcesConfigViewModel(_paths, new FakeSkillFeedProbe(true, skillCount: 7)); + + TriggerRescanAll(vm); + await vm.PendingCountRefresh!; + Assert.Contains("7 advertised", vm.InventoryRows.Single(static r => r.SourceKind == SkillSourceKind.RemoteSkillServer).Detail); + + // Disable the feed in config, then rescan: the previously cached "7 skills" must not survive. + File.WriteAllText( + _paths.NetclawConfigPath, + "{\"configVersion\":1,\"SkillFeeds\":{\"Feeds\":[{\"Name\":\"custom-feed\",\"Url\":\"https://feed.example.test\",\"Enabled\":false}]}}"); + TriggerRescanAll(vm); + if (vm.PendingCountRefresh is { } refresh) + await refresh; + + Assert.DoesNotContain( + "advertised", + vm.InventoryRows.Single(static r => r.SourceKind == SkillSourceKind.RemoteSkillServer).Detail, + StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task Rescan_all_skips_and_warns_when_a_stored_token_cannot_be_decrypted() + { + File.WriteAllText( + _paths.NetclawConfigPath, + "{\"configVersion\":1,\"SkillFeeds\":{\"Feeds\":[{\"Name\":\"custom-feed\",\"Url\":\"https://feed.example.test\",\"ApiKey\":\"ENC:not-valid-for-this-keyring\",\"Enabled\":true}]}}"); + using var vm = new SkillSourcesConfigViewModel(_paths, new FakeSkillFeedProbe(true, skillCount: 7)); + + TriggerRescanAll(vm); + if (vm.PendingCountRefresh is { } refresh) + await refresh; + + var remoteRow = vm.InventoryRows.Single(static row => row.SourceKind == SkillSourceKind.RemoteSkillServer); + Assert.DoesNotContain("advertised", remoteRow.Detail, StringComparison.OrdinalIgnoreCase); // not probed unauthenticated + Assert.Equal(ConfigStatusTone.Warning, vm.Status.Value.Tone); + Assert.Contains("could not be decrypted", vm.Status.Value.Text, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task Rescan_all_warns_when_a_remote_server_does_not_respond() + { + File.WriteAllText( + _paths.NetclawConfigPath, + "{\"configVersion\":1,\"SkillFeeds\":{\"Feeds\":[{\"Name\":\"custom-feed\",\"Url\":\"https://feed.example.test\",\"Enabled\":true}]}}"); + using var vm = new SkillSourcesConfigViewModel(_paths, new FakeSkillFeedProbe(false)); + + TriggerRescanAll(vm); + await vm.PendingCountRefresh!; + + // An unreachable server must not read as a green success, and must leave no badge. + Assert.Equal(ConfigStatusTone.Warning, vm.Status.Value.Tone); + Assert.Contains("did not respond", vm.Status.Value.Text, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain( + "advertised", + vm.InventoryRows.Single(static r => r.SourceKind == SkillSourceKind.RemoteSkillServer).Detail, + StringComparison.OrdinalIgnoreCase); + } + [Fact] public void Skill_sources_dashboard_entry_routes_to_real_editor() { @@ -716,7 +820,17 @@ private IConfigurationSection SingleFeedSection() private string Decrypt(string encrypted) => SecretsProtection.CreateProtector(_paths).Unprotect(encrypted); - private sealed class FakeSkillFeedProbe(bool success, bool requiresAuth = false, bool failWithToken = false) + private static void TriggerRescanAll(SkillSourcesConfigViewModel vm) + { + var rescanIndex = vm.InventoryRows + .Select(static (row, index) => (row, index)) + .Single(static entry => entry.row.Action == SkillSourcesInventoryAction.RescanAll) + .index; + vm.SelectedRow.Value = rescanIndex; + vm.ActivateSelected(); + } + + private sealed class FakeSkillFeedProbe(bool success, bool requiresAuth = false, bool failWithToken = false, int? skillCount = null) : ISkillFeedReachabilityProbe { // When set, ProbeAsync blocks on this gate before returning so tests can stage an in-flight @@ -739,11 +853,11 @@ public async Task ProbeAsync(string baseUrl, string return failWithToken ? new SkillFeedReachabilityResult(false, "unreachable") - : new SkillFeedReachabilityResult(true, "reachable"); + : new SkillFeedReachabilityResult(true, "reachable", SkillCount: skillCount); } return success - ? new SkillFeedReachabilityResult(true, "reachable") + ? new SkillFeedReachabilityResult(true, "reachable", SkillCount: skillCount) : new SkillFeedReachabilityResult(false, "unreachable"); } } diff --git a/src/Netclaw.Cli/Tui/Config/SkillSourcesConfigViewModel.cs b/src/Netclaw.Cli/Tui/Config/SkillSourcesConfigViewModel.cs index 8c8e3a619..fc281818e 100644 --- a/src/Netclaw.Cli/Tui/Config/SkillSourcesConfigViewModel.cs +++ b/src/Netclaw.Cli/Tui/Config/SkillSourcesConfigViewModel.cs @@ -18,7 +18,7 @@ namespace Netclaw.Cli.Tui.Config; -internal sealed record SkillFeedReachabilityResult(bool Success, string Message, bool RequiresAuth = false); +internal sealed record SkillFeedReachabilityResult(bool Success, string Message, bool RequiresAuth = false, int? SkillCount = null); internal interface ISkillFeedReachabilityProbe { @@ -35,7 +35,10 @@ public async Task ProbeAsync( { try { - var timeout = TimeSpan.FromSeconds(Math.Clamp(timeoutSeconds, 1, 10)); + // Ceiling raised to 60s so the background Rescan all count refresh can honor a feed's configured + // timeout (daemon default 30s); interactive add/test callers cap their own input at 10s for snappy + // feedback (see StartBackgroundProbe). + var timeout = TimeSpan.FromSeconds(Math.Clamp(timeoutSeconds, 1, 60)); // Link the caller's token to the per-probe timeout so a superseded/abandoned probe // (caller cancels via ct) and a slow server (timeout) both unwind the same way. using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct); @@ -51,7 +54,15 @@ public async Task ProbeAsync( using var response = await client.SendAsync(request, cts.Token); if (response.IsSuccessStatusCode) - return new SkillFeedReachabilityResult(true, "Skill feed discovery endpoint is reachable."); + { + var skillCount = await CountSkillsAsync(response, cts.Token); + // "advertised", not "available/loaded": this is the count the server publishes in its index; + // the daemon may load fewer after content-scan / hash / version filtering. + var message = skillCount is { } n + ? $"Skill feed reachable — {n} skill{(n == 1 ? "" : "s")} advertised." + : "Skill feed discovery endpoint is reachable."; + return new SkillFeedReachabilityResult(true, message, SkillCount: skillCount); + } if (response.StatusCode is HttpStatusCode.Unauthorized or HttpStatusCode.Forbidden) return new SkillFeedReachabilityResult(false, $"Skill feed authentication failed with HTTP {(int)response.StatusCode}.", RequiresAuth: true); @@ -70,6 +81,41 @@ public async Task ProbeAsync( return new SkillFeedReachabilityResult(false, $"Skill feed probe failed: {ex.Message}"); } } + + // The discovery index is the RFC agent-skills index ({ "skills": [...] }); count its entries so the + // operator sees how many skills the server actually serves. Best-effort: reachability already succeeded, + // so a malformed/slow body just leaves the count unknown (null) rather than failing the probe. + private static async Task CountSkillsAsync(HttpResponseMessage response, CancellationToken ct) + { + try + { + var json = await response.Content.ReadAsStringAsync(ct); + return CountSkillsInIndexJson(json); + } + catch (Exception ex) when (ex is IOException or HttpRequestException or OperationCanceledException) + { + return null; + } + } + + // Count the entries in the RFC agent-skills index ({ "skills": [...] }). Returns null for a body that + // is not a JSON object with a "skills" array, so a malformed feed leaves the count unknown, not zero. + internal static int? CountSkillsInIndexJson(string json) + { + try + { + using var doc = JsonDocument.Parse(json); + return doc.RootElement.ValueKind == JsonValueKind.Object + && doc.RootElement.TryGetProperty("skills", out var skills) + && skills.ValueKind == JsonValueKind.Array + ? skills.GetArrayLength() + : null; + } + catch (JsonException) + { + return null; + } + } } internal enum SkillSourceKind @@ -181,12 +227,23 @@ internal sealed class SkillSourcesConfigViewModel : ReactiveViewModel { private const int DefaultFeedTimeoutSeconds = 30; + // Interactive add/test probes cap their wait here so reachability feedback stays snappy even for a slow + // server; the background Rescan all count refresh deliberately uses the feed's full configured timeout. + private const int InteractiveProbeTimeoutSeconds = 10; + private readonly NetclawPaths _paths; private readonly ISkillFeedReachabilityProbe _probe; private readonly StringComparer _nameComparer = StringComparer.OrdinalIgnoreCase; private CancellationTokenSource? _probeCts; private Task? _probeTask; private SkillFeedReachabilityResult? _pendingRemoteProbeResult; + // Off-loop refresh (Rescan all) of per-server skill counts, cached by source name for the inventory row + // Detail. ConcurrentDictionary because thread-pool probe continuations write it while the loop thread + // reads it during render; OrdinalIgnoreCase to match the source-name identity used everywhere else. + private readonly System.Collections.Concurrent.ConcurrentDictionary _remoteSkillCounts = + new(StringComparer.OrdinalIgnoreCase); + private CancellationTokenSource? _countRefreshCts; + private Task? _countRefreshTask; private string? _lastProbeFingerprint; private List _sources = []; private SkillSourceKind? _selectedKind; @@ -577,6 +634,8 @@ public override void Dispose() // resulting OperationCanceledException, so there is no unobserved exception to worry about. _probeCts?.Cancel(); _probeCts?.Dispose(); + _countRefreshCts?.Cancel(); + _countRefreshCts?.Dispose(); Screen.Dispose(); SelectedRow.Dispose(); Draft.Dispose(); @@ -607,10 +666,16 @@ private void StartBackgroundProbe( _probeCts = new CancellationTokenSource(); SetStatus(testingMessage, ConfigStatusTone.Neutral); RequestRedraw(); - _probeTask = RunProbeAsync(url, apiKey, timeoutSeconds, _probeCts.Token, onResult); + // Interactive probe: cap the wait so add/test feedback stays snappy even for a slow server. The + // background Rescan all count refresh calls ExecuteProbeAsync directly with the feed's full timeout. + _probeTask = ExecuteProbeAsync( + url, apiKey, Math.Min(timeoutSeconds, InteractiveProbeTimeoutSeconds), _probeCts.Token, onResult); } - private async Task RunProbeAsync( + // Shared off-loop probe execution for both the interactive add/test probe and the Rescan all count + // refresh: run the probe, drop quietly if superseded/cancelled, otherwise hand the result to onResult and + // redraw. onResult MUST be status/field-only (never navigation) — it may run on a thread-pool thread. + private async Task ExecuteProbeAsync( string url, string? apiKey, int timeoutSeconds, @@ -630,7 +695,7 @@ private async Task RunProbeAsync( if (ct.IsCancellationRequested) return; - onResult(result); // MUST be status-only (Status/fields), never navigation + onResult(result); RequestRedraw(); } @@ -1747,8 +1812,95 @@ private void RescanAll() { ReloadSources(); var localCount = _sources.Count(s => s.Kind == SkillSourceKind.LocalFolder); - var remoteCount = _sources.Count(s => s.Kind == SkillSourceKind.RemoteSkillServer); - SetStatus($"Rescanned {localCount} local folder(s) and {remoteCount} skill server(s).", ConfigStatusTone.Success); + RefreshRemoteSkillCounts($"Rescanned {localCount} local folder(s)."); + } + + // Exposes the in-flight Rescan-all count refresh so tests can await it deterministically (no Task.Delay). + // Always a Task once Rescan all has run (Task.CompletedTask when nothing was probed), never null after. + internal Task? PendingCountRefresh => _countRefreshTask; + + // Refresh cached skill counts for every ENABLED remote server OFF the loop (triggered by Rescan all). Each + // probe runs on the thread pool (ExecuteProbeAsync — the same off-loop discipline as the single add/test + // probe) and writes its count into the thread-safe cache. Owns the Rescan all status: the in-progress line + // is posted BEFORE the summary task is launched, so a synchronously-completing probe's final status (via + // SummarizeRemoteCountsAsync) lands last instead of being overwritten by a "Refreshing…" line. + private void RefreshRemoteSkillCounts(string localSummary) + { + _countRefreshCts?.Cancel(); + _countRefreshCts?.Dispose(); + _countRefreshCts = new CancellationTokenSource(); + var ct = _countRefreshCts.Token; + + var probes = new List>(); + var credentialFailures = 0; + if (TryLoadSkillFeeds(out var feeds)) + { + // Disabled feeds contribute nothing to the running daemon, so don't probe or badge them. + foreach (var source in _sources.Where(static s => s.Kind == SkillSourceKind.RemoteSkillServer && s.Enabled)) + { + var feed = FindRemoteSource(feeds, source.Name); + if (feed is null) + continue; + + // A stored token that will not decrypt must NOT silently degrade to an anonymous probe — that + // would hide the credential failure and send an unauthenticated request to a server the operator + // configured a token for. Skip the feed and surface the failure through the Rescan all status. + if (!TryGetFeedApiKeyPlaintext(feed, out var apiKey, out _)) + { + credentialFailures++; + continue; + } + + probes.Add(ProbeAndCacheSkillCountAsync(source.Name, feed.Url, apiKey, feed.TimeoutSeconds, ct)); + } + } + + var summary = localSummary; + if (credentialFailures > 0) + summary += $" {credentialFailures} server(s) skipped — stored token could not be decrypted."; + + if (probes.Count > 0) + SetStatus( + $"{summary} Refreshing {probes.Count} skill server count(s)…", + credentialFailures > 0 ? ConfigStatusTone.Warning : ConfigStatusTone.Neutral); + else + SetStatus(summary, credentialFailures > 0 ? ConfigStatusTone.Warning : ConfigStatusTone.Success); + + _countRefreshTask = probes.Count > 0 ? SummarizeRemoteCountsAsync(probes, ct) : Task.CompletedTask; + } + + // Probe one server with the feed's full configured timeout and cache its advertised count. Returns true + // only if the server actually reported a count, so RescanAll can report how many responded. + private async Task ProbeAndCacheSkillCountAsync(string name, string url, string? apiKey, int timeoutSeconds, CancellationToken ct) + { + var reported = false; + await ExecuteProbeAsync(url, apiKey, timeoutSeconds, ct, result => + { + if (result.SkillCount is not { } count) + return; + + _remoteSkillCounts[name] = count; + reported = true; + }); + return reported; + } + + // Once every count probe has settled, report how many servers actually responded so an all-failed refresh + // does not read as success. Off-loop; the ct guard drops the update if the refresh was superseded/disposed. + private async Task SummarizeRemoteCountsAsync(List> probes, CancellationToken ct) + { + var outcomes = await Task.WhenAll(probes); + if (ct.IsCancellationRequested) + return; + + var reported = outcomes.Count(static r => r); + var total = outcomes.Length; + if (reported == total) + SetStatus($"Updated skill counts for {total} skill server(s).", ConfigStatusTone.Success); + else + SetStatus( + $"Updated skill counts for {reported} of {total} skill server(s); {total - reported} did not respond.", + ConfigStatusTone.Warning); } private IReadOnlyList BuildInventoryRows() @@ -1767,7 +1919,7 @@ private IReadOnlyList BuildInventoryRows() rows.Add(new SkillSourcesInventoryRow(SkillSourcesInventoryAction.AddLocalFolder, null, null, "+ Add local folder", "Scan a directory on this machine.", ConfigStatusTone.Neutral)); rows.Add(new SkillSourcesInventoryRow(SkillSourcesInventoryAction.AddSkillServer, null, null, "+ Add skill server", "Connect to a remote skill feed.", ConfigStatusTone.Neutral)); - rows.Add(new SkillSourcesInventoryRow(SkillSourcesInventoryAction.RescanAll, null, null, "Rescan all", "Refresh local source status.", ConfigStatusTone.Neutral)); + rows.Add(new SkillSourcesInventoryRow(SkillSourcesInventoryAction.RescanAll, null, null, "Rescan all", "Refresh local status and remote skill counts.", ConfigStatusTone.Neutral)); rows.Add(new SkillSourcesInventoryRow(SkillSourcesInventoryAction.Done, null, null, "Done", "Return to Settings Areas.", ConfigStatusTone.Neutral)); return rows; } @@ -1822,6 +1974,10 @@ private void ReloadSources() var external = ConfigFileHelper.LoadSection(root, "ExternalSkills"); var feeds = LoadSkillFeedsSection(root); _sources = BuildSources(external, feeds).ToList(); + // A reload means a source's identity or config may have changed (rename, URL change, remove, + // enable/disable) — any of which invalidates a cached remote skill count. Drop the cache so a + // stale or misattributed count cannot survive the change; the next Rescan all repopulates it. + _remoteSkillCounts.Clear(); Version.Value++; } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or JsonException) @@ -2109,13 +2265,17 @@ private static string FormatSourceLabel(SkillSourceDisplay source) return $"[{enabled}] {FormatDisplayName(source.Name)}"; } - private static string FormatSourceDetail(SkillSourceDisplay source) + private string FormatSourceDetail(SkillSourceDisplay source) { if (source.Kind == SkillSourceKind.LocalFolder) return $"{TruncateMiddle(source.Location, 58)} | {source.StatusText}"; var auth = source.HasApiKey ? "Token configured" : "No auth"; - return $"{TruncateMiddle(HostOrLocation(source.Location), 42)} | {auth}"; + var detail = $"{TruncateMiddle(HostOrLocation(source.Location), 42)} | {auth}"; + // "advertised" = the count the server publishes; the daemon may load fewer after scan/hash filtering. + if (_remoteSkillCounts.TryGetValue(source.Name, out var count)) + detail += $" | {count} advertised"; + return detail; } private static string FormatDisplayName(string value)