From d5f0c34cb4c0827bed0cb888ed3dc0988e078a18 Mon Sep 17 00:00:00 2001 From: Daniel Klose Date: Tue, 28 Apr 2026 15:53:09 +0200 Subject: [PATCH] feat(mcp): send orchestration hints via InitializeResult.instructions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MCP spec defines `serverInfo.instructions` as a free-text field servers return during the initialize handshake. Clients that honor it (Claude Code, VSCode Copilot, Goose, Cursor v1.6+) inject it into the LLM's system prompt automatically. Before this change, AdLoop set `instructions` to a 3-sentence placeholder. This commit replaces it with a compact ~500-token orchestration hint covering the must-knows that prevent the most expensive mistakes: - Two-step write pattern (preview -> confirm_and_apply, never skip). - dry_run=true defaults, require_dry_run config override. - max_daily_budget cap, PAUSED-by-default for new campaigns/RSAs. - BROAD match requires Smart Bidding (refuse on MANUAL_CPC). - Verify final_url before creating ads/sitelinks. - Don't throw budget at zero-conversion campaigns; fix tracking first. - GDPR consent gap is normal in EU (clicks > sessions != tracking bug). - Geo + language targeting mandatory on new campaigns. - Pointer to the full ruleset (.cursor/rules/adloop.mdc / install-rules). The full ruleset stays canonical in the existing locations; `instructions` is intentionally compact because the spec describes it as "a hint to the model" rather than a place to ship 50KB user manuals. Tests cap the field at <5KB so we don't accidentally regress. Net effect: every MCP client that honors `instructions` (most major implementations, per spec compliance) now gets the safety must-knows automatically — no install-rules step needed for the bare-minimum orchestration. install-rules remains valuable for full rules + slash commands, especially for clients like Cowork that may not yet honor the field. 6 new tests verify content coverage, compactness cap, and wiring through to the FastMCP instance. Full suite: 190 passed (184 + 6 new). --- src/adloop/server.py | 68 ++++++++++++++++++++++++++++++++++++++--- tests/test_server.py | 72 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 133 insertions(+), 7 deletions(-) diff --git a/src/adloop/server.py b/src/adloop/server.py index fb9035a..912fae8 100644 --- a/src/adloop/server.py +++ b/src/adloop/server.py @@ -18,13 +18,71 @@ _WRITE = ToolAnnotations(readOnlyHint=False, destructiveHint=False) _DESTRUCTIVE = ToolAnnotations(readOnlyHint=False, destructiveHint=True) +def _build_orchestration_instructions() -> str: + """Compact orchestration hint sent via MCP ``InitializeResult.instructions``. + + Per the MCP spec, ``instructions`` is described as "a hint to the model" — + not a place to dump a 50KB manual. So we send a curated, ~500-token subset + covering the rules that matter most when used **without** the full + orchestration rules loaded (e.g. in MCP clients that don't pick up project + rules). The full ruleset stays canonical at: + + - ``.cursor/rules/adloop.mdc`` (Cursor — auto-loaded as a workspace rule) + - ``.claude/rules/adloop.md`` (Claude Code in this repo) + - ``~/.claude/CLAUDE.md`` (after the user runs ``adloop install-rules``) + + Clients that honor ``instructions`` (Claude Code, VSCode Copilot, Goose, + Cursor v1.6+) will inject this hint into the LLM's system prompt + automatically — so even users running AdLoop without any per-project + rules file get the absolute must-knows around safety, dry-run defaults, + and the most common cost-burning mistakes. + """ + return ( + "AdLoop connects Google Ads + Google Analytics (GA4) + your codebase. " + "These are the *minimum* orchestration rules — the full ruleset lives " + "in `.cursor/rules/adloop.mdc` or `~/.claude/CLAUDE.md` (run " + "`adloop install-rules` to install globally). Read these before using " + "any write tool.\n\n" + "SAFETY (always):\n" + "- Every write tool returns a PREVIEW with a `plan_id`. Show the " + "preview to the user and wait for explicit approval before calling " + "`confirm_and_apply`.\n" + "- Default to `dry_run=true` on `confirm_and_apply`. Only set " + "`dry_run=false` after the user explicitly approves the preview. " + "`require_dry_run` in config can override this.\n" + "- Respect the config's `max_daily_budget` cap.\n" + "- New campaigns and RSAs are created PAUSED. The user must enable " + "them after review.\n" + "- One change at a time — don't batch unrelated writes.\n\n" + "PRE-WRITE CHECKS (before any `draft_*`):\n" + "- BROAD match keywords require Smart Bidding (MAXIMIZE_CONVERSIONS, " + "tCPA, tROAS). Refuse BROAD on MANUAL_CPC. This is the #1 cause of " + "wasted ad spend.\n" + "- Verify `final_url` exists before creating ads or sitelinks. URLs " + "to 404 pages destroy quality score.\n" + "- If a campaign has zero conversions and high spend, fix tracking " + "before adding more ads/keywords. Don't just throw budget at it.\n" + "- If keyword quality scores are <5, fix ad relevance and landing " + "pages before adding more keywords.\n\n" + "DATA LITERACY:\n" + "- Ads clicks > GA4 sessions is normal in EU markets due to GDPR " + "consent rejection (typically 30-70% of users opt out). It's not a " + "tracking bug. Use `analyze_campaign_conversions` and " + "`attribution_check` — they factor this in.\n" + "- `cost_micros / 1,000,000` = actual currency. Read tools " + "auto-compute `metrics.cost`; only `run_gaql` returns raw micros.\n" + "- New campaigns MUST have `geo_target_ids` and `language_ids` set. " + "Untargeted campaigns waste budget.\n\n" + "For full orchestration patterns (when to call which tools, GAQL " + "reference, language-specific copy guidance, PMax handling, " + "shared-set lifecycle, RSA pinning trade-offs), see the canonical " + "rules file." + ) + + mcp = FastMCP( "AdLoop", - instructions=( - "AdLoop connects Google Ads and Google Analytics (GA4) data to your " - "codebase. Use the read tools to analyze performance, and the write " - "tools (with safety confirmation) to manage campaigns." - ), + instructions=_build_orchestration_instructions(), ) _config = load_config() diff --git a/tests/test_server.py b/tests/test_server.py index c997afa..61d712c 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -1,7 +1,7 @@ -"""Tests for server error formatting.""" +"""Tests for server error formatting and MCP instructions field.""" from adloop.ads.gaql import _parse_gaql_error -from adloop.server import _structured_error +from adloop.server import _build_orchestration_instructions, _structured_error def test_structured_error_detects_invalid_developer_token(): @@ -65,3 +65,71 @@ def test_parse_gaql_error_detects_test_only_developer_token(): assert result.startswith("DEVELOPER_TOKEN_NOT_APPROVED:") assert "test accounts" in result + + +# --------------------------------------------------------------------------- +# MCP InitializeResult.instructions — orchestration hint +# --------------------------------------------------------------------------- + + +class TestOrchestrationInstructions: + """The MCP `instructions` field is sent during the initialize handshake + and (per spec) MAY be injected into the LLM's system prompt by clients + that honor it. We send a compact must-knows summary, NOT the full ruleset. + """ + + def test_instructions_cover_safety_essentials(self): + text = _build_orchestration_instructions() + + # Must mention the two-step write pattern. + assert "PREVIEW" in text or "preview" in text + assert "plan_id" in text + assert "confirm_and_apply" in text + + # Must mention dry_run defaults. + assert "dry_run" in text + + def test_instructions_cover_pre_write_checks(self): + text = _build_orchestration_instructions() + # Most expensive mistake we want to prevent. + assert "BROAD" in text and "Smart Bidding" in text + # Second-most expensive: dead URLs. + assert "final_url" in text or "URL" in text + # Avoid throwing budget at broken tracking. + assert "tracking" in text.lower() or "conversions" in text.lower() + + def test_instructions_cover_data_literacy(self): + text = _build_orchestration_instructions() + # GDPR consent gap is the #1 source of misdiagnosed "tracking issues". + assert "GDPR" in text or "consent" in text.lower() + # Geo / language targeting is mandatory. + assert "geo" in text.lower() + assert "language" in text.lower() + + def test_instructions_point_at_full_ruleset(self): + text = _build_orchestration_instructions() + # The hint should tell the model where the full rules live. + assert "install-rules" in text or "adloop.mdc" in text or "CLAUDE.md" in text + + def test_instructions_are_compact_not_full_rules(self): + # Spec describes `instructions` as a "hint" — not a manual. + # Hard cap so we don't accidentally regress to dumping 50KB through + # the handshake. + text = _build_orchestration_instructions() + assert len(text) < 5_000, ( + f"instructions field is {len(text)} bytes — should be a compact " + f"hint (<5KB). For full rules use install-rules." + ) + + def test_instructions_are_attached_to_mcp_server(self): + # FastMCP forwards the constructor arg into the wire-protocol + # InitializeResult.instructions. Sanity check it's actually wired. + from adloop.server import mcp + + instructions = getattr(mcp, "instructions", None) + assert instructions is not None + assert isinstance(instructions, str) + assert len(instructions) > 100, ( + "MCP instructions field should be the compact orchestration hint, " + "not a placeholder" + )