Repository navigation
fix: resolve desktop and session usability issues - #351
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Too many files changed for review (117 files, 100 file limit). Bypass the limit by tagging |
WalkthroughThe pull request adds durable session renaming across the manager, APIs, CLI, native tools, built-in tools, and web UI. It also adds filesystem root discovery, desktop zoom support, window-layer ordering, and contained model-list scrolling. ChangesSession rename
Filesystem roots
Desktop interaction updates
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant WebUI
participant RenameAPI
participant SessionManager
participant Catalog
User->>WebUI: Select Rename session
WebUI->>RenameAPI: PATCH workspace/session with name
RenameAPI->>SessionManager: Rename workspace session
SessionManager->>Catalog: Persist updated session
Catalog-->>SessionManager: Persistence result
SessionManager-->>RenameAPI: Updated session
RenameAPI-->>WebUI: Return renamed session
WebUI-->>User: Close dialog and refresh session data
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
web/src/systems/os/components/__tests__/os-window.test.tsx (1)
3-4: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the seam part of the visual-layer invariant.
The test says that floating frames stay above tiled resize seams, but it renders only
OsWindowand checks only frame z-index values. It does not verifyOsSnapSeamor itsz-[2]layer. A seam-layer regression can therefore pass. Add a seam-specific assertion to this existing canonical suite.As per path instructions, this is a product-contract test, so a focused style or class assertion is allowed instead of a snapshot. Based on the supplied test invariant and PR objective.
Also applies to: 171-184
🤖 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 `@web/src/systems/os/components/__tests__/os-window.test.tsx` around lines 3 - 4, Extend the canonical visual-layer test around OsWindow to render or query the OsSnapSeam element and assert its seam-specific z-[2] styling. Keep the existing frame z-index assertions, adding a focused class/style check that verifies resize seams remain below floating frames.Source: Path instructions
desktop/src-tauri/tests/contracts.rs (1)
296-315: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the builder setting, not only its permission boundary.
This test can pass if
.zoom_hotkeys_enabled(true)is removed fromdesktop/src-tauri/src/windowing.rsLine 75 because it checks only JSON configuration. Add a focused runtime or contract check for the main-window builder, or document why the available Tauri test surface cannot observe this setting.Based on the supplied implementation and contract-test coverage.
🤖 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 `@desktop/src-tauri/tests/contracts.rs` around lines 296 - 315, The test should also verify that the main-window builder enables zoom hotkeys via the windowing implementation’s zoom configuration, not only the capability JSON. Add a focused runtime or contract assertion tied to the main-window builder and its zoom-hotkey setting; if Tauri’s test surface cannot observe it, document that limitation in the test.web/src/systems/os/lib/window-visual-layer.ts (1)
8-11: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a minimum-layer regression test.
OsWindowFrameModel.layerhas a minimum of1, so the first floating frame renders at z-index3, aboveos-snap-seam.tsx’sz-[2]. Add awindowVisualLayertest forlayer: 1.🤖 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 `@web/src/systems/os/lib/window-visual-layer.ts` around lines 8 - 11, Add a regression test for windowVisualLayer using a floating frame with layer: 1, and assert it returns WINDOW_VISUAL_LAYER.seam plus 1 (the expected z-index above the snap seam). Keep the test focused on this minimum-layer boundary and use the existing test conventions for the function.
🤖 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 `@internal/api/core/handlers_test.go`:
- Around line 1053-1083: Strengthen the assertions in the “Should reject unknown
request fields” and “Should hide a session outside the route workspace” subtests
by using assertAPIErrorResponse alongside the existing status checks. Verify the
400 response identifies legacy_name as an unknown field, and verify the 404
response matches the session not found error contract; keep the Rename()
call-count assertions unchanged.
In `@internal/api/spec/registry_sessions.go`:
- Around line 47-51: Update the operation’s Responses list in the session rename
specification to include the HTTP 503 response returned when the configured
session manager lacks core.SessionRenameManager, using the appropriate error
payload and description consistent with the handler contract.
In `@internal/cli/session_test.go`:
- Around line 923-967: Wrap the test body in
internal/cli/session_test.go:923-967 in t.Run("Should rename a session and
return the updated record", ...), and call t.Parallel() inside that subtest.
Also wrap the client rename assertions in internal/cli/client_test.go:2606-2609
in a dedicated t.Run with a descriptive “Should …” name and call t.Parallel()
inside it.
In `@internal/daemon/native_tools_test.go`:
- Around line 6474-6493: Extend the “Should rename a session” test around
ToolIDSessionRename with a request targeting ws-foreign-stable, asserting
ErrToolDenied with ReasonWorkspaceAccessDenied. Track rename-call count in the
test manager and verify it does not increase for the rejected request, while
preserving the existing successful rename and target/name assertions.
In `@web/src/hooks/routes/use-session-rename-dialog.ts`:
- Around line 16-20: Update confirmRename in use-session-rename-dialog so
rejected onRename requests are no longer discarded: capture the failure in
dialog-visible error state and render it, or propagate it through the hook’s
caller, while preserving the existing success behavior that closes the dialog.
---
Nitpick comments:
In `@desktop/src-tauri/tests/contracts.rs`:
- Around line 296-315: The test should also verify that the main-window builder
enables zoom hotkeys via the windowing implementation’s zoom configuration, not
only the capability JSON. Add a focused runtime or contract assertion tied to
the main-window builder and its zoom-hotkey setting; if Tauri’s test surface
cannot observe it, document that limitation in the test.
In `@web/src/systems/os/components/__tests__/os-window.test.tsx`:
- Around line 3-4: Extend the canonical visual-layer test around OsWindow to
render or query the OsSnapSeam element and assert its seam-specific z-[2]
styling. Keep the existing frame z-index assertions, adding a focused
class/style check that verifies resize seams remain below floating frames.
In `@web/src/systems/os/lib/window-visual-layer.ts`:
- Around line 8-11: Add a regression test for windowVisualLayer using a floating
frame with layer: 1, and assert it returns WINDOW_VISUAL_LAYER.seam plus 1 (the
expected z-index above the snap seam). Keep the test focused on this
minimum-layer boundary and use the existing test conventions for the function.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1ecee5f5-2d6e-400a-8a19-e19f9eb9757e
⛔ Files ignored due to path filters (35)
desktop/src-tauri/capabilities/main-zoom.jsonis excluded by!**/*.jsondesktop/src-tauri/tauri.conf.jsonis excluded by!**/*.jsondocs/qa/charters/CH-add-workspace-from-root.mdis excluded by!**/*.mddocs/qa/charters/CH-desktop-page-zoom.mdis excluded by!**/*.mddocs/qa/charters/CH-rename-session-parity.mdis excluded by!**/*.mddocs/qa/charters/CH-runtime-selector-scroll-boundary.mdis excluded by!**/*.mddocs/qa/charters/CH-window-layer-seam.mdis excluded by!**/*.mddocs/qa/journeys/J-add-workspace-by-browsing.mdis excluded by!**/*.mddocs/qa/journeys/J-desktop-attach-daily.mdis excluded by!**/*.mddocs/qa/journeys/J-rename-session.mdis excluded by!**/*.mddocs/qa/reports/2026-08-11-open-issues.mdis excluded by!**/*.mddocs/qa/scenarios/APP-desktop-page-zoom.mdis excluded by!**/*.mddocs/qa/scenarios/ET-window-manager-layout-gestures.mdis excluded by!**/*.mddocs/qa/scenarios/MS-051.mdis excluded by!**/*.mddocs/qa/scenarios/MS-web-workspace-add-directory-browser.mdis excluded by!**/*.mddocs/qa/scenarios/RT-038.mdis excluded by!**/*.mddocs/qa/scenarios/RT-068.mdis excluded by!**/*.mddocs/qa/scenarios/RT-session-rename-durable.mdis excluded by!**/*.mdinternal/tools/builtin/testdata/native-tool-catalog.jsonis excluded by!**/*.jsonopenapi/compozy.jsonis excluded by!**/*.jsonpackages/site/content/docs/cli/session/index.mdxis excluded by!**/*.mdxpackages/site/content/docs/cli/session/meta.jsonis excluded by!**/*.jsonpackages/site/content/docs/cli/session/rename.mdxis excluded by!**/*.mdxskills/compozy/references/desktop.mdis excluded by!**/*.mdskills/compozy/references/native-tools.mdis excluded by!**/*.mdskills/compozy/references/runtime-operations.mdis excluded by!**/*.mdweb/e2e/__tests__/os-shell.spec.tsis excluded by!web/e2e/**web/e2e/__tests__/session-provider-override.spec.tsis excluded by!web/e2e/**web/e2e/__tests__/workspace-setup.spec.tsis excluded by!web/e2e/**web/src/generated/compozy-openapi.d.tsis excluded by!**/generated/**,!**/generated/**,!**/*.d.tsweb/src/systems/agent/components/stories/agent-sessions-list.stories.tsxis excluded by!**/*.stories.tsxweb/src/systems/onboarding/components/stories/directory-browser.stories.tsxis excluded by!**/*.stories.tsxweb/src/systems/os/components/stories/attention-surfaces.stories.tsxis excluded by!**/*.stories.tsxweb/src/systems/session/components/stories/session-sidebar.stories.tsxis excluded by!**/*.stories.tsxweb/src/systems/workspace/components/stories/workspace-setup.stories.tsxis excluded by!**/*.stories.tsx
📒 Files selected for processing (79)
desktop/src-tauri/src/windowing.rsdesktop/src-tauri/tests/contracts.rsinternal/api/contract/fs.gointernal/api/contract/session_runtime_payloads.gointernal/api/core/fs_browse.gointernal/api/core/fs_browse_test.gointernal/api/core/fs_roots_unix.gointernal/api/core/fs_roots_windows.gointernal/api/core/handlers_test.gointernal/api/core/session_rename.gointernal/api/core/session_rename_interfaces.gointernal/api/core/test_helpers_test.gointernal/api/httpapi/handlers_test.gointernal/api/httpapi/session_routes.gointernal/api/spec/paths.gointernal/api/spec/registry_filesystem.gointernal/api/spec/registry_sessions.gointernal/api/testutil/session_stub.gointernal/api/udsapi/handlers_test.gointernal/api/udsapi/session_routes.gointernal/cli/client_session_api.gointernal/cli/client_session_rename.gointernal/cli/client_session_types.gointernal/cli/client_test.gointernal/cli/helpers_test.gointernal/cli/session_command.gointernal/cli/session_rename.gointernal/cli/session_test.gointernal/daemon/native_session_rename.gointernal/daemon/native_tool_binding_groups.gointernal/daemon/native_tools_test.gointernal/session/manager_transition.gointernal/session/manager_transition_test.gointernal/session/session_rename.gointernal/toolmeta/native_entries.gointernal/toolmeta/registry_test.gointernal/tools/builtin/builtin_test.gointernal/tools/builtin/session_rename.gointernal/tools/builtin/sessions.gointernal/tools/builtin/toolsets.gointernal/tools/builtin_ids.goweb/src/hooks/routes/__tests__/use-session-page-controls.test.tsxweb/src/hooks/routes/use-session-page-controls.tsweb/src/hooks/routes/use-session-rename-dialog.tsweb/src/systems/agent/components/__tests__/agent-sessions-list.test.tsxweb/src/systems/onboarding/components/__tests__/directory-browser.test.tsxweb/src/systems/onboarding/components/directory-browser.tsxweb/src/systems/onboarding/components/step-workspaces.tsxweb/src/systems/onboarding/hooks/__tests__/use-onboarding-workspaces.test.tsxweb/src/systems/onboarding/hooks/use-onboarding-workspaces.tsweb/src/systems/onboarding/mocks/setup-fixtures.tsweb/src/systems/os/apps/agents/agent-detail-location.tsxweb/src/systems/os/apps/agents/use-agent-detail.tsweb/src/systems/os/apps/session/session-window-content.tsxweb/src/systems/os/apps/session/use-session-window-controller.tsxweb/src/systems/os/apps/session/use-session-window-sidebar.tsweb/src/systems/os/components/__tests__/os-window.test.tsxweb/src/systems/os/components/__tests__/sessions-modal.test.tsxweb/src/systems/os/components/desktop-shell.tsxweb/src/systems/os/components/os-snap-seam.tsxweb/src/systems/os/components/os-window.tsxweb/src/systems/os/lib/window-visual-layer.tsweb/src/systems/runtime/components/runtime-selector/model-list.tsxweb/src/systems/runtime/mocks/handlers.tsweb/src/systems/session/adapters/__tests__/session-api.test.tsweb/src/systems/session/adapters/session-api.tsweb/src/systems/session/components/__tests__/session-rename-dialog.test.tsxweb/src/systems/session/components/session-rename-dialog.tsxweb/src/systems/session/components/session-row-actions.tsxweb/src/systems/session/hooks/__tests__/use-session-actions.test.tsxweb/src/systems/session/hooks/__tests__/use-session-topbar-slot.test.tsxweb/src/systems/session/hooks/use-session-actions.tsweb/src/systems/session/hooks/use-session-lifecycle-actions.tsweb/src/systems/session/hooks/use-session-topbar-slot.tsxweb/src/systems/session/index.tsweb/src/systems/session/types.tsweb/src/systems/workspace/components/__tests__/workspace-setup.test.tsxweb/src/systems/workspace/components/workspace-setup-location-pane.tsxweb/src/systems/workspace/hooks/use-workspace-setup-content.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/src/hooks/routes/__tests__/use-session-page-controls.test.tsx (1)
392-420: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the dialog-hook tests to the canonical hook suite.
Lines 393-420 test
useSessionRenameDialog, but this suite ownsuseSessionPageControls. Move these tests toweb/src/hooks/routes/__tests__/use-session-rename-dialog.test.tsx.As per coding guidelines, “Before adding a Vitest or Playwright test, identify the invariant, owning layer, and canonical suite; update existing route, hook, component, story, or E2E suites first.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/hooks/routes/__tests__/use-session-page-controls.test.tsx` around lines 392 - 420, Move the useSessionRenameDialog test block from the use-session-page-controls suite into the canonical use-session-rename-dialog.test.tsx suite. Preserve both failure and successful-rename assertions unchanged, and remove the duplicated tests from the useSessionPageControls suite.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.
Nitpick comments:
In `@web/src/hooks/routes/__tests__/use-session-page-controls.test.tsx`:
- Around line 392-420: Move the useSessionRenameDialog test block from the
use-session-page-controls suite into the canonical
use-session-rename-dialog.test.tsx suite. Preserve both failure and
successful-rename assertions unchanged, and remove the duplicated tests from the
useSessionPageControls suite.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 94e4c871-852e-4949-8182-ba7e4ef9d5d4
⛔ Files ignored due to path filters (5)
docs/qa/bugs/BUG-20260812-rename-dialog-double-escape.mdis excluded by!**/*.mddocs/qa/reports/2026-08-12-pr-351-review-round-1.mdis excluded by!**/*.mddocs/qa/scenarios/RT-session-rename-durable.mdis excluded by!**/*.mdopenapi/compozy.jsonis excluded by!**/*.jsonweb/src/generated/compozy-openapi.d.tsis excluded by!**/generated/**,!**/generated/**,!**/*.d.ts
📒 Files selected for processing (16)
desktop/src-tauri/tests/contracts.rsinternal/api/core/handlers_test.gointernal/api/spec/registry_sessions.gointernal/cli/client_test.gointernal/cli/session_test.gointernal/daemon/native_tools_test.goweb/src/hooks/routes/__tests__/use-session-page-controls.test.tsxweb/src/hooks/routes/use-session-rename-dialog.tsweb/src/systems/os/apps/session/session-window-content.tsxweb/src/systems/os/components/__tests__/os-window.test.tsxweb/src/systems/os/components/os-snap-seam.tsxweb/src/systems/os/lib/window-visual-layer.tsweb/src/systems/session/components/__tests__/session-rename-dialog.test.tsxweb/src/systems/session/components/session-rename-dialog.tsxweb/src/systems/session/hooks/__tests__/use-session-topbar-slot.test.tsxweb/src/systems/session/hooks/use-session-topbar-slot.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- desktop/src-tauri/tests/contracts.rs
- internal/cli/session_test.go
- internal/cli/client_test.go
- web/src/systems/os/apps/session/session-window-content.tsx
- internal/api/spec/registry_sessions.go
- web/src/systems/os/lib/window-visual-layer.ts
- internal/daemon/native_tools_test.go
- internal/api/core/handlers_test.go
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/src/hooks/routes/__tests__/use-session-rename-dialog.test.tsx (1)
38-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the error assertion cover a real retry.
result.current.errorisnullwhen this test starts. The test never creates a previous failure, so Line 39 cannot detect a stale error after a failed rename. First reject one rename, then resolve a second rename, and assert that the dialog closes witherrorset tonull.The hook contract in
web/src/hooks/routes/use-session-rename-dialog.tsLines 12-46 requires this failed-then-successful transition.Example retry setup
- const { result } = renderHook(() => useSessionRenameDialog(() => rename.promise)); + const onRename = vi.fn() + .mockRejectedValueOnce(new Error("Rename unavailable.")) + .mockReturnValueOnce(rename.promise); + const { result } = renderHook(() => useSessionRenameDialog(onRename)); act(() => result.current.openDialog()); act(() => result.current.confirmRename("Release review")); + await waitFor(() => expect(result.current.error).toBe("Rename unavailable.")); + expect(result.current.open).toBe(true); + act(() => result.current.confirmRename("Release review")); + expect(result.current.open).toBe(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 `@web/src/hooks/routes/__tests__/use-session-rename-dialog.test.tsx` around lines 38 - 39, Update the test around the useSessionRenameDialog hook to exercise a failed-then-successful rename: make the first rename reject, assert the failure state, then make a second rename resolve and verify the dialog closes with result.current.error reset to null. Preserve the existing initial closed and error-free assertions.
🤖 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.
Nitpick comments:
In `@web/src/hooks/routes/__tests__/use-session-rename-dialog.test.tsx`:
- Around line 38-39: Update the test around the useSessionRenameDialog hook to
exercise a failed-then-successful rename: make the first rename reject, assert
the failure state, then make a second rename resolve and verify the dialog
closes with result.current.error reset to null. Preserve the existing initial
closed and error-free assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 11d77c95-5d2d-44bd-bfea-86325aa361da
📒 Files selected for processing (2)
web/src/hooks/routes/__tests__/use-session-page-controls.test.tsxweb/src/hooks/routes/__tests__/use-session-rename-dialog.test.tsx
💤 Files with no reviewable changes (1)
- web/src/hooks/routes/tests/use-session-page-controls.test.tsx
Summary
Resolve five open desktop and session usability issues as one cross-surface change:
Root-cause fixes
Runtime selector scrolling
The model list is now the real bounded scroll owner and uses
overscroll-behavior: contain, so wheel input at either end cannot escape into the desktop behind the popup.Filesystem roots
GET /api/fs/browsenow returns daemon-owned roots alongside directory entries. Unix reports/; Windows discovers logical drives through the operating system. The shared directory browser renders these roots as Locations, without browser-side path guesses or config fallbacks.Durable session rename
Renames validate and normalize a user-session display name, enforce workspace ownership, and persist active, stopped, and archived sessions without changing identity, transcript, runtime, archive metadata, lineage, or ownership. The same contract ships through:
PATCH /api/workspaces/{workspace_id}/sessions/{session_id}over HTTP and UDS;compozy session rename <id> <name>;compozy__session_renamein the sessions toolset;Generated OpenAPI/TypeScript contracts, CLI docs, native-tool catalog, and the official Compozy skill references are updated in the same commit.
Desktop page zoom
Tauri's standard WebView zoom hotkeys are enabled, with a dedicated capability restricted to the
mainwindow and loopback daemon origins. The in-product single-window Zoom action remains a separate layout command.Window visual ordering
Visual layer is derived from semantic placement: tiled windows use layer 1, shared seams layer 2, and floating windows start above both before applying focus order. This preserves uncovered seam interaction without allowing a seam to intercept a floating window.
Verification
make gate-full— PASS, current fullmake verifyrecord (2b4e627dce87e186362ba92cd2da74ab0a3f7736)-race; 2 platform-only skipsclean: trueReal-user QA walked all five changed journeys in Web, CLI, API, runtime, and the native desktop shell. Evidence and debriefs are recorded in
docs/qa/reports/2026-08-11-open-issues.md.Compozy Impact Audit
compozy__session_rename, sessions toolset membership, descriptor/catalog metadata, runtime binding, capability coverage, and contract tests; no native-tool impact for scroll, roots, desktop zoom, or visual layers after checking their registries and descriptors.skills/compozy/; no hook, bridge SDK, MCP sidecar, registry, orconfig.tomllifecycle changes. Filesystem roots extend the existing browse response without a new setting.Closes #341
Closes #342
Closes #344
Closes #345
Closes #346
Summary by CodeRabbit
New Features
Bug Fixes