Skip to content

fix(linux): resolve optional token storage at startup - #301

Open
bobtista wants to merge 2 commits into
developmentfrom
fix/linux-startup-dependencies
Open

fix(linux): resolve optional token storage at startup#301
bobtista wants to merge 2 commits into
developmentfrom
fix/linux-startup-dependencies

Conversation

@bobtista

@bobtista bobtista commented Jul 27, 2026

Copy link
Copy Markdown

Summary

Fix Linux startup composition when GitHub token storage is unavailable.

Changes

  • Resolve IGitHubTokenStorage optionally.
  • Preserve Windows token storage.
  • Add Linux and Windows composition-root tests with isolated temporary paths.

Testing

  • Release solution build passed.
  • All 1,224 tests passed.

These tests validate DI composition, not packaged GUI startup.

Risks and rollback

Low risk; revert this change to restore previous behavior.

Related issues

Fixes #299

Greptile Summary

Fixes cross-platform startup composition by:

  • Resolving GitHub token storage as an optional dependency for the settings view model.
  • Adding Linux composition coverage without token storage.
  • Adding Windows composition coverage that verifies token storage remains registered and usable.
  • Isolating process-wide environment changes in nonparallel test collections.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
GenHub/GenHub/Infrastructure/DependencyInjection/SharedViewModelModule.cs Makes token storage optional during settings view-model resolution, matching the existing nullable constructor and null-safe behavior.
GenHub/GenHub.Tests/GenHub.Tests.Linux/Infrastructure/DependencyInjection/LinuxApplicationCompositionTests.cs Verifies the Linux startup view-model graph resolves when no token-storage implementation is registered.
GenHub/GenHub.Tests/GenHub.Tests.Windows/Infrastructure/DependencyInjection/WindowsApplicationCompositionTests.cs Verifies Windows retains its token-storage registration and injects it into the startup settings view model.

Reviews (2): Last reviewed commit: "test(windows): verify token storage inje..." | Re-trigger Greptile

Context used:

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 20a679a6-9132-4d90-aced-4bfe7f2aeae5

📥 Commits

Reviewing files that changed from the base of the PR and between 5ba2a51 and 16f72a5.

📒 Files selected for processing (5)
  • GenHub/GenHub.Tests/GenHub.Tests.Linux/Infrastructure/DependencyInjection/ApplicationCompositionCollection.cs
  • GenHub/GenHub.Tests/GenHub.Tests.Linux/Infrastructure/DependencyInjection/LinuxApplicationCompositionTests.cs
  • GenHub/GenHub.Tests/GenHub.Tests.Windows/Infrastructure/DependencyInjection/ApplicationCompositionCollection.cs
  • GenHub/GenHub.Tests/GenHub.Tests.Windows/Infrastructure/DependencyInjection/WindowsApplicationCompositionTests.cs
  • GenHub/GenHub/Infrastructure/DependencyInjection/SharedViewModelModule.cs

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved application startup when optional GitHub token storage is unavailable.
    • Settings and related startup view models can now be created without requiring token storage.
  • Tests

    • Added Windows and Linux coverage for dependency-injection composition and startup view-model resolution.
    • Added safeguards to prevent environment-dependent tests from running in parallel.

Walkthrough

Shared dependency injection now treats GitHub token storage as optional, allowing Linux startup composition without a token-storage registration. New Linux and Windows composition tests validate platform services, temporary environment isolation, token storage behavior, and startup view-model wiring.

Changes

Application composition

Layer / File(s) Summary
Optional token storage resolution
GenHub/GenHub/Infrastructure/DependencyInjection/SharedViewModelModule.cs
The settings view-model factory uses optional resolution for IGitHubTokenStorage.
Linux composition coverage
GenHub/GenHub.Tests/GenHub.Tests.Linux/Infrastructure/DependencyInjection/*
Linux tests isolate application environment variables and validate platform services and startup view-model resolution without GitHub token storage.
Windows composition coverage
GenHub/GenHub.Tests/GenHub.Tests.Windows/Infrastructure/DependencyInjection/*
Windows tests isolate environment variables and validate Windows token storage, service resolution, and shared view-model identity.
Estimated code review effort: 3 (Moderate) ~20 minutes

Poem

I’m a rabbit with tests in my burrow,
Linux starts without token sorrow.
Windows keeps its secret near,
View-models resolve clear.
Hop, hop—the services all follow!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 50.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title follows conventional commit format and accurately summarizes the optional token storage startup fix.
Description check ✅ Passed The description is directly related to the DI composition fix and new platform tests.
Linked Issues check ✅ Passed The changes meet #299 by making token storage optional, preserving Windows registration, and adding Linux and Windows composition-root tests.
Out of Scope Changes check ✅ Passed The added collection definitions and test helpers support the stated DI composition fix and do not introduce unrelated scope.
📋 Issue Planner

Built with CodeRabbit's Coding Plans for faster development and fewer bugs.

View plan used: #299

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/linux-startup-dependencies

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@bobtista
bobtista marked this pull request as ready for review July 27, 2026 15:42
@kilo-code-bot

kilo-code-bot Bot commented Jul 27, 2026

Copy link
Copy Markdown

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (1 file)
  • GenHub/GenHub.Tests/GenHub.Tests.Windows/Infrastructure/DependencyInjection/WindowsApplicationCompositionTests.cs - 0 issues

Review Notes:

The incremental changes improve the Windows composition test by:

  • Adding Moq support for mocking IGitHubTokenStorage
  • Verifying that WindowsGitHubTokenStorage is correctly registered as a singleton
  • Testing that SettingsViewModel properly integrates with token storage service
  • Confirming that HasToken() is called during SettingsViewModel initialization

The test changes are well-implemented and follow proper mocking patterns. The assertions verify:

  1. Service registration correctness (service type, implementation type, lifetime)
  2. Mock singleton replacement behavior
  3. SettingsViewModel integration with token storage
  4. Expected interaction with the mocked dependency

No security vulnerabilities, logic bugs, or runtime errors found in the incremental changes.

Previous Review Summary (commit 5cc455e)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 5cc455e)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (5 files)
  • GenHub/GenHub/Infrastructure/DependencyInjection/SharedViewModelModule.cs - 0 issues
  • GenHub/GenHub.Tests/GenHub.Tests.Linux/Infrastructure/DependencyInjection/LinuxApplicationCompositionTests.cs - 0 issues
  • GenHub/GenHub.Tests/GenHub.Tests.Windows/Infrastructure/DependencyInjection/WindowsApplicationCompositionTests.cs - 0 issues
  • GenHub/GenHub.Tests/GenHub.Tests.Linux/Infrastructure/DependencyInjection/ApplicationCompositionCollection.cs - 0 issues
  • GenHub/GenHub.Tests/GenHub.Tests.Windows/Infrastructure/DependencyInjection/ApplicationCompositionCollection.cs - 0 issues

Review Notes:

The PR correctly implements the fix for Linux startup composition by making IGitHubTokenStorage optional in SharedViewModelModule.cs. The change from GetRequiredService<IGitHubTokenStorage>() to GetService<IGitHubTokenStorage>() allows Linux to start without the Windows-specific token storage service while preserving the functionality on Windows.

The new composition tests are well-implemented:

  • Properly isolated with xUnit collections to prevent test interference
  • Use unique GUID-based temporary paths for each test run
  • Correctly restore environment variables on cleanup
  • Validate both Linux and Windows startup view model resolution
  • Appropriate use of platform-specific test attributes

No security vulnerabilities, logic bugs, or runtime errors found in the changed code.


Reviewed by glm-4.7 · Input: 28.7K · Output: 3.3K · Cached: 169.6K

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant