From 4060f003901aee495b6f816a89dcc87df1b99df6 Mon Sep 17 00:00:00 2001 From: Shooksie Date: Tue, 14 Jul 2026 13:22:50 -0600 Subject: [PATCH 01/15] fix(orchestrator-core): pin environment host across prepare/exec/teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Environment plugins are stateful across the three-call contract: prepare registers an in-memory run (e.g. a live WS relay to a remote node) that exec/exec_stream/teardown reuse by handle. The previous per-call resident-host lease dropped between calls, letting the shared registry LRU-evict or ping-reap the process between prepare and exec — losing that state and surfacing as "no live relay connection for handle". EnvironmentClient now holds ONE ResidentHostLease for its lifetime (acquired lazily on the first RPC via pinned_host), so every RPC lands on the same pinned process. The client is now non-Clone (callers Arc it) so the single lease is not duplicated. Death handling splits by call kind: exec/exec_stream stay at-most-once (no retry — side effects + lost state), while the idempotent control ops prepare/teardown retry once. Either way a dead lease is reaped generation-scoped (drop from the client + invalidate the exact generation in the registry) so the next run spawns a live process instead of re-leasing the corpse, and a concurrent call's fresh lease is never evicted by a late failure of an older generation. Removes the now-unused with_resident_host / with_resident_host_once / ping_is_dead helpers and their ping tests. --- .../src/workflow/environment_client.rs | 433 +++++++++--------- 1 file changed, 207 insertions(+), 226 deletions(-) diff --git a/crates/orchestrator-core/src/workflow/environment_client.rs b/crates/orchestrator-core/src/workflow/environment_client.rs index a6540adc..8ac34fbd 100644 --- a/crates/orchestrator-core/src/workflow/environment_client.rs +++ b/crates/orchestrator-core/src/workflow/environment_client.rs @@ -20,25 +20,44 @@ //! //! ## Resident-host model //! -//! Mirrors [`super::journal_client`] and -//! `orchestrator_config::workflow_config::config_source_client`: the client -//! routes every RPC through the process-global [`ResidentHostRegistry`] — ONE -//! warm plugin host per binary, kept across calls, with a death-aware -//! reap+respawn+retry-once wrapper ([`with_resident_host`]) and a sync↔async -//! bridge ([`run_blocking`]). +//! Draws on the process-global [`ResidentHostRegistry`] like +//! [`super::journal_client`] and +//! `orchestrator_config::workflow_config::config_source_client`, but with a +//! crucial difference: those roles are STATELESS, so they take a fresh lease per +//! call and let the host be shared / evicted between calls. Environment plugins +//! are STATEFUL across the three-call contract — `prepare` registers an in-memory +//! run (e.g. a live WS relay to a remote node) that `exec` / `exec_stream` / +//! `teardown` reuse by handle — so this client instead acquires ONE +//! [`ResidentHostLease`] on the first RPC and PINS it for its lifetime (see +//! [`EnvironmentClient::pinned_host`]). Every RPC therefore lands on the SAME warm +//! process, and the lease keeps that process safe from LRU eviction / +//! ping-reaping between `prepare` and `exec` (which would otherwise drop the +//! in-memory registration and surface as `no live relay connection for handle`). +//! A sync↔async bridge ([`run_blocking`]) drives the calls from the sync API. +//! +//! Death handling splits by call kind. `exec` / `exec_stream` are AT-MOST-ONCE: a +//! death-like failure is NOT retried — the RPC may already have run with side +//! effects, and the run's in-memory state died with the process, so the call +//! fails and any orphaned remote node is reclaimed by the plugin's own GC sweep. +//! The idempotent CONTROL ops (`prepare` / `teardown`) DO retry once: a `prepare` +//! that died before returning its handle left nothing usable, so a fresh prepare +//! on a fresh host is the correct recovery (and re-pins that host for the rest of +//! the run), and `teardown` is a dispose-by-id. Either way a dead lease is REAPED +//! — dropped from this client and its generation invalidated in the registry — so +//! the retry (or the NEXT run) spawns a live process instead of re-leasing the +//! corpse (which would otherwise keep failing with `ConnectionLost` until +//! LRU/shutdown). //! //! Environment plugins spawn with the same base context as `config_source` / -//! `workflow_journal` (full parent env forwarded, no working dir), so a single -//! plugin binary that ALSO serves one of those roles collapses to one shared -//! process (see [`orchestrator_plugin_host::resident_host_registry`] for the -//! spawn-context fingerprint semantics). Environment plugins run full-env because -//! they materialize workspaces (clone URLs, region/host config) the same way -//! config_source replaces the kernel's env-reading interpolator. Unlike those -//! non-streaming roles, environment honors the manifest's -//! `notification_buffer_size` (it is the streaming role — `exec_stream` fans -//! `environment/output` through the host broadcast channel), so a plugin that -//! declares a larger buffer gets its own correctly-sized host; a plugin that -//! declares none still shares the collapsed process. +//! `workflow_journal` (full parent env forwarded, no working dir); the pinned +//! lease is keyed by the same binary-path + mtime + spawn-context fingerprint +//! (see [`orchestrator_plugin_host::resident_host_registry`]). Environment plugins +//! run full-env because they materialize workspaces (clone URLs, region/host +//! config) the same way config_source replaces the kernel's env-reading +//! interpolator. Unlike those non-streaming roles, environment honors the +//! manifest's `notification_buffer_size` (it is the streaming role — `exec_stream` +//! fans `environment/output` through the host broadcast channel), so a plugin that +//! declares a larger buffer gets a correctly-sized host. //! //! ## Discovery //! @@ -91,11 +110,36 @@ const EXEC_RPC_TIMEOUT_HEADROOM: Duration = Duration::from_secs(30); /// Host-side client bound to one installed `environment` plugin for one project /// root. Cheap to construct (discovery only, no spawn); the warm plugin process -/// is spawned lazily on the first RPC and shared via the resident-host registry. -#[derive(Debug, Clone)] +/// is spawned lazily on the first RPC and PINNED for the client's lifetime. +/// +/// Environment plugins are STATEFUL across the three-call contract: `prepare` +/// registers an in-memory run (e.g. a live WS relay connection to a remote node) +/// that `exec` / `exec_stream` / `teardown` then reuse by handle. A per-call +/// resident-host lease (as the stateless `config_source` / `journal` roles use) +/// would drop the lease between calls, letting the shared registry LRU-evict or +/// ping-reap the process BETWEEN `prepare` and `exec` — losing that in-memory +/// state (surfacing as `no live relay connection for handle`). This client +/// therefore holds ONE [`ResidentHostLease`] for its own lifetime, so every RPC +/// lands on the SAME pinned process. Non-`Clone` on purpose: callers share it via +/// `Arc` so the single lease is not duplicated. pub struct EnvironmentClient { plugin: DiscoveredPlugin, project_root: PathBuf, + /// The resident-host lease pinned for this client's lifetime, acquired lazily + /// on the first RPC (see [`Self::pinned_host`]) and then reused by every + /// subsequent call so `prepare` → `exec`/`exec_stream` → `teardown` share the + /// SAME plugin process (and thus its in-memory run state). + pinned: tokio::sync::Mutex>, +} + +impl std::fmt::Debug for EnvironmentClient { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // `pinned` holds a `ResidentHostLease` (not `Debug`); omit it. + f.debug_struct("EnvironmentClient") + .field("plugin", &self.plugin) + .field("project_root", &self.project_root) + .finish_non_exhaustive() + } } impl EnvironmentClient { @@ -116,7 +160,7 @@ impl EnvironmentClient { )); } let plugin = select_environment_plugin(plugins, plugin_id)?; - Ok(Self { plugin, project_root: project_root.to_path_buf() }) + Ok(Self { plugin, project_root: project_root.to_path_buf(), pinned: tokio::sync::Mutex::new(None) }) } /// The bound plugin's discovered name. @@ -145,12 +189,12 @@ impl EnvironmentClient { /// its own limit; `timeout = None` leaves the RPC unbounded (the environment /// enforces its own policy) so an un-timed long command is not aborted. /// - /// AT-MOST-ONCE: unlike the control RPCs, exec is NOT retried on a death-like - /// host failure — a harness command may have side effects (file mutations, - /// git, deploys) and could already have run before the plugin died, so a - /// blind retry risks double-execution. A spawn happens once (the command has - /// not run yet at spawn time); a failure after the request is sent surfaces - /// as an error for the caller to handle. + /// AT-MOST-ONCE: exec runs against the client's pinned host and is NOT retried + /// on a death-like host failure — a harness command may have side effects (file + /// mutations, git, deploys) and could already have run before the plugin died, + /// so a blind retry risks double-execution; and the pinned process holds the + /// run's in-memory state, which a fresh process could not recover anyway. A + /// failure surfaces as an error for the caller to handle. pub fn exec( &self, handle: &EnvironmentHandle, @@ -162,7 +206,7 @@ impl EnvironmentClient { let request = build_exec_request(handle, command, extra_env, stdin, timeout); let rpc_timeout = exec_rpc_timeout(request.timeout_secs); let params = serde_json::to_value(&request).context("serializing ExecRequest for environment/exec")?; - let value = run_blocking(with_resident_host_once(&self.plugin, &self.project_root, move |host| async move { + let value = run_blocking(self.with_pinned_host(move |host| async move { environment_rpc(&host, METHOD_ENVIRONMENT_EXEC, params, rpc_timeout).await }))??; serde_json::from_value(value) @@ -205,7 +249,7 @@ impl EnvironmentClient { let handle_id = handle.id.clone(); let params = serde_json::to_value(&request).context("serializing ExecRequest for environment/exec_stream")?; - let value = run_blocking(with_resident_host_once(&self.plugin, &self.project_root, move |host| async move { + let value = run_blocking(self.with_pinned_host(move |host| async move { exec_stream_call(&host, params, &handle_id, rpc_timeout, &on_output).await }))??; @@ -223,24 +267,147 @@ impl EnvironmentClient { Ok(()) } - /// Serialize `params`, run a CONTROL RPC (prepare / teardown) against the - /// resident host (spawning it once if absent), reap+respawn+retry-once on a - /// death-like failure, and return the raw response `Value`. + /// Serialize `params` and run a CONTROL RPC (prepare / teardown) against this + /// client's PINNED resident host, returning the raw response `Value`. /// - /// Retry is safe here because the control ops are effectively idempotent from - /// the caller's view: a teardown re-issues a dispose-by-id (a no-op if - /// already gone), and a prepare that died before returning its handle left no - /// handle the caller can use, so a fresh prepare is the correct recovery. - /// Exec is deliberately NOT routed through this path (see [`Self::exec`]). + /// Control RPCs go through the single pinned host so the whole `prepare` → … + /// → `teardown` sequence reuses one process (and its in-memory run state). + /// Unlike `exec`, a control RPC is RETRIED ONCE on a death-like failure (see + /// [`Self::control_rpc`]): a control op is safe to replay — a `prepare` that + /// died before returning its handle left no usable handle (a fresh prepare + /// on a fresh host is the correct recovery, and it re-pins that fresh host for + /// the subsequent exec/teardown), and `teardown` is an idempotent + /// dispose-by-id. fn call_blocking

(&self, method: &'static str, params: P, timeout: Duration) -> Result where P: serde::Serialize + Send + 'static, { let params = serde_json::to_value(¶ms).with_context(|| format!("serializing params for {method}"))?; - run_blocking(with_resident_host(&self.plugin, &self.project_root, move |host| { - let params = params.clone(); - async move { environment_rpc(&host, method, params, Some(timeout)).await } - }))? + run_blocking(self.control_rpc(method, params, timeout))? + } + + /// A control RPC (prepare / teardown) against the pinned host, retried ONCE on + /// a death-like failure. + /// + /// The first attempt's [`Self::run_once`] already reaped the dead lease, so the + /// retry's [`Self::pinned_host`] leases a freshly-spawned host — and stores it + /// in `self.pinned`, so a retried `prepare` re-pins that live host for the rest + /// of the run. A structured (non-death) error is NOT retried; it would only + /// fail the same way. Retry is safe ONLY for these idempotent control ops — + /// `exec` / `exec_stream` deliberately go through [`Self::with_pinned_host`] + /// (at-most-once) instead. + async fn control_rpc(&self, method: &'static str, params: Value, timeout: Duration) -> Result { + let params_retry = params.clone(); + match self + .run_once(move |host| async move { environment_rpc(&host, method, params, Some(timeout)).await }) + .await + { + Ok(value) => Ok(value), + Err(ResidentCallError::Other(err)) => Err(err), + Err(ResidentCallError::Death(_)) => match self + .run_once(move |host| async move { environment_rpc(&host, method, params_retry, Some(timeout)).await }) + .await + { + Ok(value) => Ok(value), + Err(ResidentCallError::Death(err)) | Err(ResidentCallError::Other(err)) => Err(err), + }, + } + } + + /// The client's pinned resident host plus its lease GENERATION, acquiring the + /// lease on first use. + /// + /// The lease is stored in `self.pinned` and held for the client's whole + /// lifetime, so it never becomes LRU-evictable or ping-reapable between RPCs — + /// every call returns a clone of the SAME [`PluginHost`], preserving the + /// plugin's in-memory run registration across `prepare` → exec → `teardown`. + /// The returned generation identifies exactly this host so a death-like failure + /// reaps only this generation (see [`Self::reap_pinned`]). + async fn pinned_host(&self) -> Result<(PluginHost, u64)> { + let mut guard = self.pinned.lock().await; + if guard.is_none() { + let lease = acquire_resident_lease( + &global_resident_host_registry(), + &self.plugin, + binary_mtime_nanos(&self.plugin.path), + &environment_spawn_context(self.plugin.manifest.notification_buffer_size), + ) + .await?; + *guard = Some(lease); + } + let lease = guard.as_ref().expect("lease populated above"); + Ok((lease.host().clone(), lease.generation())) + } + + /// Run `call` against this client's pinned host EXACTLY once (no retry), + /// mapping a resident-call error back to `anyhow`. + /// + /// Used by `exec` / `exec_stream`: the pinned host holds the run's in-memory + /// state (the WS relay registration), so a death-like failure means that state + /// is already gone and a fresh process could not recover it, and the RPC may + /// already have run with side effects — so the call fails (at-most-once) and + /// any orphaned remote node is reclaimed by the plugin's own GC sweep. The dead + /// lease is still reaped (inside [`Self::run_once`]) so the NEXT run spawns a + /// live process; reaping is not a replay. + async fn with_pinned_host(&self, call: F) -> Result + where + F: FnOnce(PluginHost) -> Fut, + Fut: Future>, + { + match self.run_once(call).await { + Ok(value) => Ok(value), + Err(ResidentCallError::Death(err)) | Err(ResidentCallError::Other(err)) => Err(err), + } + } + + /// Acquire the pinned host and run `call` once, REAPING the pinned lease on a + /// death-like failure (so the next acquire spawns a fresh host) and returning + /// the classified error so the caller can decide whether to retry. Never + /// replays `call`. + async fn run_once(&self, call: F) -> std::result::Result + where + F: FnOnce(PluginHost) -> Fut, + Fut: Future>, + { + let (host, generation) = self.pinned_host().await.map_err(ResidentCallError::Other)?; + match call(host).await { + Ok(value) => Ok(value), + Err(ResidentCallError::Death(err)) => { + self.reap_pinned(generation).await; + Err(ResidentCallError::Death(err)) + } + Err(other) => Err(other), + } + } + + /// Reap the host generation `generation` that just failed: drop this client's + /// pinned lease iff it is still that exact generation, and invalidate that + /// generation in the shared registry, so a dead host is not re-leased by the + /// next run. + /// + /// Strictly generation-scoped, which makes it safe under concurrent RPCs on one + /// `Arc`: if another call has meanwhile re-acquired a FRESH + /// lease (a different generation), this leaves BOTH `self.pinned` and the + /// registry entry for that new host untouched — a late failure of the OLD + /// generation can never evict the healthy replacement. Idempotent: reaping the + /// same generation twice is a no-op the second time. + async fn reap_pinned(&self, generation: u64) { + { + let mut guard = self.pinned.lock().await; + if guard.as_ref().is_some_and(|lease| lease.generation() == generation) { + // Drops the dead lease, releasing its `active` ref so the registry + // can evict the entry below. + *guard = None; + } + } + global_resident_host_registry() + .invalidate_generation( + &self.plugin.path, + binary_mtime_nanos(&self.plugin.path), + &environment_spawn_context(self.plugin.manifest.notification_buffer_size), + generation, + ) + .await; } } @@ -447,137 +614,6 @@ where } } -/// Acquire the shared resident host for `plugin` and run `call` against a clone -/// of it, retrying once (reap + re-spawn) on a death-like failure. All other -/// errors propagate without a re-spawn. -/// -/// The host lives in the process-global [`ResidentHostRegistry`] keyed by the -/// plugin's binary path + mtime + spawn-context, shared with the other -/// full-env resident roles. The lease is held across the RPC `.await` so LRU -/// pressure from another role can never evict the host mid-call. -async fn with_resident_host(plugin: &DiscoveredPlugin, _project_root: &Path, mut call: F) -> Result -where - F: FnMut(PluginHost) -> Fut, - Fut: Future>, -{ - let registry = global_resident_host_registry(); - let mtime = binary_mtime_nanos(&plugin.path); - let context = environment_spawn_context(plugin.manifest.notification_buffer_size); - - let lease = acquire_resident_lease(®istry, plugin, mtime, &context).await?; - let generation = lease.generation(); - match call(lease.host().clone()).await { - Ok(value) => Ok(value), - Err(ResidentCallError::Other(err)) => Err(err), - Err(ResidentCallError::Death(err)) => { - // Reap ONLY the exact host that failed (a concurrent caller may have - // already replaced it), then re-spawn once and retry. - drop(lease); - registry.invalidate_generation(&plugin.path, mtime, &context, generation).await; - let lease = acquire_resident_lease(®istry, plugin, mtime, &context).await?; - match call(lease.host().clone()).await { - Ok(value) => Ok(value), - Err(ResidentCallError::Other(retry_err)) => Err(retry_err), - Err(ResidentCallError::Death(retry_err)) => Err(retry_err.context(format!( - "environment plugin {} still failing after one re-spawn (first error: {err})", - plugin.name - ))), - } - } - } -} - -/// AT-MOST-ONCE variant of [`with_resident_host`]: run the side-effectful `call` -/// against the shared resident host EXACTLY once, but heal a stale/idle-dead -/// cached host FIRST so the single attempt lands on a live process. Used by -/// `exec` / `exec_stream`. -/// -/// Liveness preflight (closes the idle-death window without risking a double -/// run): the resident registry hands back a cached host without checking its -/// process is still alive, so a host that exited while idle would fail the first -/// exec with `ConnectionLost` even though the command never ran. Before sending -/// the command we `$/ping` the leased host; a death-like ping failure means the -/// process is already gone, so we reap that generation and re-spawn a fresh host -/// — the command has NOT been sent, so this is safe. A structured ping error (or -/// a plugin that does not implement `$/ping`) counts as alive; only a death-like -/// ping outcome triggers the pre-send re-spawn. -/// -/// After the ping-verified send, a death-like failure is NOT retried — the -/// command may already have run with side effects on the now-dead process, so a -/// blind re-issue risks double-execution. The dead generation is still reaped so -/// the NEXT exec spawns a fresh host rather than leasing the wedged one. -async fn with_resident_host_once(plugin: &DiscoveredPlugin, _project_root: &Path, call: F) -> Result -where - F: FnOnce(PluginHost) -> Fut, - Fut: Future>, -{ - let registry = global_resident_host_registry(); - let mtime = binary_mtime_nanos(&plugin.path); - let context = environment_spawn_context(plugin.manifest.notification_buffer_size); - - // Lease + liveness preflight. A freshly-spawned host (cache miss) was just - // handshaked, so it is live and needs no ping; only a cache-reused host can - // be idle-dead. We ping unconditionally for simplicity — it is a sub-ms - // round-trip on a healthy host — and re-spawn once if the ping is death-like. - let mut lease = acquire_resident_lease(®istry, plugin, mtime, &context).await?; - if ping_is_dead(lease.host()).await { - let dead_generation = lease.generation(); - drop(lease); - registry.invalidate_generation(&plugin.path, mtime, &context, dead_generation).await; - // Re-spawn a fresh host; if THIS one is also unusable we surface the - // spawn/handshake error (still nothing executed). - lease = acquire_resident_lease(®istry, plugin, mtime, &context).await?; - } - - let generation = lease.generation(); - match call(lease.host().clone()).await { - Ok(value) => Ok(value), - Err(ResidentCallError::Other(err)) => Err(err), - Err(ResidentCallError::Death(err)) => { - // Do NOT re-run the command (it may already have executed with side - // effects), but the host's process is presumed dead — reap this exact - // generation so the NEXT exec spawns a fresh host instead of leasing a - // wedged one and failing until a control call happens to invalidate it. - drop(lease); - registry.invalidate_generation(&plugin.path, mtime, &context, generation).await; - Err(err) - } - } -} - -/// `$/ping` the host and report whether its transport is dead. For a LIVENESS -/// probe, any plugin response — success OR any RPC error (including a structured -/// error, or a plugin that does not implement `$/ping`) — proves the process is -/// alive; only a CLOSED transport (connection lost / process exited) counts as -/// dead, so the exec preflight never re-spawns a live-but-quiet plugin. This -/// deliberately does NOT use `classify`, which maps unstructured `Rpc` errors to -/// `DeathLike` for its retry-decision purpose — the opposite interpretation of -/// what a liveness probe needs. -/// -/// A `Timeout` is treated as ALIVE, not dead: the resident host is shared across -/// roles and callers, so a plugin busy serving another in-flight `exec` may not -/// answer `$/ping` within the short probe window even though its process is fine. -/// Reaping that generation here would `shutdown()` the process and abort the -/// concurrent command. A genuinely wedged host still fails the subsequent exec on -/// its own — same outcome, no collateral damage to a live sibling call. Only a -/// closed transport is an unambiguous death signal (the idle-exit case this -/// preflight exists to catch surfaces as `ConnectionLost`). -async fn ping_is_dead(host: &PluginHost) -> bool { - const PING_TIMEOUT: Duration = Duration::from_secs(2); - match host.request_typed_with_timeout("$/ping", None, PING_TIMEOUT).await { - Ok(_) => false, - // Any RPC error frame came back over a live transport: the plugin is up. - Err(HostError::Rpc(_)) => false, - // A protocol/capability rejection is still a response from a live process. - Err(HostError::IncompatibleProtocol(_)) | Err(HostError::CapabilityNotSupported(_)) => false, - // Ambiguous (busy vs wedged) — err toward alive so a busy shared host is - // never killed out from under a concurrent in-flight command. - Err(HostError::Timeout(_)) => false, - // Closed transport: the process is gone / unreachable — unambiguously dead. - Err(HostError::ConnectionLost) | Err(HostError::ProcessExited(_)) => true, - } -} - /// Lease the shared resident host for `plugin`, spawning + handshaking it once /// via the [`ResidentHostRegistry`] if it is not already live. A handshake /// failure inside the spawn closure tears the half-started child down (it is @@ -828,61 +864,6 @@ mod tests { EnvironmentHandle { id: "env-1".to_string(), workspace_root: "/work".to_string(), metadata: Value::Null } } - /// A host whose plugin task answers `initialize` then exits, so its reader - /// closes and any subsequent request (e.g. `$/ping`) sees `ConnectionLost`. - async fn dead_after_handshake_host() -> PluginHost { - let (host_reader, mut plugin_writer) = duplex(8192); - let (plugin_reader, host_writer) = duplex(8192); - tokio::spawn(async move { - let mut reader = BufReader::new(plugin_reader); - let mut line = String::new(); - if reader.read_line(&mut line).await.expect("read line") > 0 { - let request: RpcRequest = serde_json::from_str(line.trim()).expect("parse request"); - let result = InitializeResult { - protocol_version: "1.0.0".to_string(), - plugin_info: PluginInfo { - name: "dead".to_string(), - version: "0.1.0".to_string(), - plugin_kind: PLUGIN_KIND_ENVIRONMENT.to_string(), - plugin_kinds: vec![PLUGIN_KIND_ENVIRONMENT.to_string()], - description: None, - }, - capabilities: PluginCapabilities::default(), - kind_capabilities: std::collections::HashMap::new(), - }; - write_response(&mut plugin_writer, RpcResponse::ok(request.id, serde_json::json!(result))).await; - } - // Read the follow-up `$/ping` request (so the handshake response is - // fully consumed by the host before we drop the transport), then - // return WITHOUT responding: the task ends, both duplex ends drop, - // and the pending `$/ping` awaiter observes ConnectionLost. - let mut ping = String::new(); - let _ = reader.read_line(&mut ping).await; - // Task returns here: both duplex ends drop, the host's reader closes. - }); - PluginHost::from_streams("dead", host_reader, host_writer) - } - - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn ping_treats_method_not_supported_as_alive() { - // The fake answers `$/ping` with METHOD_NOT_SUPPORTED (a structured - // error): the process is up, so the exec preflight must NOT re-spawn it. - let spawns = Arc::new(AtomicUsize::new(0)); - let host = fake_environment_host(spawns).await; - host.handshake().await.expect("handshake"); - assert!(!ping_is_dead(&host).await, "a live plugin that lacks $/ping is alive, not dead"); - } - - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn ping_detects_a_dead_host() { - // The plugin exits right after the handshake, so `$/ping` sees the - // transport close (ConnectionLost) — death-like, so the preflight - // re-spawns before sending a side-effectful exec. - let host = dead_after_handshake_host().await; - host.handshake().await.expect("handshake"); - assert!(ping_is_dead(&host).await, "a plugin whose transport closed is dead"); - } - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn prepare_exec_teardown_round_trip() { let _guard = registry_lock().lock().await; From 4b1b4cc2d317d22cee4be6516c7da3e5b0024ce3 Mon Sep 17 00:00:00 2001 From: Shooksie Date: Tue, 14 Jul 2026 20:12:00 -0600 Subject: [PATCH 02/15] fix(orchestrator-core): give environment/prepare its own 6min RPC timeout (REQUIREMENT-049) prepare shared the 60s ENVIRONMENT_RPC_TIMEOUT, but materializing a real execution context is slow + fail-prone: the Railway environment plugin creates a service, waits for deploy, and blocks up to its 300s dial timeout for the container to dial home, plus workspace clone time. A 60s host-side timeout fired mid-create and control_rpc's retry-once spun up a SECOND node while the first was still deploying (leak + failure on slow deploys). Dedicated ENVIRONMENT_PREPARE_TIMEOUT of 360s covers dial + clone + margin. teardown keeps the 60s control bound. --- .../src/workflow/environment_client.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/orchestrator-core/src/workflow/environment_client.rs b/crates/orchestrator-core/src/workflow/environment_client.rs index 8ac34fbd..8545002f 100644 --- a/crates/orchestrator-core/src/workflow/environment_client.rs +++ b/crates/orchestrator-core/src/workflow/environment_client.rs @@ -108,6 +108,17 @@ const ENVIRONMENT_RPC_TIMEOUT: Duration = Duration::from_secs(60); /// return `timed_out = true` before the host gives up on the RPC. const EXEC_RPC_TIMEOUT_HEADROOM: Duration = Duration::from_secs(30); +/// `environment/prepare` gets its OWN (much longer) RPC timeout, separate from +/// the 60s control-op bound. Materializing a real execution context can be slow +/// and fail-prone: the Railway environment plugin, for example, creates a +/// service, waits for it to deploy, and blocks up to its dial timeout (default +/// 300s) for the container to dial home — plus workspace clone time on top. If +/// the host-side timeout fired mid-create, `control_rpc`'s retry-once would spin +/// up a SECOND node while the first was still deploying (a guaranteed leak + +/// failure on slow deploys). Six minutes comfortably covers dial + clone + margin. +#[allow(clippy::duration_suboptimal_units)] +const ENVIRONMENT_PREPARE_TIMEOUT: Duration = Duration::from_secs(360); + /// Host-side client bound to one installed `environment` plugin for one project /// root. Cheap to construct (discovery only, no spawn); the warm plugin process /// is spawned lazily on the first RPC and PINNED for the client's lifetime. @@ -172,7 +183,7 @@ impl EnvironmentClient { /// `spec` and return its [`EnvironmentHandle`]. pub fn prepare(&self, spec: EnvironmentSpec) -> Result { let request = PrepareRequest { spec }; - let value = self.call_blocking(METHOD_ENVIRONMENT_PREPARE, request, ENVIRONMENT_RPC_TIMEOUT)?; + let value = self.call_blocking(METHOD_ENVIRONMENT_PREPARE, request, ENVIRONMENT_PREPARE_TIMEOUT)?; let resp: PrepareResponse = serde_json::from_value(value) .with_context(|| format!("decoding PrepareResponse from environment plugin {}", self.plugin.name))?; Ok(resp.handle) From 62ddd0d878a35f9457ddfb7d01dccf8117103ef6 Mon Sep 17 00:00:00 2001 From: Shooksie Date: Tue, 14 Jul 2026 20:59:56 -0600 Subject: [PATCH 03/15] feat(daemon): cross-phase ephemeral-environment broker (REQ-048) Move ephemeral node lifecycle from the per-phase runner into the daemon so all phases of ONE workflow run share ONE node, torn down once at run end. - New `dispatch::environment_broker::EnvironmentBroker`: a `run_id -> lease` map with single-flight `acquire` (per-run mutex; first call resolves a daemon-resident `EnvironmentClient` + `prepare`s the node once, later calls return the same handle), `exec_stream` proxy, and idempotent `teardown`. Resolving the client daemon-side keeps ONE plugin process (one relay = one node) pinned across prepare/exec/teardown for the whole run. - Private local-socket server (interprocess::local_socket tokio, newline-JSON) implementing the Acquire + Exec wire contract, per-daemon bearer auth, owner-only socket under the scoped state dir. - Durable JSON lease records (`workflow-environments/.json`, temp+rename before prepare); startup reaper cold-tears-down records owned by a prior daemon instance (dead relay), then deletes them. - ProcessManager: when a dispatch routes to a NON-local environment (config routing rules/default) and a broker is wired, set the four ANIMUS_ENVIRONMENT_BROKER_* env vars on the runner and inject metadata.animus_run_id into the prepared spec. run_id is stable across a run's phases (resume dispatches reuse the persisted workflow id; a fresh first phase mints one the runner adopts). - Terminal-state teardown wired into completed-process reconciliation (Completed/Failed/Cancelled/Escalated => teardown; Running/Paused => keep). - Broker constructed + threaded into ProcessManager in the daemon composition path (bind failure is non-fatal: runners fall back to per-phase preparation). --- Cargo.lock | 3 + .../runtime/runtime_daemon/daemon_run.rs | 19 + crates/orchestrator-daemon-runtime/Cargo.toml | 3 +- .../src/dispatch/environment_broker.rs | 829 ++++++++++++++++++ .../src/dispatch/mod.rs | 5 + .../src/dispatch/process_manager.rs | 136 +++ crates/orchestrator-daemon-runtime/src/lib.rs | 10 +- 7 files changed, 1000 insertions(+), 5 deletions(-) create mode 100644 crates/orchestrator-daemon-runtime/src/dispatch/environment_broker.rs diff --git a/Cargo.lock b/Cargo.lock index 14a5eb85..0119cb38 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2153,8 +2153,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "069323743400cb7ab06a8fe5c1ed911d36b6919ec531661d034c89083629595b" dependencies = [ "doctest-file", + "futures-core", "libc", "recvmsg", + "tokio", "widestring", "windows-sys 0.61.2", ] @@ -2895,6 +2897,7 @@ name = "orchestrator-daemon-runtime" version = "0.1.0" dependencies = [ "animus-actor", + "animus-config-protocol", "animus-control-protocol", "animus-log-storage-protocol", "animus-plugin-protocol 0.1.18", diff --git a/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_run.rs b/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_run.rs index 62a62dfb..962a6b67 100644 --- a/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_run.rs +++ b/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_run.rs @@ -766,6 +766,25 @@ pub(super) async fn handle_daemon_run(args: DaemonRunArgs, project_root: &str, j let mut process_manager = ProcessManager::new().with_timeout(runtime_options.phase_timeout_secs); process_manager.phase_routing = daemon_config.and_then(|d| d.phase_routing.clone()); process_manager.mcp_config = daemon_config.and_then(|d| d.mcp.clone()); + // REQ-048 cross-phase environment broker: route non-local environments + // through a daemon-owned node shared by every phase of a run. The routing + // table decides which dispatches are brokered; the broker binds a private + // local socket and reaps any nodes leaked by a prior daemon instance. A + // bind failure is non-fatal — the runner then falls back to its own + // per-phase environment path. + process_manager.environment_routing = workflow_config.config.environment_routing.clone(); + match orchestrator_daemon_runtime::EnvironmentBroker::start(project_root).await { + Ok(broker) => { + process_manager = process_manager.with_environment_broker(broker); + } + Err(error) => { + tracing::warn!( + target: "animus.runtime.environment_broker", + %error, + "failed to start environment broker; workflow runners fall back to per-phase environment preparation" + ); + } + } let mut host = CliDaemonRunHost::new(project_root, json, start_config); let logger = host.logger(); let mut driver: SlimProjectTickDriver<'_> = diff --git a/crates/orchestrator-daemon-runtime/Cargo.toml b/crates/orchestrator-daemon-runtime/Cargo.toml index ede8b42e..1da23df1 100644 --- a/crates/orchestrator-daemon-runtime/Cargo.toml +++ b/crates/orchestrator-daemon-runtime/Cargo.toml @@ -24,6 +24,7 @@ orchestrator-core = { workspace = true } orchestrator-logging = { workspace = true } orchestrator-plugin-host = { workspace = true } animus-plugin-protocol = { workspace = true } +animus-config-protocol = { workspace = true } # All animus-protocol git deps pin the SAME tag (v0.1.26) so the transitively # pulled `animus-subject-protocol` resolves to ONE source rev. v0.1.26 additively # adds `actor: Option` to the control request types relayed by the daemon. @@ -45,7 +46,7 @@ animus-runtime-shared = { workspace = true } # v0.5.1 fold-in (item 7): unified Unix-socket + Windows-named-pipe client # for the reattach back-channel. Replaces the previous `cfg(unix)`-only # `tokio::net::UnixStream` path with a single cross-platform implementation. -interprocess = "2.4" +interprocess = { version = "2.4", features = ["tokio"] } [dev-dependencies] protocol = { workspace = true, features = ["test-utils"] } diff --git a/crates/orchestrator-daemon-runtime/src/dispatch/environment_broker.rs b/crates/orchestrator-daemon-runtime/src/dispatch/environment_broker.rs new file mode 100644 index 00000000..eeef6f21 --- /dev/null +++ b/crates/orchestrator-daemon-runtime/src/dispatch/environment_broker.rs @@ -0,0 +1,829 @@ +//! Cross-phase ephemeral-environment broker (daemon side). +//! +//! The daemon dispatches ONE workflow-runner subprocess PER PHASE. Without a +//! broker each runner would prepare its OWN ephemeral node (via the in-runner +//! [`EnvironmentClient`]) and tear it down at exit, so phases of one workflow run +//! never share a workspace. This broker moves node lifecycle into the DAEMON: +//! ONE node per workflow RUN, shared by every phase, torn down once at run end. +//! +//! ## Model +//! +//! - The broker keeps a `run_id -> lease` map. The FIRST [`Self::acquire`] for a +//! run resolves a daemon-resident [`EnvironmentClient`] and prepares the node +//! ONCE (single-flight, per-`run_id` mutex); later acquires for the same run +//! return the SAME handle without re-preparing. +//! - Because the client is resolved DAEMON-side, the process-global +//! resident-host registry keeps ONE plugin process alive for the whole run +//! (one relay = one node), pinned across prepare / exec / teardown. +//! - [`Self::teardown`] disposes the node once, at terminal workflow state. +//! - Durable JSON lease records under the scoped state root let a fresh daemon +//! reap nodes leaked by a PRIOR daemon instance (its relay is dead) on startup. +//! +//! ## IPC +//! +//! Each per-phase runner talks to the broker over a private local socket +//! (newline-delimited JSON, [`interprocess::local_socket`]). The wire is a +//! PRIVATE daemon<->runner contract — NOT `animus-protocol` — with two RPCs: +//! `acquire` (one request / one response) and `exec` (one request then a stream +//! of output frames and a terminal frame). The serde structs are defined +//! independently here and in the runner; see `broker-wire-contract.md`. + +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex as StdMutex}; +use std::time::Duration; + +use anyhow::{anyhow, bail, Context, Result}; +use serde::{Deserialize, Serialize}; +use serde_json::json; +use tokio::sync::Mutex as AsyncMutex; + +use orchestrator_core::environment::{EnvironmentHandle, EnvironmentSpec, ExecStream, HarnessCommand}; +use orchestrator_core::EnvironmentClient; + +/// Local-socket path the per-phase runner dials to reach the broker. +pub const ANIMUS_ENVIRONMENT_BROKER_SOCKET_ENV: &str = "ANIMUS_ENVIRONMENT_BROKER_SOCKET"; +/// Per-daemon bearer capability echoed on every broker frame. +pub const ANIMUS_ENVIRONMENT_BROKER_TOKEN_ENV: &str = "ANIMUS_ENVIRONMENT_BROKER_TOKEN"; +/// Workflow run id this dispatch belongs to (the broker's single-flight key). +pub const ANIMUS_ENVIRONMENT_BROKER_RUN_ID_ENV: &str = "ANIMUS_ENVIRONMENT_BROKER_RUN_ID"; +/// Resolved environment plugin id (e.g. `animus-environment-railway`). +pub const ANIMUS_ENVIRONMENT_BROKER_ENVIRONMENT_ID_ENV: &str = "ANIMUS_ENVIRONMENT_BROKER_ENVIRONMENT_ID"; + +/// Environment plugin ids that materialize a LOCAL workspace on the daemon host. +/// A run routed to one of these does NOT go through the broker — the per-phase +/// node-sharing problem is specific to remote/ephemeral environments. +const LOCAL_ENVIRONMENT_IDS: &[&str] = &["worktree", "local"]; + +/// `true` when `environment_id` names a local (non-brokered) environment. +pub fn is_local_environment(environment_id: &str) -> bool { + LOCAL_ENVIRONMENT_IDS.contains(&environment_id) +} + +/// Metadata key the railway (and any deterministic-naming) environment plugin +/// reads to name the node stably per workflow run. Injected by the broker into +/// the spec before `prepare` so every phase of a run maps to the same node name. +const ANIMUS_RUN_ID_METADATA_KEY: &str = "animus_run_id"; + +// --------------------------------------------------------------------------- +// Wire types (private daemon<->runner IPC — mirror broker-wire-contract.md). +// --------------------------------------------------------------------------- + +#[derive(Debug, Deserialize)] +#[serde(tag = "op", rename_all = "lowercase")] +enum BrokerRequest { + Acquire { + token: String, + run_id: String, + environment_id: String, + spec: EnvironmentSpec, + }, + Exec { + token: String, + run_id: String, + handle_id: String, + command: HarnessCommand, + #[serde(default)] + stdin: Option, + #[serde(default)] + timeout_secs: Option, + }, +} + +// --------------------------------------------------------------------------- +// Durable lease record (survives a daemon restart so the reaper can cold-tear +// down a node the prior daemon instance leaked). +// --------------------------------------------------------------------------- + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +enum LeaseState { + Preparing, + Ready, + TearingDown, + TornDown, + Failed, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +struct LeaseRecord { + run_id: String, + daemon_instance_id: String, + environment_id: String, + project_root: String, + state: LeaseState, + #[serde(default, skip_serializing_if = "Option::is_none")] + handle: Option, + created_at: String, + updated_at: String, +} + +// --------------------------------------------------------------------------- +// In-memory lease. +// --------------------------------------------------------------------------- + +/// A ready, prepared node for a run: the pinned daemon-resident client plus the +/// environment handle every phase of the run execs against. +struct ReadyLease { + environment_id: String, + project_root: String, + client: Arc, + handle: EnvironmentHandle, +} + +/// Context the daemon registers at spawn time (it, not the runner, is the +/// authority on `project_root`). `acquire` resolves the [`EnvironmentClient`] +/// against this. +#[derive(Clone)] +struct PendingContext { + project_root: String, + environment_id: String, +} + +struct Inner { + daemon_instance_id: String, + token: String, + socket_path: String, + /// Directory holding `.json` durable lease records + the socket. + records_dir: PathBuf, + /// run_id -> ready lease. Only READY leases live here; a failed/torn-down + /// run is absent (a later acquire re-prepares). + leases: AsyncMutex>, + /// Per-run single-flight mutex so concurrent first-acquires prepare once. + key_locks: StdMutex>>>, + /// run_id -> spawn-time context (project_root + expected environment id). + pending: StdMutex>, + /// The socket acceptor task; aborted + socket unlinked on drop. + acceptor: StdMutex>>, +} + +impl Drop for Inner { + fn drop(&mut self) { + if let Some(handle) = self.acceptor.lock().unwrap_or_else(|p| p.into_inner()).take() { + handle.abort(); + } + // Best-effort: unlink the socket file so a restart can rebind cleanly. + if looks_like_filesystem(&self.socket_path) { + let _ = std::fs::remove_file(&self.socket_path); + } + } +} + +/// Daemon-side broker handle. Cheap to clone (`Arc`); every clone shares the one +/// lease map + socket server. +#[derive(Clone)] +pub struct EnvironmentBroker { + inner: Arc, +} + +impl EnvironmentBroker { + /// Bind the broker's local socket under `project_root`'s scoped state root, + /// start the accept loop on the current Tokio runtime, and reap any lease + /// records owned by a PRIOR daemon instance (their relay is dead). + /// + /// Must be called from within the daemon's multi-threaded runtime: the + /// resident [`EnvironmentClient`] the broker drives spawns its warm plugin + /// host onto THIS runtime and is pinned there for the run's lifetime. + pub async fn start(project_root: &str) -> std::io::Result { + let records_dir = broker_records_dir(project_root); + std::fs::create_dir_all(&records_dir)?; + let socket_path = broker_socket_path(&records_dir); + + let listener = bind_listener(&socket_path)?; + + let inner = Arc::new(Inner { + daemon_instance_id: uuid::Uuid::new_v4().simple().to_string(), + token: uuid::Uuid::new_v4().simple().to_string(), + socket_path, + records_dir, + leases: AsyncMutex::new(HashMap::new()), + key_locks: StdMutex::new(HashMap::new()), + pending: StdMutex::new(HashMap::new()), + acceptor: StdMutex::new(None), + }); + + let broker = Self { inner: inner.clone() }; + let accept_broker = broker.clone(); + let handle = tokio::spawn(async move { accept_loop(listener, accept_broker).await }); + *inner.acceptor.lock().unwrap_or_else(|p| p.into_inner()) = Some(handle); + + broker.reap_prior_daemon_records().await; + Ok(broker) + } + + /// Local-socket path handed to the runner via + /// [`ANIMUS_ENVIRONMENT_BROKER_SOCKET_ENV`]. + pub fn socket_path(&self) -> &str { + &self.inner.socket_path + } + + /// Per-daemon bearer token handed to the runner via + /// [`ANIMUS_ENVIRONMENT_BROKER_TOKEN_ENV`] and echoed on every frame. + pub fn token(&self) -> &str { + &self.inner.token + } + + /// Record the spawn-time context for `run_id` so the runner's later + /// `acquire` resolves the [`EnvironmentClient`] against the daemon-authored + /// `project_root` (the wire frame never carries it). Idempotent per run. + pub fn register_run(&self, run_id: &str, project_root: &str, environment_id: &str) { + self.inner.pending.lock().unwrap_or_else(|p| p.into_inner()).insert( + run_id.to_string(), + PendingContext { project_root: project_root.to_string(), environment_id: environment_id.to_string() }, + ); + } + + /// Idempotently dispose the node for `run_id`: `Ready -> TearingDown -> + /// TornDown`, delete the durable record, forget the run's pending context. + /// A no-op when no lease exists (already torn down, or never prepared). + pub async fn teardown(&self, run_id: &str) { + let lease = self.inner.leases.lock().await.remove(run_id); + if let Some(lease) = lease { + self.write_record(run_id, &lease.environment_id, &lease.project_root, LeaseState::TearingDown, None); + let client = lease.client.clone(); + let handle = lease.handle.clone(); + // Sync RPC against the pinned host. `teardown` bridges async→sync + // internally (its own `block_in_place`), so it is called directly — + // wrapping it in another `block_in_place` would nest and is not + // needed. + if let Err(error) = client.teardown(&handle) { + tracing::warn!( + target: "animus.runtime.environment_broker", + run_id, + %error, + "environment teardown failed; deleting lease record anyway (node may be reclaimed by the plugin GC sweep)" + ); + } + } + self.delete_record(run_id); + self.forget_run(run_id); + } + + fn forget_run(&self, run_id: &str) { + self.inner.pending.lock().unwrap_or_else(|p| p.into_inner()).remove(run_id); + self.inner.key_locks.lock().unwrap_or_else(|p| p.into_inner()).remove(run_id); + } + + /// SINGLE-FLIGHT acquire: return the shared workspace_root + handle_id for + /// `run_id`, preparing the node ONCE on the first call. Rejects a different + /// `environment_id` for an already-bound run. + async fn acquire(&self, run_id: &str, environment_id: &str, spec: EnvironmentSpec) -> Result<(String, String)> { + let pending = + self.inner.pending.lock().unwrap_or_else(|p| p.into_inner()).get(run_id).cloned().ok_or_else(|| { + anyhow!("no pending environment context for run {run_id} (daemon did not register this run)") + })?; + if pending.environment_id != environment_id { + bail!("run {run_id} is bound to environment '{}', not '{environment_id}'", pending.environment_id); + } + + let key_lock = self.key_lock(run_id); + let _guard = key_lock.lock().await; + + // Fast path: an already-prepared lease is reused by every later phase. + if let Some(lease) = self.inner.leases.lock().await.get(run_id) { + if lease.environment_id != environment_id { + bail!("run {run_id} is already bound to environment '{}'", lease.environment_id); + } + return Ok((lease.handle.workspace_root.clone(), lease.handle.id.clone())); + } + + // Slow path: prepare the node ONCE. Record BEFORE prepare so a crash + // mid-prepare still leaves a durable marker for the startup reaper. + self.write_record(run_id, environment_id, &pending.project_root, LeaseState::Preparing, None); + + let mut spec = spec; + set_run_id_metadata(&mut spec, run_id); + + let project_root = pending.project_root.clone(); + let environment_id_owned = environment_id.to_string(); + // `resolve` is pure discovery; `prepare` bridges async→sync internally + // (its own `block_in_place`), so both are called directly — the daemon + // worker is handed off for the duration of the prepare RPC by the client. + let prepared = (|| { + let client = EnvironmentClient::resolve(Path::new(&project_root), &environment_id_owned) + .with_context(|| format!("resolving environment '{environment_id_owned}' for run {run_id}"))?; + let client = Arc::new(client); + let handle = client.prepare(spec).with_context(|| format!("preparing environment for run {run_id}"))?; + Ok::<_, anyhow::Error>((client, handle)) + })(); + + match prepared { + Ok((client, handle)) => { + self.write_record(run_id, environment_id, &pending.project_root, LeaseState::Ready, Some(&handle)); + let response = (handle.workspace_root.clone(), handle.id.clone()); + self.inner.leases.lock().await.insert( + run_id.to_string(), + ReadyLease { + environment_id: environment_id.to_string(), + project_root: pending.project_root.clone(), + client, + handle, + }, + ); + Ok(response) + } + Err(error) => { + // Mark failed + reap any partial node the plugin left behind, so + // a failed prepare never leaks a durable record. + self.write_record(run_id, environment_id, &pending.project_root, LeaseState::Failed, None); + self.delete_record(run_id); + Err(error) + } + } + } + + /// Look up the run's lease, assert `handle_id` matches it (a runner can + /// NEVER exec into another run's node), and return the pinned client + + /// handle to drive `exec_stream` against. The lease lock is NOT held across + /// the exec, so a long command does not block other broker ops. + async fn exec_target(&self, run_id: &str, handle_id: &str) -> Result<(Arc, EnvironmentHandle)> { + let leases = self.inner.leases.lock().await; + let lease = + leases.get(run_id).ok_or_else(|| anyhow!("no prepared environment for run {run_id} (acquire first)"))?; + if lease.handle.id != handle_id { + bail!("handle '{handle_id}' does not match the lease for run {run_id}"); + } + Ok((lease.client.clone(), lease.handle.clone())) + } + + fn key_lock(&self, run_id: &str) -> Arc> { + self.inner + .key_locks + .lock() + .unwrap_or_else(|p| p.into_inner()) + .entry(run_id.to_string()) + .or_insert_with(|| Arc::new(AsyncMutex::new(()))) + .clone() + } + + // -- durable records ---------------------------------------------------- + + fn record_path(&self, run_id: &str) -> PathBuf { + self.inner.records_dir.join(format!("{}.json", protocol::sanitize_identifier(run_id, "run"))) + } + + fn write_record( + &self, + run_id: &str, + environment_id: &str, + project_root: &str, + state: LeaseState, + handle: Option<&EnvironmentHandle>, + ) { + let now = chrono::Utc::now().to_rfc3339(); + let record = LeaseRecord { + run_id: run_id.to_string(), + daemon_instance_id: self.inner.daemon_instance_id.clone(), + environment_id: environment_id.to_string(), + project_root: project_root.to_string(), + state, + handle: handle.cloned(), + created_at: now.clone(), + updated_at: now, + }; + let path = self.record_path(run_id); + if let Err(error) = write_record_atomic(&path, &record) { + tracing::warn!( + target: "animus.runtime.environment_broker", + run_id, + %error, + "failed to persist environment lease record (best-effort; startup reap may miss this node)" + ); + } + } + + fn delete_record(&self, run_id: &str) { + let _ = std::fs::remove_file(self.record_path(run_id)); + } + + /// Cold-teardown every lease record owned by a PRIOR daemon instance: its + /// relay died with that daemon, so a fresh `resolve` + `teardown(handle)` + /// (dispose-by-id on a fresh plugin process) reclaims the leaked node. Then + /// delete the record. Records owned by THIS instance are left untouched. + async fn reap_prior_daemon_records(&self) { + let entries = match std::fs::read_dir(&self.inner.records_dir) { + Ok(entries) => entries, + Err(_) => return, + }; + for entry in entries.flatten() { + let path = entry.path(); + if path.extension().and_then(|e| e.to_str()) != Some("json") { + continue; + } + let record: LeaseRecord = + match std::fs::read_to_string(&path).ok().and_then(|raw| serde_json::from_str(&raw).ok()) { + Some(record) => record, + None => { + let _ = std::fs::remove_file(&path); + continue; + } + }; + if record.daemon_instance_id == self.inner.daemon_instance_id { + continue; + } + if let Some(handle) = record.handle.clone() { + let environment_id = record.environment_id.clone(); + let project_root = record.project_root.clone(); + let run_id = record.run_id.clone(); + // `resolve` + `teardown` bridge async→sync internally; call + // directly (no outer `block_in_place`). + let outcome = (|| { + let client = EnvironmentClient::resolve(Path::new(&project_root), &environment_id)?; + client.teardown(&handle) + })(); + if let Err(error) = outcome { + tracing::warn!( + target: "animus.runtime.environment_broker", + run_id = %run_id, + %error, + "startup reap: cold teardown of an orphaned node failed; deleting the record anyway" + ); + } else { + tracing::info!( + target: "animus.runtime.environment_broker", + run_id = %run_id, + "startup reap: cold tore down a node leaked by a prior daemon instance" + ); + } + } + let _ = std::fs::remove_file(&path); + } + } +} + +// --------------------------------------------------------------------------- +// Socket server. +// --------------------------------------------------------------------------- + +fn bind_listener(socket_path: &str) -> std::io::Result { + use animus_runtime_shared::reattach::local_socket_name_for; + use interprocess::local_socket::ListenerOptions; + + if looks_like_filesystem(socket_path) && Path::new(socket_path).exists() { + // Clear a corpse socket from a prior run so bind succeeds. + let _ = std::fs::remove_file(socket_path); + } + let name = local_socket_name_for(socket_path)?; + let listener = ListenerOptions::new().name(name).create_tokio()?; + + #[cfg(unix)] + if looks_like_filesystem(socket_path) { + use std::os::unix::fs::PermissionsExt; + if let Ok(meta) = std::fs::metadata(socket_path) { + let mut perms = meta.permissions(); + perms.set_mode(0o600); + let _ = std::fs::set_permissions(socket_path, perms); + } + } + Ok(listener) +} + +async fn accept_loop(listener: interprocess::local_socket::tokio::Listener, broker: EnvironmentBroker) { + use interprocess::local_socket::traits::tokio::Listener as _; + loop { + let conn = match listener.accept().await { + Ok(conn) => conn, + Err(error) => { + tracing::debug!( + target: "animus.runtime.environment_broker", + %error, + "environment broker accept loop exiting on accept error" + ); + return; + } + }; + let conn_broker = broker.clone(); + tokio::spawn(async move { + if let Err(error) = handle_connection(conn, conn_broker).await { + tracing::debug!( + target: "animus.runtime.environment_broker", + %error, + "environment broker connection handler exited with error" + ); + } + }); + } +} + +async fn handle_connection( + conn: interprocess::local_socket::tokio::Stream, + broker: EnvironmentBroker, +) -> std::io::Result<()> { + use tokio::io::{AsyncBufReadExt, BufReader}; + + let mut reader = BufReader::new(&conn); + let mut line = String::new(); + if reader.read_line(&mut line).await? == 0 { + return Ok(()); + } + + let request: BrokerRequest = match serde_json::from_str(line.trim()) { + Ok(request) => request, + Err(error) => { + write_json_line(&conn, &json!({ "ok": false, "error": format!("malformed broker request: {error}") })) + .await?; + return Ok(()); + } + }; + + match request { + BrokerRequest::Acquire { token, run_id, environment_id, spec } => { + if token != broker.token() { + write_json_line(&conn, &json!({ "ok": false, "error": "unauthorized" })).await?; + return Ok(()); + } + match broker.acquire(&run_id, &environment_id, spec).await { + Ok((workspace_root, handle_id)) => { + write_json_line( + &conn, + &json!({ "ok": true, "workspace_root": workspace_root, "handle_id": handle_id }), + ) + .await?; + } + Err(error) => { + write_json_line(&conn, &json!({ "ok": false, "error": error.to_string() })).await?; + } + } + } + BrokerRequest::Exec { token, run_id, handle_id, command, stdin, timeout_secs } => { + if token != broker.token() { + write_json_line(&conn, &json!({ "error": "unauthorized" })).await?; + return Ok(()); + } + handle_exec(&conn, broker, run_id, handle_id, command, stdin, timeout_secs).await?; + } + } + Ok(()) +} + +#[allow(clippy::too_many_arguments)] +async fn handle_exec( + conn: &interprocess::local_socket::tokio::Stream, + broker: EnvironmentBroker, + run_id: String, + handle_id: String, + command: HarnessCommand, + stdin: Option, + timeout_secs: Option, +) -> std::io::Result<()> { + let (client, handle) = match broker.exec_target(&run_id, &handle_id).await { + Ok(target) => target, + Err(error) => { + return write_json_line(conn, &json!({ "error": error.to_string() })).await; + } + }; + + // Forward incremental output through a channel so the streamed exec (a + // blocking RPC that bridges async→sync internally) and the socket writer run + // concurrently: `exec_stream`'s own `block_in_place` hands off the runtime + // worker for the command's duration while this task's writer loop drains rx. + let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::(); + let timeout = timeout_secs.map(Duration::from_secs); + let exec_task = tokio::spawn(async move { + client.exec_stream( + &handle, + command, + std::collections::BTreeMap::new(), + stdin, + timeout, + |stream: ExecStream, text: &str| { + let _ = tx.send(json!({ "out": exec_stream_str(stream), "text": text })); + }, + ) + }); + + while let Some(frame) = rx.recv().await { + write_json_line(conn, &frame).await?; + } + + match exec_task.await { + Ok(Ok(response)) => { + let done = serde_json::to_value(&response).unwrap_or(serde_json::Value::Null); + write_json_line(conn, &json!({ "done": done })).await?; + } + Ok(Err(error)) => { + write_json_line(conn, &json!({ "error": error.to_string() })).await?; + } + Err(join_error) => { + write_json_line(conn, &json!({ "error": format!("exec task panicked: {join_error}") })).await?; + } + } + Ok(()) +} + +async fn write_json_line( + mut conn: &interprocess::local_socket::tokio::Stream, + value: &serde_json::Value, +) -> std::io::Result<()> { + use tokio::io::AsyncWriteExt; + let mut bytes = serde_json::to_vec(value).unwrap_or_default(); + bytes.push(b'\n'); + conn.write_all(&bytes).await?; + conn.flush().await +} + +fn exec_stream_str(stream: ExecStream) -> &'static str { + match stream { + ExecStream::Stdout => "stdout", + ExecStream::Stderr => "stderr", + } +} + +// --------------------------------------------------------------------------- +// Helpers. +// --------------------------------------------------------------------------- + +fn looks_like_filesystem(value: &str) -> bool { + value.contains(std::path::MAIN_SEPARATOR) || value.contains('/') +} + +/// Inject `metadata.animus_run_id = run_id` so a deterministic-naming plugin +/// (e.g. railway) names the node stably per run. Preserves any existing +/// metadata object; replaces a non-object metadata value. +fn set_run_id_metadata(spec: &mut EnvironmentSpec, run_id: &str) { + match spec.metadata.as_object_mut() { + Some(map) => { + map.insert(ANIMUS_RUN_ID_METADATA_KEY.to_string(), json!(run_id)); + } + None => { + spec.metadata = json!({ ANIMUS_RUN_ID_METADATA_KEY: run_id }); + } + } +} + +/// Directory holding the broker's durable records and (length permitting) its +/// socket. Scoped state root when resolvable; a `$TMPDIR` fallback otherwise +/// (tests / non-git contexts) so the broker still works without cross-restart +/// reaping. +fn broker_records_dir(project_root: &str) -> PathBuf { + protocol::scoped_state_root(Path::new(project_root)).map(|root| root.join("workflow-environments")).unwrap_or_else( + || std::env::temp_dir().join("animus-workflow-environments").join(std::process::id().to_string()), + ) +} + +/// Conservative Unix-domain-socket path cap (SUN_LEN is ~104 on macOS, 108 on +/// Linux); leave headroom for platform quirks. +const MAX_UNIX_SOCKET_PATH_BYTES: usize = 100; + +/// Pick a socket path under `records_dir`, falling back to `$TMPDIR` when the +/// canonical path would exceed the SUN_LEN budget. +fn broker_socket_path(records_dir: &Path) -> String { + let canonical = records_dir.join("broker.sock"); + if canonical.as_os_str().len() <= MAX_UNIX_SOCKET_PATH_BYTES { + return canonical.to_string_lossy().into_owned(); + } + std::env::temp_dir() + .join("animus-env-broker") + .join(std::process::id().to_string()) + .join("broker.sock") + .to_string_lossy() + .into_owned() +} + +fn write_record_atomic(path: &Path, record: &LeaseRecord) -> std::io::Result<()> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + let tmp = path.with_extension("json.tmp"); + let bytes = serde_json::to_vec_pretty(record).map_err(std::io::Error::other)?; + std::fs::write(&tmp, bytes)?; + std::fs::rename(&tmp, path) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn local_environment_ids_are_recognized() { + assert!(is_local_environment("worktree")); + assert!(is_local_environment("local")); + assert!(!is_local_environment("railway")); + assert!(!is_local_environment("animus-environment-railway")); + } + + #[test] + fn set_run_id_metadata_preserves_existing_object() { + let mut spec = EnvironmentSpec { + kind: "railway".to_string(), + repos: Vec::new(), + image: None, + resources: None, + env: std::collections::BTreeMap::new(), + metadata: json!({ "region": "us-west" }), + }; + set_run_id_metadata(&mut spec, "wf-abc"); + assert_eq!(spec.metadata["animus_run_id"], json!("wf-abc")); + assert_eq!(spec.metadata["region"], json!("us-west")); + } + + #[test] + fn set_run_id_metadata_replaces_null_metadata() { + let mut spec = EnvironmentSpec { + kind: "railway".to_string(), + repos: Vec::new(), + image: None, + resources: None, + env: std::collections::BTreeMap::new(), + metadata: serde_json::Value::Null, + }; + set_run_id_metadata(&mut spec, "wf-xyz"); + assert_eq!(spec.metadata["animus_run_id"], json!("wf-xyz")); + } + + #[test] + fn acquire_request_parses_from_wire() { + let line = + r#"{"op":"acquire","token":"t","run_id":"wf-1","environment_id":"railway","spec":{"kind":"railway"}}"#; + let request: BrokerRequest = serde_json::from_str(line).expect("parse acquire"); + match request { + BrokerRequest::Acquire { token, run_id, environment_id, spec } => { + assert_eq!(token, "t"); + assert_eq!(run_id, "wf-1"); + assert_eq!(environment_id, "railway"); + assert_eq!(spec.kind, "railway"); + } + BrokerRequest::Exec { .. } => panic!("expected acquire"), + } + } + + #[test] + fn exec_request_parses_from_wire() { + let line = r#"{"op":"exec","token":"t","run_id":"wf-1","handle_id":"h-1","command":{"program":"echo","args":["hi"]},"stdin":null,"timeout_secs":30}"#; + let request: BrokerRequest = serde_json::from_str(line).expect("parse exec"); + match request { + BrokerRequest::Exec { run_id, handle_id, command, timeout_secs, .. } => { + assert_eq!(run_id, "wf-1"); + assert_eq!(handle_id, "h-1"); + assert_eq!(command.program, "echo"); + assert_eq!(timeout_secs, Some(30)); + } + BrokerRequest::Acquire { .. } => panic!("expected exec"), + } + } + + #[test] + fn lease_record_round_trips() { + let record = LeaseRecord { + run_id: "wf-1".to_string(), + daemon_instance_id: "daemon-a".to_string(), + environment_id: "railway".to_string(), + project_root: "/tmp/project".to_string(), + state: LeaseState::Ready, + handle: Some(EnvironmentHandle { + id: "node-1".to_string(), + workspace_root: "/workspace".to_string(), + metadata: serde_json::Value::Null, + }), + created_at: "2026-01-01T00:00:00Z".to_string(), + updated_at: "2026-01-01T00:00:01Z".to_string(), + }; + let json = serde_json::to_string(&record).expect("serialize"); + let decoded: LeaseRecord = serde_json::from_str(&json).expect("deserialize"); + assert_eq!(decoded.run_id, "wf-1"); + assert_eq!(decoded.state, LeaseState::Ready); + assert_eq!(decoded.handle.unwrap().id, "node-1"); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn teardown_is_idempotent_without_a_lease() { + let temp = tempfile::tempdir().expect("tempdir"); + let broker = EnvironmentBroker::start(temp.path().to_string_lossy().as_ref()).await.expect("start broker"); + // No lease for this run: teardown must be a clean no-op. + broker.teardown("wf-missing").await; + broker.teardown("wf-missing").await; + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn acquire_rejects_unregistered_run() { + let temp = tempfile::tempdir().expect("tempdir"); + let broker = EnvironmentBroker::start(temp.path().to_string_lossy().as_ref()).await.expect("start broker"); + let spec = EnvironmentSpec { + kind: "railway".to_string(), + repos: Vec::new(), + image: None, + resources: None, + env: std::collections::BTreeMap::new(), + metadata: serde_json::Value::Null, + }; + let err = broker.acquire("wf-unregistered", "railway", spec).await.expect_err("must reject"); + assert!(err.to_string().contains("no pending environment context"), "got: {err}"); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn acquire_rejects_environment_mismatch_with_registration() { + let temp = tempfile::tempdir().expect("tempdir"); + let broker = EnvironmentBroker::start(temp.path().to_string_lossy().as_ref()).await.expect("start broker"); + broker.register_run("wf-1", temp.path().to_string_lossy().as_ref(), "railway"); + let spec = EnvironmentSpec { + kind: "container".to_string(), + repos: Vec::new(), + image: None, + resources: None, + env: std::collections::BTreeMap::new(), + metadata: serde_json::Value::Null, + }; + let err = broker.acquire("wf-1", "container", spec).await.expect_err("must reject mismatch"); + assert!(err.to_string().contains("is bound to environment 'railway'"), "got: {err}"); + } +} diff --git a/crates/orchestrator-daemon-runtime/src/dispatch/mod.rs b/crates/orchestrator-daemon-runtime/src/dispatch/mod.rs index 5dcee86b..e4017074 100644 --- a/crates/orchestrator-daemon-runtime/src/dispatch/mod.rs +++ b/crates/orchestrator-daemon-runtime/src/dispatch/mod.rs @@ -22,6 +22,7 @@ mod dispatch_selection_source; mod dispatch_support; mod dispatch_workflow_start; mod dispatch_workflow_start_summary; +pub mod environment_broker; pub mod event_pipe; mod process_manager; mod ready_dispatch_plan; @@ -44,6 +45,10 @@ pub use dispatch_support::{ }; pub use dispatch_workflow_start::DispatchWorkflowStart; pub use dispatch_workflow_start_summary::DispatchWorkflowStartSummary; +pub use environment_broker::{ + is_local_environment, EnvironmentBroker, ANIMUS_ENVIRONMENT_BROKER_ENVIRONMENT_ID_ENV, + ANIMUS_ENVIRONMENT_BROKER_RUN_ID_ENV, ANIMUS_ENVIRONMENT_BROKER_SOCKET_ENV, ANIMUS_ENVIRONMENT_BROKER_TOKEN_ENV, +}; #[cfg(unix)] #[allow(unused_imports)] pub use event_pipe::SubprocessEventPipe; diff --git a/crates/orchestrator-daemon-runtime/src/dispatch/process_manager.rs b/crates/orchestrator-daemon-runtime/src/dispatch/process_manager.rs index 13c7c286..61753fa8 100644 --- a/crates/orchestrator-daemon-runtime/src/dispatch/process_manager.rs +++ b/crates/orchestrator-daemon-runtime/src/dispatch/process_manager.rs @@ -14,6 +14,10 @@ use tokio::task::JoinHandle; #[cfg(unix)] use crate::control::WorkflowEventBroadcaster; +use crate::dispatch::environment_broker::{ + is_local_environment, EnvironmentBroker, ANIMUS_ENVIRONMENT_BROKER_ENVIRONMENT_ID_ENV, + ANIMUS_ENVIRONMENT_BROKER_RUN_ID_ENV, ANIMUS_ENVIRONMENT_BROKER_SOCKET_ENV, ANIMUS_ENVIRONMENT_BROKER_TOKEN_ENV, +}; #[cfg(unix)] use crate::dispatch::event_pipe::SubprocessEventPipe; use crate::{build_runner_command_with_resume, CompletedProcess, RunnerEvent}; @@ -66,6 +70,10 @@ const DAEMON_MANAGED_RUNNER_ENV_KEYS: &[&str] = &[ "ANIMUS_WORKFLOW_REATTACH_SOCKET", "ANIMUS_WORKFLOW_EVENT_PIPE", animus_runtime_shared::phase_skills::ANIMUS_PHASE_SKILLS_ENV, + ANIMUS_ENVIRONMENT_BROKER_SOCKET_ENV, + ANIMUS_ENVIRONMENT_BROKER_TOKEN_ENV, + ANIMUS_ENVIRONMENT_BROKER_RUN_ID_ENV, + ANIMUS_ENVIRONMENT_BROKER_ENVIRONMENT_ID_ENV, ]; /// Upper bound on the serialized phase-skills payload the daemon will put on @@ -167,6 +175,11 @@ struct WorkflowProcess { /// TARGETED run rather than leaving it Running — which would otherwise let /// the resume sweep re-dispatch it every tick in an endless loop. resume_workflow_id: Option, + /// REQ-048 cross-phase environment broker: the broker `run_id` this spawn + /// was bound to (the workflow run id), set ONLY when the dispatch routes to + /// a non-local environment and a broker is wired. The daemon tears the run's + /// shared node down by this id once the workflow reaches a terminal state. + environment_run_id: Option, } pub struct ProcessManager { @@ -188,6 +201,15 @@ pub struct ProcessManager { /// spawn requests beyond this point are rejected; the dispatcher then /// leaves the entry in the ready queue for the next tick. workflow_concurrency_max: Option, + /// REQ-048: config-level environment routing (kind/harness rules + default). + /// Drives the daemon-side decision of whether a dispatch routes to a + /// non-local environment (and thus through the [`EnvironmentBroker`]). + pub environment_routing: Option, + /// REQ-048: the cross-phase ephemeral-environment broker. When wired AND the + /// dispatch routes to a non-local environment, the daemon sets the four + /// `ANIMUS_ENVIRONMENT_BROKER_*` env vars on the runner so it acquires the + /// run's shared node from the daemon instead of preparing its own. + environment_broker: Option, } impl Default for ProcessManager { @@ -222,6 +244,8 @@ impl ProcessManager { #[cfg(unix)] pipe_root: None, workflow_concurrency_max, + environment_routing: None, + environment_broker: None, } } @@ -259,6 +283,16 @@ impl ProcessManager { self } + /// REQ-048: wire the cross-phase ephemeral-environment broker. Once set, + /// [`Self::spawn_workflow_runner`] routes dispatches bound to a non-local + /// environment through the broker (four `ANIMUS_ENVIRONMENT_BROKER_*` env + /// vars on the runner), and [`Self::check_running`] tears the run's shared + /// node down when the workflow reaches a terminal state. + pub fn with_environment_broker(mut self, broker: EnvironmentBroker) -> Self { + self.environment_broker = Some(broker); + self + } + pub fn spawn_workflow_runner(&mut self, dispatch: &SubjectDispatch, project_root: &str) -> Result<()> { self.spawn_workflow_runner_inner(dispatch, project_root, None) } @@ -399,6 +433,14 @@ impl ProcessManager { command.env(animus_runtime_shared::reattach::ANIMUS_WORKFLOW_REATTACH_SOCKET_ENV, path.as_os_str()); } + // REQ-048 cross-phase environment broker: when this dispatch routes to a + // non-local environment and a broker is wired, register the run and set + // the four `ANIMUS_ENVIRONMENT_BROKER_*` env vars so the runner acquires + // the run's SHARED node from the daemon instead of preparing its own. + // Returns the broker run_id so the completion path can tear it down. + let environment_run_id = + self.configure_environment_broker(dispatch, project_root, resume_workflow_id, &mut command); + // Bind the subprocess workflow_events back-channel before fork so the // env var we set on the child points to a listener that's already // accepting. Best-effort: if bind fails (eg no Unix DS support in a @@ -482,11 +524,57 @@ impl ProcessManager { agent_session_id, project_root: Some(project_root_path), resume_workflow_id: resume_workflow_id.map(String::from), + environment_run_id, }); Ok(()) } + /// REQ-048: decide whether this dispatch routes to a non-local environment + /// and, if so, register the run with the broker + set the four + /// `ANIMUS_ENVIRONMENT_BROKER_*` env vars on the runner command. Returns the + /// broker `run_id` (the workflow run id) when brokered, else `None` (the + /// runner then uses its legacy owned-environment path). + /// + /// The run_id must be STABLE across every phase of a workflow run so all + /// phases share one node: a phase-boundary RESUME dispatch reuses the + /// persisted `resume_workflow_id`; the FIRST (fresh) phase mints one, which + /// the runner adopts as its workflow id so later resume phases key on the + /// SAME id. + fn configure_environment_broker( + &self, + dispatch: &SubjectDispatch, + project_root: &str, + resume_workflow_id: Option<&str>, + command: &mut Command, + ) -> Option { + let broker = self.environment_broker.as_ref()?; + // Config-level routing only (kind rules + default): the daemon dispatches + // per phase and cannot know the phase's harness/`environment:` override + // here, and the node is per-RUN anyway, so a single run-level environment + // is the correct granularity for the broker decision. + let environment_id = orchestrator_config::workflow_config::resolve_environment( + dispatch.subject_kind(), + None, + None, + None, + self.environment_routing.as_ref(), + )?; + if is_local_environment(&environment_id) { + return None; + } + let run_id = match resume_workflow_id { + Some(id) => id.to_string(), + None => format!("wf-{}", uuid::Uuid::new_v4().simple()), + }; + broker.register_run(&run_id, project_root, &environment_id); + command.env(ANIMUS_ENVIRONMENT_BROKER_SOCKET_ENV, broker.socket_path()); + command.env(ANIMUS_ENVIRONMENT_BROKER_TOKEN_ENV, broker.token()); + command.env(ANIMUS_ENVIRONMENT_BROKER_RUN_ID_ENV, &run_id); + command.env(ANIMUS_ENVIRONMENT_BROKER_ENVIRONMENT_ID_ENV, &environment_id); + Some(run_id) + } + /// Bind a fresh per-spawn event pipe and attach the /// `ANIMUS_WORKFLOW_EVENT_PIPE` env var to `command` so the runner can /// connect. Returns `None` when the back-channel isn't configured on @@ -551,6 +639,9 @@ impl ProcessManager { async fn check_running_with_timeout(&mut self, timeout_secs: Option) -> Vec { let mut completed = Vec::new(); let mut active = Vec::with_capacity(self.processes.len()); + // Cloned out of `self` up front so the terminal-teardown calls below do + // not conflict with the `&mut self.processes` drain borrow. + let environment_broker = self.environment_broker.clone(); for mut process in self.processes.drain(..) { if let Some(timeout) = timeout_secs { @@ -569,6 +660,14 @@ impl ProcessManager { #[cfg(unix)] drain_event_pipe(&mut process.event_pipe).await; cleanup_agent_record(&process); + // Timeout kill is a terminal Failed outcome: dispose the + // run's shared node (if brokered) before reaping the record. + teardown_environment_if_terminal( + &environment_broker, + process.environment_run_id.take(), + Some(WorkflowStatus::Failed), + ) + .await; completed.push(CompletedProcess { subject_id: process.subject_key, subject_kind: Some(process.subject_kind), @@ -649,6 +748,16 @@ impl ProcessManager { workflow_status = Some(WorkflowStatus::Failed); } + // Terminal workflow state => tear the run's shared node down + // (one node per run). Non-terminal (Running/Paused between + // phases) => KEEP the node for the next phase's runner. + teardown_environment_if_terminal( + &environment_broker, + process.environment_run_id.take(), + workflow_status, + ) + .await; + completed.push(CompletedProcess { subject_id: process.subject_key, subject_kind: Some(process.subject_kind), @@ -698,6 +807,33 @@ impl ProcessManager { } } +/// REQ-048: tear the run's shared ephemeral node down IFF the completed phase +/// landed the workflow in a terminal state (Completed/Failed/Cancelled/ +/// Escalated). A non-terminal exit (Running/Paused between phases) keeps the +/// node for the next phase's runner. A no-op when the dispatch was not brokered +/// (`run_id` is `None`) or no broker is wired. `teardown` is idempotent. +async fn teardown_environment_if_terminal( + broker: &Option, + run_id: Option, + status: Option, +) { + let (Some(broker), Some(run_id)) = (broker.as_ref(), run_id) else { + return; + }; + if is_terminal_workflow_status(status) { + broker.teardown(&run_id).await; + } +} + +fn is_terminal_workflow_status(status: Option) -> bool { + matches!( + status, + Some( + WorkflowStatus::Completed | WorkflowStatus::Failed | WorkflowStatus::Cancelled | WorkflowStatus::Escalated + ) + ) +} + fn cleanup_agent_record(process: &WorkflowProcess) { if let (Some(project_root), Some(id)) = (process.project_root.as_ref(), process.agent_session_id.as_ref()) { super::agent_record::delete_record(project_root, id); diff --git a/crates/orchestrator-daemon-runtime/src/lib.rs b/crates/orchestrator-daemon-runtime/src/lib.rs index 4881f89f..0bc16d04 100644 --- a/crates/orchestrator-daemon-runtime/src/lib.rs +++ b/crates/orchestrator-daemon-runtime/src/lib.rs @@ -26,10 +26,12 @@ pub use daemon::{ pub use dispatch::{ active_workflow_subject_ids, active_workflow_task_ids, build_completion_reconciliation_plan, build_runner_command, build_runner_command_from_dispatch, build_runner_command_with_resume, dispatch_capacity_for_options, - execute_dispatch_plan_via_runner, is_terminally_completed_workflow, ready_dispatch_limit, schedule_headroom, - workflow_current_phase_id, CompletedProcess, CompletedProcessReconciliation, CompletionReconciliationPlan, - DispatchNotice, DispatchNoticeSink, DispatchSelectionSource, DispatchWorkflowStart, DispatchWorkflowStartSummary, - PlannedDispatchStart, ProcessManager, TickBudget, WorkflowConcurrencyCapReached, WorkflowFailureEvent, + execute_dispatch_plan_via_runner, is_local_environment, is_terminally_completed_workflow, ready_dispatch_limit, + schedule_headroom, workflow_current_phase_id, CompletedProcess, CompletedProcessReconciliation, + CompletionReconciliationPlan, DispatchNotice, DispatchNoticeSink, DispatchSelectionSource, DispatchWorkflowStart, + DispatchWorkflowStartSummary, EnvironmentBroker, PlannedDispatchStart, ProcessManager, TickBudget, + WorkflowConcurrencyCapReached, WorkflowFailureEvent, ANIMUS_ENVIRONMENT_BROKER_ENVIRONMENT_ID_ENV, + ANIMUS_ENVIRONMENT_BROKER_RUN_ID_ENV, ANIMUS_ENVIRONMENT_BROKER_SOCKET_ENV, ANIMUS_ENVIRONMENT_BROKER_TOKEN_ENV, }; /// v0.5.1 P2 #6.2 round-3: daemon-side reattach client surface, exposed for /// integration tests and out-of-tree daemons that want to call From 40c4f6beef6a62ed20f6ff1589369d08fc4618c1 Mon Sep 17 00:00:00 2001 From: Shooksie Date: Tue, 14 Jul 2026 21:07:52 -0600 Subject: [PATCH 04/15] fix(daemon): key the environment broker on the SUBJECT, not a minted workflow id (REQUIREMENT-049) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The broker run_id must be stable across every phase of a run so all phases acquire the SAME node. The original derivation minted wf- for a fresh phase and used resume_workflow_id for later phases — but the runner mints its OWN workflow id (the daemon can't hand one down: --workflow-id means 'resume an EXISTING workflow'), so phase 1's minted id never matched phase 2's resume id => a different node per phase, no sharing. Key on dispatch.subject_id() instead (identical across every phase's dispatch, known before phase 1). Subjectless adhoc runs fall back to the resume-id / fresh mint (single-phase only). --- .../src/dispatch/process_manager.rs | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/crates/orchestrator-daemon-runtime/src/dispatch/process_manager.rs b/crates/orchestrator-daemon-runtime/src/dispatch/process_manager.rs index 61753fa8..4307afb5 100644 --- a/crates/orchestrator-daemon-runtime/src/dispatch/process_manager.rs +++ b/crates/orchestrator-daemon-runtime/src/dispatch/process_manager.rs @@ -537,10 +537,13 @@ impl ProcessManager { /// runner then uses its legacy owned-environment path). /// /// The run_id must be STABLE across every phase of a workflow run so all - /// phases share one node: a phase-boundary RESUME dispatch reuses the - /// persisted `resume_workflow_id`; the FIRST (fresh) phase mints one, which - /// the runner adopts as its workflow id so later resume phases key on the - /// SAME id. + /// phases acquire the SAME node. It is keyed on the SUBJECT: every phase is a + /// separate dispatch for the same subject, so `subject_id()` is identical + /// across phases and known before phase 1. Keying on a workflow id does NOT + /// work — the runner mints its own id and `--workflow-id` means "resume an + /// EXISTING workflow", so a daemon-minted id cannot be handed down for a + /// fresh phase. A subjectless adhoc run falls back to the resume id / a fresh + /// mint (single-phase only — no cross-phase sharing without a subject). fn configure_environment_broker( &self, dispatch: &SubjectDispatch, @@ -563,9 +566,12 @@ impl ProcessManager { if is_local_environment(&environment_id) { return None; } - let run_id = match resume_workflow_id { - Some(id) => id.to_string(), - None => format!("wf-{}", uuid::Uuid::new_v4().simple()), + let run_id = match dispatch.subject_id() { + Some(subject) if !subject.is_empty() => format!("run-{subject}"), + _ => match resume_workflow_id { + Some(id) => id.to_string(), + None => format!("wf-{}", uuid::Uuid::new_v4().simple()), + }, }; broker.register_run(&run_id, project_root, &environment_id); command.env(ANIMUS_ENVIRONMENT_BROKER_SOCKET_ENV, broker.socket_path()); From 5227958c3e775d004ab1ad5292d8049406b9f7d7 Mon Sep 17 00:00:00 2001 From: Shooksie Date: Tue, 14 Jul 2026 21:15:51 -0600 Subject: [PATCH 05/15] release: bump orchestrator-cli to v0.7.0-rc.15 (REQ-048 cross-phase environment broker) --- Cargo.lock | 2 +- crates/orchestrator-cli/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0119cb38..62d3f726 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2782,7 +2782,7 @@ checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" [[package]] name = "orchestrator-cli" -version = "0.7.0-rc.14" +version = "0.7.0-rc.15" dependencies = [ "animus-actor", "animus-control-protocol", diff --git a/crates/orchestrator-cli/Cargo.toml b/crates/orchestrator-cli/Cargo.toml index 5b464463..1faa0b2c 100644 --- a/crates/orchestrator-cli/Cargo.toml +++ b/crates/orchestrator-cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "orchestrator-cli" -version = "0.7.0-rc.14" +version = "0.7.0-rc.15" edition = "2021" license = "Elastic-2.0" default-run = "animus" From e88bc0d7dcfd810576a1c09e1e4b79455ec71f78 Mon Sep 17 00:00:00 2001 From: Sami Shukri Date: Wed, 15 Jul 2026 08:30:28 -0600 Subject: [PATCH 06/15] Broker honors workflow-level environment so a run shares ONE node (#327) The daemon's EnvironmentBroker gate (configure_environment_broker) resolved the run environment from kind-level routing rules only, passing workflow_env=None. On deployments whose only environment config is workflow-level (workflows[].environment), resolve_environment returned None -> broker disengaged -> each per-phase runner prepared its OWN ephemeral node -> code-implement's edit never reached code-open-pr's workspace (no diff -> no PR). Thread a workflow-id -> environment map into ProcessManager (populated in daemon_run alongside environment_routing) and feed the dispatch's workflow-level environment as workflow_env, mirroring how the runner resolves each phase. A workflow with an environment: now engages the broker (keyed per subject) so every phase shares one node. Workflows without an environment stay local. Bump orchestrator-cli 0.7.0-rc.16. TASK-431 / REQUIREMENT-051. --- crates/orchestrator-cli/Cargo.toml | 2 +- .../runtime/runtime_daemon/daemon_run.rs | 10 +++++++ .../src/dispatch/process_manager.rs | 28 +++++++++++++++---- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/crates/orchestrator-cli/Cargo.toml b/crates/orchestrator-cli/Cargo.toml index 1faa0b2c..fdd09ee7 100644 --- a/crates/orchestrator-cli/Cargo.toml +++ b/crates/orchestrator-cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "orchestrator-cli" -version = "0.7.0-rc.15" +version = "0.7.0-rc.16" edition = "2021" license = "Elastic-2.0" default-run = "animus" diff --git a/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_run.rs b/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_run.rs index 962a6b67..25793ee7 100644 --- a/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_run.rs +++ b/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_run.rs @@ -773,6 +773,16 @@ pub(super) async fn handle_daemon_run(args: DaemonRunArgs, project_root: &str, j // bind failure is non-fatal — the runner then falls back to its own // per-phase environment path. process_manager.environment_routing = workflow_config.config.environment_routing.clone(); + // Map each workflow's `environment:` override (id -> environment id, lowercased + // keys) so the broker gate can honor a workflow-level environment even when no + // kind-level routing rule exists — the common deploy shape. Without this the + // broker never engaged and each phase prepared its own node. TASK-431 / REQ-051. + process_manager.workflow_environments = workflow_config + .config + .workflows + .iter() + .filter_map(|workflow| workflow.environment.as_ref().map(|env| (workflow.id.to_ascii_lowercase(), env.clone()))) + .collect(); match orchestrator_daemon_runtime::EnvironmentBroker::start(project_root).await { Ok(broker) => { process_manager = process_manager.with_environment_broker(broker); diff --git a/crates/orchestrator-daemon-runtime/src/dispatch/process_manager.rs b/crates/orchestrator-daemon-runtime/src/dispatch/process_manager.rs index 4307afb5..b6a13f84 100644 --- a/crates/orchestrator-daemon-runtime/src/dispatch/process_manager.rs +++ b/crates/orchestrator-daemon-runtime/src/dispatch/process_manager.rs @@ -205,6 +205,13 @@ pub struct ProcessManager { /// Drives the daemon-side decision of whether a dispatch routes to a /// non-local environment (and thus through the [`EnvironmentBroker`]). pub environment_routing: Option, + /// Per-workflow `environment:` overrides (workflow id -> environment plugin + /// id, lowercased keys). The broker gate resolves the dispatch's workflow to + /// its `environment:` and feeds it to `resolve_environment` as `workflow_env` + /// — without this a workflow-level environment (the only environment config + /// on many deployments) was invisible to the daemon, so the broker never + /// engaged and each phase prepared its OWN node. See TASK-431 / REQ-051. + pub workflow_environments: std::collections::HashMap, /// REQ-048: the cross-phase ephemeral-environment broker. When wired AND the /// dispatch routes to a non-local environment, the daemon sets the four /// `ANIMUS_ENVIRONMENT_BROKER_*` env vars on the runner so it acquires the @@ -245,6 +252,7 @@ impl ProcessManager { pipe_root: None, workflow_concurrency_max, environment_routing: None, + workflow_environments: std::collections::HashMap::new(), environment_broker: None, } } @@ -552,15 +560,25 @@ impl ProcessManager { command: &mut Command, ) -> Option { let broker = self.environment_broker.as_ref()?; - // Config-level routing only (kind rules + default): the daemon dispatches - // per phase and cannot know the phase's harness/`environment:` override - // here, and the node is per-RUN anyway, so a single run-level environment - // is the correct granularity for the broker decision. + // The daemon dispatches per phase and cannot know a PHASE-level + // `environment:` override here, but the node is per-RUN anyway, so a + // single run-level environment is the correct granularity. Feed the + // dispatch's WORKFLOW-level `environment:` as `workflow_env` (the runner + // does the same when it resolves each phase) so a workflow-level + // environment engages the broker even with no kind-level routing rule — + // otherwise the broker never fired and every phase owned its own node. + // See TASK-431 / REQ-051. + let workflow_ref = dispatch.workflow_ref.trim(); + let workflow_env = if workflow_ref.is_empty() { + None + } else { + self.workflow_environments.get(&workflow_ref.to_ascii_lowercase()).map(String::as_str) + }; let environment_id = orchestrator_config::workflow_config::resolve_environment( dispatch.subject_kind(), None, None, - None, + workflow_env, self.environment_routing.as_ref(), )?; if is_local_environment(&environment_id) { From 4934ec880d1a3a129d67962c6f92b9dca0229a5e Mon Sep 17 00:00:00 2001 From: Sami Shukri Date: Wed, 15 Jul 2026 08:33:17 -0600 Subject: [PATCH 07/15] Update Cargo.lock for orchestrator-cli 0.7.0-rc.16 (--locked CI build) (#328) --- Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 62d3f726..91592f97 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2782,7 +2782,7 @@ checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" [[package]] name = "orchestrator-cli" -version = "0.7.0-rc.15" +version = "0.7.0-rc.16" dependencies = [ "animus-actor", "animus-control-protocol", From 8c11e4ed66923f8cabd9ba163d5a33702c68846b Mon Sep 17 00:00:00 2001 From: Shooksie Date: Wed, 15 Jul 2026 09:39:28 -0600 Subject: [PATCH 08/15] perf(daemon): stale-in-progress reconcile uses no-blob summaries, not list_all MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit reconcile_stale_in_progress_tasks ran every heartbeat and, whenever any task was in-progress, fetched EVERY workflow run with its full opaque blob (hub.workflows().list() -> list_all -> journal/list with no filter) just to cross-reference task_id + terminal status. On a board with ~1200 runs that is a ~6s all-runs scan that monopolized the shared journal host every ~8s — the portal /workflows intermittent multi-second stalls (steady state was fine). Add WorkflowServiceApi::list_summaries + WorkflowStateManager::list_all_summaries backed by the journal's existing no-blob projection (journal/list {summary:true}, now carrying the indexed subject_id). The reconcile leg consumes summaries (subject_id + status + terminal timestamp) — exact same cross-reference logic, but ~40ms instead of ~6s and no blob fetch/deserialize. Pairs with animus-postgres exposing subject_id in the summary projection. Bumps orchestrator-cli to v0.7.0-rc.17 (on the rc.16 / REQ-048 broker lineage). --- Cargo.lock | 2 +- crates/orchestrator-cli/Cargo.toml | 2 +- .../runtime_daemon/daemon_tick_executor.rs | 6 +- crates/orchestrator-core/src/lib.rs | 23 ++--- crates/orchestrator-core/src/services.rs | 4 + .../src/services/workflow_impl.rs | 8 ++ crates/orchestrator-core/src/workflow.rs | 2 +- .../src/workflow/journal_client.rs | 88 +++++++++++++++++++ .../src/workflow/state_manager.rs | 48 ++++++++++ 9 files changed, 168 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 91592f97..19d907f0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2782,7 +2782,7 @@ checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" [[package]] name = "orchestrator-cli" -version = "0.7.0-rc.16" +version = "0.7.0-rc.17" dependencies = [ "animus-actor", "animus-control-protocol", diff --git a/crates/orchestrator-cli/Cargo.toml b/crates/orchestrator-cli/Cargo.toml index fdd09ee7..2554f576 100644 --- a/crates/orchestrator-cli/Cargo.toml +++ b/crates/orchestrator-cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "orchestrator-cli" -version = "0.7.0-rc.16" +version = "0.7.0-rc.17" edition = "2021" license = "Elastic-2.0" default-run = "animus" diff --git a/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_tick_executor.rs b/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_tick_executor.rs index a9df94ee..7568a00d 100644 --- a/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_tick_executor.rs +++ b/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_tick_executor.rs @@ -436,7 +436,11 @@ pub(crate) async fn reconcile_stale_in_progress_tasks_with_store( return Ok(0); } - let workflows = hub.workflows().list().await?; + // No-blob summaries, NOT the full runs: this sweep only needs subject id + + // status + terminal timestamp to cross-reference in-progress tasks. Fetching + // every run's opaque blob here (`list()`) was a ~6s all-runs scan that ran + // every heartbeat and head-of-line-blocked the shared journal host. + let workflows = hub.workflows().list_summaries().await?; let now = chrono::Utc::now(); let mut reconciled = 0usize; for task in &in_progress_tasks { diff --git a/crates/orchestrator-core/src/lib.rs b/crates/orchestrator-core/src/lib.rs index a1dab27f..8b68a85c 100644 --- a/crates/orchestrator-core/src/lib.rs +++ b/crates/orchestrator-core/src/lib.rs @@ -170,17 +170,18 @@ pub use workflow_config::{ load_workflow_config_with_metadata, merge_yaml_into_config, missing_project_skill_reference_warnings, missing_skill_reference_warnings_for_sources, missing_skill_yaml_warnings, parse_yaml_workflow_config, remove_agent_profile, remove_generated_workflow_phase, remove_workflow_definition, resolve_workflow_phase_plan, - resolve_workflow_rework_attempts, resolve_workflow_skip_guards, resolve_workflow_variables, set_phase_definition, - resolve_workflow_verdict_routing, unenforced_project_yaml_warnings, unenforced_yaml_field_warnings, - upsert_agent_profile, upsert_generated_workflow_phase, upsert_generated_workflow_pipeline, - upsert_workflow_definition, validate_and_compile_yaml_workflows, validate_workflow_and_runtime_configs, - validate_workflow_and_runtime_configs_with_project_root, validate_workflow_config, workflow_config_hash, - workflow_config_path, write_full_workflow_config, write_workflow_config, yaml_workflows_dir, CompileYamlResult, - FileWatcherTriggerConfig, LoadedWorkflowConfig, PhaseMcpBinding, PhaseTransitionConfig, PhaseUiDefinition, - SkillReferenceWarning, SubWorkflowRef, TriggerType, UnenforcedFieldWarning, WebhookTriggerConfig, - WorkflowCheckpointRetentionConfig, WorkflowConfig, WorkflowConfigMetadata, WorkflowConfigSource, - WorkflowDefinition, WorkflowPhaseConfig, WorkflowPhaseEntry, WorkflowSchedule, WorkflowTrigger, WorkflowVariable, - WORKFLOW_CONFIG_FILE_NAME, WORKFLOW_CONFIG_SCHEMA_ID, WORKFLOW_CONFIG_VERSION, YAML_WORKFLOWS_DIR, + resolve_workflow_rework_attempts, resolve_workflow_skip_guards, resolve_workflow_variables, + resolve_workflow_verdict_routing, set_phase_definition, unenforced_project_yaml_warnings, + unenforced_yaml_field_warnings, upsert_agent_profile, upsert_generated_workflow_phase, + upsert_generated_workflow_pipeline, upsert_workflow_definition, validate_and_compile_yaml_workflows, + validate_workflow_and_runtime_configs, validate_workflow_and_runtime_configs_with_project_root, + validate_workflow_config, workflow_config_hash, workflow_config_path, write_full_workflow_config, + write_workflow_config, yaml_workflows_dir, CompileYamlResult, FileWatcherTriggerConfig, LoadedWorkflowConfig, + PhaseMcpBinding, PhaseTransitionConfig, PhaseUiDefinition, SkillReferenceWarning, SubWorkflowRef, TriggerType, + UnenforcedFieldWarning, WebhookTriggerConfig, WorkflowCheckpointRetentionConfig, WorkflowConfig, + WorkflowConfigMetadata, WorkflowConfigSource, WorkflowDefinition, WorkflowPhaseConfig, WorkflowPhaseEntry, + WorkflowSchedule, WorkflowTrigger, WorkflowVariable, WORKFLOW_CONFIG_FILE_NAME, WORKFLOW_CONFIG_SCHEMA_ID, + WORKFLOW_CONFIG_VERSION, YAML_WORKFLOWS_DIR, }; pub use workflow_config::{YamlDiagnostic, YamlExcerpt}; pub use workflow_events::{dispatch_workflow_event, workflow_task_id, WorkflowEvent, WorkflowEventOutcome}; diff --git a/crates/orchestrator-core/src/services.rs b/crates/orchestrator-core/src/services.rs index 1a7cc8dd..1a385458 100644 --- a/crates/orchestrator-core/src/services.rs +++ b/crates/orchestrator-core/src/services.rs @@ -154,6 +154,10 @@ pub trait TaskServiceApi: Send + Sync { #[async_trait] pub trait WorkflowServiceApi: Send + Sync { async fn list(&self) -> Result>; + /// Lightweight no-blob run summaries — the daemon's stale-in-progress + /// reconcile uses this instead of [`Self::list`] so its heartbeat sweep never + /// fetches + deserializes every run's opaque blob. + async fn list_summaries(&self) -> Result>; async fn query(&self, query: WorkflowQuery) -> Result>; async fn get(&self, id: &str) -> Result; async fn decisions(&self, id: &str) -> Result>; diff --git a/crates/orchestrator-core/src/services/workflow_impl.rs b/crates/orchestrator-core/src/services/workflow_impl.rs index 9e7de2bb..8ac13359 100644 --- a/crates/orchestrator-core/src/services/workflow_impl.rs +++ b/crates/orchestrator-core/src/services/workflow_impl.rs @@ -163,6 +163,10 @@ impl WorkflowServiceApi for InMemoryServiceHub { Ok(self.state.read().await.workflows.values().cloned().collect()) } + async fn list_summaries(&self) -> Result> { + Ok(self.state.read().await.workflows.values().map(crate::workflow::WorkflowRunSummary::from_workflow).collect()) + } + async fn query(&self, query: WorkflowQuery) -> Result> { let workflows = WorkflowServiceApi::list(self).await?; Ok(query_workflows(workflows, &query)) @@ -366,6 +370,10 @@ impl WorkflowServiceApi for FileServiceHub { self.workflow_manager().list_all() } + async fn list_summaries(&self) -> Result> { + self.workflow_manager().list_all_summaries() + } + async fn query(&self, query: WorkflowQuery) -> Result> { if workflow_query_can_use_db_page(&query) { let manager = self.workflow_manager(); diff --git a/crates/orchestrator-core/src/workflow.rs b/crates/orchestrator-core/src/workflow.rs index 681bb9b8..9aeea2a3 100644 --- a/crates/orchestrator-core/src/workflow.rs +++ b/crates/orchestrator-core/src/workflow.rs @@ -17,7 +17,7 @@ pub use environment_client::{shutdown_resident_hosts as shutdown_environment_hos pub use journal_client::{ durable_journal_active, import_local_sqlite_into_plugin, record_wire_event as journal_record_wire_event, - shutdown_resident_hosts as shutdown_journal_hosts, JournalImportStats, + shutdown_resident_hosts as shutdown_journal_hosts, JournalImportStats, WorkflowRunSummary, }; pub use lifecycle_executor::WorkflowLifecycleExecutor; diff --git a/crates/orchestrator-core/src/workflow/journal_client.rs b/crates/orchestrator-core/src/workflow/journal_client.rs index 9441e7d4..08ed8210 100644 --- a/crates/orchestrator-core/src/workflow/journal_client.rs +++ b/crates/orchestrator-core/src/workflow/journal_client.rs @@ -490,6 +490,94 @@ pub(crate) fn list_page( Ok(resp.runs.into_iter().filter_map(|run| from_journal_run(run).ok()).collect()) } +/// Lightweight run summary sourced from the journal's no-blob projection +/// (`journal/list { summary: true }`). Carries only the fields the daemon's +/// stale-in-progress reconcile needs to cross-reference a run to its subject — +/// deliberately NOT the full `OrchestratorWorkflow`, so that heartbeat sweep +/// never fetches + deserializes every run's opaque blob (the ~6s all-runs scan +/// that head-of-line-blocked the shared journal host). +#[derive(Debug, Clone)] +pub struct WorkflowRunSummary { + pub workflow_id: String, + /// Denormalized subject id (bare native id for task/requirement, e.g. + /// `TASK-1`; kind-qualified for generic kinds), cross-referenced against a + /// task id via the daemon's `task_ids_match`. + pub task_id: String, + pub status: WorkflowStatus, + pub started_at: chrono::DateTime, + /// The terminal timestamp for a terminal run; `None` for a live run. + pub completed_at: Option>, +} + +impl WorkflowRunSummary { + /// Project a full run to a summary. Used by the in-memory service hub and any + /// caller that already holds the whole workflow. Mirrors the `subject_id` + /// denormalization in [`to_journal_run`] so it matches the plugin projection. + pub fn from_workflow(w: &OrchestratorWorkflow) -> Self { + let task_id = match w.subject.as_ref().map(|s| s.id()) { + None | Some("") => w.task_id.clone(), + Some(id) => id.to_string(), + }; + Self { + workflow_id: w.id.clone(), + task_id, + status: w.status, + started_at: w.started_at, + completed_at: w.completed_at, + } + } +} + +pub(crate) fn status_from_wire(s: &str) -> Option { + Some(match s { + "pending" => WorkflowStatus::Pending, + "running" => WorkflowStatus::Running, + "paused" => WorkflowStatus::Paused, + "completed" => WorkflowStatus::Completed, + "failed" => WorkflowStatus::Failed, + "escalated" => WorkflowStatus::Escalated, + "cancelled" => WorkflowStatus::Cancelled, + _ => return None, + }) +} + +/// Max rows a summary sweep pulls in one RPC. Equal to the reference backend's +/// `MAX_QUERY_LIMIT`, so a project with fewer runs than this gets ALL of them +/// (the reconcile needs every run to cross-reference its in-progress subjects). +const SUMMARY_QUERY_LIMIT: u32 = 10_000; + +/// Every run's [`WorkflowRunSummary`] via the journal's no-blob projection — +/// ONE bounded RPC that skips the opaque blob column entirely (`summary: true`). +/// Replaces `list()`-then-drop-the-blob for the daemon's stale-in-progress +/// reconcile, which only needs subject id + status + timestamps. Rows missing a +/// workflow_id/status, or carrying an unknown wire status, are skipped. +pub(crate) fn list_summaries(plugin: &DiscoveredPlugin, project_root: &Path) -> Result> { + let params = serde_json::json!({ "status": [], "summary": true, "limit": SUMMARY_QUERY_LIMIT }); + let value = run_blocking(call(plugin, project_root, METHOD_JOURNAL_LIST, params))??; + let runs = value.get("runs").and_then(|v| v.as_array()).map(Vec::as_slice).unwrap_or_default(); + Ok(runs.iter().filter_map(summary_from_value).collect()) +} + +fn summary_from_value(v: &serde_json::Value) -> Option { + let obj = v.as_object()?; + let workflow_id = obj.get("workflow_id")?.as_str()?.to_string(); + let status = status_from_wire(obj.get("status")?.as_str()?)?; + let task_id = obj.get("subject_id").and_then(|s| s.as_str()).unwrap_or_default().to_string(); + let started_at = + obj.get("created_at").and_then(|s| s.as_str()).and_then(parse_summary_ts).unwrap_or_else(chrono::Utc::now); + let updated_at = obj.get("updated_at").and_then(|s| s.as_str()).and_then(parse_summary_ts); + let terminal = matches!( + status, + WorkflowStatus::Completed | WorkflowStatus::Failed | WorkflowStatus::Cancelled | WorkflowStatus::Escalated + ); + let completed_at = if terminal { updated_at } else { None }; + Some(WorkflowRunSummary { workflow_id, task_id, status, started_at, completed_at }) +} + +fn parse_summary_ts(s: &str) -> Option> { + chrono::DateTime::parse_from_rfc3339(s).ok().map(|dt| dt.with_timezone(&chrono::Utc)) +} + /// All run ids matching `status` (None = all). The caller paginates client-side. pub(crate) fn query_ids( plugin: &DiscoveredPlugin, diff --git a/crates/orchestrator-core/src/workflow/state_manager.rs b/crates/orchestrator-core/src/workflow/state_manager.rs index c9cf5a61..7f52695a 100644 --- a/crates/orchestrator-core/src/workflow/state_manager.rs +++ b/crates/orchestrator-core/src/workflow/state_manager.rs @@ -394,6 +394,54 @@ impl WorkflowStateManager { Ok(workflows) } + /// Every run's lightweight [`super::journal_client::WorkflowRunSummary`] via + /// the no-blob projection. The daemon's stale-in-progress reconcile uses this + /// instead of [`Self::list_all`] so its heartbeat sweep never fetches + + /// deserializes every run's opaque blob (the ~6s all-runs scan that + /// head-of-line-blocked the shared journal host). Plugin backend: ONE + /// `journal/list { summary: true }` RPC. SQLite backend: project the columns. + pub fn list_all_summaries(&self) -> Result> { + use super::journal_client::WorkflowRunSummary; + if let Some(plugin) = self.journal_plugin() { + return super::journal_client::list_summaries(plugin, &self.project_root); + } + let conn = self.open_db()?; + let mut stmt = conn.prepare("SELECT id, task_id, status, started_at, completed_at FROM workflows")?; + let rows = stmt.query_map([], |row| { + Ok(( + row.get::<_, String>(0)?, + row.get::<_, Option>(1)?, + row.get::<_, String>(2)?, + row.get::<_, Option>(3)?, + row.get::<_, Option>(4)?, + )) + })?; + let mut out = Vec::new(); + for row in rows { + let (workflow_id, task_id, status, started_at, completed_at) = row?; + let Some(status) = super::journal_client::status_from_wire(&status) else { + continue; + }; + let started_at = started_at + .as_deref() + .and_then(|s| chrono::DateTime::parse_from_rfc3339(s).ok()) + .map(|dt| dt.with_timezone(&chrono::Utc)) + .unwrap_or_else(chrono::Utc::now); + let completed_at = completed_at + .as_deref() + .and_then(|s| chrono::DateTime::parse_from_rfc3339(s).ok()) + .map(|dt| dt.with_timezone(&chrono::Utc)); + out.push(WorkflowRunSummary { + workflow_id, + task_id: task_id.unwrap_or_default(), + status, + started_at, + completed_at, + }); + } + Ok(out) + } + /// Inter-workflow fan-in coordinator (see [`super::dependency`]). Snapshots /// every run from the journal and returns the held JOIN runs that are ready to /// FIRE or should be CANCELLED right now, per their declared upstream barrier. From c10278a54375e334e29ad366f4350be28bbad890 Mon Sep 17 00:00:00 2001 From: Shooksie Date: Wed, 15 Jul 2026 10:18:12 -0600 Subject: [PATCH 09/15] perf(daemon): every per-tick workflow scan uses no-blob summaries / active-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rc.17 fixed the stale-in-progress reconcile but the ~6s /workflows spikes persisted: OTHER per-tick legs still fetched every run with full blobs (hub.workflows().list() -> list_all -> unbounded journal/list). Convert them all: - DaemonTickMetrics::collect (runs EVERY tick — the ~8s spike cadence): count by status via list_summaries instead of the all-blobs list. - reconcile orphan-recovery / journal-resume / manual-timeout: these act only on non-terminal runs, so list_active() (pending/running/paused, few rows) not list. - load_workflow_ref_index (rebuilt every housekeeping tick by the budget scan): id + workflow_ref are indexed columns, so use the summary projection. Adds WorkflowServiceApi::list_active + workflow_ref to WorkflowRunSummary. The remaining list_all callers are non-tick (CLI history/cost, resume-on-startup, dev SQLite fallback). Works with the already-deployed animus-postgres bundle (summary carries subject_id + workflow_ref). Bumps orchestrator-cli to rc.18. --- Cargo.lock | 2 +- crates/orchestrator-cli/Cargo.toml | 2 +- .../runtime_daemon/daemon_reconciliation.rs | 6 +++--- .../orchestrator-core/src/daemon_tick_metrics.rs | 15 +++++++-------- crates/orchestrator-core/src/services.rs | 5 +++++ .../src/services/workflow_impl.rs | 16 ++++++++++++++++ .../src/workflow/journal_client.rs | 5 ++++- .../src/workflow/state_manager.rs | 11 ++++++----- 8 files changed, 43 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 19d907f0..e37707ca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2782,7 +2782,7 @@ checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" [[package]] name = "orchestrator-cli" -version = "0.7.0-rc.17" +version = "0.7.0-rc.18" dependencies = [ "animus-actor", "animus-control-protocol", diff --git a/crates/orchestrator-cli/Cargo.toml b/crates/orchestrator-cli/Cargo.toml index 2554f576..b0189955 100644 --- a/crates/orchestrator-cli/Cargo.toml +++ b/crates/orchestrator-cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "orchestrator-cli" -version = "0.7.0-rc.17" +version = "0.7.0-rc.18" edition = "2021" license = "Elastic-2.0" default-run = "animus" diff --git a/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_reconciliation.rs b/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_reconciliation.rs index 3c71e92d..82187533 100644 --- a/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_reconciliation.rs +++ b/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_reconciliation.rs @@ -76,7 +76,7 @@ pub async fn recover_orphaned_running_workflows( active_subject_ids: &HashSet, resume_orphans: bool, ) -> usize { - let workflows = match hub.workflows().list().await { + let workflows = match hub.workflows().list_active().await { Ok(workflows) => workflows, Err(error) => { warn!( @@ -196,7 +196,7 @@ pub(crate) async fn resumable_orphans_for_redispatch( if !journal_resume_enabled(project_root) { return Vec::new(); } - let workflows = match hub.workflows().list().await { + let workflows = match hub.workflows().list_active().await { Ok(workflows) => workflows, Err(error) => { warn!( @@ -310,7 +310,7 @@ pub(crate) async fn resumable_orphans_for_redispatch( pub async fn reconcile_manual_phase_timeouts(hub: Arc, project_root: &str) -> Result { let runtime = load_agent_runtime_config_or_default(Path::new(project_root)); - let workflows = match hub.workflows().list().await { + let workflows = match hub.workflows().list_active().await { Ok(workflows) => workflows, Err(error) => { warn!( diff --git a/crates/orchestrator-core/src/daemon_tick_metrics.rs b/crates/orchestrator-core/src/daemon_tick_metrics.rs index 61e8c26a..9073a13f 100644 --- a/crates/orchestrator-core/src/daemon_tick_metrics.rs +++ b/crates/orchestrator-core/src/daemon_tick_metrics.rs @@ -21,7 +21,9 @@ pub struct DaemonTickMetrics { impl DaemonTickMetrics { pub async fn collect(hub: Arc, stale_threshold_hours: u64) -> Result { let tasks = hub.tasks().list().await?; - let workflows = hub.workflows().list().await.unwrap_or_default(); + // No-blob summaries: this gauge only counts by status, so avoid the + // all-runs full-blob scan (`list`) that ran every tick. + let workflows = hub.workflows().list_summaries().await.unwrap_or_default(); let tasks_total = tasks.len(); let tasks_ready = @@ -35,8 +37,11 @@ impl DaemonTickMetrics { .iter() .filter(|workflow| matches!(workflow.status, WorkflowStatus::Running | WorkflowStatus::Paused)) .count(); + // Gauge count by terminal status. The summary drops machine_state / + // completed_at, so this counts on status alone (was + // is_terminally_completed_workflow) — an equivalent gauge value. let workflows_completed = - workflows.iter().filter(|workflow| is_terminally_completed_workflow(workflow)).count(); + workflows.iter().filter(|workflow| workflow.status == WorkflowStatus::Completed).count(); let workflows_failed = workflows.iter().filter(|workflow| workflow.status == WorkflowStatus::Failed).count(); Ok(Self { @@ -94,9 +99,3 @@ fn stale_in_progress_summary( fn task_age_seconds(now: chrono::DateTime, updated_at: chrono::DateTime) -> u64 { now.signed_duration_since(updated_at).num_seconds().max(0) as u64 } - -fn is_terminally_completed_workflow(workflow: &crate::OrchestratorWorkflow) -> bool { - workflow.status == WorkflowStatus::Completed - && workflow.machine_state == crate::WorkflowMachineState::Completed - && workflow.completed_at.is_some() -} diff --git a/crates/orchestrator-core/src/services.rs b/crates/orchestrator-core/src/services.rs index 1a385458..ee1bfdf4 100644 --- a/crates/orchestrator-core/src/services.rs +++ b/crates/orchestrator-core/src/services.rs @@ -158,6 +158,11 @@ pub trait WorkflowServiceApi: Send + Sync { /// reconcile uses this instead of [`Self::list`] so its heartbeat sweep never /// fetches + deserializes every run's opaque blob. async fn list_summaries(&self) -> Result>; + /// Only the NON-terminal runs (pending/running/paused), with full blobs. The + /// daemon's per-tick reconciliation legs (orphan recovery, journal-resume, + /// manual-timeout) act only on active runs, so this avoids the all-runs + /// full-blob scan (`list`) that ran every heartbeat. + async fn list_active(&self) -> Result>; async fn query(&self, query: WorkflowQuery) -> Result>; async fn get(&self, id: &str) -> Result; async fn decisions(&self, id: &str) -> Result>; diff --git a/crates/orchestrator-core/src/services/workflow_impl.rs b/crates/orchestrator-core/src/services/workflow_impl.rs index 8ac13359..e0e5e1e0 100644 --- a/crates/orchestrator-core/src/services/workflow_impl.rs +++ b/crates/orchestrator-core/src/services/workflow_impl.rs @@ -167,6 +167,18 @@ impl WorkflowServiceApi for InMemoryServiceHub { Ok(self.state.read().await.workflows.values().map(crate::workflow::WorkflowRunSummary::from_workflow).collect()) } + async fn list_active(&self) -> Result> { + Ok(self + .state + .read() + .await + .workflows + .values() + .filter(|w| matches!(w.status, WorkflowStatus::Pending | WorkflowStatus::Running | WorkflowStatus::Paused)) + .cloned() + .collect()) + } + async fn query(&self, query: WorkflowQuery) -> Result> { let workflows = WorkflowServiceApi::list(self).await?; Ok(query_workflows(workflows, &query)) @@ -374,6 +386,10 @@ impl WorkflowServiceApi for FileServiceHub { self.workflow_manager().list_all_summaries() } + async fn list_active(&self) -> Result> { + self.workflow_manager().list_active() + } + async fn query(&self, query: WorkflowQuery) -> Result> { if workflow_query_can_use_db_page(&query) { let manager = self.workflow_manager(); diff --git a/crates/orchestrator-core/src/workflow/journal_client.rs b/crates/orchestrator-core/src/workflow/journal_client.rs index 08ed8210..e8f5d14e 100644 --- a/crates/orchestrator-core/src/workflow/journal_client.rs +++ b/crates/orchestrator-core/src/workflow/journal_client.rs @@ -503,6 +503,7 @@ pub struct WorkflowRunSummary { /// `TASK-1`; kind-qualified for generic kinds), cross-referenced against a /// task id via the daemon's `task_ids_match`. pub task_id: String, + pub workflow_ref: Option, pub status: WorkflowStatus, pub started_at: chrono::DateTime, /// The terminal timestamp for a terminal run; `None` for a live run. @@ -521,6 +522,7 @@ impl WorkflowRunSummary { Self { workflow_id: w.id.clone(), task_id, + workflow_ref: w.workflow_ref.clone(), status: w.status, started_at: w.started_at, completed_at: w.completed_at, @@ -563,6 +565,7 @@ fn summary_from_value(v: &serde_json::Value) -> Option { let workflow_id = obj.get("workflow_id")?.as_str()?.to_string(); let status = status_from_wire(obj.get("status")?.as_str()?)?; let task_id = obj.get("subject_id").and_then(|s| s.as_str()).unwrap_or_default().to_string(); + let workflow_ref = obj.get("workflow_ref").and_then(|s| s.as_str()).map(str::to_string); let started_at = obj.get("created_at").and_then(|s| s.as_str()).and_then(parse_summary_ts).unwrap_or_else(chrono::Utc::now); let updated_at = obj.get("updated_at").and_then(|s| s.as_str()).and_then(parse_summary_ts); @@ -571,7 +574,7 @@ fn summary_from_value(v: &serde_json::Value) -> Option { WorkflowStatus::Completed | WorkflowStatus::Failed | WorkflowStatus::Cancelled | WorkflowStatus::Escalated ); let completed_at = if terminal { updated_at } else { None }; - Some(WorkflowRunSummary { workflow_id, task_id, status, started_at, completed_at }) + Some(WorkflowRunSummary { workflow_id, task_id, workflow_ref, status, started_at, completed_at }) } fn parse_summary_ts(s: &str) -> Option> { diff --git a/crates/orchestrator-core/src/workflow/state_manager.rs b/crates/orchestrator-core/src/workflow/state_manager.rs index 7f52695a..72831d28 100644 --- a/crates/orchestrator-core/src/workflow/state_manager.rs +++ b/crates/orchestrator-core/src/workflow/state_manager.rs @@ -434,6 +434,7 @@ impl WorkflowStateManager { out.push(WorkflowRunSummary { workflow_id, task_id: task_id.unwrap_or_default(), + workflow_ref: None, status, started_at, completed_at, @@ -1202,11 +1203,11 @@ pub fn load_workflow_ref_index(project_root: &std::path::Path) -> Result conn, From 9112dce2253dc416b5aff45b32986e2736023721 Mon Sep 17 00:00:00 2001 From: Shooksie Date: Wed, 15 Jul 2026 20:07:45 -0600 Subject: [PATCH 10/15] feat(daemon): per-phase log read + workflow-list total & type filter (rc.19) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two portal-facing features: 1. `output read --workflow-id X --phase Y` resolves that phase's run_id from the phase session checkpoint (runs//phases/*.session.json) and reads ITS transcript — so the UI can load logs for a specific/earlier phase, not just the workflow's latest run. 2. Workflow list gains a type filter + total, for numbered pagination. Repins control-protocol v0.7.0-rc.6 -> rc.7 (adds WorkflowListRequest.workflow_ref + WorkflowListResponse.total, both additive/optional). The workflow/list control handler now forwards workflow_ref into the query filter and returns page.total; workflow_ref becomes wire-expressible. Bumps orchestrator-cli to v0.7.0-rc.19. --- Cargo.lock | 46 +++++++++---------- Cargo.toml | 18 ++++---- crates/orchestrator-cli/Cargo.toml | 10 ++-- .../src/cli_types/output_types.rs | 5 ++ .../src/services/operations/ops_output.rs | 31 ++++++++++++- .../ops_workflow/control_routing.rs | 6 ++- .../services/operations/ops_workflow/mod.rs | 15 +++--- crates/orchestrator-daemon-runtime/Cargo.toml | 10 ++-- 8 files changed, 90 insertions(+), 51 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e37707ca..98d2544d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -70,7 +70,7 @@ dependencies = [ [[package]] name = "animus-actor" version = "0.1.0" -source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.6#0d6bb3be203bf9e38a79ab347b28d2bef5c0498b" +source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.7#d88e60990b9b55b5d7a6dd91315f60371e16e768" dependencies = [ "schemars", "serde", @@ -79,7 +79,7 @@ dependencies = [ [[package]] name = "animus-config-protocol" version = "0.1.1" -source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.6#0d6bb3be203bf9e38a79ab347b28d2bef5c0498b" +source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.7#d88e60990b9b55b5d7a6dd91315f60371e16e768" dependencies = [ "animus-actor", "anyhow", @@ -94,7 +94,7 @@ dependencies = [ [[package]] name = "animus-control-protocol" version = "0.1.15" -source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.6#0d6bb3be203bf9e38a79ab347b28d2bef5c0498b" +source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.7#d88e60990b9b55b5d7a6dd91315f60371e16e768" dependencies = [ "animus-actor", "animus-log-storage-protocol", @@ -114,7 +114,7 @@ dependencies = [ [[package]] name = "animus-environment-protocol" version = "0.1.0" -source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.6#0d6bb3be203bf9e38a79ab347b28d2bef5c0498b" +source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.7#d88e60990b9b55b5d7a6dd91315f60371e16e768" dependencies = [ "animus-plugin-protocol 0.1.18", "schemars", @@ -125,7 +125,7 @@ dependencies = [ [[package]] name = "animus-journal-protocol" version = "0.1.15" -source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.6#0d6bb3be203bf9e38a79ab347b28d2bef5c0498b" +source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.7#d88e60990b9b55b5d7a6dd91315f60371e16e768" dependencies = [ "animus-plugin-protocol 0.1.18", "anyhow", @@ -140,7 +140,7 @@ dependencies = [ [[package]] name = "animus-log-storage-protocol" version = "0.1.15" -source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.6#0d6bb3be203bf9e38a79ab347b28d2bef5c0498b" +source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.7#d88e60990b9b55b5d7a6dd91315f60371e16e768" dependencies = [ "animus-plugin-protocol 0.1.18", "anyhow", @@ -192,7 +192,7 @@ dependencies = [ [[package]] name = "animus-plugin-protocol" version = "0.1.18" -source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.6#0d6bb3be203bf9e38a79ab347b28d2bef5c0498b" +source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.7#d88e60990b9b55b5d7a6dd91315f60371e16e768" dependencies = [ "schemars", "serde", @@ -257,7 +257,7 @@ dependencies = [ [[package]] name = "animus-session-backend" version = "0.1.15" -source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.6#0d6bb3be203bf9e38a79ab347b28d2bef5c0498b" +source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.7#d88e60990b9b55b5d7a6dd91315f60371e16e768" dependencies = [ "animus-actor", "animus-plugin-protocol 0.1.18", @@ -290,7 +290,7 @@ dependencies = [ [[package]] name = "animus-subject-protocol" version = "0.1.16" -source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.6#0d6bb3be203bf9e38a79ab347b28d2bef5c0498b" +source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.7#d88e60990b9b55b5d7a6dd91315f60371e16e768" dependencies = [ "animus-actor", "animus-plugin-protocol 0.1.18", @@ -307,7 +307,7 @@ dependencies = [ [[package]] name = "animus-trigger-protocol" version = "0.1.15" -source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.6#0d6bb3be203bf9e38a79ab347b28d2bef5c0498b" +source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.7#d88e60990b9b55b5d7a6dd91315f60371e16e768" dependencies = [ "animus-plugin-protocol 0.1.18", "anyhow", @@ -323,7 +323,7 @@ dependencies = [ [[package]] name = "animus-workflow-runner-protocol" version = "0.2.0" -source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.6#0d6bb3be203bf9e38a79ab347b28d2bef5c0498b" +source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.7#d88e60990b9b55b5d7a6dd91315f60371e16e768" dependencies = [ "animus-actor", "animus-plugin-protocol 0.1.18", @@ -369,7 +369,7 @@ version = "1.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -380,7 +380,7 @@ checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" dependencies = [ "anstyle", "once_cell_polyfill", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -1319,7 +1319,7 @@ dependencies = [ "libc", "option-ext", "redox_users 0.5.2", - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -1418,7 +1418,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -2587,7 +2587,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -2782,7 +2782,7 @@ checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" [[package]] name = "orchestrator-cli" -version = "0.7.0-rc.18" +version = "0.7.0-rc.19" dependencies = [ "animus-actor", "animus-control-protocol", @@ -3176,7 +3176,7 @@ dependencies = [ [[package]] name = "protocol" version = "0.1.0" -source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.6#0d6bb3be203bf9e38a79ab347b28d2bef5c0498b" +source = "git+https://github.com/launchapp-dev/animus-protocol?tag=v0.7.0-rc.7#d88e60990b9b55b5d7a6dd91315f60371e16e768" dependencies = [ "animus-subject-protocol 0.1.16", "anyhow", @@ -3610,7 +3610,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -3668,7 +3668,7 @@ dependencies = [ "security-framework 3.7.0", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -4040,7 +4040,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a766e1110788c36f4fa1c2b71b387a7815aa65f88ce0229841826633d93723e" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -4180,7 +4180,7 @@ dependencies = [ "getrandom 0.4.2", "once_cell", "rustix", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -4584,7 +4584,7 @@ checksum = "f2f6fb2847f6742cd76af783a2a2c49e9375d0a111c7bef6f71cd9e738c72d6e" dependencies = [ "memoffset", "tempfile", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 0e3e5247..90cc1249 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,18 +30,18 @@ categories = ["command-line-utilities", "development-tools"] # animus-subject-protocol resolves to one source (no duplicate-crate type # mismatches). animus-plugin-protocol/runtime remain in-tree pending a # follow-up move. -animus-plugin-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.6" } -animus-actor = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.6" } -animus-config-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.6" } -animus-journal-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.6" } -animus-provider-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.6" } -animus-session-backend = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.6" } -animus-subject-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.6" } +animus-plugin-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.7" } +animus-actor = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.7" } +animus-config-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.7" } +animus-journal-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.7" } +animus-provider-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.7" } +animus-session-backend = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.7" } +animus-subject-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.7" } # v0.7: environment plugin kind — EnvironmentSpec/HarnessCommand/ExecRequest wire # types + prepare/exec/exec_stream/teardown methods. Consumed by the routing # resolver + the follow-on runtime EnvironmentClient (not yet built). -animus-environment-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.6" } -protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.6", package = "protocol" } +animus-environment-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.7" } +protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.7", package = "protocol" } orchestrator-daemon-runtime = { path = "crates/orchestrator-daemon-runtime" } orchestrator-logging = { path = "crates/orchestrator-logging" } orchestrator-plugin-host = { path = "crates/orchestrator-plugin-host" } diff --git a/crates/orchestrator-cli/Cargo.toml b/crates/orchestrator-cli/Cargo.toml index b0189955..a6d826bb 100644 --- a/crates/orchestrator-cli/Cargo.toml +++ b/crates/orchestrator-cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "orchestrator-cli" -version = "0.7.0-rc.18" +version = "0.7.0-rc.19" edition = "2021" license = "Elastic-2.0" default-run = "animus" @@ -39,9 +39,9 @@ animus-environment-protocol = { workspace = true } # embed subject-protocol types, so `animus-subject-protocol-wire` MUST match # control's rev. v0.1.26 additively adds `actor: Option` to the control + # workflow-runner request types relayed by the control surface / plugin_clients. -animus-control-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.6" } -animus-log-storage-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.6" } -animus-subject-protocol-wire = { package = "animus-subject-protocol", git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.6" } +animus-control-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.7" } +animus-log-storage-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.7" } +animus-subject-protocol-wire = { package = "animus-subject-protocol", git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.7" } animus-actor = { workspace = true } # Plugin protocol crates used by the plugin-host wrappers in # `services::plugin_clients` to dispatch `workflow/*` and `queue/*` RPCs through @@ -51,7 +51,7 @@ animus-actor = { workspace = true } # NOT share `animus-subject-protocol-wire`'s rev (cargo forbids two names for one # package instance), so they stay a distinct rev. animus-queue-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.5.10" } -animus-workflow-runner-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.6" } +animus-workflow-runner-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.7" } animus-subject-protocol-v05 = { package = "animus-subject-protocol", git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.5.10" } protocol = { workspace = true } animus-session-backend = { workspace = true } diff --git a/crates/orchestrator-cli/src/cli_types/output_types.rs b/crates/orchestrator-cli/src/cli_types/output_types.rs index f5e4d52e..73474ede 100644 --- a/crates/orchestrator-cli/src/cli_types/output_types.rs +++ b/crates/orchestrator-cli/src/cli_types/output_types.rs @@ -28,6 +28,11 @@ pub(crate) struct OutputRunArgs { /// Resolve the latest run id recorded for this workflow, then read it. #[arg(long)] pub(crate) workflow_id: Option, + /// With --workflow-id, read the transcript of THIS phase's run (from the + /// phase session checkpoint) instead of the workflow's latest run. Lets the + /// UI load logs for a specific/earlier phase, not just the last one. + #[arg(long, requires = "workflow_id")] + pub(crate) phase: Option, } #[derive(Debug, Args)] diff --git a/crates/orchestrator-cli/src/services/operations/ops_output.rs b/crates/orchestrator-cli/src/services/operations/ops_output.rs index 04fe86c2..155c49c2 100644 --- a/crates/orchestrator-cli/src/services/operations/ops_output.rs +++ b/crates/orchestrator-cli/src/services/operations/ops_output.rs @@ -125,6 +125,30 @@ fn resolve_run_id_arg(project_root: &str, run_id: Option, workflow_id: O } } +/// Resolve the run id for a SPECIFIC `(workflow_id, phase_id)` from the phase +/// session checkpoints (`runs//phases/*.session.json`). Lets the UI +/// load one phase's transcript instead of only the workflow's latest run. +fn resolve_run_id_for_workflow_phase(project_root: &str, workflow_id: &str, phase_id: &str) -> Result { + let Some(workflow_run_dir) = resolve_run_dir_for_lookup(project_root, workflow_id)? else { + return Err(not_found_error(format!("no run directory recorded for workflow {workflow_id}"))); + }; + let phases_dir = workflow_run_dir.join("phases"); + if phases_dir.is_dir() { + for entry in fs::read_dir(&phases_dir).with_context(|| format!("read phases dir {}", phases_dir.display()))? { + let path = entry?.path(); + if !path.file_name().and_then(|n| n.to_str()).is_some_and(|n| n.ends_with(".session.json")) { + continue; + } + if let Ok(Some(checkpoint)) = phase_session::read_path(&path) { + if checkpoint.phase_id == phase_id { + return Ok(checkpoint.run_id); + } + } + } + } + Err(not_found_error(format!("no run recorded for phase '{phase_id}' of workflow {workflow_id}"))) +} + fn extract_timestamp_hint(line: &str) -> Option { let parsed = serde_json::from_str::(line).ok()?; parsed @@ -277,7 +301,12 @@ pub(crate) fn get_phase_outputs( pub(crate) async fn handle_output(command: OutputCommand, project_root: &str, json: bool) -> Result<()> { match command { OutputCommand::Read(args) => { - let run_id = resolve_run_id_arg(project_root, args.run_id, args.workflow_id)?; + let run_id = match (&args.phase, &args.workflow_id) { + (Some(phase), Some(workflow_id)) => { + resolve_run_id_for_workflow_phase(project_root, workflow_id, phase)? + } + _ => resolve_run_id_arg(project_root, args.run_id, args.workflow_id)?, + }; let run_dir = resolve_run_dir_for_lookup(project_root, &run_id)? .ok_or_else(|| not_found_error(format!("run directory not found for {run_id}")))?; let events_path = run_dir.join("events.jsonl"); diff --git a/crates/orchestrator-cli/src/services/operations/ops_workflow/control_routing.rs b/crates/orchestrator-cli/src/services/operations/ops_workflow/control_routing.rs index c85d6a52..00f827ac 100644 --- a/crates/orchestrator-cli/src/services/operations/ops_workflow/control_routing.rs +++ b/crates/orchestrator-cli/src/services/operations/ops_workflow/control_routing.rs @@ -172,7 +172,8 @@ impl WorkflowRouting for WorkflowRoutingImpl { let query = WorkflowQuery { filter: WorkflowFilter { status: request.status.map(wire_status_to_core), - workflow_ref: None, + // v0.7.0-rc.7: the workflow-type filter is now wire-expressible. + workflow_ref: request.workflow_ref.clone(), task_id: None, phase_id: None, search_text: None, @@ -181,9 +182,10 @@ impl WorkflowRouting for WorkflowRoutingImpl { sort: Default::default(), }; let page: ListPage = hub.workflows().query(query).await.map_err(internal)?; + let total = page.total as u32; let runs: Vec = page.items.iter().map(workflow_summary_from_core).collect(); let next_cursor = page.next_offset.map(|n| n.to_string()); - Ok(WireListResponse { runs, next_cursor }) + Ok(WireListResponse { runs, next_cursor, total: Some(total) }) } async fn workflow_get(&self, request: WireGetRequest) -> Result { diff --git a/crates/orchestrator-cli/src/services/operations/ops_workflow/mod.rs b/crates/orchestrator-cli/src/services/operations/ops_workflow/mod.rs index 3e420dc9..a988512d 100644 --- a/crates/orchestrator-cli/src/services/operations/ops_workflow/mod.rs +++ b/crates/orchestrator-cli/src/services/operations/ops_workflow/mod.rs @@ -802,7 +802,9 @@ pub(crate) async fn handle_workflow( /// wire: the wire request only carries status/cursor/limit, has no /// sort parameter, and collapses `escalated` into `Failed`. fn workflow_list_expressible_on_wire(args: &crate::WorkflowListArgs) -> bool { - if args.workflow_ref.is_some() || args.subject_id.is_some() || args.phase_id.is_some() || args.search.is_some() { + // `workflow_ref` (the type filter) is wire-expressible as of control-protocol + // v0.7.0-rc.7; subject/phase/search filters still fall back to local. + if args.subject_id.is_some() || args.phase_id.is_some() || args.search.is_some() { return false; } match parse_workflow_query_sort_opt(args.sort.as_deref()) { @@ -833,7 +835,7 @@ async fn try_workflow_list_via_control( }; let limit = args.limit.map(|l| u32::try_from(l).unwrap_or(u32::MAX)); let cursor = if args.offset == 0 { None } else { Some(args.offset.to_string()) }; - let request = WireRequest { status, cursor, limit }; + let request = WireRequest { status, cursor, limit, workflow_ref: args.workflow_ref.clone() }; match client.workflow_list(request).await { Ok(response) => Ok(Some(response)), Err(err) if is_method_unavailable(&err) => { @@ -977,14 +979,15 @@ mod tests { let mut args = list_args(); args.sort = Some("started-at".to_string()); assert!(workflow_list_expressible_on_wire(&args), "default sort is expressible"); - } - #[test] - fn workflow_list_expressible_on_wire_gates_local_only_filters() { + // The workflow-type filter is wire-expressible as of control-protocol rc.7. let mut args = list_args(); args.workflow_ref = Some("standard".to_string()); - assert!(!workflow_list_expressible_on_wire(&args)); + assert!(workflow_list_expressible_on_wire(&args), "workflow_ref is expressible on the wire"); + } + #[test] + fn workflow_list_expressible_on_wire_gates_local_only_filters() { let mut args = list_args(); args.subject_id = Some("TASK-1".to_string()); assert!(!workflow_list_expressible_on_wire(&args)); diff --git a/crates/orchestrator-daemon-runtime/Cargo.toml b/crates/orchestrator-daemon-runtime/Cargo.toml index 1da23df1..133e255b 100644 --- a/crates/orchestrator-daemon-runtime/Cargo.toml +++ b/crates/orchestrator-daemon-runtime/Cargo.toml @@ -28,12 +28,12 @@ animus-config-protocol = { workspace = true } # All animus-protocol git deps pin the SAME tag (v0.1.26) so the transitively # pulled `animus-subject-protocol` resolves to ONE source rev. v0.1.26 additively # adds `actor: Option` to the control request types relayed by the daemon. -animus-control-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.6" } -animus-log-storage-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.6" } +animus-control-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.7" } +animus-log-storage-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.7" } animus-actor = { workspace = true } # Subject / SubjectChangedEvent share the same v0.1.26 surface as the # control-protocol (one source rev so the protocol versions stay in lockstep). -animus-subject-protocol-wire = { package = "animus-subject-protocol", git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.6" } +animus-subject-protocol-wire = { package = "animus-subject-protocol", git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.7" } protocol = { workspace = true } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" @@ -52,14 +52,14 @@ interprocess = { version = "2.4", features = ["tokio"] } protocol = { workspace = true, features = ["test-utils"] } tempfile = "3" tokio = { version = "1", features = ["io-util", "macros", "process", "rt", "signal", "test-util", "time"] } -animus-control-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.6", features = ["client"] } +animus-control-protocol = { git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.7", features = ["client"] } async-trait = "0.1" chrono = { version = "0.4", features = ["serde"] } futures-core = "0.3" futures-util = "0.3" serde_json = "1.0" uuid = { version = "1", features = ["v4"] } -animus-subject-protocol-wire = { package = "animus-subject-protocol", git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.6" } +animus-subject-protocol-wire = { package = "animus-subject-protocol", git = "https://github.com/launchapp-dev/animus-protocol", tag = "v0.7.0-rc.7" } animus-runtime-shared = { workspace = true } [lints] From f8ef72812ef1e696c0e7e672906978f0a385f31e Mon Sep 17 00:00:00 2001 From: Shooksie Date: Wed, 15 Jul 2026 20:42:23 -0600 Subject: [PATCH 11/15] fix(workflow): accurate list total past 1000 runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The numbered-pager total came from query_ids -> ids.len(), and journal_client sent limit: None, which the reference backend defaults to 1000 — so a board with >1000 runs reported total 1000 and the pager capped at 20 pages. Request the backend's max (10_000) for the id/count query (ids are cheap, no blobs) so the total is accurate up to that ceiling. Bumps orchestrator-cli to rc.20. --- Cargo.lock | 2 +- crates/orchestrator-cli/Cargo.toml | 2 +- crates/orchestrator-core/src/workflow/journal_client.rs | 9 ++++++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 98d2544d..acdfb40e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2782,7 +2782,7 @@ checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" [[package]] name = "orchestrator-cli" -version = "0.7.0-rc.19" +version = "0.7.0-rc.20" dependencies = [ "animus-actor", "animus-control-protocol", diff --git a/crates/orchestrator-cli/Cargo.toml b/crates/orchestrator-cli/Cargo.toml index a6d826bb..0c8329f0 100644 --- a/crates/orchestrator-cli/Cargo.toml +++ b/crates/orchestrator-cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "orchestrator-cli" -version = "0.7.0-rc.19" +version = "0.7.0-rc.20" edition = "2021" license = "Elastic-2.0" default-run = "animus" diff --git a/crates/orchestrator-core/src/workflow/journal_client.rs b/crates/orchestrator-core/src/workflow/journal_client.rs index e8f5d14e..c330788b 100644 --- a/crates/orchestrator-core/src/workflow/journal_client.rs +++ b/crates/orchestrator-core/src/workflow/journal_client.rs @@ -581,6 +581,13 @@ fn parse_summary_ts(s: &str) -> Option> { chrono::DateTime::parse_from_rfc3339(s).ok().map(|dt| dt.with_timezone(&chrono::Utc)) } +/// Ceiling on the id set `query_ids` pulls in one RPC. The caller derives the +/// list `total` from `ids.len()`, so an unset limit (which the reference backend +/// defaults to 1000) would cap the reported total — and the numbered pager — at +/// 1000 even with more runs. Request the backend's max so the count is accurate +/// up to this ceiling (ids are cheap — just the id column, no blobs). +const QUERY_IDS_LIMIT: u32 = 10_000; + /// All run ids matching `status` (None = all). The caller paginates client-side. pub(crate) fn query_ids( plugin: &DiscoveredPlugin, @@ -591,7 +598,7 @@ pub(crate) fn query_ids( status: status.map(|s| vec![status_wire(s).to_string()]).unwrap_or_default(), workflow_ref: None, updated_since: None, - limit: None, + limit: Some(QUERY_IDS_LIMIT), }; let value = run_blocking(call(plugin, project_root, METHOD_JOURNAL_QUERY_IDS, query))??; let resp: QueryIdsResult = serde_json::from_value(value).context("decoding journal QueryIdsResult")?; From e17a3fa3cda99fbc5df73dd11e52ead4288589b3 Mon Sep 17 00:00:00 2001 From: Shooksie Date: Wed, 15 Jul 2026 20:47:55 -0600 Subject: [PATCH 12/15] perf(workflow): push workflow_ref (type) filter to the fast DB-page path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Filtering the /workflows list by type was slow: workflow_query_can_use_db_page excluded workflow_ref, so a filtered query fell back to list_all() — fetching every run with full blobs and filtering in memory (~6s). The journal backend's list/query_ids already support a workflow_ref filter, so thread it through list_page + query_ids and let workflow_ref stay on the bounded DB-page path. Filtered lists are now server-side bounded + accurate-total, same as unfiltered. Folded into rc.20. --- .../src/services/workflow_impl.rs | 11 +++++--- .../src/workflow/journal_client.rs | 6 ++-- .../src/workflow/state_manager.rs | 28 +++++++++++++++---- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/crates/orchestrator-core/src/services/workflow_impl.rs b/crates/orchestrator-core/src/services/workflow_impl.rs index e0e5e1e0..1d2fbdc7 100644 --- a/crates/orchestrator-core/src/services/workflow_impl.rs +++ b/crates/orchestrator-core/src/services/workflow_impl.rs @@ -149,9 +149,11 @@ fn query_workflows(workflows: Vec, query: &WorkflowQuery) } fn workflow_query_can_use_db_page(query: &WorkflowQuery) -> bool { + // `workflow_ref` (the type filter) is pushed down to the journal backend's + // bounded list/query_ids, so it stays on the fast DB-page path; task_id / + // phase_id / search still fall back to the in-memory scan. query.page.limit.is_some() && matches!(query.sort, WorkflowQuerySort::StartedAt) - && query.filter.workflow_ref.is_none() && query.filter.task_id.is_none() && query.filter.phase_id.is_none() && query.filter.search_text.as_deref().is_none_or(|value| value.trim().is_empty()) @@ -398,15 +400,16 @@ impl WorkflowServiceApi for FileServiceHub { // query for `total`, instead of query_ids + a per-id load loop (N+1 — // one RPC per run, which serialized behind other journal traffic and // made the list intermittently multi-second). + let workflow_ref = query.filter.workflow_ref.as_deref(); if query.page.offset == 0 { if let Some(limit) = query.page.limit { - let (_ids, total) = manager.query_ids(query.page, query.filter.status)?; - let items = manager.list_page(query.filter.status, limit)?; + let (_ids, total) = manager.query_ids(query.page, query.filter.status, workflow_ref)?; + let items = manager.list_page(query.filter.status, workflow_ref, limit)?; return Ok(ListPage::new(items, total, query.page)); } } // Offset > 0: the journal list RPC has no offset, so keep the id-walk. - let (ids, total) = manager.query_ids(query.page, query.filter.status)?; + let (ids, total) = manager.query_ids(query.page, query.filter.status, workflow_ref)?; let mut items = Vec::with_capacity(ids.len()); for id in ids { if let Ok(workflow) = manager.load(&id) { diff --git a/crates/orchestrator-core/src/workflow/journal_client.rs b/crates/orchestrator-core/src/workflow/journal_client.rs index c330788b..df29d6b4 100644 --- a/crates/orchestrator-core/src/workflow/journal_client.rs +++ b/crates/orchestrator-core/src/workflow/journal_client.rs @@ -477,11 +477,12 @@ pub(crate) fn list_page( plugin: &DiscoveredPlugin, project_root: &Path, status: Option, + workflow_ref: Option<&str>, limit: usize, ) -> Result> { let query = JournalQuery { status: status.map(|s| vec![status_wire(s).to_string()]).unwrap_or_default(), - workflow_ref: None, + workflow_ref: workflow_ref.map(str::to_string), updated_since: None, limit: Some(limit as u32), }; @@ -593,10 +594,11 @@ pub(crate) fn query_ids( plugin: &DiscoveredPlugin, project_root: &Path, status: Option, + workflow_ref: Option<&str>, ) -> Result> { let query = JournalQuery { status: status.map(|s| vec![status_wire(s).to_string()]).unwrap_or_default(), - workflow_ref: None, + workflow_ref: workflow_ref.map(str::to_string), updated_since: None, limit: Some(QUERY_IDS_LIMIT), }; diff --git a/crates/orchestrator-core/src/workflow/state_manager.rs b/crates/orchestrator-core/src/workflow/state_manager.rs index 72831d28..6bd1a5eb 100644 --- a/crates/orchestrator-core/src/workflow/state_manager.rs +++ b/crates/orchestrator-core/src/workflow/state_manager.rs @@ -368,13 +368,17 @@ impl WorkflowStateManager { pub fn list_page( &self, status: Option, + workflow_ref: Option<&str>, limit: usize, ) -> Result> { if let Some(plugin) = self.journal_plugin() { - return super::journal_client::list_page(plugin, &self.project_root, status, limit); + return super::journal_client::list_page(plugin, &self.project_root, status, workflow_ref, limit); } - // SQLite dev fallback: fetch all and truncate (no plugin round-trips). + // SQLite dev fallback: fetch all, filter by ref, truncate (no plugin round-trips). let mut all = self.list_all()?; + if let Some(wref) = workflow_ref { + all.retain(|w| w.workflow_ref.as_deref() == Some(wref)); + } all.truncate(limit); Ok(all) } @@ -476,17 +480,31 @@ impl WorkflowStateManager { &self, page: ListPageRequest, status: Option, + workflow_ref: Option<&str>, ) -> Result<(Vec, usize)> { if let Some(plugin) = self.journal_plugin() { // The journal protocol's query_ids has no offset/total, so fetch the - // full matching id set from the plugin and paginate client-side to - // preserve the (page, total) contract. - let all_ids = super::journal_client::query_ids(plugin, &self.project_root, status)?; + // full matching id set from the plugin (status + workflow_ref filtered + // server-side) and paginate client-side to preserve the (page, total) + // contract. + let all_ids = super::journal_client::query_ids(plugin, &self.project_root, status, workflow_ref)?; let total = all_ids.len(); let (start, end) = page.bounds(total); let ids = all_ids.into_iter().skip(start).take(end.saturating_sub(start)).collect(); return Ok((ids, total)); } + // SQLite dev fallback: a workflow_ref filter is applied in memory (small + // local store) to avoid a combinatorial SQL branch; the plugin path above + // is the production one. + if let Some(wref) = workflow_ref { + let mut runs = self.list_all()?; + runs.retain(|w| status.is_none_or(|s| w.status == s) && w.workflow_ref.as_deref() == Some(wref)); + runs.sort_by(|a, b| b.started_at.cmp(&a.started_at).then_with(|| a.id.cmp(&b.id))); + let total = runs.len(); + let (start, end) = page.bounds(total); + let ids = runs[start..end].iter().map(|w| w.id.clone()).collect(); + return Ok((ids, total)); + } let conn = self.open_db()?; let total: usize = match status { From 31fd05c187279734da5176feccbedf790b52e127 Mon Sep 17 00:00:00 2001 From: Shooksie Date: Fri, 17 Jul 2026 11:28:00 -0600 Subject: [PATCH 13/15] fix(daemon): normalize bare-vs-qualified subject-id in orphan-reconciliation A remote-animus (REQ-052) node runs the delegated workflow standalone and journals its run's subject UPSTREAM in the BARE form (TASK-1), while the delegating runner registers that subject QUALIFIED (task:TASK-1) in active_subject_ids. The orphan-reconciliation exclusion checks compared the two raw forms, never matched, and misclassified the node's own Running row as a resumable orphan mid-session -> re-dispatched it every tick -> spawned a duplicate ephemeral node until dispatch was paused (one enqueue -> N nodes). Add bare_subject_id() and normalize both sides before the exclusion checks in recover_orphaned_running_workflows and resumable_orphans_for_redispatch (including the live-orphan guard). Adds a unit test. --- .../runtime_daemon/daemon_reconciliation.rs | 50 +++++++++++++++++-- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_reconciliation.rs b/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_reconciliation.rs index 82187533..07a05c35 100644 --- a/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_reconciliation.rs +++ b/crates/orchestrator-cli/src/services/runtime/runtime_daemon/daemon_reconciliation.rs @@ -70,6 +70,21 @@ fn is_resumable_orphan(workflow: &OrchestratorWorkflow) -> bool { workflow.current_phase.is_some() || workflow.phases.get(workflow.current_phase_index).is_some() } +/// Normalize a subject id to its BARE form by stripping a leading `kind:` +/// prefix (`task:TASK-1` -> `TASK-1`; a bare `TASK-1` is unchanged). +/// +/// A remote-animus (REQ-052) node runs the delegated workflow standalone and +/// journals its run's subject UPSTREAM into this daemon's journal in the BARE +/// form, while the delegating runner registers that subject QUALIFIED in +/// `active_subject_ids`. Comparing the two raw forms never matches, so the +/// node's own `Running` row is misclassified as a resumable orphan mid-session +/// and re-dispatched — spawning a duplicate ephemeral node every reconciliation +/// tick until dispatch is paused. Normalizing BOTH sides to bare before the +/// exclusion checks closes that fan-out loop. +fn bare_subject_id(id: &str) -> &str { + id.split_once(':').map_or(id, |(_, rest)| rest) +} + pub async fn recover_orphaned_running_workflows( hub: Arc, project_root: &str, @@ -99,6 +114,9 @@ pub async fn recover_orphaned_running_workflows( } }; let now = chrono::Utc::now(); + // Compare a node's BARE upstream-journaled subject against the delegating + // runner's QUALIFIED active id (see `bare_subject_id`). + let active_subject_bare: HashSet<&str> = active_subject_ids.iter().map(|s| bare_subject_id(s)).collect(); let mut recovered = 0usize; for workflow in workflows { @@ -113,7 +131,7 @@ pub async fn recover_orphaned_running_workflows( } if active_subject_ids.contains(&workflow.id) || externally_active_workflows.contains(&workflow.id) - || workflow.subject.as_ref().is_some_and(|s| active_subject_ids.contains(s.id())) + || workflow.subject.as_ref().is_some_and(|s| active_subject_bare.contains(bare_subject_id(s.id()))) { continue; } @@ -267,6 +285,10 @@ pub(crate) async fn resumable_orphans_for_redispatch( None => HashSet::new(), }; let now = chrono::Utc::now(); + // Normalize to BARE form so a node's upstream-journaled subject matches the + // delegating runner's QUALIFIED active/orphan ids (see `bare_subject_id`). + let active_subject_bare: HashSet<&str> = active_subject_ids.iter().map(|s| bare_subject_id(s)).collect(); + let live_orphan_bare: HashSet<&str> = live_orphan_subjects.iter().map(|s| bare_subject_id(s)).collect(); let mut candidates = Vec::new(); for workflow in workflows { @@ -281,14 +303,14 @@ pub(crate) async fn resumable_orphans_for_redispatch( } if active_subject_ids.contains(&workflow.id) || externally_active_workflows.contains(&workflow.id) - || workflow.subject.as_ref().is_some_and(|s| active_subject_ids.contains(s.id())) + || workflow.subject.as_ref().is_some_and(|s| active_subject_bare.contains(bare_subject_id(s.id()))) { continue; } // Skip subjects whose detached runner from a previous daemon is still // alive (see `live_orphan_subjects` above). - if workflow.subject.as_ref().is_some_and(|s| live_orphan_subjects.contains(s.id())) - || (!workflow.task_id.is_empty() && live_orphan_subjects.contains(&workflow.task_id)) + if workflow.subject.as_ref().is_some_and(|s| live_orphan_bare.contains(bare_subject_id(s.id()))) + || (!workflow.task_id.is_empty() && live_orphan_bare.contains(bare_subject_id(&workflow.task_id))) { continue; } @@ -449,6 +471,26 @@ mod tests { assert!(commit.success(), "initial commit should succeed"); } + // REQ-052 fan-out fix: a delegating runner registers a QUALIFIED subject id + // (`task:TASK-1`) in `active_subject_ids`, while the remote node journals + // the same run's subject UPSTREAM in the BARE form (`TASK-1`). The orphan + // sweep's exclusion checks now normalize BOTH sides via `bare_subject_id` + // so they match; without it the node's own Running row was re-dispatched + // every tick, spawning a duplicate node. + #[test] + fn bare_subject_id_strips_leading_kind_prefix() { + use super::bare_subject_id; + assert_eq!(bare_subject_id("task:TASK-632"), "TASK-632"); + assert_eq!(bare_subject_id("TASK-632"), "TASK-632"); // already bare — unchanged + assert_eq!(bare_subject_id("requirement:REQ-1"), "REQ-1"); + assert_eq!(bare_subject_id("transcript:TRANSCRIPT-001"), "TRANSCRIPT-001"); + // only the FIRST ':' is the kind separator; the remainder is preserved + assert_eq!(bare_subject_id("task:weird:id"), "weird:id"); + assert_eq!(bare_subject_id(""), ""); + // the qualified/bare pair that caused the fan-out must normalize equal + assert_eq!(bare_subject_id("task:TASK-632"), bare_subject_id("TASK-632")); + } + #[tokio::test] async fn registered_runner_pid_shields_old_running_workflow_from_orphan_cancel() { let _lock = test_env_lock().lock().unwrap_or_else(|p| p.into_inner()); From 28bc167f6de619dbedf90f850cfe8c19279ae008 Mon Sep 17 00:00:00 2001 From: Shooksie Date: Fri, 17 Jul 2026 11:40:03 -0600 Subject: [PATCH 14/15] chore(release): bump orchestrator-cli to v0.7.0-rc.22 --- crates/orchestrator-cli/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/orchestrator-cli/Cargo.toml b/crates/orchestrator-cli/Cargo.toml index 0c8329f0..e30fe7e5 100644 --- a/crates/orchestrator-cli/Cargo.toml +++ b/crates/orchestrator-cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "orchestrator-cli" -version = "0.7.0-rc.20" +version = "0.7.0-rc.22" edition = "2021" license = "Elastic-2.0" default-run = "animus" From 88bf3fad156b100882edee509efe68d932df2dd2 Mon Sep 17 00:00:00 2001 From: Shooksie Date: Fri, 17 Jul 2026 11:43:59 -0600 Subject: [PATCH 15/15] chore(release): bump orchestrator-cli to v0.7.0-rc.23 + refresh Cargo.lock --- Cargo.lock | 2 +- crates/orchestrator-cli/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index acdfb40e..8afd8047 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2782,7 +2782,7 @@ checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" [[package]] name = "orchestrator-cli" -version = "0.7.0-rc.20" +version = "0.7.0-rc.23" dependencies = [ "animus-actor", "animus-control-protocol", diff --git a/crates/orchestrator-cli/Cargo.toml b/crates/orchestrator-cli/Cargo.toml index e30fe7e5..f91b6b89 100644 --- a/crates/orchestrator-cli/Cargo.toml +++ b/crates/orchestrator-cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "orchestrator-cli" -version = "0.7.0-rc.22" +version = "0.7.0-rc.23" edition = "2021" license = "Elastic-2.0" default-run = "animus"