Editor CLI robustness for container agents; stage validation; skill discipline

docs/skill/SKILL.md

@@ -50,9 +50,10 @@ Paths are relative to this file's directory.
   selected, and you MUST read
   `references/domains/editor-collaboration.md` first to see what they
   see (selection cell, `sca snapshot`, cell references) and how to
-  hand the work back. While the editor runs, change its notebooks with
-  `sca cell` (or the file; never both blindly): the editor is the
-  file's writer.
+  hand the work back. While the editor runs, `sca cell` is the only way to
+  change a regular cell; edit the file only when no editor is running, or
+  for the setup / `@app.function` cells `sca cell` refuses: the editor is
+  the file's writer.
 - Creating, installing, updating, or removing a SimpleCADAPI addon (a
   third-party skill+tooling package around `.scadpkg`) enters no
   workflow; you MUST use `references/domains/addon-development.md`
@@ -147,6 +148,13 @@ it and stop that route; never invent an alternative route.
     them in the editor for a person); never enable
     marimo's `[tool.marimo.runtime] cache_cells` in the editor, and
     never commit the generated `__marimo__/` directory.
+12. Manage the modeling project the way software is managed: put the
+    workspace under git from the first file, commit at verified
+    milestones (a passed verification, not a wall clock) with messages
+    that say what changed and why, and use a branch per design
+    direction when alternatives are on the table. Generated state —
+    `__marimo__/`, `.sca-editor/`, `out/` — belongs in `.gitignore`,
+    never in a commit.
 
 ## Boundaries
 

docs/skill/references/domains/editor-collaboration.md

@@ -12,11 +12,17 @@ tuned value) is also something you can read from disk. Never rely on what the
 person says they selected when the file or the snapshot can tell you.
 
 The editor holds the open notebook in memory and is the one writer of its
-file. You have two safe ways to change it while it runs; use the first:
-
-1. **`sca cell` (preferred).** It edits through the editor: the page shows the
+file. While it runs, **`sca cell` is the only way to change a regular cell**:
+writing the file behind the editor's back races its next save, and the page's
+version or yours is lost. Edit the file directly only when no editor is
+running (then nothing else holds it), or for the cells `sca cell` refuses
+(the setup cell and `@app.function` cells): the editor takes those file
+changes in.
+
+1. **`sca cell` (always, for regular cells).** It edits through the editor: the page shows the
    change live (no reload, no flicker), the cell runs, and you get its state back.
-2. **Edit the file.** The editor notices a saved change within a second, compares
+2. **Edit the file** — only with no editor running, or for a setup / `@app.function` cell. The
+   editor notices a saved change within a second, compares
    it with its cells and applies only what differs (changed cells are edited in
    place and run again, added or removed cells appear or go away). A change to the
    setup cell or an `@app.function` cell restarts the kernel instead. A file that

docs/skill/references/domains/sketch-and-features.md

@@ -81,6 +81,10 @@ Read the exact page under `references/docs/api/` for every API used:
 ## Validation gates
 
 - Record plane mapping, closure, solve status, and remaining DOF in the brief.
+- `dof=0` says the constraint system closed, not that the shape is the intended
+  one: angle and symmetry constraints can solve to the mirror solution. Verify
+  the promoted geometry by measurement (bounding box, area, edge counts), never
+  by the solver status alone.
 - When sketch promotion may change measured geometry, build Wire and Sketch
   from one parameter source and compare closure/area/bounds/segmentation before
   promoting.

src/simplecadapi/editor/cli.py

@@ -154,49 +154,109 @@ def _print_json(report: dict[str, Any]) -> None:
     print(json.dumps(report, indent=2, ensure_ascii=False))
 
 
+def _opener() -> Any:
+    """An opener that bypasses the proxy environment for the local editor.
+
+    The editor is on this machine (its url is 127.0.0.1); a container's
+    ``http_proxy`` does not know that and swallows the request, which once
+    read as "no editor is running" while one was.
+    """
+    import urllib.request
+
+    return urllib.request.build_opener(urllib.request.ProxyHandler({}))
+
+
 def _find_editor(start: Path) -> tuple[Path, str] | None:
-    """The running editor whose workspace holds *start*: ``(workspace, base url)``."""
+    """The running editor whose workspace holds *start*: ``(workspace, base url)``.
+
+    A record whose server does not answer is an error, not a stale record to
+    skip quietly: "an editor was running here" is exactly the belief that
+    must not be dropped in silence.  The one definite stale case is a record
+    whose process is gone (a crashed editor leaves its server.json behind).
+    """
     import json
-    import urllib.request
+    import os
+    import urllib.error
 
     here = start.expanduser().resolve()
+    unreachable: list[str] = []
+    opener = _opener()
     for directory in (here, *here.parents):
         record = directory / ".sca-editor" / "server.json"
         if not record.is_file():
             continue
         try:
-            url = json.loads(record.read_text(encoding="utf-8"))["url"]
-            with urllib.request.urlopen(f"{url}/api/health", timeout=1.5):
+            body = json.loads(record.read_text(encoding="utf-8"))
+            url = body["url"]
+        except (OSError, ValueError, KeyError) as error:
+            unreachable.append(f"{record}: unreadable ({error})")
+            continue
+        pid = body.get("pid")
+        if isinstance(pid, int):
+            try:
+                os.kill(pid, 0)
+            except ProcessLookupError:
+                unreachable.append(
+                    f"stale record {record} (pid {pid} is gone); delete it or restart the editor"
+                )
+                continue
+            except OSError:
+                pass  # a live process we cannot signal
+        try:
+            with opener.open(f"{url}/api/health", timeout=1.5):
                 pass
-        except (OSError, ValueError, KeyError):
-            continue  # a stale record: the editor is gone
+        except urllib.error.HTTPError:
+            pass  # an editor that answers at all is running
+        except OSError as error:
+            unreachable.append(f"found {record} but {url} is unreachable: {error}")
+            continue
         return directory, str(url)
+    if unreachable:
+        raise ValueError("; ".join(unreachable))
     return None
 
 
-def _call(url: str, path: str, body: dict | None = None) -> dict[str, Any]:
+def _call_timeout() -> float:
+    """How long one editor call may take; a heavy downstream rerun is on the
+    clock of the write that triggered it. ``SCA_EDITOR_CALL_TIMEOUT`` seconds."""
+    import os
+
+    return float(os.environ.get("SCA_EDITOR_CALL_TIMEOUT", "300"))
+
+
+def _call(url: str, path: str, body: dict | None = None, timeout: float | None = None) -> dict[str, Any]:
     import json
+    import socket
     import urllib.error
     import urllib.request
 
     data = None if body is None else json.dumps(body).encode("utf-8")
     request = urllib.request.Request(
         url + path, data=data, headers={"Content-Type": "application/json"}, method="GET" if body is None else "POST"
     )
+    seconds = _call_timeout() if timeout is None else timeout
     try:
-        with urllib.request.urlopen(request, timeout=300) as response:
+        with _opener().open(request, timeout=seconds) as response:
             return json.loads(response.read())
     except urllib.error.HTTPError as error:
         try:
             return json.loads(error.read())
         except ValueError:
             return {"error": f"editor answered {error.code}"}
+    except (socket.timeout, TimeoutError) as error:
+        raise TimeoutError(f"the editor did not answer {path} within {seconds:g}s") from error
 
 
 def _session(args: argparse.Namespace, *, write: bool) -> tuple[str | None, Path | None, str]:
     """``(editor url or None, notebook path or None, relative notebook name)`` for a cell command."""
     start = args.workspace or (args.notebook.parent if args.notebook else Path.cwd())
-    found = _find_editor(start)
+    try:
+        found = _find_editor(start)
+    except ValueError as error:
+        if write:
+            raise
+        _warn(f"editor not reached: {error}")
+        found = None
     notebook = args.notebook.expanduser().resolve() if args.notebook else None
     if found is None:
         if write:
@@ -206,6 +266,7 @@ def _session(args: argparse.Namespace, *, write: bool) -> tuple[str | None, Path
             )
         if notebook is None:
             raise ValueError("no editor is running here; name the notebook with --notebook")
+        _warn(f"no editor is running on this workspace; reading the file: {notebook.name}")
         return None, notebook, notebook.name
     workspace, url = found
     open_path = _call(url, "/api/cells").get("notebook")
@@ -221,9 +282,17 @@ def _session(args: argparse.Namespace, *, write: bool) -> tuple[str | None, Path
             f"the editor has {open_path} open, not {relative}: edit {relative} directly "
             "(the editor does not hold it), or open it in the editor first"
         )
+    _warn(f"the editor has {open_path} open; reading {relative} from the file")
     return None, notebook, relative
 
 
+def _warn(message: str) -> None:
+    """Say a fallback out loud: silence once read as "the editor agreed"."""
+    import sys
+
+    print(f"warning: {message}", file=sys.stderr)
+
+
 def _cells_from_file(notebook: Path) -> list[dict[str, Any]]:
     from ..runtime._marimo import parse_notebook
 
@@ -239,7 +308,7 @@ def _cells_from_file(notebook: Path) -> list[dict[str, Any]]:
 def _slim(report: dict[str, Any], *, code: bool) -> dict[str, Any]:
     if "cells" not in report:
         return report
-    keep = ("index", "key", "name", "title", "status", "errors", "output", "ref", "readonly")
+    keep = ("index", "key", "name", "title", "status", "errors", "output", "console", "upstream", "ref", "readonly")
     return {
         **report,
         "cells": [
@@ -285,13 +354,25 @@ def _read_code(args: argparse.Namespace) -> str:
 def _write_op(args: argparse.Namespace, op: str, body: dict[str, Any]) -> tuple[dict[str, Any], int]:
     url, _notebook, _name = _session(args, write=True)
     assert url is not None
-    report = _call(url, f"/api/cells/{op}", body)
+    try:
+        report = _call(url, f"/api/cells/{op}", body)
+    except TimeoutError:
+        # The server took the write and reruns on its own; the clock that ran
+        # out is this call's, not the editor's.
+        return {
+            "error": "the edit was applied and the rerun is still going; check "
+            "`sca cell ls` (SCA_EDITOR_CALL_TIMEOUT raises the wait)"
+        }, 1
     if "error" in report:
         return _slim(report, code=False), 1
     touched = set(report.get("touched", []))
     report = _slim(report, code=False)
-    # What the change did: the touched cells with their state; the rest as a count.
-    report["cells"] = [c for c in report["cells"] if c.get("index") in touched] or report["cells"]
+    # What the change did: the touched cells with their state, plus any cell
+    # that is not healthy after the run — a rerun dependent failing is the
+    # feedback the edit was after, hiding it once read as "the edit was fine".
+    report["cells"] = [
+        c for c in report["cells"] if c.get("index") in touched or c.get("errors") or c.get("blocked")
+    ] or report["cells"]
     return report, 0
 
 

src/simplecadapi/editor/server.py

@@ -37,6 +37,7 @@
 
 from ..runtime.cells import cell_keys
 from ..runtime.config import NotebookConfig, read_notebook_config
+from ..runtime._marimo import compile_cell
 from ..runtime.params import (
     ParamSpec,
     commit_field_value,
@@ -601,7 +602,7 @@ def ingest_external(self, kernel: MarimoKernel) -> None:
     # -- the same cell edits, for an agent (``sca cell``) -------------------------
 
     def _brief(self, number: int, cell: dict) -> dict:
-        return {
+        brief = {
             "index": number,
             "id": cell["id"],
             "key": cell.get("key"),
@@ -615,13 +616,34 @@ def _brief(self, number: int, cell: dict) -> dict:
             "ref": cell.get("ref"),
             "code": cell["code"],
         }
+        if cell.get("blocked"):
+            # The cell did not itself fail: an ancestor did (its report says which).
+            brief["blocked"] = True
+        return brief
 
     def cells_report(self, *, refresh: bool = True) -> dict:
         if refresh and self.kernel is not None:
             self.push_cells()
+        upstream = _upstream_of(self.cells)
+        place = {cell["id"]: (number, cell) for number, cell in enumerate(self.cells, start=1)}
+        briefs = []
+        for number, cell in enumerate(self.cells, start=1):
+            brief = self._brief(number, cell)
+            if brief["errors"] or brief.get("blocked"):
+                # The cell the error names is often not the one at fault: say
+                # what it reads, the failing ancestors first.
+                brief["upstream"] = [
+                    {
+                        "index": place[dep][0],
+                        "name": place[dep][1].get("name"),
+                        "failing": bool(place[dep][1].get("errors")),
+                    }
+                    for dep in sorted(upstream.get(cell["id"], ()), key=lambda cid: place[cid][0])
+                ]
+            briefs.append(brief)
         return {
             "notebook": self.notebook.relative_to(self.workspace).as_posix() if self.notebook else None,
-            "cells": [self._brief(n, c) for n, c in enumerate(self.cells, start=1)],
+            "cells": briefs,
         }
 
     def _find_cell(self, ref: Any) -> dict:
@@ -654,9 +676,16 @@ def cell_command(self, op: str, body: dict) -> dict:
                 if body.get("base") is not None and _module_code(str(body["base"])).strip() != _module_code(cell["code"]).strip():
                     return {"error": "conflict: the cell changed since you read it", "theirs": cell["code"]}
                 if code.strip() != _module_code(cell["code"]).strip():
-                    (kernel.edit_cell if body.get("run", True) else kernel.save_cell)(cell["id"], code)
+                    run = bool(body.get("run", True))
+                    (kernel.edit_cell if run else kernel.save_cell)(cell["id"], code)
+                    if run:
+                        # The edit runs the cell itself; the sweep adds the
+                        # dependents it made stale — their state is the
+                        # feedback the caller asked for.
+                        kernel.wait_idle()
+                        kernel.run_until_settled([])
                 elif body.get("run", True):
-                    kernel.run_cells([cell["id"]])
+                    kernel.run_until_settled([cell["id"]])
                 touched = [cell["id"]]
             elif op == "insert":
                 after = body.get("after")
@@ -668,9 +697,11 @@ def cell_command(self, op: str, body: dict) -> dict:
                     anchor = self._find_cell(after)["id"]
                 code = _module_code(str(body.get("code", ""))) or "# new cell"
                 name = str(body["name"]) if body.get("name") else None
-                created = _created_id(kernel.create_cell_after(anchor, code, name, run=bool(body.get("run", True))))
+                created = _created_id(kernel.create_cell_after(anchor, code, name, run=False))
                 if created is None:
                     return {"error": "the kernel did not create the cell"}
+                if body.get("run", True):
+                    kernel.run_until_settled([created])
                 touched = [created]
             elif op == "delete":
                 cell = self._find_cell(body.get("cell"))
@@ -1430,8 +1461,10 @@ def write_endpoint(self, host: str) -> None:
             directory.mkdir(parents=True, exist_ok=True)
             import os
 
+            # The record is read on this machine, so the url is always the
+            # loopback: a 0.0.0.0 url is no address (and walks into proxies).
             (directory / "server.json").write_text(
-                json.dumps({"url": f"http://{host}:{self.port}", "pid": os.getpid(), "workspace": str(self.workspace)}),
+                json.dumps({"url": f"http://127.0.0.1:{self.port}", "pid": os.getpid(), "workspace": str(self.workspace)}),
                 encoding="utf-8",
             )
         except OSError:
@@ -1486,6 +1519,36 @@ def _export(self) -> None:
         self.hub.broadcast({"type": "status", "exported": str(target)})
 
 
+def _upstream_of(cells: list[dict]) -> dict[str, set[str]]:
+    """Per cell id: the ids of the cells it (transitively) reads.
+
+    Compiled with marimo's own cell compiler, so a name is a dependency
+    exactly when marimo's dataflow says so; a cell mid-edit (its code does
+    not compile) contributes no edges rather than wrong ones.
+    """
+    owner: dict[str, str] = {}
+    refs: dict[str, frozenset[str]] = {}
+    for cell in cells:
+        try:
+            impl = compile_cell(_module_code(cell["code"]), cell["id"])
+        except Exception:
+            refs[cell["id"]] = frozenset()
+            continue
+        refs[cell["id"]] = impl.refs
+        for name in impl.defs:
+            owner.setdefault(name, cell["id"])
+    direct = {cid: {owner[name] for name in names if name in owner and owner[name] != cid} for cid, names in refs.items()}
+    closure: dict[str, set[str]] = {}
+
+    def walk(cid: str) -> set[str]:
+        if cid not in closure:
+            closure[cid] = set()  # marimo forbids cycles; the placeholder guards one anyway
+            closure[cid] = set().union(*(walk(dep) | {dep} for dep in direct.get(cid, ()))) if direct.get(cid) else set()
+        return closure[cid]
+
+    return {cid: walk(cid) for cid in refs}
+
+
 def _module_code(code: str) -> str:
     """marimo's code_mode compiles cell code as a MODULE: a ``return`` in the
     cell's own scope (normal in marimo's file format) is a SyntaxError there.