[PR #4142] [MERGED] fix(process): preserve terminal cell widths during normalization #6081

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:11:43 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4142
Author: @hichipli
Created: 10/15/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @morluto

Base: main ← Head: fix/terminal-wide-character-padding

📝 Commits (1)

b15ced4 fix(process): preserve terminal cell widths during normalization

📊 Changes

3 files changed (+58 additions, -5 deletions)

View changed files

📝 src/process/capture/TerminalRenderer.test.ts (+21 -0)
📝 src/process/capture/TerminalRenderer.ts (+5 -5)
📝 tests/boundary/process/processCaptureTerminal.test.ts (+32 -0)

📄 Description

Summary

Fixes #3868. Space normalization of a four-column terminal row containing 界 now produces 界__, rather than inventing a third blank cell (界___).

Problem and expected behavior

xterm already returns the visible row by terminal-cell width. Padding that row to a JavaScript string length adds cells for wide characters. Consumers then retain the invented cells as captured output.

Change and scope

Use xterm's complete row directly; retain a full-width blank fallback for a missing row. Keep normalization before trailing-space trimming and preserve byte budgets, raw PTY output, serialized state and cleanup ownership. Add Unicode/ASCII renderer controls and a real process-capture regression.

This is a focused correction within the existing capture boundary; an advance maintainer scope decision is not applicable.

Contract and boundary impact

Semantic owner: TerminalRenderer, after xterm parses the terminal row and before normalization.
CLI/MCP schemas, tool catalog, dependencies and generated metadata: unchanged.
Evidence: corrected visible rows; existing raw output and serialized terminal state remain authoritative.
Existing workflow effects: launch the caller-selected process and capture its output; no new execution, mutation or network authority.
Compatibility: pinned xterm Unicode widths are preserved, including its one-cell emoji classification.

Evidence and regression coverage

On base a40ee741, the CJK renderer regression and actual PTY capture regression fail with an extra underscore. With the fix, the four-column capture retains raw 界, cursor column 2 and rendered 界__. The ASCII control retains ab__.

Validation performed

Node 24.18.0 / npm 11.16.0, macOS arm64:

npm run test:focused -- src/process/capture/TerminalRenderer.test.ts src/process/capture/ProcessCaptureObservation.test.ts tests/boundary/process/processCaptureTerminal.test.ts tests/boundary/process/processCaptureNormalization.test.ts tests/boundary/process/processCli.test.ts — 34 passed, 2 Linux-only cases skipped.
npm run check — passed typecheck, format, lint, unused-code, metadata, module boundaries and architecture guards; lint reports existing repository warnings and zero errors.
Actual compiled CLI capture-process calls using /usr/bin/printf, four columns and space-to-underscore normalization — CJK and ASCII assertions passed. Both report the owned process group verified and temporary root removed. Existing macOS sampling limitations remain in the returned Evidence.
git diff --check — passed.
GitHub Actions on b15ced4: all selected lanes passed except Windows native conformance, where the ACL-inspection PowerShell subprocess exceeded its 15-second limit after 453 Windows tests passed (25 skipped). CI required reports that dependency failure. A targeted rerun has been requested; no retry has executed because this contributor lacks Actions write access. The timeout's root cause remains unconfirmed.

Compatibility, safety, and release

No schema, package or migration changes. The complete deterministic suite is left to CI; Linux/Windows runtime behavior was not executed locally. No performance improvement is claimed.

Author readiness

Human owner: @hichipli

On 2026-10-15, @hichipli confirmed review of the final diff, validation evidence and limitations, accepted responsibility, and explicitly declared this PR ready for initial maintainer review.

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 within the existing contribution scope.
The PR has one focused outcome and links its issue.
Regression coverage checks observable behavior, raw output and cleanup.
No owning contract, documentation or generated-metadata update is required.
Final diff checked for secrets, unrelated changes 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/4142 **Author:** [@hichipli](https://github.com/hichipli) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/terminal-wide-character-padding` --- ### 📝 Commits (1) - [`b15ced4`](https://github.com/morluto/rea/commit/b15ced4902e06e792d675d86ff62a25e17aa7a96) fix(process): preserve terminal cell widths during normalization ### 📊 Changes **3 files changed** (+58 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `src/process/capture/TerminalRenderer.test.ts` (+21 -0) 📝 `src/process/capture/TerminalRenderer.ts` (+5 -5) 📝 `tests/boundary/process/processCaptureTerminal.test.ts` (+32 -0) </details> ### 📄 Description ## Summary Fixes #3868. Space normalization of a four-column terminal row containing `界` now produces `界__`, rather than inventing a third blank cell (`界___`). ## Problem and expected behavior xterm already returns the visible row by terminal-cell width. Padding that row to a JavaScript string length adds cells for wide characters. Consumers then retain the invented cells as captured output. ## Change and scope Use xterm's complete row directly; retain a full-width blank fallback for a missing row. Keep normalization before trailing-space trimming and preserve byte budgets, raw PTY output, serialized state and cleanup ownership. Add Unicode/ASCII renderer controls and a real process-capture regression. This is a focused correction within the existing capture boundary; an advance maintainer scope decision is not applicable. ## Contract and boundary impact - Semantic owner: `TerminalRenderer`, after xterm parses the terminal row and before normalization. - CLI/MCP schemas, tool catalog, dependencies and generated metadata: unchanged. - Evidence: corrected visible rows; existing raw output and serialized terminal state remain authoritative. - Existing workflow effects: launch the caller-selected process and capture its output; no new execution, mutation or network authority. - Compatibility: pinned xterm Unicode widths are preserved, including its one-cell emoji classification. ## Evidence and regression coverage On base `a40ee741`, the CJK renderer regression and actual PTY capture regression fail with an extra underscore. With the fix, the four-column capture retains raw `界`, cursor column 2 and rendered `界__`. The ASCII control retains `ab__`. ## Validation performed Node 24.18.0 / npm 11.16.0, macOS arm64: - `npm run test:focused -- src/process/capture/TerminalRenderer.test.ts src/process/capture/ProcessCaptureObservation.test.ts tests/boundary/process/processCaptureTerminal.test.ts tests/boundary/process/processCaptureNormalization.test.ts tests/boundary/process/processCli.test.ts` — 34 passed, 2 Linux-only cases skipped. - `npm run check` — passed typecheck, format, lint, unused-code, metadata, module boundaries and architecture guards; lint reports existing repository warnings and zero errors. - Actual compiled CLI `capture-process` calls using `/usr/bin/printf`, four columns and space-to-underscore normalization — CJK and ASCII assertions passed. Both report the owned process group verified and temporary root removed. Existing macOS sampling limitations remain in the returned Evidence. - `git diff --check` — passed. - GitHub Actions on `b15ced4`: all selected lanes passed except Windows native conformance, where the ACL-inspection PowerShell subprocess exceeded its 15-second limit after 453 Windows tests passed (25 skipped). [`CI required` reports that dependency failure](https://github.com/morluto/rea/actions/runs/38749446613/job/116750746322). [A targeted rerun has been requested](https://github.com/morluto/rea/pull/4142#issuecomment-6210828366); no retry has executed because this contributor lacks Actions write access. The timeout's root cause remains unconfirmed. ## Compatibility, safety, and release No schema, package or migration changes. The complete deterministic suite is left to CI; Linux/Windows runtime behavior was not executed locally. No performance improvement is claimed. ## Author readiness Human owner: @hichipli On 2026-10-15, @hichipli confirmed review of the final diff, validation evidence and limitations, accepted responsibility, and explicitly declared this PR ready for initial maintainer review. - [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 within the existing contribution scope. - [x] The PR has one focused outcome and links its issue. - [x] Regression coverage checks observable behavior, raw output and cleanup. - [x] No owning contract, documentation or generated-metadata update is required. - [x] Final diff checked for secrets, unrelated changes and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:11:43 +00:00