Conversation
Anonymous responses contain no per-user answer state and are identical for all visitors, so set Cache-Control: public, max-age=0, s-maxage=300, stale-while-revalidate=600 for anonymous requests. Logged-in and degraded (vote stats failure) responses are deliberately excluded. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…oute load() unit tests - E2E: add logged-in test asserting s-maxage is absent (personalized responses must not be shared-cached) - Unit: add tagIds branch coverage and fix header key to Cache-Control (canonical case) - .claude/rules/testing.md: document route load() unit test pattern with setHeaders spy Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthrough
Changes匿名アクセス CDN キャッシュ制御
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/routes/problems/`+page.server.ts:
- Around line 40-42: The code currently sets Cache-Control headers only when
session is null and voteStatsOk is true, but does not explicitly set headers for
the complementary case (when session is not null or voteStatsOk is false). To
meet the requirement of forbidding shared caching for logged-in or degraded
states, add an else branch after the existing if statement that explicitly sets
Cache-Control headers with a directive that prevents shared caching (such as
'private' or 'private, max-age=0') rather than relying on runtime or middleware
defaults. This ensures the cache guarantee is explicitly codified in the code
for both branches.
In `@src/routes/problems/page_server.test.ts`:
- Around line 71-73: The Cache-Control header tests are using partial matching
which misses regressions of important directives. At
src/routes/problems/page_server.test.ts lines 71-73 (anchor), replace the
partial toContain assertions with a complete expectation that validates all
required directives: public, s-maxage=300, max-age=0, and
stale-while-revalidate=600 together (consider using a fixed expected string or
toMatch with a regex that ensures all directives are present). At
src/routes/problems/page_server.test.ts lines 82-84 (sibling), apply the same
complete contract validation for the tagIds case. At e2e/problems_cache.spec.ts
lines 16-17 (sibling), enhance the wire-level validation to also check for
max-age=0 and stale-while-revalidate=600 in the Cache-Control response header.
At e2e/problems_cache.spec.ts line 39 (sibling), when validating the logged-in
case, confirm not only that s-maxage is absent but also that the public
directive is not present in the Cache-Control header.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 604abd5c-8355-41e1-8577-68ae22b13274
📒 Files selected for processing (4)
.claude/rules/testing.mde2e/problems_cache.spec.tssrc/routes/problems/+page.server.tssrc/routes/problems/page_server.test.ts
Unit: replace loose toContain() with exact toBe() for the full directive string — partial matches let malformed headers silently pass. E2E: add max-age=0, stale-while-revalidate=600, and public assertions to cover the complete contract; logged-in test also asserts public is absent. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
e2e/problems_cache.spec.ts (1)
16-19:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win匿名レスポンスの Cache-Control 検証は完全一致にしてください。
現状の
toContain連鎖だと、余計な/矛盾するディレクティブ混入を見逃せます。
ユニットテストと同様に、E2E でもヘッダー契約を1本で固定するのが安全です。差分例
- expect(headers['cache-control']).toContain('public'); - expect(headers['cache-control']).toContain('max-age=0'); - expect(headers['cache-control']).toContain('s-maxage=300'); - expect(headers['cache-control']).toContain('stale-while-revalidate=600'); + expect(headers['cache-control']).toBe( + 'public, max-age=0, s-maxage=300, stale-while-revalidate=600', + );🤖 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 `@e2e/problems_cache.spec.ts` around lines 16 - 19, Replace the four separate `expect(...).toContain()` assertions in the anonymous response cache validation (lines checking for 'public', 'max-age=0', 's-maxage=300', and 'stale-while-revalidate=600') with a single exact match assertion that verifies the complete Cache-Control header value equals the full directive string. This ensures no extraneous or conflicting directives can slip through undetected, matching the same validation pattern used in unit tests.
🤖 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.
Duplicate comments:
In `@e2e/problems_cache.spec.ts`:
- Around line 16-19: Replace the four separate `expect(...).toContain()`
assertions in the anonymous response cache validation (lines checking for
'public', 'max-age=0', 's-maxage=300', and 'stale-while-revalidate=600') with a
single exact match assertion that verifies the complete Cache-Control header
value equals the full directive string. This ensures no extraneous or
conflicting directives can slip through undetected, matching the same validation
pattern used in unit tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 56c9a4f0-64de-45a8-b599-0d2dd8ce1028
📒 Files selected for processing (2)
e2e/problems_cache.spec.tssrc/routes/problems/page_server.test.ts
close #3666
Summary by CodeRabbit
リリースノート