Skip to content

fix: preserve nullable fields in MCP tool output - #570

Closed
doistbot wants to merge 1 commit into
mainfrom
doistbot/auto-fix/issue-20842-1786531950
Closed

fix: preserve nullable fields in MCP tool output#570
doistbot wants to merge 1 commit into
mainfrom
doistbot/auto-fix/issue-20842-1786531950

Conversation

@doistbot

Copy link
Copy Markdown
Member

Pull Request

Closes https://github.com/Doist/Issues/issues/20842

Short description

Preserves nullable structured-output fields through MCP serialization so unscoped workspace insights responses retain folderId: null. Added a regression test covering both structured content and legacy JSON output.

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.

Automated fix generated by Doistbot based on triage analysis.
Issue: #20842
@doistbot

Copy link
Copy Markdown
Member Author

CI failed for this fix attempt. Failing checks:

Marking this for human review.

@scottlovegrove

Copy link
Copy Markdown
Collaborator

@doistbot I don't think the solution here is to just stop removing null fields. I think the solution here is to update the schema (or whatever is validating) to accept a missing folderId field.

@doistbot

Copy link
Copy Markdown
Member Author

I agree the schema should be the source of truth, but get-workspace-insights currently declares folderId as z.string().nullable()—not optional—and execute() always returns the key with null when there’s no folder. Removing that field therefore changes a valid string | null response into one that no longer matches its declared output schema. The same applies to the nullable health and progress fields in ProjectInsightSchema.

Making folderId optional would avoid this one validation error, but it would also change the output contract from “present and possibly null” to “possibly absent,” and we’d need to make every other nullable field compatible with the recursive sanitizer as well. Preserving the nulls in both structuredContent and the legacy JSON representation keeps the runtime output aligned with the existing schemas rather than weakening those schemas.

@scottlovegrove scottlovegrove self-assigned this Aug 12, 2026
@scottlovegrove scottlovegrove changed the title fix: preserve nullable fields in MCP tool output (Doist/Issues#20842) fix: preserve nullable fields in MCP tool output Aug 12, 2026
@scottlovegrove

Copy link
Copy Markdown
Collaborator

Superseded by #571. The field is not just failing validation — the API removed folder_id from the workspace insights endpoint in Doist/todoist#27366, so nothing can populate it. #571 drops it from the tool, gives the three other tools with the same required-and-nullable output fields the same treatment, keeps removeNullFields, and adds a registry-wide test that rejects nullable output fields.

@scottlovegrove
scottlovegrove deleted the doistbot/auto-fix/issue-20842-1786531950 branch August 12, 2026 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants