Skip to content

Keep local terminal tabs on their selected context - #1963

Merged
nadaverell merged 1 commit into
mainfrom
fix/local-terminal-context-guard
Oct 2, 2026
Merged

nadaverell merged 1 commit into
mainfrom
fix/local-terminal-context-guard

Conversation

@nadaverell

@nadaverell nadaverell commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

When a local terminal opens for A, switching Radar to B can cause a later reconnect to create a B shell inside that same tab. Each tab now retains the full context selected at the opening action, and the server rejects an open or reconnect if its active context differs.

Related to #1953.

The comparison uses the same client-state snapshot as the temporary kubeconfig export and happens before WebSocket upgrade or shell startup. A rejected attempt preserves the pending first command. Existing shells keep running, reconnecting retains their context label, and returning to A allows a fresh A shell without replaying an already-sent command.

The existing terminal toolbar shows the context notice and a New terminal action; Reconnect and Retry are disabled while another context is selected. A request made before startup knows the context asks the user to retry; Desktop recovery with no kubeconfig retains an explicitly unconfirmed shell. Startup configuration errors report the context already selected by the client. Local and pod terminals retain their terminal container behind the error overlay so Retry can establish a connection.

Verification

  • Web: 1,850 tests pass; shared UI: 4,133 pass, one existing skip. Both TypeScript checks pass.
  • Full go test -p 2 ./... passed; Desktop, k8s and server packages rechecked after the final startup-status correction. Real HTTP/WebSocket/PTY tests cover missing/empty/wrong/source-qualified context, disconnected recovery, origin rejection and cleanup after a failed upgrade.
  • make build verifies the embedded frontend and binary.
  • Visual/live test: A remains on A with the same PID after switching Radar to B; stale native requests return HTTP 409 without opening a socket or adding a session; a new B shell is independent; returning to A reconnects without command replay. Retry after a deliberately rejected native handshake delivers the pending command once. Eleven inspected captures include 1280px views and the existing single 32px toolbar row. A real export-failure injection on the final build verifies an alive unconfirmed shell and accurate “Requested context differs” wording after switching the UI. The real startup UI also shows feedback with no tab/socket; restoring context updates does not replay the refused request. A recorded-intent tab seeded through the real DockProvider shows the already-mismatched mount state with zero sockets, then reconnects successfully after a real return to A; click-time capture is independently covered by integration tests.
  • The live fixture uses two private context aliases pointing to one real cluster. No physical cross-cluster isolation claim; no cluster resources were modified. Desktop no-kubeconfig recovery is covered by frontend integration and real server/PTY tests, rather than a live Desktop GUI launch.

A stays alive after Radar switches to B

Reconnect stays disabled under B and preserves A's history

The existing original/inherited kubeconfig fallback remains labelled Context not confirmed, with Requested context differs when the UI selection changes. Shell profiles, command flags and same-name physical retargeting are outside this selection guard. Native WebSocket handshake errors can remain generic when a failed switch leaves status and the active client different.

@nadaverell
nadaverell force-pushed the fix/local-terminal-context-guard branch from 674f123 to 871b3b4 Compare October 2, 2026 19:56
@nadaverell
nadaverell marked this pull request as ready for review October 2, 2026 19:56
@nadaverell
nadaverell requested a review from hisco as a code owner October 2, 2026 19:56
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Keep local terminal tabs bound to their selected context

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Capture each tab’s selected context so reconnects cannot silently open a shell for another
 context.
• Reject mismatched requests before WebSocket upgrade while preserving pending commands and existing
 shells.
• Clarify context notices and recovery behavior, with server and frontend regression tests.
Diagram

sequenceDiagram
    actor User
    participant UI as Radar UI
    participant Dock as Dock Tab
    participant Terminal as Shared Terminal
    participant Server as Terminal Server
    participant Snapshot as Kubeconfig Snapshot
    participant Shell as Local Shell
    User->>UI: Open terminal
    UI->>Dock: Store selected context
    Dock->>Terminal: Mount tab with intent
    Terminal->>Server: Connect with expected context
    Server->>Snapshot: Validate and export
    alt Context differs
        Snapshot-->>Server: Context mismatch
        Server-->>Terminal: HTTP 409
    else Context matches
        Snapshot-->>Server: Temporary kubeconfig
        Server->>Shell: Start shell
        Server-->>Terminal: Session metadata
    end
    User->>Terminal: Reconnect when permitted
Loading
High-Level Assessment

Keep the PR’s paired client-and-server guard. A UI-only check would not protect requests racing a context switch, while checking connection status separately from kubeconfig export could disagree with the state used to start the shell. Comparing against the export snapshot and retaining the tab’s intent addresses both.

Files changed (19) +558 / -67

Enhancement (1) +1 / -0
DockContext.tsxAdd local terminal context intent to dock tabs +1/-0

Add local terminal context intent to dock tabs

• Adds an optional full-context field to the shared DockTab model so the host can retain each local terminal’s opening selection.

packages/k8s-ui/src/components/dock/DockContext.tsx

Bug fix (10) +132 / -51
main.goPreserve the selected context in Desktop startup errors +1/-0

Preserve the selected context in Desktop startup errors

• Includes the active context in disconnected status when Kubernetes initialization fails, allowing recovery terminals to retain the context the client selected.

cmd/desktop/main.go

client.goValidate terminal intent against the kubeconfig snapshot +10/-3

Validate terminal intent against the kubeconfig snapshot

• Adds an optional exact expected-context check to snapshot creation, including empty and source-qualified names, and a distinct mismatch error. Existing callers can still request an unchecked snapshot.

internal/k8s/client.go

localterm.goReject stale local terminal requests before WebSocket upgrade +24/-5

Reject stale local terminal requests before WebSocket upgrade

• Requires explicit context intent and an allowed origin, returning HTTP 400, 403, or 409 before upgrade as appropriate. Reuses the validated kubeconfig snapshot for the shell and cleans up its temporary file even when upgrade fails.

internal/server/localterm.go

LocalTerminalTab.tsxGuard reconnect attempts without discarding terminal state +21/-5

Guard reconnect attempts without discarding terminal state

• Adds host-provided connection permission and error callbacks, disabling Reconnect and Retry when needed and checking permission again at attempt time. Keeps the terminal container mounted behind errors so Retry can recover.

packages/k8s-ui/src/components/dock/LocalTerminalTab.tsx

TerminalTab.tsxKeep pod terminal mounted behind connection errors +2/-4

Keep pod terminal mounted behind connection errors

• Retains the pod terminal container while showing its error overlay, allowing Retry to establish a connection without losing the mount target.

packages/k8s-ui/src/components/dock/TerminalTab.tsx

App.tsxWait for a selected context before opening a terminal +3/-2

Wait for a selected context before opening a terminal

• Disables the local-terminal action while context selection is pending and explains why in its tooltip. Disconnected recovery remains available.

web/src/App.tsx

ConnectionErrorView.tsxUse context-aware opening in connection recovery +2/-1

Use context-aware opening in connection recovery

• Routes recovery terminal actions through the web host’s context-capturing hook instead of the shared hook.

web/src/components/ConnectionErrorView.tsx

BottomDock.tsxPass each tab’s retained context to its terminal +1/-1

Pass each tab’s retained context to its terminal

• Supplies stored local-terminal context intent to the web terminal wrapper when rendering dock tabs.

web/src/components/dock/BottomDock.tsx

DockContext.tsxCapture the active context when a local tab opens +22/-2

Capture the active context when a local tab opens

• Adds a host-specific opening hook that stores the latest committed full context on each new tab. It waits during unknown-context loading but permits explicit empty intent for disconnected recovery.

web/src/components/dock/DockContext.tsx

LocalTerminalTab.tsxBind terminal requests and controls to tab intent +46/-28

Bind terminal requests and controls to tab intent

• Sends the retained context in the WebSocket URL, permits attempts only under the matching selection, and refreshes connection status after handshake errors. Preserves session labels across reconnects and distinguishes requested intent from confirmed or fallback kubeconfig state.

web/src/components/dock/LocalTerminalTab.tsx

Tests (6) +395 / -13
context_registry_test.goTest exact source-qualified context matching +7/-1

Test exact source-qualified context matching

• Verifies that a matching qualified context exports successfully while its unqualified name is rejected without producing a snapshot path.

internal/k8s/context_registry_test.go

localterm_context_unix_test.goExercise context guards through real terminal handshakes +121/-4

Exercise context guards through real terminal handshakes

• Tests missing, empty, mismatched, qualified, and cross-origin requests without starting a shell. Also covers failed-upgrade cleanup, disconnected-context recovery, and an explicitly empty-context fallback.

internal/server/localterm_context_unix_test.go

LocalTerminalTab.test.tsxTest blocked reconnects and terminal Retry recovery +42/-0

Test blocked reconnects and terminal Retry recovery

• Checks that a refused reconnect leaves terminal state and one-time command delivery intact. Verifies Retry after creation errors for both local and pod terminals.

packages/k8s-ui/src/components/dock/LocalTerminalTab.test.tsx

ConnectionErrorView.test.tsxMock the host-owned recovery terminal hook +2/-0

Mock the host-owned recovery terminal hook

• Moves the terminal-opening mock to the web dock module, matching the recovery view’s new import.

web/src/components/ConnectionErrorView.test.tsx

LocalTerminalContext.test.tsxCover context retention across terminal lifecycles +204/-0

Cover context retention across terminal lifecycles

• Exercises open-before-mount races, independent tabs, blocked reconnects, handshake failures, command delivery, missing intent, empty-context recovery, and startup errors through dock and terminal integration tests.

web/src/components/dock/LocalTerminalContext.test.tsx

LocalTerminalTab.test.tsxVerify retained labels and context notices +19/-8

Verify retained labels and context notices

• Updates wrapper tests to supply per-tab intent and checks confirmed, requested, and unconfirmed notices. Ensures reconnect metadata clearing does not erase an established label.

web/src/components/dock/LocalTerminalTab.test.tsx

Documentation (2) +30 / -3
configuration.mdDocument context-bound local terminal behavior +22/-3

Document context-bound local terminal behavior

• Explains context retention, disabled reconnects, requested and unconfirmed notices, and empty-context Desktop recovery. Clarifies that selection guarding does not constrain commands or physical cluster retargeting.

docs/configuration.md

README.mdDescribe dock intent and shared-terminal connection hooks +8/-0

Describe dock intent and shared-terminal connection hooks

• Documents the optional retained context field, the reconnect predicate, and the handshake-error callback, including the distinction between empty intent and no intent.

packages/k8s-ui/README.md

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Two terminal canvases use raw colors 📘 Rule violation ⚙ Maintainability
Description
The relocated terminal containers in LocalTerminalTab and TerminalTab use bg-[#0f172a] and
!bg-[#0f172a] instead of theme background tokens. Both modified components apply those classes to
their terminal canvases, with no documented design-spec exception for a fixed background.
Code

packages/k8s-ui/src/components/dock/LocalTerminalTab.tsx[293]

+      <div ref={terminalRef} className="absolute top-8 left-0 right-0 bottom-0 bg-[#0f172a] [&_.xterm-viewport]:!bg-[#0f172a]" />
Evidence
Rule 3036653 permits only theme background utilities unless a design-spec comment explicitly
requires a fixed background. Both changed container lines retain fixed hex-color utilities without
such a comment.

Rule 3036653: Use theme background tokens instead of hardcoded utility color classes
packages/k8s-ui/src/components/dock/LocalTerminalTab.tsx[293-293]
packages/k8s-ui/src/components/dock/TerminalTab.tsx[301-301]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Both modified terminal containers use fixed-color background utilities rather than theme tokens.
## Fix Focus Areas
- packages/k8s-ui/src/components/dock/LocalTerminalTab.tsx[293-293]
- packages/k8s-ui/src/components/dock/TerminalTab.tsx[301-301]
## Recommended Fix
Use an approved theme background token for each container and viewport. If the terminal canvas must retain a fixed color, document the design-spec exception above each element.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

2. Assistant terminal requests vanish during startup ✓ Resolved
Description
The new useOpenLocalTerminal returns without adding a tab or giving feedback when
connection.context is empty and the state is not disconnected. The radar:open-local-terminal
event from AI/diagnose actions calls this hook while the connection is still connecting with an
empty context, so nothing opens and the requested command is lost.
Code

web/src/components/dock/DockContext.tsx[R23-24]

+    const { context, state } = contextRef.current
+    if (!context && state !== 'disconnected') return
Evidence
The connection state starts as {state: 'connecting', context: ''} until the first status arrives.
In that window the event handler calls openLocalTerminal and gets no result. Before this PR, the
shared hook always added a tab.

web/src/components/dock/DockContext.tsx[16-32]
web/src/App.tsx[615-625]
web/src/context/ConnectionContext.tsx[100-106]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The web `useOpenLocalTerminal` silently returns when no context is known and the state is not disconnected. The toolbar button disables itself in that case, but DOM-event callers (AI/diagnose launch) get no feedback and lose the initial command.
## Fix Focus Areas
- web/src/components/dock/DockContext.tsx[16-32]
- web/src/App.tsx[618-625]
## Recommended Fix
Pick one:
- Queue the request until a context (or the disconnected state) arrives.
- Have the hook return a boolean and show a toast when it refuses.
- Add the tab without a context so it shows the existing "No context selected" notice and its New terminal action.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread packages/k8s-ui/src/components/dock/LocalTerminalTab.tsx
Comment thread web/src/components/dock/DockContext.tsx Outdated
@nadaverell
nadaverell force-pushed the fix/local-terminal-context-guard branch from 871b3b4 to 5dc4329 Compare October 2, 2026 20:06
@nadaverell
nadaverell merged commit b926d09 into main Oct 2, 2026
10 checks 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