Skip to content

Conversation

@nick-y-snyk
Copy link
Contributor

@nick-y-snyk nick-y-snyk commented Jan 12, 2026

Summary

  • Simplified folder config population logic to avoid unnecessary object creation and merging
  • Cleaned up folder config cloning to properly preserve all settings
  • Removed unnecessary filtering function since filtering happens at source
  • Removed premature auto-save trigger during folder org field initialization
  • Removed obsolete tests that were testing removed filtering behavior

Changes

  1. populateFolderConfigs: Now directly uses stored configs instead of creating empty configs and merging fields
  2. base_scan.go: Properly clones all folder config settings when creating config for scanned directory
  3. config_html.go: Removed filterFolderConfigs function since filtering happens at source in populateFolderConfigs; updated comment to reflect this
  4. config_html_test.go: Removed TestConfigHtmlRenderer_FiltersFolderConfigsNotInWorkspace and TestConfigHtmlRenderer_NoWorkspaceFiltersAllFolders tests that were testing the removed filtering function
  5. folders.js: Removed skipTrigger parameter and auto-save trigger during initialization
  6. form-state.js: Improved event listener setup for better clarity
  7. xdg.go: Fixed createNewStoredConfig to use proper Save function instead of direct Set
  8. lsp.go: Cleaned up FolderConfig.Clone() implementation

Test plan

  • Verify all remaining configuration tests pass
  • Verify folder configs load correctly in configuration UI
  • Verify organization field toggles work without premature saves
  • Verify scanned directory configs inherit all settings from working directory
  • Verify folder configs persist correctly across sessions

@snyk-io
Copy link

snyk-io bot commented Jan 12, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @nick-y-snyk, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses several issues related to folder configuration management, primarily focusing on eliminating race conditions and improving the robustness of configuration handling. The changes simplify how folder configurations are populated and cloned, ensuring consistency and preventing data loss or incorrect states. Additionally, premature auto-save triggers in the UI have been removed to enhance stability during initialization.

Highlights

  • Race Condition Fixes: Resolved race conditions occurring during folder configuration handling, particularly during the configuration UI initialization process.
  • Simplified Configuration Logic: Streamlined the logic for populating folder configurations, eliminating unnecessary object creation and merging operations.
  • Improved Folder Config Cloning: Ensured that all settings are correctly preserved when cloning folder configurations, especially when creating configurations for newly scanned directories.
  • Prevented Premature Auto-Saves: Removed auto-save triggers that were firing prematurely during initialization, which contributed to race conditions.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces several positive changes to address race conditions in folder configuration handling. The simplifications in config population, the robust cloning of folder settings, and the removal of premature auto-save triggers are all welcome improvements. However, I've identified a potential issue in the FolderConfig.Clone() method where a map is now being shallow-copied, which could lead to unintended side effects. I've also noted a minor formatting inconsistency. Please see my detailed comments below.

Comment on lines +65 to +67
} else {
dom.addEvent(inputs[i], "blur", formState.triggerChangeHandlers);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The indentation in this else block is inconsistent with the rest of the file, which appears to use tabs. Using mixed indentation (spaces and tabs) can make the code harder to read and maintain. It's best to stick to a consistent style throughout the file.

Suggested change
} else {
dom.addEvent(inputs[i], "blur", formState.triggerChangeHandlers);
}
} else {
dom.addEvent(inputs[i], "blur", formState.triggerChangeHandlers);
}

Fixes multiple race conditions and inefficiencies in folder configuration handling:

1. Simplified populateFolderConfigs to use stored configs directly instead of creating empty configs and merging
2. Fixed base_scan.go to properly clone all folder config settings when creating config for scanned directory
3. Removed unnecessary filterFolderConfigs function - filtering now happens at source in populateFolderConfigs
4. Removed premature auto-save trigger during folder org field initialization to prevent race conditions
5. Improved form-state.js event listener setup for better clarity
6. Fixed createNewStoredConfig to use proper Save function instead of direct Set
7. Cleaned up FolderConfig.Clone() implementation to be more consistent

These changes ensure folder configs are properly initialized and prevent race conditions during config updates.
@nick-y-snyk nick-y-snyk force-pushed the fix/folder-config-update-race branch from a4b0252 to 38b4d04 Compare January 12, 2026 19:05
@nick-y-snyk nick-y-snyk changed the title fix: resolve folder config update race conditions refactor: cleanup folder config handling and remove obsolete tests Jan 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants