[PR #6247] [MERGED] fix(environment): report capture payload failures at their field paths #7224

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/6119-environment-payload-error-path

📝 Commits (1)

fc0f3f7 fix(environment): report capture payload failures at their field paths

📊 Changes

2 files changed (+311 additions, -89 deletions)

View changed files

📝 src/application/environment/environmentSource.test.ts (+138 -2)
📝 src/application/environment/environmentSource.ts (+173 -87)

📄 Description

What broke: environment-serve and environment-export refused authenticated capture Evidence with a supported operation but a malformed normalized_result by blaming evidence.operation, and dropped the real schema issues.
Fix: parseCaptureEvidence now dispatches on the operation to per-kind parsers. Each one reports its schema issues beneath evidence.normalized_result and keeps the Zod failure as the cause. The view parser used for the manifest and state files likewise separates operation, kind and payload failures.
Risk: Low. These inputs were already refused. Only the issue paths and messages change, with the same invalid_request/invalid_input code and recovery. Valid pairs load as before.

Summary

Fixes #6119.

Before this change, loadEnvironmentSource returned [{ path: ["evidence","operation"], expected: "inspect_web_network_capture or capture_browser_scenario" }] for capture Evidence whose operation was already one of those values, whenever the payload failed its capture schema. Now an operation no capture schema owns still blames evidence.operation. For a recognized operation, the refusal lists the fields that failed the selected schema, at their full paths.

Problem and expected behavior

Smallest trigger: digest-valid Evidence {operation: "inspect_web_network_capture" | "capture_browser_scenario", normalized_result: {}}, paired with a manifest that cites it. Both recognized branches returned only when the schema matched. When it did not, they fell through to the generic operation refusal. The correction metadata then pointed at a valid field, and the caller had no way to find the field that actually failed.

Change and scope

src/application/environment/environmentSource.ts:
- schemaInputError is the single place that prefixes projected Zod issues with a file-role path and retains the cause. parseEvidenceFile (authentication), the view payload and both capture payloads use it.
- parseCaptureEvidence dispatches to parseScenarioCapture and parseHarCapture. Each returns its schema failure under evidence.normalized_result. The non-HAR format refusal is unchanged. Only an operation that neither parser owns reaches the evidence.operation refusal.
- parseViewEvidence had the same conflation: a wrong operation, a wrong or unknown view kind, and a malformed payload of the requested kind all blamed <role>.normalized_result.kind. It now reports <role>.operation, <role>.normalized_result.kind or the payload field paths, in that order. The full view schema is parsed only once the discriminator matches.
Not included: no message rewriting downstream, no change to the shared projectInputIssues helper, and no change to how many schema issues are reported. That count already follows the existing envelope convention.

Contribution scope: not applicable. This is an ordinary fix within existing boundaries.

Contract and boundary impact

Semantic owner and earliest changed stage: the environment Evidence loader (environmentSource.ts), during request validation before pairing.
Contribution scope fit and linked maintainer decision: not applicable (ordinary fix).
CLI and MCP/tool-catalog contract: no schema change. Refusal details.issues paths and message text change for malformed recognized payloads and for wrong-operation manifest or state files.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none. Authentication still runs first, so tampered Evidence still stops there.
Target selection, authority, upstream behavior, and cleanup details: not applicable. No listener is started and no bundle is written on refusal, as before.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.

Evidence and regression coverage

Tests added (src/application/environment/environmentSource.test.ts, new "payload attribution" block):
- Both capture operations with {}, under both environment-serve and environment-export: every issue path starts with evidence.normalized_result, the cause is retained, and the code and command stay invalid_request/invalid_input.
- Nested fields: HAR artifact.sha256 = 42 gives evidence.normalized_result.artifact.sha256 with reason invalid_type. Scenario browser.product = 7 gives evidence.normalized_result.browser.product.
- An unknown operation (inspect_other_capture) still gives exactly one evidence.operation issue.
- Manifest file: capture Evidence gives manifest.operation; a malformed name gives manifest.normalized_result.name; an unknown kind gives manifest.normalized_result.kind.
- Existing tests still cover valid pairs loading, the other view kind, a non-HAR format, a parent mismatch and tampered Evidence stopping at authentication.
Base reproduction: the issue's loader test, run as a source test on base d77bc0e8a, printed {"issues":[{"path":["evidence","operation"],...}]} for both operations and both commands. With only environmentSource.ts reverted, the 3 new tests that assert the corrected paths fail. The unknown-operation counterexample passes on both, as expected.

User-visible CLI output (built node scripts/rea.mjs, the issue's synthetic Evidence pairs, all 4 runs exited 1 with no bundle directory created):

inspect_web_network_capture serve/export: "The evidence file is not a complete web network capture." ["evidence.normalized_result.format"]
capture_browser_scenario serve/export:   "The evidence file is not a complete browser scenario capture." ["evidence.normalized_result.browser", ".scenario", ".duration_ms", ".steps", ".events", ".completeness", ".limitations"]

Remaining proof gaps: no browser or real capture producer was run. The malformed inputs are synthetic, digest-valid Evidence, as in the issue.
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

Rebased onto 4a4bbab3c; the test suites, check:fast and format:check below were rerun after the rebase. The issue repro and the built-CLI run were done before the rebase, on base d77bc0e8a; this code did not change in the rebase.

Issue repro (temporary source test, not committed): base reproduced the evidence.operation attribution; with the fix the issues are under evidence.normalized_result.
npm run test:local -- src/application/environment/*.test.ts (13 files): 135 passed.
npm run test:focused -- tests/boundary/mcp/environmentServeMcp.test.ts tests/boundary/cli/environmentExportCli.test.ts tests/boundary/cli/environmentServeRemediation.test.ts tests/boundary/cli/environmentServeCli.test.ts tests/boundary/cli/environmentExportInputsCli.test.ts tests/boundary/process/environmentServeShutdown.test.ts tests/boundary/cli/cliStartupImports.test.ts: 35 passed.
Built CLI environment-serve / environment-export on the issue's pairs: output shown above.
npm run check:fast: passed.
npm run format:check: passed.

Compatibility, safety, and release

Breaking changes or migration steps: none. These are refusal correction details for inputs that were already refused.
Real Hopper/Ghidra, browser, or OS coverage: none needed. No browser was run.
Package or release metadata impact: none.
Security, privacy, process, or containment review: issues go through the existing secret-safe projectInputIssues. The Zod cause is retained internally and is not in the projected output.

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/6247 **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/6119-environment-payload-error-path` --- ### 📝 Commits (1) - [`fc0f3f7`](https://github.com/morluto/rea/commit/fc0f3f7f61d44c4167ee3ddd8e43e28a2890610e) fix(environment): report capture payload failures at their field paths ### 📊 Changes **2 files changed** (+311 additions, -89 deletions) <details> <summary>View changed files</summary> 📝 `src/application/environment/environmentSource.test.ts` (+138 -2) 📝 `src/application/environment/environmentSource.ts` (+173 -87) </details> ### 📄 Description **What broke:** `environment-serve` and `environment-export` refused authenticated capture Evidence with a supported operation but a malformed `normalized_result` by blaming `evidence.operation`, and dropped the real schema issues. **Fix:** `parseCaptureEvidence` now dispatches on the operation to per-kind parsers. Each one reports its schema issues beneath `evidence.normalized_result` and keeps the Zod failure as the cause. The view parser used for the manifest and state files likewise separates operation, kind and payload failures. **Risk:** Low. These inputs were already refused. Only the issue paths and messages change, with the same `invalid_request`/`invalid_input` code and recovery. Valid pairs load as before. ## Summary Fixes #6119. Before this change, `loadEnvironmentSource` returned `[{ path: ["evidence","operation"], expected: "inspect_web_network_capture or capture_browser_scenario" }]` for capture Evidence whose operation was already one of those values, whenever the payload failed its capture schema. Now an operation no capture schema owns still blames `evidence.operation`. For a recognized operation, the refusal lists the fields that failed the selected schema, at their full paths. ## Problem and expected behavior Smallest trigger: digest-valid Evidence `{operation: "inspect_web_network_capture" | "capture_browser_scenario", normalized_result: {}}`, paired with a manifest that cites it. Both recognized branches returned only when the schema matched. When it did not, they fell through to the generic operation refusal. The correction metadata then pointed at a valid field, and the caller had no way to find the field that actually failed. ## Change and scope - `src/application/environment/environmentSource.ts`: - `schemaInputError` is the single place that prefixes projected Zod issues with a file-role path and retains the cause. `parseEvidenceFile` (authentication), the view payload and both capture payloads use it. - `parseCaptureEvidence` dispatches to `parseScenarioCapture` and `parseHarCapture`. Each returns its schema failure under `evidence.normalized_result`. The non-HAR format refusal is unchanged. Only an operation that neither parser owns reaches the `evidence.operation` refusal. - `parseViewEvidence` had the same conflation: a wrong operation, a wrong or unknown view kind, and a malformed payload of the requested kind all blamed `<role>.normalized_result.kind`. It now reports `<role>.operation`, `<role>.normalized_result.kind` or the payload field paths, in that order. The full view schema is parsed only once the discriminator matches. - Not included: no message rewriting downstream, no change to the shared `projectInputIssues` helper, and no change to how many schema issues are reported. That count already follows the existing envelope convention. Contribution scope: not applicable. This is an ordinary fix within existing boundaries. ## Contract and boundary impact - Semantic owner and earliest changed stage: the environment Evidence loader (`environmentSource.ts`), during request validation before pairing. - Contribution scope fit and linked maintainer decision: not applicable (ordinary fix). - CLI and MCP/tool-catalog contract: no schema change. Refusal `details.issues` paths and `message` text change for malformed recognized payloads and for wrong-operation manifest or state files. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. Authentication still runs first, so tampered Evidence still stops there. - Target selection, authority, upstream behavior, and cleanup details: not applicable. No listener is started and no bundle is written on refusal, as before. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added (`src/application/environment/environmentSource.test.ts`, new "payload attribution" block): - Both capture operations with `{}`, under both `environment-serve` and `environment-export`: every issue path starts with `evidence.normalized_result`, the cause is retained, and the code and command stay `invalid_request`/`invalid_input`. - Nested fields: HAR `artifact.sha256 = 42` gives `evidence.normalized_result.artifact.sha256` with reason `invalid_type`. Scenario `browser.product = 7` gives `evidence.normalized_result.browser.product`. - An unknown operation (`inspect_other_capture`) still gives exactly one `evidence.operation` issue. - Manifest file: capture Evidence gives `manifest.operation`; a malformed `name` gives `manifest.normalized_result.name`; an unknown kind gives `manifest.normalized_result.kind`. - Existing tests still cover valid pairs loading, the other view kind, a non-HAR format, a parent mismatch and tampered Evidence stopping at authentication. - Base reproduction: the issue's loader test, run as a source test on base `d77bc0e8a`, printed `{"issues":[{"path":["evidence","operation"],...}]}` for both operations and both commands. With only `environmentSource.ts` reverted, the 3 new tests that assert the corrected paths fail. The unknown-operation counterexample passes on both, as expected. - User-visible CLI output (built `node scripts/rea.mjs`, the issue's synthetic Evidence pairs, all 4 runs exited 1 with no bundle directory created): ``` inspect_web_network_capture serve/export: "The evidence file is not a complete web network capture." ["evidence.normalized_result.format"] capture_browser_scenario serve/export: "The evidence file is not a complete browser scenario capture." ["evidence.normalized_result.browser", ".scenario", ".duration_ms", ".steps", ".events", ".completeness", ".limitations"] ``` - Remaining proof gaps: no browser or real capture producer was run. The malformed inputs are synthetic, digest-valid Evidence, as in the issue. - [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 Rebased onto `4a4bbab3c`; the test suites, `check:fast` and `format:check` below were rerun after the rebase. The issue repro and the built-CLI run were done before the rebase, on base `d77bc0e8a`; this code did not change in the rebase. - Issue repro (temporary source test, not committed): base reproduced the `evidence.operation` attribution; with the fix the issues are under `evidence.normalized_result`. - `npm run test:local -- src/application/environment/*.test.ts` (13 files): 135 passed. - `npm run test:focused -- tests/boundary/mcp/environmentServeMcp.test.ts tests/boundary/cli/environmentExportCli.test.ts tests/boundary/cli/environmentServeRemediation.test.ts tests/boundary/cli/environmentServeCli.test.ts tests/boundary/cli/environmentExportInputsCli.test.ts tests/boundary/process/environmentServeShutdown.test.ts tests/boundary/cli/cliStartupImports.test.ts`: 35 passed. - Built CLI `environment-serve` / `environment-export` on the issue's pairs: output shown above. - `npm run check:fast`: passed. - `npm run format:check`: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. These are refusal correction details for inputs that were already refused. - Real Hopper/Ghidra, browser, or OS coverage: none needed. No browser was run. - Package or release metadata impact: none. - Security, privacy, process, or containment review: issues go through the existing secret-safe `projectInputIssues`. The Zod cause is retained internally and is not in the projected output. ## 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:46:16 +00:00