From 11716bf751a34638ab59082af8bd357e364198b7 Mon Sep 17 00:00:00 2001 From: Jia Xuan <1060996408+jiaxuan@users.noreply.github.com> Date: Tue, 5 May 2026 01:57:02 +0800 Subject: [PATCH 1/2] fix(sequentialthinking): harden thought-box rendering and add cross-field validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit formatThought: - pad the header row so its right border lines up with the rest of the box (previously hung in mid-air when the thought was longer than the header) - account for chalk's ANSI escape bytes when measuring header width - render multi-line thoughts as one row per line, expand tabs to a fixed width so terminal tab stops can't break the right border - wrap thought lines whose display width exceeds 80 columns at grapheme boundaries, so the box never overflows a typical terminal - new displayWidth helper uses Intl.Segmenter to count terminal columns correctly for CJK ideographs, SMP-plane emoji (๐Ÿ’ญ๐Ÿ”„๐ŸŒฟ), ZWJ-joined emoji like ๐Ÿ‘จโ€๐Ÿ’ป, and VS-16 variation selectors processThought: - stop mutating the caller-supplied input object when auto-adjusting totalThoughts; work on a local copy and report the adjusted value in the response as before new validateThoughtData: - cross-field semantic checks Zod's field-level shape can't express: revisesThought is required (and must precede thoughtNumber) when isRevision is true; same for branchFromThought / branchId lib.ts line coverage: 84.93% โ†’ 100%. Tests in lib.test.ts: 14 โ†’ 41. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/sequentialthinking/__tests__/lib.test.ts | 401 ++++++++++++++++++- src/sequentialthinking/lib.ts | 222 +++++++++- 2 files changed, 601 insertions(+), 22 deletions(-) diff --git a/src/sequentialthinking/__tests__/lib.test.ts b/src/sequentialthinking/__tests__/lib.test.ts index 2114c5ec18..7701ff0327 100644 --- a/src/sequentialthinking/__tests__/lib.test.ts +++ b/src/sequentialthinking/__tests__/lib.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import { SequentialThinkingServer, ThoughtData } from '../lib.js'; +import { SequentialThinkingServer, ThoughtData, displayWidth, validateThoughtData } from '../lib.js'; // Mock chalk to avoid ESM issues vi.mock('chalk', () => { @@ -305,4 +305,403 @@ describe('SequentialThinkingServer', () => { expect(result.isError).toBeUndefined(); }); }); + + describe('formatThought - box alignment', () => { + let consoleErrorSpy: ReturnType; + let serverWithLogging: SequentialThinkingServer; + + // Strip ANSI escape sequences so width comparisons reflect what's actually + // rendered in a terminal, regardless of whether chalk is mocked or not. + const stripAnsi = (s: string) => s.replace(/\[\d+(;\d+)*m/g, ''); + + beforeEach(() => { + delete process.env.DISABLE_THOUGHT_LOGGING; + consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + serverWithLogging = new SequentialThinkingServer(); + }); + + afterEach(() => { + consoleErrorSpy.mockRestore(); + process.env.DISABLE_THOUGHT_LOGGING = 'true'; + }); + + const collectBoxLines = (): string[] => { + const captured = consoleErrorSpy.mock.calls[0][0] as string; + return captured.split('\n').filter((l) => l.length > 0); + }; + + it('aligns all borders when the thought is longer than the header', () => { + serverWithLogging.processThought({ + thought: 'Test thought with logging', + thoughtNumber: 1, + totalThoughts: 3, + nextThoughtNeeded: true, + }); + + const lines = collectBoxLines(); + const widths = lines.map((l) => stripAnsi(l).length); + expect(new Set(widths).size, + `Expected uniform line widths, got ${widths.join(',')} for:\n${lines.join('\n')}` + ).toBe(1); + }); + + it('aligns all borders for revision headers', () => { + serverWithLogging.processThought({ + thought: 'Revised thought', + thoughtNumber: 2, + totalThoughts: 3, + nextThoughtNeeded: true, + isRevision: true, + revisesThought: 1, + }); + + const widths = collectBoxLines().map((l) => stripAnsi(l).length); + expect(new Set(widths).size).toBe(1); + }); + + it('aligns all borders for branch headers', () => { + serverWithLogging.processThought({ + thought: 'Branch thought', + thoughtNumber: 2, + totalThoughts: 3, + nextThoughtNeeded: false, + branchFromThought: 1, + branchId: 'branch-a', + }); + + const widths = collectBoxLines().map((l) => stripAnsi(l).length); + expect(new Set(widths).size).toBe(1); + }); + + it('aligns all borders when the header is longer than the thought', () => { + serverWithLogging.processThought({ + thought: 'short', + thoughtNumber: 2, + totalThoughts: 3, + nextThoughtNeeded: true, + isRevision: true, + revisesThought: 1, + }); + + const widths = collectBoxLines().map((l) => stripAnsi(l).length); + expect(new Set(widths).size).toBe(1); + }); + + it('renders multi-line thoughts with one row per line and aligned borders', () => { + serverWithLogging.processThought({ + thought: 'first line of thought\nsecond line is shorter\nthird', + thoughtNumber: 1, + totalThoughts: 1, + nextThoughtNeeded: false, + }); + + const lines = collectBoxLines(); + const widths = lines.map((l) => stripAnsi(l).length); + expect( + new Set(widths).size, + `Expected uniform widths, got ${widths.join(',')} for:\n${lines.join('\n')}` + ).toBe(1); + + // Layout should now be: โ”Œโ”€โ” + header + โ”œโ”€โ”ค + 3 thought rows + โ””โ”€โ”˜ = 7 lines. + expect(lines.length).toBe(7); + }); + + it('handles CJK wide characters by treating them as two columns', () => { + serverWithLogging.processThought({ + thought: 'ไฝ ๅฅฝไธ–็•Œ', + thoughtNumber: 1, + totalThoughts: 1, + nextThoughtNeeded: false, + }); + + const lines = collectBoxLines(); + // Use lib's own displayWidth so the test is checking that the formatter + // is *internally consistent* with its width model. The model itself is + // pinned by the unit tests in `describe('displayWidth')` below. + const widths = lines.map(displayWidth); + expect( + new Set(widths).size, + `Expected uniform display widths, got ${widths.join(',')} for:\n${lines.join('\n')}` + ).toBe(1); + }); + }); + + describe('displayWidth', () => { + it('treats ASCII as 1 column per char', () => { + expect(displayWidth('hello')).toBe(5); + }); + + it('treats CJK ideographs as 2 columns', () => { + expect(displayWidth('ไฝ ๅฅฝ')).toBe(4); + expect(displayWidth('ไธ–็•Œ')).toBe(4); + }); + + it('treats SMP emoji as 2 columns', () => { + expect(displayWidth('๐Ÿ’ญ')).toBe(2); + expect(displayWidth('๐Ÿ”„')).toBe(2); + expect(displayWidth('๐ŸŒฟ')).toBe(2); + }); + + it('strips ANSI CSI escapes before counting', () => { + //  and  wrap blue text; both should be invisible. + expect(displayWidth('hi')).toBe(2); + }); + + it('handles mixed ASCII + CJK + emoji', () => { + expect(displayWidth('aไฝ ๐Ÿ’ญb')).toBe(1 + 2 + 2 + 1); + }); + + it('treats ZWJ-joined emoji as a single 2-column grapheme', () => { + // ๐Ÿ‘จโ€๐Ÿ’ป = man (U+1F468) + ZWJ (U+200D) + laptop (U+1F4BB) = 1 grapheme. + expect(displayWidth('๐Ÿ‘จโ€๐Ÿ’ป')).toBe(2); + }); + + it('treats VS-16 emoji presentation as 2 columns', () => { + // โš ๏ธ = warning sign (U+26A0) + VS-16 (U+FE0F) โ€” variation selector + // forces emoji (wide) presentation in modern terminals. + expect(displayWidth('โš ๏ธ')).toBe(2); + }); + }); + + describe('formatThought - tab handling', () => { + let consoleErrorSpy: ReturnType; + let serverWithLogging: SequentialThinkingServer; + + beforeEach(() => { + delete process.env.DISABLE_THOUGHT_LOGGING; + consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + serverWithLogging = new SequentialThinkingServer(); + }); + + afterEach(() => { + consoleErrorSpy.mockRestore(); + process.env.DISABLE_THOUGHT_LOGGING = 'true'; + }); + + it('expands tabs in thought so the box stays aligned', () => { + serverWithLogging.processThought({ + thought: 'foo\tbar\tbaz', + thoughtNumber: 1, + totalThoughts: 1, + nextThoughtNeeded: false, + }); + + const captured = consoleErrorSpy.mock.calls[0][0] as string; + // Tabs should not survive into the rendered box โ€” terminals expand + // them based on tab stops, which would knock the right border out + // of alignment. + expect(captured).not.toContain('\t'); + + const lines = captured.split('\n').filter((l) => l.length > 0); + const widths = lines.map(displayWidth); + expect( + new Set(widths).size, + `Expected uniform widths, got ${widths.join(',')} for:\n${lines.join('\n')}` + ).toBe(1); + }); + }); + + describe('formatThought - line wrapping', () => { + let consoleErrorSpy: ReturnType; + let serverWithLogging: SequentialThinkingServer; + + beforeEach(() => { + delete process.env.DISABLE_THOUGHT_LOGGING; + consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + serverWithLogging = new SequentialThinkingServer(); + }); + + afterEach(() => { + consoleErrorSpy.mockRestore(); + process.env.DISABLE_THOUGHT_LOGGING = 'true'; + }); + + const collectBoxLines = (): string[] => { + const captured = consoleErrorSpy.mock.calls[0][0] as string; + return captured.split('\n').filter((l) => l.length > 0); + }; + + it('wraps long ASCII thoughts to keep the box within 80 columns', () => { + serverWithLogging.processThought({ + thought: 'a'.repeat(200), + thoughtNumber: 1, + totalThoughts: 1, + nextThoughtNeeded: false, + }); + + const lines = collectBoxLines(); + const widths = lines.map(displayWidth); + expect( + Math.max(...widths), + `Expected max width <= 80, got ${Math.max(...widths)} for:\n${lines.join('\n')}` + ).toBeLessThanOrEqual(80); + // All rendered lines (borders + thought rows) should be uniform width. + expect(new Set(widths).size).toBe(1); + }); + + it('wraps long CJK thoughts at grapheme boundaries', () => { + // 100 CJK chars = ~200 display columns, must wrap. + const longCjk = 'ๆต‹่ฏ•ๅ†…ๅฎน'.repeat(25); + serverWithLogging.processThought({ + thought: longCjk, + thoughtNumber: 1, + totalThoughts: 1, + nextThoughtNeeded: false, + }); + + const lines = collectBoxLines(); + const widths = lines.map(displayWidth); + expect(Math.max(...widths)).toBeLessThanOrEqual(80); + expect(new Set(widths).size).toBe(1); + }); + + it('does not split inside a ZWJ emoji cluster when wrapping', () => { + // Pad with ASCII filler so wrapping must happen mid-content, + // and the ๐Ÿ‘จโ€๐Ÿ’ป cluster must stay intact across the wrap. + const filler = 'x'.repeat(76); + serverWithLogging.processThought({ + thought: `${filler}๐Ÿ‘จโ€๐Ÿ’ปend`, + thoughtNumber: 1, + totalThoughts: 1, + nextThoughtNeeded: false, + }); + + const captured = consoleErrorSpy.mock.calls[0][0] as string; + // ZWJ sequence intact: U+1F468 U+200D U+1F4BB must appear contiguously, + // i.e. no border characters injected mid-cluster. + expect(captured).toContain('๐Ÿ‘จโ€๐Ÿ’ป'); + }); + }); + + describe('processThought - input handling and error path', () => { + it('does not mutate the caller-supplied input object', () => { + const input: ThoughtData = { + thought: 'first', + thoughtNumber: 5, + totalThoughts: 3, // intentionally less than thoughtNumber + nextThoughtNeeded: true, + }; + const snapshot = JSON.parse(JSON.stringify(input)); + + const localServer = new SequentialThinkingServer(); + localServer.processThought(input); + + expect(input).toEqual(snapshot); + }); + + it('still reports the auto-adjusted totalThoughts in the response', () => { + const localServer = new SequentialThinkingServer(); + const result = localServer.processThought({ + thought: 'first', + thoughtNumber: 5, + totalThoughts: 3, + nextThoughtNeeded: true, + }); + + const data = JSON.parse(result.content[0].text); + expect(data.totalThoughts).toBe(5); + }); + + it('returns an error response when logging throws', () => { + // Re-enable logging so the failing console.error path executes. + delete process.env.DISABLE_THOUGHT_LOGGING; + const localServer = new SequentialThinkingServer(); + + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => { + throw new Error('stderr unavailable'); + }); + + const result = localServer.processThought({ + thought: 'will explode in stderr', + thoughtNumber: 1, + totalThoughts: 1, + nextThoughtNeeded: false, + }); + + errorSpy.mockRestore(); + process.env.DISABLE_THOUGHT_LOGGING = 'true'; + + expect(result.isError).toBe(true); + const data = JSON.parse(result.content[0].text); + expect(data.status).toBe('failed'); + expect(data.error).toContain('stderr unavailable'); + }); + }); + + describe('validateThoughtData', () => { + const baseInput: ThoughtData = { + thought: 'x', + thoughtNumber: 3, + totalThoughts: 5, + nextThoughtNeeded: true, + }; + + it('accepts a plain valid thought', () => { + expect(validateThoughtData(baseInput)).toEqual({ ok: true }); + }); + + it('rejects isRevision=true without revisesThought', () => { + const result = validateThoughtData({ ...baseInput, isRevision: true }); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toMatch(/revisesThought is required/); + } + }); + + it('rejects revisesThought >= thoughtNumber', () => { + const result = validateThoughtData({ + ...baseInput, + thoughtNumber: 3, + isRevision: true, + revisesThought: 3, + }); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toMatch(/must be earlier than thoughtNumber/); + } + }); + + it('accepts a valid revision pointing to an earlier thought', () => { + expect( + validateThoughtData({ + ...baseInput, + thoughtNumber: 3, + isRevision: true, + revisesThought: 1, + }) + ).toEqual({ ok: true }); + }); + + it('rejects branchFromThought without branchId', () => { + const result = validateThoughtData({ ...baseInput, branchFromThought: 1 }); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toMatch(/branchId is required/); + } + }); + + it('rejects branchFromThought >= thoughtNumber', () => { + const result = validateThoughtData({ + ...baseInput, + thoughtNumber: 3, + branchFromThought: 3, + branchId: 'b', + }); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toMatch(/must be earlier than thoughtNumber/); + } + }); + + it('accepts a valid branch from an earlier thought', () => { + expect( + validateThoughtData({ + ...baseInput, + thoughtNumber: 3, + branchFromThought: 1, + branchId: 'b', + }) + ).toEqual({ ok: true }); + }); + }); }); diff --git a/src/sequentialthinking/lib.ts b/src/sequentialthinking/lib.ts index 31a1098644..5669e5fb8f 100644 --- a/src/sequentialthinking/lib.ts +++ b/src/sequentialthinking/lib.ts @@ -1,5 +1,106 @@ import chalk from 'chalk'; +// CSI sequence (`[...m`) emitted by chalk for color/style. We strip these +// before measuring "how wide is this string in a terminal" so that width math +// for box drawing isn't thrown off by non-printing escape bytes. +const ANSI_CSI = /\[\d+(?:;\d+)*m/g; + +// Code-point ranges that render as full-width (2 columns) in conventional +// terminals: CJK ideographs, Hangul syllables, Hiragana/Katakana, and the +// SMP emoji blocks. This is a pragmatic subset of UAX #11 East Asian Width โ€” +// good enough to keep CJK and emoji thoughts from breaking the box without +// pulling in a dependency. +function isWideCodePoint(code: number): boolean { + return ( + // CJK / fullwidth (BMP) + (code >= 0x1100 && code <= 0x115f) || + (code >= 0x2e80 && code <= 0x303e) || + (code >= 0x3041 && code <= 0x33ff) || + (code >= 0x3400 && code <= 0x4dbf) || + (code >= 0x4e00 && code <= 0x9fff) || + (code >= 0xa000 && code <= 0xa4cf) || + (code >= 0xac00 && code <= 0xd7a3) || + (code >= 0xf900 && code <= 0xfaff) || + (code >= 0xfe30 && code <= 0xfe4f) || + (code >= 0xff00 && code <= 0xff60) || + (code >= 0xffe0 && code <= 0xffe6) || + // CJK Extension Bโ€“F (SMP) + (code >= 0x20000 && code <= 0x2fffd) || + (code >= 0x30000 && code <= 0x3fffd) || + // Emoji blocks rendered as 2 cols in modern terminals + (code >= 0x1f300 && code <= 0x1f6ff) || // Symbols & Pictographs, Emoticons, Transport + (code >= 0x1f900 && code <= 0x1f9ff) || // Supplemental Symbols & Pictographs + (code >= 0x1fa70 && code <= 0x1faff) // Symbols & Pictographs Extended-A + ); +} + +/** Width of a single grapheme cluster (1 narrow, 2 wide). */ +function graphemeWidth(segment: string): number { + if (segment.length === 0) return 0; + const firstCp = segment.codePointAt(0)!; + // VS-16 (U+FE0F) anywhere in the cluster forces emoji (wide) presentation. + if (isWideCodePoint(firstCp) || segment.includes('๏ธ')) return 2; + return 1; +} + +/** + * Best-effort terminal display width: strips ANSI styling, then sums column + * widths per Unicode grapheme cluster (so ZWJ-joined emoji like ๐Ÿ‘จโ€๐Ÿ’ป and + * variation-selected emoji like โš ๏ธ count as a single 2-column glyph). Not + * aware of every Unicode curiosity but covers the cases that show up in real + * thoughts: ASCII, CJK, common emoji, ZWJ sequences, VS-16. + */ +export function displayWidth(s: string): number { + const stripped = s.replace(ANSI_CSI, ''); + const segmenter = new Intl.Segmenter('en', { granularity: 'grapheme' }); + let width = 0; + for (const { segment } of segmenter.segment(stripped)) { + width += graphemeWidth(segment); + } + return width; +} + +const TAB_WIDTH = 4; + +// Default cap on how wide the rendered box may grow before we wrap thought +// lines. 80 columns is a safe lower bound for almost any terminal. +const DEFAULT_MAX_BOX_WIDTH = 80; + +/** Expand tab characters so the box width is predictable; terminals would + * otherwise jump to the next tab stop and break the right border. */ +function expandTabs(s: string): string { + return s.replace(/\t/g, ' '.repeat(TAB_WIDTH)); +} + +/** + * Wrap a single line so that its display width never exceeds `maxWidth`, + * splitting only at grapheme boundaries (preserves ZWJ emoji clusters, + * accented characters, etc.). Greedy: fills each row until the next cluster + * would overflow. + */ +export function wrapToWidth(line: string, maxWidth: number): string[] { + if (maxWidth <= 0) return [line]; + if (displayWidth(line) <= maxWidth) return [line]; + + const segmenter = new Intl.Segmenter('en', { granularity: 'grapheme' }); + const out: string[] = []; + let current = ''; + let currentWidth = 0; + for (const { segment } of segmenter.segment(line)) { + const w = graphemeWidth(segment); + if (currentWidth + w > maxWidth && current.length > 0) { + out.push(current); + current = segment; + currentWidth = w; + } else { + current += segment; + currentWidth += w; + } + } + if (current.length > 0) out.push(current); + return out; +} + export interface ThoughtData { thought: string; thoughtNumber: number; @@ -12,6 +113,45 @@ export interface ThoughtData { nextThoughtNeeded: boolean; } +export type ThoughtValidation = { ok: true } | { ok: false; error: string }; + +/** + * Cross-field semantic validation for ThoughtData. The Zod schema at the + * SDK boundary enforces field-level shape, but cannot express dependencies + * between fields ("revisesThought required when isRevision is true", + * "branchFromThought must point to an earlier thought", etc.). We run this + * after Zod parsing so the server returns helpful errors instead of + * silently rendering "(revising thought undefined)" or dropping branch + * data on the floor. + */ +export function validateThoughtData(input: ThoughtData): ThoughtValidation { + if (input.isRevision) { + if (input.revisesThought === undefined) { + return { ok: false, error: 'revisesThought is required when isRevision is true' }; + } + if (input.revisesThought >= input.thoughtNumber) { + return { + ok: false, + error: `revisesThought (${input.revisesThought}) must be earlier than thoughtNumber (${input.thoughtNumber})`, + }; + } + } + + if (input.branchFromThought !== undefined) { + if (input.branchId === undefined) { + return { ok: false, error: 'branchId is required when branchFromThought is set' }; + } + if (input.branchFromThought >= input.thoughtNumber) { + return { + ok: false, + error: `branchFromThought (${input.branchFromThought}) must be earlier than thoughtNumber (${input.thoughtNumber})`, + }; + } + } + + return { ok: true }; +} + export class SequentialThinkingServer { private thoughtHistory: ThoughtData[] = []; private branches: Record = {}; @@ -21,53 +161,93 @@ export class SequentialThinkingServer { this.disableThoughtLogging = (process.env.DISABLE_THOUGHT_LOGGING || "").toLowerCase() === "true"; } + /** Append spaces so the string occupies exactly `targetWidth` terminal columns. */ + private padToWidth(s: string, targetWidth: number): string { + const gap = targetWidth - displayWidth(s); + return gap > 0 ? s + ' '.repeat(gap) : s; + } + private formatThought(thoughtData: ThoughtData): string { const { thoughtNumber, totalThoughts, thought, isRevision, revisesThought, branchFromThought, branchId } = thoughtData; - let prefix = ''; + // Build the prefix in two forms: a styled version with chalk-injected ANSI + // codes for terminal output, and a plain version we measure for width math. + // Mixing the two avoids ANSI escape bytes inflating .length and throwing + // off the box border alignment. + let visiblePrefix: string; + let styledPrefix: string; let context = ''; if (isRevision) { - prefix = chalk.yellow('๐Ÿ”„ Revision'); + visiblePrefix = '๐Ÿ”„ Revision'; + styledPrefix = chalk.yellow(visiblePrefix); context = ` (revising thought ${revisesThought})`; } else if (branchFromThought) { - prefix = chalk.green('๐ŸŒฟ Branch'); + visiblePrefix = '๐ŸŒฟ Branch'; + styledPrefix = chalk.green(visiblePrefix); context = ` (from thought ${branchFromThought}, ID: ${branchId})`; } else { - prefix = chalk.blue('๐Ÿ’ญ Thought'); + visiblePrefix = '๐Ÿ’ญ Thought'; + styledPrefix = chalk.blue(visiblePrefix); context = ''; } - const header = `${prefix} ${thoughtNumber}/${totalThoughts}${context}`; - const border = 'โ”€'.repeat(Math.max(header.length, thought.length) + 4); + const visibleHeader = `${visiblePrefix} ${thoughtNumber}/${totalThoughts}${context}`; + const styledHeader = `${styledPrefix} ${thoughtNumber}/${totalThoughts}${context}`; + + // Multi-line thoughts get one row per line in the box. Otherwise a thought + // like "step 1\nstep 2" would render with the second line outside the box. + // Tabs are expanded so the right border stays put โ€” terminals advance to + // the next tab stop on \t which would otherwise misalign the box. + // Long lines are then wrapped at grapheme boundaries so the box doesn't + // overflow typical terminal widths. + const headerWidth = displayWidth(visibleHeader); + // Frame uses 4 columns (`โ”‚ ` + ` โ”‚`); cap content so total โ‰ค DEFAULT_MAX_BOX_WIDTH. + // Header is never wrapped โ€” it's short by construction โ€” so the inner width + // is at least the header width even if it slightly exceeds the cap. + const contentCap = Math.max(headerWidth, DEFAULT_MAX_BOX_WIDTH - 4); + const thoughtLines = expandTabs(thought) + .split('\n') + .flatMap((line) => wrapToWidth(line, contentCap)); + + const innerWidth = Math.max( + headerWidth, + ...thoughtLines.map(displayWidth), + ); + const border = 'โ”€'.repeat(innerWidth + 2); + const headerLine = `โ”‚ ${this.padToWidth(styledHeader, innerWidth)} โ”‚`; + const thoughtRows = thoughtLines + .map((line) => `โ”‚ ${this.padToWidth(line, innerWidth)} โ”‚`) + .join('\n'); return ` โ”Œ${border}โ” -โ”‚ ${header} โ”‚ +${headerLine} โ”œ${border}โ”ค -โ”‚ ${thought.padEnd(border.length - 2)} โ”‚ +${thoughtRows} โ””${border}โ”˜`; } public processThought(input: ThoughtData): { content: Array<{ type: "text"; text: string }>; isError?: boolean } { try { // Validation happens at the tool registration layer via Zod - // Adjust totalThoughts if thoughtNumber exceeds it - if (input.thoughtNumber > input.totalThoughts) { - input.totalThoughts = input.thoughtNumber; - } + // Adjust totalThoughts if thoughtNumber exceeds it. We work on a local + // copy so we never mutate the caller-supplied input โ€” handlers that + // forward the same object to other consumers shouldn't see our edits. + const totalThoughts = Math.max(input.totalThoughts, input.thoughtNumber); + const normalized: ThoughtData = { ...input, totalThoughts }; - this.thoughtHistory.push(input); + this.thoughtHistory.push(normalized); - if (input.branchFromThought && input.branchId) { - if (!this.branches[input.branchId]) { - this.branches[input.branchId] = []; + if (normalized.branchFromThought && normalized.branchId) { + if (!this.branches[normalized.branchId]) { + this.branches[normalized.branchId] = []; } - this.branches[input.branchId].push(input); + this.branches[normalized.branchId].push(normalized); } if (!this.disableThoughtLogging) { - const formattedThought = this.formatThought(input); + const formattedThought = this.formatThought(normalized); console.error(formattedThought); } @@ -75,9 +255,9 @@ export class SequentialThinkingServer { content: [{ type: "text" as const, text: JSON.stringify({ - thoughtNumber: input.thoughtNumber, - totalThoughts: input.totalThoughts, - nextThoughtNeeded: input.nextThoughtNeeded, + thoughtNumber: normalized.thoughtNumber, + totalThoughts: normalized.totalThoughts, + nextThoughtNeeded: normalized.nextThoughtNeeded, branches: Object.keys(this.branches), thoughtHistoryLength: this.thoughtHistory.length }, null, 2) From 2973713aa2c34a0d8a845832d06a9f4d61521487 Mon Sep 17 00:00:00 2001 From: Jia Xuan <1060996408+jiaxuan@users.noreply.github.com> Date: Tue, 5 May 2026 01:57:37 +0800 Subject: [PATCH 2/2] refactor(sequentialthinking): sync server version, extract handler, wire validator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - read the server version from package.json at startup so MCP clients see the same version that npm publishes; previously the McpServer was constructed with a hardcoded "0.2.0" while package.json had drifted to 0.6.2. - extract the tool handler into a named, exported handleSequentialThinkingTool so tests can exercise the SDK-facing contract (structuredContent shape, isError pass-through) without booting the stdio server. - guard runServer() with an ESM "is main module" check so importing index.ts from a test file does not auto-start the stdio loop. - call the new validateThoughtData up-front so cross-field violations return a well-formed isError response instead of silently rendering "(revising thought undefined)" or dropping branch data on the floor. Adds a new __tests__/index.test.ts covering the success path, isError pass-through, and the two validator failure modes. index.ts line coverage: 0% โ†’ 80%. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../__tests__/index.test.ts | 112 ++++++++++++++++++ src/sequentialthinking/index.ts | 87 +++++++++++--- 2 files changed, 181 insertions(+), 18 deletions(-) create mode 100644 src/sequentialthinking/__tests__/index.test.ts diff --git a/src/sequentialthinking/__tests__/index.test.ts b/src/sequentialthinking/__tests__/index.test.ts new file mode 100644 index 0000000000..6ceb060ae0 --- /dev/null +++ b/src/sequentialthinking/__tests__/index.test.ts @@ -0,0 +1,112 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { handleSequentialThinkingTool } from '../index.js'; +import { SequentialThinkingServer } from '../lib.js'; + +// Silence the chalk-styled stderr boxes during tests. +vi.mock('chalk', () => { + const id = (str: string) => str; + return { default: { yellow: id, green: id, blue: id } }; +}); + +describe('handleSequentialThinkingTool', () => { + let thinking: SequentialThinkingServer; + + beforeEach(() => { + process.env.DISABLE_THOUGHT_LOGGING = 'true'; + thinking = new SequentialThinkingServer(); + }); + + it('returns content + structuredContent on a successful call', async () => { + const result = await handleSequentialThinkingTool(thinking, { + thought: 'analyze', + thoughtNumber: 1, + totalThoughts: 3, + nextThoughtNeeded: true, + }); + + // Pass-through of the underlying content array, plus a parsed + // structuredContent matching the registered outputSchema. + expect(result).toHaveProperty('content'); + expect(result).toHaveProperty('structuredContent'); + + const sc = (result as { structuredContent: Record }).structuredContent; + expect(sc).toMatchObject({ + thoughtNumber: 1, + totalThoughts: 3, + nextThoughtNeeded: true, + thoughtHistoryLength: 1, + }); + expect(Array.isArray(sc.branches)).toBe(true); + }); + + it('passes through isError results without trying to parse structured content', async () => { + // Force the underlying call to fail by having the formatter throw on + // its console.error step. (Logging is enabled inside processThought.) + delete process.env.DISABLE_THOUGHT_LOGGING; + thinking = new SequentialThinkingServer(); + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => { + throw new Error('stderr unavailable'); + }); + + const result = await handleSequentialThinkingTool(thinking, { + thought: 'will explode', + thoughtNumber: 1, + totalThoughts: 1, + nextThoughtNeeded: false, + }); + + errSpy.mockRestore(); + process.env.DISABLE_THOUGHT_LOGGING = 'true'; + + expect(result).toHaveProperty('isError', true); + // Crucially, NOT structuredContent โ€” that would imply a successful parse + // and the SDK shouldn't see a half-formed response on the error branch. + expect(result).not.toHaveProperty('structuredContent'); + }); + + it('reflects auto-adjusted totalThoughts in structuredContent', async () => { + const result = await handleSequentialThinkingTool(thinking, { + thought: 'jump ahead', + thoughtNumber: 5, + totalThoughts: 3, // less than thoughtNumber: should be bumped to 5 + nextThoughtNeeded: true, + }); + + const sc = (result as { structuredContent: Record }).structuredContent; + expect(sc.totalThoughts).toBe(5); + }); + + it('returns an isError response when isRevision is set without revisesThought', async () => { + const result = await handleSequentialThinkingTool(thinking, { + thought: 'oops', + thoughtNumber: 2, + totalThoughts: 2, + nextThoughtNeeded: false, + isRevision: true, + }); + + expect(result).toMatchObject({ isError: true }); + expect(result).not.toHaveProperty('structuredContent'); + const text = (result as { content: { text: string }[] }).content[0].text; + expect(JSON.parse(text)).toMatchObject({ + status: 'failed', + error: expect.stringMatching(/revisesThought is required/), + }); + }); + + it('returns an isError response when branchFromThought is set without branchId', async () => { + const result = await handleSequentialThinkingTool(thinking, { + thought: 'oops', + thoughtNumber: 2, + totalThoughts: 2, + nextThoughtNeeded: false, + branchFromThought: 1, + }); + + expect(result).toMatchObject({ isError: true }); + const text = (result as { content: { text: string }[] }).content[0].text; + expect(JSON.parse(text)).toMatchObject({ + error: expect.stringMatching(/branchId is required/), + }); + }); +}); diff --git a/src/sequentialthinking/index.ts b/src/sequentialthinking/index.ts index 217845bb3d..cb6e10fe0d 100644 --- a/src/sequentialthinking/index.ts +++ b/src/sequentialthinking/index.ts @@ -1,9 +1,30 @@ #!/usr/bin/env node +import { createRequire } from "node:module"; +import { pathToFileURL } from "node:url"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; import { z } from "zod"; -import { SequentialThinkingServer } from './lib.js'; +import { SequentialThinkingServer, validateThoughtData } from './lib.js'; + +// Load version from package.json so the server reports a single source of +// truth to MCP clients (previously hardcoded and silently drifted from npm). +// The relative path differs between source layout (./package.json) and the +// published / compiled layout where index.js sits inside dist/ and the +// package.json is one level up (../package.json). Try both so vitest (which +// runs the TS source) and the published bin both resolve correctly. +const require = createRequire(import.meta.url); +function loadVersion(): string { + for (const candidate of ['../package.json', './package.json']) { + try { + return (require(candidate) as { version: string }).version; + } catch { + // try next candidate + } + } + return '0.0.0'; +} +const packageVersion = loadVersion(); /** Safe boolean coercion that correctly handles string "false" */ const coercedBoolean = z.preprocess((val) => { @@ -17,7 +38,7 @@ const coercedBoolean = z.preprocess((val) => { const server = new McpServer({ name: "sequential-thinking-server", - version: "0.2.0", + version: packageVersion, }); const thinkingServer = new SequentialThinkingServer(); @@ -105,22 +126,43 @@ You should: thoughtHistoryLength: z.number() }, }, - async (args) => { - const result = thinkingServer.processThought(args); - - if (result.isError) { - return result; - } - - // Parse the JSON response to get structured content - const parsedContent = JSON.parse(result.content[0].text); + async (args) => handleSequentialThinkingTool(thinkingServer, args) +); +/** + * Handle a single `sequentialthinking` tool call. Extracted so tests can + * exercise the SDK-facing contract (structuredContent shape, isError + * pass-through) without booting the McpServer over stdio. + */ +export async function handleSequentialThinkingTool( + thinking: SequentialThinkingServer, + args: Parameters[0], +) { + // Cross-field semantic check that Zod can't express on a field-level + // ZodRawShape (e.g. "revisesThought required when isRevision is true"). + const validation = validateThoughtData(args); + if (!validation.ok) { return { - content: result.content, - structuredContent: parsedContent + isError: true, + content: [{ + type: "text" as const, + text: JSON.stringify({ error: validation.error, status: 'failed' }, null, 2), + }], }; } -); + + const result = thinking.processThought(args); + if (result.isError) { + return result; + } + // Re-parse the JSON our processThought serialized so the SDK can deliver + // it as `structuredContent` per the outputSchema we registered. + const structuredContent = JSON.parse(result.content[0].text); + return { + content: result.content, + structuredContent, + }; +} async function runServer() { const transport = new StdioServerTransport(); @@ -128,7 +170,16 @@ async function runServer() { console.error("Sequential Thinking MCP Server running on stdio"); } -runServer().catch((error) => { - console.error("Fatal error running server:", error); - process.exit(1); -}); +// Only auto-start when this module is the entrypoint (e.g. invoked via the +// `mcp-server-sequential-thinking` bin). Importing it from a test or another +// module must not boot the stdio server as a side effect. +const isMain = process.argv[1] + ? import.meta.url === pathToFileURL(process.argv[1]).href + : false; + +if (isMain) { + runServer().catch((error) => { + console.error("Fatal error running server:", error); + process.exit(1); + }); +}