Chore/tavanogui

[tlgimenes]

What is this Contribution About?

Please provide a brief description of the changes or enhancements you are proposing in this pull request.

Issue Link

Please link to the relevant issue that this pull request addresses:

• Issue: #ISSUE_NUMBER

Loom Video

Record a quick screencast describing your changes to help the team understand and review your contribution. This will greatly assist in the review process.

Demonstration Link

Provide a link to a branch or environment where this pull request can be tested and seen in action.

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Improved shopping cart validation to ensure consistency between stored cart data and user identity.
  • Enhanced cart cookie handling to properly clear invalid carts when needed.

• Performance

  • Added cache optimization headers for checkout pages to improve response handling.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The PR modifies VTEX cart and cookie handling to propagate a shouldClearCartCookie decision through the request/response pipeline. When a mismatched cart is detected in the loader, this flag is returned and used to conditionally clear the checkout cookie via proxySetCookie. Checkout-related proxy routes now include Vary headers for caching.

Changes

Cohort / File(s)	Summary
Cart mismatch detection vtex/loaders/cart.ts	Modified logMismatchedCart to return an explicit object { shouldClearCartCookie: boolean } in all branches instead of sometimes returning undefined. Email comparison logic shifted from type-guarded checks to truthiness checks. Loader updated to capture the returned shouldClearCartCookie value and pass it to proxySetCookie.
Cookie clearing implementation vtex/utils/cookies.ts	Added optional options parameter to proxySetCookie. When options.shouldClearCartCookie is true and the cookie name is "checkout.vtex.com", the function now clears that cookie by setting it with empty value and zero expiration.
Checkout response headers vtex/loaders/proxy.ts	Added conditional customHeaders property to proxy route handlers. Routes with "/checkout" in pathTemplate now include Vary header with Cookie and Accept-Encoding; other routes use empty customHeaders.

Sequence Diagram

sequenceDiagram
    participant Loader as Loader
    participant LogMismatch as logMismatchedCart
    participant Cookie as proxySetCookie
    participant Response as Response

    Loader->>LogMismatch: Check cart mismatch
    alt Cart mismatch detected
        LogMismatch->>LogMismatch: Log mismatch
        LogMismatch-->>Loader: { shouldClearCartCookie: true }
    else No mismatch
        LogMismatch-->>Loader: { shouldClearCartCookie: false }
    end
    
    Loader->>Cookie: proxySetCookie(..., { shouldClearCartCookie })
    alt shouldClearCartCookie && cookie is checkout.vtex.com
        Cookie->>Response: Set cookie with empty value<br/>(expires/maxAge = 0)
    else
        Cookie->>Response: Set cookie normally
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• vtex/loaders/cart.ts: Logic change in mismatch detection with email comparison shift to truthiness checks—verify this doesn't unintentionally broaden mismatch detection and review the return value propagation.
• vtex/utils/cookies.ts: Cookie clearing implementation—confirm the empty value and zero expiration correctly clear the checkout cookie across browsers.
• Control flow integration: Ensure shouldClearCartCookie flag correctly propagates from loader detection through cookie clearing in the response pipeline.

Possibly related PRs

• chore: add logs to cart #1482: Introduced the logMismatchedCart logging helper that this PR refactors to return a decision flag for cookie clearing.
• Deduplicate vtex cookie #1480: Related VTEX cart cookie handling modifications that likely coordinate with these cookie-clearing changes.

Suggested reviewers

• guitavano

Poem

🐰 Mismatched carts now send a call,
A flag hops through the code so tall,
"Should I clear?" the cookie asks with glee,
Empty value, zero—gone it be!
Vary headers guard the checkout way,
Fresher responses saved the day! 🍪✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is entirely blank with only the template placeholders filled in; no actual contribution details, issue links, video, or demo information are provided.	Fill in the template sections with actual details: describe the changes made, link the relevant issue, provide a Loom video, and include a demonstration link.
Title check	❓ Inconclusive	The title 'Chore/tavanogui' is vague and generic, using a branch name format rather than describing the actual changes made to the codebase.	Replace with a descriptive title that summarizes the main change, such as 'Handle cart cookie clearing logic with explicit return types' or similar.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch chore/tavanogui

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

• Provide your own instructions using the high_level_summary_instructions setting.
• Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
• Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
4. 📊 Contributor Summary — Include a Markdown table showing contributions:
   | Contributor | Lines Added | Lines Removed | Files Changed |
5. ✔️ Additional Notes — Add any extra reviewer context.
   Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Tagging Options

Should a new tag be published when this PR is merged?

• 👍 for Patch 0.133.7 update
• 🎉 for Minor 0.134.0 update
• 🚀 for Major 1.0.0 update

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

vtex/utils/cookies.ts (1)

12-40: Tighten cookie‑clearing attributes (domain/path reuse) for reliability

The shouldClearCartCookie flow looks correct and the signature change is backward compatible, but the clear branch can be a bit safer:

• When toDomain is not provided, domain is forced to "", instead of reusing cookie.domain or omitting domain entirely.
• path is hard‑coded to "/", which assumes the original checkout.vtex.com cookie always uses that path. If VTEX ever ships it with a different path, this clear may not take effect.

You can reuse the derived newCookie as a base to avoid drifting from the original attributes:

-    if(cookie.name === "checkout.vtex.com" && options?.shouldClearCartCookie) {
-     
-      setCookie(to, {
-        name: "checkout.vtex.com",
-        value: "",
-        expires: new Date(0),
-        maxAge: 0,
-        path: "/",
-        secure: true,
-        httpOnly: true,
-        domain: newDomain ? newDomain?.hostname : "",
-      });
-    } else {
-      setCookie(to, newCookie);
-    }
+    if (cookie.name === "checkout.vtex.com" && options?.shouldClearCartCookie) {
+      setCookie(to, {
+        ...newCookie,
+        value: "",
+        expires: new Date(0),
+        maxAge: 0,
+      });
+    } else {
+      setCookie(to, newCookie);
+    }

This keeps domain/path/flags aligned with whatever VTEX sends while still clearing the cookie.

🧹 Nitpick comments (1)

vtex/loaders/cart.ts (1)

109-115: Update comment to reflect side‑effect (not just “temporary logging”)

logMismatchedCart is now used to decide whether to clear the cart cookie, not just for “temporary logging”:

// Temporary logging to check for cart mismatch
const { shouldClearCartCookie } = logMismatchedCart(cart, req, ctx);
proxySetCookie(..., { shouldClearCartCookie });

Consider updating the comment (or removing it) to avoid misleading future readers into thinking this is non‑behavioral telemetry.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4fae26e and 9a8a83b.

📒 Files selected for processing (3)

• vtex/loaders/cart.ts (3 hunks)
• vtex/loaders/proxy.ts (1 hunks)
• vtex/utils/cookies.ts (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

vtex/loaders/cart.ts (1)

vtex/utils/cookies.ts (1)

• proxySetCookie (8-42)

🔇 Additional comments (2)

vtex/loaders/cart.ts (1)

28-74: Semantics of when shouldClearCartCookie is true look coherent

The refactor of logMismatchedCart to always return { shouldClearCartCookie: boolean } is clean, and limiting true to the explicit email‑mismatch case makes the behavior predictable. Early exits on missing auth cookie or JWT parse errors returning false are reasonable defaults.

If product requirements ever change (e.g., “any suspicious auth cookie issue should trigger a cart reset”), you now have a single place to adjust that policy, which is nice. No functional issues spotted here.

vtex/loaders/proxy.ts (1)

80-85: Handler confirms Vary header will overwrite, not merge—but no conflict in this change

Verification confirms your concern: the proxy handler applies customHeaders via headers.set(key, value) (line 146 in website/handlers/proxy.ts), which overwrites instead of merging. Only the cookie header has special append logic (line 141).

However, this doesn't create a practical issue here:

• The headers object is freshly initialized from incoming request headers, which typically don't include Vary (it's a response header).
• No other code in vtex/loaders/proxy.ts sets customHeaders with a conflicting Vary header.
• The added Vary: Cookie, Accept-Encoding for /checkout paths is applied cleanly without overwriting anything.

The architectural point stands (handler could benefit from Vary header merging like it does for cookies), but this specific change is safe to proceed with.