feat: Arius Web - #115
Conversation
…fold, Angular foundation Arius.Core: - Add SnapshotsQuery (version/timestamp/fileCount per snapshot) and StatsQuery (files/original-size from manifest + distinct-chunk stored-size/unique-chunks from the chunk index). ISnapshotService.GetVersion + IChunkIndexService.GetStatsAsync + ChunkIndexLocalStore.GetStats. Registered in AddArius. TUnit tests for both. Arius.Api (new ASP.NET minimal API over Arius.Core): - Per-repository service-provider registry (AddMediator + AddArius), app SQLite (storage_accounts/repositories/jobs/schedules), Data-Protection secret encryption, AzureBlobServiceFactory wiring, accounts/repos CRUD + health endpoints. Arius.Web (new Angular 20 + Metronic v9 Tailwind/KTUI, from the official seed): - demo8 shell (icon rail + floating content card + top bar), Arius design tokens, StateRing component (flags->colours, exact handoff SVG), KtInit directive, placeholder Overview/Repos/Jobs/Settings screens. Builds clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Also restores the Arius.Api AppData layer (storage_accounts/repositories/jobs/schedules +
SecretProtector), which the phase-1 commit dropped due to a case-insensitive-filesystem
collision between the source folder and the runtime data dir; the layer now lives under
AppData/ with the dev DB at .appstate/ to avoid the clash. App-DB types are public so the
SignalR hub can inject them.
Arius.Api:
- REST endpoints namespaced under /api (so they don't collide with the SPA's /repos, /jobs
client routes). SignalR JobsHub at /hubs/arius with StreamEntries (ListQuery over the per-repo
read provider). GET /api/repos/{id}/snapshots (SnapshotsQuery) and /stats (StatsQuery).
EntryDto maps RepositoryEntry incl. decoded RepositoryEntryState flags for the state ring.
Arius.Web:
- ApiService (typed REST) + RealtimeService (@microsoft/signalr entry streaming) + dev proxy.
- Overview (KPIs + repos table), Repository detail (header + tabs), Files tab (snapshot
time-travel bar + scrubber, folder tree, file detail with state rings, collect set, state-
legend popover), Statistics tab, Properties tab. Verified via Playwright screenshots.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Arius.Api: - Per-job execution (JobRunner) in a fresh per-repo provider; writers serialized per repo. - Event→SignalR bridge: a per-job JobSink (inert for read providers) + INotificationHandler forwarders for the archive/restore events (auto-discovered by the Mediator generator), pushing console log lines + progress/stat updates to the job's SignalR group. - Restore cost-approval handshake: RestoreOptions.ConfirmRehydration parks on a TaskCompletionSource (RestoreApprovalRegistry) resolved by JobsHub.Approve. - JobsHub.StartArchive / StartRestore (caller joins the job group before events flow) + Approve. Arius.Web: - RealtimeService job feeds (log/progress/cost/done) + start/approve. DrawerStore drives the archive (idle→running→done) and restore (idle→running→cost→running→done) state machines. - Archive + Restore slide-over drawer (forms, progress bar + stat grid, dark LiveConsole, cost-approval modal). Wired from the repo header and the Files tab "Restore collected". Verified end-to-end against a live Azure repo: restore streamed all 18 files (incl. a tar bundle) to a temp dir with live log + progress; archive/restore drawer forms screenshot-verified. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ron scheduler Arius.Api: - App-DB job + schedule persistence; JobRunner records each run (insert running → complete). - Cron SchedulerService (BackgroundService + Cronos): wakes each minute, fires due archive schedules. Container discovery for the Add wizard via JobsHub.StreamContainers (configured account's stored key, or an explicit new name+key). GET /api/jobs, repo schedules CRUD. Arius.Web: - Jobs screen (runs table with status pills + progress + unified live console). - Add-existing wizard (account select/new → live container discovery → configure) and New repository wizard (account → new container with tier + passphrase confirmation + warning). - Schedule management in the Properties tab (list/add/delete cron). Verified live: Add wizard discovered the real account's containers (test/testexplorer/ v5migrationtest); Jobs screen shows the persisted restore run; Create wizard renders. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Arius.Api: - JobsHub.SearchAll: streams cross-repository filename-search hits (recursive ListQuery filter across every repo, each repo's failure isolated). SearchHitDto. - Serve the built Angular SPA from wwwroot (UseStaticFiles + MapFallbackToFile) so one Kestrel host serves both the SPA and the /api + /hubs endpoints. Arius.Web: - Global cross-repo search overlay (⌘K or the top-bar box, hidden on Overview): live results with state ring + repo·path + size, click opens the repo. SearchStore + RealtimeService.searchAll. Docker: - Multi-stage Dockerfile (Angular prod build → dotnet publish → aspnet runtime with the SPA in wwwroot) + docker-compose for Synology (mounted /data + repo overlay volumes) + .dockerignore. Verified: production image built and run — served the SPA (with deep-link fallback) + API on one port, connected to live Azure (snapshots + search) end-to-end. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a Playwright suite (src/Arius.Web/e2e) that drives the real Arius.Api + ng serve against a configured repository — codifying the manual verifications. playwright.config boots both servers (reuseExistingServer), globalSetup seeds/ensures a repo from ARIUS_E2E_* and purges scratch repos, a `repo` fixture targets the configured container, and data-testid hooks were added across the components. Specs (16 default, green): shell nav + ⌘K search, overview KPIs + repo table, files tab (snapshot bar, folder tree, state rings, filter, legend), time-travel, statistics, properties + schedule add/delete, restore drawer, archive drawer (tier + mutually-exclusive toggles), add-wizard live container discovery, create-wizard form gating, jobs table, global search → navigate. Plus an opt-in @Write spec (ARIUS_E2E_WRITE) doing a real archive to a dedicated container. A cost-approval @Write spec is test.fixme pending a restore-of-fresh-archive investigation. Two product bugs the suite caught, fixed here: - create-wizard: `canCreate` was a computed() over plain (non-signal) alias/passphrase, so the Create button never enabled — converted to a method re-evaluated under OnPush. - Arius.Api DeleteRepository now cascade-deletes the repo's jobs + schedules (foreign_keys=on otherwise blocks deletion of any repo that has run a job). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nation The cost-approval modal never triggered because the scratch repos restored into their own source folder, so every file was correctly skipped as identical (0 files restored → the archive-tier chunk was never resolved → no rehydration prompt). Not a product bug. Fix in the specs: clear the repo's localPath before restoring so the restore writes to a fresh temp dir; the archive-tier file is then actually restored, classifies as needing rehydration, and opens the cost-approval modal (declined). Also adds a real successful whole-repo restore to the archive @Write spec. Full suite now 18/18 green with ARIUS_E2E_WRITE=1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…README Playwright guide - restore-roundtrip.spec (@Write): archives a folder, then restores it twice — to an empty destination (files downloaded) and to a destination that already holds them (all skipped as identical) — codifying the two restore-destination behaviours. archive.spec reverts to archive-only. - README: full Playwright guidance (live full-stack model, env vars, default vs @Write runs, coverage, and how globalSetup/fixtures/testids are wired). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 34048869 | Triggered | Generic Password | dad387b | src/Arius.Web/e2e/specs/cost-approval.spec.ts | View secret |
| 34048869 | Triggered | Generic Password | 140c1ba | src/Arius.Web/e2e/specs/restore-roundtrip.spec.ts | View secret |
| 34048869 | Triggered | Generic Password | dad387b | src/Arius.Web/e2e/specs/archive.spec.ts | View secret |
| 34048869 | Triggered | Generic Password | ffa932c | src/Arius.Web/e2e/specs/restore-roundtrip.spec.ts | View secret |
| 34048869 | Triggered | Generic Password | ffa932c | src/Arius.Web/e2e/specs/cost-approval.spec.ts | View secret |
| 34048869 | Triggered | Generic Password | ffa932c | src/Arius.Web/e2e/specs/archive.spec.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
Warning Review limit reached
More reviews will be available in 9 minutes and 53 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds a new .NET API host, Angular web app, Core snapshot and statistics queries, SignalR job and search flows, SQLite-backed app data, Docker packaging, design handoff materials, Playwright E2E coverage, and CI/release workflow updates. ChangesArius web management stack
Sequence Diagram(s)sequenceDiagram
participant Web as Arius.Web
participant Hub as JobsHub
participant Runner as JobRunner
participant Core as Arius.Core
participant UI as DrawerStore
Web->>Hub: StartArchive / StartRestore
Hub->>Runner: run job
Runner->>Core: send ArchiveCommand / RestoreCommand
Core-->>Hub: forwarded progress events
Hub-->>Web: Log / Progress / CostEstimate / Done
Web->>UI: update drawer state
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #115 +/- ##
==========================================
+ Coverage 81.33% 81.51% +0.17%
==========================================
Files 100 103 +3
Lines 6579 6653 +74
Branches 899 908 +9
==========================================
+ Hits 5351 5423 +72
- Misses 992 994 +2
Partials 236 236
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Introduces a new web management UI (Angular + Metronic/Tailwind) backed by a new ASP.NET Core minimal API host that exposes Arius.Core via REST + SignalR, plus adds Core queries for snapshots and repository statistics used by the UI.
Changes:
- Add
Arius.WebAngular app (routes, components, formatting utilities, Tailwind/Metronic wiring) and Playwright E2E suite. - Add
Arius.Apihost (REST endpoints + SignalR hub) to browse/list/search and run archive/restore jobs, including cron-based scheduling. - Add Arius.Core
SnapshotsQueryandStatsQuery, plus chunk-index aggregation support for distinct-chunk stats.
Reviewed changes
Copilot reviewed 107 out of 157 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Directory.Packages.props | Adds Cronos package version for scheduling. |
| src/Arius.Web/tsconfig.spec.json | Adds TS config for unit tests. |
| src/Arius.Web/tsconfig.json | Base TS/Angular compiler configuration. |
| src/Arius.Web/tsconfig.app.json | App TS config for Angular build. |
| src/Arius.Web/tailwind.config.js | Tailwind content scanning config. |
| src/Arius.Web/src/tailwind.css | Tailwind CSS entrypoint. |
| src/Arius.Web/src/styles.scss | App design tokens + UI utility classes. |
| src/Arius.Web/src/main.ts | Angular bootstrap entrypoint. |
| src/Arius.Web/src/index.html | SPA HTML shell + Metronic assets. |
| src/Arius.Web/src/app/shared/state-ring/state-ring.component.ts | State ring SVG component. |
| src/Arius.Web/src/app/shared/state-ring/repository-entry-state.ts | Web-side mirror of RepositoryEntryState flags. |
| src/Arius.Web/src/app/shared/state-legend/state-legend.component.ts | “State legend” popover. |
| src/Arius.Web/src/app/shared/live-console/live-console.component.ts | Streaming console component for job logs. |
| src/Arius.Web/src/app/shared/format.ts | Shared formatting helpers (bytes/count/tier color). |
| src/Arius.Web/src/app/features/settings/settings.component.ts | Placeholder Settings screen. |
| src/Arius.Web/src/app/features/search/global-search-overlay.component.ts | Global ⌘K search overlay UI. |
| src/Arius.Web/src/app/features/repos/repos.component.ts | Repositories list screen. |
| src/Arius.Web/src/app/features/repo/statistics/statistics-tab.component.ts | Repository Statistics tab UI. |
| src/Arius.Web/src/app/features/repo/repo-detail.component.ts | Repository detail shell + tab navigation. |
| src/Arius.Web/src/app/features/repo/properties/properties-tab.component.ts | Properties tab + schedules UI. |
| src/Arius.Web/src/app/features/placeholder.component.ts | Generic placeholder component. |
| src/Arius.Web/src/app/features/overview/overview.component.ts | Overview screen + repo table. |
| src/Arius.Web/src/app/features/jobs/jobs.component.ts | Jobs screen + live output feed. |
| src/Arius.Web/src/app/core/state/search.store.ts | Client-side search state & debounced streaming. |
| src/Arius.Web/src/app/core/state/drawer.store.ts | Archive/restore drawer state machine + streams. |
| src/Arius.Web/src/app/core/services/metronic-init.service.ts | KTUI init wrapper for dynamic DOM. |
| src/Arius.Web/src/app/core/ktui/kt-init.directive.ts | Directive to re-init KTUI after render. |
| src/Arius.Web/src/app/core/api/realtime.service.ts | SignalR client for browse/jobs/search streams. |
| src/Arius.Web/src/app/core/api/api.service.ts | REST client for accounts/repos/jobs/stats/schedules. |
| src/Arius.Web/src/app/core/api/api-models.ts | Frontend DTO contracts. |
| src/Arius.Web/src/app/app.routes.ts | SPA routes. |
| src/Arius.Web/src/app/app.config.ts | Angular app providers + router/http config. |
| src/Arius.Web/src/app/app.component.ts | Application shell (rail + topbar + router outlet). |
| src/Arius.Web/README.md | Web dev + E2E test documentation. |
| src/Arius.Web/public/assets/media/arius-iceberg.svg | Adds Arius branding asset. |
| src/Arius.Web/public/assets/media/app/mini-logo.svg | Adds Metronic/brand asset. |
| src/Arius.Web/public/assets/media/app/mini-logo-square-gray.svg | Adds Metronic/brand asset. |
| src/Arius.Web/public/assets/media/app/mini-logo-square-gray-dark.svg | Adds Metronic/brand asset. |
| src/Arius.Web/public/assets/media/app/mini-logo-primary.svg | Adds Metronic/brand asset. |
| src/Arius.Web/public/assets/media/app/mini-logo-primary-dark.svg | Adds Metronic/brand asset. |
| src/Arius.Web/public/assets/media/app/mini-logo-gray.svg | Adds Metronic/brand asset. |
| src/Arius.Web/public/assets/media/app/mini-logo-gray-dark.svg | Adds Metronic/brand asset. |
| src/Arius.Web/public/assets/media/app/mini-logo-circle.svg | Adds Metronic/brand asset. |
| src/Arius.Web/public/assets/media/app/mini-logo-circle-success.svg | Adds Metronic/brand asset. |
| src/Arius.Web/public/assets/media/app/mini-logo-circle-primary.svg | Adds Metronic/brand asset. |
| src/Arius.Web/public/assets/media/app/mini-logo-circle-primary-dark.svg | Adds Metronic/brand asset. |
| src/Arius.Web/public/assets/media/app/mini-logo-circle-dark.svg | Adds Metronic/brand asset. |
| src/Arius.Web/public/assets/media/app/default-logo.svg | Adds Metronic/brand asset. |
| src/Arius.Web/public/assets/media/app/default-logo-dark.svg | Adds Metronic/brand asset. |
| src/Arius.Web/proxy.conf.json | Dev proxy to API for /api + /hubs. |
| src/Arius.Web/playwright.config.ts | Playwright config to boot API + ng serve. |
| src/Arius.Web/package.json | Angular/Tailwind/Playwright dependencies and scripts. |
| src/Arius.Web/e2e/support/global-setup.ts | E2E global setup + seeding logic. |
| src/Arius.Web/e2e/support/fixtures.ts | E2E fixtures for repo + patch helpers. |
| src/Arius.Web/e2e/specs/time-travel.spec.ts | E2E time-travel snapshot picker test. |
| src/Arius.Web/e2e/specs/statistics.spec.ts | E2E statistics tab KPI test. |
| src/Arius.Web/e2e/specs/shell.spec.ts | E2E shell navigation + search visibility test. |
| src/Arius.Web/e2e/specs/search.spec.ts | E2E global search + navigation test. |
| src/Arius.Web/e2e/specs/restore.spec.ts | E2E restore drawer UI smoke test. |
| src/Arius.Web/e2e/specs/restore-roundtrip.spec.ts | Destructive E2E restore round-trip test (@Write). |
| src/Arius.Web/e2e/specs/properties.spec.ts | E2E properties + schedule add/delete test. |
| src/Arius.Web/e2e/specs/overview.spec.ts | E2E overview KPIs + repo navigation test. |
| src/Arius.Web/e2e/specs/jobs.spec.ts | E2E jobs screen + console smoke test. |
| src/Arius.Web/e2e/specs/files.spec.ts | E2E file browser smoke tests (tree/list/filter/legend). |
| src/Arius.Web/e2e/specs/create-wizard.spec.ts | E2E create wizard gating test. |
| src/Arius.Web/e2e/specs/cost-approval.spec.ts | Destructive E2E cost approval flow (@Write). |
| src/Arius.Web/e2e/specs/archive.spec.ts | Archive drawer tests (+ destructive archive @Write). |
| src/Arius.Web/e2e/specs/add-wizard.spec.ts | E2E add-existing container discovery test. |
| src/Arius.Web/e2e/.env.example | Example env vars for E2E seeding. |
| src/Arius.Web/angular.json | Angular build/serve/test configuration + assets/styles/scripts. |
| src/Arius.Web/.postcssrc.json | PostCSS config for Tailwind v4. |
| src/Arius.Web/.gitignore | Ignores node/dist/angular caches + Playwright state. |
| src/Arius.Web/.editorconfig | Editor defaults for TS/MD formatting. |
| src/Arius.slnx | Adds Arius.Api project to solution. |
| src/Arius.Core/Shared/Snapshot/SnapshotService.cs | Adds GetVersion helper to snapshot service. |
| src/Arius.Core/Shared/ChunkIndex/IChunkIndexService.cs | Adds stats aggregation contract for chunk index. |
| src/Arius.Core/Shared/ChunkIndex/ChunkIndexService.cs | Implements GetStatsAsync by loading all shard prefixes. |
| src/Arius.Core/Shared/ChunkIndex/ChunkIndexLocalStore.cs | Adds SQLite aggregate query for distinct chunks + stored size. |
| src/Arius.Core/ServiceCollectionExtensions.cs | Registers SnapshotsQuery + StatsQuery handlers in DI. |
| src/Arius.Core/Features/StatsQuery/StatsQuery.cs | New Core stats query + handler. |
| src/Arius.Core/Features/SnapshotsQuery/SnapshotsQuery.cs | New Core snapshots query + handler. |
| src/Arius.Core.Tests/Shared/Snapshot/Fakes/FakeSnapshotService.cs | Updates fake snapshot service for GetVersion. |
| src/Arius.Core.Tests/Features/StatsQuery/StatsQueryHandlerTests.cs | Adds tests for stats aggregation correctness. |
| src/Arius.Core.Tests/Features/SnapshotsQuery/SnapshotsQueryHandlerTests.cs | Adds tests for snapshot enumeration semantics. |
| src/Arius.Api/Properties/launchSettings.json | Adds launch profile for API. |
| src/Arius.Api/Program.cs | Minimal API host config, CORS, static files + hub + endpoints. |
| src/Arius.Api/Jobs/SchedulerService.cs | Background cron schedule runner using Cronos. |
| src/Arius.Api/Jobs/RestoreApprovalRegistry.cs | Tracks restore cost-approval handshake. |
| src/Arius.Api/Jobs/JobSink.cs | SignalR job message sink + aggregated counters. |
| src/Arius.Api/Jobs/JobFormat.cs | Shared byte formatter for job logs. |
| src/Arius.Api/Hubs/RestoreForwarders.cs | Mediator notification forwarders for restore events. |
| src/Arius.Api/Hubs/JobsHub.cs | Main SignalR hub (browse streams, job starts, search, approvals). |
| src/Arius.Api/Hubs/ArchiveForwarders.cs | Mediator notification forwarders for archive events. |
| src/Arius.Api/Endpoints/RepositoryEndpoints.cs | REST CRUD for repositories + provider eviction. |
| src/Arius.Api/Endpoints/JobEndpoints.cs | REST jobs list + schedules CRUD. |
| src/Arius.Api/Endpoints/BrowseEndpoints.cs | REST snapshots + stats endpoints backed by Core queries. |
| src/Arius.Api/Endpoints/AccountEndpoints.cs | REST CRUD for accounts. |
| src/Arius.Api/Contracts/EntryDto.cs | Streaming entry/search DTOs + mapping from Core entries. |
| src/Arius.Api/Contracts/Dtos.cs | REST DTOs for accounts/repos/snapshots/stats/jobs/schedules. |
| src/Arius.Api/Composition/RepositoryProviderRegistry.cs | Per-repo provider cache/build logic for Core service graphs. |
| src/Arius.Api/Arius.Api.csproj | New API project + package references. |
| src/Arius.Api/appsettings.json | Serilog + Arius path settings. |
| src/Arius.Api/AppData/SecretProtector.cs | Data Protection-based secret encryption for app DB. |
| src/Arius.Api/AppData/Records.cs | App DB record types for accounts/repos/jobs/schedules. |
| src/Arius.Api/.gitignore | Ignores dev .appstate artifacts. |
| openspec/changes/2026-06-16-arius-web-claude-design-handoff/assets/arius-iceberg.svg | Design handoff asset import. |
| Dockerfile | Multi-stage build to serve SPA from API wwwroot. |
| docker-compose.yml | Deployment example with volumes + ports. |
| AGENTS.md | Documents new hosts (Api/Web) architecture. |
| .dockerignore | Excludes build outputs and caches from Docker context. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| close(): void { | ||
| this.open.set(false); | ||
| this.subscription?.unsubscribe(); | ||
| } |
There was a problem hiding this comment.
Actionable comments posted: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Arius.Api/AppData/Records.cs (1)
1-48: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winMake app data records internal.
All five records in this file are declared
publicbut are only consumed within theArius.Apiassembly (byAppDatabase, endpoints, and job infrastructure). As per coding guidelines, non-test classes should be internal unless they must be consumed by another non-test assembly.♻️ Proposed fix
-public sealed record AccountRecord(long Id, string Name, string? EncryptedAccountKey, DateTimeOffset CreatedAt); +internal sealed record AccountRecord(long Id, string Name, string? EncryptedAccountKey, DateTimeOffset CreatedAt); -public sealed record RepositoryRecord( +internal sealed record RepositoryRecord( -public sealed record RepositoryConnection( +internal sealed record RepositoryConnection( -public sealed record JobRecord( +internal sealed record JobRecord( -public sealed record ScheduleRecord( +internal sealed record ScheduleRecord(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Api/AppData/Records.cs` around lines 1 - 48, Change the access modifier of all five records (AccountRecord, RepositoryRecord, RepositoryConnection, JobRecord, and ScheduleRecord) from public to internal. Replace each occurrence of "public sealed record" with "internal sealed record" for all records in this file, as they are only consumed within the Arius.Api assembly and should follow the coding guideline that non-test classes be internal unless consumed by another non-test assembly.Source: Coding guidelines
src/Arius.Api/Contracts/Dtos.cs (1)
1-60: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winMake contract DTOs internal.
All DTO records in this file are declared
publicbut are serialized over HTTP/JSON rather than consumed as compiled assembly references. As per coding guidelines, these should be internal.♻️ Proposed fix
-public sealed record AccountDto(long Id, string Name, int Repositories, bool HasKey); +internal sealed record AccountDto(long Id, string Name, int Repositories, bool HasKey); -public sealed record CreateAccountRequest(string Name, string? AccountKey); +internal sealed record CreateAccountRequest(string Name, string? AccountKey); -public sealed record RepositoryDto( +internal sealed record RepositoryDto( -public sealed record CreateRepositoryRequest( +internal sealed record CreateRepositoryRequest( -public sealed record UpdateRepositoryRequest( +internal sealed record UpdateRepositoryRequest( -public sealed record SnapshotDto(string Version, DateTimeOffset Timestamp, long FileCount); +internal sealed record SnapshotDto(string Version, DateTimeOffset Timestamp, long FileCount); -public sealed record StatsDto(long Files, long OriginalSize, long StoredSize, long UniqueChunks, bool Pending); +internal sealed record StatsDto(long Files, long OriginalSize, long StoredSize, long UniqueChunks, bool Pending); -public sealed record JobDto( +internal sealed record JobDto( -public sealed record ScheduleDto(long Id, long RepoId, string Cron, string Kind, bool Enabled, DateTimeOffset? NextRun); +internal sealed record ScheduleDto(long Id, long RepoId, string Cron, string Kind, bool Enabled, DateTimeOffset? NextRun); -public sealed record CreateScheduleRequest(string Cron, string? Kind); +internal sealed record CreateScheduleRequest(string Cron, string? Kind);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Api/Contracts/Dtos.cs` around lines 1 - 60, All DTO records in the Dtos.cs file are currently declared as public, but they should be internal since they are serialized over HTTP/JSON rather than consumed as compiled assembly references. Change the access modifier from public to internal for all DTO records: AccountDto, CreateAccountRequest, RepositoryDto, CreateRepositoryRequest, UpdateRepositoryRequest, SnapshotDto, StatsDto, JobDto, ScheduleDto, and CreateScheduleRequest. Replace each occurrence of "public sealed record" with "internal sealed record" throughout the file.Source: Coding guidelines
🟡 Minor comments (11)
src/Arius.Core.Tests/Features/StatsQuery/StatsQueryHandlerTests.cs-53-53 (1)
53-53:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse FakeLogger instead of NullLogger in tests.
As per coding guidelines, test projects should use
FakeLogger<T>fromMicrosoft.Extensions.Diagnostics.Testinginstead ofNullLogger<T>.📝 Proposed fix
- var handler = new StatsQueryHandler(fixture.Snapshot, fixture.Index, NullLogger<StatsQueryHandler>.Instance); + var handler = new StatsQueryHandler(fixture.Snapshot, fixture.Index, new FakeLogger<StatsQueryHandler>());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Core.Tests/Features/StatsQuery/StatsQueryHandlerTests.cs` at line 53, The StatsQueryHandler instantiation in StatsQueryHandlerTests is using NullLogger<StatsQueryHandler>.Instance which does not comply with coding guidelines. Replace NullLogger<StatsQueryHandler>.Instance with FakeLogger<StatsQueryHandler> from the Microsoft.Extensions.Diagnostics.Testing namespace. Ensure the FakeLogger is properly imported and instantiated where the StatsQueryHandler constructor is called in the test setup.Source: Coding guidelines
src/Arius.Core.Tests/Features/StatsQuery/StatsQueryHandlerTests.cs-37-37 (1)
37-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse FakeLogger instead of NullLogger in tests.
As per coding guidelines, test projects should use
FakeLogger<T>fromMicrosoft.Extensions.Diagnostics.Testinginstead ofNullLogger<T>. This allows tests to assert on log output when needed.📝 Proposed fix
- var handler = new StatsQueryHandler(fixture.Snapshot, fixture.Index, NullLogger<StatsQueryHandler>.Instance); + var handler = new StatsQueryHandler(fixture.Snapshot, fixture.Index, new FakeLogger<StatsQueryHandler>());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Core.Tests/Features/StatsQuery/StatsQueryHandlerTests.cs` at line 37, In the StatsQueryHandler instantiation within the test setup, replace NullLogger<StatsQueryHandler>.Instance with FakeLogger<StatsQueryHandler> from the Microsoft.Extensions.Diagnostics.Testing namespace. This change enables test assertions on log output while following the project's coding guidelines for test logging practices.Source: Coding guidelines
src/Arius.Core/Features/StatsQuery/StatsQuery.cs-43-47 (1)
43-47:⚠️ Potential issue | 🟡 MinorAll feature handlers currently use
public sealed class, which contradicts the coding guideline.While StatsQueryHandler follows the established pattern of all 8 feature handlers (e.g., ArchiveCommandHandler), the coding guideline requires non-test classes to be
internalunless consumed by another non-test assembly, withInternalsVisibleTofor test access. Mediator's assembly scanning is not the constraint here—handlers are manually registered via factories inServiceCollectionExtensions, which can instantiate internal classes within the same assembly. SinceInternalsVisibleTois already configured in Arius.Core, this pattern should be changed systematically across all feature handlers tointernal sealed class, not addressed in isolation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Core/Features/StatsQuery/StatsQuery.cs` around lines 43 - 47, The StatsQueryHandler class is declared as public sealed class, but it should be internal sealed class according to the coding guidelines. Handlers are manually registered via factories in ServiceCollectionExtensions and do not need to be public, and InternalsVisibleTo is already configured in Arius.Core for test access. Change the access modifier of the StatsQueryHandler class declaration from public to internal, and apply this same change systematically across all other feature handlers in the codebase (such as ArchiveCommandHandler) to maintain consistency with the established coding standard.src/Arius.Web/src/app/core/state/search.store.ts-21-30 (1)
21-30:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCancel queued debounce and clear loading state when closing search.
Lines 21-24 stop only the current subscription. A pending timeout from Line 29 can still fire after close, and
loadingmay staytrueuntil another run completes.Suggested fix
close(): void { this.open.set(false); + clearTimeout(this.debounce); + this.debounce = undefined; this.subscription?.unsubscribe(); + this.subscription = undefined; + this.loading.set(false); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Web/src/app/core/state/search.store.ts` around lines 21 - 30, The close() method unsubscribes from the current subscription but does not clear the pending debounce timeout that may have been set by the setQuery() method. This causes the queued run() call to execute after close() is invoked, keeping the loading state set to true. In the close() method, add a clearTimeout call for the this.debounce property (similar to what is done in setQuery()) to cancel any pending timeout, ensuring the loading state is properly cleared when the search is closed.src/Arius.Web/.gitignore-33-37 (1)
33-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the final
.vscodeignore rule that cancels your allowlist.Line 55 re-ignores the whole
.vscodefolder after Lines 34–37 unignore specific files, so those files still won’t be tracked.Suggested fix
-.vscodeAlso applies to: 55-55
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Web/.gitignore` around lines 33 - 37, The .gitignore file has conflicting rules where the allowlist for specific .vscode files (settings.json, tasks.json, launch.json, extensions.json) created with negation patterns is being overridden by a subsequent rule that re-ignores the entire .vscode directory. Remove the line that re-ignores the .vscode folder (the one at the end that just says .vscode or .vscode/ without the negation operator) so that the specific files you've explicitly unignored with the ! prefix remain tracked in version control.src/Arius.Web/src/app/features/search/global-search-overlay.component.ts-27-27 (1)
27-27:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a collision-safe tracking key for search hits.
Line 27 concatenates
repoIdand path without a delimiter, which can produce duplicate keys and unstable row reuse.Suggested fix
- `@for` (hit of store.results(); track hit.repoId + hit.entry.relativePath) { + `@for` (hit of store.results(); track hit.repoId + '|' + hit.entry.relativePath) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Web/src/app/features/search/global-search-overlay.component.ts` at line 27, The track expression in the `@for` directive of global-search-overlay.component.ts concatenates hit.repoId and hit.entry.relativePath without a delimiter, which can cause key collisions (e.g., "abc" + "def" equals "ab" + "cdef"). Add a delimiter such as a pipe character "|" between hit.repoId and hit.entry.relativePath in the track expression to create a collision-safe composite key that uniquely identifies each search result row.src/Arius.Web/src/app/features/repo/properties/properties-tab.component.ts-44-50 (1)
44-50:⚠️ Potential issue | 🟡 MinorDisplay next run time in UTC to match the label.
Line 49 renders
nextRunin local time, but the label on line 44 states schedules fire in UTC. Add the timezone parameter to the date pipe and clarify the display:Fix
- <span style="font-size:12px;color:`#a1a1aa`;margin-left:auto">{{ s.nextRun ? 'next ' + (s.nextRun | date:'dd MMM HH:mm') : 'computing…' }}</span> + <span style="font-size:12px;color:`#a1a1aa`;margin-left:auto">{{ s.nextRun ? 'next ' + (s.nextRun | date:'dd MMM HH:mm':'UTC') + ' UTC' : 'computing…' }}</span>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Web/src/app/features/repo/properties/properties-tab.component.ts` around lines 44 - 50, The label on line 44 states that cron schedules fire in UTC, but the date pipe formatting for the nextRun property on line 49 does not include a timezone specification, causing it to display in local time instead. Modify the date pipe for s.nextRun by adding the UTC timezone parameter to the format string so that the displayed next run time matches the UTC timezone mentioned in the label and provides consistent information to the user.src/Arius.Web/src/app/features/repo/repo-detail.component.ts-66-69 (1)
66-69:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDo not map load failures to the same state as “loading”.
At Line 66-Line 69,
catchError(() => of(null))makes failures indistinguishable from initial loading, so the fallback at Line 49 can show “Loading repository…” forever on errors/not-found.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Web/src/app/features/repo/repo-detail.component.ts` around lines 66 - 69, The issue is that the catchError operator in the repo signal's pipe (in the switchMap for this.api.getRepository) maps load failures to null, which is indistinguishable from the initial loading state. This causes the template fallback to display "Loading repository…" indefinitely on errors. Instead of catching the error and returning of(null), which conflates error state with loading state, remove the catchError entirely to allow errors to propagate, or restructure the observable chain to use a different value or pattern that allows the template at line 49 to differentiate between loading, success, and error conditions.src/Arius.Web/src/index.html-10-23 (1)
10-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate placeholder metadata to reflect Arius branding.
The document contains placeholder Metronic template metadata that should be customized for production:
- Lines 11-16: Twitter metadata references
@keenthemesand"Metronic - Tailwind CSS "- Lines 17-23: OpenGraph metadata has the same placeholder values
- Lines 10, 15, 22: Description fields are empty
These values appear in browser tabs, search results, and social media previews.
📝 Suggested updates
- <meta content="" name="description" /> - <meta content="`@keenthemes`" name="twitter:site" /> - <meta content="`@keenthemes`" name="twitter:creator" /> + <meta content="Arius - Deduplicating backup and archival system" name="description" /> + <meta content="`@arius`" name="twitter:site" /> + <meta content="`@arius`" name="twitter:creator" /> <meta content="summary_large_image" name="twitter:card" /> - <meta content="Metronic - Tailwind CSS " name="twitter:title" /> - <meta content="" name="twitter:description" /> + <meta content="Arius" name="twitter:title" /> + <meta content="Deduplicating backup and archival system" name="twitter:description" /> <meta content="assets/media/app/og-image.png" name="twitter:image" /> <meta content="/" property="og:url" /> <meta content="en_US" property="og:locale" /> <meta content="website" property="og:type" /> - <meta content="`@keenthemes`" property="og:site_name" /> - <meta content="Metronic - Tailwind CSS " property="og:title" /> - <meta content="" property="og:description" /> + <meta content="Arius" property="og:site_name" /> + <meta content="Arius" property="og:title" /> + <meta content="Deduplicating backup and archival system" property="og:description" />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Web/src/index.html` around lines 10 - 23, The index.html file contains placeholder metadata from the Metronic template that must be updated to reflect Arius branding for production use. Replace all instances of "`@keenthemes`" in the meta tags with name="twitter:site", name="twitter:creator", and property="og:site_name" with appropriate Arius branding values. Update the meta tags with name="twitter:title" and property="og:title" from "Metronic - Tailwind CSS " to an appropriate Arius product title. Fill in the currently empty description fields in the meta tags with name="description", name="twitter:description", and property="og:description" with meaningful descriptions of the Arius application. These metadata values are critical as they appear in browser tabs, search engine results, and social media previews.src/Arius.Web/e2e/support/fixtures.ts-18-19 (1)
18-19:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFail fast on fixture API failures.
The fixture consumes
/api/reposand PATCH responses without asserting success, so backend regressions can surface later as confusing test failures.Proposed fix
- const repos = await (await request.get('/api/repos')).json(); + const reposResponse = await request.get('/api/repos'); + expect(reposResponse.ok()).toBeTruthy(); + const repos = await reposResponse.json(); @@ - await use(async (id, body) => { await request.patch(`/api/repos/${id}`, { data: body }); }); + await use(async (id, body) => { + const patchResponse = await request.patch(`/api/repos/${id}`, { data: body }); + expect(patchResponse.ok()).toBeTruthy(); + });Also applies to: 29-29
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Web/e2e/support/fixtures.ts` around lines 18 - 19, The fixture is consuming API responses from the `/api/repos` endpoint and the PATCH call (mentioned at line 29) without asserting that the requests were successful before parsing the JSON. Add response status assertions by checking response.ok or the status code after each request.get() and request.patch() call to ensure the HTTP response indicates success before calling .json() on it. This will surface backend failures with clear error messages instead of allowing confusing JSON parsing errors to propagate through the tests.src/Arius.Web/e2e/specs/files.spec.ts-19-21 (1)
19-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssertion checks only one row while the test claims all rows.
This currently verifies the first row only, so regressions in other rows can slip through.
Proposed fix
- // every file row renders the state-ring SVG - await expect(files.first().locator('arius-state-ring svg')).toBeVisible(); + // every file row renders the state-ring SVG + const rowCount = await files.count(); + await expect(files.locator('arius-state-ring svg')).toHaveCount(rowCount);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Web/e2e/specs/files.spec.ts` around lines 19 - 21, The test assertion in the files.first() locator only validates the arius-state-ring SVG in the first file row, but the comment indicates every file row should be checked. Modify the assertion to validate all file rows instead of just the first one by using a method that checks every row in the files collection for the presence of the arius-state-ring svg element, such as iterating through all rows or using a count-based assertion.
🧹 Nitpick comments (10)
src/Arius.Web/src/app/core/services/metronic-init.service.ts (1)
8-8: 💤 Low valueRemove the empty constructor.
The empty constructor serves no purpose and can be removed. Angular will provide dependency injection without an explicit constructor when none is needed.
♻️ Proposed fix
- constructor() { } - private initKtComponent(name: string): void {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Web/src/app/core/services/metronic-init.service.ts` at line 8, The MetronicInitService class has an empty constructor that is unnecessary and should be removed. Angular provides automatic dependency injection without requiring an explicit empty constructor to be defined. Delete the entire constructor() { } line from the MetronicInitService class, as it adds no value and violates the principle of removing unused code.src/Arius.Api/Hubs/JobsHub.cs (1)
19-26: ⚡ Quick winReduce hub visibility to internal unless cross-assembly consumption is required.
JobsHubcurrently expands public surface without clear cross-assembly need in this change.Suggested visibility change
-public sealed class JobsHub( +internal sealed class JobsHub(As per coding guidelines: "Make non-test classes internal; only make them public when they must be consumed by another non-test assembly; for test access, prefer InternalsVisibleTo".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Api/Hubs/JobsHub.cs` around lines 19 - 26, The JobsHub class is currently declared as public, but according to coding guidelines it should be internal unless there is a clear need for cross-assembly consumption. Change the visibility modifier of the JobsHub class declaration from public to internal, unless this hub is explicitly required to be consumed by another assembly outside of the current project.Source: Coding guidelines
src/Arius.Api/Hubs/RestoreForwarders.cs (1)
9-66: ⚡ Quick winMake restore forwarders internal and split into dedicated files.
All forwarders here are public top-level classes in a single file, which conflicts with the repo’s class/file organization and visibility guidance.
As per coding guidelines, "Make non-test classes internal; only make them public when they must be consumed by another non-test assembly" and "Prefer one top-level class per file, with the filename matching the class name".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Api/Hubs/RestoreForwarders.cs` around lines 9 - 66, Change the visibility of all six forwarder classes (SnapshotResolvedForwarder, TreeTraversalCompleteForwarder, RehydrationStatusForwarder, RehydrationStartedForwarder, ChunkDownloadStartedForwarder, and FileRestoredForwarder) from public to internal. Additionally, move each forwarder class into its own dedicated file, naming each file to match its corresponding class name, following the one-class-per-file convention.Source: Coding guidelines
src/Arius.Api/Jobs/JobRunner.cs (1)
19-25: ⚡ Quick winNarrow
JobRunnervisibility to internal if it is assembly-local.As per coding guidelines, "Make non-test classes internal; only make them public when they must be consumed by another non-test assembly; for test access, prefer InternalsVisibleTo".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Api/Jobs/JobRunner.cs` around lines 19 - 25, The JobRunner class is currently declared with public visibility but should be internal according to coding guidelines since it appears to be assembly-local. Change the access modifier of the JobRunner class from public to internal by replacing "public sealed class JobRunner" with "internal sealed class JobRunner" in the class declaration.Source: Coding guidelines
src/Arius.Api/Composition/RepositoryProviderRegistry.cs (1)
24-25: ⚡ Quick winReduce public surface and keep one top-level class per file.
Line 24 exposes a non-test type publicly, and this file also contains a second top-level class at Line 155.
As per coding guidelines, "Make non-test classes internal; only make them public when they must be consumed by another non-test assembly" and "Prefer one top-level class per file, with the filename matching the class name".
Also applies to: 154-159
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Api/Composition/RepositoryProviderRegistry.cs` around lines 24 - 25, Change the access modifier of RepositoryProviderRegistry from public sealed to internal sealed on line 24, as it should not be exposed publicly based on the coding guidelines. Additionally, extract the second top-level class located around line 155 (the one mentioned in the "Also applies to" section) into its own separate file with a matching filename to follow the one-class-per-file convention.Source: Coding guidelines
src/Arius.Api/Jobs/RestoreApprovalRegistry.cs (1)
10-10: ⚡ Quick winMake this registry internal unless another assembly consumes it.
As per coding guidelines, "Make non-test classes internal; only make them public when they must be consumed by another non-test assembly; for test access, prefer InternalsVisibleTo".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Api/Jobs/RestoreApprovalRegistry.cs` at line 10, The RestoreApprovalRegistry class is currently declared as public but should be internal unless it is consumed by another non-test assembly. Change the access modifier of the RestoreApprovalRegistry class from public to internal, keeping the sealed modifier intact. If another assembly does depend on this class, confirm that dependency exists before making this change.Source: Coding guidelines
src/Arius.Api/Jobs/JobSink.cs (2)
79-80: ⚡ Quick winRemove duplicate byte-format logic.
FormatBytesduplicatesJobFormat.Bytes; keeping both increases drift risk in UI stats formatting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Api/Jobs/JobSink.cs` around lines 79 - 80, The FormatBytes method in JobSink.cs contains duplicate byte-formatting logic that already exists in JobFormat.Bytes, creating maintenance risk and potential drift between the two implementations. Remove the FormatBytes method entirely and replace all its usages within JobSink.cs with calls to the existing JobFormat.Bytes method instead to maintain a single source of truth for byte formatting.
13-13: ⚡ Quick winNarrow
JobSinkvisibility unless cross-assembly access is required.This non-test class is public but appears API-internal.
As per coding guidelines, "Make non-test classes internal; only make them public when they must be consumed by another non-test assembly; for test access, prefer InternalsVisibleTo".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Api/Jobs/JobSink.cs` at line 13, The JobSink class is declared as public but appears to be internal to the API and not meant for cross-assembly consumption. Change the access modifier of the JobSink class from public to internal, unless this class is explicitly required by another non-test assembly. If test access is needed, consider using InternalsVisibleTo attribute instead of making it public.Source: Coding guidelines
src/Arius.Api/Hubs/ArchiveForwarders.cs (1)
11-71: ⚡ Quick winMake forwarders internal and split them into one class per file.
This file defines multiple public non-test top-level classes together, which makes API surface and navigation noisier than needed.
As per coding guidelines, "Make non-test classes internal; only make them public when they must be consumed by another non-test assembly" and "Prefer one top-level class per file, with the filename matching the class name".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Api/Hubs/ArchiveForwarders.cs` around lines 11 - 71, Change all the forwarder classes (ScanCompleteForwarder, FileHashedForwarder, TarBundleSealingForwarder, TarBundleUploadedForwarder, ChunkUploadedForwarder, and SnapshotCreatedForwarder) from public to internal by replacing each "public sealed class" declaration with "internal sealed class". Then move each forwarder class into its own separate file where the filename matches the class name (for example, ScanCompleteForwarder.cs, FileHashedForwarder.cs, and so on), keeping the constructor dependency injection and Handle method implementation intact in each new file.Source: Coding guidelines
src/Arius.Web/e2e/specs/statistics.spec.ts (1)
5-11: ⚡ Quick winStrengthen the KPI assertions to match the test intent.
The test name promises “four KPI cards with real figures,” but only one card is checked for numeric content.
Suggested assertion upgrade
await expect(page.getByTestId('kpi-card')).toHaveCount(4, { timeout: 30_000 }); - const filesCard = page.getByTestId('kpi-card').filter({ hasText: 'Files' }).first(); - await expect(filesCard).toContainText(/\d/); // a real number, not just a dash - await expect(filesCard).not.toContainText('—'); + const cards = page.getByTestId('kpi-card'); + for (let i = 0; i < 4; i++) { + await expect(cards.nth(i)).toContainText(/\d/); // real number, not placeholder-only + await expect(cards.nth(i)).not.toContainText('—'); + } await expect(page.getByText('Unique chunks')).toBeVisible();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Web/e2e/specs/statistics.spec.ts` around lines 5 - 11, The test currently only validates that the 'Files' KPI card contains numeric content, but the test intent is to verify all four KPI cards have real figures. Extend the assertions to check all four KPI cards returned by the page.getByTestId('kpi-card') selector. Apply the same numeric validation logic (using toContainText with a regex pattern for digits and not.toContainText for the dash character '—') that is applied to the filesCard variable to each of the four KPI cards to ensure they all contain real numbers rather than placeholder dashes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f98136f3-2b39-4bac-a601-9169c5a9f075
⛔ Files ignored due to path filters (43)
openspec/changes/2026-06-16-arius-web-claude-design-handoff/assets/arius-iceberg.svgis excluded by!**/*.svgopenspec/changes/2026-06-16-arius-web-claude-design-handoff/assets/keenicons/duotone/fonts/keenicons-duotone.ttfis excluded by!**/*.ttfopenspec/changes/2026-06-16-arius-web-claude-design-handoff/assets/keenicons/duotone/fonts/keenicons-duotone.woffis excluded by!**/*.woffopenspec/changes/2026-06-16-arius-web-claude-design-handoff/assets/keenicons/filled/fonts/keenicons-filled.ttfis excluded by!**/*.ttfopenspec/changes/2026-06-16-arius-web-claude-design-handoff/assets/keenicons/filled/fonts/keenicons-filled.woffis excluded by!**/*.woffopenspec/changes/2026-06-16-arius-web-claude-design-handoff/assets/keenicons/outline/fonts/keenicons-outline.ttfis excluded by!**/*.ttfopenspec/changes/2026-06-16-arius-web-claude-design-handoff/assets/keenicons/outline/fonts/keenicons-outline.woffis excluded by!**/*.woffsrc/Arius.Web/package-lock.jsonis excluded by!**/package-lock.jsonsrc/Arius.Web/public/assets/media/app/apple-touch-icon.pngis excluded by!**/*.pngsrc/Arius.Web/public/assets/media/app/default-logo-dark.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/media/app/default-logo.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/media/app/favicon-16x16.pngis excluded by!**/*.pngsrc/Arius.Web/public/assets/media/app/favicon-32x32.pngis excluded by!**/*.pngsrc/Arius.Web/public/assets/media/app/favicon.icois excluded by!**/*.icosrc/Arius.Web/public/assets/media/app/mini-logo-circle-dark.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/media/app/mini-logo-circle-primary-dark.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/media/app/mini-logo-circle-primary.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/media/app/mini-logo-circle-success.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/media/app/mini-logo-circle.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/media/app/mini-logo-gray-dark.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/media/app/mini-logo-gray.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/media/app/mini-logo-primary-dark.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/media/app/mini-logo-primary.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/media/app/mini-logo-square-gray-dark.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/media/app/mini-logo-square-gray.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/media/app/mini-logo.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/media/app/og-image.pngis excluded by!**/*.pngsrc/Arius.Web/public/assets/media/arius-iceberg.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/vendors/apexcharts/apexcharts.min.jsis excluded by!**/*.min.jssrc/Arius.Web/public/assets/vendors/keenicons/fonts/keenicons-duotone.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/vendors/keenicons/fonts/keenicons-duotone.ttfis excluded by!**/*.ttfsrc/Arius.Web/public/assets/vendors/keenicons/fonts/keenicons-duotone.woffis excluded by!**/*.woffsrc/Arius.Web/public/assets/vendors/keenicons/fonts/keenicons-filled.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/vendors/keenicons/fonts/keenicons-filled.ttfis excluded by!**/*.ttfsrc/Arius.Web/public/assets/vendors/keenicons/fonts/keenicons-filled.woffis excluded by!**/*.woffsrc/Arius.Web/public/assets/vendors/keenicons/fonts/keenicons-outline.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/vendors/keenicons/fonts/keenicons-outline.ttfis excluded by!**/*.ttfsrc/Arius.Web/public/assets/vendors/keenicons/fonts/keenicons-outline.woffis excluded by!**/*.woffsrc/Arius.Web/public/assets/vendors/keenicons/fonts/keenicons-solid.svgis excluded by!**/*.svgsrc/Arius.Web/public/assets/vendors/keenicons/fonts/keenicons-solid.ttfis excluded by!**/*.ttfsrc/Arius.Web/public/assets/vendors/keenicons/fonts/keenicons-solid.woffis excluded by!**/*.woffsrc/Arius.Web/public/assets/vendors/ktui/ktui.min.jsis excluded by!**/*.min.jssrc/Arius.Web/public/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (114)
.dockerignoreAGENTS.mdDockerfiledocker-compose.ymlopenspec/changes/2026-06-16-arius-web-claude-design-handoff/Arius.Web.dc.htmlopenspec/changes/2026-06-16-arius-web-claude-design-handoff/EXECUTION.mdopenspec/changes/2026-06-16-arius-web-claude-design-handoff/PLAN.mdopenspec/changes/2026-06-16-arius-web-claude-design-handoff/PLAYWRIGHT-PLAN.mdopenspec/changes/2026-06-16-arius-web-claude-design-handoff/README.mdopenspec/changes/2026-06-16-arius-web-claude-design-handoff/Shells.dc.htmlopenspec/changes/2026-06-16-arius-web-claude-design-handoff/assets/keenicons/duotone/style.cssopenspec/changes/2026-06-16-arius-web-claude-design-handoff/assets/keenicons/filled/style.cssopenspec/changes/2026-06-16-arius-web-claude-design-handoff/assets/keenicons/outline/style.cssopenspec/changes/2026-06-16-arius-web-claude-design-handoff/support.jssrc/Arius.Api/.gitignoresrc/Arius.Api/AppData/AppDatabase.cssrc/Arius.Api/AppData/Records.cssrc/Arius.Api/AppData/SecretProtector.cssrc/Arius.Api/Arius.Api.csprojsrc/Arius.Api/Composition/RepositoryProviderRegistry.cssrc/Arius.Api/Contracts/Dtos.cssrc/Arius.Api/Contracts/EntryDto.cssrc/Arius.Api/Endpoints/AccountEndpoints.cssrc/Arius.Api/Endpoints/BrowseEndpoints.cssrc/Arius.Api/Endpoints/JobEndpoints.cssrc/Arius.Api/Endpoints/RepositoryEndpoints.cssrc/Arius.Api/Hubs/ArchiveForwarders.cssrc/Arius.Api/Hubs/JobsHub.cssrc/Arius.Api/Hubs/RestoreForwarders.cssrc/Arius.Api/Jobs/JobFormat.cssrc/Arius.Api/Jobs/JobRunner.cssrc/Arius.Api/Jobs/JobSink.cssrc/Arius.Api/Jobs/RestoreApprovalRegistry.cssrc/Arius.Api/Jobs/SchedulerService.cssrc/Arius.Api/Program.cssrc/Arius.Api/Properties/launchSettings.jsonsrc/Arius.Api/appsettings.jsonsrc/Arius.Core.Tests/Features/SnapshotsQuery/SnapshotsQueryHandlerTests.cssrc/Arius.Core.Tests/Features/StatsQuery/StatsQueryHandlerTests.cssrc/Arius.Core.Tests/Shared/Snapshot/Fakes/FakeSnapshotService.cssrc/Arius.Core/Features/SnapshotsQuery/SnapshotsQuery.cssrc/Arius.Core/Features/StatsQuery/StatsQuery.cssrc/Arius.Core/ServiceCollectionExtensions.cssrc/Arius.Core/Shared/ChunkIndex/ChunkIndexLocalStore.cssrc/Arius.Core/Shared/ChunkIndex/ChunkIndexService.cssrc/Arius.Core/Shared/ChunkIndex/IChunkIndexService.cssrc/Arius.Core/Shared/Snapshot/SnapshotService.cssrc/Arius.Web/.editorconfigsrc/Arius.Web/.gitignoresrc/Arius.Web/.postcssrc.jsonsrc/Arius.Web/README.mdsrc/Arius.Web/angular.jsonsrc/Arius.Web/e2e/.env.examplesrc/Arius.Web/e2e/specs/add-wizard.spec.tssrc/Arius.Web/e2e/specs/archive.spec.tssrc/Arius.Web/e2e/specs/cost-approval.spec.tssrc/Arius.Web/e2e/specs/create-wizard.spec.tssrc/Arius.Web/e2e/specs/files.spec.tssrc/Arius.Web/e2e/specs/jobs.spec.tssrc/Arius.Web/e2e/specs/overview.spec.tssrc/Arius.Web/e2e/specs/properties.spec.tssrc/Arius.Web/e2e/specs/restore-roundtrip.spec.tssrc/Arius.Web/e2e/specs/restore.spec.tssrc/Arius.Web/e2e/specs/search.spec.tssrc/Arius.Web/e2e/specs/shell.spec.tssrc/Arius.Web/e2e/specs/statistics.spec.tssrc/Arius.Web/e2e/specs/time-travel.spec.tssrc/Arius.Web/e2e/support/fixtures.tssrc/Arius.Web/e2e/support/global-setup.tssrc/Arius.Web/package.jsonsrc/Arius.Web/playwright.config.tssrc/Arius.Web/proxy.conf.jsonsrc/Arius.Web/public/assets/css/styles.csssrc/Arius.Web/public/assets/js/core.bundle.jssrc/Arius.Web/public/assets/vendors/apexcharts/apexcharts.csssrc/Arius.Web/public/assets/vendors/keenicons/styles.bundle.csssrc/Arius.Web/src/app/app.component.tssrc/Arius.Web/src/app/app.config.tssrc/Arius.Web/src/app/app.routes.tssrc/Arius.Web/src/app/core/api/api-models.tssrc/Arius.Web/src/app/core/api/api.service.tssrc/Arius.Web/src/app/core/api/realtime.service.tssrc/Arius.Web/src/app/core/ktui/kt-init.directive.tssrc/Arius.Web/src/app/core/services/metronic-init.service.tssrc/Arius.Web/src/app/core/state/drawer.store.tssrc/Arius.Web/src/app/core/state/search.store.tssrc/Arius.Web/src/app/features/drawer/archive-restore-drawer.component.tssrc/Arius.Web/src/app/features/jobs/jobs.component.tssrc/Arius.Web/src/app/features/overview/overview.component.tssrc/Arius.Web/src/app/features/placeholder.component.tssrc/Arius.Web/src/app/features/repo/files/files-tab.component.tssrc/Arius.Web/src/app/features/repo/properties/properties-tab.component.tssrc/Arius.Web/src/app/features/repo/repo-detail.component.tssrc/Arius.Web/src/app/features/repo/statistics/statistics-tab.component.tssrc/Arius.Web/src/app/features/repos/repos.component.tssrc/Arius.Web/src/app/features/search/global-search-overlay.component.tssrc/Arius.Web/src/app/features/settings/settings.component.tssrc/Arius.Web/src/app/features/wizards/add/add-repo-wizard.component.tssrc/Arius.Web/src/app/features/wizards/create/create-repo-wizard.component.tssrc/Arius.Web/src/app/shared/format.tssrc/Arius.Web/src/app/shared/live-console/live-console.component.tssrc/Arius.Web/src/app/shared/state-legend/state-legend.component.tssrc/Arius.Web/src/app/shared/state-ring/repository-entry-state.tssrc/Arius.Web/src/app/shared/state-ring/state-ring.component.tssrc/Arius.Web/src/index.htmlsrc/Arius.Web/src/main.tssrc/Arius.Web/src/styles.scsssrc/Arius.Web/src/tailwind.csssrc/Arius.Web/tailwind.config.jssrc/Arius.Web/tsconfig.app.jsonsrc/Arius.Web/tsconfig.jsonsrc/Arius.Web/tsconfig.spec.jsonsrc/Arius.slnxsrc/Directory.Packages.props
…rius.Web README The Dockerfile/docker-compose (repo root) weren't documented anywhere. Adds a Docker section (build, plain `docker run`, and `docker compose up -d --build`, plus the in-container paths) and a "Local state" table showing where the app SQLite, Data-Protection keys, Arius.Core caches and the default restore destination live when running the API locally. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a Playwright e2e job to ci.yml (PRs + master pushes + release tags): sets up .NET 10 + Node 22, installs the chromium browser, and runs the live full-stack suite with ARIUS_E2E_WRITE=1, uploading the HTML report as an artifact. Add a docker job to release.yml that builds the root Dockerfile and pushes woutervanranst/ariusweb:<version> + :latest (linux/amd64) on v* tags; the GitHub Release now also needs the image push to succeed. Centralize the @Write scratch container naming in e2e/support/scratch.ts: SCRATCH_PREFIX defaults to <ARIUS_E2E_CONTAINER>-e2e-arius- so CI scratch containers are scoped to the configured source container. global-setup, fixtures, and the three @Write specs all derive from it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The branch added the SnapshotsQuery and StatsQuery features (registered in Arius.Core's AddArius). Mediator's source generator resolves every command handler when a command is sent, so CliHarness — which builds a minimal provider with only mocked handlers — must supply these two as well, otherwise every CLI invocation fails activating SnapshotsQueryHandler (needs a real ISnapshotService the harness has no reason to wire up). All 149 Arius.Cli.Tests pass again. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The read-only Playwright specs (files, statistics, search, time-travel) assert against a populated repository. In CI the source container starts empty, so global-setup now archives a small fixture tree (a root file + a docs/ subfolder) into it via the JobsHub StartArchive method when no snapshot exists yet, then waits for the archive job to complete. Idempotent — a repo that already has a snapshot (reused dev DB, or a prior CI run against the same container) is left untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…shots When the source container doesn't exist yet, the read-only snapshots endpoint returns 500 (ContainerNotFound), which broke the seed pre-check on res.json(). Guard on res.ok() and treat any non-OK/non-array response as "no snapshot" — the seed archive's ReadWrite preflight then creates the container. Verified locally end-to-end against a fresh container (16 passed, @Write skipped). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- stats: drop IsPending/Pending; GetStatsAsync stays eager so the response blocks until figures are final (no partial "pending" state) - restore: decline (cancel) a parked cost approval on connection drop and on drawer close, so a never-answered modal can't hold the per-repo write lock - repo tabs: reload via effect() keyed on repoId (router reuses the component across /repos/:id navigations) - realtime: drop the cached start promise on failure so a later call can retry - search.store: clear debounce timer + loading on close - jobs: dispose the log$ subscription with takeUntilDestroyed() - overview: refresh() is just location.reload() (removed dead navigation) - JobSink: reuse JobFormat.Bytes instead of a duplicate formatter - global-search: collision-safe @for track key - e2e: fail fast on fixture API errors; assert all rows/cards - docs: README Angular 21 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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)
src/Arius.Core.Tests/Features/StatsQuery/StatsQueryHandlerTests.cs (1)
37-37:⚠️ Potential issue | 🟡 MinorReplace
NullLogger<T>withFakeLogger<T>in test projects.Test projects should use
FakeLogger<T>fromMicrosoft.Extensions.Diagnostics.Testinginstead ofNullLogger<T>to keep logs assertable and inspectable. This applies at lines 37 and 52.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Core.Tests/Features/StatsQuery/StatsQueryHandlerTests.cs` at line 37, In the StatsQueryHandlerTests class, replace the NullLogger<StatsQueryHandler>.Instance with FakeLogger<StatsQueryHandler>.Instance in the StatsQueryHandler constructor calls. First, ensure the using statement includes Microsoft.Extensions.Diagnostics.Testing to import FakeLogger. Update all occurrences where NullLogger<StatsQueryHandler> is instantiated to use FakeLogger<StatsQueryHandler> instead, which will allow test assertions and inspection of logged messages.Source: Coding guidelines
♻️ Duplicate comments (1)
src/Arius.Api/Jobs/RestoreApprovalRegistry.cs (1)
18-19:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRegister can still wait forever for a connected-but-idle client.
Line 18 still has no timeout/cancellation integration; disconnect handling alone doesn’t cover users who keep the socket open and never answer. That can still park restore progress indefinitely.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Api/Jobs/RestoreApprovalRegistry.cs` around lines 18 - 19, The Register method returns a Task that can wait indefinitely because there is no timeout or cancellation mechanism integrated into the TaskCompletionSource. When a client connects but remains idle without responding, the task will never complete. Add timeout or cancellation token support to the Register method so that if no response is received within a specified duration, the task completes or throws an exception rather than waiting indefinitely. This ensures restore progress is not blocked by idle connected clients.
🧹 Nitpick comments (2)
.github/workflows/ci.yml (1)
122-123: 💤 Low valueConsider setting
persist-credentials: falsefor security hardening.The static analysis tool flagged that the checkout action doesn't explicitly disable credential persistence. While not a functional issue for read-only E2E tests, setting
persist-credentials: falseis a security best practice to prevent accidental credential leakage through artifacts.🔒 Suggested change
- name: 🛎️ Checkout uses: actions/checkout@v6 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 122 - 123, The actions/checkout@v6 action in the workflow file is missing the persist-credentials security hardening parameter. Add the `persist-credentials: false` option to the checkout action to prevent accidental credential leakage through workflow artifacts, which is a security best practice even for read-only operations.Source: Linters/SAST tools
src/Arius.Web/e2e/support/global-setup.ts (1)
94-102: 💤 Low valueConsider cleanup or documentation for temporary seed directory.
The seed fixture is created in a temporary directory (line 95) and the repo's
localPathis updated to point to it (line 102). After the archive completes, this temp directory may be cleaned up by the OS, leaving the repo record with a stale path.Since read-only E2E specs work with remote snapshots and likely don't rely on
localPath, this shouldn't break functionality. However, it's slightly messy. Consider either:
- Clearing or resetting
localPathafter the archive job completes- Adding a comment explaining why the stale path is acceptable
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Web/e2e/support/global-setup.ts` around lines 94 - 102, The temporary seed directory created by mkdtempSync in the global-setup function may be cleaned up by the OS after the archive completes, leaving the repo's localPath pointing to a non-existent path. Address this by either resetting the repo's localPath back to null or empty after the archive job completes (by making another PATCH request to clear the localPath field), or add a clarifying comment in the code explaining that the stale path is acceptable since the E2E specs work with remote snapshots and don't depend on the localPath value being valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Arius.Api/Jobs/RestoreApprovalRegistry.cs`:
- Around line 21-22: The Track method is storing job ownership for all restore
jobs at the start, but many jobs never request approval and accumulate as stale
entries in _ownerByJob, causing unbounded memory growth. Move the ownership
tracking logic from the Track method to the Register method instead, so that job
ownership is only recorded when approval is actually requested rather than for
every restore that starts. This ensures only jobs that genuinely need approval
tracking consume memory in the dictionary.
- Line 12: Change the visibility modifier of the RestoreApprovalRegistry class
from public to internal since it is only consumed within the same assembly and
does not need to be part of the public API surface. Replace the public keyword
with internal in the class declaration to align with the coding guidelines that
recommend making non-test classes internal unless they are explicitly consumed
by another non-test assembly.
In `@src/Arius.Web/src/app/features/repo/properties/properties-tab.component.ts`:
- Around line 89-93: The effect that runs when repoId changes is not cancelling
previous subscriptions, causing a race condition where old HTTP requests from
getRepository() can complete and overwrite fresh data after new requests finish.
Fix this by returning a cleanup function from the effect that unsubscribes both
the getRepository() subscription and any subscriptions created by
loadSchedules(). Store the subscription returned by getRepository().subscribe()
in a variable and the subscriptions from loadSchedules() (or inline its logic),
then return an unsubscribe callback that cleans up both to prevent stale data
from overwriting the current repository state.
In `@src/Arius.Web/src/app/features/repo/statistics/statistics-tab.component.ts`:
- Around line 41-45: The effect function needs to return a cleanup function to
properly cancel the HTTP subscription from api.getStats(id) when the effect
re-runs due to repoId changes or when the component is destroyed. Store the
subscription returned by the subscribe() method call, then return an arrow
function from the effect that calls unsubscribe() on that stored subscription.
This ensures old requests are cancelled before new ones are made, preventing
stale data from overwriting fresh stats and eliminating the memory leak.
---
Outside diff comments:
In `@src/Arius.Core.Tests/Features/StatsQuery/StatsQueryHandlerTests.cs`:
- Line 37: In the StatsQueryHandlerTests class, replace the
NullLogger<StatsQueryHandler>.Instance with
FakeLogger<StatsQueryHandler>.Instance in the StatsQueryHandler constructor
calls. First, ensure the using statement includes
Microsoft.Extensions.Diagnostics.Testing to import FakeLogger. Update all
occurrences where NullLogger<StatsQueryHandler> is instantiated to use
FakeLogger<StatsQueryHandler> instead, which will allow test assertions and
inspection of logged messages.
---
Duplicate comments:
In `@src/Arius.Api/Jobs/RestoreApprovalRegistry.cs`:
- Around line 18-19: The Register method returns a Task that can wait
indefinitely because there is no timeout or cancellation mechanism integrated
into the TaskCompletionSource. When a client connects but remains idle without
responding, the task will never complete. Add timeout or cancellation token
support to the Register method so that if no response is received within a
specified duration, the task completes or throws an exception rather than
waiting indefinitely. This ensures restore progress is not blocked by idle
connected clients.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 122-123: The actions/checkout@v6 action in the workflow file is
missing the persist-credentials security hardening parameter. Add the
`persist-credentials: false` option to the checkout action to prevent accidental
credential leakage through workflow artifacts, which is a security best practice
even for read-only operations.
In `@src/Arius.Web/e2e/support/global-setup.ts`:
- Around line 94-102: The temporary seed directory created by mkdtempSync in the
global-setup function may be cleaned up by the OS after the archive completes,
leaving the repo's localPath pointing to a non-existent path. Address this by
either resetting the repo's localPath back to null or empty after the archive
job completes (by making another PATCH request to clear the localPath field), or
add a clarifying comment in the code explaining that the stale path is
acceptable since the E2E specs work with remote snapshots and don't depend on
the localPath value being valid.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1152bb38-2edf-456b-b9f7-25a9a69a34b9
📒 Files selected for processing (28)
.github/workflows/ci.yml.github/workflows/release.ymlsrc/Arius.Api/Contracts/Dtos.cssrc/Arius.Api/Endpoints/BrowseEndpoints.cssrc/Arius.Api/Hubs/JobsHub.cssrc/Arius.Api/Jobs/JobSink.cssrc/Arius.Api/Jobs/RestoreApprovalRegistry.cssrc/Arius.Cli.Tests/TestSupport/CliHarness.cssrc/Arius.Core.Tests/Features/StatsQuery/StatsQueryHandlerTests.cssrc/Arius.Core/Features/StatsQuery/StatsQuery.cssrc/Arius.Web/README.mdsrc/Arius.Web/e2e/specs/archive.spec.tssrc/Arius.Web/e2e/specs/cost-approval.spec.tssrc/Arius.Web/e2e/specs/files.spec.tssrc/Arius.Web/e2e/specs/restore-roundtrip.spec.tssrc/Arius.Web/e2e/specs/statistics.spec.tssrc/Arius.Web/e2e/support/fixtures.tssrc/Arius.Web/e2e/support/global-setup.tssrc/Arius.Web/e2e/support/scratch.tssrc/Arius.Web/src/app/core/api/api-models.tssrc/Arius.Web/src/app/core/api/realtime.service.tssrc/Arius.Web/src/app/core/state/drawer.store.tssrc/Arius.Web/src/app/core/state/search.store.tssrc/Arius.Web/src/app/features/jobs/jobs.component.tssrc/Arius.Web/src/app/features/overview/overview.component.tssrc/Arius.Web/src/app/features/repo/properties/properties-tab.component.tssrc/Arius.Web/src/app/features/repo/statistics/statistics-tab.component.tssrc/Arius.Web/src/app/features/search/global-search-overlay.component.ts
💤 Files with no reviewable changes (1)
- src/Arius.Web/src/app/core/api/api-models.ts
✅ Files skipped from review due to trivial changes (1)
- src/Arius.Web/README.md
🚧 Files skipped from review as they are similar to previous changes (15)
- src/Arius.Web/e2e/specs/archive.spec.ts
- src/Arius.Web/e2e/specs/statistics.spec.ts
- src/Arius.Api/Endpoints/BrowseEndpoints.cs
- src/Arius.Api/Contracts/Dtos.cs
- src/Arius.Web/e2e/specs/files.spec.ts
- src/Arius.Web/src/app/features/jobs/jobs.component.ts
- src/Arius.Web/src/app/core/state/search.store.ts
- src/Arius.Web/src/app/core/state/drawer.store.ts
- src/Arius.Web/e2e/support/fixtures.ts
- src/Arius.Web/src/app/features/search/global-search-overlay.component.ts
- src/Arius.Web/e2e/specs/cost-approval.spec.ts
- src/Arius.Api/Jobs/JobSink.cs
- src/Arius.Api/Hubs/JobsHub.cs
- src/Arius.Web/e2e/specs/restore-roundtrip.spec.ts
- src/Arius.Web/src/app/core/api/realtime.service.ts
… tier The master merge (#114 dynamic shard prefixes) removed EnsurePrefixLoadedAndSynchronizedAsync, which GetStatsAsync still called. Instead of rescanning shard blobs, aggregate straight from the local SQLite cache (the local mirror of the index): dedupe by chunk hash and GROUP BY storage_tier_hint. No blob reads — figures reflect the cache's current coverage (browsed/looked-up entries plus pending archive entries), matching the existing StatsQueryHandlerTests which seed via AddEntry without flushing. - ChunkIndexLocalStore.GetStats(): per-tier distinct-chunk aggregate - IChunkIndexService: GetStatsAsync -> sync GetStats() : IReadOnlyList<ChunkTierStat> - RepositoryStats / StatsDto gain StoredByTier (TierStatDto); totals summed from it - tests: dedup aggregate + multi-tier split Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The chunk-index tier-split refactor added a required StoredByTier parameter to RepositoryStats; the CLI harness mock still used the 4-arg constructor, breaking the build on all three OS runners. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The breakdown had no coverage. Add a Karma component spec (the only place multi-tier rendering can be exercised — the live e2e seed archives a single tier) verifying one row per StatsDto.storedByTier entry, in order, with the correct labels/chunk counts, plus the empty-state gate that hides the panel. Strengthen the e2e statistics spec to warm the local chunk-index cache via the Files tab first (stats now read the cache only, no blob reads), then assert the tier breakdown renders with at least one known-tier row. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ius7 into arius.explorer
… summary Add @estruyf/github-actions-reporter, enabled only under GITHUB_ACTIONS, so the Playwright e2e job posts a per-spec pass/skip/fail summary to the run summary — mirroring what the .NET Test & Coverage jobs already show. Local `npm run e2e` is unaffected (reporter is gated on the CI env var). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Arius.Api/Jobs/RestoreApprovalRegistry.cs`:
- Around line 22-25: The race condition occurs because CancelForConnection only
cancels jobs already recorded in _ownerByJob, but ownership is not recorded
until Register is called. If the socket disconnects before Register executes,
the subsequent await will block indefinitely. To fix this, maintain a separate
collection to track disconnected connections, and in the Register method, check
if the provided connectionId has already been disconnected - if it has,
immediately return a completed task with null instead of creating a new pending
TaskCompletionSource. Additionally, update CancelForConnection to mark
connections as disconnected before attempting to cancel their jobs, ensuring
that any late calls to Register for that connection will detect the
disconnection state and return null deterministically.
In `@src/Arius.Core.Tests/Features/StatisticsQuery/StatsQueryHandlerTests.cs`:
- Around line 79-83: The tier assertions in the tier-split test are incomplete
because they only verify StoredByTier stored sizes but not the per-tier unique
chunk counts, which can mask aggregation bugs. Add assertions to check the
ChunkCount property for each tier entry (BlobTier.Cool and BlobTier.Archive)
alongside the existing StoredSize assertions. Additionally, in the no-snapshot
test section around lines 95-99, add an assertion to verify that
stats.StoredByTier is empty or has a count of zero, since the comment indicates
this test should ensure StoredByTier remains empty when there is no snapshot
data.
In `@src/Arius.Core/Shared/ChunkIndex/ChunkIndexService.cs`:
- Around line 64-70: Add validation for the maxShardEntryCount parameter in the
ChunkIndexService constructor to ensure it is strictly positive. Before
assigning the value to the _maxShardEntryCount field, check that
maxShardEntryCount is greater than zero and throw an appropriate exception (such
as ArgumentException) if it is zero or negative, with a descriptive message
explaining that maxShardEntryCount must be a positive integer.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: adcb7206-25f3-45bc-bc43-1c3f3d6a01e6
⛔ Files ignored due to path filters (1)
src/Arius.Web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (22)
AGENTS.mdsrc/Arius.Api/Contracts/Dtos.cssrc/Arius.Api/Endpoints/BrowseEndpoints.cssrc/Arius.Api/Hubs/JobsHub.cssrc/Arius.Api/Jobs/JobRunner.cssrc/Arius.Api/Jobs/RestoreApprovalRegistry.cssrc/Arius.Cli.Tests/TestSupport/CliHarness.cssrc/Arius.Core.Tests/Features/StatisticsQuery/StatsQueryHandlerTests.cssrc/Arius.Core/Features/StatisticsQuery/StatsQuery.cssrc/Arius.Core/ServiceCollectionExtensions.cssrc/Arius.Core/Shared/ChunkIndex/ChunkIndexLocalStore.cssrc/Arius.Core/Shared/ChunkIndex/ChunkIndexService.cssrc/Arius.Core/Shared/ChunkIndex/IChunkIndexService.cssrc/Arius.Web/e2e/specs/statistics.spec.tssrc/Arius.Web/package.jsonsrc/Arius.Web/playwright.config.tssrc/Arius.Web/src/app/core/api/api-models.tssrc/Arius.Web/src/app/core/api/api.service.tssrc/Arius.Web/src/app/features/repo/properties/properties-tab.component.tssrc/Arius.Web/src/app/features/repo/statistics/statistics-tab.component.spec.tssrc/Arius.Web/src/app/features/repo/statistics/statistics-tab.component.tssrc/Directory.Packages.props
✅ Files skipped from review due to trivial changes (2)
- src/Arius.Core/Features/StatisticsQuery/StatsQuery.cs
- src/Directory.Packages.props
🚧 Files skipped from review as they are similar to previous changes (8)
- src/Arius.Core/ServiceCollectionExtensions.cs
- src/Arius.Web/package.json
- src/Arius.Api/Endpoints/BrowseEndpoints.cs
- src/Arius.Cli.Tests/TestSupport/CliHarness.cs
- src/Arius.Web/playwright.config.ts
- src/Arius.Web/src/app/core/api/api.service.ts
- src/Arius.Web/src/app/core/api/api-models.ts
- src/Arius.Api/Hubs/JobsHub.cs
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Arius.Api/Jobs/RestoreApprovalRegistry.cs`:
- Around line 22-25: The race condition occurs because CancelForConnection only
cancels jobs already recorded in _ownerByJob, but ownership is not recorded
until Register is called. If the socket disconnects before Register executes,
the subsequent await will block indefinitely. To fix this, maintain a separate
collection to track disconnected connections, and in the Register method, check
if the provided connectionId has already been disconnected - if it has,
immediately return a completed task with null instead of creating a new pending
TaskCompletionSource. Additionally, update CancelForConnection to mark
connections as disconnected before attempting to cancel their jobs, ensuring
that any late calls to Register for that connection will detect the
disconnection state and return null deterministically.
In `@src/Arius.Core.Tests/Features/StatisticsQuery/StatsQueryHandlerTests.cs`:
- Around line 79-83: The tier assertions in the tier-split test are incomplete
because they only verify StoredByTier stored sizes but not the per-tier unique
chunk counts, which can mask aggregation bugs. Add assertions to check the
ChunkCount property for each tier entry (BlobTier.Cool and BlobTier.Archive)
alongside the existing StoredSize assertions. Additionally, in the no-snapshot
test section around lines 95-99, add an assertion to verify that
stats.StoredByTier is empty or has a count of zero, since the comment indicates
this test should ensure StoredByTier remains empty when there is no snapshot
data.
In `@src/Arius.Core/Shared/ChunkIndex/ChunkIndexService.cs`:
- Around line 64-70: Add validation for the maxShardEntryCount parameter in the
ChunkIndexService constructor to ensure it is strictly positive. Before
assigning the value to the _maxShardEntryCount field, check that
maxShardEntryCount is greater than zero and throw an appropriate exception (such
as ArgumentException) if it is zero or negative, with a descriptive message
explaining that maxShardEntryCount must be a positive integer.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: adcb7206-25f3-45bc-bc43-1c3f3d6a01e6
⛔ Files ignored due to path filters (1)
src/Arius.Web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (22)
AGENTS.mdsrc/Arius.Api/Contracts/Dtos.cssrc/Arius.Api/Endpoints/BrowseEndpoints.cssrc/Arius.Api/Hubs/JobsHub.cssrc/Arius.Api/Jobs/JobRunner.cssrc/Arius.Api/Jobs/RestoreApprovalRegistry.cssrc/Arius.Cli.Tests/TestSupport/CliHarness.cssrc/Arius.Core.Tests/Features/StatisticsQuery/StatsQueryHandlerTests.cssrc/Arius.Core/Features/StatisticsQuery/StatsQuery.cssrc/Arius.Core/ServiceCollectionExtensions.cssrc/Arius.Core/Shared/ChunkIndex/ChunkIndexLocalStore.cssrc/Arius.Core/Shared/ChunkIndex/ChunkIndexService.cssrc/Arius.Core/Shared/ChunkIndex/IChunkIndexService.cssrc/Arius.Web/e2e/specs/statistics.spec.tssrc/Arius.Web/package.jsonsrc/Arius.Web/playwright.config.tssrc/Arius.Web/src/app/core/api/api-models.tssrc/Arius.Web/src/app/core/api/api.service.tssrc/Arius.Web/src/app/features/repo/properties/properties-tab.component.tssrc/Arius.Web/src/app/features/repo/statistics/statistics-tab.component.spec.tssrc/Arius.Web/src/app/features/repo/statistics/statistics-tab.component.tssrc/Directory.Packages.props
✅ Files skipped from review due to trivial changes (2)
- src/Arius.Core/Features/StatisticsQuery/StatsQuery.cs
- src/Directory.Packages.props
🚧 Files skipped from review as they are similar to previous changes (8)
- src/Arius.Core/ServiceCollectionExtensions.cs
- src/Arius.Web/package.json
- src/Arius.Api/Endpoints/BrowseEndpoints.cs
- src/Arius.Cli.Tests/TestSupport/CliHarness.cs
- src/Arius.Web/playwright.config.ts
- src/Arius.Web/src/app/core/api/api.service.ts
- src/Arius.Web/src/app/core/api/api-models.ts
- src/Arius.Api/Hubs/JobsHub.cs
🛑 Comments failed to post (3)
src/Arius.Api/Jobs/RestoreApprovalRegistry.cs (1)
22-25:
⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDisconnect-before-register race can block restore indefinitely.
CancelForConnection(Line 37-41) only resolves jobs already present in_ownerByJob, but ownership is recorded only atRegister(Line 24). If the socket disconnects beforeRegisteris reached, the later await can stay parked forever, which also holds the repository lock.Please make disconnect state observable to
Register(or reintroduce early owner tracking with explicit completion cleanup) so late registration on a dead connection deterministically returnsnullinstead of waiting forever.Also applies to: 37-41
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Api/Jobs/RestoreApprovalRegistry.cs` around lines 22 - 25, The race condition occurs because CancelForConnection only cancels jobs already recorded in _ownerByJob, but ownership is not recorded until Register is called. If the socket disconnects before Register executes, the subsequent await will block indefinitely. To fix this, maintain a separate collection to track disconnected connections, and in the Register method, check if the provided connectionId has already been disconnected - if it has, immediately return a completed task with null instead of creating a new pending TaskCompletionSource. Additionally, update CancelForConnection to mark connections as disconnected before attempting to cancel their jobs, ensuring that any late calls to Register for that connection will detect the disconnection state and return null deterministically.src/Arius.Core.Tests/Features/StatisticsQuery/StatsQueryHandlerTests.cs (1)
79-83:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStrengthen tier assertions to prevent silent aggregation regressions.
The tier-split test currently checks tier stored sizes but not per-tier unique chunk counts, and the no-snapshot test does not assert
StoredByTieris empty. That leaves room for regressions without test failure.Suggested assertion additions
stats.StoredByTier.Count.ShouldBe(2); stats.StoredByTier[0].Tier.ShouldBe(BlobTier.Cool); + stats.StoredByTier[0].UniqueChunks.ShouldBe(1); stats.StoredByTier[0].StoredSize.ShouldBe(40); stats.StoredByTier[1].Tier.ShouldBe(BlobTier.Archive); + stats.StoredByTier[1].UniqueChunks.ShouldBe(1); stats.StoredByTier[1].StoredSize.ShouldBe(60); @@ stats.Files.ShouldBe(0); stats.OriginalSize.ShouldBe(0); stats.UniqueChunks.ShouldBe(0); stats.StoredSize.ShouldBe(0); + stats.StoredByTier.ShouldBeEmpty();As per coding guidelines,
**/*.Tests/**: “Focus on test coverage gaps and assertion quality rather than style.”Also applies to: 95-99
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Core.Tests/Features/StatisticsQuery/StatsQueryHandlerTests.cs` around lines 79 - 83, The tier assertions in the tier-split test are incomplete because they only verify StoredByTier stored sizes but not the per-tier unique chunk counts, which can mask aggregation bugs. Add assertions to check the ChunkCount property for each tier entry (BlobTier.Cool and BlobTier.Archive) alongside the existing StoredSize assertions. Additionally, in the no-snapshot test section around lines 95-99, add an assertion to verify that stats.StoredByTier is empty or has a count of zero, since the comment indicates this test should ensure StoredByTier remains empty when there is no snapshot data.Source: Coding guidelines
src/Arius.Core/Shared/ChunkIndex/ChunkIndexService.cs (1)
64-70:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard
maxShardEntryCountagainst invalid values.Line 64 accepts any integer, but split/repair paths assume a strictly positive threshold. Passing
0or negative can lead to pathological recursion/splitting behavior instead of a fast failure.Suggested minimal fix
public ChunkIndexService( IBlobContainerService blobs, IEncryptionService encryption, ICompressionService compression, ISnapshotService snapshotService, string accountName, string containerName, ILoggerFactory? loggerFactory = null, int maxShardEntryCount = MaxShardEntryCount) { + if (maxShardEntryCount < 1) + throw new ArgumentOutOfRangeException(nameof(maxShardEntryCount), "Value must be greater than zero."); + _blobs = blobs; _encryption = encryption; _compression = compression; _maxShardEntryCount = maxShardEntryCount;📝 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.public ChunkIndexService( IBlobContainerService blobs, IEncryptionService encryption, ICompressionService compression, ISnapshotService snapshotService, string accountName, string containerName, ILoggerFactory? loggerFactory = null, int maxShardEntryCount = MaxShardEntryCount) { if (maxShardEntryCount < 1) throw new ArgumentOutOfRangeException(nameof(maxShardEntryCount), "Value must be greater than zero."); _blobs = blobs; _encryption = encryption; _compression = compression; _maxShardEntryCount = maxShardEntryCount; _logger = loggerFactory?.CreateLogger<ChunkIndexService>() ?? NullLogger<ChunkIndexService>.Instance;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Arius.Core/Shared/ChunkIndex/ChunkIndexService.cs` around lines 64 - 70, Add validation for the maxShardEntryCount parameter in the ChunkIndexService constructor to ensure it is strictly positive. Before assigning the value to the _maxShardEntryCount field, check that maxShardEntryCount is greater than zero and throw an appropriate exception (such as ArgumentException) if it is zero or negative, with a descriptive message explaining that maxShardEntryCount must be a positive integer.
Summary by CodeRabbit
New Features
Infrastructure
Documentation