diff --git a/.gitignore b/.gitignore index 85805de..b5726a4 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,5 @@ credentials.json .envrc .claude dogfood-output/ + +__pycache__/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 1775b74..4ad9876 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,39 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- `link_asset_to_asset_group`: link an existing asset to a Performance Max + asset group via `assetGroupAssetOperation.create`. `upload_image_asset` only + ever created the asset — nothing attached it, so uploaded images sat unused + in the account's asset library. A PMax asset group without a + `MARKETING_IMAGE`, `SQUARE_MARKETING_IMAGE` and `LOGO` is "Not eligible" and + never serves, so this is the step that makes an asset group deliverable. The + field type is validated client-side against the 11 types PMax accepts, since + the API error for a bad one is opaque. +- `add_asset_group_signal`: add search themes and/or audience signals to a + Performance Max asset group via `assetGroupSignalOperation.create`. PMax does + not take audiences as campaign criteria — `add_audience_targeting` writes a + `campaignCriterion`, which the API rejects for a PMax campaign. Accepts + search themes, audience IDs, or both, and enforces the 80-character search + theme limit before the request goes out. +- `scripts/gads_mutate.py`: send a `googleAds:mutate` payload and print + Google's full error tree, with `--check` for `validateOnly`. The server + collapses API failures to "Request contains an invalid argument", which hides + the field-level reason that actually explains a rejected mutate. + +### Fixed + +- `create_pmax_campaign` now sets `campaign_budget.explicitly_shared = false`. + Budgets default to shared server-side, and Performance Max rejects a shared + budget with `BIDDING_STRATEGY_TYPE_INCOMPATIBLE_WITH_SHARED_BUDGET`. + `draft_campaign` got this fix in 5670d2d; the PMax path was missed. +- `create_pmax_campaign` now sets + `campaign.contains_eu_political_advertising = DOES_NOT_CONTAIN_EU_POLITICAL_ADVERTISING`. + The field is required on every campaign create, and omitting it failed the + entire mutate with `fieldError=REQUIRED`. + + Together these two meant `create_pmax_campaign` could not create any + Performance Max campaign — every call failed at `confirm_and_apply`. + - `set_campaign_geo_target_type`: set a campaign's `campaign.geo_target_type_setting` — `positive_geo_target_type` and/or `negative_geo_target_type` — via a `campaignOperation.update` with a matching diff --git a/scripts/gads_mutate.py b/scripts/gads_mutate.py new file mode 100755 index 0000000..a58243a --- /dev/null +++ b/scripts/gads_mutate.py @@ -0,0 +1,129 @@ +#!/usr/bin/env python3 +"""Run a googleAds:mutate request and print Google's full error detail. + +The MCP server collapses API failures to "Request contains an invalid argument", +which hides the field-level reason. This sends the same payload and prints the +whole error tree, so a rejected mutate can actually be diagnosed. + +Usage: + scripts/gads_mutate.py operations.json # apply + scripts/gads_mutate.py operations.json --check # validate_only, changes nothing + +operations.json is the JSON array that goes in "mutateOperations". +Reads .env from the repo root (same file scripts/mcp-env.sh uses). +""" +import json +import os +import sys +import urllib.error +import urllib.parse +import urllib.request + +REPO = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +API = "https://googleads.googleapis.com/v23" + + +def load_env(): + env = {} + path = os.environ.get("MCP_GOOGLE_ADS_ENV", os.path.join(REPO, ".env")) + with open(path) as fh: + for line in fh: + line = line.strip() + if not line or line.startswith("#") or "=" not in line: + continue + k, _, v = line.partition("=") + env[k.strip()] = v.strip().strip('"').strip("'") + return env + + +def access_token(env): + with open(os.path.expanduser(env["GOOGLE_ADS_CREDENTIALS_PATH"])) as fh: + cred = json.load(fh)["installed"] + with open(os.path.expanduser(env["GOOGLE_ADS_TOKEN_PATH"])) as fh: + refresh = json.load(fh)["refresh_token"] + body = urllib.parse.urlencode({ + "client_id": cred["client_id"], + "client_secret": cred["client_secret"], + "refresh_token": refresh, + "grant_type": "refresh_token", + }).encode() + req = urllib.request.Request("https://oauth2.googleapis.com/token", data=body) + with urllib.request.urlopen(req) as resp: + return json.load(resp)["access_token"] + + +def headers(env, token): + h = { + "Authorization": f"Bearer {token}", + "developer-token": env["GOOGLE_ADS_DEVELOPER_TOKEN"], + "Content-Type": "application/json", + } + login = env.get("GOOGLE_ADS_LOGIN_CUSTOMER_ID", "").replace("-", "") + if login: + h["login-customer-id"] = login + return h + + +def mutate(env, token, operations, validate_only=False, customer_id=None): + cust = (customer_id or env["GOOGLE_ADS_CUSTOMER_ID"]).replace("-", "") + payload = {"mutateOperations": operations, "validateOnly": validate_only} + req = urllib.request.Request( + f"{API}/customers/{cust}/googleAds:mutate", + data=json.dumps(payload).encode(), + headers=headers(env, token), + ) + try: + with urllib.request.urlopen(req) as resp: + return json.load(resp), None + except urllib.error.HTTPError as e: + return None, json.load(e) + + +def describe(err): + """Flatten Google's nested error tree into readable lines.""" + lines = [] + for detail in err.get("error", {}).get("details", []): + for item in detail.get("errors", []): + code = item.get("errorCode", {}) + code_str = ", ".join(f"{k}={v}" for k, v in code.items()) + loc = item.get("location", {}) + path = ".".join( + str(f.get("fieldName", f.get("index", ""))) + for f in loc.get("fieldPathElements", []) + ) + lines.append(f" [{code_str}] {item.get('message', '')}") + if path: + lines.append(f" at: {path}") + if not lines: + lines.append(f" {err.get('error', {}).get('message', json.dumps(err))}") + return "\n".join(lines) + + +def main(): + args = [a for a in sys.argv[1:] if not a.startswith("--")] + validate_only = "--check" in sys.argv + if not args: + print(__doc__) + return 2 + + with open(args[0]) as fh: + operations = json.load(fh) + + env = load_env() + token = access_token(env) + cid = os.environ.get("GADS_CUSTOMER_ID") + result, err = mutate(env, token, operations, validate_only, cid) + + if err: + print(f"FAILED ({'validate' if validate_only else 'apply'}):", file=sys.stderr) + print(describe(err), file=sys.stderr) + return 1 + + print(f"OK ({'validated' if validate_only else 'applied'}) " + f"{len(operations)} operation(s)") + print(json.dumps(result, indent=2)) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/src/lib.rs b/src/lib.rs index 11b3d78..5ded7fc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -447,6 +447,34 @@ pub struct UploadTextAssetToolParams { pub text_content: String, } +/// Parameters for linking an asset to a Performance Max asset group. +#[derive(Debug, Deserialize, Serialize, JsonSchema)] +pub struct LinkAssetToAssetGroupToolParams { + /// Customer ID (e.g. 123-456-7890). Defaults to configured customer_id. + pub customer_id: Option, + /// The asset group ID to link the asset to. + pub asset_group_id: String, + /// The asset ID to link (from upload_image_asset / upload_text_asset). + pub asset_id: String, + /// Field type: MARKETING_IMAGE, SQUARE_MARKETING_IMAGE, PORTRAIT_MARKETING_IMAGE, + /// LOGO, LANDSCAPE_LOGO, YOUTUBE_VIDEO, HEADLINE, DESCRIPTION, LONG_HEADLINE, + /// BUSINESS_NAME, CALL_TO_ACTION_SELECTION. + pub field_type: String, +} + +/// Parameters for adding signals to a Performance Max asset group. +#[derive(Debug, Deserialize, Serialize, JsonSchema)] +pub struct AddAssetGroupSignalToolParams { + /// Customer ID (e.g. 123-456-7890). Defaults to configured customer_id. + pub customer_id: Option, + /// The asset group ID to add signals to. + pub asset_group_id: String, + /// Search themes (max 80 chars each) telling PMax what queries to look for. + pub search_themes: Option>, + /// Audience IDs referencing customers/{cid}/audiences/{id}. + pub audience_ids: Option>, +} + /// Parameters for setting campaign ad schedule. #[derive(Debug, Deserialize, Serialize, JsonSchema)] pub struct SetCampaignScheduleToolParams { @@ -1403,6 +1431,59 @@ impl GoogleAdsMcp { } } + #[tool( + description = "Link an uploaded asset to a Performance Max asset group. Required for images \ + to serve — upload_image_asset alone leaves the asset unattached. Returns a preview." + )] + async fn link_asset_to_asset_group( + &self, + Parameters(params): Parameters, + ) -> String { + if let Some(err) = self.check_write_allowed() { + return err; + } + let cid = self.resolve_customer_id(params.customer_id.as_deref()); + let config = self.config.clone(); + + match tools::assets::link_asset_to_asset_group( + &config, + &cid, + ¶ms.asset_group_id, + ¶ms.asset_id, + ¶ms.field_type, + ) { + Ok(preview) => preview.to_string(), + Err(e) => serde_json::json!({"error": e.to_string()}).to_string(), + } + } + + #[tool( + description = "Add search themes and/or audience signals to a Performance Max asset group. \ + Use this for PMax audiences — add_audience_targeting writes a campaign \ + criterion, which PMax rejects. Returns a preview." + )] + async fn add_asset_group_signal( + &self, + Parameters(params): Parameters, + ) -> String { + if let Some(err) = self.check_write_allowed() { + return err; + } + let cid = self.resolve_customer_id(params.customer_id.as_deref()); + let config = self.config.clone(); + + match tools::audiences::add_asset_group_signal( + &config, + &cid, + ¶ms.asset_group_id, + ¶ms.search_themes.unwrap_or_default(), + ¶ms.audience_ids.unwrap_or_default(), + ) { + Ok(preview) => preview.to_string(), + Err(e) => serde_json::json!({"error": e.to_string()}).to_string(), + } + } + // ── Phase 5: Scheduling ───────────────────────────────────────────── #[tool( diff --git a/src/tools/assets.rs b/src/tools/assets.rs index 9024c9c..327c814 100644 --- a/src/tools/assets.rs +++ b/src/tools/assets.rs @@ -125,6 +125,95 @@ pub fn upload_text_asset( Ok(preview) } +/// Asset group field types accepted by `link_asset_to_asset_group`. +/// +/// Performance Max rejects any other field type on an `assetGroupAsset`, and the +/// API error for a bad one is opaque, so we reject client-side instead. +pub const VALID_ASSET_GROUP_FIELD_TYPES: &[&str] = &[ + "HEADLINE", + "DESCRIPTION", + "LONG_HEADLINE", + "BUSINESS_NAME", + "MARKETING_IMAGE", + "SQUARE_MARKETING_IMAGE", + "PORTRAIT_MARKETING_IMAGE", + "LOGO", + "LANDSCAPE_LOGO", + "YOUTUBE_VIDEO", + "CALL_TO_ACTION_SELECTION", +]; + +/// Link an existing asset to a Performance Max asset group. +/// +/// `upload_image_asset` only creates the asset — it lands in the account's asset +/// library unattached. A PMax asset group with no MARKETING_IMAGE, +/// SQUARE_MARKETING_IMAGE and LOGO is "Not eligible" and never serves, so this +/// is the step that actually makes an asset group deliverable. +/// +/// Returns a ChangePlan preview that must be confirmed via `confirm_and_apply`. +pub fn link_asset_to_asset_group( + config: &Config, + customer_id: &str, + asset_group_id: &str, + asset_id: &str, + field_type: &str, +) -> Result { + check_blocked_operation("link_asset_to_asset_group", &config.safety)?; + + if asset_group_id.is_empty() { + return Err(McpGoogleAdsError::Validation( + "Asset group ID cannot be empty".to_string(), + )); + } + + if asset_id.is_empty() { + return Err(McpGoogleAdsError::Validation( + "Asset ID cannot be empty".to_string(), + )); + } + + let field_type = field_type.to_uppercase(); + if !VALID_ASSET_GROUP_FIELD_TYPES.contains(&field_type.as_str()) { + return Err(McpGoogleAdsError::Validation(format!( + "Invalid asset group field type '{}'. Must be one of: {}", + field_type, + VALID_ASSET_GROUP_FIELD_TYPES.join(", ") + ))); + } + + let cid = crate::client::GoogleAdsClient::normalize_customer_id(customer_id); + + let operation = json!({ + "assetGroupAssetOperation": { + "create": { + "assetGroup": format!("customers/{}/assetGroups/{}", cid, asset_group_id), + "asset": format!("customers/{}/assets/{}", cid, asset_id), + "fieldType": field_type + } + } + }); + + let changes = json!({ + "asset_group_id": asset_group_id, + "asset_id": asset_id, + "field_type": field_type + }); + + let plan = ChangePlan::new( + "link_asset_to_asset_group".to_string(), + "asset_group_asset".to_string(), + asset_group_id.to_string(), + cid, + changes, + false, + vec![operation], + ); + + let preview = plan.to_preview(); + store_plan(plan); + Ok(preview) +} + #[cfg(test)] mod tests { use super::*; @@ -193,4 +282,61 @@ mod tests { let err = result.err().map(|e| e.to_string()).unwrap_or_default(); assert!(err.contains("blocked")); } + + #[test] + fn test_link_asset_to_asset_group_success() { + let config = Config::default(); + let result = link_asset_to_asset_group( + &config, + "123-456-7890", + "6738426770", + "404988595285", + "MARKETING_IMAGE", + ); + assert!(result.is_ok()); + let preview = result.ok().unwrap_or_default(); + assert_eq!(preview["operation"], "link_asset_to_asset_group"); + assert_eq!(preview["changes"]["field_type"], "MARKETING_IMAGE"); + } + + #[test] + fn test_link_asset_to_asset_group_lowercase_field_type_is_normalized() { + let config = Config::default(); + let result = link_asset_to_asset_group( + &config, + "123-456-7890", + "123", + "456", + "square_marketing_image", + ); + assert!(result.is_ok()); + let preview = result.ok().unwrap_or_default(); + assert_eq!(preview["changes"]["field_type"], "SQUARE_MARKETING_IMAGE"); + } + + #[test] + fn test_link_asset_to_asset_group_invalid_field_type() { + let config = Config::default(); + let result = link_asset_to_asset_group(&config, "123-456-7890", "123", "456", "SITELINK"); + assert!(result.is_err()); + let err = result.err().map(|e| e.to_string()).unwrap_or_default(); + assert!(err.contains("Invalid asset group field type")); + } + + #[test] + fn test_link_asset_to_asset_group_empty_ids() { + let config = Config::default(); + assert!(link_asset_to_asset_group(&config, "123-456-7890", "", "456", "LOGO").is_err()); + assert!(link_asset_to_asset_group(&config, "123-456-7890", "123", "", "LOGO").is_err()); + } + + #[test] + fn test_link_asset_to_asset_group_blocked() { + let mut config = Config::default(); + config.safety.blocked_operations = vec!["link_asset_to_asset_group".to_string()]; + let result = link_asset_to_asset_group(&config, "123-456-7890", "123", "456", "LOGO"); + assert!(result.is_err()); + let err = result.err().map(|e| e.to_string()).unwrap_or_default(); + assert!(err.contains("blocked")); + } } diff --git a/src/tools/audiences.rs b/src/tools/audiences.rs index ca1b517..8c7cd9f 100644 --- a/src/tools/audiences.rs +++ b/src/tools/audiences.rs @@ -158,6 +158,108 @@ pub fn add_audience_targeting( Ok(preview) } +/// Maximum length of a Performance Max search theme, per the Google Ads API. +const MAX_SEARCH_THEME_LEN: usize = 80; + +/// Add signals to a Performance Max asset group. +/// +/// PMax does NOT take audiences as campaign criteria — `add_audience_targeting` +/// writes a `campaignCriterion`, which the API rejects for a PMax campaign. +/// Audience signals and search themes belong on `asset_group_signal`, and they +/// are what PMax uses to seed targeting before it has conversion history. +/// +/// Supply `search_themes`, `audience_ids`, or both. Audience IDs reference +/// `customers/{cid}/audiences/{id}` — an Audience resource, not a user list. +/// +/// Returns a ChangePlan preview that must be confirmed via `confirm_and_apply`. +pub fn add_asset_group_signal( + config: &Config, + customer_id: &str, + asset_group_id: &str, + search_themes: &[String], + audience_ids: &[String], +) -> Result { + check_blocked_operation("add_asset_group_signal", &config.safety)?; + + if asset_group_id.is_empty() { + return Err(McpGoogleAdsError::Validation( + "Asset group ID cannot be empty".to_string(), + )); + } + + if search_themes.is_empty() && audience_ids.is_empty() { + return Err(McpGoogleAdsError::Validation( + "At least one search theme or audience ID is required".to_string(), + )); + } + + for theme in search_themes { + if theme.trim().is_empty() { + return Err(McpGoogleAdsError::Validation( + "Search theme cannot be empty".to_string(), + )); + } + if theme.chars().count() > MAX_SEARCH_THEME_LEN { + return Err(McpGoogleAdsError::Validation(format!( + "Search theme '{}' is {} chars, exceeds the {} char limit", + theme, + theme.chars().count(), + MAX_SEARCH_THEME_LEN + ))); + } + } + + let cid = crate::client::GoogleAdsClient::normalize_customer_id(customer_id); + let asset_group_resource = format!("customers/{}/assetGroups/{}", cid, asset_group_id); + + let mut operations: Vec = Vec::new(); + + for theme in search_themes { + operations.push(json!({ + "assetGroupSignalOperation": { + "create": { + "assetGroup": asset_group_resource, + "searchTheme": { "text": theme } + } + } + })); + } + + for audience_id in audience_ids { + operations.push(json!({ + "assetGroupSignalOperation": { + "create": { + "assetGroup": asset_group_resource, + "audience": { + "audience": format!("customers/{}/audiences/{}", cid, audience_id) + } + } + } + })); + } + + let changes = json!({ + "asset_group_id": asset_group_id, + "search_themes": search_themes, + "audience_ids": audience_ids, + "signal_count": operations.len() + }); + + let plan = ChangePlan::new( + "add_asset_group_signal".to_string(), + "asset_group_signal".to_string(), + asset_group_id.to_string(), + cid, + changes, + false, + operations, + ); + + let preview = plan.to_preview(); + store_plan(plan); + Ok(preview) +} + #[cfg(test)] mod tests { use super::*; @@ -266,4 +368,71 @@ mod tests { let err = result.err().map(|e| e.to_string()).unwrap_or_default(); assert!(err.contains("blocked")); } + + #[test] + fn test_add_asset_group_signal_search_themes() { + let config = Config::default(); + let themes = vec!["web design sydney".to_string(), "online shops".to_string()]; + let result = add_asset_group_signal(&config, "123-456-7890", "6738426770", &themes, &[]); + assert!(result.is_ok()); + let preview = result.ok().unwrap_or_default(); + assert_eq!(preview["operation"], "add_asset_group_signal"); + assert_eq!(preview["changes"]["signal_count"], 2); + } + + #[test] + fn test_add_asset_group_signal_combines_themes_and_audiences() { + let config = Config::default(); + let themes = vec!["web design sydney".to_string()]; + let audiences = vec!["111".to_string(), "222".to_string()]; + let result = + add_asset_group_signal(&config, "123-456-7890", "6738426770", &themes, &audiences); + assert!(result.is_ok()); + let preview = result.ok().unwrap_or_default(); + assert_eq!(preview["changes"]["signal_count"], 3); + } + + #[test] + fn test_add_asset_group_signal_requires_at_least_one_signal() { + let config = Config::default(); + let result = add_asset_group_signal(&config, "123-456-7890", "6738426770", &[], &[]); + assert!(result.is_err()); + } + + #[test] + fn test_add_asset_group_signal_rejects_overlong_theme() { + let config = Config::default(); + let themes = vec!["a".repeat(MAX_SEARCH_THEME_LEN + 1)]; + let result = add_asset_group_signal(&config, "123-456-7890", "123", &themes, &[]); + assert!(result.is_err()); + let err = result.err().map(|e| e.to_string()).unwrap_or_default(); + assert!(err.contains("exceeds")); + } + + #[test] + fn test_add_asset_group_signal_rejects_empty_theme() { + let config = Config::default(); + let themes = vec![" ".to_string()]; + let result = add_asset_group_signal(&config, "123-456-7890", "123", &themes, &[]); + assert!(result.is_err()); + } + + #[test] + fn test_add_asset_group_signal_empty_asset_group_id() { + let config = Config::default(); + let themes = vec!["web design".to_string()]; + let result = add_asset_group_signal(&config, "123-456-7890", "", &themes, &[]); + assert!(result.is_err()); + } + + #[test] + fn test_add_asset_group_signal_blocked() { + let mut config = Config::default(); + config.safety.blocked_operations = vec!["add_asset_group_signal".to_string()]; + let themes = vec!["web design".to_string()]; + let result = add_asset_group_signal(&config, "123-456-7890", "123", &themes, &[]); + assert!(result.is_err()); + let err = result.err().map(|e| e.to_string()).unwrap_or_default(); + assert!(err.contains("blocked")); + } } diff --git a/src/tools/pmax.rs b/src/tools/pmax.rs index 33730a1..23b06ba 100644 --- a/src/tools/pmax.rs +++ b/src/tools/pmax.rs @@ -112,6 +112,10 @@ pub fn create_pmax_campaign(params: &CreatePmaxCampaignParams) -> Result Result Vec { + let config = Config::default(); + let params = default_params(&config); + let preview = create_pmax_campaign(¶ms).unwrap(); + let plan_id = preview["plan_id"].as_str().unwrap_or_default(); + let plan = crate::safety::preview::get_plan(plan_id).expect("plan stored"); + plan.mutate_operations.clone() + } + + #[test] + fn test_create_pmax_budget_not_shared() { + // Budgets default to shared server-side, and PMax rejects a shared budget + // with BIDDING_STRATEGY_TYPE_INCOMPATIBLE_WITH_SHARED_BUDGET. draft_campaign + // got this fix in 5670d2d; create_pmax_campaign was missed. + let ops = pmax_ops(); + let budget_create = ops + .iter() + .find_map(|op| op.pointer("/campaignBudgetOperation/create")) + .expect("budget operation present"); + assert_eq!(budget_create["explicitlyShared"], json!(false)); + } + + #[test] + fn test_create_pmax_sets_eu_political_advertising() { + // Required on every campaign create; without it the whole mutate fails + // with fieldError=REQUIRED on contains_eu_political_advertising. + let ops = pmax_ops(); + let campaign_create = ops + .iter() + .find_map(|op| op.pointer("/campaignOperation/create")) + .expect("campaign operation present"); + assert_eq!( + campaign_create["containsEuPoliticalAdvertising"], + json!("DOES_NOT_CONTAIN_EU_POLITICAL_ADVERTISING") + ); + } }