Repository navigation
fix(mcp): converge managed adapter credential revisions - #10307
Merged
Merged
Conversation
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> # Conflicts: # agents/hermes/mcp-config-transaction.py # src/lib/actions/sandbox/mcp-bridge-adapter-hermes.test.ts # src/lib/actions/sandbox/mcp-bridge-adapter-registration.test.ts # src/lib/actions/sandbox/mcp-bridge-adapter-status.ts # test/agents/hermes/hermes-mcp-credential-revision.test.ts # test/e2e/support/mcp-bridge-hermes-lifecycle.test.ts
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> # Conflicts: # agents/hermes/mcp-config-transaction.py # src/lib/actions/sandbox/mcp-bridge-adapter-hermes.test.ts # src/lib/actions/sandbox/mcp-bridge-adapter-hermes.ts # test/agents/hermes/hermes-mcp-credential-revision.test.ts # test/e2e/support/mcp-bridge-hermes-lifecycle.test.ts
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Remove unrelated branch drift and keep only MCP runtime, tests, and corresponding docs. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
ericksoa
changed the base branch from
codex/fix-hermes-mcp-runtime-contract-10155
to
main
August 26, 2026 04:03
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-10307.docs.buildwithfern.com/nemoclaw |
Keep the PR diff limited to the MCP regression fix and its direct coverage. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Exclude unrelated E2E changes that landed on main from the MCP regression diff. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
rsliter
reviewed
Aug 26, 2026
rsliter
left a comment
rsliter
left a comment
Collaborator
There was a problem hiding this comment.
Reviewing exact commit 8068bd5. Two merge blockers remain. First, the effective MCP diff still includes unrelated PR Review Advisor workflow, tool, test, and documentation rewrites that are not described by this PR and must be removed from the candidate. Second, this branch carries material Charan Jagwani work from #10270, while #10270 does not contain Charan own PR-body Signed-off-by declaration. The superseded-PR attribution policy forbids selecting or merging the transferred replacement until Charan adds that declaration personally. Current CI and changes-requested gates remain separate blockers.
17 of 27 tasks
Collaborator
|
Fresh CI on |
Collaborator
|
Attribution blocker cleared: I added my personal For the remaining scope blocker, please reset the unrelated PR Review Advisor changes in #10307 to current |
Merge current main because GitHub reported the PR unmergeable; retain only the tested MCP delta. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
ericksoa
marked this pull request as draft
August 26, 2026 05:44
Contributor
|
PR review advisory complete for commit |
cjagwani
marked this pull request as ready for review
August 26, 2026 06:22
6 tasks
ericksoa
added a commit
that referenced
this pull request
Aug 26, 2026
## Summary Revert #10307 and restore `main` to the exact tree that existed immediately before merge commit `896760b7cd407606050ce609dbcce429b06cc3f1`. Requested as an immediate maintainer rollback. ## Verification - `git diff --quiet fded75f -- .` passed after applying the revert. - Commit and push hooks passed after rebuilding CLI artifacts. --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
ericksoa
added a commit
that referenced
this pull request
Aug 27, 2026
## Summary Restore the exact code state from merged PR #10307 after its requested rollback. The replacement branch tree is byte-for-byte identical to original merge commit `896760b7cd407606050ce609dbcce429b06cc3f1`. ## Related Issue Fixes #10300 Related: #10298 ## Verification - Exact replacement head: `b5286eb8e` - Exact tree match: `fdb4b3cdbbaf5dba67c016d259cedac9da83977f` - Commit and push hooks passed. - Prior exact-head CI: https://github.com/NVIDIA/NemoClaw/actions/runs/32934865387 - Prior managed-image qualification: https://github.com/NVIDIA/NemoClaw/actions/runs/32934865411 - Prior focused E2E: https://github.com/NVIDIA/NemoClaw/actions/runs/32937684548 This PR is intentionally opened as a draft. --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Reliability** * Improved MCP credential synchronization during registration, restart, and recovery. * Added safeguards for unavailable, unstable, or outdated credential revisions. * Improved recovery after lifecycle failures, timeouts, and interrupted runtime operations. * **Security** * Strengthened credential authorization validation and prevented unversioned credential placeholders. * Restricted runtime state access to the required read-only and startup paths. * **Bug Fixes** * Improved Hermes startup ordering and runtime configuration validation. * Added retry handling for qualifying OpenClaw onboarding failures. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
wscurran
added
area: e2e
rsliter
pushed a commit
that referenced
this pull request
Nov 6, 2026
<!-- markdownlint-disable MD041 --> ## Outcome Managed MCP add now labels its two policy phases and reports credential synchronization and agent registration. The command explains that synchronization is required even with `--no-probe`, so the wait no longer appears to be an unexplained duplicate policy application. ## Reason The macOS reproduction in #12672 completed successfully after about 70 seconds, beyond its automation timeout. OpenShell requires the route policy before provider attachment and the credential binding afterward. Removing either phase or its readiness checks would change credential safety. ### Related issues Refs #12672. This addresses the progress messages. The external QA harness timeout and first-add latency on macOS still need validation, so this PR does not close the issue. Prior work: #9721 added post-binding credential republishing; #10307 required stable credential revisions before adapter registration. This change preserves those safeguards. ## Changes Add progress messages in the existing MCP add lifecycle before opening the route, binding the credential, waiting for synchronization, and registering the server. An orchestration regression checks that each message is visible before the corresponding operation and excludes the host credential value. ## Verification - The new regression failed before the production change because all five observed operation boundaries had no progress message. - `npx --no-install vitest run --project cli src/lib/actions/sandbox/mcp-bridge-hermes-add-finality.test.ts src/lib/actions/sandbox/mcp-bridge-provider.test.ts src/lib/actions/sandbox/mcp-bridge-status-resolution.test.ts src/lib/actions/sandbox/mcp-bridge-restart-recovery.test.ts --coverage=false`: 106 tests passed, including the subprocess `--no-probe` dispatch cases. - `npx --no-install oxlint --deny-warnings src/lib/actions/sandbox/mcp-bridge-add-restart.ts src/lib/actions/sandbox/mcp-bridge-hermes-add-finality.test.ts` and `git diff --check`: passed. - Normal pre-commit, commit-msg, and pre-push hooks passed, including publication validation and CLI typechecking. - Live macOS first-add validation was not run. The contributor doctor rejected Docker's 7.7 GiB memory allocation. The lifecycle test uses the shared add path through a Hermes fixture; external services are mocked. - The two-file diff contains no secrets, API keys, or credentials. No live E2E assertion or timeout changes. ## Review notes Decision: Accept the progress-message change. Reason: the reported duplicate application is a required two-phase policy sequence. Placement: the existing MCP add orchestrator. Accountable maintainer: @deepujain. Validation plan: the failing-then-passing progress regression, provider and recovery tests, `--no-probe` dispatch coverage, publication hooks, and independent CI and review. Independent pre-publication review of latest PR commit `812fe11f0039ac073151ec6aa9273779c35ef42d` found no blocking correctness, security, scope, or writing issues. Hosted CI and automated reviews remain pending. DGX Station hardware evidence is not applicable. --- Signed-off-by: Deepak Jain <deepujain@gmail.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **New Features** * Adding an MCP server now displays progress updates as setup advances, including policy configuration, credential synchronization, and server registration. Credential synchronization progress is shown even when probing is skipped. * **Bug Fixes** * Improved admission checks to reliably read command output and confirm that no native sandbox commands run during the check. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Deepak Jain <deepujain@gmail.com>
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
Fix the post-#10173 MCP revision regressions in this PR. Managed adapters now register against a stable opaque credential revision observed through fresh OpenShell execs. A delayed provider projection can no longer leave committed adapter configuration stale, and teardown never derives credential authority from an unrelated provider resource version.
Related Issue
Fixes #10300
Related: #10298
Changes
Type of Change
Quality Gates
Verification
Exact local head:
44088b59cnpm run build:clipassednpm run checks:repositorypassed: 1,821 files, 5,760 edges, 0 cyclesnpx prek run --all-filespassedSigned-off-by: Aaron Erickson aerickson@nvidia.com