revert(webapp): stale-deploy asset recovery (#4260)#4278
Conversation
|
WalkthroughRemoved the stale asset recovery component and its integration from the web application. Removed build-version response stamping, the 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| --- | ||
| area: webapp | ||
| type: fix | ||
| --- | ||
|
|
||
| Fix pages occasionally loading unstyled or failing to load during deploys. The dashboard now detects this and reloads to recover automatically, or prompts you to reload if it can't. |
There was a problem hiding this comment.
🟡 Server-only change ships without a release-notes entry
This change only touches the dashboard server code with no package changes (.server-changes/stale-deploy-asset-recovery.md:1-6 is deleted and none is added to replace it), so the removal of the auto-recovery behavior will not appear in the user-facing release notes.
Impact: Users get a behavior change (automatic reload/recovery during deploys is gone) with no corresponding note in the changelog.
Repository rule requiring a .server-changes file
Both .claude/REVIEW.md ("Server-only changes have .server-changes/*.md. Required for apps/webapp/, apps/supervisor/ edits with no public-package change ... Missing → 🟡.") and CONTRIBUTING.md require a .server-changes/*.md entry for PRs that modify only apps/webapp/ with no packages/ changes. This PR modifies apps/webapp/ exclusively (server.ts, entry.server.tsx, root.tsx, and deletes StaleAssetRecovery.tsx) and deletes the previous note without adding a new one.
Prompt for agents
This PR is a server-only change (only apps/webapp files are touched, no packages/ changes) but it does not include a .server-changes/*.md entry, which REVIEW.md and CONTRIBUTING.md require. The previous note (.server-changes/stale-deploy-asset-recovery.md) is being deleted as part of the revert and nothing replaces it. Add a new .server-changes/*.md file (area: webapp, type: fix) with a 1-2 sentence, non-technical, user-facing description of the behavior change so it appears in release notes. Consider whether the reverted feature was ever released to users when deciding wording.
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fd1b4bcc-5a39-46ee-908e-6380d9dd75dc
📒 Files selected for processing (6)
.claude/rules/server-apps.md.server-changes/stale-deploy-asset-recovery.mdapps/webapp/app/components/StaleAssetRecovery.tsxapps/webapp/app/entry.server.tsxapps/webapp/app/root.tsxapps/webapp/server.ts
💤 Files with no reviewable changes (5)
- .server-changes/stale-deploy-asset-recovery.md
- apps/webapp/app/components/StaleAssetRecovery.tsx
- apps/webapp/server.ts
- apps/webapp/app/root.tsx
- apps/webapp/app/entry.server.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (19)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 12)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: code-quality / code-quality
- GitHub Check: audit
- GitHub Check: audit
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
| --- | ||
| Fix pages occasionally loading unstyled during deploys. The dashboard now recovers automatically. | ||
| Brief description of what changed and why. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the required user-facing release-note format here.
This placeholder still only asks for a “brief description,” so it does not enforce the PR’s intended 1–2 short, non-technical sentences for dashboard users. Since the body is published verbatim, make the example explicitly state those constraints.
Suggested wording
-Brief description of what changed and why.
+Write 1–2 short, non-technical sentences for dashboard users; this text is copied verbatim into user-facing release notes.📝 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.
| Brief description of what changed and why. | |
| Write 1–2 short, non-technical sentences for dashboard users; this text is copied verbatim into user-facing release notes. |
1 participant
Reverts #4260.
The client-side stale-asset recovery it introduced is net-negative during normal deploys:
fetchinterception treats any?_data=request (Remix loader and action traffic) as a navigation and, on a build-id mismatch,location.assignes the tab to the fetched URL. During a rolling deploy an open dashboard tab can be hard-navigated to a raw data URL (e.g. a/resources/*loader), losing unsaved input./buildasset error (a network blip, an extension, an unrelated failed dynamic import) blanks the page behind a full-screen overlay for ~60s before offering a manual reload.sessionStorageto restore them across the reload.This returns the webapp to the pre-#4260 baseline as a fast, low-risk step — the diff is an exact inverse of #4260 (nothing else has touched those files since). A simpler reload-only recovery, plus a load-balancer stickiness change that addresses the root cause, will follow as separate PRs.