Repository navigation
Stop saying an agent can never be in a Ping - #853
Conversation
Basecamp now lets an agent be in a Ping with a person who can use Pings (Circle.with_a_person?), and lists the agents a person can reach among the pingable people (Person::ReachablePeople, principals). Two comments still said agents can never be pinged. Say what holds now, and why the project-scope fallback still matters: the pingable set holds the agents a person can reach, not every agent on the project. No behaviour changes.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28092f5c51
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
|
@codex review |
|
@cubic-dev-ai review |
@jeremy I have started the AI code review. It will take a few minutes to complete. |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Review state at head 03fc106
Fixed after review: the resolver's own comments in |
Two comments in the mention code still say an agent can never be in a Ping. That stopped being true when bc3 shipped agents in Pings (bc3 #13444, #13446, #13448).
What bc3 does now, on origin/master:
app/models/circle.rb,Circle.with_a_person?ispeople.any? { it.user? && it.can_ping? }: an agent can start a Ping, and write in one, when a person who can use Pings is in it.CirclesControllerallows the agent principal oncreate.Circles::PeopleController#indexlistsreachable_people.principals, andprincipalsisUserandAgent. So the pingable set holds the agents a person can reach.What changes here:
internal/commands/helpers.go: the comment now says the pingable set holds the agents a person can reach, not every agent on a project, which is why the project-scope fallback is still consulted.internal/commands/mention_agents_test.go: the comment now says the fake leaves the agent out of the pingable set so that the fallback is what finds it. It no longer claims Basecamp forbids agents in Pings.Comments only, no behaviour change. Whether mention resolution should lean on the pingable set for agents now is a separate question, left alone here.
Summary by cubic
Updates the comments in the mention code that still claimed agents can never be in a Ping, which stopped being true when Basecamp shipped agents in Pings. The pingable set now lists the agents a person can reach, not every agent on a project, so the project-scope fallback is still needed. Corrects the resolver's own comments on the same point. Comments only; no behavior change.
Written for commit 03fc106. Summary will update on new commits.