morluto/rea

Fork 0

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

[PR #6221] [MERGED] fix(cli): keep committed evidence bundle receipts after cancellation #7209

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6221
Author: @kaoru0822-kitauji
Created: 10/22/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/6095-cancelled-bundle-receipt

📝 Commits (1)

dac3fdf fix(cli): keep committed evidence bundle receipts after cancellation

📊 Changes

8 files changed (+453 additions, -84 deletions)

View changed files

📝 docs/cli.md (+8 -0)
📝 src/application/EvidenceBundleCommands.ts (+40 -6)
📝 src/cli/analysisEvidenceDelivery.ts (+35 -40)
📝 tests/boundary/cli/commandEvidenceDelivery.test.ts (+73 -1)
➕ tests/boundary/cli/evidenceOutputPublicationCli.test.ts (+213 -0)
📝 tests/boundary/cli/managedMemberSummaryCli.test.ts (+2 -4)
➕ tests/fixtures/evidencePublicationPreload.mjs (+82 -0)
➖ tests/fixtures/managedPublicationSignalPreload.mjs (+0 -33)

📄 Description

What broke: --detail summary --evidence_output DEST commands (model, Go, Rust, GPU, IL2CPP, desktop, web network capture, artifact, objdump/GDB, native function matching) could commit the bundle and then return a result with no receipt. This happened when SIGINT/SIGTERM arrived during staging cleanup after commit (typed cancelled) and when staging cleanup stayed incomplete after commit (cleanup_incomplete without published_path).
Fix: These commands now select their view first and then publish through publishEvidenceDelivery, which was extracted from the owner inspect-managed-members --evidence-bundle already used. A committed receipt is kept as details.diagnostics.evidence_bundle after a cancellation and as details.published_path/bytes when cleanup stays incomplete.
Risk: Low to medium. Success output shapes are unchanged. Some failure cases now report differently (listed under Compatibility): pre-commit cancellation names the command's operation, a cleanup retry that succeeds now returns success, and a summary-view failure now happens before anything is written.

Summary

Fixes #6095.

publishCliAnalysisEvidence kept the bundle receipt in a local variable and returned it only on success. Publication ran inside finishAnalysisCommand, which then created the post-close cancellation with only the summary as its partial observation. When cleanup failed after commit, it wrapped the writer's EvidenceFileError (which still had published) in a ProviderCleanupError, so the receipt was lost in both cases. The application owner deliverAnalysisEvidence already handled this correctly for inspect-managed-members. The CLI helper had its own separate copy of the publication lifecycle.

Problem and expected behavior

Smallest trigger (from the issue): inspect-model-artifact fixture.safetensors --detail summary --evidence_output DEST --json with SIGINT delivered after the destination is committed, during the staged-output cleanup. It exits 130 with code: cancelled, and the valid bundle is at DEST, but the JSON never mentions DEST. This breaks docs/tool-design.md ("For atomic publication … preserve any committed output's publication receipt").

Expected: a cancellation after commit keeps {path, bytes} alongside the retained Evidence. A cancellation before commit creates nothing and reports no receipt.

Change and scope

src/application/EvidenceBundleCommands.ts: new publishEvidenceDelivery({bundle, complete, delivered, outputPath, operation, signal}). It persists the bundle and retries the exact staged-output cleanup through the existing persistEvidenceBundleCommand, dispatches queued interrupts, then checks the signal. Before commit, a failure retains the complete Evidence. After commit, a cancellation carries diagnostics.evidence_bundle. deliverAnalysisEvidence (managed members) now calls it, with no behavior change.
src/cli/analysisEvidenceDelivery.ts: publishCliAnalysisEvidence computes the requested view (still the inline summary/custom view, so view Evidence identities are unchanged) before publishing, then calls publishEvidenceDelivery. The CLI copy of the lifecycle (ArtifactResourceScope + finishAnalysisCommand + captured receipt variable) is removed. All --evidence_output commands reach it through deliverCliAnalysisEvidence or call it directly.
docs/cli.md: documents the commit cutoff and where the receipt appears.
Tests: tests/fixtures/managedPublicationSignalPreload.mjs is generalized to evidencePublicationPreload.mjs. It adds a staging phase (signal before commit) and cleanup_failures (rejects staging-directory removal N times). The managed test now uses it.
Not included: MCP. No MCP tool has a summary-plus-output delivery like this. MCP export_evidence_bundle already returns its receipt when publication wins a late cancellation, and failures go through the same projectAnalysisError. That path is unchanged.

Contribution scope: not applicable (ordinary fix within existing boundaries).

Contract and boundary impact

Semantic owner and earliest changed stage: publishEvidenceDelivery (application). It is now the one owner of the publish-then-deliver lifecycle for CLI --evidence_output and managed --evidence-bundle.
Contribution scope fit and linked maintainer decision: not applicable.
CLI and MCP/tool-catalog contract: no schema or catalog change. A post-commit CLI cancellation adds details.diagnostics.evidence_bundle: {path, bytes}, the same shape managed members already used.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: no Evidence changes. Retained partial observations are the summary (after commit) or the complete Evidence (before commit or on failure).
Target selection, authority, upstream behavior, and cleanup details: staged-output cleanup is unchanged and is now retried once by the shared owner, as evidence-export already does.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.
Change process memory, registers, files, or configuration. (Existing behavior: the caller-selected output file. Nothing new.)

Evidence and regression coverage

Tests added or updated:
- tests/boundary/cli/evidenceOutputPublicationCli.test.ts (new, compiled CLI, real filesystem and real OS signals on POSIX). Two command families, inspect-model-artifact and inspect-desktop-application:
  - SIGINT/SIGTERM after commit, during final cleanup, for summary and complete detail: exit 130/143, diagnostics.evidence_bundle equals the file's path and byte count, the partial observation is the linked summary (or the complete parent), and no staging directory is left behind.
  - SIGINT before commit: exit 130, no destination, no diagnostics, the destination appears nowhere in the JSON, and the complete Evidence is retained.
  - One staging-cleanup failure followed by a successful retry: exit 0 with the normal {summary, evidence_bundle}.
  - Cleanup that stays incomplete: cleanup_incomplete with published: true, published_path, bytes, and the complete Evidence.
- tests/boundary/cli/commandEvidenceDelivery.test.ts: objdump, objdump architectures and GDB call publishCliAnalysisEvidence directly. SIGINT during final staging cleanup keeps the receipt. SIGINT before commit publishes nothing.
- tests/boundary/cli/managedMemberSummaryCli.test.ts: moved to the renamed preload. Its assertions are unchanged.
Base reproduction: the issue's receipt-repro.mjs/signal-hook.mjs was run on base ce5bc8cb0 (built). The results were normal exit 0 with receipt, before exit 130 with no file, and after exit 130 with the file present but no receipt. With the fix, after reports details.diagnostics.evidence_bundle: {path: …/after.bundle.json, bytes: 2910}. With only the two src/ files reverted, 15 of the new tests fail. That includes all cleanup-phase cancellation cases and the persistent-cleanup case, where base dropped published_path.

User-visible CLI output (after case, abbreviated):

{"code":"cancelled","details":{"operation":"inspect_model_artifact","diagnostics":{"evidence_bundle":{"path":"/…/after.bundle.json","bytes":2910}},"partial_observation":{"…":"summary Evidence linked to the bundled parent"}}}

Remaining proof gaps: Windows was not run; it uses event signal delivery in the test. MCP has no equivalent summary-delivery path to exercise. Only two of the ten --evidence_output families were run end to end with signals; the others share the same call site, which is source-derived plus the in-process objdump/GDB tests. Natural signal timing was not measured. Token and timing costs were not measured: no extra I/O or projections were added, and the only output change is the small diagnostics object on cancellation.
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

Issue repro driver on base ce5bc8cb0: reproduced (above). With the fix, after carries the receipt.
npm run test:focused -- tests/boundary/cli/evidenceOutputPublicationCli.test.ts tests/boundary/cli/managedMemberSummaryCli.test.ts tests/boundary/cli/commandEvidenceDelivery.test.ts tests/boundary/filesystem/evidenceBundleFiles.test.ts tests/boundary/cli/desktopApplicationCli.test.ts tests/acceptance/modelArtifactCli.test.ts tests/acceptance/tools/nativeFunctionMatching.test.ts tests/acceptance/godotPck.test.ts tests/process-global/managedBundleReadGuards.test.ts: 101 passed.
npm run test:focused -- with the IL2CPP, Rust, Go, protobuf, evidenceBundleCommands, reverseEngineeringCommands, MCP evidenceExportCancellation and jsonOutputOwnership suites plus the files above (15 files): 174 passed.
The same new tests with only src/ reverted: 15 failed, 12 passed.
npm run check:fast: passed.
npm run format:check: passed.
npm run docs:check: passed.

Compatibility, safety, and release

Breaking changes or migration steps: none to success output. These failure outcomes changed:
- Pre-commit cancelled from an --evidence_output command now reports details.operation as the command's operation (for example inspect_model_artifact) instead of export_evidence_bundle.
- If a staging cleanup failure is confirmed released by the retry, the command now returns success with its receipt instead of an execution_failure that still pointed to the published file. This matches evidence-export.
- If the summary view fails, the bundle is no longer written. The error now retains the complete Evidence as partial_observation, so nothing is committed without being reported.
Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 only, with real OS signals. No providers involved.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new file effects. The output is still the caller-selected path, written atomically and never overwritten.

Author readiness

Human owner: @kaoru0822-kitauji

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.

🤖 Generated with Claude Code

_{🔄 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/6221 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/6095-cancelled-bundle-receipt` --- ### 📝 Commits (1) - [`dac3fdf`](https://github.com/morluto/rea/commit/dac3fdf3a65e1989a75b0a1567add5806950950f) fix(cli): keep committed evidence bundle receipts after cancellation ### 📊 Changes **8 files changed** (+453 additions, -84 deletions) <details> <summary>View changed files</summary> 📝 `docs/cli.md` (+8 -0) 📝 `src/application/EvidenceBundleCommands.ts` (+40 -6) 📝 `src/cli/analysisEvidenceDelivery.ts` (+35 -40) 📝 `tests/boundary/cli/commandEvidenceDelivery.test.ts` (+73 -1) ➕ `tests/boundary/cli/evidenceOutputPublicationCli.test.ts` (+213 -0) 📝 `tests/boundary/cli/managedMemberSummaryCli.test.ts` (+2 -4) ➕ `tests/fixtures/evidencePublicationPreload.mjs` (+82 -0) ➖ `tests/fixtures/managedPublicationSignalPreload.mjs` (+0 -33) </details> ### 📄 Description **What broke:** `--detail summary --evidence_output DEST` commands (model, Go, Rust, GPU, IL2CPP, desktop, web network capture, artifact, objdump/GDB, native function matching) could commit the bundle and then return a result with no receipt. This happened when SIGINT/SIGTERM arrived during staging cleanup after commit (typed `cancelled`) and when staging cleanup stayed incomplete after commit (`cleanup_incomplete` without `published_path`). **Fix:** These commands now select their view first and then publish through `publishEvidenceDelivery`, which was extracted from the owner `inspect-managed-members --evidence-bundle` already used. A committed receipt is kept as `details.diagnostics.evidence_bundle` after a cancellation and as `details.published_path`/`bytes` when cleanup stays incomplete. **Risk:** Low to medium. Success output shapes are unchanged. Some failure cases now report differently (listed under Compatibility): pre-commit cancellation names the command's operation, a cleanup retry that succeeds now returns success, and a summary-view failure now happens before anything is written. ## Summary Fixes #6095. `publishCliAnalysisEvidence` kept the bundle receipt in a local variable and returned it only on success. Publication ran inside `finishAnalysisCommand`, which then created the post-close cancellation with only the summary as its partial observation. When cleanup failed after commit, it wrapped the writer's `EvidenceFileError` (which still had `published`) in a `ProviderCleanupError`, so the receipt was lost in both cases. The application owner `deliverAnalysisEvidence` already handled this correctly for `inspect-managed-members`. The CLI helper had its own separate copy of the publication lifecycle. ## Problem and expected behavior Smallest trigger (from the issue): `inspect-model-artifact fixture.safetensors --detail summary --evidence_output DEST --json` with SIGINT delivered after the destination is committed, during the staged-output cleanup. It exits 130 with `code: cancelled`, and the valid bundle is at DEST, but the JSON never mentions DEST. This breaks docs/tool-design.md ("For atomic publication … preserve any committed output's publication receipt"). Expected: a cancellation after commit keeps `{path, bytes}` alongside the retained Evidence. A cancellation before commit creates nothing and reports no receipt. ## Change and scope - `src/application/EvidenceBundleCommands.ts`: new `publishEvidenceDelivery({bundle, complete, delivered, outputPath, operation, signal})`. It persists the bundle and retries the exact staged-output cleanup through the existing `persistEvidenceBundleCommand`, dispatches queued interrupts, then checks the signal. Before commit, a failure retains the complete Evidence. After commit, a cancellation carries `diagnostics.evidence_bundle`. `deliverAnalysisEvidence` (managed members) now calls it, with no behavior change. - `src/cli/analysisEvidenceDelivery.ts`: `publishCliAnalysisEvidence` computes the requested view (still the inline summary/custom view, so view Evidence identities are unchanged) before publishing, then calls `publishEvidenceDelivery`. The CLI copy of the lifecycle (`ArtifactResourceScope` + `finishAnalysisCommand` + captured `receipt` variable) is removed. All `--evidence_output` commands reach it through `deliverCliAnalysisEvidence` or call it directly. - `docs/cli.md`: documents the commit cutoff and where the receipt appears. - Tests: `tests/fixtures/managedPublicationSignalPreload.mjs` is generalized to `evidencePublicationPreload.mjs`. It adds a `staging` phase (signal before commit) and `cleanup_failures` (rejects staging-directory removal N times). The managed test now uses it. - Not included: MCP. No MCP tool has a summary-plus-output delivery like this. MCP `export_evidence_bundle` already returns its receipt when publication wins a late cancellation, and failures go through the same `projectAnalysisError`. That path is unchanged. Contribution scope: not applicable (ordinary fix within existing boundaries). ## Contract and boundary impact - Semantic owner and earliest changed stage: `publishEvidenceDelivery` (application). It is now the one owner of the publish-then-deliver lifecycle for CLI `--evidence_output` and managed `--evidence-bundle`. - Contribution scope fit and linked maintainer decision: not applicable. - CLI and MCP/tool-catalog contract: no schema or catalog change. A post-commit CLI cancellation adds `details.diagnostics.evidence_bundle: {path, bytes}`, the same shape managed members already used. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: no Evidence changes. Retained partial observations are the summary (after commit) or the complete Evidence (before commit or on failure). - Target selection, authority, upstream behavior, and cleanup details: staged-output cleanup is unchanged and is now retried once by the shared owner, as `evidence-export` already does. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. - [x] Change process memory, registers, files, or configuration. (Existing behavior: the caller-selected output file. Nothing new.) ## Evidence and regression coverage - Tests added or updated: - `tests/boundary/cli/evidenceOutputPublicationCli.test.ts` (new, compiled CLI, real filesystem and real OS signals on POSIX). Two command families, `inspect-model-artifact` and `inspect-desktop-application`: - SIGINT/SIGTERM after commit, during final cleanup, for summary and complete detail: exit 130/143, `diagnostics.evidence_bundle` equals the file's path and byte count, the partial observation is the linked summary (or the complete parent), and no staging directory is left behind. - SIGINT before commit: exit 130, no destination, no `diagnostics`, the destination appears nowhere in the JSON, and the complete Evidence is retained. - One staging-cleanup failure followed by a successful retry: exit 0 with the normal `{summary, evidence_bundle}`. - Cleanup that stays incomplete: `cleanup_incomplete` with `published: true`, `published_path`, `bytes`, and the complete Evidence. - `tests/boundary/cli/commandEvidenceDelivery.test.ts`: objdump, objdump architectures and GDB call `publishCliAnalysisEvidence` directly. SIGINT during final staging cleanup keeps the receipt. SIGINT before commit publishes nothing. - `tests/boundary/cli/managedMemberSummaryCli.test.ts`: moved to the renamed preload. Its assertions are unchanged. - Base reproduction: the issue's `receipt-repro.mjs`/`signal-hook.mjs` was run on base `ce5bc8cb0` (built). The results were `normal` exit 0 with receipt, `before` exit 130 with no file, and `after` exit 130 with the file present but no receipt. With the fix, `after` reports `details.diagnostics.evidence_bundle: {path: …/after.bundle.json, bytes: 2910}`. With only the two `src/` files reverted, 15 of the new tests fail. That includes all cleanup-phase cancellation cases and the persistent-cleanup case, where base dropped `published_path`. - User-visible CLI output (`after` case, abbreviated): ```json {"code":"cancelled","details":{"operation":"inspect_model_artifact","diagnostics":{"evidence_bundle":{"path":"/…/after.bundle.json","bytes":2910}},"partial_observation":{"…":"summary Evidence linked to the bundled parent"}}} ``` - Remaining proof gaps: Windows was not run; it uses `event` signal delivery in the test. MCP has no equivalent summary-delivery path to exercise. Only two of the ten `--evidence_output` families were run end to end with signals; the others share the same call site, which is source-derived plus the in-process objdump/GDB tests. Natural signal timing was not measured. Token and timing costs were not measured: no extra I/O or projections were added, and the only output change is the small diagnostics object on cancellation. - [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 - Issue repro driver on base `ce5bc8cb0`: reproduced (above). With the fix, `after` carries the receipt. - `npm run test:focused -- tests/boundary/cli/evidenceOutputPublicationCli.test.ts tests/boundary/cli/managedMemberSummaryCli.test.ts tests/boundary/cli/commandEvidenceDelivery.test.ts tests/boundary/filesystem/evidenceBundleFiles.test.ts tests/boundary/cli/desktopApplicationCli.test.ts tests/acceptance/modelArtifactCli.test.ts tests/acceptance/tools/nativeFunctionMatching.test.ts tests/acceptance/godotPck.test.ts tests/process-global/managedBundleReadGuards.test.ts`: 101 passed. - `npm run test:focused --` with the IL2CPP, Rust, Go, protobuf, `evidenceBundleCommands`, `reverseEngineeringCommands`, MCP `evidenceExportCancellation` and `jsonOutputOwnership` suites plus the files above (15 files): 174 passed. - The same new tests with only `src/` reverted: 15 failed, 12 passed. - `npm run check:fast`: passed. - `npm run format:check`: passed. - `npm run docs:check`: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none to success output. These failure outcomes changed: - Pre-commit `cancelled` from an `--evidence_output` command now reports `details.operation` as the command's operation (for example `inspect_model_artifact`) instead of `export_evidence_bundle`. - If a staging cleanup failure is confirmed released by the retry, the command now returns success with its receipt instead of an `execution_failure` that still pointed to the published file. This matches `evidence-export`. - If the summary view fails, the bundle is no longer written. The error now retains the complete Evidence as `partial_observation`, so nothing is committed without being reported. - Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 only, with real OS signals. No providers involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new file effects. The output is still the caller-selected path, written atomically and never overwritten. ## Author readiness Human owner: @kaoru0822-kitauji - [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 - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <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:09 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-03 13:39:52 +00:00

[PR #7209] [CLOSED] test(setup): expect the forwarded MCP response limit in registrations #7718

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

No description provided.

Rows
Columns