[rohitg00]

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.

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

[CassieFox23]

TL;DR: I tested this fix over a real MCP connection, and it works. With the fix, close_binary waits for an import that is still reading its file. An import sent after close waits for close too.

I sent each group of calls in the same tick over stdio. The bundle was a 31 MB file with 20,000 records, so the read took about 1.4 seconds. A named pipe did not work, because REA only reads regular files.

Results

Each cell counts the runs that showed the behaviour this PR describes, out of 3.

Case	Base 0043d9b (#2239)	Head d308e7f
Import, then close at once, no binary open: close waits, ledger empty after	0/3	3/3
Import, then close at once, binary open: close waits, ledger empty after	0/3	3/3
Close, then import at once, binary open: import waits, keeps its 4 records	0/3	3/3
Import, close, second import, no binary open: only the second bundle stays	0/3	3/3
Import, close, second import, binary open: only the second bundle stays	0/3	3/3
Import alone	3/3	3/3
Import, wait for the reply, then close	3/3	3/3

What base did:

• Import, then close: close answered in 3 ms. The import then put 20,000 records into the closed session.
• Close, then import: the import said "imported 4, total 6". Then close erased those 4 records.
• Import, close, second import: the first bundle's 20,000 records stayed in the closed session.

Timings

A normal import took about 1.4 s on both. With no binary open, the median was 1,358 ms on head and 1,375 ms on base. With a binary open, it was 1,420 ms and 1,402 ms. With the fix, close now waits as long as the import. Here that was 1.3 to 1.8 s. On base with no binary open, close took 3 ms.

The PR's own test file, copied into the base tree for the base row:

Check	Node 22.23.1	Node 24.18.0	Node 26.11.1
Head, 2 tests	pass	pass	pass
Base, 2 tests	fail	fail	fail

Not run

• Cancelling an import while close waits, and a bad bundle file while close waits.
• The PR's three other test files, and the full check.
• A merge with current main, which is now 3b0ea59 (fix(native): honor Mach-O chained pointer locations and library ordinals #1709).
• macOS and Windows.

Thanks, rohitg00, for the fix, and for tests that hold the file read open on purpose.

Test setup

• Box: Nobara 44 (Fedora 44 base), Linux 7.2.9
• CPU: AMD Ryzen 9 5900X, 12 cores / 24 threads
• RAM: 32 GB DDR4-3600
• Node: 22.23.1, 24.18.0, 26.11.1 (harness and builds on 24.18.0)
• Head is d308e7f. Base is 0043d9b (fix(browser): report mismatched event family selection as unknown in capture comparison #2239). The head's parent is aeee507 (feat(sqlite): inspect offline snapshots with selective evidence #2271). The one source file the PR changes is the same on both.
• Other: Ghidra 12.1.4 (Apache-2.0); a 12 KB x86-64 ELF I compiled from a short C file; the MCP client library the repo pins (2.3.1)
• The box also ran another test agent, so timings are rough.
• Each run used a new server process, with an empty home folder.

Commands

# build, once per tree (Node 24.18.0)
npm ci && npm run build && npm run test:prepare      # exit 0 on head and base

# bundles: 4-record export over MCP from the ELF; 20,000-record file from the tree's own builders
export_evidence_bundle                               # 4 records, 37,853 bytes
node genbig.mjs <tree> big-20k.json 20000 1000       # 31,106,821 bytes

# 54 runs: 3 repetitions x 9 cases x base/head, each a new "rea.mjs mcp" over stdio
#   import_evidence_bundle and close_binary written to stdin in the same tick
#   then get_evidence_bundle (summary) and binary_session

# PR test file, Node 22, 24, 26, one at a time
npm run test:focused -- tests/boundary/mcp/evidenceImportLifecycle.test.ts   # head: exit 0 x3, base: exit 1 x3

🤖 Generated with Claude Code (Sonnet 5.5, verified by Opus 5.5) 🦊

[morluto]

@greptileai review

[greptile-apps]

This PR is set for a Plus review. TREX isn't supported with Plus or Apex reviews yet, so Greptile reviews this PR at Base while TREX is on.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

import_evidence_bundle now uses the existing session admission helpers around file reading, validation, record updates, and result construction.

• Evidence imports now follow the session’s close order.

No actionable correctness, security, or repository-rule issues were found. Tests were not run during this review. Checking for a superseding PR was blocked because GitHub access was not authenticated.

Diagram

sequenceDiagram
  participant Client
  participant Import as Import tool
  participant Session
  participant File
  Client->>Import: Import earlier bundle
  Import->>Session: Admit workflow
  Import->>File: Read and validate bundle
  Client->>Session: close_binary
  Session->>Session: Wait for earlier import
  Client->>Import: Import later bundle
  Import->>Session: Wait for close
  File-->>Import: Bundle ready
  Import->>Session: Add earlier records
  Import-->>Client: Construct import result
  Session->>Session: Clear records and finish close
  Session-->>Import: Admit later workflow
  Import->>File: Read later bundle
  Import->>Session: Add later records

Loading

_{Reviews (1) · Last reviewed commit: "fix(evidence): admit bundle imports acro..." · Reviewed by Greptile}