Conversation
…y-ai into feature/romka/tool-skill
arutkowski00
left a comment
arutkowski00
left a comment
There was a problem hiding this comment.
Solid addition. The mcp-tool.md guide is detailed and covers the full workflow clearly — the codegen reminder especially is the kind of thing that gets missed and causes confusing type errors. Main thing I'd flag is the duplication: CLAUDE.md has a condensed version of the same "Adding a New Platform API Tool" steps that mcp-tool.md already covers thoroughly. That's going to drift. Either have CLAUDE.md defer to the command (see /mcp-tool) or pick one as the canonical source. The README's new Development section only mentions the Cursor skill but not the Claude command, even though the Cursor SKILL.md is just a thin pointer to the Claude command anyway.
|
|
||
| ## Architecture Overview | ||
|
|
||
| All platform API tools live under `src/core/tools/platform-api-tools/`. Each tool extends `BaseMondayApiTool<InputSchema, Output>` from `base-monday-api-tool.ts`, which itself implements the `Tool` interface from `src/core/tool.ts`. |
There was a problem hiding this comment.
The type parameter name InputSchema here doesn't match the actual class definition, which uses Input extends ZodRawShape | undefined. Minor, but since this is a reference doc it'd be cleaner to match the real signature.
| abstract annotations: ToolAnnotations; | ||
| abstract getDescription(): string; | ||
| abstract getInputSchema(): Input; | ||
| protected abstract executeInternal(input): Promise<ToolOutputType<Output>>; |
There was a problem hiding this comment.
The real executeInternal signature has input? (optional), not input. Matches the base class: protected abstract executeInternal(input?: ToolInputType<Input>).
|
|
||
| protected async executeInternal( | ||
| input: ToolInputType<typeof myToolSchema> | ||
| ): Promise<ToolOutputType<never>> { |
There was a problem hiding this comment.
ToolOutputType<never> works fine since never is the default for the Output type parameter, but it reads as a bit cryptic to someone unfamiliar with the type. Worth adding a short comment like // 'never' = no typed metadata or just using ToolOutputType<Record<string, never>>.
| Tools are organized into three groups (see `src/core/tools/index.ts`): | ||
|
|
||
| - **`platform-api-tools/`** — GraphQL-based tools for boards, items, columns, workforms, docs, dashboards, search, etc. Registered via `allGraphqlApiTools`. | ||
| - **`monday-apps-tools/`** — REST-based tools for app management, features, versions, deployment, and storage. Registered via `allMondayAppsTools`. |
There was a problem hiding this comment.
This "Adding a New Platform API Tool" section is essentially a condensed version of what mcp-tool.md already covers in detail. Two sources for the same process will drift. Could simplify this to a one-liner pointing to /mcp-tool instead of maintaining parallel step lists.
|
|
||
| When modifying an existing tool's behavior, review and update `getDescription()` to reflect the new capabilities. The description is what LLMs see when selecting tools — stale descriptions lead to incorrect tool usage. | ||
|
|
||
| ### Version Bumping |
There was a problem hiding this comment.
"Document changes in CHANGELOG.md" — the CHANGELOG.md in this package is empty. If that's the intended convention going forward, fine, but worth noting that it's currently unpopulated.
|
|
||
| ## Development | ||
|
|
||
| A Cursor skill is available at `.cursor/skills/agent-toolkit-api-tools/SKILL.md` with detailed guidance on creating and modifying platform API tools, including the GraphQL codegen pipeline, tool registration, and version bumping. |
There was a problem hiding this comment.
This only mentions the Cursor skill but the Cursor SKILL.md is just a thin pointer to .claude/commands/mcp-tool.md. Might be worth referencing the Claude command too (or the Claude command as the primary, since Cursor just delegates to it).
2 participants
No description provided.