Skip to content

Conversation

@ironAiken2
Copy link
Contributor

@ironAiken2 ironAiken2 commented Nov 10, 2025

Resolves FR-1655

Summary

Migrates the SFTP/SSH server launch functionality from Lit-based components to React, following the modern architecture patterns established in the codebase.

Changes

  • Extracted FolderExplorerHeaderActions as a new React component containing both File Browser and SFTP launch actions
  • Migrated SFTP session creation logic to use the useStartSession hook for consistency
  • Implemented comprehensive permission checking for SFTP volume access
  • Added validation for SFTP scaling groups by current project
  • Enhanced error handling with clear user feedback when SFTP is unavailable
  • Updated all 21 language files with new permission error messages

Technical Details

  • Uses React Suspense and lazy loading for better performance
  • Leverages Relay for GraphQL data fetching
  • Follows the established pattern of using useStartSession for session creation
  • Maintains backward compatibility with existing functionality

Testing

The process for determining whether an SFTP session can be executed is as follows:

  1. Determine whether the current project has a host that supports SFTP and is accessible.
  2. If a host supporting SFTP exists, determine whether the user has access permissions for that host.
  3. If access permissions are also present, determine whether an image supporting SFTP exists.
  • File browser launch functionality works as before
  • SFTP session creation works with proper permissions
  • Error messages display correctly when SFTP is unavailable
  • Permission checks properly validate volume access
  • All translations are properly loaded

Copy link
Contributor Author

ironAiken2 commented Nov 10, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • flow:merge-queue - adds this PR to the back of the merge queue
  • flow:hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has required the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ironAiken2 ironAiken2 marked this pull request as ready for review November 10, 2025 05:49
@github-actions
Copy link

github-actions bot commented Nov 10, 2025

Coverage report for ./react

St.
Category Percentage Covered / Total
🔴 Statements 4.69% 547/11664
🔴 Branches 3.82% 314/8215
🔴 Functions 2.91% 104/3577
🔴 Lines 4.64% 529/11405

Test suite run success

125 tests passing in 14 suites.

Report generated by 🧪jest coverage report action from 971d534

@ironAiken2 ironAiken2 force-pushed the feat/FR-1656-enhance-file-browser-session-notifications branch from eeeeac7 to e601415 Compare November 10, 2025 05:59
@ironAiken2 ironAiken2 force-pushed the feat/FR-1655-migrate-sftp-feature-to-react branch 2 times, most recently from c6f460a to 1432a1d Compare November 10, 2025 07:13
@github-actions github-actions bot added size:XL 500~ LoC and removed size:L 100~500 LoC labels Nov 10, 2025
@yomybaby yomybaby changed the base branch from feat/FR-1656-enhance-file-browser-session-notifications to graphite-base/4610 November 11, 2025 02:16
@ironAiken2 ironAiken2 force-pushed the feat/FR-1655-migrate-sftp-feature-to-react branch from 1432a1d to 5c9adde Compare November 11, 2025 05:44
@ironAiken2 ironAiken2 changed the base branch from graphite-base/4610 to refactor/FR-1656-extract-filebrowser-image-logic November 11, 2025 05:45
@ironAiken2 ironAiken2 force-pushed the feat/FR-1655-migrate-sftp-feature-to-react branch from 5c9adde to 22d8d15 Compare November 11, 2025 07:53
@graphite-app graphite-app bot changed the base branch from refactor/FR-1656-extract-filebrowser-image-logic to graphite-base/4610 November 11, 2025 07:59
@graphite-app graphite-app bot force-pushed the feat/FR-1655-migrate-sftp-feature-to-react branch from 22d8d15 to 3400bf9 Compare November 11, 2025 08:03
@graphite-app graphite-app bot force-pushed the graphite-base/4610 branch from 113d240 to 4c0bf66 Compare November 11, 2025 08:03
@graphite-app graphite-app bot changed the base branch from graphite-base/4610 to main November 11, 2025 08:03
@graphite-app graphite-app bot force-pushed the feat/FR-1655-migrate-sftp-feature-to-react branch from 3400bf9 to 6322cdf Compare November 11, 2025 08:04
Copilot AI review requested due to automatic review settings November 12, 2025 05:56
@ironAiken2 ironAiken2 force-pushed the feat/FR-1655-migrate-sftp-feature-to-react branch from 6322cdf to 76f6aca Compare November 12, 2025 05:56
Copilot finished reviewing on behalf of ironAiken2 November 12, 2025 05:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR migrates the SFTP server launch functionality from Lit-based components to React, aligning with the modern architecture patterns in the codebase. The migration introduces proper permission checking, validation for SFTP scaling groups, and enhanced error handling.

Key Changes

  • Created a new SFTPServerButton React component with comprehensive permission and resource validation
  • Consolidated file browser and SFTP image hooks into a single useDefaultImagesWithFallback.ts file
  • Added three new i18n keys across 21 language files for permission-related error messages

Reviewed Changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
react/src/components/SFTPServerButton.tsx New React component implementing SFTP session launch with permission checks and validation
react/src/components/FolderExplorerHeader.tsx Updated to use new SFTPServerButton component with Suspense wrapper
react/src/components/FileBrowserButton.tsx Enhanced with permission checking for volume access
react/src/hooks/useDefaultImagesWithFallback.ts Consolidated file browser and SSH/SFTP image hooks
react/src/hooks/useDefaultFileBrowserImageWithFallback.ts Removed (functionality moved to consolidated hook)
react/src/hooks/useStartSession.tsx Improved session creation handling with notification for existing sessions
react/src/pages/SessionLauncherPage.tsx Added 'system' session type to interface
resources/i18n/*.json Added three new translation keys for permission error messages
Comments suppressed due to low confidence (3)

react/src/components/SFTPServerButton.tsx:33

  • Unknown directive: 'use memo'.
  'use memo';

react/src/hooks/useDefaultImagesWithFallback.ts:50

  • Unknown directive: 'use memo'.
  'use memo';

react/src/hooks/useDefaultImagesWithFallback.ts:97

  • Unknown directive: 'use memo'.
  'use memo';

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ironAiken2 ironAiken2 force-pushed the feat/FR-1655-migrate-sftp-feature-to-react branch from 76f6aca to 52e13e4 Compare November 12, 2025 09:02
@ironAiken2 ironAiken2 requested a review from yomybaby November 12, 2025 09:02
Copy link
Contributor

@agatha197 agatha197 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image.png

If the sftp session is pending status and click the sftp app directly, it shows undefined and it is not updated before clicking the sftp app again.

@ironAiken2 ironAiken2 force-pushed the feat/FR-1655-migrate-sftp-feature-to-react branch from 52e13e4 to 4f4fff8 Compare November 13, 2025 04:30
Copy link
Member

@yomybaby yomybaby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@agatha197 agatha197 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ironAiken2 ironAiken2 requested a review from agatha197 November 14, 2025 08:23
@ironAiken2 ironAiken2 force-pushed the feat/FR-1655-migrate-sftp-feature-to-react branch from 4f4fff8 to 75e5bb8 Compare November 14, 2025 08:23
Copy link
Contributor

@agatha197 agatha197 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

@nowgnuesLee nowgnuesLee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yomybaby yomybaby force-pushed the feat/FR-1655-migrate-sftp-feature-to-react branch from 75e5bb8 to 5e23245 Compare November 14, 2025 10:08
@graphite-app
Copy link

graphite-app bot commented Nov 14, 2025

Merge activity

Resolves [FR-1655](https://lablup.atlassian.net/browse/FR-1655)

## Summary
Migrates the SFTP/SSH server launch functionality from Lit-based components to React, following the modern architecture patterns established in the codebase.

## Changes
- Extracted `FolderExplorerHeaderActions` as a new React component containing both File Browser and SFTP launch actions
- Migrated SFTP session creation logic to use the `useStartSession` hook for consistency
- Implemented comprehensive permission checking for SFTP volume access
- Added validation for SFTP scaling groups by current project
- Enhanced error handling with clear user feedback when SFTP is unavailable
- Updated all 21 language files with new permission error messages

## Technical Details
- Uses React Suspense and lazy loading for better performance
- Leverages Relay for GraphQL data fetching
- Follows the established pattern of using `useStartSession` for session creation
- Maintains backward compatibility with existing functionality

## Testing
The process for determining whether an SFTP session can be executed is as follows:
1. Determine whether the current project has a host that supports SFTP and is accessible.
2. If a host supporting SFTP exists, determine whether the user has access permissions for that host.
3. If access permissions are also present, determine whether an image supporting SFTP exists.

- [ ] File browser launch functionality works as before
- [ ] SFTP session creation works with proper permissions
- [ ] Error messages display correctly when SFTP is unavailable
- [ ] Permission checks properly validate volume access
- [ ] All translations are properly loaded

[FR-1655]: https://lablup.atlassian.net/browse/FR-1655?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
@graphite-app graphite-app bot force-pushed the feat/FR-1655-migrate-sftp-feature-to-react branch from 5e23245 to 971d534 Compare November 14, 2025 10:09
@graphite-app graphite-app bot merged commit 971d534 into main Nov 14, 2025
11 checks passed
@graphite-app graphite-app bot deleted the feat/FR-1655-migrate-sftp-feature-to-react branch November 14, 2025 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:i18n Localization area:ux UI / UX issue. size:XL 500~ LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants