Skip to content

feat: Add search filter to sidebar #562

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.

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@
- Add binary signature verification. This can be disabled with
`coder.disableSignatureVerification` if you purposefully run a binary that is
not signed by Coder (for example a binary you built yourself).
- Add search functionality to the "All Workspaces" view with performance optimizations.
- Implement smart search that prioritizes exact word matches over substring matches.
- Add debounced search input (150ms delay) to improve responsiveness during typing.
- Pre-compile regex patterns and cache metadata strings for better performance.
- Include comprehensive error handling for malformed data and edge cases.
- Add input validation to prevent performance issues from long search terms.

## [v1.9.2](https://github.com/coder/vscode-coder/releases/tag/v1.9.2) 2025-06-25

Expand Down
27 changes: 27 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,18 @@
"title": "Coder: Open App Status",
"icon": "$(robot)",
"when": "coder.authenticated"
},
{
"command": "coder.searchWorkspaces",
"title": "Coder: Search Workspaces",
"icon": "$(search)",
"when": "coder.authenticated && coder.isOwner"
},
{
"command": "coder.clearWorkspaceSearch",
"title": "Coder: Clear Workspace Search",
"icon": "$(clear-all)",
"when": "coder.authenticated && coder.isOwner"
}
],
"menus": {
Expand Down Expand Up @@ -244,6 +256,21 @@
"command": "coder.refreshWorkspaces",
"when": "coder.authenticated && view == myWorkspaces",
"group": "navigation"
},
{
"command": "coder.searchWorkspaces",
"when": "coder.authenticated && coder.isOwner && view == allWorkspaces",
"group": "navigation"
},
{
"command": "coder.refreshWorkspaces",
"when": "coder.authenticated && coder.isOwner && view == allWorkspaces",
"group": "navigation"
},
Comment on lines +265 to +269
Copy link
Member

Choose a reason for hiding this comment

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

feels a little weird to put this in the middle

Copy link
Author

Choose a reason for hiding this comment

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

where do you suggest we move it to? or should we remove it entirely?

{
"command": "coder.clearWorkspaceSearch",
"when": "coder.authenticated && coder.isOwner && view == allWorkspaces",
"group": "navigation"
}
],
"view/item/context": [
Expand Down
40 changes: 40 additions & 0 deletions src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,46 @@ export class Commands {
});
}

/**
* Search/filter workspaces in the All Workspaces view.
* This method will be called from the view title menu.
*/
public async searchWorkspaces(): Promise<void> {
const quickPick = vscode.window.createQuickPick();
quickPick.placeholder =
"Type to search workspaces by name, owner, template, status, or agent";
quickPick.title = "Search All Workspaces";
quickPick.value = "";

// Get current search filter to show in the input
const currentFilter = (await vscode.commands.executeCommand(
"coder.getWorkspaceSearchFilter",
)) as string;
if (currentFilter) {
quickPick.value = currentFilter;
}

quickPick.ignoreFocusOut = true; // Keep open when clicking elsewhere
Copy link
Member

Choose a reason for hiding this comment

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

this seems like an odd thing to set. can you explain your rationale?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, here was my thought process on user behavior -

When a user clicks outside the Quickpick (like on the workspace list or elsewhere in VS Code), the quickpick automatically closes

  1. They type "docker", see the filtered results, but accidentally click outside -> Quickpick closes
  2. User clicks on a workspace to see details, but that closes the search input
  3. User has to reopen the search every time they want to redefine their search

With the current approach it solves the above scenarios. The context is still preserved in state (private searchFilter) so the filtered results will not change if the input closes - its more so just a means for the user to not have to click on the search button again if they are not satisfied or have not completed their search

quickPick.canSelectMany = false; // Don't show selection list

quickPick.onDidChangeValue((value) => {
// Update the search filter in real-time as user types
vscode.commands.executeCommand("coder.setWorkspaceSearchFilter", value);
});

quickPick.onDidAccept(() => {
// When user presses Enter, close the search
quickPick.hide();
});

quickPick.onDidHide(() => {
// Don't clear the search when closed - keep the filter active
quickPick.dispose();
});

quickPick.show();
}

/**
* Return agents from the workspace.
*
Expand Down
16 changes: 16 additions & 0 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,22 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
"coder.viewLogs",
commands.viewLogs.bind(commands),
);
vscode.commands.registerCommand(
"coder.searchWorkspaces",
commands.searchWorkspaces.bind(commands),
);
vscode.commands.registerCommand(
"coder.setWorkspaceSearchFilter",
(searchTerm: string) => {
allWorkspacesProvider.setSearchFilter(searchTerm);
},
);
vscode.commands.registerCommand("coder.getWorkspaceSearchFilter", () => {
return allWorkspacesProvider.getSearchFilter();
});
vscode.commands.registerCommand("coder.clearWorkspaceSearch", () => {
allWorkspacesProvider.clearSearchFilter();
});

// Since the "onResolveRemoteAuthority:ssh-remote" activation event exists
// in package.json we're able to perform actions before the authority is
Expand Down
221 changes: 220 additions & 1 deletion src/workspacesProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ export class WorkspaceProvider
private timeout: NodeJS.Timeout | undefined;
private fetching = false;
private visible = false;
private searchFilter = "";
private metadataCache: Record<string, string> = {};
private searchDebounceTimer: NodeJS.Timeout | undefined;

constructor(
private readonly getWorkspacesQuery: WorkspaceQuery,
Expand All @@ -52,6 +55,43 @@ export class WorkspaceProvider
// No initialization.
}

setSearchFilter(filter: string) {
// Validate search term length to prevent performance issues
if (filter.length > 200) {
filter = filter.substring(0, 200);
}
Comment on lines +59 to +62
Copy link
Member

Choose a reason for hiding this comment

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

what performance issue? why 200 characters?

Copy link
Author

Choose a reason for hiding this comment

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

image

it was mainly implemented due to this comment from Copilot

Copy link
Author

@yelnatscoding yelnatscoding Jul 30, 2025

Choose a reason for hiding this comment

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

Also, i'd like to note:

  • each word in the search term gets compiled into a regex pattern even though we are moving to a word boundary search, we are then testing each word against every workspace
  • we also run string operations .toLowerCase(), .split(), and multiple .includes() checks on concatenated text
  • We're storing all of these regex patterns in memory

The 200 char limit was just an arbitrary number to limit the length of the search input

We could do the following -

   const searchWords = searchTerm.split(/\s+/).filter(word => word.length > 0);
   if (searchWords.length > 10) {
       // Limit to first 10 words
   }

Let me know how we should proceed or what you suggest


// Clear any existing debounce timer
if (this.searchDebounceTimer) {
clearTimeout(this.searchDebounceTimer);
}

// Debounce the search operation to improve performance
this.searchDebounceTimer = setTimeout(() => {
this.searchFilter = filter;
this.refresh(undefined);
this.searchDebounceTimer = undefined;
}, 150); // 150ms debounce delay - good balance between responsiveness and performance
}

getSearchFilter(): string {
return this.searchFilter;
}

/**
* Clear the search filter immediately without debouncing.
* Use this when the user explicitly clears the search.
*/
clearSearchFilter(): void {
// Clear any pending debounce timer
if (this.searchDebounceTimer) {
clearTimeout(this.searchDebounceTimer);
this.searchDebounceTimer = undefined;
}
this.searchFilter = "";
this.refresh(undefined);
}

// fetchAndRefresh fetches new workspaces, re-renders the entire tree, then
// keeps refreshing (if a timer length was provided) as long as the user is
// still logged in and no errors were encountered fetching workspaces.
Expand All @@ -63,6 +103,9 @@ export class WorkspaceProvider
}
this.fetching = true;

// Clear metadata cache when refreshing to ensure data consistency
this.clearMetadataCache();

// It is possible we called fetchAndRefresh() manually (through the button
// for example), in which case we might still have a pending refresh that
// needs to be cleared.
Expand Down Expand Up @@ -201,6 +244,11 @@ export class WorkspaceProvider
clearTimeout(this.timeout);
this.timeout = undefined;
}
// clear search debounce timer
if (this.searchDebounceTimer) {
clearTimeout(this.searchDebounceTimer);
this.searchDebounceTimer = undefined;
}
}

/**
Expand Down Expand Up @@ -300,7 +348,178 @@ export class WorkspaceProvider

return Promise.resolve([]);
}
return Promise.resolve(this.workspaces || []);

// Filter workspaces based on search term
let filteredWorkspaces = this.workspaces || [];
const trimmedFilter = this.searchFilter.trim();
if (trimmedFilter) {
const searchTerm = trimmedFilter.toLowerCase();
filteredWorkspaces = filteredWorkspaces.filter((workspace) =>
this.matchesSearchTerm(workspace, searchTerm),
);
}

return Promise.resolve(filteredWorkspaces);
}

/**
* Extract and normalize searchable text fields from a workspace.
* This helper method reduces code duplication between exact word and substring matching.
*/
private extractSearchableFields(workspace: WorkspaceTreeItem): {
workspaceName: string;
ownerName: string;
templateName: string;
status: string;
agentNames: string[];
agentMetadataText: string;
} {
// Handle null/undefined workspace data safely
const workspaceName = (workspace.workspace.name || "").toLowerCase();
const ownerName = (workspace.workspace.owner_name || "").toLowerCase();
Copy link
Member

Choose a reason for hiding this comment

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

same here

const templateName = (
workspace.workspace.template_display_name ||
workspace.workspace.template_name ||
""
Copy link
Member

Choose a reason for hiding this comment

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

this first || does actually do something, because template_display_name can be empty, and this will fall back to template_name, but if both are empty then it'll be "" anyway so the second || is redundant

).toLowerCase();
const status = (
workspace.workspace.latest_build?.status || ""
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure why there's a ?. on latest_build, it isn't optional

).toLowerCase();

// Extract agent names with null safety
const agents = extractAgents(
workspace.workspace.latest_build?.resources || [],
Copy link
Member

Choose a reason for hiding this comment

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

same here, and the || [] does nothing because resources is also not optional

);
const agentNames = agents
.map((agent) => (agent.name || "").toLowerCase())
.filter((name) => name.length > 0);

// Extract and cache agent metadata with error handling
let agentMetadataText = "";
const metadataCacheKey = agents.map((agent) => agent.id).join(",");

if (this.metadataCache[metadataCacheKey]) {
agentMetadataText = this.metadataCache[metadataCacheKey];
} else {
const metadataStrings: string[] = [];
agents.forEach((agent) => {
const watcher = this.agentWatchers[agent.id];
if (watcher?.metadata) {
watcher.metadata.forEach((metadata) => {
try {
metadataStrings.push(JSON.stringify(metadata).toLowerCase());
} catch (error) {
// Handle JSON serialization errors gracefully
this.storage.output.warn(
`Failed to serialize metadata for agent ${agent.id}: ${error}`,
);
Comment on lines +412 to +415
Copy link
Member

Choose a reason for hiding this comment

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

but now the metadata still gets cached even with a missing entry, right? this seems like a cache corruption bug

Copy link
Author

Choose a reason for hiding this comment

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

fixed this - de2dd41. We now only cache the data if all metadata is serialized successfully

}
});
}
});
agentMetadataText = metadataStrings.join(" ");
this.metadataCache[metadataCacheKey] = agentMetadataText;
}

return {
workspaceName,
ownerName,
templateName,
status,
agentNames,
agentMetadataText,
};
}

/**
* Check if a workspace matches the given search term using smart search logic.
* Prioritizes exact word matches over substring matches.
*/
private matchesSearchTerm(
workspace: WorkspaceTreeItem,
searchTerm: string,
): boolean {
// Early return for empty search terms
if (!searchTerm || searchTerm.trim().length === 0) {
return true;
}

// Extract all searchable fields once
const fields = this.extractSearchableFields(workspace);

// Pre-compile regex patterns for exact word matching
const searchWords = searchTerm
.split(/\s+/)
.filter((word) => word.length > 0);

const regexPatterns: RegExp[] = [];
for (const word of searchWords) {
try {
// Escape special regex characters to prevent injection
const escapedWord = word.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
regexPatterns.push(new RegExp(`\\b${escapedWord}\\b`, "i"));
Copy link
Member

Choose a reason for hiding this comment

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

it's a bit odd to me to try escaping these symbols instead of interpreting them as word boundaries

Copy link
Author

Choose a reason for hiding this comment

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

You're totally right! I was trying to solve: when searching "container", it should show Docker workspaces (since Docker is related to containers). So I wanted fuzzy matching rather than strict exact word matching. The regex approach gives us that flexibility - users can search for "container" and it'll match "Docker" workspaces. I can change this to interpret word boundaries.

} catch (error) {
// Handle invalid regex patterns
this.storage.output.warn(
`Invalid regex pattern for search word "${word}": ${error}`,
);
// Fall back to simple substring matching for this word
continue;
Copy link
Member

Choose a reason for hiding this comment

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

continue at the end of a loop doesn't really do anything

Copy link
Author

Choose a reason for hiding this comment

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

removed - de2dd41

}
}

// Combine all text for exact word matching
const allText = [
fields.workspaceName,
fields.ownerName,
fields.templateName,
fields.status,
...fields.agentNames,
fields.agentMetadataText,
].join(" ");

// Check for exact word matches (higher priority)
const hasExactWordMatch =
regexPatterns.length > 0 &&
regexPatterns.some((pattern) => {
try {
return pattern.test(allText);
Copy link
Member

Choose a reason for hiding this comment

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

does test ever throw an error? it's only documented as returning a boolean on MDN and I can't even get it to throw a TypeError; it just coerces the value to bool.

Copy link
Author

Choose a reason for hiding this comment

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

Yes you are correct here is my understanding of what it does -

Converts the argument to a string using ToString()
Returns true if the regex matches, false otherwise
Never throws an error - even with invalid inputs

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it you're right it never throws an error. That was just a bit of over engineering here. Will be removing this in next commit

Copy link
Author

Choose a reason for hiding this comment

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

removed - de2dd41

} catch (error) {
// Handle regex test errors gracefully
this.storage.output.warn(
`Regex test failed for pattern ${pattern}: ${error}`,
);
return false;
}
});

// Check for substring matches (lower priority) - only if no exact word match
const hasSubstringMatch =
!hasExactWordMatch &&
(fields.workspaceName.includes(searchTerm) ||
fields.ownerName.includes(searchTerm) ||
fields.templateName.includes(searchTerm) ||
fields.status.includes(searchTerm) ||
fields.agentNames.some((agentName) => agentName.includes(searchTerm)) ||
fields.agentMetadataText.includes(searchTerm));

// Return true if either exact word match or substring match
return hasExactWordMatch || hasSubstringMatch;
}

/**
* Clear the metadata cache when workspaces are refreshed to ensure data consistency.
* Also clears cache if it grows too large to prevent memory issues.
*/
private clearMetadataCache(): void {
// Clear cache if it grows too large (prevent memory issues)
const cacheSize = Object.keys(this.metadataCache).length;
if (cacheSize > 1000) {
this.storage.output.info(
`Clearing metadata cache due to size (${cacheSize} entries)`,
);
}
this.metadataCache = {};
}
}

Expand Down