From d2989b543543e2ca7dddfdad35355a508fbc146b Mon Sep 17 00:00:00 2001 From: Brett Chien Date: Fri, 21 Aug 2026 23:15:22 +0800 Subject: [PATCH 1/2] feat(oabctl): extract ProvisionDriver trait for the write path (K8s driver slice 3a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit studio_api's write path (provision/scale/delete) called ECS-specific code (apply_manifests, ecsctl::scale_service, delete::run_with_bucket) directly, with no trait boundary — ADR-2's "RuntimeDriver is the only layer with vendor terms" only held for state classification, not CRUD. Introduce ProvisionDriver and an EcsDriver impl that thin-wraps the existing free functions unchanged, and route studio_api's write path through it. No behavior change: EcsDriver's methods are pass-throughs, all 85 existing oabctl tests stay green. This gives K8s driver (slice 3b, studio#97) a real seam with an actual caller to dispatch through, instead of a speculative trait with no consumer. Ref: studio#97 (K8s driver — ADR #63 slice 3, sub-slice tracking) --- crates/oabctl/Cargo.toml | 1 + crates/oabctl/src/driver.rs | 87 +++++++++++++++++++++++++++++++++ crates/oabctl/src/lib.rs | 2 + crates/oabctl/src/studio_api.rs | 24 +++++---- 4 files changed, 101 insertions(+), 13 deletions(-) create mode 100644 crates/oabctl/src/driver.rs diff --git a/crates/oabctl/Cargo.toml b/crates/oabctl/Cargo.toml index 4cee9c9..3a0bb13 100644 --- a/crates/oabctl/Cargo.toml +++ b/crates/oabctl/Cargo.toml @@ -30,6 +30,7 @@ aws-sdk-secretsmanager = "1" aws-sdk-apigatewayv2 = "1" aws-sdk-servicediscovery = "1" ecsctl = { git = "https://github.com/oablab/ecsctl.git", rev = "90a6cd1" } +async-trait = "0.1" clap = { version = "4.5", features = ["derive"] } chrono = "0.4" serde = { version = "1.0", features = ["derive"] } diff --git a/crates/oabctl/src/driver.rs b/crates/oabctl/src/driver.rs new file mode 100644 index 0000000..e43e32c --- /dev/null +++ b/crates/oabctl/src/driver.rs @@ -0,0 +1,87 @@ +//! Provisioning driver seam (K8s driver, ADR #63 slice 3, sub-slice 3a). +//! +//! `studio_api`'s write path (`provision`/`redeploy`/`scale`/`delete`) is the +//! actual surface `studio-cp` depends on for mutation. This trait carves the +//! ECS specifics out from behind that surface so a `K8sDriver` can be added +//! later (slice 3b) without changing `studio_api`'s public signatures again. +//! +//! `EcsDriver` below is a pass-through: it wraps the existing free functions +//! in `apply`/`delete`/`ecsctl` unchanged, so this file changes no behavior. +//! It has exactly one implementation and isn't yet load-bearing for dispatch +//! (there's nothing to dispatch to until 3b) — same shape as +//! `manifest::Runtime::Kubernetes`, which has sat as a validated-but-rejected +//! schema stub since slice-0 for the same reason: declare the seam, fill the +//! second side in when it exists. + +use crate::apply::{ApplyOptions, ApplyReport}; +use crate::manifest::OABServiceManifest; +use anyhow::Result; +use async_trait::async_trait; + +#[async_trait] +pub trait ProvisionDriver { + /// Create-or-update the given manifests. + async fn apply(&self, manifests: &[OABServiceManifest], opts: &ApplyOptions) -> Result; + + /// Scale a single service. OAB services carry a single bot token, so + /// `size` must be 0 (off) or 1 (on) — enforced by the implementation. + async fn scale(&self, cluster: &str, namespace: &str, name: &str, size: i32) -> Result<()>; + + /// Delete a control-plane resource (`resource` is currently always + /// `"oabservice"`). `control_plane_bucket` is the already-resolved bucket + /// (see `control_plane::resolve_bucket`) — the driver does not resolve it. + async fn delete( + &self, + resource: &str, + name: &str, + cluster: &str, + namespace: &str, + control_plane_bucket: &str, + ) -> Result<()>; +} + +/// The ECS implementation. Every method is a thin wrapper over the existing +/// `apply`/`delete`/`ecsctl` free functions — no logic moved or changed. +pub struct EcsDriver<'a> { + pub aws_config: &'a aws_config::SdkConfig, +} + +#[async_trait] +impl<'a> ProvisionDriver for EcsDriver<'a> { + async fn apply(&self, manifests: &[OABServiceManifest], opts: &ApplyOptions) -> Result { + crate::apply::apply_manifests(self.aws_config, manifests, opts) + .await + .map_err(|e| anyhow::anyhow!("apply failed [{:?}]: {e}", e.kind)) + } + + async fn scale(&self, cluster: &str, namespace: &str, name: &str, size: i32) -> Result<()> { + if size != 0 && size != 1 { + anyhow::bail!( + "invalid size: {size}. OAB services scale only to 0 (off) or 1 (on) — \ + each runs a single bot token and scaling above 1 duplicates responses." + ); + } + let service_name = format!("oab-{namespace}-{name}"); + let ecs = aws_sdk_ecs::Client::new(self.aws_config); + ecsctl::scale::scale_service(&ecs, cluster, &service_name, size, false).await + } + + async fn delete( + &self, + resource: &str, + name: &str, + cluster: &str, + namespace: &str, + control_plane_bucket: &str, + ) -> Result<()> { + crate::delete::run_with_bucket( + self.aws_config, + resource, + name, + cluster, + namespace, + control_plane_bucket, + ) + .await + } +} diff --git a/crates/oabctl/src/lib.rs b/crates/oabctl/src/lib.rs index 955de1f..e071275 100644 --- a/crates/oabctl/src/lib.rs +++ b/crates/oabctl/src/lib.rs @@ -53,6 +53,7 @@ mod config; mod control_plane; mod create; mod delete; +pub mod driver; pub mod events; mod get; mod ingress; @@ -72,6 +73,7 @@ pub use manifest::{ Runtime, Spec, }; +pub use driver::{EcsDriver, ProvisionDriver}; pub use events::{fetch_ecs_events, EcsEvent, DEFAULT_EVENTS_LOG_GROUP}; pub use status::{instance_status, service_status, InstanceStatus, ServiceStatus}; diff --git a/crates/oabctl/src/studio_api.rs b/crates/oabctl/src/studio_api.rs index 8800cf3..8f3893c 100644 --- a/crates/oabctl/src/studio_api.rs +++ b/crates/oabctl/src/studio_api.rs @@ -12,6 +12,7 @@ //! environment, so an MCP/host process with no oabctl config can still drive //! writes. +use crate::driver::ProvisionDriver; use crate::manifest::{OABFleetManifest, OABServiceManifest, RawManifest}; use anyhow::{Context, Result}; use aws_sdk_s3::primitives::ByteStream; @@ -106,14 +107,15 @@ pub async fn provision( // pulls the task up and reads config/persona/skills from it. push_bundle(config, control_plane_bucket, objects).await?; - // 2. Apply the service manifest at its chosen image tag. `apply_manifests` is - // config-free and reconciles create-or-update. + // 2. Apply the service manifest at its chosen image tag, through the + // provisioning driver seam (config-free, reconciles create-or-update). let manifests = parse_manifests(manifest_yaml)?; let mut opts = crate::apply::ApplyOptions::new(cluster); if let Some(bucket) = control_plane_bucket { opts = opts.with_control_plane_bucket(bucket); } - crate::apply::apply_manifests(config, &manifests, &opts) + crate::driver::EcsDriver { aws_config: config } + .apply(&manifests, &opts) .await .context("failed to apply manifest during provision") } @@ -199,15 +201,9 @@ pub async fn scale( name: &str, size: i32, ) -> Result<()> { - if size != 0 && size != 1 { - anyhow::bail!( - "invalid size: {size}. OAB services scale only to 0 (off) or 1 (on) — \ - each runs a single bot token and scaling above 1 duplicates responses." - ); - } - let service_name = format!("oab-{namespace}-{name}"); - let ecs = aws_sdk_ecs::Client::new(config); - ecsctl::scale::scale_service(&ecs, cluster, &service_name, size, false).await + crate::driver::EcsDriver { aws_config: config } + .scale(cluster, namespace, name, size) + .await } /// Delete a control-plane resource (currently `oabservice`). @@ -224,7 +220,9 @@ pub async fn delete( control_plane_bucket: Option<&str>, ) -> Result<()> { let bucket = crate::control_plane::resolve_bucket(config, control_plane_bucket).await?; - crate::delete::run_with_bucket(config, resource, name, cluster, namespace, &bucket).await + crate::driver::EcsDriver { aws_config: config } + .delete(resource, name, cluster, namespace, &bucket) + .await } #[cfg(test)] From 81c7167748c820c341d3e4a63a1753a130552cb4 Mon Sep 17 00:00:00 2001 From: Brett Chien Date: Sat, 22 Aug 2026 00:20:36 +0800 Subject: [PATCH 2/2] fix(oabctl): move cluster off ProvisionDriver's per-call signature, rename ecs_service_name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found while starting K8sDriver (slice 3b): ProvisionDriver::apply took apply::ApplyOptions directly (an ECS-shaped type — 'cluster' field documented as "ECS cluster name or ARN"), and scale/delete took `cluster: &str` as a per-call parameter. Neither generalizes to a k8s driver, which has no "cluster" — it has a context+namespace bound at driver-construction time, same as EcsDriver already binds aws_config. - EcsDriver now holds `cluster` as an instance field alongside aws_config, matching how it already holds the AWS credential/region context. - New ProvisionOptions carries only what's actually generic (bucket, wait); apply::ApplyOptions (still ECS-specific, used directly by apply_manifests) is unchanged. - AppliedService.ecs_service_name -> resource_name: the only field crossing the trait boundary that still had a vendor-specific name. ServiceTarget (ECS-internal error detail, never crosses the trait) keeps its ECS name — it's genuinely ECS-only, not a leak. No behavior change for the ECS path; 85/85 tests green. This was going to bite immediately on K8sDriver's first method — fixing it now, before #98 merges, is cheaper than a second breaking change to studio_api after. --- crates/oabctl/src/apply.rs | 7 ++-- crates/oabctl/src/driver.rs | 60 +++++++++++++++++++-------------- crates/oabctl/src/lib.rs | 2 +- crates/oabctl/src/studio_api.rs | 18 +++++----- 4 files changed, 48 insertions(+), 39 deletions(-) diff --git a/crates/oabctl/src/apply.rs b/crates/oabctl/src/apply.rs index 25e1806..a7edf20 100644 --- a/crates/oabctl/src/apply.rs +++ b/crates/oabctl/src/apply.rs @@ -61,7 +61,8 @@ impl From<&OABServiceManifest> for ServiceTarget { pub struct AppliedService { pub namespace: String, pub name: String, - pub ecs_service_name: String, + /// The driver-native resource name (ECS service name today). + pub resource_name: String, pub action: ApplyAction, pub webhook_urls: Vec, pub warnings: Vec, @@ -1063,7 +1064,7 @@ async fn apply_ecs( Ok(AppliedService { namespace: m.metadata.namespace.clone(), name: m.metadata.name.clone(), - ecs_service_name: service_name, + resource_name: service_name, action, webhook_urls, warnings, @@ -1290,7 +1291,7 @@ spec: let completed_service = AppliedService { namespace: "prod".to_string(), name: "done".to_string(), - ecs_service_name: "oab-prod-done".to_string(), + resource_name: "oab-prod-done".to_string(), action: ApplyAction::Updated, webhook_urls: vec!["https://example.test/webhook".to_string()], warnings: vec!["degraded".to_string()], diff --git a/crates/oabctl/src/driver.rs b/crates/oabctl/src/driver.rs index e43e32c..3d716b9 100644 --- a/crates/oabctl/src/driver.rs +++ b/crates/oabctl/src/driver.rs @@ -3,58 +3,73 @@ //! `studio_api`'s write path (`provision`/`redeploy`/`scale`/`delete`) is the //! actual surface `studio-cp` depends on for mutation. This trait carves the //! ECS specifics out from behind that surface so a `K8sDriver` can be added -//! later (slice 3b) without changing `studio_api`'s public signatures again. +//! (slice 3b) without changing `studio_api`'s public signatures again. //! //! `EcsDriver` below is a pass-through: it wraps the existing free functions -//! in `apply`/`delete`/`ecsctl` unchanged, so this file changes no behavior. +//! in `apply`/`delete`/`ecsctl` unchanged, so this file changes no ECS +//! behavior. `cluster` lives on the driver instance rather than in a per-call +//! parameter or in `ApplyOptions` — a target (ECS cluster, or later a k8s +//! context+namespace) is a property of *which driver* you built, not +//! something you repeat on every call. `ProvisionOptions` carries only the +//! fields that actually generalize across drivers (bucket, wait); ECS's own +//! `ApplyOptions` (used by `apply_manifests` directly, still ECS-specific) +//! is unaffected. +//! //! It has exactly one implementation and isn't yet load-bearing for dispatch //! (there's nothing to dispatch to until 3b) — same shape as //! `manifest::Runtime::Kubernetes`, which has sat as a validated-but-rejected //! schema stub since slice-0 for the same reason: declare the seam, fill the //! second side in when it exists. -use crate::apply::{ApplyOptions, ApplyReport}; +use crate::apply::ApplyReport; use crate::manifest::OABServiceManifest; use anyhow::Result; use async_trait::async_trait; +/// Generic apply options — the subset of `apply::ApplyOptions` that isn't +/// ECS-specific. `cluster` lives on the driver instance instead (see module +/// docs). +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct ProvisionOptions { + pub control_plane_bucket: Option, + pub wait: bool, +} + #[async_trait] pub trait ProvisionDriver { /// Create-or-update the given manifests. - async fn apply(&self, manifests: &[OABServiceManifest], opts: &ApplyOptions) -> Result; + async fn apply(&self, manifests: &[OABServiceManifest], opts: &ProvisionOptions) -> Result; /// Scale a single service. OAB services carry a single bot token, so /// `size` must be 0 (off) or 1 (on) — enforced by the implementation. - async fn scale(&self, cluster: &str, namespace: &str, name: &str, size: i32) -> Result<()>; + async fn scale(&self, namespace: &str, name: &str, size: i32) -> Result<()>; /// Delete a control-plane resource (`resource` is currently always /// `"oabservice"`). `control_plane_bucket` is the already-resolved bucket /// (see `control_plane::resolve_bucket`) — the driver does not resolve it. - async fn delete( - &self, - resource: &str, - name: &str, - cluster: &str, - namespace: &str, - control_plane_bucket: &str, - ) -> Result<()>; + async fn delete(&self, resource: &str, name: &str, namespace: &str, control_plane_bucket: &str) -> Result<()>; } /// The ECS implementation. Every method is a thin wrapper over the existing /// `apply`/`delete`/`ecsctl` free functions — no logic moved or changed. pub struct EcsDriver<'a> { pub aws_config: &'a aws_config::SdkConfig, + pub cluster: &'a str, } #[async_trait] impl<'a> ProvisionDriver for EcsDriver<'a> { - async fn apply(&self, manifests: &[OABServiceManifest], opts: &ApplyOptions) -> Result { - crate::apply::apply_manifests(self.aws_config, manifests, opts) + async fn apply(&self, manifests: &[OABServiceManifest], opts: &ProvisionOptions) -> Result { + let mut ecs_opts = crate::apply::ApplyOptions::new(self.cluster).with_wait(opts.wait); + if let Some(bucket) = &opts.control_plane_bucket { + ecs_opts = ecs_opts.with_control_plane_bucket(bucket.clone()); + } + crate::apply::apply_manifests(self.aws_config, manifests, &ecs_opts) .await .map_err(|e| anyhow::anyhow!("apply failed [{:?}]: {e}", e.kind)) } - async fn scale(&self, cluster: &str, namespace: &str, name: &str, size: i32) -> Result<()> { + async fn scale(&self, namespace: &str, name: &str, size: i32) -> Result<()> { if size != 0 && size != 1 { anyhow::bail!( "invalid size: {size}. OAB services scale only to 0 (off) or 1 (on) — \ @@ -63,22 +78,15 @@ impl<'a> ProvisionDriver for EcsDriver<'a> { } let service_name = format!("oab-{namespace}-{name}"); let ecs = aws_sdk_ecs::Client::new(self.aws_config); - ecsctl::scale::scale_service(&ecs, cluster, &service_name, size, false).await + ecsctl::scale::scale_service(&ecs, self.cluster, &service_name, size, false).await } - async fn delete( - &self, - resource: &str, - name: &str, - cluster: &str, - namespace: &str, - control_plane_bucket: &str, - ) -> Result<()> { + async fn delete(&self, resource: &str, name: &str, namespace: &str, control_plane_bucket: &str) -> Result<()> { crate::delete::run_with_bucket( self.aws_config, resource, name, - cluster, + self.cluster, namespace, control_plane_bucket, ) diff --git a/crates/oabctl/src/lib.rs b/crates/oabctl/src/lib.rs index e071275..6541921 100644 --- a/crates/oabctl/src/lib.rs +++ b/crates/oabctl/src/lib.rs @@ -73,7 +73,7 @@ pub use manifest::{ Runtime, Spec, }; -pub use driver::{EcsDriver, ProvisionDriver}; +pub use driver::{EcsDriver, ProvisionDriver, ProvisionOptions}; pub use events::{fetch_ecs_events, EcsEvent, DEFAULT_EVENTS_LOG_GROUP}; pub use status::{instance_status, service_status, InstanceStatus, ServiceStatus}; diff --git a/crates/oabctl/src/studio_api.rs b/crates/oabctl/src/studio_api.rs index 8f3893c..e73ec04 100644 --- a/crates/oabctl/src/studio_api.rs +++ b/crates/oabctl/src/studio_api.rs @@ -110,11 +110,11 @@ pub async fn provision( // 2. Apply the service manifest at its chosen image tag, through the // provisioning driver seam (config-free, reconciles create-or-update). let manifests = parse_manifests(manifest_yaml)?; - let mut opts = crate::apply::ApplyOptions::new(cluster); - if let Some(bucket) = control_plane_bucket { - opts = opts.with_control_plane_bucket(bucket); - } - crate::driver::EcsDriver { aws_config: config } + let opts = crate::driver::ProvisionOptions { + control_plane_bucket: control_plane_bucket.map(str::to_string), + wait: false, + }; + crate::driver::EcsDriver { aws_config: config, cluster } .apply(&manifests, &opts) .await .context("failed to apply manifest during provision") @@ -201,8 +201,8 @@ pub async fn scale( name: &str, size: i32, ) -> Result<()> { - crate::driver::EcsDriver { aws_config: config } - .scale(cluster, namespace, name, size) + crate::driver::EcsDriver { aws_config: config, cluster } + .scale(namespace, name, size) .await } @@ -220,8 +220,8 @@ pub async fn delete( control_plane_bucket: Option<&str>, ) -> Result<()> { let bucket = crate::control_plane::resolve_bucket(config, control_plane_bucket).await?; - crate::driver::EcsDriver { aws_config: config } - .delete(resource, name, cluster, namespace, &bucket) + crate::driver::EcsDriver { aws_config: config, cluster } + .delete(resource, name, namespace, &bucket) .await }