Skip to content

fix(ssh): refresh stale remote shell profiles for agent installs#2255

Merged
jschwxrz merged 7 commits into
mainfrom
fix-stale-remote-shell
May 28, 2026
Merged

fix(ssh): refresh stale remote shell profiles for agent installs#2255
jschwxrz merged 7 commits into
mainfrom
fix-stale-remote-shell

Conversation

@jschwxrz
Copy link
Copy Markdown
Collaborator

  • refreshes remote shell env before dependency install commands
  • lets user-triggered agent discovery force a fresh remote shell profile
  • keeps background dependency probes from forcing extra recaptures
  • dedupes concurrent SSH shell profile refreshes
  • retries remote agent launch once after exit code 127 with a refreshed profile

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 28, 2026

Greptile Summary

This PR fixes stale SSH shell profiles preventing agent commands from being found after installation. It threads a refreshShellEnv flag through the dependency probe/install pipeline so user-triggered checks re-capture the remote environment, while background probes and post-install refreshes are kept lightweight.

  • SshClientProxy.refreshRemoteShellProfile deduplicates concurrent refresh calls and correctly preempts an in-flight get capture; SshConversationProvider retries a session once after exit code 127 by refreshing the shell profile first.
  • DependencyManager.install now refreshes the shell env both before the install command (so the installer is findable) and after (so the newly installed binary is discoverable during the re-probe).
  • DependenciesStore distinguishes user-triggered loads (refreshShellEnv: true) from background reconnect and post-install probes (refreshShellEnv: false), keeping unnecessary re-captures out of the hot path.

Confidence Score: 4/5

Safe to merge; the shell-refresh logic is well-guarded and thoroughly tested. The one thing worth a second look is how downstream subscribers to agentSessionExitedChannel react to a 127 exit that is actually followed by a silent retry.

The retry and deduplication logic is correct and each behaviour is covered by targeted tests. The only open question is UX consistency: subscribers to agentSessionExitedChannel receive an exit-127 event before the 500 ms retry fires, which could leave error indicators on screen until the retry clears them. The redundant if/else in loadAgentStatuses is a minor style matter. Neither issue affects correctness of the SSH profile refresh or install flow.

Pay closest attention to ssh-conversation.ts — specifically whether consumers of agentSessionExitedChannel need to be aware that a 127 exit may be followed by a transparent retry.

Important Files Changed

Filename Overview
src/main/core/conversations/impl/ssh-conversation.ts Splits startSession into a public wrapper and private startSessionInternal to carry a shellRefreshRetried flag; on exit code 127 (non-tmux, first attempt) schedules a shell profile refresh then retries once. agentSessionExitedChannel is emitted before the retry fires, consistent with the existing respawn pattern but may confuse subscribers that treat 127 exits as permanent.
src/main/core/ssh/lifecycle/ssh-client-proxy.ts Adds a mode field to the loading state and a shared captureRemoteShellProfileFor helper; refreshRemoteShellProfile deduplicates concurrent refresh calls, and a refresh correctly preempts an in-flight get capture by overwriting state. Logic and tests look correct.
src/main/core/dependencies/dependency-manager.ts Adds DependencyProbeOptions to probeAll and probeCategory; calls refreshShellEnv before the install command and again after a successful install (before re-probing), so the new binary is discoverable. Background initialize() call still passes no options, avoiding unneeded refreshes.
src/renderer/lib/stores/dependencies-store.ts User-triggered loads (demand Resource, probeAll) pass refreshShellEnv: true; background/post-install refreshes pass false. loadAgentStatuses has a redundant if/else branch — both arms call probeCategory identically except for the optional third argument, which could be unified into a single call.
src/main/core/dependencies/controller.ts Straightforward pass-through of DependencyProbeOptions to probeAll and probeCategory on the manager. No issues.
src/main/core/dependencies/types.ts Adds DependencyProbeOptions type with a single optional refreshShellEnv boolean. Clean and minimal.
src/renderer/lib/stores/app-state.ts Makes the onConnectionReady callback explicit about skipping shell env refresh during background reconnect probing. No issues.

Sequence Diagram

sequenceDiagram
    participant UI as DependenciesStore (UI)
    participant Ctrl as DependenciesController (main)
    participant Mgr as DependencyManager
    participant Proxy as SshClientProxy
    participant SSH as Remote SSH

    Note over UI,SSH: User-triggered remote agent discovery
    UI->>Ctrl: "probeCategory('agent', connId, {refreshShellEnv:true})"
    Ctrl->>Mgr: "probeCategory('agent', {refreshShellEnv:true})"
    Mgr->>Proxy: refreshShellEnv()
    Proxy->>SSH: "captureRemoteShellProfile (mode='refresh')"
    SSH-->>Proxy: fresh profile
    Proxy-->>Mgr: done
    Mgr->>SSH: probe each agent dependency
    SSH-->>Mgr: results
    Mgr-->>UI: statuses updated

    Note over UI,SSH: Background reconnect probe (no shell refresh)
    UI->>Ctrl: probeCategory('agent', connId) [no options]
    Ctrl->>Mgr: "probeCategory('agent', {})"
    Mgr->>SSH: probe each agent dependency
    SSH-->>Mgr: results

    Note over UI,SSH: SSH agent install
    UI->>Ctrl: install(id, connId)
    Ctrl->>Mgr: install(id)
    Mgr->>Proxy: refreshShellEnv() [before install]
    Proxy->>SSH: captureRemoteShellProfile
    SSH-->>Proxy: profile
    Mgr->>SSH: run install command
    SSH-->>Mgr: success
    Mgr->>Proxy: refreshShellEnv() [after install]
    Proxy->>SSH: captureRemoteShellProfile
    SSH-->>Proxy: updated profile
    Mgr->>SSH: re-probe dependency
    SSH-->>Mgr: available

    Note over UI,SSH: Agent launch exit 127 shell refresh retry
    UI->>SSH: startSession (via SshConversationProvider)
    SSH-->>UI: exit 127
    UI->>UI: "emit agentSessionExitedChannel (exitCode=127)"
    UI->>Proxy: refreshRemoteShellProfile()
    Proxy->>SSH: "captureRemoteShellProfile (mode='refresh')"
    SSH-->>Proxy: fresh profile
    UI->>SSH: "startSessionInternal (shellRefreshRetried=true)"
    SSH-->>UI: session running or exits again (no further retry)
Loading
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/renderer/lib/stores/dependencies-store.ts:186-191
**Redundant branch in `loadAgentStatuses`**

`rpc.dependencies.probeCategory` accepts `options` as an optional third argument; passing `undefined` is identical to omitting it. The `if (probeOptions)` branch adds dead code paths and makes the two call sites diverge syntactically for no functional reason.

```suggestion
    const probeOptions = options.refreshShellEnv ? { refreshShellEnv: true } : undefined;
    await rpc.dependencies.probeCategory('agent', connectionId, probeOptions);
```

### Issue 2 of 2
src/main/core/conversations/impl/ssh-conversation.ts:177-201
**`agentSessionExitedChannel` emitted before the 500 ms shell-refresh retry**

When the agent exits with code 127 and `shouldRetryAfterShellRefresh` is true, `agentSessionExitedChannel` is already emitted with `exitCode: 127` before the delayed retry fires. Any subscriber that reacts to this event (e.g., showing an error banner, marking the conversation as stopped, logging an error) will act on what looks like a permanent failure. 500 ms later the session silently restarts, leaving those effects stale. The existing normal-respawn path has the same pattern, but the 127 path is new and more likely to confuse automated tooling or UX that treats 127 distinctly from other unexpected exits.

Reviews (1): Last reviewed commit: "test(worktrees): remove duplicate host f..." | Re-trigger Greptile

Comment thread src/renderer/lib/stores/dependencies-store.ts Outdated
Comment thread src/main/core/conversations/impl/ssh-conversation.ts
@jschwxrz jschwxrz merged commit 844511b into main May 28, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant