Skip to content

Next#1192

Open
guitavano wants to merge 11 commits into
mainfrom
next
Open

Next#1192
guitavano wants to merge 11 commits into
mainfrom
next

Conversation

@guitavano

@guitavano guitavano commented May 15, 2026

Copy link
Copy Markdown
Contributor

Summary by cubic

Stabilizes the deco_segment cookie to cut Set-Cookie churn and improve HTML edge cache hits. Makes /live/_meta cacheable to reduce origin load. Updates packages to 1.197.1-next.2.

  • Performance

    • Persist only active in deco_segment; write cookie only when it changes.
    • Set /live/_meta cache-control to public, max-age=60, s-maxage=60, stale-while-revalidate=300.
  • Dependencies

    • Bump @deco/deco, @deco/dev, and @deco/scripts to 1.197.1-next.2.

Written for commit 0347fbf. Summary will update on new commits. Review in cubic

Summary by CodeRabbit

  • Chores

    • Bumped package version to 1.197.1-next.2 across configuration files
  • Documentation

    • Updated repository contribution guide wording
  • Bug Fixes

    • Optimized segment tracking and cookie persistence behavior
    • Enhanced cache control headers for improved performance

Review Change Stack

vibe-dex and others added 6 commits May 14, 2026 16:08
E2E test commit for the new next-channel prerelease pipeline.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…_meta

Two changes to recover edge cache hit rates that have been quietly degrading
HTML cacheability across deco-cx storefronts.

1. **Segment cookie thrash (runtime/middleware.ts).** The `deco_segment` cookie
   tracked two sets — `active` (currently-active segment flags) and
   `inactiveDrawn` (a write-only bookkeeping set of flags previously evaluated
   to `false`). `inactiveDrawn` was never read by any matcher logic across
   this entire repo. Its only effect was to grow as users navigated across
   pages drawing different matchers, churning the cookie value, which tripped
   the `Set-Cookie => Cache-Control: no-store` kill-switch a few lines below
   on essentially every request.

   Live measurement against farmrio.com.br (deco@1.197.0): PDP HTML
   cache hit rate ~1%, other HTML ~28% — origin was serving ~55 GiB/day of
   HTML that should have been edge-cached.

   The fix: persist only `active` in the cookie, and only re-write the cookie
   when `active` actually changes. Cookies in the wild that still carry the
   old `{active, inactiveDrawn}` shape are read for `active` only; the
   `inactiveDrawn` field on disk is harmless and gets dropped on the next
   real cohort change.

2. **/live/_meta cache-control (runtime/routes/_meta.ts).** Was
   `must-revalidate`, which Cloudflare honors as effectively no-store. The
   admin UI polls this endpoint every ~30s; serving it as
   `public, max-age=60, s-maxage=60, stale-while-revalidate=300, must-revalidate`
   collapses upstream load (~370 MiB/day at farmrio alone, more across the
   fleet) without affecting admin freshness — ETag still drives 304s.

Validation:
- Replayed homepage GET with a returning-user `deco_segment` cookie: HIT
  with no Set-Cookie response header (previously: BYPASS, new cookie).
- Verified PDP first-visit still emits the cookie (one-time per cohort
  change), while subsequent navigations no longer re-emit it.
- /live/_meta now reports `cf-cache-status: HIT` after first warmup; ETag
  unchanged.

Risk: low. `inactiveDrawn` is grep-clean across this repo, deco-cx/apps,
and the farmrio storefront. Worst case for an external consumer reading it:
they see an empty/missing field and treat the user as if they hadn't been
drawn into any inactive segment, which is the same behavior as a
first-visit user — i.e. the existing first-visit path.

Companion PRs:
- deco-cx/apps: cacheControlOverride for proxy + cacheable=true on
  website/matchers/userAgent.ts
- deco-sites/farmrio: PRIVATE_COOKIES allowlist in routes/_middleware.ts

Co-authored-by: Cursor <cursoragent@cursor.com>
…eta-tll

perf(runtime): keep deco_segment stable; cache /live/_meta
@github-actions

Copy link
Copy Markdown
Contributor

Tagging Options

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

  • 👍 for Patch 1.197.1 update
  • 🎉 for Minor 1.198.0 update
  • 🚀 for Major 2.0.0 update

@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown

Caution

Review failed

Failed to post review comments

📝 Walkthrough

Walkthrough

This PR bumps the package version to 1.197.1-next.2 across manifests, updates CONTRIBUTING.md documentation wording, refactors the deco_segment cookie logic to persist only the active segment set, and expands the metadata route's cache-control directives for improved caching behavior.

Changes

Release and runtime updates

Layer / File(s) Summary
Version bumps and documentation wording
deno.json, dev/deno.json, scripts/deno.json, CONTRIBUTING.md
Version field updated from 1.197.0 to 1.197.1-next.2 in three manifest files; release channels documentation updated from "ships" to "publishes".
Segment cookie active-only refactor
runtime/middleware.ts
deco_segment cookie computation now snapshots prior active set, mutates active set from flags where isSegment applies, serializes both lists, and writes cookie only when active set is non-empty and changed—removing prior inactiveDrawn tracking.
Metadata route cache-control expansion
runtime/routes/_meta.ts
JSON metadata route Cache-Control header expanded from "must-revalidate" to "public, max-age=60, s-maxage=60, stale-while-revalidate=300, must-revalidate" for enhanced public caching.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • deco-cx/deco#1186: Both PRs modify runtime/middleware.ts to refactor deco_segment cookie persistence to track only the active set and adjust runtime/routes/_meta.ts cache-control headers.
  • deco-cx/deco#1031: Both PRs update version fields in deno.json, dev/deno.json, and scripts/deno.json for release versioning.
  • deco-cx/deco#1198: Both PRs refine runtime/middleware.ts segment cookie logic with further restrictions on segment mapping and cookie write conditions.

Suggested reviewers

  • igoramf

Poem

🐰 Versions bump up, docs now speak true,
Segments simplify, cookies renew,
Cache headers bloom with max-age delight,
The metadata route now caches just right! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Next' is vague and generic, providing no meaningful information about the changeset's primary purpose or content. Consider a more descriptive title that reflects the main change, such as 'Enable CDN caching for matcher-wrapped pages and stabilize deco_segment cookie' or 'Prepare next prerelease with caching improvements.'
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch next
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch next

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (1)
runtime/middleware.ts (1)

409-423: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clear deco_segment when the active set becomes empty.

When the last segment is removed, activeChanged is true but Line 409 skips the write, so the browser keeps sending the previous non-empty cookie. On the next request previousActive is rebuilt from that stale value and the user stays pinned to the old cohort.

Proposed fix
-        if (active.size > 0 && activeChanged) {
-          const value = JSON.stringify({ active: newActive });
-          const date = new Date();
-          date.setTime(date.getTime() + 30 * 24 * 60 * 60 * 1000); // 1 month
-          setCookie(
-            newHeaders,
-            {
-              name: DECO_SEGMENT,
-              value,
-              path: "/",
-              expires: date,
-              sameSite: "Lax",
-            },
-            { encode: true },
-          );
+        if (activeChanged) {
+          const expires = newActive.length > 0
+            ? new Date(Date.now() + 30 * 24 * 60 * 60 * 1000)
+            : new Date(0);
+          setCookie(
+            newHeaders,
+            {
+              name: DECO_SEGMENT,
+              value: newActive.length > 0
+                ? JSON.stringify({ active: newActive })
+                : "",
+              path: "/",
+              expires,
+              sameSite: "Lax",
+            },
+            { encode: true },
+          );
         }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@runtime/middleware.ts` around lines 409 - 423, When activeChanged is true but
active.size is 0 the code currently skips writing the cookie so the browser
keeps the old deco_segment; update the logic around DECO_SEGMENT (the block that
currently runs when active.size > 0) to also handle the case active.size === 0
&& activeChanged by clearing the cookie on newHeaders (use setCookie with
DECO_SEGMENT and an empty value and an immediate expiry or equivalent removal
options) so the stale cookie is removed and previousActive cannot be rebuilt
from it; keep references to newActive, activeChanged, setCookie and DECO_SEGMENT
when implementing the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@runtime/routes/_meta.ts`:
- Around line 24-25: The Cache-Control header string currently includes both
"must-revalidate" and "stale-while-revalidate=300", which conflict; update the
"cache-control" header value (the "cache-control" property in this module) to
remove "must-revalidate" so it becomes "public, max-age=60, s-maxage=60,
stale-while-revalidate=300" (or remove the stale directive instead if strict
revalidation is desired).

---

Outside diff comments:
In `@runtime/middleware.ts`:
- Around line 409-423: When activeChanged is true but active.size is 0 the code
currently skips writing the cookie so the browser keeps the old deco_segment;
update the logic around DECO_SEGMENT (the block that currently runs when
active.size > 0) to also handle the case active.size === 0 && activeChanged by
clearing the cookie on newHeaders (use setCookie with DECO_SEGMENT and an empty
value and an immediate expiry or equivalent removal options) so the stale cookie
is removed and previousActive cannot be rebuilt from it; keep references to
newActive, activeChanged, setCookie and DECO_SEGMENT when implementing the
change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 283a4cce-36a2-490e-9893-f6f697bf4eb2

📥 Commits

Reviewing files that changed from the base of the PR and between 18d9459 and c40aff8.

📒 Files selected for processing (6)
  • CONTRIBUTING.md
  • deno.json
  • dev/deno.json
  • runtime/middleware.ts
  • runtime/routes/_meta.ts
  • scripts/deno.json

Comment thread runtime/routes/_meta.ts
Comment on lines +24 to +25
"cache-control":
"public, max-age=60, s-maxage=60, stale-while-revalidate=300, must-revalidate",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

HTTP cache-control must-revalidate and stale-while-revalidate compatibility

💡 Result:

The Cache-Control directives must-revalidate and stale-while-revalidate serve fundamentally different purposes and are generally considered incompatible when used together, as they impose conflicting requirements on how a cache must handle stale content [1][2][3]. must-revalidate The must-revalidate directive is a strict instruction that prohibits a cache from serving a stale response without first successfully revalidating it with the origin server [1][2][3]. If the cache is unable to reach the origin (e.g., due to network disconnection), it must return an error (typically a 504 Gateway Timeout) rather than serving the stale content [2][3][4]. This directive is intended for scenarios where serving stale data could lead to incorrect operations, such as financial transactions [2][3][4]. stale-while-revalidate The stale-while-revalidate directive is designed to improve performance by allowing a cache to serve a stale response immediately while simultaneously revalidating it in the background [1][5][6]. This directive explicitly permits the use of stale content to avoid blocking the user, prioritizing availability and latency over strict freshness [5][6]. Compatibility and Interaction When both directives are present in the same Cache-Control header, they create a conflict: - must-revalidate mandates that stale content must not be served until revalidation is complete [2][3]. - stale-while-revalidate permits stale content to be served immediately while revalidation happens in the background [5][6]. In practice, because must-revalidate is a strict requirement that caches are mandated not to ignore [2][3][4], it effectively overrides the permissive behavior of stale-while-revalidate. A compliant cache will prioritize the must-revalidate constraint, forcing the client to wait for the revalidation to complete before receiving a response, thereby negating the performance benefits of stale-while-revalidate [1][2][3]. Consequently, these directives should not be used together if the goal is to leverage the background revalidation capabilities of stale-while-revalidate [1].

Citations:


🏁 Script executed:

cat -n runtime/routes/_meta.ts | sed -n '20,30p'

Repository: deco-cx/deco

Length of output: 414


Remove must-revalidate to enable stale-while-revalidate behavior.

The cache-control header combines must-revalidate and stale-while-revalidate=300, which are conflicting directives. Per RFC 9111:

  • must-revalidate strictly prohibits serving stale content without successful revalidation
  • stale-while-revalidate=300 permits serving stale content for 300 seconds while revalidating in the background

When both are present, must-revalidate takes precedence, effectively nullifying the stale-while-revalidate optimization. The 300-second stale window will never be used.

If the intent is to allow stale responses during revalidation (performance optimization), remove must-revalidate. If strict freshness is required, remove stale-while-revalidate=300.

Proposed fix (assuming stale-while-revalidate is intended)
      "cache-control":
-        "public, max-age=60, s-maxage=60, stale-while-revalidate=300, must-revalidate",
+        "public, max-age=60, s-maxage=60, stale-while-revalidate=300",
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"cache-control":
"public, max-age=60, s-maxage=60, stale-while-revalidate=300, must-revalidate",
"cache-control":
"public, max-age=60, s-maxage=60, stale-while-revalidate=300",
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@runtime/routes/_meta.ts` around lines 24 - 25, The Cache-Control header
string currently includes both "must-revalidate" and
"stale-while-revalidate=300", which conflict; update the "cache-control" header
value (the "cache-control" property in this module) to remove "must-revalidate"
so it becomes "public, max-age=60, s-maxage=60, stale-while-revalidate=300" (or
remove the stale directive instead if strict revalidation is desired).

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 6 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="runtime/routes/_meta.ts">

<violation number="1" location="runtime/routes/_meta.ts:24">
P2: This response is now publicly cacheable while CORS headers vary by request origin, but `Vary: Origin` is missing. Add `Vary: Origin` (or avoid shared caching) to prevent cached cross-origin header mismatches.</violation>

<violation number="2" location="runtime/routes/_meta.ts:25">
P2: `must-revalidate` and `stale-while-revalidate=300` are conflicting directives. Per RFC 9111, `must-revalidate` strictly prohibits serving stale content, which nullifies the `stale-while-revalidate` optimization that allows serving stale responses during background revalidation. The 300-second stale window will never be used. Remove `must-revalidate` if the intent is to benefit from stale-while-revalidate, or remove `stale-while-revalidate=300` if strict freshness is required.</violation>
</file>

<file name="runtime/middleware.ts">

<violation number="1" location="runtime/middleware.ts:409">
P1: When all active segment flags are removed, the cookie is never updated/cleared, leaving stale `deco_segment` state.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic

Comment thread runtime/middleware.ts
const activeChanged =
JSON.stringify(previousActive) !== JSON.stringify(newActive);

if (active.size > 0 && activeChanged) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When all active segment flags are removed, the cookie is never updated/cleared, leaving stale deco_segment state.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At runtime/middleware.ts, line 409:

<comment>When all active segment flags are removed, the cookie is never updated/cleared, leaving stale `deco_segment` state.</comment>

<file context>
@@ -390,27 +390,24 @@ export const middlewareFor = <TAppManifest extends AppManifest = AppManifest>(
+        const activeChanged =
+          JSON.stringify(previousActive) !== JSON.stringify(newActive);
+
+        if (active.size > 0 && activeChanged) {
+          const value = JSON.stringify({ active: newActive });
           const date = new Date();
</file context>

Comment thread runtime/routes/_meta.ts
Comment on lines +24 to +25
"cache-control":
"public, max-age=60, s-maxage=60, stale-while-revalidate=300, must-revalidate",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This response is now publicly cacheable while CORS headers vary by request origin, but Vary: Origin is missing. Add Vary: Origin (or avoid shared caching) to prevent cached cross-origin header mismatches.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At runtime/routes/_meta.ts, line 24:

<comment>This response is now publicly cacheable while CORS headers vary by request origin, but `Vary: Origin` is missing. Add `Vary: Origin` (or avoid shared caching) to prevent cached cross-origin header mismatches.</comment>

<file context>
@@ -21,7 +21,8 @@ export const handler = createHandler(async (ctx) => {
     headers: {
       "Content-Type": "application/json",
-      "cache-control": "must-revalidate",
+      "cache-control":
+        "public, max-age=60, s-maxage=60, stale-while-revalidate=300, must-revalidate",
       etag,
</file context>
Suggested change
"cache-control":
"public, max-age=60, s-maxage=60, stale-while-revalidate=300, must-revalidate",
"cache-control":
"public, max-age=60, s-maxage=60, stale-while-revalidate=300, must-revalidate",
vary: "Origin",

Comment thread runtime/routes/_meta.ts
"Content-Type": "application/json",
"cache-control": "must-revalidate",
"cache-control":
"public, max-age=60, s-maxage=60, stale-while-revalidate=300, must-revalidate",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: must-revalidate and stale-while-revalidate=300 are conflicting directives. Per RFC 9111, must-revalidate strictly prohibits serving stale content, which nullifies the stale-while-revalidate optimization that allows serving stale responses during background revalidation. The 300-second stale window will never be used. Remove must-revalidate if the intent is to benefit from stale-while-revalidate, or remove stale-while-revalidate=300 if strict freshness is required.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At runtime/routes/_meta.ts, line 25:

<comment>`must-revalidate` and `stale-while-revalidate=300` are conflicting directives. Per RFC 9111, `must-revalidate` strictly prohibits serving stale content, which nullifies the `stale-while-revalidate` optimization that allows serving stale responses during background revalidation. The 300-second stale window will never be used. Remove `must-revalidate` if the intent is to benefit from stale-while-revalidate, or remove `stale-while-revalidate=300` if strict freshness is required.</comment>

<file context>
@@ -21,7 +21,8 @@ export const handler = createHandler(async (ctx) => {
       "Content-Type": "application/json",
-      "cache-control": "must-revalidate",
+      "cache-control":
+        "public, max-age=60, s-maxage=60, stale-while-revalidate=300, must-revalidate",
       etag,
       ...allowCorsFor(ctx.req.raw),
</file context>
Suggested change
"public, max-age=60, s-maxage=60, stale-while-revalidate=300, must-revalidate",
"public, max-age=60, s-maxage=60, stale-while-revalidate=300",

vibe-dex and others added 3 commits May 26, 2026 23:31
Drop `Vary: cookie` from sticky-session matchers and stop treating
framework-managed Set-Cookies (deco_matcher_*, deco_segment) as a
reason to disable caching. Wire ctx.var.vary.shouldCache into the
full-page kill-switch so loaders that declare cache:"no-store"
(personalizing loaders) still veto caching.

Adds a Deco-Cache-Vary-Cookies hint header so CDN operators can
discover which cookies belong in the custom cache key.

Extracts applyPageCacheDecision() from the inlined kill-switch as
an exported pure function; the request middleware is the only
production caller. Adds tests for matcher.ts and middleware.ts —
the first tests in blocks/ and for runtime/middleware.ts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ty-next

feat(matcher): make matcher-wrapped pages cacheable at CDN edge

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

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

⚠️ Outside diff range comments (1)
runtime/middleware.ts (1)

464-466: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Fix type error: decodeCookie receives string | undefined.

The pipeline fails because currentCookies[DECO_SEGMENT] may be undefined when the cookie doesn't exist, but decodeCookie expects a string.

🐛 Proposed fix
       const cookieSegment = tryOrDefault(
-        () => decodeCookie(currentCookies[DECO_SEGMENT]),
+        () => decodeCookie(currentCookies[DECO_SEGMENT] ?? ""),
         "",
       );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@runtime/middleware.ts` around lines 464 - 466, The call to decodeCookie is
receiving currentCookies[DECO_SEGMENT] which can be undefined; change the lambda
passed to tryOrDefault so decodeCookie always gets a string (e.g., use a
nullish-coalescing fallback like currentCookies[DECO_SEGMENT] ?? "" or guard
with a ternary) to satisfy decodeCookie's string parameter and return the
empty-string default when the cookie is missing; locate this in the same
expression that uses tryOrDefault, decodeCookie, currentCookies and
DECO_SEGMENT.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@runtime/middleware.ts`:
- Around line 464-466: The call to decodeCookie is receiving
currentCookies[DECO_SEGMENT] which can be undefined; change the lambda passed to
tryOrDefault so decodeCookie always gets a string (e.g., use a
nullish-coalescing fallback like currentCookies[DECO_SEGMENT] ?? "" or guard
with a ternary) to satisfy decodeCookie's string parameter and return the
empty-string default when the cookie is missing; locate this in the same
expression that uses tryOrDefault, decodeCookie, currentCookies and
DECO_SEGMENT.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c1778f29-e042-486a-8128-14d52375a785

📥 Commits

Reviewing files that changed from the base of the PR and between c40aff8 and 063f2c5.

📒 Files selected for processing (7)
  • blocks/matcher.test.ts
  • blocks/matcher.ts
  • deno.json
  • dev/deno.json
  • runtime/middleware.test.ts
  • runtime/middleware.ts
  • scripts/deno.json
💤 Files with no reviewable changes (1)
  • blocks/matcher.ts
✅ Files skipped from review due to trivial changes (2)
  • scripts/deno.json
  • dev/deno.json

vibe-dex added 2 commits May 26, 2026 23:57
…kie-safety-next"

This reverts commit a7aecde, reversing
changes made to c40aff8.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found across 7 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="dev/deno.json">

<violation number="1" location="dev/deno.json:3">
P1: Version regression in `dev/deno.json`: changed from `1.200.1-next.1` to `1.197.1-next.2`, but the PR intent is to bump to `1.200.1-next.1`. This downgrade will cause `@deco/dev` to be published at an older version than intended, creating inconsistency with sibling packages.</violation>
</file>

<file name="scripts/deno.json">

<violation number="1" location="scripts/deno.json:3">
P2: Version downgraded from 1.200.1-next.1 to 1.197.1-next.2, which contradicts the PR's stated goal of bumping to 1.200.1-next.1. This may be a merge/revert artifact that left scripts/deno.json at an older version than intended.</violation>
</file>

<file name="blocks/matcher.ts">

<violation number="1">
P1: Vary: cookie should be set unconditionally for sticky matcher responses, not only when the cookie value changes. Placing it inside the conditional `if (result !== isMatchFromCookie)` means responses where the cookie already matches will lack the Vary header, causing inconsistent CDN cache key behavior — a subsequent request with a different cookie might hit a cached response that was stored without Vary: cookie.</violation>
</file>

<file name="runtime/middleware.ts">

<violation number="1">
P1: Treating all `Set-Cookie` headers as cache-disqualifying also blocks caching for framework segment cookies.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread dev/deno.json
{
"name": "@deco/dev",
"version": "1.197.0",
"version": "1.197.1-next.2",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Version regression in dev/deno.json: changed from 1.200.1-next.1 to 1.197.1-next.2, but the PR intent is to bump to 1.200.1-next.1. This downgrade will cause @deco/dev to be published at an older version than intended, creating inconsistency with sibling packages.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/deno.json, line 3:

<comment>Version regression in `dev/deno.json`: changed from `1.200.1-next.1` to `1.197.1-next.2`, but the PR intent is to bump to `1.200.1-next.1`. This downgrade will cause `@deco/dev` to be published at an older version than intended, creating inconsistency with sibling packages.</comment>

<file context>
@@ -1,6 +1,6 @@
 {
   "name": "@deco/dev",
-  "version": "1.200.1-next.1",
+  "version": "1.197.1-next.2",
   "exports": {
     "./tailwind": "./tailwind.ts"
</file context>
Suggested change
"version": "1.197.1-next.2",
"version": "1.200.1-next.1",

Comment thread runtime/middleware.ts
@@ -390,27 +390,24 @@ export const middlewareFor = <TAppManifest extends AppManifest = AppManifest>(
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Treating all Set-Cookie headers as cache-disqualifying also blocks caching for framework segment cookies.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At runtime/middleware.ts, line 432:

<comment>Treating all `Set-Cookie` headers as cache-disqualifying also blocks caching for framework segment cookies.</comment>

<file context>
@@ -501,16 +424,29 @@ export const middlewareFor = <TAppManifest extends AppManifest = AppManifest>(
-        shouldCacheFromVary: ctx.var?.vary?.shouldCache !== false,
-      });
+
+      if (hasSetCookie) {
+        // Set-cookie present: never cache (same behavior as main)
+        newHeaders.set("Cache-Control", "no-store, no-cache, must-revalidate");
</file context>

Comment thread scripts/deno.json
{
"name": "@deco/scripts",
"version": "1.197.0",
"version": "1.197.1-next.2",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Version downgraded from 1.200.1-next.1 to 1.197.1-next.2, which contradicts the PR's stated goal of bumping to 1.200.1-next.1. This may be a merge/revert artifact that left scripts/deno.json at an older version than intended.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/deno.json, line 3:

<comment>Version downgraded from 1.200.1-next.1 to 1.197.1-next.2, which contradicts the PR's stated goal of bumping to 1.200.1-next.1. This may be a merge/revert artifact that left scripts/deno.json at an older version than intended.</comment>

<file context>
@@ -1,6 +1,6 @@
 {
   "name": "@deco/scripts",
-  "version": "1.200.1-next.1",
+  "version": "1.197.1-next.2",
   "exports": {
     "./release": "./release.ts",
</file context>
Suggested change
"version": "1.197.1-next.2",
"version": "1.200.1-next.1",

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants