diff --git a/agent/cli/_legacy.py b/agent/cli/_legacy.py index 1804eff9b..c880f6a2c 100644 --- a/agent/cli/_legacy.py +++ b/agent/cli/_legacy.py @@ -20,6 +20,8 @@ import os import re import shutil +import signal +import subprocess import sys import threading import time @@ -4032,6 +4034,40 @@ def _build_parser() -> argparse.ArgumentParser: subparsers.add_parser("init", help="Interactive setup: create ~/.vibe-trading/.env") + # Cross-platform frontend setup. See cmd_setup() for details. + setup_parser = subparsers.add_parser( + "setup", + help="Install frontend dependencies and build the production bundle", + ) + setup_parser.add_argument( + "--frontend-dir", + default=str(AGENT_DIR.parent / "frontend"), + help="Path to the frontend directory (default: /frontend)", + ) + + # Cross-platform dev mode. See cmd_dev() for details. + dev_parser = subparsers.add_parser( + "dev", + help="Start backend + frontend dev servers in one process", + ) + dev_parser.add_argument( + "--port", + type=int, + default=8899, + help="Backend port (default: 8899)", + ) + dev_parser.add_argument( + "--frontend-port", + type=int, + default=5899, + help="Vite dev server port, must match vite.config.ts (default: 5899)", + ) + dev_parser.add_argument( + "--frontend-dir", + default=str(AGENT_DIR.parent / "frontend"), + help="Path to the frontend directory (default: /frontend)", + ) + memory_parser = subparsers.add_parser("memory", help="Inspect persistent memory") memory_subparsers = memory_parser.add_subparsers(dest="memory_command") @@ -4601,6 +4637,273 @@ def cmd_init() -> int: return 0 +# --------------------------------------------------------------------------- +# Cross-platform frontend setup / dev commands. +# +# These exist to bridge a real Windows footgun: the package's frontend uses +# TypeScript, but `npx tsc` on Windows does NOT resolve the locally-installed +# TypeScript binary. Instead, npx hits the npm registry and downloads an +# abandoned 10-year-old package called `tsc@2.0.4` that prints +# "This is not the tsc command you are looking for". The fix is to always +# invoke TypeScript via `npm exec --package=typescript tsc ...` (or +# `npx --package=typescript tsc ...`) on Windows; on POSIX, `npm run build` +# already works because npm prepends ./node_modules/.bin to PATH for local +# scripts. +# --------------------------------------------------------------------------- + + +def _is_windows() -> bool: + """True when running on a Windows-like platform (win32, including Cygwin/MSYS).""" + return sys.platform == "win32" + + +def _resolve_node_and_npm() -> tuple[Optional[str], Optional[str]]: + """Return ``(node_path, npm_path)`` if both are on PATH, else ``(None, None)``. + + Used by ``cmd_setup`` to fail fast with a clear message instead of + surfacing a cryptic ENOENT from npm itself. + """ + node = shutil.which("node") + npm = shutil.which("npm") + return node, npm + + +def _build_frontend_cmd(frontend_dir: Path) -> list[list[str]]: + """Return the ordered list of subprocess invocations needed to build the frontend. + + On Windows we explicitly pin ``--package=typescript`` / ``--package=vite`` + so npm cannot accidentally fetch the abandoned ``tsc`` package from the + registry. On POSIX systems, ``npm run build`` is sufficient because npm + prepends ``./node_modules/.bin`` to ``PATH`` for local scripts. + + Each inner list is a single ``subprocess.run`` invocation. Returned as a + list of steps so the caller can stream progress. + """ + is_win = _is_windows() + if is_win: + # `npm exec --package=typescript tsc -b` is the safe form on Windows; + # plain `npx tsc` will fetch the abandoned `tsc@2.0.4` package. + return [ + ["npm", "install", "--no-audit", "--no-fund"], + ["npm", "exec", "--package=typescript", "--", "tsc", "-b"], + ["npm", "exec", "--package=vite", "--", "vite", "build"], + ] + return [ + ["npm", "install", "--no-audit", "--no-fund"], + ["npm", "run", "build"], + ] + + +def _run_step( + description: str, + cmd: list[str], + cwd: Path, +) -> bool: + """Run one subprocess step, returning True on success. + + Decodes subprocess output as UTF-8 with ``errors="replace"`` so that + non-ASCII bytes emitted by tools like Vite do not raise + ``UnicodeDecodeError`` on platforms whose default codec is GBK/CP936 + (notably Windows). The captured text is only used to surface a + friendly error message; lossy decoding is acceptable here. + """ + console.print(f"[dim] {description} …[/dim]") + try: + result = subprocess.run( + cmd, + cwd=str(cwd), + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + shell=False, + check=False, + ) + except FileNotFoundError as exc: + console.print(f"[red] failed:[/red] {exc}") + return False + if result.returncode != 0: + err = (result.stderr or result.stdout or "").strip() + # Show the last 20 lines to keep noise manageable. + tail = "\n".join(err.splitlines()[-20:]) if err else "(no output)" + console.print(f"[red] {description} failed:[/red]\n{tail}") + return False + return True + + +def cmd_setup(frontend_dir: Path) -> int: + """Install frontend dependencies and build the production bundle. + + Cross-platform wrapper that hides the ``npx tsc`` / ``npm exec tsc`` + Windows footgun. Equivalent to running ``cd frontend && npm install + && npm run build`` from a POSIX shell, but works on Windows without + the user having to know about the abandoned ``tsc`` package on the + npm registry. + """ + console.print( + Panel( + f"[bold cyan]Vibe-Trading frontend setup[/bold cyan]\n" + f"[dim]{frontend_dir}[/dim]", + border_style="cyan", + padding=(0, 1), + ) + ) + + if not frontend_dir.exists(): + console.print( + f"[red]Frontend directory not found:[/red] {frontend_dir}\n" + "[dim]Pass --frontend-dir to point at a different location.[/dim]" + ) + return EXIT_USAGE_ERROR + + node, npm = _resolve_node_and_npm() + if not node or not npm: + missing = [name for name, path_ in (("node", node), ("npm", npm)) if not path_] + console.print( + f"[red]Required tool not on PATH:[/red] {', '.join(missing)}\n" + "[dim]Install Node.js (>= 18) from https://nodejs.org and retry.[/dim]" + ) + return EXIT_USAGE_ERROR + + # On Windows, ``npm`` is shipped as ``npm.cmd``; ``subprocess.run`` does + # not consult ``PATHEXT`` for bare command names, so it would raise + # ``FileNotFoundError`` even though ``shutil.which("npm")`` returned a + # valid path. Resolve to the full path before invoking. + npm_path = npm + if _is_windows(): + steps = [ + [npm_path, *step[1:]] if step and step[0] == "npm" else step + for step in _build_frontend_cmd(frontend_dir) + ] + else: + steps = _build_frontend_cmd(frontend_dir) + for step in steps: + description = " ".join(step[:3]) # e.g. "npm install --no-audit" + if not _run_step(description, step, frontend_dir): + return EXIT_RUN_FAILED + + console.print( + Panel( + "[green]Frontend built.[/green]\n" + f" Artifacts: [cyan]{frontend_dir / 'dist'}[/cyan]\n" + "[dim]Run [bold]vibe-trading serve[/bold] to serve everything on one port.[/dim]", + border_style="green", + padding=(0, 1), + ) + ) + return EXIT_SUCCESS + + +def cmd_dev( + backend_port: int = 8899, + frontend_port: int = 5899, + frontend_dir: Optional[Path] = None, +) -> int: + """Start backend + Vite dev server in one foreground process. + + Spawns two child processes: + + * The FastAPI backend, launched from ``AGENT_DIR`` so that + ``python -m cli._legacy serve`` resolves the in-repo ``cli`` package + (launching it from the repo root would fail with + ``ModuleNotFoundError: No module named 'cli'``). + * The Vite dev server, launched from ``frontend_dir`` with the port + from ``vite.config.ts`` (currently 5899). We do NOT hardcode + ``5173`` — that would be wrong for this project. + + Both children inherit stdout/stderr so their logs are interleaved + with the dev banner. ``Ctrl+C`` (SIGINT) and ``SIGTERM`` cleanly + terminate both children. + """ + frontend_dir = frontend_dir or (AGENT_DIR.parent / "frontend") + if not frontend_dir.exists(): + console.print( + f"[red]Frontend directory not found:[/red] {frontend_dir}\n" + "[dim]Pass --frontend-dir to point at a different location.[/dim]" + ) + return EXIT_USAGE_ERROR + + node, npm = _resolve_node_and_npm() + if not node or not npm: + missing = [name for name, path_ in (("node", node), ("npm", npm)) if not path_] + console.print( + f"[red]Required tool not on PATH:[/red] {', '.join(missing)}\n" + "[dim]Install Node.js (>= 18) from https://nodejs.org and retry.[/dim]" + ) + return EXIT_USAGE_ERROR + + backend_cmd = [sys.executable, "-m", "cli._legacy", "serve", "--port", str(backend_port)] + # On Windows, ``npm`` is typically ``npm.cmd``. ``subprocess.Popen`` does + # not consult ``PATHEXT`` for bare command names, so the call would fail + # with ``FileNotFoundError`` even though ``shutil.which("npm")`` returned + # a path. Use the resolved executable path directly. + npm_executable = npm if _is_windows() else "npm" + frontend_cmd = [npm_executable, "run", "dev", "--", "--port", str(frontend_port)] + + console.print( + Panel( + f"[bold cyan]Vibe-Trading dev[/bold cyan]\n" + f" Backend → [cyan]http://127.0.0.1:{backend_port}[/cyan] " + f"(cwd: {AGENT_DIR})\n" + f" Frontend → [cyan]http://localhost:{frontend_port}[/cyan] " + f"(cwd: {frontend_dir})", + border_style="cyan", + padding=(0, 1), + ) + ) + console.print("[dim]Press Ctrl+C to stop both servers.[/dim]\n") + + backend = subprocess.Popen(backend_cmd, cwd=str(AGENT_DIR)) + frontend = subprocess.Popen(frontend_cmd, cwd=str(frontend_dir)) + children = [backend, frontend] + + def _terminate_all() -> None: + for child in children: + if child.poll() is None: + try: + child.terminate() + except OSError: + pass + + # Wire signal handlers. On Windows, SIGTERM does not exist and signal + # handlers must be installed from the main thread; KeyboardInterrupt is + # the cross-platform path for Ctrl+C. + if threading.current_thread() is threading.main_thread(): + try: + signal.signal(signal.SIGINT, lambda *_: _terminate_all()) + except (ValueError, OSError): + pass + try: + signal.signal(signal.SIGTERM, lambda *_: _terminate_all()) + except (AttributeError, ValueError, OSError): + pass + + try: + # Wait for whichever process exits first; if it's the backend we + # bring the frontend down too, and vice versa. + while True: + time.sleep(0.5) + if backend.poll() is not None or frontend.poll() is not None: + break + except KeyboardInterrupt: + pass + finally: + _terminate_all() + # Give the children a brief grace period, then force-kill. + deadline = time.time() + 5 + for child in children: + remaining = max(0.0, deadline - time.time()) + try: + child.wait(timeout=remaining) + except subprocess.TimeoutExpired: + try: + child.kill() + except OSError: + pass + + return EXIT_SUCCESS + + def main(argv: list[str] | None = None) -> int: """CLI entrypoint returning a process exit code.""" raw_argv = list(sys.argv[1:] if argv is None else argv) @@ -4616,6 +4919,18 @@ def main(argv: list[str] | None = None) -> int: if args.command == "init": return cmd_init() + if args.command == "setup": + return _coerce_exit_code( + cmd_setup(frontend_dir=Path(args.frontend_dir)) + ) + if args.command == "dev": + return _coerce_exit_code( + cmd_dev( + backend_port=args.port, + frontend_port=args.frontend_port, + frontend_dir=Path(args.frontend_dir), + ) + ) if args.command == "serve": return serve_main(raw_argv[1:]) if args.command == "provider": diff --git a/agent/cli/main.py b/agent/cli/main.py index 410388261..7449de8fa 100644 --- a/agent/cli/main.py +++ b/agent/cli/main.py @@ -1335,6 +1335,40 @@ def _show(run_id: str = typer.Argument(...)) -> None: def _init() -> None: run_onboarding(console=get_console()) + @app.command("setup", help="Install frontend deps and build the production bundle (cross-platform).") + def _setup( + frontend_dir: str = typer.Option( + "frontend", + "--frontend-dir", + help="Path to the frontend directory (relative to repo root or absolute).", + ), + ) -> None: + sys.exit(main(["setup", "--frontend-dir", frontend_dir])) + + @app.command("dev", help="Start backend + Vite dev server in one process.") + def _dev( + port: int = typer.Option(8899, "--port", help="Backend port."), + frontend_port: int = typer.Option( + 5899, + "--frontend-port", + help="Vite dev server port (must match vite.config.ts).", + ), + frontend_dir: str = typer.Option("frontend", "--frontend-dir"), + ) -> None: + sys.exit( + main( + [ + "dev", + "--port", + str(port), + "--frontend-port", + str(frontend_port), + "--frontend-dir", + frontend_dir, + ] + ) + ) + return app diff --git a/agent/tests/test_cli_setup_dev.py b/agent/tests/test_cli_setup_dev.py new file mode 100644 index 000000000..8c37b6563 --- /dev/null +++ b/agent/tests/test_cli_setup_dev.py @@ -0,0 +1,193 @@ +"""Unit tests for the cross-platform `vibe-trading setup` and +`vibe-trading dev` commands. + +These tests cover the entrypoint and platform-aware build-command +selection. They do not actually invoke ``npm`` (we mock ``shutil.which`` +and ``subprocess.run``), so they run in any environment. +""" + +from __future__ import annotations + +import sys +from pathlib import Path +from unittest.mock import MagicMock, patch + +import cli + + +class TestIsWindows: + def test_true_on_win32(self) -> None: + with patch.object(cli._legacy.sys, "platform", "win32"): + assert cli._legacy._is_windows() is True + + def test_false_on_linux(self) -> None: + with patch.object(cli._legacy.sys, "platform", "linux"): + assert cli._legacy._is_windows() is False + + def test_false_on_darwin(self) -> None: + with patch.object(cli._legacy.sys, "platform", "darwin"): + assert cli._legacy._is_windows() is False + + +class TestBuildFrontendCmd: + """The Windows sequence must use ``npm exec --package=...`` to avoid + npx pulling the abandoned ``tsc@2.0.4`` package from the registry.""" + + def test_windows_pins_typescript_and_vite_packages(self) -> None: + with patch.object(cli._legacy.sys, "platform", "win32"): + steps = cli._legacy._build_frontend_cmd(Path("frontend")) + # First step is always `npm install`. + assert steps[0][:1] == ["npm"] + assert steps[0][1] == "install" + # Subsequent steps must pin --package= so npx/npm exec cannot + # accidentally fetch the abandoned `tsc` package. + assert any( + "--package=typescript" in step and "tsc" in step for step in steps[1:] + ), f"expected an explicit typescript pin, got: {steps[1:]}" + assert any( + "--package=vite" in step and "vite" in step and "build" in step + for step in steps[1:] + ), f"expected an explicit vite build pin, got: {steps[1:]}" + + def test_posix_uses_npm_run_build(self) -> None: + with patch.object(cli._legacy.sys, "platform", "linux"): + steps = cli._legacy._build_frontend_cmd(Path("frontend")) + assert steps[-1] == ["npm", "run", "build"] + + def test_posix_does_not_pin_packages(self) -> None: + """On POSIX, npm's local PATH magic makes ``npm run build`` work + without explicit package pinning. Keep it that way.""" + with patch.object(cli._legacy.sys, "platform", "linux"): + steps = cli._legacy._build_frontend_cmd(Path("frontend")) + for step in steps: + assert "--package=" not in step, f"unexpected pin on POSIX: {step}" + + +class TestCmdSetupNodeMissing: + """When node or npm is not on PATH we should fail fast with a clear + message and the USAGE exit code, instead of letting the user see a + raw ENOENT from npm.""" + + def test_fails_when_node_missing(self, tmp_path: Path, capsys) -> None: + # Build a fake frontend dir so the directory check passes. + frontend_dir = tmp_path / "frontend" + frontend_dir.mkdir() + + with patch.object(cli._legacy, "_resolve_node_and_npm", return_value=(None, "/usr/bin/npm")): + rc = cli._legacy.cmd_setup(frontend_dir=frontend_dir) + assert rc == cli._legacy.EXIT_USAGE_ERROR + out = capsys.readouterr().out + assert "node" in out + + def test_fails_when_npm_missing(self, tmp_path: Path, capsys) -> None: + frontend_dir = tmp_path / "frontend" + frontend_dir.mkdir() + + with patch.object(cli._legacy, "_resolve_node_and_npm", return_value=("/usr/bin/node", None)): + rc = cli._legacy.cmd_setup(frontend_dir=frontend_dir) + assert rc == cli._legacy.EXIT_USAGE_ERROR + out = capsys.readouterr().out + assert "npm" in out + + def test_fails_when_frontend_dir_missing(self, tmp_path: Path, capsys) -> None: + missing = tmp_path / "no-such-frontend" + with patch.object(cli._legacy, "_resolve_node_and_npm", return_value=("/usr/bin/node", "/usr/bin/npm")): + rc = cli._legacy.cmd_setup(frontend_dir=missing) + assert rc == cli._legacy.EXIT_USAGE_ERROR + out = capsys.readouterr().out + assert "not found" in out.lower() or "not on PATH" in out + + +class TestCmdSetupRunsSteps: + """When Node and the frontend dir are present, cmd_setup should + invoke the right number of steps in order and return success.""" + + def test_runs_all_steps(self, tmp_path: Path) -> None: + frontend_dir = tmp_path / "frontend" + frontend_dir.mkdir() + (frontend_dir / "dist").mkdir() # simulate a build output + + with patch.object(cli._legacy, "_resolve_node_and_npm", return_value=("/usr/bin/node", "/usr/bin/npm")): + with patch.object(cli._legacy, "_is_windows", return_value=False): + with patch("cli._legacy.subprocess.run") as mock_run: + mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="") + rc = cli._legacy.cmd_setup(frontend_dir=frontend_dir) + + assert rc == cli._legacy.EXIT_SUCCESS + # On POSIX: exactly 2 steps (install + run build). + assert mock_run.call_count == 2 + # The last invocation should be `npm run build`. + last_cmd = mock_run.call_args_list[-1].args[0] + assert last_cmd[:3] == ["npm", "run", "build"] + + def test_returns_run_failed_when_step_fails(self, tmp_path: Path) -> None: + frontend_dir = tmp_path / "frontend" + frontend_dir.mkdir() + + with patch.object(cli._legacy, "_resolve_node_and_npm", return_value=("/usr/bin/node", "/usr/bin/npm")): + with patch.object(cli._legacy, "_is_windows", return_value=False): + with patch("cli._legacy.subprocess.run") as mock_run: + mock_run.return_value = MagicMock( + returncode=1, stdout="", stderr="boom: failed" + ) + rc = cli._legacy.cmd_setup(frontend_dir=frontend_dir) + + assert rc == cli._legacy.EXIT_RUN_FAILED + # Should bail out on the first failing step, not run them all. + assert mock_run.call_count == 1 + + +class TestCmdDev: + """`vibe-trading dev` should use vite.config.ts's port (5899), not + 5173, and launch the backend from AGENT_DIR so the in-repo `cli` + package is importable.""" + + def test_default_frontend_port_is_5899(self) -> None: + """Hardcoded wrong-port regression: this is the value users see + in the banner. Vite config sets 5899; we must not print 5173.""" + # We don't actually run the command (it never returns), just + # inspect the documented default by calling the function with + # patched subprocess that raises immediately. + with patch("cli._legacy._resolve_node_and_npm", return_value=("/usr/bin/node", "/usr/bin/npm")): + with patch("cli._legacy.subprocess.Popen") as mock_popen: + # Both children "exit" immediately so the wait loop ends. + proc = MagicMock() + proc.poll.return_value = 0 + mock_popen.return_value = proc + with patch.object(cli._legacy.time, "sleep", side_effect=KeyboardInterrupt): + rc = cli._legacy.cmd_dev() + + # First Popen call: backend. Second: frontend. + backend_call = mock_popen.call_args_list[0] + frontend_call = mock_popen.call_args_list[1] + backend_cmd = backend_call.args[0] + frontend_cmd = frontend_call.args[0] + # Backend must run from AGENT_DIR, not the repo root. + assert backend_call.kwargs.get("cwd") == str(cli._legacy.AGENT_DIR) + # Backend invocation must be `python -m cli._legacy serve`. + assert backend_cmd[:4] == [sys.executable, "-m", "cli._legacy", "serve"] + # Frontend invocation must pass the configured vite port (5899 by + # default; --port 5173 would be a bug). + assert "--port" in frontend_cmd + port_idx = frontend_cmd.index("--port") + 1 + assert frontend_cmd[port_idx] == "5899", ( + f"dev printed wrong frontend port; got {frontend_cmd[port_idx]!r}, expected '5899'" + ) + # Process should have returned cleanly. + assert rc == cli._legacy.EXIT_SUCCESS + + def test_custom_frontend_port_propagates(self, tmp_path: Path) -> None: + frontend_dir = tmp_path / "frontend" + frontend_dir.mkdir() + with patch("cli._legacy._resolve_node_and_npm", return_value=("/usr/bin/node", "/usr/bin/npm")): + with patch("cli._legacy.subprocess.Popen") as mock_popen: + proc = MagicMock() + proc.poll.return_value = 0 + mock_popen.return_value = proc + with patch.object(cli._legacy.time, "sleep", side_effect=KeyboardInterrupt): + cli._legacy.cmd_dev(frontend_port=6000, frontend_dir=frontend_dir) + + frontend_call = mock_popen.call_args_list[1] + frontend_cmd = frontend_call.args[0] + port_idx = frontend_cmd.index("--port") + 1 + assert frontend_cmd[port_idx] == "6000"