Reject booleans for numeric config fields

[xttraa]

Summary

This PR makes numeric config parsing reject YAML booleans instead of silently treating them as numbers.

It adds shared parser helpers for integer and float fields, then uses them for numeric config values such as executor limits, request/tool timeouts, retry settings, compaction settings, MCP timeouts, terminal scrollback, and guardrail ask timeouts.

Why

YAML parses true and false as booleans. In Python, booleans are also integers, so direct casts behave like this:

int(False) == 0
int(True) == 1
float(True) == 1.0

That meant a typo like this could pass through parsing:

executor:
  max_iterations: false

Instead of failing, Omnigent treated it as max_iterations: 0. Similar mistakes could turn timeout: true into a 1 second timeout.

User impact

Invalid numeric config now fails early with a clear OmnigentError, for example:

executor.max_iterations must be an integer, got boolean False

Normal numeric values still work, including existing integer or float strings that already parsed successfully before.

Tests

• uv run --extra dev python -m pytest tests/spec/test_parser.py -q
• uv run --extra dev python -m pytest tests/spec -q
• uv run --extra dev ruff check omnigent/spec/parser.py tests/spec/test_parser.py
• git diff --check

[omnigent-ci]

Polly AI Review

Review: Reject booleans for numeric config fields

Blocking issues

None. The change is correct and surgical. The bool-rejection guard runs before int()/float(), so YAML booleans (true/false) now fail loud with a clear OmnigentError instead of silently coercing to 0/1. Verified that all genuinely-boolean fields are left untouched (retry.jitter, terminal allow_cwd_override/allow_sandbox_override/tmux_* still use bool(...)), so no previously-valid boolean config regresses. The two refactored helpers (_parse_guardrails_ask_timeout, _parse_policy_ask_timeout) preserve their prior OmnigentError-on-non-integer behavior and still apply the value <= 0 check afterward; the message wording for the non-int case is materially equivalent. Full tests/spec/test_parser.py suite passes (170 passed).

Security vulnerabilities

None. No deserialization, injection, auth, or boundary changes — this only tightens input validation, strictly narrowing what parses successfully.

Non-blocking notes

• retry.retryable_status_codes: null still falls through to iterating None (omnigent/spec/parser.py:481), raising a raw TypeError rather than an OmnigentError. This is pre-existing and out of scope, but if the goal is "all invalid numeric config fails with OmnigentError," it's the one remaining gap in this area.
• Float-like strings for int fields (e.g. max_iterations: "3.5") still raise ValueError → wrapped as OmnigentError, matching prior behavior. Float values fed to an int field truncate (int(3.5) == 3) as before — behavior preserved, not a regression.
• Test coverage is solid for bool rejection across the claimed fields. Worth adding a couple of positive regression tests — e.g. retry.jitter: false remaining valid, and a numeric string like request_timeout: "300" still parsing — to lock in that the guard didn't over-reject. Optional.

Summary

A clean, well-scoped fix that closes a real silent-coercion footgun (max_iterations: false → 0, timeout: true → 1). The shared helpers are applied consistently across the numeric config surface the description claims (LLM/tool timeouts, retry numerics, executor limits + context window, terminal scrollback, compaction, both HTTP and stdio MCP timeouts, guardrail and policy ask timeouts), boolean-only fields are correctly left alone, prior coercion semantics for strings/floats are preserved, and tests pass. No blocking or security concerns — ready for human merge. Consider the optional positive regression tests and the null retryable_status_codes edge as low-priority follow-ups.

---

_{Automated review by Polly · workflow run}

[SabhyaC26]

/merge

[github-actions]

🤖 /merge from @SabhyaC26, auto-merge enabled (squash, delete branch). ⌛ gate not green yet. Required checks not satisfied:

• E2E Tests (shard 0/4) (workflow ran but has not succeeded and the check is missing -- still pending or cancelled)
• E2E Tests (shard 1/4) (workflow ran but has not succeeded and the check is missing -- still pending or cancelled)
• E2E Tests (shard 2/4) (workflow ran but has not succeeded and the check is missing -- still pending or cancelled)
• E2E Tests (shard 3/4) (workflow ran but has not succeeded and the check is missing -- still pending or cancelled)
• E2E UI Tests (shard 0/3) (workflow ran but has not succeeded and the check is missing -- still pending or cancelled)
• E2E UI Tests (shard 1/3) (workflow ran but has not succeeded and the check is missing -- still pending or cancelled)
• E2E UI Tests (shard 2/3) (workflow ran but has not succeeded and the check is missing -- still pending or cancelled)
• Integration (claude-sdk) (workflow ran but has not succeeded and the check is missing -- still pending or cancelled)
• Integration (openai-agents) (workflow ran but has not succeeded and the check is missing -- still pending or cancelled)
• Integration (codex) (workflow ran but has not succeeded and the check is missing -- still pending or cancelled)
  The merge will fire once these turn green.