Skip to content

fix(batch): export NODE_OPTIONS as a statement so compound shell commands work (#925)#934

Open
hakanbaysal wants to merge 1 commit into
mksglu:nextfrom
hakanbaysal:fix/shell-compound-nodeopts-925
Open

fix(batch): export NODE_OPTIONS as a statement so compound shell commands work (#925)#934
hakanbaysal wants to merge 1 commit into
mksglu:nextfrom
hakanbaysal:fix/shell-compound-nodeopts-925

Conversation

@hakanbaysal

Copy link
Copy Markdown

What / Why / How

Fixes #925.

Batch shell commands that start with a compound construct (if, for,
while, a (...) subshell, or a { ...; } group) failed with
syntax error near unexpected token. The failed run was then indexed, so
ctx_search surfaced the syntax error instead of the intended output.

Root cause: the POSIX branch of buildBatchNodeOptionsPrefix emitted an inline
NODE_OPTIONS='...' <command> assignment. That form is only valid before a
simple command, so prefixing if .../for .../(...) produced invalid
shell.

Fix: emit a standalone export NODE_OPTIONS='...'; statement instead. This
mirrors the PowerShell ($env:...; ) and cmd (set "..." && ) branches, which
already use a separate statement, and composes with any following command. As a
bonus, the exported value now also reaches node invocations inside compound
constructs (e.g. a for loop body), which the inline form could not do.

Affected platforms

  • All platforms

Test plan

  • Real-shell verification: the old NODE_OPTIONS='...' if ...; then ...; fi
    errors at then; the new export NODE_OPTIONS='...'; if ... runs, as do
    for loops and (...) | sed subshell pipes. Confirmed export propagates
    NODE_OPTIONS to child node processes.
  • Updated the POSIX prefix unit test and added coverage asserting the prefix
    composes with the three compound examples from the issue.
  • npx vitest run tests/core/server.test.ts -t "buildBatchNodeOptionsPrefix"
    → 4 passed; runBatchCommands suite → 25 passed.
  • npx tsc --noEmit → clean.

Checklist

  • Tests added/updated (TDD: red → green)
  • npm test passes (server suite; full build requires Node >=22.5)
  • npm run typecheck passes
  • Docs updated if needed — n/a
  • No Windows path regressions (Windows/PowerShell/cmd branches unchanged)
  • Targets next branch

…ands work (mksglu#925)

The POSIX branch of buildBatchNodeOptionsPrefix emitted an inline
`NODE_OPTIONS='...' <command>` assignment. That form is only valid before a
*simple* command, so any batch command starting with a compound construct
(`if`, `for`, `while`, a `(...)` subshell, or a `{ ...; }` group) was rewritten
into invalid shell and failed with `syntax error near unexpected token`. The
failed run was then indexed, surfacing the syntax error instead of the intended
output.

Emit a standalone `export NODE_OPTIONS='...'; ` statement instead. This mirrors
the PowerShell (`$env:...; `) and cmd (`set "..." && `) branches, which already
use a separate statement, and composes with any following command. As a bonus
the exported value now also reaches `node` invocations *inside* compound
constructs (e.g. a `for` loop body), which the inline form could not do.

Updates the POSIX prefix test and adds real-compound-command coverage.
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