Skip to content

Native Codex App Server integration + workspace/thread persistence#8

Merged
hdprajwal merged 13 commits into
mainfrom
feat/codex-native-support
May 26, 2026
Merged

Native Codex App Server integration + workspace/thread persistence#8
hdprajwal merged 13 commits into
mainfrom
feat/codex-native-support

Conversation

@hdprajwal

@hdprajwal hdprajwal commented May 26, 2026

Copy link
Copy Markdown
Owner

Summary

Brings native Codex App Server support to QuackCode and adds the missing persistence/UX layer around it.

Codex integration (Phases 0–5):

  • App-server protocol spike, runtime skeleton + sidebar status row
  • Codex pane with threads, composer, approvals
  • Auth (browser + device-code) and history drawer
  • Version capture, archive, desktop notifications
  • Tool-call rendering, markdown, transcript width fix
  • Unified terminals + Codex threads in the sidebar; expanded Codex pane

New in the tail of this branch:

  • Claude Code CLI option in the workspace "+" menu — spawns a PTY running `$SHELL -lc claude` so the interactive session starts without typing anything.
  • Codex threads auto-name from the first user message; user/server-supplied titles are preserved.
  • Open workspaces persisted to `~/.config/quackcode/workspaces.json` and restored on next launch (missing paths skipped).
  • Codex threads (with transcript turns/items) persisted to `~/.config/quackcode/codex_threads.json`, relinked to projects by path on load, server thread ids preserved so conversations resume.

Test plan

  • `cargo fmt --check` clean
  • `cargo test` — 118 passed, 0 failed (includes new tests for auto-title derivation, workspace persistence, and Codex thread persistence round-trips)
  • Manual: open the app, create a Codex thread + a Claude CLI session, restart, verify the thread reappears in the sidebar with its transcript and the same workspaces are open
  • Manual: verify thread title updates to the first prompt's text on submit
  • Manual: verify the "+" menu offers New Terminal / New Codex Thread / New Claude Code CLI

Summary by CodeRabbit

  • New Features

    • Dual-mode workspace with integrated Codex AI pane (sign-in, history, transcript, composer)
    • Codex thread lifecycle: create, resume, fork, archive, and message approvals
    • Composer: rich editing, image attachments, link handling, and Markdown-rendered agent messages
    • Sidebar new-item menu to start terminals or Codex sessions
  • Improvements

    • Thread and workspace persistence across launches
    • Keyboard shortcuts for Codex composer and paste/submit flows
    • Session spawning supports initial command overrides

Review Change Stack

hdprajwal added 11 commits May 25, 2026 16:16
Drives `codex app-server --listen stdio://` over JSONL with hand-rolled
frame, pending-response table, and child-process driver under
src/codex_spike/. Tests cover JSON-RPC framing (header-less wire format),
pending correlation, and a live #[ignore]-gated end-to-end flow against
real codex (initialize → account/read → thread/start → turn/start →
agent deltas → turn/completed → clean shutdown).

Shutdown drops stdin to send EOF, since the user-installed `codex` is
typically a node shim that orphans the rust binary on SIGKILL.
Replaces the Phase 0 spike with the real src/app/codex/ module tree:
rpc (frame + pending), schema (status mapping + sandbox enum), thread
(reducer for thread/turn/item notifications), auth, approval (placeholder),
process (child driver), and runtime (CodexRuntimeHandle, command/event
loop, account/read on EnsureStarted, agent-message-delta coalescer).

AppView gains codex_runtime/status/account fields plus handle_codex_event,
wired through a new flume consumer in app::run mirroring dirty/exited/bell.
Sidebar renders a Codex status row (disconnected / starting / ready / signed
out / rate limited / failed). Runtime is lazy: spawned at startup but the
codex app-server child isn't launched until something sends EnsureStarted.

Tests: 57 unit tests + one #[ignore] live integration test that drives the
runtime end-to-end (initialize → account/read → status events → shutdown).
Phase 2 wires a native Codex pane that replaces the terminal canvas
when active:

- AppView gains content_mode, codex_threads, active_codex_thread,
  next_codex_thread_id, and a codex_composer buffer.
- New CodexCommand variants StartThread/SubmitPrompt/InterruptTurn
  plus CodexEvent::ThreadStarted/ThreadStartFailed reconcile a local
  Draft thread with the server's threadId.
- thread/start uses sandbox "workspace-write" (kebab-case) and
  turn/start sends `input` (not `items`) — both confirmed by Phase 0.
- The sidebar shows a "Codex" group per project with thread rows,
  running/waiting status dots, unread markers, and a + button to
  start a new thread.
- Ctrl+Shift+X opens the pane and lazy-starts the runtime;
  Ctrl+Shift+Enter submits; Esc returns to the terminal. Composer
  keys (printable chars, backspace, newline) are captured locally.
- ServerNotification routing finds the right thread by server_thread_id
  via the new notification_thread_id helper and delegates to the
  existing thread::apply_notification reducer.

Phase 3 lifts pending approvals into the UI:

- ApprovalDecision + decision_payload move into codex/approval.rs.
  Permissions approvals send { permissions } (granting the requested
  subset on Accept and nothing on Decline); command/file-change
  approvals send { decision }, omitting acceptForSession per spec.
- dispatch_codex_server_request parses the three supported approval
  methods, stashes a CodexApproval on the matching thread, and lifts
  status to WaitingForApproval.
- The Codex pane renders Accept/Decline/Cancel cards inline; the
  sidebar shows an approval count badge on background threads.
- serverRequest/resolved clears pending approvals across threads,
  even when the server resolves without a user decision (turn
  interrupt).

Adds CodexCommand::ResolveServerRequest plumbing through the runtime
so user decisions reach the app-server as JSON-RPC responses to the
original request id.

69 unit tests pass (+5 for approval decision payloads, +6 for the
new runtime helpers); the live #[ignore] integration test still
passes in 0.34s.
Implements the Phase 4 surface from the spec: ChatGPT browser login,
device-code fallback, rate-limit overlay, and `thread/list` / `resume`
/ `fork` for prior sessions.

Wire / runtime:
- `account/login/start` (chatgpt + chatgptDeviceCode), `account/login/
  cancel`, and the `xdg-open` browser opener live in the runtime; failure
  to open the browser surfaces as `browser_open_failed` so the UI can
  fall through to device code without a user round-trip.
- `account/login/completed`, `account/updated`, and `account/rateLimits/
  updated` notifications drive the AppView's account state directly.
  Rate-limit overlays preserve the underlying account so the email
  stays visible.
- `thread/list` is filtered by project cwd. `thread/resume` and
  `thread/fork` reuse the existing draft-thread reconciliation path —
  the runtime echoes `ThreadStarted` carrying the same `local_id` the
  GPUI side allocated.

UI:
- Codex pane gates the transcript behind a sign-in card when the
  account state is `SignedOutRequiresAuth` or a login is pending. The
  card surfaces the browser URL, falls back to device code on browser-
  open failure, and shows the user code prominently.
- Pane header gains History + Fork buttons; History toggles a drawer
  that renders the `thread/list` result and lets the user resume a
  thread into a fresh local record.
- Rate-limit banner renders above the transcript when the account
  state is `RateLimited`.

Tests:
- 19 new pure-helper tests cover login params/parsing, rate-limit
  payload parsing + state overlay, and thread-list/resume/fork wire
  shapes (88 unit tests pass overall).
- Live `runtime_ensure_started_round_trip` still passes in ~0.33s.
Tightly-scoped polish pass focused on the highest-value items from the
spec's Phase 5 list. The settings UI (nested config-shape migration),
file-diff transcript rendering, skill mentions, inline rename, and the
5-minute approval expiry stay deferred and are tracked in
`implementation-notes.html`.

Server version capture: `parse_server_version` reads
`serverInfo.version` from the `initialize` response; the runtime emits
`CodexEvent::ServerInfo` and the sidebar status row surfaces it next
to the account state. Useful both as a debug signal and as a starting
point for the spec's "version compatibility" guidance.

Thread archive: `CodexCommand::ArchiveThread` wraps
`thread/archive` / `thread/unarchive`. The reducer already reacted to
the resulting `thread/archived` notification (Phase 1 work), so the
client side is just an inline pane-header button that returns the
user to the terminal once the command is queued.

Desktop notifications: background-thread approval requests and turn
completions fire a `notify-rust` toast. Foreground activity stays
silent — the existing unread/approval-count sidebar badges still hold
for the active view, the toast just covers "user is in another pane
when the long-running task finishes".
Four real-world issues showed up once the pane was used against a live
codex run; this fixes all four together since they cluster around the
item-render path.

Tool rows were empty because extract_item_text only knew agentMessage
and userMessage. It now dispatches on kind: commandExecution probes
command (string or argv array), cwd, exit code (both wire shapes), and
stdout/stderr; fileChange probes paths or path plus unifiedDiff;
reasoning probes summary or parts[].text. All probes tolerate missing
fields.

Reasoning summaries lingered as empty rows. Added two reducer arms
for item/reasoning/summaryTextDelta and summaryPartAdded that
accumulate into a transient Reasoning item, and made item/completed
for a Reasoning item remove it from the turn instead of marking it
complete — so the row appears while the model is thinking and
disappears as soon as it moves on.

Long transcript text pushed the composer's Send button off-screen.
The scroll container, every item/approval row, and the composer now
carry w_full() + min_w_0(); the composer adds overflow_hidden() on
the textarea and flex_shrink_0() on Send so the button stays anchored
no matter the response length.

Tool rows now render as a one-line "Tool - Bash <cmd>" /
"Tool - Edit <path>" summary backed by a per-item-id HashSet on
AppView; clicking toggles a monospace expanded body below the row.

AgentMessage items route through a new codex_markdown module that
parses with markdown-rs (1.0) and walks the mdast into GPUI blocks:
paragraphs, headings (depth -> size offset), bulleted/ordered lists
with indented tail blocks, fenced code on surface_background, block
quotes with a border_l_2, thematic breaks. Inline emphasis/strong/
code/links flatten into their containing paragraph with markdown
markers preserved so word-wrap stays correct (GPUI 0.2.2 doesn't
make per-span styling easy inside a wrapping paragraph).

103 unit tests pass (+12 new) and the live integration test still
passes in ~0.36s.
- Drop the dedicated "Codex" subheader; render terminals and threads
  as one flat list under each workspace, with a palette icon to
  distinguish threads.
- The "+" button next to a workspace now opens a popup with two
  options: New Terminal / New Codex Thread. Click-outside dismisses.
- Wire localImage attachments through turn/start input.
- Composer key handling (paste, navigation, submit) and richer
  transcript rendering (tool groups, linkable file paths).
Adds a third entry next to New Terminal / New Codex Thread that spawns a
regular PTY in the project root with the user's login shell running
'claude' directly via -lc, so the interactive session starts without
having to type the command.
Default "New thread" placeholder is replaced with a short single-line
title derived from the first user message — both eagerly at submit time
and again when the server echoes the userMessage item. Custom titles set
by the server (thread/name/updated) or the user are preserved.
Saved list of project root paths lives at
~/.config/quackcode/workspaces.json. On startup the app loads the saved
set (skipping paths whose directories no longer exist) and falls back to
the launch cwd when no file is present. add_project writes the file each
time the list changes, so the project sidebar reopens to the same state
on next launch.
Threads (with their turn/item transcripts) are saved to
~/.config/quackcode/codex_threads.json each time the reducer mutates
them or the user performs a thread action (new/fork/archive/resume/
submit). On launch the saved set is reloaded and relinked to current
projects by path; threads whose project is no longer open are skipped.
Local thread ids are reassigned on load, the server-side thread ids are
preserved so the user can keep talking to an existing thread.
@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR introduces a complete Codex AI assistant integration into Quackcode. The implementation adds a native app-server subprocess integration over JSON-RPC, client-side thread/turn/approval state management with disk persistence, a Markdown-rendered Codex pane UI, and wiring for workspace persistence, input handling, and app startup. The feature enables users to create Codex threads within projects, compose prompts with optional image attachments, view agent responses with rendered formatting, handle approval requests, and persist workspace state across sessions.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Poem

🐰 In Quackcode's warren, Claude now hops about,
Markdown thoughts flow, with images clout,
Threads weave through time, persisted with care,
Approval cards glow in the composer's lair,
One pane for code, another for thought—
Integration complete, and tightly it's wrought!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.23% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main additions: native Codex App Server integration and workspace/thread persistence, which are the core features across all the file changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/codex-native-support

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 9

🤖 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 `@src/app/app_view.rs`:
- Around line 465-469: The current handle_codex_event unconditionally calls
persist_codex_threads on every CodexEvent, causing excessive disk I/O; change it
to persist only when the event actually mutates durable thread state by either
(a) making handle_codex_event_inner return a bool (or enum) like
did_persist_needed indicating whether state changed and then call
persist_codex_threads only when true, or (b) match on event variants inside
handle_codex_event and only call persist for mutation events; alternatively,
implement a debounce/coalescing mechanism that batches writes (e.g., schedule a
single delayed persist) if high-frequency notification events occur—update
handle_codex_event, handle_codex_event_inner, and any scheduled-persist helper
accordingly.

In `@src/app/codex_view.rs`:
- Around line 155-160: The click handler is being registered even when the
control is disabled; change the fluent builder so handler registration is gated
by the `disabled` flag: move the `.on_mouse_down(MouseButton::Left, listener)`
call into the `when(!disabled, |el| { ... })` branch (or conditionally call
`.on_mouse_down` only when `!disabled`) and ensure the
`.cursor_pointer().hover(|s| s.opacity(0.8))` and `.on_mouse_down(...)` are both
inside that same `when(!disabled, |el| { ... })` closure; apply the same change
for the other occurrence around the `1152-1168` block so `disabled`,
`.on_mouse_down`, `MouseButton::Left`, and `listener` are never registered when
`disabled` is true.
- Around line 1191-1194: The InteractiveText element id is currently built only
from stable_text_id(&text) (in InteractiveText::new / ElementId::Name), which
causes duplicate messages to share the same id; change the id construction to
include a per-item stable key (for example combine the server item id and the
item's index or another unique per-item identifier along with stable_text_id) so
each rendered message gets a unique ElementId::Name; update the call site that
creates InteractiveText (the block using InteractiveText::new and
StyledText::new.with_highlights) to concatenate or format the id from
stable_text_id(&text) plus the item-specific key (e.g.,
"{server_id}-{index}-{stable_text_id}") so interaction targeting is stable and
unique.

In `@src/app/codex/approval.rs`:
- Around line 38-47: Currently the Accept branch returns the raw
approval.params["permissions"]; instead, use the typed/normalized permissions
stored on the CodexApproval model (e.g., a field like CodexApproval::permissions
or similar) and build the response from that normalized structure; update the
code path that handles ApprovalDecision::Accept (the block checking
approval.method == "item/permissions/requestApproval") to read the permissions
from the CodexApproval instance, serialize only the approved/validated fields,
and return json!({ "permissions": codex_approval.permissions }) rather than
echoing approval.params so unvalidated fields are never granted.

In `@src/app/codex/persistence.rs`:
- Around line 135-141: The current save path uses fs::write(path, bytes) which
can corrupt the live codex_threads.json if interrupted; change the save logic in
the function that calls serde_json::to_vec_pretty(&saved) (the block handling
Ok(bytes)) to perform an atomic replace: create a temporary file in the same
directory (e.g., derive tmp name from path), write the bytes there and
flush/sync the file, then rename (fs::rename) the temp file over path only after
the write succeeds; ensure any write/sync errors are handled the same way you
currently log fs::write errors and that the temp file is removed on failure.

In `@src/app/codex/process.rs`:
- Around line 163-170: The request() call registers a waiter via
self.pending.register() before sending; if self.write_frame(&frame) fails the
waiter is left in PendingTable. Fix by unregistering/clearing the pending entry
on write failure (after obtaining id from self.pending.register()), e.g. call
the appropriate removal method on PendingTable (self.pending.unregister(id) or
self.pending.remove(id)) immediately when write_frame() returns Err, then return
the write error so you don't leak the waiter; ensure the original error is
propagated.

In `@src/app/codex/runtime.rs`:
- Around line 584-619: The relay currently forwards ServerEvent->CodexEvent 1:1
because Coalescer::absorb is stateless; change Coalescer into a stateful
coalescer that buffers and merges consecutive item/agentMessage/delta
notifications by itemId and only emits merged CodexEvent(s) when a different
event arrives or a flush condition is met. Concretely: make Coalescer hold an
internal map or last_pending entry keyed by itemId (or the identifying field in
ServerEvent notifications), update/merge delta payloads inside Coalescer::absorb
when it sees an item/agentMessage/delta for the same itemId, and return an empty
Vec until a non-mergeable event forces emission of the merged CodexEvent(s);
update relay_server_events to call coalescer.absorb for each received
ServerEvent, send any returned CodexEvent(s) to app, and on loop exit (channel
closed) call a Coalescer::flush method to emit any remaining buffered events
before returning.

In `@src/app/input.rs`:
- Around line 95-99: When ContentMode::Codex is active and the composer has
focus, keystrokes must not fall through to the PTY; update the input-handling
logic so that when this.content_mode == ContentMode::Codex &&
!this.sidebar_focused you do not forward keys to the terminal: either (A) if
handle_codex_composer_key(this, ks, mods, cx) is called, make the Codex path
always consume the key (return) even when handle_codex_composer_key returns
false, or (B) keep handle_codex_composer_key as-is but add a guard around the
PTY/terminal dispatch (the code currently forwarding keystrokes to the PTY on
lines 122-124) so it only runs when this.content_mode != ContentMode::Codex; use
the identifiers this.content_mode, ContentMode::Codex, this.sidebar_focused,
handle_codex_composer_key, and the PTY/terminal dispatch site to locate and
change the logic.

In `@src/app/mod.rs`:
- Around line 55-58: The code unconditionally calls
codex_handle.send(CodexCommand::EnsureStarted) which breaks lazy-start; change
it so EnsureStarted is sent only when Codex is likely to be used (e.g., behind
the existing lazy-start condition or a user-action hook). Locate the call to
codex_handle.send and wrap it with the appropriate conditional (the same
predicate used elsewhere for lazy-start, or a new flag like should_prewarm_codex
derived from user config/UI intent) or move the send into the handler that runs
when the user opens Codex; ensure the symbol CodexCommand::EnsureStarted remains
unchanged and only its invocation is gated.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1137773f-f7c9-40b1-a7b0-b461dd1405e2

📥 Commits

Reviewing files that changed from the base of the PR and between 1e20202 and 39f80e7.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (19)
  • Cargo.toml
  • src/app/app_view.rs
  • src/app/codex/approval.rs
  • src/app/codex/auth.rs
  • src/app/codex/mod.rs
  • src/app/codex/persistence.rs
  • src/app/codex/process.rs
  • src/app/codex/rpc.rs
  • src/app/codex/runtime.rs
  • src/app/codex/schema.rs
  • src/app/codex/thread.rs
  • src/app/codex_markdown.rs
  • src/app/codex_view.rs
  • src/app/input.rs
  • src/app/mod.rs
  • src/app/project.rs
  • src/app/session.rs
  • src/app/sidebar.rs
  • src/app/workspace_state.rs

Comment thread src/app/app_view.rs
Comment thread src/app/codex_view.rs Outdated
Comment thread src/app/codex_view.rs
Comment thread src/app/codex/approval.rs
Comment thread src/app/codex/persistence.rs
Comment thread src/app/codex/process.rs
Comment thread src/app/codex/runtime.rs
Comment thread src/app/input.rs Outdated
Comment thread src/app/mod.rs Outdated
hdprajwal added 2 commits May 26, 2026 08:55
- Persist codex_threads.json only at durable checkpoints (thread/turn/item
  events), not on every notification — keeps per-token deltas off the disk.
- Atomic write for codex_threads.json via temp file + rename so an
  interrupted save can't leave a partial JSON that load_codex_threads_from
  silently treats as empty state.
- Clean up the pending RPC waiter when write_frame fails so repeated send
  failures don't leak entries until cancel_all.
- Activate delta coalescing in the live relay path: drain bursts via
  try_recv and run coalesce_agent_message_deltas before pushing into the
  bounded app channel.
- Stop forwarding keystrokes to the PTY when ContentMode::Codex owns
  focus — unhandled keys like Up/Tab/Ctrl-C no longer interrupt the
  hidden terminal.
- Gate header_button and composer_button click handlers behind
  !disabled so disabled controls are truly non-interactive.
- Build unique InteractiveText element ids from a per-item stable key
  (server_item_id) so duplicate messages don't share an id.
CI was failing on `cargo clippy --all-targets -- -D warnings`:
- derive Default for CodexAccountState / SandboxMode via #[default]
- start_client takes &Path, not &PathBuf
- compare PathBuf with &Path / DEFAULT_CODEX_BINARY str directly
- replace `while let Ok(_) = …` with `.is_ok()`
- box ErrorObject inside ClientError::Rpc so the enum stays small
- allow too_many_arguments on spawn_pty_session_with_command

Also addresses the two remaining CodeRabbit findings:
- CodexApproval now captures the permission grant at construction time
  (CodexApproval::new) into `granted_permissions`. Accept builds its
  response from that field, so a permissions field the approval UI didn't
  render can't be granted by a later raw-params read. Adds a regression
  test that mutates `params` post-construction and confirms the response
  still reflects the originally-captured grant.
- Drop the unconditional CodexCommand::EnsureStarted prewarm at app
  launch. ensure_codex_started() already runs on every entry point that
  opens Codex (open_codex_pane / new_codex_thread /
  new_codex_thread_in_project / login flow), and the worker is a single
  FIFO queue, so the prewarm only saved binary-spawn latency at the cost
  of running the child for users who never open Codex.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/app/app_view.rs (1)

640-657: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Recompute thread status after resolving the last approval.

This branch removes the pending approval but never mirrors the recompute_status(...) call used when approvals are added below. If that was the last pending approval, the thread can stay stuck in WaitingForApproval until some later notification happens.

Suggested fix
         if method == "serverRequest/resolved" {
             if let Some(req_id) = params.get("requestId").and_then(|v| v.as_u64()) {
                 let mut changed = false;
                 for t in &mut self.codex_threads {
                     if let Some(idx) = t
                         .pending_approvals
                         .iter()
                         .position(|a| a.request_id == req_id)
                     {
                         t.pending_approvals.remove(idx);
+                        t.recompute_status(crate::app::codex::ServerThreadStatus::Active);
                         changed = true;
                     }
                 }
                 if changed {
                     cx.notify();

Also applies to: 738-740

🤖 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 `@src/app/app_view.rs` around lines 640 - 657, When removing a pending approval
in the "serverRequest/resolved" branch, the code updates codex_threads but never
calls recompute_status for the affected thread(s), which can leave a thread
stuck in WaitingForApproval; after removing the approval (inside the loop where
pending_approvals.remove(idx) and changed = true), call the same
recompute_status(...) used elsewhere for that thread (use the same parameter
pattern as the add-approval branch) so the thread's status is recalculated
immediately, and keep the existing cx.notify() behavior; apply the same fix to
the other identical spot referenced (the branch around the 738-740 area).
src/app/mod.rs (1)

65-73: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't overwrite the saved workspace list with the filtered startup set.

Filtering load_workspaces() down to existing directories and then immediately calling save_workspaces(&initial_paths) turns a transiently missing workspace into a permanent config loss. If all saved paths are unavailable at launch, this also replaces the whole list with cwd.

💡 Minimal fix
-            let saved_workspaces: Vec<PathBuf> = load_workspaces()
-                .into_iter()
+            let saved_workspaces = load_workspaces();
+            let restored_workspaces: Vec<PathBuf> = saved_workspaces
+                .iter()
+                .cloned()
                 .filter(|p| p.is_dir())
                 .collect();
-            let initial_paths: Vec<PathBuf> = if saved_workspaces.is_empty() {
+            let initial_paths: Vec<PathBuf> = if restored_workspaces.is_empty() {
                 vec![cwd.clone()]
             } else {
-                saved_workspaces
+                restored_workspaces
             };
@@
-            save_workspaces(&initial_paths);
+            if saved_workspaces.is_empty() {
+                save_workspaces(&initial_paths);
+            }

Also applies to: 100-102

🤖 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 `@src/app/mod.rs` around lines 65 - 73, The code currently replaces the
persisted workspace list by saving the filtered startup set (initial_paths),
which can permanently lose entries that are temporarily missing; change the
startup logic so load_workspaces() is filtered for runtime use but do not call
save_workspaces(&initial_paths) unconditionally on startup — keep
saved_workspaces intact and only call save_workspaces(...) when the user
explicitly updates workspaces (or when you intentionally prune and persist
invalid entries after explicit confirmation); locate symbols saved_workspaces,
initial_paths, load_workspaces, save_workspaces, and cwd to implement this
conditional save behavior.
🤖 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.

Outside diff comments:
In `@src/app/app_view.rs`:
- Around line 640-657: When removing a pending approval in the
"serverRequest/resolved" branch, the code updates codex_threads but never calls
recompute_status for the affected thread(s), which can leave a thread stuck in
WaitingForApproval; after removing the approval (inside the loop where
pending_approvals.remove(idx) and changed = true), call the same
recompute_status(...) used elsewhere for that thread (use the same parameter
pattern as the add-approval branch) so the thread's status is recalculated
immediately, and keep the existing cx.notify() behavior; apply the same fix to
the other identical spot referenced (the branch around the 738-740 area).

In `@src/app/mod.rs`:
- Around line 65-73: The code currently replaces the persisted workspace list by
saving the filtered startup set (initial_paths), which can permanently lose
entries that are temporarily missing; change the startup logic so
load_workspaces() is filtered for runtime use but do not call
save_workspaces(&initial_paths) unconditionally on startup — keep
saved_workspaces intact and only call save_workspaces(...) when the user
explicitly updates workspaces (or when you intentionally prune and persist
invalid entries after explicit confirmation); locate symbols saved_workspaces,
initial_paths, load_workspaces, save_workspaces, and cwd to implement this
conditional save behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 51870583-ed1a-45f0-9a8b-41be7159923c

📥 Commits

Reviewing files that changed from the base of the PR and between 39f80e7 and fcd6009.

📒 Files selected for processing (12)
  • src/app/app_view.rs
  • src/app/codex/approval.rs
  • src/app/codex/auth.rs
  • src/app/codex/persistence.rs
  • src/app/codex/process.rs
  • src/app/codex/rpc.rs
  • src/app/codex/runtime.rs
  • src/app/codex/schema.rs
  • src/app/codex_view.rs
  • src/app/input.rs
  • src/app/mod.rs
  • src/app/session.rs

@hdprajwal hdprajwal merged commit 6a36c9f into main May 26, 2026
2 checks passed
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.

1 participant