Repository navigation
Declare an outputSchema on every MCP tool - #431
Conversation
The ChatGPT app-submission scan flagged that no tool declares an
outputSchema, which OpenAI recommends so a model knows the shape of a
result before it calls the tool. None of the 29 registered tools had one.
Each tool now declares a schema and returns structured content alongside
its text block, as MCP requires of a tool that declares one. The text
block is unchanged: `structuredText` and `structuredCompact` serialize
exactly as `asText` and `compactText` did, so an existing client reads
the same string it read before. The tools that render prose — the
research and developer searches — keep that prose as their text and
carry the underlying records as structured content.
Two fastmcp behaviours shape the schemas, and src/tool-output.ts
documents both. fastmcp advertises a schema through `strictJsonSchema`,
which sets `additionalProperties: false`, so a schema has to name each
top-level key a tool can return; and it validates structured content
against the schema and keeps the parsed value, so an unnamed key is
dropped from the structured payload while the text block keeps the whole
response. Values that pass through from the Firecrawl API are therefore
left unconstrained (`z.unknown()`, advertised as `{}`) and only fields
whose scalar type is part of the API contract are typed — a wrong guess
would fail validation and turn a working call into an error.
The new test asserts every listed tool declares an object schema with at
least one property, and that a call returns structured content matching
its unchanged text.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K8seGYuqrxhiMnzAJt97su
There was a problem hiding this comment.
1 issue found across 7 files
Confidence score: 4/5
- In
src/tool-output.ts, the shared paper output schema allows canonical ID, title, abstract, and authors to be omitted even though research-paper responses expect them, weakening contract validation and potentially letting incomplete results reach consumers — make these fields required while preserving the category field behavior.
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="src/tool-output.ts">
<violation number="1" location="src/tool-output.ts:379">
P2: The shared paper output schema marks the canonical ID, title, abstract, and authors optional even though research-paper responses treat them as always present. Make those contract fields required while keeping `categories`, `createdDate`, and `updateDate` optional so clients can rely on the advertised paper shape.
(Based on your team's feedback about research paper response fields.)</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| paperId: z.string().optional().describe('Canonical paper identifier.'), | ||
| primaryId: z.string().optional().describe('Display identifier, ordered for citation and fetch use.'), | ||
| ids: z.unknown().optional().describe('Source identifiers by namespace, such as `arxiv`, `doi`, or `pmid`.'), | ||
| title: z.string().optional().describe('Paper title.'), | ||
| abstract: z.string().optional().describe('Paper abstract.'), | ||
| authors: z.unknown().optional().describe('Authors, as a comma-joined string or as `{name, affiliation}` entries.'), | ||
| categories: z.array(z.string()).optional().describe('Paper categories, such as `cs.LG`.'), | ||
| createdDate: z.string().optional().describe('Date the paper was first indexed or published.'), | ||
| updateDate: z.string().optional().describe('Date the paper was last updated.'), | ||
| }); |
There was a problem hiding this comment.
P2: The shared paper output schema marks the canonical ID, title, abstract, and authors optional even though research-paper responses treat them as always present. Make those contract fields required while keeping categories, createdDate, and updateDate optional so clients can rely on the advertised paper shape.
(Based on your team's feedback about research paper response fields.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tool-output.ts, line 379:
<comment>The shared paper output schema marks the canonical ID, title, abstract, and authors optional even though research-paper responses treat them as always present. Make those contract fields required while keeping `categories`, `createdDate`, and `updateDate` optional so clients can rely on the advertised paper shape.
(Based on your team's feedback about research paper response fields.) </comment>
<file context>
@@ -0,0 +1,453 @@
+ * the advertised schema and validation.
+ */
+const paperSchema = z.looseObject({
+ paperId: z.string().optional().describe('Canonical paper identifier.'),
+ primaryId: z.string().optional().describe('Display identifier, ordered for citation and fetch use.'),
+ ids: z.unknown().optional().describe('Source identifiers by namespace, such as `arxiv`, `doi`, or `pmid`.'),
</file context>
| paperId: z.string().optional().describe('Canonical paper identifier.'), | |
| primaryId: z.string().optional().describe('Display identifier, ordered for citation and fetch use.'), | |
| ids: z.unknown().optional().describe('Source identifiers by namespace, such as `arxiv`, `doi`, or `pmid`.'), | |
| title: z.string().optional().describe('Paper title.'), | |
| abstract: z.string().optional().describe('Paper abstract.'), | |
| authors: z.unknown().optional().describe('Authors, as a comma-joined string or as `{name, affiliation}` entries.'), | |
| categories: z.array(z.string()).optional().describe('Paper categories, such as `cs.LG`.'), | |
| createdDate: z.string().optional().describe('Date the paper was first indexed or published.'), | |
| updateDate: z.string().optional().describe('Date the paper was last updated.'), | |
| }); | |
| paperId: z.string().describe('Canonical paper identifier.'), | |
| primaryId: z.string().describe('Display identifier, ordered for citation and fetch use.'), | |
| ids: z.unknown().optional().describe('Source identifiers by namespace, such as `arxiv`, `doi`, or `pmid`.'), | |
| title: z.string().describe('Paper title.'), | |
| abstract: z.string().describe('Paper abstract.'), | |
| authors: z.unknown().describe('Authors, as a comma-joined string or as `{name, affiliation}` entries.'), | |
| categories: z.array(z.string()).optional().describe('Paper categories, such as `cs.LG`.'), | |
| createdDate: z.string().optional().describe('Date the paper was first indexed or published.'), | |
| updateDate: z.string().optional().describe('Date the paper was last updated.'), | |
| }); |
There was a problem hiding this comment.
Not taking this one, and I think the suggestion would break the tool.
The only consumers of these responses treat all four as absent-able: displayId falls back to 'missing-primary-id' when primaryId is unset, fmtHits renders r.title ?? '(untitled)' and (r.abstract || '(no abstract)'), and fmtAuthors returns null when authors is missing. That defensive handling is the codebase's own statement that the fields are not guaranteed.
Requiring them in the output schema would convert that tolerated absence into a hard failure: fastmcp validates structuredContent against the schema and raises a UserError when validation fails, so a single indexed paper with no abstract would turn a working firecrawl_research_search_papers call into an error instead of a result. Everything typed in this file is optional and nullable for exactly that reason.
If the research API does guarantee these four, the fix belongs in fmtHits/displayId first and the schema can follow.
Generated by Claude Code
Conflict in src/index.ts: main moved firecrawl_find_tools into a shared `findToolsTool` object and gave `alexandriaFeedbackAvailable` a session argument, while this branch changed the same return from `compactText` to `structuredCompact`. Kept both. The search-surface copies main added spread the full-surface tool, so they inherit its outputSchema. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K8seGYuqrxhiMnzAJt97su
Three findings from the cubic review. `parseOutputSchema` named fields hosted parse does not return (`uploadUrl`, `uploadRef`, `instructions`) and missed the ones it does, so phase one's `upload`, `nextToolCall` and `notes` were dropped from structured content and a client reading only that could not finish the upload flow. The schema now declares them, `upload` field by field, and the hosted-parse test asserts the structured result matches the text payload whole. Every declared scalar is now optional and nullable, through `str`, `num` and `bool` helpers. `next` and `expiresAt` were already nullable for this reason; `exitCode` on an interact call with no code, and `completed` and `total` on a freshly queued crawl, are the same case, and a value the schema rejects fails validation and turns a working call into an error. Declaring a scalar narrows the type without narrowing what is accepted. The output-schema test compared the text block against the structured content beside it, which only showed the two agreed. It now pins both against the fixture payload, so a change to the serialization is caught. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K8seGYuqrxhiMnzAJt97su
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
|
AX sim (EXP-058): no behaviour change for Codex, but a few fields now vanish for it. Codex (0.147.0) sends
Same ten queries executed in both; the token and agent-use gap traces to one query that timed out on prod and completed here. Keys the schemas drop from what Codex now sees (checked on every result short enough to compare in full):
No tool input uses these today, so nothing broke. Worth naming them in the schemas before merge so a future thread-continuation input doesn't silently fail on Codex. Findings: firecrawl/agent-experience#848. 🤖 Generated with Claude Code |
…hemas
Codex sends structuredContent to the model in place of the text block, so a
top-level key an output schema does not name disappears for Codex. The AX run
on this branch (agent-experience EXP-058) found firecrawl_agent dropping threadId
and threadTurn, firecrawl_agent_status dropping expiresAt, model, mode, threadId
and threadTurn, and firecrawl_feedback reducing to {success: true}. Name those
fields, and pin them in the output-schema smoke test with fake agent routes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Pushed 57fd282: the output schemas now name the fields EXP-058 found missing on Codex.
The output-schema smoke test gains fake 🤖 Generated with Claude Code |
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
…urned id Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
The ChatGPT app-submission scan flagged that none of our tools declare an
outputSchema, which OpenAI recommends so a model knows the shape of a result before it calls the tool. That was accurate: no tool declared one.Every registered tool now declares a schema and returns structured content alongside its text block, as MCP requires of a tool that declares one.
What changes for a client
Nothing it already reads.
structuredTextandstructuredCompactserialize exactly asasTextandcompactTextdid, so the text block is byte-for-byte what it was — same pretty-printing, same key order. New clients additionally getstructuredContent. The tools that render prose (firecrawl_research_*,firecrawl_developer_search) keep that prose as their text and carry the underlying records —results,paper,passages— as structured content, which is the part this buys them.How the schemas are written
Two fastmcp behaviours drive the shape, and
src/tool-output.tsdocuments both at the top:strictJsonSchema, which setsadditionalProperties: falseon every object it emits. A schema therefore has to name each top-level key a tool can return — az.looseObjector az.recorddoes not survive the conversion.structuredContentagainst the schema and keeps the parsed value, so a key the schema does not name is dropped from the structured payload. The text block still carries the whole response.So each schema names the documented keys and leaves pass-through values from the Firecrawl API unconstrained (
z.unknown(), advertised as{}, accepts any shape). Only fields whose scalar type is part of the API contract are typed, and every declared scalar is optional and nullable through thestr/num/boolhelpers: a value the schema rejects would fail validation at runtime and turn a working call into an error, and an API that reports an absent value asnullis the common case.The trade-off worth knowing: if the API grows a top-level field we have not named, it appears in the text block but not in
structuredContentuntil the schema names it too.Tests
tests/mcp-smoke.test.mjsgains a case asserting that every listed tool declares an object schema with at least one property, and that a call returns structured content matching its unchanged text. It fails today without the rest of this change, so the gap cannot come back silently.npm testpasses (115),tsc --noEmitis clean, andnpm run lintreports only the three pre-existingsrc/alexandria.tserrors that are also onmain.Not in this PR
The scan's second finding, the
chatgpt-app-submission.jsonannotation drift. That file is on no branch, and the one flag that actually disagrees with the server isfirecrawl_scrape'sreadOnlyHint(#405 flipped the server on Sep 21, after the file was written) — worth settling separately, since it turns on whether scrape should be non-read-only for every scrape or only in safe mode.🤖 Generated with Claude Code
https://claude.ai/code/session_01K8seGYuqrxhiMnzAJt97su
Summary by cubic
Declares an
outputSchemaon every registered MCP tool, closing the gap the ChatGPT app-submission scan flagged: none of the 29 tools declared one, so a model couldn't know a result's shape before calling.Existing clients see no change: the text block stays byte-for-byte identical, since
structuredTextandstructuredCompactserialize exactly asasTextandcompactTextdid. New clients additionally getstructuredContent, and the prose-rendering research and developer tools carry their underlying records as structured content.How the schemas work
strictJsonSchema, which setsadditionalProperties: false, so each schema names the top-level keys a tool can return.z.unknown()); only fields with a typed API contract are typed, and every field is optional and nullable so a wrong guess can't break a working call.upload,nextToolCall, andnotesare named so a client reading only structured content can still finish the upload flow.structuredContentin place of the text block, so an unnamed key vanishes for it.structuredContentuntil the schema names it.Tests
tests/mcp-smoke.test.mjsasserts every listed tool declares an object schema with at least one property and returns structured content matching its unchanged text (both pinned against fixture payloads); it fails without this change.Written for commit 844bdcd. Summary will update on new commits.