Merge pull request #137 from CodeGraphContext/fix/slack-finite-timings

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

Author: Shashankss1205

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,252 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/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"