Add MCP - #689
Conversation
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a repository-hosted Copilot plugin + MCP server/CLI integration for QDK Chemistry, including workspace binding enforcement, plugin install/update/rebind automation, and extensive tests/docs. It also updates some core chemistry defaults/serialization behaviors to better support MCP-driven workflows.
Changes:
- Introduce an MCP server entry point (
qdkchemmcp), workspace binding middleware, and a CLI plugin installer workflow for deploying Copilot agents/skills + MCP configs into a workspace. - Add Copilot plugin assets (agents, skills, marketplace metadata) and documentation describing agent-driven workflows.
- Update nuclear-derivative defaults (analytic gradients + Hessian support) and Orbitals/HDF5 serialization to preserve empty active-space partitions.
Reviewed changes
Copilot reviewed 60 out of 63 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents Copilot plugin installation and workspace binding expectations. |
| python/tests/ui/test_workspace_binding.py | Tests workspace binding and middleware enforcement. |
| python/tests/ui/test_plugin_installer.py | Tests plugin install/update/rebind flows and workspace deployment. |
| python/tests/ui/test_mcp_tools.py | Validates MCP tools behavior, remote execution timing, conversions, cataloging. |
| python/tests/ui/test_mcp_resource_estimator.py | Tests QRE-backed MCP resource estimation tool shape/behavior. |
| python/tests/ui/test_mcp_catalog.py | Tests MCP tool description compaction and catalog invariants. |
| python/tests/ui/test_agent_plugin.py | Validates plugin file completeness and MCP env constraints. |
| python/tests/ui/init.py | Declares ui test package. |
| python/tests/test_nuclear_derivative.py | Updates expected default derivative implementation + geometry optimizer registration. |
| python/tests/test_circuit_executor_data.py | Adds content-hash error-path coverage for unsupported metadata. |
| python/tests/test_cache.py | Adds duplicate cache registration error coverage. |
| python/src/qdk_chemistry/ui/workspace.py | Implements workspace binding tool + middleware gate for MCP processes. |
| python/src/qdk_chemistry/ui/validation.py | Adds filename marker validation + project validation decorator. |
| python/src/qdk_chemistry/ui/plugin_installer.py | Implements Copilot plugin installation and venv-bound MCP command rebinding. |
| python/src/qdk_chemistry/ui/mcp.py | Adds MCP server CLI with compact tool descriptions and compatibility flags. |
| python/src/qdk_chemistry/ui/io.py | Adds generic file I/O helpers for JSON/HDF5 tool paths. |
| python/src/qdk_chemistry/ui/config.py | Adds MCP server configuration with scratch/projects/cache/jobs directories. |
| python/src/qdk_chemistry/ui/init.py | Lazy-loads UI submodules to reduce import-time side effects. |
| python/src/pybind11/algorithms/nuclear_derivative.cpp | Exposes derivative hash and updates QDK derivative docs/behavior. |
| python/src/pybind11/algorithms/factory_bindings.hpp | Adds helper templates for remote execution proxying and run hashing. |
| python/pyproject.toml | Adds MCP/argcomplete deps and new CLI scripts. |
| docs/source/user/agents.rst | New user docs for agent/MCP workflows. |
| docs/source/index.rst | Adds agents doc page to TOC. |
| cpp/tests/test_orbitals_comprehensive.cpp | Ensures empty active space with inactive orbitals remains a defined partition. |
| cpp/tests/test_nuclear_derivative.cpp | Updates derivative defaults + adds analytic Hessian behavior tests. |
| cpp/src/qdk/chemistry/data/orbitals.cpp | Treats inactive-space presence as “has_active_space” for partition semantics. |
| cpp/src/qdk/chemistry/data/hdf5_serialization.cpp | Writes/reads empty vectors safely for round-trip serialization. |
| cpp/src/qdk/chemistry/algorithms/qdk_nuclear_derivative.hpp | Adds QDK-specific derivative settings (finite-difference step for Hessian). |
| cpp/src/qdk/chemistry/algorithms/qdk_nuclear_derivative.cpp | Implements Hessian via finite differences of analytic gradients. |
| cpp/include/qdk/chemistry/algorithms/nuclear_derivative.hpp | Updates docs + makes qdk the default derivative implementation. |
| cpp/include/qdk/chemistry/algorithms/geometry_optimization.hpp | Defaults geometry optimization derivative calculator to qdk. |
| copilot-plugins/qdk-chemistry/skills/remote-execution/SKILL.md | Adds remote-execution skill content. |
| copilot-plugins/qdk-chemistry/skills/qdk-chemistry-overview/SKILL.md | Adds overview skill content. |
| copilot-plugins/qdk-chemistry/skills/qdk-chemistry-overview/references/workflow-patterns.md | Adds workflow routing reference. |
| copilot-plugins/qdk-chemistry/skills/qdk-chemistry-overview/references/playbook-index.md | Adds playbook index. |
| copilot-plugins/qdk-chemistry/skills/qdk-chemistry-overview/references/how-qdk-chemistry-works.md | Adds factory/tooling conventions reference. |
| copilot-plugins/qdk-chemistry/skills/qdk-chemistry-mcp/SKILL.md | Adds MCP skill content and policies. |
| copilot-plugins/qdk-chemistry/skills/qdk-chemistry-mcp/references/things-that-go-wrong.md | Adds failure-modes reference. |
| copilot-plugins/qdk-chemistry/skills/qdk-chemistry-mcp/references/quantum-resource-compression.md | Adds resource-compression reference. |
| copilot-plugins/qdk-chemistry/skills/qdk-chemistry-mcp/references/qpe-and-state-prep.md | Adds QPE/state-prep reference. |
| copilot-plugins/qdk-chemistry/skills/qdk-chemistry-mcp/references/active-space-guide.md | Adds active-space selection reference. |
| copilot-plugins/qdk-chemistry/skills/qdk-chemistry-coding/SKILL.md | Adds Python SDK coding skill content. |
| copilot-plugins/qdk-chemistry/skills/qdk-chemistry-coding/references/python-sdk-reference.md | Adds Python SDK reference. |
| copilot-plugins/qdk-chemistry/skills/qdk-chemistry-coding/references/example-n2-stretched.md | Adds worked example reference. |
| copilot-plugins/qdk-chemistry/skills/qdk-chemistry-coding/references/example-benzene-state-prep.md | Adds worked example reference. |
| copilot-plugins/qdk-chemistry/plugin.json | Adds plugin manifest. |
| copilot-plugins/qdk-chemistry/agents/reviewer.agent.md | Adds reviewer agent definition. |
| copilot-plugins/qdk-chemistry/agents/researcher.agent.md | Adds researcher agent definition. |
| copilot-plugins/qdk-chemistry/agents/reporter.agent.md | Adds reporter agent definition. |
| copilot-plugins/qdk-chemistry/agents/quantum-agent.agent.md | Adds orchestrator agent definition. |
| copilot-plugins/qdk-chemistry/agents/chemist.agent.md | Adds chemist agent definition. |
| copilot-plugins/qdk-chemistry/.mcp.json | Adds plugin MCP server config (requires runtime workspace binding). |
| .pre-commit-config.yaml | Adjusts markdown lint exclusions and interrogate threshold. |
| .pipelines/requirements.txt | Updates pipeline lockfile to include MCP stack and deps. |
| .pipelines/requirements.in | Adds MCP dependency to pipeline inputs. |
| .gitignore | Minor formatting change. |
| .github/plugin/marketplace.json | Adds Copilot plugin marketplace manifest. |
| .devcontainer/devcontainer.json | Forwards MCP port 8081 for devcontainer usage. |
Suppressed comments (2)
docs/source/user/agents.rst:124
- The manual-start examples use
qdk_chem_mcp, but the script added in this PR isqdkchemmcp(and the CLI help text elsewhere usesqc). This mismatch will confuse users and makes the docs copy/paste-fail unless an alias exists.
.github/plugin/marketplace.json:15 - The plugin entry version is also set to
2.0.0here, which conflicts with the plugin manifest version (2.1.0). Align the entry version so marketplace clients report the correct installed version.
"name": "qdk-chemistry",
"description": "Run QDK Chemistry workflows with specialized agents, skills, and MCP tools.",
"version": "2.0.0",
"source": "copilot-plugins/qdk-chemistry"
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 61 out of 64 changed files in this pull request and generated 7 comments.
Suppressed comments (5)
Previously missed (3) — in code that hasn't changed since the last review.
.pre-commit-config.yaml:52
- Setting interrogate’s
--fail-underto 0 effectively disables docstring coverage enforcement while still running the hook. If the intent is to keep doc coverage meaningful, restore a non-trivial threshold (e.g., the previous 80) or remove the hook entirely.
args: [--fail-under=0, --verbose, --ignore-init-method]
python/src/qdk_chemistry/ui/io.py:23
- Path stripping uses
rsplit("/", ...), which is not portable on Windows paths ("\"). Useos.path.basenameto reliably drop directory components across platforms.
This issue also appears in the following locations of the same file:
- line 63
- line 86
python/src/qdk_chemistry/ui/validation.py:118
validate_projectis a decorator and always returns a callable wrapper, but its type signature/docstring currently suggest it may return a JSON string. The wrapper actually returns a plain error string on validation failure. Updating the annotation and docstring will prevent confusing API/docs for tool authors.
python/src/qdk_chemistry/ui/io.py:86
- Path stripping uses
rsplit("/", ...), which is not portable on Windows paths ("\"). Useos.path.basenameto reliably drop directory components across platforms.
python/src/qdk_chemistry/ui/io.py:63 - Path stripping uses
rsplit("/", ...), which is not portable on Windows paths ("\"). Useos.path.basenameto reliably drop directory components across platforms.
There was a problem hiding this comment.
🔵 Needs a closer look
The Sphinx config now imports project modules at conf-load time, which can hard-fail docs builds before autodoc mocking applies.
Review details
Suppressed comments (1)
docs/source/conf.py:361
normalize_public_export_modules()is executed at conf.py import time, before Sphinx has appliedautodoc_mock_imports. Ifqdk_chemistry(or its compiled extensions) isn’t importable in the docs build environment, this will raise during config load and abort the whole docs build. Consider guarding this call so docs builds that rely on mocked/absent native modules can still proceed (even if the module-name normalization is skipped).
# Sphinx 9 resolves type annotations while initializing extensions, before the
# ``builder-inited`` event. Normalize exports as part of loading this config so
# autodoc and sphinx-autodoc-typehints both see the same public object names.
normalize_public_export_modules()
- Files reviewed: 83/86 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The worker currently trusts job_cache_key from the manifest (falling back to run_hash), which can defeat the new owner-based cache scoping when job_cache_key is missing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 82/85 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The docs build configuration and UI validation docs contain correctness/robustness issues that should be addressed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
python/src/qdk_chemistry/ui/validation.py:226
validate_projectdocstring says the wrapper returns a JSON string on failure, but the implementation returns a plain text message (not JSON). This mismatch can confuse callers and documentation readers; either update the docstring or return structured JSON.
docs/source/conf.py:361normalize_public_export_modules()is executed at import-time inconf.pyand unconditionally importsqdk_chemistry.algorithms/qdk_chemistry.data. If those imports fail in a docs-only environment (e.g., native extensions not built), the entire docs build will crash. Consider guarding this with an ImportError handler so docs can still build (possibly with reduced API output).
# Sphinx 9 resolves type annotations while initializing extensions, before the
# ``builder-inited`` event. Normalize exports as part of loading this config so
# autodoc and sphinx-autodoc-typehints both see the same public object names.
normalize_public_export_modules()
- Files reviewed: 82/85 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The new UI config initializes and creates filesystem directories at import time even when workspace binding is required, which can write outside the bound workspace before bind_workspace runs.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
python/src/qdk_chemistry/ui/config.py:77
- QDKMCPConfig creates scratch/projects/cache/jobs directories during init (import time). For plugin-launched MCP processes that require bind_workspace first (QDK_REQUIRE_WORKSPACE_BINDING=1), this performs filesystem writes before the workspace is bound, undermining the intended isolation.
- Files reviewed: 82/85 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
The PR introduces a broad new MCP/plugin surface area plus remote-execution/registry behavior changes, and it includes at least one concrete robustness bug that should be fixed before approval.
Review details
- Files reviewed: 82/85 changed files
- Comments generated: 1
- Review effort level: Lite
| for source_file in sorted(path for path in source.rglob("*") if path.is_file() or path.is_symlink()): | ||
| resolved_source_file = source_file.resolve() | ||
| if not resolved_source_file.is_relative_to(source_root): | ||
| raise PluginInstallError( | ||
| f"plugin component file {source_file} resolves outside {source_root} " | ||
| "(symlink traversal is not allowed)" | ||
| ) | ||
| target = destination / source_file.relative_to(source) | ||
| target.parent.mkdir(parents=True, exist_ok=True) | ||
| shutil.copy2(resolved_source_file, target) | ||
| copied.append(str(target)) |
No description provided.