feat: add TypeScript/Skybridge emitter (--target skybridge) - #10
Merged
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughAdds a TypeScript/Skybridge emit phase and emitter to the mcp-anything pipeline and a complete Spotify playlist-generator example: TypeScript MCP server, three React Skybridge views, discovery/telemetry, build/runtime configs, Dockerfiles, evaluation fixtures, tests, and conformance/reporting updates. ChangesSkybridge Emit Backend
Spotify Playlist Generator Example
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
♻️ Duplicate comments (1)
examples/spotify-playlist-generator/server/skybridge/src/views/setup_spotify_oauth.tsx (1)
8-28:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame unguarded
JSON.parseissue asparseDatainexchange_spotify_code.tsx.
getContenthas the identical structural defect — every matched branch callsJSON.parseandreturns immediately with notry/catch, so malformed JSON from the server will crash the component. The fix and the shared-utility refactor described in theexchange_spotify_code.tsxcomments apply here equally.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/spotify-playlist-generator/server/skybridge/src/views/setup_spotify_oauth.tsx` around lines 8 - 28, The getContent function currently calls JSON.parse on multiple branches without error handling; wrap each JSON.parse call in a try/catch (or better, extract a shared safeParseJson utility and call it) so malformed JSON doesn't throw and crash the component; update getContent to use the safeParseJson(resultString) helper (or inline try/catch) and return a sensible fallback (null or the original value) on parse failure, referencing the getContent function and the new safeParseJson helper to locate and change the code.
🧹 Nitpick comments (2)
examples/spotify-playlist-generator/server/skybridge/src/views/exchange_spotify_code.tsx (1)
9-29: ⚡ Quick win
parseData/getContentare duplicated verbatim across both view files.The same normalization logic appears as
parseDatahere and asgetContentinsetup_spotify_oauth.tsx(and almost certainly ingenerate_spotify_playlist.tsxas well). Extract it to a sharedutils/parseToolData.tsfile so the fix to theJSON.parseissue above only needs to be applied once.♻️ Proposed refactor
Create
src/views/utils/parseToolData.ts:export function parseToolData(data: unknown): any { if (!data) return null; if (typeof data === "object" && "result" in data && typeof (data as any).result === "string") { try { return JSON.parse((data as any).result); } catch { return (data as any).result; } } // ... remaining branches return data; }Then in each view:
-import { useState } from "react"; +import { useState } from "react"; +import { parseToolData } from "./utils/parseToolData"; -function parseData(data: unknown): any { ... } function ExchangeSpotifyCodeView() { ... - const payload = parseData(data); + const payload = parseToolData(data);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/spotify-playlist-generator/server/skybridge/src/views/exchange_spotify_code.tsx` around lines 9 - 29, Duplicate parsing logic exists as parseData and getContent; extract and consolidate it into a single exported function parseToolData in a new module (e.g., src/views/utils/parseToolData.ts) and import it where parseData/getContent are used (setup_spotify_oauth.tsx, generate_spotify_playlist.tsx, exchange_spotify_code.tsx). Update parseToolData to preserve the existing branches but wrap each JSON.parse call in try/catch and return the original string (or the original field) on parse failure instead of throwing; keep the same null/array/object handling and return the raw data as the final fallback. Ensure callers replace parseData/getContent with parseToolData to remove duplication.src/mcp_anything/emit/typescript_skybridge/phase.py (1)
297-308: 🏗️ Heavy liftMCP resources and prompts are never generated alongside tools.
_emit_viewsgenerates one React TSX view per tool but_emit_serveronly registers tools and the single discovery resource. The coding guidelines require: "MCP resources and prompts must be generated alongside tools to enable rich context for LLM tool use." Domain context (description, use cases, glossary) is already available inself.domain_modeland could be serialised as MCPresourceentries andprompttemplates inserver.ts.As per coding guidelines: "MCP resources and prompts must be generated alongside tools to enable rich context for LLM tool use."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mcp_anything/emit/typescript_skybridge/phase.py` around lines 297 - 308, Update the emit pipeline to serialize MCP "resource" and "prompt" entries alongside each tool: in _emit_views (and/or in _emit_server) after collecting domain_desc, use_cases, and glossary from self.domain_model, create MCP resource objects and prompt templates for each tool in self.design.tools and ensure they are written/registered to server.ts the same way tools are registered; reference _emit_views, _emit_server, _generate_view, self.domain_model, and self.design.tools to locate where to add creation of resource entries (serialising domain_desc/use_cases/glossary) and prompt templates and then call the same write/registration path used for tools so each tool has an accompanying MCP resource and prompt.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/spotify-playlist-generator/server/conformance_report.json`:
- Around line 61-65: The report currently sets "passed": true even when
"coverage_ratio": 0.0 < "threshold": 0.8 and "eval_run": false, which is
misleading; update the report-generation logic that emits the "passed" field
(the code that writes "passed", "coverage_ratio", "threshold", and "eval_run")
so that when eval_run is false you do not claim success — either set "passed" to
false or null, or add an explicit "skipped": true field alongside "eval_run":
false; ensure the emitted JSON reflects the chosen approach consistently
whenever no evaluation has run.
In `@examples/spotify-playlist-generator/server/Dockerfile`:
- Around line 12-19: The runtime Docker stage runs as root; add a non-root user
and set ownership of /app so the server doesn't run as root: create a user (and
group) in the runtime stage, chown the WORKDIR (/app) and copied artifacts to
that user, and add a USER directive before CMD (referencing the Dockerfile
WORKDIR /app, COPY --from=builder lines, ENV MCP_TRANSPORT and CMD
["node","dist/server.js"] to locate where to apply changes).
In `@examples/spotify-playlist-generator/server/eval_cases.json`:
- Around line 6-8: The eval_cases.json file contains placeholder test cases with
empty "input_params", empty "expected_output_pattern", and null
"expected_error"; update each referenced case (including the blocks at lines
6-8, 15-18, 24-27, 33-36, 42-45, 51-54) to include realistic "input_params"
objects matching the server API (e.g., seed tracks, mood, length), set a
meaningful non-empty "expected_output_pattern" (regex or substring that must
appear in successful playlist responses), and for edge cases provide explicit
"expected_error" values (error codes or messages) instead of null so the eval
can assert failure paths; ensure field names remain "input_params",
"expected_output_pattern", and "expected_error" so existing eval runner (which
reads those keys) continues to work.
In `@examples/spotify-playlist-generator/server/skybridge/Dockerfile`:
- Around line 13-22: The final Dockerfile stage runs as root; create a non-root
user and group (e.g., "appuser"), chown the WORKDIR and the copied artifacts
(dist, node_modules, package*.json) to that user after the COPY steps, and
switch to that user before CMD by adding a USER instruction; update references
around WORKDIR, COPY, and CMD in the final stage to ensure the process runs
unprivileged.
In `@examples/spotify-playlist-generator/server/skybridge/package.json`:
- Line 17: Replace the unstable "latest" tags with explicit semver ranges to
make installs reproducible: update the skybridge dependency entries for
"skybridge" and "@skybridge/devtools" (the JSON keys named "skybridge" and
"@skybridge/devtools") to the concrete version or semver range you developed
against (e.g. "1.2.3" or "^1.2.3") so pnpm produces a meaningful lockfile and
repeated installs resolve the same versions.
- Around line 6-8: The engines.node constraint is too strict and incompatible
with Vite 7.3.1; update the "engines.node" field in package.json (the engines
object) to a Node range that satisfies Vite 7 (for example ">=20.19.0") so Node
20.19+ and newer (including 22.12+) are allowed; edit the engines.node value
accordingly in the package.json file.
- Around line 18-19: Update the package.json dependency constraint for zod from
"^3.22.0" to "^3.25.0" so it satisfies the SDK's peer requirement; edit the
"zod" entry in the dependencies block (the package.json file containing the
"zod" key) to use "^3.25.0" and then run your package manager install to refresh
lockfile and ensure the resolved version is >=3.25.0.
In `@examples/spotify-playlist-generator/server/skybridge/src/server.ts`:
- Around line 291-316: The telemetry call in the registerTool handler always
records "ok" in the finally block even if the handler throws, so change the
pattern to capture success vs error: either introduce a status variable (e.g.,
let status = "ok"; try { ... } catch (e) { status = "error"; throw e; } finally
{ recordCall("setup_spotify_oauth", Date.now() - start, status); }) or move
recordCall("setup_spotify_oauth", ..., "ok") into the successful path and call
recordCall(..., "error") in the catch; apply the same change for the other
handlers that call recordCall with their tool names (the handlers that currently
pass "ok" in finally).
In
`@examples/spotify-playlist-generator/server/skybridge/src/views/exchange_spotify_code.tsx`:
- Around line 9-29: The parseData function currently calls JSON.parse directly
in every branch (result, content, content[0].text, and when data is a string)
which will throw a SyntaxError for malformed JSON and crash the view; wrap each
JSON.parse invocation inside a try/catch (within parseData) and on parse failure
return a safe fallback (e.g., null or the original data) instead of letting the
error propagate so the React render won't crash; update parseData to try parsing
for the "result" branch, the "content" string branch, the "content[0].text"
branch, and the direct string branch, catching JSON.parse errors and returning a
safe value.
In `@src/mcp_anything/emit/typescript_skybridge/phase.py`:
- Around line 237-263: The emitted fetch call in _render_call for http_call
omits any authentication headers, so generated clients cannot call protected
APIs; update _render_call (the http_call generation branch) to read the auth
configuration from tool.impl (e.g., impl.auth_type) or the server/backend config
and inject the correct header pattern into the fetch headers: for API key emit a
header using an env var like process.env["<SERVER>_API_KEY"], for Bearer/OAuth2
emit Authorization: `Bearer ${process.env["<SERVER>_ACCESS_TOKEN"]}` (or token
fetch flow), and for Basic emit Authorization: `Basic
${Buffer.from(username+":"+password).toString("base64")}` or env-driven
equivalents; ensure the headers object in the generated fetch includes these
values when applicable and keep existing Content-Type logic intact.
- Around line 342-349: The post-processing block for tsx currently uses an
ambiguous loop variable `l` and swallows all errors with `except Exception:
pass`; change the list comprehension to use a clear name (e.g., iterate `line`
over `raw_lines` after assigning `raw_lines = tsx.splitlines()`), and replace
the bare except with `except Exception as exc:` that logs a warning via the
module logger (logging.getLogger(__name__).warning) including context such as
`tool.name` and the exception `exc`, then fall back to the placeholder behavior;
keep the original trimming and return of `tsx` otherwise.
- Around line 494-498: The Dockerfile header comment in _emit_dockerfile
incorrectly states "Requires Node.js 24+" while the FROM line uses node:22-slim;
update the generated comment inside the content string in _emit_dockerfile to
match the actual base image and project requirements (e.g., change to "Requires
Node.js 22+ as per Skybridge SDK requirements" or otherwise align with
package.json/README) so the comment and the FROM node tag are consistent for
builds generated for self.design.server_name.
- Around line 520-550: _emit_package_json always writes a fixed dependency set
and does not add the required "websockets>=12.0" when the design contains
protocol_call tools; update _emit_package_json to inspect self.design (e.g.
iterate self.design.tools or whatever collection holds tool definitions) and if
any tool has type or kind "protocol_call" add pkg["dependencies"]["websockets"]
= ">=12.0" before serializing, then write package.json as before so generated
servers that use protocol_call include the websockets dependency.
- Around line 520-550: The engines.node constraint in _emit_package_json
currently uses ">=22.0.0" which is too permissive for Vite 7; update the
pkg["engines"]["node"] value to ">=22.12.0" in the _emit_package_json method so
installs cannot select Node 22.0–22.11, and adjust any related comments about
the minimum Node version (e.g., Dockerfile base-image comment and any
"node:22.x" base image note) to state "node:22.12+" or ">=22.12.0" so
documentation matches the new engine constraint.
- Around line 195-214: The Zod schema builder in _render_zod_shape never adds
the required "verbose" flag, so add a final entry after the loop that appends a
"verbose" field (boolean, optional, default compact) to the returned shape—e.g.
use z.boolean().optional().describe("Return verbose details when true; default
false (compact)"). Also ensure the generator that emits the TypeScript call
signature (the same code that uses _render_call) accepts a verbose parameter so
callers can opt in; _render_call already skips passing verbose to the HTTP
payload, so keep that behavior and only expose verbose in the input schema and
TS signature.
- Around line 180-193: The generated template in _render_tool_registration
hardcodes "ok" in the finally block so failures appear successful; change the
template to track success/failure (e.g., declare a status variable = "ok" before
try, set status = "error" in catch, and call recordCall("{tool.name}",
Date.now() - start, status) in finally) or alternatively call recordCall inside
both the try (on success) and catch (on failure); update the template around the
try/catch/finally that includes call_code to ensure actual exceptions set the
recorded status instead of always "ok".
In `@src/mcp_anything/emit/typescript_skybridge/prompts.py`:
- Around line 44-49: The prompt contains a contradictory state name: replace any
mention of the `"loading"` state with the runtime-accurate `"pending"` state and
update the instructions so the component must handle exactly the four states
`"idle"`, `"pending"`, `"success"`, and `"error"`; ensure
`useCallTool("{tool_name}")` usage and returned shape `{ status, data, error,
callTool }` are documented, require assigning `data` to `const result: any =
data` before use, and explicitly require the component to render distinct views
for `"idle"`, `"pending"`, `"success"`, and `"error"` (so tests like
TestPlaceholderViews.test_all_states_handled see `"pending"`).
In `@src/mcp_anything/pipeline/llm_client.py`:
- Around line 83-105: call_llm_for_text currently does a single anthropic
client.messages.create call and lacks the project's LLM retry/error handling;
wrap the LLM call in the project's LLM JSON retry utility (e.g., llm_json_retry
or the repo's named retry helper) and add structured error handling to catch
transient errors and surface structured failure info (include the model, prompt
metadata, and the caught exception) rather than letting exceptions propagate
raw; locate the anthropic import and the client.messages.create call inside
call_llm_for_text and replace the direct invocation with a retry-wrapped call
that returns the same stripped text on success and raises a controlled exception
or returns a well-formed error per the project's LLM error conventions on
repeated failure.
In `@tests/test_emit_skybridge.py`:
- Around line 92-105: _replace tempfile.mkdtemp() in _run_emitter with a
TemporaryDirectory-backed approach so the temp dir is cleaned up by the caller;
e.g., have _run_emitter return the TemporaryDirectory handle alongside
(TypeScriptSkybridgeEmitter, Path) so callers can control lifecycle, or convert
_run_emitter into a pytest fixture that yields (emitter, out) and performs
shutil.rmtree/TemporaryDirectory cleanup in teardown; update the function that
constructs the temp dir (currently calling mkdtemp) and ensure
emitter.emit_all() is still called before returning so tests can inspect the out
Path while allowing proper cleanup via the returned TemporaryDirectory or
fixture.
---
Duplicate comments:
In
`@examples/spotify-playlist-generator/server/skybridge/src/views/setup_spotify_oauth.tsx`:
- Around line 8-28: The getContent function currently calls JSON.parse on
multiple branches without error handling; wrap each JSON.parse call in a
try/catch (or better, extract a shared safeParseJson utility and call it) so
malformed JSON doesn't throw and crash the component; update getContent to use
the safeParseJson(resultString) helper (or inline try/catch) and return a
sensible fallback (null or the original value) on parse failure, referencing the
getContent function and the new safeParseJson helper to locate and change the
code.
---
Nitpick comments:
In
`@examples/spotify-playlist-generator/server/skybridge/src/views/exchange_spotify_code.tsx`:
- Around line 9-29: Duplicate parsing logic exists as parseData and getContent;
extract and consolidate it into a single exported function parseToolData in a
new module (e.g., src/views/utils/parseToolData.ts) and import it where
parseData/getContent are used (setup_spotify_oauth.tsx,
generate_spotify_playlist.tsx, exchange_spotify_code.tsx). Update parseToolData
to preserve the existing branches but wrap each JSON.parse call in try/catch and
return the original string (or the original field) on parse failure instead of
throwing; keep the same null/array/object handling and return the raw data as
the final fallback. Ensure callers replace parseData/getContent with
parseToolData to remove duplication.
In `@src/mcp_anything/emit/typescript_skybridge/phase.py`:
- Around line 297-308: Update the emit pipeline to serialize MCP "resource" and
"prompt" entries alongside each tool: in _emit_views (and/or in _emit_server)
after collecting domain_desc, use_cases, and glossary from self.domain_model,
create MCP resource objects and prompt templates for each tool in
self.design.tools and ensure they are written/registered to server.ts the same
way tools are registered; reference _emit_views, _emit_server, _generate_view,
self.domain_model, and self.design.tools to locate where to add creation of
resource entries (serialising domain_desc/use_cases/glossary) and prompt
templates and then call the same write/registration path used for tools so each
tool has an accompanying MCP resource and prompt.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e9311760-7758-4403-9f97-74982f358590
⛔ Files ignored due to path filters (1)
examples/spotify-playlist-generator/server/skybridge/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
CLAUDE.mdexamples/spotify-playlist-generator/server/Dockerfileexamples/spotify-playlist-generator/server/SKILL.mdexamples/spotify-playlist-generator/server/conformance_report.jsonexamples/spotify-playlist-generator/server/descriptions.yamlexamples/spotify-playlist-generator/server/domain_model.jsonexamples/spotify-playlist-generator/server/eval_cases.jsonexamples/spotify-playlist-generator/server/quick_queries.jsonexamples/spotify-playlist-generator/server/skybridge/.skybridge/views.d.tsexamples/spotify-playlist-generator/server/skybridge/Dockerfileexamples/spotify-playlist-generator/server/skybridge/README.mdexamples/spotify-playlist-generator/server/skybridge/index.htmlexamples/spotify-playlist-generator/server/skybridge/package.jsonexamples/spotify-playlist-generator/server/skybridge/src/discovery.tsexamples/spotify-playlist-generator/server/skybridge/src/server.tsexamples/spotify-playlist-generator/server/skybridge/src/telemetry.tsexamples/spotify-playlist-generator/server/skybridge/src/views/exchange_spotify_code.tsxexamples/spotify-playlist-generator/server/skybridge/src/views/generate_spotify_playlist.tsxexamples/spotify-playlist-generator/server/skybridge/src/views/setup_spotify_oauth.tsxexamples/spotify-playlist-generator/server/skybridge/tsconfig.jsonexamples/spotify-playlist-generator/server/skybridge/vite.config.tsexamples/spotify-playlist-generator/server/tool_spec.yamlexamples/spotify-playlist-generator/spotify-playlist-domain.yamlsrc/mcp_anything/cli.pysrc/mcp_anything/emit/typescript_skybridge/__init__.pysrc/mcp_anything/emit/typescript_skybridge/phase.pysrc/mcp_anything/emit/typescript_skybridge/prompts.pysrc/mcp_anything/pipeline/engine.pysrc/mcp_anything/pipeline/llm_client.pytests/test_emit_skybridge.py
New emit backend generates a TypeScript MCP server + ChatGPT App with LLM-generated React views for each tool. Wires --target skybridge into cli.py and engine.py, adds call_llm_for_text to llm_client.py, and ships the spotify-playlist-generator as a worked example. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Source-side fixes (skybridge emitter): - Telemetry status tracked via try/catch — errors no longer reported as "ok" - Zod schema: inject verbose flag on every tool (C-10) - HTTP call: emit Authorization/API key headers from backend.auth config - Generated Dockerfile: USER node, --chown copies, Node 22.12+ comment - package.json: engines.node >=22.12.0 (Vite 7), zod ^3.25.0, conditional ws dep - LLM view fallback: log exception via logger instead of swallowing silently - Rename ambiguous loop variable `l` → `line` Other source fixes: - prompts.py: state list now consistently "pending" (not "loading") - llm_client.call_llm_for_text: add retry/error handling matching call_llm_for_json - validation_harness: when no live eval ran, set passed=False + skipped=true - ConformanceReport.skipped field added - test_emit_skybridge: track and clean up mkdtemp dirs via autouse fixture Example artifacts updated to match the new emitter output: - spotify-playlist-generator Dockerfiles, package.json, server.ts, view - conformance_report.json: passed=false, skipped=true Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
gabrielekarra
force-pushed
the
feat/skybridge-emitter
branch
from
May 8, 2026 01:00
21fa5b7 to
e677169
Compare
Owner
Author
|
Tip For best results, initiate chat on the files or code changes.
Just let me know what you need! 🐇 |
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
examples/spotify-playlist-generator/server/skybridge/package.json (1)
17-17:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
"latest"tags onskybridgeand@skybridge/devtoolsmake installs non-reproducible.Every fresh
pnpm install(including each Docker build) can resolve a different, potentially breaking version. Pin both to an explicit semver range.🔧 Proposed fix
- "skybridge": "latest", + "skybridge": "^1.0.0",- "@skybridge/devtools": "latest", + "@skybridge/devtools": "^1.0.0",Replace
^1.0.0with the concrete version you developed against.Also applies to: 22-22
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/spotify-playlist-generator/server/skybridge/package.json` at line 17, The package.json currently uses the non-reproducible "latest" tag for the dependencies "skybridge" and "@skybridge/devtools"; change both entries to a pinned semver range or concrete version you developed against (e.g., replace "latest" with the specific semver like "^1.0.0" or an exact version number) so installs and Docker builds are reproducible; update the entries for "skybridge" and "@skybridge/devtools" accordingly.
🧹 Nitpick comments (4)
examples/spotify-playlist-generator/server/skybridge/tsconfig.json (1)
3-5: ⚡ Quick winUse
ESNext/Bundlermodule settings for a Vite project.
"module": "NodeNext"+"moduleResolution": "NodeNext"targets a bare Node.js runtime. ThebundlermoduleResolution is designed for use with bundlers and, unlike Node.js resolution modes, never requires file extensions on relative paths in imports. WithNodeNext, TypeScript enforces.jsextensions on every relative import — when using NodeNext, you need to use.jsextensions when importing TypeScript files. That conflicts with how Vite resolves modules at build time and will produce incorrect type-checking semantics for the.tsxview files.The
bundlerconfiguration is appropriate when using webpack, Vite, or other bundlers that resolve modules during the build process.Additionally,
"declaration": truewith"outDir": "dist"emits.d.tsfiles fromtsc— the pattern for libraries. When using a bundler like Vite,moduleResolution: "bundler"andmodule: "ESNext"should be used, designed for situations where TypeScript is not responsible for emitting code. Iftscis used only for type-checking here,"noEmit": trueshould replace"declaration": true.Since this tsconfig is generated by
phase.py, the fix should be applied to the emitter template as well.🔧 Proposed fix
- "target": "ES2022", - "module": "NodeNext", - "moduleResolution": "NodeNext", + "target": "ES2022", + "module": "ESNext", + "moduleResolution": "Bundler", "strict": true, "skipLibCheck": true, - "outDir": "dist", - "rootDir": "src", - "declaration": true, + "noEmit": true, "esModuleInterop": true, "jsx": "react-jsx", "jsxImportSource": "react"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/spotify-playlist-generator/server/skybridge/tsconfig.json` around lines 3 - 5, Update the tsconfig emitter so Vite builds use bundler-style resolution: change "module" from "NodeNext" to "ESNext" and "moduleResolution" from "NodeNext" to "bundler"; remove or disable "declaration": true and instead set "noEmit": true (or ensure no declaration emission) since Vite handles bundling; keep "target" as appropriate (e.g., "ES2022"/"ESNext"); apply the same changes in the phase.py tsconfig emitter template so generated tsconfig.json files use "module":"ESNext", "moduleResolution":"bundler" and no declaration emission.examples/spotify-playlist-generator/server/skybridge/src/server.ts (1)
91-158: 💤 Low valueUnguarded
JSON.parsemasks upstream non-JSON errors.Both
tokenRequest(line 104) andspotifyFetch(line 152) callJSON.parse(text)before checkingresp.ok. If a proxy/CDN/load balancer in front of Spotify returns an HTML error page (e.g.,502 Bad Gateway, captive portal), the parse throwsSyntaxError: Unexpected token '<'and the actual HTTP status (resp.status,path) is lost from the error message. Wrap each parse so the!resp.okbranch can still surface a useful error.🛡️ Proposed defensive fix
- const text = await resp.text(); - const payload = text ? JSON.parse(text) : {}; + const text = await resp.text(); + let payload: any = {}; + if (text) { + try { payload = JSON.parse(text); } catch { /* keep payload empty; raw text used below */ } + } if (!resp.ok) { - throw new Error(`Spotify token request failed (${resp.status}): ${payload.error_description ?? text}`); + throw new Error(`Spotify token request failed (${resp.status}): ${payload?.error_description ?? text.slice(0, 200)}`); }(Apply analogous change inside
spotifyFetch.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/spotify-playlist-generator/server/skybridge/src/server.ts` around lines 91 - 158, tokenRequest and spotifyFetch call JSON.parse(text) unguarded which throws on non-JSON error pages and hides HTTP status; change both functions (tokenRequest and spotifyFetch) to parse JSON defensively: attempt JSON.parse in a try/catch (or only parse when resp.headers indicates JSON) and fall back to raw text on parse failure so the !resp.ok branch can include resp.status, path (for spotifyFetch) and the raw text in the thrown Error; ensure returned payload is the parsed object when parse succeeds and the raw text or an empty object when it fails.tests/test_emit_skybridge.py (1)
299-328: ⚡ Quick winConsider adding a test that pins the C-10
verboseflag injection.
_render_zod_shapenow auto-injects a"verbose"boolean for tools that don't declare one. There's no test asserting this contract — a regression that drops the injection (or accidentally injects a duplicate when the tool already declaresverbose) would slip through. A two-line assertion inTestZodSchemawould lock it in.✅ Suggested test additions
def test_verbose_flag_auto_injected(self) -> None: _, out = _run_emitter() # _make_design tools have no verbose param content = (out / "src" / "server.ts").read_text() assert '"verbose": z.boolean().optional()' in content def test_verbose_flag_not_duplicated(self) -> None: tool = ToolSpec( name="echo", description="Echo with verbose support.", parameters=[ ParameterSpec(name="verbose", type="boolean", required=False, description="Verbose"), ], impl=ToolImpl(strategy="stub"), ) _, out = _run_emitter(_make_design(tools=[tool])) content = (out / "src" / "server.ts").read_text() # Exactly one verbose entry (the user's), no auto-injected duplicate. assert content.count('"verbose":') == 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_emit_skybridge.py` around lines 299 - 328, Add two tests to TestZodSchema to lock in the auto-injection behavior of _render_zod_shape: create test_verbose_flag_auto_injected that calls _run_emitter (using the default _make_design with no verbose param), reads "src/server.ts" and asserts that '"verbose": z.boolean().optional()' appears; and create test_verbose_flag_not_duplicated that builds a ToolSpec with a user-declared ParameterSpec(name="verbose", type="boolean", required=False), runs _run_emitter(_make_design(tools=[tool])), reads the output and asserts content.count('"verbose":') == 1 to ensure no duplicate injection. Ensure tests reference TestZodSchema, _run_emitter, _make_design, ToolSpec and ParameterSpec.src/mcp_anything/emit/typescript_skybridge/phase.py (1)
61-67: 💤 Low valueSilent fallback on malformed domain model hides configuration bugs.
_load_domain_modelreturnsNoneon any validation error, so a typo or schema-breaking change in the manifest's domain section silently downgrades the LLM view prompt to "no domain context" instead of surfacing the problem. At minimum, log the exception so operators can diagnose why their generated views are noticeably less context-aware.🪵 Proposed fix — log instead of swallow
def _load_domain_model(self, ctx: PipelineContext) -> Optional[DomainModel]: if ctx.manifest.domain_model: try: return DomainModel.model_validate(ctx.manifest.domain_model) - except Exception: + except Exception as exc: + import logging + logging.getLogger(__name__).warning( + "Failed to load domain model for Skybridge emit; views will lack domain context: %s", + exc, + ) return None return None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mcp_anything/emit/typescript_skybridge/phase.py` around lines 61 - 67, The _load_domain_model method currently swallows validation errors from DomainModel.model_validate, causing silent fallback; modify _load_domain_model to catch Exception as e and log the error (including stack trace) before returning None so operators can diagnose malformed manifest domain sections — use a module/class logger (e.g., logging.getLogger(__name__) or self.logger) and logging.exception or logger.error(..., exc_info=True) when handling the exception inside _load_domain_model(PipelineContext) where DomainModel.model_validate is called.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/spotify-playlist-generator/server/Dockerfile`:
- Around line 6-9: The Dockerfile currently copies only skybridge/package.json
before running pnpm install, causing pnpm to regenerate the lockfile; change the
COPY order to copy both skybridge/package.json and skybridge/pnpm-lock.yaml (so
pnpm-lock.yaml is present during install) and add the --frozen-lockfile flag to
the pnpm install command (keep the subsequent COPY skybridge/. after install).
This ensures builds use the committed lockfile and prevents pnpm from updating
dependencies during image build.
In `@examples/spotify-playlist-generator/server/eval_cases.json`:
- Line 5: The eval cases use placeholder tool_name values like
"uc_01"/"uc_02"/"uc_03" which don't match the registered tool identifiers;
update each tool_name entry in eval_cases.json to the actual registered tool
names ("generate_spotify_playlist", "setup_spotify_oauth",
"exchange_spotify_code") so the runner will route to the real tools (match each
uc_0X to its corresponding registered name). Ensure every case that currently
has "uc_01"/"uc_02"/"uc_03" is replaced consistently so tool routing will work
with the names used in descriptions.yaml and views.d.ts.
In `@examples/spotify-playlist-generator/server/skybridge/Dockerfile`:
- Around line 6-11: The Dockerfile currently copies only package.json before
running pnpm install, which causes the lockfile to be regenerated each build;
modify the pre-install copy to include pnpm-lock.yaml (copy package.json and
pnpm-lock.yaml together) and change the install command in the RUN that uses
corepack/pnpm to run pnpm install --frozen-lockfile so builds use the committed
lockfile and avoid silently drifting dependency versions (update the COPY and
the RUN that calls corepack enable pnpm && pnpm install).
In `@examples/spotify-playlist-generator/server/skybridge/README.md`:
- Line 15: Update the README entry that currently reads "Node.js 22+" to
"Node.js 22.12+" so it matches the project's package.json engines field
(>=22.12.0) and Vite 7's minimum requirement; locate the string "Node.js 22+" in
examples/spotify-playlist-generator/server/skybridge/README.md and replace it
with "Node.js 22.12+".
In
`@examples/spotify-playlist-generator/server/skybridge/src/views/setup_spotify_oauth.tsx`:
- Around line 8-28: getContent currently calls JSON.parse in multiple branches
and will throw a SyntaxError on malformed JSON; replace those direct JSON.parse
calls with the same safeParse helper used in exchange_spotify_code.tsx (or
implement an equivalent try/catch wrapper) so parsing failures return null or
the original data instead of throwing; update calls inside getContent (the
branches checking "result", "content", content[0].text, and the string case) to
use safeParse(...) and preserve the existing return semantics when parse fails.
In `@src/mcp_anything/cli.py`:
- Around line 53-58: The CLI currently allows "--target skybridge" for the
"generate" command but the legacy pipeline (ALL_PHASES / implement handler in
engine.py) has no skybridge implement branch and will silently fall back to
Python/FastMCP; remove "skybridge" from the choices list passed to
gen.add_argument("--target", choices=[...]) in src/mcp_anything/cli.py so that
"generate" only accepts "fastmcp" and "mcp-use" (and update the help string to
reflect this), leaving "skybridge" available only for the build path that uses
DOMAIN_PHASES/emit.
---
Duplicate comments:
In `@examples/spotify-playlist-generator/server/skybridge/package.json`:
- Line 17: The package.json currently uses the non-reproducible "latest" tag for
the dependencies "skybridge" and "@skybridge/devtools"; change both entries to a
pinned semver range or concrete version you developed against (e.g., replace
"latest" with the specific semver like "^1.0.0" or an exact version number) so
installs and Docker builds are reproducible; update the entries for "skybridge"
and "@skybridge/devtools" accordingly.
---
Nitpick comments:
In `@examples/spotify-playlist-generator/server/skybridge/src/server.ts`:
- Around line 91-158: tokenRequest and spotifyFetch call JSON.parse(text)
unguarded which throws on non-JSON error pages and hides HTTP status; change
both functions (tokenRequest and spotifyFetch) to parse JSON defensively:
attempt JSON.parse in a try/catch (or only parse when resp.headers indicates
JSON) and fall back to raw text on parse failure so the !resp.ok branch can
include resp.status, path (for spotifyFetch) and the raw text in the thrown
Error; ensure returned payload is the parsed object when parse succeeds and the
raw text or an empty object when it fails.
In `@examples/spotify-playlist-generator/server/skybridge/tsconfig.json`:
- Around line 3-5: Update the tsconfig emitter so Vite builds use bundler-style
resolution: change "module" from "NodeNext" to "ESNext" and "moduleResolution"
from "NodeNext" to "bundler"; remove or disable "declaration": true and instead
set "noEmit": true (or ensure no declaration emission) since Vite handles
bundling; keep "target" as appropriate (e.g., "ES2022"/"ESNext"); apply the same
changes in the phase.py tsconfig emitter template so generated tsconfig.json
files use "module":"ESNext", "moduleResolution":"bundler" and no declaration
emission.
In `@src/mcp_anything/emit/typescript_skybridge/phase.py`:
- Around line 61-67: The _load_domain_model method currently swallows validation
errors from DomainModel.model_validate, causing silent fallback; modify
_load_domain_model to catch Exception as e and log the error (including stack
trace) before returning None so operators can diagnose malformed manifest domain
sections — use a module/class logger (e.g., logging.getLogger(__name__) or
self.logger) and logging.exception or logger.error(..., exc_info=True) when
handling the exception inside _load_domain_model(PipelineContext) where
DomainModel.model_validate is called.
In `@tests/test_emit_skybridge.py`:
- Around line 299-328: Add two tests to TestZodSchema to lock in the
auto-injection behavior of _render_zod_shape: create
test_verbose_flag_auto_injected that calls _run_emitter (using the default
_make_design with no verbose param), reads "src/server.ts" and asserts that
'"verbose": z.boolean().optional()' appears; and create
test_verbose_flag_not_duplicated that builds a ToolSpec with a user-declared
ParameterSpec(name="verbose", type="boolean", required=False), runs
_run_emitter(_make_design(tools=[tool])), reads the output and asserts
content.count('"verbose":') == 1 to ensure no duplicate injection. Ensure tests
reference TestZodSchema, _run_emitter, _make_design, ToolSpec and ParameterSpec.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3cb5f9c1-b123-49ae-922e-3f7c3be322ff
⛔ Files ignored due to path filters (1)
examples/spotify-playlist-generator/server/skybridge/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
CLAUDE.mdexamples/spotify-playlist-generator/server/Dockerfileexamples/spotify-playlist-generator/server/SKILL.mdexamples/spotify-playlist-generator/server/conformance_report.jsonexamples/spotify-playlist-generator/server/descriptions.yamlexamples/spotify-playlist-generator/server/domain_model.jsonexamples/spotify-playlist-generator/server/eval_cases.jsonexamples/spotify-playlist-generator/server/quick_queries.jsonexamples/spotify-playlist-generator/server/skybridge/.skybridge/views.d.tsexamples/spotify-playlist-generator/server/skybridge/Dockerfileexamples/spotify-playlist-generator/server/skybridge/README.mdexamples/spotify-playlist-generator/server/skybridge/index.htmlexamples/spotify-playlist-generator/server/skybridge/package.jsonexamples/spotify-playlist-generator/server/skybridge/src/discovery.tsexamples/spotify-playlist-generator/server/skybridge/src/server.tsexamples/spotify-playlist-generator/server/skybridge/src/telemetry.tsexamples/spotify-playlist-generator/server/skybridge/src/views/exchange_spotify_code.tsxexamples/spotify-playlist-generator/server/skybridge/src/views/generate_spotify_playlist.tsxexamples/spotify-playlist-generator/server/skybridge/src/views/setup_spotify_oauth.tsxexamples/spotify-playlist-generator/server/skybridge/tsconfig.jsonexamples/spotify-playlist-generator/server/skybridge/vite.config.tsexamples/spotify-playlist-generator/server/tool_spec.yamlexamples/spotify-playlist-generator/spotify-playlist-domain.yamlsrc/mcp_anything/cli.pysrc/mcp_anything/emit/typescript_skybridge/__init__.pysrc/mcp_anything/emit/typescript_skybridge/phase.pysrc/mcp_anything/emit/typescript_skybridge/prompts.pysrc/mcp_anything/models/validation.pysrc/mcp_anything/pipeline/engine.pysrc/mcp_anything/pipeline/llm_client.pysrc/mcp_anything/pipeline/validation_harness.pytests/test_emit_skybridge.py
- cli.py: drop 'skybridge' from `generate --target` choices since the legacy pipeline silently falls back to Python/FastMCP; Skybridge is build-only. - skybridge emitter: pin `skybridge` and `@skybridge/devtools` to ^0.36.2 instead of `latest` for reproducible installs. - example skybridge/package.json: same pinning. - example Dockerfiles (outer wrapper + skybridge/): copy pnpm-lock.yaml before install and use `pnpm install --frozen-lockfile`. - example skybridge/README.md: tighten Node minimum from 22+ to 22.12+ to match Vite 7's requirement and the pinned engines.node. - example setup_spotify_oauth.tsx: mirror the safeParse helper used in exchange_spotify_code.tsx so malformed JSON does not crash the view. - example eval_cases.json: replace placeholder uc_01/uc_02/uc_03 tool names with the real registered tool names and populate realistic input_params and expected_output_pattern values. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Owner
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
Adds `pnpm dev:ui` (runs `vite`) so users can view the generated React tool views in the browser. Updates the generated README to document both `dev` (MCP server) and `dev:ui` (Vite UI at localhost:5173). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…, and view API
Three root-cause fixes so generated servers work without manual patches:
1. Path param auto-detection (phase.py): emitter scans http_path for {param}
templates and forces style="path", so path params are never serialized as
query strings even when arg_mapping is empty or missing.
2. arg_mapping in tool design (tool_design.py): LLM prompt now requires arg_mapping
on every http_call tool with path/query/body guidance. Parser validates and
stores it into ToolImpl so the emitter has ground-truth param styles.
3. OpenAPI-first routing (tool_design.py + cli.py): when a spec file is provided
as data source, routes and param styles are extracted deterministically
(_openapi_to_tool_specs) before any LLM reshape — prevents invented paths.
Also: tsx in devDependencies (was missing, broke skybridge dev), correct skybridge
CLI scripts (dev/build/start), view prompt fixes (useViewState signature,
displayMode values, callTool args), max_tokens=16000 for view generation,
max_tokens truncation warning in llm_client.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The FastAPI analyzer treated `async def f(req: SomeModel)` as a single opaque `object` param, so generated MCP tools exposed a `req` blob instead of the model's fields. Now we walk the file's BaseModel subclasses (and follow imports for cross-file models) and expand the body into one ParameterSpec per field, with location="body" so the existing HTTP body codegen handles them correctly. Path-param collisions and non-Pydantic classes fall through unchanged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.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
1 participant
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
--target skybridgeto bothgenerateandbuildsubcommands, generating a TypeScript MCP server + ChatGPT App with LLM-generated React views per toolsrc/mcp_anything/emit/typescript_skybridge/withphase.pyandprompts.py; wired intoengine.pycall_llm_for_texttollm_client.pyfor free-form text generation (used by Skybridge for React view source)examples/spotify-playlist-generator/as a worked end-to-end example with three generated React viewsTest plan
pytest tests/test_emit_skybridge.py -v— new Skybridge emitter unit/integration testsmcp-anything build --brief examples/spotify-playlist-generator/spotify-playlist-domain.yaml --target skybridgeand verify theskybridge/output tree matches the example--target fastmcpand--target mcp-usestill work unchangedmcp-anything build --helpshowsskybridgein choices🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Chores