Skip to content

Seeder progress indicators#7510

Draft
withinfocus wants to merge 4 commits intomainfrom
seeder-progress
Draft

Seeder progress indicators#7510
withinfocus wants to merge 4 commits intomainfrom
seeder-progress

Conversation

@withinfocus
Copy link
Copy Markdown
Contributor

🎟️ Tracking

https://bitwarden.slack.com/archives/C0APGED7DK9/p1776360765504199

Reported based on usage.

📔 Objective

Adds progress indicators to the seeder given that it can take a long time to execute.

📸 Screenshots

image

@withinfocus withinfocus added the ai-review Request a Claude code review label Apr 20, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 20, 2026

Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed progress reporting additions to the seeder pipeline: a new SeederProgressEvent hierarchy with IProgress<T> plumbed through SeederDependencies, a ProgressTicker for batched events in sequential loops, inline batching for the two parallel steps, and a ConsoleProgressReporter that renders to Spectre.Console on stderr. The Spectre.Console 0.55.2 dependency already has AppSec approval documented in-thread. The prior review feedback on TickBy was addressed by removing it. Stdout/stderr split is consistent: progress and banner go to stderr while result rows and preset listings stay on stdout, preserving piping behavior.

Code Review Details

No blocking findings. Progress emission accounting is correct (per-thread local counters in Parallel.For flush in localFinally, sequential loops flush via ProgressTicker.Flush), lock discipline in ConsoleProgressReporter is appropriate for the mutable dictionary, and PhaseCompleted reliably fires after all PhaseAdvanced events because Parallel.For joins before the completion report.

Dependency Changes

Package Change Ecosystem
Spectre.Console New (0.55.2) NuGet

Comment thread util/Seeder/Pipeline/ProgressTicker.cs Outdated
Comment thread util/SeederUtility/SeederUtility.csproj

<ItemGroup>
<PackageReference Include="CommandDotNet" Version="7.0.5" />
<PackageReference Include="Spectre.Console" Version="0.55.2" />
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Security Review: Spectre.Console 0.55.2

Added to: util/SeederUtility/SeederUtility.csproj (dev-only CLI — seeder utility)
Recommendation:Approve

Package metadata

Field Value
Package Spectre.Console 0.55.2 (+ Spectre.Console.Ansi 0.55.2)
Published 2026-04-17 (3 days ago, current)
License MIT
Authors Patrik Svensson, Phil Scott, Nils Andresen, Cédric Luthi
Repo spectreconsole/spectre.console — 11,362 stars, active
Transitive deps on net8.0 None beyond Spectre.Console.Ansi (same authors/repo)

Vulnerability findings

  • GitHub Advisory DB (NuGet ecosystem, affects Spectre.Console): 0 advisories.
  • Repo security advisories: 0.
  • dotnet list package --vulnerable --include-transitive: No vulnerabilities introduced by Spectre.Console. The pre-existing High/Moderate alerts in the SeederUtility tree (AutoMapper 12.0.1, Azure.Identity 1.10.2, Microsoft.Bcl.Memory 10.0.0, System.Security.Cryptography.Xml 8.0.2) all come from Core/Infrastructure/SharedWeb, not from this change.

Health signals

Criterion Result
Maintenance ✅ Release cadence: 0.55.2 (2026-04-17), 0.55.1 hotfix (2026-04-16), 0.55.0 (2026-04-03), 0.54.0 (2025-11-12)
Maintainer count ✅ 4 named authors, 7+ contributors with 10+ commits. Bus factor ≥ 4
Community ✅ 11k stars, widely adopted .NET CLI UI lib
License ✅ MIT — compatible
Vulnerability history ✅ Clean record
Security policy ⚠️ No SECURITY.md in the repo — mild red flag, but nothing hidden in advisories

Attack surface assessment

Usage in this PR is narrow: ConsoleProgressReporter.cs calls AnsiConsole.Progress()ProgressContext.AddTaskProgressTask.Increment/StopTask. Specifically:

  • No markup injection path. Phase labels ("Creating users", "Committing to database", etc.) are hardcoded constants in the library. We do not pass user-controlled strings to AnsiConsole.Markup / MarkupLine / the prompt APIs.
  • No file/network I/O from Spectre in the code paths we use.
  • Tool is dev-only. SeederUtility runs against developer databases; it is not deployed to production or handling customer data.

Suggested follow-ups

If future changes start rendering user-supplied strings (e.g., printing a user-provided --org-name via AnsiConsole.MarkupLine), switch to Markup.Escape(...) to prevent markup injection. Not applicable today.

Bottom line: Clean package, clean transitive tree, narrow usage, dev-only tool. No blockers.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 20, 2026

Logo
Checkmarx One – Scan Summary & Detailsce79e7e7-6f17-40db-b556-ad4c8e099e4b

Great job! No new security vulnerabilities introduced in this pull request

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.13%. Comparing base (26f0702) to head (8c83df6).
⚠️ Report is 14 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7510      +/-   ##
==========================================
+ Coverage   59.10%   59.13%   +0.03%     
==========================================
  Files        2078     2077       -1     
  Lines       91705    91848     +143     
  Branches     8152     8175      +23     
==========================================
+ Hits        54201    54316     +115     
- Misses      35572    35600      +28     
  Partials     1932     1932              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sonarqubecloud
Copy link
Copy Markdown


namespace Bit.Seeder.Pipeline;

internal static class SeederPhases
Copy link
Copy Markdown
Contributor

@theMickster theMickster Apr 23, 2026

Choose a reason for hiding this comment

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

♻️ SeederPhases is here solely to hold CommittingToDatabase, while CreateCiphersStep, CreateCollectionsStep, CreateGroupsStep, CreateUsersStep, and GeneratePersonalCiphersStep each define their own file-local private const string Phase.

Let's put them all into this simple class of constants, move it to it's own file, and use it for phase definitions since multiple patterns invites drift.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants