Repository navigation
Conversation
…t kinds (#16947) Discovery today is four unconnected registries (#6828), none of them a live named "who is here and are they busy" view: AgentHealthRegistry tracks AI-stack health not activity, DistributedAgentManager tracks distributed task assignment, agent-terminal's SessionManager holds live sessions with no shared identity shape, and AgentOrgNode's heartbeat state is a poll, not live. Sessions had no registry at all. - protocols/agent_kind.py: AgentKind (COMPANY_OS, AI_STACK, SESSION, EXTERNAL), split from agent_communication.py to avoid growing a file already at its size-ratchet ceiling, and to avoid a circular import with the new presence module. - protocols/agent_communication.py: AgentIdentity gains kind/name/tenant_id (the shared identity model from #16946) -- additive, defaults preserve every existing caller's behavior exactly. Net zero growth against the 805-line ratchet ceiling: three restating-the-obvious comments removed (two in the CLI demo block, one duplicate `# noqa: E402`) to offset the new fields. - protocols/agent_presence.py: AgentPresenceRegistry, in-memory and TTL-bounded (AUTOBOT_AGENT_PRESENCE_TTL_SECONDS, default 90s) -- (kind, tenant_id, name) -> busy/idle. report() rejects a different instance_id claiming a live name (squat protection, #16946 §2) but lets the same instance heartbeat freely, and lets a new instance reclaim a name once the old one goes stale. EXTERNAL is refused outright -- owner decision 3: external A2A peers get identity for attribution only, never discovery. - protocols/agent_presence_feeds.py: pull adapters for each kind's own authoritative source (never a caller-declared blob, per #16946 §2) -- Company OS from AgentOrgNode + latest heartbeat run status (mirrors llc/api/agents.py's own join), AI-stack from AgentHealthRegistry (busy honestly reported as unknown/idle -- DistributedAgentManager's active_tasks would refine this, left as a follow-up rather than guessed), sessions from agent-terminal's SessionManager via has_running_task(). - 19 new tests: collision rejection, same-instance heartbeat, stale-name reclaim, TTL expiry (entries actually pruned from the live table, not just filtered at read time), EXTERNAL refusal, deregister ownership, and each feed adapter against a fake of its own source. Lands the identity type and the registry interface first, as instructed -- #16948 and #16949 (both blocked_by this) can now build against a real interface. The AI-stack busy signal and any push-based (non-pull) feed wiring are explicitly left as follow-ups, not silently dropped. Refs #16947
#16947) Review found tenant_id=None doing two jobs: shared infrastructure (correct for AI-stack) and unknown (wrong for a Company OS node with no company_id, and for every session unconditionally) -- an agent whose tenant couldn't be determined was becoming visible to every tenant's query. - New UNKNOWN_TENANT sentinel, distinct from None. Never returned by list_live() under any argument, including the sentinel itself passed directly -- fails closed rather than becoming a backdoor to unknowns. - sync_company_os_presence: a null company_id is UNKNOWN_TENANT, not shared. - sync_session_presence: UNKNOWN_TENANT unconditionally. Investigated the conversation_id -> owner -> org path asked for and it does not exist in code -- chat_history's session/conversation mixins (base.py, session.py, security.py) carry no user_id/org_id anywhere, and the owner username threaded through SessionManager.create_session is never persisted onto AgentTerminalSession for a later lookup to recover. Reporting this back rather than picking something else, per the review's own instruction. The real fix is threading tenant through at session-creation time, not deriving it after the fact from a field that cannot carry it. - list_live(tenant_id=None) -- returns {tenant_id} union {shared}, never unknown. No unscoped "every tenant" query on purpose, per the review: a caller that needed one would be reimplementing the leak this replaces. - Logged both rejection paths (collision, EXTERNAL) and the TTL env fallback -- were silent before. - Module docstring now states report()/deregister() enforce no caller identity of their own; the authoritative-source guarantee holds only because the three feed adapters are the sole intended callers. - 6 new tests: the negative control (tenant X doesn't see tenant Y), shared entries included in a tenant query, no-arg defaults to shared-only, an UNKNOWN_TENANT entry never surfaces under any query, a null-company_id node is unknown not shared, sessions are UNKNOWN_TENANT and never listed. Refs #16947
A terminal session had no tenant, and none could be recovered afterwards -- presence (#16947) had to report every session as UNKNOWN_TENANT. The route that looked like it might work does not exist: conversation_id is a log- linkage id only, chat_history's session/conversation stores carry no user/tenant field, and the owner username threaded through SessionManager.create_session was never persisted for a later lookup to recover. - AgentTerminalSession gains an explicit tenant_id field. - Threaded through SessionManager.create_session -> AgentTerminalService.create_session -> the one real call site with authenticated context: POST /api/agent-terminal/sessions, which already resolves `owner` from current_user.get("username") for the WebSocket ownership gate (#14989/#14960) -- tenant_id is current_user.get("org_id") alongside it, the JWT claim only. TerminalCreateSessionRequest has no org_id/tenant_id field, so a caller cannot supply one through this route. - sync_session_presence now reads session.tenant_id, falling back to UNKNOWN_TENANT only when it's unset -- a pre-#16975 session, or one created via a path with no authenticated context (e.g. the chat-tool internal session creation in tools/terminal_tool.py, which passes neither owner nor tenant_id today and is left alone: no code touches those paths in this PR, so they correctly keep reporting UNKNOWN_TENANT rather than a guess). - 6 new tests: SessionManager.create_session threads tenant_id onto the session (and leaves it unset without one); presence reports a session under its real tenant and it is invisible to a different tenant's query (the AC's own discoverability negative control); a session with no tenant stays UNKNOWN_TENANT and is never listed. - service.py's ratchet lowered 962 -> 959 (landed below ceiling after trimming its own docstring to make room for the new parameter). Refs #16975
Review found to_persist_dict() left tenant_id out, and get_session()'s
Redis-reload branch rebuilt AgentTerminalSession without it -- a session
stamped with its tenant at creation lost the stamp on any reload (a
restart, an eviction, another worker) and reappeared as UNKNOWN_TENANT.
Fails closed, so nothing leaked, but it broke AC1 across a reload.
- to_persist_dict() now includes tenant_id.
- get_session()'s Redis-reload branch restores it via
session_data.get("tenant_id").
- The pending-approval rebuild path is left as UNKNOWN_TENANT deliberately
-- it predates this PR and never guessing is correct there.
- New test drives the real persist -> drop from memory -> get_session
round trip (not the two methods in isolation), then confirms the
reloaded session is discoverable through the real presence registry in
its own tenant only.
Refs #16975, #16978 review
…16947) AC2 says busy/idle is live -- both stubs were not. has_running_task() always False (running_command_task was declared but never assigned anywhere outside models.py), and sync_ai_stack_presence hard-coded busy=False and called it a follow-up. - services/agent_terminal/service.py: new _run_tracked() helper wraps both PTY-execution call sites (_execute_auto_approved_command, _run_approved_command_body) so running_command_task is actually set for the command's real span and cleared in a finally, success or exception. - chat_workflow/manager.py: ChatWorkflowManager gains _active_streams (incremented/decremented around process_message_stream's real try/ finally, not a copy of it) and is_processing(). No persistent per-role AI-stack process exists, so this is "any stream in flight right now" for the whole registry, not attributed to one role -- documented as the honest simplification, not a guess. - protocols/agent_presence_feeds.py: sync_ai_stack_presence takes an optional chat_workflow_manager and reports its is_processing() as busy for every role; omitting it reports idle, same as before. - AUTOBOT_AGENT_PRESENCE_TTL_SECONDS registered in autobot_shared/env_registry_agent_runtime.py -- the code-quality red: it was documented by hand in ENV_VARS.md but never registered, so check_env_var_registry.py flagged it as read-but-unregistered. docs/developer/ENV_VARS.md regenerated from the registry (byte-identical to the hand-written version once regenerated). - 7 new tests: is_processing() true only for a stream's real span, across overlapping streams, and clearing on exception; has_running_task() true only for a command's real span and clearing on exception. - File-size ratchet: manager.py and service.py both landed below their ceiling (redundant-comment trims offsetting the new lines) and are lowered to match, not left with slack. Refs #16947
#16947) duplication-guard: sync_company_os_presence's docstring said it "mirrors llc/api/agents.py::list_agents's own latest-run-per-agent join" -- it did, literally, 15 lines of the exact same SQLAlchemy query pasted into both files. jscpd correctly caught it (9 lines over the whole-tree pin; main alone measures 11718, 5 under it, so this diff's own +14 was the entire overage). agent_org_nodes_with_latest_heartbeat(company_id) in llc/api/agents.py is now the one definition; list_agents and sync_company_os_presence both call it. Behavior-preserving -- same subquery, same outer joins, same where clause; list_agents adds its own .order_by(AgentOrgNode.name) after. Verified with the same jscpd invocation CI runs (autobot-backend + autobot-frontend/src, -k 70 -l 8 --skip-comments): 11718 duplicated lines, exactly main's own clean baseline, no longer 11732. Refs #16947
… issue-16947-agent-presence
… issue-16975-session-tenant
A peer agent can now reach another by its stable presence name (protocols/peer_inbox.py) instead of only through a human: delivery is scoped by tenant (never crosses tenants, and UNKNOWN_TENANT / EXTERNAL identities are never addressable) and drained at each kind's real per-turn boundary, never mid-task -- AI_STACK's chat_workflow/tool_handler.py::_dispatch_tool_call and SESSION's services/agent_terminal/service.py::execute_command, the same seams mapped for #16947's busy/idle signal. A peer message carries no approval of its own: a sensitive tool call it triggers still goes through the existing human-approval gate unchanged. protocols/idle_notice.py adds the other half of #16949: a one-shot notice the moment a busy peer's presence transitions to idle, built on the existing EventBus (events/bus.py) rather than a new transport, with a bounded wait (AUTOBOT_IDLE_NOTICE_TIMEOUT_SECONDS) so a waiter is told explicitly on expiry instead of blocking forever.
Code review on PR #16996 found that sync_ai_stack_presence (#16947) reports every AgentHealthRegistry role -- "chat", "rag" and "system_commands" -- as its own addressable AI_STACK presence entry, but the only drain wired for #16948 is chat_workflow's "chat" seam. Addressing "rag" or "system_commands" succeeded silently and then lost the message forever, since nothing ever drains those inboxes: rag_agent.py and system_command_agent.py run their own StandardizedAgent.process_request flow, with no relationship to chat_workflow's dispatch seam. PeerInboxDirectory.send() now refuses any AI_STACK name outside the addressable set with the same RecipientNotAddressableError already used for tenant/EXTERNAL refusals, so misaddressing fails loud instead of silently. Filed #16997 (sub-issue of #16946) to map rag/system_commands' own real turn boundary and lift the restriction once wired.
…aches every other agent, a forwarding hop gets its reply (#16986)
… channel, peer requests through tracking and claims (#16986)
#16965) Code review found sync_company_os_presence/sync_ai_stack_presence/ sync_session_presence and get_presence_registry had zero production callers -- correct adapters, never invoked, so the registry existed but was never populated. This wires both ends: - initialization/agent_presence_sync.py: a background task (extracted out of lifespan.py, which is grandfathered at a fixed line count and may not grow) that calls all three feed adapters on an interval (AUTOBOT_AGENT_PRESENCE_SYNC_INTERVAL_SECONDS, default 30s, comfortably under the registry's own 90s TTL). Started from lifespan.py's Phase 2, cancelled in lifespan_shutdown.py. - api/agent_presence.py: GET /agents/presence reads list_live() back, scoped to the caller's own tenant from the verified JWT/session context (require_org_context), never a request parameter. - llc/services/agent_presence_queries.py: agent_org_nodes_with_latest_heartbeat moved out of the router module llc/api/agents.py (a nit from the same review) plus a new distinct_company_ids_with_agents(), so the sync loop can iterate every tenant without protocols/ importing a FastAPI router.
… issue-16975-session-tenant-work
… issue-16948-16949-peer-messaging-merge # Conflicts: # autobot_shared/env_registry_agent_runtime.py
Independent review of PR #16996 verified idle_notice.py's own design sound (exactly one notice on a busy-to-idle transition, a bounded expiry, the real EventBus, correct {"type","payload"} unwrapping) and recommended splitting it into its own PR so it can land while #16948 waits on #16986's agent_communication consolidation. protocols/idle_notice.py: notify_agent_idle() publishes EVT_AGENT_IDLE via events/bus.py (persist=NONE, so it reaches both the in-process EventManager listener and the live/websocket channel) on exactly a busy=True -> busy=False transition; wait_for_idle() resolves immediately for an already-idle target, otherwise subscribes for one matching event or AUTOBOT_IDLE_NOTICE_TIMEOUT_SECONDS (default 300s), raising IdleWaitExpiredError on timeout and always unsubscribing. Review nit fixed: line 90 now uses asyncio.get_running_loop(), matching the same call already used at line 56, instead of the deprecated get_event_loop(). protocols/agent_presence.py::report() calls notify_agent_idle() on the transition, outside the lock (no re-entrancy, no blocking call while held).
…nce, one overload log per episode (#16986)
…isolation (#16947, #16965) Second independent-review round on PR #16965's wiring commit: - HIGH: AUTOBOT_AGENT_PRESENCE_SYNC_INTERVAL_SECONDS and AUTOBOT_AGENT_PRESENCE_TTL_SECONDS were read with plain env_float/ os.getenv, unclamped. A 0 or negative interval makes asyncio.sleep return immediately, busy-spinning _sync_once()'s DB/Redis/health- registry reads every tick; a 0 or negative TTL makes every entry look stale the instant it is reported. Both now use env_float_clamped (min 1s, max 3600s), and both EnvVarSpecs gained a matching range=. - MEDIUM: api/agent_presence_test.py's "can't pick another tenant via a parameter" test overrode require_org_context directly, so it never exercised the real chain and proved nothing about parameters. Rewritten to drive get_tenant_context/_check_org_membership for real through a fake DB session: a non-member is refused (403), a member requesting their own tenant succeeds, and a platform admin choosing another tenant succeeds -- the last one is a real, pre-existing, JWT-claim-gated exception this route was never meant to hide, so the route's docstring now names it explicitly instead of claiming "never a request parameter". - LOW: one company's sync_company_os_presence failure no longer skips the rest of the pass (try/except-and-continue, logged with the company id); added a _loop()-level test proving a failed iteration is logged and the next iteration still reaches and populates the registry.
… issue-16975-session-tenant-fix2
… issue-16948-16949-peer-messaging-merge2
… issue-16949-idle-notice
# Conflicts: # autobot-backend/protocols/idle_notice.py
# Conflicts: # autobot-backend/protocols/agent_communication.py # docs/developer/ENV_VARS.md
mrveiss
added
the
land-next
Contributor
|
Warning Review limit reachedNext included review available in 56 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: mrveiss/AutoBot-AI/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (40)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 of 4 tasks
This was referenced Sep 19, 2026
Contributor
Auto-regenerated (py3.14) to match the backend and/or SLM backend schema so the required verify-generated-types gate(s) pass. Triggered by auto-fix-generated-types.yml.
This was referenced Sep 19, 2026
Contributor
✅ SSOT Configuration Compliance: Passing🎉 No new hardcoded values of either class — Known backlog in |
mrveiss
added a commit
that referenced
this pull request
Sep 19, 2026
mrveiss
removed
the
land-next
Owner
Author
Owner
Author
|
Carried and landed — closing as a member of the rest train #17133, which merged to Evidence this PR's work is in
No code is lost and nothing here needs re-doing. The issues this PR was opened against are closed by #17133's The branch is left in place for now; it will be removed only after a check that no other PR targets it as a base. |
3 tasks
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.
Closes #16947
Closes #16975
Closes #16949
Closes #16986
Refs #16948
This vehicle carries the agent-coordination stack: five open PRs whose sessions are gone. They were claimed in the owner-directed pool split (2026-09-19) and consolidated to land as one PR.
issue-16947-agent-presenceae03d7e31eissue-16975-session-tenant89f9b96dffissue-16949-idle-noticeaf744a8dc7issue-16948-16949-peer-messaging6070c535c8issue-16986-peer-routing74ab39e95dThinking Path
The five members stack on each other: presence ← tenant ← {peer messaging, idle notice}, with peer routing on
main. Every tip was merged in order onto currentmain. Three things needed resolving:protocols/idle_notice.py, add/add. feat(agents): peer-to-peer messaging and idle notice (#16948, #16949) #16996's feat(agents): subscribe once to be told when a peer is next idle #16949 part and feat(agents): one-shot idle notice (#16949) #17015's version collided. feat(agents): one-shot idle notice (#16949) #17015's was kept, per the coordinator ruling that feat(agents): one-shot idle notice (#16949) #17015 is the canonical feat(agents): subscribe once to be told when a peer is next idle #16949 delivery. Review diffed the two: the only difference isget_event_loop()→get_running_loop().AUTOBOT_IDLE_NOTICE_TIMEOUT_SECONDS, and the text merge kept both identical blocks. One was removed, andENV_VARS.mdregenerated.protocols/agent_communication.pyimports. agents: the peer channel never delivers to the recipient — a request lands in the sender's own inbox and the sender answers itself #16986 moved the channels intoprotocols/agent_channels.py, and feat(agents): live presence — one registry of named agents with busy/idle state, across all three kinds #16947 addedAgentKind. The imports are the union, andsend_messageis agents: the peer channel never delivers to the recipient — a request lands in the sender's own inbox and the sender answers itself #16986's recipient routing. One import was folded to keep the file under its ceiling (now 706 in both mirrors).Why #16948 is Refs, not Closes. #16948's receive side is wired: peer inboxes drain at the chat turn boundary (
tool_dispatch_guards.enforce_peer_messages) and at the terminal command boundary (service._drain_peer_messages). Its send side is not.PeerInboxDirectory.send()has no production caller, so nothing a real agent does puts a message into a peer's inbox. #16996 itself said it staysRefsuntil #16986 lands and an end-to-end test through the real transport passes. #16986 lands here, but that wiring and test weren't written, so #16948 stays open for them.Overlap with #17106: #16975's tenant capture and #17106's session owner both touch
services/agent_terminal/session_manager.py. Only thecreate_sessiondocstring conflicts, so whichever PR lands second takes it.What Changed
protocols/agent_presence.py,agent_presence_feeds.py,agent_kind.py,api/agent_presence.py,initialization/agent_presence_sync.pyplus lifespan start/stop,llc/services/agent_presence_queries.py. Identity fields are onAgentIdentity.AgentTerminalSession.tenant_idis captured from the JWT at creation, persisted, and survives a reload.protocols/idle_notice.py.protocols/peer_inbox.py, plus its drains inchat_workflow/tool_dispatch_guards.pyandservices/agent_terminal/service.py.protocols/agent_channels.py(recipient delivery, the atomic id claim, capped inboxes) andagent_communication.py;base_agent.pyroutes peer requests throughexecute_with_tracking.ENV_VARS.md; size baselines at the measured counts (manager.py4067,lifespan.py2362,agent_communication.py706,agent_terminal/service.py951).Verification
Pre-push on
d568726c59:[pre-push OK] pytest: all relevant tests passfor 13 colocated files:api/agent_presence_test.pychat_workflow/manager_is_processing_16947_test.pychat_workflow/peer_messages_dispatch_16948_test.pyinitialization/agent_presence_sync_test.pyinitialization/lifespan_test.pyprotocols/agent_channels_test.pyprotocols/agent_presence_feeds_test.pyprotocols/agent_presence_test.pyprotocols/idle_notice_test.pyprotocols/peer_inbox_test.pyservices/agent_terminal/peer_messages_dispatch_16948_test.pyservices/agent_terminal/running_command_task_busy_signal_16947_test.pyservices/agent_terminal/session_manager_tenant_16975_test.pyAlso: bandit exits 0 on every changed source file; flake8 is clean on all 37 changed Python files; the baseline mirrors match the measured line counts.
Code review of the union (sonnet, at
d568726c59):directandredischannels, not a stand-in.The member PRs are closed as carried, each citing its head above. Their branches are kept, so a deleted base can't auto-close anything.
Model Used
Claude Opus 5 (
claude-opus-5)🤖 Generated with Claude Code