From 033f2fb4bf72d5ff0e45736045f109e80537e7d1 Mon Sep 17 00:00:00 2001 From: DLANSAMA Date: Sun, 16 Aug 2026 18:05:12 -0400 Subject: [PATCH] fix: emit one separator style in --json path fields on Windows A Windows `--json` envelope mixed separator styles: the typed local path fields (`file`, `path`, `output`, `workdir`, the `job` step paths, ...) came out with `\`, while human-facing messages, remote printer paths, and archive entries in the same document used `/`. A consumer could not compare or join two path fields without knowing which side produced each one. `utils._json_display_paths` already funnels every declared path key through one hook, so the fix goes there rather than at each call site: add `paths.json_path()` (separator normalization only) and apply it after the existing `~` compaction. `local_path` was the one declared local-path contract field missing from `_JSON_PATH_KEYS`; it is now covered. Home-directory `~` compaction is deliberately kept. It is a documented privacy guarantee (AGENTS.md, docs/api.md) that `tests/privacy_smoke.py` actively enforces, and `~` stays expandable via `paths.expand_path`. URL-valued path fields keep their own separators and their redaction. No behaviour change on macOS/Linux, where `os.sep` is already `/`. --- AGENTS.md | 2 +- CHANGELOG.md | 8 ++++ bambu_cli/paths.py | 20 ++++++++++ bambu_cli/utils.py | 11 +++++- docs/api.md | 2 + tests/test_job.py | 15 ++++++-- tests/test_jsonio.py | 57 ++++++++++++++++++++++++++++ tests/test_slice_stub_integration.py | 8 ++-- 8 files changed, 114 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3079c7f..660189e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,7 +18,7 @@ Prefer `job`/`send` for agent work. Always ask the user before running any comma ZIP files are opened safely. URL downloads and ZIP extraction have a 2048 MB safety limit via `--max-download-mb`. Conflicting files use a numbered sibling such as `model-1.stl`. -Agent-facing JSON path fields compact paths under the current home directory to `~`. Path-bearing JSON error messages use the same `~` compaction. +Agent-facing JSON path fields compact paths under the current home directory to `~`, and always use `/` separators (on Windows too). Path-bearing JSON error messages use the same `~` compaction. ## Agent workflows and client architecture diff --git a/CHANGELOG.md b/CHANGELOG.md index 530817f..2f31df3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ### Fixed +- Windows `--json`: local path fields (`file`, `path`, `output`, `local_path`, + `workdir`, `config_path`, the `job` step paths, …) are emitted with `/` + separators instead of `\`. A single envelope previously mixed `\` in its + typed path fields with the `/` used by human-facing messages, remote printer + paths, and archive entries, so a consumer could not compare or join two path + fields without knowing which produced which. `~` home compaction is + unchanged. No effect on macOS/Linux. + - User-facing docs now match shipped behaviour for `--confirm` on `job` / `send` (upload still runs; exit `0` `uploaded_not_printed`), the fail-closed camera streamer (opt-in, not auto-fallback), `doctor` fingerprint/`-v` diff --git a/bambu_cli/paths.py b/bambu_cli/paths.py index 6481a28..a57aa8b 100644 --- a/bambu_cli/paths.py +++ b/bambu_cli/paths.py @@ -11,6 +11,7 @@ __all__ = [ "expand_path", "display_path", + "json_path", "path_for_message", "exception_for_message", ] @@ -62,6 +63,25 @@ def display_path(path): return text +def json_path(path): + """Return a local path with separators normalized to ``/`` for JSON output. + + Agent-facing JSON path fields are emitted with forward slashes on every + platform, matching the convention ``path_for_message`` already uses for + human-facing text. Without this, a single Windows envelope mixes ``\\`` in + the typed path fields with the ``/`` that messages, remote printer paths, + and archive entries use — so a consumer cannot compare or join the two + without knowing which field came from where. + + Home-directory compaction is NOT applied here; callers pass a value that + has already been through the JSON ``~`` compaction, which is a documented + privacy guarantee (see AGENTS.md). ``expand_path`` reverses it. + """ + if path is None or os.sep == "/": + return path + return str(path).replace(os.sep, "/") + + def path_for_message(path): """Return a local path suitable for human and agent-facing messages.""" display = display_path(path) diff --git a/bambu_cli/utils.py b/bambu_cli/utils.py index 9583354..4d0c78e 100644 --- a/bambu_cli/utils.py +++ b/bambu_cli/utils.py @@ -43,6 +43,7 @@ def _ensure_parent_dir(path): "downloaded_path", "extracted_path", "file", + "local_path", "output", "path", "printable_path", @@ -70,6 +71,13 @@ def _redact_url_credentials(url): _HOME_DIR = os.path.expanduser("~") +def _json_path(path): + """Normalize separators for JSON path fields. Delegates to ``paths.json_path``.""" + from bambu_cli.paths import json_path + + return json_path(path) + + def _display_path(path): if not path: return path @@ -104,7 +112,8 @@ def _json_display_paths(value): result[key] = _compact_all_strings(item) elif key in _JSON_PATH_KEYS and (isinstance(item, str) or item is None): redacted = _redact_url_credentials(item) - result[key] = redacted if redacted != item else _display_path(item) + # A URL keeps its own separators; only local paths are normalized. + result[key] = redacted if redacted != item else _json_path(_display_path(item)) else: result[key] = _json_display_paths(item) return result diff --git a/docs/api.md b/docs/api.md index c3d71f6..659a207 100644 --- a/docs/api.md +++ b/docs/api.md @@ -53,6 +53,8 @@ Job failures may use the superset [`schemas/job_error.json`](schemas/job_error.j `next_command`, `recovery_hint`). Path fields under the current home directory are compacted to `~` in agent JSON. +Local path fields always use `/` separators, on Windows too, so one envelope +never mixes separator styles; expand `~` to get an openable path. ## Exit codes diff --git a/tests/test_job.py b/tests/test_job.py index 042a35e..f1ec548 100644 --- a/tests/test_job.py +++ b/tests/test_job.py @@ -38,11 +38,18 @@ def emit(self, record): from bambu_cli import utils # noqa: E402 from bambu_cli.cli import build_parser # noqa: E402 from bambu_cli.paths import display_path as _display_path # noqa: E402 +from bambu_cli.paths import json_path as _json_path # noqa: E402 from bambu_cli.constants import EXIT_COMMAND_ERROR, EXIT_FILE_ERROR # noqa: E402 from bambu_cli.context import RuntimeContext # noqa: E402 from bambu_cli.job import JobSteps, _run_job # noqa: E402 from bambu_cli.errors import BambuError + +def _json_path_field(path): + """The expected value of a JSON path field: ``~`` compaction, then ``/`` separators.""" + return _json_path(_display_path(str(path))) + + def default_steps(**overrides): """``JobSteps`` wired to the real command handlers, with optional fakes. @@ -460,7 +467,7 @@ def test_dry_run_local_printer_ready_file(tmp_path, capsys): assert payload["status"] == "dry_run_local_skipped" assert payload["would_upload"] is True assert payload["would_slice"] is False - assert payload["printable_path"] == _display_path(str(ready)) + assert payload["printable_path"] == _json_path_field(ready) def test_dry_run_local_gcode_3mf_is_print_ready_not_sliced(tmp_path, capsys): """A local .gcode.3mf is print-ready: dry-run must report would_slice=False.""" @@ -598,7 +605,7 @@ def test_output_created_when_needed(tmp_path, capsys): _run_job(_ctx(), args, steps) assert out_dir.is_dir() payload = _read_json(capsys) - assert payload["workdir"] == _display_path(str(out_dir)) + assert payload["workdir"] == _json_path_field(out_dir) assert payload["uploaded"] is True def test_output_ignored_for_printer_ready_local_file(tmp_path, capsys, caplog): @@ -888,7 +895,7 @@ def _download(_a): _run_job(_ctx(), args, steps) payload = _read_json(capsys) assert payload["would_download"] is True - assert payload["downloaded_path"] == _display_path(str(downloaded)) + assert payload["downloaded_path"] == _json_path_field(downloaded) assert payload["uploaded"] is True assert payload["remote_name"] == "model.3mf" @@ -916,7 +923,7 @@ def _download(_a): payload = _read_json(capsys) assert payload["would_extract"] is True assert payload["archive_entry"] == "part.stl" - assert payload["extracted_path"] == _display_path(str(extracted)) + assert payload["extracted_path"] == _json_path_field(extracted) assert payload["uploaded"] is True def test_url_invalid_max_download_mb_fails(capsys): diff --git a/tests/test_jsonio.py b/tests/test_jsonio.py index f70e432..1de5451 100644 --- a/tests/test_jsonio.py +++ b/tests/test_jsonio.py @@ -91,6 +91,63 @@ def test_display_path_requires_separator_boundary(monkeypatch): assert utils._display_path("/home/alice") == "~" +# --------------------------------------------------------------------------- +# paths.json_path / _json_display_paths — one separator style in JSON +# --------------------------------------------------------------------------- + + +def test_json_path_normalizes_windows_separators(monkeypatch): + """Local path fields emit "/" on Windows, matching path_for_message.""" + import bambu_cli.paths as paths + + monkeypatch.setattr(paths.os, "sep", "\\") + assert paths.json_path("~\\models\\cube.3mf") == "~/models/cube.3mf" + assert paths.json_path("D:\\out\\cube.3mf") == "D:/out/cube.3mf" + # Already-normalized input and None are pass-through. + assert paths.json_path("~/models/cube.3mf") == "~/models/cube.3mf" + assert paths.json_path(None) is None + + +def test_json_envelope_uses_one_separator_style(monkeypatch): + """A Windows envelope must not mix "\\" path fields with "/" everywhere else.""" + import bambu_cli.paths as paths + import bambu_cli.utils as utils + + monkeypatch.setattr(paths.os, "sep", "\\") + monkeypatch.setattr(utils, "_HOME_DIR", "C:\\Users\\alice") + monkeypatch.setattr(utils.os, "sep", "\\") + monkeypatch.setattr(utils.os, "altsep", "/") + + payload = utils._json_display_paths( + { + "file": "C:\\Users\\alice\\models\\cube.stl", + "path": "D:\\out\\cube_sliced.3mf", + "local_path": "C:\\Users\\alice\\w\\cube.3mf", + "remote_path": "/cube.3mf", + "filename": "cube_sliced.3mf", + } + ) + + assert payload["file"] == "~/models/cube.stl" + assert payload["path"] == "D:/out/cube_sliced.3mf" + # local_path is a declared local-path field, so it is normalized too. + assert payload["local_path"] == "~/w/cube.3mf" + # Remote printer paths are already "/" and must be left alone. + assert payload["remote_path"] == "/cube.3mf" + assert "\\" not in "".join(v for v in payload.values() if isinstance(v, str)) + + +def test_json_path_field_keeps_url_separators(monkeypatch): + """A URL in a path-keyed field keeps its own separators and stays redacted.""" + import bambu_cli.paths as paths + import bambu_cli.utils as utils + + monkeypatch.setattr(paths.os, "sep", "\\") + at = "@" + payload = utils._json_display_paths({"source": "https://user:pass" + at + "host.com/x.stl"}) + assert payload["source"] == "https://host.com/x.stl" + + # --------------------------------------------------------------------------- # utils._resolve_ip — do not cache failures # --------------------------------------------------------------------------- diff --git a/tests/test_slice_stub_integration.py b/tests/test_slice_stub_integration.py index 1b52337..260eca9 100644 --- a/tests/test_slice_stub_integration.py +++ b/tests/test_slice_stub_integration.py @@ -133,11 +133,13 @@ def test_success_emits_json_envelope(orca_env, capsys): payload = _last_json_object(capsys.readouterr().out) assert payload["status"] == "sliced" assert payload["command"] == "slice" - # emit_json compacts $HOME to ~ in every string value; on Windows CI the - # pytest tmpdir lives under the user profile, so expect the display form. + # emit_json compacts $HOME to ~ in every string value and normalizes local + # path separators to "/"; on Windows CI the pytest tmpdir lives under the + # user profile, so expect the compacted, forward-slashed form. + from bambu_cli.paths import json_path from bambu_cli.utils import _display_path - assert payload["path"] == _display_path(orca_env.outpath) + assert payload["path"] == json_path(_display_path(orca_env.outpath)) assert payload["filename"] == "model_sliced.3mf" assert payload["bytes"] > 0 assert payload["step_converted"] is False