From bef2bbde756cacbf37aba9d95d43954aeb2463c3 Mon Sep 17 00:00:00 2001 From: Natalie Date: Sun, 9 Aug 2026 13:58:34 -0500 Subject: [PATCH 1/4] atproto: remove redundant handleCache, serve handle through CacheDirectory ResolveAuthorHandle used a package-level go-cache (1h TTL) that was never purged, so a handle rename was invisible for up to an hour. The CacheDirectory already caches the same data and has proper invalidation via purgeIdentCache (called from refreshDriftedIdentity) and resetIdentCache (called on firehose gap). Removing the second cache eliminates the split-brain and the stale handle bug. --- pkg/atproto/atproto.go | 13 +-------- pkg/atproto/resolve_handle_test.go | 46 ++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 12 deletions(-) create mode 100644 pkg/atproto/resolve_handle_test.go diff --git a/pkg/atproto/atproto.go b/pkg/atproto/atproto.go index fb28b0de..dd2b3de8 100644 --- a/pkg/atproto/atproto.go +++ b/pkg/atproto/atproto.go @@ -7,8 +7,6 @@ import ( "sync" "time" - "github.com/patrickmn/go-cache" - "github.com/bluesky-social/indigo/atproto/identity" "github.com/bluesky-social/indigo/atproto/syntax" "github.com/bluesky-social/indigo/xrpc" @@ -21,8 +19,6 @@ import ( var SyncGetRepo = comatproto.SyncGetRepo -var handleCache = cache.New(1*time.Hour, 10*time.Minute) - func (atsync *ATProtoSynchronizer) SyncBlueskyRepoCached(ctx context.Context, handle string) (*model.Repo, error) { ctx, span := otel.Tracer("signer").Start(ctx, "SyncBlueskyRepoCached") defer span.End() @@ -357,19 +353,12 @@ func (atsync *ATProtoSynchronizer) RefreshIdentity(ctx context.Context, did stri } func (atsync *ATProtoSynchronizer) ResolveAuthorHandle(ctx context.Context, did string) string { - if cached, ok := handleCache.Get(did); ok { - return cached.(string) - } ident, err := atsync.resolveIdent(ctx, did, true) if err != nil { log.Warn(ctx, "failed to resolve author handle", "did", did, "err", err) return "" } - handle := ident.Handle.String() - if handle != "" { - handleCache.SetDefault(did, handle) - } - return handle + return ident.Handle.String() } // directory hands back the identity directory to resolve with, building the diff --git a/pkg/atproto/resolve_handle_test.go b/pkg/atproto/resolve_handle_test.go new file mode 100644 index 00000000..98b5eb5b --- /dev/null +++ b/pkg/atproto/resolve_handle_test.go @@ -0,0 +1,46 @@ +package atproto + +import ( + "context" + "testing" + + "github.com/bluesky-social/indigo/atproto/identity" + "github.com/bluesky-social/indigo/atproto/syntax" + "github.com/stretchr/testify/require" + "stream.place/streamplace/pkg/devenv" +) + +// TestResolveAuthorHandleReadsThroughCacheDirectory verifies that +// ResolveAuthorHandle is served from CacheDirectory, so purgeIdentCache +// invalidates it. (Dev accounts resolve as handle.invalid, so this tests +// the purge mechanism rather than a real rename.) +func TestResolveAuthorHandleReadsThroughCacheDirectory(t *testing.T) { + dev := devenv.WithDevEnv(t) + ctx := context.Background() + atsync, _ := backfillTestSynchronizer(t, dev) + user := dev.CreateAccount(t) + + // First call warms the CacheDirectory. + handle := atsync.ResolveAuthorHandle(ctx, user.DID) + require.NotEmpty(t, handle, "first resolve should return a handle") + + did, err := syntax.ParseDID(user.DID) + require.NoError(t, err) + + cd, ok := atsync.directory(true).(*identity.CacheDirectory) + require.True(t, ok) + _, hit, err := cd.LookupDIDWithCacheState(ctx, did) + require.NoError(t, err) + require.True(t, hit, "ResolveAuthorHandle should warm the CacheDirectory") + + // Purge, then confirm the next lookup is a cache miss. + atsync.purgeIdentCache(ctx, user.DID) + _, hit, err = cd.LookupDIDWithCacheState(ctx, did) + require.NoError(t, err) + require.False(t, hit, "purgeIdentCache should evict the entry") + + // Second call still resolves successfully after purge. + handle2 := atsync.ResolveAuthorHandle(ctx, user.DID) + require.NotEmpty(t, handle2, "resolve after purge should still succeed") + require.Equal(t, handle, handle2, "same identity, same handle") +} -- 2.51.2 From 234a324cacdcf259ffab512ef9ada6104e494b01 Mon Sep 17 00:00:00 2001 From: Natalie Bridgers Date: Sun, 9 Aug 2026 14:11:09 -0500 Subject: [PATCH 2/4] Update test comment for ResolveAuthorHandle cache invalidation Signed-off-by: Natalie Bridgers --- pkg/atproto/resolve_handle_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/atproto/resolve_handle_test.go b/pkg/atproto/resolve_handle_test.go index 98b5eb5b..47cdecf3 100644 --- a/pkg/atproto/resolve_handle_test.go +++ b/pkg/atproto/resolve_handle_test.go @@ -11,9 +11,7 @@ import ( ) // TestResolveAuthorHandleReadsThroughCacheDirectory verifies that -// ResolveAuthorHandle is served from CacheDirectory, so purgeIdentCache -// invalidates it. (Dev accounts resolve as handle.invalid, so this tests -// the purge mechanism rather than a real rename.) +// ResolveAuthorHandle is served from CacheDirectory and invalidated properly func TestResolveAuthorHandleReadsThroughCacheDirectory(t *testing.T) { dev := devenv.WithDevEnv(t) ctx := context.Background() -- 2.51.2 From d6775e1661021e7464400de5145e253bdbff7470 Mon Sep 17 00:00:00 2001 From: Natalie Bridgers Date: Sun, 9 Aug 2026 14:56:58 -0500 Subject: [PATCH 3/4] atproto: fix stale identity cache after PDS migration and handle change Signed-off-by: Natalie Bridgers --- pkg/atproto/atproto.go | 3 ++ pkg/atproto/refresh_identity_test.go | 43 ++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 pkg/atproto/refresh_identity_test.go diff --git a/pkg/atproto/atproto.go b/pkg/atproto/atproto.go index dd2b3de8..b3042706 100644 --- a/pkg/atproto/atproto.go +++ b/pkg/atproto/atproto.go @@ -349,6 +349,9 @@ func (atsync *ATProtoSynchronizer) RefreshIdentity(ctx context.Context, did stri if err != nil { return nil, fmt.Errorf("failed to update repo: %w", err) } + // Drop the cached identity so subsequent cached resolves pick up the new + // PDS/handle instead of serving the stale entry for up to 24h. + atsync.purgeIdentCache(ctx, id.DID.String()) return id, nil } diff --git a/pkg/atproto/refresh_identity_test.go b/pkg/atproto/refresh_identity_test.go new file mode 100644 index 00000000..a7bf6a20 --- /dev/null +++ b/pkg/atproto/refresh_identity_test.go @@ -0,0 +1,43 @@ +package atproto + +import ( + "context" + "testing" + + "github.com/bluesky-social/indigo/atproto/identity" + "github.com/bluesky-social/indigo/atproto/syntax" + "github.com/stretchr/testify/require" + "stream.place/streamplace/pkg/devenv" +) + +// TestRefreshIdentityPurgesCache proves that RefreshIdentity drops the cached +// identity entry, so the next cached resolve picks up a new PDS/handle instead +// of serving a stale entry for up to 24h. Without the purge, a PDS migration +// would leave every cached resolve pointing at the dead host. +func TestRefreshIdentityPurgesCache(t *testing.T) { + dev := devenv.WithDevEnv(t) + ctx := context.Background() + atsync, _ := backfillTestSynchronizer(t, dev) + user := dev.CreateAccount(t) + + // Warm the cache with a cached resolve. + _, err := atsync.resolveIdent(ctx, user.DID, true) + require.NoError(t, err) + + did, err := syntax.ParseDID(user.DID) + require.NoError(t, err) + + cd, ok := atsync.directory(true).(*identity.CacheDirectory) + require.True(t, ok) + _, hit, err := cd.LookupDIDWithCacheState(ctx, did) + require.NoError(t, err) + require.True(t, hit, "cache should be warm before refresh") + + // RefreshIdentity should purge the cached entry. + _, err = atsync.RefreshIdentity(ctx, user.DID) + require.NoError(t, err) + + _, hit, err = cd.LookupDIDWithCacheState(ctx, did) + require.NoError(t, err) + require.False(t, hit, "RefreshIdentity should purge the cached identity") +} -- 2.51.2 From 76e823460cd22d070de36a2af8c7f26127471b49 Mon Sep 17 00:00:00 2001 From: Natalie Bridgers Date: Sun, 9 Aug 2026 15:14:12 -0500 Subject: [PATCH 4/4] nuke bad comment Signed-off-by: Natalie Bridgers --- pkg/atproto/refresh_identity_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/atproto/refresh_identity_test.go b/pkg/atproto/refresh_identity_test.go index a7bf6a20..2b06ffd8 100644 --- a/pkg/atproto/refresh_identity_test.go +++ b/pkg/atproto/refresh_identity_test.go @@ -10,10 +10,8 @@ import ( "stream.place/streamplace/pkg/devenv" ) -// TestRefreshIdentityPurgesCache proves that RefreshIdentity drops the cached -// identity entry, so the next cached resolve picks up a new PDS/handle instead -// of serving a stale entry for up to 24h. Without the purge, a PDS migration -// would leave every cached resolve pointing at the dead host. +// RefreshIdentity should drop the cached identity entry, +// so the next cached resolve picks up a new PDS/handle func TestRefreshIdentityPurgesCache(t *testing.T) { dev := devenv.WithDevEnv(t) ctx := context.Background() -- 2.51.2