Repository navigation
Claude/nueva plataforma superpowers o3r31f - #8
Merged
helgarpalmieri55 merged 47 commits intoAug 20, 2026
Merged
Conversation
…able First slice of P4 (docs/SPEC.md §7). Deliberately the tool layer ONLY — no Anthropic call, no conversation loop, no HTTP surface. Those land next. Doing it in this order is the point: every security property of the agent lives here, in code that a database can verify, rather than in a prompt. The model's job is to decide WHICH tool to call; what a tool will hand back is settled before a model is ever involved. ## What each executor guarantees - **Tenant-scoped, always.** Every read goes through `tenantDb(tenantId)`, where the tenant comes from the conversation, never from model output. A tool input naming another store's product finds nothing. - **Input is parsed, not trusted.** Tool inputs are model-generated text reacting to whatever a shopper typed, so each executor runs its Zod schema before it queries — the same posture an HTTP controller takes with a body. - **Errors are returned, not thrown.** A thrown exception would kill the turn; an error result lets the model say "no pude consultar eso" in its own words. None of them carry a database message. - **Results are narrow plain objects**, not Prisma rows — so a column added later (a cost price, an internal note) cannot silently become something the agent can recite. ## Order lookups share one implementation with the tracking endpoint `get_order_status` delegates to a new `OrderTrackingService`, extracted from `order-tracking.controller.ts`. That lookup was correctly inlined while it had one caller; it now has two, and what it encodes is a security rule rather than a query — the double factor, and the requirement that "no such order" and "real order, wrong contact" stay indistinguishable. Two copies of that is exactly how one of them quietly stops matching. The agent surface makes it sharper, since the tool is driven by chat input. The tool returns city and department but never the street address, email or phone — SPEC.md §7's rule, and worth more here than on the order page because a transcript is a leakier surface. ## Other deliberate choices - `recommend_products` reranks deterministically (in-stock first) rather than making a second model call: the ranking a shopper sees should be reproducible, and every row is real and current-priced. - `create_cart_link` re-validates stock at build time rather than trusting an earlier search result — a conversation runs for minutes, and a cart built around something that sold out sends the shopper to a checkout that fails. It creates a FRESH cart rather than merging into whatever they already had. `source: 'agent'` is what powers the "ventas asistidas por IA" KPI. - `get_store_info` returns an explicit "not published yet" instead of an empty string, so the model says so rather than inventing a returns policy. - `get_product` refuses non-active products, so the agent cannot describe and quote something that cannot be ordered. 18 tests. Mutation-tested on the two properties that matter most: leaking the full address into the result fails the PII test, and swapping `tenantDb` for `platformDb` fails the cross-tenant test. Full gate 27/27. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
…dpoint
Completes the server side of SPEC §7. P4a-1 built the tools; this drives
them from a model and puts the whole thing behind an HTTP surface.
The loop (agent.service.ts) loads the last 20 messages, calls Claude with
the tenant's tool definitions, executes every tool server-side against the
CONVERSATION's tenant — never one the model names — and stops after four
model round-trips so a model that keeps calling tools cannot bill a
merchant unboundedly for one question. The Anthropic client is a Nest
provider, so the 12 loop tests drive the real code path with a fake and no
API key.
Two limits, deliberately separate:
- The monthly plan cap (agent-budget.service.ts) is checked BEFORE the
model call and recorded after, so a turn that fails costs the merchant
nothing. It counts shopper turns, not API calls, because that is the
unit a merchant was sold. A tenant with no TenantLimits row is zero
budget, not unlimited — an unprovisioned store getting free unmetered
AI is the failure that costs money and hides itself. The AgentUsage
migration revokes tenant write access to the counter: a cap a tenant
can write to is not a cap.
- The per-conversation throttle (agent-throttle.service.ts) is Redis and
fails open, since it is abuse control rather than authorization. It
runs before anything is persisted, so a client stuck in a retry loop
costs neither a row nor a token. A repeated identical message inside
the cool-down gets the previous answer back rather than a refusal —
from the shopper's side a double-tapped send should just look like it
worked.
The endpoint offers the same turn as JSON or as SSE; both call one service
method, so tenant scoping, throttling and the cap are decided in exactly
one place. The stream carries turn-level events (conversation id first,
then each tool, then the text) rather than token deltas — enough for the
widget to show what the agent is doing, and shaped so swapping in
messages.stream() later does not change what a caller consumes.
REDIS_CLIENT moved out of AppModule into its own leaf module; a feature
module could not inject it from there without an import cycle.
Also wires the fourth rate limiter SPEC §9 asks for. It is the tightest of
the four (30/min per address) because it is the only surface where an
accepted request spends the MERCHANT's money.
Mutation-tested: disabling the budget cap fails 3 tests, the message limit
2, the duplicate cool-down 2. Full gate green — 608 API tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
…the agent's output dead
Building the widget surfaced two gaps in what P4a shipped. Both are fixed
here because either one alone makes the agent look like it works while
quietly doing nothing:
- `create_cart_link` returns `/carrito?c=<key>`, and the cart page
ignored the parameter entirely. Every basket the agent assembled
opened as the shopper's own empty cart. There is now a
`POST /v1/storefront/cart/adopt` that swaps the `ventia_cart` cookie
to that cart, restricted to carts the agent itself created — a URL
leaks far more readily than an HttpOnly cookie (history, referrers, a
forwarded WhatsApp message), and agent carts are the only ones a link
is ever emitted for, so the restriction costs nothing. Adopting leaves
the shopper's previous cart intact rather than merging or deleting it.
- `Order.source` was the literal `'web'`, so the "ventas asistidas por
IA" KPI SPEC §7 specifies would have read zero forever — a bug that
presents as "the agent isn't selling anything" rather than as a bug.
It now inherits the cart's source.
The widget streams from the SSE route through a same-origin proxy that
passes the upstream body straight through; the cart proxy next door
buffers with `await res.text()`, which for this endpoint would hold every
event until the turn finished and defeat streaming entirely.
Product cards and the cart button render from the TOOL results, never from
the model's prose. That is the point rather than a style choice: the model
is instructed never to invent a price, and rendering the number the tool
returned means a shopper cannot be shown a hallucinated one even if the
instruction fails.
The launcher only mounts for a store whose plan actually includes AI
messages, via two new hints on the tenant resolve. A store that can only
answer "no puedo responderte por chat" is better off not appearing to
offer chat. The hints are display-only — the hard cap is still
AgentBudgetService — and `agentConfig` exposes just the agent's name
publicly, never the merchant's own operating text.
Mutation-tested: hardcoding the order source fails 1 test, dropping the
agent-cart restriction 1, and parsing the SSE stream per-chunk instead of
buffering across chunk boundaries fails the split-frame test — the failure
that only ever reproduces once there is real latency.
Full gate green — 619 API tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
Closes P4. The agent could be configured only by editing a JSON column by hand, and nothing anywhere showed what it cost or what it earned. `PATCH /v1/admin/settings/agent` writes `Tenant.agentConfig` — the same blob `agent/system-prompt.ts` already reads. Merged field-by-field rather than replaced: the tab edits four fields independently, and a PATCH carrying only `tone` silently blanking a carefully written store summary is data loss a merchant notices weeks later. `tone` is an enum, not free text, because it is interpolated straight into the system prompt and an arbitrary string there is an instruction channel rather than a setting. The summaries are necessarily free text — describing your own store is the point — but bounded, since an unbounded field is somewhere to paste a replacement prompt and is also billed as input tokens on every turn for the rest of the month. `GET /v1/admin/agent/usage` answers the one question a merchant actually has about this feature — is it worth what it costs — so month-to-date message usage and AI-assisted sales come back together, over the same window. Readable by staff, unlike the settings routes: it carries no credentials and no customer data, and it is the same "how are we doing" information anyone handling orders has reason to see. Cancelled orders are excluded, since counting them would flatter the agent exactly when it is doing worst. The Asistente IA tab puts the numbers above the form, on the theory that a merchant tuning the tone is precisely the person who should see the meter, and a separate page is one nobody opens until the cap is already hit. It renders SPEC §7's two thresholds — the 90% warning and the 100% hard stop — with the cap winning, since a store at 100% is also past 90% and "te estás acercando" would be wrong there. Alert gains a `warning` variant for that threshold. Amber, not red: this is "act soon", not "something broke". Mutation-tested: dropping the agent-source filter or the cancelled-order exclusion each fails a test. Full gate green — 632 API tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
…ntee them SPEC names evals as part of the Phase 4 DoD. Its six scenarios divide into two kinds, and the split is the substance of this file. Four of them are really properties of the SYSTEM, not the model. "Never states a price that differs from the tool result" is "the model is never given a price that differs" plus "the widget renders the tool's number, not the prose" — both deterministic, and worth far more pinned than asked politely of a model, because they hold on a bad day, on a new model, and under a prompt injection in the shopper's own message. A budget is never exceeded because over-budget products are never returned at all; a mismatched order contact is refused with the same string as a nonexistent order, so the pair of answers cannot confirm which order numbers are real. These run on every `pnpm test`. The rest are genuinely about the model's judgment — declining an off-topic request in character, admitting out-of-stock gracefully — and cannot be asserted without calling the real API. They are behind AGENT_LIVE_EVALS=1 plus a real key, which is the fixtures-in-CI / manual-smoke split SPEC asks for. Eval 5 is deliberately absent rather than faked: `escalate_to_human` does not exist yet, and asserting the prompt's weaker "offer the contact details" fallback as though it were the real thing would make the checklist read complete when it is not. Mutation testing caught one of my own tests being worthless: the in-stock-first assertion searched "chaqueta de cuero", where the fixtures came back in the right order regardless of the rerank. It now asks for the SOLD-OUT jacket by name, so only the rerank can put the buyable one first — and removing the rerank now fails it. README documents the agent surface, the three independent limits, and how to run the live evals. Full gate green — 641 passed, 4 skipped (the live block). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
I had deferred this to P5 on the grounds that SPEC lists "Chatwoot escalation" there. Re-reading §11, that was wrong: P4's own DoD says "all seven tools" and "all six eval scenarios pass", so leaving it out left P4 red. What genuinely belongs to P5 is Chatwoot specifically — and Chatwoot is not the part a shopper's outcome depends on. So the tool ships now with email as the notifier. The ordering inside it is the design: the conversation is marked `escalated` first (a durable fact the merchant's admin reads, independent of any delivery), the merchant is emailed fire-and-forget (a mail transport briefly down must not become an error the shopper sees, since the record already exists), and the store's own contact details go back to the model so it can tell the shopper where to go RIGHT NOW rather than only that someone will be in touch. When Chatwoot lands it becomes a second notifier beside the email, not a replacement — an owner's inbox is a useful backstop for a conversation sitting unread in a helpdesk. Plan-gated on the `TenantLimits.humanHandoff` column that already existed. Gating is enforced three times over, and deliberately: the tool is filtered out of the array, rule 7 of the prompt changes to the contact-details fallback, and the executor refuses. A store without handoff whose agent promises a callback nobody will make is worse than one that says plainly it cannot transfer you. Calling it twice in one conversation notifies once. A confused model is not a reason to mail a merchant twice about the same customer. `AgentToolsService.execute` now takes a context object rather than a bare tenantId. escalate_to_human needs the conversation, and P5's WhatsApp channel will need to know which channel it is answering on; the tenant stays the one field that must never come from the model. Eval 5 is no longer absent from the suite — deterministic halves for the escalation and the plan gate, plus a live check that the model actually reaches for the tool when a shopper asks for a person. Mutation-tested: disabling the plan gate fails 2, dropping the tool filter 1, removing the already-escalated guard 1. Full gate green — 656 passed, 5 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
…promise
Shipping escalate_to_human exposed a gap I had created myself: the handoff
email told the merchant to open "la conversación en tu panel", and no such
panel existed. For a shopper who reaches the widget from a product page and
never leaves a phone number, that email said "someone needs help" and gave
no way to learn who, or what about. The tool was telling customers a person
would follow up while making it impossible for a person to.
So `/v1/admin/conversations` — list, transcript, and a resolve action — plus
the admin page the email now deep-links into via `?c=<id>`, landing the
merchant on the exact conversation instead of a list to search.
Details that are decisions rather than defaults:
- An unrecognised `status` filter returns everything rather than nothing.
A typo yielding an empty list reads as "you have no escalations", which
is the one wrong answer this page must never give.
- `escalatedCount` is always unfiltered, so "3 sin atender" stays true
while the merchant is looking at some other view.
- `resolve` exists because without it the escalated list only grows, and a
merchant cannot tell yesterday's answered case from this morning's
unanswered one — the list becomes useless exactly as it gets long enough
to matter.
- `tool` rows are excluded from the transcript. They record which tools
ran, which is debugging, and their raw output would bury the actual
conversation a merchant came to read.
- Both roles, matching orders and payment alerts and the API's own missing
@roles(): answering a handed-over shopper is fulfilment work, and staff
are the people most likely to be doing it.
Full gate green — 668 passed, 5 skipped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
SPEC §11's P4 DoD names three things. Two were already verified; the third — "an agent-created cart converts to an order flagged source=agent" — only had its links tested individually. The tool writes source: 'agent', the adopt endpoint hands a cart over, checkout inherits the cart's source: all three pass while the chain is broken, because the adopt endpoint could hand over one cart and the checkout read another and nothing would notice. The KPI would just read zero. So this drives the whole path the way a shopper does — tool call, then two HTTP requests carrying the cookie the server itself issued (taken from the Set-Cookie header rather than reusing the key variable, which would hide a server that set the cookie to something else) — and ends on the merchant's usage endpoint, since attribution is only worth anything if the number they read moves. Writing it surfaced a money bug that is NOT mine and is NOT fixed here: `taxCents` is computed as the IVA contained in a tax-inclusive price (`x - x/(1+r)`) and then added to a subtotal that already includes it, so a $180.000 product is charged $208.739. cart.service.ts and the storefront cart page do the same, so the shopper sees the inflated figure before checkout too. It predates P4, changing it is a product decision about whether catalogue prices are gross or net, and many shipped tests encode the current arithmetic. The DoD test asserts the totals RELATIONSHIP rather than a literal specifically so it does not bless the behaviour. Full gate green — 670 passed, 5 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
I flagged this last turn as needing a product decision. It doesn't — SPEC.md §5 already decided it: > Totals: Colombian retail convention — **prices include IVA**. Order > stores the tax breakdown per line derived from each product's tax rate > (`price_cents - price_cents / (1 + rate)` for the tax portion). So `taxCents` is the IVA CONTAINED IN `subtotalCents`, recorded for the DIAN breakdown. `checkout.service.ts` added it on top anyway. A $180.000 basket at 19% was billed $208.739 — the shopper paid the IVA inside the sticker price, then paid it again. Wrong since P2b, and visible to the shopper the whole way: the cart drawer, the cart page and the checkout page each computed and displayed `subtotal + tax + shipping` too, so the quoted number matched the charged one and neither was the price on the product page. All four now show "IVA incluido" — a breakdown line, not a charge — and the admin order detail is relabelled to match, since "Subtotal / IVA / Envío / Total" read as a column that no longer adds up. Why no test caught it: the happy-path checkout test computes its expected total the same way the code does, so both were wrong together and agreed. The new `prices include IVA (SPEC §5)` block states the rule instead — sticker price plus shipping, full stop — and pins the arithmetic signature of an included tax (7.329 on 45.900, not 8.721). Re-introducing the bug now fails four tests across two files. Found by the P4 DoD test, which hand-computed a total and disagreed with the code by 57.479 pesos. I had assumed my arithmetic was wrong. Full gate green — 673 passed, 5 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
The first slice of the WhatsApp channel: wire formats only, no DB, no Nest.
Shaped like @ventia/payments — two adapters behind one interface, injected
fetch, so every behaviour below is asserted without a network.
SPEC §4 says never implement a gateway or WhatsApp API from memory, so the
Cloud adapter was built against the payload-examples, webhooks-getting-
started and messages reference pages read at implementation time, and the
Evolution adapter against its v2 send endpoint and messages.upsert event.
Each file cites what it was read from. What is NOT claimed: no Meta app or
Evolution instance existed here, so the fixtures are the docs' own example
payloads rather than captured traffic — "parses what Meta documents", not
"parses what Meta sends".
The design note (docs/superpowers/specs/2026-08-18-p5-whatsapp-design.md)
covers why routing cannot use the URL: Meta delivers ONE webhook per app
covering every number under it, so the tenant is only knowable from
phone_number_id inside the payload. That is why P5b needs a table with a
globally-unique routing key rather than another key under Tenant.settings.
Four things here are security properties rather than parsing, and each is
mutation-tested:
- Cloud fails CLOSED with no app secret. Meta documents signature
validation as optional, which is true of the protocol and false of this
system: phone_number_id appears in every delivery and is not secret, so
an unverified path lets anyone who learns one spend a merchant's AI
budget.
- Cloud re-checks phone_number_id against the config it was handed. One
app's URL receives every tenant's traffic; skipping this is one store
answering another store's customer.
- Evolution requires the instance apikey — it signs nothing per payload,
so that header is the whole of its authentication.
- Evolution never answers fromMe. Its own outbound echoes come back
through the same event, and answering one puts the agent in an unbounded
conversation with itself on the merchant's budget.
Also: group chats are ignored (a store's reply would be visible to everyone
in one), and both Evolution payload nestings are accepted because its
published examples disagree with each other and picking one would mean a
channel that looks wired up and silently drops every message.
33 tests. Full lint/typecheck/build green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
The channel end to end on the server side: a delivery arrives, is verified, routed to a tenant, answered by the agent that already existed, and sent back as text. Routing is the whole design problem. Every other webhook here carries the tenant in its path; Meta delivers ONE webhook per app covering every number under it, so `/webhooks/whatsapp/:provider` has no tenant and `phone_number_id` inside an unauthenticated payload is the only discriminator. `WhatsAppNumber.externalId` is therefore globally unique — a database-enforced safety property, not tidiness: the lookup must have exactly one answer, and two tenants claiming one id means a shopper reaching another store's agent. There is a test that signs a valid delivery for the OTHER store's number and proves it lands there and not here. The controller acks before it works. Both providers retry on non-2xx, so a slow model call would become a redelivery — and a redelivery, absent dedupe, is a second billed turn. Dedupe exists anyway (`Message.externalId`, unique per tenant): "we ack quickly" is a mitigation, a unique index is a guarantee. The consequence is that nothing after the ack can report an error to anyone, so the inbound handler never throws. Secrets are unreachable from tenant-scoped code by construction: `ventia_app` gets a COLUMN-LEVEL SELECT grant omitting `credentialsEnc` and `verifyToken`, with no write privileges at all. An unprojected `tenantDb(...).whatsAppNumber.findMany()` fails with 42501 rather than quietly loading a Meta access token into memory. RLS is ENABLE, not FORCE, and that is load-bearing rather than conventional: the routing lookup runs on `platformDb` and is inherently pre-tenant — resolving `externalId → tenantId` is HOW the tenant becomes known. Under FORCE the policy evaluates against a null `app.tenant_id`, matches zero rows, and every inbound message on the platform is silently dropped as "unknown number", in production only. The plan gate is re-checked on every inbound message, not just at connect time. A downgraded store keeps its number registered and Meta keeps delivering to it. Rendering is text-first per SPEC §7, and keeps the widget's guarantee: every price, name and link comes from the tool results, never from the model's prose — so a hallucinated price cannot reach a shopper even if the prompt rule fails. The rate limiter deliberately does not cover this route: its tenant is unknowable before the body is read, and the only key available is a Meta edge IP shared by every tenant, so limiting on it would let one busy store throttle everyone. What bounds it instead is documented in main.ts — signature verification before any real work, retry dedupe, the per-conversation throttle, the monthly budget. 46 tests across the two new suites; db suite 33. Adapter-level mutation testing in P5a, renderer mutation testing here — including one renderer test that initially survived its mutation (an ok:false fixture with no payload proved nothing) and was fixed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
…to drop it `WebhookEvent_tenantId_orderId_idx` has existed in the database since 20260815160000_webhook_event_order_id but was never declared in schema.prisma. Prisma diffs the schema against the migration history, so every `migrate dev` / `migrate diff` emitted a spurious `DROP INDEX "WebhookEvent_tenantId_orderId_idx"`. That is worse than untidy. Accepting the diff silently drops a real index in production — the one backing "what did the gateway ever tell us about THIS order?", the natural support query once the link exists. Rejecting it means re-rejecting it on every subsequent migration until someone stops reading the diffs. Declaration only; the index already exists, so there is no migration and no DDL to run. Verified both directions against a throwaway shadow database: with the declaration `migrate diff` reports an empty migration, without it the DROP reappears. Predates the WhatsApp work — found while adding those tables, confirmed present at HEAD before them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
The admin side: connect a number, see its status, enable/disable, disconnect. Provider-discriminated form, credentials never rendered back, and the callback URL surfaced with its mandatory `?number=` param in three places — a merchant pasting that URL without the param fails Meta's handshake every time, which is the likeliest support ticket this feature has. The more important half of this commit is a bug mutation testing found in what I shipped last commit. `Message.externalId` had a unique index (P5b) and an `isDuplicate` check reading it, and nothing anywhere WROTE it. The column was always NULL, so the check could never fire — WhatsApp retry dedupe did not exist. The test covering it passed anyway, for an unrelated reason: the agent's repeated-identical-message cool-down was swallowing the second delivery, and a retry carries the same text by definition. Delete the dedupe, the test still went green. `AgentService.respond` now takes and persists `externalId`, so the unique index is reached. The test asserts what actually matters — that the MODEL was not called a second time, not merely that one row exists, since the index would reject the second insert AFTER a call the merchant has already paid for. It clears the cool-down between deliveries so the only thing that can absorb the retry is the mechanism under test, and it covers a genuinely new message getting through, because a dedupe that eats real follow-ups is worse than none. Following that through corrected a claim I had made in a comment: removing `isDuplicate` fails nothing, because `respond` inserts the user row before the model call and the index stops it there. The pre-check is cheap avoidance, not the guarantee. Both are documented as what they are now. Verified by mutation: dropping the plan re-check, the disabled-number check, or `setStatus`'s tenant scoping each fails a test; so does making the insert stop recording `externalId`. Full gate green — 720 API tests, 5 skipped; 177 admin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
SPEC §7 describes `escalate_to_human` as creating a Chatwoot conversation with the summary. P4d shipped the half that a shopper's outcome actually depends on — mark the conversation escalated, email the merchant a deep link to the transcript — and deferred Chatwoot to this phase. It joins that path rather than replacing it. A helpdesk is where a team works a queue, but an owner's inbox is a useful backstop for a conversation sitting unread in one, and dropping the email would make the feature worse for exactly the single-operator stores this product sells to. Optional by construction: a tenant with no Chatwoot config escalates exactly as before, and a HALF-configured one counts as unconfigured — a token with no inbox would open conversations nowhere, so falling back to email beats half-delivering. Fire-and-forget, like the email. The conversation is already marked escalated and the merchant already has a notice, so a Chatwoot outage must never become a shopper being told nobody can help them. The summary is posted `private: true`. That is the single most damaging thing here to get wrong: it is internal context — it literally says "3 intentos sin resolver" — and a non-private message goes straight to the customer. Mutation-tested, along with the half-config check. Not verified against a live Chatwoot: there is none in this repo's compose stack and none was available here. The request shapes follow the documented application API; treat them as unconfirmed until a real instance accepts one, same caveat as the WhatsApp adapters. 10 new tests; the 10 existing escalation tests still pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
SPEC §11 P6: "custom domains + on-demand TLS". A merchant's own `tienda.example.com` cannot be enumerated ahead of time, so Caddy issues certificates on demand — and `ask` is the entire security boundary around that. `GET /internal/tls-ask` refuses unless four things hold: the domain is registered, DNS-verified, owned by a LIVE tenant, and on a plan that includes custom domains. Each is separately mutation-tested, because each removal is a distinct way to hand out certificates for domains this platform does not serve. The damage from getting this wrong is not the obvious one. An attacker who points DNS at us and gets a certificate for their own domain has gained nothing they could not get anywhere. What they have done is spend the SHARED Let's Encrypt issuance budget — which blocks renewal for every real tenant. That is why the endpoint fails closed on every uncertainty, including a missing parameter, and why it must never 500: a 500 here is a certificate outage. Verification is DNS TXT, because a `TenantDomain` row proves nothing — a merchant can type a competitor's hostname into the form, and serving it would hand them that domain's traffic the moment its DNS pointed here. The token is derived, not random, so a merchant who reloads mid-setup is not handed a different value than the one already pasted into their DNS panel; and it is HMAC'd over a server secret so knowing the domain is not enough to compute it. `normalizeDomain` rejects wildcards, single labels and leading hyphens. That strictness is load-bearing rather than fussy: the normalized value is compared against a database column to decide issuance, so a loose normalizer is a way to match a row you should not. The Caddyfile carries the production shape, commented, with Caddy's own rate-limit as an independent second bound — on-demand TLS needs a real public hostname and a reachable ACME endpoint, neither of which exists in the dev stack, so shipping it live here would be untestable theatre. 19 tests; mutation-testing all four gate conditions plus the normalizer fails 5. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
SPEC §9: "audit log on all admin/platform mutations." Thirteen files call
`writeAudit`. `orders.controller.ts` and `orders.service.ts` called it zero
times, which left order state as the ONE consequential merchant action with
no trail.
That is the wrong thing to have missed. A transition moves money and stock
— confirm decrements for COD, cancel restocks and flips `paymentStatus` —
it is the surface a dispute is actually argued over ("nobody cancelled that
order"), and with staff seats it is routinely done by someone other than
the owner. Products and settings, both far less consequential, were audited
from the start.
All five transitions now route through one helper rather than five copies.
The audit is written AFTER the transition, deliberately: `transition` throws
on an invalid state change, and a row for a mutation that did not happen is
worse than no row — it makes the log lie in exactly the situation someone
consults it. Mutation testing covers that ordering specifically; moving the
write earlier fails two tests.
The entry records the resulting `status`/`paymentStatus`, not just the verb,
so the log answers "what did this order become" without joining a row that
has since moved on again.
Found while auditing SPEC §9 for the P6 security checklist.
41 tests in the suite, 3 new.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
SPEC §5 point 5 promises six plan limits "enforced in API middleware"
returning "a typed error the UI renders as an upgrade prompt". Auditing all
six against the code found that neither half was quite true.
Two inconsistencies, both real:
- `productsMax` and `staffSeats` fail OPEN when a tenant has no
`TenantLimits` row (`if (!limits) return;`), while the other four fail
closed. A store that got provisioned outside the onboarding path would
have had unlimited products and seats, silently.
- The 402 body came in two shapes — `details: { limit }` for quotas,
`details: { feature }` for `whatsappChannel` — so the admin UI could
never tell a merchant WHICH entitlement ran out.
And one outright hole: `PATCH /v1/admin/whatsapp/numbers/:id` had no plan
check at all. A store downgraded off the WhatsApp channel could not
`connect` a number but could flip an already-registered one back to
`connected`, restoring a channel it no longer paid for. Now gated —
disabling stays ungated deliberately, because an off switch behind an
upgrade prompt is a trap.
`customDomain` was enforced NOWHERE; the column was written by onboarding
and read by nothing. The custom-domains feature added earlier this session
now goes through the shared gate rather than its own hand-rolled 402.
The fail-open posture on `productsMax`/`staffSeats` is deliberately NOT
flipped here. It is one word per call site, but `signUpWithTenant` creates
fixtures with no limits row, so flipping it would 402 product creation
across roughly a dozen suites, and it needs a backfill for tenants created
outside onboarding. Instead the posture is now a REQUIRED named argument —
stated in one readable word at each call site rather than expressed
accidentally through the shape of an `if` — with both postures pinned by
tests and the flip path documented. In production the branch is
unreachable, so this is a latent default rather than an active leak.
The CSV dry-run's "limitExceeded" preview had its own hand-written
comparison that could drift from the commit-time check; both now call one
function, so preview and outcome agree by construction.
Nothing was weakened: staff's stricter rule (pending invites occupy seats)
and products' (archived rows do not count) both survive as count callbacks.
Six mutations, all caught: the new PATCH gate, the missing-row default
flipped to true, the block/allow posture ignored, the `feature` field
dropped, a quota off-by-one, and report/assert disagreement.
119 tests green across plan-limits, products, staff, csv-import,
whatsapp-webhooks and custom-domains.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
…tabase
The Ventia operator's surface: list/search tenants with GMV and
month-to-date AI usage, tenant detail, assign plan, suspend, reactivate.
## Where the privilege lives, and why not in a row
There was no platform-operator role. The obvious move is
`Membership{role:'platform_admin'}` — the enum value already exists and
`tenantId` is nullable, so it needs no migration. It was rejected, and the
reason is worth stating: `Membership` rows are already written by the
merchant-facing staff-invite path, and the only thing separating "invite a
colleague" from "mint a platform operator" is that `staffInviteSchema`
happens not to accept `role`. That is one `...body` spread away from total
compromise. A privilege a merchant-facing write path could ever produce is
not a platform privilege.
So the grant is `PLATFORM_ADMIN_EMAILS`, an env allowlist. No HTTP request,
buggy or otherwise, writes `process.env`. Unset means nobody — an empty set
that denies everyone, loudly logged. `User.isPlatformAdmin` is the better
long-term answer and is recorded as a follow-up, but it should be required
IN ADDITION, never instead: the column gives audit-join visibility, the env
var keeps the grant outside the database the product also writes to.
The guard additionally requires a VERIFIED email, and that is load-bearing
rather than belt-and-braces. `auth.ts` sets `requireEmailVerification:
false`, so anyone can sign up as an address they do not own and hold a
valid session immediately. Operator addresses at a company's own domain are
guessable — without this check, registering `ops@ventia.co` before the real
operator does is a full takeover. It also reads the session via
`auth.api.getSession` directly rather than the shared context helper, so
the admit decision touches zero bytes a merchant write path can influence.
## Suspension bites immediately
`DomainResolver` caches the resolved tenant — status included — for 60s, so
a bare status UPDATE left a suspended store serving for up to a minute.
That is wrong for both reasons you suspend: a minute more unpaid, or a
minute more of the abuse you were stopping. And the operator watching the
storefront concludes the button is broken and clicks again. `setStatus` now
evicts the cache key for EVERY domain of the tenant, not just the primary —
a custom-domain tenant's shoppers are on the other host. Redis failure
degrades to reporting `within-60s` rather than failing a mutation that
already committed.
`planLimitsFor` delegates to the existing `PLANS` table rather than
restating the numbers, deviating from my brief on purpose: `PLANS` already
feeds the seeder and onboarding, so a second literal copy would create the
drift that "single source of truth" exists to prevent. Plan and limits are
written in ONE transaction — a premium tenant whose limits write failed
would read premium in the UI while the budget service, treating a missing
row as zero, refused every shopper.
Impersonation is deliberately NOT built. It means minting a session that
merchant guards accept: a new credential type with its own expiry,
revocation and blast radius, which deserves its own review rather than
being smuggled in beside CRUD.
26 tests. Mutation-verified independently: removing the email-verification
requirement fails a dedicated test, as do fail-open on an empty allowlist
and dropping the cache eviction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
`Payment` has been in the schema since the initial migration with no reader
and no writer. SPEC.md §8 specifies it (`order_id, provider, provider_ref,
amount_cents, status, raw`), so the answer was never "drop it or keep it" —
it was "implement it".
## What one slot cannot hold
`Order.paymentStatus`/`paymentProvider`/`providerRef` answer "where does
this order stand now". That is one slot, and it structurally cannot
represent a shopper whose first card declined and whose second succeeded —
an ordinary enough flow that `markPaid` was widened in P3 wave 2 to accept
PENDING→FAILED→PAID, after which the order shows the second attempt and the
declined one is simply gone. Nor a partial refund, `paymentStatus` being a
single enum. So: `Order` is current state, `Payment` is history.
## Both settle paths write it
The webhook controller and the reconciliation worker. A ledger only one
path writes is worse than none, because it looks complete — mutation-tested
in both directions, each breaking 5 tests.
Three placement calls, each commented at the site:
- Refused events write NOTHING. `amount_mismatch`, `currency_mismatch`,
`cross_tenant_reference` and friends were refused precisely because
they are not established to be about this order's money; writing a
forged amount into the one table whose purpose is to be believed would
defeat it. Their disposition already lives on `WebhookEvent.result`.
- BEFORE the settle, never after. The row records what the gateway said,
not what we did — and a failed insert after a settle would 500 a
request whose order already moved, so the retry would find `markPaid`
no-op and record a false `paid_order_not_settleable` alarm,
manufacturing the exact signal that alarm exists to make trustworthy.
- `Order.paymentStatus` stays authoritative. Nothing reads the ledger to
decide order state; a PAID row can sit beside a cancelled order,
because that pairing is the truth.
Append-only, like `WebhookEvent`: SELECT for the tenant (you cannot refund a
shopper you cannot see was charged), no INSERT/UPDATE/DELETE, since the rows
most worth tampering with are exactly those recording a charge someone would
rather forget. ENABLE not FORCE — the reconciliation sweep is cross-tenant
with no `app.tenant_id`, and under FORCE its inserts would be rejected for
every tenant, producing the very "only one path writes it" failure this
avoids.
The dedupe key is load-bearing: the sweep re-asks about a PENDING/FAILED
order every two minutes forever, which without it is one row per two
minutes for an order where nothing is happening.
My brief was wrong on one point — `Payment.tenantId` already existed and is
already in the RLS list; nothing needed adding.
96 API tests (13 new, 83 existing unchanged) + 42 in packages/db (9 new).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
You have a Meta app, so the "unverified wire format" caveat becomes a deploy-time task. This is the shortest path to closing it. The runbook is deliberately blunt about where I expect the first breakage, ordered by likelihood rather than severity. Top of that list is my own invention: the `?number=` query parameter on the callback URL. Meta's GET handshake sends only hub.mode/challenge/verify_token — nothing identifying WHICH number is being verified — so that parameter is how the endpoint knows whose token to compare against. If Meta strips or rejects a query string there, the handshake cannot be attributed and setup fails at step 1. The fallback is small and named in the doc: try the supplied token against every configured number. Then the pinned Graph version (v21.0, one constant), `pushName` possibly coming back empty if Meta formats `wa_id` and `from` differently (harmless, display-only, but it will look like a bug), and the 24-hour customer service window — which does not bite today because every agent reply answers an inbound message, and which is exactly why template notifications are not built yet. Also fixes a real gap: `.env.example` had drifted behind this session's work. A deployer following it would have got an API where the agent never answers (no ANTHROPIC_API_KEY) and platform admin denies everyone — the latter correct but silent. Added ANTHROPIC_API_KEY, PLATFORM_ADMIN_EMAILS (with why unset means nobody, and why the verified-email requirement is load-bearing), DOMAIN_VERIFICATION_SECRET, and the agent rate limit. Noted explicitly that WhatsApp credentials are per-tenant, not per-deployment, so nobody goes looking for env vars that do not exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
…F up The P6 DoD item. Each row states what was verified rather than what was intended, and the unmet requirements are listed as unmet. The finding worth acting on: `/v1/admin/*` — products, orders, settings, staff, WhatsApp numbers — has no CSRF token and no origin check. That check exists only for `/v1/auth/*`. The entire protection for every admin mutation is the session cookie's SameSite attribute, which is a default inside a dependency and was pinned by nothing. It is adequate today; I verified it empirically rather than by grepping — signing in for real and asserting the Set-Cookie, after a grep of the dependency turned up nothing. But a better-auth release, or a config change made for an unrelated reason, could flip it to None and turn "cancel this order" and "rewire payment credentials" into things any site a signed-in merchant visits can trigger, with every existing test still green. `test/session-cookie.test.ts` now asserts HttpOnly, SameSite and Path. The audit also records the two things it turned up that are already fixed: order transitions having no audit log (96347cc) and two plan limits failing open (7ab3713). Largest outstanding gap, stated plainly rather than omitted: the Ley 1581 customer-data deletion flow and the agent-conversation retention purge are not built. Both are legal requirements in Colombia, neither is technically hard, and a platform selling to Colombian SMBs cannot ship to real merchants without them. Also honest about what this audit did NOT cover — dependency scanning, pen testing, CI secret handling, and the fact that no real payment-gateway sandbox has ever been available, so those adapters remain fixture-verified only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
SPEC §11 P6 DoD: "restore drill documented and executed". A backup nobody
has restored is not a backup, so this is the executed half — I ran it end
to end against the dev database before committing, and it passed 15/15:
Step 0 checksums match; ventia_app present
Step 1 pg_restore --exit-on-error completed
Step 2 table set identical (35 tables); row counts match on all 32
Step 3 RLS enabled on the same 28 tables; all 29 policy definitions
byte-identical (md5 837f5af6…)
Step 4 ventia_app's grant matrix identical (97 pairs); no access to the
7 auth/platform tables; no BYPASSRLS
Step 5 each tenant's GUC sees exactly its own rows; cross-tenant read
returns 0; no GUC returns 0; cross-tenant INSERT refused by
WITH CHECK
Step 6 20 migrations restored, latest 20260819120000_payment_ledger
Step 5 is the one that earns the script. Steps 3 and 4 assert the isolation
machinery is PRESENT after a restore; step 5 connects as `ventia_app`
against the restored data and checks it BITES. A dump that restores every
row but silently drops `tenant_isolation` looks perfectly healthy and
serves every tenant's data to every tenant — that is the failure mode a
row-count check cannot see, and it is the reason this drill exists rather
than a `pg_restore && echo ok`.
Grants are why backup.sh writes a globals dump alongside the data: grants
travel in the dump, but the ROLE they are granted to does not, so a restore
into a fresh cluster would land privileges on a role that does not exist.
The drill also surfaced a nice confirmation of recent work — it noticed
`WhatsAppNumber` as a table `ventia_app` cannot even SELECT, which is the
column-level grant added with the WhatsApp channel doing exactly what it
was written to do.
Scratch database is dropped on exit; the source is never written to.
The load-test script is still being iterated and is deliberately not in
this commit, along with docs/operations.md.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
`CheckoutService.checkout()` called `ShippingService.isCodAllowed()` and
`.priceFor()` from INSIDE `platformDb.$transaction()`. Both go through
`tenantDb()`, which needs a SECOND pooled connection to run its
`SET LOCAL ROLE` transaction — asked for while this request already holds
one. A burst of checkouts wider than the pool therefore deadlocks: every
connection parked inside a transaction, waiting for a connection only a
parked transaction can release, until Prisma's 15s interactive-transaction
timeout fires and the request returns a bare 500 with no error code. The
shopper waits fifteen seconds, gets "Internal server error", and has no way
to know whether they were charged.
Measured on the dev stack at Prisma's default pool (9 on 4 vCPUs), 100
concurrent checkouts, fix reverted and restored minutes apart:
before 7 x 201, 93 x 500 20 327 ms
after 100 x 201 1 926 ms
ShippingService now exposes `loadConfig(tenantId)` — the single
`Tenant.settings` read — plus pure siblings `isCodAllowedIn` / `priceForIn` /
`findMethodLabelIn` that answer from an already-loaded config with no
database access. Checkout calls `loadConfig` before opening the transaction
and prices from memory inside it, so it needs exactly one connection.
Only the I/O moved. Both questions are asked from the same place in the same
order and still throw the same SHIPPING_METHOD_UNAVAILABLE 400s at the same
point, so no error ordering changed. The data was never transactionally
consistent with the order write anyway: the transaction is on `platformDb`
and `tenantDb` never joined it.
`OrdersService.transition()` had the identical shape — `findMethodLabel` on
`tenantDb` from inside its own `platformDb.$transaction`, its comment citing
checkout's now-removed precedent as justification. Fixed the same way. Lower
traffic, same deadlock.
Sizing the pool above peak concurrency was a mitigation that moved the cliff
rather than removing it, and is no longer needed.
Also completes the P6 load-test DoD ("100 concurrent checkouts — isolation
suite green under load") with the script that found this:
scripts/load-test.mjs three scenarios, each splitting INVARIANTS
(correctness: stock accounting, tenant isolation, dense per-tenant order
numbering) from CAPACITY (was the offered load actually served), because
a platform that refuses work and a platform that oversells are not the
same problem. Stock scenarios drive `wompi` on purpose — COD does not
decrement stock at checkout, so a COD oversell probe asserts nothing.
docs/operations.md the runbook for all three scripts, with the evidence
for this bug and the two green/red configurations.
Green at 100 concurrent on the plain dev stack, default pool:
capacity stock 100 -> 100 x 201, stock 0, numbers dense 1..100, ledger -100
oversell stock 50 -> exactly 50 x 201 + 50 x 400 INSUFFICIENT_STOCK
isolation 2 tenants x 50 -> 50 orders each numbered 1..50, 0 foreign lines,
and under RLS as ventia_app each tenant sees own 50 / other 0
Zero oversell in every run at every level, including the wedged ones — the
bug cost availability, never correctness.
77/77 pass across checkout, shipping-quote and orders-transitions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
…he pulled copy SPEC.md §10 asks for a nightly `pg_dump` to R2 with 30-day retention. `backup.sh` produced the artifacts and pruned them, but nothing uploaded anything anywhere — and a backup that only exists on the machine running the database is lost in exactly the incident it exists for. `scripts/backup-upload.mjs` closes that. Written against the S3 API rather than anything R2-specific; the only R2-shaped thing is the endpoint URL you configure. Uses `@aws-sdk/client-s3`, already a dependency for product-image storage, so there is no S3 CLI to provision on the database host (added as a root devDependency — pnpm does not hoist it to where a repo-root script resolves from). Three properties worth stating, because each one is a way this could have been written that would look fine and not be a backup: ALL FOUR FILES, OR NONE. A run is the dump, the globals, the manifest and the checksums. Three of four offsite is a failed backup: a dump restored without its globals comes up in a cluster that has never seen `ventia_app`, which means tenant isolation switched off. Non-zero exit, no partial success, and `--list` marks any run that is not 4/4. EVERY UPLOAD IS VERIFIED BY READING IT BACK. A PutObject returning 200 says the request was accepted, not that the bytes that landed are the bytes you sent, and a silently truncated backup is indistinguishable from a good one until the day you need it. Each file is streamed back out of the bucket and re-hashed before the run is called uploaded. Proved non-vacuous: with the upload deliberately truncated to its first 10 bytes, the check caught it and refused the run. `--pull` EXISTS BECAUSE AN OFFSITE BACKUP NOBODY HAS FETCHED IS AN ASSUMPTION. It downloads a run and checks each file against the `.sha256` `backup.sh` wrote BEFORE the upload happened, so it verifies the whole round trip rather than two numbers this script computed agreeing. Wrong bucket, wrong prefix, credentials that can PUT but not GET, an object stored but unreadable — all of those present as "everything looks fine" until an incident. Executed end to end against the compose MinIO (S3-compatible, so the code path is the real one; only the endpoint differs from production R2): upload 4/4 files, each sha256-verified by read-back --list 4/4 ventia-ventia-20260819T011638Z 104533 bytes --pull 3/3 checksummed files match their backup-time sha256 drill restore-drill.sh --backup <the pulled dump> -> 15 passed, 0 failed That last line is the point: the DOWNLOADED copy restored and passed all 15 checks, including tenant isolation actually biting under RLS on the restored data. The offsite copy is known-restorable, not merely present. Not exercised, and said so in the doc: real Cloudflare R2 (no account here — the remaining risk is credentials and endpoint, not code), the cron itself, and alerting on a failed nightly run. Also in this commit: README phase status. P4 and P5 were still ⬜ despite being complete; both now describe what actually shipped. P6 is marked 🚧 with its done and outstanding halves listed explicitly rather than a single box that would be wrong either way. Full API suite re-run after the checkout connection-pool fix: 55 files, 815 passed, 5 skipped, exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
…ention Two independent pieces of P6 hardening. ## User.isPlatformAdmin — required IN ADDITION to the env allowlist The guard's own doc comment has said since it was written that the column is the right long-term answer and that when it lands it must be required in addition to `PLATFORM_ADMIN_EMAILS`, never instead of it. It has landed, and it is. Each control covers the other's weakness. The allowlist lives outside the database, so no application bug — not even arbitrary writes through the ORM — can add an operator; but it names an ADDRESS, which is a claim about identity rather than identity itself. The column names a specific user row and is written by no code path in the product: better-auth is deliberately configured with no `user.additionalFields`, so it does not know the column exists and cannot set it during sign-up or profile update, and no endpoint updates it. Granting it is one deliberate SQL statement by someone who already holds database credentials. An attacker now needs an allowlisted, verified mailbox AND a row written in a database they should not be able to write. The practical payoff is revocation: flipping the column removes an operator immediately, with no deploy, while the env allowlist keeps bounding which addresses could ever qualify. The column defaults to false, so the migration grants nothing to anyone — every existing operator is denied until someone runs the UPDATE. That is the correct direction for a privilege migration to fail, and the guard logs the exact statement to run when it hits that state, because "my allowlisted, verified account gets a 403" is otherwise a genuinely confusing thing to debug. Three new tests, and the pair that matters is stated as one claim: the flag without the allowlist denies, and the allowlist without the flag denies. A third asserts a freshly signed-up user does NOT get the flag — if better-auth ever gains an `additionalFields` entry for it, or an endpoint starts accepting it, that goes red. Mutation-tested: replacing the flag check with `if (false)` fails exactly one test, the one that targets it. 29/29 pass restored. ## Conversation retention purge (SPEC §9) "agent conversations covered by the same retention/purge policy". Shopper transcripts are the one table whose contents nobody controls — a shopper types a name, address or phone into a chat box because the agent asked something that invited it — so unlike an order, whose fields can be stripped one by one, the only defensible treatment of an aged-out transcript is deleting the row. Daily BullMQ sweep, 12 months by default via `AGENT_RETENTION_MONTHS`, with a malformed value falling back to the default rather than turning the job into a silent no-op that retains everything forever. Follows the stock-reservation worker exactly: the sweep is a plain exported function tests call directly, and a separate class is the only thing that touches BullMQ. Batched at 200, capped at 2000 per run with a warning when it caps, per-tenant failures caught so one bad tenant does not stop the sweep. Two things the implementation got right that a naive version would not: AGE IS INACTIVITY, NOT AGE-SINCE-CREATION. `whatsapp-inbound.service.ts` reuses the most recent non-resolved conversation for a returning shopper forever — on WhatsApp the thread IS the history — so a loyal customer of a two-year-old store is writing into a row whose `startedAt` is two years old. Eligibility is `startedAt < cutoff` AND no message since the cutoff. ONLY `escalated` IS STATUS-PROTECTED, NOT `open`. Verified independently: `open` is what every conversation is created with on both channels, and nothing ever moves one out of it — `escalated` comes from the handoff tool, `resolved` only from the merchant's manual PATCH. Protecting `open` would have made the purge near-inert: Ley 1581 satisfied on paper, violated in the database. Liveness is handled by the activity cutoff instead, which is strictly stronger than a status check. `AgentUsage` survives — confirmed against the schema to have no FK to either table, no relation field and no PII, so the merchant's billing history is not collateral damage. Nothing else references Conversation/Message. Retention is per DEPLOYMENT, not per tenant, and deliberately: the data subject is the shopper, not the merchant, and a store owner who could raise their own retention window in Configuración would be editing an obligation whose consequences land on someone who never gets a vote. Mutation-tested, three of them: dropping the age cutoff reddens 5 tests, dropping the escalation guard reddens 2, deleting AgentUsage in the purge transaction reddens 1. Batching was NOT proved binding — that is a known gap. Also known and not done: no index supports the platform-wide `startedAt` scan (a partial index is the right follow-up; survivable at SMB scale, not at scale). 58/58 pass across platform-admin, conversation-retention and stock-reservation-worker. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
The nightly purge runs platform-wide with no tenant filter, and the only
index on "Conversation" was ("tenantId", "startedAt") — a query that does not
constrain `tenantId` cannot use an index whose leading column it is. Same
problem 20260815180000_order_sweep_indexes fixed for the two "Order" sweeps,
and the retention worker's own report flagged it as real open work.
Measured on a scratch database seeded with 60 000 conversations across 20
tenants, taking one 200-row batch:
before 40.8 ms Seq Scan "Conversation" (34 881 rows) + Seq Scan "Message"
(24 521 rows) -> Hash Anti Join -> top-N heapsort over
34 814 rows, to return 200
after 1.3 ms Index Scan "Conversation_retention_sweep_idx" (stops at 200,
no sort) -> Index Only Scan
"Message_conversationId_createdAt_idx" per row
~31x here, and the ratio grows with the table: the seq-scan path costs
O(every conversation ever held) while the index path costs O(batch).
Two decisions worth recording:
NOT PARTIAL, unlike the "Order" sweep indexes. Those cover a self-draining
slice — an order leaves `PENDING AND stockReservedUntil IS NOT NULL` within
fifteen minutes, so the index stays the size of "orders currently
mid-payment". Conversations have no such slice: EVERY conversation eventually
becomes a purge candidate, that being the entire point of a retention policy,
so a partial index would converge on a full one while also paying for rows
moving in and out of its predicate. `status` is left out for the same reason
the reconciliation index leaves out `paymentStatus` — it is mutable, and a
mutable column in a partial predicate churns the index.
("conversationId", "createdAt") REPLACES ("conversationId") rather than
supplementing it. The old index finds a conversation's messages but then has
to read every one to answer "is any newer than the cutoff" — and a
long-running WhatsApp thread, the exact case the activity check exists to
protect, is where that list is longest. The composite turns it into a range
probe that stops at the first hit, and answers every lookup the single-column
index answered, since an index serves any prefix of its own key. Keeping both
would pay twice for one capability on the highest-write table in the agent
path.
`prisma migrate diff` reports no drift between the migration and the schema.
20/20 retention tests still pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
SPEC §9: "customer data deletion flow: request -> anonymize orders (keep amounts for accounting, strip PII)". A legal requirement in Colombia — Ley 1581 de 2012 gives a data subject the right to have their data suppressed — and the largest outstanding compliance gap on this branch. Anonymization, not deletion, because Colombian accounting law requires the merchant to keep the transaction. The money stays, the person disappears. POST /v1/admin/customers/:id/anonymize, owner-only on that handler while the list and detail routes stay open to staff. POST rather than DELETE, because nothing is deleted. Cleaned: Customer (email/phone/name), Order (email, phone, shippingAddress rebuilt field by field keeping only departamento/municipio, billingFields nulled), OrderEvent.data, Conversation.shopperRef, Message.content/toolCalls, NotificationLog.recipient, AuditLog.data, WebhookEvent.payload. Untouched: every amount, date, status, `number`, `reference`, OrderItem, InventoryMovement, AgentUsage, Payment. Row counts do not change — rows are rewritten in place. Three decisions worth recording: THE AUDIT ROW HAS NO FREE-TEXT FIELD. "Who requested it" is a closed enum (correo|whatsapp|telefono|presencial|otro) plus the acting user id. A note field would be the one place a merchant could paste back the PII the action just erased. The audit entry is written after commit and after the AuditLog redaction pass, so it cannot scrub itself. IDEMPOTENT WITHOUT A SCHEMA COLUMN. Every target value is a pure function of the row id, so a second run computes the same state, changes nothing, and reports alreadyAnonymized. An order placed AFTER a first run is still cleaned by a second run — tested, because that is the case a naive "already done" flag gets wrong. WEBHOOK PAYLOADS ARE REDACTED, NOT EXEMPTED. A gateway's verified payload is deliberately immutable evidence, but Wompi and Mercado Pago both put the payer's name and email in it, and a supresión request does not stop at the tables we find convenient. The redaction is structure-preserving, so every amount, status, id and timestamp the evidence exists for survives byte-for-byte. No `tenantDb()` inside the transaction — one `platformDb.$transaction` with `SET LOCAL ROLE ventia_app` + `set_config` + an advisory lock, every statement through `tx` with an explicit tenantId. AuditLog and WebhookEvent have all privileges revoked from `ventia_app`, so those two steps `RESET ROLE` at the end; both still name tenantId and are further narrowed to this customer's own entities and orders. A jsonb key-order bug surfaced during testing and was fixed: Postgres does not preserve key order, so a JSON.stringify before/after comparison reported a change on every run and destroyed idempotency. Replaced with a key-sorted canonical serializer. The load-bearing assertion does not trust the ORM: it enumerates every base table in information_schema and runs a `t::text ILIKE '%needle%'` scan for each of nine PII strings, so it covers columns and tables this service never mentions. Mutation-tested, five red: dropping the tenant scoping (cross-tenant 201 instead of 404), skipping the shippingAddress rewrite, skipping the WebhookEvent redaction, putting the PII set into the audit row, and reverting the canonical JSON compare. Two honest negatives recorded by the agent that built it: removing ONLY the app-level tenantId predicate stays green (RLS catches it) and removing ONLY the role switch stays green (the predicate catches it) — the layers back each other up, but neither is independently pinned by these tests. Known gaps, all deliberate: WebhookEvent rows with a null orderId are not reachable from a customer; web-widget conversations with a null shopperRef cannot be tied to a person; Payment.raw is untouched (both current writers store PII-free shapes, verified, but a future writer could change that); the JSON redactor is defense in depth over an exact key set, not a proof; and the Clientes dialog has no Playwright coverage. 25 API tests (8 Testcontainers + 17 unit) and 185 admin tests pass; tsc clean in both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
SPEC §9: "per-tenant privacy policy template (autofilled with store data)". Until now every store's /privacidad rendered "Esta tienda aún no ha configurado su política de privacidad" — shipping a platform whose default puts every merchant in breach of Ley 1581, which requires anyone collecting personal data to publish a política de tratamiento. The template follows what the law actually requires rather than plausible-sounding sections. Decreto 1074 de 2015 art. 2.2.2.25.3.1 fixes the minimum contents, and the document covers each in order: identity and contact of the Responsable; tratamiento y finalidades; the Titular's rights under Ley 1581 art. 8 including suprimir and revocar; the área responsable and the procedure with the statutory deadlines from arts. 14 and 15 (consulta 10 hábiles + 5, reclamo 15 + 8) and the art. 16 requisito de procedibilidad before the SIC; and the fecha de entrada en vigencia. Every factual claim is traced to code rather than assumed. Data categories come from checkoutAddressSchema and the Order/Customer/Conversation columns; "we never hold your card" is true because every gateway adapter returns a redirect URL, so card data is typed into the gateway and never reaches this system. Conditional sections are driven by the tenant's real configuration — the WhatsApp and conversation paragraphs only for a store with a connected number, the Anthropic paragraph only when the plan includes agent messages, Chatwoot only when handoff is on, gateways listed from those with saved credentials, and COD changing what §3 says about handing an address to a delivery person. What it refuses to claim matters as much: Order.billingFields exists in the schema but has no writer, so the template does not say we collect it. Three shape decisions: GENERATION NEVER WRITES. `POST .../generate` is a pure read that returns the text for review; publishing is a separate explicit PUT. "Don't overwrite the merchant's words" is expressed in the API, not left to a UI convention. THE DISCLAIMER IS MERCHANT-FACING ONLY. It says this is a template rather than legal advice and that the Responsable is the merchant, not Ventia (Ventia is Encargado). It is returned by the endpoint and rendered in the admin; it never enters bodyMd, where it would confuse shoppers. THE LAUNCH PROMPT DOES NOT GATE. Publishing a policy is the merchant's legal duty, unverifiable by us, and a hard gate would only teach people to paste a word into a box. The launch panel warns when policy_privacy is empty and links to the generator; "Lanzar tienda" stays enabled. The advisory read is wrapped so a failure can never stop the checklist rendering. The storefront fallback was rewritten rather than filled: a store that has published nothing must NOT be given a fabricated policy, since that is a legal statement made in the merchant's name about handling nobody agreed to. The new copy tells the shopper no policy is published, that their rights exist anyway, and where to go — the store's contact page, then the SIC. It informs without impersonating. There was no write path to TenantContent anywhere in the repo — the storefront read it and the agent read it, but nothing could put anything there — so a generator alone would have been a dead end. GET/PUT /v1/admin/content ship with it, owner-only on writes, staff able to read because the agent answers shoppers out of this content. 13 new tests plus 17 regression, all green; admin 185, storefront 127; tsc clean across api, admin and storefront. Mutation-tested three ways: never interpolating the store name reddens 2, returning the raw unfilled template reddens the contact-data assertion (the important one — it pins the tenant's own data independently of the store name), and returning '' instead of a visible [COMPLETAR] marker reddens the missing-data test on the sentence that would have read "es , propietario de la tienda". The "states everything Ley 1581 requires" test survives all three by design: it asserts structure, and structure and data are pinned by different tests. Known gaps, all deliberate. storeSettingsSchema has no legalName/taxId/address fields, so a real merchant sees five [COMPLETAR] markers — adding them is the highest-value follow-up and the generator already reads those keys defensively. policy-page.tsx splits on blank lines rather than rendering markdown, so the template is written to read correctly as plain paragraphs. And checkout still collects email, phone and address with no autorización checkbox and no link to /privacidad — a completed purchase is defensible as a conducta inequívoca under Decreto 1377 art. 7, which is how §4 is worded, but a checkbox and a link are what make it informed. That is now the largest remaining compliance gap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
The privacy section recorded all three SPEC §9 requirements as the largest outstanding gap in the document. All three are now built, so the section says what each one actually guarantees rather than leaving a checkmark to imply more than it should — including what the anonymizer does NOT reach (webhook rows with a null orderId, web-widget conversations with a null shopperRef, Payment.raw), and why retention keys on inactivity rather than age. The one genuinely open item is now named as such: consent at collection. A completed purchase is defensible as a conducta inequívoca under Decreto 1377 art. 7, but art. 9 wants authorization prior, express and informed, and art. 8 gives the Titular a right to prueba de la autorización. Two rows added to the security table, one of them a "was not": Platform-operator access now requires two independent grants — the env allowlist and a database column no product code path writes. Connection-pool exhaustion under checkout load. At 100 concurrent checkouts on the default pool, 7 requests were served and 93 returned bare 500s after a 15-second wait. Fixed, and the load test that found it now passes 100/100 on that same pool. Reliability gains the offsite-backup row, and states plainly what is still missing there: no cron and no alerting on a failed nightly run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
The last open item from SPEC §9. Checkout collected the shopper's name, email, phone and address with no authorization control and no link to the policy — and nothing in the entire storefront linked to any of the four policy pages, which existed only at guessable URLs. A policy nobody can navigate to has not been published in any sense Decreto 1074 recognises. ## A required checkbox, and why blocking is the honest behaviour Colombian law offers no GDPR-style "necessary to perform a contract" basis; art. 10's exceptions are public-register data, public-entity requests, medical emergencies and historical/statistical/scientific processing, and a shirt sale is none of them. So an authorization is genuinely required. That does not make a checkout without a checkbox automatically unlawful — Decreto 1377 art. 7 accepts *conducta inequívoca* as a valid FORM of authorization, which is how the generated policy already words this. The checkbox is a choice about EVIDENCE, not legality: art. 8 lit. e) lets the Titular demand *prueba de la autorización*, and "they completed a purchase, from which authorization may be inferred" is an argument, whereas a timestamped explicit acceptance is a record. The person who has to make that argument to the SIC is the merchant — they are the Responsable, Ventia is only Encargado — so taking the weaker evidence to save one tap is a bad trade made on someone else's behalf. Unticked by default, because art. 7 also says silence is not authorization and a pre-ticked box manufactures a record of something that never happened. Enforced server-side with `=== true`, not truthiness, and not only in the browser: evidence produced by a rule only the client enforces is worth nothing. `CheckoutInput.acceptedPrivacyPolicy` is typed `true` rather than `boolean`, so "order creation proceeding without authorization" is unrepresentable past the controller. ## The evidence, and why it is not personal data Two nullable columns on `Order`. `privacyAcceptedAt` is when; `privacyPolicyVersion` is a `sha256:<16 hex>` fingerprint of the exact `title` + `bodyMd` published at that moment, or the sentinel `sin-politica-publicada`. `TenantContent` keeps no revision history and not even an `updatedAt`, so a fingerprint is the only way to say WHICH policy. It makes the claim falsifiable — hand it a candidate text and it tells you — though it cannot resurrect a policy the merchant later overwrote. That limit is stated in the module rather than hidden. Both values are computed server-side. The browser is the party whose authorization is being evidenced; letting it name the version or the timestamp would make the evidence a copy of the assertion it is meant to corroborate. A test asserts the client sends one boolean and nothing else. Neither column narrows to a person — the timestamp is a fact about the order, which already carries `createdAt`, and the fingerprint is a fact about the merchant's own policy text, byte-identical across every shopper who checked out against that revision. So the anonymizer deliberately leaves both alone, and must: a supresión request erases the person, not the record that the treatment was authorized while it happened. Erasing it would leave the merchant unable to answer for that processing, at the request of the one person entitled to demand the proof. Pinned by tests — the whole-database PII scan stays clean with the columns populated, the recorded version is identical across two shoppers, and a pre-migration order's nulls stay null. The policy read happens BEFORE `platformDb.$transaction()` opens, following the `loadConfig` precedent, with a comment saying why — no `tenantDb()` inside an open transaction. ## Also Footer linking all four policy pages, rendered unconditionally in RootLayout (verified mounted, not just written). `legalName`, `taxId`, `address`, `municipio`, `departamento` added to `storeSettingsSchema.storeInfo` and to the Tienda tab, with key names matched exactly to what the policy template already reads — until now there was no writer for any of them, so every generated policy carried five [COMPLETAR] markers. `departamento` is a Select over the complete 33-entry list; `municipio` stays free text because MUNICIPIOS is a ~73-entry shipping subset of Colombia's 1.100+, and a merchant domiciled outside it must still be able to state their own domicilio in a legal document. `privacy-policy.test.ts`'s fixture now saves those fields through the real PATCH endpoint rather than writing JSON into the column, so it would catch the schema silently stripping them again; a new test generates for a bare tenant, saves the fields, regenerates, and asserts the markers are gone — while the one remaining marker (a tenant with no domain) is asserted as still present, because the generator must keep saying so. Table-level GRANTs cover the new columns; verified against the running Postgres with has_column_privilege rather than assumed. Mutation-tested, three binding: stubbing the server-side consent check reddens 2 checkout tests, folding the shopper's email into the fingerprint reddens the "identical for two different shoppers" test, and having the anonymizer null the consent columns reddens both prueba tests. Full API suite: 60 files, 893 passed, 5 skipped, 0 failures. Storefront 133, admin 185. No migration drift. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
SPEC §6 (M9) asks for impersonation with a banner, a 30-minute expiry and every action audit-logged. It was deferred twice while the rest of P6 shipped, on purpose: it is the one feature that deliberately crosses the tenant boundary the whole architecture exists to hold, so it gets designed rather than discovered. The decision everything else follows from is whose identity an impersonated request carries. Minting a session AS the merchant — the obvious implementation — is rejected: `writeAudit` records `session.userId`, so every action would be attributed to the merchant in the permanent record. Not unattributed, MISATTRIBUTED, which is worse — the merchant's own audit log would assert they did something they did not do, with no field saying otherwise. SPEC's "every action audit-logged" would be satisfied on a technicality and defeated in substance. It also creates a real credential: a minted cookie for the merchant's account is a full takeover sitting in a browser, a log, or a screenshot. Chosen instead: the operator stays themselves and borrows a scope. A separate short-lived signed grant says "this operator may act within tenant X until T"; `getSessionContext` keeps returning the operator's userId and adds the granted tenantId. Every audit row then names the operator with no change to any call site, there is no merchant credential anywhere, and revocation is deleting one thing used only for this. A signed token rather than a column on `Session`, because it expires by construction — past `exp` it does not verify, so no code path can forget to check. The operator id is IN the payload and compared against the live session on every request; that check is what turns "signed" into "bound", without it a leaked token is a bearer token for someone else's store. Thirty minutes hard, no sliding window: an operator who leaves a tab open must not be inside a merchant's store indefinitely. The section likely to be argued with is what an operator may DO. The tempting answer is full access since it is audited. That is wrong, because an audit row is a remedy for "who did this?" and no remedy at all for an action whose effect has already left the building — a Ley 1581 anonymization (deliberately one-way), an email or WhatsApp message landing in a real shopper's inbox signed by the merchant's store, or a staff invite that outlives the thirty minutes. So: allow-list, not deny-list, with writes starting at zero. A deny-list permits every future endpoint by default and fails silently. RLS is unchanged and does not need to change — `tenantDb` still sets `app.tenant_id` under `ventia_app`, so impersonation widens which tenant an operator may name without weakening what keeps a request inside it. That property is free, and it is why this is safe to build at all. Includes the test list up front, so the implementation is not graded on its own homework — the operator-attribution assertion and the grant-binding check are called out as the two that must be mutation-tested, since if either passes with the code broken the feature is not built. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
"Subscription tracking (v1 manual): plan, price, paid-until date, notes; auto-suspend N days past due (configurable, default 7) with warning email at N-3." No payment processor is involved and none is built — an operator records that a merchant paid until a date, and the system acts on that date. ## The security finding: Subscription was in the wrong RLS bucket The table has existed since `init` and was never written to, so nothing had ever exercised it. `20260723182728_rls` swept it into the TENANT bucket purely because it has a `tenantId` column — granting `ventia_app` SELECT/INSERT/ UPDATE/DELETE on it. That is the wrong classification, and the `UPDATE` is the sharp end: `paidUntil` is an ENFORCEMENT INPUT. Under the old grant, any tenant-scoped write path that could be induced to touch this table would let a merchant set their own paid-until to 2099 and never be suspended. `priceCents` is also commercial terms between Ventia and one merchant — it leaks merchant-to- merchant the moment the policy is loosened — and nothing merchant-facing reads this table at all: no storefront, no admin screen, no agent tool. Moved to the `20260723205801` bucket instead: REVOKE ALL FROM ventia_app, and removed from TENANT_MODELS. RLS and the tenant_isolation policy are deliberately KEPT even though the grant is now zero — it costs nothing (the owner is exempt) and bounds the damage if a future `GRANT ... ON ALL TABLES` re-grants by accident, which is exactly how this table got misclassified in the first place. Verified against the running Postgres: 0 privileges for `ventia_app`, RLS still enabled, policy still present. `packages/db/test/ rls.test.ts` gains five tests pinning that it cannot read, update, insert or delete. ## Idempotency without a new column "Already warned" is a `NotificationLog` row keyed `subscription:<tenantId>:subscription_due_warning:<paidUntil ISO>` — the table SPEC §6 M10 already defines, with uniqueness enforced by the database rather than a read-then-write that races. Binding the key to `paidUntil` makes it per CYCLE: pay and move the date, and a future lapse warns again; re-save the same date and it does not. A `failed` send is re-claimable, a `sent` one never is — a warning nobody received is worse than a duplicate. ## Never suspends the ambiguous cases The candidate query reads FROM `Subscription`, so a tenant nobody has recorded a payment for is structurally invisible to the sweep rather than excluded by a filter someone could drop. Null `paidUntil` is skipped explicitly, and `draft` tenants are skipped because reactivating one later would LAUNCH a store that never finished onboarding. Suspending every pre-existing tenant the day this ships was the failure mode to design against. Suspension routes through the existing `setStatus`, so the resolver-cache eviction behind §6's "under 60 s on the storefront" AC is reused rather than duplicated — and that reuse is load-bearing, proved by mutation. Audit rows carry PREVIOUS and NEW values, because the table holds only current state: that audit trail IS the payment history. ## Verification Sweep logic is a plain exported function tests call directly; the BullMQ class is separate and started only from main.ts's real-boot branch. 26 new tests including a storefront request through the real TenantMiddleware returning 503 right after a sweep, with suspension asserted by raw SQL against Postgres rather than a return value. 64 pass across subscription/platform-admin/ stock-reservation; 47 across packages/db. tsc clean. Migration drift: none. Four mutations, all red then reverted: suspending on the warn cutoff instead of the suspend cutoff (6 red, including the boundary cases at -2, 0, 1, 6 and 6.999 days), dropping the `status: 'live'` guard (re-suspends and re-audits), making a lost NotificationLog claim unconditionally re-claimable (re-warns), and bypassing `suspendForNonPayment` for a direct update (storefront stays up). Known gaps, all deliberate: no "you have been suspended" email — SPEC specifies one email and this builds exactly that; a tenant that crosses both N-3 and N while the sweep is down is suspended un-warned, because telling someone "in three days" when their window already closed is a false statement; and the batch caps are reasoned rather than measured, which the migration comment says outright instead of implying a benchmark that never ran. Admin UI is not in this commit — it lands separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
The platform API has existed and been tested since 93f719c with no UI at all. This is the UI: tenant list with search and status/plan filters, detail with GMV, orders and AI usage/cost month-to-date, assign plan, suspend, reactivate. ## Making it unmistakable, which is an AC and not decoration SPEC's failure mode is an operator forgetting whose store they are looking at and treating a real merchant's account as a demo. Five redundant layers: A different colour world built from the SAME components — a `[data-surface='plataforma']` scope re-points the `@ventia/ui` CSS variables rather than forking anything, chosen specifically so the two surfaces cannot drift back toward each other later. Merchant admin is white/slate/blue and rounded; the console is near-black/amber and squared off. A different structural shape (full-width top bar with a hazard band, versus the merchant's left sidebar), so they do not resemble each other in peripheral vision. Standing text naming the surface. A different browser-tab title, because an operator with both windows open reads the tab first. And the detail page opens with an identity block — name, slug, status — before a single metric. Badge weight is inverted from the obvious: `live` is muted, `draft` amber, `suspended` red. A console listing forty healthy stores should not paint forty accent pills; the exceptions are what should stop you. ## The suspend confirmation is the tenant's own slug Not a fixed word like the merchant Clientes page's ANONIMIZAR. The realistic failure here is not "clicked without thinking" but "right intent, wrong company", and a constant word becomes muscle memory by the third use and stops discriminating between rows. A per-tenant token cannot be produced without reading the identifier of the store actually on screen. Reactivation is deliberately much lighter — its failure mode is a store that works. Writing the test for this found a real hole: a tenant with a blank slug would have made an empty confirmation box match. Fixed the function, not the test. ## Real control versus cosmetic, stated plainly The real control is `PlatformAdminGuard`; every `/v1/platform/*` response is 401/403 for a non-operator regardless of anything in this app. The layout gate is cosmetic-but-honest: it does not read a client-side claim — there is no `isPlatformAdmin` on `/v1/admin/me` and none was invented — it asks the guarded endpoint itself, server-side, and renders whatever the guard says. It fails closed by construction, since `operator` is returned for exactly one status. Deleting it would leak nothing. No `/plataforma` entry in either role's sidebar, bound by a test. `navItems` is keyed on MEMBERSHIP role, and platform authority is deliberately not a membership — gating a platform link on `role === 'owner'` would state the opposite relationship in the UI and show every store owner a link they cannot use. An explicit 403 page rather than a fake 404: the endpoint is discoverable anyway, so the obscurity would be fictional while the confusion would be real. ## Verified against the real API, not assumed shapes A second API instance on :4010 with an allowlist set (the dev API on :4000 was left untouched), a real operator granted both controls by SQL, every endpoint exercised. List and detail shapes match the TypeScript field for field including `percentUsed: null` when the limit is 0; the guard ladder is exactly 401 → 403 → 200 as the pure gate function assumes; `TENANT_NOT_SUSPENDED` 409, `TENANT_NOT_FOUND` 404 and non-UUID 400 are all real. `?q=&status=` really does return 400 VALIDATION_FAILED — the exact bug `buildTenantListQuery` exists to prevent, confirmed empirically rather than assumed. Audit rows carry actorEmail, reason and previous values, so the UI copy about the audit trail is accurate rather than aspirational. Then the UI itself was driven in Chromium against that API: anonymous redirects to login, the console renders with the right surface token, search narrows, and the suspend button stays disabled for a wrong slug and enables for the right one in any case. Probe tenant, operator, sessions and audit rows deleted afterwards. 221/221 tests in 23 files, up from 185/21, no regressions. tsc and eslint clean. Two mutations caught: collapsing the slug confirmation to "any non-empty string" reddens 2, and making the access gate fail OPEN on an unexpected status reddens 2 — so a 500 can never silently render a console. Impersonation is deliberately absent, with no placeholder button — its design is in docs/superpowers/specs/2026-08-19-impersonation-design.md. The subscription panel is read-only and renders only fields certain to survive, leaving a seam for the editing UI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
Every dialog in both apps — the Clientes anonymize confirmation, the Equipo
staff dialogs, the new operator console's suspend confirmation — has been
rendering in the top-left corner of the viewport rather than centred.
The UA stylesheet centres a modal with `dialog:modal { position: fixed;
inset: 0; margin: auto }`. Tailwind v4's preflight resets `margin: 0` on
`*, ::before, ::after, ::backdrop`, which silently overrides it. Nothing in
this repo asked for that; it is a rule from a dependency's reset defeating a
rule from the browser, and neither one is visible at the call site.
Found while reviewing the operator console, which flagged it as pre-existing
and out of its scope — correctly, since it affects the merchant app
identically.
Measured rather than reasoned about: a minimal page carrying Tailwind v4's
exact preflight margin reset, driven in Chromium at 1200x800 with the same
448px max-width this component uses.
without m-auto top 0, left 0
with m-auto top 367, left 376
(1200 - 448) / 2 = 376, so the second is exactly centred.
One class, in the shared component, so both apps and every future dialog get
it. The comment explains what it is compensating for, because `m-auto` on a
dialog reads like arbitrary styling and would be an easy thing to "clean up".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
P6's last Definition-of-Done item is "two pilot merchants live on custom domains" — the only one that cannot be completed from a development environment, because it needs real merchants, real gateway credentials and real DNS. This is everything that has to be true before that is possible, assembled from what the code actually requires rather than from a general idea of what deployment involves. Every "we'll configure that at deploy time" deferral made during this build is collected here with what it actually needs: the Meta app decision that is platform-level because Meta delivers one webhook per app; the R2 credentials the offsite upload refuses to run half-configured without; the Caddy `on_demand_tls` block that ships commented out because it needs a real public hostname; and the two-step operator grant, where `PLATFORM_ADMIN_EMAILS` alone is deliberately not enough and the `isPlatformAdmin` UPDATE has to be run by hand because nothing in the product writes it. Facts were checked against the code rather than written from memory — the TLS-ask gate really does require all four conditions (registered, DNS-verified, tenant live, plan includes customDomain), the Caddyfile block really is commented, and the uploader really does refuse a partial configuration. Three things are stated more bluntly than a deployment guide usually would, because each is a thing a reader would otherwise discover at the worst moment: Losing PAYMENTS_ENCRYPTION_KEY is not recoverable. Every merchant's stored gateway and WhatsApp credentials become undecryptable ciphertext. Back it up somewhere that is neither the database's machine nor the database's backup — a backup holding both the ciphertext and its key protects neither. Nothing in this repo schedules the backups. The scripts exist and have been exercised, but a backup whose failures go to a logfile nobody reads is a backup nobody has. RPO is one nightly dump — up to 24 hours of orders — and that should be a decision rather than a discovery. No gateway and no WhatsApp number has ever been exercised against live traffic. All of it is verified against recorded fixtures and documented signature schemes. The first real transaction on each is a test, and should be a small one the merchant is standing next to. Ends with what is still missing and is not pretended otherwise: no CI/CD, no container images, no migration-on-deploy ordering, no staging environment, no load test against real hardware, no dependency-vulnerability gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
…admin) Three console follow-ups, plus a real timezone bug found while doing them. ## Dates were a day late outside Colombia The API sends day-boundary timestamps — `paidUntil: '2026-09-01'` comes back as `2026-09-02T04:59:59.999Z`, and the suspension day as `2026-09-09T04:59:59.999Z`. The existing `formatDateCO` used the RUNTIME timezone, so the same instant rendered as: America/Bogota 8 de septiembre de 2026 UTC (runtime zone) 9 de septiembre de 2026 A Colombian merchant was therefore shown the wrong suspension date whenever the admin ran anywhere but Colombia — which, for a Node process on a cloud host, is the default. `formatDateBogota` / `formatLongDateBogota` / `toBogotaDateInput` are pinned to `America/Bogota`. Mutation-tested: dropping the timezone reddens four tests. ## Subscription editing The panel was read-only. It now edits through `PUT /v1/platform/tenants/:id/subscription`, with no client-side date arithmetic anywhere: `dueState`, `suspendsOn`, `warnsOn` and `graceDays` all come from the API, so a deployment running `SUBSCRIPTION_GRACE_DAYS=14` reads correctly instead of quietly assuming seven. Hardcoding 7 reddens a test. It re-reads the subscription when the editor opens, because PUT is a whole-body upsert and editing a stale copy silently reverts another operator's change; if that read fails it says so rather than pretending. And it tells the truth about what saving does: recording a paid-until date does NOT reactivate an already-suspended store. `tenantStatus` from the PUT response drives a warning saying the store is still suspended and that reactivation is a separate action. Ignoring `tenantStatus` reddens a test. No "reactivate from here" shortcut was added — reactivation is a separately audited decision with its own confirmation, and duplicating the trigger next to a billing form is how it becomes reflexive. ## Operator identity The header read `/v1/admin/me`, which 403s `NO_TENANT` for a Ventia employee with no store of their own — so it silently showed nothing on the one surface where knowing which account you are acting as matters most. Now reads `GET /v1/platform/me`, with the parsing extracted as a pure function so the degradation is proven rather than hoped for. ## Impersonation banner Built to the design doc's §5: its sole input is the server's `/v1/admin/me` response, read once and passed down — no cookie, route param or localStorage read anywhere. No dismiss control. A block in a flex column, so it displaces rather than overlays. Names the store, ticks a countdown off the grant's hard expiry, and holds the exit. One deliberate asymmetry: it returns "no banner" ONLY when the server reports no context. A malformed payload still renders the banner with degraded copy, because a warning that hides itself is exactly the failure the AC exists to prevent. Making it fail silent reddens three tests. It also closed a hole it created: a lapsed grant makes `/v1/admin/me` answer 403, which the old code mapped to "no tenant" and redirected to `/onboarding` — inviting a Ventia employee to found a store. Both shells now route back to the console. The banner does not auto-redirect on expiry. It says the session ended and leaves the exit under the operator's hand rather than yanking them out of a half-finished page. ## Verification and its limits 279 tests in 24 files, up from 221 in 23; tsc and lint clean; storefront unaffected. The assumed shape for the impersonation payload was written blind and is isolated in one adapter; the API landed mid-task and matched it field for field, confirmed by reading the landed source rather than executing it. No running API was reachable, so ground truth for the wire format came from executing the API's own `subscription-window.ts` directly. Nothing here claims a live round trip. Not proven binding: everything rendered rather than computed. This app's vitest is node-env with no testing-library, so nothing asserts the banner is un-dismissable, that it displaces rather than overlays, or that the layout mounts it. Those are review-verified only, and that is a real gap rather than an oversight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
Implements docs/superpowers/specs/2026-08-19-impersonation-design.md, the last
substantive SPEC §6 item. The design's §7 test list was written before any
code precisely so this would not be graded on its own homework; all of it is
built.
## The shape
The operator's session is never replaced. A signed, self-expiring grant —
`{ op, ten, exp }`, HMAC-SHA256 over base64url with `AUTH_SECRET`, 30 minutes
hard — rides in its own `ventia_impersonation` cookie. `getSessionContext`
verifies it and keeps returning the OPERATOR's `userId`, adding the granted
`tenantId` and an impersonation context.
No `Session` row, no `Membership` row, no merchant credential exists anywhere.
`writeAudit` and `writePlatformAudit` were not touched: they already record
`session.userId`, which is now the right answer by construction rather than by
a call-site change that could be forgotten.
Hand-rolled rather than JWT, so there is no `alg` header to trust. Signature
checked before expiry, length-checked before `timingSafeEqual`, so a truncated
signature is a refusal and not a 500. The cookie's `Secure` flag derives from
`API_URL`, mirroring better-auth's own rule rather than sniffing `NODE_ENV`
(which this deployment never sets) — so the grant cookie and the session cookie
cannot disagree about TLS.
Writes are default-deny against an allow-list that ships EMPTY, enforced in
both `AdminSessionGuard` and `AuthenticatedGuard` — leaving the second out
would have made "default deny" a property of one guard rather than of the
system. A `HARD_DENY` table for the design's three irreversible categories wins
over the allow-list; it changes no behaviour today and exists so that whoever
first widens the allow-list cannot take the Ley 1581 anonymize route with them.
Two additions beyond the design, both deliberate: platform privilege is
re-checked on every impersonated request, because `UPDATE "User" SET
"isPlatformAdmin" = false` is this codebase's documented revocation and it
should not take up to 30 minutes to reach a cookie already in a browser.
## The one thing the agent flagged, and what I did about it
Design §4 says writes start at zero; §7 requires proving a write under
impersonation records the operator. With a literally empty allow-list there is
no write to perform, so the design's most important assertion was untestable.
Rather than allow-list a route — a security decision made by a test — the
implementation added a lexically-scoped, restoring override used by that one
test, and flagged it for review rather than burying it.
It was right to flag it. `writeAllowlist` is module state, so the widening is
process-wide for the duration of the call: harmless in a test, a cross-request
privilege leak in a serving process. Being reachable only by editing
application code made that a review question rather than an impossibility.
It is now impossible. Vitest sets both `VITEST` and `NODE_ENV=test`; this
deployment sets neither. The override throws in a serving process. Verified by
running it under `NODE_ENV=production` with `VITEST` unset: it refuses, and
the allow-list stays `[]`.
## Verification
44 new tests covering every item in design §7 plus cookie attributes, the
audit-before-token ordering, and the absence of any Session/Membership row.
76 pass across impersonation, platform-admin and session-cookie.
Five mutations reported binding by the implementation. I re-ran the one the
design names as most important myself rather than taking it on trust:
disabling the `op` binding fails exactly two tests — "rejects operator A's
grant presented with operator B's session" and "rejects an operator's grant
presented with a MERCHANT's session". That second one is the takeover case,
and it is the reason the check exists.
The other four: operator attribution (returning the tenant owner's id made
`AuditLog.actorUserId` the merchant's), the anonymize hard-deny (with the deny
removed AND the allow-list widened, the anonymize returned 201 and actually
ran), expiry, and the tenant cross-check.
## Known gaps
There is no *Impersonar* button anywhere in the admin — the banner and the
exit exist, the entry point does not. It needs a POST to the impersonate route
from the console's tenant detail followed by a full navigation, since the
shell reads the session server-side.
The full ~60-file suite was not re-run by the implementation (each file spins
its own containers); files touching `AdminSessionGuard` that were not
exercised include orders, products, csv-import and the agent suites. The guard
change is additive and inert without a grant cookie, but that was reasoned
rather than proven for those files.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
Impersonation shipped with a banner and an exit but no way in — the API route existed and nothing called it. This is the entry point, on the console's tenant detail. ## Why it confirms rather than just linking Every other control on that page acts ON a merchant's account from the outside. This one puts the operator INSIDE it: for thirty minutes their requests resolve to that tenant, and the merchant's own audit log will show a Ventia operator moving around their store. The merchant did not ask for that and is not notified. So the dialog spends its words on the two things an operator should have understood before clicking rather than after. That it is READ-ONLY — the write allow-list ships empty, so a write attempt 403s rather than doing something surprising, and saying so up front turns "why did that button not work" into an expectation. And that it is RECORDED UNDER THEIR OWN NAME, which is the entire point of the design's chosen shape and something an operator should know is true rather than assume is not. The reason field is optional, matching the API — issuing is itself the audited event, so the reason is colour rather than the record. It is offered because an operator reading their own audit trail in three weeks will want it, not because anything is gated on it. ## Why it is deliberately NOT the destructive-confirmation pattern `StatusPanel` makes you type the tenant's slug to suspend. This does not. Impersonating the wrong store is recoverable in a way suspending the wrong store is not: nothing changes, the banner names the store you actually landed in, and leaving is one click. Making every operator action feel equally grave is how the grave one stops registering. ## The one non-obvious mechanic Success navigates with `window.location.assign`, not `router.push`. The merchant shell resolves the session SERVER-side, so a client-side navigation would render it from a tree built before the grant cookie existed — the operator would land in the merchant admin with no banner, which is exactly the state design §5's "visually unmistakable" AC exists to make impossible. There is also no `finally` re-enabling the button on success: the page is navigating away, and re-enabling would offer a second click that starts a second session. 279 tests still pass in 24 files; tsc clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
…flight) WIP CHECKPOINT, committed deliberately rather than because it is finished. The agent building this is still writing the queue-health Testcontainers test as of this commit. Committing now because this session has already lost an uncommitted tree to an environment rollback once, and the work that exists is in a coherent, verified state: tsc --noEmit clean observability-scrub 37 tests passing, with observability-sentry health.test.ts the full app still boots with the module registered NOT yet verified by me, and the reason this commit says so in its subject: `test/observability-queues.test.ts` was being written while this was staged, so the queue-health endpoint's own coverage is unconfirmed. The full ~60-file suite has not been re-run since `main.ts` changed. Both land in a follow-up once the agent reports and I have checked them. Wires the two remaining "❌ Not wired" rows from docs/security-checklist.md (SPEC §9 Reliability): Sentry error reporting, and visibility into the four BullMQ repeatable workers. Reviewer's note for the follow-up: the parts to look hardest at are the PII scrubber (this codebase handles Colombian shopper names, phones, addresses and cédula-adjacent data under Ley 1581, and an error reporter is a data egress path that bypasses every control the anonymization flow adds), and the access control on queue state, which is cross-tenant platform data and must sit behind PlatformAdminGuard rather than any weaker check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
Completes 6cf21ac, whose subject said the queue-health test was still in flight. It has landed: 48 tests across scrub/sentry/queues, all green. ## The reported test failure was mine, not the code's That checkpoint noted one failing test, `payment-alerts.test.ts › "still allows the INSERT the append-only design depends on"`, failing with `Argument 'tenantId' is missing`. The agent reported it as pre-existing and not theirs, and proved that by restoring `main.ts` to HEAD and watching it still fail. That conclusion was right; the diagnosis was incomplete. The real cause: `packages/db/dist/tenant-models.js` was dated 07-26 and did not contain `WebhookEventReview`. The source did. `@ventia/db` resolves to `dist`, so `tenantDb(t).webhookEventReview.create()` was not being handed its `tenantId` by an extension reading a stale allow-list. That stale artifact is mine. When this session's environment rolled back to 8df6e06 I rebuilt `@ventia/core`, `@ventia/payments`, `@ventia/whatsapp` and `@ventia/ui` — and missed `@ventia/db`. Every test run since, including the agent's full-suite run, was against a months-old copy of that module. Rebuilt, the test passes 35/35 with no code change. Full suite re-run against a correct build: **65 files, 1011 passed, 5 skipped, 0 failures.** The previous "1 failed" number should be discarded rather than explained. ## Sentry Initialized before Nest builds a provider, so startup crashes are captured. An unset DSN returns `{enabled:false}` BEFORE the dynamic `import('@sentry/node')` — the SDK's OTel tree is never loaded, `http` is never patched, no `uncaughtException` handler is installed. A malformed DSN logs one loud line and stays off rather than throwing: a deliberate divergence from `PAYMENTS_ENCRYPTION_KEY`, because a telemetry typo must not take a storefront offline. The exception filter extends `BaseExceptionFilter`, so `super.catch()` gives byte-identical responses and its only contribution is the capture. The Express error middleware is mounted AFTER `listen()` — the only position from which it sees `TenantMiddleware`'s `next(err)`. ## The scrubber, which is the part that matters An error reporter is a data-egress path that bypasses every control the Ley 1581 work added, so this was the piece to get right. It imports `PII_KEYS` from `src/privacy/redact.ts` rather than copying it — one list, not a second weaker one. I verified it independently of its own tests: eleven planted values (shopper name, phone, address, cédula, session cookie, bearer token, IP, encrypted credentials, customer email, a phone inside free prose) through `scrubEvent`. Zero leaked. Request body dropped whole rather than walked, cookies dropped, headers allowlisted down to `content-type`, and the exception message `Unique constraint failed (email: …, cc: 1020345678)` came out as `(email: [redacted:email], cc: [redacted:id-number])`. The gap it does NOT close, stated rather than hidden: a bare name in free prose survives, because a name has no shape. Pinned as an explicit test and documented with the operational rule and the structural mitigations. ## Queue health: a JSON endpoint, not bull-board `GET /v1/observability/queues`, behind `PlatformAdminGuard`. The reasoning is worth keeping: bull-board mounts its own Express router, and Nest guards protect Nest routes — this codebase already knows that seam, it is why the rate limiters live on the adapter. Cross-tenant queue state is not the thing to protect with a hand-rolled second copy of the platform-admin check. It is also read-write by default, and its "retry" button on `subscription-sweep` re-runs a job that suspends tenants. Three mutations proved binding: passing `request` through instead of allowlisting it, removing the digit-run rule that catches cédulas and NITs, and removing the guard from the controller. Not proved binding: the boot-failure and Express-middleware captures, which no test reaches by design. Never verified: a real DSN. Nothing has reached sentry.io. Everything is proven up to the socket — envelope contents, produced by the SDK's own pipeline against a stub transport — but DSN correctness and alert rules are deployment steps. The checklist row says 🚧, not ✅. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
The P6 entry still listed Ley 1581 compliance, subscription tracking, the platform-admin UI and observability as outstanding. All four shipped. It now says what is built, and separates that cleanly from what is left. What is left is one thing: the pilot with real merchants on custom domains. That is not an engineering task and cannot be done from a development environment — it needs real merchants, real gateway credentials and real DNS — so it is named as such rather than left implied by a 🚧. Everything it requires is in docs/deploying.md. The entry also keeps three caveats that a status line would normally drop: no gateway and no WhatsApp number has ever seen live traffic, there is no CI/CD or staging environment, and nothing schedules the backups. A phase summary that omits those reads as more finished than the branch is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
Found while preparing the first real deploy, and not reachable before it.
`onboarding.service.ts` gives every tenant `${slug}.${PLATFORM_ROOT_DOMAIN}`
— verified, `isPrimary: true`. It is the only address a `basico` store has.
`isDomainAllowed`, the gate Caddy asks before issuing a certificate, required
`limits.customDomain`. `basico.customDomain` is `false`. So Caddy would have
been refused a certificate for the store's own address: no TLS, no storefront,
for every tenant on the entry plan.
Reproduced against the real database before touching anything — a live
`basico` tenant with a verified primary subdomain, asked exactly what Caddy
asks:
isDomainAllowed('certbug….ventia.co') -> false
It never surfaced in development because `.localhost` is a reserved TLD
(RFC 6761) that needs no certificate, so this endpoint's first real caller
would have been production.
The fix is a distinction the code did not make: the plan sells CUSTOM domains,
while the `${slug}.${root}` subdomain is the product. The gate now applies
only to domains outside our own zone. The other three conditions —
registered, DNS-verified, tenant live — still apply to everything.
`isPlatformSubdomain` matches on `endsWith('.' + root)` WITH the leading dot.
Without it, `evilventia.co` matches `ventia.co`, and anyone who registers a
lookalike is treated as living in our zone and skips the plan gate entirely.
Both cases are now tests, and both are binding: reverting the gate reddens the
subdomain test, dropping the dot reddens the lookalike test, each failing
alone. 21/21 pass.
## docker/Caddyfile.prod
The production reverse proxy, `caddy validate` clean and `caddy fmt` clean.
Reads `PLATFORM_ROOT_DOMAIN` from the environment so it is not edited per
deployment — and it MUST match the API's, since that is the value onboarding
mints subdomains from and the value the fix above recognises them by.
Known hostnames (apex, `api.`, `admin.`, `cdn.`) get ordinary HTTP-01
certificates. Everything else — tenant subdomains and custom domains alike —
goes through on-demand issuance behind the `ask` gate.
Two things worth knowing about it:
`/internal/*` returns 404 on the public `api.` host. `tls-ask` lives there
and is unauthenticated by necessity (Caddy calls it mid-handshake, before a
session could exist); the only thing keeping it private is that nothing
proxies it. Caddy reaches it as `http://api:4000` over the compose network,
which never passes through that site block.
Current Caddy has REMOVED the `interval`/`burst` limit that used to sit
alongside `ask` as an independent second bound. `ask` is now the only thing
between a stranger's DNS record and an ACME request in our name. That raises
the stakes on the endpoint rather than lowering them, and the config says so
rather than silently dropping the directive.
`cdn.` proxies MinIO for product images and answers 405 to anything that is
not GET/HEAD, so the S3 write API is not exposed even though the same port
serves it internally.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
`services/api/package.json` pins `@sentry/node` at `^10.70.0`; the committed
lockfile recorded the specifier as `^10`. The observability work edited one
without regenerating the other, and an ordinary `pnpm install` papers over it
by silently rewriting the lockfile — which is why it survived a full green
test run.
`--frozen-lockfile` does not paper over it, and that is the mode that matters
here: it is pnpm's default in CI, and it is what the Docker builds for this
deploy will use. Verified both directions rather than assumed:
committed lockfile ERR_PNPM_OUTDATED_LOCKFILE — "specifiers in the
lockfile don't match specs in package.json"
corrected lockfile "Lockfile is up to date, resolution step is skipped"
Left unfixed, the first `docker build` on the VPS would have failed at the
install layer, with an error about a Sentry version specifier that has nothing
to do with whatever the operator thought they were deploying.
One-character change to the lockfile; no dependency moves.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
…images Deployment scripting for a single Ubuntu 24.04 VPS: Postgres, MinIO and image builds all on the box, per the chosen shape. ## The gap this closes: product images were never backed up `backup.sh` dumps Postgres. With MinIO on the same VPS, losing the box would restore the database perfectly and lose every merchant's product images — a recovery where every order, price and customer is intact and every product renders a broken image. Worse than an obvious failure, because it looks like success. `backup-objects.mjs` archives the bucket alongside the dump: same BACKUP_DIR, same checksum-and-manifest shape, same offsite upload verified by read-back, same retention, and a `--drill` that restores into a scratch bucket and compares against the live one. Plain ustar, hand-written, no dependency. That is not gratuitous: real keys are `tenants/<uuid>/products/<uuid>/<uuid>.jpg` — 131 characters — so ustar's 100-char `name` plus 155-char `prefix` split is load-bearing rather than theoretical. Verified rather than assumed: system GNU tar lists and extracts all 13 seeded objects with paths intact. I ran the whole cycle myself against the dev MinIO: archive → upload (3/3 verified by read-back) → `--pull` → `--drill` on the PULLED copy, 7 passed 0 failed. Then flipped one byte deep inside an image payload, leaving the archive structurally valid — `tar` still read it, and three checks went red (backup-time checksums, per-object manifest hashes, byte-identical read-back). The drill is not vacuous. It is a full copy every run: correct at pilot scale, wrong later, and the script says so above `BACKUP_OBJECTS_MAX_BYTES` rather than quietly getting slower. It refuses when the offsite target is the bucket it is archiving, which would otherwise archive last night's archive. ## Alerting, which is the point of scheduling anything Both runbooks say a backup whose failures go to a logfile nobody reads is a backup nobody has. `notify.sh` is one configurable channel — an arbitrary command, or a webhook whose payload shape is Slack's and which is stated plainly as never having been tested against a real service. Unset exits 78 with a loud banner, and `install-cron.sh` REFUSES to install without a channel unless explicitly overridden. `cron-run.sh` prints "ALERT NOT DELIVERED — this failure reached nobody" when notification itself fails, so "job failed" and "job failed and nobody heard" never look alike. The nightly chain is `&&` throughout, so a failed dump is never followed by an upload that makes a bad run look good — proved by pointing it at a wrong password and watching it exit at step one with zero upload lines. ## deploy.sh Nine phases with timing, and a pre-migration backup — a migration is the most likely moment to need a restore and the runbook had no such step. It refuses rather than guesses: no migrate service and no `MIGRATE_CMD` is a refusal, because a wrong guess either skips migrations or runs the wrong thing against production data. HTTP checks hit container IPs directly and assert on the body, not just the status. Its env check is mostly about dev DEFAULTS rather than absence: `env.ts` gives almost everything a working default, so the API boots happily in production with `dev-secret-change-me` and `*.localhost` URLs. A missing `RESEND_API_KEY` is fatal because the mailer silently falls back to a console transport — every order confirmation printed to stdout instead of sent. ## provision-ubuntu.sh Idempotent, `--dry-run`, per-item ok/changed. Swap sized from RAM because two Next.js builds peak around 1.5–2 GB each on top of Postgres/Redis/MinIO. ufw reads sshd's EFFECTIVE port from `sshd -T` and refuses to enable without an SSH allow rule; password auth is disabled only when an authorized key already exists, otherwise it refuses rather than locking you out. It also flags that ufw does not filter Docker-published ports. ## What was NOT run `provision-ubuntu.sh` was never executed — this is a container, not a VPS, so ufw, systemd, apt, swap and sshd behaviour are unverified beyond linting. `deploy.sh` phases needing `docker/compose.prod.yaml` could not run because that file was still being written. Both are called out here rather than left to look tested. shellcheck and `bash -n` clean on all five shell scripts. No rollback in deploy.sh — stated in its own header with the exact recovery commands for before and after the migrate phase. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
…gress) A checkpoint, not a finished deliverable. `docker/Dockerfile.storefront`, `docker/Dockerfile.admin`, `docker/compose.prod.yaml` and `.env.production.example` are still being written, and none of the three images has been built yet. Nothing references these files, so committing them early changes no behaviour. Committed now rather than held because they are complete and self-contained in themselves, and the alternative was leaving them untracked across a session that has already been rolled back several times. Two things in here that were checked rather than assumed: `CMD ["node", "dist/main.js"]` — `services/api` runs under `tsx` in development, so a production image copying that would compile on every boot and depend on a devDependency. Verified the package has `build: tsc` and tsconfig sets `outDir: dist`, so the compiled entrypoint genuinely exists. `EXPOSE 4000` is documentation for `docker inspect`, not a bound port — `API_PORT` is read at runtime by `loadEnv()`. Verification of the actual build, image sizes and a running stack comes with the rest of the set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
Three multi-stage Dockerfiles, a production compose stack, and the env template. Completes the checkpoint in 0e84204. ## Port exposure, which is the line that matters on a public VPS Only Caddy publishes anything (80, 443, 443/udp). Postgres, Redis, MinIO and all three Node services have no `ports:` at all — verified by reading the file, not just asserted. The dev compose publishes every one of them, and carrying that habit to a public box puts the database on the internet, because Docker's published-port rules are inserted AHEAD of ufw's and the firewall does not save you. ## The migration gate is now real `docs/deploying.md` §9 listed migration-on-deploy ordering as unenforced. It is enforced here: a `migrate` service built from the same Dockerfile as the API (different target, so the migrations are by construction the ones the image was built against), and `condition: service_completed_successfully` on the API. A failed migration leaves the OLD API running, or on a first deploy no API at all, rather than starting one against a schema it does not match. ## The packaging problem, and two wrong answers before the right one `pnpm install --prod` does not do what it looks like. pnpm's virtual store is workspace-wide and built from the lockfile, so `--prod` prunes each importer's `node_modules` but not `node_modules/.pnpm` — measured at a 1.82 GB API image still containing vitest, tsx, testcontainers, supertest, typescript, the Prisma CLI and 274 MB of `@next/swc`. `pnpm deploy --config.node-linker=hoisted` gives a genuinely filtered tree, but re-resolves against the registry and honours neither `--frozen-lockfile` nor `--offline`. A conformance assertion caught 51 packages the lockfile does not pin, including `@aws-sdk/client-s3@3.1113.0` and the whole `@better-auth/*` set at 1.7.1. Shipping unpinned dependency versions on a payments platform is not a trade worth making for image size. Final shape: a lockfile-exact `--prod` install, then prune the virtual store by reachability — following each package's own dependencies but NOT peer links, which is how typescript, prisma, vitest and next stay alive in a "production" tree. 989 MB to 269 MB. The build asserts the result rather than hoping: dev deps absent, `@ventia/db` importable, `TENANT_MODELS` present, migrations present. ## Four bugs the build itself surfaced Prisma engine mis-detection (wrong `libquery_engine` for the runtime, which fails on first query rather than at build — `openssl` in the base fixes it); corepack's cache being written as root and re-downloaded by the `node` user on every migrator run; a CA file at mode 0400 warning on every non-root run; and `RESEND_API_KEY=` EMPTY being a hard boot crash-loop while absent is fine — after migrations have already applied. The env template now says to comment it out rather than blank it. ## What I verified, and what I did not I confirmed statically: no service but Caddy publishes ports, the `service_completed_successfully` gate is wired to the API, and `docker compose config` is valid against the full variable set in `.env.production.example`. Both apps still typecheck with `output: 'standalone'`. I could NOT rebuild the images myself. This sandbox routes outbound traffic through an intercepting proxy: without it corepack cannot fetch pnpm from the npm registry, and with it apt cannot reach the Debian mirrors. Neither failure can occur on a VPS with ordinary internet access, and the Dockerfile's `--secret id=proxy_ca` hook exists for environments like this one — but the image sizes, the running stack, the health checks and the port-exposure test in the build report are the agent's evidence, not mine, and are recorded as such. Also unverified anywhere: HTTPS through `Caddyfile.prod`. ACME cannot complete in this sandbox, so the first real certificate issuance on the VPS is the first time that path runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
`docs/deploying.md` was written before any deployment artifact existed — it listed what a deploy would need. The artifacts now exist, so it describes how to use them. Adds §0, the short path: provision, DNS, secrets, deploy, make the operator real, schedule the backups. Copy-pasteable, and each step points at the section explaining why it is there. The DNS block is the part most likely to be skipped and hardest to debug after the fact. The wildcard `*.yourdomain.co` record is what makes a new merchant's store reachable the moment they sign up, with no per-tenant DNS change — miss it and every store 404s at the edge while the platform looks healthy. Three sections were stale and are now true rather than aspirational: §6 pointed at a commented-out block in the dev Caddyfile. `Caddyfile.prod` now exists and validates. It also carries the certificate bug this deploy prep surfaced — every basic-plan storefront would have been refused a certificate for its own subdomain — because a reader hitting a TLS problem should find that first, not deduce it. §7 said no cron and no alerting existed. Both do; `install-cron.sh` refuses to install without a notification channel. It now also covers product images, which `backup.sh` never touched — with MinIO on this box that would have been a recovery where every order is intact and every product renders a broken image. §9 listed migration-on-deploy ordering as unenforced. It is enforced now, by compose's `service_completed_successfully` gate. Minimum box stated up front: 4 GB RAM, because two Next.js builds peak around 1.5–2 GB each on top of Postgres, Redis and MinIO, and swap is a safety net for a build rather than a substitute for memory. The "Not exercised" blocks are kept and extended rather than quietly dropped: the cron firing on a real box, and the first ACME certificate issuance, are both still unverified — ACME cannot complete in this sandbox at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012tUfx41TMxana567RboGMb
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
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.
No description provided.