[PR #1722] [CLOSED] fix(mcp): advertise collection-scoped selectors for analysis views #4777

Closed
opened 2026-10-31 00:54:56 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1722
Author: @DeryFerd
Created: 10/11/2026
Status: ❌ Closed

Base: main ← Head: fix/analysis-view-advertised-selectors


📝 Commits (1)

  • 7826c56 fix(mcp): advertise collection-scoped selectors for analysis views

📊 Changes

2 files changed (+119 additions, -37 deletions)

View changed files

📝 src/domain/analysisView/analysisView.test.ts (+80 -0)
📝 src/domain/analysisView/analysisView.ts (+39 -37)

📄 Description

Summary

Fixes #1695. inspect_analysis_view validated the collection/selector pairing only through a Zod superRefine, and refinements disappear when the input contract is converted to JSON Schema, so tools/list advertised a four-way selector union for every item collection while the runtime rejected half of those combinations. This PR moves the existing pairing rules into the contract itself: modules are now expressed as a structural alternative that accepts only path or node_id, and sections and symbols as an alternative that accepts only index or name. The converted schema carries the constraints on its own, agents validating against the advertised schema stop constructing requests the server would refuse, and runtime selection semantics stay exactly as they were.

Problem and expected behavior

Smallest trigger: call inspect_analysis_view with an item view whose selector does not match its collection, for example modules with { index: 0 } or sections with { path: "renderer.js" }. Prior behavior: the real advertised schema, compiled with a draft-2020-12 validator, accepted every combination, and the MCP server then rejected the incompatible ones during input validation with view.selector: Module items are selected by exact path or node_id, not index or name. The violated invariant: the advertised input schema and the runtime validation must describe the same accepted set, because clients and agents discover callable arguments only from tools/list. Expected behavior: the converted schema rejects modules + index/name and sections/symbols + path/node_id before any request is sent, the runtime keeps rejecting the same set with the same guidance text, and the four valid pairings plus summary, facet, page, and native views continue to work unchanged.

Change and scope

The owning contract in src/domain/analysisView/analysisView.ts now models the pairing structurally: the single item object with an independent collection enum, a four-way selector union, and a superRefine is replaced by two item alternatives inside the request union, each pairing its collection with its own selector union, and the request union itself becomes a plain union so both item alternatives can carry kind: "item". A Zod 4 error map on each selector union keeps the original guidance strings, and a request-level error map promotes the failing branch's selector message so MCP clients still see a named constraint rather than a generic union failure; CLI-side projection already flattens nested union branch issues, so it surfaces the same messages at view.selector. Intentionally not included: no change to projection layers, result schemas, paging, native views, or Evidence sealing; path ambiguity for shared module paths remains the separate runtime condition that directs callers to node_id.

Contract and boundary impact

  • Semantic owner and earliest changed stage: domain contract analysisViewRequestSchema in src/domain/analysisView/analysisView.ts; the advertised input contract is the first and only changed stage, with application workflows and projectors untouched.
  • CLI and MCP/tool-catalog contract: inspect_analysis_view input schema shape changes from one item object plus refinement to two structural item alternatives; the advertised JSON Schema now constrains collection/selector pairs; runtime accept/reject sets are identical; the catalog digest moves with the contract because it is computed at runtime from the canonical contracts, and no catalog file is committed.
  • Provider, bridge, target-format, or platform compatibility: none; the tool is provider-neutral and no bridge or platform path is involved.
  • Evidence, artifact, provenance, or reconstruction contract: none; view results, view_digest sealing, parent Evidence validation, and coverage projections are unchanged.
  • Process execution, authorization, cleanup, or containment impact: none; this is schema and validation structure only.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in skill-src/ and never commit build outputs): none committed. .cache/mcp-tool-catalog.json is regenerated test metadata and is not part of the repository; docs/mcp-contracts.md describes the tool in prose and embeds no input schema, so no generated document changes.

Evidence and regression coverage

  • Tests added or updated: two cases in src/domain/analysisView/analysisView.test.ts. One compiles the converted input schema with Ajv's draft-2020-12 validator and asserts that modules+index, modules+name, sections+path, and symbols+node_id are rejected while the four valid pairings, summary, and page views are accepted. The other parses the runtime input schema and asserts the unchanged accept/reject sets plus the exact original guidance messages for both directions.
  • Base reproduction or other evidence: on base 13e03e94 the advertised-schema case fails because the converted schema accepts modules+index and sections+path; the runtime-semantics case already passes there and stays green after the restructure, which pins that the accepted set did not move.
  • User-visible CLI/MCP output: for the four incompatible combinations the guidance text is preserved verbatim; the MCP-rendered prefix becomes view: <guidance> because the constraint is now expressed structurally at the union rather than as a refinement on view.selector. Valid requests produce byte-identical results.
  • Remaining proof gaps: a live third-party MCP client was not run manually; the real stdio MCP boundary suite covers the advertised-schema and validation behavior through the pinned SDK client, and the issue's Ajv 8.20.0 draft-2020-12 reproduction is mirrored by the new Ajv2020-based test. No real-provider or browser coverage applies to this contract.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • npx vitest run src/domain/analysisView/analysisView.test.ts — 19 passed, including the new advertised-schema and runtime-guidance cases.
  • npx vitest run tests/boundary/mcp/analysisViewMcp.test.ts — 8 passed through the real stdio MCP workflow.
  • npx vitest run tests/boundary/mcp/toolSchemaValidity.test.ts — 7 passed after npm run mcp-catalog:generate.
  • npx vitest run tests/evaluation/model-evals/codexAgentEval.test.ts — 12 passed.
  • npx vitest run tests/boundary/cli/analysisViewCli.test.ts — 6 passed, 1 failed; the portable-bundle workflow failure reproduces identically on the pristine base in this Windows host and is unrelated to this change.
  • npx vitest run tests/boundary/mcp/analysisSummaryDetailMcp.test.ts — 2 failures that also reproduce identically on the pristine base (pre-existing on this Windows host); the remaining test passes.
  • npm run typecheck — exit 0 after rebasing onto 8be7ad49.
  • npx oxlint on both changed files — 0 warnings, 0 errors; oxfmt --check on the committed (LF) form — clean.
  • Real Hopper/Ghidra, browser, or OS lanes: not applicable; no provider, bridge, or platform-specific code is touched.

Compatibility, safety, and release

  • Breaking changes or migration steps: none for valid callers. Requests that already succeeded keep identical results, and requests that were already rejected at runtime are now also rejected by the advertised schema, which is the intended tightening; clients that validate against tools/list fail earlier with the same guidance instead of after a round trip.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable; the boundary suites above cover the changed contract.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: the change removes a discoverability contradiction between advertised and enforced arguments; it adds no network, process, file, or authority surface and does not alter evidence handling.

Review checklist

  • This PR addresses a concrete problem or an agreed enhancement.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🔄 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/1722 **Author:** [@DeryFerd](https://github.com/DeryFerd) **Created:** 10/11/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/analysis-view-advertised-selectors` --- ### 📝 Commits (1) - [`7826c56`](https://github.com/morluto/rea/commit/7826c56c74ba290865fef55b48aae6b1653d3354) fix(mcp): advertise collection-scoped selectors for analysis views ### 📊 Changes **2 files changed** (+119 additions, -37 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/analysisView/analysisView.test.ts` (+80 -0) 📝 `src/domain/analysisView/analysisView.ts` (+39 -37) </details> ### 📄 Description ## Summary Fixes #1695. `inspect_analysis_view` validated the collection/selector pairing only through a Zod `superRefine`, and refinements disappear when the input contract is converted to JSON Schema, so `tools/list` advertised a four-way selector union for every item collection while the runtime rejected half of those combinations. This PR moves the existing pairing rules into the contract itself: modules are now expressed as a structural alternative that accepts only `path` or `node_id`, and sections and symbols as an alternative that accepts only `index` or `name`. The converted schema carries the constraints on its own, agents validating against the advertised schema stop constructing requests the server would refuse, and runtime selection semantics stay exactly as they were. ## Problem and expected behavior Smallest trigger: call `inspect_analysis_view` with an item view whose selector does not match its collection, for example `modules` with `{ index: 0 }` or `sections` with `{ path: "renderer.js" }`. Prior behavior: the real advertised schema, compiled with a draft-2020-12 validator, accepted every combination, and the MCP server then rejected the incompatible ones during input validation with `view.selector: Module items are selected by exact path or node_id, not index or name.` The violated invariant: the advertised input schema and the runtime validation must describe the same accepted set, because clients and agents discover callable arguments only from `tools/list`. Expected behavior: the converted schema rejects `modules` + `index`/`name` and `sections`/`symbols` + `path`/`node_id` before any request is sent, the runtime keeps rejecting the same set with the same guidance text, and the four valid pairings plus summary, facet, page, and native views continue to work unchanged. ## Change and scope The owning contract in `src/domain/analysisView/analysisView.ts` now models the pairing structurally: the single item object with an independent `collection` enum, a four-way selector union, and a `superRefine` is replaced by two item alternatives inside the request union, each pairing its collection with its own selector union, and the request union itself becomes a plain union so both item alternatives can carry `kind: "item"`. A Zod 4 error map on each selector union keeps the original guidance strings, and a request-level error map promotes the failing branch's selector message so MCP clients still see a named constraint rather than a generic union failure; CLI-side projection already flattens nested union branch issues, so it surfaces the same messages at `view.selector`. Intentionally not included: no change to projection layers, result schemas, paging, native views, or Evidence sealing; path ambiguity for shared module paths remains the separate runtime condition that directs callers to `node_id`. ## Contract and boundary impact - Semantic owner and earliest changed stage: domain contract `analysisViewRequestSchema` in `src/domain/analysisView/analysisView.ts`; the advertised input contract is the first and only changed stage, with application workflows and projectors untouched. - CLI and MCP/tool-catalog contract: `inspect_analysis_view` input schema shape changes from one item object plus refinement to two structural item alternatives; the advertised JSON Schema now constrains collection/selector pairs; runtime accept/reject sets are identical; the catalog digest moves with the contract because it is computed at runtime from the canonical contracts, and no catalog file is committed. - Provider, bridge, target-format, or platform compatibility: none; the tool is provider-neutral and no bridge or platform path is involved. - Evidence, artifact, provenance, or reconstruction contract: none; view results, `view_digest` sealing, parent Evidence validation, and coverage projections are unchanged. - Process execution, authorization, cleanup, or containment impact: none; this is schema and validation structure only. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `skill-src/` and never commit build outputs): none committed. `.cache/mcp-tool-catalog.json` is regenerated test metadata and is not part of the repository; `docs/mcp-contracts.md` describes the tool in prose and embeds no input schema, so no generated document changes. ## Evidence and regression coverage - Tests added or updated: two cases in `src/domain/analysisView/analysisView.test.ts`. One compiles the converted input schema with Ajv's draft-2020-12 validator and asserts that `modules`+`index`, `modules`+`name`, `sections`+`path`, and `symbols`+`node_id` are rejected while the four valid pairings, summary, and page views are accepted. The other parses the runtime input schema and asserts the unchanged accept/reject sets plus the exact original guidance messages for both directions. - Base reproduction or other evidence: on base `13e03e94` the advertised-schema case fails because the converted schema accepts `modules`+`index` and `sections`+`path`; the runtime-semantics case already passes there and stays green after the restructure, which pins that the accepted set did not move. - User-visible CLI/MCP output: for the four incompatible combinations the guidance text is preserved verbatim; the MCP-rendered prefix becomes `view: <guidance>` because the constraint is now expressed structurally at the union rather than as a refinement on `view.selector`. Valid requests produce byte-identical results. - Remaining proof gaps: a live third-party MCP client was not run manually; the real stdio MCP boundary suite covers the advertised-schema and validation behavior through the pinned SDK client, and the issue's Ajv 8.20.0 draft-2020-12 reproduction is mirrored by the new Ajv2020-based test. No real-provider or browser coverage applies to this contract. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npx vitest run src/domain/analysisView/analysisView.test.ts` — 19 passed, including the new advertised-schema and runtime-guidance cases. - `npx vitest run tests/boundary/mcp/analysisViewMcp.test.ts` — 8 passed through the real stdio MCP workflow. - `npx vitest run tests/boundary/mcp/toolSchemaValidity.test.ts` — 7 passed after `npm run mcp-catalog:generate`. - `npx vitest run tests/evaluation/model-evals/codexAgentEval.test.ts` — 12 passed. - `npx vitest run tests/boundary/cli/analysisViewCli.test.ts` — 6 passed, 1 failed; the portable-bundle workflow failure reproduces identically on the pristine base in this Windows host and is unrelated to this change. - `npx vitest run tests/boundary/mcp/analysisSummaryDetailMcp.test.ts` — 2 failures that also reproduce identically on the pristine base (pre-existing on this Windows host); the remaining test passes. - `npm run typecheck` — exit 0 after rebasing onto `8be7ad49`. - `npx oxlint` on both changed files — 0 warnings, 0 errors; `oxfmt --check` on the committed (LF) form — clean. - Real Hopper/Ghidra, browser, or OS lanes: not applicable; no provider, bridge, or platform-specific code is touched. ## Compatibility, safety, and release - Breaking changes or migration steps: none for valid callers. Requests that already succeeded keep identical results, and requests that were already rejected at runtime are now also rejected by the advertised schema, which is the intended tightening; clients that validate against `tools/list` fail earlier with the same guidance instead of after a round trip. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; the boundary suites above cover the changed contract. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the change removes a discoverability contradiction between advertised and enforced arguments; it adds no network, process, file, or authority surface and does not alter evidence handling. ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 00:54:56 +00:00
Sign in to join this conversation.
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#4777
No description provided.