Conversation
Introduce ephemeral per-session web preview sessions so users can open an arbitrary URL in an alternate session view while keeping the prompt input available for iteration. The server now creates short-lived preview tokens, proxies HTTP and WebSocket traffic through authenticated /previews paths, and shares lower-level proxy forwarding with SideCars instead of duplicating the proxy implementation. The UI extracts the SideCar iframe shell into a reusable BrowserFrame with navigation controls, viewport presets, and no-injection same-origin element selection for comments. Preview comments append structured page and element references to the prompt draft while leaving the user's actual comment as normal text. All new user-facing strings are localized across the existing message catalogs. Validation: npm run typecheck --workspace @codenomad/ui; npm run typecheck --workspace @neuralnomads/codenomad.
|
I want you to perform the gatekeeper checks for latest PR state check for regressions |
|
Gatekeeper review for latest PR state. I interpreted the request as a strict regression/security/implementation review only; I did not make code changes. Findings:
This needs isolation before merge. Safer directions: serve previews from a separate origin/site, use a sandboxed iframe without
If the feature promise is arbitrary URLs, this needs URL rewriting or a different reverse-proxy model, ideally on an isolated origin where absolute paths naturally belong to that preview origin.
The responsive/default mode should match the old SideCar shell exactly; only fixed viewport presets should introduce the padded canvas. Validation attempted:
Recommendation: do not merge as-is. The same-origin arbitrary content issue is a hard blocker, and fixing it likely changes the preview/comment-targeting architecture. -- |
Restore the responsive browser frame to a full-bleed iframe so existing SideCar tabs keep their prior layout unless a fixed viewport preset is selected. Improve trusted preview compatibility by rewriting common root-relative HTML and CSS URLs back under the preview token path, and add a conservative authenticated referer fallback for root-relative asset requests that escape the preview prefix. This intentionally keeps same-origin preview rendering because element-level hover/comment targeting is a product requirement; the PR description should frame preview URLs as trusted inputs rather than a sandboxed arbitrary browsing surface. Validation: npm run typecheck --workspace @codenomad/ui; npm run typecheck --workspace @neuralnomads/codenomad.
|
Follow-up after gatekeeper review:
Validation rerun:
|
|
DOM inspection is non-negotiable. Review again |
|
Re-review of latest PR state, with DOM inspection treated as non-negotiable. I’m no longer evaluating this as “must sandbox and remove DOM access.” I’m evaluating it as a trusted, same-origin DOM-inspection preview surface. Under that model, the code still needs tighter boundaries because proxied remote HTTP semantics are being applied directly to the CodeNomad origin. Findings:
Fix before merge: drop
Fix before merge or explicitly accept the limitation: move the preview-referer fallback ahead of static serving for authenticated requests, while keeping the protected prefixes excluded. That makes fallback behavior deterministic instead of depending on whether the UI bundle has a matching path.
The PR description and UI still present this as opening arbitrary HTTP(S) URLs, while the implementation intentionally grants the previewed page same-origin script execution with CodeNomad for DOM inspection. Given the product requirement, this may be acceptable, but it must be explicit at the point of use. Fix before merge or as a required follow-up: rename/copy should say trusted preview URL, and the first-use/open dialog should warn users not to open untrusted sites. This is not a substitute for stripping cookies above; it documents the intentional DOM-inspection trust model. Resolved from prior review: The SideCar responsive layout regression appears fixed. Improved but still incomplete: The body rewriting at Validation attempted:
Recommendation: do not merge until at least the preview cookie handling is fixed. With DOM inspection as a hard requirement, the same-origin model can be intentional, but upstream HTTP cookies must not be allowed to mutate or receive CodeNomad-origin state. -- |
Prevent trusted preview targets from mutating CodeNomad origin state by stripping Set-Cookie response headers and withholding CodeNomad cookies from preview HTTP and WebSocket proxy requests. Move the authenticated preview referer fallback ahead of static UI serving so root-relative preview assets are proxied deterministically instead of colliding with bundled CodeNomad files like favicon or manifest assets. Validation: npm run typecheck --workspace @codenomad/ui; npm run typecheck --workspace @neuralnomads/codenomad.
|
Addressed the latest gatekeeper review:
Validation rerun:
|
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/25639077266 Artifacts expire in 7 days.
|
|
Check again |
|
Re-check of latest PR state after the cookie/fallback follow-up. I reviewed this under the accepted constraint that DOM inspection is non-negotiable and the preview is intentionally a trusted same-origin surface. Findings:
The implementation now intentionally gives previewed pages same-origin DOM/script capabilities so parent-side DOM inspection can work. That can be an acceptable product tradeoff, but the open-preview copy still says only “Enter a URL to preview in this session” / “Preview URL” ( This is no longer a proxy correctness blocker, but it is a product/security boundary issue. I would add explicit wording in the dialog such as “Only open trusted local/staging pages; previewed pages run in the CodeNomad preview context for DOM inspection.” This needs to be localized with the existing i18n keys. Resolved from prior review:
Residual risk: The URL rewriting/fallback approach is still best-effort and will not cover every web platform behavior ( Validation attempted:
Recommendation: no remaining blocking code findings from this re-check. I would still update the dialog copy before merge if this is going to ship as a trusted same-origin DOM-inspection preview. -- |
Make the open-preview prompt explicit that preview URLs should be trusted local or staging pages because the preview runs in CodeNomad's preview context to support DOM inspection. Localize the trusted-preview wording across all existing session message catalogs. Validation: npm run typecheck --workspace @codenomad/ui.
|
Addressed the trusted-URL copy finding:
Validation rerun:
|
Keep the trusted local/staging URL guidance in the preview dialog while removing the implementation-specific DOM inspection explanation from user-facing copy. Validation: npm run typecheck --workspace @codenomad/ui.
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/25639203366 Artifacts expire in 7 days. |
|
How about now? |
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/25639300658 Artifacts expire in 7 days. |
|
Re-check of latest PR state after the trusted-preview copy update. Findings: No remaining blocking or medium-severity findings from this pass. Verified:
Residual risk, accepted by scope: The preview proxy and URL rewriting are still best-effort rather than a complete browser-grade rewriting layer. Given the explicit trusted local/staging preview positioning and non-negotiable DOM inspection requirement, I do not consider that a merge blocker for this PR. Validation attempted:
Recommendation: gatekeeper approval from code-review perspective, subject to CI/typecheck passing in the proper dependency environment. -- |
Summary
User-facing behavior
Implementation notes
/previews/:token.Validation
npm run typecheck --workspace @codenomad/uinpm run typecheck --workspace @neuralnomads/codenomadNotes
package.jsonandpackage-lock.jsonwere intentionally left out of this PR.