Skip to content

Fix search tool crash on CRLF search results#309

Merged
aebrer merged 5 commits into
masterfrom
feature/issue-308-fix-search-crlf-crash
Jul 1, 2026
Merged

Fix search tool crash on CRLF search results#309
aebrer merged 5 commits into
masterfrom
feature/issue-308-fix-search-crlf-crash

Conversation

@aebrer

@aebrer aebrer commented Jul 1, 2026

Copy link
Copy Markdown
Owner

Closes #308

Fix the search tool crash when CRLF-containing search output reaches the interactive TUI.

Implementation plan posted as a comment below.

@aebrer

aebrer commented Jul 1, 2026

Copy link
Copy Markdown
Owner Author

Implementation Plan

Analysis

The TUI crash is caused by raw carriage returns surviving through the search result rendering path. Source files can be indexed with CRLF line endings, formatResults() currently splits chunk content only on "\n", and formatSearchResult() also splits only on "\n". When the resulting string is rendered by Text in soft-wrap mode, the component can return an array entry that still contains "\r", and TUI.assertLineFits() correctly fails loud because component render output must contain one logical line per entry.

The invariant in packages/tui/src/tui.ts should stay strict. The fix should normalize line endings before component output reaches the TUI guard, with tests covering both the search-specific path and the reusable text-rendering path.

Deliverables

  • Normalize search result display text so CRLF and lone-CR input cannot reach the Text component as embedded raw carriage returns.
  • Normalize semantic-search preview formatting so search output is clean at the producer boundary as well as the coding-agent renderer boundary.
  • Harden the Text component against CRLF/lone-CR input so its own render() contract is upheld even when callers pass platform line endings.
  • Add regression tests for search formatting, semantic-search formatting, and Text soft-wrap rendering.
  • Leave the TUI raw CR/LF guard intact.

Files to modify

  • packages/semantic-search/src/format.ts

    • Split chunk previews with CRLF-aware line handling instead of plain split("\n").
    • Preserve current preview truncation and “more lines” behavior.
  • packages/coding-agent/src/core/tools/search.ts

    • Normalize result text before line slicing/styling.
    • Prefer the existing shared display-output helpers where appropriate, or use equivalent CRLF/lone-CR normalization local to the formatter.
    • Preserve collapsed vs expanded behavior and index-stats footer behavior.
  • packages/tui/src/components/text.ts

    • Normalize input line endings before the soft-wrap split and hard-wrap flow.
    • Preserve tab replacement, padding, background erase, and wrappable-line behavior.
  • packages/coding-agent/test/search/search-tool.test.ts

    • Add formatSearchResult() coverage for CRLF and lone-CR result text.
    • Verify returned formatted output contains no raw "\r" and still renders expected logical lines/truncation behavior.
  • packages/semantic-search/test/mcp-server.test.ts

    • Add formatResults() coverage for chunk content with CRLF line endings.
    • Verify preview lines do not contain raw "\r" and “more lines” counts remain correct.
  • packages/tui/test/markdown-softwrap.test.ts or a focused text-component test file

    • Add Text soft-wrap coverage for CRLF and lone-CR input.
    • Assert every rendered array entry contains no raw CR/LF and expected logical text is preserved.

Acceptance Criteria

  • Search results sourced from CRLF-containing content do not crash the interactive TUI.
  • Search result rendering emits clean logical lines with no embedded raw carriage returns.
  • Text.render() does not return raw CR/LF entries for CRLF or lone-CR input.
  • Existing strict TUI.assertLineFits() behavior remains unchanged.
  • Existing search result truncation, expanded mode, and index-stats footer behavior remain unchanged.
  • Regression tests cover CRLF and lone-CR cases.

Testing Approach

Targeted tests after implementation:

cd packages/coding-agent && npx vitest --run test/search/search-tool.test.ts test/tool-execution-component.test.ts
cd packages/semantic-search && npx vitest --run test/mcp-server.test.ts
cd packages/tui && node --test --import tsx test/markdown-softwrap.test.ts test/wrap-soft.test.ts

Broader validation:

cd packages/coding-agent && npm test
cd packages/semantic-search && npm test
cd packages/tui && npm test

Required repository build after code changes before any real binary testing:

npm run build

Risks and Open Questions

  • Normalizing lone "\r" to line breaks versus deleting it can affect display shape. The implementation should choose the behavior that best preserves logical lines and document it in tests.
  • Search formatting is consumed outside the interactive TUI, so producer-side normalization in semantic-search should avoid surprising MCP consumers while still removing terminal-hostile carriage returns.
  • ANSI styling should be preserved when normalizing coding-agent display text.

Plan created by mach6

@aebrer

aebrer commented Jul 1, 2026

Copy link
Copy Markdown
Owner Author

Progress Update

Implemented the CRLF search-result crash fix:

  • Normalized CRLF and lone-CR line endings in semantic-search result previews.
  • Normalized search tool result text before collapsed/expanded slicing and styling.
  • Hardened the TUI Text component so CRLF/lone-CR input renders as separate logical lines without raw CR/LF entries.
  • Added regression coverage for semantic-search formatting, search tool result rendering, TUI Text soft-wrap rendering, and stabilized the length-retry test fixture against live model registry token-limit changes discovered during validation.

Verification completed:

  • cd packages/coding-agent && npx vitest --run test/search/search-tool.test.ts test/tool-execution-component.test.ts
  • cd packages/semantic-search && npx vitest --run test/mcp-server.test.ts
  • cd packages/tui && node --test --import tsx test/markdown-softwrap.test.ts test/wrap-soft.test.ts
  • npx biome check --write ...
  • npm test
  • npm run build
  • npm run verify-workspace-links
  • pre-commit hook test suite passed during commit

Commit: aaeea5b


Progress tracked by mach6

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

Vitest coverage

Metric Covered Total Coverage
Statements 18180 34424 52.81%
Branches 9871 21325 46.28%
Functions 2862 5404 52.96%
Lines 16062 30620 52.45%

View full coverage run

@aebrer aebrer marked this pull request as ready for review July 1, 2026 15:17
@aebrer

aebrer commented Jul 1, 2026

Copy link
Copy Markdown
Owner Author

Code Review

Critical

None.

Important

None.

Suggestions

Finding 1 — Missing hard-wrap regression test for Text line-ending normalization
Severity: low. Confidence: 91.
packages/tui/src/components/text.ts now normalizes CRLF and lone-CR input before both soft-wrap and default hard-wrap rendering, but the added Text coverage only exercises the soft-wrap path. Add a default-mode test with CRLF/lone-CR text to prove hard-wrap rendering also strips raw carriage returns and preserves logical line splitting.

Finding 2 — Simplify normalized search output expression
Severity: low. Confidence: 98.
packages/coding-agent/src/core/tools/search.ts can replace the ternary around result.content[0]?.text with normalizeLineEndings(result.content[0]?.text ?? "").trim() to avoid repeating the indexed lookup while preserving behavior.

Finding 3 — Extract repeated stable Sonnet retry-test fixture
Severity: low. Confidence: 93.
packages/coding-agent/test/agent-session-retry.test.ts repeats the same cloned Sonnet model fixture with a pinned maxTokens value in two tests. A small helper would keep that intent in one place while still returning a fresh cloned model for each test.

Strengths

  • The fix normalizes line endings at producer, renderer, and reusable TUI component boundaries while leaving the strict TUI raw CR/LF guard intact.
  • Regression tests cover CRLF and lone-CR behavior in semantic-search previews, search result rendering, and the original soft-wrap TUI path.
  • The semantic-search preview test verifies the “more lines” count remains correct after CRLF/lone-CR splitting.
  • The retry-test fixture adjustment appears to improve determinism by decoupling tests from live model registry token-limit changes.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier


Reviewed by mach6

@aebrer

aebrer commented Jul 1, 2026

Copy link
Copy Markdown
Owner Author

Review Assessment

Review comment: #309 (comment)

Classifications

Finding Classification Reasoning
Finding 1: Missing hard-wrap regression test for Text line-ending normalization genuine Confirmed. Text.render() now normalizes CRLF/lone-CR before both soft-wrap and default hard-wrap paths, but only the soft-wrap CR/LF behavior is directly tested. Since the PR changed hard-wrap behavior too, add a direct regression test before merge.
Finding 2: Simplify normalized search output expression nitpick The suggested normalizeLineEndings(result.content[0]?.text ?? "").trim() expression is equivalent for the typed text?: string case, but the current code is correct and tested. This is readability polish only.
Finding 3: Extract repeated stable Sonnet retry-test fixture nitpick The duplicated cloned Sonnet fixture is clear and does not affect correctness. A helper would reduce repetition, but this is maintainability polish only.

Action Plan

  1. Add a default hard-wrap Text regression test with CRLF and lone-CR input, asserting no raw CR/LF remains and the logical lines split correctly.

Assessment by mach6

@aebrer

aebrer commented Jul 1, 2026

Copy link
Copy Markdown
Owner Author

Progress Update

Fixed review finding 1 by adding hard-wrap Text regression coverage for CRLF and lone-CR input.

  • Added a default hard-wrap test in packages/tui/test/markdown-softwrap.test.ts.
  • The test asserts rendered entries contain no raw CR/LF, logical lines split correctly, hard-wrap output is not marked as wrappable, and padded line widths remain correct.

Verification completed before commit:

  • cd packages/tui && node --test --import tsx test/markdown-softwrap.test.ts
  • npx biome check --write packages/tui/test/markdown-softwrap.test.ts
  • npm test
  • npm run build
  • npm run verify-workspace-links
  • pre-commit hook test suite passed during commit

Commit: 41f1de6


Progress tracked by mach6

@aebrer aebrer merged commit fd4d999 into master Jul 1, 2026
3 checks passed
@aebrer aebrer deleted the feature/issue-308-fix-search-crlf-crash branch July 1, 2026 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix search tool crash on CRLF search results

1 participant