From 8682bb53b45b9d95ad17b9ffb5d36a4ca9334713 Mon Sep 17 00:00:00 2001 From: Shashank Shekhar Singh Date: Fri, 31 Jul 2026 22:37:45 +0530 Subject: [PATCH] Name the variable when a GRAPHARC_* env value cannot be coerced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A non-numeric GRAPHARC_MAX_TOKENS crashed grapharc run with a raw ValueError traceback, exit 1 and an empty stdout — so a CI gate on --check-only read "topology refused" when the truth was "someone exported GRAPHARC_MAX_TOKENS=unlimited in the job environment". plan did not traceback, but its broad handler relabelled the failure "could not build the plan: invalid literal…", pointing the reader at the model rather than their shell. Settings.resolve now wraps the env coercion and raises ConfigError with the wording the file layer already uses for the same mistake: GRAPHARC_MAX_TOKENS must be int, got 'unlimited'. An exported variable is invisible on the command line, which is exactly why the error must say which one it was. Every caller already catches ConfigError and routes it through fail(...) with exit 2, so in --json mode the failure is one parseable document on stdout with stderr empty — nothing else needed touching. Fixes #16 Co-Authored-By: Claude Fable 5 --- grapharc/cli/config.py | 11 ++++++++- tests/test_config.py | 53 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/grapharc/cli/config.py b/grapharc/cli/config.py index 34609e7..d4794bb 100644 --- a/grapharc/cli/config.py +++ b/grapharc/cli/config.py @@ -82,7 +82,16 @@ def resolve(self, key: str, flag_value: Any, default: Any = None) -> Any: env = os.environ.get(f"{ENV_PREFIX}{key.upper()}") if env: self.sources[key] = "env" - return KEYS.get(key, str)(env) + want = KEYS.get(key, str) + # Same named error the file layer raises for a mistyped value. An + # exported variable is invisible on the command line, which is + # exactly why the message must say which one it was. + try: + return want(env) + except ValueError as exc: + raise ConfigError( + f"{ENV_PREFIX}{key.upper()} must be {want.__name__}, got {env!r}" + ) from exc if key in self.values: self.sources[key] = "config" return self.values[key] diff --git a/tests/test_config.py b/tests/test_config.py index dd05cc2..c5a2112 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -161,6 +161,59 @@ def test_a_bool_is_not_accepted_where_a_count_belongs(tmp_path): load(path) +def test_a_non_coercible_env_value_names_the_variable(monkeypatch): + """The file layer already names this mistake; the env layer used to let it + escape as a raw ValueError. An exported variable is invisible on the + command line, which is exactly why the error must say which one it was.""" + monkeypatch.setenv("GRAPHARC_MAX_TOKENS", "unlimited") + + with pytest.raises(ConfigError, match="GRAPHARC_MAX_TOKENS must be int, got 'unlimited'"): + Settings().resolve("max_tokens", None) + + +def test_run_under_a_bad_env_value_fails_as_one_document(tmp_path, monkeypatch, capsys): + """The `--json` contract, held even for this failure: exit 2, one parseable + document on stdout, nothing on stderr. It used to be exit 1, an empty + stdout and a traceback — a CI gate read that as "topology refused".""" + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("GRAPHARC_MAX_TOKENS", "unlimited") + graph = tmp_path / "graph.json" + graph.write_text( + json.dumps( + { + "nodes": [{"name": "triage"}], + "edges": [ + {"source": "__start__", "target": "triage"}, + {"source": "triage", "target": "__end__"}, + ], + } + ), + encoding="utf-8", + ) + + code = main(["run", str(graph), "--check-only", "--json"]) + out, err = capsys.readouterr() + payload = json.loads(out) + + assert code == 2 + assert payload["ok"] is False + assert "GRAPHARC_MAX_TOKENS must be int, got 'unlimited'" in payload["error"] + assert err == "" + + +def test_plan_reports_the_env_error_not_a_planner_one(tmp_path, monkeypatch, capsys): + """The broad handler used to relabel this "could not build the plan: + invalid literal…" — pointing the reader at the model, not their shell.""" + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("GRAPHARC_MAX_ROUNDS", "abc") + + code, payload = _plan_payload(capsys) + + assert code == 2 + assert "GRAPHARC_MAX_ROUNDS must be int, got 'abc'" in payload["error"] + assert "could not build the plan" not in payload["error"] + + def test_an_explicit_config_that_does_not_exist_is_an_error(tmp_path): with pytest.raises(ConfigError, match="no such file"): load(tmp_path / "absent.toml")