Watched a real operator copy the callback URL — as the MCP page tells
them to — and then open it in a browser tab, landing here with no query
string and getting "Missing code or state." Accurate, and completely
useless to the person reading it.
Only LinkedIn should ever call this URL, carrying ?code=...&state=...
after a real login, so a bare visit is a strong signal that somebody
misread the copy step. The response is the last place left to tell them,
so it now says what the URL is for and where it actually belongs
(Developer Portal -> app -> Auth -> Authorized redirect URLs), and that
Authenticate LinkedIn is the button that starts a real login.
LinkedIn's own errors are untouched: a genuine refusal still shows
LinkedIn's wording, not this guidance, since that is a different failure
and the operator needs LinkedIn's own words. Pinned by tests, including
one asserting the old bare symptom message cannot come back.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Investigated a "redirect_uri does not match the registered value" error
on a second laptop. Traced the code fully rather than assume a bug:
_linkedin_redirect_uri() computes one value, the frontend already
displays that exact value labeled "Add this callback URL in LinkedIn
Developer Portal," and the OAuth start endpoint sends that same value —
internally consistent, nothing to fix in application code.
Found the real cause instead: two separate LinkedIn login mechanisms
exist in this codebase from different points in its history — an old
terminal script (scripts/start-linkedin-oauth.ps1, port 44002, via the
linkedin-mcp-server npm package) and the current in-app button (port
8088, handled directly by the backend). The LinkedIn Developer Portal app
only ever had the old URL registered.
Documents both mechanisms side by side so they're never conflated again,
and records that this fix is shared across every machine using the same
LinkedIn app — unlike Google Cloud credentials, which are per-machine.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Written from an actual first-time setup on a second machine, in order,
including every wall that was hit and the exact click-path past it: the
two authentication methods, the two separate (legacy + managed)
organization policies that can block service account key creation, the
Vertex AI User role requirement, and a troubleshooting table mapping each
literal error message we saw to its real cause.
Leads with the one fact that caused most of the confusion: none of this
setup travels with git pull. Every machine running MaskanX needs it done
locally, once, even with identical code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The diagnostic logging just added (cf42152) immediately paid off: the
second laptop's real error, for all five fallback models, was google-auth's
own "Your default credentials were not found" — raised client-side before
any request reaches Google. This laptop has no service account JSON and
has never run `gcloud auth application-default login`; it was never about
Vertex AI Model Garden rollout at all.
The bug was mine: that message contains the literal substring "not
found", so _is_model_not_found_error's naive check misread "you have no
Google credentials on this machine" as "this specific model doesn't
exist" — retrying all five models for an identical, unfixable-by-retrying
failure, then still reporting the wrong final message ("model does not
exist or is not available") because _gemini_error_message made the same
substring mistake.
_is_missing_credentials_error checks for this specific error first, in
both places: the fallback loop now fails fast on the first model instead
of wasting four more identical round trips, and the final message tells
the operator exactly what is actually true — no Google Cloud credentials
exist on this machine, it is per-machine state, and there are two ways to
fix it (paste a service account JSON, or run the gcloud login command
locally) — not that a model is unavailable, which was never the case.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The reported failure now shows every model is unavailable through Vertex
on the second laptop, not only the newest one — meaning the fallback loop
(876b315) is running and exhausting all five candidates. That is a
genuinely different situation from "one new model isn't rolled out yet,"
and diagnosing it requires seeing what Google actually said.
Nothing was logged server-side before this: _gemini_error_message only
ever returned a paraphrase to the browser, and the fallback loop silently
swallowed a "not found" exception on every model but the last one with no
log line at all — four of five real error messages vanished before
anyone could read them.
Now: every exception this funnels through gets logged with its raw text
at the single point all callers pass through, and each skipped candidate
in the fallback loop logs before moving to the next. The next failure on
that laptop will show, in its own terminal, exactly what Google said for
every one of the five models tried — auth, permission, billing, or a
real 404 — instead of one collapsed sentence.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every test run this session logged the same warning: "PytestCacheWarning:
could not create cache path ...\.pytest_cache\v\cache\nodeids: [WinError 5]
Access is denied." The project lives under OneDrive on this machine, and
OneDrive's sync agent transiently locks files it's syncing — a classic
source of WinError 5 on Windows, and not something specific to this one
laptop's permissions.
`npm run test` now points pytest at .pytest-temp/.pytest-cache instead of
the default .pytest_cache, both gitignored. Harmless either way: it's
cache, not state.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reported: identical code and the same API key on two laptops, one said
"connected", the other said "Gemini (Google) model does not exist or is
not available." Traced to the connectivity probe hardcoding a single
model, gemini-3.6-flash — the newest entry in GEMINI_MODELS. Vertex AI
Model Garden availability is scoped per Google Cloud project and region,
so a brand-new model can reach one project's console before another; the
key, billing and every older model can be completely fine on the "failing"
laptop and this probe would still report the connection as broken.
Root env-var mechanism is separate and expected: GOOGLE_GENAI_USE_VERTEXAI
/ GOOGLE_CLOUD_PROJECT / GOOGLE_CLOUD_LOCATION live in envs.json, which is
machine-local and does not travel with `git pull` — each laptop configures
its own. That part is not a bug. The bug is that whichever laptop's project
had not yet had gemini-3.6-flash enabled got a false "everything is
broken" instead of "your key works, one specific model isn't rolled out
here yet."
Fix: try every model in the registry (oldest to newest, skipping the
tool-calling "-customtools" variant) and succeed on the first one that
works. A 404 on one model moves to the next; any other error (auth,
permission, quota, billing) stops immediately and is reported as-is,
since that applies no matter which model is asked for.
Also fixes a regression introduced while writing this: client construction
moved outside the per-model try/except, so bad service-account JSON or a
missing google-genai install would have escaped as an unhandled exception
instead of the graceful {"success": False, ...} every other path returns.
Caught by test_client_construction_failure_is_reported_gracefully before
it shipped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Code 100 / subcode 1885183 stopped the connection test after nine
consecutive field-mapping fixes, and it is not one: "Ads creative post was
created by an app that is in development mode. It must be in public to
create this ad." Decoded the access token via /debug_token to confirm
which app — MaskanXAds Integration, id 1592299622228272 — rather than
guess. Toggling it to Live on developers.facebook.com is the only fix;
nothing in MaskanX can do this on the user's behalf.
Documented next to the payment-method gate it sits beside in the setup
guide.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fourth failure in the same chain, one object further each time: "Advantage
audience flag required ... setting the advantage_audience flag to either 1
or 0 within the targeting_automation field" (code 100, subcode 1870227).
We were never sending targeting_automation at all.
Defaulting to 0 (off) for the same reason bid_strategy defaults to
LOWEST_COST_WITHOUT_CAP: an ad set should reach the audience it was told
to — the countries/age/gender actually set on the campaign — not whatever
Meta's Advantage+ expansion additionally decides to include.
advanced.advantage_audience (0 or 1) overrides it per campaign.
Ordering note: targeting_automation is added unconditionally, so the
existing "no audience" guard (raises when targeting is empty) had to move
before it — otherwise targeting would never be empty and that guard would
go silently dead. A test pins this: an empty CampaignSpec.targeting must
still be refused.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The moment a real MASKANX_OPERATOR_TOKENS got set in Settings >
Environments (this session, for the first time on this machine), 28 tests
in test_campaign_launch.py started failing with 401 instead of the status
each was actually testing.
Cause: adclaw/__init__.py calls load_envs_into_environ() at import time,
so envs.json — the exact file the Settings UI writes — becomes ambient
environment for every test process, not only the ones that opt in.
Most campaign tests deliberately exercise the unauthenticated path and
never send an operator header; once a real token map exists,
require_operator starts demanding one they don't send.
This was already named as a deferred minor in the Phase 2 ledger
("test_campaign_api client fixture does not delenv MASKANX_OPERATOR_TOKENS")
but scoped to one file. It affects nine. An autouse fixture in
tests/conftest.py clears it before every test; a test that wants a token
still gets one, since its own monkeypatch.setenv runs after and overrides
it. Confirmed narrowly scoped: adding only this one delenv brought the
suite from 28 failed back to 1212 passed with nothing left over — no need
to guess at a wider blast radius.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
You added funds and the UI kept showing "Balance: INR 0.00". Verified
directly against the live account: `balance` is 0 because that field means
amount *owed*, which is correctly nothing on a prepay account — the money
you loaded was never billed to you, so there is nothing to owe. The ₹50 was
real the whole time; MaskanX was just asking Meta the wrong question.
Meta only exposes the wallet amount through `funding_source_details`,
which we were not requesting, as a pre-formatted string:
"Available balance (₹50.00 INR)". There is no separate numeric field for
it — confirmed live, not assumed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
VITE_META_AD_ACCOUNT_ID has no default and no .env ships with the repo, so
on a fresh checkout it is empty and the Ad account card on the Campaigns
page just does not render — no error, no placeholder, nothing on screen
saying why the balance and spend are missing.
GET /campaigns/account returns whichever account META_ADS_ACCOUNT_ID names
on the backend, which was already required for every other campaign
feature to work at all. The frontend now asks for that instead of carrying
its own copy of the same setting, so the two cannot disagree silently.
Kept /campaigns/account/{id} for an explicit lookup.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Third failure in the same chain, one object further each time: "Performance
goal isn't available: You can't use the selected performance goal with your
campaign objective" (code 100, subcode 2490408).
DEFAULT_OPTIMIZATION_GOAL was a single global constant, LEAD_GENERATION,
applied whatever the objective was. That is correct for OUTCOME_LEADS and
wrong for every other objective Meta offers — including OUTCOME_TRAFFIC,
which is what the connection test builds.
Defaults now come from a per-objective table, and an unknown objective
falls back to LINK_CLICKS: valid for the widest range of objectives, where
a lead goal is valid for exactly one. An explicit advanced.optimization_goal
still wins, and is still checked against the allowlist.
Deliberately not validating goal-against-objective beyond the defaults.
Meta's full compatibility matrix is not something this code can assert
confidently, and a wrong matrix would reject setups that actually work —
worse than the failure it would prevent, which sync already surfaces
legibly now.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Refusing outright was too blunt. A failed connection test leaves a
campaign in the ad account; the reconciler adopts it as "imported"; and
from that moment nobody can remove it from MaskanX at all — not even the
account owner, and not even though MaskanX created it. The only route left
was Ads Manager, which is the thing this whole feature set exists to avoid.
The guard was protecting against an accidental click, so that is all it
does now. delete_in_meta=true is required for an imported campaign,
carries an operator identity, and is logged with the Meta id. The 409 that
refuses without it names the flag, so the refusal points somewhere instead
of stranding the caller.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The first real connection test left a live campaign in the ad account.
sync_campaign assigns meta_campaign_id on the spec it was handed before it
creates the ad set, so when the ad set was rejected the exception escaped
with `synced` unbound and `created` still empty — and the cleanup keyed on
exactly those two. Nothing was deleted, and nothing said so.
The reconciler then found the unknown campaign and adopted it, which is
why /discover reported zero: by the time anyone looked, the orphan was a
known record. A leak that hides itself.
Cleanup now falls back to the spec's own meta_campaign_id. Two tests: a
mid-chain failure deletes the campaign, and a failure before any campaign
exists still issues no delete at all.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
With the campaign-level field fixed, the live run got one object further
and failed on the ad set: "Bid amount or bid constraints required for bid
strategy" (code 100, subcode 2490487). We were sending no bid_strategy at
all, and Graph requires one on every ad set that carries its own budget —
which, since MaskanX never uses campaign budget, is all of them.
LOWEST_COST_WITHOUT_CAP is the only strategy that needs nothing else from
us: COST_CAP and LOWEST_COST_WITH_BID_CAP need a bid amount, and
LOWEST_COST_WITH_MIN_ROAS needs bid constraints plus a VALUE optimisation
goal. CampaignSpec carries none of those, so the allowlist holds exactly
one value and build_ad_set_payload refuses the rest before any network
call — a rejection at sync time would land after the campaign object
already exists, leaving a half-built chain behind.
It also suits the guardrails: automatic bidding spends the daily budget
and never exceeds it, where the cap strategies bound unit price rather
than total spend.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The connection test finally said what was wrong: "Must specify True or
False in is_adset_budget_sharing_enabled ... if you are not using campaign
budget." Code 100, subcode 4834011 — the same failure that has been
blocking a live sync, previously reported only as "Invalid parameter".
MaskanX always puts the budget on the ad set, so the campaign never
carries one and Graph never treats this field as optional. It was never
sent at all, so no campaign could be created in this account.
Defaulting to false, not true. True lets ad sets lend each other up to 20%
of their budget, which means an ad set can outspend the daily budget we
set for it — and validate_guardrails treats that number as a ceiling.
Predictable spend beats Meta's optimisation. Overridable per call.
Sent as the lowercase literal "false": form-encoding a Python bool would
put "True" on the wire, which Graph rejects.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Running the diagnostics against the real account returned exactly two
blockers — no payment method, and no operator token — and both of them
disabled the connection test, because the button was gated on `ready`.
That is backwards. The connection test creates PAUSED objects and deletes
them; Meta does not require a funding source for that, and an approval
identity has nothing to do with it. Gating on full readiness put the check
out of reach of the person who most needs it: someone with an unfunded
account trying to find out whether Meta accepts what MaskanX sends at all.
That question is the reason this feature exists — a live sync failed with
subcode 4834011 and nobody could tell why.
`can_smoke_test` reports the weaker condition, and `smoke_test_blocking`
names what would actually stop it, so the UI can say which check to fix
rather than "all of them".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Running the new diagnostics against the live account returned "No ad
account is configured. Set META_ADS_ACCOUNT_ID in Settings >
Environments." That instruction was wrong: the Settings page renders
_KEY_REGISTRY, and neither META_ADS_ACCOUNT_ID nor META_PAGE_ID was in it,
so there was no field to type either one into. Arbitrary keys can be
stored, but only listed ones are shown, which left the operator holding an
instruction they could not follow anywhere but a terminal — the exact
thing the panel exists to avoid.
The test reads the remedy strings out of diagnostics.py and asserts every
env var they name is in the registry, so a future check that mentions a
new key cannot ship pointing at a page that does not offer it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A live sync against the real account failed with Meta code 100, subcode
4834011, and the only thing we stored was the literal string "Invalid
parameter". Meta had written an explanation — it puts one in
error_user_title and error_user_msg — and MetaError was discarding it.
Every explanatory field is now kept, describe() prefers the text written
for a person, and as_dict() carries the lot into the API response and the
stored sync_error.
That still left diagnosis needing a terminal, which is no good: the person
who has to attach a payment method or fix a Page id is a client, not a
developer. GET /campaigns/diagnostics runs seven ordered readiness checks
and returns a remedy with each failure. It stops at the first hard failure
— with no token every later check fails for the same reason, and five
identical errors hide the one that matters.
POST /campaigns/diagnostics/smoke-test closes the remaining gap: the unit
suite proves we send what we think we send, not that Meta accepts it. It
builds the real chain, reads back from Graph that every object is PAUSED,
and deletes in a finally. If cleanup fails, the ids come back in the
response rather than being abandoned in a real ad account.
Both routes are declared above /{campaign_id}. /diagnostics is a single
path segment, so the wildcard would otherwise answer it — the same
shadowing /discover and /adopt are already guarded against. Four tests go
through HTTP to pin the ordering, because the rest of this file's tests
call the functions directly and would pass against an unreachable
endpoint.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Setup, first campaign, daily use, and the chat prompts — including the
MCP path for adding CRM fields from a conversation.
Records the two things currently blocking a live launch: the ad account
has no payment method, which only Business Manager can fix, and the
operator token has to be set before an approval can authorise spend.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Guardrails halt a campaign that exceeds its own limits, deterministically
rather than through the LLM cron. Insights are stored and served as a
dashboard. Leads and campaign figures reach Maskan CRM server-to-server,
and MCP can add CRM fields from a conversation.
Review of these phases found and fixed a critical persistence bug that
predates them — create_campaign never wrote meta_campaign_id, which made
adopt duplicate and the reconciler re-import the same campaign on every
cycle — plus a spend figure reported a hundredfold to the CRM, and a lead
sync that re-sent a week of leads every five minutes.
Every lead from the last seven days was re-sent on every cycle. It was
correct — the CRM deduplicates on Meta's lead id — but at a five-minute
interval a campaign with fifty leads a week meant fourteen thousand CRM
requests a day to re-send leads the CRM already had.
Each campaign now records how far its leads have been sent, and the next
run reads from there minus an hour. The overlap matters: Meta filters on
creation time, so without it a lead created moments before a run started
falls between two runs and is never sent.
The cursor advances only after every lead in the batch has been accepted,
so a run that fails half-way is retried in full by the next one — the
self-healing the naive version had, without the traffic. Nothing is
written when there were no leads, since that would mean an event row per
campaign every five minutes.
Also batches insight upserts into one executemany. A single campaign sync
writes a month of days times six breakdowns, which was a few hundred
round trips per campaign per cycle.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two tools on the maskan_crm server: one to list the fields that exist, one
to define a new one. Asking in MaskanX chat for a field now creates it in
the CRM, where it is usable on the next record and visible in the CRM's
own screens.
The list tool exists mainly so the create tool has something to check
against — without it an agent invents a near-duplicate of a field that is
already there under a slightly different name, and its description says
so.
Nulls are stripped from the definition before it is sent, so the CRM's
defaults apply rather than being overwritten with None. A rejected
definition comes back as the CRM's own message, which is what lets an
agent correct itself and retry rather than reporting a constraint
violation to the user.
_request now accepts a list response behind an explicit flag. Every
endpoint returns an object except the collection reads, and an unexpected
array is more likely a proxy's error page than data.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Server to server over the CRM's integration API, deliberately not through
MCP. MCP is for an agent deciding to do something; leads have to reach the
CRM on a schedule whether or not anyone is talking to the agent, and a
lead that arrives only when someone asks for it arrives too late.
Leads are re-sent for a trailing window on every run rather than tracked
as new-since-last-time. Meta's lead id is the external id, so the CRM
ignores one it already has — which makes a half-failed run heal itself on
the next cycle with no bookkeeping.
Meta returns form answers as a list under names the form's author chose,
so mapping is best-effort against aliases: full_name or first/last, phone
or phone_number or mobile. Unrecognised answers are kept in metadata
rather than dropped, and a lead with a phone but no name still gets
through — the CRM requires a first name, and losing a real enquiry to
satisfy a validator would be the wrong trade.
Campaign figures come from stored insights, not Meta: this runs every few
minutes and re-reading Meta would spend the quota on numbers that change
hourly. They go through an upsert rather than the idempotent create used
for leads, because a campaign's figures change every time they are read.
One care point, learnt the hard way and now commented and tested: stored
metrics are already in minor units, so summing them with the raw-row
normaliser reports spend a hundredfold.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Covers the three things most likely to be broken by a well-meant change:
insights are re-fetched and upserted rather than appended, breakdown rows
duplicate the spend they break down and must be filtered out of
aggregates, and both dashboard windows end yesterday because today is
partial.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
create_campaign never wrote meta_campaign_id, sync_status, sync_error or
approved_by. An adopted campaign arrives already linked to a Meta object,
and the link was dropped on the way into the database.
The consequences compounded. Every lookup keyed on meta_campaign_id
missed the row, so adopting a campaign twice created a second copy
instead of returning the first, and the reconciler — which imports any
Meta campaign it cannot find locally — imported the same campaign again
on every cycle. At the default 120s interval that is a new row every two
minutes, indefinitely.
The unit tests could not have caught this: their fake repository stores
the spec object itself, so no field can be lost between write and read.
It took a real database. The two regression tests added here run against
one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds the insights reader, a pure guardrail evaluator, and the sweep that
applies them to live campaigns.
Three things carry the weight here:
Units. Meta reports spend in major units ("12.34") while budgets and
guardrails are in minor units (1234). normalise_row converts, rounding
half up rather than using round(), which rounds halves to even and can
record an exact half-unit of spend as nothing.
Leads. Meta has no leads field; leads live in the actions array, under
several action types depending on whether the lead came from a Facebook
form or a pixel. Cost per lead is computed from spend and leads over the
same window rather than read from cost_per_action_type, so the two can
never disagree.
Order. The campaign is paused on Meta before the local record changes. A
campaign recorded as paused but still delivering is the outcome this
exists to prevent. One campaign's failure never aborts the sweep, so a
rate limit on the third does not leave the fourth unguarded.
Cost rules are skipped until the first lead or click: no leads yet is not
an infinite cost per lead, and pausing for that would kill every campaign
in its first hour.
Deliberately a deterministic loop rather than a maskanx_cron_jobs entry.
That scheduler runs prompts through an agent, and asking a language model
whether a budget has been exceeded would make an arithmetic guarantee
probabilistic. Follows reconcile.py's lifespan-task pattern instead, and
warns every cycle if Meta is unconfigured — a safety system that cannot
run should be loud.
require_approval_for_budget_increase is enforced at the API layer, where
the budget is actually edited: raising it while a campaign awaits approval
returns 409, since it would change what the approver is reviewing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Campaigns now reach Meta. Sync builds the campaign/ad set/creative/ad
chain with every object PAUSED; launch is the only action that starts
spend and is gated on an identified operator and a funding source.
Campaigns authored in Ads Manager can be discovered and adopted, and a
background loop keeps local status in step with Meta.
Review of the phase found three defects, fixed here: launch activated
only the campaign so nothing delivered, POST payloads went in the query
string so image uploads could not work, and an ad set with no targeting
reached Meta and failed opaquely.
Every write went out as a query string. Graph accepts that for small
payloads, but upload_ad_image base64-encodes an entire image file, and no
real image fits in a URL — image uploads could not have worked.
POSTs now use a form body and get a longer timeout, since an upload is the
one Meta call that carries real weight. Reads and deletes keep their
parameters in the query string, where Graph expects them.
Adds the first tests over the default transport: every other Meta test
injects a fake one, so nothing covered this path.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Graph requires targeting on every ad set. An empty targeting dict reached
Meta and failed opaquely part-way into building the object chain, leaving
a campaign and nothing else. This names the missing field before any
network call, matching how the module already handles a missing objective
and a missing daily budget.
The wizard defaults age and countries, so this only fires on campaigns
built through the API without targeting.
Also fixes the live smoke test, which used Graph's nested geo_locations
shape rather than the flat spec.targeting["countries"] the mapper reads.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Launch set only the campaign to ACTIVE. Sync creates the ad set and ad
PAUSED, and Meta delivers only when the ad, its ad set and its campaign
are all active — so Launch reported "live" while nothing ran. This was
the central promise of Phase 2 and it did not work.
The chain is now activated children-first, campaign last. Nothing under a
paused campaign delivers, so a failure part-way leaves the campaign unable
to spend. That ordering is also why pause and stop only flip the campaign.
An imported campaign has no stored ad set or ad ids, so launching it still
touches only the campaign and its children keep the statuses set in Ads
Manager.
Deleting an imported campaign is now refused rather than silently
pointless: deleting it on Meta would destroy work MaskanX did not author,
and deleting only the local row achieved nothing because the reconciler
re-imported it on the next cycle. Deleting it in Ads Manager is what
sticks, after which the reconciler archives the record.
Also corrects the README's reconciler interval: it is 120s, not 300s.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The README still described Phase 1's "creates nothing in Meta" contract,
which is no longer true. It now covers the lifecycle, that sync creates
only paused objects and why that is enforced in three places, what launch
requires, adopt/delete semantics for imported campaigns, and the
reconciler interval.
Also documents the separate campaign_test database. Without it the
repository integration tests skip and the suite still passes, so the skip
count matters as much as the exit code.
The live smoke test is the only thing that proves Meta accepts what the
client sends: every other test runs against a fake transport. It syncs a
real campaign, reads back from Graph that all three objects are PAUSED,
and deletes them in a finally block. Opt-in via MASKANX_LIVE_TESTS=1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deleting a campaign only removed the local row. A live campaign deleted
in MaskanX kept spending on Meta with nothing left here recording that it
existed. Meta is now deleted first, and a failure there keeps the local
row and answers 502, since forgetting it here while it still spends there
is the worse of the two outcomes.
An imported campaign is never deleted on Meta. MaskanX did not author it,
and forgetting the import must not destroy work done in Ads Manager.
Deleting a campaign cascades to its ad sets and ads, so only the campaign
id is sent. The creative is left behind deliberately: it is an
account-level asset that other ads may reference.
Meta reports a delete of an absent object as code 100, which delete_object
swallows, so retrying a partially-failed cleanup is safe.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Meta publishes no webhooks for campaign create or delete, so this polls every
120s by default (MASKANX_CAMPAIGN_RECONCILE_SECONDS; 0 disables).
Meta is authoritative for delivery state; MaskanX keeps its own metadata.
Guardrails, approvals and audit history are never touched by reconciliation,
and campaigns still in a local-only status are skipped entirely so a draft
that has never reached Meta cannot be overwritten.
A campaign that disappears from Meta is archived, not deleted: its spend
history has to stay reportable.
The loop swallows and logs a failed cycle rather than dying, and is cancelled
on shutdown alongside the watchdog. It only starts when META_ADS_ACCOUNT_ID
is set.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Campaigns can be authored on either side. Anything MaskanX did not create is
origin="imported": MaskanX reports on it and can pause or stop it, but it was
built elsewhere.
Adoption is keyed on meta_campaign_id, which carries a unique index, so
adopting the same campaign twice returns the existing record instead of
violating the constraint. Discover excludes anything already adopted.
The literal /discover and /adopt routes are declared before /{campaign_id} so
they cannot be captured as campaign ids; a test pins that ordering.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Launch is the only action that starts real spend, so it refuses unless every
precondition holds: the campaign is synced, its status allows launching, an
identified operator approved it, and the ad account has a payment method.
Each refusal names what to do rather than letting Meta fail opaquely later.
An approval recorded as "unauthenticated" does not authorise spend. With
MASKANX_OPERATOR_TOKENS unset every approval is unattributable, so launch is
blocked until operator auth is configured.
Stop and pause set the Meta object PAUSED before recording the local change,
so a Meta failure cannot leave a campaign that is stopped in MaskanX but
still delivering.
Replaces the Phase 1 test asserting /launch did not exist with one asserting
it is unreachable from draft; the guarded property is unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Creates the campaign, ad set, creative and ad chain, every object PAUSED.
Nothing here starts delivery; Launch is a separate explicit action.
Each Meta id is persisted as soon as it is obtained, so a failure part-way
through is resumable: a retry reuses the stored ids and creates only what is
still missing. Without that, retrying a partial failure would create a second
set of ad objects in a real advertising account.
Page id and ad account are resolved before any network call, so missing
configuration cannot leave a half-built chain in the account. A Meta failure
records sync_status=failed with Meta's code and subcode, leaves the status at
approved, and keeps the ids already obtained.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Task 4 review: Approved with no Critical findings, but three Important
and two hardening items. All five addressed:
- objects.build_campaign_payload: normalise a bare string
special_ad_categories value to a single-element list instead of
exploding it into characters via list() - the field controls
regulated-advertising compliance and advanced is unvalidated input.
- objects.build_ad_set_payload: validate optimization_goal and
billing_event against new allowlists before forwarding them, since
billing_event determines how the ad account is charged and both come
from client-controlled advanced.
- validation.validate_budget: reject a lifetime-only budget (Meta's
create_ad_set only accepts daily_budget) so it fails at validation
time instead of passing approval and only failing at sync.
- client.py: create_campaign/create_ad_set/create_ad now send the
CREATE_STATUS constant rather than the caller's status object, so a
str subclass with a lying __ne__ can no longer slip "ACTIVE" past the
guard.
- client._post: refuse any POST to a campaign/adset/ad creation path
with a non-PAUSED status before touching the transport, as defence in
depth if a future caller bypasses the typed create_* guards.
10 new tests (8 in test_meta_writes.py, 2 in test_campaign_validation.py).
Adds the write half of the Meta Graph API client: create_campaign,
create_ad_set, upload_ad_image, create_ad_creative, create_ad,
update_object_status, and list_campaigns/list_ad_sets/list_ads. Every
create_* helper hard-codes status="PAUSED" and raises ValueError with
zero network calls if asked for anything else; update_object_status is
the only method allowed to send ACTIVE. Dict params Graph expects as
JSON strings (targeting, object_story_spec, special_ad_categories) are
json.dumps-encoded before being sent.
Also adds src/adclaw/meta/objects.py with pure functions mapping a
CampaignSpec's budget/targeting/advanced fields onto Graph parameter
names, keeping that mapping unit-testable without a transport.
26 new tests in tests/test_meta_writes.py, all against a FakeTransport
(no live network calls, no real Meta objects created).
Phase 1 covered only the pure row mapper, so every SQL path in
CampaignRepository was untested — parameter ordering, RETURNING clauses and
transaction behaviour had never been executed. Phase 2 creates real Meta
objects on top of this layer, so the gap is closed first.
Seven tests cover the CRUD round trip, LookupError on unknown ids, event
recording, and that update_campaign_with_event writes both rows or neither.
They skip rather than fail when PostgreSQL is unreachable, and delete every
row they create in a finally block.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
approved_by was client-supplied, making approval forgeable ahead of
Phase 2 pushing real spend to Meta. Add a proportionate operator-token
check (no login system) via a new require_operator FastAPI dependency:
MASKANX_OPERATOR_TOKENS maps name:token pairs, and approve_campaign now
derives its actor solely from the resolved operator, never from the
client-supplied ActorPayload.actor. Unset env resolves to
"unauthenticated" so local dev and existing tests are not blocked;
set-but-unrecognised tokens 401. submit and reject remain
unauthenticated since neither authorises spend.
Updates test_submit_then_approve_moves_through_states to assert
approved_by == "unauthenticated" (env var unset in tests) instead of
the previously-trusted client actor, since a forged actor must now be
ignored.
_transition previously called update_campaign then add_event as two
separate commits, so a failed add_event left a status change with no
audit row. Phase 2 launches campaigns that authorise spend, so a
compliance hole like that has to close first.
Add CampaignRepository.update_campaign_with_event, which runs the
UPDATE and the event INSERT on one connection inside one transaction
and commits once. Route campaign transitions (submit/approve/reject)
through it instead of the update_campaign + add_event pair; update_campaign
and add_event themselves are unchanged for other callers.
Also update test_campaign_api.py's FakeRepo/monkeypatch to implement the
new repository method, since production code now calls it in the
submit/approve/reject path.
Campaign records, approval workflow, budget guardrails against the ad
account minimum, and Meta ad preview. Creates nothing in Meta.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Records what Phase 1 does and deliberately does not do: no Meta writes, no
launch or sync endpoint, previews rendered without creating objects, budget
minimums enforced locally, and the fact that a payment method can only be
attached in Meta Business Manager.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PUT /campaigns/{id} validated the raw request payload instead of the
merged campaign state, so omitting ad_account_id skipped the minimum
check and a full model_dump() overwrote every unset field (targeting,
guardrails, objective, etc.) with its default, silently destroying
data. Validate against the effective post-merge budget/guardrails/
account and only apply fields the client actually sent
(exclude_unset=True).
Also: persist company_id on UPDATE (was accepted but dropped), log
a warning when Meta account lookups fail so the budget-minimum
fail-open is observable, reword the non-editable-status 409 message
to not prescribe an impossible reject-to-draft action, and add
coverage for Meta error surfacing, LookupError-to-404 on PUT/submit,
reject's pending_approval->draft transition, and GET-unknown-id 404.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
create_campaign/update_campaign previously committed then opened a
second connection via get_campaign() just to re-read the row they had
just written, adding an extra round-trip and a race window where a
concurrent delete between commit and re-fetch was misreported as
"disappeared immediately after insert/update". Both now RETURNING the
row from the same statement/cursor that wrote it, using a shared
_RETURNING_CLAUSE derived from _SELECT_COLUMNS so the two column lists
cannot drift apart. update_campaign now raises LookupError for an
unknown id (0 rows affected) instead of misdiagnosing it as a race.
Also: delete_campaign uses the (cur.rowcount or 0) > 0 idiom already
established in postgres_repo.py, the module docstring documents that
this repository's SQL paths rely on integration tests rather than
unit tests, and a new DB-free test asserts the RETURNING and SELECT
column lists stay identical.
Adds CampaignRepository with list/get/create/update/delete for
campaigns plus add_event/list_events for the audit trail, following
the existing postgres_repo.py connection pattern. Replaces the
brief's two bare `assert created/updated is not None` checks with
explicit RuntimeError raises so the guard survives python -O.
Address code review findings on the read-only Meta Graph client: add
monkeypatch-based tests for access_token_from_env/MetaNotConfiguredError,
assert generate_previews JSON-encodes the creative param, pin the
existing fail-fast-on-partial-error contract with a dedicated test, and
declare httpx as an explicit dependency since it was only resolving
transitively.
