morluto/rea

Fork 0

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

[GH-ISSUE #2009] binary_session capability sets label the selected provider with its unresolved identity #667

New issue

Closed

opened 2026-10-15 18:27:07 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-15 18:27:07 +00:00

Owner

Originally created by @kaoru0822-kitauji on GitHub (Oct 12, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/2009

Observed behavior

After a deep provider is selected, binary_session reports two different identities for the same provider. Ghidra and Hopper publish a static identity with version: null and resolve their concrete version during profile resolution.

analysis_provider_binding.provider and the selected analysis_provider_candidates[].provider carry the resolved identity, e.g. { id: "ghidra", version: "12.1.4" }.
The capability sets grouped by #1988, both top-level capabilities[].provider and the selected candidate's capabilities[].provider, still carry the static identity, { id: "ghidra", version: null }.

A consumer that joins capability sets to the binding or candidate by provider identity therefore sees two providers, and it loses the known version for the operations the selected provider owns. Codex flagged this on #1988 (review comment 4243566041), and the comment was not addressed before merge.

Reproduction

tests/composition/analysis-sessions/providerBinding.test.ts uses a deep provider fixture with the same shape: static identity version: null, resolved profile version: "1". Open a target with providerId: "alpha" and read session.status(). The selected candidate reports provider.version: "1", but its capability set reports provider.version: null.

Cause

AnalysisProviderRegistry.selectedResult replaces the selected candidate's provider with the resolved profile identity but leaves its capability descriptors on the static identity. SessionProviderRouter builds route capabilities from binding.provider.capabilities(), which is also static. BinarySession.providerIdentity and DirectAnalysis.providerIdentityForRoute already compensate by substituting the profile identity downstream, but the status projection does not.

Expected behavior

Every capability fact for the selected provider carries the same resolved identity as the binding.

Originally created by @kaoru0822-kitauji on GitHub (Oct 12, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2009 ## Observed behavior After a deep provider is selected, `binary_session` reports two different identities for the same provider. Ghidra and Hopper publish a static identity with `version: null` and resolve their concrete version during profile resolution. - `analysis_provider_binding.provider` and the selected `analysis_provider_candidates[].provider` carry the resolved identity, e.g. `{ id: "ghidra", version: "12.1.4" }`. - The capability sets grouped by #1988, both top-level `capabilities[].provider` and the selected candidate's `capabilities[].provider`, still carry the static identity, `{ id: "ghidra", version: null }`. A consumer that joins capability sets to the binding or candidate by provider identity therefore sees two providers, and it loses the known version for the operations the selected provider owns. Codex flagged this on #1988 (review comment 4243566041), and the comment was not addressed before merge. ## Reproduction `tests/composition/analysis-sessions/providerBinding.test.ts` uses a deep provider fixture with the same shape: static identity `version: null`, resolved profile `version: "1"`. Open a target with `providerId: "alpha"` and read `session.status()`. The selected candidate reports `provider.version: "1"`, but its capability set reports `provider.version: null`. ## Cause `AnalysisProviderRegistry.selectedResult` replaces the selected candidate's `provider` with the resolved profile identity but leaves its capability descriptors on the static identity. `SessionProviderRouter` builds route capabilities from `binding.provider.capabilities()`, which is also static. `BinarySession.providerIdentity` and `DirectAnalysis.providerIdentityForRoute` already compensate by substituting the profile identity downstream, but the status projection does not. ## Expected behavior Every capability fact for the selected provider carries the same resolved identity as the binding.

bsakaguchi

2026-10-15 18:27:07 +00:00

closed this issue
added the
bug
label

bsakaguchi commented

2026-10-15 18:27:08 +00:00

Author

Owner

@jimpablo commented on GitHub (Oct 12, 2026):

I'd like to work on this.

The two-identity split matches the source as I read it: AnalysisProviderRegistry.selectedResult swaps the selected candidate's provider for the resolved profile identity, but its capability descriptors keep the static identity, and SessionProviderRouter builds route capabilities from binding.provider.capabilities(), which never saw profile resolution. BinarySession.providerIdentity and DirectAnalysis.providerIdentityForRoute then substitute the resolved identity again downstream — three places knowing about one fact.

Planned direction, per the producing-representation rule:

Resolve the identity once at the producing boundary — the registry's selected result (and the router's route capabilities) carry the resolved identity for the selected provider, so every capability fact joins to the binding by the same identity.
Then remove the downstream providerIdentity substitutions in BinarySession and DirectAnalysis as superseded representations, with their consumers' expectations updated.
Composition tests through the existing deep-provider fixture shape in tests/composition/analysis-sessions/providerBinding.test.ts (static version: null, resolved "1"), asserting one identity across analysis_provider_binding, the selected candidate, and both capability-set groupings; plus whatever session-status boundary test pins the projected output.

One question before I start: should the selected candidate's capability descriptors be re-labeled with the resolved identity (keeping a single identity per fact), or is a resolved-vs-static distinction in capabilities ever load-bearing for a consumer you know of? I'll read the #1988 Codex thread before implementing either way.

@jimpablo commented on GitHub (Oct 12, 2026): I'd like to work on this. The two-identity split matches the source as I read it: `AnalysisProviderRegistry.selectedResult` swaps the selected candidate's `provider` for the resolved profile identity, but its capability descriptors keep the static identity, and `SessionProviderRouter` builds route capabilities from `binding.provider.capabilities()`, which never saw profile resolution. `BinarySession.providerIdentity` and `DirectAnalysis.providerIdentityForRoute` then substitute the resolved identity again downstream — three places knowing about one fact. Planned direction, per the producing-representation rule: - Resolve the identity once at the producing boundary — the registry's selected result (and the router's route capabilities) carry the resolved identity for the selected provider, so every capability fact joins to the binding by the same identity. - Then remove the downstream `providerIdentity` substitutions in `BinarySession` and `DirectAnalysis` as superseded representations, with their consumers' expectations updated. - Composition tests through the existing deep-provider fixture shape in `tests/composition/analysis-sessions/providerBinding.test.ts` (static `version: null`, resolved `"1"`), asserting one identity across `analysis_provider_binding`, the selected candidate, and both capability-set groupings; plus whatever session-status boundary test pins the projected output. One question before I start: should the selected candidate's capability descriptors be re-labeled with the resolved identity (keeping a single identity per fact), or is a resolved-vs-static distinction in capabilities ever load-bearing for a consumer you know of? I'll read the #1988 Codex thread before implementing either way.

bsakaguchi commented

2026-10-15 18:27:08 +00:00

Author

Owner

@jimpablo commented on GitHub (Oct 12, 2026):

PR #2013 it is — withdrawing my claim. I had only read the surrounding code, so nothing is lost; your root-cause notes in the issue and the #1988 review thread were exactly where I'd landed too. I'll take the review on #2013 instead — the context is fresh on my side.

@jimpablo commented on GitHub (Oct 12, 2026): PR #2013 it is — withdrawing my claim. I had only read the surrounding code, so nothing is lost; your root-cause notes in the issue and the #1988 review thread were exactly where I'd landed too. I'll take the review on #2013 instead — the context is fresh on my side.

bsakaguchi referenced this issue

2026-10-16 21:14:56 +00:00

[PR #667] [MERGED] fix(dotnet): preserve leading Unicode content in metadata strings #2150

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

No description provided.

Rows
Columns