Repository navigation
feat(wework): stream direct chat through executor - #1478
Closed
icycrystal4 wants to merge 5 commits into
Closed
icycrystal4 wants to merge 5 commits into
icycrystal4 wants to merge 5 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughImplements end-to-end Wework direct chat: the local Executor hosts a Socket.IO ChangesWework Direct Chat — End-to-End
Sequence Diagram(s)sequenceDiagram
participant Wework
participant BackendAPI
participant DirectChatService
participant ExecutorDirectChatServer
participant LocalRunner
participant CallbackEndpoint
rect rgba(100, 150, 255, 0.5)
Note over Wework, CallbackEndpoint: Device Registration
ExecutorDirectChatServer->>BackendAPI: register_device (direct_chat capability)
BackendAPI-->>ExecutorDirectChatServer: direct_chat_secret + allowed_origins
end
rect rgba(100, 220, 150, 0.5)
Note over Wework, CallbackEndpoint: Connection Setup
Wework->>BackendAPI: POST /devices/{id}/direct-chat/connections
BackendAPI->>DirectChatService: create_connection (HMAC token)
DirectChatService-->>Wework: connection_id + token + endpoint
Wework->>ExecutorDirectChatServer: connect to /wework-chat (token auth)
end
rect rgba(255, 200, 100, 0.5)
Note over Wework, CallbackEndpoint: Turn Execution
Wework->>ExecutorDirectChatServer: chat:send
ExecutorDirectChatServer->>BackendAPI: POST /direct-chat/turns/prepare
BackendAPI->>DirectChatService: prepare_turn (context, RAG, task creation)
DirectChatService-->>ExecutorDirectChatServer: task_id + subtask_id + execution_request
ExecutorDirectChatServer->>LocalRunner: enqueue_direct_task
LocalRunner-->>Wework: chat:start / chat:chunk / chat:done (via DirectChatTransport)
LocalRunner->>CallbackEndpoint: terminal callback
CallbackEndpoint-->>LocalRunner: completed_at
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
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 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 |
icycrystal4
force-pushed
the
codex/wework-direct-chat
branch
from
June 19, 2026 08:12
957d17d to
379f31c
Compare
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![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: 3
🧹 Nitpick comments (4)
wework/src/stream/directWorkbenchSocket.ts (1)
47-48: ⚖️ Poor tradeoffUnbounded growth of task/subtask device mappings.
The
taskDevicesandsubtaskDevicesmaps accumulate entries indefinitely as new tasks and subtasks are processed. Over long sessions, this can lead to memory growth. Consider adding cleanup when tasks complete or implementing a size limit with LRU eviction.Also applies to: 292-306
🤖 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/directWorkbenchSocket.ts` around lines 47 - 48, The taskDevices and subtaskDevices Map instances defined in the class accumulate entries indefinitely without cleanup, causing unbounded memory growth. Add a cleanup mechanism that removes entries from these maps when tasks or subtasks complete, or alternatively implement a maximum size limit on these maps with LRU (Least Recently Used) eviction policy. Identify where task and subtask completion is handled (around lines 292-306 as indicated) and add corresponding delete operations for the entries from taskDevices and subtaskDevices maps when tasks finish execution.backend/app/api/endpoints/local_executor.py (1)
40-67: ⚡ Quick winMissing OpenTelemetry tracing on new endpoints.
Per coding guidelines, async backend functions should use
@trace_async()decorator. Both new endpoints lack tracing instrumentation.🔧 Proposed fix to add tracing
+from shared.telemetry.decorators import trace_async + `@router.post`( "/devices/{device_id}/direct-chat/connections", response_model=DirectChatConnectionResponse, ) +@trace_async() async def create_direct_chat_connection( device_id: str, db: Session = Depends(get_db), current_user: User = Depends(security.get_current_user), ) -> DirectChatConnectionResponse: """Authorize Wework to open a direct Socket.IO connection to one executor.""" return await direct_chat_service.create_connection( db=db, user=current_user, device_id=device_id, ) `@router.post`( "/direct-chat/turns/prepare", response_model=DirectChatTurnPrepareResponse, ) +@trace_async() async def prepare_direct_chat_turn( request: DirectChatTurnPrepareRequest, db: Session = Depends(get_db), current_user: User = Depends(security.get_current_user), ) -> DirectChatTurnPrepareResponse: """Persist a Wework direct-chat turn and return executor context.""" return await direct_chat_service.prepare_turn( db=db, user=current_user, request=request, )🤖 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/endpoints/local_executor.py` around lines 40 - 67, The two async endpoint functions create_direct_chat_connection and prepare_direct_chat_turn are missing the OpenTelemetry tracing decorator required by coding guidelines. Add the `@trace_async`() decorator to both functions, placing it above the existing `@router.post`() decorator for prepare_direct_chat_turn and above the function definition for create_direct_chat_connection, to enable proper tracing instrumentation on these new async endpoints.Source: Coding guidelines
backend/app/services/direct_chat_service.py (1)
49-121: ⚡ Quick winMissing OpenTelemetry tracing on async service methods.
Per coding guidelines, async backend functions should use
@trace_async()decorator.🔧 Proposed fix
+from shared.telemetry.decorators import trace_async + class DirectChatService: """Backend control plane for Wework-to-executor direct chat.""" + `@trace_async`() async def create_connection( self, *, db: Session, user: User, device_id: str, ) -> DirectChatConnectionResponse:Apply similarly to
prepare_turnmethod.🤖 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/services/direct_chat_service.py` around lines 49 - 121, The create_connection async method in the DirectChatService class is missing the OpenTelemetry tracing decorator required by coding guidelines. Add the `@trace_async`() decorator above the create_connection method definition to enable proper tracing instrumentation for this async service method, consistent with how other async methods like prepare_turn are decorated.Source: Coding guidelines
executor/modes/local/direct_chat.py (1)
571-576: 💤 Low valueConsider restricting CORS origins for the direct chat server.
cors_allowed_origins="*"allows any origin to connect. While this executor runs locally and is accessed by the desktop app, restricting origins (e.g., to specific localhost ports ornullfor file:// origins) would provide defense-in-depth against cross-origin attacks from malicious websites.🤖 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/direct_chat.py` around lines 571 - 576, The AsyncServer initialization in the direct_chat.py file has cors_allowed_origins set to "*" which allows connections from any origin, creating a potential security vulnerability. Replace the wildcard "*" with a restrictive list of allowed origins, such as specific localhost ports where the desktop app connects (e.g., localhost:3000) and null for file:// protocol origins if needed, instead of permitting all origins globally.
🤖 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/services/direct_chat_service.py`:
- Around line 139-149: The database query filtering for Team by Kind.id,
Kind.kind, and Kind.is_active is missing the required user_id filter as per
coding guidelines. Add a filter condition for Kind.user_id in the db.query
filter chain to include the current user's identifier (which should be available
as a parameter to this function, likely as current_user_id or similar). This
ensures the query properly validates that the team belongs to the authenticated
user before returning it.
In `@wework/src/stream/directWorkbenchSocket.ts`:
- Around line 59-68: The emit method contains a hardcoded error message in
Chinese ('连接未建立，请刷新页面重试') in the catch block, which violates the coding
guideline requiring all strings to be in English. Replace this Chinese error
message with an equivalent English error message that conveys the same meaning
about the connection not being established and the user needing to refresh.
- Around line 124-142: The error message in the emitAsync method uses Chinese
text that violates the English-only guideline for error messages. Replace the
Chinese error message '未找到可用设备' in the thrown Error with an appropriate English
equivalent that clearly describes the issue when a device ID cannot be resolved
and the event is not a task:join or task:leave event.
---
Nitpick comments:
In `@backend/app/api/endpoints/local_executor.py`:
- Around line 40-67: The two async endpoint functions
create_direct_chat_connection and prepare_direct_chat_turn are missing the
OpenTelemetry tracing decorator required by coding guidelines. Add the
`@trace_async`() decorator to both functions, placing it above the existing
`@router.post`() decorator for prepare_direct_chat_turn and above the function
definition for create_direct_chat_connection, to enable proper tracing
instrumentation on these new async endpoints.
In `@backend/app/services/direct_chat_service.py`:
- Around line 49-121: The create_connection async method in the
DirectChatService class is missing the OpenTelemetry tracing decorator required
by coding guidelines. Add the `@trace_async`() decorator above the
create_connection method definition to enable proper tracing instrumentation for
this async service method, consistent with how other async methods like
prepare_turn are decorated.
In `@executor/modes/local/direct_chat.py`:
- Around line 571-576: The AsyncServer initialization in the direct_chat.py file
has cors_allowed_origins set to "*" which allows connections from any origin,
creating a potential security vulnerability. Replace the wildcard "*" with a
restrictive list of allowed origins, such as specific localhost ports where the
desktop app connects (e.g., localhost:3000) and null for file:// protocol
origins if needed, instead of permitting all origins globally.
In `@wework/src/stream/directWorkbenchSocket.ts`:
- Around line 47-48: The taskDevices and subtaskDevices Map instances defined in
the class accumulate entries indefinitely without cleanup, causing unbounded
memory growth. Add a cleanup mechanism that removes entries from these maps when
tasks or subtasks complete, or alternatively implement a maximum size limit on
these maps with LRU (Least Recently Used) eviction policy. Identify where task
and subtask completion is handled (around lines 292-306 as indicated) and add
corresponding delete operations for the entries from taskDevices and
subtaskDevices maps when tasks finish execution.
🪄 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: aee9ea8e-7016-444e-9086-69b4af414e7f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
AGENTS.mdbackend/app/api/endpoints/internal/callback.pybackend/app/api/endpoints/local_executor.pybackend/app/api/ws/device_namespace.pybackend/app/schemas/device.pybackend/app/schemas/direct_chat.pybackend/app/services/device/local_provider.pybackend/app/services/device_service.pybackend/app/services/direct_chat_service.pydocs/en/developer-guide/wework-direct-chat.mddocs/zh/developer-guide/wework-direct-chat.mdexecutor/modes/local/direct_chat.pyexecutor/modes/local/runner.pyexecutor/modes/local/session_handler.pyexecutor/modes/local/websocket_client.pypackages/chat-core/src/workbench-message-reducer.test.tspackages/chat-core/src/workbench-message-reducer.tswework/src/components/chat/MessageList.test.tsxwework/src/components/chat/MessageList.tsxwework/src/components/chat/blocks/ToolBlocksDisplay.test.tsxwework/src/components/chat/blocks/ToolBlocksDisplay.tsxwework/src/features/auth/useAuth.tswework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/features/workbench/messageReducer.test.tswework/src/features/workbench/workbenchReducer.tswework/src/stream/chatStream.test.tswework/src/stream/chatStream.tswework/src/stream/directWorkbenchSocket.tswework/src/types/api.ts
✅ Files skipped from review due to trivial changes (2)
- AGENTS.md
- docs/zh/developer-guide/wework-direct-chat.md
🚧 Files skipped from review as they are similar to previous changes (20)
- wework/src/stream/chatStream.test.ts
- backend/app/services/device/local_provider.py
- backend/app/schemas/device.py
- backend/app/services/device_service.py
- backend/app/schemas/direct_chat.py
- wework/src/components/chat/MessageList.tsx
- wework/src/components/chat/MessageList.test.tsx
- wework/src/features/workbench/messageReducer.test.ts
- backend/app/api/endpoints/internal/callback.py
- wework/src/components/chat/blocks/ToolBlocksDisplay.test.tsx
- docs/en/developer-guide/wework-direct-chat.md
- executor/modes/local/runner.py
- wework/src/types/api.ts
- wework/src/features/workbench/WorkbenchProvider.test.tsx
- executor/modes/local/session_handler.py
- wework/src/stream/chatStream.ts
- packages/chat-core/src/workbench-message-reducer.test.ts
- wework/src/components/chat/blocks/ToolBlocksDisplay.tsx
- wework/src/features/workbench/WorkbenchProvider.tsx
- executor/modes/local/websocket_client.py
Comment on lines
+139
to
+149
| team = ( | ||
| db.query(Kind) | ||
| .filter( | ||
| Kind.id == payload.team_id, | ||
| Kind.kind == "Team", | ||
| Kind.is_active == True, | ||
| ) | ||
| .first() | ||
| ) | ||
| if not team: | ||
| raise HTTPException(status_code=404, detail="Team not found") |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how Team queries are handled elsewhere in the codebase
rg -n -A5 'Kind\.kind\s*==\s*"Team"' --type=py backend/Repository: wecode-ai/Wegent
Length of output: 1947
🌐 Web query:
wecode-ai Wegent direct_chat_service Team query authorization user_id filter
💡 Result:
In the Wegent platform, the direct chat service handles team communication and authorization through a combination of WebSocket-based session management and database-level filters [1][2]. When interacting with the direct chat service, authorization and user scoping are implemented as follows: 1. Authentication Context: Every request, whether via WebSocket or API, resolves the caller's identity (user_id) from the provided credentials, typically a JWT or API Key [1][3][4]. In WebSocket sessions (chat_namespace.py), the user_id is extracted directly from the session object [1]. 2. Authorization and Team Filtering: When a user attempts to interact with a team, the service performs an explicit authorization check [1][5]. For example, in WebSocket chat handling, the service queries the database for the team using filters that verify the team's identity (team_id), ensure it is of the correct kind ("Team"), and confirm it is active (is_active=True) [1]. 3. Resource Access: For chat tasks, the platform validates access permissions by checking whether the user is either the owner of the task or an approved member (ResourceMember) of the specific resource [5]. When a group chat member interacts with a task, the service verifies their membership status (MemberStatus.APPROVED) before allowing access to the task resources [5]. 4. API Access: The checkDirectChat API endpoint specifically checks a team's communication capabilities (supporting modes like sse, websocket, or http_callback) based on the team's configuration [6][2]. Access to these endpoints requires a valid Authorization bearer token [6][7]. Overall, the platform enforces security through a layered architecture where the entry point validates identity, and subsequent service-layer queries apply specific filters (user_id, team_id, and resource membership) to ensure users only access resources they are authorized to view or modify [5][4].
Citations:
- 1: https://github.com/wecode-ai/Wegent/blob/32e235c9/backend/app/api/ws/chat_namespace.py
- 2: https://github.com/wecode-ai/Wegent/blob/32e235c9/backend/app/api/endpoints/adapter/chat.py
- 3: https://github.com/wecode-ai/Wegent/blob/32e235c9/backend/app/core/security.py
- 4: https://deepwiki.com/wecode-ai/Wegent/3.7-authentication-and-security
- 5: https://github.com/wecode-ai/Wegent/blob/32e235c9/backend/app/services/chat/storage/task_manager.py
- 6: https://github.com/wecode-ai/Wegent/blob/32e235c9/frontend/src/apis/chat.ts
- 7: https://wecode-ai.github.io/wegent-docs/docs/reference/openapi-responses-api
🏁 Script executed:
# Try a different approach - search for any publicly available files or documentation
# Check if there's a GitHub-visible schema or architectural documentation
curl -s https://api.github.com/repos/wecode-ai/Wegent/contents/backend/app/services/ 2>/dev/null | head -100Repository: wecode-ai/Wegent
Length of output: 1917
Add required user_id filter to Team query for guideline compliance.
The query filters by Kind.id, Kind.kind, and Kind.is_active, but coding guidelines mandate that Kind resources must be filtered by all three identifying fields: namespace, name, and user_id. The missing user_id filter violates this requirement. Update the query to include .filter(..., Kind.user_id == current_user_id) or similar.
🧰 Tools
🪛 Ruff (0.15.17)
[error] 144-144: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
🤖 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/services/direct_chat_service.py` around lines 139 - 149, The
database query filtering for Team by Kind.id, Kind.kind, and Kind.is_active is
missing the required user_id filter as per coding guidelines. Add a filter
condition for Kind.user_id in the db.query filter chain to include the current
user's identifier (which should be available as a parameter to this function,
likely as current_user_id or similar). This ensures the query properly validates
that the team belongs to the authenticated user before returning it.
Source: Coding guidelines
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
backend/app/services/direct_chat_service.py (1)
167-198:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid committing RUNNING state before the execution request is built.
If
build_execution_request()fails after Line 175/Line 179, the assistant subtask and stream registry can remain RUNNING even though the executor never receives a request. Build the request first, then mark the stream as running, or add explicit cleanup on failure.🛠️ Proposed ordering fix
execution_request: Optional[dict[str, Any]] = None if should_trigger_ai and result.assistant_subtask: + request_payload = await build_execution_request( + task=result.task, + assistant_subtask=result.assistant_subtask, + team=team, + user=user, + message=payload.message, + payload=payload, + device_id=device_id, + user_subtask_id=( + result.user_subtask.id if result.user_subtask else None + ), + ) + execution_request = request_payload.to_dict() + subtask_store.update_fields( db, subtask=result.assistant_subtask, status=SubtaskStatus.RUNNING, executor_name=f"device-{device_id}", @@ user_id=user.id, username=user.user_name, ) - - request_payload = await build_execution_request( - task=result.task, - assistant_subtask=result.assistant_subtask, - team=team, - user=user, - message=payload.message, - payload=payload, - device_id=device_id, - user_subtask_id=( - result.user_subtask.id if result.user_subtask else None - ), - ) - execution_request = request_payload.to_dict()🤖 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/services/direct_chat_service.py` around lines 167 - 198, The subtask is being marked as RUNNING and committed to the database before the execution request is built via build_execution_request(), which means if that method fails, the subtask remains in a RUNNING state without an actual execution request being created. Reorder the code to call build_execution_request() before calling subtask_store.update_fields() and db.commit(), so the subtask status is only updated after the request is successfully constructed. This ensures the database is not left in an inconsistent state where a subtask is marked RUNNING but no executor will receive the request.executor/modes/local/runner.py (1)
101-101:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKey direct-chat transports by subtask, not just task.
task_ididentifies the workbench task/thread, while direct streams are subtask-specific. Concurrent or overlapping turns for the same task can overwrite_direct_task_transports[task_id], causing deltas/completion to be emitted through the wrongDirectChatTransport.Proposed fix
- self._direct_task_transports: Dict[int, Any] = {} + self._direct_task_transports: Dict[tuple[int, int], Any] = {} @@ async def enqueue_direct_task( self, task_data: ExecutionRequest, transport: Any, ) -> None: """Add a direct-chat task to the execution queue.""" - self._direct_task_transports[task_data.task_id] = transport + self._direct_task_transports[ + (task_data.task_id, task_data.subtask_id) + ] = transport await self.enqueue_task(task_data) @@ finally: self._running_tasks.pop(task_id, None) - self._direct_task_transports.pop(task_id, None) + self._direct_task_transports.pop( + (task_data.task_id, task_data.subtask_id), None + ) @@ - transport = self._direct_task_transports.get(task_id) + transport = self._direct_task_transports.get((task_id, subtask_id))Also applies to: 371-378, 572-573, 618-623
🤖 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` at line 101, The _direct_task_transports dictionary is currently keyed by task_id, but since multiple subtasks can exist for the same task and execute concurrently, using task_id alone causes different subtasks to overwrite each other's transport entries. Change the dictionary to be keyed by subtask_id instead of task_id. Update all usages of this dictionary throughout the file (the initialization at line 101, and all accesses/assignments at the locations marked as 371-378, 572-573, and 618-623) to use subtask_id as the key instead of task_id to ensure each subtask's DirectChatTransport is independently stored and retrieved.backend/app/api/ws/device_namespace.py (1)
1083-1118:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not bypass CRD upsert for direct-chat registrations.
When the recent-registration cache hits, this branch only reads the old
directChatSecretand skips persisting the currentpayload.direct_chat. A reconnect after a non-direct registration or a changed executor base URL can leavespec.directChatmissing/stale while registration still proceeds. Force the upsert path whenever the current payload includes direct chat.Proposed fix
- if persisted_display_name is None: + if persisted_display_name is None or payload.direct_chat: ( success, persisted_display_name, direct_chat_secret, 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 `@backend/app/api/ws/device_namespace.py` around lines 1083 - 1118, The issue is that when a recent registration is found in cache (persisted_display_name is not None), the elif block for payload.direct_chat only retrieves the old direct_chat_secret without persisting the current payload.direct_chat to the CRD. This causes changes to direct_chat data or reconnect scenarios to be missed. Modify the logic so that whenever payload.direct_chat is present, the full _register_device path is executed to persist the new direct_chat data, rather than bypassing the upsert when there's a cache hit. The condition should ensure the registration flow runs if payload.direct_chat is provided, regardless of whether persisted_display_name was cached.
🧹 Nitpick comments (7)
wework/src/components/projects/ProjectCreateDialog.test.tsx (1)
9-16: ⚡ Quick winCentralize the
directChattest fixture to avoid contract drift.The same
direct_chatobject is now duplicated across multiple test files; extracting a shared fixture/helper keeps capability contract updates in one place.As per coding guidelines, “never copy-paste code or write duplicate logic,” and based on learnings, “Long-term maintainability over short-term simplicity.”
Also applies to: 28-28, 39-39
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/projects/ProjectCreateDialog.test.tsx` around lines 9 - 16, The directChat test fixture is duplicated across multiple test files including ProjectCreateDialog.test.tsx. Create a centralized test fixture or helper file (e.g., in a shared test utilities directory) that exports the directChat object configuration, then update ProjectCreateDialog.test.tsx and all other test files that define this object to import it from the shared fixture instead of duplicating the definition. This ensures any updates to the directChat contract only need to be made in one place, reducing maintenance burden and preventing drift across test files.Sources: Coding guidelines, Learnings
wework/src/components/projects/ProjectCreateDialog.tsx (1)
755-818: ⚡ Quick winExtract unavailable-device rendering into a small helper/component.
This inline IIFE now combines status derivation, upgrade policy, and UI rendering in one block; moving it to a dedicated helper will reduce branching and make this path easier to test and evolve.
As per coding guidelines, “When a module is complex, prioritize extracting common logic into utilities or creating new modules rather than adding more complexity to the existing module,” and based on learnings, “Manage complexity through extraction.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/projects/ProjectCreateDialog.tsx` around lines 755 - 818, Extract the inline IIFE that renders the unavailable device (currently spanning from the device state derivations through the button with data-testid pattern project-device-unavailable and upgrade-project-device) into a separate helper component or function. Create a new component that accepts props including selectedUnavailableDevice, upgradingDevices, onUpgradeDevice, and handleUpgradeDevice, then encapsulate all the state derivations (upgradeRequired, upgrading, canUpgrade, reason), the conditional rendering logic, and the upgrade button rendering within this new helper. Replace the current inline IIFE with a call to this extracted component, passing the necessary props to maintain the same functionality while reducing complexity in the parent component.Sources: Coding guidelines, Learnings
wework/src/components/layout/DeviceStatusPrompt.tsx (1)
359-413: ⚡ Quick winExtract duplicated upgrade prompt rendering into a shared helper
The amber “upgrade required” banner/button markup is duplicated across two branches, which makes future behavior/text updates easy to diverge.
♻️ Suggested refactor shape
+function renderUpgradeRequiredBanner({ + message, + compact, + className, + manualPending, + upgradeDeviceIds, + onUpgrade, + actionLabel, +}: { + message: string + compact: boolean + className?: string + manualPending: boolean + upgradeDeviceIds: string[] + onUpgrade: (deviceIds: string[]) => void + actionLabel: string +}) { + return ( + <div className={cn( + 'flex items-center gap-2 rounded-lg border border-amber-300/60 bg-amber-50 px-3 py-2 text-xs text-amber-900', + compact && 'mx-0', + className, + )}> + <AlertCircle className="h-3.5 w-3.5 shrink-0 text-amber-600" /> + <span className="min-w-0 flex-1">{message}</span> + {upgradeDeviceIds.length > 0 && ( + <button + type="button" + disabled={manualPending} + onClick={() => onUpgrade(upgradeDeviceIds)} + className="inline-flex h-7 shrink-0 items-center gap-1 rounded-md px-2 font-medium text-amber-900 hover:bg-amber-100 disabled:cursor-not-allowed disabled:opacity-60" + > + {manualPending ? <Loader2 className="h-3.5 w-3.5 animate-spin" /> : <ArrowUpCircle className="h-3.5 w-3.5" />} + {actionLabel} + </button> + )} + </div> + ) +}As per coding guidelines, “When a module is complex, prioritize extracting common logic into utilities” and “never copy-paste code or write duplicate logic.” Based on learnings, “Manage complexity through extraction.”
Also applies to: 441-504
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/layout/DeviceStatusPrompt.tsx` around lines 359 - 413, The amber upgrade prompt banner markup with the AlertCircle icon, message span, and upgrade button is duplicated in both the sidebar-action presentation branch (in the renderSidebarAction call) and the default return statement. Extract this duplicated rendering logic into a shared helper function that accepts the necessary parameters (message, upgradeDeviceIds, manualPending, and the runUpgrades callback). Then call this helper from both branches to eliminate duplication and ensure consistent behavior and styling across different presentation modes.Sources: Coding guidelines, Learnings
wework/src/types/devices.ts (1)
29-36: ⚡ Quick winCentralize the direct-chat capability type.
DirectChatCapabilityis declared here and inwework/src/types/api.tswith the same contract. Move it to one shared type module and import/re-export it from both places to avoid drift in the direct socket contract.♻️ Suggested shape
+import type { DirectChatCapability } from './direct-chat' + export interface DeviceInfo { id: number device_id: string name: string @@ -} - -export interface DirectChatCapability { - enabled: boolean - transport: 'socket.io' - base_url: string - socket_path: string - namespace: string - version: number } + +export type { DirectChatCapability } from './direct-chat'// wework/src/types/direct-chat.ts export interface DirectChatCapability { enabled: boolean transport: 'socket.io' base_url: string socket_path: string namespace: string version: number }As per coding guidelines,
**/*.{ts,tsx,js,jsx,py}changes should "never copy-paste code or write duplicate logic" and should extract shared utilities immediately.🤖 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/types/devices.ts` around lines 29 - 36, The DirectChatCapability interface is duplicated across wework/src/types/devices.ts and wework/src/types/api.ts, which violates the no-duplication guideline and risks type drift. Create a new shared type module at wework/src/types/direct-chat.ts and move the DirectChatCapability interface definition there. Then replace the duplicate declarations in both devices.ts and api.ts with imports of this interface from the new shared module, ensuring both files re-export it for backward compatibility if needed.Source: Coding guidelines
executor/modes/local/direct_chat.py (1)
568-1047: 🏗️ Heavy liftSplit the direct-chat module to stay within the Python file-size limit.
executor/modes/local/direct_chat.pyis already 1,047 lines and combines backend HTTP client, transport translation, stream-state mutation, token auth, and Socket.IO routing. Extracting modules such asdirect_chat/backend_client.py,direct_chat/transport.py, anddirect_chat/server.pywill reduce coupling before more protocol behavior lands here.As per coding guidelines,
**/*.py: "File size limit: if a file exceeds 1000 lines, split it into multiple sub-modules"; based on learnings, "Manage complexity through extraction."🤖 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/direct_chat.py` around lines 568 - 1047, The DirectChatServer class and related components in executor/modes/local/direct_chat.py exceed the 1000-line file-size limit. Refactor the module by creating a direct_chat sub-package with the following structure: extract DirectChatBackendClient to direct_chat/backend_client.py, extract DirectChatTransport to direct_chat/transport.py, and move DirectChatServer along with its stream-state mutation methods to direct_chat/server.py. Keep shared types like DirectStreamState, DirectConnection, and DirectTaskContext in direct_chat/__init__.py or a dedicated types module, and update all imports throughout the codebase to reference the new module locations.Sources: Coding guidelines, Learnings
backend/app/core/config.py (1)
113-113: ⚡ Quick winName the direct-chat TTL constants.
12 * 60 * 60and300are now part of the ticket expiration contract; move them to named constants so the default and clamp stay aligned with docs/tests. As per coding guidelines,**/*.py: “Extract magic numbers to named constants.”Proposed refactor
+MIN_WEWORK_DIRECT_CHAT_TICKET_TTL_SECONDS = 300 +DEFAULT_WEWORK_DIRECT_CHAT_TICKET_TTL_SECONDS = 12 * 60 * 60 + class Settings(BaseSettings): @@ - WEWORK_DIRECT_CHAT_TICKET_TTL_SECONDS: int = 12 * 60 * 60 + WEWORK_DIRECT_CHAT_TICKET_TTL_SECONDS: int = ( + DEFAULT_WEWORK_DIRECT_CHAT_TICKET_TTL_SECONDS + ) @@ def validate_direct_chat_ticket_ttl(cls, v: int) -> int: """Validate direct chat ticket TTL.""" - return max(v, 300) + return max(v, MIN_WEWORK_DIRECT_CHAT_TICKET_TTL_SECONDS)Also applies to: 327-331
🤖 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/config.py` at line 113, Extract the magic numbers 12 * 60 * 60 and 300 from the direct-chat ticket TTL configuration into named constants. Create descriptive constant names that clearly indicate these represent time durations in seconds (for example, something like DIRECT_CHAT_TICKET_DEFAULT_TTL_SECONDS for the 12 * 60 * 60 value and another for 300), then replace all hardcoded occurrences of these magic numbers with references to the named constants throughout the file including at line 113 (WEWORK_DIRECT_CHAT_TICKET_TTL_SECONDS) and the other locations mentioned at lines 327-331, ensuring consistency across the codebase.Source: Coding guidelines
backend/app/api/ws/device_namespace.py (1)
411-490: 🏗️ Heavy liftMove cloud registration mutation out of the Socket.IO namespace.
_update_cloud_device_registration_syncperforms CRD mutation, display-name persistence, and secret generation in the API layer, duplicating service-layer device registration behavior while this file is already over 1000 lines. Put this behind a device service method and keep the namespace orchestration-only. As per coding guidelines,backend/**/*.py: business logic belongs inapp/services/;**/*.{ts,tsx,js,jsx,py}: avoid duplicate logic;**/*.py: split files over 1000 lines. Based on learnings, manage complexity through extraction.🤖 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/device_namespace.py` around lines 411 - 490, The function _update_cloud_device_registration_sync contains business logic for CRD mutation, display-name persistence, and secret generation that duplicates service-layer behavior and violates the architectural guideline of keeping business logic in app/services/. Extract the core business logic from this function into a new method in the device service (app/services/device/), then call this new service method from _update_cloud_device_registration_sync in the namespace file, keeping only the orchestration and namespace-specific logic in this file.Sources: Coding guidelines, 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/.env.example`:
- Line 27: The WEWORK_DIRECT_CHAT_ALLOWED_ORIGINS variable in the .env.example
file is currently using JSON-style array notation with quoted strings, but the
backend parser expects comma-separated values. Replace the JSON-style format
(with brackets and quotes) with a simple comma-separated list of the origins.
This will keep the example lint-clean and prevent dotenv-linter warnings about
quote characters.
In `@backend/app/api/ws/device_namespace.py`:
- Around line 1171-1175: The `direct_chat_secret` response field is being
returned whenever it exists, regardless of whether the current payload request
includes `payload.direct_chat`, which can expose direct-chat signing material to
non-direct registrations. Modify the condition that adds `direct_chat_secret` to
the response to also check `payload.direct_chat` in addition to checking if
`direct_chat_secret` exists, so that both direct-chat related fields are
properly gated on the `payload.direct_chat` flag.
- Line 243: Replace the SQLAlchemy boolean comparison pattern `Kind.is_active ==
True` with `Kind.is_active.is_(True)` to follow SQLAlchemy best practices and
clear the E712 lint error. This pattern needs to be updated in four locations
throughout the file: at the Kind.is_active comparison around line 243, and in
similar boolean comparisons at lines 300, 440, and 1800. Use SQLAlchemy's .is_()
method for all boolean column comparisons instead of the equality operator.
In `@backend/app/services/device/cloud_provider.py`:
- Line 196: The list_devices() method now includes the direct_chat field in its
payload, but the get_status() method in the same cloud provider class does not,
causing inconsistency when refreshing individual devices. Locate the
get_status() method and add the same direct_chat field with
spec.get("directChat") to its payload construction, matching the format and
structure used in list_devices() to ensure both methods return consistent device
capability information.
In `@executor/modes/local/websocket_client.py`:
- Around line 534-563: Refactor the direct chat registration response handling
to prevent stale values from masking validation issues and to avoid logging
sensitive data. Instead of directly assigning to self.direct_chat_secret and
self.direct_chat_allowed_origins within conditional blocks, first extract the
values from the response into local variables, validate those locals against the
self.direct_chat_endpoint flag, and only assign to the instance variables after
all validation passes. Additionally, ensure that any logging of the full
registration response redacts or excludes the direct_chat_secret field to
prevent leaking sensitive information in logs.
In `@wework/src/components/projects/ProjectCreateDialog.tsx`:
- Around line 659-663: The directoryDeviceUnavailableMessage assignment
currently prioritizes the upgrade-required check before considering offline
state, which causes devices that are both offline and upgrade-required to show
the upgrade message instead of the offline message. Reorder the ternary
condition to check the device's offline state first, and only if the device is
online should you then check isDeviceUpgradeRequiredForWeWork. This ensures
offline devices display the offline recovery message regardless of upgrade
requirements, consistent with the reason branch computed below in the same
component.
In `@wework/src/stream/directWorkbenchSocket.ts`:
- Around line 278-281: Replace the Chinese error message "连接设备超时" with an
English equivalent in the timeout error reject statement within the setTimeout
callback. The error message should clearly convey that a connection timeout has
occurred, maintaining consistency with the codebase coding guidelines that
require all error messages to be in English.
---
Outside diff comments:
In `@backend/app/api/ws/device_namespace.py`:
- Around line 1083-1118: The issue is that when a recent registration is found
in cache (persisted_display_name is not None), the elif block for
payload.direct_chat only retrieves the old direct_chat_secret without persisting
the current payload.direct_chat to the CRD. This causes changes to direct_chat
data or reconnect scenarios to be missed. Modify the logic so that whenever
payload.direct_chat is present, the full _register_device path is executed to
persist the new direct_chat data, rather than bypassing the upsert when there's
a cache hit. The condition should ensure the registration flow runs if
payload.direct_chat is provided, regardless of whether persisted_display_name
was cached.
In `@backend/app/services/direct_chat_service.py`:
- Around line 167-198: The subtask is being marked as RUNNING and committed to
the database before the execution request is built via
build_execution_request(), which means if that method fails, the subtask remains
in a RUNNING state without an actual execution request being created. Reorder
the code to call build_execution_request() before calling
subtask_store.update_fields() and db.commit(), so the subtask status is only
updated after the request is successfully constructed. This ensures the database
is not left in an inconsistent state where a subtask is marked RUNNING but no
executor will receive the request.
In `@executor/modes/local/runner.py`:
- Line 101: The _direct_task_transports dictionary is currently keyed by
task_id, but since multiple subtasks can exist for the same task and execute
concurrently, using task_id alone causes different subtasks to overwrite each
other's transport entries. Change the dictionary to be keyed by subtask_id
instead of task_id. Update all usages of this dictionary throughout the file
(the initialization at line 101, and all accesses/assignments at the locations
marked as 371-378, 572-573, and 618-623) to use subtask_id as the key instead of
task_id to ensure each subtask's DirectChatTransport is independently stored and
retrieved.
---
Nitpick comments:
In `@backend/app/api/ws/device_namespace.py`:
- Around line 411-490: The function _update_cloud_device_registration_sync
contains business logic for CRD mutation, display-name persistence, and secret
generation that duplicates service-layer behavior and violates the architectural
guideline of keeping business logic in app/services/. Extract the core business
logic from this function into a new method in the device service
(app/services/device/), then call this new service method from
_update_cloud_device_registration_sync in the namespace file, keeping only the
orchestration and namespace-specific logic in this file.
In `@backend/app/core/config.py`:
- Line 113: Extract the magic numbers 12 * 60 * 60 and 300 from the direct-chat
ticket TTL configuration into named constants. Create descriptive constant names
that clearly indicate these represent time durations in seconds (for example,
something like DIRECT_CHAT_TICKET_DEFAULT_TTL_SECONDS for the 12 * 60 * 60 value
and another for 300), then replace all hardcoded occurrences of these magic
numbers with references to the named constants throughout the file including at
line 113 (WEWORK_DIRECT_CHAT_TICKET_TTL_SECONDS) and the other locations
mentioned at lines 327-331, ensuring consistency across the codebase.
In `@executor/modes/local/direct_chat.py`:
- Around line 568-1047: The DirectChatServer class and related components in
executor/modes/local/direct_chat.py exceed the 1000-line file-size limit.
Refactor the module by creating a direct_chat sub-package with the following
structure: extract DirectChatBackendClient to direct_chat/backend_client.py,
extract DirectChatTransport to direct_chat/transport.py, and move
DirectChatServer along with its stream-state mutation methods to
direct_chat/server.py. Keep shared types like DirectStreamState,
DirectConnection, and DirectTaskContext in direct_chat/__init__.py or a
dedicated types module, and update all imports throughout the codebase to
reference the new module locations.
In `@wework/src/components/layout/DeviceStatusPrompt.tsx`:
- Around line 359-413: The amber upgrade prompt banner markup with the
AlertCircle icon, message span, and upgrade button is duplicated in both the
sidebar-action presentation branch (in the renderSidebarAction call) and the
default return statement. Extract this duplicated rendering logic into a shared
helper function that accepts the necessary parameters (message,
upgradeDeviceIds, manualPending, and the runUpgrades callback). Then call this
helper from both branches to eliminate duplication and ensure consistent
behavior and styling across different presentation modes.
In `@wework/src/components/projects/ProjectCreateDialog.test.tsx`:
- Around line 9-16: The directChat test fixture is duplicated across multiple
test files including ProjectCreateDialog.test.tsx. Create a centralized test
fixture or helper file (e.g., in a shared test utilities directory) that exports
the directChat object configuration, then update ProjectCreateDialog.test.tsx
and all other test files that define this object to import it from the shared
fixture instead of duplicating the definition. This ensures any updates to the
directChat contract only need to be made in one place, reducing maintenance
burden and preventing drift across test files.
In `@wework/src/components/projects/ProjectCreateDialog.tsx`:
- Around line 755-818: Extract the inline IIFE that renders the unavailable
device (currently spanning from the device state derivations through the button
with data-testid pattern project-device-unavailable and upgrade-project-device)
into a separate helper component or function. Create a new component that
accepts props including selectedUnavailableDevice, upgradingDevices,
onUpgradeDevice, and handleUpgradeDevice, then encapsulate all the state
derivations (upgradeRequired, upgrading, canUpgrade, reason), the conditional
rendering logic, and the upgrade button rendering within this new helper.
Replace the current inline IIFE with a call to this extracted component, passing
the necessary props to maintain the same functionality while reducing complexity
in the parent component.
In `@wework/src/types/devices.ts`:
- Around line 29-36: The DirectChatCapability interface is duplicated across
wework/src/types/devices.ts and wework/src/types/api.ts, which violates the
no-duplication guideline and risks type drift. Create a new shared type module
at wework/src/types/direct-chat.ts and move the DirectChatCapability interface
definition there. Then replace the duplicate declarations in both devices.ts and
api.ts with imports of this interface from the new shared module, ensuring both
files re-export it for backward compatibility if needed.
🪄 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: daebd589-32e4-494c-a690-0ad307709d67
📒 Files selected for processing (38)
backend/.env.examplebackend/app/api/ws/device_namespace.pybackend/app/core/config.pybackend/app/schemas/direct_chat.pybackend/app/services/device/cloud_provider.pybackend/app/services/device_service.pybackend/app/services/direct_chat_service.pydocs/en/developer-guide/wework-direct-chat.mddocs/zh/developer-guide/wework-direct-chat.mdexecutor/docs/LOCAL_MODE.mdexecutor/modes/local/direct_chat.pyexecutor/modes/local/handlers.pyexecutor/modes/local/runner.pyexecutor/modes/local/session_handler.pyexecutor/modes/local/websocket_client.pywework/src/components/chat/ChatInput.test.tsxwework/src/components/chat/composer/ProjectWorkBar.test.tsxwework/src/components/chat/composer/ProjectWorkBar.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/layout/DeviceStatusPrompt.test.tsxwework/src/components/layout/DeviceStatusPrompt.tsxwework/src/components/layout/MobileWorkbenchLayout.test.tsxwework/src/components/layout/MobileWorkbenchLayout.tsxwework/src/components/projects/ProjectCreateDialog.test.tsxwework/src/components/projects/ProjectCreateDialog.tsxwework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/lib/device-capabilities.test.tswework/src/lib/device-capabilities.tswework/src/lib/device-selection.test.tswework/src/lib/device-selection.tswework/src/stream/chatStream.tswework/src/stream/directWorkbenchSocket.tswework/src/types/api.tswework/src/types/devices.ts
💤 Files with no reviewable changes (1)
- backend/app/schemas/direct_chat.py
✅ Files skipped from review due to trivial changes (2)
- executor/docs/LOCAL_MODE.md
- docs/en/developer-guide/wework-direct-chat.md
🚧 Files skipped from review as they are similar to previous changes (4)
- backend/app/services/device_service.py
- executor/modes/local/session_handler.py
- wework/src/stream/chatStream.ts
- wework/src/features/workbench/WorkbenchProvider.test.tsx
|
|
||
| # Wework direct chat configuration | ||
| # Allowed origins are sent to local Executor during device registration. | ||
| WEWORK_DIRECT_CHAT_ALLOWED_ORIGINS=["http://127.0.0.1:1420","http://localhost:1420","tauri://127.0.0.1","tauri://localhost"] |
Contributor
There was a problem hiding this comment.
Use the comma-separated allowlist form in .env.example.
The backend parser accepts comma-separated values, and this JSON-style value is already flagged by dotenv-linter for quote characters. Keeping the example lint-clean avoids copy/paste warnings.
Proposed fix
-WEWORK_DIRECT_CHAT_ALLOWED_ORIGINS=["http://127.0.0.1:1420","http://localhost:1420","tauri://127.0.0.1","tauri://localhost"]
+WEWORK_DIRECT_CHAT_ALLOWED_ORIGINS=http://127.0.0.1:1420,http://localhost:1420,tauri://127.0.0.1,tauri://localhost📝 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
| WEWORK_DIRECT_CHAT_ALLOWED_ORIGINS=["http://127.0.0.1:1420","http://localhost:1420","tauri://127.0.0.1","tauri://localhost"] | |
| WEWORK_DIRECT_CHAT_ALLOWED_ORIGINS=http://127.0.0.1:1420,http://localhost:1420,tauri://127.0.0.1,tauri://localhost |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 27-27: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🤖 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/.env.example` at line 27, The WEWORK_DIRECT_CHAT_ALLOWED_ORIGINS
variable in the .env.example file is currently using JSON-style array notation
with quoted strings, but the backend parser expects comma-separated values.
Replace the JSON-style format (with brackets and quotes) with a simple
comma-separated list of the origins. This will keep the example lint-clean and
prevent dotenv-linter warnings about quote characters.
Source: Linters/SAST tools
| Kind.kind == "Device", | ||
| Kind.namespace == "default", | ||
| Kind.name == device_id, | ||
| Kind.is_active == True, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify the SQLAlchemy boolean comparison lint finding is gone.
uv run ruff check backend/app/api/ws/device_namespace.py --select E712Repository: wecode-ai/Wegent
Length of output: 103
🏁 Script executed:
cat -n backend/app/api/ws/device_namespace.py | sed -n '235,255p'Repository: wecode-ai/Wegent
Length of output: 823
🏁 Script executed:
#!/bin/bash
# Try to run ruff directly
which ruff && ruff check backend/app/api/ws/device_namespace.py --select E712
# Also search for other boolean comparisons in the file
echo "=== Other boolean comparisons in the file ==="
rg "(== True|== False)" backend/app/api/ws/device_namespace.py -nRepository: wecode-ai/Wegent
Length of output: 391
Use SQLAlchemy's boolean filter form here.
The code at line 243 uses Kind.is_active == True, which should use SQLAlchemy's .is_(True) method instead. This change clears the E712 lint error and follows SQLAlchemy best practices for boolean comparisons.
Note: The same pattern appears at lines 300, 440, and 1800 in this file and should also be updated.
Proposed fix
- Kind.is_active == True,
+ Kind.is_active.is_(True),📝 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
| Kind.is_active == True, | |
| Kind.is_active.is_(True), |
🧰 Tools
🪛 Ruff (0.15.17)
[error] 243-243: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
🤖 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/device_namespace.py` at line 243, Replace the SQLAlchemy
boolean comparison pattern `Kind.is_active == True` with
`Kind.is_active.is_(True)` to follow SQLAlchemy best practices and clear the
E712 lint error. This pattern needs to be updated in four locations throughout
the file: at the Kind.is_active comparison around line 243, and in similar
boolean comparisons at lines 300, 440, and 1800. Use SQLAlchemy's .is_() method
for all boolean column comparisons instead of the equality operator.
Source: Linters/SAST tools
| "client_ip": spec.get("clientIp"), | ||
| "cloud_config": spec.get("cloudConfig"), | ||
| "bind_shell": spec.get("bindShell", "claudecode"), | ||
| "direct_chat": spec.get("directChat"), |
Contributor
There was a problem hiding this comment.
Keep cloud list/detail payloads consistent.
list_devices() now exposes direct_chat, but get_status() in the same provider still omits it. Callers that refresh a single cloud device can lose the capability and incorrectly treat the device as upgrade-required.
Proposed fix
"client_ip": spec.get("clientIp"),
"cloud_config": spec.get("cloudConfig"),
"bind_shell": spec.get("bindShell", "claudecode"),
+ "direct_chat": spec.get("directChat"),
}🤖 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/services/device/cloud_provider.py` at line 196, The
list_devices() method now includes the direct_chat field in its payload, but the
get_status() method in the same cloud provider class does not, causing
inconsistency when refreshing individual devices. Locate the get_status() method
and add the same direct_chat field with spec.get("directChat") to its payload
construction, matching the format and structure used in list_devices() to ensure
both methods return consistent device capability information.
Comment on lines
+659
to
663
| const directoryDeviceUnavailableMessage = | ||
| selectedDevice && isDeviceUpgradeRequiredForWeWork(selectedDevice) | ||
| ? t('workbench.project_directory_upgrade_device_hint', '升级当前设备后可选择目录') | ||
| : t('workbench.project_directory_unavailable_device_hint', '设备恢复在线后可选择目录') | ||
|
|
Contributor
There was a problem hiding this comment.
Prioritize offline state before upgrade-required copy.
Devices that are both offline and upgrade-required currently show upgrade-first hints/details, which hides the immediate recovery action and conflicts with the offline-first reason branch already computed below.
💡 Suggested fix
- const directoryDeviceUnavailableMessage =
- selectedDevice && isDeviceUpgradeRequiredForWeWork(selectedDevice)
+ const directoryDeviceUnavailableMessage =
+ selectedDevice?.status === 'online' && isDeviceUpgradeRequiredForWeWork(selectedDevice)
? t('workbench.project_directory_upgrade_device_hint', '升级当前设备后可选择目录')
: t('workbench.project_directory_unavailable_device_hint', '设备恢复在线后可选择目录')
...
- {isDeviceUpgradeRequiredForWeWork(selectedUnavailableDevice)
+ {selectedUnavailableDevice.status === 'online' &&
+ isDeviceUpgradeRequiredForWeWork(selectedUnavailableDevice)
? t('workbench.project_selected_device_upgrade_title')
: t('workbench.project_selected_device_unavailable_title')}
...
- : upgradeRequired
+ : device.status === 'online' && upgradeRequired
? t('workbench.project_device_upgrade_detail')
: reason}
...
- {upgradeRequired && (
+ {device.status === 'online' && upgradeRequired && (Also applies to: 751-754, 757-801
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/components/projects/ProjectCreateDialog.tsx` around lines 659 -
663, The directoryDeviceUnavailableMessage assignment currently prioritizes the
upgrade-required check before considering offline state, which causes devices
that are both offline and upgrade-required to show the upgrade message instead
of the offline message. Reorder the ternary condition to check the device's
offline state first, and only if the device is online should you then check
isDeviceUpgradeRequiredForWeWork. This ensures offline devices display the
offline recovery message regardless of upgrade requirements, consistent with the
reason branch computed below in the same component.
icycrystal4
force-pushed
the
codex/wework-direct-chat
branch
from
June 19, 2026 16:37
0b48e3a to
3102d3c
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
executor/modes/local/websocket_client.py (1)
531-563:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRedact secret in logs and defer state assignment until after validation.
Two issues remain from the past review:
- Line 531 logs the full response which may contain
direct_chat_secret- Lines 536 and 541-545 assign to
self.direct_chat_secretandself.direct_chat_allowed_originsbefore the validation at lines 546-563. If validation fails and returns early, stale state from a previous registration persists.Proposed fix
response = await self.sio.call( "device:register", register_data, namespace="/local-executor", timeout=timeout, ) - logger.info(f"device:register response: {response}") + log_response = dict(response) if isinstance(response, dict) else response + if isinstance(log_response, dict) and "direct_chat_secret" in log_response: + log_response = {**log_response, "direct_chat_secret": "***"} + logger.info(f"device:register response: {log_response}") if response and response.get("success"): direct_chat_secret = response.get("direct_chat_secret") - if isinstance(direct_chat_secret, str) and direct_chat_secret: - self.direct_chat_secret = direct_chat_secret + next_secret = ( + direct_chat_secret + if isinstance(direct_chat_secret, str) and direct_chat_secret + else None + ) direct_chat_allowed_origins = response.get( "direct_chat_allowed_origins" ) + next_origins: list[str] = [] if isinstance(direct_chat_allowed_origins, list): - self.direct_chat_allowed_origins = [ + next_origins = [ str(origin).strip() for origin in direct_chat_allowed_origins if str(origin).strip() ] - if self.direct_chat_endpoint and not self.direct_chat_secret: + if self.direct_chat_endpoint and not next_secret: error = "Direct chat secret missing from registration response" logger.error(error) return DeviceRegistrationResult( success=False, backend_responded=True, error=error, ) - if self.direct_chat_endpoint and not self.direct_chat_allowed_origins: + if self.direct_chat_endpoint and not next_origins: error = ( "Direct chat CORS allowlist missing from registration response" ) logger.error(error) return DeviceRegistrationResult( success=False, backend_responded=True, error=error, ) + self.direct_chat_secret = next_secret + self.direct_chat_allowed_origins = next_origins self._registered = True🤖 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/websocket_client.py` around lines 531 - 563, The response being logged at the beginning of the validation block may contain the direct_chat_secret which is sensitive information that should not be exposed in logs. Additionally, the assignments to self.direct_chat_secret and self.direct_chat_allowed_origins are happening before the validation checks at lines 546-563, so if validation fails and returns early, stale state from a previous registration persists on the object. Fix this by creating a redacted copy of the response that excludes the direct_chat_secret key before logging it, and move the state assignments to self.direct_chat_secret and self.direct_chat_allowed_origins to occur only after all validation checks pass, just before returning a successful DeviceRegistrationResult.
🧹 Nitpick comments (4)
backend/tests/api/test_admin_password_bootstrap.py (1)
175-177: ⚡ Quick winModule-level teardown affects all tests in the file.
The
teardown_function()runs after every test in this module, potentially interfering with test isolation. Tests that don't monkeypatchCHECK_SYSTEM_INITIALIZATION_STATUSwill still have their settings reset.♻️ Prefer fixture-based cleanup for better isolation
+@pytest.fixture(autouse=False) +def reset_admin_password_state(): + """Reset admin password setup state after tests that modify it.""" + yield + settings.CHECK_SYSTEM_INITIALIZATION_STATUS = True + reset_admin_password_setup_state_cache() + + def test_current_user_handshake_returns_setup_required_from_startup_cache( test_client: TestClient, test_db: Session, + reset_admin_password_state, ):Then apply to only the tests that need cleanup:
test_current_user_handshake_skips_setup_status_when_check_disabledtest_admin_password_setup_status_respects_disabled_initialization_check🤖 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/api/test_admin_password_bootstrap.py` around lines 175 - 177, The `teardown_function()` module-level teardown runs after every test in the file, which interferes with test isolation by resetting settings even for tests that didn't modify them. Convert this module-level teardown into a pytest fixture with function scope and apply it only to the two tests that need cleanup: `test_current_user_handshake_skips_setup_status_when_check_disabled` and `test_admin_password_setup_status_respects_disabled_initialization_check`. Create a fixture that handles the cleanup of `CHECK_SYSTEM_INITIALIZATION_STATUS` and the cache reset, then have only those specific tests use this fixture to ensure other tests maintain proper isolation.packages/chat-core/src/workbench-message-reducer.ts (2)
124-129: 💤 Low valueDefensive guards against required field are redundant.
The type declares
createdAt: stringas required, soaction.createdAtis always defined. The?? message.createdAtfallback on line 124 and theif (!action.createdAt) return stateguard on line 129 are unreachable.If the intent is to allow optional
createdAtat runtime, consider changing the type tocreatedAt?: string. Otherwise, removing these guards improves clarity.♻️ Suggested simplification
status: 'streaming' as const, - createdAt: action.createdAt ?? message.createdAt, + createdAt: action.createdAt, } : message ) } - if (!action.createdAt) return state return [🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/chat-core/src/workbench-message-reducer.ts` around lines 124 - 129, The createdAt field is declared as required in the action type, making the defensive checks on lines 124 and 129 redundant. Either update the action type definition to make createdAt optional by changing createdAt: string to createdAt?: string if optional createdAt is intended, or remove both the nullish coalescing fallback (the ?? message.createdAt part) from the createdAt assignment and the if (!action.createdAt) return state guard to simplify the code since createdAt is guaranteed to be defined when the type is required.
158-158: 💤 Low valueSame redundant guard pattern.
Same issue as line 129 - the type requires
createdAt: string, so this guard is unreachable.🤖 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 `@packages/chat-core/src/workbench-message-reducer.ts` at line 158, Remove the redundant guard pattern checking if action.createdAt is falsy in the workbench-message-reducer.ts file. Since the TypeScript type definition requires createdAt to be a string (a non-optional field), the guard statement `if (!action.createdAt) return state` is unreachable and should be deleted to eliminate dead code. This mirrors the same redundant guard pattern that should have been removed elsewhere in the same file.wework/src/stream/chatStream.ts (1)
39-43: 💤 Low valueConsider typing device online/offline/status payloads for consistency.
onDeviceSlotUpdateandonDeviceUpgradeStatususe explicit payload types, whileonDeviceOnline,onDeviceOffline, andonDeviceStatususeunknown. If the payload shapes are defined elsewhere, consider importing and applying them for type safety and IDE autocompletion.🤖 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/chatStream.ts` around lines 39 - 43, The callback handlers onDeviceOnline, onDeviceOffline, and onDeviceStatus in the chatStream.ts file are using the generic unknown type for their payloads, while onDeviceSlotUpdate and onDeviceUpgradeStatus use explicit typed payloads (DeviceSlotUpdatePayload and DeviceUpgradeStatusPayload respectively). For consistency and better type safety, identify or define the appropriate payload types for the three generic handlers (onDeviceOnline, onDeviceOffline, onDeviceStatus), import them if they exist elsewhere in the codebase, and replace the unknown type annotations with their corresponding explicit payload types.
🤖 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/schemas/user.py`:
- Around line 177-182: The AdminPasswordSetupStatusResponse class in
backend/app/schemas/user.py is missing the admin_username field that the
frontend expects. Add a new field admin_username: str to the
AdminPasswordSetupStatusResponse class definition to match the frontend's
interface expectations and prevent runtime errors when the frontend tries to
access this field.
In `@backend/tests/api/test_admin_password_bootstrap.py`:
- Around line 90-116: The test assertions in
test_admin_password_setup_status_returns_current_bootstrap_state and
test_admin_password_setup_status_respects_disabled_initialization_check
currently only validate the `required` field in the response JSON. Once the
AdminPasswordSetupStatusResponse schema is updated to include the
`admin_username` field, update both response.json() assertions to validate the
complete response structure by including validation of both the `required` and
`admin_username` fields instead of only checking the `required` field.
In `@executor/modes/local/session_handler.py`:
- Around line 498-504: In the add_app_configurator method, add a guard clause to
check if self.gateway is not None before accessing self.gateway.is_running and
self.gateway.app_configurators. When gateway_enabled is False, self.gateway will
be None and accessing these attributes directly will raise an AttributeError.
Add a None check at the beginning of the method and raise an appropriate
RuntimeError with a descriptive message if the gateway is not available.
---
Duplicate comments:
In `@executor/modes/local/websocket_client.py`:
- Around line 531-563: The response being logged at the beginning of the
validation block may contain the direct_chat_secret which is sensitive
information that should not be exposed in logs. Additionally, the assignments to
self.direct_chat_secret and self.direct_chat_allowed_origins are happening
before the validation checks at lines 546-563, so if validation fails and
returns early, stale state from a previous registration persists on the object.
Fix this by creating a redacted copy of the response that excludes the
direct_chat_secret key before logging it, and move the state assignments to
self.direct_chat_secret and self.direct_chat_allowed_origins to occur only after
all validation checks pass, just before returning a successful
DeviceRegistrationResult.
---
Nitpick comments:
In `@backend/tests/api/test_admin_password_bootstrap.py`:
- Around line 175-177: The `teardown_function()` module-level teardown runs
after every test in the file, which interferes with test isolation by resetting
settings even for tests that didn't modify them. Convert this module-level
teardown into a pytest fixture with function scope and apply it only to the two
tests that need cleanup:
`test_current_user_handshake_skips_setup_status_when_check_disabled` and
`test_admin_password_setup_status_respects_disabled_initialization_check`.
Create a fixture that handles the cleanup of
`CHECK_SYSTEM_INITIALIZATION_STATUS` and the cache reset, then have only those
specific tests use this fixture to ensure other tests maintain proper isolation.
In `@packages/chat-core/src/workbench-message-reducer.ts`:
- Around line 124-129: The createdAt field is declared as required in the action
type, making the defensive checks on lines 124 and 129 redundant. Either update
the action type definition to make createdAt optional by changing createdAt:
string to createdAt?: string if optional createdAt is intended, or remove both
the nullish coalescing fallback (the ?? message.createdAt part) from the
createdAt assignment and the if (!action.createdAt) return state guard to
simplify the code since createdAt is guaranteed to be defined when the type is
required.
- Line 158: Remove the redundant guard pattern checking if action.createdAt is
falsy in the workbench-message-reducer.ts file. Since the TypeScript type
definition requires createdAt to be a string (a non-optional field), the guard
statement `if (!action.createdAt) return state` is unreachable and should be
deleted to eliminate dead code. This mirrors the same redundant guard pattern
that should have been removed elsewhere in the same file.
In `@wework/src/stream/chatStream.ts`:
- Around line 39-43: The callback handlers onDeviceOnline, onDeviceOffline, and
onDeviceStatus in the chatStream.ts file are using the generic unknown type for
their payloads, while onDeviceSlotUpdate and onDeviceUpgradeStatus use explicit
typed payloads (DeviceSlotUpdatePayload and DeviceUpgradeStatusPayload
respectively). For consistency and better type safety, identify or define the
appropriate payload types for the three generic handlers (onDeviceOnline,
onDeviceOffline, onDeviceStatus), import them if they exist elsewhere in the
codebase, and replace the unknown type annotations with their corresponding
explicit payload types.
🪄 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: d71264a9-6fe2-4bab-80f8-ada042aa813b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (58)
AGENTS.mdbackend/.env.examplebackend/app/api/endpoints/auth.pybackend/app/api/endpoints/internal/callback.pybackend/app/api/endpoints/local_executor.pybackend/app/api/ws/device_namespace.pybackend/app/core/config.pybackend/app/schemas/device.pybackend/app/schemas/direct_chat.pybackend/app/schemas/user.pybackend/app/services/device/cloud_provider.pybackend/app/services/device/local_provider.pybackend/app/services/device_service.pybackend/app/services/direct_chat_service.pybackend/tests/api/test_admin_password_bootstrap.pybackend/tests/api/ws/test_device_namespace_registration.pybackend/tests/api/ws/test_device_reconnect_storm.pydocs/en/developer-guide/wework-direct-chat.mddocs/zh/developer-guide/wework-direct-chat.mdexecutor/docs/LOCAL_MODE.mdexecutor/modes/local/direct_chat.pyexecutor/modes/local/handlers.pyexecutor/modes/local/runner.pyexecutor/modes/local/session_handler.pyexecutor/modes/local/websocket_client.pypackages/chat-core/src/workbench-message-reducer.test.tspackages/chat-core/src/workbench-message-reducer.tswework/package.jsonwework/src/components/chat/ChatInput.test.tsxwework/src/components/chat/MessageList.test.tsxwework/src/components/chat/MessageList.tsxwework/src/components/chat/blocks/ToolBlocksDisplay.test.tsxwework/src/components/chat/blocks/ToolBlocksDisplay.tsxwework/src/components/chat/composer/ProjectWorkBar.test.tsxwework/src/components/chat/composer/ProjectWorkBar.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/layout/DeviceStatusPrompt.test.tsxwework/src/components/layout/DeviceStatusPrompt.tsxwework/src/components/layout/MobileWorkbenchLayout.test.tsxwework/src/components/layout/MobileWorkbenchLayout.tsxwework/src/components/projects/ProjectCreateDialog.test.tsxwework/src/components/projects/ProjectCreateDialog.tsxwework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/features/workbench/messageReducer.test.tswework/src/features/workbench/workbenchReducer.tswework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/lib/device-capabilities.test.tswework/src/lib/device-capabilities.tswework/src/lib/device-selection.test.tswework/src/lib/device-selection.tswework/src/stream/chatStream.test.tswework/src/stream/chatStream.tswework/src/stream/directWorkbenchSocket.tswework/src/types/api.tswework/src/types/devices.ts
✅ Files skipped from review due to trivial changes (6)
- backend/app/services/device/cloud_provider.py
- AGENTS.md
- executor/docs/LOCAL_MODE.md
- backend/app/services/device/local_provider.py
- docs/zh/developer-guide/wework-direct-chat.md
- wework/src/i18n/locales/en/common.json
🚧 Files skipped from review as they are similar to previous changes (36)
- wework/src/components/chat/composer/ProjectWorkBar.test.tsx
- wework/src/components/chat/ChatInput.test.tsx
- wework/src/components/layout/DesktopWorkbenchMain.tsx
- backend/app/schemas/direct_chat.py
- wework/src/components/layout/MobileWorkbenchLayout.test.tsx
- wework/src/stream/chatStream.test.ts
- wework/src/components/projects/ProjectCreateDialog.test.tsx
- backend/app/schemas/device.py
- backend/app/services/device_service.py
- wework/src/lib/device-selection.test.ts
- wework/src/features/workbench/messageReducer.test.ts
- wework/src/types/devices.ts
- wework/src/types/api.ts
- backend/app/core/config.py
- wework/src/components/layout/MobileWorkbenchLayout.tsx
- wework/src/components/chat/blocks/ToolBlocksDisplay.test.tsx
- backend/app/api/endpoints/internal/callback.py
- wework/src/components/chat/MessageList.tsx
- wework/src/components/layout/DeviceStatusPrompt.tsx
- wework/src/components/layout/DeviceStatusPrompt.test.tsx
- wework/src/components/chat/composer/ProjectWorkBar.tsx
- wework/src/components/chat/MessageList.test.tsx
- packages/chat-core/src/workbench-message-reducer.test.ts
- executor/modes/local/runner.py
- wework/src/i18n/locales/zh-CN/common.json
- wework/src/stream/directWorkbenchSocket.ts
- wework/src/features/workbench/workbenchReducer.ts
- wework/src/lib/device-selection.ts
- wework/src/components/projects/ProjectCreateDialog.tsx
- wework/src/lib/device-capabilities.test.ts
- wework/src/components/layout/DesktopWorkbenchLayout.test.tsx
- wework/src/components/chat/blocks/ToolBlocksDisplay.tsx
- wework/src/lib/device-capabilities.ts
- wework/src/features/workbench/WorkbenchProvider.tsx
- docs/en/developer-guide/wework-direct-chat.md
- wework/src/features/workbench/WorkbenchProvider.test.tsx
Comment on lines
+177
to
+182
| class AdminPasswordSetupStatusResponse(BaseModel): | ||
| """Response body for the initial admin password setup status.""" | ||
|
|
||
| required: bool | ||
|
|
||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify frontend expects admin_username and find the admin username value
echo "=== Frontend interface definition and usage ==="
rg -n -A 2 'interface AdminPasswordSetupStatusResponse' --type=ts
echo -e "\n=== Frontend code accessing admin_username ==="
rg -n 'admin_username' --type=ts wework/src
echo -e "\n=== Backend default admin username ==="
rg -n "user_name.*admin" backend/app/core/yaml_init.pyRepository: wecode-ai/Wegent
Length of output: 882
Critical contract break: Frontend expects admin_username field.
The backend response schema only includes required: bool, but the frontend AdminPasswordSetupStatusResponse interface at wework/src/api/auth.ts:14-16 expects both required: boolean and admin_username: string. The frontend directly accesses this field at wework/src/pages/LoginPage.tsx:85, causing a runtime error.
🔧 Proposed fix to restore the missing field
class AdminPasswordSetupStatusResponse(BaseModel):
"""Response body for the initial admin password setup status."""
required: bool
+ admin_username: str📝 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
| class AdminPasswordSetupStatusResponse(BaseModel): | |
| """Response body for the initial admin password setup status.""" | |
| required: bool | |
| class AdminPasswordSetupStatusResponse(BaseModel): | |
| """Response body for the initial admin password setup status.""" | |
| required: bool | |
| admin_username: str |
🤖 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/schemas/user.py` around lines 177 - 182, The
AdminPasswordSetupStatusResponse class in backend/app/schemas/user.py is missing
the admin_username field that the frontend expects. Add a new field
admin_username: str to the AdminPasswordSetupStatusResponse class definition to
match the frontend's interface expectations and prevent runtime errors when the
frontend tries to access this field.
Comment on lines
+90
to
+116
| def test_admin_password_setup_status_returns_current_bootstrap_state( | ||
| test_client: TestClient, | ||
| test_db: Session, | ||
| ): | ||
| ensure_default_user(test_db) | ||
|
|
||
| response = test_client.get("/api/auth/admin-password/status") | ||
|
|
||
| assert response.status_code == 200 | ||
| assert response.json() == {"required": True} | ||
| assert get_cached_admin_password_setup_required() is True | ||
|
|
||
|
|
||
| def test_admin_password_setup_status_respects_disabled_initialization_check( | ||
| monkeypatch, | ||
| test_client: TestClient, | ||
| test_db: Session, | ||
| ): | ||
| monkeypatch.setattr(settings, "CHECK_SYSTEM_INITIALIZATION_STATUS", False) | ||
| ensure_default_user(test_db) | ||
|
|
||
| response = test_client.get("/api/auth/admin-password/status") | ||
|
|
||
| assert response.status_code == 200 | ||
| assert response.json() == {"required": False} | ||
| assert get_cached_admin_password_setup_required() is False | ||
|
|
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Tests need to validate admin_username field once schema is updated.
The tests correctly validate the required field behavior, but once admin_username is added back to AdminPasswordSetupStatusResponse (see comment on schema file), these assertions should be updated to verify the complete response structure.
📝 Suggested test assertions after schema fix
response = test_client.get("/api/auth/admin-password/status")
assert response.status_code == 200
- assert response.json() == {"required": True}
+ assert response.json() == {"required": True, "admin_username": "admin"}
assert get_cached_admin_password_setup_required() is True response = test_client.get("/api/auth/admin-password/status")
assert response.status_code == 200
- assert response.json() == {"required": False}
+ assert response.json() == {"required": False, "admin_username": "admin"}
assert get_cached_admin_password_setup_required() is False🤖 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/api/test_admin_password_bootstrap.py` around lines 90 - 116,
The test assertions in
test_admin_password_setup_status_returns_current_bootstrap_state and
test_admin_password_setup_status_respects_disabled_initialization_check
currently only validate the `required` field in the response JSON. Once the
AdminPasswordSetupStatusResponse schema is updated to include the
`admin_username` field, update both response.json() assertions to validate the
complete response structure by including validation of both the `required` and
`admin_username` fields instead of only checking the `required` field.
icycrystal4
force-pushed
the
codex/wework-direct-chat
branch
from
June 20, 2026 03:01
331fd69 to
b8b87e5
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
executor/modes/local/websocket_client.py (2)
531-531:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRedact
direct_chat_secretbefore logging the registration response.The full registration response is logged at line 531, which can include the
direct_chat_secret. This sensitive value should be redacted before logging to prevent credential leakage in logs.Proposed fix
- logger.info(f"device:register response: {response}") + log_response = dict(response) if isinstance(response, dict) else response + if isinstance(log_response, dict) and "direct_chat_secret" in log_response: + log_response["direct_chat_secret"] = "***" + logger.info("device:register response: %s", log_response)🤖 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/websocket_client.py` at line 531, The logger.info call that logs the device:register response is exposing the sensitive direct_chat_secret in plain text. Before logging the response at this location, create a copy of the response dictionary, remove or redact the direct_chat_secret field from the copy, and log only the redacted version instead of the full response object. This ensures sensitive credentials are not written to logs while still preserving other useful debugging information from the registration response.
534-569:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate direct chat response fields before assigning to instance state.
The code assigns
self.direct_chat_secret(line 536) andself.direct_chat_allowed_origins(lines 541-545) before validating that both are present when required (lines 546-563). If validation fails, instance state has already been partially updated, which can cause stale state from a previous successful registration to be overwritten with incomplete data from a failed one.Extract values to locals first, validate, then assign to
selfonly on success.Proposed fix
if response and response.get("success"): - direct_chat_secret = response.get("direct_chat_secret") - if isinstance(direct_chat_secret, str) and direct_chat_secret: - self.direct_chat_secret = direct_chat_secret - direct_chat_allowed_origins = response.get( + next_direct_chat_secret = response.get("direct_chat_secret") + next_direct_chat_secret = ( + next_direct_chat_secret + if isinstance(next_direct_chat_secret, str) and next_direct_chat_secret + else None + ) + next_direct_chat_allowed_origins = response.get( "direct_chat_allowed_origins" ) - if isinstance(direct_chat_allowed_origins, list): - self.direct_chat_allowed_origins = [ - str(origin).strip() - for origin in direct_chat_allowed_origins - if str(origin).strip() - ] - if self.direct_chat_endpoint and not self.direct_chat_secret: + if isinstance(next_direct_chat_allowed_origins, list): + next_direct_chat_allowed_origins = [ + str(o).strip() for o in next_direct_chat_allowed_origins if str(o).strip() + ] + else: + next_direct_chat_allowed_origins = [] + + if self.direct_chat_endpoint and not next_direct_chat_secret: error = "Direct chat secret missing from registration response" logger.error(error) return DeviceRegistrationResult( success=False, backend_responded=True, error=error, ) - if self.direct_chat_endpoint and not self.direct_chat_allowed_origins: + if self.direct_chat_endpoint and not next_direct_chat_allowed_origins: error = ( "Direct chat CORS allowlist missing from registration response" ) logger.error(error) return DeviceRegistrationResult( success=False, backend_responded=True, error=error, ) + + self.direct_chat_secret = next_direct_chat_secret + self.direct_chat_allowed_origins = next_direct_chat_allowed_origins self._registered = True🤖 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/websocket_client.py` around lines 534 - 569, The code assigns values to instance variables self.direct_chat_secret and self.direct_chat_allowed_origins before validating them, causing partial state updates even if validation fails. Extract the values from the response into local variables first instead of directly assigning to self, then perform all the validation checks on these local variables, and only assign to the instance variables self.direct_chat_secret and self.direct_chat_allowed_origins after all validations pass successfully. This ensures that instance state remains consistent and unchanged if any validation fails.
🧹 Nitpick comments (1)
executor/modes/local/direct_chat.py (1)
875-894: ⚖️ Poor tradeoffConsider adding OpenTelemetry tracing to key entry points.
Per coding guidelines, async functions in executor should use
@trace_async. Key candidates:_handle_chat_send,DirectChatTransport.send,_submit_terminal_callback. This improves production observability for the new direct chat flow.🤖 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/direct_chat.py` around lines 875 - 894, The _handle_chat_send async method is missing OpenTelemetry tracing instrumentation as required by coding guidelines for async entry points in the executor. Add the `@trace_async` decorator to the _handle_chat_send method definition to enable distributed tracing for this key entry point in the direct chat flow. Additionally, check if DirectChatTransport.send and _submit_terminal_callback methods also need this decorator applied as they are also key candidates for tracing as mentioned in the guidelines.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/endpoints/local_executor.py`:
- Around line 40-50: The async endpoint function create_direct_chat_connection
is missing the required OpenTelemetry tracing decorator. Add the `@trace_async`()
decorator above this function with appropriate parameters including span_name,
tracer_name, and extract_attributes to enable distributed tracing
instrumentation. Apply the same decorator pattern to the other async endpoint
function referenced in the comment range (lines 57-67) as both are async
functions in backend API endpoints and require this instrumentation per coding
guidelines.
In `@backend/app/services/direct_chat_service.py`:
- Around line 49-93: The async service methods create_connection and
prepare_turn in DirectChatService are missing OpenTelemetry tracing
instrumentation as required by coding guidelines. Add the `@trace_async` decorator
to both the create_connection method and the prepare_turn method to properly
instrument these async functions for tracing.
In `@executor/modes/local/direct_chat.py`:
- Around line 896-897: The code in the direct_chat.py file retrieves task_id
from the prepared dictionary and immediately casts it to int without validating
it exists, which causes a TypeError when task_id is None instead of gracefully
handling the error. Before calling int(task_id) in the await self._join_task
call, add a validation check to ensure task_id is not None. If task_id is
missing or None, return an appropriate error response to the client instead of
allowing the int() cast to fail.
---
Duplicate comments:
In `@executor/modes/local/websocket_client.py`:
- Line 531: The logger.info call that logs the device:register response is
exposing the sensitive direct_chat_secret in plain text. Before logging the
response at this location, create a copy of the response dictionary, remove or
redact the direct_chat_secret field from the copy, and log only the redacted
version instead of the full response object. This ensures sensitive credentials
are not written to logs while still preserving other useful debugging
information from the registration response.
- Around line 534-569: The code assigns values to instance variables
self.direct_chat_secret and self.direct_chat_allowed_origins before validating
them, causing partial state updates even if validation fails. Extract the values
from the response into local variables first instead of directly assigning to
self, then perform all the validation checks on these local variables, and only
assign to the instance variables self.direct_chat_secret and
self.direct_chat_allowed_origins after all validations pass successfully. This
ensures that instance state remains consistent and unchanged if any validation
fails.
---
Nitpick comments:
In `@executor/modes/local/direct_chat.py`:
- Around line 875-894: The _handle_chat_send async method is missing
OpenTelemetry tracing instrumentation as required by coding guidelines for async
entry points in the executor. Add the `@trace_async` decorator to the
_handle_chat_send method definition to enable distributed tracing for this key
entry point in the direct chat flow. Additionally, check if
DirectChatTransport.send and _submit_terminal_callback methods also need this
decorator applied as they are also key candidates for tracing as mentioned in
the guidelines.
🪄 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: d95b3018-5ec1-4221-8083-36b526afa02d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (55)
AGENTS.mdbackend/.env.examplebackend/app/api/endpoints/internal/callback.pybackend/app/api/endpoints/local_executor.pybackend/app/api/ws/device_namespace.pybackend/app/core/config.pybackend/app/schemas/device.pybackend/app/schemas/direct_chat.pybackend/app/services/device/cloud_provider.pybackend/app/services/device/local_provider.pybackend/app/services/device_service.pybackend/app/services/direct_chat_service.pybackend/tests/api/ws/test_device_namespace_registration.pybackend/tests/api/ws/test_device_reconnect_storm.pydocs/en/developer-guide/wework-direct-chat.mddocs/zh/developer-guide/wework-direct-chat.mdexecutor/docs/LOCAL_MODE.mdexecutor/modes/local/direct_chat.pyexecutor/modes/local/handlers.pyexecutor/modes/local/runner.pyexecutor/modes/local/session_handler.pyexecutor/modes/local/websocket_client.pypackages/chat-core/src/workbench-message-reducer.test.tspackages/chat-core/src/workbench-message-reducer.tswework/package.jsonwework/src/components/chat/ChatInput.test.tsxwework/src/components/chat/MessageList.test.tsxwework/src/components/chat/MessageList.tsxwework/src/components/chat/blocks/ToolBlocksDisplay.test.tsxwework/src/components/chat/blocks/ToolBlocksDisplay.tsxwework/src/components/chat/composer/ProjectWorkBar.test.tsxwework/src/components/chat/composer/ProjectWorkBar.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/layout/DeviceStatusPrompt.test.tsxwework/src/components/layout/DeviceStatusPrompt.tsxwework/src/components/layout/MobileWorkbenchLayout.test.tsxwework/src/components/layout/MobileWorkbenchLayout.tsxwework/src/components/projects/ProjectCreateDialog.test.tsxwework/src/components/projects/ProjectCreateDialog.tsxwework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/features/workbench/messageReducer.test.tswework/src/features/workbench/workbenchReducer.tswework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/lib/device-capabilities.test.tswework/src/lib/device-capabilities.tswework/src/lib/device-selection.test.tswework/src/lib/device-selection.tswework/src/stream/chatStream.test.tswework/src/stream/chatStream.tswework/src/stream/directWorkbenchSocket.tswework/src/types/api.tswework/src/types/devices.ts
✅ Files skipped from review due to trivial changes (9)
- wework/src/lib/device-selection.test.ts
- backend/app/schemas/device.py
- wework/src/components/chat/composer/ProjectWorkBar.test.tsx
- executor/docs/LOCAL_MODE.md
- AGENTS.md
- wework/src/i18n/locales/en/common.json
- wework/src/types/devices.ts
- docs/zh/developer-guide/wework-direct-chat.md
- docs/en/developer-guide/wework-direct-chat.md
🚧 Files skipped from review as they are similar to previous changes (38)
- backend/tests/api/ws/test_device_namespace_registration.py
- wework/package.json
- wework/src/lib/device-capabilities.test.ts
- backend/app/services/device/local_provider.py
- wework/src/stream/chatStream.test.ts
- wework/src/components/layout/MobileWorkbenchLayout.test.tsx
- backend/app/services/device/cloud_provider.py
- wework/src/i18n/locales/zh-CN/common.json
- wework/src/components/projects/ProjectCreateDialog.test.tsx
- wework/src/components/chat/ChatInput.test.tsx
- wework/src/types/api.ts
- backend/app/schemas/direct_chat.py
- wework/src/components/layout/DesktopWorkbenchMain.tsx
- wework/src/components/chat/MessageList.tsx
- wework/src/components/layout/MobileWorkbenchLayout.tsx
- wework/src/components/chat/composer/ProjectWorkBar.tsx
- backend/app/services/device_service.py
- wework/src/components/chat/MessageList.test.tsx
- wework/src/components/layout/DeviceStatusPrompt.test.tsx
- executor/modes/local/session_handler.py
- wework/src/components/layout/DeviceStatusPrompt.tsx
- wework/src/lib/device-selection.ts
- packages/chat-core/src/workbench-message-reducer.test.ts
- wework/src/features/workbench/messageReducer.test.ts
- backend/app/api/endpoints/internal/callback.py
- wework/src/features/workbench/workbenchReducer.ts
- wework/src/components/chat/blocks/ToolBlocksDisplay.tsx
- backend/app/core/config.py
- wework/src/lib/device-capabilities.ts
- wework/src/components/projects/ProjectCreateDialog.tsx
- wework/src/stream/chatStream.ts
- packages/chat-core/src/workbench-message-reducer.ts
- executor/modes/local/runner.py
- wework/src/components/chat/blocks/ToolBlocksDisplay.test.tsx
- wework/src/stream/directWorkbenchSocket.ts
- wework/src/components/layout/DesktopWorkbenchLayout.test.tsx
- wework/src/features/workbench/WorkbenchProvider.tsx
- wework/src/features/workbench/WorkbenchProvider.test.tsx
Comment on lines
+40
to
+50
| async def create_direct_chat_connection( | ||
| device_id: str, | ||
| db: Session = Depends(get_db), | ||
| current_user: User = Depends(security.get_current_user), | ||
| ) -> DirectChatConnectionResponse: | ||
| """Authorize Wework to open a direct Socket.IO connection to one executor.""" | ||
| return await direct_chat_service.create_connection( | ||
| db=db, | ||
| user=current_user, | ||
| device_id=device_id, | ||
| ) |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add OpenTelemetry tracing decorators to async endpoint functions.
Per coding guidelines, async functions in backend/**/*.py require @trace_async(span_name, tracer_name, extract_attributes) decorators. Both endpoint functions are missing this instrumentation.
Proposed fix
+from app.core.tracing import trace_async
+
`@router.post`(
"/devices/{device_id}/direct-chat/connections",
response_model=DirectChatConnectionResponse,
)
+@trace_async("create_direct_chat_connection", "local_executor")
async def create_direct_chat_connection(
device_id: str,
db: Session = Depends(get_db),
current_user: User = Depends(security.get_current_user),
) -> DirectChatConnectionResponse: `@router.post`(
"/direct-chat/turns/prepare",
response_model=DirectChatTurnPrepareResponse,
)
+@trace_async("prepare_direct_chat_turn", "local_executor")
async def prepare_direct_chat_turn(
request: DirectChatTurnPrepareRequest,
db: Session = Depends(get_db),
current_user: User = Depends(security.get_current_user),
) -> DirectChatTurnPrepareResponse:Also applies to: 57-67
🧰 Tools
🪛 Ruff (0.15.17)
[warning] 42-42: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
[warning] 43-43: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🤖 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/endpoints/local_executor.py` around lines 40 - 50, The async
endpoint function create_direct_chat_connection is missing the required
OpenTelemetry tracing decorator. Add the `@trace_async`() decorator above this
function with appropriate parameters including span_name, tracer_name, and
extract_attributes to enable distributed tracing instrumentation. Apply the same
decorator pattern to the other async endpoint function referenced in the comment
range (lines 57-67) as both are async functions in backend API endpoints and
require this instrumentation per coding guidelines.
Source: Coding guidelines
Comment on lines
+49
to
+93
| async def create_connection( | ||
| self, | ||
| *, | ||
| db: Session, | ||
| user: User, | ||
| device_id: str, | ||
| ) -> DirectChatConnectionResponse: | ||
| """Authorize one Wework direct Socket.IO connection on a local executor.""" | ||
|
|
||
| device = device_service.get_device_by_device_id(db, user.id, device_id) | ||
| if not device: | ||
| raise HTTPException(status_code=404, detail="Device not found") | ||
|
|
||
| direct_chat = self._get_direct_chat_capability(device) | ||
| if not direct_chat.enabled: | ||
| raise HTTPException( | ||
| status_code=409, | ||
| detail="Device does not expose direct chat", | ||
| ) | ||
|
|
||
| direct_chat_secret = self._get_direct_chat_secret(device) | ||
|
|
||
| connection_id = f"dc_{secrets.token_urlsafe(16)}" | ||
| expires_at = datetime.now(timezone.utc) + timedelta( | ||
| seconds=settings.WEWORK_DIRECT_CHAT_TICKET_TTL_SECONDS | ||
| ) | ||
| token = self._sign_direct_chat_token( | ||
| secret=direct_chat_secret, | ||
| payload={ | ||
| "connection_id": connection_id, | ||
| "user_id": user.id, | ||
| "user_name": user.user_name, | ||
| "device_id": device_id, | ||
| "exp": int(expires_at.timestamp()), | ||
| }, | ||
| ) | ||
|
|
||
| return DirectChatConnectionResponse( | ||
| connection_id=connection_id, | ||
| token=token, | ||
| device_id=device_id, | ||
| expires_at=expires_at, | ||
| endpoint=direct_chat, | ||
| ) | ||
|
|
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add OpenTelemetry tracing decorators to async service methods.
Per coding guidelines, async functions in backend/**/*.py require @trace_async decorators. Both create_connection and prepare_turn are missing instrumentation.
Proposed fix
+from app.core.tracing import trace_async
+
class DirectChatService:
"""Backend control plane for Wework-to-executor direct chat."""
+ `@trace_async`("direct_chat_create_connection", "direct_chat_service")
async def create_connection(
self,
*,
db: Session,
user: User,
device_id: str,
) -> DirectChatConnectionResponse:+ `@trace_async`("direct_chat_prepare_turn", "direct_chat_service")
async def prepare_turn(
self,
*,
db: Session,
user: User,
request: DirectChatTurnPrepareRequest,
) -> DirectChatTurnPrepareResponse:Also applies to: 94-233
🤖 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/services/direct_chat_service.py` around lines 49 - 93, The async
service methods create_connection and prepare_turn in DirectChatService are
missing OpenTelemetry tracing instrumentation as required by coding guidelines.
Add the `@trace_async` decorator to both the create_connection method and the
prepare_turn method to properly instrument these async functions for tracing.
Source: Coding guidelines
Comment on lines
+896
to
+897
| task_id = prepared.get("task_id") | ||
| await self._join_task(sid, int(task_id)) |
Contributor
There was a problem hiding this comment.
Validate task_id before casting to int.
If the backend response omits task_id, int(None) raises TypeError crashing the handler instead of returning an error to the client.
Suggested fix
task_id = prepared.get("task_id")
+ if not task_id:
+ return {"success": False, "error": "Backend prepare response missing task_id"}
await self._join_task(sid, int(task_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
| task_id = prepared.get("task_id") | |
| await self._join_task(sid, int(task_id)) | |
| task_id = prepared.get("task_id") | |
| if not task_id: | |
| return {"success": False, "error": "Backend prepare response missing task_id"} | |
| await self._join_task(sid, int(task_id)) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@executor/modes/local/direct_chat.py` around lines 896 - 897, The code in the
direct_chat.py file retrieves task_id from the prepared dictionary and
immediately casts it to int without validating it exists, which causes a
TypeError when task_id is None instead of gracefully handling the error. Before
calling int(task_id) in the await self._join_task call, add a validation check
to ensure task_id is not None. If task_id is missing or None, return an
appropriate error response to the client instead of allowing the int() cast to
fail.
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
/chatsocket.direct_chat_secretanddirect_chat_allowed_origins.expires_at, and keep reconnecting when executors start after Wework is already open.Validation
cd backend && uv run python -m py_compile app/core/config.py app/api/ws/device_namespace.py app/services/direct_chat_service.py app/services/device_service.py app/services/device/cloud_provider.py app/schemas/direct_chat.pycd executor && uv run python -m py_compile config/config.py modes/local/direct_chat.py modes/local/runner.py modes/local/session_handler.py modes/local/websocket_client.py modes/local/handlers.pypnpm --dir wework lintpnpm --dir wework buildpnpm --dir wework testgit diff --check./start.sh --restart backendhealth check passed after updatingbackend/.envlocally.Notes
WEWORK_DIRECT_CHAT_ALLOWED_ORIGINSis configured as a JSON array inbackend/.env.examplebecause pydantic-settings parseslist[str]env values as JSON before validators run.WEWORK_DIRECT_CHAT_TICKET_TTL_SECONDSdefaults to 12 hours and is clamped to at least 300 seconds.backend/.envwas updated locally for the dev environment but is ignored by git; committed defaults are inbackend/.env.exampleand Backend settings.Summary by CodeRabbit
New Features
Bug Fixes
Documentation