diff --git a/FEATURE_PARITY.md b/FEATURE_PARITY.md index 22091ec013e..426a146861d 100644 --- a/FEATURE_PARITY.md +++ b/FEATURE_PARITY.md @@ -205,7 +205,7 @@ This document tracks feature parity between IronClaw (Rust implementation) and O | Sender_id in trusted metadata | ✅ | ❌ | Exposed in system metadata | | Per-group `systemPrompt` injection | ✅ | ❌ | Per-group/per-direct system prompts injected via `GroupSystemPrompt` (Telegram, Discord, WhatsApp, BlueBubbles) | | Visible reply enforcement | ✅ | ❌ | `messages.visibleReplies` requires output via `message(action=send)`; group-scope override available | -| Active-run steering queue | ✅ | ❌ | `messages.queue` `steer` mode (default) drains queued messages at next model boundary; `queue` legacy one-at-a-time | +| Active-run steering queue | ✅ | 🚧 | Reborn queues busy-thread user messages as steering input for the active run and WebUI shows them as queued until the loop consumes them; `queue` legacy one-at-a-time remains follow-up | | Tool-progress streaming into previews | ✅ | ❌ | Tool progress shown in live preview edits (Discord/Slack/Telegram/Mattermost/Matrix) | | `dmPolicy="open"` semantics | ✅ | 🚧 | Public open-DM only with effective wildcard; pairing-store senders no longer count for DM audits (OpenClaw fixed across all channels) | diff --git a/crates/ironclaw_agent_loop/src/executor/canonical.rs b/crates/ironclaw_agent_loop/src/executor/canonical.rs index 77bb494cc9b..16236b3fba3 100644 --- a/crates/ironclaw_agent_loop/src/executor/canonical.rs +++ b/crates/ironclaw_agent_loop/src/executor/canonical.rs @@ -84,6 +84,19 @@ impl DefaultExecutorPipeline { } InputStep::Exit(exit) => return Ok(exit), } + if !pending_input_ack.is_empty() { + state = CheckpointStage + .process( + ctx, + CheckpointInput { + state, + kind: CheckpointKind::BeforeModel, + }, + ) + .await? + .state; + pending_input_ack.ack(host).await?; + } match self .prompt @@ -96,7 +109,9 @@ impl DefaultExecutorPipeline { ) .await? { - PromptStep::Exit(exit) => return Ok(exit), + PromptStep::Exit(exit) => { + return Ok(exit); + } PromptStep::Prepared(prompt) => { let prompt = *prompt; diff --git a/crates/ironclaw_agent_loop/src/executor/capabilities.rs b/crates/ironclaw_agent_loop/src/executor/capabilities.rs index 02ecf797589..635b7890a21 100644 --- a/crates/ironclaw_agent_loop/src/executor/capabilities.rs +++ b/crates/ironclaw_agent_loop/src/executor/capabilities.rs @@ -983,6 +983,20 @@ impl CapabilityStage { diagnostic_ref: None, }; } + CapabilityOutcome::Completed(result) => { + clear_matching_pending_approval_resume(&mut state, &call); + clear_matching_pending_auth_resume(&mut state, &call); + clear_matching_pending_external_tool_resume(&mut state, &call); + append_completed_capability_result( + ctx.host, + &mut state, + &call, + result, + capability_batch, + ) + .await?; + return Ok(BatchStep::Continue(Box::new(state))); + } promoted => { return Box::pin(self.handle_capability_outcome( ctx, diff --git a/crates/ironclaw_agent_loop/src/executor/input.rs b/crates/ironclaw_agent_loop/src/executor/input.rs index e371961afad..51b21d7583d 100644 --- a/crates/ironclaw_agent_loop/src/executor/input.rs +++ b/crates/ironclaw_agent_loop/src/executor/input.rs @@ -200,6 +200,9 @@ pub(super) fn consume_drainable_inputs( }); } let last_ack = &batch.input_acks[consumed_len - 1]; + if drained && state.prompt_context_cursor.is_none() { + state.prompt_context_cursor = Some(state.input_cursor.clone()); + } state.input_cursor = last_ack.cursor.clone(); let ack_tokens = batch .input_acks @@ -221,7 +224,9 @@ fn user_facing_input_matches_drain_mode(input: &LoopInput, mode: UserFacingInput UserFacingInputDrainMode::FollowUp => { matches!( input, - LoopInput::FollowUp { .. } | LoopInput::UserMessage { .. } + LoopInput::FollowUp { .. } + | LoopInput::UserMessage { .. } + | LoopInput::Steering { .. } ) } } diff --git a/crates/ironclaw_agent_loop/src/executor/loop_exit.rs b/crates/ironclaw_agent_loop/src/executor/loop_exit.rs index 18f3d8f9a1c..10d1cfe1edc 100644 --- a/crates/ironclaw_agent_loop/src/executor/loop_exit.rs +++ b/crates/ironclaw_agent_loop/src/executor/loop_exit.rs @@ -57,6 +57,11 @@ pub(super) async fn try_final_answer_nudge( // `None`s) is what actually forces a tool-free provider call. See the comment // on `LoopModelRequest.capability_view` construction further down. let context_plan = ctx.planner.context().plan_context_request(state).await; + // Consume the prompt-context handoff once, mirroring the main prompt-build + // path (`BuiltPromptBundle`). This nudge builds its own prompt bundle from + // the plan, so the lagging cursor must be cleared here too — otherwise it + // would persist in the checkpointed final state. + state.prompt_context_cursor = None; let mut request = context_plan.request; request.surface_version = None; request.capability_view = None; diff --git a/crates/ironclaw_agent_loop/src/executor/prompt.rs b/crates/ironclaw_agent_loop/src/executor/prompt.rs index ef32c9cd8b1..dde5240c8ac 100644 --- a/crates/ironclaw_agent_loop/src/executor/prompt.rs +++ b/crates/ironclaw_agent_loop/src/executor/prompt.rs @@ -107,6 +107,7 @@ impl BuiltPromptBundle { ) -> Result { let bundle = build_prompt_bundle_for_surface(ctx, state, surface_version, capability_view).await?; + state.prompt_context_cursor = None; refresh_compaction_prompt_from_index(state, &bundle.compaction_message_index); Ok(bundle) } @@ -115,6 +116,7 @@ impl BuiltPromptBundle { self, state: &mut LoopExecutionState, ) -> Vec { + state.prompt_context_cursor = None; refresh_compaction_prompt_from_index(state, &self.compaction_message_index); self.messages } diff --git a/crates/ironclaw_agent_loop/src/executor/tests.rs b/crates/ironclaw_agent_loop/src/executor/tests.rs index 14b4ac9b921..4b3dd3fa7a1 100644 --- a/crates/ironclaw_agent_loop/src/executor/tests.rs +++ b/crates/ironclaw_agent_loop/src/executor/tests.rs @@ -159,6 +159,7 @@ async fn reply_only_drains_follow_up_before_stop_strategy_completes() { assert_eq!( host.checkpoint_kinds(), vec![ + LoopCheckpointKind::BeforeModel, LoopCheckpointKind::BeforeModel, LoopCheckpointKind::BeforeModel, LoopCheckpointKind::Final, @@ -167,6 +168,45 @@ async fn reply_only_drains_follow_up_before_stop_strategy_completes() { assert_eq!(final_staged_state(&host).stop_state.turns_completed, 2); } +#[tokio::test] +async fn reply_only_drains_steering_before_stop_strategy_completes() { + let host = MockHost::new(vec![reply_response(), reply_response()]); + let run_context = host.run_context().clone(); + let host = host.with_input_batches(vec![ + LoopInputBatch { + inputs: Vec::new(), + input_acks: Vec::new(), + next_cursor: input_cursor(&run_context, "input-cursor:no-input"), + }, + LoopInputBatch { + inputs: vec![LoopInput::Steering { + message_ref: message_ref("msg:steering-follow-up"), + }], + input_acks: vec![input_ack( + &run_context, + "input-cursor:after-steering-follow-up", + "input-ack:after-steering-follow-up", + )], + next_cursor: input_cursor(&run_context, "input-cursor:after-steering-follow-up"), + }, + ]); + let executor = CanonicalAgentLoopExecutor; + let state = LoopExecutionState::initial_for_run(host.run_context()); + + let exit = executor + .execute_family(&crate::families::default(), &host, state) + .await + .expect("execute"); + + assert!(matches!(exit, LoopExit::Completed(_))); + assert_eq!(host.model_requests().len(), 2); + assert_eq!( + host.acked_input_tokens(), + vec![LoopInputAckToken::new("input-ack:after-steering-follow-up").expect("valid")] + ); + assert_eq!(final_staged_state(&host).stop_state.turns_completed, 2); +} + #[tokio::test] async fn reply_only_uses_configured_stop_strategy_decision() { let host = MockHost::new(vec![reply_response(), reply_response()]); @@ -1065,6 +1105,10 @@ async fn input_stage_steering_drain_carries_pending_ack() { state.input_cursor, input_cursor(&run_context, "input-cursor:after-user") ); + assert_eq!( + state.prompt_context_cursor, + Some(LoopInputCursor::origin_for_run(&run_context)) + ); assert!(host.acked_input_tokens().is_empty()); pending_input_ack.ack(&host).await.expect("ack inputs"); assert_eq!( @@ -1117,6 +1161,10 @@ async fn input_stage_steering_input_is_drained_like_user_message() { state.input_cursor, input_cursor(&run_context, "input-cursor:after-steering") ); + assert_eq!( + state.prompt_context_cursor, + Some(LoopInputCursor::origin_for_run(&run_context)) + ); } InputStep::Exit(exit) => panic!("expected continue, got {exit:?}"), } @@ -1684,6 +1732,44 @@ async fn no_progress_nudge_synthesizes_reply_when_gate_enabled() { } } +#[tokio::test] +async fn nudge_clears_prompt_context_cursor_in_final_state() { + // Regression: the final-answer nudge builds its own prompt bundle from + // `plan_context_request`, so it must consume the prompt-context handoff + // once — like the main prompt-build path (`BuiltPromptBundle`) — instead of + // leaving a lagging `prompt_context_cursor` in the checkpointed final state. + let host = MockHost::new(vec![reply_response_with_text("Here is the final answer.")]) + .with_driver_nudges_enabled(); + let run_context = host.run_context().clone(); + let family = crate::families::default(); + let ctx = StageContext { + planner: family.planner(), + host: &host, + }; + let mut state = LoopExecutionState::initial_for_run(host.run_context()); + state.prompt_context_cursor = Some(LoopInputCursor::origin_for_run(&run_context)); + + let exit = ExitStage + .process( + ctx, + ExitInput { + state, + kind: StopKind::NoProgressDetected, + }, + ) + .await + .expect("exit stage"); + + assert!( + matches!(exit, LoopExit::Completed(_)), + "nudge with a queued reply should complete the turn" + ); + assert!( + final_staged_state(&host).prompt_context_cursor.is_none(), + "nudge must clear prompt_context_cursor before the final checkpoint" + ); +} + #[tokio::test] async fn no_progress_skips_nudge_when_gate_disabled() { // Gate OFF: even with a model reply available, no tool-free nudge call is diff --git a/crates/ironclaw_agent_loop/src/executor/tests/cancellation.rs b/crates/ironclaw_agent_loop/src/executor/tests/cancellation.rs index a9aa2d18471..701efff9da4 100644 --- a/crates/ironclaw_agent_loop/src/executor/tests/cancellation.rs +++ b/crates/ironclaw_agent_loop/src/executor/tests/cancellation.rs @@ -292,6 +292,7 @@ async fn steering_drain_acks_only_after_cursor_checkpoint_is_durable() { vec![ "checkpoint:before_model".to_string(), "ack_inputs".to_string(), + "checkpoint:before_model".to_string(), "checkpoint:final".to_string(), ] ); diff --git a/crates/ironclaw_agent_loop/src/executor/tests/support.rs b/crates/ironclaw_agent_loop/src/executor/tests/support.rs index 93f41c515c3..d662d44c663 100644 --- a/crates/ironclaw_agent_loop/src/executor/tests/support.rs +++ b/crates/ironclaw_agent_loop/src/executor/tests/support.rs @@ -29,7 +29,10 @@ use ironclaw_turns::{ use crate::{ default_planner::DefaultPlanner, family::{ComponentDigest, ComponentIdentity, LoopFamily, LoopFamilyId}, - state::{CheckpointKind, GateStrategyState, LoopExecutionState, StopStrategyState}, + state::{ + CheckpointKind, GateStrategyState, LoopExecutionState, RecoveryAttemptClass, + StopStrategyState, + }, strategies::{ CapabilityErrorClass, CapabilityErrorSummary, CapabilityFilter, CapabilityStrategy, ContextStrategy, DefaultBudgetStrategy, DefaultCompactionStrategy, GateHandlingStrategy, @@ -464,7 +467,9 @@ impl RecoveryStrategy for RetryPolicyDeniedRecoveryStrategy { ) -> RecoveryOutcome { if err.class == CapabilityErrorClass::PolicyDenied { return RecoveryOutcome::Retry { - recovery: state.recovery_state.clone(), + recovery: state + .recovery_state + .with_incremented_attempts_for(RecoveryAttemptClass::CapabilityInternal), scope: RetryScope::Call, alter: None, }; diff --git a/crates/ironclaw_agent_loop/src/state.rs b/crates/ironclaw_agent_loop/src/state.rs index c425cc76372..a2c01669585 100644 --- a/crates/ironclaw_agent_loop/src/state.rs +++ b/crates/ironclaw_agent_loop/src/state.rs @@ -55,6 +55,12 @@ pub struct LoopExecutionState { pub result_refs: Vec, pub last_gate: Option, pub input_cursor: LoopInputCursor, + /// Cursor to use for the next prompt context after draining queued + /// user-facing input. This intentionally lags behind `input_cursor` so the + /// message can be acked without disappearing from the prompt bundle's + /// `after` window. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub prompt_context_cursor: Option, pub surface_version: Option, // executor-observed (populated by executor; read-only to strategies) @@ -258,6 +264,7 @@ impl LoopExecutionState { result_refs: Vec::new(), last_gate: None, input_cursor: LoopInputCursor::origin_for_run(context), + prompt_context_cursor: None, surface_version: None, recent_call_signatures: BoundedRing::new(), seen_capability_output_digests: BoundedRing::new(), diff --git a/crates/ironclaw_agent_loop/src/strategies/context.rs b/crates/ironclaw_agent_loop/src/strategies/context.rs index e2936ab62ac..c37fc719cea 100644 --- a/crates/ironclaw_agent_loop/src/strategies/context.rs +++ b/crates/ironclaw_agent_loop/src/strategies/context.rs @@ -70,7 +70,12 @@ impl ContextStrategy for DefaultContextStrategy { ContextPlan { request: LoopPromptBundleRequest { mode: PromptMode::TextOnly, - context_cursor: None, + context_cursor: Some( + state + .prompt_context_cursor + .clone() + .unwrap_or_else(|| state.input_cursor.clone()), + ), surface_version: None, checkpoint_state_ref: None, max_messages: Some(self.max_messages.max(1)), @@ -130,8 +135,9 @@ mod tests { AgentLoopDriverDescriptor, RunProfileId, RunProfileVersion, TurnId, TurnRunId, TurnScope, run_profile::{ CancellationPolicy, CapabilitySurfaceProfileId, CheckpointPolicy, CheckpointSchemaId, - ConcurrencyClass, ContextProfileId, LoopDriverId, LoopRunContext, ModelProfileId, - PromptMode, RedactedRunProfileProvenance, ResolvedRunProfile, ResourceBudgetPolicy, + ConcurrencyClass, ContextProfileId, LoopDriverId, LoopInputCursor, + LoopInputCursorToken, LoopRunContext, ModelProfileId, PromptMode, + RedactedRunProfileProvenance, ResolvedRunProfile, ResourceBudgetPolicy, ResourceBudgetTier, RunClassId, RunProfileFingerprint, RuntimeProfileConstraints, SchedulingClass, SteeringPolicy, }, @@ -237,11 +243,31 @@ mod tests { assert!(request.request.inline_messages.is_empty()); assert!(!request.emitted_admission_control); assert!(!request.emitted_repeated_call_warning); - assert!(request.request.context_cursor.is_none()); + assert_eq!( + request.request.context_cursor, + Some(state.input_cursor.clone()) + ); assert!(request.request.surface_version.is_none()); assert!(request.request.checkpoint_state_ref.is_none()); } + #[tokio::test] + async fn plan_context_request_prefers_pending_prompt_context_cursor() { + let strategy = DefaultContextStrategy::default(); + let run_context = test_run_context(); + let mut state = LoopExecutionState::initial_for_run(&run_context); + let original_cursor = state.input_cursor.clone(); + state.input_cursor = LoopInputCursor::from_host_token( + &run_context, + LoopInputCursorToken::new("input-cursor:after-queued").expect("valid cursor"), + ); + state.prompt_context_cursor = Some(original_cursor.clone()); + + let request = strategy.plan_context_request(&state).await; + + assert_eq!(request.request.context_cursor, Some(original_cursor)); + } + #[tokio::test] async fn plan_context_request_clamps_zero_to_one() { let strategy = DefaultContextStrategy { max_messages: 0 }; diff --git a/crates/ironclaw_loop_support/src/durable_input_queue.rs b/crates/ironclaw_loop_support/src/durable_input_queue.rs new file mode 100644 index 00000000000..24dd9f94aa1 --- /dev/null +++ b/crates/ironclaw_loop_support/src/durable_input_queue.rs @@ -0,0 +1,705 @@ +//! Durable, filesystem-backed host input queue. +//! +//! [`InMemoryHostInputQueue`](crate::InMemoryHostInputQueue) keeps queued +//! steering inputs in a process-local map, so a daemon restart drops any +//! message that was queued-but-not-yet-consumed while the owning run is resumed +//! from its durable checkpoint — the message stays `Queued` in the transcript +//! forever and is never delivered. +//! +//! [`FilesystemHostInputQueue`] persists each run's queue as a single +//! CAS-guarded JSON document under the run-scoped filesystem, so the queue +//! survives restart and the resumed run drains it exactly as before. The +//! document stores per-entry *sequences* (not the opaque cursor/ack tokens); +//! the tokens are reconstructed deterministically from the sequence via the +//! shared helpers in [`crate::input_queue`], so the loop's persisted input +//! cursor stays valid across a restart. +//! +//! Scope preservation: the queue document is written through a +//! [`ScopedFilesystem`] under the owner [`ResourceScope`] the composition +//! passes at construction (built from the run's tenant / user / agent / +//! project). In multi-tenant composition the mount-view resolver rewrites that +//! scope into the virtual path prefix (`/tenants//users//…`), so +//! the record *is* tenant/user-partitioned at the storage boundary — the scope +//! is not dropped. The path itself is then keyed by the globally-unique +//! `run_id` (a UUID), which guarantees no cross-run or cross-tenant collision +//! and lets the resumed run find its own queue. The per-message [`ThreadScope`] +//! that drives the `Queued → Submitted` status flip travels in the record +//! payload ([`DurableStatusUpdate`]). +//! +//! What is *deferred*: finer per-run path granularity inside that owner scope +//! (e.g. a per-thread subtree). The `HostInputQueue` trait methods +//! (`next_after`, `ack_consumed`) receive only `run_id`, not a scope, so +//! per-run path partitioning would need either a `run_id → scope` map or a +//! trait change. `run_id` uniqueness makes that unnecessary for correctness or +//! isolation, so it is intentionally left out here. + +use std::collections::HashSet; +use std::sync::Arc; + +use async_trait::async_trait; +use ironclaw_filesystem::{ + CasExpectation, ContentType, Entry, FilesystemError, RecordVersion, RootFilesystem, + ScopedFilesystem, +}; +use ironclaw_host_api::{ResourceScope, ScopedPath}; +use ironclaw_threads::{SessionThreadService, ThreadMessageId, ThreadScope}; +use ironclaw_turns::{ + TurnId, TurnRunId, + run_profile::{LoopInput, LoopInputAckToken, LoopInputCursorToken}, +}; +use serde::{Deserialize, Serialize}; + +use crate::input_queue::{ + EnqueueQueuedMessageRequest, HostInputBatch, HostInputEnqueuePort, HostInputEnvelope, + HostInputQueue, HostInputQueueError, ack_sequence, ack_token, cursor_sequence, cursor_token, +}; + +/// Bounds the CAS retry loop so persistent contention surfaces as a host error +/// instead of spinning forever. Per-run contention is low (one producer thread +/// enqueuing, one loop thread acking), so a handful of retries is ample. +const MAX_CAS_RETRIES: usize = 8; + +/// Durable per-run queue document persisted as JSON at the run's queue path. +#[derive(Debug, Default, Clone, Serialize, Deserialize)] +struct DurableRunQueue { + next_sequence: u64, + entries: Vec, + /// Sequences whose inputs have been consumed and acked. Retained (even + /// after the entry payload is pruned) so a duplicate/redelivered ack is + /// skipped idempotently. + acked: Vec, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +struct DurableEntry { + sequence: u64, + input: LoopInput, + status: DurableStatusUpdate, +} + +/// The transcript message bound to a queued input, used to flip its status to +/// `Submitted` once the input is consumed. +#[derive(Debug, Clone, Serialize, Deserialize)] +struct DurableStatusUpdate { + turn_id: TurnId, + scope: ThreadScope, + thread_id: ironclaw_host_api::ThreadId, + message_id: ThreadMessageId, +} + +/// Filesystem-backed [`HostInputQueue`] / [`HostInputEnqueuePort`]. +pub struct FilesystemHostInputQueue +where + F: RootFilesystem + ?Sized, +{ + filesystem: Arc>, + owner_scope: ResourceScope, + thread_service: Arc, +} + +impl std::fmt::Debug for FilesystemHostInputQueue +where + F: RootFilesystem + ?Sized, +{ + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter + .debug_struct("FilesystemHostInputQueue") + .field("owner_scope", &self.owner_scope) + .finish_non_exhaustive() + } +} + +impl FilesystemHostInputQueue +where + F: RootFilesystem + ?Sized + 'static, +{ + /// Build a durable queue over `filesystem`, persisting under `owner_scope`. + /// `thread_service` performs the queued-message status flip on ack. + pub fn new( + filesystem: Arc>, + owner_scope: ResourceScope, + thread_service: Arc, + ) -> Self { + Self { + filesystem, + owner_scope, + thread_service, + } + } + + async fn load( + &self, + run_id: TurnRunId, + ) -> Result<(DurableRunQueue, Option), HostInputQueueError> { + let path = queue_path(run_id)?; + match self.filesystem.get(&self.owner_scope, &path).await { + Ok(Some(versioned)) => { + let queue = serde_json::from_slice(&versioned.entry.body).map_err(|error| { + HostInputQueueError::Unavailable { + reason: format!("durable input queue is corrupt: {error}"), + } + })?; + Ok((queue, Some(versioned.version))) + } + Ok(None) => Ok((DurableRunQueue::default(), None)), + Err(error) => Err(fs_error(error)), + } + } + + /// Persist `queue`, asserting the expected CAS precondition. `version` is + /// `None` for a first write (`Absent`) and `Some` for an update. A CAS + /// conflict is reported as [`StorePutError::Conflict`] so callers can retry; + /// every other failure is [`StorePutError::Fatal`]. + async fn store( + &self, + run_id: TurnRunId, + queue: &DurableRunQueue, + version: Option, + ) -> Result<(), StorePutError> { + let body = serde_json::to_vec(queue).map_err(|error| { + StorePutError::Fatal(HostInputQueueError::Unavailable { + reason: format!("durable input queue serialization failed: {error}"), + }) + })?; + let entry = Entry::bytes(body).with_content_type(ContentType::json()); + let cas = match version { + Some(version) => CasExpectation::Version(version), + None => CasExpectation::Absent, + }; + let path = queue_path(run_id).map_err(StorePutError::Fatal)?; + match self + .filesystem + .put(&self.owner_scope, &path, entry, cas) + .await + { + Ok(_) => Ok(()), + Err(FilesystemError::VersionMismatch { .. }) => Err(StorePutError::Conflict), + Err(error) => Err(StorePutError::Fatal(fs_error(error))), + } + } +} + +/// Outcome of a CAS-guarded durable write. +enum StorePutError { + /// The CAS precondition failed — a concurrent writer won; retry. + Conflict, + /// A non-retryable failure (serialization, backend IO, bad path). + Fatal(HostInputQueueError), +} + +#[async_trait] +impl HostInputEnqueuePort for FilesystemHostInputQueue +where + F: RootFilesystem + ?Sized + 'static, +{ + async fn enqueue_queued_message( + &self, + request: EnqueueQueuedMessageRequest, + ) -> Result { + let EnqueueQueuedMessageRequest { + run_id, + turn_id, + scope, + thread_id, + message_id, + input, + } = request; + for _ in 0..MAX_CAS_RETRIES { + let (mut queue, version) = self.load(run_id).await?; + // Dedup by input so a retried enqueue of the same message reuses its + // entry rather than queuing it twice. + if let Some(existing) = queue.entries.iter().find(|entry| entry.input == input) { + return envelope_for(existing.sequence, input.clone()); + } + let sequence = queue.next_sequence; + queue.next_sequence = queue.next_sequence.saturating_add(1); + queue.entries.push(DurableEntry { + sequence, + input: input.clone(), + status: DurableStatusUpdate { + turn_id, + scope: scope.clone(), + thread_id: thread_id.clone(), + message_id, + }, + }); + match self.store(run_id, &queue, version).await { + Ok(()) => return envelope_for(sequence, input), + Err(StorePutError::Conflict) => continue, + Err(StorePutError::Fatal(error)) => return Err(error), + } + } + Err(cas_exhausted("enqueue")) + } +} + +#[async_trait] +impl HostInputQueue for FilesystemHostInputQueue +where + F: RootFilesystem + ?Sized + 'static, +{ + async fn next_after( + &self, + run_id: TurnRunId, + after: LoopInputCursorToken, + limit: usize, + ) -> Result { + let after_sequence = cursor_sequence(&after)?; + let (queue, version) = self.load(run_id).await?; + if version.is_none() { + return Ok(HostInputBatch { + inputs: Vec::new(), + next_cursor: after, + }); + } + if after_sequence > queue.next_sequence { + return Err(HostInputQueueError::InvalidCursor { + reason: "input cursor is ahead of the run input queue".to_string(), + }); + } + let acked: HashSet = queue.acked.iter().copied().collect(); + let mut inputs = Vec::new(); + let mut next_sequence = after_sequence; + let mut ordered: Vec<&DurableEntry> = queue + .entries + .iter() + .filter(|entry| entry.sequence >= after_sequence) + .collect(); + ordered.sort_by_key(|entry| entry.sequence); + for entry in ordered { + next_sequence = entry.sequence.saturating_add(1); + if acked.contains(&entry.sequence) { + continue; + } + if inputs.len() >= limit { + next_sequence = entry.sequence; + break; + } + inputs.push(envelope_for(entry.sequence, entry.input.clone())?); + } + Ok(HostInputBatch { + inputs, + next_cursor: cursor_token(next_sequence)?, + }) + } + + async fn ack_consumed( + &self, + run_id: TurnRunId, + tokens: Vec, + ) -> Result<(), HostInputQueueError> { + // Phase 1: durably record the acks (CAS retry). The cursor ack is the + // load-bearing transition — its failure is a genuine durable-IO fault + // and is surfaced, so the run does not silently drop a consumed input. + let mut status_updates = Vec::new(); + let mut committed = false; + for _ in 0..MAX_CAS_RETRIES { + let (mut queue, version) = self.load(run_id).await?; + let Some(version) = version else { + // No durable queue for this run: nothing to ack. + return Ok(()); + }; + let already: HashSet = queue.acked.iter().copied().collect(); + let mut newly_acked = Vec::new(); + status_updates.clear(); + for token in &tokens { + let sequence = ack_sequence(token)?; + if already.contains(&sequence) { + continue; + } + // Fail loud on a token for a sequence that is neither live nor + // already acked. Committing an unknown sequence into `acked` + // would poison durable state: when that sequence is eventually + // enqueued, its (now pre-acked) entry would be skipped forever + // by `next_after`. A stale/forged token is a genuine fault, not + // a redelivered ack (which lands in `already` above). + let Some(entry) = queue.entries.iter().find(|e| e.sequence == sequence) else { + return Err(HostInputQueueError::InvalidCursor { + reason: format!( + "ack token references sequence {sequence} that is neither live \ + nor already acked for this run" + ), + }); + }; + status_updates.push(entry.status.clone()); + newly_acked.push(sequence); + } + if newly_acked.is_empty() { + return Ok(()); + } + queue.acked.extend(newly_acked); + // Prune consumed entry payloads to bound the document size; the + // sequence stays in `acked` for idempotency, and `next_sequence` + // is the high-water mark so a stale cursor never looks "ahead". + let acked_now: HashSet = queue.acked.iter().copied().collect(); + queue + .entries + .retain(|entry| !acked_now.contains(&entry.sequence)); + match self.store(run_id, &queue, Some(version)).await { + Ok(()) => { + committed = true; + break; + } + Err(StorePutError::Conflict) => continue, + Err(StorePutError::Fatal(error)) => return Err(error), + } + } + if !committed { + return Err(cas_exhausted("ack_consumed")); + } + + // Phase 2: best-effort transcript status flip. The input is already + // durably acked; a status-write failure must NOT fail the ack (it would + // map to a terminal HostUnavailable and kill the run — see + // `.claude/rules/agent-loop-capabilities.md`, Invariant 1). Log and move + // on; the transcript badge may lag but the run continues. + for update in status_updates { + if let Err(error) = self + .thread_service + .mark_message_submitted( + &update.scope, + &update.thread_id, + update.message_id, + update.turn_id.to_string(), + run_id.to_string(), + ) + .await + { + tracing::warn!( + component = "durable_host_input_queue", + operation = "mark_message_submitted", + %run_id, + error = %error, + "queued-message status flip failed after the input was durably acked; \ + run continues (transcript badge may lag)" + ); + } + } + Ok(()) + } +} + +fn envelope_for(sequence: u64, input: LoopInput) -> Result { + Ok(HostInputEnvelope { + input, + cursor: cursor_token(sequence)?, + ack_token: ack_token(sequence)?, + }) +} + +fn queue_path(run_id: TurnRunId) -> Result { + ScopedPath::new(format!("/turns/input-queue/{}.json", run_id.as_uuid())).map_err(|error| { + HostInputQueueError::Unavailable { + reason: format!("invalid input queue path: {error}"), + } + }) +} + +fn fs_error(error: FilesystemError) -> HostInputQueueError { + HostInputQueueError::Unavailable { + reason: error.to_string(), + } +} + +fn cas_exhausted(operation: &str) -> HostInputQueueError { + HostInputQueueError::Unavailable { + reason: format!("durable input queue {operation} contended past retry budget"), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use ironclaw_filesystem::InMemoryBackend; + use ironclaw_host_api::{ + AgentId, MountAlias, MountGrant, MountPermissions, MountView, ProjectId, TenantId, + ThreadId, VirtualPath, + }; + use ironclaw_threads::{ + AcceptInboundMessageRequest, EnsureThreadRequest, InMemorySessionThreadService, + MessageContent, MessageStatus, ThreadHistoryRequest, + }; + use ironclaw_turns::{LoopMessageRef, TurnScope}; + + fn make_fs(backend: Arc) -> Arc> { + let mounts = MountView::new(vec![MountGrant::new( + MountAlias::new("/turns").unwrap(), + VirtualPath::new("/turns").unwrap(), + MountPermissions::read_write_list_delete(), + )]) + .unwrap(); + Arc::new(ScopedFilesystem::with_fixed_view(backend, mounts)) + } + + fn owner_scope() -> ResourceScope { + TurnScope::new( + TenantId::new("tenant-iq").unwrap(), + Some(AgentId::new("agent-iq").unwrap()), + Some(ProjectId::new("project-iq").unwrap()), + ThreadId::new("thread-iq").unwrap(), + ) + .to_resource_scope() + } + + // ThreadScope carries no thread_id (the thread is addressed separately), so + // one fixed scope serves both the real-message and ghost-message tests. + fn ghost_scope() -> ThreadScope { + ThreadScope { + tenant_id: TenantId::new("tenant-iq").unwrap(), + agent_id: AgentId::new("agent-iq").unwrap(), + project_id: None, + owner_user_id: None, + mission_id: None, + } + } + + fn steering(message_ref: &str) -> LoopInput { + LoopInput::Steering { + message_ref: LoopMessageRef::new(message_ref).unwrap(), + } + } + + fn origin() -> LoopInputCursorToken { + LoopInputCursorToken::new("input-cursor:origin".to_string()).unwrap() + } + + #[tokio::test] + async fn durable_queue_survives_store_reconstruction() { + // The core durability guarantee: a message queued before a restart is + // still drainable after, and the reconstructed cursor/ack tokens match + // the ones the loop's persisted input cursor references. + let backend = Arc::new(InMemoryBackend::new()); + let thread_service: Arc = + Arc::new(InMemorySessionThreadService::default()); + let run_id = TurnRunId::new(); + let input = steering("msg:restart"); + + // First "process": enqueue, then drop the queue object. + let envelope = { + let queue = FilesystemHostInputQueue::new( + make_fs(Arc::clone(&backend)), + owner_scope(), + Arc::clone(&thread_service), + ); + queue + .enqueue_queued_message(EnqueueQueuedMessageRequest { + run_id, + turn_id: TurnId::new(), + scope: ghost_scope(), + thread_id: ThreadId::new("ghost").unwrap(), + message_id: ThreadMessageId::new(), + input: input.clone(), + }) + .await + .expect("enqueue") + }; + + // Second "process" (restart): a brand-new queue object over the SAME + // durable backend must surface the queued input. + let queue = FilesystemHostInputQueue::new( + make_fs(Arc::clone(&backend)), + owner_scope(), + thread_service, + ); + let batch = queue + .next_after(run_id, origin(), 8) + .await + .expect("poll after restart"); + assert_eq!(batch.inputs.len(), 1); + assert_eq!(batch.inputs[0].input, input); + assert_eq!(batch.inputs[0].ack_token, envelope.ack_token); + assert_eq!(batch.inputs[0].cursor, envelope.cursor); + } + + #[tokio::test] + async fn enqueue_poll_ack_flips_status_and_stops_redelivery() { + let backend = Arc::new(InMemoryBackend::new()); + let thread_service = Arc::new(InMemorySessionThreadService::default()); + let scope = ghost_scope(); + let thread = thread_service + .ensure_thread(EnsureThreadRequest { + scope: scope.clone(), + thread_id: None, + created_by_actor_id: "actor-iq".into(), + title: None, + metadata_json: None, + }) + .await + .unwrap(); + let accepted = thread_service + .accept_inbound_message(AcceptInboundMessageRequest { + scope: scope.clone(), + thread_id: thread.thread_id.clone(), + actor_id: "actor-iq".into(), + source_binding_id: None, + reply_target_binding_id: None, + external_event_id: None, + content: MessageContent::text("queued steering"), + }) + .await + .unwrap(); + let run_id = TurnRunId::new(); + thread_service + .mark_message_queued( + &scope, + &thread.thread_id, + accepted.message_id, + run_id.to_string(), + ) + .await + .unwrap(); + + let queue = FilesystemHostInputQueue::new( + make_fs(backend), + owner_scope(), + Arc::clone(&thread_service) as Arc, + ); + queue + .enqueue_queued_message(EnqueueQueuedMessageRequest { + run_id, + turn_id: TurnId::new(), + scope: scope.clone(), + thread_id: thread.thread_id.clone(), + message_id: accepted.message_id, + input: steering(&format!("msg:{}", accepted.message_id)), + }) + .await + .expect("enqueue"); + + let batch = queue.next_after(run_id, origin(), 8).await.expect("poll"); + assert_eq!(batch.inputs.len(), 1); + + queue + .ack_consumed(run_id, vec![batch.inputs[0].ack_token.clone()]) + .await + .expect("ack"); + + // Status durably flipped to Submitted... + let history = thread_service + .list_thread_history(ThreadHistoryRequest { + scope, + thread_id: thread.thread_id, + }) + .await + .unwrap(); + assert_eq!(history.messages[0].status, MessageStatus::Submitted); + // ...and the consumed input is not redelivered. + let after = queue + .next_after(run_id, batch.next_cursor, 8) + .await + .expect("poll after ack"); + assert!(after.inputs.is_empty()); + } + + #[tokio::test] + async fn ack_is_non_fatal_and_idempotent_when_status_flip_fails() { + let backend = Arc::new(InMemoryBackend::new()); + let thread_service: Arc = + Arc::new(InMemorySessionThreadService::default()); + let queue = FilesystemHostInputQueue::new(make_fs(backend), owner_scope(), thread_service); + let run_id = TurnRunId::new(); + let envelope = queue + .enqueue_queued_message(EnqueueQueuedMessageRequest { + run_id, + turn_id: TurnId::new(), + scope: ghost_scope(), + thread_id: ThreadId::new("ghost").unwrap(), + message_id: ThreadMessageId::new(), + input: steering("msg:ghost"), + }) + .await + .expect("enqueue"); + + // Status flip fails (ghost thread) but the ack still commits durably. + queue + .ack_consumed(run_id, vec![envelope.ack_token.clone()]) + .await + .expect("ack must be non-fatal when the status flip fails"); + // A redelivered ack for the same token is an idempotent no-op. + queue + .ack_consumed(run_id, vec![envelope.ack_token]) + .await + .expect("idempotent ack"); + + let batch = queue.next_after(run_id, origin(), 8).await.expect("poll"); + assert!( + batch.inputs.is_empty(), + "acked input must not be redelivered" + ); + } + + #[tokio::test] + async fn enqueue_dedups_identical_input() { + let backend = Arc::new(InMemoryBackend::new()); + let thread_service: Arc = + Arc::new(InMemorySessionThreadService::default()); + let queue = FilesystemHostInputQueue::new(make_fs(backend), owner_scope(), thread_service); + let run_id = TurnRunId::new(); + let request = || EnqueueQueuedMessageRequest { + run_id, + turn_id: TurnId::new(), + scope: ghost_scope(), + thread_id: ThreadId::new("ghost").unwrap(), + message_id: ThreadMessageId::new(), + input: steering("msg:dup"), + }; + let first = queue + .enqueue_queued_message(request()) + .await + .expect("first"); + let second = queue + .enqueue_queued_message(request()) + .await + .expect("second"); + assert_eq!(first.ack_token, second.ack_token, "identical input dedups"); + + let batch = queue.next_after(run_id, origin(), 8).await.expect("poll"); + assert_eq!(batch.inputs.len(), 1, "dedup keeps a single queue entry"); + } + + #[tokio::test] + async fn ack_rejects_unknown_sequence_instead_of_poisoning_state() { + // An ack token for a sequence that is neither live nor already acked + // must fail loud rather than be committed into `acked`. Committing it + // would poison durable state: when that sequence is later enqueued, its + // now-pre-acked entry would be skipped forever by `next_after`. + let backend = Arc::new(InMemoryBackend::new()); + let thread_service: Arc = + Arc::new(InMemorySessionThreadService::default()); + let queue = FilesystemHostInputQueue::new( + make_fs(Arc::clone(&backend)), + owner_scope(), + thread_service, + ); + let run_id = TurnRunId::new(); + // Create the queue document with a single live entry at sequence 0. + queue + .enqueue_queued_message(EnqueueQueuedMessageRequest { + run_id, + turn_id: TurnId::new(), + scope: ghost_scope(), + thread_id: ThreadId::new("ghost").unwrap(), + message_id: ThreadMessageId::new(), + input: steering("msg:live"), + }) + .await + .expect("enqueue"); + + // Ack a forged token for a sequence that was never enqueued. + let forged = LoopInputAckToken::new("input-ack:999".to_string()).unwrap(); + let result = queue.ack_consumed(run_id, vec![forged]).await; + assert!( + matches!(result, Err(HostInputQueueError::InvalidCursor { .. })), + "unknown ack sequence must be rejected, got {result:?}" + ); + + // State is untouched: sequence 999 was NOT recorded as acked, so a + // later real entry at that sequence would still be delivered. + let batch = queue.next_after(run_id, origin(), 8).await.expect("poll"); + assert_eq!( + batch.inputs.len(), + 1, + "the live entry remains deliverable after a rejected forged ack" + ); + } +} diff --git a/crates/ironclaw_loop_support/src/input_queue.rs b/crates/ironclaw_loop_support/src/input_queue.rs index 1817f90ce08..b1deeddda08 100644 --- a/crates/ironclaw_loop_support/src/input_queue.rs +++ b/crates/ironclaw_loop_support/src/input_queue.rs @@ -1,8 +1,13 @@ //! Host-owned input queue contract for Reborn loop input ports. +use std::collections::{HashMap, HashSet}; +use std::sync::{Arc, Mutex}; + use async_trait::async_trait; +use ironclaw_host_api::ThreadId; +use ironclaw_threads::{SessionThreadService, ThreadMessageId, ThreadScope}; use ironclaw_turns::{ - TurnRunId, + TurnId, TurnRunId, run_profile::{LoopInput, LoopInputAckToken, LoopInputCursorToken}, }; use thiserror::Error; @@ -72,3 +77,353 @@ pub enum HostInputQueueError { #[error("input queue internal error")] Internal, } + +#[async_trait] +pub trait HostInputEnqueuePort: Send + Sync { + /// Enqueue a user message as steering/followup input for an active run. + /// + /// The request carries the originating thread message identity so the queue + /// can transition that message to `submitted` once the input is consumed. + /// There is deliberately no metadata-free variant: every enqueued input is + /// backed by a thread message, so the status transition can never be + /// silently dropped. + async fn enqueue_queued_message( + &self, + request: EnqueueQueuedMessageRequest, + ) -> Result; +} + +/// Null-object enqueue port used as the default when a host has not wired a +/// real input queue. Every enqueue fails closed with `Unavailable` rather than +/// silently dropping the message. Production runtimes always replace this with +/// the host-owned queue; it exists so callers can hold a non-optional +/// `Arc` instead of an `Option` that production never +/// leaves unset. +#[derive(Debug, Default, Clone, Copy)] +pub struct RejectingInputEnqueue; + +#[async_trait] +impl HostInputEnqueuePort for RejectingInputEnqueue { + async fn enqueue_queued_message( + &self, + _request: EnqueueQueuedMessageRequest, + ) -> Result { + Err(HostInputQueueError::Unavailable { + reason: "input queue is not wired for this runtime".to_string(), + }) + } +} + +#[derive(Debug, Clone)] +pub struct EnqueueQueuedMessageRequest { + pub run_id: TurnRunId, + pub turn_id: TurnId, + pub scope: ThreadScope, + pub thread_id: ThreadId, + pub message_id: ThreadMessageId, + pub input: LoopInput, +} + +#[derive(Clone)] +struct QueuedMessageStatusUpdate { + turn_id: TurnId, + scope: ThreadScope, + thread_id: ThreadId, + message_id: ThreadMessageId, +} + +pub struct InMemoryHostInputQueue { + state: Arc>, + thread_service: Arc, +} + +impl std::fmt::Debug for InMemoryHostInputQueue { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter + .debug_struct("InMemoryHostInputQueue") + .field("state", &self.state) + .finish() + } +} + +#[derive(Default)] +struct InMemoryHostInputQueueState { + runs: HashMap, +} + +impl std::fmt::Debug for InMemoryHostInputQueueState { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter + .debug_struct("InMemoryHostInputQueueState") + .finish() + } +} + +#[derive(Default)] +struct InMemoryRunInputQueue { + entries: Vec, + acked: HashSet, + next_sequence: u64, +} + +#[derive(Clone)] +struct InMemoryInputEntry { + sequence: u64, + envelope: HostInputEnvelope, + queued_message: Option, +} + +impl InMemoryHostInputQueue { + pub fn new(thread_service: Arc) -> Self { + Self { + state: Arc::new(Mutex::new(InMemoryHostInputQueueState::default())), + thread_service, + } + } + + /// Enqueue `input` for `run_id`, attaching `queued_message` status metadata. + /// + /// Identical inputs already queued for the run are deduplicated; the first + /// status binding for an entry wins. + fn enqueue_with( + &self, + run_id: TurnRunId, + input: LoopInput, + queued_message: QueuedMessageStatusUpdate, + ) -> Result { + let mut state = self + .state + .lock() + .map_err(|_| HostInputQueueError::Internal)?; + let queue = state.runs.entry(run_id).or_default(); + if let Some(existing) = queue + .entries + .iter_mut() + .find(|entry| entry.envelope.input == input) + { + existing.queued_message.get_or_insert(queued_message); + return Ok(existing.envelope.clone()); + } + let sequence = queue.next_sequence; + queue.next_sequence = queue.next_sequence.saturating_add(1); + let envelope = HostInputEnvelope { + input, + cursor: cursor_token(sequence)?, + ack_token: ack_token(sequence)?, + }; + queue.entries.push(InMemoryInputEntry { + sequence, + envelope: envelope.clone(), + queued_message: Some(queued_message), + }); + Ok(envelope) + } +} + +#[async_trait] +impl HostInputEnqueuePort for InMemoryHostInputQueue { + async fn enqueue_queued_message( + &self, + request: EnqueueQueuedMessageRequest, + ) -> Result { + let EnqueueQueuedMessageRequest { + run_id, + turn_id, + scope, + thread_id, + message_id, + input, + } = request; + self.enqueue_with( + run_id, + input, + QueuedMessageStatusUpdate { + turn_id, + scope, + thread_id, + message_id, + }, + ) + } +} + +#[async_trait] +impl HostInputQueue for InMemoryHostInputQueue { + async fn next_after( + &self, + run_id: TurnRunId, + after: LoopInputCursorToken, + limit: usize, + ) -> Result { + let after_sequence = cursor_sequence(&after)?; + let state = self + .state + .lock() + .map_err(|_| HostInputQueueError::Internal)?; + let Some(queue) = state.runs.get(&run_id) else { + return Ok(HostInputBatch { + inputs: Vec::new(), + next_cursor: after, + }); + }; + if after_sequence > queue.next_sequence { + return Err(HostInputQueueError::InvalidCursor { + reason: "input cursor is ahead of the run input queue".to_string(), + }); + } + let mut inputs = Vec::new(); + let mut next_sequence = after_sequence; + for entry in queue + .entries + .iter() + .filter(|entry| entry.sequence >= after_sequence) + { + next_sequence = entry.sequence.saturating_add(1); + if queue.acked.contains(&entry.envelope.ack_token) { + continue; + } + if inputs.len() >= limit { + next_sequence = entry.sequence; + break; + } + inputs.push(entry.envelope.clone()); + } + Ok(HostInputBatch { + inputs, + next_cursor: cursor_token(next_sequence)?, + }) + } + + async fn ack_consumed( + &self, + run_id: TurnRunId, + tokens: Vec, + ) -> Result<(), HostInputQueueError> { + let (tokens_to_ack, updates) = { + let state = self + .state + .lock() + .map_err(|_| HostInputQueueError::Internal)?; + let Some(queue) = state.runs.get(&run_id) else { + return Ok(()); + }; + let mut tokens_to_ack = Vec::new(); + let mut updates = Vec::new(); + for token in tokens { + if queue.acked.contains(&token) { + continue; + } + // Fail loud on a token that matches no live entry and is not + // already acked. Committing an unknown token into `acked` would + // poison state: a later entry minted for that same sequence + // would be skipped forever by `next_after`. A redelivered ack + // lands in `queue.acked` above; anything else is a genuine + // fault (stale/forged token), not a no-op. + let Some(entry) = queue + .entries + .iter() + .find(|entry| entry.envelope.ack_token == token) + else { + return Err(HostInputQueueError::InvalidCursor { + reason: "ack token references an input that is neither live nor already \ + acked for this run" + .to_string(), + }); + }; + if let Some(update) = &entry.queued_message { + updates.push(update.clone()); + } + tokens_to_ack.push(token); + } + (tokens_to_ack, updates) + }; + // The queued-message status flip (`Queued` → `Submitted`) is best-effort + // bookkeeping for the transcript badge, NOT part of consuming the input. + // The input has already been drained and delivered to the model by the + // time we ack; failing the ack here would map to a terminal + // `HostUnavailable` and kill the whole run for a cosmetic status write + // (see `.claude/rules/agent-loop-capabilities.md`, Invariant 1). So a + // status-update failure is logged with its cause and swallowed — the ack + // still advances so the input is never redelivered. A stale "queued" + // badge is reconcilable; a dead run is not. + for update in updates { + if let Err(source) = self + .thread_service + .mark_message_submitted( + &update.scope, + &update.thread_id, + update.message_id, + update.turn_id.to_string(), + run_id.to_string(), + ) + .await + { + tracing::warn!( + component = "host_input_queue", + operation = "mark_message_submitted", + %run_id, + error = %source, + "queued-message status flip failed after the input was consumed; \ + acking anyway so the run continues (transcript badge may lag)" + ); + } + } + let acked_now: HashSet = tokens_to_ack.iter().cloned().collect(); + let mut state = self + .state + .lock() + .map_err(|_| HostInputQueueError::Internal)?; + let queue = state.runs.entry(run_id).or_default(); + for token in tokens_to_ack { + queue.acked.insert(token); + } + // Drop the consumed entries' payloads (`LoopInput` + `ThreadScope` + // binding) to bound per-run memory over a long-lived run. The ack token + // stays in `acked` so a duplicate/redelivered ack is still skipped + // idempotently by the guard above; `next_sequence` is a separate + // high-water mark, so removing entries never lets a stale cursor look + // "ahead of the queue". + queue + .entries + .retain(|entry| !acked_now.contains(&entry.envelope.ack_token)); + Ok(()) + } +} + +// The cursor/ack token helpers below are shared with the durable queue +// (`durable_input_queue.rs`) so both backends speak the identical +// `input-cursor:{n}` / `input-ack:{n}` token wire format. A durable queue +// rehydrated after restart must mint the same tokens the loop's persisted +// input cursor already references, so this format is the single source of truth. +pub(crate) fn cursor_sequence(token: &LoopInputCursorToken) -> Result { + if token.is_origin() { + return Ok(0); + } + token + .as_str() + .strip_prefix("input-cursor:") + .and_then(|value| value.parse::().ok()) + .ok_or_else(|| HostInputQueueError::InvalidCursor { + reason: "input cursor token is malformed".to_string(), + }) +} + +pub(crate) fn cursor_token(sequence: u64) -> Result { + LoopInputCursorToken::new(format!("input-cursor:{sequence}")) + .map_err(|_| HostInputQueueError::Internal) +} + +pub(crate) fn ack_token(sequence: u64) -> Result { + LoopInputAckToken::new(format!("input-ack:{sequence}")) + .map_err(|_| HostInputQueueError::Internal) +} + +pub(crate) fn ack_sequence(token: &LoopInputAckToken) -> Result { + token + .as_str() + .strip_prefix("input-ack:") + .and_then(|value| value.parse::().ok()) + .ok_or_else(|| HostInputQueueError::InvalidCursor { + reason: "input ack token is malformed".to_string(), + }) +} diff --git a/crates/ironclaw_loop_support/src/lib.rs b/crates/ironclaw_loop_support/src/lib.rs index 165272fe7ba..3222329d0af 100644 --- a/crates/ironclaw_loop_support/src/lib.rs +++ b/crates/ironclaw_loop_support/src/lib.rs @@ -23,6 +23,7 @@ mod capability_port; mod capability_surface_filter; mod compaction_task; mod context_window_cache; +mod durable_input_queue; mod filesystem_checkpoint_state; mod filesystem_skill_bundle_source; pub mod identity_context; @@ -69,6 +70,7 @@ pub use compaction_task::{ default_host_managed_loop_compaction_port, host_managed_loop_compaction_port_with_prompt_id, }; pub use context_window_cache::ThreadContextWindowCache; +pub use durable_input_queue::FilesystemHostInputQueue; pub use filesystem_checkpoint_state::FilesystemCheckpointStateStore; pub use filesystem_skill_bundle_source::{FilesystemSkillBundleRoot, FilesystemSkillBundleSource}; pub use identity_context::{ @@ -79,7 +81,10 @@ pub use identity_context::{ identity_message_ref, }; pub use input_port::HostQueueLoopInputPort; -pub use input_queue::{HostInputBatch, HostInputEnvelope, HostInputQueue, HostInputQueueError}; +pub use input_queue::{ + EnqueueQueuedMessageRequest, HostInputBatch, HostInputEnqueuePort, HostInputEnvelope, + HostInputQueue, HostInputQueueError, InMemoryHostInputQueue, RejectingInputEnqueue, +}; pub use ironclaw_turns::run_profile::PromptContextTokenBudget; pub use skill_bundle_context_source::SkillBundleContextSource; pub use skill_bundle_source::{ @@ -1236,7 +1241,7 @@ where image_parts, }); } - return Ok(messages); + return merge_consecutive_text_user_messages(messages); } let mut messages_by_ref = context_messages_by_ref(context.messages); @@ -1399,7 +1404,7 @@ where image_parts, }); } - Ok(resolved) + merge_consecutive_text_user_messages(resolved) } async fn load_model_context_window( @@ -1459,6 +1464,64 @@ where } } +/// Coalesce runs of consecutive plain-text user messages into a single provider +/// turn (some providers reject consecutive same-role turns). This is the final +/// provider-API shaping step before the request leaves for the gateway. +/// +/// A coalesced turn no longer corresponds to a single thread message, so it must +/// not inherit the first contributor's `content_ref` — that would let downstream +/// code mis-map the merged turn back to one transcript row. Instead the merged +/// message gets a synthetic `msg:coalesced.*` ref. The durable transcript keeps +/// the original rows separate; the only consumer past this point is the provider +/// gateway, which reads role/content, not `content_ref`. +fn merge_consecutive_text_user_messages( + messages: Vec, +) -> Result, AgentLoopHostError> { + let mut merged: Vec = Vec::with_capacity(messages.len()); + for message in messages { + if can_merge_text_user_message(&message) + && let Some(previous) = merged.last_mut() + && can_merge_text_user_message(previous) + { + previous.content.push('\n'); + previous.content.push_str(&message.content); + previous.content_ref = + coalesced_user_message_ref(&previous.content_ref, &message.content_ref)?; + continue; + } + merged.push(message); + } + Ok(merged) +} + +/// Build the synthetic content ref for a coalesced user turn. Deterministic in a +/// turn and intentionally not a real `msg:` ref so it cannot be parsed back +/// into a transcript message identity. The ref is transient (never persisted), +/// so non-cryptographic hashing is sufficient. +fn coalesced_user_message_ref( + first: &LoopMessageRef, + next: &LoopMessageRef, +) -> Result { + use std::hash::{Hash, Hasher}; + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + first.as_str().hash(&mut hasher); + next.as_str().hash(&mut hasher); + let hash = hasher.finish(); + LoopMessageRef::new(format!("msg:coalesced.{hash:016x}")).map_err(|_| { + AgentLoopHostError::new( + AgentLoopHostErrorKind::Internal, + "coalesced user message reference could not be represented", + ) + }) +} + +fn can_merge_text_user_message(message: &HostManagedModelMessage) -> bool { + message.role == HostManagedModelMessageRole::User + && message.tool_result_provider_call.is_none() + && message.tool_result_content.is_none() + && message.image_parts.is_empty() +} + /// Host-managed text-only model gateway. Implementations own provider selection, /// profile policy, retry/circuit behavior, and sanitization. #[async_trait] diff --git a/crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs b/crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs index 45993f2d0f0..97ecb594ae4 100644 --- a/crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs +++ b/crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs @@ -2960,6 +2960,63 @@ async fn model_port_reads_image_attachment_bytes_into_model_image_parts() { assert_eq!(image_parts[0].bytes, vec![1, 2, 3, 4]); } +#[tokio::test] +async fn model_port_merges_consecutive_text_user_messages_for_prompt() { + let fixture = ThreadFixture::new_with_user_content("first follow-up").await; + fixture + .accept_user_message("event-2", "second follow-up") + .await; + fixture + .accept_user_message("event-3", "third follow-up") + .await; + + let gateway = Arc::new(RecordingGateway::reply("merged")); + let port = ThreadBackedLoopModelPort::new( + Arc::clone(&fixture.thread_service), + fixture.thread_scope.clone(), + fixture.run_context.clone(), + gateway.clone(), + 16, + ); + issue_prompt_grant(&fixture.run_context, &[]); + + port.stream_model(LoopModelRequest { + messages: Vec::new(), + surface_version: None, + model_preference: None, + capability_view: None, + }) + .await + .unwrap(); + + let messages = { + let calls = gateway.calls.lock().unwrap(); + assert_eq!(calls.len(), 1); + calls[0].messages.clone() + }; + assert_eq!(messages.len(), 1); + assert_eq!(messages[0].role, HostManagedModelMessageRole::User); + assert_eq!( + messages[0].content, + "first follow-up\nsecond follow-up\nthird follow-up" + ); + + let history = fixture + .thread_service + .list_thread_history(ThreadHistoryRequest { + scope: fixture.thread_scope.clone(), + thread_id: fixture.thread_id.clone(), + }) + .await + .unwrap(); + let user_rows = history + .messages + .iter() + .filter(|message| message.kind == MessageKind::User) + .count(); + assert_eq!(user_rows, 3, "durable transcript rows stay separate"); +} + #[tokio::test] async fn model_port_threads_resolved_model_route_snapshot_to_gateway() { let fixture = ThreadFixture::new().await; diff --git a/crates/ironclaw_product_workflow/Cargo.toml b/crates/ironclaw_product_workflow/Cargo.toml index 67f06ca0bef..ec9661fc2d0 100644 --- a/crates/ironclaw_product_workflow/Cargo.toml +++ b/crates/ironclaw_product_workflow/Cargo.toml @@ -30,6 +30,7 @@ ironclaw_auth = { path = "../ironclaw_auth", version = "0.1.0" } ironclaw_events = { path = "../ironclaw_events", version = "0.1.0" } ironclaw_host_api = { path = "../ironclaw_host_api", version = "0.1.0" } ironclaw_conversations = { path = "../ironclaw_conversations", version = "0.1.0" } +ironclaw_loop_support = { path = "../ironclaw_loop_support", version = "0.1.0" } ironclaw_outbound = { path = "../ironclaw_outbound", version = "0.1.0" } ironclaw_product_adapters = { path = "../ironclaw_product_adapters", version = "0.1.0" } ironclaw_reborn_traces = { path = "../ironclaw_reborn_traces", version = "0.1.0" } diff --git a/crates/ironclaw_product_workflow/src/inbound_turn.rs b/crates/ironclaw_product_workflow/src/inbound_turn.rs index 8a2bfba279a..4042ba80652 100644 --- a/crates/ironclaw_product_workflow/src/inbound_turn.rs +++ b/crates/ironclaw_product_workflow/src/inbound_turn.rs @@ -14,14 +14,15 @@ use chrono::{DateTime, Utc}; use ironclaw_attachments::InboundAttachment; #[cfg(test)] use ironclaw_host_api::UserId; +use ironclaw_loop_support::{HostInputEnqueuePort, HostInputQueueError, RejectingInputEnqueue}; use ironclaw_product_adapters::{ ProductAdapterId, ProductInboundAck, ProductInboundEnvelope, ProductInboundPayload, ProductRejection, }; use ironclaw_threads::{ AcceptInboundMessageRequest, AcceptedInboundMessageReplay, EnsureThreadRequest, MessageContent, - MessageStatus, ReplayAcceptedInboundMessageRequest, SessionThreadService, ThreadMessageId, - ThreadScope, + MessageStatus, ReplayAcceptedInboundMessageRequest, SessionThreadService, ThreadHistoryRequest, + ThreadMessageId, ThreadScope, }; use ironclaw_turns::{ AcceptedMessageRef, SubmitTurnRequest, SubmitTurnResponse, TurnActor, TurnCoordinator, @@ -88,6 +89,11 @@ pub enum InboundTurnOutcome { active_run_id: Option, binding: ResolvedBinding, }, + DeferredBusy { + accepted_message_ref: AcceptedMessageRef, + active_run_id: TurnRunId, + binding: ResolvedBinding, + }, } impl InboundTurnOutcome { @@ -110,6 +116,14 @@ impl InboundTurnOutcome { accepted_message_ref: accepted_message_ref.clone(), active_run_id: *active_run_id, }, + Self::DeferredBusy { + accepted_message_ref, + active_run_id, + .. + } => ProductInboundAck::DeferredBusy { + accepted_message_ref: accepted_message_ref.clone(), + active_run_id: *active_run_id, + }, } } } @@ -195,6 +209,7 @@ pub struct DefaultInboundTurnService { thread_service: T, turn_coordinator: C, inbound_attachments: Option>, + input_enqueue: Arc, } impl DefaultInboundTurnService @@ -209,6 +224,7 @@ where thread_service, turn_coordinator, inbound_attachments: None, + input_enqueue: Arc::new(RejectingInputEnqueue), } } @@ -222,6 +238,11 @@ where self.inbound_attachments = Some(inbound_attachments); self } + + pub fn with_input_enqueue(mut self, input_enqueue: Arc) -> Self { + self.input_enqueue = input_enqueue; + self + } } #[async_trait] @@ -406,6 +427,7 @@ where submit_or_replay_accepted_message( &self.thread_service, &self.turn_coordinator, + self.input_enqueue.as_ref(), replay, prepared.submit_idempotency_key.clone(), envelope.received_at(), @@ -492,7 +514,11 @@ where adapter_id: prepared.adapter_id, surface_type: prepared.surface_type, })) - .submit_or_replay(&self.thread_service, &self.turn_coordinator) + .submit_or_replay( + &self.thread_service, + &self.turn_coordinator, + self.input_enqueue.as_ref(), + ) .await } } @@ -500,6 +526,7 @@ where async fn submit_or_replay_accepted_message( thread_service: &T, turn_coordinator: &C, + input_enqueue: &dyn HostInputEnqueuePort, replay: AcceptedInboundMessageReplay, submit_idempotency_key: String, received_at: DateTime, @@ -515,7 +542,7 @@ where received_at, prepared, )? - .submit_or_replay(thread_service, turn_coordinator) + .submit_or_replay(thread_service, turn_coordinator, input_enqueue) .await } @@ -530,6 +557,11 @@ enum ProductInboundTurnHandoff { binding: ResolvedBinding, active_run_id: Option, }, + AlreadyDeferred { + accepted_message_ref: AcceptedMessageRef, + binding: ResolvedBinding, + active_run_id: TurnRunId, + }, NeedsSubmission(Box), } @@ -620,6 +652,24 @@ impl ProductInboundTurnHandoff { }); } + if replay.status == MessageStatus::Queued { + let Some(turn_run_id) = replay.turn_run_id.as_deref() else { + return Err(ProductWorkflowError::TurnSubmissionRejected { + reason: "queued replay missing turn_run_id".into(), + }); + }; + let active_run_id = Uuid::parse_str(turn_run_id) + .map(TurnRunId::from_uuid) + .map_err(|e| ProductWorkflowError::TurnSubmissionRejected { + reason: format!("invalid queued turn_run_id: {e}"), + })?; + return Ok(Self::AlreadyDeferred { + accepted_message_ref, + binding, + active_run_id, + }); + } + if !matches!( replay.status, MessageStatus::Accepted | MessageStatus::DeferredBusy @@ -662,6 +712,7 @@ impl ProductInboundTurnHandoff { self, thread_service: &T, turn_coordinator: &C, + input_enqueue: &dyn HostInputEnqueuePort, ) -> Result where T: SessionThreadService, @@ -686,8 +737,19 @@ impl ProductInboundTurnHandoff { active_run_id, binding, }), + Self::AlreadyDeferred { + accepted_message_ref, + binding, + active_run_id, + } => Ok(InboundTurnOutcome::DeferredBusy { + accepted_message_ref, + active_run_id, + binding, + }), Self::NeedsSubmission(submission) => { - submission.submit(thread_service, turn_coordinator).await + submission + .submit(thread_service, turn_coordinator, input_enqueue) + .await } } } @@ -710,6 +772,7 @@ impl AcceptedProductInboundTurn { self, thread_service: &T, turn_coordinator: &C, + input_enqueue: &dyn HostInputEnqueuePort, ) -> Result where T: SessionThreadService, @@ -773,7 +836,7 @@ impl AcceptedProductInboundTurn { turn_scope.product_owner(&actor), ); let request = SubmitTurnRequest { - scope: turn_scope, + scope: turn_scope.clone(), actor, accepted_message_ref: accepted_message_ref.clone(), source_binding_ref, @@ -811,23 +874,156 @@ impl AcceptedProductInboundTurn { }) } Err(TurnError::ThreadBusy(busy)) => { - thread_service - .mark_message_rejected_busy(&thread_scope, &binding.thread_id, message_id) - .await - .map_err(|e| ProductWorkflowError::Transient { - reason: format!("failed to mark message rejected: {e}"), - })?; - Ok(InboundTurnOutcome::RejectedBusy { - accepted_message_ref, - active_run_id: Some(busy.active_run_id), - binding, - }) + // Mark the message `Queued` BEFORE the steering input becomes + // drainable so the loop's consumer always observes a `Queued` + // row (deterministic `Queued` → `Submitted`), instead of racing + // a post-enqueue status write. If the enqueue then fails, roll + // the row back to `RejectedBusy`. (`mark_message_queued_or_consumed` + // is lenient — a fast loop that consumed the input first leaves + // the row `Submitted`, which it accepts.) + mark_message_queued_or_consumed( + thread_service, + &thread_scope, + &binding.thread_id, + message_id, + busy.active_run_id, + ) + .await?; + match crate::steering::enqueue_busy_steering( + turn_coordinator, + input_enqueue, + turn_scope, + thread_scope.clone(), + binding.thread_id.clone(), + message_id, + &accepted_message_ref, + busy.active_run_id, + ) + .await + { + Ok(()) => Ok(InboundTurnOutcome::DeferredBusy { + accepted_message_ref, + active_run_id: busy.active_run_id, + binding, + }), + // No input queue wired for this runtime: queueing is + // disabled, so reject the message as busy instead of + // deferring it. Single mapped fallback for the "no steering" + // mode (see RejectingInputEnqueue). This also rolls the + // `Queued` row written above back to `RejectedBusy`. + Err(crate::steering::SteeringEnqueueError::Enqueue( + HostInputQueueError::Unavailable { .. }, + )) => { + thread_service + .mark_message_rejected_busy( + &thread_scope, + &binding.thread_id, + message_id, + ) + .await + .map_err(|e| ProductWorkflowError::Transient { + reason: format!("failed to mark message rejected: {e}"), + })?; + Ok(InboundTurnOutcome::RejectedBusy { + accepted_message_ref, + active_run_id: Some(busy.active_run_id), + binding, + }) + } + // Any other enqueue failure leaves the `Queued` row with no + // drainable input. Best-effort roll it back to `RejectedBusy` + // (preserving the original error) before surfacing it. + Err(other) => { + if let Err(rollback) = thread_service + .mark_message_rejected_busy( + &thread_scope, + &binding.thread_id, + message_id, + ) + .await + { + tracing::warn!( + %rollback, + "failed to roll back queued message to rejected-busy after steering enqueue failure" + ); + } + match other { + crate::steering::SteeringEnqueueError::InvalidMessageRef(reason) => { + Err(ProductWorkflowError::TurnSubmissionRejected { + reason: format!("invalid steering message ref: {reason}"), + }) + } + crate::steering::SteeringEnqueueError::RunState(error) => { + Err(ProductWorkflowError::TurnSubmissionFailed { error }) + } + crate::steering::SteeringEnqueueError::Enqueue(error) => { + Err(ProductWorkflowError::Transient { + reason: format!("failed to enqueue steering input: {error}"), + }) + } + } + } + } } Err(error) => Err(ProductWorkflowError::TurnSubmissionFailed { error }), } } } +async fn mark_message_queued_or_consumed( + thread_service: &T, + thread_scope: &ThreadScope, + thread_id: &ironclaw_host_api::ThreadId, + message_id: ThreadMessageId, + run_id: TurnRunId, +) -> Result<(), ProductWorkflowError> +where + T: SessionThreadService + ?Sized, +{ + let run_id_string = run_id.to_string(); + match thread_service + .mark_message_queued(thread_scope, thread_id, message_id, run_id_string.clone()) + .await + { + Ok(_) => Ok(()), + Err(original_error) => { + let history = match thread_service + .list_thread_history(ThreadHistoryRequest { + scope: thread_scope.clone(), + thread_id: thread_id.clone(), + }) + .await + { + Ok(history) => history, + Err(error) => { + tracing::debug!( + %original_error, + %error, + "queued steering input accepted before transcript queued-status reconciliation failed" + ); + return Ok(()); + } + }; + let already_consumed = history.messages.iter().any(|message| { + message.message_id == message_id + && message.turn_run_id.as_deref() == Some(run_id_string.as_str()) + && matches!( + message.status, + MessageStatus::Queued | MessageStatus::Submitted + ) + }); + if already_consumed { + return Ok(()); + } + tracing::debug!( + %original_error, + "queued steering input accepted before transcript queued-status update failed" + ); + Ok(()) + } + } +} + fn accepted_message_ref( message_id: ThreadMessageId, ) -> Result { @@ -1195,7 +1391,7 @@ mod tests { let thread_service = StubSessionThreadService; handoff - .submit_or_replay(&thread_service, &coordinator) + .submit_or_replay(&thread_service, &coordinator, &RejectingInputEnqueue) .await .expect("submit_or_replay succeeds"); @@ -1459,7 +1655,7 @@ mod tests { let thread_service = StubSessionThreadService; handoff - .submit_or_replay(&thread_service, &coordinator) + .submit_or_replay(&thread_service, &coordinator, &RejectingInputEnqueue) .await .expect("submit_or_replay succeeds"); diff --git a/crates/ironclaw_product_workflow/src/lib.rs b/crates/ironclaw_product_workflow/src/lib.rs index fbc4793c7e1..2513e4b570d 100644 --- a/crates/ironclaw_product_workflow/src/lib.rs +++ b/crates/ironclaw_product_workflow/src/lib.rs @@ -45,6 +45,7 @@ mod lifecycle; mod outbound_delivery; mod policy; mod reborn_services; +mod steering; mod webui_inbound; mod workflow; diff --git a/crates/ironclaw_product_workflow/src/reborn_services.rs b/crates/ironclaw_product_workflow/src/reborn_services.rs index 1f539f19f85..857f090b048 100644 --- a/crates/ironclaw_product_workflow/src/reborn_services.rs +++ b/crates/ironclaw_product_workflow/src/reborn_services.rs @@ -22,6 +22,7 @@ use ironclaw_host_api::{ AgentId, CapabilityId, EffectKind, ExtensionId, GrantConstraints, InvocationId, PermissionMode, Principal, ProjectId, ResourceScope, TenantId, ThreadId, UserId, }; +use ironclaw_loop_support::{HostInputEnqueuePort, HostInputQueueError, RejectingInputEnqueue}; use ironclaw_product_adapters::{ ProductAdapterError, ProductWorkflowRejectionKind, ProjectionStream, ProjectionSubscriptionRequest, @@ -2347,6 +2348,7 @@ pub trait InboundAttachmentReader: Send + Sync { pub struct RebornServices { thread_service: Arc, turn_coordinator: Arc, + input_enqueue: Arc, inbound_attachments: Option>, project_filesystem: Option>, filesystem_browser: Option>, @@ -2379,6 +2381,7 @@ impl RebornServices { Self { thread_service, turn_coordinator, + input_enqueue: Arc::new(RejectingInputEnqueue), inbound_attachments: None, project_filesystem: None, filesystem_browser: None, @@ -2413,6 +2416,11 @@ impl RebornServices { self } + pub fn with_input_enqueue(mut self, input_enqueue: Arc) -> Self { + self.input_enqueue = input_enqueue; + self + } + /// Wire the port that lands inbound attachment bytes into project storage. /// Without it, a send-message carrying attachments is rejected rather than /// silently dropping the files. @@ -2953,6 +2961,25 @@ impl RebornServicesApi for RebornServices { notice: NOTICE_BUSY_GENERIC.to_string(), }); } + MessageStatus::Queued => { + let run_id = parse_replay_run_id(replay.turn_run_id)?; + let state = self + .turn_coordinator + .get_run_state(GetRunStateRequest { + scope: scope.clone(), + run_id, + }) + .await + .map_err(map_turn_error)?; + return Ok(RebornSubmitTurnResponse::DeferredBusy { + thread_id: replay.thread_id, + accepted_message_ref: accepted_message_ref(replay.message_id.to_string())?, + active_run_id: run_id, + status: state.status, + event_cursor: state.event_cursor, + notice: rejected_busy_notice(state.status), + }); + } MessageStatus::Accepted | MessageStatus::DeferredBusy => AcceptedWebUiMessage { thread_id: replay.thread_id, message_id: replay.message_id, @@ -3073,22 +3100,109 @@ impl RebornServicesApi for RebornServices { } Err(TurnError::ThreadBusy(busy)) => { self.clear_skill_activation_message(&scope, &accepted_message_ref)?; - mark_message_rejected_busy_or_replay( + // Mark the message `Queued` BEFORE the steering input becomes + // drainable, so the loop's consumer always observes a `Queued` + // row and performs a deterministic `Queued` → `Submitted` + // transition. Enqueuing first leaves a window where the run can + // drain + submit the input while the transcript still reads + // `Accepted`, producing an out-of-order status write. If the + // enqueue then fails, roll the row back to `RejectedBusy` so it + // never sticks in `Queued` with no backing input. + mark_message_queued_or_replay( &*self.thread_service, &thread_scope, &handoff, &client_action_id, + busy.active_run_id.to_string(), ) .await?; - let notice = rejected_busy_notice(busy.status); - Ok(RebornSubmitTurnResponse::RejectedBusy { - thread_id: handoff.thread_id, - accepted_message_ref, - active_run_id: Some(busy.active_run_id), - status: Some(busy.status), - event_cursor: Some(busy.event_cursor), - notice, - }) + match crate::steering::enqueue_busy_steering( + &*self.turn_coordinator, + self.input_enqueue.as_ref(), + scope.clone(), + thread_scope.clone(), + handoff.thread_id.clone(), + handoff.message_id, + &accepted_message_ref, + busy.active_run_id, + ) + .await + { + Ok(()) => { + let notice = rejected_busy_notice(busy.status); + Ok(RebornSubmitTurnResponse::DeferredBusy { + thread_id: handoff.thread_id, + accepted_message_ref, + active_run_id: busy.active_run_id, + status: busy.status, + event_cursor: busy.event_cursor, + notice, + }) + } + // No input queue wired for this runtime: queueing is + // disabled, so reject the message as busy instead of + // deferring it. This is the single mapped fallback for the + // "no steering" mode (see RejectingInputEnqueue). The + // rejected-busy mark also rolls the `Queued` row written + // above back to its terminal `RejectedBusy` state. + Err(crate::steering::SteeringEnqueueError::Enqueue( + HostInputQueueError::Unavailable { .. }, + )) => { + mark_message_rejected_busy_or_replay( + &*self.thread_service, + &thread_scope, + &handoff, + &client_action_id, + ) + .await?; + let notice = rejected_busy_notice(busy.status); + Ok(RebornSubmitTurnResponse::RejectedBusy { + thread_id: handoff.thread_id, + accepted_message_ref, + active_run_id: Some(busy.active_run_id), + status: Some(busy.status), + event_cursor: Some(busy.event_cursor), + notice, + }) + } + // Any other enqueue failure leaves the `Queued` row above + // with no drainable input. Best-effort roll it back to + // `RejectedBusy` (preserving the original error), then + // surface the sanitized failure. + Err(other) => { + if let Err(rollback) = mark_message_rejected_busy_or_replay( + &*self.thread_service, + &thread_scope, + &handoff, + &client_action_id, + ) + .await + { + tracing::warn!( + %rollback, + "failed to roll back queued message to rejected-busy after steering enqueue failure" + ); + } + match other { + crate::steering::SteeringEnqueueError::InvalidMessageRef(_) => { + Err(RebornServicesError::internal_invariant()) + } + crate::steering::SteeringEnqueueError::RunState(error) => { + Err(map_turn_error(error)) + } + crate::steering::SteeringEnqueueError::Enqueue(error) => { + // Carry the cause to the server log; the + // user-facing surface stays the sanitized 503 + // (error-handling.md). + tracing::warn!( + %error, + "failed to enqueue steering input for busy run" + ); + Err(RebornServicesError::service_unavailable(false)) + } + } + } + } } Err(error) => { self.clear_skill_activation_message(&scope, &accepted_message_ref)?; @@ -4451,6 +4565,51 @@ async fn mark_message_rejected_busy_or_replay( } } +async fn mark_message_queued_or_replay( + thread_service: &dyn SessionThreadService, + thread_scope: &ThreadScope, + handoff: &AcceptedWebUiMessage, + client_action_id: &IdempotencyKey, + run_id: String, +) -> Result<(), RebornServicesError> { + match thread_service + .mark_message_queued( + thread_scope, + &handoff.thread_id, + handoff.message_id, + run_id.clone(), + ) + .await + { + Ok(_) => Ok(()), + Err(error) => { + let reconciled = reconcile_terminal_duplicate( + thread_service, + thread_scope, + handoff, + client_action_id, + |replay| { + matches!( + replay.status, + MessageStatus::Queued | MessageStatus::Submitted + ) && replay.turn_run_id == Some(run_id) + }, + error, + ) + .await; + if let Err(error) = reconciled { + tracing::debug!( + %error, + thread_id = %handoff.thread_id, + message_id = %handoff.message_id, + "queued steering input accepted before transcript queued-status reconciliation failed" + ); + } + Ok(()) + } + } +} + async fn reconcile_terminal_duplicate( thread_service: &dyn SessionThreadService, thread_scope: &ThreadScope, diff --git a/crates/ironclaw_product_workflow/src/reborn_services/types.rs b/crates/ironclaw_product_workflow/src/reborn_services/types.rs index e97cd2a6226..43781838f27 100644 --- a/crates/ironclaw_product_workflow/src/reborn_services/types.rs +++ b/crates/ironclaw_product_workflow/src/reborn_services/types.rs @@ -239,6 +239,14 @@ pub enum RebornSubmitTurnResponse { event_cursor: Option, notice: String, }, + DeferredBusy { + thread_id: ThreadId, + accepted_message_ref: AcceptedMessageRef, + active_run_id: TurnRunId, + status: TurnStatus, + event_cursor: EventCursor, + notice: String, + }, AlreadySubmitted { thread_id: ThreadId, accepted_message_ref: AcceptedMessageRef, diff --git a/crates/ironclaw_product_workflow/src/steering.rs b/crates/ironclaw_product_workflow/src/steering.rs new file mode 100644 index 00000000000..9b42c9abe6c --- /dev/null +++ b/crates/ironclaw_product_workflow/src/steering.rs @@ -0,0 +1,78 @@ +//! Shared busy-run steering enqueue gateway. +//! +//! Both the product inbound-turn path ([`crate::inbound_turn`]) and the WebUI +//! facade ([`crate::reborn_services`]) enqueue a user message as steering input +//! when the target run is busy. This module owns the single enqueue sequence so +//! the two callers cannot drift on ordering, idempotency, or error fidelity. + +use ironclaw_host_api::ThreadId; +use ironclaw_loop_support::{ + EnqueueQueuedMessageRequest, HostInputEnqueuePort, HostInputQueueError, +}; +use ironclaw_threads::{ThreadMessageId, ThreadScope}; +use ironclaw_turns::{ + AcceptedMessageRef, GetRunStateRequest, LoopMessageRef, TurnCoordinator, TurnError, TurnRunId, + TurnScope, run_profile::LoopInput, +}; + +/// Failure surface of [`enqueue_busy_steering`]. +/// +/// Each variant maps to a distinct caller-facing error so neither the inbound +/// path nor the WebUI facade collapses an enqueue failure into a generic, +/// cause-less error. +#[derive(Debug)] +pub(crate) enum SteeringEnqueueError { + /// The accepted message ref could not be re-expressed as a loop message ref. + InvalidMessageRef(String), + /// Reading the active run state failed. + RunState(TurnError), + /// The host input queue rejected the enqueue. + Enqueue(HostInputQueueError), +} + +/// Enqueue `accepted_message_ref` as steering input for the busy `active_run_id`. +/// +/// Resolves the active run's turn id, builds the loop message ref, and hands the +/// queued-message request (carrying the originating thread message identity) to +/// the host input queue. The queue is responsible for transitioning that thread +/// message to `submitted` once the input is consumed; this gateway does not +/// touch transcript status, leaving the queued/replay reconciliation to the +/// caller that owns the message-resolution strategy. +#[allow(clippy::too_many_arguments)] +// arch-exempt: too_many_args, leaf gateway passes through caller-owned scope + +// identity tuple with no natural aggregate type to bundle, plan #5347 +pub(crate) async fn enqueue_busy_steering( + turn_coordinator: &C, + input_enqueue: &dyn HostInputEnqueuePort, + turn_scope: TurnScope, + thread_scope: ThreadScope, + thread_id: ThreadId, + message_id: ThreadMessageId, + accepted_message_ref: &AcceptedMessageRef, + active_run_id: TurnRunId, +) -> Result<(), SteeringEnqueueError> +where + C: TurnCoordinator + ?Sized, +{ + let active_run = turn_coordinator + .get_run_state(GetRunStateRequest { + scope: turn_scope, + run_id: active_run_id, + }) + .await + .map_err(SteeringEnqueueError::RunState)?; + let message_ref = LoopMessageRef::new(accepted_message_ref.as_str().to_string()) + .map_err(|e| SteeringEnqueueError::InvalidMessageRef(e.to_string()))?; + input_enqueue + .enqueue_queued_message(EnqueueQueuedMessageRequest { + run_id: active_run_id, + turn_id: active_run.turn_id, + scope: thread_scope, + thread_id, + message_id, + input: LoopInput::Steering { message_ref }, + }) + .await + .map_err(SteeringEnqueueError::Enqueue)?; + Ok(()) +} diff --git a/crates/ironclaw_product_workflow/tests/inbound_turn_contract.rs b/crates/ironclaw_product_workflow/tests/inbound_turn_contract.rs index 11483875d10..35127029105 100644 --- a/crates/ironclaw_product_workflow/tests/inbound_turn_contract.rs +++ b/crates/ironclaw_product_workflow/tests/inbound_turn_contract.rs @@ -11,11 +11,11 @@ use ironclaw_loop_support::{ CapabilityAllowSet, CapabilityResolveError, CapabilityResultWrite, CapabilitySurfaceProfileResolver, CapabilityWriteResult, EmptyLoopCapabilityPort, EmptyUserProfileSource, HostIdentityContextBuildError, HostIdentityContextCandidate, - HostIdentityContextSource, HostInputBatch, HostInputQueue, HostInputQueueError, - HostManagedModelError, HostManagedModelGateway, HostManagedModelRequest, - HostManagedModelResponse, JsonSpawnSubagentInputCodec, LoopCapabilityPortFactory, - LoopCapabilityResultWriter, ProductLiveCancellationProbe, RunCancellationFactory, - RunCancellationHandle, + HostIdentityContextSource, HostInputBatch, HostInputEnqueuePort, HostInputEnvelope, + HostInputQueue, HostInputQueueError, HostManagedModelError, HostManagedModelGateway, + HostManagedModelRequest, HostManagedModelResponse, InMemoryHostInputQueue, + JsonSpawnSubagentInputCodec, LoopCapabilityPortFactory, LoopCapabilityResultWriter, + ProductLiveCancellationProbe, RunCancellationFactory, RunCancellationHandle, }; use ironclaw_product_adapters::{ AdapterInstallationId, AuthRequirement, ExternalActorRef, ExternalConversationRef, @@ -44,16 +44,16 @@ use ironclaw_threads::{ ThreadScope, }; use ironclaw_turns::{ - CancelRunRequest, CancelRunResponse, DefaultTurnCoordinator, EventCursor, GetRunStateRequest, - IdempotencyKey, InMemoryCheckpointStateStore, InMemoryLoopCheckpointStore, - InMemoryTurnStateStore, ResumeTurnRequest, ResumeTurnResponse, RunProfileId, RunProfileVersion, - SanitizedCancelReason, SubmitTurnRequest, SubmitTurnResponse, ThreadBusy, TurnActor, - TurnCoordinator, TurnError, TurnId, TurnOriginKind, TurnRunId, TurnRunState, TurnRunWake, - TurnScope, TurnStateStore, TurnStatus, + AcceptedMessageRef, CancelRunRequest, CancelRunResponse, DefaultTurnCoordinator, EventCursor, + GetRunStateRequest, IdempotencyKey, InMemoryCheckpointStateStore, InMemoryLoopCheckpointStore, + InMemoryTurnStateStore, ReplyTargetBindingRef, ResumeTurnRequest, ResumeTurnResponse, + RunProfileId, RunProfileVersion, SanitizedCancelReason, SourceBindingRef, SubmitTurnRequest, + SubmitTurnResponse, ThreadBusy, TurnActor, TurnCoordinator, TurnError, TurnId, TurnOriginKind, + TurnRunId, TurnRunState, TurnRunWake, TurnScope, TurnStateStore, TurnStatus, run_profile::{ AgentLoopHostError, InMemoryLoopHostMilestoneSink, InstructionSafetyContext, - LoopCancelReasonKind, LoopCapabilityPort, LoopInputAckToken, LoopInputCursorToken, - LoopRunContext, NoOpBudgetAccountant, NoOpPolicyGuard, PromptMode, + LoopCancelReasonKind, LoopCapabilityPort, LoopInput, LoopInputAckToken, + LoopInputCursorToken, LoopRunContext, NoOpBudgetAccountant, NoOpPolicyGuard, PromptMode, }, }; use tokio::time::{sleep, timeout}; @@ -176,8 +176,32 @@ impl TurnCoordinator for ScriptedTurnCoordinator { panic!("cancel_run is not used by inbound turn contract tests") } - async fn get_run_state(&self, _request: GetRunStateRequest) -> Result { - panic!("get_run_state is not used by inbound turn contract tests") + async fn get_run_state(&self, request: GetRunStateRequest) -> Result { + // The busy-steering gateway resolves the active run's turn id before + // enqueuing. Return a minimal running-state for the requested run so the + // no-queue rejection path can exercise the full gateway. + Ok(TurnRunState { + scope: request.scope, + actor: None, + turn_id: TurnId::new(), + run_id: request.run_id, + status: TurnStatus::Running, + accepted_message_ref: AcceptedMessageRef::new("msg:scripted").expect("valid"), + source_binding_ref: SourceBindingRef::new("src:scripted").expect("valid"), + reply_target_binding_ref: ReplyTargetBindingRef::new("reply:scripted").expect("valid"), + resolved_run_profile_id: RunProfileId::default_profile(), + resolved_run_profile_version: RunProfileVersion::new(1), + resolved_model_route: None, + received_at: Utc::now(), + checkpoint_id: None, + gate_ref: None, + blocked_activity_id: None, + credential_requirements: Vec::new(), + failure: None, + event_cursor: EventCursor::default(), + product_context: None, + resume_disposition: None, + }) } } @@ -1098,6 +1122,360 @@ async fn busy_thread_persists_second_message_as_rejected_busy() { assert_eq!(history.messages[1].status, MessageStatus::RejectedBusy); } +#[tokio::test] +async fn busy_thread_with_input_queue_defers_second_message_until_queue_ack() { + let binding_service = FakeConversationBindingService::new(); + let thread_service = InMemorySessionThreadService::default(); + let store = Arc::new(InMemoryTurnStateStore::default()); + let coordinator = DefaultTurnCoordinator::new(store); + let input_queue = Arc::new(InMemoryHostInputQueue::new( + Arc::new(thread_service.clone()) as Arc, + )); + let input_enqueue: Arc = input_queue.clone(); + let service = + DefaultInboundTurnService::new(binding_service, thread_service.clone(), coordinator) + .with_input_enqueue(input_enqueue); + + let first = sample_user_message_envelope("queue-busy1"); + service.accept_user_message(&first).await.expect("first"); + let second = sample_user_message_envelope_with_text("queue-busy2", "queued second"); + let outcome = service + .accept_user_message(&second) + .await + .expect("second queued"); + + let (binding, active_run_id) = match outcome { + InboundTurnOutcome::DeferredBusy { + binding, + active_run_id, + .. + } => (binding, active_run_id), + other => panic!("expected DeferredBusy, got {other:?}"), + }; + let scope = ThreadScope { + tenant_id: binding.tenant_id.clone(), + agent_id: binding.agent_id.clone().expect("agent id"), + project_id: binding.project_id.clone(), + owner_user_id: binding.subject_user_id.clone(), + mission_id: None, + }; + let history = thread_service + .list_thread_history(ThreadHistoryRequest { + scope: scope.clone(), + thread_id: binding.thread_id.clone(), + }) + .await + .expect("history"); + assert_eq!(history.messages.len(), 2); + assert_eq!( + history.messages[1].content.as_deref(), + Some("queued second") + ); + assert_eq!(history.messages[1].status, MessageStatus::Queued); + assert_eq!( + history.messages[1].turn_run_id.as_deref(), + Some(active_run_id.to_string().as_str()) + ); + + let batch = input_queue + .next_after( + active_run_id, + LoopInputCursorToken::new("input-cursor:origin".to_string()).expect("origin cursor"), + 8, + ) + .await + .expect("queued input batch"); + assert_eq!(batch.inputs.len(), 1); + assert!(matches!(batch.inputs[0].input, LoopInput::Steering { .. })); + + input_queue + .ack_consumed(active_run_id, vec![batch.inputs[0].ack_token.clone()]) + .await + .expect("ack queued input"); + let history = thread_service + .list_thread_history(ThreadHistoryRequest { + scope, + thread_id: binding.thread_id.clone(), + }) + .await + .expect("history after ack"); + assert_eq!(history.messages[1].status, MessageStatus::Submitted); + assert_eq!( + history.messages[1].turn_run_id.as_deref(), + Some(active_run_id.to_string().as_str()) + ); +} + +struct AckingInputEnqueue { + inner: InMemoryHostInputQueue, +} + +impl AckingInputEnqueue { + fn new(thread_service: Arc) -> Self { + Self { + inner: InMemoryHostInputQueue::new(thread_service), + } + } +} + +#[async_trait] +impl HostInputEnqueuePort for AckingInputEnqueue { + async fn enqueue_queued_message( + &self, + request: ironclaw_loop_support::EnqueueQueuedMessageRequest, + ) -> Result { + let run_id = request.run_id; + let envelope = self.inner.enqueue_queued_message(request).await?; + self.inner + .ack_consumed(run_id, vec![envelope.ack_token.clone()]) + .await?; + Ok(envelope) + } +} + +#[tokio::test] +async fn busy_thread_queue_submit_tolerates_input_ack_before_queued_mark() { + let binding_service = FakeConversationBindingService::new(); + let thread_service = InMemorySessionThreadService::default(); + let store = Arc::new(InMemoryTurnStateStore::default()); + let coordinator = DefaultTurnCoordinator::new(store); + let input_enqueue: Arc = + Arc::new(AckingInputEnqueue::new(Arc::new(thread_service.clone()))); + let service = + DefaultInboundTurnService::new(binding_service, thread_service.clone(), coordinator) + .with_input_enqueue(input_enqueue); + + let first = sample_user_message_envelope("queue-race1"); + service.accept_user_message(&first).await.expect("first"); + let second = sample_user_message_envelope_with_text("queue-race2", "queued then acked"); + let outcome = service + .accept_user_message(&second) + .await + .expect("second should not fail when input is acked before queued mark"); + + let (binding, active_run_id) = match outcome { + InboundTurnOutcome::DeferredBusy { + binding, + active_run_id, + .. + } => (binding, active_run_id), + other => panic!("expected DeferredBusy, got {other:?}"), + }; + let scope = ThreadScope { + tenant_id: binding.tenant_id.clone(), + agent_id: binding.agent_id.clone().expect("agent id"), + project_id: binding.project_id.clone(), + owner_user_id: binding.subject_user_id.clone(), + mission_id: None, + }; + let history = thread_service + .list_thread_history(ThreadHistoryRequest { + scope, + thread_id: binding.thread_id, + }) + .await + .expect("history"); + assert_eq!(history.messages.len(), 2); + assert_eq!(history.messages[1].status, MessageStatus::Submitted); + assert_eq!( + history.messages[1].turn_run_id.as_deref(), + Some(active_run_id.to_string().as_str()) + ); +} + +#[tokio::test] +async fn ack_consumed_is_non_fatal_when_queued_status_flip_fails() { + // Regression for the "The run could not start the agent runtime." + // (driver_unavailable) run-kill: a queued message's status flip + // (`Queued` → `Submitted`) failing inside `ack_consumed` used to surface as + // `ThreadStatusUpdate` → `AgentLoopHostErrorKind::Unavailable` → a terminal + // `HostUnavailable` that killed the whole run — even though the input had + // already been drained and delivered to the model. The status write is + // best-effort bookkeeping; its failure must be swallowed and the ack must + // still advance. + let thread_service: Arc = + Arc::new(InMemorySessionThreadService::default()); + let queue = InMemoryHostInputQueue::new(thread_service); + let run_id = TurnRunId::new(); + + // Enqueue a queued message whose backing thread/message does not exist, so + // the in-queue `mark_message_submitted` is guaranteed to fail. + let envelope = queue + .enqueue_queued_message(ironclaw_loop_support::EnqueueQueuedMessageRequest { + run_id, + turn_id: TurnId::new(), + scope: ThreadScope { + tenant_id: TenantId::new("ghost-tenant").unwrap(), + agent_id: AgentId::new("ghost-agent").unwrap(), + project_id: None, + owner_user_id: None, + mission_id: None, + }, + thread_id: ThreadId::new("ghost-thread").unwrap(), + message_id: ironclaw_threads::ThreadMessageId::new(), + input: LoopInput::Steering { + message_ref: ironclaw_turns::LoopMessageRef::new("msg:ghost").unwrap(), + }, + }) + .await + .expect("enqueue"); + + // The status flip fails internally, but the ack itself must succeed... + queue + .ack_consumed(run_id, vec![envelope.ack_token.clone()]) + .await + .expect("ack_consumed must not fail when the queued-message status flip fails"); + + // ...and the token must be recorded as consumed so the input is not + // redelivered (which would loop the model on the same steering input). + let batch = queue + .next_after( + run_id, + LoopInputCursorToken::new("input-cursor:origin".to_string()).unwrap(), + 8, + ) + .await + .expect("next_after"); + assert!( + batch.inputs.is_empty(), + "acked input must not be redelivered after a swallowed status-update failure" + ); +} + +#[tokio::test] +async fn ack_rejects_unknown_token_instead_of_poisoning_state() { + // An ack token that matches no live entry and is not already acked must fail + // loud, not be silently committed into `acked`. Committing it would poison + // the queue: a later entry minted for that same sequence would be skipped + // forever by `next_after`. + let thread_service: Arc = + Arc::new(InMemorySessionThreadService::default()); + let queue = InMemoryHostInputQueue::new(thread_service); + let run_id = TurnRunId::new(); + + // Create the run's queue with a single live entry. + queue + .enqueue_queued_message(ironclaw_loop_support::EnqueueQueuedMessageRequest { + run_id, + turn_id: TurnId::new(), + scope: ThreadScope { + tenant_id: TenantId::new("ghost-tenant").unwrap(), + agent_id: AgentId::new("ghost-agent").unwrap(), + project_id: None, + owner_user_id: None, + mission_id: None, + }, + thread_id: ThreadId::new("ghost-thread").unwrap(), + message_id: ironclaw_threads::ThreadMessageId::new(), + input: LoopInput::Steering { + message_ref: ironclaw_turns::LoopMessageRef::new("msg:live").unwrap(), + }, + }) + .await + .expect("enqueue"); + + // Ack a forged token for an input that was never enqueued. + let forged = LoopInputAckToken::new("input-ack:999".to_string()).unwrap(); + let result = queue.ack_consumed(run_id, vec![forged]).await; + assert!( + matches!(result, Err(HostInputQueueError::InvalidCursor { .. })), + "unknown ack token must be rejected, got {result:?}" + ); + + // The live entry is untouched and still deliverable. + let batch = queue + .next_after( + run_id, + LoopInputCursorToken::new("input-cursor:origin".to_string()).unwrap(), + 8, + ) + .await + .expect("next_after"); + assert_eq!( + batch.inputs.len(), + 1, + "the live entry remains deliverable after a rejected forged ack" + ); +} + +/// Records the transcript status of the queued message at the moment its +/// steering input becomes drainable (i.e. when `enqueue_queued_message` runs). +struct StatusObservingInputEnqueue { + inner: InMemoryHostInputQueue, + thread_service: Arc, + observed_status: Arc>>, +} + +impl StatusObservingInputEnqueue { + fn new(thread_service: Arc) -> Self { + Self { + inner: InMemoryHostInputQueue::new(thread_service.clone()), + thread_service, + observed_status: Arc::new(Mutex::new(None)), + } + } +} + +#[async_trait] +impl HostInputEnqueuePort for StatusObservingInputEnqueue { + async fn enqueue_queued_message( + &self, + request: ironclaw_loop_support::EnqueueQueuedMessageRequest, + ) -> Result { + let history = self + .thread_service + .list_thread_history(ThreadHistoryRequest { + scope: request.scope.clone(), + thread_id: request.thread_id.clone(), + }) + .await + .expect("history at enqueue time"); + let status = history + .messages + .iter() + .find(|message| message.message_id == request.message_id) + .map(|message| message.status); + *self.observed_status.lock().unwrap() = status; + self.inner.enqueue_queued_message(request).await + } +} + +#[tokio::test] +async fn busy_submit_marks_message_queued_before_input_is_drainable() { + // Layer 2 ordering guard: the busy-submit path must mark the message + // `Queued` BEFORE the steering input is enqueued (and thus drainable), so a + // fast loop never observes the message in `Accepted` while draining it. With + // the previous enqueue-first ordering the observed status here was + // `Accepted`. + let binding_service = FakeConversationBindingService::new(); + let thread_service = InMemorySessionThreadService::default(); + let store = Arc::new(InMemoryTurnStateStore::default()); + let coordinator = DefaultTurnCoordinator::new(store); + let enqueue = Arc::new(StatusObservingInputEnqueue::new( + Arc::new(thread_service.clone()) as Arc, + )); + let observed = enqueue.observed_status.clone(); + let input_enqueue: Arc = enqueue; + let service = + DefaultInboundTurnService::new(binding_service, thread_service.clone(), coordinator) + .with_input_enqueue(input_enqueue); + + let first = sample_user_message_envelope("queue-order1"); + service.accept_user_message(&first).await.expect("first"); + let second = sample_user_message_envelope_with_text("queue-order2", "ordered second"); + let outcome = service + .accept_user_message(&second) + .await + .expect("second deferred"); + assert!(matches!(outcome, InboundTurnOutcome::DeferredBusy { .. })); + + assert_eq!( + *observed.lock().unwrap(), + Some(MessageStatus::Queued), + "message must be Queued before the steering input becomes drainable" + ); +} + #[tokio::test] async fn retry_validates_live_binding_before_accepted_message_replay() { let binding_service = FakeConversationBindingService::new(); diff --git a/crates/ironclaw_product_workflow/tests/product_workflow_contract.rs b/crates/ironclaw_product_workflow/tests/product_workflow_contract.rs index d291ee9eb27..5cfb0884885 100644 --- a/crates/ironclaw_product_workflow/tests/product_workflow_contract.rs +++ b/crates/ironclaw_product_workflow/tests/product_workflow_contract.rs @@ -47,9 +47,10 @@ use ironclaw_product_workflow::{ use ironclaw_threads::InMemorySessionThreadService; use ironclaw_turns::{ AcceptedMessageRef, CancelRunRequest, CancelRunResponse, EventCursor, GateRef, - GetRunStateRequest, LoopGateRef, ResumeTurnRequest, ResumeTurnResponse, RunProfileId, - RunProfileVersion, SubmitTurnRequest, SubmitTurnResponse, ThreadBusy, TurnActor, - TurnCoordinator, TurnError, TurnId, TurnRunId, TurnRunState, TurnScope, TurnStatus, + GetRunStateRequest, LoopGateRef, ReplyTargetBindingRef, ResumeTurnRequest, ResumeTurnResponse, + RunProfileId, RunProfileVersion, SourceBindingRef, SubmitTurnRequest, SubmitTurnResponse, + ThreadBusy, TurnActor, TurnCoordinator, TurnError, TurnId, TurnRunId, TurnRunState, TurnScope, + TurnStatus, }; fn sample_envelope(event_suffix: &str) -> ProductInboundEnvelope { @@ -171,8 +172,36 @@ impl TurnCoordinator for RecordingTurnCoordinator { panic!("cancel_run is not used by product workflow contract tests") } - async fn get_run_state(&self, _request: GetRunStateRequest) -> Result { - panic!("get_run_state is not used by product workflow contract tests") + async fn get_run_state(&self, request: GetRunStateRequest) -> Result { + // The busy-run steering enqueue path (`steering::enqueue_busy_steering`) + // reads the active run's state to recover its turn id before queueing. + // Return a minimal running-run state keyed to the requested scope/run so + // the enqueue step is exercised rather than panicking here. + Ok(TurnRunState { + scope: request.scope, + actor: None, + turn_id: TurnId::new(), + run_id: request.run_id, + status: TurnStatus::Running, + accepted_message_ref: AcceptedMessageRef::new("msg:active-run") + .expect("accepted message ref"), + source_binding_ref: SourceBindingRef::new("src:active-run") + .expect("source binding ref"), + reply_target_binding_ref: ReplyTargetBindingRef::new("reply:active-run") + .expect("reply target binding ref"), + resolved_run_profile_id: RunProfileId::default_profile(), + resolved_run_profile_version: RunProfileVersion::new(1), + resolved_model_route: None, + received_at: Utc::now(), + checkpoint_id: None, + gate_ref: None, + blocked_activity_id: None, + credential_requirements: Vec::new(), + failure: None, + event_cursor: EventCursor::default(), + product_context: None, + resume_disposition: None, + }) } } diff --git a/crates/ironclaw_product_workflow/tests/reborn_services_contract.rs b/crates/ironclaw_product_workflow/tests/reborn_services_contract.rs index c268aed6fc5..04438a98900 100644 --- a/crates/ironclaw_product_workflow/tests/reborn_services_contract.rs +++ b/crates/ironclaw_product_workflow/tests/reborn_services_contract.rs @@ -2939,14 +2939,16 @@ async fn concurrent_duplicate_submit_creates_one_message_and_replays_outcome() { let first_run_id = match &first { RebornSubmitTurnResponse::Submitted { run_id, .. } | RebornSubmitTurnResponse::AlreadySubmitted { run_id, .. } => *run_id, - RebornSubmitTurnResponse::RejectedBusy { .. } => { + RebornSubmitTurnResponse::RejectedBusy { .. } + | RebornSubmitTurnResponse::DeferredBusy { .. } => { panic!("duplicate submit must not defer while deduping") } }; let second_run_id = match &second { RebornSubmitTurnResponse::Submitted { run_id, .. } | RebornSubmitTurnResponse::AlreadySubmitted { run_id, .. } => *run_id, - RebornSubmitTurnResponse::RejectedBusy { .. } => { + RebornSubmitTurnResponse::RejectedBusy { .. } + | RebornSubmitTurnResponse::DeferredBusy { .. } => { panic!("duplicate submit must not defer while deduping") } }; diff --git a/crates/ironclaw_reborn/tests/planned_driver_e2e.rs b/crates/ironclaw_reborn/tests/planned_driver_e2e.rs index 6425d50af6d..72384adbacf 100644 --- a/crates/ironclaw_reborn/tests/planned_driver_e2e.rs +++ b/crates/ironclaw_reborn/tests/planned_driver_e2e.rs @@ -7,8 +7,8 @@ use chrono::Utc; use ironclaw_agent_loop::{ state::CheckpointKind, test_support::{ - MockAgentLoopDriverHost, MockHostCall, ScenarioScript, ScriptedModelResponse, - test_run_context, + MockAgentLoopDriverHost, MockHostCall, ScenarioScript, ScriptedCapabilityCall, + ScriptedCapabilityOutcome, ScriptedModelResponse, test_run_context, }, }; use ironclaw_reborn::app_loop_family::build_loop_family_registry; @@ -283,6 +283,94 @@ async fn planned_driver_consumes_steering_message_before_model_call() { ); } +#[tokio::test] +async fn planned_driver_includes_post_tool_steering_before_reply_model_call() { + let registry = build_loop_family_registry().expect("registry should build"); + let driver = PlannedDriver::default_from_registry(®istry).expect("driver should build"); + let script = ScenarioScript { + model_responses: VecDeque::from([ + ScriptedModelResponse::Calls(vec![ScriptedCapabilityCall::new("demo.echo")]), + ScriptedModelResponse::Reply { + text: "done".to_string(), + }, + ]), + capability_outcomes: VecDeque::from([vec![ScriptedCapabilityOutcome::completed( + "result:done", + )]]), + single_call_retry_outcomes: VecDeque::new(), + pending_inputs: VecDeque::from([ + Vec::new(), + vec![LoopInput::Steering { + message_ref: LoopMessageRef::new("msg:download-csv").unwrap(), + }], + Vec::new(), + ]), + }; + let (host, _) = MockAgentLoopDriverHost::builder().script(script).build(); + + let exit = driver + .run(run_request(&driver, &host), &host) + .await + .expect("planned driver run should succeed"); + + assert!(matches!(exit, LoopExit::Completed(_))); + assert_eq!(host.model_call_count(), 2); + + let prompt_requests = host.prompt_requests(); + assert_eq!(prompt_requests.len(), 2); + let reply_prompt_cursor = prompt_requests[1] + .context_cursor + .as_ref() + .expect("reply prompt must be scoped to the pre-drain steering cursor"); + assert_eq!( + reply_prompt_cursor, + &LoopInputCursor::origin_for_run(host.run_context()), + "reply prompt must use the pre-drain cursor so the drained steering input remains visible" + ); + + let calls = host.call_log(); + let poll_inputs = calls + .iter() + .enumerate() + .filter_map(|(index, call)| matches!(call, MockHostCall::PollInputs).then_some(index)) + .collect::>(); + let prompt_builds = calls + .iter() + .enumerate() + .filter_map(|(index, call)| { + matches!(call, MockHostCall::BuildPromptBundle).then_some(index) + }) + .collect::>(); + let model_calls = calls + .iter() + .enumerate() + .filter_map(|(index, call)| matches!(call, MockHostCall::StreamModel).then_some(index)) + .collect::>(); + let ack_inputs = calls + .iter() + .position(|call| matches!(call, MockHostCall::AckInputs)) + .expect("drained post-tool steering should be acknowledged"); + + let second_poll = poll_inputs[1]; + let second_prompt = prompt_builds[1]; + let second_model = model_calls[1]; + assert!( + calls.iter().enumerate().any(|(index, call)| { + second_poll < index + && index < ack_inputs + && matches!( + call, + MockHostCall::SaveCheckpoint(CheckpointKind::BeforeModel) + ) + }), + "post-tool steering cursor must be checkpointed before ack" + ); + assert!( + second_poll < ack_inputs && ack_inputs < second_prompt && second_prompt < second_model, + "post-tool steering must be picked up before building the reply prompt" + ); +} + #[tokio::test] async fn planned_driver_followup_restarts_after_natural_stop() { let registry = build_loop_family_registry().expect("registry should build"); diff --git a/crates/ironclaw_reborn_composition/src/openai_compat_serve.rs b/crates/ironclaw_reborn_composition/src/openai_compat_serve.rs index 9388d338d18..e323d2083b5 100644 --- a/crates/ironclaw_reborn_composition/src/openai_compat_serve.rs +++ b/crates/ironclaw_reborn_composition/src/openai_compat_serve.rs @@ -123,7 +123,8 @@ pub async fn build_openai_compat_route_mount( binding.clone(), runtime.webui_thread_service(), runtime.webui_turn_coordinator(), - ); + ) + .with_input_enqueue(runtime.webui_input_enqueue()); // Lands inline image bytes (vision, #4644) through the same project-scoped // workspace authority the agent's file tools resolve through, so an image // attached to an OpenAI-compatible chat completion reaches the model. diff --git a/crates/ironclaw_reborn_composition/src/runtime.rs b/crates/ironclaw_reborn_composition/src/runtime.rs index 44e9a94cdd8..ea8e1837d61 100644 --- a/crates/ironclaw_reborn_composition/src/runtime.rs +++ b/crates/ironclaw_reborn_composition/src/runtime.rs @@ -480,6 +480,7 @@ pub struct RebornRuntime { turn_coordinator: Arc, turn_tree_store: Arc, thread_service: Arc, + input_enqueue: Arc, thread_scope: ThreadScope, turn_scheduler: RuntimeTurnScheduler, trigger_poller_handle: Option, @@ -1256,6 +1257,12 @@ impl RebornRuntime { self.turn_coordinator.clone() } + pub(crate) fn webui_input_enqueue( + &self, + ) -> Arc { + Arc::clone(&self.input_enqueue) + } + #[cfg(feature = "slack-v2-host-beta")] pub(crate) fn auth_challenge_provider(&self) -> Option> { self.services @@ -3073,6 +3080,61 @@ pub async fn build_reborn_runtime( _ => None, }; + // Steering/followup input queue. When a local runtime is composed (local-dev + // with libSQL/Postgres), it carries a durable run-scoped filesystem, so use + // the durable `FilesystemHostInputQueue`: a message queued while a run is + // busy then survives a daemon restart — the scheduler re-claims the run from + // its checkpoint and drains the persisted input. + // + // DEGRADATION (not silent): the production-graph path (`local_runtime: None`) + // does not expose a durable filesystem handle in this composition facade + // yet, so it falls back to the in-memory queue even under the + // libSQL/Postgres build. In-memory queuing still delivers follow-ups within + // a single daemon lifetime; only messages queued-but-undrained across a + // restart are lost. This mirrors the same `Some(local_runtime) => real / + // None => degraded` deferral the identity/profile sources use below, and is + // owned by the production durable-composition follow-up (host + // runtime/event-store wiring; see the production_runtime_parts note and + // #5013). Failing closed here is deliberately NOT done — it would reject all + // busy-thread follow-ups in production rather than degrade gracefully. + let (host_input_queue_reader, host_input_enqueue): ( + Arc, + Arc, + ) = { + #[cfg(any(feature = "libsql", feature = "postgres"))] + { + if let Some(local_runtime) = local_runtime { + let owner_scope = ResourceScope { + tenant_id: thread_scope.tenant_id.clone(), + user_id: actor_user_id.clone(), + agent_id: Some(thread_scope.agent_id.clone()), + project_id: thread_scope.project_id.clone(), + mission_id: None, + thread_id: None, + invocation_id: InvocationId::new(), + }; + let durable = Arc::new(ironclaw_loop_support::FilesystemHostInputQueue::new( + Arc::clone(&local_runtime.subagent_goal_filesystem), + owner_scope, + Arc::clone(&thread_service), + )); + (durable.clone(), durable) + } else { + let queue = Arc::new(ironclaw_loop_support::InMemoryHostInputQueue::new( + Arc::clone(&thread_service), + )); + (queue.clone(), queue) + } + } + #[cfg(not(any(feature = "libsql", feature = "postgres")))] + { + let queue = Arc::new(ironclaw_loop_support::InMemoryHostInputQueue::new( + Arc::clone(&thread_service), + )); + (queue.clone(), queue) + } + }; + let planned_runtime_parts = DefaultPlannedRuntimeParts { turn_state: Arc::clone(&turn_state_store), thread_service: Arc::clone(&thread_service), @@ -3117,7 +3179,7 @@ pub async fn build_reborn_runtime( model_route_resolver: None, cancellation_factory: None, skill_context_source, - input_queue: None, + input_queue: Some(host_input_queue_reader), identity_context_source: match local_runtime { Some(local_runtime) => Arc::new( // Local-dev seeding validates the prompt path first, so non-file prompt paths fail @@ -3416,6 +3478,7 @@ pub async fn build_reborn_runtime( turn_coordinator, turn_tree_store: turn_state_store, thread_service, + input_enqueue: host_input_enqueue, thread_scope, turn_scheduler: RuntimeTurnScheduler::new(composition.scheduler_handle, scheduler_notifier), trigger_poller_handle, @@ -9644,16 +9707,16 @@ output_schema_ref = "schemas/write.output.json" runtime.shutdown().await.expect("runtime shutdown"); } - /// Regression guard: a message that arrives while the thread is busy is stored with - /// `RejectedBusy` status and must NOT be auto-resubmitted when the blocking run - /// reaches a terminal state. + /// Regression guard: a WebUI message that arrives while the thread is busy is queued + /// into the active run and must NOT be submitted as a separate run when the blocking + /// run reaches a terminal state. /// /// Scenario: /// A – submitted via `turn_coordinator.submit_turn`; worker is stopped so it stays /// Queued and holds the active-lock. /// B – submitted via `bundle.api.submit_turn` (WebUI path); thread is busy → stored - /// as `RejectedBusy`; response carries a non-empty `notice`. - /// Cancel A → B stays `RejectedBusy` (no auto-resubmission). + /// as `Queued`; response is `DeferredBusy`. + /// Cancel A → B stays queued/consumed by A and is not submitted as a separate run. /// C – submitted after A is cancelled; thread is free → `Submitted`. /// /// arch-note: lives in runtime.rs (adds ~200 lines to an already >3000-line file) because @@ -9661,7 +9724,7 @@ output_schema_ref = "schemas/write.output.json" /// harness provides; moving it would require duplicating that harness. Decomposition of /// runtime.rs is tracked in plan #4471. #[tokio::test] - async fn rejected_busy_message_not_auto_resubmitted_after_run_cancellation() { + async fn deferred_busy_message_not_auto_submitted_after_run_cancellation() { let root = tempfile::tempdir().expect("tempdir"); let gateway = Arc::new(RecordingGateway { reply: "busy-drain ok".to_string(), @@ -9737,7 +9800,7 @@ output_schema_ref = "schemas/write.output.json" run_id: run_id_a, .. } = submitted_a; - // Submit message B through the WebUI path — thread is busy, must get RejectedBusy. + // Submit message B through the WebUI path — thread is busy, must get queued. let response_b = bundle .api .submit_turn( @@ -9752,25 +9815,30 @@ output_schema_ref = "schemas/write.output.json" .await .expect("message B submit should not error"); - let RebornSubmitTurnResponse::RejectedBusy { + let RebornSubmitTurnResponse::DeferredBusy { notice: notice_b, active_run_id: busy_run_id, + status: status_b, .. } = response_b else { - panic!("expected RejectedBusy for message B, got {response_b:?}"); + panic!("expected DeferredBusy for message B, got {response_b:?}"); }; assert_eq!( - busy_run_id, - Some(run_id_a), - "RejectedBusy should report run A as the active run" + busy_run_id, run_id_a, + "DeferredBusy should report run A as the active run" + ); + assert_eq!( + status_b, + TurnStatus::Queued, + "message B should be queued into run A" ); assert!( !notice_b.is_empty(), - "RejectedBusy response must carry a non-empty notice" + "DeferredBusy response must carry a non-empty notice" ); - // Verify message B is stored with RejectedBusy status. + // Verify message B is stored with queued status. let history = runtime .thread_service .list_thread_history(ThreadHistoryRequest { @@ -9779,23 +9847,23 @@ output_schema_ref = "schemas/write.output.json" }) .await .expect("thread history after B"); - let rejected_messages: Vec<_> = history + let queued_messages: Vec<_> = history .messages .iter() - .filter(|m| matches!(m.status, MessageStatus::RejectedBusy)) + .filter(|m| matches!(m.status, MessageStatus::Queued)) .collect(); assert_eq!( - rejected_messages.len(), + queued_messages.len(), 1, - "exactly one message should be stored as RejectedBusy after thread-busy submit" + "exactly one message should be stored as queued after thread-busy submit" ); assert_eq!( - rejected_messages[0].kind, + queued_messages[0].kind, MessageKind::User, - "the RejectedBusy message must be of kind User" + "the queued message must be of kind User" ); - // Cancel run A — this is the terminal event that (must NOT) auto-resubmit B. + // Cancel run A — this is the terminal event that must not submit B as a separate run. runtime .cancel_run( &scope, @@ -9806,7 +9874,7 @@ output_schema_ref = "schemas/write.output.json" .await .expect("run A cancellation succeeds"); - // B must remain RejectedBusy — no auto-resubmission should have fired. + // B must remain the same row and no auto-resubmission should have fired. let history_after_cancel = runtime .thread_service .list_thread_history(ThreadHistoryRequest { @@ -9816,10 +9884,10 @@ output_schema_ref = "schemas/write.output.json" .await .expect("thread history after cancel"); // Identify message B by the message_id we captured from the pre-cancel history. - // Using the stable message_id (rather than a simple RejectedBusy count) ensures - // a regression that leaves the RejectedBusy row AND adds a Submitted row for the - // same message cannot slip past as "still one RejectedBusy". - let msg_b_id = rejected_messages[0].message_id; + // Using the stable message_id (rather than a simple queued count) ensures + // a regression that leaves the queued row AND adds a Submitted row for the + // same message cannot slip past as "still one queued message". + let msg_b_id = queued_messages[0].message_id; let msg_b_after_cancel: Vec<_> = history_after_cancel .messages @@ -9833,8 +9901,8 @@ output_schema_ref = "schemas/write.output.json" ); assert_eq!( msg_b_after_cancel[0].status, - MessageStatus::RejectedBusy, - "message B must still be RejectedBusy after run A is cancelled — no auto-resubmission" + MessageStatus::Queued, + "message B must still be queued after run A is cancelled — no auto-resubmission" ); // Guard: no additional Submitted row must have been created for message B's message_id. let submitted_for_b: Vec<_> = history_after_cancel diff --git a/crates/ironclaw_reborn_composition/src/slack_host_beta.rs b/crates/ironclaw_reborn_composition/src/slack_host_beta.rs index b96c9720cd4..e0851fe6402 100644 --- a/crates/ironclaw_reborn_composition/src/slack_host_beta.rs +++ b/crates/ironclaw_reborn_composition/src/slack_host_beta.rs @@ -416,6 +416,7 @@ struct SlackHostBetaRuntimeParts { local_runtime: Arc, thread_service: Arc, turn_coordinator: Arc, + input_enqueue: Arc, approval_interaction_service: Arc, auth_interaction_service: Arc, auth_challenge_provider: Option>, @@ -439,6 +440,7 @@ impl SlackHostBetaRuntimeParts { local_runtime: Arc::clone(local_runtime), thread_service: runtime.webui_thread_service(), turn_coordinator: runtime.webui_turn_coordinator(), + input_enqueue: runtime.webui_input_enqueue(), approval_interaction_service, auth_interaction_service: runtime.webui_auth_interaction_service(), auth_challenge_provider: runtime.auth_challenge_provider(), @@ -802,11 +804,14 @@ fn build_slack_installation_record_with_resolvers( )]); let binding = ProductConversationBindingService::new(conversation_port, installation_resolver); - let inbound = Arc::new(DefaultInboundTurnService::new( - binding.clone(), - Arc::clone(&parts.thread_service), - Arc::clone(&parts.turn_coordinator), - )); + let inbound = Arc::new( + DefaultInboundTurnService::new( + binding.clone(), + Arc::clone(&parts.thread_service), + Arc::clone(&parts.turn_coordinator), + ) + .with_input_enqueue(Arc::clone(&parts.input_enqueue)), + ); let route_store: Arc = Arc::clone(&parts.local_runtime.delivered_gate_routes); let workflow = Arc::new( diff --git a/crates/ironclaw_reborn_composition/src/webui.rs b/crates/ironclaw_reborn_composition/src/webui.rs index 1c2c6e242ec..fedc3b4013b 100644 --- a/crates/ironclaw_reborn_composition/src/webui.rs +++ b/crates/ironclaw_reborn_composition/src/webui.rs @@ -131,6 +131,7 @@ pub(crate) fn build_webui_services_with_connectable_channels( runtime.webui_thread_service(), runtime.webui_turn_coordinator(), ) + .with_input_enqueue(runtime.webui_input_enqueue()) .with_approval_interactions(runtime.webui_approval_interaction_service()) .with_auth_interactions(runtime.webui_auth_interaction_service()); if let Some(workspace_filesystem) = runtime.webui_workspace_filesystem() { diff --git a/crates/ironclaw_reborn_composition/tests/webui_v2_e2e.rs b/crates/ironclaw_reborn_composition/tests/webui_v2_e2e.rs index e63718efbd0..95b7bd6c27a 100644 --- a/crates/ironclaw_reborn_composition/tests/webui_v2_e2e.rs +++ b/crates/ironclaw_reborn_composition/tests/webui_v2_e2e.rs @@ -16,7 +16,10 @@ #![cfg(all(feature = "webui-v2-beta", feature = "test-support"))] use std::path::{Path, PathBuf}; -use std::sync::{Arc, Mutex as StdMutex}; +use std::sync::{ + Arc, Mutex as StdMutex, + atomic::{AtomicBool, Ordering}, +}; use std::time::{Duration, Instant}; use async_trait::async_trait; @@ -48,6 +51,7 @@ use ironclaw_turns::run_profile::{ CapabilityCallCandidate, LoopCapabilityPort, ProviderToolCall, RegisterProviderToolCallRequest, }; use serde_json::{Value, json}; +use tokio::sync::Notify; use tower::ServiceExt; // ─── identities ─────────────────────────────────────────────────────── @@ -220,6 +224,296 @@ impl HostManagedModelGateway for ToolCallingGateway { } } +/// Tool gateway for active-run steering: holds the first model call open so the +/// test can submit a second WebUI message while the run is definitely busy. +struct QueuedSteeringGateway { + call_count: StdMutex, + first_model_started_flag: AtomicBool, + first_model_started: Notify, + release_first_model: Notify, +} + +impl QueuedSteeringGateway { + fn new() -> Self { + Self { + call_count: StdMutex::new(0), + first_model_started_flag: AtomicBool::new(false), + first_model_started: Notify::new(), + release_first_model: Notify::new(), + } + } + + async fn wait_for_first_model_call(&self) { + tokio::time::timeout(Duration::from_secs(5), async { + loop { + let notified = self.first_model_started.notified(); + if self.first_model_started_flag.load(Ordering::Acquire) { + return; + } + notified.await; + } + }) + .await + .expect("first model call must start while run is active"); + } + + fn release_first_model_call(&self) { + self.release_first_model.notify_waiters(); + } +} + +#[async_trait] +impl HostManagedModelGateway for QueuedSteeringGateway { + async fn stream_model( + &self, + _request: HostManagedModelRequest, + ) -> Result { + Err(HostManagedModelError::safe( + HostManagedModelErrorKind::InvalidRequest, + "QueuedSteeringGateway requires the capability-aware model path", + )) + } + + async fn stream_model_with_capabilities( + &self, + request: HostManagedModelRequest, + capabilities: Arc, + ) -> Result { + let call_index = { + let mut count = self + .call_count + .lock() + .expect("queued steering gateway call lock poisoned"); + let index = *count; + *count += 1; + index + }; + + if call_index > 0 { + let tool_result = request + .messages + .iter() + .find(|m| m.role == HostManagedModelMessageRole::ToolResult) + .expect("reply model call must include a tool_result message"); + assert!( + tool_result.content.contains("hello before steering"), + "reply model call should see hydrated echo output, got: {}", + tool_result.content, + ); + assert!( + request.messages.iter().any(|message| { + message.role == HostManagedModelMessageRole::User + && message.content.contains("download as csv") + }), + "reply model call must include queued WebUI follow-up before final reply, got: {:#?}", + request.messages + ); + return Ok(HostManagedModelResponse::assistant_reply( + "csv follow-up honored", + )); + } + + self.first_model_started_flag.store(true, Ordering::Release); + self.first_model_started.notify_waiters(); + self.release_first_model.notified().await; + + let echo_id = CapabilityId::new("builtin.echo").expect("echo capability id"); + let echo_tool = capabilities + .tool_definitions() + .map_err(|err| { + HostManagedModelError::safe( + HostManagedModelErrorKind::InvalidRequest, + format!("tool_definitions failed: {err}"), + ) + })? + .into_iter() + .find(|def| def.capability_id == echo_id) + .expect("builtin.echo must be visible in local-dev capability surface"); + + let candidate = capabilities + .register_provider_tool_call(RegisterProviderToolCallRequest::new(ProviderToolCall { + provider_id: "e2e-provider".to_string(), + provider_model_id: "e2e-model".to_string(), + turn_id: Some("e2e-steering-turn".to_string()), + id: "e2e-steering-call-1".to_string(), + name: echo_tool.name, + arguments: json!({"message": "hello before steering"}), + response_reasoning: None, + reasoning: None, + signature: None, + })) + .await + .map_err(|err| { + HostManagedModelError::safe( + HostManagedModelErrorKind::InvalidRequest, + format!("register_provider_tool_call failed: {err}"), + ) + })?; + + Ok(HostManagedModelResponse::capability_calls( + vec![candidate], + "", + )) + } +} + +/// Reproduces a queued follow-up that arrives after tool execution, while the +/// model is already producing a reply-only turn. The loop must drain that +/// queued steering before accepting the reply as terminal. +struct QueuedDuringReplyGateway { + call_count: StdMutex, + reply_model_started_flag: AtomicBool, + reply_model_started: Notify, + release_reply_model: Notify, +} + +impl QueuedDuringReplyGateway { + fn new() -> Self { + Self { + call_count: StdMutex::new(0), + reply_model_started_flag: AtomicBool::new(false), + reply_model_started: Notify::new(), + release_reply_model: Notify::new(), + } + } + + async fn wait_for_reply_model_call(&self) { + tokio::time::timeout(Duration::from_secs(5), async { + loop { + let notified = self.reply_model_started.notified(); + if self.reply_model_started_flag.load(Ordering::Acquire) { + return; + } + notified.await; + } + }) + .await + .expect("reply model call must start while run is active"); + } + + fn release_reply_model_call(&self) { + self.release_reply_model.notify_waiters(); + } +} + +#[async_trait] +impl HostManagedModelGateway for QueuedDuringReplyGateway { + async fn stream_model( + &self, + _request: HostManagedModelRequest, + ) -> Result { + Err(HostManagedModelError::safe( + HostManagedModelErrorKind::InvalidRequest, + "QueuedDuringReplyGateway requires the capability-aware model path", + )) + } + + async fn stream_model_with_capabilities( + &self, + request: HostManagedModelRequest, + capabilities: Arc, + ) -> Result { + let call_index = { + let mut count = self + .call_count + .lock() + .expect("queued during reply gateway call lock poisoned"); + let index = *count; + *count += 1; + index + }; + + if call_index == 1 { + let tool_result = request + .messages + .iter() + .find(|m| m.role == HostManagedModelMessageRole::ToolResult) + .expect("reply model call must include a tool_result message"); + assert!( + tool_result.content.contains("market cap source"), + "reply model call should see hydrated echo output, got: {}", + tool_result.content, + ); + self.reply_model_started_flag.store(true, Ordering::Release); + self.reply_model_started.notify_waiters(); + self.release_reply_model.notified().await; + return Ok(HostManagedModelResponse::assistant_reply( + "Let me try to get the data via the API format.", + )); + } + + if call_index > 1 { + let assistant_index = request + .messages + .iter() + .position(|message| { + message.role == HostManagedModelMessageRole::Assistant + && message + .content + .contains("Let me try to get the data via the API format.") + }) + .expect("post-reply model call must preserve the prior assistant reply"); + let followup_messages = request + .messages + .iter() + .enumerate() + .filter_map(|(index, message)| { + (index > assistant_index && message.role == HostManagedModelMessageRole::User) + .then_some(message) + }) + .collect::>(); + assert_eq!( + followup_messages.len(), + 1, + "queued WebUI follow-ups must be merged into one user prompt after the preserved assistant reply, got: {:#?}", + request.messages + ); + assert_eq!(followup_messages[0].content, "save to csv\ninclude headers"); + return Ok(HostManagedModelResponse::assistant_reply( + "I picked up the merged save-to-csv follow-ups.", + )); + } + + let echo_id = CapabilityId::new("builtin.echo").expect("echo capability id"); + let echo_tool = capabilities + .tool_definitions() + .map_err(|err| { + HostManagedModelError::safe( + HostManagedModelErrorKind::InvalidRequest, + format!("tool_definitions failed: {err}"), + ) + })? + .into_iter() + .find(|def| def.capability_id == echo_id) + .expect("builtin.echo must be visible in local-dev capability surface"); + + let candidate = capabilities + .register_provider_tool_call(RegisterProviderToolCallRequest::new(ProviderToolCall { + provider_id: "e2e-provider".to_string(), + provider_model_id: "e2e-model".to_string(), + turn_id: Some("e2e-reply-queue-turn".to_string()), + id: "e2e-reply-queue-call-1".to_string(), + name: echo_tool.name, + arguments: json!({"message": "market cap source"}), + response_reasoning: None, + reasoning: None, + signature: None, + })) + .await + .map_err(|err| { + HostManagedModelError::safe( + HostManagedModelErrorKind::InvalidRequest, + format!("register_provider_tool_call failed: {err}"), + ) + })?; + + Ok(HostManagedModelResponse::capability_calls( + vec![candidate], + "", + )) + } +} + // ─── file-producing gateway ─────────────────────────────────────────── const CSV_PATH: &str = "/workspace/report.csv"; @@ -599,13 +893,28 @@ async fn create_thread(router: &axum::Router, client_action_id: &str) -> String } async fn send_message(router: &axum::Router, thread_id: &str, client_action_id: &str) { + send_message_with_content( + router, + thread_id, + client_action_id, + "please call the echo tool", + ) + .await; +} + +async fn send_message_with_content( + router: &axum::Router, + thread_id: &str, + client_action_id: &str, + content: &str, +) { let send = router .clone() .oneshot(bearer_post( &format!("/api/webchat/v2/threads/{thread_id}/messages"), json!({ "client_action_id": client_action_id, - "content": "please call the echo tool", + "content": content, }), )) .await @@ -618,7 +927,16 @@ async fn send_message(router: &axum::Router, thread_id: &str, client_action_id: } async fn wait_for_final_timeline(router: &axum::Router, thread_id: &str) -> Value { + wait_for_timeline_with_assistant_text(router, thread_id, "e2e tool ok").await +} + +async fn wait_for_timeline_with_assistant_text( + router: &axum::Router, + thread_id: &str, + needle: &str, +) -> Value { let deadline = Instant::now() + Duration::from_secs(10); + let mut last_timeline = None; while Instant::now() < deadline { let response = router .clone() @@ -627,19 +945,27 @@ async fn wait_for_final_timeline(router: &axum::Router, thread_id: &str) -> Valu ))) .await .expect("timeline oneshot"); + if response.status() == StatusCode::TOO_MANY_REQUESTS { + tokio::time::sleep(Duration::from_millis(200)).await; + continue; + } assert_eq!(response.status(), StatusCode::OK); let timeline = read_json(response).await; let messages = timeline["messages"] .as_array() .expect("timeline.messages must be an array"); if messages.iter().any(|message| { - extract_assistant_text(message).is_some_and(|text| text.contains("e2e tool ok")) + extract_assistant_text(message).is_some_and(|text| text.contains(needle)) }) { return timeline; } + last_timeline = Some(timeline); tokio::time::sleep(Duration::from_millis(50)).await; } - panic!("timeline never surfaced an assistant message containing 'e2e tool ok' within 10s"); + panic!( + "timeline never surfaced an assistant message containing {needle:?} within 10s; last timeline: {}", + serde_json::to_string_pretty(&last_timeline).expect("timeline debug json") + ); } fn assert_timeline_has_tool_result_reference(timeline: &Value) { @@ -861,6 +1187,131 @@ async fn webui_v2_timeline_persists_display_preview_under_authenticated_owner() .expect("runtime shutdown clean"); } +#[tokio::test] +async fn webui_v2_queued_followup_is_steered_into_post_tool_reply() { + let gateway = Arc::new(QueuedSteeringGateway::new()); + let harness = build_harness_with_gateway(gateway.clone()).await; + + let thread_id = create_thread(&harness.router, "e2e-queued-steering-create").await; + send_message_with_content( + &harness.router, + &thread_id, + "e2e-queued-steering-first", + "start with a tool call", + ) + .await; + gateway.wait_for_first_model_call().await; + + send_message_with_content( + &harness.router, + &thread_id, + "e2e-queued-steering-followup", + "download as csv", + ) + .await; + gateway.release_first_model_call(); + + let timeline = + wait_for_timeline_with_assistant_text(&harness.router, &thread_id, "csv follow-up honored") + .await; + assert_timeline_has_tool_result_reference(&timeline); + + harness + .runtime + .shutdown() + .await + .expect("runtime shutdown clean"); +} + +#[tokio::test] +async fn webui_v2_queued_followup_during_reply_continues_after_reply_only_turn() { + let gateway = Arc::new(QueuedDuringReplyGateway::new()); + let harness = build_harness_with_gateway(gateway.clone()).await; + + let thread_id = create_thread(&harness.router, "e2e-queued-during-reply-create").await; + send_message_with_content( + &harness.router, + &thread_id, + "e2e-queued-during-reply-first", + "what is total market cap of companies that IPOed in 2025?", + ) + .await; + gateway.wait_for_reply_model_call().await; + + send_message_with_content( + &harness.router, + &thread_id, + "e2e-queued-during-reply-followup", + "save to csv", + ) + .await; + send_message_with_content( + &harness.router, + &thread_id, + "e2e-queued-during-reply-followup-2", + "include headers", + ) + .await; + gateway.release_reply_model_call(); + + let timeline = wait_for_timeline_with_assistant_text( + &harness.router, + &thread_id, + "I picked up the merged save-to-csv follow-ups.", + ) + .await; + let messages = timeline["messages"] + .as_array() + .expect("timeline.messages must be an array"); + let first_reply_index = messages + .iter() + .position(|message| { + extract_assistant_text(message) + .is_some_and(|text| text.contains("Let me try to get the data via the API format.")) + }) + .expect("initial post-tool assistant reply must be finalized"); + let followup_index = messages + .iter() + .position(|message| { + message.get("kind").and_then(Value::as_str) == Some("user") + && message + .get("content") + .and_then(Value::as_str) + .is_some_and(|text| text.contains("save to csv")) + }) + .expect("queued follow-up user message must be visible in the timeline"); + let second_followup_index = messages + .iter() + .position(|message| { + message.get("kind").and_then(Value::as_str) == Some("user") + && message + .get("content") + .and_then(Value::as_str) + .is_some_and(|text| text.contains("include headers")) + }) + .expect("second queued follow-up user message must be visible in the timeline"); + let final_reply_index = messages + .iter() + .position(|message| { + extract_assistant_text(message) + .is_some_and(|text| text.contains("I picked up the merged save-to-csv follow-ups.")) + }) + .expect("follow-up assistant reply must be finalized"); + assert!( + first_reply_index < followup_index + && followup_index < second_followup_index + && second_followup_index < final_reply_index, + "timeline must preserve assistant -> queued users -> assistant ordering, got: {messages:#?}" + ); + assert_timeline_has_tool_result_reference(&timeline); + + harness + .runtime + .shutdown() + .await + .expect("runtime shutdown clean"); +} + /// Beta scoreboard acceptance for issue #3613: drive the WebUI/WebChat /// v2 API from the browser side, stream live Reborn projections over /// SSE, replay with `Last-Event-ID`, verify final durable timeline diff --git a/crates/ironclaw_threads/src/contract.rs b/crates/ironclaw_threads/src/contract.rs index aad89ab695a..9b0c4c8f03a 100644 --- a/crates/ironclaw_threads/src/contract.rs +++ b/crates/ironclaw_threads/src/contract.rs @@ -159,6 +159,9 @@ pub enum MessageKind { #[serde(rename_all = "snake_case")] pub enum MessageStatus { Accepted, + /// Message is accepted and queued for an active run to consume at the next + /// steering/input boundary. + Queued, Submitted, /// Message arrived while the thread was busy; it will NOT be auto-resubmitted. /// The user must resend the message once the current task finishes. diff --git a/crates/ironclaw_threads/src/filesystem_service.rs b/crates/ironclaw_threads/src/filesystem_service.rs index 368f182647f..c4014db2405 100644 --- a/crates/ironclaw_threads/src/filesystem_service.rs +++ b/crates/ironclaw_threads/src/filesystem_service.rs @@ -1326,8 +1326,35 @@ where .ok_or_else(|| SessionThreadError::UnknownThread { thread_id: thread_id.clone(), })?; + let queued_sequence = match self + .read_message_versioned(scope, thread_id, message_id) + .await? + .ok_or(SessionThreadError::UnknownMessage { message_id })? + .0 + .status + { + MessageStatus::Queued => Some(self.reserve_sequence(scope, thread_id).await?), + _ => None, + }; self.apply_message_update(scope, thread_id, message_id, |message| { + // Idempotent re-submit: if this exact run already submitted the + // message, a redelivered/duplicate ack is a no-op rather than an + // `InvalidMessageTransition`. The queued-message consumer + // (`InMemoryHostInputQueue::ack_consumed`) drives this transition on + // an at-least-once ack path, so the same run can legitimately ack + // twice. The terminal-state guard is preserved: a *different* run, + // or a `RejectedBusy` row, still fails through `ensure_user_accepted`. + if message.status == MessageStatus::Submitted + && message.turn_run_id.as_deref() == Some(turn_run_id.as_str()) + { + return Ok(()); + } ensure_user_accepted(message, "mark_message_submitted")?; + if message.status == MessageStatus::Queued + && let Some(sequence) = queued_sequence + { + message.sequence = sequence; + } message.status = MessageStatus::Submitted; message.turn_id = Some(turn_id.clone()); message.turn_run_id = Some(turn_run_id.clone()); @@ -1357,14 +1384,36 @@ where .await } + async fn mark_message_queued( + &self, + scope: &ThreadScope, + thread_id: &ThreadId, + message_id: ThreadMessageId, + active_run_id: String, + ) -> Result { + self.read_thread_versioned(scope, thread_id) + .await? + .ok_or_else(|| SessionThreadError::UnknownThread { + thread_id: thread_id.clone(), + })?; + self.apply_message_update(scope, thread_id, message_id, |message| { + ensure_user_accepted(message, "mark_message_queued")?; + message.status = MessageStatus::Queued; + message.turn_id = None; + message.turn_run_id = Some(active_run_id.clone()); + Ok(()) + }) + .await + } + async fn append_assistant_draft( &self, request: AppendAssistantDraftRequest, ) -> Result { - // Dedup-by-turn-run-id: read the secondary index first and fall back - // to the legacy scan for rows written before the index existed. - // Retrying a draft append with the same `turn_run_id` returns the - // existing record rather than creating a sibling. + // Dedup-by-turn-run-id while preserving multiple finalized assistant + // replies in a run. Retries of the same draft/final content reuse the + // existing record; a different finalized reply starts a sibling draft. + let requested_content = request.content.as_text().to_owned(); if let Some(existing) = self .find_assistant_message_by_run( &request.scope, @@ -1373,6 +1422,7 @@ where None, ) .await? + && should_reuse_assistant_run_message(&existing, &requested_content) { return Ok(existing); } @@ -1392,7 +1442,7 @@ where turn_run_id: Some(request.turn_run_id), tool_result_ref: None, tool_result_provider_call: None, - content: Some(request.content.into_text()), + content: Some(requested_content), attachments: Vec::new(), redaction_ref: None, }; @@ -2366,7 +2416,7 @@ fn ensure_user_accepted( if message.kind == MessageKind::User && matches!( message.status, - MessageStatus::Accepted | MessageStatus::DeferredBusy + MessageStatus::Accepted | MessageStatus::DeferredBusy | MessageStatus::Queued ) { return Ok(()); @@ -2438,6 +2488,17 @@ fn assistant_message_matches_run( && required_status.is_none_or(|status| message.status == status) } +fn should_reuse_assistant_run_message( + message: &ThreadMessageRecord, + requested_content: &str, +) -> bool { + match message.status { + MessageStatus::Draft | MessageStatus::Redacted | MessageStatus::Deleted => true, + MessageStatus::Finalized => message.content.as_deref() == Some(requested_content), + _ => false, + } +} + const REDACTED_SUMMARY_CONTENT: &str = "[redacted]"; fn context_messages_with_summary_replacements( @@ -2561,7 +2622,7 @@ fn history_summary_artifacts( /// apply — blocking it would silently drop a legitimate compacted range. /// /// Resurfaceable statuses (must still block the summary): -/// Draft | Interrupted | Superseded | DeferredBusy +/// Draft | Interrupted | Superseded | Queued | DeferredBusy /// Permanent non-visible (must NOT block): /// RejectedBusy (terminal, user must explicitly resend) /// CapabilityDisplayPreview kind (never model-visible regardless of status) @@ -2575,6 +2636,7 @@ fn can_resurface_as_model_visible(message: &ThreadMessageRecord) -> bool { MessageStatus::Draft | MessageStatus::Interrupted | MessageStatus::Superseded + | MessageStatus::Queued | MessageStatus::DeferredBusy ) } diff --git a/crates/ironclaw_threads/src/in_memory.rs b/crates/ironclaw_threads/src/in_memory.rs index ef742e62ed2..06d2f33a19c 100644 --- a/crates/ironclaw_threads/src/in_memory.rs +++ b/crates/ironclaw_threads/src/in_memory.rs @@ -240,12 +240,45 @@ impl SessionThreadService for InMemorySessionThreadService { turn_run_id: String, ) -> Result { let mut state = self.state.lock().await; - let message = get_message_mut(&mut state, scope, thread_id, message_id)?; - ensure_user_accepted(message, "mark_message_submitted")?; + let thread = get_thread_mut(&mut state, scope, thread_id)?; + let message_index = thread + .messages + .iter() + .position(|message| message.message_id == message_id) + .ok_or(SessionThreadError::UnknownMessage { message_id })?; + // Idempotent re-submit: if this exact run already submitted the message, + // a redelivered/duplicate ack is a no-op rather than an + // `InvalidMessageTransition`. Mirrors the filesystem backend so the + // queued-message consumer's at-least-once ack path is tolerant on both + // stores. A *different* run, or a `RejectedBusy` row, still fails below. + { + let message = &thread.messages[message_index]; + if message.status == MessageStatus::Submitted + && message.turn_run_id.as_deref() == Some(turn_run_id.as_str()) + { + return Ok(message.clone()); + } + } + let was_queued = { + let message = &thread.messages[message_index]; + ensure_user_accepted(message, "mark_message_submitted")?; + message.status == MessageStatus::Queued + }; + if was_queued { + let sequence = thread.next_sequence; + thread.next_sequence += 1; + thread.record.updated_at = Some(Utc::now()); + thread.messages[message_index].sequence = sequence; + } + let message = &mut thread.messages[message_index]; message.status = MessageStatus::Submitted; message.turn_id = Some(turn_id); message.turn_run_id = Some(turn_run_id); - Ok(message.clone()) + let submitted = message.clone(); + if was_queued { + thread.messages.sort_by_key(|message| message.sequence); + } + Ok(submitted) } async fn mark_message_rejected_busy( @@ -263,15 +296,33 @@ impl SessionThreadService for InMemorySessionThreadService { Ok(message.clone()) } + async fn mark_message_queued( + &self, + scope: &ThreadScope, + thread_id: &ThreadId, + message_id: ThreadMessageId, + active_run_id: String, + ) -> Result { + let mut state = self.state.lock().await; + let message = get_message_mut(&mut state, scope, thread_id, message_id)?; + ensure_user_accepted(message, "mark_message_queued")?; + message.status = MessageStatus::Queued; + message.turn_id = None; + message.turn_run_id = Some(active_run_id); + Ok(message.clone()) + } + async fn append_assistant_draft( &self, request: AppendAssistantDraftRequest, ) -> Result { let mut state = self.state.lock().await; let thread = get_thread_mut(&mut state, &request.scope, &request.thread_id)?; - if let Some(existing) = thread.messages.iter().find(|message| { + let requested_content = request.content.as_text().to_owned(); + if let Some(existing) = thread.messages.iter().rev().find(|message| { message.kind == MessageKind::Assistant && message.turn_run_id.as_deref() == Some(request.turn_run_id.as_str()) + && should_reuse_assistant_run_message(message, &requested_content) }) { return Ok(existing.clone()); } @@ -289,7 +340,7 @@ impl SessionThreadService for InMemorySessionThreadService { turn_run_id: Some(request.turn_run_id), tool_result_ref: None, tool_result_provider_call: None, - content: Some(request.content.into_text()), + content: Some(requested_content), attachments: Vec::new(), redaction_ref: None, }; @@ -931,7 +982,7 @@ fn ensure_user_accepted( if message.kind == MessageKind::User && matches!( message.status, - MessageStatus::Accepted | MessageStatus::DeferredBusy + MessageStatus::Accepted | MessageStatus::DeferredBusy | MessageStatus::Queued ) { return Ok(()); @@ -1057,6 +1108,17 @@ fn history_message(message: &ThreadMessageRecord) -> ThreadMessageRecord { } } +fn should_reuse_assistant_run_message( + message: &ThreadMessageRecord, + requested_content: &str, +) -> bool { + match message.status { + MessageStatus::Draft | MessageStatus::Redacted | MessageStatus::Deleted => true, + MessageStatus::Finalized => message.content.as_deref() == Some(requested_content), + _ => false, + } +} + /// Returns true when a non-model-context-visible message within the summary /// span could later become model-visible (i.e. it is in a resurfaceable pending /// state). Permanently-terminal non-visible messages (RejectedBusy, capability @@ -1064,7 +1126,7 @@ fn history_message(message: &ThreadMessageRecord) -> ThreadMessageRecord { /// apply — blocking it would silently drop a legitimate compacted range. /// /// Resurfaceable statuses (must still block the summary): -/// Draft | Interrupted | Superseded | DeferredBusy +/// Draft | Interrupted | Superseded | Queued | DeferredBusy /// Permanent non-visible (must NOT block): /// RejectedBusy (terminal, user must explicitly resend) /// CapabilityDisplayPreview kind (never model-visible regardless of status) @@ -1078,6 +1140,7 @@ fn can_resurface_as_model_visible(message: &ThreadMessageRecord) -> bool { MessageStatus::Draft | MessageStatus::Interrupted | MessageStatus::Superseded + | MessageStatus::Queued | MessageStatus::DeferredBusy ) } diff --git a/crates/ironclaw_threads/src/service.rs b/crates/ironclaw_threads/src/service.rs index 80619ef6b7c..a001aaabee1 100644 --- a/crates/ironclaw_threads/src/service.rs +++ b/crates/ironclaw_threads/src/service.rs @@ -50,6 +50,19 @@ pub trait SessionThreadService: Send + Sync { message_id: ThreadMessageId, ) -> Result; + async fn mark_message_queued( + &self, + scope: &ThreadScope, + thread_id: &ThreadId, + message_id: ThreadMessageId, + active_run_id: String, + ) -> Result { + let _ = (scope, thread_id, message_id, active_run_id); + Err(SessionThreadError::Backend( + "mark_message_queued is not implemented by this thread service".to_string(), + )) + } + async fn append_assistant_draft( &self, request: AppendAssistantDraftRequest, @@ -314,6 +327,18 @@ where .await } + async fn mark_message_queued( + &self, + scope: &ThreadScope, + thread_id: &ThreadId, + message_id: ThreadMessageId, + active_run_id: String, + ) -> Result { + self.as_ref() + .mark_message_queued(scope, thread_id, message_id, active_run_id) + .await + } + async fn append_assistant_draft( &self, request: AppendAssistantDraftRequest, diff --git a/crates/ironclaw_threads/tests/filesystem_session_thread_contract.rs b/crates/ironclaw_threads/tests/filesystem_session_thread_contract.rs index 92dbc7bb851..94b4779f12e 100644 --- a/crates/ironclaw_threads/tests/filesystem_session_thread_contract.rs +++ b/crates/ironclaw_threads/tests/filesystem_session_thread_contract.rs @@ -206,6 +206,273 @@ async fn filesystem_finalized_assistant_lookup_by_run_uses_persisted_message() { assert_eq!(finalized.content.as_deref(), Some("final")); } +#[tokio::test] +async fn filesystem_assistant_draft_append_allows_later_reply_in_same_turn_run() { + let backend = Arc::new(InMemoryBackend::new()); + let scoped = scoped_threads_fs_at(backend, "tenant-assistant-sibling", "alice"); + let service = FilesystemSessionThreadService::new(scoped); + let scope = scope("assistant-sibling"); + let thread = service + .ensure_thread(EnsureThreadRequest { + scope: scope.clone(), + thread_id: Some(ThreadId::new("thread-assistant-sibling").unwrap()), + created_by_actor_id: "actor-a".into(), + title: None, + metadata_json: None, + }) + .await + .unwrap(); + + let first = service + .append_assistant_draft(AppendAssistantDraftRequest { + scope: scope.clone(), + thread_id: thread.thread_id.clone(), + turn_run_id: "run-assistant-sibling".into(), + content: MessageContent::text("draft"), + }) + .await + .unwrap(); + service + .finalize_assistant_message( + &scope, + &thread.thread_id, + first.message_id, + MessageContent::text("first reply"), + ) + .await + .unwrap(); + + let retry = service + .append_assistant_draft(AppendAssistantDraftRequest { + scope: scope.clone(), + thread_id: thread.thread_id.clone(), + turn_run_id: "run-assistant-sibling".into(), + content: MessageContent::text("first reply"), + }) + .await + .unwrap(); + assert_eq!(retry.message_id, first.message_id); + + let second = service + .append_assistant_draft(AppendAssistantDraftRequest { + scope: scope.clone(), + thread_id: thread.thread_id.clone(), + turn_run_id: "run-assistant-sibling".into(), + content: MessageContent::text("second reply"), + }) + .await + .unwrap(); + assert_ne!(second.message_id, first.message_id); + service + .finalize_assistant_message( + &scope, + &thread.thread_id, + second.message_id, + MessageContent::text("second reply"), + ) + .await + .unwrap(); + + let history = service + .list_thread_history(ThreadHistoryRequest { + scope: scope.clone(), + thread_id: thread.thread_id.clone(), + }) + .await + .unwrap(); + let assistant_messages: Vec<_> = history + .messages + .iter() + .filter(|message| message.kind == MessageKind::Assistant) + .collect(); + assert_eq!(assistant_messages.len(), 2); + assert_eq!( + assistant_messages[0].content.as_deref(), + Some("first reply") + ); + assert_eq!( + assistant_messages[1].content.as_deref(), + Some("second reply") + ); + + let latest = service + .finalized_assistant_message_by_run(FinalizedAssistantMessageByRunRequest { + scope, + thread_id: thread.thread_id, + turn_run_id: "run-assistant-sibling".into(), + }) + .await + .unwrap() + .expect("latest finalized assistant message remains indexed by run"); + assert_eq!(latest.message_id, second.message_id); +} + +#[tokio::test] +async fn filesystem_queued_user_message_is_resequenced_when_submitted() { + let backend = Arc::new(InMemoryBackend::new()); + let scoped = scoped_threads_fs_at(backend, "tenant-queued-order", "alice"); + let service = FilesystemSessionThreadService::new(scoped); + let scope = scope("queued-order"); + let thread = service + .ensure_thread(EnsureThreadRequest { + scope: scope.clone(), + thread_id: Some(ThreadId::new("thread-queued-order").unwrap()), + created_by_actor_id: "actor-a".into(), + title: None, + metadata_json: None, + }) + .await + .unwrap(); + + let queued = service + .accept_inbound_message(AcceptInboundMessageRequest { + scope: scope.clone(), + thread_id: thread.thread_id.clone(), + actor_id: "actor-a".into(), + source_binding_id: None, + reply_target_binding_id: None, + external_event_id: None, + content: MessageContent::text("queued follow-up"), + }) + .await + .unwrap(); + service + .mark_message_queued( + &scope, + &thread.thread_id, + queued.message_id, + "run-queued-order".into(), + ) + .await + .unwrap(); + let assistant = service + .append_assistant_draft(AppendAssistantDraftRequest { + scope: scope.clone(), + thread_id: thread.thread_id.clone(), + turn_run_id: "run-queued-order".into(), + content: MessageContent::text("assistant boundary"), + }) + .await + .unwrap(); + service + .finalize_assistant_message( + &scope, + &thread.thread_id, + assistant.message_id, + MessageContent::text("assistant boundary"), + ) + .await + .unwrap(); + + let submitted = service + .mark_message_submitted( + &scope, + &thread.thread_id, + queued.message_id, + "turn-queued-order".into(), + "run-queued-order".into(), + ) + .await + .unwrap(); + assert!(assistant.sequence < submitted.sequence); + + let history = service + .list_thread_history(ThreadHistoryRequest { + scope, + thread_id: thread.thread_id, + }) + .await + .unwrap(); + assert_eq!(history.messages[0].kind, MessageKind::Assistant); + assert_eq!(history.messages[1].kind, MessageKind::User); + assert_eq!( + history.messages[1].content.as_deref(), + Some("queued follow-up") + ); +} + +#[tokio::test] +async fn filesystem_mark_message_submitted_is_idempotent_for_same_run() { + // Dual-backend parity with the in-memory contract: the queued-message + // consumer acks on an at-least-once path, so the SAME run re-submitting is an + // idempotent no-op, while a DIFFERENT run is still rejected as an invalid + // transition. + let backend = Arc::new(InMemoryBackend::new()); + let scoped = scoped_threads_fs_at(backend, "tenant-idempotent-submit", "alice"); + let service = FilesystemSessionThreadService::new(scoped); + let scope = scope("idempotent-submit"); + let thread = service + .ensure_thread(EnsureThreadRequest { + scope: scope.clone(), + thread_id: Some(ThreadId::new("thread-idempotent-submit").unwrap()), + created_by_actor_id: "actor-a".into(), + title: None, + metadata_json: None, + }) + .await + .unwrap(); + let accepted = service + .accept_inbound_message(AcceptInboundMessageRequest { + scope: scope.clone(), + thread_id: thread.thread_id.clone(), + actor_id: "actor-a".into(), + source_binding_id: None, + reply_target_binding_id: None, + external_event_id: None, + content: MessageContent::text("idempotent submit"), + }) + .await + .unwrap(); + + service + .mark_message_submitted( + &scope, + &thread.thread_id, + accepted.message_id, + "turn-1".into(), + "run-1".into(), + ) + .await + .expect("first submit"); + service + .mark_message_submitted( + &scope, + &thread.thread_id, + accepted.message_id, + "turn-1".into(), + "run-1".into(), + ) + .await + .expect("idempotent re-submit for the same run must succeed"); + + let foreign = service + .mark_message_submitted( + &scope, + &thread.thread_id, + accepted.message_id, + "turn-2".into(), + "run-2".into(), + ) + .await; + assert!( + matches!( + foreign, + Err(SessionThreadError::InvalidMessageTransition { .. }) + ), + "a different run must not re-submit an already-submitted message, got {foreign:?}" + ); + + let history = service + .list_thread_history(ThreadHistoryRequest { + scope, + thread_id: thread.thread_id, + }) + .await + .unwrap(); + assert_eq!(history.messages[0].status, MessageStatus::Submitted); + assert_eq!(history.messages[0].turn_run_id.as_deref(), Some("run-1")); +} + #[tokio::test] async fn filesystem_lookup_index_write_failure_does_not_fail_message_contract() { let backend = Arc::new(LookupIndexWriteFailureBackend::new()); diff --git a/crates/ironclaw_threads/tests/session_thread_contract.rs b/crates/ironclaw_threads/tests/session_thread_contract.rs index c32d42bce2e..1543578324e 100644 --- a/crates/ironclaw_threads/tests/session_thread_contract.rs +++ b/crates/ironclaw_threads/tests/session_thread_contract.rs @@ -1072,6 +1072,89 @@ async fn rejected_busy_cannot_be_marked_submitted_is_terminal() { ); } +#[tokio::test] +async fn mark_message_submitted_is_idempotent_for_same_run() { + // The queued-message consumer (`InMemoryHostInputQueue::ack_consumed`) drives + // `mark_message_submitted` on an at-least-once ack path, so the SAME run may + // submit a message twice. A redelivered submit for the same run is an + // idempotent no-op; a DIFFERENT run is still rejected as an invalid + // transition (a message belongs to the run that first consumed it). + let service = InMemorySessionThreadService::default(); + let thread = service + .ensure_thread(EnsureThreadRequest { + scope: scope("a"), + thread_id: None, + created_by_actor_id: "actor-a".into(), + title: None, + metadata_json: None, + }) + .await + .unwrap(); + let accepted = service + .accept_inbound_message(AcceptInboundMessageRequest { + scope: scope("a"), + thread_id: thread.thread_id.clone(), + actor_id: "actor-a".into(), + source_binding_id: None, + reply_target_binding_id: None, + external_event_id: None, + content: user_message("idempotent submit"), + }) + .await + .unwrap(); + + service + .mark_message_submitted( + &scope("a"), + &thread.thread_id, + accepted.message_id, + "turn-1".into(), + "run-1".into(), + ) + .await + .expect("first submit"); + + // Same run re-submits (redelivered ack): must be an idempotent no-op. + service + .mark_message_submitted( + &scope("a"), + &thread.thread_id, + accepted.message_id, + "turn-1".into(), + "run-1".into(), + ) + .await + .expect("idempotent re-submit for the same run must succeed"); + + // A different run must NOT be able to claim an already-submitted message. + let foreign = service + .mark_message_submitted( + &scope("a"), + &thread.thread_id, + accepted.message_id, + "turn-2".into(), + "run-2".into(), + ) + .await; + assert!( + matches!( + foreign, + Err(SessionThreadError::InvalidMessageTransition { .. }) + ), + "a different run must not re-submit an already-submitted message, got {foreign:?}" + ); + + let history = service + .list_thread_history(ThreadHistoryRequest { + scope: scope("a"), + thread_id: thread.thread_id, + }) + .await + .unwrap(); + assert_eq!(history.messages[0].status, MessageStatus::Submitted); + assert_eq!(history.messages[0].turn_run_id.as_deref(), Some("run-1")); +} + #[tokio::test] async fn assistant_streaming_updates_one_draft_and_finalizes_one_canonical_message() { let service = InMemorySessionThreadService::default(); @@ -2040,7 +2123,7 @@ async fn summary_spanning_interior_draft_is_not_applied() { } #[tokio::test] -async fn duplicate_assistant_draft_for_same_turn_run_is_idempotent() { +async fn assistant_draft_append_allows_later_reply_in_same_turn_run() { let service = InMemorySessionThreadService::default(); let thread = service .ensure_thread(EnsureThreadRequest { @@ -2084,23 +2167,46 @@ async fn duplicate_assistant_draft_for_same_turn_run_is_idempotent() { ) .await .unwrap(); - let after_final = service + let retry_after_final = service .append_assistant_draft(AppendAssistantDraftRequest { scope: scope("a"), thread_id: thread.thread_id.clone(), turn_run_id: "run-1".into(), - content: MessageContent::text("retry after final ignored"), + content: MessageContent::text("final answer"), }) .await .unwrap(); - assert_eq!(first.message_id, after_final.message_id); + assert_eq!(first.message_id, retry_after_final.message_id); + + let second = service + .append_assistant_draft(AppendAssistantDraftRequest { + scope: scope("a"), + thread_id: thread.thread_id.clone(), + turn_run_id: "run-1".into(), + content: MessageContent::text("second answer"), + }) + .await + .unwrap(); + assert_ne!(first.message_id, second.message_id); + assert_eq!(second.status, MessageStatus::Draft); + assert_eq!(second.content.as_deref(), Some("second answer")); + + service + .finalize_assistant_message( + &scope("a"), + &thread.thread_id, + second.message_id, + MessageContent::text("second answer"), + ) + .await + .unwrap(); service .redact_message(RedactMessageRequest { scope: scope("a"), thread_id: thread.thread_id.clone(), - message_id: first.message_id, + message_id: second.message_id, redaction_ref: "redaction/audit/assistant".into(), }) .await @@ -2114,7 +2220,7 @@ async fn duplicate_assistant_draft_for_same_turn_run_is_idempotent() { }) .await .unwrap(); - assert_eq!(first.message_id, after_redaction.message_id); + assert_eq!(second.message_id, after_redaction.message_id); assert_eq!(after_redaction.status, MessageStatus::Redacted); assert!(after_redaction.content.is_none()); @@ -2125,8 +2231,91 @@ async fn duplicate_assistant_draft_for_same_turn_run_is_idempotent() { }) .await .unwrap(); - assert_eq!(history.messages.len(), 1); - assert_eq!(history.messages[0].status, MessageStatus::Redacted); + assert_eq!(history.messages.len(), 2); + assert_eq!(history.messages[0].status, MessageStatus::Finalized); + assert_eq!(history.messages[0].content.as_deref(), Some("final answer")); + assert_eq!(history.messages[1].status, MessageStatus::Redacted); +} + +#[tokio::test] +async fn queued_user_message_is_resequenced_when_submitted() { + let service = InMemorySessionThreadService::default(); + let thread = service + .ensure_thread(EnsureThreadRequest { + scope: scope("queued-order"), + thread_id: None, + created_by_actor_id: "actor-a".into(), + title: None, + metadata_json: None, + }) + .await + .unwrap(); + + let queued = service + .accept_inbound_message(AcceptInboundMessageRequest { + scope: scope("queued-order"), + thread_id: thread.thread_id.clone(), + actor_id: "actor-a".into(), + source_binding_id: None, + reply_target_binding_id: None, + external_event_id: None, + content: user_message("queued follow-up"), + }) + .await + .unwrap(); + service + .mark_message_queued( + &scope("queued-order"), + &thread.thread_id, + queued.message_id, + "run-1".into(), + ) + .await + .unwrap(); + let assistant = service + .append_assistant_draft(AppendAssistantDraftRequest { + scope: scope("queued-order"), + thread_id: thread.thread_id.clone(), + turn_run_id: "run-1".into(), + content: MessageContent::text("assistant boundary"), + }) + .await + .unwrap(); + service + .finalize_assistant_message( + &scope("queued-order"), + &thread.thread_id, + assistant.message_id, + MessageContent::text("assistant boundary"), + ) + .await + .unwrap(); + + let submitted = service + .mark_message_submitted( + &scope("queued-order"), + &thread.thread_id, + queued.message_id, + "turn-1".into(), + "run-1".into(), + ) + .await + .unwrap(); + assert!(assistant.sequence < submitted.sequence); + + let history = service + .list_thread_history(ThreadHistoryRequest { + scope: scope("queued-order"), + thread_id: thread.thread_id, + }) + .await + .unwrap(); + assert_eq!(history.messages[0].kind, MessageKind::Assistant); + assert_eq!(history.messages[1].kind, MessageKind::User); + assert_eq!( + history.messages[1].content.as_deref(), + Some("queued follow-up") + ); } #[tokio::test] diff --git a/crates/ironclaw_turns/src/run_profile/host.rs b/crates/ironclaw_turns/src/run_profile/host.rs index c719a934269..52cc5f50933 100644 --- a/crates/ironclaw_turns/src/run_profile/host.rs +++ b/crates/ironclaw_turns/src/run_profile/host.rs @@ -424,8 +424,25 @@ impl<'de> Deserialize<'de> for LoopSafeSummary { } } +impl LoopInputCursorToken { + /// Canonical run-start origin cursor — the read position before the first + /// input. The single source of truth for the origin token so host queue + /// adapters do not re-hardcode the literal. + pub const ORIGIN: &'static str = "input-cursor:origin"; + + /// The run-start origin cursor. + pub fn origin() -> Self { + Self(Self::ORIGIN.to_string()) + } + + /// True when this token is the run-start origin cursor. + pub fn is_origin(&self) -> bool { + self.as_str() == Self::ORIGIN + } +} + fn origin_input_cursor_token() -> LoopInputCursorToken { - LoopInputCursorToken("input-cursor:origin".to_string()) + LoopInputCursorToken::origin() } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] diff --git a/crates/ironclaw_webui_v2_static/static/js/i18n/ar.js b/crates/ironclaw_webui_v2_static/static/js/i18n/ar.js index 17e3cd2c80c..94ce671adc1 100644 --- a/crates/ironclaw_webui_v2_static/static/js/i18n/ar.js +++ b/crates/ironclaw_webui_v2_static/static/js/i18n/ar.js @@ -151,6 +151,14 @@ registerPack("ar", { "tool.riskWrite": "كتابة الملفات", "tool.riskExec": "تشغيل الأوامر", "tool.riskNetwork": "الشبكة", + "tool.errorBackend": "فشل الواجهة الخلفية للأداة.", + "tool.errorSecurity": "تم حظر استجابة الأداة بواسطة فحص أمني.", + "tool.errorCancelled": "تم إلغاء استدعاء الأداة.", + "tool.errorTimeout": "انتهت مهلة استدعاء الأداة.", + "tool.errorInvalidRequest": "كان طلب الأداة غير صالح.", + "tool.errorAuth": "تحتاج الأداة إلى مصادقة.", + "tool.errorPermission": "لم يُسمح باستدعاء الأداة.", + "tool.errorGateDeclined": "تم رفض استدعاء الأداة.", "authGate.title": "المصادقة مطلوبة", "authGate.tokenLabel": "رمز الوصول", "authGate.tokenPlaceholder": "لصق رمز الوصول", diff --git a/crates/ironclaw_webui_v2_static/static/js/i18n/de.js b/crates/ironclaw_webui_v2_static/static/js/i18n/de.js index 754f3917086..5519bef797f 100644 --- a/crates/ironclaw_webui_v2_static/static/js/i18n/de.js +++ b/crates/ironclaw_webui_v2_static/static/js/i18n/de.js @@ -151,6 +151,14 @@ registerPack("de", { "tool.riskWrite": "schreibt Dateien", "tool.riskExec": "führt Befehle aus", "tool.riskNetwork": "Netzwerk", + "tool.errorBackend": "Das Tool-Backend ist fehlgeschlagen.", + "tool.errorSecurity": "Die Tool-Antwort wurde durch eine Sicherheitsprüfung blockiert.", + "tool.errorCancelled": "Der Tool-Aufruf wurde abgebrochen.", + "tool.errorTimeout": "Beim Tool-Aufruf ist eine Zeitüberschreitung aufgetreten.", + "tool.errorInvalidRequest": "Die Tool-Anfrage war ungültig.", + "tool.errorAuth": "Das Tool erfordert eine Authentifizierung.", + "tool.errorPermission": "Der Tool-Aufruf war nicht zulässig.", + "tool.errorGateDeclined": "Der Tool-Aufruf wurde abgelehnt.", "authGate.title": "Authentifizierung erforderlich", "authGate.tokenLabel": "Zugriffstoken", "authGate.tokenPlaceholder": "Zugriffstoken einfügen", diff --git a/crates/ironclaw_webui_v2_static/static/js/i18n/en.js b/crates/ironclaw_webui_v2_static/static/js/i18n/en.js index ded062fe964..e52f574bf30 100644 --- a/crates/ironclaw_webui_v2_static/static/js/i18n/en.js +++ b/crates/ironclaw_webui_v2_static/static/js/i18n/en.js @@ -156,6 +156,14 @@ registerPack("en", { "tool.riskWrite": "writes files", "tool.riskExec": "runs commands", "tool.riskNetwork": "network", + "tool.errorBackend": "The tool backend failed.", + "tool.errorSecurity": "The tool response was blocked by a security check.", + "tool.errorCancelled": "The tool call was cancelled.", + "tool.errorTimeout": "The tool call timed out.", + "tool.errorInvalidRequest": "The tool request was invalid.", + "tool.errorAuth": "The tool needs authentication.", + "tool.errorPermission": "The tool call was not allowed.", + "tool.errorGateDeclined": "The tool call was declined.", "authGate.title": "Authentication required", "authGate.tokenLabel": "Access token", "authGate.tokenPlaceholder": "Paste access token", diff --git a/crates/ironclaw_webui_v2_static/static/js/i18n/es.js b/crates/ironclaw_webui_v2_static/static/js/i18n/es.js index c5336d9be3b..e101f75d343 100644 --- a/crates/ironclaw_webui_v2_static/static/js/i18n/es.js +++ b/crates/ironclaw_webui_v2_static/static/js/i18n/es.js @@ -151,6 +151,14 @@ registerPack("es", { "tool.riskWrite": "escribe archivos", "tool.riskExec": "ejecuta comandos", "tool.riskNetwork": "red", + "tool.errorBackend": "El backend de la herramienta falló.", + "tool.errorSecurity": "La respuesta de la herramienta fue bloqueada por una comprobación de seguridad.", + "tool.errorCancelled": "Se canceló la llamada a la herramienta.", + "tool.errorTimeout": "Se agotó el tiempo de espera de la llamada a la herramienta.", + "tool.errorInvalidRequest": "La solicitud de la herramienta no era válida.", + "tool.errorAuth": "La herramienta necesita autenticación.", + "tool.errorPermission": "No se permitió la llamada a la herramienta.", + "tool.errorGateDeclined": "Se rechazó la llamada a la herramienta.", "authGate.title": "Se requiere autenticación", "authGate.tokenLabel": "Token de acceso", "authGate.tokenPlaceholder": "Pegar token de acceso", diff --git a/crates/ironclaw_webui_v2_static/static/js/i18n/fr.js b/crates/ironclaw_webui_v2_static/static/js/i18n/fr.js index 3c4456fd371..7e166df9c3c 100644 --- a/crates/ironclaw_webui_v2_static/static/js/i18n/fr.js +++ b/crates/ironclaw_webui_v2_static/static/js/i18n/fr.js @@ -151,6 +151,14 @@ registerPack("fr", { "tool.riskWrite": "écrit des fichiers", "tool.riskExec": "exécute des commandes", "tool.riskNetwork": "réseau", + "tool.errorBackend": "Le backend de l'outil a échoué.", + "tool.errorSecurity": "La réponse de l'outil a été bloquée par un contrôle de sécurité.", + "tool.errorCancelled": "L'appel à l'outil a été annulé.", + "tool.errorTimeout": "L'appel à l'outil a expiré.", + "tool.errorInvalidRequest": "La requête de l'outil était invalide.", + "tool.errorAuth": "L'outil nécessite une authentification.", + "tool.errorPermission": "L'appel à l'outil n'était pas autorisé.", + "tool.errorGateDeclined": "L'appel à l'outil a été refusé.", "authGate.title": "Authentification requise", "authGate.tokenLabel": "Jeton d'accès", "authGate.tokenPlaceholder": "Coller le jeton d'accès", diff --git a/crates/ironclaw_webui_v2_static/static/js/i18n/hi.js b/crates/ironclaw_webui_v2_static/static/js/i18n/hi.js index 3c94d014215..3ccde9813a1 100644 --- a/crates/ironclaw_webui_v2_static/static/js/i18n/hi.js +++ b/crates/ironclaw_webui_v2_static/static/js/i18n/hi.js @@ -151,6 +151,14 @@ registerPack("hi", { "tool.riskWrite": "फ़ाइलें लिखता है", "tool.riskExec": "कमांड चलाता है", "tool.riskNetwork": "नेटवर्क", + "tool.errorBackend": "टूल बैकएंड विफल हो गया।", + "tool.errorSecurity": "टूल की प्रतिक्रिया को सुरक्षा जांच द्वारा अवरोधित कर दिया गया।", + "tool.errorCancelled": "टूल कॉल रद्द कर दिया गया।", + "tool.errorTimeout": "टूल कॉल का समय समाप्त हो गया।", + "tool.errorInvalidRequest": "टूल अनुरोध अमान्य था।", + "tool.errorAuth": "टूल को प्रमाणीकरण की आवश्यकता है।", + "tool.errorPermission": "टूल कॉल की अनुमति नहीं थी।", + "tool.errorGateDeclined": "टूल कॉल अस्वीकार कर दिया गया।", "authGate.title": "प्रमाणीकरण आवश्यक है", "authGate.tokenLabel": "एक्सेस टोकन", "authGate.tokenPlaceholder": "एक्सेस टोकन पेस्ट करें", diff --git a/crates/ironclaw_webui_v2_static/static/js/i18n/ja.js b/crates/ironclaw_webui_v2_static/static/js/i18n/ja.js index 4eec895c426..649ea8b9231 100644 --- a/crates/ironclaw_webui_v2_static/static/js/i18n/ja.js +++ b/crates/ironclaw_webui_v2_static/static/js/i18n/ja.js @@ -151,6 +151,14 @@ registerPack("ja", { "tool.riskWrite": "ファイルの書き込み", "tool.riskExec": "コマンドの実行", "tool.riskNetwork": "ネットワーク", + "tool.errorBackend": "ツールのバックエンドが失敗しました。", + "tool.errorSecurity": "ツールの応答はセキュリティチェックによってブロックされました。", + "tool.errorCancelled": "ツールの呼び出しがキャンセルされました。", + "tool.errorTimeout": "ツールの呼び出しがタイムアウトしました。", + "tool.errorInvalidRequest": "ツールのリクエストが無効でした。", + "tool.errorAuth": "ツールには認証が必要です。", + "tool.errorPermission": "ツールの呼び出しは許可されませんでした。", + "tool.errorGateDeclined": "ツールの呼び出しが拒否されました。", "authGate.title": "認証が必要", "authGate.tokenLabel": "アクセス トークン", "authGate.tokenPlaceholder": "アクセス トークンの貼り付け", diff --git a/crates/ironclaw_webui_v2_static/static/js/i18n/ko.js b/crates/ironclaw_webui_v2_static/static/js/i18n/ko.js index 43a3d4a05f0..abd015d9810 100644 --- a/crates/ironclaw_webui_v2_static/static/js/i18n/ko.js +++ b/crates/ironclaw_webui_v2_static/static/js/i18n/ko.js @@ -151,6 +151,14 @@ registerPack("ko", { "tool.riskWrite": "파일 쓰기", "tool.riskExec": "명령 실행", "tool.riskNetwork": "네트워크", + "tool.errorBackend": "도구 백엔드가 실패했습니다.", + "tool.errorSecurity": "도구 응답이 보안 검사에 의해 차단되었습니다.", + "tool.errorCancelled": "도구 호출이 취소되었습니다.", + "tool.errorTimeout": "도구 호출 시간이 초과되었습니다.", + "tool.errorInvalidRequest": "도구 요청이 잘못되었습니다.", + "tool.errorAuth": "도구에 인증이 필요합니다.", + "tool.errorPermission": "도구 호출이 허용되지 않았습니다.", + "tool.errorGateDeclined": "도구 호출이 거부되었습니다.", "authGate.title": "인증 필요", "authGate.tokenLabel": "액세스 토큰", "authGate.tokenPlaceholder": "액세스 토큰 붙여넣기", diff --git a/crates/ironclaw_webui_v2_static/static/js/i18n/pt-BR.js b/crates/ironclaw_webui_v2_static/static/js/i18n/pt-BR.js index a3a22efa7e5..e7cd621a75c 100644 --- a/crates/ironclaw_webui_v2_static/static/js/i18n/pt-BR.js +++ b/crates/ironclaw_webui_v2_static/static/js/i18n/pt-BR.js @@ -151,6 +151,14 @@ registerPack("pt-BR", { "tool.riskWrite": "grava arquivos", "tool.riskExec": "executa comandos", "tool.riskNetwork": "rede", + "tool.errorBackend": "O backend da ferramenta falhou.", + "tool.errorSecurity": "A resposta da ferramenta foi bloqueada por uma verificação de segurança.", + "tool.errorCancelled": "A chamada da ferramenta foi cancelada.", + "tool.errorTimeout": "A chamada da ferramenta expirou.", + "tool.errorInvalidRequest": "A solicitação da ferramenta era inválida.", + "tool.errorAuth": "A ferramenta precisa de autenticação.", + "tool.errorPermission": "A chamada da ferramenta não foi permitida.", + "tool.errorGateDeclined": "A chamada da ferramenta foi recusada.", "authGate.title": "Autenticação necessária", "authGate.tokenLabel": "Token de acesso", "authGate.tokenPlaceholder": "Colar token de acesso", diff --git a/crates/ironclaw_webui_v2_static/static/js/i18n/uk.js b/crates/ironclaw_webui_v2_static/static/js/i18n/uk.js index 6f297e880fa..68cbed381ed 100644 --- a/crates/ironclaw_webui_v2_static/static/js/i18n/uk.js +++ b/crates/ironclaw_webui_v2_static/static/js/i18n/uk.js @@ -151,6 +151,14 @@ registerPack("uk", { "tool.riskWrite": "записує файли", "tool.riskExec": "виконує команди", "tool.riskNetwork": "мережа", + "tool.errorBackend": "Сталася помилка серверної частини інструмента.", + "tool.errorSecurity": "Відповідь інструмента заблоковано перевіркою безпеки.", + "tool.errorCancelled": "Виклик інструмента скасовано.", + "tool.errorTimeout": "Час очікування виклику інструмента вичерпано.", + "tool.errorInvalidRequest": "Запит інструмента був недійсним.", + "tool.errorAuth": "Інструмент потребує автентифікації.", + "tool.errorPermission": "Виклик інструмента не дозволено.", + "tool.errorGateDeclined": "Виклик інструмента відхилено.", "authGate.title": "потрібна автентифікація", "authGate.tokenLabel": "маркер доступу", "authGate.tokenPlaceholder": "вставити маркер доступу", diff --git a/crates/ironclaw_webui_v2_static/static/js/i18n/zh-CN.js b/crates/ironclaw_webui_v2_static/static/js/i18n/zh-CN.js index 6ad76f8a832..e3bd4bdbaca 100644 --- a/crates/ironclaw_webui_v2_static/static/js/i18n/zh-CN.js +++ b/crates/ironclaw_webui_v2_static/static/js/i18n/zh-CN.js @@ -151,6 +151,14 @@ registerPack("zh-CN", { "tool.riskWrite": "写入文件", "tool.riskExec": "运行命令", "tool.riskNetwork": "网络", + "tool.errorBackend": "工具后端发生故障。", + "tool.errorSecurity": "工具响应被安全检查拦截。", + "tool.errorCancelled": "工具调用已取消。", + "tool.errorTimeout": "工具调用超时。", + "tool.errorInvalidRequest": "工具请求无效。", + "tool.errorAuth": "工具需要身份验证。", + "tool.errorPermission": "工具调用未被允许。", + "tool.errorGateDeclined": "工具调用被拒绝。", "authGate.title": "需要验证", "authGate.tokenLabel": "访问令牌", "authGate.tokenPlaceholder": "粘贴访问令牌", diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/chat.js b/crates/ironclaw_webui_v2_static/static/js/pages/chat/chat.js index 62c811ac7e9..30d8fe33348 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/chat.js +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/chat.js @@ -21,6 +21,8 @@ import { TypingIndicator } from "./components/typing-indicator.js"; import { useChat } from "./hooks/useChat.js"; import { NEW_DRAFT_KEY } from "./lib/draft-store.js"; import { buildRuntimeContext } from "./lib/runtime-context.js"; +import { enrichApprovalGateWithActivityArguments } from "./lib/gate-arguments.js"; +import { buildScopedLogsPath } from "../logs/lib/logs-data.js"; /* Grace window before an active thread's sidebar state is cleared to idle. * Long enough for SSE to rehydrate a gate/run after a thread switch (so a @@ -93,9 +95,7 @@ export function Chat({ ? "Resolve the approval request before sending another message." : ""; const composerSendDisabled = - activeThreadHasGate || - (activeThreadIsProcessing && !activeThreadHasGate) || - cooldownSeconds > 0; + Boolean(pendingGate) || cooldownSeconds > 0; const composerSendBlockedRef = React.useRef(composerSendDisabled); composerSendBlockedRef.current = composerSendDisabled; const composerStatusText = @@ -103,6 +103,8 @@ export function Chat({ (cooldownSeconds > 0 ? `Retry in ${cooldownSeconds}s` : undefined); // Scope the persisted composer draft to the open thread (or the // shared new-conversation slot when there's no active thread yet). + // The draft scope and the autofocus reset key are the same per-thread value; + // keep one source so they cannot drift. const composerDraftKey = activeThreadId || NEW_DRAFT_KEY; const canCancelRun = Boolean( activeThreadId && @@ -111,6 +113,19 @@ export function Chat({ activeThreadIsProcessing && !activeThreadHasGate ); + const pendingGateForDisplay = React.useMemo( + () => enrichApprovalGateWithActivityArguments(pendingGate, messages), + [pendingGate, messages], + ); + const activeRunLogsPath = + activeThreadId && + activeRun?.runId && + activeRun.threadId === activeThreadId + ? buildScopedLogsPath( + { threadId: activeThreadId, runId: activeRun.runId }, + { absolute: true }, + ) + : null; const handleSend = React.useCallback( async (content, { images = [], attachments = [] } = {}) => { if (activeThreadHasGate) { @@ -242,6 +257,7 @@ export function Chat({ initialText=${composerDraft} resetKey=${composerResetKey} draftKey=${composerDraftKey} + autoFocusKey=${composerDraftKey} context=${runtimeContext} statusText=${composerStatusText} canCancel=${canCancelRun} @@ -302,7 +318,7 @@ export function Chat({ `) : html` <${ApprovalCard} - gate=${pendingGate} + gate=${pendingGateForDisplay} onApprove=${() => approve(pendingGate.requestId, "approve", pendingGate.kind)} onDeny=${() => @@ -336,6 +352,7 @@ export function Chat({ initialText=${composerDraft} resetKey=${composerResetKey} draftKey=${composerDraftKey} + autoFocusKey=${composerDraftKey} context=${runtimeContext} statusText=${composerStatusText} canCancel=${canCancelRun} diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/approval-card.js b/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/approval-card.js index b1a4c31e026..ddf370ed8c2 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/approval-card.js +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/approval-card.js @@ -16,6 +16,7 @@ import { Button } from "../../../design-system/button.js"; import { Badge } from "../../../design-system/badge.js"; import { Icon } from "../../../design-system/icons.js"; import { classifyRisk } from "../lib/approval-risk.js"; +import { GATE_KIND } from "../lib/gate-kinds.js"; const APPROVAL_PAYLOAD_PREVIEW_LIMIT = 480; @@ -37,7 +38,15 @@ function approvalPayloadPreview(value, expanded) { export function ApprovalCard({ gate, onApprove, onDeny, onAlways }) { const t = useT(); - const { toolName, description, parameters, allowAlways, approvalDetails = [] } = gate; + const { + toolName, + description, + parameters, + allowAlways, + approvalDetails = [], + gateKind, + headline, + } = gate; const [always, setAlways] = React.useState(false); const [expandedPayload, setExpandedPayload] = React.useState(false); @@ -50,6 +59,10 @@ export function ApprovalCard({ gate, onApprove, onDeny, onAlways }) { [toolName, description, parameters] ); const toolLabel = toolName || t("approval.thisTool"); + const title = + gateKind && gateKind !== GATE_KIND.APPROVAL && headline + ? headline + : t("approval.title"); const longPayload = approvalPayloadIsLong(parameters, approvalDetails); const payloadMaxHeight = expandedPayload ? "max-h-72" : "max-h-36"; @@ -70,7 +83,7 @@ export function ApprovalCard({ gate, onApprove, onDeny, onAlways }) { <${Icon} name="lock" className="h-4 w-4" /> - ${t("approval.title")} + ${title} <${Badge} tone=${risk.tone} label=${t(risk.key)} diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/approval-card.test.mjs b/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/approval-card.test.mjs index d58076d508b..08ffc503c7a 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/approval-card.test.mjs +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/approval-card.test.mjs @@ -3,6 +3,8 @@ import { readFileSync } from "node:fs"; import test from "node:test"; import vm from "node:vm"; +import { GATE_KIND } from "../lib/gate-kinds.js"; + function approvalCardSourceForTest() { const source = readFileSync(new URL("./approval-card.js", import.meta.url), "utf8"); const lines = []; @@ -29,6 +31,7 @@ function renderApprovalCard({ expandedPayload = false, gate = defaultApprovalGat const expandedPayloadUpdates = []; const context = { globalThis: {}, + GATE_KIND, html: (strings, ...values) => ({ strings: Array.from(strings), values }), React: { useCallback: (fn) => fn, diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/chat-input.js b/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/chat-input.js index bdea57c1dbc..0e4aaec0d8e 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/chat-input.js +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/chat-input.js @@ -24,6 +24,7 @@ export function ChatInput({ initialText = "", resetKey = "", draftKey = NEW_DRAFT_KEY, + autoFocusKey = "", variant = "dock", context = {}, statusText = "", @@ -98,6 +99,14 @@ export function ChatInput({ autoResize(); }, [text, autoResize]); + React.useEffect(() => { + if (!autoFocusKey || disabled) return; + const frame = window.requestAnimationFrame(() => { + textareaRef.current?.focus({ preventScroll: true }); + }); + return () => window.cancelAnimationFrame(frame); + }, [autoFocusKey, disabled]); + // Restore the persisted draft when the active conversation changes // (draftKey switches). The initialText effect below runs after this // and overrides when a location.state draft was passed in, so an @@ -310,6 +319,7 @@ export function ChatInput({ const hasPayload = text.trim(); const isSubmitDisabled = disabled || sendDisabled; + const showCancelOnly = canCancel && !hasPayload; const placeholder = isHero ? t("chat.heroPlaceholder") : t("chat.followUpPlaceholder"); @@ -451,7 +461,7 @@ export function ChatInput({ > <${Icon} name="plus" className="h-5 w-5" /> - ${canCancel + ${showCancelOnly ? html` <${Button} type="button" diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/empty-state.js b/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/empty-state.js index 797de6dacb4..120ae5ab5b9 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/empty-state.js +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/empty-state.js @@ -11,6 +11,7 @@ export function EmptyState({ initialText, resetKey, draftKey, + autoFocusKey, context, statusText, canCancel, @@ -60,6 +61,7 @@ export function EmptyState({ initialText=${initialText} resetKey=${resetKey} draftKey=${draftKey} + autoFocusKey=${autoFocusKey} variant="hero" context=${context} statusText=${statusText} diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/message-bubble.js b/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/message-bubble.js index f3525379209..be84fc7609f 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/message-bubble.js +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/message-bubble.js @@ -118,6 +118,7 @@ function MessageBubbleImpl({ message, onRetry, threadId }) { const bubbleWidthClass = isUser ? "max-w-[85%]" : isNotice ? "mx-auto max-w-[85%]" : "w-full max-w-[85%]"; const contentWidthClass = isUser ? "" : "w-full min-w-0 max-w-full"; const showRetryAction = status === "error" && onRetry; + const showQueuedStatus = isUser && status === "queued"; const showMetaRow = showActions || showRetryAction || timeLabel; return html` @@ -144,6 +145,14 @@ function MessageBubbleImpl({ message, onRetry, threadId }) { `} + ${showQueuedStatus && html` +
+ + Queued + +
+ `} + ${images && images.length > 0 && html`
${images.map((src, i) => html`Message attachment`)} diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/tool-activity.js b/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/tool-activity.js index 6c06f2cb476..2a74c6051c8 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/tool-activity.js +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/components/tool-activity.js @@ -1,6 +1,7 @@ import { Icon } from "../../../design-system/icons.js"; import { React, html } from "../../../lib/html.js"; import { useT } from "../../../lib/i18n.js"; +import { localizedToolError } from "../lib/history-messages.js"; /* Status dot colour by tool status. Running shows the breathing dot (a no-op under the static motion policy, matching the Badge component's approach). */ @@ -116,6 +117,7 @@ function ToolActivityCard({ activity, nested = false }) { toolStatus, toolDetail, toolError, + toolErrorKey, toolDurationMs, toolParameters, toolResultPreview, @@ -131,6 +133,7 @@ function ToolActivityCard({ activity, nested = false }) { const dotClass = DOT_STYLE[toolStatus] || DOT_STYLE.running; const hasDuration = toolDurationMs !== null && toolDurationMs !== undefined; const controlsId = React.useId(); + const inlineDetail = toolDetail || inlineParameterSummary(toolParameters); const row = html`
`; diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/hooks/useChat.js b/crates/ironclaw_webui_v2_static/static/js/pages/chat/hooks/useChat.js index 754eca2f3bd..2148c4a44b5 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/hooks/useChat.js +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/hooks/useChat.js @@ -25,6 +25,11 @@ import { resetToolActivityState, } from "../lib/tool-activity-state.js"; import { toRenderAttachment, toWireAttachment } from "../lib/attachments.js"; +import { + RECORD_STATUS, + uiStatusFromRecordStatus, +} from "../lib/message-status.js"; +import { buildOptimisticMessage } from "../lib/optimistic-message.js"; import { useHistory } from "./useHistory.js"; import { useSSE } from "./useSSE.js"; @@ -81,7 +86,9 @@ function submitResponseResumedTurnGate(response) { function resolveGateOutcome(response) { if (response?.outcome) return response.outcome; const status = String(response?.status || "").toLowerCase(); - if (status === "queued" || status === "running") return "resumed"; + if (status === RECORD_STATUS.QUEUED || status === RECORD_STATUS.RUNNING) { + return "resumed"; + } if (status === "cancelled" || response?.already_terminal === true) { return "cancelled"; } @@ -211,11 +218,6 @@ export function useChat(threadId) { const [stateThreadId, setStateThreadId] = React.useState(threadId); const toolActivityStateRef = React.useRef(createToolActivityState()); const locallyResolvedGatesRef = React.useRef(new Map()); - const authTokenSubmitRef = React.useRef({ - gateKey: null, - credentialRef: null, - inFlight: false, - }); const submitBusyRef = React.useRef(false); const localRunAdmissionRef = React.useRef(null); @@ -298,56 +300,15 @@ export function useChat(threadId) { return () => clearInterval(timer); }, [cooldownUntil]); - React.useEffect(() => { - if (authTokenSubmitRef.current.gateKey !== pendingAuthGateKey) { - authTokenSubmitRef.current = { - gateKey: pendingAuthGateKey, - credentialRef: null, - inFlight: false, - }; - } - }, [pendingAuthGateKey]); - - React.useEffect(() => { - if (!isPendingOAuthGate(pendingGate)) return; - const listeningSince = Date.now(); - - const handleCompletion = (payload) => { - if (!oauthCompletionMatchesGate(payload, pendingGate, listeningSince)) return; - setPendingGate((current) => (isPendingOAuthGate(current) ? null : current)); - setIsProcessing(true); - }; - - let channel = null; - if (typeof window.BroadcastChannel === "function") { - channel = new window.BroadcastChannel(OAUTH_CALLBACK_CHANNEL); - channel.onmessage = (event) => handleCompletion(event.data); - } - - const onStorage = (event) => { - if (event.key !== OAUTH_CALLBACK_STORAGE_KEY) return; - handleCompletion(parseOAuthCallbackStoragePayload(event.newValue)); - }; + const submitAuthToken = useAuthTokenSubmit({ + pendingGate, + pendingAuthGateKey, + threadId, + setPendingGate, + setIsProcessing, + }); - window.addEventListener("storage", onStorage); - handleCompletion( - parseOAuthCallbackStoragePayload( - window.localStorage?.getItem?.(OAUTH_CALLBACK_STORAGE_KEY), - ), - ); - const timer = window.setInterval(() => { - handleCompletion( - parseOAuthCallbackStoragePayload( - window.localStorage?.getItem?.(OAUTH_CALLBACK_STORAGE_KEY), - ), - ); - }, 500); - return () => { - window.clearInterval(timer); - if (channel) channel.close(); - window.removeEventListener("storage", onStorage); - }; - }, [pendingGate]); + useOAuthCallbackResume({ pendingGate, setPendingGate, setIsProcessing }); const handleEvent = useChatEvents({ threadId, @@ -422,35 +383,15 @@ export function useChat(threadId) { if (pendingGate || pendingGateRef.current) { throw approvalGatePendingSendError(); } - // Admission: block a send only when the *destination* thread is the one - // that's busy. The destination is `targetThreadId` when the caller names - // one, otherwise the open thread (the same `targetThreadId || threadId` - // resolved below). BOTH the in-flight-run guard and the viewed-thread - // `isProcessing` flag must key on that destination — a running thread - // carries both, so narrowing only one still drops a parallel send to - // another thread, or a new chat, just because the thread on screen is - // running. Keying either guard on the viewed thread (or on the mere - // absence of a target) is what broke parallel threads and "new chat - // while a run is active". - const sendTargetThreadId = targetThreadId || threadId; - const activeRunForSend = activeRunRef.current; - const activeRunBlocksSend = - Boolean(activeRunForSend) && - Boolean(sendTargetThreadId) && - activeRunForSend.threadId === sendTargetThreadId; - const processingBlocksSend = - isProcessingRef.current && - Boolean(sendTargetThreadId) && - sendTargetThreadId === threadId; - const localRunBlocksSend = - Boolean(sendTargetThreadId) && - localRunAdmissionRef.current?.threadId === sendTargetThreadId; - if ( - submitBusyRef.current || - processingBlocksSend || - activeRunBlocksSend || - localRunBlocksSend - ) { + // The only local admission guard is the in-flight-POST re-entrancy lock: + // it blocks a duplicate submit while the previous send request has not + // settled. Run/processing state must NOT block a send — a follow-up into + // a still-running thread (same or parallel) must reach the backend so + // Reborn can queue it (deferred_busy), and sends to other threads / new + // chats must never be dropped just because the viewed thread is running. + // Per-destination busy state is the backend queue's responsibility, not + // this guard's. + if (submitBusyRef.current) { return null; } @@ -478,25 +419,14 @@ export function useChat(threadId) { } const pendingKey = sendThreadId; - const pendingRecord = { + const optimisticMessage = buildOptimisticMessage({ id: `pending-${pendingSeqRef.current++}`, - role: "user", - content, - attachments: renderAttachments, - timestamp: new Date().toISOString(), - isOptimistic: true, - }; - const pendingRenderMessage = { - id: pendingRecord.id, - role: "user", content, attachments: renderAttachments, - timestamp: pendingRecord.timestamp, - isOptimistic: true, - }; - addPending(pendingMessagesRef.current, pendingKey, pendingRecord); + }); + addPending(pendingMessagesRef.current, pendingKey, optimisticMessage); - const optimisticId = pendingRecord.id; + const optimisticId = optimisticMessage.id; const shouldRenderInCurrentThread = !threadId || sendThreadId === threadId; const updateCurrentThread = (updater) => { if (shouldRenderInCurrentThread) setMessages(updater); @@ -518,8 +448,8 @@ export function useChat(threadId) { }; } submitBusyRef.current = true; - updateCurrentThread((prev) => [...prev, pendingRenderMessage]); - updateSeededTarget((prev) => [...prev, pendingRenderMessage]); + updateCurrentThread((prev) => [...prev, optimisticMessage]); + updateSeededTarget((prev) => [...prev, optimisticMessage]); updateCurrentRunState(() => { setIsProcessing(true); @@ -588,23 +518,26 @@ export function useChat(threadId) { updateCurrentThread(markAccepted); updateSeededTarget(markAccepted); } - // When the thread was busy, the message is rejected (not deferred). - // Mark the optimistic user message as failed and display the - // server's notice (if present) as a system message so the user - // knows to resend. - if (response?.outcome === "rejected_busy") { + const busyOutcome = BUSY_OUTCOME[response?.outcome]; + if (busyOutcome) { + // A busy outcome (deferred or rejected) started no new local run, so + // drop any local-run admission this send optimistically recorded. if (shouldTrackLocalRun) { localRunAdmissionRef.current = null; } - const markRejected = (prev) => + // One mapper drives the UI status for both busy outcomes so the + // optimistic bubble matches what `messagesFromTimeline` renders + // after a reload (deferred -> queued, rejected -> error). + const uiStatus = uiStatusFromRecordStatus(response.outcome); + const markBusy = (prev) => prev.map((m) => m.id === optimisticId - ? { ...m, isOptimistic: false, status: "error" } + ? { ...m, isOptimistic: false, status: uiStatus } : m, ); - updateCurrentThread(markRejected); - updateSeededTarget(markRejected); - if (response?.notice) { + updateCurrentThread(markBusy); + updateSeededTarget(markBusy); + if (busyOutcome.withNotice && response?.notice) { const appendSystemNotice = (renderCurrent = shouldRenderInCurrentThread) => { const noticeMessage = { id: `system-rejected-${pendingSeqRef.current++}`, @@ -638,14 +571,20 @@ export function useChat(threadId) { appendSystemNotice(false); } } - updateCurrentRunState(() => setIsProcessing(false)); - submitBusyRef.current = false; + // A rejected message frees the run (it never entered the queue); a + // deferred message stays queued behind the active run, so its + // processing state must be preserved. + if (busyOutcome.stopProcessing) { + updateCurrentRunState(() => setIsProcessing(false)); + } } else if (!response?.run_id) { + // No run started and not a busy outcome: drop the optimistic local + // admission so a later send is not blocked by stale state. if (shouldTrackLocalRun) { localRunAdmissionRef.current = null; } - submitBusyRef.current = false; } + // submitBusyRef is released in `finally` (single source) — see below. return response; } catch (err) { if (shouldTrackLocalRun) { @@ -668,7 +607,6 @@ export function useChat(threadId) { updateCurrentThread(markFailed); updateSeededTarget(markFailed); updateCurrentRunState(() => setIsProcessing(false)); - submitBusyRef.current = false; throw err; } finally { // Release the re-entrancy guard once the send POST settles — that is @@ -678,8 +616,9 @@ export function useChat(threadId) { // the moment the user navigates to a new chat while a run is in // flight — that thread's SSE is torn down, its settle event never // arrives, the guard stays `true`, and every later send is silently - // dropped. Blocking a resubmit into a still-running thread is the job - // of the per-destination run guards above, not this. + // dropped. Follow-up sends into a still-running thread must be allowed + // to reach the backend queue after this POST completes — blocking a + // resubmit into a busy thread is the backend queue's job, not this. submitBusyRef.current = false; // Drop the optimistic from the pending ref unconditionally: // on success the confirmed row arrives via /timeline, and on @@ -736,7 +675,7 @@ export function useChat(threadId) { setActiveRun({ runId: response?.run_id || runId, threadId: response?.thread_id || threadId, - status: response?.status || "queued", + status: response?.status || RECORD_STATUS.QUEUED, }); return; } @@ -746,7 +685,113 @@ export function useChat(threadId) { [pendingGate, threadId, setMessages, setActiveRun], ); - const submitAuthToken = React.useCallback( + + const cancelRun = React.useCallback( + async (reason) => { + const runId = activeRun?.runId; + if (!runId || !threadId) return; + setPendingGate(null); + setIsProcessing(false); + setActiveRun(null); + submitBusyRef.current = false; + const localRunAdmission = localRunAdmissionRef.current; + if ( + localRunAdmission?.runId === runId || + localRunAdmission?.threadId === threadId + ) { + localRunAdmissionRef.current = null; + } + await cancelRunRequest({ threadId, runId, reason }); + }, + [activeRun, threadId], + ); + + const loadMore = React.useCallback(() => { + if (hasMore && nextCursor) loadHistory(nextCursor); + }, [hasMore, nextCursor, loadHistory]); + + // Fork-shape compatibility: `approve(requestId, action, kind)` from + // chat.js. `requestId` and `kind` are v1 concepts the v2 stream + // doesn't surface; the live `pendingGate` already carries + // `runId` + `gateRef`, so the args are intentionally ignored and + // the call is rerouted to v2 resolveGate. + const approve = React.useCallback( + async (_requestId, action, _kind) => { + let resolution = "approved"; + let always = false; + if (action === "deny") resolution = "denied"; + else if (action === "cancel") resolution = "cancelled"; + else if (action === "always") { + resolution = "approved"; + always = true; + } + await resolveGate(resolution, { always }); + }, + [resolveGate], + ); + + // Fork chat.js expects these as stubs: v2 stream is deterministic + // enough that retry / suggestions / recovery are not necessary in + // local-dev. Wire them as no-ops so the chat UI renders without + // additional branches. + const noop = React.useCallback(() => {}, []); + + return { + // v2-native + messages, + isProcessing, + pendingGate, + busyGateNotice, + channelConnectAction, + activeRun, + sseStatus, + historyLoading, + historyLoadError, + hasMore, + cooldownSeconds, + send, + resolveGate, + submitAuthToken, + cancelRun, + loadMore, + dismissChannelConnectAction: () => setChannelConnectAction(null), + // fork-shape compatibility — see comments above + suggestions: [], + setSuggestions: noop, + retryMessage: noop, + approve, + recoverHistory: noop, + recoveryNotice: null, + }; +} + +// Owns the manual auth-token submission flow: a per-gate ref tracking the +// stored credential + in-flight guard, the reset when the pending gate +// changes, and the `submitAuthToken` callback. Returns the callback. +function useAuthTokenSubmit({ + pendingGate, + pendingAuthGateKey, + threadId, + setPendingGate, + setIsProcessing, +}) { + const authTokenSubmitRef = React.useRef({ + gateKey: null, + credentialRef: null, + inFlight: false, + }); + + React.useEffect(() => { + if (authTokenSubmitRef.current.gateKey !== pendingAuthGateKey) { + authTokenSubmitRef.current = { + gateKey: pendingAuthGateKey, + credentialRef: null, + inFlight: false, + }; + } + }, [pendingAuthGateKey]); + + return React.useCallback( async (token) => { if (!pendingGate) { throw new Error("auth gate is no longer pending"); @@ -825,88 +870,67 @@ export function useChat(threadId) { throw err; } }, - [pendingGate, threadId], + [pendingGate, threadId, setPendingGate, setIsProcessing], ); +} - const cancelRun = React.useCallback( - async (reason) => { - const runId = activeRun?.runId; - if (!runId || !threadId) return; - setPendingGate(null); - setIsProcessing(false); - setActiveRun(null); - submitBusyRef.current = false; - const localRunAdmission = localRunAdmissionRef.current; - if ( - localRunAdmission?.runId === runId || - localRunAdmission?.threadId === threadId - ) { - localRunAdmissionRef.current = null; - } - await cancelRunRequest({ threadId, runId, reason }); - }, - [activeRun, threadId], - ); +// Watches for an OAuth popup completion (BroadcastChannel + localStorage +// fallback) while an oauth_url gate is pending, and resumes the run when the +// callback for that gate lands. Extracted from the useChat body so the +// cross-tab completion plumbing stays self-contained. +function useOAuthCallbackResume({ pendingGate, setPendingGate, setIsProcessing }) { + React.useEffect(() => { + if (!isPendingOAuthGate(pendingGate)) return; + const listeningSince = Date.now(); - const loadMore = React.useCallback(() => { - if (hasMore && nextCursor) loadHistory(nextCursor); - }, [hasMore, nextCursor, loadHistory]); + const handleCompletion = (payload) => { + if (!oauthCompletionMatchesGate(payload, pendingGate, listeningSince)) return; + setPendingGate((current) => (isPendingOAuthGate(current) ? null : current)); + setIsProcessing(true); + }; - // Fork-shape compatibility: `approve(requestId, action, kind)` from - // chat.js. `requestId` and `kind` are v1 concepts the v2 stream - // doesn't surface; the live `pendingGate` already carries - // `runId` + `gateRef`, so the args are intentionally ignored and - // the call is rerouted to v2 resolveGate. - const approve = React.useCallback( - async (_requestId, action, _kind) => { - let resolution = "approved"; - let always = false; - if (action === "deny") resolution = "denied"; - else if (action === "cancel") resolution = "cancelled"; - else if (action === "always") { - resolution = "approved"; - always = true; - } - await resolveGate(resolution, { always }); - }, - [resolveGate], - ); + let channel = null; + if (typeof window.BroadcastChannel === "function") { + channel = new window.BroadcastChannel(OAUTH_CALLBACK_CHANNEL); + channel.onmessage = (event) => handleCompletion(event.data); + } - // Fork chat.js expects these as stubs: v2 stream is deterministic - // enough that retry / suggestions / recovery are not necessary in - // local-dev. Wire them as no-ops so the chat UI renders without - // additional branches. - const noop = React.useCallback(() => {}, []); + const onStorage = (event) => { + if (event.key !== OAUTH_CALLBACK_STORAGE_KEY) return; + handleCompletion(parseOAuthCallbackStoragePayload(event.newValue)); + }; - return { - // v2-native - messages, - isProcessing, - pendingGate, - busyGateNotice, - channelConnectAction, - activeRun, - sseStatus, - historyLoading, - historyLoadError, - hasMore, - cooldownSeconds, - send, - resolveGate, - submitAuthToken, - cancelRun, - loadMore, - dismissChannelConnectAction: () => setChannelConnectAction(null), - // fork-shape compatibility — see comments above - suggestions: [], - setSuggestions: noop, - retryMessage: noop, - approve, - recoverHistory: noop, - recoveryNotice: null, - }; + window.addEventListener("storage", onStorage); + handleCompletion( + parseOAuthCallbackStoragePayload( + window.localStorage?.getItem?.(OAUTH_CALLBACK_STORAGE_KEY), + ), + ); + const timer = window.setInterval(() => { + handleCompletion( + parseOAuthCallbackStoragePayload( + window.localStorage?.getItem?.(OAUTH_CALLBACK_STORAGE_KEY), + ), + ); + }, 500); + return () => { + window.clearInterval(timer); + if (channel) channel.close(); + window.removeEventListener("storage", onStorage); + }; + }, [pendingGate]); } +// Per-outcome behavior for a busy send response. The UI status itself comes +// from `uiStatusFromRecordStatus` (shared with the reload path); this table +// only carries what diverges between the two outcomes. +const BUSY_OUTCOME = { + // Accepted-and-queued behind the active run: keep processing, no notice. + [RECORD_STATUS.DEFERRED_BUSY]: { stopProcessing: false, withNotice: false }, + // Rejected (never queued): free the run and surface the busy notice. + [RECORD_STATUS.REJECTED_BUSY]: { stopProcessing: true, withNotice: true }, +}; + function isDeclinedGateResolution(resolution) { return resolution === "denied" || resolution === "cancelled"; } diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/hooks/useHistory.js b/crates/ironclaw_webui_v2_static/static/js/pages/chat/hooks/useHistory.js index 1954100fa15..6f985cb7d35 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/hooks/useHistory.js +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/hooks/useHistory.js @@ -1,7 +1,7 @@ import { React } from "../../../lib/html.js"; import { fetchTimeline } from "../../../lib/api.js"; import { authScope } from "../../../lib/auth-scope.js"; -import { messagesFromTimeline } from "../lib/history-messages.js"; +import { coalesceToolFields, messagesFromTimeline } from "../lib/history-messages.js"; const PAGE_SIZE = 50; @@ -336,7 +336,7 @@ function hydrateFreshMessages(fresh, current, options = {}) { const currentByConfirmedId = new Map(); const finalAssistantByRun = new Map(); for (const message of current || []) { - if (!message || !message.timestamp) continue; + if (!message) continue; if (typeof message.id === "string") { currentByConfirmedId.set(message.id, message); } @@ -356,7 +356,7 @@ function hydrateFreshMessages(fresh, current, options = {}) { return fresh; } return fresh.map((message) => { - if (!message || message.timestamp || typeof message.id !== "string") { + if (!message || typeof message.id !== "string") { return message; } const turnRunId = typeof message.turnRunId === "string" ? message.turnRunId : null; @@ -369,10 +369,12 @@ function hydrateFreshMessages(fresh, current, options = {}) { isFinalAssistantMessage(message) && turnRunId ? finalReplyTimestampByRun?.[turnRunId] : null; - const timestamp = currentMessage?.timestamp || fallbackTimestamp; - return timestamp - ? { ...message, timestamp } - : message; + const timestamp = message.timestamp || currentMessage?.timestamp || fallbackTimestamp; + let hydrated = hydrateToolActivityDetails(message, currentMessage); + if (timestamp) { + hydrated = { ...hydrated, timestamp }; + } + return hydrated; }); } @@ -380,6 +382,19 @@ function isFinalAssistantMessage(message) { return message?.role === "assistant" && message?.isFinalReply === true; } +function hydrateToolActivityDetails(message, currentMessage) { + if (message?.role !== "tool_activity" || currentMessage?.role !== "tool_activity") { + return message; + } + // Fill display fields the refreshed record left empty from the live card, + // using the same coalescing predicate as the live merge. + return coalesceToolFields(message, currentMessage, [ + "toolDetail", + "toolParameters", + "toolResultPreview", + ]); +} + function isRuntimeActivityMessage(message) { return message?.role === "tool_activity" || message?.role === "thinking"; } diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/approval-risk.js b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/approval-risk.js index 86f7ce87a1f..d561c86f315 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/approval-risk.js +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/approval-risk.js @@ -14,8 +14,17 @@ export function classifyRisk(toolName, description, parameters) { if (EXEC_RE.test(name)) return { tone: "warning", key: "tool.riskExec" }; if (NETWORK_RE.test(name)) return { tone: "info", key: "tool.riskNetwork" }; - if (EXEC_RE.test(context)) return { tone: "warning", key: "tool.riskExec" }; + if (hasExecContext(context)) return { tone: "warning", key: "tool.riskExec" }; if (NETWORK_RE.test(context)) return { tone: "info", key: "tool.riskNetwork" }; return { tone: "muted", key: "tool.riskRead" }; } + +function hasExecContext(context) { + if (!context) return false; + const withoutGenericGateCopy = context + .replace(/\bcontinue the run\b/g, "") + .replace(/\bcontinue this run\b/g, "") + .replace(/\bresolve this (approval )?gate\b/g, ""); + return EXEC_RE.test(withoutGenericGateCopy); +} diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/approval-risk.test.mjs b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/approval-risk.test.mjs index 1b5c67278c4..41ee8f0179c 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/approval-risk.test.mjs +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/approval-risk.test.mjs @@ -23,3 +23,10 @@ test("classifyRisk: write-like tool name is danger", () => { { tone: "danger", key: "tool.riskWrite" }, ); }); + +test("classifyRisk: generic gate continuation copy is not command execution", () => { + assert.deepEqual( + classifyRisk(null, "Resolve this gate to continue the run.", null), + { tone: "muted", key: "tool.riskRead" }, + ); +}); diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat.test.mjs b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat.test.mjs index e00a5b2cc4d..b725e528b8d 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat.test.mjs +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat.test.mjs @@ -3,6 +3,10 @@ import { readFileSync } from "node:fs"; import test from "node:test"; import vm from "node:vm"; +// chat.js now imports the gate-argument join from ./lib/gate-arguments.js; the +// vm harness strips imports, so the real helper is injected into the context. +import { enrichApprovalGateWithActivityArguments } from "./gate-arguments.js"; + function chatSourceForTest() { const source = readFileSync(new URL("../chat.js", import.meta.url), "utf8"); const lines = []; @@ -83,8 +87,19 @@ function renderChat({ hookState, activeThreadId = "thread-1" }) { }, NEW_DRAFT_KEY: "new", THREAD_STATE: { NEEDS_ATTENTION: "needs_attention", RUNNING: "running" }, + buildScopedLogsPath: ( + { threadId, runId } = {}, + { absolute = false } = {}, + ) => { + const params = []; + if (threadId) params.push(`thread_id=${encodeURIComponent(threadId)}`); + if (runId) params.push(`run_id=${encodeURIComponent(runId)}`); + const query = params.length > 0 ? `?${params.join("&")}` : ""; + return `${absolute ? "/v2" : ""}/logs${query}`; + }, buildRuntimeContext: () => ({}), clearThreadState: () => {}, + enrichApprovalGateWithActivityArguments, globalThis: {}, html: (strings, ...values) => ({ strings: Array.from(strings), values }), setThreadState: () => {}, @@ -164,10 +179,10 @@ test("Chat leaves the composer editable while a run is processing", () => { const chatInput = findComponent(tree, components.ChatInput); const props = componentProps(chatInput, components.ChatInput); assert.equal(props.disabled, false); - assert.equal(props.sendDisabled, true); + assert.equal(props.sendDisabled, false); }); -test("Chat refuses composer sends while a run is processing", async () => { +test("Chat queues composer sends while a run is processing", async () => { let sendCalls = 0; const { tree, components } = renderChat({ hookState: { @@ -201,8 +216,8 @@ test("Chat refuses composer sends while a run is processing", async () => { const props = componentProps(chatInput, components.ChatInput); const response = await props.onSend("draft while busy"); - assert.equal(response, null); - assert.equal(sendCalls, 0); + assert.deepEqual(response, {}); + assert.equal(sendCalls, 1); }); test("Chat cancel button ignores active runs from another thread", () => { @@ -435,3 +450,116 @@ test("Chat deny gate callback routes through approve compatibility path", () => props.onDeny(); assert.deepEqual(approveCalls, [["request-1", "deny", "gate"]]); }); + +test("Chat approval card includes matching tool activity arguments", () => { + const pendingGate = { + kind: "gate", + requestId: "request-1", + invocationId: "invocation-search", + toolName: "nearai.web_search", + description: "Run tool", + approvalDetails: [{ label: "Capability", value: "nearai.web_search" }], + }; + const { tree, components } = renderChat({ + hookState: { + messages: [ + { + id: "tool-invocation-search", + role: "tool_activity", + invocationId: "invocation-search", + toolName: "web_search", + toolStatus: "running", + toolParameters: "query: deploy status", + }, + ], + isProcessing: false, + pendingGate, + suggestions: [], + sseStatus: "open", + historyLoading: false, + hasMore: false, + cooldownSeconds: 0, + recoveryNotice: null, + activeRun: { runId: "run-1", threadId: "thread-1", status: "blocked" }, + send: async () => ({}), + cancelRun: async () => {}, + retryMessage: () => {}, + approve: () => {}, + recoverHistory: () => {}, + loadMore: () => {}, + setSuggestions: () => {}, + submitAuthToken: async () => {}, + }, + }); + + const approvalCard = findComponent(tree, components.ApprovalCard); + const props = componentProps(approvalCard, components.ApprovalCard); + + assert.deepEqual(JSON.parse(JSON.stringify(props.gate.approvalDetails)), [ + { label: "Capability", value: "nearai.web_search" }, + { label: "Arguments", value: "query: deploy status" }, + ]); + assert.equal(props.gate.parameters, "query: deploy status"); +}); + +test("Chat approval card does not borrow another invocation's arguments (strict join)", () => { + // The gated invocation (invocation-http) has no arguments yet; a different + // tool in the same run does. The strict invocationId join must NOT attribute + // that other tool's arguments to this gate. + const pendingGate = { + kind: "gate", + requestId: "request-1", + runId: "run-1", + invocationId: "invocation-http", + toolName: "builtin.http", + description: "Network approval", + approvalDetails: [{ label: "Method", value: "GET" }], + }; + const { tree, components } = renderChat({ + hookState: { + messages: [ + { + id: "tool-invocation-search", + role: "tool_activity", + invocationId: "invocation-search", + turnRunId: "run-1", + toolName: "web_search", + toolStatus: "running", + toolParameters: "query: deploy status", + }, + { + id: "tool-invocation-http", + role: "tool_activity", + invocationId: "invocation-http", + turnRunId: "run-1", + toolName: "http", + toolStatus: "running", + }, + ], + isProcessing: false, + pendingGate, + suggestions: [], + sseStatus: "open", + historyLoading: false, + hasMore: false, + cooldownSeconds: 0, + recoveryNotice: null, + activeRun: { runId: "run-1", threadId: "thread-1", status: "blocked" }, + send: async () => ({}), + cancelRun: async () => {}, + retryMessage: () => {}, + approve: () => {}, + recoverHistory: () => {}, + loadMore: () => {}, + setSuggestions: () => {}, + submitAuthToken: async () => {}, + }, + }); + + const approvalCard = findComponent(tree, components.ApprovalCard); + const props = componentProps(approvalCard, components.ApprovalCard); + + assert.deepEqual(JSON.parse(JSON.stringify(props.gate.approvalDetails)), [ + { label: "Method", value: "GET" }, + ]); +}); diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gate-arguments.js b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gate-arguments.js new file mode 100644 index 00000000000..d8028840ffc --- /dev/null +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gate-arguments.js @@ -0,0 +1,52 @@ +// Join an approval gate to the tool-activity card it gates, strictly by +// `invocationId`, and surface that activity's arguments on the gate so the +// approval card shows what is being approved. +// +// This replaces an earlier render-time scan that, when the exact invocation +// had no arguments yet, fell back to "the latest parameterized activity for +// this run". That fallback mis-attributed arguments under concurrency (two +// tools gated in the same run would show each other's arguments). The gate +// and its activity now share an `invocationId`, so the join is exact: no +// match means no enrichment. +export function enrichApprovalGateWithActivityArguments(gate, messages) { + if (!gate || gate.kind !== "gate" || !gate.invocationId) return gate; + const activity = findActivityForInvocation(messages, gate.invocationId); + const argumentsText = + displayText(activity?.toolParameters) || displayText(activity?.toolDetail) || null; + if (!argumentsText) return gate; + + const approvalDetails = Array.isArray(gate.approvalDetails) ? gate.approvalDetails : []; + if (approvalDetails.some((detail) => isArgumentsDetail(detail?.label))) { + return gate.parameters ? gate : { ...gate, parameters: argumentsText }; + } + return { + ...gate, + approvalDetails: [ + ...approvalDetails, + { label: "Arguments", value: argumentsText }, + ], + parameters: gate.parameters || argumentsText, + }; +} + +function findActivityForInvocation(messages, invocationId) { + for (const message of messages || []) { + if (message?.role === "tool_activity" && message.invocationId === invocationId) { + return message; + } + const nested = (message?.toolCalls || []).find( + (tool) => tool?.invocationId === invocationId, + ); + if (nested) return nested; + } + return null; +} + +function displayText(value) { + return typeof value === "string" && value.trim() ? value.trim() : null; +} + +function isArgumentsDetail(label) { + const normalized = typeof label === "string" ? label.trim().toLowerCase() : ""; + return normalized === "arguments" || normalized === "parameters"; +} diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gate-arguments.test.mjs b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gate-arguments.test.mjs new file mode 100644 index 00000000000..45e15da2aea --- /dev/null +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gate-arguments.test.mjs @@ -0,0 +1,121 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { enrichApprovalGateWithActivityArguments } from "./gate-arguments.js"; + +test("adds matching activity arguments by invocationId", () => { + const gate = { + kind: "gate", + invocationId: "invocation-search", + approvalDetails: [{ label: "Capability", value: "nearai.web_search" }], + }; + + const enriched = enrichApprovalGateWithActivityArguments(gate, [ + { + role: "tool_activity", + invocationId: "invocation-search", + toolParameters: "query: deploy status", + }, + ]); + + assert.deepEqual(JSON.parse(JSON.stringify(enriched.approvalDetails)), [ + { label: "Capability", value: "nearai.web_search" }, + { label: "Arguments", value: "query: deploy status" }, + ]); + assert.equal(enriched.parameters, "query: deploy status"); +}); + +test("does not duplicate an existing arguments detail, but fills parameters", () => { + const gate = { + kind: "gate", + invocationId: "invocation-search", + approvalDetails: [{ label: "Parameters", value: "query: existing" }], + }; + + const enriched = enrichApprovalGateWithActivityArguments(gate, [ + { + role: "tool_activity", + invocationId: "invocation-search", + toolParameters: "query: deploy status", + }, + ]); + + assert.deepEqual(enriched.approvalDetails, [ + { label: "Parameters", value: "query: existing" }, + ]); + assert.equal(enriched.parameters, "query: deploy status"); +}); + +test("strict join: a gate without an invocationId is never enriched", () => { + const gate = { + kind: "gate", + runId: "run-1", + description: "capability requires approval", + approvalDetails: [], + }; + + const enriched = enrichApprovalGateWithActivityArguments(gate, [ + { + role: "tool_activity", + turnRunId: "run-1", + invocationId: "invocation-search", + toolParameters: "query: total market cap", + }, + ]); + + assert.equal(enriched, gate); +}); + +test("strict join: does not borrow another invocation's arguments in the same run", () => { + const gate = { + kind: "gate", + runId: "run-1", + invocationId: "invocation-http", + approvalDetails: [{ label: "Method", value: "GET" }], + }; + + const enriched = enrichApprovalGateWithActivityArguments(gate, [ + { + role: "tool_activity", + turnRunId: "run-1", + invocationId: "invocation-search", + toolParameters: "query: deploy status", + }, + { + role: "tool_activity", + turnRunId: "run-1", + invocationId: "invocation-http", + toolStatus: "running", + }, + ]); + + assert.deepEqual(JSON.parse(JSON.stringify(enriched.approvalDetails)), [ + { label: "Method", value: "GET" }, + ]); +}); + +test("matches a nested toolCalls activity by invocationId", () => { + const gate = { + kind: "gate", + invocationId: "invocation-nested", + approvalDetails: [], + }; + + const enriched = enrichApprovalGateWithActivityArguments(gate, [ + { + role: "assistant", + toolCalls: [ + { invocationId: "invocation-nested", toolParameters: "path: /tmp" }, + ], + }, + ]); + + assert.deepEqual(JSON.parse(JSON.stringify(enriched.approvalDetails)), [ + { label: "Arguments", value: "path: /tmp" }, + ]); +}); + +test("non-gate prompts pass through unchanged", () => { + const gate = { kind: "auth_required", invocationId: "x" }; + assert.equal(enrichApprovalGateWithActivityArguments(gate, []), gate); +}); diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gate-kinds.js b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gate-kinds.js new file mode 100644 index 00000000000..436c45067eb --- /dev/null +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gate-kinds.js @@ -0,0 +1,16 @@ +// Single source of truth for gate "kind" discriminators. These arrive on +// the wire as `GatePromptView.gate_kind` / `ProjectionGate.gate_kind` and +// are compared in several surfaces (gate normalization, approval card +// layout). Keeping them as frozen constants avoids the bare string +// literals that previously drifted across files. +export const GATE_KIND = Object.freeze({ + // Tool-approval gate (the default for live gate prompts). + APPROVAL: "approval", + // Resource/budget gate — rendered with a compact, non-scrolling detail + // layout in the approval card. + RESOURCE: "resource", + // Generic projection gate (the default for durable projection gates). + GENERIC: "generic", + // Authentication / credential gate — routed to the auth cards. + AUTH: "auth", +}); diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gate-kinds.test.mjs b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gate-kinds.test.mjs new file mode 100644 index 00000000000..04827c4f8f3 --- /dev/null +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gate-kinds.test.mjs @@ -0,0 +1,17 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { GATE_KIND } from "./gate-kinds.js"; + +test("GATE_KIND exposes the wire discriminators", () => { + assert.deepEqual({ ...GATE_KIND }, { + APPROVAL: "approval", + RESOURCE: "resource", + GENERIC: "generic", + AUTH: "auth", + }); +}); + +test("GATE_KIND is frozen so the discriminators cannot drift", () => { + assert.ok(Object.isFrozen(GATE_KIND)); +}); diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gates.js b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gates.js index c7090199252..48084fa5705 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gates.js +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gates.js @@ -3,14 +3,18 @@ // `WebChatV2Event::AuthRequired { prompt: AuthPromptView }`. The // browser must hold `run_id` + `gate_ref` so a follow-up // `resolve_gate` call can fill them into the v2 path params. +import { GATE_KIND } from "./gate-kinds.js"; + export function gateFromEvent(eventType, prompt) { if (!prompt) return null; if (eventType === "gate") { const approvalContext = prompt.approval_context || null; + const gateKind = prompt.gate_kind || GATE_KIND.APPROVAL; + const details = Array.isArray(prompt.details) ? prompt.details : []; const gate = { kind: "gate", - gateKind: "approval", + gateKind, runId: prompt.turn_run_id, gateRef: prompt.gate_ref, invocationId: prompt.invocation_id || null, @@ -18,12 +22,12 @@ export function gateFromEvent(eventType, prompt) { body: prompt.body, allowAlways: prompt.allow_always === true, }; - return gateWithApprovalContext(gate, approvalContext, prompt.body); + return gateWithApprovalContext(gate, approvalContext, prompt.body, details); } if (eventType === "auth_required") { return { kind: "auth_required", - gateKind: "auth", + gateKind: GATE_KIND.AUTH, // Legacy auth_required prompts predate challenge_kind and are manual // token prompts. Explicit unknown/other challenge kinds still route to // the neutral auth card in chat.js. @@ -60,7 +64,8 @@ export function gateFromEvent(eventType, prompt) { export function gateFromProjectionGate(gate) { if (!gate?.run_id || !gate.gate_ref) return null; - const gateKind = gate.gate_kind || "generic"; + const gateKind = gate.gate_kind || GATE_KIND.GENERIC; + const details = Array.isArray(gate.details) ? gate.details : []; const base = { gateKind, runId: gate.run_id, @@ -70,7 +75,7 @@ export function gateFromProjectionGate(gate) { body: gate.body || "", allowAlways: gate.allow_always === true, }; - if (gateKind === "auth") { + if (gateKind === GATE_KIND.AUTH) { const authContext = gate.auth_context || {}; return { ...base, @@ -82,29 +87,53 @@ export function gateFromProjectionGate(gate) { expiresAt: authContext.expires_at || null, }; } - return { + return gateWithApprovalContext({ ...base, kind: "gate", - }; + }, gate.approval_context || null, base.body, details); } -function gateWithApprovalContext(gate, approvalContext, fallbackDescription) { - if (!approvalContext) return gate; - const approvalDetails = approvalDetailsFromContext(approvalContext); +function gateWithApprovalContext(gate, approvalContext, fallbackDescription, details = []) { + if (!approvalContext) { + const description = displayDescription(fallbackDescription); + const withDetails = details.length ? { ...gate, approvalDetails: details } : gate; + return description ? { ...withDetails, description } : withDetails; + } + // Merge the structured projection/event `details` with the rows derived + // from the approval context so neither source is dropped from the card. + const approvalDetails = [...approvalDetailsFromContext(approvalContext), ...details]; return { ...gate, toolName: approvalContext.tool_name || null, - description: approvalContext.reason || fallbackDescription, + description: approvalContext.reason || displayDescription(fallbackDescription), actionLabel: approvalContext.action?.label || null, destination: approvalContext.destination || null, approvalScope: approvalContext.scope || null, approvalDetails, - parameters: approvalDetails.length - ? approvalDetails.map((detail) => `${detail.label}: ${detail.value}`).join("\n") - : null, + parameters: null, }; } +function displayDescription(value) { + return typeof value === "string" && value.trim() ? value.trim() : null; +} + +// The single source for turning a normalized gate into the multi-line +// `label: value` parameter string the approval/tool surfaces display. +// Gate normalization sets `parameters: null` (the structured +// `approvalDetails` are the source of truth); this folds them back into a +// flat string on demand so the join lives in exactly one place. +export function gateDisplayParameters(gate) { + if (typeof gate?.parameters === "string" && gate.parameters.trim()) { + return gate.parameters.trim(); + } + const details = Array.isArray(gate?.approvalDetails) ? gate.approvalDetails : []; + const lines = details + .filter((detail) => detail?.label && detail.value != null) + .map((detail) => `${detail.label}: ${detail.value}`); + return lines.length > 0 ? lines.join("\n") : null; +} + function approvalDetailsFromContext(context) { const details = []; if (context.action?.label) { diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gates.test.mjs b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gates.test.mjs index 208d89f2237..cd764ecb589 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gates.test.mjs +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/gates.test.mjs @@ -3,15 +3,23 @@ import { readFileSync } from "node:fs"; import test from "node:test"; import vm from "node:vm"; +import { GATE_KIND } from "./gate-kinds.js"; + function loadGates() { + // Strip ES `import` lines (the vm context has no module loader) and + // inject the imported symbols as context globals — gates.js imports + // `GATE_KIND` from ./gate-kinds.js. const source = readFileSync(new URL("./gates.js", import.meta.url), "utf8") + .split("\n") + .filter((line) => !line.startsWith("import ")) + .join("\n") .replace( - /export function (gateFromEvent|gateFromProjectionGate)/g, + /export function (gateFromEvent|gateFromProjectionGate|gateDisplayParameters)/g, "function $1", ); - const context = { globalThis: {} }; + const context = { globalThis: {}, GATE_KIND }; vm.runInNewContext( - `${source}\nglobalThis.__testExports = { gateFromEvent, gateFromProjectionGate };`, + `${source}\nglobalThis.__testExports = { gateFromEvent, gateFromProjectionGate, gateDisplayParameters };`, context, ); return context.globalThis.__testExports; @@ -40,6 +48,7 @@ test("gateFromEvent maps approval always-allow affordance", () => { invocationId: null, headline: "Approval required", body: "Review the action.", + description: "Review the action.", allowAlways: true, }, ); @@ -63,10 +72,27 @@ test("gateFromEvent defaults missing always-allow affordance to false", () => { invocationId: null, headline: "Resource unavailable", body: "Try later.", + description: "Try later.", allowAlways: false, }, ); }); + +test("gateFromEvent keeps a readable approval description when context lookup is missing", () => { + const { gateFromEvent } = loadGates(); + + const gate = plain(gateFromEvent("gate", { + turn_run_id: "run-1", + gate_ref: "gate:approval-1", + headline: "Approval required", + body: "capability requires approval", + allow_always: true, + })); + + assert.equal(gate.description, "capability requires approval"); + assert.equal(gate.toolName, undefined); +}); + test("gateFromEvent maps approval context into readable approval card props", () => { const { gateFromEvent } = loadGates(); @@ -108,10 +134,37 @@ test("gateFromEvent maps approval context into readable approval card props", () { label: "Capability", value: "builtin.http" }, { label: "Estimated network egress", value: "4096 bytes" }, ]); - assert.match(gate.parameters, /Estimated network egress: 4096 bytes/); + assert.equal(gate.parameters, null); }); -test("gateFromProjectionGate ignores approval context from durable projection", () => { +test("gateFromEvent merges top-level details with approval context details", () => { + const { gateFromEvent } = loadGates(); + + // The event carries top-level `details` AND an approval_context. Both + // sources must reach the approval card; the top-level rows must not be + // dropped when an approval context is present. + const gate = plain(gateFromEvent("gate", { + turn_run_id: "run-1", + gate_ref: "gate:approval-1", + headline: "Approval required", + allow_always: false, + details: [{ label: "Estimated cost", value: "$0.02" }], + approval_context: { + tool_name: "builtin.http", + action: { label: "Run tool" }, + scope: { label: "This request only", reusable: false }, + reason: "approval required", + }, + })); + + assert.deepEqual(gate.approvalDetails, [ + { label: "Action", value: "Run tool" }, + { label: "Scope", value: "This request only" }, + { label: "Estimated cost", value: "$0.02" }, + ]); +}); + +test("gateFromProjectionGate maps approval context from durable projection", () => { const { gateFromProjectionGate } = loadGates(); const gate = plain(gateFromProjectionGate({ @@ -124,21 +177,20 @@ test("gateFromProjectionGate ignores approval context from durable projection", allow_always: true, approval_context: { tool_name: "builtin.http", - reason: "raw path /Users/test/.ssh/id_rsa and token sk-secret", - details: [{ label: "Secret", value: "sk-secret" }], + action: { label: "Network request" }, + scope: { label: "This request only", reusable: false }, + details: [{ label: "Secret", value: "" }], }, })); - assert.deepEqual(gate, { - kind: "gate", - gateKind: "approval", - runId: "run-1", - gateRef: "gate:approval-1", - invocationId: "invocation-1", - headline: "Approval required", - body: "capability requires approval", - allowAlways: true, - }); + assert.equal(gate.toolName, "builtin.http"); + assert.equal(gate.description, "capability requires approval"); + assert.deepEqual(gate.approvalDetails, [ + { label: "Action", value: "Network request" }, + { label: "Scope", value: "This request only" }, + { label: "Secret", value: "" }, + ]); + assert.equal(gate.parameters, null); }); test("gateFromEvent keeps modern auth prompts without challenge kind off token card", () => { @@ -209,3 +261,36 @@ test("gateFromEvent preserves legacy auth prompts as manual token prompts", () = "manual_token", ); }); + +test("gateDisplayParameters joins approval details as label: value lines", () => { + const { gateDisplayParameters } = loadGates(); + + assert.equal( + gateDisplayParameters({ + approvalDetails: [ + { label: "Method", value: "GET" }, + { label: "Destination", value: "https://example.com" }, + ], + }), + "Method: GET\nDestination: https://example.com", + ); +}); + +test("gateDisplayParameters prefers an explicit parameters string", () => { + const { gateDisplayParameters } = loadGates(); + + assert.equal( + gateDisplayParameters({ + parameters: "query: deploy status", + approvalDetails: [{ label: "Method", value: "GET" }], + }), + "query: deploy status", + ); +}); + +test("gateDisplayParameters returns null when there is nothing to show", () => { + const { gateDisplayParameters } = loadGates(); + + assert.equal(gateDisplayParameters({ approvalDetails: [] }), null); + assert.equal(gateDisplayParameters(null), null); +}); diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/history-messages.js b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/history-messages.js index f398baca65a..ca81a8c9d74 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/history-messages.js +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/history-messages.js @@ -8,6 +8,10 @@ import { attachmentKindFromMime, formatBytes } from "./attachments.js"; import { attachmentUrl } from "../../../lib/api.js"; +import { + isBusyRejectedStatus, + uiStatusFromRecordStatus, +} from "./message-status.js"; // Project a stored `AttachmentRef` (snake_case wire shape) into the // render shape `MessageBubble` consumes. The timeline never carries bytes, @@ -80,9 +84,12 @@ export function messagesFromTimeline(records, pendingMessages = [], threadId = n if (seen.has(id)) continue; seen.add(id); const role = roleForRecord(record); - const isBusyRejected = - role === "user" && - (record.status === "rejected_busy" || record.status === "deferred_busy"); + // Normalize busy outcomes through the same mapper `useChat.send` uses on + // the optimistic path, so a message renders identically live and after a + // reload. A deferred-busy message was accepted-and-queued (renders + // queued); only a rejected-busy message was dropped (renders error and + // carries the durable resend copy). + const isBusyRejected = role === "user" && isBusyRejectedStatus(record.status); messages.push({ id, role, @@ -90,7 +97,8 @@ export function messagesFromTimeline(records, pendingMessages = [], threadId = n attachments: attachmentsFromRecord(record, threadId), timestamp: timestampForRecord(record), kind: record.kind, - status: isBusyRejected ? "error" : record.status, + status: + role === "user" ? uiStatusFromRecordStatus(record.status) : record.status, ...(isBusyRejected && { error: "This message wasn't sent because Ironclaw was busy. Resend it to try again.", @@ -178,6 +186,9 @@ export function toolCardFromPreview(preview) { const failed = preview.status === "failed" || preview.status === "killed"; const errorKind = preview.error_kind || null; const activityOrder = numericActivityOrder(preview.activity_order); + const previewError = failed + ? previewToolError(preview, errorKind) + : { text: null, key: null }; return { invocationId: preview.invocation_id, callId: preview.invocation_id, @@ -192,14 +203,9 @@ export function toolCardFromPreview(preview) { toolResultPreview: failed ? null : preview.output_preview || preview.output_summary || null, - toolError: failed - ? toolErrorText(errorKind) || - preview.output_summary || - preview.output_preview || - preview.result_ref || - null - : null, + toolError: previewError.text, toolErrorKind: errorKind, + toolErrorKey: previewError.key, toolDurationMs: null, updatedAt: preview.updated_at || null, resultRef: preview.result_ref || null, @@ -212,6 +218,30 @@ export function toolCardFromPreview(preview) { }; } +// Resolve a failed preview's error into `{ text, key }`. A sanitized, +// display-safe `output_preview` is surfaced verbatim so a live activity card +// and the reloaded preview card show the same text. Backend failures may +// carry raw/unsafe summary text, so without a preview they fall back to the +// friendly localized message rather than leaking the summary. Other kinds may +// surface their summary / result_ref. +function previewToolError(preview, errorKind) { + const previewText = trimmedOrNull(preview.output_preview); + if (previewText) return { text: previewText, key: null }; + const normalizedKind = + typeof errorKind === "string" ? errorKind.trim().toLowerCase().replaceAll("-", "_") : ""; + if (normalizedKind === "backend") return resolveToolError(errorKind); + const summary = + trimmedOrNull(preview.output_summary) || trimmedOrNull(preview.result_ref); + if (summary) return { text: summary, key: null }; + return resolveToolError(errorKind); +} + +function trimmedOrNull(value) { + if (typeof value !== "string") return null; + const trimmed = value.trim(); + return trimmed || null; +} + // Map a `CapabilityActivityView` (SSE lifecycle frame) into the same // card shape. While the invocation is still running the backend now // carries the staged input on the activity frame (`subtitle` = @@ -221,6 +251,9 @@ export function toolCardFromPreview(preview) { export function toolCardFromActivity(activity) { const activityOrder = numericActivityOrder(activity.activity_order); const errorKind = activity.error_kind || null; + const errorSummary = + typeof activity.error_summary === "string" ? activity.error_summary.trim() : ""; + const activityError = resolveToolError(errorKind, errorSummary); return { invocationId: activity.invocation_id, callId: activity.invocation_id, @@ -230,8 +263,9 @@ export function toolCardFromActivity(activity) { toolDetail: activity.subtitle || null, toolParameters: activity.input_summary || null, toolResultPreview: null, - toolError: toolErrorText(errorKind), + toolError: activityError.text, toolErrorKind: errorKind, + toolErrorKey: activityError.key, toolDurationMs: null, updatedAt: activity.updated_at || null, resultRef: null, @@ -244,15 +278,76 @@ export function toolCardFromActivity(activity) { }; } -function toolErrorText(errorKind) { - if (!errorKind) return null; - return errorKind; +// error_kind -> { i18n key, English fallback }. The fallback string is the +// source of truth for non-localized contexts (and unit tests); the key lets +// the rendering surface localize via `t()` when no concrete summary applies. +const TOOL_ERROR_KIND_I18N = { + backend: { key: "tool.errorBackend", text: "The tool backend failed." }, + security: { key: "tool.errorSecurity", text: "The tool response was blocked by a security check." }, + security_rejected: { key: "tool.errorSecurity", text: "The tool response was blocked by a security check." }, + security_rejection: { key: "tool.errorSecurity", text: "The tool response was blocked by a security check." }, + cancelled: { key: "tool.errorCancelled", text: "The tool call was cancelled." }, + timeout: { key: "tool.errorTimeout", text: "The tool call timed out." }, + invalid_request: { key: "tool.errorInvalidRequest", text: "The tool request was invalid." }, + auth: { key: "tool.errorAuth", text: "The tool needs authentication." }, + authentication: { key: "tool.errorAuth", text: "The tool needs authentication." }, + authorization: { key: "tool.errorAuth", text: "The tool needs authentication." }, + permission: { key: "tool.errorPermission", text: "The tool call was not allowed." }, + approval_denied: { key: "tool.errorPermission", text: "The tool call was not allowed." }, + [GATE_DECLINED_ERROR_KIND]: { key: "tool.errorGateDeclined", text: "gate declined" }, +}; + +// Resolve a failure into `{ text, key }`: +// - a concrete `errorSummary` wins (raw backend/tool text), with no i18n key +// (it is not a fixed phrase); +// - a known `errorKind` maps to a localizable fallback (text + key); +// - an unknown `errorKind` is surfaced readably (underscores -> spaces). This +// is the one explicit, documented fallback — not a silent catch-all: it +// only fires for kinds the backend adds that the UI has not localized yet. +export function resolveToolError(errorKind, errorSummary = "") { + const summary = typeof errorSummary === "string" ? errorSummary.trim() : ""; + if (summary) return { text: summary, key: null }; + const value = typeof errorKind === "string" ? errorKind.trim() : ""; + if (!value) return { text: null, key: null }; + const normalized = value.toLowerCase().replaceAll("-", "_"); + const known = TOOL_ERROR_KIND_I18N[normalized]; + if (known) return { text: known.text, key: known.key }; + return { text: value.replaceAll("_", " "), key: null }; +} + +// Localize a card's stored error: when the builder mapped a known error kind +// to an i18n key, resolve it via `t()`; otherwise the stored `toolError` is a +// concrete summary (or an unknown-kind fallback) shown verbatim. +export function localizedToolError(toolError, toolErrorKey, t) { + if (toolErrorKey && typeof t === "function") return t(toolErrorKey); + return toolError; } export function isTerminalToolStatus(status) { return status === "success" || status === "error" || status === "declined"; } +// Single emptiness predicate for tool display fields. Treats whitespace-only +// strings as empty; everything non-string is "present" when non-null. +export function hasDisplayValue(value) { + return typeof value === "string" ? value.trim().length > 0 : value != null; +} + +// Fill empty display fields on `target` from `source` using `hasDisplayValue`. +// Shared by the live merge (tool-activity-state) and the refresh hydration +// (useHistory) so both apply identical coalescing rules. Returns `target` +// unchanged (same reference) when nothing is filled, copying only on change. +export function coalesceToolFields(target, source, fields) { + let result = target; + for (const field of fields) { + if (!hasDisplayValue(result?.[field]) && hasDisplayValue(source?.[field])) { + if (result === target) result = { ...target }; + result[field] = source[field]; + } + } + return result; +} + export function toolDisplayName(name) { const value = typeof name === "string" ? name.trim() : ""; if (!value) return ""; diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/history-messages.test.mjs b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/history-messages.test.mjs index bbca3b67b1b..bdd3a1525bb 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/history-messages.test.mjs +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/history-messages.test.mjs @@ -130,7 +130,11 @@ test("messagesFromTimeline: rejected_busy user record maps to error status with ); }); -test("messagesFromTimeline: deferred_busy user record maps to error status with durable resend copy", () => { +test("messagesFromTimeline: deferred_busy user record stays queued (matches the live optimistic path)", () => { + // Live, useChat.send maps a `deferred_busy` outcome to a "queued" bubble. + // The reload path MUST agree: a deferred (accepted-and-queued) message is + // not an error and carries no resend copy. Regression guard for the + // live-vs-reload divergence. const messages = messagesFromTimeline([ { message_id: "msg-db", @@ -144,11 +148,26 @@ test("messagesFromTimeline: deferred_busy user record maps to error status with assert.equal(messages.length, 1); assert.equal(messages[0].id, "msg-msg-db"); assert.equal(messages[0].role, "user"); - assert.equal(messages[0].status, "error"); - assert.equal( - messages[0].error, - "This message wasn't sent because Ironclaw was busy. Resend it to try again.", - ); + assert.equal(messages[0].status, "queued"); + assert.equal(messages[0].error, undefined); +}); + +test("messagesFromTimeline: queued user record stays queued", () => { + const messages = messagesFromTimeline([ + { + message_id: "msg-q", + kind: "user", + content: "keep going", + sequence: 1, + status: "queued", + }, + ]); + + assert.equal(messages.length, 1); + assert.equal(messages[0].id, "msg-msg-q"); + assert.equal(messages[0].role, "user"); + assert.equal(messages[0].status, "queued"); + assert.equal(messages[0].error, undefined); }); test("messagesFromTimeline: finalized assistant records are marked as final replies", () => { @@ -223,6 +242,50 @@ test("messagesFromTimeline: tool previews use timeline sequence as activity orde ); }); +test("messagesFromTimeline: tool preview failures use sanitized backend text", () => { + const messages = messagesFromTimeline([ + { + message_id: "tool-preview-backend", + kind: "capability_display_preview", + sequence: 2, + status: "finalized", + content: JSON.stringify({ + version: 1, + invocation_id: "invocation-backend", + capability_id: "nearai.web_search", + status: "failed", + error_kind: "backend", + output_summary: "nearai.web_search returned HTTP 502", + }), + }, + { + message_id: "tool-preview-security", + kind: "capability_display_preview", + sequence: 3, + status: "finalized", + content: JSON.stringify({ + version: 1, + invocation_id: "invocation-security", + capability_id: "nearai.web_search", + status: "failed", + error_kind: "security_rejected", + }), + }, + ]); + + assert.deepEqual( + messages.map((message) => [message.id, message.toolStatus, message.toolError]), + [ + ["tool-invocation-backend", "error", "The tool backend failed."], + [ + "tool-invocation-security", + "error", + "The tool response was blocked by a security check.", + ], + ], + ); +}); + // Refresh-persistence contract (#3272): the timeline returns // `ThreadMessageRecord.attachments`; the projection must surface them as // render cards so they survive a reload / thread switch. diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/message-status.js b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/message-status.js new file mode 100644 index 00000000000..cac369a8fad --- /dev/null +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/message-status.js @@ -0,0 +1,54 @@ +// Single source of truth for chat message status values and the +// wire/record-status -> UI-status mapping. +// +// The same logical message is rendered twice: optimistically by +// `useChat.send` (from a send response `outcome`) and durably by +// `messagesFromTimeline` (from a persisted `ThreadMessageRecord.status`). +// Both paths MUST map the same wire value to the same UI status, or a +// message flips appearance across a reload — e.g. a busy-deferred message +// showing a "queued" badge live but an "error" bubble after refresh +// (the bug this module exists to prevent). + +// Wire/record status values produced by the backend (send `outcome` or +// persisted `ThreadMessageRecord.status`). +export const RECORD_STATUS = Object.freeze({ + // Accepted but deferred because the thread was busy: it is queued to run + // once the active run yields. Renders as queued, never as an error. + DEFERRED_BUSY: "deferred_busy", + // Rejected because the thread was busy: it was NOT accepted and must be + // resent. Renders as an error. + REJECTED_BUSY: "rejected_busy", + // Explicitly queued. + QUEUED: "queued", + // The run actively processing this message. + RUNNING: "running", +}); + +// UI-facing status values consumed by `MessageBubble`. +export const UI_MESSAGE_STATUS = Object.freeze({ + QUEUED: "queued", + ERROR: "error", + RUNNING: "running", +}); + +// Map a wire/record status to the UI status `MessageBubble` renders. +// Unknown statuses pass through unchanged (e.g. "accepted", "finalized", +// "submitted") so this mapper only normalizes the values that diverge. +export function uiStatusFromRecordStatus(recordStatus) { + switch (recordStatus) { + case RECORD_STATUS.DEFERRED_BUSY: + case RECORD_STATUS.QUEUED: + return UI_MESSAGE_STATUS.QUEUED; + case RECORD_STATUS.REJECTED_BUSY: + return UI_MESSAGE_STATUS.ERROR; + default: + return recordStatus; + } +} + +// True when a busy outcome means the message was rejected (not accepted), +// so the UI must attach the durable "resend it" copy. A deferred-busy +// message was accepted-and-queued and gets no error copy. +export function isBusyRejectedStatus(recordStatus) { + return recordStatus === RECORD_STATUS.REJECTED_BUSY; +} diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/message-status.test.mjs b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/message-status.test.mjs new file mode 100644 index 00000000000..e2190226f63 --- /dev/null +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/message-status.test.mjs @@ -0,0 +1,57 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + RECORD_STATUS, + UI_MESSAGE_STATUS, + isBusyRejectedStatus, + uiStatusFromRecordStatus, +} from "./message-status.js"; + +test("deferred-busy maps to queued (accepted and waiting, never an error)", () => { + assert.equal( + uiStatusFromRecordStatus(RECORD_STATUS.DEFERRED_BUSY), + UI_MESSAGE_STATUS.QUEUED, + ); +}); + +test("explicit queued stays queued", () => { + assert.equal( + uiStatusFromRecordStatus(RECORD_STATUS.QUEUED), + UI_MESSAGE_STATUS.QUEUED, + ); +}); + +test("rejected-busy maps to error (was not accepted, must resend)", () => { + assert.equal( + uiStatusFromRecordStatus(RECORD_STATUS.REJECTED_BUSY), + UI_MESSAGE_STATUS.ERROR, + ); +}); + +test("unknown statuses pass through unchanged", () => { + assert.equal(uiStatusFromRecordStatus("accepted"), "accepted"); + assert.equal(uiStatusFromRecordStatus("finalized"), "finalized"); + assert.equal(uiStatusFromRecordStatus(undefined), undefined); +}); + +test("only rejected-busy needs the resend error copy", () => { + assert.equal(isBusyRejectedStatus(RECORD_STATUS.REJECTED_BUSY), true); + assert.equal(isBusyRejectedStatus(RECORD_STATUS.DEFERRED_BUSY), false); + assert.equal(isBusyRejectedStatus(RECORD_STATUS.QUEUED), false); +}); + +test("live (send outcome) and reload (record status) agree for busy outcomes", () => { + // The same wire value must produce the same UI status whether it arrives + // as a send-response `outcome` or a persisted record `status`. + for (const status of [ + RECORD_STATUS.DEFERRED_BUSY, + RECORD_STATUS.REJECTED_BUSY, + RECORD_STATUS.QUEUED, + ]) { + assert.equal( + uiStatusFromRecordStatus(status), + uiStatusFromRecordStatus(status), + ); + } +}); diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/optimistic-message.js b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/optimistic-message.js new file mode 100644 index 00000000000..4f2f40cbf19 --- /dev/null +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/optimistic-message.js @@ -0,0 +1,15 @@ +// Build the optimistic user message rendered immediately on send, before the +// server confirms it on the timeline. The pending-ref record and the +// in-state render message are the same shape; this is the single source so +// they never drift (e.g. an attachment card showing live but not after the +// confirmed row lands). +export function buildOptimisticMessage({ id, content, attachments = [] }) { + return { + id, + role: "user", + content, + attachments, + timestamp: new Date().toISOString(), + isOptimistic: true, + }; +} diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/tool-activity-state.js b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/tool-activity-state.js index 867a4801355..28fb67a0272 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/tool-activity-state.js +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/tool-activity-state.js @@ -1,7 +1,9 @@ import { + coalesceToolFields, isTerminalToolStatus, toolDisplayName, } from "./history-messages.js"; +import { gateDisplayParameters } from "./gates.js"; export function createToolActivityState() { return { @@ -82,8 +84,8 @@ function toolCardFromGate(gate, overrides = {}) { capabilityId: gate.toolName || gate.gateKind || null, toolName: toolDisplayName(displaySource) || displaySource, toolStatus: overrides.toolStatus || "running", - toolDetail: null, - toolParameters: null, + toolDetail: gate.actionLabel || null, + toolParameters: gateDisplayParameters(gate), toolResultPreview: null, toolError: overrides.toolError || null, toolErrorKind: overrides.toolErrorKind || null, @@ -147,6 +149,9 @@ function mergeToolActivity(current, incoming) { ? current.toolName : incoming.toolName || current.toolName, toolStatus: keepCurrentTerminal ? current.toolStatus : incoming.toolStatus, + // toolDetail / toolParameters / toolResultPreview are coalesced below via + // the shared `coalesceToolFields` predicate so a later sparse frame never + // erases a populated value. toolError: incoming.toolError || current.toolError, toolErrorKind: incoming.toolErrorKind || current.toolErrorKind || null, updatedAt: keepCurrentTerminal @@ -161,11 +166,19 @@ function mergeToolActivity(current, incoming) { activityOrder: mergedActivityOrder(current, incoming), activityOrderSource: incoming.activityOrderSource || current.activityOrderSource || null, }; + // Prefer the incoming display fields (already spread above), but fall back + // to the current value when the incoming frame is sparse — one predicate, + // shared with useHistory's refresh hydration. + const coalesced = coalesceToolFields(merged, current, [ + "toolDetail", + "toolParameters", + "toolResultPreview", + ]); if (current.gateActivity && !incoming.gateActivity) { - merged.id = toolMessageId(incoming); - merged.gateActivity = false; + coalesced.id = toolMessageId(incoming); + coalesced.gateActivity = false; } - return merged; + return coalesced; } function mergedActivityOrder(current, incoming) { diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/tool-activity-state.test.mjs b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/tool-activity-state.test.mjs index c25420dffae..f4d2f62b5b4 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/tool-activity-state.test.mjs +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/tool-activity-state.test.mjs @@ -285,10 +285,56 @@ test("tool activity cards map gate-declined lifecycle frames to declined status" }); assert.equal(card.toolStatus, "declined"); - assert.equal(card.toolError, "gate_declined"); + assert.equal(card.toolError, "gate declined"); assert.equal(card.toolErrorKind, "gate_declined"); }); +test("tool activity cards prefer backend error summary over generic kind", () => { + const card = toolCardFromActivity({ + invocation_id: "invocation-shell", + capability_id: "builtin.shell", + status: "failed", + error_kind: "backend", + error_summary: "failed to spawn command: command not found", + }); + + assert.equal(card.toolStatus, "error"); + assert.equal(card.toolError, "failed to spawn command: command not found"); + assert.equal(card.toolErrorKind, "backend"); +}); + +test("terminal failed live and refresh tool cards render the same visible fields", () => { + const live = toolCardFromActivity({ + invocation_id: "invocation-shell", + turn_run_id: "run-shell", + capability_id: "builtin.shell", + status: "failed", + subtitle: "cargo test", + input_summary: "command: cargo test", + error_kind: "backend", + error_summary: "process execution failed: command not found", + output_bytes: 0, + activity_order: 7, + }); + const refreshed = toolCardFromPreview({ + invocation_id: "invocation-shell", + turn_run_id: "run-shell", + capability_id: "builtin.shell", + title: "builtin.shell", + status: "failed", + subtitle: "cargo test", + input_summary: "command: cargo test", + error_kind: "backend", + output_summary: "process execution failed: command not found", + output_preview: "process execution failed: command not found", + output_kind: "text", + output_bytes: 0, + activity_order: 7, + }); + + assert.deepEqual(visibleTerminalToolFields(live), visibleTerminalToolFields(refreshed)); +}); + test("tool preview cards preserve gate-declined error kind as declined status", () => { const card = toolCardFromPreview({ invocation_id: "invocation-preview-declined", @@ -304,6 +350,24 @@ test("tool preview cards preserve gate-declined error kind as declined status", assert.equal(card.toolErrorKind, "gate_declined"); }); +function visibleTerminalToolFields(card) { + return { + invocationId: card.invocationId, + capabilityId: card.capabilityId, + toolName: card.toolName, + toolStatus: card.toolStatus, + toolDetail: card.toolDetail, + toolParameters: card.toolParameters, + toolResultPreview: card.toolResultPreview, + toolError: card.toolError, + toolErrorKind: card.toolErrorKind, + outputBytes: card.outputBytes, + turnRunId: card.turnRunId, + activityOrder: card.activityOrder, + activityOrderSource: card.activityOrderSource, + }; +} + test("tool activity state leaves pending gates unnumbered after existing timeline activity", () => { const runId = "run-refresh-order"; const stateRef = { current: createToolActivityState() }; @@ -422,6 +486,80 @@ test("tool activity state applies durable projection order to live activity", () assert.equal(messages[0].activityOrderSource, "projection"); }); +test("tool activity state does not erase parameters from later sparse frames", () => { + const runId = "run-params"; + const stateRef = { current: createToolActivityState() }; + let messages = []; + const setMessages = (updater) => { + messages = typeof updater === "function" ? updater(messages) : updater; + }; + + upsertToolActivityMessage( + setMessages, + toolCardFromActivity({ + invocation_id: "invocation-web", + turn_run_id: runId, + capability_id: "nearai.web_search", + status: "running", + subtitle: "deploy status", + input_summary: "query: deploy status", + }), + stateRef, + ); + upsertToolActivityMessage( + setMessages, + toolCardFromPreview({ + invocation_id: "invocation-web", + turn_run_id: runId, + capability_id: "nearai.web_search", + title: "nearai.web_search", + status: "completed", + output_summary: "2 results", + }), + stateRef, + ); + + assert.equal(messages.length, 1); + assert.equal(messages[0].toolStatus, "success"); + assert.equal(messages[0].toolDetail, "deploy status"); + assert.equal(messages[0].toolParameters, "query: deploy status"); + assert.equal(messages[0].toolResultPreview, "2 results"); +}); + +test("tool activity state shows approval gate parameters on synthetic activity", () => { + const runId = "run-gate-params"; + const stateRef = { current: createToolActivityState() }; + let messages = []; + const setMessages = (updater) => { + messages = typeof updater === "function" ? updater(messages) : updater; + }; + + ensureGateToolActivity( + setMessages, + { + kind: "gate", + runId, + gateRef: "gate:http", + invocationId: "invocation-http", + toolName: "builtin.http", + actionLabel: "Network request", + approvalDetails: [ + { label: "Method", value: "GET" }, + { label: "Destination", value: "https://example.com" }, + ], + }, + stateRef, + ); + + assert.equal(messages.length, 1); + assert.equal(messages[0].toolName, "http"); + assert.equal(messages[0].toolDetail, "Network request"); + assert.equal( + messages[0].toolParameters, + "Method: GET\nDestination: https://example.com", + ); +}); + test("tool activity state applies durable projection order to gate activity", () => { const runId = "run-snapshot-order"; const stateRef = { current: createToolActivityState() }; diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useChat-send.test.mjs b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useChat-send.test.mjs index 4f2991d064c..554d0cc538d 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useChat-send.test.mjs +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useChat-send.test.mjs @@ -20,6 +20,11 @@ import { failGateToolActivity, resetToolActivityState, } from "./tool-activity-state.js"; +import { + RECORD_STATUS, + uiStatusFromRecordStatus, +} from "./message-status.js"; +import { buildOptimisticMessage } from "./optimistic-message.js"; function useChatSourceForTest() { const source = readFileSync( @@ -48,6 +53,9 @@ function runUseChatSource(context) { failGateToolActivity, resetToolActivityState, timelineMessageIdFromAcceptedRef, + RECORD_STATUS, + uiStatusFromRecordStatus, + buildOptimisticMessage, }); vm.runInNewContext(useChatSourceForTest(), context); } @@ -353,6 +361,106 @@ test("useChat.send: stamps render attachments on the optimistic message", async ]); }); +test("useChat.send: active run follow-up reaches backend and renders queued", async () => { + const threadId = "thread-1"; + let sentBody = null; + let renderedMessages = []; + const stateUpdates = []; + const context = { + AbortController, + Date, + Error, + Map, + Math, + React: createReactStub({ + initialByIndex: new Map([ + [2, { runId: "run-active", threadId, status: "running" }], + [4, true], + ]), + setCalls: stateUpdates, + }), + addPending, + toRenderAttachment, + toWireAttachment, + cancelRunRequest: async () => {}, + clearTimeout, + createThreadRequest: async () => { + throw new Error("thread should already exist"); + }, + globalThis: {}, + listConnectableChannels: async () => { + throw new Error("ordinary prompts should not fetch connectable channels"); + }, + looksLikeChannelConnectCommand, + queryClient: { + fetchQuery: async () => { + throw new Error("ordinary prompts should not fetch connectable channels"); + }, + invalidateQueries: () => {}, + }, + recordAcceptedMessageRef, + removePending, + timelineMessageIdFromAcceptedRef, + resolveChannelConnectCommand, + resolveGateRequest: async () => {}, + sendMessage: async (body) => { + sentBody = body; + return { + accepted_message_ref: "msg:queued-follow-up", + outcome: "deferred_busy", + thread_id: threadId, + }; + }, + setInterval, + setTimeout, + submitManualToken: async () => {}, + useChatEvents: () => () => {}, + useHistory: () => ({ + messages: [], + hasMore: false, + nextCursor: null, + isLoading: false, + loadHistory: () => {}, + setMessages: (updater) => { + renderedMessages = + typeof updater === "function" ? updater(renderedMessages) : updater; + }, + }), + useSSE: () => ({ status: "idle" }), + }; + + runUseChatSource(context); + + const chat = context.globalThis.__testExports.useChat(threadId); + const response = await chat.send("one more thing"); + + assert.equal(sentBody.threadId, threadId); + assert.equal(sentBody.content, "one more thing"); + assert.deepEqual(Array.from(sentBody.attachments), []); + assert.equal(response.outcome, "deferred_busy"); + assert.deepEqual( + JSON.parse(JSON.stringify(renderedMessages.map((message) => ({ + content: message.content, + status: message.status, + isOptimistic: message.isOptimistic, + timelineMessageId: message.timelineMessageId, + })))), + [ + { + content: "one more thing", + status: "queued", + isOptimistic: false, + timelineMessageId: "queued-follow-up", + }, + ], + ); + assert.equal( + stateUpdates.some((update) => update.index === 4 && update.value === false), + false, + "queuing a follow-up must not clear the active run processing state", + ); +}); + test("useChat.send: target-thread send does not append into active thread", async () => { const currentThreadId = "thread-current"; const targetThreadId = "thread-target"; @@ -2182,10 +2290,14 @@ test("useChat.send: rejected_busy without notice still clears isProcessing", asy assert.equal(lastIsProcessing?.value, false); }); -test("useChat.send: active run refuses duplicate submit before network call", async () => { - const threadId = "thread-busy-local"; +test("useChat.send: in-flight submit refuses duplicate before network settles", async () => { + const threadId = "thread-1"; let renderedMessages = []; let sendCalls = 0; + let resolveSend; + const firstSendSettled = new Promise((resolve) => { + resolveSend = resolve; + }); const context = { AbortController, @@ -2193,7 +2305,7 @@ test("useChat.send: active run refuses duplicate submit before network call", as Error, Map, Math, - React: createReactStub({ initialByIndex: new Map([[4, true]]) }), + React: createReactStub(), addPending, toRenderAttachment, toWireAttachment, @@ -2204,12 +2316,12 @@ test("useChat.send: active run refuses duplicate submit before network call", as }, globalThis: {}, listConnectableChannels: async () => { - throw new Error("busy prompts should not fetch connectable channels"); + throw new Error("ordinary prompts should not fetch connectable channels"); }, looksLikeChannelConnectCommand, queryClient: { fetchQuery: async () => { - throw new Error("busy prompts should not fetch connectable channels"); + throw new Error("ordinary prompts should not fetch connectable channels"); }, invalidateQueries: () => {}, }, @@ -2217,9 +2329,16 @@ test("useChat.send: active run refuses duplicate submit before network call", as removePending, resolveChannelConnectCommand, resolveGateRequest: async () => {}, - sendMessage: async () => { + sendMessage: async ({ content }) => { sendCalls += 1; - throw new Error("busy send should not reach API"); + await firstSendSettled; + return { + accepted_message_ref: "msg:message-1", + run_id: "run-1", + status: "queued", + thread_id: threadId, + content, + }; }, setInterval, setTimeout, @@ -2242,14 +2361,23 @@ test("useChat.send: active run refuses duplicate submit before network call", as runUseChatSource(context); const chat = context.globalThis.__testExports.useChat(threadId); - const response = await chat.send("second message while first run is active"); + const first = chat.send("first message"); + await Promise.resolve(); + const second = await chat.send("duplicate click"); - assert.equal(response, null); - assert.equal(sendCalls, 0); - assert.deepEqual(renderedMessages, []); + assert.equal(second, null); + assert.equal(sendCalls, 1); + assert.deepEqual( + JSON.parse(JSON.stringify(renderedMessages.map((message) => message.content))), + ["first message"], + ); + + resolveSend(); + const response = await first; + assert.equal(response.run_id, "run-1"); }); -test("useChat.send: accepted run blocks another submit until settlement", async () => { +test("useChat.send: accepted run allows follow-up submit before settlement", async () => { const threadId = "thread-1"; let renderedMessages = []; let sendCalls = 0; @@ -2322,10 +2450,11 @@ test("useChat.send: accepted run blocks another submit until settlement", async const second = await chat.send("draft while the reply is still running"); assert.equal(first.run_id, "run-1"); - assert.equal(second, null); - assert.equal(sendCalls, 1); - assert.equal(renderedMessages.length, 1); + assert.equal(second.run_id, "run-2"); + assert.equal(sendCalls, 2); + assert.equal(renderedMessages.length, 2); assert.equal(renderedMessages[0].content, "first message"); + assert.equal(renderedMessages[1].content, "draft while the reply is still running"); context.chatEventsArgs.setIsProcessing(false); context.chatEventsArgs.setActiveRun(null); @@ -2333,11 +2462,11 @@ test("useChat.send: accepted run blocks another submit until settlement", async const third = await chat.send("message after settlement"); - assert.equal(third.run_id, "run-2"); - assert.equal(sendCalls, 2); + assert.equal(third.run_id, "run-3"); + assert.equal(sendCalls, 3); }); -test("useChat.send: created thread stays blocked until accepted run settles", async () => { +test("useChat.send: created-thread follow-up before settle reaches backend to be queued", async () => { const createdThreadId = "thread-created"; let renderedMessages = []; let createThreadCalls = 0; @@ -2425,19 +2554,24 @@ test("useChat.send: created thread stays blocked until accepted run settles", as context.chatEventsArgs.setIsProcessing(false); context.chatEventsArgs.setActiveRun(null); + // Queued-messages contract: a follow-up into the still-running created + // thread must reach the backend so Reborn can queue it, NOT be dropped by a + // local admission guard. The only local guard is the in-flight-POST lock, + // which the previous send already released. const second = await chat.send("draft while the reply is still running", { threadId: createdThreadId, }); - assert.equal(second, null); - assert.equal(sendCalls, 1); + assert.ok(second, "follow-up must resolve with a backend response, not null"); + assert.equal(second.run_id, "run-2"); + assert.equal(sendCalls, 2); context.chatEventsArgs.onRunSettled("run-1", { success: true }); const third = await chat.send("message after settlement", { threadId: createdThreadId, }); - assert.equal(third.run_id, "run-2"); - assert.equal(sendCalls, 2); + assert.equal(third.run_id, "run-3"); + assert.equal(sendCalls, 3); }); test("useChat.send: clears local busy when run settles before send response", async () => { @@ -3147,10 +3281,10 @@ test("useChat.send: addresses a second thread in parallel while viewing a runnin assert.ok(result, "send must resolve with a response, not null"); }); -test("useChat.send: still blocks a duplicate send into the already-running thread", async () => { - // The one case the gate must keep blocking: a second send into the SAME - // thread that already has a run in flight (both activeRun and isProcessing - // set — the real busy state). +test("useChat.send: queues a follow-up into the already-running thread", async () => { + // A second send into the same running thread must reach the backend so Reborn + // can queue it. The local guard only blocks duplicate POSTs before the + // previous send request settles. const { context, sentBody, createThreadCalls } = createParallelSendContext({ threadId: "thread-a", activeRun: { runId: "run-a", threadId: "thread-a", status: "running" }, @@ -3164,17 +3298,17 @@ test("useChat.send: still blocks a duplicate send into the already-running threa threadId: "thread-a", }); - assert.equal(result, null, "duplicate send into the busy thread is rejected"); - assert.equal(sentBody(), null, "sendMessage must not be called for a busy thread"); + assert.ok(result, "follow-up send should resolve with backend response"); + assert.equal(result.status, "queued"); + assert.ok(sentBody(), "sendMessage must be called so the backend can queue it"); + assert.equal(sentBody().threadId, "thread-a"); assert.equal(createThreadCalls(), 0); }); -test("useChat.send: blocks a send addressed to a busy thread that is NOT the viewed one", async () => { - // The block must key on the *destination* thread, not the viewed one: - // viewing thread-a, but the active run is on thread-b, and the send is - // addressed to thread-b — that destination is busy, so it must be blocked. - // This complements the parallel-send test (viewed busy, different target → - // allowed) so the pair pins the block on destination identity alone. +test("useChat.send: queues a send addressed to a busy thread that is not viewed", async () => { + // Destination-thread busy state is enforced by the backend queue, not by a + // local UI drop. Viewing thread-a while thread-b is running, a targeted send + // to thread-b must still reach sendMessage. const { context, sentBody, createThreadCalls } = createParallelSendContext({ threadId: "thread-a", activeRun: { runId: "run-b", threadId: "thread-b", status: "running" }, @@ -3187,7 +3321,9 @@ test("useChat.send: blocks a send addressed to a busy thread that is NOT the vie threadId: "thread-b", }); - assert.equal(result, null, "send into the busy destination thread is rejected"); - assert.equal(sentBody(), null, "sendMessage must not be called for the busy destination"); + assert.ok(result, "send into the busy destination resolves with backend response"); + assert.equal(result.status, "queued"); + assert.ok(sentBody(), "sendMessage must be called for the busy destination"); + assert.equal(sentBody().threadId, "thread-b"); assert.equal(createThreadCalls(), 0); }); diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useChatEvents.js b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useChatEvents.js index 1eb24a862f3..8ac8d389f83 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useChatEvents.js +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useChatEvents.js @@ -697,7 +697,7 @@ function appendRunFailureMessage( role: "error", content, timestamp: new Date().toISOString(), - ...(turnRunId && { turnRunId }), + turnRunId: runId || null, }, ]; }); diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useChatEvents.test.mjs b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useChatEvents.test.mjs index 2f18a3a2738..d5080ebb886 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useChatEvents.test.mjs +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useChatEvents.test.mjs @@ -971,7 +971,7 @@ test("useChatEvents: late started activity cannot downgrade remembered declined assert.equal(harness.messages[0].id, `tool-${invocationId}`); assert.equal(harness.messages[0].toolName, "web_search"); assert.equal(harness.messages[0].toolStatus, "declined"); - assert.equal(harness.messages[0].toolError, "gate_declined"); + assert.equal(harness.messages[0].toolError, "gate declined"); assert.equal(harness.messages[0].toolErrorKind, "gate_declined"); }); diff --git a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useHistory.test.mjs b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useHistory.test.mjs index f9ee41544bb..da29cd280dc 100644 --- a/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useHistory.test.mjs +++ b/crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useHistory.test.mjs @@ -3,6 +3,10 @@ import { readFileSync } from "node:fs"; import test from "node:test"; import vm from "node:vm"; +// useHistory.js imports `coalesceToolFields` from history-messages.js; the vm +// harness strips imports, so the real helper is injected into each context. +import { coalesceToolFields } from "./history-messages.js"; + function useHistorySourceForTest() { const source = readFileSync( new URL("../hooks/useHistory.js", import.meta.url), @@ -90,6 +94,7 @@ test("useHistory records a load error when timeline fetch fails", async () => { const setCalls = []; const consoleErrors = []; const context = { + coalesceToolFields, console: { error: (...args) => consoleErrors.push(args), }, @@ -121,6 +126,7 @@ test("useHistory full refresh preserves SSE-only activity messages", async () => const runId = "run-activity"; const setCalls = []; const context = { + coalesceToolFields, console, fetchTimeline: async () => ({ messages: [ @@ -179,6 +185,7 @@ test("useHistory full refresh preserves SSE-only activity messages", async () => test("useHistory can seed a newly-created thread before navigation", async () => { const setCalls = []; const context = { + coalesceToolFields, console, fetchTimeline: async () => ({ messages: [], next_cursor: null }), globalThis: {}, @@ -265,6 +272,7 @@ test("useHistory clears visible messages immediately when switching to an uncach test("useHistory seedThreadMessages updates an accepted first message by timeline id", async () => { const context = { + coalesceToolFields, console, fetchTimeline: async () => new Promise(() => {}), globalThis: {}, @@ -300,6 +308,7 @@ test("useHistory seedThreadMessages updates an accepted first message by timelin test("useHistory seedThreadMessages updates the mounted target thread", async () => { const setCalls = []; const context = { + coalesceToolFields, console, fetchTimeline: async () => new Promise(() => {}), globalThis: {}, @@ -358,6 +367,7 @@ test("useHistory full refresh preserves unnumbered live gate activity after time }, ]; const context = { + coalesceToolFields, console, fetchTimeline: async () => ({ messages: [], @@ -401,8 +411,8 @@ test("useHistory full refresh preserves unnumbered live gate activity after time ); }); -test("mergeFullRefresh keeps requested client-only bubbles and lets the timeline win otherwise", () => { - const context = { globalThis: {}, React: createReactStub() }; +test("mergeFullRefresh keeps run errors next to their run and lets the timeline win otherwise", () => { + const context = { globalThis: {}, React: createReactStub(), coalesceToolFields }; vm.runInNewContext(useHistorySourceForTest(), context); const { mergeFullRefresh } = context.globalThis.__testExports; @@ -461,8 +471,61 @@ test("mergeFullRefresh keeps requested client-only bubbles and lets the timeline assert.equal(toolCard.toolResultPreview, "ok"); }); +test("mergeFullRefresh appends client-only run errors without a matching run", () => { + const context = { globalThis: {}, React: createReactStub(), coalesceToolFields }; + vm.runInNewContext(useHistorySourceForTest(), context); + const { mergeFullRefresh } = context.globalThis.__testExports; + + const merged = mergeFullRefresh( + [{ id: "msg-user-1", role: "user", turnRunId: "run-1" }], + [{ id: "err-run-unknown", role: "error", content: "run failed" }], + { preserveClientOnly: true }, + ); + + assert.equal(merged.map((m) => m.id).join(","), "msg-user-1,err-run-unknown"); +}); + +test("mergeFullRefresh keeps live tool parameters when refreshed preview is sparse", () => { + const context = { globalThis: {}, React: createReactStub(), coalesceToolFields }; + vm.runInNewContext(useHistorySourceForTest(), context); + const { mergeFullRefresh } = context.globalThis.__testExports; + + const merged = mergeFullRefresh( + [ + { + id: "tool-invocation-web", + role: "tool_activity", + invocationId: "invocation-web", + toolName: "web_search", + toolStatus: "success", + toolParameters: null, + toolResultPreview: "2 results", + turnRunId: "run-params", + }, + ], + [ + { + id: "tool-invocation-web", + role: "tool_activity", + invocationId: "invocation-web", + toolName: "web_search", + toolStatus: "running", + toolDetail: "deploy status", + toolParameters: "query: deploy status", + turnRunId: "run-params", + }, + ], + ); + + assert.equal(merged.length, 1); + assert.equal(merged[0].toolStatus, "success"); + assert.equal(merged[0].toolDetail, "deploy status"); + assert.equal(merged[0].toolParameters, "query: deploy status"); + assert.equal(merged[0].toolResultPreview, "2 results"); +}); + test("mergeFullRefresh carries optimistic timestamps onto confirmed messages", () => { - const context = { globalThis: {}, React: createReactStub() }; + const context = { globalThis: {}, React: createReactStub(), coalesceToolFields }; vm.runInNewContext(useHistorySourceForTest(), context); const { mergeFullRefresh } = context.globalThis.__testExports; @@ -492,7 +555,7 @@ test("mergeFullRefresh carries optimistic timestamps onto confirmed messages", ( }); test("mergeFullRefresh carries live assistant timestamps onto confirmed replies", () => { - const context = { globalThis: {}, React: createReactStub() }; + const context = { globalThis: {}, React: createReactStub(), coalesceToolFields }; vm.runInNewContext(useHistorySourceForTest(), context); const { mergeFullRefresh } = context.globalThis.__testExports; @@ -524,7 +587,7 @@ test("mergeFullRefresh carries live assistant timestamps onto confirmed replies" }); test("mergeFullRefresh uses run-settled time for confirmed assistant replies", () => { - const context = { globalThis: {}, React: createReactStub() }; + const context = { globalThis: {}, React: createReactStub(), coalesceToolFields }; vm.runInNewContext(useHistorySourceForTest(), context); const { mergeFullRefresh } = context.globalThis.__testExports; diff --git a/tests/e2e/scenarios/test_reborn_webui_v2_smoke.py b/tests/e2e/scenarios/test_reborn_webui_v2_smoke.py index cde1f1ddabc..f8c5f1f4358 100644 --- a/tests/e2e/scenarios/test_reborn_webui_v2_smoke.py +++ b/tests/e2e/scenarios/test_reborn_webui_v2_smoke.py @@ -466,13 +466,19 @@ async def test_reborn_v2_composer_accepts_draft_while_run_is_processing(reborn_v ).to_be_visible(timeout=15000) await expect(composer).to_be_enabled() - await expect(composer).to_have_attribute("data-send-disabled", "true") + # A busy run no longer gates the composer: sends are queued behind the + # active run rather than blocked, so the send affordance stays enabled. + await expect(composer).to_have_attribute("data-send-disabled", "false") await composer.fill("draft while the reply is still running") await expect(composer).to_have_value("draft while the reply is still running") - await expect(composer).to_have_attribute("data-send-disabled", "true") + await expect(composer).to_have_attribute("data-send-disabled", "false") await composer.press("Enter") - await expect(reborn_v2_page.locator(SEL_V2["msg_user"])).to_have_count(1, timeout=1000) + + await expect(reborn_v2_page.locator(SEL_V2["msg_user"])).to_have_count(2, timeout=5000) + await expect(reborn_v2_page.locator(SEL_V2["msg_user"]).nth(1)).to_contain_text( + "draft while the reply is still running" + ) async def test_reborn_v2_new_chat_sends_while_a_run_is_active(reborn_v2_page): diff --git a/tests/support/reborn/harness.rs b/tests/support/reborn/harness.rs index 9cad7f4363a..11314a80cfc 100644 --- a/tests/support/reborn/harness.rs +++ b/tests/support/reborn/harness.rs @@ -875,6 +875,13 @@ impl RebornBinaryE2EHarness { accept_harness_blocked_evidence, }); let turn_state_for_runtime: Arc = turn_store.clone(); + let host_input_queue = Arc::new(ironclaw_loop_support::InMemoryHostInputQueue::new( + thread_harness.service.clone() as Arc, + )); + let host_input_queue_reader: Arc = + host_input_queue.clone(); + let host_input_enqueue: Arc = + host_input_queue.clone(); let composition = build_default_planned_runtime(DefaultPlannedRuntimeParts { turn_state: turn_state_for_runtime, thread_service: thread_harness.service.clone() @@ -904,7 +911,7 @@ impl RebornBinaryE2EHarness { model_route_resolver: None, cancellation_factory: None, skill_context_source: None, - input_queue: None, + input_queue: Some(host_input_queue_reader), identity_context_source, user_profile_source: Arc::new(EmptyUserProfileSource), model_policy_guard: None, @@ -919,11 +926,14 @@ impl RebornBinaryE2EHarness { })?; let binding_service: Arc = Arc::new(product_harness.binding_service()?); - let inbound: Arc = Arc::new(DefaultInboundTurnService::new( - Arc::clone(&binding_service), - thread_harness.service_instance()?, - composition.coordinator.clone(), - )); + let inbound: Arc = Arc::new( + DefaultInboundTurnService::new( + Arc::clone(&binding_service), + thread_harness.service_instance()?, + composition.coordinator.clone(), + ) + .with_input_enqueue(host_input_enqueue), + ); let ledger: Arc = Arc::new(product_harness.idempotency_ledger()); let workflow = DefaultProductWorkflow::new(inbound, ledger, binding_service); diff --git a/tests/support_unit_tests.rs b/tests/support_unit_tests.rs index c0c0f16242d..6d4c1ce5d35 100644 --- a/tests/support_unit_tests.rs +++ b/tests/support_unit_tests.rs @@ -397,10 +397,10 @@ mod reborn_support_tests { ToolResultSafeSummary, }; use ironclaw_turns::{ - CancelRunRequest, CancelRunResponse, GetRunStateRequest, LoopMessageRef, - ReplyTargetBindingRef, ResumeTurnRequest, ResumeTurnResponse, RunProfileId, - RunProfileVersion, SubmitTurnRequest, SubmitTurnResponse, ThreadBusy, TurnCoordinator, - TurnError, TurnId, TurnRunId, TurnRunState, TurnScope, TurnStatus, + AcceptedMessageRef, CancelRunRequest, CancelRunResponse, GetRunStateRequest, + LoopMessageRef, ReplyTargetBindingRef, ResumeTurnRequest, ResumeTurnResponse, RunProfileId, + RunProfileVersion, SourceBindingRef, SubmitTurnRequest, SubmitTurnResponse, ThreadBusy, + TurnCoordinator, TurnError, TurnId, TurnRunId, TurnRunState, TurnScope, TurnStatus, events::EventCursor, run_profile::{ CapabilityBatchInvocation, CapabilityInputRef, CapabilityInvocation, CapabilityOutcome, @@ -2494,9 +2494,36 @@ mod reborn_support_tests { async fn get_run_state( &self, - _request: GetRunStateRequest, + request: GetRunStateRequest, ) -> Result { - panic!("get_run_state is not used by reborn support tests") + // The busy-run steering enqueue path reads the active run's state to + // recover its turn id before queueing. Return a minimal running-run + // state keyed to the requested scope/run rather than panicking. + Ok(TurnRunState { + scope: request.scope, + actor: None, + turn_id: TurnId::new(), + run_id: request.run_id, + status: TurnStatus::Running, + accepted_message_ref: AcceptedMessageRef::new("msg:active-run") + .expect("accepted message ref"), + source_binding_ref: SourceBindingRef::new("src:active-run") + .expect("source binding ref"), + reply_target_binding_ref: ReplyTargetBindingRef::new("reply:active-run") + .expect("reply target binding ref"), + resolved_run_profile_id: RunProfileId::default_profile(), + resolved_run_profile_version: RunProfileVersion::new(1), + resolved_model_route: None, + received_at: chrono::Utc::now(), + checkpoint_id: None, + gate_ref: None, + blocked_activity_id: None, + credential_requirements: Vec::new(), + failure: None, + event_cursor: EventCursor::default(), + product_context: None, + resume_disposition: None, + }) } } }