[PR #5119] [CLOSED] fix(cli): clamp exhausted failure token-window start to the token total #6770

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5119
Author: @nosmile99
Created: 10/18/2026
Status: ❌ Closed

Base: main ← Head: fix/cli-exhausted-token-window-5110


📝 Commits (3)

  • f4faea8 fix(cli): clamp exhausted failure token-window start to the token total
  • 317a9f7 test(cli): assert exhausted token window renders an empty slice
  • 5ef45f3 Merge origin/main into fix/cli-exhausted-token-window-5110

📊 Changes

2 files changed (+63 additions, -1 deletions)

View changed files

📝 src/cli/cliResultOutput.ts (+5 -1)
📝 tests/boundary/cli/jsonOutput.test.ts (+58 -0)

📄 Description

Fixes #5110.

AI-assisted contribution (no human owner available): Author readiness declarations are left unchecked per the repo's readiness gate. Maintainer may mark ready per #4668 precedent.

Summary

--format toon --full-output --token-offset 100000 on a failed command printed showing tokens 100000–90 of 90: fullFailureEnvelope clamped the window end to the token total but printed the raw requested offset as the start, producing a reversed range.

Problem and expected behavior

Smallest trigger: any failed command with --full-output --token-offset <n> where n exceeds the failure text's token total (issue #5110's repro: --format toon --full-output --token-offset 100000 → showing tokens 100000–90 of 90, exit 1).

Expected: an exhausted offset describes an empty range at the end of the failure text, e.g. showing tokens 90–90 of 90 — the displayed start must never exceed its end or the total.

Change and scope

6-line change in src/cli/cliResultOutput.ts (fullFailureEnvelope): clamp the window start at the producing boundary (actualStart = Math.min(offset, total)) and use it for both the token slice and the displayed range. No behavior change for in-range windows (offset <= total implies actualStart === offset, identical output).

Contribution scope: ordinary display bug within existing CLI boundaries; no new providers, no target execution/injection/mutation — "not applicable" for the maintainer scope decision.

Contract and boundary impact

  • Semantic owner and earliest changed stage: CLI result-output boundary (fullFailureEnvelope), formatting only.
  • Contribution scope fit and linked maintainer decision: ordinary fix, not applicable.
  • CLI and MCP/tool-catalog contract: no contract change; output text corrected for an already-documented --token-offset flag.
  • 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): none.
  • Generated metadata, package, or installation impact: none.

Expected effects:

  • Documentation or usability only; no target interaction.

Evidence and regression coverage

  • Tests added or updated: 1 in tests/boundary/cli/jsonOutput.test.ts ("renders an exhausted beyond-end offset as a non-reversed empty range") — drives the issue's exact repro argv, asserts start == end == total and no nextOffset.
  • Base reproduction or other evidence: the issue's repro script on pinned 32f0e51 printed showing tokens 100000–90 of 90 (exit 1) before the fix; showing tokens 90–90 of 90 after.
  • User-visible CLI/MCP output: the corrected showing tokens … truncation line shown above.
  • Remaining proof gaps: full check:pr gate not run (only check:fast); repro ran on /usr/bin/node v24.20.0 (pinned 24.18.0 not available on this machine).

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

  • New test: FAILS pre-fix (expected 100000 to be 47), PASSES post-fix.
  • npm run test:focused -- tests/boundary/cli/jsonOutput.test.ts — 42/42 green.
  • npm run test:focused -- tests/boundary/cli/cliOutput.test.ts — 38/38 green.
  • npm run verify:test-discovery — 1591 files discovered exactly once.
  • npm run check:fast (turbo typecheck + lint) — 2/2 successful.

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable (CLI formatting boundary).
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: none — output formatting only.

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 (Fixes #5110).
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed. (None apply — behavior now matches documented flag semantics.)
  • 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/5119 **Author:** [@nosmile99](https://github.com/nosmile99) **Created:** 10/18/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/cli-exhausted-token-window-5110` --- ### 📝 Commits (3) - [`f4faea8`](https://github.com/morluto/rea/commit/f4faea8d8c9dfe457d8f41a0574044770a3110d1) fix(cli): clamp exhausted failure token-window start to the token total - [`317a9f7`](https://github.com/morluto/rea/commit/317a9f74601a7fe4bd3c3e9ddd3f4fa203e0e23e) test(cli): assert exhausted token window renders an empty slice - [`5ef45f3`](https://github.com/morluto/rea/commit/5ef45f38a21c825da42f2df0ca2045ab969073d8) Merge origin/main into fix/cli-exhausted-token-window-5110 ### 📊 Changes **2 files changed** (+63 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/cli/cliResultOutput.ts` (+5 -1) 📝 `tests/boundary/cli/jsonOutput.test.ts` (+58 -0) </details> ### 📄 Description Fixes #5110. AI-assisted contribution (no human owner available): Author readiness declarations are left unchecked per the repo's readiness gate. Maintainer may mark ready per #4668 precedent. ## Summary `--format toon --full-output --token-offset 100000` on a failed command printed `showing tokens 100000–90 of 90`: `fullFailureEnvelope` clamped the window *end* to the token total but printed the raw requested *offset* as the start, producing a reversed range. ## Problem and expected behavior Smallest trigger: any failed command with `--full-output --token-offset <n>` where `n` exceeds the failure text's token total (issue #5110's repro: `--format toon --full-output --token-offset 100000` → `showing tokens 100000–90 of 90`, exit 1). Expected: an exhausted offset describes an empty range at the end of the failure text, e.g. `showing tokens 90–90 of 90` — the displayed start must never exceed its end or the total. ## Change and scope 6-line change in `src/cli/cliResultOutput.ts` (`fullFailureEnvelope`): clamp the window start at the producing boundary (`actualStart = Math.min(offset, total)`) and use it for both the token slice and the displayed range. No behavior change for in-range windows (`offset <= total` implies `actualStart === offset`, identical output). Contribution scope: ordinary display bug within existing CLI boundaries; no new providers, no target execution/injection/mutation — "not applicable" for the maintainer scope decision. ## Contract and boundary impact - Semantic owner and earliest changed stage: CLI result-output boundary (`fullFailureEnvelope`), formatting only. - Contribution scope fit and linked maintainer decision: ordinary fix, not applicable. - CLI and MCP/tool-catalog contract: no contract change; output text corrected for an already-documented `--token-offset` flag. - 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): none. - Generated metadata, package, or installation impact: none. Expected effects: - [x] Documentation or usability only; no target interaction. ## Evidence and regression coverage - Tests added or updated: 1 in `tests/boundary/cli/jsonOutput.test.ts` ("renders an exhausted beyond-end offset as a non-reversed empty range") — drives the issue's exact repro argv, asserts start == end == total and no `nextOffset`. - Base reproduction or other evidence: the issue's repro script on pinned `32f0e51` printed `showing tokens 100000–90 of 90` (exit 1) before the fix; `showing tokens 90–90 of 90` after. - User-visible CLI/MCP output: the corrected `showing tokens …` truncation line shown above. - Remaining proof gaps: full `check:pr` gate not run (only `check:fast`); repro ran on `/usr/bin/node` v24.20.0 (pinned 24.18.0 not available on this machine). 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 - New test: FAILS pre-fix (`expected 100000 to be 47`), PASSES post-fix. - `npm run test:focused -- tests/boundary/cli/jsonOutput.test.ts` — 42/42 green. - `npm run test:focused -- tests/boundary/cli/cliOutput.test.ts` — 38/38 green. - `npm run verify:test-discovery` — 1591 files discovered exactly once. - `npm run check:fast` (turbo typecheck + lint) — 2/2 successful. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable (CLI formatting boundary). - Package or release metadata impact: none. - Security, privacy, process, or containment review: none — output formatting only. ## 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`. - [x] Related issue is linked (`Fixes #5110`). - [x] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. (None apply — behavior now matches documented flag semantics.) - [x] User-visible CLI/MCP changes include representative output. - [x] 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:42:56 +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#6770
No description provided.