feat: /goal goal-driven execution, /best-practices, and Codex agentic-loop adoption - #117
Merged
Conversation
/goal sets a persistent thread goal (GoalState in session state) that kicks off work with a success-criteria derivation prompt and is re-injected on later turns as a continuation reminder carrying Codex's fidelity rules and evidence-based completion audit (ported from codex-rs prompts/templates/goals/). Objectives are framed as untrusted data in <objective> tags. Subcommands: view, pause, resume, clear. GoalModeInjectionProvider mirrors the plan-mode throttled full/sparse cadence, announces goal changes immediately, skips subagents and paused goals, and re-fires the full reminder after compaction. /best-practices (alias /bp) injects opt-in engineering guidance distilled from the Codex system prompts — code-change discipline, dirty-worktree safety, specific-to-broad testing, todo hygiene, progress-update cadence, debugging methodology, and final-answer style — without consuming a turn; an optional argument selects a single section.
Port the Codex plan-tool contract (plan_spec.rs: at most one step can be in_progress at a time): SetTodoList now rejects lists with more than one in_progress item with a corrective tool error so the model self-heals on the next step, and the tool description plus the system-prompt todo guidance gain matching status discipline (no single-step lists, no pending-to-done jumps, no batch-completion).
Extend the review and code-reviewer ROLE_ADDITIONAL overlays with the judgment guidance from codex-rs prompts/templates/review/rubric.md: an explicit finding bar (discrete, actionable issues the author would fix; rigor matched to the codebase; no unstated-intent assumptions; ripple effects must name provably affected code; prefer zero findings over speculation, but list every qualifying one), comment-construction rules (severity honesty, trigger conditions, one matter-of-fact paragraph, max 3 lines of quoted code), and an overall-correctness verdict with justification at the end of the review summary.
Complete the Codex goals port: the agent can now mark the active /goal 'complete' (only after the evidence-based completion audit) or 'blocked' (only after the strict three-strike blocked audit) via the new root-only UpdateGoal tool. Marking the goal stops reminders and continuations; /goal resume reactivates either state. With goal.auto_continue = true (new [goal] config table, default off, max_continuations 1-10 capped at 3 by default), every non-slash user submission is followed by automatic continuation turns carrying the Codex continuation prompt until the goal is marked, a tool call is rejected, or the cap is reached; the final continuation appends a budget-style wrap-up instruction (goal_wrap_up.md). Hard stops (cancellation, MaxStepsReached, provider errors) propagate and end the loop with the run. goal_continuation.md now carries Codex's full UpdateGoal completion contract and blocked audit. The config change also introduces the compact_prompt key; it is wired into compaction in the follow-up commit.
…uidance Wire the new compact_prompt config key into SimpleCompaction: when set it replaces the built-in summarization prompt for both manual and automatic compaction, with the per-invocation /compact focus still appended on top; unset preserves current behavior byte-for-byte. Auto/yolo-mode injections now tell the agent to proactively run tests and lint before finishing (no user present to confirm validation), and the back-to-interactive reminder defers slow test/lint commands to user confirmation except for test-related tasks — ported from the Codex CLI validation philosophy (approval-mode-aware validation).
Port the Codex User Updates spec into the system prompt as a Progress cadence bullet (short notes on meaningful insights, goal/constraints/ next-steps before the first tool call of substantial work, heads-down announcements, explicit plan-change callouts). Document the new [goal] config table and compact_prompt key, update the /goal reference for UpdateGoal and auto-continuation, and add the changelog entries.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds a persistent thread goal system ( ChangesGoal workflow, continuation loop, and policy updates
Sequence Diagram(s)sequenceDiagram
participant User
participant PythinkerSoul
participant GoalModeInjectionProvider
participant UpdateGoalTool
User->>PythinkerSoul: /goal set {objective}
PythinkerSoul->>PythinkerSoul: persist session.state.goal
PythinkerSoul->>GoalModeInjectionProvider: request injections for turn
GoalModeInjectionProvider-->>PythinkerSoul: inject reminder (full/sparse)
PythinkerSoul->>UpdateGoalTool: call UpdateGoal(status="complete")
UpdateGoalTool-->>PythinkerSoul: persist status, return guidance
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pythinker_code/soul/pythinkersoul.py (1)
1021-1044:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStop goal auto-continuation when the primary turn already ended in a non-continuable state.
run()always calls_run_goal_continuations()for non-slash input, but the initial_turn()outcome is ignored. That can still launch continuation turns after a primarytool_rejected/stuckoutcome, which breaks the intended stop conditions.Suggested fix
@@ - else: - await self._turn(user_message) + else: + primary_outcome = await self._turn(user_message) @@ - if command_call is None: + if command_call is None and ( + primary_outcome is None or primary_outcome.stop_reason == "no_tool_calls" + ): await self._run_goal_continuations()Also applies to: 1103-1127
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pythinker_code/soul/pythinkersoul.py` around lines 1021 - 1044, The code currently calls _run_goal_continuations() unconditionally after awaiting _turn(user_message), which can start continuations even when the primary turn ended in a non-continuable state (e.g., tool_rejected or stuck); change the first await self._turn(user_message) call to capture the turn outcome (e.g., result = await self._turn(user_message) or have _turn return a status), then guard the call to self._run_goal_continuations() so it only executes when that result indicates the turn is continuable; apply the same change to the other occurrence mentioned (the block that mirrors lines 1103-1127) and ensure existing logic around command_call and _stop_hook_active is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/en/configuration/config-files.md`:
- Line 41: Update the docs to mark compact_prompt as nullable: change its type
from `string` to `string | null` (or indicate "string or null/unset") and note
that leaving it null/unset preserves the default handoff-structured prompt (with
`/compact` still appended); reference the `compact_prompt` config key and ensure
wording matches the runtime/schema which accepts null/None.
In `@src/pythinker_code/agents/default/system.md`:
- Around line 171-173: The two rules in "Status discipline." ("at most one item
`in_progress` at a time") and "One todo per dispatched child." (require one
`in_progress` sub-todo per child before batch start) contradict the validator;
update the contract text to allow an explicit exception for parallel child
batches by changing the "Status discipline." line to say that at most one
top-level todo may be `in_progress` except when launching parallel children, in
which case each dispatched child must have its own `in_progress` sub-todo as
described in "One todo per dispatched child."; reference the exact rule headings
"Status discipline." and "One todo per dispatched child." and ensure the
validator text/requirements mention this exception so parallel-child batches
validate correctly.
In `@src/pythinker_code/prompts/best_practices.md`:
- Line 1: The markdown file best_practices.md is missing a top-level H1 header;
add a one-line H1 at the very top of the file (e.g., "# Best Practices") so the
document begins with an H1 instead of body text to satisfy MD041; update any
other prompt markdown files the same way to ensure consistent structure and
clean markdownlint output.
In `@src/pythinker_code/tools/goal/__init__.py`:
- Around line 55-60: The code currently allows immediate transition to
params.status == "blocked" for any active goal; add a persisted
blocked-strike/audit gate on the GoalState so blocked requires repeated
confirmations. When handling params.status == "blocked" (around where
self._runtime.session.state.goal and GoalState are set), read or initialize a
persisted counter field on the current goal (e.g., blocked_strikes or
blocked_audit) in self._runtime.session.state.goal, increment it and save the
session, and only set status="blocked" on GoalState once the counter reaches a
configurable threshold (e.g., 2); if below threshold, do not change the active
goal status to blocked and return/reject the premature blocked update (and
persist the incremented counter and appropriate next_step message). Ensure you
handle the case where no goal exists and always call
self._runtime.session.save_state() after modifying the counter or goal state.
In `@tasks/todo.md`:
- Line 71: Replace the malformed fence token "```report" with inline code using
single backticks so the phrase reads `report` (i.e., change the sequence
"```report" to "`report`" in the sentence beginning "Codex P0-P3 JSON review
schema (pythinker has its own ...") to prevent it being parsed as a code fence
and to restore proper Markdown rendering.
In `@tests/core/test_config.py`:
- Around line 74-75: Add explicit boundary tests for the new goal config fields
by writing tests that set goal.max_continuations to invalid edge values (e.g., 0
and 11) and assert schema validation fails (raise the same
ValidationError/ConfigError your project uses) rather than only snapshotting
defaults; locate the tests in tests/core/test_config.py, add parametric cases
for goal.max_continuations (0 and 11) to the existing config validation test
(the one that currently checks {"goal": {"auto_continue": False,
"max_continuations": 3}, "compact_prompt": None}) and ensure the tests assert an
error is raised for those inputs while leaving the happy-path default snapshot
unchanged.
In `@tests/tools/test_update_goal.py`:
- Around line 36-46: Add a new regression test (or modify the existing test
suite) that verifies a single/early "blocked" update is rejected until the
required repeated-blocked threshold is reached: call the UpdateGoal tool via
update_goal_tool(Params(status="blocked", ...)) when the
runtime.session.state.goal.blocked_count (or initial state) is below the
threshold, assert result.is_error is True and result.output contains the
rejection message, and assert runtime.session.state.goal.status remains
unchanged; use the same symbols from the diff (test_marks_goal_blocked /
update_goal_tool / Params / runtime.session.state.goal) so the test targets the
lifecycle contract rather than accepting the first blocked submission.
---
Outside diff comments:
In `@src/pythinker_code/soul/pythinkersoul.py`:
- Around line 1021-1044: The code currently calls _run_goal_continuations()
unconditionally after awaiting _turn(user_message), which can start
continuations even when the primary turn ended in a non-continuable state (e.g.,
tool_rejected or stuck); change the first await self._turn(user_message) call to
capture the turn outcome (e.g., result = await self._turn(user_message) or have
_turn return a status), then guard the call to self._run_goal_continuations() so
it only executes when that result indicates the turn is continuable; apply the
same change to the other occurrence mentioned (the block that mirrors lines
1103-1127) and ensure existing logic around command_call and _stop_hook_active
is preserved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 96e0e5af-d047-41f2-82a3-a8c4a79e84eb
📒 Files selected for processing (37)
CHANGELOG.mddocs/en/configuration/config-files.mddocs/en/reference/slash-commands.mdsrc/pythinker_code/agents/default/agent.yamlsrc/pythinker_code/agents/default/code_reviewer.yamlsrc/pythinker_code/agents/default/review.yamlsrc/pythinker_code/agents/default/system.mdsrc/pythinker_code/config.pysrc/pythinker_code/prompts/__init__.pysrc/pythinker_code/prompts/best_practices.mdsrc/pythinker_code/prompts/goal_continuation.mdsrc/pythinker_code/prompts/goal_set.mdsrc/pythinker_code/prompts/goal_wrap_up.mdsrc/pythinker_code/session_state.pysrc/pythinker_code/soul/compaction.pysrc/pythinker_code/soul/dynamic_injections/auto_mode.pysrc/pythinker_code/soul/dynamic_injections/goal_mode.pysrc/pythinker_code/soul/pythinkersoul.pysrc/pythinker_code/soul/slash.pysrc/pythinker_code/tools/goal/__init__.pysrc/pythinker_code/tools/goal/update_goal.mdsrc/pythinker_code/tools/todo/__init__.pysrc/pythinker_code/tools/todo/set_todo_list.mdtasks/todo.mdtests/core/test_agent_spec.pytests/core/test_auto_injection.pytests/core/test_best_practices_slash.pytests/core/test_config.pytests/core/test_default_agent.pytests/core/test_goal_auto_continuation.pytests/core/test_goal_mode_injection_provider.pytests/core/test_goal_slash.pytests/core/test_simple_compaction.pytests/tools/test_todo.pytests/tools/test_tool_descriptions.pytests/tools/test_update_goal.pytests/utils/test_pyinstaller_utils.py
The wire-protocol initialize handshake pins the full slash-command list; the two commands added in this branch were missing from the expected payload, failing the CI test matrix. Snapshot refixed via --inline-snapshot=fix and diff-reviewed; full tests_e2e suite green locally (65 passed, 4 skipped).
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
- Goal auto-continuation now requires the primary turn to end cleanly (no_tool_calls): a tool rejection or stuck primary turn no longer triggers continuations, matching the rule already applied between continuation turns. Ralph-loop runs never continue (own strategy). - Soften the SetTodoList single-in_progress invariant from a hard rejection to a corrective notice: pythinker's parallel-subagent fan-out legitimately tracks one in_progress sub-todo per running child (system.md orchestration rules), so rejecting such lists would break the documented workflow. The tool description, system.md status-discipline bullet, and changelog wording are reconciled to state the sequential rule and its fan-out exception. - Document compact_prompt as nullable in the config reference, fix a malformed report fence token in tasks/todo.md, and add boundary tests for goal.max_continuations (1-10). Declined (with rationale): mechanical enforcement of the blocked-audit three-strike gate inside UpdateGoal — Codex itself enforces it as a prompt contract, and 'same blocking condition' is semantic, so code enforcement would misfire on legitimate impasses. H1 headings for the prompt markdown assets — injected prompt files conventionally start with body text in this repo (init.md, compact.md) and there is no markdownlint gate.
Member
Author
|
Addressed the CodeRabbit review in 555107e: Fixed
Declined with rationale
Verification: full |
This was referenced Jun 11, 2026
This was referenced Jun 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ports the goal-driven execution system and agentic-loop best practices from Codex CLI (
blackbox/codex-main) into Pythinker. Scouted via primary sources (codex-rs/prompts/templates/goals/*,templates/review/rubric.md,gpt_5_2_prompt.md,gpt-5.1-codex-max_prompt.md)./goal— persistent thread goal (Codex goals port)/goal <objective>persists aGoalStatein session state, kicks off work with a success-criteria derivation prompt, and re-injects the Codex continuation contract on later turns (fidelity rules: no scope-shrinking, no easier-to-test substitutes; evidence-based completion audit). Objectives are framed as untrusted data in<objective>tags.view,pause,resume,clear. Survives restarts and context compaction (full reminder re-fires after compaction); root-agent only; goal replacement announces immediately.UpdateGoaltool: marks the goalcomplete(only after the completion audit proves every requirement) orblocked(only after the strict three-strike blocked audit), stopping reminders and continuations.[goal] auto_continue, default off;max_continuations1–10, default 3): each non-slash user submission is followed by automatic continuation turns toward the active goal until it is marked, a tool call is rejected, or the cap is hit — the final continuation carries a budget-style wrap-up instruction./best-practices(alias/bp)Injects opt-in engineering guidance distilled from the Codex system prompts (code-change discipline, dirty-worktree safety, specific-to-broad testing, todo hygiene, progress cadence, debugging methodology, final-answer style) into context without consuming a turn; optional per-section filter.
Loop-discipline adoption
in_progressinvariant (corrective tool error) + matching status-discipline guidance insystem.md.review.yaml,code_reviewer.yaml) adopt the Codex review rubric: finding bar, comment-construction rules, overall-correctness verdict.compact_promptconfig override for the compaction summarization prompt (unset = byte-identical behavior).Verification
test_goal_slash,test_goal_mode_injection_provider,test_goal_auto_continuation,test_update_goal,test_best_practices_slash, todo-invariant, compaction-override, auto-injection pins).make check-pythinker-codegreen (ruff, format, pyright 0 errors).--inline-snapshot=fixand diff-reviewed.Notes for review
agents/default/system.mdgains two bullets (status discipline, progress cadence); reviewer overlay additions are scoped toROLE_ADDITIONAL.goal.auto_continue=false,compact_prompt=None).Summary by CodeRabbit
New Features
Configuration
Behavior Changes
Documentation
Tests