morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #2836] [CLOSED] fix(mcp): refuse unreadable navigation selections before success #5426

New issue

Closed

opened 2026-10-31 01:06:33 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:06:33 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2836
Author: @yasshmittal
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: fix/mcp-navigation-unreadable-selection

📝 Commits (1)

08b6344 fix(mcp): refuse unreadable navigation selections before success

📊 Changes

3 files changed (+162 additions, -16 deletions)

View changed files

📝 src/application/AnalysisContextQueries.test.ts (+39 -0)
📝 src/application/AnalysisContextQueries.ts (+19 -16)
➕ tests/boundary/mcp/navigationContextMcp.test.ts (+104 -0)

📄 Description

Summary

Fixes #2516.

When a provider reported a null current document or current address, get_navigation_context returned success with that null, although its output contract requires both fields to be strings. Through MCP, the SDK then rejected the result and clients received a generic Output validation error with no structured diagnostic.

Problem and expected behavior

getNavigationContext (src/application/AnalysisContextQueries.ts) forwarded non-string observations into a successful result, and carried them through an explicit branch for a non-string address. An unreadable provider observation should become a typed provider-output failure before success delivery, consistent with resolveAnalysisDocument.

Change and scope

A non-string current document returns AnalysisOutputError("current_document", …).
A non-string current address returns AnalysisOutputError("current_address", …).
Both are refused before resolve_containing_procedure is called.
The branches that carried non-string selections into success are removed; they could only produce results the tool's own schema rejects.
The advertised string contract is unchanged, and empty strings remain valid as before.
An explicit document argument still skips current_document, and a cursor outside a procedure still returns procedure: null.

Contribution scope: an ordinary fix within existing boundaries; no maintainer scope decision applies.

Contract and boundary impact

Semantic owner and earliest changed stage: navigation composition in src/application/AnalysisContextQueries.ts.
Contribution scope fit and linked maintainer decision: not applicable (bug fix).
CLI and MCP/tool-catalog contract: output schema unchanged; unreadable selections now return a typed unreadable_output error instead of an SDK validation message.
Provider, bridge, target-format, or platform compatibility: providers that return string selections are unaffected.
Evidence, artifact, provenance, or reconstruction contract: unchanged.
Target selection, authority, upstream behavior, and cleanup details: unchanged.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.

Evidence and regression coverage

Tests added or updated:
- src/application/AnalysisContextQueries.test.ts: both null variants return AnalysisOutputError naming the unreadable operation, without calling resolve_containing_procedure.
- tests/boundary/mcp/navigationContextMcp.test.ts (new): the production server and tool registration with a simulated provider and the in-memory MCP client. Both variants deliver a typed error (code: "unreadable_output", details.operation) and not Output validation error.
Base reproduction or other evidence: on the base revision all four tests fail. The MCP variants return Output validation error: … expected string, received null, as in the issue.

User-visible CLI/MCP output:

{"error":{"code":"unreadable_output","details":{"operation":"current_address", …}}}

Remaining proof gaps: no live provider returning null was observed; the provider is simulated, as in the issue.
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

Host: Windows 11, Node.js 24.21.0.

npx vitest run src/application/AnalysisContextQueries.test.ts: 7 passed.
npx vitest run tests/boundary/mcp/navigationContextMcp.test.ts: 2 passed.
npx vitest run src/server src/application/CapabilityInventory.test.ts: 30 passed.
npm run check, npm run verify:test-discovery: passed.

Compatibility, safety, and release

Breaking changes or migration steps: none. Such calls previously failed with an SDK validation error; they now fail with a typed error.
Real Hopper/Ghidra, browser, or OS coverage: Windows only; simulated provider.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new I/O, process, or network behavior.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
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.

🤖 Generated with Claude Code

_{🔄 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/2836 **Author:** [@yasshmittal](https://github.com/yasshmittal) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/mcp-navigation-unreadable-selection` --- ### 📝 Commits (1) - [`08b6344`](https://github.com/morluto/rea/commit/08b63442bb7db156bc46f9c33a6645782b1346b7) fix(mcp): refuse unreadable navigation selections before success ### 📊 Changes **3 files changed** (+162 additions, -16 deletions) <details> <summary>View changed files</summary> 📝 `src/application/AnalysisContextQueries.test.ts` (+39 -0) 📝 `src/application/AnalysisContextQueries.ts` (+19 -16) ➕ `tests/boundary/mcp/navigationContextMcp.test.ts` (+104 -0) </details> ### 📄 Description ## Summary Fixes #2516. When a provider reported a `null` current document or current address, `get_navigation_context` returned success with that `null`, although its output contract requires both fields to be strings. Through MCP, the SDK then rejected the result and clients received a generic `Output validation error` with no structured diagnostic. ## Problem and expected behavior `getNavigationContext` (`src/application/AnalysisContextQueries.ts`) forwarded non-string observations into a successful result, and carried them through an explicit branch for a non-string address. An unreadable provider observation should become a typed provider-output failure before success delivery, consistent with `resolveAnalysisDocument`. ## Change and scope - A non-string current document returns `AnalysisOutputError("current_document", …)`. - A non-string current address returns `AnalysisOutputError("current_address", …)`. - Both are refused before `resolve_containing_procedure` is called. - The branches that carried non-string selections into success are removed; they could only produce results the tool's own schema rejects. - The advertised string contract is unchanged, and empty strings remain valid as before. - An explicit `document` argument still skips `current_document`, and a cursor outside a procedure still returns `procedure: null`. Contribution scope: an ordinary fix within existing boundaries; no maintainer scope decision applies. ## Contract and boundary impact - Semantic owner and earliest changed stage: navigation composition in `src/application/AnalysisContextQueries.ts`. - Contribution scope fit and linked maintainer decision: not applicable (bug fix). - CLI and MCP/tool-catalog contract: output schema unchanged; unreadable selections now return a typed `unreadable_output` error instead of an SDK validation message. - Provider, bridge, target-format, or platform compatibility: providers that return string selections are unaffected. - Evidence, artifact, provenance, or reconstruction contract: unchanged. - Target selection, authority, upstream behavior, and cleanup details: unchanged. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: - `src/application/AnalysisContextQueries.test.ts`: both null variants return `AnalysisOutputError` naming the unreadable operation, without calling `resolve_containing_procedure`. - `tests/boundary/mcp/navigationContextMcp.test.ts` (new): the production server and tool registration with a simulated provider and the in-memory MCP client. Both variants deliver a typed error (`code: "unreadable_output"`, `details.operation`) and not `Output validation error`. - Base reproduction or other evidence: on the base revision all four tests fail. The MCP variants return `Output validation error: … expected string, received null`, as in the issue. - User-visible CLI/MCP output: ```json {"error":{"code":"unreadable_output","details":{"operation":"current_address", …}}} ``` - Remaining proof gaps: no live provider returning `null` was observed; the provider is simulated, as in the issue. - [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 Host: Windows 11, Node.js 24.21.0. - `npx vitest run src/application/AnalysisContextQueries.test.ts`: 7 passed. - `npx vitest run tests/boundary/mcp/navigationContextMcp.test.ts`: 2 passed. - `npx vitest run src/server src/application/CapabilityInventory.test.ts`: 30 passed. - `npm run check`, `npm run verify:test-discovery`: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Such calls previously failed with an SDK validation error; they now fail with a typed error. - Real Hopper/Ghidra, browser, or OS coverage: Windows only; simulated provider. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new I/O, process, or network behavior. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:06:33 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:44:06 +00:00

[PR #5427] [CLOSED] fix(adb): read the package id from appId on Android 16 #6925

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#5426

No description provided.

Rows
Columns