This looks good so far.

The possible reasons are:

I think the cleanest approach might be to store the actual "reason" on the commit, in addition to the other data, then mostly just do string rendering at display time instead of trying to figure out the "why".

The reason happens in DiffusionLowLevelCommitFieldsQuery->executeQuery(). Either we find a revision ID in the commit message (near line 31) explicitly, as "Differential Revision: ...":

31     $revision_id = idx($fields, 'revisionID');

...or we find one by matching a hash (near line 44):

43        $revision = $this->pickBestRevision($revisions);
44        $fields['revisionID'] = $revision->getID();

In the first case, there's actually a sub-case in T5693, which is that we find "Differential Revision: ..." but choose not to use it. This is currently very difficult to undersatnd/debug, and I spent about an hour walking a user through it to discover that something was configured as http:// instead of https://.

So I think we probably want something like to export a pile more information from DiffusionLowLevelCommitFieldsQuery and/or reorganize that code to produce both a result and a reason, then store that on the commit data.

Ideally, the "reason" should store all the information we need in order to display it to the user later without having to figure anything out. I think that would be something like:

usedURI = true|false # Did we use "Differential Revision: ..." from the commit message?
foundURI = null|http://... # Value we found in "Differential revision: ..." field, if any.
expectURI = http://... # Domain we expected in the field.
matchHashType = null|gttr|gtcm|hgcm
matchHashValue = null|abcdef29832

The big reason to denormalize information that we can figure out later, like expectURI, is to keep the messages stable when users change configuration.

Then we could display a two-part message, with the first part being one of these:

We didn't find a "Differential Revision" field in the commit message.
We found a "Differential Revision" field with value "{$foundURI}" in the commit message, but the domain on this URI did not match the configured domain for this install, "{$expectURI}", so we ignored it under the assumption that it refers to some third-party revision.
We found a "Differential Revision" field with value "{$foundURI}" in the commit message, and the domain on the URI matches this install, so we linked this commit to D123.

And the second part being one of:

(Omit if we found a revision using the commit message.)
This commit and the active diff of D123 had the same commit hash (abcdef1234) so we linked this commit to D123.
This commit and the active diff of D123 had the same tree hash (abcdef1234) so we linked this commit to D123.

In particular, structuring things this way also lets us add an "Explain Why this was not linked" sort of link from the Diffusion side, which is about as confusing to users as why things were linked (putting the controller in Diffusion instead of Differential might make sense).

ends up turning into garbage via the wonders of serialization and de-serialization

Ah, yeah. We can't serialize PHP objects since the field stores as JSON, but it should be OK to save plain arrays with strings in them (e.g., put all that junk above in some closeReason thing if you want).