morluto/rea

Fork 0

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

[PR #7152] [MERGED] fix(contracts): advertise a v-mode portable Windows absolute-path pattern #7678

New issue

Closed

opened 2026-11-03 13:39:35 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-03 13:39:35 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7152
Author: @pablodiazjorge
Created: 10/25/2026
Status: ✅ Merged
Merged: 10/26/2026
Merged by: @gensokyo-dev

Base: main ← Head: fix/windows-absolute-path-v-mode

📝 Commits (1)

c9507dd fix(contracts): advertise a v-mode portable Windows absolute-path pattern

📊 Changes

2 files changed (+42 additions, -15 deletions)

View changed files

📝 src/domain/localPath.test.ts (+33 -14)
📝 src/domain/localPath.ts (+9 -1)

📄 Description

Summary

Fixes #7151.

On Windows, every field built on absoluteLocalPathStringSchema advertised ^(?:[\\/]|[A-Za-z]:[\\/]). The unescaped / inside the class compiles under Annex B and u, but v mode rejects it, so a client that compiles advertised patterns in v mode rejects the complete tools request when REA runs on Windows. This PR spells the two separators as hex escapes. The grammar is unchanged, and the pattern compiles in all three modes.

Problem and expected behavior

Smallest trigger: on Windows, tools/list, then new RegExp(pattern, "v") on close_binary.snapshot_path. It throws Invalid character in character class. The default compact catalog has 5 such fields; REA_MCP_INPUT_SCHEMA_PROFILE=full has 56 fields in 43 tools, including open_binary. Linux and macOS advertise ^/ and are not affected.

Expected: advertised patterns compile in Annex B, u and v on every host (the invariant behind #1212, #1581 and advertisedPatternsVMode.test.ts), and the Windows pattern keeps accepting exactly what path.win32.isAbsolute accepts.

Change and scope

src/domain/localPath.ts: the Windows branch becomes windowsAbsolutePathPattern, /^(?:[\x5C\x2F]|[A-Za-z]:[\x5C\x2F])/u, with a comment on why. Hex escapes follow the convention already used for NUL (nulFreeStringSchema) and for the DEX descriptor parentheses (dexDescriptorSchema). The POSIX branch (^/) is unchanged.
The pattern is exported so that a unit test can check the Windows spelling on any host. CI runs the general source suite on Linux, where advertisedPatternsVMode.test.ts only sees ^/.
Not included: the two full-profile patterns in capture_browser_scenario (from #6946), noted in #7151. They fail on every host and belong to a separate change. Extending advertisedPatternsVMode.test.ts to the full profile would fail on main until those are fixed too.

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

Contract and boundary impact

Semantic owner and earliest changed stage: absoluteLocalPathStringSchema in src/domain/localPath.ts, the single producer of the advertised host-absolute path pattern; every consumer inherits it unchanged.
Contribution scope fit and linked maintainer decision: ordinary fix; not applicable.
CLI and MCP/tool-catalog contract: on Windows, the advertised pattern string of host-absolute path fields changes from ^(?:[\\/]|[A-Za-z]:[\\/]) to ^(?:[\x5C\x2F]|[A-Za-z]:[\x5C\x2F]), which accepts the same strings. Linux and macOS catalogs are unchanged.
Provider, bridge, target-format, or platform compatibility: Windows hosts only; no provider change.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none; no tracked generated file changes.

Expected effects:

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: src/domain/localPath.test.ts adds "keeps the Windows absolute-path pattern valid in every client mode": it compiles windowsAbsolutePathPattern with no flags, u and v, and checks it against path.win32.isAbsolute on the existing path list, which now lives in a shared constant. With the original pattern restored, the test fails on v with Invalid character in character class; with this change it passes. It runs on any host.
Base reproduction or other evidence: on Windows at 447907f28, npx vitest run tests/boundary/mcp/advertisedPatternsVMode.test.ts fails with the 5 compact-catalog entries listed in #7151; with this change it passes.

User-visible CLI/MCP output: compiling every pattern advertised by the built server over stdio on Windows:

Profile	`447907f28`	this branch
default (compact)	5 of 1,569 fail in `v` mode	0
`full`	58 of 2,348 fail (56 host-absolute paths, 2 in `capture_browser_scenario`)	2 (the `capture_browser_scenario` ones, out of scope)

Remaining proof gaps: no v-mode client (such as DeepSeek Harness) was run against a Windows host; the check compiles the advertised patterns directly, as the boundary test does.

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

Windows 11 x64 (10.0.26300), Node 24.22.1, on 447907f28 plus this change, with the rea-agents@6.8.0 Windows native controls:

npx vitest run src/domain/localPath.test.ts tests/boundary/mcp/advertisedPatternsVMode.test.ts: 3 passed.
npx vitest run src/domain src/contracts tests/boundary/mcp/toolSchemaValidity.test.ts tests/boundary/mcp/compactInputSchemaMcp.test.ts tests/boundary/mcp/contractPresentation.test.ts tests/boundary/mcp/mcpDoctor.test.ts tests/boundary/mcp/advertisedPatternsVMode.test.ts: 8101 passed.
npm run build, npm run test:prepare, npm run docs:check and npm run check: passed; git status shows only the two source files.
The stdio scan in the table above, with REA_MCP_INPUT_SCHEMA_PROFILE unset and set to full.

Compatibility, safety, and release

Breaking changes or migration steps: none. The Windows pattern accepts exactly the same strings.
Real Hopper/Ghidra, browser, or OS coverage: Windows host for the advertised catalog; no provider is involved.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no change to which paths are admitted.

Author readiness

Human owner: @pablodiazjorge

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/7152 **Author:** [@pablodiazjorge](https://github.com/pablodiazjorge) **Created:** 10/25/2026 **Status:** ✅ Merged **Merged:** 10/26/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `fix/windows-absolute-path-v-mode` --- ### 📝 Commits (1) - [`c9507dd`](https://github.com/morluto/rea/commit/c9507ddc09454f4cf64e99c8c518d2c6ea6ad6d1) fix(contracts): advertise a v-mode portable Windows absolute-path pattern ### 📊 Changes **2 files changed** (+42 additions, -15 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/localPath.test.ts` (+33 -14) 📝 `src/domain/localPath.ts` (+9 -1) </details> ### 📄 Description ## Summary Fixes #7151. On Windows, every field built on `absoluteLocalPathStringSchema` advertised `^(?:[\\/]|[A-Za-z]:[\\/])`. The unescaped `/` inside the class compiles under Annex B and `u`, but `v` mode rejects it, so a client that compiles advertised patterns in `v` mode rejects the complete tools request when REA runs on Windows. This PR spells the two separators as hex escapes. The grammar is unchanged, and the pattern compiles in all three modes. ## Problem and expected behavior Smallest trigger: on Windows, `tools/list`, then `new RegExp(pattern, "v")` on `close_binary.snapshot_path`. It throws `Invalid character in character class`. The default compact catalog has 5 such fields; `REA_MCP_INPUT_SCHEMA_PROFILE=full` has 56 fields in 43 tools, including `open_binary`. Linux and macOS advertise `^/` and are not affected. Expected: advertised patterns compile in Annex B, `u` and `v` on every host (the invariant behind #1212, #1581 and `advertisedPatternsVMode.test.ts`), and the Windows pattern keeps accepting exactly what `path.win32.isAbsolute` accepts. ## Change and scope - `src/domain/localPath.ts`: the Windows branch becomes `windowsAbsolutePathPattern`, `/^(?:[\x5C\x2F]|[A-Za-z]:[\x5C\x2F])/u`, with a comment on why. Hex escapes follow the convention already used for NUL (`nulFreeStringSchema`) and for the DEX descriptor parentheses (`dexDescriptorSchema`). The POSIX branch (`^/`) is unchanged. - The pattern is exported so that a unit test can check the Windows spelling on any host. CI runs the general source suite on Linux, where `advertisedPatternsVMode.test.ts` only sees `^/`. - Not included: the two full-profile patterns in `capture_browser_scenario` (from #6946), noted in #7151. They fail on every host and belong to a separate change. Extending `advertisedPatternsVMode.test.ts` to the full profile would fail on main until those are fixed too. Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `absoluteLocalPathStringSchema` in `src/domain/localPath.ts`, the single producer of the advertised host-absolute path pattern; every consumer inherits it unchanged. - Contribution scope fit and linked maintainer decision: ordinary fix; not applicable. - CLI and MCP/tool-catalog contract: on Windows, the advertised `pattern` string of host-absolute path fields changes from `^(?:[\\/]|[A-Za-z]:[\\/])` to `^(?:[\x5C\x2F]|[A-Za-z]:[\x5C\x2F])`, which accepts the same strings. Linux and macOS catalogs are unchanged. - Provider, bridge, target-format, or platform compatibility: Windows hosts only; no provider change. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none; no tracked generated file changes. Expected effects: - [ ] 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. ## Evidence and regression coverage - Tests added or updated: `src/domain/localPath.test.ts` adds "keeps the Windows absolute-path pattern valid in every client mode": it compiles `windowsAbsolutePathPattern` with no flags, `u` and `v`, and checks it against `path.win32.isAbsolute` on the existing path list, which now lives in a shared constant. With the original pattern restored, the test fails on `v` with `Invalid character in character class`; with this change it passes. It runs on any host. - Base reproduction or other evidence: on Windows at `447907f28`, `npx vitest run tests/boundary/mcp/advertisedPatternsVMode.test.ts` fails with the 5 compact-catalog entries listed in #7151; with this change it passes. - User-visible CLI/MCP output: compiling every pattern advertised by the built server over stdio on Windows: | Profile | `447907f28` | this branch | |---|---|---| | default (compact) | 5 of 1,569 fail in `v` mode | 0 | | `full` | 58 of 2,348 fail (56 host-absolute paths, 2 in `capture_browser_scenario`) | 2 (the `capture_browser_scenario` ones, out of scope) | - Remaining proof gaps: no `v`-mode client (such as DeepSeek Harness) was run against a Windows host; the check compiles the advertised patterns directly, as the boundary test does. 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 Windows 11 x64 (10.0.26300), Node 24.22.1, on `447907f28` plus this change, with the `rea-agents@6.8.0` Windows native controls: - `npx vitest run src/domain/localPath.test.ts tests/boundary/mcp/advertisedPatternsVMode.test.ts`: 3 passed. - `npx vitest run src/domain src/contracts tests/boundary/mcp/toolSchemaValidity.test.ts tests/boundary/mcp/compactInputSchemaMcp.test.ts tests/boundary/mcp/contractPresentation.test.ts tests/boundary/mcp/mcpDoctor.test.ts tests/boundary/mcp/advertisedPatternsVMode.test.ts`: 8101 passed. - `npm run build`, `npm run test:prepare`, `npm run docs:check` and `npm run check`: passed; `git status` shows only the two source files. - The stdio scan in the table above, with `REA_MCP_INPUT_SCHEMA_PROFILE` unset and set to `full`. ## Compatibility, safety, and release - Breaking changes or migration steps: none. The Windows pattern accepts exactly the same strings. - Real Hopper/Ghidra, browser, or OS coverage: Windows host for the advertised catalog; no provider is involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no change to which paths are admitted. ## Author readiness Human owner: @pablodiazjorge - [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-03 13:39:35 +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#7678

No description provided.

Rows
Columns