plans: correct root cause to publicGroupId; fix in updateGroupProfile

Deep investigation (three parallel traces) overturned the domain-claim
hypothesis. The claim/proof is persisted and converges (and the group
proof is Nothing anyway), so it cannot cause a perpetual repeat. By
elimination the only non-convergent Eq field is publicGroupId:
updateGroupProfile omits public_group_id from its UPDATE (unlike the two
INSERT paths and updateRelayGroupKeys), so a directory whose stored
public_group_id is NULL can never converge with the link's populated
publicGroupId -> groupUpdated true every 30 min -> re-approval on repeat.

Fix: add public_group_id = COALESCE(public_group_id, ?) to
updateGroupProfile (populate-if-null, never change/erase an immutable
identity). Converges the profiles, stopping re-approval, as a general
core completeness fix. Alternative (scope the moderation comparison) only
hides the symptom.

plans/2026-07-06-fix-directory-name-reapproval.md

@@ -1,185 +1,168 @@
 # Fix: SimpleX directory re-approves a channel every 30 min after a blockchain name is added
 
-Status: proposed — **fix gated on a diagnostic step (see §5); root-cause delta not yet observed**
+Status: proposed — **root cause deduced from code (see §3); fix in core**
 Branch: `nd/fix-directory-names`
 Base: `origin/master` (`b38015c7b`)
 
+> This supersedes two earlier drafts of this plan that blamed the domain claim/proof.
+> Deep investigation showed that was a **red herring** — the claim/proof round-trips
+> and converges. The real non-convergent field is **`publicGroupId`**. See §3.
+
 ## 1. Problem
 
-After an owner adds a SimpleX name (blockchain domain claim) to a public
-channel, the SimpleX Directory service asks the owner to re-approve the channel
-**every 30 minutes, indefinitely**, with no changes on the owner's side. The
-channel keeps flipping to "hidden until approved."
+After an owner adds a SimpleX name (blockchain domain claim) to a public channel,
+the SimpleX Directory service asks the owner to re-approve the channel **every
+30 minutes, indefinitely**, with no changes on the owner's side. The channel keeps
+flipping to "hidden until approved."
 
-## 2. What is confirmed (facts, file:line)
+## 2. Confirmed trigger (facts, file:line)
 
-1. **The 30-min cadence is the directory's link-check timer.** `linkCheckInterval`
+1. **30-min cadence = the directory's link-check timer.** `linkCheckInterval`
    defaults to **1800 s** (`apps/simplex-directory-service/src/Directory/Options.hs:181`);
-   `linkCheckThread_` sleeps that long and enqueues a `DEGroupLinkCheck` for every
-   non-removed registered group (`.../Directory/Service.hs:199-211`).
+   `linkCheckThread_` sleeps that interval and enqueues a `DEGroupLinkCheck` for
+   every non-removed registered group (`.../Directory/Service.hs:199-211`).
+2. **`deGroupLinkCheck` re-approves iff `groupUpdated` is true** —
+   `when groupUpdated $ reapprove …` (`Service.hs:824`), flipping `GRSActive →
+   GRSPendingApproval` and re-sending for admin approval (`:843-854`).
+3. **`groupUpdated` is a whole-profile byte inequality** — the `Bool` returned by
+   `updateGroupFromLinkData`: `profileChanged = p /= groupProfile`
+   (`src/Simplex/Chat/Library/Internal.hs:1466-1482`), `p` = directory's *stored*
+   `GroupProfile`, `groupProfile` = profile from the *fetched* link short-link data.
+4. **The flag is behaviorally directory-only** — the mobile/desktop/iOS
+   `GroupLinkPlan.Known` variant does not carry it (`SimpleXAPI.kt:7158`; iOS
+   `AppAPITypes.swift`); only the directory consumes it (`Service.hs:824`, `:825`,
+   `:988`→`deReregistration:1043/1046`).
 
-2. **`DEGroupLinkCheck` re-approves iff `groupUpdated` is true.** `deGroupLinkCheck`
-   runs `APIConnectPlan` against the channel link and, on a `GLPKnown` plan, does
-   `when groupUpdated $ reapprove …` (`Service.hs:815-854`, trigger at `:824`),
-   which sets `GRSPendingApproval` and re-sends for admin approval.
+## 3. Root cause (deduced by elimination — code-confirmed)
 
-3. **`groupUpdated` is a whole-profile byte inequality.** It is the `Bool` returned
-   by `updateGroupFromLinkData`: `profileChanged = p /= groupProfile`
-   (`src/Simplex/Chat/Library/Internal.hs:1466-1482`), where `p` is the directory's
-   **stored** `GroupProfile` and `groupProfile` is the profile in the **fetched
-   link data**.
+Three independent traces close the case without needing a runtime diagnostic:
 
-4. **`groupUpdated` is behaviorally directory-only.** The mobile/desktop clients do
-   not even deserialize it: the client `GroupLinkPlan.Known` variant carries only
-   `groupInfo` — `class Known(val groupInfo: GroupInfo)`
-   (`apps/multiplatform/.../model/SimpleXAPI.kt:7158`; iOS
-   `apps/ios/Shared/Model/AppAPITypes.swift`, `enum GroupLinkPlan`). The only
-   consumers of the flag are in the directory service: re-approval (`Service.hs:824`),
-   listing refresh (`:825`), and owner re-registration (`:988` →
-   `deReregistration:1043,1046`). (All other `groupUpdated` matches in the tree are
-   an unrelated chat event, `RcvGroupEvent`/`CR.GroupUpdated`.)
+- **The fetched link profile is byte-identical across fetches.** It is served
+  verbatim from the owner's published, owner-authorized `LSET` blob via `LGET`
+  (`simplexmq` agent `getConnShortLink` → `decryptLinkData`); there is no
+  timestamp, per-fetch nonce, relay reconstruction, or server-computed member
+  count. Member count lives in `GroupShortLinkData.publicGroupData` (a sibling of
+  `groupProfile`) and drives `countChanged`, not `groupUpdated`. So the link side
+  cannot make `groupUpdated` flip on its own.
+- **Every `Eq`-relevant `GroupProfile` field except one is persisted by
+  `updateGroupProfile` and therefore converges** after the first store
+  (`Internal.hs:1471`). The single exception is **`publicGroupId`**
+  (`Types.hs:858`, part of `PublicGroupProfile`, `deriving Eq`): the UPDATE in
+  `updateGroupProfile` (`src/Simplex/Chat/Store/Groups.hs:2721-2732`) writes
+  `group_type, group_link, group_web_page, group_domain, domain_web_page,
+  allow_embedding, group_domain_proof, …` but **omits `public_group_id`**. It is
+  read back from the stored column (`Shared.hs:699`, `Groups.hs:2779`), and
+  `toPublicGroupProfile` returns `publicGroup = Nothing` unless `group_type` **and**
+  `group_link` **and** `public_group_id` are all present (`Shared.hs:713-716`).
+- **The domain claim/proof is not the cause.** `group_domain`/`group_domain_proof`
+  *are* persisted and round-trip symmetrically (`Shared.hs:721`/`:727`), so they
+  converge. In the group flow the proof is `Nothing` anyway (`SimplexDomainProof`
+  is never constructed in `src/`; group claims use `mkDomainClaim`, `proof = Nothing`,
+  `Names.hs:56`, `Commands.hs:5693`), and group verification is a separate scalar
+  column `group_domain_verified` (`Groups.hs:2740-2746`).
 
-## 3. What is NOT yet established
+**Deduction:** a *perpetual* re-approval requires a field that never reconciles.
+The link side is deterministic, and every field except `publicGroupId` converges
+after one store — so the only field that can hold `p /= groupProfile` forever is
+`publicGroupId`. It does so because the directory's stored `public_group_id` is
+**NULL / stale** and `updateGroupProfile` — the only function that syncs a group's
+profile from link data or `XGrpInfo` — cannot write that column.
 
-**Why the two profiles fail to converge every cycle is unproven.** An ordinary
-profile change self-corrects: when `profileChanged` is true,
-`updateGroupFromLinkData` writes the link profile to the DB (`Internal.hs:1471`),
-and the store round-trips the domain claim + proof faithfully
-(`group_domain_proof` column; symmetric `publicGroupAccessRow` /
-`toPublicGroupAccess`, `src/Simplex/Chat/Store/Shared.hs:721`/`:727`). So a
-one-time change re-approves **once** and then converges. A perpetual 30-min
-re-approval requires the directory's stored profile to be **permanently unequal**
-to the fetched link profile — and which field differs has **not been observed**.
+**Why the directory's `public_group_id` is NULL, and why it correlates with the
+name.** `public_group_id` is written in only three places, all *creation-time*:
+the two group INSERTs (`Groups.hs:405`, `:912`, from the profile's `publicGroup`)
+and `updateRelayGroupKeys` (`:2293`). If the directory joined the channel while it
+had no `publicGroupId` in its link profile (an older channel, or one that became a
+public/named channel later), its `public_group_id` was inserted NULL and can never
+be populated afterward. When the owner adds the SimpleX name, the owner republishes
+the link with a fully-populated `publicGroup` (`publicGroupId` + `publicGroupAccess`),
+so the fetched profile's `publicGroup` becomes `Just{…}` while the directory's
+stored copy stays `Nothing` — a permanent inequality, re-approved every 30 min.
 
-Two hypotheses were considered and both are insufficient as stated:
+**Confirmation step (cheap, not blocking):** on the affected directory,
+`SELECT public_group_id FROM group_profiles` joined to the channel's group should
+show NULL, while the link's `publicGroupId` is present.
 
-- **Relay drops the domain claim from the served link data**
-  (`src/Simplex/Chat/Library/Commands.hs:4326-4328` handles this case). But this
-  **converges after one cycle**: the directory stores the claim-less link profile,
-  and with no further owner change nothing restores the claim, so the next check
-  matches. It explains a single reapproval, not a repeat.
-- **A re-presented proof with a fresh nonce.** This does **not** apply to domain
-  claims. `PHTest`/`badgeProof` (fresh nonce per presentation) is the separate
-  *badge* feature, attached to `Profile.badge` (`Internal.hs:2085-2086`), not to
-  the domain claim. A `SimplexDomainProof`'s `presHeader` is **inside the signed
-  payload** (`Commands.hs:4869`, verified at `:4867`), so it cannot be re-presented
-  with a new nonce without re-signing by the owner — it is static. The group claim
-  is also frequently created with `proof = Nothing` (`mkDomainClaim`,
-  `src/Simplex/Chat/Names.hs:56`; used at `Commands.hs:5693`, `:1510`).
+## 4. The fix (core, minimal): let `updateGroupProfile` populate `public_group_id`
 
-So the correlation ("started right after adding a name") strongly implicates the
-domain claim as the differing field, but the **exact field and the reason it
-never converges are not yet demonstrated.** This plan therefore treats the fix as
-gated on a diagnostic.
+`updateGroupProfile` is missing a profile column that its sibling INSERT paths
+already persist. Add `public_group_id` to its UPDATE, **guarded by `COALESCE` so it
+only populates a currently-NULL value and never changes or erases an existing
+identity** (respecting `publicGroupId`'s immutability):
 
-## 4. Likely mechanism (hypothesis, to be confirmed)
-
-The differing field is the group profile's `publicGroup.publicGroupAccess.groupDomainClaim`
-(or its `proof`): the directory's stored copy and the fetched link copy carry
-different claim representations on every fetch, in a way that storing the link
-copy does not reconcile. Because the domain claim is verified **independently** —
-cryptographically + on-chain by `verifyEntityDomain` (`Commands.hs:4840`), with
-verification tracked in a *separate* field `group_domain_verified`
-(`Store/Groups.hs:2710-2713`) — it is not admin-moderated content and should never
-gate re-approval. The bug is that `groupUpdated` conflates "profile changed at
-all" (used to decide whether to *store* the fresh claim) with "profile changed in
-a way needing admin *re-moderation*" (used to decide re-approval).
-
-## 5. Required diagnostic (gate before the fix)
-
-Add a temporary log at the one comparison that flips everything,
-`Internal.hs:1482`, dumping the two profiles' domain claims when `profileChanged`
-fires, and run it against the affected channel across ≥2 link-check cycles:
-
-```haskell
--- temporary:
-when (p /= groupProfile) $ logInfo $ "linkcheck delta: stored=" <> tshow (claimOf p)
-                                   <> " link=" <> tshow (claimOf groupProfile)
+```sql
+-- src/Simplex/Chat/Store/Groups.hs, updateGroupProfile_ UPDATE:
+SET display_name = ?, full_name = ?, short_descr = ?, description = ?, image = ?,
+    group_type = ?, group_link = ?, public_group_id = COALESCE(public_group_id, ?),
+    group_web_page = ?, group_domain = ?, domain_web_page = ?, allow_embedding = ?,
+    group_domain_proof = ?, preferences = ?, member_admission = ?, updated_at = ?
 ```
 
-Decision:
-- **Delta is the domain claim, identical each cycle** → structural asymmetry;
-  apply §6 (proof redaction covers it if the delta is the proof; if the delta is
-  the whole claim, widen the redaction to the claim — decide from the log).
-- **Delta changes each cycle** → identify the varying field from the log; if it is
-  not the claim, this plan's fix does not apply and the diagnosis restarts here.
-
-The fix in §6 is only justified once the log shows the per-cycle delta is confined
-to the domain claim/proof.
-
-## 6. The fix (conditional on §5)
-
-In `updateGroupFromLinkData` (`Internal.hs`), keep storing on the full
-`profileChanged` (so the claim/proof and verification stay current), but report
-`groupUpdated` with the domain-claim **proof** redacted — reusing the codebase's
-existing proof-redaction idiom (`Types.hs:750` `clearProofs`; `Internal.hs:1265`
-`redactedDomain`), rather than new helpers:
+with the new `?` sourced exactly like the INSERT paths (`Groups.hs:384-385`):
 
 ```haskell
-        pure (g'', moderationChanged)      -- was: profileChanged
-  ...
-  where
-    profileChanged = p /= groupProfile     -- keep: still gates storing the fresh claim
-    -- A domain-claim change is verified independently (on-chain + owner signature)
-    -- and is not admin-moderated, so it must not trigger directory re-approval.
-    -- Redact the proof before comparing (same idiom as Types.clearProofs).
-    moderationChanged = redactProof p /= redactProof groupProfile
-    redactProof gp@GroupProfile {publicGroup} = gp {publicGroup = redactPg <$> publicGroup}
-    redactPg pg@PublicGroupProfile {publicGroupAccess} = pg {publicGroupAccess = redactAccess <$> publicGroupAccess}
-    redactAccess a@PublicGroupAccess {groupDomainClaim} =
-      a {groupDomainClaim = (\d -> d {proof = Nothing} :: SimplexDomainClaim) <$> groupDomainClaim}
+publicGroupId_ = case publicGroup of
+  Just PublicGroupProfile {publicGroupId} -> Just publicGroupId
+  Nothing -> Nothing
 ```
 
-Redacting **only the proof** (not the whole claim) is the surgical choice: it kills
-any proof-representation delta while still letting a genuine domain add/change
-register as a one-time change (which then converges). If §5 shows the delta is the
-whole claim (e.g. claim present vs absent every cycle), replace `redactAccess` with
-`groupDomainClaim = Nothing`; note this also suppresses the legitimate one-time
-reapproval on a name add.
+Effect: on the next link check, the directory's NULL `public_group_id` is populated
+from the authoritative link profile → its stored `publicGroup` reconstructs as
+`Just{…}` equal to the link's → `profileChanged` becomes false → re-approval stops.
 
-Storage keys off the full `profileChanged`, so the claim/proof and
-`setGroupDomainVerified` still update — by-name lookups and the verified badge stay
-correct.
+## 5. Why this is the most correct fix
 
-## 7. Why this is safe (given §5 confirms the delta)
+- **Fixes the actual root cause, and converges.** After one sync the profiles are
+  equal, so `groupUpdated` correctly reports "no change." Symptom and cause both go.
+- **It is a plain completeness fix.** The two INSERT paths and `updateRelayGroupKeys`
+  already persist `public_group_id`; `updateGroupProfile` omitting it is the bug.
+- **Safe under `COALESCE(public_group_id, ?)`** — it *only* fills a NULL. It never
+  changes a set identity, so it cannot corrupt a correct `publicGroupId`, and it
+  cannot be erased by a claim-less `XGrpInfo` (new value `Nothing` → keep old).
+  This preserves the "immutable identity" invariant while repairing missing data.
+- **General, not a directory workaround.** Any client whose `public_group_id` is
+  missing gets it repaired on the next profile sync; the fix is in shared core, as
+  intended, and needs no judgment about which fields are "moderatable."
+- **Blast radius is safe.** `updateGroupProfile` is widely called, but the only
+  behavioral change is *populating a NULL column when a profile carrying a
+  `publicGroupId` is stored* — a pure improvement for every caller.
 
-- **Directory-only blast radius** (fact, §2.4). All three consumers behave
-  correctly with the proof/claim excluded:
-  - `Service.hs:824` periodic re-approval → **fixed**.
-  - `Service.hs:988` → `deReregistration` (`:1043`/`:1046`) — an owner re-submitting a
-    name-only change gets "already listed" instead of forced re-approval.
-  - `Service.hs:825` `listingsUpdated` — minor trade-off: a name-only change no longer
-    *immediately* refreshes web-listing files (still refreshes on any real change or
-    member-count change). If prompt refresh matters, keep `:825` on the raw
-    `profileChanged` while `:824` uses `moderationChanged`.
-- **Moderation not weakened.** Re-approval still fires on any moderatable field
-  (name, description, image); only proof/claim deltas are excluded, and those are
-  independently verified.
+## 6. Alternative considered — and why not
 
-## 8. Residual behaviour (accepted)
+**Scope the re-approval comparison** (return `groupUpdated` from
+`updateGroupFromLinkData` computed over only the moderatable display fields,
+excluding `publicGroup`). This mirrors the contact path's `clearProofs` /
+`sameProfileContent` redaction (`Types.hs:747-750`, `:803`) and the existing
+`DirectoryTests` expectation that link-only changes must not re-approve
+(`tests/Bots/DirectoryTests.hs:296-326`). It would stop the re-approval — **but it
+only hides the symptom**: the stored `public_group_id` stays NULL, the profiles
+stay unequal, so `updateGroupFromLinkData` keeps calling `updateGroupProfile` every
+30 min to store a profile that never fully matches (idempotent churn), and any
+other consumer of the difference stays broken. §4 is preferred because it removes
+the inequality itself. (This scoping could still be added later as defense in depth
+for genuinely non-moderatable fields, but it is not needed for this bug.)
 
-If the claim/proof genuinely differs each cycle, the store branch re-writes the
-`group_profiles` row every 30 min. This is idempotent and does not thrash
-verification (`claimChanged` in `updateGroupProfile` compares the *domain*, not the
-proof — `Groups.hs:2708-2709`). A follow-up could skip the store on proof-only
-deltas; out of scope.
+## 7. Verification plan
 
-## 9. Verification plan
+1. Confirm on the affected directory that the channel's `public_group_id` is NULL
+   while the link carries a `publicGroupId` (validates §3; one SELECT).
+2. Apply §4.
+3. **Regression test** in `tests/Bots/DirectoryTests.hs` (sits alongside the
+   existing "link-only / whitespace-only must not re-approve" tests at `:296-326`):
+   register a channel whose stored `public_group_id` is NULL → add a SimpleX name /
+   populate the link `publicGroupId` → run ≥2 link checks → assert the registration
+   stays `GRSActive` (no `GRSPendingApproval`, no owner re-approval message). Also
+   assert the group's `public_group_id` is populated after the first check.
+4. Build the directory service (which statically links core); run the directory
+   test suite.
 
-1. Land the §5 diagnostic, capture the delta on the affected channel, confirm it is
-   the domain claim/proof, and choose proof-vs-whole-claim redaction from the log.
-2. Apply §6.
-3. **Regression test** in `tests/Bots/DirectoryTests.hs`: register a channel →
-   approve → add a SimpleX name → trigger ≥2 link checks → assert the registration
-   stays `GRSActive` (no `GRSPendingApproval`, no owner re-approval message).
-4. Build the directory service; run the directory test suite.
+## 8. Files touched
 
-## 10. Files touched
+- `src/Simplex/Chat/Store/Groups.hs` — `updateGroupProfile`: add
+  `public_group_id = COALESCE(public_group_id, ?)` to the UPDATE and thread the
+  `publicGroupId_` parameter (as the INSERT paths do). No other logic change.
+- `tests/Bots/DirectoryTests.hs` — add the name-added / NULL-`public_group_id`
+  no-reapproval regression test.
 
-- `src/Simplex/Chat/Library/Internal.hs` — `updateGroupFromLinkData`: return a
-  claim/proof-insensitive change flag (store logic unchanged). Temporary diagnostic
-  log removed before merge.
-- `tests/Bots/DirectoryTests.hs` — add the name-added-no-reapproval regression test.
-
-No schema, wire-format, or API changes. `GLPKnown.groupUpdated`'s type is unchanged;
-only its value semantics (directory-only consumer) are refined.
+No schema, wire-format, or API changes. `updateGroupProfile`'s signature is
+unchanged; it simply stops dropping a column it is already given.