From dbc9b8d225c961f528cde6505b40177b75c47905 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Sun, 25 Oct 2026 19:35:35 +0100 Subject: [PATCH 01/13] mason: retain trusted bind config content and source provenance (#891) Capture user/project observations in the selected harness's order, rejecting unknown or oversized reads instead of selecting a lower-priority file. Migration rereads use the same byte limit. Pass the immutable local snapshot through a scoped internal binding on every repeatable configure attempt; wire JSON cannot supply that scope. Prepared configure reuses content, selected source paths, migration outcome and legacy notice without file reselection. Normal configure and cache recovery remain separate. Correct the comments that conflated AFT's retained call records with SUBC preserving a client route handle. The focused two-core compiler check, two prepared-bind tests and thirteen local-reader tests pass. Independent restored-baseline/source/notice controls are pending remote admission and will be reported with this review chunk. Config observation is still at the existing bind admission site until the later ticket/preflight mechanism; this dependency commit is not a disk-free-loop claim. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- .../release-notes/next/a0-prepared-config.md | 1 + crates/aft/src/commands/configure.rs | 143 ++++++++-- crates/aft/src/config_fix.rs | 46 ++- crates/aft/src/config_live.rs | 15 +- crates/aft/src/subc/mod.rs | 57 +++- crates/aft/src/subc_config.rs | 26 +- crates/aft/src/subc_config/prepared.rs | 264 ++++++++++++++++++ 7 files changed, 504 insertions(+), 48 deletions(-) create mode 100644 .alfonso/release-notes/next/a0-prepared-config.md create mode 100644 crates/aft/src/subc_config/prepared.rs diff --git a/.alfonso/release-notes/next/a0-prepared-config.md b/.alfonso/release-notes/next/a0-prepared-config.md new file mode 100644 index 0000000000..36abf0e6bc --- /dev/null +++ b/.alfonso/release-notes/next/a0-prepared-config.md @@ -0,0 +1 @@ +- Bind configuration, source tracking and legacy-location notices now use one bounded local snapshot for the bind's selected harness, without rereading config during configure retries. diff --git a/crates/aft/src/commands/configure.rs b/crates/aft/src/commands/configure.rs index 22f703d3af..2bd138a8b9 100644 --- a/crates/aft/src/commands/configure.rs +++ b/crates/aft/src/commands/configure.rs @@ -3687,9 +3687,26 @@ fn handle_configure_inner( // already holds this request's resolved tiers, so nothing is re-resolved // and the recorded config sources are left as they are. let recovery = prepared.is_some(); + let bind_config = if recovery { + None + } else { + crate::subc_config::current_prepared_bind_config() + }; + if bind_config + .as_ref() + .is_some_and(|snapshot| !snapshot.matches(&root_path, &harness)) + { + return Response::error( + &req.id, + "config_unknown", + "prepared local configuration does not match this bind's root and harness", + ); + } ctx.begin_configure_ack_step("tier_file_reads"); let (tiers, user_config_migration) = if recovery { (Vec::new(), None) + } else if let Some(snapshot) = bind_config.as_ref() { + (snapshot.tiers().to_vec(), snapshot.migration()) } else { match resolve_config_tiers_for_configure(params, &root_path, Some(&harness)) { Ok(read) => read, @@ -3721,14 +3738,17 @@ fn handle_configure_inner( } ctx.begin_configure_ack_step("source_tracking"); if !recovery { - crate::config_live::stage_configure_sources( + let sources = if let Some(snapshot) = bind_config.as_ref() { + snapshot.sources(&next_config) + } else { crate::config_live::ConfigSources::from_configure( parse_cortexkit_user_config_path(params).ok().flatten(), &root_path, &tiers, &next_config, - ), - ); + ) + }; + crate::config_live::stage_configure_sources(sources); } ctx.begin_configure_ack_step("validate_params"); let mut configure_warnings = config_diagnostics @@ -3759,22 +3779,27 @@ fn handle_configure_inner( "message": migration.notice(), })); } - if let Some(project) = find_config_tier(&tiers, "project") { - if let Some(notice) = crate::subc_config::legacy_project_config_location_notice( - &root_path, - Path::new(&project.source), - ) - .filter(|_| { - crate::subc_config::project_config_read_path(&root_path, Some(&harness)) - == Path::new(&project.source) - }) { - configure_warnings.push(json!({ - "code": "legacy_project_config_location", - "tier": "project", - "hint": notice, - "message": notice, - })); - } + let legacy_notice = if let Some(snapshot) = bind_config.as_ref() { + snapshot.legacy_notice() + } else { + find_config_tier(&tiers, "project").and_then(|project| { + crate::subc_config::legacy_project_config_location_notice( + &root_path, + Path::new(&project.source), + ) + .filter(|_| { + crate::subc_config::project_config_read_path(&root_path, Some(&harness)) + == Path::new(&project.source) + }) + }) + }; + if let Some(notice) = legacy_notice { + configure_warnings.push(json!({ + "code": "legacy_project_config_location", + "tier": "project", + "hint": notice, + "message": notice, + })); } // NO configure-time SSRF guard on semantic.base_url — deliberate (config @@ -16619,6 +16644,86 @@ mod tests { assert_eq!(fs::read_to_string(&project_path).unwrap(), project_text); } + #[test] + fn prepared_bind_branches_do_not_reselect_config_files() { + let source = include_str!("configure.rs"); + for marker in [ + "else if let Some(snapshot) = bind_config.as_ref() {", + "let sources = if let Some(snapshot) = bind_config.as_ref() {", + "let legacy_notice = if let Some(snapshot) = bind_config.as_ref() {", + ] { + let body = source + .split_once(marker) + .expect("prepared branch exists") + .1 + .split_once("} else") + .unwrap() + .0; + assert!(body.contains("snapshot.")); + for forbidden in [ + "resolve_config_tiers_for_configure", + "from_configure", + "project_config_read_path", + ".exists(", + "std::fs::", + ] { + assert!( + !body.contains(forbidden), + "prepared configure re-observes files: {forbidden}" + ); + } + } + } + + #[test] + fn prepared_bind_config_keeps_content_sources_and_notice_across_attempts() { + let temp = tempfile::tempdir().unwrap(); + let ctx = test_context(); + let user = temp.path().join("user.jsonc"); + let project = temp.path().join(".pi/aft.jsonc"); + let canonical = temp.path().join(".cortexkit/aft.jsonc"); + write_config(&user, r#"{"url_fetch_allow_private":true}"#); + write_config(&project, r#"{"callgraph_chunk_size":7}"#); + write_config( + &temp.path().join(".opencode/aft.jsonc"), + r#"{"callgraph_chunk_size":8}"#, + ); + let snapshot = crate::subc_config::PreparedBindConfig::read( + Some(&user), + temp.path(), + &crate::harness::Harness::Pi, + ) + .unwrap(); + write_config(&user, r#"{"url_fetch_allow_private":false}"#); + write_config(&canonical, r#"{"callgraph_chunk_size":9}"#); + fs::remove_file(&project).unwrap(); + let request = configure_request_with_params(json!({ + "project_root": temp.path(), "harness":"pi", "cortexkit_user_config_path":user, + })); + for _ in 0..2 { + let response = + crate::subc_config::with_prepared_bind_config(Arc::clone(&snapshot), || { + handle_configure_for_test(&request, &ctx) + }); + assert!(response.success, "{:?}", response.data); + assert_eq!(ctx.config().callgraph_chunk_size, 7); + assert!(ctx.config().url_fetch_allow_private); + let sources = ctx.config_live().sources().expect("applied sources"); + assert_eq!(sources.project.path.as_deref(), Some(project.as_path())); + assert!(sources.project.from_file); + assert_eq!( + sources.project.text.as_deref(), + Some(r#"{"callgraph_chunk_size":7}"#) + ); + assert!(response.data["warnings"] + .as_array() + .unwrap() + .iter() + .any(|warning| warning["code"] == "legacy_project_config_location")); + } + assert!(crate::subc_config::current_prepared_bind_config().is_none()); + } + #[test] fn configure_legacy_project_file_is_read_in_place_and_reports_its_location() { for harness in ["opencode", "pi"] { diff --git a/crates/aft/src/config_fix.rs b/crates/aft/src/config_fix.rs index c7615e8cb5..b0414ab84d 100644 --- a/crates/aft/src/config_fix.rs +++ b/crates/aft/src/config_fix.rs @@ -600,7 +600,49 @@ fn auto_migrate_user_config_with_writer( path: &Path, write: impl FnOnce(&Path, &str) -> std::io::Result>, ) -> Option { - let text = std::fs::read_to_string(path).ok()?; + auto_migrate_user_config_with_reader(path, write, |path| std::fs::read_to_string(path)) +} + +pub(crate) fn auto_migrate_user_config_bounded( + path: &Path, + limit: usize, +) -> std::io::Result> { + use std::io::Read; + let mut failure = None; + let result = auto_migrate_user_config_with_reader( + path, + jsonc_edit::write_atomic_with_sync_warning, + |path| { + let read = (|| { + let file = std::fs::File::open(path)?; + let mut bytes = Vec::new(); + file.take((limit + 1) as u64).read_to_end(&mut bytes)?; + if bytes.len() > limit { + return Err(std::io::Error::other( + "configuration exceeds migration byte limit", + )); + } + String::from_utf8(bytes) + .map_err(|error| std::io::Error::new(std::io::ErrorKind::InvalidData, error)) + })(); + if let Err(error) = &read { + failure = Some(std::io::Error::new(error.kind(), error.to_string())); + } + read + }, + ); + match failure { + Some(error) => Err(error), + None => Ok(result), + } +} + +fn auto_migrate_user_config_with_reader( + path: &Path, + write: impl FnOnce(&Path, &str) -> std::io::Result>, + mut read: impl FnMut(&Path) -> std::io::Result, +) -> Option { + let text = read(path).ok()?; let translation = retired_key_translation(&text)?; let display = path.display().to_string(); let not_migrated = |reason: String, translation: &feature_config::DocumentTranslation| { @@ -632,7 +674,7 @@ fn auto_migrate_user_config_with_writer( }; // Re-read under the lock: another process may have finished meanwhile. - let text = match std::fs::read_to_string(&target) { + let text = match read(&target) { Ok(text) => text, Err(error) => return not_migrated(format!("could not read it: {error}"), &translation), }; diff --git a/crates/aft/src/config_live.rs b/crates/aft/src/config_live.rs index 65f26f5ecd..934d8f2add 100644 --- a/crates/aft/src/config_live.rs +++ b/crates/aft/src/config_live.rs @@ -133,10 +133,21 @@ impl ConfigSources { tiers: &[ConfigTier], connected: &Config, ) -> Self { - let owns_user_watch = user_config_path.is_some() && process_user_config_path().is_none(); - let user_path = user_config_path.or_else(process_user_config_path); let project_path = crate::subc_config::project_config_read_path(project_root, connected.harness.as_ref()); + Self::from_observed_paths(user_config_path, project_path, tiers, connected) + } + + /// Record the source paths selected by a trusted local observation without + /// probing them again. Reload must watch the same files that supplied tiers. + pub(crate) fn from_observed_paths( + user_config_path: Option, + project_path: PathBuf, + tiers: &[ConfigTier], + connected: &Config, + ) -> Self { + let owns_user_watch = user_config_path.is_some() && process_user_config_path().is_none(); + let user_path = user_config_path.or_else(process_user_config_path); let source_for = |tier: &'static str, path: Option| { let used = tiers.iter().find(|candidate| candidate.tier == tier); let from_file = match (used, path.as_ref()) { diff --git a/crates/aft/src/subc/mod.rs b/crates/aft/src/subc/mod.rs index 447f8f8ac8..85e42a1202 100644 --- a/crates/aft/src/subc/mod.rs +++ b/crates/aft/src/subc/mod.rs @@ -3769,8 +3769,9 @@ async fn teardown_installed_route( lifecycle_probe: Option<&SubcTestLifecycleProbe>, ) -> Result<(), SubcError> { // Only the consumer's own close withdraws held permission asks. A - // higher-epoch rebind keeps the logical route, and a transport drop never - // reaches here, so their asks stay open for the retried call. + // higher-epoch allocation is a new daemon route/client handle. AFT retains + // scoped permission work by its call identity, not by that old route handle. + // A transport drop never reaches here. if cancellation_reason == "Goodbye" { consent::note_route_closed(channel); } @@ -3828,8 +3829,9 @@ async fn teardown_installed_route( ) .await?; } - // A higher-epoch replacement keeps replayable work because the logical - // route remains live. A route that actually closes keeps only calls that + // AFT keeps registered native work across higher-epoch replacement using + // its retained call/job records; the daemon does not preserve the old handle. + // An explicit close keeps only calls that // already started; pre-execution work has no response to replay and must // not retain scheduler capacity or an epoch-reader admission. let work_disposition = if replacement_root.is_some() { @@ -6986,6 +6988,7 @@ fn route_bind_error_code_for_configure_response(response: &Response) -> &'static // contention); the client retries the bind rather than treating the // root as permanently divergent. Some("cache_key_probe_failed") => "cache_key_probe_failed", + Some("config_unknown") => "config_unknown", // Actor lifecycle gaps are transient from the daemon/client viewpoint: // a fresh bind can create or join a healthy actor, so do not classify // them as permanent config divergence. @@ -7900,22 +7903,40 @@ async fn handle_control_request( // The resolver selects only this bind's harness override before it // applies the unchanged user/project trust boundary. metrics.mark_frame_loop_wait("disk/local_config", None); - let local_tiers = crate::subc_config::read_local_cortexkit_config_tiers( - user_config_path, - Path::new(&bind_project_root), - ); + // Invalid harnesses still reach configure's existing typed refusal. + let active_harness = bind_harness.parse::().ok(); + let prepared_config = match active_harness.as_ref() { + Some(harness) => match crate::subc_config::PreparedBindConfig::read( + user_config_path, + Path::new(&bind_project_root), + harness, + ) { + Ok(snapshot) => Some(snapshot), + Err(error) => { + let code = if error.code == "config_rejected" { + "config_divergence" + } else { + error.code + }; + return send_route_bind_error(tx, frame, code, &error.to_string(), metrics) + .await; + } + }, + None => None, + }; + let local_tiers = prepared_config + .as_ref() + .map_or(&[][..], |snapshot| snapshot.tiers()); let config_tiers: Vec = local_tiers .iter() .map(|t| json!({ "tier": t.tier, "source": t.source, "doc": t.doc })) .collect(); // Let configure return its structured invalid-harness error rather // than panicking while computing this optional registration setting. - let active_harness = bind_harness.parse::().ok(); - let bind_config = crate::config_resolve::resolve_config_for_harness( - &local_tiers, - active_harness.as_ref(), - ) - .config; + let bind_config = prepared_config + .as_ref() + .map(|snapshot| snapshot.resolved().clone()) + .unwrap_or_default(); let diagnostics_on_edit = bind_config.diagnostics_on_edit; let bind_disabled_tools = Arc::new(bind_config.disabled_tools); let configure_json = json!({ @@ -8005,7 +8026,13 @@ async fn handle_control_request( Arc::new(move |ctx| { log_ctx::with_session(Some(configure_session.clone()), || { let bind_started = Instant::now(); - let response = dispatch(configure_req.clone(), ctx); + let response = match prepared_config.as_ref() { + Some(snapshot) => crate::subc_config::with_prepared_bind_config( + Arc::clone(snapshot), + || dispatch(configure_req.clone(), ctx), + ), + None => dispatch(configure_req.clone(), ctx), + }; if response.success { // Revoke matching standing publication before the // session is installed, but never on the frame loop. diff --git a/crates/aft/src/subc_config.rs b/crates/aft/src/subc_config.rs index 69a4587d65..6683f13b6b 100644 --- a/crates/aft/src/subc_config.rs +++ b/crates/aft/src/subc_config.rs @@ -18,6 +18,11 @@ use crate::harness::Harness; use std::ffi::OsStr; use std::path::{Path, PathBuf}; +mod prepared; +pub(crate) use prepared::{ + current_prepared_bind_config, with_prepared_bind_config, PreparedBindConfig, +}; + /// CortexKit user config home: `$XDG_CONFIG_HOME/cortexkit/aft.jsonc`, falling /// back to `~/.config/cortexkit/aft.jsonc`. Matches the shared CortexKit /// convention (`~/.config/cortexkit/.jsonc`) alongside `subc.jsonc` and @@ -64,11 +69,8 @@ fn cortexkit_project_config_path(project_root: &Path) -> PathBuf { /// Prefer the shared file, then the active harness's legacy files, without writes. /// Within a harness, aft.jsonc precedes aft.json. Without a plugin harness, /// OpenCode is checked before Pi, matching the TypeScript fallback ordering. -pub fn project_config_read_path(project_root: &Path, harness: Option<&Harness>) -> PathBuf { - let canonical = cortexkit_project_config_path(project_root); - if canonical.exists() { - return canonical; - } +fn project_config_candidates(project_root: &Path, harness: Option<&Harness>) -> Vec { + let mut candidates = vec![cortexkit_project_config_path(project_root)]; let directories = if matches!(harness, Some(Harness::Pi)) { [".pi", ".opencode"] } else { @@ -76,13 +78,17 @@ pub fn project_config_read_path(project_root: &Path, harness: Option<&Harness>) }; for directory in directories { for filename in ["aft.jsonc", "aft.json"] { - let path = project_root.join(directory).join(filename); - if path.exists() { - return path; - } + candidates.push(project_root.join(directory).join(filename)); } } - canonical + candidates +} + +pub fn project_config_read_path(project_root: &Path, harness: Option<&Harness>) -> PathBuf { + project_config_candidates(project_root, harness) + .into_iter() + .find(|path| path.exists()) + .unwrap_or_else(|| cortexkit_project_config_path(project_root)) } pub fn legacy_project_config_location_notice(project_root: &Path, path: &Path) -> Option { diff --git a/crates/aft/src/subc_config/prepared.rs b/crates/aft/src/subc_config/prepared.rs new file mode 100644 index 0000000000..4ccd0a2218 --- /dev/null +++ b/crates/aft/src/subc_config/prepared.rs @@ -0,0 +1,264 @@ +//! Trusted-local config observations retained across a bind's configure attempts. + +use super::*; +use std::cell::RefCell; +use std::io::Read; +use std::sync::Arc; + +const CONFIG_FILE_LIMIT: usize = 1024 * 1024; + +#[derive(Clone, Debug)] +pub(crate) enum ConfigObservation { + Present(ConfigTier), + Missing, +} + +#[derive(Debug)] +pub(crate) struct ConfigObservationError { + pub(crate) code: &'static str, + detail: String, +} + +impl std::fmt::Display for ConfigObservationError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.detail) + } +} + +#[derive(Clone, Debug)] +pub(crate) struct PreparedBindConfig { + root: PathBuf, + harness: Harness, + user_path: Option, + project_path: PathBuf, + tiers: Vec, + migration: Option, + resolved: crate::config::Config, +} + +impl PreparedBindConfig { + pub(crate) fn read( + user_path: Option<&Path>, + root: &Path, + harness: &Harness, + ) -> Result, ConfigObservationError> { + let mut observations = Vec::new(); + let mut migration = None; + if let Some(path) = user_path { + // Distinguish a missing user file from other read errors before + // migration. Migration's own rereads use the same byte limit. + let first = observe(path, "user")?; + if matches!(first, ConfigObservation::Present(_)) { + migration = + crate::config_fix::auto_migrate_user_config_bounded(path, CONFIG_FILE_LIMIT) + .map_err(|error| unknown(path, &error))?; + observations.push(observe(path, "user")?); + } else { + observations.push(first); + } + } + let mut project_path = cortexkit_project_config_path(root); + for candidate in super::project_config_candidates(root, Some(harness)) { + let observation = observe(&candidate, "project")?; + let present = matches!(observation, ConfigObservation::Present(_)); + if present { + project_path = candidate; + observations.push(observation); + break; + } + observations.push(observation); + } + let tiers: Vec<_> = observations + .iter() + .filter_map(|observation| match observation { + ConfigObservation::Present(tier) => Some(tier.clone()), + ConfigObservation::Missing => None, + }) + .collect(); + let resolution = crate::config_resolve::resolve_config_for_harness(&tiers, Some(harness)); + if !resolution.errors.is_empty() { + return Err(ConfigObservationError { + code: "config_rejected", + detail: format!("configuration rejected: {}", resolution.errors.join(", ")), + }); + } + Ok(Arc::new(Self { + root: root.into(), + harness: harness.clone(), + user_path: user_path.map(Path::to_path_buf), + project_path, + tiers, + migration, + resolved: resolution.config, + })) + } + + pub(crate) fn matches(&self, root: &Path, harness: &Harness) -> bool { + self.root == root && &self.harness == harness + } + + pub(crate) fn tiers(&self) -> &[ConfigTier] { + &self.tiers + } + pub(crate) fn migration(&self) -> Option { + self.migration.clone() + } + pub(crate) fn resolved(&self) -> &crate::config::Config { + &self.resolved + } + pub(crate) fn sources( + &self, + config: &crate::config::Config, + ) -> crate::config_live::ConfigSources { + crate::config_live::ConfigSources::from_observed_paths( + self.user_path.clone(), + self.project_path.clone(), + &self.tiers, + config, + ) + } + pub(crate) fn legacy_notice(&self) -> Option { + self.tiers + .iter() + .any(|tier| tier.tier == "project") + .then(|| legacy_project_config_location_notice(&self.root, &self.project_path)) + .flatten() + } +} + +fn unknown(path: &Path, error: &std::io::Error) -> ConfigObservationError { + ConfigObservationError { + code: "config_unknown", + detail: format!( + "could not observe configuration {}: {}", + path.display(), + error + ), + } +} + +fn observe(path: &Path, tier: &str) -> Result { + let file = match std::fs::File::open(path) { + Ok(file) => file, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + return Ok(ConfigObservation::Missing) + } + Err(error) => return Err(unknown(path, &error)), + }; + let mut bytes = Vec::new(); + file.take((CONFIG_FILE_LIMIT + 1) as u64) + .read_to_end(&mut bytes) + .map_err(|error| unknown(path, &error))?; + if bytes.len() > CONFIG_FILE_LIMIT { + return Err(unknown( + path, + &std::io::Error::other("configuration exceeds the 1 MiB observation limit"), + )); + } + let doc = String::from_utf8(bytes) + .map_err(|_| unknown(path, &std::io::Error::other("configuration is not UTF-8")))?; + Ok(ConfigObservation::Present(ConfigTier { + tier: tier.into(), + source: path.to_string_lossy().into_owned(), + doc, + })) +} + +thread_local! { + static PREPARED: RefCell>> = const { RefCell::new(None) }; +} + +pub(crate) fn current_prepared_bind_config() -> Option> { + PREPARED.with(|slot| slot.borrow().clone()) +} + +pub(crate) fn with_prepared_bind_config( + prepared: Arc, + run: impl FnOnce() -> T, +) -> T { + struct Restore(Option>); + impl Drop for Restore { + fn drop(&mut self) { + PREPARED.with(|slot| *slot.borrow_mut() = self.0.take()); + } + } + let _restore = Restore(PREPARED.with(|slot| slot.replace(Some(prepared)))); + run() +} + +#[cfg(test)] +mod tests { + use super::*; + + fn put(path: &Path, text: &str) { + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, text).unwrap(); + } + + #[test] + fn prepared_config_uses_active_harness_and_stops_on_unknown_candidate() { + let dir = tempfile::tempdir().unwrap(); + put( + &dir.path().join(".pi/aft.jsonc"), + r#"{"callgraph_chunk_size":7}"#, + ); + put( + &dir.path().join(".opencode/aft.jsonc"), + r#"{"callgraph_chunk_size":8}"#, + ); + assert_eq!( + PreparedBindConfig::read(None, dir.path(), &Harness::Pi) + .unwrap() + .resolved() + .callgraph_chunk_size, + 7 + ); + assert_eq!( + PreparedBindConfig::read(None, dir.path(), &Harness::Opencode) + .unwrap() + .resolved() + .callgraph_chunk_size, + 8 + ); + std::fs::create_dir_all(dir.path().join(".cortexkit/aft.jsonc")).unwrap(); + let error = PreparedBindConfig::read(None, dir.path(), &Harness::Pi).unwrap_err(); + assert_eq!( + error.code, "config_unknown", + "an unreadable canonical candidate must not select legacy config" + ); + } + + #[test] + fn prepared_config_bounds_reads_and_distinguishes_missing_from_invalid() { + let dir = tempfile::tempdir().unwrap(); + assert!(PreparedBindConfig::read(None, dir.path(), &Harness::Pi) + .unwrap() + .tiers() + .is_empty()); + let path = dir.path().join(".cortexkit/aft.jsonc"); + put(&path, &" ".repeat(CONFIG_FILE_LIMIT + 1)); + assert_eq!( + PreparedBindConfig::read(None, dir.path(), &Harness::Pi) + .unwrap_err() + .code, + "config_unknown" + ); + put(&path, "{ invalid config"); + assert_eq!( + PreparedBindConfig::read(None, dir.path(), &Harness::Pi) + .unwrap_err() + .code, + "config_rejected" + ); + } + + #[test] + fn prepared_config_scope_restores_on_unwind() { + let dir = tempfile::tempdir().unwrap(); + let snapshot = PreparedBindConfig::read(None, dir.path(), &Harness::Pi).unwrap(); + let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + with_prepared_bind_config(snapshot, || panic!("scope unwind")) + })); + assert!(current_prepared_bind_config().is_none()); + } +} From f44c4b86910ac7efee78b2ae2bc502e1fcdda8d6 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Sun, 25 Oct 2026 20:54:40 +0100 Subject: [PATCH 02/13] mason: publish A0 routes after raw-writer ACK receipts (#891) Keep control and management replies off the native call-ledger proxy. Split configure-ready from ACK-written publication: route maps, push replay and candidate dispatch remain unavailable until the actual writer finishes the ACK. Reserve completion capacity before exposing bytes so a fast peer cannot race an asynchronously scheduled publication event. Queue the ACK synchronously when writer capacity is available to preserve pre-turn bind priority. Bound outstanding publication waiters, use the existing actor_not_ready retry code on saturation, retract only ACKs not yet writing, and cancel queued publication at loop exit. Native tool replies and teardown still use their ledger path. Two-core focused checks pass: native compiler and fmt, six egress unit tests, the strengthened ACK-order test, scope-workdir and bind-log fixtures, plus management, reliable-flood priority, pre-ACK refusal, Goodbye-cancel and duplicate-bind integration tests. Early-receipt and post-proxy-sender deliberate breaks each redden only their named egress regression. Three independent prepared-config baseline/source/notice controls also redden only the snapshot regression; source was restored between controls and all positive filters passed. This is the second dependency mechanism, not completed A0: owners, intake tickets/deadlines, incumbent retirement and narrowed unknown-root guards follow after this chunk is reviewed. Existing inline lifecycle/finalizer/config-observation waits are not claimed eliminated. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- .../release-notes/next/a0-control-egress.md | 1 + crates/aft/src/subc/a0_egress.rs | 454 ++++++++++++++++++ crates/aft/src/subc/bind_log.rs | 2 + crates/aft/src/subc/mod.rs | 264 +++++++--- crates/aft/src/subc/scope_workdir_tests.rs | 36 +- crates/aft/src/subc/tool_provider.rs | 22 + crates/aft/src/subc/wire.rs | 67 ++- 7 files changed, 784 insertions(+), 62 deletions(-) create mode 100644 .alfonso/release-notes/next/a0-control-egress.md create mode 100644 crates/aft/src/subc/a0_egress.rs diff --git a/.alfonso/release-notes/next/a0-control-egress.md b/.alfonso/release-notes/next/a0-control-egress.md new file mode 100644 index 0000000000..a5588378f4 --- /dev/null +++ b/.alfonso/release-notes/next/a0-control-egress.md @@ -0,0 +1 @@ +- Prepared route-bind and management replies use a writer path independent of tool-outcome persistence, and routes and replayed events are published only after the bind acknowledgement is written. diff --git a/crates/aft/src/subc/a0_egress.rs b/crates/aft/src/subc/a0_egress.rs new file mode 100644 index 0000000000..7d6b2cecf6 --- /dev/null +++ b/crates/aft/src/subc/a0_egress.rs @@ -0,0 +1,454 @@ +//! Bind publication and control replies do not wait for native tool calls to +//! record their outcomes in the call ledger. + +use super::wire::WriterEnqueueOutcome; +use super::*; +use std::sync::atomic::AtomicU8; + +const QUEUED: u8 = 0; +const WRITING: u8 = 1; +const WRITTEN: u8 = 2; +const CANCELLED: u8 = 3; + +#[derive(Clone, Default, Debug)] +pub(super) struct AckPublication(Arc); + +impl AckPublication { + pub(super) fn begin_write(&self) -> bool { + self.0 + .compare_exchange(QUEUED, WRITING, Ordering::AcqRel, Ordering::Acquire) + .is_ok() + } + pub(super) fn written(&self) { + self.0.store(WRITTEN, Ordering::Release); + } + pub(super) fn cancel_before_write(&self) -> bool { + self.0 + .compare_exchange(QUEUED, CANCELLED, Ordering::AcqRel, Ordering::Acquire) + .is_ok() + } +} + +pub(super) enum ControlCompletion { + ConfigureReady(RouteBindCompletion), + AckWritten(RouteBindCompletion), + AckCancelled(Box), + ManagementAckWritten { + route: RouteChannel, + principal: Option, + corr: u64, + }, + WriteFailed, +} + +impl ControlCompletion { + pub(super) fn corr(&self) -> u64 { + match self { + Self::ConfigureReady(done) | Self::AckWritten(done) => done.corr, + Self::ManagementAckWritten { corr, .. } => *corr, + Self::AckCancelled(done) => done.corr(), + Self::WriteFailed => 0, + } + } +} + +pub(super) struct AckCompletion { + permit: mpsc::OwnedPermit, + completion: ControlCompletion, + metrics: Arc, + _slot: tokio::sync::OwnedSemaphorePermit, +} + +impl AckCompletion { + pub(super) fn publish(self, cancelled: bool) { + let completion = if cancelled { + ControlCompletion::AckCancelled(Box::new(self.completion)) + } else { + self.completion + }; + self.metrics + .control_completion_queued + .fetch_add(1, Ordering::Relaxed); + let _ = self.permit.send(completion); + } +} + +pub(super) struct A0Egress { + pub(super) writer: WriterSender, + completions: mpsc::Sender, + stop: PersistentCancelSignal, + publications: StdMutex>, + slots: Arc, + probe: Option, +} + +impl A0Egress { + pub(super) fn new(writer: WriterSender, completions: mpsc::Sender) -> Self { + Self { + writer, + completions, + stop: PersistentCancelSignal::new(), + publications: StdMutex::new(HashMap::new()), + slots: Arc::new(tokio::sync::Semaphore::new(WRITER_QUEUE_CAPACITY)), + probe: None, + } + } + + pub(super) fn with_probe(mut self, probe: Option) -> Self { + self.probe = probe; + self + } + + pub(super) fn stop(&self) { + for publication in self.publications.lock().unwrap().values() { + publication.cancel_before_write(); + } + self.stop.cancel(); + } + + pub(super) fn cancel(&self, route: RouteChannel) { + if let Some(publication) = self.publications.lock().unwrap().get(&route) { + publication.cancel_before_write(); + } + } + + pub(super) fn retire(&self, route: RouteChannel) { + self.publications.lock().unwrap().remove(&route); + } + + pub(super) fn enqueue_ack( + &self, + route: RouteChannel, + frame: Frame, + completion: ControlCompletion, + metrics: &Arc, + ) -> Result { + let Ok(slot) = Arc::clone(&self.slots).try_acquire_owned() else { + return Err(completion); + }; + // Reserve the completion before exposing the ACK to the socket. The + // writer can then publish its event synchronously, before a fast peer's + // first route request is polled on the shared current-thread runtime. + let Ok(permit) = self.completions.clone().try_reserve_owned() else { + return Err(completion); + }; + let publication = AckPublication::default(); + let corr = completion.corr(); + self.publications + .lock() + .unwrap() + .insert(route, publication.clone()); + let (mut outgoing, receipt) = WriterFrame::with_ack_receipt(frame, publication.clone()); + outgoing.set_ack_completion(AckCompletion { + permit, + completion, + metrics: Arc::clone(metrics), + _slot: slot, + }); + let writer = self.writer.clone(); + let completions = self.completions.clone(); + let metrics = Arc::clone(metrics); + let stop = self.stop.clone(); + let probe = self.probe.clone(); + let pending = match wire::try_enqueue_writer_item(&writer, &metrics, outgoing) { + WriterEnqueueOutcome::Enqueued => { + if let Some(probe) = self.probe.as_ref() { + probe.route_bind_ack_queued(corr); + } + None + } + WriterEnqueueOutcome::Full(outgoing) => Some(outgoing), + WriterEnqueueOutcome::Closed => None, + }; + tokio::spawn(async move { + let result = tokio::select! { + biased; + _ = stop.cancelled() => return, + result = async { + if let Some(outgoing) = pending { + wire::send_reliable_writer_item(&writer, &metrics, outgoing, "A0 bind ACK").await?; + if let Some(probe) = probe.as_ref() { probe.route_bind_ack_queued(corr); } + } + receipt.await.map_err(|_| SubcError::WriterClosed)? + } => result, + }; + let completion = match result { + Ok(_) => return, + Err(_) => ControlCompletion::WriteFailed, + }; + tokio::select! { + biased; + _ = stop.cancelled() => {}, + _ = send_counted_channel(&completions, &metrics.control_completion_queued, completion) => {}, + } + }); + Ok(publication) + } +} + +impl Drop for A0Egress { + fn drop(&mut self) { + self.stop(); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::future::Future; + use std::pin::Pin; + use std::task::{Context, Poll}; + use tokio::io::AsyncWrite; + + struct HeldWriter { + entered: Option>, + release: oneshot::Receiver<()>, + released: bool, + bytes: Arc>>, + } + + impl AsyncWrite for HeldWriter { + fn poll_write( + mut self: Pin<&mut Self>, + cx: &mut Context<'_>, + bytes: &[u8], + ) -> Poll> { + if let Some(entered) = self.entered.take() { + let _ = entered.send(()); + } + if !self.released { + match Pin::new(&mut self.release).poll(cx) { + Poll::Pending => return Poll::Pending, + Poll::Ready(_) => self.released = true, + } + } + self.bytes.lock().unwrap().extend_from_slice(bytes); + Poll::Ready(Ok(bytes.len())) + } + fn poll_flush(self: Pin<&mut Self>, _: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + fn poll_shutdown(self: Pin<&mut Self>, _: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + } + + #[test] + fn a0_sender_is_captured_before_ledger_proxy_wraps_the_bulk_lane() { + let source = include_str!("mod.rs"); + let body = source + .split_once("async fn run_module_loop<") + .unwrap() + .1 + .split_once("fn classify_connection_end(") + .unwrap() + .0; + let raw = body.find("let raw_writer_tx = writer_tx.clone();").unwrap(); + let proxy = body.find("ledger_edge.proxy(").unwrap(); + assert!( + raw < proxy, + "A0 replies would queue behind native ledger waits" + ); + assert!(body.contains("A0Egress::new(raw_writer_tx, control_completion_tx.clone())")); + let management = body.split_once("handle_management_request(").unwrap().1; + assert!(management.trim_start().starts_with("&a0.writer,")); + } + + #[test] + fn cancellation_can_retract_only_an_ack_that_has_not_started_writing() { + let cancelled = AckPublication::default(); + assert!(cancelled.cancel_before_write()); + assert!(!cancelled.begin_write()); + let writing = AckPublication::default(); + assert!(writing.begin_write()); + assert!(!writing.cancel_before_write()); + writing.written(); + assert!(!writing.cancel_before_write()); + } + + #[tokio::test] + async fn stopping_egress_retracts_a_queued_ack_before_it_reaches_the_socket() { + let (writer, mut frames) = mpsc::channel(1); + let (events, mut completed) = mpsc::channel(1); + let a0 = A0Egress::new(writer, events); + assert!(a0 + .enqueue_ack( + route_key(7, 1), + route_bind_ack_frame(PROTOCOL_VERSION, 891, control_flags()).unwrap(), + ControlCompletion::ManagementAckWritten { + route: route_key(7, 1), + principal: None, + corr: 891 + }, + &Arc::new(DispatchPathMetrics::new()) + ) + .is_ok()); + let mut queued = frames.recv().await.unwrap(); + a0.stop(); + assert!(!queued.begin_ack_write()); + assert!(matches!( + completed.recv().await, + Some(ControlCompletion::AckCancelled(_)) + )); + } + + #[tokio::test] + async fn ack_publication_reserves_capacity_before_spawning_a_waiter() { + let (writer, mut frames) = mpsc::channel(1); + let (events, _completed) = mpsc::channel(1); + let a0 = A0Egress::new(writer, events); + let held = a0 + .slots + .acquire_many(WRITER_QUEUE_CAPACITY as u32) + .await + .unwrap(); + let result = a0.enqueue_ack( + route_key(7, 1), + route_bind_ack_frame(PROTOCOL_VERSION, 891, control_flags()).unwrap(), + ControlCompletion::ManagementAckWritten { + route: route_key(7, 1), + principal: None, + corr: 891, + }, + &Arc::new(DispatchPathMetrics::new()), + ); + assert!(matches!( + result, + Err(ControlCompletion::ManagementAckWritten { .. }) + )); + assert!(frames.try_recv().is_err()); + assert!(a0.publications.lock().unwrap().is_empty()); + drop(held); + } + + #[tokio::test] + async fn ack_completion_waits_for_actual_write_not_queue_acceptance() { + let metrics = Arc::new(DispatchPathMetrics::new()); + let (tx, rx) = mpsc::channel(8); + let (pong, pong_rx) = mpsc::channel(8); + let (done, mut completions) = mpsc::channel(8); + let a0 = A0Egress::new(tx, done); + let (entered, entering) = oneshot::channel(); + let (release, released) = oneshot::channel(); + let bytes = Arc::new(StdMutex::new(Vec::new())); + let writer = spawn_writer_task( + HeldWriter { + entered: Some(entered), + release: released, + released: false, + bytes: Arc::clone(&bytes), + }, + rx, + pong_rx, + Arc::clone(&metrics), + ); + assert!(a0 + .enqueue_ack( + route_key(7, 1), + route_bind_ack_frame(PROTOCOL_VERSION, 891, control_flags()).unwrap(), + ControlCompletion::ManagementAckWritten { + route: route_key(7, 1), + principal: None, + corr: 891, + }, + &metrics, + ) + .is_ok()); + entering.await.unwrap(); + assert!( + completions.try_recv().is_err(), + "ACK published before write_all finished" + ); + assert!(bytes.lock().unwrap().is_empty()); + release.send(()).unwrap(); + assert!(matches!( + completions.recv().await, + Some(ControlCompletion::ManagementAckWritten { .. }) + )); + assert!(!bytes.lock().unwrap().is_empty()); + a0.stop(); + drop(a0); + drop(pong); + writer.await.unwrap().unwrap(); + } + + #[tokio::test] + async fn control_ack_bypasses_a_real_ledger_detach_wait() { + let (_dir, root) = test_support::test_root("a0-held-ledger-detach"); + let executor = Arc::new(Executor::new()); + assert!(executor.register_actor(root.clone(), test_support::test_ctx())); + let (entered, entering) = std::sync::mpsc::channel(); + let (release, released) = std::sync::mpsc::channel(); + let held = executor.submit_async( + root.clone(), + Lane::Mutating, + "held-egress-lane".into(), + Box::new(move |_| { + entered.send(()).unwrap(); + let _ = released.recv_timeout(Duration::from_secs(10)); + Response::success("held-egress-lane", json!({})) + }), + ); + entering.recv_timeout(Duration::from_secs(5)).unwrap(); + let metrics = Arc::new(DispatchPathMetrics::new()); + let (actual, mut receiving) = mpsc::channel(8); + let (edge, _ready) = tool_provider::LedgerEdge::new(); + edge.retain_shell_for_egress_test(root.clone(), route_key(5, 1), 1); + let (proxy, proxy_task) = + edge.proxy(actual.clone(), Arc::clone(&executor), Arc::clone(&metrics)); + proxy + .send( + build_error_frame( + PROTOCOL_VERSION, + 5, + 1, + 1, + control_flags(), + "detached", + "running", + ) + .unwrap() + .into(), + ) + .await + .unwrap(); + let queued = tokio::time::timeout(Duration::from_secs(5), async { + loop { + if executor.try_mutating_job_state_label(&root, "ledger-detach") == Some("queued") { + break; + } + tokio::task::yield_now().await; + } + }) + .await; + let (completed, mut completion_rx) = mpsc::channel(8); + let a0 = A0Egress::new(actual, completed); + assert!(a0 + .enqueue_ack( + route_key(7, 1), + route_bind_ack_frame(PROTOCOL_VERSION, 891, control_flags()).unwrap(), + ControlCompletion::ManagementAckWritten { + route: route_key(7, 1), + principal: None, + corr: 891, + }, + &metrics, + ) + .is_ok()); + let ack = tokio::time::timeout(Duration::from_secs(1), receiving.recv()).await; + let _ = release.send(()); + let _ = held.await; + queued.expect("ledger-detach was never queued behind the holder"); + let mut ack = ack.expect("A0 ACK waited on ledger-detach").unwrap(); + assert_eq!(ack.header.corr, 891); + assert!(ack.begin_ack_write()); + ack.finish_ack_write(true); + assert!(matches!( + completion_rx.recv().await, + Some(ControlCompletion::ManagementAckWritten { .. }) + )); + drop(proxy); + proxy_task.await.unwrap(); + a0.stop(); + } +} diff --git a/crates/aft/src/subc/bind_log.rs b/crates/aft/src/subc/bind_log.rs index fb3f8617a2..de1774ddf1 100644 --- a/crates/aft/src/subc/bind_log.rs +++ b/crates/aft/src/subc/bind_log.rs @@ -67,6 +67,8 @@ mod tests { .unwrap(); handle_control_request( &writer, + &writer, + &A0Egress::new(writer.clone(), completion.clone()), &frame, &app, &executor, diff --git a/crates/aft/src/subc/mod.rs b/crates/aft/src/subc/mod.rs index 85e42a1202..7ee2814eb6 100644 --- a/crates/aft/src/subc/mod.rs +++ b/crates/aft/src/subc/mod.rs @@ -197,7 +197,9 @@ impl fmt::Display for RouteChannel { type PushEnvelope = (ProjectRootId, PushFrame); type LossyPushEnvelope = (u64, ProjectRootId, PushFrame); type RetryBuffer = HashMap>; +mod a0_egress; mod access_grants; +use a0_egress::{A0Egress, ControlCompletion}; mod bash; mod bind_log; mod call_policy; @@ -1433,6 +1435,7 @@ struct RootMeta { #[derive(Debug)] struct PendingBind { + ack_publication: Option, bind_root_id: ProjectRootId, inserted_new_actor: bool, cancelled: bool, @@ -4732,7 +4735,9 @@ async fn connect_and_authenticate_once( #[allow(clippy::too_many_arguments)] async fn process_route_bind_completion( writer_tx: &WriterSender, - completion: RouteBindCompletion, + completion: ControlCompletion, + a0: &A0Egress, + management_routes: &mut HashMap>, routes: &mut HashMap, root_channels: &mut HashMap>, session_identity: &mut HashMap<(ProjectRootId, String), RetainedSessionIdentity>, @@ -4746,9 +4751,33 @@ async fn process_route_bind_completion( lifecycle_probe: Option<&SubcTestLifecycleProbe>, ) -> Result<(), SubcError> { decrement_counted_channel(&metrics.control_completion_queued); + let (completion, ack_written) = match completion { + ControlCompletion::ConfigureReady(completion) => (completion, false), + ControlCompletion::AckWritten(completion) => (completion, true), + ControlCompletion::AckCancelled(completion) => match *completion { + ControlCompletion::AckWritten(completion) => (completion, false), + ControlCompletion::ManagementAckWritten { route, .. } => { + a0.retire(route); + return Ok(()); + } + _ => return Err(SubcError::WriterClosed), + }, + ControlCompletion::ManagementAckWritten { + route, principal, .. + } => { + a0.retire(route); + if installed_route_epochs.get(&route.channel) == Some(&route.epoch) { + management_routes.insert(route, principal); + } + return Ok(()); + } + ControlCompletion::WriteFailed => return Err(SubcError::WriterClosed), + }; handle_route_bind_completion( writer_tx, completion, + ack_written, + a0, routes, root_channels, session_identity, @@ -4766,7 +4795,9 @@ async fn process_route_bind_completion( #[allow(clippy::too_many_arguments)] async fn drain_pending_route_bind_completions( - control_completion_rx: &mut mpsc::Receiver, + control_completion_rx: &mut mpsc::Receiver, + a0: &A0Egress, + management_routes: &mut HashMap>, writer_tx: &WriterSender, routes: &mut HashMap, root_channels: &mut HashMap>, @@ -4785,6 +4816,8 @@ async fn drain_pending_route_bind_completions( process_route_bind_completion( writer_tx, completion, + a0, + management_routes, routes, root_channels, session_identity, @@ -4908,6 +4941,7 @@ where Arc::clone(&dispatch_path_metrics), ); let (ledger_edge, mut ledger_ready_rx) = tool_provider::LedgerEdge::new(); + let raw_writer_tx = writer_tx.clone(); let (writer_tx, mut ledger_writer_task) = ledger_edge.proxy( writer_tx, Arc::clone(&executor), @@ -4992,7 +5026,9 @@ where }; let (bash_poll_touch_tx, mut bash_poll_touch_rx) = mpsc::channel::(256); let (control_completion_tx, mut control_completion_rx) = - mpsc::channel::(256); + mpsc::channel::(256); + let a0 = A0Egress::new(raw_writer_tx, control_completion_tx.clone()) + .with_probe(lifecycle_probe.clone()); let (lossy_tx, mut lossy_rx) = mpsc::channel::(1024); let lossy_overflow = Arc::new(push::LossyOverflow::default()); let lossy_seq = Arc::new(AtomicU64::new(0)); @@ -5130,6 +5166,8 @@ where ); match drain_pending_route_bind_completions( &mut control_completion_rx, + &a0, + &mut management_routes, &writer_tx, &mut routes, &mut root_channels, @@ -5174,7 +5212,7 @@ where 0, ); if let Err(error) = expire_overdue_route_binds( - &writer_tx, + &a0.writer, &executor, &mut pending_binds, &mut installed_route_epochs, @@ -5337,10 +5375,12 @@ where } } Some(completion) = control_completion_rx.recv() => { - dispatch_path_metrics.mark_frame_loop_step("process_route_bind_completion", "route_bind", completion.corr); + dispatch_path_metrics.mark_frame_loop_step("process_route_bind_completion", "route_bind", completion.corr()); if let Err(error) = process_route_bind_completion( &writer_tx, completion, + &a0, + &mut management_routes, &mut routes, &mut root_channels, &mut session_identity, @@ -5437,6 +5477,7 @@ where break Ok(ModuleLoopExit::Graceful); } FrameType::Goodbye => { + a0.cancel(route_key(frame.header.channel, frame.header.epoch)); let channel = route_key(frame.header.channel, frame.header.epoch); if let Some(stamp) = routes.get(&channel).and_then(|identity| identity.scope.clone()) { scope_describes.goodbye(stamp); @@ -5516,7 +5557,9 @@ where } FrameType::Request if frame.header.channel == 0 => { if let Err(error) = handle_control_request( + &a0.writer, &writer_tx, + &a0, &frame, &shared_app, &executor, @@ -5704,7 +5747,7 @@ where Ok(()) } else if management_routes.contains_key(&route) { handle_management_request( - &writer_tx, + &a0.writer, &frame, &shared_app, &executor, @@ -6370,6 +6413,7 @@ where } }; + a0.stop(); // The exit budget runs from the drain, so the teardown between the loop // ending and the index flush is timed and logged as its own exit phase. let teardown_started = Instant::now(); @@ -6486,6 +6530,8 @@ where // so flow reads can ask the daemon whether their parent scope is still // live; the writer tasks cannot drain while the handle is alive. drop(control); + a0.stop(); + drop(a0); drop(writer_tx); drop(pong_tx); if tokio::time::timeout(Duration::from_secs(5), &mut ledger_writer_task) @@ -6614,6 +6660,10 @@ where continue; }; prefer_pong = !control; + decrement_counted_channel(&metrics.writer_queued); + if !queued.begin_ack_write() { + continue; + } queued.project_tool_result().map_err(|error| { subc_transport::FrameIoError::Io(io::Error::other(error.to_string())) })?; @@ -6623,7 +6673,6 @@ where .then(crate::run_tool_call::ThreadCpuSample::now) .flatten(); metrics.writer_active.store(true, Ordering::Relaxed); - decrement_counted_channel(&metrics.writer_queued); let write_timing = write_frame_contiguous( &mut write, queued.frame(), @@ -6636,12 +6685,14 @@ where let write_timing = match write_timing { Ok(write_timing) => write_timing, Err(error) => { + queued.finish_ack_write(false); if let Some(trace) = queued.tool_response_trace.as_ref() { trace.note_write_failure(&error.to_string()); } return Err(error); } }; + queued.finish_ack_write(true); metrics.tool_replied(queued.frame()); if let (Some(trace), Some(dequeued), Some(write_timing)) = @@ -6992,7 +7043,7 @@ fn route_bind_error_code_for_configure_response(response: &Response) -> &'static // Actor lifecycle gaps are transient from the daemon/client viewpoint: // a fresh bind can create or join a healthy actor, so do not classify // them as permanent config divergence. - Some("actor_not_registered" | "actor_fatal") => "actor_not_ready", + Some("actor_not_registered" | "actor_fatal" | "actor_not_ready") => "actor_not_ready", // The retention binding record sits behind a storage-wide file lock // that background sweeps also take. A bind that times out waiting for // it is contention, not divergent configuration, so the client retries @@ -7059,7 +7110,9 @@ fn queue_post_bind_configure_and_completion_maintenance( #[allow(clippy::too_many_arguments)] async fn handle_route_bind_completion( tx: &WriterSender, - completion: RouteBindCompletion, + mut completion: RouteBindCompletion, + ack_written: bool, + a0: &A0Egress, routes: &mut HashMap, root_channels: &mut HashMap>, session_identity: &mut HashMap<(ProjectRootId, String), RetainedSessionIdentity>, @@ -7073,6 +7126,47 @@ async fn handle_route_bind_completion( lifecycle_probe: Option<&SubcTestLifecycleProbe>, ) -> Result<(), SubcError> { let route_id = completion.route; + if !ack_written + && completion.configure_response.success + && pending_binds + .get(&route_id) + .is_some_and(|pending| !pending.cancelled && !pending.deadline_reported) + { + if pending_binds[&route_id].ack_publication.is_some() { + return Ok(()); + } + let restore_watcher = live_roots + .get(&completion.bind_root_id) + .is_some_and(|meta| meta.idle_artifacts_evicted || meta.unbound_quiesced); + if let Some(ctx) = executor.actor_context(&completion.bind_root_id) { + ctx.mark_subc_bound(); + if restore_watcher { + crate::commands::configure::ensure_project_watcher(&ctx); + } + } + let ack = route_bind_ack_frame(completion.ver, completion.corr, completion.flags)?; + match a0.enqueue_ack( + route_id, + ack, + ControlCompletion::AckWritten(completion), + metrics, + ) { + Ok(publication) => { + pending_binds.get_mut(&route_id).unwrap().ack_publication = Some(publication); + return Ok(()); + } + Err(ControlCompletion::AckWritten(mut rejected)) => { + rejected.configure_response = Response::error( + "bind-ack", + "actor_not_ready", + "route bind ACK publication is busy; retry shortly", + ); + completion = rejected; + } + Err(_) => return Err(SubcError::WriterClosed), + } + } + a0.retire(route_id); let Some(pending) = pending_binds.remove(&route_id) else { log::warn!( "subc attach: dropping RouteBind completion for non-pending route {}", @@ -7175,7 +7269,7 @@ async fn handle_route_bind_completion( let fatal = response_is_fatal_panic(response); let error_code = route_bind_error_code_for_configure_response(response); send_route_bind_error_parts( - tx, + &a0.writer, completion.ver, completion.corr, completion.flags, @@ -7209,9 +7303,6 @@ async fn handle_route_bind_completion( Some(&completion.bind_root_id), Some(metrics), ); - let restore_watcher = live_roots - .get(&completion.bind_root_id) - .is_some_and(|meta| meta.idle_artifacts_evicted || meta.unbound_quiesced); live_roots .entry(completion.bind_root_id.clone()) .and_modify(|meta| { @@ -7225,31 +7316,13 @@ async fn handle_route_bind_completion( meta.maintenance_poisoned = false; } if let Some(ctx) = executor.actor_context(&completion.bind_root_id) { - ctx.mark_subc_bound(); - if restore_watcher { - crate::commands::configure::ensure_project_watcher(&ctx); - } + debug_assert!(ack_written); if crate::commands::configure::ignore_rules_load_pending(&ctx) { submit_ignore_rule_load(executor, &completion.bind_root_id); } } - let ack = - serde_json::to_vec(&ModuleControlResponse::RouteBindAck {}).map_err(SubcError::Json)?; - let response = Frame::build_with_version( - completion.ver, - FrameType::Response, - control_flags(), - 0, - 0, - completion.corr, - ack, - ) - .map_err(SubcError::FrameBuild)?; - send_reliable_writer_frame(tx, metrics, response, "RouteBindAck").await?; - if let Some(probe) = lifecycle_probe { - probe.route_bind_ack_queued(completion.corr); - } + debug_assert!(ack_written); start_post_ack_database_open(executor, &completion.bind_root_id); if let Some(identity) = routes .get(&route_id) @@ -7321,7 +7394,13 @@ async fn expire_overdue_route_binds( .iter() .filter_map(|(route, pending)| { let age = now.saturating_duration_since(pending.started_at); - (!pending.deadline_reported && age >= ROUTE_BIND_DEADLINE).then(|| { + (!pending.deadline_reported + && age >= ROUTE_BIND_DEADLINE + && pending + .ack_publication + .as_ref() + .is_none_or(|ack| ack.cancel_before_write())) + .then(|| { ( *route, pending.corr, @@ -7602,6 +7681,8 @@ fn log_scope_workdir_comparison( #[allow(clippy::too_many_arguments)] async fn handle_control_request( tx: &WriterSender, + legacy_tx: &WriterSender, + a0: &A0Egress, frame: &Frame, shared_app: &Arc, executor: &Arc, @@ -7622,7 +7703,7 @@ async fn handle_control_request( retry_buffer: &mut RetryBuffer, push_buffer: &mut HashMap>, shutdown: &Arc, - control_completion_tx: &mpsc::Sender, + control_completion_tx: &mpsc::Sender, metrics: &Arc, lifecycle_probe: Option<&SubcTestLifecycleProbe>, health_rollup_cache: &HealthRollupCache, @@ -7728,8 +7809,9 @@ async fn handle_control_request( ) .await; } + a0.cancel(route_key(route_channel, installed_epoch)); teardown_installed_route( - tx, + legacy_tx, metrics, executor, route_key(route_channel, installed_epoch), @@ -7768,15 +7850,32 @@ async fn handle_control_request( } installed_route_epochs.insert(route_channel, epoch); - management_routes.insert(route_id, principal); - return send_route_bind_ack( - tx, - frame.header.ver, - frame.header.corr, - frame.header.flags, - metrics, - ) - .await; + let ack = + route_bind_ack_frame(frame.header.ver, frame.header.corr, frame.header.flags)?; + if a0 + .enqueue_ack( + route_id, + ack, + ControlCompletion::ManagementAckWritten { + route: route_id, + principal, + corr: frame.header.corr, + }, + metrics, + ) + .is_err() + { + remove_installed_route(installed_route_epochs, route_id); + return send_route_bind_error( + tx, + frame, + "actor_not_ready", + "route bind ACK publication is busy; retry shortly", + metrics, + ) + .await; + } + return Ok(()); } if matches!(&target, RouteTarget::InternalService { .. }) { return send_route_bind_error( @@ -7815,8 +7914,9 @@ async fn handle_control_request( .await; } }; + a0.cancel(route_key(route_channel, installed_epoch)); teardown_installed_route( - tx, + legacy_tx, metrics, executor, route_key(route_channel, installed_epoch), @@ -8055,6 +8155,7 @@ async fn handle_control_request( pending_binds.insert( route_id, PendingBind { + ack_publication: None, bind_root_id: bind_root_id.clone(), inserted_new_actor, cancelled: false, @@ -8100,7 +8201,7 @@ async fn handle_control_request( if send_counted_channel( &completion_tx, &completion_metrics.control_completion_queued, - completion, + ControlCompletion::ConfigureReady(completion), ) .await .is_err() @@ -8849,18 +8950,11 @@ fn install_bash_compressor(ctx: &AppContext) { ); } -async fn send_route_bind_ack( - tx: &WriterSender, - ver: u8, - corr: u64, - flags: Flags, - metrics: &DispatchPathMetrics, -) -> Result<(), SubcError> { +fn route_bind_ack_frame(ver: u8, corr: u64, flags: Flags) -> Result { let body = serde_json::to_vec(&ModuleControlResponse::RouteBindAck {}).map_err(SubcError::Json)?; - let response = Frame::build_with_version(ver, FrameType::Response, flags, 0, 0, corr, body) - .map_err(SubcError::FrameBuild)?; - send_reliable_writer_frame(tx, metrics, response, "RouteBindAck").await + Frame::build_with_version(ver, FrameType::Response, flags, 0, 0, corr, body) + .map_err(SubcError::FrameBuild) } /// Longest decode-error text a refusal carries back to the sender; serde can @@ -14481,6 +14575,8 @@ mod tests { let cpu = cpu_hunt_process_cpu_us(); handle_control_request( &writer_tx, + &writer_tx, + &A0Egress::new(writer_tx.clone(), completion_tx.clone()), &frame, &app, &executor, @@ -14526,6 +14622,9 @@ mod tests { .await .expect("configure completes") .expect("completion delivered"); + let ControlCompletion::ConfigureReady(completion) = completion else { + panic!("configure completion expected"); + }; assert!(completion.configure_response.success); assert_eq!(pending_binds.len(), 1, "the bind must reach admission"); assert_eq!( @@ -19127,6 +19226,7 @@ mod tests { let mut pending_binds = HashMap::from([( route, PendingBind { + ack_publication: None, bind_root_id: root, inserted_new_actor: false, cancelled: false, @@ -19146,11 +19246,16 @@ mod tests { let (writer_tx, mut writer_rx) = mpsc::channel(8); let metrics = Arc::new(DispatchPathMetrics::new()); let executor = Arc::new(Executor::new()); + let mut published_routes = HashMap::new(); + let (ack_tx, mut ack_rx) = mpsc::channel(8); + let a0 = A0Egress::new(writer_tx.clone(), ack_tx); handle_route_bind_completion( &writer_tx, completion, - &mut HashMap::new(), + false, + &a0, + &mut published_routes, &mut HashMap::new(), &mut HashMap::new(), &mut push_buffer, @@ -19165,7 +19270,41 @@ mod tests { .await .unwrap(); - let ack = writer_rx.try_recv().expect("RouteBindAck"); + assert!( + !published_routes.contains_key(&route), + "route published before actual ACK write" + ); + + let mut ack = writer_rx.recv().await.expect("RouteBindAck"); + assert!( + writer_rx.try_recv().is_err(), + "route egress must remain buffered" + ); + assert!(ack.begin_ack_write()); + ack.finish_ack_write(true); + let ControlCompletion::AckWritten(completion) = ack_rx.recv().await.unwrap() else { + panic!("ACK write completion expected"); + }; + handle_route_bind_completion( + &writer_tx, + completion, + true, + &a0, + &mut published_routes, + &mut HashMap::new(), + &mut HashMap::new(), + &mut push_buffer, + &mut HashMap::new(), + &mut pending_binds, + &mut installed_route_epochs, + &executor, + &Arc::new(Notify::new()), + &metrics, + None, + ) + .await + .unwrap(); + assert!(published_routes.contains_key(&route)); assert_eq!(ack.header.ty, FrameType::Response); assert_eq!((ack.header.channel, ack.header.epoch), (0, 0)); let route_frame = writer_rx.try_recv().expect("post-ack route frame"); @@ -19215,6 +19354,7 @@ mod tests { let mut pending_binds = HashMap::from([( route, PendingBind { + ack_publication: None, bind_root_id: root, inserted_new_actor: false, cancelled: false, @@ -19236,6 +19376,8 @@ mod tests { handle_route_bind_completion( &writer_tx, completion, + true, + &A0Egress::new(writer_tx.clone(), mpsc::channel(8).0), &mut HashMap::new(), &mut HashMap::new(), &mut HashMap::new(), @@ -19265,6 +19407,7 @@ mod tests { async fn a_held_bind_is_refused_before_the_daemon_bind_relay_times_out() { let (_dir, root) = test_root("route-bind-held-configure"); let pending_bind = |age: Duration, corr: u64| PendingBind { + ack_publication: None, bind_root_id: root.clone(), inserted_new_actor: false, cancelled: false, @@ -19366,6 +19509,7 @@ mod tests { let mut pending_binds = HashMap::from([( route, PendingBind { + ack_publication: None, bind_root_id: root.clone(), inserted_new_actor: false, cancelled: false, @@ -19560,6 +19704,7 @@ mod tests { let mut pending_binds = HashMap::from([( route, PendingBind { + ack_publication: None, bind_root_id: root.clone(), inserted_new_actor: false, cancelled: false, @@ -19802,6 +19947,7 @@ mod tests { let mut pending_binds = HashMap::from([( route, PendingBind { + ack_publication: None, bind_root_id: root.clone(), inserted_new_actor: false, cancelled: false, diff --git a/crates/aft/src/subc/scope_workdir_tests.rs b/crates/aft/src/subc/scope_workdir_tests.rs index 8721e8dd2e..244b96db3f 100644 --- a/crates/aft/src/subc/scope_workdir_tests.rs +++ b/crates/aft/src/subc/scope_workdir_tests.rs @@ -81,6 +81,7 @@ fn bind_observed( let metrics = Arc::new(DispatchPathMetrics::new()); let (writer_tx, mut writer_rx) = mpsc::channel(8); let (completion_tx, mut completion_rx) = mpsc::channel(8); + let a0 = A0Egress::new(writer_tx.clone(), completion_tx.clone()); let (lossy_tx, _lossy_rx) = mpsc::channel(8); let (reliable_tx, _reliable_rx) = mpsc::unbounded_channel(); let senders = PushSenders { @@ -122,6 +123,8 @@ fn bind_observed( .map(|_| observe_next_comparison()); handle_control_request( &writer_tx, + &writer_tx, + &a0, &frame, &app, &executor, @@ -159,6 +162,9 @@ fn bind_observed( .unwrap() .unwrap(); let expected_root = ProjectRootId::from_path(root).unwrap(); + let ControlCompletion::ConfigureReady(completion) = completion else { + panic!("configure completion expected"); + }; assert!(completion.configure_response.success); assert_eq!(completion.bind_root_id, expected_root); assert_eq!(completion.identity.root, expected_root); @@ -185,6 +191,35 @@ fn bind_observed( handle_route_bind_completion( &writer_tx, completion, + false, + &a0, + &mut routes, + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut pending, + &mut installed, + &executor, + &Arc::new(Notify::new()), + &metrics, + None, + ) + .await + .unwrap(); + assert!(routes.is_empty(), "route must wait for ACK write"); + let mut ack = writer_rx.recv().await.expect("successful bind reply"); + assert!(ack.begin_ack_write()); + ack.finish_ack_write(true); + let ControlCompletion::AckWritten(completion) = completion_rx.recv().await.unwrap() + else { + panic!("written ACK expected"); + }; + handle_route_bind_completion( + &writer_tx, + completion, + true, + &a0, &mut routes, &mut HashMap::new(), &mut HashMap::new(), @@ -200,7 +235,6 @@ fn bind_observed( .await .unwrap(); assert_eq!(routes[&route_key(7, 1)].root, expected_root); - let ack = writer_rx.try_recv().expect("successful bind reply"); assert_eq!(ack.header.ty, FrameType::Response); assert_eq!( (ack.header.channel, ack.header.epoch, ack.header.corr), diff --git a/crates/aft/src/subc/tool_provider.rs b/crates/aft/src/subc/tool_provider.rs index 955eacea7e..7ed424346a 100644 --- a/crates/aft/src/subc/tool_provider.rs +++ b/crates/aft/src/subc/tool_provider.rs @@ -1768,6 +1768,28 @@ async fn kill_on_owning_root( } impl LedgerEdge { + #[cfg(test)] + pub(super) fn retain_shell_for_egress_test( + &self, + root: crate::path_identity::ProjectRootId, + route: super::RouteChannel, + corr: u64, + ) { + self.calls.lock().unwrap().insert( + (route, corr), + LedgerCall { + root, + key: crate::db::call_ledger::Key { + carrier: "egress-test".into(), + call_key: "held-detach".into(), + }, + shell: true, + detached: false, + expires_at: std::time::Instant::now() + std::time::Duration::from_secs(60), + withdrawn: None, + }, + ); + } pub(super) fn new() -> ( std::sync::Arc, tokio::sync::mpsc::UnboundedReceiver, diff --git a/crates/aft/src/subc/wire.rs b/crates/aft/src/subc/wire.rs index bcada549ff..24eac91b05 100644 --- a/crates/aft/src/subc/wire.rs +++ b/crates/aft/src/subc/wire.rs @@ -138,6 +138,11 @@ pub(super) struct WriterFrame { pub(super) tool_response_trace: Option, result_projection: Option<(ToolResponsePolicy, usize, Option)>, prepared_projection: Option>>, + ack_receipt: Option<( + super::a0_egress::AckPublication, + tokio::sync::oneshot::Sender>, + )>, + ack_completion: Option, } impl From for WriterFrame { @@ -162,6 +167,8 @@ impl WriterFrame { tool_response_trace: None, result_projection: None, prepared_projection: None, + ack_receipt: None, + ack_completion: None, } } @@ -173,9 +180,65 @@ impl WriterFrame { tool_response_trace: None, result_projection: None, prepared_projection: None, + ack_receipt: None, + ack_completion: None, } } + pub(super) fn with_ack_receipt( + frame: Frame, + publication: super::a0_egress::AckPublication, + ) -> ( + Self, + tokio::sync::oneshot::Receiver>, + ) { + let (tx, rx) = tokio::sync::oneshot::channel(); + let mut outgoing = Self::plain(frame); + outgoing.ack_receipt = Some((publication, tx)); + (outgoing, rx) + } + + pub(super) fn begin_ack_write(&mut self) -> bool { + if self + .ack_receipt + .as_ref() + .is_some_and(|(publication, _)| !publication.begin_write()) + { + if let Some(completion) = self.ack_completion.take() { + completion.publish(true); + } + if let Some((_, receipt)) = self.ack_receipt.take() { + let _ = receipt.send(Ok(false)); + } + return false; + } + true + } + + pub(super) fn finish_ack_write(&mut self, written: bool) { + if written { + if let Some(completion) = self.ack_completion.take() { + completion.publish(false); + } + } else { + self.ack_completion.take(); + } + if let Some((publication, receipt)) = self.ack_receipt.take() { + if written { + publication.written(); + } + let _ = receipt.send(if written { + Ok(true) + } else { + Err(SubcError::WriterClosed) + }); + } + } + + pub(super) fn set_ack_completion(&mut self, completion: super::a0_egress::AckCompletion) { + self.ack_completion = Some(completion); + } + fn traced_tool_response(frame: impl Into, trace: ToolResponseWriteTrace) -> Self { let mut outgoing = frame.into(); outgoing.tool_response_trace = Some(trace); @@ -483,7 +546,7 @@ fn enqueue_writer_item( permit.send(item); } -fn try_enqueue_writer_item( +pub(super) fn try_enqueue_writer_item( tx: &WriterSender, metrics: &DispatchPathMetrics, mut item: WriterFrame, @@ -524,7 +587,7 @@ pub(super) fn try_enqueue_shared_push_frame( try_enqueue_writer_item(tx, metrics, WriterFrame::shared_push(frame, body)) } -async fn send_reliable_writer_item( +pub(super) async fn send_reliable_writer_item( tx: &WriterSender, metrics: &DispatchPathMetrics, mut item: WriterFrame, From 293287d7d1a0f186119d9c2ceb226b51513129f1 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Mon, 26 Oct 2026 02:22:06 +0100 Subject: [PATCH 03/13] mason: retain the prepared bind through identity recovery (#891) Carry the same private prepared-config Arc through provisional identity replay and record the snapshot actually applied by configure before route publication. Keep the config read bounded at the I/O boundary, including locked migration rereads, and test nested scope restoration plus real captured-source watch attachment. Configure fixtures now execute configure rather than inventing a successful response with no applied witness. Linux RLIMIT_NOFILE=4096: cargo fmt --check, cargo check --tests --bins, prepared_bind_ (6), subc_config:: (16), config_fix:: (25), config_live::tests:: (46) passed. Independent controls reddened only their named test: discard replay snapshot; remove production scope; remove publication witness; restore None rather than nested scope; use read-all migration; reselect watched source; remove read-time bound. Each job restored source bytes before pristine green filters. See the carried investigation for exact output and earlier content/source/notice controls. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- crates/aft/src/commands/configure.rs | 52 +++++ crates/aft/src/config_fix.rs | 11 +- crates/aft/src/config_live.rs | 19 ++ crates/aft/src/config_live_tests.rs | 49 ++++ crates/aft/src/provisional_identity_retry.rs | 8 +- .../src/subc/access_grants_restore_tests.rs | 1 + crates/aft/src/subc/mod.rs | 65 +++++- crates/aft/src/subc/prepared_bind_tests.rs | 209 ++++++++++++++++++ crates/aft/src/subc/scope_workdir_tests.rs | 8 +- crates/aft/src/subc/tool_result_tests.rs | 6 +- crates/aft/src/subc_config.rs | 3 +- crates/aft/src/subc_config/prepared.rs | 97 ++++++-- .../aft/tests/integration/subc_bridge_test.rs | 4 + 13 files changed, 498 insertions(+), 34 deletions(-) create mode 100644 crates/aft/src/subc/prepared_bind_tests.rs diff --git a/crates/aft/src/commands/configure.rs b/crates/aft/src/commands/configure.rs index 2bd138a8b9..8b1a2e304f 100644 --- a/crates/aft/src/commands/configure.rs +++ b/crates/aft/src/commands/configure.rs @@ -3561,6 +3561,7 @@ fn handle_configure_prepared( // refused connect that left it in place. let active_before = prepared.is_none().then(|| ctx.config_unpinned()); if prepared.is_none() { + ctx.config_live().clear_applied_bind_snapshot(); ctx.note_connect_started(); } let _sources_scope = crate::config_live::configure_sources_scope(); @@ -17450,6 +17451,57 @@ mod tests { ); } + #[test] + fn prepared_bind_identity_replay_retains_the_original_observation() { + let _git_env = crate::test_env::hermetic_git_env_guard(); + let (_temp, root, storage, lock) = busy_bind_fixture(); + let ctx = test_context(); + let user = root.join("user.jsonc"); + let legacy = root.join(".pi/aft.jsonc"); + write_config( + &user, + r#"{"indexes":{"trigram":true,"semantic":false,"callgraph":false},"views":{"enabled":false},"url_fetch_allow_private":true}"#, + ); + write_config(&legacy, r#"{"callgraph_chunk_size":7}"#); + let snapshot = crate::subc_config::PreparedBindConfig::read( + Some(&user), + &root, + &crate::harness::Harness::Pi, + ) + .unwrap(); + let request = configure_request_with_params( + json!({"project_root":root,"storage_dir":storage,"harness":"pi","cortexkit_user_config_path":user}), + ); + crate::provisional_identity_retry::with_timing_for_test( + Duration::ZERO, + Duration::from_secs(600), + || { + let response = crate::subc_config::with_prepared_bind_config(snapshot, || { + super::handle_configure(&request, &ctx) + }); + assert!(response.success, "{}", response.data); + super::drain_deferred_configure_maintenance(&ctx); + assert!(ctx.provisional_identity_retry().is_armed()); + write_config(&user, r#"{"url_fetch_allow_private":false}"#); + write_config( + &root.join(".cortexkit/aft.jsonc"), + r#"{"callgraph_chunk_size":9}"#, + ); + fs::remove_file(&legacy).unwrap(); + lock.execute_batch("ROLLBACK").unwrap(); + drop(lock); + super::drain_deferred_configure_maintenance(&ctx); + assert!(!ctx.provisional_identity_retry().is_armed()); + assert_eq!(ctx.config().callgraph_chunk_size, 7); + assert!(ctx.config().url_fetch_allow_private); + assert_eq!( + ctx.config_live().sources().unwrap().project.path.as_deref(), + Some(legacy.as_path()) + ); + }, + ); + } + /// A replay can resolve and memoize the settled key and still be refused /// before its commit (for example by a storage error at its retention or /// artifact-owner step). The root then stays provisional with the settled diff --git a/crates/aft/src/config_fix.rs b/crates/aft/src/config_fix.rs index b0414ab84d..5a52c8f28e 100644 --- a/crates/aft/src/config_fix.rs +++ b/crates/aft/src/config_fix.rs @@ -607,7 +607,6 @@ pub(crate) fn auto_migrate_user_config_bounded( path: &Path, limit: usize, ) -> std::io::Result> { - use std::io::Read; let mut failure = None; let result = auto_migrate_user_config_with_reader( path, @@ -615,15 +614,7 @@ pub(crate) fn auto_migrate_user_config_bounded( |path| { let read = (|| { let file = std::fs::File::open(path)?; - let mut bytes = Vec::new(); - file.take((limit + 1) as u64).read_to_end(&mut bytes)?; - if bytes.len() > limit { - return Err(std::io::Error::other( - "configuration exceeds migration byte limit", - )); - } - String::from_utf8(bytes) - .map_err(|error| std::io::Error::new(std::io::ErrorKind::InvalidData, error)) + crate::subc_config::read_bounded_config(file, limit) })(); if let Err(error) = &read { failure = Some(std::io::Error::new(error.kind(), error.to_string())); diff --git a/crates/aft/src/config_live.rs b/crates/aft/src/config_live.rs index 934d8f2add..182d434e3c 100644 --- a/crates/aft/src/config_live.rs +++ b/crates/aft/src/config_live.rs @@ -113,6 +113,7 @@ pub struct TierSource { /// the files changed and which changed keys are still waiting for a connect. #[derive(Debug, Clone)] pub struct ConfigSources { + pub(crate) prepared_bind_snapshot_id: Option, pub user: TierSource, pub project: TierSource, /// The resolver output of the last configure, before configure's own @@ -163,6 +164,7 @@ impl ConfigSources { } }; Self { + prepared_bind_snapshot_id: None, user: source_for("user", user_path), project: source_for("project", Some(project_path)), connected: connected.clone(), @@ -255,7 +257,11 @@ pub fn finish_configure(ctx: &AppContext, success: bool) { ctx.begin_configure_ack_step("config_sources_commit"); { let _reload = ctx.config_live().reload_lock.lock(); + let snapshot_id = sources.prepared_bind_snapshot_id.unwrap_or(0); *ctx.config_live().sources.lock() = Some(sources); + ctx.config_live() + .applied_bind_snapshot + .store(snapshot_id, Ordering::Release); // A connect resolves the files afresh; nothing it applied is held. ctx.config_live() .project_hold_active @@ -268,6 +274,7 @@ pub fn finish_configure(ctx: &AppContext, success: bool) { /// Per-root live reload state, owned by the root's `AppContext`. #[derive(Default)] pub struct ConfigLiveState { + applied_bind_snapshot: AtomicU64, signal: Arc, sources: parking_lot::Mutex>, watches: parking_lot::Mutex, @@ -295,6 +302,16 @@ struct ContextWatches { } impl ConfigLiveState { + pub(crate) fn applied_bind_snapshot(&self) -> Option { + match self.applied_bind_snapshot.load(Ordering::Acquire) { + 0 => None, + id => Some(id), + } + } + + pub(crate) fn clear_applied_bind_snapshot(&self) { + self.applied_bind_snapshot.store(0, Ordering::Release); + } pub fn signal(&self) -> Arc { Arc::clone(&self.signal) } @@ -658,7 +675,9 @@ fn reload_config_inner(ctx: &AppContext) -> ReloadOutcome { { let mut record = state.sources.lock(); + state.clear_applied_bind_snapshot(); if let Some(record) = record.as_mut() { + record.prepared_bind_snapshot_id = None; record.user = user; // While a project loosening is held, keep the project text the // held values came from as the reference, so later reloads keep diff --git a/crates/aft/src/config_live_tests.rs b/crates/aft/src/config_live_tests.rs index 46382a7c55..10ebb1f5ba 100644 --- a/crates/aft/src/config_live_tests.rs +++ b/crates/aft/src/config_live_tests.rs @@ -97,6 +97,55 @@ fn staged_sources_are_cleared_at_entry_and_on_unwind() { ); } +#[test] +fn prepared_bind_watches_keep_the_captured_source() { + const NAME: &str = "config_live::tests::prepared_bind_watches_keep_the_captured_source"; + const CHILD: &str = "AFT_TEST_PREPARED_WATCH_CHILD"; + if std::env::var_os(CHILD).is_none() { + let output = std::process::Command::new(std::env::current_exe().unwrap()) + .args(["--exact", NAME, "--nocapture", "--color=never"]) + .env(CHILD, "1") + .env_remove("AFT_TEST_DISABLE_FILE_WATCHER") + .output() + .unwrap(); + assert!( + output.status.success() + && String::from_utf8_lossy(&output.stdout).contains("1 passed; 0 failed"), + "watch child failed: {:?} {:?}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + return; + } + let _git_env = crate::test_env::hermetic_git_env_guard(); + let fixture = Fixture::new( + r#"{"indexes":{"trigram":false,"semantic":false,"callgraph":false},"views":{"enabled":false}}"#, + None, + ); + let legacy = fixture.root.join(".pi/aft.jsonc"); + write(&legacy, r#"{"callgraph_chunk_size":7}"#); + let prepared = crate::subc_config::PreparedBindConfig::read( + Some(&fixture.user_path), + &fixture.root, + &crate::harness::Harness::Pi, + ) + .unwrap(); + write(&fixture.project_path, r#"{"callgraph_chunk_size":9}"#); + std::fs::remove_file(&legacy).unwrap(); + enable_config_watches_for_test(); + let request: RawRequest = serde_json::from_value(json!({"id":"prepared-watch","command":"configure","project_root":fixture.root,"harness":"pi","cortexkit_user_config_path":fixture.user_path})).unwrap(); + let response = crate::subc_config::with_prepared_bind_config(prepared, || { + crate::commands::configure::handle_configure(&request, &fixture.ctx) + }); + assert!(response.success, "{}", response.data); + fixture.ctx.config_live().wait_for_watch_attachment(); + let watches = fixture.ctx.config_live().watches.lock(); + assert_eq!( + watches.project_fallback.as_ref().unwrap().file(), + legacy.as_path() + ); +} + #[test] fn inspect_categories_and_lsp_idle_reload_without_restart() { let fixture = Fixture::new(r#"{"lsp":{"idle_minutes":60}}"#, None); diff --git a/crates/aft/src/provisional_identity_retry.rs b/crates/aft/src/provisional_identity_retry.rs index c457e8d3eb..977c9436a6 100644 --- a/crates/aft/src/provisional_identity_retry.rs +++ b/crates/aft/src/provisional_identity_retry.rs @@ -169,6 +169,7 @@ enum RetryKind { struct PendingRetry { /// The configure request whose bind left a retryable degradation. request: RawRequest, + prepared_bind: Option>, kind: RetryKind, root: std::path::PathBuf, connect_generation: u64, @@ -258,6 +259,7 @@ pub(crate) fn note_configure_outcome(ctx: &AppContext, request: &RawRequest, suc } *slot = Some(PendingRetry { request: request.clone(), + prepared_bind: crate::subc_config::current_prepared_bind_config(), kind, root, connect_generation: ctx.connect_generation(), @@ -496,7 +498,7 @@ impl PreparedReplay { if retry.kind == RetryKind::CacheCoordination { retry.attempts = retry.attempts.saturating_add(1); } - let response = match retry.kind { + let run = || match retry.kind { RetryKind::Identity => { crate::commands::configure::handle_configure(&retry.request, ctx) } @@ -513,6 +515,10 @@ impl PreparedReplay { ) } }; + let response = match retry.prepared_bind.as_ref() { + Some(prepared) => crate::subc_config::with_prepared_bind_config(prepared.clone(), run), + None => run(), + }; if response.data["code"] == "configure_needs_exclusive" { // A shared-hold run handed its worker back before changing the // root; the executor runs this job again as an exclusive writer. diff --git a/crates/aft/src/subc/access_grants_restore_tests.rs b/crates/aft/src/subc/access_grants_restore_tests.rs index 45e94a52cf..d3f242c906 100644 --- a/crates/aft/src/subc/access_grants_restore_tests.rs +++ b/crates/aft/src/subc/access_grants_restore_tests.rs @@ -80,6 +80,7 @@ impl WireRestore { app, executor, |request, ctx| match request.command.as_str() { + "configure" => crate::commands::configure::handle_configure(&request, ctx), "read" => crate::commands::read::handle_read(&request, ctx), "write" => crate::commands::write::handle_write(&request, ctx), "bash" => crate::commands::bash::handle(&request, ctx), diff --git a/crates/aft/src/subc/mod.rs b/crates/aft/src/subc/mod.rs index 7ee2814eb6..4b6caedce9 100644 --- a/crates/aft/src/subc/mod.rs +++ b/crates/aft/src/subc/mod.rs @@ -228,6 +228,8 @@ pub use self::health::SubcTestHealthRollupProbe; mod manifest; mod page_collect; mod persistence; +#[cfg(test)] +mod prepared_bind_tests; mod push; mod readiness; mod remote_policy; @@ -1454,6 +1456,8 @@ struct PendingBind { } struct RouteBindCompletion { + expected_snapshot_id: Option, + applied_snapshot_id: Option, route: RouteChannel, identity: RouteIdentity, bind_root_id: ProjectRootId, @@ -7126,6 +7130,16 @@ async fn handle_route_bind_completion( lifecycle_probe: Option<&SubcTestLifecycleProbe>, ) -> Result<(), SubcError> { let route_id = completion.route; + if completion.configure_response.success + && completion.expected_snapshot_id.is_some() + && completion.expected_snapshot_id != completion.applied_snapshot_id + { + completion.configure_response = Response::error( + "bind-config-witness", + "config_unknown", + "configure did not apply this bind's prepared configuration snapshot", + ); + } if !ack_written && completion.configure_response.success && pending_binds @@ -8106,6 +8120,9 @@ async fn handle_control_request( sync_bg_live_delivery_sessions(executor, routes, Some(&bind_root_id), Some(metrics)); let configure_request_id = configure_req.id.clone(); + let expected_snapshot_id = prepared_config.as_ref().map(|snapshot| snapshot.id()); + let applied_snapshot = Arc::new(AtomicU64::new(0)); + let job_snapshot = Arc::clone(&applied_snapshot); installed_route_epochs.insert(route_channel, epoch); if let Some(meta) = live_roots.get_mut(&bind_root_id) { meta.maintenance_queued_kinds.clear(); @@ -8126,6 +8143,7 @@ async fn handle_control_request( Arc::new(move |ctx| { log_ctx::with_session(Some(configure_session.clone()), || { let bind_started = Instant::now(); + job_snapshot.store(0, Ordering::Release); let response = match prepared_config.as_ref() { Some(snapshot) => crate::subc_config::with_prepared_bind_config( Arc::clone(snapshot), @@ -8133,6 +8151,12 @@ async fn handle_control_request( ), None => dispatch(configure_req.clone(), ctx), }; + if response.success { + job_snapshot.store( + ctx.config_live().applied_bind_snapshot().unwrap_or(0), + Ordering::Release, + ); + } if response.success { // Revoke matching standing publication before the // session is installed, but never on the frame loop. @@ -8188,6 +8212,11 @@ async fn handle_control_request( // read data, so a later maintenance pass can do it without delaying the // daemon's confirmation that the route is usable. let completion = RouteBindCompletion { + expected_snapshot_id, + applied_snapshot_id: match applied_snapshot.load(Ordering::Acquire) { + 0 => None, + id => Some(id), + }, route: route_key(completion_route_channel, epoch), identity: completion_identity, bind_root_id: completion_root, @@ -13938,7 +13967,13 @@ mod tests { &fixture_path.join("absent-connection.json"), app, loop_executor, - |request, _| Response::success(request.id, json!({})), + |request, ctx| { + if request.command == "configure" { + crate::commands::configure::handle_configure(&request, ctx) + } else { + Response::success(request.id, json!({})) + } + }, Some(fixture_path.join("absent-user-config.json")), false, usize::MAX, @@ -14102,7 +14137,13 @@ mod tests { &fixture_path.join("absent-connection.json"), app, loop_executor, - |request, _| Response::success(request.id, json!({})), + |request, ctx| { + if request.command == "configure" { + crate::commands::configure::handle_configure(&request, ctx) + } else { + Response::success(request.id, json!({})) + } + }, Some(fixture_path.join("absent-user-config.json")), false, usize::MAX, @@ -14605,7 +14646,13 @@ mod tests { None, &cache, &senders, - |request, _| Response::success(request.id, json!({})), + |request, ctx| { + if request.command == "configure" { + crate::commands::configure::handle_configure(&request, ctx) + } else { + Response::success(request.id, json!({})) + } + }, Some(&dir.path().join("absent-user-config.json")), usize::MAX, None, @@ -19217,6 +19264,8 @@ mod tests { identity, bind_root_id: root.clone(), inserted_new_actor: false, + expected_snapshot_id: None, + applied_snapshot_id: None, configure_response: Response::success("subc-bind-7", json!({})), diagnostics_on_edit: false, ver: PROTOCOL_VERSION, @@ -19342,6 +19391,8 @@ mod tests { identity, bind_root_id: root.clone(), inserted_new_actor: false, + expected_snapshot_id: None, + applied_snapshot_id: None, configure_response: Response::success("subc-bind-8", json!({})), diagnostics_on_edit: false, ver: PROTOCOL_VERSION, @@ -23566,7 +23617,13 @@ mod pong_priority_tests { &storage_path.join("connection.json"), App::default_shared(), module_executor, - |request, _| Response::success(request.id, json!({})), + |request, ctx| { + if request.command == "configure" { + crate::commands::configure::handle_configure(&request, ctx) + } else { + Response::success(request.id, json!({})) + } + }, Some(storage_path.join("user.json")), false, usize::MAX, diff --git a/crates/aft/src/subc/prepared_bind_tests.rs b/crates/aft/src/subc/prepared_bind_tests.rs new file mode 100644 index 0000000000..97b390f9ad --- /dev/null +++ b/crates/aft/src/subc/prepared_bind_tests.rs @@ -0,0 +1,209 @@ +use super::*; + +#[tokio::test] +async fn bind_publication_rejects_missing_or_mismatched_applied_snapshot() { + let (_dir, root) = test_support::test_root("prepared-witness-guard"); + for applied in [None, Some(8)] { + let route = route_key(7, 1); + let (writer, mut frames) = mpsc::channel(8); + let (done, _completed) = mpsc::channel(8); + let a0 = A0Egress::new(writer.clone(), done); + let completion = RouteBindCompletion { + expected_snapshot_id: Some(9), + applied_snapshot_id: applied, + route, + identity: test_support::route_identity(&root, "witness"), + bind_root_id: root.clone(), + inserted_new_actor: false, + configure_response: Response::success("witness", json!({})), + diagnostics_on_edit: false, + ver: PROTOCOL_VERSION, + corr: 891, + flags: control_flags(), + }; + let mut pending = HashMap::from([( + route, + PendingBind { + ack_publication: None, + bind_root_id: root.clone(), + inserted_new_actor: false, + cancelled: false, + configure_request_id: "witness".into(), + started_at: Instant::now(), + warned_half_deadline: false, + deadline_reported: false, + corr: 891, + ver: PROTOCOL_VERSION, + flags: control_flags(), + cancellation: JobCancellation::new(), + }, + )]); + let mut routes = HashMap::new(); + handle_route_bind_completion( + &writer, + completion, + false, + &a0, + &mut routes, + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut pending, + &mut HashMap::from([(7, 1)]), + &Arc::new(Executor::new()), + &Arc::new(Notify::new()), + &Arc::new(DispatchPathMetrics::new()), + None, + ) + .await + .unwrap(); + let frame = frames.recv().await.unwrap(); + assert_eq!( + frame.header.ty, + FrameType::Error, + "an unapplied snapshot must not produce an ACK" + ); + assert_eq!(frame.header.corr, 891); + assert_eq!( + serde_json::from_slice::(&frame.body).unwrap()["code"], + "config_unknown" + ); + assert!(routes.is_empty()); + } +} + +#[tokio::test] +async fn real_bind_dispatch_scope_survives_config_edits_before_executor_admission() { + let (dir, root) = test_support::test_root("prepared-production-bind"); + let user = dir.path().join("user.jsonc"); + let project = root.as_path().join(".pi/aft.jsonc"); + std::fs::create_dir_all(project.parent().unwrap()).unwrap(); + std::fs::write(&user, r#"{"indexes":{"trigram":false,"semantic":false,"callgraph":false},"views":{"enabled":false},"url_fetch_allow_private":true}"#).unwrap(); + std::fs::write(&project, r#"{"callgraph_chunk_size":7}"#).unwrap(); + let app = App::default_shared(); + let ctx = Arc::new(crate::context::AppContext::from_app( + app.clone(), + crate::config::Config { + storage_dir: Some(dir.path().join("storage")), + ..Default::default() + }, + )); + let executor = Arc::new(Executor::new()); + assert!(executor.register_actor(root.clone(), ctx.clone())); + let (entered, entering) = std::sync::mpsc::channel(); + let (release, released) = std::sync::mpsc::channel(); + let holder = executor.submit_async( + root.clone(), + Lane::Mutating, + "prepared-bind-holder".into(), + Box::new(move |_| { + entered.send(()).unwrap(); + let _ = released.recv_timeout(Duration::from_secs(30)); + Response::success("prepared-bind-holder", json!({})) + }), + ); + entering.recv_timeout(Duration::from_secs(5)).unwrap(); + let (writer, _frames) = mpsc::channel(8); + let (done, mut completed) = mpsc::channel(8); + let a0 = A0Egress::new(writer.clone(), done.clone()); + let metrics = Arc::new(DispatchPathMetrics::new()); + let (lossy_tx, _lossy) = mpsc::channel(8); + let (reliable_tx, _reliable) = mpsc::unbounded_channel(); + let senders = PushSenders { + lossy_tx, + reliable_tx, + lossy_overflow: Arc::default(), + lossy_seq: Arc::new(AtomicU64::new(0)), + fleet_status_client: FleetStatusClient::channel(1).0, + }; + let request = ModuleControlRequest::RouteBind { + route_channel: 7, + epoch: 1, + target: RouteTarget::ToolProvider { + module_id: "aft".into(), + }, + identity: subc_protocol::BindIdentity::new(root.as_path(), "pi", "prepared-bind"), + principal: Some(subc_protocol::Principal::Direct), + consumer_capabilities: None, + admission_facts: Default::default(), + scope: None, + role_versions: None, + call_mode: None, + }; + let frame = Frame::build( + FrameType::Request, + control_flags(), + 0, + 0, + 891, + serde_json::to_vec(&request).unwrap(), + ) + .unwrap(); + let admitted = handle_control_request( + &writer, + &writer, + &a0, + &frame, + &app, + &executor, + &Arc::new(standing::StandingActor::new(app.clone(), executor.clone())), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &Arc::new(StdMutex::new(HashMap::new())), + &mut PendingSubcResponses::default(), + &mut HashMap::new(), + &mut HashMap::new(), + &Arc::new(Notify::new()), + &done, + &metrics, + None, + &HealthRollupCache::new(), + &senders, + |request, ctx| crate::commands::configure::handle_configure(&request, ctx), + Some(&user), + usize::MAX, + None, + ) + .await; + std::fs::write(&user, r#"{"url_fetch_allow_private":false}"#).unwrap(); + let canonical = root.as_path().join(".cortexkit/aft.jsonc"); + std::fs::create_dir_all(canonical.parent().unwrap()).unwrap(); + std::fs::write(canonical, r#"{"callgraph_chunk_size":9}"#).unwrap(); + let _ = release.send(()); + let _ = holder.await; + admitted.unwrap(); + let ControlCompletion::ConfigureReady(completion) = + tokio::time::timeout(Duration::from_secs(15), completed.recv()) + .await + .unwrap() + .unwrap() + else { + panic!("configure completion expected"); + }; + assert!( + completion.configure_response.success, + "{}", + completion.configure_response.data + ); + assert!(completion.expected_snapshot_id.is_some()); + assert_eq!( + completion.expected_snapshot_id, + completion.applied_snapshot_id + ); + assert_eq!(ctx.config().callgraph_chunk_size, 7); + assert!(ctx.config().url_fetch_allow_private); + assert_eq!( + ctx.config_live().sources().unwrap().project.path.as_deref(), + Some(project.as_path()) + ); +} diff --git a/crates/aft/src/subc/scope_workdir_tests.rs b/crates/aft/src/subc/scope_workdir_tests.rs index 244b96db3f..79641bbd69 100644 --- a/crates/aft/src/subc/scope_workdir_tests.rs +++ b/crates/aft/src/subc/scope_workdir_tests.rs @@ -150,7 +150,13 @@ fn bind_observed( None, &HealthRollupCache::new(), &senders, - |request, _| Response::success(request.id, request.params), + |request, ctx| { + let configured = crate::commands::configure::handle_configure(&request, ctx); + if !configured.success { + return configured; + } + Response::success(request.id, request.params) + }, Some(&root.join("absent-user-config.json")), usize::MAX, None, diff --git a/crates/aft/src/subc/tool_result_tests.rs b/crates/aft/src/subc/tool_result_tests.rs index 5886da4035..0d3278e15b 100644 --- a/crates/aft/src/subc/tool_result_tests.rs +++ b/crates/aft/src/subc/tool_result_tests.rs @@ -249,7 +249,7 @@ async fn tool_result_ast_search_t6_complete_and_budget_cut_on_negotiated_route() loop_executor, |request, ctx| { if request.command == "configure" { - Response::success(request.id, json!({})) + crate::commands::configure::handle_configure(&request, ctx) } else { crate::commands::ast_search::handle_ast_search(&request, &ctx) } @@ -517,9 +517,9 @@ async fn tool_result_capability_is_selected_per_bind_not_per_call() { &module_root.join("absent-connection.json"), App::default_shared(), loop_executor, - |request, _| { + |request, ctx| { if request.command == "configure" { - return Response::success(request.id, json!({})); + return crate::commands::configure::handle_configure(&request, ctx); } Response::error_with_data( "same-id", diff --git a/crates/aft/src/subc_config.rs b/crates/aft/src/subc_config.rs index 6683f13b6b..9a2f3e7a4e 100644 --- a/crates/aft/src/subc_config.rs +++ b/crates/aft/src/subc_config.rs @@ -20,7 +20,8 @@ use std::path::{Path, PathBuf}; mod prepared; pub(crate) use prepared::{ - current_prepared_bind_config, with_prepared_bind_config, PreparedBindConfig, + current_prepared_bind_config, read_bounded_config, with_prepared_bind_config, + PreparedBindConfig, }; /// CortexKit user config home: `$XDG_CONFIG_HOME/cortexkit/aft.jsonc`, falling diff --git a/crates/aft/src/subc_config/prepared.rs b/crates/aft/src/subc_config/prepared.rs index 4ccd0a2218..a551a183ea 100644 --- a/crates/aft/src/subc_config/prepared.rs +++ b/crates/aft/src/subc_config/prepared.rs @@ -3,8 +3,11 @@ use super::*; use std::cell::RefCell; use std::io::Read; +use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::Arc; +static NEXT_SNAPSHOT_ID: AtomicU64 = AtomicU64::new(1); + const CONFIG_FILE_LIMIT: usize = 1024 * 1024; #[derive(Clone, Debug)] @@ -27,6 +30,7 @@ impl std::fmt::Display for ConfigObservationError { #[derive(Clone, Debug)] pub(crate) struct PreparedBindConfig { + snapshot_id: u64, root: PathBuf, harness: Harness, user_path: Option, @@ -83,6 +87,7 @@ impl PreparedBindConfig { }); } Ok(Arc::new(Self { + snapshot_id: NEXT_SNAPSHOT_ID.fetch_add(1, Ordering::Relaxed), root: root.into(), harness: harness.clone(), user_path: user_path.map(Path::to_path_buf), @@ -96,6 +101,9 @@ impl PreparedBindConfig { pub(crate) fn matches(&self, root: &Path, harness: &Harness) -> bool { self.root == root && &self.harness == harness } + pub(crate) fn id(&self) -> u64 { + self.snapshot_id + } pub(crate) fn tiers(&self) -> &[ConfigTier] { &self.tiers @@ -110,12 +118,14 @@ impl PreparedBindConfig { &self, config: &crate::config::Config, ) -> crate::config_live::ConfigSources { - crate::config_live::ConfigSources::from_observed_paths( + let mut sources = crate::config_live::ConfigSources::from_observed_paths( self.user_path.clone(), self.project_path.clone(), &self.tiers, config, - ) + ); + sources.prepared_bind_snapshot_id = Some(self.snapshot_id); + sources } pub(crate) fn legacy_notice(&self) -> Option { self.tiers @@ -145,18 +155,8 @@ fn observe(path: &Path, tier: &str) -> Result return Err(unknown(path, &error)), }; - let mut bytes = Vec::new(); - file.take((CONFIG_FILE_LIMIT + 1) as u64) - .read_to_end(&mut bytes) - .map_err(|error| unknown(path, &error))?; - if bytes.len() > CONFIG_FILE_LIMIT { - return Err(unknown( - path, - &std::io::Error::other("configuration exceeds the 1 MiB observation limit"), - )); - } - let doc = String::from_utf8(bytes) - .map_err(|_| unknown(path, &std::io::Error::other("configuration is not UTF-8")))?; + let doc = + read_bounded_config(file, CONFIG_FILE_LIMIT).map_err(|error| unknown(path, &error))?; Ok(ConfigObservation::Present(ConfigTier { tier: tier.into(), source: path.to_string_lossy().into_owned(), @@ -164,6 +164,23 @@ fn observe(path: &Path, tier: &str) -> Result std::io::Result { + let detection_limit = limit + .checked_add(1) + .ok_or_else(|| std::io::Error::other("invalid config byte limit"))?; + let mut bytes = Vec::new(); + reader + .take(detection_limit as u64) + .read_to_end(&mut bytes)?; + if bytes.len() > limit { + return Err(std::io::Error::other("configuration exceeds byte limit")); + } + String::from_utf8(bytes) + .map_err(|error| std::io::Error::new(std::io::ErrorKind::InvalidData, error)) +} + thread_local! { static PREPARED: RefCell>> = const { RefCell::new(None) }; } @@ -195,6 +212,43 @@ mod tests { std::fs::write(path, text).unwrap(); } + #[test] + fn prepared_read_never_requests_bytes_beyond_limit_plus_one() { + struct ReadFence { + bytes: usize, + } + impl Read for &mut ReadFence { + fn read(&mut self, out: &mut [u8]) -> std::io::Result { + if out.is_empty() { + return Ok(0); + } + assert!(self.bytes < 17, "read beyond config limit+1"); + let count = out.len().min(17 - self.bytes); + out[..count].fill(b' '); + self.bytes += count; + Ok(count) + } + } + let mut reader = ReadFence { bytes: 0 }; + assert!(read_bounded_config(&mut reader, 16).is_err()); + assert_eq!(reader.bytes, 17); + } + + #[test] + fn prepared_migration_rereads_use_the_same_read_time_bound() { + let source = include_str!("../config_fix.rs"); + let body = source + .split_once("pub(crate) fn auto_migrate_user_config_bounded(") + .unwrap() + .1 + .split_once("fn auto_migrate_user_config_with_reader(") + .unwrap() + .0; + assert!(body.contains("read_bounded_config(file, limit)")); + assert!(!body.contains("read_to_string")); + assert!(!body.contains("read_to_end")); + } + #[test] fn prepared_config_uses_active_harness_and_stops_on_unknown_candidate() { let dir = tempfile::tempdir().unwrap(); @@ -261,4 +315,19 @@ mod tests { })); assert!(current_prepared_bind_config().is_none()); } + + #[test] + fn prepared_nested_scope_restores_the_previous_snapshot_after_unwind() { + let dir = tempfile::tempdir().unwrap(); + let outer = PreparedBindConfig::read(None, dir.path(), &Harness::Pi).unwrap(); + let inner = PreparedBindConfig::read(None, dir.path(), &Harness::Opencode).unwrap(); + let expected = outer.id(); + with_prepared_bind_config(outer, || { + let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + with_prepared_bind_config(inner, || panic!("nested scope unwind")) + })); + assert_eq!(current_prepared_bind_config().unwrap().id(), expected); + }); + assert!(current_prepared_bind_config().is_none()); + } } diff --git a/crates/aft/tests/integration/subc_bridge_test.rs b/crates/aft/tests/integration/subc_bridge_test.rs index ad1939cb11..9f1a179e90 100644 --- a/crates/aft/tests/integration/subc_bridge_test.rs +++ b/crates/aft/tests/integration/subc_bridge_test.rs @@ -706,6 +706,10 @@ impl BridgeState { } fn configure_bridge_context(req: &RawRequest, ctx: &AppContext) -> Response { + let configured = aft::commands::configure::handle_configure(req, ctx); + if !configured.success { + return configured; + } let root = match req.params.get("project_root").and_then(Value::as_str) { Some(root) => std::path::PathBuf::from(root), None => { From 62b4d3f493827c8d66dbfbf58977ca7f9cf32db1 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Mon, 26 Oct 2026 10:40:23 +0100 Subject: [PATCH 04/13] mason: answer retracted binds and gate candidate calls on written ACKs (#891) Reserve bind receipt capacity before configure side effects. Replace retracted queued ACKs in place with one existing cancelled, bind_superseded or module_reloading control error, and hold candidate requests until the bind ACK completes. Bound held frames and wire bytes, preserve failure-order replies, and never execute a cancelled held call. Receipt slots now cover waiter lifetime; configure-ready messages cannot deadlock against ACK-reserved completion slots. The approved protocol change intentionally replaces the old pre-ACK route_not_bound assertion with hold-then-serve, and zero-terminal cancelled-bind expectations with one error. Already closed stale route generations remain silent. ACK write-start wins success and may finish after its deadline; queued refusals name their actual wait stage. The separate busy text names route binds rather than storage. This is a preserved review step, not final delivery: 42 recorded controls include 41 sole-test reds plus one initially surviving live receipt control, now defended by direct consumption tests. Linux and Windows GNU compile checks passed before the latest test-only RESERVED deadline extension. Compatibility follow-up remains: the broad tool-result fixture hit a loaded-host watchdog, and the existing long bind-deadline integration timed out. Fixture index startup and deadline diagnostics are pending build-server capacity; see the carried investigation and exact proof artifacts. Do not read this intermediate commit as all final gates passing. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- .../release-notes/next/a0-control-egress.md | 2 +- crates/aft/src/subc/a0_egress.rs | 1138 +++++++++++++++-- crates/aft/src/subc/bind_ack_fixup_tests.rs | 287 +++++ crates/aft/src/subc/bind_candidates.rs | 523 ++++++++ crates/aft/src/subc/bind_capacity_tests.rs | 161 +++ crates/aft/src/subc/mod.rs | 197 ++- crates/aft/src/subc/wire.rs | 31 +- .../aft/tests/integration/subc_bridge_test.rs | 56 +- 8 files changed, 2259 insertions(+), 136 deletions(-) create mode 100644 crates/aft/src/subc/bind_ack_fixup_tests.rs create mode 100644 crates/aft/src/subc/bind_candidates.rs create mode 100644 crates/aft/src/subc/bind_capacity_tests.rs diff --git a/.alfonso/release-notes/next/a0-control-egress.md b/.alfonso/release-notes/next/a0-control-egress.md index a5588378f4..760ce687e5 100644 --- a/.alfonso/release-notes/next/a0-control-egress.md +++ b/.alfonso/release-notes/next/a0-control-egress.md @@ -1 +1 @@ -- Prepared route-bind and management replies use a writer path independent of tool-outcome persistence, and routes and replayed events are published only after the bind acknowledgement is written. +- Route binds and management replies no longer wait for tool-outcome persistence; candidate calls and replayed events wait for the written bind acknowledgement, and cancelled queued binds receive one control error instead of losing their reply. diff --git a/crates/aft/src/subc/a0_egress.rs b/crates/aft/src/subc/a0_egress.rs index 7d6b2cecf6..e070d0bcd0 100644 --- a/crates/aft/src/subc/a0_egress.rs +++ b/crates/aft/src/subc/a0_egress.rs @@ -9,23 +9,153 @@ const QUEUED: u8 = 0; const WRITING: u8 = 1; const WRITTEN: u8 = 2; const CANCELLED: u8 = 3; +const RESERVED: u8 = 4; +pub(super) const BIND_BUSY_MESSAGE: &str = + "too many requests are waiting for a route bind; retry shortly"; -#[derive(Clone, Default, Debug)] -pub(super) struct AckPublication(Arc); +#[derive(Clone, Copy, Debug)] +struct BindMeta { + route: RouteChannel, + ver: u8, + corr: u64, + flags: Flags, +} + +struct PublicationState { + phase: AtomicU8, + meta: Option, + error: StdMutex, + error_claimed: AtomicBool, + enqueued: AtomicBool, + enqueue_failed: AtomicBool, + enqueue_changed: Notify, + terminal_written: AtomicBool, + succeeded: AtomicBool, +} + +#[derive(Clone)] +pub(super) struct AckPublication(Arc); + +impl fmt::Debug for AckPublication { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("AckPublication") + .field("phase", &self.0.phase.load(Ordering::Acquire)) + .finish() + } +} + +impl Default for AckPublication { + fn default() -> Self { + Self::new(None, QUEUED) + } +} impl AckPublication { + fn new(meta: Option, phase: u8) -> Self { + Self(Arc::new(PublicationState { + phase: AtomicU8::new(phase), + meta, + error: StdMutex::new(ErrorBody::new("cancelled", "request cancelled")), + error_claimed: AtomicBool::new(false), + enqueued: AtomicBool::new(false), + enqueue_failed: AtomicBool::new(false), + enqueue_changed: Notify::new(), + terminal_written: AtomicBool::new(false), + succeeded: AtomicBool::new(false), + })) + } pub(super) fn begin_write(&self) -> bool { self.0 + .phase .compare_exchange(QUEUED, WRITING, Ordering::AcqRel, Ordering::Acquire) .is_ok() } pub(super) fn written(&self) { - self.0.store(WRITTEN, Ordering::Release); + self.0.phase.store(WRITTEN, Ordering::Release); + self.finished(true); + } + pub(super) fn finished(&self, succeeded: bool) { + self.0.succeeded.store(succeeded, Ordering::Release); + self.0.terminal_written.store(true, Ordering::Release); + } + pub(super) fn outcome(&self) -> Option { + self.0 + .terminal_written + .load(Ordering::Acquire) + .then(|| self.0.succeeded.load(Ordering::Acquire)) + } + pub(super) fn is_queued(&self) -> bool { + self.0.phase.load(Ordering::Acquire) == QUEUED } + pub(super) fn is_committed(&self) -> bool { + matches!(self.0.phase.load(Ordering::Acquire), WRITING | WRITTEN) + } + pub(super) fn is_retracted(&self) -> bool { + self.0.phase.load(Ordering::Acquire) == CANCELLED + } + #[cfg(test)] pub(super) fn cancel_before_write(&self) -> bool { + self.retract(ErrorBody::new("cancelled", "request cancelled")) + } + pub(super) fn retract(&self, error: ErrorBody) -> bool { + let mut reason = self.0.error.lock().unwrap(); + let phase = self.0.phase.load(Ordering::Acquire); + if !matches!(phase, RESERVED | QUEUED) + || self + .0 + .phase + .compare_exchange(phase, CANCELLED, Ordering::AcqRel, Ordering::Acquire) + .is_err() + { + return false; + } + *reason = error; + true + } + pub(super) fn error_frame(&self, ack: &Frame) -> Result, SubcError> { + if !self.is_retracted() || self.0.error_claimed.swap(true, Ordering::AcqRel) { + return Ok(None); + } + let error = self.0.error.lock().unwrap().clone(); + Frame::build_with_version( + ack.header.ver, + FrameType::Error, + ack.header.flags, + 0, + 0, + ack.header.corr, + serde_json::to_vec(&error).map_err(SubcError::Json)?, + ) + .map(Some) + .map_err(SubcError::FrameBuild) + } + fn queued(&self) { self.0 - .compare_exchange(QUEUED, CANCELLED, Ordering::AcqRel, Ordering::Acquire) - .is_ok() + .phase + .compare_exchange(RESERVED, QUEUED, Ordering::AcqRel, Ordering::Acquire) + .ok(); + } + fn mark_enqueued(&self, failed: bool) { + if failed { + self.0.enqueue_failed.store(true, Ordering::Release); + } else { + self.0.enqueued.store(true, Ordering::Release); + } + self.0.enqueue_changed.notify_waiters(); + } + pub(super) async fn wait_enqueued(&self) -> Result<(), SubcError> { + loop { + let changed = self.0.enqueue_changed.notified(); + tokio::pin!(changed); + changed.as_mut().enable(); + if self.0.enqueue_failed.load(Ordering::Acquire) { + return Err(SubcError::WriterClosed); + } + if self.0.enqueued.load(Ordering::Acquire) { + return Ok(()); + } + changed.await; + } } } @@ -38,6 +168,10 @@ pub(super) enum ControlCompletion { principal: Option, corr: u64, }, + BindRetracted { + route: RouteChannel, + corr: u64, + }, WriteFailed, } @@ -45,7 +179,7 @@ impl ControlCompletion { pub(super) fn corr(&self) -> u64 { match self { Self::ConfigureReady(done) | Self::AckWritten(done) => done.corr, - Self::ManagementAckWritten { corr, .. } => *corr, + Self::ManagementAckWritten { corr, .. } | Self::BindRetracted { corr, .. } => *corr, Self::AckCancelled(done) => done.corr(), Self::WriteFailed => 0, } @@ -56,9 +190,7 @@ pub(super) struct AckCompletion { permit: mpsc::OwnedPermit, completion: ControlCompletion, metrics: Arc, - _slot: tokio::sync::OwnedSemaphorePermit, } - impl AckCompletion { pub(super) fn publish(self, cancelled: bool) { let completion = if cancelled { @@ -73,13 +205,55 @@ impl AckCompletion { } } +struct PublicationBudget { + slot: tokio::sync::OwnedSemaphorePermit, + permit: mpsc::OwnedPermit, +} +struct PublicationEntry { + publication: AckPublication, + budget: Option, + metrics: Arc, +} + +pub(super) struct BindReservation<'a> { + egress: &'a A0Egress, + route: RouteChannel, + publication: AckPublication, + committed: bool, +} +impl BindReservation<'_> { + pub(super) fn commit(mut self) -> AckPublication { + self.committed = true; + self.publication.clone() + } +} +impl Drop for BindReservation<'_> { + fn drop(&mut self) { + if !self.committed { + let mut entries = self.egress.publications.lock().unwrap(); + if entries.get(&self.route).is_some_and(|entry| { + Arc::ptr_eq(&entry.publication.0, &self.publication.0) && entry.budget.is_some() + }) { + entries.remove(&self.route); + } + } + } +} + pub(super) struct A0Egress { pub(super) writer: WriterSender, completions: mpsc::Sender, - stop: PersistentCancelSignal, - publications: StdMutex>, + publications: StdMutex>, slots: Arc, + closing: AtomicBool, probe: Option, + #[cfg(test)] + waiter_gate: Option>, +} +#[cfg(test)] +pub(super) struct WaiterGate { + pub(super) entered: tokio::sync::Semaphore, + pub(super) release: tokio::sync::Semaphore, } impl A0Egress { @@ -87,35 +261,132 @@ impl A0Egress { Self { writer, completions, - stop: PersistentCancelSignal::new(), publications: StdMutex::new(HashMap::new()), slots: Arc::new(tokio::sync::Semaphore::new(WRITER_QUEUE_CAPACITY)), + closing: AtomicBool::new(false), probe: None, + #[cfg(test)] + waiter_gate: None, } } - pub(super) fn with_probe(mut self, probe: Option) -> Self { self.probe = probe; self } - + pub(super) fn reserve( + &self, + route: RouteChannel, + ver: u8, + corr: u64, + flags: Flags, + metrics: &Arc, + ) -> Result, ErrorBody> { + let busy = + || ErrorBody::new("busy", BIND_BUSY_MESSAGE).with_detail(json!({"retryable":true})); + if self.closing.load(Ordering::Acquire) { + return Err(busy()); + } + let slot = self.slots.clone().try_acquire_owned().map_err(|_| busy())?; + let permit = self + .completions + .clone() + .try_reserve_owned() + .map_err(|_| busy())?; + let publication = AckPublication::new( + Some(BindMeta { + route, + ver, + corr, + flags, + }), + RESERVED, + ); + let mut entries = self.publications.lock().unwrap(); + if entries.contains_key(&route) { + return Err(busy()); + } + entries.insert( + route, + PublicationEntry { + publication: publication.clone(), + budget: Some(PublicationBudget { slot, permit }), + metrics: metrics.clone(), + }, + ); + Ok(BindReservation { + egress: self, + route, + publication, + committed: false, + }) + } + pub(super) fn publication(&self, route: RouteChannel) -> Option { + self.publications + .lock() + .unwrap() + .get(&route) + .map(|entry| entry.publication.clone()) + } pub(super) fn stop(&self) { - for publication in self.publications.lock().unwrap().values() { - publication.cancel_before_write(); + self.closing.store(true, Ordering::Release); + let routes: Vec<_> = self.publications.lock().unwrap().keys().copied().collect(); + for route in routes { + self.reject( + route, + ErrorBody::new(drain::MODULE_DRAINING_ERROR_CODE, "module is reloading") + .with_detail(json!({"retryable":true})), + ); } - self.stop.cancel(); } - pub(super) fn cancel(&self, route: RouteChannel) { - if let Some(publication) = self.publications.lock().unwrap().get(&route) { - publication.cancel_before_write(); - } + self.reject(route, ErrorBody::new("cancelled", "request cancelled")); + } + pub(super) fn supersede(&self, route: RouteChannel) { + self.reject( + route, + ErrorBody::new("bind_superseded", "route bind superseded by a newer epoch"), + ); } - pub(super) fn retire(&self, route: RouteChannel) { - self.publications.lock().unwrap().remove(&route); + let mut entries = self.publications.lock().unwrap(); + if entries + .get(&route) + .is_some_and(|entry| entry.publication.outcome().is_some()) + { + entries.remove(&route); + } + } + pub(super) fn reject(&self, route: RouteChannel, error: ErrorBody) -> bool { + let pending = { + let mut entries = self.publications.lock().unwrap(); + let Some(entry) = entries.get_mut(&route) else { + return false; + }; + if !entry.publication.retract(error) { + return false; + } + entry + .budget + .take() + .map(|budget| (entry.publication.clone(), budget, entry.metrics.clone())) + }; + if let Some((publication, budget, metrics)) = pending { + let meta = publication.0.meta.unwrap(); + let frame = route_bind_ack_frame(meta.ver, meta.corr, meta.flags) + .expect("small bind ACK is encodable"); + self.queue( + publication, + frame, + ControlCompletion::BindRetracted { + route: meta.route, + corr: meta.corr, + }, + budget, + &metrics, + ); + } + true } - pub(super) fn enqueue_ack( &self, route: RouteChannel, @@ -123,69 +394,90 @@ impl A0Egress { completion: ControlCompletion, metrics: &Arc, ) -> Result { - let Ok(slot) = Arc::clone(&self.slots).try_acquire_owned() else { - return Err(completion); + let reserved = { + let mut entries = self.publications.lock().unwrap(); + entries.get_mut(&route).and_then(|entry| { + entry + .budget + .take() + .map(|budget| (entry.publication.clone(), budget)) + }) }; - // Reserve the completion before exposing the ACK to the socket. The - // writer can then publish its event synchronously, before a fast peer's - // first route request is polled on the shared current-thread runtime. - let Ok(permit) = self.completions.clone().try_reserve_owned() else { + let Some((publication, budget)) = reserved else { return Err(completion); }; - let publication = AckPublication::default(); + publication.queued(); + self.queue(publication.clone(), frame, completion, budget, metrics); + Ok(publication) + } + fn queue( + &self, + publication: AckPublication, + frame: Frame, + completion: ControlCompletion, + budget: PublicationBudget, + metrics: &Arc, + ) { let corr = completion.corr(); - self.publications - .lock() - .unwrap() - .insert(route, publication.clone()); let (mut outgoing, receipt) = WriterFrame::with_ack_receipt(frame, publication.clone()); outgoing.set_ack_completion(AckCompletion { - permit, + permit: budget.permit, completion, - metrics: Arc::clone(metrics), - _slot: slot, + metrics: metrics.clone(), }); let writer = self.writer.clone(); let completions = self.completions.clone(); - let metrics = Arc::clone(metrics); - let stop = self.stop.clone(); + let metrics = metrics.clone(); let probe = self.probe.clone(); let pending = match wire::try_enqueue_writer_item(&writer, &metrics, outgoing) { WriterEnqueueOutcome::Enqueued => { + publication.mark_enqueued(false); if let Some(probe) = self.probe.as_ref() { probe.route_bind_ack_queued(corr); } None } WriterEnqueueOutcome::Full(outgoing) => Some(outgoing), - WriterEnqueueOutcome::Closed => None, + WriterEnqueueOutcome::Closed => { + publication.mark_enqueued(true); + None + } }; + #[cfg(test)] + let waiter_gate = self.waiter_gate.clone(); tokio::spawn(async move { - let result = tokio::select! { - biased; - _ = stop.cancelled() => return, - result = async { - if let Some(outgoing) = pending { - wire::send_reliable_writer_item(&writer, &metrics, outgoing, "A0 bind ACK").await?; - if let Some(probe) = probe.as_ref() { probe.route_bind_ack_queued(corr); } + // The slot covers the entire receipt waiter, even after its writer + // completion has been published and consumed by the frame loop. + let _slot = budget.slot; + let result = async { + if let Some(outgoing) = pending { + wire::send_reliable_writer_item(&writer, &metrics, outgoing, "A0 bind ACK") + .await?; + publication.mark_enqueued(false); + if let Some(probe) = probe.as_ref() { + probe.route_bind_ack_queued(corr); } - receipt.await.map_err(|_| SubcError::WriterClosed)? - } => result, - }; - let completion = match result { - Ok(_) => return, - Err(_) => ControlCompletion::WriteFailed, - }; - tokio::select! { - biased; - _ = stop.cancelled() => {}, - _ = send_counted_channel(&completions, &metrics.control_completion_queued, completion) => {}, + } + receipt.await.map_err(|_| SubcError::WriterClosed)? + } + .await; + #[cfg(test)] + if let Some(gate) = waiter_gate { + gate.entered.add_permits(1); + let _ = gate.release.acquire().await; + } + if result.is_err() { + publication.mark_enqueued(true); + let _ = send_counted_channel( + &completions, + &metrics.control_completion_queued, + ControlCompletion::WriteFailed, + ) + .await; } }); - Ok(publication) } } - impl Drop for A0Egress { fn drop(&mut self) { self.stop(); @@ -271,6 +563,15 @@ mod tests { let (writer, mut frames) = mpsc::channel(1); let (events, mut completed) = mpsc::channel(1); let a0 = A0Egress::new(writer, events); + a0.reserve( + route_key(7, 1), + PROTOCOL_VERSION, + 891, + control_flags(), + &Arc::new(DispatchPathMetrics::new()), + ) + .unwrap() + .commit(); assert!(a0 .enqueue_ack( route_key(7, 1), @@ -285,7 +586,9 @@ mod tests { .is_ok()); let mut queued = frames.recv().await.unwrap(); a0.stop(); - assert!(!queued.begin_ack_write()); + assert!(queued.begin_ack_write()); + assert_eq!(queued.header.ty, FrameType::Error); + queued.finish_ack_write(true); assert!(matches!( completed.recv().await, Some(ControlCompletion::AckCancelled(_)) @@ -302,20 +605,14 @@ mod tests { .acquire_many(WRITER_QUEUE_CAPACITY as u32) .await .unwrap(); - let result = a0.enqueue_ack( + let result = a0.reserve( route_key(7, 1), - route_bind_ack_frame(PROTOCOL_VERSION, 891, control_flags()).unwrap(), - ControlCompletion::ManagementAckWritten { - route: route_key(7, 1), - principal: None, - corr: 891, - }, + PROTOCOL_VERSION, + 891, + control_flags(), &Arc::new(DispatchPathMetrics::new()), ); - assert!(matches!( - result, - Err(ControlCompletion::ManagementAckWritten { .. }) - )); + assert_eq!(result.err().unwrap().code, "busy"); assert!(frames.try_recv().is_err()); assert!(a0.publications.lock().unwrap().is_empty()); drop(held); @@ -328,6 +625,15 @@ mod tests { let (pong, pong_rx) = mpsc::channel(8); let (done, mut completions) = mpsc::channel(8); let a0 = A0Egress::new(tx, done); + a0.reserve( + route_key(7, 1), + PROTOCOL_VERSION, + 891, + control_flags(), + &metrics, + ) + .unwrap() + .commit(); let (entered, entering) = oneshot::channel(); let (release, released) = oneshot::channel(); let bytes = Arc::new(StdMutex::new(Vec::new())); @@ -362,7 +668,9 @@ mod tests { assert!(bytes.lock().unwrap().is_empty()); release.send(()).unwrap(); assert!(matches!( - completions.recv().await, + tokio::time::timeout(Duration::from_secs(5), completions.recv()) + .await + .expect("actual write did not publish its reserved completion"), Some(ControlCompletion::ManagementAckWritten { .. }) )); assert!(!bytes.lock().unwrap().is_empty()); @@ -372,11 +680,31 @@ mod tests { writer.await.unwrap().unwrap(); } - #[tokio::test] - async fn control_ack_bypasses_a_real_ledger_detach_wait() { + async fn control_ack_bypasses_held_ledger_wait(settling: bool) { let (_dir, root) = test_support::test_root("a0-held-ledger-detach"); let executor = Arc::new(Executor::new()); - assert!(executor.register_actor(root.clone(), test_support::test_ctx())); + let ctx = test_support::test_ctx(); + let key = crate::db::call_ledger::Key { + carrier: "egress-test".into(), + call_key: "held-detach".into(), + }; + let db = settling.then(|| { + Arc::new(StdMutex::new( + crate::db::open(&_dir.path().join("aft.db")).unwrap(), + )) + }); + if let Some(db) = db.as_ref() { + crate::db::call_ledger::admit( + &db.lock().unwrap(), + &key, + "digest", + None, + crate::db::call_ledger::State::DispatchStarted, + ) + .unwrap(); + ctx.set_db(db.clone()); + } + assert!(executor.register_actor(root.clone(), ctx)); let (entered, entering) = std::sync::mpsc::channel(); let (release, released) = std::sync::mpsc::channel(); let held = executor.submit_async( @@ -404,7 +732,7 @@ mod tests { 1, 1, control_flags(), - "detached", + if settling { "done" } else { "detached" }, "running", ) .unwrap() @@ -414,7 +742,15 @@ mod tests { .unwrap(); let queued = tokio::time::timeout(Duration::from_secs(5), async { loop { - if executor.try_mutating_job_state_label(&root, "ledger-detach") == Some("queued") { + if executor.try_mutating_job_state_label( + &root, + if settling { + "ledger-settle" + } else { + "ledger-detach" + }, + ) == Some("queued") + { break; } tokio::task::yield_now().await; @@ -423,6 +759,15 @@ mod tests { .await; let (completed, mut completion_rx) = mpsc::channel(8); let a0 = A0Egress::new(actual, completed); + a0.reserve( + route_key(7, 1), + PROTOCOL_VERSION, + 891, + control_flags(), + &metrics, + ) + .unwrap() + .commit(); assert!(a0 .enqueue_ack( route_key(7, 1), @@ -447,8 +792,645 @@ mod tests { completion_rx.recv().await, Some(ControlCompletion::ManagementAckWritten { .. }) )); + let terminal = tokio::time::timeout(Duration::from_secs(5), receiving.recv()) + .await + .unwrap() + .unwrap(); + assert_eq!(terminal.header.channel, 5); + if let Some(db) = db { + assert_eq!( + crate::db::call_ledger::get(&db.lock().unwrap(), &key) + .unwrap() + .unwrap() + .state, + crate::db::call_ledger::State::Settled + ); + } drop(proxy); proxy_task.await.unwrap(); a0.stop(); } + + #[tokio::test] + async fn control_ack_bypasses_a_real_ledger_detach_wait() { + control_ack_bypasses_held_ledger_wait(false).await; + } + + #[tokio::test] + async fn control_ack_bypasses_a_real_ledger_settle_wait() { + control_ack_bypasses_held_ledger_wait(true).await; + } +} + +#[cfg(test)] +mod fixup_tests { + use super::*; + + async fn cancelled_receipt(tool: bool) { + let (_dir, root) = test_support::test_root("retracted-ack-receipt-consumption"); + let route = route_key(7, 1); + let (writer, mut frames) = mpsc::channel(8); + let (done, mut completed) = mpsc::channel(8); + let metrics = Arc::new(DispatchPathMetrics::new()); + let a0 = A0Egress::new(writer.clone(), done); + let publication = a0 + .reserve(route, PROTOCOL_VERSION, 891, control_flags(), &metrics) + .unwrap() + .commit(); + let mut pending = HashMap::new(); + let completion = if tool { + pending.insert( + route, + PendingBind { + ack_publication: Some(publication.clone()), + bind_root_id: root.clone(), + inserted_new_actor: false, + cancelled: true, + configure_request_id: "retracted-receipt".into(), + started_at: Instant::now(), + warned_half_deadline: false, + deadline_reported: false, + corr: 891, + ver: PROTOCOL_VERSION, + flags: control_flags(), + cancellation: JobCancellation::new(), + }, + ); + ControlCompletion::AckWritten(RouteBindCompletion { + expected_snapshot_id: None, + applied_snapshot_id: None, + route, + identity: test_support::route_identity(&root, "receipt"), + bind_root_id: root, + inserted_new_actor: false, + configure_response: Response::success("receipt", json!({})), + diagnostics_on_edit: false, + ver: PROTOCOL_VERSION, + corr: 891, + flags: control_flags(), + }) + } else { + ControlCompletion::ManagementAckWritten { + route, + principal: None, + corr: 891, + } + }; + assert!(a0 + .enqueue_ack( + route, + route_bind_ack_frame(PROTOCOL_VERSION, 891, control_flags()).unwrap(), + completion, + &metrics + ) + .is_ok()); + let mut queued = frames.recv().await.unwrap(); + a0.cancel(route); + assert!(queued.begin_ack_write()); + assert_eq!(queued.header.ty, FrameType::Error); + queued.finish_ack_write(true); + let completion = completed.recv().await.unwrap(); + assert!(matches!(completion, ControlCompletion::AckCancelled(_))); + let mut installed = HashMap::from([(7, 2)]); + process_route_bind_completion( + &writer, + completion, + &a0, + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut pending, + &mut installed, + &Arc::new(Executor::new()), + &Arc::new(Notify::new()), + &metrics, + None, + ) + .await + .expect("a valid retraction receipt must not close the connection"); + assert!(a0.publications.lock().unwrap().is_empty()); + assert!(pending.is_empty()); + assert_eq!( + installed.get(&7), + Some(&2), + "late receipt erased a newer route epoch" + ); + assert_eq!(metrics.control_completion_queued.load(Ordering::Acquire), 0); + } + + #[tokio::test] + async fn retracted_management_ack_receipt_retires_without_closing_transport() { + cancelled_receipt(false).await; + } + #[tokio::test] + async fn retracted_tool_ack_receipt_retires_without_closing_transport() { + cancelled_receipt(true).await; + } + + #[tokio::test] + async fn queued_ack_deadline_refuses_but_writing_ack_has_already_succeeded() { + let (_dir, root) = test_support::test_root("ack-deadline-stages"); + for stage in [RESERVED, QUEUED, WRITING] { + let writing = stage == WRITING; + let route = route_key(7, 1); + let (writer, mut frames) = mpsc::channel(8); + let (done, mut completed) = mpsc::channel(8); + let metrics = Arc::new(DispatchPathMetrics::new()); + let a0 = A0Egress::new(writer.clone(), done); + let publication = a0 + .reserve(route, PROTOCOL_VERSION, 891, control_flags(), &metrics) + .unwrap() + .commit(); + let mut ack = if stage != RESERVED { + assert!(a0 + .enqueue_ack( + route, + route_bind_ack_frame(PROTOCOL_VERSION, 891, control_flags()).unwrap(), + ControlCompletion::ManagementAckWritten { + route, + principal: None, + corr: 891 + }, + &metrics + ) + .is_ok()); + Some(frames.recv().await.unwrap()) + } else { + None + }; + if writing { + assert!(ack.as_mut().unwrap().begin_ack_write()); + } + let mut pending = HashMap::from([( + route, + PendingBind { + ack_publication: Some(publication.clone()), + bind_root_id: root.clone(), + inserted_new_actor: false, + cancelled: false, + configure_request_id: "ack-deadline".into(), + started_at: Instant::now() - ROUTE_BIND_DEADLINE - Duration::from_millis(1), + warned_half_deadline: false, + deadline_reported: false, + corr: 891, + ver: PROTOCOL_VERSION, + flags: control_flags(), + cancellation: JobCancellation::new(), + }, + )]); + expire_overdue_route_binds( + &writer, + &a0, + &Arc::new(Executor::new()), + &mut pending, + &mut HashMap::from([(7, 1)]), + &metrics, + ) + .await + .unwrap(); + assert_eq!(pending[&route].deadline_reported, !writing); + let mut ack = ack.take().unwrap_or_else(|| { + frames + .try_recv() + .expect("RESERVED bind deadline did not queue an error") + }); + assert!( + frames.try_recv().is_err(), + "deadline must not create a second terminal" + ); + if !writing { + assert!(ack.begin_ack_write()); + assert_eq!(ack.header.ty, FrameType::Error); + let body: Value = serde_json::from_slice(&ack.body).unwrap(); + assert_eq!(body["code"], "actor_not_ready"); + assert!(body["message"] + .as_str() + .unwrap() + .contains(if stage == RESERVED { + "configure did not finish" + } else { + "ACK writer queue" + })); + } else { + assert_eq!(ack.header.ty, FrameType::Response); + } + ack.finish_ack_write(true); + assert_eq!(publication.outcome(), Some(writing)); + let completion = completed.recv().await.unwrap(); + assert_eq!( + matches!(completion, ControlCompletion::AckCancelled(_)), + !writing + ); + } + } + + #[tokio::test] + async fn receipt_waiters_keep_their_slot_after_completion_is_consumed() { + let (writer, mut frames) = mpsc::channel(1); + let (done, mut completed) = mpsc::channel(1); + let metrics = Arc::new(DispatchPathMetrics::new()); + let mut a0 = A0Egress::new(writer, done); + let gate = Arc::new(WaiterGate { + entered: tokio::sync::Semaphore::new(0), + release: tokio::sync::Semaphore::new(0), + }); + a0.waiter_gate = Some(gate.clone()); + for channel in 1..=WRITER_QUEUE_CAPACITY as u16 { + let route = route_key(channel, 1); + a0.reserve( + route, + PROTOCOL_VERSION, + channel as u64, + control_flags(), + &metrics, + ) + .unwrap() + .commit(); + assert!(a0 + .enqueue_ack( + route, + route_bind_ack_frame(PROTOCOL_VERSION, channel as u64, control_flags()) + .unwrap(), + ControlCompletion::ManagementAckWritten { + route, + principal: None, + corr: channel as u64 + }, + &metrics + ) + .is_ok()); + let mut ack = frames.recv().await.unwrap(); + assert!(ack.begin_ack_write()); + ack.finish_ack_write(true); + completed.recv().await.unwrap(); + a0.retire(route); + } + let entered = gate + .entered + .acquire_many(WRITER_QUEUE_CAPACITY as u32) + .await + .unwrap(); + assert!(a0.publications.lock().unwrap().is_empty()); + assert_eq!( + a0.reserve( + route_key(1000, 1), + PROTOCOL_VERSION, + 1000, + control_flags(), + &metrics + ) + .err() + .unwrap() + .code, + "busy" + ); + drop(entered); + gate.release.add_permits(WRITER_QUEUE_CAPACITY); + } + + #[tokio::test] + async fn full_writer_ack_waits_off_loop_and_closed_writer_never_publishes() { + let (writer, mut frames) = mpsc::channel(1); + let (done, mut completed) = mpsc::channel(2); + let metrics = Arc::new(DispatchPathMetrics::new()); + let a0 = A0Egress::new(writer.clone(), done); + writer + .send( + Frame::build(FrameType::Push, control_flags(), 7, 1, 0, b"{}".to_vec()) + .unwrap() + .into(), + ) + .await + .unwrap(); + let route = route_key(7, 1); + let publication = a0 + .reserve(route, PROTOCOL_VERSION, 891, control_flags(), &metrics) + .unwrap() + .commit(); + assert!(a0 + .enqueue_ack( + route, + route_bind_ack_frame(PROTOCOL_VERSION, 891, control_flags()).unwrap(), + ControlCompletion::ManagementAckWritten { + route, + principal: None, + corr: 891 + }, + &metrics + ) + .is_ok()); + assert!(!publication.0.enqueued.load(Ordering::Acquire)); + assert!(completed.try_recv().is_err()); + frames.recv().await.unwrap(); + tokio::time::timeout(Duration::from_secs(1), publication.wait_enqueued()) + .await + .unwrap() + .unwrap(); + let mut ack = frames.recv().await.unwrap(); + assert!(ack.begin_ack_write()); + ack.finish_ack_write(false); + assert!(matches!( + completed.recv().await, + Some(ControlCompletion::WriteFailed) + )); + assert_eq!(publication.outcome(), None); + assert!(completed.try_recv().is_err()); + } +} + +#[cfg(test)] +mod ordering_tests { + use super::*; + + #[tokio::test] + async fn actual_ack_write_precedes_cross_lane_push_bg_and_candidate_result() { + let (_dir, root) = test_support::test_root("cross-lane-ack-publication"); + let route = route_key(7, 1); + let identity = test_support::route_identity(&root, "session-1"); + let replay_key = push::ReplayKey::from_identity(&identity); + let metrics = Arc::new(DispatchPathMetrics::new()); + let executor = Arc::new(Executor::new()); + let (actual, frames) = mpsc::channel(8); + let (pong, pongs) = mpsc::channel(8); + let (done, mut completed) = mpsc::channel(8); + let a0 = A0Egress::new(actual.clone(), done); + let publication = a0 + .reserve(route, PROTOCOL_VERSION, 891, control_flags(), &metrics) + .unwrap() + .commit(); + let (edge, _ready) = tool_provider::LedgerEdge::new(); + let (proxy, proxy_task) = edge.proxy(actual.clone(), executor.clone(), metrics.clone()); + let (mut peer, socket) = tokio::io::duplex(128 * 1024); + let (entered, entering) = oneshot::channel(); + let (release, released) = oneshot::channel(); + let writer = spawn_writer_task( + crate::subc::bind_ack_fixup_tests::GateWrite { + inner: socket, + armed: Arc::new(AtomicBool::new(true)), + entered: Some(entered), + release: released, + passed: false, + }, + frames, + pongs, + metrics.clone(), + ); + let completion = RouteBindCompletion { + expected_snapshot_id: None, + applied_snapshot_id: None, + route, + identity, + bind_root_id: root.clone(), + inserted_new_actor: false, + configure_response: Response::success("bind", json!({})), + diagnostics_on_edit: false, + ver: PROTOCOL_VERSION, + corr: 891, + flags: control_flags(), + }; + let mut pending = HashMap::from([( + route, + PendingBind { + ack_publication: Some(publication), + bind_root_id: root.clone(), + inserted_new_actor: false, + cancelled: false, + configure_request_id: "bind".into(), + started_at: Instant::now(), + warned_half_deadline: false, + deadline_reported: false, + corr: 891, + ver: PROTOCOL_VERSION, + flags: control_flags(), + cancellation: JobCancellation::new(), + }, + )]); + let mut routes = HashMap::new(); + let mut roots = HashMap::new(); + let mut sessions = HashMap::new(); + let mut buffered = HashMap::from([( + replay_key.clone(), + VecDeque::from([crate::subc::test_support::completion_frame( + "cross-lane-replay", + )]), + )]); + let mut live = HashMap::new(); + let mut installed = HashMap::from([(7, 1)]); + let mut management = HashMap::new(); + let shutdown = Arc::new(Notify::new()); + handle_route_bind_completion( + &proxy, + completion, + false, + &a0, + &mut routes, + &mut roots, + &mut sessions, + &mut buffered, + &mut live, + &mut pending, + &mut installed, + &executor, + &shutdown, + &metrics, + None, + ) + .await + .unwrap(); + tokio::time::timeout(Duration::from_secs(5), entering) + .await + .expect("ACK never reached actual writer") + .unwrap(); + assert!( + routes.is_empty(), + "a queued ACK is not publication authority" + ); + assert_eq!( + buffered[&replay_key].len(), + 1, + "buffer ownership must survive the blocked ACK" + ); + assert!(completed.try_recv().is_err()); + let mut candidates = bind_candidates::CandidateRequests::default(); + for corr in [1, 2] { + let frame = DecodedFrame { + frame: Frame::build( + FrameType::Request, + control_flags(), + 7, + 1, + corr, + b"{}".to_vec(), + ) + .unwrap(), + phase_trace: PhaseTrace::new(Instant::now()), + parsed_body: Default::default(), + confinement: None, + codemode_policies: None, + launch_restore: None, + }; + assert!(candidates + .intercept(frame, &a0, &mut pending, &executor, &metrics) + .await + .unwrap() + .is_none()); + } + candidates + .release(&a0, &routes, &management, &metrics) + .await + .unwrap(); + assert!( + candidates.next_ready().is_none(), + "no candidate work may execute while ACK write is blocked" + ); + release.send(()).unwrap(); + let completion = tokio::time::timeout(Duration::from_secs(5), completed.recv()) + .await + .expect("actual ACK write produced no completion") + .unwrap(); + process_route_bind_completion( + &proxy, + completion, + &a0, + &mut management, + &mut routes, + &mut roots, + &mut sessions, + &mut buffered, + &mut live, + &mut pending, + &mut installed, + &executor, + &shutdown, + &metrics, + None, + ) + .await + .unwrap(); + candidates + .release(&a0, &routes, &management, &metrics) + .await + .unwrap(); + let fast = candidates.next_ready().unwrap(); + assert_eq!(fast.frame.header.corr, 1); + send_reliable_writer_frame( + &proxy, + &metrics, + Frame::build( + FrameType::Response, + control_flags(), + 7, + 1, + 1, + b"{}".to_vec(), + ) + .unwrap(), + "fast candidate result", + ) + .await + .unwrap(); + let subscription = candidates.next_ready().unwrap(); + assert_eq!(subscription.frame.header.corr, 2); + let mut wakes = BgWakePending::from([(route, BgWakeState::armed(Instant::now()))]); + let subscriptions = HashMap::from([( + route, + BgSub { + corr: 2, + ver: PROTOCOL_VERSION, + flags: control_flags(), + root, + session: "session-1".into(), + }, + )]); + push::emit_bg_event_wakes(&proxy, &metrics, &subscriptions, &mut wakes); + let ack = tokio::time::timeout(Duration::from_secs(5), read_frame(&mut peer)) + .await + .expect("ACK not written") + .unwrap() + .unwrap(); + assert_eq!((ack.header.channel, ack.header.corr), (0, 891)); + let mut seen = HashSet::new(); + for _ in 0..3 { + let frame = tokio::time::timeout(Duration::from_secs(5), read_frame(&mut peer)) + .await + .expect("cross-lane replay not written") + .unwrap() + .unwrap(); + assert_eq!(frame.header.channel, 7); + seen.insert((frame.header.ty as u8, frame.header.corr)); + } + assert!(seen.contains(&(FrameType::Push as u8, 0))); + assert!(seen.contains(&(FrameType::Response as u8, 1))); + assert!(seen.contains(&(FrameType::StreamData as u8, 2))); + drop(proxy); + proxy_task.await.unwrap(); + drop(a0); + drop(actual); + drop(pong); + writer.await.unwrap().unwrap(); + } + + #[test] + fn production_control_lane_and_completion_priority_are_explicit() { + let source = include_str!("mod.rs"); + let body = source + .split_once("async fn run_module_loop<") + .unwrap() + .1 + .split_once("fn classify_connection_end(") + .unwrap() + .0 + .split_whitespace() + .collect::(); + assert_eq!(body.matches("handle_control_request(").count(), 1); + assert!(body.contains("handle_control_request(&a0.writer,&writer_tx,&a0,")); + assert!(body.contains("expire_overdue_route_binds(&a0.writer,&a0,")); + assert!(body.contains("handle_management_request(&a0.writer,")); + assert!(body.contains("let(configure_completion_tx,mutconfigure_completion_rx)=")); + assert!(body.contains("&configure_completion_tx,")); + let drain = body + .find("matchdrain_pending_route_bind_completions(") + .unwrap(); + assert!(drain < body.find("push::drain_lossy_push_batch(").unwrap()); + assert!(body.contains("tokio::select!{biased;Some(ended)=scopes_ended_rx.recv()")); + let select = body.find("tokio::select!{biased;Some(ended)").unwrap(); + assert!( + select + < body + .find("completion=control_completion_rx.recv()") + .unwrap() + ); + assert!( + body.find("completion=control_completion_rx.recv()") + .unwrap() + < body + .find("(maybe_frame,database_waited,ledger_waited)") + .unwrap() + ); + let egress = include_str!("a0_egress.rs"); + let publish = egress + .split_once("pub(super) fn publish(self, cancelled: bool)") + .unwrap() + .1 + .split_once("struct PublicationBudget") + .unwrap() + .0; + assert!(publish.contains("self.permit.send(completion)")); + assert!(!publish.contains("tokio::spawn")); + assert!(!publish.contains(".await")); + let queue = egress + .split_once(" fn queue(") + .unwrap() + .1 + .split_once("impl Drop for A0Egress") + .unwrap() + .0; + assert!( + queue.find("set_ack_completion(").unwrap() + < queue.find("try_enqueue_writer_item(").unwrap() + ); + } } diff --git a/crates/aft/src/subc/bind_ack_fixup_tests.rs b/crates/aft/src/subc/bind_ack_fixup_tests.rs new file mode 100644 index 0000000000..d39b84ea16 --- /dev/null +++ b/crates/aft/src/subc/bind_ack_fixup_tests.rs @@ -0,0 +1,287 @@ +use super::*; +use std::future::Future; +use std::pin::Pin; +use std::task::{Context, Poll}; + +pub(super) struct GateWrite { + pub(super) inner: W, + pub(super) armed: Arc, + pub(super) entered: Option>, + pub(super) release: oneshot::Receiver<()>, + pub(super) passed: bool, +} + +impl AsyncWrite for GateWrite { + fn poll_write( + mut self: Pin<&mut Self>, + cx: &mut Context<'_>, + bytes: &[u8], + ) -> Poll> { + if self.armed.load(Ordering::Acquire) && !self.passed { + if let Some(entered) = self.entered.take() { + let _ = entered.send(()); + } + match Pin::new(&mut self.release).poll(cx) { + Poll::Pending => return Poll::Pending, + Poll::Ready(_) => self.passed = true, + } + } + Pin::new(&mut self.inner).poll_write(cx, bytes) + } + fn poll_flush(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + Pin::new(&mut self.inner).poll_flush(cx) + } + fn poll_shutdown(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + Pin::new(&mut self.inner).poll_shutdown(cx) + } +} + +fn bind(root: &Path, tool: bool, epoch: u32, corr: u64) -> Frame { + let request = ModuleControlRequest::RouteBind { + route_channel: 7, + epoch, + target: if tool { + RouteTarget::ToolProvider { + module_id: "aft".into(), + } + } else { + RouteTarget::ManagementSurface { + module_id: "aft".into(), + } + }, + identity: subc_protocol::BindIdentity::new(root, "opencode", "retracted-bind"), + principal: Some(subc_protocol::Principal::Direct), + consumer_capabilities: None, + admission_facts: Default::default(), + scope: None, + role_versions: None, + call_mode: None, + }; + Frame::build( + FrameType::Request, + control_flags(), + 0, + 0, + corr, + serde_json::to_vec(&request).unwrap(), + ) + .unwrap() +} + +async fn retracted_bind(tool: bool, trigger: &str, expected_code: &str) { + let (dir, root) = test_support::test_root("retracted-control-bind"); + let app = App::default_shared(); + let executor = Arc::new(Executor::new()); + let storage = dir.path().join("storage"); + if tool { + let ctx = Arc::new(crate::context::AppContext::from_app( + app.clone(), + crate::config::Config { + storage_dir: Some(storage.clone()), + ..Default::default() + }, + )); + assert!(executor.register_actor(root.clone(), ctx)); + } + let user = dir.path().join("user.jsonc"); + std::fs::write(&user,r#"{"indexes":{"trigram":false,"semantic":false,"callgraph":false},"views":{"enabled":false}}"#).unwrap(); + let (mut daemon, module) = tokio::io::duplex(128 * 1024); + let (read, write) = tokio::io::split(module); + let armed = Arc::new(AtomicBool::new(false)); + let (entered, entering) = oneshot::channel(); + let (release, released) = oneshot::channel(); + let frames = Arc::new(AtomicUsize::new(0)); + let metrics = Arc::new(DispatchPathMetrics::new()); + let (events, mut observed) = mpsc::unbounded_channel(); + let probe = SubcTestLifecycleProbe::new(events); + let before = frames.clone(); + let hooks = stall_watchdog::FrameLoopTestHooks { + metrics: metrics.clone(), + config: None, + before_frame: Arc::new(move || { + before.fetch_add(1, Ordering::Release); + }), + before_turn_work: None, + writer_setup: None, + }; + let gated = GateWrite { + inner: write, + armed: armed.clone(), + entered: Some(entered), + release: released, + passed: false, + }; + let module = std::thread::spawn(move || { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap(); + runtime.block_on(run_module_loop( + read, + gated, + &storage.join("missing-connection.json"), + app, + executor, + |request, ctx| crate::commands::configure::handle_configure(&request, ctx), + Some(user), + false, + usize::MAX, + Some(probe), + &storage, + QuiescedLsp::ReapInBackground, + None, + Some(hooks), + )) + }); + let hello = read_frame(&mut daemon).await.unwrap().unwrap(); + assert_eq!(hello.header.ty, FrameType::Hello); + write_frame( + &mut daemon, + &Frame::build( + FrameType::HelloAck, + control_flags(), + 0, + 0, + HELLO_CORR, + b"{}".to_vec(), + ) + .unwrap(), + ) + .await + .unwrap(); + armed.store(true, Ordering::Release); + write_frame( + &mut daemon, + &Frame::build(FrameType::Ping, control_flags(), 0, 0, 1, Vec::new()).unwrap(), + ) + .await + .unwrap(); + entering.await.unwrap(); + write_frame(&mut daemon, &bind(root.as_path(), tool, 1, 891)) + .await + .unwrap(); + let queued = tokio::time::timeout(Duration::from_secs(15), async { + loop { + if let Some(SubcLifecycleEvent::RouteBindAckQueued { corr: 891, .. }) = + observed.recv().await + { + break; + } + } + }) + .await; + if queued.is_ok() { + let frame = match trigger { + "goodbye" => { + Frame::build(FrameType::Goodbye, control_flags(), 7, 1, 0, Vec::new()).unwrap() + } + "replacement" => bind(root.as_path(), tool, 2, 892), + "stop" => { + Frame::build(FrameType::Goodbye, control_flags(), 0, 0, 0, Vec::new()).unwrap() + } + _ => unreachable!(), + }; + write_frame(&mut daemon, &frame).await.unwrap(); + if trigger != "stop" { + write_frame( + &mut daemon, + &Frame::build( + FrameType::Request, + control_flags(), + 0, + 0, + 999, + serde_json::to_vec(&ModuleControlRequest::HealthCheck {}).unwrap(), + ) + .unwrap(), + ) + .await + .unwrap(); + } + } + let handled = tokio::time::timeout(Duration::from_secs(5), async { + loop { + if trigger == "stop" { + if frames.load(Ordering::Acquire) >= 3 { + break; + } + } else if frames.load(Ordering::Acquire) >= 4 { + break; + } + tokio::task::yield_now().await; + } + }) + .await; + let _ = release.send(()); + let mut terminals = Vec::new(); + let read_result = tokio::time::timeout(Duration::from_secs(8), async { + while let Some(frame) = read_frame(&mut daemon).await.unwrap() { + if frame.header.channel == 0 && frame.header.corr == 891 { + terminals.push(frame); + } else if trigger != "stop" + && frame.header.ty == FrameType::Response + && frame.header.corr == 999 + { + break; + } + } + }) + .await; + drop(daemon); + let result = module.join().unwrap(); + queued.unwrap_or_else(|error| { + panic!( + "ACK never queued: {error:?}; terminals={terminals:?}; loop={}; frames={}", + metrics.frame_loop_context(), + frames.load(Ordering::Acquire) + ) + }); + handled.expect("retraction trigger never handled"); + read_result.expect("writer never drained"); + result.unwrap(); + assert_eq!( + terminals.len(), + 1, + "an open bind control request needs one terminal" + ); + assert_eq!(terminals[0].header.ty, FrameType::Error); + assert_eq!( + serde_json::from_slice::(&terminals[0].body).unwrap()["code"], + expected_code + ); + let expected_message = match trigger { + "goodbye" => "request cancelled", + "replacement" => "route bind superseded by a newer epoch", + "stop" => "module is reloading", + _ => unreachable!(), + }; + assert_eq!( + serde_json::from_slice::(&terminals[0].body).unwrap()["message"], + expected_message + ); +} + +#[tokio::test] +async fn retracted_bind_management_goodbye_has_one_control_error() { + retracted_bind(false, "goodbye", "cancelled").await; +} +#[tokio::test] +async fn retracted_bind_management_replacement_has_one_control_error() { + retracted_bind(false, "replacement", "bind_superseded").await; +} +#[tokio::test] +async fn retracted_bind_management_stop_has_one_control_error() { + retracted_bind(false, "stop", "module_reloading").await; +} +#[tokio::test] +async fn retracted_bind_tool_goodbye_has_one_control_error() { + retracted_bind(true, "goodbye", "cancelled").await; +} +#[tokio::test] +async fn retracted_bind_tool_replacement_has_one_control_error() { + retracted_bind(true, "replacement", "bind_superseded").await; +} +#[tokio::test] +async fn retracted_bind_tool_stop_has_one_control_error() { + retracted_bind(true, "stop", "module_reloading").await; +} diff --git a/crates/aft/src/subc/bind_candidates.rs b/crates/aft/src/subc/bind_candidates.rs new file mode 100644 index 0000000000..978f310864 --- /dev/null +++ b/crates/aft/src/subc/bind_candidates.rs @@ -0,0 +1,523 @@ +//! Candidate requests are not executed until the bind ACK has finished writing. +//! If the bind is refused, their errors follow its control-channel error. + +use super::*; + +const HELD_REQUESTS: usize = 256; +const HELD_BYTES: usize = 64 * 1024 * 1024; + +#[derive(Default)] +pub(super) struct CandidateRequests { + held: HashMap)>, + count: usize, + bytes: usize, + ready: VecDeque, + closing: HashMap, + cancelled: HashSet<(RouteChannel, u64)>, +} + +impl CandidateRequests { + pub(super) fn next_ready(&mut self) -> Option { + let frame = self.ready.pop_front()?; + self.count -= 1; + self.bytes -= frame.frame.body.len(); + Some(frame) + } + fn take(&mut self, route: RouteChannel) -> VecDeque { + let frames = self + .held + .remove(&route) + .map(|(_, frames)| frames) + .unwrap_or_default(); + self.count -= frames.len(); + self.bytes -= frames + .iter() + .map(|frame| frame.frame.body.len()) + .sum::(); + frames + } + + pub(super) async fn intercept( + &mut self, + frame: DecodedFrame, + a0: &A0Egress, + pending: &mut HashMap, + executor: &Arc, + metrics: &Arc, + ) -> Result, SubcError> { + if frame.frame.header.ty == FrameType::Cancel { + let header = &frame.frame.header; + let route = route_key(header.channel, header.epoch); + if self.held.get(&route).is_some_and(|(_, frames)| { + frames + .iter() + .any(|held| held.frame.header.corr == header.corr) + }) { + self.cancelled.insert((route, header.corr)); + return Ok(None); + } + } + if frame.frame.header.ty != FrameType::Request || frame.frame.header.channel == 0 { + return Ok(Some(frame)); + } + let route = route_key(frame.frame.header.channel, frame.frame.header.epoch); + let Some(publication) = a0.publication(route) else { + return Ok(Some(frame)); + }; + if !publication.is_retracted() + && self.count < HELD_REQUESTS + && frame.frame.body.len() <= HELD_BYTES.saturating_sub(self.bytes) + { + self.count += 1; + self.bytes += frame.frame.body.len(); + self.held + .entry(route) + .or_insert_with(|| (publication, VecDeque::new())) + .1 + .push_back(frame); + return Ok(None); + } + + // An ACK already writing has won admission. Overflow can retract only + // a bind whose ACK has not started writing; it must not emit both an + // ACK and an error for the same channel-0 request. + let rejected = a0.reject( + route, + ErrorBody::new("busy", a0_egress::BIND_BUSY_MESSAGE) + .with_detail(json!({"retryable":true})), + ); + if rejected { + if let Some(bind) = pending.get_mut(&route) { + bind.cancelled = true; + executor.cancel_job(&bind.bind_root_id, &bind.cancellation); + } + } + publication.wait_enqueued().await?; + if publication.is_retracted() { + let frames = self.take(route); + for held in frames { + self.refuse(&a0.writer, held, metrics).await?; + } + } + self.refuse(&a0.writer, frame, metrics).await?; + if rejected { + self.closing.insert(route, publication); + } + Ok(None) + } + + pub(super) async fn release( + &mut self, + a0: &A0Egress, + routes: &HashMap, + management: &HashMap>, + metrics: &Arc, + ) -> Result<(), SubcError> { + let resolved: Vec<_> = self + .held + .iter() + .filter_map(|(route, (publication, _))| { + publication.outcome().and_then(|success| { + (!success || routes.contains_key(route) || management.contains_key(route)) + .then_some((*route, success)) + }) + }) + .collect(); + for (route, success) in resolved { + let frames = self.take(route); + if success { + for frame in frames { + if self.cancelled.contains(&(route, frame.frame.header.corr)) { + self.refuse(&a0.writer, frame, metrics).await?; + } else { + self.count += 1; + self.bytes += frame.frame.body.len(); + self.ready.push_back(frame); + } + } + } else { + for frame in frames { + self.refuse(&a0.writer, frame, metrics).await?; + } + } + } + let closed: Vec<_> = self + .closing + .iter() + .filter_map(|(route, publication)| publication.outcome().map(|_| *route)) + .collect(); + for route in closed { + self.closing.remove(&route); + send_reliable_writer_frame( + &a0.writer, + metrics, + build_goodbye_frame(PROTOCOL_VERSION, route.channel, route.epoch, 0)?, + "candidate bind overflow close", + ) + .await?; + } + Ok(()) + } + + pub(super) async fn stop( + &mut self, + a0: &A0Egress, + metrics: &DispatchPathMetrics, + ) -> Result<(), SubcError> { + let routes: Vec<_> = self.held.keys().copied().collect(); + for route in routes { + self.held[&route].0.wait_enqueued().await?; + for frame in self.take(route) { + self.refuse(&a0.writer, frame, metrics).await?; + } + } + while let Some(frame) = self.next_ready() { + self.refuse(&a0.writer, frame, metrics).await?; + } + Ok(()) + } + + async fn refuse( + &mut self, + tx: &WriterSender, + frame: DecodedFrame, + metrics: &DispatchPathMetrics, + ) -> Result<(), SubcError> { + let header = frame.frame.header; + let (code, message) = if self + .cancelled + .remove(&(route_key(header.channel, header.epoch), header.corr)) + { + ("cancelled", "request cancelled") + } else { + ("route_not_bound", "route is not bound before tool call") + }; + send_reliable_writer_frame( + tx, + metrics, + build_error_frame( + header.ver, + header.channel, + header.epoch, + header.corr, + header.flags, + code, + message, + )?, + "candidate bind refusal", + ) + .await + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn request(corr: u64) -> DecodedFrame { + DecodedFrame { + frame: Frame::build( + FrameType::Request, + control_flags(), + 7, + 1, + corr, + b"{}".to_vec(), + ) + .unwrap(), + phase_trace: PhaseTrace::new(Instant::now()), + parsed_body: Default::default(), + confinement: None, + codemode_policies: None, + launch_restore: None, + } + } + + fn egress( + capacity: usize, + ) -> ( + A0Egress, + mpsc::Receiver, + mpsc::Receiver, + Arc, + ) { + let (writer, frames) = mpsc::channel(capacity); + let (completed, completions) = mpsc::channel(WRITER_QUEUE_CAPACITY); + let metrics = Arc::new(DispatchPathMetrics::new()); + let a0 = A0Egress::new(writer, completed); + a0.reserve( + route_key(7, 1), + PROTOCOL_VERSION, + 891, + control_flags(), + &metrics, + ) + .unwrap() + .commit(); + (a0, frames, completions, metrics) + } + + #[tokio::test] + async fn candidate_calls_wait_through_configure_queue_and_actual_ack_write() { + let (a0, mut frames, mut completions, metrics) = egress(8); + let mut candidates = CandidateRequests::default(); + let executor = Arc::new(Executor::new()); + let mut pending = HashMap::new(); + assert!(candidates + .intercept(request(1), &a0, &mut pending, &executor, &metrics) + .await + .unwrap() + .is_none()); + assert!( + frames.try_recv().is_err(), + "configure has not produced an ACK" + ); + assert!(a0 + .enqueue_ack( + route_key(7, 1), + route_bind_ack_frame(PROTOCOL_VERSION, 891, control_flags()).unwrap(), + ControlCompletion::ManagementAckWritten { + route: route_key(7, 1), + principal: None, + corr: 891 + }, + &metrics + ) + .is_ok()); + candidates + .intercept(request(2), &a0, &mut pending, &executor, &metrics) + .await + .unwrap(); + let mut ack = frames.recv().await.unwrap(); + assert!(ack.begin_ack_write()); + let management = HashMap::from([(route_key(7, 1), None)]); + candidates + .release(&a0, &HashMap::new(), &management, &metrics) + .await + .unwrap(); + assert!( + candidates.next_ready().is_none(), + "write-start is not completed write" + ); + assert!(completions.try_recv().is_err()); + ack.finish_ack_write(true); + completions.recv().await.unwrap(); + candidates + .release(&a0, &HashMap::new(), &management, &metrics) + .await + .unwrap(); + assert_eq!( + candidates.count, 2, + "ready frames retain their admission credit" + ); + assert_eq!(candidates.next_ready().unwrap().frame.header.corr, 1); + assert_eq!(candidates.next_ready().unwrap().frame.header.corr, 2); + assert!(candidates.next_ready().is_none()); + assert_eq!(candidates.count, 0); + assert_eq!(candidates.bytes, 0); + } + + #[tokio::test] + async fn cancelled_candidate_never_executes_or_answers_twice_after_ack() { + let (a0, mut frames, mut completed, metrics) = egress(8); + let mut candidates = CandidateRequests::default(); + let executor = Arc::new(Executor::new()); + candidates + .intercept(request(1), &a0, &mut HashMap::new(), &executor, &metrics) + .await + .unwrap(); + let mut cancel = request(1); + cancel.frame = + Frame::build(FrameType::Cancel, control_flags(), 7, 1, 1, Vec::new()).unwrap(); + assert!(candidates + .intercept(cancel, &a0, &mut HashMap::new(), &executor, &metrics) + .await + .unwrap() + .is_none()); + assert!(a0 + .enqueue_ack( + route_key(7, 1), + route_bind_ack_frame(PROTOCOL_VERSION, 891, control_flags()).unwrap(), + ControlCompletion::ManagementAckWritten { + route: route_key(7, 1), + principal: None, + corr: 891 + }, + &metrics + ) + .is_ok()); + let mut ack = frames.recv().await.unwrap(); + assert!(ack.begin_ack_write()); + ack.finish_ack_write(true); + completed.recv().await.unwrap(); + let management = HashMap::from([(route_key(7, 1), None)]); + candidates + .release(&a0, &HashMap::new(), &management, &metrics) + .await + .unwrap(); + assert!( + candidates.next_ready().is_none(), + "cancelled candidate escaped to execution" + ); + let terminal = frames.try_recv().expect("one cancelled candidate terminal"); + assert_eq!(terminal.header.corr, 1); + let body: Value = serde_json::from_slice(&terminal.body).unwrap(); + assert_eq!(body["code"], "cancelled"); + assert_eq!(body["message"], "request cancelled"); + candidates + .release(&a0, &HashMap::new(), &management, &metrics) + .await + .unwrap(); + assert!(frames.try_recv().is_err()); + assert!(candidates.cancelled.is_empty()); + assert_eq!(candidates.count, 0); + } + + #[tokio::test] + async fn failed_bind_answers_held_requests_after_its_terminal_in_arrival_order() { + let (a0, mut frames, mut completions, metrics) = egress(8); + let mut candidates = CandidateRequests::default(); + for corr in [1, 2, 3] { + candidates + .intercept( + request(corr), + &a0, + &mut HashMap::new(), + &Arc::new(Executor::new()), + &metrics, + ) + .await + .unwrap(); + } + assert!(a0.reject( + route_key(7, 1), + ErrorBody::new("config_divergence", "configure failed during route bind") + )); + candidates + .release(&a0, &HashMap::new(), &HashMap::new(), &metrics) + .await + .unwrap(); + let mut terminal = frames.try_recv().expect("bind refusal was not queued"); + assert!(terminal.begin_ack_write()); + assert_eq!(terminal.header.channel, 0); + assert_eq!(terminal.header.corr, 891); + assert_eq!(terminal.header.ty, FrameType::Error); + assert!( + frames.try_recv().is_err(), + "held calls escaped before bind terminal" + ); + terminal.finish_ack_write(true); + completions.recv().await.unwrap(); + candidates + .release(&a0, &HashMap::new(), &HashMap::new(), &metrics) + .await + .unwrap(); + for corr in [1, 2, 3] { + let refused = frames.recv().await.unwrap(); + assert_eq!(refused.header.corr, corr); + let body: Value = serde_json::from_slice(&refused.body).unwrap(); + assert_eq!(body["code"], "route_not_bound"); + assert_eq!(body["message"], "route is not bound before tool call"); + } + assert!(frames.try_recv().is_err()); + } + + #[tokio::test] + async fn candidate_overflow_fails_bind_then_refuses_every_owner_before_close() { + let (a0, mut frames, mut completions, metrics) = egress(HELD_REQUESTS + 8); + let mut candidates = CandidateRequests::default(); + let executor = Arc::new(Executor::new()); + for corr in 1..=HELD_REQUESTS as u64 + 2 { + candidates + .intercept(request(corr), &a0, &mut HashMap::new(), &executor, &metrics) + .await + .unwrap(); + } + assert_eq!(candidates.count, 0); + let mut terminal = frames.try_recv().expect("bind refusal was not queued"); + assert!(terminal.begin_ack_write()); + assert_eq!(terminal.header.corr, 891); + let body: Value = serde_json::from_slice(&terminal.body).unwrap(); + assert_eq!(body["code"], "busy"); + assert_eq!(body["message"], a0_egress::BIND_BUSY_MESSAGE); + terminal.finish_ack_write(true); + completions.recv().await.unwrap(); + candidates + .release(&a0, &HashMap::new(), &HashMap::new(), &metrics) + .await + .unwrap(); + for corr in 1..=HELD_REQUESTS as u64 + 2 { + let refused = frames.recv().await.unwrap(); + assert_eq!(refused.header.ty, FrameType::Error); + assert_eq!(refused.header.channel, 7); + assert_eq!(refused.header.corr, corr); + } + assert_eq!(frames.recv().await.unwrap().header.ty, FrameType::Goodbye); + assert!(frames.try_recv().is_err()); + } + + #[tokio::test] + async fn candidate_wire_bytes_are_bounded_before_count_capacity() { + let (a0, mut frames, mut completed, metrics) = egress(16); + let mut candidates = CandidateRequests::default(); + let executor = Arc::new(Executor::new()); + for corr in 1..=9 { + let mut frame = request(corr); + frame.frame = Frame::build( + FrameType::Request, + control_flags(), + 7, + 1, + corr, + vec![b' '; 8 * 1024 * 1024], + ) + .unwrap(); + candidates + .intercept(frame, &a0, &mut HashMap::new(), &executor, &metrics) + .await + .unwrap(); + } + assert_eq!(candidates.count, 0, "byte overflow did not reject the bind"); + assert_eq!(candidates.bytes, 0); + let mut terminal = frames + .try_recv() + .expect("byte overflow did not queue a bind refusal"); + assert!(terminal.begin_ack_write()); + assert_eq!(terminal.header.channel, 0); + let body: Value = serde_json::from_slice(&terminal.body).unwrap(); + assert_eq!(body["code"], "busy"); + terminal.finish_ack_write(true); + completed.recv().await.unwrap(); + candidates + .release(&a0, &HashMap::new(), &HashMap::new(), &metrics) + .await + .unwrap(); + for corr in 1..=9 { + assert_eq!(frames.try_recv().unwrap().header.corr, corr); + } + assert_eq!(frames.try_recv().unwrap().header.ty, FrameType::Goodbye); + } + + #[test] + fn candidate_interception_precedes_all_admission_and_ready_frames_precede_new_ingress() { + let source = include_str!("mod.rs"); + let body = source + .split_once("async fn run_module_loop<") + .unwrap() + .1 + .split_once("fn classify_connection_end(") + .unwrap() + .0 + .split_whitespace() + .collect::(); + assert_eq!(body.matches("candidate_requests.intercept(").count(), 1); + let hold = body.find("candidate_requests.intercept(").unwrap(); + assert!(hold < body.find("!ingress_route_should_be_processed(").unwrap()); + assert!(hold < body.find("codemode::admit(").unwrap()); + assert!( + body.find("candidate_requests.next_ready()").unwrap() + < body.find("frame=reader_rx.recv()").unwrap() + ); + assert!(body.contains("candidate_requests.release(&a0,&routes,&management_routes")); + } +} diff --git a/crates/aft/src/subc/bind_capacity_tests.rs b/crates/aft/src/subc/bind_capacity_tests.rs new file mode 100644 index 0000000000..b16b39212b --- /dev/null +++ b/crates/aft/src/subc/bind_capacity_tests.rs @@ -0,0 +1,161 @@ +use super::*; + +async fn exhausted_bind(tool: bool, completion_capacity: bool) { + let (dir, root) = test_support::test_root("bind-capacity-before-effects"); + let user = dir.path().join("must-not-migrate.jsonc"); + let original = + "{\n // my settings\n \"search_index\": false,\n \"hoist_builtin_tools\": false\n}\n"; + std::fs::write(&user, original).unwrap(); + let app = App::default_shared(); + let executor = Arc::new(Executor::new()); + let metrics = Arc::new(DispatchPathMetrics::new()); + let (writer, mut frames) = mpsc::channel(8); + let (completed, _completions) = mpsc::channel(if completion_capacity { + 1 + } else { + WRITER_QUEUE_CAPACITY + 1 + }); + let a0 = A0Egress::new(writer.clone(), completed.clone()); + let completion_permit = + completion_capacity.then(|| completed.clone().try_reserve_owned().unwrap()); + let mut slots = Vec::new(); + if !completion_capacity { + for channel in 10..10 + WRITER_QUEUE_CAPACITY as u16 { + slots.push( + a0.reserve( + route_key(channel, 1), + PROTOCOL_VERSION, + channel as u64, + control_flags(), + &metrics, + ) + .unwrap(), + ); + } + } + let request = ModuleControlRequest::RouteBind { + route_channel: 7, + epoch: 1, + target: if tool { + RouteTarget::ToolProvider { + module_id: "aft".into(), + } + } else { + RouteTarget::ManagementSurface { + module_id: "aft".into(), + } + }, + identity: subc_protocol::BindIdentity::new(root.as_path(), "pi", "capacity"), + principal: Some(Principal::Direct), + consumer_capabilities: None, + admission_facts: Default::default(), + scope: None, + role_versions: None, + call_mode: None, + }; + let frame = Frame::build( + FrameType::Request, + control_flags(), + 0, + 0, + 891, + serde_json::to_vec(&request).unwrap(), + ) + .unwrap(); + let (lossy_tx, _lossy) = mpsc::channel(8); + let (reliable_tx, _reliable) = mpsc::unbounded_channel(); + let senders = PushSenders { + lossy_tx, + reliable_tx, + lossy_overflow: Arc::default(), + lossy_seq: Arc::new(AtomicU64::new(0)), + fleet_status_client: FleetStatusClient::channel(1).0, + }; + let mut live = HashMap::new(); + let mut pending = HashMap::new(); + let mut installed = HashMap::new(); + handle_control_request( + &writer, + &writer, + &a0, + &frame, + &app, + &executor, + &Arc::new(standing::StandingActor::new(app.clone(), executor.clone())), + &mut live, + &mut pending, + &mut installed, + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &mut HashMap::new(), + &Arc::new(StdMutex::new(HashMap::new())), + &mut PendingSubcResponses::default(), + &mut HashMap::new(), + &mut HashMap::new(), + &Arc::new(Notify::new()), + &completed, + &metrics, + None, + &HealthRollupCache::new(), + &senders, + |_, _| panic!("configure must not run without reserved reply capacity"), + Some(&user), + usize::MAX, + None, + ) + .await + .unwrap(); + assert_eq!( + std::fs::read_to_string(&user).unwrap(), + original, + "capacity refusal migrated configuration" + ); + let config_files: Vec<_> = std::fs::read_dir(dir.path()) + .unwrap() + .map(|entry| entry.unwrap().file_name()) + .filter(|name| name.to_string_lossy().contains("must-not-migrate")) + .collect(); + assert_eq!( + config_files, + vec![user.file_name().unwrap().to_os_string()], + "capacity refusal left a migration backup or lock" + ); + assert!( + executor.actor_context(&root).is_none(), + "capacity refusal registered an actor" + ); + assert!(live.is_empty()); + assert!(pending.is_empty()); + assert!(installed.is_empty(), "capacity refusal installed an epoch"); + let terminal = frames.try_recv().expect("exactly one capacity refusal"); + assert_eq!(terminal.header.ty, FrameType::Error); + assert_eq!(terminal.header.corr, 891); + let error: Value = serde_json::from_slice(&terminal.body).unwrap(); + assert_eq!(error["code"], "busy"); + assert_eq!(error["message"], a0_egress::BIND_BUSY_MESSAGE); + assert!(frames.try_recv().is_err()); + drop(slots); + drop(completion_permit); +} + +#[tokio::test] +async fn tool_bind_slot_exhaustion_has_no_configure_side_effects() { + exhausted_bind(true, false).await; +} +#[tokio::test] +async fn tool_bind_completion_exhaustion_has_no_configure_side_effects() { + exhausted_bind(true, true).await; +} +#[tokio::test] +async fn management_bind_slot_exhaustion_has_no_install_side_effects() { + exhausted_bind(false, false).await; +} +#[tokio::test] +async fn management_bind_completion_exhaustion_has_no_install_side_effects() { + exhausted_bind(false, true).await; +} diff --git a/crates/aft/src/subc/mod.rs b/crates/aft/src/subc/mod.rs index 4b6caedce9..6625318c83 100644 --- a/crates/aft/src/subc/mod.rs +++ b/crates/aft/src/subc/mod.rs @@ -199,8 +199,13 @@ type LossyPushEnvelope = (u64, ProjectRootId, PushFrame); type RetryBuffer = HashMap>; mod a0_egress; mod access_grants; +#[cfg(test)] +mod bind_ack_fixup_tests; +#[cfg(test)] +mod bind_capacity_tests; use a0_egress::{A0Egress, ControlCompletion}; mod bash; +mod bind_candidates; mod bind_log; mod call_policy; mod carrier_read_only; @@ -389,15 +394,19 @@ impl SubcTestLifecycleProbe { fn route_bind_ack_queued(&self, corr: u64) { let guard = self.bind_priority.lock().expect("bind priority probe lock"); - if let Some(priority) = guard.as_ref().filter(|priority| priority.corr == corr) { - let batches_at_arrival = priority - .batches_at_arrival - .expect("bind completion arrival observed"); - let _ = self.events_tx.send(SubcLifecycleEvent::RouteBindAckQueued { - corr, - reliable_push_batches: priority.reliable_push_batches - batches_at_arrival, - }); - } + let reliable_push_batches = + if let Some(priority) = guard.as_ref().filter(|priority| priority.corr == corr) { + let batches_at_arrival = priority + .batches_at_arrival + .expect("bind completion arrival observed"); + priority.reliable_push_batches - batches_at_arrival + } else { + 0 + }; + let _ = self.events_tx.send(SubcLifecycleEvent::RouteBindAckQueued { + corr, + reliable_push_batches, + }); } fn attach_decision(&self, attempt: u32, will_retry: bool) { @@ -4762,6 +4771,12 @@ async fn process_route_bind_completion( ControlCompletion::AckWritten(completion) => (completion, false), ControlCompletion::ManagementAckWritten { route, .. } => { a0.retire(route); + remove_installed_route(installed_route_epochs, route); + return Ok(()); + } + ControlCompletion::BindRetracted { route, .. } => { + a0.retire(route); + remove_installed_route(installed_route_epochs, route); return Ok(()); } _ => return Err(SubcError::WriterClosed), @@ -4776,6 +4791,11 @@ async fn process_route_bind_completion( return Ok(()); } ControlCompletion::WriteFailed => return Err(SubcError::WriterClosed), + ControlCompletion::BindRetracted { route, .. } => { + a0.retire(route); + remove_installed_route(installed_route_epochs, route); + return Ok(()); + } }; handle_route_bind_completion( writer_tx, @@ -4800,6 +4820,7 @@ async fn process_route_bind_completion( #[allow(clippy::too_many_arguments)] async fn drain_pending_route_bind_completions( control_completion_rx: &mut mpsc::Receiver, + configure_completion_rx: &mut mpsc::Receiver, a0: &A0Egress, management_routes: &mut HashMap>, writer_tx: &WriterSender, @@ -4816,7 +4837,10 @@ async fn drain_pending_route_bind_completions( lifecycle_probe: Option<&SubcTestLifecycleProbe>, ) -> Result { let mut drained = 0; - while let Ok(completion) = control_completion_rx.try_recv() { + while let Ok(completion) = control_completion_rx + .try_recv() + .or_else(|_| configure_completion_rx.try_recv()) + { process_route_bind_completion( writer_tx, completion, @@ -5031,8 +5055,14 @@ where let (bash_poll_touch_tx, mut bash_poll_touch_rx) = mpsc::channel::(256); let (control_completion_tx, mut control_completion_rx) = mpsc::channel::(256); + // ACK receipts have admission-reserved slots. Configure-ready messages use + // their own bounded channel so those reservations cannot block the very + // work that must consume them. + let (configure_completion_tx, mut configure_completion_rx) = + mpsc::channel::(WRITER_QUEUE_CAPACITY); let a0 = A0Egress::new(raw_writer_tx, control_completion_tx.clone()) .with_probe(lifecycle_probe.clone()); + let mut candidate_requests = bind_candidates::CandidateRequests::default(); let (lossy_tx, mut lossy_rx) = mpsc::channel::(1024); let lossy_overflow = Arc::new(push::LossyOverflow::default()); let lossy_seq = Arc::new(AtomicU64::new(0)); @@ -5170,6 +5200,7 @@ where ); match drain_pending_route_bind_completions( &mut control_completion_rx, + &mut configure_completion_rx, &a0, &mut management_routes, &writer_tx, @@ -5196,6 +5227,12 @@ where Err(error) => break Err(error), } + if let Err(error) = candidate_requests + .release(&a0, &routes, &management_routes, &dispatch_path_metrics) + .await + { + break Err(error); + } if tokio::time::Instant::now() >= next_drain_at { dispatch_path_metrics.mark_frame_loop_step("push::emit_bg_event_wakes", "", 0); push::emit_bg_event_wakes( @@ -5217,6 +5254,7 @@ where ); if let Err(error) = expire_overdue_route_binds( &a0.writer, + &a0, &executor, &mut pending_binds, &mut installed_route_epochs, @@ -5378,7 +5416,12 @@ where } } } - Some(completion) = control_completion_rx.recv() => { + Some(completion) = async { + tokio::select! { biased; + completion = control_completion_rx.recv() => completion, + completion = configure_completion_rx.recv() => completion, + } + } => { dispatch_path_metrics.mark_frame_loop_step("process_route_bind_completion", "route_bind", completion.corr()); if let Err(error) = process_route_bind_completion( &writer_tx, @@ -5401,6 +5444,9 @@ where { break Err(error); } + if let Err(error) = candidate_requests.release(&a0, &routes, &management_routes, &dispatch_path_metrics).await { + break Err(error); + } next_maintenance_at = tokio::time::Instant::now() + DRAIN_TICK_PERIOD; health_rollup_worker.request_refresh(); } @@ -5415,6 +5461,9 @@ where break Ok(ModuleLoopExit::SkipSearchFlush); } (maybe_frame, database_waited, ledger_waited) = async { + if let Some(frame) = candidate_requests.next_ready() { + return (Some(Ok(frame)), false, false); + } tokio::select! { frame = reader_rx.recv() => (frame, false, false), frame = run_ready_rx.recv() => (frame.map(Ok), false, false), @@ -5438,6 +5487,12 @@ where (hooks.before_frame)(); } + let frame = match candidate_requests.intercept(frame, &a0, &mut pending_binds, &executor, &dispatch_path_metrics).await { + Ok(Some(frame)) => frame, + Ok(None) => continue, + Err(error) => break Err(error), + }; + if !(ledger_waited && ledger_edge.is_withdrawn(&frame.frame)) && !ingress_route_should_be_processed( &installed_route_epochs, &reclaimed_routes, @@ -5584,7 +5639,7 @@ where &mut retry_buffer, &mut push_buffer, &shutdown, - &control_completion_tx, + &configure_completion_tx, &dispatch_path_metrics, lifecycle_probe.as_ref(), &health_rollup_cache, @@ -6418,6 +6473,14 @@ where }; a0.stop(); + let loop_result = if matches!(loop_result, Ok(ModuleLoopExit::Graceful)) { + candidate_requests + .stop(&a0, &dispatch_path_metrics) + .await + .and(loop_result) + } else { + loop_result + }; // The exit budget runs from the drain, so the teardown between the loop // ending and the index flush is timed and logged as its own exit phase. let teardown_started = Instant::now(); @@ -7146,7 +7209,11 @@ async fn handle_route_bind_completion( .get(&route_id) .is_some_and(|pending| !pending.cancelled && !pending.deadline_reported) { - if pending_binds[&route_id].ack_publication.is_some() { + if pending_binds[&route_id] + .ack_publication + .as_ref() + .is_some_and(|ack| ack.is_queued() || ack.is_committed() || ack.is_retracted()) + { return Ok(()); } let restore_watcher = live_roots @@ -7218,8 +7285,8 @@ async fn handle_route_bind_completion( } let inserted_new_actor = pending.inserted_new_actor || completion.inserted_new_actor; - // The deadline path already answered (and recorded) an overdue bind, and a - // torn-down bind is never answered. + // Cancelled or overdue binds already own their control error through A0. + // A late configure completion only cleans up; it must not reply again. if !pending.cancelled && !pending.deadline_reported { metrics.record_bind_ack(pending.started_at.elapsed()); } @@ -7282,16 +7349,20 @@ async fn handle_route_bind_completion( let message = response_message(response, fallback); let fatal = response_is_fatal_panic(response); let error_code = route_bind_error_code_for_configure_response(response); - send_route_bind_error_parts( - &a0.writer, - completion.ver, - completion.corr, - completion.flags, - error_code, - &message, - metrics, - ) - .await?; + if a0.publication(route_id).is_some() { + a0.reject(route_id, ErrorBody::new(error_code, &message)); + } else { + send_route_bind_error_parts( + &a0.writer, + completion.ver, + completion.corr, + completion.flags, + error_code, + &message, + metrics, + ) + .await?; + } remove_installed_route(installed_route_epochs, route_id); if fatal { signal_fatal_teardown( @@ -7398,6 +7469,7 @@ async fn handle_route_bind_completion( async fn expire_overdue_route_binds( tx: &WriterSender, + a0: &A0Egress, executor: &Arc, pending_binds: &mut HashMap, installed_route_epochs: &mut HashMap, @@ -7413,7 +7485,10 @@ async fn expire_overdue_route_binds( && pending .ack_publication .as_ref() - .is_none_or(|ack| ack.cancel_before_write())) + // Write-start wins the bind: completing that frame and + // publishing it after the deadline is intentional. Only an + // ACK still waiting in the queue can become a refusal. + .is_none_or(|ack| !ack.is_committed() && !ack.is_retracted())) .then(|| { ( *route, @@ -7423,15 +7498,33 @@ async fn expire_overdue_route_binds( pending.bind_root_id.clone(), pending.configure_request_id.clone(), age, + pending + .ack_publication + .as_ref() + .is_some_and(|ack| ack.is_queued()), ) }) }) .collect(); - for (route, corr, ver, flags, root_id, configure_request_id, age) in expired { + for (route, corr, ver, flags, root_id, configure_request_id, age, ack_queued) in expired { // Read the blockers before cancelling: once cancelled, the bind is no // longer queued and the snapshot can no longer say what held it. let blockers = executor.bind_blocker_snapshot(&root_id, &configure_request_id); + let (code, message) = if ack_queued { + ("actor_not_ready", format!("route bind deadline exceeded after {}ms (deadline {}ms): the bind waited in the ACK writer queue", age.as_millis(), ROUTE_BIND_DEADLINE.as_millis())) + } else { + route_bind_deadline_refusal( + age, + ROUTE_BIND_DEADLINE, + DAEMON_BIND_RELAY_TIMEOUT, + &blockers, + ) + }; + let reserved = a0.publication(route).is_some(); + if reserved && !a0.reject(route, ErrorBody::new(code, &message)) { + continue; + } if let Some(pending) = pending_binds.get_mut(&route) { pending.cancelled = true; pending.deadline_reported = true; @@ -7443,13 +7536,9 @@ async fn expire_overdue_route_binds( remove_installed_route(installed_route_epochs, route); metrics.record_bind_ack(age); let deadline_ms = ROUTE_BIND_DEADLINE.as_millis(); - let (code, message) = route_bind_deadline_refusal( - age, - ROUTE_BIND_DEADLINE, - DAEMON_BIND_RELAY_TIMEOUT, - &blockers, - ); - send_route_bind_error_parts(tx, ver, corr, flags, code, &message, metrics).await?; + if !reserved { + send_route_bind_error_parts(tx, ver, corr, flags, code, &message, metrics).await?; + } log::warn!( "subc attach: route {} bind for root {} exceeded {}ms deadline (configure_request_id={}, code={})", route, @@ -7788,6 +7877,31 @@ async fn handle_control_request( }; let bind_trust = trust_for_bind(&identity.harness, &principal); + if installed_route_epochs + .get(&route_channel) + .is_some_and(|installed| *installed >= epoch) + { + return send_route_bind_error( + tx, + frame, + "config_divergence", + "route bind generation is not newer than the installed generation", + metrics, + ) + .await; + } + let reservation = match a0.reserve( + route_id, + frame.header.ver, + frame.header.corr, + frame.header.flags, + metrics, + ) { + Ok(reservation) => reservation, + Err(refusal) => { + return send_route_bind_error_body(tx, frame, &refusal, metrics).await + } + }; if let RouteTarget::ManagementSurface { module_id } = &target { if let Err(refusal) = flow_read::admit_management(scope.as_ref()) { return send_route_bind_error_body(tx, frame, &refusal, metrics).await; @@ -7823,7 +7937,7 @@ async fn handle_control_request( ) .await; } - a0.cancel(route_key(route_channel, installed_epoch)); + a0.supersede(route_key(route_channel, installed_epoch)); teardown_installed_route( legacy_tx, metrics, @@ -7864,6 +7978,7 @@ async fn handle_control_request( } installed_route_epochs.insert(route_channel, epoch); + let _publication = reservation.commit(); let ack = route_bind_ack_frame(frame.header.ver, frame.header.corr, frame.header.flags)?; if a0 @@ -7928,7 +8043,7 @@ async fn handle_control_request( .await; } }; - a0.cancel(route_key(route_channel, installed_epoch)); + a0.supersede(route_key(route_channel, installed_epoch)); teardown_installed_route( legacy_tx, metrics, @@ -8135,6 +8250,7 @@ async fn handle_control_request( "executor.scheduler/submit_bind", Some(executor.scheduler_lock_diagnostics()), ); + let publication = reservation.commit(); let (configure_rx, configure_cancellation) = executor.submit_bind_cancellable_async( bind_root_id.clone(), configure_request_id.clone(), @@ -8179,7 +8295,7 @@ async fn handle_control_request( pending_binds.insert( route_id, PendingBind { - ack_publication: None, + ack_publication: Some(publication), bind_root_id: bind_root_id.clone(), inserted_new_actor, cancelled: false, @@ -19298,6 +19414,9 @@ mod tests { let mut published_routes = HashMap::new(); let (ack_tx, mut ack_rx) = mpsc::channel(8); let a0 = A0Egress::new(writer_tx.clone(), ack_tx); + a0.reserve(route, PROTOCOL_VERSION, 91, control_flags(), &metrics) + .unwrap() + .commit(); handle_route_bind_completion( &writer_tx, @@ -19487,6 +19606,7 @@ mod tests { expire_overdue_route_binds( &writer_tx, + &A0Egress::new(writer_tx.clone(), mpsc::channel(8).0), &executor, &mut pending_binds, &mut installed_route_epochs, @@ -19582,6 +19702,7 @@ mod tests { expire_overdue_route_binds( &writer_tx, + &A0Egress::new(writer_tx.clone(), mpsc::channel(8).0), &executor, &mut pending_binds, &mut installed_route_epochs, @@ -19803,6 +19924,7 @@ mod tests { } expire_overdue_route_binds( &writer_tx, + &A0Egress::new(writer_tx.clone(), mpsc::channel(8).0), &executor, &mut pending_binds, &mut installed, @@ -20017,6 +20139,7 @@ mod tests { let metrics = Arc::new(DispatchPathMetrics::new()); expire_overdue_route_binds( &writer_tx, + &A0Egress::new(writer_tx.clone(), mpsc::channel(8).0), &executor, &mut pending_binds, &mut installed, diff --git a/crates/aft/src/subc/wire.rs b/crates/aft/src/subc/wire.rs index 24eac91b05..59225c6d87 100644 --- a/crates/aft/src/subc/wire.rs +++ b/crates/aft/src/subc/wire.rs @@ -204,11 +204,13 @@ impl WriterFrame { .as_ref() .is_some_and(|(publication, _)| !publication.begin_write()) { - if let Some(completion) = self.ack_completion.take() { - completion.publish(true); - } - if let Some((_, receipt)) = self.ack_receipt.take() { - let _ = receipt.send(Ok(false)); + let publication = &self.ack_receipt.as_ref().unwrap().0; + if let Ok(Some(error)) = publication.error_frame(&self.frame) { + // When the bind is retracted, replace its queued writer frame + // with the error frame in place. Keeping its queue position + // prevents candidate refusals overtaking the bind's error. + self.frame = error; + return true; } return false; } @@ -216,17 +218,28 @@ impl WriterFrame { } pub(super) fn finish_ack_write(&mut self, written: bool) { + let retracted = self + .ack_receipt + .as_ref() + .is_some_and(|(publication, _)| publication.is_retracted()); + if written { + if let Some((publication, _)) = self.ack_receipt.as_ref() { + if retracted { + publication.finished(false); + } else { + publication.written(); + } + } + } if written { if let Some(completion) = self.ack_completion.take() { - completion.publish(false); + completion.publish(retracted); } } else { self.ack_completion.take(); } if let Some((publication, receipt)) = self.ack_receipt.take() { - if written { - publication.written(); - } + let _ = publication; let _ = receipt.send(if written { Ok(true) } else { diff --git a/crates/aft/tests/integration/subc_bridge_test.rs b/crates/aft/tests/integration/subc_bridge_test.rs index 9f1a179e90..bcbfe021d2 100644 --- a/crates/aft/tests/integration/subc_bridge_test.rs +++ b/crates/aft/tests/integration/subc_bridge_test.rs @@ -2473,9 +2473,9 @@ fn subc_bridge_duplicate_routebind_rejects_in_flight_bind() { } #[test] -fn subc_bridge_tool_call_before_bind_ack_is_route_not_bound() { +fn subc_bridge_tool_call_before_bind_ack_is_held_until_written() { run_subc_bridge_test( - "subc_bridge_tool_call_before_bind_ack_is_route_not_bound", + "subc_bridge_tool_call_before_bind_ack_is_held_until_written", Duration::from_secs(30), drive_pending_bind_tool_call_daemon, |_, _, _| {}, @@ -7594,8 +7594,8 @@ async fn drive_pending_bind_tool_call_daemon(input: FakeDaemonInput) { .. } = open_fake_daemon_session(input).await; - // P5b B2 finding (d): a route-channel tool call sent before its bind ack is - // a protocol error and remains route_not_bound while the bind is pending. + // Candidate calls wait for bind authority instead of racing configure and + // receiving an unbound-route error before its bind ACK finishes writing. let pending_tool_base = state.begin_slow_configure_wave(); send_route_bind_with_doc( &mut stream, @@ -7612,10 +7612,13 @@ async fn drive_pending_bind_tool_call_daemon(input: FakeDaemonInput) { inner.slow_configure_started > pending_tool_base }); send_tool_call(&mut stream, 12, 1201, "echo", json!({ "case": "fast" })).await; - expect_error_frame(&mut stream, 12, 1201, "route_not_bound").await; state.release_slow_configures(); state.wait_for_slow_configure_finished(pending_tool_base + 1); expect_route_bind_ack(&mut stream, 1200).await; + let response = read_frame_timeout(&mut stream, "held candidate response").await; + assert_eq!(response.header.ty, FrameType::Response); + assert_eq!(response.header.channel, 12); + assert_eq!(response.header.corr, 1201); send_connection_goodbye(&mut stream).await; } @@ -7662,9 +7665,7 @@ async fn drive_goodbye_cancels_pending_bind_daemon(input: FakeDaemonInput) { .expect("goodbye barrier ping"), ) .await; - let pong = read_frame_timeout(&mut stream, "goodbye barrier pong").await; - assert_eq!(pong.header.ty, FrameType::Pong, "{:?}", pong.header); - assert_eq!(pong.header.corr, 1109); + expect_cancelled_bind_with_control_reply(&mut stream, 110, 1109, FrameType::Pong).await; // NO manual release: the Goodbye's operational cancellation must itself // settle the stuck configure, or this test proves only bookkeeping. state.wait_for_slow_configure_finished(goodbye_bind_base + 1); @@ -7809,7 +7810,7 @@ async fn drive_cancelled_first_bind_does_not_orphan_later_bind_daemon(input: Fak // NO manual release: cancelling route 20's configure must free the // mutating lane so route 21's queued configure runs and acks. state.wait_for_slow_configure_finished(slow_bind_base + 1); - expect_route_bind_ack(&mut stream, 210).await; + expect_cancelled_bind_with_control_reply(&mut stream, 200, 210, FrameType::Response).await; send_tool_call(&mut stream, 21, 211, "echo", json!({ "case": "fast" })).await; let joined = read_frame_timeout(&mut stream, "post-cancel sibling bind response").await; assert_eq!(joined.header.channel, 21); @@ -7934,12 +7935,12 @@ async fn drive_goodbye_removes_queued_bind_daemon(input: FakeDaemonInput) { .expect("queued-cancel barrier ping"), ) .await; - let pong = read_frame_timeout(&mut stream, "queued-cancel barrier pong").await; - assert_eq!(pong.header.ty, FrameType::Pong, "{:?}", pong.header); + expect_cancelled_bind_with_control_reply(&mut stream, 410, 412, FrameType::Pong).await; // Cancel the first via Goodbye too; operational cancellation settles it. send_route_goodbye(&mut stream, 40, 401).await; state.wait_for_slow_configure_finished(slow_bind_base + 1); + expect_route_bind_error(&mut stream, 400, "cancelled").await; assert_eq!( state.slow_configure_started_count(), slow_bind_base + 1, @@ -14849,6 +14850,39 @@ async fn expect_route_bind_ack(stream: &mut tokio::net::TcpStream, corr: u64) { assert_eq!(ack, ModuleControlResponse::RouteBindAck {}); } +async fn expect_cancelled_bind_with_control_reply( + stream: &mut tokio::net::TcpStream, + bind_corr: u64, + other_corr: u64, + other_type: FrameType, +) { + // Pong uses the priority lane and the sibling ACK is independent. Neither + // ordering is promised, but the cancelled bind must answer exactly once. + let mut seen = HashSet::new(); + for _ in 0..2 { + let frame = read_frame_timeout(stream, "cancelled bind and control reply").await; + assert_eq!(frame.header.channel, 0); + assert!(seen.insert(frame.header.corr), "duplicate control reply"); + if frame.header.corr == bind_corr { + assert_error_frame(&frame, 0, bind_corr, "cancelled"); + assert_eq!( + serde_json::from_slice::(&frame.body).unwrap()["message"], + "request cancelled" + ); + } else { + assert_eq!(frame.header.corr, other_corr); + assert_eq!(frame.header.ty, other_type); + if other_type == FrameType::Response { + assert_eq!( + serde_json::from_slice::(&frame.body).unwrap(), + ModuleControlResponse::RouteBindAck {} + ); + } + } + } + assert_eq!(seen, HashSet::from([bind_corr, other_corr])); +} + async fn expect_route_bind_error(stream: &mut tokio::net::TcpStream, corr: u64, code: &str) { expect_error_frame(stream, 0, corr, code).await; } From 7703340d73467435e5c27188541f999b116f10b7 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Mon, 26 Oct 2026 11:16:23 +0100 Subject: [PATCH 05/13] mason: keep protocol fixtures off background index startup (#891) The snapshot-witness path now runs real configure in transport tests. Give those fixtures an explicit user config with indexes and views disabled rather than an absent file resolving to enabled index defaults. Retain the AST actor and assert the applied switches stay off; no production reply or index behavior changes. Committed at the parent's explicit checkpoint request. These small test-only changes and the RESERVED deadline-unit extension have not been compiled or executed: six Linux2/4-core submissions were refused for more than73 minutes, including the focused finish job. Earlier Linux and Windows compile checks passed, but the existing bind-deadline recovery integration timed out and a tool-result run watchdog-exited on a starved host. Those are recorded as failures, not passes. The PR's three-platform CI will compile and test this checkpoint; no new chunks, mutation checks or full-suite worker gates were started after the process change. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- crates/aft/src/subc/mod.rs | 15 +++++++++++---- crates/aft/src/subc/scope_workdir_tests.rs | 2 +- crates/aft/src/subc/tool_result_tests.rs | 14 +++++++++++--- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/crates/aft/src/subc/mod.rs b/crates/aft/src/subc/mod.rs index 6625318c83..e1c68e0f10 100644 --- a/crates/aft/src/subc/mod.rs +++ b/crates/aft/src/subc/mod.rs @@ -11971,6 +11971,13 @@ fn opaque_field_refusal_frame( #[cfg(test)] pub(crate) mod test_support { + /// Write user configuration with indexes and views disabled for transport tests. + /// Default configure can start index builds that these tests do not exercise. + pub(super) fn bind_config_without_indexes(root: &std::path::Path) -> std::path::PathBuf { + let path = root.join("protocol-user-config.json"); + std::fs::write(&path, r#"{"indexes":{"trigram":false,"semantic":false,"callgraph":false},"views":{"enabled":false}}"#).unwrap(); + path + } use super::*; use crate::bash_background::BgTaskStatus; use crate::protocol::{ @@ -14090,7 +14097,7 @@ mod tests { Response::success(request.id, json!({})) } }, - Some(fixture_path.join("absent-user-config.json")), + Some(test_support::bind_config_without_indexes(&fixture_path)), false, usize::MAX, None, @@ -14260,7 +14267,7 @@ mod tests { Response::success(request.id, json!({})) } }, - Some(fixture_path.join("absent-user-config.json")), + Some(test_support::bind_config_without_indexes(&fixture_path)), false, usize::MAX, None, @@ -14769,7 +14776,7 @@ mod tests { Response::success(request.id, json!({})) } }, - Some(&dir.path().join("absent-user-config.json")), + Some(&test_support::bind_config_without_indexes(dir.path())), usize::MAX, None, ) @@ -23747,7 +23754,7 @@ mod pong_priority_tests { Response::success(request.id, json!({})) } }, - Some(storage_path.join("user.json")), + Some(test_support::bind_config_without_indexes(&storage_path)), false, usize::MAX, None, diff --git a/crates/aft/src/subc/scope_workdir_tests.rs b/crates/aft/src/subc/scope_workdir_tests.rs index 79641bbd69..c18a3f2347 100644 --- a/crates/aft/src/subc/scope_workdir_tests.rs +++ b/crates/aft/src/subc/scope_workdir_tests.rs @@ -157,7 +157,7 @@ fn bind_observed( } Response::success(request.id, request.params) }, - Some(&root.join("absent-user-config.json")), + Some(&test_support::bind_config_without_indexes(&root)), usize::MAX, None, ) diff --git a/crates/aft/src/subc/tool_result_tests.rs b/crates/aft/src/subc/tool_result_tests.rs index 0d3278e15b..1f8ae275e5 100644 --- a/crates/aft/src/subc/tool_result_tests.rs +++ b/crates/aft/src/subc/tool_result_tests.rs @@ -232,7 +232,7 @@ async fn tool_result_ast_search_t6_complete_and_budget_cut_on_negotiated_route() // Reuse a configured actor without starting unrelated background indexing. let ctx = Arc::new(context(&root)); let app = ctx.app().clone(); - executor.register_actor(root_id, ctx); + executor.register_actor(root_id, ctx.clone()); let loop_executor = executor.clone(); let (mut daemon, module) = tokio::io::duplex(64 * 1024); let module_thread = std::thread::spawn(move || { @@ -254,7 +254,7 @@ async fn tool_result_ast_search_t6_complete_and_budget_cut_on_negotiated_route() crate::commands::ast_search::handle_ast_search(&request, &ctx) } }, - Some(module_root.join("absent-user-config.json")), + Some(test_support::bind_config_without_indexes(&module_root)), false, usize::MAX, None, @@ -327,6 +327,14 @@ async fn tool_result_ast_search_t6_complete_and_budget_cut_on_negotiated_route() serde_json::from_slice::(&ack.body).unwrap(), ModuleControlResponse::RouteBindAck {} )); + let configured = ctx.config(); + assert!( + !configured.indexes.trigram + && !configured.indexes.semantic + && !configured.indexes.callgraph, + "protocol fixture unexpectedly enabled background indexes" + ); + drop(configured); let mut replies = Vec::new(); for (corr, file) in [(10, "small.js"), (11, "large.js")] { let call = json!({"name":"ast_search", "arguments":{ @@ -528,7 +536,7 @@ async fn tool_result_capability_is_selected_per_bind_not_per_call() { json!({"producer_budget": result_body::active()}), ) }, - Some(module_root.join("absent-user-config.json")), + Some(test_support::bind_config_without_indexes(&module_root)), false, usize::MAX, None, From a398c7402ce5e1ffab0697a99353baec2c7bb1e5 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Mon, 26 Oct 2026 11:28:51 +0100 Subject: [PATCH 06/13] search-quality descriptor for frame-loop-a0 Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- .../aft-search/slice-descriptors/land-frame-loop-a0.json | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 benchmarks/aft-search/slice-descriptors/land-frame-loop-a0.json diff --git a/benchmarks/aft-search/slice-descriptors/land-frame-loop-a0.json b/benchmarks/aft-search/slice-descriptors/land-frame-loop-a0.json new file mode 100644 index 0000000000..ebd574711e --- /dev/null +++ b/benchmarks/aft-search/slice-descriptors/land-frame-loop-a0.json @@ -0,0 +1,8 @@ +{ + "targeted_mechanism": "none", + "kind": "harness", + "slice_class": "non_ranking", + "fixtures": [ + "harness-goldens" + ] +} From 518241ddcfb00eaa48fccbb43e978592cb0586f8 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Mon, 26 Oct 2026 11:29:08 +0100 Subject: [PATCH 07/13] Release note in user terms Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- .alfonso/release-notes/next/a0-control-egress.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.alfonso/release-notes/next/a0-control-egress.md b/.alfonso/release-notes/next/a0-control-egress.md index 760ce687e5..9666cb5bc2 100644 --- a/.alfonso/release-notes/next/a0-control-egress.md +++ b/.alfonso/release-notes/next/a0-control-egress.md @@ -1 +1 @@ -- Route binds and management replies no longer wait for tool-outcome persistence; candidate calls and replayed events wait for the written bind acknowledgement, and cancelled queued binds receive one control error instead of losing their reply. +- When the disk is slow, AFT no longer holds up new sessions connecting to it while it saves earlier results, and a request sent while a session is still connecting now waits for the connection instead of failing. From 049364d6a2fc1c88d4ab87defd6e2549d84e7320 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Mon, 26 Oct 2026 13:31:52 +0100 Subject: [PATCH 08/13] mason: retain bind-gated status and keep native loop state on the heap (#984) Reproduced the merged checkpoint on Linux: conformance24 and scope7 pass; bridge130 pass,5 fail,2 ignored. Preserve bounded latest status while an ACK is pending so a newly published sibling gets the coalesced snapshot exactly once. Construct the large loop future in a non-inlined heap-allocation boundary: Windows CI logs show successful bind/status followed by native main stack overflow. Restore generic lifecycle observers' prior event stream with explicit ACK-queue observation for the queued-ACK fixture. Run real configure in the S1 guard fixture, use a supported page harness, and hold configure's actual database in the confinement test. Observe flood pressure before releasing configure rather than requiring the raw ACK lane to lose a scheduling race. No immediate-route_not_bound contract assertion was reversed in this follow-up; the flood pressure and ordering assertions remain. Committed before the requested single filtered Linux2c verification job. No mutation checks or full-suite gate. The final result will record that job's exact outcomes, including small-native-stack coverage and the exact storm regression. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- crates/aft/src/subc/bind_ack_fixup_tests.rs | 2 +- crates/aft/src/subc/mod.rs | 67 +++++++++++++------ crates/aft/src/subc/push.rs | 65 +++++++++++++++--- .../integration/confine_admission_test.rs | 21 +++--- .../tests/integration/page_collect_test.rs | 2 +- .../aft/tests/integration/subc_bridge_test.rs | 38 +++++++++-- 6 files changed, 151 insertions(+), 44 deletions(-) diff --git a/crates/aft/src/subc/bind_ack_fixup_tests.rs b/crates/aft/src/subc/bind_ack_fixup_tests.rs index d39b84ea16..5a24075ae4 100644 --- a/crates/aft/src/subc/bind_ack_fixup_tests.rs +++ b/crates/aft/src/subc/bind_ack_fixup_tests.rs @@ -93,7 +93,7 @@ async fn retracted_bind(tool: bool, trigger: &str, expected_code: &str) { let frames = Arc::new(AtomicUsize::new(0)); let metrics = Arc::new(DispatchPathMetrics::new()); let (events, mut observed) = mpsc::unbounded_channel(); - let probe = SubcTestLifecycleProbe::new(events); + let probe = SubcTestLifecycleProbe::new(events).with_bind_ack_queue_for_test(891); let before = frames.clone(); let hooks = stall_watchdog::FrameLoopTestHooks { metrics: metrics.clone(), diff --git a/crates/aft/src/subc/mod.rs b/crates/aft/src/subc/mod.rs index e1c68e0f10..b709cee33f 100644 --- a/crates/aft/src/subc/mod.rs +++ b/crates/aft/src/subc/mod.rs @@ -318,6 +318,7 @@ pub enum SubcLifecycleEvent { pub struct SubcTestLifecycleProbe { events_tx: mpsc::UnboundedSender, bind_priority: Arc>>, + ack_queue_corr: Option, health_rollup: Option, } @@ -334,10 +335,18 @@ impl SubcTestLifecycleProbe { Self { events_tx, bind_priority: Arc::new(StdMutex::new(None)), + ack_queue_corr: None, health_rollup: None, } } + /// Observe one bind's queued ACK without changing other lifecycle observers. + #[doc(hidden)] + pub fn with_bind_ack_queue_for_test(mut self, corr: u64) -> Self { + self.ack_queue_corr = Some(corr); + self + } + /// Hold the next turn that drains reliable pushes at its end. Releasing the /// returned sender lets the next turn see work queued during the rendezvous. #[doc(hidden)] @@ -400,8 +409,10 @@ impl SubcTestLifecycleProbe { .batches_at_arrival .expect("bind completion arrival observed"); priority.reliable_push_batches - batches_at_arrival - } else { + } else if self.ack_queue_corr == Some(corr) { 0 + } else { + return; }; let _ = self.events_tx.send(SubcLifecycleEvent::RouteBindAckQueued { corr, @@ -4144,25 +4155,27 @@ fn run_subc_mode_inner( connection_file_path.display() ); let (read_half, write_half) = tokio::io::split(stream); - run_module_loop( - read_half, - write_half, - connection_file_path, - shared_app, - executor_for_loop, - dispatch, - user_config_path, - allow_native_passthrough, - tool_response_body_limit, - lifecycle_probe, - &storage_dir, - // The exit phase below takes and shuts down every root's servers. - QuiescedLsp::LeaveForExitSweep, - #[cfg(test)] - None, - #[cfg(test)] - None, - ) + heap_module_future(|| { + run_module_loop( + read_half, + write_half, + connection_file_path, + shared_app, + executor_for_loop, + dispatch, + user_config_path, + allow_native_passthrough, + tool_response_body_limit, + lifecycle_probe, + &storage_dir, + // The exit phase below takes and shuts down every root's servers. + QuiescedLsp::LeaveForExitSweep, + #[cfg(test)] + None, + #[cfg(test)] + None, + ) + }) .await }); @@ -4864,6 +4877,14 @@ async fn drain_pending_route_bind_completions( Ok(drained) } +// Construct the large protocol future in a separate stack frame, then keep its +// state on the heap while polling. Native Windows main threads have a smaller +// default stack than Rust worker threads, and must still handle ordinary calls. +#[inline(never)] +fn heap_module_future(build: impl FnOnce() -> F) -> std::pin::Pin> { + Box::pin(build()) +} + /// ModuleHello → HelloAck → control/route loop. Runs until the daemon closes /// the connection (EOF), sends channel-0 Goodbye, or a fatal mutating executor /// response requests whole-connection teardown. @@ -5233,6 +5254,12 @@ where { break Err(error); } + status_pushes.awaiting_binds( + pending_binds + .values() + .filter(|pending| !pending.cancelled && !pending.deadline_reported) + .map(|pending| pending.bind_root_id.clone()), + ); if tokio::time::Instant::now() >= next_drain_at { dispatch_path_metrics.mark_frame_loop_step("push::emit_bg_event_wakes", "", 0); push::emit_bg_event_wakes( diff --git a/crates/aft/src/subc/push.rs b/crates/aft/src/subc/push.rs index e54c25aa2a..45640bed44 100644 --- a/crates/aft/src/subc/push.rs +++ b/crates/aft/src/subc/push.rs @@ -757,11 +757,10 @@ fn matching_route_channels( root: &ProjectRootId, frame: &PushFrame, ) -> Vec { - let Some(channels) = root_channels.get(root) else { - return Vec::new(); - }; - if let PushFrame::Progress(progress) = frame { + let Some(channels) = root_channels.get(root) else { + return Vec::new(); + }; let Some(origin) = progress.origin else { return Vec::new(); }; @@ -777,8 +776,25 @@ fn matching_route_channels( }; } - let session = frame_session(frame); - let bash_observation = frame_is_bash_observation(frame); + matching_session_route_channels( + routes, + root_channels, + root, + frame_session(frame), + frame_is_bash_observation(frame), + ) +} + +fn matching_session_route_channels( + routes: &HashMap, + root_channels: &HashMap>, + root: &ProjectRootId, + session: Option<&str>, + bash_observation: bool, +) -> Vec { + let Some(channels) = root_channels.get(root) else { + return Vec::new(); + }; channels .iter() .copied() @@ -1196,6 +1212,7 @@ pub(super) fn drain_reliable_push_turn( pub(super) struct StatusPushes { pending: HashMap, published: HashMap, + awaiting_binds: HashSet, } struct PendingStatus { @@ -1205,11 +1222,17 @@ struct PendingStatus { recipients: VecDeque, cursor: Option, session: Option, + queued_recipients: HashSet, } const STATUS_FANOUT_TURN_BYTES: usize = 1024 * 1024; impl StatusPushes { + pub(super) fn awaiting_binds(&mut self, roots: impl Iterator) { + self.awaiting_binds.clear(); + self.awaiting_binds.extend(roots); + } + fn queue( &mut self, root: ProjectRootId, @@ -1239,6 +1262,7 @@ impl StatusPushes { recipients: VecDeque::new(), cursor, session, + queued_recipients: HashSet::new(), }, ); } @@ -1255,6 +1279,9 @@ impl StatusPushes { self.published .retain(|channel, _| routes.contains_key(channel)); for (root, pending) in &mut self.pending { + pending + .queued_recipients + .retain(|channel| routes.contains_key(channel)); if let Some(frame) = pending.frame.take() { let envelope = PushFrame::StatusChanged(frame); let mut recipients = matching_route_channels(routes, roots, root, &envelope); @@ -1268,7 +1295,7 @@ impl StatusPushes { recipients.rotate_left(start % len); } } - if !recipients.is_empty() { + if !recipients.is_empty() || self.awaiting_binds.contains(root) { let PushFrame::StatusChanged(frame) = envelope else { unreachable!() }; @@ -1278,6 +1305,21 @@ impl StatusPushes { pending.recipients = recipients.into(); } } + // The ACK may finish after a snapshot was sent to existing routes. + // Retain only its bounded bytes and session, then select any newly + // published siblings without sending the snapshot twice to old ones. + if pending.body.is_some() && pending.recipients.is_empty() { + let mut recipients = matching_session_route_channels( + routes, + roots, + root, + pending.session.as_deref(), + false, + ); + recipients.retain(|channel| !pending.queued_recipients.contains(channel)); + recipients.sort_by_key(|route| (route.channel, route.epoch)); + pending.recipients = recipients.into(); + } let Some(body) = pending.body.as_ref() else { continue; }; @@ -1303,6 +1345,7 @@ impl StatusPushes { .get(&channel) .is_some_and(|latest| order <= *latest) }) { + pending.queued_recipients.insert(channel); pending.recipients.pop_front(); continue; } @@ -1314,6 +1357,7 @@ impl StatusPushes { } match try_send_shared_push_body(writer, metrics, channel, Arc::clone(body)) { PushSendOutcome::Sent => { + pending.queued_recipients.insert(channel); if let Some(order) = pending.order { // The writer's FIFO now owns this snapshot, even if // the socket has not written it yet. @@ -1329,8 +1373,11 @@ impl StatusPushes { } } } - self.pending - .retain(|_, pending| pending.frame.is_some() || !pending.recipients.is_empty()); + self.pending.retain(|root, pending| { + self.awaiting_binds.contains(root) + || pending.frame.is_some() + || !pending.recipients.is_empty() + }); } } diff --git a/crates/aft/tests/integration/confine_admission_test.rs b/crates/aft/tests/integration/confine_admission_test.rs index 6bbe984eb0..44399b4d17 100644 --- a/crates/aft/tests/integration/confine_admission_test.rs +++ b/crates/aft/tests/integration/confine_admission_test.rs @@ -86,20 +86,25 @@ fn held_aft_db_lock_does_not_stop_the_frame_loop_while_worker_confinement_is_rea } = open_fake_daemon_session(input).await; std::fs::write(root1.join("admitted.txt"), "worker-read-admitted\n").unwrap(); bind_scoped_worker(&mut stream, &root1).await; - // This harness configures no persistence for its binds, so give - // the root the aft.db handle a daemon bind opens for it. + // Configure starts opening aft.db asynchronously. Wait for ctx.db() + // to expose its handle instead of installing a second temporary + // database that could race with the configured database's open. let ctx = root_context(&executor, &root1); - assert!(ctx.db().is_none()); - let storage = tempfile::tempdir().unwrap(); - ctx.set_db(Arc::new(Mutex::new( - aft::db::open(&storage.path().join("aft.db")).unwrap(), - ))); + let db = tokio::time::timeout(Duration::from_secs(10), async { + loop { + if let Some(db) = ctx.db() { + break db; + } + tokio::time::sleep(Duration::from_millis(20)).await; + } + }) + .await + .expect("configured aft.db never became available"); // Hold the aft.db lock on another thread, as a catalog job's // policy freeze does while SQLite waits for its write lock. The // holder lets go on its own after a bound, so a frame loop that // does block fails the timing check below instead of hanging. - let db = ctx.db().unwrap(); let (held_tx, held_rx) = std::sync::mpsc::channel(); let (release_tx, release_rx) = std::sync::mpsc::channel::<()>(); let holder = thread::spawn(move || { diff --git a/crates/aft/tests/integration/page_collect_test.rs b/crates/aft/tests/integration/page_collect_test.rs index c7830862c4..462b818f66 100644 --- a/crates/aft/tests/integration/page_collect_test.rs +++ b/crates/aft/tests/integration/page_collect_test.rs @@ -12,7 +12,7 @@ use super::*; use base64::Engine; -const HARNESS: &str = "test-harness"; +const HARNESS: &str = "runner"; const SESSION: &str = "page-session"; const CORE_CHANNEL: u16 = 12; const OTHER_CHANNEL: u16 = 13; diff --git a/crates/aft/tests/integration/subc_bridge_test.rs b/crates/aft/tests/integration/subc_bridge_test.rs index bcbfe021d2..eae88de091 100644 --- a/crates/aft/tests/integration/subc_bridge_test.rs +++ b/crates/aft/tests/integration/subc_bridge_test.rs @@ -3860,7 +3860,7 @@ static S1_ATTACKER_CONFIG_REACHED_DISPATCH: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); const S1_ATTACKER_MARKER: &str = "S1_ATTACKER_SECRET_ENV"; -fn s1_guard_dispatch(req: RawRequest, _ctx: &AppContext) -> Response { +fn s1_guard_dispatch(req: RawRequest, ctx: &AppContext) -> Response { // The RouteBind reconcile dispatches a clean `configure` (no marker); only a // forwarded tool-call `configure` would carry the marker. If the marker ever // arrives here, the fail-closed gate failed and the cap was bypassed. @@ -3869,6 +3869,7 @@ fn s1_guard_dispatch(req: RawRequest, _ctx: &AppContext) -> Response { if raw.contains(S1_ATTACKER_MARKER) { S1_ATTACKER_CONFIG_REACHED_DISPATCH.store(true, std::sync::atomic::Ordering::SeqCst); } + return aft::commands::configure::handle_configure(&req, ctx); } Response::success(req.id, json!({})) } @@ -3953,7 +3954,8 @@ fn subc_rejects_forwarded_configure_tool_call_in_production() { heavy_permits: 1, drr_quantum: 1, })); - let user_config_path = storage.path().join("nonexistent-user-aft.jsonc"); + let user_config_path = storage.path().join("user-aft.jsonc"); + std::fs::write(&user_config_path, r#"{"indexes":{"trigram":false,"semantic":false,"callgraph":false},"views":{"enabled":false}}"#).unwrap(); // PRODUCTION entry — passthrough is OFF. run_subc_mode( @@ -7424,6 +7426,28 @@ async fn drive_routebind_priority_daemon(input: FakeDaemonInput) { .await; expect_priority_event(&mut events, SubcLifecycleEvent::ReliablePushTurnHeld).await; + // The independent ACK lane can overtake proxied flood frames. Confirm actual + // flood pressure while configure is still held, not by requiring that race + // to resolve the same way after its ACK becomes eligible. + let mut flood_seen = 0_u64; + let mut flood_response_seen = false; + while flood_seen == 0 { + let frame = + read_any_frame_timeout(&mut stream, "flood pressure before bind completion").await; + match frame.header.ty { + FrameType::Push => { + let body: Value = serde_json::from_slice(&frame.body).unwrap(); + if push_type(&body) == Some("bash_completed") + && push_task_id(&body).is_some_and(|task| task.starts_with(flood_prefix)) + { + flood_seen += 1; + } + } + FrameType::Response if frame.header.corr == 901 => flood_response_seen = true, + other => panic!("unexpected frame before bind completion: {other:?}"), + } + } + // Hold the loop at a turn boundary, not the configure worker: completion // handoff must actually reach the loop's channel before the next turn runs. state.release_slow_configures(); @@ -7474,8 +7498,6 @@ async fn drive_routebind_priority_daemon(input: FakeDaemonInput) { &ctx, bash_completed_push(after_ack_task, "session-1") )); - let mut flood_seen = 0_u64; - let mut flood_response_seen = false; let mut ack_seen = false; let mut ready_seen = false; let mut after_ack_seen = false; @@ -14843,7 +14865,13 @@ fn extract_bash_task_id(text: &str) -> String { async fn expect_route_bind_ack(stream: &mut tokio::net::TcpStream, corr: u64) { let frame = read_frame_timeout(stream, "RouteBindAck").await; - assert_eq!(frame.header.ty, FrameType::Response); + assert_eq!( + frame.header.ty, + FrameType::Response, + "bind corr={corr}: header={:?} body={}", + frame.header, + String::from_utf8_lossy(&frame.body) + ); assert_eq!(frame.header.channel, 0); assert_eq!(frame.header.corr, corr); let ack: ModuleControlResponse = serde_json::from_slice(&frame.body).expect("ack body"); From c8ae4a5d747882687d428da154e60b0b63af75a6 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Mon, 26 Oct 2026 14:05:54 +0100 Subject: [PATCH 09/13] mason: settle bind setup before testing held database and flood work (#984) The filtered Linux verification passed conformance24 and scope7 under a1MiB native stack, the storm regression, and133 bridge tests. Two fixtures still raced setup: taking the real DB before scope-bind settled blocked unrelated registration, and observing flood pressure after arming a turn hold could deadlock before the flood was drained. Fence the scope-bind mutating lane after a control round-trip, then hold the configured DB. Observe the first flood frame before arming the hold and use an explicit push to reach the held turn. Keep the2s health bound, pressure assertion, exact flood count and ACK/push ordering assertions. Re-run only these two remaining failures; no mutation checks or full-suite gate. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- .../integration/confine_admission_test.rs | 38 +++++++++++++------ .../aft/tests/integration/subc_bridge_test.rs | 19 ++++++---- 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/crates/aft/tests/integration/confine_admission_test.rs b/crates/aft/tests/integration/confine_admission_test.rs index 44399b4d17..538ba57da2 100644 --- a/crates/aft/tests/integration/confine_admission_test.rs +++ b/crates/aft/tests/integration/confine_admission_test.rs @@ -86,20 +86,34 @@ fn held_aft_db_lock_does_not_stop_the_frame_loop_while_worker_confinement_is_rea } = open_fake_daemon_session(input).await; std::fs::write(root1.join("admitted.txt"), "worker-read-admitted\n").unwrap(); bind_scoped_worker(&mut stream, &root1).await; - // Configure starts opening aft.db asynchronously. Wait for ctx.db() - // to expose its handle instead of installing a second temporary - // database that could race with the configured database's open. + // The ACK precedes asynchronous database and scope registration. + // A health round-trip lets the loop submit scope-bind; a mutating + // lane barrier then waits for that job before we hold its database. let ctx = root_context(&executor, &root1); - let db = tokio::time::timeout(Duration::from_secs(10), async { - loop { - if let Some(db) = ctx.db() { - break db; - } - tokio::time::sleep(Duration::from_millis(20)).await; + send_control_request(&mut stream, 30_001, ModuleControlRequest::HealthCheck {}).await; + loop { + let frame = read_frame_timeout(&mut stream, "configured root health").await; + if frame.header.channel == 0 && frame.header.corr == 30_001 { + assert_eq!(frame.header.ty, FrameType::Response); + break; } - }) - .await - .expect("configured aft.db never became available"); + } + let ready = executor.submit_async( + ProjectRootId::from_path(&root1).unwrap(), + aft::executor::Lane::Mutating, + "confine-fixture-ready".into(), + Box::new(|_| Response::success("confine-fixture-ready", json!({}))), + ); + assert!( + tokio::time::timeout(Duration::from_secs(10), ready) + .await + .expect("scope registration did not settle") + .unwrap() + .success + ); + let db = ctx + .db() + .expect("configured aft.db must be available after scope-bind"); // Hold the aft.db lock on another thread, as a catalog job's // policy freeze does while SQLite waits for its write lock. The diff --git a/crates/aft/tests/integration/subc_bridge_test.rs b/crates/aft/tests/integration/subc_bridge_test.rs index eae88de091..e69cc6c3ca 100644 --- a/crates/aft/tests/integration/subc_bridge_test.rs +++ b/crates/aft/tests/integration/subc_bridge_test.rs @@ -7413,7 +7413,6 @@ async fn drive_routebind_priority_daemon(input: FakeDaemonInput) { inner.slow_configure_started > slow_bind_base }); - let release_turn = probe.hold_reliable_push_turn_for_test(19); let flood_prefix = "routebind-priority-flood"; let flood_count = 1024_u64; send_tool_call( @@ -7424,11 +7423,9 @@ async fn drive_routebind_priority_daemon(input: FakeDaemonInput) { json!({ "prefix": flood_prefix, "count": flood_count }), ) .await; - expect_priority_event(&mut events, SubcLifecycleEvent::ReliablePushTurnHeld).await; - - // The independent ACK lane can overtake proxied flood frames. Confirm actual - // flood pressure while configure is still held, not by requiring that race - // to resolve the same way after its ACK becomes eligible. + // Confirm flood pressure before holding the loop. The independent ACK lane + // may overtake proxied frames once configure completes, and the held batch + // need not be visible on the socket until the loop resumes. let mut flood_seen = 0_u64; let mut flood_response_seen = false; while flood_seen == 0 { @@ -7447,6 +7444,14 @@ async fn drive_routebind_priority_daemon(input: FakeDaemonInput) { other => panic!("unexpected frame before bind completion: {other:?}"), } } + let root_id = ProjectRootId::from_path(&root1).expect("root1 id"); + let ctx = executor.actor_context(&root_id).expect("root1 actor"); + let release_turn = probe.hold_reliable_push_turn_for_test(19); + assert!(emit_push_frame( + &ctx, + bash_completed_push("routebind-priority-turn-hold", "session-1") + )); + expect_priority_event(&mut events, SubcLifecycleEvent::ReliablePushTurnHeld).await; // Hold the loop at a turn boundary, not the configure worker: completion // handoff must actually reach the loop's channel before the next turn runs. @@ -7456,8 +7461,6 @@ async fn drive_routebind_priority_daemon(input: FakeDaemonInput) { SubcLifecycleEvent::RouteBindCompletionQueued { corr: 19 }, ) .await; - let root_id = ProjectRootId::from_path(&root1).expect("root1 id"); - let ctx = executor.actor_context(&root_id).expect("root1 actor"); let ready_task = "routebind-priority-ready"; assert!(emit_push_frame( &ctx, From 985602812c621b323ccf5c57d1273ceba6f17dfb Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Mon, 26 Oct 2026 23:29:48 +0100 Subject: [PATCH 10/13] mason: expose bind and ledger refusal details in regression failures (#984) Keep both assertions unchanged, but print the first control frame's type/body when health is not correlation40 and enable the existing durable-ledger warning for the exact codemode regression. These diagnostics distinguish an early bind Error from an ACK and a failed ledger write from a missing/mismatched terminal row. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- crates/aft/tests/integration/scope_revocation_test.rs | 6 ++++++ crates/aft/tests/integration/subc_bridge_test.rs | 8 +++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/aft/tests/integration/scope_revocation_test.rs b/crates/aft/tests/integration/scope_revocation_test.rs index 96867e6483..13fbf9d1c1 100644 --- a/crates/aft/tests/integration/scope_revocation_test.rs +++ b/crates/aft/tests/integration/scope_revocation_test.rs @@ -1784,6 +1784,12 @@ fn codemode_bash_runs_in_a_real_shell_without_rewrites() { #[test] fn codemode_call_runs_with_head_principal_and_run_route_finds_its_ledger_row() { + // The call-ledger warning explains why an outcome_unknown result could not + // be confirmed as durably recorded. + let _ = env_logger::Builder::new() + .filter_module("aft::subc::tool_provider", log::LevelFilter::Warn) + .is_test(true) + .try_init(); run_subc_bridge_test_with_dispatch( "codemode-ledger-carrier", Duration::from_secs(30), diff --git a/crates/aft/tests/integration/subc_bridge_test.rs b/crates/aft/tests/integration/subc_bridge_test.rs index e69cc6c3ca..c567e00993 100644 --- a/crates/aft/tests/integration/subc_bridge_test.rs +++ b/crates/aft/tests/integration/subc_bridge_test.rs @@ -6504,7 +6504,13 @@ async fn drive_back_to_back_binds_beside_held_mutating_job(input: FakeDaemonInpu release_tx.send(()).unwrap(); held.recv_timeout(Duration::from_secs(5)).unwrap(); let health = health.expect("the frame loop must service health while the writer is held"); - assert_eq!((health.header.channel, health.header.corr), (0, 40)); + assert_eq!( + (health.header.channel, health.header.corr), + (0, 40), + "first control frame while Mutating was held: header={:?} body={}", + health.header, + String::from_utf8_lossy(&health.body) + ); let mut acks = HashSet::new(); while acks.len() < 2 { let ack = read_frame_timeout(&mut stream, "deferred bind ack").await; From ca859e0fecb214366be8ec2d951902c4372cf103 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Tue, 27 Oct 2026 00:01:58 +0100 Subject: [PATCH 11/13] mason: publish bind fixture config atomically and isolate its ledger storage (#984) The requested20-run Linux comparisons were main20/20 for both tests, main+#984 bind19/20 and ledger20/20. Unchanged-assertion diagnostics then captured7/20 bind refusals: correlation20 carried config_divergence / invalid_config_document, not a missing acknowledgement. Adjacent binds rewrote the same live JSON file while prepared config read it. Use sibling temporary files and atomic replacement in both bind publishers. Per the parent decision, limit storage changes to the serialized fixture. Select its private directory via AFT_STORAGE_DIR unless env_setup explicitly controls that variable, and assert both actor configs and SQLite's main file remain there. Production actor storage selection and recovery semantics are unchanged; the pre-existing module/actor storage propagation mismatch is documented separately as a migration question. All original health, acknowledgement and ledger-shell assertions remain. The fixture-isolation diagnostic does not claim that the reported rare ledger settlement error was reproduced: it passed20/20 on main and two20-run merged loops. Capture the existing ledger warning if it recurs. No mutation checks or full-suite gate; repeated exact tests follow this committed fix. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- .../integration/scope_revocation_test.rs | 34 +++++++++++++ .../aft/tests/integration/subc_bridge_test.rs | 48 ++++++++++++------- 2 files changed, 66 insertions(+), 16 deletions(-) diff --git a/crates/aft/tests/integration/scope_revocation_test.rs b/crates/aft/tests/integration/scope_revocation_test.rs index 13fbf9d1c1..adb646d87c 100644 --- a/crates/aft/tests/integration/scope_revocation_test.rs +++ b/crates/aft/tests/integration/scope_revocation_test.rs @@ -1855,6 +1855,40 @@ fn codemode_call_runs_with_head_principal_and_run_route_finds_its_ledger_row() { assert!(Instant::now() < until, "aft.db never opened"); tokio::time::sleep(Duration::from_millis(20)).await; } + // Both contexts share App.db. Verify that configuration and the + // opened database stay in this fixture's private storage instead + // of using the process-global default data directory. + let storage = std::path::PathBuf::from( + std::env::var_os("AFT_STORAGE_DIR").expect("fixture storage selection"), + ) + .canonicalize() + .unwrap(); + for project in [&daemon.root1, &daemon.root2] { + let ctx = daemon + .executor + .actor_context(&ProjectRootId::from_path(project).unwrap()) + .expect("bound actor"); + let actor_storage = ctx + .config() + .storage_dir + .clone() + .expect("configured storage"); + assert_eq!(actor_storage.canonicalize().unwrap(), storage); + let db = ctx.db().expect("ready database"); + let database: String = db + .lock() + .unwrap() + .query_row( + "SELECT file FROM pragma_database_list WHERE name='main'", + [], + |row| row.get(0), + ) + .unwrap(); + assert_eq!( + std::path::PathBuf::from(database).canonicalize().unwrap(), + storage.join("aft.db") + ); + } let keyed = json!({ "name": "glob", "preset": "head", diff --git a/crates/aft/tests/integration/subc_bridge_test.rs b/crates/aft/tests/integration/subc_bridge_test.rs index c567e00993..2e4a555d7f 100644 --- a/crates/aft/tests/integration/subc_bridge_test.rs +++ b/crates/aft/tests/integration/subc_bridge_test.rs @@ -1670,6 +1670,20 @@ fn run_subc_bridge_test_inner( install_bridge_state(Arc::clone(&state)); let roots = SubcBridgeTestRoots::new(); + // Bind actors start with default configuration, so the entry context's + // explicit storage path alone does not isolate their database or recovery. + // Give this serialized fixture a private process-wide storage selection. + let _fixture_storage = if _env_guards + .iter() + .any(|guard| guard.key == "AFT_STORAGE_DIR") + { + None + } else { + Some(set_test_env( + "AFT_STORAGE_DIR", + &roots.storage.path().to_string_lossy(), + )) + }; // Strict untrusted routes re-exec the native launcher. This fixture runs the // bridge in libtest, so use the real binary without bypassing confinement. #[cfg(unix)] @@ -11077,6 +11091,22 @@ fn minimal_bind_doc() -> Value { }) } +fn write_bind_project_config(root: &std::path::Path, doc: &Value) { + let path = root.join(".cortexkit").join("aft.jsonc"); + let parent = path.parent().expect("project config parent"); + std::fs::create_dir_all(parent).expect("create .cortexkit dir"); + // An earlier bind may be reading this path while the next bind is sent. + // Publish a complete sibling file instead of truncating the live document. + let mut temporary = + tempfile::NamedTempFile::new_in(parent).expect("create project config temp"); + std::io::Write::write_all( + &mut temporary, + &serde_json::to_vec(doc).expect("serialize bind doc"), + ) + .expect("write project config temp"); + temporary.persist(&path).expect("publish project config"); +} + fn subc_mcp_principal() -> Principal { Principal::Reserved { module_id: "subc-mcp".to_string(), @@ -11122,14 +11152,7 @@ async fn send_route_bind_with_elicitation_capability( corr: u64, root: &std::path::Path, ) { - let project_cfg = root.join(".cortexkit").join("aft.jsonc"); - std::fs::create_dir_all(project_cfg.parent().expect("cortexkit dir parent")) - .expect("create .cortexkit dir"); - std::fs::write( - &project_cfg, - serde_json::to_string(&minimal_bind_doc()).expect("serialize bind doc"), - ) - .expect("write project config"); + write_bind_project_config(root, &minimal_bind_doc()); let body = ModuleControlRequest::RouteBind { route_channel, @@ -14273,14 +14296,7 @@ async fn send_route_bind_with_harness_session_principal_and_doc_epoch( // to the project config file so it reaches handle_control_request's local // read. The wire `config` is still attached but no longer consulted by // production; keeping it asserts it's harmlessly ignored. - let project_cfg = root.join(".cortexkit").join("aft.jsonc"); - std::fs::create_dir_all(project_cfg.parent().expect("cortexkit dir parent")) - .expect("create .cortexkit dir"); - std::fs::write( - &project_cfg, - serde_json::to_string(&doc).expect("serialize bind doc"), - ) - .expect("write project config"); + write_bind_project_config(root, &doc); let request = ModuleControlRequest::RouteBind { route_channel, From 9699c6f369a76a21a15d9b57497d422c82e83ea9 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Tue, 27 Oct 2026 04:22:18 +0100 Subject: [PATCH 12/13] mason: flush the stderr shutdown tail with a bounded exit fence (#984) The failing drain-close artifact contains the loop's exit0 classification but ends before the binary's final shutdown record. Durable-file flushing does not acknowledge the separate best-effort stderr queue. Add a process-exit-only acknowledged fence, sharing the error-exit budget and using the remaining graceful-exit budget. Ordinary logger writes/flushes remain nonblocking; no stderr I/O lock is taken by the producer. The strict descriptor census reproduced on main1a655 plus the rebased PR: ast_t6_route retains eventfd/eventpoll transiently. Add failure-only thread names without changing the census predicate or exempting descriptors so the owning teardown can be identified. The A0 receipt lane spawns tasks on the existing module runtime, not one runtime per route; fixture/product classification remains under diagnosis. No mutation checks or full-suite gate. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- .../next/subc-exit-stderr-flush.md | 1 + crates/aft/src/logging.rs | 8 ++ crates/aft/src/logging/stderr_mirror.rs | 75 ++++++++++++++++++- crates/aft/src/main.rs | 9 ++- crates/aft/src/subc/tool_result_tests.rs | 13 ++++ 5 files changed, 101 insertions(+), 5 deletions(-) create mode 100644 .alfonso/release-notes/next/subc-exit-stderr-flush.md diff --git a/.alfonso/release-notes/next/subc-exit-stderr-flush.md b/.alfonso/release-notes/next/subc-exit-stderr-flush.md new file mode 100644 index 0000000000..765f05ab12 --- /dev/null +++ b/.alfonso/release-notes/next/subc-exit-stderr-flush.md @@ -0,0 +1 @@ +- Supervised shutdowns now give final stderr messages a short, bounded flush, so the exit reason is not left queued when the process stops. diff --git a/crates/aft/src/logging.rs b/crates/aft/src/logging.rs index f62463a0cd..f06a598d86 100644 --- a/crates/aft/src/logging.rs +++ b/crates/aft/src/logging.rs @@ -1608,6 +1608,14 @@ pub fn flush_durable_log(timeout: std::time::Duration) -> bool { done_rx.recv_timeout(timeout).unwrap_or(false) } +/// Wait only during process exit for the queued stderr flush fence. +/// File-log durability is a separate fence; ordinary log flushes never wait here. +pub fn flush_stderr_on_exit(timeout: std::time::Duration) -> bool { + STDERR_MIRROR + .get() + .is_none_or(|mirror| mirror.drain_on_exit(timeout)) +} + /// Persist the last process-exit line without depending on the async writer. /// Taking its active sink prevents later rotation or reconfiguration from /// moving the marker. Queued diagnostics may be abandoned once their bounded diff --git a/crates/aft/src/logging/stderr_mirror.rs b/crates/aft/src/logging/stderr_mirror.rs index 6723eee97d..9f8e7f2e30 100644 --- a/crates/aft/src/logging/stderr_mirror.rs +++ b/crates/aft/src/logging/stderr_mirror.rs @@ -5,6 +5,7 @@ use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::mpsc::{self, SyncSender, TrySendError}; use std::sync::Arc; use std::thread::{self, JoinHandle}; +use std::time::{Duration, Instant}; pub(super) const QUEUE_CAPACITY: usize = 256; const MAX_RECORD_BYTES: usize = 64 * 1024; @@ -35,7 +36,7 @@ impl MirrorHealth { self.dropped_lines .fetch_add(super::log_line_count(bytes), Ordering::Relaxed); } - Message::Flush => { + Message::Flush | Message::Drain(_) => { self.dropped_flushes.fetch_add(1, Ordering::Relaxed); } } @@ -45,6 +46,7 @@ impl MirrorHealth { enum Message { Record(Vec), Flush, + Drain(SyncSender), } #[derive(Clone)] @@ -68,19 +70,22 @@ impl Mirror { // No channel or file-sink lock is held during stderr I/O. let result = match &message { Message::Record(bytes) => sink.write_all(bytes), - Message::Flush => sink.flush(), + Message::Flush | Message::Drain(_) => sink.flush(), }; if result.is_err() { match &message { Message::Record(_) => { worker_health.write_errors.fetch_add(1, Ordering::Relaxed); } - Message::Flush => { + Message::Flush | Message::Drain(_) => { worker_health.flush_errors.fetch_add(1, Ordering::Relaxed); } } worker_health.drop_message(&message); } + if let Message::Drain(done) = &message { + let _ = done.send(result.is_ok()); + } } })?; Ok(( @@ -113,6 +118,36 @@ impl Mirror { self.enqueue(Message::Flush); } + // Only process teardown waits for this fence. Ordinary logger flushes stay + // nonblocking, including when stderr is full or its sink is wedged. + pub(super) fn drain_on_exit(&self, timeout: Duration) -> bool { + let Some(tx) = self.tx.as_ref() else { + return false; + }; + let deadline = Instant::now() + timeout; + let (done, received) = mpsc::sync_channel(1); + let mut message = Message::Drain(done); + loop { + match tx.try_send(message) { + Ok(()) => break, + Err(TrySendError::Disconnected(message)) => { + self.health.drop_message(&message); + return false; + } + Err(TrySendError::Full(returned)) => message = returned, + } + let remaining = deadline.saturating_duration_since(Instant::now()); + if remaining.is_zero() { + self.health.drop_message(&message); + return false; + } + thread::sleep(remaining.min(Duration::from_millis(1))); + } + received + .recv_timeout(deadline.saturating_duration_since(Instant::now())) + .unwrap_or(false) + } + fn enqueue(&self, message: Message) { let Some(tx) = self.tx.as_ref() else { self.health.drop_message(&message); @@ -158,6 +193,40 @@ mod tests { } } + #[test] + fn exit_drain_confirms_accepted_records_and_has_a_bounded_wait() { + let bytes = Arc::new(Mutex::new(Vec::new())); + let (entered, entering) = mpsc::channel(); + let (release, released) = mpsc::channel(); + let (mirror, worker) = Mirror::spawn( + GateWriter { + bytes: bytes.clone(), + write_gate: Some((entered, released)), + flush_gate: None, + }, + 8, + Arc::new(MirrorHealth::default()), + ) + .unwrap(); + mirror.record(b"shutdown reason\n"); + entering.recv_timeout(Duration::from_secs(1)).unwrap(); + let probe = mirror.clone(); + let (done, received) = mpsc::channel(); + let drainer = thread::spawn(move || { + let _ = done.send(probe.drain_on_exit(Duration::from_millis(20))); + }); + let bounded = received.recv_timeout(Duration::from_secs(1)); + // Release the sink before asserting, so even a broken fence cannot leave + // the worker blocked behind the test's gate after a failure. + release.send(()).unwrap(); + drainer.join().unwrap(); + assert!(!bounded.expect("exit drain exceeded its wait budget")); + assert!(mirror.drain_on_exit(Duration::from_secs(1))); + assert_eq!(&*bytes.lock().unwrap(), b"shutdown reason\n"); + drop(mirror); + worker.join().unwrap(); + } + #[test] fn full_queue_drops_whole_records_and_flushes_without_waiting() { let bytes = Arc::new(Mutex::new(Vec::new())); diff --git a/crates/aft/src/main.rs b/crates/aft/src/main.rs index 38285ac24e..d3857d0d42 100644 --- a/crates/aft/src/main.rs +++ b/crates/aft/src/main.rs @@ -94,14 +94,16 @@ mod stderr_mode_tests { } } -/// Upper bound on how long an exit waits for the durable log to catch up. +/// Shared wait budget for durable-file and stderr flushing on error exits. const DURABLE_LOG_EXIT_FLUSH: Duration = Duration::from_secs(2); -/// Flush the durable log before `process::exit`, which skips every destructor +/// Flush the logs before `process::exit`, which skips every destructor /// and would otherwise drop the lines still queued for the log writer: the /// ones that explain the exit. fn exit_after_log_flush(code: i32) -> i32 { + let started = Instant::now(); aft::logging::flush_durable_log(DURABLE_LOG_EXIT_FLUSH); + aft::logging::flush_stderr_on_exit(DURABLE_LOG_EXIT_FLUSH.saturating_sub(started.elapsed())); code } @@ -428,6 +430,9 @@ fn main() { } #[cfg(debug_assertions)] storage_lease_exit_probe(); + // The stderr mirror has its own queue: a durable-file flush does + // not deliver the shutdown reason to redirected module stderr. + aft::logging::flush_stderr_on_exit(aft::subc::exit_log_flush_wait()); if !native_idle { aft::ort_lifecycle::exit_without_native_teardown(0); } diff --git a/crates/aft/src/subc/tool_result_tests.rs b/crates/aft/src/subc/tool_result_tests.rs index 1f8ae275e5..6a0c49f65f 100644 --- a/crates/aft/src/subc/tool_result_tests.rs +++ b/crates/aft/src/subc/tool_result_tests.rs @@ -1430,6 +1430,19 @@ fn tool_result_fixtures_release_descriptors() { .collect(); println!("FD_CENSUS SUMMARY {name} before={before} after={after} retained={retained:?}"); if after > before { + let mut threads: Vec<_> = std::fs::read_dir("/proc/self/task") + .unwrap() + .filter_map(Result::ok) + .filter_map(|entry| { + std::fs::read_to_string(entry.path().join("comm")) + .ok() + .map(|name| { + format!("{}:{}", entry.file_name().to_string_lossy(), name.trim()) + }) + }) + .collect(); + threads.sort(); + println!("FD_CENSUS_THREADS {name} {threads:?}"); leaks.push(format!("{name}: {before} -> {after}; {retained:?}")); } } From 240e71a0eba730f0dfe5b340090208fec25cc7a7 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Tue, 27 Oct 2026 04:41:00 +0100 Subject: [PATCH 13/13] mason: keep transport fixtures free of detached project-watcher teardown (#984) Real configure introduced deferred project-watcher startup into fixtures which only test result transport. notify8.2 drops its public watcher by signalling a detached mio event loop, so returning from the fixture or joining AFT's filter is not a barrier for its eventfd/eventpoll closure. The strict census reproduced the temporary pair and later returned to baseline. Use a scoped watcher-disabled setting under the existing reentrant process-env lock for the AST and capability fixtures, and assert AST never starts a project watcher. The setting is restored before unlocking. No census predicate, allowance, anon_inode exemption or timeout changed; module runtimes and reader/writer tasks retain their normal joined teardown. A0 owns no runtime per route. The exact census will be verified again with stronger repetitions; no mutation or full-suite gate. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- crates/aft/src/subc/tool_result_tests.rs | 6 ++++++ crates/aft/src/test_env.rs | 15 +++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/crates/aft/src/subc/tool_result_tests.rs b/crates/aft/src/subc/tool_result_tests.rs index 6a0c49f65f..652e7ceed2 100644 --- a/crates/aft/src/subc/tool_result_tests.rs +++ b/crates/aft/src/subc/tool_result_tests.rs @@ -222,6 +222,7 @@ fn tool_result_images_are_ordered_and_kept_in_structured_content() { #[tokio::test] async fn tool_result_ast_search_t6_complete_and_budget_cut_on_negotiated_route() { + let _watcher_disabled = crate::test_env::without_project_watcher(); let (project, root_id) = test_support::test_root("ast-search-t6"); let root = project.path().to_path_buf(); std::fs::write(root.join("small.js"), "console.log(1);\n".repeat(3)).unwrap(); @@ -356,6 +357,10 @@ async fn tool_result_ast_search_t6_complete_and_budget_cut_on_negotiated_route() .unwrap(); replies.push(receive(&mut daemon, 1, corr).await); } + assert!( + !ctx.watcher_runtime_active(), + "transport fixture started a project watcher" + ); drop(daemon); module_thread.join().unwrap().unwrap(); for (frame, total) in replies.iter().zip([3, 24000]) { @@ -506,6 +511,7 @@ fn tool_result_catalog_switch_preserves_schemas_digests_and_pins() { #[tokio::test] async fn tool_result_capability_is_selected_per_bind_not_per_call() { + let _watcher_disabled = crate::test_env::without_project_watcher(); let project = scratch_project(); let root = project.path().to_path_buf(); let module_root = root.clone(); diff --git a/crates/aft/src/test_env.rs b/crates/aft/src/test_env.rs index 13d2ecdda8..ab31c965a3 100644 --- a/crates/aft/src/test_env.rs +++ b/crates/aft/src/test_env.rs @@ -108,6 +108,21 @@ pub(crate) fn without_storage_override() -> StorageOverrideGuard { } } +/// Keep transport fixtures from starting project watchers unrelated to their +/// protocol assertions. Restore the setting before releasing the shared env lock. +pub(crate) struct ProjectWatcherDisabledGuard { + _setting: ScopedEnvVar, + _lock: ProcessEnvLockGuard, +} + +pub(crate) fn without_project_watcher() -> ProjectWatcherDisabledGuard { + let lock = process_env_lock(); + ProjectWatcherDisabledGuard { + _setting: ScopedEnvVar::set("AFT_TEST_DISABLE_FILE_WATCHER", "1"), + _lock: lock, + } +} + struct ScopedEnvVar { key: &'static str, previous: Option,