Skip to content

fix(tui): preserve quoted post-create command arguments - #4137

Merged
johntmyers merged 1 commit into
NVIDIA:mainfrom
thotashashank302:fix/4113-tui-command-quoting/thotashashank302
Oct 3, 2026
Merged

johntmyers merged 1 commit into
NVIDIA:mainfrom
thotashashank302:fix/4113-tui-command-quoting/thotashashank302

Conversation

@thotashashank302

Copy link
Copy Markdown
Contributor

Summary

Preserve quoted arguments in the TUI Create Sandbox command field. /bin/sh -c "echo GOOD; read x" now reaches the shell as three arguments instead of splitting the script at each space. Invalid quoting leaves the form open with an error before sandbox creation is queued.

Related Issue

Closes #4113

Changes

  • Parse the command once on form submission with shell-words, already present in the lockfile, and retain the argument vector through post-create execution.
  • Escape each parsed argument individually at the SSH boundary so variables, substitutions, and operators remain literal unless the user explicitly invokes a shell.
  • Add form regressions for quoting, escaping, empty arguments, empty commands, and malformed quotes; add Unix shell-execution tests for script input and literal argument preservation.
  • Document the Command field behavior and update the TUI contributor guidance.

Testing

  • Checks appropriate to the affected code and behavior pass
  • Unit tests added/updated
  • E2E tests added/updated (live gateway/SSH interaction not exercised)

Verified on macOS arm64 with Rust 1.95.0:

  • cargo fmt -p openshell-tui --check
  • cargo test --locked -p openshell-tui: 91 passed
  • cargo clippy --locked -p openshell-tui --all-targets -- -D warnings
  • cargo test --locked -p openshell-cli --features openshell-server/prebuilt-z3: 640 passed, 2 integration tests and 4 doctests ignored. The prebuilt feature supplies Z3 because the system library is absent.
  • Both new form regressions fail when whitespace splitting is temporarily restored and pass with the fix.
  • target/debug/openshell term --help
  • Documentation navigation check and pinned Markdown lint: passed
  • Pinned Fern 5.112.0 validation: 0 errors, 3 warnings concerning an unrelated gateway configuration MDX page, unauthenticated redirect checking, and light-mode accent contrast
  • git diff --check

The shell-execution tests use a local /bin/sh to verify the serialized remote command and stdin handling. A running sandbox, interactive TUI over SSH, Linux, and Windows have not been exercised. Upstream CI and maintainer review remain pending.

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • User documentation and related TUI guidance updated
  • Architecture docs updated (not applicable)

Signed-off-by: Thota shashank <thotashashank302@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Oct 2, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@johntmyers johntmyers added the test:e2e Requires end-to-end coverage label Oct 3, 2026
@johntmyers

Copy link
Copy Markdown
Collaborator

/ok to test ad5f783

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown

Label test:e2e applied for ad5f783. Open Branch E2E Checks, find the run for commit ad5f783, and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway, sandbox, and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@johntmyers johntmyers left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

The initial review found no blocking defects. The patch preserves parsed arguments through the SSH boundary, covers quoted scripts and literal arguments with regressions, and documents the Command field behavior.

Action required: A maintainer must approve the current-head Trivy Changes workflow, which is held for workflow approval. The sandbox denied the workflow approval endpoint. Branch Checks, Helm Lint, and a fresh Branch E2E Checks run have started after test:e2e and /ok to test were applied.

Blocking findings:

  • No blocking code findings remain.

Carried findings:

  • None.
Gator metadata
  • Validation: Focused fix for accepted issue #4113; closed PR #4135 was its earlier vouch-gated submission.
  • Docs: Fern sandbox overview updated; navigation unchanged because this updates an existing page.
  • Checks: DCO and vouch pass; Branch Checks and Helm Lint queued/running; Trivy Changes requires workflow approval.
  • E2E: test:e2e applied; fresh current-head run 37149894889 started with the label present. The label-help rerun hint arrived during mirror creation; no earlier mirror E2E run existed to rerun.
  • Head SHA: ad5f783e918aef67ae0ddd6876babd42ee9a0781
  • Base SHA: 046fd2a0246d6765845b3abf6f9cb2c74005781f
  • Merge base SHA: 046fd2a0246d6765845b3abf6f9cb2c74005781f
  • Patch ID: 029ec6ec3bcb21a8ffdc73063c6f08cb4808c9d0
  • Gator payload: 10
  • Review mode: initial
  • Previous reviewed SHA: none
  • Review budget exhausted: no
  • Maintainer decision required: no
  • Next state: gator:blocked
  • Blocked reason: workflow_approval_required

@johntmyers johntmyers added the gator:blocked Gator is blocked by process or repository gates label Oct 3, 2026
@johntmyers
johntmyers added this pull request to the merge queue Oct 3, 2026
@johntmyers johntmyers added gator:merge-ready and removed gator:blocked Gator is blocked by process or repository gates labels Oct 3, 2026
Merged via the queue into NVIDIA:main with commit a2429fc Oct 3, 2026
123 of 125 checks passed
@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

Monitoring Complete

Monitoring is complete because this PR has merged.

Final status: The PR was in gator:merge-ready with maintainer approval present. The initial Gator review found no blocking code findings.

I am removing the active gator:* label because there is nothing left for Gator to monitor on this PR.

Gator metadata
  • Head SHA: ad5f783e918aef67ae0ddd6876babd42ee9a0781
  • Gator payload: 10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: TUI create command loses quoted argument grouping

2 participants