Escape bash matches per quoting context and fix the word-break boundary

A third review round, again verified by driving real TAB presses through a
pty and executing the resulting line, found the bash helper still corrupted
values in two situations.

The word-break scan treated every COMP_WORDBREAKS character in the word as
readline's replacement boundary, but readline does not split on one that is
backslash-escaped or inside quotes. Measuring what readline actually
replaces confirms it: `node:` and `a=b` keep a head, while `\(w`, `semi\;c`,
`a\:b` and any quoted word are replaced whole. Trimming a head readline was
not going to keep silently dropped the front of the value, so
`deploy --mode \(w<TAB>` ran with `whoami)`. The scan now tracks escaping
and quoting and only counts a boundary outside both.

Inside a quote the user opened, the match was inserted with no escaping for
that context, so `"$H<TAB>` produced a `$HOME` the shell then expanded, and
`'it<TAB>` produced an unterminated quote. Matches are now escaped for the
enclosing quote: the expandable characters in a double-quoted context, and
the `'\''` splice in a single-quoted one, since a single quote cannot be
escaped inside single quotes. The splice has to be built with a bare
assignment — within double quotes `\'` is not an escape, which double-escaped
it and left the shell at a continuation prompt.

Also make the helper name collision-proof. `sanitizeFunctionName` maps any
character to `_`, so a subcommand named `-choices` produced exactly
`_deploy__choices` and overwrote the helper; the name is now chosen against
the set of function names the script actually emits.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: godu

packages/effect/src/unstable/cli/internal/completions/bash.ts

@@ -16,6 +16,24 @@ const escapeForBash = (s: string): string => s.replace(/'/g, "'\\''")
 
 const sanitizeFunctionName = (s: string): string => s.replace(/[^a-zA-Z0-9_]/g, "_")
 
+/**
+ * Every function name `generateFunction` will emit. `sanitizeFunctionName` maps
+ * any character to `_`, so a subcommand can produce any name — the shared helper
+ * has to pick one that is provably not among them.
+ */
+const emittedFunctionNames = (
+  descriptor: Completions.CommandDescriptor,
+  parentPath: ReadonlyArray<string>,
+  names: Set<string>
+): Set<string> => {
+  const currentPath = [...parentPath, descriptor.name]
+  names.add(`_${currentPath.map(sanitizeFunctionName).join("_")}`)
+  for (const sub of descriptor.subcommands) {
+    emittedFunctionNames(sub, currentPath, names)
+  }
+  return names
+}
+
 const flagNamesForWordlist = (flag: Completions.FlagDescriptor): Array<string> => {
   const names: Array<string> = [`--${flag.name}`]
   for (const alias of flag.aliases) {
@@ -65,14 +83,19 @@ const buildFlagGroupDeclarations = (
  *
  * `compgen -W` re-expands every word of its list, which mangles values holding
  * quotes, spaces or glob characters, so matches are filtered from an explicitly
- * quoted list instead. Two further readline details are handled here:
+ * quoted list instead. The rest of the helper deals with how readline inserts
+ * the match:
  *
- * - Bash inserts COMPREPLY entries verbatim when the word is unquoted, so each
- *   match is requoted with `printf %q`. When the user has opened a quote, bash
- *   quotes the entry itself and requoting would double-escape it.
- * - Readline only replaces the text after the last COMP_WORDBREAKS character
- *   (`:` among them), so that head has to be trimmed off every match or a value
- *   like `node:20` is appended to what was typed rather than replacing it.
+ * - An unquoted word is replaced verbatim, so the match is escaped with
+ *   `printf %q`. Inside a quote the user opened, bash closes the quote for us
+ *   but escapes nothing, so the match is escaped for that quote context —
+ *   including the `\'` splice, since a single quote cannot be escaped within
+ *   single quotes.
+ * - Readline replaces only the text after the last COMP_WORDBREAKS character,
+ *   so that head is trimmed from every match; otherwise a value like `node:20`
+ *   is appended to what was typed rather than replacing it. Wordbreaks that are
+ *   backslash-escaped or inside quotes do not split the word, so they must not
+ *   be treated as the boundary.
  *
  * Prefix matching uses the dequoted word. Dequoting is best effort: it strips
  * one opening quote and any backslash escapes, so a value whose own text
@@ -83,26 +106,50 @@ const choicesHelper = (helperName: string, lines: Array<string>): void => {
   lines.push(`{`)
   lines.push(`  local _cur="$1"; shift`)
   lines.push(`  local _prefix="\${_cur#[\\"\\']}"; _prefix="\${_prefix//\\\\/}"`)
-  lines.push(`  local _quoted_word=""`)
-  lines.push(`  [[ "$_cur" == [\\"\\']* ]] && _quoted_word=1`)
+  lines.push(`  local _open=""`)
+  lines.push(`  [[ "$_cur" == [\\"\\']* ]] && _open="\${_cur:0:1}"`)
+  lines.push(``)
   lines.push(`  COMPREPLY=()`)
-  lines.push(`  local _choice _quoted`)
+  lines.push(`  local _choice _match`)
   lines.push(`  for _choice in "$@"; do`)
   lines.push(`    [[ "$_choice" == "$_prefix"* ]] || continue`)
-  lines.push(`    if [[ -n "$_quoted_word" ]]; then`)
-  lines.push(`      COMPREPLY+=("$_choice")`)
-  lines.push(`    else`)
-  lines.push(`      printf -v _quoted '%q' "$_choice"`)
-  lines.push(`      COMPREPLY+=("$_quoted")`)
-  lines.push(`    fi`)
+  lines.push(`    case "$_open" in`)
+  lines.push(`      '"')`)
+  lines.push(`        _match="\${_choice//\\\\/\\\\\\\\}"`)
+  lines.push(`        _match="\${_match//\\$/\\\\$}"`)
+  lines.push(`        _match="\${_match//\\\`/\\\\\\\`}"`)
+  lines.push(`        _match="\${_match//\\"/\\\\\\"}"`)
+  lines.push(`        ;;`)
+  lines.push(`      "'")`)
+  lines.push(`        # bare assignment: inside double quotes \\' is not an escape`)
+  lines.push(`        _match=\${_choice//\\'/\\'\\\\\\'\\'}`)
+  lines.push(`        ;;`)
+  lines.push(`      *)`)
+  lines.push(`        printf -v _match '%q' "$_choice"`)
+  lines.push(`        ;;`)
+  lines.push(`    esac`)
+  lines.push(`    COMPREPLY+=("$_match")`)
   lines.push(`  done`)
-  lines.push(`  local _head="$_cur"`)
-  lines.push(`  while [[ -n "$_head" ]]; do`)
-  lines.push(`    [[ "$COMP_WORDBREAKS" == *"\${_head: -1}"* ]] && break`)
-  lines.push(`    _head="\${_head%?}"`)
+  lines.push(``)
+  lines.push(`  # Boundary = last wordbreak character that is neither escaped nor quoted`)
+  lines.push(`  local _i _c _quote="" _escaped=0 _cut=0`)
+  lines.push(`  for ((_i = 0; _i < \${#_cur}; _i++)); do`)
+  lines.push(`    _c="\${_cur:_i:1}"`)
+  lines.push(`    if ((_escaped)); then _escaped=0; continue; fi`)
+  lines.push(`    case "$_c" in`)
+  lines.push(`      \\\\) _escaped=1 ;;`)
+  lines.push(`      \\"|\\')`)
+  lines.push(`        if [[ -z "$_quote" ]]; then _quote="$_c"`)
+  lines.push(`        elif [[ "$_quote" == "$_c" ]]; then _quote=""`)
+  lines.push(`        fi`)
+  lines.push(`        ;;`)
+  lines.push(`      *)`)
+  lines.push(`        if [[ -z "$_quote" && "$COMP_WORDBREAKS" == *"$_c"* ]]; then _cut=$((_i + 1)); fi`)
+  lines.push(`        ;;`)
+  lines.push(`    esac`)
   lines.push(`  done`)
-  lines.push(`  if [[ -n "$_head" ]]; then`)
-  lines.push(`    local _i`)
+  lines.push(`  if ((_cut > 0)); then`)
+  lines.push(`    local _head="\${_cur:0:_cut}"`)
   lines.push(`    for ((_i = 0; _i < \${#COMPREPLY[@]}; _i++)); do`)
   lines.push(`      COMPREPLY[_i]="\${COMPREPLY[_i]#"$_head"}"`)
   lines.push(`    done`)
@@ -293,7 +340,9 @@ export const generate = (
 ): string => {
   const lines: Array<string> = []
   const safeName = sanitizeFunctionName(executableName)
-  const helperName = `_${safeName}__choices`
+  const taken = emittedFunctionNames(descriptor, [], new Set())
+  let helperName = `_${safeName}__choices`
+  while (taken.has(helperName)) helperName += "_"
 
   lines.push(`###-begin-${escapeForBash(executableName)}-completions-###`)
   lines.push(`#`)

packages/effect/test/unstable/cli/completions/completions.test.ts

@@ -235,21 +235,38 @@ describe("Bash completions", () => {
     assert.notInclude(script, `compgen -W 'it`)
   })
 
-  it("requotes matches for insertion but not inside a quote the user opened", () => {
+  it("escapes matches for the quoting context of the word being completed", () => {
     const desc = fromCommand(withTrickyChoices)
     const script = Bash.generate("deploy", desc)
     assert.include(script, `local _prefix="\${_cur#[\\"\\']}"; _prefix="\${_prefix//\\\\/}"`)
-    assert.include(script, `[[ "$_cur" == [\\"\\']* ]] && _quoted_word=1`)
-    assert.include(script, `printf -v _quoted '%q' "$_choice"`)
+    assert.include(script, `[[ "$_cur" == [\\"\\']* ]] && _open="\${_cur:0:1}"`)
+    // unquoted: %q. Double quotes: escape what the shell still expands there.
+    // Single quotes: splice, since a quote cannot be escaped inside them.
+    assert.include(script, `printf -v _match '%q' "$_choice"`)
+    assert.include(script, `_match="\${_match//\\$/\\\\$}"`)
+    assert.include(script, `_match=\${_choice//\\'/\\'\\\\\\'\\'}`)
   })
 
-  it("trims the COMP_WORDBREAKS head so colon values replace the typed word", () => {
+  it("treats only unescaped, unquoted word-break characters as the replacement boundary", () => {
     const desc = fromCommand(withTrickyChoices)
     const script = Bash.generate("deploy", desc)
-    assert.include(script, `[[ "$COMP_WORDBREAKS" == *"\${_head: -1}"* ]] && break`)
+    assert.include(script, `if [[ -z "$_quote" && "$COMP_WORDBREAKS" == *"$_c"* ]]; then _cut=$((_i + 1)); fi`)
     assert.include(script, `COMPREPLY[_i]="\${COMPREPLY[_i]#"$_head"}"`)
   })
 
+  it("picks a choices-helper name that no generated command function can collide with", () => {
+    const collidingName = Command.make("deploy", {
+      mode: Flag.choice("mode", ["a"])
+    }).pipe(
+      Command.withSubcommands([Command.make("-choices", { m: Flag.choice("m", ["b"]) })])
+    )
+    const script = Bash.generate("deploy", fromCommand(collidingName))
+    assert.include(script, "_deploy__choices()")
+    assert.include(script, "_deploy__choices_()")
+    assert.include(script, `_deploy__choices_ "$cur" 'a'`)
+    assert.notInclude(script, `_deploy__choices "$cur"`)
+  })
+
   it("generates separate functions for nested subcommands", () => {
     const desc = fromCommand(withSubcommands)
     const script = Bash.generate("server", desc)