Skip to content

fix(worker-shell): stop double-encoding non-ASCII bytes in redirected output - #287

Open
github-actions[bot] wants to merge 1 commit into
mainfrom
gardener/issue-286
Open

github-actions[bot] wants to merge 1 commit into
mainfrom
gardener/issue-286

Conversation

@github-actions

@github-actions github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #286

Cause

just-bash passes redirected output from cat, heredocs and tee to writeFile/appendFile as a byte string (one char per byte) with encoding "binary". WorkspaceFsAdapter.writeFile and appendFile ignored that argument (packages/computer/src/backends/worker-shell/adapter.ts:153-179), so the store UTF-8-encoded the string again and every byte >= 0x80 became two bytes (é stored as c3 83 c2 a9).

Change

A small binaryContent helper converts strings tagged "binary" or "latin1" (string or {encoding} option) into bytes, one byte per char. Both methods use it. Untagged and utf8 strings behave as before. Adds two tests to adapter.test.ts and a patch changeset.

Why this option

It fixes the problem at the one place that misreads the encoding, adds no public API, and leaves text writes (printf, echo) untouched.

Alternatives considered

  • Patching just-bash to emit UTF-8 text: it's a dependency we don't own, and the "binary" contract is correct.
  • Latin-1 decoding only in appendFile: leaves writeFile broken.

Checks

  • The two new tests fail on main and pass with the change.
  • adapter.test.ts: 34 tests pass.
  • npm run typecheck in packages/computer passes.

Commits pushed by this task don't start CI. A maintainer can mention @ask-gardener on this pull request to switch to an alternative or ask for changes.


Devin Review

Fixes #286

Gardener-Operation: op_76514dadb79ba4e92db823cd9e0b8e5c8ac9482ac492a6bd31f06eae26e993a5:d97657717d579f3ff754f537d5cf85ad47258678bdcdf82478509989615e2b02
@changeset-bot

changeset-bot Bot commented Oct 29, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 3d76f6a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@cloudflare/computer Patch
@cloudflare/dofs Patch
@cloudflare/computer-rpc Patch
@cloudflare/computerd Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

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

Labels

allow-pr Allow a PR to remain open.

0 participants