Skip to content

Validate CLAUDE_MODEL env var against VALID_MODELS at startup#137

Merged
dcellison merged 1 commit intomainfrom
fix/validate-claude-model-env
Mar 23, 2026
Merged

Validate CLAUDE_MODEL env var against VALID_MODELS at startup#137
dcellison merged 1 commit intomainfrom
fix/validate-claude-model-env

Conversation

@dcellison
Copy link
Copy Markdown
Owner

Summary

  • Validate CLAUDE_MODEL env var against VALID_MODELS at startup in load_config()
  • A typo like CLAUDE_MODEL=sonet now raises SystemExit with a clear message instead of silently passing through to a confusing runtime failure
  • Matches the validation already in place for workspace config models (config.py:431-440)
  • Uses SystemExit consistent with other fatal config errors (missing token, bad user IDs, etc.)

Test plan

  • test_invalid_claude_model - typo raises SystemExit matching "CLAUDE_MODEL"
  • test_claude_model_from_env - valid model ("opus") is read correctly
  • All 1049 tests pass, make check clean

Fixes #130

A typo like CLAUDE_MODEL=sonet silently passed through startup and
only failed at runtime when the Claude CLI rejected the unknown model.
Now raises SystemExit with a clear message, matching the validation
already in place for workspace config models.

Fixes #130
@dcellison
Copy link
Copy Markdown
Owner Author

Review by Kai

PR looks clean. The change is focused, correct, and consistent with existing patterns. No bugs, security issues, or missing error handling found.

One minor suggestion:

suggestionconfig.py, new validation block

CLAUDE_MODEL="" (explicitly set to empty string) will correctly fail validation and raise SystemExit, but the error message will read CLAUDE_MODEL '' is not valid... which is slightly confusing. A user setting an empty env var might not understand why. Not a bug — the behavior is safe and the message still points at the right place — but a quick guard like:

if not claude_model:
    raise SystemExit("CLAUDE_MODEL must not be empty")

before the VALID_MODELS check would make it marginally clearer. This is genuinely optional.

Everything else is solid:

  • Single read of the env var, reused — avoids any theoretical TOCTOU.
  • sorted(VALID_MODELS) in the error message gives deterministic output.
  • Test placement is correct: invalid model in the required-fields class, valid model read in optional.
  • match="CLAUDE_MODEL" in the test pins the error message to the right variable name without over-constraining the full string.

@dcellison dcellison merged commit ba2e3a6 into main Mar 23, 2026
1 check passed
@dcellison dcellison deleted the fix/validate-claude-model-env branch March 23, 2026 22:07
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.

CLAUDE_MODEL env var not validated against VALID_MODELS at startup

2 participants