mirror of
https://github.com/openai/codex.git
synced 2026-11-02 19:43:24 +00:00
codex: address PR review feedback (#16282)
This commit is contained in:
Eric Traut
@@ -178,7 +178,7 @@ Example with notification opt-out:
|
||||
- `fs/changed` — notification emitted when watched paths change, including the `watchId` and `changedPaths`.
|
||||
- `model/list` — list available models (set `includeHidden: true` to include entries with `hidden: true`), with reasoning effort options, optional legacy `upgrade` model ids, optional `upgradeInfo` metadata (`model`, `upgradeCopy`, `modelLink`, `migrationMarkdown`), and optional `availabilityNux` metadata.
|
||||
- `experimentalFeature/list` — list feature flags with stage metadata (`beta`, `underDevelopment`, `stable`, etc.), enabled/default-enabled state, and cursor pagination. For non-beta flags, `displayName`/`description`/`announcement` are `null`.
|
||||
- `experimentalFeature/enablement/set` — patch the in-memory process-wide runtime feature enablement for the currently supported feature keys (`apps`, `plugins`, `job_scheduler`). For each feature, precedence is: cloud requirements > --enable <feature_name> > config.toml > experimentalFeature/enablement/set (new) > code default.
|
||||
- `experimentalFeature/enablement/set` — patch the in-memory process-wide runtime feature enablement for the currently supported feature keys (`apps`, `plugins`). For each feature, precedence is: cloud requirements > --enable <feature_name> > config.toml > experimentalFeature/enablement/set (new) > code default.
|
||||
- `collaborationMode/list` — list available collaboration mode presets (experimental, no pagination). This response omits built-in developer instructions; clients should either pass `settings.developer_instructions: null` when setting a mode to use Codex's built-in instructions, or provide their own instructions explicitly.
|
||||
- `skills/list` — list skills for one or more `cwd` values (optional `forceReload`).
|
||||
- `plugin/list` — list discovered plugin marketplaces and plugin state, including effective marketplace install/auth policy metadata, fail-open `marketplaceLoadErrors` entries for marketplace files that could not be parsed or loaded, and best-effort `featuredPluginIds` for the official curated marketplace. `interface.category` uses the marketplace category when present; otherwise it falls back to the plugin manifest category. Pass `forceRemoteSync: true` to refresh curated plugin state before listing (**under development; do not call from production clients yet**).
|
||||
|
||||
@@ -495,7 +495,7 @@ async fn append_message_records_assistant_message() {
|
||||
.expect("append_message should succeed");
|
||||
assert!(!submission_id.is_empty());
|
||||
|
||||
timeout(Duration::from_secs(15), async {
|
||||
timeout(Duration::from_secs(5), async {
|
||||
loop {
|
||||
let history_items = thread
|
||||
.codex
|
||||
@@ -1478,7 +1478,7 @@ async fn resume_thread_subagent_restores_stored_nickname_and_role() {
|
||||
let state_db = child_thread
|
||||
.state_db()
|
||||
.expect("sqlite state db should be available for nickname resume test");
|
||||
timeout(Duration::from_secs(15), async {
|
||||
timeout(Duration::from_secs(5), async {
|
||||
loop {
|
||||
if let Ok(Some(metadata)) = state_db.get_thread(child_thread_id).await
|
||||
&& metadata.agent_nickname.is_some()
|
||||
|
||||
@@ -34,7 +34,6 @@ use core_test_support::PathExt;
|
||||
use core_test_support::TempDirExt;
|
||||
use core_test_support::test_absolute_path;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serial_test::serial;
|
||||
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
@@ -5725,96 +5724,80 @@ shell_tool = true
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn approvals_reviewer_defaults_to_manual_only_without_guardian_feature() -> std::io::Result<()>
|
||||
{
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
ConfigToml::default(),
|
||||
ConfigOverrides {
|
||||
cwd: Some(codex_home.path().to_path_buf()),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.path().to_path_buf(),
|
||||
)?;
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.build()
|
||||
.await?;
|
||||
|
||||
assert_eq!(config.approvals_reviewer, ApprovalsReviewer::User);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn approvals_reviewer_stays_manual_only_when_guardian_feature_is_enabled()
|
||||
-> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let raw_config = r#"[features]
|
||||
std::fs::write(
|
||||
codex_home.path().join(CONFIG_TOML_FILE),
|
||||
r#"[features]
|
||||
guardian_approval = true
|
||||
"#;
|
||||
std::fs::write(codex_home.path().join(CONFIG_TOML_FILE), raw_config)?;
|
||||
let config_toml = toml::from_str::<ConfigToml>(raw_config)
|
||||
.map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err))?;
|
||||
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
config_toml,
|
||||
ConfigOverrides {
|
||||
cwd: Some(codex_home.path().to_path_buf()),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.path().to_path_buf(),
|
||||
"#,
|
||||
)?;
|
||||
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.build()
|
||||
.await?;
|
||||
|
||||
assert_eq!(config.approvals_reviewer, ApprovalsReviewer::User);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn approvals_reviewer_can_be_set_in_config_without_guardian_approval() -> std::io::Result<()>
|
||||
{
|
||||
let codex_home = TempDir::new()?;
|
||||
let raw_config = r#"approvals_reviewer = "user"
|
||||
"#;
|
||||
std::fs::write(codex_home.path().join(CONFIG_TOML_FILE), raw_config)?;
|
||||
let config_toml = toml::from_str::<ConfigToml>(raw_config)
|
||||
.map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err))?;
|
||||
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
config_toml,
|
||||
ConfigOverrides {
|
||||
cwd: Some(codex_home.path().to_path_buf()),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.path().to_path_buf(),
|
||||
std::fs::write(
|
||||
codex_home.path().join(CONFIG_TOML_FILE),
|
||||
r#"approvals_reviewer = "user"
|
||||
"#,
|
||||
)?;
|
||||
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.build()
|
||||
.await?;
|
||||
|
||||
assert_eq!(config.approvals_reviewer, ApprovalsReviewer::User);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn approvals_reviewer_can_be_set_in_profile_without_guardian_approval() -> std::io::Result<()>
|
||||
{
|
||||
let codex_home = TempDir::new()?;
|
||||
let raw_config = r#"profile = "guardian"
|
||||
std::fs::write(
|
||||
codex_home.path().join(CONFIG_TOML_FILE),
|
||||
r#"profile = "guardian"
|
||||
|
||||
[profiles.guardian]
|
||||
approvals_reviewer = "guardian_subagent"
|
||||
"#;
|
||||
std::fs::write(codex_home.path().join(CONFIG_TOML_FILE), raw_config)?;
|
||||
let config_toml = toml::from_str::<ConfigToml>(raw_config)
|
||||
.map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err))?;
|
||||
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
config_toml,
|
||||
ConfigOverrides {
|
||||
cwd: Some(codex_home.path().to_path_buf()),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.path().to_path_buf(),
|
||||
"#,
|
||||
)?;
|
||||
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.build()
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.approvals_reviewer,
|
||||
ApprovalsReviewer::GuardianSubagent
|
||||
@@ -5823,25 +5806,21 @@ approvals_reviewer = "guardian_subagent"
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn smart_approvals_alias_is_ignored() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let raw_config = r#"[features]
|
||||
std::fs::write(
|
||||
codex_home.path().join(CONFIG_TOML_FILE),
|
||||
r#"[features]
|
||||
smart_approvals = true
|
||||
"#;
|
||||
std::fs::write(codex_home.path().join(CONFIG_TOML_FILE), raw_config)?;
|
||||
let config_toml = toml::from_str::<ConfigToml>(raw_config)
|
||||
.map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err))?;
|
||||
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
config_toml,
|
||||
ConfigOverrides {
|
||||
cwd: Some(codex_home.path().to_path_buf()),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.path().to_path_buf(),
|
||||
"#,
|
||||
)?;
|
||||
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.build()
|
||||
.await?;
|
||||
|
||||
assert!(!config.features.enabled(Feature::GuardianApproval));
|
||||
assert_eq!(config.approvals_reviewer, ApprovalsReviewer::User);
|
||||
|
||||
@@ -5854,27 +5833,23 @@ smart_approvals = true
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn smart_approvals_alias_is_ignored_in_profiles() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let raw_config = r#"profile = "guardian"
|
||||
std::fs::write(
|
||||
codex_home.path().join(CONFIG_TOML_FILE),
|
||||
r#"profile = "guardian"
|
||||
|
||||
[profiles.guardian.features]
|
||||
smart_approvals = true
|
||||
"#;
|
||||
std::fs::write(codex_home.path().join(CONFIG_TOML_FILE), raw_config)?;
|
||||
let config_toml = toml::from_str::<ConfigToml>(raw_config)
|
||||
.map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err))?;
|
||||
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
config_toml,
|
||||
ConfigOverrides {
|
||||
cwd: Some(codex_home.path().to_path_buf()),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.path().to_path_buf(),
|
||||
"#,
|
||||
)?;
|
||||
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.build()
|
||||
.await?;
|
||||
|
||||
assert!(!config.features.enabled(Feature::GuardianApproval));
|
||||
assert_eq!(config.approvals_reviewer, ApprovalsReviewer::User);
|
||||
|
||||
|
||||
+59
-13
@@ -19,6 +19,13 @@ const EVERY_PREFIX: &str = "@every ";
|
||||
const EVERY_SECONDS_PREFIX: &str = "@every:";
|
||||
pub const JOB_UPDATED_BACKGROUND_EVENT_PREFIX: &str = "job_updated:";
|
||||
pub const JOB_FIRED_BACKGROUND_EVENT_PREFIX: &str = "job_fired:";
|
||||
pub const MAX_ACTIVE_JOBS_PER_THREAD: usize = 256;
|
||||
const ONE_SHOT_JOB_TURN_INSTRUCTIONS: &str =
|
||||
include_str!("../templates/jobs/one_shot_turn_instructions.md");
|
||||
const RECURRING_JOB_TURN_INSTRUCTIONS: &str =
|
||||
include_str!("../templates/jobs/recurring_turn_instructions.md");
|
||||
const ONE_SHOT_JOB_PROMPT: &str = include_str!("../templates/jobs/one_shot_prompt.md");
|
||||
const RECURRING_JOB_PROMPT: &str = include_str!("../templates/jobs/recurring_prompt.md");
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct ThreadJob {
|
||||
@@ -124,6 +131,11 @@ impl JobsState {
|
||||
now: DateTime<Utc>,
|
||||
timer_cancel: Option<CancellationToken>,
|
||||
) -> Result<ThreadJob, String> {
|
||||
if self.jobs.len() >= MAX_ACTIVE_JOBS_PER_THREAD {
|
||||
return Err(format!(
|
||||
"too many active jobs; each thread supports at most {MAX_ACTIVE_JOBS_PER_THREAD} jobs"
|
||||
));
|
||||
}
|
||||
let schedule = JobSchedule::parse(&cron_expression)?;
|
||||
let job = ThreadJob {
|
||||
id: id.clone(),
|
||||
@@ -227,26 +239,17 @@ impl JobsState {
|
||||
|
||||
pub(crate) fn job_turn_developer_instructions(job: &JobTurnContext) -> String {
|
||||
if job.run_once {
|
||||
format!(
|
||||
"This turn was triggered by a one-shot thread job.\ncurrentJobId: {}\nSchedule: {}\n\nThe job prompt has already been injected as hidden context for this turn.\nThis one-shot job has already been removed from the schedule, so you do not need to call JobDelete.\nDo not expose scheduler internals unless they matter to the user.",
|
||||
job.current_job_id, job.cron_expression
|
||||
)
|
||||
render_job_prompt_template(ONE_SHOT_JOB_TURN_INSTRUCTIONS, job)
|
||||
} else {
|
||||
format!(
|
||||
"This turn was triggered by a recurring thread job.\ncurrentJobId: {}\nSchedule: {}\n\nThe job prompt has already been injected as hidden context for this turn.\nDo not delete this recurring job just because you completed one run.\nOnly call JobDelete with the exact arguments {{\"id\":\"{}\"}} if the user explicitly asked for a stopping condition and that condition is now satisfied, or if the user explicitly asked to stop the recurring job.\nDo not expose scheduler internals unless they matter to the user.",
|
||||
job.current_job_id, job.cron_expression, job.current_job_id
|
||||
)
|
||||
render_job_prompt_template(RECURRING_JOB_TURN_INSTRUCTIONS, job)
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn job_prompt_input_item(job: &JobTurnContext) -> ResponseInputItem {
|
||||
let text = if job.run_once {
|
||||
format!("One-shot scheduled job prompt:\n{}", job.prompt)
|
||||
render_job_prompt_template(ONE_SHOT_JOB_PROMPT, job)
|
||||
} else {
|
||||
format!(
|
||||
"Recurring scheduled job prompt:\n{}\n\nThis job should keep running on its schedule unless the user asked for a stopping condition and that condition is now satisfied.\nIf that stopping condition is satisfied, stop the job by calling JobDelete with {{\"id\":\"{}\"}}.",
|
||||
job.prompt, job.current_job_id
|
||||
)
|
||||
render_job_prompt_template(RECURRING_JOB_PROMPT, job)
|
||||
};
|
||||
ResponseInputItem::Message {
|
||||
role: "developer".to_string(),
|
||||
@@ -254,6 +257,15 @@ pub(crate) fn job_prompt_input_item(job: &JobTurnContext) -> ResponseInputItem {
|
||||
}
|
||||
}
|
||||
|
||||
fn render_job_prompt_template(template: &str, job: &JobTurnContext) -> String {
|
||||
template
|
||||
.replace("{{CURRENT_JOB_ID}}", &job.current_job_id)
|
||||
.replace("{{SCHEDULE}}", &job.cron_expression)
|
||||
.replace("{{PROMPT}}", &job.prompt)
|
||||
.trim_end()
|
||||
.to_string()
|
||||
}
|
||||
|
||||
fn parse_duration_literal(raw: &str) -> Option<u64> {
|
||||
let mut digits = String::new();
|
||||
let mut unit = String::new();
|
||||
@@ -280,6 +292,7 @@ mod tests {
|
||||
use super::JobSchedule;
|
||||
use super::JobTurnContext;
|
||||
use super::JobsState;
|
||||
use super::MAX_ACTIVE_JOBS_PER_THREAD;
|
||||
use super::job_prompt_input_item;
|
||||
use chrono::TimeZone;
|
||||
use chrono::Utc;
|
||||
@@ -325,6 +338,39 @@ mod tests {
|
||||
assert!(jobs.list_jobs().is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn create_job_rejects_more_than_maximum_active_jobs() {
|
||||
let now = Utc.timestamp_opt(100, 0).single().expect("valid timestamp");
|
||||
let mut jobs = JobsState::default();
|
||||
for index in 0..MAX_ACTIVE_JOBS_PER_THREAD {
|
||||
jobs.create_job(
|
||||
format!("job-{index}"),
|
||||
AFTER_TURN_CRON_EXPRESSION.to_string(),
|
||||
format!("prompt-{index}"),
|
||||
/*run_once*/ false,
|
||||
now,
|
||||
/*timer_cancel*/ None,
|
||||
)
|
||||
.expect("job should be created");
|
||||
}
|
||||
|
||||
let result = jobs.create_job(
|
||||
"job-overflow".to_string(),
|
||||
AFTER_TURN_CRON_EXPRESSION.to_string(),
|
||||
"overflow".to_string(),
|
||||
/*run_once*/ false,
|
||||
now,
|
||||
/*timer_cancel*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
result,
|
||||
Err(format!(
|
||||
"too many active jobs; each thread supports at most {MAX_ACTIVE_JOBS_PER_THREAD} jobs"
|
||||
))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn job_prompt_input_is_hidden_developer_input() {
|
||||
let item = job_prompt_input_item(&JobTurnContext {
|
||||
|
||||
@@ -1,3 +1,8 @@
|
||||
//! Built-in tool handlers for thread-local runtime job management.
|
||||
//!
|
||||
//! These handlers bridge `JobCreate`, `JobDelete`, and `JobList` tool calls
|
||||
//! onto the current thread session's in-memory job registry.
|
||||
|
||||
use async_trait::async_trait;
|
||||
use serde::Deserialize;
|
||||
|
||||
|
||||
@@ -1074,7 +1074,7 @@ async fn multi_agent_v2_assign_task_interrupts_busy_child_without_losing_message
|
||||
)
|
||||
}));
|
||||
|
||||
timeout(Duration::from_secs(15), async {
|
||||
timeout(Duration::from_secs(5), async {
|
||||
loop {
|
||||
let history_items = thread
|
||||
.codex
|
||||
|
||||
@@ -0,0 +1,2 @@
|
||||
One-shot scheduled job prompt:
|
||||
{{PROMPT}}
|
||||
@@ -0,0 +1,7 @@
|
||||
This turn was triggered by a one-shot thread job.
|
||||
currentJobId: {{CURRENT_JOB_ID}}
|
||||
Schedule: {{SCHEDULE}}
|
||||
|
||||
The job prompt has already been injected as hidden context for this turn.
|
||||
This one-shot job has already been removed from the schedule, so you do not need to call JobDelete.
|
||||
Do not expose scheduler internals unless they matter to the user.
|
||||
@@ -0,0 +1,5 @@
|
||||
Recurring scheduled job prompt:
|
||||
{{PROMPT}}
|
||||
|
||||
This job should keep running on its schedule unless the user asked for a stopping condition and that condition is now satisfied.
|
||||
If that stopping condition is satisfied, stop the job by calling JobDelete with {"id":"{{CURRENT_JOB_ID}}"}.
|
||||
@@ -0,0 +1,8 @@
|
||||
This turn was triggered by a recurring thread job.
|
||||
currentJobId: {{CURRENT_JOB_ID}}
|
||||
Schedule: {{SCHEDULE}}
|
||||
|
||||
The job prompt has already been injected as hidden context for this turn.
|
||||
Do not delete this recurring job just because you completed one run.
|
||||
Only call JobDelete with the exact arguments {"id":"{{CURRENT_JOB_ID}}"} if the user explicitly asked for a stopping condition and that condition is now satisfied, or if the user explicitly asked to stop the recurring job.
|
||||
Do not expose scheduler internals unless they matter to the user.
|
||||
@@ -1,4 +1,5 @@
|
||||
use anyhow::Result;
|
||||
use anyhow::anyhow;
|
||||
use codex_core::jobs::AFTER_TURN_CRON_EXPRESSION;
|
||||
use codex_core::jobs::JOB_FIRED_BACKGROUND_EVENT_PREFIX;
|
||||
use codex_core::jobs::ThreadJob;
|
||||
@@ -41,7 +42,7 @@ async fn assert_after_turn_job_starts_and_emits_fired_event() -> Result<()> {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::JobScheduler)
|
||||
.expect("test config should allow feature update");
|
||||
.unwrap_or_else(|err| panic!("test config should allow feature update: {err}"));
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
@@ -53,7 +54,7 @@ async fn assert_after_turn_job_starts_and_emits_fired_event() -> Result<()> {
|
||||
/*run_once*/ false,
|
||||
)
|
||||
.await
|
||||
.expect("job should be created");
|
||||
.map_err(|err| anyhow!("{err}"))?;
|
||||
|
||||
let payload = wait_for_event_match(&test.codex, |event| match event {
|
||||
EventMsg::BackgroundEvent(event) => event
|
||||
@@ -68,7 +69,7 @@ async fn assert_after_turn_job_starts_and_emits_fired_event() -> Result<()> {
|
||||
last_run_at: Some(
|
||||
fired
|
||||
.last_run_at
|
||||
.expect("claimed job should record last_run_at"),
|
||||
.ok_or_else(|| anyhow!("claimed job should record last_run_at"))?,
|
||||
),
|
||||
..created
|
||||
};
|
||||
|
||||
@@ -1,3 +1,8 @@
|
||||
//! Responses API tool specs for thread-local runtime job management.
|
||||
//!
|
||||
//! These specs expose the `JobCreate`, `JobDelete`, and `JobList` built-in
|
||||
//! tools so models can create, inspect, and delete jobs on the current thread.
|
||||
|
||||
use crate::JsonSchema;
|
||||
use crate::ResponsesApiTool;
|
||||
use crate::ToolSpec;
|
||||
|
||||
+18
@@ -0,0 +1,18 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
assertion_line: 9607
|
||||
expression: popup
|
||||
---
|
||||
Update Model Permissions
|
||||
|
||||
› 1. Default Codex can read and edit files in the current
|
||||
workspace, and run commands. Approval is required to
|
||||
access the internet or edit other files.
|
||||
2. Guardian Approvals Same workspace-write permissions as Default, but
|
||||
eligible `on-request` approvals are routed through
|
||||
the guardian reviewer subagent.
|
||||
3. Full Access Codex can edit files outside this workspace and
|
||||
access the internet without asking for approval.
|
||||
Exercise caution when using.
|
||||
|
||||
Press enter to confirm or esc to go back
|
||||
+10
@@ -0,0 +1,10 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
assertion_line: 12789
|
||||
expression: combined
|
||||
---
|
||||
• Running UserPromptSubmit hook: checking go-workflow input policy
|
||||
|
||||
UserPromptSubmit hook (stopped)
|
||||
warning: go-workflow must start from PlanMode
|
||||
stop: prompt blocked
|
||||
@@ -1,127 +0,0 @@
|
||||
# `/loop` Backed by Thread Jobs
|
||||
|
||||
## Summary
|
||||
|
||||
Implement `/loop` as a TUI command that creates and manages per-thread jobs.
|
||||
|
||||
Defaults locked in:
|
||||
|
||||
- `/loop <spec>` creates a job.
|
||||
- Bare `/loop` lists jobs for the current thread and offers delete actions.
|
||||
- The entire feature is gated behind the experimental feature flag `job_scheduler`.
|
||||
- Natural-language parsing happens in the caller, not in core or app-server.
|
||||
- `JobCreate` accepts structured inputs: `cron_expression`, `prompt`, and optional `run_once`.
|
||||
- `thread/job/create` also accepts the optional `run_once` field, exposed on the v2 wire as `runOnce`.
|
||||
- Each thread has a hard limit of 256 active jobs to prevent resource exhaustion.
|
||||
- Jobs are runtime-only and do not persist across restart, resume, or fork.
|
||||
- Conditions like “until tests pass” stay inside `prompt`; the model stops the job by calling `JobDelete`.
|
||||
|
||||
## Key Changes
|
||||
|
||||
- Add a job scheduler module in core, preferably `codex-rs/core/src/jobs.rs`, to own:
|
||||
- `Job { id, cron_expression, prompt, run_once, created_at, next_run_at, last_run_at, pending_run }`
|
||||
- due-run tracking, idle-thread dispatch, and `next_run_at` computation
|
||||
- Gate all core, app-server, and TUI job functionality behind `Feature::JobScheduler` / the experimental feature key `job_scheduler`.
|
||||
- Add three built-in tools in core:
|
||||
- `JobCreate { cron_expression: String, prompt: String, run_once: Option<bool> }`
|
||||
- `JobDelete { id: String }`
|
||||
- `JobList {}`
|
||||
- Tool semantics:
|
||||
- `JobCreate` validates the structured job and registers it on the current thread.
|
||||
- `JobCreate` rejects creation when the thread already has 256 active jobs.
|
||||
- `JobDelete` removes one job by id on the current thread.
|
||||
- `JobList` returns all jobs on the current thread.
|
||||
- `run_once=true` means the job is deleted immediately after its execution is claimed, removed from `JobList` results, and the thread emits `thread/job/updated` with the new list.
|
||||
- Job execution behavior in core:
|
||||
- due jobs run only when the owning thread is idle
|
||||
- if a job becomes due while busy, keep at most one pending firing
|
||||
- manual user turns and queued user prompts beat job runs
|
||||
- job-triggered turns inherit the current thread context, including plan mode, sandbox, approvals, and model settings
|
||||
- interrupts stop the current turn but do not implicitly delete jobs
|
||||
- Job-triggered prompt execution stays off-transcript as a normal user-authored message. Clients should show explicit runtime/info events instead.
|
||||
- Add app-server v2 RPCs in `codex-rs/app-server-protocol/src/protocol/v2.rs`:
|
||||
- `thread/job/create { threadId, cronExpression, prompt, runOnce? }`
|
||||
- `thread/job/delete { threadId, id }`
|
||||
- `thread/job/list { threadId }`
|
||||
- `thread/job/updated` notification with `threadId` and the full current list
|
||||
- when `job_scheduler` is disabled, job RPCs fail with a clear unsupported-feature error instead of silently no-oping
|
||||
- `thread/job/create` fails with a clear invalid-request or limit-exceeded error when the thread already has 256 active jobs
|
||||
|
||||
## Natural-Language Parsing
|
||||
|
||||
- The caller converts `/loop <spec>` into structured job params by issuing a small model parse request before calling `thread/job/create`.
|
||||
- Clients should use the existing app-server side-prompt path for that parse request rather than inventing a separate parser-specific backend surface.
|
||||
- Define a shared parse contract for clients:
|
||||
- input: raw `spec: String`
|
||||
- output: `{ cronExpression: String, prompt: String, runOnce?: bool }`
|
||||
- Parsing prompt should be minimal and deterministic:
|
||||
- extract only schedule timing into `cronExpression`
|
||||
- keep all stop conditions and task intent inside `prompt`
|
||||
- set `runOnce=true` only when the spec clearly describes a one-shot run
|
||||
- if no explicit wall-clock schedule is present, return the special immediate-follow-up expression used by the scheduler
|
||||
- return structured JSON only, no prose
|
||||
- TUI uses this parse step internally for `/loop <spec>`.
|
||||
- Other clients can reuse the same prompt/contract, but core and app-server only validate the structured fields they receive.
|
||||
- Parse failures or invalid structured output fail closed: do not create a job.
|
||||
|
||||
## Job Identity And Subagents
|
||||
|
||||
- Every job-triggered turn carries `currentJobId` in hidden turn context. The model uses that id when it decides the job should stop and calls `JobDelete(currentJobId)`.
|
||||
- `currentJobId` is present only for turns fired by a job on that same thread. It must not be written into visible prompt text.
|
||||
- Jobs are strictly thread-local:
|
||||
- parent and child threads have separate job lists
|
||||
- spawning, resuming, or forking a child does not copy the parent’s jobs
|
||||
- a child spawned from a parent job turn does not inherit the parent’s `currentJobId`
|
||||
- Built-in job tools are available on any thread, including subagents, but always operate on that thread’s own jobs only. No cross-thread job mutation is allowed in v1.
|
||||
- If a parent job spawns a child and the child concludes the parent’s stop condition is met, the child must report that back to the parent thread; it cannot directly delete the parent’s job.
|
||||
- Closing a child thread implicitly drops that child’s runtime jobs with the thread.
|
||||
- Multi-agent clients must treat `thread/job/updated` and job-fire events as thread-scoped so background child-thread jobs are not mistaken for parent-thread activity.
|
||||
|
||||
## TUI Behavior
|
||||
|
||||
- Add `SlashCommand::Loop` in `codex-rs/tui/src/slash_command.rs`.
|
||||
- `/loop <spec>`:
|
||||
- runs the parse prompt
|
||||
- maps the structured result into `thread/job/create`
|
||||
- shows a confirmation/info event with the normalized job
|
||||
- Bare `/loop` lists current jobs for the active thread and supports deletion.
|
||||
- While a turn is running, `/loop <spec>` still works so the next job can be armed immediately.
|
||||
- In multi-agent views, job events and lists should remain attached to the owning thread so users can tell which agent armed or fired a job.
|
||||
- TUI only shows `/loop` when `job_scheduler` is enabled for the current session/config.
|
||||
|
||||
## Test Plan
|
||||
|
||||
- Core:
|
||||
- create, list, and delete multiple jobs on one thread
|
||||
- reject the 257th active job on a thread
|
||||
- validate `cron_expression` and `run_once`
|
||||
- ensure due jobs fire once when idle and do not backlog endlessly while busy
|
||||
- ensure a `run_once` job is deleted after execution and disappears from subsequent list results
|
||||
- ensure job-triggered turns include hidden `currentJobId`
|
||||
- ensure non-job turns do not include `currentJobId`
|
||||
- ensure parent `currentJobId` is not inherited by spawned child threads
|
||||
- ensure child-thread jobs are isolated from parent-thread jobs
|
||||
- ensure closing a child thread drops its runtime jobs
|
||||
- ensure restart, resume, and fork clear runtime jobs
|
||||
- App-server:
|
||||
- create/list/delete RPC round-trips, including omitted and explicit `runOnce`
|
||||
- `thread/job/create` returns the expected error once the thread reaches 256 active jobs
|
||||
- `thread/job/updated` emits the full current job set after creates, deletes, and run-once deletion after execution
|
||||
- thread-scoped notifications stay associated with the correct parent or child thread
|
||||
- disabled-feature behavior rejects job RPCs cleanly
|
||||
- TUI:
|
||||
- `/loop <spec>` issues the parse request and converts the structured result into the expected create RPC
|
||||
- parser snapshots cover recurring, immediate-follow-up, and one-shot examples
|
||||
- bare `/loop` lists jobs for the active thread only
|
||||
- delete actions call the delete RPC on the active thread
|
||||
- job-fire info events render with the owning thread identity in multi-agent scenarios
|
||||
- `/loop` is hidden or rejected when `job_scheduler` is disabled
|
||||
|
||||
## Assumptions
|
||||
|
||||
- `job` is the public product and API term; the underlying module/file can still be named `scheduled.rs` if that keeps implementation boundaries cleaner.
|
||||
- Core and app-server do not parse freeform schedule text.
|
||||
- The parser uses a model call rather than handwritten NLP rules.
|
||||
- The scheduler supports a special non-cron expression for immediate post-turn execution, because plain cron is a bad fit for “run again right after this turn.”
|
||||
- Cron expressions are evaluated in the local timezone of the process hosting the thread unless the existing scheduler infrastructure already defines a better thread-level timezone source.
|
||||
- The experimental gate is named exactly `job_scheduler`.
|
||||
Reference in New Issue
Block a user