morluto/rea

Fork 0

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

[PR #716] [MERGED] fix(setup): accept leading BOM in JSON client configs #2197

New issue

Closed

opened 2026-10-16 21:15:32 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:15:32 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/client-configuration-leading-bom

📝 Commits (1)

fe843ec fix(setup): accept leading BOM in JSON client configs

📊 Changes

3 files changed (+238 additions, -1 deletions)

View changed files

➕ src/application/ClientConfigurationDocument.test.ts (+105 -0)
📝 src/application/ClientConfigurationDocument.ts (+3 -1)
➕ tests/boundary/filesystem/clientConfigurationBom.test.ts (+130 -0)

📄 Description

Summary

Accept a single leading UTF-8 BOM when reading JSON/JSONC client configurations, preserving the original document for edits.

No separate issue: this is a reproduced supported-client configuration mismatch.

Problem and expected behavior

VS Code supports UTF-8-with-BOM files, and its MCP configuration reader uses a parser that accepts U+FEFF whitespace. REA rejects the same otherwise-valid MCP configuration as Invalid JSON/JSONC at offset 0: InvalidSymbol.

That blocks setup preflight and configuration, and makes registration/alignment reporting treat an existing valid configuration as malformed.

Change and scope

For the JSON/JSONC parse copy only, replace exactly one leading U+FEFF with an ordinary space. The one-character replacement preserves diagnostic offsets into the original text.

Continue passing unchanged originalText to the existing serializer. Preserve the BOM, comments, CRLF, unrelated settings and embedded U+FEFF content. TOML handling is unchanged. Duplicate or nonleading BOM characters outside strings remain invalid.

Contract and boundary impact

Semantic owner: shared client-configuration document parsing.
CLI/setup: supported BOM-prefixed client files can pass preflight, configuration and registration readback.
MCP analysis tools: no tool/schema/catalog changes.
File preservation: original bytes remain the serializer input; existing backups and idempotence remain intact.
Process/authorization: no approval, installation or execution-authority changes.
Generated metadata/package: no source metadata or dependency changes.

Evidence and regression coverage

On unchanged main af48fe8, the exact 62-case focused run has 30 failures and 32 passing controls. The fixed run passes all 62.

The 51 pure cases cover all five shared JSON-backed formats, BOM/no-BOM input, malformed-input offsets, repeated/nonleading markers, embedded markers, CRLF/comments/siblings and add/update/remove readback. TOML's existing behavior remains covered.

Eleven owned-filesystem cases exercise preflight, configuration, alignment, registration reporting, unchanged second runs and byte-exact original backups across the five client formats. Malformed BOM-prefixed input remains untouched, without creating a backup.

No real client configuration or user home was accessed.

Validation performed

Focused parser, serializer and owned configuration/readiness tests: 62 passed.
Build and npm run check: passed; all five check tasks succeeded (109 warnings, zero errors; none in changed files).
npm run verify:package: passed unchanged, including planned/idempotent setup, four clients, backup/readback and failure recovery, 118 MCP tools and six prompts.
npm pack --dry-run: passed (724 files).
npm run jscpd (2.36%), npm run scan:todos, and git diff --check: passed.
Independent review completed before publication with no blocking findings.

Node.js 24.19.0, npm 11.9.0, exact locked dependencies. Optional Turbo telemetry was disabled for local verification. Hosted CI is tracked separately; no full local-suite pass is claimed.

Compatibility, safety, and release

Only the leading marker in a temporary JSON parse copy changes. Emitted files retain their encoding marker and unrelated content. No TOML, canonical error, dependency or package-format change.

Tests use pure helpers, disposable configuration roots and the standard package verifier. No real client application or separate real Hopper/Ghidra/browser workflow was run. Generated tarballs are excluded from this PR.

Prepared with AI assistance.

_{🔄 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/716 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/6/2026 **Status:** ✅ Merged **Merged:** 10/6/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/client-configuration-leading-bom` --- ### 📝 Commits (1) - [`fe843ec`](https://github.com/morluto/rea/commit/fe843ec0b101544e22d7599d93b36377d3e3bc34) fix(setup): accept leading BOM in JSON client configs ### 📊 Changes **3 files changed** (+238 additions, -1 deletions) <details> <summary>View changed files</summary> ➕ `src/application/ClientConfigurationDocument.test.ts` (+105 -0) 📝 `src/application/ClientConfigurationDocument.ts` (+3 -1) ➕ `tests/boundary/filesystem/clientConfigurationBom.test.ts` (+130 -0) </details> ### 📄 Description ## Summary Accept a single leading UTF-8 BOM when reading JSON/JSONC client configurations, preserving the original document for edits. No separate issue: this is a reproduced supported-client configuration mismatch. ## Problem and expected behavior VS Code supports [UTF-8-with-BOM files](https://code.visualstudio.com/updates/vMarch#_utf8-bom-support), and its [MCP configuration reader](https://github.com/microsoft/vscode/blob/7b0ccc7c006b52b7a30aa2c9190ac3e99c81807a/src/vs/platform/mcp/common/mcpResourceScannerService.ts#L237-L252) uses a parser that [accepts U+FEFF whitespace](https://github.com/microsoft/vscode/blob/7b0ccc7c006b52b7a30aa2c9190ac3e99c81807a/src/vs/base/common/json.ts#L561-L565). REA rejects the same otherwise-valid MCP configuration as `Invalid JSON/JSONC at offset 0: InvalidSymbol`. That blocks setup preflight and configuration, and makes registration/alignment reporting treat an existing valid configuration as malformed. ## Change and scope For the JSON/JSONC parse copy only, replace exactly one leading U+FEFF with an ordinary space. The one-character replacement preserves diagnostic offsets into the original text. Continue passing unchanged originalText to the existing serializer. Preserve the BOM, comments, CRLF, unrelated settings and embedded U+FEFF content. TOML handling is unchanged. Duplicate or nonleading BOM characters outside strings remain invalid. ## Contract and boundary impact - Semantic owner: shared client-configuration document parsing. - CLI/setup: supported BOM-prefixed client files can pass preflight, configuration and registration readback. - MCP analysis tools: no tool/schema/catalog changes. - File preservation: original bytes remain the serializer input; existing backups and idempotence remain intact. - Process/authorization: no approval, installation or execution-authority changes. - Generated metadata/package: no source metadata or dependency changes. ## Evidence and regression coverage On unchanged main `af48fe8`, the exact 62-case focused run has 30 failures and 32 passing controls. The fixed run passes all 62. The 51 pure cases cover all five shared JSON-backed formats, BOM/no-BOM input, malformed-input offsets, repeated/nonleading markers, embedded markers, CRLF/comments/siblings and add/update/remove readback. TOML's existing behavior remains covered. Eleven owned-filesystem cases exercise preflight, configuration, alignment, registration reporting, unchanged second runs and byte-exact original backups across the five client formats. Malformed BOM-prefixed input remains untouched, without creating a backup. No real client configuration or user home was accessed. ## Validation performed - Focused parser, serializer and owned configuration/readiness tests: 62 passed. - Build and `npm run check`: passed; all five check tasks succeeded (109 warnings, zero errors; none in changed files). - `npm run verify:package`: passed unchanged, including planned/idempotent setup, four clients, backup/readback and failure recovery, 118 MCP tools and six prompts. - `npm pack --dry-run`: passed (724 files). - `npm run jscpd` (2.36%), `npm run scan:todos`, and `git diff --check`: passed. - Independent review completed before publication with no blocking findings. Node.js 24.19.0, npm 11.9.0, exact locked dependencies. Optional Turbo telemetry was disabled for local verification. Hosted CI is tracked separately; no full local-suite pass is claimed. ## Compatibility, safety, and release Only the leading marker in a temporary JSON parse copy changes. Emitted files retain their encoding marker and unrelated content. No TOML, canonical error, dependency or package-format change. Tests use pure helpers, disposable configuration roots and the standard package verifier. No real client application or separate real Hopper/Ghidra/browser workflow was run. Generated tarballs are excluded from this PR. Prepared with AI assistance. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-16 21:15:32 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-10-31 01:03:09 +00:00

[PR #2197] [MERGED] fix(cli): label each uninstall skill root by its client #5076

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

No description provided.

Rows
Columns