Skip to content

Handle quoted admin submit_job paths#4456

Merged
holgerroth merged 8 commits intoNVIDIA:mainfrom
holgerroth:codex/fix-submit-job-quoted-path
Apr 22, 2026
Merged

Handle quoted admin submit_job paths#4456
holgerroth merged 8 commits intoNVIDIA:mainfrom
holgerroth:codex/fix-submit-job-quoted-path

Conversation

@holgerroth
Copy link
Copy Markdown
Collaborator

Summary

  • make HCI command parsing honor shell-style quoting for admin commands such as submit_job
  • keep a legacy whitespace-splitting fallback for malformed quoting to avoid new parse failures
  • add focused unit tests for quoted job paths and existing # command-props behavior

Why

Dragging a job folder into the admin console on Ubuntu can produce a single-quoted path. The current parser only switches to shlex.split() when a command contains double quotes, so single-quoted paths are passed through literally and submit_job rejects the resulting folder path.

Root Cause

nvflare.fuel.hci.cmd_arg_utils.split_to_args() and parse_command_line() special-case double quotes instead of using shell-like tokenization generally. That leaves single quotes in the parsed argument, which then flows into the client-side file transfer path lookup.

Impact

This change allows quoted drag-and-drop job paths to be submitted from the admin console without manually stripping surrounding quotes.

Fixes #1328.

Validation

  • pytest tests/unit_test/fuel/hci/cmd_arg_utils_test.py -q

@holgerroth holgerroth changed the title [codex] Handle quoted admin submit_job paths Handle quoted admin submit_job paths Apr 17, 2026
@holgerroth holgerroth marked this pull request as ready for review April 17, 2026 21:09
@holgerroth
Copy link
Copy Markdown
Collaborator Author

/build

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 17, 2026

Greptile Summary

This PR fixes single-quoted path parsing in the admin console by generalising the " guard in split_to_args and parse_command_line to cover any quote character, routing quoted input through shlex.split with a whitespace-split fallback for malformed quoting. The previously raised concerns about backslash semantics and single-quoted hash paths are both addressed and regression-tested.

Confidence Score: 5/5

Safe to merge; the only remaining finding is a minor edge-case documentation/test gap that does not affect the primary fix.

All P0/P1 concerns from prior review rounds are resolved. The one new observation (props silently dropped for single-quoted commands that also use the hash notation) is P2: the old double-quoted path had the same limitation, the combined scenario was already broken before this PR, and submit_job — the primary target — never uses props.

No files require special attention.

Important Files Changed

Filename Overview
nvflare/fuel/hci/cmd_arg_utils.py Refactors split_to_args and parse_command_line to honor single-quoted paths via shlex.split with a whitespace-split fallback; props are silently dropped for any quoted line.
tests/unit_test/fuel/hci/cmd_arg_utils_test.py New test file covering single-quoted paths, backslash preservation, unmatched-quote fallback, hash-in-quoted-path, double-quote backward compat, and unquoted props; missing combined quoted-args+props scenario.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Input line] --> B{_has_quotes?}
    B -- Yes --> C[_split_quoted_args]
    B -- No --> D[# prop split]
    C --> E{shlex.split succeeds?}
    E -- Yes --> F[Return shlex tokens, props=None]
    E -- No / ValueError --> G[_split_unquoted_args fallback, props=None]
    D --> H[strip line, capture props]
    H --> I[_split_unquoted_args]
    I --> J[Return tokens + props]
Loading

Reviews (6): Last reviewed commit: "Merge branch 'main' into codex/fix-submi..." | Re-trigger Greptile

Comment thread nvflare/fuel/hci/cmd_arg_utils.py Outdated
Comment thread nvflare/fuel/hci/cmd_arg_utils.py Outdated
Copy link
Copy Markdown
Collaborator

@chesterxgchen chesterxgchen left a comment

Choose a reason for hiding this comment

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

This PR is not needed,

  1. the fix is already in PR #4449
  • HCI command parsing already honors shell-style quoting when
    the command contains ":
    • nvflare/fuel/hci/cmd_arg_utils.py:22
    • split_to_args() uses shlex.split(line) if " is present
  • The server registry uses that tokenizer directly:
    • nvflare/fuel/hci/server/reg.py:67
  1. the malformat string should fallback, should fail

@holgerroth
Copy link
Copy Markdown
Collaborator Author

/build

Copy link
Copy Markdown
Collaborator Author

I re-checked this against current upstream/main because issue #1328 is specifically about Ubuntu drag-and-drop wrapping the job folder in single quotes, for example:

submit_job '/home/.../job_configs/cifar10_splitnn'

That case still reproduces on today's upstream/main after #4449. split_to_args() and parse_command_line() still only switch to shlex.split() when the command contains a double quote, so single-quoted paths are still passed through with the quotes intact, and quoted paths containing # still get split incorrectly in parse_command_line().

So from the perspective of issue #1328, I don't think #4449 fully covers the problem yet. If the concern is specifically the malformed-input fallback behavior, I'm happy to adjust that part separately, but the single-quoted drag-and-drop case from #1328 still appears to be unfixed on main.

Comment thread nvflare/fuel/hci/cmd_arg_utils.py Outdated
@holgerroth holgerroth enabled auto-merge (squash) April 20, 2026 18:54
@holgerroth
Copy link
Copy Markdown
Collaborator Author

/build

Comment thread nvflare/fuel/hci/cmd_arg_utils.py
@holgerroth
Copy link
Copy Markdown
Collaborator Author

/build

@holgerroth
Copy link
Copy Markdown
Collaborator Author

/build

@holgerroth holgerroth merged commit d724370 into NVIDIA:main Apr 22, 2026
29 checks passed
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.

Allow drag&drop job folder into admin console

3 participants