A malformed marker crashed the courtesy URL, and the hint guessed the port (#93)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: Shashankss1205

grapharc/cli/plan.py

@@ -107,14 +107,22 @@ def watch_url(trace_path: Path, *, run_id: str | None = None, timeout: float = 0
     marker = Path(".grapharc") / "live-server.json"
     try:
         record = json.loads(marker.read_text(encoding="utf-8"))
+        # TypeError is in the net for the marker shapes JSON allows but this
+        # code does not: a non-object document (indexing a list raises it) and
+        # a null port (`int(None)`). A malformed marker must degrade to the
+        # hint, never escape as a traceback from a command that only wanted to
+        # print a courtesy URL.
         root = Path(record["live_root"])
         base = str(record["url"])
         host, port = str(record["host"]), int(record["port"])
-    except (OSError, ValueError, KeyError):
+    except (OSError, ValueError, KeyError, TypeError):
         return None
     try:
-        rel = trace_path.resolve().relative_to(root)
-    except ValueError:
+        # The marker's root is resolved by the serve that wrote it; resolve it
+        # again here so a hand-edited or symlinked root still matches the same
+        # directory instead of failing the lexical comparison.
+        rel = trace_path.resolve().relative_to(root.resolve())
+    except (ValueError, OSError):
         return None
     try:
         with socket.create_connection((host, port), timeout=timeout):
@@ -127,22 +135,42 @@ def watch_url(trace_path: Path, *, run_id: str | None = None, timeout: float = 0
     return url
 
 
+def _marker_base() -> str:
+    """The last-known server base URL, from the marker; the default otherwise.
+
+    A marker that exists but whose server stopped answering still names the
+    host and port the operator actually uses — an instruction quoting a
+    different port than their `grapharc serve` command is a wrong instruction.
+    Read with the same tolerance as `watch_url`: any defect means the default.
+    """
+    import json
+
+    try:
+        record = json.loads(
+            (Path(".grapharc") / "live-server.json").read_text(encoding="utf-8")
+        )
+        return str(record["url"]).rstrip("/")
+    except (OSError, ValueError, KeyError, TypeError):
+        return "http://127.0.0.1:8000"
+
+
 def watch_hint(trace_path: Path) -> str:
     """What to print when no live server answers: the command, then the URL.
 
     The user asked for the link to always exist — so when it cannot be exact,
     it is an instruction that produces the exact one.
     """
+    base = _marker_base()
     try:
         rel = trace_path.resolve().relative_to((Path(".grapharc") / "runs").resolve())
         from urllib.parse import quote
 
-        would_be = f"http://127.0.0.1:8000/live/view?trace={quote(rel.as_posix(), safe='')}"
+        would_be = f"{base}/live/view?trace={quote(rel.as_posix(), safe='')}"
         return f"run `grapharc serve --live-root .grapharc/runs` then open {would_be}"
     except ValueError:
         return (
             f"run `grapharc serve --live-root {trace_path.parent}` "
-            f"then open http://127.0.0.1:8000/live"
+            f"then open {base}/live"
         )
 
 

tests/test_cli.py

@@ -2039,6 +2039,103 @@ def test_a_stale_marker_with_no_listener_falls_back_to_the_hint(
     assert payload["watch_url"] is None
 
 
+def test_a_malformed_marker_degrades_to_the_hint_instead_of_crashing(
+    tmp_path, monkeypatch
+):
+    """The marker shapes JSON allows but the reader does not: null port, a
+    non-object document. Both used to escape `watch_url` as a TypeError
+    traceback from a command that only wanted to print a courtesy URL."""
+    from grapharc.cli.plan import watch_url
+
+    monkeypatch.chdir(tmp_path)
+    marker = tmp_path / ".grapharc" / "live-server.json"
+    marker.parent.mkdir(parents=True, exist_ok=True)
+    trace = tmp_path / ".grapharc" / "runs" / "r1" / "trace.jsonl"
+    trace.parent.mkdir(parents=True, exist_ok=True)
+    trace.write_text("", encoding="utf-8")
+
+    marker.write_text(
+        json.dumps(
+            {
+                "url": "http://127.0.0.1:8000",
+                "host": "127.0.0.1",
+                "port": None,
+                "live_root": str((tmp_path / ".grapharc" / "runs").resolve()),
+            }
+        ),
+        encoding="utf-8",
+    )
+    assert watch_url(trace) is None
+
+    marker.write_text(json.dumps(["not", "an", "object"]), encoding="utf-8")
+    assert watch_url(trace) is None
+
+
+def test_an_unresolved_marker_root_still_matches_through_a_symlink(
+    tmp_path, monkeypatch
+):
+    """`serve` writes its root resolved; a hand-edited marker may not be. The
+    comparison resolves both sides now, so a symlinked spelling of the same
+    directory is the same directory rather than a lexical mismatch."""
+    import socket
+
+    from grapharc.cli.plan import watch_url
+
+    monkeypatch.chdir(tmp_path)
+    real = tmp_path / "real-runs"
+    real.mkdir()
+    link = tmp_path / "link-runs"
+    link.symlink_to(real, target_is_directory=True)
+    trace = real / "r1" / "trace.jsonl"
+    trace.parent.mkdir(parents=True)
+    trace.write_text("", encoding="utf-8")
+
+    listener = socket.socket()
+    listener.bind(("127.0.0.1", 0))
+    listener.listen(1)
+    port = listener.getsockname()[1]
+    try:
+        marker = tmp_path / ".grapharc" / "live-server.json"
+        marker.parent.mkdir(parents=True, exist_ok=True)
+        marker.write_text(
+            json.dumps(
+                {
+                    "url": f"http://127.0.0.1:{port}",
+                    "host": "127.0.0.1",
+                    "port": port,
+                    "live_root": str(link),  # deliberately unresolved
+                }
+            ),
+            encoding="utf-8",
+        )
+        url = watch_url(trace)
+    finally:
+        listener.close()
+    assert url is not None and "trace=r1%2Ftrace.jsonl" in url
+
+
+def test_the_hint_quotes_the_marker_port_when_the_server_is_down(
+    tmp_path, monkeypatch, capsys
+):
+    """A stale marker still names the port the operator actually serves on;
+    an instruction quoting a different port than their own `grapharc serve`
+    command is a wrong instruction."""
+    import socket
+
+    monkeypatch.chdir(tmp_path)
+    probe = socket.socket()
+    probe.bind(("127.0.0.1", 0))
+    dead_port = probe.getsockname()[1]
+    probe.close()
+    _write_live_marker(tmp_path, port=dead_port)
+    code = main(["plan", "look into it", "--scripted"])
+    printed = capsys.readouterr().out
+    assert code == 0
+    watch = next(line for line in printed.splitlines() if line.startswith("watch"))
+    assert "grapharc serve --live-root .grapharc/runs" in watch
+    assert f"127.0.0.1:{dead_port}/live/view?trace=" in watch
+
+
 def test_a_trace_outside_the_live_root_gets_no_exact_url(tmp_path, monkeypatch, capsys):
     import socket