|
| 1 | +# Plan: `mpftp shell` — an interactive FTP-style REPL |
| 2 | + |
| 3 | +Status: not started. Picked up later, independent of any other in-flight work. |
| 4 | + |
| 5 | +## Context |
| 6 | + |
| 7 | +The mpftp split/restructure plan is complete; this is unrelated follow-on work. The |
| 8 | +question that prompted it: could mpftp speak real FTP-protocol commands instead of |
| 9 | +mpremote-based ones? Real FTP (RFC 959) is infeasible as a wire-protocol swap — most |
| 10 | +boards are reached over USB serial via raw-REPL, not TCP, and adopting it would drop |
| 11 | +every non-networked board. What's actually wanted is the *ergonomics*: an interactive |
| 12 | +session with `open`/`cd`/`pwd`/`get`/`put`/`mget`/`mput`/`bye` instead of one-shot |
| 13 | +`mpftp <verb> <path>` invocations. This is purely additive — none of the existing flat |
| 14 | +subcommands (`ls`, `get`, `put`, `rm`, `mkdir`, …) are renamed or touched, since |
| 15 | +scripts, the VS Code extension, and the MCP server all depend on them as-is. |
| 16 | + |
| 17 | +## Design |
| 18 | + |
| 19 | +- New `cli/src/mpftp/shell.py`, a `FtpShell(cmd.Cmd)` subclass (stdlib `cmd` — no new |
| 20 | + dependency, matches the project's stdlib-only bias seen in `pwa.py`'s hand-rolled |
| 21 | + WebSocket). `cmd.Cmd`'s `do_*` methods are individually callable via |
| 22 | + `shell.onecmd("ls")` without driving the real `input()` loop, which is exactly the |
| 23 | + test pattern `test_mip.py` already uses (`FakeClient` + direct calls + |
| 24 | + `redirect_stdout`). |
| 25 | +- Board paths have no server-side cwd concept (confirmed: zero `cwd`/`chdir`/`getcwd` |
| 26 | + hits in `sidecar.py`; every `fs_*` RPC takes an absolute path string). `FtpShell` |
| 27 | + tracks `self.board_cwd: str = "/"` client-side and resolves every path through: |
| 28 | + ```python |
| 29 | + def _resolve(self, path: str) -> str: |
| 30 | + if not path: |
| 31 | + return self.board_cwd |
| 32 | + if path.startswith("/"): |
| 33 | + return posixpath.normpath(path) |
| 34 | + joined = posixpath.normpath(posixpath.join(self.board_cwd, path)) |
| 35 | + return joined if joined != "." else "/" |
| 36 | + ``` |
| 37 | + (handles `cd ..`, `cd /`, and absolute get/put paths bypassing `board_cwd` for free). |
| 38 | + `cd` validates the target by calling `fs_listdir(new_path)` and catching the error — |
| 39 | + cheaper and more correct than stat + bit-masking `st_mode`, and reuses `ls`'s error |
| 40 | + path. |
| 41 | +- Local-side `lcd` just calls `os.chdir`; `get`/`put` resolve local paths via |
| 42 | + `Path(local).expanduser()` against the real `os.getcwd()` — no separate local-cwd |
| 43 | + field needed. |
| 44 | +- `RpcClient.call()` raises `RuntimeError` on RPC errors (never returns an |
| 45 | + error-shaped dict — confirmed in both `TcpClient.call` and `SidecarClient.call`). |
| 46 | + Every `do_*` body runs through a `_safe(self, fn, *a, **kw)` wrapper that catches |
| 47 | + `Exception`, prints `?{e}`, and returns — a failed command must never crash the |
| 48 | + REPL loop, unlike every existing one-shot `cmd_*` in `cli.py` where an uncaught |
| 49 | + exception propagating to `SystemExit(1)` is correct. |
| 50 | +- Command surface: `open [device] [baud]`, `close`, `bye`/`quit`/`EOF` (aliased to one |
| 51 | + impl, does **not** call `client.close()` itself — that stays the caller's job, |
| 52 | + matching every existing `cmd_*`'s separation of concerns), `ls`/`dir` (aliased), |
| 53 | + `cd`, `pwd`, `lcd`, `get`, `put`, `mget` (fnmatch against one |
| 54 | + `fs_listdir(board_cwd)` call), `mput` (stdlib `glob.glob` in the local cwd), |
| 55 | + `delete`/`rm` (alias), `mkdir`, `prompt` (toggles y/n confirm for mget/mput, |
| 56 | + default on), `ascii`/`binary` (one-line no-op informational prints — board fs is |
| 57 | + always byte-exact), `status`, `help` (free from `cmd.Cmd`). |
| 58 | +- `get`/`put` are a deliberately trimmed reimplementation (no `--mpy` compile, no |
| 59 | + `--verify`) — extracting shared helpers from `cmd_get`/`cmd_put` in `cli.py` would |
| 60 | + mean threading `board_cwd` resolution and the REPL's non-fatal error handling back |
| 61 | + through one-shot functions, for ~15 lines of savings each. Not worth it now; |
| 62 | + revisit only if verify-on-shell turns out to matter in practice (still just an |
| 63 | + inline `fs_hash` call, not an import from `cli.py`). |
| 64 | +- `shell.py` must **not** import from `cli.py` (that would be circular, since |
| 65 | + `cli.py` imports `shell.py`) — accept the client as a duck-typed constructor arg. |
| 66 | + |
| 67 | +## Files |
| 68 | + |
| 69 | +- **New `cli/src/mpftp/shell.py`** — `FtpShell(cmd.Cmd)`, `_resolve`, `_safe`, the |
| 70 | + `do_*` methods above, dynamic `self.prompt = f"mpftp:{self.board_cwd}> "` updated |
| 71 | + on every state-changing command. |
| 72 | +- **Edited `cli/src/mpftp/cli.py`** — `from . import shell` near the existing |
| 73 | + `from . import config`; new `cmd_shell(ns)` near `cmd_watch_repl`/`cmd_watch`: |
| 74 | + ```python |
| 75 | + def cmd_shell(ns: argparse.Namespace) -> None: |
| 76 | + client, mode = get_client() |
| 77 | + try: |
| 78 | + if ns.device: |
| 79 | + ensure_device(client, ns.device, ns.baud) |
| 80 | + shell.FtpShell(client, mode, ns.device, ns.baud).cmdloop() |
| 81 | + finally: |
| 82 | + if mode.startswith("sidecar"): |
| 83 | + client.close() |
| 84 | + ``` |
| 85 | + and one new subparser registration next to `watch-repl`: |
| 86 | + `sub.add_parser("shell", parents=[device_opts], help="Interactive FTP-style session").set_defaults(func=cmd_shell)`. |
| 87 | + No other existing function changes. |
| 88 | +- **New `cli/tests/test_shell.py`** — mirrors `test_mip.py`'s `FakeClient(RpcClient)` |
| 89 | + + `redirect_stdout` pattern, driving via `shell.onecmd(...)`. Cases: `cd` into a |
| 90 | + subdir then `cd ..` returns to `/`; `cd` to a nonexistent dir leaves `board_cwd` |
| 91 | + unchanged and prints an error instead of raising; `get`/`put` with the second arg |
| 92 | + omitted; a `RuntimeError` from a fake client's `call()` is caught and printed, not |
| 93 | + raised, and `onecmd` returns cleanly; `bye` returns a truthy stop value and does |
| 94 | + **not** call `client.close()`. |
| 95 | +- **Optional doc note in `docs/agent-guide.md`** (near the existing `watch-repl` |
| 96 | + mention) — state explicitly that `mpftp shell` is for humans and agents should |
| 97 | + keep scripting the flat one-shot subcommands, so a future agent reading the docs |
| 98 | + doesn't try to drive the REPL. |
| 99 | +- **Optional short subsection in `docs/user-guide.md`** near the existing |
| 100 | + connect/transfer docs, for discoverability. |
| 101 | + |
| 102 | +## Verification |
| 103 | + |
| 104 | +```bash |
| 105 | +cd cli && python -m pytest tests/test_shell.py -v |
| 106 | +python -m pytest tests/ # full suite — confirm the new cli.py import causes no regressions |
| 107 | +``` |
| 108 | + |
| 109 | +Manual smoke test (non-device parts need no hardware): |
| 110 | + |
| 111 | +``` |
| 112 | +mpftp shell |
| 113 | +mpftp:/> help |
| 114 | +mpftp:/> status # shows disconnected |
| 115 | +mpftp:/> open COM4 # or /dev/ttyACM0 — needs a real board |
| 116 | +mpftp:/> ls |
| 117 | +mpftp:/> cd lib |
| 118 | +mpftp:/> pwd |
| 119 | +mpftp:/> cd .. |
| 120 | +mpftp:/> lcd /tmp |
| 121 | +mpftp:/> put ./test.py |
| 122 | +mpftp:/> get test.py |
| 123 | +mpftp:/> mkdir scratch |
| 124 | +mpftp:/> delete /scratch |
| 125 | +mpftp:/> ascii |
| 126 | +mpftp:/> bye |
| 127 | +``` |
| 128 | + |
| 129 | +Then confirm the flat commands are unaffected: run the full existing test suite, plus |
| 130 | +one manual `mpftp ls /` outside the shell, to confirm no state leaked between the two |
| 131 | +code paths. |
0 commit comments