diff --git a/milestones/README.md b/milestones/README.md index 36a9737..9114321 100644 --- a/milestones/README.md +++ b/milestones/README.md @@ -5,10 +5,13 @@ repositories that install `docgen` and maintain their own demo bundle. The library no longer ships an in-repo dogfood; consumers are the integration test of record. -**Active:** **[wizard-json-parse.md](wizard-json-parse.md)** — -wizard JSON bodies must parse; garbage JSON must not look like `{}`. +**Active:** **[wizard-path-ref.md](wizard-path-ref.md)** — +wizard `open-bundle` `path` and `tool/update` `ref` must be JSON strings. **Shipped:** +- **[wizard-json-parse.md](wizard-json-parse.md)** — + wizard JSON bodies must parse; garbage JSON must not look like `{}` + (#132). - **[wizard-source-paths.md](wizard-source-paths.md)** — wizard `source_paths` must be a string array; prose fields must be strings (#131). diff --git a/milestones/wizard-json-parse.md b/milestones/wizard-json-parse.md index d7ceb08..8e74ce3 100644 --- a/milestones/wizard-json-parse.md +++ b/milestones/wizard-json-parse.md @@ -1,7 +1,7 @@ # Milestone: wizard JSON bodies must parse -**Status:** Active -**PR:** (pending) +**Status:** Shipped +**PR:** [#132](https://github.com/jmjava/documentation-generator/pull/132) **Depends on:** `milestones/wizard-source-paths.md` (PR #131) ## Problem @@ -20,12 +20,12 @@ Empty / whitespace-only bodies stay ``{}``. Invalid JSON raises ## Done when -- [ ] Invalid JSON POST returns 400 and does not write state -- [ ] JSON ``null`` returns 400 -- [ ] Empty body still succeeds as ``{}`` -- [ ] `ruff check src/ tests/` -- [ ] `pytest tests/` -- [ ] `docgen benchmark` (no clock change; meets baseline) +- [x] Invalid JSON POST returns 400 and does not write state +- [x] JSON ``null`` returns 400 +- [x] Empty body still succeeds as ``{}`` +- [x] `ruff check src/ tests/` +- [x] `pytest tests/` (758 passed, 1 skipped) +- [x] `docgen benchmark` (no clock change; meets baseline) ## Out of scope diff --git a/milestones/wizard-path-ref.md b/milestones/wizard-path-ref.md new file mode 100644 index 0000000..0871bad --- /dev/null +++ b/milestones/wizard-path-ref.md @@ -0,0 +1,35 @@ +# Milestone: wizard open-bundle path and tool/update ref must be strings + +**Status:** Active +**PR:** [#133](https://github.com/jmjava/documentation-generator/pull/133) +**Depends on:** `milestones/wizard-json-parse.md` (PR #132) + +## Problem + +PR #131 typed wizard prose fields and ``source_paths``. ``POST /api/open-bundle`` +and ``POST /api/tool/update`` still used ``str(data.get(...) or default)``. + +``path: ["/tmp/bundle"]`` became ``"['/tmp/bundle']"`` and looked like a +missing yaml file. ``ref: true`` became ``"True"``, which matches the git-ref +allowlist and would ``pip install`` from that ref. ``ref: 0`` / ``ref: false`` +are falsy, so they silently fell through to ``"main"``. + +## Goal + +``path`` and ``ref`` must be JSON strings when present. Missing / null ``path`` +still means empty (existing ``path is required``). Missing / null ``ref`` still +defaults to ``main``. Empty ``ref`` still defaults to ``main``. + +## Done when + +- [x] List / bool / number ``path`` returns 400 +- [x] Bool / number / list ``ref`` returns 400 (does not pip-install) +- [x] Missing ``ref`` still defaults to ``main`` +- [x] `ruff check src/ tests/` +- [x] `pytest tests/` (760 passed, 1 skipped) +- [x] `docgen benchmark` (no clock change; meets baseline) + +## Out of scope + +- Wizard ``except Exception`` around ``narration_topic_label`` +- Focus ``paths`` missing vs empty (already typed as a list of strings) diff --git a/src/docgen/wizard.py b/src/docgen/wizard.py index f1d05b9..ec1babe 100644 --- a/src/docgen/wizard.py +++ b/src/docgen/wizard.py @@ -540,10 +540,11 @@ def api_session(): def api_open_bundle(): try: data = request_json_object() + path = require_json_string(data, "path", default="") or "" except WizardError as exc: return jsonify({"error": str(exc)}), 400 try: - cfg = open_bundle_config(str(data.get("path") or "")) + cfg = open_bundle_config(path) except ValueError as exc: return jsonify({"ok": False, "error": str(exc)}), 400 app.config["DOCGEN"] = cfg @@ -582,9 +583,9 @@ def api_tool_update(): data = request_json_object() with_manim = require_json_bool(data, "with_manim", default=False) update_req = require_json_bool(data, "update_requirements", default=True) + ref = require_json_string(data, "ref", default="main") or "main" except WizardError as exc: return jsonify({"error": str(exc)}), 400 - ref = str(data.get("ref") or "main") bundle = cfg.base_dir if cfg else None try: result = update_docgen_install( diff --git a/tests/test_gui_packaging.py b/tests/test_gui_packaging.py index 013279a..f93d741 100644 --- a/tests/test_gui_packaging.py +++ b/tests/test_gui_packaging.py @@ -197,6 +197,9 @@ def test_session_and_open_bundle(tmp_path: Path) -> None: assert missing.status_code == 400 empty = client.post("/api/open-bundle", json={"path": ""}) assert empty.status_code == 400 + listed = client.post("/api/open-bundle", json={"path": [str(tmp_path)]}) + assert listed.status_code == 400 + assert "path must be a JSON string" in listed.get_json()["error"] def test_open_bundle_config_helper(tmp_path: Path) -> None: diff --git a/tests/test_install_spec.py b/tests/test_install_spec.py index 2aba6b3..e70a0bc 100644 --- a/tests/test_install_spec.py +++ b/tests/test_install_spec.py @@ -133,8 +133,19 @@ class _Proc: assert data["restart_required"] is True assert read_requirements_pin(tmp_path / "requirements-docgen.txt") == "main" + with patch("docgen.install_spec.subprocess.run", return_value=_Proc()): + missing_ref = client.post( + "/api/tool/update", + json={"update_requirements": True, "with_manim": False}, + ) + assert missing_ref.status_code == 200, missing_ref.get_json() + assert missing_ref.get_json()["ref"] == "main" + bad = client.post("/api/tool/update", json={"ref": "main;id"}) assert bad.status_code == 400 + typed = client.post("/api/tool/update", json={"ref": True}) + assert typed.status_code == 400 + assert "ref must be a JSON string" in typed.get_json()["error"] def test_tool_info_without_requirements(tmp_path: Path) -> None: diff --git a/tests/test_wizard.py b/tests/test_wizard.py index d6a999d..eaed29a 100644 --- a/tests/test_wizard.py +++ b/tests/test_wizard.py @@ -210,6 +210,21 @@ def test_api_tool_update_rejects_string_with_manim(tmp_path): assert "with_manim must be a JSON boolean" in res.get_json()["error"] +def test_api_open_bundle_rejects_non_string_path(tmp_path): + client, _cfg = _wizard_client(tmp_path) + res = client.post("/api/open-bundle", json={"path": [str(tmp_path)]}) + assert res.status_code == 400 + assert "path must be a JSON string" in res.get_json()["error"] + + +def test_api_tool_update_rejects_non_string_ref(tmp_path): + client, _cfg = _wizard_client(tmp_path) + for payload in ({"ref": True}, {"ref": 0}, {"ref": False}, {"ref": ["main"]}): + res = client.post("/api/tool/update", json=payload) + assert res.status_code == 400, payload + assert "ref must be a JSON string" in res.get_json()["error"] + + def test_api_run_from_rejects_string_llm_scene_spec(tmp_path): client, _cfg = _wizard_client(tmp_path) res = client.post("/api/run-from/tts/01", json={"llm_scene_spec": "true"})