Release v5.2.1 #34
Merged
Release v5.2.1 #34
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When rejecting a tool/command (like exec_cmd), the sendingDisabled state was being set to true, which caused subsequent messages to be queued instead of sent directly. This fix excludes tool/command rejection cases from setting sendingDisabled, allowing normal message flow after rejection.
Contributor
|
ContextRelease v5.2.1 focusing on CI/CD optimization and branding alignment within the terminal command generation utility. ImplementationSummary By MatterAI
🔄 What ChangedThis update removes the deprecated 🔍 Impact of the Change
📁 Total Files ChangedClick to Expand
🧪 Test Added/RecommendedAdded
Recommended
🔒 Security VulnerabilitiesN/A. No security-sensitive code was modified. Screenshots
How to Test
Get in Touch
⏳ Estimated code review effortLOW (~5 minutes) Tip Quality Recommendations
♫ Tanka Poem
Sequence DiagramsequenceDiagram
participant T as TestRunner
participant G as TerminalCommandGenerator
participant V as VSCode API
Note over T,V: Branding Update Flow
T->>G: generateCommand()
G->>V: showInputBox({ prompt: "Axon: Describe..." })
V-->>G: userInput
G-->>T: validatePromptString()
|
![matter-code-review[bot]](https://avatars.githubusercontent.com/in/474549?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧪 PR Review is completed: Review of Release v5.2.1. Major findings include broken directory skip patterns for root-level files, a bug in the .gitignore skip pattern, and potential race conditions in the chat UI when handling command acceptances.
Skipped files
jetbrains/plugin/src/main/resources/icons/matterai-ic.png: File hunk diff too largejetbrains/plugin/src/main/resources/icons/matterai-ic.svg: Skipped file patternsrc/assets/icons/matterai-ic.png: File hunk diff too largesrc/assets/icons/matterai-ic.svg: Skipped file patternwebview-ui/src/i18n/locales/en/memories.json: Skipped file pattern
⬇️ Low Priority Suggestions (2)
webview-ui/src/components/chat/ChatView.tsx (1 suggestion)
Location:
webview-ui/src/components/chat/ChatView.tsx(Lines 868-878)🟡 Medium Priority - Logic Error
Issue: The updated logic prevents disabling input for ALL responses to commands/tools, including when the user clicks "Yes" (acceptance). This leaves the input enabled during the latency between clicking "Yes" and the bot state updating to "busy", potentially allowing race conditions or confusing UX.
Fix: Only skip disabling input if the response is a rejection (
noButtonClicked).Impact: Ensures the UI is correctly locked when the bot is about to execute a command.
- // Only set sendingDisabled to true for cases that need it (not for command_output abort or tool/command rejection) - // When rejecting a tool/command, we don't want to block subsequent messages - if ( - clineAsk !== "command_output" && - clineAsk !== "command" && - clineAsk !== "tool" && - clineAsk !== "browser_action_launch" && - clineAsk !== "use_mcp_server" - ) { - setSendingDisabled(true) - } + // Only set sendingDisabled to true for cases that need it (not for command_output abort or tool/command rejection) + // When rejecting a tool/command, we don't want to block subsequent messages + const isRejection = askResponse === "noButtonClicked" + if ( + !isRejection && ( + clineAsk !== "command_output" && + clineAsk !== "command" && + clineAsk !== "tool" && + clineAsk !== "browser_action_launch" && + clineAsk !== "use_mcp_server" + ) + ) { + setSendingDisabled(true) + }
src/core/webview/webviewMessageHandler.ts (1 suggestion)
Location:
src/core/webview/webviewMessageHandler.ts(Lines 99-377)🔵 Low Priority - Code Quality
Issue:
skipPatternsandshouldSkipFileare duplicated in the test fileskipPatterns.spec.ts. This creates a maintenance risk where the implementation and tests can diverge.Fix: Export
skipPatternsandshouldSkipFileso they can be imported by the test file.Impact: Improves maintainability and ensures tests run against the actual implementation.
- const skipPatterns = [ - // Package managers - /package-lock\.json$/, - /\.env$/, - /\.vscodeignore$/, - /Makefile$/, - /\.md$/, - /\.mdx$/, - /\.json$/, - /yarn\.lock$/, - /pnpm-lock\.yaml$/, - /Podfile\.lock$/, - /Gemfile\.lock$/, - /composer\.lock$/, - /cargo\.lock$/, - /poetry\.lock$/, - /Pipfile\.lock$/, - /project\.assets\.json$/, - /packages\.lock\.json$/, - /npm-shrinkwrap\.json$/, - /bun\.lockb$/, - - /\.firebaserc$/, - /firebase\.json$/, - /\.prettierrc\.json$/, - /\.npmrc$/, - /config-overrides\.js$/, - /jsconfig\.json$/, - /robots\.txt$/, - - // Build directories - ALL FIXED with proper anchoring - /\/dist\//, - /\/build\//, - /\/\.next\//, - /\/node_modules\//, - /\/out\//, - /\/\.gradle\//, - /\/\.dart_tool\//, - /\/\.pub-cache\//, - /\/\.pub\//, - /\/\.nuxt\//, - /\/\.output\//, - /\/target\//, - /\/bin\//, - /\/obj\//, - /\/\.venv\//, - /\/venv\//, - /\/env\//, - /\/__pycache__\//, - /\/\.pytest_cache\//, - /\/\.tox\//, - /\/\.eggs\//, - /\/\.bundle\//, - /\/vendor\//, - - // Minified files - /\.min\.js$/, - /\.min\.css$/, - /\.bundle\.js$/, - /\.bundle\.css$/, - - // iOS/Xcode - /\.pbxproj$/, - /\.xcworkspacedata$/, - /\.xcscheme$/, - /\.xcuserstate$/, - /\.plist$/, - /\.nib$/, - /\.storyboardc$/, - - // Android - /\.apk$/, - /\.aab$/, - /R\.java$/, - /BuildConfig\.java$/, - /\.iml$/, - /\.aar$/, - /\.dex$/, - - // Flutter - /\.g\.dart$/, - /\.freezed\.dart$/, - /flutter_export_environment\.sh$/, - /Flutter\.podspec$/, - - // Generated files across frameworks - FIXED - /\/generated\//, - /\/\.generated\//, - /\/\.gen\//, - /\/auto-generated\//, - /\.designer\./, - /\.Designer\./, - - // Config and metadata files - FIXED - /\.DS_Store$/, - /Thumbs\.db$/, - /desktop\.ini$/, - /\/\.idea\//, - /\/\.vscode\//, - /\/\.vs\//, - /\.project$/, - /\.classpath$/, - /\/\.settings\//, - /\.editorconfig$/, - /\.gitattributes$/, - /\/\.gitignore\//, // Fixed but this might need to be /\.gitignore$/ if you want the file itself - /\/\.vercel\//, - /\/netlify\//, // Fixed from /\netlify/ - - // Compiled binaries - /\.so$/, - /\.dylib$/, - /\.dll$/, - /\.class$/, - /\.pyc$/, - /\.pyo$/, - /\.o$/, - /\.obj$/, - /\.a$/, - /\.lib$/, - /\.exe$/, - /\.pdb$/, - /\.ilk$/, - /\.map$/, - /\.jar$/, - /\.war$/, - /\.ear$/, - - /prometheus\.yml$/, - /grafana\.yml$/, - /grafana-dashboard\.yml$/, - - // Filter all files with .cursor in their path - FIXED - /\/\.cursor\//, - /\/\.github\//, - - // Go files - // Skip Go files generated by protoc-gen - /\.pb\.go$/, - // Skip gRPC-Gateway generated Go files - /\.pb\.gw\.go$/, - // Skip other common generated Go files - /\.gen\.go$/, - /mock_.+\.go$/, - /_string\.go$/, - /go\.sum$/, - /go\.mod$/, - /\.go\.orig$/, - - // API specs - /swagger\.json$/, - /swagger\.yaml$/, - /swagger\.yml$/, - /openapi\.json$/, - /openapi\.yaml$/, - /openapi\.yml$/, - - // Properties and config files - /buildconfig\.properties$/, - /application\.properties$/, - /application\.yml$/, - /application-.*\.properties$/, - /application-.*\.yml$/, - /\.config$/, - /\.conf$/, - /\.ini$/, - /\.toml$/, - /\.png$/, - /\.jpg$/, - /\.jpeg$/, - /\.gif$/, - /\.bmp$/, - /\.webp$/, - /\.svg$/, - /\.ico$/, - /\.tiff$/, - /\.tif$/, - /\.mp4$/, - /\.mp3$/, - /\.wav$/, - /\.ogg$/, - /\.mov$/, - /\.avi$/, - /\.wmv$/, - /\.flv$/, - /\.mkv$/, - /\.pdf$/, - /\.zip$/, - /\.tar$/, - /\.gz$/, - /\.rar$/, - /\.7z$/, - /\.exe$/, - /\.dll$/, - /\.bin$/, - /\.so$/, - /\.dylib$/, - /\.jar$/, - /\.wasm$/, - /\.psd$/, - /\.ai$/, - /\.eps$/, - /\.ttf$/, - /\.woff$/, - /\.woff2$/, - /\.eot$/, - /\.otf$/, - /\.apk$/, - /\.ipa$/, - /\.dmg$/, - /\.iso$/, - /\.csv$/, - - // Data files, datasets, and dumps - /\.tsv$/, - /\.psv$/, - /\.xlsx$/, - /\.xls$/, - /\.xlsm$/, - /\.xlsb$/, - /\.ods$/, - /\.sql$/, - /\.dump$/, - /\.dmp$/, - /\.sqlite$/, - /\.sqlite3$/, - /\.db$/, - /\.db3$/, - /\.psql$/, - /\.pgsql$/, - /\.parquet$/, - /\.avro$/, - /\.feather$/, - /\.orc$/, - /\.rds$/, - /\.rdata$/, - - // Log and temporary artifacts - /\.log(\.[\w.-]+)?$/, - /\.tmp$/, - /\.temp$/, - /\.bak$/, - /\.backup$/, - /\.swp$/, - /\.swo$/, - /\.swn$/, - /\.orig$/, - /\.rej$/, - /\.diff$/, - /\.patch$/, - - // Archive and binary blobs - /\.tgz$/, - /\.bz2$/, - /\.xz$/, - /\.lz4$/, - - // Coverage, cache, and artifact directories - /\/coverage\//, - /\/reports?\//, - /\/artifacts?\//, - /\/test-output\//, - /\/build-output\//, - /\/build-artifacts\//, - /\/\.nyc_output\//, - /\/\.cache\//, - /\/\.sass-cache\//, - /\/logs\//, - /\/log\//, - /\/tmp\//, - /\/temp\//, - ] - - /** - * Check if a file should be skipped from code review based on skip patterns - * @param filePath - The file path to check (relative or absolute) - * @returns true if the file should be skipped, false otherwise - */ - function shouldSkipFile(filePath: string): boolean { + export const skipPatterns = [ + // Package managers + /package-lock\.json$/, + /\.env$/, + /\.vscodeignore$/, + /Makefile$/, + /\.md$/, + /\.mdx$/, + /\.json$/, + /yarn\.lock$/, + /pnpm-lock\.yaml$/, + /Podfile\.lock$/, + /Gemfile\.lock$/, + /composer\.lock$/, + /cargo\.lock$/, + /poetry\.lock$/, + /Pipfile\.lock$/, + /project\.assets\.json$/, + /packages\.lock\.json$/, + /npm-shrinkwrap\.json$/, + /bun\.lockb$/, + + /\.firebaserc$/, + /firebase\.json$/, + /\.prettierrc\.json$/, + /\.npmrc$/, + /config-overrides\.js$/, + /jsconfig\.json$/, + /robots\.txt$/, + + // Build directories - ALL FIXED with proper anchoring + /\/dist\//, + /\/build\//, + /\/\.next\//, + /\/node_modules\//, + /\/out\//, + /\/\.gradle\//, + /\/\.dart_tool\//, + /\/\.pub-cache\//, + /\/\.pub\//, + /\/\.nuxt\//, + /\/\.output\//, + /\/target\//, + /\/bin\//, + /\/obj\//, + /\/\.venv\//, + /\/venv\//, + /\/env\//, + /\/__pycache__\//, + /\/\.pytest_cache\//, + /\/\.tox\//, + /\/\.eggs\//, + /\/\.bundle\//, + /\/vendor\//, + + // Minified files + /\.min\.js$/, + /\.min\.css$/, + /\.bundle\.js$/, + /\.bundle\.css$/, + + // iOS/Xcode + /\.pbxproj$/, + /\.xcworkspacedata$/, + /\.xcscheme$/, + /\.xcuserstate$/, + /\.plist$/, + /\.nib$/, + /\.storyboardc$/, + + // Android + /\.apk$/, + /\.aab$/, + /R\.java$/, + /BuildConfig\.java$/, + /\.iml$/, + /\.aar$/, + /\.dex$/, + + // Flutter + /\.g\.dart$/, + /\.freezed\.dart$/, + /flutter_export_environment\.sh$/, + /Flutter\.podspec$/, + + // Generated files across frameworks - FIXED + /\/generated\//, + /\/\.generated\//, + /\/\.gen\//, + /\/auto-generated\//, + /\.designer\./, + /\.Designer\./, + + // Config and metadata files - FIXED + /\.DS_Store$/, + /Thumbs\.db$/, + /desktop\.ini$/, + /\/\.idea\//, + /\/\.vscode\//, + /\/\.vs\//, + /\.project$/, + /\.classpath$/, + /\/\.settings\//, + /\.editorconfig$/, + /\.gitattributes$/, + /\/\.gitignore\//, // Fixed but this might need to be /\.gitignore$/ if you want the file itself + /\/\.vercel\//, + /\/netlify\//, // Fixed from /\netlify/ + + // Compiled binaries + /\.so$/, + /\.dylib$/, + /\.dll$/, + /\.class$/, + /\.pyc$/, + /\.pyo$/, + /\.o$/, + /\.obj$/, + /\.a$/, + /\.lib$/, + /\.exe$/, + /\.pdb$/, + /\.ilk$/, + /\.map$/, + /\.jar$/, + /\.war$/, + /\.ear$/, + + /prometheus\.yml$/, + /grafana\.yml$/, + /grafana-dashboard\.yml$/, + + // Filter all files with .cursor in their path - FIXED + /\/\.cursor\//, + /\/\.github\//, + + // Go files + // Skip Go files generated by protoc-gen + /\.pb\.go$/, + // Skip gRPC-Gateway generated Go files + /\.pb\.gw\.go$/, + // Skip other common generated Go files + /\.gen\.go$/, + /mock_.+\.go$/, + /_string\.go$/, + /go\.sum$/, + /go\.mod$/, + /\.go\.orig$/, + + // API specs + /swagger\.json$/, + /swagger\.yaml$/, + /swagger\.yml$/, + /openapi\.json$/, + /openapi\.yaml$/, + /openapi\.yml$/, + + // Properties and config files + /buildconfig\.properties$/, + /application\.properties$/, + /application\.yml$/, + /application-.*\.properties$/, + /application-.*\.yml$/, + /\.config$/, + /\.conf$/, + /\.ini$/, + /\.toml$/, + /\.png$/, + /\.jpg$/, + /\.jpeg$/, + /\.gif$/, + /\.bmp$/, + /\.webp$/, + /\.svg$/, + /\.ico$/, + /\.tiff$/, + /\.tif$/, + /\.mp4$/, + /\.mp3$/, + /\.wav$/, + /\.ogg$/, + /\.mov$/, + /\.avi$/, + /\.wmv$/, + /\.flv$/, + /\.mkv$/, + /\.pdf$/, + /\.zip$/, + /\.tar$/, + /\.gz$/, + /\.rar$/, + /\.7z$/, + /\.exe$/, + /\.dll$/, + /\.bin$/, + /\.so$/, + /\.dylib$/, + /\.jar$/, + /\.wasm$/, + /\.psd$/, + /\.ai$/, + /\.eps$/, + /\.ttf$/, + /\.woff$/, + /\.woff2$/, + /\.eot$/, + /\.otf$/, + /\.apk$/, + /\.ipa$/, + /\.dmg$/, + /\.iso$/, + /\.csv$/, + + // Data files, datasets, and dumps + /\.tsv$/, + /\.psv$/, + /\.xlsx$/, + /\.xls$/, + /\.xlsm$/, + /\.xlsb$/, + /\.ods$/, + /\.sql$/, + /\.dump$/, + /\.dmp$/, + /\.sqlite$/, + /\.sqlite3$/, + /\.db$/, + /\.db3$/, + /\.psql$/, + /\.pgsql$/, + /\.parquet$/, + /\.avro$/, + /\.feather$/, + /\.orc$/, + /\.rds$/, + /\.rdata$/, + + // Log and temporary artifacts + /\.log(\.[\w.-]+)?$/, + /\.tmp$/, + /\.temp$/, + /\.bak$/, + /\.backup$/, + /\.swp$/, + /\.swo$/, + /\.swn$/, + /\.orig$/, + /\.rej$/, + /\.diff$/, + /\.patch$/, + + // Archive and binary blobs + /\.tgz$/, + /\.bz2$/, + /\.xz$/, + /\.lz4$/, + + // Coverage, cache, and artifact directories + /\/coverage\//, + /\/reports?\//, + /\/artifacts?\//, + /\/test-output\//, + /\/build-output\//, + /\/build-artifacts\//, + /\/\.nyc_output\//, + /\/\.cache\//, + /\/\.sass-cache\//, + /\/logs\//, + /\/log\//, + /\/tmp\//, + /\/temp\//, + ] + + /** + * Check if a file should be skipped from code review based on skip patterns + * @param filePath - The file path to check (relative or absolute) + * @returns true if the file should be skipped, false otherwise + */ + export function shouldSkipFile(filePath: string): boolean {
Comment on lines
+377
to
+389
| function shouldSkipFile(filePath: string): boolean { | ||
| // Normalize the path to use forward slashes for consistent pattern matching | ||
| const normalizedPath = filePath.replace(/\\/g, "/") | ||
|
|
||
| // Check against all skip patterns | ||
| for (const pattern of skipPatterns) { | ||
| if (pattern.test(normalizedPath)) { | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| return false | ||
| } |
Contributor
There was a problem hiding this comment.
🔴 High Priority - Logic Error
Issue: The directory skip patterns (e.g., /\/dist\//) expect a leading slash. However, file paths from git diff and untracked files are typically relative (e.g., dist/main.js) and do not start with /. This causes root-level directories to NOT be skipped as intended.
Fix: Prepend a slash to the normalized path in shouldSkipFile to ensure consistent matching for both root and nested directories against the defined patterns.
Impact: Ensures files in ignored directories like dist, node_modules, etc., are correctly excluded from the review.
Suggested change
| function shouldSkipFile(filePath: string): boolean { | |
| // Normalize the path to use forward slashes for consistent pattern matching | |
| const normalizedPath = filePath.replace(/\\/g, "/") | |
| // Check against all skip patterns | |
| for (const pattern of skipPatterns) { | |
| if (pattern.test(normalizedPath)) { | |
| return true | |
| } | |
| } | |
| return false | |
| } | |
| function shouldSkipFile(filePath: string): boolean { | |
| // Normalize the path to use forward slashes for consistent pattern matching | |
| const normalizedPath = filePath.replace(/\\/g, "/") | |
| // Ensure path starts with / to match directory patterns that expect it (e.g. /dist/) | |
| const pathToCheck = normalizedPath.startsWith("/") ? normalizedPath : "/" + normalizedPath | |
| // Check against all skip patterns | |
| for (const pattern of skipPatterns) { | |
| if (pattern.test(pathToCheck)) { | |
| return true | |
| } | |
| } | |
| return false | |
| } |
| /\/\.settings\//, | ||
| /\.editorconfig$/, | ||
| /\.gitattributes$/, | ||
| /\/\.gitignore\//, // Fixed but this might need to be /\.gitignore$/ if you want the file itself |
Contributor
There was a problem hiding this comment.
🟡 Medium Priority - Regex Bug
Issue: The pattern /\/\.gitignore\// matches a directory named .gitignore, not the file itself. The comment acknowledges this might be incorrect.
Fix: Update the regex to match the .gitignore file at the end of a path.
Impact: Correctly skips .gitignore files from review.
Suggested change
| /\/\.gitignore\//, // Fixed but this might need to be /\.gitignore$/ if you want the file itself | |
| /\.gitignore$/, |
Comment on lines
+1960
to
+1968
| case "delete_memory": { | ||
| const { MemoryManager } = await import("../../services/chat-memory/MemoryManager") | ||
| const memoryManager = new MemoryManager(provider.context.globalStorageUri.fsPath) | ||
| await memoryManager.deleteMemory(message.memoryId!) | ||
| await provider.postMessageToWebview({ | ||
| type: "memory_deleted", | ||
| }) | ||
| break | ||
| } |
Contributor
There was a problem hiding this comment.
🔴 High Priority - Null Safety
Issue: message.memoryId is accessed with a non-null assertion (!) without prior validation. If the webview sends a malformed message, this could throw an error or pass undefined to deleteMemory.
Fix: Add a check to ensure memoryId exists before proceeding.
Impact: Prevents runtime errors and potential crashes.
Suggested change
| case "delete_memory": { | |
| const { MemoryManager } = await import("../../services/chat-memory/MemoryManager") | |
| const memoryManager = new MemoryManager(provider.context.globalStorageUri.fsPath) | |
| await memoryManager.deleteMemory(message.memoryId!) | |
| await provider.postMessageToWebview({ | |
| type: "memory_deleted", | |
| }) | |
| break | |
| } | |
| case "delete_memory": { | |
| const { MemoryManager } = await import("../../services/chat-memory/MemoryManager") | |
| const memoryManager = new MemoryManager(provider.context.globalStorageUri.fsPath) | |
| if (message.memoryId) { | |
| await memoryManager.deleteMemory(message.memoryId) | |
| await provider.postMessageToWebview({ | |
| type: "memory_deleted", | |
| }) | |
| } | |
| break | |
| } |
- Update translations across all supported languages - Add new memories.json locale files for webview-ui - Update documentation for browser-use and custom-modes features - Remove deprecated changeset-release workflow - Update terminal command generator tests
Contributor
|
✅ Reviewed the changes: The changes correctly update the test expectations to match the new 'Axon' branding. The mock translation and the expected call arguments are consistent. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Release v5.2.1 changes