Repository navigation
feat(wework): unify cloud runtime IPC - #1797
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughIntroduces a stable ChangesBackend runtime_instance_id and device schema
Estimated code review effort: 4 (Complex) | ~60 minutes Executor and Tauri runtime_instance_id plumbing
Estimated code review effort: 3 (Moderate) | ~30 minutes Frontend cloud runtime IPC client and device/work merging
Estimated code review effort: 4 (Complex) | ~75 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
…ntime-ipc # Conflicts: # wework/src/features/workbench/WorkbenchProvider.test.tsx # wework/src/features/workbench/useWorkbenchRuntimeMessaging.ts
Micro66
marked this pull request as ready for review
July 8, 2026 10:08
![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: 4
🧹 Nitpick comments (4)
wework/src/api/backend/runtimeIpc.ts (1)
53-60: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant wrapper function in
subscribe.
runtimeHandlerwrapshandlerwithout adding behavior. Passhandlerdirectly to reduce indirection.♻️ Proposed refactor
async subscribe(handler: (event: LocalExecutorEvent) => void): Promise<() => void> { await client.ensureConnected() - const runtimeHandler = (event: LocalExecutorEvent) => { - handler(event) - } - client.socket.on(RUNTIME_EVENT, runtimeHandler) - return () => client.socket.off(RUNTIME_EVENT, runtimeHandler) + client.socket.on(RUNTIME_EVENT, handler) + return () => client.socket.off(RUNTIME_EVENT, handler) },🤖 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/backend/runtimeIpc.ts` around lines 53 - 60, `subscribe` in `runtimeIpc` uses a redundant `runtimeHandler` wrapper that only forwards to `handler` without adding behavior. Simplify this by registering `handler` directly with `client.socket.on(RUNTIME_EVENT, ...)` and removing the extra local function, while keeping the unsubscribe callback paired with the same function reference.wework/src/api/local/localServices.ts (3)
242-247: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider exporting
RuntimeWorkIpcOptions.
createRuntimeWorkApiFromIpcis exported and now consumed cross-module (hybridServices.tspasses aresolveDeviceIdoverride), but the options type itself isn't exported, so external callers can't reference it for strongly-typed option objects.🤖 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 242 - 247, The RuntimeWorkIpcOptions type used by createRuntimeWorkApiFromIpc is currently internal, which prevents cross-module consumers like hybridServices from typing their option objects. Export RuntimeWorkIpcOptions alongside createRuntimeWorkApiFromIpc so external callers can reference resolveDeviceId, normalizeDeviceRecord, and adaptListResponse with proper TypeScript support.
1410-1561: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
normalizeDeviceRecord/resolveDeviceIdhooks are inconsistently applied across methods.
requestWithLocalDevicecorrectly funnels through the configurablenormalizeDeviceRecord/resolveDeviceId(Lines 1418-1431), but several other exported methods bypass this entirely:
guideRuntimeTask(Lines 1546-1550) hardcodesnormalizeLocalDeviceRecordinstead of the configurednormalizeDeviceRecord.sendRuntimeMessage/rollbackRuntimeTask/createRuntimeTaskcallresolveDeviceIdbut never applynormalizeDeviceRecordat all, relying oncreateLocalRuntimeSendPayload/createLocalRuntimeTaskPayloadinstead.Since
hybridServices.tscurrently only overridesresolveDeviceId(notnormalizeDeviceRecord), there's no active bug today, but the abstraction is misleading: a future override ofnormalizeDeviceRecordfor cloud routing would silently not apply to these methods.Separately,
requestWithLocalDevicecallsresolveDeviceIdtwice — once insidenormalizeRequest(Line 1423) and again directly (Line 1431) — which is redundant.♻️ Avoid the redundant resolveDeviceId call
const requestWithLocalDevice = async <TResponse, TRequest extends object>( method: string, data: TRequest ): Promise<TResponse> => { - const normalizedData = await normalizeRequest(data) - const deviceId = await resolveDeviceId(normalizedData) + const deviceId = await resolveDeviceId(data as unknown as Record<string, unknown>) + const normalizedData = normalizeDeviceRecord(data as Record<string, unknown>, deviceId) as TRequest & + Record<string, unknown>🤖 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 1410 - 1561, The device normalization hooks are applied inconsistently in createRuntimeWorkApiFromIpc, and requestWithLocalDevice also resolves the device twice. Update sendRuntimeMessage, rollbackRuntimeTask, createRuntimeTask, and guideRuntimeTask to route through the configurable normalizeDeviceRecord and resolveDeviceId behavior used by normalizeRequest/requestWithLocalDevice, instead of hardcoding normalizeLocalDeviceRecord or bypassing normalization via the payload builders. Also remove the extra resolveDeviceId call in requestWithLocalDevice by reusing the device ID from normalizeRequest so the hook flow is consistent across all exported methods.Source: Learnings
1304-1408: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winLabel-based dedup heuristic can hide unrelated empty remote workspaces; duplicated workspace-path parsing logic.
Two concerns in
adaptRuntimeWorkListResponse:
- The new first pass (Lines 1324-1339) collects labels from all non-
remoteworkspaces and later (Lines 1369-1373) drops any zero-taskremoteworkspace whose label collides with one of those. Labels default to the last path segment (workspaceLabel), so two unrelated workspaces from different hosts sharing a common folder name (e.g.project,src) would cause a legitimate, still-empty remote workspace to be silently hidden from the UI.- The workspacePath derivation logic (Lines 1327-1333 and 1343-1349) is duplicated verbatim between the label-collection loop and the main loop.
♻️ Suggested extraction to remove duplication
+function resolveWorkspacePath(workspace: Record<string, unknown>): string | undefined { + return ( + stringValue(workspace.workspacePath) ?? + stringValue(workspace.workspace_path) ?? + stringValue(workspace.projectWorkspacePath) ?? + stringValue(workspace.project_workspace_path) ?? + stringValue(workspace.cwd) ?? + stringValue(workspace.path) + ) +}Based on learnings, "avoid duplicate logic/copy-paste" and "extract common logic into utilities or modules".
🤖 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 1304 - 1408, `adaptRuntimeWorkListResponse` is using a label-based dedup rule that can incorrectly hide unrelated empty remote workspaces, and it repeats the same workspace-path parsing in both loops. Extract the workspacePath resolution into a shared helper used by both passes, then replace the `localWorkspaceLabels` heuristic with a safer dedup key based on a more specific identifier such as workspacePath plus source/host context (or remove the filter if that context is unavailable) so only true duplicates are skipped.Source: Learnings
🤖 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 `@backend/app/api/ws/wework_runtime_namespace.py`:
- Line 178: The `max_output_bytes` argument parsing in `on_runtime_request` can
throw on malformed client input, bypassing the existing
`RuntimeRpcError`/`DeviceCommandError` handling. Update the
`int(params.get("max_output_bytes") or 1024 * 1024)` logic to parse defensively
inside `wework_runtime_namespace.py`, preferably by introducing a safe helper or
inline validation before constructing the request payload, and fall back to the
default value when the input is missing or invalid. If appropriate, also enforce
an upper bound so `max_output_bytes` cannot be set excessively high.
- Around line 111-133: `on_runtime_request` needs a top-level type check before
any field extraction so non-dict payloads don’t trigger uncaught AttributeError.
Add the same `isinstance(data, dict)` guard used by `on_runtime_event` in
`wework_runtime_namespace`/`device_namespace` logic before calling
`request_id_from`, `string_field`, or `data.get`, and return a bad_request IPC
error when the payload is not an object.
In `@wework/src/api/backend/runtimeIpc.ts`:
- Around line 95-106: The ack handling in the runtime IPC callback is too
permissive and can also hide missing data. Update the success check in
runtimeIpc’s acknowledgement handler to require an explicit true value on ok
instead of treating a missing field as success, and avoid casting a missing
result to T when resolving. Use the existing formatRuntimeIpcError path for
failures, and make sure the resolve branch only returns a real result from the
ack payload so malformed or incomplete acknowledgements do not masquerade as
successful responses.
In `@wework/src/api/hybrid/hybridServices.ts`:
- Around line 480-497: The `searchRuntimeWork` cloud fan-out in
`hybridServices.ts` is bypassing the same device filtering used by
`listCloudRuntimeWork`, so it can query offline devices and duplicate results
for the same `runtime_instance_id`. Reuse the existing `isUsableDevice` plus
local-runtime deduplication logic before building the `cloudDevicesPromise`, and
apply it consistently to the `listCloudDevices()` result before calling
`cloudRuntimeApi(...).searchRuntimeWork(data)`. Keep the behavior aligned with
`listCloudRuntimeWork` so both paths query only reachable, non-duplicate cloud
devices.
---
Nitpick comments:
In `@wework/src/api/backend/runtimeIpc.ts`:
- Around line 53-60: `subscribe` in `runtimeIpc` uses a redundant
`runtimeHandler` wrapper that only forwards to `handler` without adding
behavior. Simplify this by registering `handler` directly with
`client.socket.on(RUNTIME_EVENT, ...)` and removing the extra local function,
while keeping the unsubscribe callback paired with the same function reference.
In `@wework/src/api/local/localServices.ts`:
- Around line 242-247: The RuntimeWorkIpcOptions type used by
createRuntimeWorkApiFromIpc is currently internal, which prevents cross-module
consumers like hybridServices from typing their option objects. Export
RuntimeWorkIpcOptions alongside createRuntimeWorkApiFromIpc so external callers
can reference resolveDeviceId, normalizeDeviceRecord, and adaptListResponse with
proper TypeScript support.
- Around line 1410-1561: The device normalization hooks are applied
inconsistently in createRuntimeWorkApiFromIpc, and requestWithLocalDevice also
resolves the device twice. Update sendRuntimeMessage, rollbackRuntimeTask,
createRuntimeTask, and guideRuntimeTask to route through the configurable
normalizeDeviceRecord and resolveDeviceId behavior used by
normalizeRequest/requestWithLocalDevice, instead of hardcoding
normalizeLocalDeviceRecord or bypassing normalization via the payload builders.
Also remove the extra resolveDeviceId call in requestWithLocalDevice by reusing
the device ID from normalizeRequest so the hook flow is consistent across all
exported methods.
- Around line 1304-1408: `adaptRuntimeWorkListResponse` is using a label-based
dedup rule that can incorrectly hide unrelated empty remote workspaces, and it
repeats the same workspace-path parsing in both loops. Extract the workspacePath
resolution into a shared helper used by both passes, then replace the
`localWorkspaceLabels` heuristic with a safer dedup key based on a more specific
identifier such as workspacePath plus source/host context (or remove the filter
if that context is unavailable) so only true duplicates are skipped.
🪄 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: f499d382-107a-40f8-833a-752e41aad52c
📒 Files selected for processing (53)
AGENTS.mdbackend/app/api/ws/device_namespace.pybackend/app/api/ws/wework_runtime_namespace.pybackend/app/main.pybackend/app/schemas/device.pybackend/app/services/device/cloud_provider.pybackend/app/services/device/local_provider.pybackend/app/services/device/remote_provider.pybackend/app/services/device_service.pybackend/tests/api/ws/test_device_reconnect_storm.pybackend/tests/api/ws/test_wework_runtime_namespace.pybackend/tests/services/test_device_alias_preservation.pybackend/tests/services/test_local_device_provider.pydocs/en/developer-guide/wework-cloud-connection.mddocs/zh/developer-guide/wework-cloud-connection.mdexecutor/src/app/mod.rsexecutor/src/config/device.rsexecutor/src/local/app_ipc.rsexecutor/src/local/backend.rsexecutor/src/local/backend/client.rsexecutor/src/local/backend/config.rsexecutor/tests/config_contract.rsexecutor/tests/local_backend_contract.rsexecutor/tests/local_backend_device_migration_contract.rsexecutor/tests/local_backend_dispatch_contract.rsexecutor/tests/local_backend_resilience_contract.rswework/src-tauri/src/local_executor.rswework/src/api/backend/runtimeIpc.tswework/src/api/executorAccess.test.tswework/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/components/layout/DesktopSidebar.test.tsxwework/src/components/layout/DesktopSidebar.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/projects/ProjectCreateDialog.test.tsxwework/src/components/projects/ProjectCreateDialog.tsxwework/src/components/projects/StandaloneProjectDialogs.tsxwework/src/features/cloud-connection/CloudConnectionSidebarButton.tsxwework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/useWorkbenchDataRefresh.tswework/src/features/workbench/useWorkbenchRuntimeMessaging.tswework/src/features/workbench/workbenchCloudStatus.test.tswework/src/features/workbench/workbenchCloudStatus.tswework/src/features/workbench/workbenchReducer.test.tswework/src/features/workbench/workbenchReducer.tswework/src/lib/device-capabilities.tswework/src/tauri/localExecutor.tswework/src/types/api.tswework/src/types/devices.tswework/src/types/workbench.ts
Comment on lines
+111
to
+133
| async def on_runtime_request(self, sid: str, data: dict) -> dict: | ||
| """Relay one app IPC-style runtime request to an online executor.""" | ||
|
|
||
| session = await self.get_session(sid) | ||
| user_id = session.get("user_id") if session else None | ||
| if not user_id: | ||
| return ipc_error(data, "unauthorized", "Not authenticated") | ||
|
|
||
| request_id = request_id_from(data) | ||
| method = string_field(data, "method") | ||
| device_id = string_field(data, "device_id") or string_field(data, "deviceId") | ||
| params = data.get("params") | ||
| if params is None: | ||
| params = data.get("payload", {}) | ||
|
|
||
| if not method: | ||
| return ipc_error(data, "bad_request", "method is required", request_id) | ||
| if not device_id: | ||
| return ipc_error(data, "bad_request", "device_id is required", request_id) | ||
| if not isinstance(params, dict): | ||
| return ipc_error( | ||
| data, "bad_request", "params must be an object", request_id | ||
| ) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard data as a dict before field extraction.
on_runtime_request never validates that data is a dict. If a client sends a non-dict payload, string_field(data, "method") (and data.get("params")) call .get on a non-dict and raise AttributeError, which isn't caught by the RuntimeRpcError/DeviceCommandError handler — the client gets no error ACK. Note that on_runtime_event in device_namespace.py already performs this guard.
🛡️ Proposed guard
session = await self.get_session(sid)
user_id = session.get("user_id") if session else None
if not user_id:
return ipc_error(data, "unauthorized", "Not authenticated")
+ if not isinstance(data, dict):
+ return ipc_error(data, "bad_request", "payload must be an object")
request_id = request_id_from(data)📝 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
| async def on_runtime_request(self, sid: str, data: dict) -> dict: | |
| """Relay one app IPC-style runtime request to an online executor.""" | |
| session = await self.get_session(sid) | |
| user_id = session.get("user_id") if session else None | |
| if not user_id: | |
| return ipc_error(data, "unauthorized", "Not authenticated") | |
| request_id = request_id_from(data) | |
| method = string_field(data, "method") | |
| device_id = string_field(data, "device_id") or string_field(data, "deviceId") | |
| params = data.get("params") | |
| if params is None: | |
| params = data.get("payload", {}) | |
| if not method: | |
| return ipc_error(data, "bad_request", "method is required", request_id) | |
| if not device_id: | |
| return ipc_error(data, "bad_request", "device_id is required", request_id) | |
| if not isinstance(params, dict): | |
| return ipc_error( | |
| data, "bad_request", "params must be an object", request_id | |
| ) | |
| async def on_runtime_request(self, sid: str, data: dict) -> dict: | |
| """Relay one app IPC-style runtime request to an online executor.""" | |
| session = await self.get_session(sid) | |
| user_id = session.get("user_id") if session else None | |
| if not user_id: | |
| return ipc_error(data, "unauthorized", "Not authenticated") | |
| if not isinstance(data, dict): | |
| return ipc_error(data, "bad_request", "payload must be an object") | |
| request_id = request_id_from(data) | |
| method = string_field(data, "method") | |
| device_id = string_field(data, "device_id") or string_field(data, "deviceId") | |
| params = data.get("params") | |
| if params is None: | |
| params = data.get("payload", {}) | |
| if not method: | |
| return ipc_error(data, "bad_request", "method is required", request_id) | |
| if not device_id: | |
| return ipc_error(data, "bad_request", "device_id is required", request_id) | |
| if not isinstance(params, dict): | |
| return ipc_error( | |
| data, "bad_request", "params must be an object", request_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 `@backend/app/api/ws/wework_runtime_namespace.py` around lines 111 - 133,
`on_runtime_request` needs a top-level type check before any field extraction so
non-dict payloads don’t trigger uncaught AttributeError. Add the same
`isinstance(data, dict)` guard used by `on_runtime_event` in
`wework_runtime_namespace`/`device_namespace` logic before calling
`request_id_from`, `string_field`, or `data.get`, and return a bad_request IPC
error when the payload is not an object.
| args=params.get("args") if isinstance(params.get("args"), list) else [], | ||
| env=params.get("env") if isinstance(params.get("env"), dict) else {}, | ||
| timeout_seconds=timeout_seconds, | ||
| max_output_bytes=int(params.get("max_output_bytes") or 1024 * 1024), |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
max_output_bytes parsing can crash on malformed client input.
int(params.get("max_output_bytes") or 1024 * 1024) raises ValueError/TypeError for client-supplied values like "abc" or a list. That exception escapes the except (RuntimeRpcError, DeviceCommandError) in on_runtime_request, so the request fails without a structured error ACK. Consider parsing defensively (and optionally clamping to an upper bound).
🛡️ Proposed defensive parse
- return await local_device_command_service.execute_command(
+ try:
+ max_output_bytes = int(params.get("max_output_bytes") or 1024 * 1024)
+ except (TypeError, ValueError):
+ max_output_bytes = 1024 * 1024
+ return await local_device_command_service.execute_command(
user_id=user_id,
device_id=device_id,
command=command.command,
path=params.get("path") if isinstance(params.get("path"), str) else None,
cwd=params.get("cwd") if isinstance(params.get("cwd"), str) else None,
args=params.get("args") if isinstance(params.get("args"), list) else [],
env=params.get("env") if isinstance(params.get("env"), dict) else {},
timeout_seconds=timeout_seconds,
- max_output_bytes=int(params.get("max_output_bytes") or 1024 * 1024),
+ max_output_bytes=max_output_bytes,
)📝 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
| max_output_bytes=int(params.get("max_output_bytes") or 1024 * 1024), | |
| try: | |
| max_output_bytes = int(params.get("max_output_bytes") or 1024 * 1024) | |
| except (TypeError, ValueError): | |
| max_output_bytes = 1024 * 1024 | |
| return await local_device_command_service.execute_command( | |
| user_id=user_id, | |
| device_id=device_id, | |
| command=command.command, | |
| path=params.get("path") if isinstance(params.get("path"), str) else None, | |
| cwd=params.get("cwd") if isinstance(params.get("cwd"), str) else None, | |
| args=params.get("args") if isinstance(params.get("args"), list) else [], | |
| env=params.get("env") if isinstance(params.get("env"), dict) else {}, | |
| timeout_seconds=timeout_seconds, | |
| max_output_bytes=max_output_bytes, | |
| ) |
🤖 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 `@backend/app/api/ws/wework_runtime_namespace.py` at line 178, The
`max_output_bytes` argument parsing in `on_runtime_request` can throw on
malformed client input, bypassing the existing
`RuntimeRpcError`/`DeviceCommandError` handling. Update the
`int(params.get("max_output_bytes") or 1024 * 1024)` logic to parse defensively
inside `wework_runtime_namespace.py`, preferably by introducing a safe helper or
inline validation before constructing the request payload, and fall back to the
default value when the input is missing or invalid. If appropriate, also enforce
an upper bound so `max_output_bytes` cannot be set excessively high.
Comment on lines
+95
to
+106
| (ack: RuntimeIpcAck<T> | undefined) => { | ||
| window.clearTimeout(timeout) | ||
| if (!ack) { | ||
| reject(new Error(`${method} returned an empty acknowledgement`)) | ||
| return | ||
| } | ||
| if (ack.ok === false || ack.error) { | ||
| reject(new Error(formatRuntimeIpcError(ack))) | ||
| return | ||
| } | ||
| resolve((ack.result ?? null) as T) | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Unsafe null cast and permissive ack success check.
Two related issues in the ack handling:
-
ack.ok === falsetreats a missingokfield as success. An empty or malformed ack ({}) resolves as a successful response. Useack.ok !== trueto require explicit success confirmation. -
resolve((ack.result ?? null) as T)silently castsnulltoT. If the server returns{ ok: true }without aresult, callers receivenullat runtime while TypeScript believes it'sT, causing downstream null-pointer errors.
🔧 Proposed fix for ack handling
(ack: RuntimeIpcAck<T> | undefined) => {
window.clearTimeout(timeout)
if (!ack) {
reject(new Error(`${method} returned an empty acknowledgement`))
return
}
- if (ack.ok === false || ack.error) {
+ if (ack.ok !== true || ack.error) {
reject(new Error(formatRuntimeIpcError(ack)))
return
}
- resolve((ack.result ?? null) as T)
+ if (ack.result === undefined || ack.result === null) {
+ reject(new Error(`${method} returned no result`))
+ return
+ }
+ resolve(ack.result as T)
}📝 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
| (ack: RuntimeIpcAck<T> | undefined) => { | |
| window.clearTimeout(timeout) | |
| if (!ack) { | |
| reject(new Error(`${method} returned an empty acknowledgement`)) | |
| return | |
| } | |
| if (ack.ok === false || ack.error) { | |
| reject(new Error(formatRuntimeIpcError(ack))) | |
| return | |
| } | |
| resolve((ack.result ?? null) as T) | |
| } | |
| (ack: RuntimeIpcAck<T> | undefined) => { | |
| window.clearTimeout(timeout) | |
| if (!ack) { | |
| reject(new Error(`${method} returned an empty acknowledgement`)) | |
| return | |
| } | |
| if (ack.ok !== true || ack.error) { | |
| reject(new Error(formatRuntimeIpcError(ack))) | |
| return | |
| } | |
| if (ack.result === undefined || ack.result === null) { | |
| reject(new Error(`${method} returned no result`)) | |
| return | |
| } | |
| resolve(ack.result as T) | |
| } |
🤖 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/backend/runtimeIpc.ts` around lines 95 - 106, The ack handling
in the runtime IPC callback is too permissive and can also hide missing data.
Update the success check in runtimeIpc’s acknowledgement handler to require an
explicit true value on ok instead of treating a missing field as success, and
avoid casting a missing result to T when resolving. Use the existing
formatRuntimeIpcError path for failures, and make sure the resolve branch only
returns a real result from the ack payload so malformed or incomplete
acknowledgements do not masquerade as successful responses.
Comment on lines
+480
to
+497
| const cloudDevicesPromise = listCloudDevices() | ||
| const [localResult, cloudDevicesResult] = await Promise.allSettled([ | ||
| localServices.runtimeWorkApi!.searchRuntimeWork(data), | ||
| cloudServices.runtimeWorkApi!.searchRuntimeWork(data), | ||
| cloudDevicesPromise.then(devices => | ||
| Promise.allSettled( | ||
| devices.map(device => cloudRuntimeApi(device.device_id).searchRuntimeWork(data)) | ||
| ) | ||
| ), | ||
| ]) | ||
| const cloudItems = | ||
| cloudDevicesResult.status === 'fulfilled' | ||
| ? cloudDevicesResult.value.flatMap(result => | ||
| result.status === 'fulfilled' ? result.value.items : [] | ||
| ) | ||
| : [] | ||
| return mergeSearchResults( | ||
| fulfilledValue(localResult, { items: [] }), | ||
| fulfilledValue(cloudResult, { items: [] }), | ||
| { items: cloudItems }, |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
searchRuntimeWork's cloud fan-out is missing the same filtering listCloudRuntimeWork applies.
listCloudRuntimeWork (Lines 389-393) filters cloud devices with isUsableDevice and excludes devices sharing a runtime_instance_id with a local device before querying them. searchRuntimeWork queries listCloudDevices() unfiltered (Line 480, 485), so it can issue requests to offline devices and produce duplicate results for a device reachable via two routes (local + cloud-relay to the same runtime instance).
🐛 Suggested fix — reuse the same filter
async searchRuntimeWork(data: RuntimeWorkSearchRequest) {
- const cloudDevicesPromise = listCloudDevices()
+ const cloudDevicesPromise = listCloudDevices().then(devices =>
+ devices.filter(
+ device =>
+ isUsableDevice(device) &&
+ !(device.runtime_instance_id && localRuntimeInstanceIds.has(device.runtime_instance_id))
+ )
+ )📝 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 cloudDevicesPromise = listCloudDevices() | |
| const [localResult, cloudDevicesResult] = await Promise.allSettled([ | |
| localServices.runtimeWorkApi!.searchRuntimeWork(data), | |
| cloudServices.runtimeWorkApi!.searchRuntimeWork(data), | |
| cloudDevicesPromise.then(devices => | |
| Promise.allSettled( | |
| devices.map(device => cloudRuntimeApi(device.device_id).searchRuntimeWork(data)) | |
| ) | |
| ), | |
| ]) | |
| const cloudItems = | |
| cloudDevicesResult.status === 'fulfilled' | |
| ? cloudDevicesResult.value.flatMap(result => | |
| result.status === 'fulfilled' ? result.value.items : [] | |
| ) | |
| : [] | |
| return mergeSearchResults( | |
| fulfilledValue(localResult, { items: [] }), | |
| fulfilledValue(cloudResult, { items: [] }), | |
| { items: cloudItems }, | |
| const cloudDevicesPromise = listCloudDevices().then(devices => | |
| devices.filter( | |
| device => | |
| isUsableDevice(device) && | |
| !(device.runtime_instance_id && localRuntimeInstanceIds.has(device.runtime_instance_id)) | |
| ) | |
| ) | |
| const [localResult, cloudDevicesResult] = await Promise.allSettled([ | |
| localServices.runtimeWorkApi!.searchRuntimeWork(data), | |
| cloudDevicesPromise.then(devices => | |
| Promise.allSettled( | |
| devices.map(device => cloudRuntimeApi(device.device_id).searchRuntimeWork(data)) | |
| ) | |
| ), | |
| ]) | |
| const cloudItems = | |
| cloudDevicesResult.status === 'fulfilled' | |
| ? cloudDevicesResult.value.flatMap(result => | |
| result.status === 'fulfilled' ? result.value.items : [] | |
| ) | |
| : [] | |
| return mergeSearchResults( | |
| fulfilledValue(localResult, { items: [] }), | |
| { items: cloudItems }, |
🤖 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 480 - 497, The
`searchRuntimeWork` cloud fan-out in `hybridServices.ts` is bypassing the same
device filtering used by `listCloudRuntimeWork`, so it can query offline devices
and duplicate results for the same `runtime_instance_id`. Reuse the existing
`isUsableDevice` plus local-runtime deduplication logic before building the
`cloudDevicesPromise`, and apply it consistently to the `listCloudDevices()`
result before calling `cloudRuntimeApi(...).searchRuntimeWork(data)`. Keep the
behavior aligned with `listCloudRuntimeWork` so both paths query only reachable,
non-duplicate cloud devices.
This was referenced Jul 9, 2026
This was referenced Jul 17, 2026
This was referenced Jul 24, 2026
Merged
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.
Summary
Unifies Wework cloud runtime execution with the local socket protocol by carrying app IPC requests and events through backend WebSocket transport.
Changes
/wework-runtimeSocket.IO namespace that authenticates users and relays app IPC requests to the selected device runtime executor.runtime:eventwithout translating them into chat events.Validation
uv run pytest tests/api/ws/test_wework_runtime_namespace.pyuv run python -m py_compile app/api/ws/wework_runtime_namespace.py app/api/ws/device_namespace.py app/main.pycargo check -p wegent-executorpnpm --filter wework typecheckpnpm --filter wework test -- src/api/hybrid/hybridServices.test.ts src/api/local/localServices.test.ts src/api/local/localChatStream.test.tsSummary by CodeRabbit
New Features
Bug Fixes
Documentation