Repository navigation
[codex] Add Wework local-first desktop runtime - #1549
Merged
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughAdds local-first runtime mode selection, local executor IPC and sidecar startup, WeWork-specific websocket room routing, updated workbench/local service wiring, and matching tests/docs. ChangesWeWork local-first runtime
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.23.0)executor/scripts/dev_sidecar.py┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.14][ERROR]: unable to find a config; path executor/tests/runtime_work/test_local_task_store.py┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.12][ERROR]: unable to find a config; path executor/tests/scripts/test_wework_dev_mac_app.py┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.34][ERROR]: unable to find a config; path
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
force-pushed
the
codex/wework-local-first-app
branch
from
June 25, 2026 09:59
c02c136 to
80f26d4
Compare
qdaxb
force-pushed
the
codex/wework-local-first-app
branch
from
June 25, 2026 10:13
80f26d4 to
121afa8
Compare
qdaxb
marked this pull request as ready for review
June 25, 2026 11:56
![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: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (11)
wework/src-tauri/src/local_executor.rs-66-73 (1)
66-73: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winWait after killing configured sidecar processes.
For
std::process::Child,kill()does not reap the process. Restart/close paths can leave zombie configured-sidecar processes unless this branch also callswait().Proposed fix
LocalExecutorChild::Process(mut child) => { let _ = child.kill(); + let _ = child.wait(); }🤖 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-tauri/src/local_executor.rs` around lines 66 - 73, The LocalExecutorChild::kill implementation only sends a kill signal for the std::process::Child branch and does not reap the process, which can leave zombie configured-sidecar processes. Update the Process variant in kill(self) to also call wait() after kill(), mirroring the cleanup behavior needed for restart/close paths, and keep the Tauri branch unchanged.docs/zh/developer-guide/runtime-local-work.md-140-140 (1)
140-140: 🎯 Functional Correctness | 🟡 Minor统一“本地优先”与“空工作区”两条路径的文档描述
第 140 行描述
local-first模式下“如果缺少workspacePath必须在调用前失败”，这与第 157–160 行描述的“无项目 / 空工作区”本地对话路径冲突。请修订第 140 行，删除对workspacePath的硬性要求，明确local-first创建任务时应兼容空工作区上下文。🤖 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 `@docs/zh/developer-guide/runtime-local-work.md` at line 140, The description in the local-first workflow is too strict about requiring workspacePath and conflicts with the empty-workspace path. Update the wording in the runtime-local-work documentation section around the local-first task creation flow to align with the empty-workspace behavior, using the local-first and executionRequest descriptions as anchors, and remove the hard failure requirement tied to workspacePath so the flow clearly supports creating tasks with an empty workspace context.docs/en/developer-guide/runtime-local-work.md-140-140 (1)
140-140: 🎯 Functional Correctness | 🟡 MinorClarify
workspacePathrequirement for empty workspace contextLine 140 states that local-first task creation must fail if
workspacePathis missing, but Lines 157–160 explicitly document a valid path where a task is created from an "empty workspace context" when no project is selected. These sections present contradictory requirements: the "local-first" paragraph enforces a mandatory path, while the "Project-backed" section allows creation without one.Please reconcile these logic blocks so the documentation reflects whether a task can be initiated from a transient or empty workspace state without failing, or clarify how the empty context satisfies the
workspacePathconstraint.🤖 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 `@docs/en/developer-guide/runtime-local-work.md` at line 140, The local-first task creation description conflicts with the empty-workspace flow, so reconcile the `workspacePath` requirement in the runtime-local-work docs. Update the text around the local-first executionRequest path and the “Project-backed” section to clearly state whether `workspacePath` is strictly required or whether an empty/transient workspace context is valid, and if valid, explain how that context satisfies the `runtime.tasks.create` input without implying a hard failure.docs/zh/user-guide/ai-devices/local-device-support.md-228-228 (1)
228-228: 🎯 Functional Correctness | 🟡 Minor补充配置
WEGENT_AUTH_TOKEN/connection.auth_token的说明。当前文档在描述连接远端 Backend 时仅提到了地址配置（
WEGENT_BACKEND_URL/connection.backend_url），但遗漏了连接所必需的认证信息。实际启动逻辑要求必须同时设置认证 Token。建议在描述 URL 配置的同时，补充说明需要一并配置
WEGENT_AUTH_TOKEN环境变量或配置文件中的connection.auth_token，以避免用户因缺少认证信息导致连接失败。🤖 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 `@docs/zh/user-guide/ai-devices/local-device-support.md` at line 228, The local-device support docs only mention WEGENT_BACKEND_URL and connection.backend_url for remote Backend setup, but they also need to mention the required authentication token. Update the explanation around executor startup and Backend connection to include WEGENT_AUTH_TOKEN and connection.auth_token, using the existing device configuration wording so readers understand both URL and token must be set for local device mode to connect successfully.wework/src/stream/responseApiStream.ts-366-377 (1)
366-377: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEmpty error string can leak; default message is unreachable.
eventContent(payload)always returns astring(falling back to''), so the?? 'Response stream error'default is dead code, and an error event with no usable message produces an emptyerrorfield rather than the intended fallback.🐛 Proposed fix
const errorRecord = asRecord(error) return ( stringField(payload, 'error') ?? stringField(data, 'message') ?? stringField(errorRecord, 'message') ?? - eventContent(payload) ?? - 'Response stream error' + (eventContent(payload) || 'Response stream error') )🤖 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/stream/responseApiStream.ts` around lines 366 - 377, The errorMessage helper can return an empty string because eventContent(payload) always yields a string, making the final fallback unreachable. Update errorMessage in responseApiStream so it treats empty strings as missing values by checking for non-empty text before returning, and ensure the fallback 'Response stream error' is used when payload.error, data.message, and errorRecord.message are absent or blank.wework/src/features/workbench/WorkbenchProvider.tsx-1919-1925 (1)
1919-1925: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNo-op empty archive requests before requiring the runtime API.
archiveChatConversations([])should return before checkingruntimeWorkApi; otherwise an empty selection can incorrectly surface “Local runtime work is unavailable”.Proposed fix
const archiveChatConversations = useCallback( async (addresses: RuntimeTaskAddress[]) => { + if (addresses.length === 0) return + const runtimeWorkApi = resolvedServices.runtimeWorkApi if (!runtimeWorkApi) { dispatch({ type: 'error_set', error: 'Local runtime work is unavailable' }) return } - if (addresses.length === 0) return - const responses = await Promise.all(🤖 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 1919 - 1925, `archiveChatConversations` is checking `runtimeWorkApi` before handling an empty `addresses` array, which makes `archiveChatConversations([])` surface a runtime error instead of no-oping. Update the `archiveChatConversations` flow in `WorkbenchProvider` to return immediately when `addresses.length === 0` before accessing `resolvedServices.runtimeWorkApi`, and keep the existing runtime API error dispatch only for non-empty requests that actually need it.shared/logger.py-94-100 (1)
94-100: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRename the helper argument away from
format.Ruff flags the new
_file_log_handlerparameter for shadowing the Python built-in. Keep the publicsetup_logger(..., format=...)contract, but uselog_formatinside the helper.Proposed fix
def _file_log_handler( *, level: int, - format: str, + log_format: str, datefmt: str, include_request_id: bool, ) -> Optional[RotatingFileHandler]: @@ - handler.setFormatter(logging.Formatter(format, datefmt)) + handler.setFormatter(logging.Formatter(log_format, datefmt)) @@ handler = _file_log_handler( level=level, - format="%(asctime)s - [%(request_id)s] - [%(name)s] - %(levelname)s - %(message)s", + log_format="%(asctime)s - [%(request_id)s] - [%(name)s] - %(levelname)s - %(message)s", datefmt="%Y-%m-%d %H:%M:%S", include_request_id=True, ) @@ file_handler = _file_log_handler( level=level, - format=format, + log_format=format, datefmt=datefmt, include_request_id=include_request_id, ) @@ file_handler = _file_log_handler( level=level, - format=format, + log_format=format, datefmt=datefmt, include_request_id=include_request_id, )Also applies to: 119-125, 147-152, 222-227, 286-291
🤖 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 `@shared/logger.py` around lines 94 - 100, The `_file_log_handler` helper is shadowing the Python built-in by using a parameter named `format`; rename that internal argument to `log_format` while keeping `setup_logger(..., format=...)` unchanged. Update every call site and reference in `shared/logger.py` that passes or reads this helper argument so the logging setup continues to work without Ruff warnings, including the related helper usages in the affected blocks.Source: Linters/SAST tools
shared/logger.py-115-124 (1)
115-124: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle basename-only log paths.
If
WEGENT_LOG_FILE_PATHisexecutor.log,os.path.dirname(log_file)is empty andos.makedirs("")raises before file logging starts.Proposed fix
if _FILE_HANDLER is None: - os.makedirs(os.path.dirname(log_file), exist_ok=True) + log_dir = os.path.dirname(log_file) + if log_dir: + os.makedirs(log_dir, exist_ok=True) max_bytes = _get_int_env("WEGENT_LOG_FILE_MAX_BYTES", 10 * 1024 * 1024)🤖 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 `@shared/logger.py` around lines 115 - 124, The file handler setup in shared/logger.py fails for basename-only paths because os.path.dirname(log_file) can be empty, causing os.makedirs("") to raise before RotatingFileHandler is created. Update the initialization logic around the _FILE_HANDLER setup to guard the directory creation so it only calls os.makedirs when the derived directory is non-empty, allowing paths like executor.log to work without errors.backend/tests/docker/test_executor_docker_contract.py-23-24 (1)
23-24: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winIgnore commented Dockerfile lines in the contract check.
A plain substring match would pass on
# ENV EXECUTOR_MODE=docker. Check normalized, non-comment instruction lines instead.Proposed fix
for dockerfile in dockerfiles: - assert "ENV EXECUTOR_MODE=docker" in dockerfile + instructions = { + line.strip() + for line in dockerfile.splitlines() + if line.strip() and not line.lstrip().startswith("#") + } + assert "ENV EXECUTOR_MODE=docker" in instructions🤖 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/tests/docker/test_executor_docker_contract.py` around lines 23 - 24, The contract check in test_executor_docker_contract.py is too permissive because it matches commented Dockerfile lines as well as real instructions. Update the dockerfile validation logic used in the loop over dockerfiles to normalize each line and ignore comments before checking for EXECUTOR_MODE, so the assertion only considers non-comment Dockerfile instruction lines rather than a raw substring match.shared/logger.py-108-113 (1)
108-113: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDetach the stale file handler before replacing it.
When the log path changes, the old
_FILE_HANDLERis closed but remains attached to any logger that already received it, so later records can still traverse a stale handler. Remove it from configured loggers before closing.Proposed fix
def _logger_has_handler(logger: logging.Logger, handler: logging.Handler) -> bool: return any(existing is handler for existing in logger.handlers) +def _remove_handler_from_loggers(handler: logging.Handler) -> None: + loggers = [logging.getLogger()] + for candidate in logging.Logger.manager.loggerDict.values(): + if isinstance(candidate, logging.Logger): + loggers.append(candidate) + + for current_logger in loggers: + if _logger_has_handler(current_logger, handler): + current_logger.removeHandler(handler) + + def _file_log_handler( @@ if _FILE_HANDLER is not None and _FILE_HANDLER_PATH != log_file: try: + _remove_handler_from_loggers(_FILE_HANDLER) _FILE_HANDLER.close() finally: _FILE_HANDLER = None🤖 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 `@shared/logger.py` around lines 108 - 113, When the log path changes, the stale _FILE_HANDLER in shared/logger.py is only closed but still attached to existing loggers, so update the handler swap logic around _FILE_HANDLER and _FILE_HANDLER_PATH to first remove the old handler from any configured logger(s) before closing it, then clear the globals and attach the new handler cleanly.executor/modes/local/runner.py-818-821 (1)
818-821: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRemove the new file handler from these loggers during cleanup too.
These logger names receive
file_handler, but_cleanup_file_logging()does not remove it from them. After shutdown/restart in the same process, those loggers can retain closed or duplicate handlers.Proposed cleanup alignment
logger_names = [ "local_runner", "websocket_client", "local_heartbeat", "local_handlers", + "local_app_ipc", + "local_command_handler", + "runtime_work_rpc_handler", + "codex_session_discovery", "task_executor", "executor.config.config_loader",🤖 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/modes/local/runner.py` around lines 818 - 821, The cleanup in _cleanup_file_logging() is missing the new file handler for the loggers configured in runner.py, so those loggers can retain closed or duplicate handlers across restart. Update the cleanup logic to remove the same file_handler from every logger that receives it during setup, including the loggers identified by local_app_ipc, local_command_handler, runtime_web_rpc_handler, and codex_session_discovery, using the existing cleanup path in _cleanup_file_logging() and the related logger setup code as the reference points.
🧹 Nitpick comments (6)
wework/src/features/local-runtime/LocalRuntimeInitializer.tsx (1)
27-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLocalize the executor status error strings.
localRuntimeErrorreturns hardcoded English messages that surface directly in the failed-state UI viastate.error, while the rest of this component is localized throught('localRuntime'...). Non-English users will see mixed-language output on the most actionable text.Consider returning a stable error key and mapping it through
t()in the render path.🤖 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/local-runtime/LocalRuntimeInitializer.tsx` around lines 27 - 32, The localRuntimeError helper is returning hardcoded English strings that flow into the UI through state.error, so make these status messages localization-friendly. Update localRuntimeError in LocalRuntimeInitializer to return a stable error key or status identifier instead of user-facing text, then map that value through t('localRuntime'...) in the render path where the failed state is displayed. Keep the existing status cases for status.error, running, and ready checks, but ensure the final shown message comes from the localization layer.wework/src/api/local/localServices.test.ts (1)
56-74: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: derive the fallback version from the shared constant.
executor_version: '1.8.5'duplicates theWEWORK_MIN_EXECUTOR_VERSIONfallback used inlocalDeviceFromStatus. Importing and asserting against the constant keeps this test from silently breaking when the minimum version is bumped.🤖 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.test.ts` around lines 56 - 74, The test for local device visibility hardcodes the fallback executor version, duplicating the value used by localDeviceFromStatus. Update the assertion in localServices.test.ts to import and reference WEWORK_MIN_EXECUTOR_VERSION instead of a literal so the expectation stays aligned with the shared fallback when the minimum version changes.wework/src/api/local/localChatStream.ts (1)
30-42: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd rejection handling for
deps.subscribe.The
void ... .then(...)chain has no.catch. Ifdeps.subscriberejects (e.g. the Tauri event listener fails to attach), this becomes an unhandled promise rejection and the subscription silently fails with no error surfaced to handlers.🛡️ Proposed fix
void deps .subscribe(event => { if (active) { emitResponseApiEvent(handlers, event.event, event.payload, state) } }) .then(unlisten => { if (active) { cleanup = unlisten return } unlisten() }) + .catch(error => { + handlers.onChatError?.({ + task_id: 0, + subtask_id: 0, + error: error instanceof Error ? error.message : String(error), + }) + })🤖 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/localChatStream.ts` around lines 30 - 42, The subscription setup in localChatStream’s deps.subscribe chain does not handle promise rejection, so failures to attach the Tauri listener can become unhandled and silent. Update the subscribe flow near emitResponseApiEvent to add rejection handling on the promise returned by deps.subscribe, and surface the error through the existing handlers/state path or equivalent error callback so the failure is visible instead of being swallowed.backend/app/core/constants.py (1)
35-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd docstrings to the new public room helpers.
These are public Python functions and should document the room format they return. As per coding guidelines, “include docstrings for all public functions and classes.”
🤖 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/core/constants.py` around lines 35 - 40, The new public helpers get_wework_task_room and get_wework_user_room need docstrings. Add brief docstrings to both functions in constants.py that describe the returned room string format, including that each prefixes the given ID with WEWORK_TASK_ROOM_PREFIX or WEWORK_USER_ROOM_PREFIX respectively.Source: Coding guidelines
executor/main.py (1)
112-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAnnotate the new startup helper boundaries.
_load_device_config_for_modelacks a return type, and_run_local_runner_with_app_ipcleaves both parameters untyped.Proposed fix
-def _load_device_config_for_mode(config_path: str | None): +def _load_device_config_for_mode(config_path: str | None) -> Any: @@ async def _run_local_runner_with_app_ipc( - runner, - app_ipc_server, + runner: Any, + app_ipc_server: Any, ) -> None: @@ -def _log_runner_task_result(task: asyncio.Task) -> None: +def _log_runner_task_result(task: asyncio.Task[Any]) -> None:As per coding guidelines,
**/*.py: “Use type hints for all function parameters and return values.”Also applies to: 198-201, 222-222
🤖 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/main.py` around lines 112 - 115, Annotate the new startup helper boundaries to match the type-hinting guideline: `_load_device_config_for_mode` should declare an explicit return type, and `_run_local_runner_with_app_ipc` should type both of its parameters. Use the existing symbols `_load_device_config_for_mode`, `_run_local_runner_with_app_ipc`, and `load_device_config` to update the function signatures without changing their behavior.Source: Coding guidelines
shared/logger.py (1)
179-184: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the return type for
_log_stream.The new helper is missing an explicit return annotation. As per coding guidelines,
**/*.py: “Use type hints for all function parameters and return values.”🤖 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 `@shared/logger.py` around lines 179 - 184, The helper _log_stream in logger.py is missing an explicit return type annotation, which violates the project’s type-hinting guidelines. Add a return annotation to _log_stream that reflects the stream object it returns, and keep the existing logic unchanged; this should be done directly on the function definition so the type hint is clear for both sys.stderr and sys.stdout.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 `@backend/app/api/ws/chat_namespace.py`:
- Around line 470-476: The task-room selection in the chat namespace is
origin-specific, but the leave path still uses only the legacy task room, which
can leave Wework sockets subscribed to stale streams. Update the task-room leave
logic in chat_namespace to reuse the same room-selection behavior as the join
path, using the client_origin check and get_wework_task_room / task:{task_id}
mapping consistently wherever task rooms are joined or left.
In `@docs/en/user-guide/ai-devices/local-device-support.md`:
- Line 226: The local-device-support documentation currently says the executor
connects to the remote Backend when only the backend URL is set, which is
incomplete. Update the sentence in the affected markdown section to state that
remote connection requires both the backend URL and auth token, using the same
paired condition for the environment variables and for the device config keys
(`WEGENT_BACKEND_URL` with `WEGENT_AUTH_TOKEN`, and `connection.backend_url`
with `connection.auth_token`).
In `@executor/main.py`:
- Around line 257-264: The runtime handler task created by
_attach_runtime_work_handler(app_ipc_server) is not being monitored like
backend_task, so startup failures can stay hidden while the IPC server continues
running. Add a done callback for runtime_handler_task, using the same
task-result logging pattern as _log_runner_task_result, so failures are surfaced
immediately before shutdown and the sidecar-critical task is handled
consistently.
In `@executor/modes/local/app_ipc.py`:
- Around line 235-237: The _prepare_socket_path() flow is deleting whatever
exists at socket_path when _remove_socket_path(force=True) is called, so a
misconfigured WEGENT_EXECUTOR_APP_IPC_SOCKET can unlink a regular file. Update
_prepare_socket_path() and the _remove_socket_path() logic in app_ipc.py to
verify the path is actually a stale Unix socket before removing it, and refuse
to unlink non-socket files; keep the existing socket cleanup behavior only for
socket_path entries that are sockets.
- Around line 124-128: The local IPC startup in app_ipc.start() uses
asyncio.start_unix_server(), which is Unix-only and has no Windows fallback. Add
an explicit platform gate around this Unix socket path or implement a
Windows-compatible transport for the Local Runner IPC, and make sure the
matching Rust client in local_executor.rs stays aligned with the chosen
transport (it is currently #[cfg(unix)] with UnixStream).
- Around line 584-589: In _attach_runtime_work_handler, a RuntimeWorkRpcHandler
is being started even though LocalRunner in _run_backend_runner already creates
and starts its own watcher, causing duplicate runtime-work discovery and event
fan-out. Remove the start_codex_watcher call from the IPC-side handler in
executor/modes/local/app_ipc.py and let the LocalRunner-owned handler remain the
single source of runtime tracking, or otherwise refactor _run_backend_runner to
reuse the server’s shared handler instead of creating a second one.
In `@wework/src-tauri/build.rs`:
- Around line 33-35: The build script currently only tracks the
WEWORK_EXECUTOR_SIDECAR env var, so a sidecar binary rebuilt in place can be
missed by Cargo. Update the configured_sidecar_source handling in build.rs to
emit a rerun-if-changed for the resolved source path before calling copy_sidecar
and remove_debug_marker, so Cargo reruns the script whenever that binary
changes. Use the existing configured_sidecar_source, copy_sidecar, and
remove_debug_marker flow to place the tracking in the same branch.
In `@wework/src-tauri/src/local_executor.rs`:
- Around line 742-748: The Local executor response path currently waits
indefinitely on receiver.recv() inside the async block, so add an explicit
timeout around the IPC receive in the local executor flow. Update the response
handling in the LocalExecutor/execute request path to use a bounded wait and
return a timeout error if no response arrives, ensuring the pending entry is
cleared instead of leaving the Tauri command hung forever.
- Around line 667-674: The non-Unix branch of start_executor_if_needed_unlocked
in local_executor.rs currently hard-fails, which conflicts with packaging the
app for all targets. Either add a Windows-compatible IPC path in this function
and any related LocalExecutorState handling, or change the tauri.conf.json
bundle.targets setting so unsupported platforms are excluded. Make sure the
executor startup logic and platform gating stay aligned so the app does not ship
a target that immediately fails at runtime.
- Around line 159-177: The app IPC path resolver in app_ipc_socket_path() does
not honor the executor’s tilde-expansion contract, so WEGENT_EXECUTOR_HOME and
the socket env var can be interpreted differently than the sidecar. Update
app_ipc_socket_path() to normalize the configured path by expanding a leading ~
before building the PathBuf, matching the behavior used by the executor-side
path resolution so both sides derive the same socket location.
- Around line 424-449: The ready-event setup in local_executor::start_execution
leaves a half-attached stream behind when handle_executor_line_inner or the
subsequent ready check fails, which can wedge retries. Update the startup path
around the state lock and ready-line handling to roll back the session on any
failure by clearing inner.stream and restoring inner.running/ready/error to a
safe idle state before returning Err. Use the existing local executor state
fields in inner and the generation/ready-line flow to ensure a failed startup
does not keep has_connected_stream() true.
In `@wework/src/features/workbench/WorkbenchProvider.tsx`:
- Around line 3047-3049: The runtime cancel flow in WorkbenchProvider only
handles the non-accepted ack case, but it does not catch when
runtimeWorkApi.cancelRuntimeTask rejects outright. Update the cancel path around
cancelRuntimeTask so rejected IPC/socket failures are caught and routed into the
same workbench error handling/state as other task failures, using the existing
WorkbenchProvider callback flow and state setters rather than letting the
promise bubble uncaught.
- Around line 509-517: The fallback routing logic in WorkbenchProvider currently
marks hasMultipleFallbacks on any second fallback, even when another workspace
resolves to the same {deviceId, localTaskId}. Update the fallback selection
block in WorkbenchProvider so it only treats routing as ambiguous when a
different task address is found; compare the full fallback address before
setting hasMultipleFallbacks, and keep a unique match as the recoverable
fallbackAddress.
In `@wework/src/features/workbench/workbenchReducer.ts`:
- Around line 169-172: The task remapping logic in workbenchReducer’s runtime
task selection is falling back to currentRuntimeTask when
findRuntimeTaskAddressByLocalTaskId returns null, which preserves a stale device
reference. Update the remapping path to return the lookup result directly (or
otherwise clear the selection) so that missing or ambiguous matches do not keep
pointing at a nonexistent device; use the currentRuntimeTask/localTaskId lookup
in this reducer to locate the change.
---
Minor comments:
In `@backend/tests/docker/test_executor_docker_contract.py`:
- Around line 23-24: The contract check in test_executor_docker_contract.py is
too permissive because it matches commented Dockerfile lines as well as real
instructions. Update the dockerfile validation logic used in the loop over
dockerfiles to normalize each line and ignore comments before checking for
EXECUTOR_MODE, so the assertion only considers non-comment Dockerfile
instruction lines rather than a raw substring match.
In `@docs/en/developer-guide/runtime-local-work.md`:
- Line 140: The local-first task creation description conflicts with the
empty-workspace flow, so reconcile the `workspacePath` requirement in the
runtime-local-work docs. Update the text around the local-first executionRequest
path and the “Project-backed” section to clearly state whether `workspacePath`
is strictly required or whether an empty/transient workspace context is valid,
and if valid, explain how that context satisfies the `runtime.tasks.create`
input without implying a hard failure.
In `@docs/zh/developer-guide/runtime-local-work.md`:
- Line 140: The description in the local-first workflow is too strict about
requiring workspacePath and conflicts with the empty-workspace path. Update the
wording in the runtime-local-work documentation section around the local-first
task creation flow to align with the empty-workspace behavior, using the
local-first and executionRequest descriptions as anchors, and remove the hard
failure requirement tied to workspacePath so the flow clearly supports creating
tasks with an empty workspace context.
In `@docs/zh/user-guide/ai-devices/local-device-support.md`:
- Line 228: The local-device support docs only mention WEGENT_BACKEND_URL and
connection.backend_url for remote Backend setup, but they also need to mention
the required authentication token. Update the explanation around executor
startup and Backend connection to include WEGENT_AUTH_TOKEN and
connection.auth_token, using the existing device configuration wording so
readers understand both URL and token must be set for local device mode to
connect successfully.
In `@executor/modes/local/runner.py`:
- Around line 818-821: The cleanup in _cleanup_file_logging() is missing the new
file handler for the loggers configured in runner.py, so those loggers can
retain closed or duplicate handlers across restart. Update the cleanup logic to
remove the same file_handler from every logger that receives it during setup,
including the loggers identified by local_app_ipc, local_command_handler,
runtime_web_rpc_handler, and codex_session_discovery, using the existing cleanup
path in _cleanup_file_logging() and the related logger setup code as the
reference points.
In `@shared/logger.py`:
- Around line 94-100: The `_file_log_handler` helper is shadowing the Python
built-in by using a parameter named `format`; rename that internal argument to
`log_format` while keeping `setup_logger(..., format=...)` unchanged. Update
every call site and reference in `shared/logger.py` that passes or reads this
helper argument so the logging setup continues to work without Ruff warnings,
including the related helper usages in the affected blocks.
- Around line 115-124: The file handler setup in shared/logger.py fails for
basename-only paths because os.path.dirname(log_file) can be empty, causing
os.makedirs("") to raise before RotatingFileHandler is created. Update the
initialization logic around the _FILE_HANDLER setup to guard the directory
creation so it only calls os.makedirs when the derived directory is non-empty,
allowing paths like executor.log to work without errors.
- Around line 108-113: When the log path changes, the stale _FILE_HANDLER in
shared/logger.py is only closed but still attached to existing loggers, so
update the handler swap logic around _FILE_HANDLER and _FILE_HANDLER_PATH to
first remove the old handler from any configured logger(s) before closing it,
then clear the globals and attach the new handler cleanly.
In `@wework/src-tauri/src/local_executor.rs`:
- Around line 66-73: The LocalExecutorChild::kill implementation only sends a
kill signal for the std::process::Child branch and does not reap the process,
which can leave zombie configured-sidecar processes. Update the Process variant
in kill(self) to also call wait() after kill(), mirroring the cleanup behavior
needed for restart/close paths, and keep the Tauri branch unchanged.
In `@wework/src/features/workbench/WorkbenchProvider.tsx`:
- Around line 1919-1925: `archiveChatConversations` is checking `runtimeWorkApi`
before handling an empty `addresses` array, which makes
`archiveChatConversations([])` surface a runtime error instead of no-oping.
Update the `archiveChatConversations` flow in `WorkbenchProvider` to return
immediately when `addresses.length === 0` before accessing
`resolvedServices.runtimeWorkApi`, and keep the existing runtime API error
dispatch only for non-empty requests that actually need it.
In `@wework/src/stream/responseApiStream.ts`:
- Around line 366-377: The errorMessage helper can return an empty string
because eventContent(payload) always yields a string, making the final fallback
unreachable. Update errorMessage in responseApiStream so it treats empty strings
as missing values by checking for non-empty text before returning, and ensure
the fallback 'Response stream error' is used when payload.error, data.message,
and errorRecord.message are absent or blank.
---
Nitpick comments:
In `@backend/app/core/constants.py`:
- Around line 35-40: The new public helpers get_wework_task_room and
get_wework_user_room need docstrings. Add brief docstrings to both functions in
constants.py that describe the returned room string format, including that each
prefixes the given ID with WEWORK_TASK_ROOM_PREFIX or WEWORK_USER_ROOM_PREFIX
respectively.
In `@executor/main.py`:
- Around line 112-115: Annotate the new startup helper boundaries to match the
type-hinting guideline: `_load_device_config_for_mode` should declare an
explicit return type, and `_run_local_runner_with_app_ipc` should type both of
its parameters. Use the existing symbols `_load_device_config_for_mode`,
`_run_local_runner_with_app_ipc`, and `load_device_config` to update the
function signatures without changing their behavior.
In `@shared/logger.py`:
- Around line 179-184: The helper _log_stream in logger.py is missing an
explicit return type annotation, which violates the project’s type-hinting
guidelines. Add a return annotation to _log_stream that reflects the stream
object it returns, and keep the existing logic unchanged; this should be done
directly on the function definition so the type hint is clear for both
sys.stderr and sys.stdout.
In `@wework/src/api/local/localChatStream.ts`:
- Around line 30-42: The subscription setup in localChatStream’s deps.subscribe
chain does not handle promise rejection, so failures to attach the Tauri
listener can become unhandled and silent. Update the subscribe flow near
emitResponseApiEvent to add rejection handling on the promise returned by
deps.subscribe, and surface the error through the existing handlers/state path
or equivalent error callback so the failure is visible instead of being
swallowed.
In `@wework/src/api/local/localServices.test.ts`:
- Around line 56-74: The test for local device visibility hardcodes the fallback
executor version, duplicating the value used by localDeviceFromStatus. Update
the assertion in localServices.test.ts to import and reference
WEWORK_MIN_EXECUTOR_VERSION instead of a literal so the expectation stays
aligned with the shared fallback when the minimum version changes.
In `@wework/src/features/local-runtime/LocalRuntimeInitializer.tsx`:
- Around line 27-32: The localRuntimeError helper is returning hardcoded English
strings that flow into the UI through state.error, so make these status messages
localization-friendly. Update localRuntimeError in LocalRuntimeInitializer to
return a stable error key or status identifier instead of user-facing text, then
map that value through t('localRuntime'...) in the render path where the failed
state is displayed. Keep the existing status cases for status.error, running,
and ready checks, but ensure the final shown message comes from the localization
layer.
🪄 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: 9cc513d8-0a0d-4464-b60f-8ecf890c1134
⛔ Files ignored due to path filters (1)
wework/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (76)
.gitignorebackend/app/api/endpoints/remote_devices.pybackend/app/api/ws/chat_namespace.pybackend/app/api/ws/device_namespace.pybackend/app/api/ws/local_task_responses.pybackend/app/core/constants.pybackend/tests/api/ws/test_chat_namespace_connect.pybackend/tests/api/ws/test_chat_namespace_task_join.pybackend/tests/api/ws/test_device_capabilities_state.pybackend/tests/docker/test_executor_docker_contract.pybackend/tests/docker/test_standalone_wework_contract.pydocker/device/Dockerfiledocker/executor/Dockerfiledocker/standalone/start.shdocs/en/developer-guide/local-device-architecture.mddocs/en/developer-guide/runtime-local-work.mddocs/en/user-guide/ai-devices/local-device-support.mddocs/superpowers/plans/2026-06-25-wework-local-first-app.mddocs/superpowers/specs/2026-06-25-wework-local-first-app-design.mddocs/zh/developer-guide/local-device-architecture.mddocs/zh/developer-guide/runtime-local-work.mddocs/zh/user-guide/ai-devices/local-device-support.mdexecutor/docs/LOCAL_MODE.mdexecutor/local.shexecutor/main.pyexecutor/modes/local/app_ipc.pyexecutor/modes/local/runner.pyexecutor/scripts/build_local.pyexecutor/scripts/local_executor_install.ps1executor/scripts/local_executor_install.shexecutor/tests/scripts/test_build_local.pyexecutor/tests/scripts/test_local_sh.pyexecutor/tests/test_local_app_ipc.pyfrontend/e2e/fixtures/claudecode-executor/Dockerfilepackages/chat-core/src/socket/authenticatedSocketClient.test.tspackages/chat-core/src/socket/authenticatedSocketClient.tsshared/logger.pywework/scripts/dev-mac-app.shwework/src-tauri/Cargo.tomlwework/src-tauri/build.rswework/src-tauri/capabilities/default.jsonwework/src-tauri/src/lib.rswework/src-tauri/src/local_executor.rswework/src-tauri/tauri.conf.jsonwework/src/App.apps.test.tsxwework/src/App.plugins.test.tsxwework/src/App.tsxwework/src/api/local/localChatStream.test.tswework/src/api/local/localChatStream.tswework/src/api/local/localServices.test.tswework/src/api/local/localServices.tswework/src/api/local/localSession.tswework/src/components/chat/MessageList.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/config/runtime.test.tswework/src/config/runtime.tswework/src/features/auth/AuthProvider.test.tsxwework/src/features/auth/AuthProvider.tsxwework/src/features/local-runtime/LocalRuntimeInitializer.test.tsxwework/src/features/local-runtime/LocalRuntimeInitializer.tsxwework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/features/workbench/workbenchReducer.test.tswework/src/features/workbench/workbenchReducer.tswework/src/i18n/index.tswework/src/i18n/locales/en/localRuntime.jsonwework/src/i18n/locales/zh-CN/localRuntime.jsonwework/src/pages/LoginPage.test.tsxwework/src/pages/PluginManagementPage.tsxwework/src/pages/PluginsPage.tsxwework/src/stream/chatStream.test.tswework/src/stream/chatStream.tswework/src/stream/responseApiStream.tswework/src/tauri/localExecutor.test.tswework/src/tauri/localExecutor.tswework/src/types/api.ts
💤 Files with no reviewable changes (10)
- wework/src/pages/PluginManagementPage.tsx
- docker/standalone/start.sh
- backend/tests/docker/test_standalone_wework_contract.py
- executor/scripts/local_executor_install.ps1
- executor/scripts/local_executor_install.sh
- wework/src/components/layout/DesktopWorkbenchLayout.tsx
- backend/app/api/endpoints/remote_devices.py
- executor/local.sh
- wework/src/pages/PluginsPage.tsx
- docker/device/Dockerfile
Comment on lines
+470
to
+476
| # Join only the stream room for this client origin. Wework uses the | ||
| # raw response.* stream; frontend keeps the legacy chat:* stream. | ||
| task_room = ( | ||
| get_wework_task_room(payload.task_id) | ||
| if session.get("client_origin") == CLIENT_ORIGIN_WEWORK | ||
| else f"task:{payload.task_id}" | ||
| ) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Mirror the origin-specific room when leaving task rooms.
This joins Wework clients to wework:task:{task_id}, but on_task_leave still leaves only task:{task_id} in the surrounding code, so Wework sockets can remain subscribed to stale task streams after leaving. Reuse the same room-selection logic for leave and any other task-room joins.
🤖 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/chat_namespace.py` around lines 470 - 476, The task-room
selection in the chat namespace is origin-specific, but the leave path still
uses only the legacy task room, which can leave Wework sockets subscribed to
stale streams. Update the task-room leave logic in chat_namespace to reuse the
same room-selection behavior as the join path, using the client_origin check and
get_wework_task_room / task:{task_id} mapping consistently wherever task rooms
are joined or left.
| ``` | ||
|
|
||
| The installer and first startup create `~/.wegent-executor/device-config.json`. Configuration priority is environment variables, device config, then defaults. On later startups, if `EXECUTOR_MODE`, `WEGENT_BACKEND_URL`, or `WEGENT_AUTH_TOKEN` is not set, the executor reads `mode`, `connection.backend_url`, and `connection.auth_token` from that file. | ||
| The installer and first startup create `~/.wegent-executor/device-config.json`. Configuration priority is environment variables, device config, then defaults. With no arguments and no remote address, the executor listens on `~/.wegent-executor/app-ipc.sock` and does not connect to Backend. After `WEGENT_BACKEND_URL` or `connection.backend_url` is set, the executor keeps the local socket available and also connects to the remote Backend as a local device. `~/.wegent-executor/app-ipc.lock` limits each user home to one executor process so the web app does not see duplicate device connections. Logs are written to `~/.wegent-executor/logs/executor.log`. |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat docs/en/user-guide/ai-devices/local-device-support.md | head -n 250Repository: wecode-ai/Wegent
Length of output: 15407
Mention the auth token requirement alongside the backend URL.
The sentence describing the remote connection condition currently implies that setting WEGENT_BACKEND_URL alone triggers the remote connection. It should explicitly state that both the backend URL and the auth token are required for the executor to connect to the remote Backend.
Relevant snippet context
... With no arguments and no remote address, the executor listens on `~/.wegent-executor/app-ipc.sock` and does not connect to Backend. After `WEGENT_BACKEND_URL` or `connection.backend_url` is set, the executor keeps the local socket available and also connects to the remote Backend as a local device. ...Update the condition to reflect that the remote connection relies on both the URL and the authentication token:
"After WEGENT_BACKEND_URL (and WEGENT_AUTH_TOKEN) or connection.backend_url (and connection.auth_token) are set..."
🤖 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 `@docs/en/user-guide/ai-devices/local-device-support.md` at line 226, The
local-device-support documentation currently says the executor connects to the
remote Backend when only the backend URL is set, which is incomplete. Update the
sentence in the affected markdown section to state that remote connection
requires both the backend URL and auth token, using the same paired condition
for the environment variables and for the device config keys
(`WEGENT_BACKEND_URL` with `WEGENT_AUTH_TOKEN`, and `connection.backend_url`
with `connection.auth_token`).
Comment on lines
+257
to
+264
| runtime_handler_task = asyncio.create_task( | ||
| _attach_runtime_work_handler(app_ipc_server) | ||
| ) | ||
|
|
||
| backend_task = asyncio.create_task( | ||
| _run_backend_runner(device_config, app_ipc_server.emit_event) | ||
| ) | ||
| backend_task.add_done_callback(_log_runner_task_result) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Log runtime handler startup failures immediately.
If _attach_runtime_work_handler() fails, the IPC socket can keep serving while runtime.* requests later fail as unavailable. Add a done callback so this sidecar-critical task is surfaced before shutdown.
Proposed fix
def _log_runner_task_result(task: asyncio.Task) -> None:
if task.cancelled():
return
@@
except Exception:
logger.exception("Local backend runner stopped unexpectedly")
+def _log_runtime_handler_task_result(task: asyncio.Task[Any]) -> None:
+ if task.cancelled():
+ return
+ try:
+ task.result()
+ except Exception:
+ logger.exception("Local app IPC runtime handler stopped unexpectedly")
+
+
async def _run_backend_runner(device_config: Any, app_event_emitter: Any) -> None:
@@
runtime_handler_task = asyncio.create_task(
_attach_runtime_work_handler(app_ipc_server)
)
+ runtime_handler_task.add_done_callback(_log_runtime_handler_task_result)
backend_task = asyncio.create_task(
_run_backend_runner(device_config, app_ipc_server.emit_event)📝 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
| runtime_handler_task = asyncio.create_task( | |
| _attach_runtime_work_handler(app_ipc_server) | |
| ) | |
| backend_task = asyncio.create_task( | |
| _run_backend_runner(device_config, app_ipc_server.emit_event) | |
| ) | |
| backend_task.add_done_callback(_log_runner_task_result) | |
| def _log_runtime_handler_task_result(task: asyncio.Task[Any]) -> None: | |
| if task.cancelled(): | |
| return | |
| try: | |
| task.result() | |
| except Exception: | |
| logger.exception("Local app IPC runtime handler stopped unexpectedly") | |
| runtime_handler_task = asyncio.create_task( | |
| _attach_runtime_work_handler(app_ipc_server) | |
| ) | |
| runtime_handler_task.add_done_callback(_log_runtime_handler_task_result) | |
| backend_task = asyncio.create_task( | |
| _run_backend_runner(device_config, app_ipc_server.emit_event) | |
| ) | |
| backend_task.add_done_callback(_log_runner_task_result) |
🤖 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/main.py` around lines 257 - 264, The runtime handler task created by
_attach_runtime_work_handler(app_ipc_server) is not being monitored like
backend_task, so startup failures can stay hidden while the IPC server continues
running. Add a done callback for runtime_handler_task, using the same
task-result logging pattern as _log_runner_task_result, so failures are surfaced
immediately before shutdown and the sidecar-critical task is handled
consistently.
Comment on lines
+124
to
+128
| self._server = await asyncio.start_unix_server( | ||
| self._handle_client, | ||
| path=str(self.socket_path), | ||
| ) | ||
| os.chmod(self.socket_path, 0o600) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether the local app IPC path has any Windows transport fallback.
rg -n -C3 'start_unix_server|open_unix|UnixStream|NamedPipe|named pipe|127\.0\.0\.1|WEGENT_EXECUTOR_APP_IPC_SOCKET' executor wework/src-tauriRepository: wecode-ai/Wegent
Length of output: 35232
Add a Windows-compatible IPC transport or gate this path.
asyncio.start_unix_server() is Unix-only in standard Python runtimes. The corresponding Rust client in wework/src-tauri/src/local_executor.rs is also gated by #[cfg(unix)] and uses UnixStream. Neither side implements a fallback mechanism (e.g., Named Pipes or TCP) that would allow the Local Runner sidecar to communicate on Windows. Without an explicit feature gate or Windows-compatible implementation, the application will fail to initialize the local executor mode on Windows.
https://github.com/wecode-ai/Wegent/blob/main/executor/modes/local/app_ipc.py#L124
https://github.com/wecode-ai/Wegent/blob/main/wework/src-tauri/src/local_executor.rs#L393
🤖 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/modes/local/app_ipc.py` around lines 124 - 128, The local IPC
startup in app_ipc.start() uses asyncio.start_unix_server(), which is Unix-only
and has no Windows fallback. Add an explicit platform gate around this Unix
socket path or implement a Windows-compatible transport for the Local Runner
IPC, and make sure the matching Rust client in local_executor.rs stays aligned
with the chosen transport (it is currently #[cfg(unix)] with UnixStream).
Comment on lines
+235
to
+237
| def _prepare_socket_path(self) -> None: | ||
| self.socket_path.parent.mkdir(parents=True, exist_ok=True) | ||
| self._remove_socket_path(force=True) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Refuse to unlink non-socket files before binding.
When WEGENT_EXECUTOR_APP_IPC_SOCKET points at an existing regular file, _prepare_socket_path() calls _remove_socket_path(force=True) and unlinks it without checking that it is a stale socket. That can delete arbitrary user files from a misconfigured override.
Proposed guard
import os
import shlex
+import stat
@@
def _remove_socket_path(self, *, force: bool = False) -> None:
expected_stat = self._socket_stat
if not force and expected_stat is None:
return
try:
- current_stat = self.socket_path.stat()
+ current_stat = self.socket_path.lstat()
+ if force and not stat.S_ISSOCK(current_stat.st_mode):
+ raise FileExistsError(
+ f"Refusing to remove non-socket app IPC path: {self.socket_path}"
+ )
if (
not forceAlso applies to: 286-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/modes/local/app_ipc.py` around lines 235 - 237, The
_prepare_socket_path() flow is deleting whatever exists at socket_path when
_remove_socket_path(force=True) is called, so a misconfigured
WEGENT_EXECUTOR_APP_IPC_SOCKET can unlink a regular file. Update
_prepare_socket_path() and the _remove_socket_path() logic in app_ipc.py to
verify the path is actually a stale Unix socket before removing it, and refuse
to unlink non-socket files; keep the existing socket cleanup behavior only for
socket_path entries that are sockets.
Comment on lines
+667
to
+674
| #[cfg(not(unix))] | ||
| async fn start_executor_if_needed_unlocked( | ||
| _app: tauri::AppHandle, | ||
| state: &LocalExecutorState, | ||
| ) -> Result<(), String> { | ||
| let message = "Local executor socket IPC is not available on this platform".to_string(); | ||
| set_executor_error(state, message.clone()); | ||
| Err(message) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the code in local_executor.rs around lines 667-674
sed -n '660,685p' wework/src-tauri/src/local_executor.rsRepository: wecode-ai/Wegent
Length of output: 871
🏁 Script executed:
#!/bin/bash
# Check the bundle targets in tauri.conf.json
cat wework/src-tauri/tauri.conf.json | jq '.bundle.targets'Repository: wecode-ai/Wegent
Length of output: 160
🏁 Script executed:
#!/bin/bash
# Search for usages of start_executor_if_needed to see if it's called in a way that handles the error
rg "start_executor_if_needed" wework/src-tauri/src/local_executor.rs --type rust -A 3 -B 3Repository: wecode-ai/Wegent
Length of output: 1513
Resolve the Windows packaging mismatch before shipping all bundle targets.
The local_executor.rs implementation at lines 667-674 explicitly returns an error on non-Unix platforms, yet tauri.conf.json is configured with bundle.targets: "all". This configuration will cause the application to build for Windows, but core executor-dependent functionality will fail immediately at runtime. Either implement Windows IPC or restrict the build targets to exclude non-supported platforms.
🤖 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-tauri/src/local_executor.rs` around lines 667 - 674, The non-Unix
branch of start_executor_if_needed_unlocked in local_executor.rs currently
hard-fails, which conflicts with packaging the app for all targets. Either add a
Windows-compatible IPC path in this function and any related LocalExecutorState
handling, or change the tauri.conf.json bundle.targets setting so unsupported
platforms are excluded. Make sure the executor startup logic and platform gating
stay aligned so the app does not ship a target that immediately fails at
runtime.
Comment on lines
+742
to
+748
| tauri::async_runtime::spawn_blocking(move || { | ||
| receiver | ||
| .recv() | ||
| .unwrap_or_else(|_| Err("Local executor disconnected".to_string())) | ||
| }) | ||
| .await | ||
| .map_err(|error| error.to_string())? |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Add a timeout for executor IPC responses.
receiver.recv() can block forever if the sidecar accepts a request but never sends a response. That leaves the Tauri command pending and keeps the request in pending until process/socket teardown.
🤖 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-tauri/src/local_executor.rs` around lines 742 - 748, The Local
executor response path currently waits indefinitely on receiver.recv() inside
the async block, so add an explicit timeout around the IPC receive in the local
executor flow. Update the response handling in the LocalExecutor/execute request
path to use a bounded wait and return a timeout error if no response arrives,
ensuring the pending entry is cleared instead of leaving the Tauri command hung
forever.
Comment on lines
+509
to
+517
| if (workspace.deviceId !== route.deviceId) { | ||
| if (fallbackAddress) { | ||
| hasMultipleFallbacks = true | ||
| } else { | ||
| fallbackAddress = { | ||
| deviceId: workspace.deviceId, | ||
| localTaskId: task.localTaskId, | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Only mark fallback routing ambiguous for a different task address.
Line 510 flips hasMultipleFallbacks for any second fallback, even if it is the same {deviceId, localTaskId} found through another workspace. That can make a uniquely recoverable legacy route fall back to the stale route.
Proposed fix
if (workspace.deviceId !== route.deviceId) {
- if (fallbackAddress) {
- hasMultipleFallbacks = true
+ const address = {
+ deviceId: workspace.deviceId,
+ localTaskId: task.localTaskId,
+ }
+ if (fallbackAddress) {
+ if (!isSameRuntimeTaskIdentity(fallbackAddress, address)) {
+ hasMultipleFallbacks = true
+ }
} else {
- fallbackAddress = {
- deviceId: workspace.deviceId,
- localTaskId: task.localTaskId,
- }
+ fallbackAddress = address
}
continue
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (workspace.deviceId !== route.deviceId) { | |
| if (fallbackAddress) { | |
| hasMultipleFallbacks = true | |
| } else { | |
| fallbackAddress = { | |
| deviceId: workspace.deviceId, | |
| localTaskId: task.localTaskId, | |
| } | |
| } | |
| if (workspace.deviceId !== route.deviceId) { | |
| const address = { | |
| deviceId: workspace.deviceId, | |
| localTaskId: task.localTaskId, | |
| } | |
| if (fallbackAddress) { | |
| if (!isSameRuntimeTaskIdentity(fallbackAddress, address)) { | |
| hasMultipleFallbacks = true | |
| } | |
| } else { | |
| fallbackAddress = address | |
| } | |
| continue | |
| } |
🤖 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 509 - 517,
The fallback routing logic in WorkbenchProvider currently marks
hasMultipleFallbacks on any second fallback, even when another workspace
resolves to the same {deviceId, localTaskId}. Update the fallback selection
block in WorkbenchProvider so it only treats routing as ambiguous when a
different task address is found; compare the full fallback address before
setting hasMultipleFallbacks, and keep a unique match as the recoverable
fallbackAddress.
Comment on lines
+3047
to
+3049
| const ack = await runtimeWorkApi.cancelRuntimeTask(state.currentRuntimeTask) | ||
|
|
||
| if (ack.error || ack.success === false) { | ||
| if (!ack.accepted) { |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Catch rejected runtime cancel calls.
Line 3047 only handles { accepted: false }; if cancelRuntimeTask rejects due to executor IPC/socket failure, the UI callback rejects without setting a workbench error.
Proposed fix
- const ack = await runtimeWorkApi.cancelRuntimeTask(state.currentRuntimeTask)
+ let ack
+ try {
+ ack = await runtimeWorkApi.cancelRuntimeTask(state.currentRuntimeTask)
+ } catch (error) {
+ dispatch({
+ type: 'error_set',
+ error: normalizeGuidanceError(error instanceof Error ? error.message : '取消当前回复失败'),
+ })
+ return
+ }
if (!ack.accepted) {📝 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 ack = await runtimeWorkApi.cancelRuntimeTask(state.currentRuntimeTask) | |
| if (ack.error || ack.success === false) { | |
| if (!ack.accepted) { | |
| let ack | |
| try { | |
| ack = await runtimeWorkApi.cancelRuntimeTask(state.currentRuntimeTask) | |
| } catch (error) { | |
| dispatch({ | |
| type: 'error_set', | |
| error: normalizeGuidanceError(error instanceof Error ? error.message : '取消当前回复失败'), | |
| }) | |
| return | |
| } | |
| if (!ack.accepted) { |
🤖 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 3047 -
3049, The runtime cancel flow in WorkbenchProvider only handles the non-accepted
ack case, but it does not catch when runtimeWorkApi.cancelRuntimeTask rejects
outright. Update the cancel path around cancelRuntimeTask so rejected IPC/socket
failures are caught and routed into the same workbench error handling/state as
other task failures, using the existing WorkbenchProvider callback flow and
state setters rather than letting the promise bubble uncaught.
Comment on lines
+169
to
+172
| return ( | ||
| findRuntimeTaskAddressByLocalTaskId(runtimeWork, currentRuntimeTask.localTaskId) ?? | ||
| currentRuntimeTask | ||
| ) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clear stale task addresses when remapping is unavailable.
After the current device disappears, findRuntimeTaskAddressByLocalTaskId already returns null for no match or ambiguous matches. Falling back to currentRuntimeTask keeps the UI pointed at a nonexistent device.
Proposed fix
- return (
- findRuntimeTaskAddressByLocalTaskId(runtimeWork, currentRuntimeTask.localTaskId) ??
- currentRuntimeTask
- )
+ return findRuntimeTaskAddressByLocalTaskId(runtimeWork, currentRuntimeTask.localTaskId)📝 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
| return ( | |
| findRuntimeTaskAddressByLocalTaskId(runtimeWork, currentRuntimeTask.localTaskId) ?? | |
| currentRuntimeTask | |
| ) | |
| return findRuntimeTaskAddressByLocalTaskId(runtimeWork, currentRuntimeTask.localTaskId) |
🤖 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 169 - 172,
The task remapping logic in workbenchReducer’s runtime task selection is falling
back to currentRuntimeTask when findRuntimeTaskAddressByLocalTaskId returns
null, which preserves a stale device reference. Update the remapping path to
return the lookup result directly (or otherwise clear the selection) so that
missing or ambiguous matches do not keep pointing at a nonexistent device; use
the currentRuntimeTask/localTaskId lookup in this reducer to locate the change.
This was referenced Jun 26, 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
1 participant
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
~/.wegent-executor/logs/executor.log, and can optionally connect to Backend when a remote address is configured.response.*events to Wework-specific rooms while keeping the legacychat:*compatibility stream isolated for Wegent frontend clients.EXECUTOR_MODE=docker, and update local device/executor docs for the packaged app process model.Validation
pnpm --dir wework test(86 files, 748 tests)pnpm --filter wework lintpnpm --dir wework exec tsc --noEmit --pretty falsepnpm --filter @wegent/chat-core test(5 files, 100 tests)cd backend && uv run pytest tests/api/ws/test_device_capabilities_state.py::test_response_like_event_name_routes_to_response_api_handler tests/api/ws/test_device_capabilities_state.py::test_device_status_broadcast_reaches_frontend_and_wework_rooms tests/api/ws/test_device_capabilities_state.py::test_responses_api_event_passes_raw_response_event_to_task_room tests/api/ws/test_device_capabilities_state.py::test_local_task_response_event_passes_raw_response_event_to_wework_clients tests/api/ws/test_device_capabilities_state.py::test_local_task_reasoning_event_emits_chat_chunk tests/api/ws/test_device_capabilities_state.py::test_local_task_tool_event_emits_block_created tests/api/ws/test_device_capabilities_state.py::test_local_task_streaming_tool_arguments_emit_generating_block tests/api/ws/test_chat_namespace_connect.py::test_wework_connect_joins_wework_user_room tests/api/ws/test_chat_namespace_task_join.py::test_wework_task_join_joins_wework_task_room(9 passed)cd backend && uv run pytest tests/docker/test_executor_docker_contract.py tests/docker/test_standalone_wework_contract.py(18 passed)cargo test --manifest-path wework/src-tauri/Cargo.toml local_executor(5 passed)cargo check --manifest-path wework/src-tauri/Cargo.tomlcd executor && uv run pytest(713 passed)WEWORK_DRY_RUN=1 pnpm --dir wework run dev:macgit diff --checkWEGENT_EXECUTOR_HOMEwith noWEGENT_BACKEND_URL,uv run python -m executor.main, connected toapp-ipc.sock, receivedexecutor.ready, anddevice.execute_command/pwdreturned success.AI_VERIFIED=1: Frontend ESLint, TypeScript, unit tests, Black, isort, Python syntax, settings config, and Alembic multi-head checks passed; backend/executor pytest were skipped by the hook per repository defaults.Summary by CodeRabbit