Harden MCP OAuth lifecycle

[Aaronontheweb]

Summary

Swapping Netclaw's OAuth implementation for the MCP C# SDK's invalidated two credentials the previous release had been using successfully. Both notion and textforge were connected minutes before a binary swap and reported requires OAuth authorization minutes after. Notion recovered by reauthorizing; textforge could not, because re-registration is impossible against it.

• The SDK owns PKCE, authorization-code exchange, refresh, and bearer injection. Netclaw's parallel implementation of those (McpOAuthService, McpTokenCacheAdapter — 752 lines) is deleted, along with the mcp-oauth-metadata.json runtime dependency.
• Netclaw owns protected-resource discovery and RFC 7591 client registration, in McpOAuthClientRegistrar. This is a deliberate exception to SDK ownership — see below.
• Legacy credentials migrate instead of failing closed. An upgrade no longer invalidates a credential the previous release was using.
• Generation-aware client publication with a per-server gate, reconnect coalescing, and candidate-then-publish so a failed authorization cannot displace a working connection.
• Structured OAuth diagnostics, safe callback errors, a callback port derived from DaemonConfig.Port, and static Authorization header precedence.

Why registration is not delegated to the SDK

ClientOAuthProvider.PerformDynamicClientRegistrationAsync hard-codes token_endpoint_auth_method: "client_secret_post" and never consults the authorization server's advertised token_endpoint_auth_methods_supported. A server accepting public clients only — TextForge advertises ["none"] — rejects that with 400 invalid_client_metadata, and 1.4.1 exposes no hook to change it: DynamicClientRegistrationOptions has four properties and its ResponseDelegate fires only on success.

This is csharp-sdk#1611, open since 2026-05-29 and unfixed in 1.4.1, every 2.0 prerelease, and main. PR #1615 adds ClientOAuthOptions.TokenEndpointAuthMethod, which covers the token request only — the registration body is untouched, so it does not fix this.

Netclaw's own registration sent "none" and worked, as does the TypeScript SDK, which negotiates from the advertised list. The registrar registers with the same method the SDK will later select for the token request, so the two cannot diverge, and seeds ClientOAuthOptions.ClientId so the SDK's registration path never runs. A missing or failing registration_endpoint names OAuthClientId as the remedy.

Server-side tracking: petabridge/llm-email-gateway#784.

Upgrade-path fixes

Three defects made the migration cosmetic:

• ObtainedAt is stamped on migration. Absent from pre-existing records, it deserialized to 0001-01-01, and ExpiresAt - ObtainedAt saturated the int conversion to int.MaxValue, so the SDK treated every migrated credential as permanently expired. No test caught it because no fixture set ExpiresAt.
• McpServerUrl is retained rather than nulled, so migration can repeat if an endpoint is corrected later and a rollback keeps its binding.
• Resource equivalence tolerates a trailing slash, path case, and a bare-origin indicator. Scheme, host, port and query must agree; origin-to-path narrowing is accepted, a path-scoped credential is never widened to a sibling path. Rejections log both bindings instead of returning null silently.

LoadDurableState also no longer throws from a singleton constructor, where one unreadable secrets leaf took down daemon startup.

RejectedDynamicClientId is replaced by discarding the rejected identity and keeping the tokens. A persisted marker is a latch: it survives restart and, against a server that cannot complete registration, makes every later attempt fail identically.

Deferred, with the gaps recorded

These are unchecked tasks in openspec/changes/simplify-mcp-oauth-lifecycle/tasks.md, not silent drops:

• In-flight invocation draining (tasks 4.6, 4.8). A replaced or shutting-down client is disposed once its replacement publishes, so a call in flight is cancelled rather than drained. The previous release behaved the same way, so this is a non-improvement rather than a regression.
• Cross-process secrets locking and the remaining caller migration (task 7.2). SecretsFileWriter.Update locks within one process, and its key resolves a symlinked config directory. SecretsCommand, PairCommand, ProviderCommand, ProviderManagerViewModel, ExposureModeStepViewModel, ProviderCredentialWriter and OAuthTokenPersistence still do unlocked read-modify-write, as they did before this change — so a netclaw secrets set issued against a running daemon can still lose against a concurrent token refresh. The credential store and the two TUI save paths did adopt the transaction, the latter because they were overwriting daemon-written McpOAuthTokens.

Size

MCP production code is 2488 lines against 1370 before this branch (McpClientManager 1256 + credential store 604 + flow broker 386 + registrar 242, vs manager 618 + McpOAuthService 663 + McpTokenCacheAdapter 89). The increase is the diagnostics taxonomy for #1475, durable client identity, resource binding with migration, and candidate-then-publish — none of which existed before. Net-negative production LOC was predicted in the proposal and not achieved; the proposal now records the measured figure rather than the prediction.

Roughly 800 lines of work unrelated to MCP OAuth were removed from the branch and deferred to their own change: the cross-process mutex, the Netclaw.SecretsLockProbe project, and transactional rollback for ProviderRenamer and BootstrapDeviceSeeder.

Verification

• CI green on ubuntu-latest, macos-26 and windows-latest, plus native smoke on Linux and macOS, install-script smoke on all three, screenshot regression, Docker build, Slopwatch, copyright headers and SemVer conformance.
• Netclaw.Daemon.Tests: 907 passed, 0 failed.
• OpenSpec validation passes.
• Live upgrade verified: after a binary swap, a credential stored 2026-05-13 reconnected with 23 tools, with no reauthorization and with a temporary OAuthClientId override removed. Daemon log records Migrated legacy MCP OAuth credentials for 'textforge' after exact resource match.
• Behavioral evals waived for this change; deterministic SDK OAuth integration coverage is included.

Notes

• ModelContextProtocol.Core remains pinned to 1.4.1.
• MCP OAuth: track upstream single-flight refresh and harden Netclaw diagnostics #1696 remains open until the upstream single-flight refresh fix ships in a stable SDK package.
• OAuth redirect URI must derive from effective exposure mode #297 is advanced but not closed — the callback port is now configurable, remote callback identity is unresolved.
• The obsolete McpOAuthServerMetadata type and NetclawPaths.McpOAuthMetadataPath property are removed; CLI and HTTP contracts are unchanged.
• OpenSpec sync and archive remain post-merge work.

Fixes #1475

[Copilot]

Pull request overview

This PR hardens Netclaw’s MCP OAuth and client lifecycle by removing the daemon’s bespoke OAuth protocol implementation, delegating OAuth discovery/PKCE/DCR/token handling to the MCP .NET SDK, and tightening daemon-owned lifecycle coordination, persistence, and operator diagnostics.

Changes:

• Replace Netclaw-owned MCP OAuth protocol code with an SDK-driven flow broker + durable credential store boundary, plus safer HTTP/CLI error surfacing.
• Add cross-process transactional secrets.json mutation (path-scoped mutex + atomic replace) and migrate secrets writers to replay changes against the latest locked document.
• Improve MCP tool publication consistency and cancellation/error behavior, plus broaden regression coverage (end-to-end + concurrency/locking scenarios).

Reviewed changes

Copilot reviewed 63 out of 63 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
tests/Netclaw.SecretsLockProbe/Program.cs	Cross-process probe used to validate secrets.json interprocess locking/serialization.
tests/Netclaw.SecretsLockProbe/Netclaw.SecretsLockProbe.csproj	Adds probe test project to the repo.
src/Netclaw.Providers/OAuth/OAuthTokenPersistence.cs	Switch provider token persistence to transactional SecretsFileWriter.Update.
src/Netclaw.Daemon/Security/BootstrapDeviceSeeder.cs	Persist bootstrap device token via transactional secrets update and add rollback semantics.
src/Netclaw.Daemon/Properties/AssemblyInfo.cs	Exposes daemon internals to CLI test assembly.
src/Netclaw.Daemon/Program.cs	Replaces McpOAuthService wiring with credential store + flow broker + runtime abstraction.
src/Netclaw.Daemon/Mcp/McpTokenCacheAdapter.cs	Removes legacy token-cache adapter used by the deleted manual OAuth stack.
src/Netclaw.Daemon/Mcp/McpOAuthService.cs	Removes daemon-owned OAuth discovery/DCR/PKCE/exchange/refresh implementation.
src/Netclaw.Daemon/Mcp/McpOAuthFlowBroker.cs	Introduces brokered, daemon-owned interactive OAuth flow state/lifetime coordination.
src/Netclaw.Daemon/Mcp/McpEndpointRouteBuilderExtensions.cs	Updates MCP OAuth endpoints and status DTOs; adds structured error payloads and safer callback handling.
src/Netclaw.Daemon.Tests/Security/BootstrapDeviceSeederTests.cs	Adds regression coverage for transactional secrets commit/rollback behavior.
src/Netclaw.Daemon.Tests/Netclaw.Daemon.Tests.csproj	Adds MCP ASP.NET Core dependency for updated MCP/OAuth test coverage.
src/Netclaw.Daemon.Tests/Mcp/McpTokenCacheAdapterTests.cs	Removes tests for deleted McpTokenCacheAdapter.
src/Netclaw.Daemon.Tests/Mcp/McpSmokeHarness.cs	Rewires harness to use credential store + flow broker instead of McpOAuthService/PKCE HTTP.
src/Netclaw.Daemon.Tests/Mcp/McpReconnectionServiceTests.cs	Updates status shaping to include lastErrorAt timestamp.
src/Netclaw.Daemon.Tests/Mcp/McpOAuthServiceTests.cs	Removes tests for deleted McpOAuthService.
src/Netclaw.Daemon.Tests/Mcp/McpOAuthHeaderConflictTests.cs	Removes tests tied to old OAuth stack/header conflict logic.
src/Netclaw.Daemon.Tests/Mcp/McpOAuthFlowBrokerTests.cs	Adds unit coverage for broker semantics (coalescing, expiry, one-time state, cancellation).
src/Netclaw.Daemon.Tests/Mcp/McpEndpointRouteBuilderExtensionsTests.cs	Updates endpoint tests for new brokered flow + structured errors + lastErrorAt exposure.
src/Netclaw.Daemon.Tests/Mcp/McpClientManagerStatusTests.cs	Strengthens redaction/safe error formatting and verifies timestamp propagation.
src/Netclaw.Daemon.Tests/Gateway/DaemonRuntimeStatusServiceTests.cs	Updates MCP health reporting integration to new credential/broker wiring.
src/Netclaw.Configuration/SensitiveString.cs	Updates converter docs to reflect new MCP OAuth credential store usage.
src/Netclaw.Configuration/SecretsFileWriter.cs	Adds path-scoped cross-process locking and transactional update API for secrets.json mutations.
src/Netclaw.Configuration/NetclawPaths.cs	Removes deprecated MCP OAuth metadata path.
src/Netclaw.Configuration/McpOAuthTokenSet.cs	Expands durable MCP OAuth credential model (active/pending envelope + binding/epoch fields).
src/Netclaw.Configuration/McpOAuthServerMetadata.cs	Removes obsolete discovery metadata cache type.
src/Netclaw.Configuration.Tests/SecretsFileWriterTests.cs	Adds concurrency, cross-process, and loud-failure regression tests for secrets transactions.
src/Netclaw.Configuration.Tests/Netclaw.Configuration.Tests.csproj	Adds non-output reference to lock-probe project for test execution.
src/Netclaw.Cli/Tui/Wizard/WizardConfigBuilder.cs	Replays wizard secret contributions against latest locked secrets.json instead of stale snapshots.
src/Netclaw.Cli/Tui/Wizard/Steps/ExposureModeStepViewModel.cs	Writes local device token via UpdateSecretsFile transaction helper.
src/Netclaw.Cli/Tui/Sections/ConfigEditorSession.cs	Replays secret mutations against latest secrets.json on save (avoids overwriting concurrent updates).
src/Netclaw.Cli/Tui/ProviderManagerViewModel.cs	Migrates provider secret writes/removals to transactional UpdateSecretsFile.
src/Netclaw.Cli/Secrets/SecretsCommand.cs	Migrates secrets set to transactional update.
src/Netclaw.Cli/Provider/ProviderRenamer.cs	Makes rename atomic-ish across config+secrets with rollback on failure; collision check moved into secrets transaction.
src/Netclaw.Cli/Provider/ProviderCommand.cs	Migrates provider remove secret deletion to transactional UpdateSecretsFile.
src/Netclaw.Cli/Mcp/McpCommand.cs	Improves daemon error propagation/formatting (structured errors, status fallback) and avoids blank error output.
src/Netclaw.Cli/Daemon/PairCommand.cs	Writes device token via transactional UpdateSecretsFile.
src/Netclaw.Cli/Config/ProviderCredentialWriter.cs	Migrates provider API key persistence to transactional UpdateSecretsFile.
src/Netclaw.Cli/Config/ConfigFileHelper.cs	Adds UpdateSecretsFile helpers bridging dictionary mutations onto SecretsFileWriter.Update.
src/Netclaw.Cli.Tests/Tui/Wizard/WizardConfigBuilderTests.cs	Adds regression test for replaying wizard secret contributions against latest secrets.json.
src/Netclaw.Cli.Tests/Tui/Sections/ConfigEditorSessionTests.cs	Adds regression test for replaying config editor secret actions without clobbering refreshed MCP tokens.
src/Netclaw.Cli.Tests/Provider/ProviderRenamerTests.cs	Adds rollback/collision/failure-mode coverage for new rename flow.
src/Netclaw.Cli.Tests/Mcp/McpOAuthEndToEndTests.cs	End-to-end regression for bodyless DCR failures propagating through daemon serialization and CLI output.
src/Netclaw.Cli.Tests/Mcp/McpCommandTests.cs	Adds structured error parsing + empty/malformed body fallback tests.
src/Netclaw.Actors/Tools/ToolRegistry.cs	Adds synchronization and atomic MCP tool publication hook to keep registry and snapshot state consistent.
src/Netclaw.Actors/Tools/ToolRegistrationExtensions.cs	Splits MCP tool adapter construction from registration to support new atomic publication pattern.
src/Netclaw.Actors/Tools/McpToolAdapter.cs	Ensures caller cancellation propagates rather than being formatted as a tool error string.
src/Netclaw.Actors.Tests/Tools/McpToolAdapterTests.cs	Adds regression test verifying cancellation propagation behavior.
openspec/changes/simplify-mcp-oauth-lifecycle/tasks.md	Tracks implementation tasks and completion state for this change.
openspec/changes/simplify-mcp-oauth-lifecycle/specs/transactional-secrets/spec.md	Defines requirements/scenarios for transactional secrets mutations.
openspec/changes/simplify-mcp-oauth-lifecycle/specs/netclaw-mcp/spec.md	Updates MCP lifecycle/diagnostic requirements for the new model.
openspec/changes/simplify-mcp-oauth-lifecycle/specs/mcp-oauth/spec.md	Defines SDK-owned OAuth ownership boundary, brokered interactive flow, and diagnostic requirements.
openspec/changes/simplify-mcp-oauth-lifecycle/proposal.md	Change proposal and rationale for the refactor.
openspec/changes/simplify-mcp-oauth-lifecycle/.openspec.yaml	Adds OpenSpec change metadata.
Netclaw.slnx	Adds the new lock-probe test project to the solution.
feeds/skills/.system/files/netclaw-operations/SKILL.md	Updates operational guidance for new MCP OAuth flow/diagnostics and bumps skill version.
docs/prd/PRD-006-mcp-tool-integration.md	Updates PRD requirements to reflect SDK-owned OAuth + concurrent lifecycle hardening.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/Netclaw.Configuration/SecretsFileWriter.cs:262

• AcquireSecretsLock only serializes secrets.json mutations within the current process (SemaphoreSlim). That still allows lost updates when two processes write concurrently (e.g., daemon refreshing MCP OAuth tokens while a CLI/config command updates another secrets section). This also conflicts with the PR’s stated goal of serializing secrets.json mutations across processes.

Consider using the existing cross-process named Mutex pattern (SHA-256 of canonicalized path) so the lock spans read/decrypt/mutate/encrypt/atomic-replace across processes, not just threads.

    /// <summary>
    /// Serializes read-modify-write against one secrets file <em>within this process</em>.
    ///
    /// This does NOT serialize against another process. Nothing stops `netclaw secrets set`
    /// or `netclaw provider add` from running while the daemon is live, and those paths still

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

src/Netclaw.Configuration/SecretsFileWriter.cs:263

• The PR description says secrets.json mutations are serialized across processes, but the new gate explicitly states it only serializes within the current process and that cross-process locking + migrating remaining callers are deferred. This is a material discrepancy for operators relying on the PR summary (CLI writes can still race the daemon).

    /// Serializes read-modify-write against one secrets file <em>within this process</em>.
    ///
    /// This does NOT serialize against another process. Nothing stops `netclaw secrets set`
    /// or `netclaw provider add` from running while the daemon is live, and those paths still
    /// write the file without taking this gate, so a CLI write can still lose against a

src/Netclaw.Daemon/Mcp/McpOAuthCredentialStore.cs:325

• There is a duplicated if (cache.Published || !cache.ExplicitAuthorization) statement, which looks like an accidental copy/paste and adds noise in a security-sensitive persistence path.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

src/Netclaw.Daemon/Mcp/McpOAuthCredentialStore.cs:324

• Redundant nested if statements here (if (cache.Published || !cache.ExplicitAuthorization) repeated twice) make the control flow harder to read and look accidental. Collapsing to a single if keeps the same behavior and avoids confusion.
  src/Netclaw.Configuration/SecretsFileWriter.cs:263
• PR description says secrets.json mutations are serialized across processes, but this implementation (and its doc comment) explicitly states the lock only serializes within the current process. Please either update the PR description to match reality, or add cross-process locking (e.g., a named mutex keyed by the canonical path) and migrate remaining writers to use it.

    /// Serializes read-modify-write against one secrets file <em>within this process</em>.
    ///
    /// This does NOT serialize against another process. Nothing stops `netclaw secrets set`
    /// or `netclaw provider add` from running while the daemon is live, and those paths still
    /// write the file without taking this gate, so a CLI write can still lose against a

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

src/Netclaw.Daemon/Mcp/McpOAuthClientRegistrar.cs:25

• The PR description says the MCP SDK is the sole owner of OAuth discovery and dynamic client registration (DCR), but this new component performs protected-resource discovery + RFC 7591 registration in Netclaw. Please reconcile the PR description (or call out this intentional exception) so reviewers/operators don't assume DCR moved into the SDK runtime path.
  src/Netclaw.Configuration/SecretsFileWriter.cs:263
• The PR description claims secrets.json mutations are serialized "across processes", but SecretsFileWriter's lock is explicitly in-process only (SemaphoreSlim in a static dictionary). Either update the PR description to match current behavior, or implement a cross-process lock (e.g., named mutex keyed by canonical path) if cross-process serialization is a hard requirement.

    /// Serializes read-modify-write against one secrets file <em>within this process</em>.
    ///
    /// This does NOT serialize against another process. Nothing stops `netclaw secrets set`
    /// or `netclaw provider add` from running while the daemon is live, and those paths still
    /// write the file without taking this gate, so a CLI write can still lose against a

src/Netclaw.Daemon/Mcp/McpOAuthFlowBroker.cs:281

• If the MCP SDK cancels the redirect-delegate call (sdkCancellation) before the browser callback delivers a code, _delegateOwner remains claimed and the flow can get stuck in "authorization already in progress" until expiry. That blocks subsequent delegate invocations from owning the flow and can wedge explicit auth/reconnect attempts for up to 5 minutes.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

src/Netclaw.Configuration/SecretsFileWriter.cs:9

• Unused using directive using System.Text; (no Encoding/StringBuilder usage in this file). Consider removing to keep the file warning-clean.

using System.Collections.Concurrent;
using System.Text;
using System.Text.Json;
using System.Text.Json.Nodes;

src/Netclaw.Configuration.Tests/SecretsFileWriterTests.cs:10

• Unused using directive using System.Diagnostics; (no diagnostics types referenced). Consider removing to keep the test project warning-clean.

using System.Diagnostics;
using System.Runtime.InteropServices;
using System.Text.Json;
using System.Text.Json.Nodes;
using Netclaw.Configuration.Secrets;

src/Netclaw.Daemon/Mcp/McpOAuthClientRegistrar.cs:204

• TryReadAuthorizationServerAsync treats any HttpRequestException (including transient network errors or HTTP 5xx) as “no OAuth metadata”, which can misclassify an OAuth-protected server as unauthenticated and let the SDK’s own DCR path run (undoing the registrar’s purpose). Consider only treating 404 / invalid JSON as “no metadata” and failing explicit authorization loudly for other HTTP/network errors.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/Netclaw.Daemon/Mcp/McpOAuthFlowBroker.cs:282

• HandleAuthorizationRedirectAsync permanently claims _delegateOwner via CompareExchange but never releases it if the SDK cancels the delegate before the callback delivers a code. In that case the flow stays Pending but future delegate invocations will throw McpOAuthAuthorizationInProgressException, effectively wedging the flow until expiry even though no owner is waiting anymore.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (6)

src/Netclaw.Daemon/Mcp/McpEndpointRouteBuilderExtensions.cs:1

• The PR description says “CLI and HTTP contracts are unchanged,” but these public response DTOs have new fields (LastErrorAt, structured Error, and extra McpErrorResponse members). If external clients consume these endpoints, this is an API contract change; either (mandatory) update the PR description to reflect the contract change (and/or document versioning/compat expectations), or (optional) consider keeping backward-compatible shapes (e.g., additive fields are usually OK, but any changed status codes or client expectations should be explicitly called out).
  src/Netclaw.Daemon/Mcp/McpOAuthFlowBroker.cs:1
• PruneTombstones() uses flow.ExpiresAt as the pruning timestamp, but ExpiresAt is set to “created-at + FlowLifetime”. That means a flow that completes quickly will typically be retained for ~2× FlowLifetime (until now - FlowLifetime >= created-at + FlowLifetime). (Mandatory) Either adjust the pruning condition to reflect the intended retention window (e.g., prune based on a terminal timestamp) or rename/comment the logic so the actual retention policy is unambiguous and matches the code.
  src/Netclaw.Daemon/Mcp/McpOAuthFlowBroker.cs:1
• redirectUri is not used. If analyzers are enabled, this can create noise and also misses an opportunity to validate that the SDK-provided redirect matches the daemon’s expected callback (which would strengthen callback safety). (Optional) Either remove the parameter if it’s never needed, or use it for validation/logging (or explicitly discard it) to make the intent clear.
  src/Netclaw.Actors/Tools/ToolRegistry.cs:80
• GetRegistrationsSnapshot() allocates a new list on every read, and many public APIs now call it. If these methods sit on hot paths (tool selection/suggestion/index generation), this can add noticeable allocation/GC pressure. (Optional) Consider maintaining a volatile immutable snapshot (e.g., ImmutableArray<ToolRegistration> or ToolRegistration[]) updated on writes, so reads are lock-free and allocation-free while preserving thread safety.

    /// <summary>All registered tools as AITool for ChatOptions.Tools.</summary>
    public IReadOnlyList<AITool> GetAllTools() =>
        GetRegistrationsSnapshot().Select(t => t.Tool.ToAITool()).ToList();

src/Netclaw.Actors/Tools/ToolRegistry.cs:466

• GetRegistrationsSnapshot() allocates a new list on every read, and many public APIs now call it. If these methods sit on hot paths (tool selection/suggestion/index generation), this can add noticeable allocation/GC pressure. (Optional) Consider maintaining a volatile immutable snapshot (e.g., ImmutableArray<ToolRegistration> or ToolRegistration[]) updated on writes, so reads are lock-free and allocation-free while preserving thread safety.

    private IReadOnlyList<ToolRegistration> GetRegistrationsSnapshot()
    {
        lock (_sync)
            return _tools.ToList();
    }

src/Netclaw.Configuration/SecretsFileWriter.cs:277

• Gates is a static dictionary that never evicts entries. If tests or tooling create many temporary secretsPath values in-process, this can grow unbounded over time. (Optional) If Netclaw can truly only ever use one secrets path per process, documenting that assumption here would help; otherwise consider an eviction strategy (or a different lock identity approach) to prevent long-lived growth.

    private static readonly ConcurrentDictionary<string, SemaphoreSlim> Gates = new(StringComparer.Ordinal);

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/Netclaw.Daemon/Mcp/McpOAuthClientRegistrar.cs:227

• DescribeOAuthError includes provider-controlled error / error_description text verbatim in the exception message. That message is logged (and OTLP-exported), so newlines or very long values can lead to log forging/noisy multi-line entries and unbounded log payloads. Sanitize these fields (collapse CR/LF to spaces, trim, and cap length) before formatting them into the message.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

src/Netclaw.Daemon/Mcp/McpOAuthFlowBroker.cs:365

• EndLifetime() calls _expiryTimer.Change(...) without handling ObjectDisposedException. During daemon shutdown, McpOAuthFlowBroker.Dispose() can dispose the flow while another thread completes/fails it, which can throw here and surface as an unhandled shutdown-path exception.
  src/Netclaw.Daemon/Mcp/McpOAuthClientRegistrar.cs:172
• token_endpoint_auth_methods_supported parsing keeps empty/whitespace entries. If the auth server returns "none" with surrounding whitespace (or an empty string entry), FirstOrDefault() can select an invalid method and the DCR request will fail.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.