[PR #6753] [CLOSED] fix(environment): preserve astral token boundaries #7406

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6753
Author: @gongfpp
Created: 10/23/2026
Status: ❌ Closed

Base: main ← Head: fix/environment-unicode-delimiters-6635


📝 Commits (1)

  • 56fae12 fix(environment): preserve astral token boundaries

📊 Changes

3 files changed (+99 additions, -6 deletions)

View changed files

📝 src/domain/analysisView/environmentStateWriteValues.test.ts (+12 -0)
📝 src/domain/analysisView/environmentStateWriteValues.ts (+5 -5)
📝 tests/boundary/mcp/environmentServeWritesMcp.test.ts (+82 -1)

📄 Description

Summary

Fixes #6635. Preserve astral Unicode words when rebuilding delimited identifiers in captured environment state.

Problem and expected behavior

delimitedOffsets checked one UTF-16 code unit on either side of a match. Half of an astral letter or number does not match the existing Unicode letter/number predicate, so a token inside 𐐀3𐐁 or 𝟙3𝟚 was incorrectly eligible for replacement. A later served create could silently change those strings to contain the new entity id.

Complete neighboring code points must determine word boundaries; returned offsets must remain UTF-16 indices for existing slice-based consumers. Emoji and path punctuation must remain valid delimiters.

Change and scope

Inspect at most two UTF-16 units per neighbor with anchored versions of the existing Unicode letter/number predicate. Preserve empty-token handling, accepted-match stepping, and rejected overlapping-match search. Remove the superseded single-unit helper; existing consumers require no changes.

This is a narrow correction at the shared producer, not a state-target redesign or a new provider. PR #6385 has adjacent consumer/test work but does not change this helper; this PR does not alter its consumers or shared fixtures.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Contract and boundary impact

  • Semantic owner and earliest changed stage: delimitedOffsets in environmentStateWriteValues.ts, before template inference/replacement.
  • Contribution scope fit and linked maintainer decision: not applicable; ordinary correctness fix within existing captured-state reconstruction boundaries. No new execution, injection, or target-mutation capability.
  • CLI and MCP/tool-catalog contract: no schema, command, or catalog changes; existing served create/read tools now preserve Unicode words correctly.
  • Provider, bridge, target-format, or platform compatibility: no new dependency or platform requirement; offsets remain UTF-16.
  • Evidence, artifact, provenance, or reconstruction contract: existing capture identities and provenance unchanged; only incorrectly inferred token boundaries are corrected.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): unchanged. Boundary tests use synthetic captures, the compiled local CLI, a loopback MCP server, and the existing managed-process/temporary-directory fixtures; no external target or browser is required.
  • 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; generated artifacts checked, not committed.

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.

Process/network effects above describe the existing local CLI/MCP verification workflow, not new target interaction. The test server contacts only its loopback client.

Evidence and regression coverage

  • Tests added or updated: twelve delimiter-table cases covering astral letters/numbers on both sides, mixed rejected/accepted matches, UTF-16 offsets after emoji, astral tokens, and ASCII/path controls. Existing overlap and empty-token cases retained.
  • Base reproduction or other evidence: applied only the final two test-file changes to a detached 2bcd7e6d13f0a8118d6619718254255e84f75d86 worktree. The two-file run failed with 9 failures (8 delimiter cases plus the served MCP regression), demonstrating that the final fixture reaches the bug rather than merely failing to expose a tool.
  • User-visible CLI/MCP output (if applicable): the added boundary test starts the compiled rea environment-serve, connects the real MCP SDK client, and creates/reads ids 3 and 4. For id 4, letter: "𐐀3𐐁", digit: "𝟙3𝟚", and the one-sided Unicode words remain unchanged, while emoji: "😀4😀" and path: "/books/4" update. Create/read return status 201/200 and state-backed typed results. The affected base instead rewrites the Unicode words with id 4.
  • Remaining proof gaps: synthetic captured-state workflow, not a live browser/provider capture. The full test suite and other operating systems were not run locally. Independent review was static; runtime evidence comes from the executed tests below.

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

Executed on macOS with Node 24.22.1 and npm 11.16.0, on head 56fae12c175cf285a051d3426134a59081a78d35 rebased onto main 5f702dd6ccd9e8e505e224854b91678439c3b41f:

  • npm run test:focused -- src/domain/analysisView/environmentStateWriteValues.test.ts src/domain/analysisView/environmentStateWriteExplain.test.ts src/domain/analysisView/environmentStateWriteApply.test.ts src/domain/analysisView/environmentStateWrites.test.ts src/application/environment/environmentStateWriteAnswer.test.ts tests/boundary/mcp/environmentServeWritesMcp.test.ts — 420 passed, including both real CLI/MCP boundary tests.
  • npm run check:fast — typecheck and lint passed.
  • npm run docs:check — generated metadata/document facts/license checks passed; README consistency 9 passed.
  • git diff --check — passed; worktree clean after commit/rebase.

Current-head CI status

CI 39967057574 on 56fae12c175cf285a051d3426134a59081a78d35 finished failure, not green. Its Source tests report four failures: environment bundle discovery-budget fixture, repeated-query exactness fixture, webpack:// source-map resolution, and ELF note remediation text. I ran those same four suites on the unmodified exact parent 5f702dd6ccd9e8e505e224854b91678439c3b41f (Node 24.22.1/npm 11.16.0): the same four cases fail, 36 controls pass. These are upstream production/test-contract conflicts; this Unicode delimiter PR does not include unrelated fixes. The existing base CI is separate evidence, not claimed successful. Built-test attribution is now complete for the observed failures: only shard 5 actually failed, with the two javascriptIndirectLoaderOwners.test.ts require.resolve CLI/MCP cases (resolved expected, unavailable observed). The unmodified exact parent shard 5 independently failed the same two cases, at the same assertion and with identical totals (2 failed / 1879 passed / 35 skipped). Shards 3/4/6/8 were cancelled by matrix fail-fast, not four additional demonstrated regressions; cancelled coverage is not claimed passed. Portable tests and CI required are downstream failures. The current-head CI remains red; no unrelated source, loader-ownership or CI-policy changes are bundled here. Baseline coordination is needed before claiming full qualification.

Compatibility, safety, and release

  • Breaking changes or migration steps: none; preserve the documented Unicode letter/number boundary rule and existing UTF-16 offset contract.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable to this pure producer fix; no such provider exercised. Real compiled CLI/MCP exercised locally on macOS.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no new authority or external service access; synthetic data only. No speculative security expansion or unrelated cleanup.

Author readiness

Human owner: @gongfpp

  • 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/6753 **Author:** [@gongfpp](https://github.com/gongfpp) **Created:** 10/23/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/environment-unicode-delimiters-6635` --- ### 📝 Commits (1) - [`56fae12`](https://github.com/morluto/rea/commit/56fae12c175cf285a051d3426134a59081a78d35) fix(environment): preserve astral token boundaries ### 📊 Changes **3 files changed** (+99 additions, -6 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/analysisView/environmentStateWriteValues.test.ts` (+12 -0) 📝 `src/domain/analysisView/environmentStateWriteValues.ts` (+5 -5) 📝 `tests/boundary/mcp/environmentServeWritesMcp.test.ts` (+82 -1) </details> ### 📄 Description ## Summary Fixes #6635. Preserve astral Unicode words when rebuilding delimited identifiers in captured environment state. ## Problem and expected behavior `delimitedOffsets` checked one UTF-16 code unit on either side of a match. Half of an astral letter or number does not match the existing Unicode letter/number predicate, so a token inside `𐐀3𐐁` or `𝟙3𝟚` was incorrectly eligible for replacement. A later served create could silently change those strings to contain the new entity id. Complete neighboring code points must determine word boundaries; returned offsets must remain UTF-16 indices for existing slice-based consumers. Emoji and path punctuation must remain valid delimiters. ## Change and scope Inspect at most two UTF-16 units per neighbor with anchored versions of the existing Unicode letter/number predicate. Preserve empty-token handling, accepted-match stepping, and rejected overlapping-match search. Remove the superseded single-unit helper; existing consumers require no changes. This is a narrow correction at the shared producer, not a state-target redesign or a new provider. PR #6385 has adjacent consumer/test work but does not change this helper; this PR does not alter its consumers or shared fixtures. 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. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. ## Contract and boundary impact - Semantic owner and earliest changed stage: `delimitedOffsets` in `environmentStateWriteValues.ts`, before template inference/replacement. - Contribution scope fit and linked maintainer decision: not applicable; ordinary correctness fix within existing captured-state reconstruction boundaries. No new execution, injection, or target-mutation capability. - CLI and MCP/tool-catalog contract: no schema, command, or catalog changes; existing served create/read tools now preserve Unicode words correctly. - Provider, bridge, target-format, or platform compatibility: no new dependency or platform requirement; offsets remain UTF-16. - Evidence, artifact, provenance, or reconstruction contract: existing capture identities and provenance unchanged; only incorrectly inferred token boundaries are corrected. - Target selection, authority, upstream behavior, and cleanup details (if applicable): unchanged. Boundary tests use synthetic captures, the compiled local CLI, a loopback MCP server, and the existing managed-process/temporary-directory fixtures; no external target or browser is required. - 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; generated artifacts checked, not committed. 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. - [x] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [x] 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. Process/network effects above describe the existing local CLI/MCP verification workflow, not new target interaction. The test server contacts only its loopback client. ## Evidence and regression coverage - Tests added or updated: twelve delimiter-table cases covering astral letters/numbers on both sides, mixed rejected/accepted matches, UTF-16 offsets after emoji, astral tokens, and ASCII/path controls. Existing overlap and empty-token cases retained. - Base reproduction or other evidence: applied only the final two test-file changes to a detached `2bcd7e6d13f0a8118d6619718254255e84f75d86` worktree. The two-file run failed with **9 failures** (8 delimiter cases plus the served MCP regression), demonstrating that the final fixture reaches the bug rather than merely failing to expose a tool. - User-visible CLI/MCP output (if applicable): the added boundary test starts the compiled `rea environment-serve`, connects the real MCP SDK client, and creates/reads ids 3 and 4. For id 4, `letter: "𐐀3𐐁"`, `digit: "𝟙3𝟚"`, and the one-sided Unicode words remain unchanged, while `emoji: "😀4😀"` and `path: "/books/4"` update. Create/read return status 201/200 and state-backed typed results. The affected base instead rewrites the Unicode words with id 4. - Remaining proof gaps: synthetic captured-state workflow, not a live browser/provider capture. The full test suite and other operating systems were not run locally. Independent review was static; runtime evidence comes from the executed tests below. 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 Executed on macOS with Node **24.22.1** and npm **11.16.0**, on head `56fae12c175cf285a051d3426134a59081a78d35` rebased onto main `5f702dd6ccd9e8e505e224854b91678439c3b41f`: - `npm run test:focused -- src/domain/analysisView/environmentStateWriteValues.test.ts src/domain/analysisView/environmentStateWriteExplain.test.ts src/domain/analysisView/environmentStateWriteApply.test.ts src/domain/analysisView/environmentStateWrites.test.ts src/application/environment/environmentStateWriteAnswer.test.ts tests/boundary/mcp/environmentServeWritesMcp.test.ts` — **420 passed**, including both real CLI/MCP boundary tests. - `npm run check:fast` — typecheck and lint passed. - `npm run docs:check` — generated metadata/document facts/license checks passed; README consistency **9 passed**. - `git diff --check` — passed; worktree clean after commit/rebase. ### Current-head CI status CI [39967057574](https://github.com/morluto/rea/actions/runs/39967057574) on `56fae12c175cf285a051d3426134a59081a78d35` finished **failure**, not green. Its [Source tests](https://github.com/morluto/rea/actions/runs/39967057574/job/120460486845) report four failures: environment bundle discovery-budget fixture, repeated-query exactness fixture, `webpack://` source-map resolution, and ELF note remediation text. I ran those same four suites on the **unmodified exact parent** `5f702dd6ccd9e8e505e224854b91678439c3b41f` (Node 24.22.1/npm 11.16.0): **the same four cases fail, 36 controls pass**. These are upstream production/test-contract conflicts; this Unicode delimiter PR does not include unrelated fixes. The existing [base CI](https://github.com/morluto/rea/actions/runs/39966102793) is separate evidence, not claimed successful. Built-test attribution is now complete for the observed failures: only [shard 5](https://github.com/morluto/rea/actions/runs/39967057574/job/120460882736) actually failed, with the two `javascriptIndirectLoaderOwners.test.ts` require.resolve CLI/MCP cases (`resolved` expected, `unavailable` observed). The **unmodified exact parent** [shard 5](https://github.com/morluto/rea/actions/runs/39966102793/job/120470561053) independently failed the **same two cases**, at the same assertion and with identical totals (2 failed / 1879 passed / 35 skipped). Shards 3/4/6/8 were **cancelled by matrix fail-fast**, not four additional demonstrated regressions; cancelled coverage is not claimed passed. Portable tests and CI required are downstream failures. The current-head CI remains red; no unrelated source, loader-ownership or CI-policy changes are bundled here. Baseline coordination is needed before claiming full qualification. ## Compatibility, safety, and release - Breaking changes or migration steps: none; preserve the documented Unicode letter/number boundary rule and existing UTF-16 offset contract. - Real Hopper/Ghidra, browser, or OS coverage: not applicable to this pure producer fix; no such provider exercised. Real compiled CLI/MCP exercised locally on macOS. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new authority or external service access; synthetic data only. No speculative security expansion or unrelated cleanup. ## Author readiness Human owner: @gongfpp - [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 - [ ] 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. --- <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:38 +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#7406
No description provided.