feat(google-ads): budget type (daily vs CUSTOM_PERIOD total) get and set (#366)#370
Closed
hyoshi wants to merge 3 commits into
Closed
Conversation
…ds_ad_sets_update (#367) - Expose end_time (ISO 8601 string | UNIX timestamp | 0 to clear the end date per the Meta Marketing API convention) and lifetime_budget on the meta_ads_ad_sets_update tool inputSchema and forward them to update_ad_set. - Normalize the string "0" to the integer 0 before the contradiction guard so a string-typed zero cannot sidestep it; reject bool, negative, and blank end_time values. - Validate daily_budget/lifetime_budget mutual exclusion and refuse lifetime_budget combined with end_time=0 (a lifetime budget requires a schedule end). - Route lifetime_budget through _validate_positive_money. - StrategyPolicyGate: new max_lifetime_budget_per_campaign guardrail so lifetime-budget mutations cannot sidestep STRATEGY.md budget caps (fail-open contract preserved); documented in the strategy skill. Closes #367
…/366-google-ads-budget-period
…set (#366) - get_budget now returns name, period (DAILY / CUSTOM_PERIOD), delivery_method, status (enum-mapped), total_budget / total_amount_micros (null unless CUSTOM_PERIOD), and reference_count. - update_budget accepts total_amount / total_amount_micros alongside the daily amount, with a dynamic update mask. period is create-only — the API marks CampaignBudget.period immutable. - create_budget accepts period (DAILY / CUSTOM_PERIOD whitelist) and total amounts; totals require CUSTOM_PERIOD and vice versa (fail fast). - All amount paths (daily + total, create + update) route through the shared _resolve_amount_micros validator: positive, catastrophe ceiling, bool/inf rejection, pair mutual exclusion. - StrategyPolicyGate: max_lifetime_budget_per_campaign now also catches Google total budgets in both spellings (total_amount, currency units; total_amount_micros, micros); the daily cap additionally reads amount_micros — closing parameter-form bypasses found in review. - BYOD get_budget shape parity; budget tool schemas + handlers updated; _mureo-google-ads and _mureo-strategy skills documented. Covers issue #366 task 3. Closes #366 together with #369. Claude-Session: https://claude.ai/code/session_0195WBK6YopTveq9RAQE2xMz
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #366 (task 3 — budget type get/set). Stacked on #369 (which stacks on #368): merge #368 → #369 first, then retarget this PR's base to
main. Closes #366 together with #369.Summary
Google Ads budgets come in two types — DAILY and CUSTOM_PERIOD (campaign-lifetime total). mureo previously read/wrote only the daily amount. This PR:
google_ads_budget_getname,period(DAILY / CUSTOM_PERIOD),delivery_method, enum-mappedstatus,total_budget/total_amount_micros(null unless CUSTOM_PERIOD),reference_count— fixing the long-standing drift where the description promised these fieldsgoogle_ads_budget_updatetotal_amount/total_amount_microsalongside (or instead of) the daily amount, with a dynamic update maskgoogle_ads_budget_createperiod+ total amounts;requiredrelaxed to["name"]+ anyOf so a pure total budget needs no daily amountDesign notes
periodis create-only: the Google Ads API marksCampaignBudget.periodImmutable (verified from the proto docstrings), soupdate_budgetdeliberately does not accept it, despite the issue text suggesting both. Symmetric fail-fast guards: a total withoutperiod='CUSTOM_PERIOD'and CUSTOM_PERIOD without a total are bothValueErrors before any API call._resolve_amount_micros— positive,_MAX_BUDGET_AMOUNT_MICROSceiling, bool/inf rejection, pair mutual exclusion. The create path's legacy bareint(amount_micros)(no ceiling) is gone.max_lifetime_budget_per_campaignnow catches Google totals in both spellings (total_amountcurrency units,total_amount_micros÷1e6), and the daily cap additionally readsamount_micros— closing parameter-form bypasses found in security review.get_budgetshape parity (period=DAILY, null totals)._mureo-google-ads+_mureo-strategyskills (canonical + packaged byte-identical).Test plan
TDD (tests first). Reviews: security-reviewer (HIGH
total_amountgate bypass → fixed → CONFIRMED) + python-reviewer (MEDIUM schema wording + 2 LOW → fixed → CONFIRM).https://claude.ai/code/session_0195WBK6YopTveq9RAQE2xMz