From f63738b4d86c26a6725a88b0149a6e9fcf01d929 Mon Sep 17 00:00:00 2001 From: Noah-Bytes Date: Wed, 22 Jul 2026 11:18:48 +0800 Subject: [PATCH] fix(owner-sync): backfill empty owner name + encodeURIComponent member ids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: syncSelf returns early when the owner is unchanged (`coreOwnerId === localOwnerId`), before the owner-name fetch. Owner-name fetch failures/timeouts are silently swallowed (cosmetic — must not block the owner bind), so the owner gets bound with an empty name. Every subsequent periodic sync then short-circuits at that check and skips the name fetch, so the empty name never backfills (until the owner member_id itself changes). Display-only impact — access control keys on member_id, which stays correct. Fix (mirrors claude-openmax PR #16): - Short-circuit only when id matches AND the local name is already non-empty; otherwise fall through to re-fetch/backfill the name. - Only write when something actually changed (new owner id, or a non-empty fetched name differing from local) — if core still returns no name for an already-bound owner, skip the write so a steady state does not re-persist an empty name every tick. - encodeURIComponent both member ids in the /members/{id} paths (hardening). Note: claude-openmax / zylos-openmax share this owner-sync lineage; same fix landed on claude-openmax PR #16 (41ae89c6). Test: two-phase backfill regression (name fetch fails -> empty name -> next sync backfills) + no-redundant-persist (id matches, core still nameless -> no write). All 175 tests pass + tsc build clean. Co-Authored-By: Claude Opus 4.8 --- src/owner-sync.ts | 25 ++++++++++++++++++++---- test/owner-sync.test.ts | 43 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/src/owner-sync.ts b/src/owner-sync.ts index 1b82801..cf62f3d 100644 --- a/src/owner-sync.ts +++ b/src/owner-sync.ts @@ -54,7 +54,7 @@ export function makeSyncSelf(http: HttpForOrg, provider: ConfigProvider, log: Lo let member: MemberRecord; try { - member = (await http.getForOrg(orgId, http.apiPath(`/members/${selfMemberId}`))) as MemberRecord; + member = (await http.getForOrg(orgId, http.apiPath(`/members/${encodeURIComponent(selfMemberId)}`))) as MemberRecord; } catch (err) { log.warn?.(`[${orgId}] owner-sync: fetch self member failed: ${err instanceof Error ? err.message : String(err)} — keeping local owner`); return { nameReady: false, reason: `fetch self member failed: ${err instanceof Error ? err.message : String(err)}` }; @@ -73,18 +73,35 @@ export function makeSyncSelf(http: HttpForOrg, provider: ConfigProvider, log: Lo if (!coreOwnerId) return { nameReady: true, displayName: coreDisplayName || undefined }; const localOwnerId = orgConfig.owner?.member_id || ""; - if (coreOwnerId === localOwnerId) return { nameReady: true, displayName: coreDisplayName || undefined }; + const localOwnerName = orgConfig.owner?.name || ""; + // Fully in sync (id matches AND we already have a name) → nothing to do. Do NOT + // early-return when the id matches but the local name is EMPTY: a prior owner-name + // fetch may have failed/timed out and bound an empty name (the catch below swallows + // it — name is cosmetic, must not block the bind); this is the path that backfills + // it on a later tick. + if (coreOwnerId === localOwnerId && localOwnerName) + return { nameReady: true, displayName: coreDisplayName || undefined }; let ownerName = ""; try { - const ownerMember = (await http.getForOrg(orgId, http.apiPath(`/members/${coreOwnerId}`))) as MemberRecord; + const ownerMember = (await http.getForOrg(orgId, http.apiPath(`/members/${encodeURIComponent(coreOwnerId)}`))) as MemberRecord; ownerName = ownerMember?.display_name || ownerMember?.username || ""; } catch { // owner display_name is cosmetic — a fetch failure must not block the owner bind. } + + // Only write when something ACTUALLY changed — a new owner id, or a non-empty + // fetched name that differs from the local one (the backfill case). If core still + // returns no name for an already-bound owner, skip the write so a steady state does + // not re-persist an empty name on every periodic tick. + const idChanged = coreOwnerId !== localOwnerId; + const nameChanged = !!ownerName && ownerName !== localOwnerName; + if (!idChanged && !nameChanged) + return { nameReady: true, displayName: coreDisplayName || undefined }; + provider.setOwner(orgId, coreOwnerId, ownerName); orgConfig.owner = { member_id: coreOwnerId, name: ownerName }; - log.info?.(`[${orgId}] owner synced from core: ${localOwnerId || "(none)"} → ${coreOwnerId}${ownerName ? ` (${ownerName})` : ""}`); + log.info?.(`[${orgId}] owner synced from core: ${localOwnerId || "(none)"} → ${coreOwnerId}${ownerName ? ` (${ownerName})` : ""}${idChanged ? "" : " (name backfill)"}`); return { nameReady: true, displayName: coreDisplayName || undefined }; }; diff --git a/test/owner-sync.test.ts b/test/owner-sync.test.ts index 6b849bb..67c0628 100644 --- a/test/owner-sync.test.ts +++ b/test/owner-sync.test.ts @@ -98,4 +98,47 @@ describe("makeSyncSelf", () => { expect(res.nameReady).toBe(false); expect(org.owner).toEqual({ member_id: "local_owner", name: "Local" }); }); + + it("当 owner 名字首拉失败先存空名后，下一次 sync 应回填名字（不因 owner 未变而永久空名）", async () => { + const { provider, org, reloadedOrg } = setup(); + // owner_1 暂不在册 → 首次绑定时名字拉取 404 失败（cosmetic，不阻塞绑定）。 + const members: Record> = { + m_self: { display_name: "Codex", owner_member_id: "owner_1" }, + }; + const sync = makeSyncSelf(fakeHttp(members), provider, quietLog); + + // 第一次：owner 名字拉取失败 → 用空名绑定 owner。 + await sync(org); + expect(org.owner).toEqual({ member_id: "owner_1", name: "" }); + + // owner 名字随后可用（超时/短暂失败恢复）。 + members.owner_1 = { display_name: "Owner One" }; + + // 第二次：owner 未变但本地名字为空 → 应继续回填，而非在「owner 未变」处提前返回把空名留下。 + await sync(org); + expect(org.owner).toEqual({ member_id: "owner_1", name: "Owner One" }); + expect(reloadedOrg().owner).toEqual({ member_id: "owner_1", name: "Owner One" }); + }); + + it("owner id 匹配且 core 仍无名字时，不做冗余写入（稳态不每 tick 重复 persist 空名）", async () => { + const { provider, org } = setup({ member_id: "owner_1", name: "" }); + let setOwnerCalls = 0; + const origSetOwner = provider.setOwner.bind(provider); + provider.setOwner = ((...args: Parameters) => { + setOwnerCalls++; + return origSetOwner(...args); + }) as typeof provider.setOwner; + + // owner_1 在册但无 display_name/username → 名字仍解析为空。 + const http = fakeHttp({ + m_self: { display_name: "Codex", owner_member_id: "owner_1" }, + owner_1: {}, + }); + const res = await makeSyncSelf(http, provider, quietLog)(org); + + // id 未变且名字仍为空 → 无变化 → 不调用 setOwner（不重复落盘空名）。 + expect(res.nameReady).toBe(true); + expect(setOwnerCalls).toBe(0); + expect(org.owner).toEqual({ member_id: "owner_1", name: "" }); + }); });