morluto/rea

Fork 0

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

[PR #6969] [MERGED] fix(functions): name the rejected side and supplied kind in function Evidence errors #7481

New issue

Closed

opened 2026-11-01 21:48:08 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:48:08 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6969
Author: @dhruvac29
Created: 10/24/2026
Status: ✅ Merged
Merged: 10/24/2026
Merged by: @morluto

Base: main ← Head: fix/5932-function-input-kind-diagnostics

📝 Commits (1)

add3fa8 fix(functions): name the rejected side and supplied kind in Evidence errors

📊 Changes

2 files changed (+139 additions, -44 deletions)

View changed files

📝 src/domain/functionDossierEvidence.ts (+19 -4)
📝 tests/boundary/mcp/functionComparisonMcp.test.ts (+120 -40)

📄 Description

Summary

compare_functions already rejected authentic Evidence from another workflow as invalid_request instead of an integrity failure, but the diagnostic did not identify the failing side or the supplied operation/predicate. It only said that analyze_function Evidence with the rea.analysis predicate was required. I added the rejected side (or call-path location), the supplied operation and predicate, and the expected pair to the typed input issue.

Fixes #5932

Problem and expected behavior

Smallest trigger: pass authenticated Evidence with a different operation or predicate to compare_functions as either left or right. The record passes authentication, so this is an input-selection error, not an integrity failure. Before this change the issue path named the side but the message did not, and no expected pair or supplied values were attached. After this change the same call returns invalid_request / invalid_input with recovery.kind = inspect_input_contract, the failing side path, a message containing the supplied operation and predicate, and an expected object containing the analyze_function / rea.analysis pair.

Change and scope

src/domain/functionDossierEvidence.ts: I updated the shared function-Evidence selector to build a location-aware message and attach the expected operation/predicate to the AnalysisInputError issue. The parser is shared by compare_functions and build_call_path, so both report the same detail; build_call_path locations are rendered as functions.0 rather than a left/right side.
tests/boundary/mcp/functionComparisonMcp.test.ts: I replaced the weak wrong-operation test with parameterized MCP coverage for both sides and independent operation/predicate mismatches, a retained-reference case, and a tampered-Evidence integrity control. The existing inline success test remains the positive control.

I intentionally left out changes to authentication, Evidence schemas, comparison semantics, and provider behavior. The artifact-bound check remains a separate selection error with its existing message.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block integration. This is an ordinary diagnostic fix within existing boundaries; no maintainer scope decision is required.

Contract and boundary impact

Semantic owner and earliest changed stage: function Evidence input selection in src/domain/functionDossierEvidence.ts; request parsing/validation.
Contribution scope fit and linked maintainer decision: not applicable — ordinary fix within existing boundaries.
CLI and MCP/tool-catalog contract: no schema or tool-catalog change. The MCP invalid_request error gains the supplied values in details.issues[].message and an expected object.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: unchanged; authentication still runs first, and tampered records remain evidence_integrity_mismatch.
Target selection, authority, upstream behavior, and cleanup details: none.
Generated metadata (docs/public/product-catalog.json), 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.

No target interaction: the change affects only the construction of a typed input error.

Evidence and regression coverage

Tests added or updated: tests/boundary/mcp/functionComparisonMcp.test.ts now covers both left and right, independent operation and predicate mismatches, inline and retained-reference inputs, a valid inline success control, and a tampered-Evidence integrity control.
Base reproduction or other evidence: I derived the base behavior from source rather than running the base revision. On the base revision the new parameterized assertions fail because details.issues[0].message is the fixed expected-only text and expected is absent; the retained-reference case fails for the same reason. The tampered control is expected to pass on both revisions.
User-visible CLI/MCP output (if applicable): I observed the following output through the registered MCP server with inline wrong-operation Evidence (temporary probe, removed afterward):

{
  "code": "invalid_request",
  "category": "invalid_input",
  "message": "Function analysis requires analyze_function Evidence with the rea.analysis predicate; the left Evidence has operation inventory_artifact and predicate rea.analysis.",
  "recovery": {
    "kind": "inspect_input_contract",
    "operation": "compare_functions"
  },
  "details": {
    "operation": "compare_functions",
    "issues": [
      {
        "path": ["left"],
        "reason": "invalid_value",
        "message": "Function analysis requires analyze_function Evidence with the rea.analysis predicate; the left Evidence has operation inventory_artifact and predicate rea.analysis.",
        "expected": {
          "operation": "analyze_function",
          "predicate_type": "rea.analysis"
        }
      }
    ]
  }
}

Remaining proof gaps: no real provider or target is involved. The MCP path is the reported interface; I did not capture separate CLI output.

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.

Not an evidence-bearing change; the checks above confirm the existing Evidence contract is untouched.

Validation performed

I ran npm run test:focused -- tests/boundary/mcp/functionComparisonMcp.test.ts — 16 tests passed.
I ran npm run typecheck — passed.
I ran npm run lint — 0 errors, 2565 warnings; module boundaries checked in 2895 source files.
Pre-push check:fast (typecheck + lint) — passed.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not applicable; the test uses a fixture session and no provider is launched.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none. The message includes only the caller-supplied operation and predicate from authenticated Evidence, not target content.

Author readiness

Human owner: @dhruvac29

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.

_{🔄 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/6969 **Author:** [@dhruvac29](https://github.com/dhruvac29) **Created:** 10/24/2026 **Status:** ✅ Merged **Merged:** 10/24/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/5932-function-input-kind-diagnostics` --- ### 📝 Commits (1) - [`add3fa8`](https://github.com/morluto/rea/commit/add3fa88c74b08f247d570851a61f6d6d668a860) fix(functions): name the rejected side and supplied kind in Evidence errors ### 📊 Changes **2 files changed** (+139 additions, -44 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/functionDossierEvidence.ts` (+19 -4) 📝 `tests/boundary/mcp/functionComparisonMcp.test.ts` (+120 -40) </details> ### 📄 Description ## Summary `compare_functions` already rejected authentic Evidence from another workflow as `invalid_request` instead of an integrity failure, but the diagnostic did not identify the failing side or the supplied operation/predicate. It only said that `analyze_function` Evidence with the `rea.analysis` predicate was required. I added the rejected side (or call-path location), the supplied operation and predicate, and the expected pair to the typed input issue. Fixes #5932 ## Problem and expected behavior Smallest trigger: pass authenticated Evidence with a different operation or predicate to `compare_functions` as either `left` or `right`. The record passes authentication, so this is an input-selection error, not an integrity failure. Before this change the issue path named the side but the message did not, and no expected pair or supplied values were attached. After this change the same call returns `invalid_request` / `invalid_input` with `recovery.kind = inspect_input_contract`, the failing side path, a message containing the supplied operation and predicate, and an `expected` object containing the `analyze_function` / `rea.analysis` pair. ## Change and scope - `src/domain/functionDossierEvidence.ts`: I updated the shared function-Evidence selector to build a location-aware message and attach the expected operation/predicate to the `AnalysisInputError` issue. The parser is shared by `compare_functions` and `build_call_path`, so both report the same detail; `build_call_path` locations are rendered as `functions.0` rather than a left/right side. - `tests/boundary/mcp/functionComparisonMcp.test.ts`: I replaced the weak wrong-operation test with parameterized MCP coverage for both sides and independent operation/predicate mismatches, a retained-reference case, and a tampered-Evidence integrity control. The existing inline success test remains the positive control. I intentionally left out changes to authentication, Evidence schemas, comparison semantics, and provider behavior. The artifact-bound check remains a separate selection error with its existing message. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. This is an ordinary diagnostic fix within existing boundaries; no maintainer scope decision is required. ## Contract and boundary impact - Semantic owner and earliest changed stage: function Evidence input selection in `src/domain/functionDossierEvidence.ts`; request parsing/validation. - Contribution scope fit and linked maintainer decision: not applicable — ordinary fix within existing boundaries. - CLI and MCP/tool-catalog contract: no schema or tool-catalog change. The MCP `invalid_request` error gains the supplied values in `details.issues[].message` and an `expected` object. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: unchanged; authentication still runs first, and tampered records remain `evidence_integrity_mismatch`. - Target selection, authority, upstream behavior, and cleanup details: none. - Generated metadata (`docs/public/product-catalog.json`), 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. - [x] 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. No target interaction: the change affects only the construction of a typed input error. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/mcp/functionComparisonMcp.test.ts` now covers both `left` and `right`, independent operation and predicate mismatches, inline and retained-reference inputs, a valid inline success control, and a tampered-Evidence integrity control. - Base reproduction or other evidence: I derived the base behavior from source rather than running the base revision. On the base revision the new parameterized assertions fail because `details.issues[0].message` is the fixed expected-only text and `expected` is absent; the retained-reference case fails for the same reason. The tampered control is expected to pass on both revisions. - User-visible CLI/MCP output (if applicable): I observed the following output through the registered MCP server with inline wrong-operation Evidence (temporary probe, removed afterward): ```json { "code": "invalid_request", "category": "invalid_input", "message": "Function analysis requires analyze_function Evidence with the rea.analysis predicate; the left Evidence has operation inventory_artifact and predicate rea.analysis.", "recovery": { "kind": "inspect_input_contract", "operation": "compare_functions" }, "details": { "operation": "compare_functions", "issues": [ { "path": ["left"], "reason": "invalid_value", "message": "Function analysis requires analyze_function Evidence with the rea.analysis predicate; the left Evidence has operation inventory_artifact and predicate rea.analysis.", "expected": { "operation": "analyze_function", "predicate_type": "rea.analysis" } } ] } } ``` - Remaining proof gaps: no real provider or target is involved. The MCP path is the reported interface; I did not capture separate CLI output. For evidence-bearing changes: - [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. Not an evidence-bearing change; the checks above confirm the existing Evidence contract is untouched. ## Validation performed - I ran `npm run test:focused -- tests/boundary/mcp/functionComparisonMcp.test.ts` — 16 tests passed. - I ran `npm run typecheck` — passed. - I ran `npm run lint` — 0 errors, 2565 warnings; module boundaries checked in 2895 source files. - Pre-push `check:fast` (typecheck + lint) — passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; the test uses a fixture session and no provider is launched. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none. The message includes only the caller-supplied operation and predicate from authenticated Evidence, not target content. ## Author readiness Human owner: @dhruvac29 - [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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:48:08 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns