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..9666cb5bc2 --- /dev/null +++ b/.alfonso/release-notes/next/a0-control-egress.md @@ -0,0 +1 @@ +- 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. 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/.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/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" + ] +} diff --git a/crates/aft/src/commands/configure.rs b/crates/aft/src/commands/configure.rs index 22f703d3af..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(); @@ -3687,9 +3688,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 +3739,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 +3780,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 +16645,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"] { @@ -17345,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 c7615e8cb5..5a52c8f28e 100644 --- a/crates/aft/src/config_fix.rs +++ b/crates/aft/src/config_fix.rs @@ -600,7 +600,40 @@ 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> { + 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)?; + crate::subc_config::read_bounded_config(file, limit) + })(); + 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 +665,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..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 @@ -133,10 +134,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()) { @@ -152,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(), @@ -244,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 @@ -257,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, @@ -284,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) } @@ -647,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/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/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/a0_egress.rs b/crates/aft/src/subc/a0_egress.rs new file mode 100644 index 0000000000..e070d0bcd0 --- /dev/null +++ b/crates/aft/src/subc/a0_egress.rs @@ -0,0 +1,1436 @@ +//! 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; +const RESERVED: u8 = 4; +pub(super) const BIND_BUSY_MESSAGE: &str = + "too many requests are waiting for a route bind; retry shortly"; + +#[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.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 + .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; + } + } +} + +pub(super) enum ControlCompletion { + ConfigureReady(RouteBindCompletion), + AckWritten(RouteBindCompletion), + AckCancelled(Box), + ManagementAckWritten { + route: RouteChannel, + principal: Option, + corr: u64, + }, + BindRetracted { + route: RouteChannel, + corr: u64, + }, + WriteFailed, +} + +impl ControlCompletion { + pub(super) fn corr(&self) -> u64 { + match self { + Self::ConfigureReady(done) | Self::AckWritten(done) => done.corr, + Self::ManagementAckWritten { corr, .. } | Self::BindRetracted { corr, .. } => *corr, + Self::AckCancelled(done) => done.corr(), + Self::WriteFailed => 0, + } + } +} + +pub(super) struct AckCompletion { + permit: mpsc::OwnedPermit, + completion: ControlCompletion, + metrics: Arc, +} +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); + } +} + +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, + 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 { + pub(super) fn new(writer: WriterSender, completions: mpsc::Sender) -> Self { + Self { + writer, + completions, + 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) { + 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})), + ); + } + } + pub(super) fn cancel(&self, route: RouteChannel) { + 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) { + 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, + frame: Frame, + completion: ControlCompletion, + metrics: &Arc, + ) -> Result { + 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)) + }) + }; + let Some((publication, budget)) = reserved else { + return Err(completion); + }; + 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(); + let (mut outgoing, receipt) = WriterFrame::with_ack_receipt(frame, publication.clone()); + outgoing.set_ack_completion(AckCompletion { + permit: budget.permit, + completion, + metrics: metrics.clone(), + }); + let writer = self.writer.clone(); + let completions = self.completions.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 => { + publication.mark_enqueued(true); + None + } + }; + #[cfg(test)] + let waiter_gate = self.waiter_gate.clone(); + tokio::spawn(async move { + // 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)? + } + .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; + } + }); + } +} +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); + 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), + 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_eq!(queued.header.ty, FrameType::Error); + queued.finish_ack_write(true); + 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.reserve( + route_key(7, 1), + PROTOCOL_VERSION, + 891, + control_flags(), + &Arc::new(DispatchPathMetrics::new()), + ); + assert_eq!(result.err().unwrap().code, "busy"); + 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); + 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())); + 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!( + 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()); + a0.stop(); + drop(a0); + drop(pong); + writer.await.unwrap().unwrap(); + } + + 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()); + 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( + 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(), + if settling { "done" } else { "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, + if settling { + "ledger-settle" + } else { + "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); + a0.reserve( + route_key(7, 1), + PROTOCOL_VERSION, + 891, + control_flags(), + &metrics, + ) + .unwrap() + .commit(); + 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 { .. }) + )); + 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/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/bind_ack_fixup_tests.rs b/crates/aft/src/subc/bind_ack_fixup_tests.rs new file mode 100644 index 0000000000..5a24075ae4 --- /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).with_bind_ack_queue_for_test(891); + 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/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 447f8f8ac8..b709cee33f 100644 --- a/crates/aft/src/subc/mod.rs +++ b/crates/aft/src/subc/mod.rs @@ -197,8 +197,15 @@ impl fmt::Display for RouteChannel { type PushEnvelope = (ProjectRootId, PushFrame); 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; @@ -226,6 +233,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; @@ -309,6 +318,7 @@ pub enum SubcLifecycleEvent { pub struct SubcTestLifecycleProbe { events_tx: mpsc::UnboundedSender, bind_priority: Arc>>, + ack_queue_corr: Option, health_rollup: Option, } @@ -325,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)] @@ -385,15 +403,21 @@ 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 if self.ack_queue_corr == Some(corr) { + 0 + } else { + return; + }; + let _ = self.events_tx.send(SubcLifecycleEvent::RouteBindAckQueued { + corr, + reliable_push_batches, + }); } fn attach_decision(&self, attempt: u32, will_retry: bool) { @@ -1433,6 +1457,7 @@ struct RootMeta { #[derive(Debug)] struct PendingBind { + ack_publication: Option, bind_root_id: ProjectRootId, inserted_new_actor: bool, cancelled: bool, @@ -1451,6 +1476,8 @@ struct PendingBind { } struct RouteBindCompletion { + expected_snapshot_id: Option, + applied_snapshot_id: Option, route: RouteChannel, identity: RouteIdentity, bind_root_id: ProjectRootId, @@ -3769,8 +3796,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 +3856,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() { @@ -4126,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 }); @@ -4730,7 +4761,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>, @@ -4744,9 +4777,44 @@ 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); + 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), + }, + 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), + ControlCompletion::BindRetracted { route, .. } => { + a0.retire(route); + remove_installed_route(installed_route_epochs, route); + return Ok(()); + } + }; handle_route_bind_completion( writer_tx, completion, + ack_written, + a0, routes, root_channels, session_identity, @@ -4764,7 +4832,10 @@ 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, + configure_completion_rx: &mut mpsc::Receiver, + a0: &A0Egress, + management_routes: &mut HashMap>, writer_tx: &WriterSender, routes: &mut HashMap, root_channels: &mut HashMap>, @@ -4779,10 +4850,15 @@ 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, + a0, + management_routes, routes, root_channels, session_identity, @@ -4801,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. @@ -4906,6 +4990,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), @@ -4990,7 +5075,15 @@ 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); + // 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)); @@ -5128,6 +5221,9 @@ where ); match drain_pending_route_bind_completions( &mut control_completion_rx, + &mut configure_completion_rx, + &a0, + &mut management_routes, &writer_tx, &mut routes, &mut root_channels, @@ -5152,6 +5248,18 @@ where Err(error) => break Err(error), } + if let Err(error) = candidate_requests + .release(&a0, &routes, &management_routes, &dispatch_path_metrics) + .await + { + 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( @@ -5172,7 +5280,8 @@ where 0, ); if let Err(error) = expire_overdue_route_binds( - &writer_tx, + &a0.writer, + &a0, &executor, &mut pending_binds, &mut installed_route_epochs, @@ -5334,11 +5443,18 @@ where } } } - Some(completion) = control_completion_rx.recv() => { - dispatch_path_metrics.mark_frame_loop_step("process_route_bind_completion", "route_bind", completion.corr); + 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, completion, + &a0, + &mut management_routes, &mut routes, &mut root_channels, &mut session_identity, @@ -5355,6 +5471,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(); } @@ -5369,6 +5488,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), @@ -5392,6 +5514,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, @@ -5435,6 +5563,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); @@ -5514,7 +5643,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, @@ -5535,7 +5666,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, @@ -5702,7 +5833,7 @@ where Ok(()) } else if management_routes.contains_key(&route) { handle_management_request( - &writer_tx, + &a0.writer, &frame, &shared_app, &executor, @@ -6368,6 +6499,15 @@ 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(); @@ -6484,6 +6624,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) @@ -6612,6 +6754,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())) })?; @@ -6621,7 +6767,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(), @@ -6634,12 +6779,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)) = @@ -6986,10 +7133,11 @@ 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. - 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 @@ -7056,7 +7204,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>, @@ -7070,6 +7220,61 @@ 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 + .get(&route_id) + .is_some_and(|pending| !pending.cancelled && !pending.deadline_reported) + { + 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 + .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 {}", @@ -7107,8 +7312,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()); } @@ -7171,16 +7376,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( - tx, - 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( @@ -7206,9 +7415,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| { @@ -7222,31 +7428,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) @@ -7308,6 +7496,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, @@ -7318,7 +7507,16 @@ 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() + // 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, pending.corr, @@ -7327,15 +7525,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; @@ -7347,13 +7563,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, @@ -7599,6 +7811,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, @@ -7619,7 +7833,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, @@ -7690,6 +7904,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; @@ -7725,8 +7964,9 @@ async fn handle_control_request( ) .await; } + a0.supersede(route_key(route_channel, installed_epoch)); teardown_installed_route( - tx, + legacy_tx, metrics, executor, route_key(route_channel, installed_epoch), @@ -7765,15 +8005,33 @@ 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 _publication = reservation.commit(); + 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( @@ -7812,8 +8070,9 @@ async fn handle_control_request( .await; } }; + a0.supersede(route_key(route_channel, installed_epoch)); teardown_installed_route( - tx, + legacy_tx, metrics, executor, route_key(route_channel, installed_epoch), @@ -7900,22 +8159,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!({ @@ -7985,6 +8262,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(); @@ -7997,6 +8277,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(), @@ -8005,7 +8286,20 @@ 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); + 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), + || dispatch(configure_req.clone(), ctx), + ), + 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. @@ -8028,6 +8322,7 @@ async fn handle_control_request( pending_binds.insert( route_id, PendingBind { + ack_publication: Some(publication), bind_root_id: bind_root_id.clone(), inserted_new_actor, cancelled: false, @@ -8060,6 +8355,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, @@ -8073,7 +8373,7 @@ async fn handle_control_request( if send_counted_channel( &completion_tx, &completion_metrics.control_completion_queued, - completion, + ControlCompletion::ConfigureReady(completion), ) .await .is_err() @@ -8822,18 +9122,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 @@ -11705,6 +11998,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::{ @@ -13817,8 +14117,14 @@ mod tests { &fixture_path.join("absent-connection.json"), app, loop_executor, - |request, _| Response::success(request.id, json!({})), - Some(fixture_path.join("absent-user-config.json")), + |request, ctx| { + if request.command == "configure" { + crate::commands::configure::handle_configure(&request, ctx) + } else { + Response::success(request.id, json!({})) + } + }, + Some(test_support::bind_config_without_indexes(&fixture_path)), false, usize::MAX, None, @@ -13981,8 +14287,14 @@ mod tests { &fixture_path.join("absent-connection.json"), app, loop_executor, - |request, _| Response::success(request.id, json!({})), - Some(fixture_path.join("absent-user-config.json")), + |request, ctx| { + if request.command == "configure" { + crate::commands::configure::handle_configure(&request, ctx) + } else { + Response::success(request.id, json!({})) + } + }, + Some(test_support::bind_config_without_indexes(&fixture_path)), false, usize::MAX, None, @@ -14454,6 +14766,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, @@ -14482,8 +14796,14 @@ mod tests { None, &cache, &senders, - |request, _| Response::success(request.id, json!({})), - Some(&dir.path().join("absent-user-config.json")), + |request, ctx| { + if request.command == "configure" { + crate::commands::configure::handle_configure(&request, ctx) + } else { + Response::success(request.id, json!({})) + } + }, + Some(&test_support::bind_config_without_indexes(dir.path())), usize::MAX, None, ) @@ -14499,6 +14819,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!( @@ -19091,6 +19414,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, @@ -19100,6 +19425,7 @@ mod tests { let mut pending_binds = HashMap::from([( route, PendingBind { + ack_publication: None, bind_root_id: root, inserted_new_actor: false, cancelled: false, @@ -19119,11 +19445,19 @@ 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); + a0.reserve(route, PROTOCOL_VERSION, 91, control_flags(), &metrics) + .unwrap() + .commit(); handle_route_bind_completion( &writer_tx, completion, - &mut HashMap::new(), + false, + &a0, + &mut published_routes, &mut HashMap::new(), &mut HashMap::new(), &mut push_buffer, @@ -19138,7 +19472,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"); @@ -19176,6 +19544,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, @@ -19188,6 +19558,7 @@ mod tests { let mut pending_binds = HashMap::from([( route, PendingBind { + ack_publication: None, bind_root_id: root, inserted_new_actor: false, cancelled: false, @@ -19209,6 +19580,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(), @@ -19238,6 +19611,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, @@ -19266,6 +19640,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, @@ -19339,6 +19714,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, @@ -19360,6 +19736,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, @@ -19533,6 +19910,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, @@ -19580,6 +19958,7 @@ mod tests { } expire_overdue_route_binds( &writer_tx, + &A0Egress::new(writer_tx.clone(), mpsc::channel(8).0), &executor, &mut pending_binds, &mut installed, @@ -19775,6 +20154,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, @@ -19793,6 +20173,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, @@ -23393,8 +23774,14 @@ mod pong_priority_tests { &storage_path.join("connection.json"), App::default_shared(), module_executor, - |request, _| Response::success(request.id, json!({})), - Some(storage_path.join("user.json")), + |request, ctx| { + if request.command == "configure" { + crate::commands::configure::handle_configure(&request, ctx) + } else { + Response::success(request.id, json!({})) + } + }, + Some(test_support::bind_config_without_indexes(&storage_path)), false, usize::MAX, None, 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/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/src/subc/scope_workdir_tests.rs b/crates/aft/src/subc/scope_workdir_tests.rs index 8721e8dd2e..c18a3f2347 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, @@ -147,8 +150,14 @@ fn bind_observed( None, &HealthRollupCache::new(), &senders, - |request, _| Response::success(request.id, request.params), - Some(&root.join("absent-user-config.json")), + |request, ctx| { + let configured = crate::commands::configure::handle_configure(&request, ctx); + if !configured.success { + return configured; + } + Response::success(request.id, request.params) + }, + Some(&test_support::bind_config_without_indexes(&root)), usize::MAX, None, ) @@ -159,6 +168,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 +197,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 +241,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/tool_result_tests.rs b/crates/aft/src/subc/tool_result_tests.rs index 5886da4035..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(); @@ -232,7 +233,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 || { @@ -249,12 +250,12 @@ 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) } }, - Some(module_root.join("absent-user-config.json")), + Some(test_support::bind_config_without_indexes(&module_root)), false, usize::MAX, None, @@ -327,6 +328,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":{ @@ -348,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]) { @@ -498,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(); @@ -517,9 +531,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", @@ -528,7 +542,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, @@ -1422,6 +1436,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:?}")); } } diff --git a/crates/aft/src/subc/wire.rs b/crates/aft/src/subc/wire.rs index bcada549ff..59225c6d87 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,78 @@ 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()) + { + 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; + } + true + } + + 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(retracted); + } + } else { + self.ack_completion.take(); + } + if let Some((publication, receipt)) = self.ack_receipt.take() { + let _ = publication; + 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 +559,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 +600,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, diff --git a/crates/aft/src/subc_config.rs b/crates/aft/src/subc_config.rs index 69a4587d65..9a2f3e7a4e 100644 --- a/crates/aft/src/subc_config.rs +++ b/crates/aft/src/subc_config.rs @@ -18,6 +18,12 @@ use crate::harness::Harness; use std::ffi::OsStr; use std::path::{Path, PathBuf}; +mod prepared; +pub(crate) use prepared::{ + current_prepared_bind_config, read_bounded_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 +70,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 +79,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..a551a183ea --- /dev/null +++ b/crates/aft/src/subc_config/prepared.rs @@ -0,0 +1,333 @@ +//! Trusted-local config observations retained across a bind's configure attempts. + +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)] +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 { + snapshot_id: u64, + 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 { + 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), + 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 id(&self) -> u64 { + self.snapshot_id + } + + 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 { + 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 + .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 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(), + doc, + })) +} + +/// Bound I/O itself, including migration's locked reread, rather than reading +/// the entire file and checking its size after allocation. +pub(crate) fn read_bounded_config(reader: impl Read, limit: usize) -> 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) }; +} + +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_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(); + 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()); + } + + #[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/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, diff --git a/crates/aft/tests/integration/confine_admission_test.rs b/crates/aft/tests/integration/confine_admission_test.rs index 6bbe984eb0..538ba57da2 100644 --- a/crates/aft/tests/integration/confine_admission_test.rs +++ b/crates/aft/tests/integration/confine_admission_test.rs @@ -86,20 +86,39 @@ 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. + // 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); - 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(), - ))); + 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; + } + } + 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 // 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/scope_revocation_test.rs b/crates/aft/tests/integration/scope_revocation_test.rs index 96867e6483..adb646d87c 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), @@ -1849,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 ad1939cb11..2e4a555d7f 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 => { @@ -1666,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)] @@ -2469,9 +2487,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, |_, _, _| {}, @@ -3856,7 +3874,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. @@ -3865,6 +3883,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!({})) } @@ -3949,7 +3968,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( @@ -6498,7 +6518,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; @@ -7407,7 +7433,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( @@ -7418,6 +7443,34 @@ async fn drive_routebind_priority_daemon(input: FakeDaemonInput) { json!({ "prefix": flood_prefix, "count": flood_count }), ) .await; + // 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 { + 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:?}"), + } + } + 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 @@ -7428,8 +7481,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, @@ -7470,8 +7521,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; @@ -7590,8 +7639,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, @@ -7608,10 +7657,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; } @@ -7658,9 +7710,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); @@ -7805,7 +7855,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); @@ -7930,12 +7980,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, @@ -11041,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(), @@ -11086,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, @@ -14237,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, @@ -14838,13 +14890,52 @@ 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"); 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; }