From 18293ab21f1b497c589cc3382ed2ceafe50fdad4 Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Tue, 7 Apr 2026 15:02:55 +0200 Subject: [PATCH 01/19] feat: add shell completion for jmp-admin and shared completion factory Extract a shared make_completion_command factory into jumpstarter-cli-common and use it in both jmp and jmp-admin CLIs. This adds a `completion` subcommand to jmp-admin (and by extension `jmp admin`) supporting bash, zsh, and fish. Generated-By: Forge/20260407_145514_2280530_d26e63ff Co-Authored-By: Claude Opus 4.6 --- .../jumpstarter_cli_admin/__init__.py | 2 + .../jumpstarter_cli_admin/completion.py | 10 +++++ .../jumpstarter_cli_admin/completion_test.py | 43 +++++++++++++++++++ .../jumpstarter_cli_common/completion.py | 17 ++++++++ .../jumpstarter_cli/completion.py | 17 +++----- 5 files changed, 78 insertions(+), 11 deletions(-) create mode 100644 python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/completion.py create mode 100644 python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/completion_test.py create mode 100644 python/packages/jumpstarter-cli-common/jumpstarter_cli_common/completion.py diff --git a/python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/__init__.py b/python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/__init__.py index 41eb3f9f3..89ef858b5 100644 --- a/python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/__init__.py +++ b/python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/__init__.py @@ -3,6 +3,7 @@ from jumpstarter_cli_common.opt import opt_log_level from jumpstarter_cli_common.version import version +from .completion import completion from .create import create from .delete import delete from .get import get @@ -16,6 +17,7 @@ def admin(): """Jumpstarter Kubernetes cluster admin CLI tool""" +admin.add_command(completion) admin.add_command(get) admin.add_command(create) admin.add_command(delete) diff --git a/python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/completion.py b/python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/completion.py new file mode 100644 index 000000000..c6fa73de8 --- /dev/null +++ b/python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/completion.py @@ -0,0 +1,10 @@ +from jumpstarter_cli_common.completion import make_completion_command + + +def _get_admin(): + from jumpstarter_cli_admin import admin + + return admin + + +completion = make_completion_command(_get_admin, "jmp-admin", "_JMP_ADMIN_COMPLETE") diff --git a/python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/completion_test.py b/python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/completion_test.py new file mode 100644 index 000000000..b714ab9cc --- /dev/null +++ b/python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/completion_test.py @@ -0,0 +1,43 @@ +from click.testing import CliRunner + +from . import admin + + +def test_completion_bash(): + runner = CliRunner() + result = runner.invoke(admin, ["completion", "bash"]) + assert result.exit_code == 0 + assert len(result.output) > 0 + assert "complete" in result.output.lower() + assert "jmp-admin" in result.output.lower() + + +def test_completion_zsh(): + runner = CliRunner() + result = runner.invoke(admin, ["completion", "zsh"]) + assert result.exit_code == 0 + assert len(result.output) > 0 + assert "compdef" in result.output.lower() + + +def test_completion_fish(): + runner = CliRunner() + result = runner.invoke(admin, ["completion", "fish"]) + assert result.exit_code == 0 + assert len(result.output) > 0 + assert "complete" in result.output.lower() + assert "--command jmp-admin" in result.output.lower() + + +def test_completion_no_args(): + runner = CliRunner() + result = runner.invoke(admin, ["completion"]) + assert result.exit_code == 2 + assert "Missing argument" in result.output or "bash" in result.output + + +def test_completion_unsupported_shell(): + runner = CliRunner() + result = runner.invoke(admin, ["completion", "powershell"]) + assert result.exit_code == 2 + assert "Invalid value" in result.output or "powershell" in result.output diff --git a/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/completion.py b/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/completion.py new file mode 100644 index 000000000..c9841ddbd --- /dev/null +++ b/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/completion.py @@ -0,0 +1,17 @@ +from typing import Callable + +import click +from click.shell_completion import get_completion_class + + +def make_completion_command(cli_group_factory: Callable[[], click.Command], prog_name: str, complete_var: str): + @click.command("completion") + @click.argument("shell", type=click.Choice(["bash", "zsh", "fish"])) + def completion(shell: str): + """Generate shell completion script.""" + cli_group = cli_group_factory() + comp_cls = get_completion_class(shell) + comp = comp_cls(cli_group, {}, prog_name, complete_var) + click.echo(comp.source()) + + return completion diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/completion.py b/python/packages/jumpstarter-cli/jumpstarter_cli/completion.py index f21def97f..eab81ed4c 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/completion.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/completion.py @@ -1,15 +1,10 @@ -import click -from click.shell_completion import get_completion_class +from jumpstarter_cli_common.completion import make_completion_command -@click.command("completion") -@click.argument("shell", type=click.Choice(["bash", "zsh", "fish"])) -def completion(shell: str): - """Generate shell completion script.""" +def _get_jmp(): from jumpstarter_cli.jmp import jmp - comp_cls = get_completion_class(shell) - if comp_cls is None: - raise click.ClickException(f"Unsupported shell: {shell}") - comp = comp_cls(jmp, {}, "jmp", "_JMP_COMPLETE") - click.echo(comp.source()) + return jmp + + +completion = make_completion_command(_get_jmp, "jmp", "_JMP_COMPLETE") From cb5674e34108fdc365de6811ba740b0d48e47300 Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Tue, 7 Apr 2026 15:09:12 +0200 Subject: [PATCH 02/19] fix: add unit tests for make_completion_command factory in cli-common The shared make_completion_command factory lacked direct unit tests in its own package, relying only on indirect coverage from downstream packages. This adds tests exercising the factory with a minimal Click group for all shell types and error cases. Generated-By: Forge/20260407_145514_2280530_d26e63ff Co-Authored-By: Claude Opus 4.6 --- .../jumpstarter_cli_common/completion_test.py | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 python/packages/jumpstarter-cli-common/jumpstarter_cli_common/completion_test.py diff --git a/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/completion_test.py b/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/completion_test.py new file mode 100644 index 000000000..c8b146a15 --- /dev/null +++ b/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/completion_test.py @@ -0,0 +1,64 @@ +import click +from click.testing import CliRunner + +from .completion import make_completion_command + +PROG_NAME = "testcli" +COMPLETE_VAR = "_TESTCLI_COMPLETE" + + +def _make_test_group(): + @click.group() + def cli(): + pass + + return cli + + +def _make_test_cli_with_completion(): + @click.group() + def cli(): + pass + + cli.add_command(make_completion_command(_make_test_group, PROG_NAME, COMPLETE_VAR)) + return cli + + +def test_completion_bash_produces_completion_script(): + cli = _make_test_cli_with_completion() + runner = CliRunner() + result = runner.invoke(cli, ["completion", "bash"]) + assert result.exit_code == 0 + assert "complete" in result.output.lower() + assert PROG_NAME in result.output.lower() + + +def test_completion_zsh_produces_compdef(): + cli = _make_test_cli_with_completion() + runner = CliRunner() + result = runner.invoke(cli, ["completion", "zsh"]) + assert result.exit_code == 0 + assert "compdef" in result.output.lower() + + +def test_completion_fish_produces_complete_command(): + cli = _make_test_cli_with_completion() + runner = CliRunner() + result = runner.invoke(cli, ["completion", "fish"]) + assert result.exit_code == 0 + assert "complete" in result.output.lower() + assert f"--command {PROG_NAME}" in result.output.lower() + + +def test_completion_missing_argument_exits_with_error(): + cli = _make_test_cli_with_completion() + runner = CliRunner() + result = runner.invoke(cli, ["completion"]) + assert result.exit_code == 2 + + +def test_completion_unsupported_shell_exits_with_error(): + cli = _make_test_cli_with_completion() + runner = CliRunner() + result = runner.invoke(cli, ["completion", "powershell"]) + assert result.exit_code == 2 From 7364afd20c18611af7b5f8e9f115e48d748e1095 Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Tue, 7 Apr 2026 17:30:52 +0200 Subject: [PATCH 03/19] feat: add shell completion for jmp, jmp-admin and j in jmp shell Auto-source completions for jmp, jmp-admin, and j when entering jmp shell (bash, zsh, fish). For j, command names are baked into the shell init script at startup to avoid slow gRPC calls on every TAB press. Adds `j completion {bash,zsh,fish}` subcommand for standalone use, with a fast-path dispatch that avoids the full async stack. Catches SystemExit from Click's completion handler before it propagates through anyio task groups. Closes #35 Co-Authored-By: Claude Opus 4.6 --- .../jumpstarter-cli/jumpstarter_cli/j.py | 24 +++ .../jumpstarter_cli/j_completion_test.py | 54 ++++++ .../jumpstarter-cli/jumpstarter_cli/shell.py | 17 +- .../jumpstarter/jumpstarter/common/utils.py | 172 ++++++++++++------ .../jumpstarter/common/utils_test.py | 77 +++++++- 5 files changed, 282 insertions(+), 62 deletions(-) create mode 100644 python/packages/jumpstarter-cli/jumpstarter_cli/j_completion_test.py diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/j.py b/python/packages/jumpstarter-cli/jumpstarter_cli/j.py index c077fce51..a67cb36da 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/j.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/j.py @@ -1,4 +1,5 @@ import concurrent.futures._base +import os import sys from contextlib import ExitStack from typing import cast @@ -7,6 +8,7 @@ from anyio import create_task_group, get_cancelled_exc_class, run, to_thread from anyio.from_thread import BlockingPortal from click.exceptions import Exit as ClickExit +from jumpstarter_cli_common.completion import make_completion_command from jumpstarter_cli_common.exceptions import ( ClickExceptionRed, async_handle_exceptions, @@ -19,6 +21,22 @@ from jumpstarter.common.exceptions import EnvironmentVariableNotSetError from jumpstarter.utils.env import env_async +j_completion = make_completion_command(lambda: click.Group("j"), "j", "_J_COMPLETE") + + +async def _j_shell_complete(): + async with BlockingPortal() as portal: + with ExitStack() as stack: + async with env_async(portal, stack) as client: + + def _run_completion(): + try: + client.cli()(standalone_mode=False) + except SystemExit: + pass + + await to_thread.run_sync(_run_completion) + async def j_async(): @async_handle_exceptions @@ -60,6 +78,12 @@ async def cli(): def j(): traceback.install() + if len(sys.argv) >= 2 and sys.argv[1] == "completion": + j_completion(args=sys.argv[2:]) + return + if "_J_COMPLETE" in os.environ: + run(_j_shell_complete) + return run(j_async) diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/j_completion_test.py b/python/packages/jumpstarter-cli/jumpstarter_cli/j_completion_test.py new file mode 100644 index 000000000..2ac7a72ca --- /dev/null +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/j_completion_test.py @@ -0,0 +1,54 @@ +from unittest.mock import AsyncMock, MagicMock, patch + +from anyio import run +from click.testing import CliRunner + +from .j import _j_shell_complete, j_completion + + +def test_j_completion_bash_produces_script(): + runner = CliRunner() + result = runner.invoke(j_completion, ["bash"]) + assert result.exit_code == 0 + assert "complete" in result.output.lower() + assert "_J_COMPLETE" in result.output + + +def test_j_completion_zsh_produces_compdef(): + runner = CliRunner() + result = runner.invoke(j_completion, ["zsh"]) + assert result.exit_code == 0 + assert "compdef" in result.output.lower() + + +def test_j_completion_fish_produces_complete_command(): + runner = CliRunner() + result = runner.invoke(j_completion, ["fish"]) + assert result.exit_code == 0 + assert "complete" in result.output.lower() + assert "--command j" in result.output.lower() + + +def test_j_completion_no_args_exits_with_error(): + runner = CliRunner() + result = runner.invoke(j_completion, []) + assert result.exit_code == 2 + + +def test_j_completion_unsupported_shell_exits_with_error(): + runner = CliRunner() + result = runner.invoke(j_completion, ["powershell"]) + assert result.exit_code == 2 + + +def test_j_shell_complete_handles_system_exit_cleanly(): + mock_cli_group = MagicMock() + mock_cli_group.side_effect = SystemExit(0) + mock_client = MagicMock() + mock_client.cli.return_value = mock_cli_group + + with patch("jumpstarter_cli.j.env_async") as mock_env: + mock_env.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_env.return_value.__aexit__ = AsyncMock(return_value=False) + run(_j_shell_complete) + mock_client.cli.assert_called_once() diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py b/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py index 47e7952b5..b85f12c96 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py @@ -42,7 +42,7 @@ -def _run_shell_only(lease, config, command, path: str) -> int: +def _run_shell_only(lease, config, command, path: str, j_commands: list[str] | None = None) -> int: """Run just the shell command without log streaming.""" allow = config.drivers.allow if config is not None else getattr(lease, "allow", []) unsafe = config.drivers.unsafe if config is not None else getattr(lease, "unsafe", False) @@ -59,6 +59,7 @@ def _run_shell_only(lease, config, command, path: str) -> int: lease=lease, insecure=insecure, passphrase=passphrase, + j_commands=j_commands, ) @@ -324,8 +325,18 @@ async def _run_shell_with_lease_async(lease, exporter_logs, config, command, can warning_text = monitor.status_message[len(HOOK_WARNING_PREFIX) :] click.echo(click.style(f"Warning: {warning_text}", fg="yellow", bold=True)) - # Run the shell command - exit_code = await anyio.to_thread.run_sync(_run_shell_only, lease, config, command, path) + # Extract j command names for static shell completion + j_commands = None + try: + cli_group = client.cli() + if hasattr(cli_group, "list_commands"): + j_commands = cli_group.list_commands(None) + except Exception: + pass + + exit_code = await anyio.to_thread.run_sync( + _run_shell_only, lease, config, command, path, j_commands + ) # Shell has exited. For auto-created leases (release=True), call # EndSession to trigger afterLease hook while keeping log stream diff --git a/python/packages/jumpstarter/jumpstarter/common/utils.py b/python/packages/jumpstarter/jumpstarter/common/utils.py index 7a3e13921..0e4ca0049 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils.py @@ -1,6 +1,7 @@ import os import signal import sys +import tempfile from contextlib import ExitStack, asynccontextmanager, contextmanager from datetime import timedelta from functools import partial @@ -84,7 +85,51 @@ def _run_process( return process.wait() -def launch_shell( +def _generate_shell_init(shell_name: str, use_profiles: bool, j_commands: list[str] | None = None) -> str: + if shell_name.endswith("bash"): + lines = [] + if use_profiles: + lines.append('[ -f ~/.bashrc ] && source ~/.bashrc') + lines.append('eval "$(jmp completion bash 2>/dev/null)"') + lines.append('eval "$(jmp-admin completion bash 2>/dev/null)"') + if j_commands: + cmds = " ".join(j_commands) + lines.append( + f'_j_completion() {{ COMPREPLY=($(compgen -W "{cmds}" -- "${{COMP_WORDS[COMP_CWORD]}}")); }}' + ) + lines.append("complete -o default -F _j_completion j") + else: + lines.append('eval "$(j completion bash 2>/dev/null)"') + return "\n".join(lines) + "\n" + + elif shell_name == "zsh": + lines = [] + if use_profiles: + lines.append('[ -f ~/.zshrc ] && source ~/.zshrc') + lines.append('eval "$(jmp completion zsh 2>/dev/null)"') + lines.append('eval "$(jmp-admin completion zsh 2>/dev/null)"') + if j_commands: + cmds = " ".join(j_commands) + lines.append(f"compdef '_arguments \"1:(({cmds}))\"' j") + else: + lines.append('eval "$(j completion zsh 2>/dev/null)"') + return "\n".join(lines) + "\n" + + elif shell_name == "fish": + lines = [] + lines.append("jmp completion fish 2>/dev/null | source") + lines.append("jmp-admin completion fish 2>/dev/null | source") + if j_commands: + for cmd in j_commands: + lines.append(f"complete -c j -f -n '__fish_use_subcommand' -a {cmd}") + else: + lines.append("j completion fish 2>/dev/null | source") + return "\n".join(lines) + "\n" + + return "" + + +def launch_shell( # noqa: C901 host: str, context: str, allow: list[str], @@ -95,29 +140,16 @@ def launch_shell( lease=None, insecure: bool = False, passphrase: str | None = None, + j_commands: list[str] | None = None, ) -> int: - """Launch a shell with a custom prompt indicating the exporter type. - - Args: - host: The jumpstarter host path - context: The context of the shell (e.g. "local" or exporter name) - allow: List of allowed drivers - unsafe: Whether to allow drivers outside of the allow list - use_profiles: Whether to load shell profile files - command: Optional command to run instead of launching an interactive shell - lease: Optional Lease object to set up lease ending callback - - Returns: - The exit code of the shell or command process - """ + """Launch a shell with a custom prompt indicating the exporter type.""" shell = os.environ.get("SHELL", "bash") shell_name = os.path.basename(shell) - common_env = os.environ | { JUMPSTARTER_HOST: host, JMP_DRIVERS_ALLOW: "UNSAFE" if unsafe else ",".join(allow), - "_JMP_SUPPRESS_DRIVER_WARNINGS": "1", # Already warned during client initialization + "_JMP_SUPPRESS_DRIVER_WARNINGS": "1", } if insecure: common_env = common_env | {JMP_GRPC_INSECURE: "1"} @@ -127,44 +159,68 @@ def launch_shell( if command: return _run_process(list(command), common_env, lease) - if shell_name.endswith("bash"): - env = common_env | { - "PS1": f"{ANSI_GRAY}{PROMPT_CWD} {ANSI_YELLOW}⚡{ANSI_WHITE}{context} {ANSI_YELLOW}➤{ANSI_RESET} ", - } - cmd = [shell] - if not use_profiles: - cmd.extend(["--norc", "--noprofile"]) - return _run_process(cmd, env, lease) - - elif shell_name == "fish": - fish_fn = ( - "function fish_prompt; " - "set_color grey; " - 'printf "%s" (basename $PWD); ' - "set_color yellow; " - 'printf "⚡"; ' - "set_color white; " - f'printf "{context}"; ' - "set_color yellow; " - 'printf "➤ "; ' - "set_color normal; " - "end" - ) - cmd = [shell, "--init-command", fish_fn] - return _run_process(cmd, common_env, lease) - - elif shell_name == "zsh": - env = common_env | { - "PS1": f"%F{{8}}%1~ %F{{yellow}}⚡%F{{white}}{context} %F{{yellow}}➤%f ", - } - if "HISTFILE" not in env: - env["HISTFILE"] = os.path.join(os.path.expanduser("~"), ".zsh_history") - - cmd = [shell] - if not use_profiles: - cmd.append("--no-rcs") - cmd.extend(["-o", "inc_append_history", "-o", "share_history"]) - return _run_process(cmd, env, lease) - - else: - return _run_process([shell], common_env, lease) + init_content = _generate_shell_init(shell_name, use_profiles, j_commands) + init_file = None + if init_content: + init_file = tempfile.NamedTemporaryFile(mode="w", suffix=".sh", delete=False) + init_file.write(init_content) + init_file.close() + + try: + if shell_name.endswith("bash"): + env = common_env | { + "PS1": f"{ANSI_GRAY}{PROMPT_CWD} {ANSI_YELLOW}⚡{ANSI_WHITE}{context} {ANSI_YELLOW}➤{ANSI_RESET} ", + } + cmd = [shell] + if init_file: + cmd.extend(["--rcfile", init_file.name]) + elif not use_profiles: + cmd.extend(["--norc", "--noprofile"]) + return _run_process(cmd, env, lease) + + elif shell_name == "fish": + fish_fn = ( + "function fish_prompt; " + "set_color grey; " + 'printf "%s" (basename $PWD); ' + "set_color yellow; " + 'printf "⚡"; ' + "set_color white; " + f'printf "{context}"; ' + "set_color yellow; " + 'printf "➤ "; ' + "set_color normal; " + "end" + ) + init_cmd = fish_fn + if init_file: + init_cmd += f"; source {init_file.name}" + return _run_process([shell, "--init-command", init_cmd], common_env, lease) + + elif shell_name == "zsh": + env = common_env | { + "PS1": f"%F{{8}}%1~ %F{{yellow}}⚡%F{{white}}{context} %F{{yellow}}➤%f ", + } + if "HISTFILE" not in env: + env["HISTFILE"] = os.path.join(os.path.expanduser("~"), ".zsh_history") + cmd = [shell] + if init_file: + cmd.extend(["--rcs", "-o", "inc_append_history", "-o", "share_history"]) + env["ZDOTDIR"] = os.path.dirname(init_file.name) + zshrc_path = os.path.join(os.path.dirname(init_file.name), ".zshrc") + with open(zshrc_path, "w") as f: + f.write(init_content) + else: + if not use_profiles: + cmd.append("--no-rcs") + cmd.extend(["-o", "inc_append_history", "-o", "share_history"]) + return _run_process(cmd, env, lease) + + else: + return _run_process([shell], common_env, lease) + finally: + if init_file: + try: + os.unlink(init_file.name) + except OSError: + pass diff --git a/python/packages/jumpstarter/jumpstarter/common/utils_test.py b/python/packages/jumpstarter/jumpstarter/common/utils_test.py index 86cd52dfd..68cb2bcb4 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils_test.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils_test.py @@ -1,6 +1,6 @@ import shutil -from .utils import launch_shell +from .utils import _generate_shell_init, launch_shell def test_launch_shell(tmp_path, monkeypatch): @@ -22,3 +22,78 @@ def test_launch_shell(tmp_path, monkeypatch): use_profiles=False ) assert exit_code == 1 + + +def test_generate_bash_init_with_j_commands(): + content = _generate_shell_init("bash", use_profiles=False, j_commands=["power", "serial", "ssh"]) + assert "_j_completion" in content + assert "power serial ssh" in content + assert "jmp completion bash" in content + assert "jmp-admin completion bash" in content + assert ".bashrc" not in content + + +def test_generate_bash_init_with_profiles(): + content = _generate_shell_init("bash", use_profiles=True, j_commands=["power"]) + assert ".bashrc" in content + assert "_j_completion" in content + + +def test_generate_bash_init_without_j_commands(): + content = _generate_shell_init("bash", use_profiles=False, j_commands=None) + assert "j completion bash" in content + assert "_j_completion" not in content + + +def test_generate_zsh_init_with_j_commands(): + content = _generate_shell_init("zsh", use_profiles=False, j_commands=["power", "qemu"]) + assert "power qemu" in content + assert "jmp completion zsh" in content + assert "compdef" in content + + +def test_generate_bash_init_with_profiles_sources_bashrc(): + content = _generate_shell_init("bash", use_profiles=True, j_commands=None) + assert ".bashrc" in content + assert "j completion bash" in content + + +def test_generate_zsh_init_without_j_commands(): + content = _generate_shell_init("zsh", use_profiles=False, j_commands=None) + assert "j completion zsh" in content + assert "compdef" not in content + + +def test_generate_zsh_init_with_profiles_sources_zshrc(): + content = _generate_shell_init("zsh", use_profiles=True, j_commands=["power"]) + assert ".zshrc" in content + + +def test_generate_fish_init_with_j_commands(): + content = _generate_shell_init("fish", use_profiles=False, j_commands=["power", "qemu"]) + assert "power" in content + assert "qemu" in content + assert "jmp completion fish" in content + + +def test_generate_fish_init_without_j_commands(): + content = _generate_shell_init("fish", use_profiles=False, j_commands=None) + assert "j completion fish" in content + + +def test_generate_shell_init_unknown_shell(): + content = _generate_shell_init("csh", use_profiles=False, j_commands=["power"]) + assert content == "" + + +def test_launch_shell_with_j_commands(tmp_path, monkeypatch): + monkeypatch.setenv("SHELL", shutil.which("true")) + exit_code = launch_shell( + host=str(tmp_path / "test.sock"), + context="remote", + allow=["*"], + unsafe=False, + use_profiles=False, + j_commands=["power", "serial"], + ) + assert exit_code == 0 From 724391d34f4ffecdb1c4491d2166c83b07ec1635 Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Tue, 7 Apr 2026 18:04:44 +0200 Subject: [PATCH 04/19] fix: address shell completion review findings - Fix zsh temp file leak by cleaning up .zshrc in _launch_zsh finally block (F001) - Validate j_commands against safe pattern to prevent shell injection (F002) - Refactor launch_shell into per-shell helpers to reduce complexity (F003) - Add debug logging when j command extraction fails (F004) - Add test for zsh temp file cleanup in launch_shell (F005) - Add 5-second timeout to _j_shell_complete to prevent shell freezes (F006) - Rename admin completion tests to describe expected behavior (F007) Co-Authored-By: Claude Opus 4.6 --- .../jumpstarter_cli_admin/completion_test.py | 10 +- .../jumpstarter-cli/jumpstarter_cli/j.py | 26 ++-- .../jumpstarter_cli/j_completion_test.py | 19 ++- .../jumpstarter-cli/jumpstarter_cli/shell.py | 4 +- .../jumpstarter/jumpstarter/common/utils.py | 124 +++++++++++------- .../jumpstarter/common/utils_test.py | 48 ++++++- 6 files changed, 164 insertions(+), 67 deletions(-) diff --git a/python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/completion_test.py b/python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/completion_test.py index b714ab9cc..3a7fd03c9 100644 --- a/python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/completion_test.py +++ b/python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/completion_test.py @@ -3,7 +3,7 @@ from . import admin -def test_completion_bash(): +def test_completion_bash_produces_script_with_jmp_admin(): runner = CliRunner() result = runner.invoke(admin, ["completion", "bash"]) assert result.exit_code == 0 @@ -12,7 +12,7 @@ def test_completion_bash(): assert "jmp-admin" in result.output.lower() -def test_completion_zsh(): +def test_completion_zsh_produces_compdef_for_jmp_admin(): runner = CliRunner() result = runner.invoke(admin, ["completion", "zsh"]) assert result.exit_code == 0 @@ -20,7 +20,7 @@ def test_completion_zsh(): assert "compdef" in result.output.lower() -def test_completion_fish(): +def test_completion_fish_produces_complete_command_for_jmp_admin(): runner = CliRunner() result = runner.invoke(admin, ["completion", "fish"]) assert result.exit_code == 0 @@ -29,14 +29,14 @@ def test_completion_fish(): assert "--command jmp-admin" in result.output.lower() -def test_completion_no_args(): +def test_completion_missing_argument_exits_with_error(): runner = CliRunner() result = runner.invoke(admin, ["completion"]) assert result.exit_code == 2 assert "Missing argument" in result.output or "bash" in result.output -def test_completion_unsupported_shell(): +def test_completion_unsupported_shell_exits_with_error(): runner = CliRunner() result = runner.invoke(admin, ["completion", "powershell"]) assert result.exit_code == 2 diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/j.py b/python/packages/jumpstarter-cli/jumpstarter_cli/j.py index a67cb36da..7ddf6627e 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/j.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/j.py @@ -4,6 +4,7 @@ from contextlib import ExitStack from typing import cast +import anyio import click from anyio import create_task_group, get_cancelled_exc_class, run, to_thread from anyio.from_thread import BlockingPortal @@ -24,18 +25,25 @@ j_completion = make_completion_command(lambda: click.Group("j"), "j", "_J_COMPLETE") +_COMPLETION_TIMEOUT_SECONDS = 5 + + async def _j_shell_complete(): - async with BlockingPortal() as portal: - with ExitStack() as stack: - async with env_async(portal, stack) as client: + try: + with anyio.fail_after(_COMPLETION_TIMEOUT_SECONDS): + async with BlockingPortal() as portal: + with ExitStack() as stack: + async with env_async(portal, stack) as client: - def _run_completion(): - try: - client.cli()(standalone_mode=False) - except SystemExit: - pass + def _run_completion(): + try: + client.cli()(standalone_mode=False) + except SystemExit: + pass - await to_thread.run_sync(_run_completion) + await to_thread.run_sync(_run_completion) + except TimeoutError: + pass async def j_async(): diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/j_completion_test.py b/python/packages/jumpstarter-cli/jumpstarter_cli/j_completion_test.py index 2ac7a72ca..08d9490cf 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/j_completion_test.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/j_completion_test.py @@ -1,9 +1,10 @@ from unittest.mock import AsyncMock, MagicMock, patch +import anyio from anyio import run from click.testing import CliRunner -from .j import _j_shell_complete, j_completion +from .j import _COMPLETION_TIMEOUT_SECONDS, _j_shell_complete, j_completion def test_j_completion_bash_produces_script(): @@ -52,3 +53,19 @@ def test_j_shell_complete_handles_system_exit_cleanly(): mock_env.return_value.__aexit__ = AsyncMock(return_value=False) run(_j_shell_complete) mock_client.cli.assert_called_once() + + +def test_j_shell_complete_returns_empty_on_timeout(): + from contextlib import asynccontextmanager + + @asynccontextmanager + async def slow_env(*args, **kwargs): + await anyio.sleep(_COMPLETION_TIMEOUT_SECONDS + 1) + yield MagicMock() + + with patch("jumpstarter_cli.j.env_async", slow_env): + run(_j_shell_complete) + + +def test_completion_timeout_is_positive(): + assert _COMPLETION_TIMEOUT_SECONDS > 0 diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py b/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py index b85f12c96..8ded3d837 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py @@ -331,8 +331,8 @@ async def _run_shell_with_lease_async(lease, exporter_logs, config, command, can cli_group = client.cli() if hasattr(cli_group, "list_commands"): j_commands = cli_group.list_commands(None) - except Exception: - pass + except Exception as e: + logger.debug("Failed to extract j commands for completion: %s", e) exit_code = await anyio.to_thread.run_sync( _run_shell_only, lease, config, command, path, j_commands diff --git a/python/packages/jumpstarter/jumpstarter/common/utils.py b/python/packages/jumpstarter/jumpstarter/common/utils.py index 0e4ca0049..80594cebe 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils.py @@ -1,4 +1,5 @@ import os +import re import signal import sys import tempfile @@ -85,7 +86,17 @@ def _run_process( return process.wait() +_SAFE_COMMAND_NAME = re.compile(r"^[a-zA-Z0-9_-]+$") + + +def _validate_j_commands(j_commands: list[str] | None) -> list[str] | None: + if j_commands is None: + return None + return [cmd for cmd in j_commands if _SAFE_COMMAND_NAME.match(cmd)] + + def _generate_shell_init(shell_name: str, use_profiles: bool, j_commands: list[str] | None = None) -> str: + j_commands = _validate_j_commands(j_commands) if shell_name.endswith("bash"): lines = [] if use_profiles: @@ -129,7 +140,67 @@ def _generate_shell_init(shell_name: str, use_profiles: bool, j_commands: list[s return "" -def launch_shell( # noqa: C901 +def _launch_bash(shell, init_file, use_profiles, common_env, context, lease): + env = common_env | { + "PS1": f"{ANSI_GRAY}{PROMPT_CWD} {ANSI_YELLOW}⚡{ANSI_WHITE}{context} {ANSI_YELLOW}➤{ANSI_RESET} ", + } + cmd = [shell] + if init_file: + cmd.extend(["--rcfile", init_file.name]) + elif not use_profiles: + cmd.extend(["--norc", "--noprofile"]) + return _run_process(cmd, env, lease) + + +def _launch_fish(shell, init_file, common_env, context, lease): + fish_fn = ( + "function fish_prompt; " + "set_color grey; " + 'printf "%s" (basename $PWD); ' + "set_color yellow; " + 'printf "⚡"; ' + "set_color white; " + f'printf "{context}"; ' + "set_color yellow; " + 'printf "➤ "; ' + "set_color normal; " + "end" + ) + init_cmd = fish_fn + if init_file: + init_cmd += f"; source {init_file.name}" + return _run_process([shell, "--init-command", init_cmd], common_env, lease) + + +def _launch_zsh(shell, init_file, init_content, common_env, context, lease, use_profiles): + env = common_env | { + "PS1": f"%F{{8}}%1~ %F{{yellow}}⚡%F{{white}}{context} %F{{yellow}}➤%f ", + } + if "HISTFILE" not in env: + env["HISTFILE"] = os.path.join(os.path.expanduser("~"), ".zsh_history") + cmd = [shell] + zshrc_path = None + if init_file: + cmd.extend(["--rcs", "-o", "inc_append_history", "-o", "share_history"]) + env["ZDOTDIR"] = os.path.dirname(init_file.name) + zshrc_path = os.path.join(os.path.dirname(init_file.name), ".zshrc") + with open(zshrc_path, "w") as f: + f.write(init_content) + else: + if not use_profiles: + cmd.append("--no-rcs") + cmd.extend(["-o", "inc_append_history", "-o", "share_history"]) + try: + return _run_process(cmd, env, lease) + finally: + if zshrc_path: + try: + os.unlink(zshrc_path) + except OSError: + pass + + +def launch_shell( host: str, context: str, allow: list[str], @@ -142,8 +213,6 @@ def launch_shell( # noqa: C901 passphrase: str | None = None, j_commands: list[str] | None = None, ) -> int: - """Launch a shell with a custom prompt indicating the exporter type.""" - shell = os.environ.get("SHELL", "bash") shell_name = os.path.basename(shell) common_env = os.environ | { @@ -168,54 +237,11 @@ def launch_shell( # noqa: C901 try: if shell_name.endswith("bash"): - env = common_env | { - "PS1": f"{ANSI_GRAY}{PROMPT_CWD} {ANSI_YELLOW}⚡{ANSI_WHITE}{context} {ANSI_YELLOW}➤{ANSI_RESET} ", - } - cmd = [shell] - if init_file: - cmd.extend(["--rcfile", init_file.name]) - elif not use_profiles: - cmd.extend(["--norc", "--noprofile"]) - return _run_process(cmd, env, lease) - + return _launch_bash(shell, init_file, use_profiles, common_env, context, lease) elif shell_name == "fish": - fish_fn = ( - "function fish_prompt; " - "set_color grey; " - 'printf "%s" (basename $PWD); ' - "set_color yellow; " - 'printf "⚡"; ' - "set_color white; " - f'printf "{context}"; ' - "set_color yellow; " - 'printf "➤ "; ' - "set_color normal; " - "end" - ) - init_cmd = fish_fn - if init_file: - init_cmd += f"; source {init_file.name}" - return _run_process([shell, "--init-command", init_cmd], common_env, lease) - + return _launch_fish(shell, init_file, common_env, context, lease) elif shell_name == "zsh": - env = common_env | { - "PS1": f"%F{{8}}%1~ %F{{yellow}}⚡%F{{white}}{context} %F{{yellow}}➤%f ", - } - if "HISTFILE" not in env: - env["HISTFILE"] = os.path.join(os.path.expanduser("~"), ".zsh_history") - cmd = [shell] - if init_file: - cmd.extend(["--rcs", "-o", "inc_append_history", "-o", "share_history"]) - env["ZDOTDIR"] = os.path.dirname(init_file.name) - zshrc_path = os.path.join(os.path.dirname(init_file.name), ".zshrc") - with open(zshrc_path, "w") as f: - f.write(init_content) - else: - if not use_profiles: - cmd.append("--no-rcs") - cmd.extend(["-o", "inc_append_history", "-o", "share_history"]) - return _run_process(cmd, env, lease) - + return _launch_zsh(shell, init_file, init_content, common_env, context, lease, use_profiles) else: return _run_process([shell], common_env, lease) finally: diff --git a/python/packages/jumpstarter/jumpstarter/common/utils_test.py b/python/packages/jumpstarter/jumpstarter/common/utils_test.py index 68cb2bcb4..9ea0a096e 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils_test.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils_test.py @@ -1,6 +1,8 @@ +import os import shutil +from unittest.mock import patch -from .utils import _generate_shell_init, launch_shell +from .utils import _generate_shell_init, _validate_j_commands, launch_shell def test_launch_shell(tmp_path, monkeypatch): @@ -97,3 +99,47 @@ def test_launch_shell_with_j_commands(tmp_path, monkeypatch): j_commands=["power", "serial"], ) assert exit_code == 0 + + +def test_validate_j_commands_filters_unsafe_names(): + assert _validate_j_commands(None) is None + assert _validate_j_commands(["power", "serial"]) == ["power", "serial"] + assert _validate_j_commands(["good-cmd", "good_cmd"]) == ["good-cmd", "good_cmd"] + assert _validate_j_commands(["$(evil)", "power"]) == ["power"] + assert _validate_j_commands(["bad;cmd", "ok"]) == ["ok"] + assert _validate_j_commands(["bad cmd", "ok"]) == ["ok"] + assert _validate_j_commands(['"injection', "ok"]) == ["ok"] + + +def test_generate_shell_init_excludes_unsafe_j_commands(): + content = _generate_shell_init("bash", use_profiles=False, j_commands=["power", "$(evil)", "serial"]) + assert "power" in content + assert "serial" in content + assert "$(evil)" not in content + + +def test_launch_shell_zsh_cleans_up_all_temp_files(tmp_path, monkeypatch): + monkeypatch.setenv("SHELL", "/usr/bin/zsh") + zshrc_paths = [] + + def mock_run_process(cmd, env, lease=None): + zdotdir = env.get("ZDOTDIR") + if zdotdir: + zshrc = os.path.join(zdotdir, ".zshrc") + zshrc_paths.append(zshrc) + assert os.path.exists(zshrc) + return 0 + + with patch("jumpstarter.common.utils._run_process", mock_run_process): + exit_code = launch_shell( + host=str(tmp_path / "test.sock"), + context="remote", + allow=["*"], + unsafe=False, + use_profiles=False, + j_commands=["power", "serial"], + ) + assert exit_code == 0 + + assert len(zshrc_paths) == 1 + assert not os.path.exists(zshrc_paths[0]) From 3035ec51821ab33aef4263afd342b8c40ed110fd Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Tue, 7 Apr 2026 20:16:39 +0200 Subject: [PATCH 05/19] fix: address remaining shell completion review findings - Add None guard for get_completion_class return value (F001) - Simplify zsh temp file handling with mkdtemp, eliminating unnecessary intermediate file (F002) - Quote fish completion argument for defense-in-depth (F003) Co-Authored-By: Claude Opus 4.6 --- .../jumpstarter_cli_common/completion.py | 2 ++ .../jumpstarter/jumpstarter/common/utils.py | 30 ++++++++++--------- .../jumpstarter/common/utils_test.py | 30 +++++++++++++++++-- 3 files changed, 46 insertions(+), 16 deletions(-) diff --git a/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/completion.py b/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/completion.py index c9841ddbd..0f16f62b5 100644 --- a/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/completion.py +++ b/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/completion.py @@ -11,6 +11,8 @@ def completion(shell: str): """Generate shell completion script.""" cli_group = cli_group_factory() comp_cls = get_completion_class(shell) + if comp_cls is None: + raise click.ClickException(f"Unsupported shell: {shell}") comp = comp_cls(cli_group, {}, prog_name, complete_var) click.echo(comp.source()) diff --git a/python/packages/jumpstarter/jumpstarter/common/utils.py b/python/packages/jumpstarter/jumpstarter/common/utils.py index 80594cebe..a8999f441 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils.py @@ -132,7 +132,7 @@ def _generate_shell_init(shell_name: str, use_profiles: bool, j_commands: list[s lines.append("jmp-admin completion fish 2>/dev/null | source") if j_commands: for cmd in j_commands: - lines.append(f"complete -c j -f -n '__fish_use_subcommand' -a {cmd}") + lines.append(f"complete -c j -f -n '__fish_use_subcommand' -a '{cmd}'") else: lines.append("j completion fish 2>/dev/null | source") return "\n".join(lines) + "\n" @@ -172,20 +172,21 @@ def _launch_fish(shell, init_file, common_env, context, lease): return _run_process([shell, "--init-command", init_cmd], common_env, lease) -def _launch_zsh(shell, init_file, init_content, common_env, context, lease, use_profiles): +def _launch_zsh(shell, init_content, common_env, context, lease, use_profiles): env = common_env | { "PS1": f"%F{{8}}%1~ %F{{yellow}}⚡%F{{white}}{context} %F{{yellow}}➤%f ", } if "HISTFILE" not in env: env["HISTFILE"] = os.path.join(os.path.expanduser("~"), ".zsh_history") cmd = [shell] - zshrc_path = None - if init_file: - cmd.extend(["--rcs", "-o", "inc_append_history", "-o", "share_history"]) - env["ZDOTDIR"] = os.path.dirname(init_file.name) - zshrc_path = os.path.join(os.path.dirname(init_file.name), ".zshrc") + tmpdir = None + if init_content: + tmpdir = tempfile.mkdtemp() + zshrc_path = os.path.join(tmpdir, ".zshrc") with open(zshrc_path, "w") as f: f.write(init_content) + cmd.extend(["--rcs", "-o", "inc_append_history", "-o", "share_history"]) + env["ZDOTDIR"] = tmpdir else: if not use_profiles: cmd.append("--no-rcs") @@ -193,11 +194,10 @@ def _launch_zsh(shell, init_file, init_content, common_env, context, lease, use_ try: return _run_process(cmd, env, lease) finally: - if zshrc_path: - try: - os.unlink(zshrc_path) - except OSError: - pass + if tmpdir: + import shutil + + shutil.rmtree(tmpdir, ignore_errors=True) def launch_shell( @@ -229,6 +229,10 @@ def launch_shell( return _run_process(list(command), common_env, lease) init_content = _generate_shell_init(shell_name, use_profiles, j_commands) + + if shell_name == "zsh": + return _launch_zsh(shell, init_content, common_env, context, lease, use_profiles) + init_file = None if init_content: init_file = tempfile.NamedTemporaryFile(mode="w", suffix=".sh", delete=False) @@ -240,8 +244,6 @@ def launch_shell( return _launch_bash(shell, init_file, use_profiles, common_env, context, lease) elif shell_name == "fish": return _launch_fish(shell, init_file, common_env, context, lease) - elif shell_name == "zsh": - return _launch_zsh(shell, init_file, init_content, common_env, context, lease, use_profiles) else: return _run_process([shell], common_env, lease) finally: diff --git a/python/packages/jumpstarter/jumpstarter/common/utils_test.py b/python/packages/jumpstarter/jumpstarter/common/utils_test.py index 9ea0a096e..a2ef645a7 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils_test.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils_test.py @@ -73,8 +73,8 @@ def test_generate_zsh_init_with_profiles_sources_zshrc(): def test_generate_fish_init_with_j_commands(): content = _generate_shell_init("fish", use_profiles=False, j_commands=["power", "qemu"]) - assert "power" in content - assert "qemu" in content + assert "'power'" in content + assert "'qemu'" in content assert "jmp completion fish" in content @@ -143,3 +143,29 @@ def mock_run_process(cmd, env, lease=None): assert len(zshrc_paths) == 1 assert not os.path.exists(zshrc_paths[0]) + + +def test_launch_shell_zsh_uses_tmpdir_without_intermediate_file(tmp_path, monkeypatch): + monkeypatch.setenv("SHELL", "/usr/bin/zsh") + temp_dirs = [] + + def mock_run_process(cmd, env, lease=None): + zdotdir = env.get("ZDOTDIR") + if zdotdir: + temp_dirs.append(zdotdir) + entries = os.listdir(zdotdir) + assert entries == [".zshrc"], f"Expected only .zshrc in ZDOTDIR, found: {entries}" + return 0 + + with patch("jumpstarter.common.utils._run_process", mock_run_process): + launch_shell( + host=str(tmp_path / "test.sock"), + context="remote", + allow=["*"], + unsafe=False, + use_profiles=False, + j_commands=["power"], + ) + + assert len(temp_dirs) == 1 + assert not os.path.exists(temp_dirs[0]) From fefea1f6cd045a597fb3f93e76606a44d5e4941e Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Wed, 8 Apr 2026 14:31:20 +0200 Subject: [PATCH 06/19] fix: address CodeRabbit review findings on shell completion - Fix shell injection in fish prompt by passing context via env var - Add COMP_CWORD guard to limit bash completion to first argument - Restore original ZDOTDIR in generated zshrc to preserve user config - Add abandon_on_cancel=True to completion thread for clean timeout - Strengthen test assertions for completion and timeout behavior Co-Authored-By: Claude Opus 4.6 --- .../jumpstarter-cli/jumpstarter_cli/j.py | 2 +- .../jumpstarter_cli/j_completion_test.py | 4 +- .../jumpstarter/jumpstarter/common/utils.py | 14 +++-- .../jumpstarter/common/utils_test.py | 56 +++++++++++++++++++ 4 files changed, 70 insertions(+), 6 deletions(-) diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/j.py b/python/packages/jumpstarter-cli/jumpstarter_cli/j.py index 7ddf6627e..e9fc93d57 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/j.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/j.py @@ -41,7 +41,7 @@ def _run_completion(): except SystemExit: pass - await to_thread.run_sync(_run_completion) + await to_thread.run_sync(_run_completion, abandon_on_cancel=True) except TimeoutError: pass diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/j_completion_test.py b/python/packages/jumpstarter-cli/jumpstarter_cli/j_completion_test.py index 08d9490cf..23b7030c6 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/j_completion_test.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/j_completion_test.py @@ -53,6 +53,7 @@ def test_j_shell_complete_handles_system_exit_cleanly(): mock_env.return_value.__aexit__ = AsyncMock(return_value=False) run(_j_shell_complete) mock_client.cli.assert_called_once() + mock_cli_group.assert_called_once() def test_j_shell_complete_returns_empty_on_timeout(): @@ -64,7 +65,8 @@ async def slow_env(*args, **kwargs): yield MagicMock() with patch("jumpstarter_cli.j.env_async", slow_env): - run(_j_shell_complete) + result = run(_j_shell_complete) + assert result is None def test_completion_timeout_is_positive(): diff --git a/python/packages/jumpstarter/jumpstarter/common/utils.py b/python/packages/jumpstarter/jumpstarter/common/utils.py index a8999f441..0a998bed0 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils.py @@ -1,5 +1,6 @@ import os import re +import shlex import signal import sys import tempfile @@ -105,9 +106,11 @@ def _generate_shell_init(shell_name: str, use_profiles: bool, j_commands: list[s lines.append('eval "$(jmp-admin completion bash 2>/dev/null)"') if j_commands: cmds = " ".join(j_commands) - lines.append( - f'_j_completion() {{ COMPREPLY=($(compgen -W "{cmds}" -- "${{COMP_WORDS[COMP_CWORD]}}")); }}' + completion_fn = ( + f'_j_completion() {{ [[ ${{COMP_CWORD}} -eq 1 ]]' + f' && COMPREPLY=($(compgen -W "{cmds}" -- "${{COMP_WORDS[COMP_CWORD]}}")); }}' ) + lines.append(completion_fn) lines.append("complete -o default -F _j_completion j") else: lines.append('eval "$(j completion bash 2>/dev/null)"') @@ -153,6 +156,7 @@ def _launch_bash(shell, init_file, use_profiles, common_env, context, lease): def _launch_fish(shell, init_file, common_env, context, lease): + fish_env = common_env | {"_JMP_SHELL_CONTEXT": context} fish_fn = ( "function fish_prompt; " "set_color grey; " @@ -160,7 +164,7 @@ def _launch_fish(shell, init_file, common_env, context, lease): "set_color yellow; " 'printf "⚡"; ' "set_color white; " - f'printf "{context}"; ' + 'printf "%s" "$_JMP_SHELL_CONTEXT"; ' "set_color yellow; " 'printf "➤ "; ' "set_color normal; " @@ -169,7 +173,7 @@ def _launch_fish(shell, init_file, common_env, context, lease): init_cmd = fish_fn if init_file: init_cmd += f"; source {init_file.name}" - return _run_process([shell, "--init-command", init_cmd], common_env, lease) + return _run_process([shell, "--init-command", init_cmd], fish_env, lease) def _launch_zsh(shell, init_content, common_env, context, lease, use_profiles): @@ -183,7 +187,9 @@ def _launch_zsh(shell, init_content, common_env, context, lease, use_profiles): if init_content: tmpdir = tempfile.mkdtemp() zshrc_path = os.path.join(tmpdir, ".zshrc") + original_zdotdir = env.get("ZDOTDIR", os.path.expanduser("~")) with open(zshrc_path, "w") as f: + f.write(f"ZDOTDIR={shlex.quote(original_zdotdir)}\n") f.write(init_content) cmd.extend(["--rcs", "-o", "inc_append_history", "-o", "share_history"]) env["ZDOTDIR"] = tmpdir diff --git a/python/packages/jumpstarter/jumpstarter/common/utils_test.py b/python/packages/jumpstarter/jumpstarter/common/utils_test.py index a2ef645a7..805f805bd 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils_test.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils_test.py @@ -145,6 +145,62 @@ def mock_run_process(cmd, env, lease=None): assert not os.path.exists(zshrc_paths[0]) +def test_launch_fish_passes_context_via_env(tmp_path, monkeypatch): + monkeypatch.setenv("SHELL", "/usr/bin/fish") + captured_env = {} + captured_cmd = [] + + def mock_run_process(cmd, env, lease=None): + captured_env.update(env) + captured_cmd.extend(cmd) + return 0 + + context = "test-context" + with patch("jumpstarter.common.utils._run_process", mock_run_process): + launch_shell( + host=str(tmp_path / "test.sock"), + context=context, + allow=["*"], + unsafe=False, + use_profiles=False, + ) + + assert captured_env.get("_JMP_SHELL_CONTEXT") == context + init_cmd_arg = captured_cmd[captured_cmd.index("--init-command") + 1] + assert context not in init_cmd_arg + + +def test_generate_bash_init_limits_completion_to_first_arg(): + content = _generate_shell_init("bash", use_profiles=False, j_commands=["power", "serial"]) + assert "COMP_CWORD" in content + assert "-eq 1" in content + + +def test_launch_shell_zsh_restores_zdotdir(tmp_path, monkeypatch): + monkeypatch.setenv("SHELL", "/usr/bin/zsh") + home_dir = os.path.expanduser("~") + + def mock_run_process(cmd, env, lease=None): + zdotdir = env.get("ZDOTDIR") + if zdotdir: + zshrc = os.path.join(zdotdir, ".zshrc") + with open(zshrc) as f: + first_line = f.readline().strip() + assert "ZDOTDIR=" in first_line + assert home_dir in first_line + return 0 + + with patch("jumpstarter.common.utils._run_process", mock_run_process): + launch_shell( + host=str(tmp_path / "test.sock"), + context="remote", + allow=["*"], + unsafe=False, + use_profiles=False, + j_commands=["power"], + ) + + def test_launch_shell_zsh_uses_tmpdir_without_intermediate_file(tmp_path, monkeypatch): monkeypatch.setenv("SHELL", "/usr/bin/zsh") temp_dirs = [] From 1358556f59a23971add5fa9650358f8273d751e4 Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Wed, 8 Apr 2026 19:03:00 +0200 Subject: [PATCH 07/19] fix: initialize zsh completion system before using compdef The zsh init script used compdef without ensuring compinit had been called. On macOS zsh where compinit is not loaded by /etc/zshrc, this caused "command not found: compdef" errors. Co-Authored-By: Claude Opus 4.6 --- .../jumpstarter/jumpstarter/common/utils.py | 1 + .../jumpstarter/common/utils_test.py | 28 ++++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/python/packages/jumpstarter/jumpstarter/common/utils.py b/python/packages/jumpstarter/jumpstarter/common/utils.py index 0a998bed0..a0d34b0ce 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils.py @@ -120,6 +120,7 @@ def _generate_shell_init(shell_name: str, use_profiles: bool, j_commands: list[s lines = [] if use_profiles: lines.append('[ -f ~/.zshrc ] && source ~/.zshrc') + lines.append("autoload -Uz compinit && compinit") lines.append('eval "$(jmp completion zsh 2>/dev/null)"') lines.append('eval "$(jmp-admin completion zsh 2>/dev/null)"') if j_commands: diff --git a/python/packages/jumpstarter/jumpstarter/common/utils_test.py b/python/packages/jumpstarter/jumpstarter/common/utils_test.py index 805f805bd..9a34257c6 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils_test.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils_test.py @@ -54,6 +54,29 @@ def test_generate_zsh_init_with_j_commands(): assert "compdef" in content +def test_generate_zsh_init_loads_compinit_before_completions(): + content = _generate_shell_init("zsh", use_profiles=False, j_commands=["power"]) + assert "autoload -Uz compinit && compinit" in content + compinit_pos = content.index("autoload -Uz compinit && compinit") + eval_jmp_pos = content.index('eval "$(jmp completion zsh') + assert compinit_pos < eval_jmp_pos + + +def test_generate_zsh_init_loads_compinit_before_compdef(): + content = _generate_shell_init("zsh", use_profiles=False, j_commands=["power", "qemu"]) + compinit_pos = content.index("autoload -Uz compinit && compinit") + compdef_pos = content.index("compdef") + assert compinit_pos < compdef_pos + + +def test_generate_zsh_init_without_j_commands_loads_compinit(): + content = _generate_shell_init("zsh", use_profiles=False, j_commands=None) + assert "autoload -Uz compinit && compinit" in content + compinit_pos = content.index("autoload -Uz compinit && compinit") + eval_jmp_pos = content.index('eval "$(jmp completion zsh') + assert compinit_pos < eval_jmp_pos + + def test_generate_bash_init_with_profiles_sources_bashrc(): content = _generate_shell_init("bash", use_profiles=True, j_commands=None) assert ".bashrc" in content @@ -66,9 +89,12 @@ def test_generate_zsh_init_without_j_commands(): assert "compdef" not in content -def test_generate_zsh_init_with_profiles_sources_zshrc(): +def test_generate_zsh_init_with_profiles_loads_compinit_after_zshrc(): content = _generate_shell_init("zsh", use_profiles=True, j_commands=["power"]) assert ".zshrc" in content + zshrc_pos = content.index(".zshrc") + compinit_pos = content.index("autoload -Uz compinit && compinit") + assert zshrc_pos < compinit_pos def test_generate_fish_init_with_j_commands(): From ce5026fc4e6a2f55cb0d5358aea33775f8497e57 Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Thu, 9 Apr 2026 07:39:50 +0200 Subject: [PATCH 08/19] fix: address nitpick review findings on shell completion Co-Authored-By: Claude Opus 4.6 --- .../jumpstarter/jumpstarter/common/utils.py | 11 ++++---- .../jumpstarter/common/utils_test.py | 25 +++++++++++++++++++ 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/python/packages/jumpstarter/jumpstarter/common/utils.py b/python/packages/jumpstarter/jumpstarter/common/utils.py index a0d34b0ce..867e273fe 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils.py @@ -116,7 +116,7 @@ def _generate_shell_init(shell_name: str, use_profiles: bool, j_commands: list[s lines.append('eval "$(j completion bash 2>/dev/null)"') return "\n".join(lines) + "\n" - elif shell_name == "zsh": + elif shell_name.endswith("zsh"): lines = [] if use_profiles: lines.append('[ -f ~/.zshrc ] && source ~/.zshrc') @@ -130,7 +130,7 @@ def _generate_shell_init(shell_name: str, use_profiles: bool, j_commands: list[s lines.append('eval "$(j completion zsh 2>/dev/null)"') return "\n".join(lines) + "\n" - elif shell_name == "fish": + elif shell_name.endswith("fish"): lines = [] lines.append("jmp completion fish 2>/dev/null | source") lines.append("jmp-admin completion fish 2>/dev/null | source") @@ -173,7 +173,8 @@ def _launch_fish(shell, init_file, common_env, context, lease): ) init_cmd = fish_fn if init_file: - init_cmd += f"; source {init_file.name}" + fish_env["_JMP_SHELL_INIT"] = init_file.name + init_cmd += '; source "$_JMP_SHELL_INIT"' return _run_process([shell, "--init-command", init_cmd], fish_env, lease) @@ -237,7 +238,7 @@ def launch_shell( init_content = _generate_shell_init(shell_name, use_profiles, j_commands) - if shell_name == "zsh": + if shell_name.endswith("zsh"): return _launch_zsh(shell, init_content, common_env, context, lease, use_profiles) init_file = None @@ -249,7 +250,7 @@ def launch_shell( try: if shell_name.endswith("bash"): return _launch_bash(shell, init_file, use_profiles, common_env, context, lease) - elif shell_name == "fish": + elif shell_name.endswith("fish"): return _launch_fish(shell, init_file, common_env, context, lease) else: return _run_process([shell], common_env, lease) diff --git a/python/packages/jumpstarter/jumpstarter/common/utils_test.py b/python/packages/jumpstarter/jumpstarter/common/utils_test.py index 9a34257c6..6651e504b 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils_test.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils_test.py @@ -196,6 +196,31 @@ def mock_run_process(cmd, env, lease=None): assert context not in init_cmd_arg +def test_launch_fish_passes_init_file_via_env(tmp_path, monkeypatch): + monkeypatch.setenv("SHELL", "/usr/bin/fish") + captured_env = {} + captured_cmd = [] + + def mock_run_process(cmd, env, lease=None): + captured_env.update(env) + captured_cmd.extend(cmd) + return 0 + + with patch("jumpstarter.common.utils._run_process", mock_run_process): + launch_shell( + host=str(tmp_path / "test.sock"), + context="remote", + allow=["*"], + unsafe=False, + use_profiles=False, + j_commands=["power"], + ) + + assert "_JMP_SHELL_INIT" in captured_env + init_cmd_arg = captured_cmd[captured_cmd.index("--init-command") + 1] + assert captured_env["_JMP_SHELL_INIT"] not in init_cmd_arg + + def test_generate_bash_init_limits_completion_to_first_arg(): content = _generate_shell_init("bash", use_profiles=False, j_commands=["power", "serial"]) assert "COMP_CWORD" in content From cb783ba4f0c1b7a087f20cdaa3a5c81aad812d47 Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Thu, 9 Apr 2026 09:05:24 +0200 Subject: [PATCH 09/19] test: add shell integration tests for bash, zsh, and fish Co-Authored-By: Claude Opus 4.6 --- .github/workflows/python-tests.yaml | 9 ++- .../jumpstarter/common/utils_test.py | 56 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/.github/workflows/python-tests.yaml b/.github/workflows/python-tests.yaml index ef127eb79..9dc9e8e7b 100644 --- a/.github/workflows/python-tests.yaml +++ b/.github/workflows/python-tests.yaml @@ -72,11 +72,11 @@ jobs: sudo apt-get update sudo apt-get install -y qemu-system-arm qemu-system-x86 - - name: Install libgpiod-dev (Linux) + - name: Install libgpiod-dev and fish (Linux) if: runner.os == 'Linux' run: | sudo apt-get update - sudo apt-get install -y libgpiod-dev liblgpio-dev + sudo apt-get install -y libgpiod-dev liblgpio-dev fish - name: Install Renode (Linux) if: runner.os == 'Linux' @@ -100,6 +100,11 @@ jobs: run: | brew install renode/tap/renode + - name: Install fish (macOS) + if: runner.os == 'macOS' + run: | + brew install fish + - name: Cache Fedora Cloud images id: cache-fedora-cloud-images uses: actions/cache@v5 diff --git a/python/packages/jumpstarter/jumpstarter/common/utils_test.py b/python/packages/jumpstarter/jumpstarter/common/utils_test.py index 6651e504b..8b8c954c9 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils_test.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils_test.py @@ -1,7 +1,11 @@ import os import shutil +import subprocess +import tempfile from unittest.mock import patch +import pytest + from .utils import _generate_shell_init, _validate_j_commands, launch_shell @@ -276,3 +280,55 @@ def mock_run_process(cmd, env, lease=None): assert len(temp_dirs) == 1 assert not os.path.exists(temp_dirs[0]) + + +@pytest.mark.skipif(not shutil.which("zsh"), reason="zsh not installed") +def test_zsh_init_does_not_produce_compdef_errors(): + init_content = _generate_shell_init("zsh", use_profiles=False, j_commands=["power", "serial"]) + with tempfile.TemporaryDirectory() as tmpdir: + zshrc_path = os.path.join(tmpdir, ".zshrc") + with open(zshrc_path, "w") as f: + f.write(init_content) + result = subprocess.run( + ["zsh", "--rcs", "-c", "exit 0"], + env={"ZDOTDIR": tmpdir, "HOME": tmpdir, "PATH": os.environ.get("PATH", "")}, + capture_output=True, + text=True, + timeout=10, + ) + assert "command not found: compdef" not in result.stderr + assert result.returncode == 0 + + +@pytest.mark.skipif(not shutil.which("bash"), reason="bash not installed") +def test_bash_init_produces_no_errors(): + init_content = _generate_shell_init("bash", use_profiles=False, j_commands=["power", "serial"]) + with tempfile.NamedTemporaryFile(mode="w", suffix=".sh", delete=False) as f: + f.write(init_content) + rcfile = f.name + try: + result = subprocess.run( + ["bash", "--rcfile", rcfile, "-c", "exit 0"], + env={"HOME": "/nonexistent", "PATH": os.environ.get("PATH", "")}, + capture_output=True, + text=True, + timeout=10, + ) + assert "command not found" not in result.stderr + assert result.returncode == 0 + finally: + os.unlink(rcfile) + + +@pytest.mark.skipif(not shutil.which("fish"), reason="fish not installed") +def test_fish_init_produces_no_errors(): + init_content = _generate_shell_init("fish", use_profiles=False, j_commands=["power", "serial"]) + result = subprocess.run( + ["fish", "--init-command", init_content, "-c", "exit 0"], + env={"HOME": "/nonexistent", "PATH": os.environ.get("PATH", "")}, + capture_output=True, + text=True, + timeout=10, + ) + assert "command not found" not in result.stderr + assert result.returncode == 0 From a02aa2640b2becba9548c85229f0ef7a8e2010de Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Thu, 9 Apr 2026 09:57:41 +0200 Subject: [PATCH 10/19] fix: address CodeRabbit review findings on shell completion Co-Authored-By: Claude Opus 4.6 --- .../jumpstarter/jumpstarter/common/utils.py | 4 ++-- .../jumpstarter/common/utils_test.py | 24 ++++++++++--------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/python/packages/jumpstarter/jumpstarter/common/utils.py b/python/packages/jumpstarter/jumpstarter/common/utils.py index 867e273fe..e22353691 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils.py @@ -118,14 +118,14 @@ def _generate_shell_init(shell_name: str, use_profiles: bool, j_commands: list[s elif shell_name.endswith("zsh"): lines = [] + lines.append("autoload -Uz compinit && compinit") if use_profiles: lines.append('[ -f ~/.zshrc ] && source ~/.zshrc') - lines.append("autoload -Uz compinit && compinit") lines.append('eval "$(jmp completion zsh 2>/dev/null)"') lines.append('eval "$(jmp-admin completion zsh 2>/dev/null)"') if j_commands: cmds = " ".join(j_commands) - lines.append(f"compdef '_arguments \"1:(({cmds}))\"' j") + lines.append(f"compdef '_arguments \"1:subcommand:({cmds})\"' j") else: lines.append('eval "$(j completion zsh 2>/dev/null)"') return "\n".join(lines) + "\n" diff --git a/python/packages/jumpstarter/jumpstarter/common/utils_test.py b/python/packages/jumpstarter/jumpstarter/common/utils_test.py index 8b8c954c9..3b65c2be3 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils_test.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils_test.py @@ -53,9 +53,9 @@ def test_generate_bash_init_without_j_commands(): def test_generate_zsh_init_with_j_commands(): content = _generate_shell_init("zsh", use_profiles=False, j_commands=["power", "qemu"]) - assert "power qemu" in content assert "jmp completion zsh" in content assert "compdef" in content + assert "1:subcommand:(power qemu)" in content def test_generate_zsh_init_loads_compinit_before_completions(): @@ -93,12 +93,12 @@ def test_generate_zsh_init_without_j_commands(): assert "compdef" not in content -def test_generate_zsh_init_with_profiles_loads_compinit_after_zshrc(): +def test_generate_zsh_init_with_profiles_loads_compinit_before_zshrc(): content = _generate_shell_init("zsh", use_profiles=True, j_commands=["power"]) assert ".zshrc" in content - zshrc_pos = content.index(".zshrc") compinit_pos = content.index("autoload -Uz compinit && compinit") - assert zshrc_pos < compinit_pos + zshrc_pos = content.index(".zshrc") + assert compinit_pos < zshrc_pos def test_generate_fish_init_with_j_commands(): @@ -285,19 +285,21 @@ def mock_run_process(cmd, env, lease=None): @pytest.mark.skipif(not shutil.which("zsh"), reason="zsh not installed") def test_zsh_init_does_not_produce_compdef_errors(): init_content = _generate_shell_init("zsh", use_profiles=False, j_commands=["power", "serial"]) - with tempfile.TemporaryDirectory() as tmpdir: - zshrc_path = os.path.join(tmpdir, ".zshrc") - with open(zshrc_path, "w") as f: - f.write(init_content) + with tempfile.NamedTemporaryFile(mode="w", suffix=".zsh", delete=False) as f: + f.write(init_content) + init_file = f.name + try: result = subprocess.run( - ["zsh", "--rcs", "-c", "exit 0"], - env={"ZDOTDIR": tmpdir, "HOME": tmpdir, "PATH": os.environ.get("PATH", "")}, + ["zsh", "-c", f"source {init_file}; exit 0"], + env={"HOME": "/nonexistent", "PATH": os.environ.get("PATH", "")}, capture_output=True, text=True, timeout=10, ) assert "command not found: compdef" not in result.stderr assert result.returncode == 0 + finally: + os.unlink(init_file) @pytest.mark.skipif(not shutil.which("bash"), reason="bash not installed") @@ -308,7 +310,7 @@ def test_bash_init_produces_no_errors(): rcfile = f.name try: result = subprocess.run( - ["bash", "--rcfile", rcfile, "-c", "exit 0"], + ["bash", "-c", f"source {rcfile}; exit 0"], env={"HOME": "/nonexistent", "PATH": os.environ.get("PATH", "")}, capture_output=True, text=True, From 0e25f3f772fda5eaf11576b7301b60c2b412aa1e Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Fri, 10 Apr 2026 14:31:09 +0200 Subject: [PATCH 11/19] fix: set shell prompt after user profile to prevent override When use_profiles=True, the user's ~/.zshrc (or ~/.bashrc) is sourced inside the init script, which typically sets PROMPT/PS1 and overrides the custom jumpstarter prompt set via environment variable. Fix by appending the prompt assignment at the end of the init script so it runs after user profile sourcing. Pass context via _JMP_SHELL_CONTEXT env var (consistent with fish) instead of interpolating it directly. Also refactor _launch_bash to manage its own init file lifecycle, matching the pattern used by _launch_zsh. Co-Authored-By: Claude Opus 4.6 --- .../jumpstarter/jumpstarter/common/utils.py | 35 ++- .../jumpstarter/common/utils_test.py | 215 +++++++++++++++++- 2 files changed, 243 insertions(+), 7 deletions(-) diff --git a/python/packages/jumpstarter/jumpstarter/common/utils.py b/python/packages/jumpstarter/jumpstarter/common/utils.py index e22353691..b0a38df05 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils.py @@ -144,16 +144,33 @@ def _generate_shell_init(shell_name: str, use_profiles: bool, j_commands: list[s return "" -def _launch_bash(shell, init_file, use_profiles, common_env, context, lease): +def _launch_bash(shell, init_content, use_profiles, common_env, context, lease): env = common_env | { + "_JMP_SHELL_CONTEXT": context, "PS1": f"{ANSI_GRAY}{PROMPT_CWD} {ANSI_YELLOW}⚡{ANSI_WHITE}{context} {ANSI_YELLOW}➤{ANSI_RESET} ", } cmd = [shell] - if init_file: + init_file = None + if init_content: + init_content += ( + f'PS1="{ANSI_GRAY}{PROMPT_CWD} {ANSI_YELLOW}⚡{ANSI_WHITE}' + '$_JMP_SHELL_CONTEXT' + f' {ANSI_YELLOW}➤{ANSI_RESET} "\n' + ) + init_file = tempfile.NamedTemporaryFile(mode="w", suffix=".sh", delete=False) + init_file.write(init_content) + init_file.close() cmd.extend(["--rcfile", init_file.name]) elif not use_profiles: cmd.extend(["--norc", "--noprofile"]) - return _run_process(cmd, env, lease) + try: + return _run_process(cmd, env, lease) + finally: + if init_file: + try: + os.unlink(init_file.name) + except OSError: + pass def _launch_fish(shell, init_file, common_env, context, lease): @@ -180,6 +197,7 @@ def _launch_fish(shell, init_file, common_env, context, lease): def _launch_zsh(shell, init_content, common_env, context, lease, use_profiles): env = common_env | { + "_JMP_SHELL_CONTEXT": context, "PS1": f"%F{{8}}%1~ %F{{yellow}}⚡%F{{white}}{context} %F{{yellow}}➤%f ", } if "HISTFILE" not in env: @@ -187,6 +205,10 @@ def _launch_zsh(shell, init_content, common_env, context, lease, use_profiles): cmd = [shell] tmpdir = None if init_content: + init_content += ( + 'PROMPT="%F{8}%1~ %F{yellow}⚡%F{white}' + '${_JMP_SHELL_CONTEXT} %F{yellow}➤%f "\n' + ) tmpdir = tempfile.mkdtemp() zshrc_path = os.path.join(tmpdir, ".zshrc") original_zdotdir = env.get("ZDOTDIR", os.path.expanduser("~")) @@ -241,6 +263,9 @@ def launch_shell( if shell_name.endswith("zsh"): return _launch_zsh(shell, init_content, common_env, context, lease, use_profiles) + if shell_name.endswith("bash"): + return _launch_bash(shell, init_content, use_profiles, common_env, context, lease) + init_file = None if init_content: init_file = tempfile.NamedTemporaryFile(mode="w", suffix=".sh", delete=False) @@ -248,9 +273,7 @@ def launch_shell( init_file.close() try: - if shell_name.endswith("bash"): - return _launch_bash(shell, init_file, use_profiles, common_env, context, lease) - elif shell_name.endswith("fish"): + if shell_name.endswith("fish"): return _launch_fish(shell, init_file, common_env, context, lease) else: return _run_process([shell], common_env, lease) diff --git a/python/packages/jumpstarter/jumpstarter/common/utils_test.py b/python/packages/jumpstarter/jumpstarter/common/utils_test.py index 3b65c2be3..6f06a603b 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils_test.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils_test.py @@ -6,7 +6,16 @@ import pytest -from .utils import _generate_shell_init, _validate_j_commands, launch_shell +from .utils import ( + ANSI_GRAY, + ANSI_RESET, + ANSI_WHITE, + ANSI_YELLOW, + PROMPT_CWD, + _generate_shell_init, + _validate_j_commands, + launch_shell, +) def test_launch_shell(tmp_path, monkeypatch): @@ -334,3 +343,207 @@ def test_fish_init_produces_no_errors(): ) assert "command not found" not in result.stderr assert result.returncode == 0 + + +def test_launch_zsh_sets_prompt_after_profile_in_init(tmp_path, monkeypatch): + monkeypatch.setenv("SHELL", "/usr/bin/zsh") + captured_zshrc = [] + + def mock_run_process(cmd, env, lease=None): + zdotdir = env.get("ZDOTDIR") + if zdotdir: + zshrc = os.path.join(zdotdir, ".zshrc") + with open(zshrc) as f: + captured_zshrc.append(f.read()) + return 0 + + with patch("jumpstarter.common.utils._run_process", mock_run_process): + launch_shell( + host=str(tmp_path / "test.sock"), + context="test-device", + allow=["*"], + unsafe=False, + use_profiles=True, + j_commands=["power"], + ) + + assert len(captured_zshrc) == 1 + content = captured_zshrc[0] + assert "PROMPT=" in content + zshrc_pos = content.index(".zshrc") + prompt_pos = content.index("PROMPT=") + assert prompt_pos > zshrc_pos + + +def test_launch_zsh_passes_context_via_env(tmp_path, monkeypatch): + monkeypatch.setenv("SHELL", "/usr/bin/zsh") + captured_env = {} + + def mock_run_process(cmd, env, lease=None): + captured_env.update(env) + return 0 + + with patch("jumpstarter.common.utils._run_process", mock_run_process): + launch_shell( + host=str(tmp_path / "test.sock"), + context="test-device", + allow=["*"], + unsafe=False, + use_profiles=False, + ) + + assert captured_env.get("_JMP_SHELL_CONTEXT") == "test-device" + + +def test_launch_zsh_prompt_references_env_var_not_literal_context(tmp_path, monkeypatch): + monkeypatch.setenv("SHELL", "/usr/bin/zsh") + captured_zshrc = [] + + def mock_run_process(cmd, env, lease=None): + zdotdir = env.get("ZDOTDIR") + if zdotdir: + zshrc = os.path.join(zdotdir, ".zshrc") + with open(zshrc) as f: + captured_zshrc.append(f.read()) + return 0 + + with patch("jumpstarter.common.utils._run_process", mock_run_process): + launch_shell( + host=str(tmp_path / "test.sock"), + context="test-device-name", + allow=["*"], + unsafe=False, + use_profiles=False, + j_commands=["power"], + ) + + content = captured_zshrc[0] + prompt_line = [line for line in content.split("\n") if "PROMPT=" in line][0] + assert "${_JMP_SHELL_CONTEXT}" in prompt_line + assert "test-device-name" not in prompt_line + + +def test_launch_bash_sets_prompt_after_profile_in_init(tmp_path, monkeypatch): + monkeypatch.setenv("SHELL", "/usr/bin/bash") + captured_content = [] + + def mock_run_process(cmd, env, lease=None): + if "--rcfile" in cmd: + rcfile = cmd[cmd.index("--rcfile") + 1] + with open(rcfile) as f: + captured_content.append(f.read()) + return 0 + + with patch("jumpstarter.common.utils._run_process", mock_run_process): + launch_shell( + host=str(tmp_path / "test.sock"), + context="test-device", + allow=["*"], + unsafe=False, + use_profiles=True, + j_commands=["power"], + ) + + assert len(captured_content) == 1 + content = captured_content[0] + assert "PS1=" in content + bashrc_pos = content.index(".bashrc") + ps1_pos = content.index("PS1=") + assert ps1_pos > bashrc_pos + + +def test_launch_bash_passes_context_via_env(tmp_path, monkeypatch): + monkeypatch.setenv("SHELL", "/usr/bin/bash") + captured_env = {} + + def mock_run_process(cmd, env, lease=None): + captured_env.update(env) + return 0 + + with patch("jumpstarter.common.utils._run_process", mock_run_process): + launch_shell( + host=str(tmp_path / "test.sock"), + context="test-device", + allow=["*"], + unsafe=False, + use_profiles=False, + ) + + assert captured_env.get("_JMP_SHELL_CONTEXT") == "test-device" + + +@pytest.mark.skipif(not shutil.which("zsh"), reason="zsh not installed") +def test_zsh_prompt_survives_user_profile_override(): + home_dir = tempfile.mkdtemp() + try: + with open(os.path.join(home_dir, ".zshrc"), "w") as f: + f.write('PROMPT="user-prompt> "\n') + + init_content = _generate_shell_init("zsh", use_profiles=True, j_commands=["power"]) + init_content += ( + 'PROMPT="%F{8}%1~ %F{yellow}⚡%F{white}' + '${_JMP_SHELL_CONTEXT} %F{yellow}➤%f "\n' + ) + + with tempfile.NamedTemporaryFile(mode="w", suffix=".zsh", delete=False) as f: + f.write(init_content) + init_file = f.name + + try: + result = subprocess.run( + ["zsh", "-c", f"source {init_file}; echo \"$PROMPT\""], + env={ + "HOME": home_dir, + "PATH": os.environ.get("PATH", ""), + "_JMP_SHELL_CONTEXT": "test-device", + }, + capture_output=True, + text=True, + timeout=10, + ) + assert result.returncode == 0, f"zsh failed: {result.stderr}" + assert "user-prompt" not in result.stdout + assert "test-device" in result.stdout + finally: + os.unlink(init_file) + finally: + shutil.rmtree(home_dir, ignore_errors=True) + + +@pytest.mark.skipif(not shutil.which("bash"), reason="bash not installed") +def test_bash_prompt_survives_user_profile_override(): + home_dir = tempfile.mkdtemp() + try: + with open(os.path.join(home_dir, ".bashrc"), "w") as f: + f.write('PS1="user-prompt> "\n') + + init_content = _generate_shell_init("bash", use_profiles=True, j_commands=["power"]) + init_content += ( + f'PS1="{ANSI_GRAY}{PROMPT_CWD} {ANSI_YELLOW}⚡{ANSI_WHITE}' + '$_JMP_SHELL_CONTEXT' + f' {ANSI_YELLOW}➤{ANSI_RESET} "\n' + ) + + with tempfile.NamedTemporaryFile(mode="w", suffix=".sh", delete=False) as f: + f.write(init_content) + rcfile = f.name + + try: + result = subprocess.run( + ["bash", "-c", f'source {rcfile}; echo "$PS1"'], + env={ + "HOME": home_dir, + "PATH": os.environ.get("PATH", ""), + "_JMP_SHELL_CONTEXT": "test-device", + }, + capture_output=True, + text=True, + timeout=10, + ) + assert result.returncode == 0, f"bash failed: {result.stderr}" + assert "user-prompt" not in result.stdout + assert "test-device" in result.stdout + finally: + os.unlink(rcfile) + finally: + shutil.rmtree(home_dir, ignore_errors=True) From 4fc31e843e847f2eee15b2960e947195964386ca Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Mon, 13 Apr 2026 18:42:37 +0200 Subject: [PATCH 12/19] fix: use absolute paths for completion commands in shell init When the user's shell profile modifies PATH (common on macOS where /etc/zshenv runs path_helper, or with oh-my-zsh), the bare command names jmp/jmp-admin/j may no longer be found by the time the completion eval commands run. Resolve the CLI binary paths at init generation time using shutil.which() so the completion scripts reference absolute paths that survive PATH modifications. Co-Authored-By: Claude Opus 4.6 --- .../jumpstarter/jumpstarter/common/utils.py | 27 ++++++++++++------- .../jumpstarter/common/utils_test.py | 13 +++++++-- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/python/packages/jumpstarter/jumpstarter/common/utils.py b/python/packages/jumpstarter/jumpstarter/common/utils.py index b0a38df05..c53e64925 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils.py @@ -1,6 +1,7 @@ import os import re import shlex +import shutil import signal import sys import tempfile @@ -96,14 +97,22 @@ def _validate_j_commands(j_commands: list[str] | None) -> list[str] | None: return [cmd for cmd in j_commands if _SAFE_COMMAND_NAME.match(cmd)] +def _resolve_cli_paths() -> tuple[str, str, str]: + jmp = shutil.which("jmp") or "jmp" + jmp_admin = shutil.which("jmp-admin") or "jmp-admin" + j = shutil.which("j") or "j" + return jmp, jmp_admin, j + + def _generate_shell_init(shell_name: str, use_profiles: bool, j_commands: list[str] | None = None) -> str: j_commands = _validate_j_commands(j_commands) + jmp, jmp_admin, j = _resolve_cli_paths() if shell_name.endswith("bash"): lines = [] if use_profiles: lines.append('[ -f ~/.bashrc ] && source ~/.bashrc') - lines.append('eval "$(jmp completion bash 2>/dev/null)"') - lines.append('eval "$(jmp-admin completion bash 2>/dev/null)"') + lines.append(f'eval "$({jmp} completion bash 2>/dev/null)"') + lines.append(f'eval "$({jmp_admin} completion bash 2>/dev/null)"') if j_commands: cmds = " ".join(j_commands) completion_fn = ( @@ -113,7 +122,7 @@ def _generate_shell_init(shell_name: str, use_profiles: bool, j_commands: list[s lines.append(completion_fn) lines.append("complete -o default -F _j_completion j") else: - lines.append('eval "$(j completion bash 2>/dev/null)"') + lines.append(f'eval "$({j} completion bash 2>/dev/null)"') return "\n".join(lines) + "\n" elif shell_name.endswith("zsh"): @@ -121,24 +130,24 @@ def _generate_shell_init(shell_name: str, use_profiles: bool, j_commands: list[s lines.append("autoload -Uz compinit && compinit") if use_profiles: lines.append('[ -f ~/.zshrc ] && source ~/.zshrc') - lines.append('eval "$(jmp completion zsh 2>/dev/null)"') - lines.append('eval "$(jmp-admin completion zsh 2>/dev/null)"') + lines.append(f'eval "$({jmp} completion zsh 2>/dev/null)"') + lines.append(f'eval "$({jmp_admin} completion zsh 2>/dev/null)"') if j_commands: cmds = " ".join(j_commands) lines.append(f"compdef '_arguments \"1:subcommand:({cmds})\"' j") else: - lines.append('eval "$(j completion zsh 2>/dev/null)"') + lines.append(f'eval "$({j} completion zsh 2>/dev/null)"') return "\n".join(lines) + "\n" elif shell_name.endswith("fish"): lines = [] - lines.append("jmp completion fish 2>/dev/null | source") - lines.append("jmp-admin completion fish 2>/dev/null | source") + lines.append(f"{jmp} completion fish 2>/dev/null | source") + lines.append(f"{jmp_admin} completion fish 2>/dev/null | source") if j_commands: for cmd in j_commands: lines.append(f"complete -c j -f -n '__fish_use_subcommand' -a '{cmd}'") else: - lines.append("j completion fish 2>/dev/null | source") + lines.append(f"{j} completion fish 2>/dev/null | source") return "\n".join(lines) + "\n" return "" diff --git a/python/packages/jumpstarter/jumpstarter/common/utils_test.py b/python/packages/jumpstarter/jumpstarter/common/utils_test.py index 6f06a603b..a91895a89 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils_test.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils_test.py @@ -39,6 +39,15 @@ def test_launch_shell(tmp_path, monkeypatch): assert exit_code == 1 +def test_generate_shell_init_uses_absolute_paths_for_completion(): + content = _generate_shell_init("zsh", use_profiles=True, j_commands=None) + for line in content.splitlines(): + if "completion zsh" in line and "eval" in line: + dollar_paren = line.split("$(")[1].split(")")[0] + cmd = dollar_paren.split()[0] + assert cmd.startswith("/"), f"Expected absolute path for command, got: {cmd}" + + def test_generate_bash_init_with_j_commands(): content = _generate_shell_init("bash", use_profiles=False, j_commands=["power", "serial", "ssh"]) assert "_j_completion" in content @@ -71,7 +80,7 @@ def test_generate_zsh_init_loads_compinit_before_completions(): content = _generate_shell_init("zsh", use_profiles=False, j_commands=["power"]) assert "autoload -Uz compinit && compinit" in content compinit_pos = content.index("autoload -Uz compinit && compinit") - eval_jmp_pos = content.index('eval "$(jmp completion zsh') + eval_jmp_pos = content.index("completion zsh") assert compinit_pos < eval_jmp_pos @@ -86,7 +95,7 @@ def test_generate_zsh_init_without_j_commands_loads_compinit(): content = _generate_shell_init("zsh", use_profiles=False, j_commands=None) assert "autoload -Uz compinit && compinit" in content compinit_pos = content.index("autoload -Uz compinit && compinit") - eval_jmp_pos = content.index('eval "$(jmp completion zsh') + eval_jmp_pos = content.index("completion zsh") assert compinit_pos < eval_jmp_pos From 2a05ddb125e315902e8f8fd18336cfd28d7332c5 Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Mon, 13 Apr 2026 18:59:15 +0200 Subject: [PATCH 13/19] fix: mock shutil.which in absolute path test to work in CI The test relied on jmp being installed on PATH but CI environments do not have it, causing shutil.which to return None and the test to fail with bare command names instead of absolute paths. Co-Authored-By: Claude Opus 4.6 --- .../packages/jumpstarter/jumpstarter/common/utils_test.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/python/packages/jumpstarter/jumpstarter/common/utils_test.py b/python/packages/jumpstarter/jumpstarter/common/utils_test.py index a91895a89..489f373b8 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils_test.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils_test.py @@ -39,7 +39,12 @@ def test_launch_shell(tmp_path, monkeypatch): assert exit_code == 1 -def test_generate_shell_init_uses_absolute_paths_for_completion(): +def test_generate_shell_init_uses_absolute_paths_for_completion(monkeypatch): + def fake_which(name): + return f"/usr/bin/{name}" + + monkeypatch.setattr(shutil, "which", fake_which) + content = _generate_shell_init("zsh", use_profiles=True, j_commands=None) for line in content.splitlines(): if "completion zsh" in line and "eval" in line: From efc04ee289040a8f917d1d9a0e62f3fdcbce6df2 Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Thu, 16 Apr 2026 15:02:39 +0200 Subject: [PATCH 14/19] fix(tests): relocate get_completion_class mock test to match refactored module The completion module was refactored to delegate to make_completion_command from jumpstarter_cli_common. The test for the get_completion_class-returns-None defensive path was patching jumpstarter_cli.completion.get_completion_class which no longer exists in that module. Remove the stale test and unused import from jumpstarter_cli, and add an equivalent test in jumpstarter_cli_common where the defensive code now lives. Generated-By: Forge/20260416_144214_369510_5d940644 Co-Authored-By: Claude Opus 4.6 --- .../jumpstarter_cli_common/completion_test.py | 11 +++++++++++ .../jumpstarter_cli/completion_test.py | 10 ---------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/completion_test.py b/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/completion_test.py index c8b146a15..7398cebbe 100644 --- a/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/completion_test.py +++ b/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/completion_test.py @@ -1,3 +1,5 @@ +from unittest.mock import patch + import click from click.testing import CliRunner @@ -62,3 +64,12 @@ def test_completion_unsupported_shell_exits_with_error(): runner = CliRunner() result = runner.invoke(cli, ["completion", "powershell"]) assert result.exit_code == 2 + + +def test_completion_raises_when_get_completion_class_returns_none(): + with patch("jumpstarter_cli_common.completion.get_completion_class", return_value=None): + cli = _make_test_cli_with_completion() + runner = CliRunner() + result = runner.invoke(cli, ["completion", "bash"]) + assert result.exit_code == 1 + assert "Unsupported shell" in result.output diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/completion_test.py b/python/packages/jumpstarter-cli/jumpstarter_cli/completion_test.py index f8d74ae05..100517be4 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/completion_test.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/completion_test.py @@ -1,5 +1,3 @@ -from unittest.mock import patch - from click.testing import CliRunner from .jmp import jmp @@ -43,11 +41,3 @@ def test_completion_unsupported_shell(): result = runner.invoke(jmp, ["completion", "powershell"]) assert result.exit_code == 2 assert "Invalid value" in result.output or "powershell" in result.output - - -def test_completion_raises_when_get_completion_class_returns_none(): - with patch("jumpstarter_cli.completion.get_completion_class", return_value=None): - runner = CliRunner() - result = runner.invoke(jmp, ["completion", "bash"]) - assert result.exit_code == 1 - assert "Unsupported shell" in result.output From 907612d334d035e963d237c1c89915366f1ea8a9 Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Fri, 17 Apr 2026 12:45:28 +0200 Subject: [PATCH 15/19] fix: source zshrc before compinit to fix macOS compdef errors On macOS, .zshrc may set up fpath entries needed by compinit. Loading compinit before profiles causes 'compdef: command not found' when the completion system cannot find its function definitions. Co-Authored-By: Claude Opus 4.6 --- python/packages/jumpstarter/jumpstarter/common/utils.py | 2 +- python/packages/jumpstarter/jumpstarter/common/utils_test.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/python/packages/jumpstarter/jumpstarter/common/utils.py b/python/packages/jumpstarter/jumpstarter/common/utils.py index c53e64925..37e123b23 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils.py @@ -127,9 +127,9 @@ def _generate_shell_init(shell_name: str, use_profiles: bool, j_commands: list[s elif shell_name.endswith("zsh"): lines = [] - lines.append("autoload -Uz compinit && compinit") if use_profiles: lines.append('[ -f ~/.zshrc ] && source ~/.zshrc') + lines.append("autoload -Uz compinit && compinit") lines.append(f'eval "$({jmp} completion zsh 2>/dev/null)"') lines.append(f'eval "$({jmp_admin} completion zsh 2>/dev/null)"') if j_commands: diff --git a/python/packages/jumpstarter/jumpstarter/common/utils_test.py b/python/packages/jumpstarter/jumpstarter/common/utils_test.py index 489f373b8..f81053ecb 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils_test.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils_test.py @@ -116,12 +116,12 @@ def test_generate_zsh_init_without_j_commands(): assert "compdef" not in content -def test_generate_zsh_init_with_profiles_loads_compinit_before_zshrc(): +def test_generate_zsh_init_with_profiles_loads_zshrc_before_compinit(): content = _generate_shell_init("zsh", use_profiles=True, j_commands=["power"]) assert ".zshrc" in content compinit_pos = content.index("autoload -Uz compinit && compinit") zshrc_pos = content.index(".zshrc") - assert compinit_pos < zshrc_pos + assert zshrc_pos < compinit_pos def test_generate_fish_init_with_j_commands(): From cae89180a90f9bd6ce9c4e1aa4b24419bf00c92b Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Mon, 20 Apr 2026 17:03:54 +0200 Subject: [PATCH 16/19] fix(shell): source original zshenv when overriding ZDOTDIR When ZDOTDIR is set to a temp directory for zsh init, the user's ~/.zshenv is never read because zsh reads .zshenv from ZDOTDIR before .zshrc. Create a .zshenv in the temp directory that sources the original ~/.zshenv so user environment setup is preserved. Generated-By: Forge/20260420_165007_876667_3532a927 --- .../jumpstarter/jumpstarter/common/utils.py | 6 +++- .../jumpstarter/common/utils_test.py | 34 +++++++++++++++++-- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/python/packages/jumpstarter/jumpstarter/common/utils.py b/python/packages/jumpstarter/jumpstarter/common/utils.py index 37e123b23..1f3978afd 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils.py @@ -219,8 +219,12 @@ def _launch_zsh(shell, init_content, common_env, context, lease, use_profiles): '${_JMP_SHELL_CONTEXT} %F{yellow}➤%f "\n' ) tmpdir = tempfile.mkdtemp() - zshrc_path = os.path.join(tmpdir, ".zshrc") original_zdotdir = env.get("ZDOTDIR", os.path.expanduser("~")) + original_zshenv = os.path.join(original_zdotdir, ".zshenv") + zshenv_path = os.path.join(tmpdir, ".zshenv") + with open(zshenv_path, "w") as f: + f.write(f"[ -f {shlex.quote(original_zshenv)} ] && source {shlex.quote(original_zshenv)}\n") + zshrc_path = os.path.join(tmpdir, ".zshrc") with open(zshrc_path, "w") as f: f.write(f"ZDOTDIR={shlex.quote(original_zdotdir)}\n") f.write(init_content) diff --git a/python/packages/jumpstarter/jumpstarter/common/utils_test.py b/python/packages/jumpstarter/jumpstarter/common/utils_test.py index f81053ecb..dc7a95807 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils_test.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils_test.py @@ -279,7 +279,7 @@ def mock_run_process(cmd, env, lease=None): ) -def test_launch_shell_zsh_uses_tmpdir_without_intermediate_file(tmp_path, monkeypatch): +def test_launch_shell_zsh_uses_tmpdir_with_zshrc_and_zshenv(tmp_path, monkeypatch): monkeypatch.setenv("SHELL", "/usr/bin/zsh") temp_dirs = [] @@ -287,8 +287,8 @@ def mock_run_process(cmd, env, lease=None): zdotdir = env.get("ZDOTDIR") if zdotdir: temp_dirs.append(zdotdir) - entries = os.listdir(zdotdir) - assert entries == [".zshrc"], f"Expected only .zshrc in ZDOTDIR, found: {entries}" + entries = sorted(os.listdir(zdotdir)) + assert entries == [".zshenv", ".zshrc"], f"Expected .zshenv and .zshrc in ZDOTDIR, found: {entries}" return 0 with patch("jumpstarter.common.utils._run_process", mock_run_process): @@ -305,6 +305,34 @@ def mock_run_process(cmd, env, lease=None): assert not os.path.exists(temp_dirs[0]) +def test_launch_shell_zsh_sources_original_zshenv(tmp_path, monkeypatch): + monkeypatch.setenv("SHELL", "/usr/bin/zsh") + home_dir = os.path.expanduser("~") + original_zshenv = os.path.join(home_dir, ".zshenv") + + def mock_run_process(cmd, env, lease=None): + zdotdir = env.get("ZDOTDIR") + if zdotdir: + zshenv_path = os.path.join(zdotdir, ".zshenv") + assert os.path.exists(zshenv_path), ".zshenv must exist in temp ZDOTDIR" + with open(zshenv_path) as f: + content = f.read() + assert original_zshenv in content, ( + f".zshenv must source original {original_zshenv}" + ) + return 0 + + with patch("jumpstarter.common.utils._run_process", mock_run_process): + launch_shell( + host=str(tmp_path / "test.sock"), + context="remote", + allow=["*"], + unsafe=False, + use_profiles=False, + j_commands=["power"], + ) + + @pytest.mark.skipif(not shutil.which("zsh"), reason="zsh not installed") def test_zsh_init_does_not_produce_compdef_errors(): init_content = _generate_shell_init("zsh", use_profiles=False, j_commands=["power", "serial"]) From 9664f0cd3b7ba7281700ea02dfe1072d730d66b7 Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Mon, 20 Apr 2026 17:04:54 +0200 Subject: [PATCH 17/19] docs(shell): add docstrings to shell init and launch functions Add concise docstrings to _validate_j_commands, _resolve_cli_paths, _generate_shell_init, _launch_bash, _launch_fish, and _launch_zsh to restore documentation that was stripped during refactoring. Generated-By: Forge/20260420_165007_876667_3532a927 --- python/packages/jumpstarter/jumpstarter/common/utils.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/python/packages/jumpstarter/jumpstarter/common/utils.py b/python/packages/jumpstarter/jumpstarter/common/utils.py index 1f3978afd..2b6864357 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils.py @@ -92,12 +92,14 @@ def _run_process( def _validate_j_commands(j_commands: list[str] | None) -> list[str] | None: + """Filter j_commands to only include safe alphanumeric names.""" if j_commands is None: return None return [cmd for cmd in j_commands if _SAFE_COMMAND_NAME.match(cmd)] def _resolve_cli_paths() -> tuple[str, str, str]: + """Resolve absolute paths for jmp, jmp-admin, and j CLI tools.""" jmp = shutil.which("jmp") or "jmp" jmp_admin = shutil.which("jmp-admin") or "jmp-admin" j = shutil.which("j") or "j" @@ -105,6 +107,7 @@ def _resolve_cli_paths() -> tuple[str, str, str]: def _generate_shell_init(shell_name: str, use_profiles: bool, j_commands: list[str] | None = None) -> str: + """Generate shell-specific init script content for completion and profile sourcing.""" j_commands = _validate_j_commands(j_commands) jmp, jmp_admin, j = _resolve_cli_paths() if shell_name.endswith("bash"): @@ -154,6 +157,7 @@ def _generate_shell_init(shell_name: str, use_profiles: bool, j_commands: list[s def _launch_bash(shell, init_content, use_profiles, common_env, context, lease): + """Launch a bash shell with completion init and custom prompt.""" env = common_env | { "_JMP_SHELL_CONTEXT": context, "PS1": f"{ANSI_GRAY}{PROMPT_CWD} {ANSI_YELLOW}⚡{ANSI_WHITE}{context} {ANSI_YELLOW}➤{ANSI_RESET} ", @@ -183,6 +187,7 @@ def _launch_bash(shell, init_content, use_profiles, common_env, context, lease): def _launch_fish(shell, init_file, common_env, context, lease): + """Launch a fish shell with completion init and custom prompt.""" fish_env = common_env | {"_JMP_SHELL_CONTEXT": context} fish_fn = ( "function fish_prompt; " @@ -205,6 +210,7 @@ def _launch_fish(shell, init_file, common_env, context, lease): def _launch_zsh(shell, init_content, common_env, context, lease, use_profiles): + """Launch a zsh shell with completion init, custom prompt, and ZDOTDIR management.""" env = common_env | { "_JMP_SHELL_CONTEXT": context, "PS1": f"%F{{8}}%1~ %F{{yellow}}⚡%F{{white}}{context} %F{{yellow}}➤%f ", From 43fef1576c6950204f755cca16ecab2839d17ddf Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Mon, 20 Apr 2026 17:05:35 +0200 Subject: [PATCH 18/19] refactor(shell): move fish init file creation into _launch_fish Align fish launcher with bash and zsh by handling init file creation and cleanup inside _launch_fish instead of in launch_shell. All three shell launchers now follow the same pattern of receiving init_content and managing their own temp files. Generated-By: Forge/20260420_165007_876667_3532a927 --- .../jumpstarter/jumpstarter/common/utils.py | 36 +++++++++---------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/python/packages/jumpstarter/jumpstarter/common/utils.py b/python/packages/jumpstarter/jumpstarter/common/utils.py index 2b6864357..052e65a4b 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils.py @@ -186,7 +186,7 @@ def _launch_bash(shell, init_content, use_profiles, common_env, context, lease): pass -def _launch_fish(shell, init_file, common_env, context, lease): +def _launch_fish(shell, init_content, common_env, context, lease): """Launch a fish shell with completion init and custom prompt.""" fish_env = common_env | {"_JMP_SHELL_CONTEXT": context} fish_fn = ( @@ -203,10 +203,21 @@ def _launch_fish(shell, init_file, common_env, context, lease): "end" ) init_cmd = fish_fn - if init_file: + init_file = None + if init_content: + init_file = tempfile.NamedTemporaryFile(mode="w", suffix=".sh", delete=False) + init_file.write(init_content) + init_file.close() fish_env["_JMP_SHELL_INIT"] = init_file.name init_cmd += '; source "$_JMP_SHELL_INIT"' - return _run_process([shell, "--init-command", init_cmd], fish_env, lease) + try: + return _run_process([shell, "--init-command", init_cmd], fish_env, lease) + finally: + if init_file: + try: + os.unlink(init_file.name) + except OSError: + pass def _launch_zsh(shell, init_content, common_env, context, lease, use_profiles): @@ -285,20 +296,7 @@ def launch_shell( if shell_name.endswith("bash"): return _launch_bash(shell, init_content, use_profiles, common_env, context, lease) - init_file = None - if init_content: - init_file = tempfile.NamedTemporaryFile(mode="w", suffix=".sh", delete=False) - init_file.write(init_content) - init_file.close() + if shell_name.endswith("fish"): + return _launch_fish(shell, init_content, common_env, context, lease) - try: - if shell_name.endswith("fish"): - return _launch_fish(shell, init_file, common_env, context, lease) - else: - return _run_process([shell], common_env, lease) - finally: - if init_file: - try: - os.unlink(init_file.name) - except OSError: - pass + return _run_process([shell], common_env, lease) From c17cc996033c32d77fb11ebef27b6658be67cdf8 Mon Sep 17 00:00:00 2001 From: Paul Wallrabe Date: Mon, 20 Apr 2026 17:06:04 +0200 Subject: [PATCH 19/19] test(shell): add fish launch integration test for init file lifecycle Add test verifying that the fish launcher creates the init file during process execution and cleans it up after the process exits, matching the coverage already present for bash and zsh launchers. Generated-By: Forge/20260420_165007_876667_3532a927 --- .../jumpstarter/common/utils_test.py | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/python/packages/jumpstarter/jumpstarter/common/utils_test.py b/python/packages/jumpstarter/jumpstarter/common/utils_test.py index dc7a95807..cb137ce09 100644 --- a/python/packages/jumpstarter/jumpstarter/common/utils_test.py +++ b/python/packages/jumpstarter/jumpstarter/common/utils_test.py @@ -198,6 +198,32 @@ def mock_run_process(cmd, env, lease=None): assert not os.path.exists(zshrc_paths[0]) +def test_launch_fish_cleans_up_temp_init_file(tmp_path, monkeypatch): + monkeypatch.setenv("SHELL", "/usr/bin/fish") + init_file_paths = [] + + def mock_run_process(cmd, env, lease=None): + init_path = env.get("_JMP_SHELL_INIT") + if init_path: + init_file_paths.append(init_path) + assert os.path.exists(init_path), "init file must exist during process run" + return 0 + + with patch("jumpstarter.common.utils._run_process", mock_run_process): + exit_code = launch_shell( + host=str(tmp_path / "test.sock"), + context="remote", + allow=["*"], + unsafe=False, + use_profiles=False, + j_commands=["power", "serial"], + ) + + assert exit_code == 0 + assert len(init_file_paths) == 1 + assert not os.path.exists(init_file_paths[0]), "init file must be cleaned up after process exits" + + def test_launch_fish_passes_context_via_env(tmp_path, monkeypatch): monkeypatch.setenv("SHELL", "/usr/bin/fish") captured_env = {}