diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index b4d60350..470465af 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -98,7 +98,7 @@ **Call-graph and navigation flow:** 1. Configure project root and initialize file watching -- `crates/aft/src/commands/configure.rs` -2. Query workspace-wide call dependencies via the persisted background-built callgraph store -- `crates/aft/src/callgraph_store/mod.rs`. Under read-only mode, queries run via `ReadonlyCallGraphStore` directly against the SQLite database without write or rebuild operations. +2. Query workspace-wide call dependencies via the persisted background-built callgraph store -- `crates/aft/src/callgraph_store/mod.rs`. The active database file is resolved dynamically via generation pointers (`.current` files) to allow atomic swaps and non-blocking reads. Under read-only mode, queries run via `ReadonlyCallGraphStore` directly against the SQLite database without write or rebuild operations. 3. Serve navigation commands such as callers, call-tree, impact, trace-to, and trace-data using the callgraph store adapter -- `crates/aft/src/commands/call_tree.rs`, `crates/aft/src/commands/callers.rs`, `crates/aft/src/commands/impact.rs`, `crates/aft/src/commands/trace_data.rs`, `crates/aft/src/commands/trace_to.rs`, `crates/aft/src/commands/trace_to_symbol.rs`, `crates/aft/src/commands/callgraph_store_adapter.rs`. By default, hide test files from results (controlled via the `includeTests` parameter) and collapse unresolved stdlib or external leaf calls in `call_tree` unless `includeUnresolved` is active. Truncate and return a summary (`hub_summary`) when results exceed 20 entries to save token context cost. 4. Serve symbol-level zoom inspection (`aft_zoom`), which fetches a symbol's implementation. If the target is a large container (class, struct, interface, etc., exceeding 150 lines), it renders a member-signature menu instead of the full body. For standard functions, it dedupes outgoing (`calls_out`) and incoming (`called_by`) call sites by name, aggregating duplicate occurrences under `extra_count` to minimize context token cost. @@ -137,13 +137,13 @@ **Artifact ownership and read-only caching flow:** -1. During `configure`, verify repository scopes, and resolve root-keyed cache directories: `/callgraph/` and `/inspect/` -- `crates/aft/src/commands/configure.rs`, `crates/aft/src/artifact_owner.rs`, `crates/aft/src/root_cache.rs`. +1. During `configure`, verify repository scopes, and resolve root-keyed cache directories: `/callgraph/` and `/inspect/` -- `crates/aft/src/commands/configure.rs`, `crates/aft/src/artifact_owner.rs`, `crates/aft/src/root_cache.rs`. If the `storage_dir` parameter is not supplied (e.g. daemon connections), default it to the shared CortexKit storage root `~/.local/share/cortexkit/aft/` to prevent RAM-only fallback and index regeneration. Retrieve or record the cache key in `/cache-keys.json` to memoize the mapping and avoid spawning redundant git process probes. 2. Write an `owner.json` manifest to the cache directory carrying the current checkout's scope key, path, PID, and hostname. 3. If no manifest exists, or if the existing manifest belongs to the same checkout, or if the owning process is dead (stale heartbeat or inactive process ID/hostname), reclaim and write a new lease ("Owner" mode). 4. If an active process on another checkout owns the manifest, claim "ReadOnly" mode. 5. Coordinate concurrent cache writes by acquiring a domain-specific `WriterLease` via `WriterLease::acquire_shared`. In "ReadOnly" mode or when a write lease cannot be acquired, heavy operations like cold callgraph builds, search index generation, and semantic index warming are disabled. Any search or semantic search queries read the cached index files using strict read-only openers (including `ReadonlyCallGraphStore` for callgraph reads) -- `crates/aft/src/readonly_artifacts.rs`, `crates/aft/src/root_cache.rs`, `crates/aft/src/callgraph_store/mod.rs`. -6. Write a `0600` read-marker file under `/readers//` to register active reader sessions and prevent garbage collection cleanup from deleting active database files -- `crates/aft/src/root_cache.rs`. -7. Enforce coexistence guards using `legacy_partitions.rs` to refuse write operations into legacy harness-scoped folders (`///`) and check free space requirements using a 1.5× disk-floor preflight before copying legacy folders to the new root-keyed layout. +6. Write a `0600` read-marker file under `/readers//` to register active reader sessions -- `crates/aft/src/root_cache.rs`. Active readers touch their markers to update heartbeats (at most once every 5 seconds). During sweeps, garbage collection removes old generation SQLite databases unless they have a protected read marker, or if their age exceeds the absolute retention limit of 6 hours (`MARKED_GENERATION_RETENTION_TTL`). +7. Enforce coexistence guards using `legacy_partitions.rs` to refuse write operations into legacy layout partitions and check free space requirements using a 1.5× disk-floor preflight before copying legacy folders to the new root-keyed layout. If a legacy callgraph partition is detected, migrate it using the non-blocking, online SQLite backup API (`rusqlite::Connection::backup`) running page-by-page (128 pages per step) under retry and wall-clock budgets. 8. The active "Owner" session emits periodic heartbeat file-writes to the lease manifest file during its event loop tick -- `crates/aft/src/main.rs`. ## Key Abstractions @@ -203,7 +203,7 @@ **CallGraphStore:** - Purpose: Persisted SQLite database of project-wide call dependencies. - Location: `crates/aft/src/callgraph_store/mod.rs` -- Pattern: Background-built SQLite schema containing resolved and name-only call edges, refreshed incrementally on file edits, and queried by navigation commands. Under read-only mode, queries read the SQLite file via `ReadonlyCallGraphStore` to prevent write collisions. Returns a `Building` status during cold builds. Cold-build warming is deferred while a cold semantic index seed is actively collecting or embedding. +- Pattern: Background-built SQLite schema containing resolved and name-only call edges, refreshed incrementally on file edits, and queried by navigation commands. Stores are resolved dynamically via generation pointers (`.current` files) to allow safe atomic swaps and non-blocking reads. Under read-only mode, queries read the SQLite file via `ReadonlyCallGraphStore` to prevent write collisions. Returns a `Building` status during cold builds. Cold-build warming is deferred while a cold semantic index seed is actively collecting or embedding. **Oxc Liveness Engine:** - Purpose: Perform liveness and dead-code analysis for JavaScript/TypeScript and other supported files. @@ -273,7 +273,7 @@ - Purpose: Coordinate safe multi-session access to the root-keyed project cache. - Location: `crates/aft/src/root_cache.rs` - Pattern: File-locked writer lease ensuring single-writer exclusivity combined with private read-marker JSON files (created `0600` under `/readers/`) to track active reader processes. -- Contains: `WriterLease` domain mapping (`RootCacheDomain::Callgraph` or `RootCacheDomain::Inspect`), epoch validation, and process identification metadata for reader heartbeat cleanup. +- Contains: `WriterLease` domain mapping (`RootCacheDomain::Callgraph` or `RootCacheDomain::Inspect`), epoch validation, and process identification metadata for reader heartbeat cleanup. Sweeps clean up dead-PID and expired cross-host markers and delete old SQLite generation files. **LegacyPartitionGuards:** - Purpose: Protect legacy harness-scoped folders and manage space limits during layout migration. diff --git a/crates/aft/src/bash_background/registry.rs b/crates/aft/src/bash_background/registry.rs index b103bdb7..e8bb8459 100644 --- a/crates/aft/src/bash_background/registry.rs +++ b/crates/aft/src/bash_background/registry.rs @@ -3742,7 +3742,7 @@ fn write_unix_command_script(command: &str, paths: &TaskPaths) -> Result Command { let shell = resolve_posix_shell(); - let mut cmd = Command::new(&shell); + let mut cmd = crate::effective_path::new_command(&shell); // Keep the user-provided command body out of argv and shell `-c` parsing. // The direct child is still a tiny wrapper so it can write the authoritative // exit marker after the command script exits (including if that script calls diff --git a/crates/aft/src/callgraph_store/mod.rs b/crates/aft/src/callgraph_store/mod.rs index f429fea9..2b6aa413 100644 --- a/crates/aft/src/callgraph_store/mod.rs +++ b/crates/aft/src/callgraph_store/mod.rs @@ -177,6 +177,14 @@ pub struct CallGraphStore { /// scheme this is `/.g<...>.sqlite` (resolved via the pointer) or, /// for a pre-generation store, the legacy `/.sqlite`. sqlite_path: PathBuf, + /// Root-keyed directory whose pointer controls this store. For a legacy + /// fallback this intentionally differs from `sqlite_path.parent()`, so a + /// newly published root-keyed generation invalidates the fallback reader. + publication_dir: PathBuf, + /// True only when the root-keyed read path opened data from a legacy + /// harness partition. Writer-capable callers use this to schedule migration + /// without making read-only/worktree callers acquire a writer lease. + legacy_fallback: bool, /// The generation file NAME this store opened (e.g. `.g..sqlite`), /// or `None` when it opened the legacy single-file DB. Used to detect when /// another process has published a newer generation so this process can @@ -272,6 +280,12 @@ struct SourceFingerprint { blake3: String, } +#[derive(Clone, Debug)] +struct PublishedLegacyMigration { + generation: String, + migrated_bytes: u64, +} + #[derive(Debug, Clone)] pub struct ColdBuildStats { pub files: usize, @@ -632,6 +646,8 @@ impl CallGraphStore { project_root, project_key, sqlite_path, + callgraph_dir, + false, generation, None, Some(read_marker), @@ -660,6 +676,8 @@ impl CallGraphStore { project_root, project_key, target.sqlite_path, + callgraph_dir, + true, target.generation, None, Some(read_marker), @@ -849,6 +867,57 @@ impl CallGraphStore { Ok((store, Some(stats))) } + /// Migrate a legacy harness-partition store without falling through to a + /// cold build. This is used after a query has already opened a read-only + /// fallback: the caller runs it on the same limited background lane as cold + /// builds while queries continue using that fallback. Public so crash/retry + /// tests can drive the migration synchronously on a thread where the + /// thread-local failure seams apply. + pub fn migrate_legacy_with_lease( + callgraph_dir: PathBuf, + project_root: PathBuf, + ) -> Result> { + std::fs::create_dir_all(&callgraph_dir)?; + let project_key = crate::search_index::artifact_cache_key(&project_root); + let writer_lease = acquire_writer_lease(&callgraph_dir, &project_key)?; + cleanup_incomplete_migrations(&callgraph_dir, &project_key); + + // Another writer may have completed the migration while this worker was + // waiting for the lease. Adopt its root-keyed generation rather than + // copying the legacy source a second time. + if let Some((sqlite_path, generation)) = resolve_ready_target(&callgraph_dir, &project_key) + { + let OpenedStore { store, root_repair } = Self::open_at_path( + project_root, + project_key, + sqlite_path, + generation, + true, + Some(writer_lease), + None, + )?; + return match root_repair { + OpenRootRepair::None | OpenRootRepair::ReRooted => Ok(Some(store)), + OpenRootRepair::NeedsRebuild { reason, .. } => { + Err(CallGraphStoreError::Unavailable(format!( + "root-keyed store discovered during legacy migration requires a cold rebuild: {reason}" + ))) + } + }; + } + + let store = try_legacy_migration_or_fallback( + &callgraph_dir, + &project_root, + &project_key, + writer_lease, + )?; + // A disk-floor or backup-budget failure returns a readable legacy store. + // Keep the already-resident fallback instead of sending this duplicate + // reader through the background-install channel. + Ok(store.filter(|store| !store.is_legacy_fallback())) + } + /// Build a fresh DB and publish it as a new generation, then atomically flip /// the `.current` pointer to it. NEVER replaces an open DB file, so it /// succeeds even when other processes hold an older generation open (the @@ -974,10 +1043,16 @@ impl CallGraphStore { } (None, _, _) => None, }; + let publication_dir = sqlite_path + .parent() + .map(Path::to_path_buf) + .unwrap_or_default(); let store = Self::from_connection( project_root, project_key, sqlite_path, + publication_dir, + false, generation, writer_lease, read_marker, @@ -1009,6 +1084,8 @@ impl CallGraphStore { project_root: PathBuf, project_key: String, sqlite_path: PathBuf, + publication_dir: PathBuf, + legacy_fallback: bool, generation: Option, writer_lease: Option>, read_marker: Option, @@ -1018,6 +1095,8 @@ impl CallGraphStore { project_root, project_key, sqlite_path, + publication_dir, + legacy_fallback, generation, writer_lease, read_marker, @@ -1037,6 +1116,19 @@ impl CallGraphStore { &self.sqlite_path } + /// Whether this store is reading from a legacy harness partition because + /// the root-keyed store has not published a generation yet. + pub fn is_legacy_fallback(&self) -> bool { + self.legacy_fallback + } + + pub(crate) fn is_legacy_migration(&self) -> bool { + self.generation.as_deref().is_some_and(|generation| { + migration_generation_requires_manifest(generation) + && migration_manifest_valid(&self.publication_dir, generation) + }) + } + pub fn writer_epoch_for_test(&self) -> Option<&str> { self.writer_lease.as_ref().map(|lease| lease.epoch()) } @@ -1064,10 +1156,13 @@ impl CallGraphStore { /// pointer keeps the current store (legacy DB still valid, or transient). pub fn is_current(&self) -> bool { let _ = self.refresh_read_marker(); - let Some(dir) = self.sqlite_path.parent() else { - return true; - }; - match (read_pointer(dir, &self.project_key), &self.generation) { + match ( + read_pointer(&self.publication_dir, &self.project_key), + &self.generation, + ) { + // Even when both generations happen to have the same filename, the + // root-keyed pointer names a different directory from the fallback. + (Some(_), _) if self.legacy_fallback => false, (Some(published), Some(opened)) => &published == opened, // A generation now supersedes the legacy single-file DB we opened. (Some(_), None) => false, @@ -1945,6 +2040,11 @@ impl ReadonlyCallGraphStore { self.inner.sqlite_path() } + /// Whether this reader is temporarily serving a legacy harness partition. + pub fn is_legacy_fallback(&self) -> bool { + self.inner.is_legacy_fallback() + } + pub fn is_current(&self) -> bool { self.inner.is_current() } @@ -2851,6 +2951,29 @@ fn verify_writer_lease(lease: &crate::root_cache::WriterLease) -> Result<()> { } } +fn legacy_migration_completion_line( + project_key: &str, + method: &str, + legacy_bytes: u64, + migrated_bytes: u64, +) -> String { + format!( + "migrated root-keyed callgraph store key={project_key} method={method} legacy={legacy_bytes} migrated={migrated_bytes}" + ) +} + +fn log_legacy_migration_completion( + project_key: &str, + method: &str, + legacy_bytes: u64, + migrated_bytes: u64, +) { + crate::slog_info!( + "{}", + legacy_migration_completion_line(project_key, method, legacy_bytes, migrated_bytes) + ); +} + fn try_legacy_migration_or_fallback( callgraph_dir: &Path, project_root: &Path, @@ -2878,17 +3001,18 @@ fn try_legacy_migration_or_fallback( &source, Arc::clone(&writer_lease), ) { - Ok(generation) => { - crate::slog_info!( - "migrated root-keyed callgraph store for key {} from superseded legacy generation {}", + Ok(published) => { + log_legacy_migration_completion( project_key, - source.sqlite_path.display() + "generation_copy", + source.source_bytes, + published.migrated_bytes, ); return CallGraphStore::open_generation( callgraph_dir, project_root.to_path_buf(), project_key.to_string(), - generation, + published.generation, writer_lease, ) .map(Some); @@ -2924,17 +3048,18 @@ fn try_legacy_migration_or_fallback( &source, Arc::clone(&writer_lease), ) { - Ok(generation) => { - crate::slog_info!( - "migrated root-keyed callgraph store for key {} with SQLite backup from legacy {}", + Ok(published) => { + log_legacy_migration_completion( project_key, - source.sqlite_path.display() + "sqlite_backup", + source.source_bytes, + published.migrated_bytes, ); return CallGraphStore::open_generation( callgraph_dir, project_root.to_path_buf(), project_key.to_string(), - generation, + published.generation, writer_lease, ) .map(Some); @@ -2983,6 +3108,8 @@ fn open_legacy_fallback_store( project_root.to_path_buf(), project_key.to_string(), target.sqlite_path, + callgraph_dir.to_path_buf(), + true, target.generation, None, Some(read_marker), @@ -3067,6 +3194,39 @@ fn root_storage_dir(callgraph_dir: &Path) -> Option { domain_dir.parent().map(Path::to_path_buf) } +pub(crate) fn all_legacy_partitions_migrated_for_keys( + callgraph_dir: &Path, + configured_keys: &BTreeSet, +) -> Result { + let Some(storage_root) = root_storage_dir(callgraph_dir) else { + return Ok(false); + }; + let legacy_keys = crate::legacy_partitions::inventory_legacy_partitions(&storage_root)? + .into_iter() + .filter(|entry| { + entry.kind == crate::legacy_partitions::LegacyPartitionKind::Callgraph + && configured_keys.contains(&entry.key) + }) + .map(|entry| entry.key) + .collect::>(); + if legacy_keys.is_empty() { + return Ok(false); + } + + for key in legacy_keys { + let migrated_dir = storage_root.join("callgraph").join(&key); + let Some(generation) = read_pointer(&migrated_dir, &key) else { + return Ok(false); + }; + if !migration_generation_requires_manifest(&generation) + || !migration_manifest_valid(&migrated_dir, &generation) + { + return Ok(false); + } + } + Ok(true) +} + fn newest_superseded_legacy_generation( partition: &LegacyCallgraphPartition, ) -> Result> { @@ -3179,7 +3339,7 @@ fn publish_generation_copy_migration( project_key: &str, source: &LegacyCallgraphTarget, writer_lease: Arc, -) -> Result { +) -> Result { let generation = migration_generation_file_name(project_key, "copy"); let temp_path = migration_temp_path(callgraph_dir, &generation); remove_sqlite_file_set(&temp_path); @@ -3188,17 +3348,21 @@ fn publish_generation_copy_migration( let mut source = source.clone(); let fingerprint = sqlite_file_set_fingerprint(&temp_path)?; - source.source_bytes = fingerprint.bytes; source.source_blake3 = fingerprint.blake3; - publish_migrated_generation( + let generation = publish_migrated_generation( callgraph_dir, project_key, &generation, &temp_path, &source, + fingerprint.bytes, writer_lease, "generation_copy", - ) + )?; + Ok(PublishedLegacyMigration { + generation, + migrated_bytes: fingerprint.bytes, + }) } fn publish_backup_migration( @@ -3206,7 +3370,7 @@ fn publish_backup_migration( project_key: &str, source: &LegacyCallgraphTarget, writer_lease: Arc, -) -> Result { +) -> Result { if MIGRATION_FORCE_BACKUP_BUDGET_EXHAUSTED.with(|slot| slot.get()) { return Err(CallGraphStoreError::Unavailable( "legacy callgraph backup migration budget exhausted by test seam".to_string(), @@ -3266,17 +3430,21 @@ fn publish_backup_migration( let mut source = source.clone(); let fingerprint = sqlite_file_set_fingerprint(&temp_path)?; - source.source_bytes = fingerprint.bytes; source.source_blake3 = fingerprint.blake3; - publish_migrated_generation( + let generation = publish_migrated_generation( callgraph_dir, project_key, &generation, &temp_path, &source, + fingerprint.bytes, writer_lease, "sqlite_backup", - ) + )?; + Ok(PublishedLegacyMigration { + generation, + migrated_bytes: fingerprint.bytes, + }) } fn publish_migrated_generation( @@ -3285,6 +3453,7 @@ fn publish_migrated_generation( generation: &str, temp_path: &Path, source: &LegacyCallgraphTarget, + migrated_bytes: u64, writer_lease: Arc, method: &str, ) -> Result { @@ -3296,7 +3465,7 @@ fn publish_migrated_generation( verify_writer_lease(&writer_lease)?; publish_pointer(callgraph_dir, project_key, generation)?; - write_migration_manifest(callgraph_dir, generation, source, method)?; + write_migration_manifest(callgraph_dir, generation, source, migrated_bytes, method)?; Ok(generation.to_string()) } @@ -3417,6 +3586,7 @@ fn write_migration_manifest( callgraph_dir: &Path, generation: &str, source: &LegacyCallgraphTarget, + migrated_bytes: u64, method: &str, ) -> Result<()> { let manifest_path = migration_manifest_path(callgraph_dir, generation); @@ -3434,6 +3604,7 @@ fn write_migration_manifest( "source_generation": source.generation, "source_bytes": source.source_bytes, "source_blake3": source.source_blake3, + "migrated_bytes": migrated_bytes, }); { use std::io::Write as _; @@ -8709,6 +8880,14 @@ mod cold_build_insert_tests { ); } + #[test] + fn legacy_migration_completion_log_has_operator_fields() { + assert_eq!( + legacy_migration_completion_line("abc123", "generation_copy", 176, 177), + "migrated root-keyed callgraph store key=abc123 method=generation_copy legacy=176 migrated=177" + ); + } + fn write_generation_with_age( dir: &Path, project_key: &str, @@ -8797,6 +8976,8 @@ mod cold_build_insert_tests { dir.path().to_path_buf(), project_key, sqlite_path, + dir.path().to_path_buf(), + false, Some(generation.clone()), None, None, diff --git a/crates/aft/src/cli/warmup.rs b/crates/aft/src/cli/warmup.rs index f3cb5b87..89686d85 100644 --- a/crates/aft/src/cli/warmup.rs +++ b/crates/aft/src/cli/warmup.rs @@ -563,6 +563,7 @@ fn symbol_cache_state(search_index: &SubsystemState) -> SubsystemState { /// build error). fn trigger_callgraph_warm(ctx: &AppContext) -> Option { match ctx.callgraph_store_for_ops() { + CallgraphStoreAccess::Ready(_) if ctx.callgraph_store_rx().lock().is_some() => None, CallgraphStoreAccess::Ready(_) => Some(SubsystemState::Ready), // Building (or just-started cold build) -> drive to completion via the // wait loop draining `callgraph_store_rx`. @@ -639,6 +640,9 @@ fn callgraph_store_state( if let Some(state) = override_state { return state.clone(); } + if ctx.callgraph_store_rx().lock().is_some() { + return SubsystemState::Pending("building".to_string()); + } if ctx .callgraph_store() .read() @@ -647,9 +651,6 @@ fn callgraph_store_state( { return SubsystemState::Ready; } - if ctx.callgraph_store_rx().lock().is_some() { - return SubsystemState::Pending("building".to_string()); - } // No store, no in-flight build: the cold build finished without producing a // store (failure already drained) — report ready so warmup doesn't hang. SubsystemState::Ready diff --git a/crates/aft/src/commands/configure.rs b/crates/aft/src/commands/configure.rs index ce4540da..45e39a95 100644 --- a/crates/aft/src/commands/configure.rs +++ b/crates/aft/src/commands/configure.rs @@ -1,7 +1,6 @@ use std::fs; use std::panic::{catch_unwind, AssertUnwindSafe}; use std::path::{Component, Path, PathBuf}; -use std::process::Command; use std::sync::atomic::{AtomicBool, AtomicU64, AtomicUsize, Ordering}; use std::sync::{mpsc, Arc, Mutex}; use std::thread; @@ -12,7 +11,7 @@ use notify::{RecursiveMode, Watcher}; use serde_json::{json, Value}; use std::collections::{HashMap, HashSet}; -use crate::config::SemanticBackendConfig; +use crate::config::{Config, SemanticBackendConfig}; use crate::context::{ AppContext, CallgraphStoreAccess, ConfigureMaintenanceJob, SemanticIndexEvent, SemanticIndexStatus, SemanticRefreshEvent, SemanticRefreshRequest, SemanticRefreshWorkerSlot, @@ -249,6 +248,21 @@ fn install_project_watcher(ctx: &AppContext, root_path: &Path) { install_project_watcher_with(ctx, root_path, extra_watch_paths, create_project_watcher); } +/// Restore the watcher after an idle root released its runtime. Artifact stores +/// are lazy by design, so the first request is the natural point to reattach +/// external-change invalidation as well. +pub(crate) fn ensure_project_watcher(ctx: &AppContext) { + if ctx.watcher_runtime_active() { + return; + } + let Some(root_path) = ctx.canonical_cache_root_opt() else { + return; + }; + if root_path.exists() { + install_project_watcher(ctx, &root_path); + } +} + /// Backoff for build-level retries when the embedding backend is unreachable. /// Ramps 15s -> 30s -> 60s then holds at 60s. Keeps the retry cadence cheap /// (the build re-walks files each attempt) while recovering within a minute of @@ -525,11 +539,20 @@ fn has_parent_component(path: &Path) -> bool { .any(|component| matches!(component, Component::ParentDir)) } -fn detect_worktree_bridge(project_root: &Path) -> (bool, Option) { +fn detect_worktree_bridge(ctx: &AppContext, project_root: &Path) -> (bool, Option) { if std::env::var_os("AFT_TEST_ALLOW_WORKTREE_STORE_BUILD").is_some() { return (false, None); } - let output = Command::new("git") + if let Some(result) = ctx.cached_worktree_bridge(project_root) { + return result; + } + + // This is intentionally separate from artifact-cache-key memoization. The + // cache key must validate the current HEAD, while worktree topology is + // stable until the root's `.git` marker changes. + #[cfg(test)] + ctx.record_worktree_bridge_probe_spawn_for_test(); + let output = crate::effective_path::new_command("git") .arg("-C") .arg(project_root) .args([ @@ -555,7 +578,9 @@ fn detect_worktree_bridge(project_root: &Path) -> (bool, Option) { }; let git_dir = std::fs::canonicalize(&git_dir).unwrap_or(git_dir); let common_dir = std::fs::canonicalize(&common_dir).unwrap_or(common_dir); - (git_dir != common_dir, Some(common_dir)) + let is_worktree_bridge = git_dir != common_dir; + ctx.cache_worktree_bridge(project_root, is_worktree_bridge, common_dir.clone()); + (is_worktree_bridge, Some(common_dir)) } fn semantic_fingerprint_config_changed( @@ -567,6 +592,17 @@ fn semantic_fingerprint_config_changed( || previous.base_url != next.base_url } +fn should_clear_failed_spawns( + previous: &Config, + next: &Config, + equivalent_warm_config: bool, +) -> bool { + !equivalent_warm_config + || previous.lsp_paths_extra != next.lsp_paths_extra + || previous.lsp_auto_install_binaries != next.lsp_auto_install_binaries + || previous.lsp_inflight_installs != next.lsp_inflight_installs +} + fn workspace_manifest_fingerprint(project_root: &Path) -> String { let mut parts = Vec::new(); push_manifest_fingerprint(&mut parts, project_root.join("package.json")); @@ -1454,7 +1490,7 @@ pub fn handle_configure(req: &RawRequest, ctx: &AppContext) -> Response { let canonical_cache_root = std::fs::canonicalize(&root_path).unwrap_or_else(|_| root_path.clone()); debug_assert!(canonical_cache_root.is_absolute()); - let (is_worktree_bridge, git_common_dir) = detect_worktree_bridge(&canonical_cache_root); + let (is_worktree_bridge, git_common_dir) = detect_worktree_bridge(ctx, &canonical_cache_root); let previous_config = ctx.config(); let previous_project_root = previous_config.project_root.clone(); @@ -1888,6 +1924,8 @@ pub fn handle_configure(req: &RawRequest, ctx: &AppContext) -> Response { if search_index { let cache_dir = resolve_cache_dir_with_key(&project_key, storage_dir.as_deref()); + // Unlike worktree topology, HEAD is a cache-freshness input and may + // change between equivalent rebinds, so this probe remains live. let current_head = current_git_head(&canonical_cache_root); let root_for_prewarm = canonical_cache_root.clone(); @@ -2609,10 +2647,11 @@ pub fn handle_configure(req: &RawRequest, ctx: &AppContext) -> Response { let refresh_project_runtime = !equivalent_warm_config || project_root_changed; let sync_bash_compress_flag = !equivalent_warm_config || previous_config.experimental_bash_compress != next_config.experimental_bash_compress; - let clear_failed_spawns = !equivalent_warm_config - || previous_config.lsp_paths_extra != next_config.lsp_paths_extra - || previous_config.lsp_auto_install_binaries != next_config.lsp_auto_install_binaries - || previous_config.lsp_inflight_installs != next_config.lsp_inflight_installs; + let clear_failed_spawns = should_clear_failed_spawns( + &previous_config, + &next_config, + equivalent_warm_config, + ); ctx.enqueue_configure_maintenance(ConfigureMaintenanceJob { generation: configure_generation, root_path: root_path.clone(), @@ -2770,15 +2809,16 @@ pub(crate) fn drain_deferred_configure_maintenance(ctx: &AppContext) { } let db_path = job.storage_root.join("aft.db"); - match crate::db::open(&db_path) { - Ok(conn) => { - let shared = Arc::new(Mutex::new(conn)); - ctx.set_db(shared.clone()); + match ctx.app().open_db(&db_path) { + Ok(shared) => { ctx.backup().lock().set_db_pool(shared.clone()); ctx.bash_background().set_db_pool(shared); } Err(err) => { - ctx.clear_db(); + // Do not clear the process-shared handle if another root is + // already using it. A failed root configure must not close that + // root's SQLite connection and WAL descriptors. + ctx.app().clear_db_for_path(&db_path); ctx.backup().lock().clear_db_pool(); ctx.bash_background().clear_db_pool(); slog_warn!( @@ -2890,7 +2930,6 @@ mod tests { use std::ffi::OsString; use std::io::{BufRead, BufReader, Read, Write}; use std::net::TcpListener; - #[cfg(unix)] use std::path::PathBuf; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{mpsc, Arc, Barrier, Mutex}; @@ -2898,7 +2937,7 @@ mod tests { use super::{ external_ignore_watch_paths, install_project_watcher_with, parse_lsp_paths_extra, - semantic_build_retry_backoff, validate_storage_dir, + semantic_build_retry_backoff, should_clear_failed_spawns, validate_storage_dir, }; use crate::config::{Config, SemanticBackendConfig}; use crate::context::AppContext; @@ -3520,6 +3559,56 @@ mod tests { assert_eq!(ctx.cache_role(), "main"); } + #[test] + fn configure_reuses_cached_worktree_probe_until_forced_to_reprobe() { + let _git_env = crate::test_env::hermetic_git_env_guard(); + let temp = tempfile::tempdir().unwrap(); + let root = temp.path().join("repo"); + let storage = temp.path().join("storage"); + init_git_fixture(&root); + let ctx = test_context(); + let request = || { + configure_request_with_params(json!({ + "project_root": root.clone(), + "harness": "opencode", + "storage_dir": storage.clone(), + "config": [user_tier(json!({ + "search_index": false, + "semantic_search": false, + "callgraph_store": false, + }))], + })) + }; + + assert!(handle_configure_for_test(&request(), &ctx).success); + assert_eq!(ctx.worktree_bridge_probe_spawns_for_test(), 1); + assert!(handle_configure_for_test(&request(), &ctx).success); + assert_eq!( + ctx.worktree_bridge_probe_spawns_for_test(), + 1, + "an equivalent configure must reuse the successful git topology probe" + ); + + filetime::set_file_mtime( + root.join(".git"), + filetime::FileTime::from_system_time( + std::time::SystemTime::now() + Duration::from_secs(5), + ), + ) + .expect("advance root git marker mtime"); + assert!(handle_configure_for_test(&request(), &ctx).success); + assert_eq!( + ctx.worktree_bridge_probe_spawns_for_test(), + 2, + "a changed root .git marker must invalidate the cached topology" + ); + + ctx.force_worktree_bridge_reprobe_for_test(true); + assert!(handle_configure_for_test(&request(), &ctx).success); + assert_eq!(ctx.worktree_bridge_probe_spawns_for_test(), 3); + ctx.force_worktree_bridge_reprobe_for_test(false); + } + #[test] fn handle_configure_rejects_git_like_root_when_cache_key_probe_fails_without_memo() { let _probe_lock = crate::search_index::git_root_commit_probe_override_lock_for_test(); @@ -3621,15 +3710,27 @@ mod tests { let canonical_main = std::fs::canonicalize(&main).unwrap(); let canonical_worktree = std::fs::canonicalize(&worktree).unwrap(); - let (main_is_worktree, main_common) = super::detect_worktree_bridge(&canonical_main); + let ctx = test_context(); + let (main_is_worktree, main_common) = super::detect_worktree_bridge(&ctx, &canonical_main); let (linked_is_worktree, linked_common) = - super::detect_worktree_bridge(&canonical_worktree); + super::detect_worktree_bridge(&ctx, &canonical_worktree); assert!(!main_is_worktree); assert!(linked_is_worktree); let expected_common = canonical_main.join(".git"); assert_eq!(main_common.as_deref(), Some(expected_common.as_path())); assert_eq!(linked_common, main_common); + assert_eq!(ctx.worktree_bridge_probe_spawns_for_test(), 2); + + let repeated_main = super::detect_worktree_bridge(&ctx, &canonical_main); + let repeated_worktree = super::detect_worktree_bridge(&ctx, &canonical_worktree); + assert_eq!(repeated_main, (main_is_worktree, main_common)); + assert_eq!(repeated_worktree, (linked_is_worktree, linked_common)); + assert_eq!( + ctx.worktree_bridge_probe_spawns_for_test(), + 2, + "main and linked-worktree roots must each retain their own cached result" + ); } #[test] @@ -3751,7 +3852,9 @@ mod tests { let canonical_worktree = std::fs::canonicalize(&worktree).unwrap(); let project_key = crate::search_index::artifact_cache_key(&canonical_main); let worktree_scope = crate::path_identity::project_scope_key(&canonical_worktree); - let (_, common_dir) = super::detect_worktree_bridge(&canonical_worktree); + let worktree_probe_ctx = test_context(); + let (_, common_dir) = + super::detect_worktree_bridge(&worktree_probe_ctx, &canonical_worktree); let common_dir = common_dir.expect("linked worktree common dir"); crate::artifact_owner::write_synthetic_manifest_with_git_common_dir_for_test( &storage, @@ -4847,4 +4950,14 @@ mod tests { fn semantic_max_files_defaults_to_20k() { assert_eq!(SemanticBackendConfig::default().max_files, 20_000); } + + #[test] + fn lsp_paths_extra_change_clears_failed_spawns_for_retry() { + let previous = Config::default(); + let mut next = previous.clone(); + next.lsp_paths_extra.push(PathBuf::from("/cache/lsp/.bin")); + + assert!(should_clear_failed_spawns(&previous, &next, true)); + assert!(!should_clear_failed_spawns(&previous, &previous, true)); + } } diff --git a/crates/aft/src/commands/conflicts.rs b/crates/aft/src/commands/conflicts.rs index 023aab5d..251bc374 100644 --- a/crates/aft/src/commands/conflicts.rs +++ b/crates/aft/src/commands/conflicts.rs @@ -6,7 +6,6 @@ use std::collections::HashSet; use std::path::{Path, PathBuf}; -use std::process::Command; use crate::context::AppContext; use crate::protocol::{RawRequest, Response}; @@ -24,7 +23,7 @@ struct ConflictRegion { /// Resolve the git toplevel for `base_dir`. fn git_toplevel(base_dir: &Path) -> Result { - let output = Command::new("git") + let output = crate::effective_path::new_command("git") .args(["rev-parse", "--show-toplevel"]) .current_dir(base_dir) .output() @@ -53,7 +52,7 @@ fn discover_conflicted_files(base_dir: &Path) -> Result<(PathBuf, Vec), let mut files: Vec = Vec::new(); let mut seen = HashSet::new(); - let output = Command::new("git") + let output = crate::effective_path::new_command("git") .args(["ls-files", "--unmerged"]) .current_dir(&toplevel) .output() @@ -78,7 +77,7 @@ fn discover_conflicted_files(base_dir: &Path) -> Result<(PathBuf, Vec), } } - let grep_output = Command::new("git") + let grep_output = crate::effective_path::new_command("git") .args(["grep", "-lE", r"^(<<<<<<< |>>>>>>> )"]) .current_dir(&toplevel) .output() diff --git a/crates/aft/src/commands/read.rs b/crates/aft/src/commands/read.rs index 1a874e14..00b6e1d1 100644 --- a/crates/aft/src/commands/read.rs +++ b/crates/aft/src/commands/read.rs @@ -82,7 +82,6 @@ fn is_binary(content: &[u8]) -> bool { content_inspector::inspect(content).is_binary() } - fn read_magic(path: &Path) -> std::io::Result> { let mut file = fs::File::open(path)?; let mut magic = [0u8; MEDIA_MAGIC_BYTES]; diff --git a/crates/aft/src/context.rs b/crates/aft/src/context.rs index 661dac6f..9f179002 100644 --- a/crates/aft/src/context.rs +++ b/crates/aft/src/context.rs @@ -3,7 +3,7 @@ use std::io::{self, BufWriter}; use std::path::{Component, Path, PathBuf}; use std::sync::atomic::{AtomicBool, AtomicU64, AtomicUsize, Ordering}; use std::sync::{mpsc, Arc, Mutex, RwLock}; -use std::time::{Duration, Instant}; +use std::time::{Duration, Instant, SystemTime}; use lsp_types::FileChangeType; use notify::RecommendedWatcher; @@ -597,12 +597,29 @@ pub fn default_language_provider_factory() -> Box { Box::new(TreeSitterProvider::new()) } +fn database_path_key(path: &Path) -> PathBuf { + if let Ok(canonical) = std::fs::canonicalize(path) { + return canonical; + } + let Some(parent) = path.parent() else { + return path.to_path_buf(); + }; + let canonical_parent = std::fs::canonicalize(parent).unwrap_or_else(|_| parent.to_path_buf()); + path.file_name() + .map(|name| canonical_parent.join(name)) + .unwrap_or_else(|| canonical_parent.join(path)) +} + /// Process-global services shared by all project actors in this AFT process. /// /// `App` owns only true process services. Per-root caches and the live /// language provider instance stay in [`AppContext`]. pub struct App { - db: parking_lot::Mutex>>>, + /// One process-wide handle for the current AFT database. Every project + /// actor points at this handle so roots do not open duplicate SQLite/WAL + /// descriptors for the same database. + db: parking_lot::Mutex>)>>, + active_watchers: AtomicUsize, lsp_child_registry: crate::lsp::child_registry::LspChildRegistry, stdout_writer: SharedStdoutWriter, provider_factory: LanguageProviderFactory, @@ -612,6 +629,7 @@ impl App { pub fn new(provider_factory: LanguageProviderFactory) -> Self { Self { db: parking_lot::Mutex::new(None), + active_watchers: AtomicUsize::new(0), lsp_child_registry: crate::lsp::child_registry::LspChildRegistry::new(), stdout_writer: Arc::new(Mutex::new(BufWriter::new(io::stdout()))), provider_factory, @@ -639,16 +657,66 @@ impl App { Arc::clone(&self.stdout_writer) } + /// Return the process-shared database handle, opening it only when the + /// requested path is not already resident. The connection mutex serializes + /// transactions from all roots; callers never hold the App lock while using + /// the returned connection. + pub fn open_db(&self, path: &Path) -> Result>, crate::db::OpenError> { + let key = database_path_key(path); + let mut slot = self.db.lock(); + if let Some((existing_path, conn)) = slot.as_ref() { + if existing_path == &key { + return Ok(Arc::clone(conn)); + } + } + + let conn = Arc::new(Mutex::new(crate::db::open(path)?)); + *slot = Some((key, Arc::clone(&conn))); + Ok(conn) + } + pub fn set_db(&self, conn: Arc>) { - *self.db.lock() = Some(conn); + *self.db.lock() = Some((PathBuf::new(), conn)); } pub fn clear_db(&self) { *self.db.lock() = None; } + /// Clear the shared handle only when it still refers to `path`. A failed + /// reconfigure for one root must not tear down a database used by another + /// root. + pub fn clear_db_for_path(&self, path: &Path) { + let key = database_path_key(path); + let mut slot = self.db.lock(); + if slot.as_ref().is_some_and(|(existing_path, _)| { + existing_path.as_os_str().is_empty() || existing_path == &key + }) { + *slot = None; + } + } + pub fn db(&self) -> Option>> { - self.db.lock().clone() + self.db.lock().as_ref().map(|(_, conn)| Arc::clone(conn)) + } + + pub(crate) fn watcher_started(&self) { + self.active_watchers.fetch_add(1, Ordering::SeqCst); + } + + pub(crate) fn watcher_stopped(&self) { + self.active_watchers + .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |count| { + Some(count.saturating_sub(1)) + }) + .ok(); + } + + /// Number of live watcher filter runtimes registered by this process. + /// This is intentionally process-scoped so subc tests and diagnostics can + /// verify that idle roots do not retain OS watcher threads. + pub fn watcher_count(&self) -> usize { + self.active_watchers.load(Ordering::SeqCst) } } @@ -668,6 +736,50 @@ const _: fn() = || { assert_send::(); }; +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum GitEntryKind { + Missing, + File, + Directory, + Other, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +struct GitEntrySignature { + kind: GitEntryKind, + modified: Option, +} + +#[derive(Clone, Debug)] +struct WorktreeBridgeCacheEntry { + git_entry: GitEntrySignature, + is_worktree_bridge: bool, + git_common_dir: Option, +} + +fn git_entry_signature(project_root: &Path) -> GitEntrySignature { + match std::fs::symlink_metadata(project_root.join(".git")) { + Ok(metadata) => GitEntrySignature { + kind: if metadata.file_type().is_file() { + GitEntryKind::File + } else if metadata.file_type().is_dir() { + GitEntryKind::Directory + } else { + GitEntryKind::Other + }, + modified: metadata.modified().ok(), + }, + Err(error) if error.kind() == io::ErrorKind::NotFound => GitEntrySignature { + kind: GitEntryKind::Missing, + modified: None, + }, + Err(_) => GitEntrySignature { + kind: GitEntryKind::Other, + modified: None, + }, + } +} + /// Shared application context threaded through all command handlers. /// /// Holds the language provider, backup/checkpoint stores, and configuration. @@ -710,6 +822,7 @@ pub struct AppContext { callgraph_store: RwLock>>, callgraph_store_force_rebuild: parking_lot::Mutex, callgraph_store_rx: parking_lot::Mutex>>, + callgraph_legacy_migration_summary_logged: Arc, pending_callgraph_store_paths: parking_lot::Mutex>, search_index: RwLock>, search_index_rx: RwLock>>, @@ -750,6 +863,13 @@ pub struct AppContext { configure_maintenance_jobs: parking_lot::Mutex>, artifact_cache_keys: parking_lot::Mutex>, artifact_cache_key_derivations: AtomicU64, + /// Successful git worktree probes, keyed by canonical root and guarded by + /// the root's `.git` entry shape and modification time. + worktree_bridge_cache: parking_lot::Mutex>, + #[cfg(test)] + worktree_bridge_probe_spawns: AtomicU64, + #[cfg(test)] + force_worktree_bridge_reprobe: AtomicBool, /// Last-seen value of `InspectManager::reuse_completion_count()`, so the /// per-request inspect drain can detect watcher-driven Tier-2 scans that /// finished since the previous tick and refresh the status bar (#3). @@ -823,6 +943,7 @@ impl Drop for AppContext { fn drop(&mut self) { self.artifact_owner_lease.get_mut().take(); if let Some(runtime) = self.watcher_thread.get_mut().take() { + self.app.watcher_stopped(); runtime.shutdown_and_join(); } } @@ -846,6 +967,13 @@ pub enum CallgraphStoreAccess { Error(CallGraphStoreError), } +#[derive(Clone, Copy)] +enum CallgraphBackgroundWork { + Ensure, + ForceRebuild, + LegacyMigration, +} + /// Inline wait window for a callgraph-store cold build before returning /// `Building`. Default `0` (pure-async: never block the request thread). /// Tests set `AFT_CALLGRAPH_BUILD_WAIT_MS` large so small fixture builds @@ -921,6 +1049,7 @@ impl AppContext { callgraph_store: RwLock::new(None), callgraph_store_force_rebuild: parking_lot::Mutex::new(false), callgraph_store_rx: parking_lot::Mutex::new(None), + callgraph_legacy_migration_summary_logged: Arc::new(AtomicBool::new(false)), pending_callgraph_store_paths: parking_lot::Mutex::new(BTreeSet::new()), search_index: RwLock::new(None), search_index_rx: RwLock::new(None), @@ -956,6 +1085,11 @@ impl AppContext { configure_maintenance_jobs: parking_lot::Mutex::new(VecDeque::new()), artifact_cache_keys: parking_lot::Mutex::new(BTreeMap::new()), artifact_cache_key_derivations: AtomicU64::new(0), + worktree_bridge_cache: parking_lot::Mutex::new(BTreeMap::new()), + #[cfg(test)] + worktree_bridge_probe_spawns: AtomicU64::new(0), + #[cfg(test)] + force_worktree_bridge_reprobe: AtomicBool::new(false), last_seen_reuse_completions: AtomicU64::new(0), configure_warnings_tx, configure_warnings_rx, @@ -1580,6 +1714,60 @@ impl AppContext { self.artifact_cache_keys.lock().get(canonical_root).cloned() } + /// Return a worktree probe result only while the root's `.git` marker still + /// matches the marker present when the successful probe was cached. + pub(crate) fn cached_worktree_bridge( + &self, + canonical_root: &Path, + ) -> Option<(bool, Option)> { + #[cfg(test)] + if self.force_worktree_bridge_reprobe.load(Ordering::SeqCst) { + return None; + } + + let signature = git_entry_signature(canonical_root); + self.worktree_bridge_cache + .lock() + .get(canonical_root) + .filter(|entry| entry.git_entry == signature) + .map(|entry| (entry.is_worktree_bridge, entry.git_common_dir.clone())) + } + + /// Cache only successful git worktree probes. Failed probes remain retryable + /// because a transient process or filesystem error must not become sticky. + pub(crate) fn cache_worktree_bridge( + &self, + canonical_root: &Path, + is_worktree_bridge: bool, + git_common_dir: PathBuf, + ) { + self.worktree_bridge_cache.lock().insert( + canonical_root.to_path_buf(), + WorktreeBridgeCacheEntry { + git_entry: git_entry_signature(canonical_root), + is_worktree_bridge, + git_common_dir: Some(git_common_dir), + }, + ); + } + + #[cfg(test)] + pub(crate) fn record_worktree_bridge_probe_spawn_for_test(&self) { + self.worktree_bridge_probe_spawns + .fetch_add(1, Ordering::SeqCst); + } + + #[cfg(test)] + pub(crate) fn worktree_bridge_probe_spawns_for_test(&self) -> u64 { + self.worktree_bridge_probe_spawns.load(Ordering::SeqCst) + } + + #[cfg(test)] + pub(crate) fn force_worktree_bridge_reprobe_for_test(&self, enabled: bool) { + self.force_worktree_bridge_reprobe + .store(enabled, Ordering::SeqCst); + } + pub fn memoized_artifact_cache_key(&self, canonical_root: &Path) -> String { let mut keys = self.artifact_cache_keys.lock(); if let Some(key) = keys.get(canonical_root).cloned() { @@ -1973,6 +2161,7 @@ impl AppContext { if !self.heavy_root_work_allowed() { return Ok(None); } + self.revalidate_callgraph_store_generation(); if let Some(store) = { let guard = self .callgraph_store @@ -1980,6 +2169,11 @@ impl AppContext { .unwrap_or_else(std::sync::PoisonError::into_inner); guard.as_ref().map(Arc::clone) } { + self.schedule_legacy_callgraph_migration_if_needed( + store.as_ref(), + store.project_root().to_path_buf(), + self.callgraph_store_dir(), + ); return Ok(Some(store)); } @@ -1988,29 +2182,52 @@ impl AppContext { }; let callgraph_dir = self.callgraph_store_dir(); let force_rebuild = self.take_callgraph_store_force_rebuild(); - if self.callgraph_writer() { - if force_rebuild { - let files = crate::callgraph::walk_project_files(&project_root).collect::>(); - let (store, _stats) = CallGraphStore::cold_build_with_lease_chunked( - callgraph_dir.clone(), - project_root.clone(), - &files, - self.config().callgraph_chunk_size, - )?; - drop(store); - } else if CallGraphStore::needs_cold_build(&callgraph_dir, &project_root)? { - let files = crate::callgraph::walk_project_files(&project_root).collect::>(); - let (store, _stats) = CallGraphStore::ensure_built_with_lease_chunked( - callgraph_dir.clone(), - project_root.clone(), - &files, - self.config().callgraph_chunk_size, - )?; - drop(store); + + // Preserve a readable legacy fallback while writer-capable processes + // migrate it on the cold-build lane. Opening before the writer path is + // also the cheap fast path for an already-published root generation. + if !force_rebuild { + if let Some(store) = + CallGraphStore::open_readonly(callgraph_dir.clone(), project_root.clone())? + { + let store = Arc::new(store); + { + let mut guard = self + .callgraph_store + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner); + *guard = Some(Arc::clone(&store)); + } + self.schedule_legacy_callgraph_migration_if_needed( + store.as_ref(), + project_root, + callgraph_dir, + ); + return Ok(Some(store)); } - } else if force_rebuild { + } + + if !self.callgraph_writer() { return Ok(None); } + let files = crate::callgraph::walk_project_files(&project_root).collect::>(); + if force_rebuild { + let (store, _stats) = CallGraphStore::cold_build_with_lease_chunked( + callgraph_dir.clone(), + project_root.clone(), + &files, + self.config().callgraph_chunk_size, + )?; + drop(store); + } else { + let (store, _stats) = CallGraphStore::ensure_built_with_lease_chunked( + callgraph_dir.clone(), + project_root.clone(), + &files, + self.config().callgraph_chunk_size, + )?; + drop(store); + } let Some(store) = CallGraphStore::open_readonly(callgraph_dir, project_root)? else { return Ok(None); @@ -2037,43 +2254,34 @@ impl AppContext { }) } - /// Access the persisted callgraph store for the five store-backed edge-query - /// ops **without ever blocking the request thread on a cold build**. - /// - /// - Store resident -> `Ready`. - /// - Warm on-disk DB present -> opened synchronously (cheap) -> `Ready`. - /// - Genuine cold build needed -> kicked off in the background, returns - /// `Building`; the watcher keeps the store fresh once it lands. - /// - Worktree without a built store, or not configured -> `Unavailable`. - /// - /// A build already in flight (`callgraph_store_rx` set) also returns - /// `Building` without starting a second build. - /// Drop the resident callgraph store when another process (or a local cold - /// rebuild) has published a newer generation, so the next access reopens via - /// the pointer. No-op when no store is resident, a build is in flight, or the - /// store is still current. Must run before serving ops AND before any - /// incremental write, so every process converges on the current generation - /// rather than writing to a stale one. + /// Drop a cached reader when another process published a newer generation. + /// The next access reopens through the pointer and converges to that + /// generation instead of serving a stale long-lived connection. pub fn revalidate_callgraph_store_generation(&self) { - // Never disturb the store while a background build's result is pending - // install (the rx-install path replaces it wholesale). - if self.callgraph_store_rx.lock().is_some() { - return; - } - let superseded = { + let (superseded, legacy_fallback) = { let guard = self .callgraph_store .read() .unwrap_or_else(std::sync::PoisonError::into_inner); - guard.as_ref().is_some_and(|store| !store.is_current()) + guard + .as_ref() + .map(|store| (!store.is_current(), store.is_legacy_fallback())) + .unwrap_or((false, false)) }; - if superseded { - let mut guard = self - .callgraph_store - .write() - .unwrap_or_else(std::sync::PoisonError::into_inner); - *guard = None; + if !superseded { + return; } + // A local migration publishes its pointer just before sending the new + // store to the main-loop drain. Keep queries on the fallback during that + // narrow handoff instead of reporting a transient Building state. + if legacy_fallback && self.callgraph_store_rx.lock().is_some() { + return; + } + let mut guard = self + .callgraph_store + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner); + *guard = None; } pub fn callgraph_store_for_ops(&self) -> CallgraphStoreAccess { @@ -2092,6 +2300,11 @@ impl AppContext { .unwrap_or_else(std::sync::PoisonError::into_inner); guard.as_ref().map(Arc::clone) } { + self.schedule_legacy_callgraph_migration_if_needed( + store.as_ref(), + store.project_root().to_path_buf(), + self.callgraph_store_dir(), + ); return CallgraphStoreAccess::Ready(store); } @@ -2117,6 +2330,11 @@ impl AppContext { .unwrap_or_else(std::sync::PoisonError::into_inner); *guard = Some(Arc::clone(&store)); } + self.schedule_legacy_callgraph_migration_if_needed( + store.as_ref(), + project_root.clone(), + callgraph_dir.clone(), + ); return CallgraphStoreAccess::Ready(store); } Ok(None) => { @@ -2150,11 +2368,13 @@ impl AppContext { // `AFT_CALLGRAPH_BUILD_WAIT_MS` (default 0) optionally waits a bounded // window inline for the build to land before returning `Building`; tests // set it large so fixture builds resolve to `Ready` synchronously. - if !self.spawn_callgraph_store_cold_build( - project_root.clone(), - callgraph_dir.clone(), - force_rebuild, - ) { + let work = if force_rebuild { + CallgraphBackgroundWork::ForceRebuild + } else { + CallgraphBackgroundWork::Ensure + }; + if !self.spawn_callgraph_store_cold_build(project_root.clone(), callgraph_dir.clone(), work) + { return CallgraphStoreAccess::Building; } @@ -2213,17 +2433,52 @@ impl AppContext { CallgraphStoreAccess::Building } - /// Atomically mark a cold build in-flight and spawn the background builder. - /// - /// The `callgraph_store_rx` lock covers the full check + receiver install + - /// thread spawn sequence, so concurrent cold callers cannot both observe an - /// empty in-flight slot and double-spawn builders. Returns `false` when - /// another caller already has a build in flight. + fn schedule_legacy_callgraph_migration_if_needed( + &self, + store: &ReadonlyCallGraphStore, + project_root: PathBuf, + callgraph_dir: PathBuf, + ) { + if !store.is_legacy_fallback() + || !self.callgraph_writer() + || !self.heavy_root_work_allowed() + { + return; + } + if self.semantic_cold_seed_active() { + self.defer_callgraph_store_warm_for_semantic_cold_seed(); + return; + } + let _ = self.spawn_callgraph_store_cold_build( + project_root, + callgraph_dir, + CallgraphBackgroundWork::LegacyMigration, + ); + } + + fn configured_callgraph_keys(&self, current_root: &Path) -> BTreeSet { + let mut roots = self + .configured_session_roots + .lock() + .iter() + .map(|(root, _session)| root.clone()) + .collect::>(); + roots.insert(current_root.to_path_buf()); + roots + .iter() + .map(|root| crate::search_index::artifact_cache_key(root)) + .collect() + } + + /// Atomically mark root-keyed callgraph maintenance in flight and spawn it + /// on the cold-build lane. The same receiver/install path handles cold + /// builds and legacy migrations, so watcher edits are queued and replayed + /// against whichever root-keyed generation publishes. fn spawn_callgraph_store_cold_build( &self, project_root: PathBuf, callgraph_dir: PathBuf, - force_rebuild: bool, + work: CallgraphBackgroundWork, ) -> bool { if !self.heavy_root_work_allowed() || !self.callgraph_writer() { return false; @@ -2233,6 +2488,8 @@ impl AppContext { let chunk_size = self.config().callgraph_chunk_size; let build_generation = self.configure_generation(); let generation_flag = self.configure_generation_flag(); + let configured_keys = self.configured_callgraph_keys(&project_root); + let summary_logged = Arc::clone(&self.callgraph_legacy_migration_summary_logged); let mut rx_guard = self.callgraph_store_rx.lock(); if rx_guard.is_some() { @@ -2241,13 +2498,13 @@ impl AppContext { let Some(permit) = crate::cold_build_limiter::try_acquire() else { crate::slog_info!( - "callgraph store cold build deferred by cold build limit ({})", + "callgraph store background work deferred by cold build limit ({})", crate::cold_build_limiter::limit() ); return false; }; - if force_rebuild { + if matches!(work, CallgraphBackgroundWork::ForceRebuild) { // Consume the force flag now so a follow-up request doesn't queue a // second forced build while this one is in flight. self.take_callgraph_store_force_rebuild(); @@ -2260,26 +2517,64 @@ impl AppContext { std::thread::spawn(move || { let _permit = permit; crate::log_ctx::with_session(session_id, || { - let files = crate::callgraph::walk_project_files(&project_root).collect::>(); - let built = if force_rebuild { - CallGraphStore::cold_build_with_lease_chunked( - callgraph_dir, - project_root, - &files, - chunk_size, - ) - .map(|(store, _)| store) - } else { - CallGraphStore::ensure_built_with_lease_chunked( - callgraph_dir, - project_root, - &files, - chunk_size, - ) - .map(|(store, _)| store) + let built = match work { + CallgraphBackgroundWork::LegacyMigration => { + CallGraphStore::migrate_legacy_with_lease( + callgraph_dir.clone(), + project_root.clone(), + ) + } + CallgraphBackgroundWork::ForceRebuild => { + let files = + crate::callgraph::walk_project_files(&project_root).collect::>(); + CallGraphStore::cold_build_with_lease_chunked( + callgraph_dir.clone(), + project_root.clone(), + &files, + chunk_size, + ) + .map(|(store, _)| Some(store)) + } + CallgraphBackgroundWork::Ensure => { + let files = + crate::callgraph::walk_project_files(&project_root).collect::>(); + CallGraphStore::ensure_built_with_lease_chunked( + callgraph_dir.clone(), + project_root.clone(), + &files, + chunk_size, + ) + .map(|(store, _)| Some(store)) + } }; match built { - Ok(store) => { + Ok(Some(store)) => { + if store.is_legacy_migration() { + match crate::callgraph_store::all_legacy_partitions_migrated_for_keys( + &callgraph_dir, + &configured_keys, + ) { + Ok(true) + if summary_logged + .compare_exchange( + false, + true, + Ordering::SeqCst, + Ordering::SeqCst, + ) + .is_ok() => + { + crate::slog_info!( + "all legacy callgraph partitions migrated for configured roots" + ); + } + Ok(_) => {} + Err(error) => crate::slog_warn!( + "failed to inspect legacy callgraph migration completion: {}", + error + ), + } + } if generation_flag.load(Ordering::SeqCst) == build_generation { let _ = tx.send(store); } else { @@ -2289,10 +2584,11 @@ impl AppContext { ); } } + Ok(None) => {} Err(error) => { - crate::slog_warn!("callgraph store cold build failed: {}", error); + crate::slog_warn!("callgraph store background work failed: {}", error); // Dropping tx disconnects the channel; the drain clears - // the receiver so a later op can retry the build. + // the receiver so a later op can retry the work. } } }); @@ -2957,20 +3253,97 @@ impl AppContext { rx: crossbeam_channel::Receiver, runtime: WatcherThreadHandle, ) { + let replaced = self.watcher_thread.lock().replace(runtime); + if let Some(runtime) = replaced { + self.app.watcher_stopped(); + runtime.shutdown_and_join(); + } else { + self.app.watcher_started(); + } *self.watcher_rx.lock() = Some(rx); - *self.watcher_thread.lock() = Some(runtime); } /// Stop the watcher filter thread (if any) and clear the dispatch receiver. /// Used on reconfigure, watcher failure, root deletion, and test teardown. pub fn stop_watcher_runtime(&self) { if let Some(runtime) = self.watcher_thread.lock().take() { + self.app.watcher_stopped(); runtime.shutdown_and_join(); } *self.watcher_rx.lock() = None; *self.watcher.lock() = None; } + /// Request watcher shutdown without joining on the executor lane. Some + /// platform watcher backends can block while their OS thread unwinds, so + /// idle-root and root-deleted cleanup performs the join on a detached + /// reaper thread instead. + pub fn stop_watcher_runtime_in_background(&self) { + if let Some(runtime) = self.watcher_thread.lock().take() { + self.app.watcher_stopped(); + std::thread::spawn(move || runtime.shutdown_and_join()); + } + *self.watcher_rx.lock() = None; + *self.watcher.lock() = None; + } + + /// Process-scoped watcher count used by maintenance diagnostics and + /// regression tests. The count drops as soon as shutdown is requested. + pub fn watcher_registry_count(&self) -> usize { + self.app.watcher_count() + } + + pub(crate) fn watcher_runtime_active(&self) -> bool { + self.watcher_thread.lock().is_some() + } + + /// Return whether artifact eviction would discard work that still needs a + /// live handle. Callers use this as the single safety gate before clearing + /// resident stores and inspect caches. + pub fn artifact_eviction_blocked(&self) -> bool { + if crate::runtime_drain::any_build_in_flight(self) + || self.inspect_manager.tier2_any_in_flight() + || !self.bash_background.running_tasks().is_empty() + || !self.pending_callgraph_store_paths.lock().is_empty() + || !self.pending_search_index_paths.lock().is_empty() + || !self.pending_tier2_paths.lock().is_empty() + || !self.pending_semantic_index_paths.lock().is_empty() + || *self.pending_semantic_corpus_refresh.lock() + { + return true; + } + + let search_has_pending_disk_changes = self + .search_index + .read() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .as_ref() + .is_some_and(SearchIndex::has_pending_disk_changes); + search_has_pending_disk_changes + } + + /// Drop idle root-scoped artifact handles. Persistent data remains on disk + /// and each command's existing lazy-open path recreates the handle later. + /// Returns false when an active build, bash task, inspect scan, or pending + /// disk update makes eviction unsafe. + pub fn evict_idle_artifacts(&self) -> bool { + if self.artifact_eviction_blocked() { + return false; + } + + self.callgraph_store + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .take(); + self.search_index + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .take(); + self.inspect_manager.evict_idle_caches(); + self.reset_symbol_cache(); + true + } + /// Access the LSP manager. pub fn lsp(&self) -> parking_lot::MutexGuard<'_, LspManager> { self.lsp_manager.lock() @@ -4493,6 +4866,44 @@ mod harness_path_tests { } } +#[cfg(test)] +mod shared_db_tests { + use super::*; + use tempfile::tempdir; + + #[test] + fn app_contexts_share_one_database_connection() { + let storage = tempdir().expect("storage tempdir"); + let root_one = tempdir().expect("first root tempdir"); + let root_two = tempdir().expect("second root tempdir"); + let app = App::default_shared(); + let ctx_one = AppContext::from_app( + Arc::clone(&app), + Config { + project_root: Some(root_one.path().to_path_buf()), + ..Config::default() + }, + ); + let ctx_two = AppContext::from_app( + Arc::clone(&app), + Config { + project_root: Some(root_two.path().to_path_buf()), + ..Config::default() + }, + ); + let path = storage.path().join("aft.db"); + + let first = app.open_db(&path).expect("open shared database"); + let second = app.open_db(&path).expect("reuse shared database"); + + assert!(Arc::ptr_eq(&first, &second)); + assert!(Arc::ptr_eq( + &ctx_one.db().expect("first context database"), + &ctx_two.db().expect("second context database") + )); + } +} + #[cfg(test)] mod gitignore_tests { use super::*; diff --git a/crates/aft/src/effective_path.rs b/crates/aft/src/effective_path.rs index 579e2db2..facc5f83 100644 --- a/crates/aft/src/effective_path.rs +++ b/crates/aft/src/effective_path.rs @@ -20,10 +20,27 @@ use std::time::{Duration, Instant}; #[cfg(unix)] use std::os::unix::ffi::{OsStrExt, OsStringExt}; #[cfg(unix)] +use std::os::unix::io::AsRawFd; +#[cfg(unix)] use std::os::unix::process::CommandExt; +#[cfg(unix)] +use serde::{Deserialize, Serialize}; + +#[cfg(not(unix))] static EFFECTIVE_PATH: OnceLock = OnceLock::new(); +#[cfg(unix)] +#[derive(Clone, Debug)] +struct PathState { + path: &'static OsStr, + is_fallback: bool, + last_probe_attempt: Option, +} + +#[cfg(unix)] +static EFFECTIVE_PATH_STATE: std::sync::Mutex> = std::sync::Mutex::new(None); + #[cfg(unix)] const LOGIN_SHELL_PATH_PROBE_TIMEOUT: Duration = Duration::from_secs(3); @@ -46,16 +63,141 @@ pub fn initialize_process_path() -> &'static OsStr { path } +/// Create a new `Command` with the effective PATH set on Unix. +#[cfg(unix)] +pub fn new_command>(program: S) -> std::process::Command { + let mut cmd = std::process::Command::new(program); + cmd.env("PATH", effective_path()); + cmd +} + +/// On Windows the process PATH is already correct (registry-backed +/// environment block); pass through without touching the child env. +#[cfg(not(unix))] +pub fn new_command>(program: S) -> std::process::Command { + std::process::Command::new(program) +} + /// Return the cached PATH that subprocesses should inherit. /// /// On Windows this is the process PATH unchanged: Windows daemon environments /// already receive PATH from the registry-backed environment block. +#[cfg(not(unix))] pub fn effective_path() -> &'static OsStr { EFFECTIVE_PATH .get_or_init(compute_effective_path) .as_os_str() } +#[cfg(unix)] +pub fn effective_path() -> &'static OsStr { + // Test seam: integration tests construct exact PATHs (e.g. to simulate a + // missing formatter binary); probing and enrichment would re-add real tool + // dirs from the host and break that isolation. Checked at runtime because + // the spawned test binary is a production build. + // "0" reads as unset so the PATH feature's own integration tests can + // opt back in to probing under a test harness that defaults the seam on. + if std::env::var_os("AFT_TEST_RAW_PATH").is_some_and(|v| v != "0" && !v.is_empty()) { + static RAW: OnceLock = OnceLock::new(); + return RAW + .get_or_init(|| std::env::var_os("PATH").unwrap_or_default()) + .as_os_str(); + } + let mut guard = EFFECTIVE_PATH_STATE + .lock() + .unwrap_or_else(|e| e.into_inner()); + let state_opt = guard.clone(); + if let Some(state) = state_opt { + if state.is_fallback { + let now = Instant::now(); + let should_retry = state + .last_probe_attempt + .map(|last| now.duration_since(last) >= Duration::from_secs(60)) + .unwrap_or(true); + + if should_retry { + let last_attempt = Some(now); + let mut temp_state = state.clone(); + temp_state.last_probe_attempt = last_attempt; + *guard = Some(temp_state); + drop(guard); + + let new_path_opt = probe_login_shell_path(); + + let mut guard = EFFECTIVE_PATH_STATE + .lock() + .unwrap_or_else(|e| e.into_inner()); + if let Some(new_path) = new_path_opt { + write_login_path_memo(&new_path); + let current = std::env::var_os("PATH").unwrap_or_default(); + let merged = merge_login_and_current_path(&new_path, ¤t); + let home = std::env::var_os("HOME"); + let enriched = + append_missing_standard_dirs(&merged, home.as_deref(), |dir| dir.is_dir()); + let leaked: &'static OsStr = Box::leak(enriched.into_boxed_os_str()); + let new_state = PathState { + path: leaked, + is_fallback: false, + last_probe_attempt: None, + }; + *guard = Some(new_state); + return leaked; + } else { + let memo_path_opt = read_login_path_memo(); + if let Some(mut current_state) = guard.clone() { + current_state.last_probe_attempt = Some(Instant::now()); + if let Some(memo_path) = memo_path_opt { + let current = std::env::var_os("PATH").unwrap_or_default(); + let merged = merge_login_and_current_path(&memo_path, ¤t); + let home = std::env::var_os("HOME"); + let enriched = + append_missing_standard_dirs(&merged, home.as_deref(), |dir| { + dir.is_dir() + }); + let leaked: &'static OsStr = Box::leak(enriched.into_boxed_os_str()); + current_state.path = leaked; + current_state.is_fallback = false; + } + *guard = Some(current_state); + return guard.as_ref().unwrap().path; + } + } + } + } + return state.path; + } + + let current = std::env::var_os("PATH").unwrap_or_default(); + let home = std::env::var_os("HOME"); + + let mut is_fallback = false; + let mut last_probe_attempt = None; + + let path = if !path_is_impoverished(¤t, home.as_deref(), |dir| dir.is_dir()) { + current.to_os_string() + } else { + if let Some(login_path) = probe_login_shell_path() { + write_login_path_memo(&login_path); + merge_login_and_current_path(&login_path, ¤t) + } else if let Some(memo_path) = read_login_path_memo() { + merge_login_and_current_path(&memo_path, ¤t) + } else { + is_fallback = true; + last_probe_attempt = Some(Instant::now()); + current.to_os_string() + } + }; + + let enriched = append_missing_standard_dirs(&path, home.as_deref(), |dir| dir.is_dir()); + let leaked: &'static OsStr = Box::leak(enriched.into_boxed_os_str()); + *guard = Some(PathState { + path: leaked, + is_fallback, + last_probe_attempt, + }); + leaked +} + #[cfg(windows)] fn compute_effective_path() -> OsString { std::env::var_os("PATH").unwrap_or_default() @@ -66,47 +208,13 @@ fn compute_effective_path() -> OsString { std::env::var_os("PATH").unwrap_or_default() } -#[cfg(unix)] -fn compute_effective_path() -> OsString { - let current = std::env::var_os("PATH").unwrap_or_default(); - let home = std::env::var_os("HOME"); - compute_effective_path_with( - ¤t, - home.as_deref(), - |dir| dir.is_dir(), - probe_login_shell_path, - ) -} - -#[cfg(unix)] -fn compute_effective_path_with( - current: &OsStr, - home: Option<&OsStr>, - dir_exists: D, - mut probe_login_path: P, -) -> OsString -where - D: FnMut(&Path) -> bool, - P: FnMut() -> Option, -{ - if !path_is_impoverished(current, home, dir_exists) { - return current.to_os_string(); - } - - let Some(login_path) = probe_login_path().filter(|path| login_path_is_acceptable(path)) else { - return current.to_os_string(); - }; - - merge_login_and_current_path(&login_path, current) -} - #[cfg(unix)] fn path_is_impoverished(current: &OsStr, home: Option<&OsStr>, mut dir_exists: D) -> bool where D: FnMut(&Path) -> bool, { let current_entries: Vec = std::env::split_paths(current).collect(); - user_standard_path_dirs(home).into_iter().any(|dir| { + core_standard_path_dirs(home).into_iter().any(|dir| { dir_exists(&dir) && !current_entries .iter() @@ -114,8 +222,13 @@ where }) } +/// Core tool dirs whose absence from PATH signals an impoverished daemon +/// environment worth paying a login-shell probe for. Deliberately excludes +/// the interactive-gated extras below: those are appended unconditionally by +/// `append_missing_standard_dirs`, so missing them alone never justifies a +/// probe. #[cfg(unix)] -fn user_standard_path_dirs(home: Option<&OsStr>) -> Vec { +fn core_standard_path_dirs(home: Option<&OsStr>) -> Vec { let mut dirs = vec![ PathBuf::from("/opt/homebrew/bin"), PathBuf::from("/usr/local/bin"), @@ -128,6 +241,45 @@ fn user_standard_path_dirs(home: Option<&OsStr>) -> Vec { dirs } +/// All dirs merged into every constructed PATH when present on disk. Includes +/// installers that only amend interactive shell rc blocks (bun, pnpm, mise, +/// deno, volta), which even a successful login-shell probe cannot see. +#[cfg(unix)] +fn user_standard_path_dirs(home: Option<&OsStr>) -> Vec { + let mut dirs = core_standard_path_dirs(home); + if let Some(home) = home { + let home = PathBuf::from(home); + dirs.push(home.join(".bun/bin")); + dirs.push(home.join("Library/pnpm")); + dirs.push(home.join(".local/share/pnpm")); + dirs.push(home.join(".local/share/mise/shims")); + dirs.push(home.join(".deno/bin")); + dirs.push(home.join(".volta/bin")); + } + dirs +} + +#[cfg(unix)] +fn append_missing_standard_dirs( + path: &OsStr, + home: Option<&OsStr>, + mut dir_exists: D, +) -> OsString +where + D: FnMut(&Path) -> bool, +{ + let mut entries: Vec = std::env::split_paths(path).collect(); + let mut seen: HashSet = entries.iter().cloned().collect(); + + for dir in user_standard_path_dirs(home) { + if dir_exists(&dir) && seen.insert(dir.clone()) { + entries.push(dir); + } + } + + std::env::join_paths(entries).unwrap_or_else(|_| path.to_os_string()) +} + #[cfg(unix)] fn probe_login_shell_path() -> Option { let candidates = login_shell_candidates(); @@ -144,19 +296,60 @@ fn probe_login_shell_path() -> Option { #[cfg(unix)] fn login_shell_candidates() -> Vec { + if cfg!(test) { + if let Some(val) = std::env::var_os("AFT_TEST_LOGIN_SHELL_CANDIDATES") { + return std::env::split_paths(&val).collect(); + } + } + let mut candidates = Vec::new(); if let Some(shell) = std::env::var_os("SHELL").filter(|value| !value.is_empty()) { - return vec![PathBuf::from(shell)]; + candidates.push(PathBuf::from(shell)); + } + let zsh = PathBuf::from("/bin/zsh"); + let bash = PathBuf::from("/bin/bash"); + if !candidates.contains(&zsh) { + candidates.push(zsh); } - vec![PathBuf::from("/bin/zsh"), PathBuf::from("/bin/bash")] + if !candidates.contains(&bash) { + candidates.push(bash); + } + candidates +} + +#[cfg(unix)] +fn set_nonblocking(file: &F) -> std::io::Result<()> { + let fd = file.as_raw_fd(); + unsafe { + let flags = libc::fcntl(fd, libc::F_GETFL); + if flags < 0 { + return Err(std::io::Error::last_os_error()); + } + if libc::fcntl(fd, libc::F_SETFL, flags | libc::O_NONBLOCK) < 0 { + return Err(std::io::Error::last_os_error()); + } + } + Ok(()) } #[cfg(unix)] fn probe_login_shell_path_once(shell: &Path, timeout: Duration) -> Option { let mut command = Command::new(shell); + let is_fish = shell + .file_name() + .and_then(|n| n.to_str()) + .map(|s| s.eq_ignore_ascii_case("fish")) + .unwrap_or(false); + + let cmd_str = if is_fish { + r#"printf %s (string join : $PATH)"# + } else { + r#"printf %s "$PATH""# + }; + command .arg("-l") .arg("-c") - .arg(r#"printf %s "$PATH""#) + .arg(cmd_str) .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(Stdio::null()); @@ -171,21 +364,68 @@ fn probe_login_shell_path_once(shell: &Path, timeout: Duration) -> Option { + let wait_deadline = Instant::now() + Duration::from_secs(1); + loop { + match child.try_wait() { + Ok(Some(_)) => break, + Ok(None) if Instant::now() >= wait_deadline => { + kill_login_shell_probe(&mut child); + break; + } + Ok(None) => { + std::thread::sleep(Duration::from_millis(10)); + } + Err(_) => { + kill_login_shell_probe(&mut child); + break; + } + } + } + return Some(OsString::from_vec(output_bytes)); + } + Ok(n) => { + output_bytes.extend_from_slice(&buf[..n]); + } + Err(ref e) if e.kind() == std::io::ErrorKind::WouldBlock => { + // No data available right now + } + Err(_) => { + kill_login_shell_probe(&mut child); + return None; + } + } + match child.try_wait() { Ok(Some(_)) => { - let output = child.wait_with_output().ok()?; - return Some(OsString::from_vec(output.stdout)); + loop { + match stdout.read(&mut buf) { + Ok(0) => break, + Ok(n) => output_bytes.extend_from_slice(&buf[..n]), + Err(ref e) if e.kind() == std::io::ErrorKind::WouldBlock => { + break; + } + Err(_) => break, + } + } + let _ = child.wait(); + return Some(OsString::from_vec(output_bytes)); } Ok(None) if Instant::now() >= deadline => { kill_login_shell_probe(&mut child); return None; } Ok(None) => { - let remaining = deadline.saturating_duration_since(Instant::now()); - std::thread::sleep(remaining.min(Duration::from_millis(25))); + std::thread::sleep(Duration::from_millis(25)); } Err(_) => { kill_login_shell_probe(&mut child); @@ -219,6 +459,18 @@ fn login_path_is_acceptable(path: &OsStr) -> bool { return false; } + if bytes.contains(&b' ') { + let mut abs_count = 0; + for part in bytes.split(|&b| b == b' ') { + if part.first() == Some(&b'/') { + abs_count += 1; + } + } + if abs_count > 1 { + return false; + } + } + bytes .split(|byte| *byte == b':') .all(|entry| entry.first() == Some(&b'/')) @@ -238,6 +490,34 @@ fn merge_login_and_current_path(login_path: &OsStr, current_path: &OsStr) -> OsS std::env::join_paths(merged).unwrap_or_else(|_| current_path.to_os_string()) } +#[cfg(unix)] +#[derive(Serialize, Deserialize)] +struct LoginPathMemo { + login_path: String, +} + +#[cfg(unix)] +fn read_login_path_memo() -> Option { + let memo_path = crate::bash_background::storage_dir(None).join("login-path-memo.json"); + let content = std::fs::read_to_string(&memo_path).ok()?; + let memo: LoginPathMemo = serde_json::from_str(&content).ok()?; + Some(OsString::from(memo.login_path)) +} + +#[cfg(unix)] +fn write_login_path_memo(path: &OsStr) { + let memo_path = crate::bash_background::storage_dir(None).join("login-path-memo.json"); + if let Some(parent) = memo_path.parent() { + let _ = std::fs::create_dir_all(parent); + } + let memo = LoginPathMemo { + login_path: path.to_string_lossy().into_owned(), + }; + if let Ok(content) = serde_json::to_string(&memo) { + let _ = std::fs::write(&memo_path, content); + } +} + #[cfg(test)] #[cfg(unix)] mod tests { @@ -245,17 +525,54 @@ mod tests { use std::fs; use std::os::unix::fs::PermissionsExt; + static TEST_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + struct EnvVarGuard { + key: &'static str, + old_value: Option, + } + + impl EnvVarGuard { + fn set(key: &'static str, value: &str) -> Self { + let old_value = std::env::var_os(key); + std::env::set_var(key, value); + Self { key, old_value } + } + } + + impl Drop for EnvVarGuard { + fn drop(&mut self) { + if let Some(val) = &self.old_value { + std::env::set_var(self.key, val); + } else { + std::env::remove_var(self.key); + } + } + } + + fn reset_effective_path_state() { + let mut guard = EFFECTIVE_PATH_STATE + .lock() + .unwrap_or_else(|e| e.into_inner()); + *guard = None; + } + #[test] fn impoverished_path_merges_login_first_and_keeps_current_entries() { let current = OsStr::new("/usr/bin:/bin:/home/alice/.cargo/bin:/custom/current"); let login = OsString::from("/fake/login/bin:/usr/bin:/opt/homebrew/bin"); - let effective = compute_effective_path_with( - current, - Some(OsStr::new("/home/alice")), - |_| true, - || Some(login.clone()), - ); + let mut dir_exists = |_dir: &Path| true; + let probe_login_path = || Some(login.clone()); + + if !path_is_impoverished(current, Some(OsStr::new("/home/alice")), &mut dir_exists) { + panic!("should be impoverished"); + } + + let login_path = probe_login_path() + .filter(|path| login_path_is_acceptable(path)) + .unwrap(); + let effective = merge_login_and_current_path(&login_path, current); assert_eq!( effective, @@ -267,22 +584,16 @@ mod tests { #[test] fn invalid_probe_paths_are_rejected() { - let current = OsStr::new("/usr/bin:/bin"); let rejected = vec![ OsString::new(), OsString::from("/fake/login/bin\n/usr/bin"), OsString::from("/fake/login/bin:relative/bin"), OsString::from_vec(b"/fake/login/bin\0/usr/bin".to_vec()), + OsString::from("/usr/bin /bin /opt/homebrew/bin"), // fish-shaped space-joined ]; for probe_path in rejected { - let effective = compute_effective_path_with( - current, - Some(OsStr::new("/home/alice")), - |_| true, - || Some(probe_path.clone()), - ); - assert_eq!(effective, current); + assert!(!login_path_is_acceptable(&probe_path)); } } @@ -292,14 +603,11 @@ mod tests { "/opt/homebrew/bin:/usr/local/bin:/home/alice/.cargo/bin:/home/alice/.local/bin:/usr/bin:/bin", ); - let effective = compute_effective_path_with( + assert!(!path_is_impoverished( current, Some(OsStr::new("/home/alice")), - |_| true, - || panic!("rich PATH must not invoke the login-shell probe"), - ); - - assert_eq!(effective, current); + |_| true + )); } #[test] @@ -326,4 +634,115 @@ mod tests { "login-shell PATH probe exceeded the 5s test budget" ); } + + #[test] + fn test_login_shell_candidates_includes_fallbacks() { + let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let _shell_guard = EnvVarGuard::set("SHELL", "/opt/zerobrew/bin/fish"); + let candidates = login_shell_candidates(); + assert_eq!(candidates[0], PathBuf::from("/opt/zerobrew/bin/fish")); + assert!(candidates.contains(&PathBuf::from("/bin/zsh"))); + assert!(candidates.contains(&PathBuf::from("/bin/bash"))); + } + + #[test] + fn test_memo_written_and_read() { + let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let temp = tempfile::tempdir().unwrap(); + let _cache_guard = EnvVarGuard::set("AFT_CACHE_DIR", temp.path().to_str().unwrap()); + + let test_path = OsStr::new("/opt/homebrew/bin:/usr/bin:/bin"); + write_login_path_memo(test_path); + + let read_path = read_login_path_memo().unwrap(); + assert_eq!(read_path, test_path); + } + + #[test] + fn test_bounded_re_probe() { + let _guard = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + reset_effective_path_state(); + + let temp = tempfile::tempdir().unwrap(); + let _cache_guard = EnvVarGuard::set("AFT_CACHE_DIR", temp.path().to_str().unwrap()); + + // Force impoverished PATH + let _path_guard = EnvVarGuard::set("PATH", "/usr/bin:/bin"); + + // Set SHELL to a non-existent shell so probe fails + let _shell_guard = EnvVarGuard::set("SHELL", "/nonexistent/shell"); + + // Set candidates override to nonexistent shell to force probe failure + let _candidates_guard = + EnvVarGuard::set("AFT_TEST_LOGIN_SHELL_CANDIDATES", "/nonexistent/shell"); + + // First call: probe fails, no memo, falls back to the impoverished + // PATH (plus whatever standard dirs exist on the test machine, which + // the unconditional append may add — assert the prefix, not equality). + let path1 = effective_path(); + assert!(path1.to_string_lossy().starts_with("/usr/bin:/bin")); + + { + let guard = EFFECTIVE_PATH_STATE + .lock() + .unwrap_or_else(|e| e.into_inner()); + let state = guard.as_ref().unwrap(); + assert!(state.is_fallback); + assert!(state.last_probe_attempt.is_some()); + } + + // Second call immediately: should NOT retry probe (returns cached fallback) + let path2 = effective_path(); + assert_eq!(path2, path1); + + // Simulate 61 seconds passing by modifying last_probe_attempt + { + let mut guard = EFFECTIVE_PATH_STATE + .lock() + .unwrap_or_else(|e| e.into_inner()); + let state = guard.as_mut().unwrap(); + state.last_probe_attempt = Some(Instant::now() - Duration::from_secs(61)); + } + + // Now write a memo so the next probe/fallback can succeed + write_login_path_memo(OsStr::new("/opt/homebrew/bin:/usr/bin:/bin")); + + // Third call: should retry probe (which still fails because SHELL is nonexistent), + // but now it reads from the memo! + let path3 = effective_path(); + assert!(path3.to_string_lossy().contains("/opt/homebrew/bin")); + } + + #[test] + fn test_append_missing_standard_dirs() { + let home = OsStr::new("/home/alice"); + let path = OsStr::new("/usr/bin:/bin"); + + // Mock dir_exists to return true only for ~/.bun/bin + let dir_exists = |dir: &Path| dir == Path::new("/home/alice/.bun/bin"); + + let enriched = append_missing_standard_dirs(path, Some(home), dir_exists); + assert_eq!( + enriched, + OsString::from("/usr/bin:/bin:/home/alice/.bun/bin") + ); + } + + #[test] + fn test_append_missing_standard_dirs_dedup_and_order() { + let home = OsStr::new("/home/alice"); + // ~/.bun/bin is already in the path, but /opt/homebrew/bin is missing + let path = OsStr::new("/home/alice/.bun/bin:/usr/bin:/bin"); + + let dir_exists = |dir: &Path| { + dir == Path::new("/home/alice/.bun/bin") || dir == Path::new("/opt/homebrew/bin") + }; + + let enriched = append_missing_standard_dirs(path, Some(home), dir_exists); + // /opt/homebrew/bin should be appended at the end, and ~/.bun/bin should not be duplicated + assert_eq!( + enriched, + OsString::from("/home/alice/.bun/bin:/usr/bin:/bin:/opt/homebrew/bin") + ); + } } diff --git a/crates/aft/src/executor/mod.rs b/crates/aft/src/executor/mod.rs index 3227985d..35995d0c 100644 --- a/crates/aft/src/executor/mod.rs +++ b/crates/aft/src/executor/mod.rs @@ -98,7 +98,14 @@ impl ExecutorConfig { let max_actor_cap = pool_size.saturating_sub(1).max(1); let actor_cap = self.actor_cap.max(1).min(max_actor_cap); let read_cap = self.read_cap.max(1).min(actor_cap).min(4); - let heavy_permits = self.heavy_permits.clamp(2, 3); + // HeavyInit jobs share workers with RouteBind/configure. Keep one worker + // available even in a two-worker pool so a heavy-init storm cannot hold + // a fresh bind behind every executor worker. + let heavy_permits = self + .heavy_permits + .max(1) + .min(pool_size.saturating_sub(1).max(1)) + .min(3); let drr_quantum = self.drr_quantum.max(1); let deficit_cap = (actor_cap.max(1) as isize) * 4; let interactive_reserve = if pool_size >= 4 { 2 } else { 1 }; @@ -166,6 +173,23 @@ pub struct MutatingLaneSnapshot { pub started_age_ms: u64, } +/// Non-blocking scheduler explanation attached to a delayed RouteBind warning. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct BindBlockerSnapshot { + pub configure_state: &'static str, + pub blockers: Vec, +} + +#[derive(Debug, Clone)] +struct RunningJob { + root_id: ProjectRootId, + request_id: String, + command: String, + job_class: JobClass, + lane: Lane, + started_at: Instant, +} + #[derive(Debug, Clone)] struct RunningMutatingJob { request_id: String, @@ -274,6 +298,16 @@ impl Executor { state.actors.contains_key(root_id) } + /// Snapshot one actor context without retaining the scheduler lock while + /// maintenance drops root-scoped resources. + pub fn actor_context(&self, root_id: &ProjectRootId) -> Option> { + let state = self.inner.state.lock(); + state + .actors + .get(root_id) + .map(|actor| Arc::clone(&actor.ctx)) + } + /// Snapshot the registered actor contexts. /// /// The returned [`Arc`]s keep contexts alive after the scheduler lock is @@ -468,6 +502,20 @@ impl Executor { .map(|state| state.mutating_job_state_label(root_id, request_id)) } + /// Snapshot RouteBind blockers without waiting on scheduler state. The subc + /// health path uses this only for a delayed-bind breadcrumb, so contention + /// is reported as scheduler busy rather than delaying the transport loop. + pub fn try_bind_blocker_snapshot( + &self, + root_id: &ProjectRootId, + request_id: &str, + ) -> Option { + self.inner + .state + .try_lock() + .map(|state| state.bind_blocker_snapshot(root_id, request_id)) + } + pub fn nonrunnable_dispatch_count(&self) -> usize { self.inner.nonrunnable_dispatches.load(Ordering::Acquire) } @@ -525,6 +573,7 @@ struct SchedulerState { interactive_inflight: usize, maintenance_inflight: usize, config: EffectiveConfig, + running_jobs: HashMap<(ProjectRootId, String), RunningJob>, } impl SchedulerState { @@ -537,6 +586,7 @@ impl SchedulerState { interactive_inflight: 0, maintenance_inflight: 0, config, + running_jobs: HashMap::new(), } } @@ -603,6 +653,78 @@ impl SchedulerState { } "not_found" } + + fn bind_blocker_snapshot( + &self, + root_id: &ProjectRootId, + request_id: &str, + ) -> BindBlockerSnapshot { + let configure_state = self.mutating_job_state_label(root_id, request_id); + let mut blockers = Vec::new(); + + if let Some(actor) = self.actors.get(root_id) { + if configure_state == "queued" { + let configure_count = actor.pending_configure_count(); + if configure_count > 0 { + blockers.push(format!("queued_behind_configure({configure_count})")); + } + if actor.read_inflight > 0 || actor.lsp_inflight { + blockers.push("waiting_on_readers".to_string()); + } + } + } + + if configure_state == "queued" { + let maintenance: Vec<_> = self + .running_jobs + .values() + .filter(|job| job.job_class == JobClass::Maintenance) + .collect(); + if !maintenance.is_empty() { + blockers.push(format!( + "queued_behind_maintenance({})", + format_running_jobs(&maintenance) + )); + } + } + + if self.idle_workers == 0 { + let running: Vec<_> = self.running_jobs.values().collect(); + blockers.push(format!( + "idle_workers==0({})", + format_running_jobs(&running) + )); + } + + BindBlockerSnapshot { + configure_state, + blockers, + } + } +} + +fn is_configure_request(request_id: &str) -> bool { + request_id.starts_with("subc-bind-") +} + +fn format_running_jobs(jobs: &[&RunningJob]) -> String { + let now = Instant::now(); + let mut labels: Vec<_> = jobs + .iter() + .map(|job| { + format!( + "job={} command={} lane={:?} root={} age_ms={}", + job.request_id, + job.command, + job.lane, + job.root_id.as_path().display(), + duration_millis_u64(now.saturating_duration_since(job.started_at)) + ) + }) + .collect(); + labels.sort(); + labels.truncate(4); + labels.join("; ") } #[derive(Default)] @@ -714,6 +836,15 @@ impl ActorState { self.interactive.has_queued_mutating_job(request_id) || self.maintenance.has_queued_mutating_job(request_id) } + + fn pending_configure_count(&self) -> usize { + usize::from( + self.mutating_inflight + .as_ref() + .is_some_and(|job| is_configure_request(&job.request_id)), + ) + self.interactive.queued_configure_count() + + self.maintenance.queued_configure_count() + } } struct ClassQueues { @@ -775,6 +906,13 @@ impl ClassQueues { self.mutating.iter().any(|job| job.request_id == request_id) } + fn queued_configure_count(&self) -> usize { + self.mutating + .iter() + .filter(|job| is_configure_request(&job.request_id)) + .count() + } + fn queue(&self, lane: Lane) -> &VecDeque { match lane { Lane::PureRead => &self.pure_reads, @@ -878,6 +1016,7 @@ struct RunJob { struct CompletionEvent { root_id: ProjectRootId, + request_id: String, job_class: JobClass, lane: Lane, heavy_permit: Option, @@ -934,11 +1073,13 @@ fn process_scheduler_event(event: SchedulerEvent, state: &mut SchedulerState) -> fn complete_job(state: &mut SchedulerState, event: CompletionEvent) { let CompletionEvent { root_id, + request_id, job_class, lane, heavy_permit, panicked, } = event; + state.running_jobs.remove(&(root_id.clone(), request_id)); match job_class { JobClass::Interactive => { @@ -1073,6 +1214,17 @@ fn dispatch_runnable_class( }; if let Some(run_job) = run_job { + state.running_jobs.insert( + (run_job.root_id.clone(), run_job.request_id.clone()), + RunningJob { + root_id: run_job.root_id.clone(), + request_id: run_job.request_id.clone(), + command: run_job.command.clone(), + job_class: run_job.job_class, + lane: run_job.lane, + started_at: Instant::now(), + }, + ); state.idle_workers -= 1; match job_class { JobClass::Interactive => state.interactive_inflight += 1, @@ -1199,6 +1351,7 @@ fn worker_loop(run_rx: Receiver, event_tx: Sender) { } let completion = CompletionEvent { root_id: run_job.root_id, + request_id: run_job.request_id, job_class: run_job.job_class, lane: run_job.lane, heavy_permit: run_job.heavy_permit.take(), diff --git a/crates/aft/src/executor/tests.rs b/crates/aft/src/executor/tests.rs index b7f99465..4315b4eb 100644 --- a/crates/aft/src/executor/tests.rs +++ b/crates/aft/src/executor/tests.rs @@ -341,6 +341,229 @@ fn heavy_bound() { assert_eq!(dirs.len(), job_count); } +#[test] +fn heavy_init_storm_leaves_a_worker_for_a_fresh_route_bind() { + let executor = test_executor(2, 1, 1, 2); + assert_eq!( + executor.heavy_permits(), + 1, + "HeavyInit must leave a worker available for RouteBind/configure" + ); + + let mut dirs = Vec::new(); + let mut roots = Vec::new(); + for label in ["heavy-a", "heavy-b", "fresh-bind"] { + let (dir, root) = test_root(label); + executor.register_actor(root.clone(), test_ctx()); + dirs.push(dir); + roots.push(root); + } + + let (heavy_started_tx, heavy_started_rx) = crossbeam_channel::bounded(2); + let (release_heavy_tx, release_heavy_rx) = crossbeam_channel::bounded(2); + let mut heavy_jobs = Vec::new(); + for (index, root) in roots[..2].iter().cloned().enumerate() { + let started_tx = heavy_started_tx.clone(); + let release_rx = release_heavy_rx.clone(); + heavy_jobs.push(executor.submit( + root, + Lane::HeavyInit, + format!("heavy-storm-{index}"), + Box::new(move |_| { + started_tx.send(index).expect("signal injected heavy delay"); + release_rx + .recv_timeout(Duration::from_secs(2)) + .expect("release injected heavy delay"); + ok(format!("heavy-storm-{index}")) + }), + )); + } + heavy_started_rx + .recv_timeout(Duration::from_secs(1)) + .expect("one HeavyInit job starts"); + assert!( + heavy_started_rx + .recv_timeout(Duration::from_millis(75)) + .is_err(), + "the HeavyInit cap must leave one worker idle" + ); + + let (bind_started_tx, bind_started_rx) = crossbeam_channel::bounded(1); + let bind = executor.submit( + roots[2].clone(), + Lane::Mutating, + "subc-bind-fresh-root".to_string(), + Box::new(move |_| { + bind_started_tx + .send(()) + .expect("signal fresh RouteBind start"); + ok("fresh-route-bind") + }), + ); + bind_started_rx + .recv_timeout(Duration::from_millis(300)) + .expect("fresh RouteBind starts during the HeavyInit storm"); + bind.recv_timeout(Duration::from_secs(1)) + .expect("fresh RouteBind acknowledgement"); + + for _ in 0..heavy_jobs.len() { + release_heavy_tx + .send(()) + .expect("release injected heavy delay"); + } + for heavy in heavy_jobs { + heavy + .recv_timeout(Duration::from_secs(1)) + .expect("HeavyInit completion response"); + } + assert_eq!(executor.nonrunnable_dispatch_count(), 0); + assert_eq!(dirs.len(), 3); +} + +#[test] +fn bind_blocker_snapshot_attributes_queue_reader_maintenance_and_worker_pressure() { + let executor = test_executor(2, 1, 1, 2); + let (_reader_dir, reader_root) = test_root("blocker-reader"); + executor.register_actor(reader_root.clone(), test_ctx()); + let (reader_started_tx, reader_started_rx) = crossbeam_channel::bounded(1); + let (release_reader_tx, release_reader_rx) = crossbeam_channel::bounded(1); + let reader = executor.submit( + reader_root.clone(), + Lane::PureRead, + "reader".to_string(), + Box::new(move |_| { + reader_started_tx.send(()).expect("reader starts"); + release_reader_rx + .recv_timeout(Duration::from_secs(2)) + .expect("release reader"); + ok("reader") + }), + ); + reader_started_rx + .recv_timeout(Duration::from_secs(1)) + .expect("reader starts before queued configure jobs"); + let first_bind = executor.submit( + reader_root.clone(), + Lane::Mutating, + "subc-bind-first".to_string(), + Box::new(|_| ok("first-bind")), + ); + let second_bind = executor.submit( + reader_root.clone(), + Lane::Mutating, + "subc-bind-second".to_string(), + Box::new(|_| ok("second-bind")), + ); + // try_bind_blocker_snapshot is try-lock-only by contract and may lose to + // the dispatcher's own scheduler-lock windows; retry briefly rather than + // demanding the first attempt wins the race. + let deadline = std::time::Instant::now() + Duration::from_secs(2); + let reader_snapshot = loop { + if let Some(snapshot) = + executor.try_bind_blocker_snapshot(&reader_root, "subc-bind-second") + { + break snapshot; + } + assert!( + std::time::Instant::now() < deadline, + "bind blocker snapshot stayed lock-contended for 2s" + ); + std::thread::sleep(Duration::from_millis(5)); + }; + assert_eq!(reader_snapshot.configure_state, "queued"); + assert!(reader_snapshot + .blockers + .iter() + .any(|blocker| blocker == "queued_behind_configure(2)")); + assert!(reader_snapshot + .blockers + .iter() + .any(|blocker| blocker == "waiting_on_readers")); + + release_reader_tx.send(()).expect("release reader"); + reader + .recv_timeout(Duration::from_secs(1)) + .expect("reader completion"); + first_bind + .recv_timeout(Duration::from_secs(1)) + .expect("first configure completion"); + second_bind + .recv_timeout(Duration::from_secs(1)) + .expect("second configure completion"); + + let executor = test_executor(2, 1, 1, 2); + let (_maintenance_dir, maintenance_root) = test_root("blocker-maintenance"); + let (_occupied_dir, occupied_root) = test_root("blocker-occupied"); + let (_target_dir, target_root) = test_root("blocker-target"); + for root in [&maintenance_root, &occupied_root, &target_root] { + executor.register_actor(root.clone(), test_ctx()); + } + let (maintenance_started_tx, maintenance_started_rx) = crossbeam_channel::bounded(1); + let (release_maintenance_tx, release_maintenance_rx) = crossbeam_channel::bounded(1); + let maintenance = executor.submit_maintenance_async( + maintenance_root, + Lane::Mutating, + "subc-maintenance-drain-watcher".to_string(), + Box::new(move |_| { + maintenance_started_tx.send(()).expect("maintenance starts"); + release_maintenance_rx + .recv_timeout(Duration::from_secs(2)) + .expect("release maintenance"); + ok("maintenance") + }), + ); + maintenance_started_rx + .recv_timeout(Duration::from_secs(1)) + .expect("maintenance starts"); + let (occupied_started_tx, occupied_started_rx) = crossbeam_channel::bounded(1); + let (release_occupied_tx, release_occupied_rx) = crossbeam_channel::bounded(1); + let occupied = executor.submit( + occupied_root, + Lane::PureRead, + "occupied-worker".to_string(), + Box::new(move |_| { + occupied_started_tx.send(()).expect("occupied read starts"); + release_occupied_rx + .recv_timeout(Duration::from_secs(2)) + .expect("release occupied read"); + ok("occupied") + }), + ); + occupied_started_rx + .recv_timeout(Duration::from_secs(1)) + .expect("second worker starts"); + let target_bind = executor.submit( + target_root.clone(), + Lane::Mutating, + "subc-bind-target".to_string(), + Box::new(|_| ok("target-bind")), + ); + let pressure_snapshot = executor + .try_bind_blocker_snapshot(&target_root, "subc-bind-target") + .expect("nonblocking pressure snapshot"); + assert_eq!(pressure_snapshot.configure_state, "queued"); + assert!(pressure_snapshot + .blockers + .iter() + .any(|blocker| blocker.starts_with("queued_behind_maintenance("))); + assert!(pressure_snapshot + .blockers + .iter() + .any(|blocker| blocker.starts_with("idle_workers==0("))); + + release_maintenance_tx + .send(()) + .expect("release maintenance"); + release_occupied_tx.send(()).expect("release occupied read"); + assert!(recv_async(maintenance).success); + occupied + .recv_timeout(Duration::from_secs(1)) + .expect("occupied completion"); + target_bind + .recv_timeout(Duration::from_secs(1)) + .expect("target bind completion"); +} + #[test] fn single_flight() { let flight = Arc::new(SingleFlight::::new()); diff --git a/crates/aft/src/format.rs b/crates/aft/src/format.rs index f379fc21..9b4fcbc0 100644 --- a/crates/aft/src/format.rs +++ b/crates/aft/src/format.rs @@ -151,7 +151,7 @@ pub fn run_external_tool( working_dir: Option<&Path>, timeout_secs: u32, ) -> Result { - let mut cmd = Command::new(command); + let mut cmd = crate::effective_path::new_command(command); cmd.args(args).stdout(Stdio::piped()).stderr(Stdio::piped()); if let Some(dir) = working_dir { @@ -547,7 +547,7 @@ fn ruff_format_available_uncached(project_root: Option<&Path>) -> bool { Some(command) => command, None => return false, }; - let output = match Command::new(&command) + let output = match crate::effective_path::new_command(&command) .arg("--version") .stdout(Stdio::piped()) .stderr(Stdio::null()) @@ -1496,7 +1496,7 @@ pub fn run_external_tool_capture( working_dir: Option<&Path>, timeout_secs: u32, ) -> Result { - let mut cmd = Command::new(command); + let mut cmd = crate::effective_path::new_command(command); cmd.args(args).stdout(Stdio::piped()).stderr(Stdio::piped()); if let Some(dir) = working_dir { diff --git a/crates/aft/src/inspect/manager.rs b/crates/aft/src/inspect/manager.rs index 5dbc1aad..5ddaec63 100644 --- a/crates/aft/src/inspect/manager.rs +++ b/crates/aft/src/inspect/manager.rs @@ -423,6 +423,19 @@ impl InspectManager { .unwrap_or(false) } + /// Release per-project inspect caches so their SQLite readers and writer + /// leases do not remain open after a root has gone idle. Callers must check + /// [`Self::tier2_any_in_flight`] first so a running scan never loses its + /// cache while it is being used. + pub fn evict_idle_caches(&self) { + if let Ok(mut caches) = self.caches.lock() { + caches.clear(); + } + if let Ok(mut facts) = self.oxc_facts_cache.lock() { + *facts = OxcFactsCache::new(); + } + } + pub fn drain_completions(&self) -> usize { let mut drained = 0usize; while let Ok(result) = self.result_rx.try_recv() { diff --git a/crates/aft/src/lsp/client.rs b/crates/aft/src/lsp/client.rs index d4ba5502..1a429e5c 100644 --- a/crates/aft/src/lsp/client.rs +++ b/crates/aft/src/lsp/client.rs @@ -1,7 +1,9 @@ use std::collections::{HashMap, HashSet, VecDeque}; use std::io::{self, BufRead, BufReader, BufWriter}; use std::path::{Path, PathBuf}; -use std::process::{Child, Command, Stdio}; +use std::process::{Child, Stdio}; +#[cfg(windows)] +use std::process::Command; use std::str::FromStr; use std::sync::atomic::{AtomicI64, Ordering}; use std::sync::{Arc, Mutex}; @@ -157,7 +159,7 @@ impl LspClient { Command::new(binary) }; #[cfg(not(windows))] - let mut command = Command::new(binary); + let mut command = crate::effective_path::new_command(binary); command .args(args) .current_dir(&root) diff --git a/crates/aft/src/main.rs b/crates/aft/src/main.rs index 3b878d08..30b8d577 100644 --- a/crates/aft/src/main.rs +++ b/crates/aft/src/main.rs @@ -979,18 +979,18 @@ mod pending_response_tests { ) -> BgTaskSnapshot { let deadline = Instant::now() + Duration::from_secs(10); loop { - if let Some(snapshot) = ctx.bash_background().status( - task_id, - session_id, - Some(root), - Some(storage), - 1024, - ) { + if let Some(snapshot) = + ctx.bash_background() + .status(task_id, session_id, Some(root), Some(storage), 1024) + { if predicate(&snapshot) { return snapshot; } } - assert!(Instant::now() < deadline, "timed out waiting for snapshot for {task_id}"); + assert!( + Instant::now() < deadline, + "timed out waiting for snapshot for {task_id}" + ); thread::sleep(Duration::from_millis(25)); } } @@ -1107,7 +1107,8 @@ mod pending_response_tests { assert!(spawn_response.success); assert_eq!(spawn_response.data["status"], serde_json::json!("running")); let task_id = spawn_response.data["task_id"].as_str().unwrap().to_string(); - let outcome = aft::commands::bash_orchestrate::build_bash_outcome(&request, &ctx, spawn_response); + let outcome = + aft::commands::bash_orchestrate::build_bash_outcome(&request, &ctx, spawn_response); let mut pending = PendingResponses::default(); match outcome { aft::response_finalize::DispatchOutcome::Deferred(pending_response) => { @@ -1119,7 +1120,10 @@ mod pending_response_tests { } let mut writer = Vec::new(); - assert_eq!(write_ready_pending_to_writer(&ctx, &mut pending, &mut writer).unwrap(), 0); + assert_eq!( + write_ready_pending_to_writer(&ctx, &mut pending, &mut writer).unwrap(), + 0 + ); assert!(writer.is_empty()); let detach = super::dispatch( @@ -1135,7 +1139,10 @@ mod pending_response_tests { assert!(detach.success); assert_eq!(detach.data["detached"], serde_json::json!(true)); - assert_eq!(write_ready_pending_to_writer(&ctx, &mut pending, &mut writer).unwrap(), 1); + assert_eq!( + write_ready_pending_to_writer(&ctx, &mut pending, &mut writer).unwrap(), + 1 + ); let values = line_values(&writer); let response = values.last().unwrap(); let output = response["output"].as_str().unwrap(); diff --git a/crates/aft/src/readonly_artifacts.rs b/crates/aft/src/readonly_artifacts.rs index 316ec874..bb9a26dd 100644 --- a/crates/aft/src/readonly_artifacts.rs +++ b/crates/aft/src/readonly_artifacts.rs @@ -1,6 +1,5 @@ use std::collections::HashSet; use std::path::{Path, PathBuf}; -use std::process::Command; // These openers borrow cache artifacts that may be owned by a different AFT // session. They therefore only read and verify the opened snapshot; any repair, @@ -242,7 +241,7 @@ fn nearest_existing_parent(path: &Path) -> Option { } fn git_toplevel(base_dir: &Path) -> Result { - let output = Command::new("git") + let output = crate::effective_path::new_command("git") .args(["rev-parse", "--show-toplevel"]) .current_dir(base_dir) .output() @@ -270,6 +269,7 @@ mod tests { use std::collections::BTreeMap; use std::fs; + use std::process::Command; use std::time::SystemTime; use tempfile::TempDir; diff --git a/crates/aft/src/runtime_drain.rs b/crates/aft/src/runtime_drain.rs index 356be27f..3110ae83 100644 --- a/crates/aft/src/runtime_drain.rs +++ b/crates/aft/src/runtime_drain.rs @@ -1324,7 +1324,7 @@ pub fn drain_watcher_events_bounded(ctx: &AppContext, max_events: usize) -> Drai let mut watcher_status_changed = false; if root_deleted { - ctx.stop_watcher_runtime(); + ctx.stop_watcher_runtime_in_background(); let _ = ctx.add_degraded_reason("project_root_deleted".to_string()); aft::slog_warn!( "project root deleted; dropping watcher to avoid delete-storm: {:?}", @@ -1334,7 +1334,7 @@ pub fn drain_watcher_events_bounded(ctx: &AppContext, max_events: usize) -> Drai changed.clear(); rescan_required = false; } else if let Some(error) = watcher_failed { - ctx.stop_watcher_runtime(); + ctx.stop_watcher_runtime_in_background(); let _ = ctx.add_degraded_reason("watcher_unavailable".to_string()); aft::slog_warn!( "file watcher unavailable; continuing without live external-change invalidation: {}", diff --git a/crates/aft/src/search_index.rs b/crates/aft/src/search_index.rs index a71c382a..6c79a077 100644 --- a/crates/aft/src/search_index.rs +++ b/crates/aft/src/search_index.rs @@ -2,7 +2,6 @@ use std::collections::{BTreeMap, BTreeSet, BinaryHeap, HashMap, HashSet}; use std::fs::{self, File, OpenOptions}; use std::io::{BufReader, BufWriter, Cursor, Read, Seek, SeekFrom, Write}; use std::path::{Component, Path, PathBuf}; -use std::process::Command; use std::sync::{ atomic::{AtomicBool, AtomicUsize, Ordering}, Arc, Mutex, OnceLock, @@ -3599,7 +3598,7 @@ fn git_root_commit_once(project_root: &Path) -> RootCommitProbe { } fn git_root_commit_once_real(project_root: &Path) -> RootCommitProbe { - let output = match Command::new("git") + let output = match crate::effective_path::new_command("git") .arg("-C") .arg(project_root) .args(["rev-list", "--max-parents=0", "HEAD"]) @@ -4209,7 +4208,7 @@ fn remaining_bytes(reader: &mut R, total_len: usize) -> Option { } fn run_git(root: &Path, args: &[&str]) -> Option { - let output = Command::new("git") + let output = crate::effective_path::new_command("git") .arg("-C") .arg(root) .args(args) @@ -4229,7 +4228,7 @@ fn run_git(root: &Path, args: &[&str]) -> Option { fn apply_git_diff_updates(index: &mut SearchIndex, root: &Path, from: &str, to: &str) -> bool { let diff_range = format!("{}..{}", from, to); - let output = match Command::new("git") + let output = match crate::effective_path::new_command("git") .arg("-C") .arg(root) .args(["diff", "--name-status", "-M", &diff_range]) diff --git a/crates/aft/src/subc/health.rs b/crates/aft/src/subc/health.rs index 0fb946f2..f7d5567b 100644 --- a/crates/aft/src/subc/health.rs +++ b/crates/aft/src/subc/health.rs @@ -5,6 +5,7 @@ use super::{ Instant, Ordering, PendingBind, RootHealthSnapshot, RouteChannel, Value, DISPATCH_PATH_BIND_WARN_AFTER, WRITER_QUEUE_CAPACITY, }; +use crate::executor::BindBlockerSnapshot; pub(super) struct DispatchPathMetrics { pub(super) origin: Instant, @@ -127,20 +128,47 @@ pub(super) fn warn_slow_pending_binds( continue; } pending.warned_half_deadline = true; - let configure_state = executor - .try_mutating_job_state_label(&pending.bind_root_id, &pending.configure_request_id) - .unwrap_or("scheduler_busy"); + let snapshot = executor + .try_bind_blocker_snapshot(&pending.bind_root_id, &pending.configure_request_id) + .unwrap_or_else(|| BindBlockerSnapshot { + configure_state: "scheduler_busy", + blockers: vec!["scheduler_busy".to_string()], + }); crate::slog_warn!( - "subc attach: pending RouteBind route {} for root {} crossed {}ms (configure_request_id={}, configure_state={})", - route, - pending.bind_root_id.as_path().display(), - duration_millis_u64(age), - pending.configure_request_id, - configure_state + "{}", + pending_bind_breadcrumb( + *route, + &pending.bind_root_id, + age, + &pending.configure_request_id, + &snapshot, + ) ); } } +fn pending_bind_breadcrumb( + route: RouteChannel, + root_id: &crate::path_identity::ProjectRootId, + age: Duration, + configure_request_id: &str, + snapshot: &BindBlockerSnapshot, +) -> String { + let blockers = if snapshot.blockers.is_empty() { + "none".to_string() + } else { + snapshot.blockers.join(", ") + }; + format!( + "subc attach: pending RouteBind route {route} for root {} crossed {}ms (configure_request_id={}, configure_state={}, blockers=[{}])", + root_id.as_path().display(), + duration_millis_u64(age), + configure_request_id, + snapshot.configure_state, + blockers, + ) +} + fn mutating_lanes_metrics(executor: &Executor) -> Value { match executor.try_mutating_lane_snapshots() { Some(snapshots) => Value::Array( @@ -230,6 +258,34 @@ mod tests { use super::*; use serde_json::json; + #[test] + fn pending_bind_breadcrumb_names_every_blocker_class() { + let (_dir, root) = test_root("breadcrumb-blockers"); + let cases = [ + "queued_behind_configure(2)", + "queued_behind_maintenance(job=subc-maintenance-drain-watcher lane=Mutating root=/tmp/a age_ms=1)", + "waiting_on_readers", + "idle_workers==0(job=subc-bind-other lane=Mutating root=/tmp/b age_ms=2)", + ]; + + for blocker in cases { + let breadcrumb = pending_bind_breadcrumb( + 7, + &root, + Duration::from_secs(6), + "subc-bind-7", + &BindBlockerSnapshot { + configure_state: "queued", + blockers: vec![blocker.to_string()], + }, + ); + assert!( + breadcrumb.contains(blocker), + "breadcrumb omitted blocker class: {breadcrumb}" + ); + } + } + #[test] fn health_report_includes_nonblocking_dispatch_liveness_for_queued_interactive() { let executor = Executor::with_config(crate::executor::ExecutorConfig { diff --git a/crates/aft/src/subc/mod.rs b/crates/aft/src/subc/mod.rs index 791a82f0..334c4086 100644 --- a/crates/aft/src/subc/mod.rs +++ b/crates/aft/src/subc/mod.rs @@ -72,6 +72,11 @@ const CONTROL_SEND_TIMEOUT: Duration = Duration::from_millis(250); /// loop turn so busy select arms cannot starve it. const DRAIN_TICK_PERIOD: Duration = Duration::from_millis(250); +/// Root-scoped stores and watcher runtimes are reopened lazily after this +/// period without tool traffic. Keeping the value fixed avoids per-client +/// eviction policies competing inside the module loop. +const IDLE_ROOT_TTL: Duration = Duration::from_secs(30 * 60); + const WRITER_QUEUE_CAPACITY: usize = 256; /// Keep reliable Push bursts from monopolizing the current-thread subc loop; @@ -264,6 +269,7 @@ struct RootMeta { last_touched: Instant, diagnostics_on_edit: bool, active_bash_waits: usize, + idle_artifacts_evicted: bool, } #[derive(Debug)] @@ -378,11 +384,13 @@ impl RootMeta { last_touched: now, diagnostics_on_edit: false, active_bash_waits: 0, + idle_artifacts_evicted: false, } } fn touch(&mut self) { self.last_touched = Instant::now(); + self.idle_artifacts_evicted = false; } } @@ -449,6 +457,59 @@ fn due_maintenance_jobs( (jobs, deferred) } +/// Reap root-scoped resources from the existing subc maintenance loop. The +/// actor remains registered so a bound route can continue to receive requests; +/// only disposable handles are cleared and the next request reopens them. +fn reap_idle_roots( + now: Instant, + live_roots: &mut HashMap, + pending_binds: &HashMap, + executor: &Arc, +) -> usize { + let pending_bind_roots = pending_binds + .values() + .map(|pending| pending.bind_root_id.clone()) + .collect::>(); + let candidates = live_roots + .iter() + .filter_map(|(root_id, meta)| { + if meta.idle_artifacts_evicted + || now.saturating_duration_since(meta.last_touched) < IDLE_ROOT_TTL + || meta.active_bash_waits > 0 + || meta.maintenance_pending + || !meta.maintenance_queued_kinds.is_empty() + || pending_bind_roots.contains(root_id) + { + return None; + } + Some(root_id.clone()) + }) + .collect::>(); + + let mut reaped = 0; + for root_id in candidates { + let Some(ctx) = executor.actor_context(&root_id) else { + continue; + }; + if !ctx.evict_idle_artifacts() { + continue; + } + // The watcher backend owns the OS watcher and can block while joining + // on macOS. Request shutdown here, but let a dedicated reaper thread + // perform the join rather than holding up an executor maintenance lane. + ctx.stop_watcher_runtime_in_background(); + if let Some(meta) = live_roots.get_mut(&root_id) { + meta.idle_artifacts_evicted = true; + } + reaped += 1; + log::info!( + "subc attach: evicted idle root artifacts for {}", + root_id.as_path().display() + ); + } + reaped +} + #[allow(clippy::too_many_arguments)] fn submit_due_maintenance_jobs( executor: &Arc, @@ -1837,6 +1898,15 @@ where // inbound route/control messages and push completions have run, // so maintenance does not block the actor from handling the // first request that arrives after a route bind is acknowledged. + let reaped = reap_idle_roots( + Instant::now(), + &mut live_roots, + &pending_binds, + &executor, + ); + if reaped > 0 { + log::debug!("subc attach: reaped {reaped} idle root(s)"); + } submit_due_maintenance_jobs( &executor, &mut live_roots, @@ -2179,6 +2249,9 @@ async fn handle_route_bind_completion( let replay_key = push::ReplayKey::from_identity(&completion.identity); let bind_trust = completion.identity.trust; insert_route_channel(routes, root_channels, route_id, completion.identity); + let restore_watcher = live_roots + .get(&completion.bind_root_id) + .is_some_and(|meta| meta.idle_artifacts_evicted); live_roots .entry(completion.bind_root_id.clone()) .and_modify(|meta| { @@ -2191,6 +2264,11 @@ async fn handle_route_bind_completion( meta.diagnostics_on_edit = completion.diagnostics_on_edit; meta.maintenance_poisoned = false; } + if restore_watcher { + if let Some(ctx) = executor.actor_context(&completion.bind_root_id) { + crate::commands::configure::ensure_project_watcher(&ctx); + } + } let ack = serde_json::to_vec(&ModuleControlResponse::RouteBindAck {}).map_err(SubcError::Json)?; @@ -2641,9 +2719,17 @@ async fn handle_tool_call( )?; return send_reliable_writer_frame(tx, metrics, error, "route_not_bound error").await; }; + let restore_watcher = live_roots + .get(&identity.root) + .is_some_and(|meta| meta.idle_artifacts_evicted); if let Some(meta) = live_roots.get_mut(&identity.root) { meta.touch(); } + if restore_watcher { + if let Some(ctx) = executor.actor_context(&identity.root) { + crate::commands::configure::ensure_project_watcher(&ctx); + } + } let is_bg_events_subscribe = serde_json::from_slice::(&frame.body) .ok() @@ -3358,6 +3444,80 @@ mod tests { ); } + #[test] + fn idle_root_reaper_closes_artifacts_and_stops_watcher() { + let (root_dir, root) = test_root("idle-root-reaper"); + let storage = tempfile::tempdir().expect("storage tempdir"); + std::fs::write( + root_dir.path().join("main.rs"), + "fn entry() { leaf(); }\nfn leaf() {}\n", + ) + .expect("source file"); + let canonical_root = std::fs::canonicalize(root_dir.path()).expect("canonical root"); + let app = App::default_shared(); + let ctx = Arc::new(AppContext::from_app( + Arc::clone(&app), + Config { + project_root: Some(canonical_root.clone()), + storage_dir: Some(storage.path().to_path_buf()), + callgraph_store: true, + search_index: true, + ..Config::default() + }, + )); + ctx.set_canonical_cache_root(canonical_root.clone()); + assert!(ctx + .ensure_callgraph_store() + .expect("build callgraph store") + .is_some()); + + let cache_dir = + crate::search_index::resolve_cache_dir(&canonical_root, Some(storage.path())); + let mut index = crate::search_index::SearchIndex::build(&canonical_root); + let git_head = index.stored_git_head().map(str::to_owned); + index.write_to_disk(&cache_dir, git_head.as_deref()); + *ctx.search_index() + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner) = Some(index); + + let (dispatch_tx, dispatch_rx) = crate::watcher_filter::watcher_dispatch_channel(); + let _dispatch_tx = dispatch_tx; + let shutdown = Arc::new(AtomicBool::new(false)); + let thread_shutdown = Arc::clone(&shutdown); + let join = std::thread::spawn(move || { + while !thread_shutdown.load(Ordering::SeqCst) { + std::thread::yield_now(); + } + }); + ctx.install_watcher_runtime( + dispatch_rx, + crate::watcher_filter::WatcherThreadHandle::new(shutdown, join), + ); + assert_eq!(ctx.watcher_registry_count(), 1); + + let executor = Arc::new(Executor::new()); + assert!(executor.register_actor(root.clone(), Arc::clone(&ctx))); + let mut live_roots = HashMap::new(); + let mut meta = RootMeta::new(Instant::now()); + meta.last_touched = Instant::now() - IDLE_ROOT_TTL - Duration::from_secs(1); + live_roots.insert(root.clone(), meta); + + assert_eq!( + reap_idle_roots(Instant::now(), &mut live_roots, &HashMap::new(), &executor), + 1 + ); + assert!(ctx.search_index().read().unwrap().is_none()); + assert_eq!(ctx.watcher_registry_count(), 0); + assert!( + crate::search_index::SearchIndex::read_from_disk(&cache_dir, &canonical_root).is_some() + ); + assert!(ctx + .ensure_callgraph_store() + .expect("reopen callgraph store") + .is_some()); + assert!(live_roots[&root].idle_artifacts_evicted); + } + #[test] fn due_maintenance_jobs_skip_poisoned_roots() { let (_healthy_dir, healthy_root) = test_root("maintenance-healthy"); diff --git a/crates/aft/tests/callgraph_store_test.rs b/crates/aft/tests/callgraph_store_test.rs index ed02e677..314a257e 100644 --- a/crates/aft/tests/callgraph_store_test.rs +++ b/crates/aft/tests/callgraph_store_test.rs @@ -3,7 +3,8 @@ mod test_helpers; use aft::callgraph::walk_project_files; use aft::callgraph_store::{ - live_callgraph_edge_snapshot, project_dead_code_snapshot, CallGraphStore, StoredEdge, + live_callgraph_edge_snapshot, project_dead_code_snapshot, CallGraphRead, CallGraphStore, + StoredEdge, }; use aft::commands::callgraph_store_adapter; use aft::config::Config; @@ -15,7 +16,7 @@ use aft::parser::{SymbolCache, TreeSitterProvider}; use filetime::FileTime; use rusqlite::{params, Connection}; use serde_json::{json, Value}; -use std::collections::BTreeSet; +use std::collections::{BTreeSet, HashSet}; use std::fs; use std::path::{Path, PathBuf}; use std::process::Command; @@ -23,6 +24,7 @@ use std::sync::{ atomic::{AtomicBool, AtomicI64, Ordering}, Arc, RwLock, }; +use std::time::{Duration, Instant}; use tempfile::tempdir; static NEXT_MTIME: AtomicI64 = AtomicI64::new(1_800_000_000); @@ -878,10 +880,16 @@ fn root_keyed_configure_migrates_newest_superseded_legacy_generation() { ctx.set_canonical_cache_root(root.clone()); ctx.set_cache_role(false, None); - let migrated = ctx + let fallback = ctx .ensure_callgraph_store() .unwrap() - .expect("legacy generation should be migrated instead of cold-built"); + .expect("legacy generation should remain readable during migration"); + assert!(fallback.is_legacy_fallback()); + wait_for_root_keyed_callgraph(&ctx, Duration::from_secs(20)); + let migrated = match ctx.callgraph_store_for_ops() { + CallgraphStoreAccess::Ready(store) => store, + _ => panic!("background migration should install a root-keyed reader"), + }; assert!(migrated .sqlite_path() .starts_with(ctx.callgraph_store_dir())); @@ -912,6 +920,148 @@ fn root_keyed_configure_migrates_newest_superseded_legacy_generation() { ); } +#[test] +fn writer_access_serves_legacy_fallback_while_background_migration_and_refresh_converge() { + let dir = tempdir().unwrap(); + let root = fs::canonicalize(dir.path()).unwrap_or_else(|_| dir.path().to_path_buf()); + let source = root.join("main.ts"); + write_file( + &source, + "export function entry() { oldLeaf(); }\nfunction oldLeaf() {}\n", + ); + let storage = root.join("storage"); + let legacy_dir = storage.join("opencode/callgraph"); + let legacy_build_dir = root.join(".legacy-callgraph-build"); + + CallGraphStore::cold_build_with_lease( + legacy_build_dir.clone(), + root.clone(), + &project_files(&root), + ) + .unwrap(); + write_file( + &source, + "export function entry() { fallbackLeaf(); }\nfunction fallbackLeaf() {}\n", + ); + CallGraphStore::cold_build_with_lease( + legacy_build_dir.clone(), + root.clone(), + &project_files(&root), + ) + .unwrap(); + copy_dir_all(&legacy_build_dir, &legacy_dir).unwrap(); + + let ctx = root_keyed_test_context(&root, &storage, false); + let fallback = ctx + .ensure_callgraph_store() + .expect("legacy fallback open should succeed") + .expect("legacy fallback should remain queryable"); + assert!(fallback.is_legacy_fallback()); + assert!(fallback.sqlite_path().starts_with(&legacy_dir)); + assert_eq!(entry_leaf_from_store(fallback.as_ref()), "fallbackLeaf"); + assert!( + ctx.callgraph_store_rx().lock().is_some(), + "writer-capable fallback access must schedule migration on the background lane" + ); + + // Snapshot after the fallback reader is open so SQLite's read-only WAL setup + // is not mistaken for a migration or incremental-write mutation. WAL/shm + // sidecars stay excluded entirely: any read-only connection open/close + // (e.g. migration candidate readiness probes) rewrites them by design, and + // the preservation invariant is about the data files. + let legacy_before = without_wal_sidecars(directory_file_snapshot(&legacy_dir)); + write_file( + &source, + "export function entry() { migratedLeaf(); }\nfunction migratedLeaf() {}\n", + ); + aft::runtime_drain::refresh_callgraph_store_for_watcher(&ctx, &HashSet::from([source.clone()])); + wait_for_root_keyed_callgraph(&ctx, Duration::from_secs(20)); + assert!( + !fallback.is_current(), + "published root-keyed pointer must supersede an open legacy fallback" + ); + + let migrated = match ctx.callgraph_store_for_ops() { + CallgraphStoreAccess::Ready(store) => store, + _ => panic!("completed migration should install a root-keyed reader"), + }; + assert!(!migrated.is_legacy_fallback()); + assert!(migrated + .sqlite_path() + .starts_with(ctx.callgraph_store_dir())); + assert_eq!(entry_leaf_from_store(migrated.as_ref()), "migratedLeaf"); + assert_eq!( + without_wal_sidecars(directory_file_snapshot(&legacy_dir)), + legacy_before, + "migration and watcher refresh must leave every legacy SQLite data byte untouched" + ); + + let key = artifact_cache_key_for_test(&root); + let generation = + fs::read_to_string(ctx.callgraph_store_dir().join(format!("{key}.current"))).unwrap(); + let manifest: Value = serde_json::from_slice( + &fs::read( + ctx.callgraph_store_dir() + .join(format!("{}.migration.json", generation.trim())), + ) + .unwrap(), + ) + .unwrap(); + assert_eq!(manifest["method"], "generation_copy"); + assert!(manifest["source_bytes"] + .as_u64() + .is_some_and(|bytes| bytes > 0)); + assert!(manifest["migrated_bytes"] + .as_u64() + .is_some_and(|bytes| bytes > 0)); +} + +#[test] +fn readonly_worktree_legacy_fallback_never_schedules_migration_or_takes_writer_lease() { + let dir = tempdir().unwrap(); + let root = fs::canonicalize(dir.path()).unwrap_or_else(|_| dir.path().to_path_buf()); + write_file( + &root.join("main.ts"), + "export function entry() { leaf(); }\nfunction leaf() {}\n", + ); + let storage = root.join("storage"); + let legacy_dir = storage.join("opencode/callgraph"); + let legacy_build_dir = root.join(".legacy-callgraph-build"); + CallGraphStore::cold_build_with_lease( + legacy_build_dir.clone(), + root.clone(), + &project_files(&root), + ) + .unwrap(); + copy_dir_all(&legacy_build_dir, &legacy_dir).unwrap(); + + let ctx = root_keyed_test_context(&root, &storage, true); + for _ in 0..2 { + let fallback = ctx + .ensure_callgraph_store() + .expect("read-only fallback open should succeed") + .expect("read-only worktree should serve the legacy fallback"); + assert!(fallback.is_legacy_fallback()); + assert_eq!(entry_leaf_from_store(fallback.as_ref()), "leaf"); + } + + std::thread::sleep(Duration::from_millis(50)); + assert!(ctx.callgraph_store_rx().lock().is_none()); + assert!(!ctx.callgraph_store_dir().join("writer.lease").exists()); + assert!(!ctx + .callgraph_store_dir() + .read_dir() + .unwrap() + .any(|entry| entry.ok().is_some_and(|entry| entry + .file_name() + .to_string_lossy() + .ends_with(".migration.json")))); + assert!(!ctx + .callgraph_store_dir() + .join(format!("{}.current", artifact_cache_key_for_test(&root))) + .exists()); +} + #[test] fn root_keyed_migration_disk_floor_skips_to_legacy_fallback_without_cold_build() { let dir = tempdir().unwrap(); @@ -951,10 +1101,12 @@ fn root_keyed_migration_disk_floor_skips_to_legacy_fallback_without_cold_build() ctx.set_canonical_cache_root(root.clone()); ctx.set_cache_role(false, None); - let store = ctx - .ensure_callgraph_store() - .unwrap() - .expect("fallback should serve while migration is skipped"); + let (store, _) = CallGraphStore::ensure_built_with_lease( + ctx.callgraph_store_dir(), + root.clone(), + &project_files(&root), + ) + .unwrap(); aft::callgraph_store::set_legacy_migration_available_disk_for_test(None); aft::callgraph_store::set_cold_build_swap_observer(None); @@ -1017,7 +1169,12 @@ fn root_keyed_migration_backup_budget_failure_serves_legacy_without_cold_build() ctx.set_harness(Harness::Opencode); ctx.set_canonical_cache_root(root.clone()); ctx.set_cache_role(false, None); - let fallback = ctx.ensure_callgraph_store().unwrap().unwrap(); + let (fallback, _) = CallGraphStore::ensure_built_with_lease( + ctx.callgraph_store_dir(), + root.clone(), + &project_files(&root), + ) + .unwrap(); aft::callgraph_store::set_legacy_migration_backup_budget_exhausted_for_test(false); aft::callgraph_store::set_cold_build_swap_observer(None); @@ -1070,24 +1227,20 @@ fn root_keyed_migration_redoes_partial_copy_without_valid_manifest() { copy_dir_all(&legacy_build_dir, &legacy_dir).unwrap(); aft::callgraph_store::set_legacy_migration_fail_after_temp_copy_for_test(true); - let failing_ctx = AppContext::new( - Box::new(TreeSitterProvider::new()), - Config { - project_root: Some(root.clone()), - storage_dir: Some(storage.clone()), - callgraph_store: true, - ..Config::default() - }, - ); - failing_ctx.set_harness(Harness::Opencode); - failing_ctx.set_canonical_cache_root(root.clone()); - failing_ctx.set_cache_role(false, None); - let fallback = failing_ctx.ensure_callgraph_store().unwrap().unwrap(); + let callgraph_dir = storage + .join("callgraph") + .join(artifact_cache_key_for_test(&root)); + let (fallback, _) = CallGraphStore::ensure_built_with_lease( + callgraph_dir.clone(), + root.clone(), + &project_files(&root), + ) + .unwrap(); + assert!(fallback.is_legacy_fallback()); assert!(fallback .sqlite_path() .starts_with(storage.join("opencode/callgraph"))); - assert!(!failing_ctx - .callgraph_store_dir() + assert!(!callgraph_dir .join(format!("{}.current", artifact_cache_key_for_test(&root))) .exists()); @@ -1104,7 +1257,13 @@ fn root_keyed_migration_redoes_partial_copy_without_valid_manifest() { retry_ctx.set_harness(Harness::Opencode); retry_ctx.set_canonical_cache_root(root.clone()); retry_ctx.set_cache_role(false, None); - let migrated = retry_ctx.ensure_callgraph_store().unwrap().unwrap(); + let fallback = retry_ctx.ensure_callgraph_store().unwrap().unwrap(); + assert!(fallback.is_legacy_fallback()); + wait_for_root_keyed_callgraph(&retry_ctx, Duration::from_secs(20)); + let migrated = match retry_ctx.callgraph_store_for_ops() { + CallgraphStoreAccess::Ready(store) => store, + _ => panic!("retry should complete the root-keyed migration"), + }; assert!(migrated .sqlite_path() .starts_with(retry_ctx.callgraph_store_dir())); @@ -1182,7 +1341,13 @@ fn root_keyed_migration_uses_sqlite_backup_for_only_current_legacy_generation() ctx.set_harness(Harness::Opencode); ctx.set_canonical_cache_root(root.clone()); ctx.set_cache_role(false, None); - let migrated = ctx.ensure_callgraph_store().unwrap().unwrap(); + let fallback = ctx.ensure_callgraph_store().unwrap().unwrap(); + assert!(fallback.is_legacy_fallback()); + wait_for_root_keyed_callgraph(&ctx, Duration::from_secs(20)); + let migrated = match ctx.callgraph_store_for_ops() { + CallgraphStoreAccess::Ready(store) => store, + _ => panic!("backup migration should install a root-keyed reader"), + }; stop.store(true, Ordering::SeqCst); writer.join().unwrap(); @@ -1824,6 +1989,92 @@ fn cold_edges(root: &Path) -> BTreeSet { store.edge_snapshot().unwrap() } +fn root_keyed_test_context(root: &Path, storage: &Path, worktree: bool) -> AppContext { + let ctx = AppContext::new( + Box::new(TreeSitterProvider::new()), + Config { + project_root: Some(root.to_path_buf()), + storage_dir: Some(storage.to_path_buf()), + callgraph_store: true, + ..Config::default() + }, + ); + ctx.set_harness(Harness::Opencode); + ctx.set_canonical_cache_root(root.to_path_buf()); + ctx.set_cache_role(worktree, None); + ctx +} + +fn entry_leaf_from_store(store: &impl CallGraphRead) -> String { + store + .call_tree(Path::new("main.ts"), "entry", 1) + .unwrap() + .children[0] + .name + .clone() +} + +fn wait_for_callgraph_background(ctx: &AppContext, budget: Duration) { + let deadline = Instant::now() + budget; + loop { + aft::runtime_drain::drain_callgraph_store_events(ctx); + if ctx.callgraph_store_rx().lock().is_none() { + return; + } + assert!( + Instant::now() < deadline, + "callgraph background work did not finish in time" + ); + std::thread::sleep(Duration::from_millis(10)); + } +} + +fn wait_for_root_keyed_callgraph(ctx: &AppContext, budget: Duration) { + wait_for_callgraph_background(ctx, budget); + assert!( + ctx.callgraph_store() + .read() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .as_ref() + .is_some_and(|store| !store.is_legacy_fallback()), + "background legacy migration did not publish and install a root-keyed store" + ); +} + +/// Drop `-wal`/`-shm` sidecars from a snapshot: SQLite rewrites them on any +/// connection open/close, including strictly read-only ones, so byte-equality +/// assertions about source preservation must compare only the data files. +fn without_wal_sidecars(files: Vec<(PathBuf, Vec)>) -> Vec<(PathBuf, Vec)> { + files + .into_iter() + .filter(|(path, _)| { + let name = path.to_string_lossy(); + !name.ends_with("-wal") && !name.ends_with("-shm") + }) + .collect() +} + +fn directory_file_snapshot(dir: &Path) -> Vec<(PathBuf, Vec)> { + fn collect(base: &Path, dir: &Path, files: &mut Vec<(PathBuf, Vec)>) { + for entry in fs::read_dir(dir).unwrap() { + let entry = entry.unwrap(); + if entry.file_type().unwrap().is_dir() { + collect(base, &entry.path(), files); + } else if entry.file_type().unwrap().is_file() { + files.push(( + entry.path().strip_prefix(base).unwrap().to_path_buf(), + fs::read(entry.path()).unwrap(), + )); + } + } + } + + let mut files = Vec::new(); + collect(dir, dir, &mut files); + files.sort_by(|left, right| left.0.cmp(&right.0)); + files +} + fn project_files(root: &Path) -> Vec { walk_project_files(root).collect() } diff --git a/crates/aft/tests/helpers/mod.rs b/crates/aft/tests/helpers/mod.rs index fa4a3430..1f7a17bb 100644 --- a/crates/aft/tests/helpers/mod.rs +++ b/crates/aft/tests/helpers/mod.rs @@ -76,6 +76,10 @@ pub struct AftProcess { } impl AftProcess { + pub fn cache_dir(&self) -> &Path { + self._cache_dir.path() + } + /// Spawn the aft binary with piped stdin/stdout/stderr. /// Sets AFT_CACHE_DIR to a temp path so tests don't pollute the user's cache. pub fn spawn() -> Self { @@ -150,6 +154,12 @@ impl AftProcess { // opt back in via spawn_with_real_watcher (which overrides this to // "0"). Explicit `envs` below can override it. .env("AFT_TEST_DISABLE_FILE_WATCHER", "1") + // Keep the child's PATH exactly what the test constructed. The + // production login-shell probe + standard-dir enrichment would + // re-add real tool dirs (e.g. /usr/local/bin on CI runners), + // breaking tests that simulate missing formatters/checkers by + // building an impoverished PATH. + .env("AFT_TEST_RAW_PATH", "1") .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(if pipe_stderr || diag_enabled { diff --git a/crates/aft/tests/integration/bash_test.rs b/crates/aft/tests/integration/bash_test.rs index ebfd6b10..345ec8c4 100644 --- a/crates/aft/tests/integration/bash_test.rs +++ b/crates/aft/tests/integration/bash_test.rs @@ -69,6 +69,10 @@ exit 64 let login_path = format!("{}:/usr/bin:/bin", fake_bin.display()); let mut aft = AftProcess::spawn_with_env(&[ + // The harness defaults AFT_TEST_RAW_PATH=1 (PATH isolation for + // formatter/checker tests); this test IS the PATH feature, so opt back + // in to the real probe+enrichment pipeline. + ("AFT_TEST_RAW_PATH", std::ffi::OsStr::new("0")), ("PATH", std::ffi::OsStr::new("/usr/bin:/bin")), ("HOME", home.as_os_str()), ("SHELL", fake_shell.as_os_str()), diff --git a/crates/aft/tests/integration/root_keyed_adversarial_test.rs b/crates/aft/tests/integration/root_keyed_adversarial_test.rs index 3647b5b0..45812a5d 100644 --- a/crates/aft/tests/integration/root_keyed_adversarial_test.rs +++ b/crates/aft/tests/integration/root_keyed_adversarial_test.rs @@ -194,13 +194,27 @@ fn root_keyed_migration_mid_crash_cleans_partial_and_preserves_legacy_source() { let legacy_parts_before = sqlite_file_set_parts(&legacy_source); let legacy_hash_before = sqlite_file_set_hash(&legacy_source); + // Migration now runs on a background maintenance lane in production, so + // the crash/retry mechanics are driven synchronously here: the failure + // seam is thread-local and must fire on the thread running the migration. + let ctx_for_dirs = root_keyed_context(&root, &storage); + let root_keyed_dir = ctx_for_dirs.callgraph_store_dir(); + aft::callgraph_store::set_legacy_migration_fail_after_temp_copy_for_test(true); let _fail_reset = MigrationFailReset; - let failing_ctx = root_keyed_context(&root, &storage); - let fallback = failing_ctx - .ensure_callgraph_store() + // A failing migration is swallowed into the legacy-fallback path by + // design (queries must keep working); the direct entry point reports it + // as Ok(None) because the fallback duplicate is filtered for callers that + // already hold one. + let failed = CallGraphStore::migrate_legacy_with_lease(root_keyed_dir.clone(), root.clone()); + assert!( + matches!(failed, Ok(None)), + "fail-after-temp-copy seam should abort the migration (got a published store instead): {failed:?}" + ); + // While migration is failing, readers still get the legacy fallback. + let fallback = CallGraphStore::open_readonly(root_keyed_dir.clone(), root.clone()) .unwrap() - .expect("failed migration should fall back to legacy data"); + .expect("failed migration should leave legacy data readable via fallback"); assert!(fallback.sqlite_path().starts_with(&legacy_dir)); drop(fallback); assert_eq!( @@ -210,7 +224,6 @@ fn root_keyed_migration_mid_crash_cleans_partial_and_preserves_legacy_source() { sqlite_file_set_parts(&legacy_source) ); - let root_keyed_dir = failing_ctx.callgraph_store_dir(); let partials_after_failure = incomplete_migration_artifacts(&root_keyed_dir, &root); assert!( !partials_after_failure.is_empty(), @@ -218,14 +231,10 @@ fn root_keyed_migration_mid_crash_cleans_partial_and_preserves_legacy_source() { ); aft::callgraph_store::set_legacy_migration_fail_after_temp_copy_for_test(false); - let retry_ctx = root_keyed_context(&root, &storage); - let migrated = retry_ctx - .ensure_callgraph_store() + let migrated = CallGraphStore::migrate_legacy_with_lease(root_keyed_dir.clone(), root.clone()) .unwrap() .expect("retry should clean the partial copy and migrate cleanly"); - assert!(migrated - .sqlite_path() - .starts_with(retry_ctx.callgraph_store_dir())); + assert!(migrated.sqlite_path().starts_with(&root_keyed_dir)); assert_eq!(entry_leaf(&migrated), "oldLeaf"); drop(migrated); @@ -236,12 +245,11 @@ fn root_keyed_migration_mid_crash_cleans_partial_and_preserves_legacy_source() { sqlite_file_set_parts(&legacy_source) ); assert!( - incomplete_migration_artifacts(&retry_ctx.callgraph_store_dir(), &root).is_empty(), + incomplete_migration_artifacts(&root_keyed_dir, &root).is_empty(), "cleanup_incomplete_migrations should remove stale temp/unmanifested files before retry" ); - let generation = current_generation(&retry_ctx.callgraph_store_dir(), &root); - assert!(retry_ctx - .callgraph_store_dir() + let generation = current_generation(&root_keyed_dir, &root); + assert!(root_keyed_dir .join(format!("{generation}.migration.json")) .is_file()); } @@ -300,11 +308,29 @@ fn root_keyed_old_binary_legacy_writer_does_not_corrupt_or_delete_legacy_data() copy_dir_all(&legacy_build_dir, &legacy_dir).unwrap(); let ctx = root_keyed_context(&root, &storage); + // First access serves the legacy fallback and schedules the migration on + // the background lane; poll until the migrated root-keyed store installs. + let first = ctx + .ensure_callgraph_store() + .unwrap() + .expect("legacy fallback should be readable while migration runs"); + assert!(first.is_legacy_fallback()); + drop(first); + wait_until(Duration::from_secs(20), || { + // The background result installs via the main loop's drain in + // production; drive it here like the runtime would. + aft::runtime_drain::drain_callgraph_store_events(&ctx); + matches!( + ctx.ensure_callgraph_store(), + Ok(Some(store)) if !store.is_legacy_fallback() + ) + }) + .expect("background legacy migration should publish a root-keyed generation"); let migrated = ctx .ensure_callgraph_store() .unwrap() .expect("root-keyed side should migrate from legacy"); - assert_eq!(entry_leaf(&migrated), "migratedLeaf"); + assert_eq!(entry_leaf(migrated.as_ref()), "migratedLeaf"); let root_keyed_generation = current_generation(&ctx.callgraph_store_dir(), &root); let root_keyed_sqlite = ctx.callgraph_store_dir().join(&root_keyed_generation); drop(migrated); diff --git a/crates/aft/tests/integration/subc_bridge_test.rs b/crates/aft/tests/integration/subc_bridge_test.rs index b9232f86..f5456835 100644 --- a/crates/aft/tests/integration/subc_bridge_test.rs +++ b/crates/aft/tests/integration/subc_bridge_test.rs @@ -1123,6 +1123,16 @@ pub(super) fn bridge_dispatch(req: RawRequest, ctx: &AppContext) -> Response { } } +fn bridge_executor_config() -> ExecutorConfig { + ExecutorConfig { + pool_size: 4, + read_cap: 3, + actor_cap: 3, + heavy_permits: 2, + drr_quantum: 1, + } +} + fn run_subc_bridge_test(name: &'static str, watchdog: Duration, driver: F, after: A) where F: FnOnce(FakeDaemonInput) -> Fut + Send + 'static, @@ -1160,6 +1170,7 @@ fn run_subc_bridge_test_with_env( after, true, bridge_dispatch, + bridge_executor_config(), ); } @@ -1181,6 +1192,7 @@ fn run_subc_bridge_production_test( after, false, bridge_dispatch, + bridge_executor_config(), ); } @@ -1195,7 +1207,40 @@ pub(super) fn run_subc_bridge_test_with_dispatch( Fut: Future + 'static, A: FnOnce(&Arc, &Arc, &SubcBridgeTestRoots), { - run_subc_bridge_test_inner(name, watchdog, Vec::new, driver, after, true, dispatch); + run_subc_bridge_test_inner( + name, + watchdog, + Vec::new, + driver, + after, + true, + dispatch, + bridge_executor_config(), + ); +} + +pub(super) fn run_subc_bridge_test_with_dispatch_and_executor_config( + name: &'static str, + watchdog: Duration, + driver: F, + after: A, + dispatch: aft::subc::DispatchFn, + executor_config: ExecutorConfig, +) where + F: FnOnce(FakeDaemonInput) -> Fut + Send + 'static, + Fut: Future + 'static, + A: FnOnce(&Arc, &Arc, &SubcBridgeTestRoots), +{ + run_subc_bridge_test_inner( + name, + watchdog, + Vec::new, + driver, + after, + true, + dispatch, + executor_config, + ); } fn run_subc_bridge_test_inner( @@ -1206,6 +1251,7 @@ fn run_subc_bridge_test_inner( after: A, allow_native_passthrough: bool, dispatch: aft::subc::DispatchFn, + executor_config: ExecutorConfig, ) where E: FnOnce() -> Vec, F: FnOnce(FakeDaemonInput) -> Fut + Send + 'static, @@ -1229,13 +1275,7 @@ fn run_subc_bridge_test_inner( ..Config::default() }, )); - let executor = Arc::new(Executor::with_config(ExecutorConfig { - pool_size: 4, - read_cap: 3, - actor_cap: 3, - heavy_permits: 2, - drr_quantum: 1, - })); + let executor = Arc::new(Executor::with_config(executor_config)); let executor_for_daemon = Arc::clone(&executor); let executor_for_check = Arc::clone(&executor); diff --git a/crates/aft/tests/integration/subc_storm_test.rs b/crates/aft/tests/integration/subc_storm_test.rs index 27e1d98f..427d0db1 100644 --- a/crates/aft/tests/integration/subc_storm_test.rs +++ b/crates/aft/tests/integration/subc_storm_test.rs @@ -11,6 +11,8 @@ use std::thread; use std::time::{Duration, Instant}; use aft::context::AppContext; +use aft::executor::{ExecutorConfig, Lane}; +use aft::path_identity::ProjectRootId; use aft::protocol::{RawRequest, Response}; use serde_json::{json, Value}; use subc_protocol::session::{HealthReport, ModuleControlRequest, ModuleControlResponse}; @@ -250,6 +252,24 @@ fn subc_storm_rebinds_stay_live_under_build_and_tool_traffic() { ); } +#[test] +fn subc_storm_heavy_init_saturation_does_not_delay_fresh_bind() { + subc_bridge_test::run_subc_bridge_test_with_dispatch_and_executor_config( + "subc_storm_heavy_init_saturation_does_not_delay_fresh_bind", + Duration::from_secs(30), + drive_heavy_init_saturation_daemon, + |_, _, _| {}, + storm_dispatch, + ExecutorConfig { + pool_size: 2, + read_cap: 1, + actor_cap: 1, + heavy_permits: 2, + drr_quantum: 1, + }, + ); +} + #[test] fn subc_storm_completion_channel_saturation_does_not_delay_binds() { subc_bridge_test::run_subc_bridge_test_with_dispatch( @@ -470,6 +490,83 @@ async fn drive_storm_daemon(input: FakeDaemonInput, scale: StormScale) { send_goodbye_and_wait(&tx).await; } +async fn drive_heavy_init_saturation_daemon(input: FakeDaemonInput) { + let session = subc_bridge_test::open_fake_daemon_session(input).await; + let executor = Arc::clone(&session.executor); + let (tx, mut rx) = start_io(session.stream); + let mut corr = 1_500_u64; + + for (channel, root) in [(1_u16, &session.root1), (2_u16, &session.root2)] { + send_bind( + &tx, + channel, + corr, + root, + &format!("heavy-init-{channel}"), + storm_project_config(false, false, false, 0), + ); + expect_ack_within(&mut rx, corr, BIND_ACK_BOUND).await; + corr += 1; + } + + assert_eq!( + executor.heavy_permits(), + 1, + "two-worker storm rigs reserve one worker for RouteBind admission" + ); + let (started_tx, started_rx) = crossbeam_channel::bounded(2); + let (release_tx, release_rx) = crossbeam_channel::bounded(2); + let mut heavy_jobs = Vec::new(); + for (index, root) in [&session.root1, &session.root2].into_iter().enumerate() { + let root_id = ProjectRootId::from_path(root).expect("bound heavy root id"); + let started_tx = started_tx.clone(); + let release_rx = release_rx.clone(); + heavy_jobs.push(executor.submit( + root_id, + Lane::HeavyInit, + format!("storm-heavy-init-{index}"), + Box::new(move |_| { + started_tx + .send(index) + .expect("signal injected HeavyInit delay"); + release_rx + .recv_timeout(Duration::from_secs(5)) + .expect("release injected HeavyInit delay"); + Response::success(format!("storm-heavy-init-{index}"), json!({ "ok": true })) + }), + )); + } + started_rx + .recv_timeout(Duration::from_secs(1)) + .expect("one HeavyInit job starts"); + assert!( + started_rx.recv_timeout(Duration::from_millis(75)).is_err(), + "HeavyInit storm must leave a worker free for a bind" + ); + + let fresh_root = tempfile::tempdir().expect("fresh bind root"); + corr += 1; + send_bind( + &tx, + 3, + corr, + fresh_root.path(), + "heavy-init-fresh-bind", + storm_project_config(false, false, false, 0), + ); + expect_ack_within(&mut rx, corr, BIND_ACK_BOUND).await; + + for _ in 0..heavy_jobs.len() { + release_tx.send(()).expect("release HeavyInit storm job"); + } + for heavy in heavy_jobs { + heavy + .recv_timeout(Duration::from_secs(2)) + .expect("HeavyInit storm completion"); + } + send_goodbye_and_wait(&tx).await; +} + async fn drive_completion_saturation_daemon(input: FakeDaemonInput) { let session = subc_bridge_test::open_fake_daemon_session(input).await; let (tx, mut rx) = start_io(session.stream); diff --git a/crates/aft/tests/integration/tool_call_parity_test.rs b/crates/aft/tests/integration/tool_call_parity_test.rs index 4952fd50..da605e49 100644 --- a/crates/aft/tests/integration/tool_call_parity_test.rs +++ b/crates/aft/tests/integration/tool_call_parity_test.rs @@ -81,8 +81,16 @@ fn tool_call_matches_direct_spine_envelopes() { .as_str() .unwrap_or_else(|| panic!("tool_call response missing text for {}", case.label)); assert_eq!( - normalize_text(&expected_text, direct_project.path()), - normalize_text(actual_text, tool_call_project.path()), + normalize_text( + &expected_text, + direct_project.path(), + direct_aft.cache_dir() + ), + normalize_text( + actual_text, + tool_call_project.path(), + tool_call_aft.cache_dir() + ), "formatted text mismatch for {}", case.label ); @@ -93,8 +101,16 @@ fn tool_call_matches_direct_spine_envelopes() { .expect("direct response is object") .insert("text".to_string(), Value::String(expected_text)); - let expected_envelope = normalized_envelope(expected_envelope, direct_project.path()); - let actual_envelope = normalized_envelope(tool_call_response, tool_call_project.path()); + let expected_envelope = normalized_envelope( + expected_envelope, + direct_project.path(), + direct_aft.cache_dir(), + ); + let actual_envelope = normalized_envelope( + tool_call_response, + tool_call_project.path(), + tool_call_aft.cache_dir(), + ); assert_eq!( expected_envelope, actual_envelope, "full envelope mismatch for {}", @@ -416,17 +432,17 @@ fn response_from_wire(value: &Value) -> Response { } } -fn normalized_envelope(mut value: Value, project_root: &Path) -> Value { - normalize_value(&mut value, project_root); +fn normalized_envelope(mut value: Value, project_root: &Path, cache_dir: &Path) -> Value { + normalize_value(&mut value, project_root, cache_dir); value } -fn normalize_value(value: &mut Value, project_root: &Path) { +fn normalize_value(value: &mut Value, project_root: &Path, cache_dir: &Path) { match value { - Value::String(text) => *text = normalize_text(text, project_root), + Value::String(text) => *text = normalize_text(text, project_root, cache_dir), Value::Array(items) => { for item in items { - normalize_value(item, project_root); + normalize_value(item, project_root, cache_dir); } } Value::Object(map) => { @@ -455,20 +471,26 @@ fn normalize_value(value: &mut Value, project_root: &Path) { } } for value in map.values_mut() { - normalize_value(value, project_root); + normalize_value(value, project_root, cache_dir); } } Value::Null | Value::Bool(_) | Value::Number(_) => {} } } -fn normalize_text(text: &str, project_root: &Path) -> String { +fn normalize_text(text: &str, project_root: &Path, cache_dir: &Path) -> String { // Base root forms: the raw path plus its canonicalized form (macOS /var -> // /private/var, Windows verbatim prefixes, etc.). - let mut base_roots = vec![project_root.to_string_lossy().to_string()]; + let mut base_roots = vec![ + project_root.to_string_lossy().to_string(), + cache_dir.to_string_lossy().to_string(), + ]; if let Ok(canonical) = fs::canonicalize(project_root) { base_roots.push(canonical.to_string_lossy().to_string()); } + if let Ok(canonical) = fs::canonicalize(cache_dir) { + base_roots.push(canonical.to_string_lossy().to_string()); + } // The `status` tool is the only spine case rendered via serde_json // `to_string_pretty`, which JSON-ESCAPES backslashes — so a Windows root diff --git a/packages/aft-bridge/src/__tests__/pool-configure-override.test.ts b/packages/aft-bridge/src/__tests__/pool-configure-override.test.ts index 31441a51..05381312 100644 --- a/packages/aft-bridge/src/__tests__/pool-configure-override.test.ts +++ b/packages/aft-bridge/src/__tests__/pool-configure-override.test.ts @@ -23,6 +23,30 @@ import { describe, expect, test } from "bun:test"; import { BridgePool } from "../pool.js"; describe("BridgePool.setConfigureOverride", () => { + test("reconfigure pushes changed paths to an existing bridge", async () => { + const pool = new BridgePool("/fake/aft", { idleTimeoutMs: Infinity }); + const bridge = pool.getBridge("/project/a"); + const requests: Array<{ command: string; params: Record }> = []; + bridge.isAlive = () => true; + bridge.send = async (command, params = {}) => { + requests.push({ command, params }); + return { success: true }; + }; + + await pool.reconfigure("/project/a", { lsp_paths_extra: ["/cache/bin"] }); + + expect(requests).toEqual([ + { + command: "configure", + params: { + project_root: "/project/a", + lsp_paths_extra: ["/cache/bin"], + }, + }, + ]); + expect(pool._testGetConfigOverrides().lsp_paths_extra).toEqual(["/cache/bin"]); + }); + test("setting an override updates the future-spawn map", () => { const pool = new BridgePool("/fake/aft", { idleTimeoutMs: Infinity }); diff --git a/packages/aft-bridge/src/config-keys.ts b/packages/aft-bridge/src/config-keys.ts new file mode 100644 index 00000000..2b1b698d --- /dev/null +++ b/packages/aft-bridge/src/config-keys.ts @@ -0,0 +1,17 @@ +/** + * Top-level config keys owned by one host plugin rather than the shared config + * loader. Both harnesses read the same CortexKit config file, so each schema + * strips the other harness's declared keys before strict validation. + */ +export const OPENCODE_ONLY_KEYS = ["hoist_builtin_tools", "auto_update"] as const; + +/** Pi currently has no top-level keys that OpenCode does not understand. */ +export const PI_ONLY_KEYS = [] as const; + +/** Strip only known top-level keys owned by the other harness. */ +export function stripHarnessSpecificConfigKeys(value: unknown, keys: readonly string[]): unknown { + if (!value || typeof value !== "object" || Array.isArray(value)) return value; + const stripped = { ...(value as Record) }; + for (const key of keys) delete stripped[key]; + return stripped; +} diff --git a/packages/aft-bridge/src/index.ts b/packages/aft-bridge/src/index.ts index f04a115f..f2cb908f 100644 --- a/packages/aft-bridge/src/index.ts +++ b/packages/aft-bridge/src/index.ts @@ -52,6 +52,12 @@ export { isEmptyParam, } from "./coerce.js"; export { LONG_RUNNING_COMMAND_TIMEOUT_MS, timeoutForCommand } from "./command-timeouts.js"; +// --- shared harness config keys --- +export { + OPENCODE_ONLY_KEYS, + PI_ONLY_KEYS, + stripHarnessSpecificConfigKeys, +} from "./config-keys.js"; // --- config tiers --- export type { ConfigTier } from "./config-tiers.js"; export { formatDroppedKeyWarnings, inlineUserConfigTier, readConfigTiers } from "./config-tiers.js"; diff --git a/packages/aft-bridge/src/pool.ts b/packages/aft-bridge/src/pool.ts index c56c4ce9..79ad6875 100644 --- a/packages/aft-bridge/src/pool.ts +++ b/packages/aft-bridge/src/pool.ts @@ -235,15 +235,7 @@ export class BridgePool implements AftTransportPool { // project's `.opencode/aft.jsonc` was visible at plugin init for ALL // sessions, ignoring the actual session's project config. See the // `projectConfigLoader` doc-comment on PoolOptions. - let projectOverrides: Record = {}; - if (this.projectConfigLoader) { - try { - projectOverrides = this.projectConfigLoader(key) ?? {}; - } catch (err) { - const message = err instanceof Error ? err.message : String(err); - this.error(`projectConfigLoader failed; using global overrides only: ${message}`); - } - } + const projectOverrides = this.loadProjectOverrides(key); const mergedOverrides = { ...this.configOverrides, ...projectOverrides }; const bridge = new BinaryBridge(this.binaryPath, key, this.bridgeOptions, mergedOverrides); @@ -410,6 +402,47 @@ export class BridgePool implements AftTransportPool { } } + /** + * Reapply configure overrides to a live bridge without replacing it. + * + * Async resources such as the LSP install cache can become available after a + * bridge has configured. Sending a normal configure request keeps the bridge's + * warm Rust state while allowing those process-state paths to take effect. + */ + async reconfigure(projectRoot: string, overrides: Record): Promise { + const key = normalizeKey(projectRoot); + for (const [name, value] of Object.entries(overrides)) { + this.setConfigureOverride(name, value); + } + + const bridge = this.getActiveBridgeForRoot(key); + if (!bridge) return; + + const projectOverrides = this.loadProjectOverrides(key); + const response = await bridge.send("configure", { + project_root: key, + ...this.configOverrides, + ...projectOverrides, + ...overrides, + }); + if (response.success === false) { + throw new Error( + `configure refresh failed for ${key}: ${String(response.message ?? "unknown error")}`, + ); + } + } + + private loadProjectOverrides(key: string): Record { + if (!this.projectConfigLoader) return {}; + try { + return this.projectConfigLoader(key) ?? {}; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + this.error(`projectConfigLoader failed; using global overrides only: ${message}`); + return {}; + } + } + /** Number of active bridges in the pool. */ get size(): number { return this.bridges.size; diff --git a/packages/aft-bridge/src/revivable-transport.ts b/packages/aft-bridge/src/revivable-transport.ts index 6b276327..ea76b3a0 100644 --- a/packages/aft-bridge/src/revivable-transport.ts +++ b/packages/aft-bridge/src/revivable-transport.ts @@ -70,6 +70,15 @@ export class RevivableTransportPool implements AftTransportPool { this.activePool.setConfigureOverride(key, value); } + async reconfigure(projectRoot: string, overrides: Record): Promise { + for (const [key, value] of Object.entries(overrides)) { + if (value === undefined) this.configureOverrides.delete(key); + else this.configureOverrides.set(key, value); + } + const pool = await this.ensureActivePool(); + await pool.reconfigure(projectRoot, overrides); + } + async replaceBinary(path: string): Promise { const replaced = await this.activePool.replaceBinary(path); this.onBinaryReplaced?.(replaced); diff --git a/packages/aft-bridge/src/subc-transport.ts b/packages/aft-bridge/src/subc-transport.ts index fc2f6cfc..dc1dff22 100644 --- a/packages/aft-bridge/src/subc-transport.ts +++ b/packages/aft-bridge/src/subc-transport.ts @@ -858,6 +858,9 @@ export class SubcTransportPool implements AftTransportPool { /** No-op over subc: config is read locally by AFT (wire tiers are ignored). */ setConfigureOverride(_key: string, _value: unknown): void {} + /** No-op over subc: the daemon owns the live module's configure lifecycle. */ + async reconfigure(_projectRoot: string, _overrides: Record): Promise {} + /** No-op over subc: the daemon supervises the binary, not the plugin. */ async replaceBinary(path: string): Promise { return path; diff --git a/packages/aft-bridge/src/transport.ts b/packages/aft-bridge/src/transport.ts index 59040c26..323473b1 100644 --- a/packages/aft-bridge/src/transport.ts +++ b/packages/aft-bridge/src/transport.ts @@ -60,6 +60,8 @@ export interface AftTransportPool { options?: ToolCallOptions, ): Promise; setConfigureOverride(key: string, value: unknown): void; + /** Reapply configure-time overrides to an already-live project bridge. */ + reconfigure(projectRoot: string, overrides: Record): Promise; replaceBinary(path: string): Promise; /** True when this pool instance has reached its terminal shutdown state. */ isShutdown(): boolean; diff --git a/packages/aft-cli/src/__tests__/lsp-doctor.test.ts b/packages/aft-cli/src/__tests__/lsp-doctor.test.ts index 29a5a0da..426e9fdd 100644 --- a/packages/aft-cli/src/__tests__/lsp-doctor.test.ts +++ b/packages/aft-cli/src/__tests__/lsp-doctor.test.ts @@ -6,7 +6,12 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { acquireEnv } from "../../../aft-bridge/src/__tests__/test-utils/env-guard.js"; import type { HarnessAdapter, HarnessConfigPaths } from "../adapters/types.js"; -import { printLspDoctorHelp, renderLspInspection, runLspDoctor } from "../commands/lsp.js"; +import { + printLspDoctorHelp, + renderLspInspection, + runLspDoctor, + typescriptPackageWarning, +} from "../commands/lsp.js"; import type { AftRequest, AftResponse } from "../lib/aft-bridge.js"; function makeAdapter(opts: { kind?: "opencode" | "pi" } = {}): HarnessAdapter { @@ -111,6 +116,46 @@ describe("doctor lsp", () => { expect(seenArgv[0]).toEqual(["./sample.py", "--harness", "pi"]); }); + test("warns when a spawned TypeScript server cannot resolve the project package", () => { + const projectRoot = tempRoot("aft-lsp-ts-project-"); + const response = { + success: true as const, + file: `${projectRoot}/main.ts`, + extension: "ts", + project_root: projectRoot, + matching_servers: [ + { + id: "typescript", + name: "TypeScript Language Server", + kind: "typescript", + extensions: ["ts"], + root_markers: ["package.json"], + binary_name: "typescript-language-server", + binary_path: "/cache/typescript-language-server", + binary_source: "lsp_paths_extra", + workspace_root: projectRoot, + spawn_status: "ok", + args: ["--stdio"], + }, + ], + diagnostics_count: 0, + diagnostics: [], + }; + expect(typescriptPackageWarning(response)).toBe( + "typescript package not resolvable from project — server will produce no diagnostics", + ); + + const output = renderLspInspection("./main.ts", { + ...response, + typescript_package_warning: + "typescript package not resolvable from project — server will produce no diagnostics", + }); + + expect(output).toContain( + "Warning: typescript package not resolvable from project — server will produce no diagnostics", + ); + }); + test("renders structured lsp_inspect response as human text", () => { const output = renderLspInspection("./main.py", { success: true, diff --git a/packages/aft-cli/src/commands/lsp.ts b/packages/aft-cli/src/commands/lsp.ts index cdf7c806..293ecd3e 100644 --- a/packages/aft-cli/src/commands/lsp.ts +++ b/packages/aft-cli/src/commands/lsp.ts @@ -1,4 +1,5 @@ import { existsSync, readdirSync, statSync } from "node:fs"; +import { createRequire } from "node:module"; import { dirname, join, resolve } from "node:path"; import { readConfigTiers } from "@cortexkit/aft-bridge"; @@ -32,6 +33,7 @@ export interface LspInspectResponse { matching_servers?: LspServerInspection[]; diagnostics_count?: number; diagnostics?: LspDiagnostic[]; + typescript_package_warning?: string; } export interface LspServerInspection { @@ -164,7 +166,14 @@ export async function runLspDoctor(options: LspDoctorOptions): Promise { return 1; } - console.log(renderLspInspection(file, inspect as LspInspectResponse)); + const inspection = inspect as LspInspectResponse; + const typescriptWarning = typescriptPackageWarning(inspection); + console.log( + renderLspInspection(file, { + ...inspection, + ...(typescriptWarning ? { typescript_package_warning: typescriptWarning } : {}), + }), + ); return 0; } @@ -206,6 +215,11 @@ export function renderLspInspection(inputFile: string, response: LspInspectRespo } } + if (response.typescript_package_warning) { + lines.push(""); + lines.push(`Warning: ${response.typescript_package_warning}`); + } + const diagnostics = response.diagnostics ?? []; lines.push(""); lines.push(`Diagnostics (${response.diagnostics_count ?? diagnostics.length} found):`); @@ -310,6 +324,21 @@ function formatList(values: string[]): string { return values.length === 0 ? "(none)" : values.join(", "); } +export function typescriptPackageWarning(response: LspInspectResponse): string | null { + const typescriptServerSpawned = response.matching_servers?.some( + (server) => server.id === "typescript" && server.spawn_status === "ok", + ); + const diagnosticsCount = response.diagnostics_count ?? response.diagnostics?.length ?? 0; + if (!typescriptServerSpawned || !response.project_root || diagnosticsCount > 0) return null; + + try { + createRequire(join(response.project_root, "package.json")).resolve("typescript"); + return null; + } catch { + return "typescript package not resolvable from project — server will produce no diagnostics"; + } +} + function installHint(binaryName: string): string { if (binaryName === "ty") return "Install with `uv tool install ty` or `pip install ty`."; if (binaryName === "pyright-langserver") return "Install with `npm install -g pyright`."; diff --git a/packages/opencode-plugin/src/__tests__/config.test.ts b/packages/opencode-plugin/src/__tests__/config.test.ts index 6fb4a2d1..bf528be3 100644 --- a/packages/opencode-plugin/src/__tests__/config.test.ts +++ b/packages/opencode-plugin/src/__tests__/config.test.ts @@ -771,8 +771,14 @@ describe("loadAftConfig", () => { } }); - test("strict cutover rejects manually re-added old keys", () => { - expect(AftConfigSchema.safeParse({ experimental_search_index: true }).success).toBe(false); + test("strict schema still rejects keys outside both harnesses", () => { + expect(AftConfigSchema.safeParse({ genuinely_unknown_key: true }).success).toBe(false); + }); + + test("OpenCode-only keys remain available to OpenCode", () => { + expect(AftConfigSchema.parse({ hoist_builtin_tools: false, auto_update: false })).toMatchObject( + { hoist_builtin_tools: false, auto_update: false }, + ); }); test("loads semantic config block and propagates nested fields", () => { diff --git a/packages/opencode-plugin/src/__tests__/e2e/helpers.ts b/packages/opencode-plugin/src/__tests__/e2e/helpers.ts index 88b47e26..a8161fc1 100644 --- a/packages/opencode-plugin/src/__tests__/e2e/helpers.ts +++ b/packages/opencode-plugin/src/__tests__/e2e/helpers.ts @@ -499,6 +499,7 @@ export function harnessPool(harness: E2EHarness): AftTransportPool { toolCall: (_projectRoot, runtime, name, rawArgs, options) => harness.bridge.toolCall(runtime.sessionID, name, rawArgs, options), setConfigureOverride: () => {}, + reconfigure: async () => {}, replaceBinary: async (path) => path, shutdown: async () => {}, closeSession: async () => {}, diff --git a/packages/opencode-plugin/src/__tests__/lsp-auto-install.test.ts b/packages/opencode-plugin/src/__tests__/lsp-auto-install.test.ts index 31dfbefb..f8d78da4 100644 --- a/packages/opencode-plugin/src/__tests__/lsp-auto-install.test.ts +++ b/packages/opencode-plugin/src/__tests__/lsp-auto-install.test.ts @@ -4,7 +4,12 @@ import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync import { tmpdir } from "node:os"; import { join } from "node:path"; import { acquireEnv } from "../../../aft-bridge/src/__tests__/test-utils/env-guard.js"; -import { type AutoInstallConfig, ensureInstallAnchor, runAutoInstall } from "../lsp-auto-install"; +import { + type AutoInstallConfig, + ensureInstallAnchor, + pushLspPathsAfterAutoInstall, + runAutoInstall, +} from "../lsp-auto-install"; import { lspBinaryPath, writeInstalledMeta } from "../lsp-cache"; const DAY_MS = 24 * 60 * 60 * 1000; @@ -79,6 +84,29 @@ function fakeInstalled(npmPackage: string, binary: string): string { } describe("runAutoInstall", () => { + test("pushes refreshed paths to live bridges and future spawns", async () => { + const overrides: Record = {}; + const reconfigures: Array<{ root: string; params: Record }> = []; + const pool = { + setConfigureOverride(key: string, value: unknown) { + overrides[key] = value; + }, + async reconfigure(root: string, params: Record) { + reconfigures.push({ root, params }); + }, + }; + + await pushLspPathsAfterAutoInstall(pool, tempProject, ["/cache/a", "/cache/a", "/cache/b"]); + + expect(overrides.lsp_paths_extra).toEqual(["/cache/a", "/cache/b"]); + expect(reconfigures).toEqual([ + { + root: tempProject, + params: { lsp_paths_extra: ["/cache/a", "/cache/b"] }, + }, + ]); + }); + test("returns no cached paths when nothing is installed", async () => { // Empty project — no relevant servers, nothing to install. const result = await runAutoInstall(tempProject, defaultConfig(), fakeFetch()); diff --git a/packages/opencode-plugin/src/config.ts b/packages/opencode-plugin/src/config.ts index 8655e9fd..a0cfe03e 100644 --- a/packages/opencode-plugin/src/config.ts +++ b/packages/opencode-plugin/src/config.ts @@ -3,9 +3,11 @@ import { type AftConfigFileMigrationResult, type ConfigTier, migrateAftConfigFile as migrateLegacyAftConfigFile, + PI_ONLY_KEYS, readConfigTiers, resolveCortexKitConfigPaths, resolveLegacyAftConfigSources, + stripHarnessSpecificConfigKeys, stripJsoncSymbols, } from "@cortexkit/aft-bridge"; import { parse as parseJsonc, stringify as stringifyJsonc } from "comment-json"; @@ -240,118 +242,121 @@ const BackupConfigSchema = z.object({ max_file_size: z.number().int().positive().optional(), }); -export const AftConfigSchema = z - .object({ - /** - * Optional JSON Schema URL for editor tooling. Ignored by the plugin at - * runtime — only present so VS Code/Cursor/etc. pick up the published - * schema for autocomplete + validation. `aft setup` auto-inserts this. - */ - $schema: z.string().optional(), - /** - * Master switch for AFT in this config scope. Default: true. Set false in - * user config to disable AFT everywhere, or in project config to disable it - * only for that project. Project config may set this field because turning - * AFT off is not a privilege escalation. - */ - enabled: z.boolean().optional(), - /** - * Whether to auto-format files after edits. Default: false — formatting can - * reflow the file under the agent and stale the next edit's context. Opt in - * with `true` if you want AFT to format after edits. - */ - format_on_edit: z.boolean().optional(), - /** - * Maximum seconds an external formatter is allowed to run before AFT - * kills it and reports `format_skipped_reason: "timeout"`. Bounded - * 1..=600. Default: 10. Raise for slow formatters (e.g. ruff in large - * Python projects); lower for tighter test loops. - */ - formatter_timeout_secs: z.number().int().min(1).max(600).optional(), - /** Auto-validate after edits: "syntax" (tree-sitter) or "full" (runs type checker). */ - validate_on_edit: z.enum(["syntax", "full"]).optional(), - /** Per-language formatter overrides. Keys: "typescript", "python", "rust", "go". */ - formatter: z.record(z.string(), FormatterEnum).optional(), - /** Per-language type checker overrides. Keys: "typescript", "python", "rust", "go". */ - checker: z.record(z.string(), CheckerEnum).optional(), - /** - * How missing formatter/checker/LSP warnings are shown after configure. - * - `toast`: 10s TUI toast (or HTTP show-toast when available); no session chat - * - `log`: plugin log only - * - `chat`: legacy ignored user messages in the session transcript - * - * There is no top-level `formatters` key — use `format_on_edit`, `formatter`, and - * `checker` instead. - */ - configure_warnings_delivery: ConfigureWarningsDeliveryEnum.optional(), - /** - * Replace opencode's built-in read/write/edit/apply_patch tools with AFT's - * faster Rust implementations. Adds backup tracking, auto-formatting, - * inline diagnostics, and permission checks. Default: true. - */ - hoist_builtin_tools: z.boolean().optional(), - /** - * Tool surface level. Controls which tools are registered: - * - "minimal": aft_outline, aft_zoom, aft_safety (no hoisting) - * - "recommended": minimal + hoisted read/write/edit/apply_patch - * + ast_grep_search/replace + aft_import (default) - * - "all": recommended + aft_callgraph, aft_delete, aft_move, aft_refactor - */ - tool_surface: z.enum(["minimal", "recommended", "all"]).optional(), - /** - * List of tool names to disable. Disabled tools are not registered with - * OpenCode and will be invisible to agents. Use exact tool names, e.g. - * ["aft_callgraph", "aft_refactor"]. Hoisted names ("read", "edit") and - * aft-prefixed names both work. Applied after tool_surface filtering. - */ - disabled_tools: z.array(z.string()).optional(), - /** - * Restrict file operations to within the project root directory. - * When true, write-capable commands reject paths outside project_root. - * Default: false (matches OpenCode's built-in behavior). - */ - restrict_to_project_root: z.boolean().optional(), - /** Enable indexed search for grep and glob hoisting. Default: false. */ - search_index: z.boolean().optional(), - /** Enable semantic search. Default: false. */ - semantic_search: z.boolean().optional(), - /** Enable the persisted callgraph store substrate. Default: true. */ - callgraph_store: z.boolean().optional(), - /** Number of files to parse in a single batch during callgraph store cold build. Lower values reduce peak memory during cold build. Default: 100. */ - callgraph_chunk_size: z.number().optional(), - /** Codebase health inspection config. Enabled by default; set inspect.enabled=false to hide aft_inspect. */ - inspect: InspectConfigSchema.optional(), - /** Undo backup config. User-only: project config cannot disable or shrink a user's safety net. */ - backup: BackupConfigSchema.optional(), - /** - * Bash tool family (hoist + rewrite + compress + background execution). - * Default on for `tool_surface: recommended`/`all`, off for `minimal`. - * - * Accepts three shapes: - * - `true` — all sub-features on, hoist enabled - * - `false` — hoist disabled entirely; OpenCode's native bash stays - * - `{ rewrite?, compress?, background?, ... }` — partial override; - * missing sub-keys default to `true` - * - * Replaces `experimental.bash.*` (still accepted for backward compat). - */ - bash: BashConfigSchema.optional(), - /** Experimental opt-in features. Default: all false. */ - experimental: ExperimentalConfigSchema.optional(), - /** User-defined and built-in LSP server configuration. */ - lsp: LspConfigSchema.optional(), - /** Allow URL fetch tools to request private/link-local hosts. Default: false. */ - url_fetch_allow_private: z.boolean().optional(), - /** External semantic backend configuration for embedding and retrieval. */ - semantic: SemanticConfigSchema.optional(), - /** Auto-refresh OpenCode's cached @cortexkit/aft-opencode package when a newer channel version exists. */ - auto_update: z.boolean().optional(), - /** Per-bridge transport timeout and hang-escalation (USER-only; shared pool). */ - bridge: BridgeConfigSchema.optional(), - /** Subconscious daemon transport selection (USER-only; presence ⇒ subc mode). */ - subc: SubcConfigSchema.optional(), - }) - .strict(); +export const AftConfigSchema = z.preprocess( + (value) => stripHarnessSpecificConfigKeys(value, PI_ONLY_KEYS), + z + .object({ + /** + * Optional JSON Schema URL for editor tooling. Ignored by the plugin at + * runtime — only present so VS Code/Cursor/etc. pick up the published + * schema for autocomplete + validation. `aft setup` auto-inserts this. + */ + $schema: z.string().optional(), + /** + * Master switch for AFT in this config scope. Default: true. Set false in + * user config to disable AFT everywhere, or in project config to disable it + * only for that project. Project config may set this field because turning + * AFT off is not a privilege escalation. + */ + enabled: z.boolean().optional(), + /** + * Whether to auto-format files after edits. Default: false — formatting can + * reflow the file under the agent and stale the next edit's context. Opt in + * with `true` if you want AFT to format after edits. + */ + format_on_edit: z.boolean().optional(), + /** + * Maximum seconds an external formatter is allowed to run before AFT + * kills it and reports `format_skipped_reason: "timeout"`. Bounded + * 1..=600. Default: 10. Raise for slow formatters (e.g. ruff in large + * Python projects); lower for tighter test loops. + */ + formatter_timeout_secs: z.number().int().min(1).max(600).optional(), + /** Auto-validate after edits: "syntax" (tree-sitter) or "full" (runs type checker). */ + validate_on_edit: z.enum(["syntax", "full"]).optional(), + /** Per-language formatter overrides. Keys: "typescript", "python", "rust", "go". */ + formatter: z.record(z.string(), FormatterEnum).optional(), + /** Per-language type checker overrides. Keys: "typescript", "python", "rust", "go". */ + checker: z.record(z.string(), CheckerEnum).optional(), + /** + * How missing formatter/checker/LSP warnings are shown after configure. + * - `toast`: 10s TUI toast (or HTTP show-toast when available); no session chat + * - `log`: plugin log only + * - `chat`: legacy ignored user messages in the session transcript + * + * There is no top-level `formatters` key — use `format_on_edit`, `formatter`, and + * `checker` instead. + */ + configure_warnings_delivery: ConfigureWarningsDeliveryEnum.optional(), + /** + * Replace opencode's built-in read/write/edit/apply_patch tools with AFT's + * faster Rust implementations. Adds backup tracking, auto-formatting, + * inline diagnostics, and permission checks. Default: true. + */ + hoist_builtin_tools: z.boolean().optional(), + /** + * Tool surface level. Controls which tools are registered: + * - "minimal": aft_outline, aft_zoom, aft_safety (no hoisting) + * - "recommended": minimal + hoisted read/write/edit/apply_patch + * + ast_grep_search/replace + aft_import (default) + * - "all": recommended + aft_callgraph, aft_delete, aft_move, aft_refactor + */ + tool_surface: z.enum(["minimal", "recommended", "all"]).optional(), + /** + * List of tool names to disable. Disabled tools are not registered with + * OpenCode and will be invisible to agents. Use exact tool names, e.g. + * ["aft_callgraph", "aft_refactor"]. Hoisted names ("read", "edit") and + * aft-prefixed names both work. Applied after tool_surface filtering. + */ + disabled_tools: z.array(z.string()).optional(), + /** + * Restrict file operations to within the project root directory. + * When true, write-capable commands reject paths outside project_root. + * Default: false (matches OpenCode's built-in behavior). + */ + restrict_to_project_root: z.boolean().optional(), + /** Enable indexed search for grep and glob hoisting. Default: false. */ + search_index: z.boolean().optional(), + /** Enable semantic search. Default: false. */ + semantic_search: z.boolean().optional(), + /** Enable the persisted callgraph store substrate. Default: true. */ + callgraph_store: z.boolean().optional(), + /** Number of files to parse in a single batch during callgraph store cold build. Lower values reduce peak memory during cold build. Default: 100. */ + callgraph_chunk_size: z.number().optional(), + /** Codebase health inspection config. Enabled by default; set inspect.enabled=false to hide aft_inspect. */ + inspect: InspectConfigSchema.optional(), + /** Undo backup config. User-only: project config cannot disable or shrink a user's safety net. */ + backup: BackupConfigSchema.optional(), + /** + * Bash tool family (hoist + rewrite + compress + background execution). + * Default on for `tool_surface: recommended`/`all`, off for `minimal`. + * + * Accepts three shapes: + * - `true` — all sub-features on, hoist enabled + * - `false` — hoist disabled entirely; OpenCode's native bash stays + * - `{ rewrite?, compress?, background?, ... }` — partial override; + * missing sub-keys default to `true` + * + * Replaces `experimental.bash.*` (still accepted for backward compat). + */ + bash: BashConfigSchema.optional(), + /** Experimental opt-in features. Default: all false. */ + experimental: ExperimentalConfigSchema.optional(), + /** User-defined and built-in LSP server configuration. */ + lsp: LspConfigSchema.optional(), + /** Allow URL fetch tools to request private/link-local hosts. Default: false. */ + url_fetch_allow_private: z.boolean().optional(), + /** External semantic backend configuration for embedding and retrieval. */ + semantic: SemanticConfigSchema.optional(), + /** Auto-refresh OpenCode's cached @cortexkit/aft-opencode package when a newer channel version exists. */ + auto_update: z.boolean().optional(), + /** Per-bridge transport timeout and hang-escalation (USER-only; shared pool). */ + bridge: BridgeConfigSchema.optional(), + /** Subconscious daemon transport selection (USER-only; presence ⇒ subc mode). */ + subc: SubcConfigSchema.optional(), + }) + .strict(), +); export type AftConfig = z.infer; diff --git a/packages/opencode-plugin/src/index.ts b/packages/opencode-plugin/src/index.ts index c6b5d45e..4f754b14 100644 --- a/packages/opencode-plugin/src/index.ts +++ b/packages/opencode-plugin/src/index.ts @@ -40,7 +40,11 @@ import { } from "./configure-warnings.js"; import { createAutoUpdateCheckerHook } from "./hooks/auto-update-checker/index.js"; import { bridgeLogger, error, log, warn } from "./logger.js"; -import { abortInFlightAutoInstalls, runAutoInstall } from "./lsp-auto-install.js"; +import { + abortInFlightAutoInstalls, + pushLspPathsAfterAutoInstall, + runAutoInstall, +} from "./lsp-auto-install.js"; import { abortInFlightGithubInstalls, discoverRelevantGithubServers, @@ -319,6 +323,7 @@ async function initializePluginForDirectory(input: Parameters[0]) { bash_permissions: true, storage_dir: storageDir, }); + let lspInstallCompletion: Promise | null = null; const isFastembedSemanticBackend = (aftConfig.semantic?.backend ?? "fastembed") === "fastembed"; @@ -415,7 +420,8 @@ async function initializePluginForDirectory(input: Parameters[0]) { if (lspInflightInstalls.length > 0) { configOverrides.lsp_inflight_installs = lspInflightInstalls; } - if (npmResult.installsStarted > 0 || ghResult.installsStarted > 0) { + const installsWereStarted = npmResult.installsStarted > 0 || ghResult.installsStarted > 0; + if (installsWereStarted) { log( `[lsp] auto-install: ${npmResult.installsStarted} npm + ${ghResult.installsStarted} github install(s) running in background`, ); @@ -434,8 +440,22 @@ async function initializePluginForDirectory(input: Parameters[0]) { // "not relevant to project" or "already installed" which are routine. // // Fire-and-forget; never block plugin startup. - Promise.all([npmResult.installsComplete, ghResult.installsComplete]) + const installCompletion = Promise.all([npmResult.installsComplete, ghResult.installsComplete]) .then(() => { + if (installsWereStarted) { + const updatedPaths = [ + ...new Set([...npmResult.getCachedBinDirs(), ...ghResult.getCachedBinDirs()]), + ]; + if (updatedPaths.length > 0) { + configOverrides.lsp_paths_extra = updatedPaths; + } else { + delete configOverrides.lsp_paths_extra; + } + return updatedPaths; + } + return null; + }) + .then((updatedPaths) => { const actionable = [...npmResult.skipped, ...ghResult.skipped].filter((s) => { const r = s.reason.toLowerCase(); // Routine skips — don't notify. @@ -446,21 +466,26 @@ async function initializePluginForDirectory(input: Parameters[0]) { if (r === "another install in progress") return false; return true; }); - if (actionable.length === 0) return; - - const lines = actionable.map((s) => ` • ${s.id}: ${s.reason}`).join("\n"); - const message = - `AFT skipped or failed to install ${actionable.length} LSP server(s):\n${lines}\n\n` + - "See `/aft-status` for details, or check the plugin log. " + - 'Pin a working version with `lsp.versions: { "": "" }` if grace is blocking, ' + - "or set `lsp.auto_install: false` to suppress this entirely."; - sendWarning({ client: input.client, directory: input.directory }, message).catch((err) => { - warn(`[lsp] failed to deliver install summary: ${err}`); - }); + if (actionable.length > 0) { + const lines = actionable.map((s) => ` • ${s.id}: ${s.reason}`).join("\n"); + const message = + `AFT skipped or failed to install ${actionable.length} LSP server(s):\n${lines}\n\n` + + "See `/aft-status` for details, or check the plugin log. " + + 'Pin a working version with `lsp.versions: { "": "" }` if grace is blocking, ' + + "or set `lsp.auto_install: false` to suppress this entirely."; + sendWarning({ client: input.client, directory: input.directory }, message).catch( + (err) => { + warn(`[lsp] failed to deliver install summary: ${err}`); + }, + ); + } + return updatedPaths; }) .catch((err) => { warn(`[lsp] install-summary aggregation failed: ${err}`); + return null; }); + if (installsWereStarted) lspInstallCompletion = installCompletion; } catch (err) { // Auto-install failures must never block plugin startup. warn(`[lsp] auto-install setup failed: ${err instanceof Error ? err.message : String(err)}`); @@ -616,6 +641,20 @@ async function initializePluginForDirectory(input: Parameters[0]) { }); }, }); + if (lspInstallCompletion) { + lspInstallCompletion.then((updatedPaths) => { + if (!updatedPaths) return; + void pushLspPathsAfterAutoInstall(pool, input.directory, updatedPaths) + .then(() => { + log( + `[lsp] lsp_paths_extra updated after auto-install: ${updatedPaths.length} dirs pushed to live bridges`, + ); + }) + .catch((err) => { + warn(`[lsp] live bridge lsp_paths_extra update failed: ${err}`); + }); + }); + } pool.setConfigureOverride("harness", "opencode"); const ctx: PluginContext = { pool, diff --git a/packages/opencode-plugin/src/lsp-auto-install.ts b/packages/opencode-plugin/src/lsp-auto-install.ts index f405f015..a5f577c3 100644 --- a/packages/opencode-plugin/src/lsp-auto-install.ts +++ b/packages/opencode-plugin/src/lsp-auto-install.ts @@ -41,7 +41,7 @@ import { writeFileSync, } from "node:fs"; import { join } from "node:path"; -import { npmSpawnEnv, resolveNpm } from "@cortexkit/aft-bridge"; +import { type AftTransportPool, npmSpawnEnv, resolveNpm } from "@cortexkit/aft-bridge"; import { error, log, warn } from "./logger.js"; import { isInstalled, @@ -104,6 +104,8 @@ export interface AutoInstallResult { * the array shared with the synchronous return value). */ installsComplete: Promise; + /** Re-scan the cache after background installs settle. */ + getCachedBinDirs: () => string[]; } /** @@ -596,9 +598,9 @@ function validateCachedNpmInstall(spec: NpmServerSpec): boolean { * have an installed binary AND kicks off background installs for missing * packages relevant to this project. * - * Caller passes `cachedBinDirs` to Rust as `lsp_paths_extra`. The result - * is correct on first launch even though some installs may still be - * running — those binaries appear in the cache on the NEXT session. + * Caller passes `cachedBinDirs` to Rust as `lsp_paths_extra`. When a background + * install settles, the plugin calls `getCachedBinDirs()` to refresh the list and + * reconfigures live bridges; future bridges use the pool override. */ export function runAutoInstall( projectRoot: string, @@ -675,5 +677,20 @@ export function runAutoInstall( }, skipped, installsComplete: Promise.all(installPromises).then(() => {}), + getCachedBinDirs: () => + NPM_LSP_TABLE.filter( + (spec) => isInstalled(spec.npm, spec.binary) && validateCachedNpmInstall(spec), + ).map((spec) => lspBinDir(spec.npm)), }; } + +/** Apply newly discovered cache paths to both future and live bridge config. */ +export async function pushLspPathsAfterAutoInstall( + pool: Pick, + projectRoot: string, + cachedBinDirs: readonly string[], +): Promise { + const paths = [...new Set(cachedBinDirs)]; + pool.setConfigureOverride("lsp_paths_extra", paths); + await pool.reconfigure(projectRoot, { lsp_paths_extra: paths }); +} diff --git a/packages/opencode-plugin/src/lsp-github-install.ts b/packages/opencode-plugin/src/lsp-github-install.ts index 8eaeea07..ef1edcf4 100644 --- a/packages/opencode-plugin/src/lsp-github-install.ts +++ b/packages/opencode-plugin/src/lsp-github-install.ts @@ -1136,6 +1136,8 @@ export interface GithubAutoInstallResult { * Plugin startup ignores this; tests await it. */ installsComplete: Promise; + /** Re-scan the cache after background installs settle. */ + getCachedBinDirs: () => string[]; } interface InFlightGithubInstall { @@ -1203,6 +1205,7 @@ export function runGithubAutoInstall( installingBinaries: [], skipped, installsComplete: Promise.resolve(), + getCachedBinDirs: () => [...cachedBinDirs], }; } @@ -1264,6 +1267,15 @@ export function runGithubAutoInstall( }, skipped, installsComplete: Promise.all(installPromises).then(() => {}), + getCachedBinDirs: () => { + const currentHost = detectHostPlatform(); + if (!currentHost) return []; + return GITHUB_LSP_TABLE.filter( + (spec) => + isGithubInstalled(spec, currentHost.platform) && + validateCachedGithubInstall(spec, currentHost.platform), + ).map((spec) => ghBinDir(spec)); + }, }; } diff --git a/packages/pi-plugin/src/__tests__/config.test.ts b/packages/pi-plugin/src/__tests__/config.test.ts index 796ca4f9..2e88053e 100644 --- a/packages/pi-plugin/src/__tests__/config.test.ts +++ b/packages/pi-plugin/src/__tests__/config.test.ts @@ -879,6 +879,17 @@ describe("loadAftConfig", () => { } }); + test("silently strips OpenCode-only keys but still rejects unknown keys", () => { + const otherHarness = AftConfigSchema.safeParse({ + hoist_builtin_tools: true, + auto_update: false, + }); + expect(otherHarness.success).toBe(true); + if (otherHarness.success) expect(otherHarness.data).toEqual({}); + + expect(AftConfigSchema.safeParse({ genuinely_unknown_key: true }).success).toBe(false); + }); + test("strict cutover rejects manually re-added old keys", () => { expect(AftConfigSchema.safeParse({ experimental_search_index: true }).success).toBe(false); }); diff --git a/packages/pi-plugin/src/__tests__/lsp-auto-install.test.ts b/packages/pi-plugin/src/__tests__/lsp-auto-install.test.ts index c1bc7d37..f3549e27 100644 --- a/packages/pi-plugin/src/__tests__/lsp-auto-install.test.ts +++ b/packages/pi-plugin/src/__tests__/lsp-auto-install.test.ts @@ -4,7 +4,12 @@ import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync import { tmpdir } from "node:os"; import { join } from "node:path"; import { acquireEnv } from "../../../aft-bridge/src/__tests__/test-utils/env-guard.js"; -import { type AutoInstallConfig, ensureInstallAnchor, runAutoInstall } from "../lsp-auto-install"; +import { + type AutoInstallConfig, + ensureInstallAnchor, + pushLspPathsAfterAutoInstall, + runAutoInstall, +} from "../lsp-auto-install"; import { lspBinaryPath, writeInstalledMeta } from "../lsp-cache"; const DAY_MS = 24 * 60 * 60 * 1000; @@ -79,6 +84,29 @@ function fakeInstalled(npmPackage: string, binary: string): string { } describe("runAutoInstall", () => { + test("pushes refreshed paths to live bridges and future spawns", async () => { + const overrides: Record = {}; + const reconfigures: Array<{ root: string; params: Record }> = []; + const pool = { + setConfigureOverride(key: string, value: unknown) { + overrides[key] = value; + }, + async reconfigure(root: string, params: Record) { + reconfigures.push({ root, params }); + }, + }; + + await pushLspPathsAfterAutoInstall(pool, tempProject, ["/cache/a", "/cache/a", "/cache/b"]); + + expect(overrides.lsp_paths_extra).toEqual(["/cache/a", "/cache/b"]); + expect(reconfigures).toEqual([ + { + root: tempProject, + params: { lsp_paths_extra: ["/cache/a", "/cache/b"] }, + }, + ]); + }); + test("returns no cached paths when nothing is installed", async () => { // Empty project — no relevant servers, nothing to install. const result = await runAutoInstall(tempProject, defaultConfig(), fakeFetch()); diff --git a/packages/pi-plugin/src/config.ts b/packages/pi-plugin/src/config.ts index c5acce22..6c14f751 100644 --- a/packages/pi-plugin/src/config.ts +++ b/packages/pi-plugin/src/config.ts @@ -3,9 +3,11 @@ import { type AftConfigFileMigrationResult, type ConfigTier, migrateAftConfigFile as migrateLegacyAftConfigFile, + OPENCODE_ONLY_KEYS, readConfigTiers, resolveCortexKitConfigPaths, resolveLegacyAftConfigSources, + stripHarnessSpecificConfigKeys, stripJsoncSymbols, } from "@cortexkit/aft-bridge"; import { parse as parseJsonc, stringify as stringifyJsonc } from "comment-json"; @@ -489,51 +491,54 @@ const BackupConfigSchema = z.object({ max_file_size: z.number().int().positive().optional(), }); -export const AftConfigSchema = z - .object({ - /** - * Optional JSON Schema URL for editor tooling. Ignored by the plugin at - * runtime — only present so VS Code/Cursor/etc. pick up the published - * schema for autocomplete + validation. `aft setup` auto-inserts this. - */ - $schema: z.string().optional(), - /** Master switch for AFT. Default true. Project config may set it because turning AFT off is not a privilege escalation. */ - enabled: z.boolean().optional(), - /** - * Whether to auto-format files after edits. Default: false — formatting can - * reflow the file under the agent and stale the next edit's context. Opt in - * with `true` if you want AFT to format after edits. - */ - format_on_edit: z.boolean().optional(), - formatter_timeout_secs: z.number().int().min(1).max(600).optional(), - validate_on_edit: z.enum(["syntax", "full"]).optional(), - formatter: z.record(z.string(), FormatterEnum).optional(), - checker: z.record(z.string(), CheckerEnum).optional(), - configure_warnings_delivery: ConfigureWarningsDeliveryEnum.optional(), - tool_surface: z.enum(["minimal", "recommended", "all"]).optional(), - disabled_tools: z.array(z.string()).optional(), - restrict_to_project_root: z.boolean().optional(), - search_index: z.boolean().optional(), - semantic_search: z.boolean().optional(), - callgraph_store: z.boolean().optional(), - callgraph_chunk_size: z.number().optional(), - inspect: InspectConfigSchema.optional(), - backup: BackupConfigSchema.optional(), - /** - * Bash tool family (hoist + rewrite + compress + background execution). - * Default on for `tool_surface: recommended`/`all`, off for `minimal`. - * Three shapes: `true`, `false`, or `{ rewrite?, compress?, background?, ... }`. - * Replaces `experimental.bash.*` (still accepted for backward compat). - */ - bash: BashConfigSchema.optional(), - experimental: ExperimentalConfigSchema.optional(), - lsp: LspConfigSchema.optional(), - url_fetch_allow_private: z.boolean().optional(), - semantic: SemanticConfigSchema.optional(), - bridge: BridgeConfigSchema.optional(), - subc: SubcConfigSchema.optional(), - }) - .strict(); +export const AftConfigSchema = z.preprocess( + (value) => stripHarnessSpecificConfigKeys(value, OPENCODE_ONLY_KEYS), + z + .object({ + /** + * Optional JSON Schema URL for editor tooling. Ignored by the plugin at + * runtime — only present so VS Code/Cursor/etc. pick up the published + * schema for autocomplete + validation. `aft setup` auto-inserts this. + */ + $schema: z.string().optional(), + /** Master switch for AFT. Default true. Project config may set it because turning AFT off is not a privilege escalation. */ + enabled: z.boolean().optional(), + /** + * Whether to auto-format files after edits. Default: false — formatting can + * reflow the file under the agent and stale the next edit's context. Opt in + * with `true` if you want AFT to format after edits. + */ + format_on_edit: z.boolean().optional(), + formatter_timeout_secs: z.number().int().min(1).max(600).optional(), + validate_on_edit: z.enum(["syntax", "full"]).optional(), + formatter: z.record(z.string(), FormatterEnum).optional(), + checker: z.record(z.string(), CheckerEnum).optional(), + configure_warnings_delivery: ConfigureWarningsDeliveryEnum.optional(), + tool_surface: z.enum(["minimal", "recommended", "all"]).optional(), + disabled_tools: z.array(z.string()).optional(), + restrict_to_project_root: z.boolean().optional(), + search_index: z.boolean().optional(), + semantic_search: z.boolean().optional(), + callgraph_store: z.boolean().optional(), + callgraph_chunk_size: z.number().optional(), + inspect: InspectConfigSchema.optional(), + backup: BackupConfigSchema.optional(), + /** + * Bash tool family (hoist + rewrite + compress + background execution). + * Default on for `tool_surface: recommended`/`all`, off for `minimal`. + * Three shapes: `true`, `false`, or `{ rewrite?, compress?, background?, ... }`. + * Replaces `experimental.bash.*` (still accepted for backward compat). + */ + bash: BashConfigSchema.optional(), + experimental: ExperimentalConfigSchema.optional(), + lsp: LspConfigSchema.optional(), + url_fetch_allow_private: z.boolean().optional(), + semantic: SemanticConfigSchema.optional(), + bridge: BridgeConfigSchema.optional(), + subc: SubcConfigSchema.optional(), + }) + .strict(), +); function normalizeLspExtension(extension: string): string { return extension.trim().replace(/^\.+/, ""); diff --git a/packages/pi-plugin/src/index.ts b/packages/pi-plugin/src/index.ts index d9059280..34af4acf 100644 --- a/packages/pi-plugin/src/index.ts +++ b/packages/pi-plugin/src/index.ts @@ -64,7 +64,11 @@ import { resolveBridgePoolTransportOptions, } from "./config.js"; import { bridgeLogger, error, log, warn } from "./logger.js"; -import { abortInFlightAutoInstalls, runAutoInstall } from "./lsp-auto-install.js"; +import { + abortInFlightAutoInstalls, + pushLspPathsAfterAutoInstall, + runAutoInstall, +} from "./lsp-auto-install.js"; import { abortInFlightGithubInstalls, discoverRelevantGithubServers, @@ -530,6 +534,7 @@ export default async function (pi: ExtensionAPI): Promise { const configOverrides = buildConfigTierConfigureParams(projectRoot, { storage_dir: storageDir, }); + let lspInstallCompletion: Promise | null = null; // _ort_dylib_dir is patched in asynchronously below once ensureOnnxRuntime // settles. Bridges spawned before that resolution don't get ORT and // semantic search returns "still building" until they restart. @@ -576,7 +581,8 @@ export default async function (pi: ExtensionAPI): Promise { if (lspInflightInstalls.length > 0) { configOverrides.lsp_inflight_installs = lspInflightInstalls; } - if (npmResult.installsStarted > 0 || ghResult.installsStarted > 0) { + const installsWereStarted = npmResult.installsStarted > 0 || ghResult.installsStarted > 0; + if (installsWereStarted) { log( `[lsp] auto-install: ${npmResult.installsStarted} npm + ${ghResult.installsStarted} github install(s) running in background`, ); @@ -590,8 +596,22 @@ export default async function (pi: ExtensionAPI): Promise { // `warn()` (visible at WARN level) so users running with default logging // see them. Routine skips (already-installed, not-relevant, disabled) // stay out of the warning summary. - Promise.all([npmResult.installsComplete, ghResult.installsComplete]) + const installCompletion = Promise.all([npmResult.installsComplete, ghResult.installsComplete]) .then(() => { + if (installsWereStarted) { + const updatedPaths = [ + ...new Set([...npmResult.getCachedBinDirs(), ...ghResult.getCachedBinDirs()]), + ]; + if (updatedPaths.length > 0) { + configOverrides.lsp_paths_extra = updatedPaths; + } else { + delete configOverrides.lsp_paths_extra; + } + return updatedPaths; + } + return null; + }) + .then((updatedPaths) => { const actionable = [...npmResult.skipped, ...ghResult.skipped].filter((s) => { const r = s.reason.toLowerCase(); if (r === "auto_install: false") return false; @@ -601,17 +621,21 @@ export default async function (pi: ExtensionAPI): Promise { if (r === "another install in progress") return false; return true; }); - if (actionable.length === 0) return; - const lines = actionable.map((s) => ` • ${s.id}: ${s.reason}`).join("\n"); - warn( - `[lsp] skipped or failed to install ${actionable.length} server(s):\n${lines}\n` + - 'Pin a working version with `lsp.versions: { "": "" }` if grace is blocking, ' + - "or set `lsp.auto_install: false` to suppress.", - ); + if (actionable.length > 0) { + const lines = actionable.map((s) => ` • ${s.id}: ${s.reason}`).join("\n"); + warn( + `[lsp] skipped or failed to install ${actionable.length} server(s):\n${lines}\n` + + 'Pin a working version with `lsp.versions: { "": "" }` if grace is blocking, ' + + "or set `lsp.auto_install: false` to suppress.", + ); + } + return updatedPaths; }) .catch((err) => { warn(`[lsp] install-summary aggregation failed: ${err}`); + return null; }); + if (installsWereStarted) lspInstallCompletion = installCompletion; } catch (err) { warn(`[lsp] auto-install setup failed: ${err instanceof Error ? err.message : String(err)}`); } @@ -699,6 +723,20 @@ export default async function (pi: ExtensionAPI): Promise { }); }, }); + if (lspInstallCompletion) { + lspInstallCompletion.then((updatedPaths) => { + if (!updatedPaths) return; + void pushLspPathsAfterAutoInstall(pool, projectRoot, updatedPaths) + .then(() => { + log( + `[lsp] lsp_paths_extra updated after auto-install: ${updatedPaths.length} dirs pushed to live bridges`, + ); + }) + .catch((err) => { + warn(`[lsp] live bridge lsp_paths_extra update failed: ${err}`); + }); + }); + } pool.setConfigureOverride("harness", "pi"); // Tell Rust whether `aft_search` is registered for this surface so the // grep-rewrite footer steers there (vs the grep tool). Set before the eager diff --git a/packages/pi-plugin/src/lsp-auto-install.ts b/packages/pi-plugin/src/lsp-auto-install.ts index a9cdddf2..7fe35770 100644 --- a/packages/pi-plugin/src/lsp-auto-install.ts +++ b/packages/pi-plugin/src/lsp-auto-install.ts @@ -22,10 +22,9 @@ * 3. Spawn `npm install --no-save @ --ignore-scripts` * in the background. Drop a lockfile while running. Log progress. * - * 4. The newly-installed binary will be picked up on the user's NEXT - * plugin session — first session with auto-install just kicks off - * the install. This matches OpenCode's "may need restart" UX and - * avoids mid-session bridge restarts. + * 4. When the background install settles, the plugin re-scans the cache and + * reconfigures any live bridge without restarting it. Future bridges use + * the same paths through the pool's configure overrides. */ import { spawn } from "node:child_process"; @@ -41,7 +40,7 @@ import { writeFileSync, } from "node:fs"; import { join } from "node:path"; -import { npmSpawnEnv, resolveNpm } from "@cortexkit/aft-bridge"; +import { type AftTransportPool, npmSpawnEnv, resolveNpm } from "@cortexkit/aft-bridge"; import { error, log, warn } from "./logger.js"; import { isInstalled, @@ -104,6 +103,8 @@ export interface AutoInstallResult { * the array shared with the synchronous return value). */ installsComplete: Promise; + /** Re-scan the cache after background installs settle. */ + getCachedBinDirs: () => string[]; } /** @@ -567,9 +568,9 @@ function validateCachedNpmInstall(spec: NpmServerSpec): boolean { * have an installed binary AND kicks off background installs for missing * packages relevant to this project. * - * Caller passes `cachedBinDirs` to Rust as `lsp_paths_extra`. The result - * is correct on first launch even though some installs may still be - * running — those binaries appear in the cache on the NEXT session. + * Caller passes `cachedBinDirs` to Rust as `lsp_paths_extra`. When a background + * install settles, the plugin calls `getCachedBinDirs()` to refresh the list and + * reconfigures live bridges; future bridges use the pool override. */ export function runAutoInstall( projectRoot: string, @@ -648,5 +649,20 @@ export function runAutoInstall( }, skipped, installsComplete: Promise.all(installPromises).then(() => {}), + getCachedBinDirs: () => + NPM_LSP_TABLE.filter( + (spec) => isInstalled(spec.npm, spec.binary) && validateCachedNpmInstall(spec), + ).map((spec) => lspBinDir(spec.npm)), }; } + +/** Apply newly discovered cache paths to both future and live bridge config. */ +export async function pushLspPathsAfterAutoInstall( + pool: Pick, + projectRoot: string, + cachedBinDirs: readonly string[], +): Promise { + const paths = [...new Set(cachedBinDirs)]; + pool.setConfigureOverride("lsp_paths_extra", paths); + await pool.reconfigure(projectRoot, { lsp_paths_extra: paths }); +} diff --git a/packages/pi-plugin/src/lsp-github-install.ts b/packages/pi-plugin/src/lsp-github-install.ts index a295ce4e..b81cc3e6 100644 --- a/packages/pi-plugin/src/lsp-github-install.ts +++ b/packages/pi-plugin/src/lsp-github-install.ts @@ -1064,6 +1064,8 @@ export interface GithubAutoInstallResult { * Plugin startup ignores this; tests await it. */ installsComplete: Promise; + /** Re-scan the cache after background installs settle. */ + getCachedBinDirs: () => string[]; } interface InFlightGithubInstall { @@ -1134,6 +1136,7 @@ export function runGithubAutoInstall( installingBinaries: [], skipped, installsComplete: Promise.resolve(), + getCachedBinDirs: () => [...cachedBinDirs], }; } @@ -1194,6 +1197,15 @@ export function runGithubAutoInstall( }, skipped, installsComplete: Promise.all(installPromises).then(() => {}), + getCachedBinDirs: () => { + const currentHost = detectHostPlatform(); + if (!currentHost) return []; + return GITHUB_LSP_TABLE.filter( + (spec) => + isGithubInstalled(spec, currentHost.platform) && + validateCachedGithubInstall(spec, currentHost.platform), + ).map((spec) => ghBinDir(spec)); + }, }; }