diff --git a/.changeset/sequentialthinking-header-row-cap.md b/.changeset/sequentialthinking-header-row-cap.md new file mode 100644 index 0000000000..a753cedf67 --- /dev/null +++ b/.changeset/sequentialthinking-header-row-cap.md @@ -0,0 +1,5 @@ +--- +"@modelcontextprotocol/server-sequential-thinking": patch +--- + +Count the thought box's header rows toward its 1000-row cap, so a long `branchId` can no longer draw past it on stderr: line breaks in a `branchId` are shown as `\r` and `\n`, and a header too long for the cap is cut with a row saying how many characters it left out (#5526). diff --git a/src/sequentialthinking/README.md b/src/sequentialthinking/README.md index 81ae0ae26a..ea3563224f 100644 --- a/src/sequentialthinking/README.md +++ b/src/sequentialthinking/README.md @@ -134,7 +134,7 @@ On Windows, use `cmd /c` to launch `npx`: To disable logging of thought information set env var: `DISABLE_THOUGHT_LOGGING` to `true`. -Each thought is logged to stderr as a box, one row per line of the thought. The box is at most 100 characters wide, with longer lines (and a long branch ID in the header) wrapped, and draws at most 1000 rows of thought text; past that it ends with a row saying how many lines it left out. Only the log is shortened: the server keeps the whole thought in its history, and the tool result is unchanged. +Each thought is logged to stderr as a box, one row per line of the thought. The box is at most 100 characters wide, with longer lines (and a long branch ID in the header) wrapped, and draws at most 1000 rows of text, header and thought together; past that it ends with a row saying how many lines it left out. A line break in a branch ID is shown in the header as `\r` or `\n`, and a header too long for those rows (a very long branch ID) is cut, with a row saying how many characters it left out. Only the log is shortened: the server keeps the whole thought in its history, and the tool result is unchanged. To bound memory use, the server retains at most `MAX_THOUGHT_HISTORY` thoughts per connection (a positive integer; default `1000`). Past the cap the oldest thought is dropped, from its branch too, so `thoughtHistoryLength` in the result stops at the cap and a branch whose thoughts have all been dropped is no longer listed in `branches`. diff --git a/src/sequentialthinking/__tests__/thought-logging.test.ts b/src/sequentialthinking/__tests__/thought-logging.test.ts index 6b9345e295..dda69ba097 100644 --- a/src/sequentialthinking/__tests__/thought-logging.test.ts +++ b/src/sequentialthinking/__tests__/thought-logging.test.ts @@ -2,7 +2,7 @@ // "true" (any case), each accepted thought is drawn as a box on stderr, with // a header that differs for plain thoughts, revisions and branches, and one // row per line of the thought, wrapped at MAX_BOX_TEXT_WIDTH and cut off -// after MAX_BOX_ROWS. Thoughts +// once header and thought fill MAX_BOX_ROWS rows. Thoughts // are driven through the in-process client and stderr is captured by spying // on console.error, which also keeps the boxes off the test output. chalk's // colour level depends on the terminal the tests run in, so it is pinned to 0 @@ -232,15 +232,18 @@ ${rows.join("\n")} }); // #5433: past MAX_BOX_ROWS the box stops drawing and says how many lines - // it left out; the call itself is unaffected. + // it left out; the call itself is unaffected. The header's one row counts + // toward the cap (#5526), so the thought gets the rest. it("draws a thought with very many lines", async () => { const lines = Array.from({ length: 200_000 }, (_, i) => `${i % 10}`); const result = await conn!.think(thought({ thought: lines.join("\n") })); expect(result.isError).toBeFalsy(); const out = logged()[0].split("\n"); - expect(out).toHaveLength(MAX_BOX_ROWS + 1 + 5); + // A leading blank, the borders and separator, MAX_BOX_ROWS rows of + // header and thought, and the summary row. + expect(out).toHaveLength(MAX_BOX_ROWS + 5); expect(out.at(-2)).toContain( - `… ${200_000 - MAX_BOX_ROWS} more lines not shown`, + `… ${200_000 - (MAX_BOX_ROWS - 1)} more lines not shown`, ); // The summary is wider than every one-character line and the header, // so it sets the box's width; it must not stick out of it. @@ -284,28 +287,33 @@ ${row("short")} const result = await conn!.think(thought({ thought: text })); expect(result.isError).toBeFalsy(); const out = logged()[0]; - // 200 wrapped rows for the long line, short lines up to the row cap, - // then the summary row; each row is W + 6 characters and a newline. - const shortShown = MAX_BOX_ROWS - n / W; - expect(out.split("\n")).toHaveLength(MAX_BOX_ROWS + 1 + 5); + // The header row, 200 wrapped rows for the long line, short lines up to + // the row cap, then the summary row; each row is W + 6 characters and a + // newline. + const shortShown = MAX_BOX_ROWS - 1 - n / W; + expect(out.split("\n")).toHaveLength(MAX_BOX_ROWS + 5); expect(out).toContain(`… ${n - 1 - shortShown} more lines not shown`); expect(out.length).toBeLessThan((MAX_BOX_ROWS + 10) * (W + 7)); }); it("counts a line the row cap cuts off as not shown", async () => { - const text = `${"s\n".repeat(MAX_BOX_ROWS - 1)}${"L".repeat(2 * W)}`; + // The header row and these short lines leave one row for the long one. + const text = `${"s\n".repeat(MAX_BOX_ROWS - 2)}${"L".repeat(2 * W)}`; await conn!.think(thought({ thought: text })); const rows = logged()[0].split("\n"); - expect(rows).toHaveLength(MAX_BOX_ROWS + 1 + 5); + expect(rows).toHaveLength(MAX_BOX_ROWS + 5); expect(rows.at(-3)).toBe(row("L".repeat(W))); expect(rows.at(-2)).toBe(row("… 1 more line not shown")); }); it("draws a thought that exactly fills the cap with no summary row", async () => { - const text = Array.from({ length: MAX_BOX_ROWS }, () => "s").join("\n"); + // With the header's one row, these lines fill the cap exactly. + const text = Array.from({ length: MAX_BOX_ROWS - 1 }, () => "s").join( + "\n", + ); await conn!.think(thought({ thought: text })); const rows = logged()[0].split("\n"); - expect(rows).toHaveLength(MAX_BOX_ROWS + 5); + expect(rows).toHaveLength(MAX_BOX_ROWS + 4); expect(logged()[0]).not.toContain("not shown"); }); @@ -338,6 +346,81 @@ ${row("short")} expect(out.length).toBeLessThan((lines + 10) * (W + 7)); }); + // #5526: a long branchId's header rows count toward MAX_BOX_ROWS, so they + // cannot add rows past the cap the thought's rows are held to. + it("counts a wrapped header's rows toward the row cap", async () => { + const branchId = "b".repeat(2 * W); + const header = `🌿 Branch 1/3 (from thought 1, ID: ${branchId})`; + const headerRows = Math.ceil(header.length / W); + expect(headerRows).toBeGreaterThan(1); + await conn!.think( + thought({ + thought: Array.from({ length: MAX_BOX_ROWS }, () => "s").join("\n"), + branchFromThought: 1, + branchId, + }), + ); + const rows = logged()[0].split("\n"); + expect(rows).toHaveLength(MAX_BOX_ROWS + 5); + expect(rows.at(-2)).toBe(row(`… ${headerRows} more lines not shown`)); + }); + + // #5526: a line ending in a branchId would start a line of output the + // row cap never counted; the header shows it escaped instead. + it("escapes line endings in a branchId so the header stays on its rows", async () => { + const branchId = "x\r\ny\rz\n".repeat(500); + const escaped = "x\\r\\ny\\rz\\n".repeat(500); + const header = `🌿 Branch 1/3 (from thought 1, ID: ${escaped})`; + await conn!.think( + thought({ thought: "t", branchFromThought: 1, branchId }), + ); + const out = logged()[0]; + expect(out).not.toContain("\r"); + const rows = out.split("\n"); + const headerRows = Math.ceil(header.length / W); + // A leading blank, the borders and separator, the header rows and the + // thought's one row. + expect(rows).toHaveLength(headerRows + 5); + expect( + rows + .slice(2, 2 + headerRows) + .map((r) => r.slice(2, 2 + W).trimEnd()) + .join(""), + ).toBe(header); + }); + + // #5526: a header too long for every row is cut, so a huge branchId logs + // no more than the cap; its last row says how much was left out. + it("cuts a header longer than the row cap", async () => { + const branchId = "b".repeat(MAX_BOX_ROWS * W); + const header = `🌿 Branch 1/3 (from thought 1, ID: ${branchId})`; + const result = await conn!.think( + thought({ thought: "t", branchFromThought: 1, branchId }), + ); + expect(result.isError).toBeFalsy(); + const out = logged()[0]; + const rows = out.split("\n"); + // A leading blank, the top border, MAX_BOX_ROWS header rows, the + // separator, the thought's summary row and the bottom border. + expect(rows).toHaveLength(MAX_BOX_ROWS + 5); + const shown = (MAX_BOX_ROWS - 1) * W; + expect( + rows + .slice(2, MAX_BOX_ROWS + 1) + .map((r) => r.slice(2, 2 + W)) + .join(""), + ).toBe(header.slice(0, shown)); + expect(rows[MAX_BOX_ROWS + 1]).toBe( + row(`… ${header.length - shown} more characters not shown`), + ); + expect(rows[MAX_BOX_ROWS + 2]).toBe(`├${border}┤`); + expect(rows.at(-2)).toBe(row("… 1 more line not shown")); + expect(new Set(rows.slice(1).map((r) => r.length))).toEqual( + new Set([W + 6]), + ); + expect(out.length).toBeLessThan((MAX_BOX_ROWS + 10) * (W + 7)); + }); + it("never splits an emoji across rows", async () => { // The emoji's two UTF-16 halves would straddle the cut at W. const text = `${"a".repeat(W - 1)}😀tail`; diff --git a/src/sequentialthinking/lib.ts b/src/sequentialthinking/lib.ts index b9299910bc..cc13c50840 100644 --- a/src/sequentialthinking/lib.ts +++ b/src/sequentialthinking/lib.ts @@ -22,12 +22,15 @@ export interface ThoughtData { export const MAX_BOX_TEXT_WIDTH = 100; /** - * The most rows of thought text a box draws (#5433). Each row is padded to - * the box's width, so a thought of many short lines would still log about a - * hundred times its own size; past this many rows the box ends with one row - * saying how many lines it left out. Header rows (a long branchId) are not - * counted. Only stderr is cut: the thought kept in thoughtHistory is whole, - * and the tool result, which carries no thought text, is unchanged. + * The most rows of text a box draws, header and thought together (#5433, + * #5526). Each row is padded to the box's width, so a thought of many short + * lines would still log about a hundred times its own size; past this many + * rows the box ends with one row saying how many lines it left out. The + * header's rows count too, so a long branchId, which wraps onto rows of its + * own, cannot draw past the cap: a header too long for every row is cut, its + * last row saying how many characters it left out. Only stderr is cut: the + * thought kept in thoughtHistory is whole, and the tool result, which carries + * no thought text, is unchanged. */ export const MAX_BOX_ROWS = 1000; @@ -130,9 +133,14 @@ export class SequentialThinkingServer { } else if (branchFromThought) { label = "🌿 Branch"; colour = chalk.green; + // #5526: a line ending in the branchId is shown as its escape, \r or + // \n, so the header stays on rows the box draws and MAX_BOX_ROWS counts. context = branchId !== undefined - ? ` (from thought ${branchFromThought}, ID: ${branchId})` + ? ` (from thought ${branchFromThought}, ID: ${branchId.replace( + /[\r\n]/g, + (eol) => (eol === "\r" ? "\\r" : "\\n"), + )})` : ` (from thought ${branchFromThought})`; } else { label = "💭 Thought"; @@ -158,15 +166,32 @@ export class SequentialThinkingServer { ); // A header longer than the cap (a long branchId) wraps like a thought // line; only its first row, which starts with the label, is coloured. - const headerTexts = wrapRow(visibleHeader, wrapWidth); - // Rows are drawn up to MAX_BOX_ROWS; the lines left over are counted, not - // drawn, so a huge thought costs no more than the cap. + // #5526: its rows count toward MAX_BOX_ROWS. Only one character more than + // the cap's rows can hold is wrapped, so a huge branchId costs no more + // than the cap, and more than MAX_BOX_ROWS rows means the header does not + // fit. It then keeps MAX_BOX_ROWS - 1 rows and a last one saying how many + // characters it left out: always more than one row's worth. Such a header + // is longer than the box is wide, so wrapWidth is MAX_BOX_TEXT_WIDTH and + // that row fits it. + const headerTexts = wrapRow( + visibleHeader.slice(0, MAX_BOX_ROWS * wrapWidth + 1), + wrapWidth, + ); + if (headerTexts.length > MAX_BOX_ROWS) { + headerTexts.length = MAX_BOX_ROWS - 1; + const shown = headerTexts.reduce((n, text) => n + text.length, 0); + headerTexts.push( + `… ${visibleHeader.length - shown} more characters not shown`, + ); + } + // Thought rows fill what the header left of MAX_BOX_ROWS; the lines left + // over are counted, not drawn, so a huge thought costs no more than the cap. const texts: string[] = []; // Lines drawn in full; a line cut off by the cap counts as hidden. let whole = 0; for (const line of lines) { const wrapped = wrapRow(line, wrapWidth); - const room = MAX_BOX_ROWS - texts.length; + const room = MAX_BOX_ROWS - headerTexts.length - texts.length; texts.push(...wrapped.slice(0, room)); if (wrapped.length > room) break; whole++;