diff --git a/internal/store/context.go b/internal/store/context.go index 12dc8d27..18ee3ebf 100644 --- a/internal/store/context.go +++ b/internal/store/context.go @@ -3,6 +3,7 @@ package store import ( "context" "encoding/json" + "strings" "github.com/google/uuid" ) @@ -201,8 +202,20 @@ func SenderIDFromContext(ctx context.Context) string { } // ActorIDFromContext returns the acting principal — the entity performing -// the action. In group chats (Telegram/Discord) this is the individual -// sender; in DMs / HTTP / cron it is the same as UserIDFromContext. +// the action. The resolution is context-aware: +// +// - Group / guild scope (UserID has "group:" or "guild:" prefix): +// returns SenderID (the individual sender) when set, else falls back +// to UserID. SenderID is the only stable actor identity in a group +// because UserID is the shared group principal. +// +// - DM / HTTP / cron / anywhere UserID is a real user identifier: +// returns UserID. This preserves tenant-user merging — the gateway +// consumer rewrites UserID to the merged tenant identity (e.g. +// "viettx") for DMs via ContactCollector.ResolveTenantUserID, so +// ownership/audit records use the cross-channel stable identity +// rather than a channel-specific raw sender (e.g. "386246614"). +// SenderID is used only as a fallback when UserID is empty. // // Use for: // - permission checks (file writers, role gates) @@ -213,10 +226,19 @@ func SenderIDFromContext(ctx context.Context) string { // - memory / KG / session scope (use UserIDFromContext / MemoryUserID / KGUserID) // - file-system or per-scope isolation (scope = group principal on purpose) func ActorIDFromContext(ctx context.Context) string { - if sid := SenderIDFromContext(ctx); sid != "" { - return sid + uid := UserIDFromContext(ctx) + // Group/guild: UserID is the scope namespace, not an actor. Prefer SenderID. + if strings.HasPrefix(uid, "group:") || strings.HasPrefix(uid, "guild:") { + if sid := SenderIDFromContext(ctx); sid != "" { + return sid + } + return uid } - return UserIDFromContext(ctx) + // DM / HTTP / cron: UserID is (possibly merged) actor identity. + if uid != "" { + return uid + } + return SenderIDFromContext(ctx) } // WithSelfEvolve returns a new context with the self-evolve flag. diff --git a/internal/store/context_actor_id_test.go b/internal/store/context_actor_id_test.go index de9bd7c5..9bae6d32 100644 --- a/internal/store/context_actor_id_test.go +++ b/internal/store/context_actor_id_test.go @@ -5,29 +5,64 @@ import ( "testing" ) -func TestActorIDFromContext_SenderSetReturnsSender(t *testing.T) { +// In group context, SenderID (the individual sender) is the actor — +// UserID is the shared group namespace, not a user identity. +func TestActorIDFromContext_GroupUsesSender(t *testing.T) { ctx := WithUserID(context.Background(), "group:telegram:-100123") ctx = WithSenderID(ctx, "42") if got := ActorIDFromContext(ctx); got != "42" { - t.Errorf("sender_set: got %q, want %q", got, "42") + t.Errorf("group_uses_sender: got %q, want %q", got, "42") } } -func TestActorIDFromContext_SenderEmptyFallbackToUser(t *testing.T) { +// Guild scope mirrors group scope — SenderID wins. +func TestActorIDFromContext_GuildUsesSender(t *testing.T) { + ctx := WithUserID(context.Background(), "guild:1234:user:5678") + ctx = WithSenderID(ctx, "5678") + if got := ActorIDFromContext(ctx); got != "5678" { + t.Errorf("guild_uses_sender: got %q, want %q", got, "5678") + } +} + +// DM context — UserID is the (possibly tenant-merged) actor identity; +// SenderID is the raw channel-specific sender. Prefer UserID so +// cross-channel merged identities ("viettx") are preserved over raw +// numeric senders ("386246614"). +func TestActorIDFromContext_DMPrefersUserIDForTenantMerge(t *testing.T) { + ctx := WithUserID(context.Background(), "viettx") + ctx = WithSenderID(ctx, "386246614") + if got := ActorIDFromContext(ctx); got != "viettx" { + t.Errorf("dm_merge: got %q, want %q (merged tenant identity should win in DM)", got, "viettx") + } +} + +// DM without merge — UserID == SenderID. Either works. +func TestActorIDFromContext_DMUnmergedReturnsUser(t *testing.T) { + ctx := WithUserID(context.Background(), "386246614") + ctx = WithSenderID(ctx, "386246614") + if got := ActorIDFromContext(ctx); got != "386246614" { + t.Errorf("dm_unmerged: got %q, want %q", got, "386246614") + } +} + +// Group with no SenderID — fall back to UserID (the group principal). +// Rare but avoids returning empty when only scope is known. +func TestActorIDFromContext_GroupFallbackToUserWhenSenderEmpty(t *testing.T) { + ctx := WithUserID(context.Background(), "group:telegram:-100123") + if got := ActorIDFromContext(ctx); got != "group:telegram:-100123" { + t.Errorf("group_fallback: got %q, want %q", got, "group:telegram:-100123") + } +} + +// HTTP/cron — no SenderID. UserID is the actor. +func TestActorIDFromContext_HTTPReturnsUserID(t *testing.T) { ctx := WithUserID(context.Background(), "user-7") if got := ActorIDFromContext(ctx); got != "user-7" { - t.Errorf("fallback_to_user: got %q, want %q", got, "user-7") - } -} - -func TestActorIDFromContext_BothSetSenderWins(t *testing.T) { - ctx := WithUserID(context.Background(), "group:discord:G|U") - ctx = WithSenderID(ctx, "123456") - if got := ActorIDFromContext(ctx); got != "123456" { - t.Errorf("both_set_sender_wins: got %q, want %q", got, "123456") + t.Errorf("http_user_id: got %q, want %q", got, "user-7") } } +// Neither set — empty. func TestActorIDFromContext_NeitherSetReturnsEmpty(t *testing.T) { ctx := context.Background() if got := ActorIDFromContext(ctx); got != "" { @@ -35,11 +70,20 @@ func TestActorIDFromContext_NeitherSetReturnsEmpty(t *testing.T) { } } -// SenderID with "|" delimiter (Discord guild:user|user format) is returned as-is — -// ActorIDFromContext is not responsible for numeric split; callers do that themselves -// (see config_permission_store.go:68 strings.SplitN(senderID, "|", 2)[0]). -func TestActorIDFromContext_DelimitedSenderReturnedAsIs(t *testing.T) { - ctx := WithSenderID(context.Background(), "42|extra") +// Only SenderID set (no UserID) — fall back to SenderID. +func TestActorIDFromContext_SenderOnlyFallback(t *testing.T) { + ctx := WithSenderID(context.Background(), "42") + if got := ActorIDFromContext(ctx); got != "42" { + t.Errorf("sender_only: got %q, want %q", got, "42") + } +} + +// Delimited sender ("42|extra") is returned as-is in group context; +// callers that need a numeric ID do the split themselves +// (config_permission_store.go splits on "|" before lookup). +func TestActorIDFromContext_GroupDelimitedSenderAsIs(t *testing.T) { + ctx := WithUserID(context.Background(), "group:telegram:-100123") + ctx = WithSenderID(ctx, "42|extra") if got := ActorIDFromContext(ctx); got != "42|extra" { t.Errorf("delimited_sender_as_is: got %q, want %q", got, "42|extra") }