Repository navigation
Add Claude adapter overlay and MCP directory review fixes - #18
Conversation
Anthropic's MCP Directory review asked that Claude not create or delete Sender Profiles and not send in bulk. The server now enforces that for hosted Claude connections; the Claude adapter mirrors it through a source-controlled overlay (adapter-sources/claude/overlay.yaml) that withholds the provisioning tools and adjusts provisioning, compliance, and messaging guidance. The canonical package, repository root, and Codex adapter keep all 27 tools. Generation fails when an overlay edit no longer matches canonical text, and validation checks the Claude skills keep confirmation wording for every retained consequential tool. Also: neutral feedback guidance, US toll-free verification caveat, correct revocation path (Dashboard > API Keys), and version 0.3.0.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟡
scripts/validate.py— Maintainers can ship Claude guidance that tells Claude to callsender_profiles.createor.delete, and validation will still pass. The withheld-tool gate scans onlyclaude-plugins/sent/skills/**/*.md. It also accepts any line that contains "do not call" anywhere. Fix: scan every Markdown and YAML file the Claude adapter ships, including.claude/commands/*.mdandREADME.md, and require the withheld name to be the object of a do-not-call phrase, not just share a line with one.Why this was flagged
The scan at scripts/validate.py:478 globs only
CLAUDE_ADAPTER / "skills". The slash commands copied intoclaude-plugins/sent/.claude/commands(scripts/generate_adapters.py:112) are agent instructions too, and overlay edits can target them because paths are relative to claude-plugins/sent. Separately, the check at scripts/validate.py:480 is a line-level substring test: it only asks whether "do not call" appears anywhere on the line. claude-plugins/sent/skills/sent-compliance/SKILL.md:20 already holds a long multi-clause line with "do not callsender_profiles.create". Appending "...otherwise callsender_profiles.createwith the plan" to that line would also pass. The new gate test claude_withheld_tool_instruction exercises only a row without the phrase, so neither gap is covered. This guard is new in this PR and is what the PR description relies on to keep Claude guidance from instructing withheld calls.Verification: nit. The new gate in validate_claude_adapter (scripts/validate.py) scans only
claude-plugins/sent/skills, yet the overlay already editsREADME.md(overlay.yaml:85) and could target.claude/commands/*.md(copied at generate_adapters.py:112). The check is a plain substring test on the line. Today the gate passes correctly, so nothing breaks at merge.
| - 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; offer a report on the missing tool only after addressing it. | ||
| rationale: sent-messaging owns the task; report the missing tool only if the user then asks. |
There was a problem hiding this comment.
🟡 nit (optional): The last sent-feedback routing case is still labelled ambiguous, though the new no-solicitation policy makes it a no_trigger. Its own rationale now says to report only if the user then asks. The query never asks to report. A sibling cancel-gap query was flipped to no_trigger in this same change. Fix: set this case's expect to no_trigger so the eval matches the new policy, keeping one other ambiguous case, which validate_evals requires.
Why this was flagged
evals/sent-feedback.yaml:42 has the query about stopping tomorrow's scheduled send with no cancel tool, and it still expects ambiguous. The new canonical description at packages/sent/skills/sent-feedback/SKILL.md:3 says to trigger only when the user asks to report. The skill body adds 'Do not solicit reports'. The new rationale at line 44 says sent-feedback applies only if the user later asks, so the case is really a hard negative, like the cancel-gap case at lines 9-11 that was changed to no_trigger. scripts/run_model_routing_eval.py:132 counts any prediction other than ambiguous as a failure, and line 153 fails the whole run below 0.95 accuracy. A correct no_trigger prediction therefore counts as a misroute, and the case drops out of the hard-negative metric at line 137. The file still has an ambiguous case at lines 39-41, so the validate_evals check at scripts/validate.py:765 keeps passing after the fix.
Verification: evals/sent-feedback.yaml:42-44 still has expect: ambiguous, but packages/sent/skills/sent-feedback/SKILL.md:3 now only says to use the skill when "the user asks to report a bug, request a missing feature, or pass along a confusing result or praise". A model that correctly applies the new description would answer no_trigger. The eval counts that as a miss (run_model_routing_eval.py:132).
| target = PurePosixPath(str(relative)) | ||
| if target.is_absolute() or ".." in target.parts or not target.parts: | ||
| raise MetadataError(f"{CLAUDE_OVERLAY}: edit path {relative!r} must stay inside the Claude adapter") | ||
| if not isinstance(entries, list) or not entries: | ||
| raise MetadataError(f"{CLAUDE_OVERLAY}: {relative} needs a non-empty list of edits") | ||
| pairs = [] | ||
| for index, entry in enumerate(entries, 1): | ||
| if (not isinstance(entry, dict) or set(entry) != {"find", "replace"} | ||
| or not isinstance(entry["find"], str) or not entry["find"] | ||
| or not isinstance(entry["replace"], str)): | ||
| raise MetadataError(f"{CLAUDE_OVERLAY}: {relative}#{index} needs string find and replace") | ||
| pairs.append((entry["find"], entry["replace"])) | ||
| edits[str(target)] = tuple(pairs) |
There was a problem hiding this comment.
🟡 (optional) Maintainers can lose a whole block of Claude overlay edits with no error, e.g. the one-recipient messages.send rule. This happens when an edit path appears twice in overlay.yaml. yaml.safe_load (line 160) keeps only the last duplicate key. Paths are also normalized through PurePosixPath, so edits[str(target)] (line 192) lets ./skills/x silently replace skills/x. Fix: reject duplicate keys and paths that collide after normalization in load_claude_overlay. For example, use a SafeLoader subclass whose construct_mapping raises on a repeated key, plus a check that str(target) is not already in edits.
Why this was flagged
Trigger: a maintainer adds a second skills/sent-messaging/SKILL.md: block to adapter-sources/claude/overlay.yaml. Another trigger is spelling a path two ways, such as ./skills/.... PyYAML's SafeConstructor does not check for duplicate mapping keys, so yaml.safe_load at scripts/repository_metadata.py:160 keeps only the last value. Separately, line 180 normalizes the path and line 192 overwrites any earlier entry under the same key. The dropped edits are never applied. The overlay header (overlay.yaml:9-10) says drift fails generation "instead of drifting silently", but that guard is the per-find matches != 1 check in generate_adapters.py:71-73, and it only sees edits that survived loading. generate_adapters.py --check, validate_claude_adapter and test_claude_overlay_applies_each_edit_once all read the same reduced overlay, so they all pass. As a result the Claude adapter can ship without guidance the Directory review required, and every gate stays green.
Verification: nit. The trigger is a maintainer mistake in adapter-sources/claude/overlay.yaml: a duplicate path key, or one target written two ways (./skills/x and skills/x). The current overlay has neither. Its six keys (lines 17, 47, 65, 71, 79, 85) are all distinct, so this is a hazard for future edits, not a bug that affects today's output.
arbianst
left a comment
arbianst
left a comment
There was a problem hiding this comment.
Reviewed. Overlay matches the server policy for hosted Claude connections. The three automated nits are non-blocking and can follow later.
Anthropic's MCP Directory review asked that Claude not create or delete Sender Profiles and not send in bulk. The server now enforces that for hosted Claude connections; the Claude adapter mirrors it through a source-controlled overlay (adapter-sources/claude/overlay.yaml) that withholds the provisioning tools and adjusts provisioning, compliance, and messaging guidance. The canonical package, repository root, and Codex adapter keep all 27 tools.
Generation fails when an overlay edit no longer matches canonical text, and validation checks the Claude skills keep confirmation wording for every retained consequential tool.
Also: neutral feedback guidance, US toll-free verification caveat, correct revocation path (Dashboard > API Keys), and version 0.3.0.