Repository navigation
fix(runtime): use Codex thread as source of truth - #1968
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughCodex runtime handling now retrieves transcripts and thread lists directly, derives task status from Codex thread state, and removes notification/transcript caches. Workbench panes buffer stream actions during transcript rebuilds, use message seeds, and gate queued messages on stream settlement. Image prompts preserve original attachment paths. ChangesCodex runtime state and data flow
Workbench transcript and runtime synchronization
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
qdaxb
marked this pull request as ready for review
July 15, 2026 04:32
![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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
executor/src/runtime_work/handler.rs (1)
1108-1213: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPropagate the local running state into the Codex transcript response
Thethread/readbranch should also OR inlocal_execution_running; otherwise a just-spawned turn can briefly returnrunning: falseuntil Codex flips the thread status.🤖 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/handler.rs` around lines 1108 - 1213, Update the `thread/read` branch in the transcript handler so the `running` value combines `codex_thread_has_active_turn(&thread)` with `local_execution_running` using OR semantics. Preserve the existing Codex status check while ensuring a just-spawned local task returns `running: true` until both sources report inactive.
🧹 Nitpick comments (2)
executor/src/runtime_work/handler.rs (2)
4445-4447: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
codex_thread_stateis now a no-op clone; avoid the extra allocation at call sites.Now that this is just
thread.clone(), both call sites (line 1182 and line 3577) only use the result by reference afterward (transcript_context_usage(&transcript_thread),transcript_messages(&transcript_thread, ...)). WithincludeTurns: true,threadcan carry the full turn history, so cloning it just to immediately borrow it back is wasted work.♻️ Suggested fix
- let transcript_thread = codex_thread_state(&thread); - let context_usage = transcript_context_usage(&transcript_thread); + let context_usage = transcript_context_usage(&thread); let transcript_messages = if include_full_content { - full_transcript_messages(&transcript_thread, &self.device_id) + full_transcript_messages(&thread, &self.device_id) } else { - transcript_messages(&transcript_thread, &self.device_id) + transcript_messages(&thread, &self.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 `@executor/src/runtime_work/handler.rs` around lines 4445 - 4447, Remove the redundant codex_thread_state wrapper and update both call sites to pass thread by reference directly to transcript_context_usage and transcript_messages. Preserve the existing includeTurns behavior and downstream borrowing semantics without cloning the full thread value.
1168-1196: 🚀 Performance & Scalability | 🔵 TrivialRemoving all thread-list/transcript caching means every call now pays full Codex round-trip cost.
transcript()always callsthread/readwithincludeTurns:true(fetching the entire turn history regardless oflimit/before_cursor/after_cursor, which now only slice client-side), andcodex_threads()— invoked fromcollect_links(), which backslist_tasks,search_tasks,archive_project_conversations,workspace_search_root_is_allowed, etc. — always paginatesthread/listup toCODEX_THREAD_LIST_MAX_ITEMS(500) with no cache. This is clearly intentional per the PR's goal of removing stale caches, but it's worth keeping an eye on IPC/latency if these RPCs are polled frequently by the Wework UI, since there's no longer any short-lived cache to absorb bursts ofruntime.tasks.list/runtime.tasks.transcriptcalls.Also applies to: 3185-3247
🤖 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/handler.rs` around lines 1168 - 1196, No code change is required: preserve the intentional cache removal in transcript() and codex_threads(), including full thread/read retrieval and thread/list pagination. Treat the note as a performance consideration only and avoid reintroducing caching or changing client-side slicing 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.
Inline comments:
In `@executor/src/runtime_work/response.rs`:
- Around line 683-700: The status normalization in
executor/src/runtime_work/response.rs lines 683-700 should remain the single
shared implementation via codex_thread_status_type and codex_thread_is_active.
Update executor/src/runtime_work/handler.rs lines 4431-4443 so
codex_thread_has_active_turn calls that shared helper instead of duplicating
status extraction and normalization; no direct change is needed in response.rs
unless required to expose the helper.
In `@wework/src/components/layout/useWorkbenchPaneSession.ts`:
- Around line 1149-1152: Update the early-return condition in the queued-message
handling flow to use OR logic, returning when either
paneStatus.canSendQueuedMessage is false or streamSettled is false. Preserve the
subsequent sending-status and queued-message checks.
---
Outside diff comments:
In `@executor/src/runtime_work/handler.rs`:
- Around line 1108-1213: Update the `thread/read` branch in the transcript
handler so the `running` value combines `codex_thread_has_active_turn(&thread)`
with `local_execution_running` using OR semantics. Preserve the existing Codex
status check while ensuring a just-spawned local task returns `running: true`
until both sources report inactive.
---
Nitpick comments:
In `@executor/src/runtime_work/handler.rs`:
- Around line 4445-4447: Remove the redundant codex_thread_state wrapper and
update both call sites to pass thread by reference directly to
transcript_context_usage and transcript_messages. Preserve the existing
includeTurns behavior and downstream borrowing semantics without cloning the
full thread value.
- Around line 1168-1196: No code change is required: preserve the intentional
cache removal in transcript() and codex_threads(), including full thread/read
retrieval and thread/list pagination. Treat the note as a performance
consideration only and avoid reintroducing caching or changing client-side
slicing 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: 7b5696be-2d1d-4ef6-abeb-20146815ac24
📒 Files selected for processing (13)
executor/src/agents/codex.rsexecutor/src/runtime_work/codex_rollout.rsexecutor/src/runtime_work/handler.rsexecutor/src/runtime_work/mod.rsexecutor/src/runtime_work/response.rsexecutor/src/runtime_work/runtime_handle_messages.rsexecutor/src/runtime_work/store.rsexecutor/src/runtime_work/transcript_cache.rsexecutor/tests/app_runtime_work_native_update_contract.rsexecutor/tests/codex_attachment_input_contract.rswework/src/components/layout/useWorkbenchPaneSession.tswework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsx
💤 Files with no reviewable changes (4)
- executor/src/runtime_work/mod.rs
- wework/src/features/workbench/WorkbenchProvider.tsx
- executor/src/runtime_work/transcript_cache.rs
- executor/src/runtime_work/store.rs
qdaxb
force-pushed
the
fix/codex-thread-source-of-truth
branch
16 times, most recently
from
July 15, 2026 06:15
880c2fb to
f6f0801
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@wework/src/features/workbench/WorkbenchProvider.tsx`:
- Around line 173-186: Update the currentRuntimeTaskRunning calculation around
authoritativeRuntimeTaskRunning and activeRuntimeTasks so stale transient
activity cannot override an authoritative running: false result. Only consult
activeRuntimeTasks when getRuntimePaneTaskExecution returns an unknown or
unavailable authoritative execution state, while preserving authoritative
running: true and false values when present.
In `@wework/src/features/workbench/workbenchReducer.ts`:
- Around line 590-603: Unify active runtime-task identity by replacing the
wildcard behavior in sameRuntimeTaskActivity with the exact route-key semantics
used by WorkbenchProvider.tsx. Reuse getRuntimeTaskRouteKey or extract a shared
comparator, then apply it consistently in upsertActiveRuntimeTask and the
corresponding settle/removal logic around the other sameRuntimeTaskActivity
usage.
🪄 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: b874b5da-c2de-44e2-b318-d89fc278d367
📒 Files selected for processing (17)
executor/src/agents/codex.rsexecutor/src/runtime_work/codex_rollout.rsexecutor/src/runtime_work/handler.rsexecutor/src/runtime_work/mod.rsexecutor/src/runtime_work/response.rsexecutor/src/runtime_work/runtime_handle_messages.rsexecutor/src/runtime_work/store.rsexecutor/src/runtime_work/transcript_cache.rsexecutor/tests/app_runtime_work_native_update_contract.rsexecutor/tests/codex_attachment_input_contract.rsscripts/hooks/ai-push-gate.shwework/src/components/layout/useWorkbenchPaneSession.tswework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/features/workbench/useWorkbenchRuntimeMessaging.tswework/src/features/workbench/workbenchReducer.tswework/src/types/workbench.ts
💤 Files with no reviewable changes (3)
- executor/src/runtime_work/mod.rs
- executor/src/runtime_work/store.rs
- executor/src/runtime_work/transcript_cache.rs
🚧 Files skipped from review as they are similar to previous changes (9)
- executor/tests/codex_attachment_input_contract.rs
- executor/tests/app_runtime_work_native_update_contract.rs
- executor/src/runtime_work/response.rs
- executor/src/agents/codex.rs
- wework/src/features/workbench/WorkbenchProvider.test.tsx
- wework/src/components/layout/useWorkbenchPaneSession.ts
- executor/src/runtime_work/codex_rollout.rs
- executor/src/runtime_work/runtime_handle_messages.rs
- executor/src/runtime_work/handler.rs
Comment on lines
+173
to
+186
| const authoritativeRuntimeTaskRunning = useMemo( | ||
| () => getRuntimePaneTaskExecution(state.runtimeWork, state.currentRuntimeTask).running, | ||
| [state.currentRuntimeTask, state.runtimeWork] | ||
| ) | ||
| const currentRuntimeTaskRunning = useMemo( | ||
| () => | ||
| authoritativeRuntimeTaskRunning || | ||
| (state.currentRuntimeTask !== null && | ||
| state.activeRuntimeTasks.some( | ||
| address => | ||
| getRuntimeTaskRouteKey(address) === getRuntimeTaskRouteKey(state.currentRuntimeTask!) | ||
| )), | ||
| [authoritativeRuntimeTaskRunning, state.activeRuntimeTasks, state.currentRuntimeTask] | ||
| ) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not let transient activity override a known authoritative idle state.
If a settlement event is missed, activeRuntimeTasks remains stale and this unconditional OR keeps the pane disabled even when Codex explicitly reports running: false. Use activity only while authoritative execution is unknown.
Possible fix
- const authoritativeRuntimeTaskRunning = useMemo(
- () => getRuntimePaneTaskExecution(state.runtimeWork, state.currentRuntimeTask).running,
+ const authoritativeRuntimeTaskExecution = useMemo(
+ () => getRuntimePaneTaskExecution(state.runtimeWork, state.currentRuntimeTask),
[state.currentRuntimeTask, state.runtimeWork]
)
+ const authoritativeRuntimeTaskRunning = authoritativeRuntimeTaskExecution.running
const currentRuntimeTaskRunning = useMemo(
() =>
- authoritativeRuntimeTaskRunning ||
- (state.currentRuntimeTask !== null &&
+ authoritativeRuntimeTaskExecution.known
+ ? authoritativeRuntimeTaskExecution.running
+ : state.currentRuntimeTask !== null &&
state.activeRuntimeTasks.some(
address =>
getRuntimeTaskRouteKey(address) === getRuntimeTaskRouteKey(state.currentRuntimeTask!)
- )),
- [authoritativeRuntimeTaskRunning, state.activeRuntimeTasks, state.currentRuntimeTask]
+ ),
+ [authoritativeRuntimeTaskExecution, state.activeRuntimeTasks, state.currentRuntimeTask]
)📝 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
| const authoritativeRuntimeTaskRunning = useMemo( | |
| () => getRuntimePaneTaskExecution(state.runtimeWork, state.currentRuntimeTask).running, | |
| [state.currentRuntimeTask, state.runtimeWork] | |
| ) | |
| const currentRuntimeTaskRunning = useMemo( | |
| () => | |
| authoritativeRuntimeTaskRunning || | |
| (state.currentRuntimeTask !== null && | |
| state.activeRuntimeTasks.some( | |
| address => | |
| getRuntimeTaskRouteKey(address) === getRuntimeTaskRouteKey(state.currentRuntimeTask!) | |
| )), | |
| [authoritativeRuntimeTaskRunning, state.activeRuntimeTasks, state.currentRuntimeTask] | |
| ) | |
| const authoritativeRuntimeTaskExecution = useMemo( | |
| () => getRuntimePaneTaskExecution(state.runtimeWork, state.currentRuntimeTask), | |
| [state.currentRuntimeTask, state.runtimeWork] | |
| ) | |
| const authoritativeRuntimeTaskRunning = authoritativeRuntimeTaskExecution.running | |
| const currentRuntimeTaskRunning = useMemo( | |
| () => | |
| authoritativeRuntimeTaskExecution.known | |
| ? authoritativeRuntimeTaskExecution.running | |
| : state.currentRuntimeTask !== null && | |
| state.activeRuntimeTasks.some( | |
| address => | |
| getRuntimeTaskRouteKey(address) === getRuntimeTaskRouteKey(state.currentRuntimeTask!) | |
| ), | |
| [authoritativeRuntimeTaskExecution, state.activeRuntimeTasks, 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/WorkbenchProvider.tsx` around lines 173 - 186,
Update the currentRuntimeTaskRunning calculation around
authoritativeRuntimeTaskRunning and activeRuntimeTasks so stale transient
activity cannot override an authoritative running: false result. Only consult
activeRuntimeTasks when getRuntimePaneTaskExecution returns an unknown or
unavailable authoritative execution state, while preserving authoritative
running: true and false values when present.
Comment on lines
+590
to
+603
| function sameRuntimeTaskActivity(left: RuntimeTaskAddress, right: RuntimeTaskAddress): boolean { | ||
| if (left.deviceId !== right.deviceId || left.taskId !== right.taskId) return false | ||
| if (!left.workspacePath || !right.workspacePath) return true | ||
| return left.workspacePath === right.workspacePath | ||
| } | ||
|
|
||
| function startRuntimeTask( | ||
| current: RuntimeWorkListResponse | null | undefined, | ||
| function upsertActiveRuntimeTask( | ||
| current: RuntimeTaskAddress[], | ||
| address: RuntimeTaskAddress | ||
| ): RuntimeWorkListResponse | null { | ||
| if (!current) return null | ||
|
|
||
| const startWorkspace = (workspace: RuntimeDeviceWorkspace): RuntimeDeviceWorkspace => { | ||
| if (workspace.deviceId !== address.deviceId) return workspace | ||
| return { | ||
| ...workspace, | ||
| tasks: workspace.tasks.map(task => { | ||
| if (task.taskId !== address.taskId) return task | ||
| if ( | ||
| address.workspacePath && | ||
| getRuntimeTaskWorkspacePath(workspace, task) !== address.workspacePath | ||
| ) { | ||
| return task | ||
| } | ||
| return { | ||
| ...task, | ||
| running: true, | ||
| updatedAt: new Date().toISOString(), | ||
| } | ||
| }), | ||
| } | ||
| } | ||
|
|
||
| const projects = current.projects.map(project => { | ||
| const deviceWorkspaces = project.deviceWorkspaces.map(startWorkspace) | ||
| return { | ||
| ...project, | ||
| deviceWorkspaces, | ||
| totalTasks: countRuntimeTasks(deviceWorkspaces), | ||
| } | ||
| }) | ||
| const chats = current.chats.map(startWorkspace) | ||
| const nextRuntimeWork = { | ||
| ...current, | ||
| projects, | ||
| chats, | ||
| } | ||
| return { | ||
| ...nextRuntimeWork, | ||
| totalTasks: countRuntimeWorkTasks(nextRuntimeWork), | ||
| } | ||
| ): RuntimeTaskAddress[] { | ||
| return [ | ||
| ...current.filter(activeAddress => !sameRuntimeTaskActivity(activeAddress, address)), | ||
| address, | ||
| ] |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use one consistent identity rule for active runtime tasks.
Missing workspacePath is a wildcard here, but WorkbenchProvider.tsx compares exact route keys. A pathless start can therefore fail to mark the resolved pane active, while a pathless settle can remove activity for another workspace.
Reuse getRuntimeTaskRouteKey, or define a shared comparator with identical semantics at both sites.
Possible fix
function sameRuntimeTaskActivity(left: RuntimeTaskAddress, right: RuntimeTaskAddress): boolean {
- if (left.deviceId !== right.deviceId || left.taskId !== right.taskId) return false
- if (!left.workspacePath || !right.workspacePath) return true
- return left.workspacePath === right.workspacePath
+ return getRuntimeTaskRouteKey(left) === getRuntimeTaskRouteKey(right)
}Also applies to: 1119-1130
🤖 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/workbenchReducer.ts` around lines 590 - 603,
Unify active runtime-task identity by replacing the wildcard behavior in
sameRuntimeTaskActivity with the exact route-key semantics used by
WorkbenchProvider.tsx. Reuse getRuntimeTaskRouteKey or extract a shared
comparator, then apply it consistently in upsertActiveRuntimeTask and the
corresponding settle/removal logic around the other sameRuntimeTaskActivity
usage.
This was referenced Jul 15, 2026
This was referenced Jul 22, 2026
This was referenced Aug 7, 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.
What changed
thread/readwith turns includedRoot cause
Executor and Wework maintained parallel copies of assistant output and running state. When an event was missed or a local inferred state diverged from Codex, the pane could remain stuck without surfacing the completed response or an error.
Impact
Codex thread state is now authoritative. Reopening or rebuilding a pane reads the current Codex thread and releases events received during that read, eliminating stale local assistant/process state and fallback reconstruction paths.
Validation
cargo fmt --checkcargo test --all-features --libcargo clippySummary by CodeRabbit
status/runningderivation from Codex thread activity, including archived/failed cases.