morluto/rea

Fork 0

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

[PR #2971] [CLOSED] fix(reconstruction): check every selected replay environment #5526

New issue

Closed

opened 2026-10-31 01:07:20 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:07:20 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/2622-replay-environment-every-source

📝 Commits (1)

acb8e74 fix(reconstruction): check every selected replay environment

📊 Changes

2 files changed (+110 additions, -14 deletions)

View changed files

📝 src/domain/reconstructionVerification.test.ts (+105 -8)
📝 src/domain/reconstructionVerification.ts (+5 -6)

📄 Description

Summary

Fixes #2622.

A behavioral reconstruction claim checked the controlled-replay environment of only the first left source and the first right source. A process comparison can select several observations per side (left_evidence_ids / right_evidence_ids, accepted by comparisonSourceEvidenceSides and reachable through import_evidence_bundle). So a later observation recorded in a different environment, or with none, did not stop the claim from reporting pass. That made an incomparable claim look verified.

After this change, every selected source must carry an environment and all of them must be identical. Otherwise the existing Controlled replay environments are unavailable or incompatible. limitation is added and the claim stays unknown. Single-pair comparisons, which are what the public compare_process_captures producer emits today, behave exactly as before.

Problem and expected behavior

Smallest trigger: a rea.process-comparison record with two observations per side, where one of the later observations was captured in a different (or missing) environment:

Side	Observation 1	Observation 2
left	`env-a`	`env-b`
right	`env-a`	`env-a`

Before: verifyReconstruction returns pass.
Expected: unknown, with Controlled replay environments are unavailable or incompatible.

Violated invariant: a pass may only cover declared claims whose source observations are comparable (.agents/skills/reverse-engineer-anything/references/evidence-workflows.md). The environment check in src/domain/reconstructionVerification.ts looked up sides.left[0] and sides.right[0] only, so comparability was never established for the remaining sources.

Change and scope

authorityLimitations already receives sources, which is exactly the resolved, deduplicated union of both sides. Every id there has already been resolved, and a missing record throws earlier. The fix compares every source's environment against the first one, and flags null environments, in that same function. It removes the first-source-only lookup map instead of adding a second check next to it, so there is one owner for this rule.

The environment comparison still uses JSON.stringify, as before, so the only behavioural change is which sources are checked.

Not included: changing how environments are compared (canonical vs. serialized). That is unrelated to this bug.

Contribution scope: an ordinary correctness fix within an existing boundary, so a maintainer scope decision is not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: src/domain/reconstructionVerification.ts, in authorityLimitations (domain, pure).
Contribution scope fit and linked maintainer decision: bug fix within the existing reconstruction-verification boundary; not applicable.
CLI and MCP/tool-catalog contract: none. No schema, tool, or catalog change. verify_reconstruction / verify-reconstruction can now return unknown where it previously returned pass, but only for multi-source comparisons with mismatched or missing environments.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: stricter. A behavioral pass now requires every selected observation to share one environment. The limitation text is unchanged.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.

Expected effects:

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

(The verifier only reads in-memory Evidence; it launches nothing.)

Evidence and regression coverage

Tests added or updated: src/domain/reconstructionVerification.test.ts, test limits behavioral claims by every selected replay environment. It covers the restoration checks listed in the issue:
- a later left source with a different environment → unknown + limitation;
- a later right source with a different environment → unknown + limitation (so the result can't depend on source order);
- a later source with a missing (null) environment → unknown + limitation;
- a mismatch in the first source → unknown + limitation (existing behaviour kept);
- all four sources compatible → pass, without the environment limitation.
To support this, the source fixture takes an options object { captureResult?, replayEnvironment? } in place of its trailing captureResult parameter, which keeps it within max-params. A multiSourceProcessComparison helper builds left_evidence_ids / right_evidence_ids comparisons. The one existing caller that passed captureResult positionally is updated.
Base reproduction: with only the source change reverted, the new test fails (1 failed | 9 passed). Base returns pass for the later-mismatch cases. With the change, all 10 tests in the file pass.
User-visible CLI/MCP output: no new fields. Affected multi-source claims report status: "unknown" and include the existing environment limitation.
Remaining proof gaps: I didn't run an end-to-end import_evidence_bundle → verify_reconstruction MCP session with a hand-built multi-source bundle. The domain function changed here is the one those adapters call.

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

All run on macOS (arm64), Node 26.5.0, npm 11.17.0, with locked dependencies, rebased on origin/main at 7a6defab1:

npx vitest run --project domain src/domain/reconstructionVerification.test.ts: 10 passed. With the source fix reverted: 1 failed (the new test), 9 passed.
npx vitest run --project domain src/domain/reconstructionVerification.test.ts src/domain/reconstructionVerification.functionSources.test.ts: 78 passed.
npx vitest run --project domain: 162 files, 2503 passed, 3 skipped.
npx vitest run tests/boundary/mcp/reconstructionCoverageMcp.test.ts tests/boundary/mcp/reconstructionObligationLedgerMcp.test.ts tests/boundary/mcp/processEvidenceIdentityMcp.test.ts: passed.
npm run test:focused -- tests/boundary/cli/reconstructionObligationLedgerCli.test.ts src/domain/reconstructionVerification.test.ts: 11 passed.
npm run check:fast (typecheck + lint + module boundaries): passed, also via the pre-push hook.
npx oxfmt --check and npx oxlint on both changed files: clean.

Compatibility, safety, and release

Breaking changes or migration steps: none. The verifier is stricter only for multi-source behavioral comparisons whose environments differ or are missing, which were previously false passes.
Real Hopper/Ghidra, browser, or OS coverage: not applicable (pure domain logic, no provider).
Package or release metadata impact: none.
Security, privacy, process, or containment review: no process, filesystem, or network interaction.

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.

_{🔄 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/2971 **Author:** [@555aaditya](https://github.com/555aaditya) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/2622-replay-environment-every-source` --- ### 📝 Commits (1) - [`acb8e74`](https://github.com/morluto/rea/commit/acb8e74b716eb4451a729e671a1a60d2cd4e8b85) fix(reconstruction): check every selected replay environment ### 📊 Changes **2 files changed** (+110 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/reconstructionVerification.test.ts` (+105 -8) 📝 `src/domain/reconstructionVerification.ts` (+5 -6) </details> ### 📄 Description ## Summary Fixes #2622. A behavioral reconstruction claim checked the controlled-replay environment of only the **first** left source and the **first** right source. A process comparison can select several observations per side (`left_evidence_ids` / `right_evidence_ids`, accepted by `comparisonSourceEvidenceSides` and reachable through `import_evidence_bundle`). So a later observation recorded in a different environment, or with none, did not stop the claim from reporting `pass`. That made an incomparable claim look verified. After this change, every selected source must carry an environment and all of them must be identical. Otherwise the existing `Controlled replay environments are unavailable or incompatible.` limitation is added and the claim stays `unknown`. Single-pair comparisons, which are what the public `compare_process_captures` producer emits today, behave exactly as before. ## Problem and expected behavior **Smallest trigger:** a `rea.process-comparison` record with two observations per side, where one of the later observations was captured in a different (or missing) environment: | Side | Observation 1 | Observation 2 | |---|---|---| | left | `env-a` | `env-b` | | right | `env-a` | `env-a` | - **Before:** `verifyReconstruction` returns `pass`. - **Expected:** `unknown`, with `Controlled replay environments are unavailable or incompatible.` **Violated invariant:** a `pass` may only cover declared claims whose source observations are comparable (`.agents/skills/reverse-engineer-anything/references/evidence-workflows.md`). The environment check in `src/domain/reconstructionVerification.ts` looked up `sides.left[0]` and `sides.right[0]` only, so comparability was never established for the remaining sources. ## Change and scope `authorityLimitations` already receives `sources`, which is exactly the resolved, deduplicated union of both sides. Every id there has already been resolved, and a missing record throws earlier. The fix compares every source's environment against the first one, and flags `null` environments, in that same function. It removes the first-source-only lookup map instead of adding a second check next to it, so there is one owner for this rule. The environment comparison still uses `JSON.stringify`, as before, so the only behavioural change is *which* sources are checked. Not included: changing how environments are compared (canonical vs. serialized). That is unrelated to this bug. Contribution scope: an ordinary correctness fix within an existing boundary, so a maintainer scope decision is not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/domain/reconstructionVerification.ts`, in `authorityLimitations` (domain, pure). - Contribution scope fit and linked maintainer decision: bug fix within the existing reconstruction-verification boundary; not applicable. - CLI and MCP/tool-catalog contract: none. No schema, tool, or catalog change. `verify_reconstruction` / `verify-reconstruction` can now return `unknown` where it previously returned `pass`, but only for multi-source comparisons with mismatched or missing environments. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: stricter. A behavioral `pass` now requires every selected observation to share one environment. The limitation text is unchanged. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. (The verifier only reads in-memory Evidence; it launches nothing.) ## Evidence and regression coverage - **Tests added or updated:** `src/domain/reconstructionVerification.test.ts`, test `limits behavioral claims by every selected replay environment`. It covers the restoration checks listed in the issue: - a later **left** source with a different environment → `unknown` + limitation; - a later **right** source with a different environment → `unknown` + limitation (so the result can't depend on source order); - a later source with a **missing** (`null`) environment → `unknown` + limitation; - a mismatch in the **first** source → `unknown` + limitation (existing behaviour kept); - all four sources compatible → `pass`, without the environment limitation. To support this, the `source` fixture takes an options object `{ captureResult?, replayEnvironment? }` in place of its trailing `captureResult` parameter, which keeps it within `max-params`. A `multiSourceProcessComparison` helper builds `left_evidence_ids` / `right_evidence_ids` comparisons. The one existing caller that passed `captureResult` positionally is updated. - **Base reproduction:** with only the source change reverted, the new test fails (`1 failed | 9 passed`). Base returns `pass` for the later-mismatch cases. With the change, all 10 tests in the file pass. - **User-visible CLI/MCP output:** no new fields. Affected multi-source claims report `status: "unknown"` and include the existing environment limitation. - **Remaining proof gaps:** I didn't run an end-to-end `import_evidence_bundle` → `verify_reconstruction` MCP session with a hand-built multi-source bundle. The domain function changed here is the one those adapters call. 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 All run on macOS (arm64), Node 26.5.0, npm 11.17.0, with locked dependencies, rebased on `origin/main` at `7a6defab1`: - `npx vitest run --project domain src/domain/reconstructionVerification.test.ts`: 10 passed. With the source fix reverted: 1 failed (the new test), 9 passed. - `npx vitest run --project domain src/domain/reconstructionVerification.test.ts src/domain/reconstructionVerification.functionSources.test.ts`: 78 passed. - `npx vitest run --project domain`: 162 files, 2503 passed, 3 skipped. - `npx vitest run tests/boundary/mcp/reconstructionCoverageMcp.test.ts tests/boundary/mcp/reconstructionObligationLedgerMcp.test.ts tests/boundary/mcp/processEvidenceIdentityMcp.test.ts`: passed. - `npm run test:focused -- tests/boundary/cli/reconstructionObligationLedgerCli.test.ts src/domain/reconstructionVerification.test.ts`: 11 passed. - `npm run check:fast` (typecheck + lint + module boundaries): passed, also via the pre-push hook. - `npx oxfmt --check` and `npx oxlint` on both changed files: clean. ## Compatibility, safety, and release - Breaking changes or migration steps: none. The verifier is stricter only for multi-source behavioral comparisons whose environments differ or are missing, which were previously false passes. - Real Hopper/Ghidra, browser, or OS coverage: not applicable (pure domain logic, no provider). - Package or release metadata impact: none. - Security, privacy, process, or containment review: no process, filesystem, or network interaction. ## 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:07:20 +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#5526

No description provided.

Rows
Columns