fix(slack): reject non-finite timing settings and approval waits

docs/cookbook/06-serving-and-ops.md

@@ -1707,6 +1707,11 @@ Exit codes are part of the interface: `0` did the job, `1` ran and the answer
 was negative (two runs differed, a run id had no events, no backend was usable),
 `2` could not run at all (missing file, missing component, unknown model spec).
 
+`trace`, `metrics` and `viz` also exit `2` when a trace contains an invalid
+event or non-UTF-8 bytes. The error names the file and its 1-based line; with
+`--json` it is one failure document on stdout and stderr is empty. These
+strict readers refuse the file rather than present a partial audit trail.
+
 A whole session, verbatim (run ids are random per run and durations are
 wall-clock; the test maps the former, masks the latter, and byte-compares every
 other character):

docs/cookbook/07-slack.md

@@ -116,6 +116,12 @@ Configuration is environment-only, read once at startup:
 | `GRAPHARC_SLACK_LIVE_INTERVAL` | `2.5` | seconds between two edits of the status message |
 | `GRAPHARC_SLACK_LIVE_URL` | unset | base URL of a `grapharc serve --live-root` the requester can reach; posts a "watch live" link |
 
+The timeout and live-interval environment values must be finite and positive;
+fractions of a second are accepted. `NaN`, infinity and overflowing values
+such as `1e309` are startup errors naming the variable. A requester-supplied
+`--approval-timeout` must also be finite and positive and fit within the
+command's existing timeout ceiling.
+
 The bot reads tokens from the process environment only. The model gateway's
 `.env` loader is deliberately not used here — even though it now reads the
 working directory alone rather than searching upward: a bot that a whole

docs/deep-dive.md

@@ -254,7 +254,7 @@ A stable system is not one that claims to have no edges — it is one whose edge
 - **`.env` and `grapharc.toml` follow the same discovery rule: the working directory, and nowhere else.** Neither searches parent directories — a run must not be governed by a file you did not know about, and must not be *billed* to one either. **This is a behaviour change:** the credential loader used to walk up to `/`, so a `.env` in an ancestor directory (a `$HOME` one on a shared box, a client project one above a demo checkout) was picked up silently. If you relied on that, move the file into the directory you run from, `export` the variable, or pass `env_file=` to name it explicitly. A real environment variable still beats any file.
 - **`grapharc run` has no budget unless you give it one.** Set any of `--max-tokens`, `--max-iterations`, `--max-seconds`, or `--max-concurrency`; without them each dimension is unlimited and the gate admits a topology of any worst-case cost.
 
-**Verified this pass:** `pytest` → green, 2,225 selected and 13 deselected (the live ones); `ruff check .` clean; all eight `grapharc demo` stages green, plus the `trace` / `metrics` / `viz` / `replay` tour against a freshly recorded demo trace; the wheel builds and imports all submodules in a clean virtualenv with `[all]`, and `0.1.8` on PyPI is that wheel. The counts are a snapshot, not a property of the project — `pytest` re-derives them in one command, which is the only reason they are quoted, and `tests/test_deep_dive.py` fails this line rather than letting it drift.
+**Verified this pass:** `pytest` → green, 2,267 selected and 13 deselected (the live ones); `ruff check .` clean; all eight `grapharc demo` stages green, plus the `trace` / `metrics` / `viz` / `replay` tour against a freshly recorded demo trace; the wheel builds and imports all submodules in a clean virtualenv with `[all]`, and `0.1.8` on PyPI is that wheel. The counts are a snapshot, not a property of the project — `pytest` re-derives them in one command, which is the only reason they are quoted, and `tests/test_deep_dive.py` fails this line rather than letting it drift.
 
 [ROADMAP.md](../ROADMAP.md) tracks what is built and what is not, item by item.
 

grapharc/observe/trace.py

@@ -232,13 +232,19 @@ def read_events(self, run_id: str | None = None) -> list[TraceEvent]:
         if not self.path.exists():
             return []
         events = []
-        with self.path.open(encoding="utf-8") as f:
-            for line_number, line in enumerate(f, start=1):
-                if not line.strip():
-                    continue
+        # Decode one JSONL line at a time. TextIO's buffered decoder can fail
+        # before yielding an earlier valid line when later bytes are not UTF-8,
+        # losing both the trace-error boundary and the offending line number.
+        with self.path.open("rb") as f:
+            # Preserve the text reader's LF, CRLF and CR line boundaries.
+            lines = (line for raw in f for line in raw.splitlines(keepends=True))
+            for line_number, raw in enumerate(lines, start=1):
                 try:
+                    line = raw.decode("utf-8")
+                    if not line.strip():
+                        continue
                     ev = TraceEvent.model_validate_json(line)
-                except ValidationError as exc:
+                except (UnicodeDecodeError, ValidationError) as exc:
                     raise TraceReadError(self.path, line_number, exc) from exc
                 if run_id is None or ev.run_id == run_id:
                     events.append(ev)

grapharc/slack/command.py

@@ -40,6 +40,7 @@
 from __future__ import annotations
 
 import importlib
+import math
 import shlex
 import tomllib
 import uuid
@@ -530,10 +531,10 @@ def parse_command(
                 raise SlackCommandError(
                     f"`--approval-timeout` wants a number of seconds, got {supplied!r}"
                 ) from None
-            if asked <= 0 or asked > ceiling:
+            if not math.isfinite(asked) or asked <= 0 or asked > ceiling:
                 raise SlackCommandError(
                     f"`--approval-timeout {supplied}` does not fit this command's "
-                    f"budget: the wait must be between 1 and {ceiling:.0f} seconds, "
+                    f"budget: the wait must be finite, positive and at most {ceiling:.0f} seconds, "
                     "so that a run nobody answers ends by reporting a timeout "
                     "rather than by being killed mid-wait"
                 )

grapharc/slack/config.py

@@ -11,6 +11,7 @@
 
 from __future__ import annotations
 
+import math
 import os
 from dataclasses import dataclass, field
 from pathlib import Path
@@ -86,8 +87,8 @@ def from_env(cls, environ: dict[str, str] | None = None) -> SlackBotConfig:
             raise SlackConfigError(
                 f"GRAPHARC_SLACK_TIMEOUT must be a number of seconds, got {raw_timeout!r}"
             ) from None
-        if timeout <= 0:
-            raise SlackConfigError("GRAPHARC_SLACK_TIMEOUT must be positive")
+        if not math.isfinite(timeout) or timeout <= 0:
+            raise SlackConfigError("GRAPHARC_SLACK_TIMEOUT must be positive and finite")
 
         raw_work_timeout = env.get("GRAPHARC_SLACK_WORK_TIMEOUT", "1800")
         try:
@@ -97,8 +98,8 @@ def from_env(cls, environ: dict[str, str] | None = None) -> SlackBotConfig:
                 "GRAPHARC_SLACK_WORK_TIMEOUT must be a number of seconds, "
                 f"got {raw_work_timeout!r}"
             ) from None
-        if work_timeout <= 0:
-            raise SlackConfigError("GRAPHARC_SLACK_WORK_TIMEOUT must be positive")
+        if not math.isfinite(work_timeout) or work_timeout <= 0:
+            raise SlackConfigError("GRAPHARC_SLACK_WORK_TIMEOUT must be positive and finite")
         # A work budget under the reader budget is almost certainly a typo, and
         # the failure it produces is confusing: `plan --go` would be killed
         # sooner than `metrics`. Take the larger rather than obeying literally.
@@ -112,8 +113,8 @@ def from_env(cls, environ: dict[str, str] | None = None) -> SlackBotConfig:
                 "GRAPHARC_SLACK_LIVE_INTERVAL must be a number of seconds, "
                 f"got {raw_interval!r}"
             ) from None
-        if live_interval <= 0:
-            raise SlackConfigError("GRAPHARC_SLACK_LIVE_INTERVAL must be positive")
+        if not math.isfinite(live_interval) or live_interval <= 0:
+            raise SlackConfigError("GRAPHARC_SLACK_LIVE_INTERVAL must be positive and finite")
 
         live_url_base = env.get("GRAPHARC_SLACK_LIVE_URL", "").rstrip("/") or None
         if live_url_base is not None and not live_url_base.startswith(("http://", "https://")):

tests/test_slack_finite_timings.py

@@ -0,0 +1,59 @@
+"""Non-finite time settings must not reach Slack's timers or command runner."""
+
+from __future__ import annotations
+
+import pytest
+
+from grapharc.slack.command import SlackCommandError, parse_command
+from grapharc.slack.config import SlackBotConfig, SlackConfigError
+
+
+@pytest.mark.parametrize("value", ["nan", "inf", "-inf", "1e309"])
+@pytest.mark.parametrize(
+    "key", ["GRAPHARC_SLACK_TIMEOUT", "GRAPHARC_SLACK_WORK_TIMEOUT", "GRAPHARC_SLACK_LIVE_INTERVAL"]
+)
+def test_environment_timing_values_must_be_finite(tmp_path, key, value):
+    env = {
+        "SLACK_BOT_TOKEN": "test-bot",
+        "SLACK_APP_TOKEN": "test-app",
+        "GRAPHARC_SLACK_WORKDIR": str(tmp_path),
+        key: value,
+    }
+
+    with pytest.raises(SlackConfigError, match=key):
+        SlackBotConfig.from_env(env)
+
+
+@pytest.mark.parametrize("form", ["--approval-timeout nan", "--approval-timeout=NaN"])
+def test_a_nan_approval_wait_is_refused(tmp_path, form):
+    with pytest.raises(SlackCommandError, match="does not fit this command's budget"):
+        parse_command(
+            f"plan goal --scripted --go {form}",
+            workdir=tmp_path,
+            timeout_seconds=60,
+            work_timeout_seconds=180,
+        )
+
+
+def test_finite_fractional_timings_remain_configurable(tmp_path):
+    config = SlackBotConfig.from_env(
+        {
+            "SLACK_BOT_TOKEN": "test-bot",
+            "SLACK_APP_TOKEN": "test-app",
+            "GRAPHARC_SLACK_WORKDIR": str(tmp_path),
+            "GRAPHARC_SLACK_TIMEOUT": "12.5",
+            "GRAPHARC_SLACK_WORK_TIMEOUT": "25.5",
+            "GRAPHARC_SLACK_LIVE_INTERVAL": "0.25",
+        }
+    )
+
+    assert config.timeout_seconds == 12.5
+    assert config.work_timeout_seconds == 25.5
+    assert config.live_interval_seconds == 0.25
+    argv = parse_command(
+        "plan goal --scripted --approval-timeout 1.5",
+        workdir=tmp_path,
+        timeout_seconds=config.timeout_seconds,
+        work_timeout_seconds=config.work_timeout_seconds,
+    )
+    assert argv[argv.index("--approval-timeout") + 1] == "1.5"

tests/test_trace_encoding.py

@@ -0,0 +1,84 @@
+"""Strict trace readers report corrupt encoding at its actual JSONL line."""
+
+from __future__ import annotations
+
+import json
+
+import pytest
+
+from grapharc.cli.main import main
+from grapharc.observe.trace import TraceReadError, TraceRecorder
+
+BAD_LINES = [
+    pytest.param(b'{"node":"\xff"}\n', id="invalid-byte"),
+    pytest.param(b"\xff\xfe\x00binary\n", id="utf16"),
+    pytest.param(b'{"node":"\xe2\x82"}\n', id="incomplete-character"),
+]
+READERS = [
+    pytest.param(["trace"], id="trace"),
+    pytest.param(["metrics", "r1"], id="metrics"),
+    pytest.param(["viz", "r1"], id="viz"),
+]
+
+
+def _trace(tmp_path, bad_line: bytes, line_number: int = 2):
+    recorder = TraceRecorder(tmp_path / "bad.jsonl")
+    if line_number == 2:
+        recorder.event(run_id="r1", graph="g", node="मॉडल", phase="start", step=1)
+    with recorder.path.open("ab") as handle:
+        handle.write(bad_line)
+    return recorder
+
+
+@pytest.mark.parametrize("bad_line", BAD_LINES)
+@pytest.mark.parametrize("line_number", [1, 2])
+def test_encoding_errors_use_the_trace_error_and_actual_line(tmp_path, bad_line, line_number):
+    recorder = _trace(tmp_path, bad_line, line_number)
+
+    with pytest.raises(TraceReadError) as caught:
+        recorder.read_events("r1")
+
+    assert caught.value.path == recorder.path
+    assert caught.value.line_number == line_number
+    assert isinstance(caught.value.cause, UnicodeDecodeError)
+
+
+@pytest.mark.parametrize("argv", READERS)
+@pytest.mark.parametrize("bad_line", BAD_LINES)
+@pytest.mark.parametrize("as_json", [False, True], ids=["text", "json"])
+def test_cli_reports_non_utf8_trace_without_a_traceback(tmp_path, capsys, argv, bad_line, as_json):
+    recorder = _trace(tmp_path, bad_line)
+    flags = ["--json"] if as_json else []
+
+    assert main([argv[0], str(recorder.path), *argv[1:], *flags]) == 2
+
+    captured = capsys.readouterr()
+    message = f"unreadable trace file: {recorder.path}: line 2 is not a trace event"
+    if as_json:
+        assert json.loads(captured.out) == {"ok": False, "command": argv[0], "error": message}
+        assert captured.err == ""
+    else:
+        assert captured.out == ""
+        assert captured.err == f"error: {message}\n"
+
+
+@pytest.mark.parametrize("newline", [b"\n", b"\r\n", b"\r"], ids=["lf", "crlf", "cr"])
+def test_utf8_content_and_blank_lines_keep_their_meaning(tmp_path, newline):
+    recorder = TraceRecorder(tmp_path / "valid.jsonl")
+    recorder.path.write_text("\u2003\n", encoding="utf-8")
+    recorder.event(
+        run_id="r1",
+        graph="g",
+        node="मॉडल",
+        phase="end",
+        step=1,
+        state_delta={"answer": "café ✓"},
+    )
+    # A valid final event does not require a trailing newline.
+    recorder.path.write_bytes(recorder.path.read_bytes().rstrip(b"\n").replace(b"\n", newline))
+
+    events = recorder.read_events("r1")
+
+    assert len(events) == 1
+    assert events[0].node == "मॉडल"
+    assert events[0].state_delta == {"answer": "café ✓"}