Merged
Conversation
Contributor
Reviewer's GuideAdjusts nginx frontend and deploy configurations to ensure index.html is never cached, Vite-generated assets are served with strict long-term caching and proper 404s, and cache headers from the frontend are propagated through the deploy proxy, with changelog updated for version 2.1.1. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Consider adding the
alwaysflag to theadd_headerdirectives for the HTML no-cache rules so they also apply to 304/4xx/5xx responses (e.g.add_header Cache-Control "no-cache, no-store, must-revalidate" always;), which helps ensure browsers don’t cache error or conditional responses. - By carving out
/assets/with long-lived immutable caching, you’re assuming everything under that path is fingerprinted; if that’s not guaranteed for all current and future assets, you may want a narrower pattern (e.g. only hashed filenames) to avoid accidentally locking in stale non-hashed resources for a year.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adding the `always` flag to the `add_header` directives for the HTML no-cache rules so they also apply to 304/4xx/5xx responses (e.g. `add_header Cache-Control "no-cache, no-store, must-revalidate" always;`), which helps ensure browsers don’t cache error or conditional responses.
- By carving out `/assets/` with long-lived immutable caching, you’re assuming everything under that path is fingerprinted; if that’s not guaranteed for all current and future assets, you may want a narrower pattern (e.g. only hashed filenames) to avoid accidentally locking in stale non-hashed resources for a year.
## Individual Comments
### Comment 1
<location path="frontend/nginx.conf" line_range="25-30" />
<code_context>
}
+ # Never cache index.html (SPA entry point — must always fetch latest)
+ location = /index.html {
+ add_header Cache-Control "no-cache, no-store, must-revalidate";
+ add_header Pragma "no-cache";
+ add_header Expires "0";
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Consider using `add_header ... always;` so the no-cache headers apply to 304s and non-200 responses as well.
Without `always`, these headers may not be sent on 304s or some error responses (depending on nginx version/config), so `/index.html` can still be cached in those cases. For an SPA entry point that must never be cached, consider:
```nginx
add_header Cache-Control "no-cache, no-store, must-revalidate" always;
add_header Pragma "no-cache" always;
add_header Expires "0" always;
```
This enforces no-cache semantics consistently, including on conditional and error responses.
```suggestion
# Never cache index.html (SPA entry point — must always fetch latest)
location = /index.html {
add_header Cache-Control "no-cache, no-store, must-revalidate" always;
add_header Pragma "no-cache" always;
add_header Expires "0" always;
}
```
</issue_to_address>
### Comment 2
<location path="CHANGELOG.md" line_range="3" />
<code_context>
+## 2.1.1
+
+- FIXED system - html and js frontend cache cause failures
+
## 2.1.0
</code_context>
<issue_to_address>
**suggestion (typo):** Consider fixing capitalization and grammar in this changelog entry.
For example, you could write: `- FIXED system - HTML and JS frontend caches causing failures` or `- FIXED system - HTML and JS frontend cache caused failures`.
```suggestion
- FIXED system - HTML and JS frontend caches causing failures
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Adjust frontend and deployment Nginx caching rules to prevent stale HTML/JS while keeping static asset caching, and document the change in the changelog.
Bug Fixes:
Enhancements:
Documentation: