Skip to content

PM-41429: Chore: Decouple dependencies - #7238

Merged
david-livefront merged 1 commit into
mainfrom
PM-41429-decouple-dependencies
Aug 4, 2026
Merged

PM-41429: Chore: Decouple dependencies#7238
david-livefront merged 1 commit into
mainfrom
PM-41429-decouple-dependencies

Conversation

@david-livefront

@david-livefront david-livefront commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

🎟️ Tracking

PM-41429

📔 Objective

This PR decouples the FlightRecorderWriter and FileManager from the network layer. This will allow the Flight Recorder to be instantiated much sooner in the app lifecycle.

@david-livefront
david-livefront requested a review from a team as a code owner August 3, 2026 18:18
@david-livefront david-livefront changed the title Pm 41429 decouple dependencies PM-41429: Chore: Decouple dependencies Aug 3, 2026
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context t:tech-debt Change Type - Tech debt labels Aug 3, 2026
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed this tech-debt refactor that decouples FileManager and FlightRecorderWriter from the network layer. FileManager.downloadFileToCache(url) is replaced by streamFileToCache(stream): Result<File>, with the DownloadService call moved up into CipherManagerImpl.downloadAttachment, and FlightRecorderWriterImpl now reads server config from ConfigDiskSource instead of ServerConfigRepository. The UuidManager provider moves from the authenticator's PlatformManagerModule to CoreManagerModule, and the now-unused DownloadResult model is deleted. Verified no leftover downloadFileToCache/DownloadResult references, a single UuidManager binding in the graph, and that ConfigDiskSource is available in both app graphs via the shared DiskModule.

Code Review Details

No findings.

Notes from validation (no action required):

  • The ConfigDiskSource swap in FlightRecorderWriterImpl is behavior-preserving: ServerConfigRepositoryImpl.serverConfigStateFlow is a get() property that rebuilt a stateIn from configDiskSource.serverConfig on each access, so reading the disk source directly is equivalent and cheaper.
  • streamFileToCache catching only RuntimeException (letting IOException escape a Result-returning function) and leaving the partial cache file on failure both match the pre-change implementation, so they are pre-existing rather than introduced here.
  • The response body is still closed on the success path via stream.use { } inside streamFileToCache.

@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.33%. Comparing base (b2ba4ec) to head (d8572eb).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7238      +/-   ##
==========================================
- Coverage   85.72%   85.33%   -0.39%     
==========================================
  Files        1053     1112      +59     
  Lines       67779    68345     +566     
  Branches     9826     9849      +23     
==========================================
+ Hits        58101    58321     +220     
- Misses       6166     6514     +348     
+ Partials     3512     3510       -2     
Flag Coverage Δ
app-data 18.00% <54.54%> (-0.26%) ⬇️
app-ui-auth-tools 19.21% <0.00%> (+0.17%) ⬆️
app-ui-platform 17.02% <0.00%> (+0.53%) ⬆️
app-ui-vault 27.83% <0.00%> (+<0.01%) ⬆️
authenticator 6.09% <0.00%> (-0.04%) ⬇️
lib-core-network-bridge 4.10% <0.00%> (-0.02%) ⬇️
lib-data-ui 1.20% <45.45%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.

@david-livefront

Copy link
Copy Markdown
Collaborator Author

Thanks @aj-rosado

@david-livefront
david-livefront added this pull request to the merge queue Aug 4, 2026
Merged via the queue into main with commit 33a8429 Aug 4, 2026
23 of 26 checks passed
@david-livefront
david-livefront deleted the PM-41429-decouple-dependencies branch August 4, 2026 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app:authenticator Bitwarden Authenticator app context app:password-manager Bitwarden Password Manager app context t:tech-debt Change Type - Tech debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants