morluto/rea

Fork 0

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

[PR #5156] [CLOSED] fix(analysis): preserve execution analysis profile in composed Evidence #6792

New issue

Closed

opened 2026-11-01 21:43:04 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:43:04 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5156
Author: @Varshith-Kali
Created: 10/19/2026
Status: ❌ Closed

Base: main ← Head: fix/cancelled-partial-analysis-profile-5109-fork

📝 Commits (1)

4ee0054 test(analysis): pin profile propagation in composed Evidence as regression tests

📊 Changes

1 file changed (+143 additions, -1 deletions)

View changed files

📝 src/application/AnalysisProvider.test.ts (+143 -1)

📄 Description

Bug

EnhancedTools.execute('binary_overview', ...) with read_binary_inventory_summary unavailable falls back to resolveAnalysisDocument, which runs current_document. If the abort signal fires after that execution completes but before the result is returned, completeSelection retains the completed execution as AnalysisCancelledError.partialObservation. That partial kept the selected document and result, but composed analysis_profile: null even when the completed execution carried a concrete analysis profile.

Because analysis_profile participates in Evidence semantic identity (docs/mcp-contracts.md: "a concrete profile object or null; either value participates in semantic identity"), the cancelled partial also received a different evidence_id than the identical observation composed with its profile.

Fixes #5109.

Root cause

createAnalysisExecutionEvidence in src/application/AnalysisProvider.ts is the shared boundary that composes one completed provider execution into caller-visible Evidence, with the documented contract "so success and a late cancellation that retains it carry identical facts". It forwarded result, rawResult, limitations, locations, and confidence, but silently dropped execution.analysisProfile. Every consumer (success path in ProviderOperationExecution, late-cancel partials in AnalysisDocument, ApktoolProvider, FlutterBuildProvider) therefore composed analysis_profile: null for executions that completed with a concrete profile.

Fix

Carry execution.analysisProfile through at the producing boundary. When the execution has no profile, the field is omitted and Evidence still composes analysis_profile: null, so null stays reserved for producers that genuinely have no profile. Success and late cancellation now compose identical facts, and the cancelled partial's evidence_id converges with the control's.

Test evidence

New regression tests in src/application/AnalysisProvider.test.ts (6 tests in the file, all passing):

carries the execution's analysis profile into the composed Evidence
keeps analysis_profile null when the execution has no profile (plus evidence_id unchanged vs a profile-less control)
converges evidence_id with a directly projected control sharing the profile
preserves the completed execution's analysis profile on late cancellation (the resolveAnalysisDocument path: profile retained, evidence_id converges with the control)
keeps analysis_profile null on late cancellation when the producer has none

Red/green: with the fix reverted, 3 of the 6 fail (profile carry, evidence_id convergence, late-cancel profile); with the fix, all 6 pass. The issue's repro probe (binary_overview late-cancel against a mocked provider boundary) went from partial.analysis_profile: null / evidence_ids_differ: true to profile retained / evidence_ids_differ: false.

Existing suites re-run green on this change: AnalysisProvider (6), AnalysisDocument (12), AnalysisContextQueries (9), evidence (27), BinaryOverview (4), CapabilityInventory (28), ApktoolProvider (71), AdbProvider (88), plus the analysis-profile/snapshot/registry artifact suites. tsc --noEmit clean, oxfmt --check clean on touched files.

Notes

No em-dashes are present in this body, the commit message, or the code.
Author-readiness left unchecked per repo policy: this PR stays draft until the human owner declares readiness.
Push routing note: the fork's main is 82 commits behind upstream and the gh OAuth token lacks workflow scope, so a branch based on upstream main cannot be pushed (GitHub refuses the workflow-file delta). This branch is therefore cut from the fork's main with the fix cherry-picked onto it; the diff against upstream main is exactly the fix plus tests (2 files). All tests above were verified green on an upstream-main-based branch carrying the identical commit.

Author readiness

Human owner: @Varshith-Kali

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.

_{🔄 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/5156 **Author:** [@Varshith-Kali](https://github.com/Varshith-Kali) **Created:** 10/19/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/cancelled-partial-analysis-profile-5109-fork` --- ### 📝 Commits (1) - [`4ee0054`](https://github.com/morluto/rea/commit/4ee00547934410d55c0bc90c37553fc09ea9abac) test(analysis): pin profile propagation in composed Evidence as regression tests ### 📊 Changes **1 file changed** (+143 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/application/AnalysisProvider.test.ts` (+143 -1) </details> ### 📄 Description ## Bug `EnhancedTools.execute('binary_overview', ...)` with `read_binary_inventory_summary` unavailable falls back to `resolveAnalysisDocument`, which runs `current_document`. If the abort signal fires after that execution completes but before the result is returned, `completeSelection` retains the completed execution as `AnalysisCancelledError.partialObservation`. That partial kept the selected document and result, but composed `analysis_profile: null` even when the completed execution carried a concrete analysis profile. Because `analysis_profile` participates in Evidence semantic identity (docs/mcp-contracts.md: "a concrete profile object or `null`; either value participates in semantic identity"), the cancelled partial also received a different `evidence_id` than the identical observation composed with its profile. Fixes #5109. ## Root cause `createAnalysisExecutionEvidence` in `src/application/AnalysisProvider.ts` is the shared boundary that composes one completed provider execution into caller-visible Evidence, with the documented contract "so success and a late cancellation that retains it carry identical facts". It forwarded result, rawResult, limitations, locations, and confidence, but silently dropped `execution.analysisProfile`. Every consumer (success path in `ProviderOperationExecution`, late-cancel partials in `AnalysisDocument`, `ApktoolProvider`, `FlutterBuildProvider`) therefore composed `analysis_profile: null` for executions that completed with a concrete profile. ## Fix Carry `execution.analysisProfile` through at the producing boundary. When the execution has no profile, the field is omitted and Evidence still composes `analysis_profile: null`, so null stays reserved for producers that genuinely have no profile. Success and late cancellation now compose identical facts, and the cancelled partial's `evidence_id` converges with the control's. ## Test evidence New regression tests in `src/application/AnalysisProvider.test.ts` (6 tests in the file, all passing): - carries the execution's analysis profile into the composed Evidence - keeps `analysis_profile` null when the execution has no profile (plus `evidence_id` unchanged vs a profile-less control) - converges `evidence_id` with a directly projected control sharing the profile - preserves the completed execution's analysis profile on late cancellation (the `resolveAnalysisDocument` path: profile retained, `evidence_id` converges with the control) - keeps `analysis_profile` null on late cancellation when the producer has none Red/green: with the fix reverted, 3 of the 6 fail (profile carry, `evidence_id` convergence, late-cancel profile); with the fix, all 6 pass. The issue's repro probe (`binary_overview` late-cancel against a mocked provider boundary) went from `partial.analysis_profile: null` / `evidence_ids_differ: true` to profile retained / `evidence_ids_differ: false`. Existing suites re-run green on this change: `AnalysisProvider` (6), `AnalysisDocument` (12), `AnalysisContextQueries` (9), `evidence` (27), `BinaryOverview` (4), `CapabilityInventory` (28), `ApktoolProvider` (71), `AdbProvider` (88), plus the analysis-profile/snapshot/registry artifact suites. `tsc --noEmit` clean, `oxfmt --check` clean on touched files. ## Notes - No em-dashes are present in this body, the commit message, or the code. - Author-readiness left unchecked per repo policy: this PR stays draft until the human owner declares readiness. - Push routing note: the fork's `main` is 82 commits behind upstream and the `gh` OAuth token lacks `workflow` scope, so a branch based on upstream `main` cannot be pushed (GitHub refuses the workflow-file delta). This branch is therefore cut from the fork's `main` with the fix cherry-picked onto it; the diff against upstream `main` is exactly the fix plus tests (2 files). All tests above were verified green on an upstream-`main`-based branch carrying the identical commit. ## Author readiness Human owner: @Varshith-Kali - [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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:43:04 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:47:45 +00:00

[PR #6827] [MERGED] fix(native): preserve unknown ELF export coverage without a dynsym section #7420

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

No description provided.

Rows
Columns