fix(@redhat-cloud-services/frontend-components-config-utilities): reload nav on CRD change - #2391
Merged
karelhala merged 4 commits intoJul 24, 2026
Conversation
…oad nav on CRD change RHCLOUD-49555 When feoConfigEnabled is true and the user edits deploy/frontend.yaml, the chokidar watcher correctly reloads the CRD object in memory but the browser never re-fetches navigation data. Two changes fix this: 1. Add watchFiles for the CRD path so webpack-dev-server sends a live-reload signal to the browser on file change. 2. Strip ETag/Last-Modified and set Cache-Control: no-store on intercepted FEO responses so the browser cannot serve stale navigation from its HTTP cache. Also removes unused matchNavigationRequest import.
WalkthroughThe change adds a local FEO CRD interceptor, routes eligible dev-proxy requests through it, prevents conditional caching from bypassing modifications, and reloads connected browsers after CRD updates. ChangesFEO CRD interception and reload
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant DevProxy
participant CRDInterceptor
participant Upstream
participant WebpackDevServer
DevProxy->>DevProxy: Detect FEO and load frontend CRD
DevProxy->>CRDInterceptor: Start interceptor server
Browser->>DevProxy: Request interceptable API path
DevProxy->>CRDInterceptor: Forward request
CRDInterceptor->>Upstream: Send request without conditional headers
Upstream-->>CRDInterceptor: Return response
CRDInterceptor-->>Browser: Return CRD-modified response with no-cache headers
CRDInterceptor->>DevProxy: Detect CRD update
DevProxy->>WebpackDevServer: Send content-changed
WebpackDevServer-->>Browser: Trigger reload
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@packages/config-utils/src/proxy.ts`:
- Around line 246-250: Update the intercepted-request handling in proxy.ts to
remove incoming If-None-Match and If-Modified-Since headers before forwarding
the request, ensuring conditional requests cannot produce reusable 304 responses
for CRD content. Keep the existing response cache-header removal in place.
- Around line 342-346: Coordinate the webpack-dev-server reload configured by
watchFiles with the chokidar callback that updates frontendCrdRef.current,
ensuring reloads occur only after a successful CRD read and assignment. Update
the existing frontend CRD watcher flow and avoid relying on the independent
watchFiles trigger when it can race the in-memory update.
🪄 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.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 09821cd3-1a5d-416a-af00-68b7401b7abd
📒 Files selected for processing (1)
packages/config-utils/src/proxy.ts
…vent stale CRD responses RHCLOUD-49555 Strip If-None-Match and If-Modified-Since request headers on intercepted FEO responses so upstream 304s cannot bypass CRD content modifications. Replace independent watchFiles reload with a coordinated trigger from the chokidar CRD watcher, ensuring the browser only reloads after the in-memory CRD has been updated. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
charlesmulder
changed the title
Collaborator
Investigation: This PR only fixes
|
…rception to dev-proxy mode RHCLOUD-49555 When feoConfigEnabled is true, fec dev-proxy now starts a CRD interceptor server that proxies the 4 chrome-service API endpoints through a local Node server which applies modifyRequest() to inject local frontend.yaml changes. This brings CRD interception parity between fec dev and fec dev-proxy, reusing 100% of the existing interception code from config-utils. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
Thanks for the thorough investigation @karelhala! Your analysis of the I've implemented your proposed solution — a CRD interceptor server that runs as a host-side Node process alongside the existing dev-proxy concurrent processes. Here's what the new commit adds: New file:
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
packages/config/src/bin/crd-interceptor-server.ts (1)
49-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
unknown[]overany[]for log data.
log's...data: any[]bypasses type checking on log payloads.- const log = (logType: LogType, ...data: any[]) => { + const log = (logType: LogType, ...data: unknown[]) => {As per path instructions, "Flag use of 'any' types - suggest specific types or 'unknown' with type guards" for
packages/*/src/**/*.{ts,tsx}.🤖 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 `@packages/config/src/bin/crd-interceptor-server.ts` around lines 49 - 52, Update the log function’s variadic data parameter from any[] to unknown[] while preserving the existing debug filtering and fecLogger delegation behavior.Source: Path instructions
🤖 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 `@packages/config-utils/src/proxy.ts`:
- Around line 247-254: Add comprehensive unit tests for the proxy’s onProxyReq
handler and triggerReload flow. Verify conditional headers are removed only when
FEOFeaturesEnabled, frontendCrdRef.current, and isInterceptAbleRequest(req.url)
all permit interception, while preserving them otherwise; also verify
triggerReload invokes server.sendMessage with the content-changed event. Use the
existing proxy test setup and mocks.
In `@packages/config/src/bin/crd-interceptor-server.ts`:
- Around line 96-137: Configure explicit timeouts for both upstream requests
created in the main handler and proxyPassthrough, using the existing
https.request calls as the change points. Handle timeout events by terminating
the stalled proxy request and completing the client response with the
established upstream-failure behavior, ensuring no request remains pending
indefinitely.
- Around line 62-69: Add an 'error' event handler to the chokidar watcher
created in the frontendCRDPath watch chain, logging the watcher error through
the existing log(LogType.error, ...) mechanism. Keep the existing change handler
and CRD reload behavior unchanged.
- Around line 86-106: Update the interceptor server setup and request handler to
listen explicitly on 127.0.0.1, validate that the incoming URL is a relative
path included in INTERCEPTABLE_PATHS, and reject or normalize absolute req.url
values before constructing upstreamUrl with hccEnvUrl. Preserve proxying only
for allowed interceptable paths and prevent arbitrary upstream hosts.
In `@packages/config/src/bin/dev-proxy-script.ts`:
- Around line 230-249: Update the CRD interception startup flow around
readFrontendCRD, hasFEOFeaturesEnabled, and startCRDInterceptorServer so
expected CRD-read failures retain the benign “features not available” handling,
while failures from startCRDInterceptorServer are logged separately with an
accurate message and the underlying error details. Ensure interceptorPort
remains unset when server startup fails.
---
Nitpick comments:
In `@packages/config/src/bin/crd-interceptor-server.ts`:
- Around line 49-52: Update the log function’s variadic data parameter from
any[] to unknown[] while preserving the existing debug filtering and fecLogger
delegation behavior.
🪄 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.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: b6c433e1-0675-485a-ade3-33825836ec7e
📒 Files selected for processing (3)
packages/config-utils/src/proxy.tspackages/config/src/bin/crd-interceptor-server.tspackages/config/src/bin/dev-proxy-script.ts
Comment on lines
+247
to
+254
| onProxyReq: (proxyReq, req) => { | ||
| // Strip conditional request headers for intercepted FEO responses to | ||
| // prevent 304 responses that would bypass CRD content modifications | ||
| if (FEOFeaturesEnabled && frontendCrdRef.current && isInterceptAbleRequest(req.url)) { | ||
| proxyReq.removeHeader('if-none-match'); | ||
| proxyReq.removeHeader('if-modified-since'); | ||
| } | ||
| }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Add unit tests for the new onProxyReq header-stripping and triggerReload reload signaling.
Neither the conditional-request header removal (Lines 247-254) nor the triggerReload → server.sendMessage(..., 'content-changed') wiring (Lines 357-365) appear to have accompanying tests. As per coding guidelines, "All functions in @redhat-cloud-services/frontend-components-config-utilities must have comprehensive unit tests."
Also applies to: 357-365
🤖 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 `@packages/config-utils/src/proxy.ts` around lines 247 - 254, Add comprehensive
unit tests for the proxy’s onProxyReq handler and triggerReload flow. Verify
conditional headers are removed only when FEOFeaturesEnabled,
frontendCrdRef.current, and isInterceptAbleRequest(req.url) all permit
interception, while preserving them otherwise; also verify triggerReload invokes
server.sendMessage with the content-changed event. Use the existing proxy test
setup and mocks.
Source: Coding guidelines
…terceptor RHCLOUD-49555 Address CodeRabbit review: bind to 127.0.0.1, validate interceptable paths, add request timeouts, chokidar error handler, unknown[] types, separate error handling for interceptor startup failures. Fix prettier formatting for createRoutesConfig params.
platex-rehor-bot
changed the title
karelhala
approved these changes
Jul 24, 2026
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
2 participants
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.
Description
When
feoConfigEnabledis true and a developer editsdeploy/frontend.yaml, the chokidar file watcher correctly reloads the CRD object in memory, but the browser never re-fetches navigation data — requiring a full dev-server restart to see changes.This PR fixes the issue with two small changes in
proxy.ts:watchFilesfor the CRD path — webpack-dev-server now watchesfrontend.yamland sends a live-reload signal to the browser when it changes.ETag/Last-Modifiedand setsCache-Control: no-storeso the browser cannot serve stale navigation data from its HTTP cache on page reload.Also removes an unused
matchNavigationRequestimport.RHCLOUD-49555
Anything reviewers should know?
watchFilesoption is only added whenFEOFeaturesEnabledis true, so non-FEO setups are unaffected.bundles-generated.json,search-index-generated.json, etc.) — other proxied responses retain their original caching behavior.onProxyReshandler already readfrontendCrdRef.currentat request time (not a snapshot), so the data path was correct — only the browser notification/caching was missing.Checklist
AI disclosure
Assisted by: Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
304bypass) and forcing no-cache responses.