diff --git a/cmd/gateway_announce_queue.go b/cmd/gateway_announce_queue.go index 561f067d13..6c21d7b8c6 100644 --- a/cmd/gateway_announce_queue.go +++ b/cmd/gateway_announce_queue.go @@ -37,13 +37,22 @@ func enqueueAnnounce(key string, entry announceEntry) bool { // announceRouting holds the shared routing info captured by the first goroutine. type announceRouting struct { - LeadAgent string - LeadSessionKey string - OrigChannel string - OrigChatID string - OrigPeerKind string - OrigLocalKey string - OriginUserID string + LeadAgent string + LeadSessionKey string + OrigChannel string + OrigChatID string + OrigPeerKind string + OrigLocalKey string + OriginUserID string + // OriginSenderID and OriginRole carry the real acting human's identity + // from the original team-task dispatch. Without them, the Lead's session + // resumes from this re-ingress with an empty SenderID, which then fails + // CheckFileWriterPermission / CheckCronPermission in group contexts + // ("system context cannot write files in group chats"). The subagent + // announce queue (subagentAnnounceRouting) already carries these fields + // — this keeps the team-task announce queue at parity. (#915 follow-up) + OriginSenderID string + OriginRole string TeamID string TeamWorkspace string OriginTraceID string @@ -83,13 +92,20 @@ func processAnnounceLoop( content := buildMergedAnnounceContent(entries, snapshot, r.TeamWorkspace) req := agent.RunRequest{ - SessionKey: r.LeadSessionKey, - Message: content, - Channel: r.OrigChannel, - ChatID: r.OrigChatID, - PeerKind: r.OrigPeerKind, - LocalKey: r.OrigLocalKey, - UserID: r.OriginUserID, + SessionKey: r.LeadSessionKey, + Message: content, + Channel: r.OrigChannel, + ChatID: r.OrigChatID, + PeerKind: r.OrigPeerKind, + LocalKey: r.OrigLocalKey, + UserID: r.OriginUserID, + // SenderID + Role propagate the original human acting through this + // team-task announce. loop_context.injectContext gates WithSenderID + // on req.SenderID being non-empty, so missing them silently strips + // the Lead's identity on resume — and group-scoped permission + // checks then deny write_file etc. (#915 follow-up) + SenderID: r.OriginSenderID, + Role: r.OriginRole, RunID: fmt.Sprintf("teammate-announce-%s-%d", r.LeadAgent, len(entries)), RunKind: "announce", HideInput: true, diff --git a/cmd/gateway_announce_routing_test.go b/cmd/gateway_announce_routing_test.go new file mode 100644 index 0000000000..681ce47b1f --- /dev/null +++ b/cmd/gateway_announce_routing_test.go @@ -0,0 +1,74 @@ +package cmd + +import ( + "testing" + + "github.com/nextlevelbuilder/goclaw/internal/tools" +) + +// TestAnnounceRouting_PropagatesSenderAndRole guards against the regression +// where team-task completion announces drop SenderID/Role on re-ingress to +// the Lead session — the failure mode reported as +// `permission denied: system context cannot write files in group chats` +// when the Lead tries write_file inside the announce-triggered turn. +// +// team_tool_dispatch.go already stores MetaOriginSenderID + MetaOriginRole +// at dispatch time. The bug: announceRouting + the RunRequest it builds +// must read these back from inMeta on completion, otherwise loop_context +// skips WithSenderID and the Lead's resume has empty sender attribution. +func TestAnnounceRouting_PropagatesSenderAndRole(t *testing.T) { + const ( + realSender = "5218954741" // Telegram numeric user id + realRole = "admin" + realUserID = "group:telegram:-1003812294018" + ) + + // Simulate what consumer_handlers.go reads from a teammate-message inMeta. + inMeta := map[string]string{ + tools.MetaOriginSenderID: realSender, + tools.MetaOriginRole: realRole, + tools.MetaOriginUserID: realUserID, + tools.MetaTeamID: "019d8a59-6e40-730f-89b2-8a41b7e1fad2", + } + + r := announceRouting{ + OriginUserID: inMeta[tools.MetaOriginUserID], + OriginSenderID: inMeta[tools.MetaOriginSenderID], + OriginRole: inMeta[tools.MetaOriginRole], + TeamID: inMeta[tools.MetaTeamID], + } + + if r.OriginSenderID != realSender { + t.Fatalf("OriginSenderID = %q, want %q (team-task announce dropped sender attribution)", + r.OriginSenderID, realSender) + } + if r.OriginRole != realRole { + t.Fatalf("OriginRole = %q, want %q (team-task announce dropped RBAC role)", + r.OriginRole, realRole) + } + if r.OriginUserID != realUserID { + t.Fatalf("OriginUserID = %q, want %q", r.OriginUserID, realUserID) + } +} + +// TestAnnounceRouting_EmptyMetaPropagatesEmpty asserts the wire-through is +// faithful when upstream legitimately has no sender (e.g. a system-initiated +// dispatch). We must NOT fabricate a synthetic sender just because the field +// is empty — that would defeat the deny-on-empty guard in +// CheckFileWriterPermission. +func TestAnnounceRouting_EmptyMetaPropagatesEmpty(t *testing.T) { + inMeta := map[string]string{ + tools.MetaTeamID: "team-uuid", + } + r := announceRouting{ + OriginUserID: inMeta[tools.MetaOriginUserID], + OriginSenderID: inMeta[tools.MetaOriginSenderID], + OriginRole: inMeta[tools.MetaOriginRole], + } + if r.OriginSenderID != "" { + t.Errorf("OriginSenderID = %q, want empty (no upstream sender to propagate)", r.OriginSenderID) + } + if r.OriginRole != "" { + t.Errorf("OriginRole = %q, want empty", r.OriginRole) + } +} diff --git a/cmd/gateway_consumer_handlers.go b/cmd/gateway_consumer_handlers.go index 66216cfeea..36baef0b4a 100644 --- a/cmd/gateway_consumer_handlers.go +++ b/cmd/gateway_consumer_handlers.go @@ -363,13 +363,19 @@ func handleTeammateMessage( } routing := announceRouting{ - LeadAgent: leadAgent, - LeadSessionKey: leadSessionKey, - OrigChannel: origCh, - OrigChatID: origChatID, - OrigPeerKind: origPeerKind, - OrigLocalKey: origLocalKey, - OriginUserID: inMeta[tools.MetaOriginUserID], + LeadAgent: leadAgent, + LeadSessionKey: leadSessionKey, + OrigChannel: origCh, + OrigChatID: origChatID, + OrigPeerKind: origPeerKind, + OrigLocalKey: origLocalKey, + OriginUserID: inMeta[tools.MetaOriginUserID], + // Carry the real acting sender + role through the team-task + // announce so the Lead's resumed turn doesn't lose attribution + // and trip group-scope permission checks. team_tool_dispatch.go + // already populates these fields in the dispatch metadata. (#915) + OriginSenderID: inMeta[tools.MetaOriginSenderID], + OriginRole: inMeta[tools.MetaOriginRole], TeamID: inMeta[tools.MetaTeamID], TeamWorkspace: inMeta[tools.MetaTeamWorkspace], OriginTraceID: inMeta[tools.MetaOriginTraceID],