Skip to content

Modularize MCP bridge, add functional tests, fix version drift#27

Merged
hyperpolymath merged 1 commit into
mainfrom
claude/analyze-repo-improvements-LqEoD
Apr 9, 2026
Merged

Modularize MCP bridge, add functional tests, fix version drift#27
hyperpolymath merged 1 commit into
mainfrom
claude/analyze-repo-improvements-LqEoD

Conversation

@JoshuaJewell

Copy link
Copy Markdown
Owner

Summary

  • Modularize MCP bridge: Split the monolithic 1126-line main.js into 7 focused modules (security.js, api-clients.js, tools.js, logger.js, version.js, offline-menu.js, generate-offline-menu.js), reducing main.js to ~230 lines of transport + dispatch logic
  • Add 44 functional tests: New tests/security_test.js exercising actual code paths — injection detection (including unicode bypass prevention via zero-width chars, Cyrillic confusables, fullwidth chars), rate limiting, input validation, tool/cartridge name validation, and error sanitization
  • Fix version drift: Sync version 0.3.1 across package.json, mcp-bridge/package.json, Justfile, and new lib/version.js single source of truth
  • Fix documentation: Correct internal dev machine paths in EXPLAINME.adoc, update cartridge count (92→96) in TOPOLOGY.md, update file map to reflect actual repo structure
  • Improve security: Add unicode normalization to prompt injection detection (strips zero-width chars, normalizes Cyrillic/fullwidth confusables, collapses whitespace) to prevent bypass techniques
  • Add structured logging: JSON-structured log output to stderr with configurable log levels via BOJ_LOG_LEVEL
  • Wire npm test: Now runs real tests instead of echo no-op; adds ESLint config for the MCP bridge

Test plan

  • All 44 security module tests pass (npm test)
  • Injection detection correctly classifies none/low/medium/high/critical
  • Unicode bypass prevention verified (zero-width, Cyrillic, fullwidth)
  • Input size limits, field validation, tool name validation all tested
  • Error sanitization strips paths, stack traces, env vars
  • Manual: verify node mcp-bridge/main.js starts and responds to MCP initialize
  • Manual: verify existing Deno smoke/E2E tests still pass

https://claude.ai/code/session_01FJczaDsKEoetZGKrZ1fdtc

Split the monolithic 1126-line mcp-bridge/main.js into focused modules:
- lib/security.js: injection detection, rate limiting, input validation,
  error sanitization with unicode normalization for bypass prevention
- lib/api-clients.js: GitHub, GitLab API passthroughs and BoJ REST wrappers
- lib/tools.js: MCP tool schema definitions
- lib/logger.js: structured JSON logging to stderr
- lib/version.js: single source of truth for server version (0.3.1)
- lib/offline-menu.js: static cartridge manifest for offline mode

Add 44 functional tests (tests/security_test.js) exercising actual code
paths: injection detection, unicode bypass prevention, rate limiting,
input validation, tool name validation, and error sanitization.

Fix version drift: sync 0.3.1 across package.json, mcp-bridge/package.json,
Justfile, and version.js. Fix cartridge count (92 -> 96) in TOPOLOGY.md.
Fix internal dev machine paths in EXPLAINME.adoc. Update file map to
reflect actual repo structure. Wire npm test to run real tests.

https://claude.ai/code/session_01FJczaDsKEoetZGKrZ1fdtc

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a27083aa2d

ℹ️ 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".

{ name: "ml-mcp", version: "0.1.0", domain: "ML/AI", protocols: ["MCP","REST"], status: "Available", available: true },
{ name: "research-mcp", version: "0.1.0", domain: "Research", protocols: ["MCP","REST"], status: "Available", available: true },
{ name: "codeseeker-mcp", version: "0.1.0", domain: "Code Intelligence", protocols: ["MCP","REST"], status: "Available", available: true },
{ name: "lang-mcp", version: "0.1.0", domain: "Languages", protocols: ["MCP","REST"], status: "Available", available: true },

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve lang-mcp metadata in offline manifest

The lang-mcp entry in the new offline manifest dropped its languages array, so when the REST API is unreachable both boj_menu and boj_cartridge_info now return less data than before for that cartridge. This is a functional regression for offline clients that rely on supported-language metadata (the API spec also documents this field as available for lang-mcp), and it makes capability discovery incomplete exactly in the fallback path this file is meant to support.

Useful? React with 👍 / 👎.

@hyperpolymath hyperpolymath merged commit 129c869 into main Apr 9, 2026
15 of 21 checks passed
@hyperpolymath hyperpolymath deleted the claude/analyze-repo-improvements-LqEoD branch April 9, 2026 10:23
hyperpolymath added a commit that referenced this pull request Jun 1, 2026
…name sync (closes #183) (#184)

## Summary

Closes the boj-server side of the cartridge-schema-validation campaign
that landed in boj-server-cartridges#21+#23+#25+#26+#27 this session.
Three changes ship together:

1. **\`BojRest.Catalog\` validates strictly at boot** — adds
\`ex_json_schema\`, compiles \`schemas/cartridge-v1.json\` once,
validates every manifest against it on load, and rejects (does not
insert into ETS, logs an error) any cartridge that fails. Strict from
day one per owner direction.
2. **\`scripts/fetch-cartridges.sh\` runs registry-side strict
validation** as a defense-in-depth pre-flight, and now copies the
registry's \`schemas/\` directory beside the cartridges cache so the
catalog's default \`:schema_path\` finds the mirror without
configuration.
3. **In-tree \`cartridges/\` refreshed against the cleaned canonical** —
the 2026-05-26 snapshot had 43/125 manifests failing current strict
validation. Refresh script (\`scripts/refresh-bundled-cartridges.sh\`)
preserves the 125-cartridge curation per the owner's \"curate-keep at
125\" direction; the three rename targets from boj-server-cartridges#27
propagate (\`boj-health\` → \`boj-health-mcp\`, \`origenemcp\` →
\`origene-mcp\`, \`opendatamcp\` → \`opendata-mcp\`). Post-refresh:
**125/125 strict-valid**.

## Tests

\`elixir/test/catalog_test.exs\` updated:

- \`boj-health\` references → \`boj-health-mcp\`.
- \"auth method known value\" test tightened to the canonical schema
enum \`[none, api-key, oauth2, vault]\` only.
- New: every loaded cartridge carries every schema-required top-level
field.
- New: \`schemas/cartridge-v1.json\` SHA-256 equals the \`Pin SHA-256:\`
line in \`schemas/PINNED-SHA\` — guards silent mirror drift.

## Compatibility

Any external consumer that called
\`BojRest.Catalog.get(\"boj-health\")\` / \`get(\"origenemcp\")\` /
\`get(\"opendatamcp\")\` now gets \`:not_found\`. The old names violate
the canonical schema's role-suffix pattern; alias support was
deliberately not added because re-introducing the violating names
defeats the strict gate.

The fetcher remains backward-compatible: if the registry is an older
snapshot without \`tools/validate-cartridges/\`, or if Deno isn't
installed, the strict pre-flight is skipped with a clear log line and
the catalog still validates at load.

## Test plan

- [x] Refresh script run locally against current canonical; 125/125
strict-valid.
- [x] All commits GPG-signed.
- [ ] CI green (Elixir 1.18 / OTP 25; deno strict in fetcher path).

Closes #183.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

3 participants