fix(config): quoted YAML booleans silently coerced to true across 9 config loaders - #558
Open
Tet-9 wants to merge 2 commits into
Open
fix(config): quoted YAML booleans silently coerced to true across 9 config loaders#558Tet-9 wants to merge 2 commits into
Tet-9 wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (18)
🚧 Files skipped from review as they are similar to previous changes (17)
WalkthroughAdds shared YAML boolean coercion and applies it to feature configuration and receipt auto-approval settings. Regression tests cover quoted boolean strings, recognized spellings, whitespace handling, and fallback behavior. ChangesConfiguration boolean coercion
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
storage
Tet-9
force-pushed
the
fix/compile-two-phase-quoted-false-yaml
branch
from
July 25, 2026 09:43
47f8083 to
8b80900
Compare
Contributor
Author
|
@plind-junior review if you may |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/vouch/config_coerce.py`:
- Line 1: Lowercase the prose sentence starts in the affected docstrings:
`Shared` in `src/vouch/config_coerce.py` lines 1-1, `Parse` in lines 29-30, and
`The` and `Callers` in `src/vouch/proposals.py` lines 480-489; preserve the
remaining wording and behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0313d580-7924-4d1e-881d-07141d57e47d
📒 Files selected for processing (18)
src/vouch/admission.pysrc/vouch/capture.pysrc/vouch/compile.pysrc/vouch/config_coerce.pysrc/vouch/inbox.pysrc/vouch/proposals.pysrc/vouch/recall.pysrc/vouch/session_split.pysrc/vouch/volunteer_context.pytests/test_admission.pytests/test_capture.pytests/test_compile.pytests/test_config_coerce.pytests/test_inbox.pytests/test_recall.pytests/test_receipt_auto_approve.pytests/test_session_split.pytests/test_volunteer_context.py
Contributor
Author
|
Contributor
Author
|
@plind-junior , the remaining to do is all yours to take a look at |
…onfig loaders
bool(x) on an arbitrary config value is a trap: yaml.safe_load resolves an
unquoted true/false to a real bool, but a mistakenly-quoted "false" stays
a non-empty string, and bool("false") is True. This exact bug was
independently repeated in 9 places:
- compile.py: two_phase
- session_split.py, admission.py, inbox.py, recall.py, capture.py,
volunteer_context.py: enabled
- admission.py: reject_uncited_session_pages (found alongside the enabled
fix, same function)
- proposals.py: auto_approve_on_receipt, read via _review_config() at 4
call sites (2 of which used the bug directly with no bool() at all --
a raw truthy check on the config dict value)
The last one is the one that matters most: auto_approve_on_receipt gates
whether a claim can be durably approved without a human reviewer. A
mistakenly-quoted "false" in config.yaml was silently read as enabled by
every caller, not just the one this PR happened to start from.
- New shared src/vouch/config_coerce.py: coerce_bool(value, default),
fail-soft (bad/unrecognized values degrade to the caller's default,
same posture compile.py's own _coerce() already documents for its
numeric fields) -- as opposed to install_adapter.py's install.yaml
flags, which correctly raise hard, appropriate for a one-time CLI
install but not for config read on every session/render.
- _review_config() (proposals.py) now normalizes auto_approve_on_receipt
once at the read boundary -- the single source of truth every caller
already funnels through -- rather than patching each read site
individually.
- 8 config loaders switched from bool(raw.get(...)) to coerce_bool(...).
- New tests/test_config_coerce.py for the coercer itself, plus a quoted-
false regression test added to every affected module's existing test
file, plus a quoted-true test in test_receipt_auto_approve.py proving
the fix doesn't break the legitimate quoted case.
Not included: openclaw/types.py's CompactParams.force field uses the same
bool(raw.get(...)) shape but parses a JSON wire payload from an external
host (OpenClaw), not YAML config -- JSON has native booleans, so the
quoted-string trap doesn't apply the same way, and it's a different root
cause than the rest of this sweep.
CodeRabbit review comment: src/vouch/** comments and review notes must be lowercase per path instructions. Fixes 3 capitalized sentence-starts introduced in the previous commit: - config_coerce.py:1 (module docstring) - config_coerce.py: coerce_bool() docstring - proposals.py: _review_config() docstring, two sentence-starts
Tet-9
force-pushed
the
fix/compile-two-phase-quoted-false-yaml
branch
from
July 27, 2026 11:48
ef210b4 to
52358dc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
what
bool(x)on an arbitrary config value is a trap:yaml.safe_loadresolves an unquotedtrue/falseto a real Python bool, but a mistakenly-quoted"false"in config.yaml stays a non-empty string — andbool("false")isTrue. This exact bug was independently repeated across the codebase in 9 places:compile.py:two_phasesession_split.py,admission.py,inbox.py,recall.py,capture.py,volunteer_context.py:enabledadmission.py:reject_uncited_session_pages(found alongside theenabledfix, same function)proposals.py:auto_approve_on_receipt, read via_review_config()at 4 call sites — 2 of which didn't even wrap it inbool(), just used the raw config value in a truthyifThat last one is the one that actually matters:
auto_approve_on_receiptgates whether a claim can be durably approved with no human reviewer. A mistakenly-quoted"false"in config.yaml was silently read as enabled by every caller, not just the site this PR happened to start from.why
Rather than patch each of the 9 read sites individually with a repeated inline fix, this adds one shared, tested primitive (
coerce_bool) and — for the security-relevant case — normalizes at the single choke point (_review_config()) all 4 callers already funnel through, so the fix can't drift out of sync the way 9 independentbool()calls already had.coerce_bool()is deliberately fail-soft (bad/unrecognized value degrades to the caller's default), matching the posturecompile.py's own_coerce()already documents for its numeric fields. This is a different, and correct, choice frominstall_adapter.py'sinstall.yamlflags, which raise hard — appropriate for a one-time CLI install, wrong for config read on every session/render.Not included:
openclaw/types.py'sCompactParams.forceuses the samebool(raw.get(...))shape, but it parses a JSON wire payload from an external host (OpenClaw), not YAML config. JSON has native booleans, so the quoted-string trap doesn't apply the same way — different root cause, left out of this sweep.invariants held
proposals.py's security-relevant field, which now has one authoritative parse instead of 4 independent (and inconsistent) ones."true"still works correctly (tested explicitly, not just the negative case).tests
tests/test_config_coerce.py: unit tests for the coercer itself (recognized spellings, case-insensitivity, whitespace, fallback behavior)."false"-does-not-enable regression test added to every affected module's existing test file (test_compile.py,test_session_split.py,test_admission.py,test_inbox.py,test_recall.py,test_capture.py,test_volunteer_context.py).test_receipt_auto_approve.py: two regression tests through the real approval path (approve()+auto_approve_receipts()) proving self-approval stays blocked on a quoted-"false"gate, plus one proving quoted-"true"still works.test_cli.py/test_delete.py/test_hot_memory.py, unrelated — verified no reference to any file this PR touches).Summary by CodeRabbit
Bug Fixes
"false"/"true"are now interpreted deterministically, preventing unintended feature enablement.Tests