[Feature]: centralize hosted conversation runtime overrides#1194
Merged
chumyin merged 12 commits intoeastreams:devfrom Apr 17, 2026
Merged
[Feature]: centralize hosted conversation runtime overrides#1194chumyin merged 12 commits intoeastreams:devfrom
chumyin merged 12 commits intoeastreams:devfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a reusable, feature-gated HostedConversationRuntime wrapper and loader helpers ( Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "Tasks CLI"
participant Hosted as "HostedConversationRuntime<R>"
participant Inner as "DefaultConversationRuntime"
participant Spawner as "BackgroundTaskSpawner / DetachedTasksSpawner"
CLI->>Hosted: load_hosted_default_conversation_runtime(config)
Hosted->>Inner: wrap boxed default runtime
CLI->>Hosted: enqueue background task (owner_kind = BackgroundTaskHost)
Hosted->>Spawner: use overridden background_task_spawner.spawn(...)
Spawner-->>Hosted: spawn result
Hosted-->>CLI: return created task (owner_kind = "background_task_host")
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
daemon
2 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/app/src/conversation/runtime.rs`:
- Around line 2198-2225: Add a symmetric test that verifies
with_async_delegate_spawner() overrides the async delegate spawner without
affecting the background task spawner: create a test (e.g.,
hosted_runtime_overrides_async_delegate_spawner_without_changing_background_task_spawner)
mirroring the existing hosted_runtime_overrides_background_task_spawner... test
but call
HostedConversationRuntime::new(inner_runtime).with_async_delegate_spawner(override_async_spawner),
then resolve both hosted_runtime.async_delegate_spawner(&config) and
hosted_runtime.background_task_spawner(&config) and assert Arc::ptr_eq that the
async resolver returns the override_async_spawner while the background resolver
still returns the original inner_background_spawner; use the same types/fixtures
(SpawnerAwareRuntime, LoongClawConfig, NoopTestSpawner) so the test covers the
missing override seam.
- Around line 1479-1668: The HostedConversationRuntime impl must explicitly
delegate build_context to the inner runtime instead of using the trait default;
add a build_context(&self, config: &LoongClawConfig, session_id: &str,
include_system_prompt: bool, tool_view: &ToolView, binding:
ConversationRuntimeBinding<'_>) -> CliResult<AssembledConversationContext>
method on the HostedConversationRuntime impl that simply calls and returns
self.inner.build_context(...) so the inner runtime (e.g.,
DefaultConversationRuntime) preserves its assembled-context (prompt
fragments/metadata) behavior rather than being rebuilt from build_messages().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 224be94c-9632-46d8-ad7d-7ac7ef8dab04
📒 Files selected for processing (3)
crates/app/src/conversation/mod.rscrates/app/src/conversation/runtime.rscrates/daemon/src/tasks_cli.rs
Comment on lines
+1479
to
+1668
| #[cfg(feature = "memory-sqlite")] | ||
| #[async_trait] | ||
| impl<R> ConversationRuntime for HostedConversationRuntime<R> | ||
| where | ||
| R: ConversationRuntime, | ||
| { | ||
| fn session_context( | ||
| &self, | ||
| config: &LoongClawConfig, | ||
| session_id: &str, | ||
| binding: ConversationRuntimeBinding<'_>, | ||
| ) -> CliResult<SessionContext> { | ||
| self.inner.session_context(config, session_id, binding) | ||
| } | ||
|
|
||
| fn tool_view( | ||
| &self, | ||
| config: &LoongClawConfig, | ||
| session_id: &str, | ||
| binding: ConversationRuntimeBinding<'_>, | ||
| ) -> CliResult<ToolView> { | ||
| self.inner.tool_view(config, session_id, binding) | ||
| } | ||
|
|
||
| fn async_delegate_spawner( | ||
| &self, | ||
| config: &LoongClawConfig, | ||
| ) -> Option<Arc<dyn AsyncDelegateSpawner>> { | ||
| let override_spawner = self.async_delegate_spawner_override.clone(); | ||
| match override_spawner { | ||
| Some(override_spawner) => Some(override_spawner), | ||
| None => self.inner.async_delegate_spawner(config), | ||
| } | ||
| } | ||
|
|
||
| fn background_task_spawner( | ||
| &self, | ||
| config: &LoongClawConfig, | ||
| ) -> Option<Arc<dyn AsyncDelegateSpawner>> { | ||
| let override_spawner = self.background_task_spawner_override.clone(); | ||
| match override_spawner { | ||
| Some(override_spawner) => Some(override_spawner), | ||
| None => self.inner.background_task_spawner(config), | ||
| } | ||
| } | ||
|
|
||
| async fn bootstrap( | ||
| &self, | ||
| config: &LoongClawConfig, | ||
| session_id: &str, | ||
| kernel_ctx: &KernelContext, | ||
| ) -> CliResult<ContextEngineBootstrapResult> { | ||
| self.inner.bootstrap(config, session_id, kernel_ctx).await | ||
| } | ||
|
|
||
| async fn ingest( | ||
| &self, | ||
| session_id: &str, | ||
| message: &Value, | ||
| kernel_ctx: &KernelContext, | ||
| ) -> CliResult<ContextEngineIngestResult> { | ||
| self.inner.ingest(session_id, message, kernel_ctx).await | ||
| } | ||
|
|
||
| async fn build_messages( | ||
| &self, | ||
| config: &LoongClawConfig, | ||
| session_id: &str, | ||
| include_system_prompt: bool, | ||
| tool_view: &ToolView, | ||
| binding: ConversationRuntimeBinding<'_>, | ||
| ) -> CliResult<Vec<Value>> { | ||
| self.inner | ||
| .build_messages( | ||
| config, | ||
| session_id, | ||
| include_system_prompt, | ||
| tool_view, | ||
| binding, | ||
| ) | ||
| .await | ||
| } | ||
|
|
||
| async fn request_completion( | ||
| &self, | ||
| config: &LoongClawConfig, | ||
| messages: &[Value], | ||
| binding: ConversationRuntimeBinding<'_>, | ||
| ) -> CliResult<String> { | ||
| self.inner | ||
| .request_completion(config, messages, binding) | ||
| .await | ||
| } | ||
|
|
||
| async fn request_turn( | ||
| &self, | ||
| config: &LoongClawConfig, | ||
| session_id: &str, | ||
| turn_id: &str, | ||
| messages: &[Value], | ||
| tool_view: &ToolView, | ||
| binding: ConversationRuntimeBinding<'_>, | ||
| ) -> CliResult<ProviderTurn> { | ||
| self.inner | ||
| .request_turn(config, session_id, turn_id, messages, tool_view, binding) | ||
| .await | ||
| } | ||
|
|
||
| async fn request_turn_streaming( | ||
| &self, | ||
| config: &LoongClawConfig, | ||
| session_id: &str, | ||
| turn_id: &str, | ||
| messages: &[Value], | ||
| tool_view: &ToolView, | ||
| binding: ConversationRuntimeBinding<'_>, | ||
| on_token: crate::provider::StreamingTokenCallback, | ||
| ) -> CliResult<ProviderTurn> { | ||
| self.inner | ||
| .request_turn_streaming( | ||
| config, session_id, turn_id, messages, tool_view, binding, on_token, | ||
| ) | ||
| .await | ||
| } | ||
|
|
||
| async fn persist_turn( | ||
| &self, | ||
| session_id: &str, | ||
| role: &str, | ||
| content: &str, | ||
| binding: ConversationRuntimeBinding<'_>, | ||
| ) -> CliResult<()> { | ||
| self.inner | ||
| .persist_turn(session_id, role, content, binding) | ||
| .await | ||
| } | ||
|
|
||
| async fn after_turn( | ||
| &self, | ||
| session_id: &str, | ||
| user_input: &str, | ||
| assistant_reply: &str, | ||
| messages: &[Value], | ||
| kernel_ctx: &KernelContext, | ||
| ) -> CliResult<()> { | ||
| self.inner | ||
| .after_turn( | ||
| session_id, | ||
| user_input, | ||
| assistant_reply, | ||
| messages, | ||
| kernel_ctx, | ||
| ) | ||
| .await | ||
| } | ||
|
|
||
| async fn compact_context( | ||
| &self, | ||
| config: &LoongClawConfig, | ||
| session_id: &str, | ||
| messages: &[Value], | ||
| kernel_ctx: &KernelContext, | ||
| ) -> CliResult<()> { | ||
| self.inner | ||
| .compact_context(config, session_id, messages, kernel_ctx) | ||
| .await | ||
| } | ||
|
|
||
| async fn prepare_subagent_spawn( | ||
| &self, | ||
| parent_session_id: &str, | ||
| subagent_session_id: &str, | ||
| kernel_ctx: &KernelContext, | ||
| ) -> CliResult<()> { | ||
| self.inner | ||
| .prepare_subagent_spawn(parent_session_id, subagent_session_id, kernel_ctx) | ||
| .await | ||
| } | ||
|
|
||
| async fn on_subagent_ended( | ||
| &self, | ||
| parent_session_id: &str, | ||
| subagent_session_id: &str, | ||
| kernel_ctx: &KernelContext, | ||
| ) -> CliResult<()> { | ||
| self.inner | ||
| .on_subagent_ended(parent_session_id, subagent_session_id, kernel_ctx) | ||
| .await | ||
| } | ||
| } |
There was a problem hiding this comment.
Delegate build_context explicitly.
HostedConversationRuntime currently falls back to the trait’s default build_context(), so it does not preserve an inner runtime’s custom build_context() behavior. For DefaultConversationRuntime, that means the wrapper rebuilds AssembledConversationContext from build_messages() and drops assembled-context data like prompt fragments/metadata. That breaks the “delegate the full runtime trait” contract.
Proposed fix
#[cfg(feature = "memory-sqlite")]
#[async_trait]
impl<R> ConversationRuntime for HostedConversationRuntime<R>
where
R: ConversationRuntime,
{
@@
async fn ingest(
&self,
session_id: &str,
message: &Value,
kernel_ctx: &KernelContext,
) -> CliResult<ContextEngineIngestResult> {
self.inner.ingest(session_id, message, kernel_ctx).await
}
+
+ async fn build_context(
+ &self,
+ config: &LoongClawConfig,
+ session_id: &str,
+ include_system_prompt: bool,
+ binding: ConversationRuntimeBinding<'_>,
+ ) -> CliResult<AssembledConversationContext> {
+ self.inner
+ .build_context(config, session_id, include_system_prompt, binding)
+ .await
+ }
async fn build_messages(
&self,
config: &LoongClawConfig,
session_id: &str,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/app/src/conversation/runtime.rs` around lines 1479 - 1668, The
HostedConversationRuntime impl must explicitly delegate build_context to the
inner runtime instead of using the trait default; add a build_context(&self,
config: &LoongClawConfig, session_id: &str, include_system_prompt: bool,
tool_view: &ToolView, binding: ConversationRuntimeBinding<'_>) ->
CliResult<AssembledConversationContext> method on the HostedConversationRuntime
impl that simply calls and returns self.inner.build_context(...) so the inner
runtime (e.g., DefaultConversationRuntime) preserves its assembled-context
(prompt fragments/metadata) behavior rather than being rebuilt from
build_messages().
Comment on lines
+2198
to
+2225
| #[cfg(feature = "memory-sqlite")] | ||
| #[test] | ||
| fn hosted_runtime_overrides_background_task_spawner_without_changing_async_delegate_spawner() { | ||
| let config = LoongClawConfig::default(); | ||
| let inner_async_spawner: Arc<dyn AsyncDelegateSpawner> = Arc::new(NoopTestSpawner); | ||
| let inner_background_spawner: Arc<dyn AsyncDelegateSpawner> = Arc::new(NoopTestSpawner); | ||
| let override_background_spawner: Arc<dyn AsyncDelegateSpawner> = Arc::new(NoopTestSpawner); | ||
| let inner_runtime = SpawnerAwareRuntime { | ||
| async_delegate_spawner: Some(inner_async_spawner.clone()), | ||
| background_task_spawner: Some(inner_background_spawner), | ||
| }; | ||
|
|
||
| let hosted_runtime = HostedConversationRuntime::new(inner_runtime) | ||
| .with_background_task_spawner(override_background_spawner.clone()); | ||
|
|
||
| let resolved_async_spawner = hosted_runtime | ||
| .async_delegate_spawner(&config) | ||
| .expect("async delegate spawner"); | ||
| let resolved_background_spawner = hosted_runtime | ||
| .background_task_spawner(&config) | ||
| .expect("background task spawner"); | ||
|
|
||
| assert!(Arc::ptr_eq(&resolved_async_spawner, &inner_async_spawner)); | ||
| assert!(Arc::ptr_eq( | ||
| &resolved_background_spawner, | ||
| &override_background_spawner | ||
| )); | ||
| } |
There was a problem hiding this comment.
Add the symmetric regression for with_async_delegate_spawner.
This only locks down the background override path. with_async_delegate_spawner() can regress independently, so the new hosted wrapper still has an uncovered override seam.
Suggested follow-up test shape
+ #[cfg(feature = "memory-sqlite")]
+ #[test]
+ fn hosted_runtime_overrides_async_delegate_spawner_without_changing_background_task_spawner() {
+ let config = LoongClawConfig::default();
+ let override_async_spawner: Arc<dyn AsyncDelegateSpawner> = Arc::new(NoopTestSpawner);
+ let inner_background_spawner: Arc<dyn AsyncDelegateSpawner> = Arc::new(NoopTestSpawner);
+ let inner_runtime = SpawnerAwareRuntime {
+ async_delegate_spawner: None,
+ background_task_spawner: Some(inner_background_spawner.clone()),
+ };
+
+ let hosted_runtime = HostedConversationRuntime::new(inner_runtime)
+ .with_async_delegate_spawner(override_async_spawner.clone());
+
+ let resolved_async_spawner = hosted_runtime
+ .async_delegate_spawner(&config)
+ .expect("async delegate spawner");
+ let resolved_background_spawner = hosted_runtime
+ .background_task_spawner(&config)
+ .expect("background task spawner");
+
+ assert!(Arc::ptr_eq(&resolved_async_spawner, &override_async_spawner));
+ assert!(Arc::ptr_eq(
+ &resolved_background_spawner,
+ &inner_background_spawner
+ ));
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #[cfg(feature = "memory-sqlite")] | |
| #[test] | |
| fn hosted_runtime_overrides_background_task_spawner_without_changing_async_delegate_spawner() { | |
| let config = LoongClawConfig::default(); | |
| let inner_async_spawner: Arc<dyn AsyncDelegateSpawner> = Arc::new(NoopTestSpawner); | |
| let inner_background_spawner: Arc<dyn AsyncDelegateSpawner> = Arc::new(NoopTestSpawner); | |
| let override_background_spawner: Arc<dyn AsyncDelegateSpawner> = Arc::new(NoopTestSpawner); | |
| let inner_runtime = SpawnerAwareRuntime { | |
| async_delegate_spawner: Some(inner_async_spawner.clone()), | |
| background_task_spawner: Some(inner_background_spawner), | |
| }; | |
| let hosted_runtime = HostedConversationRuntime::new(inner_runtime) | |
| .with_background_task_spawner(override_background_spawner.clone()); | |
| let resolved_async_spawner = hosted_runtime | |
| .async_delegate_spawner(&config) | |
| .expect("async delegate spawner"); | |
| let resolved_background_spawner = hosted_runtime | |
| .background_task_spawner(&config) | |
| .expect("background task spawner"); | |
| assert!(Arc::ptr_eq(&resolved_async_spawner, &inner_async_spawner)); | |
| assert!(Arc::ptr_eq( | |
| &resolved_background_spawner, | |
| &override_background_spawner | |
| )); | |
| } | |
| #[cfg(feature = "memory-sqlite")] | |
| #[test] | |
| fn hosted_runtime_overrides_background_task_spawner_without_changing_async_delegate_spawner() { | |
| let config = LoongClawConfig::default(); | |
| let inner_async_spawner: Arc<dyn AsyncDelegateSpawner> = Arc::new(NoopTestSpawner); | |
| let inner_background_spawner: Arc<dyn AsyncDelegateSpawner> = Arc::new(NoopTestSpawner); | |
| let override_background_spawner: Arc<dyn AsyncDelegateSpawner> = Arc::new(NoopTestSpawner); | |
| let inner_runtime = SpawnerAwareRuntime { | |
| async_delegate_spawner: Some(inner_async_spawner.clone()), | |
| background_task_spawner: Some(inner_background_spawner), | |
| }; | |
| let hosted_runtime = HostedConversationRuntime::new(inner_runtime) | |
| .with_background_task_spawner(override_background_spawner.clone()); | |
| let resolved_async_spawner = hosted_runtime | |
| .async_delegate_spawner(&config) | |
| .expect("async delegate spawner"); | |
| let resolved_background_spawner = hosted_runtime | |
| .background_task_spawner(&config) | |
| .expect("background task spawner"); | |
| assert!(Arc::ptr_eq(&resolved_async_spawner, &inner_async_spawner)); | |
| assert!(Arc::ptr_eq( | |
| &resolved_background_spawner, | |
| &override_background_spawner | |
| )); | |
| } | |
| #[cfg(feature = "memory-sqlite")] | |
| #[test] | |
| fn hosted_runtime_overrides_async_delegate_spawner_without_changing_background_task_spawner() { | |
| let config = LoongClawConfig::default(); | |
| let override_async_spawner: Arc<dyn AsyncDelegateSpawner> = Arc::new(NoopTestSpawner); | |
| let inner_background_spawner: Arc<dyn AsyncDelegateSpawner> = Arc::new(NoopTestSpawner); | |
| let inner_runtime = SpawnerAwareRuntime { | |
| async_delegate_spawner: None, | |
| background_task_spawner: Some(inner_background_spawner.clone()), | |
| }; | |
| let hosted_runtime = HostedConversationRuntime::new(inner_runtime) | |
| .with_async_delegate_spawner(override_async_spawner.clone()); | |
| let resolved_async_spawner = hosted_runtime | |
| .async_delegate_spawner(&config) | |
| .expect("async delegate spawner"); | |
| let resolved_background_spawner = hosted_runtime | |
| .background_task_spawner(&config) | |
| .expect("background task spawner"); | |
| assert!(Arc::ptr_eq(&resolved_async_spawner, &override_async_spawner)); | |
| assert!(Arc::ptr_eq( | |
| &resolved_background_spawner, | |
| &inner_background_spawner | |
| )); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/app/src/conversation/runtime.rs` around lines 2198 - 2225, Add a
symmetric test that verifies with_async_delegate_spawner() overrides the async
delegate spawner without affecting the background task spawner: create a test
(e.g.,
hosted_runtime_overrides_async_delegate_spawner_without_changing_background_task_spawner)
mirroring the existing hosted_runtime_overrides_background_task_spawner... test
but call
HostedConversationRuntime::new(inner_runtime).with_async_delegate_spawner(override_async_spawner),
then resolve both hosted_runtime.async_delegate_spawner(&config) and
hosted_runtime.background_task_spawner(&config) and assert Arc::ptr_eq that the
async resolver returns the override_async_spawner while the background resolver
still returns the original inner_background_spawner; use the same types/fixtures
(SpawnerAwareRuntime, LoongClawConfig, NoopTestSpawner) so the test covers the
missing override seam.
2 tasks
github-actions
Bot
added
tools
Collaborator
Author
|
Follow-up pushed in Added the next narrow runtime-truth slice on top of the hosted-runtime foundation:
Revalidated locally after the follow-up:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/app/src/conversation/turn_coordinator.rs (1)
7917-7937:⚠️ Potential issue | 🟡 MinorUse a production-valid
owner_kindin the async recovery fixtures.
build_delegate_async_enqueue_request()now always stamps queued async delegates withSome(ConstrainedSubagentOwnerKind::AsyncDelegateSpawner). Keeping these async fixtures atNonemeans these recovery tests no longer exercise the state that new async rows actually persist, which weakens coverage for owner-kind propagation and any terminal rendering built on it.♻️ Suggested adjustment
- owner_kind: None, + owner_kind: Some(ConstrainedSubagentOwnerKind::AsyncDelegateSpawner), ... - owner_kind: None, + owner_kind: Some(ConstrainedSubagentOwnerKind::AsyncDelegateSpawner),Also applies to: 8050-8070
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/app/src/conversation/turn_coordinator.rs` around lines 7917 - 7937, The async recovery fixture uses ConstrainedSubagentExecution with owner_kind set to None, but async delegates are now stamped with Some(ConstrainedSubagentOwnerKind::AsyncDelegateSpawner); update the fixture(s) (e.g., in build_delegate_async_enqueue_request and the ConstrainedSubagentExecution instances around ConstrainedSubagentMode::Async at the shown locations) to set owner_kind: Some(crate::conversation::ConstrainedSubagentOwnerKind::AsyncDelegateSpawner) so tests persist the actual owner-kind used in production (also apply the same change to the similar fixtures at the other range).
🧹 Nitpick comments (2)
crates/app/src/operator/delegate_runtime.rs (1)
715-745: Add one positiveowner_kindpropagation test.The updated fixtures all use
owner_kind: None, so they only prove the new field is optional. A singleSome(...)case that asserts bothseed.execution.owner_kindand the serialized lifecycle payload would lock down the new contract and catch silent drops.Also applies to: 750-777, 886-906
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/app/src/operator/delegate_runtime.rs` around lines 715 - 745, Add a positive test that sets owner_kind = Some("<kind>".to_owned()) on the DelegateChildExecutionPolicy passed into build_delegate_child_lifecycle_seed and assert that the returned seed has seed.execution.owner_kind == Some("<kind>".to_owned()); also assert the serialized lifecycle payload (the lifecycle/request blob produced by the seed) contains the same owner_kind value to ensure it is propagated into the serialized message. Target the existing test blocks around build_delegate_child_lifecycle_seed / DelegateChildExecutionPolicy (the seed variable and seed.execution fields) and mirror the style of the current assertions so the new test covers owner_kind propagation for the Async case (and repeat similarly for the other test locations noted).crates/app/src/conversation/subagent.rs (1)
651-656: Optional: also lock string mapping forAsyncDelegateSpawner.You already verify
"background_task_host"; adding the second variant helps guard future regressions in literal mapping.✅ Optional test addition
assert_eq!( ConstrainedSubagentExecution::from_event_payload(&payload) .and_then(|execution| execution.owner_kind) .map(ConstrainedSubagentOwnerKind::as_str), Some("background_task_host") ); + assert_eq!( + ConstrainedSubagentOwnerKind::AsyncDelegateSpawner.as_str(), + "async_delegate_spawner" + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/app/src/conversation/subagent.rs` around lines 651 - 656, Add a second assertion that locks the string mapping for the AsyncDelegateSpawner variant: mirror the existing check by calling ConstrainedSubagentExecution::from_event_payload(&payload).and_then(|execution| execution.owner_kind).map(ConstrainedSubagentOwnerKind::as_str) and assert it equals Some("async_delegate_spawner") (using ConstrainedSubagentOwnerKind::AsyncDelegateSpawner as the variant reference) so the literal mapping is validated alongside "background_task_host".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@crates/app/src/conversation/turn_coordinator.rs`:
- Around line 7917-7937: The async recovery fixture uses
ConstrainedSubagentExecution with owner_kind set to None, but async delegates
are now stamped with Some(ConstrainedSubagentOwnerKind::AsyncDelegateSpawner);
update the fixture(s) (e.g., in build_delegate_async_enqueue_request and the
ConstrainedSubagentExecution instances around ConstrainedSubagentMode::Async at
the shown locations) to set owner_kind:
Some(crate::conversation::ConstrainedSubagentOwnerKind::AsyncDelegateSpawner) so
tests persist the actual owner-kind used in production (also apply the same
change to the similar fixtures at the other range).
---
Nitpick comments:
In `@crates/app/src/conversation/subagent.rs`:
- Around line 651-656: Add a second assertion that locks the string mapping for
the AsyncDelegateSpawner variant: mirror the existing check by calling
ConstrainedSubagentExecution::from_event_payload(&payload).and_then(|execution|
execution.owner_kind).map(ConstrainedSubagentOwnerKind::as_str) and assert it
equals Some("async_delegate_spawner") (using
ConstrainedSubagentOwnerKind::AsyncDelegateSpawner as the variant reference) so
the literal mapping is validated alongside "background_task_host".
In `@crates/app/src/operator/delegate_runtime.rs`:
- Around line 715-745: Add a positive test that sets owner_kind =
Some("<kind>".to_owned()) on the DelegateChildExecutionPolicy passed into
build_delegate_child_lifecycle_seed and assert that the returned seed has
seed.execution.owner_kind == Some("<kind>".to_owned()); also assert the
serialized lifecycle payload (the lifecycle/request blob produced by the seed)
contains the same owner_kind value to ensure it is propagated into the
serialized message. Target the existing test blocks around
build_delegate_child_lifecycle_seed / DelegateChildExecutionPolicy (the seed
variable and seed.execution fields) and mirror the style of the current
assertions so the new test covers owner_kind propagation for the Async case (and
repeat similarly for the other test locations noted).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7551b04e-534c-4e46-8403-92261d7a23b1
📒 Files selected for processing (9)
crates/app/src/conversation/mod.rscrates/app/src/conversation/subagent.rscrates/app/src/conversation/tests.rscrates/app/src/conversation/turn_coordinator.rscrates/app/src/conversation/workspace_isolation.rscrates/app/src/operator/delegate_runtime.rscrates/app/src/tools/runtime_config.rscrates/daemon/src/tasks_cli.rscrates/daemon/tests/integration/tasks_cli.rs
chumyin
added a commit
to chumyin/loongclaw
that referenced
this pull request
Apr 10, 2026
Default conversation runtime construction was still scattered across coordinator, turn loop, channel dispatch, delegate support, turn engine, and daemon task bootstrap sites. That drift weakened the HostedConversationRuntime seam and made future runtime-host work more likely to fork behavior. This change introduces one shared default-runtime loader in the conversation runtime module, adds a hosted-default loader for runtime host composition, and routes the remaining app/daemon bootstrap sites through those helpers. Observer failure signaling remains intact while background-task runtime composition now reuses the same bootstrap path. Constraint: Preserve existing context-engine selection and observer failure behavior Rejected: Introduce a larger runtime factory trait now | unnecessary scope for this convergence slice Rejected: Add a global cached runtime singleton | premature before lifecycle ownership is explicit Confidence: high Scope-risk: narrow Reversibility: clean Directive: New app/daemon default runtime bootstrap sites should use load_default_conversation_runtime or load_hosted_default_conversation_runtime instead of calling from_config_or_env directly Tested: cargo fmt --all -- --check Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No new benchmark or latency measurement in this refactor-only slice Related: eastreams#1198 Related: eastreams#1194
github-actions
Bot
added
the
channels
2 tasks
Collaborator
Author
|
Follow-up pushed for the runtime-host convergence lane. New commit: What changed in this PR update:
Verification rerun after the follow-up:
Related issue: #1198 |
There was a problem hiding this comment.
♻️ Duplicate comments (2)
crates/app/src/conversation/runtime.rs (2)
2216-2257:⚠️ Potential issue | 🟡 MinorAdd the symmetric regression test for
with_async_delegate_spawner.Current tests cover background override and hosted-default loader behavior, but the async override seam is still unguarded. Please add a mirror test to verify async override precedence without changing background behavior.
Suggested test addition
+ #[cfg(feature = "memory-sqlite")] + #[test] + fn hosted_runtime_overrides_async_delegate_spawner_without_changing_background_task_spawner() { + let config = LoongClawConfig::default(); + let override_async_spawner: Arc<dyn AsyncDelegateSpawner> = Arc::new(NoopTestSpawner); + let inner_background_spawner: Arc<dyn AsyncDelegateSpawner> = Arc::new(NoopTestSpawner); + let inner_runtime = SpawnerAwareRuntime { + async_delegate_spawner: None, + background_task_spawner: Some(inner_background_spawner.clone()), + }; + + let hosted_runtime = HostedConversationRuntime::new(inner_runtime) + .with_async_delegate_spawner(override_async_spawner.clone()); + + let resolved_async_spawner = hosted_runtime + .async_delegate_spawner(&config) + .expect("async delegate spawner"); + let resolved_background_spawner = hosted_runtime + .background_task_spawner(&config) + .expect("background task spawner"); + + assert!(Arc::ptr_eq(&resolved_async_spawner, &override_async_spawner)); + assert!(Arc::ptr_eq( + &resolved_background_spawner, + &inner_background_spawner + )); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/app/src/conversation/runtime.rs` around lines 2216 - 2257, Add a symmetric unit test that mirrors hosted_runtime_overrides_background_task_spawner_without_changing_async_delegate_spawner but exercises with_async_delegate_spawner: construct an inner SpawnerAwareRuntime with both async_delegate_spawner and background_task_spawner set (use NoopTestSpawner), create HostedConversationRuntime::new(inner_runtime).with_async_delegate_spawner(override_async_spawner), then call hosted_runtime.async_delegate_spawner(&config) and hosted_runtime.background_task_spawner(&config) and assert that the resolved async spawner is the override (Arc::ptr_eq to override_async_spawner) while the background spawner remains the original inner background spawner; use the same config, types, and naming conventions as the existing tests.
1497-1686:⚠️ Potential issue | 🟠 MajorDelegate
build_contextexplicitly in the hosted wrapper.In this impl block (starting Line 1499),
build_contextis not overridden, so the trait default at Line 1394 is used. That bypasses inner-runtime custombuild_contextbehavior and can drop assembled-context details (e.g., prompt fragments/metadata) for runtimes likeDefaultConversationRuntime.Proposed fix
#[cfg(feature = "memory-sqlite")] #[async_trait] impl<R> ConversationRuntime for HostedConversationRuntime<R> where R: ConversationRuntime, { @@ async fn ingest( &self, session_id: &str, message: &Value, kernel_ctx: &KernelContext, ) -> CliResult<ContextEngineIngestResult> { self.inner.ingest(session_id, message, kernel_ctx).await } + + async fn build_context( + &self, + config: &LoongClawConfig, + session_id: &str, + include_system_prompt: bool, + binding: ConversationRuntimeBinding<'_>, + ) -> CliResult<AssembledConversationContext> { + self.inner + .build_context(config, session_id, include_system_prompt, binding) + .await + } async fn build_messages( &self, config: &LoongClawConfig, session_id: &str,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/app/src/conversation/runtime.rs` around lines 1497 - 1686, The HostedConversationRuntime<R> impl is missing an override for build_context so it uses the trait default and bypasses inner runtime behavior; add an async fn build_context with the same signature as the trait and simply delegate to and return self.inner.build_context(...).await (i.e., implement build_context on HostedConversationRuntime to call into self.inner.build_context with the same params so runtimes like DefaultConversationRuntime retain their assembled-context details).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/app/src/conversation/runtime.rs`:
- Around line 2216-2257: Add a symmetric unit test that mirrors
hosted_runtime_overrides_background_task_spawner_without_changing_async_delegate_spawner
but exercises with_async_delegate_spawner: construct an inner
SpawnerAwareRuntime with both async_delegate_spawner and background_task_spawner
set (use NoopTestSpawner), create
HostedConversationRuntime::new(inner_runtime).with_async_delegate_spawner(override_async_spawner),
then call hosted_runtime.async_delegate_spawner(&config) and
hosted_runtime.background_task_spawner(&config) and assert that the resolved
async spawner is the override (Arc::ptr_eq to override_async_spawner) while the
background spawner remains the original inner background spawner; use the same
config, types, and naming conventions as the existing tests.
- Around line 1497-1686: The HostedConversationRuntime<R> impl is missing an
override for build_context so it uses the trait default and bypasses inner
runtime behavior; add an async fn build_context with the same signature as the
trait and simply delegate to and return self.inner.build_context(...).await
(i.e., implement build_context on HostedConversationRuntime to call into
self.inner.build_context with the same params so runtimes like
DefaultConversationRuntime retain their assembled-context details).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f8320ac8-883b-41f4-b2f2-31387f4540df
📒 Files selected for processing (9)
crates/app/src/channel/dispatch.rscrates/app/src/conversation/delegate_support.rscrates/app/src/conversation/mod.rscrates/app/src/conversation/runtime.rscrates/app/src/conversation/tests.rscrates/app/src/conversation/turn_coordinator.rscrates/app/src/conversation/turn_engine.rscrates/app/src/conversation/turn_loop.rscrates/daemon/src/tasks_cli.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/app/src/conversation/tests.rs
- crates/app/src/conversation/mod.rs
chumyin
added a commit
to chumyin/loongclaw
that referenced
this pull request
Apr 11, 2026
Provider request sessions and model-catalog fetches were still building a fresh reqwest client on every call. That left one of the remaining always-on/runtime-host seams unnecessarily short-lived even after the conversation runtime bootstrap convergence work. This change adds a small host-owned provider HTTP client cache keyed by the transport policy field that currently affects client construction and routes both request-session and model-catalog paths through the shared helper. Auth headers and auth-context lifetimes remain request-scoped, so transport reuse does not widen credential lifetime. Constraint: Keep auth headers, auth context, and retry state request-scoped while reusing only transport clients Rejected: Cache full provider request sessions | would mix transport reuse with auth/model-candidate lifecycle Rejected: Add benchmark plumbing in the same slice | unnecessary scope for a correctness-first host convergence step Confidence: high Scope-risk: narrow Reversibility: clean Directive: If future provider transport options affect reqwest client construction, extend ProviderHttpClientCacheKey instead of bypassing the shared cache Tested: cargo fmt --all -- --check Tested: cargo clippy -p loongclaw-app --all-targets --all-features -- -D warnings Tested: cargo test -p loongclaw-app --lib provider_http_client_cache_ -- --test-threads=1 Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No dedicated TTFT benchmark yet; this slice proves cache reuse and preserves behavior only Related: eastreams#1202 Related: eastreams#1194
github-actions
Bot
added
the
providers
2 tasks
Collaborator
Author
|
Another runtime-host follow-up is now pushed onto this PR. New commit: What changed in this PR update:
Verification rerun after the follow-up:
Related issue: #1202 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/app/src/provider/http_client_runtime.rs (1)
23-25: Consider bounding cache growth for long-lived processes.Line 24 and Line 98 create a process-wide cache with no eviction/upper bound. If timeout values vary widely at runtime, key cardinality can grow without limit.
Also applies to: 98-99
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/app/src/provider/http_client_runtime.rs`:
- Around line 88-94: The cache miss path is not atomic: concurrent callers can
both call build_provider_http_client for the same cache_key; fix by implementing
an atomic get-or-build for provider clients — e.g., use a per-key lock or the
map's entry API/once_cell semantics: call
load_cached_provider_http_client(cache_key), and on miss acquire a shared
per-key mutex (or use cache.entry(cache_key).or_insert_with), re-check the cache
inside the lock, then call build_provider_http_client(cache_key) only if still
missing, store via store_provider_http_client(cache_key, built_client), release
the lock and return the cached client; ensure the functions
load_cached_provider_http_client, build_provider_http_client and
store_provider_http_client are used in this double-checked locking / entry-based
flow so duplicate builds are prevented.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8d547723-34f4-43f7-a852-8ca8b849d0d1
📒 Files selected for processing (1)
crates/app/src/provider/http_client_runtime.rs
Comment on lines
+88
to
+94
| if let Some(cached_client) = load_cached_provider_http_client(cache_key) { | ||
| return Ok(cached_client); | ||
| } | ||
|
|
||
| let built_client = build_provider_http_client(cache_key)?; | ||
| let cached_client = store_provider_http_client(cache_key, built_client); | ||
|
|
There was a problem hiding this comment.
Non-atomic cache miss path allows duplicate client builds under contention.
Line 88 through Line 94 split cache read/build/write across separate critical sections. Two concurrent misses for the same key can both execute build_provider_http_client, so cold-start churn still happens.
Suggested fix (atomic get-or-build path)
+fn load_or_build_provider_http_client(
+ cache_key: ProviderHttpClientCacheKey,
+) -> CliResult<reqwest::Client> {
+ with_provider_http_client_cache(|cache| {
+ if let Some(cached_client) = cache.entries.get(&cache_key) {
+ return Ok(cached_client.clone());
+ }
+
+ let built_client = build_provider_http_client(cache_key)?;
+ cache.entries.insert(cache_key, built_client.clone());
+ Ok(built_client)
+ })
+}
+
pub(super) fn build_http_client(
request_policy: &policy::ProviderRequestPolicy,
) -> CliResult<reqwest::Client> {
let cache_key = ProviderHttpClientCacheKey::from_request_policy(request_policy);
-
- if let Some(cached_client) = load_cached_provider_http_client(cache_key) {
- return Ok(cached_client);
- }
-
- let built_client = build_provider_http_client(cache_key)?;
- let cached_client = store_provider_http_client(cache_key, built_client);
-
- Ok(cached_client)
+ load_or_build_provider_http_client(cache_key)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/app/src/provider/http_client_runtime.rs` around lines 88 - 94, The
cache miss path is not atomic: concurrent callers can both call
build_provider_http_client for the same cache_key; fix by implementing an atomic
get-or-build for provider clients — e.g., use a per-key lock or the map's entry
API/once_cell semantics: call load_cached_provider_http_client(cache_key), and
on miss acquire a shared per-key mutex (or use
cache.entry(cache_key).or_insert_with), re-check the cache inside the lock, then
call build_provider_http_client(cache_key) only if still missing, store via
store_provider_http_client(cache_key, built_client), release the lock and return
the cached client; ensure the functions load_cached_provider_http_client,
build_provider_http_client and store_provider_http_client are used in this
double-checked locking / entry-based flow so duplicate builds are prevented.
chumyin
added a commit
to chumyin/loongclaw
that referenced
this pull request
Apr 11, 2026
LoongClaw's streaming turns already emitted token deltas, but the runtime had no direct first-token signal for operators or live surfaces. That made TTFT regressions hard to see while runtime-host work was converging around longer-lived transport and bootstrap seams. This change adds additive elapsed timing metadata to streaming token observer events, stamps it from provider-request start in the coordinator, and threads the first-token latency into the live chat surface so active turns can show TTFT without changing provider semantics or persistence contracts. Constraint: Keep the change additive and backward-compatible for existing streaming event consumers Rejected: Add a separate telemetry subsystem first | broader than the next safe maturity slice Rejected: Measure only end-to-end request duration | misses the first-token latency signal needed for interactive runtime tuning Confidence: high Scope-risk: narrow Reversibility: clean Directive: Future streaming observer consumers should treat elapsed_ms as optional metadata and avoid assuming every provider path can produce TTFT Tested: cargo fmt --all -- --check Tested: cargo test -p loongclaw-app --lib map_streaming_callback_data_to_token_event_ -- --test-threads=1 Tested: cargo test -p loongclaw-app --lib handle_turn_with_observer_uses_streaming_request_and_emits_live_events -- --test-threads=1 Tested: cargo test -p loongclaw-app --lib render_cli_chat_live_surface_lines_show_pipeline_status_and_preview -- --test-threads=1 Tested: cargo test -p loongclaw-app --lib cli_chat_live_surface_observer_emits_phase_and_stream_preview_batches -- --test-threads=1 Tested: cargo test -p loongclaw-app --lib cli_chat_live_surface_observer_resets_request_scoped_buffers_between_rounds -- --test-threads=1 Tested: cargo clippy -p loongclaw-app --all-targets --all-features -- -D warnings Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No separate benchmark harness or persisted TTFT analytics in this slice Related: eastreams#1203 Related: eastreams#1194
github-actions
Bot
added
the
acp
2 tasks
Collaborator
Author
|
Another runtime-host maturity follow-up is now pushed onto this PR. New commit: What changed in this PR update:
Verification rerun after the follow-up:
Related issue: #1203 |
chumyin
added a commit
to chumyin/loongclaw
that referenced
this pull request
Apr 11, 2026
LoongClaw's runtime host path can now reuse provider HTTP clients, but that cache was still opaque to runtime snapshots and operator read models. Without an observable transport-host signal, it remained hard to confirm whether the cache was warm or whether transport churn had regressed. This change adds additive provider transport cache metrics in the app runtime layer and threads them into daemon runtime snapshot state plus JSON/text rendering. The cache remains in-process only and request semantics stay unchanged; this is strictly observability over the host-owned transport seam. Constraint: Keep transport metrics additive and observational without changing request/auth semantics Rejected: Leave the cache opaque and rely on profiling only | makes always-on regressions harder to diagnose in normal operator flows Rejected: Add a larger telemetry pipeline first | broader than the next safe observability step Confidence: high Scope-risk: narrow Reversibility: clean Directive: If new reqwest-client construction knobs are added, update both ProviderHttpClientCacheKey and the exported runtime metrics so snapshot truth stays aligned with actual cache behavior Tested: cargo fmt --all -- --check Tested: cargo test -p loongclaw-app --lib provider_http_client_cache_ -- --test-threads=1 Tested: cargo test -p loongclaw --test integration runtime_snapshot_json_payload_includes_provider_tool_and_external_skill_inventory -- --test-threads=1 Tested: cargo test -p loongclaw --test integration runtime_snapshot_text_highlights_experiment_relevant_sections -- --test-threads=1 Tested: cargo clippy -p loongclaw-app --all-targets --all-features -- -D warnings Tested: cargo clippy -p loongclaw --all-targets --all-features -- -D warnings Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No cross-process transport cache sharing or benchmark harness in this slice Related: eastreams#1204 Related: eastreams#1194
github-actions
Bot
added
size: XL
github-actions
Bot
added
the
documentation
chumyin
added a commit
to chumyin/loongclaw
that referenced
this pull request
Apr 17, 2026
Default conversation runtime construction was still scattered across coordinator, turn loop, channel dispatch, delegate support, turn engine, and daemon task bootstrap sites. That drift weakened the HostedConversationRuntime seam and made future runtime-host work more likely to fork behavior. This change introduces one shared default-runtime loader in the conversation runtime module, adds a hosted-default loader for runtime host composition, and routes the remaining app/daemon bootstrap sites through those helpers. Observer failure signaling remains intact while background-task runtime composition now reuses the same bootstrap path. Constraint: Preserve existing context-engine selection and observer failure behavior Rejected: Introduce a larger runtime factory trait now | unnecessary scope for this convergence slice Rejected: Add a global cached runtime singleton | premature before lifecycle ownership is explicit Confidence: high Scope-risk: narrow Reversibility: clean Directive: New app/daemon default runtime bootstrap sites should use load_default_conversation_runtime or load_hosted_default_conversation_runtime instead of calling from_config_or_env directly Tested: cargo fmt --all -- --check Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No new benchmark or latency measurement in this refactor-only slice Related: eastreams#1198 Related: eastreams#1194
chumyin
added a commit
to chumyin/loongclaw
that referenced
this pull request
Apr 17, 2026
Provider request sessions and model-catalog fetches were still building a fresh reqwest client on every call. That left one of the remaining always-on/runtime-host seams unnecessarily short-lived even after the conversation runtime bootstrap convergence work. This change adds a small host-owned provider HTTP client cache keyed by the transport policy field that currently affects client construction and routes both request-session and model-catalog paths through the shared helper. Auth headers and auth-context lifetimes remain request-scoped, so transport reuse does not widen credential lifetime. Constraint: Keep auth headers, auth context, and retry state request-scoped while reusing only transport clients Rejected: Cache full provider request sessions | would mix transport reuse with auth/model-candidate lifecycle Rejected: Add benchmark plumbing in the same slice | unnecessary scope for a correctness-first host convergence step Confidence: high Scope-risk: narrow Reversibility: clean Directive: If future provider transport options affect reqwest client construction, extend ProviderHttpClientCacheKey instead of bypassing the shared cache Tested: cargo fmt --all -- --check Tested: cargo clippy -p loongclaw-app --all-targets --all-features -- -D warnings Tested: cargo test -p loongclaw-app --lib provider_http_client_cache_ -- --test-threads=1 Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No dedicated TTFT benchmark yet; this slice proves cache reuse and preserves behavior only Related: eastreams#1202 Related: eastreams#1194
chumyin
added a commit
to chumyin/loongclaw
that referenced
this pull request
Apr 17, 2026
LoongClaw's streaming turns already emitted token deltas, but the runtime had no direct first-token signal for operators or live surfaces. That made TTFT regressions hard to see while runtime-host work was converging around longer-lived transport and bootstrap seams. This change adds additive elapsed timing metadata to streaming token observer events, stamps it from provider-request start in the coordinator, and threads the first-token latency into the live chat surface so active turns can show TTFT without changing provider semantics or persistence contracts. Constraint: Keep the change additive and backward-compatible for existing streaming event consumers Rejected: Add a separate telemetry subsystem first | broader than the next safe maturity slice Rejected: Measure only end-to-end request duration | misses the first-token latency signal needed for interactive runtime tuning Confidence: high Scope-risk: narrow Reversibility: clean Directive: Future streaming observer consumers should treat elapsed_ms as optional metadata and avoid assuming every provider path can produce TTFT Tested: cargo fmt --all -- --check Tested: cargo test -p loongclaw-app --lib map_streaming_callback_data_to_token_event_ -- --test-threads=1 Tested: cargo test -p loongclaw-app --lib handle_turn_with_observer_uses_streaming_request_and_emits_live_events -- --test-threads=1 Tested: cargo test -p loongclaw-app --lib render_cli_chat_live_surface_lines_show_pipeline_status_and_preview -- --test-threads=1 Tested: cargo test -p loongclaw-app --lib cli_chat_live_surface_observer_emits_phase_and_stream_preview_batches -- --test-threads=1 Tested: cargo test -p loongclaw-app --lib cli_chat_live_surface_observer_resets_request_scoped_buffers_between_rounds -- --test-threads=1 Tested: cargo clippy -p loongclaw-app --all-targets --all-features -- -D warnings Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No separate benchmark harness or persisted TTFT analytics in this slice Related: eastreams#1203 Related: eastreams#1194
chumyin
added a commit
to chumyin/loongclaw
that referenced
this pull request
Apr 17, 2026
LoongClaw's runtime host path can now reuse provider HTTP clients, but that cache was still opaque to runtime snapshots and operator read models. Without an observable transport-host signal, it remained hard to confirm whether the cache was warm or whether transport churn had regressed. This change adds additive provider transport cache metrics in the app runtime layer and threads them into daemon runtime snapshot state plus JSON/text rendering. The cache remains in-process only and request semantics stay unchanged; this is strictly observability over the host-owned transport seam. Constraint: Keep transport metrics additive and observational without changing request/auth semantics Rejected: Leave the cache opaque and rely on profiling only | makes always-on regressions harder to diagnose in normal operator flows Rejected: Add a larger telemetry pipeline first | broader than the next safe observability step Confidence: high Scope-risk: narrow Reversibility: clean Directive: If new reqwest-client construction knobs are added, update both ProviderHttpClientCacheKey and the exported runtime metrics so snapshot truth stays aligned with actual cache behavior Tested: cargo fmt --all -- --check Tested: cargo test -p loongclaw-app --lib provider_http_client_cache_ -- --test-threads=1 Tested: cargo test -p loongclaw --test integration runtime_snapshot_json_payload_includes_provider_tool_and_external_skill_inventory -- --test-threads=1 Tested: cargo test -p loongclaw --test integration runtime_snapshot_text_highlights_experiment_relevant_sections -- --test-threads=1 Tested: cargo clippy -p loongclaw-app --all-targets --all-features -- -D warnings Tested: cargo clippy -p loongclaw --all-targets --all-features -- -D warnings Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No cross-process transport cache sharing or benchmark harness in this slice Related: eastreams#1204 Related: eastreams#1194
chumyin
force-pushed
the
feat/runtime-host-foundation-20260410
branch
from
b709755 to
9611cd4
Compare
chumyin
added a commit
to chumyin/loongclaw
that referenced
this pull request
Apr 17, 2026
LoongClaw already records prompt-frame drift and cache-shape truth, but operators could not see it through the session status surface. That made prompt progressive-disclosure behavior and token-efficiency debugging too dependent on raw event logs. This change adds a best-effort prompt-frame summary block to `sessions status`, renders a compact operator-facing summary, and extends the status integration coverage with a seeded prompt-frame snapshot event. While wiring full verification, I also fixed a gateway owner-state test isolation issue by making its loaded-config fixtures use per-test runtime paths instead of a shared `/tmp/loongclaw.toml` baseline, which was leaking ACP state across tests and invalidating all-features evidence. Constraint: Keep prompt-frame inspection additive and best-effort so sessions status still works when no prompt-frame events are present Rejected: Expose raw prompt-frame event payloads verbatim | too noisy for operator inspection Rejected: Add a larger prompt telemetry subsystem first | broader than the next safe operator-truth slice Confidence: high Scope-risk: narrow Reversibility: clean Directive: Operator-facing prompt-frame summaries should stay derived from typed event summaries, not ad hoc string parsing of raw assistant history Tested: cargo fmt --all -- --check Tested: cargo test -p loongclaw --test integration execute_sessions_command_status_surfaces_workflow_recipes_and_rendered_summary -- --test-threads=1 Tested: cargo test -p loongclaw --test integration gateway_owner_state_localhost_control_surface_requires_auth_and_stops_runtime -- --test-threads=1 Tested: cargo clippy -p loongclaw --all-targets --all-features -- -D warnings Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No gateway API/schema expansion for prompt-frame truth in this slice Related: eastreams#1207 Related: eastreams#1194
chumyin
added a commit
to chumyin/loongclaw
that referenced
this pull request
Apr 17, 2026
Prompt-frame truth had reached `sessions status`, but the more common background-task surface still lacked it. That left operator inspection fragmented for async delegate children and encouraged duplicated prompt inspection logic in daemon CLI code. This change extracts the prompt-frame loading/rendering helper into a shared daemon module, reuses it from sessions and tasks surfaces, and adds task-status prompt-frame rendering. While landing it, full verification also reconfirmed the earlier gateway owner-state fixture isolation fix, so the all-features gate remains trustworthy. Constraint: Keep prompt-frame inspection additive and derived from typed summaries instead of raw event payload parsing in each surface Rejected: Duplicate prompt-frame logic inside tasks_cli | would reintroduce slop debt and surface drift Rejected: Expand gateway/API schemas in the same slice | unnecessary scope for operator CLI parity Confidence: high Scope-risk: narrow Reversibility: clean Directive: Any future operator surface that needs prompt-frame truth should reuse session_prompt_frame_cli helper paths rather than open-coding summary extraction again Tested: cargo fmt --all -- --check Tested: cargo test -p loongclaw --test integration execute_sessions_command_status_surfaces_workflow_recipes_and_rendered_summary -- --test-threads=1 Tested: cargo test -p loongclaw --test integration execute_tasks_command_status_surfaces_approval_and_tool_policy -- --test-threads=1 Tested: cargo test -p loongclaw --test integration gateway_owner_state_localhost_control_surface_requires_auth_and_stops_runtime -- --test-threads=1 Tested: cargo clippy -p loongclaw --all-targets --all-features -- -D warnings Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No gateway/API prompt-frame parity yet in this slice Related: eastreams#1208 Related: eastreams#1194
`loong tasks` used a bespoke DetachedTasksRuntime that reimplemented the full ConversationRuntime trait only to swap the background-task spawner. That kept runtime truth fragmented and made future runtime-owned surfaces more likely to grow their own copy-paste wrappers. This change introduces a reusable HostedConversationRuntime wrapper in the conversation runtime layer. It composes over an inner runtime, delegates all normal runtime behavior, and overrides only the async/background spawner seams when explicitly requested. Tasks CLI now uses the shared wrapper instead of owning its own full trait implementation, and a focused regression test locks the override precedence. Constraint: Keep this slice refactor-only; do not redesign task execution semantics or introduce a full runtime host yet Constraint: Preserve existing task create behavior and background owner truth while removing wrapper duplication Rejected: Keep DetachedTasksRuntime bespoke | preserves drift-prone runtime duplication Rejected: Build a full persistent AgentRuntimeHost in one patch | too broad for the next trustworthy slice Confidence: high Scope-risk: narrow Reversibility: clean Directive: New runtime-owned surfaces should compose through HostedConversationRuntime before adding another bespoke ConversationRuntime wrapper Tested: cargo fmt --all -- --check Tested: CARGO_TARGET_DIR=/Users/chum/loongclaw/.cargo-target-shared cargo clippy -p loongclaw-app -p loongclaw --all-targets --all-features -- -D warnings Tested: CARGO_TARGET_DIR=/Users/chum/loongclaw/.cargo-target-shared cargo test -p loongclaw-app hosted_runtime --lib -- --test-threads=1 Tested: CARGO_TARGET_DIR=/Users/chum/loongclaw/.cargo-target-shared cargo test -p loongclaw --test integration execute_tasks_command_create_queues_background_task_and_surfaces_follow_up_recipes -- --nocapture Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No additional runtime bootstrap caching was added in this slice Closes: eastreams#1192
Async delegate lifecycle already carried queue/start/timeout truth, but the operator surfaces still had to infer which runtime host owned a child. That was especially muddy for background tasks, where a queued/running child could look the same as a normal async delegate even though it was owned by the background-task host. This change adds an additive owner-kind field to constrained subagent execution metadata, stamps it when async delegates and background tasks are queued, and projects it into task payloads and text rendering. The existing lifecycle model stays authoritative; this only makes the owner seam explicit. Constraint: Keep owner truth additive and reuse the existing delegate lifecycle model rather than inventing a parallel status machine Constraint: Preserve existing task behavior and session history shape except for the new explicit owner field Rejected: Infer owner kind ad hoc in tasks_cli from code-path conventions | keeps runtime truth implicit and drift-prone Rejected: Add pid-level worker tracking in this slice | too broad for the next minimal correct step Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep runtime ownership projected through delegate execution metadata so operator surfaces do not guess from call-site conventions Tested: cargo fmt --all -- --check Tested: CARGO_TARGET_DIR=/Users/chum/loongclaw/.cargo-target-shared cargo clippy -p loongclaw-app -p loongclaw --all-targets --all-features -- -D warnings Tested: CARGO_TARGET_DIR=/Users/chum/loongclaw/.cargo-target-shared cargo test -p loongclaw-app constrained_subagent_execution_round_trips_event_payload --lib -- --test-threads=1 Tested: CARGO_TARGET_DIR=/Users/chum/loongclaw/.cargo-target-shared cargo test -p loongclaw-app handle_turn_with_runtime_delegate_async_preserves_kernel_binding_in_spawn_request --lib -- --test-threads=1 Tested: CARGO_TARGET_DIR=/Users/chum/loongclaw/.cargo-target-shared cargo test -p loongclaw --test integration tasks_cli::execute_tasks_command_create_queues_background_task_and_surfaces_follow_up_recipes -- --nocapture Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No pid/heartbeat-level owner telemetry yet; this slice only adds owner-kind truth Related: eastreams#1192 Closes: eastreams#1195
Default conversation runtime construction was still scattered across coordinator, turn loop, channel dispatch, delegate support, turn engine, and daemon task bootstrap sites. That drift weakened the HostedConversationRuntime seam and made future runtime-host work more likely to fork behavior. This change introduces one shared default-runtime loader in the conversation runtime module, adds a hosted-default loader for runtime host composition, and routes the remaining app/daemon bootstrap sites through those helpers. Observer failure signaling remains intact while background-task runtime composition now reuses the same bootstrap path. Constraint: Preserve existing context-engine selection and observer failure behavior Rejected: Introduce a larger runtime factory trait now | unnecessary scope for this convergence slice Rejected: Add a global cached runtime singleton | premature before lifecycle ownership is explicit Confidence: high Scope-risk: narrow Reversibility: clean Directive: New app/daemon default runtime bootstrap sites should use load_default_conversation_runtime or load_hosted_default_conversation_runtime instead of calling from_config_or_env directly Tested: cargo fmt --all -- --check Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No new benchmark or latency measurement in this refactor-only slice Related: eastreams#1198 Related: eastreams#1194
Provider request sessions and model-catalog fetches were still building a fresh reqwest client on every call. That left one of the remaining always-on/runtime-host seams unnecessarily short-lived even after the conversation runtime bootstrap convergence work. This change adds a small host-owned provider HTTP client cache keyed by the transport policy field that currently affects client construction and routes both request-session and model-catalog paths through the shared helper. Auth headers and auth-context lifetimes remain request-scoped, so transport reuse does not widen credential lifetime. Constraint: Keep auth headers, auth context, and retry state request-scoped while reusing only transport clients Rejected: Cache full provider request sessions | would mix transport reuse with auth/model-candidate lifecycle Rejected: Add benchmark plumbing in the same slice | unnecessary scope for a correctness-first host convergence step Confidence: high Scope-risk: narrow Reversibility: clean Directive: If future provider transport options affect reqwest client construction, extend ProviderHttpClientCacheKey instead of bypassing the shared cache Tested: cargo fmt --all -- --check Tested: cargo clippy -p loongclaw-app --all-targets --all-features -- -D warnings Tested: cargo test -p loongclaw-app --lib provider_http_client_cache_ -- --test-threads=1 Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No dedicated TTFT benchmark yet; this slice proves cache reuse and preserves behavior only Related: eastreams#1202 Related: eastreams#1194
LoongClaw's streaming turns already emitted token deltas, but the runtime had no direct first-token signal for operators or live surfaces. That made TTFT regressions hard to see while runtime-host work was converging around longer-lived transport and bootstrap seams. This change adds additive elapsed timing metadata to streaming token observer events, stamps it from provider-request start in the coordinator, and threads the first-token latency into the live chat surface so active turns can show TTFT without changing provider semantics or persistence contracts. Constraint: Keep the change additive and backward-compatible for existing streaming event consumers Rejected: Add a separate telemetry subsystem first | broader than the next safe maturity slice Rejected: Measure only end-to-end request duration | misses the first-token latency signal needed for interactive runtime tuning Confidence: high Scope-risk: narrow Reversibility: clean Directive: Future streaming observer consumers should treat elapsed_ms as optional metadata and avoid assuming every provider path can produce TTFT Tested: cargo fmt --all -- --check Tested: cargo test -p loongclaw-app --lib map_streaming_callback_data_to_token_event_ -- --test-threads=1 Tested: cargo test -p loongclaw-app --lib handle_turn_with_observer_uses_streaming_request_and_emits_live_events -- --test-threads=1 Tested: cargo test -p loongclaw-app --lib render_cli_chat_live_surface_lines_show_pipeline_status_and_preview -- --test-threads=1 Tested: cargo test -p loongclaw-app --lib cli_chat_live_surface_observer_emits_phase_and_stream_preview_batches -- --test-threads=1 Tested: cargo test -p loongclaw-app --lib cli_chat_live_surface_observer_resets_request_scoped_buffers_between_rounds -- --test-threads=1 Tested: cargo clippy -p loongclaw-app --all-targets --all-features -- -D warnings Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No separate benchmark harness or persisted TTFT analytics in this slice Related: eastreams#1203 Related: eastreams#1194
LoongClaw's runtime host path can now reuse provider HTTP clients, but that cache was still opaque to runtime snapshots and operator read models. Without an observable transport-host signal, it remained hard to confirm whether the cache was warm or whether transport churn had regressed. This change adds additive provider transport cache metrics in the app runtime layer and threads them into daemon runtime snapshot state plus JSON/text rendering. The cache remains in-process only and request semantics stay unchanged; this is strictly observability over the host-owned transport seam. Constraint: Keep transport metrics additive and observational without changing request/auth semantics Rejected: Leave the cache opaque and rely on profiling only | makes always-on regressions harder to diagnose in normal operator flows Rejected: Add a larger telemetry pipeline first | broader than the next safe observability step Confidence: high Scope-risk: narrow Reversibility: clean Directive: If new reqwest-client construction knobs are added, update both ProviderHttpClientCacheKey and the exported runtime metrics so snapshot truth stays aligned with actual cache behavior Tested: cargo fmt --all -- --check Tested: cargo test -p loongclaw-app --lib provider_http_client_cache_ -- --test-threads=1 Tested: cargo test -p loongclaw --test integration runtime_snapshot_json_payload_includes_provider_tool_and_external_skill_inventory -- --test-threads=1 Tested: cargo test -p loongclaw --test integration runtime_snapshot_text_highlights_experiment_relevant_sections -- --test-threads=1 Tested: cargo clippy -p loongclaw-app --all-targets --all-features -- -D warnings Tested: cargo clippy -p loongclaw --all-targets --all-features -- -D warnings Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No cross-process transport cache sharing or benchmark harness in this slice Related: eastreams#1204 Related: eastreams#1194
LoongClaw already records prompt-frame drift and cache-shape truth, but operators could not see it through the session status surface. That made prompt progressive-disclosure behavior and token-efficiency debugging too dependent on raw event logs. This change adds a best-effort prompt-frame summary block to `sessions status`, renders a compact operator-facing summary, and extends the status integration coverage with a seeded prompt-frame snapshot event. While wiring full verification, I also fixed a gateway owner-state test isolation issue by making its loaded-config fixtures use per-test runtime paths instead of a shared `/tmp/loongclaw.toml` baseline, which was leaking ACP state across tests and invalidating all-features evidence. Constraint: Keep prompt-frame inspection additive and best-effort so sessions status still works when no prompt-frame events are present Rejected: Expose raw prompt-frame event payloads verbatim | too noisy for operator inspection Rejected: Add a larger prompt telemetry subsystem first | broader than the next safe operator-truth slice Confidence: high Scope-risk: narrow Reversibility: clean Directive: Operator-facing prompt-frame summaries should stay derived from typed event summaries, not ad hoc string parsing of raw assistant history Tested: cargo fmt --all -- --check Tested: cargo test -p loongclaw --test integration execute_sessions_command_status_surfaces_workflow_recipes_and_rendered_summary -- --test-threads=1 Tested: cargo test -p loongclaw --test integration gateway_owner_state_localhost_control_surface_requires_auth_and_stops_runtime -- --test-threads=1 Tested: cargo clippy -p loongclaw --all-targets --all-features -- -D warnings Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No gateway API/schema expansion for prompt-frame truth in this slice Related: eastreams#1207 Related: eastreams#1194
Prompt-frame truth had reached `sessions status`, but the more common background-task surface still lacked it. That left operator inspection fragmented for async delegate children and encouraged duplicated prompt inspection logic in daemon CLI code. This change extracts the prompt-frame loading/rendering helper into a shared daemon module, reuses it from sessions and tasks surfaces, and adds task-status prompt-frame rendering. While landing it, full verification also reconfirmed the earlier gateway owner-state fixture isolation fix, so the all-features gate remains trustworthy. Constraint: Keep prompt-frame inspection additive and derived from typed summaries instead of raw event payload parsing in each surface Rejected: Duplicate prompt-frame logic inside tasks_cli | would reintroduce slop debt and surface drift Rejected: Expand gateway/API schemas in the same slice | unnecessary scope for operator CLI parity Confidence: high Scope-risk: narrow Reversibility: clean Directive: Any future operator surface that needs prompt-frame truth should reuse session_prompt_frame_cli helper paths rather than open-coding summary extraction again Tested: cargo fmt --all -- --check Tested: cargo test -p loongclaw --test integration execute_sessions_command_status_surfaces_workflow_recipes_and_rendered_summary -- --test-threads=1 Tested: cargo test -p loongclaw --test integration execute_tasks_command_status_surfaces_approval_and_tool_policy -- --test-threads=1 Tested: cargo test -p loongclaw --test integration gateway_owner_state_localhost_control_surface_requires_auth_and_stops_runtime -- --test-threads=1 Tested: cargo clippy -p loongclaw --all-targets --all-features -- -D warnings Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No gateway/API prompt-frame parity yet in this slice Related: eastreams#1208 Related: eastreams#1194
Add shared daemon-side loading and rendering for safe-lane and turn-checkpoint summaries, then wire those summaries into session and task inspection surfaces alongside the existing prompt-frame truth. This keeps operator status flows anchored to the existing conversation summary loaders instead of introducing a separate read model. This slice also aligns task CLI terminal rendering with the session CLI sanitization pattern so runtime-truth lines, task labels, lookup errors, and event kinds do not leak raw control characters. Regression coverage now locks both the new runtime-truth payload/render paths and the terminal-sanitization behavior. Constraint: Keep the change additive to existing session/task payload shapes and text shells Rejected: Build a new daemon-wide runtime read model first | existing summary loaders already expose the required truth with a smaller diff Rejected: Add only safe-lane or only turn-checkpoint status lines | operators need loop-health and durability context together Confidence: high Scope-risk: narrow Directive: Keep operator-facing runtime truth additive and sourced from the existing conversation summary loaders unless a single shared runtime host/read-model seam is introduced deliberately Tested: cargo fmt --all -- --check; cargo clippy --workspace --all-targets --all-features -- -D warnings; cargo test --workspace --locked -- --test-threads=1; cargo test --workspace --all-features --locked -- --test-threads=1; ./scripts/check_architecture_boundaries.sh; ./scripts/check_dep_graph.sh; cargo test -p loongclaw --test integration execute_sessions_command_status_surfaces_workflow_recipes_and_rendered_summary -- --test-threads=1; cargo test -p loongclaw --test integration execute_tasks_command_status_surfaces_approval_and_tool_policy -- --test-threads=1; cargo test -p loongclaw --test integration render_tasks_status_text_escapes_control_characters -- --test-threads=1; cargo test -p loongclaw --test integration render_tasks_events_text_escapes_control_characters -- --test-threads=1 Not-tested: Best-effort degradation when secondary task inspection queries fail independently of core session status
Task operator inspection already had enough status truth to remain useful when approval-list or tool-policy lookups fail, but the read path still treated those secondary queries as hard dependencies. This change keeps session/task status as the fail-closed gate while degrading approval and tool-policy lookups into additive error fields that preserve the rest of the payload. The slice also stabilizes the fallback task payload shape used by create/cancel/recover hydration failures so downstream readers do not see a narrower schema during degraded paths. Focused tests now lock the local degradation helpers, the composed detail payload, the fallback schema, and the rendered warning lines. Constraint: Preserve existing task detail fields and text shells while making the degradation additive Rejected: Introduce a new daemon-wide read model for task inspection | too large for the bounded operator-surface fix Rejected: Make all task inspection dependencies best-effort | core task/session status still needs to fail closed for invisible or non-background targets Confidence: high Scope-risk: narrow Directive: Keep secondary operator lookups additive and degrade them into explicit payload warnings instead of replacing the whole task detail with a narrower fallback shape Tested: cargo fmt --all -- --check; cargo clippy --workspace --all-targets --all-features -- -D warnings; cargo test --workspace --locked -- --test-threads=1; cargo test --workspace --all-features --locked -- --test-threads=1; ./scripts/check_architecture_boundaries.sh; ./scripts/check_dep_graph.sh; cargo test -p loongclaw --lib best_effort_task_ -- --test-threads=1; cargo test -p loongclaw --lib compose_task_detail_payload_keeps_core_status_truth_when_secondary_lookups_degrade -- --test-threads=1; cargo test -p loongclaw --test integration execute_tasks_command_create_returns_queued_outcome_when_task_hydration_fails -- --test-threads=1; cargo test -p loongclaw --test integration render_tasks_status_text_escapes_control_characters -- --test-threads=1; cargo test -p loongclaw --test integration execute_tasks_command_status_surfaces_approval_and_tool_policy -- --test-threads=1 Not-tested: End-to-end execution where session_status succeeds while approval or tool-policy lookup independently fails for a live repository-backed task record
chumyin
force-pushed
the
feat/runtime-host-foundation-20260410
branch
from
9611cd4 to
219119c
Compare
chumyin
added a commit
that referenced
this pull request
Apr 17, 2026
Default conversation runtime construction was still scattered across coordinator, turn loop, channel dispatch, delegate support, turn engine, and daemon task bootstrap sites. That drift weakened the HostedConversationRuntime seam and made future runtime-host work more likely to fork behavior. This change introduces one shared default-runtime loader in the conversation runtime module, adds a hosted-default loader for runtime host composition, and routes the remaining app/daemon bootstrap sites through those helpers. Observer failure signaling remains intact while background-task runtime composition now reuses the same bootstrap path. Constraint: Preserve existing context-engine selection and observer failure behavior Rejected: Introduce a larger runtime factory trait now | unnecessary scope for this convergence slice Rejected: Add a global cached runtime singleton | premature before lifecycle ownership is explicit Confidence: high Scope-risk: narrow Reversibility: clean Directive: New app/daemon default runtime bootstrap sites should use load_default_conversation_runtime or load_hosted_default_conversation_runtime instead of calling from_config_or_env directly Tested: cargo fmt --all -- --check Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No new benchmark or latency measurement in this refactor-only slice Related: #1198 Related: #1194
chumyin
added a commit
that referenced
this pull request
Apr 17, 2026
Provider request sessions and model-catalog fetches were still building a fresh reqwest client on every call. That left one of the remaining always-on/runtime-host seams unnecessarily short-lived even after the conversation runtime bootstrap convergence work. This change adds a small host-owned provider HTTP client cache keyed by the transport policy field that currently affects client construction and routes both request-session and model-catalog paths through the shared helper. Auth headers and auth-context lifetimes remain request-scoped, so transport reuse does not widen credential lifetime. Constraint: Keep auth headers, auth context, and retry state request-scoped while reusing only transport clients Rejected: Cache full provider request sessions | would mix transport reuse with auth/model-candidate lifecycle Rejected: Add benchmark plumbing in the same slice | unnecessary scope for a correctness-first host convergence step Confidence: high Scope-risk: narrow Reversibility: clean Directive: If future provider transport options affect reqwest client construction, extend ProviderHttpClientCacheKey instead of bypassing the shared cache Tested: cargo fmt --all -- --check Tested: cargo clippy -p loongclaw-app --all-targets --all-features -- -D warnings Tested: cargo test -p loongclaw-app --lib provider_http_client_cache_ -- --test-threads=1 Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No dedicated TTFT benchmark yet; this slice proves cache reuse and preserves behavior only Related: #1202 Related: #1194
chumyin
added a commit
that referenced
this pull request
Apr 17, 2026
LoongClaw's streaming turns already emitted token deltas, but the runtime had no direct first-token signal for operators or live surfaces. That made TTFT regressions hard to see while runtime-host work was converging around longer-lived transport and bootstrap seams. This change adds additive elapsed timing metadata to streaming token observer events, stamps it from provider-request start in the coordinator, and threads the first-token latency into the live chat surface so active turns can show TTFT without changing provider semantics or persistence contracts. Constraint: Keep the change additive and backward-compatible for existing streaming event consumers Rejected: Add a separate telemetry subsystem first | broader than the next safe maturity slice Rejected: Measure only end-to-end request duration | misses the first-token latency signal needed for interactive runtime tuning Confidence: high Scope-risk: narrow Reversibility: clean Directive: Future streaming observer consumers should treat elapsed_ms as optional metadata and avoid assuming every provider path can produce TTFT Tested: cargo fmt --all -- --check Tested: cargo test -p loongclaw-app --lib map_streaming_callback_data_to_token_event_ -- --test-threads=1 Tested: cargo test -p loongclaw-app --lib handle_turn_with_observer_uses_streaming_request_and_emits_live_events -- --test-threads=1 Tested: cargo test -p loongclaw-app --lib render_cli_chat_live_surface_lines_show_pipeline_status_and_preview -- --test-threads=1 Tested: cargo test -p loongclaw-app --lib cli_chat_live_surface_observer_emits_phase_and_stream_preview_batches -- --test-threads=1 Tested: cargo test -p loongclaw-app --lib cli_chat_live_surface_observer_resets_request_scoped_buffers_between_rounds -- --test-threads=1 Tested: cargo clippy -p loongclaw-app --all-targets --all-features -- -D warnings Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No separate benchmark harness or persisted TTFT analytics in this slice Related: #1203 Related: #1194
chumyin
added a commit
that referenced
this pull request
Apr 17, 2026
LoongClaw's runtime host path can now reuse provider HTTP clients, but that cache was still opaque to runtime snapshots and operator read models. Without an observable transport-host signal, it remained hard to confirm whether the cache was warm or whether transport churn had regressed. This change adds additive provider transport cache metrics in the app runtime layer and threads them into daemon runtime snapshot state plus JSON/text rendering. The cache remains in-process only and request semantics stay unchanged; this is strictly observability over the host-owned transport seam. Constraint: Keep transport metrics additive and observational without changing request/auth semantics Rejected: Leave the cache opaque and rely on profiling only | makes always-on regressions harder to diagnose in normal operator flows Rejected: Add a larger telemetry pipeline first | broader than the next safe observability step Confidence: high Scope-risk: narrow Reversibility: clean Directive: If new reqwest-client construction knobs are added, update both ProviderHttpClientCacheKey and the exported runtime metrics so snapshot truth stays aligned with actual cache behavior Tested: cargo fmt --all -- --check Tested: cargo test -p loongclaw-app --lib provider_http_client_cache_ -- --test-threads=1 Tested: cargo test -p loongclaw --test integration runtime_snapshot_json_payload_includes_provider_tool_and_external_skill_inventory -- --test-threads=1 Tested: cargo test -p loongclaw --test integration runtime_snapshot_text_highlights_experiment_relevant_sections -- --test-threads=1 Tested: cargo clippy -p loongclaw-app --all-targets --all-features -- -D warnings Tested: cargo clippy -p loongclaw --all-targets --all-features -- -D warnings Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No cross-process transport cache sharing or benchmark harness in this slice Related: #1204 Related: #1194
chumyin
added a commit
that referenced
this pull request
Apr 17, 2026
LoongClaw already records prompt-frame drift and cache-shape truth, but operators could not see it through the session status surface. That made prompt progressive-disclosure behavior and token-efficiency debugging too dependent on raw event logs. This change adds a best-effort prompt-frame summary block to `sessions status`, renders a compact operator-facing summary, and extends the status integration coverage with a seeded prompt-frame snapshot event. While wiring full verification, I also fixed a gateway owner-state test isolation issue by making its loaded-config fixtures use per-test runtime paths instead of a shared `/tmp/loongclaw.toml` baseline, which was leaking ACP state across tests and invalidating all-features evidence. Constraint: Keep prompt-frame inspection additive and best-effort so sessions status still works when no prompt-frame events are present Rejected: Expose raw prompt-frame event payloads verbatim | too noisy for operator inspection Rejected: Add a larger prompt telemetry subsystem first | broader than the next safe operator-truth slice Confidence: high Scope-risk: narrow Reversibility: clean Directive: Operator-facing prompt-frame summaries should stay derived from typed event summaries, not ad hoc string parsing of raw assistant history Tested: cargo fmt --all -- --check Tested: cargo test -p loongclaw --test integration execute_sessions_command_status_surfaces_workflow_recipes_and_rendered_summary -- --test-threads=1 Tested: cargo test -p loongclaw --test integration gateway_owner_state_localhost_control_surface_requires_auth_and_stops_runtime -- --test-threads=1 Tested: cargo clippy -p loongclaw --all-targets --all-features -- -D warnings Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No gateway API/schema expansion for prompt-frame truth in this slice Related: #1207 Related: #1194
chumyin
added a commit
that referenced
this pull request
Apr 17, 2026
Prompt-frame truth had reached `sessions status`, but the more common background-task surface still lacked it. That left operator inspection fragmented for async delegate children and encouraged duplicated prompt inspection logic in daemon CLI code. This change extracts the prompt-frame loading/rendering helper into a shared daemon module, reuses it from sessions and tasks surfaces, and adds task-status prompt-frame rendering. While landing it, full verification also reconfirmed the earlier gateway owner-state fixture isolation fix, so the all-features gate remains trustworthy. Constraint: Keep prompt-frame inspection additive and derived from typed summaries instead of raw event payload parsing in each surface Rejected: Duplicate prompt-frame logic inside tasks_cli | would reintroduce slop debt and surface drift Rejected: Expand gateway/API schemas in the same slice | unnecessary scope for operator CLI parity Confidence: high Scope-risk: narrow Reversibility: clean Directive: Any future operator surface that needs prompt-frame truth should reuse session_prompt_frame_cli helper paths rather than open-coding summary extraction again Tested: cargo fmt --all -- --check Tested: cargo test -p loongclaw --test integration execute_sessions_command_status_surfaces_workflow_recipes_and_rendered_summary -- --test-threads=1 Tested: cargo test -p loongclaw --test integration execute_tasks_command_status_surfaces_approval_and_tool_policy -- --test-threads=1 Tested: cargo test -p loongclaw --test integration gateway_owner_state_localhost_control_surface_requires_auth_and_stops_runtime -- --test-threads=1 Tested: cargo clippy -p loongclaw --all-targets --all-features -- -D warnings Tested: cargo clippy --workspace --all-targets --all-features -- -D warnings Tested: cargo test --workspace --locked -- --test-threads=1 Tested: cargo test --workspace --all-features --locked -- --test-threads=1 Tested: ./scripts/check_architecture_boundaries.sh Tested: ./scripts/check_dep_graph.sh Not-tested: No gateway/API prompt-frame parity yet in this slice Related: #1208 Related: #1194
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
loong tasksowned a bespokeDetachedTasksRuntimethat reimplemented the fullConversationRuntimetrait only to swap the background-task spawner.HostedConversationRuntime<R>wrapper in the conversation runtime layer, delegated the full runtime trait through it, exposed explicit async/background spawner overrides, and switched tasks CLI to the shared wrapper.AgentRuntimeHostyet.Linked Issues
Change Type
Touched Areas
Risk Track
Validation
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo test --workspace --lockedcargo test --workspace --all-features --lockedCommands and evidence:
User-visible / Operator-visible Changes
Failure Recovery
loong tasks createno longer queuing detached background work, or background-task spawner overrides silently not taking effect.Reviewer Focus
crates/app/src/conversation/runtime.rs: hosted wrapper delegation and override precedence.crates/daemon/src/tasks_cli.rs: replacement of the bespoke runtime wrapper with the shared host wrapper.crates/app/src/conversation/runtime.rstests: regression coverage for override semantics.Summary by CodeRabbit
New Features
Refactor
Tests
Performance