mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Add goal model tools (3 / 5) (#18075)
Adds the model-facing goal tools on top of the app-server API from PR 2. ## Why Once goals are persisted and exposed to clients, the model needs a small, constrained tool surface for goal workflows. The tool contract should let the model inspect goals, create them only when explicitly requested, and mark them complete without giving it broad control over user/runtime-owned state. ## What changed - Added `get_goal`, `create_goal`, and `update_goal` tool specs behind the `goals` feature flag. - Added core goal tool handlers that validate objectives and token budgets before mutating persisted state. - Constrained `create_goal` to create only when no goal exists, with optional `token_budget` only when a budget is explicitly provided. - Tightened the `create_goal` instructions so the model does not infer goals from ordinary task requests. - Constrained `update_goal` to expose only goal completion; pause, resume, clear, and budget-limited transitions remain user- or runtime-controlled. - Registered the goal tools in the tool registry and kept them out of review contexts where they should not appear. ## Verification - Added tool-registry coverage for feature gating and tool availability. - Added core session tests for create/get/update behavior, duplicate goal rejection, budget validation, and completion-only updates.
This commit is contained in:
committed by
GitHub
Unverified
parent
6c874f9b34
commit
32ace07ac5
@@ -24,6 +24,7 @@ pub(super) async fn spawn_review_thread(
|
||||
let _ = review_features.disable(Feature::WebSearchRequest);
|
||||
let _ = review_features.disable(Feature::WebSearchCached);
|
||||
let review_web_search_mode = WebSearchMode::Disabled;
|
||||
let goal_tools_supported = !config.ephemeral && parent_turn_context.tools_config.goal_tools;
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &review_model_info,
|
||||
available_models: &sess
|
||||
@@ -51,6 +52,7 @@ pub(super) async fn spawn_review_thread(
|
||||
.with_spawn_agent_usage_hint(config.multi_agent_v2.usage_hint_enabled)
|
||||
.with_spawn_agent_usage_hint_text(config.multi_agent_v2.usage_hint_text.clone())
|
||||
.with_hide_spawn_agent_metadata(config.multi_agent_v2.hide_spawn_agent_metadata)
|
||||
.with_goal_tools_allowed(goal_tools_supported)
|
||||
.with_max_concurrent_threads_per_session(config.agent_max_threads)
|
||||
.with_agent_type_description(crate::agent::role::spawn_tool_spec::build(
|
||||
&config.agent_roles,
|
||||
|
||||
@@ -57,6 +57,7 @@ use crate::tasks::execute_user_shell_command;
|
||||
use crate::tools::ToolRouter;
|
||||
use crate::tools::context::ToolInvocation;
|
||||
use crate::tools::context::ToolPayload;
|
||||
use crate::tools::handlers::GoalHandler;
|
||||
use crate::tools::handlers::ShellHandler;
|
||||
use crate::tools::handlers::UnifiedExecHandler;
|
||||
use crate::tools::registry::ToolHandler;
|
||||
@@ -101,6 +102,7 @@ use codex_protocol::protocol::ResumedHistory;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_protocol::protocol::Submission;
|
||||
use codex_protocol::protocol::ThreadGoalStatus;
|
||||
use codex_protocol::protocol::ThreadRolledBackEvent;
|
||||
use codex_protocol::protocol::TokenCountEvent;
|
||||
use codex_protocol::protocol::TokenUsage;
|
||||
@@ -3348,6 +3350,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
session_configuration.cwd.clone(),
|
||||
"turn_id".to_string(),
|
||||
skills_outcome,
|
||||
/*goal_tools_supported*/ true,
|
||||
);
|
||||
|
||||
let (mailbox, mailbox_rx) = crate::agent::Mailbox::new();
|
||||
@@ -4703,6 +4706,7 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx(
|
||||
session_configuration.cwd.clone(),
|
||||
"turn_id".to_string(),
|
||||
skills_outcome,
|
||||
/*goal_tools_supported*/ true,
|
||||
));
|
||||
|
||||
let (mailbox, mailbox_rx) = crate::agent::Mailbox::new();
|
||||
@@ -6852,6 +6856,222 @@ async fn sample_rollout(
|
||||
)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn create_goal_tool_rejects_existing_goal() {
|
||||
let (mut session, turn_context) = make_session_and_context().await;
|
||||
let _ = session.features.enable(Feature::Goals);
|
||||
let session = Arc::new(session);
|
||||
upsert_goal_tool_test_thread(session.as_ref()).await;
|
||||
let turn_context = Arc::new(turn_context);
|
||||
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
|
||||
let handler = GoalHandler;
|
||||
|
||||
handler
|
||||
.handle(ToolInvocation {
|
||||
session: Arc::clone(&session),
|
||||
turn: Arc::clone(&turn_context),
|
||||
cancellation_token: CancellationToken::new(),
|
||||
tracker: Arc::clone(&tracker),
|
||||
call_id: "create-goal-1".to_string(),
|
||||
tool_name: codex_tools::ToolName::plain("create_goal"),
|
||||
source: ToolCallSource::Direct,
|
||||
payload: ToolPayload::Function {
|
||||
arguments: serde_json::json!({
|
||||
"objective": "Keep the watcher alive",
|
||||
"token_budget": 123,
|
||||
})
|
||||
.to_string(),
|
||||
},
|
||||
})
|
||||
.await
|
||||
.expect("initial create_goal should succeed");
|
||||
|
||||
let response = handler
|
||||
.handle(ToolInvocation {
|
||||
session: Arc::clone(&session),
|
||||
turn: Arc::clone(&turn_context),
|
||||
cancellation_token: CancellationToken::new(),
|
||||
tracker,
|
||||
call_id: "create-goal-2".to_string(),
|
||||
tool_name: codex_tools::ToolName::plain("create_goal"),
|
||||
source: ToolCallSource::Direct,
|
||||
payload: ToolPayload::Function {
|
||||
arguments: serde_json::json!({
|
||||
"objective": "Replace the watcher",
|
||||
"token_budget": 456,
|
||||
})
|
||||
.to_string(),
|
||||
},
|
||||
})
|
||||
.await;
|
||||
|
||||
let Err(FunctionCallError::RespondToModel(output)) = response else {
|
||||
panic!("expected create_goal to reject an existing goal");
|
||||
};
|
||||
assert_eq!(
|
||||
output,
|
||||
"cannot create a new goal because this thread already has a goal; use update_goal only when the existing goal is complete"
|
||||
);
|
||||
|
||||
let goal = session
|
||||
.get_thread_goal()
|
||||
.await
|
||||
.expect("read thread goal")
|
||||
.expect("goal should still exist");
|
||||
assert_eq!(goal.objective, "Keep the watcher alive");
|
||||
assert_eq!(goal.token_budget, Some(123));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_goal_tool_rejects_pausing_goal() {
|
||||
let (mut session, turn_context) = make_session_and_context().await;
|
||||
let _ = session.features.enable(Feature::Goals);
|
||||
let session = Arc::new(session);
|
||||
upsert_goal_tool_test_thread(session.as_ref()).await;
|
||||
let turn_context = Arc::new(turn_context);
|
||||
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
|
||||
let handler = GoalHandler;
|
||||
|
||||
handler
|
||||
.handle(ToolInvocation {
|
||||
session: Arc::clone(&session),
|
||||
turn: Arc::clone(&turn_context),
|
||||
cancellation_token: CancellationToken::new(),
|
||||
tracker: Arc::clone(&tracker),
|
||||
call_id: "create-goal".to_string(),
|
||||
tool_name: codex_tools::ToolName::plain("create_goal"),
|
||||
source: ToolCallSource::Direct,
|
||||
payload: ToolPayload::Function {
|
||||
arguments: serde_json::json!({
|
||||
"objective": "Keep the watcher alive",
|
||||
"token_budget": 123,
|
||||
})
|
||||
.to_string(),
|
||||
},
|
||||
})
|
||||
.await
|
||||
.expect("initial create_goal should succeed");
|
||||
|
||||
let response = handler
|
||||
.handle(ToolInvocation {
|
||||
session: Arc::clone(&session),
|
||||
turn: Arc::clone(&turn_context),
|
||||
cancellation_token: CancellationToken::new(),
|
||||
tracker,
|
||||
call_id: "pause-goal".to_string(),
|
||||
tool_name: codex_tools::ToolName::plain("update_goal"),
|
||||
source: ToolCallSource::Direct,
|
||||
payload: ToolPayload::Function {
|
||||
arguments: serde_json::json!({
|
||||
"status": "paused",
|
||||
})
|
||||
.to_string(),
|
||||
},
|
||||
})
|
||||
.await;
|
||||
|
||||
let Err(FunctionCallError::RespondToModel(output)) = response else {
|
||||
panic!("expected update_goal to reject pausing a goal");
|
||||
};
|
||||
assert_eq!(
|
||||
output,
|
||||
"update_goal can only mark the existing goal complete; pause, resume, and budget-limited status changes are controlled by the user or system"
|
||||
);
|
||||
|
||||
let goal = session
|
||||
.get_thread_goal()
|
||||
.await
|
||||
.expect("read thread goal")
|
||||
.expect("goal should still exist");
|
||||
assert_eq!(goal.status, ThreadGoalStatus::Active);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_goal_tool_marks_goal_complete() {
|
||||
let (mut session, turn_context) = make_session_and_context().await;
|
||||
let _ = session.features.enable(Feature::Goals);
|
||||
let session = Arc::new(session);
|
||||
upsert_goal_tool_test_thread(session.as_ref()).await;
|
||||
let turn_context = Arc::new(turn_context);
|
||||
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
|
||||
let handler = GoalHandler;
|
||||
|
||||
handler
|
||||
.handle(ToolInvocation {
|
||||
session: Arc::clone(&session),
|
||||
turn: Arc::clone(&turn_context),
|
||||
cancellation_token: CancellationToken::new(),
|
||||
tracker: Arc::clone(&tracker),
|
||||
call_id: "create-goal".to_string(),
|
||||
tool_name: codex_tools::ToolName::plain("create_goal"),
|
||||
source: ToolCallSource::Direct,
|
||||
payload: ToolPayload::Function {
|
||||
arguments: serde_json::json!({
|
||||
"objective": "Keep the watcher alive",
|
||||
"token_budget": 123,
|
||||
})
|
||||
.to_string(),
|
||||
},
|
||||
})
|
||||
.await
|
||||
.expect("initial create_goal should succeed");
|
||||
|
||||
handler
|
||||
.handle(ToolInvocation {
|
||||
session: Arc::clone(&session),
|
||||
turn: Arc::clone(&turn_context),
|
||||
cancellation_token: CancellationToken::new(),
|
||||
tracker,
|
||||
call_id: "complete-goal".to_string(),
|
||||
tool_name: codex_tools::ToolName::plain("update_goal"),
|
||||
source: ToolCallSource::Direct,
|
||||
payload: ToolPayload::Function {
|
||||
arguments: serde_json::json!({
|
||||
"status": "complete",
|
||||
})
|
||||
.to_string(),
|
||||
},
|
||||
})
|
||||
.await
|
||||
.expect("update_goal should mark the goal complete");
|
||||
|
||||
let goal = session
|
||||
.get_thread_goal()
|
||||
.await
|
||||
.expect("read thread goal")
|
||||
.expect("goal should still exist");
|
||||
assert_eq!(goal.status, ThreadGoalStatus::Complete);
|
||||
}
|
||||
|
||||
async fn upsert_goal_tool_test_thread(session: &Session) {
|
||||
let config = session.get_config().await;
|
||||
let state_db = codex_state::StateRuntime::init(
|
||||
config.sqlite_home.clone(),
|
||||
config.model_provider_id.clone(),
|
||||
)
|
||||
.await
|
||||
.expect("state db should initialize");
|
||||
let mut builder = codex_state::ThreadMetadataBuilder::new(
|
||||
session.conversation_id,
|
||||
config
|
||||
.codex_home
|
||||
.join("goal-tool-test-rollout.jsonl")
|
||||
.to_path_buf(),
|
||||
chrono::Utc::now(),
|
||||
SessionSource::Exec,
|
||||
);
|
||||
builder.cwd = config.cwd.to_path_buf();
|
||||
builder.model_provider = Some(config.model_provider_id.clone());
|
||||
builder.cli_version = Some(env!("CARGO_PKG_VERSION").to_string());
|
||||
builder.sandbox_policy = config.permissions.sandbox_policy.get().clone();
|
||||
builder.approval_mode = config.permissions.approval_policy.value();
|
||||
let metadata = builder.build(config.model_provider_id.as_str());
|
||||
state_db
|
||||
.upsert_thread(&metadata)
|
||||
.await
|
||||
.expect("thread metadata should be upserted");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn rejects_escalated_permissions_when_policy_not_on_request() {
|
||||
use crate::exec::ExecParams;
|
||||
|
||||
@@ -180,6 +180,7 @@ impl TurnContext {
|
||||
.with_spawn_agent_usage_hint(config.multi_agent_v2.usage_hint_enabled)
|
||||
.with_spawn_agent_usage_hint_text(config.multi_agent_v2.usage_hint_text.clone())
|
||||
.with_hide_spawn_agent_metadata(config.multi_agent_v2.hide_spawn_agent_metadata)
|
||||
.with_goal_tools_allowed(self.tools_config.goal_tools)
|
||||
.with_max_concurrent_threads_per_session(config.agent_max_threads)
|
||||
.with_agent_type_description(crate::agent::role::spawn_tool_spec::build(
|
||||
&config.agent_roles,
|
||||
@@ -405,6 +406,7 @@ impl Session {
|
||||
cwd: AbsolutePathBuf,
|
||||
sub_id: String,
|
||||
skills_outcome: Arc<SkillLoadOutcome>,
|
||||
goal_tools_supported: bool,
|
||||
) -> TurnContext {
|
||||
let reasoning_effort = session_configuration.collaboration_mode.reasoning_effort();
|
||||
let reasoning_summary = session_configuration
|
||||
@@ -441,6 +443,7 @@ impl Session {
|
||||
.with_spawn_agent_usage_hint(per_turn_config.multi_agent_v2.usage_hint_enabled)
|
||||
.with_spawn_agent_usage_hint_text(per_turn_config.multi_agent_v2.usage_hint_text.clone())
|
||||
.with_hide_spawn_agent_metadata(per_turn_config.multi_agent_v2.hide_spawn_agent_metadata)
|
||||
.with_goal_tools_allowed(goal_tools_supported)
|
||||
.with_max_concurrent_threads_per_session(per_turn_config.agent_max_threads)
|
||||
.with_agent_type_description(crate::agent::role::spawn_tool_spec::build(
|
||||
&per_turn_config.agent_roles,
|
||||
@@ -653,6 +656,7 @@ impl Session {
|
||||
.skills_for_config(&skills_input, fs)
|
||||
.await,
|
||||
);
|
||||
let goal_tools_supported = !per_turn_config.ephemeral && self.state_db().is_some();
|
||||
let mut turn_context: TurnContext = Self::make_turn_context(
|
||||
self.conversation_id,
|
||||
Some(Arc::clone(&self.services.auth_manager)),
|
||||
@@ -679,6 +683,7 @@ impl Session {
|
||||
cwd,
|
||||
sub_id,
|
||||
skills_outcome,
|
||||
goal_tools_supported,
|
||||
);
|
||||
turn_context.realtime_active = self.conversation.running_state().await.is_some();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user