Memory Wiki

I'll start by loading all reference documents and source code in parallel.

Now let me read all source code files in parallel.

Now reading all T52 files in parallel.

Let me check the webhook handler tests and repository for the conversion retry implementation.

Now let me run the verification commands.

Let me check for the critical migration issue I spotted, and verify test syntax.

Let me check a few more things — the interface contracts and README.

Now I have everything I need. Let me compile the complete SGS Phase Gate verdict.

Here is my complete SGS Phase Gate verdict: --- ## SGS Phase Gate Verdict ### Architecture: FAIL The architecture follows the SA's plan correctly (Option C: Both — server-side API + embedded checkout). T52 creates the Google Ads interface, T17 wires conversion upload into the webhook, T15 adds uiMode + GCLID capture. The module dependency chain is sound. **But there are structural gaps:** 1. **Missing migration for conversion retry columns (CRITICAL).** The `CreateWebhookEventsMigration.php` DDL creates the `profe_webhook_events` table with 9 columns. But the Repository and Handler code references `conversion_retry_count`, `conversion_retry_at`, and `conversion_payload` — none of which exist in the migration. There is no second migration adding them either (searched entire profe-modules tree for `ALTER TABLE` or `conversion_retry` — zero ALTERs found). The README correctly documents these columns, confirming they were intended. Deploying this will produce MySQL "Unknown column" errors on any conversion retry operation. 2. **No scheduler for retry processing.** `processConversionRetries()` is public and well-implemented (exponential backoff: 5min, 15min, 1hr, 6hr, 24hr), but nothing calls it. No cron job, no WP Cron hook, no scheduler registration. Without invocation, failed conversions silently sit in the database forever. 3. **T52's `uploadClickConversion()` is a stub.** The actual Google Ads PHP SDK call is commented out behind a simulated `sprintf()` return. The plan specifies "thin wrapper around the Google Ads PHP SDK" — this wrapper is not wired. Acceptable for Phase 1 scaffolding but noted. --- ### Security: PASS - **GCLID input validation** — Regex `/^[a-zA-Z0-9_-]+$/` applied to both cookie and URL-sourced GCLID. XSS in GCLID payload is blocked. - **CSRF enforcement** — CheckoutServiceModule's `checkAuthWithCsrf()` calls `requireValidNonce()` on both POST endpoints (create-session, success). - **Webhook route exempt** — `registerWebhookExemption()` calls `addExemptRoute('stripe-webhook')` per FAULT-004 requirement. - **Signature verification** — Uses Stripe SDK `\Stripe\Webhook::constructEvent()` via StripeServiceInterface. No hand-rolled HMAC. - **No quadruple backslashes** — FAULT-011 verification passes: 0 occurrences of quadruple backslashes across all 4 modules. - **API keys via ConfigManager** — Not hardcoded. Credentials validated via `isReady()`. - **No injection vectors** — Prepared statements in Repository. Email normalized (lowercase + trim + filter_var) before SHA-256. --- ### FAULT compliance: FAIL - **FAULT-006 (Missing migration columns): RECURS.** The original CreateWebhookEventsMigration (FAULT-006 corrected in Phase 1) created the table. But the new conversion retry feature adds 3 columns that have no migration. This is a recurrence of FAULT-006. - **FAULT-004 (CSRF): PASS** — Both checkout endpoints enforce CSRF. - **FAULT-005 (No tests): PASS** — Exceeds 12 tests per module. - **FAULT-008 (Injected dependencies): PASS** — GoogleAdsConversionInterface injected into WebhookHandler constructor. - **FAULT-011 (Quadruple backslash): PASS** — Zero occurrences. - **FAULT-017 (Exception location): PASS** — Single canonical location. --- ### Backwards compatibility: PASS - `uiMode` parameter defaults to `'hosted'` in both StripeServiceInterface and CheckoutServiceInterface. - `metadata` parameter defaults to `[]` in StripeServiceInterface. - Existing callers (checkout_service → stripe_service) work unchanged when omitting the new params. - Old Stripe Checkout redirect flow is preserved — embedded mode is additive. - The `getGclidFromCookie()` returns null when no Site Kit cookie or URL param is present, so existing sessions with no GCLID metadata are unaffected. - StripeService `createRefund()`, `retrieveSubscription()`, `syncSubscription()` all unchanged. --- ### Test quality: PASS Test counts across modules: | Module | Tests | Assertions | Quality | |--------|-------|------------|---------| | T52 GoogleAdsConversionServiceTest | 19 | ~50 | Real assertions on actual return values. Tests isReady() all 5 config keys, hashEmail normalization, uploadConversion resource name format, enhanced conversions disabled path, credential failure path, currency uppercasing. | | T52 GoogleAdsConversionModuleTest | 11 | ~20 | DI registration, interface compliance, boot/shutdown lifecycle. | | T52 GoogleAdsConversionExceptionTest | 9 | ~12 | Factory methods, message prefixing, code propagation. | | T17 WebhookHandlerTest | 16 | ~40 | Signature verification, duplicate detection, event routing (all 5 types), error marking. NEW: GCLID skip path, upload called with correct params, retry enqueue on failure, processConversionRetries success + failure paths. | | T15 CheckoutServiceTest | 18 | ~35 | Hosted mode, embedded mode returns client_secret, GCLID cookie pass-through, GCLID URL fallback, malformed GCLID rejection, event emission with uiMode+gclid. | | T15 StripeServiceTest | 10 | ~15 | Interface signature verification, uiMode default value, metadata default value, webhookSecret, user not found paths. | No false-positive tests. No try/catch swallowing in tests. All use `expectException()` or `assert*()` with real value checks. The `preg_match` test for malformed GCLID specifically verifies XSS `<script>` payloads are rejected. --- ### YMYL fitness: PASS (with caveats) **The good:** - Non-blocking conversion upload — webhook processing continues even if Google Ads API fails - Exponential backoff retry (5min → 15min → 1hr → 6hr → 24hr) - Dedup-safe: uses `$session->created` timestamp as the conversionDateTime for consistent dedup key - Both `GoogleAdsConversionException` and `\Throwable` caught in upload path, both enqueue retry - HTTP 500 returned for unhandled errors (not silent 200) - Stripe idempotency via UNIQUE KEY on `event_id` - Conversion data stored as JSON in `conversion_payload` for audit **The bad:** - **Retry mechanism won't work without the migration columns** — this is the showstopper - **Retry mechanism won't trigger without a scheduler** — `processConversionRetries()` exists but is never called - T52 SDK stub means no actual Google Ads API calls happen — conversion upload returns a simulated resource name --- ### Critical findings: 1. **BLOCKING: Migration missing 3 columns for conversion retry.** `conversion_retry_count`, `conversion_retry_at`, and `conversion_payload` are referenced by Repository code and documented in README but do not exist in any migration DDL. Deploying as-is will cause SQL errors. 2. **BLOCKING: No scheduler for `processConversionRetries()`.** The retry processor is never called — no cron hook, no WP Cron job, no scheduler registration. Failed conversions will never be retried. 3. **HIGH: T52 `uploadClickConversion()` is a stub.** The real Google Ads PHP SDK integration is commented out. The module cannot actually upload conversions to Google Ads. --- ### Verdict: REDLIGHT Reasoning: Two blocking issues. 1. **Migration columns missing** — The `profe_webhook_events` table DDL lacks the 3 columns that the entire conversion retry system depends on. Every `storeConversionRetry()`, `findPendingConversionRetries()`, `updateConversionRetry()`, and `clearConversionRetry()` call will fail with MySQL "Unknown column" errors. This makes the retry mechanism — a core reliability requirement from the SA's plan (Section 4.5 Idempotency & Reliability) — entirely non-functional. 2. **No retry scheduler** — Even if the migration were fixed, `processConversionRetries()` has no invocation mechanism. The plan explicitly requires "retry via a queue" with exponential backoff. The enqueue logic is correct, but the dequeue/processing is not wired. **These are fixable.** Required actions before GREENLIGHT: 1. **Create a migration** (either alter the existing `CreateWebhookEventsMigration.php` DDL, or create a new `AddConversionRetryColumnsMigration.php`) that adds: - `conversion_retry_count` TINYINT(3) UNSIGNED DEFAULT 0 - `conversion_retry_at` DATETIME NULL - `conversion_payload` JSON NULL 2. **Register the retry processor** — Either a WP Cron hook (`wp_schedule_event`) or a simple `add_action('init', ...)` that calls `processConversionRetries()`. The cron approach is preferred to avoid blocking web requests. 3. **Wire the real SDK call** in `uploadClickConversion()` or add a clear TODO/stub ticket for Phase 2. 4. **Add concurrency guard** — `SELECT ... FOR UPDATE` on the retry query, or at minimum `LOCK IN SHARE MODE`, to prevent double-processing if multiple workers run simultaneously.