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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/ironclaw_reborn_composition/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ pub enum RebornBuildError {
EmptyProductionTrustPolicy,
#[error("reborn composition requires live turn scheduler wake notifier")]
MissingTurnRunWakeNotifier,
#[error("reborn planned run-profile resolver build failed: {reason}")]
PlannedRunProfileResolver { reason: String },
Comment on lines +15 to +16

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Instead of manually mapping the error to a string, create a specific error variant for this failure mode. To maintain observability without leaking implementation details, log the detailed error from the dependency using a structured logger (e.g., tracing) at the call site and return a sanitized, generic error variant.

Suggested change
#[error("reborn planned run-profile resolver build failed: {reason}")]
PlannedRunProfileResolver { reason: String },
#[error("reborn planned run-profile resolver build failed")]
PlannedRunProfileResolverBuildFailed,
References
  1. Create specific error variants for different failure modes to provide semantically correct and clear error messages.
  2. When handling errors from external dependencies, log the detailed error using a structured logger for observability, while returning a sanitized, generic error to the caller to avoid leaking implementation details.

#[error("reborn composition failed production validation")]
ProductionWiring {
report: ironclaw_host_runtime::ProductionWiringReport,
Expand Down
15 changes: 15 additions & 0 deletions crates/ironclaw_reborn_composition/src/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ use ironclaw_processes::ProcessServices;
use ironclaw_resources::InMemoryResourceGovernor;
use ironclaw_run_state::{InMemoryApprovalRequestStore, InMemoryRunStateStore};
use ironclaw_trust::HostTrustPolicy;
#[cfg(any(feature = "libsql", feature = "postgres"))]
use ironclaw_turns::InMemoryRunProfileResolver;
use ironclaw_turns::{DefaultTurnCoordinator, InMemoryTurnStateStore};

use crate::input::RebornStorageInput;
Expand Down Expand Up @@ -226,6 +228,17 @@ fn production_wiring(
})
}

#[cfg(any(feature = "libsql", feature = "postgres"))]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The planned run-profile resolver is currently only wired for production backends (libsql and postgres). To ensure dev/prod parity, it should also be wired into the local-dev profile (around line 121). This ensures that local development environments use the same Reborn-specific run profiles and drivers as production, avoiding potential discrepancies in behavior.

fn planned_run_profile_resolver() -> Result<Arc<InMemoryRunProfileResolver>, RebornBuildError> {
Ok(Arc::new(
ironclaw_reborn::planned_driver_factory::default_planned_run_profile_resolver().map_err(
|error| RebornBuildError::PlannedRunProfileResolver {
reason: error.to_string(),
},
)?,
))
}

#[cfg(feature = "libsql")]
async fn build_libsql_production(
profile: RebornCompositionProfile,
Expand Down Expand Up @@ -281,6 +294,7 @@ async fn build_libsql_production(
.await?
.with_libsql_turn_state_store(db)
.await?
.with_run_profile_resolver(planned_run_profile_resolver()?)
.with_turn_run_wake_notifier(production_wiring.turn_run_wake_notifier);

let turn_coordinator: Arc<dyn ironclaw_turns::TurnCoordinator> =
Expand Down Expand Up @@ -343,6 +357,7 @@ async fn build_postgres_production(
.await?
.with_postgres_turn_state_store(pool)
.await?
.with_run_profile_resolver(planned_run_profile_resolver()?)
.with_turn_run_wake_notifier(production_wiring.turn_run_wake_notifier);

let turn_coordinator: Arc<dyn ironclaw_turns::TurnCoordinator> =
Expand Down
8 changes: 8 additions & 0 deletions crates/ironclaw_reborn_composition/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@ pub enum RebornCompositionError {
EventStore(#[from] RebornEventStoreError),
#[error("reborn turn substrate failed: {0}")]
Turn(#[from] TurnError),
#[error("reborn run-profile resolver substrate failed: {0}")]
RunProfile(#[from] ironclaw_turns::run_profile::RunProfileRegistryError),
}

/// Build production-wired host-runtime services over libSQL-backed substrates.
Expand Down Expand Up @@ -274,6 +276,9 @@ where
.with_capability_leases(capability_leases)
.with_secret_store(Arc::clone(&secret_store))
.with_turn_run_wake_notifier(config.turn_run_wake_notifier)
.with_run_profile_resolver(Arc::new(
ironclaw_reborn::planned_driver_factory::default_planned_run_profile_resolver()?,
))
.with_libsql_run_state_approval_store(Arc::clone(&config.database))
.await?
.with_libsql_turn_state_store(Arc::clone(&config.database))
Expand Down Expand Up @@ -337,6 +342,9 @@ where
.with_capability_leases(capability_leases)
.with_secret_store(Arc::clone(&secret_store))
.with_turn_run_wake_notifier(config.turn_run_wake_notifier)
.with_run_profile_resolver(Arc::new(
ironclaw_reborn::planned_driver_factory::default_planned_run_profile_resolver()?,
))
.with_postgres_run_state_approval_store(config.pool.clone())
.await?
.with_postgres_turn_state_store(config.pool.clone())
Expand Down
193 changes: 179 additions & 14 deletions crates/ironclaw_reborn_composition/tests/facade_factory.rs
Original file line number Diff line number Diff line change
@@ -1,35 +1,45 @@
#[cfg(feature = "libsql")]
#[cfg(any(feature = "libsql", feature = "postgres"))]
use std::sync::Arc;

#[cfg(feature = "libsql")]
use ironclaw_host_api::{EffectKind, PackageId};
#[cfg(feature = "libsql")]
#[cfg(any(feature = "libsql", feature = "postgres"))]
use chrono::Utc;
#[cfg(feature = "postgres")]
use deadpool_postgres::tokio_postgres;
#[cfg(any(feature = "libsql", feature = "postgres"))]
use ironclaw_host_api::{AgentId, EffectKind, PackageId, ProjectId, TenantId, ThreadId, UserId};
#[cfg(any(feature = "libsql", feature = "postgres"))]
use ironclaw_host_runtime::{
SchedulerTurnRunWakeNotifier, TurnRunExecutor, TurnRunExecutorError, TurnRunScheduler,
TurnRunSchedulerConfig, TurnRunSchedulerHandle,
};
#[cfg(any(feature = "libsql", feature = "postgres"))]
use ironclaw_reborn::planned_driver_factory::PLANNED_DEFAULT_PROFILE_ID;
#[cfg(all(feature = "postgres", not(feature = "libsql")))]
use ironclaw_reborn_composition::RebornCompositionProfile;
#[cfg(feature = "libsql")]
use ironclaw_reborn_composition::{RebornBuildError, RebornCompositionProfile};
use ironclaw_reborn_composition::{RebornBuildInput, RebornReadinessState, build_reborn_services};
#[cfg(feature = "libsql")]
#[cfg(any(feature = "libsql", feature = "postgres"))]
use ironclaw_secrets::SecretMaterial;
#[cfg(feature = "libsql")]
#[cfg(any(feature = "libsql", feature = "postgres"))]
use ironclaw_trust::{AdminConfig, AdminEntry, HostTrustAssignment, HostTrustPolicy};
#[cfg(feature = "libsql")]
#[cfg(any(feature = "libsql", feature = "postgres"))]
use ironclaw_turns::{
InMemoryTurnStateStore,
AcceptedMessageRef, IdempotencyKey, InMemoryTurnStateStore, ReplyTargetBindingRef,
RunProfileRequest, SourceBindingRef, SubmitTurnRequest, SubmitTurnResponse, TurnActor,
TurnScope,
runner::{ClaimedTurnRun, TurnRunTransitionPort},
};

#[cfg(feature = "libsql")]
#[cfg(any(feature = "libsql", feature = "postgres"))]
fn test_master_key() -> SecretMaterial {
SecretMaterial::from("x".repeat(32))
SecretMaterial::from("01234567890123456789012345678901")
}

#[cfg(feature = "libsql")]
#[cfg(any(feature = "libsql", feature = "postgres"))]
struct NoopTurnRunExecutor;

#[cfg(feature = "libsql")]
#[cfg(any(feature = "libsql", feature = "postgres"))]
#[async_trait::async_trait]
impl TurnRunExecutor for NoopTurnRunExecutor {
async fn execute_claimed_run(
Expand All @@ -41,7 +51,7 @@ impl TurnRunExecutor for NoopTurnRunExecutor {
}
}

#[cfg(feature = "libsql")]
#[cfg(any(feature = "libsql", feature = "postgres"))]
fn production_trust_policy() -> Arc<HostTrustPolicy> {
Arc::new(
HostTrustPolicy::new(vec![Box::new(AdminConfig::with_entries([
Expand All @@ -61,7 +71,7 @@ fn empty_trust_policy() -> Arc<HostTrustPolicy> {
Arc::new(HostTrustPolicy::empty())
}

#[cfg(feature = "libsql")]
#[cfg(any(feature = "libsql", feature = "postgres"))]
fn live_wake_notifier() -> (Arc<SchedulerTurnRunWakeNotifier>, TurnRunSchedulerHandle) {
let transitions: Arc<dyn TurnRunTransitionPort> = Arc::new(InMemoryTurnStateStore::default());
let executor: Arc<dyn TurnRunExecutor> = Arc::new(NoopTurnRunExecutor);
Expand All @@ -80,6 +90,97 @@ async fn libsql_db_at(path: impl AsRef<std::path::Path>) -> Arc<libsql::Database
)
}

#[cfg(any(feature = "libsql", feature = "postgres"))]
fn submit_turn_request(thread: &str, idempotency_key: &str) -> SubmitTurnRequest {
SubmitTurnRequest {
scope: TurnScope::new(
TenantId::new("tenant1").unwrap(),
Some(AgentId::new("agent1").unwrap()),
Some(ProjectId::new("project1").unwrap()),
ThreadId::new(thread).unwrap(),
),
actor: TurnActor::new(UserId::new("user1").unwrap()),
accepted_message_ref: AcceptedMessageRef::new(format!("message-{thread}")).unwrap(),
source_binding_ref: SourceBindingRef::new("source-web").unwrap(),
reply_target_binding_ref: ReplyTargetBindingRef::new("reply-web").unwrap(),
requested_run_profile: Some(RunProfileRequest::new("default").unwrap()),
idempotency_key: IdempotencyKey::new(idempotency_key).unwrap(),
received_at: Utc::now(),
}
}

#[cfg(feature = "postgres")]
async fn postgres_pool_or_skip() -> Option<(
testcontainers_modules::testcontainers::ContainerAsync<
testcontainers_modules::postgres::Postgres,
>,
deadpool_postgres::Pool,
String,
)> {
let (container, database_url) = start_postgres_container().await?;
let config: tokio_postgres::Config = database_url
.parse()
.expect("testcontainer database URL must parse");
let manager = deadpool_postgres::Manager::new(config, tokio_postgres::NoTls);
let pool = deadpool_postgres::Pool::builder(manager)
.max_size(4)
.build()
.expect("Postgres pool must build");
let _connection = pool
.get()
.await
.expect("Postgres testcontainer must accept connections");
Some((container, pool, database_url))
}

#[cfg(feature = "postgres")]
async fn start_postgres_container() -> Option<(
testcontainers_modules::testcontainers::ContainerAsync<
testcontainers_modules::postgres::Postgres,
>,
String,
)> {
use testcontainers_modules::testcontainers::{ImageExt, runners::AsyncRunner};

let image = testcontainers_modules::postgres::Postgres::default()
.with_db_name("ironclaw_test")
.with_user("postgres")
.with_password("postgres")
.with_tag("16-alpine");

let container = match image.start().await {
Ok(container) => container,
Err(error) => {
eprintln!(
"skipping Postgres composition tests: docker/testcontainers unavailable ({error})"
);
return None;
}
};
let host = match container.get_host().await {
Ok(host) => host,
Err(error) => {
eprintln!(
"skipping Postgres composition tests: could not resolve container host ({error})"
);
return None;
}
};
let port = match container.get_host_port_ipv4(5432).await {
Ok(port) => port,
Err(error) => {
eprintln!(
"skipping Postgres composition tests: could not resolve container port ({error})"
);
return None;
}
};
Some((
container,
format!("postgres://postgres:postgres@{host}:{port}/ironclaw_test"),
))
}

#[tokio::test]
async fn disabled_returns_empty_services() {
let services = build_reborn_services(RebornBuildInput::disabled("test-owner"))
Expand Down Expand Up @@ -240,6 +341,70 @@ async fn migration_dry_run_validates_libsql_shape() {
.await
.unwrap();

let response = services
.turn_coordinator
.as_ref()
.expect("migration dry-run must expose turn coordinator")
.submit_turn(submit_turn_request(
"thread-planned-profile",
"idem-planned-profile",
))
.await
.unwrap();
let SubmitTurnResponse::Accepted {
resolved_run_profile_id,
..
} = response;
assert_eq!(resolved_run_profile_id.as_str(), PLANNED_DEFAULT_PROFILE_ID);

handle.shutdown().await;

assert_eq!(
services.readiness.state,
RebornReadinessState::MigrationDryRunValidated
);
assert!(services.host_runtime.is_some());
assert!(services.turn_coordinator.is_some());
}

#[cfg(feature = "postgres")]
#[tokio::test]
async fn migration_dry_run_validates_postgres_planned_turn_profile() {
let Some((_container, pool, database_url)) = postgres_pool_or_skip().await else {
return;
};
let (notifier, handle) = live_wake_notifier();

let services = build_reborn_services(
RebornBuildInput::postgres(
RebornCompositionProfile::MigrationDryRun,
"test-owner",
pool,
SecretMaterial::from(database_url),
test_master_key(),
)
.with_production_trust_policy(production_trust_policy())
.with_turn_run_wake_notifier(notifier),
)
.await
.unwrap();

let response = services
.turn_coordinator
.as_ref()
.expect("migration dry-run must expose turn coordinator")
.submit_turn(submit_turn_request(
"thread-postgres-planned-profile",
"idem-postgres-planned-profile",
))
.await
.unwrap();
let SubmitTurnResponse::Accepted {
resolved_run_profile_id,
..
} = response;
assert_eq!(resolved_run_profile_id.as_str(), PLANNED_DEFAULT_PROFILE_ID);

handle.shutdown().await;

assert_eq!(
Expand Down
Loading