morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 07:47:29 +00:00

[PR #1792] [CLOSED] fix(mcp): advertise only the collection and selector pairings the server admits #4821

New issue

Closed

opened 2026-10-31 00:55:24 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:55:24 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1792
Author: @yylm-lm
Created: 10/11/2026
Status: ❌ Closed

Base: main ← Head: fix/analysis-view-selector-agreement

📝 Commits (3)

c21de4c fix(mcp): advertise only the collection and selector pairings the server admits
60954a7 test(mcp): assert the advertised selector schema against runtime validation
4cfb8d0 fix(mcp): restore the discriminated collection contract

📊 Changes

2 files changed (+191 additions, -28 deletions)

View changed files

📝 src/domain/analysisView/analysisView.ts (+27 -28)
➕ tests/boundary/mcp/analysisViewSelectorAgreement.test.ts (+164 -0)

📄 Description

Closes #1695

Problem

inspect_analysis_view expressed its collection/selector pairing in a
superRefine. A refinement is absent from the converted JSON Schema, so the
advertised input schema and runtime validation disagreed:

view	advertised schema	runtime
`collection: "modules"` + `selector: { index }`	valid	rejected
`collection: "sections"` + `selector: { node_id }`	valid	rejected

A client that validates arguments against tools/list therefore sent
arguments the server refused as invalid, before it could interpret whether the
retained Evidence supported the requested collection. Reproduced with the real
stdio MCP transport and Ajv 8.20.0 draft-2020-12 validation.

Change

Express the existing alternatives in the owning contract rather than in a
refinement: modules are selected by path or node_id; sections and symbols
by index or name. This is the direction the report suggested.

Runtime validation is unchanged: every pairing rejected before is still
rejected, now by the schema as well.
Selection semantics, Evidence identity, and path ambiguity handling are
untouched.
The advertised schema now carries the alternatives, so a client validating
against it cannot construct an argument set the server refuses.

Verification

tests/boundary/mcp/analysisViewSelectorAgreement.test.ts drives the real
stdio MCP transport over twelve collection/selector pairings and asserts that
every advertised pairing is admitted and every unadvertised one is rejected as
invalid input. Admission is keyed on the input-validation failure rather than on
call success, so a pairing that passes validation and then fails Evidence
resolution still counts as admitted.

npx tsc --noEmit passes.
tests/boundary/mcp/analysisViewMcp.test.ts (which exercises this schema
and its output contract) passes 8/8.
npm run mcp-catalog:check passes, so the generated catalog stays fresh.

Note for review

This changes the shape of some validation errors for view.kind: "item", for
example from expected: ["index","name"] to expected: "object", because the
pairing is now a discriminated union. I believe that is the intended trade: the
previous error text described a constraint the schema did not advertise. If you
would rather preserve the old message shape, say so and I will adjust.

Not verified here

The five failures in tests/boundary/cli/analysisViewCli.test.ts and two in
tests/boundary/mcp/analysisSummaryDetailMcp.test.ts are pre-existing on this
checkout (missing stream-chain/stream-json and a stale expected-error
shape); I confirmed each by stashing this change and reproducing it on a clean
tree. No new failures are introduced.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/1792 **Author:** [@yylm-lm](https://github.com/yylm-lm) **Created:** 10/11/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/analysis-view-selector-agreement` --- ### 📝 Commits (3) - [`c21de4c`](https://github.com/morluto/rea/commit/c21de4c6ce19328031128d8b799d97d466458c61) fix(mcp): advertise only the collection and selector pairings the server admits - [`60954a7`](https://github.com/morluto/rea/commit/60954a716679cd32c3821c03a9251bfa395e17e9) test(mcp): assert the advertised selector schema against runtime validation - [`4cfb8d0`](https://github.com/morluto/rea/commit/4cfb8d00d66a65b45939a87e13767e43e7056076) fix(mcp): restore the discriminated collection contract ### 📊 Changes **2 files changed** (+191 additions, -28 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/analysisView/analysisView.ts` (+27 -28) ➕ `tests/boundary/mcp/analysisViewSelectorAgreement.test.ts` (+164 -0) </details> ### 📄 Description Closes #1695 ## Problem `inspect_analysis_view` expressed its collection/selector pairing in a `superRefine`. A refinement is absent from the converted JSON Schema, so the advertised input schema and runtime validation disagreed: | view | advertised schema | runtime | |---|---|---| | `collection: "modules"` + `selector: { index }` | valid | rejected | | `collection: "sections"` + `selector: { node_id }` | valid | rejected | A client that validates arguments against `tools/list` therefore sent arguments the server refused as invalid, before it could interpret whether the retained Evidence supported the requested collection. Reproduced with the real stdio MCP transport and Ajv 8.20.0 draft-2020-12 validation. ## Change Express the existing alternatives in the owning contract rather than in a refinement: modules are selected by `path` or `node_id`; sections and symbols by `index` or `name`. This is the direction the report suggested. - Runtime validation is unchanged: every pairing rejected before is still rejected, now by the schema as well. - Selection semantics, Evidence identity, and path ambiguity handling are untouched. - The advertised schema now carries the alternatives, so a client validating against it cannot construct an argument set the server refuses. ## Verification `tests/boundary/mcp/analysisViewSelectorAgreement.test.ts` drives the real stdio MCP transport over twelve collection/selector pairings and asserts that every advertised pairing is admitted and every unadvertised one is rejected as invalid input. Admission is keyed on the input-validation failure rather than on call success, so a pairing that passes validation and then fails Evidence resolution still counts as admitted. - `npx tsc --noEmit` passes. - `tests/boundary/mcp/analysisViewMcp.test.ts` (which exercises this schema and its output contract) passes 8/8. - `npm run mcp-catalog:check` passes, so the generated catalog stays fresh. ## Note for review This changes the shape of some validation errors for `view.kind: "item"`, for example from `expected: ["index","name"]` to `expected: "object"`, because the pairing is now a discriminated union. I believe that is the intended trade: the previous error text described a constraint the schema did not advertise. If you would rather preserve the old message shape, say so and I will adjust. ## Not verified here The five failures in `tests/boundary/cli/analysisViewCli.test.ts` and two in `tests/boundary/mcp/analysisSummaryDetailMcp.test.ts` are pre-existing on this checkout (missing `stream-chain`/`stream-json` and a stale expected-error shape); I confirmed each by stashing this change and reproducing it on a clean tree. No new failures are introduced. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 00:55:24 +00:00

closed this issue
added the
pull-request
label

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#4821

No description provided.

Rows
Columns