Skip to content

feat: declare input schemas for the selector-taking agent tools - #2

Open
erkamyaman wants to merge 2 commits into
santoshyadavdev:mainfrom
erkamyaman:feat/mcp-tool-input-schemas
Open

erkamyaman wants to merge 2 commits into
santoshyadavdev:mainfrom
erkamyaman:feat/mcp-tool-input-schemas

Conversation

@erkamyaman

Copy link
Copy Markdown

Stacked on #1, which repairs the build. The last commit is the only one specific to this PR.

ng-devtools_highlight, _inspect-signals and _inspect-providers each advertise no parameters:

"inputSchema": { "type": "object", "properties": {} }

while all three handlers read args.selector. An MCP client has no way to discover the argument, and a wrong guess fails silently:

tools/call ng-devtools_highlight {"componentSelector": "app-root"}
-> "Highlighted `undefined` in the page overlay."

Added a raw inputSchema to each. Verified over stdio that tools/list now advertises selector as required and the handler receives it.

Used inputSchema rather than the preferred args, because args advertises positional arg0 and passes the payload through unchanged, so handlers reading args.selector would break. valibot also exposes no ~standard.jsonSchema, so args would degrade to {type: "object", additionalProperties: true} and lose the description.

Two other things I ran into while testing the MCP server, not fixed here:

  • get-routes, get-components, get-signals and get-providers fail every tools/call with -32602, because they return v.array(...) and devframe advertises an object outputSchema for it. Fix submitted upstream as fix(agentic): do not advertise an output schema devframe cannot derive devframes/devframe#400.
  • inspect-signals / inspect-providers always return {} rather than their "No signal graph available" fallback. my.rpc.broadcast is Promise<void> and never rejects, so the catch is unreachable. Happy to open a separate PR if you want a particular shape for it.

- use .ts import specifiers in devframe.ts; the package has no build
  step and Node's type stripping does not remap .js to .ts
- enable allowImportingTsExtensions and rewriteRelativeImportExtensions
  in tsconfig.app.json so ng build still compiles src/server.ts
- declare cac, an optional peer of devframe that nothing installs but
  both bin.mjs call through createCac
- provide Router in app.spec.ts and drop the assertion on an h1 that
  app.html no longer renders
highlight, inspect-signals and inspect-providers each advertised
`inputSchema: { type: 'object', properties: {} }` while their handlers
read `args.selector`, so an MCP client had no way to discover the
argument and a wrong guess failed silently.
@coderabbitai

coderabbitai Bot commented Sep 20, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 14 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

This review ran on the open-source allowance, not this organization's plan, because the pull request author doesn't have an assigned seat. Waiting won't change this — ask an organization admin to assign them a seat, or add seats in Billing if every seat is already assigned, then retry.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: d2a8e799-1b85-40b1-a73f-a54e9d26a55a

📥 Commits

Reviewing files that changed from the base of the PR and between 44a7e20 and 08477a4.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (4)
  • packages/ng-devtools/package.json
  • packages/ng-devtools/src/devframe.ts
  • src/app/app.spec.ts
  • tsconfig.app.json

Comment @coderabbitai help to get the list of available commands.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant