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..f4ae32d 100644 --- a/scripts/repository_metadata.py +++ b/scripts/repository_metadata.py @@ -155,9 +155,28 @@ 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: + # 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( + "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 +208,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..6b91331 100644 --- a/scripts/test_validation_gates.py +++ b/scripts/test_validation_gates.py @@ -307,11 +307,69 @@ 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_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" + 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 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"), ("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..5dcf85f 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_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 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_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): + return False + return True def validate_documentation(validation: Validation) -> None: