fix: parse rsconnect-args with shell quoting so values can contain spaces - #80
Conversation
…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>
| | `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"`. | |
There was a problem hiding this comment.
| | `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`. | |
| malformed input (e.g. an unbalanced quote). | ||
| """ | ||
| try: | ||
| return shlex.split(raw) |
There was a problem hiding this comment.
Awful lot of wrapping for a one liner--can we just python -m this?
| working-directory: ${{ inputs.path }} | ||
| run: ${{ github.action_path }}/scripts/generate-requirements.sh | ||
|
|
||
| # Splits rsconnect-args with shell quoting rules (Python shlex.split) so a |
There was a problem hiding this comment.
Why does this need to be a separate step in the job, rather than just calling the new python utility in deploy.sh?
|
@claude what do you think of my questions, can this be simplified any? |
|
Claude finished @nealrichardson's task in 1m 38s —— View job Reviewing simplification questions
Both of Neal's points are valid, and yes — this can be simplified quite a bit. On the separate module ( On the separate action step: Unlike Suggested simplification: Remove the module, the CLI subcommand, and the separate action step. Instead, have # 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")
fiAnd in 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 The test coverage for this becomes an e2e concern (which the existing workflow already covers with |
|
@claude please do the simplifications in #80 (comment) |
|
Claude finished @nealrichardson's task in 2m 42s —— View job Simplifying rsconnect-args parsing
Done. Removed ~170 lines by inlining the |
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>
|
Preview deployed to Connect ( Deployed from commit b5fcc29. |
|
Cleaned up 1 preview bundle(s) on http://localhost:3942: 8 |
|
Preview deployed to Connect ( Deployed from commit b5fcc29. |
|
Preview deployed to Connect ( Deployed from commit b5fcc29. |
|
Cleaned up 1 preview bundle(s) on http://localhost:3940: 8 |
|
Cleaned up 1 preview bundle(s) on http://localhost:3941: 8 |
tdstein
left a comment
There was a problem hiding this comment.
LGTM (can't approve because I'm the author), thanks for iterating on it!
Bug
deploy/scripts/deploy.shexpands thersconnect-argsinput unquoted: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), andApp"— and the click-based CLI fails withGot 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
--titlematching 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_FILESpattern (newline-delimited passing, since GitHub Actions outputs can't safely carry arbitrary whitespace/quoting):connect_actions.rsconnect_argsmodule:parse_rsconnect_args()splits the raw string withshlex.split(POSIX shell quoting rules), raisingRsconnectArgsErroron malformed input (e.g. an unbalanced quote).resolve-rsconnect-argsCLI subcommand (cli.py) readsINPUT_RSCONNECT_ARGS, parses it, and writes the result as a newline-delimitedrsconnect_argsGITHUB_OUTPUTvalue (same heredoc-style multi-line outputextra_filesalready uses). Parse errors printError: ...to stderr and exit 1.deploy/action.yml: new "Parse rsconnect-args" step runs the subcommand before the deploy step; the deploy step'sRSCONNECT_ARGSenv now comes from this step's output instead of the raw input.deploy/scripts/deploy.sh: reads the newline-delimited value into anRSCONNECT_ARGS_ARRarray (mirroring the existingEXTRA_FILE_ARGSloop) and expands it as"${RSCONNECT_ARGS_ARR[@]}"instead of unquoted${RSCONNECT_ARGS:-}.README.md: documented thatrsconnect-argssupports shell-style quoting, e.g.--title "My App".Backward compatible: simple space-separated args like
--verbose --newparse and behave identically to the old unquoted expansion (verified against the existing e2e workflow'srsconnect-args: --override-python-version ...usage).Test plan
uv run pytest— all 77 tests pass, including new unit tests intests/test_rsconnect_args.py(parsing: empty input, simple flags, double/single quoted values with spaces, mixed args, unbalanced-quote error) andtests/test_cli.py(newresolve-rsconnect-argssubcommand: simple flags, quoted values, empty input, parse-error exit code).shellcheck deploy/scripts/deploy.sh— clean, no warnings.🤖 Generated with Claude Code