Memory Wiki

Let me read the remaining portions of the files.

Now I have full context on all three briefs and the GS review. Let me make all the fixes. **T17 — Fixes needed:** 1. `catch (\Throwable)` in `uploadConversionToGoogleAds()` must enqueue retry (Issue 2/CRITICAL) 2. Error handling table needs row for Throwable → enqueue retry 3. Test requirements need test for Throwable retry enqueue 4. Add note about `handleStripeWebhook()` returning 500 on Throwable (Issue 1/CRITICAL) 5. Add note about StripeService delegation (Issue 3/CRITICAL) 6. Add note about `getWebhookSecret()` missing from interface (Issue 9) 7. Fix test count header mismatch (GS T17-03) **T15 — Fixes needed:** 1. Replace false-positive test specs with real code-path tests (Issues 4-5/CRITICAL) 2. Add GET gclid regex validation (Issue 8) 3. Add note about `getWebhookSecret()` gap (Issue 9) **T52 — Fixes needed:** 1. Fix weak test assertions to verify actual data passed to stub API (Issue 6/HIGH) 2. Add `enhanced_conversions = false` test (Issue 7/HIGH) 3. Add dead catch/rethrow fix note (Issue 10) Let me start with T17:

Good — the first occurrence at line 182 needs the retry enqueue added. Let me use enough surrounding context to make it unique.

Good. Now let me update the error handling table in T17 section 7 to add the Throwable and fix the "All 6 retries exhausted" row. Then update the test requirements. I'll also add the new sections for the other critical issues.

Now update the test requirements — fix count header, add test for Throwable retry enqueue, handleWebhook 500 response, and StripeService delegation:

T17 done. Now moving to T15. Let me fix the false-positive tests, add GET gclid regex validation, and add `getWebhookSecret()` gap note. First, the GET gclid regex validation fix:

Now replace all the bad test specs in T15 section 8 with real code-path tests:

T15 done. Now updating T52 — fixing weak assertions, adding `enhanced_conversions = false` test, fixing dead catch/rethrow:

All three briefs updated. Let me verify the changes look correct by spot-checking each file:

Let me verify the key sections are correct in each updated file:

All changes verified. Here is a summary of everything done: --- ## Summary ### Files Modified 1. **`/root/.hermes/plans/task-t17-conversion.md`** — Webhook Handler (CRITICAL fixes) 2. **`/root/.hermes/plans/task-t15-embedded-checkout.md`** — Embedded Checkout (CRITICAL fixes) 3. **`/root/.hermes/plans/task-t52-build-plan.md`** — Google Ads Conversion (HIGH fixes) --- ### T17 — What Changed | Issue | Fix | |-------|-----| | **1. Silent 200 on Throwable** (CRITICAL) | Added error handling table row: `handleWebhook()` must return HTTP 500 (not 200) on unhandled `Throwable` so Stripe retries. Added test `testHandleWebhookReturns500OnThrowable()` | | **2. Non-GAS exception data loss** (CRITICAL) | Added `$this->enqueueConversionRetry(...)` call inside the `catch (\Throwable)` block. Previously only logged — retry was missing, permanently losing conversions. Added error table row documenting this. Added test `testConversionUploadRetryEnqueuedOnThrowable()` | | **3. Stripe SDK bypass** (CRITICAL) | Added error table row: ALL Stripe SDK calls must delegate through `$this->stripeService`. Added test `testHandleInvoicePaidUsesStripeService()` | | **9. `getWebhookSecret()` gap** (MEDIUM) | Added test section `StripeServiceInterface Gap Tests` with `testGetWebhookSecretMethodExistsOnStripeService()` | | **Test count header mismatch** (GS T17-03) | Fixed header from "Minimum 5 New Tests" to "Minimum 11 New Tests + 2 Existing Test Updates = 13 Total Changes" | ### T15 — What Changed | Issue | Fix | |-------|-----| | **4. False-positive tests calling non-existent methods** (CRITICAL) | Replaced entire test section with real code-path tests. New header: "Minimum 16 New Tests — All Real Code Paths, No False Positives". Added **GS FIX** rules: no reflection-on-private-methods, no try/catch-all patterns | | **5. Try/catch-all test pattern** (CRITICAL) | Every test now has a **Real Assertion Pattern** column showing exact assertion calls (`assertSame`, `assertArrayNotHasKey`, `$this->expectException(CheckoutServiceException::class)`, etc.) | | **8. GET gclid path lacks regex validation** (MEDIUM) | Added `preg_match('/^[a-zA-Z0-9_-]+$/', $gclid)` validation to the `$_GET['gclid']` fallback path (same as cookie path). Added test `testCreateSessionGclidFromUrlRejectsInvalid()` with malicious input | | **9. `getWebhookSecret()` gap** (MEDIUM) | Added 2 tests: `testGetWebhookSecretMethodExists()` and `testGetWebhookSecretReturnsConfiguredValue()` | ### T52 — What Changed | Issue | Fix | |-------|-----| | **6. Weak test assertions** (HIGH) | All tests now have **Real Assertion Pattern** column with specific data verification. Test 9 captures the `ClickConversion` object and asserts exact field values (`gclid`, `conversion_value`, `currency_code`, etc.). Test 18 asserts exact resource name string instead of just `is_string()` | | **7. `enhanced_conversions = false` untested** (HIGH) | Added test 14: `testUploadConversionSkipsEnhancedConversionsWhenDisabled()` — verifies `UserIdentifier` is NOT attached when config key is `false` even with non-null `hashedEmail` | | **10. Dead catch/rethrow** (MEDIUM) | Added test 13: `testUploadConversionThrowsOnNonGoogleException()` — mocks `RuntimeException` (non-GAS), asserts `GoogleAdsConversionException` IS thrown, proving the catch block is NOT dead code |