Skip to content

feat(mecated): add native MCP custody reset - #1991

Merged
jhrozek merged 1 commit into
mainfrom
native-mcp-custody
Oct 15, 2026
Merged

jhrozek merged 1 commit into
mainfrom
native-mcp-custody

Conversation

@jhrozek

@jhrozek jhrozek commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Provide an attended recovery path for obsolete, missing, or mismatched native MCP credential state. mecated mcp reset-local-credentials NAME [--file PATH] confirms every affected native profile sharing the selected root, keeps their settings, and retires local sign-in credentials through a durable reset marker so interrupted resets can resume. It preserves legacy key_env custody, upstream OAuth registrations, and file keys used by another root. Explicit login or add creates fresh custody afterward.

Login errors for reset-recoverable custody identify the recovery command and retain the selected profile's settings source. Reset fails closed if its settings scope changes during confirmation or a shared file-key owner cannot be verified.

Fixes #1922

Development stage

  • Implementation — local MCP credential reset and recovery behavior

Acceptance plans are not used as implementation contracts under the current project process. This change adds no engine API or protobuf contract.

Changes

  • Add the attended mcp reset-local-credentials CLI command and list it in top-level and MCP help.
  • Make reset resumable across intent, artifact-removal, and namespace-retirement boundaries; validate the artifact before destructive changes.
  • Preserve shared file keys and revalidate settings, canonical root, and affected profiles after confirmation.
  • Initialize fresh native custody during explicit login after reset and show actionable recovery guidance for missing or mismatched ready artifacts.
  • Document the reset workflow and the profiles and credentials it affects.

Verification

  • Focused tests passed: go test ./internal/adapter/mcpcredential ./internal/cliconfig and go test ./cmd/mecated -run '^(TestMCP|TestDirectMCPOnboarding)'.
  • task site:build passed.
  • task lint reports G703 in the untouched environment/microvm/launch_ownership_darwin.go:754.
  • task docs reports 22 broken links and 0 broken anchors across the workspace.
  • task test did not finish within the 600-second timeout.
  • Manual OS-keyring qualification remains pending. Automated keyring tests use a fake; no native keychain/credential-service check was run.

Special notes for reviewers

The reset-local-credentials command name replaces the earlier implementation-oriented name. The reset never revokes upstream registrations. Please review the pending manual OS-keyring qualification before merge.

@JAORMX

JAORMX commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

I think the recovery path is worth having, but I wouldn't merge this yet. Right now the operator can follow the documented steps and still be stuck.

reset-custody removes the marker and wrapping key, then tells us to run mcp login. Login loads the configured profile through mcpcredential.Open, which rejects the now-missing marker; it never calls Resolve to create new local credentials (cmd/mecated/mcplifecycle.go:736-739, cmd/mecated/mcplogin.go:258-260, internal/cliconfig/mcpprofile.go:407-410). Can we exercise the actual reset -> login path with an existing profile before calling this done? The current test calls Resolve directly after reset, so it misses this.

There are a few destructive-boundary issues I'd want fixed first:

  • A pending marker proves which path a root tried to use, not that it owns the key at that path. Root B can publish a pending marker for a file key already used by root A; resetting B accepts the locator digest and deletes A's key (internal/adapter/mcpcredential/custody.go:243-260,933-967). A isn't in B's confirmation list. Please add the two-root case and refuse deletion when ownership is ambiguous.
  • We release the settings lock before asking for confirmation and don't recheck it afterward (cmd/mecated/mcplifecycle.go:715-736). Another profile can start sharing the root while we wait, or an ancestor symlink can change which root the saved path resolves to. In either case, the operator confirmed a different scope from the one we delete. Bind the confirmation to the canonical root and affected-profile set, and refuse to continue if either changed.
  • Reset records its intent and renames the native namespace before it validates the file artifact for deletion (internal/adapter/mcpcredential/custody.go:940-969,1150-1165). An unsafe artifact leaves custody partially reset, despite AC7.2 requiring refusal without deletion. A configured file key inside that namespace is another troublesome case: after the rename, deletion at the old path fails, and retrying can't finish. Please cover both cases before the first destructive step.
  • On resume, unlinkPrivateFile returns immediately for an absent key without syncing its directory (internal/adapter/mcpcredential/custody.go:1182-1197). If we crashed after unlink but before the directory sync, we can remove the marker and claim success without making that key deletion durable.

The UX also needs a pass. "Custody" makes sense in the implementation, but I don't know what reset-custody means as an operator command. I'd use something like mcp reset-local-credentials and say what it actually does: discard saved local MCP sign-in credentials for every listed profile sharing this storage location; keep their settings; sign in again for each one. The current help says it will "recreate" custody, which it doesn't (cmd/mecated/command.go:309). The prompt says "native credentials" (cmd/mecated/mcplifecycle.go:728), and the docs demonstrate logging in just one profile even though others may be affected (user-docs/features/mcp-oauth-and-credentials.md:126-150). Also, the actionable obsolete-marker and mismatch errors from custody.go:131-150 get replaced by a generic profile-store error in internal/cliconfig/mcpprofile.go:407-410. That's the moment the person needs the recovery command, and we don't show it to them. Please carry a safe, specific remedy through to the CLI. Listing the command in top-level help would help too.

On coverage: the interruption test manually constructs each supposed crash state with private helpers (internal/adapter/mcpcredential/custody_test.go:327-355), so it doesn't check the actual ordering of Reset. The CLI tests call the handler directly rather than the command dispatcher, and they don't assert that the confirmation displays the complete shared-root list (cmd/mecated/mcplifecycle_test.go:61-106). The keyring path is fake-only; a narrow adapter check or an explicit manual qualification would make that limitation clear. I did not run tests locally for this review.

Finally, this adds a destructive command and persisted resetting state to an already approved acceptance contract (docs/acceptance/direct-mcp-onboarding.md:132-144). The project process calls for an explicitly approved contract amendment before implementing that kind of drift (docs/development-process.md:90-100). The PR description still has the stage, baseline, conformance, and verification template fields unfilled. Can we sort out the contract and update the PR description before review is complete?

@jhrozek

jhrozek commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

@JAORMX Thanks for the detailed review. I pushed the fixes in 9bac3bd0c0741e782fb3bd0c67cb7de061e261ba and updated the PR description. The reset-to-login path now initializes new local custody and keeps actionable recovery guidance visible. Reset confirms the full affected-profile set, revalidates the settings source and canonical root after confirmation, refuses ambiguous cross-root file-key ownership, validates artifacts before destructive changes, and handles keys inside the native namespace. The durable marker now drives recovery through the actual reset ordering, including repeated resets and directory sync when a key is already absent. The command is now mcp reset-local-credentials; help and docs explain that settings remain and each affected profile must sign in again.

Automated keyring tests still use a fake. I did not run native OS keychain/credential-service qualification, and marked it pending in the PR description rather than claiming it passed. Focused tests and task site:build passed; CI is running. I also noted that acceptance plans are no longer treated as implementation contracts under the current project process.

Add `mecated mcp reset-local-credentials` to recover obsolete or mismatched native MCP credential state without removing profile settings or revoking upstream OAuth registrations. The attended command confirms the affected profiles and records reset progress so interrupted resets can resume.

Protect shared file keys and revalidate the selected settings scope before deletion. Create fresh custody through the explicit login or add flows.

Co-authored-by: Mecatl <noreply@mecatl.dev>
@jhrozek
jhrozek merged commit 090ebfb into main Oct 15, 2026
39 checks passed
@jhrozek
jhrozek deleted the native-mcp-custody branch October 15, 2026 14:51

This branch was successfully deployed

1 active deployment
Preview — 371134d9 Deployed Oct 15, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Native MCP custody has no supported reset after marker incompatibility

2 participants