feat(desktop): add settings for shared web/desktop history#1161
feat(desktop): add settings for shared web/desktop history#1161glopyglerky wants to merge 5 commits intopingdotgg:mainfrom
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can suggest fixes for GitHub Check annotations.Configure the |
github-actions
bot
added
size:XXL
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
apps/web/src/connection.ts
Outdated
|
|
||
| export function resolveRuntimeWsUrl(explicitUrl?: string): string { | ||
| if (typeof window === "undefined") { | ||
| return explicitUrl ?? "ws://localhost:3773"; |
There was a problem hiding this comment.
🟢 Low src/connection.ts:43
When resolveRuntimeWsUrl is called with an empty string explicitUrl in an SSR context (typeof window === "undefined"), the ?? operator returns "" instead of falling back to "ws://localhost:3773". This is inconsistent with the browser path, where asNonEmptyString treats empty strings as falsy. Consider using asNonEmptyString(explicitUrl) ?? "ws://localhost:3773" to align the two paths.
- return explicitUrl ?? "ws://localhost:3773";
+ return asNonEmptyString(explicitUrl) ?? "ws://localhost:3773";🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/connection.ts around line 43:
When `resolveRuntimeWsUrl` is called with an empty string `explicitUrl` in an SSR context (`typeof window === "undefined"`), the `??` operator returns `""` instead of falling back to `"ws://localhost:3773"`. This is inconsistent with the browser path, where `asNonEmptyString` treats empty strings as falsy. Consider using `asNonEmptyString(explicitUrl) ?? "ws://localhost:3773"` to align the two paths.
Evidence trail:
apps/web/src/connection.ts lines 41-54 (resolveRuntimeWsUrl function), lines 3-9 (asNonEmptyString function). SSR path at line 43 uses `??` which only falls back for null/undefined, not empty strings. Browser path at line 46-54 calls resolveWsUrlFromSources which uses asNonEmptyString at line 28-31, which returns null for empty strings.
|
@codex review |
![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: 5fe7039198
ℹ️ 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".
apps/desktop/src/remoteConnection.ts
Outdated
| const remoteUrlValue = sanitizeEnvValue(settings.remoteUrl); | ||
| if (!remoteUrlValue) { | ||
| return null; |
There was a problem hiding this comment.
When settings.mode is "remote" and remoteUrl is blank, this function returns null instead of throwing a configuration error. That lets desktop:connection:set accept and persist an invalid remote config, and on restart bootstrap() treats it as non-remote and starts the local backend, so users can save "remote" mode and silently fall back to local history. Empty remote URLs should raise DesktopRemoteConnectionConfigError so the invalid state is blocked and surfaced in the settings UI.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca9e71a027
ℹ️ 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".
| if (!remoteUrl) { | ||
| return null; |
There was a problem hiding this comment.
resolveDesktopConnectionSettingsFromEnv bails out unless T3CODE_DESKTOP_REMOTE_URL is set, so T3CODE_DESKTOP_REMOTE_AUTH_TOKEN alone is ignored. This breaks the documented/expected override model for rotating secrets: if a user saves the remote URL in desktop settings and only injects a new auth token via environment, bootstrap keeps using the stale saved token and can fail remote auth while silently keeping old credentials active. Treating token-only env input as an override (or explicit config error) avoids this mismatch.
Useful? React with 👍 / 👎.
3 participants
What Changed
Settings -> Shared Historyflow for desktopT3CODE_DESKTOP_REMOTE_URLorT3CODE_DESKTOP_REMOTE_AUTH_TOKENare setREADME.mdandREMOTE.mdWhy
The remote/shared-history setup already works well when desktop and web both point at one server-backed history source, but the old env-only flow is hard to discover and awkward to manage day to day.
This keeps the same underlying behavior while making it easier to configure safely:
UI Changes
Shared Historycard to Settings.Validation
bun run --cwd packages/contracts typecheckbun run --cwd apps/desktop testbun run --cwd apps/desktop typecheckbun run --cwd apps/web typecheckbunx vitest run apps/web/src/connection.test.tsbunx vitest run apps/desktop/src/remoteConnection.test.ts apps/desktop/src/desktopConnectionSettings.test.tsbun run build:desktopNotes
bun run --cwd apps/web teststill fails in this checkout because of an existing unrelated harness issue insrc/terminalStateStore.test.ts(localStorage.clear is not a function).Checklist
This is my first GitHub PR. I'm a big fan of the team and the product, and this was built with T3 Code.
Note
Add shared web/desktop history settings to connect the desktop app to a remote server
remoteconnection mode to the desktop app that uses a configured server URL as the WebSocket backend instead of starting a local backend process.getConnectionSettings,setConnectionSettings, andrelaunchon theDesktopBridgeIPC interface so the renderer can manage connection settings and trigger relaunches.Sidebar,store.ts, andwsTransport.ts.Macroscope summarized ca9e71a.