add SSH environments from the composer - #274
Conversation
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES. The remote-host lifecycle can be overwritten by stale connection completions and status events, the legacy lock fallback can reclaim a live successor lock, and reserved selector values collide with valid SSH aliases. The changed tests do not honestly exercise one pending-connect race. This is a meaningful UI change, but the supplied PR evidence contains no screenshots or screen recording; please add screenshots or a short screen recording. GitHub check evidence is present and currently pending, so required checks still govern merge readiness.
Deterministic publication result: 3 blocking and 1 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 1 blocking screenshot-evidence requirement(s) in this review body.
🤖 Blocking · Screenshots needed
This PR changes Berd’s graphical interface. Please add screenshots or a short screen recording so the visual result can be reviewed. Screenshots are review evidence; they do not replace accessibility, responsive, theme, localization, or behavior validation.
0139c25 to
93a98e0
Compare
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES. While an SSH connection is pending, the new modal disables or suppresses every dismissal route, so a connection attempt that never completes can trap the user and block the chat workflow indefinitely. The supplied screenshot evidence covers this meaningful graphical UI change. The four prior review issues are fixed in the current comparison and their resolved threads contain substantive human-account replies. GitHub check evidence is structurally valid and currently pending; required checks still govern merge readiness.
Deterministic publication result: 1 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.
e30bfb6 to
e6869b2
Compare
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES. A connection that is forgotten or superseded while the add dialog waits can still be selected in the composer when its promise resolves, and the already-ready persistence path can lose concurrent manual-host additions by writing from a stale snapshot. The supplied evidence includes a screenshot for this meaningful graphical UI change. All five prior automated findings are resolved, fixed in the current comparison, and have substantive human-account replies. GitHub check evidence is structurally valid; required checks still govern merge readiness.
Deterministic publication result: 2 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.
e6869b2 to
103d1c2
Compare
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
COMMENT. The implementation is clear across product flow, graphical UX, architecture, and engineering, and the seven prior automated findings are fixed with substantive human-account replies in their resolved threads. One product vocabulary decision remains: the new flow calls the existing SSH host concept an SSH environment while adjacent surfaces continue to call it a host. The supplied evidence includes a screenshot for this meaningful graphical UI change. GitHub check evidence is structurally valid; required checks still govern merge readiness.
Deterministic publication result: 0 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.
|
🤖 Addressed in 67e7ccf. The user-facing action, dialog title, and accessibility label now say “Add SSH host,” matching the existing “SSH hosts” terminology. “Environment” remains the broader concept of where a chat runs. The selector coverage was updated, and the focused tests plus the full frontend gate pass. |
67e7ccf to
a5aba65
Compare
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES. A superseded or forgotten connection returns false to the add-host dialog, which silently stops loading and leaves the unchanged dialog open without explaining what happened or how to recover. The supplied evidence includes screenshots for this meaningful graphical UI change. The seven prior automated findings are resolved, fixed in the current comparison, and have substantive human-account replies. GitHub check evidence is structurally valid; required checks still govern merge readiness.
Deterministic publication result: 1 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
APPROVE. No new publishable findings remain after the full static review. The shared connection outcome can still be silently ignored by the existing Settings caller, but that is the same underlying superseded-connection feedback issue as the protected resolved thread and is therefore suppressed. The eight prior automated threads are resolved with substantive human-account replies. Supplied evidence includes screenshots for this meaningful graphical UI change. GitHub check evidence is structurally valid; required checks still govern merge readiness.
Deterministic publication result: 0 blocking and 0 non-blocking inline finding(s) publishable; 1 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.
Pending checks: 3 check(s) are not complete.
This approval reflects the completed code review only; merge readiness remains governed by the repository's required checks.

Category: improvement
User Impact: Users can add and select a new SSH environment directly from the chat composer.
Problem: The environment picker only lists SSH hosts Berd already knows about, so adding a new host requires leaving the chat flow and finding the remote-host settings. Solution: Add a separated “Add SSH environment” action that opens an accessible connection dialog, accepts an SSH config alias or
user@host, selects the environment after a successful connection, and keeps loading and error feedback in context. This PR is stacked on #269 and should merge after it.File changes
src/features/chat/ui/AddRemoteHostDialog.tsx
Adds the focused connection dialog, including validation, pending-state protection, backend error feedback, and selection after a successful connection.
src/features/chat/ui/RemoteHostSelector.tsx
Adds the separated “Add SSH environment” menu action and preserves focus while transitioning from the environment menu into the dialog.
src/features/chat/ui/tests/RemoteHostSelector.test.tsx
Covers the empty-host menu, successful add-and-select flow, and retained dialog feedback when connection fails.
src/shared/i18n/locales/en/chat.json
Adds localized copy and accessible labels for the new environment action and dialog.