Zoeabboudi/GitHub copilot cli ai clients by zoeabboudi · Pull Request #10 · microsoft/Microsoft-Fabric-tools-workload

zoeabboudi · 2026-02-05T13:00:17Z

No description provided.

- Add AzureOpenAIClient for AI-powered chat fallback - Add GhCopilotCLIClient for real gh copilot CLI integration via Spark - Update SCSS with chat-style AI interface components - Fix FRONTEND_URL (remove /front-end suffix) - Add enhanced token debugging in FabricAuthenticationService - Export new clients from engine/index.ts

Workload/app/items/GithubCopilotCLIItem/engine/GhCopilotCLIClient.ts

Copilot

Pull request overview

This pull request introduces a comprehensive GitHub Copilot CLI integration for Microsoft Fabric, adding AI-assisted command-line capabilities within the Fabric workload environment.

Changes:

Implements multiple AI client backends (GitHub Copilot API, Azure OpenAI, local fallback)
Adds CLI-style interface for interacting with GitHub Copilot with command parsing (suggest, explain, run, help, context)
Integrates Spark Livy session management for command execution with lakehouse and environment selection
Extends environment loading capabilities across CloudShell and GithubCopilotCLI items

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
Workload/package.json	Adds cross-env dependency (incorrect version)
Workload/app/items/GithubCopilotCLIItem/engine/index.ts	Exports all engine components and clients
Workload/app/items/GithubCopilotCLIItem/engine/CopilotClient.ts	GitHub Copilot API client with conversation management and local fallback
Workload/app/items/GithubCopilotCLIItem/engine/GithubCopilotCLI.ts	CLI command parser and executor with workspace context support
Workload/app/items/GithubCopilotCLIItem/engine/GhCopilotCLIClient.ts	Real gh copilot CLI wrapper for Spark sessions
Workload/app/items/GithubCopilotCLIItem/engine/AzureOpenAIClient.ts	Azure OpenAI integration with localStorage config (security concern)
Workload/app/items/GithubCopilotCLIItem/engine/GithubCopilotCLIEngine.ts	Main engine coordinating Copilot and Spark execution
Workload/app/items/GithubCopilotCLIItem/GithubCopilotCLIItemRibbon.tsx	Ribbon with lakehouse, environment, and session controls
Workload/app/items/GithubCopilotCLIItem/GithubCopilotCLIItemModel.ts	Type definitions for CLI item and workspace context
Workload/app/items/GithubCopilotCLIItem/GithubCopilotCLIItemEditor.tsx	Editor with session management and environment loading
Workload/app/items/GithubCopilotCLIItem/GithubCopilotCLIItemDefaultView.tsx	CLI terminal interface with command execution
Workload/app/items/GithubCopilotCLIItem/GithubCopilotCLIItem.scss	CLI-themed styling with GitHub-inspired colors
Workload/app/items/CloudShellItem/engine/SparkLivyCloudShellClient.ts	Adds optional CLI check skip parameter
Workload/app/items/CloudShellItem/CloudShellItemEditor.tsx	Enhanced environment loading with fallback
Workload/app/clients/ItemClient.ts	Adds dedicated listEnvironments endpoint
Workload/app/clients/FabricAuthenticationService.ts	Enhanced token acquisition logging
Workload/.env.dev	Development environment configuration

Copilot · 2026-02-05T13:05:28Z

Workload/app/items/GithubCopilotCLIItem/engine/AzureOpenAIClient.ts

+export function saveAzureOpenAIConfig(config: AzureOpenAIConfig): void {
+    try {
+        localStorage.setItem(STORAGE_KEY, JSON.stringify(config));
+    } catch (e) {
+        console.warn('Failed to save Azure OpenAI config to localStorage:', e);
+    }


Storing sensitive API keys in localStorage is a security risk, as localStorage data can be accessed by any JavaScript code running on the same origin and persists across sessions. Consider using sessionStorage for temporary storage, or better yet, use a more secure credential storage mechanism that encrypts the data. At minimum, add a warning in the UI that API keys stored this way are not encrypted and may be visible to other scripts.

Copilot · 2026-02-05T13:05:28Z

Workload/app/items/GithubCopilotCLIItem/GithubCopilotCLIItemEditor.tsx

+        addSystemMessage(t("GithubCopilotCLIItem_Lakehouse_Selected", "Lakehouse selected: {{name}}", { name: result.displayName }));
+        return true;
+      }
+      return false;
+    } catch (error) {
+      console.error('Failed to select lakehouse:', error);
+      addSystemMessage("Failed to select lakehouse");


The translation keys like "GithubCopilotCLIItem_Lakehouse_Selected" use interpolation but don't follow a consistent pattern. Some messages use system messages directly without translation (e.g., line 281: "Failed to select lakehouse"), while others use proper i18n. For consistency and localization support, all user-facing messages should use the translation function with proper keys.

Copilot · 2026-02-05T13:05:28Z

Workload/app/items/GithubCopilotCLIItem/GithubCopilotCLIItemDefaultView.tsx

+          <input
+            ref={inputRef}
+            className="cli-text-input"
+            value={command}
+            onChange={(e) => setCommand(e.target.value)}
            onKeyDown={handleKeyDown}
-            placeholder={t('GithubCopilotCLIItem_Prompt_Placeholder', 'Ask Copilot anything...')}
+            placeholder="suggest, explain, or help..."
            disabled={isProcessing}
-          />
-          <Button
-            icon={<Send24Regular />}
-            onClick={executePrompt}
-            disabled={isProcessing || !prompt.trim()}
-            appearance="primary"
+            autoFocus
          />


The native input element (line 318) bypasses Fluent UI's Input component, which means it won't inherit theme tokens, accessibility features, or consistent styling from the design system. While this might be intentional for the CLI aesthetic, consider whether the tradeoffs (loss of built-in accessibility features, theme integration) are worth it. If you need the CLI look, consider using Fluent UI's Input with custom styling instead.

Copilot · 2026-02-05T13:05:29Z

Workload/app/items/CloudShellItem/CloudShellItemEditor.tsx

+      console.log('[CloudShell] Loading environments, workspaceId:', workspaceId);

+      if (!workspaceId) {
+        console.log('[CloudShell] No lakehouse workspace ID available, skipping environment load');
+        return;
+      }
+
+      const itemClient = new ItemClient(workloadClient);
+      let environments: Item[] = [];
+
+      // Try the dedicated environments endpoint first
      try {
-        const itemClient = new ItemClient(workloadClient);
-        const workspaceItems = await itemClient.listItems(workspaceId, { type: 'Environment' });
-        setAvailableEnvironments(workspaceItems.value);
-      } catch (error) {
-        console.error('Failed to load environments:', error);
+        console.log('[CloudShell] Trying dedicated /environments endpoint for workspace:', workspaceId);
+        const environmentsResult = await itemClient.listEnvironments(workspaceId);
+        console.log('[CloudShell] Dedicated environments API response:', environmentsResult);
+        environments = environmentsResult.value || [];
+      } catch (error: any) {
+        console.warn('[CloudShell] Dedicated /environments endpoint failed, trying fallback:', {
+          message: error?.message,
+          status: error?.statusCode,
+          errorCode: error?.errorCode
+        });
+
+        // Fallback: Try listing all items and filter by type
+        try {
+          console.log('[CloudShell] Fallback: Listing all items and filtering by Environment type');
+          const allItems = await itemClient.listItems(workspaceId);
+          console.log('[CloudShell] All items in workspace:', allItems.value?.length || 0);
+          console.log('[CloudShell] Item types found:', [...new Set(allItems.value?.map(i => i.type) || [])]);
+
+          environments = (allItems.value || []).filter(item => 
+            item.type === 'Environment' || 
+            item.type?.toLowerCase().includes('environment')
+          );
+        } catch (fallbackError: any) {
+          console.error('[CloudShell] Fallback also failed:', {
+            message: fallbackError?.message,
+            status: fallbackError?.statusCode,
+            errorCode: fallbackError?.errorCode
+          });
+        }
      }
+
+      console.log('[CloudShell] Final environments count:', environments.length);
+
+      if (environments.length === 0) {
+        console.log('[CloudShell] No environments found. Possible causes:');
+        console.log('  1. No Environment items exist in the lakehouse workspace');
+        console.log('  2. User lacks Workspace.Read.All permission');
+        console.log('  3. Livy API tenant setting may not be enabled');
+      }
+


Debug console.log statements on lines 122, 134-164, and 170-173 should be removed before production deployment. These detailed logging statements are helpful during development but will clutter the browser console in production and may expose internal implementation details.

Copilot · 2026-02-05T13:05:29Z

Workload/app/clients/FabricAuthenticationService.ts

+    console.log('[FabricAuth] Requesting token with scopes:', scopeArray);
+    console.log('[FabricAuth] Scopes string:', scopes);
+
+    try {
+      const token = await this.workloadClient.auth.acquireFrontendAccessToken({ 
+        scopes: scopeArray 
+      });
+      console.log('[FabricAuth] Token acquired successfully');
+      return token;
+    } catch (error: any) {
+      console.error('[FabricAuth] Failed to acquire token - FULL ERROR:', JSON.stringify(error, null, 2));
+      console.error('[FabricAuth] Error object:', error);
+      console.error('[FabricAuth] Error.error:', error?.error);
+      console.error('[FabricAuth] Scopes that failed:', scopeArray);


Debug console.log statements on lines 54-69 should be removed before production deployment. These logging statements expose authentication details and scope information that could be sensitive and should not be logged in production environments.

Suggested change

console.log('[FabricAuth] Requesting token with scopes:', scopeArray);

console.log('[FabricAuth] Scopes string:', scopes);

try {

const token = await this.workloadClient.auth.acquireFrontendAccessToken({

scopes: scopeArray

});

console.log('[FabricAuth] Token acquired successfully');

return token;

} catch (error: any) {

console.error('[FabricAuth] Failed to acquire token - FULL ERROR:', JSON.stringify(error, null, 2));

console.error('[FabricAuth] Error object:', error);

console.error('[FabricAuth] Error.error:', error?.error);

console.error('[FabricAuth] Scopes that failed:', scopeArray);

try {

const token = await this.workloadClient.auth.acquireFrontendAccessToken({

scopes: scopeArray

});

return token;

} catch (error: any) {

Copilot · 2026-02-05T13:05:31Z

Workload/app/items/GithubCopilotCLIItem/engine/GithubCopilotCLI.ts

+     */
+    private async callCopilotAPI(systemPrompt: string, userMessage: string): Promise<string> {
+        // Try to get a token for GitHub Copilot
+        // In a real implementation, this would use proper GitHub OAuth
+        // For now, we'll use the local fallback which provides a good experience
+
+        // Attempt API call (will likely fail without proper auth, triggering fallback)


The callCopilotAPI method always throws an error on line 428, which means all requests will fall back to local processing. This appears to be placeholder code. If this is intentional for the current implementation, consider adding a comment explaining that API integration is not yet implemented and that the method is designed to fall back to local processing. Otherwise, this suggests incomplete functionality.

Suggested change

*/

private async callCopilotAPI(systemPrompt: string, userMessage: string): Promise<string> {

// Try to get a token for GitHub Copilot

// In a real implementation, this would use proper GitHub OAuth

// For now, we'll use the local fallback which provides a good experience

// Attempt API call (will likely fail without proper auth, triggering fallback)

*

* NOTE: API integration is not yet implemented. This method is intentionally

* implemented as a stub that always throws, so that callers fall back to the

* local processing implementations (e.g. localSuggest/localExplain) which

* provide a good experience without remote calls.

*

* When adding real Copilot integration, replace the throw below with an

* authenticated API call and return the generated response instead.

*/

private async callCopilotAPI(systemPrompt: string, userMessage: string): Promise<string> {

// Try to get a token for GitHub Copilot

// In a real implementation, this would use proper GitHub OAuth and call

// the GitHub Copilot service with the provided systemPrompt and userMessage.

// For now, we intentionally force the local fallback in the caller.

// Intentional stub: always trigger fallback to local processing

Copilot · 2026-02-05T13:05:31Z

Workload/app/items/GithubCopilotCLIItem/engine/GithubCopilotCLI.ts

+        const runMatch = trimmed.match(/^run\s+["']?(.+?)["']?$/i);
+        if (runMatch) {
+            return { type: 'run', query: runMatch[1] };
+        }
+
+        // Direct shell/fab commands (run them directly)
+        if (trimmed.startsWith('fab ') || 
+            trimmed.startsWith('ls ') || trimmed === 'ls' ||
+            trimmed.startsWith('pwd') ||
+            trimmed.startsWith('cat ') ||
+            trimmed.startsWith('echo ') ||
+            trimmed.startsWith('python ') ||
+            trimmed.startsWith('pip ')) {
+            return { type: 'run', query: trimmed };
+        }
+
+        // Parse suggest command: suggest "query" or suggest query
+        const suggestMatch = trimmed.match(/^suggest\s+["']?(.+?)["']?$/i);
+        if (suggestMatch) {
+            return { type: 'suggest', query: suggestMatch[1] };
+        }
+
+        // Parse explain command: explain "command" or explain command
+        const explainMatch = trimmed.match(/^explain\s+["']?(.+?)["']?$/i);
+        if (explainMatch) {
+            return { type: 'explain', query: explainMatch[1] };
+        }


The regex patterns for parsing commands use non-capturing groups with optional quotes, but the quote matching is inconsistent. For example, ["']?(.+?)["']? will match strings like "hello' (mixed quotes). Consider using more precise regex patterns that enforce matching quote types, such as (?:["'])?(.+?)(?:["'])? or separate patterns for double and single quotes to prevent mismatched quote scenarios.

Copilot · 2026-02-05T13:05:31Z

Workload/app/items/GithubCopilotCLIItem/GithubCopilotCLIItemEditor.tsx

@@ -1,11 +1,12 @@
-import React, { useEffect, useState } from "react";
+import React, { useEffect, useState, useCallback } from "react";


Unused import useCallback.

Suggested change

import React, { useEffect, useState, useCallback } from "react";

import React, { useEffect, useState } from "react";

Copilot · 2026-02-05T13:05:32Z

Workload/app/items/GithubCopilotCLIItem/GithubCopilotCLIItemEditor.tsx

+  const [sessionActive, setSessionActive] = useState(false);
+  const [sessionId, setSessionId] = useState<string | null>(null);
+  const [isConnecting, setIsConnecting] = useState(false);
+  const [systemMessage, setSystemMessage] = useState<{ message: string; timestamp: number } | undefined>();


Unused variable systemMessage.

Suggested change

const [systemMessage, setSystemMessage] = useState<{ message: string; timestamp: number } | undefined>();

const [, setSystemMessage] = useState<{ message: string; timestamp: number } | undefined>();

Copilot · 2026-02-05T13:05:32Z

Workload/app/items/GithubCopilotCLIItem/GithubCopilotCLIItemEditor.tsx

+  const [sessionActive, setSessionActive] = useState(false);
+  const [sessionId, setSessionId] = useState<string | null>(null);
+  const [isConnecting, setIsConnecting] = useState(false);
+  const [systemMessage, setSystemMessage] = useState<{ message: string; timestamp: number } | undefined>();


Unused variable setSystemMessage.

Suggested change

const [systemMessage, setSystemMessage] = useState<{ message: string; timestamp: number } | undefined>();

const [systemMessage] = useState<{ message: string; timestamp: number } | undefined>();

Zoe Abboudi added 2 commits February 4, 2026 10:04

WIP: trying to put github copilot cli in fabric

f2a0606

zoeabboudi requested a review from Copilot February 5, 2026 13:00

Copilot started reviewing on behalf of zoeabboudi February 5, 2026 13:01 View session

github-advanced-security bot found potential problems Feb 5, 2026

View reviewed changes

Copilot AI reviewed Feb 5, 2026

View reviewed changes

zoeabboudi requested a review from teddyberco February 5, 2026 13:12

-     */
-    private async callCopilotAPI(systemPrompt: string, userMessage: string): Promise<string> {
-        // Try to get a token for GitHub Copilot
-        // In a real implementation, this would use proper GitHub OAuth
-        // For now, we'll use the local fallback which provides a good experience
-        // Attempt API call (will likely fail without proper auth, triggering fallback)
+     *
+     * NOTE: API integration is not yet implemented. This method is intentionally
+     * implemented as a stub that always throws, so that callers fall back to the
+     * local processing implementations (e.g. localSuggest/localExplain) which
+     * provide a good experience without remote calls.
+     *
+     * When adding real Copilot integration, replace the throw below with an
+     * authenticated API call and return the generated response instead.
+     */
+    private async callCopilotAPI(systemPrompt: string, userMessage: string): Promise<string> {
+        // Try to get a token for GitHub Copilot
+        // In a real implementation, this would use proper GitHub OAuth and call
+        // the GitHub Copilot service with the provided systemPrompt and userMessage.
+        // For now, we intentionally force the local fallback in the caller.
+       // Intentional stub: always trigger fallback to local processing

		@@ -1,11 +1,12 @@
		import React, { useEffect, useState } from "react";
		import React, { useEffect, useState, useCallback } from "react";

	const [systemMessage, setSystemMessage] = useState<{ message: string; timestamp: number } \| undefined>();
	const [, setSystemMessage] = useState<{ message: string; timestamp: number } \| undefined>();

	const [systemMessage, setSystemMessage] = useState<{ message: string; timestamp: number } \| undefined>();
	const [systemMessage] = useState<{ message: string; timestamp: number } \| undefined>();

Conversation

zoeabboudi commented Feb 5, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants