Repository navigation
refactor: split MCP into tools/+lib, SDK migration, tests, CI and ajv validation - #1
Merged
Merged
Conversation
- Extract 16 tools from monolithic server.mjs into mcp/tools/ with
{name, description, inputSchema, handler} contract
- Extract shared helpers into mcp/lib/ (paths, kits, narrative, images, ...)
- Add mcp/registry.mjs as single dispatch/manifest source of truth
- Rewrite server.mjs on @modelcontextprotocol/sdk low-level Server with
JSON Schema tools (no zod) + CAROUSEL_TOOL_LOG JSONL call log
- Add scripts/sync-manifest.mjs to keep manifest.json tools[] in sync
- CAROUSEL_GENERATOR_HOME env for testable home dir
- Unit tests for text, paths, blocks, narrative, colors, article, const - Handler tests for carousel CRUD, edit_slide, kits, social_copy with isolated CAROUSEL_GENERATOR_HOME - Registry contract test (16 tools, schema+handler shape) - JSON-RPC integration incl. CAROUSEL_TOOL_LOG ok:true/false - manifest sync --check as test; scripts/sync-manifest.mjs in npm test - hexNorm now normalizes missing # prefix
- .github/workflows/ci.yml: unit (node:test+smoke+manifest), tools-e2e (JSON-RPC + chrome-headless-shell + fixture from-url), agent-e2e (opencode --model free + CAROUSEL_TOOL_LOG + verifier) - scripts/verify-agent-output.mjs: coverage 16/16 ok:true + artifact assertions (carousel.json v2, kit.json, PNGs, social.md, source.md); job exit = verifier, not the model - scripts/fixture-server.mjs + test-fixture-server.mjs: offline homepage/article/images for *_from_url in CI - scripts/test-from-url.mjs: e2e brand_kit_from_url + carousel_from_url against fixture (photoNeeds, narrativeAudit, category, source.md) - scripts/run-agent-e2e.mjs: spawn opencode with isolated home + tool log - tests/integration/verify-agent-output.test.mjs (5 tests) - pre-commit now checks all mcp/**/*.mjs + scripts/*.mjs + manifest sync
- AGENTS.md: new MCP layout (tools/, lib/, registry, SDK server), how to add a tool (file + registry + sync-manifest + tests + README), env vars (CAROUSEL_GENERATOR_HOME, CAROUSEL_TOOL_LOG, CHROME_PATH), CI job overview, refreshed verification checklist - README.md Development: architecture bullets, full test scripts, env vars, CI section with agent-e2e/verifier workflow
…_slide add action
…ilter - export_pdf: chrome print-to-pdf, aliases, slides subset - duplicate_carousel: copy to new company/slug with assets - delete_brand_kit: two-step preview/confirm, protects repo kits - list_carousels: optional company filter - registry 19→22, manifest synced, counts in tests/docs/e2e - fix slug(carrusel) empty-arg bypass on new tools - export_pdf format free-string for 4:5 aliases
Fix edit_slide schema aliases (field/title/desc/emoji and layout props) rejected by ajv additionalProperties. Add split, set_layout, block CRUD, item and pill actions while keeping 22 tools. Add schema-contract test scanning handler reads vs inputSchema. Declare save_carousel format and carousel_from_url category. Wire English SERVER_INSTRUCTIONS into the MCP Server; translate PHOTO/NARRATIVE protocols and handoff nextSteps. Cover new actions in handlers/validate tests, JSON-RPC smoke and agent e2e. Document actions, protocols and tool-prefix note in README/AGENTS.
…ti pattern - convert all 21 remaining tool top-level descriptions to English - keep schema field descriptions and ajv runtime errors in Spanish - honor CAROUSEL_GENERATOR_HOME in test-tools.mjs path asserts
- biome.json scopes mcp/, scripts/, tests/ and root mjs/json (app/index.html stays out). - noisy style rules are warn so lint exits clean without fighting repo idioms. - dead code removed while fixing lint findings: unused imports in brand/load_carousel, duplicated PROTECT_ROOTS, unused legacyElements arg, unused vars in install and duplicate_carousel, unused caption params and the unused isNonTinyFile helper. - npm run lint / lint:fix wired into npm test and the pre-commit hook.
…iption sync - agent-e2e is now a canary (continue-on-error) so free-model flakiness stops blocking PRs; required coverage stays in unit + tools-e2e. - sync-manifest now always derives tools[].description from the registry (they were frozen) and --check reports stale descriptions, not only name/count drift. - bundle:check fails when dist/*.mcpb is missing or its version/tool count differs from the registry; install.mjs only advertises the 1-click bundle when it is fresh. - install.mjs skips opencode/claude-desktop configs that fail to parse instead of overwriting them, and the verification hint no longer mentions a non-existent command. - test-tools uses a throwaway CAROUSEL_GENERATOR_HOME unless one is exported, so npm test never writes to the real ~/.carousel-generator. - .mcpbignore keeps dev-only files out of the bundle.
…sist render
F1 batch: the mutations that already landed on disk no longer report errors the
agent cannot act on, and destructive actions no longer default to slide 1.
- writeJsonAtomic (mcp/lib/fsutil.mjs) + pruneAssets: carousel.json and kit.json
are written via temp+rename; orphan photos in assets/ are cleaned on persist.
- readCarousel reports corrupt/invalid JSON in Spanish instead of a raw
SyntaxError; list_carousels flags those entries with {corrupt:true, error}.
- renderCarouselSafe: tools persist first, then a failed re-render comes back as
a warning (edit_slide, set_slide_bg, set_slide_photo, set_carousel_meta,
duplicate_carousel, generate_carousel, save_carousel).
- integer schemas with ranges (slide, index, to, at, items, maxHashtags, scrim)
so 1.5/0 are rejected before the handler; explicit slide required for delete.
- slug alias usable: required is now company-only and handlers validate name.
- save_carousel implements `open`, re-renders, and only treats "no existe" as a
fresh upsert (corrupt existing JSON fails loudly).
- set_slide_photo: async download/convert happens before reading the carousel,
converted files land in tmp (never next to the user source), old asset removed.
- split without payload.at on a <=3 item slide explains how to proceed instead of
silently cutting at 2; split ids are deduped and __parts survives round-trips.
- gradient round-trip stores the kit gradient's exact name; spawn gets an error
listener; preview sweeps stale PNGs; delete size is recursive.
- add mcp/lib/net.mjs safeFetch: http(s) only, private/loopback/link-local blocked, manual redirects (max 3), streaming byte cap and timeout; wire into fetchBrandHTML, downloadLogoDataURL, downloadPhotoToTmp - contain local paths (set_slide_photo source, logo.imagePath, hydrateCarousel assets, outputDir) to $HOME/$TMPDIR//tmp/CAROUSEL_GENERATOR_HOME via assertPathAllowed; bypass CAROUSEL_GENERATOR_ALLOW_LOCAL=1 - add mcp/lib/safe.mjs and sanitize css/style/kit values (set_slide_bg css, edit_slide styles and bgPos, normSlideArg background, save_carousel/save_brand_kit kits); escape style sinks in app/index.html with escAttr - mask logo base64 in load_brand_kit unless includeLogo; keep carousel_from_url temp photos cleaned up after embed; cap carousel.json reads at 32MB and save_carousel at 80 slides - tests: net/safe/paths guards plus F2 handler guards (202 pass), docs in README/AGENTS
- add tests/helpers/tools.mjs with EXPECTED_TOOL_NAMES (22) as the drift checklist, independent from mcp/registry.mjs on purpose - registry.test.mjs, verify-agent-output.mjs, its integration test and test-tools.mjs now import it instead of carrying private copies (test-tools asserts the exact count, not >= 22) - add tests/integration/docs.test.mjs: README tools table rows, "twenty-two tools" claim, package.json/manifest/SERVER_INFO version sync, manifest descriptions vs registry, SERVER_INSTRUCTIONS tool mentions
slug("") falls back to "carrusel", so the Falta `company` / Falta `name`
guards added in the integrity pass were unreachable when the handler was
called directly or ajv did not enforce the field: callers got a confusing
"no existe" instead. Validate the raw trimmed strings first, then slug,
in delete_carousel, edit_slide, load_carousel, render_preview,
review_slide_images, set_carousel_meta, set_slide_bg, set_slide_photo and
social_copy (same pattern export_pdf already used).
- tests/lib/persist.test.mjs: atomic write without .tmp leftovers, Spanish errors for missing/corrupt/invalid/oversized carousel.json, hydrateCarousel asset inlining + bg field mapping, ../ escape refusal, pruneAssets stale-entry and orphan-image reconciliation - tests/tools/set-slide-photo.test.mjs: local file and file: prefix happy paths with manifest + assets assertions, plus rejections for outside allowlist, missing file, invalid/out-of-range slides, missing args and unknown carousel
…guards Closes the remaining coverage gaps over the 22 tools: - import_editor_state: top-level upsert, base64 photo migrated to assets/ with no base64 left in carousel.json, payload string with action wrapper, unparseable/empty payload and save_carousel validation errors - render_preview: raw company/name checks, unknown carousel, slides out of range, real render of slide 1 in feed (or deterministic no-chrome fallback) - brand_kit_from_url / carousel_from_url: offline validation for missing, invalid and non-http URLs plus private/local hosts (no network needed) - edit_slide: delete requires an explicit valid slide, last slide of a carousel cannot be removed, delete_block refuses protected roots without blockId
…tions
- appendHandoff now returns {ok, summary, render, open, nextSteps} as
parseable JSON instead of prose + "---" + handoff; callTool wraps any
leftover non-JSON string as {ok, summary}, so every tool result parses
with JSON.parse
- prose-only handlers (brand_kit_from_url save/dry-run, delete_carousel,
delete_brand_kit, save_brand_kit, list_brand_kits) now return
structured JSON with the human-readable text in summary
- registry exposes MCP annotations (readOnlyHint, destructiveHint,
idempotentHint, openWorldHint) for all 22 tools; tests assert coverage,
the destructive/open-world/read-only sets and readOnly => not destructive
- SERVER_INSTRUCTIONS documents the JSON envelope and the happy path
(generate -> review -> edit -> validate -> social_copy -> export)
- nextSteps unified to English across all tools (runtime messages and
summaries stay Spanish by convention)
- inputSchema property descriptions translated to English (~141 strings
across 22 files); tool-level WHAT/WHEN/SISTERS/ANTI untouched
- scripts/lib/jsonrpc.mjs: shared stdio JSON-RPC helpers (rpc, toolCall, contentText, textJson) extracted from test-tools; test-tools now imports them instead of carrying its own copy - scripts/e2e-driver.mjs: spawns an isolated home + local fixture server and drives every one of the 22 tools over JSON-RPC, asserting each result parses as pure JSON (F5 envelope), the tool-specific payloads, and 100% tool coverage; render_preview/export_pdf accept no-chrome so the job needs no browser (real PNGs stay in tools-e2e) - ci.yml: new required `e2e-driver` job; agent-e2e comment now matches - package.json: npm run test:driver; README + AGENTS document the job
…i-page print (#2043, #2050)
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
Refactors the monolithic
mcp/server.mjs(2.4k+ lines) into a modular structure, migrates to the official MCP SDK, adds a fullnode:testsuite, GitHub Actions CI (including a free-model agent e2e), and strict ajv input validation. Bumps to 2.4.0.Changes
Architecture
mcp/tools/*.mjs— one module per tool (16 tools) with{ name, description, inputSchema, handler };inputSchemais the source of truthmcp/lib/*.mjs— shared helpers (paths, kits, narrative, images, persist, render, social, article, brand, preview, handoff, blocks, text, colors, html, const, validate)mcp/registry.mjs— static imports → single dispatch (getTool,listToolsForRpc,callTool)mcp/server.mjs— ~42-line bootstrap on@modelcontextprotocol/sdk(Server+ JSON Schema, no zod) +CAROUSEL_TOOL_LOGJSONLValidation (ajv)
mcp/lib/validate.mjsvalidatesargsincallToolbefore the handleradditionalProperties: falseon root + nested-with-properties (runtime only, not published intools/list); freeform bare objects (carousel,kit,meta) stay openFalta company (requerido).,`format` debe ser uno de: …,Propiedad no permitida: foo.)openingenerate_carousel+save_carouselschemas (would fail under strict mode)Tests (93
node:test)tests/lib/*,tests/tools/*(incl.validate.test.mjs33 cases),tests/integration/rpc.test.mjs+verify-agent-output.test.mjsCI (
.github/workflows/ci.yml)sync-manifest --check+node --test+ editor smokeopencode/mimo-v2.6-flash-free) exercises all 16 tools; exit code from verifier (16/16ok:true+ artifacts)opencode.jsonregisters the local MCP;--auto+/tmppermissions for non-interactive runDocs
AGENTS.md+README.mdupdated for new architecture, validation rules, CI jobs, and model IDTest plan
npm test— syntax, manifest sync, 93 node:test, smoke, tools e2e, fixture e2e — all greenCommits
64 files changed, +6174 / −2522