Skip to content

Fix crash on default charter: no-GM is exit 2, never a traceback - #10

Closed
chaoz23 wants to merge 1 commit into
mainfrom
fix/no-gm-crash
Closed

chaoz23 wants to merge 1 commit into
mainfrom
fix/no-gm-crash

Conversation

@chaoz23

@chaoz23 chaoz23 commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Bug

dmcheck run session.jsonl (no --gm, packaged default charter) crashed with KeyError: 'rule' instead of the intended honest exit 2. The README's headline invocation shape was affected for any user whose charter names no GM.

Mechanism

core.check() returned [{"error": ...}], 2 — a pseudo-finding without the rule key that every consumer (cli counts summary, watch dedup key, R8 in core) assumes exists. Found while writing srdcheck-style SKILL.md worked examples for the family tune-up; no test covered the default-charter path, which is how it shipped (same class as the 0.5.x init --dm packaging bug — works in a prepared checkout/fixture, breaks for installed users).

Fix (class, not instance)

  • check() raises ValueError — a finding without rule can no longer exist in the contract
  • cli: verdict call moved into the existing exit-2 error lane (JSON on stderr, exit 2)
  • watch: validates GM up front, same error shape
  • mcp: already converts the raise into a clean JSON-RPC error (verified)

Tests

3 new regression tests (check-raises, cli-no-traceback via subprocess, watch-exit-2); suite 20/20 green; preflight clean.

🤖 Generated with Claude Code

'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 <noreply@anthropic.com>
@chaoz23

chaoz23 commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

Superseded: the #7–#9 validation-module rewrite fixes this class properly (structured invalid-envelope, exit 2, no traceback — verified on main: dmcheck run tests/fixtures/clean-session.jsonl now returns status=invalid, exit 2, with 'at least one GM author is required'). The regression-test idea lives on in that architecture; closing this instance fix.

@chaoz23 chaoz23 closed this Aug 9, 2026
@chaoz23
chaoz23 deleted the fix/no-gm-crash branch August 9, 2026 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant