Skip to content

fix: keep the rest of the command line when expanding a wildcard - #603

Open
webpro wants to merge 3 commits into
open-cli-tools:mainfrom
webpro:fix/expand-wildcard-preserve-command-line
Open

fix: keep the rest of the command line when expanding a wildcard#603
webpro wants to merge 3 commits into
open-cli-tools:mainfrom
webpro:fix/expand-wildcard-preserve-command-line

Conversation

@webpro

@webpro webpro commented Jul 27, 2026

Copy link
Copy Markdown

The problem

ExpandWildcard finds the runner invocation with a regex and rebuilds the command from
the captured pieces:

/((?:npm|yarn|pnpm|bun) run|node --run|deno task) (\S+)([^&]*)/
...
command: `${command} ${script}${args}`

([^&]*) stops at the first &, and nothing captures what follows, so it is dropped.
The pattern is also unanchored, so it matches inside quoted text.

Three things go wrong, all reproducible with the published package:

A chained command is silently discarded.

$ concurrently -n a "npm run build:* && echo DONE"
[aapp] APP
[alib] LIB
       # DONE never runs. Exit code 0, no warning.

$ concurrently -n b "npm run build:app && echo DONE"   # no wildcard, for contrast
[b] APP
[b] DONE

An & inside a quoted argument truncates the command mid-string.

$ concurrently -n c 'npm run build:* -- --grep "a & b"'
[capp] /bin/sh: -c: line 0: unexpected EOF while looking for matching `"'
[capp] npm run build:app -- --grep "a  exited with code 2

A runner named inside a quoted argument is expanded as if it were an invocation.
echo "npm run build:*" matches the pattern, finds no script ending in a quote, and
returns no commands at all, so the command disappears rather than running.

The change

Locate the invocation in the parsed command rather than in the raw text, then substitute the script using the source span of the glob:

commandInfo.command.slice(0, runner.glob.pos) + script + commandInfo.command.slice(runner.glob.end)

Every other byte of the command line is preserved by construction, which is what fixes all three cases at once rather than one at a time. The runner lookup walks the parsed command, so a runner name inside a quoted argument is not a runner invocation and an invocation after cd app && still is.

The (!...) omission syntax needs no special handling: it is extglob, and the parser reads watch-*(!js) as a single word.

About the dependency

This adds unbash, a zero dependency synchronous Bash parser with no WASM and no async initialisation. I wrote it. I also wrote knip, which depends on it, so it already runs against a large amount of real world shell: knip is at about 11.7M downloads a week and unbash at about 8.5M.

Worth noting given the two shell-quote advisory bumps in #591 and #599: shell-quote stays, because expand-arguments.ts uses its quote(), which is a different job from parsing. This PR does not touch that.

Verification

pnpm vitest --project unit   635 pass, 0 fail   (631 on main, +4 regressions)
pnpm lint                    exit 0
pnpm typecheck               exit 0
pnpm build                   exit 0

Both reproductions above were confirmed against the built binary before and after.

The wildcard expander matched the runner invocation with

    /((?:npm|yarn|pnpm|bun) run|node --run|deno task) (\S+)([^&]*)/

and rebuilt the command as `${command} ${script}${args}`. Everything from
the first `&` onward was captured by nothing and dropped, so

    concurrently 'npm run build:* && echo done'

ran the two builds and silently discarded `&& echo done`, exiting 0. The
same truncation cut

    concurrently 'npm run build:* -- --grep "a & b"'

mid-string, leaving `/bin/sh` with an unterminated quote. The pattern is
also unanchored, so `echo "npm run build:*"` was expanded as if it were a
runner invocation and, finding no script ending in a quote, returned no
commands at all.

Locate the invocation in the parsed command instead, and substitute the
script in place using the source span of the glob, so every other byte of
the command line is preserved. concurrently's own `(!...)` omission
syntax is extglob, which the parser reads as one word, so it needs no
special handling.
@coveralls

coveralls commented Jul 27, 2026

Copy link
Copy Markdown

Coverage Status

coverage: 98.738% (-0.09%) from 98.83% — webpro:fix/expand-wildcard-preserve-command-line into open-cli-tools:main

@gustavohenke
gustavohenke self-requested a review August 2, 2026 08:49

@gustavohenke gustavohenke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for finding and fixing this bug.

One aspect that's not touched on here is what's the Windows support.
I see unbash states that powershell/cmd are unsupported, but what would happen if their syntax went through - does it throw, or do we get some partial structure that we can work with? Is some sort of fallback handling necessary?

I also added some comments around cleaning up the code.

Comment thread lib/command-parser/expand-wildcard.ts Outdated
Comment on lines +23 to +24
type Word = { value: string; pos: number; end: number };
type Runner = { command: string; glob: Word };

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are these not exposed from unbash?

Comment thread lib/command-parser/expand-wildcard.ts Outdated
Comment on lines +33 to +38
function findRunner(node: unknown): Runner | undefined {
if (!node || typeof node !== 'object') {
return undefined;
}

const candidate = node as { type?: string; name?: Word; suffix?: Word[] };

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ditto here; are there types exposed from unbash that'd lead to cleaner code?

@webpro

webpro commented Sep 8, 2026

Copy link
Copy Markdown
Author

Thanks for finding and fixing this bug.

One aspect that's not touched on here is what's the Windows support. I see unbash states that powershell/cmd are unsupported, but what would happen if their syntax went through - does it throw, or do we get some partial structure that we can work with? Is some sort of fallback handling necessary?

Good point, and a bit of a rabbit hole. Good learnings for positioning unbash. Overall, this pull request fixes real issues (in environments that use it the most?), but also introduces additional complexity. Below the line is the full AI-generated story. Happy to keep iterating on this PR, but this hopefully explains enough re. responsibilities of concurrently vs unbash and first decide whether it's worth pursuing getting this merged.


The Windows concern is handled using the existing resolveShell result. Its precedence stays the same: explicit shell option, npm_config_script_shell, then the platform default. createSpawn exposes that already-resolved value to ExpandWildcard, which uses unbash for bash, sh, dash, and ash executable names, including .exe variants. CMD, PowerShell, zsh, fish, and unknown shells use the existing regex without sending their command text through unbash. A custom spawn without shell metadata also keeps the old behavior. This follows the configured execution shell, so an explicit Bash on Windows can use the parser too.

The additional name check is needed because detectShellKind classifies everything outside CMD and PowerShell as posix for spawning with -c. The posix category describes how to invoke the shell; choosing a parser requires knowing which shell will interpret the command. Shell resolution itself is unchanged.

For direct runner commands that parse without errors on these shells, the PR identifies the script argument and replaces just that word. Compared with the published implementation:

  • npm run build:* && echo done keeps && echo done in each expanded command. Leading commands and pipelines are preserved too. The entire command line runs once per matched script, so surrounding commands repeat for each match.
  • npm run build:* -- --grep "a & b" keeps the quoted argument intact. The old recognizer truncates it at the ampersand, leaving a broken quote.
  • echo "npm run build:*" stays an echo command. The old recognizer treats the quoted text as a runner invocation and can remove the command when it finds no matching script.
  • npm run "build:*" can expand script names containing spaces, apostrophes, or dollar signs. Each replacement is quoted as one literal argument through unbash/quote. This avoids reconstructing the whole command and adds no shell-quote usage.

Unbash owns Bash parsing, source ranges, and diagnostics. Concurrently owns the choice of parser, supported runner discovery, wildcard and omission matching, and what to do with diagnostics. Unbash already returns an AST alongside reported parse errors; this integration falls back to the old recognizer when errors are present instead of transforming that recovered AST. Confirmed Bash parsing defects belong in unbash.

The README comparison shows unbash leading the measured parse-throughput benchmarks and supporting Bash constructs that bash-parser misses, including extglobs and process substitution. Its synchronous, zero-dependency API exposes typed command words and source ranges directly. tree-sitter-bash needs native or WASM integration and traversal of its concrete syntax tree; sh-syntax adds dialect support through WASM. For this PR, I would choose unbash or retain the existing expander.

There are several costs and limits to that approach:

  • It adds a runtime dependency and makes the Bash path depend on unbash's AST, source ranges, and quoting behavior. The shell gate does not remove that dependency on other platforms. A parser update that changes diagnostics can also change which inputs fall back. shell-quote remains for additional-argument expansion.
  • Two recognizers remain. The runner list must stay consistent between the legacy regex and the typed lookup. Script discovery, omission filtering, matching, naming, and caching are shared, so those behaviors still have one implementation.
  • The source-preservation fix only applies to the admitted shells and direct runner commands that parse without errors. Other shells keep the published behavior, including dropped prefixes and truncation at &. Runner words used as arguments to wrappers such as npx or cross-env also use the legacy path, which can still discard the wrapper itself.
  • Omission filters are concurrently syntax and are not always valid Bash. An unquoted fixed-suffix form such as test:*-unit(!slow) falls back and retains the old tail handling. Quoting the whole pattern allows both omission filtering and source preservation, as shown below.
  • Bash behavior changes outside ordinary runner commands too. The traversal follows command lists, chains, and pipelines, but leaves function bodies, substitutions, and subshells untouched. For example, (npm run build:* && echo done) previously expanded while losing the subshell and tail; it now passes through unchanged. Existing uses that relied on that textual expansion can therefore stop expanding. Only the first eligible runner is expanded, rather than every wildcard in a command line.
concurrently --shell sh 'npm run "test:*-unit(!slow)" && echo done'

On the type comments: Node, Script, and Word are exported by unbash. The traversal now imports them directly and narrows on node.type; the local type copies and unknown/object casts are gone.

The 40 common expansion cases run against both paths with explicit expected outputs. Separate cases cover shell selection, source preservation, quoting, omissions, wrappers, and nested invocations. All 726 unit and smoke tests pass, including the build and CJS/ESM checks. Another 126 legacy comparisons match published 10.0.5 exactly, and CLI checks exercise the command-chain and quoting behavior. CMD and PowerShell coverage checks expansion and spawn arguments; native Windows execution was not tested locally.

@webpro
webpro force-pushed the fix/expand-wildcard-preserve-command-line branch from 57d1d7d to 703014d Compare September 8, 2026 14:26

@gustavohenke gustavohenke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for that.

Overall, this pull request fixes real issues (in environments that use it the most?), but also introduces additional complexity [...] first decide whether it's worth pursuing getting this merged.

Agree. The documented bug should be fixed, but maybe not in this shape.

As it stands, this PR makes concurrently both much more robust, but also a bit more brittle.
For example, the allowlist means that standard POSIX would still cause the bug on zsh, since it's not strictly the bash syntax that unbash works with. Maintaining two recognisers is not ideal either.

I feel like you ended up going down the dual recogniser path due to unbash being faithful to bash syntax, so I'm wondering if we can make the fix less architecturally dramatic, while still keeping a single wildcard expansion path. Could we

  1. remove the shell gating
  2. use a tokenizer (whichever it is, shell-quote's or unbash's) to find a valid runner + subcommand + script sequence, and replace just that script token
  3. leave as much as possible of the user input unchanged, so that invalid syntax blows up at runtime, using the configured shell

?

Comment thread lib/command-parser/expand-wildcard.ts Outdated
private readonly readDeno = ExpandWildcard.readDeno,
private readonly readPackage = ExpandWildcard.readPackage,
) {}
shell?: string,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Make this the first argument, so you don't need to do the awkward undefined, undefined, shell in lib/concurrently.ts

@webpro

webpro commented Sep 10, 2026

Copy link
Copy Markdown
Author

Yes, we could.

Let's use the Bash parser ungated, as you suggest. Note that also a Bash tokenizer would not correctly handle every PowerShell/CMD input. unbash doesn't have/expose one (yet). Anyway, all 40 existing main wildcard tests pass unchanged.

✅ This fixes lost command chains, prefixes, wrappers, and quoted ampersands.
❌ Comparing with main also exposed some regressions in this iteration, such as:

  • PowerShell's & npm run build:* and CMD's @npm run build:* no longer expand.
  • A script like concurrently "npm run build:*" → concurrently selects build:app@dev → we add Bash-style single quotes that might be wrong for CMD: npm run 'build:app@dev'
  • For a script like concurrently "npm run test:*-unit(!slow)-watch":
    • Main matches test:*-unit-watch, excludes names containing slow, and runs test:fast-unit-watch.
    • In this PR, we recognize test:*-unit, add the omission filter, but forget the trailing -watch during matching. Neither configured script ends in -unit, so no tests start and concurrently exits 0. This was reproduced through zsh, but the bug is shell-independent.

The first one concerns recognizing shell-specific prefixes. The quoting and omission failures are bugs in the replacement logic that need fixing within this approach.

This all to illustrate the trade-offs. The issues presented feel like edge cases to me. And I think issues like 2 + 3 are fixable in concurrently (in this PR). I'm just not 100% sure how much of a complete picture it paints cross-environment (which is part of the reason why I went with 703014d initially).

If this feels like a better direction we can try and bring this one over the finish line.

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.

4 participants