Conversation
…e/summon/quiet/lively/hops/cooldown/topic/rename/help)
… re-dispatch for agent-to-agent fanout
…e-by-mention, DM force-respond
…in repo Pre-built LXC base images are deployer infrastructure, not user-facing releases. Move them to a dedicated repo so the main tinyagentos Releases page is reserved for actual taOS releases. Workflow publishes to jaylfc/tinyagentos-images via IMAGES_RELEASE_TOKEN (fine-grained PAT with Contents: write on that repo). Deployer (tinyagentos/agent_image.py) now pulls from the new repo's release URL.
…ents Agent frameworks have their own / command namespaces (/help, /clear, etc.); taOS intercepting / would collide with them indefinitely. Control-plane actions (mute, mode, topic, etc.) move to REST endpoints driven by UI right-click menus instead. See follow-up commits.
📝 WalkthroughWalkthroughReplaces backend slash-command interception with a bare-slash guardrail and UI-driven REST channel admin endpoints; adds mention-aware routing with hop limits and group policy, context-window construction, semantic reaction handling, bridge changes for forced/context replies, channel-store settings, and extensive new tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API as POST /api/chat/messages
participant Parser as parse_mentions()
participant DB as MessageStore
participant Router as AgentChatRouter
participant Policy as GroupPolicy
participant Context as build_context_window()
participant Bridge as BridgeSessionRegistry
participant Agent
User->>API: POST (content, channel_id)
API->>Parser: parse_mentions(content, members)
Parser-->>API: MentionSet
alt Non-DM & bare-slash & no mention
API-->>User: HTTP 400 (must address an agent)
else Valid message
API->>DB: persist message
DB-->>API: persisted_message
API->>Router: dispatch(persisted_message, channel)
Router->>Router: compute recipients (mentions, response_mode, muted, DM)
loop per candidate agent
Router->>Router: compute next_hops
alt next_hops > max_hops && not forced
Router-->>Router: skip agent
else
Router->>Policy: try_acquire(channel_id, agent, settings)
alt denied
Router-->>Router: skip agent
else allowed
Router->>Context: build_context_window(recent_messages)
Context-->>Router: context
Router->>Bridge: enqueue_user_message(agent, context, force_respond, hops)
Bridge->>Agent: deliver message
Agent-->>Bridge: reply stream
Bridge->>DB: persist reply
Bridge->>Router: dispatch(persisted_reply, channel) (re-dispatch)
Router->>Policy: record_send(channel_id, agent)
end
end
end
API-->>User: HTTP 200/201
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Incremental Review Note: Reviewed changes since fd70ab8 — minor optimizations and bug fixes in agent routing and reactions. Files Reviewed (30 files)
Reviewed by grok-code-fast-1:optimized:free · 515,378 tokens |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tinyagentos/scripts/install_openai-agents-sdk.sh (1)
67-69:⚠️ Potential issue | 🟠 Major
channel_idis passed but never sent in the reply payload.Line 79 forwards
channel_id, butpost_reply()ignorescid, so the new channel-aware reply behavior is not actually applied.🐛 Suggested fix
async def post_reply(c, u, t, mid, tid, txt, cid=None): - try: await c.post(u, json={"kind":"final","id":mid,"trace_id":tid,"content":txt}, headers={"Authorization":f"Bearer {t}"}, timeout=30) + payload = {"kind":"final","id":mid,"trace_id":tid,"content":txt} + if cid: + payload["channel_id"] = cid + try: await c.post(u, json=payload, headers={"Authorization":f"Bearer {t}"}, timeout=30) except Exception as e: log.warning("reply: %s", e)Also applies to: 79-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinyagentos/scripts/install_openai-agents-sdk.sh` around lines 67 - 69, The post_reply function is ignoring the cid parameter so channel_id never gets sent; update async def post_reply(c, u, t, mid, tid, txt, cid=None) to include the channel id in the JSON payload (e.g. add "channel_id": cid) when calling c.post (or omit it only if cid is None) so that the request body contains kind, id, trace_id, content and channel_id and the channel-aware reply behavior actually takes effect.tinyagentos/scripts/install_openai_agents_sdk.sh (1)
67-79:⚠️ Potential issue | 🟡 MinorAlign with the
install_smolagents.shpattern: conditionally addchannel_idto the reply body.While the endpoint includes a fallback mechanism to recover
channel_idfrom the original message viatrace_id, this bridge doesn't proactively send it in the payload. This causes unnecessary database lookups and creates inconsistency with other bridges—install_smolagents.shandinstall_hermes.shcorrectly includeif cid: body["channel_id"] = cid. Update lines 67–68 to match that pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinyagentos/scripts/install_openai_agents_sdk.sh` around lines 67 - 79, The post_reply/handle flow currently always sends a fixed JSON body and doesn't include channel_id, causing extra lookups; update the post_reply logic (used by async def post_reply and its caller async def handle) to build the request body conditionally and add body["channel_id"] = cid when cid is truthy (mirror the install_smolagents.sh pattern), then send that body in the POST; ensure you preserve the existing Authorization header and timeout behavior and keep the same parameter order for post_reply(mid, tid, txt, cid) usage.tinyagentos/bridge_session.py (1)
318-368:⚠️ Potential issue | 🟠 MajorStreaming replies never get re-dispatched.
persistedis only assigned in the no-placeholder branch. If a bridge sendsdeltathenfinal, this code edits the existing streaming message but leavespersistedasNone, sorouter.dispatch(...)is skipped. In practice that means other agents still won't see the common streamed-reply path.Suggested fix
persisted = None if channel_id: if pending_msg_id and self._chat_messages: # Edit the streaming placeholder to final content. await self._chat_messages.edit_message(pending_msg_id, final_content) await self._chat_messages.update_state(pending_msg_id, "complete") session.pending_hops.pop(trace_id, None) + persisted = await self._chat_messages.get_message(pending_msg_id) if self._chat_hub: await self._chat_hub.broadcast(channel_id, { "type": "message_edit", "seq": self._chat_hub.next_seq(), "message_id": pending_msg_id,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinyagentos/bridge_session.py` around lines 318 - 368, The edited streaming placeholder path currently never sets persisted so router.dispatch is skipped; after the edit branch (where pending_msg_id is truthy and you call await self._chat_messages.edit_message(pending_msg_id, final_content) and update_state), set persisted to the updated message object so re-dispatch runs — either capture the return value of self._chat_messages.edit_message (if it returns the message) or fetch/construct a minimal persisted dict containing at least "message_id": pending_msg_id and "channel_id": channel_id plus any needed metadata (e.g., trace_id, openclaw_msg_id, hops) before calling router.dispatch; ensure you still pop session.pending_hops(trace_id) as currently done.
🧹 Nitpick comments (10)
tests/test_chat_messages.py (1)
219-240: CloseChatMessageStorein these tests to avoid resource leakage.Both tests initialize a DB-backed store but never close it. Please wrap body in
try/finally(or use a fixture) so cleanup always runs.♻️ Suggested test cleanup pattern
async def test_send_message_persists_hops_metadata(tmp_path): store = ChatMessageStore(tmp_path / "msgs.db") await store.init() - msg = await store.send_message( - channel_id="c1", author_id="tom", author_type="agent", - content="yo", content_type="text", state="complete", - metadata={"hops_since_user": 2, "other": "x"}, - ) - assert msg["metadata"]["hops_since_user"] == 2 - assert msg["metadata"]["other"] == "x" + try: + msg = await store.send_message( + channel_id="c1", author_id="tom", author_type="agent", + content="yo", content_type="text", state="complete", + metadata={"hops_since_user": 2, "other": "x"}, + ) + assert msg["metadata"]["hops_since_user"] == 2 + assert msg["metadata"]["other"] == "x" + finally: + await store.close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_chat_messages.py` around lines 219 - 240, Both tests open a ChatMessageStore but never close it; wrap the test bodies of test_send_message_persists_hops_metadata and test_send_message_defaults_hops_zero_when_absent in a try/finally (or use an async fixture/context manager) and call await store.close() in the finally block to ensure the DB-backed ChatMessageStore is always closed and resources are released.tests/test_chat_channel_settings.py (1)
6-47: Use guaranteed teardown forChatChannelStorein these tests.
await store.close()is only reached if assertions pass; switching to a fixture ortry/finallywill make cleanup deterministic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_chat_channel_settings.py` around lines 6 - 47, The tests create a ChatChannelStore instance and call await store.close() only on the success path; wrap each test body that creates ChatChannelStore (e.g., test_get_channel_backfills_phase1_defaults, test_set_response_mode_persists, test_mute_and_unmute_agent) in a try/finally so await store.close() is executed in the finally block, or convert the store creation to a pytest async fixture that yields the store and calls await store.close() after yield; reference ChatChannelStore, store.init(), and store.close() when updating the tests.tests/test_agent_chat_router.py (1)
163-164: Consider movingGroupPolicyimport to module level.
GroupPolicyis imported locally in each test function. Moving it to the module-level imports would reduce repetition.♻️ Proposed refactor
from tinyagentos.agent_chat_router import AgentChatRouter +from tinyagentos.chat.group_policy import GroupPolicyThen remove the local imports from each test function.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_agent_chat_router.py` around lines 163 - 164, Move the local import of GroupPolicy to the module-level imports so tests reuse a single import instead of importing inside each test function; update the top-of-file import block to include "from tinyagentos.chat.group_policy import GroupPolicy" and remove the in-test local imports and the subsequent "state.group_policy = GroupPolicy()" initializations inside individual tests so they instead reference the module-level GroupPolicy (ensure any test-specific setup still sets state.group_policy where needed).tinyagentos/routes/chat.py (3)
257-258: Consider logging archive failures at debug level.The
try-except-pass(S110) is intentional per the comment, but logging the exception would aid debugging without blocking the chat flow.♻️ Proposed fix
except Exception: - pass # Never block chat for archive failures + logger.debug("archive recording failed", exc_info=True) # Never block chat🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinyagentos/routes/chat.py` around lines 257 - 258, The blind except in the chat archive block (the "except Exception: pass # Never block chat for archive failures" stanza) should log the failure at debug level without changing control flow; replace the bare pass with a debug log that includes the exception details (e.g., logger.debug("Chat archive failed, continuing chat", exc_info=True) or app.logger.debug(..., exc_info=True)) so archive errors are recorded for debugging but still do not block chat.
396-399: Non-integer values formax_hopsandcooldown_secondswill raiseValueError.If the request body contains a non-integer value like
{"max_hops": "abc"},int()will raiseValueErrorwhich is caught and returned as 400. This is acceptable behavior, but the error message will be Python's default (invalid literal for int()...) rather than a user-friendly message.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinyagentos/routes/chat.py` around lines 396 - 399, The route currently calls int(body["max_hops"]) and int(body["cooldown_seconds"]) before awaiting chs.set_max_hops and chs.set_cooldown_seconds which will raise a raw ValueError for non-integer input; update the handler to validate/parse these fields first (e.g., try/except around int(...) or use a helper parse_int) and on parse failure raise/return a 400 with a clear user-friendly message like "max_hops must be an integer" or "cooldown_seconds must be an integer", referencing the existing calls to chs.set_max_hops and chs.set_cooldown_seconds so the validated integer is passed into those methods.
437-454: Muted endpoint doesn't validate slug against known agents.Unlike the members endpoint (line 428-430), the muted endpoint allows adding any slug to the muted list without checking if it's a known agent. This could lead to typos going unnoticed.
♻️ Proposed fix to add validation
if action == "add": + known = {a.get("name") for a in getattr(state.config, "agents", []) or []} + if slug not in known: + return JSONResponse({"error": f"unknown agent: {slug}"}, status_code=400) await chs.mute_agent(channel_id, slug)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinyagentos/routes/chat.py` around lines 437 - 454, The modify_channel_muted handler accepts any slug and can add unknown/typoed agents to muted; before calling chs.mute_agent or chs.unmute_agent you must validate the slug exists using the same check the members endpoint uses (e.g., call the agents lookup used elsewhere like state.agents.get_agent(slug) or the members validation logic) and return a 400 error if the agent is not found; keep the rest of modify_channel_muted (action parsing and channel existence check) unchanged and only perform this existence check immediately after computing slug and before invoking chs.mute_agent/ch.sunmute_agent.tests/test_routes_chat_admin.py (2)
103-110: Prefix unused variable with underscore.The
appvariable is unpacked but never used in this test. Prefix it with_to satisfy the linter.♻️ Proposed fix
`@pytest.mark.asyncio` async def test_patch_unknown_channel_returns_404(tmp_path): - app, client = await _setup_client(tmp_path) + _app, client = await _setup_client(tmp_path) async with client: r = await client.patch( "/api/chat/channels/doesnotexist",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_routes_chat_admin.py` around lines 103 - 110, In test_patch_unknown_channel_returns_404, the unpacked app variable from the call to _setup_client is unused; rename it to _app (or simply _ ) so the test reads " _app, client = await _setup_client(tmp_path)" to satisfy the linter while keeping the rest of the test (client usage and assertions) unchanged; update the assignment in the test_patch_unknown_channel_returns_404 function where _setup_client is called.
156-169: Consider adding test for muted remove action.There's a test for
action: addto muted but not foraction: remove. Consider adding coverage for unmuting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_routes_chat_admin.py` around lines 156 - 169, Add a new async test mirroring test_muted_add_agent that verifies the "remove" action: use _setup_client and _make_channel to get app and ch_id, POST to "/api/chat/channels/{ch_id}/muted" with json {"action": "remove", "slug": "tom"} (ensure "tom" is present first by either adding him via the same endpoint or pre-populating), assert r.status_code == 200, then call app.state.chat_channels.get_channel(ch_id) and assert "tom" is not in ch["settings"]["muted"]; name the test e.g. test_muted_remove_agent to match the existing pattern.tests/test_routes_chat_slash_guard.py (1)
37-53: Consider adding edge case for whitespace-prefixed slash commands.The guardrail in
routes/chat.pyusescontent.lstrip().startswith("/"), so a message like" /help"would also be rejected. Consider adding a test to verify this behavior is intentional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_routes_chat_slash_guard.py` around lines 37 - 53, Add a new async test mirroring test_bare_slash_in_group_returns_400 that sends a message with leading whitespace (e.g. " /help") to ensure routes/chat.py's guard (which checks content.lstrip().startswith("/")) rejects whitespace-prefixed slash commands; reuse the same channel creation via app.state.chat_channels.create_channel and a POST to "/api/chat/messages", assert a 400 status and that the error contains "address an agent"; name the test something like test_whitespace_prefixed_slash_in_group_returns_400 to make the intent clear.tinyagentos/agent_chat_router.py (1)
46-47: Redundant system message check.This check duplicates the filter at line 30-31 in
dispatch(). It's harmless but unnecessary since_route_inneris only called via_routewhich is only invoked fromdispatch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinyagentos/agent_chat_router.py` around lines 46 - 47, Remove the redundant system message guard inside _route_inner: the check `if message.get("content_type") == "system": return` duplicates the filtering already performed in dispatch() (and _route) so delete that conditional from _route_inner (confirm no other external callers of _route_inner rely on it), then run tests to ensure behavior is unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@docs/superpowers/specs/2026-04-19-chat-phase-1-conversational-core-design.md`:
- Around line 324-342: The doc is inconsistent: the design now states taOS does
not intercept `/` but earlier sections still reference slash_commands.execute
and command-specific error handling/testing; update the spec by removing or
rewriting any prior references to the slash_commands.execute branch, related
command-specific error handling/testing, and any tests or examples that assume
taOS intercepts `/` so they reflect the resolved design (UI-driven REST
endpoints and the guardrail requiring `@<slug>`/`@all`), and ensure any symbol
names like slash_commands.execute, slash command error handlers, and command
testing examples are either removed or altered to describe the UI/REST-driven
flow and guardrail behavior.
- Around line 384-385: Update the modified-files summary row so it correctly
points the "re-dispatch on agent reply" behavior to
tinyagentos/bridge_session.py (where router.dispatch is called on agent replies
and message_suppressed is traced) instead of tinyagentos/routes/chat.py; remove
or relocate that phrase from the routes/chat.py row and ensure the
bridge_session.py row mentions accepting force_respond, context, hops_since_user
and calling router.dispatch on agent replies to match the implementation.
In `@tests/e2e/test_roundtable_group_chat.py`:
- Around line 42-55: The poll currently inspects the channel's last 20 messages
unscoped to this test; capture the message id returned when posting the prompt
(e.g., sent_msg_id) and restrict the polling logic to replies tied to that id by
filtering messages where m.get("parent_id") == sent_msg_id (or the equivalent
reply/root field used by the API) before computing agent_msgs, names and
cross_refs; update the list comprehension that builds agent_msgs and the
cross_refs calculation to only consider messages with that parent/root matching
sent_msg_id so older messages in the channel cannot satisfy the success
condition.
- Around line 25-31: The test currently posts a bare "/lively" message via
c.post("/api/chat/messages") for CHANNEL_ID which is rejected by the bare-slash
guardrail; replace that step with a setup call to the admin REST API to put the
channel into lively mode for CHANNEL_ID (using the test client c.post to the
admin endpoint) before exercising the roundtable flow, or if you prefer the
in-channel route change the message content to address an agent (e.g., "/lively
`@assistant`") so the guardrail accepts it; update the code that currently posts
"/lively" to use one of these two fixes (referencing CHANNEL_ID and the c.post
call in the diff).
In `@tests/test_bridge_session_phase1.py`:
- Around line 38-40: Split the semicolon-separated setup into separate
statements so the linter won't flag E702: create each mock assignment on its own
line (e.g., assign chans = MagicMock(), then set chans.update_last_message_at =
AsyncMock(); assign hub = MagicMock(), then set hub.broadcast = AsyncMock() and
hub.next_seq = MagicMock(return_value=1); finally instantiate reg =
BridgeSessionRegistry(chat_messages=store, chat_channels=chans, chat_hub=hub)).
Ensure you reference the existing symbols chans, hub, AsyncMock, MagicMock, and
BridgeSessionRegistry exactly as used in the diff.
In `@tests/test_chat_reactions.py`:
- Around line 9-10: The test sets multiple statements on one line causing E702;
split the mock setup into separate statements so each assignment is on its own
line: create bridge = MagicMock() on its own line, then set
bridge.enqueue_user_message = AsyncMock() on the next line, and then create
state = MagicMock(bridge_sessions=bridge) on its own line (apply the same fix
for the similar lines around where bridge and state are set at lines 29-30).
In `@tinyagentos/chat/context_window.py`:
- Around line 11-14: The estimate_tokens function can return 0 for short
non-empty strings, letting build_context_window include messages for free;
change estimate_tokens (referenced by build_context_window) so any non-empty
text returns at least 1 token (e.g., return max(1, len(text) // 4)) so 1–3
character messages are counted; keep the early empty check and apply the clamp
before returning.
In `@tinyagentos/chat/group_policy.py`:
- Around line 21-42: may_send()/record_send() are racy because callers can await
between them; implement an atomic acquire method (e.g.,
try_acquire_send(channel_id: str, agent: str, settings: dict) -> bool) that
checks cooldown and rate cap and, if allowed, immediately updates _last_send_at
and _recent_sends in one locked operation; add a Lock (or use existing
synchronization) inside the class and use it in try_acquire_send to guard
reads/writes of _last_send_at and _recent_sends, keep may_send/record_send for
backwards compatibility or deprecate them and update call sites to use
try_acquire_send.
In `@tinyagentos/chat/reactions.py`:
- Around line 40-52: The regenerate branch currently hard-codes "context": []
when calling bridge.enqueue_user_message, which drops room history; replace that
empty context with the same rolling context used for normal agent turns (i.e.,
build and pass the conversation window instead of an empty list) or route this
regenerate event through the existing fanout/context path that constructs the
context window; update the call in reactions.py (the dict passed to
bridge.enqueue_user_message where "context": [] and "regenerate": True are set,
and using reactor_id/message) to supply the reconstructed context window (or
call the fanout/context helper used elsewhere) so regenerated messages receive
the same recent history as regular agent turns.
In `@tinyagentos/scripts/install_langroid.sh`:
- Around line 60-72: post_reply currently ignores the cid parameter, so include
channel routing by adding channel_id to the JSON payload when cid is present:
update post_reply(c, u, t, mid, tid, txt, cid=None) to build body =
{"kind":"final","id":mid,"trace_id":tid,"content":txt} and if cid:
body["channel_id"]=cid, then send that body in the POST; keep handle() as-is (it
passes evt.get("channel_id")), and ensure log/warning behavior and timeout
remain unchanged.
---
Outside diff comments:
In `@tinyagentos/bridge_session.py`:
- Around line 318-368: The edited streaming placeholder path currently never
sets persisted so router.dispatch is skipped; after the edit branch (where
pending_msg_id is truthy and you call await
self._chat_messages.edit_message(pending_msg_id, final_content) and
update_state), set persisted to the updated message object so re-dispatch runs —
either capture the return value of self._chat_messages.edit_message (if it
returns the message) or fetch/construct a minimal persisted dict containing at
least "message_id": pending_msg_id and "channel_id": channel_id plus any needed
metadata (e.g., trace_id, openclaw_msg_id, hops) before calling router.dispatch;
ensure you still pop session.pending_hops(trace_id) as currently done.
In `@tinyagentos/scripts/install_openai_agents_sdk.sh`:
- Around line 67-79: The post_reply/handle flow currently always sends a fixed
JSON body and doesn't include channel_id, causing extra lookups; update the
post_reply logic (used by async def post_reply and its caller async def handle)
to build the request body conditionally and add body["channel_id"] = cid when
cid is truthy (mirror the install_smolagents.sh pattern), then send that body in
the POST; ensure you preserve the existing Authorization header and timeout
behavior and keep the same parameter order for post_reply(mid, tid, txt, cid)
usage.
In `@tinyagentos/scripts/install_openai-agents-sdk.sh`:
- Around line 67-69: The post_reply function is ignoring the cid parameter so
channel_id never gets sent; update async def post_reply(c, u, t, mid, tid, txt,
cid=None) to include the channel id in the JSON payload (e.g. add "channel_id":
cid) when calling c.post (or omit it only if cid is None) so that the request
body contains kind, id, trace_id, content and channel_id and the channel-aware
reply behavior actually takes effect.
---
Nitpick comments:
In `@tests/test_agent_chat_router.py`:
- Around line 163-164: Move the local import of GroupPolicy to the module-level
imports so tests reuse a single import instead of importing inside each test
function; update the top-of-file import block to include "from
tinyagentos.chat.group_policy import GroupPolicy" and remove the in-test local
imports and the subsequent "state.group_policy = GroupPolicy()" initializations
inside individual tests so they instead reference the module-level GroupPolicy
(ensure any test-specific setup still sets state.group_policy where needed).
In `@tests/test_chat_channel_settings.py`:
- Around line 6-47: The tests create a ChatChannelStore instance and call await
store.close() only on the success path; wrap each test body that creates
ChatChannelStore (e.g., test_get_channel_backfills_phase1_defaults,
test_set_response_mode_persists, test_mute_and_unmute_agent) in a try/finally so
await store.close() is executed in the finally block, or convert the store
creation to a pytest async fixture that yields the store and calls await
store.close() after yield; reference ChatChannelStore, store.init(), and
store.close() when updating the tests.
In `@tests/test_chat_messages.py`:
- Around line 219-240: Both tests open a ChatMessageStore but never close it;
wrap the test bodies of test_send_message_persists_hops_metadata and
test_send_message_defaults_hops_zero_when_absent in a try/finally (or use an
async fixture/context manager) and call await store.close() in the finally block
to ensure the DB-backed ChatMessageStore is always closed and resources are
released.
In `@tests/test_routes_chat_admin.py`:
- Around line 103-110: In test_patch_unknown_channel_returns_404, the unpacked
app variable from the call to _setup_client is unused; rename it to _app (or
simply _ ) so the test reads " _app, client = await _setup_client(tmp_path)" to
satisfy the linter while keeping the rest of the test (client usage and
assertions) unchanged; update the assignment in the
test_patch_unknown_channel_returns_404 function where _setup_client is called.
- Around line 156-169: Add a new async test mirroring test_muted_add_agent that
verifies the "remove" action: use _setup_client and _make_channel to get app and
ch_id, POST to "/api/chat/channels/{ch_id}/muted" with json {"action": "remove",
"slug": "tom"} (ensure "tom" is present first by either adding him via the same
endpoint or pre-populating), assert r.status_code == 200, then call
app.state.chat_channels.get_channel(ch_id) and assert "tom" is not in
ch["settings"]["muted"]; name the test e.g. test_muted_remove_agent to match the
existing pattern.
In `@tests/test_routes_chat_slash_guard.py`:
- Around line 37-53: Add a new async test mirroring
test_bare_slash_in_group_returns_400 that sends a message with leading
whitespace (e.g. " /help") to ensure routes/chat.py's guard (which checks
content.lstrip().startswith("/")) rejects whitespace-prefixed slash commands;
reuse the same channel creation via app.state.chat_channels.create_channel and a
POST to "/api/chat/messages", assert a 400 status and that the error contains
"address an agent"; name the test something like
test_whitespace_prefixed_slash_in_group_returns_400 to make the intent clear.
In `@tinyagentos/agent_chat_router.py`:
- Around line 46-47: Remove the redundant system message guard inside
_route_inner: the check `if message.get("content_type") == "system": return`
duplicates the filtering already performed in dispatch() (and _route) so delete
that conditional from _route_inner (confirm no other external callers of
_route_inner rely on it), then run tests to ensure behavior is unchanged.
In `@tinyagentos/routes/chat.py`:
- Around line 257-258: The blind except in the chat archive block (the "except
Exception: pass # Never block chat for archive failures" stanza) should log the
failure at debug level without changing control flow; replace the bare pass with
a debug log that includes the exception details (e.g., logger.debug("Chat
archive failed, continuing chat", exc_info=True) or app.logger.debug(...,
exc_info=True)) so archive errors are recorded for debugging but still do not
block chat.
- Around line 396-399: The route currently calls int(body["max_hops"]) and
int(body["cooldown_seconds"]) before awaiting chs.set_max_hops and
chs.set_cooldown_seconds which will raise a raw ValueError for non-integer
input; update the handler to validate/parse these fields first (e.g., try/except
around int(...) or use a helper parse_int) and on parse failure raise/return a
400 with a clear user-friendly message like "max_hops must be an integer" or
"cooldown_seconds must be an integer", referencing the existing calls to
chs.set_max_hops and chs.set_cooldown_seconds so the validated integer is passed
into those methods.
- Around line 437-454: The modify_channel_muted handler accepts any slug and can
add unknown/typoed agents to muted; before calling chs.mute_agent or
chs.unmute_agent you must validate the slug exists using the same check the
members endpoint uses (e.g., call the agents lookup used elsewhere like
state.agents.get_agent(slug) or the members validation logic) and return a 400
error if the agent is not found; keep the rest of modify_channel_muted (action
parsing and channel existence check) unchanged and only perform this existence
check immediately after computing slug and before invoking
chs.mute_agent/ch.sunmute_agent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 503bc2e6-61be-4576-b064-481c9cb9f5ac
📒 Files selected for processing (30)
docs/superpowers/specs/2026-04-19-chat-phase-1-conversational-core-design.mdpyproject.tomltests/e2e/test_roundtable_group_chat.pytests/test_agent_chat_router.pytests/test_bridge_session_phase1.pytests/test_chat_channel_settings.pytests/test_chat_channels.pytests/test_chat_context_window.pytests/test_chat_group_policy.pytests/test_chat_mentions.pytests/test_chat_messages.pytests/test_chat_reactions.pytests/test_routes_chat_admin.pytests/test_routes_chat_slash_guard.pytinyagentos/agent_chat_router.pytinyagentos/app.pytinyagentos/bridge_session.pytinyagentos/chat/channel_store.pytinyagentos/chat/context_window.pytinyagentos/chat/group_policy.pytinyagentos/chat/mentions.pytinyagentos/chat/message_store.pytinyagentos/chat/reactions.pytinyagentos/routes/chat.pytinyagentos/scripts/install_hermes.shtinyagentos/scripts/install_langroid.shtinyagentos/scripts/install_openai-agents-sdk.shtinyagentos/scripts/install_openai_agents_sdk.shtinyagentos/scripts/install_pocketflow.shtinyagentos/scripts/install_smolagents.sh
Comment on lines
+21
to
+42
| def may_send(self, channel_id: str, agent: str, settings: dict) -> bool: | ||
| now = _now() | ||
| cooldown = int(settings.get("cooldown_seconds", 5)) | ||
| cap = int(settings.get("rate_cap_per_minute", 20)) | ||
|
|
||
| last = self._last_send_at.get((channel_id, agent)) | ||
| if last is not None and (now - last) < cooldown: | ||
| return False | ||
|
|
||
| window = self._recent_sends.get(channel_id) | ||
| if window: | ||
| while window and (now - window[0]) > 60.0: | ||
| window.popleft() | ||
| if len(window) >= cap: | ||
| return False | ||
| return True | ||
|
|
||
| def record_send(self, channel_id: str, agent: str) -> None: | ||
| now = _now() | ||
| self._last_send_at[(channel_id, agent)] = now | ||
| window = self._recent_sends.setdefault(channel_id, deque(maxlen=256)) | ||
| window.append(now) |
There was a problem hiding this comment.
Policy enforcement is non-atomic across may_send()/record_send().
Because callers await between these calls, concurrent routes can all pass may_send() before any record_send() occurs, weakening cooldown/rate caps.
🐛 Suggested direction (atomic acquire)
class GroupPolicy:
@@
- def may_send(self, channel_id: str, agent: str, settings: dict) -> bool:
+ def try_acquire_send(self, channel_id: str, agent: str, settings: dict) -> bool:
now = _now()
cooldown = int(settings.get("cooldown_seconds", 5))
cap = int(settings.get("rate_cap_per_minute", 20))
@@
- return True
-
- def record_send(self, channel_id: str, agent: str) -> None:
- now = _now()
- self._last_send_at[(channel_id, agent)] = now
- window = self._recent_sends.setdefault(channel_id, deque(maxlen=256))
- window.append(now)
+ self._last_send_at[(channel_id, agent)] = now
+ window = self._recent_sends.setdefault(channel_id, deque(maxlen=256))
+ window.append(now)
+ return TrueThen call try_acquire_send(...) before enqueue, instead of separate check/record calls.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def may_send(self, channel_id: str, agent: str, settings: dict) -> bool: | |
| now = _now() | |
| cooldown = int(settings.get("cooldown_seconds", 5)) | |
| cap = int(settings.get("rate_cap_per_minute", 20)) | |
| last = self._last_send_at.get((channel_id, agent)) | |
| if last is not None and (now - last) < cooldown: | |
| return False | |
| window = self._recent_sends.get(channel_id) | |
| if window: | |
| while window and (now - window[0]) > 60.0: | |
| window.popleft() | |
| if len(window) >= cap: | |
| return False | |
| return True | |
| def record_send(self, channel_id: str, agent: str) -> None: | |
| now = _now() | |
| self._last_send_at[(channel_id, agent)] = now | |
| window = self._recent_sends.setdefault(channel_id, deque(maxlen=256)) | |
| window.append(now) | |
| def try_acquire_send(self, channel_id: str, agent: str, settings: dict) -> bool: | |
| now = _now() | |
| cooldown = int(settings.get("cooldown_seconds", 5)) | |
| cap = int(settings.get("rate_cap_per_minute", 20)) | |
| last = self._last_send_at.get((channel_id, agent)) | |
| if last is not None and (now - last) < cooldown: | |
| return False | |
| window = self._recent_sends.get(channel_id) | |
| if window: | |
| while window and (now - window[0]) > 60.0: | |
| window.popleft() | |
| if len(window) >= cap: | |
| return False | |
| self._last_send_at[(channel_id, agent)] = now | |
| window = self._recent_sends.setdefault(channel_id, deque(maxlen=256)) | |
| window.append(now) | |
| return True |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tinyagentos/chat/group_policy.py` around lines 21 - 42,
may_send()/record_send() are racy because callers can await between them;
implement an atomic acquire method (e.g., try_acquire_send(channel_id: str,
agent: str, settings: dict) -> bool) that checks cooldown and rate cap and, if
allowed, immediately updates _last_send_at and _recent_sends in one locked
operation; add a Lock (or use existing synchronization) inside the class and use
it in try_acquire_send to guard reads/writes of _last_send_at and _recent_sends,
keep may_send/record_send for backwards compatibility or deprecate them and
update call sites to use try_acquire_send.
…flow/openai-agents The smolagents and hermes bridges already build the reply body and conditionally include channel_id; the other four dropped it, relying on the host-side trace_id fallback in bridge_session._handle_reply. That fallback still works but is defense in depth, not the primary path. Threading channel_id through keeps room affinity explicit and aligns all six bridges on one pattern.
- group_policy: add try_acquire() to atomically check+record in one non-awaiting call, so concurrent asyncio routes can't both pass when only one should; router switches to try_acquire. - reactions: regenerate path now rebuilds a context window from chat messages (was hard-coded []), so 👎 retries see the same room history as regular turns. - e2e: switch /lively setup to PATCH admin endpoint (bare-slash guard now rejects it); scope the pass condition to replies correlated to this test's message id (reused e2e channel can't satisfy it with stale agent messages).
- Split chained `;`-separated statements in two test files (Ruff E702). - estimate_tokens returns 1 for 1-3 char inputs so the token budget can't silently be exceeded by many short non-empty messages. - Spec doc: remove leftover slash_commands.execute references and fix the row that claimed re-dispatch is in routes/chat.py (it's in bridge_session.py); spec file is gitignored (docs/superpowers/specs).
Owner
Author
|
Addressed all 10 CodeRabbit findings in 4 follow-up commits:
Full suite: 1908 passed / 13 skipped. The 3 |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tinyagentos/agent_chat_router.py (1)
92-113:⚠️ Potential issue | 🟠 MajorMove
try_acquire()after the deliverability checks.Line 97 records the send immediately, but
find_agent(), the running-status check, and the bridge-registry check all happen afterwards. In lively channels, a stopped/misconfigured agent can therefore burn cooldown slots and the channel-wide rate cap without anyenqueue_user_message()ever happening. Acquire the policy slot only once you know the recipient is actually sendable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinyagentos/agent_chat_router.py` around lines 92 - 113, The policy.try_acquire() call is happening too early and can consume rate/cooldown slots for agents that are later found undeliverable; move the policy.try_acquire(channel["id"], agent_name, settings) invocation so it runs only after we confirm the recipient is deliverable (i.e., after find_agent(config, agent_name) returns non-None, after the agent.get("status") == "running" check, and after the bridge != None check), keep honoring the forced = force_by_slug.get(agent_name, False) bypass (do not call try_acquire if forced is True), and leave the next_hops > max_hops short-circuit where it is.
🧹 Nitpick comments (1)
tinyagentos/scripts/install_openai-agents-sdk.sh (1)
31-81: Extract the shared bridge scaffold before it drifts again.This block now mirrors
tinyagentos/scripts/install_langroid.shalmost verbatim for context rendering,NO_RESPONSEsuppression, reply posting, and handler flow. Keeping that logic copy-pasted across installers is likely to recreate the same cross-bridge skew this PR just cleaned up. Consider moving the common Python scaffold into one shared module/template and leaving only the framework-specific_build/_runpieces per bridge.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinyagentos/scripts/install_openai-agents-sdk.sh` around lines 31 - 81, The reviewer wants the duplicated bridge scaffold (context rendering, NO_RESPONSE suppression, posting, and handler flow) extracted into a shared module/template rather than copied into each installer; refactor the common functions _render_context, _suppress, post_reply, and handle into a single shared utility (or template) and import/use them from tinyagentos/scripts/install_openai-agents-sdk.sh, leaving only bridge-specific pieces (_build and _run) in this file; ensure the shared module exposes the same function signatures (context list input, suppression semantics, post_reply signature, and an async handler wrapper) so you can call the shared handler with the bridge-specific _build/_run implementations and maintain current behavior and logging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tinyagentos/agent_chat_router.py`:
- Around line 115-125: The context-building (calls to
self._state.chat_messages.get_messages and build_context_window) should be
executed once per routed message instead of inside the recipient loop: move the
block that sets context (including the try/except with logger.warning) out of
the per-recipient loop so you compute context once for the channel (use
channel.get("id") or channel["id"] as the key) and then reuse that same context
variable when enqueuing for each recipient; preserve the existing error handling
and fallback to an empty list on failure.
- Around line 84-85: The code reads metadata.hops_since_user into current_hops
and adds 1 without validation; coerce/validate that value to an integer
(defaulting to 0 for None, non-numeric strings, or other invalid types) before
computing next_hops so a malicious or malformed message won't raise; update the
logic around current_hops and next_hops in agent_chat_router.py to parse
int(message.get("metadata", {}).get("hops_since_user", 0)) with a safe fallback
(e.g., try/except or isinstance checks) and then set next_hops =
validated_current_hops + 1.
---
Outside diff comments:
In `@tinyagentos/agent_chat_router.py`:
- Around line 92-113: The policy.try_acquire() call is happening too early and
can consume rate/cooldown slots for agents that are later found undeliverable;
move the policy.try_acquire(channel["id"], agent_name, settings) invocation so
it runs only after we confirm the recipient is deliverable (i.e., after
find_agent(config, agent_name) returns non-None, after the agent.get("status")
== "running" check, and after the bridge != None check), keep honoring the
forced = force_by_slug.get(agent_name, False) bypass (do not call try_acquire if
forced is True), and leave the next_hops > max_hops short-circuit where it is.
---
Nitpick comments:
In `@tinyagentos/scripts/install_openai-agents-sdk.sh`:
- Around line 31-81: The reviewer wants the duplicated bridge scaffold (context
rendering, NO_RESPONSE suppression, posting, and handler flow) extracted into a
shared module/template rather than copied into each installer; refactor the
common functions _render_context, _suppress, post_reply, and handle into a
single shared utility (or template) and import/use them from
tinyagentos/scripts/install_openai-agents-sdk.sh, leaving only bridge-specific
pieces (_build and _run) in this file; ensure the shared module exposes the same
function signatures (context list input, suppression semantics, post_reply
signature, and an async handler wrapper) so you can call the shared handler with
the bridge-specific _build/_run implementations and maintain current behavior
and logging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 50cc94cc-728e-4d36-bfe0-0684f23fd89e
📒 Files selected for processing (14)
docs/superpowers/specs/2026-04-19-chat-phase-1-conversational-core-design.mdtests/e2e/test_roundtable_group_chat.pytests/test_bridge_session_phase1.pytests/test_chat_context_window.pytests/test_chat_group_policy.pytests/test_chat_reactions.pytinyagentos/agent_chat_router.pytinyagentos/chat/context_window.pytinyagentos/chat/group_policy.pytinyagentos/chat/reactions.pytinyagentos/scripts/install_langroid.shtinyagentos/scripts/install_openai-agents-sdk.shtinyagentos/scripts/install_openai_agents_sdk.shtinyagentos/scripts/install_pocketflow.sh
✅ Files skipped from review due to trivial changes (3)
- tinyagentos/chat/context_window.py
- tests/test_chat_context_window.py
- tests/test_chat_group_policy.py
🚧 Files skipped from review as they are similar to previous changes (7)
- tests/test_bridge_session_phase1.py
- tinyagentos/scripts/install_openai_agents_sdk.sh
- tests/test_chat_reactions.py
- tinyagentos/chat/group_policy.py
- tinyagentos/scripts/install_pocketflow.sh
- tests/e2e/test_roundtable_group_chat.py
- docs/superpowers/specs/2026-04-19-chat-phase-1-conversational-core-design.md
…hoist, regenerate prompt) - agent_chat_router: coerce metadata.hops_since_user to int before arithmetic; avoids TypeError on malformed client payloads. - agent_chat_router: hoist context-window build out of the per-recipient loop — in a 5-agent lively channel we were doing 5 identical DB reads and context-window builds per message. - reactions: 👎 regenerate now fetches the ORIGINAL user message via trace_id and sends THAT as the prompt, not the agent's disliked reply. Previously the bridge would receive its own output as the next user turn and produce nonsense.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tinyagentos/agent_chat_router.py (1)
109-146:⚠️ Potential issue | 🟠 MajorAcquire group-policy quota only after the send is actually possible.
policy.try_acquire()runs before Line 115's agent lookup and the later status/bridge guards. Becausetry_acquire()is now atomic check+record, a missing or offline agent still burns cooldown/channel budget, which can block later valid deliveries in the same room. Move the acquire to just beforeenqueue_user_message().Suggested fix
for agent_name in recipients: forced = force_by_slug.get(agent_name, False) if not forced: if next_hops > max_hops: continue - if policy is not None and not policy.try_acquire(channel["id"], agent_name, settings): - continue agent = find_agent(config, agent_name) if agent is None: continue if agent.get("status") != "running": await self._post_system_reply( @@ if bridge is None: await self._post_system_reply( agent_name, channel["id"], "[router] bridge registry not configured on this host.", ) continue + if not forced and policy is not None and not policy.try_acquire(channel["id"], agent_name, settings): + continue await bridge.enqueue_user_message(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinyagentos/agent_chat_router.py` around lines 109 - 146, The policy.try_acquire() call should be moved from before the agent lookup to just before actually sending to avoid consuming quota for missing/offline agents: keep computing forced = force_by_slug.get(...), perform find_agent(config, agent_name), the agent status check (agent.get("status")), and bridge presence check first; then just before calling bridge.enqueue_user_message(...) if policy is not None and not forced call policy.try_acquire(channel["id"], agent_name, settings) and if it fails continue; after a successful enqueue (i.e., immediately after await bridge.enqueue_user_message(...)) call policy.record_send(channel["id"], agent_name) only when not forced and policy is not None.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tinyagentos/agent_chat_router.py`:
- Around line 134-135: The router currently overwrites the trace by always
setting "trace_id": message.get("id"); instead preserve an incoming root trace
if present by setting "trace_id" to message.get("metadata", {}).get("trace_id")
or fallback to message.get("id"). Update the code that builds the forwarded
message (the dict where "id": message.get("id") is set) to prefer the existing
metadata.trace_id from the incoming message before using the current message id
so downstream agent hops retain the original user-turn correlation.
- Around line 63-79: The code narrows recipients to only explicitly mentioned
agents when mentions.explicit is true, which breaks the "lively" behavior;
update the mentions.explicit branch in agent_chat_router.py so that if
effective_mode == "lively" you keep recipients = list(candidates) (fan-out to
all non-muted members) but still set force_by_slug for each slug in
mentions.explicit, otherwise retain the current filtering behavior (recipients =
[m for m in candidates if m in mentions.explicit]). Adjust the logic around
force_by_slug, mentions.explicit, candidates, recipients, and effective_mode to
ensure only the mentioned agents get force_by_slug=True while lively mode still
delivers to all candidates.
In `@tinyagentos/chat/reactions.py`:
- Around line 47-55: The original-message handling currently swallows errors and
can leave original_text empty, which allows the regenerate path to enqueue an
empty prompt (e.g., with force_respond=True); update the logic around
msg_store.get_message (and the variables original_text and original_id) to treat
a failed lookup or empty content as a hard failure: if original lookup raises or
returns no usable content, do not proceed to enqueue/force a regenerate—return
or skip enqueueing instead; add an explicit truthy check on original_text before
calling the enqueue/regenerate code path so an empty string will not be
enqueued.
- Around line 24-29: WantsReplyRegistry.list currently writes back an empty dict
into self._entries for channels with no alive timestamps, which lets _entries
grow; change the logic in WantsReplyRegistry.list (use variables now, bucket,
alive, self._ttl) so that after computing alive you only assign
self._entries[channel_id] = alive when alive is non-empty, and otherwise remove
the key (e.g., self._entries.pop(channel_id, None)) to avoid retaining empty
buckets; keep the returned value as sorted(alive).
---
Outside diff comments:
In `@tinyagentos/agent_chat_router.py`:
- Around line 109-146: The policy.try_acquire() call should be moved from before
the agent lookup to just before actually sending to avoid consuming quota for
missing/offline agents: keep computing forced = force_by_slug.get(...), perform
find_agent(config, agent_name), the agent status check (agent.get("status")),
and bridge presence check first; then just before calling
bridge.enqueue_user_message(...) if policy is not None and not forced call
policy.try_acquire(channel["id"], agent_name, settings) and if it fails
continue; after a successful enqueue (i.e., immediately after await
bridge.enqueue_user_message(...)) call policy.record_send(channel["id"],
agent_name) only when not forced and policy is not None.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f1792586-d7e4-44cc-bd66-f20ae5f0c8dc
📒 Files selected for processing (3)
tests/test_chat_reactions.pytinyagentos/agent_chat_router.pytinyagentos/chat/reactions.py
✅ Files skipped from review due to trivial changes (1)
- tests/test_chat_reactions.py
Comment on lines
+63
to
+79
| force_by_slug: dict[str, bool] = {} | ||
| if mentions.all: | ||
| for m in candidates: | ||
| force_by_slug[m] = True | ||
| recipients = list(candidates) | ||
| elif mentions.explicit: | ||
| recipients = [m for m in candidates if m in mentions.explicit] | ||
| for m in recipients: | ||
| force_by_slug[m] = True | ||
| elif channel_type == "dm": | ||
| recipients = list(candidates) | ||
| for m in recipients: | ||
| force_by_slug[m] = True | ||
| elif effective_mode == "quiet": | ||
| recipients = [] | ||
| else: | ||
| recipients = list(candidates) |
There was a problem hiding this comment.
Keep lively rooms fan-out even when someone is mentioned.
In a lively channel, Line 68 currently narrows recipients to only the explicitly mentioned agents. That contradicts the contract in the docstring: lively should still route to all non-muted members, with only the mentioned agents marked force_respond=True. As written, @agent-a silences every other agent in lively mode.
Suggested fix
force_by_slug: dict[str, bool] = {}
- if mentions.all:
- for m in candidates:
- force_by_slug[m] = True
- recipients = list(candidates)
- elif mentions.explicit:
- recipients = [m for m in candidates if m in mentions.explicit]
- for m in recipients:
- force_by_slug[m] = True
- elif channel_type == "dm":
+ if channel_type == "dm":
recipients = list(candidates)
- for m in recipients:
- force_by_slug[m] = True
+ force_by_slug = {m: True for m in recipients}
elif effective_mode == "quiet":
- recipients = []
+ if mentions.all:
+ recipients = list(candidates)
+ force_by_slug = {m: True for m in recipients}
+ else:
+ recipients = [m for m in candidates if m in mentions.explicit]
+ force_by_slug = {m: True for m in recipients}
else:
recipients = list(candidates)
+ if mentions.all:
+ force_by_slug = {m: True for m in recipients}
+ else:
+ force_by_slug = {m: True for m in recipients if m in mentions.explicit}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tinyagentos/agent_chat_router.py` around lines 63 - 79, The code narrows
recipients to only explicitly mentioned agents when mentions.explicit is true,
which breaks the "lively" behavior; update the mentions.explicit branch in
agent_chat_router.py so that if effective_mode == "lively" you keep recipients =
list(candidates) (fan-out to all non-muted members) but still set force_by_slug
for each slug in mentions.explicit, otherwise retain the current filtering
behavior (recipients = [m for m in candidates if m in mentions.explicit]).
Adjust the logic around force_by_slug, mentions.explicit, candidates,
recipients, and effective_mode to ensure only the mentioned agents get
force_by_slug=True while lively mode still delivers to all candidates.
Comment on lines
134
to
+135
| "id": message.get("id"), | ||
| "trace_id": message.get("id"), # MVP: message id IS the trace id | ||
| "trace_id": message.get("id"), |
There was a problem hiding this comment.
Preserve the root trace_id across agent-to-agent hops.
Line 135 resets every forwarded message's trace_id to the current message id. When this router reroutes an agent reply that already carries metadata.trace_id, downstream hops lose the original user-turn correlation, which also breaks trace-based reaction/regeneration lookups. Prefer the incoming trace when present.
Suggested fix
{
"id": message.get("id"),
- "trace_id": message.get("id"),
+ "trace_id": (
+ message.get("trace_id")
+ or (message.get("metadata") or {}).get("trace_id")
+ or message.get("id")
+ ),
"channel_id": message.get("channel_id"),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "id": message.get("id"), | |
| "trace_id": message.get("id"), # MVP: message id IS the trace id | |
| "trace_id": message.get("id"), | |
| { | |
| "id": message.get("id"), | |
| "trace_id": ( | |
| message.get("trace_id") | |
| or (message.get("metadata") or {}).get("trace_id") | |
| or message.get("id") | |
| ), | |
| "channel_id": message.get("channel_id"), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tinyagentos/agent_chat_router.py` around lines 134 - 135, The router
currently overwrites the trace by always setting "trace_id": message.get("id");
instead preserve an incoming root trace if present by setting "trace_id" to
message.get("metadata", {}).get("trace_id") or fallback to message.get("id").
Update the code that builds the forwarded message (the dict where "id":
message.get("id") is set) to prefer the existing metadata.trace_id from the
incoming message before using the current message id so downstream agent hops
retain the original user-turn correlation.
Comment on lines
+24
to
+29
| def list(self, channel_id: str) -> list[str]: | ||
| now = _now() | ||
| bucket = self._entries.get(channel_id) or {} | ||
| alive = {s: t for s, t in bucket.items() if (now - t) < self._ttl} | ||
| self._entries[channel_id] = alive | ||
| return sorted(alive) |
There was a problem hiding this comment.
Avoid retaining empty channel buckets in WantsReplyRegistry.list().
Line 28 writes back empty dicts for misses/expired channels, which can grow _entries indefinitely under repeated reads of random channel IDs.
Proposed fix
def list(self, channel_id: str) -> list[str]:
now = _now()
- bucket = self._entries.get(channel_id) or {}
+ bucket = self._entries.get(channel_id)
+ if not bucket:
+ return []
alive = {s: t for s, t in bucket.items() if (now - t) < self._ttl}
- self._entries[channel_id] = alive
+ if alive:
+ self._entries[channel_id] = alive
+ else:
+ self._entries.pop(channel_id, None)
return sorted(alive)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tinyagentos/chat/reactions.py` around lines 24 - 29, WantsReplyRegistry.list
currently writes back an empty dict into self._entries for channels with no
alive timestamps, which lets _entries grow; change the logic in
WantsReplyRegistry.list (use variables now, bucket, alive, self._ttl) so that
after computing alive you only assign self._entries[channel_id] = alive when
alive is non-empty, and otherwise remove the key (e.g.,
self._entries.pop(channel_id, None)) to avoid retaining empty buckets; keep the
returned value as sorted(alive).
Comment on lines
+47
to
+55
| if msg_store is not None and trace_id: | ||
| try: | ||
| original = await msg_store.get_message(trace_id) | ||
| if original: | ||
| original_text = original.get("content") or "" | ||
| original_id = original.get("id") or original_id | ||
| except Exception: | ||
| original_text = "" | ||
|
|
There was a problem hiding this comment.
Regenerate should not enqueue when original prompt resolution fails.
If original lookup fails or returns no content, Line 72 can enqueue an empty text with force_respond=True. That can trigger nonsensical retries instead of a true regenerate.
Proposed fix
- original_text = ""
- original_id = message.get("id")
- trace_id = (message.get("metadata") or {}).get("trace_id") or message.get("id")
+ original_text = ""
+ original_id = message.get("id")
+ trace_id = (message.get("metadata") or {}).get("trace_id")
+ if not trace_id:
+ return
if msg_store is not None and trace_id:
try:
original = await msg_store.get_message(trace_id)
if original:
original_text = original.get("content") or ""
original_id = original.get("id") or original_id
except Exception:
- original_text = ""
+ return
@@
+ if not original_text.strip():
+ return
await bridge.enqueue_user_message(
message["author_id"],
{Also applies to: 65-77
🧰 Tools
🪛 Ruff (0.15.10)
[warning] 53-53: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tinyagentos/chat/reactions.py` around lines 47 - 55, The original-message
handling currently swallows errors and can leave original_text empty, which
allows the regenerate path to enqueue an empty prompt (e.g., with
force_respond=True); update the logic around msg_store.get_message (and the
variables original_text and original_id) to treat a failed lookup or empty
content as a hard failure: if original lookup raises or returns no usable
content, do not proceed to enqueue/force a regenerate—return or skip enqueueing
instead; add an explicit truthy check on original_text before calling the
enqueue/regenerate code path so an empty string will not be enqueued.
jaylfc
added a commit
that referenced
this pull request
Apr 19, 2026
…ckages Master had the install scripts from PR #234 but the deployer was still calling bare 'pip3 install <framework>'. That path both (a) skips the bridge setup the scripts do — NO_RESPONSE gating, context rendering, force_respond handling — and (b) fails on Debian 13+ containers with PEP 668's externally-managed-environment error. When tinyagentos/scripts/install_<framework>.sh exists for the requested framework, push it into the container and run it; otherwise fall back to pip with the --break-system-packages flag so generic frameworks still install on modern Debian. Script path gets a 15-minute timeout because uv/hermes/langroid setup downloads can run long.
This was referenced Apr 19, 2026
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
1 participant
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.
Summary
Transforms the parallel 1:1-per-agent chat into a functional multi-agent room:
NO_RESPONSE)@mentions(@slug,@all,@humans) drive addressing;@mentionoverrides cooldown/hop cap👎by a human on an agent message → regenerate;🙋by an agent → wants-reply flag (5 min TTL)/namespaces (OpenClaw / SmolAgents / Hermes all ship their own/commands)/-prefixed message must@<agent>or@all, else 400. Prevents accidental broadcast of framework commands.Deferred to Phase 2 (separate brainstorm): slash autocomplete menu in the composer, per-framework command discovery.
Spec + plan
docs/superpowers/specs/2026-04-19-chat-phase-1-conversational-core-design.mddocs/superpowers/plans/2026-04-19-chat-phase-1-conversational-core.mdNew modules
tinyagentos/chat/mentions.py—parse_mentions+MentionSetdataclasstinyagentos/chat/group_policy.py— per-agent cooldown + per-channel rate cap trackertinyagentos/chat/context_window.py— rolling conversation-history buildertinyagentos/chat/reactions.py— semantic reactions +WantsReplyRegistryKey files modified
tinyagentos/agent_chat_router.py— mention-aware fanout, hop propagation, policy gatestinyagentos/bridge_session.py— event payload carrieshops_since_user/force_respond/context; agent replies re-dispatch so other agents see themtinyagentos/routes/chat.py— bare-slash guardrail, reactions POST/DELETE, channel admin PATCH/POST endpointstinyagentos/chat/channel_store.py— default Phase 1 settings backfill + mutation helpers (set_response_mode,set_max_hops,set_cooldown_seconds,mute_agent,unmute_agent)tinyagentos/scripts/install_*.sh— all 6 bridges render context, honourforce_respond, suppressNO_RESPONSErepliesAlso in this PR (docs / infra)
rolling-imagesrelease moved out of the main repo tojaylfc/tinyagentos-imagesso the Releases page is reserved for real taOS releases. Deployer URL updated. Old main-repo release deleted.Test plan
mentions,group_policy,context_window,reactions)/in group → 400;@<slug> /cmd→ ok;@all /cmd→ ok;/in DM → ok; non-slash in group → ok)TAOS_E2E_URL/TOKEN/CHANNEL)Caveats worth a reviewer eye
_handle_replydelta→final streaming path doesn't re-dispatch (persisted staysNone). All current bridges emitkind=finalonly, so it's not a live issue. Revisit when a streaming bridge is added.chat_wshandler atroutes/chat.pypersists messages directly; it doesn't apply the bare-slash guardrail. If the frontend sends chat over WS rather than HTTP POST, that path is a bypass. UI today uses POST for user messages so this is not a live gap, but a follow-up to harden if the picture changes.Follow-ups (separate issues/brainstorms)
/opens autocomplete grouped by agent (tom: /help, /clear …); picking sends as@<slug> /<cmd>frameworks.pyas the first iteration)/topic(currently 500) and/rename(currently 100) already enforced in the REST admin path; review if too tightSummary by CodeRabbit
New Features
/in group channels must target agents or is rejectedDocumentation
Tests