Repository navigation
fix(wework): keep background interactions live - #2042
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 migrates chat streaming to runtime APIs, adds asynchronous Codex user-input handling and desktop E2E coverage, retains panes for all runtime tasks, and updates desktop automation visibility checks and E2E environment setup. ChangesRuntime chat stream migration
Request-user-input workflow
Runtime workbench pane retention
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant DesktopE2ETest
participant DesktopE2EServer
participant RuntimeChatStream
participant CodexExecutor
participant DesktopUI
DesktopE2ETest->>DesktopE2EServer: setScenario(request_user_input)
DesktopUI->>RuntimeChatStream: send prompt
RuntimeChatStream->>CodexExecutor: forward runtime request
CodexExecutor->>DesktopE2EServer: emit request_user_input response
DesktopE2EServer->>RuntimeChatStream: emit tool event
RuntimeChatStream->>DesktopUI: forward scoped event
DesktopE2ETest->>DesktopE2EServer: releaseRequestUserInputResponse()
DesktopUI->>DesktopE2ETest: render question and completion
Possibly related PRs
🚥 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 18, 2026 17:36
![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
🤖 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/e2e/desktop/task-flow.e2e.mjs`:
- Around line 521-523: Update releaseRequestUserInputResponse() and the
associated task-row reopening flow to await a server acknowledgement that
resolves only after handleModelResponse() completes writeSse(), ensuring the
click exercises replay rather than live delivery. Add or update E2E regression
coverage for this ordering in the same flow.
In `@wework/src/api/local/localChatStream.ts`:
- Around line 194-201: Update the pending-block cache around the event handling
logic to store current block state keyed by taskKey and block ID, rather than
appending raw events to a 500-entry list. Merge created/updated events for each
block, remove blocks once resolved, and enforce
MAX_PENDING_BLOCK_EVENTS_PER_TASK against the number of retained blocks so late
subscribers still receive unresolved request_user_input interactions.
🪄 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: 23aa66e9-353a-4957-9cf5-47a6bb188f2d
📒 Files selected for processing (6)
.github/workflows/wework-e2e.ymlwework/e2e/desktop/task-flow.e2e.mjswework/src/api/hybrid/hybridServices.tswework/src/api/local/localChatStream.test.tswework/src/api/local/localChatStream.tswework/src/e2e/automation.ts
qdaxb
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
wework/src/components/layout/DesktopWorkbenchMain.tsx (1)
277-280: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify
pinnedPaneKeysarray.Because
validTerminalPinnedPaneKeysis filtered to only include keys that are already present invalidRuntimePaneKeySet, the union ofruntimePaneKeysandvalidTerminalPinnedPaneKeysis functionally identical toruntimePaneKeys.You can simplify this memoized value to just return
runtimePaneKeys(or removepinnedPaneKeysentirely and passruntimePaneKeysdirectly).♻️ Proposed refactor
const pinnedPaneKeys = useMemo( - () => Array.from(new Set([...runtimePaneKeys, ...validTerminalPinnedPaneKeys])), - [runtimePaneKeys, validTerminalPinnedPaneKeys] + () => runtimePaneKeys, + [runtimePaneKeys] )🤖 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/layout/DesktopWorkbenchMain.tsx` around lines 277 - 280, Update the pinnedPaneKeys useMemo in DesktopWorkbenchMain to return runtimePaneKeys directly, or remove the memo and pass runtimePaneKeys to its consumers; eliminate the redundant Set-based union while preserving the existing pinned-pane 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/agents/codex.rs`:
- Around line 1305-1309: Update the shared request-user-input response handling
around request_user_input_answers and the logic at the referenced
response-processing section to dispatch each response by its requestId/itemId
correlation key rather than mutex or receiver acquisition order. Maintain
per-request routing so out-of-order answers reach the matching pending handler,
and add a regression test covering overlapping requests answered in reverse
order.
In `@executor/tests/app_runtime_work_send_contract.rs`:
- Line 1155: Replace the fixed 500 ms timing sleep in the notification-flood
test with an observable handshake: emit a final sentinel notification after the
flood loop, await receipt of that sentinel, then call runtime.tasks.send. Apply
the same synchronization change to the corresponding flow around the other
reported location, removing reliance on timing delays while preserving the
existing notification assertions.
In `@wework/e2e/desktop/task-flow.e2e.mjs`:
- Around line 522-524: Update releaseRequestUserInputResponse() to await or wrap
requestUserInputResponseWritten with the existing withTimeout helper, using the
established timeout and diagnostic failure path so handler failures cannot hang
indefinitely; apply the same change to the corresponding occurrence noted in the
comment.
In `@wework/src/api/runtime/runtimeChatStream.ts`:
- Around line 46-58: The runtime stream currently drops relevant
response.block.* events received before a pane subscription exists. In
wework/src/api/runtime/runtimeChatStream.ts:46-58, add a per-task pending
tool-event cache, store events with no matching subscription, replay cached
events when that task subscribes, and clear the cache on terminal responses. In
wework/src/api/runtime/runtimeChatStream.test.ts:203-210, add a regression test
that emits a request_user_input block before subscription and verifies the late
subscriber receives it.
In `@wework/src/lib/developerCommandMenu.ts`:
- Around line 269-279: Update the toggle handler around
isRuntimeChatStreamDebugEnabled and setRuntimeChatStreamDebugEnabled to capture
the frontend debug flag before changing it, then restore that saved value in the
catch block when requestLocalExecutor fails. Keep the executor success path and
logging behavior unchanged.
---
Nitpick comments:
In `@wework/src/components/layout/DesktopWorkbenchMain.tsx`:
- Around line 277-280: Update the pinnedPaneKeys useMemo in DesktopWorkbenchMain
to return runtimePaneKeys directly, or remove the memo and pass runtimePaneKeys
to its consumers; eliminate the redundant Set-based union while preserving the
existing pinned-pane 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: 89a78bc1-1801-4968-a844-59104ff98204
📒 Files selected for processing (14)
executor/src/agents/codex.rsexecutor/tests/app_runtime_work_send_contract.rswework/e2e/desktop/task-flow.e2e.mjswework/scripts/ai-verify-environment.mjswework/src/api/hybrid/hybridServices.tswework/src/api/local/localServices.tswework/src/api/runtime/runtimeChatStream.test.tswework/src/api/runtime/runtimeChatStream.tswework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/layout/MobileWorkbenchLayout.tsxwework/src/components/layout/workbenchPaneStack.test.tsxwework/src/components/layout/workbenchPaneStack.tsxwework/src/lib/developerCommandMenu.test.tswework/src/lib/developerCommandMenu.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- wework/src/api/hybrid/hybridServices.ts
Comment on lines
+46
to
+58
| deps.subscribe(event => { | ||
| if (import.meta.env.DEV && event.event === 'runtime.plan.updated') { | ||
| console.warn('[Wework] Local runtime task plan event received', { | ||
| console.warn('[Wework] Runtime task plan event received', { | ||
| taskId: stringField(asRecord(event.payload), 'taskId') ?? null, | ||
| deviceId: stringField(asRecord(event.payload), 'deviceId') ?? null, | ||
| }) | ||
| } | ||
| const subscriptionEntries = Array.from(subscriptions) | ||
| if (shouldLogLocalChatStreamEvent(event.event)) { | ||
| if (shouldLogRuntimeChatStreamEvent(event.event)) { | ||
| const matchedSubscriptionCount = subscriptionEntries.filter(([, subscription]) => | ||
| isLocalExecutorEventInScope(event, subscription.handlers.scope) | ||
| ).length | ||
| logLocalChatTerminalEvent(event, matchedSubscriptionCount, subscriptionEntries.length) | ||
| logRuntimeChatTerminalEvent(event, matchedSubscriptionCount, subscriptionEntries.length) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
The eager listener still drops events before pane subscription. The runtime stream needs a per-task pending tool-event cache and a regression test that proves late replay.
wework/src/api/runtime/runtimeChatStream.ts#L46-L58: cache relevantresponse.block.*events when no matching subscription exists, replay them during subscription, and clear them on terminal responses.wework/src/api/runtime/runtimeChatStream.test.ts#L203-L210: emit arequest_user_inputblock before pane subscription and assert that the late subscriber receives it.
📍 Affects 2 files
wework/src/api/runtime/runtimeChatStream.ts#L46-L58(this comment)wework/src/api/runtime/runtimeChatStream.test.ts#L203-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/api/runtime/runtimeChatStream.ts` around lines 46 - 58, The
runtime stream currently drops relevant response.block.* events received before
a pane subscription exists. In
wework/src/api/runtime/runtimeChatStream.ts:46-58, add a per-task pending
tool-event cache, store events with no matching subscription, replay cached
events when that task subscribes, and clear the cache on terminal responses. In
wework/src/api/runtime/runtimeChatStream.test.ts:203-210, add a regression test
that emits a request_user_input block before subscription and verifies the late
subscriber receives it.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/agents/codex.rs`:
- Around line 190-247: Update InteractionAnswerRouterState with a closed flag,
set it when the background receiver loop exits before draining pending waiters,
and have InteractionAnswerRouter::receive reject new requests immediately when
closed instead of inserting them into pending. Preserve existing buffered-answer
and duplicate-key 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: 30c3f3cb-0921-4b82-aa3e-716bef157d1d
📒 Files selected for processing (6)
executor/src/agents/codex.rsexecutor/tests/app_runtime_work_send_contract.rswework/e2e/desktop/task-flow.e2e.mjswework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/lib/developerCommandMenu.test.tswework/src/lib/developerCommandMenu.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- wework/src/lib/developerCommandMenu.ts
- wework/e2e/desktop/task-flow.e2e.mjs
This was referenced Aug 19, 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
localChatStreamtoruntimeChatStreamrequest_user_inputor MCP elicitation waits for an answerrequest_user_input, return to the task, answer it, and capture the key statesRoot cause
Inactive panes could be evicted and unmounted, which unsubscribed them from runtime events. Separately, the Executor synchronously waited for an interaction answer inside the same loop that drains Codex notifications. A long wait could overflow the broadcast receiver and surface
Laggedafter the user answered.Codex remains the authoritative source for pending interactions. This change does not add frontend replay state or thread-resume behavior.
Validation
request_user_inputflow and captured all three verification states