Skip to content

fix(mcp): publish satisfiable inputSchemas for free-form JSON fields - #375

Open
syf2211 wants to merge 1 commit into
firecrawl:mainfrom
syf2211:fix/json-options-schema-inputschema
Open

fix(mcp): publish satisfiable inputSchemas for free-form JSON fields#375
syf2211 wants to merge 1 commit into
firecrawl:mainfrom
syf2211:fix/json-options-schema-inputschema

Conversation

@syf2211

@syf2211 syf2211 commented Aug 19, 2026

Copy link
Copy Markdown

Summary

Replace z.record() tool-parameter schemas with z.json()-based helpers so fastmcp's strictJsonSchema no longer publishes unsatisfiable object schemas (propertyNames + additionalProperties: false with no properties).

Motivation

firecrawl_scrape's published jsonOptions.schema rejected every legitimate JSON Schema document in MCP clients that validate tool arguments against tools/list inputSchemas before forwarding calls. The server accepted these calls at runtime, but schema-validating gateways saw 100% failure for JSON extraction with a schema.

Fixes #373

Changes

  • Add src/mcp-json-schemas.ts with shared helpers (mcpJsonObject, mcpStringMapOptional, etc.)
  • Use helpers for jsonOptions.schema, agent schema, feedback metadata, crawl webhookHeaders, and monitor body fields
  • Add regression test asserting scrape/agent schema fields are satisfiable via stdio tools/list

Tests

  • npm run build — pass
  • npm run lint — pass
  • npm test — 69/72 pass; 3 pre-existing flaky smoke/OAuth tests fail in this environment (unrelated to this change)
  • node --test tests/mcp-input-schema.test.mjs — pass

Notes

Runtime validation behavior is preserved: object-only refinements still reject arrays/null for body/metadata/schema fields, and webhookHeaders still requires string values at execution time.


Summary by cubic

Publishes satisfiable MCP inputSchemas for free‑form JSON parameters so schema‑validating clients accept real payloads. Previously z.record() generated unsatisfiable object schemas under fastmcp’s strictJsonSchema; now z.json()-based helpers describe permissive JSON objects.

  • Replace z.record() with helpers from src/mcp-json-schemas.ts (mcpJsonObject, mcpJsonObjectOptional, mcpStringMapOptional, mcpJsonSchemaDocumentOptional).
  • Affects: firecrawl_scrape jsonOptions.schema, agent schema, feedback metadata, crawl webhookHeaders, and monitor body.
  • Add tests/mcp-input-schema.test.mjs to assert tools/list publishes satisfiable schemas via stdio.
  • No caller changes required; runtime validation is unchanged (object-only fields still reject arrays/null; webhookHeaders still requires string values).

Written for commit a46eab0. Summary will update on new commits.

Review in cubic

Replace z.record() with z.json()-based helpers so fastmcp's strictJsonSchema
does not emit unsatisfiable object schemas (propertyNames + additionalProperties:
false with no properties). Fixes jsonOptions.schema rejecting real JSON Schema
documents in schema-validating MCP clients.

Fixes firecrawl#373

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

2 issues found across 4 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/mcp-input-schema.test.mjs">

<violation number="1" location="tests/mcp-input-schema.test.mjs:16">
P2: `child.kill()` is called without awaiting the child's exit (and stderr is never drained), so on failure paths the spawned Node server can be left running and startup errors are hidden behind the 2.5 s timeout. The sibling smoke test appends `t.after(() => stopChild(child))`, which awaits exit and escalates to SIGKILL after a timeout. Mirror that to guarantee the spawned `dist/index.js` is reaped and to surface startup errors.</violation>

<violation number="2" location="tests/mcp-input-schema.test.mjs:46">
P2: This test relies on a fixed 2500 ms wall-clock sleep and then immediately calls `child.kill()`, rather than waiting for the actual `tools/list` response. On a slow or loaded CI runner, server startup plus schema generation for all ~15 tools can exceed 2.5 s, making the test flaky (it throws 'tools/list response not found' and fails a healthy build). Send requests event-driven: await the initialize response, send `tools/list`, and resolve/kill only when the matching id===2 response arrives. This also matches the established pattern in `tests/mcp-smoke.test.mjs` (`StdioMcpClient.request` awaits each response and `stopChild` escalates SIGTERM→SIGKILL), instead of hand-rolling framing and timing.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

}

async function listToolsViaStdio() {
const child = spawn('node', ['dist/index.js'], {

@cubic-dev-ai cubic-dev-ai Bot Aug 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: child.kill() is called without awaiting the child's exit (and stderr is never drained), so on failure paths the spawned Node server can be left running and startup errors are hidden behind the 2.5 s timeout. The sibling smoke test appends t.after(() => stopChild(child)), which awaits exit and escalates to SIGKILL after a timeout. Mirror that to guarantee the spawned dist/index.js is reaped and to surface startup errors.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/mcp-input-schema.test.mjs, line 16:

<comment>`child.kill()` is called without awaiting the child's exit (and stderr is never drained), so on failure paths the spawned Node server can be left running and startup errors are hidden behind the 2.5 s timeout. The sibling smoke test appends `t.after(() => stopChild(child))`, which awaits exit and escalates to SIGKILL after a timeout. Mirror that to guarantee the spawned `dist/index.js` is reaped and to surface startup errors.</comment>

<file context>
@@ -0,0 +1,80 @@
+}
+
+async function listToolsViaStdio() {
+  const child = spawn('node', ['dist/index.js'], {
+    env: { ...process.env, FIRECRAWL_API_KEY: 'fc-test' },
+    stdio: ['pipe', 'pipe', 'pipe'],
</file context>
Fix with cubic

child.stdin.write(`${JSON.stringify(message)}\n`);
}

await delay(2500);

@cubic-dev-ai cubic-dev-ai Bot Aug 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: This test relies on a fixed 2500 ms wall-clock sleep and then immediately calls child.kill(), rather than waiting for the actual tools/list response. On a slow or loaded CI runner, server startup plus schema generation for all ~15 tools can exceed 2.5 s, making the test flaky (it throws 'tools/list response not found' and fails a healthy build). Send requests event-driven: await the initialize response, send tools/list, and resolve/kill only when the matching id===2 response arrives. This also matches the established pattern in tests/mcp-smoke.test.mjs (StdioMcpClient.request awaits each response and stopChild escalates SIGTERM→SIGKILL), instead of hand-rolling framing and timing.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/mcp-input-schema.test.mjs, line 46:

<comment>This test relies on a fixed 2500 ms wall-clock sleep and then immediately calls `child.kill()`, rather than waiting for the actual `tools/list` response. On a slow or loaded CI runner, server startup plus schema generation for all ~15 tools can exceed 2.5 s, making the test flaky (it throws 'tools/list response not found' and fails a healthy build). Send requests event-driven: await the initialize response, send `tools/list`, and resolve/kill only when the matching id===2 response arrives. This also matches the established pattern in `tests/mcp-smoke.test.mjs` (`StdioMcpClient.request` awaits each response and `stopChild` escalates SIGTERM→SIGKILL), instead of hand-rolling framing and timing.</comment>

<file context>
@@ -0,0 +1,80 @@
+    child.stdin.write(`${JSON.stringify(message)}\n`);
+  }
+
+  await delay(2500);
+  child.kill();
+
</file context>
Fix with cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant