feat(soul): add /goal slash command with stateful goal management#2276
feat(soul): add /goal slash command with stateful goal management#2276CommanderCrowCode wants to merge 1 commit into
Conversation
f6dd119 to
50d5b18
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50d5b1815d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| user_message = Message(role="user", content=prompt) | ||
|
|
||
| # Run the ralph loop with a single iteration (decision: continue or stop) | ||
| runner = FlowRunner.ralph_loop(user_message, max_ralph_iterations=1) |
There was a problem hiding this comment.
Let /goal continuation run beyond first CONTINUE
Using FlowRunner.ralph_loop(..., max_ralph_iterations=1) makes max_moves equal to 2, which only covers the first task node and first decision node. If the model follows the decision prompt and returns CONTINUE (the expected path for unfinished work), the next loop iteration immediately raises MaxStepsReached, so long-running goals abort after a single cycle instead of continuing automatically.
Useful? React with 👍 / 👎.
| if goal is None: | ||
| return | ||
| goal.tokens_used += tokens | ||
| goal.updated_at = time.time() |
There was a problem hiding this comment.
Restrict goal token accounting to active goals
_update_goal_tokens claims to update the “active goal” but only checks for goal is None; paused or completed goals still accumulate tokens on every LLM step. This makes /goal pause ineffective for budget tracking and can mutate post-completion metrics with unrelated later turns.
Useful? React with 👍 / 👎.
50d5b18 to
6678e4f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6678e4fa8e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| from kimi_cli.soul.slash import _trigger_goal_continuation | ||
|
|
||
| logger.info("Auto-continuing active goal: {objective}", objective=goal.objective[:50]) | ||
| await _trigger_goal_continuation(self.soul) |
There was a problem hiding this comment.
Run goal continuation through run_soul to preserve wire context
_maybe_continue_goal calls _trigger_goal_continuation directly after run_soul_command has already finished, which means the run_soul wire context has been torn down before the continuation starts. The continuation path immediately emits wire_send(TurnBegin(...)), so this can raise the wire is not None assertion and crash the shell whenever an active goal tries to auto-continue after a normal user turn.
Useful? React with 👍 / 👎.
| goal = self._runtime.session.state.goal | ||
| if goal is not None: | ||
| goal.time_used_seconds = self._goal_time_elapsed() | ||
| self._runtime.session.save_state() |
There was a problem hiding this comment.
Freeze goal elapsed time once goal stops being active
This post-turn save block recalculates time_used_seconds for every non-None goal, including paused and complete states that remain stored in session state. As a result, unrelated later turns keep increasing the recorded duration, so final goal timing is no longer trustworthy (including data used by GoalEnd/status reporting). Restrict elapsed-time updates to active goals or snapshot it at terminal states.
Useful? React with 👍 / 👎.
6678e4f to
5f3fe0b
Compare
Reviewer Notes — Codex Review Feedback AddressedThis PR has been reviewed by Codex (OpenAI). Below is a summary of the 4 issues raised and how they were resolved in the latest commit. 1. ✅ Let /goal continuation run beyond first CONTINUEIssue: Fix: Changed to
The loop now continues until the model explicitly chooses 2. ✅ Restrict goal token accounting to active goalsIssue: Fix: Added
3. ✅ Run goal continuation through run_soul to preserve wire contextIssue: Fix: Changed
4. ✅ Freeze goal elapsed time once goal stops being activeIssue: The post-turn save block recalculated Fix: Restricted elapsed-time updates to active goals in
All 806 core tests pass. Ready for maintainer review. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f3fe0bc63
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| runner = FlowRunner.ralph_loop(user_message, max_ralph_iterations=-1) | ||
| await runner.run(soul, "") |
There was a problem hiding this comment.
Stop goal continuation when status leaves active
/goal and /goal resume start an effectively unbounded Ralph loop (max_ralph_iterations=-1), but goal status is checked only once before the loop starts. If the goal becomes budget_limited (via token accounting) or complete (via UpdateGoal) during any iteration, this runner keeps executing additional turns until the model explicitly returns STOP, which can continue spending tokens after the goal has already reached a terminal state.
Useful? React with 👍 / 👎.
| if goal is not None and goal.status == "active": | ||
| goal.time_used_seconds = self._goal_time_elapsed() | ||
| self._runtime.session.save_state() |
There was a problem hiding this comment.
Persist goal state even after terminal status transitions
Goal persistence is gated on goal.status == "active", so when _update_goal_tokens flips a goal to budget_limited during a turn, the updated status and latest token usage are never saved at turn end. This leaves state.json stale (often still active), so reopening the session can resume with incorrect goal status and counters.
Useful? React with 👍 / 👎.
5f3fe0b to
060083d
Compare
Round 2 — Codex Review Feedback Addressed (commit
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 060083d336
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if current_goal.status == "active": | ||
| wire_send(TextPart(text="Goal is already active.")) | ||
| return | ||
| current_goal.status = "active" | ||
| current_goal.updated_at = time.time() |
There was a problem hiding this comment.
Restrict /goal resume to paused goals
/goal resume currently reactivates any non-active goal, not just paused ones. If a goal is already complete or budget_limited, this branch resets it to active and immediately calls continuation, which can restart work and consume additional tokens after a terminal state was reached (including budget-stop scenarios when token_budget is set). This violates the command contract in the docstring and undermines terminal-state enforcement.
Useful? React with 👍 / 👎.
| if goal is None or not goal.created_at: | ||
| return 0.0 | ||
| return time.time() - goal.created_at |
There was a problem hiding this comment.
Exclude paused intervals from goal time accounting
time_used_seconds is derived from time.time() - created_at, so after a goal is paused and later resumed, the next active-turn update includes the entire paused wall-clock interval. That inflates “time spent pursuing goal” in status/prompt reporting and any downstream metrics, even though no work happened during the pause. Track active duration incrementally (or adjust on pause/resume) instead of using raw creation-to-now elapsed time.
Useful? React with 👍 / 👎.
…onshotAI#2218) Implement feature-parity /goal command inspired by Codex's goal system. Key features: - Goal state persistence in SessionState (objective, status, token budget, usage) - Subcommands: /goal <obj>, /goal pause, /goal resume, /goal clear, /goal edit - Enhanced continuation prompt with completion audit and fidelity instructions - UpdateGoal tool for the model to mark goals complete - Token budget tracking with budget_limited status - Auto-continuation on idle (triggers after user turns when goal is active) - Wire events: GoalBegin, GoalEnd, GoalStatusUpdate - Telemetry: goal_created, goal_completed, goal_budget_limited Closes MoonshotAI#2218
060083d to
3aa0dba
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3aa0dbadfc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if current_goal.status == "paused": | ||
| wire_send(TextPart(text="Goal is already paused.")) | ||
| return | ||
| current_goal.status = "paused" |
There was a problem hiding this comment.
Reject pausing goals that are no longer active
/goal pause is specified as “Pause the active goal,” but this branch only rejects None and already-paused, then unconditionally sets status = "paused". That lets terminal goals (complete/budget_limited) be rewritten into a resumable state, which breaks terminal-state enforcement and can allow additional automated work/token spend after a goal should have ended.
Useful? React with 👍 / 👎.
| if goal.status == "active": | ||
| goal.time_used_seconds = self._goal_time_elapsed() |
There was a problem hiding this comment.
Persist final elapsed time when goal leaves active state
Goal elapsed time is only written when goal.status == "active" at turn end, so if a goal becomes complete (via UpdateGoal) or budget_limited during the turn, time_used_seconds is never updated with that final interval. In the common case where a goal finishes on its first turn, persisted duration can remain 0.0, which makes stored goal timing inaccurate for later status/reporting.
Useful? React with 👍 / 👎.
Closes #2218
Implement feature-parity /goal command inspired by Codex's goal system.
Features
/goal <objective>— create or replace the active goal/goal— show current goal status/goal pause— pause the active goal/goal resume— resume a paused goal/goal clear— clear the active goal/goal edit— edit the objective in an external editorFiles changed
Testing
All 806 core tests pass.