Modernize MCP server and add MCP Apps support#128
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2f15220a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
sidemantic/apps/__init__.py
Outdated
| Complete HTML string with the spec injected. | ||
| """ | ||
| template = _get_widget_template() | ||
| return template.replace("{{VEGA_SPEC}}", json.dumps(vega_spec)) |
There was a problem hiding this comment.
build_chart_html injects json.dumps(vega_spec) directly into a <script type="application/json"> block, but JSON encoding does not escape </script>. Because chart metadata (for example the user-provided title) flows into vega_spec, an input containing </script><script>... can break out of the JSON script tag and execute arbitrary JavaScript in MCP App hosts that render this HTML. Please serialize with HTML-safe escaping (at minimum replace < with \u003c) before template injection.
Useful? React with 👍 / 👎.
Uses mcp-ui-server to return UIResource alongside chart data when --apps flag is enabled. The chart widget is a self-contained HTML page with Vega-Lite spec embedded, rendered via vega-embed from CDN. - New sidemantic/apps/ module with chart widget HTML template - create_chart tool declares meta.ui.resourceUri for MCP Apps discovery - When apps enabled, create_chart returns [data_dict, UIResource] - CLI: --apps, --http, --port flags on mcp-serve - --apps implies HTTP transport (streamable-http) - mcp-ui-server pinned to git (PyPI v1.0.0 has wrong MIME type) - New [apps] optional dependency group
Replace < with \u003c when embedding JSON in the <script> block so user-provided strings (e.g., chart titles) containing </script> cannot break out of the JSON context and inject arbitrary JavaScript.
nicosuave
force-pushed
the
modernize-mcp
branch
from
d2f1522 to
32b8828
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32b8828a1a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if _apps_enabled: | ||
| from sidemantic.apps import create_chart_resource | ||
|
|
||
| return [result, create_chart_resource(vega_spec)] |
There was a problem hiding this comment.
When --apps is enabled this branch returns a list ([result, resource]) even though create_chart is declared to return dict[str, Any]; FastMCP uses return annotations to derive tool output schemas, so this type mismatch can cause schema/structured-output validation failures or missing structured content in MCP clients specifically on the apps path. This makes the new apps-enabled chart workflow unreliable even though direct unit calls still pass.
Useful? React with 👍 / 👎.
pyproject.toml
Outdated
| ] | ||
| apps = [ | ||
| "mcp[cli]>=1.25.0,<2", | ||
| "mcp-ui-server @ git+https://github.com/MCP-UI-Org/mcp-ui.git#subdirectory=sdks/python/server", |
There was a problem hiding this comment.
Adding mcp-ui-server @ git+https://... to project.optional-dependencies places a direct VCS reference into package metadata for every release artifact, which causes PyPI upload rejection (400 Can't have direct dependency) even when the dependency is optional. Because this project publishes to PyPI, this change can block the release pipeline until the extra uses a resolvable index package/version instead of a git URL.
Useful? React with 👍 / 👎.
- Replace git VCS dep with PyPI mcp-ui-server>=1.0.0 (git refs block PyPI uploads) - Remove allow-direct-references hatch config - Drop return type annotation from create_chart (returns dict or list depending on apps mode, FastMCP uses annotation for schema derivation) - Relax MIME type test assertion for PyPI v1.0.0 compatibility
nicosuave
force-pushed
the
modernize-mcp
branch
from
8b39570 to
e36ba62
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b39570acf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
|
|
||
| @mcp.tool() | ||
| @mcp.tool(meta={"ui": {"resourceUri": "ui://sidemantic/chart"}}) |
There was a problem hiding this comment.
create_chart now advertises _meta.ui.resourceUri as ui://sidemantic/chart, but this module never registers a matching @mcp.resource("ui://sidemantic/chart") handler (the only registered resource is semantic://catalog). In MCP Apps flows, clients resolve resourceUri via resources/read, so this URI lookup fails and the chart widget cannot be loaded even when --apps is enabled.
Useful? React with 👍 / 👎.
1 participant
Summary
list_models(subsumed byget_semantic_graph),list_segments(inget_models/get_semantic_graph),compile_query(merged intorun_queryasdry_runflag)_validate_filtersemicolon false positive using sqlglot multi-statement parsinginit_sqlsupport tomcp-serve,offsettocreate_chartmcpdependency to>=1.25.0,<2create_chartreturns interactive Vega-Lite widget viamcp-ui-serverwhen--appsflag is enabled--apps,--http,--portonmcp-serve