Adding coreview for slack workspace extension #281508

aiday-mar · 2025-12-05T12:54:00Z

No description provided.

Copilot

Pull request overview

This PR introduces a new VS Code extension for Slack workspace integration that enables code review workflows directly within VS Code. The extension provides a tree view to display Slack messages from a #codereview channel, with special handling for GitHub pull request links.

Key changes:

Complete extension implementation with authentication, message fetching, and PR integration
OAuth-based Slack authentication with session management
GitHub API integration for fetching PR metadata
Tree view UI for displaying code review messages

Reviewed changes

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

Show a summary per file

File	Description
tsconfig.json	TypeScript configuration for the extension
src/test/extension.test.ts	Placeholder test suite (needs comprehensive tests)
src/slackTreeDataProvider.ts	Tree view data provider for Slack messages UI
src/slackService.ts	Core service for Slack API communication and GitHub integration
src/slackAuthenticationProvider.ts	OAuth authentication provider for Slack
src/extension.ts	Extension entry point with command and view registration
package.json	Extension manifest with commands, views, and dependencies
oauth-redirect/index.html	OAuth callback page for handling Slack authentication
oauth-redirect/.gitignore	Ignore file for deployment artifacts
eslint.config.mjs	ESLint configuration
esbuild.js	Build configuration with environment variable injection
.vscode/launch.json	VS Code debugging configuration
.gitignore	Added .env files to gitignore

Copilot · 2025-12-05T13:08:03Z

.vscode/extensions/vscode-codereview-slack-integration/src/slackTreeDataProvider.ts

+    async fetchMessages(): Promise<void> {
+        this.isLoading = true;
+        this.errorMessage = undefined;
+        this.refresh();
+
+        try {
+            this.messages = await this.slackService.getMessages();
+        } catch (error) {
+            this.errorMessage = error instanceof Error ? error.message : 'Unknown error';
+            this.messages = [];
+        } finally {
+            this.isLoading = false;
+            this.refresh();
+            this.onMessageCountChanged?.(this.messages.length);
+        }


Potential race condition: if fetchMessages() is called multiple times concurrently (e.g., from auto-refresh and manual refresh), the isLoading flag could be set incorrectly. The second call would set isLoading = true again, but when either call completes, it sets isLoading = false, potentially allowing a third call to start before all previous calls have completed.

Consider using a promise-based approach to prevent concurrent fetches, or check if a fetch is already in progress before starting a new one.

Suggested change

async fetchMessages(): Promise<void> {

this.isLoading = true;

this.errorMessage = undefined;

this.refresh();

try {

this.messages = await this.slackService.getMessages();

} catch (error) {

this.errorMessage = error instanceof Error ? error.message : 'Unknown error';

this.messages = [];

} finally {

this.isLoading = false;

this.refresh();

this.onMessageCountChanged?.(this.messages.length);

}

private fetchMessagesPromise: Promise<void> | null = null;

async fetchMessages(): Promise<void> {

// Prevent concurrent fetches

if (this.fetchMessagesPromise) {

return this.fetchMessagesPromise;

}

this.isLoading = true;

this.errorMessage = undefined;

this.refresh();

this.fetchMessagesPromise = (async () => {

try {

this.messages = await this.slackService.getMessages();

} catch (error) {

this.errorMessage = error instanceof Error ? error.message : 'Unknown error';

this.messages = [];

} finally {

this.isLoading = false;

this.refresh();

this.onMessageCountChanged?.(this.messages.length);

this.fetchMessagesPromise = null;

}

})();

return this.fetchMessagesPromise;

Copilot · 2025-12-05T13:08:03Z

.vscode/extensions/vscode-codereview-slack-integration/src/test/extension.test.ts

+suite('Extension Test Suite', () => {
+	vscode.window.showInformationMessage('Start all tests.');
+
+	test('Sample test', () => {
+		assert.strictEqual(-1, [1, 2, 3].indexOf(5));
+		assert.strictEqual(-1, [1, 2, 3].indexOf(0));
+	});


The test suite only contains a basic sample test that doesn't test any actual functionality of the extension. Consider adding tests for the key functionality including:

SlackService authentication flows

SlackTreeDataProvider message fetching and display

Command handlers (sign in/out, open PR, refresh)

PR URL extraction and GitHub API integration

OAuth flow and state management

Copilot · 2025-12-05T13:08:04Z

.vscode/extensions/vscode-codereview-slack-integration/src/service.ts

+
+            return session;
+        } catch (error) {
+            console.error('Failed to get Slack session:', error);


Console.error statements should not be used in production code. Use a proper logging framework or VS Code output channels instead.

Copilot · 2025-12-05T13:08:04Z

.vscode/extensions/vscode-codereview-slack-integration/src/treeDataProvider.ts

+
+export interface SlackMessage {
+    id: string;
+    author: string;
+    text: string;
+    timestamp: string;
+    prUrl?: string;
+    prTitle?: string;
+    prAuthor?: string;
+    prOwner?: string;
+    prRepo?: string;
+    prNumber?: number;
+    prAdditions?: number;
+    prDeletions?: number;


Missing JSDoc documentation for the public interface SlackMessage and its properties. Since this interface is exported and used across multiple files, it should have documentation explaining the purpose of each field.

Example:

/** * Represents a message from the Slack code review channel. */ export interface SlackMessage { /** Unique identifier for the message (Slack timestamp) */ id: string; /** Display name of the message author */ author: string; // ... etc }

Suggested change

export interface SlackMessage {

id: string;

author: string;

text: string;

timestamp: string;

prUrl?: string;

prTitle?: string;

prAuthor?: string;

prOwner?: string;

prRepo?: string;

prNumber?: number;

prAdditions?: number;

prDeletions?: number;

/**

* Represents a message from the Slack code review channel.

*/

export interface SlackMessage {

/** Unique identifier for the message (Slack timestamp) */

id: string;

/** Display name of the message author */

author: string;

/** The text content of the message */

text: string;

/** The timestamp of the message (ISO string or Slack format) */

timestamp: string;

/** The URL of the related pull request, if any */

prUrl?: string;

/** The title of the related pull request, if any */

prTitle?: string;

/** The author of the pull request (GitHub handle), if any */

prAuthor?: string;

/** The owner of the repository for the pull request, if any */

prOwner?: string;

/** The name of the repository for the pull request, if any */

prRepo?: string;

/** The pull request number, if any */

prNumber?: number;

/** Number of additions in the pull request, if any */

prAdditions?: number;

/** Number of deletions in the pull request, if any */

prDeletions?: number;

/** Number of changed files in the pull request, if any */

Copilot · 2025-12-05T13:08:04Z

.vscode/extensions/vscode-codereview-slack-integration/src/authProvider.ts

+export class SlackAuthenticationProvider implements vscode.AuthenticationProvider, vscode.Disposable {
+    private _sessionChangeEmitter = new vscode.EventEmitter<vscode.AuthenticationProviderAuthenticationSessionsChangeEvent>();
+    private _disposable: vscode.Disposable;
+    private _sessions: vscode.AuthenticationSession[] = [];
+    private _pendingAuth: PendingAuth | undefined;
+    private _sessionsLoaded: boolean = false;
+
+    constructor(private readonly context: vscode.ExtensionContext) {


Missing JSDoc documentation for the exported SlackAuthenticationProvider class. This class implements the VS Code authentication provider interface and should document how it handles OAuth flow, session management, and URI callbacks.

Copilot · 2025-12-05T13:08:09Z

.vscode/extensions/vscode-codereview-slack-integration/src/extension.ts

+		try {
+			await vscode.env.openExternal(vscode.Uri.parse(item.message.prUrl));
+		} catch (error) {
+			console.error('Failed to open PR in browser:', error);


Console.error statements should not be used in production code. Use a proper logging framework or VS Code output channels instead.

Copilot · 2025-12-05T13:08:09Z

.vscode/extensions/vscode-codereview-slack-integration/package.json

+    "commands": [
+      {
+        "command": "vs-code-codereview.helloWorld",
+        "title": "Hello World"
+      },
+      {
+        "command": "vs-code-codereview.refreshMessages",
+        "title": "Refresh Messages",
+        "icon": "$(refresh)"
+      },
+      {
+        "command": "vs-code-codereview.signIn",
+        "title": "Sign in to Slack",
+        "icon": "$(sign-in)"
+      },
+      {
+        "command": "vs-code-codereview.signOut",
+        "title": "Sign out from Slack",
+        "icon": "$(sign-out)"
+      },
+      {
+        "command": "vs-code-codereview.openPrLocally",
+        "title": "Open PR Locally",
+        "icon": "$(link-external)"
+      },
+      {
+        "command": "vs-code-codereview.openPrInBrowser",
+        "title": "Open PR in Browser",
+        "icon": "$(globe)"
+      }


[nitpick] The command naming convention is inconsistent with the extension ID. The commands use 'vs-code-codereview' prefix while the package name is 'vs-code-codereview', but the authentication provider and internal references use different variations. For consistency, consider standardizing on a single naming scheme throughout (e.g., all commands should use the same prefix format as defined in the package.json name).

Copilot · 2025-12-05T13:08:09Z

.vscode/extensions/vscode-codereview-slack-integration/src/authenticationProvider.ts

+        } catch {
+            // Fall back to username if user info fetch fails


The error handling swallows the actual error without logging it. At minimum, the error should be logged to help with debugging when fetching additional user details fails.

Suggested change

} catch {

// Fall back to username if user info fetch fails

} catch (err) {

// Fall back to username if user info fetch fails

console.error('Failed to fetch additional Slack user details:', err);

Copilot · 2025-12-05T13:08:10Z

.vscode/extensions/vscode-codereview-slack-integration/src/authenticationProvider.ts

+        if (sessionsJson) {
+            try {
+                this._sessions = JSON.parse(sessionsJson);
+            } catch {


The error handling swallows the actual error without logging it. At minimum, the error should be logged to help with debugging when session loading fails.

Suggested change

} catch {

} catch (err) {

console.error('Failed to parse Slack sessions from secrets:', err);

Copilot · 2025-12-05T13:08:10Z

.vscode/extensions/vscode-codereview-slack-integration/src/extension.ts

+	// Auto-refresh messages every 60 seconds
+	const REFRESH_INTERVAL_MS = 60 * 1000; // 1 minute
+	const autoRefreshInterval = setInterval(async () => {
+		const isAuthenticated = await slackService.isAuthenticated();
+		if (isAuthenticated) {
+			await slackTreeDataProvider.fetchMessages();
+		}
+	}, REFRESH_INTERVAL_MS);
+
+	// Clean up the interval when the extension is deactivated
+	context.subscriptions.push({
+		dispose: () => clearInterval(autoRefreshInterval)


The hardcoded 60-second refresh interval may be too aggressive for production use. Consider making this configurable via VS Code settings to allow users to adjust the polling frequency based on their needs. Frequent API calls could hit rate limits on both Slack and GitHub APIs.

Suggested change

// Auto-refresh messages every 60 seconds

const REFRESH_INTERVAL_MS = 60 * 1000; // 1 minute

const autoRefreshInterval = setInterval(async () => {

const isAuthenticated = await slackService.isAuthenticated();

if (isAuthenticated) {

await slackTreeDataProvider.fetchMessages();

}

}, REFRESH_INTERVAL_MS);

// Clean up the interval when the extension is deactivated

context.subscriptions.push({

dispose: () => clearInterval(autoRefreshInterval)

// Auto-refresh messages at a configurable interval

let refreshIntervalSeconds = vscode.workspace.getConfiguration('codereview-slack').get<number>('refreshIntervalSeconds', 60);

let autoRefreshInterval: NodeJS.Timeout | undefined;

const startAutoRefresh = () => {

if (autoRefreshInterval) {

clearInterval(autoRefreshInterval);

}

autoRefreshInterval = setInterval(async () => {

const isAuthenticated = await slackService.isAuthenticated();

if (isAuthenticated) {

await slackTreeDataProvider.fetchMessages();

}

}, refreshIntervalSeconds * 1000);

};

startAutoRefresh();

// Listen for configuration changes to update the interval dynamically

const configChangeDisposable = vscode.workspace.onDidChangeConfiguration(e => {

if (e.affectsConfiguration('codereview-slack.refreshIntervalSeconds')) {

refreshIntervalSeconds = vscode.workspace.getConfiguration('codereview-slack').get<number>('refreshIntervalSeconds', 60);

startAutoRefresh();

}

});

context.subscriptions.push(configChangeDisposable);

// Clean up the interval when the extension is deactivated

context.subscriptions.push({

dispose: () => {

if (autoRefreshInterval) {

clearInterval(autoRefreshInterval);

}

}

…tings

…build scripts, and add esbuild configuration - Removed the "Hello World" command from package.json. - Updated build scripts in package.json to use Gulp for compilation and watching. - Added esbuild.mjs for building the extension with environment variable support. - Enhanced command disposal in commands.ts and view.ts to call super.dispose(). - Updated mcp.json to include configuration for vscode-playwright-mcp server.

…g in Slack service

…dling and improving token exchange process

- Updated command identifiers for opening PRs to improve clarity. - Removed the authentication provider implementation and moved it to a new file for better organization. - Simplified command registration by reducing dependencies on the Slack service. - Enhanced error handling and user feedback during authentication. - Implemented auto-refresh for message fetching in the Slack tree data provider. - Improved session management and caching for user and pull request data. - Consolidated authentication variables into a dedicated module for easier configuration.

…s and update usage in SlackTreeDataProvider

adding workspace extension

9252cfb

Copilot AI review requested due to automatic review settings December 5, 2025 12:54

aiday-mar self-assigned this Dec 5, 2025

Copilot started reviewing on behalf of aiday-mar December 5, 2025 12:54 View session

adding the extension into the build files

97a8370

Copilot finished reviewing on behalf of aiday-mar December 5, 2025 12:58

Copilot AI reviewed Dec 5, 2025

View reviewed changes

aiday-mar added 17 commits December 5, 2025 15:16

removing unused code

397b6b6

adding file commands.ts which contains the commands definitions

bb70d06

refactoring further

06714bf

polish

110fb99

renaming and refactoring

673a631

refactoring the slack service

70a8820

combining the session and the view

66f2d47

using private and public channel

4a6a58f

refactoring teh authentication provider

3aa3fe9

refactor the tree data provider

2ec18ce

refactor: clean up esbuild configuration and remove unused ESLint set…

6e44c08

…tings

refactor: fix TypeScript type definitions and improve promise handlin…

f70769f

…g in Slack service

refactor: enhance Slack authentication flow by updating user info han…

3e6242c

…dling and improving token exchange process

refactor: rename waitUntilAuthenticationStatus to authenticationStatu…

1e073b2

…s and update usage in SlackTreeDataProvider

moving fetchMessages into the trigger

609c4a5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Adding coreview for slack workspace extension #281508

Adding coreview for slack workspace extension #281508

aiday-mar commented Dec 5, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	} catch {
	} catch (err) {
	console.error('Failed to parse Slack sessions from secrets:', err);

-	// Auto-refresh messages every 60 seconds
-	const REFRESH_INTERVAL_MS = 60 * 1000; // 1 minute
-	const autoRefreshInterval = setInterval(async () => {
-		const isAuthenticated = await slackService.isAuthenticated();
-		if (isAuthenticated) {
-			await slackTreeDataProvider.fetchMessages();
-		}
-	}, REFRESH_INTERVAL_MS);
-	// Clean up the interval when the extension is deactivated
-	context.subscriptions.push({
-		dispose: () => clearInterval(autoRefreshInterval)
+	// Auto-refresh messages at a configurable interval
+	let refreshIntervalSeconds = vscode.workspace.getConfiguration('codereview-slack').get<number>('refreshIntervalSeconds', 60);
+	let autoRefreshInterval: NodeJS.Timeout | undefined;
+	const startAutoRefresh = () => {
+		if (autoRefreshInterval) {
+			clearInterval(autoRefreshInterval);
+		}
+		autoRefreshInterval = setInterval(async () => {
+			const isAuthenticated = await slackService.isAuthenticated();
+			if (isAuthenticated) {
+				await slackTreeDataProvider.fetchMessages();
+			}
+		}, refreshIntervalSeconds * 1000);
+	};
+	startAutoRefresh();
+	// Listen for configuration changes to update the interval dynamically
+	const configChangeDisposable = vscode.workspace.onDidChangeConfiguration(e => {
+		if (e.affectsConfiguration('codereview-slack.refreshIntervalSeconds')) {
+			refreshIntervalSeconds = vscode.workspace.getConfiguration('codereview-slack').get<number>('refreshIntervalSeconds', 60);
+			startAutoRefresh();
+		}
+	});
+	context.subscriptions.push(configChangeDisposable);
+	// Clean up the interval when the extension is deactivated
+	context.subscriptions.push({
+		dispose: () => {
+			if (autoRefreshInterval) {
+				clearInterval(autoRefreshInterval);
+			}
+		}

Adding coreview for slack workspace extension #281508

Are you sure you want to change the base?

Adding coreview for slack workspace extension #281508

Conversation

aiday-mar commented Dec 5, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants