Skip to content

Commit c39c31e

Browse files
committed
fix: a gateway silence read as "not my conversation"
The gateway response carried only whether a reply came back, and the forwarder used that to decide whether to suppress AstrBot's own model. But this agent is quiet on purpose far more often than it speaks: a PASS, several messages merged into one answer by the debounce, the rhythm gate. All of those return handled=false. So the forwarder read the persona's deliberate restraint as "not mine" and handed the room to its built-in model, which then answered in it as someone else. That is worse than not replying — the restraint is the behaviour being overridden, and it is the most common outcome by design. The response now carries `owned` beside `handled`, set once the turn clears the admission gates, which is the moment the answer to "is this conversation mine" is known. Everything after that point is about what to say, including saying nothing. The plugin gates stop_event() on `owned` and falls back to `handled` when the field is absent, so an older agent behaves exactly as it does today — pinned by the existing unhandled-response test. Mutation-checked: conflating the two again (the pre-change semantics) fails the assertion that a PASS still claims the conversation, and claiming every turn from the start fails the assertion that a refused one does not. Also documents the second inbound path, which nothing did. docs/deploy.md described a QQ-only deployment and never mentioned the gateway, AstrBot, or how to configure a second platform — the interface was configurable and undiscoverable. Includes the two AstrBot stages that run before plugin handlers and would throttle or silently drop a busy QQ group.
1 parent 2406c4a commit c39c31e

10 files changed

Lines changed: 235 additions & 26 deletions

File tree

CHANGELOG.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,19 @@ before it was changed and is covered by a test that fails without the fix.
2121
that is not a directory, an empty `BOT_NAME`, and any key `.env.example`
2222
does not list. Reported, never fatal — a deployment that is 90% configured
2323
should start and say what the other 10% is.
24+
- **`GATEWAY_NATIVE_PLATFORMS` — one forwarder can carry QQ too.** The gateway
25+
namespaces every id as `<platform>:<raw>` so a forwarded identity can never
26+
collide with a real QQ number. Right for Telegram, exactly wrong for QQ
27+
itself: routing QQ through the same door renamed every conversation, so
28+
memory, history and every candidate scope pointed at rooms and people that
29+
do not exist — and not repairably, because the ledgers content-address their
30+
rows over `conv_id`, so the rename moves every id derived from it. Naming a
31+
platform here makes its ids arrive bare, identical to NapCat's. Empty by
32+
default. It is an operator setting rather than something the forwarder
33+
asserts, because a bare id is the spelling `OWNER_QQ`, `QQ_GROUPS` and
34+
`PRIVATE_ALLOWED_QQS` are written in — and for the same reason those
35+
whitelists now gate on the id's shape rather than on the sink, so a native
36+
forwarder cannot both claim QQ authority and skip the QQ gate.
2437
- **`.env.example` is checked against the code.** The typo check treats the
2538
template as the authority on what a key may be called, so a test scans every
2639
`os.getenv` / `os.environ.get` in the package and asserts the template
@@ -189,6 +202,15 @@ before it was changed and is covered by a test that fails without the fix.
189202
- `.env.example` documents `AGENT_HOME`, `LLM_TIMEOUT`, `LLM_MAX_RETRIES`,
190203
`MAX_INFLIGHT_GATEWAY` and the two ledger warn-byte settings.
191204

205+
- **A gateway reply of silence no longer reads as "not my conversation".**
206+
The response said only whether a reply came back, and the forwarder used
207+
that to decide whether to suppress its own model. But this agent stays quiet
208+
on purpose far more often than it speaks — a PASS, several messages merged
209+
into one answer, the rhythm gate — so AstrBot's built-in model answered in
210+
rooms the persona had deliberately sat out, as someone else. The response
211+
now carries `owned` alongside `handled`, set once the turn clears admission;
212+
the plugin gates on that and falls back to `handled` against an older agent.
213+
192214
### Performance
193215

194216
Both items were measured before and after; the two that an audit also flagged

docs/deploy.md

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,43 @@ If (1) fails, the agent can never send. If (1) works and (2) never logs
8888
anything, the bridge's webhook is not configured or is pointed elsewhere. Both
8989
failures look identical from the outside — a bot that is running and silent.
9090

91+
## More than one platform
92+
93+
Everything above is the QQ path. For Telegram, Discord, Slack and the rest,
94+
the agent does not connect to the platform at all — a forwarder does, and
95+
POSTs a platform-neutral event to `POST /webhook/gateway`. The one in this
96+
repo is an [AstrBot](https://github.com/AstrBotDevs/AstrBot) plugin at
97+
`integrations/astrbot/astrbot_plugin_llm_persona_gateway/`; copy it into
98+
AstrBot's `data/plugins/`, and configure the platforms in AstrBot's own UI.
99+
The persona, memory, learning and typing simulation stay here.
100+
101+
Two settings and one decision:
102+
103+
- `GATEWAY_TOKEN` — shared with the plugin. Required off-host, and the
104+
request carries an HMAC-SHA256 envelope over `timestamp.nonce.body` with a
105+
replay guard, so a token seen in a log is not enough on its own.
106+
- `GATEWAY_OWNER_IDS` — platform-prefixed ids (`telegram:12345`) that get the
107+
owner branch in DMs. Gateway identities are namespaced `<platform>:<id>` so
108+
they can never collide with a QQ number.
109+
- The decision: **does QQ go through the forwarder too?**
110+
111+
Leave QQ on NapCat and you have two inbound paths but nothing to reconcile.
112+
Route it through the forwarder and there is one door and one place to
113+
configure platforms — but then set `GATEWAY_NATIVE_PLATFORMS` to the
114+
forwarder's QQ adapter name (`aiocqhttp` for AstrBot), or every QQ
115+
conversation arrives under a namespaced name it has never had before. Memory,
116+
history and every learned example are keyed the bare way, and the evidence and
117+
candidate ledgers content-address their rows over the conversation id — so the
118+
rename changes every id derived from it and cannot be undone by rewriting a
119+
field. `GATEWAY_NATIVE_PLATFORMS` keeps the ids identical to NapCat's.
120+
121+
Naming a platform there grants its forwarder QQ authority, since bare ids are
122+
what `OWNER_QQ`, `QQ_GROUPS` and `PRIVATE_ALLOWED_QQS` are compared against.
123+
Which is why those whitelists then apply to it, unlike to a namespaced
124+
platform — the forwarder's own allowlist is not the only filter any more.
125+
126+
Do not run both doors for QQ at once; the same message would arrive twice.
127+
91128
## Exposing the webhook
92129

93130
Keep the default loopback binding when the bridge and the agent share a

integrations/astrbot/astrbot_plugin_llm_persona_gateway/README.md

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,41 @@ loopback URL visible to AstrBot; never send the token over cleartext HTTP.
5353
| `group_whitelist` | list | `[]` | Group IDs to forward; empty = none. |
5454
| `private_enabled` | bool | `false` | Enable forwarding for explicitly allowlisted private senders. |
5555
| `private_whitelist` | list | `[]` | Allowed private senders; empty = none. |
56-
| `block_default` | bool | `true` | Call `event.stop_event()` only after the agent successfully accepts ownership (`handled: true`). Transport failures, invalid responses, and `handled: false` fall back to AstrBot's normal pipeline. |
57-
58-
## Important: QQ / NapCat double-handling
59-
60-
If NapCat already feeds the agent directly through `POST /webhook/qq`, keep
61-
`aiocqhttp` in `excluded_platforms` (it is there by default). Otherwise the
62-
same QQ message would reach the agent twice — once from NapCat and once from
63-
this plugin.
56+
| `block_default` | bool | `true` | Call `event.stop_event()` once the agent claims the conversation (`owned: true`). Transport failures, invalid responses and conversations the agent turned away fall back to AstrBot's normal pipeline. |
57+
58+
`owned` is not `handled`. The agent stays quiet on purpose far more often
59+
than it speaks — a PASS, several messages merged into one answer, the rhythm
60+
gate — and all of those return `handled: false`. Gating on that would hand
61+
the room to AstrBot's built-in model, which would then answer as someone else
62+
in a conversation the persona had decided to sit out. An agent too old to
63+
send `owned` falls back to `handled`, which is what it did before.
64+
65+
## QQ: two ways, and you must pick one
66+
67+
**Default — NapCat feeds the agent directly.** `aiocqhttp` stays in
68+
`excluded_platforms`, QQ goes NapCat → `POST /webhook/qq`, and this plugin
69+
carries everything else. Nothing to configure.
70+
71+
**Or route QQ through here too**, so AstrBot is the single place you configure
72+
every platform. Remove `aiocqhttp` from `excluded_platforms`, add the QQ group
73+
to `group_whitelist`, stop NapCat posting to `/webhook/qq`, and set
74+
`GATEWAY_NATIVE_PLATFORMS=aiocqhttp` on the agent.
75+
76+
That last setting is not optional and not cosmetic. Without it the agent
77+
namespaces forwarded ids, so every QQ conversation arrives under a new name
78+
and the agent addresses rooms and people that do not exist — memory, history
79+
and every learned example are keyed the old way, and the ledgers
80+
content-address their rows over the conversation id, so it cannot be renamed
81+
back afterwards. With it, a QQ message relayed by AstrBot lands on exactly the
82+
keys NapCat would have produced.
83+
84+
Do **not** do both at once: the same message would reach the agent twice.
85+
86+
Two things to change on the AstrBot side before it can carry a busy QQ group,
87+
because both run before plugin handlers: raise or disable the rate-limit stage
88+
(30 messages / 60 s by default, and it stalls rather than drops), and review
89+
`content_safety.internal_keywords`, which is on by default and will silently
90+
drop messages the persona would have answered.
6491

6592
## Request authentication
6693

integrations/astrbot/astrbot_plugin_llm_persona_gateway/_conf_schema.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
},
1717
"excluded_platforms": {
1818
"type": "list",
19-
"description": "Platform adapter names that must NOT be forwarded. Keep aiocqhttp here when NapCat already feeds the agent directly, otherwise QQ messages would be handled twice.",
19+
"description": "Platform adapter names that must NOT be forwarded. Keep aiocqhttp here when NapCat already feeds the agent directly through /webhook/qq, otherwise QQ messages would be handled twice. To make AstrBot the single inbound path for QQ as well, remove it, stop NapCat posting to /webhook/qq, and set GATEWAY_NATIVE_PLATFORMS=aiocqhttp on the agent so QQ ids stay spelled the way every store on disk already spells them.",
2020
"default": ["aiocqhttp"]
2121
},
2222
"group_whitelist": {

integrations/astrbot/astrbot_plugin_llm_persona_gateway/main.py

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ async def forward_to_agent(self, event: AstrMessageEvent):
162162
"raw_text": raw_text,
163163
}
164164

165-
delivered, replies = await self._post_to_agent(neutral_event)
165+
_replied, owned, replies = await self._post_to_agent(neutral_event)
166166

167167
first = True
168168
for item in replies:
@@ -176,9 +176,12 @@ async def forward_to_agent(self, event: AstrMessageEvent):
176176
first = False
177177
yield event.chain_result(chain)
178178

179-
if delivered and self.config.get("block_default", True):
180-
# The agent owns these conversations: keep AstrBot's built-in
181-
# LLM pipeline from producing a second reply.
179+
if owned and self.config.get("block_default", True):
180+
# Gate on ownership, not on whether a reply came back. The agent
181+
# stays quiet on purpose far more often than it speaks — PASS, a
182+
# debounce merge, the rhythm gate — and reading that as "not mine"
183+
# hands the conversation to AstrBot's built-in model, which then
184+
# answers as someone else in a room this persona chose to sit out.
182185
event.stop_event()
183186

184187
def _map_segments(self, event: AstrMessageEvent, self_id: str):
@@ -278,14 +281,23 @@ def _endpoint_is_allowed(url: str, token: str) -> tuple[bool, str]:
278281
return False, "off-host agent_url requires a non-empty gateway_token"
279282
return True, ""
280283

281-
async def _post_to_agent(self, neutral_event: dict) -> tuple[bool, list]:
284+
async def _post_to_agent(
285+
self, neutral_event: dict) -> tuple[bool, bool, list]:
286+
"""POST one event; return (replied, owned, reply items).
287+
288+
`owned` is the agent's answer to "is this conversation mine", which is
289+
NOT the same as whether it replied: a PASS, a debounce merge and a
290+
rhythm-gate skip are all deliberate silence in a conversation it owns.
291+
Falling back to `handled` keeps an older agent — one that does not send
292+
the field — behaving exactly as it does today.
293+
"""
282294
url = str(self.config.get("agent_url") or DEFAULT_AGENT_URL)
283295
timeout = float(self.config.get("timeout_s") or DEFAULT_TIMEOUT_S)
284296
token = str(self.config.get("gateway_token") or "")
285297
allowed, reason = self._endpoint_is_allowed(url, token)
286298
if not allowed:
287299
logger.warning(f"llm_persona_gateway: refusing unsafe agent_url: {reason}")
288-
return False, []
300+
return False, False, []
289301

290302
body = json.dumps(
291303
neutral_event,
@@ -317,11 +329,12 @@ async def _post_to_agent(self, neutral_event: dict) -> tuple[bool, list]:
317329
data = resp.json()
318330
except Exception as e:
319331
logger.warning(f"llm_persona_gateway: agent request failed: {e}")
320-
return False, []
332+
return False, False, []
321333
replies = data.get("replies") if isinstance(data, dict) else None
322334
if not isinstance(replies, list):
323-
return False, []
324-
return bool(data.get("handled")), [
335+
return False, False, []
336+
handled = bool(data.get("handled"))
337+
return handled, bool(data.get("owned", handled)), [
325338
r for r in replies if isinstance(r, dict)
326339
]
327340

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: astrbot_plugin_llm_persona_gateway
22
desc: Forwards messages from any AstrBot platform adapter to an external persona LLM agent (personagent) and relays its replies back.
3-
version: 0.2.0
3+
version: 0.3.0
44
author: wangkant
55
repo: https://github.com/wangkant/personagent

persona_agent/agent.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -759,7 +759,22 @@ async def handle_gateway(self, event: dict) -> dict:
759759
self._gateway_inflight[gateway_key] = remaining
760760
else:
761761
self._gateway_inflight.pop(gateway_key, None)
762-
return {"handled": bool(handled), "replies": sink.items}
762+
# `owned` is not `handled`. See GatewaySink: a forwarder needs to know
763+
# whether to suppress its own model, and "produced no reply" is the
764+
# wrong signal for that — silence is frequently the persona's answer.
765+
return {"handled": bool(handled), "owned": sink.owned,
766+
"replies": sink.items}
767+
768+
def _claim_gateway_turn(self) -> None:
769+
"""Mark the current gateway turn as ours, whatever it decides to say.
770+
771+
Called once the admission gates pass, which is the moment the answer
772+
to "is this conversation mine" is known — everything after it is about
773+
what to say, including saying nothing.
774+
"""
775+
sink = current_sink.get()
776+
if sink is not None:
777+
sink.owned = True
763778

764779
async def _handle_inner(self, payload: dict) -> bool:
765780
if not self.enabled:
@@ -806,6 +821,7 @@ async def _handle_inner(self, payload: dict) -> bool:
806821
if current_sink.get() is None or channels.is_native(user_id):
807822
if not is_owner and user_id not in self.private_allowed_qqs:
808823
return False
824+
self._claim_gateway_turn()
809825
if mid is not None:
810826
self._remember_msg_id(mid)
811827
# Gateway DM keys are forwarder-chosen → register in the LRU so an
@@ -827,6 +843,7 @@ async def _handle_inner(self, payload: dict) -> bool:
827843
and (current_sink.get() is None or channels.is_native(group_id)) \
828844
and group_id not in self.allowed_groups:
829845
return False
846+
self._claim_gateway_turn()
830847
if mid is not None:
831848
self._remember_msg_id(mid)
832849
# Gateway group keys are forwarder-chosen → register in the LRU.

persona_agent/gateway.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,13 @@ class GatewaySink:
152152
def __init__(self) -> None:
153153
self.items: list[dict] = []
154154
self.closed = False
155+
# Set once this turn clears the admission gates. It answers a
156+
# different question than `items`: whether the conversation is OURS,
157+
# not whether we chose to speak in it. A forwarder that conflates the
158+
# two hands every deliberate silence — a PASS, a debounce merge, a
159+
# rhythm-gate skip — to its own built-in model, which then answers as
160+
# someone else in a conversation this persona had decided to sit out.
161+
self.owned = False
155162

156163
def add(self, message) -> bool:
157164
if self.closed:

tests/test_astrbot_plugin.py

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,17 @@ async def post(self, url, **kwargs):
140140
return response
141141

142142

143+
class _SilentButOwnedClient(_RecordingClient):
144+
"""An agent that took the conversation and chose to say nothing."""
145+
146+
async def post(self, url, **kwargs):
147+
self.calls.append((url, kwargs))
148+
response = _Response()
149+
response.json = lambda: {
150+
"handled": False, "owned": True, "replies": []}
151+
return response
152+
153+
143154
class _Event:
144155
def __init__(self, module, *, private: bool):
145156
message_type = (
@@ -239,7 +250,7 @@ def test_signed_request_uses_canonical_body_and_replay_headers():
239250
).hexdigest()
240251

241252
try:
242-
delivered, replies = asyncio.run(plugin._post_to_agent(event))
253+
delivered, _owned, replies = asyncio.run(plugin._post_to_agent(event))
243254
finally:
244255
module.time.time = real_time
245256
module.secrets.token_hex = real_token_hex
@@ -275,9 +286,9 @@ def test_off_host_endpoint_requires_https_and_a_token():
275286
)
276287

277288
assert asyncio.run(
278-
insecure._post_to_agent({"message": "hello"})) == (False, [])
289+
insecure._post_to_agent({"message": "hello"})) == (False, False, [])
279290
assert asyncio.run(
280-
no_token._post_to_agent({"message": "hello"})) == (False, [])
291+
no_token._post_to_agent({"message": "hello"})) == (False, False, [])
281292
assert insecure._client.calls == []
282293
assert no_token._client.calls == []
283294

@@ -293,7 +304,7 @@ def test_malformed_endpoint_is_rejected_without_a_request():
293304
)
294305

295306
assert asyncio.run(
296-
plugin._post_to_agent({"message": "hello"})) == (False, [])
307+
plugin._post_to_agent({"message": "hello"})) == (False, False, [])
297308
assert plugin._client.calls == []
298309

299310

@@ -310,7 +321,7 @@ def test_forwarding_failure_does_not_stop_astrbot_fallback():
310321
event = _Event(module, private=True)
311322

312323
async def fail(_neutral_event):
313-
return False, []
324+
return False, False, []
314325

315326
plugin._post_to_agent = fail
316327

@@ -341,6 +352,31 @@ async def collect():
341352
assert event.stopped is False
342353

343354

355+
def test_a_silent_but_owned_conversation_blocks_the_fallback():
356+
"""The agent is quiet far more often than it speaks — a PASS, a debounce
357+
merge, the rhythm gate. Treating that as "not mine" hands the room to
358+
AstrBot's own model, which answers in it as someone else. The test above
359+
pins the other direction: an agent too old to send `owned` still falls
360+
back to `handled`, so nothing changes for it."""
361+
module = _import_plugin()
362+
plugin = _plugin_instance(
363+
module,
364+
{
365+
"private_enabled": True,
366+
"private_whitelist": ["user-1"],
367+
"block_default": True,
368+
},
369+
)
370+
plugin._client = _SilentButOwnedClient()
371+
event = _Event(module, private=True)
372+
373+
async def collect():
374+
return [item async for item in plugin.forward_to_agent(event)]
375+
376+
assert asyncio.run(collect()) == []
377+
assert event.stopped is True
378+
379+
344380
def test_forwarded_event_carries_source_timestamp_and_success_blocks_fallback():
345381
module = _import_plugin()
346382
plugin = _plugin_instance(
@@ -356,7 +392,7 @@ def test_forwarded_event_carries_source_timestamp_and_success_blocks_fallback():
356392

357393
async def succeed(neutral_event):
358394
captured.update(neutral_event)
359-
return True, []
395+
return True, True, []
360396

361397
plugin._post_to_agent = succeed
362398

@@ -398,6 +434,7 @@ async def collect():
398434
test_malformed_endpoint_is_rejected_without_a_request,
399435
test_forwarding_failure_does_not_stop_astrbot_fallback,
400436
test_unhandled_gateway_response_does_not_stop_astrbot_fallback,
437+
test_a_silent_but_owned_conversation_blocks_the_fallback,
401438
test_forwarded_event_carries_source_timestamp_and_success_blocks_fallback,
402439
test_missing_source_timestamp_is_not_forwarded_or_blocked,
403440
]

0 commit comments

Comments
 (0)