Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/ci-lite.yml
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,7 @@ jobs:
openhuman/agent/harness/session/tests.rs
openhuman/agent/harness/subagent_runner/tool_prep.rs
openhuman/agent_registry/agents/loader.rs
openhuman/composio/action_tool.rs
openhuman/inference/local/mod.rs
openhuman/tool_registry/ops_tests.rs
openhuman/tool_registry/schemas.rs
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/pr-quality.yml
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ jobs:
--exclude '^https://www\.star-history\.com/#tinyhumansai/openhuman&type=date&legend=top-left$'
--exclude '^https://github\.com/tinyhumansai/openhuman/stargazers'
--exclude '^https://api\.star-history\.com/'
--exclude '^https://img\.shields\.io/'
--exclude '^https://x\.com/karpathy/status/2039805659525644595$'
'docs/**/*.md'
'src/**/README.md'
Expand Down
88 changes: 78 additions & 10 deletions src/openhuman/agent/harness/session/tool_progress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,40 @@ impl TurnProgress {
/// because the orchestrator is `await`ing that sub-agent's tool call and never
/// makes its next LLM request (the subagent-stall flake). So drop the event on
/// `Full` (a missed UI tick, not a correctness issue) and `trace` on `Closed`
/// (no listener). Streaming *text deltas* keep their own blocking backpressure
/// in their forwarder tasks, so visible message text is unaffected.
/// (no listener). Streaming deltas use the same non-blocking policy in their
/// forwarder tasks; the completed provider response is still returned by the
/// turn engine, so dropping saturated UI delta frames must not wedge the turn.
fn emit(sink: &tokio::sync::mpsc::Sender<AgentProgress>, event: AgentProgress) {
let _ = emit_nonblocking(sink, event, "lifecycle event");
}

fn emit_stream_delta(
sink: &tokio::sync::mpsc::Sender<AgentProgress>,
event: AgentProgress,
) -> bool {
emit_nonblocking(sink, event, "stream delta")
}

/// Shared non-blocking sink push. `Full` drops the frame (a missed UI tick, not
/// a correctness issue) and returns `true` so the forwarder keeps draining;
/// `Closed` returns `false` so the caller can stop. Never `.await`s, so a slow
/// downstream progress bridge cannot backpressure the provider stream and wedge
/// the turn in RESPONSE.
fn emit_nonblocking(
sink: &tokio::sync::mpsc::Sender<AgentProgress>,
event: AgentProgress,
label: &str,
) -> bool {
use tokio::sync::mpsc::error::TrySendError;
match sink.try_send(event) {
Ok(()) => {}
Ok(()) => true,
Err(TrySendError::Full(_)) => {
log::trace!("[agent_loop] progress channel full — dropping lifecycle event");
log::trace!("[agent_loop] progress channel full — dropping {label}");
true
}
Err(TrySendError::Closed(_)) => {
log::trace!("[agent_loop] progress sink closed — dropping lifecycle event");
log::trace!("[agent_loop] progress sink closed — dropping {label}");
false
}
}
}
Expand Down Expand Up @@ -208,10 +231,12 @@ impl ProgressReporter for TurnProgress {
/// Returns `(None, None)` when there is no progress sink — the caller then
/// passes `stream: None` and the provider uses its non-streaming HTTP path.
///
/// Backpressure discipline: the forwarder `.await`s each `send`, so streamed
/// deltas arrive in order and are never silently dropped when the downstream
/// bridge is slow. It exits cleanly once the sender is dropped (after the chat
/// call) or the downstream closes.
/// Backpressure discipline: the forwarder drains provider deltas without
/// awaiting the downstream progress bridge. A full UI/progress channel drops
/// transient delta frames rather than backpressuring the provider stream and
/// wedging the turn in RESPONSE; the final provider response is still returned
/// through the normal chat result. It exits cleanly once the sender is dropped
/// (after the chat call) or the downstream closes.
pub(crate) fn spawn_delta_forwarder(
on_progress: Option<tokio::sync::mpsc::Sender<AgentProgress>>,
iteration: u32,
Expand Down Expand Up @@ -247,10 +272,53 @@ pub(crate) fn spawn_delta_forwarder(
}
}
};
if progress_sink.send(mapped).await.is_err() {
if !emit_stream_delta(&progress_sink, mapped) {
break;
}
}
});
(Some(tx), Some(forwarder))
}

#[cfg(test)]
mod tests {
use super::*;
use tokio::sync::mpsc;
use tokio::time::{timeout, Duration};

/// Regression (#4269): a full downstream progress channel must not park the
/// delta forwarder. It drains provider deltas with a non-blocking push, so a
/// saturated UI/progress sink drops frames instead of backpressuring the
/// provider stream and wedging the turn in RESPONSE. The forwarder must
/// finish promptly once the delta sender is dropped, even though the progress
/// sink stays full the whole time.
#[tokio::test]
async fn delta_forwarder_does_not_wait_on_full_progress_sink() {
let (progress_tx, mut progress_rx) = mpsc::channel::<AgentProgress>(1);
// Saturate the progress sink so every forwarded delta hits `Full`.
progress_tx.try_send(AgentProgress::TurnStarted).unwrap();

let (delta_tx, forwarder) = spawn_delta_forwarder(Some(progress_tx), 1);
let delta_tx = delta_tx.expect("stream sink should be created");
let forwarder = forwarder.expect("forwarder should be created");

delta_tx
.send(ProviderDelta::TextDelta {
delta: "hello".to_string(),
})
.await
.unwrap();
drop(delta_tx);

timeout(Duration::from_secs(1), forwarder)
.await
.expect("delta forwarder must not block on a full progress sink")
.unwrap();
// The pre-seeded event is still there; the dropped delta never enqueued.
assert!(matches!(
progress_rx.try_recv(),
Ok(AgentProgress::TurnStarted)
));
assert!(progress_rx.try_recv().is_err());
}
}
18 changes: 10 additions & 8 deletions src/openhuman/learning/startup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ mod tests {
use crate::core::event_bus::{init_global, publish_global, DomainEvent, DEFAULT_CAPACITY};
use crate::openhuman::learning::candidate::{self, EvidenceRef};
use crate::openhuman::learning::extract::signature::parse_signature;
use crate::openhuman::learning::extract::signature::EmailSignatureSubscriber;
use crate::openhuman::memory_store::MemoryClient;
use std::sync::atomic::{AtomicU64, Ordering};
use std::sync::Arc;
Expand Down Expand Up @@ -271,13 +272,6 @@ mod tests {

#[tokio::test]
async fn learning_subscriber_fires_with_no_channel_configured() {
init_global(DEFAULT_CAPACITY);
let (tmp, _client) = test_client();
// Make the memory client ready so the full Platform wiring runs — no
// channel runtime is ever constructed in this test.
let _ = crate::openhuman::memory::global::init(tmp.path().join("workspace"));
register_learning_subscribers(tmp.path().to_path_buf());

let source_id = unique_source_id("e2e");
let body = signature_body();
let expected = parse_signature(&body, &source_id, &source_id).len();
Expand All @@ -286,7 +280,15 @@ mod tests {
"signature body must yield at least one identity candidate"
);

publish_email_doc(&source_id, &body);
let event = DomainEvent::DocumentCanonicalized {
source_id: source_id.clone(),
source_kind: "email".to_string(),
chunks_written: 1,
chunk_ids: vec![format!("{source_id}-c1")],
canonicalized_at: 0.0,
body_preview: Some(body),
};
crate::core::event_bus::EventHandler::handle(&EmailSignatureSubscriber, &event).await;
let got = wait_for_candidates(&source_id, expected).await;
assert_eq!(
got, expected,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,10 +307,21 @@ async fn round15_composio_agent_tools_backend_cache_and_trigger_history_edges()
);
assert_eq!(action_tool.name(), "GMAIL_FETCH_EMAILS");
assert_eq!(action_tool.category().to_string(), "skill");
let contract_prompt = action_tool
.execute(json!({}))
.await
.expect("per-action tool contract prompt");
assert!(contract_prompt.is_error);
assert!(contract_prompt
.text()
.contains("Before running `GMAIL_FETCH_EMAILS`"));
assert!(contract_prompt.text().contains("Required arguments: query"));

let action_result = action_tool
.execute(json!({ "query": "from:me" }))
.await
.expect("per-action tool execute");
.expect("per-action tool retry execute");
assert!(!action_result.is_error);
assert_eq!(action_result.text(), "Fetched 1 inbox message");

let reserved = composio_authorize(&config, "gmail", Some(json!({ "toolkit": "github" })))
Expand Down
Loading