fix: DH-21945: listVariables and openVariablePanels now use right connection (#309)

DH-21945: Most of the MCP tools can handle an agent passing a DHE server
URL instead of a worker URL. There was a subtle bug with listVariables
and openVariablePanels where the call would succeed but found zero
variables due to checking the server URL.

I also added a better error message for panelUrls missing on earlier DHE
servers

### Testing
#### List Fixes
- Connect to a DHE server. And create at least 1 table on the worker
- Ask the agent to call `listVariables` tool telling it to use the
server URL explicitly instead of the worker URL. It should find it.
Before it would get zero results
- Same general test for `openVariablePanels` but it should open the
panel in VS Code

#### Panel URL message
- Connect to a grizzly server and create at least 1 table on the worker
- Ask agent about the panelUrl for the table. It should see a message
saying it isn't supported instead of speculating why it isn't there

Author: bmingles

skills/deephaven-vscode-using/SKILL.md

@@ -69,8 +69,10 @@ When choosing from multiple servers in `listServers` response, look for:
 
 **Panel URLs (for UI verification):**
 
-- **DHC**: `{origin}/iframe/widget/?name={variableTitle}`
-- **DHE**: `{origin}/iriside/embed/widget/serial/{serial}/{variableTitle}`
+Community servers always support panel URLs. Enterprise servers require Grizzly+ or later.
+
+- **DHC (Community)**: `{origin}/iframe/widget/?name={variableTitle}`
+- **DHE (Enterprise)**: `{origin}/iriside/embed/widget/serial/{serial}/{variableTitle}` (Grizzly+ or later only)
 
 ### Remote File Sources
 

src/mcp/tools/listVariables.spec.ts

@@ -76,9 +76,13 @@ describe('listVariables', () => {
       variables: [],
     },
   ])('should list panel variables: $label', async ({ variables }) => {
+    const mockConnection = {
+      serverUrl: MOCK_PARSED_URL,
+    } as IDhcService;
+
     vi.mocked(getFirstConnectionOrCreate).mockResolvedValue({
       success: true,
-      connection: {} as IDhcService,
+      connection: mockConnection,
       panelUrlFormat: MOCK_PANEL_URL_FORMAT,
     });
     vi.mocked(panelService.getVariables).mockReturnValue(variables);
@@ -93,7 +97,9 @@ describe('listVariables', () => {
       connectionUrl: MOCK_PARSED_URL,
       serverManager,
     });
-    expect(panelService.getVariables).toHaveBeenCalledWith(MOCK_PARSED_URL);
+    expect(panelService.getVariables).toHaveBeenCalledWith(
+      mockConnection.serverUrl
+    );
     expect(result.structuredContent).toEqual(
       mcpSuccessResult(`Found ${variables.length} panel variable(s)`, {
         panelUrlFormat: MOCK_PANEL_URL_FORMAT,
@@ -155,10 +161,49 @@ describe('listVariables', () => {
     expect(panelService.getVariables).not.toHaveBeenCalled();
   });
 
+  it('should use connection serverUrl when DHE server URL is provided', async () => {
+    // Simulates DHE scenario where server URL differs from worker URL
+    const serverUrl = 'http://enterprise.deephaven.io';
+    const workerUrl = new URL('http://enterprise.deephaven.io/worker-123');
+    const mockConnection = {
+      serverUrl: workerUrl,
+    } as IDhcService;
+
+    vi.mocked(getFirstConnectionOrCreate).mockResolvedValue({
+      success: true,
+      connection: mockConnection,
+      panelUrlFormat: MOCK_PANEL_URL_FORMAT,
+    });
+    vi.mocked(panelService.getVariables).mockReturnValue(MOCK_VARIABLES);
+
+    const tool = createListVariablesTool({
+      panelService,
+      serverManager,
+    });
+    const result = await tool.handler({ connectionUrl: serverUrl });
+
+    // Should call with the worker URL, not the server URL
+    expect(panelService.getVariables).toHaveBeenCalledWith(workerUrl);
+    expect(result.structuredContent).toEqual(
+      mcpSuccessResult(`Found ${MOCK_VARIABLES.length} panel variable(s)`, {
+        panelUrlFormat: MOCK_PANEL_URL_FORMAT,
+        variables: MOCK_VARIABLES.map(({ id, title, type }) => ({
+          id,
+          title,
+          type,
+        })),
+      })
+    );
+  });
+
   it('should handle errors from panelService', async () => {
+    const mockConnection = {
+      serverUrl: MOCK_PARSED_URL,
+    } as IDhcService;
+
     vi.mocked(getFirstConnectionOrCreate).mockResolvedValue({
       success: true,
-      connection: {} as IDhcService,
+      connection: mockConnection,
       panelUrlFormat: MOCK_PANEL_URL_FORMAT,
     });
     vi.mocked(panelService.getVariables).mockImplementation(() => {

src/mcp/tools/listVariables.ts

@@ -81,13 +81,15 @@ export function createListVariablesTool({
           return response.errorWithHint(errorMessage, error, hint, details);
         }
 
-        const variables = [...panelService.getVariables(parsedUrl.value)].map(
-          ({ id, title, type }) => ({
-            id,
-            title,
-            type,
-          })
-        );
+        const { connection } = firstConnectionResult;
+
+        const variables = [
+          ...panelService.getVariables(connection.serverUrl),
+        ].map(({ id, title, type }) => ({
+          id,
+          title,
+          type,
+        }));
 
         const message = `Found ${variables.length} panel variable(s)`;
 

src/mcp/tools/openVariablePanels.spec.ts

@@ -47,9 +47,13 @@ describe('openVariablePanels', () => {
   });
 
   it('should open variable panels successfully', async () => {
+    const mockConnection = {
+      serverUrl: MOCK_PARSED_URL,
+    } as IDhcService;
+
     vi.mocked(getFirstConnectionOrCreate).mockResolvedValue({
       success: true,
-      connection: {} as IDhcService,
+      connection: mockConnection,
       panelUrlFormat: 'mock.panelUrlFormat',
     });
 
@@ -64,7 +68,7 @@ describe('openVariablePanels', () => {
       connectionUrl: MOCK_PARSED_URL,
     });
     expect(execOpenVariablePanels).toHaveBeenCalledWith(
-      MOCK_PARSED_URL,
+      mockConnection.serverUrl,
       MOCK_VARIABLES
     );
     expect(result.structuredContent).toEqual(
@@ -137,10 +141,44 @@ describe('openVariablePanels', () => {
     );
   });
 
+  it('should use connection serverUrl when DHE server URL is provided', async () => {
+    // Simulates DHE scenario where server URL differs from worker URL
+    const serverUrl = 'http://enterprise.deephaven.io';
+    const workerUrl = new URL('http://enterprise.deephaven.io/worker-123');
+    const mockConnection = {
+      serverUrl: workerUrl,
+    } as IDhcService;
+
+    vi.mocked(getFirstConnectionOrCreate).mockResolvedValue({
+      success: true,
+      connection: mockConnection,
+      panelUrlFormat: 'mock.panelUrlFormat',
+    });
+
+    const tool = createOpenVariablePanelsTool({ serverManager });
+    const result = await tool.handler({
+      connectionUrl: serverUrl,
+      variables: MOCK_VARIABLES,
+    });
+
+    // Should call with the worker URL, not the server URL
+    expect(execOpenVariablePanels).toHaveBeenCalledWith(
+      workerUrl,
+      MOCK_VARIABLES
+    );
+    expect(result.structuredContent).toEqual(
+      mcpSuccessResult('Variable panels opened successfully')
+    );
+  });
+
   it('should handle errors from execOpenVariablePanels', async () => {
+    const mockConnection = {
+      serverUrl: MOCK_PARSED_URL,
+    } as IDhcService;
+
     vi.mocked(getFirstConnectionOrCreate).mockResolvedValue({
       success: true,
-      connection: {} as IDhcService,
+      connection: mockConnection,
       panelUrlFormat: 'mock.panelUrlFormat',
     });
     vi.mocked(execOpenVariablePanels).mockRejectedValue(

src/mcp/tools/openVariablePanels.ts

@@ -81,9 +81,11 @@ export function createOpenVariablePanelsTool({
         );
       }
 
+      const { connection } = firstConnectionResult;
+
       try {
         await execOpenVariablePanels(
-          parsedUrl.value,
+          connection.serverUrl,
           variables as unknown as NonEmptyArray<VariableDefintion>
         );
 

src/mcp/utils/panelUtils.spec.ts

@@ -64,21 +64,24 @@ describe('getDhePanelUrlFormat', () => {
         embedDashboardsAndWidgets: false,
       },
       workerInfo: undefined,
-      expected: undefined,
+      expected:
+        'Panel URLs are not supported by this Enterprise server version (requires Grizzly+ or later)',
     },
     {
       scenario: 'embedDashboardsAndWidgets feature is not defined',
       dheService: mockDheService,
       features: { createQueryIframe: true },
       workerInfo: undefined,
-      expected: undefined,
+      expected:
+        'Panel URLs are not supported by this Enterprise server version (requires Grizzly+ or later)',
     },
     {
       scenario: 'getServerFeatures returns undefined',
       dheService: mockDheService,
       features: undefined,
       workerInfo: undefined,
-      expected: undefined,
+      expected:
+        'Panel URLs are not supported by this Enterprise server version (requires Grizzly+ or later)',
     },
     {
       scenario: 'worker info is not available',

src/mcp/utils/panelUtils.ts

@@ -29,11 +29,15 @@ export async function getDhePanelUrlFormat(
 ): Promise<string | undefined> {
   const dheService = await serverManager.getDheServiceForWorker(connectionUrl);
 
-  const features = dheService?.getServerFeatures()?.features;
-  if (features?.embedDashboardsAndWidgets !== true) {
+  if (dheService == null) {
     return undefined;
   }
 
+  const features = dheService.getServerFeatures()?.features;
+  if (features?.embedDashboardsAndWidgets !== true) {
+    return 'Panel URLs are not supported by this Enterprise server version (requires Grizzly+ or later)';
+  }
+
   // Get worker info for DHE servers to include serial ID in panel URLs
   const workerInfo = await serverManager.getWorkerInfo(connectionUrl);
   if (workerInfo == null) {