morluto/rea

Fork 0

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

[PR #6372] [MERGED] fix(hopper): report invalid regex patterns as pattern input errors #7317

New issue

Closed

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

bsakaguchi commented

2026-11-01 21:47:00 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6372
Author: @Abelo9996
Created: 10/22/2026
Status: ✅ Merged
Merged: 10/23/2026
Merged by: @morluto

Base: main ← Head: fix-5986-hopper-invalid-regex-pattern

📝 Commits (2)

86c79cd fix(hopper): report invalid regex patterns as pattern input errors
6794804 Merge main to pick up the uniqueBy build fix

📊 Changes

2 files changed (+18 additions, -13 deletions)

View changed files

📝 src/hopper/HopperRegexSearch.ts (+0 -5)
📝 tests/boundary/process/regexMatcher.test.ts (+18 -8)

📄 Description

Summary

Fixes #5986.

Hopper search_strings and search_procedures with an invalid regex (mode: "regex", pattern: "(") were refused with the right invalid_request code, but as a HopperRemoteError, so the result had no details.issues naming pattern and no inspect_input_contract recovery. Hopper now uses the shared regex matcher's default invalid-pattern error, which already carries {"path":["pattern"],"reason":"invalid_format","message":"Supply a valid regular expression."} (since #6270) and projects with input-contract recovery. Kuna and IDA already return this shape for the same input.

Problem and expected behavior

HopperRegexSearch.#execute in src/hopper/HopperRegexSearch.ts passed an invalidPattern callback to matchRegexValues that built HopperRemoteError(-32000, "Invalid regex pattern: ...", { diagnosticType: "invalid_request" }). That class projects as invalid_request, but it is a provider-error type, so the projector's AnalysisInputError branches (structured issues and inspect_input_contract recovery) never ran. From the issue, for the string search:

{"exit":1,"code":"invalid_request","category":"invalid_input","message":"Hopper search_strings failed (-32000, invalid_request): Invalid regex pattern: Invalid regular expression: /(/iu: Unterminated group","retryable":false,"operation":"search_strings","issues":null,"recovery":null,"remediation":{"action":"Correct the reported address, document, or arguments and retry."}}

Expected: keep the rejection, code, category, operation and nonzero exit, and add a pattern issue plus inspect_input_contract recovery for the requested operation.

Change and scope

Remove Hopper's invalidPattern override so matchRegexValues falls back to its default AnalysisInputError with the pattern issue. The invalid pattern is caller input rejected locally by the regex worker before any match, not a Hopper bridge failure, so the shared input error is the right owner. Valid searches, inventory failures, cancellation and worker limits are unchanged.

Not included: the Invalid search arguments path a few lines earlier (schema failure of the search arguments) still returns HopperRemoteError. It is a separate case and could get the same treatment in a follow-up. With this change no caller passes invalidPattern any more; I left the hook in RegexMatcher.ts to keep the diff small, and can remove it here if preferred.

Follow the contribution scope.
Scope decision: not applicable (ordinary fix within existing boundaries).

Contract and boundary impact

Semantic owner and earliest changed stage: HopperRegexSearch.#execute, invalid-pattern reply from the shared regex worker.
Contribution scope fit and linked maintainer decision: ordinary bug fix, not applicable.
CLI and MCP/tool-catalog contract: no schema change. Hopper invalid-regex refusals now include details.issues naming pattern and recovery: {"kind":"inspect_input_contract","operation":...}. The message drops the Hopper search_strings failed (-32000, invalid_request): prefix and starts with Invalid regex pattern:.
Provider, bridge, target-format, or platform compatibility: Hopper regex search only. No bridge request changes; the inventory call happens as before.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details (if applicable): unchanged. Worker lifetime, deadline and heap handling are untouched.
Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs): 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.

(Diagnostic metadata only; the invalid pattern is rejected by the existing worker before any match.)

Evidence and regression coverage

Tests added or updated: tests/boundary/process/regexMatcher.test.ts, "keeps Hopper's result projection and invalid-pattern contract with the shared worker". It previously asserted _tag: "HopperRemoteError" for pattern: "["; it now projects the error and asserts invalid_request / invalid_input, the Invalid regex pattern message, inspect_input_contract recovery for search_strings, and the pattern / invalid_format issue.

Base reproduction (executed): with the updated test and without the fix:

× keeps Hopper's result projection and invalid-pattern contract with the shared worker
-     "issues": [
-       {
-         "message": "Supply a valid regular expression.",
...
-   "recovery": {
-     "kind": "inspect_input_contract",
-     "operation": "search_strings",
-   },
      Tests  1 failed | 14 passed (15)

With the fix: 15 passed.

User-visible CLI/MCP output (if applicable): projectAnalysisError for the Hopper search above after the fix (search_procedures is the same apart from the operation):

{"code":"invalid_request","category":"invalid_input","message":"Invalid regex pattern: Invalid regular expression: /[/iu: Unterminated character class","retryable":false,"remediation":{"action":"Read the declared input schema or CLI usage for search_strings; if using MCP and the catalog may be stale, call tools/list. Correct the listed fields, then retry. Do not repeat the unchanged call."},"recovery":{"kind":"inspect_input_contract","operation":"search_strings"},"details":{"operation":"search_strings","issues":[{"path":["pattern"],"reason":"invalid_format","message":"Supply a valid regular expression."}]}}

Remaining proof gaps: no real Hopper. The CLI and MCP paths were not run end to end; the output above is the shared projector both use. details.provider_id is no longer present on this refusal, matching the Kuna and IDA input errors.

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

Run with the pinned Node 24.18.0 on macOS arm64.

npm run test:focused -- tests/boundary/process/regexMatcher.test.ts: 15 passed (1 failure without the fix, shown above)
npm run test:focused -- tests/boundary/process/regexMatcher.test.ts tests/boundary/providers/hopper/hopperRegexFailure.test.ts src/hopper/HopperRegexSearch.test.ts src/process/RegexMatcher.test.ts: 4 files, 85 tests passed
lint:built (0 errors), knip, metadata:check and verify:architecture-guards passed. oxfmt --check on the two changed files is clean. typecheck reports no errors in the changed files; see the note below about the current base.

Compatibility, safety, and release

Breaking changes or migration steps: none. Code and category are unchanged; details.issues and recovery are optional and now present. Callers matching the old Hopper search_* failed (-32000, ...) message prefix will see the shorter message.
Real Hopper/Ghidra, browser, or OS coverage: macOS only, stubbed Hopper client. No real Hopper engine.
Package or release metadata impact: none.
Security, privacy, process, or containment review: the issue names the field with a fixed message; the pattern itself is not added to details.

Author readiness

Human owner: @Abelo9996

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.

_{🔄 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/6372 **Author:** [@Abelo9996](https://github.com/Abelo9996) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/23/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix-5986-hopper-invalid-regex-pattern` --- ### 📝 Commits (2) - [`86c79cd`](https://github.com/morluto/rea/commit/86c79cda5d48b66c8b16e2cdd7828b4197ccf406) fix(hopper): report invalid regex patterns as pattern input errors - [`6794804`](https://github.com/morluto/rea/commit/6794804ba5a9e05aa3feef47ac4eda56e5e5a961) Merge main to pick up the uniqueBy build fix ### 📊 Changes **2 files changed** (+18 additions, -13 deletions) <details> <summary>View changed files</summary> 📝 `src/hopper/HopperRegexSearch.ts` (+0 -5) 📝 `tests/boundary/process/regexMatcher.test.ts` (+18 -8) </details> ### 📄 Description ## Summary Fixes #5986. Hopper `search_strings` and `search_procedures` with an invalid regex (`mode: "regex"`, `pattern: "("`) were refused with the right `invalid_request` code, but as a `HopperRemoteError`, so the result had no `details.issues` naming `pattern` and no `inspect_input_contract` recovery. Hopper now uses the shared regex matcher's default invalid-pattern error, which already carries `{"path":["pattern"],"reason":"invalid_format","message":"Supply a valid regular expression."}` (since #6270) and projects with input-contract recovery. Kuna and IDA already return this shape for the same input. ## Problem and expected behavior `HopperRegexSearch.#execute` in `src/hopper/HopperRegexSearch.ts` passed an `invalidPattern` callback to `matchRegexValues` that built `HopperRemoteError(-32000, "Invalid regex pattern: ...", { diagnosticType: "invalid_request" })`. That class projects as `invalid_request`, but it is a provider-error type, so the projector's `AnalysisInputError` branches (structured issues and `inspect_input_contract` recovery) never ran. From the issue, for the string search: ```json {"exit":1,"code":"invalid_request","category":"invalid_input","message":"Hopper search_strings failed (-32000, invalid_request): Invalid regex pattern: Invalid regular expression: /(/iu: Unterminated group","retryable":false,"operation":"search_strings","issues":null,"recovery":null,"remediation":{"action":"Correct the reported address, document, or arguments and retry."}} ``` Expected: keep the rejection, code, category, operation and nonzero exit, and add a `pattern` issue plus `inspect_input_contract` recovery for the requested operation. ## Change and scope Remove Hopper's `invalidPattern` override so `matchRegexValues` falls back to its default `AnalysisInputError` with the `pattern` issue. The invalid pattern is caller input rejected locally by the regex worker before any match, not a Hopper bridge failure, so the shared input error is the right owner. Valid searches, inventory failures, cancellation and worker limits are unchanged. Not included: the `Invalid search arguments` path a few lines earlier (schema failure of the search arguments) still returns `HopperRemoteError`. It is a separate case and could get the same treatment in a follow-up. With this change no caller passes `invalidPattern` any more; I left the hook in `RegexMatcher.ts` to keep the diff small, and can remove it here if preferred. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Scope decision: not applicable (ordinary fix within existing boundaries). ## Contract and boundary impact - Semantic owner and earliest changed stage: `HopperRegexSearch.#execute`, invalid-pattern reply from the shared regex worker. - Contribution scope fit and linked maintainer decision: ordinary bug fix, not applicable. - CLI and MCP/tool-catalog contract: no schema change. Hopper invalid-regex refusals now include `details.issues` naming `pattern` and `recovery: {"kind":"inspect_input_contract","operation":...}`. The message drops the `Hopper search_strings failed (-32000, invalid_request): ` prefix and starts with `Invalid regex pattern:`. - Provider, bridge, target-format, or platform compatibility: Hopper regex search only. No bridge request changes; the inventory call happens as before. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details (if applicable): unchanged. Worker lifetime, deadline and heap handling are untouched. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): 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. - [x] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. (Diagnostic metadata only; the invalid pattern is rejected by the existing worker before any match.) ## Evidence and regression coverage - Tests added or updated: `tests/boundary/process/regexMatcher.test.ts`, "keeps Hopper's result projection and invalid-pattern contract with the shared worker". It previously asserted `_tag: "HopperRemoteError"` for `pattern: "["`; it now projects the error and asserts `invalid_request` / `invalid_input`, the `Invalid regex pattern` message, `inspect_input_contract` recovery for `search_strings`, and the `pattern` / `invalid_format` issue. - Base reproduction (executed): with the updated test and without the fix: ``` × keeps Hopper's result projection and invalid-pattern contract with the shared worker - "issues": [ - { - "message": "Supply a valid regular expression.", ... - "recovery": { - "kind": "inspect_input_contract", - "operation": "search_strings", - }, Tests 1 failed | 14 passed (15) ``` With the fix: 15 passed. - User-visible CLI/MCP output (if applicable): `projectAnalysisError` for the Hopper search above after the fix (`search_procedures` is the same apart from the operation): ```json {"code":"invalid_request","category":"invalid_input","message":"Invalid regex pattern: Invalid regular expression: /[/iu: Unterminated character class","retryable":false,"remediation":{"action":"Read the declared input schema or CLI usage for search_strings; if using MCP and the catalog may be stale, call tools/list. Correct the listed fields, then retry. Do not repeat the unchanged call."},"recovery":{"kind":"inspect_input_contract","operation":"search_strings"},"details":{"operation":"search_strings","issues":[{"path":["pattern"],"reason":"invalid_format","message":"Supply a valid regular expression."}]}} ``` - Remaining proof gaps: no real Hopper. The CLI and MCP paths were not run end to end; the output above is the shared projector both use. `details.provider_id` is no longer present on this refusal, matching the Kuna and IDA input errors. 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. ## Validation performed Run with the pinned Node 24.18.0 on macOS arm64. - `npm run test:focused -- tests/boundary/process/regexMatcher.test.ts`: 15 passed (1 failure without the fix, shown above) - `npm run test:focused -- tests/boundary/process/regexMatcher.test.ts tests/boundary/providers/hopper/hopperRegexFailure.test.ts src/hopper/HopperRegexSearch.test.ts src/process/RegexMatcher.test.ts`: 4 files, 85 tests passed - `lint:built` (0 errors), `knip`, `metadata:check` and `verify:architecture-guards` passed. `oxfmt --check` on the two changed files is clean. `typecheck` reports no errors in the changed files; see the note below about the current base. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Code and category are unchanged; `details.issues` and `recovery` are optional and now present. Callers matching the old `Hopper search_* failed (-32000, ...)` message prefix will see the shorter message. - Real Hopper/Ghidra, browser, or OS coverage: macOS only, stubbed Hopper client. No real Hopper engine. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the issue names the field with a fixed message; the pattern itself is not added to `details`. ## Author readiness Human owner: @Abelo9996 - [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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:47:00 +00:00