Retrofit Arius.Explorer to Arius7.Core + ListQuery, ContainerNamesQuery, ArchiveCommand, RestoreCommand, and ChunkHydrationStatusQuery#41
Conversation
…ositoryEntry> - LsCommand now implements IStreamQuery<RepositoryEntry> (was ICommand<LsResult>) - LsHandler rewrites to IStreamQueryHandler, emitting RepositoryFileEntry and RepositoryDirectoryEntry as a discriminated union with cloud/local merge fields - LsVerb updated to consume mediator.CreateStream with await foreach; error handling via exception catch instead of LsResult.Success check - CliTests mock updated to IStreamQueryHandler<LsCommand, RepositoryEntry> - PipelineFixture.LsAsync and LsIntegrationTests adapted to List<RepositoryFileEntry> - ServiceCollectionExtensions registers LsHandler as IStreamQueryHandler
…-3.11) New tests covering: recursive vs non-recursive depth, case-insensitive filename filter (dirs not filtered), directory merge flags (cloud+local/cloud-only/local-only), per-directory batch size lookup with null for unknown hashes, snapshot-not-found exception (no snapshots and specific version), and cancellation stops enumeration at directory boundaries.
Add ContainerNamesQuery and ContainerNamesQueryHandler to stream repository container names by detecting the snapshots/ prefix, plus a focused unit test project covering the handler's filtering behavior.
…e APIs Update Arius.Explorer to restore/build with central package versions, add a per-repository session for rebuilding core services with selected repository credentials, switch container discovery to the new AzureBlob ContainerNamesQuery, and replace the old pointer-file listing flow with streaming LsCommand-based tree loading plus RestoreCommand-based downloads.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReorganizes core into Shared/Features namespaces, replaces batch Ls with streaming ListQuery and related streaming handlers, refactors blob abstractions (IBlobService/IBlobServiceFactory and IBlobContainerService) with Azure factory/service updates, adds Archive/Restore command features, and introduces a new WPF Explorer app and discovery CI script. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Mediator
participant ListQueryHandler
participant BlobContainer
participant ChunkIndex
Client->>Mediator: CreateStream(ListQuery)
activate Mediator
Mediator->>ListQueryHandler: Handle(query)
activate ListQueryHandler
ListQueryHandler->>BlobContainer: DownloadAsync(snapshot/tree blobs)
BlobContainer-->>ListQueryHandler: blob content
ListQueryHandler->>ChunkIndex: LookupAsync(content hashes)
ChunkIndex-->>ListQueryHandler: sizes/entries
ListQueryHandler-->>Mediator: yield RepositoryDirectoryEntry
ListQueryHandler-->>Mediator: yield RepositoryFileEntry
deactivate ListQueryHandler
Mediator-->>Client: async enumerable (progressive)
deactivate Mediator
Client->>Client: await foreach entry
Client->>Client: update UI incrementally
sequenceDiagram
participant ChooseRepoVM
participant Mediator
participant ContainerNamesHandler
participant BlobService
participant BlobContainer
ChooseRepoVM->>ChooseRepoVM: debounce user input
ChooseRepoVM->>Mediator: CreateStream(ContainerNamesQuery)
activate Mediator
Mediator->>ContainerNamesHandler: Handle(query)
activate ContainerNamesHandler
ContainerNamesHandler->>BlobService: CreateAsync(account, key)
BlobService-->>ContainerNamesHandler: IBlobService
loop per container
ContainerNamesHandler->>BlobContainer: ListBlobs(prefix="snapshots/", maxResults=1)
BlobContainer-->>ContainerNamesHandler: hasSnapshot?
alt has snapshot
ContainerNamesHandler-->>Mediator: yield containerName
end
end
deactivate ContainerNamesHandler
Mediator-->>ChooseRepoVM: async enumerable
deactivate Mediator
ChooseRepoVM->>ChooseRepoVM: populate ContainerNames, select first
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #41 +/- ##
==========================================
- Coverage 78.55% 70.94% -7.62%
==========================================
Files 35 59 +24
Lines 3810 4684 +874
Branches 446 624 +178
==========================================
+ Hits 2993 3323 +330
- Misses 719 1229 +510
- Partials 98 132 +34
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 10
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (16)
src/Arius.Explorer/CLAUDE.md-3-3 (1)
3-3:⚠️ Potential issue | 🟡 MinorFix root guidance filename casing reference.
Line 3 references
CLAUDE.MD; if the actual root file isCLAUDE.md, this can break discoverability on case-sensitive filesystems. Please align the filename casing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer/CLAUDE.md` at line 3, The documentation in Arius.Explorer/CLAUDE.md currently references the root guidance file as "CLAUDE.MD" which may not match the actual filename on case-sensitive filesystems; update the reference string "CLAUDE.MD" to use the correct casing "CLAUDE.md" wherever it appears in this file (search for the exact token "CLAUDE.MD") so links and discoverability work reliably.src/Arius.Explorer/RepositoryExplorer/RepositoryExplorerWindow.xaml.cs-5-7 (1)
5-7:⚠️ Potential issue | 🟡 MinorIncorrect XML documentation comment.
The XML doc references
ChooseRepositoryWindow.xamlbut this isRepositoryExplorerWindow.xaml.📝 Proposed fix
/// <summary> -/// Interaction logic for ChooseRepositoryWindow.xaml +/// Interaction logic for RepositoryExplorerWindow.xaml /// </summary>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer/RepositoryExplorer/RepositoryExplorerWindow.xaml.cs` around lines 5 - 7, The XML documentation at the top of RepositoryExplorerWindow.xaml.cs incorrectly references ChooseRepositoryWindow.xaml; update the XML summary to reference RepositoryExplorerWindow.xaml so the class/method documentation matches the file and class name (look for the /// <summary> block above the RepositoryExplorerWindow class and replace "ChooseRepositoryWindow.xaml" with "RepositoryExplorerWindow.xaml").src/Arius.Explorer/RepositoryExplorer/RepositoryExplorerWindow.xaml.cs-19-19 (1)
19-19:⚠️ Potential issue | 🟡 MinorIncorrect class name in log message.
The log message references "ChooseRepositoryWindow" but this is
RepositoryExplorerWindow.📝 Proposed fix
- logger.LogInformation("Repository Explorer ChooseRepositoryWindow initialized"); + logger.LogInformation("RepositoryExplorerWindow initialized");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer/RepositoryExplorer/RepositoryExplorerWindow.xaml.cs` at line 19, The log message at the initialization of RepositoryExplorerWindow is incorrect; update the call to logger.LogInformation so it references "RepositoryExplorerWindow" instead of "ChooseRepositoryWindow" (locate the statement in the RepositoryExplorerWindow class where logger.LogInformation("Repository Explorer ChooseRepositoryWindow initialized") is invoked and change the message text to reflect RepositoryExplorerWindow).src/Arius.Core.Tests/FileTree/TreeServiceTests.cs-376-380 (1)
376-380:⚠️ Potential issue | 🟡 MinorThis fake no longer covers the container-side dedup path.
GetMetadataAsyncalways returnsExists = false, so everyTreeBuildertest here assumes the blob is absent remotely. After theIBlobContainerServicerefactor, that leaves the metadata-driven skip-upload branch untested even though deduplication depends on it. Please add one case withExists = trueand assertUploadedstays empty.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Core.Tests/FileTree/TreeServiceTests.cs` around lines 376 - 380, The fake IBlobContainerService used in TreeBuilder tests always returns BlobMetadata.Exists = false in GetMetadataAsync, so container-side dedup (metadata-driven skip-upload) is never exercised; update the fake GetMetadataAsync implementation (or add a new test-specific override) to return Exists = true for a chosen blob name and add a test case invoking TreeBuilder that expects that blob to be detected as present remotely (i.e., no upload performed) and assert the Uploaded collection remains empty for that blob (referencing GetMetadataAsync, TreeBuilder tests and the Uploaded assertion to locate the relevant code).src/Arius.Explorer/Arius.Explorer.csproj-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorRemove duplicate
<UseWPF>true</UseWPF>element.
UseWPFis declared twice (lines 8 and 13). Remove the duplicate to avoid confusion.🧹 Proposed fix
<UseWPF>true</UseWPF> <ApplicationIcon>Resources\iceberg.ico</ApplicationIcon> <!--Windows Application icon--> <!--<PackageIcon>Resources\iceberg.ico</PackageIcon> --><!--Nuget package icon--> - <UseWPF>true</UseWPF> <!--<UseWindowsForms>True</UseWindowsForms>-->Also applies to: 13-13
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer/Arius.Explorer.csproj` at line 8, The project file contains a duplicated XML element UseWPF; remove the redundant <UseWPF>true</UseWPF> occurrence so only a single UseWPF element remains in the project file (keep one instance and delete the other), ensuring the .csproj has one clear declaration of UseWPF.src/Arius.Explorer/Shared/Services/DialogService.cs-23-26 (1)
23-26:⚠️ Potential issue | 🟡 MinorFail fast if
ChooseRepositoryWindow.DataContextis notChooseRepositoryViewModel.Current logic silently degrades to
nullon misconfiguration, which hides wiring errors and makes troubleshooting harder.Suggested fix
public RepositoryOptions? ShowChooseRepositoryDialog(RepositoryOptions? initialRepository = null) { var dialog = serviceProvider.GetRequiredService<ChooseRepositoryWindow>(); + var viewModel = dialog.DataContext as ChooseRepositoryViewModel + ?? throw new InvalidOperationException("ChooseRepositoryWindow.DataContext must be ChooseRepositoryViewModel."); // Set the initial repository if provided - if (dialog.DataContext is ChooseRepositoryViewModel viewModel && initialRepository != null) + if (initialRepository != null) { viewModel.Repository = initialRepository; } @@ - if (dialogResult == true && dialog.DataContext is ChooseRepositoryViewModel resultViewModel) + if (dialogResult == true) { - return resultViewModel.Repository; + return viewModel.Repository; }Also applies to: 38-40
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer/Shared/Services/DialogService.cs` around lines 23 - 26, The DataContext check for ChooseRepositoryWindow silently does nothing on misconfiguration; update the logic in the method that creates/shows ChooseRepositoryWindow (the block that currently checks "if (dialog.DataContext is ChooseRepositoryViewModel viewModel && initialRepository != null)") to instead fail fast when dialog.DataContext is not a ChooseRepositoryViewModel by throwing a clear InvalidOperationException (or ArgumentException) that includes the actual DataContext type (or null) and context (e.g., window name and expected view model). Apply the same fail-fast check/exception to the second similar block (the one around lines 38-40) so both places explicitly error on miswiring rather than quietly returning.src/Arius.Explorer/Shared/Extensions/DataProtectionExtensions.cs-35-52 (1)
35-52:⚠️ Potential issue | 🟡 MinorSilent failure in
Unprotect()may expose protected data incorrectly.Returning the original (Base64-encoded protected) string on failure could result in garbage being used as credentials. The
FormatExceptioncatch for backwards compatibility is reasonable, but catching all other exceptions silently could mask corruption or security issues.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer/Shared/Extensions/DataProtectionExtensions.cs` around lines 35 - 52, The current Unprotect logic returns the original input for any Exception, which can leak Base64-protected blobs as plaintext; update the method (Unprotect / the block calling ProtectedData.Unprotect) to only swallow FormatException for legacy plain-text, but do not return the original input on other failures—instead catch specific decryption errors (e.g., CryptographicException) and either log the error with context and rethrow or return a clearly invalid/empty value (not the original protected string); ensure ProtectedData.Unprotect exceptions are not silently swallowed and include logging of the exception details for diagnostics.src/Arius.Explorer.Tests/ChooseRepository/ChooseRepositoryViewModelTests.cs-84-103 (1)
84-103:⚠️ Potential issue | 🟡 MinorAdd a failure case where the returned stream faults during enumeration.
This only covers
CreateStream(...)throwing before any items are consumed. The cleanup path can still regress when the returned stream fails after enumeration has started, especially around partially loadedContainerNamesand resettingIsLoading.As per coding guidelines,
**/*.Tests/**: Test projects. Focus on test coverage gaps and assertion quality rather than style.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer.Tests/ChooseRepository/ChooseRepositoryViewModelTests.cs` around lines 84 - 103, Add a second test that simulates CreateStream(...) returning an IAsyncEnumerable that faults during enumeration (not just throwing synchronously). In the ChooseRepositoryViewModel test suite create a stream for the mediator (matching ContainerNamesQuery with AccountName "account" and AccountKey "key") that yields one or more container names then throws when the enumerator advances; use that stream in a test similar to AccountCredentials_WhenFactoryThrows_SetsErrorAndClearsContainers and assert that ChooseRepositoryViewModel.IsLoading is false, StorageAccountError is true, ContainerNames is empty (or reset), and ContainerName is string.Empty after the fault. Ensure the test targets the CreateStream path for ChooseRepositoryViewModel and observes behavior during enumeration rather than factory throw so regressions around partial ContainerNames population are caught.src/Arius.Explorer.Tests/ChooseRepository/ChooseRepositoryViewModelTests.cs-120-143 (1)
120-143:⚠️ Potential issue | 🟡 MinorReplace the skipped happy-path command test before merge.
The valid
OpenRepositoryCommandpath is stillSkip.Test("TODO"), so this change never verifies success-path enablement or repository materialization.I can sketch the final test body or a follow-up issue if helpful. As per coding guidelines,
**/*.Tests/**: Test projects. Focus on test coverage gaps and assertion quality rather than style.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer.Tests/ChooseRepository/ChooseRepositoryViewModelTests.cs` around lines 120 - 143, Remove the Skip.Test("TODO") and enable the happy-path assertions in ChooseRepositoryViewModelTests.OpenRepositoryCommand_WhenAllFieldsAreValid_IsEnabledAndBuildsRepository so the test actually exercises ChooseRepositoryViewModel.OpenRepositoryCommand; instantiate ChooseRepositoryViewModel, set LocalDirectoryPath, AccountName, AccountKey, ContainerName, and Passphrase as shown, assert OpenRepositoryCommand.CanExecute(null) is true, call OpenRepositoryCommand.Execute(null), then assert viewModel.Repository is not null and its LocalDirectoryPath, AccountName, ContainerName, AccountKey, and Passphrase match the values you set (use the existing assertions that reference viewModel.Repository and its properties).src/Arius.Explorer/Program.cs-53-56 (1)
53-56:⚠️ Potential issue | 🟡 MinorConsider re-throwing or exiting after fatal exception.
The catch block logs the fatal exception but doesn't re-throw or set an exit code. The application continues to the
finallyblock and exits normally, which may mask the failure from parent processes or scripts.🔧 Proposed fix
catch (Exception ex) { Log.Fatal(ex, "Arius Explorer terminated unexpectedly"); + Environment.ExitCode = 1; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer/Program.cs` around lines 53 - 56, The catch in Program.cs logs the fatal exception via Log.Fatal(ex, ...) but doesn't propagate failure; modify the catch for Exception ex in the Main/Program startup so the process exits with a non-zero status or the exception is re-thrown after logging (e.g., call Environment.Exit(nonZero) or throw;) to ensure parent processes see the failure; keep the Log.Fatal call and ensure any finally block still runs before exiting.openspec/changes/archive/2026-04-01-wire-explorer-to-core/design.md-38-55 (1)
38-55:⚠️ Potential issue | 🟡 MinorAdd language specifier to fenced code block.
The static analysis tool flagged line 38: fenced code blocks should have a language specified. Since this is pseudocode, consider using
textorpseudocodeas the language identifier for proper rendering.📝 Proposed fix
-``` +```text WalkMergedAsync(treeHash?, localDirPath?, prefix, recursive, ct):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-04-01-wire-explorer-to-core/design.md` around lines 38 - 55, The fenced code block containing the WalkMergedAsync pseudocode lacks a language tag; update the opening fence for the block that starts with "WalkMergedAsync(treeHash?, localDirPath?, prefix, recursive, ct):" to include a language identifier such as "text" or "pseudocode" (e.g., change ``` to ```text) so the static analysis warning is resolved and the pseudocode renders correctly.src/Arius.Explorer/App.xaml.cs-15-17 (1)
15-17:⚠️ Potential issue | 🟡 MinorAdd nullable annotations context.
The static analyzer flagged line 17: the nullable reference type annotation (
IServiceProvider?) should be used within a#nullableannotations context for proper compiler enforcement.🔧 Proposed fix
namespace Arius.Explorer; +#nullable enable + /// <summary> /// Interaction logic for App.xaml /// </summary>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer/App.xaml.cs` around lines 15 - 17, The ServiceProvider property uses a nullable annotation but the file lacks a nullable annotations context; enable nullable annotations for this file (or project) so the compiler enforces IServiceProvider? properly—add a nullable annotations directive (e.g., enable annotations) at the top of App.xaml.cs or enable nullable annotations in the project settings, then rebuild and verify the warning for the ServiceProvider static property (and any other nullable-annotated symbols in the App class) is resolved.src/Arius.Explorer/Infrastructure/RepositorySession.cs-1-21 (1)
1-21:⚠️ Potential issue | 🟡 MinorAdd
#nullable enableto resolve nullable annotation warnings.Static analysis reports warnings on lines 17, 18, and 27 about nullable annotations used outside a
#nullablecontext. Adding the directive will enable proper nullable reference type checking.Proposed fix
using Arius.Core; using Arius.Core.Shared.Storage; using Arius.Explorer.Settings; using Mediator; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using System; using System.Collections.Generic; using System.IO; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; +#nullable enable + namespace Arius.Explorer.Infrastructure;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer/Infrastructure/RepositorySession.cs` around lines 1 - 21, Static analysis warns about nullable annotations used outside a nullable context for the IRepositorySession interface (properties Mediator and Repository and method ConnectAsync signature); fix it by enabling nullable reference types for this file: add the directive "#nullable enable" at the top of the file so the nullable annotations on IMediator? and RepositoryOptions? are recognized and the warnings are resolved.src/Arius.Explorer/Settings/ApplicationSettings.cs-41-47 (1)
41-47:⚠️ Potential issue | 🟡 MinorClamp
RecentLimitbefore trimming.Because this is a user-scoped setting, a negative value keeps the loop true after
orderedis empty and the nextRemoveAt(ordered.Count - 1)throws.🛡️ Minimal hardening
- while (ordered.Count > settings.RecentLimit) + var recentLimit = Math.Max(0, settings.RecentLimit); + while (ordered.Count > recentLimit) ordered.RemoveAt(ordered.Count - 1);Also applies to: 123-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer/Settings/ApplicationSettings.cs` around lines 41 - 47, The RecentLimit property can be negative (user-scoped), which causes the trimming loop that uses ordered and RemoveAt(ordered.Count - 1) to run forever; clamp RecentLimit to a non-negative value before it's used (either by validating/clamping in the RecentLimit setter in ApplicationSettings.cs—e.g. set to Math.Max(0, value)—or by clamping the local variable just before the trimming loop that references ordered and calls RemoveAt). Ensure any code that reads RecentLimit uses the clamped value to determine loop bounds.openspec/changes/archive/2026-04-01-wire-explorer-to-core/tasks.md-1-15 (1)
1-15:⚠️ Potential issue | 🟡 MinorUpdate this archived task list to the final
ListQuerynaming.The spec still records the work as
LsCommand/LsHandlerinLsModels.cs, but the implementation shipped asListQuery/ListQueryHandlerunderFeatures/ListQuery. Leaving the archived change in the pre-rename state makes the OpenSpec history misleading.Also applies to: 30-34, 62-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-04-01-wire-explorer-to-core/tasks.md` around lines 1 - 15, Update the archived task list to use the final "ListQuery" naming: replace occurrences of LsCommand with ListQuery, LsHandler with ListQueryHandler, and any LsModels.cs/LsResult/LsEntry references with their ListQuery-era equivalents so the checklist and descriptions match the shipped implementation (also update the similar mentions in the other checklist sections referenced by the reviewer: the ranges corresponding to items 30-34 and 62-64). Ensure the narrative and bullet points refer to the Features/ListQuery implementation symbols (ListQuery, ListQueryHandler, and the ListQuery models) rather than the old Ls* names.src/Arius.Explorer/ChooseRepository/ChooseRepositoryViewModel.cs-69-72 (1)
69-72:⚠️ Potential issue | 🟡 MinorEnable nullable annotations or remove
?annotations here.The project lacks
<Nullable>enable</Nullable>inArius.Explorer.csproj, which means theseRepositoryOptions?annotations are not enforced by the compiler. Either add<Nullable>enable</Nullable>to the project file or remove the?operators from the signatures to maintain consistency with the current compilation settings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer/ChooseRepository/ChooseRepositoryViewModel.cs` around lines 69 - 72, The RepositoryOptions? nullable annotation on the private field 'repository' and the partial method 'OnRepositoryChanged(RepositoryOptions? value)' is inconsistent with project nullability settings; either enable nullable in the project or remove the nullable markers. Fix by adding <Nullable>enable</Nullable> to the Arius.Explorer.csproj so the compiler enforces RepositoryOptions? (and update callers if nullable issues surface), or remove the '?' from the 'repository' field and the 'OnRepositoryChanged' signature in ChooseRepositoryViewModel.cs and adjust any null checks/initialization to match non-nullable semantics.
🧹 Nitpick comments (20)
src/Arius.Explorer/ChooseRepository/ChooseRepositoryWindow.xaml (1)
145-149: Consider makingOpenthe default dialog action.
SettingIsDefault="True"improves keyboard flow (Enter triggers Open).Suggested tweak
- <Button HorizontalAlignment="Right" + <Button HorizontalAlignment="Right" Width="100" Height="30" Margin="0,0,11,10" + IsDefault="True" Command="{Binding OpenRepositoryCommand}">Open</Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer/ChooseRepository/ChooseRepositoryWindow.xaml` around lines 145 - 149, The Open button should be the default dialog action so Enter triggers it; update the Button element that binds to OpenRepositoryCommand by adding IsDefault="True" (the Button with Command="{Binding OpenRepositoryCommand}" in ChooseRepositoryWindow.xaml) so pressing Enter activates the Open command—ensure no other control also sets IsDefault to True.src/Arius.Cli/Commands/Ls/LsVerb.cs (1)
115-149:lsis still buffering the entire stream before printing.Building a Spectre
Tablehere means the command still waits for full enumeration and grows O(n) with snapshot size. If this path is meant to benefit fromListQuerystreaming, render incrementally or page the output instead of materializing every row first.openspec/specs/container-names/spec.md (1)
7-10: Consider adding error-handling scenarios.The spec covers happy paths but doesn't specify behavior for error conditions (invalid credentials, network failures, partial failures mid-stream). Consider adding scenarios for:
- Invalid/expired credentials
- Network interruption during streaming
- Permission denied on specific containers
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/specs/container-names/spec.md` around lines 7 - 10, Add explicit error-handling scenarios to the container-names requirement: define expected behavior and error codes when IStreamQuery<string>/IStreamQueryHandler encounters invalid or expired credentials, network interruptions during streaming, and permission-denied errors on specific containers; specify how ListBlobs calls (with prefix "snapshots/" and maxResults=1) should surface transient vs. fatal errors (retryable vs. terminal), how partial results should be handled/sent over the Mediator streaming channel, and the format of error messages/events consumers will receive so callers of IStreamQuery<string> can distinguish auth, network, and per-container permission failures.src/Arius.Core/Shared/ChunkIndex/ChunkIndexService.cs (1)
218-222: Consider logging when L2 cache read fails.The catch-all silently deletes the L2 file and falls through to L3. While this is acceptable for a cache layer, logging the exception would help diagnose recurring issues (e.g., disk permissions, file format changes).
♻️ Suggested improvement
catch { // Stale or corrupt L2 file (e.g. old encrypted format) — treat as cache miss and fall through to L3. + // Consider: _logger?.LogWarning(ex, "L2 cache miss for prefix {Prefix}, deleting stale file", prefix); File.Delete(l2Path); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Core/Shared/ChunkIndex/ChunkIndexService.cs` around lines 218 - 222, The empty catch in ChunkIndexService that deletes the L2 file should record the failure before deleting; update the catch to catch Exception (e.g., catch (Exception ex)) and call the service's logger (e.g., _logger or existing logger instance on ChunkIndexService) to log a warning or error that includes the l2Path and the exception details, then proceed to File.Delete(l2Path) to keep the current fallback behavior to L3.src/Arius.Explorer/Shared/Converters/BytesToReadableSizeConverter.cs (1)
10-17: Consider handling additional numeric types for robustness.The converter only handles
long, but WPF bindings may pass other numeric types likeint,uint, orulong. Consider broadening the type check:♻️ Suggested improvement
public object Convert(object value, Type targetType, object parameter, CultureInfo culture) { - if (value is long bytes) + if (value is long bytes || + (value is IConvertible c && long.TryParse(c.ToString(culture), out bytes))) { return bytes.Bytes().Humanize("#.#"); } - return null; + return DependencyProperty.UnsetValue; }Returning
DependencyProperty.UnsetValueinstead ofnullis the WPF convention for indicating conversion failure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer/Shared/Converters/BytesToReadableSizeConverter.cs` around lines 10 - 17, The Convert method in BytesToReadableSizeConverter currently only handles long and returns null; update Convert (BytesToReadableSizeConverter.Convert) to accept other numeric types (int, uint, ulong, short, double, decimal, etc.) by testing for IConvertible or using Convert.ToInt64/ToUInt64 safely after checking sign/overflow, normalize the value to an appropriate unsigned/signed long for bytes and call bytes.Bytes().Humanize("#.#"), and on non-numeric or out-of-range inputs return DependencyProperty.UnsetValue (WPF convention) instead of null to indicate conversion failure.src/Arius.Core.Tests/ContainerNames/ContainerNamesQueryHandlerTests.cs (1)
10-37: Add an explicit empty-stream scenario test.The spec includes “empty storage account returns empty sequence”; locking that behavior here would improve regression protection.
As per coding guidelines "
**/*.Tests/**: Test projects. Focus on test coverage gaps and assertion quality rather than style."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Core.Tests/ContainerNames/ContainerNamesQueryHandlerTests.cs` around lines 10 - 37, Add a new unit test for ContainerNamesQueryHandler that verifies the empty-stream scenario: arrange an IBlobServiceFactory and IBlobService stub so CreateAsync("account","key", ...) returns the blob service and GetContainerNamesAsync(...) returns an empty IAsyncEnumerable (no elements), invoke handler.Handle(new ContainerNamesQuery("account","key"), CancellationToken.None) and collect results, then assert the collected results is empty and that IBlobServiceFactory.CreateAsync was called once; reference the ContainerNamesQueryHandler class, ContainerNamesQuery, IBlobServiceFactory.CreateAsync and IBlobService.GetContainerNamesAsync to locate where to add this test.src/Arius.Core/Features/ContainerNamesQuery/ContainerNamesQuery.cs (1)
10-18: Prefer constructor-injectingIBlobServiceFactoryinstead ofIServiceProvider.This removes service-locator coupling and makes handler dependencies explicit.
Suggested refactor
-public sealed class ContainerNamesQueryHandler(IServiceProvider serviceProvider) +public sealed class ContainerNamesQueryHandler(IBlobServiceFactory blobServiceFactory) : IStreamQueryHandler<ContainerNamesQuery, string> { @@ - var blobServiceFactory = serviceProvider.GetRequiredService<IBlobServiceFactory>(); var blobService = await blobServiceFactory.CreateAsync(query.AccountName, query.AccountKey, cancellationToken) .ConfigureAwait(false);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Core/Features/ContainerNamesQuery/ContainerNamesQuery.cs` around lines 10 - 18, The handler currently accepts IServiceProvider (serviceProvider) and uses it as a service locator; change ContainerNamesQueryHandler to accept IBlobServiceFactory via constructor injection, store it in a private readonly field (e.g., _blobServiceFactory), and update the Handle method to call _blobServiceFactory.CreateAsync(...) instead of resolving from serviceProvider; ensure the constructor signature of ContainerNamesQueryHandler is updated and any DI registrations supply IBlobServiceFactory so the handler is constructed with the explicit dependency.src/Arius.Explorer/RepositoryExplorer/FileItemViewModel.cs (1)
43-45: Consider color visibility:Brushes.Whitemay be invisible on light backgrounds.When
BinaryExistsis not true,BinaryFileStateColoris set toBrushes.White, which may be invisible against a white or light-colored background. Consider using a more visible alternative likeBrushes.LightGrayor adding an outline/border.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer/RepositoryExplorer/FileItemViewModel.cs` around lines 43 - 45, BinaryFileStateColor currently uses Brushes.White when file.BinaryExists is false, which can be invisible on light backgrounds; update the assignment in FileItemViewModel (the BinaryFileStateColor initialization that checks file.BinaryExists) to use a more visible default such as Brushes.LightGray (or a theme-aware/ configurable brush) or implement an outline/border approach so the absent-binary state is visible on light backgrounds; ensure any change is consistent with PointerFileStateColor and PointerFileEntryStateColor usage to maintain UI consistency.src/Arius.Explorer.Tests/Settings/ApplicationSettingsTests.cs (1)
40-42: UseThread.Sleepinstead ofTask.Delay(...).GetAwaiter().GetResult()for synchronous delays.The synchronous blocking on an async delay is unnecessary overhead. Since this is a sync test method, use
Thread.Sleep(5)directly.♻️ Proposed fix
manager.TouchOrAdd(repoA); - Task.Delay(5).GetAwaiter().GetResult(); + Thread.Sleep(5); manager.TouchOrAdd(repoB);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer.Tests/Settings/ApplicationSettingsTests.cs` around lines 40 - 42, Replace the synchronous async-wait call with a direct thread sleep: locate the code using Task.Delay(5).GetAwaiter().GetResult() between manager.TouchOrAdd(repoA) and manager.TouchOrAdd(repoB) and change it to Thread.Sleep(5). Ensure you have using System.Threading if needed and that only the single synchronous delay replacement is made so manager.TouchOrAdd remains unchanged.src/Arius.Cli.Tests/DependencyInjectionTests.cs (1)
25-31: Consider registeringIBlobServiceFactorybeforeAddAriusfor clarity.The
IBlobServiceFactoryis added afterAddArius()(line 31), which works because resolution happens later atBuildServiceProvider(). However, ifAddAriusinternally registers its ownIBlobServiceFactory, this order matters—the last registration wins in Microsoft DI. Moving the factory registration beforeAddArius()or documenting the intent would make the test setup clearer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Cli.Tests/DependencyInjectionTests.cs` around lines 25 - 31, The test registers IBlobServiceFactory after calling services.AddArius which can be ambiguous if AddArius also registers that service; move the services.AddSingleton(Substitute.For<IBlobServiceFactory>()) call to before services.AddArius(...) so your substitute is the effective registration, or alternatively adjust AddArius invocation to accept an explicit factory parameter if supported; update the test to register the IBlobServiceFactory substitute prior to calling AddArius and keep the existing IBlobContainerService substitute as-is.openspec/changes/archive/2026-04-01-wire-explorer-to-core/specs/ls-command/spec.md (1)
34-35: Minor naming inconsistency: spec referencesLsCommandbut implementation usesListQuery.The spec mentions
LsCommand(e.g., Line 34, 69, 80, 84) while the actual implementation usesListQuery. Consider updating the spec to match the implementation for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-04-01-wire-explorer-to-core/specs/ls-command/spec.md` around lines 34 - 35, The spec uses the name LsCommand but the codebase implements the feature as ListQuery; update the spec text to consistently reference ListQuery (replace occurrences of LsCommand such as the instances around the WHEN/THEN examples and other mentions on lines referenced) or, if you prefer the spec name, rename the implementation to LsCommand—whichever choice you pick, ensure all mentions (e.g., the examples at “WHEN LsCommand is executed…” and other occurrences) are changed to the chosen symbol so spec and implementation names match.src/Arius.AzureBlob/BlobServiceFactory.cs (1)
32-36: ConsiderDefaultAzureCredentialfor broader authentication support.
AzureCliCredentialonly works when the user is logged in via Azure CLI, which limits usage to development scenarios. For production environments (App Service, AKS, VMs with managed identity), useDefaultAzureCredentialinstead. It tries multiple authentication methods in order—includingManagedIdentityCredentialfor production deployments andAzureCliCredentialas a fallback—providing seamless support across development and production environments.♻️ Suggested change
- blobServiceClient = new BlobServiceClient(serviceUri, new AzureCliCredential()); + blobServiceClient = new BlobServiceClient(serviceUri, new DefaultAzureCredential());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.AzureBlob/BlobServiceFactory.cs` around lines 32 - 36, Replace the hard-coded AzureCliCredential in BlobServiceFactory (where blobServiceClient is created) with DefaultAzureCredential to enable a chain of auth methods (managed identity, environment, CLI, etc.) for both prod and dev; instantiate BlobServiceClient with new DefaultAzureCredential(), keep the authMode assignment as appropriate, and add a using/import for Azure.Identity if not already present.src/Arius.Explorer/Program.cs (1)
21-32: Log file naming uses local time.Using
DateTime.Nowfor the log filename creates files with local timezone timestamps, which may cause confusion in multi-timezone environments or when correlating logs. Consider usingDateTime.UtcNowwith a UTC suffix for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer/Program.cs` around lines 21 - 32, The log filename is generated using local time (DateTime.Now) which can confuse correlating logs across timezones; update the log filename construction in Program.cs (where logPath and Log.Logger are configured) to use DateTime.UtcNow and append a clear UTC suffix (e.g., "_UTC") to the filename so timestamps are in UTC and the filename indicates the timezone.src/Arius.Explorer/App.xaml.cs (1)
94-101: MessageBox may not display in AppDomain.UnhandledException handler.When
AppDomain.CurrentDomain.UnhandledExceptionfires withe.IsTerminating == true, the application is about to exit and UI operations likeMessageBox.Showmay fail or not display. Consider logging to a crash file or using a crash reporting mechanism instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer/App.xaml.cs` around lines 94 - 101, CurrentDomain_UnhandledException currently calls ShowExceptionMessageBox even when e.IsTerminating may be true; update the handler in CurrentDomain_UnhandledException to check e.IsTerminating and avoid UI operations when terminating by fallback to a non-UI crash path: log the full exception (you already call logger?.LogError), write a short crash dump or human-readable file via a helper (e.g., create or reuse a "WriteCrashFile" routine) including exception.ToString() and timestamp, and invoke any crash-reporting/telemetry instead of ShowExceptionMessageBox when e.IsTerminating is true; only call ShowExceptionMessageBox when e.IsTerminating is false so UI is safe.src/Arius.AzureBlob/AzureBlobService.cs (1)
54-72: Consider adding a timeout for the read-only preflight listing.The read-only preflight at lines 64-71 correctly validates list access, but there's no explicit timeout. If the storage account is slow or unresponsive, this could hang indefinitely. Consider whether a
CancellationTokenSourcewith a timeout should wrap the operation for better resilience.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.AzureBlob/AzureBlobService.cs` around lines 54 - 72, The read-only preflight listing (when preflightMode != PreflightMode.ReadWrite) lacks a timeout and can hang; wrap the listing call to containerClient.GetBlobsAsync(...).AsPages(...) and the subsequent await foreach over pages in a CancellationTokenSource with a short timeout (or create a linked token via CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, timeoutCts.Token)) and pass that token into GetBlobsAsync/AsPages and the Delete/Upload calls as appropriate; ensure the timeout CTS is disposed and that you break out on OperationCanceledException so the preflight fails fast rather than hanging.src/Arius.Core/ServiceCollectionExtensions.cs (1)
94-96: Explicit dependency injection for ContainerNamesQueryHandler.The handler receives the entire
IServiceProviderbut only needsIBlobServiceFactory. Pass the factory explicitly instead, matching the pattern used byListQueryHandlerandChunkHydrationStatusQueryHandler:services.AddSingleton<IStreamQueryHandler<ContainerNamesQuery, string>>(sp => new ContainerNamesQueryHandler( sp.GetRequiredService<IBlobServiceFactory>()));Then update the handler constructor to
ContainerNamesQueryHandler(IBlobServiceFactory blobServiceFactory). This makes dependencies explicit and testable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Core/ServiceCollectionExtensions.cs` around lines 94 - 96, Replace the anonymous factory that injects the whole IServiceProvider into the ContainerNamesQueryHandler with an explicit dependency on IBlobServiceFactory: update the registration of IStreamQueryHandler<ContainerNamesQuery, string> to resolve sp.GetRequiredService<IBlobServiceFactory>() and pass that into the handler, and change the ContainerNamesQueryHandler constructor signature to ContainerNamesQueryHandler(IBlobServiceFactory blobServiceFactory) so the handler only depends on IBlobServiceFactory (matching the pattern used by ListQueryHandler and ChunkHydrationStatusQueryHandler) and is easier to unit test.src/Arius.Core.Tests/List/ResolveFileHydrationStatusesHandlerTests.cs (1)
174-198: Consider extracting the sharedFakeBlobContainerServiceto reduce duplication.This
FakeBlobContainerServiceimplementation is nearly identical to the one inFileHydrationStatusResolverTests.cs. Consider extracting it to a shared test utilities class.Example shared location
// src/Arius.Core.Tests/Fakes/FakeBlobContainerService.cs namespace Arius.Core.Tests.Fakes; public sealed class FakeBlobContainerService : IBlobContainerService { public Dictionary<string, BlobMetadata> Metadata { get; } = new(StringComparer.Ordinal); public List<string> RequestedBlobNames { get; } = []; // ... rest of implementation }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Core.Tests/List/ResolveFileHydrationStatusesHandlerTests.cs` around lines 174 - 198, The FakeBlobContainerService class is duplicated between ResolveFileHydrationStatusesHandlerTests and FileHydrationStatusResolverTests; extract this test double into a shared test utilities namespace (e.g., Arius.Core.Tests.Fakes) and update both tests to reference the single FakeBlobContainerService type instead of defining it locally; ensure the extracted implementation preserves Metadata, RequestedBlobNames, GetMetadataAsync, ListAsync and other method stubs so existing tests (ResolveFileHydrationStatusesHandlerTests and FileHydrationStatusResolverTests) compile against the shared class.src/Arius.Explorer/Infrastructure/RepositorySession.cs (2)
51-55: RedundantNullBlobContainerServiceinstantiation.Two separate instances are created: one for the singleton registration and another passed directly to
AddArius. Reuse a single instance for consistency and to avoid confusion.Proposed fix
public static void AddRootCorePlaceholders(IServiceCollection services) { - services.AddSingleton<IBlobContainerService, NullBlobContainerService>(); - services.AddArius(new NullBlobContainerService(), passphrase: null, accountName: "root", containerName: "root"); + var nullService = new NullBlobContainerService(); + services.AddSingleton<IBlobContainerService>(nullService); + services.AddArius(nullService, passphrase: null, accountName: "root", containerName: "root"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer/Infrastructure/RepositorySession.cs` around lines 51 - 55, The method AddRootCorePlaceholders creates two separate NullBlobContainerService instances; change it to instantiate a single NullBlobContainerService, register that instance with services.AddSingleton<IBlobContainerService, NullBlobContainerService>() (or use services.AddSingleton<IBlobContainerService>(instance)) and pass the same instance into AddArius so both the DI registration and the AddArius call use the identical NullBlobContainerService instance; update AddRootCorePlaceholders to create one variable (e.g., var nullBlob = new NullBlobContainerService()), use services.AddSingleton with that variable and call services.AddArius(nullBlob, passphrase: null, accountName: "root", containerName: "root").
74-74: Add[EnumeratorCancellation]attribute toListAsyncfor proper cancellation support.The
cancellationTokenparameter should have the[EnumeratorCancellation]attribute to ensure cancellation works correctly when the async enumerable is consumed withWithCancellation().Proposed fix
- public async IAsyncEnumerable<string> ListAsync(string prefix, CancellationToken cancellationToken = default) { yield break; } + public async IAsyncEnumerable<string> ListAsync(string prefix, [EnumeratorCancellation] CancellationToken cancellationToken = default) { yield break; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Explorer/Infrastructure/RepositorySession.cs` at line 74, Add the [EnumeratorCancellation] attribute to the cancellationToken parameter of the async iterator method ListAsync so the consumer's WithCancellation(token) actually cancels enumeration; update the signature of ListAsync(string prefix, CancellationToken cancellationToken = default) to be ListAsync(string prefix, [EnumeratorCancellation] CancellationToken cancellationToken = default) and ensure you import System.Runtime.CompilerServices so the attribute resolves (keep the rest of the method behavior unchanged).src/Arius.Integration.Tests/Pipeline/PipelineFixture.cs (1)
209-222: Minor: Unnecessaryawait Task.CompletedTaskinDisposeAsync.The
await Task.CompletedTaskat line 221 is not needed since the method performs only synchronous cleanup. You could either remove it or mark the method as non-async returningValueTask.CompletedTask.Simplified implementation
public async ValueTask DisposeAsync() { // Clean up unique temp dir if (Directory.Exists(_tempRoot)) Directory.Delete(_tempRoot, recursive: true); // Clean up any cache dirs created by this test's container (unique name) var home = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); var cacheDir = Path.Combine(home, ".arius", ChunkIndexService.GetRepoDirectoryName(Account, Container.Name)); if (Directory.Exists(cacheDir)) Directory.Delete(cacheDir, recursive: true); - - await Task.CompletedTask; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Integration.Tests/Pipeline/PipelineFixture.cs` around lines 209 - 222, The DisposeAsync method contains an unnecessary await of Task.CompletedTask; change DisposeAsync to a synchronous ValueTask by removing the async modifier and the await, returning ValueTask.CompletedTask instead (or simply return new ValueTask()) after the synchronous cleanup. Update the DisposeAsync signature and implementation around the method that references _tempRoot, the cleanup of cacheDir using ChunkIndexService.GetRepoDirectoryName(Account, Container.Name), and ensure no leftover async/await remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 643a52e2-72c3-470c-8a4d-383a82b8a662
⛔ Files ignored due to path filters (1)
src/Arius.Explorer/Resources/iceberg.icois excluded by!**/*.ico
📒 Files selected for processing (140)
.github/scripts/Get-DotNetProjectMatrix.ps1.github/workflows/ci.yml.github/workflows/codeql.yml.github/workflows/release.ymlcodecov.ymlopenspec/changes/archive/2026-04-01-wire-explorer-to-core/.openspec.yamlopenspec/changes/archive/2026-04-01-wire-explorer-to-core/design.mdopenspec/changes/archive/2026-04-01-wire-explorer-to-core/proposal.mdopenspec/changes/archive/2026-04-01-wire-explorer-to-core/specs/container-names/spec.mdopenspec/changes/archive/2026-04-01-wire-explorer-to-core/specs/ls-command/spec.mdopenspec/changes/archive/2026-04-01-wire-explorer-to-core/tasks.mdopenspec/specs/container-names/spec.mdopenspec/specs/ls-command/spec.mdsrc/Arius.Architecture.Tests/DependencyTests.cssrc/Arius.AzureBlob.Tests/Arius.AzureBlob.Tests.csprojsrc/Arius.AzureBlob.Tests/AzureBlobServiceTests.cssrc/Arius.AzureBlob/Arius.AzureBlob.csprojsrc/Arius.AzureBlob/AzureBlobContainerService.cssrc/Arius.AzureBlob/AzureBlobService.cssrc/Arius.AzureBlob/BlobServiceFactory.cssrc/Arius.AzureBlob/Preflight.cssrc/Arius.Cli.Tests/CliTests.cssrc/Arius.Cli.Tests/DependencyInjectionTests.cssrc/Arius.Cli.Tests/ProgressTests.cssrc/Arius.Cli/Arius.Cli.csprojsrc/Arius.Cli/CliBuilder.cssrc/Arius.Cli/Commands/Archive/ArchiveProgressHandlers.cssrc/Arius.Cli/Commands/Archive/ArchiveVerb.cssrc/Arius.Cli/Commands/Ls/LsVerb.cssrc/Arius.Cli/Commands/Restore/RestoreProgressHandlers.cssrc/Arius.Cli/Commands/Restore/RestoreVerb.cssrc/Arius.Cli/ProgressState.cssrc/Arius.Core.Tests/ChunkIndex/ShardTests.cssrc/Arius.Core.Tests/ContainerNames/ContainerNamesQueryHandlerTests.cssrc/Arius.Core.Tests/Encryption/AesGcmEncryptionTests.cssrc/Arius.Core.Tests/Encryption/GoldenFileDecryptionTests.cssrc/Arius.Core.Tests/Encryption/PassphraseEncryptionServiceTests.cssrc/Arius.Core.Tests/Encryption/PlaintextPassthroughServiceTests.cssrc/Arius.Core.Tests/FileTree/TreeServiceTests.cssrc/Arius.Core.Tests/List/FileHydrationStatusResolverTests.cssrc/Arius.Core.Tests/List/ResolveFileHydrationStatusesHandlerTests.cssrc/Arius.Core.Tests/LocalFile/LocalFileEnumeratorTests.cssrc/Arius.Core.Tests/Ls/LsHandlerTests.cssrc/Arius.Core.Tests/Restore/PricingConfigTests.cssrc/Arius.Core.Tests/Restore/RestoreCostCalculatorTests.cssrc/Arius.Core.Tests/Snapshot/SnapshotSerializerTests.cssrc/Arius.Core.Tests/Streaming/StreamWrapperTests.cssrc/Arius.Core/Archive/ArchiveModels.cssrc/Arius.Core/Arius.Core.csprojsrc/Arius.Core/Features/ArchiveCommand/ArchiveCommand.cssrc/Arius.Core/Features/ArchiveCommand/ArchiveCommandHandler.cssrc/Arius.Core/Features/ArchiveCommand/Events.cssrc/Arius.Core/Features/ArchiveCommand/Models.cssrc/Arius.Core/Features/ChunkHydrationStatusQuery/ChunkHydrationStatusQuery.cssrc/Arius.Core/Features/ChunkHydrationStatusQuery/ChunkHydrationStatusResolver.cssrc/Arius.Core/Features/ContainerNamesQuery/ContainerNamesQuery.cssrc/Arius.Core/Features/ListQuery/ListQuery.cssrc/Arius.Core/Features/ListQuery/ListQueryHandler.cssrc/Arius.Core/Features/RestoreCommand/Events.cssrc/Arius.Core/Features/RestoreCommand/Models.cssrc/Arius.Core/Features/RestoreCommand/PricingConfig.cssrc/Arius.Core/Features/RestoreCommand/RestoreCommand.cssrc/Arius.Core/Features/RestoreCommand/RestoreCommandHandler.cssrc/Arius.Core/Features/RestoreCommand/RestoreCostCalculator.cssrc/Arius.Core/Features/RestoreCommand/pricing.jsonsrc/Arius.Core/Ls/LsHandler.cssrc/Arius.Core/Ls/LsModels.cssrc/Arius.Core/Restore/RestoreModels.cssrc/Arius.Core/ServiceCollectionExtensions.cssrc/Arius.Core/Shared/ChunkIndex/ChunkIndexService.cssrc/Arius.Core/Shared/ChunkIndex/Shard.cssrc/Arius.Core/Shared/ChunkIndex/ShardSerializer.cssrc/Arius.Core/Shared/Encryption/IEncryptionService.cssrc/Arius.Core/Shared/Encryption/PassphraseEncryptionService.cssrc/Arius.Core/Shared/Encryption/PlaintextPassthroughService.cssrc/Arius.Core/Shared/FileTree/TreeBlobSerializer.cssrc/Arius.Core/Shared/FileTree/TreeModels.cssrc/Arius.Core/Shared/FileTree/TreeService.cssrc/Arius.Core/Shared/LocalFile/LocalFileEnumerator.cssrc/Arius.Core/Shared/Snapshot/SnapshotService.cssrc/Arius.Core/Shared/Storage/BlobConstants.cssrc/Arius.Core/Shared/Storage/IBlobContainerService.cssrc/Arius.Core/Shared/Storage/IBlobService.cssrc/Arius.Core/Shared/Storage/IBlobServiceFactory.cssrc/Arius.Core/Shared/Streaming/CountingStream.cssrc/Arius.Core/Shared/Streaming/ProgressStream.cssrc/Arius.E2E.Tests/Arius.E2E.Tests.csprojsrc/Arius.E2E.Tests/AzureFixture.cssrc/Arius.E2E.Tests/E2ETests.cssrc/Arius.E2E.Tests/RehydrationE2ETests.cssrc/Arius.Explorer.Tests/Arius.Explorer.Tests.csprojsrc/Arius.Explorer.Tests/ChooseRepository/ChooseRepositoryViewModelTests.cssrc/Arius.Explorer.Tests/Settings/ApplicationSettingsTests.cssrc/Arius.Explorer/App.xamlsrc/Arius.Explorer/App.xaml.cssrc/Arius.Explorer/Arius.Explorer.csprojsrc/Arius.Explorer/Arius.Explorer.snksrc/Arius.Explorer/AssemblyInfo.cssrc/Arius.Explorer/CLAUDE.mdsrc/Arius.Explorer/ChooseRepository/ChooseRepositoryViewModel.cssrc/Arius.Explorer/ChooseRepository/ChooseRepositoryWindow.xamlsrc/Arius.Explorer/ChooseRepository/ChooseRepositoryWindow.xaml.cssrc/Arius.Explorer/ChooseRepository/PasswordBoxBehavior.cssrc/Arius.Explorer/Infrastructure/RepositorySession.cssrc/Arius.Explorer/Program.cssrc/Arius.Explorer/Properties/PublishProfiles/ClickOnceProfile.pubxmlsrc/Arius.Explorer/RepositoryExplorer/FileItemViewModel.cssrc/Arius.Explorer/RepositoryExplorer/RepositoryExplorerViewModel.cssrc/Arius.Explorer/RepositoryExplorer/RepositoryExplorerWindow.xamlsrc/Arius.Explorer/RepositoryExplorer/RepositoryExplorerWindow.xaml.cssrc/Arius.Explorer/RepositoryExplorer/StateCircle.xamlsrc/Arius.Explorer/RepositoryExplorer/StateCircle.xaml.cssrc/Arius.Explorer/RepositoryExplorer/TreeNodeViewModel.cssrc/Arius.Explorer/Settings/ApplicationSettings.cssrc/Arius.Explorer/Settings/RepositoryOptions.cssrc/Arius.Explorer/Shared/Converters/BytesToReadableSizeConverter.cssrc/Arius.Explorer/Shared/Extensions/DataProtectionExtensions.cssrc/Arius.Explorer/Shared/Services/DialogService.cssrc/Arius.Explorer/Shared/Services/IDialogService.cssrc/Arius.Explorer/appsettings.jsonsrc/Arius.Integration.Tests/Arius.Integration.Tests.csprojsrc/Arius.Integration.Tests/ChunkIndex/ChunkIndexServiceIntegrationTests.cssrc/Arius.Integration.Tests/FileTree/TreeBuilderIntegrationTests.cssrc/Arius.Integration.Tests/Pipeline/CbcEncryptionServiceAdapter.cssrc/Arius.Integration.Tests/Pipeline/ContainerCreationTests.cssrc/Arius.Integration.Tests/Pipeline/CrashRecoveryTests.cssrc/Arius.Integration.Tests/Pipeline/GcmIntegrationTests.cssrc/Arius.Integration.Tests/Pipeline/LsIntegrationTests.cssrc/Arius.Integration.Tests/Pipeline/PipelineFixture.cssrc/Arius.Integration.Tests/Pipeline/RecoveryScriptTests.cssrc/Arius.Integration.Tests/Pipeline/RehydrationStateTests.cssrc/Arius.Integration.Tests/Pipeline/RestoreCostModelTests.cssrc/Arius.Integration.Tests/Pipeline/RestoreDispositionTests.cssrc/Arius.Integration.Tests/Pipeline/RestorePointerTimestampTests.cssrc/Arius.Integration.Tests/Pipeline/RoundtripTests.cssrc/Arius.Integration.Tests/Snapshot/SnapshotServiceIntegrationTests.cssrc/Arius.Integration.Tests/Storage/AzuriteFixture.cssrc/Arius.Integration.Tests/Storage/BlobStorageServiceTests.cssrc/Arius.slnxsrc/Directory.Packages.props
💤 Files with no reviewable changes (6)
- src/Arius.Cli/Arius.Cli.csproj
- src/Arius.Integration.Tests/Arius.Integration.Tests.csproj
- src/Arius.Core/Ls/LsModels.cs
- src/Arius.Core/Ls/LsHandler.cs
- src/Arius.Core/Archive/ArchiveModels.cs
- src/Arius.Core/Restore/RestoreModels.cs
| if (Repository == null || !SelectedFiles.Any()) | ||
| return; | ||
|
|
||
| await EnsureHydrationStatusesAsync(SelectedFiles, CancellationToken.None); | ||
|
|
||
| // Show confirmation dialog | ||
| var msg = new StringBuilder(); | ||
|
|
||
| var itemsToHydrate = SelectedFiles.Where(item => item.HydrationStatus == ChunkHydrationStatus.NeedsRehydration); | ||
| if (itemsToHydrate.Any()) | ||
| msg.AppendLine($"This will start hydration on {itemsToHydrate.Count()} item(s) ({itemsToHydrate.Sum(item => item.OriginalLength).Bytes().Humanize()}). This may incur a significant cost."); | ||
|
|
||
| var itemsPending = SelectedFiles.Where(item => item.HydrationStatus == ChunkHydrationStatus.RehydrationPending); | ||
| if (itemsPending.Any()) | ||
| msg.AppendLine($"{itemsPending.Count()} item(s) ({itemsPending.Sum(item => item.OriginalLength).Bytes().Humanize()}) are already rehydrating in the cloud."); | ||
|
|
||
| var itemsToRestore = SelectedFiles.Where(item => item.HydrationStatus == ChunkHydrationStatus.Available || item.HydrationStatus == ChunkHydrationStatus.Unknown); | ||
| msg.AppendLine($"This will download {itemsToRestore.Count()} item(s) ({itemsToRestore.Sum(item => item.OriginalLength).Bytes().Humanize()})."); | ||
| msg.AppendLine(); | ||
| msg.AppendLine("Proceed?"); | ||
|
|
||
| if (MessageBox.Show(msg.ToString(), App.Name, MessageBoxButton.YesNo, MessageBoxImage.Question) == MessageBoxResult.No) | ||
| return; | ||
|
|
||
| if (repositorySession.Mediator is null) | ||
| return; | ||
|
|
||
| // Execute the restore | ||
| try | ||
| { | ||
| IsLoading = true; | ||
|
|
||
| foreach (var selectedFile in SelectedFiles) | ||
| { | ||
| var command = new RestoreCommand(new RestoreOptions | ||
| { | ||
| RootDirectory = Repository.LocalDirectoryPath, | ||
| TargetPath = selectedFile.File.RelativePath, | ||
| Overwrite = true, | ||
| NoPointers = false, | ||
| }); | ||
|
|
||
| var result = await repositorySession.Mediator.Send(command); |
There was a problem hiding this comment.
Filter out local-only files before restoring.
The merged list can contain RepositoryFileEntry rows with ExistsInCloud = false, but this method treats every selected row as restorable and sends RestoreCommand for it. A mixed selection can therefore fail on the first local-only file and abort the whole batch.
🧰 Minimal change to restore only cloud-backed selections
- await EnsureHydrationStatusesAsync(SelectedFiles, CancellationToken.None);
+ var cloudSelections = SelectedFiles
+ .Where(item => item.File.ExistsInCloud)
+ .ToList();
+
+ if (cloudSelections.Count == 0)
+ return;
+
+ await EnsureHydrationStatusesAsync(cloudSelections, CancellationToken.None);
@@
- var itemsToHydrate = SelectedFiles.Where(item => item.HydrationStatus == ChunkHydrationStatus.NeedsRehydration);
+ var itemsToHydrate = cloudSelections.Where(item => item.HydrationStatus == ChunkHydrationStatus.NeedsRehydration);
@@
- var itemsPending = SelectedFiles.Where(item => item.HydrationStatus == ChunkHydrationStatus.RehydrationPending);
+ var itemsPending = cloudSelections.Where(item => item.HydrationStatus == ChunkHydrationStatus.RehydrationPending);
@@
- var itemsToRestore = SelectedFiles.Where(item => item.HydrationStatus == ChunkHydrationStatus.Available || item.HydrationStatus == ChunkHydrationStatus.Unknown);
+ var itemsToRestore = cloudSelections.Where(item => item.HydrationStatus == ChunkHydrationStatus.Available || item.HydrationStatus == ChunkHydrationStatus.Unknown);
@@
- foreach (var selectedFile in SelectedFiles)
+ foreach (var selectedFile in cloudSelections)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (Repository == null || !SelectedFiles.Any()) | |
| return; | |
| await EnsureHydrationStatusesAsync(SelectedFiles, CancellationToken.None); | |
| // Show confirmation dialog | |
| var msg = new StringBuilder(); | |
| var itemsToHydrate = SelectedFiles.Where(item => item.HydrationStatus == ChunkHydrationStatus.NeedsRehydration); | |
| if (itemsToHydrate.Any()) | |
| msg.AppendLine($"This will start hydration on {itemsToHydrate.Count()} item(s) ({itemsToHydrate.Sum(item => item.OriginalLength).Bytes().Humanize()}). This may incur a significant cost."); | |
| var itemsPending = SelectedFiles.Where(item => item.HydrationStatus == ChunkHydrationStatus.RehydrationPending); | |
| if (itemsPending.Any()) | |
| msg.AppendLine($"{itemsPending.Count()} item(s) ({itemsPending.Sum(item => item.OriginalLength).Bytes().Humanize()}) are already rehydrating in the cloud."); | |
| var itemsToRestore = SelectedFiles.Where(item => item.HydrationStatus == ChunkHydrationStatus.Available || item.HydrationStatus == ChunkHydrationStatus.Unknown); | |
| msg.AppendLine($"This will download {itemsToRestore.Count()} item(s) ({itemsToRestore.Sum(item => item.OriginalLength).Bytes().Humanize()})."); | |
| msg.AppendLine(); | |
| msg.AppendLine("Proceed?"); | |
| if (MessageBox.Show(msg.ToString(), App.Name, MessageBoxButton.YesNo, MessageBoxImage.Question) == MessageBoxResult.No) | |
| return; | |
| if (repositorySession.Mediator is null) | |
| return; | |
| // Execute the restore | |
| try | |
| { | |
| IsLoading = true; | |
| foreach (var selectedFile in SelectedFiles) | |
| { | |
| var command = new RestoreCommand(new RestoreOptions | |
| { | |
| RootDirectory = Repository.LocalDirectoryPath, | |
| TargetPath = selectedFile.File.RelativePath, | |
| Overwrite = true, | |
| NoPointers = false, | |
| }); | |
| var result = await repositorySession.Mediator.Send(command); | |
| if (Repository == null || !SelectedFiles.Any()) | |
| return; | |
| var cloudSelections = SelectedFiles | |
| .Where(item => item.File.ExistsInCloud) | |
| .ToList(); | |
| if (cloudSelections.Count == 0) | |
| return; | |
| await EnsureHydrationStatusesAsync(cloudSelections, CancellationToken.None); | |
| // Show confirmation dialog | |
| var msg = new StringBuilder(); | |
| var itemsToHydrate = cloudSelections.Where(item => item.HydrationStatus == ChunkHydrationStatus.NeedsRehydration); | |
| if (itemsToHydrate.Any()) | |
| msg.AppendLine($"This will start hydration on {itemsToHydrate.Count()} item(s) ({itemsToHydrate.Sum(item => item.OriginalLength).Bytes().Humanize()}). This may incur a significant cost."); | |
| var itemsPending = cloudSelections.Where(item => item.HydrationStatus == ChunkHydrationStatus.RehydrationPending); | |
| if (itemsPending.Any()) | |
| msg.AppendLine($"{itemsPending.Count()} item(s) ({itemsPending.Sum(item => item.OriginalLength).Bytes().Humanize()}) are already rehydrating in the cloud."); | |
| var itemsToRestore = cloudSelections.Where(item => item.HydrationStatus == ChunkHydrationStatus.Available || item.HydrationStatus == ChunkHydrationStatus.Unknown); | |
| msg.AppendLine($"This will download {itemsToRestore.Count()} item(s) ({itemsToRestore.Sum(item => item.OriginalLength).Bytes().Humanize()})."); | |
| msg.AppendLine(); | |
| msg.AppendLine("Proceed?"); | |
| if (MessageBox.Show(msg.ToString(), App.Name, MessageBoxButton.YesNo, MessageBoxImage.Question) == MessageBoxResult.No) | |
| return; | |
| if (repositorySession.Mediator is null) | |
| return; | |
| // Execute the restore | |
| try | |
| { | |
| IsLoading = true; | |
| foreach (var selectedFile in cloudSelections) | |
| { | |
| var command = new RestoreCommand(new RestoreOptions | |
| { | |
| RootDirectory = Repository.LocalDirectoryPath, | |
| TargetPath = selectedFile.File.RelativePath, | |
| Overwrite = true, | |
| NoPointers = false, | |
| }); | |
| var result = await repositorySession.Mediator.Send(command); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Arius.Explorer/RepositoryExplorer/RepositoryExplorerViewModel.cs` around
lines 389 - 431, The selection loop sends RestoreCommand for every item in
SelectedFiles even if some entries are local-only (ExistsInCloud == false),
causing the batch to fail on those items; before building/sending RestoreCommand
(around the foreach using SelectedFiles and calling
repositorySession.Mediator.Send), filter SelectedFiles to only include entries
where ExistsInCloud is true (e.g., create a local variable like filesToRestore =
SelectedFiles.Where(f => f.ExistsInCloud)), use that collection for the
confirmation counts and the foreach that creates RestoreCommand/RestoreOptions,
and skip or exclude local-only entries so only cloud-backed files are restored.
| try | ||
| { | ||
| var data = Encoding.UTF8.GetBytes(plainText); | ||
| var protectedData = ProtectedData.Protect(data, null, DataProtectionScope.CurrentUser); | ||
| return Convert.ToBase64String(protectedData); | ||
| } | ||
| catch (Exception) | ||
| { | ||
| // If protection fails, return the original value | ||
| // This could happen on non-Windows systems or if DPAPI is not available | ||
| return plainText; | ||
| } |
There was a problem hiding this comment.
Silent failure in Protect() may store unprotected secrets.
When ProtectedData.Protect fails, returning the original plaintext silently stores the secret unprotected. This could lead to credential leakage if the user assumes encryption succeeded. Consider at minimum logging the failure, or throwing for non-recoverable exceptions while handling PlatformNotSupportedException explicitly if cross-platform is a concern.
🔒 Proposed fix to handle failures more explicitly
try
{
var data = Encoding.UTF8.GetBytes(plainText);
var protectedData = ProtectedData.Protect(data, null, DataProtectionScope.CurrentUser);
return Convert.ToBase64String(protectedData);
}
- catch (Exception)
+ catch (PlatformNotSupportedException)
{
- // If protection fails, return the original value
- // This could happen on non-Windows systems or if DPAPI is not available
+ // DPAPI not available on non-Windows; return unprotected (acceptable for this app)
return plainText;
}
+ // Let other exceptions propagate - they indicate real problems📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try | |
| { | |
| var data = Encoding.UTF8.GetBytes(plainText); | |
| var protectedData = ProtectedData.Protect(data, null, DataProtectionScope.CurrentUser); | |
| return Convert.ToBase64String(protectedData); | |
| } | |
| catch (Exception) | |
| { | |
| // If protection fails, return the original value | |
| // This could happen on non-Windows systems or if DPAPI is not available | |
| return plainText; | |
| } | |
| try | |
| { | |
| var data = Encoding.UTF8.GetBytes(plainText); | |
| var protectedData = ProtectedData.Protect(data, null, DataProtectionScope.CurrentUser); | |
| return Convert.ToBase64String(protectedData); | |
| } | |
| catch (PlatformNotSupportedException) | |
| { | |
| // DPAPI not available on non-Windows; return unprotected (acceptable for this app) | |
| return plainText; | |
| } | |
| // Let other exceptions propagate - they indicate real problems |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Arius.Explorer/Shared/Extensions/DataProtectionExtensions.cs` around
lines 16 - 27, The current catch in DataProtectionExtensions swallows all
exceptions from ProtectedData.Protect and returns plaintext; change this to
handle PlatformNotSupportedException/NotSupportedException explicitly (return
plaintext or a clear fallback) while logging the exception, and rethrow or
surface other unexpected exceptions instead of silently returning the secret;
update the catch around ProtectedData.Protect (in DataProtectionExtensions) to
log the error with context and only suppress known cross-platform DPAPI errors,
but let non-recoverable errors propagate.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Arius.Cli.Tests/CliTests.cs (1)
93-97: Consider adding a test that verifies hydration handler dispatch.The
HydrationHandleris registered and exposed publicly but no tests in this file verify it receives calls. If hydration status is queryable via CLI (e.g., a hypothetical--hydration-statusflag), consider adding a test that confirms the handler is invoked with correct arguments.If hydration is only used internally by the Explorer UI and not exposed via CLI commands, this is fine as-is.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Cli.Tests/CliTests.cs` around lines 93 - 97, Add a unit test in CliTests.cs that exercises the CLI path which should invoke the HydrationHandler: register the existing mock hydrationHandler (already added via services.AddSingleton(hydrationHandler)) and run the command or flag that triggers hydration status (e.g., simulate invoking the CLI with "--hydration-status" or the specific command name used by the app); assert that hydrationHandler.HandleAsync (or the handler method name used by HydrationHandler) was called with the expected arguments and cancellation token, and verify any returned value or side-effect. Ensure the test references the concrete mock object named hydrationHandler and the handler method (e.g., HandleAsync/Dispatch) so the test will fail if dispatching is not wired up.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Arius.Cli.Tests/CliTests.cs`:
- Around line 93-97: Add a unit test in CliTests.cs that exercises the CLI path
which should invoke the HydrationHandler: register the existing mock
hydrationHandler (already added via services.AddSingleton(hydrationHandler)) and
run the command or flag that triggers hydration status (e.g., simulate invoking
the CLI with "--hydration-status" or the specific command name used by the app);
assert that hydrationHandler.HandleAsync (or the handler method name used by
HydrationHandler) was called with the expected arguments and cancellation token,
and verify any returned value or side-effect. Ensure the test references the
concrete mock object named hydrationHandler and the handler method (e.g.,
HandleAsync/Dispatch) so the test will fail if dispatching is not wired up.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eabb782f-9698-4a0e-b7c7-bb7f069c1fc3
📒 Files selected for processing (3)
src/Arius.Cli.Tests/CliTests.cssrc/Arius.Cli/CliBuilder.cssrc/Arius.Core/Features/ArchiveCommand/ArchiveCommandHandler.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Arius.Cli/CliBuilder.cs
Summary
Arius.ExplorerandArius.Explorer.Tests, then wire repository selection, tree loading, lazy hydration state, and archive/restore actions to the Arius7.Core mediator-based APIslspath with streamingListQuery/RepositoryEntrymodels, add recursive and local/cloud merged listing support, and update CLI, integration tests, and dependency wiring to consume the streamed resultsContainerNamesQuery,ChunkHydrationStatusQuery,ArchiveCommand, andRestoreCommand, including the feature-folder rename cleanup that standardizes command/query namingArius.AzureBlob, adding Azure blob tests, tightening read-only preflight handling, and updating DI, E2E, and integration coverage around the new abstractionsDetails
ListQuerystreams both directories and files so the Explorer tree can expand progressively while the CLI still renders file rows onlySummary by CodeRabbit
New Features
Infrastructure
Tests