fix(commitments): keep table columns aligned when an id or scope is truncated#95923
Conversation
|
@clawsweeper review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
Codex review: needs maintainer review before merge. Reviewed June 24, 2026, 2:27 AM ET / 06:27 UTC. Summary PR surface: Source 0, Tests +95. Total +95 across 2 files. Reproducibility: yes. Source inspection shows current Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land one narrow commitments-table fix with command-path regression coverage; this PR is a viable candidate unless maintainers choose another open candidate. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current Is this the best way to solve the issue? Yes. Replacing the three-character suffix with the existing one-character ellipsis convention fixes the width contract at the source; a shared helper can remain separate cleanup. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a96418c65f8f. Label changesLabel justifications:
Evidence reviewedPR surface: Source 0, Tests +95. Total +95 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
8abd077 to
baddfad
Compare
0f249f6 to
867c8e0
Compare
|
Merged via squash.
|
Many Thanks, @vincentkoc 🚀 |
Closes #95921
What Problem This Solves
Fixes an issue where operators running
openclaw commitmentssee the table'scolumns drift out of alignment whenever a commitment's ID or Scope is long
enough to be truncated. The header row and every short row line up, but a row with
a truncated cell pushes
Status,Kind,Due,Scope, andSuggested texttwocharacters to the right, so the list no longer reads as a clean fixed-width table.
The columns are sized with
padEnd(16)/padEnd(28)etc., which only hold theirwidth if each cell is at most that many characters. The
truncate()helper breaksthat contract: it reserves one character for an ellipsis (
slice(0, maxChars - 1))but then appends the three-character ASCII
"...", so a truncated value ismaxChars + 2characters wide and overflows its column by two.Why This Change Was Made
truncate()now appends the single-character ellipsis…(U+2026) instead of thethree-character
.... With one reserved character and a one-character ellipsis, atruncated value is exactly
maxCharswide, sopadEndholds the column and thetable stays aligned.
This also brings
commitments.tsin line with the rest of the CLI: every othertruncation helper in the codebase already uses the one-character
…(
src/commands/flows.ts,src/commands/tasks.ts,src/commands/onboard-skills.ts).commitments.tswas the lone outlier.Scope is intentionally minimal: only truncated cells change (their tail goes from
...to…); untruncated output is byte-for-byte identical.User Impact
openclaw commitmentsrenders a correctly aligned table again, even when an ID orscope is long enough to be shortened. Truncated cells end in
…(matching the restof the CLI) instead of
.... There is no behavior change for commitments whosefields fit within their columns.
Evidence
Real
openclaw commitmentsrun (before → after). Seeded a real commitmentstore with two pending commitments - one short, one whose id (29 chars) and scope
(>28 chars) get truncated, and ran the actual CLI (
pnpm openclaw commitments,built from source) against it. Node 24 / pnpm 11.2.2.
Before (current
...):The truncated cell is exactly 16 chars and every column realigns.
Regression tests (
src/commands/commitments.test.ts). Added four focusedcases that exercise the real
commitmentsListCommandrendering path (the datastore and config are provided the same way the sibling tests in that file do):
a truncated id and scope together - asserts the truncated id cell stays
within its 16-char column and
Status/Kindalign with the header;a truncated scope only (short id) - asserts the 28-char Scope cell holds
its width and the
Suggested textcolumn still starts under its header;the exact-width boundary -- a 16-char id is rendered in full with no
ellipsis (guards against over-truncating at
length == maxChars);the one-past-boundary - a 17-char id truncates to 15 chars plus one
ellipsis (16 wide), where the old
...produced 18 and overflowed.With the fix:
8 passed (8)(5 pre-existing + 4 new).Reverting the one-character change to the old
...: 3 of the new alignmentcases fail (e.g.
expected 'cm_0123456789ab.' to be 'cm_0123456789ab…'); theexact-width boundary case still passes (it never truncates), and the
pre-existing tests still pass.
Lint/format on the two changed files:
oxfmt --check→ "All matched filesuse the correct format";
oxlint→ "Found 0 warnings and 0 errors."