[Docker] Accept the container profile before the command, and start from enter

[hujc7]

1. Summary

• ./docker/container.py <profile> <command> is now accepted, so stepping a container through its lifecycle only changes the last word: kitless build → kitless start → kitless enter. The existing <command> <profile> order keeps working.
• enter now starts the container when it is not running, instead of failing with The container '<name>' is not running.
• 3 lines of parsing logic, 15 tests, 14 documentation examples updated.

2. Why

Repeating the profile on every command puts the only varying word in the middle of the line, so the previous command cannot be reused by editing its tail. This was invisible while base was the default and usually omitted; the kit-less profile is the first one typed on every invocation.

The command set comes from argparse's own subparsers.choices rather than a second hardcoded list, so adding a subcommand needs no change here. Every subcommand takes exactly one positional — the profile — which is what makes the swap unambiguous; a test pins that profile names and command names stay disjoint.

3. enter behaviour change

stop deletes the temporary .xauth file, so enter afterwards failed twice over: x11_refresh errored with X11 forwarding is enabled but the temporary .xauth file does not exist before the "container is not running" check was even reached. Running the same setup start does resolves both.

Note for reviewers: this means enter can now trigger a build on a cold cache, so it is no longer guaranteed-fast. It prints [INFO] Container '<name>' is not running. Starting it first... so the pause is explained. A test pins both directions, so a running container is never restarted.

4. Validation

15 passed
container.py base enter --info     → Profile: base      container.py enter base --info  → Profile: base
container.py kitless build --info  → Profile: kitless   container.py build kitless --info → Profile: kitless
container.py base start --suffix custom --info → Container Name: isaac-lab-base-custom

5. Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Documentation update

6. Checklist

• I have run the pre-commit checks with ./isaaclab.sh --format
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I have added a changelog fragment for the touched Isaac Lab packages

[greptile-apps]

Greptile Summary

The PR adds support for profile-first Docker CLI invocations while preserving command-first compatibility, and makes enter start stopped containers automatically.

• Reorders an initial <profile> <command> pair before passing arguments to argparse.
• Reuses the established X11 setup and container startup sequence when enter finds a stopped container.
• Adds focused parser and lifecycle tests.
• Updates Docker documentation and the changelog for the new syntax and behavior.

Confidence Score: 5/5

The PR appears safe to merge; the new argument order and enter-from-stopped lifecycle preserve the established command behavior.

The argument rewrite is constrained to an unambiguous initial profile-command pair, while the new enter path performs the same X11 configuration and startup steps as the existing start path before refreshing credentials and entering.

Important Files Changed

Filename	Overview
docker/container.py	Adds narrowly scoped argument reordering and reuses the existing startup setup before entering a stopped container; no actionable defect was identified.
docker/test/test_container_cli.py	Covers both argument orders, representative option handling, command/profile disjointness, and both running states of the enter lifecycle.
docs/source/features/include/docker_details.inc	Updates the primary Docker workflow documentation to describe profile-first syntax and automatic startup from enter.
docs/source/setup/installation/index.rst	Updates the installation example to use the newly supported profile-first form.
source/isaaclab/changelog.d/jichuanh-container-profile-first.rst	Accurately records the additional CLI order and changed enter behavior.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Parse CLI arguments] --> B{Profile appears before command?}
    B -- Yes --> C[Swap profile and command]
    B -- No --> D[Keep arguments unchanged]
    C --> E[argparse]
    D --> E
    E --> F{Command is enter?}
    F -- No --> G[Run selected command]
    F -- Yes --> H{Container running?}
    H -- Yes --> K[Refresh X11 credentials]
    H -- No --> I[Check and configure X11]
    I --> J[Start container]
    J --> K
    K --> L[Enter container]

Loading

_{Reviews (1): Last reviewed commit: "Merge branch 'develop' into jichuanh/con..." | Re-trigger Greptile}

[isaaclab-review-bot]

Isaac Lab Review Bot

Reviewed the argv reordering, backward-compatible command-first syntax, and the new enter auto-start behavior. The changed parser, lifecycle branches, tests, documentation examples, and changelog are consistent with the intended CLI contract.

• Design and architecture: The argv pre-pass derives recognized commands from subparsers.choices, keeping profile-first detection aligned with argparse’s registered subcommands. The enter path conditionally performs the same X11 setup required by start before entering a stopped container; this introduces some duplicated setup logic but no demonstrated merge-blocking issue.
• API: Existing <command> [profile] forms remain unchanged, while <profile> <command> gains defined meaning when the first two tokens are unambiguous. The state-changing behavior of enter on a stopped container is intentional and documented, including the possibility that startup may take time.
• Implementation: The parser explicitly passes the reordered sys.argv[1:] to argparse, and the running-state guard avoids restarting an active container. Tests cover both argument orders, flag handling, command/profile disjointness, and both running states for enter; documentation and the changelog reflect the changed behavior.

No blocking issues. No inline issue met the actionable-evidence threshold; the assessment above records the review feedback.

Automated review; human maintainers own approval decisions.

[AntoineRichard]

The new regression tests are not currently executed by CI. docker/test/test_container_cli.py covers both the profile-first parser and enter-from-stopped behavior, but the Kit-less Docker workflow runs only test_container_profiles.py and test_dockerfile_nonroot.py. The general test runner defaults to source/, so this new file is not collected elsewhere either. Please add test_container_cli.py to both the workflow change-detection patterns and its pytest command, so these regressions remain enforced and test-only changes trigger the job.

[AntoineRichard]

🤖 AI-generated review (OpenAI Codex)

This review was generated by an AI coding agent and verified against commit 719e5e4c13992613de924d81e8dc33a21ffb1028.

Findings

1. Important — New regression tests are not run by CI.

   docker/test/test_container_cli.py is absent from both the change-detection patterns and pytest invocation in the Kit-less workflow. Consequently, neither new behavior is continuously enforced, and test-only changes would not trigger the job. Please add this file to both locations. This was also raised in the existing PR comment.

2. Moderate — The X11 recovery behavior is not actually tested.

   The stopped-container test forces x11_check() to return None. Removing the new X11 setup would therefore leave this test green even though enter after stop would again terminate in x11_refresh() when X11 is enabled. Please add a stopped/X11-enabled case that returns the overlay and environment, then verifies they are applied before start().

3. Moderate — The Changed fragment lacks required migration guidance.

   The repository guidelines require migration guidance for every Changed entry, but the fragment only describes the new behavior. A concise addition could tell scripts that relied on enter failing for stopped containers to check container state explicitly before invoking it.

I found no implementation-level correctness defect in the argument rewrite or auto-start control flow.

Verification

• 28 passed across test_container_cli.py and test_container_profiles.py.
• Full ./isaaclab.sh -f passed.
• With the base implementation temporarily restored, the new test file produced 12 failed, 3 passed; restoring the PR implementation produced 15 passed.
• No live Docker lifecycle test was run.