Skip to content

fix(python3): preserve user source whitespace and line numbers - #431

Open
josephbajor wants to merge 1 commit into
vercel-labs:mainfrom
josephbajor:codex/fix-python-source-preservation
Open

josephbajor wants to merge 1 commit into
vercel-labs:mainfrom
josephbajor:codex/fix-python-source-preservation

Conversation

@josephbajor

Copy link
Copy Markdown

The Python worker prepends four spaces to every line of user code to put it inside a try block. That also changes the contents of multiline strings. Valid Python can silently produce different data, or generate code that then fails with IndentationError. This happens after shell parsing, so changing heredoc quoting does not solve it; stdin, -c, and script files are all affected.

Reproduced against upstream main at 062ce005c0a7676163852fb6f0c8590cbdaa1d45 (just-bash 3.4.2).

Reproduce the silent data corruption

From a source checkout, run pnpm install --frozen-lockfile && pnpm --filter just-bash build. Save this as repro-python.mjs in the repository root, then run node repro-python.mjs:

import { Bash } from './packages/just-bash/dist/bundle/index.js';
const bash = new Bash({ python: true });
const source = 'text = """alpha\nbeta\n"""\nprint(repr(text))';
const result = await bash.exec(`python3 - <<'PY'\n${source}\nPY`);
console.log(JSON.stringify(result.stdout));
console.log('exitCode:', result.exitCode);
console.log('stderr:', result.stderr);

Current upstream:

"'alpha\\n    beta\\n    '\n"
exitCode: 0
stderr:

With this fix:

"'alpha\\nbeta\\n'\n"
exitCode: 0
stderr:

The corresponding native Python program is:

text = """alpha
beta
"""
print(repr(text))

It prints 'alpha\nbeta\n'. No spaces occur before beta or the closing delimiter in the supplied source.

How this becomes an indentation error

Replacing source in the JS example with the following program reproduces the agent-facing failure:

source = """def answer():
    return 42
print(answer())
"""
exec(source)

Native Python and this branch print 42 and exit 0. Current upstream adds four spaces before print(answer()) inside the generated string, producing an IndentationError and exit 1. The regression suite covers this exact case in all three input modes.

Change

Keep the setup and exception handler, but pass the unmodified user text to compile(..., "exec") and execute that code object in the existing module globals. This executes the user code once in the same interpreter. Cache the original text in linecache and supply the user script name (or <stdin> / <string>) so tracebacks show original source lines rather than wrapper offsets.

This also lets valid module-level from __future__ imports work. Ordinary globals, function access to those globals, and sys.exit() behavior are covered by regression tests. Actual indentation errors still fail; their diagnostics now point at the original user line.

Changes are limited to the TypeScript worker source, regression tests, and a changeset. No compiled JavaScript is committed.

Verify

# Rebuild the worker whenever its TypeScript changes.
pnpm --filter just-bash build
pnpm --filter just-bash test:wasm src/commands/python3/python3.source-preservation.test.ts
pnpm test:wasm
  • 27 new cases: multiline literals, generated Python, escaped physical newlines, tabs/spaces, future imports, globals, exit status, and diagnostic filenames/lines across stdin, -c, and files. 21 fail on unmodified upstream.
  • All WASM tests: 738 passed, 2 skipped on Node 24.10.0. The 27 new cases also pass on Node 22.21.0.
  • Build, typecheck, lint, knip, worker synchronization, and all 17 distribution tests passed.
  • Full test:run: 15,610 passed, 98 skipped; the same six existing filesystem tests fail as on unmodified upstream in this macOS environment. They concern symlinks/special mode bits and do not exercise Python.

@vercel

vercel Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

@josephbajor is attempting to deploy a commit to the Vercel Labs Team on Vercel.

A member of the Team first needs to authorize it.

@auto-maintain

auto-maintain Bot commented Sep 14, 2026

Copy link
Copy Markdown

🤖 auto-maintain review

Automated, advisory triage for @josephbajor's PR. Facts below are read from the GitHub API.

Check Result
Author's merged PRs (this repo) 0 — ⚠️ first-time contributor
Account established ✅ (age 3404d · 2 followers · 17 public repos)
Commits signed/verified ⚠️ 0/1
Changeset included ✅ (.changeset/plain-python-source.md)

Review panel: 🟡 medium highest severity

just-bash maintainer code review: 🟡 medium

The source-preservation fix works, but it regresses syntax-error diagnostics by exposing the internal wrapper traceback.

  • packages/just-bash/src/commands/python3/worker.ts:1451 — Syntax errors are now caught and rendered with `traceback.print_exc()`, which adds a misleading `Traceback` section and an internal `/tmp/_jb_script.py` frame before the user diagnostic. The new indentation test masks this by checking only from the last `File` line; handle `SyntaxError` separately or omit the wrapper frame.

General code review: 🟢 low

No actionable defects found in the complete diff.

Adversarial security: 🟢 low

No actionable adversarial security issues found in the complete diff.

Adversarial security (second opinion): 🟡 medium

No security regression: the user source is safely JSON-escaped into the Python literal and the sandbox surface is unchanged, but the new linecache entry uses str.splitlines() which mis-maps line numbers for sources containing form feeds/U+2028, and syntax errors now emit a spurious wrapper traceback frame leaking /tmp/_jb_script.py.

  • packages/just-bash/src/commands/python3/worker.ts:1445 — `_jb_source.splitlines(True)` splits on more characters than the Python tokenizer does (\v, \f, \x1c-\x1e, \x85, U+2028, U+2029). A source containing any of these — e.g. a form feed page separator, legal in Python source, or U+2028 inside a string literal — desyncs the linecache line list from compile()'s line numbering, so tracebacks print the wrong source text for every line after it. That is the exact failure this PR is meant to fix. Split on \n only (e.g. re-join from `_jb_source.split("\n")`) to match the compiler.
  • packages/just-bash/src/commands/python3/worker.ts:1449 — Moving compilation to runtime makes SyntaxError/IndentationError fall into the wrapper's `except Exception` handler, so `traceback.print_exc()` now prefixes every user syntax error with `Traceback (most recent call last):` and an internal frame `File "/tmp/_jb_script.py", line <wrapper line>, in <module>`. Real python3 emits only the bare SyntaxError block, so stderr now diverges from bash/python and exposes the sandbox wrapper path and a meaningless line number to callers parsing python3 output. The new test at python3.source-preservation.test.ts:101 sidesteps this by slicing stderr from the last ` File ` instead of asserting it in full. Catch SyntaxError from compile() separately and print only the user-facing block.

Standard Bash and host portability: 🟢 low

No actionable Bash or host-portability issues found.

Posted by auto-maintain. This automated code review is advisory; a human maintainer makes the call.

This branch has not been deployed

No deployments
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.

1 participant