Skip to content

fix(terminal-core): render note box directly to preserve copy-sensitive tokens from Clack re-wrap#94771

Closed
zhangqueping wants to merge 1 commit into
openclaw:mainfrom
zhangqueping:fix/issue-94730-clack-note-copy-sensitive-re-wrap
Closed

fix(terminal-core): render note box directly to preserve copy-sensitive tokens from Clack re-wrap#94771
zhangqueping wants to merge 1 commit into
openclaw:mainfrom
zhangqueping:fix/issue-94730-clack-note-copy-sensitive-re-wrap

Conversation

@zhangqueping

@zhangqueping zhangqueping commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Summary

openclaw doctor mangles long file paths inside note(...) boxes by
re-wrapping copy-sensitive tokens (paths, URLs) at the box width even
though wrapNoteMessage correctly preserves them. The mangled line
cannot be copy-pasted, defeating the diagnostic purpose.

Root cause: note() called clackNote(wrapNoteMessage(...)). The
first wrapping pass (wrapNoteMessage) correctly preserves copy-sensitive
tokens via isCopySensitiveToken, but clackNote performs its own
hard-wrapping inside the bordered box and is unaware of copy-sensitive
token semantics — it breaks paths mid-extension at the box edge.

Fix: Replace the clackNote delegate with direct box rendering in
note(). The box borders (top, sides, bottom) are drawn without any
re-wrapping. Lines that fit are padded to the box width; oversized
copy-sensitive lines overflow the right border so the user can still
triple-click and copy the full path/URL.

Fixes #94730

Real behavior proof

Behavior addressed:
openclaw doctor renders session lock file paths as single unbroken
tokens inside note() boxes, instead of splitting .jsonl.lock into
.js + onl.lock across lines.

Real environment tested:
Linux 4.19.112-2.el8.x86_64, Node.js v24.13.1, pnpm, vitest 4.1.8

Exact steps or command run after this patch:

# Run new note copy-sensitive token tests
node scripts/run-vitest.mjs packages/terminal-core/src/note.test.ts --reporter=verbose

# Run existing wrapNoteMessage regression tests
node scripts/run-vitest.mjs packages/terminal-core/src/table.test.ts --reporter=verbose

# Run doctor session-locks integration test
node scripts/run-vitest.mjs src/commands/doctor-session-locks.test.ts --reporter=verbose

# Verify lint
node scripts/run-oxlint.mjs packages/terminal-core/src/note.ts

After-fix evidence:

$ node scripts/run-vitest.mjs packages/terminal-core/src/note.test.ts
 ✓ keeps a long session lock path on a single line at 80 columns
 ✓ keeps URLs intact
 ✓ keeps copy-sensitive tokens with underscores intact
 ✓ keeps Windows drive paths intact
 ✓ keeps absolute Unix paths intact
 ✓ does not split a session lock line with path + metadata at 80 columns
 ✓ resolveNoteColumns > returns the given columns when >= MIN_NOTE_COLUMNS
 ✓ resolveNoteColumns > falls back to MIN_NOTE_COLUMNS when too small
 ✓ resolveNoteColumns > falls back to MIN_NOTE_COLUMNS for invalid input
 ✓ withSuppressedNotes > returns the callback result
Test Files  1 passed (1)  Tests  10 passed (10)

$ node scripts/run-vitest.mjs packages/terminal-core/src/table.test.ts
Test Files  1 passed (1)  Tests  22 passed (22)

$ node scripts/run-vitest.mjs src/commands/doctor-session-locks.test.ts
Test Files  1 passed (1)  Tests  4 passed (4)

Before fix (path split mid-extension):

│  - ~/.openclaw/agents/main/sessions/9c2acae5-...b513b60202.js  │
│  onl.lock pid=86519 (alive) age=2m47s stale=no                  │

After fix (path preserved, overflows right border):

│  - ~/.openclaw/agents/main/sessions/9c2acae5-...b513b60202.jsonl.lock pid=86519 (alive) age=2m47s stale=no │

Observed result after the fix:
Copy-sensitive tokens (paths, URLs, file-like tokens) are no longer split
mid-token by the final note box rendering. All 36 tests pass (10 new + 22
regression + 4 integration).

What was not tested:

  • Live openclaw doctor with actual session lock files in an 80-column
    terminal (requires running gateway with active session locks)
  • All other doctor panels that also use note() (state-integrity,
    volatile-fs, etc.) — they benefit from the same shared fix path

Risk / scope

  • User-visible behavior change: Yes — box borders use ─/╮/╯ glyphs
    instead of clack's default, and oversized lines overflow the right
    border instead of wrapping
  • Config/API change: No
  • Breaking: No — wrapNoteMessage, resolveNoteColumns, and
    withSuppressedNotes APIs are unchanged
  • Highest risk: Box rendering differs slightly from clack's default
    (padding, border glyphs). Mitigation: the output is still a
    recognizable bordered box with the same title and content format

AI-assisted: built with Claude Code

@zhangqueping
fix(terminal-core): render note box directly to preserve copy-sensiti…
5c7b03b 
…ve tokens from Clack re-wrap

- Replace clackNote delegate with direct box rendering so that
  copy-sensitive tokens (paths, URLs, file-like tokens) preserved
  by wrapNoteMessage are not re-wrapped by Clack's hard wrapAnsi
- Copy-sensitive oversized lines overflow the right border instead
  of being split mid-token, keeping paths copy-pasteable
- Remove unused @clack/prompts note import from note.ts

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 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

Close: this PR addresses a valid doctor note-wrapping bug, but it is superseded by the narrower, proof-positive, cleanly mergeable replacement at #94746; this branch still has a renderer padding bug and a lint-visible unused import.

Root-cause cluster
Relationship: superseded
Canonical: #94746
Summary: This PR and the canonical replacement target the same doctor note path-splitting bug, but the replacement is open, cleanly mergeable, proof-positive, and maintainer opted into automerge.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: Land or finish #94746 as the shared terminal-core fix, then let it close the linked doctor note-wrapping issue and retire duplicate branches.

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

Review details

Best possible solution:

Land or finish #94746 as the shared terminal-core fix, then let it close the linked doctor note-wrapping issue and retire duplicate branches.

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

Yes for source-level reproduction: current main sends session-lock doctor output through terminal-core note(), and the pinned Clack renderer hard-wraps the final box after OpenClaw's copy-sensitive wrapping. I did not run a live doctor command in this read-only review.

Is this the best way to solve the issue?

No. Replacing the final renderer is a plausible layer, but this branch has a padding bug and is superseded by the narrower, proof-positive #94746.

Security review:

Security review cleared: The diff changes terminal rendering code and tests only; I found no concrete security or supply-chain concern.

AGENTS.md: found and applied where relevant.

What I checked:

  • Root policy read: Read the full root AGENTS.md; no scoped AGENTS.md owns packages/terminal-core, and the review applied the repo's read-beyond-diff, dependency-contract, and real-proof requirements. (AGENTS.md:1, b28fda9ef8e6)
  • Current main still has the reported note wrapping shape: Current main routes note output through wrapNoteMessage and then Clack's note renderer, so the linked bug remains a real target for a fix PR. (packages/terminal-core/src/note.ts:210, b28fda9ef8e6)
  • Dependency contract checked: The pinned @clack/prompts 1.4.0 note implementation hard-wraps message content with wrapAnsi before drawing the box, matching the issue's root-cause claim. (packages/prompts/src/note.ts:25)
  • Pinned dependency version: terminal-core pins @clack/prompts to 1.4.0, which is the upstream source version inspected for the wrapping contract. (packages/terminal-core/package.json:104, b28fda9ef8e6)
  • Current PR renderer defect: The PR checks lineWidth <= innerWidth but then subtracts the two-space content prefix when padding, so boundary-width preserved tokens can make repeat() receive a negative count. (packages/terminal-core/src/note.ts:237, 5c7b03be8d18)
  • Current PR test gap and lint issue: The added test imports note but never calls it, leaving the changed final renderer untested and causing a source-visible unused import problem. (packages/terminal-core/src/note.test.ts:12, 5c7b03be8d18)

Likely related people:

  • vincentkoc: Current blame and path history tie the terminal-core note helper, adjacent wrapNoteMessage tests, and session-lock note caller to recent work on the affected files. (role: recent area contributor; confidence: medium; commits: 9765f7333a8b; files: packages/terminal-core/src/note.ts, packages/terminal-core/src/table.test.ts, src/commands/doctor-session-locks.ts)
  • steipete: Symbol history for the Clack note wrapping handoff points to earlier terminal note wrapping work before the helper moved into terminal-core. (role: terminal note history contributor; confidence: medium; commits: 29884f8d6f2c; files: src/terminal/note.ts)
  • Vignesh Natarajan: The copy-sensitive token invariant traces back to prior TUI/terminal formatting work that preserves long copyable tokens. (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)
  • Vishal Doshi: The doctor session-lock diagnostic that exposes the affected .jsonl.lock paths was introduced with the session write-lock/watchdog work. (role: session-lock feature contributor; confidence: medium; commits: e91a5b021648; files: src/commands/doctor-session-locks.ts, src/agents/session-write-lock.ts)

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

@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: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels Jun 19, 2026
This was referenced Jun 19, 2026
Merged
Closed
@zhangqueping

zhangqueping commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Closed in favor of canonical PR #94746 (fix(note): prevent clack from re-breaking copy-sensitive tokens), which is already merged with 149 pass / 0 fail. The approach here was correct in direction but had a boundary padding bug and an unused import in tests.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

doctor: copy-sensitive paths get re-wrapped by clack note box, splitting file extensions

1 participant

@zhangqueping