From af7fcf7fd94490db33a78402709f43b16207ea14 Mon Sep 17 00:00:00 2001 From: chaoz23 Date: Sun, 9 Aug 2026 09:02:09 -0700 Subject: [PATCH] Fix crash on default charter: no-GM is exit 2, never a traceback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 'dmcheck run session.jsonl' with the packaged default charter (which names no GM) crashed with KeyError: 'rule' — core.check() returned an error pseudo-finding lacking the 'rule' key, and cli's counts summary assumed every finding has one. Same defect class as the 0.5.x 'init --dm' bug: worked in tests because tests always pass the fixture charter. Fix the class: check() now raises ValueError so a finding without 'rule' can never exist; cli routes it through the existing exit-2 error lane (JSON on stderr); watch validates GM up front; mcp already converts the raise to a clean error response. Regression tests cover cli and watch via subprocess (no-traceback asserted). Co-Authored-By: Claude Fable 5 --- dmcheck/cli.py | 2 +- dmcheck/core.py | 4 ++-- dmcheck/watch.py | 4 ++++ tests/test_core.py | 35 ++++++++++++++++++++++++++++++++--- 4 files changed, 39 insertions(+), 6 deletions(-) diff --git a/dmcheck/cli.py b/dmcheck/cli.py index a2c4cb9..5f68b66 100644 --- a/dmcheck/cli.py +++ b/dmcheck/cli.py @@ -207,10 +207,10 @@ def main(argv=None): ch["dice_authors"] = a.dice_bot transcript = load_transcript(a.transcript) ledger = load_ledger(a.ledger) + findings, code = check(transcript, ch, ledger) except (OSError, ValueError, json.JSONDecodeError) as e: print(json.dumps({"error": str(e)}), file=sys.stderr) return 2 - findings, code = check(transcript, ch, ledger) print(json.dumps({"charter_version": ch.get("charter_version"), "messages": len(transcript), "findings": findings, diff --git a/dmcheck/core.py b/dmcheck/core.py index eb3915b..2a4e0b2 100644 --- a/dmcheck/core.py +++ b/dmcheck/core.py @@ -160,8 +160,8 @@ def check(transcript, charter, ledger=None, closed=True, now=None): enabled = set(ch.get("rules_enabled", list(RULES))) gm_idx = [r["i"] for r in T if _is_gm(r, ch)] if not ch["gm"]: - return [{"error": "charter names no GM author — cannot referee; " - "set charter.gm or pass --gm"}], 2 + raise ValueError("charter names no GM author — cannot referee; " + "set charter.gm or pass --gm") # R1 unanswered-player: a non-GM, non-dice message containing a question, # with NO GM message in the next `answer_within_messages` messages. diff --git a/dmcheck/watch.py b/dmcheck/watch.py index e8a092a..bb6666d 100644 --- a/dmcheck/watch.py +++ b/dmcheck/watch.py @@ -112,6 +112,10 @@ def watch_main(a): ch["gm"] = a.gm if a.dice_bot: ch["dice_authors"] = a.dice_bot + if not ch.get("gm"): + print(json.dumps({"error": "charter names no GM author — cannot referee; " + "set charter.gm or pass --gm"}), file=sys.stderr) + return 2 ledger = load_ledger(a.ledger) w = Watcher(ch, ledger, notify_cmd=a.notify_cmd, craft=getattr(a, "craft", False), scene=getattr(a, "scene", "SOCIAL"), diff --git a/tests/test_core.py b/tests/test_core.py index 9b94780..6363ecc 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -58,11 +58,40 @@ def test_exit_code(self): class TestGuards(unittest.TestCase): def test_no_gm_declared_is_unusable(self): + """check() raises — it never returns a finding without a 'rule' key.""" ch = load_charter(CH) ch["gm"] = [] - findings, code = check(load_transcript(os.path.join(FIX, "clean-session.jsonl")), ch) - self.assertEqual(code, 2) - self.assertIn("error", findings[0]) + with self.assertRaises(ValueError): + check(load_transcript(os.path.join(FIX, "clean-session.jsonl")), ch) + + def test_cli_no_gm_is_exit_2_not_traceback(self): + """The 0.5.5 regression class: `dmcheck run ` with the + packaged default charter (no GM) must exit 2 with a JSON error on + stderr — never a KeyError traceback from the counts summary.""" + import subprocess + r = subprocess.run( + [sys.executable, "-c", + "import sys; sys.path.insert(0, %r); from dmcheck.cli import main; " + "sys.argv = ['dmcheck', 'run', %r]; sys.exit(main())" + % (os.path.join(os.path.dirname(__file__), ".."), + os.path.join(FIX, "clean-session.jsonl"))], + capture_output=True, text=True) + self.assertEqual(r.returncode, 2, r.stderr) + self.assertNotIn("Traceback", r.stderr) + err = json.loads(r.stderr.strip().splitlines()[-1]) + self.assertIn("no GM author", err["error"]) + + def test_watch_no_gm_is_exit_2(self): + import subprocess + r = subprocess.run( + [sys.executable, "-c", + "import sys; sys.path.insert(0, %r); from dmcheck.cli import main; " + "sys.argv = ['dmcheck', 'watch', %r]; sys.exit(main())" + % (os.path.join(os.path.dirname(__file__), ".."), + os.path.join(FIX, "clean-session.jsonl"))], + capture_output=True, text=True) + self.assertEqual(r.returncode, 2, r.stderr) + self.assertNotIn("Traceback", r.stderr) if __name__ == "__main__":