[PR #5544] fix(cli): reject invalid token-window bounds #6955

New issue

Open

opened 2026-11-01 21:44:18 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5544
Author: @chqcjq
Created: 10/20/2026
Status: 🔄 Open

Base: main ← Head: fix/cli-token-window-bounds

📝 Commits (3)

f7c7200 fix(cli): reject invalid token-window bounds
ac8ea62 Merge remote-tracking branch 'origin/main' into fix/cli-token-window-bounds
42a0dcb fix(cli): validate token bounds without decimal rounding

📊 Changes

3 files changed (+155 additions, -4 deletions)

View changed files

📝 docs/cli.md (+6 -0)
📝 src/cliOutput.ts (+50 -4)
➕ tests/boundary/cli/cliTokenWindowBounds.test.ts (+99 -0)

📄 Description

Summary

Fixes #5532. CLI token limits and offsets must represent exact non-negative safe integers. Invalid windows return a complete INVALID_TOKEN_WINDOW diagnostic and exit 1 before command execution.

Previously negative/fractional values could produce inconsistent ranges or negative continuation offsets. The follow-up also rejects fractional spellings that JavaScript rounds to integers, including 1.0000000000000001, 9007199254740991.1, and underflow to zero.

Approach and scope

Validate each raw token spelling at the existing parser boundary, and expose the effective validity result to the output-argument validator. Decimal integrality is checked using coefficient digits and exponent placement, without exponent-sized padding. Remove the superseded number-only check. Preserve zero, exact decimal/exponent forms, integer radix forms, maximum-safe values and duplicate last-value precedence. No Incur dependency, MCP schema, provider capability, target authority or package metadata change. Maintainer scope decision: not applicable to this ordinary CLI input fix.

Review and CI follow-up

Commit 42a0dcb4c addresses the valid Codex P2 rounding finding. The added compiled CLI regression reproduces acceptance of the fractional spelling on the previous head and passes with the correction.
Merged main 74ed67da7 via ac8ea6256. The four old shard failures were stale recovery-text expectations in filteredFailureOutput.test.ts; they also fail with the original base implementation. Current main includes the upstream assertion/canonical-error corrections, and all five tests in that file now pass. No assertion was weakened locally to hide the failures.
Related #5119 covers exhausted positive offsets; #5578 overlaps the original negative/fractional-input fix. This updates the existing PR. Unicode slicing is outside this change.

Current executed validation

Windows, pinned Node 24.18.0/npm 11.16.0:

node scripts/run-development-tests.mjs focused tests/boundary/cli/cliTokenWindowBounds.test.ts tests/boundary/cli/cliOutput.test.ts tests/boundary/cli/filteredFailureOutput.test.ts — 45 passed.
Source-level probe of 192 generated decimal/exponent spellings against an independent exact BigInt oracle — passed. This is separate from the public CLI tests.
npm run check — passed.
npm run docs:check — passed, including 9 README consistency tests.
git diff --check and normal commit/pre-push checks — passed.

The full native-Windows npm run check:ci does not pass: its static/deep tasks, architecture checks and discovery of 2,106 test files pass, but the Python stage requires the POSIX-only resource module. The unchanged Python tests were instead executed in existing Ubuntu 24.04/WSL with Python 3.12.3:

python3 -m unittest bridge/native/test_rea_lldb_tracer.py bridge/pwndbg/test_core_context.py bridge/mitmproxy/test_capture.py -v — 41 passed.
python3 -m unittest discover -s experiments/analysis-efficiency -p 'test_*.py' -v — 91 passed, with a task-local official Node 24.18.0 Linux runtime verified against its published SHA-256. No global toolchain or repository test script was changed.

New-head remote CI/review is pending. The full JavaScript suite, macOS and performance benchmarks were not run. Python's WSL results do not establish Linux CLI or optional-provider coverage.

Author readiness

Human owner: @chqcjq

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.

The human owner's initial readiness declarations are recorded above.

_{🔄 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/5544 **Author:** [@chqcjq](https://github.com/chqcjq) **Created:** 10/20/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/cli-token-window-bounds` --- ### 📝 Commits (3) - [`f7c7200`](https://github.com/morluto/rea/commit/f7c72000d036bb2fab060fddd2bc2089e6395e87) fix(cli): reject invalid token-window bounds - [`ac8ea62`](https://github.com/morluto/rea/commit/ac8ea62560a8badabd9c30d74c800dc4abd7d7fc) Merge remote-tracking branch 'origin/main' into fix/cli-token-window-bounds - [`42a0dcb`](https://github.com/morluto/rea/commit/42a0dcb4c7fe43c8d09ed2c5a4e8e44537322385) fix(cli): validate token bounds without decimal rounding ### 📊 Changes **3 files changed** (+155 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `docs/cli.md` (+6 -0) 📝 `src/cliOutput.ts` (+50 -4) ➕ `tests/boundary/cli/cliTokenWindowBounds.test.ts` (+99 -0) </details> ### 📄 Description ## Summary Fixes #5532. CLI token limits and offsets must represent exact non-negative safe integers. Invalid windows return a complete `INVALID_TOKEN_WINDOW` diagnostic and exit 1 before command execution. Previously negative/fractional values could produce inconsistent ranges or negative continuation offsets. The follow-up also rejects fractional spellings that JavaScript rounds to integers, including `1.0000000000000001`, `9007199254740991.1`, and underflow to zero. ## Approach and scope Validate each raw token spelling at the existing parser boundary, and expose the effective validity result to the output-argument validator. Decimal integrality is checked using coefficient digits and exponent placement, without exponent-sized padding. Remove the superseded number-only check. Preserve zero, exact decimal/exponent forms, integer radix forms, maximum-safe values and duplicate last-value precedence. No Incur dependency, MCP schema, provider capability, target authority or package metadata change. Maintainer scope decision: not applicable to this ordinary CLI input fix. ## Review and CI follow-up - Commit `42a0dcb4c` addresses the valid [Codex P2 rounding finding](https://github.com/morluto/rea/pull/5544#discussion_r4326361584). The added compiled CLI regression reproduces acceptance of the fractional spelling on the previous head and passes with the correction. - Merged main `74ed67da7` via `ac8ea6256`. The four old shard failures were stale recovery-text expectations in `filteredFailureOutput.test.ts`; they also fail with the original base implementation. Current main includes the upstream assertion/canonical-error corrections, and all five tests in that file now pass. No assertion was weakened locally to hide the failures. - Related #5119 covers exhausted positive offsets; #5578 overlaps the original negative/fractional-input fix. This updates the existing PR. Unicode slicing is outside this change. ## Current executed validation Windows, pinned Node 24.18.0/npm 11.16.0: - `node scripts/run-development-tests.mjs focused tests/boundary/cli/cliTokenWindowBounds.test.ts tests/boundary/cli/cliOutput.test.ts tests/boundary/cli/filteredFailureOutput.test.ts` — **45 passed**. - Source-level probe of 192 generated decimal/exponent spellings against an independent exact BigInt oracle — passed. This is separate from the public CLI tests. - `npm run check` — passed. - `npm run docs:check` — passed, including 9 README consistency tests. - `git diff --check` and normal commit/pre-push checks — passed. The full native-Windows `npm run check:ci` does **not** pass: its static/deep tasks, architecture checks and discovery of 2,106 test files pass, but the Python stage requires the POSIX-only `resource` module. The unchanged Python tests were instead executed in existing Ubuntu 24.04/WSL with Python 3.12.3: - `python3 -m unittest bridge/native/test_rea_lldb_tracer.py bridge/pwndbg/test_core_context.py bridge/mitmproxy/test_capture.py -v` — **41 passed**. - `python3 -m unittest discover -s experiments/analysis-efficiency -p 'test_*.py' -v` — **91 passed**, with a task-local official Node 24.18.0 Linux runtime verified against its published SHA-256. No global toolchain or repository test script was changed. New-head remote CI/review is pending. The full JavaScript suite, macOS and performance benchmarks were not run. Python's WSL results do not establish Linux CLI or optional-provider coverage. ## Author readiness Human owner: @chqcjq - [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. The human owner's initial readiness declarations are recorded above. --- <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-11-01 21:44:18 +00:00

bsakaguchi referenced this issue

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

[PR #6955] [MERGED] fix(apple): keep dyld candidates inside the analyzed root on Windows #7473