More Durable Shell Bug Fixes (#2822)

* Lots of work on ResyncController, issues with stale data, and
connection switching
* Add+Fix termination messages for shell controller
* Prune done/detached jobs on a timer
* Remove circular dependencies from wcore, use blockclose event in both
jobcontroller and blockcontroller instead of direct calls
* Fix concurrency bugs with job termination
* Send object update events when modifying the block datastructure in
jobcontroller

Author: sawka

cmd/server/main-server.go

@@ -568,6 +568,7 @@ func main() {
 	go startupActivityUpdate(firstLaunch) // must be after startConfigWatcher()
 	blocklogger.InitBlockLogger()
 	jobcontroller.InitJobController()
+	blockcontroller.InitBlockController()
 	wcore.InitTabIndicatorStore()
 	go func() {
 		defer func() {

cmd/wsh/cmd/wshcmd-jobdebug.go

@@ -95,6 +95,12 @@ var jobDebugDetachJobCmd = &cobra.Command{
 	RunE:  jobDebugDetachJobRun,
 }
 
+var jobDebugBlockAttachmentCmd = &cobra.Command{
+	Use:   "blockattachment",
+	Short: "show the attached job for a block",
+	RunE:  jobDebugBlockAttachmentRun,
+}
+
 var jobIdFlag string
 var jobDebugJsonFlag bool
 var jobConnFlag string
@@ -120,6 +126,7 @@ func init() {
 	jobDebugCmd.AddCommand(jobDebugStartCmd)
 	jobDebugCmd.AddCommand(jobDebugAttachJobCmd)
 	jobDebugCmd.AddCommand(jobDebugDetachJobCmd)
+	jobDebugCmd.AddCommand(jobDebugBlockAttachmentCmd)
 
 	jobDebugListCmd.Flags().BoolVar(&jobDebugJsonFlag, "json", false, "output as JSON")
 
@@ -418,3 +425,23 @@ func jobDebugDetachJobRun(cmd *cobra.Command, args []string) error {
 	fmt.Printf("Job %s detached successfully\n", detachJobIdFlag)
 	return nil
 }
+
+func jobDebugBlockAttachmentRun(cmd *cobra.Command, args []string) error {
+	blockORef, err := resolveBlockArg()
+	if err != nil {
+		return err
+	}
+
+	blockId := blockORef.OID
+	jobStatus, err := wshclient.BlockJobStatusCommand(RpcClient, blockId, &wshrpc.RpcOpts{Timeout: 5000})
+	if err != nil {
+		return fmt.Errorf("getting block job status: %w", err)
+	}
+
+	if jobStatus.JobId == "" {
+		fmt.Printf("Block %s: no attached job\n", blockId)
+	} else {
+		fmt.Printf("Block %s: attached to job %s\n", blockId, jobStatus.JobId)
+	}
+	return nil
+}

frontend/app/block/blockframe-header.tsx

@@ -23,9 +23,18 @@ import * as jotai from "jotai";
 import * as React from "react";
 import { BlockFrameProps } from "./blocktypes";
 
-function getDurableIconProps(jobStatus: BlockJobStatusData, connStatus: ConnStatus) {
+function getDurableIconProps(jobStatus: BlockJobStatusData, connStatus: ConnStatus, isConfigedDurable?: boolean | null) {
     let color = "text-muted";
     let titleText = "Durable Session";
+    let iconType: "fa-solid" | "fa-regular" = "fa-solid";
+    
+    if (isConfigedDurable === false) {
+        color = "text-muted";
+        titleText = "Standard Session";
+        iconType = "fa-regular";
+        return { color, titleText, iconType };
+    }
+    
     const status = jobStatus?.status;
     if (status === "connected") {
         color = "text-sky-500";
@@ -57,7 +66,7 @@ function getDurableIconProps(jobStatus: BlockJobStatusData, connStatus: ConnStat
             titleText = "No Session";
         }
     }
-    return { color, titleText };
+    return { color, titleText, iconType };
 }
 
 function handleHeaderContextMenu(
@@ -209,6 +218,7 @@ const BlockFrame_Header = ({
     const preIconButton = util.useAtomValueSafe(viewModel?.preIconButton);
     const useTermHeader = util.useAtomValueSafe(viewModel?.useTermHeader);
     const termDurableStatus = util.useAtomValueSafe(viewModel?.termDurableStatus);
+    const termConfigedDurable = util.useAtomValueSafe(viewModel?.termConfigedDurable);
     const hideViewName = util.useAtomValueSafe(viewModel?.hideViewName);
     const magnified = jotai.useAtomValue(nodeModel.isMagnified);
     const prevMagifiedState = React.useRef(magnified);
@@ -230,7 +240,11 @@ const BlockFrame_Header = ({
 
     const viewIconElem = getViewIconElem(viewIconUnion, blockData);
 
-    const { color: durableIconColor, titleText: durableTitle } = getDurableIconProps(termDurableStatus, connStatus);
+    const { color: durableIconColor, titleText: durableTitle, iconType: durableIconType } = getDurableIconProps(
+        termDurableStatus,
+        connStatus,
+        termConfigedDurable
+    );
 
     return (
         <div
@@ -257,9 +271,9 @@ const BlockFrame_Header = ({
                     isTerminalBlock={isTerminalBlock}
                 />
             )}
-            {useTermHeader && termDurableStatus != null && (
+            {useTermHeader && termConfigedDurable != null && (
                 <div className="iconbutton disabled text-[13px] ml-[-4px]" key="durable-status">
-                    <i className={`fa-sharp fa-solid fa-shield ${durableIconColor}`} title={durableTitle} />
+                    <i className={`fa-sharp ${durableIconType} fa-shield ${durableIconColor}`} title={durableTitle} />
                 </div>
             )}
             <HeaderTextElems viewModel={viewModel} blockData={blockData} preview={preview} error={error} />

frontend/app/store/global.ts

@@ -425,8 +425,10 @@ function getSingleOrefAtomCache(oref: string): Map<string, Atom<any>> {
     return orefCache;
 }
 
-// this function must be kept up to date with IsBlockTermDurable in pkg/jobcontroller/jobcontroller.go
-function getBlockTermDurableAtom(blockId: string): Atom<boolean> {
+// this function should be kept up to date with IsBlockTermDurable in pkg/jobcontroller/jobcontroller.go
+// Note: null/false both map to false in the Go code, but this returns a special null value
+// to indicate when the block is not even eligible to be durable
+function getBlockTermDurableAtom(blockId: string): Atom<null | boolean> {
     const blockCache = getSingleBlockAtomCache(blockId);
     const durableAtomName = "#termdurable";
     let durableAtom = blockCache.get(durableAtomName);
@@ -438,23 +440,23 @@ function getBlockTermDurableAtom(blockId: string): Atom<boolean> {
         const block = get(blockAtom);
 
         if (block == null) {
-            return false;
+            return null;
         }
 
         // Check if view is "term", and controller is "shell"
         if (block.meta?.view != "term" || block.meta?.controller != "shell") {
-            return false;
+            return null;
         }
 
         // 1. Check if block has a JobId
         if (block.jobid != null && block.jobid != "") {
             return true;
         }
 
-        // 2. Check if connection is local or WSL (not durable)
+        // 2. Check if connection is local or WSL (not eligible for durability)
         const connName = block.meta?.connection ?? "";
         if (isLocalConnName(connName) || isWslConnName(connName)) {
-            return false;
+            return null;
         }
 
         // 3. Check config hierarchy: blockmeta → connection → global (default true)

frontend/app/view/term/term-model.ts

@@ -35,7 +35,7 @@ import {
 import * as services from "@/store/services";
 import * as keyutil from "@/util/keyutil";
 import { isMacOS, isWindows } from "@/util/platformutil";
-import { boundNumber, stringToBase64 } from "@/util/util";
+import { boundNumber, fireAndForget, stringToBase64 } from "@/util/util";
 import * as jotai from "jotai";
 import * as React from "react";
 import { getBlockingCommand } from "./shellblocking";
@@ -79,6 +79,7 @@ export class TermViewModel implements ViewModel {
     isCmdController: jotai.Atom<boolean>;
     isRestarting: jotai.PrimitiveAtom<boolean>;
     termDurableStatus: jotai.Atom<BlockJobStatusData | null>;
+    termConfigedDurable: jotai.Atom<null | boolean>;
     searchAtoms?: SearchAtoms;
 
     constructor(blockId: string, nodeModel: BlockNodeModel, tabModel: TabModel) {
@@ -312,7 +313,7 @@ export class TermViewModel implements ViewModel {
                 const buttonDecl: IconButtonDecl = {
                     elemtype: "iconbutton",
                     icon: iconName,
-                    click: this.forceRestartController.bind(this),
+                    click: () => fireAndForget(() => this.forceRestartController()),
                     title: title,
                 };
                 rtn.push(buttonDecl);
@@ -351,6 +352,7 @@ export class TermViewModel implements ViewModel {
             }
             return blockJobStatus;
         });
+        this.termConfigedDurable = getBlockTermDurableAtom(this.blockId);
         this.blockJobStatusAtom = jotai.atom(null) as jotai.PrimitiveAtom<BlockJobStatusData>;
         this.blockJobStatusVersionTs = 0;
         const initialBlockJobStatus = RpcApi.BlockJobStatusCommand(TabRpcClient, blockId);
@@ -695,7 +697,7 @@ export class TermViewModel implements ViewModel {
         }
         const shellProcStatus = globalStore.get(this.shellProcStatus);
         if ((shellProcStatus == "done" || shellProcStatus == "init") && keyutil.checkKeyPressed(waveEvent, "Enter")) {
-            this.forceRestartController();
+            fireAndForget(() => this.forceRestartController());
             return false;
         }
         const appHandled = appHandleKeyDown(waveEvent);
@@ -714,28 +716,28 @@ export class TermViewModel implements ViewModel {
         });
     }
 
-    forceRestartController() {
+    async forceRestartController() {
         if (globalStore.get(this.isRestarting)) {
             return;
         }
         this.triggerRestartAtom();
+        await RpcApi.ControllerDestroyCommand(TabRpcClient, this.blockId);
         const termsize = {
             rows: this.termRef.current?.terminal?.rows,
             cols: this.termRef.current?.terminal?.cols,
         };
-        const prtn = RpcApi.ControllerResyncCommand(TabRpcClient, {
+        await RpcApi.ControllerResyncCommand(TabRpcClient, {
             tabid: globalStore.get(atoms.staticTabId),
             blockid: this.blockId,
             forcerestart: true,
             rtopts: { termsize: termsize },
         });
-        prtn.catch((e) => console.log("error controller resync (force restart)", e));
     }
 
-    async restartSessionInStandardMode() {
+    async restartSessionWithDurability(isDurable: boolean) {
         await RpcApi.SetMetaCommand(TabRpcClient, {
             oref: WOS.makeORef("block", this.blockId),
-            meta: { "term:durable": false },
+            meta: { "term:durable": isDurable },
         });
         await RpcApi.ControllerDestroyCommand(TabRpcClient, this.blockId);
         const termsize = {
@@ -1040,7 +1042,7 @@ export class TermViewModel implements ViewModel {
         });
         advancedSubmenu.push({
             label: "Force Restart Controller",
-            click: this.forceRestartController.bind(this),
+            click: () => fireAndForget(() => this.forceRestartController()),
         });
         const isClearOnStart = blockData?.meta?.["cmd:clearonstart"];
         advancedSubmenu.push({
@@ -1145,7 +1147,17 @@ export class TermViewModel implements ViewModel {
                 submenu: [
                     {
                         label: "Restart Session in Standard Mode",
-                        click: () => this.restartSessionInStandardMode(),
+                        click: () => this.restartSessionWithDurability(false),
+                    },
+                ],
+            });
+        } else if (isDurable === false) {
+            advancedSubmenu.push({
+                label: "Session Durability",
+                submenu: [
+                    {
+                        label: "Restart Session in Durable Mode",
+                        click: () => this.restartSessionWithDurability(true),
                     },
                 ],
             });

frontend/types/custom.d.ts

@@ -312,6 +312,7 @@ declare global {
         viewText?: jotai.Atom<string | HeaderElem[]>;
 
         termDurableStatus?: jotai.Atom<BlockJobStatusData | null>;
+        termConfigedDurable?: jotai.Atom<null | boolean>;
 
         // Icon button displayed before the title in the header.
         preIconButton?: jotai.Atom<IconButtonDecl>;

pkg/blockcontroller/blockcontroller.go

@@ -23,6 +23,7 @@ import (
 	"github.com/wavetermdev/waveterm/pkg/wavebase"
 	"github.com/wavetermdev/waveterm/pkg/waveobj"
 	"github.com/wavetermdev/waveterm/pkg/wps"
+	"github.com/wavetermdev/waveterm/pkg/wshrpc/wshclient"
 	"github.com/wavetermdev/waveterm/pkg/wslconn"
 	"github.com/wavetermdev/waveterm/pkg/wstore"
 )
@@ -118,6 +119,24 @@ func getAllControllers() map[string]Controller {
 	return result
 }
 
+func InitBlockController() {
+	rpcClient := wshclient.GetBareRpcClient()
+	rpcClient.EventListener.On(wps.Event_BlockClose, handleBlockCloseEvent)
+	wshclient.EventSubCommand(rpcClient, wps.SubscriptionRequest{
+		Event:     wps.Event_BlockClose,
+		AllScopes: true,
+	}, nil)
+}
+
+func handleBlockCloseEvent(event *wps.WaveEvent) {
+	blockId, ok := event.Data.(string)
+	if !ok {
+		log.Printf("[blockclose] invalid event data type")
+		return
+	}
+	go DestroyBlockController(blockId)
+}
+
 // Public API Functions
 
 func ResyncController(ctx context.Context, tabId string, blockId string, rtOpts *waveobj.RuntimeOpts, force bool) error {
@@ -131,10 +150,22 @@ func ResyncController(ctx context.Context, tabId string, blockId string, rtOpts
 	}
 
 	controllerName := blockData.Meta.GetString(waveobj.MetaKey_Controller, "")
+	connName := blockData.Meta.GetString(waveobj.MetaKey_Connection, "")
 
 	// Get existing controller
 	existing := getController(blockId)
 
+	// Check for connection change FIRST - always destroy on conn change
+	if existing != nil {
+		existingStatus := existing.GetRuntimeStatus()
+		if existingStatus.ShellProcConnName != connName {
+			log.Printf("stopping blockcontroller %s due to conn change (from %q to %q)\n", blockId, existingStatus.ShellProcConnName, connName)
+			DestroyBlockController(blockId)
+			time.Sleep(100 * time.Millisecond)
+			existing = nil
+		}
+	}
+
 	// If no controller needed, stop existing if present
 	if controllerName == "" {
 		if existing != nil {
@@ -144,12 +175,10 @@ func ResyncController(ctx context.Context, tabId string, blockId string, rtOpts
 	}
 
 	// Determine if we should use DurableShellController vs ShellController
-	connName := blockData.Meta.GetString(waveobj.MetaKey_Connection, "")
-	shouldUseDurableShellController := jobcontroller.IsBlockTermDurable(blockData) && controllerName == BlockController_Shell
+	shouldUseDurableShellController := controllerName == BlockController_Shell && jobcontroller.IsBlockIdTermDurable(blockId)
 
 	// Check if we need to morph controller type
 	if existing != nil {
-		existingStatus := existing.GetRuntimeStatus()
 		needsReplace := false
 
 		switch existing.(type) {
@@ -175,19 +204,6 @@ func ResyncController(ctx context.Context, tabId string, blockId string, rtOpts
 			time.Sleep(100 * time.Millisecond)
 			existing = nil
 		}
-
-		// For shell/cmd, check if connection changed (but not for job controller)
-		if !needsReplace && (controllerName == BlockController_Shell || controllerName == BlockController_Cmd) {
-			if _, isShellController := existing.(*ShellController); isShellController {
-				// Check if connection changed, including between different local connections
-				if existingStatus.ShellProcStatus == Status_Running && existingStatus.ShellProcConnName != connName {
-					log.Printf("stopping blockcontroller %s due to conn change (from %q to %q)\n", blockId, existingStatus.ShellProcConnName, connName)
-					DestroyBlockController(blockId)
-					time.Sleep(100 * time.Millisecond)
-					existing = nil
-				}
-			}
-		}
 	}
 
 	// Force restart if requested
@@ -237,7 +253,6 @@ func ResyncController(ctx context.Context, tabId string, blockId string, rtOpts
 	if status.ShellProcStatus == Status_Init {
 		// For shell/cmd, check connection status first (for non-local connections)
 		if controllerName == BlockController_Shell || controllerName == BlockController_Cmd {
-			connName := blockData.Meta.GetString(waveobj.MetaKey_Connection, "")
 			if !conncontroller.IsLocalConnName(connName) {
 				err = CheckConnStatus(blockId)
 				if err != nil {