Address review feedback

- authoring.save_project: write to a temporary file in the destination's
  directory and os.replace it into place, so an interrupted write cannot
  truncate an existing project (the MCP edit path rewrites the whole file on
  every tool call).
- authoring.column_values: GeoJSON permits `"properties": null`, which made
  `.get(column)` raise AttributeError instead of the documented ValueError.
  Guard with isinstance, matching layer_properties.
- mcp/workspace.Workspace: refuse an empty root set at construction. A
  separators-only GEOLIBRE_MCP_ROOTS filtered down to no roots and every later
  relative-path resolve raised IndexError instead of WorkspaceError.
- mcp/workspace.resolve_output: require a non-empty stem before the suffix, so
  a bare dotfile named exactly `.json` or `.html` is refused as the comment
  already claimed.
- mcp/server.edit: run the destination through the same extension allowlist
  create_project writes through, and check the loaded JSON actually looks like
  a GeoLibre project. load_project accepts any JSON object and seeds `layers`,
  so a mutating tool could previously rewrite an unrelated `package.json`
  inside a workspace root as a map.
- mcp/server.create_project: route the initial zoom through authoring.set_view
  so it is clamped to [0, 24] like every other camera-mutating path.
- mcp/server.add_geojson_layer: resolve the destination before fetching or
  reading `data`, so a bad path fails without paying for the download first.
- mcp/workspace: note that `.geolibre.json` in PROJECT_SUFFIXES is a subset of
  `.json` and is listed to name the convention in the client-facing error.
- Tests for each of the above, and drop the dead `is_dir()` conditional in
  test_workspace_rejects_traversal_out_of_a_root.
- CLAUDE.md: use the repository-relative `python/tests/test_mcp_server.py`.
- docs/mcp.md: document the two new write guards.

CLAUDE.md

@@ -83,7 +83,7 @@ Rendering is MapLibre GL JS in the webview, with **deck.gl** for raster/point-cl
 
 The browser build proxies the sidecar at `/sidecar` (same-origin, no CORS); confined to `GEOLIBRE_CONVERSION_ROOTS` (default `/data`). Local MBTiles use a custom MapLibre protocol backed by Tauri commands.
 
-**MCP server** (`python/src/geolibre/mcp/`, the `geolibre-mcp` console script): a headless stdio MCP server that authors `.geolibre.json` files. It is layered so nothing duplicates: `project.py` *builds* pieces (a layer, a plugin-state blob), `authoring.py` *applies* them to a whole project (add/remove/restyle a layer, move the camera, compose the legend/colorbar/swipe controls), and both `Map` and the MCP tools delegate to `authoring.py` — so a change to how a control is composed lands in one place. `server.py` is the only module that imports the `mcp` SDK (optional extra `geolibre[mcp]`), and `workspace.py` confines every path to `GEOLIBRE_MCP_ROOTS`/`--root` the way the sidecar confines to `GEOLIBRE_CONVERSION_ROOTS`. `tests/test_mcp_server.py` skips itself without the SDK, so `publish-python.yml` installs `mcp` explicitly — drop it and the server ships untested.
+**MCP server** (`python/src/geolibre/mcp/`, the `geolibre-mcp` console script): a headless stdio MCP server that authors `.geolibre.json` files. It is layered so nothing duplicates: `project.py` *builds* pieces (a layer, a plugin-state blob), `authoring.py` *applies* them to a whole project (add/remove/restyle a layer, move the camera, compose the legend/colorbar/swipe controls), and both `Map` and the MCP tools delegate to `authoring.py` — so a change to how a control is composed lands in one place. `server.py` is the only module that imports the `mcp` SDK (optional extra `geolibre[mcp]`), and `workspace.py` confines every path to `GEOLIBRE_MCP_ROOTS`/`--root` the way the sidecar confines to `GEOLIBRE_CONVERSION_ROOTS`. `python/tests/test_mcp_server.py` skips itself without the SDK, so `publish-python.yml` installs `mcp` explicitly — drop it and the server ships untested.
 
 ## Conventions
 

docs/mcp.md

@@ -74,9 +74,11 @@ server touches it, mirroring `GEOLIBRE_CONVERSION_ROOTS` in the
 inside a root that points out of it. Relative paths resolve against the first
 root, so a client can say `city.geolibre.json` without knowing the host layout.
 
-Two more guards on writes: the server only writes files ending in `.json`
-(projects) or `.html` (exports), and it refuses to replace an existing file
-unless the call passes `overwrite`.
+Three more guards on writes: the server only writes files ending in `.json`
+(projects) or `.html` (exports) — a bare `.json` with no name is refused too —
+it refuses to replace an existing file unless the call passes `overwrite`, and
+a tool that edits an existing project first checks the file actually is one, so
+an unrelated `package.json` sitting inside a root cannot be rewritten as a map.
 
 Give it a directory meant for maps, not your home directory.
 

python/src/geolibre/authoring.py

@@ -15,6 +15,8 @@ from __future__ import annotations
 import copy
 import json
 import math
+import os
+import tempfile
 from pathlib import Path
 from typing import Any, Callable, Iterable
 
@@ -81,6 +83,12 @@ def save_project(path: str | Path, project: dict[str, Any]) -> Path:
     and two-space indentation so it reads and diffs like the rest of the repo's
     JSON.
 
+    The write goes to a temporary file alongside the destination and is then
+    moved into place, so an interrupted write cannot leave a half-written
+    project where a complete one used to be. A project inlines its GeoJSON and
+    can approach ``MAX_PROJECT_BYTES``, and the MCP server rewrites the whole
+    file on every edit, so a truncating write is a real way to lose work.
+
     Args:
         path: Destination path.
         project: The project dict to serialize.
@@ -90,7 +98,17 @@ def save_project(path: str | Path, project: dict[str, Any]) -> Path:
     """
     file = Path(path).expanduser()
     file.parent.mkdir(parents=True, exist_ok=True)
-    file.write_text(json.dumps(project, indent=2) + "\n", encoding="utf-8")
+    handle = tempfile.NamedTemporaryFile(
+        "w", encoding="utf-8", dir=file.parent, prefix=f".{file.name}.", delete=False
+    )
+    temporary = Path(handle.name)
+    try:
+        with handle:
+            handle.write(json.dumps(project, indent=2) + "\n")
+        os.replace(temporary, file)
+    except BaseException:
+        temporary.unlink(missing_ok=True)
+        raise
     return file
 
 
@@ -286,11 +304,13 @@ def column_values(layer: dict[str, Any], column: str) -> list[Any]:
             f"Layer {layer.get('name')!r} has no inlined GeoJSON, so column "
             f"{column!r} cannot be read."
         )
-    values = [
-        feature.get("properties", {}).get(column)
-        for feature in geojson.get("features", [])
-        if isinstance(feature, dict)
-    ]
+    values = []
+    for feature in geojson.get("features", []):
+        if not isinstance(feature, dict):
+            continue
+        # GeoJSON permits `"properties": null`, so this cannot assume a dict.
+        properties = feature.get("properties")
+        values.append(properties.get(column) if isinstance(properties, dict) else None)
     if all(value is None for value in values):
         raise ValueError(f"Column {column!r} not found in any feature's properties")
     return values

python/src/geolibre/mcp/server.py

@@ -54,6 +54,33 @@ feature data.
 """
 
 
+#: Keys that mark a loaded JSON object as a GeoLibre project. ``load_project``
+#: accepts any JSON object and seeds ``layers`` when it is absent, so without
+#: this check an edit tool pointed at an unrelated JSON file inside a root —
+#: ``package.json``, say — would load it, apply the change, and write a project
+#: over it. Every project this package or the app writes carries both keys.
+PROJECT_MARKERS = ("mapView", "basemapStyleUrl")
+
+
+def _require_project(file: Path, project: dict[str, Any]) -> None:
+    """Refuse to rewrite a JSON file that is not a GeoLibre project.
+
+    Args:
+        file: The resolved path the project was loaded from.
+        project: The loaded JSON object.
+
+    Raises:
+        WorkspaceError: If the object carries no sign of being a project.
+    """
+    if any(key in project for key in PROJECT_MARKERS) or project.get("layers"):
+        return
+    raise WorkspaceError(
+        f"{file} does not look like a GeoLibre project (no "
+        f"{' or '.join(PROJECT_MARKERS)} key). Refusing to overwrite it; call "
+        "create_project to start a new one."
+    )
+
+
 def _summarize(path: Path, project: dict[str, Any], **extra: Any) -> dict[str, Any]:
     """Build a tool result: what changed, plus where the project now stands."""
     return {
@@ -85,10 +112,16 @@ def build_server(workspace: Workspace) -> MCPServer:
         """Load a project, yield it for mutation, and save it back.
 
         The write happens only if the body returns normally, so a tool that
-        raises part-way leaves the file on disk untouched.
+        raises part-way leaves the file on disk untouched. The destination has
+        to pass the same extension allowlist `create_project` writes through,
+        and to look like a project, so no tool can rewrite an unrelated JSON
+        file that happens to sit inside a root.
         """
-        file = workspace.resolve(path, must_exist=True)
+        file = workspace.resolve_output(path, suffixes=PROJECT_SUFFIXES, overwrite=True)
+        if not file.is_file():
+            raise WorkspaceError(f"File not found: {file}")
         project = authoring.load_project(file)
+        _require_project(file, project)
         yield file, project
         authoring.save_project(file, project)
 
@@ -141,7 +174,11 @@ def build_server(workspace: Workspace) -> MCPServer:
             The path written and the project's starting state.
         """
         file = workspace.resolve_output(path, suffixes=PROJECT_SUFFIXES, overwrite=overwrite)
-        project = _project.build_empty_project(name, center=center, zoom=zoom)
+        project = _project.build_empty_project(name, center=center)
+        if zoom is not None:
+            # Through set_view so the initial zoom is clamped to [0, 24] exactly
+            # as a later set_view call would clamp it.
+            authoring.set_view(project, zoom=zoom)
         if basemap:
             authoring.set_basemap(project, basemap)
         authoring.save_project(file, project)
@@ -211,6 +248,9 @@ def build_server(workspace: Workspace) -> MCPServer:
         Returns:
             The new layer's id and the project's updated layer count.
         """
+        # Check the destination before fetching or reading `data`, so a bad path
+        # fails without first paying for a 50 MB download.
+        workspace.resolve(path, must_exist=True)
         collection, source_url = load_geojson(data)
         layer = _project.geojson_layer(name, collection, source_url=source_url, **(style or {}))
         return add(path, layer, index)

python/src/geolibre/mcp/workspace.py

@@ -18,7 +18,9 @@ from typing import Iterable
 ROOTS_ENV_VAR = "GEOLIBRE_MCP_ROOTS"
 
 #: Extensions the server will write a project to. Anything else is refused, so a
-#: mistaken path cannot overwrite source code or a dotfile.
+#: mistaken path cannot overwrite source code or a dotfile. ``.geolibre.json`` is
+#: a subset of ``.json`` and so never widens the check — it is listed to name the
+#: conventional extension in the error message a client sees.
 PROJECT_SUFFIXES = (".json", ".geolibre.json")
 
 #: Extensions the server will write an export to.
@@ -41,9 +43,10 @@ class Workspace:
                 directory.
 
         Raises:
-            WorkspaceError: If a named root does not exist or is not a
-                directory. Failing at startup is better than accepting every
-                later call and refusing it for a reason the client cannot see.
+            WorkspaceError: If no root is configured, or a named root does not
+                exist or is not a directory. Failing at startup is better than
+                accepting every later call and refusing it for a reason the
+                client cannot see.
         """
         candidates = list(roots) if roots else _roots_from_env()
         resolved: list[Path] = []
@@ -52,6 +55,13 @@ class Workspace:
             if not root.is_dir():
                 raise WorkspaceError(f"Workspace root is not a directory: {root}")
             resolved.append(root)
+        # A separators-only `GEOLIBRE_MCP_ROOTS` (":") survives the emptiness
+        # check in `_roots_from_env` and filters down to nothing here. Without
+        # this, `resolve` would raise IndexError on the first relative path.
+        if not resolved:
+            raise WorkspaceError(
+                f"No workspace roots configured. Pass --root or set {ROOTS_ENV_VAR}."
+            )
         self.roots = tuple(resolved)
 
     def __repr__(self) -> str:
@@ -109,7 +119,11 @@ class Workspace:
                 unsupported extension, or exists while ``overwrite`` is False.
         """
         target = self.resolve(path)
-        if not any(target.name.endswith(suffix) for suffix in suffixes):
+        # `len(name) > len(suffix)` requires a non-empty stem, so a bare dotfile
+        # named exactly `.json` or `.html` is refused rather than accepted.
+        if not any(
+            target.name.endswith(suffix) and len(target.name) > len(suffix) for suffix in suffixes
+        ):
             raise WorkspaceError(
                 f"Refusing to write {target.name}: expected a file ending in "
                 f"{' or '.join(suffixes)}."

python/tests/test_authoring.py

@@ -56,6 +56,14 @@ def test_save_project_writes_readable_json(proj, tmp_path):
     assert "\n  " in text
 
 
+def test_save_project_leaves_no_temporary_file_behind(proj, tmp_path):
+    """The write lands atomically, so only the project itself is left in place."""
+    out = tmp_path / "map.geolibre.json"
+    authoring.save_project(out, proj)
+    authoring.save_project(out, proj)
+    assert [path.name for path in tmp_path.iterdir()] == ["map.geolibre.json"]
+
+
 def test_load_project_missing_file(tmp_path):
     with pytest.raises(ValueError, match="not found"):
         authoring.load_project(tmp_path / "nope.json")
@@ -150,6 +158,19 @@ def test_column_values_rejects_a_missing_column(proj):
         authoring.column_values(proj["layers"][0], "nope")
 
 
+def test_column_values_tolerates_a_null_properties_member(proj):
+    """GeoJSON permits `"properties": null`; that feature reads as no value."""
+    layer = proj["layers"][0]
+    layer["geojson"]["features"].append(
+        {
+            "type": "Feature",
+            "properties": None,
+            "geometry": {"type": "Point", "coordinates": [0, 0]},
+        }
+    )
+    assert authoring.column_values(layer, "pop") == [10, 90, None]
+
+
 # -- layer mutation -----------------------------------------------------------
 
 

python/tests/test_mcp_server.py

@@ -9,6 +9,7 @@ from __future__ import annotations
 
 import asyncio
 import json
+import os
 
 import pytest
 
@@ -83,7 +84,7 @@ def test_workspace_rejects_a_path_outside_its_roots(tmp_path):
 
 
 def test_workspace_rejects_traversal_out_of_a_root(tmp_path):
-    workspace = Workspace([tmp_path / "inner"] if (tmp_path / "inner").is_dir() else [tmp_path])
+    workspace = Workspace([tmp_path])
     with pytest.raises(WorkspaceError, match="outside this server's workspace"):
         workspace.resolve("../../etc/passwd")
 
@@ -120,12 +121,26 @@ def test_workspace_guards_an_existing_output(tmp_path):
     assert workspace.resolve_output("a.json", suffixes=(".json",), overwrite=True)
 
 
+def test_workspace_rejects_a_bare_dotfile_output(tmp_path):
+    """`.json` is a suffix match but has no stem, so it is not a project name."""
+    workspace = Workspace([tmp_path])
+    with pytest.raises(WorkspaceError, match="Refusing to write"):
+        workspace.resolve_output(".json", suffixes=(".json",))
+
+
 def test_workspace_rejects_a_root_that_is_not_a_directory(tmp_path):
     missing = tmp_path / "nope"
     with pytest.raises(WorkspaceError, match="not a directory"):
         Workspace([missing])
 
 
+def test_workspace_rejects_an_environment_holding_only_separators(monkeypatch):
+    """A separators-only value filters down to no roots, which cannot serve."""
+    monkeypatch.setenv("GEOLIBRE_MCP_ROOTS", os.pathsep)
+    with pytest.raises(WorkspaceError, match="No workspace roots configured"):
+        Workspace()
+
+
 # -- tool surface -------------------------------------------------------------
 
 
@@ -164,6 +179,29 @@ def test_create_project_refuses_a_path_outside_the_workspace(server):
     )
 
 
+def test_create_project_clamps_the_initial_zoom(server, tmp_path):
+    """The starting camera is clamped the same way a later set_view would be."""
+    call(server, "create_project", path="map.geolibre.json", zoom=100)
+    written = json.loads((tmp_path / "map.geolibre.json").read_text(encoding="utf-8"))
+    assert written["mapView"]["zoom"] == 24
+
+
+def test_editing_refuses_a_json_file_that_is_not_a_project(server, tmp_path):
+    """An unrelated JSON file inside a root is not a project to be rewritten."""
+    package = tmp_path / "package.json"
+    package.write_text(json.dumps({"name": "app", "version": "1.0.0"}), encoding="utf-8")
+    error = call_error(
+        server, "add_geojson_layer", path="package.json", name="X", data=json.dumps(POINT_FC)
+    )
+    assert "does not look like a GeoLibre project" in error
+    assert json.loads(package.read_text(encoding="utf-8")) == {"name": "app", "version": "1.0.0"}
+
+
+def test_editing_refuses_a_destination_with_an_unsupported_extension(server, tmp_path):
+    (tmp_path / "notes.txt").write_text("{}", encoding="utf-8")
+    assert "Refusing to write" in call_error(server, "set_view", path="notes.txt", zoom=4)
+
+
 def test_add_geojson_layer_inlines_literal_geojson(server, project_path):
     result = call(
         server,