morluto/rea

Fork 0

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

[PR #7340] [MERGED] fix(mcp): report session tool target requirements accurately #7823

New issue

Closed

opened 2026-11-05 01:47:56 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-05 01:47:56 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7340
Author: @akshad-exe
Created: 10/27/2026
Status: ✅ Merged
Merged: 11/4/2026
Merged by: @morluto

Base: main ← Head: fix/mcp-session-target-availability

📝 Commits (2)

4e1160a fix(mcp): report session tool target requirements accurately
238543f test(mcp): cover closed-session and artifact target requirements

📊 Changes

4 files changed (+260 additions, -14 deletions)

View changed files

📝 src/application/CapabilityInventory.test.ts (+73 -0)
📝 src/application/CapabilityInventory.ts (+29 -9)
📝 src/contracts/sessionToolContracts.ts (+8 -5)
➕ tests/boundary/mcp/sessionTargetRequirements.test.ts (+150 -0)

📄 Description

Summary

Fixes #7310, #7399, and #7397. binary_session now reports closed-session requirements accurately and reports address-context and navigation restrictions for artifact and archive targets.

#7316 is no longer a source change in this PR. It was already fixed on main by #7896 (845080aef), which added inspect_apk_signature to the explicit-path table in workflowAvailabilityFor and evaluates that table before the target_required fallback. #8644 reached the same conclusion and added a regression for it. This PR drops its former requiresActiveTarget removal on the APK contract, so src/contracts/apkSignature/apkSignatureToolContracts.ts is identical to main.

Problem and expected behavior

#7310: with no target open, binary_session {} advertised inspect_address_context as available: true, but the call failed with target_unavailable; the sibling get_navigation_context correctly reported target_required.
#7399: after opening a plist, the default address-context request was advertised as available while the call failed with capability_unavailable / selection_reason: "target_unsupported".
#7397: navigation on artifact and archive targets was reported as a missing-provider problem instead of an unsupported target, so remediation advised provider configuration rather than changing the target.

All three were reproduced on the base revision through a built stdio MCP server.

Change and scope

CapabilityInventory now consults its existing target-availability owner before marking session-kind tools available. The address-context contract declares an active-target prerequisite. Artifact and archive targets report unsupported address-context and navigation modes, with per-mode missing_operations and remediation taken from the target restriction, while preserving explicit-document address requests. Tool execution, input validation, target selection, and Evidence delivery are unchanged. No new provider or authority is introduced; maintainer scope decision is not applicable.

Verification against current `main`

The four issues were checked individually rather than assumed. Copying this branch's regression tests onto a detached worktree at main (8c857ce56) produced 48 passed, 2 failed:

#7316 — already fixed on main. The APK assertion passes because of #7896. This PR keeps only regression coverage for it.
#7310 — still reproduces. inspect_address_context returned available: true instead of target_required.
#7399 — still reproduces. The artifact-target address case is not covered by main's target_unsupported handling.
#7397 — still reproduces. Navigation returned reason: "available", default_mode_available: true on artifact targets.

main's targetAvailability unsupported list covers only kind === "official-proxy" || kind === "enhanced", which is why plain artifact targets pass while navigation and address-context do not.

Contract and boundary impact

Semantic owner: the named tool contracts provide prerequisites; CapabilityInventory projects them into binary_session availability.
CLI and MCP/tool catalog: advertised availability changes for address-context and navigation on closed sessions and on artifact/archive targets; the tools and their schemas are unchanged.
Provider, bridge, platform, Evidence, cleanup, package and installation impact: none identified. Generated/ignored catalogs are validated by docs:check and are not committed.

Evidence and regression coverage

src/application/CapabilityInventory.test.ts: closed-session address and explicit-path APK availability, artifact and archive restrictions with per-mode navigation flags, and structured reasons rather than pinned remediation sentences.
tests/boundary/mcp/sessionTargetRequirements.test.ts: production stdio MCP regression observing both binary_session entries before and after the calls, the target_unavailable address error, the successful unsigned-ZIP APK call, and the sibling navigation prerequisite.
Boundary covered: contract to capability inventory to compacted MCP session status and actual calls. No lifecycle or provider execution changes. Real deep-analysis providers were not used; the APK call used a generated local ZIP and the production stdio server.

Validation performed

On rebased head 238543ff2 (base 8c857ce56):

npm run test:focused -- src/application/CapabilityInventory.test.ts tests/boundary/mcp/sessionTargetRequirements.test.ts tests/boundary/mcp/enhancedTools.limitations.test.ts — 61 passed, including the two stdio MCP session-status cases.
npm run check:fast — passed (typecheck and lint).
npm run format:check — passed.
git diff --check — clean.

The branch is two commits: the inventory/contract fix and its regressions. No real deep-analysis provider or native target was exercised.

Compatibility, safety, and release

No migration or execution behavior change. The status correction prevents clients from selecting an impossible address request or overlooking an available explicit-path APK inspection, and stops provider-configuration advice where the target must change instead. No real-provider claim is made.

Author readiness

Human owner: @akshad-exe

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

The human owner reviews and confirms the final diff and validation evidence.
CI and remaining required checks pass or limitations are resolved and documented.

_{🔄 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/7340 **Author:** [@akshad-exe](https://github.com/akshad-exe) **Created:** 10/27/2026 **Status:** ✅ Merged **Merged:** 11/4/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/mcp-session-target-availability` --- ### 📝 Commits (2) - [`4e1160a`](https://github.com/morluto/rea/commit/4e1160a2f63274190de986e5917827d31628c5cd) fix(mcp): report session tool target requirements accurately - [`238543f`](https://github.com/morluto/rea/commit/238543ff2355ddbea46b5169e1ab037ecb425b80) test(mcp): cover closed-session and artifact target requirements ### 📊 Changes **4 files changed** (+260 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `src/application/CapabilityInventory.test.ts` (+73 -0) 📝 `src/application/CapabilityInventory.ts` (+29 -9) 📝 `src/contracts/sessionToolContracts.ts` (+8 -5) ➕ `tests/boundary/mcp/sessionTargetRequirements.test.ts` (+150 -0) </details> ### 📄 Description ## Summary Fixes #7310, #7399, and #7397. `binary_session` now reports closed-session requirements accurately and reports address-context and navigation restrictions for artifact and archive targets. #7316 is no longer a source change in this PR. It was already fixed on `main` by #7896 (`845080aef`), which added `inspect_apk_signature` to the explicit-path table in `workflowAvailabilityFor` and evaluates that table before the `target_required` fallback. #8644 reached the same conclusion and added a regression for it. This PR drops its former `requiresActiveTarget` removal on the APK contract, so `src/contracts/apkSignature/apkSignatureToolContracts.ts` is identical to `main`. ## Problem and expected behavior - #7310: with no target open, `binary_session {}` advertised `inspect_address_context` as `available: true`, but the call failed with `target_unavailable`; the sibling `get_navigation_context` correctly reported `target_required`. - #7399: after opening a plist, the default address-context request was advertised as available while the call failed with `capability_unavailable` / `selection_reason: "target_unsupported"`. - #7397: navigation on artifact and archive targets was reported as a missing-provider problem instead of an unsupported target, so remediation advised provider configuration rather than changing the target. All three were reproduced on the base revision through a built stdio MCP server. ## Change and scope `CapabilityInventory` now consults its existing target-availability owner before marking session-kind tools available. The address-context contract declares an active-target prerequisite. Artifact and archive targets report unsupported address-context and navigation modes, with per-mode `missing_operations` and remediation taken from the target restriction, while preserving explicit-document address requests. Tool execution, input validation, target selection, and Evidence delivery are unchanged. No new provider or authority is introduced; maintainer scope decision is not applicable. ## Verification against current `main` The four issues were checked individually rather than assumed. Copying this branch's regression tests onto a detached worktree at `main` (`8c857ce56`) produced **48 passed, 2 failed**: - **#7316 — already fixed on `main`.** The APK assertion passes because of #7896. This PR keeps only regression coverage for it. - **#7310 — still reproduces.** `inspect_address_context` returned `available: true` instead of `target_required`. - **#7399 — still reproduces.** The artifact-target address case is not covered by `main`'s `target_unsupported` handling. - **#7397 — still reproduces.** Navigation returned `reason: "available"`, `default_mode_available: true` on artifact targets. `main`'s `targetAvailability` unsupported list covers only `kind === "official-proxy" || kind === "enhanced"`, which is why plain artifact targets pass while navigation and address-context do not. ## Contract and boundary impact - Semantic owner: the named tool contracts provide prerequisites; `CapabilityInventory` projects them into `binary_session` availability. - CLI and MCP/tool catalog: advertised availability changes for address-context and navigation on closed sessions and on artifact/archive targets; the tools and their schemas are unchanged. - Provider, bridge, platform, Evidence, cleanup, package and installation impact: none identified. Generated/ignored catalogs are validated by `docs:check` and are not committed. ## Evidence and regression coverage - `src/application/CapabilityInventory.test.ts`: closed-session address and explicit-path APK availability, artifact and archive restrictions with per-mode navigation flags, and structured reasons rather than pinned remediation sentences. - `tests/boundary/mcp/sessionTargetRequirements.test.ts`: production stdio MCP regression observing both `binary_session` entries before and after the calls, the `target_unavailable` address error, the successful unsigned-ZIP APK call, and the sibling navigation prerequisite. - Boundary covered: contract to capability inventory to compacted MCP session status and actual calls. No lifecycle or provider execution changes. Real deep-analysis providers were not used; the APK call used a generated local ZIP and the production stdio server. ## Validation performed On rebased head `238543ff2` (base `8c857ce56`): - `npm run test:focused -- src/application/CapabilityInventory.test.ts tests/boundary/mcp/sessionTargetRequirements.test.ts tests/boundary/mcp/enhancedTools.limitations.test.ts` — 61 passed, including the two stdio MCP session-status cases. - `npm run check:fast` — passed (typecheck and lint). - `npm run format:check` — passed. - `git diff --check` — clean. The branch is two commits: the inventory/contract fix and its regressions. No real deep-analysis provider or native target was exercised. ## Compatibility, safety, and release No migration or execution behavior change. The status correction prevents clients from selecting an impossible address request or overlooking an available explicit-path APK inspection, and stops provider-configuration advice where the target must change instead. No real-provider claim is made. ## Author readiness Human owner: @akshad-exe - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] The human owner reviews and confirms the final diff and validation evidence. - [x] CI and remaining required checks pass or limitations are resolved and documented. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-05 01:47:56 +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#7823

No description provided.

Rows
Columns