Skip to content

fix(workflow): preserve UTF-8 in command outputs - #1275

Merged
SaulMoro merged 1 commit into
Tencent:mainfrom
ydflow:fix/workflow-command-utf8
Oct 22, 2026
Merged

SaulMoro merged 1 commit into
Tencent:mainfrom
ydflow:fix/workflow-command-utf8

Conversation

@ydflow

@ydflow ydflow commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Summary

A local workflow command's UTF-8 character can cross stdout pipe chunks. startCommand decoded each Buffer independently, so the command could succeed with valid JSON while silently changing a string value before saving it and handing it to the next step.

Built-CLI reproduction, two command steps:

emitter stdout:       {"summary":"检查完成 café 😀"}
before, stored value: {"summary":"���查完成 café 😀"}
next step receives:   ���查完成 café 😀
workflow outcome:     succeeded

Use each pipe's setEncoding('utf8') so stdout and stderr keep independent decoder state across chunks and flush at EOF. After the fix, the original value reaches both steps' stored outputs and attempt result.json files. Failed-command stderr also retains the original diagnostic. The English and Chinese workflow guides state the encoding contract.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Test Plan

  • npm run build
  • npm run typecheck
  • npm run lint
  • git diff --check
  • npx vitest run src/__tests__/workflow-command-utf8.test.ts: 6 passed. The subprocess boundary emits individual bytes on interleaved stdout/stderr; the real driver and engine persist the result. Covers Chinese, accented text, emoji, ASCII, a stored failure diagnostic and an incomplete trailing sequence at EOF. Before the fix: 5 failed, ASCII control passed.
  • Real CLI after build: npx vitest run --config vitest.e2e.config.ts src/__tests__/e2e/workflow-command-utf8.test.ts --retry 0: 2 passed. Real Node commands split a UTF-8 character across two OS-pipe writes. Success checks the emitter's output, the next command's env-template input and output, both attempt result files and CLI exit 0. Failure checks the stored stderr diagnostic and CLI exit 1. Both cases fail on the pre-fix build with the damaged text above. No AI tool or live model is used.
  • Compatibility check: in a temporary worktree, combine this change, origin/main at d60e68f and fix(workflow): close the #1080 workspace gaps (#1246) #1251 at 3ef4e1884260306e2e733663d875096458f25503. The merge is clean; build, the same 6 regressions and 2 real-CLI cases pass there too. The published branch contains only this fix.
  • A completely green local suite. On Windows / Node 24.19.0 / Git 2.33.1:
    • The three-file driver/adapter/new-regression run completed with 91 passed, 30 failed, 3 skipped. All 30 observed failing names reproduce on pristine d60e68f: the unchanged driver file gives 66 passed / 23 failed / 3 skipped, and the unchanged adapter file gives 19 passed / 7 failed. No failing name in those two files differs between branch and baseline. These include POSIX shell/process-signal cases; this is not a green Windows suite.
    • A broad npx vitest run --maxWorkers=4 --bail=10 --reporter=json attempt stopped early, recording 107 passed, 7 failed, 12 pending. All seven observed failing names reproduce in unchanged local-agent.test.ts on pristine d60e68f. This is not a completed full-suite result.
    • One initial targeted run hit EPERM removing a temporary fixture directory during cleanup; the later six-case run passed with the same cleanup code and assertions. Its cause is not established; the failed attempt is retained in the local record.

The remaining platform and full-suite coverage is left to CI.

Related Issues

Follow-up to #1242, which fixed the separate prompt-tool stdout path in local-tool-adapter.ts. This fixes command steps in local-driver.ts, including stderr. No open PR was found for this path. #1251 changes workspace handling in the same module; compatibility was checked as above.

Notes for Reviewers

Runtime changes are limited to four added and two removed lines in startCommand. Its local stdout/stderr accumulators feed the close/error result, commandReport, the attempt result file and the engine's existing step state. No workflow schema, command-line option or dependency changes.

Door: two-way; reverting changes decoding behavior, with no migration.

Blast radius: local command stdout values and stderr diagnostics, including values consumed by later workflow steps.

Signed-off-by: ydflow <314143294+ydflow@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

No findings in the specified PR diff.

The testing record satisfies the review criteria: it documents a build followed by two real-CLI cases covering preserved UTF-8 output, downstream consumption, persisted results, and failure diagnostics. Baseline failures and remaining CI coverage are clearly disclosed.

Review was read-only; I did not run, build, or install anything from the PR.

@SaulMoro SaulMoro left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

@ydflow Approving. We built this head and main (d60e68f5) and ran a two-step local workflow (init --self, pull, workflow start --driver local). The first step emits {"summary":"检查完成 café 😀"}.

main this head
Success run: exit, outcome 0, succeeded 0, succeeded
Stored output, next step's input, result.json ���查完成 café 😀 检查完成 café 😀
Failing run: stored failure message ���行失败 café 😀 执行失败 café 😀

[P3 nit] The new e2e test sets itself up through internal modules (resolveProjectDataHome, syncWorkflowDefinitions) and a hand-written config.yaml. The neighbouring workflow e2e tests run init --self and pull through the CLI.

Outside this PR: src/utils/exec.ts:48-57 decodes each chunk with chunk.toString() the same way, and with stream set it also prints the broken text to the terminal. The same setEncoding('utf8') fix would apply there in a follow-up.

#1251 edits the startCommand call site for the working directory. This PR changes only the stdout and stderr decoding, so the two don't conflict.

@SaulMoro
SaulMoro merged commit 87b546a into Tencent:main Oct 22, 2026
17 checks passed
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.

3 participants