Repository navigation
fix: decode exec and clone output once per stream so split UTF-8 survives - #1281
Conversation
|
No findings in the scoped PR diff. The description documents sufficient testing: type checks, lint, focused unit tests, and a representative built-CLI run reproducing the split UTF-8 issue with before/after output. The stand-in OpenCode verification satisfies the real-CLI requirement; a full provider × agent matrix is not required. Review was read-only. I did not run, build, or install anything from the PR. |
SaulMoro
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
@arnavsingh0418 Approving. We built this head and main and ran teamai session migrate ses_utf8 -s opencode -t claude-code against a stand-in opencode that splits a multi-byte character across two writes. That path reaches execCommand through the OpenCode adapter's session list.
main:ses_utf8_a 修复���话导出 café 😀- this head:
ses_utf8_a 修复会话导出 café 😀
The three new unit tests fail on main for the stated reason and pass here.
[P2 non-blocking] captureTail in src/utils/exec.ts:118 still decodes each chunk with chunk.toString('utf8'), so the tail it returns for an error line can carry the same replacement characters. The description says it was left out. It affects one log line, so it can be a follow-up.
The src/clone.ts change is covered by the mocked unit test only. We didn't run a real clone through it.
#1208 also edits src/clone.ts, next to this hunk. This PR owns the decoder lines and #1208 owns the token redaction in the timeout message. The two merge cleanly.
Summary
execCommand(src/utils/exec.ts) andrunCommand(src/clone.ts) decoded every stdout/stderr chunk on its own withchunk.toString(), so a multi-byte UTF-8 character split across two pipe reads became U+FFFD in both the captured string and thestreamoutput. Both now keep oneStringDecoder('utf8')per stream and flush it on close, the same approach assrc/workflow/local-tool-adapter.ts:349and #1275.Type of Change
Test Plan
npx tsc --noEmitpassesnpm run lintpassesnpx vitest run src/__tests__/exec.test.ts src/__tests__/clone.test.tspasses (7 tests)New tests split a character across two chunks (exec: stdout, stderr, and the
streamwrite path; clone: a branch name fromrev-parse --abbrev-ref). Onmainthey fail with\ufffdin the output; with this change they pass.Real-CLI verification (
npm run build, then the built CLI with a stand-in OpenCode 2 on PATH whosesession list --format jsonwrites its JSON in two pipe writes split inside会, in a sandboxed HOME/XDG_DATA_HOME).teamai session migrate ses_utf8 -s opencode -t claude-code --cwd <ws>hits the ambiguous-prefix path, which prints the listed titles:Before (main):
After:
Related Issues
Fixes #1279
Notes for Reviewers
clone.tsin the same change so the two copies do not diverge, as the issue asked.streampath writes the decoder's remainder on close, so streamed output matches the captured string.captureTailinexec.tsdecodes per chunk too, but it only keeps a whitespace-collapsed tail for one log line, so I left it out to keep this change surgical. Happy to include it if you'd like.npx vitest runon Windows; failures there are timeouts and symlink cases (local-agent,workflow-cache,rules,hook-handlers, …) in files that don't importexec.tsorclone.ts. Relying on CI (Linux) for the full suite.