fix(mentions): the welcome reply teaches the handle, and names the right one - #841
Open
lilyshen0722 wants to merge 1 commit into
Open
fix(mentions): the welcome reply teaches the handle, and names the right one#841lilyshen0722 wants to merge 1 commit into
lilyshen0722 wants to merge 1 commit into
Conversation
…ght one Follow-up to #838, from Sam's review. Two changes, one of them a correction to a design I proposed and he rejected. **The gap.** The wake fires once per member per pod, so a newcomer's SECOND unaddressed message reaches nobody. I proposed a bounded continuation window — keep routing that member's messages to the greeter for N messages or T minutes. That was wrong for reasons worth recording: - It cannot tell a message meant for the agent from one meant for another human. In a pod where people talk to each other, it wakes the agent on everything a newcomer says for the whole window. - With two greeters installed it wakes both, on everything, with no notion of who owns the conversation. - My justification was bad reasoning: I argued it was principled because fires stay bounded by (member x pod) x a constant rather than by traffic. A bound on how many unwarranted wakes occur does not make any of them warranted. That is proxy-thinking — the exact failure #838's own PR text warns about. The real gap is not routing, it is discoverability: the newcomer does not know how to address the agent. So close that instead, at the one moment it lands — in the reply, from the agent that just demonstrably helped them. A pinned room description already failed at this; an in-context line from a working agent will not. **Naming the RIGHT handle, which is not the one the agent knows.** The support agent's token carries `agentName: 'hq-support'`, while the room shows "Commonly Support" and humans type `@commonly-support`. An agent asked to name its own handle answers from its token and hands the user a string they have never seen. So the cue states the handle rather than delegating the guess: `mentionHandleFor` mirrors buildMentionMap's own resolution order — displayName slug first (what the UI renders, therefore what a person copies), instanceId next, agentName last. **The self-mention guard is now load-bearing.** Instructing an agent to write its own handle puts that handle in a message the agent sent. Verified it resolves the way the guard needs: `@commonly-support` maps through BOTH the instanceId key and the displayName slug to (hq-support, commonly-support), which is the pair isSelfMention compares — so it matches and suppresses. That was previously an incidental property and is now a contract, so it has three regression tests, including that a HUMAN typing the same handle still reaches the agent. Mutation-verified: neutering isSelfMention fails all three. Tests: 23 service + 50 mention (3 new). Typecheck clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #838, from Sam's review. Two changes — and one of them is a correction to a design I proposed and he was right to reject.
The gap he found
The wake fires once per member per pod, so a newcomer's second unaddressed message reaches nobody. Only a reply-threaded follow-up routes (via #703), and someone who didn't know to
@mentionalmost certainly doesn't know to reply-thread either.I proposed a bounded continuation window: keep routing that member's messages to the greeter for N messages or T minutes. That was wrong, for reasons worth recording:
I also overstated the problem to justify the fix. "Trained then abandoned" was wrong: message 2 getting silence is not a regression, it is what every other message in the room does today. The honest version is "helped once, then normal" — strictly better than before.
What this does instead
The gap is discoverability, not routing: the newcomer doesn't know how to address the agent. So close that, at the one moment it lands — in the reply, from the agent that just demonstrably helped them. A pinned room description already failed at this; an in-context line from a working agent won't.
The cue now ends with:
No new state, no routing change, degrades gracefully with multiple agents.
Naming the right handle — which is not the one the agent knows
The support agent's token carries
agentName: 'hq-support', while the room renders "Commonly Support" and humans type@commonly-support. An agent asked to name its own handle answers from its token and hands the user a string they have never seen.So the cue states the handle rather than delegating the guess.
mentionHandleFormirrorsbuildMentionMap's own resolution order — displayName slug first (what the UI renders, therefore what a person copies), then instanceId, then agentName.The self-mention guard is now load-bearing
Instructing an agent to write its own handle puts that handle into a message the agent itself sent. Verified it resolves the way the guard requires:
@commonly-supportmaps through both the instanceId key and the displayName slug to(hq-support, commonly-support)— exactly the pairisSelfMentioncompares — so it matches and suppresses.That was previously an incidental property. It is now a contract, so it has three regression tests, including that a human typing the same handle still reaches the agent. Rename a display name carelessly and those tests are what catch the resulting wake loop.
Verification
isSelfMentionfails all three new self-mention tests, so they pin real behavior rather than passing vacuously