Skip to content

v0.15.4: ARCH-1 step 4 — extract MountManager (final ARCH-1 step)#47

Merged
NathanNeurotic merged 1 commit into
mainfrom
feature/v0.15.4-mount-manager-r
May 29, 2026
Merged

v0.15.4: ARCH-1 step 4 — extract MountManager (final ARCH-1 step)#47
NathanNeurotic merged 1 commit into
mainfrom
feature/v0.15.4-mount-manager-r

Conversation

@NathanNeurotic
Copy link
Copy Markdown
Owner

Summary

Fourth and final ARCH-1 collaborator. No user-visible changes. Rebased onto main after v0.15.3 (#46) merged (supersedes #45, which was stacked on the pre-squash branch). Closes the audit's incremental ARCH-1 work.

  • New `src/MountManager.cs` owns:
    • `BuildMountArgs(profile, fullCache, rcCommonFlags, effectiveBandwidth)` — pure helper assembling the full `rclone mount` argv (cache-mode / VFS / network-mode / RC / log / bandwidth). Unit-testable without disk.
    • `StartMountProcess(rclonePath, args, logJobWarn)` — `Process.Start` + v0.11.4 Job Object binding.
  • `TrayContext.Mount.cs` delegates argv assembly + spawn; the UI-coupled dialog flow stays put.

ARCH-1 close-out

Step Class Owns
v0.15.1 `RcloneClient` rclone invocation + `ProcessResult`
v0.15.2 `SettingsStore` settings.json read/write/cache/backup
v0.15.3 `DependencyProbe` rclone/WinFsp availability cache + probes
v0.15.4 `MountManager` mount argv + Process.Start + Job Object

Test plan

  • `scripts/run-tests.ps1` — 53/53 green (rebased on current main)
  • Real-world: mount a profile; Job Object binding still kills rclone when Pixelpipe dies

🤖 Generated with Claude Code

Fourth and final ARCH-1 collaborator extraction. No user-visible
changes.

- New src/MountManager.cs owns:
  * BuildMountArgs(profile, fullCache, rcCommonFlags, bandwidth)
    pure helper that assembles the full rclone mount argv with
    all the v0.5.x cache-mode/VFS/network-mode/RC/log/bandwidth
    flags. Unit-testable without disk.
  * StartMountProcess(rclonePath, args, logJobWarn) Process.Start
    + v0.11.4 Job Object binding.
- TrayContext.Mount.cs delegates argv assembly and the spawn. The
  dialog flow (rclone-missing, WinFsp-missing, remote-not-
  configured, drive-in-use, post-launch result) stays in
  TrayContext because it's UI-coupled.

53/53 tests green, FileVersion 0.15.4.0.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@NathanNeurotic NathanNeurotic merged commit 1e575b3 into main May 29, 2026
1 check passed
@NathanNeurotic NathanNeurotic deleted the feature/v0.15.4-mount-manager-r branch May 29, 2026 08:36
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the rclone mount process lifecycle by extracting the argument assembly and process spawning logic from Pixelpipe.Mount.cs into a new MountManager class. Feedback is provided to handle potential issues in MountManager.cs, specifically by conditionally appending the --log-file argument to prevent startup failures when the log file path is empty, and adding a null check on the spawned process to avoid a potential NullReferenceException.

Comment thread src/MountManager.cs
+ " --volname " + TrayContext.QuoteArg(p.Label)
+ " --rc " + (rcCommonFlags ?? "")
+ " --log-level INFO"
+ " --log-file " + TrayContext.QuoteArg(p.LogFile);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If p.LogFile is null or empty, passing --log-file "" to rclone can cause it to fail to start on Windows due to an invalid path. It is safer to conditionally append the --log-file argument only when a valid log file path is provided.

                + (!String.IsNullOrEmpty(p.LogFile) ? " --log-file " + TrayContext.QuoteArg(p.LogFile) : "");

Comment thread src/MountManager.cs
Comment on lines +64 to +65
Process child = Process.Start(psi);
RcloneJob.TryAssign(child, logJobWarn);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Defensive programming: Process.Start can theoretically return null (e.g., if an existing process is reused, though less likely with UseShellExecute = false). To prevent a potential NullReferenceException in RcloneJob.TryAssign, we should add a null check for child before assigning it.

            Process child = Process.Start(psi);
            if (child != null)
            {
                RcloneJob.TryAssign(child, logJobWarn);
            }

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