Open
Conversation
VictoriousRaptor
added
enhancement
…r handling Agent-Logs-Url: https://github.com/Flow-Launcher/Flow.Launcher/sessions/bfa04540-ad54-4f04-a7cf-fed5dbc06dcc Co-authored-by: VictoriousRaptor <10308169+VictoriousRaptor@users.noreply.github.com>
This comment was marked as off-topic.
This comment was marked as off-topic.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 11 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingSearchManager.cs">
<violation number="1" location="Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingSearchManager.cs:20">
P2: `_api` is written under `lock (_syncRoot)` in `ReloadApi` but read from multiple async methods without taking the lock. Mark the field `volatile` so that concurrent readers always see the latest API instance after a reload.</violation>
</file>
<file name="Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs">
<violation number="1" location="Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs:125">
P1: Handle the timeout result from `WaitAsync(TimeSpan)` before releasing the semaphore; otherwise a timed-out wait can cause `SemaphoreFullException` in `finally`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs
Outdated
Show resolved
Hide resolved
Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingSearchManager.cs
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingAPI.cs (1)
170-184:⚠️ Potential issue | 🟠 MajorCheck the timeout result before releasing
_semaphore.
SemaphoreSlim.WaitAsync(TimeSpan)returns aTask<bool>that completes withfalseon timeout. The code ignores this result, so if the wait times out, the semaphore is never acquired but thefinallyblock still callsRelease(). This increments the semaphore'sCurrentCountwithout a corresponding decrement, unbalancing its state and allowing more concurrent acquisitions than intended. Because callers fire-and-forget this method, these imbalances can silently accumulate.Suggested fix
- await _semaphore.WaitAsync(TimeSpan.FromSeconds(1)); + if (!await _semaphore.WaitAsync(TimeSpan.FromSeconds(1))) + { + return; + } try { _ = EverythingApiDllImport.Everything_IncRunCountFromFileName(fileOrFolder);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingAPI.cs` around lines 170 - 184, The IncrementRunCounterAsync method currently ignores the bool result from _semaphore.WaitAsync(TimeSpan.FromSeconds(1)), causing Release() to be called even when the wait timed out; change the method to await the Task<bool> result into a variable (e.g., var acquired = await _semaphore.WaitAsync(TimeSpan.FromSeconds(1));), only call EverythingApiDllImport.Everything_IncRunCountFromFileName(fileOrFolder) if acquired is true, and only call _semaphore.Release() in the finally block when acquired is true (skip Release when acquired is false) so the semaphore count stays balanced.
♻️ Duplicate comments (2)
Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs (2)
148-161:⚠️ Potential issue | 🟡 MinorReturn
falsefor unmapped sort options.If
TryConvertSortOptionfalls through, Line 161 still reports the sort as fast. That makes future enum members or invalid casts bypass the slow-sort path.Suggested fix
- return true; + return false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs` around lines 148 - 161, The code currently returns true at the end of the method when TryConvertSortOption fails to map the sortOption, incorrectly treating unmapped or new enum values as "fast" sorts; update the method so that if TryConvertSortOption returns false the method returns false instead of true. Specifically, in the method containing TryConvertSortOption, Everything3ApiDllImport.Everything3_IsPropertyFastSort(client, propertyId), CheckAndThrowExceptionOnErrorFromEverything3(), and Everything3ApiDllImport.Everything3_DestroyClient(client), preserve the try/finally cleanup but change the final fallback return from true to false so unmapped sort options go down the slow-sort path.
181-197:⚠️ Potential issue | 🟠 MajorFail fast on the first Everything3 setup error.
These setup calls already return
bool. If one of Lines 181-197 fails and later setup continues,Everything3_GetLastError()can be overwritten and the search keeps running with a partially configuredsearchState.Suggested fix
+ static void ThrowIfSetupFailed(bool success) + { + if (!success) + { + CheckAndThrowExceptionOnErrorFromEverything3(); + } + } + - _ = Everything3ApiDllImport.Everything3_SetSearchRegex(searchState, useRegex); - _ = Everything3ApiDllImport.Everything3_SetSearchMatchPath(searchState, option.IsFullPathSearch); - _ = Everything3ApiDllImport.Everything3_SetSearchTextW(searchState, searchText); - _ = Everything3ApiDllImport.Everything3_SetSearchHideResultOmissions(searchState, true); - _ = Everything3ApiDllImport.Everything3_SetSearchViewportOffset(searchState, (nuint)option.Offset); - _ = Everything3ApiDllImport.Everything3_SetSearchViewportCount(searchState, (nuint)option.MaxCount); + ThrowIfSetupFailed(Everything3ApiDllImport.Everything3_SetSearchRegex(searchState, useRegex)); + ThrowIfSetupFailed(Everything3ApiDllImport.Everything3_SetSearchMatchPath(searchState, option.IsFullPathSearch)); + ThrowIfSetupFailed(Everything3ApiDllImport.Everything3_SetSearchTextW(searchState, searchText)); + ThrowIfSetupFailed(Everything3ApiDllImport.Everything3_SetSearchHideResultOmissions(searchState, true)); + ThrowIfSetupFailed(Everything3ApiDllImport.Everything3_SetSearchViewportOffset(searchState, (nuint)option.Offset)); + ThrowIfSetupFailed(Everything3ApiDllImport.Everything3_SetSearchViewportCount(searchState, (nuint)option.MaxCount));In the Everything 1.5 / Everything3 SDK, do the `Everything3_SetSearch*`, `Everything3_AddSearchSort`, and `Everything3_AddSearchPropertyRequest*` functions report failure through their `bool` return value, and should `Everything3_GetLastError()` be checked immediately after a `false` result?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs` around lines 181 - 197, The setup calls that configure the Everything3 search (Everything3_SetSearchRegex, Everything3_SetSearchMatchPath, Everything3_SetSearchTextW, Everything3_SetSearchHideResultOmissions, Everything3_SetSearchViewportOffset, Everything3_SetSearchViewportCount, Everything3_AddSearchSort, Everything3_ClearSearchPropertyRequests, Everything3_AddSearchPropertyRequestHighlighted, Everything3_AddSearchPropertyRequest) currently ignore their bool return values; update the sequence to check each call's return and fail fast on the first false by calling Everything3_GetLastError() and surface/throw a meaningful error (or return a failure result) referencing the current searchState so the method does not continue with a partially configured searchState. Ensure TryConvertSortOption branch still checks Everything3_AddSearchSort's return and handles errors immediately.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs`:
- Around line 123-138: The method IncrementRunCounterAsync calls
_semaphore.WaitAsync(TimeSpan.FromSeconds(1)) but never checks its boolean
result, so Release() in the finally block can be called without a matching Wait;
change it to capture the result (e.g., var acquired = await
_semaphore.WaitAsync(...)), only execute the protected work (the
TryUseEverything3Client call) when acquired is true, and only call
_semaphore.Release() in the finally block if acquired is true; reference the
IncrementRunCounterAsync method, the _semaphore field, and the
TryUseEverything3Client/Everything3ApiDllImport.Everything3_IncRunCountFromFilenameW
invocation when making the change.
In
`@Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingSearchManager.cs`:
- Line 32: The issue is racey access to the mutable field _api during
ReloadApi/LoadDllCore: calls like IsEverythingRunningAsync and other
search/availability/run-counter methods access _api without coordination while
ReloadApi unloads and replaces native DLLs, which can detach modules under
in-flight calls. Fix by introducing safe handoff and in-flight protection:
implement an API reference wrapper (e.g., AcquireApi()/ReleaseApi() or a small
ref-counted ApiHolder) used by all callers (IsEverythingRunningAsync and the
methods in the 93-149 and 151-197 regions) so a call increments a usage count
before invoking _api and decrements after; modify ReloadApi()/LoadDllCore() to
Load() the new native DLL and create the new API instance first, then atomically
swap the active ApiHolder (Interlocked.Exchange) and only after the swap
decrement/unload the old holder when its usage count hits zero; alternatively
use a ReaderWriterLockSlim where callers take a read lock and ReloadApi takes a
write lock, but ensure Load() occurs before releasing/unloading the old native
module. Ensure all places that reference _api are changed to use the
Acquire/Release wrapper (or read-lock) so no unload happens while calls are in
flight.
In `@Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs`:
- Around line 166-167: The property initializer in Settings.cs references a
removed type EverythingApi; update it to use
EverythingApiV3.DefaultEverything15InstanceName instead. Locate the
EnableEverything15Support and Everything15InstanceName properties and change the
initializer for Everything15InstanceName to use
EverythingApiV3.DefaultEverything15InstanceName so it matches other usages of
EverythingApiV3 in the codebase.
In `@Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs`:
- Around line 669-687: The setter for EnableEverything15Support (and similarly
Everything15InstanceName) updates Settings.EnableEverything15Support before
calling Settings.EverythingManagerInstance.ReloadApi, risking a mismatch if
ReloadApi throws; change the logic so you first call ReloadApi with the
candidate values (or call ReloadApi in a try and on success assign
Settings.EnableEverything15Support/Everything15InstanceName), and if ReloadApi
fails roll back any in-memory change (or avoid changing Settings until after
successful ReloadApi); reference the properties EnableEverything15Support and
Everything15InstanceName and the method EverythingManagerInstance.ReloadApi to
locate and update the code accordingly.
---
Outside diff comments:
In `@Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingAPI.cs`:
- Around line 170-184: The IncrementRunCounterAsync method currently ignores the
bool result from _semaphore.WaitAsync(TimeSpan.FromSeconds(1)), causing
Release() to be called even when the wait timed out; change the method to await
the Task<bool> result into a variable (e.g., var acquired = await
_semaphore.WaitAsync(TimeSpan.FromSeconds(1));), only call
EverythingApiDllImport.Everything_IncRunCountFromFileName(fileOrFolder) if
acquired is true, and only call _semaphore.Release() in the finally block when
acquired is true (skip Release when acquired is false) so the semaphore count
stays balanced.
---
Duplicate comments:
In `@Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs`:
- Around line 148-161: The code currently returns true at the end of the method
when TryConvertSortOption fails to map the sortOption, incorrectly treating
unmapped or new enum values as "fast" sorts; update the method so that if
TryConvertSortOption returns false the method returns false instead of true.
Specifically, in the method containing TryConvertSortOption,
Everything3ApiDllImport.Everything3_IsPropertyFastSort(client, propertyId),
CheckAndThrowExceptionOnErrorFromEverything3(), and
Everything3ApiDllImport.Everything3_DestroyClient(client), preserve the
try/finally cleanup but change the final fallback return from true to false so
unmapped sort options go down the slow-sort path.
- Around line 181-197: The setup calls that configure the Everything3 search
(Everything3_SetSearchRegex, Everything3_SetSearchMatchPath,
Everything3_SetSearchTextW, Everything3_SetSearchHideResultOmissions,
Everything3_SetSearchViewportOffset, Everything3_SetSearchViewportCount,
Everything3_AddSearchSort, Everything3_ClearSearchPropertyRequests,
Everything3_AddSearchPropertyRequestHighlighted,
Everything3_AddSearchPropertyRequest) currently ignore their bool return values;
update the sequence to check each call's return and fail fast on the first false
by calling Everything3_GetLastError() and surface/throw a meaningful error (or
return a failure result) referencing the current searchState so the method does
not continue with a partially configured searchState. Ensure
TryConvertSortOption branch still checks Everything3_AddSearchSort's return and
handles errors immediately.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eb76069c-66f8-40f9-a56e-6472606814da
📒 Files selected for processing (11)
Plugins/Flow.Launcher.Plugin.Explorer/Main.csPlugins/Flow.Launcher.Plugin.Explorer/Search/Everything/Everything3ApiDllImport.csPlugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingAPI.csPlugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiDllImport.csPlugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.csPlugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingHelper.csPlugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingSearchManager.csPlugins/Flow.Launcher.Plugin.Explorer/Search/Everything/IEverythingApi.csPlugins/Flow.Launcher.Plugin.Explorer/Search/ResultManager.csPlugins/Flow.Launcher.Plugin.Explorer/Settings.csPlugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- Plugins/Flow.Launcher.Plugin.Explorer/Main.cs
Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs
Show resolved
Hide resolved
| try | ||
| { | ||
| if (!await EverythingApi.IsEverythingRunningAsync(token)) | ||
| if (!await _api.IsEverythingRunningAsync(token)) |
Contributor
There was a problem hiding this comment.
Make DLL reloads safe for in-flight queries.
Search/availability/run-counter calls keep using _api with no reader-side coordination, while ReloadApi() unloads and replaces the underlying native DLLs. That lets a settings change race an in-flight call into the old implementation. LoadDllCore() also unloads the active DLL before the replacement Load() is known-good, so one failed swap can leave _api pointing at an implementation whose native module is already detached. The handoff needs to keep the old API/DLL alive until the new pair is ready and prevent unload while calls are active.
Also applies to: 93-149, 151-197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingSearchManager.cs`
at line 32, The issue is racey access to the mutable field _api during
ReloadApi/LoadDllCore: calls like IsEverythingRunningAsync and other
search/availability/run-counter methods access _api without coordination while
ReloadApi unloads and replaces native DLLs, which can detach modules under
in-flight calls. Fix by introducing safe handoff and in-flight protection:
implement an API reference wrapper (e.g., AcquireApi()/ReleaseApi() or a small
ref-counted ApiHolder) used by all callers (IsEverythingRunningAsync and the
methods in the 93-149 and 151-197 regions) so a call increments a usage count
before invoking _api and decrements after; modify ReloadApi()/LoadDllCore() to
Load() the new native DLL and create the new API instance first, then atomically
swap the active ApiHolder (Interlocked.Exchange) and only after the swap
decrement/unload the old holder when its usage count hits zero; alternatively
use a ReaderWriterLockSlim where callers take a read lock and ReloadApi takes a
write lock, but ensure Load() occurs before releasing/unloading the old native
module. Ensure all places that reference _api are changed to use the
Acquire/Release wrapper (or read-lock) so no unload happens while calls are in
flight.
Contributor
Author
There was a problem hiding this comment.
@jjw24 @Jack251970 So is it acceptable that ticking/unticking the "support Everything 1.5" option requires a restart? The new Everything SDK for 1.5 alpha is not compatible with Everything 1.4. And I don't want to load both the new and the old SDK dlls, just to save memory. Loading/unloading dll in the flight causes potential issues.
Contributor
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Member
There was a problem hiding this comment.
@jjw24 @Jack251970 So is it acceptable that ticking/unticking the "support Everything 1.5" option requires a restart? The new Everything SDK for 1.5 alpha is not compatible with Everything 1.4. And I don't want to load both the new and the old SDK dlls, just to save memory. Loading/unloading dll in the flight causes potential issues.
Hi, I think it is reasonable. And I strongly recommend to work in that way to prevent some possible strange issues!
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingAPI.cs (1)
197-240:⚠️ Potential issue | 🔴 CriticalThe file has a critical syntax error: missing final return statement and unmatched brace.
The method
EverythingHighlightStringToHighlightList()is missing areturn highlightData;statement after the for loop. The entire file contains only 1 occurrence ofreturn highlightData;(the early return for empty strings), but 2 are required. Additionally, the file has 40 opening braces and only 39 closing braces, confirming it will not compile as written.The function must include
return highlightData;after the loop closes to return the processed highlight data for non-empty inputs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingAPI.cs` around lines 197 - 240, The method EverythingHighlightStringToHighlightList is missing the final return and a closing brace; after the for loop inside EverythingHighlightStringToHighlightList add "return highlightData;" and ensure you close the method/block so braces are balanced (matching the opening brace for the method/class), keeping the existing early return for empty strings intact.
♻️ Duplicate comments (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingAPI.cs (1)
17-21:⚠️ Potential issue | 🔴 CriticalKeep
IsFastSortOption()under_semaphore.This is still the only public native-call path here that bypasses the shared lock. That leaves the
Everything_IsFastSort()/CheckAndThrowExceptionOnError()pair exposed to interleaving native calls from the other methods, so the result and error check are no longer atomic.Verify that
IsFastSortOption()is the only publicEverythingApiDllImportpath without a matching semaphore acquisition. Expected:IsEverythingRunningAsync,SearchAsync, andIncrementRunCounterAsyncall acquire_semaphore;IsFastSortOption()does not.#!/bin/bash rg -n -C2 'public bool IsFastSortOption|public async ValueTask<bool> IsEverythingRunningAsync|public async IAsyncEnumerable<SearchResult> SearchAsync|public async Task IncrementRunCounterAsync|_semaphore\.(Wait|WaitAsync)|EverythingApiDllImport\.' Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingAPI.cs🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingAPI.cs` around lines 17 - 21, IsFastSortOption currently calls EverythingApiDllImport.Everything_IsFastSort(...) and CheckAndThrowExceptionOnError() without acquiring the shared _semaphore, allowing interleaved native calls; wrap the body of IsFastSortOption in a synchronous semaphore acquisition/release (use _semaphore.Wait() before calling Everything_IsFastSort and ensure _semaphore.Release() in a finally block) so the Everything_IsFastSort + CheckAndThrowExceptionOnError pair is executed atomically with the other methods that use _semaphore.
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs (1)
283-283: Remove dead code.
await Task.CompletedTask;at the end of the method serves no purpose and can be removed.Suggested fix
} } - - await Task.CompletedTask; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs` at line 283, Remove the redundant await Task.CompletedTask call at the end of the method in the EverythingApiV3 class; locate the method containing the trailing "await Task.CompletedTask;" (in EverythingApiV3) and delete that statement so the method ends normally without the no-op await. Ensure no other logic depends on that awaited task and run tests/compile to confirm no async signature changes are needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs`:
- Around line 145-166: IsFastSortOption lacks synchronization: wrap the native
DLL interaction in a semaphore acquisition/release using the existing _semaphore
to prevent concurrent native calls; specifically, acquire the semaphore before
calling TryConnectEverything3 and before invoking
Everything3ApiDllImport.Everything3_IsPropertyFastSort, and ensure the semaphore
is released in a finally block. Keep the existing IPCErrorException behavior if
TryConnectEverything3 fails, ensure
Everything3ApiDllImport.Everything3_DestroyClient(client) is still called in
cleanup, and remove the inner CheckAndThrowExceptionOnErrorFromEverything3()
call after DestroyClient (leave only the necessary error check where
appropriate) to avoid rethrowing based on destroy semantics. Ensure exception
safety by releasing _semaphore even when exceptions are thrown.
- Around line 90-94: The code mutates the caller's EverythingSearchOption by
assigning to option.Keyword; instead, read option.Keyword into a local variable
(e.g., keyword or localKeyword), set useRegex = true if
localKeyword.StartsWith("@"), and strip the leading "@" from that local variable
(localKeyword = localKeyword[1..]) before using it in the rest of
EverythingApiV3 logic; do not assign back to option.Keyword so the
EverythingSearchOption object remains unchanged.
---
Outside diff comments:
In `@Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingAPI.cs`:
- Around line 197-240: The method EverythingHighlightStringToHighlightList is
missing the final return and a closing brace; after the for loop inside
EverythingHighlightStringToHighlightList add "return highlightData;" and ensure
you close the method/block so braces are balanced (matching the opening brace
for the method/class), keeping the existing early return for empty strings
intact.
---
Duplicate comments:
In `@Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingAPI.cs`:
- Around line 17-21: IsFastSortOption currently calls
EverythingApiDllImport.Everything_IsFastSort(...) and
CheckAndThrowExceptionOnError() without acquiring the shared _semaphore,
allowing interleaved native calls; wrap the body of IsFastSortOption in a
synchronous semaphore acquisition/release (use _semaphore.Wait() before calling
Everything_IsFastSort and ensure _semaphore.Release() in a finally block) so the
Everything_IsFastSort + CheckAndThrowExceptionOnError pair is executed
atomically with the other methods that use _semaphore.
---
Nitpick comments:
In `@Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs`:
- Line 283: Remove the redundant await Task.CompletedTask call at the end of the
method in the EverythingApiV3 class; locate the method containing the trailing
"await Task.CompletedTask;" (in EverythingApiV3) and delete that statement so
the method ends normally without the no-op await. Ensure no other logic depends
on that awaited task and run tests/compile to confirm no async signature changes
are needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 43220eec-d73e-4a0f-a23d-f1e76991c8d8
📒 Files selected for processing (2)
Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingAPI.csPlugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs
Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs
Outdated
Show resolved
Hide resolved
Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
1 issue found across 18 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingSearchManager.cs">
<violation number="1" location="Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingSearchManager.cs:27">
P2: `EnableEverything15Support` is cached in a readonly field, so runtime setting changes won’t switch between legacy and v3 Everything APIs.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingSearchManager.cs
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 18 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingSearchManager.cs
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
1 issue found across 12 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs">
<violation number="1" location="Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs:111">
P2: Destroy the Everything3 client after incrementing run count; the current code leaks unmanaged client handles.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingApiV3.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Contributor
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
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
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.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.