mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Fix goal update and add /goal edit command in TUI (#21954)
## Why Users have requested the ability to edit a goal's objective after a goal has been created. This PR exposes a new `/goal edit` command in the TUI to address this request. In the process of implementing this, I also noticed an existing bug in the goal runtime. When a goal's objective is updated through the `thread/goal/set` app server API, the goal runtime didn't emit a new steering prompt to tell the agent about the new objective. This PR also fixes this hole. ## What Changed - Adds `/goal edit` in the TUI, opening an edit box prefilled with the current goal objective. - Keeps active and paused goals in their current state, resets completed goals to active, keeps budget-limited goals budget-limited, and preserves the existing token budget. - Changes the existing `thread/goal/set` behavior so editing an objective preserves goal accounting instead of resetting it. The older reset-on-new-objective behavior was left over from before `thread/goal/clear`; clients that need to reset accounting can now clear the existing goal and create a new one. - Reuses the existing goal set API path; this does not add or change app-server protocol surface area. - Adds a dedicated goal runtime steering prompt when an externally persisted goal mutation changes the objective, so active turns receive the updated objective. ## Validation - Make sure `/goal edit` returns an error if no goal currently exists - Make sure `/goal edit` displays an edit box that can be optionally canceled with no side effects - Make sure that an edited goal results in a steer so the agent starts pursuing the new objective - Make sure the new objective is reflected in the goal if you use `/goal` to display the goal summary - Make sure that `/goal edit` doesn't reset the token budget, time/token accounting on the updated goal
This commit is contained in:
committed by
GitHub
Unverified
parent
32b1ae7099
commit
1e65b3e0af
@@ -2,6 +2,7 @@ use super::*;
|
||||
use uuid::Uuid;
|
||||
|
||||
pub struct ThreadGoalUpdate {
|
||||
pub objective: Option<String>,
|
||||
pub status: Option<crate::ThreadGoalStatus>,
|
||||
pub token_budget: Option<Option<i64>>,
|
||||
pub expected_goal_id: Option<String>,
|
||||
@@ -166,10 +167,12 @@ RETURNING
|
||||
update: ThreadGoalUpdate,
|
||||
) -> anyhow::Result<Option<crate::ThreadGoal>> {
|
||||
let ThreadGoalUpdate {
|
||||
objective,
|
||||
status,
|
||||
token_budget,
|
||||
expected_goal_id,
|
||||
} = update;
|
||||
let objective = objective.as_deref();
|
||||
let expected_goal_id = expected_goal_id.as_deref();
|
||||
let now_ms = datetime_to_epoch_millis(Utc::now());
|
||||
let result = match (status, token_budget) {
|
||||
@@ -178,6 +181,7 @@ RETURNING
|
||||
r#"
|
||||
UPDATE thread_goals
|
||||
SET
|
||||
objective = COALESCE(?, objective),
|
||||
status = CASE
|
||||
WHEN status = ? AND ? = ? THEN status
|
||||
WHEN ? = 'active' AND ? IS NOT NULL AND tokens_used >= ? THEN ?
|
||||
@@ -189,6 +193,7 @@ WHERE thread_id = ?
|
||||
AND (? IS NULL OR goal_id = ?)
|
||||
"#,
|
||||
)
|
||||
.bind(objective)
|
||||
.bind(crate::ThreadGoalStatus::BudgetLimited.as_str())
|
||||
.bind(status.as_str())
|
||||
.bind(crate::ThreadGoalStatus::Paused.as_str())
|
||||
@@ -210,6 +215,7 @@ WHERE thread_id = ?
|
||||
r#"
|
||||
UPDATE thread_goals
|
||||
SET
|
||||
objective = COALESCE(?, objective),
|
||||
status = CASE
|
||||
WHEN status = ? AND ? = ? THEN status
|
||||
WHEN ? = 'active' AND token_budget IS NOT NULL AND tokens_used >= token_budget THEN ?
|
||||
@@ -220,6 +226,7 @@ WHERE thread_id = ?
|
||||
AND (? IS NULL OR goal_id = ?)
|
||||
"#,
|
||||
)
|
||||
.bind(objective)
|
||||
.bind(crate::ThreadGoalStatus::BudgetLimited.as_str())
|
||||
.bind(status.as_str())
|
||||
.bind(crate::ThreadGoalStatus::Paused.as_str())
|
||||
@@ -238,6 +245,7 @@ WHERE thread_id = ?
|
||||
r#"
|
||||
UPDATE thread_goals
|
||||
SET
|
||||
objective = COALESCE(?, objective),
|
||||
token_budget = ?,
|
||||
status = CASE
|
||||
WHEN status = 'active' AND ? IS NOT NULL AND tokens_used >= ? THEN ?
|
||||
@@ -248,6 +256,7 @@ WHERE thread_id = ?
|
||||
AND (? IS NULL OR goal_id = ?)
|
||||
"#,
|
||||
)
|
||||
.bind(objective)
|
||||
.bind(token_budget)
|
||||
.bind(token_budget)
|
||||
.bind(token_budget)
|
||||
@@ -260,13 +269,35 @@ WHERE thread_id = ?
|
||||
.await?
|
||||
}
|
||||
(None, None) => {
|
||||
let goal = self.get_thread_goal(thread_id).await?;
|
||||
return Ok(match (goal, expected_goal_id) {
|
||||
(Some(goal), Some(expected_goal_id)) if goal.goal_id != expected_goal_id => {
|
||||
None
|
||||
}
|
||||
(goal, _) => goal,
|
||||
});
|
||||
if let Some(objective) = objective {
|
||||
sqlx::query(
|
||||
r#"
|
||||
UPDATE thread_goals
|
||||
SET
|
||||
objective = ?,
|
||||
updated_at_ms = ?
|
||||
WHERE thread_id = ?
|
||||
AND (? IS NULL OR goal_id = ?)
|
||||
"#,
|
||||
)
|
||||
.bind(objective)
|
||||
.bind(now_ms)
|
||||
.bind(thread_id.to_string())
|
||||
.bind(expected_goal_id)
|
||||
.bind(expected_goal_id)
|
||||
.execute(self.pool.as_ref())
|
||||
.await?
|
||||
} else {
|
||||
let goal = self.get_thread_goal(thread_id).await?;
|
||||
return Ok(match (goal, expected_goal_id) {
|
||||
(Some(goal), Some(expected_goal_id))
|
||||
if goal.goal_id != expected_goal_id =>
|
||||
{
|
||||
None
|
||||
}
|
||||
(goal, _) => goal,
|
||||
});
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
@@ -511,6 +542,7 @@ mod tests {
|
||||
.update_thread_goal(
|
||||
thread_id,
|
||||
ThreadGoalUpdate {
|
||||
objective: None,
|
||||
status: Some(crate::ThreadGoalStatus::Paused),
|
||||
token_budget: Some(Some(200_000)),
|
||||
expected_goal_id: None,
|
||||
@@ -690,6 +722,7 @@ mod tests {
|
||||
.update_thread_goal(
|
||||
thread_id,
|
||||
ThreadGoalUpdate {
|
||||
objective: None,
|
||||
status: Some(crate::ThreadGoalStatus::Complete),
|
||||
token_budget: None,
|
||||
expected_goal_id: Some(original.goal_id),
|
||||
@@ -711,6 +744,7 @@ mod tests {
|
||||
.update_thread_goal(
|
||||
thread_id,
|
||||
ThreadGoalUpdate {
|
||||
objective: None,
|
||||
status: Some(crate::ThreadGoalStatus::Complete),
|
||||
token_budget: None,
|
||||
expected_goal_id: Some(replacement.goal_id),
|
||||
@@ -768,6 +802,58 @@ mod tests {
|
||||
assert_eq!(0, goal.time_used_seconds);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_thread_goal_objective_preserves_usage_and_created_at() {
|
||||
let runtime = test_runtime().await;
|
||||
let thread_id = test_thread_id();
|
||||
upsert_test_thread(&runtime, thread_id).await;
|
||||
|
||||
runtime
|
||||
.replace_thread_goal(
|
||||
thread_id,
|
||||
"draft the report",
|
||||
crate::ThreadGoalStatus::Active,
|
||||
/*token_budget*/ Some(100),
|
||||
)
|
||||
.await
|
||||
.expect("goal replacement should succeed");
|
||||
let outcome = runtime
|
||||
.account_thread_goal_usage(
|
||||
thread_id,
|
||||
/*time_delta_seconds*/ 12,
|
||||
/*token_delta*/ 30,
|
||||
ThreadGoalAccountingMode::ActiveOnly,
|
||||
/*expected_goal_id*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("usage accounting should succeed");
|
||||
let ThreadGoalAccountingOutcome::Updated(accounted) = outcome else {
|
||||
panic!("active goal should account usage");
|
||||
};
|
||||
|
||||
let updated = runtime
|
||||
.update_thread_goal(
|
||||
thread_id,
|
||||
ThreadGoalUpdate {
|
||||
objective: Some("draft the report clearly".to_string()),
|
||||
status: Some(crate::ThreadGoalStatus::Paused),
|
||||
token_budget: Some(Some(200)),
|
||||
expected_goal_id: Some(accounted.goal_id.clone()),
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("goal update should succeed")
|
||||
.expect("goal should exist");
|
||||
let expected = crate::ThreadGoal {
|
||||
objective: "draft the report clearly".to_string(),
|
||||
status: crate::ThreadGoalStatus::Paused,
|
||||
token_budget: Some(200),
|
||||
updated_at: updated.updated_at,
|
||||
..accounted
|
||||
};
|
||||
assert_eq!(expected, updated);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn concurrent_partial_updates_preserve_independent_fields() {
|
||||
let runtime = test_runtime().await;
|
||||
@@ -786,6 +872,7 @@ mod tests {
|
||||
let status_update = runtime.update_thread_goal(
|
||||
thread_id,
|
||||
ThreadGoalUpdate {
|
||||
objective: None,
|
||||
status: Some(crate::ThreadGoalStatus::Paused),
|
||||
token_budget: None,
|
||||
expected_goal_id: None,
|
||||
@@ -794,6 +881,7 @@ mod tests {
|
||||
let budget_update = runtime.update_thread_goal(
|
||||
thread_id,
|
||||
ThreadGoalUpdate {
|
||||
objective: None,
|
||||
status: None,
|
||||
token_budget: Some(Some(200_000)),
|
||||
expected_goal_id: None,
|
||||
@@ -843,6 +931,7 @@ mod tests {
|
||||
.update_thread_goal(
|
||||
thread_id,
|
||||
ThreadGoalUpdate {
|
||||
objective: None,
|
||||
status: Some(crate::ThreadGoalStatus::Complete),
|
||||
token_budget: None,
|
||||
expected_goal_id: None,
|
||||
@@ -983,6 +1072,7 @@ mod tests {
|
||||
.update_thread_goal(
|
||||
thread_id,
|
||||
crate::ThreadGoalUpdate {
|
||||
objective: None,
|
||||
status: Some(crate::ThreadGoalStatus::Paused),
|
||||
token_budget: None,
|
||||
expected_goal_id: None,
|
||||
@@ -1038,6 +1128,7 @@ mod tests {
|
||||
.update_thread_goal(
|
||||
thread_id,
|
||||
ThreadGoalUpdate {
|
||||
objective: None,
|
||||
status: None,
|
||||
token_budget: Some(Some(40)),
|
||||
expected_goal_id: None,
|
||||
@@ -1081,6 +1172,7 @@ mod tests {
|
||||
.update_thread_goal(
|
||||
thread_id,
|
||||
ThreadGoalUpdate {
|
||||
objective: Some("stay within budget, with clearer wording".to_string()),
|
||||
status: Some(crate::ThreadGoalStatus::Active),
|
||||
token_budget: None,
|
||||
expected_goal_id: None,
|
||||
@@ -1091,6 +1183,10 @@ mod tests {
|
||||
.expect("goal should exist");
|
||||
|
||||
assert_eq!(crate::ThreadGoalStatus::BudgetLimited, reactivated.status);
|
||||
assert_eq!(
|
||||
"stay within budget, with clearer wording",
|
||||
reactivated.objective
|
||||
);
|
||||
assert_eq!(Some(40), reactivated.token_budget);
|
||||
assert_eq!(50, reactivated.tokens_used);
|
||||
}
|
||||
@@ -1124,6 +1220,7 @@ mod tests {
|
||||
.update_thread_goal(
|
||||
thread_id,
|
||||
ThreadGoalUpdate {
|
||||
objective: None,
|
||||
status: Some(crate::ThreadGoalStatus::Paused),
|
||||
token_budget: None,
|
||||
expected_goal_id: None,
|
||||
@@ -1206,6 +1303,7 @@ mod tests {
|
||||
.update_thread_goal(
|
||||
thread_id,
|
||||
ThreadGoalUpdate {
|
||||
objective: None,
|
||||
status: Some(crate::ThreadGoalStatus::Paused),
|
||||
token_budget: None,
|
||||
expected_goal_id: None,
|
||||
|
||||
Reference in New Issue
Block a user