App Ad asset editing, billing, change history, and an identifier guard - #7
Merged
Merged
Conversation
…r guard ## Commands - `ads assets` reads an App Ad's real asset list from `app_ad.*` with slot fill and per-orientation coverage. `ad_group_ad_asset_view` retains historical associations and can report more assets than the ad carries, so it is not used as the source of truth. - `ads set-assets` edits App Ad assets in place. Those fields are whole-field replacements, so an `update_mask` on `app_ad.images` silently drops anything omitted; the command reads current state and applies an add/remove delta on top. Caps, duplicates, removing an absent asset, and stripping every visual asset are rejected before the API call. This removes the need to rebuild an ad group per creative change, which left undeletable ads behind. - `billing show` reports funding, remaining balance, and runway. account_budget returns the NET spendable amount, so a prepay top-up shown as a gross figure in the UI yields a shorter runway than expected; `--tax-rate` prints a gross-equivalent column to reconcile the two. - `changes list` surfaces change_event history, supplying the bounded date window and LIMIT the resource requires. - Report presets: assets, network, daily-campaign. Video orientation is inferred from asset names because the API does not expose video aspect ratio; explicit ratio tokens take priority over words like "portrait", which creative names often use for the subject rather than the frame. Not available through the API and left to the web UI: promotional account credits, SKAdNetwork reports, and Google's per-orientation Ad Strength breakdown for App ads (asset_group.asset_coverage is Performance Max only). ## Identifier guard Building the above put real account IDs, an account balance, live ad copy, and internal creative naming into tests and docs. detect-secrets reported clean throughout, because it recognizes credentials, not business identifiers — a real customer ID is just a ten-digit number to it. CI is also the wrong layer. It runs on push, and this repository is public: by the time CI objects the value is already published, and git history keeps it after any later fix. The gate has to run before the commit. scripts/check_identifiers.py pairs two rules, wired into pre-commit: - Denylist: anything in .private-values (gitignored) fails. Precise, but only catches values someone remembered to write down. - Allowlist: every 8+ digit number, grouped payments-shaped ID, and email must appear in .identifier-allowlist.txt. This is the rule that catches values nobody knew to list, and while wiring it up it flagged a real account_budget ID still sitting in tests after a manual sweep for known IDs. Numeric separators are stripped before matching so a balance cannot hide behind underscores. The guard also rejected the first draft of its own test file, which had used real IDs as fixtures; sample values are now assembled from fragments so no literal identifier exists in a tracked file. Adding an allowlist line is deliberately a reviewable act: that line is where a reviewer asks whether the value is real. CI keeps the check as a backstop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Summary
ads assets/ads set-assets— read an App Ad's real assets fromapp_ad.*and edit them in place (add/remove/set), instead of rebuilding the ad group per creative change.ad_group_ad_asset_viewretains historical associations and is not used as the source of truth.billing show— funding, net remaining balance, and spend runway.account_budgetreports the net amount;--tax-rateprints a gross-equivalent for reconciling with the web UI.changes list—change_eventhistory with the bounded date window andLIMITthe resource requires.assets,network,daily-campaign.scripts/check_identifiers.py+.pre-commit-config.yaml— blocks real account identifiers (IDs, balances, emails, live ad copy) from entering this public repo.detect-secretsonly recognizes credentials, not business identifiers, so it misses these entirely.Test plan
ruff check ./ruff format --check .pytest(67 tests, 65% coverage, threshold 55%)detect-secrets-hookagainst tracked filesscripts/check_identifiers.pyagainst tracked files and full git history--validate-only; no writes)🤖 Generated with Claude Code