From b922311a79a2b014fb04bc1ab953e65352dc0ff6 Mon Sep 17 00:00:00 2001 From: NIKHIL PENTAPALLI Date: Sat, 25 Jul 2026 18:33:16 -0700 Subject: [PATCH] fix(inbox): match approval keywords as whole words, not substrings resolve_from_reply decided allow/deny with 'in' checks on the whole message, so "disallow" resolved as allow (checked first, and it contains "allow") and replies containing words like "note" or "not" resolved as deny instead of being recorded as free-text answers. Since this gates parked unattended actions, a false allow is the worst-case direction. Keyword intent now requires word boundaries (with the common -d forms added); emoji checks stay as substring matches. Anything that matches neither list falls through to the existing free-text path, which records the reply verbatim instead of acting on it. --- coworker/inbox_routing.py | 7 +++++-- tests/test_inbox_routing.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/coworker/inbox_routing.py b/coworker/inbox_routing.py index ef115e95..f3efc9ed 100644 --- a/coworker/inbox_routing.py +++ b/coworker/inbox_routing.py @@ -23,6 +23,9 @@ DEFAULT_INBOX = "default" # to OpenWorker (2026-07-22); the legacy [ocw:…] spelling stays parseable so replies to # messages sent before the rename still resolve. _ID_TOKEN = re.compile(r"\[o(?:c)?w:([0-9a-f]{6,})\]") +# Whole words only — substring matching resolved "disallow" as allow and "note" as deny. +_ALLOW_WORDS = re.compile(r"\b(?:approve|approved|allow|allowed|yes)\b") +_DENY_WORDS = re.compile(r"\b(?:deny|denied|reject|rejected|no)\b") @dataclass @@ -130,9 +133,9 @@ def resolve_from_reply( return None item_id = m.group(1) lowered = reply.lower() - if any(w in lowered for w in ("approve", "allow", "yes", "👍", "✅")): + if _ALLOW_WORDS.search(lowered) or "👍" in reply or "✅" in reply: resolution = "allow" - elif any(w in lowered for w in ("deny", "reject", "no", "👎", "❌")): + elif _DENY_WORDS.search(lowered) or "👎" in reply or "❌" in reply: resolution = "deny" else: resolution = _ID_TOKEN.sub("", reply).strip() # free-text answer to a question diff --git a/tests/test_inbox_routing.py b/tests/test_inbox_routing.py index fb3377ed..be216940 100644 --- a/tests/test_inbox_routing.py +++ b/tests/test_inbox_routing.py @@ -88,3 +88,37 @@ def test_inbound_legacy_ocw_token_still_resolves(tmp_path): item = store.add_approval("s1", "Deploy?", inbox="ops") assert resolve_from_reply(f"deny [ocw:{item.id}]", store.resolve) is True assert store.get(item.id).resolution == "deny" + + +def test_disallow_is_not_parsed_as_allow(tmp_path): + store = InboxStore(tmp_path / "inbox.json") + item = store.add_approval("s1", "Deploy?", inbox="ops") + assert resolve_from_reply(f"disallow [ow:{item.id}]", store.resolve) is True + assert store.get(item.id).resolution != "allow" + + +def test_words_containing_no_are_not_parsed_as_deny(tmp_path): + store = InboxStore(tmp_path / "inbox.json") + q = store.add_question("s1", "Which region?") + assert resolve_from_reply(f"north-east node [ow:{q.id}]", store.resolve) is True + assert store.get(q.id).resolution == "north-east node" + + +def test_denied_and_approved_word_forms(tmp_path): + store = InboxStore(tmp_path / "inbox.json") + a = store.add_approval("s1", "Deploy?", inbox="ops") + b = store.add_approval("s1", "Restart?", inbox="ops") + resolve_from_reply(f"denied [ow:{a.id}]", store.resolve) + resolve_from_reply(f"approved [ow:{b.id}]", store.resolve) + assert store.get(a.id).resolution == "deny" + assert store.get(b.id).resolution == "allow" + + +def test_emoji_reactions_still_resolve(tmp_path): + store = InboxStore(tmp_path / "inbox.json") + a = store.add_approval("s1", "Deploy?", inbox="ops") + b = store.add_approval("s1", "Restart?", inbox="ops") + resolve_from_reply(f"👍 [ow:{a.id}]", store.resolve) + resolve_from_reply(f"❌ [ow:{b.id}]", store.resolve) + assert store.get(a.id).resolution == "allow" + assert store.get(b.id).resolution == "deny"