Skip to content

fix: parse rsconnect-args with shell quoting so values can contain spaces - #80

Merged
nealrichardson merged 2 commits into
mainfrom
fix/rsconnect-args-quoting
Aug 5, 2026
Merged

fix: parse rsconnect-args with shell quoting so values can contain spaces#80
nealrichardson merged 2 commits into
mainfrom
fix/rsconnect-args-quoting

Conversation

@tdstein

@tdstein tdstein commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Bug

deploy/scripts/deploy.sh expands the rsconnect-args input unquoted:

posit connect deploy "$APP_TYPE" ... ${RSCONNECT_ARGS:-} "$DEPLOY_TARGET" ...

Plain Bash word-splitting (IFS) with no quote parsing means rsconnect-args: --title "My App" reaches the CLI as three tokens — --title, "My (with the literal quote), and App" — and the click-based CLI fails with Got unexpected extra argument. There's no way to pass an argument value containing whitespace.

This matters in practice: until posit-dev/rsconnect-python#835 is fixed, passing --title matching the content's exact title is the only way to deploy a manifest under trusted publishing, and content titles routinely contain spaces.

Fixes #79

Fix

Follows the existing EXTRA_FILES pattern (newline-delimited passing, since GitHub Actions outputs can't safely carry arbitrary whitespace/quoting):

  • New connect_actions.rsconnect_args module: parse_rsconnect_args() splits the raw string with shlex.split (POSIX shell quoting rules), raising RsconnectArgsError on malformed input (e.g. an unbalanced quote).
  • New resolve-rsconnect-args CLI subcommand (cli.py) reads INPUT_RSCONNECT_ARGS, parses it, and writes the result as a newline-delimited rsconnect_args GITHUB_OUTPUT value (same heredoc-style multi-line output extra_files already uses). Parse errors print Error: ... to stderr and exit 1.
  • deploy/action.yml: new "Parse rsconnect-args" step runs the subcommand before the deploy step; the deploy step's RSCONNECT_ARGS env now comes from this step's output instead of the raw input.
  • deploy/scripts/deploy.sh: reads the newline-delimited value into an RSCONNECT_ARGS_ARR array (mirroring the existing EXTRA_FILE_ARGS loop) and expands it as "${RSCONNECT_ARGS_ARR[@]}" instead of unquoted ${RSCONNECT_ARGS:-}.
  • README.md: documented that rsconnect-args supports shell-style quoting, e.g. --title "My App".

Backward compatible: simple space-separated args like --verbose --new parse and behave identically to the old unquoted expansion (verified against the existing e2e workflow's rsconnect-args: --override-python-version ... usage).

Test plan

  • uv run pytest — all 77 tests pass, including new unit tests in tests/test_rsconnect_args.py (parsing: empty input, simple flags, double/single quoted values with spaces, mixed args, unbalanced-quote error) and tests/test_cli.py (new resolve-rsconnect-args subcommand: simple flags, quoted values, empty input, parse-error exit code).
  • shellcheck deploy/scripts/deploy.sh — clean, no warnings.

🤖 Generated with Claude Code

…aces

deploy.sh expanded rsconnect-args unquoted, so plain Bash word-splitting
turned `--title "My App"` into three tokens (including a stray literal
quote), which the click-based CLI rejected. Parse the raw input with
shlex.split (POSIX rules) in a new connect_actions.rsconnect_args module,
expose it via a resolve-rsconnect-args CLI subcommand that writes a
newline-delimited GITHUB_OUTPUT value (mirroring extra_files), and have
deploy.sh read it into an array and expand it quoted, same as EXTRA_FILES.
Simple space-separated args behave exactly as before.

Fixes #79

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment thread README.md Outdated
| `draft` | No | Deploy as a draft (preview) bundle instead of activating it. Defaults to `true` on `pull_request` events and `false` otherwise. Set it explicitly to override--e.g. `false` to publish directly from a PR, or `true` to stage a draft from a push. |
| `github-token` | No | GitHub token for commenting preview URLs on PRs |
| `rsconnect-args` | No | Additional arguments passed to `rsconnect deploy` |
| `rsconnect-args` | No | Additional arguments passed to `rsconnect deploy`. Supports shell-style quoting for values containing spaces, e.g. `--title "My App"`. |

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
| `rsconnect-args` | No | Additional arguments passed to `rsconnect deploy`. Supports shell-style quoting for values containing spaces, e.g. `--title "My App"`. |
| `rsconnect-args` | No | Additional arguments passed to `rsconnect deploy`. |

Comment thread src/connect_actions/rsconnect_args.py Outdated
malformed input (e.g. an unbalanced quote).
"""
try:
return shlex.split(raw)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awful lot of wrapping for a one liner--can we just python -m this?

Comment thread deploy/action.yml Outdated
working-directory: ${{ inputs.path }}
run: ${{ github.action_path }}/scripts/generate-requirements.sh

# Splits rsconnect-args with shell quoting rules (Python shlex.split) so a

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why does this need to be a separate step in the job, rather than just calling the new python utility in deploy.sh?

@nealrichardson

Copy link
Copy Markdown
Contributor

@claude what do you think of my questions, can this be simplified any?

@posit-connect-projects

posit-connect-projects Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Claude finished @nealrichardson's task in 1m 38s —— View job


Reviewing simplification questions

  • Read current implementation files
  • Analyze Neal's two review questions
  • Provide recommendation

Both of Neal's points are valid, and yes — this can be simplified quite a bit.

On the separate module (rsconnect_args.py): The entire module wraps a single stdlib call (shlex.split) in a custom exception class and multi-paragraph docstrings. The custom exception doesn't add value over just catching ValueError at the call site. There's no business logic to unit test beyond what shlex already guarantees.

On the separate action step: Unlike extra_files (which comes from config resolution — a genuinely separate concern that runs earlier and feeds multiple consumers), rsconnect-args parsing is purely input preprocessing for the deploy step. There's no reason it can't happen inside deploy.sh itself.

Suggested simplification: Remove the module, the CLI subcommand, and the separate action step. Instead, have deploy.sh receive the raw rsconnect-args input and parse it inline:

# In deploy.sh, replace the current RSCONNECT_ARGS_ARR block with:
RSCONNECT_ARGS_ARR=()
if [ -n "${RSCONNECT_ARGS:-}" ]; then
  while IFS= read -r rsconnect_arg; do
    [ -n "$rsconnect_arg" ] && RSCONNECT_ARGS_ARR+=("$rsconnect_arg")
  done < <(python3 -c "
import shlex, sys
try:
    print('\n'.join(shlex.split(sys.argv[1])))
except ValueError as e:
    print(f'Error: Could not parse rsconnect-args: {e}', file=sys.stderr)
    sys.exit(1)
" "$RSCONNECT_ARGS")
fi

And in action.yml, pass the raw input directly to the deploy step (RSCONNECT_ARGS: ${{ inputs.rsconnect-args }}) instead of routing through a parse step's output.

This eliminates ~130 lines (module + CLI handler + tests + action step + comments) while preserving the exact same behavior: shell-quoting-aware splitting with a clear error on malformed input. The set -euo pipefail in deploy.sh ensures a parse failure still halts the deploy.

The test coverage for this becomes an e2e concern (which the existing workflow already covers with rsconnect-args: --override-python-version ...), and you could add one e2e case with a quoted value if desired.


@nealrichardson

Copy link
Copy Markdown
Contributor

@claude please do the simplifications in #80 (comment)

@posit-connect-projects

posit-connect-projects Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Claude finished @nealrichardson's task in 2m 42s —— View job


Simplifying rsconnect-args parsing

  • Read current implementation files
  • Remove src/connect_actions/rsconnect_args.py module
  • Remove resolve-rsconnect-args CLI subcommand from cli.py
  • Remove separate "Parse rsconnect-args" step from deploy/action.yml
  • Update deploy/scripts/deploy.sh to parse inline with Python one-liner
  • Remove tests/test_rsconnect_args.py
  • Remove rsconnect-args tests from tests/test_cli.py
  • Apply tdstein's README suggestion
  • Run tests to verify (62 pass, 3 pre-existing failures from missing uv)
  • Commit and push

Done. Removed ~170 lines by inlining the shlex.split call directly in deploy.sh instead of routing through a separate Python module, CLI subcommand, and action step. The deploy script now receives the raw rsconnect-args input and parses it with a python3 -c one-liner — same quoting behavior, much less machinery. shellcheck and all relevant tests pass.
|

Remove the separate rsconnect_args module, CLI subcommand, action step,
and dedicated tests. Instead, deploy.sh parses the raw input inline with
a python3 shlex.split one-liner — same shell-quoting behavior, ~170
fewer lines.

Co-authored-by: Neal Richardson <2975928+nealrichardson@users.noreply.github.com>
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Preview deployed to Connect (localhost:3942): http://localhost:3942/connect/#/apps/e23be7a3-4807-4efd-a578-0c0596d1c78c/draft/8

Deployed from commit b5fcc29.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Cleaned up 1 preview bundle(s) on http://localhost:3942: 8

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Preview deployed to Connect (localhost:3940): http://localhost:3940/connect/#/apps/e02c7bd1-8fea-4521-9f95-efb5ab60f585/draft/8

Deployed from commit b5fcc29.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Preview deployed to Connect (localhost:3941): http://localhost:3941/connect/#/apps/8a7b1350-5f05-41aa-a77a-5de9ef67858f/draft/8

Deployed from commit b5fcc29.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Cleaned up 1 preview bundle(s) on http://localhost:3940: 8

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Cleaned up 1 preview bundle(s) on http://localhost:3941: 8

@tdstein tdstein left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

LGTM (can't approve because I'm the author), thanks for iterating on it!

@nealrichardson
nealrichardson merged commit 127872f into main Aug 5, 2026
8 checks passed
@nealrichardson
nealrichardson deleted the fix/rsconnect-args-quoting branch August 5, 2026 17:41
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.

rsconnect-args can't pass argument values containing spaces

2 participants