feat(tools): add MCP tool annotations (INS-306)#60
Closed
tonychang04 wants to merge 4 commits into
Closed
Conversation
ora's agent-readiness check flagged "No MCP server card found at /.well-known/mcp/server-card.json" for mcp.insforge.dev. Add an unauthenticated GET route to the Express HTTP server that returns the discovery document. The card exposes the five flat top-level fields ora's mcp-server-card check requires for full credit (a nested SEP-1649 card fails): name, description, version, serverUrl, tools[]. The tools array is a curated catalog (src/shared/tool-catalog.ts) whose descriptions are copied verbatim from the registerTool() calls and must stay in sync with the registered tools. Version is read from package.json at runtime so it never drifts. The route requires no OAuth or backend connection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WalkthroughAdds a static TOOL_CATALOG and capability hint annotations to tool registrations, updates integration tests to capture annotations, and exposes an unauthenticated MCP server-card endpoint that serves server metadata with a runtime-resolved version. ChangesServer Card Discovery
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/shared/tool-catalog.ts (1)
1-103: 🏗️ Heavy liftConsider automating the catalog generation to eliminate sync risk.
The static catalog approach correctly enables unauthenticated serving per the MCP server-card spec. However, the manual synchronization requirement creates a maintenance burden and drift risk if tools are added, removed, or their descriptions change in the source files.
Consider generating this catalog automatically at build time by parsing the
registerTool(...)calls or exporting tool metadata from the registration modules. This would eliminate the manual sync requirement and prevent stale documentation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shared/tool-catalog.ts` around lines 1 - 103, The TOOL_CATALOG is manually duplicated and risks drift; replace the hand-maintained TOOL_CATALOG with a build-time generated artifact by extracting metadata from the registerTool(...) calls (or by exporting tool metadata from each module: src/shared/tools/database.ts, storage.ts, functions.ts, deployment.ts, docs.ts) and emitting a static JSON (e.g., tool-catalog.generated.json) during the build; then change TOOL_CATALOG to import that generated JSON (or import a consolidated export from the registration modules) so the server still serves an unauthenticated flat list but it is kept in sync automatically. Ensure the generator collects {name, description} exactly as used in registerTool and that the runtime import preserves the same shape and export name TOOL_CATALOG.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/shared/tool-catalog.ts`:
- Around line 1-103: The TOOL_CATALOG is manually duplicated and risks drift;
replace the hand-maintained TOOL_CATALOG with a build-time generated artifact by
extracting metadata from the registerTool(...) calls (or by exporting tool
metadata from each module: src/shared/tools/database.ts, storage.ts,
functions.ts, deployment.ts, docs.ts) and emitting a static JSON (e.g.,
tool-catalog.generated.json) during the build; then change TOOL_CATALOG to
import that generated JSON (or import a consolidated export from the
registration modules) so the server still serves an unauthenticated flat list
but it is kept in sync automatically. Ensure the generator collects {name,
description} exactly as used in registerTool and that the runtime import
preserves the same shape and export name TOOL_CATALOG.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5741a3a3-256a-4219-8faf-992e3c928627
📒 Files selected for processing (2)
src/http/server.tssrc/shared/tool-catalog.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic analysis
No issues found across 2 files
Linked issue analysis
Linked issue: INS-306: Raise ora Agent Readiness Score (59 C → A): ship Discovery + Agent Integration quick wins
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Serve GET /.well-known/mcp/server-card.json as an unauthenticated discovery document | The PR adds a top-level Express route for SERVER_CARD_ENDPOINT and documents it as unauthenticated/static. |
| ✅ | Response includes the five flat top-level fields: name, description, version, serverUrl, tools | The handler returns a JSON object with exactly these keys. |
| ✅ | name is 'InsForge' | The JSON `name` field is explicitly set to 'InsForge' in the response object. |
| ✅ | version is resolved at runtime from package.json with fallbacks to npm env and a constant | The PR implements resolveServerVersion() that reads package.json relative to the bundled module and falls back to process.env.npm_package_version and '0.0.0'. |
| ✅ | serverUrl constructed from SERVER_CONFIG.publicUrl + STREAMABLE_HTTP_ENDPOINTS.mcp | The serverUrl returned is the templated combination of the configured public URL and the MCP endpoint constant. |
| ✅ | tools is a non-empty array of {name, description} entries (18 entries) and is served in the card | The PR adds a static TOOL_CATALOG array with 18 {name, description} objects and the handler returns tools: TOOL_CATALOG. |
| description is reused from repo-root server.json (kept in sync) | A human-readable SERVER_DESCRIPTION is present and a comment says it should be kept in sync with the repo-root server.json, but the code does not programmatically read server.json; it uses a hardcoded constant, so automated synchronization is not enforced. |
ora's Agent Readiness Score flagged that no tools on the InsForge MCP server carry behavior annotations. Add the MCP spec's 5 hint fields (title, readOnlyHint, destructiveHint, idempotentHint, openWorldHint) to every registered tool so agents get behavior hints in tools/list. Approach: pass an annotations object positionally between the input schema and the handler at each call site, matching the SDK's `tool(name, description, paramsSchema, annotations, cb)` overload (@modelcontextprotocol/sdk 1.28.0). The shared `registerTool` wrapper is already variadic (`...args`) and forwards straight to `server.tool`, so no wrapper change is needed and no new `any` is introduced. Per-tool rationale: - Read-only (readOnlyHint:true, destructiveHint:false): get-table-schema, get-backend-metadata, get-function, list-buckets, get-anon-key, get-container-logs, fetch-docs, fetch-sdk-docs, download-template. - Destructive (destructiveHint:true): delete-bucket, delete-function, run-raw-sql (can run arbitrary SQL incl. DROP). - Non-destructive writes (destructiveHint:false): create-bucket, create-function, update-function, bulk-upsert, create-deployment, start-deployment. - idempotentHint:true where natural (reads, upserts, update/delete by id). - openWorldHint:true everywhere: all tools hit the external InsForge backend. Also update the integration-bridge test's mock server to extract the handler as the final argument (robust to the extra annotations arg) and add an assertion that annotations are attached to registered tools. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tonychang04
changed the title
Member
Author
|
Consolidated to one PR per repo: folded the tool-annotations change (was #61) into this branch. This PR now covers:
Verified on the combined branch: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/shared/tools/database.ts`:
- Around line 209-215: The tool metadata for the "Download Starter Template" /
"download-template" tool is incorrect: change readOnlyHint from true to false
and idempotentHint from true to false in both metadata objects (the entries with
title 'Download Starter Template' / id 'download-template' found in the two
places shown in the diff) so the tool reflects that it writes local files and is
not safe to repeat; leave destructiveHint and openWorldHint as-is.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dde37d14-9986-4bd3-8b4b-b306fcc6700e
📒 Files selected for processing (6)
src/shared/tools/database.tssrc/shared/tools/deployment.tssrc/shared/tools/docs.tssrc/shared/tools/functions.tssrc/shared/tools/integration-bridge.test.tssrc/shared/tools/storage.ts
✅ Files skipped from review due to trivial changes (2)
- src/shared/tools/functions.ts
- src/shared/tools/deployment.ts
download-template scaffolds a starter template (writes files to disk in local mode, returns a scaffold command in remote mode), so it is neither read-only nor idempotent. Set readOnlyHint and idempotentHint to false at both registration sites. destructiveHint stays false (scaffolding is additive). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per scope decision, remove the MCP server-card discovery endpoint and its tool catalog. This PR now ships only the MCP tool annotations, which improve agent tool-selection ergonomics (read-only / destructive hints) independent of any discovery scanner. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tonychang04
changed the title
Member
Author
|
Scope trim: removed the |
Member
Author
|
Closing: with the server-card removed, the only remaining change here is tool annotations on mcp.insforge.dev — and ora can't see those (the server is auth-gated, so ora marks its annotations check N/A; the 1/2 annotations finding in the report was on the separate docs.insforge.dev/mcp server). So nothing in this PR moves the ora score. Closing to keep the insforge-mcp repo out of scope. The annotations are reasonable hygiene and can be revived later as a standalone quality change if wanted. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
ora's agent-readiness audit (INS-306) flagged for
mcp.insforge.dev:This PR closes that gap by serving the discovery document directly from the MCP HTTP server.
What
GET /.well-known/mcp/server-card.jsonroute tosrc/http/server.ts, alongside the existing OAuth.well-known/*routes. It requires no OAuth token and no backend connection, since it is a static discovery document.src/shared/tool-catalog.ts(TOOL_CATALOG), a curated flat list of the 18 tools with descriptions copied verbatim from eachregisterTool('<name>', '<description>', ...)call. A header comment notes it must stay in sync with the registered tools.Card shape
ora's
mcp-server-cardcheck needs five flat top-level fields for full credit (a nested SEP-1649 card fails; we do not nestnameunderserverInfoand do not use"tools": "dynamic"):name"InsForge"descriptionserver.jsonversionpackage.jsonat runtime (so it never drifts; falls back tonpm_package_version, then a constant)serverUrl${SERVER_CONFIG.publicUrl}${STREAMABLE_HTTP_ENDPOINTS.mcp}→https://mcp.insforge.dev/mcpin prodtools{ name, description }(18 entries)Note:
rootDiris./src, so a staticimport '../package.json'would fall outside the compile root. The version is therefore read at runtime relative to the bundled module.Verification
npx tsc --noEmitnpm run build(tsup)npm run lint(eslint)npm test(vitest)integration-bridge.test.tsforfetch-docsandget-anon-keyhit a live backend). Confirmed identical failures on untouchedorigin/master; no new failures introduced.Live curl
Ran the built server (
dist/http-server.js) against a local Redis withMCP_SERVER_URL=https://mcp.insforge.dev, then:Validation of the returned body:
name:'InsForge'version:'1.2.10'serverUrl:'https://mcp.insforge.dev/mcp'toolsis list: True, count: 18serverInfonesting: True;toolsis an array, not the string"dynamic"🤖 Generated with Claude Code
Summary by cubic
Add MCP tool annotations across all tools to improve agent selection and safety hints. Addresses Linear INS-306 for annotated tools; the server card is out of scope for this PR.
New Features
title,readOnlyHint,destructiveHint,idempotentHint,openWorldHint) to tools across database, storage, functions, deployment, and docs.get-table-schema,get-backend-metadata,get-function,list-buckets,get-anon-key,get-container-logs,fetch-docs,fetch-sdk-docs; destructive —delete-bucket,delete-function,run-raw-sql; non-destructive writes —create-bucket,create-function,update-function,bulk-upsert,create-deployment,start-deployment.Bug Fixes
download-templateannotations:readOnlyHint: false,idempotentHint: false(still non-destructive).Written for commit ed4acd4. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests