From 2831bf850f543af0fd06cf6a4cf229e371830af6 Mon Sep 17 00:00:00 2001 From: Arbian Shkodra Date: Mon, 19 Oct 2026 16:53:35 +0200 Subject: [PATCH 1/2] Close the review gaps in the Claude adapter gates The withheld-tool gate scanned only skills/ and accepted any line that contained "do not call" somewhere, so an instruction to call the tool in a later clause, or in README or a slash command, passed. It now reads every Markdown and YAML file the adapter ships and checks each mention: the sentence must prohibit, and the mention must not be the direct object of an un-negated call verb. load_claude_overlay rejects a duplicate YAML key (PyYAML keeps the last one and drops the rest of the block silently) and two spellings of one edit path that normalise onto each other. The sent-feedback eval case that asks for help with a scheduled send is a hard negative under the no-solicitation policy, not ambiguous. Co-Authored-By: Claude Fable 5.1 --- evals/sent-feedback.yaml | 4 ++-- scripts/repository_metadata.py | 21 +++++++++++++++- scripts/test_validation_gates.py | 41 ++++++++++++++++++++++++++++++++ scripts/validate.py | 40 +++++++++++++++++++++++++++---- 4 files changed, 98 insertions(+), 8 deletions(-) diff --git a/evals/sent-feedback.yaml b/evals/sent-feedback.yaml index b46fba2..c2b8cb8 100644 --- a/evals/sent-feedback.yaml +++ b/evals/sent-feedback.yaml @@ -40,5 +40,5 @@ cases: expect: ambiguous rationale: Use sent-messaging for investigation and sent-feedback for the authorized report. - query: I need tomorrow's scheduled send stopped, but there's no cancel tool and my last two workarounds failed. What can I do? - expect: ambiguous - rationale: sent-messaging owns the task; report the missing tool only if the user then asks. + expect: no_trigger + rationale: sent-messaging owns the task; the user asked for help, not for a report, and feedback is never solicited. diff --git a/scripts/repository_metadata.py b/scripts/repository_metadata.py index 3f418f6..f54ac62 100644 --- a/scripts/repository_metadata.py +++ b/scripts/repository_metadata.py @@ -155,9 +155,24 @@ def load_claude_overlay(root: Path, metadata: RepositoryMetadata) -> ClaudeOverl """Load the Claude-only tool withholding and text edits applied to claude-plugins/sent.""" import yaml + class UniqueKeyLoader(yaml.SafeLoader): + """PyYAML keeps the last of two equal keys; here that would drop a whole block of edits unnoticed.""" + + def construct_mapping(self, node, deep=False): # type: ignore[override] + seen: set[Any] = set() + for key_node, _ in node.value: + key = self.construct_object(key_node, deep=deep) + if key in seen: + raise yaml.constructor.ConstructorError( + "while constructing a mapping", node.start_mark, + f"found duplicate key {key!r}", key_node.start_mark, + ) + seen.add(key) + return super().construct_mapping(node, deep=deep) + path = root / CLAUDE_OVERLAY try: - raw = yaml.safe_load(path.read_text(encoding="utf-8")) + raw = yaml.load(path.read_text(encoding="utf-8"), Loader=UniqueKeyLoader) except (OSError, yaml.YAMLError) as exc: raise MetadataError(f"could not read {CLAUDE_OVERLAY}: {exc}") from exc if not isinstance(raw, dict) or set(raw) != {"withheld_tools", "edits"}: @@ -189,5 +204,9 @@ def load_claude_overlay(root: Path, metadata: RepositoryMetadata) -> ClaudeOverl or not isinstance(entry["replace"], str)): raise MetadataError(f"{CLAUDE_OVERLAY}: {relative}#{index} needs string find and replace") pairs.append((entry["find"], entry["replace"])) + # Two spellings of one path (`./skills/x` and `skills/x`) normalise to the same key, and the + # second would silently replace the first. + if str(target) in edits: + raise MetadataError(f"{CLAUDE_OVERLAY}: edit path {relative!r} collides with an earlier entry for {target}") edits[str(target)] = tuple(pairs) return ClaudeOverlay(withheld_tools=dict(withheld), edits=edits) diff --git a/scripts/test_validation_gates.py b/scripts/test_validation_gates.py index 1e8ed12..b2fdeb5 100644 --- a/scripts/test_validation_gates.py +++ b/scripts/test_validation_gates.py @@ -307,11 +307,52 @@ def change(overlay: dict) -> None: edit_claude_overlay(root, change) +def claude_withheld_tool_after_prohibition(root: Path) -> None: + """The line already says "do not call"; a later clause instructs the call anyway.""" + def change(overlay: dict) -> None: + overlay["edits"]["skills/sent-compliance/SKILL.md"][0]["replace"] += ( + " Otherwise call `sender_profiles.create` with the plan.") + edit_claude_overlay(root, change) + + +def claude_withheld_tool_in_readme(root: Path) -> None: + """README.md ships with the adapter but sat outside the old skills-only scan.""" + def change(overlay: dict) -> None: + overlay["edits"]["README.md"][0]["replace"] += " Call `sender_profiles.delete` to remove one." + edit_claude_overlay(root, change) + + +def claude_withheld_tool_in_command(root: Path) -> None: + """Slash commands ship with the adapter and are agent instructions too; the gate reads the shipped file.""" + path = root / "claude-plugins" / "sent" / ".claude" / "commands" / "sent.md" + with path.open("a", encoding="utf-8") as handle: + handle.write("\nThen call `sender_profiles.create` with the plan.\n") + + +def duplicate_claude_overlay_path(root: Path) -> None: + """PyYAML would keep only the second block and drop the first without a word.""" + path = root / "adapter-sources/claude/overlay.yaml" + with path.open("a", encoding="utf-8") as handle: + handle.write("\n skills/sent/SKILL.md:\n - find: anything\n replace: anything else\n") + + +def colliding_claude_overlay_path(root: Path) -> None: + """A second spelling of an existing path normalises onto it and would replace its edits.""" + path = root / "adapter-sources/claude/overlay.yaml" + with path.open("a", encoding="utf-8") as handle: + handle.write("\n ./skills/sent/SKILL.md:\n - find: anything\n replace: anything else\n") + + CASES: tuple[tuple[str, Mutation, str], ...] = ( ("stale Claude overlay edit", stale_claude_overlay_find, "matched 0 times"), ("unknown Claude withheld tool", unknown_claude_withheld_tool, "is not in the canonical public surface"), ("Claude retained-tool confirmation", removed_claude_confirmation, "Claude adapter sent-profile-provisioning"), ("Claude withheld-tool instruction", claude_withheld_tool_instruction, "outside a do-not-call instruction"), + ("Claude withheld-tool after a prohibition", claude_withheld_tool_after_prohibition, "outside a do-not-call instruction"), + ("Claude withheld-tool in README", claude_withheld_tool_in_readme, "outside a do-not-call instruction"), + ("Claude withheld-tool in a command", claude_withheld_tool_in_command, "outside a do-not-call instruction"), + ("duplicate Claude overlay path", duplicate_claude_overlay_path, "found duplicate key"), + ("colliding Claude overlay path", colliding_claude_overlay_path, "collides with an earlier entry"), ("ambient credential reads", ambient_credential_read, "public gate rejected ambient credential read"), ("Claude privacy policy listing", missing_claude_privacy_url, "Claude privacyPolicyUrl must match"), ("independent mutation confirmation policy", disabled_profile_confirmation, "independent safety policy"), diff --git a/scripts/validate.py b/scripts/validate.py index 13f8a84..f618c70 100644 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -687,12 +687,42 @@ def validate_claude_adapter(validation: Validation) -> None: and isinstance(description, str) and 1 <= len(description) <= 1024, f"{skill_file.relative_to(ROOT)}: Claude overlay broke skill frontmatter", ) - for path in sorted((CLAUDE_ADAPTER / "skills").rglob("*.md")): + # Every Markdown and YAML file the adapter ships is agent-facing text: skills and their + # references, the slash commands copied from adapter-sources, the README, and the agents/*.yaml + # metadata. The overlay can target any of them. + for path in sorted(p for p in CLAUDE_ADAPTER.rglob("*") if p.is_file() and p.suffix in {".md", ".yaml", ".yml"}): for number, line in enumerate(path.read_text(encoding="utf-8").splitlines(), 1): - if any(tool in line for tool in overlay.withheld_tools) and "do not call" not in line.lower(): - validation.errors.append( - f"{path.relative_to(ROOT)}:{number}: Claude skill names a withheld tool outside a do-not-call instruction" - ) + for tool in sorted(overlay.withheld_tools): + if not withheld_tool_mentions_are_prohibitions(line, tool): + validation.errors.append( + f"{path.relative_to(ROOT)}:{number}: Claude adapter names withheld tool {tool} outside a do-not-call instruction" + ) + + +WITHHELD_SENTENCE_BOUNDARY = re.compile(r"(?<=[.;!?])\s+") +WITHHELD_PROHIBITION = re.compile(r"\b(?:do not|don't|never|cannot|can't|not)\b", re.IGNORECASE) +WITHHELD_CALL_VERB = re.compile(r"\b(?:call|calls|calling|run|runs|running|invoke|invokes|use|uses|execute|executes)\s*`?\s*$", re.IGNORECASE) +WITHHELD_NEGATED_CALL = re.compile( + r"\b(?:do not|don't|never|cannot|can't|not)\s+(?:\w+\s+)?(?:call|calls|calling|run|runs|running|invoke|invokes|use|uses|execute|executes)\s*`?\s*$", + re.IGNORECASE, +) + + +def withheld_tool_mentions_are_prohibitions(line: str, tool: str) -> bool: + """True when every mention of ``tool`` on ``line`` is governed by a do-not-call instruction. + + Sharing a line with the phrase is not enough: "do not call `x`; otherwise call `x` with the plan" + instructs the call in its second clause. Each mention must sit in a sentence that prohibits + something, and must not be the direct object of an un-negated call verb. + """ + for sentence in WITHHELD_SENTENCE_BOUNDARY.split(line): + for match in re.finditer(re.escape(tool), sentence): + if not WITHHELD_PROHIBITION.search(sentence): + return False + preceding = " ".join(sentence[:match.start()].split()[-4:]) + if WITHHELD_CALL_VERB.search(preceding) and not WITHHELD_NEGATED_CALL.search(preceding): + return False + return True def validate_documentation(validation: Validation) -> None: From 27570f817230e2ae5d5cc7f1425134887207a5bd Mon Sep 17 00:00:00 2001 From: Arbian Shkodra Date: Mon, 19 Oct 2026 17:02:55 +0200 Subject: [PATCH 2/2] Require an explicit do-not-call phrase per mention and tolerate YAML merge keys Review on #19: a bare "not" or "never" anywhere in the sentence counted as a prohibition, so "it is not optional: create one with `x`" and "Never skip `x`" passed, and one filler word was allowed between the negation and the verb, so "Do not skip calling `x`" passed too. The sentence must now contain do-not/don't/never directly on a call verb, and the per-mention object check applies on top. The three real mentions pass. The duplicate-key loader now skips merge-key and non-scalar key nodes, which the parent flattens or reports, instead of failing to construct them. Co-Authored-By: Claude Fable 5.1 --- scripts/repository_metadata.py | 4 ++++ scripts/test_validation_gates.py | 17 +++++++++++++++++ scripts/validate.py | 18 +++++++++--------- 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/scripts/repository_metadata.py b/scripts/repository_metadata.py index f54ac62..f4ae32d 100644 --- a/scripts/repository_metadata.py +++ b/scripts/repository_metadata.py @@ -161,6 +161,10 @@ class UniqueKeyLoader(yaml.SafeLoader): def construct_mapping(self, node, deep=False): # type: ignore[override] seen: set[Any] = set() for key_node, _ in node.value: + # Merge keys (`<<: *anchor`) are flattened by the parent and have no constructor here; + # a non-scalar key is malformed and is left for the parent to report. + if key_node.tag == "tag:yaml.org,2002:merge" or not isinstance(key_node, yaml.ScalarNode): + continue key = self.construct_object(key_node, deep=deep) if key in seen: raise yaml.constructor.ConstructorError( diff --git a/scripts/test_validation_gates.py b/scripts/test_validation_gates.py index b2fdeb5..6b91331 100644 --- a/scripts/test_validation_gates.py +++ b/scripts/test_validation_gates.py @@ -322,6 +322,21 @@ def change(overlay: dict) -> None: edit_claude_overlay(root, change) +def claude_withheld_tool_bare_negation(root: Path) -> None: + """A bare "not" or "never" in the sentence is not a do-not-call instruction.""" + def change(overlay: dict) -> None: + overlay["edits"]["README.md"][0]["replace"] += ( + " If no profile exists, it is not optional: create one with `sender_profiles.create`.") + edit_claude_overlay(root, change) + + +def claude_withheld_tool_negated_filler(root: Path) -> None: + """"Do not skip calling" instructs the call; the negation must sit on the verb itself.""" + def change(overlay: dict) -> None: + overlay["edits"]["README.md"][0]["replace"] += " Do not skip calling `sender_profiles.create`." + edit_claude_overlay(root, change) + + def claude_withheld_tool_in_command(root: Path) -> None: """Slash commands ship with the adapter and are agent instructions too; the gate reads the shipped file.""" path = root / "claude-plugins" / "sent" / ".claude" / "commands" / "sent.md" @@ -350,6 +365,8 @@ def colliding_claude_overlay_path(root: Path) -> None: ("Claude withheld-tool instruction", claude_withheld_tool_instruction, "outside a do-not-call instruction"), ("Claude withheld-tool after a prohibition", claude_withheld_tool_after_prohibition, "outside a do-not-call instruction"), ("Claude withheld-tool in README", claude_withheld_tool_in_readme, "outside a do-not-call instruction"), + ("Claude withheld-tool under a bare negation", claude_withheld_tool_bare_negation, "outside a do-not-call instruction"), + ("Claude withheld-tool under a negated filler verb", claude_withheld_tool_negated_filler, "outside a do-not-call instruction"), ("Claude withheld-tool in a command", claude_withheld_tool_in_command, "outside a do-not-call instruction"), ("duplicate Claude overlay path", duplicate_claude_overlay_path, "found duplicate key"), ("colliding Claude overlay path", colliding_claude_overlay_path, "collides with an earlier entry"), diff --git a/scripts/validate.py b/scripts/validate.py index f618c70..5dcf85f 100644 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -700,24 +700,24 @@ def validate_claude_adapter(validation: Validation) -> None: WITHHELD_SENTENCE_BOUNDARY = re.compile(r"(?<=[.;!?])\s+") -WITHHELD_PROHIBITION = re.compile(r"\b(?:do not|don't|never|cannot|can't|not)\b", re.IGNORECASE) -WITHHELD_CALL_VERB = re.compile(r"\b(?:call|calls|calling|run|runs|running|invoke|invokes|use|uses|execute|executes)\s*`?\s*$", re.IGNORECASE) -WITHHELD_NEGATED_CALL = re.compile( - r"\b(?:do not|don't|never|cannot|can't|not)\s+(?:\w+\s+)?(?:call|calls|calling|run|runs|running|invoke|invokes|use|uses|execute|executes)\s*`?\s*$", - re.IGNORECASE, -) +WITHHELD_CALL_VERBS = r"(?:call|calls|calling|run|runs|running|invoke|invokes|use|uses|execute|executes)" +# The negation must sit directly on the verb: "do not skip calling `x`" is an instruction to call. +WITHHELD_DO_NOT_CALL = re.compile(rf"\b(?:do not|don't|never)\s+{WITHHELD_CALL_VERBS}\b", re.IGNORECASE) +WITHHELD_CALL_VERB = re.compile(rf"\b{WITHHELD_CALL_VERBS}\s*`?\s*$", re.IGNORECASE) +WITHHELD_NEGATED_CALL = re.compile(rf"\b(?:do not|don't|never)\s+{WITHHELD_CALL_VERBS}\s*`?\s*$", re.IGNORECASE) def withheld_tool_mentions_are_prohibitions(line: str, tool: str) -> bool: """True when every mention of ``tool`` on ``line`` is governed by a do-not-call instruction. Sharing a line with the phrase is not enough: "do not call `x`; otherwise call `x` with the plan" - instructs the call in its second clause. Each mention must sit in a sentence that prohibits - something, and must not be the direct object of an un-negated call verb. + instructs the call in its second clause. Each mention must sit in a sentence that contains an + explicit do-not-call phrase (a bare "not" or "never" does not count: "it is not optional, create + one with `x`"), and must not itself be the direct object of an un-negated call verb. """ for sentence in WITHHELD_SENTENCE_BOUNDARY.split(line): for match in re.finditer(re.escape(tool), sentence): - if not WITHHELD_PROHIBITION.search(sentence): + if not WITHHELD_DO_NOT_CALL.search(sentence): return False preceding = " ".join(sentence[:match.start()].split()[-4:]) if WITHHELD_CALL_VERB.search(preceding) and not WITHHELD_NEGATED_CALL.search(preceding):