fix(static): only append Vary: Accept-Encoding after a static asset match

[schiller-manuel]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
nitro.build	Ready	Preview, Comment	Mar 5, 2026 7:53pm

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request relocates the Vary: Accept-Encoding header logic in the static middleware to be set after asset resolution rather than before, and adds comprehensive unit tests for the middleware's behavior with different asset and encoding scenarios.

Changes

Cohort / File(s)	Summary
Static Middleware Header Logic src/runtime/internal/static.ts	Relocated the Vary: Accept-Encoding header assignment to occur after asset resolution instead of before, ensuring the header is set only following a successful asset lookup rather than preemptively.
Static Middleware Tests test/unit/static-middleware.test.ts	New test file covering the runtime static middleware with test cases for 404 scenarios (no asset match) and successful compressed asset responses, including assertions on Vary and Cache-Control headers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 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.
Description check	❓ Inconclusive	No pull request description was provided by the author.	Add a description explaining the change, its motivation, and any relevant context or testing considerations.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title follows conventional commits format with 'fix' prefix and clearly describes the change to static asset handling.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix-static

---

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.}

[coderabbitai]

🧹 Nitpick comments (1)

test/unit/static-middleware.test.ts (1)

33-41: Consider adding a test for requests without Accept-Encoding.

When no Accept-Encoding header is sent, encodings.length === 1 (only the empty string), so Vary: Accept-Encoding should not be appended even when an asset is matched. This edge case would strengthen coverage of the conditional at line 49 in static.ts.

💡 Suggested additional test case

it("does not append Accept-Encoding vary when asset is matched without encoding negotiation", async () => {
  getAsset.mockImplementation((id: string) => {
    if (id === "/foo.css") {
      return {
        etag: '"test"',
        mtime: Date.now(),
        type: "text/css",
        size: 100,
      };
    }
    return undefined;
  });
  isPublicAssetURL.mockReturnValue(true);
  readAsset.mockResolvedValue("body");
  const event = createEvent("/foo.css"); // no accept-encoding

  await handler(event);

  expect(event.res.headers.get("Vary")).toBe("Origin");
  expect(event.res.headers.get("Vary")).not.toContain("Accept-Encoding");
});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/unit/static-middleware.test.ts` around lines 33 - 41, Add a unit test
covering the edge case where no Accept-Encoding header is sent: mock getAsset to
return a valid asset (e.g., for "/foo.css"), isPublicAssetURL to true, and
readAsset to resolve the body, then create the event with
createEvent("/foo.css") without an Accept-Encoding header and call
handler(event); assert that event.res.headers.get("Vary") equals "Origin" and
does not contain "Accept-Encoding". This ensures the conditional in static.ts
that checks encodings.length prevents appending "Accept-Encoding" when only the
empty encoding is present.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@test/unit/static-middleware.test.ts`:
- Around line 33-41: Add a unit test covering the edge case where no
Accept-Encoding header is sent: mock getAsset to return a valid asset (e.g., for
"/foo.css"), isPublicAssetURL to true, and readAsset to resolve the body, then
create the event with createEvent("/foo.css") without an Accept-Encoding header
and call handler(event); assert that event.res.headers.get("Vary") equals
"Origin" and does not contain "Accept-Encoding". This ensures the conditional in
static.ts that checks encodings.length prevents appending "Accept-Encoding" when
only the empty encoding is present.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 80108a43-6322-4f23-b15d-e4fd0983df3d

📥 Commits

Reviewing files that changed from the base of the PR and between bfbb207 and e335268.

📒 Files selected for processing (2)

• src/runtime/internal/static.ts
• test/unit/static-middleware.test.ts