Skip to content

fix(commitments): keep table columns aligned when an id or scope is truncated#95923

Merged
vincentkoc merged 1 commit into
openclaw:mainfrom
parveshsaini:parveshsaini/fix/commitments-truncate-column-overflow
Jun 24, 2026
Merged

fix(commitments): keep table columns aligned when an id or scope is truncated#95923
vincentkoc merged 1 commit into
openclaw:mainfrom
parveshsaini:parveshsaini/fix/commitments-truncate-column-overflow

Conversation

@parveshsaini

Copy link
Copy Markdown
Contributor

Closes #95921

What Problem This Solves

Fixes an issue where operators running openclaw commitments see the table's
columns 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, and Suggested text two
characters 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 their
width if each cell is at most that many characters. The truncate() helper breaks
that contract: it reserves one character for an ellipsis (slice(0, maxChars - 1))
but then appends the three-character ASCII "...", so a truncated value is
maxChars + 2 characters wide and overflows its column by two.

Why This Change Was Made

truncate() now appends the single-character ellipsis (U+2026) instead of the
three-character .... With one reserved character and a one-character ellipsis, a
truncated value is exactly maxChars wide, so padEnd holds the column and the
table stays aligned.

This also brings commitments.ts in line with the rest of the CLI: every other
truncation helper in the codebase already uses the one-character
(src/commands/flows.ts, src/commands/tasks.ts,
src/commands/onboard-skills.ts). commitments.ts was 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 commitments renders a correctly aligned table again, even when an ID or
scope is long enough to be shortened. Truncated cells end in (matching the rest
of the CLI) instead of .... There is no behavior change for commitments whose
fields fit within their columns.

Evidence

Real openclaw commitments run (before → after). Seeded a real commitment
store 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 ...):

ID               Status     Kind             Due                      Scope                        Suggested text
cm_short         pending    event_check_in   2026-06-30T01:28:12.894Z main/telegram/+15551234567   How did it go?
cm_abcdefghijkl… pending    event_check_in   2026-06-30T01:28:12.894Z averylongagentidentifier/te… How did it go?

The truncated cell is exactly 16 chars and every column realigns.

Regression tests (src/commands/commitments.test.ts). Added four focused
cases that exercise the real commitmentsListCommand rendering path (the data
store 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/Kind align with the header;

  • a truncated scope only (short id) - asserts the 28-char Scope cell holds
    its width and the Suggested text column 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 alignment
    cases fail (e.g. expected 'cm_0123456789ab.' to be 'cm_0123456789ab…'); the
    exact-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 files
use the correct format"; oxlint → "Found 0 warnings and 0 errors."

@openclaw-barnacle openclaw-barnacle Bot added commands Command implementations size: S labels Jun 23, 2026
@parveshsaini

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

@clawsweeper

clawsweeper Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@parveshsaini

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 24, 2026, 2:27 AM ET / 06:27 UTC.

Summary
The PR changes openclaw commitments text-table truncation to use a single-character ellipsis and adds command-output regression tests for long ID and Scope values.

PR surface: Source 0, Tests +95. Total +95 across 2 files.

Reproducibility: yes. Source inspection shows current truncate() can return a value wider than the padEnd column when ID or Scope is truncated.

Review metrics: 1 noteworthy metric.

  • Open same-root candidates: 3 open PRs including this PR. Maintainers should avoid merging duplicate fixes for the same one-line commitments table bug.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #95921
Summary: This PR is one candidate fix for the canonical commitments table truncation-width bug.

Members:

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • none.

Risk before merge

  • [P1] Three open PRs currently target the same linked commitments truncation issue, so maintainers should select one canonical fix and close the remaining candidates after a merge.

Maintainer options:

  1. Decide the mitigation before merge
    Land one narrow commitments-table fix with command-path regression coverage; this PR is a viable candidate unless maintainers choose another open candidate.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] The remaining action is maintainer selection among duplicate open candidate PRs, not an automated repair on this branch.

Security
Cleared: The diff only changes CLI string formatting and colocated command tests; no security or supply-chain surface is touched.

Review details

Best 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 truncate() can return a value wider than the padEnd column when ID or Scope is truncated.

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 changes

Label justifications:

  • P3: The PR addresses a low-risk CLI table readability bug with no data loss, security, crash, or message-delivery impact.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal output from a real openclaw commitments run plus focused regression test output for the changed command path.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from a real openclaw commitments run plus focused regression test output for the changed command path.
Evidence reviewed

PR surface:

Source 0, Tests +95. Total +95 across 2 files.

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

What I checked:

Likely related people:

  • jalehman: git blame and local file history point to commit 96cee6c as the current source provenance for src/commands/commitments.ts after the command extraction/refactor. (role: recent area contributor; confidence: medium; commits: 96cee6cb6442; files: src/commands/commitments.ts, src/commands/commitments.test.ts, src/cli/program/register.status-health-sessions.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: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jun 23, 2026
@vincentkoc
vincentkoc force-pushed the parveshsaini/fix/commitments-truncate-column-overflow branch from 8abd077 to baddfad Compare June 24, 2026 05:59
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation extensions: qa-lab extensions: openshell size: L and removed size: S labels Jun 24, 2026
@vincentkoc
vincentkoc force-pushed the parveshsaini/fix/commitments-truncate-column-overflow branch from 0f249f6 to 867c8e0 Compare June 24, 2026 07:09
@openclaw-barnacle openclaw-barnacle Bot removed docs Improvements or additions to documentation extensions: qa-lab extensions: openshell size: L labels Jun 24, 2026
@vincentkoc
vincentkoc merged commit 33afb1e into openclaw:main Jun 24, 2026
104 of 109 checks passed
@vincentkoc

Copy link
Copy Markdown
Member

Merged via squash.

@parveshsaini

Copy link
Copy Markdown
Contributor Author

Merged via squash.

Many Thanks, @vincentkoc 🚀

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 25, 2026
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commands Command implementations P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S 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]: openclaw commitments table misaligns - truncate() appends a 3-char "..." but reserves 1 char, overflowing padEnd columns by 2

2 participants