Skip to content

docs(tools/mcp): scope policy — environment tooling, not workflow policy - #1712

Merged
ChenhanYu merged 1 commit into
mainfrom
chenhany/mcp-scope-policy
Jun 14, 2026
Merged

ChenhanYu merged 1 commit into
mainfrom
chenhany/mcp-scope-policy

Conversation

@ChenhanYu

@ChenhanYu ChenhanYu commented Jun 14, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Adds tools/mcp/SCOPE.md documenting the design boundary for the MCP server family (modelopt-mcp + nmm-sandbox-mcp + pensieve-intern-mcp):

  • In scope: universal verb-shaped operations on the cluster, launcher, agent engine — environment tooling that every workflow can rely on as pre-knowledge.
  • Out of scope: workflow-specific logic ("run an EAGLE3 cell", "publish a specdec release"). That belongs in SPEC text + agent reasoning, composed out of these primitives.

Why

Surfaced during the OMNIML-5123 follow-up discussion. The 14 tools currently in scope across the three servers are deliberately a small, closed set. Letting workflow-specific tools sneak in would sprawl the catalog, force per-workflow opt-in, and break the "MCP-as-pre-knowledge" promise that makes the catalog useful as a stable baseline for pensieve-intern's agents.

SCOPE.md documents:

  • The test: would this tool be useful across any workflow that uses the same environment?
  • A side-by-side table of in-scope vs out-of-scope tool shapes
  • Symptoms that a tool is misclassified
  • Why the line matters (interface-drift blast radius collapses, agent tool catalog stays learnable)
  • Four practical questions to ask before adding a tool
  • A reference list of the 14 currently-in-scope tools

Anchor

OMNIML-5123 (Epic).

Docs-only, no code changes.

Summary by CodeRabbit

  • Documentation
    • Added scope documentation clarifying that MCP environment tooling covers universal verb-shaped operations (job submission, verification, artifact reading) while workflow-specific policies belong elsewhere.
    • Documented inclusion criteria and tool inventory guidance for MCP servers.

… policy

The 14 tools across modelopt-mcp, nmm-sandbox-mcp, and pensieve-intern-mcp
are deliberately verb-shaped primitives on the cluster, launcher, and
agent engine. They are universal across workflows — environment tooling,
not workflow policy.

Adding workflow-specific tools (e.g., bench_eagle3_against_baseline,
create_specdec_release_pr) would sprawl the catalog, force per-workflow
opt-in, and break the 'pre-knowledge' promise that makes MCP-as-baseline
valuable for pensieve-intern's intern-agents.

SCOPE.md documents the test, the failure modes to watch for, the
side-by-side in-scope/out-of-scope examples, and practical guidance
for evaluating new tools. The policy applies to all three servers in
the MCP family. Anchor: OMNIML-5123 follow-up discussion (2026-06-13).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Chenhan Yu <chenhany@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jun 14, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a new tools/mcp/SCOPE.md file (102 lines) that documents the boundary between reusable environment primitives and workflow-specific policy for MCP servers, including inclusion criteria, examples, misclassification symptoms, and the current tool inventory. tools/mcp/README.md gains a four-line section referencing SCOPE.md.

Changes

MCP Scope Policy Documentation

Layer / File(s) Summary
SCOPE.md policy document and README cross-reference
tools/mcp/SCOPE.md, tools/mcp/README.md
SCOPE.md is introduced with environment-tooling vs. workflow-policy definitions, tool-inclusion rules, examples, misclassification symptoms, current in-scope tool inventory by MCP server, and Phase 3 expansion notes. README.md adds a short section linking to SCOPE.md and clarifying the MCP family boundary.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Security Anti-Patterns ❌ Error Multiple # nosec comments found in bridge.py (8 instances) without explicit justification in PR description or @NVIDIA/modelopt-setup-codeowners approval, violating security rule #5. Add explicit justification for each # nosec comment in the PR description and obtain @NVIDIA/modelopt-setup-codeowners approval, or refactor code to avoid security pattern suppressions.
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: adding a scope policy document that establishes MCP servers as environment tooling rather than workflow-specific policy.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chenhany/mcp-scope-policy

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Jun 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.12%. Comparing base (9f37fe1) to head (22c6610).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1712   +/-   ##
=======================================
  Coverage   77.12%   77.12%           
=======================================
  Files         511      511           
  Lines       56236    56236           
=======================================
  Hits        43374    43374           
  Misses      12862    12862           
Flag Coverage Δ
unit 54.39% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ChenhanYu

Copy link
Copy Markdown
Collaborator Author

/claude review

@ChenhanYu ChenhanYu self-assigned this Jun 14, 2026
@ChenhanYu
ChenhanYu enabled auto-merge (squash) June 14, 2026 04:14

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude review passed — no blocking issues found. LGTM

Docs-only PR adding tools/mcp/SCOPE.md and a short cross-reference in tools/mcp/README.md. Verified:

  • Tool inventory in SCOPE.md (9 modelopt-mcp tools) matches the README tool table and the "9 tool definitions" referenced in modelopt_mcp/server.py. No drift.
  • README→SCOPE relative link resolves.
  • No code, schema, API, algorithm, mode/state, export, or compatibility surface touched. Coverage unchanged at 77.12%.

The pre-merge security warning CodeRabbit posted (# nosec in bridge.py) refers to pre-existing code on main, not anything introduced by this PR's diff. No findings from this review.

@ChenhanYu
ChenhanYu merged commit 463229a into main Jun 14, 2026
43 checks passed
@ChenhanYu
ChenhanYu deleted the chenhany/mcp-scope-policy branch June 14, 2026 04:15
@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-06-14 04:15 UTC

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.

1 participant