fix(agent): thread agent_id into build_session_agent_inner (#584)

build_session_agent_inner took an agent_id: &str parameter but
never threaded it into the Agent::builder() chain — a `let _ =
agent_id;` silenced the unused-variable warning. AgentBuilder::build()
fell back to the legacy "main" default at builder.rs:310-312, so
every session built via Agent::from_config_for_agent carried
agent_definition_name="main" regardless of the id the caller asked
for.

Only two ids reach this path in production: "orchestrator" (legacy
Agent::from_config wrapper, cron, local_ai, escalation) and
"welcome" (channels/providers/web.rs routing after #525/#526, and
welcome_proactive). Orchestrator is benign — "main" was already
an alias for orchestrator everywhere downstream (see debug_dump.rs:249).
Welcome is the visible bug — agent_definition_name feeds three
surfaces that are all silently wrong pre-fix:

  1. Transcript filename — welcome sessions land as main_*.md
     instead of welcome_*.md.
  2. Transcript metadata — the <!-- session_transcript --> block
     stamps `agent: main` instead of `agent: welcome`.
  3. Transcript resume cross-contamination —
     try_load_session_transcript calls
     find_latest_transcript(workspace_dir, "main") which scans all
     dated session dirs for the latest main_*.md. It finds the
     last orchestrator session and resumes from it, loading its
     system prompt + user messages + assistant tool-call history
     into the welcome agent's `history` as a resume prefix before
     run_single runs. The written transcript mixes yesterday's
     orchestrator email thread with today's welcome message under
     the same main_0.md filename. Reproduced live 2026-04-16: a
     welcome_proactive session loaded 5 messages from yesterday's
     15042026/main_0.md and wrote 6 messages back that included a
     spawn_subagent(skills_agent, toolkit=gmail) tool-call the
     welcome agent never made.

Prompt rendering is NOT affected. context/prompt.rs only reads
ctx.agent_id for the is_skill_executor branch at prompt.rs:655, so
welcome/main/orchestrator all fall through the same delegator path.
The welcome persona prompt is rendered by the stamped
SystemPromptBuilder::for_subagent(target_def.system_prompt) built
at session-build time, independent of agent_definition_name. The
pre-fix main_0.md on disk contains `# Welcome Agent\n\nYou are the
Welcome agent...` as its system prompt body — correct content,
wrong filename and metadata.

skills_agent and other typed sub-agents are unaffected — they go
through subagent_runner, which constructs its prompt directly and
writes transcripts under the explicit id it receives. The
pre-existing sessions/15042026/skills_agent_0.md on disk with
`agent: skills_agent` confirms this code path has always been
correct.

Changes:

  * src/openhuman/agent/harness/session/builder.rs:
    - Add .agent_definition_name(agent_id.to_string()) to the
      builder chain in build_session_agent_inner.
    - Delete the `let _ = agent_id;` suppression.
    - Replace the misleading 8-line comment block at the call site
      (which claimed event_channel was for transport identity and
      therefore stamping agent_id wasn't needed — conflating two
      unrelated fields) with an accurate description of the three
      load-bearing surfaces.
    - Expand the docstring on AgentBuilder::agent_definition_name
      to document the surfaces and the latent prompt-section
      foot-gun the fix closes for future code.
    - New log::debug! call at the stamping site for grep-friendly
      runtime traces ([agent::builder] stamping
      agent_definition_name=<id> onto session agent).

  * src/openhuman/agent/harness/session/runtime.rs:
    - Add pub fn agent_definition_name(&self) -> &str accessor
      on Agent so tests and runtime callers can introspect the
      stamped id without reaching into the pub(super) field.

  * src/openhuman/agent/harness/session/tests.rs:
    - Add build_minimal_agent_with_definition_name helper.
    - agent_builder_threads_agent_definition_name_when_set —
      parameterised over welcome/skills_agent/orchestrator/
      trigger_triage, asserts the setter threads the id through.
    - agent_builder_falls_back_to_main_when_definition_name_unset
      — pins the legacy fallback contract direct builder users rely
      on.

Tests: 2 passed; 0 failed; 3477 filtered out; finished in 0.21s.
cargo check --lib clean; cargo fmt clean.

Verified live against G:/projects/vezures/.openhuman on 2026-04-16:
  - Pre-fix welcome_proactive run wrote sessions/16042026/main_0.md
    with `agent: main` in the metadata header.
  - Post-fix welcome_proactive run wrote sessions/16042026/welcome_0.md
    with `agent: welcome` in the metadata header.
  - Post-fix web-chat dispatch to orchestrator wrote
    sessions/16042026/orchestrator_0.md (moved off the historical
    main_*.md alias — behaviorally unchanged for orchestrator).
  - The new [agent::builder] stamping debug line fires on both
    welcome and orchestrator paths.

Does not touch: subagent_runner, spawn_subagent / dispatch_subagent,
any agent/agents/*/agent.toml, any context::prompt code, or the
AgentBuilder::build() fallback itself.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

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

@@ -186,9 +186,43 @@ impl AgentBuilder {
         self
     }
 
-    /// Sets the human-readable agent definition name used as the
-    /// `{agent}` prefix in session transcript filenames
-    /// (`sessions/DDMMYYYY/{agent}_{index}.md`).
+    /// Sets the agent definition id this session is running
+    /// (`welcome`, `orchestrator`, `skills_agent`, …).
+    ///
+    /// This value is stamped onto the built [`Agent`] and surfaces in
+    /// the following places:
+    ///
+    /// * **Transcript filename on disk** — `transcript::write_transcript`
+    ///   and `transcript::find_latest_transcript` use it as the
+    ///   `{agent}` prefix in `sessions/DDMMYYYY/{agent}_{index}.md`.
+    ///   Both the write path and the resume-lookup path read the same
+    ///   field on `self`, so a session is always self-consistent; the
+    ///   user-visible signal is which filename the transcript lands
+    ///   under. Leaving it at the legacy `"main"` fallback silently
+    ///   misfiles every non-orchestrator session under `main_*.md`.
+    /// * **Transcript metadata header** — `transcript::write_transcript`
+    ///   stamps it into the `<!-- session_transcript\nagent: {name}\n… -->`
+    ///   block at the top of every `.md` file. This is the ground-truth
+    ///   signal for "which agent definition ran this session" when
+    ///   inspecting transcripts after the fact.
+    /// * **[`PromptContext::agent_id`]** at prompt-build time (see
+    ///   `turn.rs`). Today only one prompt section reads this field —
+    ///   the `Connected Integrations` branch in `context/prompt.rs`
+    ///   that special-cases `skills_agent` vs every other agent — so
+    ///   the current user-visible impact of a wrong id is limited to
+    ///   the two bullets above. The stamped `prompt_builder` injected
+    ///   by [`Agent::from_config_for_agent`] is what actually drives
+    ///   prompt flavour per archetype, independent of this field. That
+    ///   said, any future prompt section that branches on a
+    ///   non-`skills_agent` id (e.g. welcome-specific banner, planner-
+    ///   specific rubric) would silently never fire if the field were
+    ///   left at `"main"`, so keeping it correctly stamped closes a
+    ///   latent foot-gun for code that hasn't been written yet.
+    ///
+    /// Callers building via [`Agent::from_config_for_agent`] get this
+    /// wired automatically inside `build_session_agent_inner`; direct
+    /// builder users (tests, CLI) must set it explicitly if they care
+    /// about any of the surfaces above.
     pub fn agent_definition_name(mut self, name: impl Into<String>) -> Self {
         self.agent_definition_name = Some(name.into());
         self
@@ -789,14 +823,32 @@ impl Agent {
         let effective_omit_profile = target_def.map(|def| def.omit_profile).unwrap_or(true);
         let effective_omit_memory_md = target_def.map(|def| def.omit_memory_md).unwrap_or(true);
 
-        // `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
+        // Stamp the resolved agent definition id onto the Agent via the
+        // builder. Without this call, `agent_definition_name` falls
+        // back to the legacy `"main"` default (see `AgentBuilder::build`)
+        // for every non-orchestrator caller. In the current codebase
+        // that is benign for the orchestrator (which is already aliased
+        // as `"main"` everywhere downstream) but causes two concrete
+        // bugs for the welcome agent, which is the only other id that
+        // reaches this function in practice:
+        //
+        //   1. Its session transcripts are misfiled on disk under
+        //      `sessions/DDMMYYYY/main_*.md` instead of `welcome_*.md`.
+        //   2. The `agent:` line inside each transcript's metadata
+        //      header stamps `agent: main` instead of `agent: welcome`.
+        //
+        // Skills_agent and every other typed sub-agent are unaffected
+        // because they never build via `from_config_for_agent` — they
+        // are spawned through `subagent_runner` which constructs its
+        // prompt and history directly.
+        //
+        // See the docstring on `AgentBuilder::agent_definition_name`
+        // for the full list of surfaces and the latent prompt-section
+        // foot-gun this call also closes.
+        log::debug!(
+            "[agent::builder] stamping agent_definition_name={} onto session agent",
+            agent_id
+        );
 
         Agent::builder()
             .provider(provider)
@@ -818,6 +870,7 @@ impl Agent {
             .auto_save(config.memory.auto_save)
             .post_turn_hooks(post_turn_hooks)
             .learning_enabled(config.learning.enabled)
+            .agent_definition_name(agent_id.to_string())
             .omit_profile(effective_omit_profile)
             .omit_memory_md(effective_omit_memory_md)
             .build()

src/openhuman/agent/harness/session/runtime.rs

@@ -33,6 +33,20 @@ impl Agent {
         &self.event_channel
     }
 
+    /// The agent definition id this session is running
+    /// (`"welcome"`, `"orchestrator"`, `"skills_agent"`, …).
+    ///
+    /// Exposed so callers that build sessions via
+    /// [`Agent::from_config_for_agent`] can stamp the resolved id onto
+    /// correlation logs and progress events without reaching for the
+    /// source `Config`. See [`AgentBuilder::agent_definition_name`]
+    /// for the full list of downstream surfaces (transcript filename,
+    /// transcript metadata header, and `PromptContext::agent_id`) that
+    /// read this field.
+    pub fn agent_definition_name(&self) -> &str {
+        &self.agent_definition_name
+    }
+
     /// Returns a new `AgentBuilder`.
     pub fn builder() -> AgentBuilder {
         AgentBuilder::new()

src/openhuman/agent/harness/session/tests.rs

@@ -138,6 +138,108 @@ fn _assert_builder_is_exported() -> AgentBuilder {
     Agent::builder()
 }
 
+/// Minimal in-memory `Agent` build that every agent_definition_name
+/// regression test reuses. Spins up a scratch workspace, a `none`
+/// memory backend, a one-response `MockProvider`, and a single
+/// `MockTool`, then feeds those into [`Agent::builder`]. Returns the
+/// built `Agent` so individual tests can assert against the
+/// [`Agent::agent_definition_name`] accessor.
+fn build_minimal_agent_with_definition_name(definition_name: Option<&str>) -> Agent {
+    let workspace = tempfile::TempDir::new().expect("temp workspace");
+    let workspace_path = workspace.path().to_path_buf();
+
+    let provider = Box::new(MockProvider {
+        responses: Mutex::new(vec![]),
+    });
+
+    let memory_cfg = crate::openhuman::config::MemoryConfig {
+        backend: "none".into(),
+        ..crate::openhuman::config::MemoryConfig::default()
+    };
+    let mem: Arc<dyn Memory> = Arc::from(
+        crate::openhuman::memory::create_memory(&memory_cfg, &workspace_path, None).unwrap(),
+    );
+
+    let mut builder = Agent::builder()
+        .provider(provider)
+        .tools(vec![Box::new(MockTool)])
+        .memory(mem)
+        .tool_dispatcher(Box::new(NativeToolDispatcher))
+        .workspace_dir(workspace_path);
+
+    if let Some(name) = definition_name {
+        builder = builder.agent_definition_name(name);
+    }
+
+    builder.build().expect("minimal agent build should succeed")
+}
+
+/// Regression test for the `build_session_agent_inner` agent-id
+/// threading bug.
+///
+/// Prior to the fix, `build_session_agent_inner` took an `agent_id:
+/// &str` parameter but never threaded it into the `Agent::builder()`
+/// chain. The builder's `.build()` then fell back to the legacy
+/// `"main"` default, and every session built via
+/// `Agent::from_config_for_agent` carried `agent_definition_name =
+/// "main"` at runtime regardless of which id the caller asked for.
+///
+/// In the current codebase only two ids actually reach
+/// `from_config_for_agent` in production: `"orchestrator"` (via the
+/// `Agent::from_config` legacy wrapper and the post-onboarding web
+/// dispatch path) and `"welcome"` (via `welcome_proactive` and the
+/// pre-onboarding web dispatch path). The orchestrator case is
+/// benign — `"main"` is already an alias for orchestrator everywhere
+/// downstream, so the behavior is a no-op. The welcome case is the
+/// one the user sees: welcome sessions were being misfiled on disk
+/// as `sessions/DDMMYYYY/main_*.md` instead of `welcome_*.md`, and
+/// the `agent:` line inside each transcript's `<!-- session_transcript
+/// -->` metadata header stamped `agent: main` instead of
+/// `agent: welcome`. Skills_agent and the other typed sub-agents are
+/// unaffected because they're spawned through `subagent_runner` and
+/// never touch the `from_config_for_agent` / builder fallback path.
+///
+/// This test pins the builder contract the fix relies on: calling
+/// `.agent_definition_name(id)` on the builder chain produces an
+/// `Agent` whose [`Agent::agent_definition_name`] accessor returns
+/// that id verbatim. `"welcome"` and `"orchestrator"` exercise the
+/// two ids that reach `from_config_for_agent` today; `"skills_agent"`
+/// and `"trigger_triage"` are defensive coverage so that if a
+/// future commit adds a new top-level caller for one of those ids
+/// the builder contract is already pinned.
+#[test]
+fn agent_builder_threads_agent_definition_name_when_set() {
+    for expected in ["welcome", "skills_agent", "orchestrator", "trigger_triage"] {
+        let agent = build_minimal_agent_with_definition_name(Some(expected));
+        assert_eq!(
+            agent.agent_definition_name(),
+            expected,
+            "agent.agent_definition_name() should return the value passed to the builder"
+        );
+    }
+}
+
+/// Complementary to [`agent_builder_threads_agent_definition_name_when_set`]:
+/// when a caller builds an `Agent` without ever calling
+/// [`AgentBuilder::agent_definition_name`], the legacy `"main"`
+/// fallback still applies. This pins the fallback contract that
+/// direct builder users (tests, CLI harnesses) rely on, and
+/// documents the exact misbehaviour the threading fix prevents —
+/// `build_session_agent_inner` used to hit this fallback even when
+/// a caller asked for `welcome`, because the `.agent_definition_name`
+/// setter was missing from the builder chain. The result was that
+/// welcome sessions landed on disk as `main_*.md` with `agent: main`
+/// stamped into their transcript metadata header.
+#[test]
+fn agent_builder_falls_back_to_main_when_definition_name_unset() {
+    let agent = build_minimal_agent_with_definition_name(None);
+    assert_eq!(
+        agent.agent_definition_name(),
+        "main",
+        "AgentBuilder::build should default agent_definition_name to \"main\" when unset"
+    );
+}
+
 #[tokio::test]
 async fn turn_without_tools_returns_text() {
     let workspace = tempfile::TempDir::new().expect("temp workspace");