morluto/rea

Fork 0

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

[PR #4780] test(install): pin the C locale for the truncated installer check #6408

New issue

Open

opened 2026-10-31 01:14:14 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:14:14 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4780
Author: @onionmixer
Created: 10/17/2026
Status: 🔄 Open

Base: main ← Head: test/install-script-locale

📝 Commits (1)

94b2df4 test(install): pin the C locale for the truncated installer check

📊 Changes

1 file changed (+3 additions, -1 deletions)

View changed files

📝 tests/boundary/filesystem/installScript.test.ts (+3 -1)

📄 Description

Summary

This is a small test-only change. I noticed that one installer test fails on my machine only because of my system language, and I wanted to share a possible fix. I may well be missing a reason the test was written this way, so please feel free to adjust or close it.

Problem and expected behavior

tests/boundary/filesystem/installScript.test.ts › "runs nothing from a download truncated after the npm install line" asserts that bash's stderr contains syntax error. runInstaller passes the test runner's environment to bash, so on a host with a non-English locale bash prints a translated parser message. On my host (LANG=ko_KR.UTF-8, GNU bash 5.1.16) it prints:

install-truncated.sh: 줄 3: 문법 오류: 예기치 않은 파일의 끝

The installer still behaves correctly (non-zero exit, empty stdout, npm never runs); only the English-text assertion fails.

Change and scope

The single affected call now passes { LC_ALL: "C" } as its environment override, with a short comment. Nothing else changes. The piped-installer test in the same file already runs bash with a minimal environment, so it was not affected.

Contribution scope: a test-only fix within existing boundaries; I believe no maintainer scope decision is needed, but I am happy to open an issue first if that is preferred.

Contract and boundary impact

Semantic owner and earliest changed stage: test harness only.
Contribution scope fit and linked maintainer decision: not applicable (test-only fix).
CLI and MCP/tool-catalog contract: none.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable.
Generated metadata, package, or installation impact: none; install.sh is unchanged.

Expected effects: test-only change with no target interaction (I left the checklist below for the owner).

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: the existing truncated-installer test.
Base reproduction: on 149a0fc (main at the time) with LANG=ko_KR.UTF-8, npx vitest run tests/boundary/filesystem/installScript.test.ts reports 1 failed, 34 passed; the failure is the toContain("syntax error") assertion.
Remaining proof gaps: I only tried Korean and C locales on Linux; I have not checked macOS.

Validation performed

Node 24.21.0, Ubuntu 22.04, GNU bash 5.1.16:

LANG=ko_KR.UTF-8 npx vitest run tests/boundary/filesystem/installScript.test.ts (before the change) — 1 failed, 34 passed
LANG=ko_KR.UTF-8 npx vitest run tests/boundary/filesystem/installScript.test.ts (after) — 35 passed
LANG=C LC_ALL=C npx vitest run tests/boundary/filesystem/installScript.test.ts (after) — 35 passed
npx oxfmt --check tests/boundary/filesystem/installScript.test.ts — all files use the correct format
npx oxlint tests/boundary/filesystem/installScript.test.ts — finished without findings

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not applicable.
Package or release metadata impact: none.
Security, privacy, process, or containment review: not applicable.

This change was prepared with help from an AI coding assistant (Claude Code). I am opening it as a draft so that I can review it myself before marking it ready.

Author readiness

Human owner:

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.

🤖 Generated with Claude Code

_{🔄 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/4780 **Author:** [@onionmixer](https://github.com/onionmixer) **Created:** 10/17/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `test/install-script-locale` --- ### 📝 Commits (1) - [`94b2df4`](https://github.com/morluto/rea/commit/94b2df48d63cf110df2fe06e0bb734ecf7738a71) test(install): pin the C locale for the truncated installer check ### 📊 Changes **1 file changed** (+3 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `tests/boundary/filesystem/installScript.test.ts` (+3 -1) </details> ### 📄 Description ## Summary This is a small test-only change. I noticed that one installer test fails on my machine only because of my system language, and I wanted to share a possible fix. I may well be missing a reason the test was written this way, so please feel free to adjust or close it. ## Problem and expected behavior `tests/boundary/filesystem/installScript.test.ts` › "runs nothing from a download truncated after the npm install line" asserts that bash's stderr contains `syntax error`. `runInstaller` passes the test runner's environment to bash, so on a host with a non-English locale bash prints a translated parser message. On my host (`LANG=ko_KR.UTF-8`, GNU bash 5.1.16) it prints: ``` install-truncated.sh: 줄 3: 문법 오류: 예기치 않은 파일의 끝 ``` The installer still behaves correctly (non-zero exit, empty stdout, npm never runs); only the English-text assertion fails. ## Change and scope The single affected call now passes `{ LC_ALL: "C" }` as its environment override, with a short comment. Nothing else changes. The piped-installer test in the same file already runs bash with a minimal environment, so it was not affected. Contribution scope: a test-only fix within existing boundaries; I believe no maintainer scope decision is needed, but I am happy to open an issue first if that is preferred. ## Contract and boundary impact - Semantic owner and earliest changed stage: test harness only. - Contribution scope fit and linked maintainer decision: not applicable (test-only fix). - CLI and MCP/tool-catalog contract: none. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. - Generated metadata, package, or installation impact: none; `install.sh` is unchanged. Expected effects: test-only change with no target interaction (I left the checklist below for the owner). - [ ] 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: the existing truncated-installer test. - Base reproduction: on `149a0fc` (main at the time) with `LANG=ko_KR.UTF-8`, `npx vitest run tests/boundary/filesystem/installScript.test.ts` reports 1 failed, 34 passed; the failure is the `toContain("syntax error")` assertion. - Remaining proof gaps: I only tried Korean and C locales on Linux; I have not checked macOS. ## Validation performed Node 24.21.0, Ubuntu 22.04, GNU bash 5.1.16: - `LANG=ko_KR.UTF-8 npx vitest run tests/boundary/filesystem/installScript.test.ts` (before the change) — 1 failed, 34 passed - `LANG=ko_KR.UTF-8 npx vitest run tests/boundary/filesystem/installScript.test.ts` (after) — 35 passed - `LANG=C LC_ALL=C npx vitest run tests/boundary/filesystem/installScript.test.ts` (after) — 35 passed - `npx oxfmt --check tests/boundary/filesystem/installScript.test.ts` — all files use the correct format - `npx oxlint tests/boundary/filesystem/installScript.test.ts` — finished without findings ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. - Package or release metadata impact: none. - Security, privacy, process, or containment review: not applicable. This change was prepared with help from an AI coding assistant (Claude Code). I am opening it as a draft so that I can review it myself before marking it ready. ## Author readiness Human owner:  - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-31 01:14:14 +00:00

bsakaguchi referenced this issue

2026-11-01 21:10:57 +00:00

[GH-ISSUE #3796] [Tracker] Open feature requests by area (October 2026) #1471

bsakaguchi referenced this issue

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

[GH-ISSUE #5653] [Tracker] New issue intake by area (October 2026) #3423