Skip to content

fix(desktop): don't persist ephemeral preview-proxy URLs (CODE-373) - #241

Merged
PeronGH merged 2 commits into
masterfrom
chenyu/code-373
Jul 28, 2026
Merged

fix(desktop): don't persist ephemeral preview-proxy URLs (CODE-373)#241
PeronGH merged 2 commits into
masterfrom
chenyu/code-373

Conversation

@lucas77778

Copy link
Copy Markdown
Member

Summary

  • add a shared browser URL predicate for the daemon's ephemeral <kind>--<id>.localhost preview namespace
  • clear blob and preview-proxy URLs during desktop shell serialization and rehydration while preserving normal HTTPS URLs
  • cover blob, file, artifact, service preview, and durable URL persistence behavior

Testing

  • devenv shell -- pnpm check:ci
  • devenv shell -- pnpm test (213 files, 1657 tests)

Linear: https://linear.app/arcbox/issue/CODE-373/fixdesktop-dont-persist-ephemeral-preview-proxy-urls-in-the-browser

Copilot AI review requested due to automatic review settings July 22, 2026 05:19
@linear-code

linear-code Bot commented Jul 22, 2026

Copy link
Copy Markdown

CODE-373

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown

Greptile Summary

This PR prevents temporary preview URLs from surviving desktop restarts. The main changes are:

  • A shared predicate for daemon-style .localhost preview URLs.
  • Browser URL filtering during desktop serialization and rehydration.
  • Tests for blob, file, artifact, service-preview, and HTTPS URLs.

Confidence Score: 4/5

The preview-host predicate can erase unrelated local browser URLs and needs a narrower namespace check.

  • Serialization and rehydration apply the filter consistently.

  • The package export matches the existing desktop import path.

  • Any durable .localhost service with -- in its label loses its browser location after restart.

T-Rex T-Rex Logs

What T-Rex did

  • Reproduced and confirmed that the durable local URL is discarded during both serialization and rehydration in the desktop persistence test.
  • Inspected the verbose failing test output to verify the input, serialized, and rehydrated URL values, which showed nulls indicating discard.
  • Ran the Vitest suite for the shell-state tests and confirmed all tests passed with exit code 0.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
packages/presentation/ui/src/shell/browser/normalize.ts Adds preview-host detection, but also matches unrelated .localhost hosts containing a double hyphen.
apps/desktop/src/renderer/src/shell/store/model.ts Filters blob and preview URLs consistently during serialization and rehydration.
packages/presentation/ui/src/shell/browser/index.ts Exports the new predicate through the existing browser package entry point.
apps/desktop/src/renderer/src/tests/shell-state.test.ts Adds persistence coverage for temporary preview URLs and a durable HTTPS URL.

Reviews (1): Last reviewed commit: "fix(desktop): exclude preview URLs from ..." | Re-trigger Greptile

Comment thread packages/presentation/ui/src/shell/browser/normalize.ts Outdated

@AprilNEA AprilNEA left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior is right and it still merges clean on current master. One blocking item: the predicate duplicates one we already have.

isPreviewBrowserUrl is a verbatim copy of isPreviewOrigin in packages/foundation/transport/src/preview-routes.ts — same suffix constant, same label regex, same try { new URL(url).hostname }. Importing it instead of re-deriving it costs nothing here: transport re-exports preview-routes from its root entry, that entry is browser-safe (the node-only pieces sit behind the ./server subpath), apps/desktop already depends on @linkcode/transport, and client-core already pulls it into the renderer bundle. It is also the boundary-correct home for the check — packages/presentation/ui is not allowed to import transport, so this belongs at the desktop edge rather than as a second copy in the UI package.

That also settles the review bot's P1 about http://my--app.localhost:3000: reusing the shared predicate makes the client agree with the daemon's own definition of the preview namespace by construction, instead of maintaining a parallel heuristic that can drift from it.

Things I verified that are fine, so you don't need to revisit them:

  • The separate blob: check must stay — new URL('blob:…').hostname is empty, so isPreviewOrigin cannot see it.
  • Coverage is complete. web--, file-- and artifact-- are all registered on demand by the engine, so after a restart the route table is empty and every one of them 404s.
  • BrowserPane has a url === null empty state, so dropping to null does not leave a blank pane.

Comment thread packages/presentation/ui/src/shell/browser/normalize.ts Outdated
Comment thread packages/presentation/ui/src/shell/browser/index.ts Outdated
Comment thread apps/desktop/src/renderer/src/shell/store/model.ts Outdated
Comment thread apps/desktop/src/renderer/src/shell/store/model.ts Outdated
Comment thread apps/desktop/src/renderer/src/__tests__/shell-state.test.ts Outdated
Comment thread apps/desktop/src/renderer/src/__tests__/shell-state.test.ts
Comment thread apps/desktop/src/renderer/src/__tests__/shell-state.test.ts Outdated
Comment thread apps/desktop/src/renderer/src/__tests__/shell-state.test.ts
Copilot AI review requested due to automatic review settings July 27, 2026 05:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@PeronGH
PeronGH merged commit ae0117e into master Jul 28, 2026
10 checks passed
@PeronGH
PeronGH deleted the chenyu/code-373 branch July 28, 2026 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants