You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(sequentialthinking): bound the stderr thought box - #5435
The stderr thought box from #5205 padded every row to the thought's longest line, so a thought with one long line and many short ones logged their product. Copilot found it reviewing the v2.3.0 milestone merge PR #5430: a 60 KB thought wrote 400 million characters and pushed RSS to 878 MB. A somewhat larger one would exhaust memory, from a single valid call with logging on by default.
The box is now bounded:
Width: at most MAX_BOX_TEXT_WIDTH (100). Longer lines wrap onto further rows, and so does a header longer than that (a long branchId, which would otherwise set every row's width). A cut never splits a surrogate pair.
Height: at most MAX_BOX_ROWS (1000) rows of thought text. Past that the box ends with one row, … N more lines not shown. Padding every short line to the width would otherwise still log about 100 times the thought's size.
#5205's behaviour is kept: one row per line, any line ending ends a row, no \r inside a row, every row the same width (#5108), and colour sized from the visible header (#5000). Only stderr changes. The stored thought and the tool result are unchanged.
Server Details
Server: sequentialthinking
Changes to: the thought log on stderr (lib.ts, formatThought); README; patch changeset
Motivation and Context
#5433. It is a regression from #5205, in this release (v2.3.0, #5425), so it is fixed before the release.
How Has This Been Tested?
Measured on the module (SequentialThinkingServer.processThought, stderr counted). The thought is one line of n characters, then n - 1 one-character lines:
n
thought
candidate cc602786
this branch
5,000
15 KB
25,055,028 chars, 112 MB RSS
—
20,000
60 KB
400,220,028 chars, 878 MB RSS
107,535 chars, 63 MB RSS
200,000
600 KB
—
107,535 chars, 70 MB RSS
2,000,000
6 MB
—
107,535 chars, 123 MB RSS
Inspector CLI 2.13.0, tools/call sequentialthinking with the n = 5,000 thought, both eras:
candidate legacy exit=0 {"thoughtNumber":1,"thoughtHistoryLength":1} stderr 25,105,119 chars, 5,006 lines
candidate modern exit=0 {"thoughtNumber":1,"thoughtHistoryLength":1} stderr 25,105,119 chars, 5,006 lines
branch legacy exit=0 {"thoughtNumber":1,"thoughtHistoryLength":1} stderr 112,232 chars, 1,007 lines, "… 4049 more lines not shown"
branch modern exit=0 {"thoughtNumber":1,"thoughtHistoryLength":1} stderr 112,232 chars, 1,007 lines, "… 4049 more lines not shown"
LLM client: Claude Code 2.1.318, headless, --strict-mcp-config naming this branch's build (no era switch). It was asked for one sequentialthinking call with a 300-character line and three short lines, and returned {"thoughtNumber":1,"totalThoughts":1,"nextThoughtNeeded":false,"branches":[],"thoughtHistoryLength":1}.
Tests: thought-logging.test.ts adds:
wrapping at the capped width;
Copilot's exact case staying bounded;
the row cap's summary row, including a line the cap cuts partway;
a thought that exactly fills the cap, with no summary row;
every row the same width;
a long branchId header wrapping and staying linear;
an emoji at the cut not split.
The #5205 "very many lines" test now expects the capped box. The suite is 138 passed; lib.ts coverage is 100 / 98.11 / 100 / 100. npm run local:gate: EXIT=0.
Breaking Changes
None. Only the stderr diagnostic's layout changes for thoughts with lines over 100 characters or more than 1000 rows.
Types of changes
Bug fix (non-breaking change which fixes an issue)
#5205 padded every row of the box to the thought's longest line, so one
long line and many short ones logged their product: a 60 KB thought wrote
400 million characters. The box is now at most MAX_BOX_TEXT_WIDTH (100)
wide, wrapping longer lines and a longer header without splitting a
surrogate pair, and at most MAX_BOX_ROWS (1000) rows, ending with a row
that counts the lines left out. The box tops out near 107 KB; the stored
thought and the tool result are unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Truncated thoughts containing only short lines can produce a summary row wider than the box, so box sizing must account for the summary and tests must verify consistent row widths.
Fixes the sequentialthinking stderr logging regression from #5205 by limiting thought-box expansion, addressing #5433 without changing stored thoughts or tool results.
Changes:
Caps text width at 100 UTF-16 code units and thought text at 1,000 rows, with wrapping and an omission summary.
Adds regression tests for large inputs, header wrapping, row alignment, and surrogate-pair preservation.
Documents the logging limits and adds a patch changeset.
File
Description
src/sequentialthinking/README.md
Documents thought-log wrapping and truncation.
src/sequentialthinking/lib.ts
Implements width limits, wrapping, and thought-row truncation.
A truncated thought of short lines had a summary row wider than the box
(33 characters against a 20-character border). The row texts are now
collected first and the box is sized to include the summary; widening
cannot change the wrapping, since below the cap no line wrapped and at
the cap the summary fits. The very-many-lines test checks every row's
width.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The documentation incorrectly promises returned thought text and absolute output limits, although results contain only metadata and unbounded branch IDs can exceed the stated limits.
No open issues 🎉
2 previously missed issues
In code that hasn't changed since last review
Qualify output limits to account for unbounded branch IDs
The 1000-row cap applies only to thought text; every wrapped header row is still rendered. Since branchId has no length limit, a sufficiently long ID exceeds the claimed total height and 107 KB maximum. Qualify the limit as applying to thought-text rows and remove the absolute output-size guarantee; the same height claim in src/sequentialthinking/README.md:137 also needs qualification.
Clarify thought text is retained in history, not returned by the tool
src/sequentialthinking/README.md:137
The thought text is not returned by this tool: processThought returns only counters, flags, and branch IDs, and index.ts exposes the same metadata as structured content. Saying it is “returned whole” incorrectly promises that clients can retrieve text omitted from stderr. Describe the complete thought as retained in history and the tool result as unchanged instead.
The row cap counts thought-text rows only (a long branchId's header rows
are not capped), so there is no absolute size guarantee; and the tool
result never carries the thought text, which the server keeps whole in
its history. README, changeset and the MAX_BOX_ROWS comment now say so.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
Copilot round 2: no inline findings; two "previously missed" findings in the review body (no thread to reply in), both doc accuracy, both fixed in c583da0:
Output limit overstated. The 1000-row cap counts thought-text rows only; header rows from a long branchId are wrapped but not capped, so "about 107 KB at most" was not absolute. The README, changeset and MAX_BOX_ROWS comment now say "at most 1000 rows of thought text" and drop the absolute size claim.
"Returned whole" was wrong. The tool result carries no thought text (counters, flags, branch IDs). The docs now say the server keeps the whole thought in its history and the tool result is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #5433
Description
The stderr thought box from #5205 padded every row to the thought's longest line, so a thought with one long line and many short ones logged their product. Copilot found it reviewing the v2.3.0 milestone merge PR #5430: a 60 KB thought wrote 400 million characters and pushed RSS to 878 MB. A somewhat larger one would exhaust memory, from a single valid call with logging on by default.
The box is now bounded:
MAX_BOX_TEXT_WIDTH(100). Longer lines wrap onto further rows, and so does a header longer than that (a longbranchId, which would otherwise set every row's width). A cut never splits a surrogate pair.MAX_BOX_ROWS(1000) rows of thought text. Past that the box ends with one row,… N more lines not shown. Padding every short line to the width would otherwise still log about 100 times the thought's size.#5205's behaviour is kept: one row per line, any line ending ends a row, no
\rinside a row, every row the same width (#5108), and colour sized from the visible header (#5000). Only stderr changes. The stored thought and the tool result are unchanged.Server Details
lib.ts,formatThought); README; patch changesetMotivation and Context
#5433. It is a regression from #5205, in this release (v2.3.0, #5425), so it is fixed before the release.
How Has This Been Tested?
Measured on the module (
SequentialThinkingServer.processThought, stderr counted). The thought is one line ofncharacters, thenn - 1one-character lines:cc602786Inspector CLI 2.13.0,
tools/call sequentialthinkingwith the n = 5,000 thought, both eras:LLM client: Claude Code 2.1.318, headless,
--strict-mcp-confignaming this branch's build (no era switch). It was asked for onesequentialthinkingcall with a 300-character line and three short lines, and returned{"thoughtNumber":1,"totalThoughts":1,"nextThoughtNeeded":false,"branches":[],"thoughtHistoryLength":1}.Tests:
thought-logging.test.tsadds:branchIdheader wrapping and staying linear;The #5205 "very many lines" test now expects the capped box. The suite is 138 passed;
lib.tscoverage is 100 / 98.11 / 100 / 100.npm run local:gate: EXIT=0.Breaking Changes
None. Only the stderr diagnostic's layout changes for thoughts with lines over 100 characters or more than 1000 rows.
Types of changes
Checklist
npm run changeset) if this changes what a TypeScript server publishes🤖 Generated with Claude Code