From 874f7cacdb8adedeb0e3fcc224d117f4e414f2af Mon Sep 17 00:00:00 2001
From: Bryan Van Deusen <bryan.vandeusen@fabledsword.com>
Date: Thu, 27 Aug 2026 09:31:30 -0400
Subject: [PATCH] test(rules): the kwargs assertion learns about `clear`
 (#3096, milestone 312 step 2)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

test_update_rule_only_sends_non_default_fields pins that the MCP door
forwards only what the caller actually gave. `clear` is now always
forwarded — an empty tuple is "clear nothing", a value rather than an
absent argument — so the expected kwargs gained it. The property under
test is unchanged: everything left at its default still stays out.

Two tests added beside it while the shape is in view: naming a field for
clearing reaches the service as `clear`, and the check fields are
forwarded when given.

CI 4630 otherwise green — the integration lane ran all six of the new
real-Postgres cases (72 selected, was 66) and applied 0089 -> 0090.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 tests/test_mcp_tool_rulebooks.py | 41 +++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/tests/test_mcp_tool_rulebooks.py b/tests/test_mcp_tool_rulebooks.py
index 33fbd58..1c68ffb 100644
--- a/tests/test_mcp_tool_rulebooks.py
+++ b/tests/test_mcp_tool_rulebooks.py
@@ -115,7 +115,46 @@ async def test_update_rule_only_sends_non_default_fields():
         await update_rule(rule_id=1, statement="new statement")
     args, kwargs = mock.call_args
     assert args == (1, 7)
-    assert kwargs == {"statement": "new statement"}
+    # `clear` is always forwarded — an empty tuple is "clear nothing", which is
+    # a value, not an absent argument. Everything the caller left at its
+    # default stays out: that is the property this test pins.
+    assert kwargs == {"statement": "new statement", "clear": ()}
+
+
+@pytest.mark.asyncio
+async def test_update_rule_forwards_the_fields_named_for_clearing():
+    """Naming a field is the only way to empty it through this door.
+
+    "" means "leave unchanged" here, so a caller has no value that means
+    "remove it" — which is what makes an explicit list necessary and what
+    stops a partial update from wiping the fields it did not mention.
+    """
+    rule = fake_rule(id=100, title="r", statement="s", topic_id=10)
+    mock = AsyncMock(return_value=rule)
+    with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.update_rule", mock), _plain_detail():
+        from scribe.mcp.tools.rulebooks import update_rule
+        await update_rule(rule_id=1, clear_fields=["verify_with"])
+    _args, kwargs = mock.call_args
+    assert kwargs == {"clear": ["verify_with"]}
+
+
+@pytest.mark.asyncio
+async def test_update_rule_sends_the_check_fields_when_given():
+    rule = fake_rule(id=100, title="r", statement="s", topic_id=10)
+    mock = AsyncMock(return_value=rule)
+    with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.update_rule", mock), _plain_detail():
+        from scribe.mcp.tools.rulebooks import update_rule
+        await update_rule(
+            rule_id=1,
+            verify_with="cat CI-runner/renovate/config.js",
+            expires_when="approval is turned off",
+        )
+    _args, kwargs = mock.call_args
+    assert kwargs == {
+        "verify_with": "cat CI-runner/renovate/config.js",
+        "expires_when": "approval is turned off",
+        "clear": (),
+    }
 
 
 @pytest.mark.asyncio