[PR #6020] [MERGED] fix(process): keep structural capture issues alongside semantic ones #7125

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

📋 Pull Request Information

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

Base: main ← Head: fix/5957-capture-parse-structural-issues


📝 Commits (1)

  • f3cc3d7 fix(process): keep structural capture issues alongside semantic ones

📊 Changes

5 files changed (+121 additions, -30 deletions)

View changed files

📝 src/application/ReconstructionObligationCandidates.test.ts (+36 -0)
📝 src/application/ReconstructionObligationCandidates.ts (+3 -5)
📝 src/domain/process/processCapture.validation.test.ts (+22 -9)
📝 src/domain/process/processCaptureParsing.ts (+6 -15)
📝 tests/boundary/mcp/processComparisonDiagnosticsMcp.test.ts (+54 -1)

📄 Description

What broke: parseProcessCapture re-threw only the semantic (custom) schema issues whenever any existed, so a capture that also had a structural issue (e.g. a missing manifest.completed_at) reported only the semantic one, with no remaining-issue count.
Fix: throw the schema's own ZodError so every issue is retained. describeValidationFailure then names the first issue and the number remaining. Reconstruction obligation candidates now use that formatter too.
Risk: low. Rejection and error classification are unchanged. Semantic-only failures now read at <path>: <message> instead of Invalid process capture: <path>: <message>, the same form structural failures already used.

Summary

Fixes #5957. A process capture with both a structural and a semantic schema issue lost the structural issue in the parser before any CLI/MCP caller saw it. Now all issues stay on the thrown ZodError, and the existing bounded formatter reports the first one plus (N more issues).

Problem and expected behavior

Trigger: start from FINALIZED_PROCESS_CAPTURE_EXAMPLE, delete manifest.completed_at, and set exit.finalization.signals[0].reason. The schema reports two issues. parseProcessCapture kept only the custom subset and threw it as a TypeError, so compare-process-captures, MCP compare_process_captures, bundle validation and obligation candidates showed only the finalization issue. Expected: keep every issue found in that parse, or at least the first one and a count of the rest.

Change and scope

  • src/domain/process/processCaptureParsing.ts: parseProcessCapture is now attachProof(processCaptureSchema.parse(input)). This removes the custom-only re-packaging, which was the only place issues were dropped. All consumers already accepted ZodError and formatted it with describeValidationFailure (CLI, MCP comparison, Evidence bundle files), or caught TypeError | ZodError (prompt completion, reconstruction verification).
  • src/application/ReconstructionObligationCandidates.ts: this was the one consumer that formatted errors itself, using cause.message. For a ZodError that message is the full JSON issue dump, so it now uses describeValidationFailure. The local errorMessage helper is removed.
  • No other parser has this issue-subsetting pattern (grep for code === "custom" in src/ found only this one). Native provider reply parsers that report issues[0] only are a separate concern and are not changed here.

Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: process capture parsing (parseProcessCapture).

  • Contribution scope fit and linked maintainer decision: not applicable (bug fix).

  • CLI and MCP/tool-catalog contract: no schema or catalog change. Codes and categories are unchanged (invalid_request/invalid_input on the CLI, evidence_integrity_mismatch on MCP). Message text changes as described above.

  • Provider, bridge, target-format, or platform compatibility: none.

  • Evidence, artifact, provenance, or reconstruction contract: none. Obligation-candidate limitation text now uses the shared bounded description.

  • Target selection, authority, upstream behavior, and cleanup details: not applicable.

  • Generated metadata, package, or installation impact: none.

  • Read existing files or captured data.

Evidence and regression coverage

  • Tests added or updated:

    • tests/boundary/mcp/processComparisonDiagnosticsMcp.test.ts: a mixed structural+semantic capture through MCP compare_process_captures and the CLI compareProcessEvidenceFiles. Both must report at manifest.completed_at: ... (1 more issues). This test fails on base, which reports only the finalization issue.
    • src/application/ReconstructionObligationCandidates.test.ts: the same mixed capture must produce a limitation that names the first issue and the count. This test fails on base.
    • src/domain/process/processCapture.validation.test.ts: three assertions that matched joined paths in the TypeError message now check the rejected paths on the ZodError.
  • Base reproduction: on 6d356b843, the mixed capture produced Capture evidence is malformed (Invalid process capture: exit.finalization.signals.0.reason: finalization reason is only allowed for an unverified delivery).

  • User-visible CLI output after the fix (built scripts/rea.mjs compare-process-captures --json, issue script inputs):

    • mixed: Capture evidence is malformed (at manifest.completed_at: Invalid input: expected string, received undefined (1 more issues)). Create new capture evidence, then try again. (exit 1, invalid_request)
    • semantic only: Capture evidence is malformed (at exit.finalization.signals.0.reason: finalization reason is only allowed for an unverified delivery). ...
    • structural only: unchanged. Valid: exit 0.
  • Remaining proof gaps: not run here: live capture, MCP bundle import end-to-end, other platforms, and the full suite. Performance: the parse does the same single schema pass as before and drops one flatMap. No measurable cost change is expected; CLI wall time was about 375 ms per run, dominated by startup.

  • 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 script against built runtime (npm run build:cached, then 4 real compare-process-captures CLI subprocesses) — mixed input now names manifest.completed_at with (1 more issues); valid exits 0
  • npm run test:local -- src/domain/process/*.test.ts src/process/capture/*.test.ts src/application/ReconstructionObligationCandidates.test.ts — 89 files, 1523 passed, 3 skipped
  • npm run test:local -- src/application/ReconstructionObligationLedger.closure.test.ts src/domain/reconstructionVerification*.test.ts — 87 passed
  • npm run test:focused -- tests/boundary/process/*.test.ts (non-host) tests/acceptance/process/*.test.ts tests/boundary/mcp/process*.test.ts tests/boundary/mcp/promptCompletion.test.ts tests/boundary/filesystem/evidenceBundleFiles.test.ts tests/boundary/mcp/evidenceExportCancellation.test.ts — 728 passed, 21 skipped, 1 failed: filesystemObservationRaceCli.test.ts > keeps a stable symlink observation complete (truncated: true). That test fails the same way on unmodified base 6d356b843 on this machine, so this PR did not cause it.
  • New regression tests fail on base with the source fix stashed — confirmed
  • npm run check:fast — passed
  • npm run format:check — passed

Compatibility, safety, and release

  • Breaking changes or migration steps: none. parseProcessCapture now throws ZodError for semantic failures instead of TypeError. It is an internal domain function, and all in-repo callers already handle ZodError.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable; macOS arm64 local only.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no new I/O or target interaction.

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/6020 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/21/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/5957-capture-parse-structural-issues` --- ### 📝 Commits (1) - [`f3cc3d7`](https://github.com/morluto/rea/commit/f3cc3d7dfe49ab3335e6bb5d50b1f41936c38535) fix(process): keep structural capture issues alongside semantic ones ### 📊 Changes **5 files changed** (+121 additions, -30 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ReconstructionObligationCandidates.test.ts` (+36 -0) 📝 `src/application/ReconstructionObligationCandidates.ts` (+3 -5) 📝 `src/domain/process/processCapture.validation.test.ts` (+22 -9) 📝 `src/domain/process/processCaptureParsing.ts` (+6 -15) 📝 `tests/boundary/mcp/processComparisonDiagnosticsMcp.test.ts` (+54 -1) </details> ### 📄 Description **What broke:** `parseProcessCapture` re-threw only the semantic (`custom`) schema issues whenever any existed, so a capture that also had a structural issue (e.g. a missing `manifest.completed_at`) reported only the semantic one, with no remaining-issue count. **Fix:** throw the schema's own `ZodError` so every issue is retained. `describeValidationFailure` then names the first issue and the number remaining. Reconstruction obligation candidates now use that formatter too. **Risk:** low. Rejection and error classification are unchanged. Semantic-only failures now read `at <path>: <message>` instead of `Invalid process capture: <path>: <message>`, the same form structural failures already used. ## Summary Fixes #5957. A process capture with both a structural and a semantic schema issue lost the structural issue in the parser before any CLI/MCP caller saw it. Now all issues stay on the thrown `ZodError`, and the existing bounded formatter reports the first one plus `(N more issues)`. ## Problem and expected behavior Trigger: start from `FINALIZED_PROCESS_CAPTURE_EXAMPLE`, delete `manifest.completed_at`, and set `exit.finalization.signals[0].reason`. The schema reports two issues. `parseProcessCapture` kept only the `custom` subset and threw it as a `TypeError`, so `compare-process-captures`, MCP `compare_process_captures`, bundle validation and obligation candidates showed only the finalization issue. Expected: keep every issue found in that parse, or at least the first one and a count of the rest. ## Change and scope - `src/domain/process/processCaptureParsing.ts`: `parseProcessCapture` is now `attachProof(processCaptureSchema.parse(input))`. This removes the `custom`-only re-packaging, which was the only place issues were dropped. All consumers already accepted `ZodError` and formatted it with `describeValidationFailure` (CLI, MCP comparison, Evidence bundle files), or caught `TypeError | ZodError` (prompt completion, reconstruction verification). - `src/application/ReconstructionObligationCandidates.ts`: this was the one consumer that formatted errors itself, using `cause.message`. For a `ZodError` that message is the full JSON issue dump, so it now uses `describeValidationFailure`. The local `errorMessage` helper is removed. - No other parser has this issue-subsetting pattern (`grep` for `code === "custom"` in `src/` found only this one). Native provider reply parsers that report `issues[0]` only are a separate concern and are not changed here. Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: process capture parsing (`parseProcessCapture`). - Contribution scope fit and linked maintainer decision: not applicable (bug fix). - CLI and MCP/tool-catalog contract: no schema or catalog change. Codes and categories are unchanged (`invalid_request`/`invalid_input` on the CLI, `evidence_integrity_mismatch` on MCP). Message text changes as described above. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. Obligation-candidate limitation text now uses the shared bounded description. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: - `tests/boundary/mcp/processComparisonDiagnosticsMcp.test.ts`: a mixed structural+semantic capture through MCP `compare_process_captures` and the CLI `compareProcessEvidenceFiles`. Both must report `at manifest.completed_at: ... (1 more issues)`. This test fails on base, which reports only the finalization issue. - `src/application/ReconstructionObligationCandidates.test.ts`: the same mixed capture must produce a limitation that names the first issue and the count. This test fails on base. - `src/domain/process/processCapture.validation.test.ts`: three assertions that matched joined paths in the `TypeError` message now check the rejected paths on the `ZodError`. - Base reproduction: on `6d356b843`, the mixed capture produced `Capture evidence is malformed (Invalid process capture: exit.finalization.signals.0.reason: finalization reason is only allowed for an unverified delivery)`. - User-visible CLI output after the fix (built `scripts/rea.mjs compare-process-captures --json`, issue script inputs): - mixed: `Capture evidence is malformed (at manifest.completed_at: Invalid input: expected string, received undefined (1 more issues)). Create new capture evidence, then try again.` (exit 1, `invalid_request`) - semantic only: `Capture evidence is malformed (at exit.finalization.signals.0.reason: finalization reason is only allowed for an unverified delivery). ...` - structural only: unchanged. Valid: exit 0. - Remaining proof gaps: not run here: live capture, MCP bundle import end-to-end, other platforms, and the full suite. Performance: the parse does the same single schema pass as before and drops one `flatMap`. No measurable cost change is expected; CLI wall time was about 375 ms per run, dominated by startup. - [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 script against built runtime (`npm run build:cached`, then 4 real `compare-process-captures` CLI subprocesses) — mixed input now names `manifest.completed_at` with `(1 more issues)`; valid exits 0 - `npm run test:local -- src/domain/process/*.test.ts src/process/capture/*.test.ts src/application/ReconstructionObligationCandidates.test.ts` — 89 files, 1523 passed, 3 skipped - `npm run test:local -- src/application/ReconstructionObligationLedger.closure.test.ts src/domain/reconstructionVerification*.test.ts` — 87 passed - `npm run test:focused -- tests/boundary/process/*.test.ts (non-host) tests/acceptance/process/*.test.ts tests/boundary/mcp/process*.test.ts tests/boundary/mcp/promptCompletion.test.ts tests/boundary/filesystem/evidenceBundleFiles.test.ts tests/boundary/mcp/evidenceExportCancellation.test.ts` — 728 passed, 21 skipped, 1 failed: `filesystemObservationRaceCli.test.ts > keeps a stable symlink observation complete` (`truncated: true`). That test fails the same way on unmodified base `6d356b843` on this machine, so this PR did not cause it. - New regression tests fail on base with the source fix stashed — confirmed - `npm run check:fast` — passed - `npm run format:check` — passed ## Compatibility, safety, and release - Breaking changes or migration steps: none. `parseProcessCapture` now throws `ZodError` for semantic failures instead of `TypeError`. It is an internal domain function, and all in-repo callers already handle `ZodError`. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; macOS arm64 local only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new I/O or target interaction. ## 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 - [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. 🤖 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:45:35 +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#7125
No description provided.