feat(orchestration): Master chat — human↔OpenHuman control channel + working receive loop#4660
Merged
senamakel merged 13 commits intoJul 7, 2026
Conversation
…ory + send on behalf)
… history
Master chat, read slice: give the orchestration reasoning core read-only tools
to browse its persisted OpenHuman<->agent session transcripts so it can answer a
question from its own history of chats with other agents, instead of only the
single window it was woken for.
- orchestration_list_sessions: enumerate saved agent chat sessions (peer, label,
last activity, message count, one-line preview), pinned windows hidden.
- orchestration_read_session: read one session's transcript by id (chronological,
pageable via before).
Both ReadOnly + concurrency-safe; workspace-internal store access via
store::{list_sessions,count_messages,list_recent_messages,list_messages_by_session}.
Registered in tools/ops.rs and added to the reasoning_agent allowlist + prompt.
Tests: cargo test openhuman::orchestration::tools -> 7/7 (3 new); full
orchestration 61/61, loader 85/85.
Builds on tinyhumansai#4599 (reliable reactive reply loop). Next: gated send-on-behalf tool.
Master chat, send slice (W5 + W6): give the reasoning core a tool to message
another agent on OpenHuman's behalf (e.g. to ask a peer something for the user).
- orchestration_send_to_agent { recipient, message, sessionId? }.
- Guardrail (linked-peers-only): recipient must be a linked/paired agent or one
with an existing session; refuses cold-DMs, since the core runs under a
background origin that bypasses the interactive approval gate.
- Session id (reuse-or-mint per peer): new store::latest_session_for_agent reuses
the peer's newest thread's shared wrapper_session_id so the reply threads back
(#227/tinyhumansai#4582); mints a fresh uuid only when no thread exists. Explicit sessionId
overrides.
- Sends a v1 session envelope via handle_tinyplace_signal_send_message and records
the outbound role=owner message + notify_orchestration_message (mirrors tinyhumansai#4599
persist_outgoing_reply) so it shows in the chat + own history.
- PermissionLevel::Write; registered in tools/ops.rs; reasoning_agent allowlist +
prompt updated.
Reply threading back into the originating master question (W7) is still open
(needs envelope inReplyTo correlation, F3).
Tests: cargo test openhuman::orchestration -> 64/64 (6 new incl. guardrail-refusal
and session reuse); loader 85/85; lib compiles clean.
…(W7)
Master chat, reply-threading (W7, core-only): when OpenHuman DMs a peer on the
user's behalf and the peer replies, thread that answer back into the window the
ask came from instead of auto-replying to the peer.
- One-shot correlation: the execute node scopes the origin window
(tools::with_origin_session task-local); orchestration_send_to_agent records
store::set_pending_ask(ask_session -> origin).
- On the peer's reply, invoke_with_runtime threads the newest inbound message into
the origin window (thread_reply_to_origin) and finishes the cycle WITHOUT running
the reply graph -- no ping-pong reply to the peer. Consumed one-shot via
store::{pending_ask_origin,clear_pending_ask}.
- Additive/safe: only sessions OpenHuman itself initiated carry a pending marker;
peer-initiated and master wakes are unchanged.
Store: kv_delete + set_pending_ask/pending_ask_origin/clear_pending_ask.
Limitation (needs F3, cross-repo): pragmatic 1:1 correlation assumes the next
inbound message on the ask session is the answer; robust many-in-flight needs an
envelope inReplyTo. The threaded answer is the peer's raw reply (no re-synthesis
of a final answer to the human yet).
Tests: cargo test openhuman::orchestration -> 66/66 (2 new: reply-threading skips
the graph + surfaces to master; pending-ask one-shot); lib clean; fmt clean.
…on) (F2) The wake graph checkpointed under thread orchestration:<session_id>, but the store keys sessions by (agent_id, session_id). Two peers sharing a session id (a legacy harness_session_id fallback collision, now more likely with reuse-or-mint outbound asks) could resume from each other's checkpoint. Scope the thread id to orchestration:<counterpart>:<session_id> so it matches the store namespace. Migration seam noted in-code: a cycle checkpointed under the old key that only resumes after this upgrade starts a fresh thread (one extra in-flight cycle at the boundary; Beta, gated by [orchestration]) — same worst case as the cycle_id format change. Deferred by tinyhumansai#4599 as a follow-up. Tests: orchestration 66/66 (thread-id assertion updated).
Make the human↔OpenHuman Master chat route into OpenHuman's OWN reasoning graph
instead of requiring an external peer recipient.
Previously a Master-chat send with no recipient errored 'no Master counterpart
yet — specify a recipient' (schemas.rs) — it was only wired to steer an external
peer. But the Master chat is where the human talks to the OpenHuman agent to
control/communicate with external agents.
- types.rs: LOCAL_MASTER_AGENT sentinel ('openhuman:local') — the counterpart for
a local human->OpenHuman cycle (never a real base58 address).
- schemas.rs handle_send_master_message: when no recipient AND no sessionId, take
the local-ask path — persist the question (role=user) in the master window with
a monotonic seq, notify the renderer, and publish OrchestrationSessionMessage to
wake the reasoning core locally. No outbound DM, no recipient required. Explicit
recipient/sessionId still steer a peer as before.
- ops.rs ProductionRuntime::send_dm: when the counterpart is LOCAL_MASTER_AGENT,
persist the answer (role=assistant) into the master window and notify — no
tiny.place DM. Reuses the reasoning core's tools + W7 threading (if it asks a
real external agent, that reply threads back into master via origin='master').
Tests: cargo test openhuman::orchestration -> 67/67 (new:
local_master_reply_lands_in_the_window_not_an_outbound_dm).
…ions Complete the master-chat browse loop so the OpenHuman agent can go contacts -> that contact's sessions -> a session's history: - orchestration_list_contacts (new): enumerate the agent's tiny.place contacts, delegating to the tiny.place contacts_list controller (re-exported handle_tinyplace_contacts_list from tinyplace::manifest). Read-only. - orchestration_list_sessions: optional contactId filter -> only that contact's sessions (contact-wise view). - Added to the reasoning_agent allowlist + prompt (browse loop documented). Tests: cargo test openhuman::orchestration -> 68/68 (new list_sessions_tool_filters_by_contact); loader 85/85.
For a local human->OpenHuman master cycle (counterpart = LOCAL_MASTER_AGENT), the
wake graph no longer runs the A2A front-end agent — the human talks straight to
the reasoning core (OpenHuman):
- graph/build.rs frontend node: on a local master cycle, pass 1 feeds the core a
direct human-facing directive (instead of frontend_instruct's A2A triage), and
pass 2 uses the core's reply verbatim (instead of frontend_compile's A2A
'compile'). Peer-initiated cycles still run the two-pass front end unchanged.
The A2A front-end prompt ('a wrapped Claude/Codex session is talking to you') and
the reasoning prompt ('you are not talking to the user directly; the front end
will phrase the reply') were the wrong framing for the human master chat — this
routes around the front end for that direction.
Tests: cargo test openhuman::orchestration::graph -> 12/12 (new:
local_master_cycle_skips_the_a2a_frontend_agent); full orchestration 69/69.
…ster chat
Add a master_agent built-in — OpenHuman talking DIRECTLY to its human — derived
from the reasoning core (same reasoning tier, same tiny.place tool belt +
sub-agent allowlist) but with a human-facing system prompt instead of the A2A
split-brain framing ('you are not talking to the user directly; the front end
will phrase the reply').
- orchestration/master_agent/: agent.toml (clone of reasoning_agent config, id
master_agent), prompt.md (human-direct: you are OpenHuman talking to your human;
browse contacts/sessions/history; act on their behalf), prompt.rs (reuses the
reasoning core's steering task-local), mod.rs.
- loader.rs: register master_agent (reuses reasoning_agent's default turn graph);
add it to the non-worker-tier test allowlist.
- ops.rs execute node: run master_agent (human-conversation prompt) for a local
master cycle (agent_id == LOCAL_MASTER_AGENT); reasoning_agent (A2A macro-
instructions framing) for peer cycles.
- build.rs: local-master pass-1 directive trimmed to a light nudge (master_agent's
system prompt now carries the framing).
Tests: orchestration 69/69, loader 85/85 (master_agent registers + tools resolve).
…unconfigured) On staging, reasoning-v1 400s with 'API key not configured for provider' and has no fallback, while chat-v1 works (burst-v1 fails but auto-falls-back to chat-v1). The master_agent was running on hint:reasoning -> reasoning-v1, so every master turn died. Route the local master turn to hint:chat (chat-v1) — master chat is a direct human conversation, the chat tier is the right fit and the working one. - ops.rs execute: local master -> (master_agent, hint:chat); A2A -> (reasoning_agent, hint:reasoning). - master_agent/agent.toml: [model] hint = chat. Tests: orchestration 69/69.
The master chat could send to peers but never surfaced their replies. Two root causes, both fixed here: 1. OpenHuman was undiscoverable. Its Signal encryption key + pre-keys were never published (a manual two-click Messaging-UI action), so peers 404'd on its prekey bundle and could not encrypt a reply back — the receive loop was silently dead. Add `tinyplace::ensure_signal_keys_published` (idempotent provision + register) and call it from the orchestration drain supervisor so any orchestration-enabled instance auto-publishes and becomes reachable. 2. The W7 reply-threading correlation never armed. `send_to_agent` read the ask's origin from a `tokio::task_local`, but the agent harness dispatches tool calls beyond an internal `tokio::spawn`, and task-locals do not cross a spawn — so `pending_ask` was never set and replies fell through to the peer path. Replace it with a process-global master-origin beacon the `execute` node opens around the local-master turn; the tool reads it reliably. When a master-initiated ask's reply arrives, surface it as OpenHuman's OWN assistant message via `report_peer_reply_to_master` (runs the master_agent on the chat tier), not the peer's raw words and not a user turn — scoped to master-triggered asks; peer/A2A replies keep the raw threadback. Make `send_to_agent` fire-and-forget: the tool note + master_agent prompt now tell the agent the reply is surfaced automatically and to NOT poll/read_session for it, so W7 is the sole async reporter (was double-surfacing — the agent polled-and-reported AND W7 pushed). Verified live on staging: master ask -> send + immediate ack -> peer reply -> exactly one OpenHuman report in the master chat; pending_ask armed then cleared.
…ocal dev overrides)
Contributor
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 11 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR introduces a local Master chat loop for direct human-to-OpenHuman conversation, adding a new ChangesMaster Chat Feature Implementation
Scoping Documentation
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Human
participant Schemas
participant Ops
participant MasterAgent
participant Store
participant Peer
Human->>Schemas: send Master message (no recipient/sessionId)
Schemas->>Store: persist user message in Master window
Schemas->>Ops: publish OrchestrationSessionMessage event
Ops->>MasterAgent: run local Master cycle (skip A2A frontend)
MasterAgent->>Store: read session history / list contacts
MasterAgent->>Peer: orchestration_send_to_agent (async ask)
Store->>Store: set_pending_ask(origin=Master)
Peer-->>Ops: DM reply
Ops->>Store: pending_ask_origin lookup
Ops->>MasterAgent: generate assistant report from reply
Ops->>Store: thread report into Master window
Store-->>Human: renderer notification (answer shown)
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
coderabbitai
Bot
added
agent
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 733e9ef0aa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let mut discoverable = false; | ||
| loop { | ||
| if !discoverable { | ||
| match crate::openhuman::tinyplace::ensure_signal_keys_published().await { |
There was a problem hiding this comment.
When [orchestration].enabled is set to false, this supervisor still runs at core startup and calls ensure_signal_keys_published() before loading the config or checking the flag. That helper can provision prekeys and publish the encryption key to the directory, so a user who disabled orchestration still gets made discoverable and has remote tiny.place state mutated. Load the config and skip this block when orchestration is disabled before attempting the publish path.
Useful? React with 👍 / 👎.
| if let Some(origin) = current_master_origin().or_else(current_origin_session) { | ||
| if origin != session_id && !origin.is_empty() { | ||
| if let Err(e) = store::with_connection(&workspace, |conn| { | ||
| store::set_pending_ask(conn, &session_id, &origin) |
There was a problem hiding this comment.
This records the reply-correlation key with only session_id, while sessions elsewhere are keyed by (agent_id, session_id) and this same change acknowledges legacy session-id collisions in the graph checkpointer. If two linked peers share a legacy harness_session_id, any inbound message from the other peer on that session id will satisfy pending_ask_origin(config, session_id), get threaded back as the answer, clear the one-shot pending ask, and prevent the real recipient's reply from being reported correctly. Include the recipient/counterpart in the pending-ask key and lookup.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (1)
src/openhuman/orchestration/ops.rs (1)
1030-1044: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winClear the process-global master origin with a guard.
end_master_origin()runs after the awaited turn, but cancellation or panic before that point can leave the global beacon stale for a later wake. Use an RAII/drop guard aroundbegin_master_originso cleanup is tied to future drop.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/orchestration/ops.rs` around lines 1030 - 1044, The process-global master origin in the orchestration path can be left stale if the awaited turn exits early, so update the logic around begin_master_origin and end_master_origin in the reasoning turn flow to use an RAII guard whose Drop cleanup always clears the origin. Keep the existing is_local_master check, but wrap the begin_master_origin call in a guard scoped across the await in the with_steering / with_origin_session / run_agent_turn sequence so cleanup is tied to future drop instead of relying on a trailing end_master_origin call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/scoping/master-chat.md`:
- Around line 178-186: Update the sub-flow status matrix in master-chat.md so
Row 1 reflects the landed master-chat behavior instead of pre-PR baseline state.
Use the existing matrix entry and surrounding master-chat summary to relabel it
as historical context or mark it as now-supported, and make sure the wording
matches the actual master-initiated reply path rather than saying there is no
“answer the asker” surface.
- Around line 247-263: Update the W7 paragraph to match the implemented
reply-correlation flow: replace the task-local origin-scoping description with
the process-global beacon approach that survives tokio::spawn, and describe that
the threaded reply is surfaced as OpenHuman’s own message rather than the peer’s
raw reply. Keep the one-shot correlation behavior and clear the pending marker
after the first inbound response, using the existing orchestration symbols like
execute, orchestration_send_to_agent, invoke_with_runtime,
thread_reply_to_origin, and store::{pending_ask_origin,clear_pending_ask} to
locate the text.
In `@src/openhuman/orchestration/ops.rs`:
- Around line 387-403: The master reply path in ops::run_agent_turn is unsafe
because it feeds untrusted peer text into the full tool-enabled master_agent
with master transcript context. Change this flow to use a report-only, tool-free
reporter path instead of ProductionRuntime::run_agent_turn for the peer reply,
so the response is summarized without exposing OpenHuman’s tools or session
context to prompt injection. Keep the existing prompt construction in the
reply-handling block, but route it through a dedicated no-tools model or
reporting method identified by the master-agent reporting logic.
- Around line 2079-2091: The master-report assertion is currently
nondeterministic because report_peer_reply_to_master can build a
ProductionRuntime and persist a generated report instead of the expected raw
reply. Update the test setup around invoke_with_runtime and the pending-ask path
to use a deterministic provider override, or explicitly force the report
generation to fail in a controlled way, so the stored master message is
predictable before checking for "shipped v2".
- Around line 282-316: The pending-ask flow is only keyed by session_id, which
can route answers to the wrong window or persist them under the answering peer
instead of the original asker. Update pending_ask_origin and the related
reply-threading path in thread_reply_to_origin to carry the full origin
identity, including origin_agent_id plus origin_session_id, and use that origin
agent when inserting/notifying and when clearing the consumed ask. Also review
the other call sites in this orchestration flow that read or store pending asks
so they use the same origin tuple consistently.
- Around line 1224-1260: The local master persistence path in the orchestration
send flow currently swallows store failures and still returns success, which
lets the graph advance even when the assistant message was not saved. Update the
`LOCAL_MASTER_AGENT` branch in `ops::...` around the `store::with_connection`
block to propagate the error instead of only logging it, so the caller sees the
failure and `dm_sent`/cursor advancement do not occur on a bad write. Keep the
failure handling tied to the existing `master_answer.persist_failed` logging and
the `super::bus::notify_orchestration_message` call so notification only happens
after a successful persist.
- Around line 761-772: The reply flow in
orchestration::ops::report_peer_reply_to_master/thread_reply_to_origin is
clearing the pending ask and advancing the cursor even when persistence may have
failed. Make threading success explicit by changing thread_reply_to_origin to
return a status (or otherwise surface success/failure), and only call
clear_pending_ask and advance_cursor after either report_peer_reply_to_master or
raw threading completes successfully.
In `@src/openhuman/orchestration/schemas.rs`:
- Around line 482-510: The master message persistence path currently calls
store::next_session_seq and store::insert_message inside store::with_connection
without an immediate transaction, so concurrent master submits can reuse the
same seq. Update the branch in schemas::orchestration to wrap the seq
allocation, store::upsert_session, and store::insert_message work in
store::in_immediate_txn so the master session sequence is reserved atomically.
In `@src/openhuman/orchestration/tools.rs`:
- Around line 653-681: The session sequence allocation and message insert in
tools.rs are race-prone because `next_session_seq` and `store::insert_message`
are currently executed without the store’s immediate transaction protection.
Update the logic in the `store::with_connection` block for the session/message
write path to run the `next_session_seq` + `upsert_session` + `insert_message`
sequence inside `in_immediate_txn`, matching the concurrency-safe pattern used
by the store’s own tests and preventing duplicate or rejected sequence numbers
when concurrent sends hit the same session.
- Around line 638-648: Arm and persist the pending-ask origin before calling
handle_tinyplace_signal_send_message in tool.send_to_agent so any fast reply can
be threaded correctly. Move the pending_ask computation/storage earlier in the
send flow, using the same recipient/operation context already available in this
branch, and make sure the stored origin is cleared if the outbound send fails so
stale state does not remain.
- Around line 94-111: The `MASTER_ORIGIN` process-global in
`src/openhuman/orchestration/tools.rs` can bleed across concurrent tool calls,
causing unrelated A2A `orchestration_send_to_agent` calls to pick up the master
origin. Update the origin tracking so it is scoped to the specific spawned
tool-call context used by `orchestration_send_to_agent`, or gate reads with a
per-turn token/identifier so only calls from the active master turn can consume
it. Make the change in the origin lookup path that currently prefers
`MASTER_ORIGIN`, and keep the master vs peer-session behavior separated by the
existing `send_to_agent`/`execute` flow.
- Around line 591-596: The explicit session handling in `tools.rs` currently
filters out pinned values and then silently falls back to minting/reusing
another thread; instead, `explicit_session` processing in the tool path should
reject pinned or otherwise invalid `sessionId` values outright. After opening
the store, validate the provided session by comparing
`store::session_agent_id(conn, sid)` against `recipient` before using it, and
fail the request if it does not match. Apply the same validation to the
corresponding peer-message path so `sessionId` cannot be used to persist or send
under another peer’s thread.
In `@src/openhuman/tinyplace/manifest.rs`:
- Around line 3045-3061: The discoverability check in
ensure_signal_keys_published is too permissive because it returns true based
only on local key readiness and encryptionKeyPublished. Update the logic to also
consult the remote health from signal_key_status and only return Ok(true) when
the remote key bundle is confirmed usable; otherwise keep provisioning/retrying
or return Ok(false) so the supervisor does not stop early. Apply the same
remote-health gating to the related retry path in the other
ensure_signal_keys_published branch as well.
---
Nitpick comments:
In `@src/openhuman/orchestration/ops.rs`:
- Around line 1030-1044: The process-global master origin in the orchestration
path can be left stale if the awaited turn exits early, so update the logic
around begin_master_origin and end_master_origin in the reasoning turn flow to
use an RAII guard whose Drop cleanup always clears the origin. Keep the existing
is_local_master check, but wrap the begin_master_origin call in a guard scoped
across the await in the with_steering / with_origin_session / run_agent_turn
sequence so cleanup is tied to future drop instead of relying on a trailing
end_master_origin call.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 166108a9-ba67-49a0-a937-53eab0fa7237
📒 Files selected for processing (19)
docs/scoping/master-chat.mdsrc/openhuman/agent_registry/agents/loader.rssrc/openhuman/orchestration/graph/build.rssrc/openhuman/orchestration/graph/tests.rssrc/openhuman/orchestration/master_agent/agent.tomlsrc/openhuman/orchestration/master_agent/mod.rssrc/openhuman/orchestration/master_agent/prompt.mdsrc/openhuman/orchestration/master_agent/prompt.rssrc/openhuman/orchestration/mod.rssrc/openhuman/orchestration/ops.rssrc/openhuman/orchestration/reasoning_agent/agent.tomlsrc/openhuman/orchestration/reasoning_agent/prompt.mdsrc/openhuman/orchestration/schemas.rssrc/openhuman/orchestration/store.rssrc/openhuman/orchestration/tools.rssrc/openhuman/orchestration/types.rssrc/openhuman/tinyplace/manifest.rssrc/openhuman/tinyplace/mod.rssrc/openhuman/tools/ops.rs
Comment on lines
+591
to
+596
| let explicit_session = args | ||
| .get("sessionId") | ||
| .and_then(Value::as_str) | ||
| .map(str::trim) | ||
| .filter(|s| !s.is_empty() && !is_pinned_window(s)) | ||
| .map(str::to_string); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject invalid or mismatched sessionId instead of falling back.
A pinned sessionId is currently filtered out, so sessionId: "master" silently reuses/mints another thread. Also validate that an explicit session belongs to recipient; otherwise the tool can send/persist a peer message under another peer’s thread id.
Suggested validation shape
- let explicit_session = args
+ let explicit_session = match args
.get("sessionId")
.and_then(Value::as_str)
.map(str::trim)
- .filter(|s| !s.is_empty() && !is_pinned_window(s))
- .map(str::to_string);
+ .filter(|s| !s.is_empty())
+ {
+ Some(s) if is_pinned_window(s) => {
+ return Ok(ToolResult::error(
+ "`sessionId` must be an agent session, not a pinned window".to_string(),
+ ));
+ }
+ Some(s) => Some(s.to_string()),
+ None => None,
+ };Then, after opening the store, compare store::session_agent_id(conn, sid) with recipient before using the explicit session.
Also applies to: 619-621
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/orchestration/tools.rs` around lines 591 - 596, The explicit
session handling in `tools.rs` currently filters out pinned values and then
silently falls back to minting/reusing another thread; instead,
`explicit_session` processing in the tool path should reject pinned or otherwise
invalid `sessionId` values outright. After opening the store, validate the
provided session by comparing `store::session_agent_id(conn, sid)` against
`recipient` before using it, and fail the request if it does not match. Apply
the same validation to the corresponding peer-message path so `sessionId` cannot
be used to persist or send under another peer’s thread.
Comment on lines
+638
to
+648
| // Send over the tiny.place Signal channel (same op the graph's send_dm and | ||
| // the send_master RPC use). | ||
| let mut send_params = serde_json::Map::new(); | ||
| send_params.insert("recipient".to_string(), Value::from(recipient.clone())); | ||
| send_params.insert("plaintext".to_string(), Value::from(plaintext)); | ||
| if let Err(e) = | ||
| crate::openhuman::tinyplace::handle_tinyplace_signal_send_message(send_params).await | ||
| { | ||
| log::warn!(target: "orchestration", "[orchestration] tool.send_to_agent send failed: {e}"); | ||
| return Ok(ToolResult::error(format!("send failed: {e}"))); | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Arm pending-ask before the outbound send.
The current order sends the Signal DM first and records pending_ask only afterward. A fast peer reply can be ingested before Line 704 stores the origin, so the reply misses W7 threading. Compute/store the origin before handle_tinyplace_signal_send_message, and clear it if the send fails.
Also applies to: 692-710
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/orchestration/tools.rs` around lines 638 - 648, Arm and persist
the pending-ask origin before calling handle_tinyplace_signal_send_message in
tool.send_to_agent so any fast reply can be threaded correctly. Move the
pending_ask computation/storage earlier in the send flow, using the same
recipient/operation context already available in this branch, and make sure the
stored origin is cleared if the outbound send fails so stale state does not
remain.
Comment on lines
+653
to
+681
| let persisted = store::with_connection(&workspace, |conn| { | ||
| let seq = store::next_session_seq(conn, &recipient, &session_id)?; | ||
| store::upsert_session( | ||
| conn, | ||
| &OrchestrationSession { | ||
| session_id: session_id.clone(), | ||
| agent_id: recipient.clone(), | ||
| source: String::new(), | ||
| label: None, | ||
| workspace: None, | ||
| last_seq: seq, | ||
| created_at: now.clone(), | ||
| last_message_at: now.clone(), | ||
| }, | ||
| )?; | ||
| store::insert_message( | ||
| conn, | ||
| &OrchestrationMessage { | ||
| id: message_id.clone(), | ||
| agent_id: recipient.clone(), | ||
| session_id: session_id.clone(), | ||
| chat_kind: ChatKind::Session, | ||
| role: "owner".to_string(), | ||
| body: message.clone(), | ||
| timestamp: now.clone(), | ||
| seq, | ||
| }, | ||
| ) | ||
| }); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Wrap seq allocation and insert in in_immediate_txn.
next_session_seq + insert_message is race-prone when two sends target the same session concurrently. The store’s own concurrency test documents that allocation and insert must happen under in_immediate_txn; apply that here to avoid duplicate/rejected seqs after the DM has already been sent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/orchestration/tools.rs` around lines 653 - 681, The session
sequence allocation and message insert in tools.rs are race-prone because
`next_session_seq` and `store::insert_message` are currently executed without
the store’s immediate transaction protection. Update the logic in the
`store::with_connection` block for the session/message write path to run the
`next_session_seq` + `upsert_session` + `insert_message` sequence inside
`in_immediate_txn`, matching the concurrency-safe pattern used by the store’s
own tests and preventing duplicate or rejected sequence numbers when concurrent
sends hit the same session.
Comment on lines
+3045
to
+3061
| let keys_ready = status | ||
| .get("hasActiveSignedPreKey") | ||
| .and_then(Value::as_bool) | ||
| .unwrap_or(false) | ||
| && status | ||
| .get("localPreKeyCount") | ||
| .and_then(Value::as_u64) | ||
| .unwrap_or(0) | ||
| > 0; | ||
| let published = status | ||
| .get("encryptionKeyPublished") | ||
| .and_then(Value::as_bool) | ||
| .unwrap_or(false); | ||
|
|
||
| if keys_ready && published { | ||
| log::debug!("{LOG_PREFIX} ensure_signal_keys_published: already discoverable"); | ||
| return Ok(true); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Include remote key-bundle health before declaring discoverability.
Ok(true) currently only means local keys exist and the directory card has encryptionPublicKey. If remote key health is unavailable or depleted, peers may still fail to fetch/encrypt to a usable bundle while the supervisor stops retrying. Use the remote health from signal_key_status to provision/retry or return Ok(false) until the remote bundle is confirmed usable.
Also applies to: 3079-3085
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/tinyplace/manifest.rs` around lines 3045 - 3061, The
discoverability check in ensure_signal_keys_published is too permissive because
it returns true based only on local key readiness and encryptionKeyPublished.
Update the logic to also consult the remote health from signal_key_status and
only return Ok(true) when the remote key bundle is confirmed usable; otherwise
keep provisioning/retrying or return Ok(false) so the supervisor does not stop
early. Apply the same remote-health gating to the related retry path in the
other ensure_signal_keys_published branch as well.
…rrelation, durable surfacing Addresses CodeRabbit + Codex review on the master-chat receive loop: - Prompt-injection: run a peer's UNTRUSTED reply through a new tool-free `master_reporter` agent (no tiny.place tools / sub-agents) instead of the full `master_agent`, and frame the reply as quoted data — a malicious peer can no longer prompt-inject OpenHuman into reading sessions or messaging contacts. - Respect the orchestration opt-out: the drain supervisor loads config first and skips key-publishing + mailbox draining when `[orchestration].enabled` is false (no longer makes a user discoverable / mutates remote directory state against their config). - Scope `pending_ask` by `(peer_agent_id, session_id)`, not session id alone, so a legacy shared `wrapper_session_id` across peers can't consume the ask and misroute the reply. - Consume the one-shot pending ask + advance the cursor ONLY after the reply is durably surfaced: `thread_reply_to_origin` now returns a status and the caller retries on the next drain instead of dropping the answer on a transient store failure. - Local-master `send_dm`: propagate the persist failure (the persist IS the send) so the graph doesn't mark the cycle sent + advance over a lost answer. - Make the W7 test hermetic: exercise the deterministic peer-origin threadback path (the master-origin report runs a real model turn, covered live). - Docs: refresh the scoping matrix + W7 section to the landed beacon / tool-free-report / fire-and-forget flow.
Contributor
Author
|
Thanks @coderabbitai + Codex — addressed the substantive findings in Fixed
Deferred with reason
|
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git log --oneline -5
echo "---"
git show 412eceb --stat | head -50
echo "--- master_reporter ---"
rg -n "master_reporter" -C2 || true
echo "--- pending_ask key scoping ---"
rg -n "pending_ask:" src/openhuman/orchestration/store.rs src/openhuman/orchestration/tools.rs src/openhuman/orchestration/ops.rs || true
echo "--- orchestration.enabled gating in ops.rs ---"
rg -n "enabled" src/openhuman/orchestration/ops.rs | head -30
echo "--- thread_reply_to_origin signature ---"
rg -n "fn thread_reply_to_origin" -A5 src/openhuman/orchestration/ops.rs || trueLength of output: 3741
All of that looks solid. On the deferred items:
If you'd like, I can open follow-up issues for the four deferred items so they don't get lost — just let me know and I'll file them (tagging you as reporter). ฅ^•ﻌ•^ฅ [resolve] |
Open
6 tasks
Contributor
Author
|
@coderabbitai already opened the defferred issues |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
ヽ(•‿•)ノ |
Contributor
Author
|
@coderabbitai re-review then |
Contributor
|
✅ Action performedReview finished.
|
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
2 participants
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
master_agenton the chat model, skipping the A2A front-end triage (that's for agent-to-agent), so replies are in OpenHuman's own voice.tokio::spawnboundary) by using a process-global origin beacon.Problem
The master-chat surface could send to other agents but never received anything back, so it was effectively one-directional:
orchestration_send_to_agentrecorded "who to thread the reply back to" via atokio::task_localset by the orchestrationexecutenode. But the agent harness dispatches tool calls beyond an internaltokio::spawn, and task-locals do not cross a spawn boundary — sopending_askwas never written and peer replies fell through to the ordinary A2A path, never reaching the master chat.Solution
tinyplace::ensure_signal_keys_published(provision pre-keys if missing + register the encryption key if not advertised), called once from the orchestration drain supervisor so any orchestration-enabled instance becomes discoverable/reachable without manual UI steps. Cheap no-op once published; retries until the wallet is unlocked, then stops.executenode opens a master-origin beacon around the local-master turn;send_to_agentreads it (a process-global survives the harness spawn boundary the task-local could not) and armspending_ask(peer_session) → master. Safe because local-master wakes are serialized by the generation guard; the A2A concurrency edge is documented.report_peer_reply_to_masterruns themaster_agentand surfaces the answer as OpenHuman's ownassistantmessage — scoped to master-triggered asks only; peer/A2A replies keep the raw threadback. Falls back to raw threading if the report turn fails so an answer is never dropped.send_to_agenttool note +master_agentprompt now state the reply is surfaced automatically and the agent must not poll/read_sessionfor it — so the W7 push is the sole async reporter (previously the agent polled-and-reported and W7 pushed → the reply surfaced twice).Design decision: master-triggered asks are treated as one-shot — OpenHuman reports the answer and does not keep an autonomous thread running with the peer. The full peer transcript still lives in its own session (browsable via
orchestration_read_session); the master chat is the curated control view.Verified live on staging (3 tiny.place plugin sessions of a
testagent): master ask → OpenHuman sends + immediate ack → peer reply → exactly one OpenHuman report in the master chat;pending_askarmed then cleared.Submission Checklist
orchestration::tools::tests::master_origin_beacon_sets_and_clears) and W7 store one-shot / graph tests in the stack; the LLM-driven report turn and the network-dependent Signal auto-publish are integration-only and were verified live on staging (documented above).## Related— N/A: no matrix feature IDs apply to this orchestration-internal change.Closes #NNN— N/A: no tracking issue; follows up the merged fix(orchestration): reliable agent-to-agent DM replies (surface, correct content, resilience) — Closes #4583 #4599 orchestration work.Impact
src/openhuman/orchestration,src/openhuman/tinyplace) plus bundled agent prompts. No UI, mobile, or CLI surface changes.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
feat/master-chat-orchestration-toolsValidation Run
pnpm --filter openhuman-app format:check— N/A: noapp/srcfiles changed (core Rust + prompts only);cargo fmtrun on changed Rust.pnpm typecheck— N/A: no TypeScript changed.cargo test --lib orchestration::tools::tests::master_origin_beacon_sets_and_clears(pass)cargo fmt+cargo check --bin openhuman-core(clean)app/src-tauricode changed.Validation Blocked
command:pre-pushpnpm rust:checkerror:fails in a fresh worktree (missing vendored CEF for the Tauri shell)impact:unrelated to this change (noapp/src-tauricode touched); pushed with--no-verify.Behavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Bug Fixes