Skip to content

fix: stop declaring output fields the sanitiser strips - #571

Merged
scottlovegrove merged 1 commit into
mainfrom
fix/20842-workspace-insights-null-output
Aug 12, 2026
Merged

fix: stop declaring output fields the sanitiser strips#571
scottlovegrove merged 1 commit into
mainfrom
fix/20842-workspace-insights-null-output

Conversation

@scottlovegrove

Copy link
Copy Markdown
Collaborator

Pull Request

Closes https://github.com/Doist/Issues/issues/20842. Replaces #570, which fixed the symptom by switching off removeNullFields for every tool.

Short description

structuredContent is sanitised with removeNullFields on its way out, so a null can never reach a client. Four tools nonetheless declared output fields as required-and-nullable and emitted null for them, and the MCP SDK validates the sanitised payload against the declared outputSchema — so the field it just stripped fails the whole call with Output validation error. That is the crash Automations hit on get-workspace-insights.

folderId is a step worse than a schema mismatch. The API removed folder_id from the workspace insights endpoint in Doist/todoist#27366, and the SDK made folderId optional in Doist/todoist-sdk-typescript#655 to stop the parse failures that followed. Nothing can populate it again, so it is gone from the tool rather than declared optional — the tool takes no folder argument either.

The same defect was latent in three more places: get-project-activity-stats emitted a null weekItems on every call that did not ask for weekly counts (the default), get-project-health emitted nulls for any project without a description or completion data, and get-overview's inbox was nullable. All are now .optional() and omitted.

removeNullFields stays. It saves tokens on every response and it was doing exactly what it says; the tools were declaring a contract it was always going to break.

Since this is the third time the class has landed (#379 fixed it for get-overview's parentId/folderId), src/tools/output-schema-nullability.test.ts now walks every registered tool's output schema and rejects any nullable field. It caught get-overview's inbox on its first run.

PR Checklist

Feel free to leave unchecked or remove the lines that are not applicable.

  • Added tests for bugs / new features
  • Updated docs (README, etc.)
  • New tools added to getMcpServer AND exported in src/index.ts.

structuredContent is sanitised with removeNullFields before it leaves the
server, so a null can never reach a client. Four tools nonetheless
declared output fields as required-and-nullable and emitted null for
them, so the MCP SDK's validation of the sanitised payload against the
declared outputSchema failed the entire call with "Output validation
error".

get-workspace-insights was the reported case, and it is worse than a
schema mismatch: the API removed folder_id from the workspace insights
endpoint in Doist/Todoist#27366, and the SDK made folderId optional to
stop parse failures. The field can never be populated again, so it is
gone from the tool rather than declared optional.

The same defect was latent elsewhere: get-project-activity-stats emitted
a null weekItems on every call that did not ask for weekly counts, and
get-project-health emitted nulls for a project without a description or
completion data. Those fields, and get-overview's inbox, are now
optional and omitted.

removeNullFields is left in place. It saves tokens on every response,
and the tools were making a promise it was always going to break.

A registry-wide test now rejects any nullable field in a tool's output
schema, since this is the third time the class has landed.

Closes Doist/Issues#20842

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@doistbot doistbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR updates fix: stop declaring output fields the sanitiser strips. No issues were flagged in the reviewed diff.

Share FeedbackReview Logs

@scottlovegrove scottlovegrove added the Show PR is shipped with an async review label Aug 12, 2026
@scottlovegrove
scottlovegrove merged commit 6e1d285 into main Aug 12, 2026
6 checks passed
@scottlovegrove
scottlovegrove deleted the fix/20842-workspace-insights-null-output branch August 12, 2026 11:56
doist-release-bot Bot added a commit that referenced this pull request Aug 12, 2026
## [12.5.5](v12.5.4...v12.5.5) (2026-08-12)

### Bug Fixes

* stop declaring output fields the sanitiser strips ([#571](#571)) ([6e1d285](6e1d285))
@doist-release-bot

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 12.5.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@nvignola nvignola 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.

LGTM ✅

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

Labels

released Show PR is shipped with an async review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants