feat(web): add /new preview UI#891
Open
KasenRi wants to merge 2 commits into
Open
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings
- [Major]
/newis bypassed in compiled hub builds - the preview routes are mounted only after theoptions.embeddedAssetMapbranch, but compiled/single-exe hubs enter that branch and return athub/src/web/server.ts:365. Since the single-exe pipeline embeds onlyweb/dist(package.json:10,hub/scripts/generate-embedded-web-assets.ts:59), a compiled hub will serve the existing rootindex.htmlfor/newinstead of the preview artifact/503. That makes the advertised hub-mounted preview fail for the binary distribution path.
Suggested fix:// Mount /new before the embedded static fallback, or add an embedded-preview map. if (!options.embeddedAssetMap) { if (!mountPreviewStaticRoutes(app, findPreviewWebappDistDir())) { mountMissingPreviewRoutes(app) } } else { mountMissingPreviewRoutes(app) } if (options.embeddedAssetMap) { // existing embedded root app handling }
Summary
- Review mode: initial
- One major issue found: compiled/single-exe hub builds currently cannot serve the
/newpreview path correctly. Residual risk: I did not run the full test suite in this automation pass.
Testing
- Not run (automation)
HAPI Bot
| return app | ||
| } | ||
|
|
||
| if (!mountPreviewStaticRoutes(app, findPreviewWebappDistDir())) { |
There was a problem hiding this comment.
[MAJOR] This /new mount never runs for compiled/single-exe hubs because the options.embeddedAssetMap branch above returns at line 365. The release pipeline embeds only web/dist, so /new falls through to the existing root index.html rather than the preview artifact/503.
Suggested fix:
if (!options.embeddedAssetMap) {
if (!mountPreviewStaticRoutes(app, findPreviewWebappDistDir())) {
mountMissingPreviewRoutes(app)
}
} else {
mountMissingPreviewRoutes(app)
}
There was a problem hiding this comment.
Findings
- [Major]
/newis still bypassed in compiled hub builds - the preview mount is registered only after theoptions.embeddedAssetMapbranch, but compiled/single-exe hubs return from that branch before reachinghub/src/web/server.ts:373. The embedded asset generator only packagesweb/dist, so/newserves the existing root app instead of the preview app or an explicit 503 in the binary distribution path.
Suggested fix:const previewMounted = options.embeddedAssetMap ? false : mountPreviewStaticRoutes(app, findPreviewWebappDistDir()) if (!previewMounted) { mountMissingPreviewRoutes(app) } if (options.embeddedAssetMap) { // existing embedded root app handling }
Summary
- Review mode: follow-up after new commits
- One major issue remains from the prior review: compiled/single-exe hubs still cannot serve
/newcorrectly. Residual risk: I did not execute PR code or tests in this review pass.
Testing
- Not run (automation)
HAPI Bot
| return app | ||
| } | ||
|
|
||
| if (!mountPreviewStaticRoutes(app, findPreviewWebappDistDir())) { |
There was a problem hiding this comment.
[MAJOR] This /new mount still never runs for compiled/single-exe hubs because the options.embeddedAssetMap branch above returns before this line. The release pipeline embeds only web/dist, so /new falls through to the existing root index.html instead of the preview artifact or the explicit missing-preview 503.
Suggested fix:
const previewMounted = options.embeddedAssetMap
? false
: mountPreviewStaticRoutes(app, findPreviewWebappDistDir())
if (!previewMounted) {
mountMissingPreviewRoutes(app)
}
if (options.embeddedAssetMap) {
// existing embedded root app handling
}
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
1 participant
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
/new./unchanged and still served from the existing web build.HAPI_WEB_PREVIEW_DIST_DIRor the defaultweb/dist-newartifact.Deployment
Build the preview with:
Deploy
web/dist-newand pointHAPI_WEB_PREVIEW_DIST_DIRat that directory when serving through the hub. The preview app is scoped to/new, while/api/*and/socket.io/*continue to hit the hub on the same origin.Validation Attempted
bun run test:web -- runtime-config router SessionListbun run test:hub -- previewStaticbun run typecheck:webbun run typecheck:hubbun run build:webbun run build:web:previewAll commands were attempted in the PR workspace. They could not complete because dependencies are not installed in the fresh clone (
vitest,hono,vite,vite/client,bun-types, andnodetype definitions were missing).Rollback
Rollback is to stop serving
/newby unsettingHAPI_WEB_PREVIEW_DIST_DIRand/or removing theweb/dist-newartifact. Production/remains on the existing build path and is not promoted by this change.User Testing
User testing is required on
/newbefore any promotion to/, including deep links such as/new/sessions/<id>, session creation, pending-permission flows, and PWA/service-worker scoping.