Helper method/service/trait/thing to get the active group?

What would you suggest? We put that in a service instead? Something that could return the Group detected by Group Sites? I'd be up for that, but I'm a bit strapped for time. Feel free to open an MR with your suggestion so we have a starting point for discussion.

Okay I see what you're getting at now. I would approach this slightly differently, so that we don't have to repeat this code if we care about the cacheable metadata. Domain has a pretty good class/method names for this that we can mimic.

Something like: GroupSitesNegotiator::getActiveGroup($cacheable_metadata)

public function getActiveGroup(CacheableMetadata $cacheable_metadata): GroupInterface|null {
    $cacheable_metadata->addCacheTags(['config:group_sites.settings'])
    $context_id = $this->configFactory->get('group_sites.settings')->get('context_provider');
    if ($context_id === NULL) {
      return NULL;
    }

    $contexts = $this->contextRepository->getRuntimeContexts([$context_id]);
    $context = count($contexts) ? reset($contexts) : NULL;
    if ($context) {
      $cacheable_metadata->addCacheableDependency($context);
    }

    $group = $context?->getContextValue();
    if ($group && !$group instanceof GroupInterface) {
      throw new \InvalidArgumentException('Context value is not a Group entity.');
    }

    return $group;
}

Then we can inject that into GroupSitesAccessPolicy and use that there.

Logging these notes about how where we're requiring the group from context in our code base at present. Just posting here, really notes for myself about the cacheable metadata. In the interim I'd made a .module function to get the group, we're calling this, so looking at where we're using it, some are legacy, but:

Decide which page to redirect to on login
https://github.com/localgovdrupal/localgov_microsites_group/blob/98c4e17...
I don't think we mind the cacheable metadata, but we do have it from $form_state.

Adding a relationship entity for content made outside the group forms on a group sites domain
https://github.com/localgovdrupal/localgov_microsites_group/blob/98c4e17...
and also legacy versions of doing that, which we can remove
https://github.com/localgovdrupal/localgov_microsites_group/blob/98c4e17...
https://github.com/localgovdrupal/localgov_microsites_group/blob/98c4e17...
Both cases, we've got the entity, so can pass the cacheable metadata from it, but don't think it makes a difference?

Something clever with the Taxonomy Term form
https://github.com/localgovdrupal/localgov_microsites_group/blob/4.x/mod...
which probably will benefit from having cacheable metadata.

Finally some workaround
https://github.com/localgovdrupal/localgov_microsites_group/blob/98c4e17...
where it looks like the original cache tags aren't yet correct. We can get cacheable metadata from the entity, but it's not going to make a difference here.

Switched to use GroupSitesNegotiator naming, added setting the CacheMetadata. Made this optional. These tested in the Kernel test.
Then changed the AccessPolicy to use the new Negotiator. Updated the Unit tests to reflect the changed internals, and added equivalent Unit tests for the GroupSitesNegotiator.

As we discussed in person, looking good but let's make it more defensive and clear that the negotiator is internal

Think that covered them. The additional Unit test for the Negotiator is left, but might just duplicate what is in the Access Policy test.

Sorry for the LONG delay.

I just had another go at the MR, I also noticed ContainerAwareTrait is dead so need to create an issue where we replace it with a service locator or container dependency.