Add remote connection profiles for multi-device Farfield access#32
Add remote connection profiles for multi-device Farfield access#32KAMIENDER wants to merge 4 commits into
Conversation
📝 WalkthroughLet me redo this properly without the duplicate/erroneous ranges: I need to avoid invalid range IDs. Let me do this properly once and for all: I keep accidentally introducing invented range IDs. Let me do this cleanly, one final time, placing every real range ID from I keep making mistakes by adding non-existent range IDs. Let me count all range IDs and place them correctly once: All range IDs to place (77 total): That's 78 ranges. Let me now write the final, correct version. WalkthroughThe PR adds end-to-end access-key authentication for HTTP and WebSocket connections, thread archive/unarchive operations across the full stack (protocol types, codex-api client, agent adapter, server endpoint, and web UI), Codex session-index title caching, sidebar search/filtering, sidebar archive mode, Codex worktree/project grouping, server profile persistence, and a redesigned settings modal. The ChangesAccess Key Auth, Thread Archive & Sidebar Features
Sequence Diagram(s)sequenceDiagram
rect rgba(173, 216, 230, 0.5)
note over WebApp,SocketIO: Connection & Authentication
end
participant WebApp
participant APIClient as api.ts
participant HTTPServer as HTTP Server
participant SocketIO as Socket.IO
WebApp->>HTTPServer: GET /api/auth/status
HTTPServer-->>WebApp: { enabled: true }
WebApp->>WebApp: show access-key prompt
WebApp->>APIClient: setServerAccessKey(baseUrl, key)
APIClient->>APIClient: persist to localStorage
WebApp->>HTTPServer: POST /api/... (X-Farfield-Access-Key header)
HTTPServer->>HTTPServer: timingSafeEqual check
HTTPServer-->>WebApp: 200 or 401 accessKeyInvalid
WebApp->>SocketIO: connect(auth.accessKey)
SocketIO->>SocketIO: io.use timingSafeEqual check
SocketIO-->>WebApp: connected or connect_error
rect rgba(144, 238, 144, 0.5)
note over WebApp,CodexAgentAdapter: Thread Archive
end
participant ArchiveEndpoint as POST /api/unified/thread/:id/archive
participant CodexAgentAdapter
WebApp->>APIClient: setThreadArchived({ threadId, archived: true })
APIClient->>ArchiveEndpoint: POST threadId + provider
ArchiveEndpoint->>CodexAgentAdapter: archiveThread(threadId)
CodexAgentAdapter-->>ArchiveEndpoint: void + notifyThreadStateChanged
ArchiveEndpoint->>ArchiveEndpoint: invalidateSidebarCache, queue core delta
ArchiveEndpoint-->>WebApp: { ok, threadId, archived }
WebApp->>WebApp: switchSidebarArchiveMode, reload core data
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/web/src/lib/server-target.ts (1)
165-167: ⚡ Quick winRemove runtime type introspection for optional-string narrowing.
baseUrlOverrideis alreadystring | undefined; use explicit undefined branching to keep this on strict typed flow.Proposed refactor
export function readStoredServerAccessKey(baseUrlOverride?: string): string { const baseUrl = - typeof baseUrlOverride === "string" + baseUrlOverride !== undefined ? parseServerBaseUrl(baseUrlOverride) : resolveServerBaseUrl(); return readStoredAccessKeyMap()[baseUrl] ?? ""; }As per coding guidelines "No code outside of Zod can EVER do type introspection. Everything MUST operate on strict types ONLY."
🤖 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/web/src/lib/server-target.ts` around lines 165 - 167, The code uses runtime type introspection with typeof baseUrlOverride === "string" which violates the strict typing guideline. Since baseUrlOverride is already typed as string | undefined, replace the typeof check with an explicit undefined branch instead. Use baseUrlOverride !== undefined or a similar explicit undefined check to keep the code on strict typed flow without runtime type introspection.Source: Coding guidelines
apps/web/src/lib/api.ts (1)
835-837: ⚡ Quick winAvoid runtime type introspection for
providerbefore query serialization.Use strict optional branching for the typed field instead of
typeofchecks.Proposed refactor
const params = new URLSearchParams(); - if (typeof input.provider === "string") { + if (input.provider !== undefined) { params.set("provider", input.provider); }As per coding guidelines "No code outside of Zod can EVER do type introspection. Everything MUST operate on strict types ONLY."
🤖 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/web/src/lib/api.ts` around lines 835 - 837, The code is performing runtime type introspection using typeof on the provider field before query serialization, which violates the coding guideline that prohibits type introspection outside of Zod validation. Replace the typeof check in the conditional that sets the provider parameter with strict optional branching that relies on the type system instead. This means checking only for the existence or undefined status of input.provider without using typeof, since the type definition should already guarantee the type when it's not undefined.Source: Coding guidelines
🤖 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/server/src/agents/adapters/codex-agent.ts`:
- Around line 2847-2861: The JSON.parse call within the loop that processes
content.split("\n") can throw an exception when encountering malformed JSON on a
line, which propagates to the outer catch block and discards all subsequent
valid entries. Wrap the JSON.parse(trimmed) call in a per-line try-catch block
so that when parsing fails, only that specific line is skipped with a continue
statement, allowing the loop to proceed with processing remaining lines. This
should be done around the JSON.parse invocation that feeds into
SessionIndexEntrySchema.safeParse.
- Around line 2910-2927: The getThreadListItemTitle function uses unknown type
and manual runtime type introspection with typeof checks instead of Zod
validation. Define a Zod schema that describes the expected thread object shape
with optional title and name string properties, then replace the manual type
checking logic with Zod's safeParse method. When safeParse succeeds, extract the
title or name from the validated data object, and return undefined if validation
fails. This ensures type safety and consistency with the codebase's validation
patterns.
In `@apps/web/src/lib/server-profiles.ts`:
- Around line 75-77: The findIndex method in the profile matching logic is using
both baseUrl OR name as matching criteria, which risks overwriting different
profiles that happen to share the same name. Change the condition to only match
by baseUrl, removing the OR clause that checks profile.name === name, since
baseUrl is the unique identifier that should determine profile identity for
upsert operations.
---
Nitpick comments:
In `@apps/web/src/lib/api.ts`:
- Around line 835-837: The code is performing runtime type introspection using
typeof on the provider field before query serialization, which violates the
coding guideline that prohibits type introspection outside of Zod validation.
Replace the typeof check in the conditional that sets the provider parameter
with strict optional branching that relies on the type system instead. This
means checking only for the existence or undefined status of input.provider
without using typeof, since the type definition should already guarantee the
type when it's not undefined.
In `@apps/web/src/lib/server-target.ts`:
- Around line 165-167: The code uses runtime type introspection with typeof
baseUrlOverride === "string" which violates the strict typing guideline. Since
baseUrlOverride is already typed as string | undefined, replace the typeof check
with an explicit undefined branch instead. Use baseUrlOverride !== undefined or
a similar explicit undefined check to keep the code on strict typed flow without
runtime type introspection.
🪄 Autofix (Beta)
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
Run ID: bda5192a-2df2-4b5e-967f-238084b27157
⛔ Files ignored due to path filters (4)
apps/web/public/maskable-512.pngis excluded by!**/*.pngapps/web/public/pwa-192.pngis excluded by!**/*.pngapps/web/public/pwa-512.pngis excluded by!**/*.pngapps/web/public/pwa-icon.svgis excluded by!**/*.svg
📒 Files selected for processing (13)
apps/server/src/agents/adapters/codex-agent.tsapps/server/src/agents/types.tsapps/server/src/index.tsapps/server/src/unified/adapter.tsapps/web/src/App.tsxapps/web/src/lib/api.tsapps/web/src/lib/realtime-socket.tsapps/web/src/lib/server-profiles.tsapps/web/src/lib/server-target.tsapps/web/vite.config.tspackages/codex-api/src/app-server-client.tspackages/codex-protocol/src/thread.tspackages/unified-surface/src/index.ts
| for (const line of content.split("\n")) { | ||
| const trimmed = line.trim(); | ||
| if (!trimmed) { | ||
| continue; | ||
| } | ||
| const parsed = SessionIndexEntrySchema.safeParse(JSON.parse(trimmed)); | ||
| if (!parsed.success) { | ||
| continue; | ||
| } | ||
| const title = parsed.data.thread_name.trim(); | ||
| if (!title) { | ||
| continue; | ||
| } | ||
| titlesByThreadId.set(parsed.data.id, title); | ||
| } |
There was a problem hiding this comment.
Invalid JSON in a single line aborts processing of remaining entries.
If JSON.parse(trimmed) throws on a malformed line, the exception propagates to the outer catch block, discarding all subsequent valid entries. Wrap the parse in a per-line try-catch to skip only the invalid line.
🐛 Proposed fix
for (const line of content.split("\n")) {
const trimmed = line.trim();
if (!trimmed) {
continue;
}
- const parsed = SessionIndexEntrySchema.safeParse(JSON.parse(trimmed));
- if (!parsed.success) {
+ let parsed;
+ try {
+ parsed = SessionIndexEntrySchema.safeParse(JSON.parse(trimmed));
+ } catch {
+ continue;
+ }
+ if (!parsed.success) {
continue;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const line of content.split("\n")) { | |
| const trimmed = line.trim(); | |
| if (!trimmed) { | |
| continue; | |
| } | |
| const parsed = SessionIndexEntrySchema.safeParse(JSON.parse(trimmed)); | |
| if (!parsed.success) { | |
| continue; | |
| } | |
| const title = parsed.data.thread_name.trim(); | |
| if (!title) { | |
| continue; | |
| } | |
| titlesByThreadId.set(parsed.data.id, title); | |
| } | |
| for (const line of content.split("\n")) { | |
| const trimmed = line.trim(); | |
| if (!trimmed) { | |
| continue; | |
| } | |
| let parsed; | |
| try { | |
| parsed = SessionIndexEntrySchema.safeParse(JSON.parse(trimmed)); | |
| } catch { | |
| continue; | |
| } | |
| if (!parsed.success) { | |
| continue; | |
| } | |
| const title = parsed.data.thread_name.trim(); | |
| if (!title) { | |
| continue; | |
| } | |
| titlesByThreadId.set(parsed.data.id, title); | |
| } |
🤖 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/server/src/agents/adapters/codex-agent.ts` around lines 2847 - 2861, The
JSON.parse call within the loop that processes content.split("\n") can throw an
exception when encountering malformed JSON on a line, which propagates to the
outer catch block and discards all subsequent valid entries. Wrap the
JSON.parse(trimmed) call in a per-line try-catch block so that when parsing
fails, only that specific line is skipped with a continue statement, allowing
the loop to proceed with processing remaining lines. This should be done around
the JSON.parse invocation that feeds into SessionIndexEntrySchema.safeParse.
| function getThreadListItemTitle(thread: unknown): string | null | undefined { | ||
| if (!thread || typeof thread !== "object") { | ||
| return undefined; | ||
| } | ||
|
|
||
| const record = thread as Record<string, unknown>; | ||
| const title = record["title"]; | ||
| if (title === null || typeof title === "string") { | ||
| return title; | ||
| } | ||
|
|
||
| const name = record["name"]; | ||
| if (name === null || typeof name === "string") { | ||
| return name; | ||
| } | ||
|
|
||
| return undefined; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Uses unknown type and runtime introspection instead of Zod schema.
As per coding guidelines, unknown is forbidden and type introspection must use Zod. Define a schema for the thread list item shape and use .safeParse() instead of manual type checks.
♻️ Proposed refactor using Zod
+const ThreadListItemTitleSchema = z.object({
+ title: z.union([z.string(), z.null()]).optional(),
+ name: z.union([z.string(), z.null()]).optional(),
+}).passthrough();
+
-function getThreadListItemTitle(thread: unknown): string | null | undefined {
- if (!thread || typeof thread !== "object") {
- return undefined;
- }
-
- const record = thread as Record<string, unknown>;
- const title = record["title"];
- if (title === null || typeof title === "string") {
- return title;
- }
-
- const name = record["name"];
- if (name === null || typeof name === "string") {
- return name;
- }
-
- return undefined;
-}
+function getThreadListItemTitle(thread: z.input<typeof ThreadListItemTitleSchema>): string | null | undefined {
+ const parsed = ThreadListItemTitleSchema.safeParse(thread);
+ if (!parsed.success) {
+ return undefined;
+ }
+
+ const { title, name } = parsed.data;
+ if (title !== undefined) {
+ return title;
+ }
+ if (name !== undefined) {
+ return name;
+ }
+ return undefined;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function getThreadListItemTitle(thread: unknown): string | null | undefined { | |
| if (!thread || typeof thread !== "object") { | |
| return undefined; | |
| } | |
| const record = thread as Record<string, unknown>; | |
| const title = record["title"]; | |
| if (title === null || typeof title === "string") { | |
| return title; | |
| } | |
| const name = record["name"]; | |
| if (name === null || typeof name === "string") { | |
| return name; | |
| } | |
| return undefined; | |
| } | |
| const ThreadListItemTitleSchema = z.object({ | |
| title: z.union([z.string(), z.null()]).optional(), | |
| name: z.union([z.string(), z.null()]).optional(), | |
| }).passthrough(); | |
| function getThreadListItemTitle(thread: z.input<typeof ThreadListItemTitleSchema>): string | null | undefined { | |
| const parsed = ThreadListItemTitleSchema.safeParse(thread); | |
| if (!parsed.success) { | |
| return undefined; | |
| } | |
| const { title, name } = parsed.data; | |
| if (title !== undefined) { | |
| return title; | |
| } | |
| if (name !== undefined) { | |
| return name; | |
| } | |
| return undefined; | |
| } |
🤖 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/server/src/agents/adapters/codex-agent.ts` around lines 2910 - 2927, The
getThreadListItemTitle function uses unknown type and manual runtime type
introspection with typeof checks instead of Zod validation. Define a Zod schema
that describes the expected thread object shape with optional title and name
string properties, then replace the manual type checking logic with Zod's
safeParse method. When safeParse succeeds, extract the title or name from the
validated data object, and return undefined if validation fails. This ensures
type safety and consistency with the codebase's validation patterns.
Source: Coding guidelines
| const existingIndex = current.findIndex( | ||
| (profile) => profile.baseUrl === baseUrl || profile.name === name, | ||
| ); |
There was a problem hiding this comment.
Use baseUrl as the upsert identity; matching by name risks profile corruption.
This can overwrite a different saved server when two profiles share a name, which breaks multi-device profile persistence.
Proposed fix
const current = readServerProfiles();
const existingIndex = current.findIndex(
- (profile) => profile.baseUrl === baseUrl || profile.name === name,
+ (profile) => profile.baseUrl === baseUrl,
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const existingIndex = current.findIndex( | |
| (profile) => profile.baseUrl === baseUrl || profile.name === name, | |
| ); | |
| const existingIndex = current.findIndex( | |
| (profile) => profile.baseUrl === baseUrl, | |
| ); |
🤖 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/web/src/lib/server-profiles.ts` around lines 75 - 77, The findIndex
method in the profile matching logic is using both baseUrl OR name as matching
criteria, which risks overwriting different profiles that happen to share the
same name. Change the condition to only match by baseUrl, removing the OR clause
that checks profile.name === name, since baseUrl is the unique identifier that
should determine profile identity for upsert operations.
1 participant
Summary
Notes
Connection profiles and access keys are stored in the browser's localStorage for the active client. They are not bundled into the deployed web app.
Validation
The production build still reports the existing large chunk warning from Vite.
Summary by CodeRabbit
Release Notes
New Features
Improvements