fix(console): stop blank /chat by rewriting OpenWork asset URLs - #188
fix(console): stop blank /chat by rewriting OpenWork asset URLs#188Travis-Gilbert wants to merge 1 commit into
Conversation
Root-absolute Vite assets 404 on the console origin after the /chat proxy strips the prefix, which left a blank page after login.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
📝 WalkthroughWalkthroughThe change adds shared helpers for rewriting OpenWork HTML assets and redirect locations under ChangesOpenWork chat proxy
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant OpenWork
participant ChatMiddleware
participant Browser
OpenWork->>ChatMiddleware: HTML response or Location header
ChatMiddleware->>ChatMiddleware: Rewrite assets or redirect path with /chat
ChatMiddleware->>Browser: Rewritten response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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.
🟡 Not ready to approve
The new rewrite logic has edge-case bugs around already-prefixed /chat and absolute-URL Location handling that can produce incorrect paths (e.g. /chat/chat) and should be fixed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR fixes the post-login blank /chat experience in the console by rewriting OpenWork’s HTML asset URLs (and redirects) so root-absolute references like /assets/... resolve through the existing /chat/* reverse-proxy instead of 404ing on the console origin.
Changes:
- Update
apps/consolemiddleware/chatfetch-proxy to rewrite OpenWork HTML andLocationheaders. - Introduce shared rewrite helpers (
rewriteOpenworkChatHtml,rewriteOpenworkLocation) for consistent proxy behavior. - Add Vitest coverage for the rewrite helpers.
File summaries
| File | Description |
|---|---|
| apps/console/src/middleware.ts | Uses the new rewrite helpers to fix HTML asset paths under /chat and to prefix redirect locations. |
| apps/console/src/lib/chat-openwork-proxy.ts | Adds HTML attribute/CSS url(...) and redirect Location rewriting utilities for the /chat proxy. |
| apps/console/src/lib/chat-openwork-proxy.test.ts | Adds unit tests validating the rewrite behavior for HTML assets and redirects. |
Review details
Suppressed comments (2)
apps/console/src/lib/chat-openwork-proxy.ts:25
- Same issue as the attribute rewrite: the
url(/...)rewrite only exempts/chat/…, not/chat(or/chat?…). This can produce/chat/chatin CSS/inline styles.
out = out.replace(
/\b(url)\((["']?)\/(?!\/|chat\/)/g,
'$1($2/chat/',
);
apps/console/src/lib/chat-openwork-proxy.test.ts:34
- The Location rewrite has logic for absolute URLs, but the tests only cover relative redirects. Add an assertion for an absolute URL whose pathname is not already under
/chat(e.g./chatty) to prevent regressions in the absolute-URL branch.
it('prefixes absolute redirects', () => {
expect(rewriteOpenworkLocation('/')).toBe('/chat/');
expect(rewriteOpenworkLocation('/settings')).toBe('/chat/settings');
expect(rewriteOpenworkLocation('/chat/x')).toBe('/chat/x');
});
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| it('does not double-prefix paths already under /chat', () => { | ||
| const html = `<html data-register-impl="openwork.chat"><script src="/chat/assets/x.js"></script></html>`; | ||
| expect(rewriteOpenworkChatHtml(html)).toContain('src="/chat/assets/x.js"'); | ||
| expect(rewriteOpenworkChatHtml(html)).not.toContain('src="/chat/chat/'); | ||
| }); |
| out = out.replace( | ||
| /\b(href|src|poster)=(["'])\/(?!\/|chat\/)/g, | ||
| '$1=$2/chat/', | ||
| ); |
| const url = new URL(location); | ||
| if (!url.pathname.startsWith('/chat')) { | ||
| url.pathname = `/chat${url.pathname === '/' ? '/' : url.pathname}`; | ||
| } |
![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
🤖 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 `@apps/console/src/lib/chat-openwork-proxy.ts`:
- Around line 35-39: Update the URL helper around the existing pathname-prefix
logic to accept the workspace origin and convert same-origin full HTTP(S) URLs
into relative `/chat${pathname}${search}${hash}` redirects. Parse pathname
independently before checking the prefix, preserving `/chat?tab=x` unchanged and
avoiding matches such as `/chatty`; leave third-party and non-HTTP URLs
untouched. Add coverage for same-origin workspace redirects, external OAuth
URLs, query/fragment preservation, and the `/chat?tab=x` case.
🪄 Autofix
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 Plus
Run ID: 3d678a63-d3bc-45c8-9482-07e47499c684
📒 Files selected for processing (3)
apps/console/src/lib/chat-openwork-proxy.test.tsapps/console/src/lib/chat-openwork-proxy.tsapps/console/src/middleware.ts
| const url = new URL(location); | ||
| if (!url.pathname.startsWith('/chat')) { | ||
| url.pathname = `/chat${url.pathname === '/' ? '/' : url.pathname}`; | ||
| } | ||
| return url.toString(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep workspace redirects on the console proxy.
A full upstream redirect such as https://workspace.example/settings becomes https://workspace.example/chat/settings. At apps/console/src/middleware.ts, Lines 68-70 forward that URL to the browser. The browser leaves the console origin instead of requesting /chat/settings, so the console-scoped cookie cannot support the redirected request.
Pass the workspace origin to this helper. For a full URL with that origin, return /chat${pathname}${search}${hash}. Preserve third-party and non-HTTP URLs. Also parse the pathname before testing the prefix so /chat?tab=x is not double-prefixed and /chatty is not treated as /chat.
Add coverage for a full workspace URL, an external OAuth URL, query and fragment preservation, and /chat?tab=x.
Proposed fix
-export function rewriteOpenworkLocation(location: string | null): string | null {
+export function rewriteOpenworkLocation(
+ location: string | null,
+ workspace: string,
+): string | null {
if (!location) return location;
+ if (location.startsWith('//')) return location;
if (location.startsWith('/chat/') || location === '/chat') return location;
if (location.startsWith('/')) return `/chat${location === '/' ? '/' : location}`;
try {
const url = new URL(location);
- if (!url.pathname.startsWith('/chat')) {
- url.pathname = `/chat${url.pathname === '/' ? '/' : url.pathname}`;
- }
- return url.toString();
+ if (url.origin !== new URL(workspace).origin) return location;
+ if (url.pathname === '/chat' || url.pathname.startsWith('/chat/')) {
+ return `${url.pathname}${url.search}${url.hash}`;
+ }
+ return `/chat${url.pathname === '/' ? '/' : url.pathname}${url.search}${url.hash}`;
} catch {
return location;
}
}- rewriteOpenworkLocation(location)
+ rewriteOpenworkLocation(location, WORKSPACE)🤖 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 `@apps/console/src/lib/chat-openwork-proxy.ts` around lines 35 - 39, Update the
URL helper around the existing pathname-prefix logic to accept the workspace
origin and convert same-origin full HTTP(S) URLs into relative
`/chat${pathname}${search}${hash}` redirects. Parse pathname independently
before checking the prefix, preserving `/chat?tab=x` unchanged and avoiding
matches such as `/chatty`; leave third-party and non-HTTP URLs untouched. Add
coverage for same-origin workspace redirects, external OAuth URLs,
query/fragment preservation, and the `/chat?tab=x` case.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ff5fa0864
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /<html([^>]*)>/i, | ||
| '<html$1 data-register-impl="openwork.chat">', | ||
| ); | ||
| const stamped = rewriteOpenworkChatHtml(html); |
There was a problem hiding this comment.
When this serves the rewritten OpenWork HTML at /chat, only the asset URLs are moved under the proxy prefix. I checked the proxied app: it mounts a plain BrowserRouter with no basename in apps/chat/src/index.react.tsx lines 40 and 48, and its wildcard route redirects unknown paths to /session in apps/chat/src/react-app/shell/app-root.tsx lines 289 to 290. As soon as the now-loadable bundle runs on /chat, it matches the wildcard and navigates the browser to console-root /session, outside this middleware, so /chat still will not stay on the OpenWork UI. Configure a /chat basename or otherwise rewrite the route base along with the assets.
Useful? React with 👍 / 👎.
| const url = new URL(location); | ||
| if (!url.pathname.startsWith('/chat')) { | ||
| url.pathname = `/chat${url.pathname === '/' ? '/' : url.pathname}`; | ||
| } | ||
| return url.toString(); |
There was a problem hiding this comment.
When the workspace responds with an absolute Location, this code keeps that upstream origin and only changes the path. In production CONSOLE_WORKSPACE_URL can be a private/internal Railway host, so a redirect like http://commonplace-workspace.railway.internal/settings would be sent to the browser as http://commonplace-workspace.railway.internal/chat/settings, bypassing the console origin and failing for users; external absolute redirects are also mutated. For proxied redirects, same-upstream absolute locations should become console-relative /chat..., while other origins should be left alone.
Useful? React with 👍 / 👎.
| out = out.replace( | ||
| /\b(href|src|poster)=(["'])\/(?!\/|chat\/)/g, | ||
| '$1=$2/chat/', |
There was a problem hiding this comment.
This rewrite only fixes static HTML attributes, but the proxied OpenWork bundle still derives its server base from window.location.origin: resolveOpenworkConnection() falls back to the bare origin in apps/chat/src/react-app/shell/openwork-connection.ts lines 92 to 95, and createOpenworkServerClient() then calls root paths such as /workspaces in apps/chat/src/app/lib/openwork-server.ts line 1339. Because this middleware only proxies /chat/*, those runtime requests hit the Next console origin instead of the workspace and the loaded UI cannot list workspaces or sessions. Inject or rewrite the OpenWork base to /chat (or proxy the required root API paths) along with the asset URLs.
Useful? React with 👍 / 👎.
Summary
/chat, which middleware proxies to OpenWork./assets/...and favicons; those hit the console origin and 404, so the page stayed blank.href/src(and Location redirects) under/chat/so the existing proxy serves them.Test plan
npm test -- --run src/lib/chat-openwork-proxy.test.tsinapps/consolecurl -s https://v2.theoremharness.com/chat | rg 'src="/chat/assets/'shows prefixed assets/chatloads OpenWork UI (no blank blink / no/assets404s in Network)Summary by CodeRabbit