Repository navigation
Conversation
Agent posts into group conversations, and agent participant rows in them, now follow the agent's message mode: - none: never posts to or joins a group conversation. - project, hub: unchanged; posting adds the agent as a participant. - lineage, branch: posts only to a group conversation it already belongs to; posting never adds it. It is added only by a human whom its mode lets message it (ancestry user, project owner, super-admin). One shared decision (group_message_mode.go) is used by the outbound and agent-message handlers, the participant API, conversation create, and mention fan-out. Refusals return 403 message_denied. Tracked in #4413
- check the thread conversation exists before resolving it for lineage and branch senders; refuse none and unknown modes directly - read the inbound thread sender once - decide human adders by identity type - answer 500 on a sender lookup fault - tests: mention registration, new thread with no conversation written, create writes no conversation on refusal, concrete success statuses - docs: posting by conversation ID adds project and hub agents
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements message mode authorization rules for agents in group conversations, defining how different modes (none, project, hub, lineage, and branch) govern posting and joining behaviors. It introduces the core logic in group_message_mode.go, integrates these checks into the messaging and conversation handlers, and adds corresponding tests and documentation. The review feedback identifies two potential nil pointer dereferences in handlers_conversations.go where GetAgent results are accessed without verifying if the returned agent is nil.
| if ai, ok := identity.(AgentIdentity); ok { | ||
| creator, err := s.store.GetAgent(ctx, ai.ID()) | ||
| if err != nil { | ||
| writeErrorFromErr(w, err, "") | ||
| return | ||
| } | ||
| if joinOK, joinReason := s.groupAgentJoinAllowed(ctx, identity, creator); !joinOK { | ||
| writeGroupModeDenied(w, joinReason, creator.MessageMode) | ||
| return | ||
| } | ||
| } |
There was a problem hiding this comment.
Defensive programming check: s.store.GetAgent can return a nil agent even if err is nil. If creator is nil, calling creator.MessageMode will cause a nil pointer dereference panic. Please add a nil check for creator before accessing its fields.
| if ai, ok := identity.(AgentIdentity); ok { | |
| creator, err := s.store.GetAgent(ctx, ai.ID()) | |
| if err != nil { | |
| writeErrorFromErr(w, err, "") | |
| return | |
| } | |
| if joinOK, joinReason := s.groupAgentJoinAllowed(ctx, identity, creator); !joinOK { | |
| writeGroupModeDenied(w, joinReason, creator.MessageMode) | |
| return | |
| } | |
| } | |
| if ai, ok := identity.(AgentIdentity); ok { | |
| creator, err := s.store.GetAgent(ctx, ai.ID()) | |
| if err != nil { | |
| writeErrorFromErr(w, err, "") | |
| return | |
| } | |
| if creator == nil { | |
| writeError(w, http.StatusNotFound, "not_found", "agent not found", nil) | |
| return | |
| } | |
| if joinOK, joinReason := s.groupAgentJoinAllowed(ctx, identity, creator); !joinOK { | |
| writeGroupModeDenied(w, joinReason, creator.MessageMode) | |
| return | |
| } | |
| } |
| // The caller must be allowed to add the agent under the agent's | ||
| // message mode (group_message_mode.go). | ||
| if joinOK, joinReason := s.groupAgentJoinAllowed(ctx, identity, agent); !joinOK { | ||
| writeGroupModeDenied(w, joinReason, agent.MessageMode) | ||
| return | ||
| } |
There was a problem hiding this comment.
Defensive programming check: s.store.GetAgent can return a nil agent even if err is nil. If agent is nil, calling agent.MessageMode will cause a nil pointer dereference panic. Please add a nil check for agent before accessing its fields.
| // The caller must be allowed to add the agent under the agent's | |
| // message mode (group_message_mode.go). | |
| if joinOK, joinReason := s.groupAgentJoinAllowed(ctx, identity, agent); !joinOK { | |
| writeGroupModeDenied(w, joinReason, agent.MessageMode) | |
| return | |
| } | |
| if agent == nil { | |
| writeError(w, http.StatusNotFound, "not_found", "agent not found", nil) | |
| return | |
| } | |
| // The caller must be allowed to add the agent under the agent's | |
| // message mode (group_message_mode.go). | |
| if joinOK, joinReason := s.groupAgentJoinAllowed(ctx, identity, agent); !joinOK { | |
| writeGroupModeDenied(w, joinReason, agent.MessageMode) | |
| return | |
| } |
1 participant
Agent posts to group conversations, and agent participant rows in them, now follow the agent's message mode:
none: never posts to or joins a group conversation.project,hub: unchanged.lineage,branch: post only where they are already a participant, and posting never adds them. They are added only by a human their mode lets message them (ancestry user, project owner, super-admin).One shared check in
pkg/hub/group_message_mode.gocovers the message handlers, the participant API, conversation create and mention fan-out. Refusals return 403message_denied. Docs updated.Tracked in ptone#4413