Mobile QoL: remote project add, auto-refresh, and daemon-restart recovery#455
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd5b109622
ℹ️ 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 (connectionStateRef.current === "live") { | ||
| lastThreadEventAtRef.current = Date.now(); |
There was a problem hiding this comment.
Mark stream live on activity events
Gating lastThreadEventAt updates behind connectionStateRef.current === "live" means the hook now relies on thread/live_heartbeat to ever enter/maintain live mode, but the in-repo backends currently emit attach/detach only (see src-tauri/src/codex/mod.rs and src-tauri/src/bin/codex_monitor_daemon.rs thread/live_attached/thread/live_detached paths) and no heartbeat producer. In that environment the state can stay stuck in polling, so the stale watchdog (connectionStateRef.current === "live") never triggers after daemon interruptions and live-recovery logic is effectively bypassed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 2f35d4b.
What changed:
- In
useRemoteThreadLiveConnection, thread activity events (item/*,turn/*,error,thread/tokenUsage/updated) now updatelastThreadEventAtRefand set connection state to"live". - Added regression test:
promotes polling state to live on thread activity without heartbeat.
Validation run:
npm run test -- src/features/app/hooks/useRemoteThreadLiveConnection.test.tsx src/features/app/hooks/useRemoteThreadRefreshOnFocus.test.tsx src/features/workspaces/hooks/useWorkspaceRefreshOnFocus.test.tsx src/features/workspaces/hooks/useWorkspaces.test.tsx src/features/app/components/Sidebar.test.tsxnpm run typecheck
|
Addressed a follow-up reconnect-churn risk from RepoPrompt review in commit 274d092. Change summary:
Validation run:
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 274d092bc5
ℹ️ 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 (isDocumentVisible() && isWindowFocused()) { | ||
| void reconnectLive(activeWorkspaceId, selectedThreadId, { | ||
| runResume: true, |
There was a problem hiding this comment.
Ignore self-triggered detach events before reconnecting
This unconditional reconnect on thread/live_detached fires even when the detach was initiated locally by reconnectLive itself (the dedupe path now calls threadLiveUnsubscribe before re-subscribing for the same key). In a visible/focused window, that unsubscribe emits thread/live_detached, which re-enters reconnectLive and creates overlapping reconnect sequences for the same thread, causing redundant resume/subscribe calls and racey state transitions instead of a single reconnect attempt.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in commit 16ee857.
Fixes:
- Added a self-detach ignore guard for the key currently being dedupe-unsubscribed, so a locally triggered thread/live_detached event does not recursively re-enter reconnect.
- Added in-flight same-key reconnect coalescing to avoid concurrent duplicate subscribe/resume attempts.
- Added regression tests for both behaviors.
Validation:
- npm run test -- src/features/app/hooks/useRemoteThreadLiveConnection.test.tsx src/features/app/hooks/useRemoteThreadRefreshOnFocus.test.tsx src/features/workspaces/hooks/useWorkspaceRefreshOnFocus.test.tsx src/features/workspaces/hooks/useWorkspaces.test.tsx src/features/app/components/Sidebar.test.tsx
- npm run typecheck
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f634386d30
ℹ️ 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".
| const staleLive = | ||
| connectionStateRef.current === "live" && | ||
| lastEventAt > 0 && | ||
| now - lastEventAt >= REMOTE_LIVE_STALE_TIMEOUT_MS; |
There was a problem hiding this comment.
Skip stale-live recovery when thread is idle
This stale check assumes that no events for REMOTE_LIVE_STALE_TIMEOUT_MS means a broken connection, but it never gates on activeThreadIsProcessing; as a result, once a normal item/* or turn/* event marks the connection live, an idle thread will be treated as stale after 20s and trigger a false “Server connection interrupted” toast plus reconnect. I checked the backend emitters in src-tauri/src/codex/mod.rs and src-tauri/src/bin/codex_monitor_daemon.rs, and they emit attach/detach plus thread activity, not a guaranteed heartbeat cadence for idle periods, so this branch can fire during healthy idle sessions.
Useful? React with 👍 / 👎.
|
Never again PR like that with a bit of everything. ok? Overall, there is stuff I like, but I'm fixing some other things that I don't like and that are downgrading. |
|
Overall best thing is that it can finally reconnect polling thread to live and that's so good! |
…very (Dimillian#455) Co-authored-by: Thomas Ricouard <ricouard77@gmail.com>
What
Why
How
Testing status