Skip to content

llm tools presets#3541

Merged
harbournick merged 6 commits into
mainfrom
andrii/llm-tools-bundles
May 29, 2026
Merged

llm tools presets#3541
harbournick merged 6 commits into
mainfrom
andrii/llm-tools-bundles

Conversation

@andrii-harbour

@andrii-harbour andrii-harbour commented May 27, 2026

Copy link
Copy Markdown
Contributor

SD-3128

- Added validation for the MCP_PRESET environment variable in `server.ts` to ensure only supported presets are accepted at startup, preventing silent misconfigurations.
- Introduced a new preset registry in `presets.ts`, allowing for the management of LLM tool presets, with the initial implementation supporting only the 'legacy' preset.
- Updated the Node SDK to expose preset-related functions (`getPreset`, `listPresets`, `DEFAULT_PRESET`) for easier access to preset information.
- Created tests for preset validation and registry functionality, ensuring that unknown presets trigger appropriate errors and that the legacy preset behaves as expected.
- Added corresponding Python SDK support for the preset registry, mirroring the Node implementation for consistency across languages.
@andrii-harbour
andrii-harbour marked this pull request as ready for review May 29, 2026 00:16
@andrii-harbour
andrii-harbour requested a review from a team as a code owner May 29, 2026 00:16

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

No issues found across 15 files

Re-trigger cubic

@harbournick

Copy link
Copy Markdown
Collaborator

@andrii-harbour please investigate these:
Findings

  1. [Medium] Exported Node ToolCatalog lost useful structure
    File: packages/sdk/langs/node/src/presets.ts:51
    Evidence: ToolCatalog.tools is now unknown[]; on main it was ToolCatalogEntry[] with toolName, description, inputSchema, mutates, and operations.
    Impact: TypeScript consumers inspecting getToolCatalog().tools lose property access and can fail to compile even though runtime shape is unchanged.
    Recommended fix: preserve/export the structured catalog entry type, or make ToolCatalog generic with the legacy structured entry as the default.
    Source: Codex reviewer + parent investigation.

  2. [Medium] Malformed provider bundles now fail open as empty tool lists
    File: packages/sdk/langs/node/src/presets/legacy.ts:263, packages/sdk/langs/python/superdoc/tools_api.py:45, packages/sdk/langs/python/superdoc/presets/legacy.py:145
    Evidence: both preset implementations coerce a missing/non-array tools field to []; the previous public listTools / list_tools paths threw TOOLS_ASSET_INVALID.
    Impact: a broken generated/package artifact can look like a valid empty tool set, leaving agents with no tools instead of failing fast.
    Recommended fix: validate provider bundle shape in the preset loader or list wrappers, and add malformed-bundle coverage.
    Source: Codex reviewer + parent investigation.

  3. [Low] Python choose_tools() silently treats preset: "" as default
    File: packages/sdk/langs/python/superdoc/tools_api.py:80
    Evidence: input.get('preset') or DEFAULT_PRESET turns an explicit empty string into legacy; Node chooseTools({ preset: "" }) and Python get_tool_catalog("") both raise PRESET_NOT_FOUND.
    Impact: Python can hide preset misconfiguration and diverges from Node/MCP fail-fast behavior.
    Recommended fix: default only when the preset key is absent/None; pass "" through to get_preset(""), and add parity coverage.
    Source: Codex reviewer + Claude reviewer + parent investigation.

andrii-harbour and others added 3 commits May 30, 2026 00:48
Three review findings from PR 3541:

1. Restore structured ToolCatalog.tools type. The refactor narrowed the
   public catalog row to `unknown[]`, breaking TS consumers that read
   tools[i].toolName etc. Move ToolCatalogEntry + ToolCatalogOperation
   into presets.ts as public types and tighten the catalog signature.

2. Fail fast on malformed provider bundles. Node and Python preset
   loaders previously coerced a missing or non-array `tools` field to
   `[]`, hiding broken codegen output behind a silently empty tool
   surface. Restore the pre-presets TOOLS_ASSET_INVALID throw at the
   preset boundary.

3. Cross-lang parity for empty-string presets. Python choose_tools
   treated `{'preset': ''}` as legacy via `or DEFAULT_PRESET`; Node and
   MCP both raise PRESET_NOT_FOUND. Use an explicit None check so
   Python matches.

Tests added covering structural catalog access, empty-string preset
fail-fast, and cross-lang parity for the empty-string case.

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

@harbournick harbournick left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@harbournick
harbournick merged commit 4c51e5c into main May 29, 2026
41 checks passed
@harbournick
harbournick deleted the andrii/llm-tools-bundles branch May 29, 2026 23:32
@superdoc-bot

superdoc-bot Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

🎉 This PR is included in superdoc-sdk v1.15.0

@superdoc-bot

superdoc-bot Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

🎉 This PR is included in @superdoc-dev/mcp v0.11.0

The release is available on GitHub release

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