Fix: White Page at Root URL — Revert index.html to Redirect + Checkstyle Cleanup - #517
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis pull request converts ChangesRepository root redirect and supporting changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
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 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 |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
Pull request overview
This PR fixes the broken root URL (/) behavior by reverting the root index.html back to a redirect to /manage, reducing coupling to hashed Manager SPA assets and simplifying deployment. It also includes small readability/logging/test tweaks.
Changes:
- Reverted
src/main/resources/META-INF/resources/index.htmlto a minimal redirect page to/manage(no hashed asset references). - Updated Quarkus HTTP auth permission paths to explicitly permit unauthenticated GET/HEAD access to
/alongside other static SPA entry points/assets. - Minor improvements: SQL formatting readability, an added warning log for unsupported Slack target types, and a small test mock clarification.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/ai/labs/eddi/datastore/postgres/PostgresGlobalVariableStoreTest.java | Explicitly casts mocked null to String for clarity. |
| src/main/resources/META-INF/resources/index.html | Replaces SPA asset references with a simple redirect-to-/manage page. |
| src/main/resources/application.properties | Reformats and extends static-assets auth permission paths to include /. |
| src/main/java/ai/labs/eddi/integrations/slack/SlackEventHandler.java | Adds warning log for unsupported target types in routing switch. |
| src/main/java/ai/labs/eddi/engine/tenancy/PostgresTenantQuotaStore.java | Reformats INSERT INTO tenant_usage SQL strings for readability. |
| docs/changelog.md | Documents root white-page cause/fix and related deployment notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/changelog.md (1)
12-28: ⚡ Quick winAdd an explicit “Design decisions” subsection to this entry.
The rationale is present, but the required changelog structure is missing an explicit design-decision block, which makes the entry inconsistent with the mandated format.
Suggested patch
### Fix - **`index.html`** — Reverted to a simple `<meta http-equiv="refresh">` redirect to `/manage`. No asset references, no sync needed. Keycloak works because the SPA boots at `/manage` and sets `redirectUri` to its own URL; the Keycloak client's `redirectUris: ["http://localhost:*"]` matches any path. - **`deploy-to-local-eddi-repo.ps1`** — Removed `$IndexHtml` handling (no longer needed). Script only updates `manage.html`. +### Design Decisions +- Keep `/` as a minimal redirect entrypoint instead of duplicating SPA shell content to avoid hash-coupled static asset drift. +- Keep deployment synchronization responsibility focused on `manage.html` only, reducing cross-repo coupling and deployment fragility. + ### Architecture ClarificationAs per coding guidelines: “Changelog entries must include: ... design decisions made ...”.
🤖 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 `@docs/changelog.md` around lines 12 - 28, The changelog entry for the Root Cause/Fix needs an explicit "Design decisions" subsection to conform to the required format: open the entry in docs/changelog.md and add a new "Design decisions" heading that succinctly lists the key rationale behind reverting index.html to a meta refresh, removing $IndexHtml handling from deploy-to-local-eddi-repo.ps1, and how manage.html/chat.html responsibilities and Keycloak redirect behavior influenced the choice; reference the files index.html and deploy-to-local-eddi-repo.ps1 and ensure the new subsection is placed after "Architecture Clarification" and mirrors the style/format used by other changelog entries.
🤖 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 `@docs/changelog.md`:
- Around line 19-24: Insert a blank line between the "### Architecture
Clarification" heading and the following markdown table to satisfy markdownlint
MD058; edit the docs/changelog.md section containing the heading "Architecture
Clarification" and the table rows (e.g., entries for `index.html`,
`manage.html`, `chat.html`) so there is one empty line immediately after the
heading before the table starts.
In `@src/main/resources/application.properties`:
- Around line 171-174: The issue is overlapping path rules: keep `/` only in the
permit rule and remove or narrow it from the authenticated rule so Quarkus
doesn’t require both permissions; update the configuration by removing `/` (or
replacing it with a more specific path like `/api/*`) from
quarkus.http.auth.permission.authenticated.paths and ensure
quarkus.http.auth.permission.static-assets.paths retains `/` so public root
requests are covered by the permit rule; adjust the two properties
quarkus.http.auth.permission.static-assets.paths and
quarkus.http.auth.permission.authenticated.paths accordingly.
---
Nitpick comments:
In `@docs/changelog.md`:
- Around line 12-28: The changelog entry for the Root Cause/Fix needs an
explicit "Design decisions" subsection to conform to the required format: open
the entry in docs/changelog.md and add a new "Design decisions" heading that
succinctly lists the key rationale behind reverting index.html to a meta
refresh, removing $IndexHtml handling from deploy-to-local-eddi-repo.ps1, and
how manage.html/chat.html responsibilities and Keycloak redirect behavior
influenced the choice; reference the files index.html and
deploy-to-local-eddi-repo.ps1 and ensure the new subsection is placed after
"Architecture Clarification" and mirrors the style/format used by other
changelog entries.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d70411cf-eb8b-411d-b739-e1842ce75431
📒 Files selected for processing (6)
docs/changelog.mdsrc/main/java/ai/labs/eddi/engine/tenancy/PostgresTenantQuotaStore.javasrc/main/java/ai/labs/eddi/integrations/slack/SlackEventHandler.javasrc/main/resources/META-INF/resources/index.htmlsrc/main/resources/application.propertiessrc/test/java/ai/labs/eddi/datastore/postgres/PostgresGlobalVariableStoreTest.java
ginccc
changed the title
rolandpickl
approved these changes
Jun 3, 2026
12 tasks
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.
This PR fixes a critical bug where the root URL (/) served a broken white page due to stale hashed asset references in index.html. It also resolves all outstanding Checkstyle and compiler warnings.
Root Cause
Commit 0ec6cb4 replaced the original index.html redirect with a full copy of the Manager SPA, duplicating hashed asset references from manage.html. When the deploy script rebuilt the Manager, it updated manage.html and the asset files but had no knowledge of index.html — leaving it pointing at deleted files (index-Bn-sgAam.js, index-BZNayFGO.css). With X-Content-Type-Options: nosniff, the browser blocked the HTML fallback response, producing a white page.
Changes
Root URL & Deployment
index.html reverted to a minimal redirect to /manage — no asset references, no sync needed, no future breakage on Manager rebuilds. (index.html)
deploy-to-local-eddi-repo.ps1 no longer handles index.html, focusing solely on manage.html. (changelog)
Changelog updated with root cause analysis and architecture clarification table.
Checkstyle & Compiler Warning Fixes (0 violations)
PostgresTenantQuotaStore.java — Multi-line formatting for 3× SQL INSERT INTO column lists that exceeded 150-char line length. (1, 2, 3)
application.properties — Backslash line continuation for static-assets.paths that exceeded 150-char line length. Path values are unchanged. (application.properties)
SlackEventHandler.java — Added default branch with warning log to satisfy MissingSwitchDefault rule. (SlackEventHandler.java)
PostgresGlobalVariableStoreTest.java — Explicit (String) null cast to resolve varargs ambiguity compiler warning. (PostgresGlobalVariableStoreTest.java)
What's NOT Changed
Auth permissions — authenticated.paths and static-assets.paths values are identical to main. The root path / being behind auth is pre-existing behavior (the old SPA index.html was equally inaccessible with Keycloak enabled at /; users navigate to /manage directly).
Summary by CodeRabbit
Bug Fixes
Refactor
Tests