Moves the VPN-specific app files into aVPN folder.
Adds an emptyResources folder whose contents are copied into the bundle at build time.
Adds aMutagenDaemon abstraction for managing the mutagen daemon lifecycle, this class:
- Starts the mutagen daemon usingmutagen daemon run, with aMUTAGEN_DATA_DIRECTORY inApplication Support/Coder Desktop/Mutagen, to avoid collisions with a system mutagen using~/.mutagen.
- Maintains agRPC connection to the daemon socket.
- Stops the mutagen daemon overgRPC
- Relays stdout & stderr from the daemon, and watches if the process exits unexpectedly.
- Handles replacing an orphanedmutagen daemon run process if one exists.

This PR does not embed the mutagen binaries within the bundle, it just handles the case where they're present.

Why is the file sync code in VPNLib?

When I had the FileSync code (namely protobuf definitions) in either:

The app target
A newFSLib framework target

Either the network extension crashed (in the first case) or the app crashed (in the second case) on launch.
The crash was super obtuse:

Library not loaded: @rpath/SwiftProtobuf.framework/Versions/A/SwiftProtobuf

especially consideringSwiftProtobuf doesn't have a stable ABI and shouldn't be compiled as a framework.

At least one other person has ran into this issue when importingSwiftProtobuf multiple times:
apple/swift-protobuf#1506 (comment)

Curiously, this also wasn't happening on local development builds (building and running via the XCode GUI), only when exporting via our build script.

Solution

We're just going to overloadVPNLib as the source of all our SwiftProtobuf & GRPC code. Since it's pretty big, and we don't want to embed it twice, we'll embed it once within the System Extension, and then have the app look for it in that bundle, seeLD_RUNPATH_SEARCH_PATHS. It's not exactly ideal, but I don't think it's worth going to war with XCode over.

TODO

Replace theProcess withhttps://github.com/jamf/Subprocess

Copy link

MemberAuthor

ethanndickson commentedMar 7, 2025•
edited
Loading

This stack of pull requests is managed byGraphite. Learn more aboutstacking.

ethanndickson force-pushed theethan/mutagen-lifecycle branch from6f638fd toe9fa904Compare

March 7, 2025 03:46

ethanndickson commented

Mar 7, 2025

View reviewed changes

Coder Desktop/Coder Desktop/State.swift

		@@ -4,6 +4,7 @@ import KeychainAccess
		import NetworkExtension
		import SwiftUI

		@MainActor

Copy link

MemberAuthor

ethanndicksonMar 7, 2025

Choose a reason for hiding this comment

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

drive-by fix: I believe this is implicit in theObservableObject conformance, but can't hurt to make it explicit.

Coder Desktop/Resources/.gitkeep OutdatedShow resolvedHide resolved

Coder Desktop/Coder Desktop/FileSync/FileSyncDaemon.swift Outdated

		}

		@MainActor
		class MutagenDaemon: FileSyncDaemon {

Copy link

MemberAuthor

ethanndicksonMar 7, 2025•
edited
Loading

Choose a reason for hiding this comment

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

Open to suggestions on how to test this. We can easily mock the gRPC calls, but mocking the child process is ???.

I've just done a good amount of manual testing for now.

Coder Desktop/Coder Desktop/FileSync/FileSyncDaemon.swift Outdated

Comment on lines 53 to 55

		// Stop an orphaned daemon, if there is one
		try? await connect()
		try? await stop()

Copy link

MemberAuthor

ethanndicksonMar 7, 2025•
edited
Loading

Choose a reason for hiding this comment

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

For context,mutagen daemon run acquires a file lock in theMUTAGEN_DATA_DIRECTORY. Multiple instances of the daemon will fail to start, so we need to make sure one isn't running before we start ours. If, for whatever reason, we were unable to kill the daemon, the new one will immediately fail and change theDaemonState.

ethanndickson self-assigned this

Mar 7, 2025

ethanndickson marked this pull request as ready for review

March 7, 2025 04:06

ethanndickson requested review fromdeansheather andThomasK33

March 7, 2025 04:16

ThomasK33 reviewed

Mar 7, 2025

View reviewed changes

Copy link

Member

ThomasK33 left a comment

Choose a reason for hiding this comment

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

The PR looks good on the surface, I'll have to test it locally later and look for edge cases.

Makefile OutdatedShow resolvedHide resolved

flake.nix OutdatedShow resolvedHide resolved

Coder Desktop/Coder Desktop/FileSync/daemon.proto Outdated

		@@ -0,0 +1,11 @@
		syntax = "proto3";

Copy link

Member

ThomasK33Mar 7, 2025

Choose a reason for hiding this comment

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

QQ: Haven't seen a discussion before, but have we considered using the edition2023 version instead of proto3?

Coder Desktop/Coder Desktop/FileSync/FileSyncDaemon.swift OutdatedShow resolvedHide resolved

ethanndickson force-pushed theethan/mutagen-lifecycle branch froma013cde to3384471Compare

March 10, 2025 01:37

ethanndickson mentioned this pull request

Mar 10, 2025

ci: download mutagen binary into resources#103

Merged

ethanndickson force-pushed theethan/mutagen-lifecycle branch from3384471 to7f92c54Compare

March 10, 2025 03:02

ThomasK33 reviewed

Mar 10, 2025

View reviewed changes

Makefile OutdatedShow resolvedHide resolved

ethanndickson requested review fromThomasK33 andspikecurtis and removed request fordeansheather

March 11, 2025 05:28

ThomasK33 approved these changes

Mar 11, 2025

View reviewed changes

Copy link

Member

ThomasK33 left a comment

Choose a reason for hiding this comment

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

LGTM. I think there's an issue in the Coder_DesktopApp.swift file with quitting the VPN on quit

Coder Desktop/VPNLib/FileSync/FileSyncDaemon.swiftShow resolvedHide resolved

Coder Desktop/Coder Desktop/Coder_DesktopApp.swift OutdatedShow resolvedHide resolved

ThomasK33 approved these changes

Mar 11, 2025

View reviewed changes

Copy link

Member

ThomasK33 left a comment

Choose a reason for hiding this comment

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

LGTM

ethanndickson added13 commits

March 12, 2025 13:29

chore: manage mutagen daemon lifecycle

0f362f8

handle missing mutagen

513ccd8

fixup

3291e73

gitkeep resources

9be6173

fixup

b0cbab8

only import swiftprotobuf in vpnlib to debug runtime crash

ebcadbe

extract to seperate lib

5ed3893

move back to vpnlib

c1947aa

remove duplicate framework

b13a44f

logging

c9cba6d

review

2b673b8

error handling

76abed5

log privacy

f2fc365

ethanndickson added5 commits

March 12, 2025 13:29

fixup

21bb169

fixup

2bf41aa

process -> subprocess package

16a7263

use async let

382fd9b

fixup

ad1b24d

ethanndickson force-pushed theethan/mutagen-lifecycle branch from1d0a6e1 toad1b24dCompare

March 12, 2025 02:30

ethanndickson mentioned this pull request

Mar 12, 2025

refactor: replace spaces with hyphens in directory names#110

Merged

Copy link

MemberAuthor

ethanndickson commentedMar 12, 2025•
edited
Loading

Merge activity

Mar 12, 1:43 AM EDT: A user started a stack merge that includes this pull request viaGraphite.
Mar 12, 1:43 AM EDT: A user merged this pull request withGraphite.

ethanndickson merged commit6f6049e intomain

Mar 12, 2025

4 checks passed

ethanndickson deleted the ethan/mutagen-lifecycle branch

March 12, 2025 05:43

This was referencedMar 13, 2025

chore: add mutagen gRPC client#111

Merged

chore: conditionally start file sync daemon#115

Merged

ethanndickson added a commit that referenced this pull request

Mar 24, 2025

chore: conditionally start file sync daemon (#115)

4fb7970

This makes a few improvements to#98:- The mutagen path & data directory can be now be configured on the MutagenDaemon, to support overriding it in tests (coming soon).- A mutagen daemon failure now kills the process, such that can it be restarted (TBC).- Makes start & stop transitions mutually exclusive via a semaphore, to account for actor re-entrancy.- The start operation now waits for the daemon to respond to a version request before completing.- The daemon is always started on launch, but then immediately stopped if it doesn't manage any file sync sessions, as to not run in the background unncessarily.

This was referencedMay 5, 2025

chore: add handler and router forcoder scheme URIs#145

Merged

Investigate why app & system extension shared dependencies fail to embed#149

Open

Labels

None yet

2 participants

Movatterモバイル変換

chore: manage mutagen daemon lifecycle#98

chore: manage mutagen daemon lifecycle#98

Conversation

ethanndickson commentedMar 7, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Why is the file sync code in VPNLib?

Solution

TODO

Uh oh!

ethanndickson commentedMar 7, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

ethanndicksonMar 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ethanndicksonMar 7, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ethanndicksonMar 7, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThomasK33 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ThomasK33Mar 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ThomasK33 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ThomasK33 left a comment

Choose a reason for hiding this comment

Uh oh!

ethanndickson commentedMar 12, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Uh oh!

ethanndickson commentedMar 7, 2025•
edited
Loading

ethanndickson commentedMar 7, 2025•
edited
Loading

ethanndicksonMar 7, 2025•
edited
Loading

ethanndicksonMar 7, 2025•
edited
Loading

ethanndickson commentedMar 12, 2025•
edited
Loading