Skip to content

fix(config): harden model validation and config type-checking (#128)#169

Merged
jasonssdev merged 2 commits into
mainfrom
fix/config-model-hardening
Jul 25, 2026
Merged

fix(config): harden model validation and config type-checking (#128)#169
jasonssdev merged 2 commits into
mainfrom
fix/config-model-hardening

Conversation

@jasonssdev

Copy link
Copy Markdown
Owner

Summary

Slice A of #128. Fixes the config-corruption cascade a real first-time user hit on published 0.1.1 by answering yes to the free-text model prompt in openkos initmodel: yes round-tripped through PyYAML as the boolean True, silently corrupting openkos.yaml, crashing openkos doctor, and making every LLM call fail with an opaque 400.

Three independent hardening defects, all closed at the config layer:

  1. validate_model rejects YAML 1.1 reserved wordsyes/no/true/false/on/off/null (case-insensitive), via an exact-token frozenset check applied before the character allowlist. Tags containing a reserved substring (yesmodel, on-prem) are still accepted.
  2. read_config type-checks model and embedding_model — a present non-str value raises ValueError with actionable messaging; null/absent still falls back to defaults, and unrelated bool fields (e.g. review: false) are untouched.
  3. doctor no longer crashes on a non-str model. The read_config guard fully subsumes the TypeError at model_tag_matches (verified by control-flow trace: a raising read_config leaves cfg = None, so the checks fall back to valid default tags). No production change to main.py/ollama.py — only a regression test.

Scope

Production change is a single file: src/openkos/config.py (+24). Tests: test_config.py (+98), test_doctor.py (+52).

Verification

  • uv run pytest: 2089 passed, exit 0
  • uv run ruff check . && uv run ruff format --check .: clean
  • uv run mypy .: clean
  • SDD verify: PASS — 4/4 requirements, 15/15 scenarios
  • Adversarial reliability review: 0 findings (reserved-word set confirmed to match PyYAML's default resolver exactly; defect feat(cli): add openkos init — create an OKF workspace #3 subsumption confirmed)

Strict TDD throughout (RED confirmed for the right reason before each GREEN, including a git stash to prove the doctor TypeError pre-fix).

Issue

Advances #128 (Slice A: config hardening). Does not close it — the interactive model picker (Slice B) remains as a follow-up.

Prevents the config-corruption cascade a first-time user hit by answering
`yes` to the free-text model prompt in `openkos init`.

- validate_model now rejects YAML 1.1 reserved boolean/null words
  (yes/no/true/false/on/off/null, case-insensitive) via an exact-token
  check applied before the character allowlist; tags that merely contain
  a reserved substring (yesmodel, on-prem) are still accepted.
- read_config now enforces that a present `model`/`embedding_model` value
  is a str, raising ValueError with actionable messaging on a non-str
  (e.g. a bool from a YAML round-trip); null/absent still falls back to
  defaults and unrelated bool fields are untouched.
- doctor no longer crashes with a TypeError on a non-str model: the
  read_config guard is fully subsumed at the config layer, so its
  config-valid check reports [FAIL] with remediation instead of a
  traceback. Regression tests cover both fields.

Advances #128 (Slice A: config hardening); the interactive model picker
remains as a follow-up slice.
@jasonssdev
jasonssdev merged commit 6b81811 into main Jul 25, 2026
6 checks passed
@jasonssdev
jasonssdev deleted the fix/config-model-hardening branch July 25, 2026 03:43
jasonssdev added a commit that referenced this pull request Jul 25, 2026
Merge the workspace-init and doctor-command delta specs into the main
specs tree and move the change folder to the archive. Closes the SDD
cycle for the config-hardening slice (implemented, verified PASS,
reviewed 0 findings, merged via #169).

Advances #128; the interactive model picker (Slice B, init-model-picker)
remains active.
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