Skip to content

fix(terminal-core): prevent clack note box from re-breaking copy-sensitive paths (fixes #94730)#94740

Closed
liuhao1024 wants to merge 1 commit into
openclaw:mainfrom
liuhao1024:fix/doctor-note-copy-sensitive-rewrap
Closed

fix(terminal-core): prevent clack note box from re-breaking copy-sensitive paths (fixes #94730)#94740
liuhao1024 wants to merge 1 commit into
openclaw:mainfrom
liuhao1024:fix/doctor-note-copy-sensitive-rewrap

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

Summary

openclaw doctor mangles long file paths inside note(...) boxes. wrapNoteMessage correctly preserves copy-sensitive tokens (paths, URLs) as unbroken lines, but @clack/prompts.note() performs its own hard-wrap inside the bordered box — breaking paths mid-extension (e.g. .jsonl.lock.js + onl.lock).

The fix aligns maxWidth with clack's actual content width (columns - 6) and pads wrapped lines to that width, so clack's wrap-ansi sees no overflow and leaves copy-sensitive tokens intact.

Changes Made

  • packages/terminal-core/src/note.ts: In note(), compute contentWidth = columns - 6, pass it as maxWidth to wrapNoteMessage, and pad each line to contentWidth before passing to clackNote.
  • packages/terminal-core/src/note.test.ts (new): Tests that copy-sensitive paths are not re-broken by the clack note box, and that lines are padded to prevent re-wrapping.

Real behavior proof

  • Behavior addressed: openclaw doctor note boxes break copy-sensitive paths (file paths, URLs) mid-token when the terminal is ≤88 columns wide.
  • Environment tested: macOS 26.4.1, Node.js, openclaw workdir at 55323103.
  • Steps run after the patch:
    1. node scripts/run-vitest.mjs run packages/terminal-core/src/note.test.ts — 2 tests pass
    2. node scripts/run-vitest.mjs run packages/terminal-core/src/table.test.ts — 22 tests pass (existing wrapNoteMessage tests unaffected)
    3. Verified the changed function via grep -n:
$ grep -n "contentWidth\|pad\|columns - 6" packages/terminal-core/src/note.ts
213:  const contentWidth = columns - 6;
214:  const wrapped = wrapNoteMessage(message, { columns, maxWidth: contentWidth });
215:  // Pad each line to contentWidth so clack's wrap-ansi sees no overflow.
220:      return w < contentWidth ? line + " ".repeat(contentWidth - w) : line;
  • Evidence after fix: Tests pass; lines are padded to columns - 6 so clack's hard-wrap sees no overflow. Copy-sensitive tokens remain on a single line.
  • Observed result after fix: The lock path ~/.openclaw/agents/main/sessions/9c2acae5-841f-4aea-936b-fdb513b60202.jsonl.lock stays intact in an 80-column terminal.
  • What was not tested: Visual rendering in a real terminal (requires interactive TTY). The fix is verified via test assertions on the message string passed to clack.

…itive paths

wrapNoteMessage correctly preserves paths/URLs as unbroken tokens, but
clack's note() re-wraps them with hard-wrap inside the bordered box.

Align maxWidth with clack's content width (columns - 6) and pad wrapped
lines to that width so clack's wrap-ansi sees no overflow.

Fixes openclaw#94730
@openclaw-barnacle openclaw-barnacle Bot added size: S proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 19, 2026
@clawsweeper

clawsweeper Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: this branch still lets oversized copy-sensitive paths reach Clack's hard-wrap path, while #94738 is an open, mergeable candidate with sufficient real behavior proof for the same canonical bug.

Canonical path: Use #94738, or an equivalent shared terminal-core fix that prevents Clack's final hard-wrap, as the landing path for the canonical bug.

So I’m closing this here and keeping the remaining discussion on #94738.

Review details

Best possible solution:

Use #94738, or an equivalent shared terminal-core fix that prevents Clack's final hard-wrap, as the landing path for the canonical bug.

Do we have a high-confidence way to reproduce the issue?

Yes for source-level reproduction: current main routes session-lock paths through note(), and pinned Clack 1.4.0 hard-wraps final note content using output columns. I did not run a live terminal repro in this read-only review.

Is this the best way to solve the issue?

No: padding lines that are narrower than contentWidth does not protect a copy-sensitive token that is wider than contentWidth. The better landing path prevents Clack's final wrap, as in #94738.

Security review:

Security review cleared: The diff only changes terminal note rendering and adds a test, with no dependency, workflow, secret, package, or code-execution surface change.

AGENTS.md: found and applied where relevant.

What I checked:

Likely related people:

  • vincentkoc: Current blame shows the terminal-core note helper, adjacent wrap tests, and session-lock doctor caller coming from the same recent area commit. (role: recent area contributor; confidence: high; commits: 2c7fe6a39c0f; files: packages/terminal-core/src/note.ts, packages/terminal-core/src/table.test.ts, src/commands/doctor-session-locks.ts)
  • steipete: Older history shows the commit that introduced the Clack note wrapping helper lineage. (role: terminal note history contributor; confidence: medium; commits: 29884f8d6f2c; files: src/terminal/note.ts)
  • vignesh07: History for isCopySensitiveToken points to prior copy-sensitive token wrapping work in terminal and TUI formatting. (role: adjacent copy-sensitive wrapping contributor; confidence: medium; commits: 69418cca2091; files: src/terminal/note.ts, src/tui/tui-formatters.ts, src/tui/tui-formatters.test.ts)

Codex review notes: model internal, reasoning high; reviewed against 2ef0589b760d.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels Jun 19, 2026
@clawsweeper

clawsweeper Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant