feat: add provider session ID copy actions - #1040
Open
sudo-eugene wants to merge 2 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a provider session ID lookup endpoint and service method, connects it to sidebar copy actions on mobile and desktop, and extends shared menu/dialog presentation behavior with portal support and bottom-sheet animation. ChangesProvider session ID access
Sequence Diagram(s)sequenceDiagram
participant User
participant SidebarSessionItem
participant api
participant ProviderRoute
participant SessionsService
User->>SidebarSessionItem: Open session options
SidebarSessionItem->>api: Request provider session ID
api->>ProviderRoute: GET provider-id endpoint
ProviderRoute->>SessionsService: Resolve session mapping
SessionsService-->>SidebarSessionItem: Return provider session ID
SidebarSessionItem-->>User: Copy ID and update action state
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![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: 2
🧹 Nitpick comments (1)
server/modules/providers/tests/sessions.service.test.ts (1)
40-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the missing-session error contract.
The new service method also promises
SESSION_NOT_FOUNDwith status 404, but these tests only cover success and unavailable provider IDs.Proposed test
+test('provider session id reports a missing app session', { concurrency: false }, async () => { + await withIsolatedDatabase(() => { + assert.throws( + () => sessionsService.getProviderSessionId('missing-session'), + (error: unknown) => { + const typedError = error as { code?: string; statusCode?: number }; + return typedError.code === 'SESSION_NOT_FOUND' && typedError.statusCode === 404; + }, + ); + }); +});🤖 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 `@server/modules/providers/tests/sessions.service.test.ts` around lines 40 - 52, Add a test alongside the existing provider session ID tests that calls sessionsService.getProviderSessionId with a session identifier absent from the database and asserts the thrown error has code SESSION_NOT_FOUND and statusCode 404. Keep the existing success and PROVIDER_SESSION_ID_NOT_AVAILABLE coverage unchanged.
🤖 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 `@src/components/sidebar/view/subcomponents/SidebarSessionItem.tsx`:
- Around line 98-100: Update the provider-session request flow associated with
providerSessionId and the mobile options state so closing the options
invalidates any in-flight provider-ID request before a subsequent reopen. Track
request sequencing or cancellation, and ignore responses from stale requests so
an older response cannot overwrite the latest success or error state.
- Around line 175-202: Update the copy-state handling in copyProviderSessionId,
handleCopyAction, and copyLabel so clipboard failure is represented separately
from an unavailable providerSessionId. Preserve the loaded session ID after
copyTextToClipboard fails, display a clipboard-copy failure state, and make
subsequent clicks retry copying instead of calling loadProviderSessionId;
reserve the unavailable state for missing or failed session ID loading.
---
Nitpick comments:
In `@server/modules/providers/tests/sessions.service.test.ts`:
- Around line 40-52: Add a test alongside the existing provider session ID tests
that calls sessionsService.getProviderSessionId with a session identifier absent
from the database and asserts the thrown error has code SESSION_NOT_FOUND and
statusCode 404. Keep the existing success and PROVIDER_SESSION_ID_NOT_AVAILABLE
coverage unchanged.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 53d9c645-c757-446d-aad4-e88d204dcf51
📒 Files selected for processing (8)
server/modules/providers/provider.routes.tsserver/modules/providers/services/sessions.service.tsserver/modules/providers/tests/sessions.service.test.tssrc/components/sidebar/view/subcomponents/SidebarSessionItem.tsxsrc/shared/view/ui/ActionMenu.tsxsrc/shared/view/ui/Dialog.tsxsrc/utils/api.jstailwind.config.js
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.
What changed
CloudCLI now lets users copy the provider-native Claude, Codex, Cursor, or OpenCode session ID from a compact session actions menu on mobile and desktop.
Why now
The UI exposed only CloudCLI's internal session identifier, leaving users—especially those working from the mobile PWA—without a practical way to move a conversation into native CLI workflows.
Why
Provider-native IDs remain outside normal session payloads and are fetched only when the user opens the actions menu. This preserves the existing distinction between CloudCLI's stable application ID and the provider's own ID while making the latter available for an explicit copy action.
The copy, rename, and archive/delete actions share the existing action-menu behavior on desktop rather than adding another independent popover. On mobile, the same actions use a bottom sheet with a dedicated vertical entrance so the narrow session row stays uncluttered.
Before and after
Test plan
npm run typechecknpx tsx --tsconfig server/tsconfig.json --test server/modules/providers/tests/sessions.service.test.tsnpm run buildSummary by CodeRabbit
New Features
UI Improvements
Bug Fixes