Skip to content

Add Stop, Restart, and Force Rebuild menu items to Tsunami view #2483

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sawka merged 6 commits into from
Oct 27, 2025
Merged

Add Stop, Restart, and Force Rebuild menu items to Tsunami view #2483

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f3482ca
Initial plan
Copilot Oct 27, 2025
9ce1e57
Add Restart and Force Rebuild menu items to Tsunami view
Copilot Oct 27, 2025
de00662
Refactor to reduce code duplication in controller resync methods
Copilot Oct 27, 2025
cd01857
Further refactor resyncController to use common helper method
Copilot Oct 27, 2025
f889657
add WaveApp to labels
sawka Oct 27, 2025
9cee89a
Add Stop WaveApp menu item and implement proper restart logic
Copilot Oct 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 64 additions & 11 deletions frontend/app/view/tsunami/tsunami.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,26 +72,58 @@ class TsunamiViewModel extends WebViewModel {
}, 300);
}

resyncController() {
private doControllerResync(forceRestart: boolean, logContext: string, triggerRestart: boolean = true) {
if (triggerRestart) {
if (globalStore.get(this.isRestarting)) {
return;
}
this.triggerRestartAtom();
}
const prtn = RpcApi.ControllerResyncCommand(TabRpcClient, {
tabid: globalStore.get(atoms.staticTabId),
blockid: this.blockId,
forcerestart: false,
forcerestart: forceRestart,
});
prtn.catch((e) => console.log("error controller resync", e));
prtn.catch((e) => console.log(`error controller resync (${logContext})`, e));
}
Comment on lines +75 to 88
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Return a Promise from doControllerResync and propagate errors

Callers can’t await the resync; errors are only logged. Make it async, return the underlying Promise, and rethrow to let callers handle.

-    private doControllerResync(forceRestart: boolean, logContext: string, triggerRestart: boolean = true) {
+    private async doControllerResync(
+        forceRestart: boolean,
+        logContext: string,
+        triggerRestart: boolean = true
+    ): Promise<void> {
         if (triggerRestart) {
             if (globalStore.get(this.isRestarting)) {
                 return;
             }
             this.triggerRestartAtom();
         }
-        const prtn = RpcApi.ControllerResyncCommand(TabRpcClient, {
+        return RpcApi.ControllerResyncCommand(TabRpcClient, {
             tabid: globalStore.get(atoms.staticTabId),
             blockid: this.blockId,
             forcerestart: forceRestart,
-        });
-        prtn.catch((e) => console.log(`error controller resync (${logContext})`, e));
+        }).catch((e) => {
+            console.log(`error controller resync (${logContext})`, e);
+            throw e;
+        });
     }

If RpcApi supports RpcOpts timeouts, consider adding a bounded timeout. Please confirm with:

🏁 Script executed:

#!/bin/bash
rg -n -C3 "ControllerResyncCommand\\s*\\(" \
  && rg -n -C3 "type\\s+RpcOpts|interface\\s+RpcOpts|export\\s+type\\s+RpcOpts"

Length of output: 5675

Return a Promise from doControllerResync and propagate errors

The underlying RpcApi.ControllerResyncCommand already returns Promise<void>, but this method doesn't return or await it. Make the method async to return the Promise and rethrow errors so callers can handle them. Additionally, callers should pass RpcOpts with a timeout for robustness.

-    private doControllerResync(forceRestart: boolean, logContext: string, triggerRestart: boolean = true) {
+    private async doControllerResync(
+        forceRestart: boolean,
+        logContext: string,
+        triggerRestart: boolean = true
+    ): Promise<void> {
         if (triggerRestart) {
             if (globalStore.get(this.isRestarting)) {
                 return;
             }
             this.triggerRestartAtom();
         }
-        const prtn = RpcApi.ControllerResyncCommand(TabRpcClient, {
+        return RpcApi.ControllerResyncCommand(TabRpcClient, {
             tabid: globalStore.get(atoms.staticTabId),
             blockid: this.blockId,
             forcerestart: forceRestart,
-        });
-        prtn.catch((e) => console.log(`error controller resync (${logContext})`, e));
+        }).catch((e) => {
+            console.log(`error controller resync (${logContext})`, e);
+            throw e;
+        });
     }

Update all call sites (e.g., restartAndForceRebuild, restartController) to await the result. Consider adding rtopts parameter with timeout.

🤖 Prompt for AI Agents 
In frontend/app/view/tsunami/tsunami.tsx around lines 75 to 88,
doControllerResync currently calls RpcApi.ControllerResyncCommand but neither
returns nor awaits its Promise; change the method signature to async and have it
return (or await and return) the RpcApi.ControllerResyncCommand Promise so
errors propagate to callers; rethrow errors instead of just logging (or let the
Promise reject) so callers can handle them; add an optional rtopts: RpcOpts
parameter (with a default timeout) and pass it into ControllerResyncCommand;
finally update all call sites (e.g., restartAndForceRebuild, restartController)
to await doControllerResync and handle/rethrow errors as appropriate.


forceRestartController() {
resyncController() {
this.doControllerResync(false, "resync", false);
}

stopController() {
const prtn = RpcApi.ControllerStopCommand(TabRpcClient, this.blockId);
prtn.catch((e) => console.log("error stopping controller", e));
}

Comment on lines +94 to +98
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Stop should be awaitable; verify payload shape

Expose an async API and rethrow on failure so callers (and tests) can react. Also verify whether stop expects blockId or an object with tabid/blockid.

-    stopController() {
-        const prtn = RpcApi.ControllerStopCommand(TabRpcClient, this.blockId);
-        prtn.catch((e) => console.log("error stopping controller", e));
-    }
+    async stopController() {
+        try {
+            await RpcApi.ControllerStopCommand(TabRpcClient, this.blockId);
+        } catch (e) {
+            console.log("error stopping controller", e);
+            throw e;
+        }
+    }

Run to confirm expected signature:

🏁 Script executed:

#!/bin/bash
# Locate definition and usages to confirm the data argument.
rg -n -C3 "ControllerStopCommand\\s*\\("

Length of output: 3317

Make stopController() async and properly handle errors to allow caller reaction

The verification confirms the concern: RpcApi.ControllerStopCommand returns Promise<void> and is already awaitable, but the current implementation silently swallows errors. Notably, line 106 in the same file already demonstrates the correct pattern with await RpcApi.ControllerStopCommand(...) in a try block. The payload shape is correct—this.blockId is a string as expected by the backend.

Apply the suggested refactor to match the established pattern elsewhere in the codebase:

-    stopController() {
-        const prtn = RpcApi.ControllerStopCommand(TabRpcClient, this.blockId);
-        prtn.catch((e) => console.log("error stopping controller", e));
-    }
+    async stopController() {
+        try {
+            await RpcApi.ControllerStopCommand(TabRpcClient, this.blockId);
+        } catch (e) {
+            console.log("error stopping controller", e);
+            throw e;
+        }
+    }
🤖 Prompt for AI Agents 
In frontend/app/view/tsunami/tsunami.tsx around lines 94 to 98, change
stopController to be an async method, await
RpcApi.ControllerStopCommand(TabRpcClient, this.blockId) inside a try block,
catch any thrown error, log it with contextual message, and rethrow the error so
the caller can react to failures (matching the pattern used at line 106).

async restartController() {
if (globalStore.get(this.isRestarting)) {
return;
}
this.triggerRestartAtom();
const prtn = RpcApi.ControllerResyncCommand(TabRpcClient, {
tabid: globalStore.get(atoms.staticTabId),
blockid: this.blockId,
forcerestart: true,
});
prtn.catch((e) => console.log("error controller resync (force restart)", e));
try {
// Stop the controller first
await RpcApi.ControllerStopCommand(TabRpcClient, this.blockId);
// Wait a bit for the controller to fully stop
await new Promise((resolve) => setTimeout(resolve, 300));
// Then resync to restart it
await RpcApi.ControllerResyncCommand(TabRpcClient, {
tabid: globalStore.get(atoms.staticTabId),
blockid: this.blockId,
forcerestart: false,
});
} catch (e) {
console.log("error restarting controller", e);
}
}
Comment on lines +99 to +118
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Duplicate restart risk and brittle 300ms UI gating; use best‑effort stop and an in‑flight guard

isRestarting resets after 300ms, allowing overlapping restarts and misleading UI. Add a restartInFlight guard, keep UI pulse via triggerRestartAtom, proceed to resync even if stop says “already stopped,” and allow force rebuild reuse.

-    async restartController() {
-        if (globalStore.get(this.isRestarting)) {
-            return;
-        }
-        this.triggerRestartAtom();
-        try {
-            // Stop the controller first
-            await RpcApi.ControllerStopCommand(TabRpcClient, this.blockId);
-            // Wait a bit for the controller to fully stop
-            await new Promise((resolve) => setTimeout(resolve, 300));
-            // Then resync to restart it
-            await RpcApi.ControllerResyncCommand(TabRpcClient, {
-                tabid: globalStore.get(atoms.staticTabId),
-                blockid: this.blockId,
-                forcerestart: false,
-            });
-        } catch (e) {
-            console.log("error restarting controller", e);
-        }
-    }
+    async restartController(forceRebuild: boolean = false) {
+        if (this.restartInFlight) return;
+        this.restartInFlight = true;
+        this.triggerRestartAtom();
+        try {
+            // Best‑effort stop; continue even if already stopped.
+            try {
+                await RpcApi.ControllerStopCommand(TabRpcClient, this.blockId);
+            } catch (e) {
+                console.log("controller stop failed (continuing to resync)", e);
+            }
+            await new Promise((resolve) => setTimeout(resolve, 300));
+            await RpcApi.ControllerResyncCommand(TabRpcClient, {
+                tabid: globalStore.get(atoms.staticTabId),
+                blockid: this.blockId,
+                forcerestart: forceRebuild,
+            });
+        } catch (e) {
+            console.log("error restarting controller", e);
+        } finally {
+            this.restartInFlight = false;
+        }
+    }

Add this field to the class (outside this hunk):

// Near other fields in TsunamiViewModel
private restartInFlight = false;
🤖 Prompt for AI Agents 
In frontend/app/view/tsunami/tsunami.tsx around lines 99 to 118, the restart
flow uses a brittle 300ms UI gating and can allow overlapping restarts; add a
private restartInFlight = false field on the TsunamiViewModel and update
restartController to return early if restartInFlight is true, set
restartInFlight = true at start and reset it in a finally block, keep calling
triggerRestartAtom() to preserve the UI pulse, attempt
RpcApi.ControllerStopCommand but treat failures (including “already stopped”) as
non-fatal and proceed to call RpcApi.ControllerResyncCommand regardless of stop
result, and remove the fixed 300ms timeout so you wait only for the stop Promise
(best‑effort) before resyncing.


restartAndForceRebuild() {
this.doControllerResync(true, "force rebuild");
}

Comment on lines +120 to +123
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Menu label says “Restart … and Force Rebuild” but code doesn’t stop first

Current implementation is a resync-only path. Align behavior with the label by reusing restartController(true).

-    restartAndForceRebuild() {
-        this.doControllerResync(true, "force rebuild");
-    }
+    async restartAndForceRebuild() {
+        await this.restartController(true);
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
restartAndForceRebuild() {
this.doControllerResync(true, "force rebuild");
}
async restartAndForceRebuild() {
await this.restartController(true);
}
🤖 Prompt for AI Agents 
In frontend/app/view/tsunami/tsunami.tsx around lines 120 to 123, the
restartAndForceRebuild method currently calls doControllerResync(true, "force
rebuild") which only resyncs; change it to call restartController(true) so the
controller is stopped and restarted with the force-rebuild flag. Replace the
doControllerResync call with restartController(true), preserving any
return/await behavior and updating any callers if the method signature/return
differs.

forceRestartController() {
// Keep this for backward compatibility with the Start button
this.doControllerResync(true, "force restart");
}

setAppMeta(meta: TsunamiAppMeta) {
Expand Down Expand Up @@ -125,7 +157,7 @@ class TsunamiViewModel extends WebViewModel {
getSettingsMenuItems(): ContextMenuItem[] {
const items = super.getSettingsMenuItems();
// Filter out homepage and navigation-related menu items for tsunami view
return items.filter((item) => {
const filteredItems = items.filter((item) => {
const label = item.label?.toLowerCase() || "";
return (
!label.includes("homepage") &&
Expand All @@ -134,6 +166,27 @@ class TsunamiViewModel extends WebViewModel {
!label.includes("nav")
);
});

// Add tsunami-specific menu items at the beginning
const tsunamiItems: ContextMenuItem[] = [
{
label: "Stop WaveApp",
click: () => this.stopController(),
},
{
label: "Restart WaveApp",
click: () => this.restartController(),
},
{
label: "Restart WaveApp and Force Rebuild",
click: () => this.restartAndForceRebuild(),
},
{
type: "separator",
},
];

return [...tsunamiItems, ...filteredItems];
}
}

Expand Down
Loading