fix(babysit): refuse a non-boolean wake_on_green instead of coercing it

[bolichen97]

Fixes #7644.

Problem

The gh-pr watch probe read its wake_on_green switch as:

self.wake_on_green = bool(params.get("wake_on_green", True))

The cron message is JSON, so a caller can write {"wake_on_green": "false"} (a string). bool("false") is True, so an explicit disable silently inverts to ENABLED and the watch wakes the operator on all-green anyway. Any non-empty string ("no", "0", "off") has the same effect.

Fix

In PrWatchProbe.identity() (src/kiro_crew/builtin_skills/kirocrew-dev/babysit/scripts/pr_watch.py), replaced the coercion with explicit validation that mirrors the existing malformed-parameter discipline already used for repo, pr, and coalesce_secs:

raw_wake = params.get("wake_on_green", True)
if not isinstance(raw_wake, bool):
    raise ValueError("pr_watch wake_on_green must be true or false")
...
self.wake_on_green = raw_wake

A non-boolean now raises ValueError, which the wrapper converts into a terminal Done (the watch says so once and stops) rather than running forever with the opposite of the requested behaviour. Because bool is a subclass of int, the guard uses an explicit isinstance(..., bool) check. The absent-key default remains True.

Tests

Added to test/test_babysit_pr_watch.py alongside the existing malformed-parameter cases:

• test_string_wake_on_green_is_refused_not_coerced — parametrized over "false", "no", "0", "off"; each asserts terminal Done.
• test_string_wake_on_green_does_not_coerce_to_a_wake — on an all-green rollup, asserts Done fires instead of the review-ready Report a truthy coercion would have produced.
• test_real_boolean_true_wake_on_green_still_wakes, test_real_boolean_false_wake_on_green_stays_quiet, test_absent_wake_on_green_defaults_to_waking — confirm real booleans and the default still behave.

Verification

Ran the target module offline against the sandbox Python: 70 passed. Falsification check: with the source fix reverted (test change only), the 5 new string-rejection tests fail against the old bool(...) coercion; with the fix restored they pass.

The documented full-suite entry (make backend && make test) requires building the dev venv from PyPI, which is unavailable under the sandbox's repository-access-only network mode, so full-suite CI verification should run in the pipeline.

Scope note

Targets pr_watch.py on main as directed. The relocation into kiro_crew.probes.gh_pr (PR #7634) is not yet merged; when it lands, the byte-identical expression at probes/gh_pr.py:329 will need the same guard carried across, or this change rebased onto the relocated path.

Pattern harvest

Rule candidate: semgrep

Pattern: bool(params.get("flag", default)) over a value parsed from JSON. Every
non-empty string coerces to True, so "false", "no", "0" and "off" all invert
an explicit disable into enabled. Prefer an isinstance(x, bool) guard that refuses
the value over a coercion that silently means the opposite of what was sent.

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 31b7774eff162cc7bca7d83af5f0302bb5430f8e — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Real silent-inversion harm, fixed at the parse site with the same terminal-refusal discipline its sibling fields already use — proportionate and complete.

[DESIGN-REVIEWED] 31b7774

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 31b7774eff162cc7bca7d83af5f0302bb5430f8e — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 31b7774

Verdict parsed from the review's SHA-scoped output markers for commit 31b7774eff162cc7bca7d83af5f0302bb5430f8e.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 31b7774eff162cc7bca7d83af5f0302bb5430f8e: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 31b7774eff162cc7bca7d83af5f0302bb5430f8e — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks complete. The fix deletes a coercion rather than adding surface, matches the file's existing per-field terminal-validation discipline, the only real sibling (the byte-identical copy in unmerged PR #7634's relocation) is declared in the scope note, and the repo-wide bool(x.get(...)) hits sit on differently-composed inputs (typed SPA bodies, or internal callers passing literal booleans, as in deploy/pending.py whose 2 callers pass True or omit the key).

First-Principles-Verdict: PASS

A silently inverted explicit disable becomes a loud terminal stop; the change removes a coercion instead of adding surface, and every behavior shift is declared.

What this change ships

Intent: stop the PR watch from waking an operator who explicitly turned green-wakes off — a FIX (reported defect #7644).

1. A string wake_on_green ("false", "no", "0", "off") now stops the watch with a message instead of waking anyway — justified
2. A falsy non-boolean (0, null) that previously ran quietly now also terminates the watch — declared, matches the file's terminal-on-malformed discipline
3. Real true/false and the absent-key default keep their behavior — pinned by tests, not a change

Checks run: no shared strict-bool helper is importable from this standalone skill script (grep def .*require_bool|parse_bool|as_bool: 1 hit, app-local ops_mission_control/backend/routes.py:110); wake_on_green was the only unvalidated boolean field in this message (grep isinstance(.., bool) in the file: pr and coalesce_secs already guard it); repo-wide = bool(x.get( hits (~40) take inputs composed by typed frontend code or internal literals, not hand-written JSON, so they are not siblings of this cause. The one true sibling is the unmerged relocation copy, which the description declares. The spec's generic "invalid permanent configuration raises ValueError → Done" already covers the new behavior, so no doc drift.

[FIRST-PRINCIPLES-REVIEWED] 31b7774

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 31b7774eff162cc7bca7d83af5f0302bb5430f8e and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 31b7774

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 31b7774eff162cc7bca7d83af5f0302bb5430f8e: <one-sentence reason>

[NicholasRBowers]

Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: fix with clear root cause -- bool coercion of a JSON-string wake_on_green inverted an explicit disable; now refused as a terminal invalid config, with red-before regression tests.