[PR #2303] [MERGED] fix(evidence): admit bundle imports across session transitions #5149

Closed
opened 2026-10-31 01:04:21 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2303
Author: @rohitg00
Created: 10/12/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/evidence-import-session-admission


📝 Commits (1)

  • d308e7f fix(evidence): admit bundle imports across session transitions

📊 Changes

2 files changed (+202 additions, -31 deletions)

View changed files

📝 src/server/registerSessionRecordTools.ts (+47 -31)
➕ tests/boundary/mcp/evidenceImportLifecycle.test.ts (+155 -0)

📄 Description

Summary

Keep import_evidence_bundle admitted until its file read, validation, session mutation, and result delivery finish. A concurrent close_binary now drains an earlier import before clearing the session, and an import submitted after close waits for that transition.

Problem and expected behavior

If a bundle read is still pending when close_binary runs, close currently succeeds with an empty ledger, then the import appends its records to the closed session. An import must follow the same session transition ordering as other admitted workflows. The regression is reproduced on aeee5074; the affected implementation is unchanged in upstream 0043d9b1.

No existing issue or competing PR for this ordering defect was found in the upstream recheck. The earlier import cancellation fix remains intact.

Change and scope

Wrap the complete import workflow with the existing withAdmittedAnalysis and runAdmittedToolOperation helpers. The session remains the sole owner of lifecycle ordering. Add two actual MCP SDK regressions with real bundle files and a controlled file-close boundary.

Contract and boundary impact

  • Semantic owner and earliest changed stage: session admission before file I/O.
  • Contribution scope: ordinary correctness fix within existing authority; maintainer scope decision not applicable.
  • CLI and MCP/tool catalog: no schema or tool changes. Target-free import remains supported without starting a binary provider.
  • Evidence and provenance: record identity, unknown revisions, validation, cancellation, and retained file cleanup are unchanged.
  • Target selection and cleanup: earlier imports drain before close; later imports wait for close. Existing failed-close retries remain covered.
  • Generated metadata, package, installation, or release impact: none.

Expected effects:

  • Read existing files or captured data.

Evidence and regression coverage

Both new regressions fail on the affected base: the first records close before import, and the second returns an incorrect total of 2. After the fix, the first import returns { imported: 1, unknowns_added: 0, total: 1 } and close leaves no records. A later import returns total 1, retaining only its own bundle.

The second request is explicitly observed at the MCP transport before the first read is released. Existing cancellation, persistent and transient file-close failures, target-free workflows, and session admission cancellation also pass.

  • 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

Using pinned Node 24.18.0 and npm 11.16.0 on macOS arm64:

  • npm run build:cached: passed.
  • npm run test:focused -- tests/boundary/mcp/evidenceImportLifecycle.test.ts tests/boundary/mcp/evidenceImportCancellation.test.ts tests/boundary/mcp/sessionMcp.test.ts tests/composition/analysis-sessions/binarySession.test.ts: 43 tests passed across four files.
  • npm run check: passed, including typecheck, lint, format, unused-code, metadata, module-boundary, and architecture checks.
  • npm run verify:test-discovery: all 962 test files discovered exactly once.
  • git diff --check: passed.

Compatibility, safety, and release

No breaking changes, migrations, new process authority, or release metadata changes. The tests exercise the real MCP SDK and filesystem with deterministic scheduling; a binary provider is deliberately unavailable, proving this remains a target-free operation. Other host platforms remain CI coverage.

Before merge

  • Concrete problem and focused outcome.
  • Scope and authority are resolved.
  • Related-work search and reason for no issue link are stated above.
  • Tests cover observable behavior and failure paths.
  • No owning contract or generated metadata changes are required.
  • Representative MCP results are described above.
  • Final diff reviewed 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/2303 **Author:** [@rohitg00](https://github.com/rohitg00) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/evidence-import-session-admission` --- ### 📝 Commits (1) - [`d308e7f`](https://github.com/morluto/rea/commit/d308e7f921e7c53a8fb1d26dacd5d0c7a226126f) fix(evidence): admit bundle imports across session transitions ### 📊 Changes **2 files changed** (+202 additions, -31 deletions) <details> <summary>View changed files</summary> 📝 `src/server/registerSessionRecordTools.ts` (+47 -31) ➕ `tests/boundary/mcp/evidenceImportLifecycle.test.ts` (+155 -0) </details> ### 📄 Description ## Summary Keep `import_evidence_bundle` admitted until its file read, validation, session mutation, and result delivery finish. A concurrent `close_binary` now drains an earlier import before clearing the session, and an import submitted after close waits for that transition. ## Problem and expected behavior If a bundle read is still pending when `close_binary` runs, close currently succeeds with an empty ledger, then the import appends its records to the closed session. An import must follow the same session transition ordering as other admitted workflows. The regression is reproduced on `aeee5074`; the affected implementation is unchanged in upstream `0043d9b1`. No existing issue or competing PR for this ordering defect was found in the upstream recheck. The earlier import cancellation fix remains intact. ## Change and scope Wrap the complete import workflow with the existing `withAdmittedAnalysis` and `runAdmittedToolOperation` helpers. The session remains the sole owner of lifecycle ordering. Add two actual MCP SDK regressions with real bundle files and a controlled file-close boundary. ## Contract and boundary impact - Semantic owner and earliest changed stage: session admission before file I/O. - Contribution scope: ordinary correctness fix within existing authority; maintainer scope decision not applicable. - CLI and MCP/tool catalog: no schema or tool changes. Target-free import remains supported without starting a binary provider. - Evidence and provenance: record identity, unknown revisions, validation, cancellation, and retained file cleanup are unchanged. - Target selection and cleanup: earlier imports drain before close; later imports wait for close. Existing failed-close retries remain covered. - Generated metadata, package, installation, or release impact: none. Expected effects: - [x] Read existing files or captured data. ## Evidence and regression coverage Both new regressions fail on the affected base: the first records `close` before `import`, and the second returns an incorrect total of `2`. After the fix, the first import returns `{ imported: 1, unknowns_added: 0, total: 1 }` and close leaves no records. A later import returns total `1`, retaining only its own bundle. The second request is explicitly observed at the MCP transport before the first read is released. Existing cancellation, persistent and transient file-close failures, target-free workflows, and session admission cancellation also pass. - [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 Using pinned Node 24.18.0 and npm 11.16.0 on macOS arm64: - `npm run build:cached`: passed. - `npm run test:focused -- tests/boundary/mcp/evidenceImportLifecycle.test.ts tests/boundary/mcp/evidenceImportCancellation.test.ts tests/boundary/mcp/sessionMcp.test.ts tests/composition/analysis-sessions/binarySession.test.ts`: 43 tests passed across four files. - `npm run check`: passed, including typecheck, lint, format, unused-code, metadata, module-boundary, and architecture checks. - `npm run verify:test-discovery`: all 962 test files discovered exactly once. - `git diff --check`: passed. ## Compatibility, safety, and release No breaking changes, migrations, new process authority, or release metadata changes. The tests exercise the real MCP SDK and filesystem with deterministic scheduling; a binary provider is deliberately unavailable, proving this remains a target-free operation. Other host platforms remain CI coverage. ## Before merge - [x] Concrete problem and focused outcome. - [x] Scope and authority are resolved. - [x] Related-work search and reason for no issue link are stated above. - [x] Tests cover observable behavior and failure paths. - [x] No owning contract or generated metadata changes are required. - [x] Representative MCP results are described above. - [x] Final diff reviewed 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:04:21 +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#5149
No description provided.