Skip to content

fix(shell): truncate metadata preview by bytes#29297

Open
YOMXXX wants to merge 3 commits into
anomalyco:devfrom
YOMXXX:fix/shell-preview-byte-truncation
Open

fix(shell): truncate metadata preview by bytes#29297
YOMXXX wants to merge 3 commits into
anomalyco:devfrom
YOMXXX:fix/shell-preview-byte-truncation

Conversation

@YOMXXX
Copy link
Copy Markdown

@YOMXXX YOMXXX commented May 26, 2026

Issue for this PR

Closes #29291

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Shell tool metadata previews were checking text.length, which counts UTF-16 code units rather than UTF-8 bytes. Multi-byte output could exceed the metadata byte limit without being preview-truncated, and slicing by code units could start on an invalid surrogate boundary.

This PR makes metadata preview truncation use UTF-8 byte length, trims from a valid byte boundary, and reuses the same byte-tail helper in the existing shell output tail path.

How did you verify your code works?

  • bun test test/tool/shell.test.ts -t "metadata output"
  • bun test test/tool/shell.test.ts
  • bun typecheck
  • PATH="$HOME/.bun/bin:$PATH" .husky/pre-push

Screenshots / recordings

Not applicable; shell metadata behavior only.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@github-actions
Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

Potential Duplicate Found

PR #29288: fix(shell): preview Unicode truncation uses byte length instead of char length

Why it's related: Both PRs address the same core issue—using UTF-8 byte length instead of character/code unit length for shell tool metadata preview truncation. PR #29288 appears to cover the same fix, likely related to issue #29291 that this PR is closing.

@YOMXXX
Copy link
Copy Markdown
Author

YOMXXX commented May 26, 2026

Checked #29288. It addresses the same area, but its regression test only checks that the preview has no replacement character. With the current broken implementation, the CJK case can avoid truncation entirely because text.length stays under 30,000, so that test would not catch the byte-length bug from #29291.

This PR adds assertions that metadata preview actually truncates when UTF-8 bytes exceed the limit, keeps the retained tail within the byte limit, and does not start on an unpaired low surrogate for emoji output. It also reuses the same byte-tail helper in the existing shell tail path.

@nexxeln
Copy link
Copy Markdown
Member

nexxeln commented May 26, 2026

tests failing, could you pls fix?

@YOMXXX
Copy link
Copy Markdown
Author

YOMXXX commented May 26, 2026

CI update after the latest push:

  • Fixed the Windows-only unit failure by making the UTF-8 metadata preview coverage exercise the pure preview helper instead of shell-dependent Unicode command output.
  • Local verification passed from packages/opencode: bun test test/tool/shell.test.ts and bun typecheck.
  • Root pre-push typecheck passed.
  • The new test, typecheck, and nix-eval workflow runs are currently action_required and need maintainer approval before they will execute.

The old app e2e failures match failures currently seen on dev, so they do not appear to be introduced by this shell PR.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

shell tool preview truncation uses char length instead of byte length

2 participants