morluto/rea

Fork 0

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

[PR #6367] fix(reconstruction)!: require distinct contents for fixture controls #7313

New issue

Open

opened 2026-11-01 21:46:57 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:46:57 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6367
Author: @Momilijaz96
Created: 10/22/2026
Status: 🔄 Open

Base: main ← Head: docs/4396-output-fixture-controls

📝 Commits (10+)

688d918 feat(reconstruction)!: require failing verifier controls
9472804 refactor(reconstruction): relocate proof authority to domain
50edc2f refactor(reconstruction): relocate control assessment to domain
2129aff fix(reconstruction): authenticate controls at every closure boundary
900154f Merge branch 'main' into fix/reconstruction-verifier-negative-controls
5848ce1 fix(reconstruction): preserve verifier controls in shared fixture
837ca81 docs(reconstruction): demonstrate copied-output verifier controls
15d2975 chore: include prerequisite negative-control implementation from #4991
feb6ab4 fix(reconstruction)!: require distinct contents for fixture controls
20c9005 fix(reconstruction): reassess reachable dependency proofs

📊 Changes

20 files changed (+2184 additions, -127 deletions)

View changed files

📝 docs/reconstruction-obligation-ledgers.md (+175 -1)
📝 src/application/ReconstructionObligationLedger.authority.test.ts (+50 -1)
📝 src/application/ReconstructionObligationLedger.closure.test.ts (+625 -3)
📝 src/application/ReconstructionObligationLedger.fixture.ts (+111 -48)
➕ src/application/ReconstructionObligationLedger.fixtureControls.test.ts (+143 -0)
📝 src/application/ReconstructionObligationLedgerEvaluation.ts (+9 -55)
📝 src/application/ReconstructionObligationLedgerProjection.ts (+6 -1)
📝 src/application/ReconstructionObligationOriginalCases.ts (+1 -1)
📝 src/contracts/applicationToolContracts.ts (+1 -1)
📝 src/contracts/reconstructionReadinessExample.ts (+71 -3)
📝 src/domain/reconstructionObligationAuthority.ts (+17 -3)
📝 src/domain/reconstructionObligationLedgerIntegrity.ts (+6 -1)
📝 src/domain/reconstructionObligationLedgerSchemas.ts (+27 -0)
➕ src/domain/reconstructionObligationVerifier.ts (+73 -0)
➕ src/domain/reconstructionObligationVerifierControls.ts (+210 -0)
📝 src/domain/reconstructionReadiness.test.ts (+289 -0)
➕ src/domain/reconstructionReadinessDependencies.test.ts (+216 -0)
📝 src/domain/reconstructionReadinessIntegrityFindings.ts (+69 -0)
📝 tests/boundary/cli/reconstructionObligationLedgerCli.test.ts (+34 -0)
📝 tests/boundary/mcp/reconstructionObligationLedgerMcp.test.ts (+51 -9)

📄 Description

Summary

A fixture named corrupted.json can contain exactly the same data as original.json. A different name alone does not establish a negative control.

This change requires content fingerprints in the positive and negative verifier Evidence before a fixture control can support closure:

Recorded fixture contents	Result
Original and control have different names but identical contents	Keep the obligation open
Fingerprints are missing or invalid	Keep the obligation open
Contents differ and the existing verifier proof checks pass	Allow the control to support closure

Refs #4396.

Dependency and review scope

Depends on the still-open #4991. Its commits retain their original authorship and are included as a prerequisite, so the full diff currently includes that work. This PR does not claim that implementation as a new contribution. Our additional content check is in this focused commit; the earlier copied-output example and subsequent review fixes are also included.

The prerequisite and its closure-policy decision must be resolved before merge. Once #4991 lands, the branch can be synchronized to leave the follow-up diff.

Current integration blocker (25 October 2026): main now requires reconstruction proof records to bind claims to an implementation owner and obligation version (reconstructionObligationProofSchema). The included #4991 implementation and this follow-up still use the previous ID-array proof records. A trial merge with current main found eight content conflicts, including the proof assessor, ledger fixture, and readiness tests. The merge was aborted without changing the published branch: taking either side unchanged would lose negative-control validation or the newer owner/revision proof checks. #4991 itself is also open and conflicting. Its closure policy still awaits a maintainer decision. Adapt the prerequisite to the current proof contract before synchronizing this follow-up; retain owner/revision validation and the negative-control checks together.

Change and scope

Record normalized_result.fixture_sha256s: fixture ID → SHA-256 of the exact consumed contents.
Require the selected control's digest and digests for every bound or additionally reported positive fixture. Reject a control matching any positive fixture, even under a different name.
Reuse the shared verifier assessor for both ledger construction and historical readiness. Saved historical ledgers remain unchanged.
Historical readiness reassesses every dependency reachable from required obligations, including optional and indirect dependencies; missing or open dependencies prevent readiness. Shared dependencies and cycles are visited once, and unrelated optional obligations do not affect readiness.
The ledger contract rejects duplicate obligation IDs before readiness indexes them, so a later entry cannot shadow invalid required or dependency proof.
Check fixture contents before diagnosing a passing control as a vacuous verifier. An identical renamed fixture is an invalid control, so readiness remains unknown instead of blaming the verifier.
Include a runnable Python example using separate original and corrupted data copies. No defective software build is required for this comparison-level check.

Contract and boundary impact

Semantic owner: shared domain verifier-control assessment.
Contribution scope: deterministic validation of supplied Evidence; no new target execution, provider, or mutation capability. Separate target-capability scope approval is not applicable; prerequisite closure-policy discussion remains pending in #4991.
CLI/MCP: shared behavior; invalid fixture identities report invalid-control-candidate and keep closure open. No new command or manifest fields.
Evidence contract: passing and control verifier results must carry fixture content digests. Existing manifest and stored-ledger schemas remain parseable.
Providers, platforms, target selection, authority, cleanup: no new interaction or authority. Artifact controls retain the prerequisite's behavior.
Generated metadata/package impact: the authored readiness example is updated; generated outputs are not committed. No dependency changes.

Expected effects:

Read existing files or captured data.

Evidence and regression coverage

Executed synthetic-Evidence tests, including hashes of hello ordinary and CORRUPTED. Before the implementation, eight rejection cases incorrectly returned ready; afterward they stay open. Coverage includes renamed identical contents, missing/wrong/malformed digest entries, partially identified positives, a matching later positive fixture, and additional fixtures reported only by the verifier.

Historical readiness rejects invalid controls without changing the saved ledger. CLI and MCP tests both exercise changed and identical-content fixtures. The Python example passes, while a temporary comparator weakened to ignore contents is detected.

Review regressions reproduced six incorrect readiness passes for dependency cases and an incorrect failure for a passing identical fixture; all seven now pass their assertions. Valid dependency proof remains ready even with an unrelated invalid optional obligation. The existing ledger-chain test now gives each node valid verifier proof: temporarily reducing propagation to one pass makes this test fail, and restoring production behavior makes it pass.

Two further historical-ledger regressions reproduce duplicate IDs hiding invalid required or dependency controls. Both previously reached readiness; the ledger contract now rejects their ambiguous identity before assessment.

Limits: these are deterministic validation and synthetic comparison tests, not live Docker/provider evidence. Fingerprints establish recorded content difference, not defect relevance, honest capture, equal execution settings, or complete reconstruction correctness. No full repository test suite or real-provider lane was run locally; CI owns broader coverage.

Validation performed

npm run test:focused -- src/domain/reconstructionReadinessDependencies.test.ts src/domain/reconstructionReadiness.test.ts src/application/ReconstructionObligationLedger.authority.test.ts src/application/ReconstructionObligationLedger.fixtureControls.test.ts src/application/ReconstructionObligationLedger.closure.test.ts src/application/ReconstructionObligationCandidates.test.ts src/domain/reconstructionObligationLedgerSchemas.test.ts — 89 passed.
npm run test:focused -- tests/boundary/cli/reconstructionObligationLedgerCli.test.ts tests/boundary/mcp/reconstructionObligationLedgerMcp.test.ts — 16 passed.
npm run check — passed all six tasks and architecture guards with Node 24.18.0 / npm 11.16.0.
npm run docs:check — passed generated-document checks and all nine README conformance tests.
npm run verify:test-discovery — all 2,063 test files discovered exactly once.
Extracted documented Python example — passed; the temporary content-blind variant failed as expected.
git diff --check and pre-commit formatting/lint hooks — passed.

Compatibility and migration

Breaking closure requirement: fixture controls accepted by #4991 using names alone need content digests. Re-record positive/control Evidence with fixture_sha256s, update the Evidence references, and rebuild the ledger. Missing digests keep new closure open and block historical readiness; stored ledger bytes are not rewritten. Hash contents using the same representation for both runs, never filenames.

No private data, new target execution, installation changes, or new runtime dependencies are introduced.

Author readiness

Human owner: @Momilijaz96

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

Resolve the #4991 dependency and confirm the final closure policy with maintainers.
Adapt negative controls to main's owner/revision-bound proof contract, synchronize the branch, and review the final diff and current CI results.

_{🔄 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/6367 **Author:** [@Momilijaz96](https://github.com/Momilijaz96) **Created:** 10/22/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `docs/4396-output-fixture-controls` --- ### 📝 Commits (10+) - [`688d918`](https://github.com/morluto/rea/commit/688d918982c4c1091da554d0c10127a3b85e88c3) feat(reconstruction)!: require failing verifier controls - [`9472804`](https://github.com/morluto/rea/commit/94728045e7537462fa4863749f74c31b3471e44f) refactor(reconstruction): relocate proof authority to domain - [`50edc2f`](https://github.com/morluto/rea/commit/50edc2f04e1e3e4fe4b3665a3188be85b584aa87) refactor(reconstruction): relocate control assessment to domain - [`2129aff`](https://github.com/morluto/rea/commit/2129affed275a784e650ac765d83975e4172b491) fix(reconstruction): authenticate controls at every closure boundary - [`900154f`](https://github.com/morluto/rea/commit/900154fbee16e526f0e3a3c62c4af9ec8bdc4c78) Merge branch 'main' into fix/reconstruction-verifier-negative-controls - [`5848ce1`](https://github.com/morluto/rea/commit/5848ce132c659928a66d9aff96aaf9a0a3dee43f) fix(reconstruction): preserve verifier controls in shared fixture - [`837ca81`](https://github.com/morluto/rea/commit/837ca81973cd2617c5d8087163a3d45089fdb6fa) docs(reconstruction): demonstrate copied-output verifier controls - [`15d2975`](https://github.com/morluto/rea/commit/15d29756a4325d08430ad6937fb28785ff778c92) chore: include prerequisite negative-control implementation from #4991 - [`feb6ab4`](https://github.com/morluto/rea/commit/feb6ab4a8a21c4f5451746b5dddc5ccc1b2c3202) fix(reconstruction)!: require distinct contents for fixture controls - [`20c9005`](https://github.com/morluto/rea/commit/20c9005a92566c6a50e9d7b2c918c081ec899d47) fix(reconstruction): reassess reachable dependency proofs ### 📊 Changes **20 files changed** (+2184 additions, -127 deletions) <details> <summary>View changed files</summary> 📝 `docs/reconstruction-obligation-ledgers.md` (+175 -1) 📝 `src/application/ReconstructionObligationLedger.authority.test.ts` (+50 -1) 📝 `src/application/ReconstructionObligationLedger.closure.test.ts` (+625 -3) 📝 `src/application/ReconstructionObligationLedger.fixture.ts` (+111 -48) ➕ `src/application/ReconstructionObligationLedger.fixtureControls.test.ts` (+143 -0) 📝 `src/application/ReconstructionObligationLedgerEvaluation.ts` (+9 -55) 📝 `src/application/ReconstructionObligationLedgerProjection.ts` (+6 -1) 📝 `src/application/ReconstructionObligationOriginalCases.ts` (+1 -1) 📝 `src/contracts/applicationToolContracts.ts` (+1 -1) 📝 `src/contracts/reconstructionReadinessExample.ts` (+71 -3) 📝 `src/domain/reconstructionObligationAuthority.ts` (+17 -3) 📝 `src/domain/reconstructionObligationLedgerIntegrity.ts` (+6 -1) 📝 `src/domain/reconstructionObligationLedgerSchemas.ts` (+27 -0) ➕ `src/domain/reconstructionObligationVerifier.ts` (+73 -0) ➕ `src/domain/reconstructionObligationVerifierControls.ts` (+210 -0) 📝 `src/domain/reconstructionReadiness.test.ts` (+289 -0) ➕ `src/domain/reconstructionReadinessDependencies.test.ts` (+216 -0) 📝 `src/domain/reconstructionReadinessIntegrityFindings.ts` (+69 -0) 📝 `tests/boundary/cli/reconstructionObligationLedgerCli.test.ts` (+34 -0) 📝 `tests/boundary/mcp/reconstructionObligationLedgerMcp.test.ts` (+51 -9) </details> ### 📄 Description ## Summary A fixture named `corrupted.json` can contain exactly the same data as `original.json`. A different name alone does not establish a negative control. This change requires content fingerprints in the positive and negative verifier Evidence before a fixture control can support closure: | Recorded fixture contents | Result | | --- | --- | | Original and control have different names but identical contents | Keep the obligation open | | Fingerprints are missing or invalid | Keep the obligation open | | Contents differ and the existing verifier proof checks pass | Allow the control to support closure | Refs #4396. ## Dependency and review scope **Depends on the still-open #4991.** Its commits retain their original authorship and are included as a prerequisite, so the full diff currently includes that work. This PR does not claim that implementation as a new contribution. Our additional content check is in [this focused commit](https://github.com/morluto/rea/commit/feb6ab4a8a21c4f5451746b5dddc5ccc1b2c3202); the earlier copied-output example and subsequent review fixes are also included. The prerequisite and its closure-policy decision must be resolved before merge. Once #4991 lands, the branch can be synchronized to leave the follow-up diff. **Current integration blocker (25 October 2026):** `main` now requires reconstruction proof records to bind claims to an implementation owner and obligation version (`reconstructionObligationProofSchema`). The included #4991 implementation and this follow-up still use the previous ID-array proof records. A trial merge with current `main` found eight content conflicts, including the proof assessor, ledger fixture, and readiness tests. The merge was aborted without changing the published branch: taking either side unchanged would lose negative-control validation or the newer owner/revision proof checks. #4991 itself is also open and conflicting. Its closure policy still awaits a maintainer decision. Adapt the prerequisite to the current proof contract before synchronizing this follow-up; retain owner/revision validation and the negative-control checks together. ## Change and scope - Record `normalized_result.fixture_sha256s`: fixture ID → SHA-256 of the exact consumed contents. - Require the selected control's digest and digests for every bound or additionally reported positive fixture. Reject a control matching any positive fixture, even under a different name. - Reuse the shared verifier assessor for both ledger construction and historical readiness. Saved historical ledgers remain unchanged. - Historical readiness reassesses every dependency reachable from required obligations, including optional and indirect dependencies; missing or open dependencies prevent readiness. Shared dependencies and cycles are visited once, and unrelated optional obligations do not affect readiness. - The ledger contract rejects duplicate obligation IDs before readiness indexes them, so a later entry cannot shadow invalid required or dependency proof. - Check fixture contents before diagnosing a passing control as a vacuous verifier. An identical renamed fixture is an invalid control, so readiness remains unknown instead of blaming the verifier. - Include a runnable Python example using separate original and corrupted data copies. No defective software build is required for this comparison-level check. ## Contract and boundary impact - Semantic owner: shared domain verifier-control assessment. - Contribution scope: deterministic validation of supplied Evidence; no new target execution, provider, or mutation capability. Separate target-capability scope approval is not applicable; prerequisite closure-policy discussion remains pending in #4991. - CLI/MCP: shared behavior; invalid fixture identities report `invalid-control-candidate` and keep closure open. No new command or manifest fields. - Evidence contract: passing and control verifier results must carry fixture content digests. Existing manifest and stored-ledger schemas remain parseable. - Providers, platforms, target selection, authority, cleanup: no new interaction or authority. Artifact controls retain the prerequisite's behavior. - Generated metadata/package impact: the authored readiness example is updated; generated outputs are not committed. No dependency changes. Expected effects: - [x] Read existing files or captured data. ## Evidence and regression coverage Executed synthetic-Evidence tests, including hashes of `hello ordinary` and `CORRUPTED`. Before the implementation, eight rejection cases incorrectly returned `ready`; afterward they stay `open`. Coverage includes renamed identical contents, missing/wrong/malformed digest entries, partially identified positives, a matching later positive fixture, and additional fixtures reported only by the verifier. Historical readiness rejects invalid controls without changing the saved ledger. CLI and MCP tests both exercise changed and identical-content fixtures. The Python example passes, while a temporary comparator weakened to ignore contents is detected. Review regressions reproduced six incorrect readiness passes for dependency cases and an incorrect failure for a passing identical fixture; all seven now pass their assertions. Valid dependency proof remains ready even with an unrelated invalid optional obligation. The existing ledger-chain test now gives each node valid verifier proof: temporarily reducing propagation to one pass makes this test fail, and restoring production behavior makes it pass. Two further historical-ledger regressions reproduce duplicate IDs hiding invalid required or dependency controls. Both previously reached readiness; the ledger contract now rejects their ambiguous identity before assessment. Limits: these are deterministic validation and synthetic comparison tests, not live Docker/provider evidence. Fingerprints establish recorded content difference, not defect relevance, honest capture, equal execution settings, or complete reconstruction correctness. No full repository test suite or real-provider lane was run locally; CI owns broader coverage. ## Validation performed - `npm run test:focused -- src/domain/reconstructionReadinessDependencies.test.ts src/domain/reconstructionReadiness.test.ts src/application/ReconstructionObligationLedger.authority.test.ts src/application/ReconstructionObligationLedger.fixtureControls.test.ts src/application/ReconstructionObligationLedger.closure.test.ts src/application/ReconstructionObligationCandidates.test.ts src/domain/reconstructionObligationLedgerSchemas.test.ts` — 89 passed. - `npm run test:focused -- tests/boundary/cli/reconstructionObligationLedgerCli.test.ts tests/boundary/mcp/reconstructionObligationLedgerMcp.test.ts` — 16 passed. - `npm run check` — passed all six tasks and architecture guards with Node 24.18.0 / npm 11.16.0. - `npm run docs:check` — passed generated-document checks and all nine README conformance tests. - `npm run verify:test-discovery` — all 2,063 test files discovered exactly once. - Extracted documented Python example — passed; the temporary content-blind variant failed as expected. - `git diff --check` and pre-commit formatting/lint hooks — passed. ## Compatibility and migration **Breaking closure requirement:** fixture controls accepted by #4991 using names alone need content digests. Re-record positive/control Evidence with `fixture_sha256s`, update the Evidence references, and rebuild the ledger. Missing digests keep new closure open and block historical readiness; stored ledger bytes are not rewritten. Hash contents using the same representation for both runs, never filenames. No private data, new target execution, installation changes, or new runtime dependencies are introduced. ## Author readiness Human owner: @Momilijaz96 - [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 - [ ] Resolve the #4991 dependency and confirm the final closure policy with maintainers. - [ ] Adapt negative controls to `main`'s owner/revision-bound proof contract, synchronize the branch, and review the final diff and current CI results. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-11-01 21:46:57 +00:00

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

No description provided.

Rows
Columns