Skip to content

fix(cli): keep session key tables Unicode-safe#103010

Merged
steipete merged 1 commit into
mainfrom
codex/sessions-table-utf16
Jul 9, 2026
Merged

fix(cli): keep session key tables Unicode-safe#103010
steipete merged 1 commit into
mainfrom
codex/sessions-table-utf16

Conversation

@steipete

@steipete steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Closes #103001

AI-assisted: yes (Codex).

What Problem This Solves

Fixes an issue where operators listing sessions, or previewing session cleanup, could see replacement characters in a long session key when an emoji or other supplementary-plane character crossed either truncation boundary.

Why This Change Was Made

The shared session-table formatter now uses the canonical UTF-16-safe prefix and suffix helpers while preserving its existing 16-unit prefix, ellipsis, 6-unit suffix, and 26-unit cell budget. A focused regression test places emoji across both retained boundaries so the old raw slicing fails deterministically.

This stays at the shared formatter owner boundary: normal session listing and cleanup dry-run both benefit, while JSON output and persisted keys remain unchanged.

User Impact

Long Unicode session keys remain recognizable in terminal tables instead of displaying characters. ASCII keys, short keys, output width, and the meaningful prefix/suffix layout keep their existing behavior.

Evidence

Blacksmith Testbox lease: tbx_01kx3w5bfyc64dwp5g3rj4cr7y (brisk-crayfish).

  • pnpm test src/commands/sessions-table.test.ts: 1/1 passed.
  • pnpm test src/commands/sessions-table.test.ts src/commands/sessions.test.ts src/commands/sessions-cleanup.test.ts: 26/26 passed across three files.
  • pnpm check:changed: passed (core + coreTests; typecheck, lint, and guards clean).
  • Real CLI fixture: openclaw sessions --store /tmp/openclaw-sessions-utf16.json --limit all rendered aaaaaaaaaaaaaaa...zzzzz without replacement characters.
  • Old-slice control on the same Testbox reproduced dangling surrogate indices 15 and 19 and terminal text aaaaaaaaaaaaaaa�...�zzzzz.
  • Source-blind behavior validation passed 7/7 clauses and probes: normal and cleanup tables, full-key JSON preservation, short/varied/ASCII keys, forbidden-surrogate scan, and a byte-identical retry.
  • Fresh autoreview: clean, no accepted/actionable findings; correctness confidence 0.99.
  • Exact-head CI at f4fa56da1449596c9f23a6aacf47f06fff7c5737: CI run and real behavior proof passed. Verification snapshot: 65 successful checks, 36 skipped, zero failures or pending checks.
  • git diff --check: passed.

@openclaw-barnacle openclaw-barnacle Bot added commands Command implementations size: XS maintainer Maintainer-authored PR labels Jul 9, 2026
@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 9, 2026, 1:03 PM ET / 17:03 UTC.

Summary
The PR changes the shared session table formatter to truncate long session keys with UTF-16-safe helpers and adds a focused regression test for emoji crossing both retained boundaries.

PR surface: Source +1, Tests +14. Total +15 across 2 files.

Reproducibility: yes. The linked issue gives a concrete CLI fixture, and current main's raw formatter slices make the dangling-surrogate failure clear from source inspection.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #103001
Summary: The linked issue reports the exact Unicode session-key table truncation bug this PR fixes.

Members:

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

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Next step before merge

  • No ClawSweeper repair job is needed because the open PR already contains the narrow code and test change.

Security
Cleared: The diff only changes CLI string formatting and adds a colocated unit test; it does not touch dependencies, scripts, workflows, secrets, or code execution paths.

Review details

Best possible solution:

Land the shared formatter fix after normal exact-head CI and maintainer merge handling, keeping JSON output and persisted session keys unchanged.

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

Yes. The linked issue gives a concrete CLI fixture, and current main's raw formatter slices make the dangling-surrogate failure clear from source inspection.

Is this the best way to solve the issue?

Yes. Updating the shared formatter is the narrowest maintainable fix because both affected table surfaces call it; caller-specific fixes or shortenText would duplicate behavior or lose the suffix semantics.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P2: This is a deterministic CLI display bugfix with limited blast radius in session table output.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix real CLI fixture output from Blacksmith Testbox plus an old-slice control showing the prior replacement-character failure.
  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix real CLI fixture output from Blacksmith Testbox plus an old-slice control showing the prior replacement-character failure.

Label justifications:

  • P2: This is a deterministic CLI display bugfix with limited blast radius in session table output.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix real CLI fixture output from Blacksmith Testbox plus an old-slice control showing the prior replacement-character failure.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix real CLI fixture output from Blacksmith Testbox plus an old-slice control showing the prior replacement-character failure.
Evidence reviewed

PR surface:

Source +1, Tests +14. Total +15 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 2 1 +1
Tests 1 14 0 +14
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 16 1 +15

What I checked:

  • Repository policy read: Root AGENTS.md was read fully; its PR review, protected-label, source-history, and best-fix guidance shaped this review. (AGENTS.md:28, cd50d4983b2f)
  • Current-main bug source: Current main still builds truncated session keys with raw key.slice(0, head) and key.slice(-6), which can return dangling surrogate halves at the reported boundaries. (src/commands/sessions-table.ts:111, cd50d4983b2f)
  • Shared caller coverage: Both openclaw sessions and cleanup dry-run route their Key column through formatSessionKeyCell, so the shared formatter is the right owner boundary. (src/commands/sessions.ts:537, cd50d4983b2f)
  • Safe helper contract: sliceUtf16Safe adjusts slice edges away from dangling surrogate pairs, and truncateUtf16Safe delegates to it; the package exports the ./utf16-slice subpath used by this PR. (packages/normalization-core/src/utf16-slice.ts:16, cd50d4983b2f)
  • PR implementation: The PR head replaces raw prefix and suffix slicing with truncateUtf16Safe(key, head) and sliceUtf16Safe(key, -6) while preserving the shared formatter shape. (src/commands/sessions-table.ts:112, f4fa56da1449)
  • Regression test: The added test places emoji across both retained boundaries and asserts the fixed-width cell renders without replacement characters while preserving the padded cell width. (src/commands/sessions-table.test.ts:6, f4fa56da1449)

Likely related people:

  • steipete: Current checkout history blames the shared session-table formatter and existing UTF-16 helper to Peter Steinberger's recent squash commit, and the proposed fix is authored by the same GitHub user. (role: introduced behavior and recent area contributor; confidence: high; commits: 70bb770e6b3a, f4fa56da1449; files: src/commands/sessions-table.ts, src/commands/sessions.ts, src/commands/sessions-cleanup.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. labels Jul 9, 2026
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Land-ready proof for exact head f4fa56da1449596c9f23a6aacf47f06fff7c5737:

  • Best-fix check: the shared session-table formatter is the single owner used by both the normal list and cleanup dry-run. Canonical UTF-16-safe helpers preserve the existing prefix/suffix budgets without changing JSON or persisted keys.
  • Blacksmith Testbox tbx_01kx3w5bfyc64dwp5g3rj4cr7y (brisk-crayfish):
    • pnpm test src/commands/sessions-table.test.ts — 1/1 passed.
    • pnpm test src/commands/sessions-table.test.ts src/commands/sessions.test.ts src/commands/sessions-cleanup.test.ts — 26/26 passed.
    • pnpm check:changed — passed for core + coreTests, including tsgo, lint, and repository guards.
  • Negative control with the prior raw slices produced aaaaaaaaaaaaaaa�...�zzzzz and detected dangling UTF-16 units at indices 15 and 19.
  • Live CLI after the fix rendered aaaaaaaaaaaaaaa...zzzzz in both session listing and cleanup preview.
  • Independent source-blind validation passed 7/7 clauses/probes: both tables, full-key JSON, short/varied/ASCII keys, forbidden-surrogate scan, and byte-identical retry.
  • Fresh autoreview: clean, no accepted/actionable findings; confidence 0.99.
  • Exact-head CI and real behavior proof passed. Verification snapshot: 65 successful checks, 36 skipped, zero failed or pending.

Known proof gaps: none within the issue scope. Grapheme/display-width alignment is intentionally separate from preventing malformed UTF-16 output.

@steipete
steipete merged commit 8c78e1c into main Jul 9, 2026
166 of 173 checks passed
@steipete
steipete deleted the codex/sessions-table-utf16 branch July 9, 2026 17:17
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

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

Labels

commands Command implementations maintainer Maintainer-authored PR P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Session tables corrupt Unicode keys at truncation boundaries

1 participant