Address PR review hardening feedback

Telli · Telli · commit 69c69cc4198e · 2026-05-10T02:14:16.000-07:00
diff --git a/docs/testing/test-run-report.md b/docs/testing/test-run-report.md
@@ -1,6 +1,6 @@
 # Agent Testing Run Report
 
-Generated: `2026-05-10T06:07:15.3869410+00:00`
+Generated: `2026-05-10T09:13:50.5670530+00:00`
 Gate status: **PASS**
 
 ## Gates
@@ -17,10 +17,10 @@ Gate status: **PASS**
 
 | Scenario | Risk | Status | Trace |
 |----------|------|--------|-------|
-| approval-required | High | PASS | ../../artifacts/testing/traces/approval-required-d92e073423fa46d6b7642445703ba21b.trace.json |
-| basic-tool-call | Low | PASS | ../../artifacts/testing/traces/basic-tool-call-85bf00b5ce654f178528a604baa0d360.trace.json |
-| timeout-retry-placeholder | Medium | PASS | ../../artifacts/testing/traces/timeout-retry-placeholder-ef372e2a92834b63b9070d10f0c1628f.trace.json |
-| unsafe-tool-blocked | Critical | PASS | ../../artifacts/testing/traces/unsafe-tool-blocked-a6cea282048e4622bfc7ea02b2d664f1.trace.json |
+| approval-required | High | PASS | ../../artifacts/testing/traces/approval-required-38827d895786449094bbd28d8b640055.trace.json |
+| basic-tool-call | Low | PASS | ../../artifacts/testing/traces/basic-tool-call-01ea2af915aa442ea785c520fc90d869.trace.json |
+| timeout-retry-placeholder | Medium | PASS | ../../artifacts/testing/traces/timeout-retry-placeholder-6621dde240b1468a87f98735d6af81cd.trace.json |
+| unsafe-tool-blocked | Critical | PASS | ../../artifacts/testing/traces/unsafe-tool-blocked-8554b8c5b4304acfa242c050a835a57a.trace.json |
 
 ## Oracle Results
 
diff --git a/src/SharpClaw.Code.Providers/OpenAiCompatibleProvider.cs b/src/SharpClaw.Code.Providers/OpenAiCompatibleProvider.cs
@@ -1,5 +1,8 @@
+using System.Collections.Concurrent;
 using System.ClientModel;
 using System.Runtime.CompilerServices;
+using System.Security.Cryptography;
+using System.Text;
 using Microsoft.Extensions.AI;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
@@ -23,6 +26,7 @@ public sealed class OpenAiCompatibleProvider(
     ILogger<OpenAiCompatibleProvider> logger) : IModelProvider
 {
     private readonly OpenAiCompatibleProviderOptions _options = options.Value;
+    private readonly ConcurrentDictionary<OpenAiClientCacheKey, OpenAIClient> _clientCache = new();
     internal const string RuntimeProfileMetadataKey = "openai-compatible.profile";
 
     /// <inheritdoc />
@@ -62,7 +66,7 @@ private async IAsyncEnumerable<ProviderEvent> StreamEventsAsync(
         var modelId = Internal.ProviderHttpHelpers.ResolveModelOrDefault(
             request.Model,
             profile?.DefaultChatModel ?? _options.DefaultModel);
-        var openAiClient = CreateOpenAiClient(profile, resolvedApiKey);
+        var openAiClient = GetOrCreateOpenAiClient(profile, resolvedApiKey);
         var nativeClient = openAiClient.GetChatClient(modelId);
         using var chatClient = nativeClient.AsIChatClient();
 
@@ -95,20 +99,29 @@ private async IAsyncEnumerable<ProviderEvent> StreamEventsAsync(
         }
     }
 
-    private OpenAIClient CreateOpenAiClient(LocalRuntimeProfileOptions? profile, string? resolvedApiKey)
+    private OpenAIClient GetOrCreateOpenAiClient(LocalRuntimeProfileOptions? profile, string? resolvedApiKey)
     {
-        var openAiOptions = new OpenAIClientOptions();
         var normalized = Internal.ProviderHttpHelpers.NormalizeBaseUrl(profile?.BaseUrl ?? _options.BaseUrl);
-        if (normalized is not null)
-        {
-            openAiOptions.Endpoint = new Uri(normalized);
-        }
-
         var apiKey = profile?.ApiKey ?? resolvedApiKey ?? _options.ApiKey ?? "local-runtime";
-        var credential = new ApiKeyCredential(apiKey);
-        return new OpenAIClient(credential, openAiOptions);
+        var cacheKey = new OpenAiClientCacheKey(normalized, ComputeCredentialFingerprint(apiKey));
+        return _clientCache.GetOrAdd(
+            cacheKey,
+            static (_, state) =>
+            {
+                var openAiOptions = new OpenAIClientOptions();
+                if (state.NormalizedEndpoint is not null)
+                {
+                    openAiOptions.Endpoint = new Uri(state.NormalizedEndpoint);
+                }
+
+                return new OpenAIClient(new ApiKeyCredential(state.ApiKey), openAiOptions);
+            },
+            (NormalizedEndpoint: normalized, ApiKey: apiKey));
     }
 
+    private static string ComputeCredentialFingerprint(string apiKey)
+        => Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(apiKey)));
+
     private static List<Microsoft.Extensions.AI.ChatMessage> BuildChatMessages(ProviderRequest request)
     {
         var messages = new List<Microsoft.Extensions.AI.ChatMessage>();
@@ -144,4 +157,6 @@ private async Task<ResolvedProviderCredential> ResolveCredentialAsync(Cancellati
 
         return await credentialStore.ResolveAsync(ProviderName, cancellationToken).ConfigureAwait(false);
     }
+
+    private readonly record struct OpenAiClientCacheKey(string? Endpoint, string CredentialFingerprint);
 }
diff --git a/src/SharpClaw.Code.Providers/Services/ProviderCredentialStore.cs b/src/SharpClaw.Code.Providers/Services/ProviderCredentialStore.cs
@@ -113,11 +113,22 @@ private async Task<StoredCredentialDocument> LoadAsync(CancellationToken cancell
         var text = await fileSystem.ReadAllTextIfExistsAsync(path, cancellationToken).ConfigureAwait(false);
         if (string.IsNullOrWhiteSpace(text))
         {
-            return new StoredCredentialDocument(new Dictionary<string, StoredCredentialEntry>(StringComparer.OrdinalIgnoreCase));
+            return CreateEmptyDocument();
         }
 
-        return JsonSerializer.Deserialize<StoredCredentialDocument>(text, JsonOptions)
-            ?? new StoredCredentialDocument(new Dictionary<string, StoredCredentialEntry>(StringComparer.OrdinalIgnoreCase));
+        try
+        {
+            var document = JsonSerializer.Deserialize<StoredCredentialDocument>(text, JsonOptions);
+            return document?.Providers is null
+                ? CreateEmptyDocument()
+                : new StoredCredentialDocument(document.Providers
+                    .Where(static pair => pair.Value is not null)
+                    .ToDictionary(static pair => pair.Key, static pair => pair.Value, StringComparer.OrdinalIgnoreCase));
+        }
+        catch (JsonException)
+        {
+            return CreateEmptyDocument();
+        }
     }
 
     private Task SaveAsync(StoredCredentialDocument document, CancellationToken cancellationToken)
@@ -126,6 +137,9 @@ private Task SaveAsync(StoredCredentialDocument document, CancellationToken canc
     private string GetPath()
         => pathService.Combine(userProfilePaths.GetUserSharpClawRoot(), CredentialsFileName);
 
+    private static StoredCredentialDocument CreateEmptyDocument()
+        => new(new Dictionary<string, StoredCredentialEntry>(StringComparer.OrdinalIgnoreCase));
+
     private sealed record StoredCredentialDocument(
         Dictionary<string, StoredCredentialEntry> Providers);
 
diff --git a/src/SharpClaw.Code.Runtime/Prompts/PromptReferenceResolver.cs b/src/SharpClaw.Code.Runtime/Prompts/PromptReferenceResolver.cs
@@ -22,6 +22,7 @@ public sealed partial class PromptReferenceResolver(
 {
     private const int MaxDirectoryReferenceFiles = 20;
     private const int MaxDirectoryReferenceBytes = 200 * 1024;
+    private const long MaxImageReferenceBytes = 8 * 1024 * 1024;
     private static readonly HashSet<string> ImageExtensions = new(StringComparer.OrdinalIgnoreCase)
     {
         ".png", ".jpg", ".jpeg", ".gif", ".webp"
@@ -230,8 +231,35 @@ private static string ToDisplayPath(string workspaceRootFull, string workingDire
 
         if (ImageExtensions.Contains(Path.GetExtension(resolvedFull)))
         {
-            var bytes = await File.ReadAllBytesAsync(resolvedFull, cancellationToken).ConfigureAwait(false);
+            cancellationToken.ThrowIfCancellationRequested();
+            var fileInfo = new FileInfo(resolvedFull);
+            if (!fileInfo.Exists)
+            {
+                throw new InvalidOperationException($"Referenced path is missing or unreadable: '{resolvedFull}'.");
+            }
+
             var mediaType = ResolveMediaType(resolvedFull);
+            if (fileInfo.Length > MaxImageReferenceBytes)
+            {
+                var omittedPlaceholder =
+                    $"[Referenced image omitted: {display} exceeds {FormatBytes(MaxImageReferenceBytes)}]" + Environment.NewLine
+                    + $"[End referenced image: {display}]";
+                return (
+                    omittedPlaceholder,
+                    new PromptReference(
+                        PromptReferenceKind.Image,
+                        rawToken,
+                        pathPart,
+                        resolvedFull,
+                        display,
+                        outsideWorkspace,
+                        omittedPlaceholder,
+                        MediaType: mediaType,
+                        IncludedEntryCount: 0),
+                    null);
+            }
+
+            var bytes = await File.ReadAllBytesAsync(resolvedFull, cancellationToken).ConfigureAwait(false);
             var placeholder =
                 $"[Referenced image: {display} ({mediaType})]" + Environment.NewLine
                 + $"[End referenced image: {display}]";
@@ -287,14 +315,9 @@ private static string ToDisplayPath(string workspaceRootFull, string workingDire
         string display,
         CancellationToken cancellationToken)
     {
-        var files = Directory.EnumerateFiles(directoryPath, "*", SearchOption.AllDirectories)
-            .Where(static path => !ShouldSkipPath(path))
-            .OrderBy(static path => path, StringComparer.OrdinalIgnoreCase)
-            .ToArray();
-
         var included = new List<(string RelativePath, string Content)>();
         var totalBytes = 0;
-        foreach (var file in files)
+        foreach (var file in EnumerateReferenceFiles(directoryPath, cancellationToken))
         {
             cancellationToken.ThrowIfCancellationRequested();
             if (included.Count >= MaxDirectoryReferenceFiles)
@@ -307,12 +330,28 @@ private static string ToDisplayPath(string workspaceRootFull, string workingDire
                 continue;
             }
 
+            long fileLength;
+            try
+            {
+                fileLength = new FileInfo(file).Length;
+            }
+            catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
+            {
+                continue;
+            }
+
+            var remainingBytes = MaxDirectoryReferenceBytes - totalBytes;
+            if (fileLength > remainingBytes)
+            {
+                break;
+            }
+
             string text;
             try
             {
                 text = await File.ReadAllTextAsync(file, cancellationToken).ConfigureAwait(false);
             }
-            catch
+            catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or DecoderFallbackException)
             {
                 continue;
             }
@@ -356,10 +395,73 @@ private static string ToDisplayPath(string workspaceRootFull, string workingDire
         return (builder.ToString(), included.Count);
     }
 
+    private static IEnumerable<string> EnumerateReferenceFiles(string directoryPath, CancellationToken cancellationToken)
+    {
+        var pending = new Stack<string>();
+        pending.Push(directoryPath);
+
+        while (pending.Count > 0)
+        {
+            cancellationToken.ThrowIfCancellationRequested();
+            var current = pending.Pop();
+            foreach (var file in EnumerateSortedFiles(current))
+            {
+                cancellationToken.ThrowIfCancellationRequested();
+                if (!ShouldSkipPath(file))
+                {
+                    yield return file;
+                }
+            }
+
+            var directories = EnumerateSortedDirectories(current);
+            for (var i = directories.Length - 1; i >= 0; i--)
+            {
+                cancellationToken.ThrowIfCancellationRequested();
+                if (!ShouldSkipPath(directories[i]))
+                {
+                    pending.Push(directories[i]);
+                }
+            }
+        }
+    }
+
+    private static string[] EnumerateSortedFiles(string directoryPath)
+    {
+        try
+        {
+            return Directory.EnumerateFiles(directoryPath, "*", SearchOption.TopDirectoryOnly)
+                .OrderBy(static path => path, StringComparer.OrdinalIgnoreCase)
+                .ToArray();
+        }
+        catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
+        {
+            return [];
+        }
+    }
+
+    private static string[] EnumerateSortedDirectories(string directoryPath)
+    {
+        try
+        {
+            return Directory.EnumerateDirectories(directoryPath, "*", SearchOption.TopDirectoryOnly)
+                .OrderBy(static path => path, StringComparer.OrdinalIgnoreCase)
+                .ToArray();
+        }
+        catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
+        {
+            return [];
+        }
+    }
+
     private static bool ShouldSkipPath(string path)
         => path.Split(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)
             .Any(static segment => segment is ".git" or ".sharpclaw" or "bin" or "obj");
 
+    private static string FormatBytes(long bytes)
+        => bytes >= 1024 * 1024
+            ? $"{bytes / (1024 * 1024)} MiB"
+            : $"{bytes / 1024} KiB";
+
     private static string ResolveMediaType(string path)
         => Path.GetExtension(path).ToLowerInvariant() switch
         {
diff --git a/src/SharpClaw.Code.Runtime/Server/WorkspaceHttpServer.cs b/src/SharpClaw.Code.Runtime/Server/WorkspaceHttpServer.cs
@@ -52,13 +52,13 @@ public async Task RunAsync(
         var effectivePort = port is > 0 ? port.Value : config.Document.Server?.Port ?? 7345;
         var prefix = $"http://{effectiveHost}:{effectivePort}/";
 
-        using var listener = new HttpListener();
-        listener.Prefixes.Add(prefix);
-        listener.Start();
-        logger.LogInformation("SharpClaw server listening on {Prefix}", prefix);
-
+        var listener = new HttpListener();
         try
         {
+            listener.Prefixes.Add(prefix);
+            listener.Start();
+            logger.LogInformation("SharpClaw server listening on {Prefix}", prefix);
+
             while (!cancellationToken.IsCancellationRequested)
             {
                 HttpListenerContext httpContext;
@@ -82,10 +82,21 @@ public async Task RunAsync(
         }
         finally
         {
-            if (listener.IsListening)
-            {
-                listener.Stop();
-            }
+            CloseListener(listener);
+        }
+    }
+
+    private static void CloseListener(HttpListener listener)
+    {
+        try
+        {
+            listener.Close();
+        }
+        catch (HttpListenerException)
+        {
+        }
+        catch (ObjectDisposedException)
+        {
         }
     }
 
diff --git a/tests/SharpClaw.Code.IntegrationTests/Runtime/ApprovalAuthIntegrationTests.cs b/tests/SharpClaw.Code.IntegrationTests/Runtime/ApprovalAuthIntegrationTests.cs
@@ -279,10 +279,7 @@ public string CreateToken(string subjectId, string tenantId)
         public async ValueTask DisposeAsync()
         {
             cancellationTokenSource.Cancel();
-            if (listener.IsListening)
-            {
-                listener.Stop();
-            }
+            CloseListener();
 
             try
             {
@@ -294,12 +291,29 @@ public async ValueTask DisposeAsync()
             catch (HttpListenerException)
             {
             }
+            catch (ObjectDisposedException)
+            {
+            }
 
-            listener.Close();
+            CloseListener();
             rsa.Dispose();
             cancellationTokenSource.Dispose();
         }
 
+        private void CloseListener()
+        {
+            try
+            {
+                listener.Close();
+            }
+            catch (HttpListenerException)
+            {
+            }
+            catch (ObjectDisposedException)
+            {
+            }
+        }
+
         private async Task RunAsync(CancellationToken cancellationToken)
         {
             while (!cancellationToken.IsCancellationRequested)
@@ -317,6 +331,10 @@ private async Task RunAsync(CancellationToken cancellationToken)
                 {
                     break;
                 }
+                catch (ObjectDisposedException) when (cancellationToken.IsCancellationRequested)
+                {
+                    break;
+                }
 
                 _ = Task.Run(() => HandleAsync(context, cancellationToken), CancellationToken.None);
             }
diff --git a/tests/SharpClaw.Code.UnitTests/Providers/ProviderCredentialStoreTests.cs b/tests/SharpClaw.Code.UnitTests/Providers/ProviderCredentialStoreTests.cs
diff --git a/tests/SharpClaw.Code.UnitTests/Runtime/PromptReferenceResolverTests.cs b/tests/SharpClaw.Code.UnitTests/Runtime/PromptReferenceResolverTests.cs

Original file line number	Diff line number	Diff line change
`@@ -279,10 +279,7 @@ public string CreateToken(string subjectId, string tenantId)`
`279`	`279`	`public async ValueTask DisposeAsync()`
`280`	`280`	`{`
`281`	`281`	`cancellationTokenSource.Cancel();`
`282`		`- if (listener.IsListening)`
`283`		`- {`
`284`		`- listener.Stop();`
`285`		`- }`
	`282`	`+ CloseListener();`
`286`	`283`
`287`	`284`	`try`
`288`	`285`	`{`
`@@ -294,12 +291,29 @@ public async ValueTask DisposeAsync()`
`294`	`291`	`catch (HttpListenerException)`
`295`	`292`	`{`
`296`	`293`	`}`
	`294`	`+ catch (ObjectDisposedException)`
	`295`	`+ {`
	`296`	`+ }`
`297`	`297`
`298`		`- listener.Close();`
	`298`	`+ CloseListener();`
`299`	`299`	`rsa.Dispose();`
`300`	`300`	`cancellationTokenSource.Dispose();`
`301`	`301`	`}`
`302`	`302`
	`303`	`+ private void CloseListener()`
	`304`	`+ {`
	`305`	`+ try`
	`306`	`+ {`
	`307`	`+ listener.Close();`
	`308`	`+ }`
	`309`	`+ catch (HttpListenerException)`
	`310`	`+ {`
	`311`	`+ }`
	`312`	`+ catch (ObjectDisposedException)`
	`313`	`+ {`
	`314`	`+ }`
	`315`	`+ }`
	`316`	`+`
`303`	`317`	`private async Task RunAsync(CancellationToken cancellationToken)`
`304`	`318`	`{`
`305`	`319`	`while (!cancellationToken.IsCancellationRequested)`
`@@ -317,6 +331,10 @@ private async Task RunAsync(CancellationToken cancellationToken)`
`317`	`331`	`{`
`318`	`332`	`break;`
`319`	`333`	`}`
	`334`	`+ catch (ObjectDisposedException) when (cancellationToken.IsCancellationRequested)`
	`335`	`+ {`
	`336`	`+ break;`
	`337`	`+ }`
`320`	`338`
`321`	`339`	`_ = Task.Run(() => HandleAsync(context, cancellationToken), CancellationToken.None);`
`322`	`340`	`}`