Repository navigation
feat: expose guarded native Working Plan revisions - #310
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 821af16dea
ℹ️ 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".
| def _agent_prompt_tools_list(self, agent_config: dict[str, Any]) -> str: | ||
| """Build the inventory within config, active token and interaction bounds.""" | ||
| permitted = self._permitted_tool_names(agent_config) - {"update_plan"} | ||
| permitted = self._permitted_tool_names(agent_config) |
There was a problem hiding this comment.
Keep update_plan out of legacy tool inventories
For text-only providers in Agent mode, _build_messages() and the legacy action-round prompts both populate their tool list through _agent_prompt_tools_list, so this change advertises update_plan through the string-based Action protocol as well as through native schemas. That conflicts with the native-only behavior documented by this change and gives legacy models only a prose description rather than the structured revision schema, leading to malformed plan calls or unintended JSON-string plan mutations. Keep it excluded from textual inventories and expose it only through the native provider schema set.
AGENTS.md reference: AGENTS.md:L35-L35
Useful? React with 👍 / 👎.
| permitted = {name for name in self.AVAILABLE_TOOLS if self.tool_registry.get_spec(name).visibility == 'public' and tool_capability(name,self.AVAILABLE_TOOLS) in capabilities} | ||
| if self._active_interaction_mode.get() != "agent": | ||
| permitted.discard("update_plan") |
There was a problem hiding this comment.
Return the plan revision after native plan creation
When a native model first calls the newly exposed update_plan tool with the supported complexity/tasks form, _update_plan() returns the plan ID and task IDs but omits plan.revision. Every subsequent structured edit requires expected_revision, so the model cannot reliably issue a revision—especially when this form updated an already-active plan whose revision is not predictably 1. Include the resulting revision in the creation/update response before advertising this tool to native providers.
Useful? React with 👍 / 👎.
| permitted = {name for name in self.AVAILABLE_TOOLS if self.tool_registry.get_spec(name).visibility == 'public' and tool_capability(name,self.AVAILABLE_TOOLS) in capabilities} | ||
| if self._active_interaction_mode.get() != "agent": | ||
| permitted.discard("update_plan") |
There was a problem hiding this comment.
Add native task binding before exposing plan creation
For a direct native Agent turn that uses the newly advertised update_plan schema to create tasks, subsequent tool calls cannot be associated with any returned task ID: AgentEngine._run_native() sets each call's task_id only from _delegation_task_id, which is None for foreground work, and records receipts with legacy matching disabled. Since neither ToolCall nor the individual tool schemas provide another task-ID channel, execution and verification receipts never advance these tasks, so a complex Working Plan created by the model remains pending and the turn is reported incomplete regardless of successful tool work.
Useful? React with 👍 / 👎.
Partial implementation related to #267; evaluation acceptance remains open.
Root cause: the typed update_plan tool already had revision-checked persistence and Strategic Plan guards, but the native Agent tool boundary filtered it from permitted schemas and prompts.
Approach: expose its existing structured schema only in Agent mode when effective capabilities include write; Ask and Plan remain read-only, and the legacy textual tool inventory continues to hide the structured-only operation. The active capability token and existing Strategic Plan approval and binding checks remain authoritative. This reuses the current registry and executor rather than adding a second plan-edit API.
Validation: full make test (1,028 passed, 13 skipped); make check; make docs-check; focused native continuation and Working Plan tests. A fake native provider received the schema, revised a plan, and continued with another typed call. Commit is GPG-signed and verified.
Risk: models can now revise Working Plans in Agent mode when granted write capability; plan revision still does not authorize task execution or widen an accepted Strategic Plan. This PR depends on the existing V3 plan and capability foundations. #266 matched-evaluation fixtures and live cohort are separate follow-up work.