Skip to content

Compute the shell environment once per app run instead of respawning the login shell for every terminal tab #2378

Description

@dex4er

Problem

Spawning the user's login shell is the most expensive and most conspicuous thing
Freelens does to resolve PATH (see #2377 and #2376 for the EDR side of it). It
currently happens far more often than it needs to, along two independent paths that do
not know about each other.

At startup, shell sync runs the login shell and merges the result into the main
process's global process.env
(setup-shell.injectable.ts#L21-L64).

Per terminal, ShellSession computes its own copy through
computeShellEnvironment
(shell-session.ts#L386-L393),
so the very first terminal opened respawns the login shell even though the same
computation finished seconds earlier at startup.

There is a cache, but it stores the wrong layer and refreshes on the wrong event
(shell-session.ts#L362-L379):

let env = this.dependencies.shellSessionEnvs.get(cacheKey);

if (!env) {
  env = await this.getShellEnv({ reportStatus: true });   // miss: spawns the shell
  this.dependencies.shellSessionEnvs.set(cacheKey, env);
} else {
  // refresh env in the background, silently: the shell is already running
  // and its prompt must not be written over
  this.getShellEnv().then((shellEnv: any) => {
    this.dependencies.shellSessionEnvs.set(cacheKey, shellEnv);   // hit: ALSO spawns the shell
  });
}

So a cache hit spawns the login shell too, just without blocking. Opening ten terminal
tabs in a cluster spawns ten login shells, each running the user's full rc chain.

The cache is also keyed per cluster
(shell-envs.injectable.ts,
key cluster.id or standaloneSessionId), which multiplies the misses by the number of
clusters, even though the login shell's own environment has nothing to do with which
cluster the terminal belongs to.

Proposed solution

The two layers being conflated are:

  • the shell environment — the output of computeShellEnvironment, identical for
    every session on the machine for a given $SHELL, and the only part that costs a
    process spawn;
  • the session environment — that, plus the cluster's kubectl directory, the proxy
    kubeconfig, PATH prefixes/suffixes, PTYSHELL and friends
    (shell-session.ts#L408-L435),
    which is pure, cheap, and genuinely per-session.

Cache the first, derive the second:

  1. Introduce a single shared holder for the computed shell environment in the main
    process — one injectable owning { shell, env, computedAt }, with an explicit
    get() that computes on first call and returns the memoized value afterwards.
  2. Have shell sync at startup populate that holder rather than only mutating
    process.env, and keep the process.env merge exactly as it is today. Consumers of
    the global env are implicit — the only declared ordering dependency is
    runAfter: setupShellInjectable in
    initialize-extensions.injectable.ts#L54
    — so this issue should not change when the startup sync runs or what it mutates.
  3. Have ShellSession.getShellEnv read the shared holder instead of calling
    computeShellEnvironment directly, then layer the session-specific parts on top.
    The per-cluster shellSessionEnvs cache can then either stay as a cheap derived
    cache or go away entirely, since deriving becomes pure computation.
  4. Replace refresh-on-every-hit with explicit invalidation:
    • the user changes the shell preference (userShellSetting),
    • a staleness threshold measured from computedAt (a few minutes, so a long-running
      app still picks up rc-file edits),
    • optionally the mtime of the shell's rc files,
    • optionally a user-visible "reload shell environment" action.

The result is one login shell per app run in the normal case, instead of one per
terminal tab, with a defined and documented rule for when it runs again.

Notes and risks

  • Behaviour to preserve: today's refresh-on-hit means a user who edits ~/.zshrc sees
    the new PATH in the next terminal tab. A pure memoization without any staleness
    rule would require restarting the app, which is a regression for that workflow —
    hence point 4 rather than a plain cache.
  • The status reporting distinction must survive: a cache miss reports
    "Resolving shell environment ..." to the terminal, while a background refresh stays
    silent so it does not write over a running prompt
    (shell-session.ts#L381-L391).
  • Concurrency: two terminals opened at once must share one in-flight computation rather
    than starting two, so the holder should memoize the promise, not just the result.
  • Failure handling stays as it is — a failed computation falls back to process.env
    and, at startup, emits the shell-sync failure notification. A failure should not be
    memoized as if it were a valid environment.
  • computeShellEnvironment is a no-op on Windows, so the shared holder just carries
    undefined there and nothing changes.

Acceptance criteria

  • Opening several terminal tabs, in one cluster and across clusters, spawns the login
    shell at most once per app run under the default refresh policy.
  • The startup shell sync and the first terminal session no longer compute the
    environment twice.
  • Editing the shell's rc files is still picked up without restarting the app, by
    whichever invalidation rule is chosen, and that rule is documented.
  • Two terminals opened simultaneously share one computation.
  • Existing shell session tests pass, plus a new test asserting the number of
    computeShellEnvironment calls across multiple session opens.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions