[PR #6321] [MERGED] fix(reconstruction): classify residual graph failures as invalid input #7280

Closed
opened 2026-11-01 21:46:44 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6321
Author: @OrdoAbChao7
Created: 10/22/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @morluto

Base: main ← Head: fix/residual-ledger-input-error


📝 Commits (1)

  • 69a4a1b fix(reconstruction): classify residual graph failures as invalid input

📊 Changes

3 files changed (+136 additions, -11 deletions)

View changed files

📝 src/application/ReconstructionObligationLedgerService.ts (+2 -0)
📝 src/domain/evidenceBundle.ts (+23 -11)
📝 tests/boundary/mcp/reconstructionObligationLedgerMcp.test.ts (+111 -0)

📄 Description

Summary

Fixes #6300. Malformed residual-unknown graphs supplied to build_reconstruction_obligation_ledger now return invalid_request / invalid_input with the failed constraint, instead of unreadable_output and advice to run rea doctor.

Problem and expected behavior

Create one authenticated residual-unknown mutation record and its unknown, then build a bundle containing the unknown but omitting the mutation record. Structural request parsing succeeds; graph authentication rejects the missing Evidence reference. The caller should receive the missing Evidence ID in details.issues and input-contract recovery. Complete bundles must continue to succeed.

Change and scope

The graph validator now throws InvalidResidualUnknownGraphError for its reference, history, resolution, and dependency-cycle constraints. The ledger service classifies that specific error as caller input. Internal invariant failures remain ordinary TypeError and retain the protocol-error fallback. The incremental registry validator uses the same error type as full bundle validation.

This is a routine correction within existing static contribution boundaries; advance maintainer scope approval is not applicable. AI-assisted implementation and independent AI review; human review remains pending.

Contract and boundary impact

  • Semantic owner: residual graph validation in src/domain/evidenceBundle.ts; ledger application error classification.
  • CLI/MCP: existing malformed-input failures gain the correct category, constraint message, and recovery. No input/output schema or catalog change.
  • Providers/platforms: provider-independent; no added execution, injection, mutation, or network authority.
  • Evidence/provenance: authentication and graph rejection are retained. Error subclass still extends TypeError and preserves existing messages.
  • Generated metadata, package, installation: no tracked changes.

Expected effects:

  • Read existing files or captured data.

Evidence and regression coverage

Executed on base 2c3dd946d59c870388b03c3417eeb5218af96eeb:

  • Four new registered-MCP regressions failed before production edits: missing mutation Evidence, revision-history gap, missing related unknown, and unqualified verified resolution all returned unreadable_output instead of invalid_request. Four existing MCP controls passed.
  • After the fix, all eight MCP tests passed. The missing-reference test accepts a valid bundle before rejection and accepts the repaired bundle afterward on the same connection.
  • Removing the new ledger catch branch made all four regressions fail again; restoring it returned them to green.
  • Independent review found no blocking issue and reran 18 domain/registry tests.

Representative new diagnostic: code: invalid_request; category: invalid_input; details.issues[0].message: "Residual unknown references missing evidence ev_…".

  • 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

Windows x64, Node 24.19.0 / npm 11.17.0 (development pins are 24.18.0 / 11.16.0), locked dependencies.

  • node node_modules/vitest/vitest.mjs run tests/boundary/mcp/reconstructionObligationLedgerMcp.test.ts tests/boundary/cli/reconstructionObligationLedgerCli.test.ts --maxWorkers=1 — 12 passed.
  • Focused Vitest run over evidenceBundle.test.ts, EvidenceLedger.residualUnknown.test.ts, obligation candidates, closure, authority, and ledger schemas — 36 passed.
  • npm run check — passed: typecheck, runtime compilation/bundling, lint, formatting, knip, metadata, module boundaries, architecture guards.
  • npm run verify:test-discovery — all 2,026 test files discovered exactly once.
  • git diff --check and pre-commit formatting/lint — passed.

The first check attempts were blocked by missing Windows native artifacts and a sandbox-denied default temp path. Restored the exact 6.7.0 published native artifact per the contributor guide, verified its manifest/SHA-256, and reran successfully with TEMP/TMP in the writable workspace. These artifacts are ignored and uncommitted.

Compatibility, safety, and release

No public API, dependencies, schemas, or valid-request behavior changes. Malformed graphs stay rejected; unrelated internal failures are not blanket-mapped to caller input. No external analysis engine or live browser was needed or exercised. Full deterministic suite and fresh stdio reproduction of the new malformed-input cases were not run; CLI tests cover existing shared-workflow behavior, while the new cases use registered MCP handlers with real SDK in-memory transport. No new release metadata is required.

Latest fetched upstream e93e3b1e7 changes unrelated APK signing code; affected source and contribution rules are unchanged.

Author readiness

Human owner: @OrdoAbChao7

  • 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

  • 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.

Current GitHub follow-up

Checked after submission: no merge conflict and no review comments. The CI and PR-title workflows report action_required with zero started jobs; their checks have not run. The author-declaration workflow succeeded, while the separate PR readiness status remains pending because this is a draft. Await upstream workflow approval/action and the human owner's final review/readiness declaration before requesting 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/6321 **Author:** [@OrdoAbChao7](https://github.com/OrdoAbChao7) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/residual-ledger-input-error` --- ### 📝 Commits (1) - [`69a4a1b`](https://github.com/morluto/rea/commit/69a4a1b122c0e1937fa29247918fa5288c1b8af2) fix(reconstruction): classify residual graph failures as invalid input ### 📊 Changes **3 files changed** (+136 additions, -11 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ReconstructionObligationLedgerService.ts` (+2 -0) 📝 `src/domain/evidenceBundle.ts` (+23 -11) 📝 `tests/boundary/mcp/reconstructionObligationLedgerMcp.test.ts` (+111 -0) </details> ### 📄 Description ## Summary Fixes #6300. Malformed residual-unknown graphs supplied to `build_reconstruction_obligation_ledger` now return `invalid_request` / `invalid_input` with the failed constraint, instead of `unreadable_output` and advice to run `rea doctor`. ## Problem and expected behavior Create one authenticated residual-unknown mutation record and its unknown, then build a bundle containing the unknown but omitting the mutation record. Structural request parsing succeeds; graph authentication rejects the missing Evidence reference. The caller should receive the missing Evidence ID in `details.issues` and input-contract recovery. Complete bundles must continue to succeed. ## Change and scope The graph validator now throws `InvalidResidualUnknownGraphError` for its reference, history, resolution, and dependency-cycle constraints. The ledger service classifies that specific error as caller input. Internal invariant failures remain ordinary `TypeError` and retain the protocol-error fallback. The incremental registry validator uses the same error type as full bundle validation. This is a routine correction within existing static contribution boundaries; advance maintainer scope approval is not applicable. AI-assisted implementation and independent AI review; human review remains pending. ## Contract and boundary impact - Semantic owner: residual graph validation in `src/domain/evidenceBundle.ts`; ledger application error classification. - CLI/MCP: existing malformed-input failures gain the correct category, constraint message, and recovery. No input/output schema or catalog change. - Providers/platforms: provider-independent; no added execution, injection, mutation, or network authority. - Evidence/provenance: authentication and graph rejection are retained. Error subclass still extends `TypeError` and preserves existing messages. - Generated metadata, package, installation: no tracked changes. Expected effects: - [x] Read existing files or captured data. ## Evidence and regression coverage Executed on base `2c3dd946d59c870388b03c3417eeb5218af96eeb`: - Four new registered-MCP regressions failed before production edits: missing mutation Evidence, revision-history gap, missing related unknown, and unqualified verified resolution all returned `unreadable_output` instead of `invalid_request`. Four existing MCP controls passed. - After the fix, all eight MCP tests passed. The missing-reference test accepts a valid bundle before rejection and accepts the repaired bundle afterward on the same connection. - Removing the new ledger catch branch made all four regressions fail again; restoring it returned them to green. - Independent review found no blocking issue and reran 18 domain/registry tests. Representative new diagnostic: `code: invalid_request; category: invalid_input; details.issues[0].message: "Residual unknown references missing evidence ev_…"`. - [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 Windows x64, Node 24.19.0 / npm 11.17.0 (development pins are 24.18.0 / 11.16.0), locked dependencies. - `node node_modules/vitest/vitest.mjs run tests/boundary/mcp/reconstructionObligationLedgerMcp.test.ts tests/boundary/cli/reconstructionObligationLedgerCli.test.ts --maxWorkers=1` — 12 passed. - Focused Vitest run over `evidenceBundle.test.ts`, `EvidenceLedger.residualUnknown.test.ts`, obligation candidates, closure, authority, and ledger schemas — 36 passed. - `npm run check` — passed: typecheck, runtime compilation/bundling, lint, formatting, knip, metadata, module boundaries, architecture guards. - `npm run verify:test-discovery` — all 2,026 test files discovered exactly once. - `git diff --check` and pre-commit formatting/lint — passed. The first check attempts were blocked by missing Windows native artifacts and a sandbox-denied default temp path. Restored the exact 6.7.0 published native artifact per the contributor guide, verified its manifest/SHA-256, and reran successfully with TEMP/TMP in the writable workspace. These artifacts are ignored and uncommitted. ## Compatibility, safety, and release No public API, dependencies, schemas, or valid-request behavior changes. Malformed graphs stay rejected; unrelated internal failures are not blanket-mapped to caller input. No external analysis engine or live browser was needed or exercised. Full deterministic suite and fresh stdio reproduction of the new malformed-input cases were not run; CLI tests cover existing shared-workflow behavior, while the new cases use registered MCP handlers with real SDK in-memory transport. No new release metadata is required. Latest fetched upstream `e93e3b1e7` changes unrelated APK signing code; affected source and contribution rules are unchanged. ## Author readiness Human owner: @OrdoAbChao7 - [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] 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. ## Current GitHub follow-up Checked after submission: no merge conflict and no review comments. The CI and PR-title workflows report `action_required` with zero started jobs; their checks have not run. The author-declaration workflow succeeded, while the separate `PR readiness` status remains pending because this is a draft. Await upstream workflow approval/action and the human owner's final review/readiness declaration before requesting 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:46:44 +00:00
Sign in to join this conversation.
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#7280
No description provided.