Repository navigation
fix(wework): align Codex session, project, and summary state - #1987
Merged
qdaxb merged 6 commits intoJul 16, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds remote-project synchronization, activation, ordering, and active-state tracking across Codex global state, runtime APIs, and the workbench. It also updates model-selection persistence, executor isolation controls, cloud initialization, device identity handling, environment summaries, connection settings, and desktop task-flow coverage. ChangesRemote project state and runtime integration
Runtime execution and desktop behavior
Device and environment UI
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
icycrystal4
force-pushed
the
fix/wework-codex-state-persistence
branch
2 times, most recently
from
July 16, 2026 09:16
59b6ec3 to
098d94d
Compare
icycrystal4
changed the title
icycrystal4
marked this pull request as ready for review
July 16, 2026 13:15
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
wework/src/features/workbench/WorkbenchProvider.tsx (1)
472-544: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExtract the new project lifecycle and sidebar-state responsibilities from oversized modules.
wework/src/features/workbench/WorkbenchProvider.tsx#L472-L544: extract synchronization, restoration, and activation into a dedicated hook.wework/src/components/layout/DesktopSidebar.tsx#L2549-L2599: extract sidebar project-state derivation and pinned ordering.wework/src/components/layout/DesktopSidebar.tsx#L3134-L3348: move sortable identity and reorder request construction into that abstraction.As per coding guidelines, “Keep functions focused, preferably under 50 lines” and “Split source files over 1000 lines.”
🤖 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 `@wework/src/features/workbench/WorkbenchProvider.tsx` around lines 472 - 544, Extract the synchronization, restoration, and activation effects around localRuntimeStateDeviceId from WorkbenchProvider.tsx into a dedicated project-lifecycle hook, with no direct change required at the sidebar sites until that abstraction is available. In DesktopSidebar.tsx, extract sidebar project-state derivation and pinned ordering from lines 2549-2599, and move sortable identity plus reorder-request construction from lines 3134-3348 into the same sidebar-state abstraction; update callers to use the extracted hook while preserving existing behavior.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 `@executor/src/runtime_work/codex_global_state.rs`:
- Around line 305-331: Update sync_codex_global_remote_projects to reconcile the
existing remote-project state with the supplied snapshot instead of only
appending upserts: track the normalized remote IDs and labels provided, remove
projects omitted from the snapshot, and explicitly clear stored labels when an
entry has no label. Preserve the existing validation and refresh flow, and add
regression coverage for empty and reduced snapshots.
- Around line 1180-1189: Update the remote activation branch in
apply_project_ui_state to clear or replace ACTIVE_WORKSPACE_ROOTS_KEY before
setting ACTIVE_REMOTE_PROJECT_ID_KEY and SELECTED_REMOTE_HOST_ID_KEY, ensuring
previous local roots are not preserved. Add a local-to-remote activation test
asserting that only the remote project remains active.
In `@wework/src/api/hybrid/hybridServices.ts`:
- Around line 346-350: Update the work.chats iteration in the localDeviceIds
population flow to add a chat’s deviceId only when its workspaceSource is not
'remote', matching the filtering already used in the project.deviceWorkspaces
loop. Keep local chat devices registered and exclude remote chat workspaces.
In `@wework/src/api/local/localServices.ts`:
- Around line 1512-1514: Update the filtering condition in the task-processing
flow around rawTasks and workspaceSource so empty remote projects are matched
using a stable project identity, such as the project key or repository
fingerprint, rather than the display label. Ensure unrelated local and remote
projects with the same label are both preserved.
In `@wework/src/components/settings/ConnectionsSettingsPage.tsx`:
- Around line 320-331: Update deviceDisplayName to retain the existing alias
check, then replace the raw client_ip fallback with
getWorkbenchDeviceUnavailableDisplayName so runtime_transfer_host and
network-host normalization are reused. Preserve the existing name and device_id
fallback behavior.
In `@wework/src/features/workbench/useWorkbenchProjectActions.ts`:
- Around line 154-166: The secondary state-owner mutations in
useWorkbenchProjectActions.ts at lines 154-166 and 199-210 discard RPC
responses. Capture each renameRuntimeWorkspace and removal response, then handle
accepted: false exactly as the corresponding primary rename and removal
operations do, preserving successful behavior while reporting rejected
mutations.
In `@wework/src/features/workbench/useWorkbenchRuntimeMessaging.ts`:
- Line 1092: Update both runtimeTask assignments in
wework/src/features/workbench/useWorkbenchRuntimeMessaging.ts at lines 1092-1092
and 1143-1143 to fall back to state.currentRuntimeTask only when
runtimeTaskOverride is undefined, preserving explicit null values.
In `@wework/src/features/workbench/WorkbenchProvider.tsx`:
- Around line 495-517: Update the project-restore useEffect around
findFirstSelectableProject so it waits while state.isBootstrapping is true
before setting lastProjectRestoreAttemptedRef.current or selecting a fallback
project. Add state.isBootstrapping to the effect dependency list, preserving the
existing route, selection, and workspace guards.
- Around line 477-493: Update the remote-project synchronization effect around
getRuntimeRemoteProjectRegistrations so it still calls syncRuntimeRemoteProjects
with projects: [] when localRuntimeStateDeviceId is valid but the project list
is empty. Remove only the projects.length === 0 early-return condition, while
preserving the device-ID guard, signature deduplication, refresh, and error
handling.
---
Nitpick comments:
In `@wework/src/features/workbench/WorkbenchProvider.tsx`:
- Around line 472-544: Extract the synchronization, restoration, and activation
effects around localRuntimeStateDeviceId from WorkbenchProvider.tsx into a
dedicated project-lifecycle hook, with no direct change required at the sidebar
sites until that abstraction is available. In DesktopSidebar.tsx, extract
sidebar project-state derivation and pinned ordering from lines 2549-2599, and
move sortable identity plus reorder-request construction from lines 3134-3348
into the same sidebar-state abstraction; update callers to use the extracted
hook while preserving existing behavior.
🪄 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: f9d4b70d-2008-4d54-94e5-0d77adda85ec
📒 Files selected for processing (52)
executor/src/runtime_work/codex_global_state.rsexecutor/src/runtime_work/handler.rsexecutor/src/runtime_work/response.rsexecutor/tests/app_runtime_work_workspace_contract.rswework/e2e/desktop/task-flow.e2e.mjswework/scripts/dev-mac-app.shwework/src-tauri/src/local_executor.rswework/src/App.tsxwework/src/api/executorAccess.tswework/src/api/hybrid/hybridServices.test.tswework/src/api/hybrid/hybridServices.tswework/src/api/local/localServices.test.tswework/src/api/local/localServices.tswework/src/api/runtimeWork.tswework/src/components/layout/ConversationDeviceOfflineBanner.tsxwework/src/components/layout/DesktopSidebar.test.tsxwework/src/components/layout/DesktopSidebar.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/layout/DeviceStatusPrompt.test.tsxwework/src/components/layout/DeviceStatusPrompt.tsxwework/src/components/layout/EnvironmentInfoPopover.test.tsxwework/src/components/layout/EnvironmentInfoPopover.tsxwework/src/components/layout/MobileWorkbenchLayout.test.tsxwework/src/components/layout/MobileWorkbenchLayout.tsxwework/src/components/layout/workspace-panels/WorkspacePanelActions.test.tsxwework/src/components/layout/workspace-panels/WorkspacePanelActions.tsxwework/src/components/settings/ConnectionsSettingsPage.test.tsxwework/src/components/settings/ConnectionsSettingsPage.tsxwework/src/features/cloud-connection/LocalExecutorCloudBridge.test.tsxwework/src/features/cloud-connection/LocalExecutorCloudBridge.tsxwework/src/features/cloud-connection/localExecutorCloudConnection.tswework/src/features/local-runtime/LocalRuntimeInitializer.test.tsxwework/src/features/local-runtime/LocalRuntimeInitializer.tsxwework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/features/workbench/useWorkbenchProjectActions.tswework/src/features/workbench/useWorkbenchRuntimeMessaging.tswework/src/features/workbench/workbenchCloudStatus.test.tswework/src/features/workbench/workbenchCloudStatus.tswework/src/features/workbench/workbenchContextTypes.tswework/src/features/workbench/workbenchReducer.test.tswework/src/features/workbench/workbenchReducer.tswework/src/features/workbench/workbenchRuntimeHelpers.test.tswework/src/features/workbench/workbenchRuntimeHelpers.tswework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/lib/runtime-project-state.test.tswework/src/lib/runtime-project-state.tswework/src/lib/workbench-device.test.tswework/src/lib/workbench-device.tswework/src/types/api.ts
Comment on lines
+305
to
+331
| pub(crate) fn sync_codex_global_remote_projects( | ||
| projects: &[CodexGlobalRemoteProject], | ||
| ) -> Result<(), String> { | ||
| for project in projects { | ||
| let Some(project_key) = clean_text(&project.id) else { | ||
| continue; | ||
| }; | ||
| let Some(remote_host_id) = clean_text(&project.host_id) else { | ||
| continue; | ||
| }; | ||
| let remote_path = normalize_workspace_path(&project.remote_path); | ||
| if remote_path.is_empty() { | ||
| continue; | ||
| } | ||
| append_codex_global_state_op_record(&CodexGlobalStateOplogRecord { | ||
| kind: OPLOG_KIND_UPSERT_REMOTE_PROJECT.to_owned(), | ||
| workspace_path: remote_path, | ||
| project_key: Some(project_key), | ||
| remote_host_id: Some(remote_host_id), | ||
| label: project.label.as_deref().and_then(clean_text), | ||
| updated_at: now_ms(), | ||
| ..Default::default() | ||
| })?; | ||
| } | ||
| flush_or_watch_codex_global_state_oplog(); | ||
| refresh_codex_global_project_cache(); | ||
| Ok(()) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Reconcile removed projects and cleared labels during remote sync.
This “sync” only upserts supplied entries, so projects omitted from a later snapshot remain indefinitely. Existing labels also survive when a subsequent entry has no label. Reconcile current remote IDs against the supplied set and remove stale labels/projects, with regression coverage for an empty or reduced snapshot.
Also applies to: 1162-1171
🤖 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 `@executor/src/runtime_work/codex_global_state.rs` around lines 305 - 331,
Update sync_codex_global_remote_projects to reconcile the existing
remote-project state with the supplied snapshot instead of only appending
upserts: track the normalized remote IDs and labels provided, remove projects
omitted from the snapshot, and explicitly clear stored labels when an entry has
no label. Preserve the existing validation and refresh flow, and add regression
coverage for empty and reduced snapshots.
Comment on lines
+1180
to
+1189
| if let Some(remote_host_id) = remote_host_id.and_then(clean_text) { | ||
| payload.insert( | ||
| ACTIVE_REMOTE_PROJECT_ID_KEY.to_owned(), | ||
| Value::String(project_key.to_owned()), | ||
| ); | ||
| payload.insert( | ||
| SELECTED_REMOTE_HOST_ID_KEY.to_owned(), | ||
| Value::String(remote_host_id), | ||
| ); | ||
| return; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clear local active roots when activating a remote project.
The remote branch sets active-remote-project-id but preserves active-workspace-roots. apply_project_ui_state will therefore mark both the previous local project and the remote project active.
Proposed fix
if let Some(remote_host_id) = remote_host_id.and_then(clean_text) {
+ payload.remove(ACTIVE_WORKSPACE_ROOTS_KEY);
payload.insert(
ACTIVE_REMOTE_PROJECT_ID_KEY.to_owned(),
Value::String(project_key.to_owned()),Add a local-to-remote activation test asserting that only the remote project remains active.
📝 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.
Suggested change
| if let Some(remote_host_id) = remote_host_id.and_then(clean_text) { | |
| payload.insert( | |
| ACTIVE_REMOTE_PROJECT_ID_KEY.to_owned(), | |
| Value::String(project_key.to_owned()), | |
| ); | |
| payload.insert( | |
| SELECTED_REMOTE_HOST_ID_KEY.to_owned(), | |
| Value::String(remote_host_id), | |
| ); | |
| return; | |
| if let Some(remote_host_id) = remote_host_id.and_then(clean_text) { | |
| payload.remove(ACTIVE_WORKSPACE_ROOTS_KEY); | |
| payload.insert( | |
| ACTIVE_REMOTE_PROJECT_ID_KEY.to_owned(), | |
| Value::String(project_key.to_owned()), | |
| ); | |
| payload.insert( | |
| SELECTED_REMOTE_HOST_ID_KEY.to_owned(), | |
| Value::String(remote_host_id), | |
| ); | |
| return; |
🤖 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 `@executor/src/runtime_work/codex_global_state.rs` around lines 1180 - 1189,
Update the remote activation branch in apply_project_ui_state to clear or
replace ACTIVE_WORKSPACE_ROOTS_KEY before setting ACTIVE_REMOTE_PROJECT_ID_KEY
and SELECTED_REMOTE_HOST_ID_KEY, ensuring previous local roots are not
preserved. Add a local-to-remote activation test asserting that only the remote
project remains active.
Comment on lines
+346
to
350
| project.deviceWorkspaces.forEach(workspace => { | ||
| if (workspace.workspaceSource !== 'remote') localDeviceIds.add(workspace.deviceId) | ||
| }) | ||
| }) | ||
| work.chats.forEach(workspace => localDeviceIds.add(workspace.deviceId)) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Exclude remote chat workspaces from localDeviceIds too.
The project loop filters remote sources, but Line 350 still registers every chat device as local. A remote chat can therefore misroute commands and runtime calls to localServices.
Proposed fix
- work.chats.forEach(workspace => localDeviceIds.add(workspace.deviceId))
+ work.chats.forEach(workspace => {
+ if (workspace.workspaceSource !== 'remote') localDeviceIds.add(workspace.deviceId)
+ })📝 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.
Suggested change
| project.deviceWorkspaces.forEach(workspace => { | |
| if (workspace.workspaceSource !== 'remote') localDeviceIds.add(workspace.deviceId) | |
| }) | |
| }) | |
| work.chats.forEach(workspace => localDeviceIds.add(workspace.deviceId)) | |
| project.deviceWorkspaces.forEach(workspace => { | |
| if (workspace.workspaceSource !== 'remote') localDeviceIds.add(workspace.deviceId) | |
| }) | |
| }) | |
| work.chats.forEach(workspace => { | |
| if (workspace.workspaceSource !== 'remote') localDeviceIds.add(workspace.deviceId) | |
| }) |
🤖 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 `@wework/src/api/hybrid/hybridServices.ts` around lines 346 - 350, Update the
work.chats iteration in the localDeviceIds population flow to add a chat’s
deviceId only when its workspaceSource is not 'remote', matching the filtering
already used in the project.deviceWorkspaces loop. Keep local chat devices
registered and exclude remote chat workspaces.
Comment on lines
1512
to
1514
| if (rawTasks.length === 0 && workspaceSource === 'remote' && localWorkspaceLabels.has(label)) { | ||
| continue | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Do not deduplicate projects by display label.
An empty remote project is dropped whenever an unrelated local project has the same label—for example, /local/frontend and /remote/frontend. Match a stable project identity such as the project key or repository fingerprint instead.
🤖 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 `@wework/src/api/local/localServices.ts` around lines 1512 - 1514, Update the
filtering condition in the task-processing flow around rawTasks and
workspaceSource so empty remote projects are matched using a stable project
identity, such as the project key or repository fingerprint, rather than the
display label. Ensure unrelated local and remote projects with the same label
are both preserved.
Comment on lines
+320
to
+331
| function deviceDisplayName(device: DeviceInfo): string { | ||
| const name = device.name?.trim() | ||
| const defaultNames = [ | ||
| device.device_id, | ||
| device.cloud_config?.deviceName, | ||
| device.remote_config?.deviceName, | ||
| ] | ||
| .map(value => value?.trim()) | ||
| .filter(Boolean) | ||
|
|
||
| return ( | ||
| <div | ||
| data-testid="device-metrics" | ||
| className="flex flex-wrap items-center gap-3 text-xs text-text-secondary" | ||
| > | ||
| <span className="inline-flex items-center gap-1"> | ||
| <span>CPU</span> | ||
| <span>{formatMetricPercent(metrics?.cpu_usage)}</span> | ||
| </span> | ||
| <span className="inline-flex items-center gap-1"> | ||
| <span>MEM</span> | ||
| <span>{formatMetricPercent(metrics?.memory_usage)}</span> | ||
| </span> | ||
| <span className="inline-flex items-center gap-1"> | ||
| <span>磁盘</span> | ||
| <span>{formatMetricPercent(metrics?.disk_usage)}</span> | ||
| </span> | ||
| </div> | ||
| ) | ||
| if (name && !defaultNames.includes(name)) return name | ||
| return device.client_ip?.trim() || name || device.device_id |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reuse the normalized network-host fallback.
Line 331 returns raw client_ip and ignores runtime_transfer_host, so connection cards can show an unnormalized endpoint or generated executor name. Preserve the alias check, then delegate the fallback to getWorkbenchDeviceUnavailableDisplayName.
Proposed fix
+import { getWorkbenchDeviceUnavailableDisplayName } from '`@/lib/workbench-device`'
function deviceDisplayName(device: DeviceInfo): string {
const name = device.name?.trim()
const defaultNames = [
device.device_id,
device.cloud_config?.deviceName,
device.remote_config?.deviceName,
]
.map(value => value?.trim())
.filter(Boolean)
if (name && !defaultNames.includes(name)) return name
- return device.client_ip?.trim() || name || device.device_id
+ return getWorkbenchDeviceUnavailableDisplayName(device) || name || device.device_id
}As per coding guidelines, “Use clear names and reuse existing abstractions instead of duplicating logic.”
📝 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.
Suggested change
| function deviceDisplayName(device: DeviceInfo): string { | |
| const name = device.name?.trim() | |
| const defaultNames = [ | |
| device.device_id, | |
| device.cloud_config?.deviceName, | |
| device.remote_config?.deviceName, | |
| ] | |
| .map(value => value?.trim()) | |
| .filter(Boolean) | |
| return ( | |
| <div | |
| data-testid="device-metrics" | |
| className="flex flex-wrap items-center gap-3 text-xs text-text-secondary" | |
| > | |
| <span className="inline-flex items-center gap-1"> | |
| <span>CPU</span> | |
| <span>{formatMetricPercent(metrics?.cpu_usage)}</span> | |
| </span> | |
| <span className="inline-flex items-center gap-1"> | |
| <span>MEM</span> | |
| <span>{formatMetricPercent(metrics?.memory_usage)}</span> | |
| </span> | |
| <span className="inline-flex items-center gap-1"> | |
| <span>磁盘</span> | |
| <span>{formatMetricPercent(metrics?.disk_usage)}</span> | |
| </span> | |
| </div> | |
| ) | |
| if (name && !defaultNames.includes(name)) return name | |
| return device.client_ip?.trim() || name || device.device_id | |
| import { getWorkbenchDeviceUnavailableDisplayName } from '`@/lib/workbench-device`' | |
| function deviceDisplayName(device: DeviceInfo): string { | |
| const name = device.name?.trim() | |
| const defaultNames = [ | |
| device.device_id, | |
| device.cloud_config?.deviceName, | |
| device.remote_config?.deviceName, | |
| ] | |
| .map(value => value?.trim()) | |
| .filter(Boolean) | |
| if (name && !defaultNames.includes(name)) return name | |
| return getWorkbenchDeviceUnavailableDisplayName(device) || name || device.device_id | |
| } |
🤖 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 `@wework/src/components/settings/ConnectionsSettingsPage.tsx` around lines 320
- 331, Update deviceDisplayName to retain the existing alias check, then replace
the raw client_ip fallback with getWorkbenchDeviceUnavailableDisplayName so
runtime_transfer_host and network-host normalization are reused. Preserve the
existing name and device_id fallback behavior.
Source: Coding guidelines
Comment on lines
+154
to
+166
| if ( | ||
| runtimeProject?.sidebarStateKey && | ||
| runtimeProject.stateDeviceId && | ||
| runtimeProject.stateDeviceId !== runtimeWorkspace.deviceId | ||
| ) { | ||
| await executorClient.runtime.renameRuntimeWorkspace({ | ||
| deviceId: runtimeProject.stateDeviceId, | ||
| projectKey: runtimeProject.sidebarStateKey, | ||
| workspacePath: runtimeWorkspace.workspacePath, | ||
| runtime: 'codex', | ||
| name, | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate all state-owner mutation responses. Both secondary RPC calls discard accepted, allowing partial mutations to be reported as successful.
wework/src/features/workbench/useWorkbenchProjectActions.ts#L154-L166: capture the rename response and handleaccepted: falseidentically to the primary rename.wework/src/features/workbench/useWorkbenchProjectActions.ts#L199-L210: capture the removal response and handleaccepted: falseidentically to the primary removal.
📍 Affects 1 file
wework/src/features/workbench/useWorkbenchProjectActions.ts#L154-L166(this comment)wework/src/features/workbench/useWorkbenchProjectActions.ts#L199-L210
🤖 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 `@wework/src/features/workbench/useWorkbenchProjectActions.ts` around lines 154
- 166, The secondary state-owner mutations in useWorkbenchProjectActions.ts at
lines 154-166 and 199-210 discard RPC responses. Capture each
renameRuntimeWorkspace and removal response, then handle accepted: false exactly
as the corresponding primary rename and removal operations do, preserving
successful behavior while reporting rejected mutations.
| ) => { | ||
| const messageSource = messagesOverride ?? [] | ||
| const runtimeFileChanges = state.currentRuntimeTask | ||
| const runtimeTask = runtimeTaskOverride ?? state.currentRuntimeTask |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Respect explicit null overrides.
The runtimeTaskOverride parameter is typed to allow null. However, the nullish coalescing operator (??) evaluates to the right-hand side for both undefined AND null. If a caller explicitly passes null to indicate "no task," the ?? operator will discard it and erroneously fall back to state.currentRuntimeTask.
To only fall back when the parameter is omitted, check against undefined.
wework/src/features/workbench/useWorkbenchRuntimeMessaging.ts#L1092-L1092: Change toconst runtimeTask = runtimeTaskOverride !== undefined ? runtimeTaskOverride : state.currentRuntimeTaskwework/src/features/workbench/useWorkbenchRuntimeMessaging.ts#L1143-L1143: Change toconst runtimeTask = runtimeTaskOverride !== undefined ? runtimeTaskOverride : state.currentRuntimeTask
📍 Affects 1 file
wework/src/features/workbench/useWorkbenchRuntimeMessaging.ts#L1092-L1092(this comment)wework/src/features/workbench/useWorkbenchRuntimeMessaging.ts#L1143-L1143
🤖 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 `@wework/src/features/workbench/useWorkbenchRuntimeMessaging.ts` at line 1092,
Update both runtimeTask assignments in
wework/src/features/workbench/useWorkbenchRuntimeMessaging.ts at lines 1092-1092
and 1143-1143 to fall back to state.currentRuntimeTask only when
runtimeTaskOverride is undefined, preserving explicit null values.
Comment on lines
+477
to
+493
| useEffect(() => { | ||
| const projects = getRuntimeRemoteProjectRegistrations( | ||
| state.runtimeWork, | ||
| localRuntimeStateDeviceId | ||
| ).sort((left, right) => left.id.localeCompare(right.id)) | ||
| if (!localRuntimeStateDeviceId || projects.length === 0) return | ||
| const signature = JSON.stringify({ deviceId: localRuntimeStateDeviceId, projects }) | ||
| if (remoteProjectSyncSignatureRef.current === signature) return | ||
| remoteProjectSyncSignatureRef.current = signature | ||
| void executorClient.runtime | ||
| .syncRuntimeRemoteProjects({ deviceId: localRuntimeStateDeviceId, projects }) | ||
| .then(refreshWorkLists) | ||
| .catch(error => { | ||
| remoteProjectSyncSignatureRef.current = '' | ||
| console.warn('[Wework] Failed to sync remote projects into Codex global state', error) | ||
| }) | ||
| }, [executorClient, localRuntimeStateDeviceId, refreshWorkLists, state.runtimeWork]) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Synchronize an empty project list to remove stale registrations.
Line 482 returns when the last remote project disappears, so Codex global state never receives projects: [] and retains removed remote projects.
Proposed fix
- if (!localRuntimeStateDeviceId || projects.length === 0) return
+ if (!localRuntimeStateDeviceId) return📝 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.
Suggested change
| useEffect(() => { | |
| const projects = getRuntimeRemoteProjectRegistrations( | |
| state.runtimeWork, | |
| localRuntimeStateDeviceId | |
| ).sort((left, right) => left.id.localeCompare(right.id)) | |
| if (!localRuntimeStateDeviceId || projects.length === 0) return | |
| const signature = JSON.stringify({ deviceId: localRuntimeStateDeviceId, projects }) | |
| if (remoteProjectSyncSignatureRef.current === signature) return | |
| remoteProjectSyncSignatureRef.current = signature | |
| void executorClient.runtime | |
| .syncRuntimeRemoteProjects({ deviceId: localRuntimeStateDeviceId, projects }) | |
| .then(refreshWorkLists) | |
| .catch(error => { | |
| remoteProjectSyncSignatureRef.current = '' | |
| console.warn('[Wework] Failed to sync remote projects into Codex global state', error) | |
| }) | |
| }, [executorClient, localRuntimeStateDeviceId, refreshWorkLists, state.runtimeWork]) | |
| useEffect(() => { | |
| const projects = getRuntimeRemoteProjectRegistrations( | |
| state.runtimeWork, | |
| localRuntimeStateDeviceId | |
| ).sort((left, right) => left.id.localeCompare(right.id)) | |
| if (!localRuntimeStateDeviceId) return | |
| const signature = JSON.stringify({ deviceId: localRuntimeStateDeviceId, projects }) | |
| if (remoteProjectSyncSignatureRef.current === signature) return | |
| remoteProjectSyncSignatureRef.current = signature | |
| void executorClient.runtime | |
| .syncRuntimeRemoteProjects({ deviceId: localRuntimeStateDeviceId, projects }) | |
| .then(refreshWorkLists) | |
| .catch(error => { | |
| remoteProjectSyncSignatureRef.current = '' | |
| console.warn('[Wework] Failed to sync remote projects into Codex global state', error) | |
| }) | |
| }, [executorClient, localRuntimeStateDeviceId, refreshWorkLists, state.runtimeWork]) |
🤖 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 `@wework/src/features/workbench/WorkbenchProvider.tsx` around lines 477 - 493,
Update the remote-project synchronization effect around
getRuntimeRemoteProjectRegistrations so it still calls syncRuntimeRemoteProjects
with projects: [] when localRuntimeStateDeviceId is valid but the project list
is empty. Remove only the projects.length === 0 early-return condition, while
preserving the device-ID guard, signature deduplication, refresh, and error
handling.
Comment on lines
+495
to
+517
| useEffect(() => { | ||
| if (lastProjectRestoreAttemptedRef.current || !state.runtimeWork) return | ||
| if ( | ||
| projectSelectionStartedRef.current || | ||
| parseRuntimeTaskRoute(stripAppBasePath(window.location.pathname), window.location.search) || | ||
| state.currentProject || | ||
| state.currentRuntimeTask || | ||
| state.standaloneWorkspacePath | ||
| ) { | ||
| lastProjectRestoreAttemptedRef.current = true | ||
| return | ||
| } | ||
| const candidateProjectIds = [ | ||
| readLastProjectId(user.id), | ||
| findActiveRuntimeProjectId(state.runtimeWork), | ||
| ] | ||
| lastProjectRestoreAttemptedRef.current = true | ||
| const project = findFirstSelectableProject( | ||
| state.projects, | ||
| state.runtimeWork, | ||
| candidateProjectIds | ||
| ) | ||
| if (project) dispatch({ type: 'project_selected', project }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Wait for bootstrap completion before consuming the restore attempt.
runtimeWork may arrive before state.projects. Lines 511–517 then permanently mark restoration attempted, causing a saved cloud project to be missed or an active runtime fallback to be selected instead.
Proposed fix
- if (lastProjectRestoreAttemptedRef.current || !state.runtimeWork) return
+ if (
+ lastProjectRestoreAttemptedRef.current ||
+ state.isBootstrapping ||
+ !state.runtimeWork
+ ) returnAlso add state.isBootstrapping to the dependency list.
🤖 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 `@wework/src/features/workbench/WorkbenchProvider.tsx` around lines 495 - 517,
Update the project-restore useEffect around findFirstSelectableProject so it
waits while state.isBootstrapping is true before setting
lastProjectRestoreAttemptedRef.current or selecting a fallback project. Add
state.isBootstrapping to the effect dependency list, preserving the existing
route, selection, and workspace guards.
icycrystal4
force-pushed
the
fix/wework-codex-state-persistence
branch
from
July 16, 2026 13:37
098d94d to
2438b20
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
wework/src/api/local/localServices.ts (1)
1512-1514: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not deduplicate projects by display label.
An empty remote project is dropped whenever an unrelated local project has the same label. Match a stable project identity such as the project key or repository fingerprint instead, as unrelated local and remote projects with the same label should both be preserved.
💻 Proposed fix
You will need to update the pre-pass loop (around line 1461) to track project keys instead of labels, and then update this condition to use the key:
- if (rawTasks.length === 0 && workspaceSource === 'remote' && localWorkspaceLabels.has(label)) { + if (rawTasks.length === 0 && workspaceSource === 'remote' && localWorkspaceKeys.has(projectKey)) {🤖 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 `@wework/src/api/local/localServices.ts` around lines 1512 - 1514, Replace label-based project tracking in the pre-pass loop with stable project keys or repository fingerprints, then update the empty-remote check using localWorkspaceLabels to compare that identity instead. Preserve unrelated local and remote projects even when their display labels match.wework/src/api/hybrid/hybridServices.ts (1)
346-348: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winExclude remote chat workspaces from
localDeviceIdstoo.While you correctly filtered out remote sources for project workspaces here, line 350 still registers every chat device as local without checking its source (
work.chats.forEach(workspace => localDeviceIds.add(workspace.deviceId))). A remote chat can therefore misroute commands and runtime calls tolocalServices.💻 Proposed fix
Apply the same filtering to the
work.chatsiteration just below this block:- work.chats.forEach(workspace => localDeviceIds.add(workspace.deviceId)) + work.chats.forEach(workspace => { + if (workspace.workspaceSource !== 'remote') localDeviceIds.add(workspace.deviceId) + })🤖 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 `@wework/src/api/hybrid/hybridServices.ts` around lines 346 - 348, Update the work.chats iteration in the surrounding localDeviceIds population logic to add a chat workspace’s deviceId only when its workspaceSource is not remote, matching the existing project.deviceWorkspaces filter. Preserve local registration for non-remote chat workspaces.wework/src/features/workbench/useWorkbenchRuntimeMessaging.ts (1)
1092-1092:⚠️ Potential issue | 🟡 MinorRespect explicit
nulloverrides.The
runtimeTaskOverrideparameter is typed to allownull. However, the nullish coalescing operator (??) evaluates to the right-hand side for bothundefinedANDnull. If a caller explicitly passesnullto indicate "no task," the??operator will discard it and erroneously fall back tostate.currentRuntimeTask.
wework/src/features/workbench/useWorkbenchRuntimeMessaging.ts#L1092-L1092: Update toconst runtimeTask = runtimeTaskOverride !== undefined ? runtimeTaskOverride : state.currentRuntimeTaskwework/src/features/workbench/useWorkbenchRuntimeMessaging.ts#L1143-L1143: Update toconst runtimeTask = runtimeTaskOverride !== undefined ? runtimeTaskOverride : state.currentRuntimeTask🤖 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 `@wework/src/features/workbench/useWorkbenchRuntimeMessaging.ts` at line 1092, Respect explicit null runtime-task overrides in both runtime task assignments within useWorkbenchRuntimeMessaging: update the sites at wework/src/features/workbench/useWorkbenchRuntimeMessaging.ts lines 1092-1092 and 1143-1143 to fall back to state.currentRuntimeTask only when runtimeTaskOverride is undefined, preserving null as an intentional no-task value.
🤖 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.
Duplicate comments:
In `@wework/src/api/hybrid/hybridServices.ts`:
- Around line 346-348: Update the work.chats iteration in the surrounding
localDeviceIds population logic to add a chat workspace’s deviceId only when its
workspaceSource is not remote, matching the existing project.deviceWorkspaces
filter. Preserve local registration for non-remote chat workspaces.
In `@wework/src/api/local/localServices.ts`:
- Around line 1512-1514: Replace label-based project tracking in the pre-pass
loop with stable project keys or repository fingerprints, then update the
empty-remote check using localWorkspaceLabels to compare that identity instead.
Preserve unrelated local and remote projects even when their display labels
match.
In `@wework/src/features/workbench/useWorkbenchRuntimeMessaging.ts`:
- Line 1092: Respect explicit null runtime-task overrides in both runtime task
assignments within useWorkbenchRuntimeMessaging: update the sites at
wework/src/features/workbench/useWorkbenchRuntimeMessaging.ts lines 1092-1092
and 1143-1143 to fall back to state.currentRuntimeTask only when
runtimeTaskOverride is undefined, preserving null as an intentional no-task
value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9bcd565e-f589-4d93-9a6d-cb67fe5946e6
📒 Files selected for processing (59)
executor/src/runtime_work/codex_global_state.rsexecutor/src/runtime_work/handler.rsexecutor/src/runtime_work/response.rsexecutor/tests/app_runtime_work_workspace_contract.rswework/e2e/desktop/task-flow.e2e.mjswework/scripts/dev-mac-app.shwework/src-tauri/src/local_executor.rswework/src/App.tsxwework/src/api/executorAccess.tswework/src/api/hybrid/hybridServices.test.tswework/src/api/hybrid/hybridServices.tswework/src/api/local/localServices.test.tswework/src/api/local/localServices.tswework/src/api/runtimeWork.tswework/src/components/layout/ConversationDeviceOfflineBanner.tsxwework/src/components/layout/DesktopSidebar.test.tsxwework/src/components/layout/DesktopSidebar.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/layout/DeviceStatusPrompt.test.tsxwework/src/components/layout/DeviceStatusPrompt.tsxwework/src/components/layout/EnvironmentInfoPopover.test.tsxwework/src/components/layout/EnvironmentInfoPopover.tsxwework/src/components/layout/MobileWorkbenchLayout.test.tsxwework/src/components/layout/MobileWorkbenchLayout.tsxwework/src/components/layout/workspace-panels/WorkspacePanelActions.test.tsxwework/src/components/layout/workspace-panels/WorkspacePanelActions.tsxwework/src/components/settings/ConnectionsSettingsPage.test.tsxwework/src/components/settings/ConnectionsSettingsPage.tsxwework/src/components/sites/SitesWorkspace.tsxwework/src/features/cloud-connection/LocalExecutorCloudBridge.test.tsxwework/src/features/cloud-connection/LocalExecutorCloudBridge.tsxwework/src/features/cloud-connection/localExecutorCloudConnection.tswework/src/features/local-runtime/LocalRuntimeInitializer.test.tsxwework/src/features/local-runtime/LocalRuntimeInitializer.tsxwework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/features/workbench/useWorkbenchProjectActions.tswework/src/features/workbench/useWorkbenchRuntimeMessaging.tswework/src/features/workbench/useWorkbenchRuntimeTasks.tswework/src/features/workbench/workbenchCloudStatus.test.tswework/src/features/workbench/workbenchCloudStatus.tswework/src/features/workbench/workbenchContextTypes.tswework/src/features/workbench/workbenchReducer.test.tswework/src/features/workbench/workbenchReducer.tswework/src/features/workbench/workbenchRuntimeHelpers.test.tswework/src/features/workbench/workbenchRuntimeHelpers.tswework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/lib/runtime-project-state.test.tswework/src/lib/runtime-project-state.tswework/src/lib/workbench-device.test.tswework/src/lib/workbench-device.tswework/src/pages/PluginCreatePage.tsxwework/src/pages/PluginManagementPage.tsxwework/src/pages/PluginsPage.tsxwework/src/pages/SitesPage.tsxwework/src/types/api.ts
🚧 Files skipped from review as they are similar to previous changes (44)
- wework/src/features/cloud-connection/localExecutorCloudConnection.ts
- wework/src/api/executorAccess.ts
- wework/src/features/workbench/workbenchReducer.test.ts
- wework/src/api/local/localServices.test.ts
- wework/src/lib/workbench-device.test.ts
- wework/src/components/layout/DesktopSidebar.test.tsx
- wework/src/App.tsx
- wework/src/features/cloud-connection/LocalExecutorCloudBridge.test.tsx
- wework/src/components/layout/EnvironmentInfoPopover.test.tsx
- wework/src/api/runtimeWork.ts
- wework/src/i18n/locales/en/common.json
- wework/src/api/hybrid/hybridServices.test.ts
- wework/src/features/workbench/workbenchRuntimeHelpers.test.ts
- wework/src/lib/runtime-project-state.test.ts
- wework/src/components/layout/MobileWorkbenchLayout.tsx
- wework/src/components/layout/ConversationDeviceOfflineBanner.tsx
- wework/src/features/workbench/useWorkbenchProjectActions.ts
- wework/src/i18n/locales/zh-CN/common.json
- wework/src/components/layout/workspace-panels/WorkspacePanelActions.tsx
- wework/src/types/api.ts
- wework/src/features/cloud-connection/LocalExecutorCloudBridge.tsx
- executor/src/runtime_work/response.rs
- wework/src/lib/workbench-device.ts
- wework/src/features/workbench/workbenchCloudStatus.test.ts
- wework/src/features/local-runtime/LocalRuntimeInitializer.tsx
- wework/src/components/layout/DesktopWorkbenchLayout.test.tsx
- wework/e2e/desktop/task-flow.e2e.mjs
- wework/src/lib/runtime-project-state.ts
- wework/src/features/workbench/workbenchCloudStatus.ts
- wework/scripts/dev-mac-app.sh
- wework/src/components/layout/DesktopWorkbenchMain.tsx
- wework/src-tauri/src/local_executor.rs
- wework/src/features/workbench/workbenchReducer.ts
- wework/src/components/layout/DeviceStatusPrompt.test.tsx
- wework/src/components/layout/DeviceStatusPrompt.tsx
- wework/src/components/layout/EnvironmentInfoPopover.tsx
- wework/src/features/workbench/WorkbenchProvider.tsx
- executor/src/runtime_work/handler.rs
- wework/src/components/layout/DesktopSidebar.tsx
- wework/src/features/workbench/workbenchContextTypes.ts
- wework/src/features/workbench/WorkbenchProvider.test.tsx
- executor/src/runtime_work/codex_global_state.rs
- wework/src/components/settings/ConnectionsSettingsPage.test.tsx
- wework/src/components/settings/ConnectionsSettingsPage.tsx
This was referenced Jul 22, 2026
Merged
This was referenced Aug 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related task
uRcWQd92iW— 复刻Codex会话与项目状态Root cause
What changed
modelSelection) separately from execution parameters, with compatibility for legacy requests, and restore the conversation model/options when reopening it.CODEX_HOME/.codex-global-state.json; local and remote projects now shareproject-order,pinned-project-ids, appearance, and active-project state.sifang-executor-0341b231eb06and raw device IDs are no longer presented as the user-facing name.local_executor_ensure_started, so the Executor starts once with its final Backend URL. Disconnect mode follows the same ordering.Validation
origin/mainat773973edd.pnpm --dir wework build(TypeScript and Vite production build).git diff --check.cargo fmt --check,cargo test --all-features --lib, andcargo clippyThe Wework desktop application was intentionally not started, per request. The desktop E2E scenario is present but was not executed locally; final desktop behavior will be verified manually.
Manual verification
sifang-executor-0341b231eb06displays its IP, while a renamed device displays its alias.dev-mac-app.sh --executor-isolationand--no-executor-isolationselect isolated and shared Executor homes respectively.Summary by CodeRabbit
New Features
Bug Fixes