The cc field is not required and the code was keeping the cc if it existed on an activity or replacing it with the default of an empty list when casting. If any Follow activity was received with a cc field, it would attempt to keep it.
This was noticed in !4208 where we would craft Follow requests with a cc value of nil.
Dialyzer pointed this one out.
The WorkerHelper removal in !4166 was missing this Oban.insert() and tests were not noticing any problems because we mocked the Push.send function instead of executing it and checking for the Oban job.
Before splitting the publish_one/1 function into two parts for testing purposes we had logic that checked the keys of params for :unreachable_since and if it was absent it did not set the instance as reachable. There is also a test to validate that when unreachable_since is nil, we set it as reachable.
However the default value of :unreachable_since when an instance is reachable is nil. The test appears to be testing a scenario that does not exist in the real world, and with this refactor we will always have an :unreachable_since key.
We were attempting to update the reachability upon every successful federation because we always include it when we generate the publish_one jobs.
Gun's connection pool also returns an error if duplicate workers are launched simultaneously. Snooze on this error as well, and lower the snooze to 3 seconds with the optimism that the connection will still be open by then and the delivery can be completed quickly.
The original setting of 30 seconds is pretty high and means there's an unnatural lag between deliveries of activities destined to the same server that were created at nearly the same time. This configuration should be more efficient.
Our test environment cheats by constructing a conn with a custom oauth_access/2 function. This assigns a :token to the conn but due to the way it is constructed it has the :user preloaded. When the OAuth Plug fetches a token it does not preload the user, so the check for user.disclose_client was always nil and assumed to be false.
Preloading the :user ensures the test environment matches reality.