review: clarify clearId comment; note deGroupUpdated parallel (out of scope)

Adversarial review found the fix correct/minimal/complete and the test a genuine
guard. Applied the two doc nits: clarify that clearId mirrors sameGroupProfileInfo's
record-nulling *technique* (not the same field), and document that deGroupUpdated's
sameProfile (Service.hs:548-551) still compares publicGroupId but is not currently
triggerable - flagged as a follow-up, deliberately out of scope for this fix.
This commit is contained in:
Narasimha-sc
2026-07-07 08:03:54 +00:00
parent 135237f0d4
commit 92273a1c3b
2 changed files with 12 additions and 2 deletions
@@ -207,7 +207,16 @@ not a directory policy, so it belongs where the comparison lives:
`profileChanged` true and stores). `Commands.hs:4358` is the directory path this
fix targets.
## 8. Verification (done)
### Related, out of scope (follow-up note)
The directory's *other* change detector, `deGroupUpdated`'s `sameProfile`
(`Service.hs:548-551`), still compares whole profiles including `publicGroupId`.
It is **not** currently triggerable by staleness — `CEvtGroupUpdated` carries
`fromGroup`/`toGroup` both freshly DB-loaded with the same stored id — so it is
not part of this bug. But there are now two notions of "profile changed" in the
tree; if a future path ever feeds a stale-id group into `deGroupUpdated`, the same
class of bug could reappear there. Flagged for a follow-up; deliberately not
expanded into by this one-line fix.
Regression test `testLinkCheckStalePublicGroupId` in `tests/Bots/DirectoryTests.hs`
(beside the existing "link/whitespace-only must not re-approve" tests): register +
+2 -1
View File
@@ -1482,7 +1482,8 @@ updateGroupFromLinkData user gInfo@GroupInfo {groupProfile = p, groupDomainVerif
-- publicGroupId is immutable identity, not content, and is not a column
-- updateGroupProfile syncs (it is set at group creation / by updateRelayGroupKeys).
-- A stale stored value would otherwise make this (/=) re-trigger directory
-- approval on every link check. Normalize it out (mirrors sameGroupProfileInfo).
-- approval on every link check. Normalize it out before comparing (same
-- record-nulling technique as sameGroupProfileInfo, which nulls groupPreferences).
profileChanged = clearId p /= clearId groupProfile
clearId gp@GroupProfile {publicGroup} =
gp {publicGroup = (\pg -> (pg :: PublicGroupProfile) {publicGroupId = B64UrlByteString ""}) <$> publicGroup}