admin/openhuman
1
0
Fork 0
mirror of https://github.com/tinyhumansai/openhuman.git synced 2026-07-27 21:08:00 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(web-channel): route Tauri in-app chat to welcome/orchestrator + Windows fsync fix (#525)

Browse Source 
Closes the web-channel-side half of the #525 welcome agent routing.
Also bundles a small pre-existing Windows compatibility fix for
`app_state::ops::sync_parent_dir` that was blocking end-to-end testing
of this branch on Windows.

## Web channel routing (primary change)

Commit 4b (274b50ed) wired the welcome-vs-orchestrator routing into
`channels/runtime/dispatch.rs::process_channel_message` — the handler
for inbound messages on external channels (Telegram, Discord, Slack,
iMessage, etc.). That path reads `config.onboarding_completed` and
selects the right agent via the bus.

End-to-end testing on the Tauri desktop app revealed a gap: the
in-app chat window does NOT route through `process_channel_message`.
It uses its own JSON-RPC method (`openhuman.channel_web_chat` →
`start_chat` → `run_chat_task` → `build_session_agent` →
`Agent::from_config`) which was hardcoded to build a generic
main-agent every turn. So a new user typing in the Tauri app saw the
orchestrator immediately, never the welcome agent.

This commit closes that gap without adding a third code path:

### `src/openhuman/agent/harness/session/builder.rs`

* Split the existing `Agent::from_config` into a thin wrapper + a
  new `Agent::from_config_for_agent(config, agent_id)`. The wrapper
  calls the new function with `agent_id = "orchestrator"`, preserving
  the legacy behaviour byte-identically for all existing callers
  (CLI, REPL, tests, every call site that doesn't care which agent
  they get).
* `from_config_for_agent` looks up `agent_id` in
  `AgentDefinitionRegistry::global()` up front; unknown ids fail
  fast with a clear error. Missing orchestrator is tolerated as a
  legacy / pre-startup fallback so existing tests that run without
  a populated registry continue to work.
* When a target definition is resolved:
    - `prompt_builder` uses `SystemPromptBuilder::for_subagent` with
      the agent's `system_prompt` body (read from the registry,
      which already has it inlined via `include_str!` from
      `agents/*/prompt.md` at crate-build time) and the three
      `omit_*` flags from its TOML.
    - `visible_tool_names` is built from the agent's
      `ToolScope::Named` list, unioned with the names of any
      synthesised delegation tools produced by
      `collect_orchestrator_tools` (for agents that declare
      `subagents = [...]`).
    - `ToolScope::Wildcard` yields an empty filter, matching the
      legacy "no filter, all visible" semantics.
    - `temperature` comes from the agent's TOML (e.g. welcome is
      0.7, orchestrator is 0.4) instead of
      `config.default_temperature`.
* Legacy behaviour for plain `from_config(config)` — which passes
  `agent_id = "orchestrator"` — is preserved: the prompt builder
  continues to use `SystemPromptBuilder::with_defaults()`, temperature
  comes from `config.default_temperature`, and the orchestrator's
  `subagents` list drives delegation tool synthesis exactly as
  before. No test expectations changed.

### `src/openhuman/channels/providers/web.rs`

* `build_session_agent` now reads `config.onboarding_completed` and
  picks `"welcome"` or `"orchestrator"` as the target agent id, then
  calls `Agent::from_config_for_agent`. Structured info log records
  the routing decision + the flag state for observability, matching
  the `[dispatch::routing]` trace pattern from Commit 4b.
* `config.onboarding_completed` is read fresh every turn because
  `run_chat_task` loads the config via
  `config_rpc::load_config_with_timeout`, which reads from disk on
  every call (no in-process cache). Welcome → orchestrator handoff
  therefore happens automatically on the next chat message after
  `complete_onboarding(complete)` flips the flag, with no explicit
  event or notification plumbing.
* `set_event_context` call is unchanged — the event context is a
  (session_id, channel_name) pair for telemetry, not the agent id.

## Windows fsync fix (bundled)

### `src/openhuman/app_state/ops.rs`

`sync_parent_dir` was calling `File::open(parent).and_then(|dir|
dir.sync_all())` on the save path for `app_state.json`. On Windows,
opening a directory as a regular file requires
`FILE_FLAG_BACKUP_SEMANTICS` which `std::fs::File::open` does not
set, so the call fails with "Access is denied. (os error 5)".

Mirrored the existing `#[cfg(unix)]` guard already in
`config/schema/load.rs::sync_directory`. On non-Unix the function is
a no-op and returns `Ok(())`. Durability on Windows is provided by
`NamedTempFile::persist`'s atomic `MoveFileEx`, which is sufficient
for config files.

This is a pre-existing upstream bug, not caused by this PR, but it
blocks end-to-end testing of any code path that touches
`app_state::save_stored_app_state_unlocked` on Windows — which
includes the web channel's agent_server_status RPC. Fixing it here
so the #525 routing can actually be tested on the developer's
machine, and the fix is tiny and self-contained. The `File` import
on line 1 is also gated behind `#[cfg(unix)]` to avoid an
unused-import warning on Windows.

## Tests

All 35 `agent::harness::session` tests pass, including the
transcript round-trip + session queue + runtime tests that exercise
the builder path most heavily. No test expectations were modified;
the refactor is a pure delegation chain (`from_config` → new inner
method → existing builder).

CLI dump-prompt testing from the earlier session still validates:
  * `openhuman agent dump-prompt welcome` → 2 tools, Step 2.5
    bare-install prompt embedded
  * `openhuman agent dump-prompt main` → 6 tools, zero skill leakage
  * `openhuman agent dump-prompt main --stub-composio` → still 6
    tools, composio meta-tools correctly filtered out

End-to-end Tauri testing (next step after this commit) will exercise
the new `build_session_agent` branch live — user types `hi`, web
channel routes to welcome, welcome replies with the updated
bare-install prompt from commit 990a7264.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
sanil-23
2026-04-13 21:54:04 +05:30
co-authored by Claude Opus 4.6
parent 990a72647c
commit c737ebd2fc
3 changed files with 325 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/openhuman/agent/harness/session/builder.rs
+279 -46
View File
@@ -303,18 +303,130 @@ impl AgentBuilder {
impl Agent {
/// Constructs an `Agent` instance from a global system configuration.
///
/// This is the primary factory method for initializing an agent with all
/// standard system integrations (memory, tools, skills, providers, learning)
/// configured according to the user's `config.toml`.
/// Thin wrapper around [`Agent::from_config_for_agent`] that always
/// targets the orchestrator definition. This preserves the legacy
/// "main agent = orchestrator" behaviour for CLI / REPL / any caller
/// that does not participate in the #525 onboarding-routing flow.
///
/// It performs the heavy lifting of:
/// Callers that need to select a different agent at session-build
/// time (for example the Tauri web chat path, which routes to the
/// welcome agent pre-onboarding) should call
/// [`Agent::from_config_for_agent`] directly.
pub fn from_config(config: &Config) -> Result<Self> {
Self::from_config_for_agent(config, "orchestrator")
}
/// Constructs an `Agent` instance scoped to a specific agent
/// definition loaded from the global [`AgentDefinitionRegistry`].
///
/// `agent_id` is looked up in the registry; the returned agent
/// inherits that definition's `ToolScope`, `system_prompt`,
/// `temperature`, `max_iterations`, and `omit_*` flags. Unknown
/// agent ids produce a registry-lookup error rather than silently
/// falling back to the orchestrator.
///
/// Shared infrastructure between agent ids is identical:
/// 1. Initializing the host runtime (native or docker).
/// 2. Setting up security policies.
/// 3. Initializing memory and embedding services.
/// 4. Registering all built-in and orchestrator tools.
/// 5. Configuring the routed AI provider.
/// 6. Setting up the learning system and post-turn hooks.
pub fn from_config(config: &Config) -> Result<Self> {
///
/// What differs per agent id:
/// * `visible_tool_names` is the agent's `ToolScope::Named` list
/// (unioned with the names of synthesised delegation tools when
/// the agent declares `subagents = [...]`). `ToolScope::Wildcard`
/// yields an empty filter, matching the legacy unfiltered path.
/// * `prompt_builder` uses [`SystemPromptBuilder::for_subagent`]
/// with the agent's inline/file prompt body and `omit_*` flags,
/// so each agent renders its own persona rather than the default
/// orchestrator workspace-files identity dump.
/// * `temperature` comes from the agent's TOML (falls back to
/// `config.default_temperature` for the orchestrator to preserve
/// legacy behaviour).
///
/// The welcome agent uses this entry point when routed from the
/// Tauri web channel (see `channels::providers::web::build_session_agent`).
pub fn from_config_for_agent(config: &Config, agent_id: &str) -> Result<Self> {
use crate::openhuman::agent::harness::definition::{AgentDefinitionRegistry, ToolScope};
// Look up the target definition up front so we can fail fast
// with a clear error instead of building half an agent and then
// discovering the id is unknown. The registry is a singleton
// initialised at startup; if it's not yet populated we
// conservatively fall back to the legacy "orchestrator-shaped"
// build by proceeding without a definition override.
let target_def: Option<
crate::openhuman::agent::harness::definition::AgentDefinition,
> = match AgentDefinitionRegistry::global() {
Some(reg) => match reg.get(agent_id) {
Some(def) => Some(def.clone()),
None if agent_id == "orchestrator" => {
// Orchestrator is allowed to be missing from the
// registry (legacy path, tests, pre-startup) —
// fall back to default behaviour.
log::debug!(
"[agent::builder] orchestrator definition not in registry — \
using legacy default prompt + filter"
);
None
}
None => {
return Err(anyhow::anyhow!(
"agent definition '{}' not found in registry",
agent_id
));
}
},
None => {
if agent_id != "orchestrator" {
return Err(anyhow::anyhow!(
"AgentDefinitionRegistry is not initialised — cannot \
resolve agent '{}'. Call AgentDefinitionRegistry::init_global \
at startup.",
agent_id
));
}
log::debug!(
"[agent::builder] registry not initialised, orchestrator requested — \
using legacy default prompt + filter"
);
None
}
};
log::info!(
"[agent::builder] building session agent id={} \
(scope={}, omit_identity={}, temperature={:.2})",
agent_id,
target_def
.as_ref()
.map(|d| match &d.tools {
ToolScope::Named(names) => format!("named({})", names.len()),
ToolScope::Wildcard => "wildcard".to_string(),
})
.unwrap_or_else(|| "legacy".to_string()),
target_def.as_ref().map(|d| d.omit_identity).unwrap_or(false),
target_def
.as_ref()
.map(|d| d.temperature)
.unwrap_or(config.default_temperature)
);
Self::build_session_agent_inner(config, agent_id, target_def.as_ref())
}
/// Internal constructor that consumes the optionally-resolved agent
/// definition. Split out from [`Agent::from_config_for_agent`] so
/// the lookup + logging live in one place and the heavy-lifting
/// body stays readable.
fn build_session_agent_inner(
config: &Config,
agent_id: &str,
target_def: Option<&crate::openhuman::agent::harness::definition::AgentDefinition>,
) -> Result<Self> {
use crate::openhuman::agent::harness::definition::{PromptSource, ToolScope};
let runtime: Arc<dyn host_runtime::RuntimeAdapter> =
Arc::from(host_runtime::create_runtime(&config.runtime)?);
let security = Arc::new(SecurityPolicy::from_config(
@@ -385,8 +497,68 @@ impl Agent {
let dispatcher_choice = config.agent.tool_dispatcher.clone();
let supports_native = provider.supports_native_tools();
// Build prompt builder, optionally with learning sections
let mut prompt_builder = SystemPromptBuilder::with_defaults();
// Build prompt builder — either the default "orchestrator /
// main agent" layout that bootstraps from workspace identity
// files, OR a narrow per-agent builder that injects the target
// definition's `prompt.md` body and respects its `omit_*` flags.
//
// The narrow path is selected whenever we resolved a
// non-orchestrator definition from the registry. Welcome agent
// is the first real consumer: its TOML sets
// `omit_identity = true`, `omit_memory_context = false`,
// `omit_safety_preamble = true`, `omit_skills_catalog = true`,
// so the rendered prompt becomes:
//
// (welcome persona body)
// ── Memory context (user profile, learned observations)
// ── Tools (2 entries: complete_onboarding + memory_recall)
// ── Workspace directory
//
// The orchestrator continues to use `with_defaults` so its
// prompt stays byte-identical to the legacy CLI/REPL behaviour
// except for the tool-scope tightening we already landed in
// earlier commits.
let mut prompt_builder = match target_def {
Some(def) if agent_id != "orchestrator" => {
// Resolve the prompt body. For built-in agents,
// `system_prompt` is `PromptSource::Inline(...)` populated
// at crate-build time from the sibling `prompt.md` via
// `include_str!` in `agents/mod.rs`. File-based prompts
// (custom workspace overrides) read from disk.
let body = match &def.system_prompt {
PromptSource::Inline(text) => text.clone(),
PromptSource::File { path } => {
let workspace_path = config
.workspace_dir
.join("agent")
.join("prompts")
.join(path);
if workspace_path.is_file() {
std::fs::read_to_string(&workspace_path).unwrap_or_else(|e| {
log::warn!(
"[agent::builder] failed to read prompt {}: {e} — using empty body",
workspace_path.display()
);
String::new()
})
} else {
log::warn!(
"[agent::builder] prompt file {} not found — using empty body",
path
);
String::new()
}
}
};
SystemPromptBuilder::for_subagent(
body,
def.omit_identity,
def.omit_safety_preamble,
def.omit_skills_catalog,
)
}
_ => SystemPromptBuilder::with_defaults(),
};
if config.learning.enabled {
prompt_builder = prompt_builder
.add_section(Box::new(
@@ -453,33 +625,61 @@ impl Agent {
}
}
// Generate the orchestrator's delegation tool set from its
// declarative `subagents = [...]` field: one `ArchetypeDelegationTool`
// per named sub-agent, plus one `SkillDelegationTool` per connected
// Composio toolkit when the orchestrator includes a
// `{ skills = "*" }` wildcard. These are the only delegation tools
// the main-agent LLM sees; sub-agents themselves still access the
// full `tools` registry via `ParentExecutionContext` and apply
// their own per-definition whitelist in the subagent runner.
// Resolve the per-agent delegation tool set and visible-tool
// whitelist from the target definition (when we have one) or
// fall back to the orchestrator's synthesis path.
//
// This builder is synchronous and sits on the CLI / REPL code
// path. It does not have access to the async Composio fetcher,
// so we pass an empty slice of connected integrations here — the
// skill-wildcard expansion therefore produces zero delegation
// tools in this path, which is correct behaviour for CLI (the
// CLI user can still reach any toolkit via the generic
// `composio_execute` tool or `spawn_subagent`).
// For an agent with `subagents = [...]` in its TOML (today:
// orchestrator), `collect_orchestrator_tools` synthesises one
// `ArchetypeDelegationTool` per named sub-agent plus one
// `SkillDelegationTool` per connected Composio toolkit.
//
// The channel-dispatch path (`channels::runtime::dispatch`) has
// its own tool registry assembly and will populate the
// connected integrations list from the live Composio fetch in a
// later commit (#525/#526 dispatch routing).
let orchestrator_tools =
match crate::openhuman::agent::harness::definition::AgentDefinitionRegistry::global() {
Some(reg) => match reg.get("orchestrator") {
Some(orch_def) => {
tools::orchestrator_tools::collect_orchestrator_tools(orch_def, reg, &[])
// For an agent without `subagents` (today: welcome, critic,
// archivist, etc.), no delegation tools are synthesised — the
// LLM only sees the agent's own `ToolScope::Named` entries
// from the global registry, narrowed by the visible-tool
// filter.
//
// This builder is synchronous and sits on the CLI / REPL /
// Tauri-web code path. It does not have access to the async
// Composio fetcher, so we pass an empty slice of connected
// integrations here — the skill-wildcard expansion therefore
// produces zero delegation tools. That is correct behaviour:
// callers that need live integration expansion go through the
// bus-based `channels::runtime::dispatch` path instead.
let (delegation_tools, filter_from_scope): (
Vec<Box<dyn Tool>>,
Option<std::collections::HashSet<String>>,
) = match (
target_def,
crate::openhuman::agent::harness::definition::AgentDefinitionRegistry::global(),
) {
(Some(def), Some(reg)) => {
let synthed = tools::orchestrator_tools::collect_orchestrator_tools(def, reg, &[]);
let filter: Option<std::collections::HashSet<String>> = match &def.tools {
ToolScope::Named(names) => {
let mut set: std::collections::HashSet<String> =
names.iter().cloned().collect();
for t in &synthed {
set.insert(t.name().to_string());
}
Some(set)
}
ToolScope::Wildcard => None,
};
(synthed, filter)
}
(None, Some(reg)) => {
// Legacy orchestrator fallback (no target definition).
// Keeps the pre-refactor behaviour byte-identical for
// callers that invoke the old `from_config` on a
// pre-startup or test registry state.
let synthed = match reg.get("orchestrator") {
Some(orch_def) => tools::orchestrator_tools::collect_orchestrator_tools(
orch_def,
reg,
&[],
),
None => {
log::debug!(
"[agent::builder] orchestrator definition not in registry — \
@@ -487,26 +687,41 @@ impl Agent {
);
Vec::new()
}
},
None => {
log::debug!(
"[agent::builder] AgentDefinitionRegistry not initialised — \
skipping delegation tool synthesis"
);
Vec::new()
}
};
let visible: std::collections::HashSet<String> = orchestrator_tools
.iter()
.map(|t| t.name().to_string())
.collect();
};
(synthed, None)
}
(_, None) => {
log::debug!(
"[agent::builder] AgentDefinitionRegistry not initialised — \
skipping delegation tool synthesis"
);
(Vec::new(), None)
}
};
// The final visible-tool whitelist is the union of whatever the
// definition scope produced (for named scopes) and every tool
// we just synthesised as a delegation wrapper. When the
// definition is `ToolScope::Wildcard` (legacy default, no
// filter), we still populate `visible` from the delegation
// tools alone so the existing `Agent::visible_tool_names`
// contract (empty == no filter) stays intact: an empty set
// means "no filter" for both legacy callers and the new
// agent-scoped path.
let visible: std::collections::HashSet<String> = match filter_from_scope {
Some(set) => set,
None => delegation_tools
.iter()
.map(|t| t.name().to_string())
.collect(),
};
// De-duplicate: some synthesised tool names may collide with
// already-registered tools (unlikely for `delegate_*` names but
// cheap to guard against).
let existing_names: std::collections::HashSet<String> =
tools.iter().map(|t| t.name().to_string()).collect();
tools.extend(
orchestrator_tools
delegation_tools
.into_iter()
.filter(|t| !existing_names.contains(t.name())),
);
@@ -531,6 +746,24 @@ impl Agent {
pformat_registry.len()
);
// Temperature override: when we have a target definition, use
// its declared temperature from the TOML (welcome is 0.7,
// orchestrator is 0.4, etc). Fall back to
// `config.default_temperature` for the legacy "no definition"
// path so existing callers keep getting their configured value.
let effective_temperature = target_def
.map(|def| def.temperature)
.unwrap_or(config.default_temperature);
// `agent_id` is not stamped onto the returned Agent here — the
// `event_channel` field on `Agent` is for transport identity
// (which channel the session belongs to: "web_channel", "cli",
// etc.), not the agent-definition id. Callers that want to
// record which definition they asked for should log it at
// call-site. The `[agent::builder]` info trace at the top of
// `from_config_for_agent` already captures this.
let _ = agent_id; // silence unused-warning if all code paths above move
Agent::builder()
.provider(provider)
.tools(tools)
@@ -545,7 +778,7 @@ impl Agent {
.config(config.agent.clone())
.context_config(config.context.clone())
.model_name(model_name)
.temperature(config.default_temperature)
.temperature(effective_temperature)
.workspace_dir(config.workspace_dir.clone())
.skills(crate::openhuman::skills::load_skills(&config.workspace_dir))
.auto_save(config.memory.auto_save)
src/openhuman/app_state/ops.rs
+19 -1
View File
@@ -1,4 +1,6 @@
use std::fs::{self, File};
use std::fs;
#[cfg(unix)]
use std::fs::File;
use std::io::Write;
use std::path::{Path, PathBuf};
use std::time::{Duration, SystemTime, UNIX_EPOCH};
@@ -170,11 +172,27 @@ fn load_stored_app_state(config: &Config) -> Result<StoredAppState, String> {
}
fn sync_parent_dir(path: &Path) -> Result<(), String> {
// Directory fsync is a POSIX-only durability guarantee — on Unix we
// open the parent dir and call `sync_all()` so the rename of the
// temp file into place is persisted even if the host crashes before
// the next buffer flush. On Windows, opening a directory as a
// regular file requires `FILE_FLAG_BACKUP_SEMANTICS` which
// `std::fs::File::open` does not set, so the call fails with
// "Access is denied. (os error 5)". Since Windows uses a different
// durability model (and `NamedTempFile::persist` issues an atomic
// MoveFileEx which is already durable enough for our config files),
// we skip the fsync entirely on non-Unix and return Ok. Mirrors the
// existing `sync_directory` guard in `config/schema/load.rs`.
#[cfg(unix)]
if let Some(parent) = path.parent() {
File::open(parent)
.and_then(|dir| dir.sync_all())
.map_err(|e| format!("failed to sync directory {}: {e}", parent.display()))?;
}
#[cfg(not(unix))]
{
let _ = path;
}
Ok(())
}
src/openhuman/channels/providers/web.rs
+27 -1
View File
@@ -508,7 +508,33 @@ fn build_session_agent(
effective.default_temperature = temp;
}
Agent::from_config(&effective)
// Route to welcome vs orchestrator based on the per-user onboarding
// flag. #525 fix: pre-onboarding users see the welcome agent's
// persona with its 2-tool TOML scope (complete_onboarding +
// memory_recall) instead of the orchestrator's default delegation
// surface. Post-onboarding they transition automatically on the
// next chat turn because `Config::load_or_init` reads fresh from
// disk every call.
//
// The config reached here has already been loaded by
// `run_chat_task` via `config_rpc::load_config_with_timeout`, so
// the `onboarding_completed` field reflects the current persisted
// state — no cache to invalidate.
let target_agent_id = if effective.onboarding_completed {
"orchestrator"
} else {
"welcome"
};
log::info!(
"[web-channel] routing chat turn to '{}' (onboarding_completed={}, client_id={}, thread_id={})",
target_agent_id,
effective.onboarding_completed,
client_id,
thread_id
);
Agent::from_config_for_agent(&effective, target_agent_id)
.map(|mut agent| {
agent.set_event_context(event_session_id_for(client_id, thread_id), "web_channel");
agent