mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
Added separate experimental feature flags for live update of skills and agents
This commit is contained in:
@@ -13,7 +13,6 @@ In the codex-rs folder where the rust code lives:
|
||||
- Use method references over closures when possible per https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_for_method_calls
|
||||
- When possible, make `match` statements exhaustive and avoid wildcard arms.
|
||||
- When writing tests, prefer comparing the equality of entire objects over fields one by one.
|
||||
- When making a change that adds or changes an API, ensure that the documentation in the `docs/` folder is up to date if applicable.
|
||||
- If you change `ConfigToml` or nested config types, run `just write-config-schema` to update `codex-rs/core/config.schema.json`.
|
||||
|
||||
Run `just fmt` (in `codex-rs` directory) automatically after you have finished making Rust code changes; do not ask for approval to run it. Additionally, run the tests:
|
||||
|
||||
@@ -180,6 +180,12 @@
|
||||
"include_apply_patch_tool": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"live_agents_reload": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"live_skills_reload": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"powershell_utf8": {
|
||||
"type": "boolean"
|
||||
},
|
||||
@@ -1198,6 +1204,12 @@
|
||||
"include_apply_patch_tool": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"live_agents_reload": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"live_skills_reload": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"powershell_utf8": {
|
||||
"type": "boolean"
|
||||
},
|
||||
|
||||
@@ -448,6 +448,8 @@ pub(crate) struct Session {
|
||||
pub(crate) services: SessionServices,
|
||||
agents_changed: Arc<AtomicBool>,
|
||||
agents_watch_dirs: Vec<PathBuf>,
|
||||
live_agents_reload: bool,
|
||||
live_skills_reload: bool,
|
||||
next_internal_sub_id: AtomicU64,
|
||||
}
|
||||
|
||||
@@ -622,17 +624,23 @@ impl Session {
|
||||
}
|
||||
|
||||
fn start_file_watcher_listener(self: &Arc<Self>) {
|
||||
if !self.live_agents_reload && !self.live_skills_reload {
|
||||
return;
|
||||
}
|
||||
let mut rx = self.services.file_watcher.subscribe();
|
||||
let agents_changed = Arc::clone(&self.agents_changed);
|
||||
let agents_watch_dirs = self.agents_watch_dirs.clone();
|
||||
let live_agents_reload = self.live_agents_reload;
|
||||
let live_skills_reload = self.live_skills_reload;
|
||||
let weak_sess = Arc::downgrade(self);
|
||||
tokio::spawn(async move {
|
||||
loop {
|
||||
match rx.recv().await {
|
||||
Ok(FileWatcherEvent::AgentsChanged { paths }) => {
|
||||
if paths
|
||||
.iter()
|
||||
.any(|path| agents_watch_dirs.iter().any(|root| path.starts_with(root)))
|
||||
if live_agents_reload
|
||||
&& paths.iter().any(|path| {
|
||||
agents_watch_dirs.iter().any(|root| path.starts_with(root))
|
||||
})
|
||||
&& !agents_changed.swap(true, Ordering::SeqCst)
|
||||
{
|
||||
info!(
|
||||
@@ -642,6 +650,9 @@ impl Session {
|
||||
}
|
||||
}
|
||||
Ok(FileWatcherEvent::SkillsChanged { .. }) => {
|
||||
if !live_skills_reload {
|
||||
continue;
|
||||
}
|
||||
let Some(sess) = weak_sess.upgrade() else {
|
||||
break;
|
||||
};
|
||||
@@ -897,7 +908,13 @@ impl Session {
|
||||
);
|
||||
}
|
||||
let state = SessionState::new(session_configuration.clone());
|
||||
let agents_watch_dirs = Self::build_agents_watch_dirs(&config);
|
||||
let live_agents_reload = config.features.enabled(Feature::LiveAgentsReload);
|
||||
let live_skills_reload = config.features.enabled(Feature::LiveSkillsReload);
|
||||
let agents_watch_dirs = if live_agents_reload {
|
||||
Self::build_agents_watch_dirs(&config)
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
|
||||
let services = SessionServices {
|
||||
mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::default())),
|
||||
@@ -929,6 +946,8 @@ impl Session {
|
||||
services,
|
||||
agents_changed: Arc::new(AtomicBool::new(false)),
|
||||
agents_watch_dirs,
|
||||
live_agents_reload,
|
||||
live_skills_reload,
|
||||
next_internal_sub_id: AtomicU64::new(0),
|
||||
});
|
||||
|
||||
@@ -1860,6 +1879,9 @@ impl Session {
|
||||
// If `AGENTS.md` changed, reload skills, recompute user instructions, and
|
||||
// update session state; otherwise return `None`.
|
||||
pub(crate) async fn refresh_user_instructions_if_needed(&self) -> Option<String> {
|
||||
if !self.live_agents_reload {
|
||||
return None;
|
||||
}
|
||||
if !self.agents_changed.swap(false, Ordering::SeqCst) {
|
||||
return None;
|
||||
}
|
||||
@@ -4708,6 +4730,13 @@ mod tests {
|
||||
agent_control,
|
||||
state_db: None,
|
||||
};
|
||||
let live_agents_reload = config.features.enabled(Feature::LiveAgentsReload);
|
||||
let live_skills_reload = config.features.enabled(Feature::LiveSkillsReload);
|
||||
let agents_watch_dirs = if live_agents_reload {
|
||||
Session::build_agents_watch_dirs(config.as_ref())
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
|
||||
let turn_context = Session::make_turn_context(
|
||||
Some(Arc::clone(&auth_manager)),
|
||||
@@ -4730,7 +4759,9 @@ mod tests {
|
||||
active_turn: Mutex::new(None),
|
||||
services,
|
||||
agents_changed: Arc::new(AtomicBool::new(false)),
|
||||
agents_watch_dirs: Session::build_agents_watch_dirs(&config),
|
||||
agents_watch_dirs,
|
||||
live_agents_reload,
|
||||
live_skills_reload,
|
||||
next_internal_sub_id: AtomicU64::new(0),
|
||||
};
|
||||
|
||||
@@ -4824,6 +4855,13 @@ mod tests {
|
||||
agent_control,
|
||||
state_db: None,
|
||||
};
|
||||
let live_agents_reload = config.features.enabled(Feature::LiveAgentsReload);
|
||||
let live_skills_reload = config.features.enabled(Feature::LiveSkillsReload);
|
||||
let agents_watch_dirs = if live_agents_reload {
|
||||
Session::build_agents_watch_dirs(config.as_ref())
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
|
||||
let turn_context = Arc::new(Session::make_turn_context(
|
||||
Some(Arc::clone(&auth_manager)),
|
||||
@@ -4846,7 +4884,9 @@ mod tests {
|
||||
active_turn: Mutex::new(None),
|
||||
services,
|
||||
agents_changed: Arc::new(AtomicBool::new(false)),
|
||||
agents_watch_dirs: Session::build_agents_watch_dirs(&config),
|
||||
agents_watch_dirs,
|
||||
live_agents_reload,
|
||||
live_skills_reload,
|
||||
next_internal_sub_id: AtomicU64::new(0),
|
||||
});
|
||||
|
||||
|
||||
@@ -115,6 +115,10 @@ pub enum Feature {
|
||||
Connectors,
|
||||
/// Allow prompting and installing missing MCP dependencies.
|
||||
SkillMcpDependencyInstall,
|
||||
/// Reload AGENTS.md-based instructions when AGENTS files change on disk.
|
||||
LiveAgentsReload,
|
||||
/// Reload skill metadata when skill files change on disk.
|
||||
LiveSkillsReload,
|
||||
/// Steer feature flag - when enabled, Enter submits immediately instead of queuing.
|
||||
Steer,
|
||||
/// Enable collaboration modes (Plan, Code, Pair Programming, Execute).
|
||||
@@ -527,6 +531,26 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::LiveAgentsReload,
|
||||
key: "live_agents_reload",
|
||||
stage: Stage::Experimental {
|
||||
name: "Live AGENTS reload",
|
||||
menu_description: "Reload AGENTS.md instructions on the next turn after AGENTS files change.",
|
||||
announcement: "NEW! Try live AGENTS reload to pick up AGENTS.md changes between turns. Enable in /experimental!",
|
||||
},
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::LiveSkillsReload,
|
||||
key: "live_skills_reload",
|
||||
stage: Stage::Experimental {
|
||||
name: "Live skills reload",
|
||||
menu_description: "Reload skills and notify sessions when skill files change on disk.",
|
||||
announcement: "NEW! Try live skills reload to pick up skill changes between turns. Enable in /experimental!",
|
||||
},
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::Steer,
|
||||
key: "steer",
|
||||
|
||||
@@ -19,6 +19,7 @@ use tokio::sync::mpsc;
|
||||
use tracing::warn;
|
||||
|
||||
use crate::config::Config;
|
||||
use crate::features::Feature;
|
||||
use crate::project_doc::DEFAULT_PROJECT_DOC_FILENAME;
|
||||
use crate::project_doc::LOCAL_PROJECT_DOC_FILENAME;
|
||||
use crate::project_doc::project_doc_search_dirs;
|
||||
@@ -32,6 +33,8 @@ pub enum FileWatcherEvent {
|
||||
|
||||
struct WatchState {
|
||||
skills_roots: HashSet<PathBuf>,
|
||||
agents_enabled: bool,
|
||||
skills_enabled: bool,
|
||||
}
|
||||
|
||||
struct FileWatcherInner {
|
||||
@@ -46,7 +49,7 @@ pub(crate) struct FileWatcher {
|
||||
}
|
||||
|
||||
impl FileWatcher {
|
||||
pub(crate) fn new(codex_home: PathBuf) -> notify::Result<Self> {
|
||||
pub(crate) fn new(_codex_home: PathBuf) -> notify::Result<Self> {
|
||||
let (raw_tx, raw_rx) = mpsc::unbounded_channel();
|
||||
let raw_tx_clone = raw_tx;
|
||||
let watcher = notify::recommended_watcher(move |res| {
|
||||
@@ -59,6 +62,8 @@ impl FileWatcher {
|
||||
let (tx, _) = broadcast::channel(128);
|
||||
let state = Arc::new(RwLock::new(WatchState {
|
||||
skills_roots: HashSet::new(),
|
||||
agents_enabled: false,
|
||||
skills_enabled: false,
|
||||
}));
|
||||
let file_watcher = Self {
|
||||
inner: Some(Mutex::new(inner)),
|
||||
@@ -66,8 +71,6 @@ impl FileWatcher {
|
||||
tx: tx.clone(),
|
||||
};
|
||||
file_watcher.spawn_event_loop(raw_rx, state, tx);
|
||||
file_watcher.watch_agents_root(codex_home.clone());
|
||||
file_watcher.register_skills_root(codex_home.join("skills"));
|
||||
Ok(file_watcher)
|
||||
}
|
||||
|
||||
@@ -77,6 +80,8 @@ impl FileWatcher {
|
||||
inner: None,
|
||||
state: Arc::new(RwLock::new(WatchState {
|
||||
skills_roots: HashSet::new(),
|
||||
agents_enabled: false,
|
||||
skills_enabled: false,
|
||||
})),
|
||||
tx,
|
||||
}
|
||||
@@ -87,22 +92,44 @@ impl FileWatcher {
|
||||
}
|
||||
|
||||
pub(crate) fn register_config(&self, config: &Config) {
|
||||
self.watch_agents_root(config.codex_home.clone());
|
||||
let agents_enabled = config.features.enabled(Feature::LiveAgentsReload);
|
||||
let skills_enabled = config.features.enabled(Feature::LiveSkillsReload);
|
||||
|
||||
match project_doc_search_dirs(config) {
|
||||
Ok(dirs) => {
|
||||
for dir in dirs {
|
||||
self.watch_path(dir, RecursiveMode::NonRecursive);
|
||||
}
|
||||
}
|
||||
Err(err) => {
|
||||
warn!("failed to determine AGENTS.md search dirs: {err}");
|
||||
{
|
||||
let mut state = match self.state.write() {
|
||||
Ok(state) => state,
|
||||
Err(err) => err.into_inner(),
|
||||
};
|
||||
state.agents_enabled = agents_enabled;
|
||||
state.skills_enabled = skills_enabled;
|
||||
if !skills_enabled {
|
||||
state.skills_roots.clear();
|
||||
}
|
||||
}
|
||||
|
||||
let roots = skill_roots_from_layer_stack(&config.config_layer_stack);
|
||||
for root in roots {
|
||||
self.register_skills_root(root.path);
|
||||
if agents_enabled {
|
||||
self.watch_agents_root(config.codex_home.clone());
|
||||
}
|
||||
|
||||
if agents_enabled {
|
||||
match project_doc_search_dirs(config) {
|
||||
Ok(dirs) => {
|
||||
for dir in dirs {
|
||||
self.watch_path(dir, RecursiveMode::NonRecursive);
|
||||
}
|
||||
}
|
||||
Err(err) => {
|
||||
warn!("failed to determine AGENTS.md search dirs: {err}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if skills_enabled {
|
||||
self.register_skills_root(config.codex_home.join("skills"));
|
||||
let roots = skill_roots_from_layer_stack(&config.config_layer_stack);
|
||||
for root in roots {
|
||||
self.register_skills_root(root.path);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -187,16 +214,27 @@ impl FileWatcher {
|
||||
fn classify_event(event: &Event, state: &RwLock<WatchState>) -> (Vec<PathBuf>, Vec<PathBuf>) {
|
||||
let mut agents_paths = Vec::new();
|
||||
let mut skills_paths = Vec::new();
|
||||
let skills_roots = match state.read() {
|
||||
Ok(state) => state.skills_roots.clone(),
|
||||
Err(err) => err.into_inner().skills_roots.clone(),
|
||||
let (agents_enabled, skills_enabled, skills_roots) = match state.read() {
|
||||
Ok(state) => (
|
||||
state.agents_enabled,
|
||||
state.skills_enabled,
|
||||
state.skills_roots.clone(),
|
||||
),
|
||||
Err(err) => {
|
||||
let state = err.into_inner();
|
||||
(
|
||||
state.agents_enabled,
|
||||
state.skills_enabled,
|
||||
state.skills_roots.clone(),
|
||||
)
|
||||
}
|
||||
};
|
||||
|
||||
for path in &event.paths {
|
||||
if is_agents_path(path) {
|
||||
if agents_enabled && is_agents_path(path) {
|
||||
agents_paths.push(path.clone());
|
||||
}
|
||||
if is_skills_path(path, &skills_roots) {
|
||||
if skills_enabled && is_skills_path(path, &skills_roots) {
|
||||
skills_paths.push(path.clone());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -34,6 +34,7 @@ use std::sync::Arc;
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
use tempfile::TempDir;
|
||||
use tokio::runtime::Handle;
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
use tokio::runtime::RuntimeFlavor;
|
||||
use tokio::sync::RwLock;
|
||||
use tokio::sync::broadcast;
|
||||
|
||||
Reference in New Issue
Block a user