Skip to content

Migrate to McpServer.registerTool; fix double-encoded SoQL like pattern - #5

Draft
noneck wants to merge 2 commits into
mainfrom
refactor/mcpserver-migration
Draft

Migrate to McpServer.registerTool; fix double-encoded SoQL like pattern#5
noneck wants to merge 2 commits into
mainfrom
refactor/mcpserver-migration

Conversation

@noneck

@noneck noneck commented Jul 6, 2026

Copy link
Copy Markdown
Member

Problem

  • src/index.ts used the low-level Server API: hand-written JSON-schema ListTools handler, per-case z.object(...).parse, a 7-way switch, and 7 copies of the JSON content envelope (~200 lines of boilerplate).
  • Version drift: the Server constructor hardcoded 1.0.0 while package.json says 1.0.1.
  • CityRecordNotice declared 32 optional fields that were never field-accessed (results are only JSON.stringify'd).
  • buildUrl had a single caller.
  • Bug: getNoticesByAgency embedded %25 + encodeURIComponent() inside the SoQL string, which searchParams.set encoded again — Socrata received a literal %25NAME%25 pattern (spaces as %20), not the % wildcard, so agency matches with spaces/special characters silently failed.

Fix

  • Migrated to McpServer.registerTool(name, {description, inputSchema: zodShape}, handler) (SDK 1.29.0). Tool names, descriptions, and input schemas are unchanged from a client's perspective. Server construction lives in src/server.ts (buildServer()) so tests can attach an in-memory transport; src/index.ts is now just the stdio entrypoint.
  • Version read from package.json via createRequire(import.meta.url) — one source of truth. No version bump.
  • CityRecordNotice is now Record<string, string>; the field list is preserved as a doc comment.
  • buildUrl inlined into sodaFetch.
  • Separate commit: build the SoQL like value unencoded (literal % wildcard per the SoQL like docs, single quotes escaped by doubling) and let URLSearchParams do the single encoding pass.

Test plan (already run)

  • Added minimal node:test setup (npm test): builds, then runs test/*.test.mjs.
  • test/tools.test.mjs — SDK Client over InMemoryTransport asserts the 7 tool names are unchanged and each has a description + object input schema. Passes.
  • test/encoding.test.mjs — mocks fetch, captures the final URL, asserts the decoded $where equals upper(agency_name) like upper('%Parks & Recreation%') and the raw query string contains %25 but not %2525. Written first: fails on the old code (%25Parks%20%26%20Recreation%25), passes after the fix.
  • npm run build clean.

🤖 Generated with Claude Code

noneck and others added 2 commits July 5, 2026 23:56
…lim data typing

- Replace low-level Server (hand-written ListTools JSON schemas, per-case
  zod .parse, 7-way switch, repeated JSON content envelopes) with
  McpServer.registerTool; tool names, descriptions, and input schemas
  are unchanged from a client's perspective.
- Read the server version from package.json via createRequire instead of
  a hardcoded "1.0.0" that had drifted from package.json's 1.0.1.
- Replace the 32-field CityRecordNotice type with Record<string, string>
  (results are only JSON.stringify'd); field list kept as a doc comment.
- Inline buildUrl into its single caller sodaFetch.
- Add minimal node:test setup (npm test) with a tool-list regression test
  using the SDK's InMemoryTransport.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The $where value embedded %25 and encodeURIComponent output, which
URLSearchParams.set then encoded again — Socrata received a literal
'%25NAME%25' pattern (with %20 for spaces) instead of the % wildcard,
so agency matches with spaces or special characters silently failed.

Per the SoQL like docs (https://dev.socrata.com/docs/functions/like.html)
the wildcard is a literal % in the query; %25 is only its URL encoding,
which the HTTP layer already applies once. Build the SoQL value
unencoded (escaping single quotes by doubling) and let searchParams.set
do the single encoding pass. Test written first; it fails against the
old code and passes now.

Also fix the test script glob (node --test with a bare directory arg
fails on newer Node).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@noneck
noneck marked this pull request as draft July 6, 2026 04:09
@noneck

noneck commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

The double-encoding bug fix (a034f12) has been extracted into its own PR against main (fix/agency-search-double-encoding) so it can merge without waiting on the refactor review. This branch should be rebased once that merges.

@noneck

noneck commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

Heads up: main has moved — v1.0.2 shipped today (SoQL escaping + NY-timezone fixes from #7 squash-merged, plus a version-from-package.json read and a repository field in package.json required by npm provenance). This branch needs a rebase onto current main before it can proceed; expect overlap in src/index.ts (version wiring) and package.json.

@noneck

noneck commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

Re-evaluated against current main on 2026-08-09. What still applies from this PR — and, where relevant, which of its claims have since become false — is recorded in #13, so the analysis does not depend on this branch surviving.

This PR remains open and unrebased. Its CI status is from 2026-07-06 against a 2026-07-01 merge base and should not be read as current.

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.

1 participant