Skip to content

fix(mcp): coerce string-encoded numeric params so MCP clients can pass either form#63

Merged
LakshmanTurlapati merged 2 commits into
mainfrom
fix/fsb-mcp-numeric-param-coercion
May 17, 2026
Merged

fix(mcp): coerce string-encoded numeric params so MCP clients can pass either form#63
LakshmanTurlapati merged 2 commits into
mainfrom
fix/fsb-mcp-numeric-param-coercion

Conversation

@LakshmanTurlapati

Copy link
Copy Markdown
Collaborator

Summary

Some MCP clients (observed: Claude Code) serialize integer tool params as JSON-encoded strings. The fsb-mcp-server's Zod input schemas declared bare z.number(), so every such call was rejected with Expected number, received string. This PR coerces numeric params at both the translator (jsonSchemaToZod) and the 7 hand-rolled Zod sites so the server accepts the value whether it arrives as 123 or "123".

Live repro that prompted this

During verification of PR #62 (k=2 anonymity-floor deploy), an FSB visual-session attempt failed three consecutive times:

mcp__fsb__switch_tab({tabId: 695936610})  -> Expected number, received string
mcp__fsb__close_tab({tab_id: 695936539})  -> Expected number, received string
mcp__fsb__read_page({tab_id: 695936615})  -> Expected number, received string

Root cause: the MCP tool-call transport (model -> client SDK -> server) was emitting integer params as JSON strings. The server's z.number() rejected them. The MCP SDK does NOT pre-validate JSON Schema (verified at @modelcontextprotocol/sdk/dist/esm/server/mcp.js:166-181), so Zod is the only gate — fixing Zod fixes the wire.

Design

Single fix point at mcp/src/tools/schema-bridge.ts (the jsonSchemaToZod translator). Two cases split:

case 'number':  return z.preprocess(emptyStringToNaN, z.coerce.number().finite());
case 'integer': return z.preprocess(emptyStringToNaN, z.coerce.number().int().finite());
  • z.coerce.number() accepts both 123 and "123" and emits a number.
  • .finite() rejects Number("abc") -> NaN.
  • .int() rejects "123.45" for integer schemas.
  • z.preprocess(emptyStringToNaN, ...) maps "" to NaN so Number("") -> 0 doesn't silently slip through.

Every translator-driven tool (the bulk of the manual + read-only surface) auto-fixes. The translator change is the load-bearing one.

7 hand-rolled sites that build their Zod schema directly (bypassing the translator) get the same swap:

  • mcp/src/tools/agents.ts:35 — back.tab_id
  • mcp/src/tools/vault.ts:60 — fill_credential.tab_id
  • mcp/src/tools/vault.ts:110 — use_payment_method.tab_id
  • mcp/src/tools/visual-session.ts:50 — start_visual_session.tab_id
  • mcp/src/tools/observability.ts:27 — limit
  • mcp/src/tools/observability.ts:68 — count
  • mcp/src/tools/observability.ts:91 — topN

Each keeps its existing refinement chain (.int().positive().finite().optional() etc).

Server-side only

This is purely a server-side widening — the bridge response shape still serializes IDs as numbers. Clients pass whichever form they prefer. No JSON-Schema-source edits in tool-definitions.cjs (the byte-identity parity test stays green).

Tests

New tests/mcp-numeric-param-coercion.test.js — 17 assertions across 11 sections:

  • A1: 'number' accepts 123 AND "123" (both emit number 123)
  • A2: 'integer' accepts 123 AND "123"
  • A3: 'integer' REJECTS "123.45" (fractional string)
  • A4: 'integer' accepts negative ints (-5, "-5")
  • A5: 'number' REJECTS "abc" (NaN guard via .finite())
  • A6: 'integer' REJECTS "" (preprocess "" -> NaN)
  • A7: 'number' REJECTS ""
  • A8: optional field still optional (undefined passes)
  • A9: object schema composition unaffected
  • A10: each of the 7 hand-rolled sites accepts string-encoded tab_id (7 sub-asserts)
  • A11: source-grep regression guard — zero z.number( literals remain in mcp/src/tools/ (commented-out scheduler scaffolding ignored)

Validation gates

  • node scripts/validate-extension.mjs -> OK
  • npm --prefix mcp run build -> exit 0 (tsc clean, tool-definitions.cjs synced)
  • npm --prefix showcase/angular run build -> exit 0
  • npm test -> exit 0 (17/17 in new test; tool-definitions-parity still green)

Workflow

TDD pattern — two atomic commits on the branch:

  • d30169ce — RED: failing test (13/17 PASS confirms only the cases the fix targets fail)
  • fae2aa01 — GREEN: translator + 7 hand-rolled sites + mcp rebuild -> 17/17 PASS

Operational follow-up

None. After merge + deploy, the FSB-as-MCP-tool surface accepts numeric params from any compliant MCP client whether they serialize integers as numbers or as strings.

🤖 Generated with Claude Code

…oercion

RED phase. Adds tests/mcp-numeric-param-coercion.test.js (17 assertions
across A1..A11) wired into the root npm test chain after
tool-definitions-parity.test.js.

Failures observed against the current source tree (translator + 7
hand-rolled sites still use bare z.number()):

  FAIL A2 -- number schema rejects string "123"
  FAIL A4 -- integer schema rejects string "123"
  FAIL A6 -- integer schema rejects string "-42"
  FAIL A11 -- source-grep guard caught 8 stray z.number() sites:
    schema-bridge.ts:95, agents.ts:35, vault.ts:60, vault.ts:110,
    visual-session.ts:50, observability.ts:{27,68,91}

A8 empirical: bare z.coerce.number().int().finite() ACCEPTS "" as 0
(Number("")===0). Test asserts rejection -- Task 2 must add a
z.preprocess wrapper that maps "" -> NaN before .coerce.

A10.1..A10.7 already PASS because they construct the post-fix Zod
literal in-test; the source-grep guard (A11) is what enforces the
actual source-tree matches.

Test exits 1 with 13/17 PASS -- exactly the expected RED state.
…16-mq4)

GREEN phase. Claude Code's MCP transport stringifies JSON numbers, so
every numeric tool call (switch_tab tabId, click_at x/y, vault tab_id,
agents back tab_id, visual-session tab_id, observability limit/count/topN,
...) was failing with a Zod invalid_type error.

Fix surface (8 sites total):

  Translator (covers ~35 numeric fields via TOOL_REGISTRY):
    - mcp/src/tools/schema-bridge.ts:93..96 -- split 'number' / 'integer'
      cases, wrap each in z.preprocess that maps "" -> NaN, then
      z.coerce.number().finite() ('integer' adds .int()). The preprocess
      step is required because bare z.coerce.number().int().finite()
      ACCEPTS "" as 0 (Number("") === 0) -- empirically verified against
      zod 3.25.

  Hand-rolled sites (bypass the translator):
    - mcp/src/tools/agents.ts:35           back.tab_id
    - mcp/src/tools/vault.ts:60            fill_credential.tab_id
    - mcp/src/tools/vault.ts:110           use_payment_method.tab_id
    - mcp/src/tools/visual-session.ts:50   start_visual_session.tab_id
    - mcp/src/tools/observability.ts:27    list_sessions.limit
    - mcp/src/tools/observability.ts:68    get_logs.count
    - mcp/src/tools/observability.ts:91    search_memory.topN

  All hand-rolled sites switched z.number().*.optional() ->
  z.coerce.number().int().*.finite().optional() with .positive() for tab
  ids and topN, .nonnegative() for the two count fields.

Gates:
  - tests/mcp-numeric-param-coercion.test.js: 17/17 PASS
  - node scripts/validate-extension.mjs: OK
  - npm --prefix mcp run build: exit 0
  - npm --prefix showcase/angular run build: exit 0
  - npm test (full root suite): exit 0 -- no regressions, including
    tool-definitions-parity (JSON Schema files untouched)

Source-grep regression guard (A11) now passes: zero stray z.number() in
mcp/src/tools/ (only z.coerce.number() remains).

@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: fae2aa010f

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

Comment on lines +97 to +100
zodType = z.preprocess(
(v) => (v === '' ? NaN : v),
z.coerce.number().finite(),
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Restrict numeric coercion to numeric-like inputs

Using z.coerce.number() here accepts many non-numeric JSON values via JS coercion (null/false/[] become 0, true becomes 1), so malformed MCP payloads now pass validation instead of being rejected. Because jsonSchemaToZod() is used for all registry-backed tools (mcp/src/tools/manual.ts:217 and mcp/src/tools/read-only.ts:103), this can change real tool behavior (for example, required coordinate fields can execute as 0 instead of failing fast). The coercion should be gated to actual numbers and numeric strings only.

Useful? React with 👍 / 👎.

'list_sessions',
'List all past FSB automation sessions with summary info (task, status, duration, action count, cost). Use to find session IDs for deeper inspection with get_session_detail. Related: get_session_detail (inspect a specific session), get_logs (get raw logs).',
{ limit: z.number().optional().describe('Max sessions to return (default: all, max 50)') },
{ limit: z.coerce.number().int().nonnegative().finite().optional().describe('Max sessions to return (default: all, max 50)') },

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 Reject empty strings in nonnegative observability params

These schemas use z.coerce.number().int().nonnegative() without the empty-string guard that was added in the schema bridge, so "" is coerced to 0 and silently treated as a valid limit/count. That changes request semantics from “unset/invalid” to “explicit zero” for list_sessions/get_logs, which can yield empty results instead of the documented defaults.

Useful? React with 👍 / 👎.

Comment thread mcp/src/tools/agents.ts
'back',
'Navigate one step back in browser history on the agent\'s active tab. Single-step only (no back(n)); no companion forward tool in v0.9.60. Agent-scoped: targets the agent\'s active tab unless tab_id is supplied. Ownership-enforced via the Phase 240 dispatch gate -- cross-agent calls reject with TAB_NOT_OWNED. Returns a structured result of shape { status, resultingUrl, historyDepth } where status is one of: ok (back navigation settled cleanly), no_history (history depth <= 1, nothing to go back to), cross_origin (target page is on a different origin from the source), bf_cache (back was served from the back/forward cache and the pageshow listener observed event.persisted=true), fragment_only (only the URL fragment changed via history navigation). Background-tab compatible: does not steal focus or call chrome.tabs.update({active: true}). Multi-agent contract: agent_id is FSB-issued and required (the server captures it via agent:register on first tool call -- callers do not provide it). tab_id is agent-scoped: only tabs owned by the calling agent can be addressed. The concurrency cap is configurable (default 8, range 1-64); the (N+1)th agent claim is rejected with AGENT_CAP_REACHED { cap, active }. Ownership enforcement: cross-agent calls reject with TAB_NOT_OWNED; incognito tabs reject with TAB_INCOGNITO_NOT_SUPPORTED; cross-window tabs reject with TAB_OUT_OF_SCOPE. Tool-specific typed error codes returnable by this tool: NO_BACK_HISTORY, CROSS_ORIGIN, BF_CACHE, FRAGMENT_ONLY. RETURNS change_report: the response includes a `change_report` field with a compact diff of what the back navigation mutated (URL, dialogs_opened, nodes_added, nodes_removed, attrs_changed, inputs_changed, focus_shift). Cross-origin transitions emit a URL-only report with `cross_origin:true`. Use this to learn the consequence without calling read_page next; if `change_report.truncated:true` is set, call read_page for the full state.',
{ tab_id: z.number().int().positive().optional().describe('Target tab in agent pool. If omitted, defaults to the agent\'s active tab.') },
{ tab_id: z.coerce.number().int().positive().finite().optional().describe('Target tab in agent pool. If omitted, defaults to the agent\'s active tab.') },

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 Prevent boolean coercion for tab_id in hand-rolled schemas

Switching this field to z.coerce.number() means non-numeric booleans now validate (true becomes 1), so an invalid tab_id can silently target tab 1 instead of being rejected. This affects action routing for tab-scoped tools and is especially risky when client serialization bugs send booleans for optional fields; previously z.number() failed fast.

Useful? React with 👍 / 👎.

{
sessionId: z.string().optional().describe('If provided, get logs only for this session. Otherwise get most recent logs.'),
count: z.number().optional().describe('Number of recent logs to return (default: 50, max: 200). Ignored when sessionId is provided.'),
count: z.coerce.number().int().nonnegative().finite().optional().describe('Number of recent logs to return (default: 50, max: 200). Ignored when sessionId is provided.'),

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 Reject boolean values for log count coercion

This count schema now accepts booleans because z.coerce.number() maps false to 0 and true to 1, so malformed MCP payloads can pass validation and alter paging semantics instead of returning a type error. Since this tool documents count as a numeric limit, accepting booleans creates silent behavior changes that are hard to diagnose.

Useful? React with 👍 / 👎.

@LakshmanTurlapati LakshmanTurlapati merged commit 9c25405 into main May 17, 2026
8 checks passed
@LakshmanTurlapati LakshmanTurlapati deleted the fix/fsb-mcp-numeric-param-coercion branch May 17, 2026 02:57
LakshmanTurlapati added a commit that referenced this pull request May 17, 2026
…coercion (#64)

Bumps `mcp/package.json`, `mcp/server.json` (top-level + packages[0]),
`mcp/src/version.ts` (FSB_MCP_VERSION), the parity test's canonical
target, and the rebuilt `mcp/build/version.{js,d.ts}` artifacts from
0.9.1 to 0.9.2 in lockstep. Prepends a 0.9.2 entry to `mcp/CHANGELOG.md`
referencing PR #63 (commit `fae2aa01`).

Pure release plumbing -- the numeric-param coercion fix landed via
PR #63 on 2026-05-16, but npm-published 0.9.1 still rejects every
MCP client (e.g. Claude Code) that serializes integer params as
JSON strings with `Expected number, received string`. Bumping to
0.9.2 unblocks the next `npm publish fsb-mcp-server`.

No code logic changes, no protocol changes, no dependency bumps.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
LakshmanTurlapati added a commit that referenced this pull request May 18, 2026
* docs: start milestone v0.9.70 Showcase Dashboard Reliability (Streaming + Sync + Viewport)

Three target features: (1) diagnose-first dashboard DOM-streaming fix
continuing STREAM-07 attempt 2 of 5 from v0.9.69 Phase 276; (2) Sync-tab
remote-control restoration on full-selfbrowsing.com; (3) 16:10 desktop
viewport lock on the dashboard preview pane (mobile out of scope).

PROJECT.md "Current Milestone" rewritten to v0.9.70 with goals + target
features + key constraints + deferred candidates. Footer "Last updated"
bumped to 2026-05-16. "Last shipped" updated to reflect v0.9.69 SHIP
+ four post-milestone fixes (PR #57/59/61/62/63/64) and the published
artifacts (extension v0.9.67 zip, mcp-v0.9.2 npm, Fly auto-deploy).

STATE.md frontmatter reset (milestone v0.9.70, status in_progress,
progress 0%). Current Position set to "Not started (defining requirements)".
Active Milestone Risk Register replaced with Active Milestone
Carry-Forward (STREAM-07 attempt 2 + CWS click-through). Quick Tasks
Completed table extended with PRs #59 / #61 / #62 / #63 / #64. Pending
User-Gated Actions: v0.9.0 npm publish marked superseded by v0.9.2;
added v0.9.67 -> extension-v0.9.67 retag decision.

Phases dir cleared (8 v0.9.69 phase directories removed by
gsd-tools phases clear). Carries over: 999.1-mcp-tool-gaps,
INTEGRATION-CHECK.md, v0.9.63-INTEGRATION-CHECK.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(showcase): lock dashboard preview pane to 16:10 on desktop

Phase 280 VIEWPORT requirement: the live DOM preview frame in the
showcase dashboard had no fixed aspect ratio and stretched freely to
container height, producing inconsistent letterboxing and the bug
visible in milestone 0.9.70. Pin it to 16/10 on desktop, unset on
mobile (<=768px and <=480px) where vertical layout takes over, and
keep Maximized (100vh) and PiP layouts coherent (auto for maximized,
explicit 16/10 for PiP).

Adds tests/dashboard-preview-aspect-ratio.test.js — parses the SCSS
and asserts the rule lives in the right selectors (desktop, both
mobile media queries, Maximized, PiP). Wired into the root `npm test`
chain.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

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.

1 participant