Kristofer Karlsson's patch teaches the connectivity check to ignore replacement objects. The commit message notes that fsck, prune, pack-objects and upload-pack were taught to ignore replacements in dae556bdb1 (2009), because they must work with the original objects. check_connected() came later without doing the same. Karlsson says this is not a problem seen in practice, but the two behaviours can disagree.
Junio Hamano first read "ignore replacements" as meaning the replacing objects would be skipped and only the underlying objects' connections checked. After Karlsson clarified, he agreed there was no need to worry about "doing both".
Patrick Steinhardt raised several questions. He asked whether a particular test was introduced on purpose to cover an edge case. He suggested one hunk might deserve its own commit, since it is not strictly about the connectivity check, and asked whether the other object-verifying sites had been vetted. He also wondered whether both the original and the replacement object should be verified, since the check used to look at the latter and now looks at the former.
Karlsson replied that the t5413 test and tree-verify.c came from his recent incremental connectivity check series. He noticed the replacement oddity then but deliberately kept the existing behaviour, because changing the semantics should be a separate, intentional step. He said removing the flag in tree-verify.c belongs in this change, since that code runs only for rev-list --connectivity-check=incremental, where replacements are already globally disabled.
On verifying both objects, Karlsson noted that replacements are ordinary refs (refs/replace/OLD -> NEW), so their targets should be covered by normal connectivity checks when those refs are fetched or pushed. Steinhardt said it is arguably the responsibility of whoever wrote the replace ref to make sure the new object is valid. He added that a push's connectivity check already assumes existing refs are fully connected, and concluded he is aligned with the change.
Karthik Nayak pointed out that Documentation/git-replace.adoc says replacements are used by all commands except those doing reachability traversal (prune, pack transfer and fsck). Karlsson answered that the list was never exhaustive, since index-pack, unpack-objects, commit-graph and multi-pack-index also disable replacements. He added that pack transfer's connectivity check followed replacements until now, contrary to the documentation. Whether the documentation will be changed, and whether the test change is split out, was not settled in the excerpts.