Repository navigation
feat(wework): 手动输入文件路径和分支的新建、切换 - #1256
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds backend device commands (mkdir_p and git helpers), a device createDirectory API, environment branch helpers and validation, workbench context callbacks, UI for branch selection/creation and folder creation in project dialog, component wiring, tests, and i18n keys. ChangesLocal Device and Environment Operations
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
…-061904 # Conflicts: # wework/src/api/adapters.test.ts # wework/src/features/workbench/WorkbenchProvider.tsx
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wework/src/components/projects/ProjectCreateDialog.tsx (1)
627-648:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing error handling for project creation.
The submit handler lacks a
catchblock, so ifonCreateProjectthrows, the user receives no feedback. This contrasts withhandleCreateDirectorywhich properly catches and displays errors.Suggested fix
onClick={async () => { setSubmitting(true) try { await onCreateProject({ name: finalProjectName, description: '', config: { mode: 'workspace', execution: { targetType: 'local', deviceId, }, workspace: { source: 'local_path', localPath: finalPath, }, }, }) onClose() + } catch (error) { + setDirectoryError( + error instanceof Error + ? error.message + : t('workbench.project_create_failed', '项目创建失败'), + ) } finally { setSubmitting(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 `@wework/src/components/projects/ProjectCreateDialog.tsx` around lines 627 - 648, The onClick submit handler awaiting onCreateProject lacks a catch, so thrown errors aren't surfaced to the user; update the onClick async handler around the await onCreateProject(...) to include a catch that captures the error and displays it the same way as handleCreateDirectory (e.g., show a user-facing error/toast or set an error state and log it), keep the existing finally { setSubmitting(false) } behavior, and avoid calling onClose when creation fails so the dialog stays open for corrective action.
🧹 Nitpick comments (2)
wework/src/api/adapters.test.ts (1)
255-274: ⚡ Quick winAdd a failure-path test for
createDirectorythrow behavior.This suite currently covers only success; please also assert rejection when command execution returns
success: false.Based on learnings: "Apply AAA pattern (Arrange, Act, Assert) for test code, mock external services, and test edge cases and error conditions."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/api/adapters.test.ts` around lines 255 - 274, Add a new test that covers the failure path of createDirectory: arrange by creating mockClient and vi.mocked(client.post).mockResolvedValueOnce({ success: false, stdout: '', stderr: 'some error' }), act by calling createDeviceApi(client) and invoking api.createDirectory('device-1', '/home/ubuntu/new-app'), and assert that the promise rejects (e.g., await expect(api.createDirectory(...)).rejects.toThrow() or rejects.toMatch/ToContain the stderr). Reference the createDirectory method on the object returned by createDeviceApi and mock client.post to simulate the failing command response.wework/src/components/layout/EnvironmentInfoPopover.tsx (1)
341-430: ⚡ Quick winOptional: add keyboard dismissal for the branch menu.
The menu only closes on outside
pointerdown; there's noEscapehandler, which is a common expectation for a popover/dropdown. Consider closing the branch menu onEscapefor keyboard users.🤖 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/EnvironmentInfoPopover.tsx` around lines 341 - 430, When the branch popover (branchMenuOpen) is shown, add a keydown listener that closes it on Escape by calling setBranchMenuOpen(false); implement this in a useEffect that watches branchMenuOpen, registers a window/key event handler when true, and removes it on cleanup or when branchMenuOpen becomes false; ensure the handler is named clearly (e.g. onBranchMenuKeyDown) and only triggers on key === 'Escape' so existing handlers like handleCheckoutBranch/newBranch form are unaffected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wework/src/api/devices.ts`:
- Around line 74-87: createDirectory currently sends a backend mkdir_p command
even for blank or whitespace-only paths; validate the path parameter (e.g.,
trim() and check length) at the top of createDirectory and throw a synchronous
Error like "Invalid directory path" when path is empty, null, or only
whitespace, so the function rejects locally instead of calling client.post with
an invalid arg; keep deviceId validation if needed and leave the existing
client.post block (command_key 'mkdir_p') unchanged for valid paths.
In `@wework/src/api/environment.ts`:
- Around line 251-291: Both checkoutProjectBranch and
createAndCheckoutProjectBranch accept branch names unsafely: update them to
insert the branch as a post-option argument by adding a "--" before the branch
in the args passed to runGitCommand (i.e., args: ['--', trimmedBranch] or args:
['-b', '--', trimmedBranch] for new-branch semantics if your git helper expects
'-b'); and extract the branch-name validation into a shared helper (e.g.,
validateBranchName) that enforces git check-ref-format --branch rules (either by
implementing those checks locally or by calling git check-ref-format on-device
before calling runGitCommand). Keep use of commandContext and runGitCommand
unchanged except for replacing the args with the safe form and calling
validateBranchName(trimmedBranch) at the top of both functions.
In `@wework/src/components/layout/EnvironmentInfoPopover.tsx`:
- Around line 161-199: The branch-menu collapse and checkout early/success
returns only toggle branchMenuOpen or partially reset state, causing stale
branchQuery/newBranchFormOpen/newBranchName to persist; replace those manual
resets with a single call to closeBranchMenu() in handleToggleBranchMenu (when
nextOpen is false or onListBranches missing), in the early return of
handleCheckoutBranch (when nextBranchName === info.branchName), and in the
successful checkout path (after await onCheckoutBranch) so all cleanup
(branchQuery, newBranchFormOpen, newBranchName and branchMenuOpen) is
centralized and consistent.
In `@wework/src/components/projects/ProjectCreateDialog.tsx`:
- Around line 497-551: The three folder-related buttons (data-testid
"open-create-folder-button", "cancel-create-folder-button", and
"confirm-create-folder-button") currently use h-8 (32px) which is below the 44px
touch target; update their className to use h-11 and add min-w-[44px] while
preserving the rest of their utility classes (e.g., keep rounded-md, px-2, font,
color, hover classes) so they meet the "44×44" mobile guideline without changing
behavior; ensure the same adjustment is applied for both the visible
"open-create-folder-button" and the two buttons inside the newFolderOpen block
that call handleCreateDirectory and cancel actions.
---
Outside diff comments:
In `@wework/src/components/projects/ProjectCreateDialog.tsx`:
- Around line 627-648: The onClick submit handler awaiting onCreateProject lacks
a catch, so thrown errors aren't surfaced to the user; update the onClick async
handler around the await onCreateProject(...) to include a catch that captures
the error and displays it the same way as handleCreateDirectory (e.g., show a
user-facing error/toast or set an error state and log it), keep the existing
finally { setSubmitting(false) } behavior, and avoid calling onClose when
creation fails so the dialog stays open for corrective action.
---
Nitpick comments:
In `@wework/src/api/adapters.test.ts`:
- Around line 255-274: Add a new test that covers the failure path of
createDirectory: arrange by creating mockClient and
vi.mocked(client.post).mockResolvedValueOnce({ success: false, stdout: '',
stderr: 'some error' }), act by calling createDeviceApi(client) and invoking
api.createDirectory('device-1', '/home/ubuntu/new-app'), and assert that the
promise rejects (e.g., await expect(api.createDirectory(...)).rejects.toThrow()
or rejects.toMatch/ToContain the stderr). Reference the createDirectory method
on the object returned by createDeviceApi and mock client.post to simulate the
failing command response.
In `@wework/src/components/layout/EnvironmentInfoPopover.tsx`:
- Around line 341-430: When the branch popover (branchMenuOpen) is shown, add a
keydown listener that closes it on Escape by calling setBranchMenuOpen(false);
implement this in a useEffect that watches branchMenuOpen, registers a
window/key event handler when true, and removes it on cleanup or when
branchMenuOpen becomes false; ensure the handler is named clearly (e.g.
onBranchMenuKeyDown) and only triggers on key === 'Escape' so existing handlers
like handleCheckoutBranch/newBranch form are unaffected.
🪄 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: ff84b720-5868-4a21-8aa4-f0d04312b572
📒 Files selected for processing (21)
backend/app/services/device/command_registry.pybackend/tests/services/test_local_device_command_service.pywework/src/App.plugins.test.tsxwework/src/api/adapters.test.tswework/src/api/devices.tswework/src/api/environment.test.tswework/src/api/environment.tswework/src/components/layout/DesktopSidebar.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/layout/EnvironmentInfoPopover.tsxwework/src/components/layout/workspace-panels/WorkspacePanelActions.tsxwework/src/components/projects/ProjectCreateDialog.test.tsxwework/src/components/projects/ProjectCreateDialog.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/pages/PluginManagementPage.tsxwework/src/pages/PluginsPage.tsxwework/src/pages/WorkbenchPage.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wework/src/features/workbench/WorkbenchProvider.tsx`:
- Around line 1033-1055: Wrap the await
resolvedServices.chatStream.sendMessage(payload) call in a try/catch/finally so
thrown exceptions don't leave the composer stuck: in the try block keep the
existing ack handling, in the catch setIsAwaitingAssistantStart(false),
dispatch({ type: 'error_set', error: (caught error) }) and return false, and in
finally always dispatch({ type: 'sending_finished' }) to ensure sending_finished
is dispatched; reference resolvedServices.chatStream.sendMessage,
setIsAwaitingAssistantStart, dispatch({ type: 'sending_finished' }), and
dispatch({ type: 'error_set', ... }) when making the change.
- Around line 1271-1301: The guard guidanceSendInFlightRef.current is set true
before calling resolvedServices.chatStream.cancelStream, but if cancelStream
throws the ref never gets reset; wrap the cancelStream call in a
try/catch/finally inside the function in WorkbenchProvider (around the call to
resolvedServices.chatStream.cancelStream) so that any thrown error is caught,
set the queued item status to 'failed' via setQueuedSends using
normalizeGuidanceError for the error, and always reset
guidanceSendInFlightRef.current = false in the finally block; ensure you still
handle the existing cancelAck error/success path when cancelStream returns
normally.
🪄 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: 7b7259ec-384c-4349-a12c-d78e2e3a7e9d
📒 Files selected for processing (12)
backend/app/services/device/command_registry.pybackend/tests/services/test_local_device_command_service.pywework/src/App.plugins.test.tsxwework/src/api/adapters.test.tswework/src/api/devices.tswework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/pages/WorkbenchPage.tsx
✅ Files skipped from review due to trivial changes (2)
- wework/src/i18n/locales/en/common.json
- wework/src/i18n/locales/zh-CN/common.json
🚧 Files skipped from review as they are similar to previous changes (7)
- backend/app/services/device/command_registry.py
- wework/src/api/adapters.test.ts
- wework/src/App.plugins.test.tsx
- wework/src/components/layout/DesktopWorkbenchLayout.tsx
- wework/src/components/layout/DesktopWorkbenchLayout.test.tsx
- wework/src/api/devices.ts
- wework/src/components/layout/DesktopWorkbenchMain.tsx
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wework/src/features/workbench/WorkbenchProvider.tsx`:
- Around line 1033-1055: Wrap the await
resolvedServices.chatStream.sendMessage(payload) call in a try/catch/finally so
thrown exceptions don't leave the composer stuck: in the try block keep the
existing ack handling, in the catch setIsAwaitingAssistantStart(false),
dispatch({ type: 'error_set', error: (caught error) }) and return false, and in
finally always dispatch({ type: 'sending_finished' }) to ensure sending_finished
is dispatched; reference resolvedServices.chatStream.sendMessage,
setIsAwaitingAssistantStart, dispatch({ type: 'sending_finished' }), and
dispatch({ type: 'error_set', ... }) when making the change.
- Around line 1271-1301: The guard guidanceSendInFlightRef.current is set true
before calling resolvedServices.chatStream.cancelStream, but if cancelStream
throws the ref never gets reset; wrap the cancelStream call in a
try/catch/finally inside the function in WorkbenchProvider (around the call to
resolvedServices.chatStream.cancelStream) so that any thrown error is caught,
set the queued item status to 'failed' via setQueuedSends using
normalizeGuidanceError for the error, and always reset
guidanceSendInFlightRef.current = false in the finally block; ensure you still
handle the existing cancelAck error/success path when cancelStream returns
normally.
🪄 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: 7b7259ec-384c-4349-a12c-d78e2e3a7e9d
📒 Files selected for processing (12)
backend/app/services/device/command_registry.pybackend/tests/services/test_local_device_command_service.pywework/src/App.plugins.test.tsxwework/src/api/adapters.test.tswework/src/api/devices.tswework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/pages/WorkbenchPage.tsx
✅ Files skipped from review due to trivial changes (2)
- wework/src/i18n/locales/en/common.json
- wework/src/i18n/locales/zh-CN/common.json
🚧 Files skipped from review as they are similar to previous changes (7)
- backend/app/services/device/command_registry.py
- wework/src/api/adapters.test.ts
- wework/src/App.plugins.test.tsx
- wework/src/components/layout/DesktopWorkbenchLayout.tsx
- wework/src/components/layout/DesktopWorkbenchLayout.test.tsx
- wework/src/api/devices.ts
- wework/src/components/layout/DesktopWorkbenchMain.tsx
🛑 Comments failed to post (2)
wework/src/features/workbench/WorkbenchProvider.tsx (2)
1033-1055:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winUnhandled send exceptions can leave composer state stuck
If
sendMessage()throws at Line 1048,sending_finishedis never dispatched andisAwaitingAssistantStartstays true, which can block subsequent queued sends and keep UI in a sending state.Suggested fix
const sendPreparedMessage = useCallback( async ( message: string, payload: ChatSendPayload, activeDeviceId?: string, attachments?: Attachment[] ): Promise<boolean> => { dispatch({ type: 'sending_started' }) setIsAwaitingAssistantStart(true) dispatchMessages({ type: 'user_added', message: { id: `local-${Date.now()}`, taskId: payload.task_id, role: 'user', content: message, status: 'done', attachments: attachments && attachments.length > 0 ? attachments : undefined, createdAt: new Date().toISOString(), }, }) - - const ack = await resolvedServices.chatStream.sendMessage(payload) - dispatch({ type: 'sending_finished' }) + let ack + try { + ack = await resolvedServices.chatStream.sendMessage(payload) + } catch (error) { + setIsAwaitingAssistantStart(false) + dispatch({ + type: 'error_set', + error: error instanceof Error ? error.message : '发送失败', + }) + return false + } finally { + dispatch({ type: 'sending_finished' }) + } if (ack.error || ack.success === false) { setIsAwaitingAssistantStart(false) dispatch({ type: 'error_set', error: ack.error ?? '发送失败' }) return false }📝 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.dispatch({ type: 'sending_started' }) setIsAwaitingAssistantStart(true) dispatchMessages({ type: 'user_added', message: { id: `local-${Date.now()}`, taskId: payload.task_id, role: 'user', content: message, status: 'done', attachments: attachments && attachments.length > 0 ? attachments : undefined, createdAt: new Date().toISOString(), }, }) let ack try { ack = await resolvedServices.chatStream.sendMessage(payload) } catch (error) { setIsAwaitingAssistantStart(false) dispatch({ type: 'error_set', error: error instanceof Error ? error.message : '发送失败', }) return false } finally { dispatch({ type: 'sending_finished' }) } if (ack.error || ack.success === false) { setIsAwaitingAssistantStart(false) dispatch({ type: 'error_set', error: ack.error ?? '发送失败' }) return 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 `@wework/src/features/workbench/WorkbenchProvider.tsx` around lines 1033 - 1055, Wrap the await resolvedServices.chatStream.sendMessage(payload) call in a try/catch/finally so thrown exceptions don't leave the composer stuck: in the try block keep the existing ack handling, in the catch setIsAwaitingAssistantStart(false), dispatch({ type: 'error_set', error: (caught error) }) and return false, and in finally always dispatch({ type: 'sending_finished' }) to ensure sending_finished is dispatched; reference resolvedServices.chatStream.sendMessage, setIsAwaitingAssistantStart, dispatch({ type: 'sending_finished' }), and dispatch({ type: 'error_set', ... }) when making the change.
1271-1301:
⚠️ Potential issue | 🟠 Major | ⚡ Quick win
guidanceSendInFlightRefcan get permanently locked on cancel errorsAt Line 1271 the in-flight guard is set true, but if
cancelStream()throws at Line 1280, control exits before reset, so later guidance sends are silently blocked.Suggested fix
- guidanceSendInFlightRef.current = true - setQueuedSends(items => - items.map(queued => - queued.id === id - ? { ...queued, status: 'sending', error: undefined } - : queued - ) - ) - - const cancelAck = await resolvedServices.chatStream.cancelStream({ - subtask_id: activeSubtaskId, - partial_content: activeAssistantMessage.content, - shell_type: activeAssistantMessage.shellType, - }) + guidanceSendInFlightRef.current = true + setQueuedSends(items => + items.map(queued => + queued.id === id + ? { ...queued, status: 'sending', error: undefined } + : queued + ) + ) - if (cancelAck.error || cancelAck.success === false) { - setQueuedSends(items => - items.map(queued => - queued.id === id - ? { - ...queued, - status: 'failed', - error: normalizeGuidanceError(cancelAck.error ?? '取消当前回复失败'), - } - : queued - ) - ) - guidanceSendInFlightRef.current = false - return - } + try { + const cancelAck = await resolvedServices.chatStream.cancelStream({ + subtask_id: activeSubtaskId, + partial_content: activeAssistantMessage.content, + shell_type: activeAssistantMessage.shellType, + }) + + if (cancelAck.error || cancelAck.success === false) { + setQueuedSends(items => + items.map(queued => + queued.id === id + ? { + ...queued, + status: 'failed', + error: normalizeGuidanceError(cancelAck.error ?? '取消当前回复失败'), + } + : queued + ) + ) + return + } - dispatchMessages({ - type: 'assistant_done', - subtaskId: activeSubtaskId, - content: activeAssistantMessage.content, - }) + dispatchMessages({ + type: 'assistant_done', + subtaskId: activeSubtaskId, + content: activeAssistantMessage.content, + }) - try { const sent = await sendPreparedMessage( item.content, item.payload, item.activeDeviceId, item.attachments ) @@ - } finally { + } finally { guidanceSendInFlightRef.current = 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 `@wework/src/features/workbench/WorkbenchProvider.tsx` around lines 1271 - 1301, The guard guidanceSendInFlightRef.current is set true before calling resolvedServices.chatStream.cancelStream, but if cancelStream throws the ref never gets reset; wrap the cancelStream call in a try/catch/finally inside the function in WorkbenchProvider (around the call to resolvedServices.chatStream.cancelStream) so that any thrown error is caught, set the queued item status to 'failed' via setQueuedSends using normalizeGuidanceError for the error, and always reset guidanceSendInFlightRef.current = false in the finally block; ensure you still handle the existing cancelAck error/success path when cancelStream returns normally.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wework/src/features/workbench/WorkbenchProvider.tsx (1)
77-152: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftSplit
WorkbenchProviderbefore adding more responsibilities.This file is already 1,474 lines, and these new device/environment callbacks plus the expanded send/guidance flows make the provider even harder to reason about. Please extract the environment/device actions and message-send orchestration into dedicated hooks or submodules instead of growing this file further. As per coding guidelines,
File size limit: If a file exceeds 1000 lines, split it into multiple sub-modules.Also applies to: 924-958, 1026-1109, 1258-1365, 1400-1466
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/features/workbench/WorkbenchProvider.tsx` around lines 77 - 152, The WorkbenchProvider has grown too large—extract device/environment callbacks and message-send orchestration into smaller modules: create a useDeviceActions hook (implementing getDeviceHomeDirectory, getProjectWorkspaceRoot, listDeviceDirectories, createDeviceDirectory, rememberExecutionDevice, refreshDevices), a useEnvironmentActions hook (implementing loadEnvironmentInfo, commitEnvironmentChanges, listEnvironmentBranches, checkoutEnvironmentBranch, createEnvironmentBranch), and a useMessageOrchestration hook (implementing setInput, sendCurrentInput, pauseCurrentResponse, isResponseStreaming, cancelQueuedMessage, sendQueuedAsGuidance, editQueuedMessage, cancelGuidanceMessage and related queued/guidance flows); move the implementations from WorkbenchProvider into those hooks, keep the WorkbenchContextValue signatures (e.g., WorkbenchProvider, WorkbenchContextValue methods like sendCurrentInput and getDeviceHomeDirectory) but delegate their bodies to the new hooks, and update imports/exports so WorkbenchProvider composes these hooks—this split keeps public API intact while reducing file size and complexity.
🧹 Nitpick comments (1)
wework/src/api/adapters.test.ts (1)
255-274: ⚡ Quick winCover the trimmed-path happy path.
These tests verify blank rejection and backend failures, but they never assert that a non-empty path is actually normalized before posting. If a later regression validates with
trim()and still sends the raw string, this suite will keep passing. Use a whitespace-padded input here and assertargscontains the trimmed path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/api/adapters.test.ts` around lines 255 - 274, Update the test for createDirectory to cover the trimmed-path happy path: call createDirectory with a whitespace-padded path (e.g., ' /home/ubuntu/new-app ') and assert that the mocked client.post (from mockClient) is called with an args array containing the trimmed path '/home/ubuntu/new-app'; locate the test using createDeviceApi and api.createDirectory and change the expect on client.post to verify the trimmed value in the posted payload's args rather than the raw input.
🤖 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.
Outside diff comments:
In `@wework/src/features/workbench/WorkbenchProvider.tsx`:
- Around line 77-152: The WorkbenchProvider has grown too large—extract
device/environment callbacks and message-send orchestration into smaller
modules: create a useDeviceActions hook (implementing getDeviceHomeDirectory,
getProjectWorkspaceRoot, listDeviceDirectories, createDeviceDirectory,
rememberExecutionDevice, refreshDevices), a useEnvironmentActions hook
(implementing loadEnvironmentInfo, commitEnvironmentChanges,
listEnvironmentBranches, checkoutEnvironmentBranch, createEnvironmentBranch),
and a useMessageOrchestration hook (implementing setInput, sendCurrentInput,
pauseCurrentResponse, isResponseStreaming, cancelQueuedMessage,
sendQueuedAsGuidance, editQueuedMessage, cancelGuidanceMessage and related
queued/guidance flows); move the implementations from WorkbenchProvider into
those hooks, keep the WorkbenchContextValue signatures (e.g., WorkbenchProvider,
WorkbenchContextValue methods like sendCurrentInput and getDeviceHomeDirectory)
but delegate their bodies to the new hooks, and update imports/exports so
WorkbenchProvider composes these hooks—this split keeps public API intact while
reducing file size and complexity.
---
Nitpick comments:
In `@wework/src/api/adapters.test.ts`:
- Around line 255-274: Update the test for createDirectory to cover the
trimmed-path happy path: call createDirectory with a whitespace-padded path
(e.g., ' /home/ubuntu/new-app ') and assert that the mocked client.post (from
mockClient) is called with an args array containing the trimmed path
'/home/ubuntu/new-app'; locate the test using createDeviceApi and
api.createDirectory and change the expect on client.post to verify the trimmed
value in the posted payload's args rather than the raw input.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ce3c5101-cb6e-4015-9af8-8b2fc23b69c7
📒 Files selected for processing (9)
wework/src/api/adapters.test.tswework/src/api/devices.tswework/src/api/environment.test.tswework/src/api/environment.tswework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/EnvironmentInfoPopover.tsxwework/src/components/projects/ProjectCreateDialog.test.tsxwework/src/components/projects/ProjectCreateDialog.tsxwework/src/features/workbench/WorkbenchProvider.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- wework/src/components/layout/DesktopWorkbenchLayout.test.tsx
- wework/src/api/environment.ts
- wework/src/components/layout/EnvironmentInfoPopover.tsx
- wework/src/components/projects/ProjectCreateDialog.tsx
This was referenced Jun 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 by CodeRabbit
New Features
Tests
Localization