Harden MCP OAuth lifecycle - #1708
Draft
Aaronontheweb wants to merge 8 commits into
Draft
Conversation
There was a problem hiding this comment.
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.
Swapping Netclaw's OAuth implementation for the MCP C# SDK's invalidated two credentials that the previous release was using successfully. Both notion and textforge were connected minutes before the swap and reported "requires OAuth authorization" minutes after. Notion recovered by reauthorizing; textforge could not, because re-registration is impossible against it. Registration returns to Netclaw. ClientOAuthProvider hard-codes token_endpoint_auth_method: "client_secret_post" in its RFC 7591 request and only reads the server's advertised token_endpoint_auth_methods_supported afterwards, for the token request. 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 May and unfixed in 1.4.1, every 2.0 prerelease, and main; PR netclaw-dev#1615 covers only the token request. Netclaw's own registration sent "none" and worked, as does the TypeScript SDK, which negotiates from the advertised list. McpOAuthClientRegistrar registers with the same method the SDK will later select, 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, restoring the message removed with McpOAuthService. Legacy credentials now migrate instead of failing closed: - 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 this because no fixture set ExpiresAt. - McpServerUrl is retained rather than nulled, so migration can be repeated 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 no longer throws from a singleton constructor, where one unreadable secrets leaf took down daemon startup. RejectedDynamicClientId is replaced by discarding the rejected identity. A persisted marker is a latch: it survives restart and, against a server that cannot complete registration, makes every later attempt fail identically. Discarding keeps the tokens, needs no extra field, and self-heals. Deferred to their own changes, with the deletions recorded in the proposal: - The invocation lease and drain layer. A replaced or shutting-down client is disposed once its replacement publishes, so an in-flight call is cancelled rather than drained. The pre-existing behavior was the same, so this is a non-improvement rather than a regression. - Migrating the remaining secrets.json callers and transactional rollback for ProviderRenamer and BootstrapDeviceSeeder. SecretsFileWriter.Update stays, with an in-process lock whose key resolves a symlinked config directory. The two TUI save paths keep their fix because they were overwriting daemon-written McpOAuthTokens. MCP production code is 2488 lines against 1370 before the branch. The increase is the diagnostics taxonomy for netclaw-dev#1475, durable client identity, resource binding with migration, and candidate-then-publish. Net-negative production LOC was predicted in the proposal and not achieved; the proposal now records the measured figure. Verified live: a credential stored 2026-05-13 reconnected with 23 tools after a binary swap, with no reauthorization and with the temporary OAuthClientId override removed. 6068 tests pass, slopwatch and header checks are clean. Refs netclaw-dev#1475, netclaw-dev#1696, netclaw-dev#297
LegacyResourceFromADifferentAudienceFailsClosed already covers https://mcp.example.com/tools as an InlineData case, with the same assertions MismatchedLegacyResourceFailsClosedWithoutChangingDisk made.
Comment on lines
+258
to
+269
| /// <summary> | ||
| /// Serializes read-modify-write against one secrets file within this process. The daemon | ||
| /// is the only writer at runtime, and the CLI writes while it is stopped, so a | ||
| /// cross-process lock buys nothing that this does not already cover. | ||
| /// </summary> | ||
| private static IDisposable AcquireSecretsLock(string secretsPath, CancellationToken cancellationToken) | ||
| { | ||
| var gate = Gates.GetOrAdd(GateKey(secretsPath), static _ => new SemaphoreSlim(1, 1)); | ||
| if (!gate.Wait(SecretsLockTimeout, cancellationToken)) | ||
| throw new TimeoutException($"Timed out waiting to update secrets file '{secretsPath}'."); | ||
| return new SecretsLock(gate); | ||
| } |
Copilot flagged that the change claimed cross-process serialization of secrets.json while the implementation locks within one process. It was right, and the drift was wider than the one file. The comment on AcquireSecretsLock justified the in-process gate with "the CLI writes while it is stopped." Nothing enforces that. netclaw secrets set and netclaw provider add run against a live daemon and still write the file without taking the gate, so a CLI write can lose against a concurrent token refresh. That hazard is unchanged from before this branch, where every caller performed an unlocked read-modify-write, but the comment read as though it were covered. Corrected to match the code: - transactional-secrets spec no longer requires a cross-process lock or that every caller mutate inside the transaction, and its cross-process scenario is replaced by the symlinked-path case the tests actually cover. The remaining hazard is stated rather than implied away. - design goal and D4 amendment drop the cross-process wording and record why the lock and the caller migration are deferred together instead of half-done. - tasks 7.1 and 7.3 describe the in-process lock; 7.2 is reopened as partially delivered, naming the callers still doing unlocked writes. - tasks 1.1, 2.4, 2.5, 3.3 and 4.7 drop invocation-lease and drain language for what shipped; 4.6 and 4.8 are reopened as deferred with the drain work.
There was a problem hiding this comment.
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
Comment on lines
+294
to
+297
| private sealed class SecretsLock(SemaphoreSlim gate) : IDisposable | ||
| { | ||
| public void Dispose() => gate.Release(); | ||
| } |
Comment on lines
+321
to
+324
| var replacement = CreateReplacement(tokens, cache.Credentials, cache.Identity, cache.CanonicalResource); | ||
| if (cache.Published || !cache.ExplicitAuthorization) | ||
| if (cache.Published || !cache.ExplicitAuthorization) | ||
| { |
There was a problem hiding this comment.
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.
CommitFailureLeavesActiveStateUntouched forces a write failure by putting a directory where secrets.json belongs. Unix reports that as IOException; Windows reports UnauthorizedAccessException, which derives from SystemException rather than IOException, so Test-windows-latest failed on an assertion the other two platforms passed. Assert on either file-access failure and report the actual type when neither matches. The behavior under test -- the replace fails, active credentials are untouched, and the candidate stays unpublished -- is unchanged.
There was a problem hiding this comment.
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
ifstatements here (if (cache.Published || !cache.ExplicitAuthorization)repeated twice) make the control flow harder to read and look accidental. Collapsing to a singleifkeeps 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
Comment on lines
+20
to
+23
| /// <summary> | ||
| /// RFC 7591 dynamic client registration, owned by Netclaw rather than the MCP SDK. | ||
| /// | ||
| /// The SDK hard-codes <c>token_endpoint_auth_method: "client_secret_post"</c> in its |
Aaronontheweb
added
security
There was a problem hiding this comment.
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.
Stripping the RejectedDynamicClientId carry-forward removed the assignment but
left its if header above the identical condition that guards the persist block:
if (cache.Published || !cache.ExplicitAuthorization)
if (cache.Published || !cache.ExplicitAuthorization)
{
Evaluating the same condition twice is harmless, which is why the compiler,
slopwatch, the test suite and three CI platforms all passed over it. It still
misrepresents the persistence rules to anyone reading them.
Comment on lines
+94
to
+98
| var providerDetail = new HttpRequestException( | ||
| $"Client registration POST to {registrationEndpoint} returned " + | ||
| $"{(int)response.StatusCode} {response.StatusCode}: {body}", | ||
| inner: null, | ||
| response.StatusCode); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Swapping Netclaw's OAuth implementation for the MCP C# SDK's invalidated two credentials the previous release had been using successfully. Both
notionandtextforgewere connected minutes before a binary swap and reportedrequires OAuth authorizationminutes after. Notion recovered by reauthorizing; textforge could not, because re-registration is impossible against it.McpOAuthService,McpTokenCacheAdapter— 752 lines) is deleted, along with themcp-oauth-metadata.jsonruntime dependency.McpOAuthClientRegistrar. This is a deliberate exception to SDK ownership — see below.DaemonConfig.Port, and staticAuthorizationheader precedence.Why registration is not delegated to the SDK
ClientOAuthProvider.PerformDynamicClientRegistrationAsynchard-codestoken_endpoint_auth_method: "client_secret_post"and never consults the authorization server's advertisedtoken_endpoint_auth_methods_supported. A server accepting public clients only — TextForge advertises["none"]— rejects that with400 invalid_client_metadata, and 1.4.1 exposes no hook to change it:DynamicClientRegistrationOptionshas four properties and itsResponseDelegatefires 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 addsClientOAuthOptions.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 seedsClientOAuthOptions.ClientIdso the SDK's registration path never runs. A missing or failingregistration_endpointnamesOAuthClientIdas the remedy.Server-side tracking: petabridge/llm-email-gateway#784.
Upgrade-path fixes
Three defects made the migration cosmetic:
ObtainedAtis stamped on migration. Absent from pre-existing records, it deserialized to0001-01-01, andExpiresAt - ObtainedAtsaturated theintconversion toint.MaxValue, so the SDK treated every migrated credential as permanently expired. No test caught it because no fixture setExpiresAt.McpServerUrlis retained rather than nulled, so migration can repeat if an endpoint is corrected later and a rollback keeps its binding.LoadDurableStatealso no longer throws from a singleton constructor, where one unreadable secrets leaf took down daemon startup.RejectedDynamicClientIdis 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:SecretsFileWriter.Updatelocks within one process, and its key resolves a symlinked config directory.SecretsCommand,PairCommand,ProviderCommand,ProviderManagerViewModel,ExposureModeStepViewModel,ProviderCredentialWriterandOAuthTokenPersistencestill do unlocked read-modify-write, as they did before this change — so anetclaw secrets setissued 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-writtenMcpOAuthTokens.Size
MCP production code is 2488 lines against 1370 before this branch (
McpClientManager1256 + credential store 604 + flow broker 386 + registrar 242, vs manager 618 +McpOAuthService663 +McpTokenCacheAdapter89). 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.SecretsLockProbeproject, and transactional rollback forProviderRenamerandBootstrapDeviceSeeder.Verification
Netclaw.Daemon.Tests: 907 passed, 0 failed.OAuthClientIdoverride removed. Daemon log recordsMigrated legacy MCP OAuth credentials for 'textforge' after exact resource match.Notes
ModelContextProtocol.Coreremains pinned to 1.4.1.McpOAuthServerMetadatatype andNetclawPaths.McpOAuthMetadataPathproperty are removed; CLI and HTTP contracts are unchanged.Fixes #1475