fix: add TLS session resumption

[mavonx]

What?

Configured TLS session resumption for IMAP and SMTP.

Why?

Reduce reconnect latency by avoiding full TLS handshakes.

Closes #1174

[FromSi]

If the same account is used concurrently, for example by a background IMAP reconnect and an SMTP send, this can cause a data race during the first cache initialization. It would be better to initialize the cache up front when loading/creating the account, or protect it with a mutex / sync.Once.

[FromSi]

@andrinoff Need to verify the expected behavior:

Verify session resumption is happening with a debug log.

I'm not entirely sure if this is implemented correctly or not.

[mavonx]

@andrinoff Need to verify the expected behavior:

Verify session resumption is happening with a debug log.

I'm not entirely sure if this is implemented correctly or not.

@andrinoff @FromSi

I've verified the behavior in both scenarios:

• Scenario 1
  • Log in with your account (no config.json yet).
  • Click on View Inbox. A new IMAP connection is established because there is no folder_cache.json. At this stage, I got cs.DidResume = false.
  • I fetched a new email body that was not present in the email_bodies folder, so a new IMAP connection was established again, but this time with cs.DidResume = true.
• Scenario 2
  • Same as the first scenario, but this time we test with an already existing email, so the app reads directly from config.json.

[andrinoff]

i will check it out tomorrow evening, im busy

[mavonx]

I tried to handle the race condition as @FromSi suggested above, but tests failed with:

github.com/floatpane/matcha/config.Account contains sync.Once contains sync.noCopy

I moved to a pointer-based approach by extracting sync.Once and tls.ClientSessionCache into a separate SessionCache struct, but this introduced a new failure in TestSaveAndLoadConfig — expectedConfig creates accounts without SC, while LoadConfig now initializes them with SC: &SessionCache{} so reflect.DeepEqual sees a mismatch between nil and a non-nil pointer.

As a temporary fix I added SC: &SessionCache{} to the accounts in expectedConfig to match the loaded config, and the test passes. But I'd prefer not to touch the test.

Is there a cleaner approach here?

[andrinoff]

@mavonx fix the debug logging.

this should not be the experience

[andrinoff]

debug logging could be done by having an env variable (set with ENV_VAR=1 matcha) and it will then spit out.

[andrinoff]

actually, use #1311, just merged, and rebased the branch

[mavonx]

GetClientSessionCache() panics if SC is nil. Won't happen right now, but better add a nil guard. better be safe, than sorry

@andrinoff

Regarding the nil guard — adding a nil check in GetClientSessionCache() would introduce a race condition: two goroutines could both see SC == nil simultaneously and each allocate a new &SessionCache{}, with one overwriting the other and losing cached sessions, so instead I initialize SC eagerly at every account creation site (new login, LoadConfig, and legacy migration) to eliminate the race condition — which is what forced TestSaveAndLoadConfig to need SC: &SessionCache{} in expectedConfig to match what LoadConfig returns.

Is that acceptable, or do you have a preferred approach to avoid touching the test?

[andrinoff]

ok, that's fine then. do the log, i'll resolve the comment

[andrinoff]

i fixed up this pr... why did you force-push...

[mavonx]

@andrinoff

Sorry about that! I think everything is clean now.

[andrinoff]

looks good to me, @FromSi will review later (merge after your review, please)

[floatpanebot]

The CI workflow failed. Please fix the following issues locally and push again:

Please check the CI logs for more details on the failure.