Skip to content

fix(cli): keep captured source Evidence when one-shot delivery never commits - #8632

Merged
kaoru0822-kitauji merged 3 commits into
mainfrom
fix/8589-oneshot-source-closure
Nov 2, 2026
Merged

kaoru0822-kitauji merged 3 commits into
mainfrom
fix/8589-oneshot-source-closure

Conversation

@kaoru0822-kitauji

@kaoru0822-kitauji kaoru0822-kitauji commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

What broke: A completed one-shot observation cancelled at final close (or whose cleanup failed, or whose bundle never committed) kept its Evidence links but dropped the captured source records, with no statement of what happened to them.
The fix: publishCliAnalysisEvidence, the shared --evidence_output boundary, now adds details.source_closure to those failures: authentic captured records within 64 KiB, plus explicit not_captured and not_inlined omissions.
The risk: Low. Only failures with a requested, uncommitted bundle and a completed Evidence observation change; successful delivery was byte-identical to base in a real-process comparison, and nothing is written after cancellation.

Summary

Fixes #8589

The report was observed on draft heads #8585 and #8588, whose includeEvidenceSources and rea trace --evidence_output are not on main. The same defect does ship on main at the same boundary: match-native-functions --evidence_output captures its source closure and passes it to publishCliAnalysisEvidence, whose failure branch returned the projected error without it.

Problem and expected behavior

Smallest trigger on main: rea match-native-functions REQUEST --source_bundle SOURCES --evidence_output OUT --json, with SIGINT arriving at the start of final ArtifactResourceScope.close(). Close succeeds. The result is cancelled with details.partial_observation.evidence_links, but no source record, no bundle and no statement about those sources.

Violated invariant (own the lifecycle): completed facts survive later cancellation or finalization through typed failures, with explicit coverage. Expected: keep or explicitly describe the closure, without turning cancellation into delivery, writing after cancellation, overstating retention or sending an unbounded error.

Change and scope

  • undeliveredSourceClosure in AnalysisEvidenceSources.ts walks the observation's links and the captured records breadth-first through a Set, so no link becomes a call argument and each link costs one lookup however often it repeats. Records go inline until 64 KiB (the allowance used for other retained process diagnostics; a record that does not fit is skipped, so smaller later records can still fit). Up to 64 omissions are listed and omitted_count has the total.
  • publishCliAnalysisEvidence attaches it to every failure before commit: final-close cancellation, cleanup failure, cancellation during publication, an existing output file, and a refused view. Every --evidence_output command that returns its failure through this function gets this.
  • After commit, the existing receipt owns the closure (evidenceDeliveryCommitted), so no source_closure is added.
  • Collection is best effort and never throws. If examining the closure fails, collection_failure names the error type (not its message), the closure state is unknown, and the original failure is still returned.
  • not_captured never fabricates a record, and the ancestors of such a link stay unknown. Embedded inspect_artifact sources are not duplicated. With explicit sensitiveValues, captured records stay out of error output as not_inlined.

Not included:

  • rea function --evidence_output and similar direct commands do not capture linked sources on main, on success either (source-derived from coreBinaryCommands.ts). Failures now report those links as not_captured. Changing what success delivers is outside this fix.
  • Drafts feat(native): page retained feature traces #8585 and feat(native): page retained procedure references #8588 must carry their captured sources into sourceEvidence on failure to get records rather than not_captured. runAnalysis on those heads returns sources only on success.
  • inspect-with-objdump and capture-browser-scenario return failures without calling this function, and inspect_managed_members delivers through deliverAnalysisEvidence. A source search found no linked or captured sources in their producers, so they are unchanged (source-derived, not executed).
  • [Design] Preserve source availability in one-shot analysis #8590 (trace source discoverability) is untouched.

Scope decision: not applicable; this is an ordinary fix within existing read-only boundaries.

Contract and boundary impact

  • Semantic owner and earliest changed stage: AnalysisEvidenceSources.ts owns the closure; publishCliAnalysisEvidence is the delivery consumer. finishAnalysisCommand and runAnalysis are unchanged. No superseded path to remove; isObject is now exported rather than duplicated.
  • Contribution scope fit and linked maintainer decision: fix within existing scope; no decision required.
  • CLI and MCP/tool-catalog contract: additive details.source_closure on the qualifying CLI failures, documented in docs/cli.md. code, retryable, remediation and exit status are unchanged. MCP and the catalog are unchanged.
  • Provider, bridge, target-format, or platform compatibility: none.
  • Evidence, artifact, provenance, or reconstruction contract: inline records are the original authenticated Evidence (tests re-authenticate them with parseEvidence); an unauthenticated observation cannot select sources.
  • Target selection, authority, upstream behavior, and cleanup details: no target execution or mutation; cleanup order and retained resources are unchanged.
  • Generated metadata, package, or installation impact: none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated:
    • tests/process-global/oneShotSourceClosure.test.ts drives the registered command through real final close: interrupt, cleanup failure, both together, summary delivery with clean retry, an oversized source, a dependency with 200,000 links, and the success control. The fault fires only on the scope the command gave to matching, after matching completed, and every run asserts that close was reached once; another test closes an unrelated scope first.
    • tests/boundary/filesystem/undeliveredSourceClosure.test.ts covers never-captured and transitive links, the size and omission bounds, explicit exclusions, embedded sources, cancellation before commit, an existing output, late cancellation and cleanup failure after commit, untouched failures, tampered observations, 200,000-entry link sets, exceptions during collection, deliverCliAnalysisEvidence, and production runDirectAnalysis with a synthetic provider.
  • Base reproduction (executed; real process, production createCli from dist/cli.js, real SIGINT sent by a wrapper at the start of real final close, synthetic Evidence inputs):
Run Exit Stdout bytes Bundle Source records in output
base, interrupt 130 6722 absent none
fix, interrupt 130 10388 absent 3, complete: true
base and fix, control 0 5873, cmp identical 7387, cmp identical n/a

Apart from source_closure, the base and fixed interrupt errors compared equal as parsed JSON. The wrapper fires on the first scope close; both interrupt runs recorded exactly one, and the completed observation was present.

  • Regression check (executed), with src restored from origin/main: 8 of 9 process-global and 19 of 24 boundary tests failed. The tests that passed are controls for unchanged behavior.
    With src from the previous head 2f3ba28, 2 of 9 process-global and 5 of 24 boundary tests failed. The registered command printed {"code":"UNKNOWN","message":"Maximum call stack size exceeded"} in place of the typed cancelled or cleanup_incomplete result. Faulting the first close instead of the final one failed the unrelated-scope test, and forcing evidenceDeliveryCommitted to false failed both commit-receipt tests.
  • Cost: failure payload grows by the closure only, with the bound asserted in tests (under 80 KiB in the worst tested case). Single runs showed about 0.5 s and 260 MiB peak RSS on both base and fix; this is not a benchmark and does not cover process-family memory. Token usage was not measured. The failure path adds one Evidence authentication and one lookup per link; the success path adds neither.
  • User-visible CLI/MCP output:
"source_closure": {
  "publication": "not_published",
  "complete": false,
  "records": [{ "evidence_id": "ev_221b…", "operation": "list_procedures" }],
  "omitted_count": 1,
  "omitted": [{ "evidence_id": "ev_7a80…", "reason": "not_inlined" }],
  "recovery": "No bundle was written and this invocation retains nothing after exit. Keep any records shown here, or run the operation again after following remediation; that is a new observation and may differ."
}

Records are abbreviated here; each is the complete Evidence record.

  • Remaining proof gaps:
    • No real Ghidra or other real provider was available. The trace_feature flow in the report exists only on the drafts and was not run; the match-native-functions runs and runDirectAnalysis tests use synthetic inputs and a synthetic provider.
    • The signal is injected at the final-close boundary. Natural timing was not established.
    • The bundled stock scripts/rea.mjs runtime was not exercised under cancellation, because the wrapper patches the per-module dist copy. Only its ordinary success path ran.
    • macOS arm64 only.
    • collection_failure covers thrown errors. Heap exhaustion cannot be caught; authenticating here costs the same as the parseEvidence call on the success path.

Lifecycle regressions covered: cancellation during final cleanup and during publication, cleanup failure after success, committed output followed by cancellation or incomplete cleanup, and retry after failure. Boundary regressions covered: malformed and non-Evidence observations, capacity bounds, and explicit omission.

For evidence-bearing changes:

  • 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

  • npm run check:fast — passed
  • npm run format:check — passed
  • npm run docs:check — passed
  • npm run metadata:check — passed
  • npx vitest run --project boundary --project cli-boundary --project process-global --project composition --project acceptance on the 13 related files, including evidenceBundleFiles, commandEvidenceDelivery, evidenceOutputPublicationCli, finishAnalysisCommand, commandFinalizationCancellation, historicalCaptureSnapshotRedaction, webCaptureDeliveryRedaction and acceptance nativeFunctionMatching — 13 files and 176 tests passed
  • Real-process base and fix comparison above, built with npm run build:cached — results as tabulated

Compatibility, safety, and release

  • Breaking changes or migration steps: none. The new key is additive, on failures only.
  • Real Hopper/Ghidra, browser, or OS coverage: none; macOS arm64 with synthetic inputs and a synthetic provider.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: captured records are withheld from error output when the invocation carries explicit sensitive values, and evidence IDs in omitted are digests. No new files, processes or network use.

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.

Declarations entered by Claude Code on the owner's standing written instruction.

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

…commits

A completed observation that was cancelled at final close, failed cleanup, or
never reached bundle commit returned its evidence links but dropped the source
records the invocation had already captured. The caller could neither read those
records nor tell whether they were missing, captured or written.

Report the closure at publishCliAnalysisEvidence, the shared boundary every
--evidence_output command uses. details.source_closure carries authentic captured
records within 64 KiB and explicit omissions: not_captured for a link this
invocation holds no record for, and not_inlined for a captured record outside this
response. A committed bundle keeps its receipt instead, nothing is written after
cancellation, and successful delivery is unchanged.

Fixes #8589

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kaoru0822-kitauji kaoru0822-kitauji added bug Something isn't working track:bugs Small-bug loop track PRs labels Nov 2, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-02T07:52:51.713134Z 7d8ab91 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5 Tier: apex

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

Summary

Adds bounded details.source_closure to CLI failures when a requested Evidence bundle never commits.

  • Failed CLI deliveries now show which cited sources remain available.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Completed Evidence] --> B{Final close succeeds?}
  B -->|No or cancelled| C[Original typed failure]
  B -->|Yes| D[Select view and publish bundle]
  D -->|Failure before commit| C
  C --> E[Authenticate partial observation]
  E --> F[Attach bounded source_closure]
  D -->|Failure after commit| G[Keep published receipt]
  D -->|Success| H[Return existing success output]
Loading

Reviews (3) · Last reviewed commit: "fix(cli): keep the typed failure when co..." · Reviewed by Greptile

…t finish

A captured source with a data-sized evidence_links array made
pending.push(...links) exceed V8's call-argument limit. On final-close
cancellation or a refused publication that RangeError replaced the original typed
failure, which is the one outcome this delivery step exists to preserve.

Walk the closure through a Set, which is a deduplicating breadth-first queue, so
no link becomes a call argument and each link costs one lookup however often it
repeats. undeliveredSourceClosure now owns authentication and never throws: if
examining the closure fails, it reports collection_failure with the bounded error
type and an unknown closure state, and the original failure is still returned.

Fault the final-close tests on the scope the command gave to matching, after
matching completed, and assert that close was reached, so a moved close fails
instead of passing on an earlier one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kaoru0822-kitauji
kaoru0822-kitauji merged commit 3f4aff2 into main Nov 2, 2026
49 of 51 checks passed
@kaoru0822-kitauji
kaoru0822-kitauji deleted the fix/8589-oneshot-source-closure branch November 2, 2026 08:50
coyoteXujie pushed a commit to coyoteXujie/rea that referenced this pull request Nov 2, 2026
…commits (morluto#8632)

* fix(cli): keep captured source Evidence when one-shot delivery never commits

A completed observation that was cancelled at final close, failed cleanup, or
never reached bundle commit returned its evidence links but dropped the source
records the invocation had already captured. The caller could neither read those
records nor tell whether they were missing, captured or written.

Report the closure at publishCliAnalysisEvidence, the shared boundary every
--evidence_output command uses. details.source_closure carries authentic captured
records within 64 KiB and explicit omissions: not_captured for a link this
invocation holds no record for, and not_inlined for a captured record outside this
response. A committed bundle keeps its receipt instead, nothing is written after
cancellation, and successful delivery is unchanged.

Fixes morluto#8589

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(cli): keep the typed failure when collecting source closure cannot finish

A captured source with a data-sized evidence_links array made
pending.push(...links) exceed V8's call-argument limit. On final-close
cancellation or a refused publication that RangeError replaced the original typed
failure, which is the one outcome this delivery step exists to preserve.

Walk the closure through a Set, which is a deduplicating breadth-first queue, so
no link becomes a call argument and each link costs one lookup however often it
repeats. undeliveredSourceClosure now owns authentication and never throws: if
examining the closure fails, it reports collection_failure with the bounded error
type and an unknown closure state, and the original failure is still returned.

Fault the final-close tests on the scope the command gave to matching, after
matching completed, and assert that close was reached, so a moved close fails
instead of passing on an earlier one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working track:bugs Small-bug loop track PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] One-shot final-close cancellation drops linked source Evidence

1 participant