From fb2058041df1a607dad6d0dd34366fe6c3f77c4a Mon Sep 17 00:00:00 2001 From: Shashank Shekhar Singh Date: Thu, 6 Aug 2026 01:15:46 +0530 Subject: [PATCH] A malformed marker crashed the courtesy URL, and the hint guessed the port Co-Authored-By: Claude Fable 5 --- grapharc/cli/plan.py | 38 ++++++++++++++--- tests/test_cli.py | 97 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 5 deletions(-) diff --git a/grapharc/cli/plan.py b/grapharc/cli/plan.py index 1a7af0e..a70ead4 100644 --- a/grapharc/cli/plan.py +++ b/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" ) diff --git a/tests/test_cli.py b/tests/test_cli.py index bceaae2..64c696d 100644 --- a/tests/test_cli.py +++ b/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