Repository navigation
feat(wework): fork Codex tasks from messages - #2183
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds completed-turn task forking, propagates turn identifiers through runtime and workbench messages, improves startup session polling, and recalculates navigation markers after dynamic transcript and layout changes. ChangesRuntime turn forking
Startup session polling
Turn navigation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant MessageList
participant DesktopWorkbenchMain
participant localServices
participant RuntimeWorkRpcHandler
participant CodexThread
User->>MessageList: Select Continue in new task
MessageList->>DesktopWorkbenchMain: Provide completed message turnId
DesktopWorkbenchMain->>localServices: Request fork with lastTurnId
localServices->>RuntimeWorkRpcHandler: Call runtime.tasks.fork_at_turn
RuntimeWorkRpcHandler->>CodexThread: Fork thread with excludeTurns
CodexThread-->>RuntimeWorkRpcHandler: Return forked thread
RuntimeWorkRpcHandler-->>localServices: Return target task mapping
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
qdaxb
marked this pull request as ready for review
July 24, 2026 08:01
![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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wework/src/features/workbench/useWorkbenchRuntimeTasks.ts (1)
394-417: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNormalize fork RPC failures into the workbench error state.
This handles
{ accepted: false }, but a transport/device failure that rejectsforkRuntimeTask()escapes the callback. The new hover action fire-and-forgets this promise, so the user receives no error and the app may emit an unhandled rejection. Wrap the RPC intry/catchand dispatch the existing error state for rejected requests.Proposed handling
- const response = await executorClient.runtime.forkRuntimeTask({ - source: state.currentRuntimeTask, - target, - ...options, - }) + let response + try { + response = await executorClient.runtime.forkRuntimeTask({ + source: state.currentRuntimeTask, + target, + ...options, + }) + } catch (error) { + dispatch({ + type: 'error_set', + error: error instanceof Error ? error.message : 'Failed to fork runtime task', + }) + 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 `@wework/src/features/workbench/useWorkbenchRuntimeTasks.ts` around lines 394 - 417, Update forkCurrentRuntimeTask to wrap the await executorClient.runtime.forkRuntimeTask call in try/catch. When the RPC rejects, dispatch error_set with the caught error details or the existing “Failed to fork runtime task” fallback, then return so refreshWorkLists and openRuntimeTask are not executed; preserve the existing response.accepted false handling.
🤖 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/scripts/ai-verify.mjs`:
- Line 327: Update the startup deadline calculation in the polling flow to
validate the configured timeout before adding it to Date.now(). Accept only
finite, positive timeout values; reject invalid, negative, zero, and infinite
values explicitly rather than silently falling back to startupTimeoutMs, while
preserving the normal deadline calculation for valid input.
In `@wework/src/components/chat/MessageList.test.tsx`:
- Around line 4051-4068: Add real-runtime E2E coverage for the completed-turn
flow exercised by MessageList and its fork-message handling: fork a completed
turn through the backend, open the returned task, verify workspace and
conversation context inheritance, and add recovery assertions for unavailable or
missing-turn responses. Keep the existing callback unit test, but ensure the new
E2E uses the actual task/fork APIs and UI flow rather than mocked callbacks.
In `@wework/src/components/chat/MessageList.tsx`:
- Around line 562-566: In MessageList.tsx, update the onFork condition to
require only message.turnId, leaving the action unavailable when it is absent;
in DesktopWorkbenchMain.tsx, pass only message.turnId as lastTurnId and remove
the subtaskId fallback. Keep subtaskId solely as the transcript identity field.
- Around line 1657-1674: Update the forkAction flow around the onFork button to
track whether a fork request is in flight and ignore or disable subsequent
clicks until it completes. Ensure the in-flight state is cleared on both success
and failure, while preserving the existing fork behavior and openRuntimeTask
sequencing.
- Line 111: Add onForkMessage to the changed-props comparison in
areMessageListPropsEqual by checking previous.onForkMessage against
next.onForkMessage, ensuring MessageList re-renders when the fork handler
changes.
In `@wework/src/components/layout/DesktopWorkbenchMain.tsx`:
- Around line 1746-1749: Update the fork request payload near
currentRuntimeTask.deviceId to use the already-resolved runtimeTaskWorkspacePath
instead of falling back to an empty string for workspacePath. Preserve the
existing forkTurnId behavior and ensure the fork inherits the current workspace
directory.
---
Outside diff comments:
In `@wework/src/features/workbench/useWorkbenchRuntimeTasks.ts`:
- Around line 394-417: Update forkCurrentRuntimeTask to wrap the await
executorClient.runtime.forkRuntimeTask call in try/catch. When the RPC rejects,
dispatch error_set with the caught error details or the existing “Failed to fork
runtime task” fallback, then return so refreshWorkLists and openRuntimeTask are
not executed; preserve the existing response.accepted false handling.
🪄 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 Plus
Run ID: 7edbc66a-d18f-4db9-8be6-f9fd765a5116
📒 Files selected for processing (18)
docs/en/wework/tasks.mddocs/zh/wework/tasks.mdexecutor/src/runtime_work/handler.rsexecutor/src/runtime_work/handler/tasks.rsexecutor/src/runtime_work/transcript.rswework/scripts/ai-verify.mjswework/src/api/local/localServices.tswework/src/components/chat/MessageList.test.tsxwework/src/components/chat/MessageList.tsxwework/src/components/chat/ScrollableMessageArea.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/features/workbench/runtimePaneMessages.tswework/src/features/workbench/useWorkbenchRuntimeTasks.tswework/src/features/workbench/workbenchContextTypes.tswework/src/i18n/locales/en/chat.jsonwework/src/i18n/locales/zh-CN/chat.jsonwework/src/types/api.tswework/src/types/workbench.ts
Comment on lines
+4051
to
+4068
| test('continues a completed Codex turn in a new task', async () => { | ||
| const onForkMessage = vi.fn() | ||
| const message = { | ||
| id: 'assistant-turn-1', | ||
| role: 'assistant' as const, | ||
| content: '已完成当前修改。', | ||
| status: 'done' as const, | ||
| turnId: 'turn-1', | ||
| createdAt: '2026-05-25T18:38:00.000+08:00', | ||
| } | ||
|
|
||
| render(<MessageList messages={[message]} onForkMessage={onForkMessage} />) | ||
|
|
||
| const button = screen.getByTestId('fork-message-button') | ||
| expect(button).toHaveAttribute('aria-label', '在新任务中继续') | ||
| await userEvent.click(button) | ||
| expect(onForkMessage).toHaveBeenCalledWith(message) | ||
| }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Add real-runtime E2E coverage for task forking.
This only verifies the callback. Add a real-backend E2E that forks a completed turn, opens the returned task, verifies workspace/context inheritance, and covers unavailable or missing-turn recovery.
As per coding guidelines, “Changes affecting core flows must add or update automated E2E regression coverage in the same change,” including failure recovery when affected.
🤖 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/chat/MessageList.test.tsx` around lines 4051 - 4068,
Add real-runtime E2E coverage for the completed-turn flow exercised by
MessageList and its fork-message handling: fork a completed turn through the
backend, open the returned task, verify workspace and conversation context
inheritance, and add recovery assertions for unavailable or missing-turn
responses. Keep the existing callback unit test, but ensure the new E2E uses the
actual task/fork APIs and UI flow rather than mocked callbacks.
Source: Coding guidelines
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
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/tasks.rs (1)
9-14: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftMake the running-state check atomic and reject either active signal.
mark_task_running_for_sendsetssource.running = truebeforespawn_turncallsmark_active_local_task, so this&&check has a real window where a source turn is already starting but the fork proceeds. Use one authoritative synchronized state, or at minimum reject when either indicator is active and reserve the source before callingthread/fork.Proposed minimum guard
- if source.running && self.is_active_local_task(&source.local_task_id) { + if source.running || self.is_active_local_task(&source.local_task_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/tasks.rs` around lines 9 - 14, Update the guard in the task handler around task_link_from_payload so it rejects the fork when either source.running or is_active_local_task(&source.local_task_id) is true, rather than requiring both. Ensure the source is reserved atomically or otherwise remains protected before invoking thread/fork to prevent a concurrent turn from starting.
🤖 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/handler/tasks.rs`:
- Around line 21-22: Update the handler’s last-turn resolution around
runtime_turn_id_from_link so an unresolved lastTurnId is not passed through as a
native Codex turn ID. Distinguish native turn IDs from Wework subtask IDs using
the existing mapping/validation behavior, and return bad_request when the value
cannot be resolved; only call thread/fork with a validated native turn ID.
---
Outside diff comments:
In `@executor/src/runtime_work/handler/tasks.rs`:
- Around line 9-14: Update the guard in the task handler around
task_link_from_payload so it rejects the fork when either source.running or
is_active_local_task(&source.local_task_id) is true, rather than requiring both.
Ensure the source is reserved atomically or otherwise remains protected before
invoking thread/fork to prevent a concurrent turn from starting.
🪄 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 Plus
Run ID: 2b4349db-1a0d-4caa-b656-b9436d0e401c
📒 Files selected for processing (4)
executor/src/runtime_work/handler/tasks.rsexecutor/src/runtime_work/handler/tests.rsexecutor/src/runtime_work/handler/turns.rswework/src/features/workbench/useWorkbenchRuntimeTasks.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- wework/src/features/workbench/useWorkbenchRuntimeTasks.ts
qdaxb
force-pushed
the
feature/codex-message-fork
branch
3 times, most recently
from
July 24, 2026 09:16
d87745c to
0d2a0a6
Compare
qdaxb
force-pushed
the
feature/codex-message-fork
branch
from
July 24, 2026 15:12
0d2a0a6 to
2bb321b
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wework/e2e/desktop/task-flow.e2e.mjs (1)
1171-1209: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAvoid closing the model selector while retrying to reveal the option.
model-selector-buttoncallsonToggleon click, and existing tests show clicking it after the menu is already open removesmodel-selector-menu. In the retry loop, the fallback click can therefore close an opened menu before the targeted model option has a chance to appear, halving effective retries. Gate this branch so it only runs when the selector menu is absent.🤖 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/e2e/desktop/task-flow.e2e.mjs` around lines 1171 - 1209, The fallback branch in ensureModelOptionVisible should click model-selector-button only when the selector menu is absent. Check for model-selector-menu before invoking clickWhenEnabled, while preserving the existing hover behavior and retry flow when the menu is already open.
🧹 Nitpick comments (2)
executor/src/runtime_work/handler/turns.rs (1)
281-283: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regression assertions for the emitted
turnId.The supplied tests cover persistence and fork lookup, but do not assert
response.completed.turnIdfor eitherCompletedorWaitingForUserInput. Add focused assertions so lookup or JSON-payload regressions cannot pass unnoticed.Also applies to: 291-291, 301-301
🤖 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/turns.rs` around lines 281 - 283, Extend the tests covering the Completed and WaitingForUserInput response paths around the local_task_link/runtime_turn_id_from_link flow to assert the emitted response.completed.turnId. Verify each assertion against the expected turn ID, including the JSON payload path, while preserving the existing persistence and fork-lookup checks.wework/e2e/desktop/task-flow.e2e.mjs (1)
4629-4629: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the local-project naming dialog flow into a shared helper.
The
waitFor 'local-project-create-dialog'→fill 'local-project-create-name-input'→clickWhenEnabled 'confirm-local-project-create-button'sequence is repeated identically in three dialog flows. Add a small helper such asconfirmLocalProjectName(control, name)so dialog selector/timing changes are centralized and the E2E coverage stays maintainable.The referenced testids match the Wework component markup.
🤖 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/e2e/desktop/task-flow.e2e.mjs` at line 4629, Extract the repeated local-project naming dialog sequence into a shared helper such as confirmLocalProjectName(control, name), using the existing waitFor, fill, and clickWhenEnabled operations with their current selectors. Replace all three identical dialog flows, including the flow around project-create-local-option, with calls to this helper while preserving their existing names and timing 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 `@wework/src/components/chat/MessageTurnNavigation.tsx`:
- Line 1: Stabilize marker-calculation dependencies in MessageTurnNavigation by
deriving a stable turn/change signature instead of using the fresh array
returned by buildUserTurnsForNavigation. Update calculateMarkers and
scheduleCalculateMarkers to depend on and consume that signature, ensuring
setMarkers does not trigger repeated rescheduling when the underlying turns have
not changed.
---
Outside diff comments:
In `@wework/e2e/desktop/task-flow.e2e.mjs`:
- Around line 1171-1209: The fallback branch in ensureModelOptionVisible should
click model-selector-button only when the selector menu is absent. Check for
model-selector-menu before invoking clickWhenEnabled, while preserving the
existing hover behavior and retry flow when the menu is already open.
---
Nitpick comments:
In `@executor/src/runtime_work/handler/turns.rs`:
- Around line 281-283: Extend the tests covering the Completed and
WaitingForUserInput response paths around the
local_task_link/runtime_turn_id_from_link flow to assert the emitted
response.completed.turnId. Verify each assertion against the expected turn ID,
including the JSON payload path, while preserving the existing persistence and
fork-lookup checks.
In `@wework/e2e/desktop/task-flow.e2e.mjs`:
- Line 4629: Extract the repeated local-project naming dialog sequence into a
shared helper such as confirmLocalProjectName(control, name), using the existing
waitFor, fill, and clickWhenEnabled operations with their current selectors.
Replace all three identical dialog flows, including the flow around
project-create-local-option, with calls to this helper while preserving their
existing names and timing 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 Plus
Run ID: 13e925d2-4ee9-4f80-960f-4e8d28cf18aa
📒 Files selected for processing (28)
docs/en/wework/tasks.mddocs/zh/wework/tasks.mdexecutor/src/runtime_work/handler.rsexecutor/src/runtime_work/handler/tasks.rsexecutor/src/runtime_work/handler/tests.rsexecutor/src/runtime_work/handler/turns.rsexecutor/src/runtime_work/transcript.rsexecutor/tests/app_runtime_work_send_contract.rspackages/chat-core/src/workbench-message-reducer.test.tspackages/chat-core/src/workbench-message-reducer.tswework/e2e/desktop/task-flow.e2e.mjswework/scripts/ai-verify.mjswework/scripts/ai-verify.test.mjswework/src/api/local/localServices.tswework/src/components/chat/MessageList.test.tsxwework/src/components/chat/MessageList.tsxwework/src/components/chat/MessageTurnNavigation.tsxwework/src/components/chat/ScrollableMessageArea.test.tsxwework/src/components/chat/ScrollableMessageArea.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/features/workbench/runtimePaneMessages.test.tswework/src/features/workbench/runtimePaneMessages.tswework/src/features/workbench/useWorkbenchRuntimeTasks.tswework/src/features/workbench/workbenchContextTypes.tswework/src/i18n/locales/en/chat.jsonwework/src/i18n/locales/zh-CN/chat.jsonwework/src/types/api.tswework/src/types/workbench.ts
🚧 Files skipped from review as they are similar to previous changes (23)
- executor/src/runtime_work/handler.rs
- wework/src/i18n/locales/en/chat.json
- wework/src/features/workbench/workbenchContextTypes.ts
- wework/src/components/chat/MessageList.test.tsx
- wework/src/types/api.ts
- executor/src/runtime_work/transcript.rs
- docs/zh/wework/tasks.md
- wework/src/types/workbench.ts
- wework/src/components/chat/ScrollableMessageArea.tsx
- packages/chat-core/src/workbench-message-reducer.test.ts
- wework/src/features/workbench/useWorkbenchRuntimeTasks.ts
- docs/en/wework/tasks.md
- wework/scripts/ai-verify.mjs
- wework/src/features/workbench/runtimePaneMessages.ts
- wework/src/components/layout/DesktopWorkbenchMain.tsx
- wework/scripts/ai-verify.test.mjs
- executor/tests/app_runtime_work_send_contract.rs
- packages/chat-core/src/workbench-message-reducer.ts
- wework/src/components/chat/MessageList.tsx
- executor/src/runtime_work/handler/tasks.rs
- wework/src/features/workbench/runtimePaneMessages.test.ts
- wework/src/api/local/localServices.ts
- executor/src/runtime_work/handler/tests.rs
…-fork # Conflicts: # wework/e2e/desktop/task-flow.e2e.mjs
This was referenced Jul 25, 2026
This was referenced Aug 9, 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/forkwithlastTurnIdWhy
Users need to explore a different direction from an earlier response without rolling back or changing the original task. Codex already provides precise turn-bounded thread forking, but Wework did not expose it as a visible task workflow.
Validation
cargo fmt --check,cargo test --all-features --lib, andcargo clippySummary by CodeRabbit