fix(prompting): hide eval guidance when disabled#4644
Conversation
Stop advertising eval in the default prompt and workflow notice when no eval backend is enabled. Gate bash guidance on live eval backend availability and cover the disabled-backend rendering contract. Agent-Milestone: tooling: hide eval prompt guidance when eval backends are disabled Signed-off-by: Christian Stewart <christian@aperture.us>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b765657f28
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| - NEVER shell out to search content or files: `grep/rg` → `grep`. | ||
| - NEVER use `ls` or `find` to list or locate files — `ls` → `read` (a directory path lists entries), `find` → the `glob` tool (globbing). This is non-negotiable, even for a single quick listing. |
There was a problem hiding this comment.
In sessions where bash is active but grep, read, or glob are disabled or omitted from an explicit toolNames list, the newly added bash descriptor still tells the model to call those tools instead of shelling out. This can happen for restricted/SDK subagents (the new test even builds tools with only bash after eval is filtered), so the prompt now points at tools that are not actually available; guard these lines with the existing hasGrep/hasGlob/active-tool checks or phrase them without naming missing tools.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 92765fc. Bash now renders file/search guidance from the active tool set, not only settings, and the restricted bash/eval-disabled prompt test asserts it no longer names inactive grep/read/glob tools. Verified with bun test packages/coding-agent/test/system-prompt-inventory.test.ts packages/coding-agent/test/tools/index.test.ts packages/coding-agent/test/modes/workflow.test.ts.
| @@ -1,8 +1,8 @@ | |||
| <system-notice> | |||
| The user's message above contains the **workflowz** keyword: drive this task as a deterministic multi-subagent workflow. Author the orchestration as Python in the `eval` tool and fan out subagents — to be comprehensive (decompose and cover in parallel), to be confident (independent perspectives and adversarial checks before you commit), or to take on scale one context can't hold (audits, migrations, broad sweeps). This overrides any default tendency to do the whole task inline when fanning out would be more thorough. | |||
| The user's message above contains the **workflowz** keyword: drive this task as a deterministic multi-subagent workflow. Use the `task` tool for batched fan-out — to be comprehensive (decompose and cover in parallel), to be confident (independent perspectives and adversarial checks before you commit), or to take on scale one context can't hold (audits, migrations, broad sweeps). This overrides any default tendency to do the whole task inline when fanning out would be more thorough. | |||
There was a problem hiding this comment.
After this line switches the workflowz steer to direct task tool fan-out, the same notice still documents eval-only helper APIs and examples below (agent(..., schema=...), parallel, pipeline, and “Every eval call”), which are not part of the task tool and include arguments that task rejects. In workflowz sessions—especially the eval-disabled case this change is trying to support—the hidden prompt now gives mutually incompatible instructions and can drive invalid calls; either keep this notice eval-gated or rewrite the helper/examples to the task schema.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 92765fc. The workflowz notice now documents the task batch contract end to end and removes the eval-only helper API/examples, so eval-disabled workflowz sessions get one orchestration surface. Verified with bun test packages/coding-agent/test/system-prompt-inventory.test.ts packages/coding-agent/test/tools/index.test.ts packages/coding-agent/test/modes/workflow.test.ts.
Signed-off-by: Christian Stewart <christian@aperture.us>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92765fc409
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| <helpers> | ||
| State persists across eval calls, so scout in one call and fan out in the next. Every eval call has: | ||
| <task-contract> | ||
| Call `task` once per independent fan-out batch. Put shared background in `context`, and put each independent work item in `tasks[]`. Do not emulate batching with shell loops or eval helper APIs. |
There was a problem hiding this comment.
When task.batch is disabled, the task tool uses the flat single-spawn schema and validateShapeParams rejects both context and tasks; I checked the task schema/validator in packages/coding-agent/src/task/index.ts. This static workflowz notice is appended whenever the keyword is present and now always tells the model to use the batch shape, so sessions with task.batch: false will be steered into invalid task calls instead of one flat assignment call per subagent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 25c94fa. The workflowz notice now renders against the active task.batch setting: batch-enabled sessions still get the context/tasks[] contract, while task.batch: false sessions get flat one-subagent-per-call guidance and an explicit warning not to pass context or tasks[]. AgentSession now renders the notice at injection time from settings. Verified with bun test packages/coding-agent/test/system-prompt-inventory.test.ts packages/coding-agent/test/tools/index.test.ts packages/coding-agent/test/modes/workflow.test.ts packages/coding-agent/test/agent-session-magic-keywords.test.ts.
Signed-off-by: Christian Stewart <christian@aperture.us>
There was a problem hiding this comment.
💡 Codex Review
Gate the remaining eval hint when eval is inactive
When all eval backends are disabled, createTools filters the eval tool out and renders this template with hasEval=false, but this unconditional instruction still tells the model to prefer eval. In a bash-only or eval-disabled session, the bash description therefore points at a missing tool for exact pipelines/output truncation; wrap this line in hasEval or provide a no-eval alternative.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const toolFactorySession: ToolSession = { | ||
| ...session, | ||
| isToolActive: name => activeToolNames.has(name), | ||
| }; |
There was a problem hiding this comment.
Because the tool factories now receive a cloned toolFactorySession, tools that keep the session reference (notably TaskTool) no longer see properties assigned to the original object after createTools returns. In the SDK startup path, toolSession.mcpManager is populated after built-in tools are created, and TaskTool later reads this.session.mcpManager ?? MCPManager.instance() when spawning subagents; with the clone, nested task calls in SDK/subagent sessions can miss the intended MCP manager or fall back to the wrong process-global one. Add isToolActive without replacing the session object, or keep the clone synchronized for late-bound fields.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 96850a1. createTools now annotates the original ToolSession with isToolActive instead of passing a cloned session into factories, so tools that retain the session still see late-bound fields like mcpManager. Added a regression asserting the original session records the active tool set. Verified with bun test packages/coding-agent/test/system-prompt-inventory.test.ts packages/coding-agent/test/tools/index.test.ts packages/coding-agent/test/modes/workflow.test.ts packages/coding-agent/test/agent-session-magic-keywords.test.ts.
Signed-off-by: Christian Stewart <christian@aperture.us>
|
Fixed the follow-on eval-disabled bash prompt review in 96850a1. The remaining exact-pipeline/output-truncation hint is now gated on |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96850a1642
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| role: "custom", | ||
| customType: "workflow-notice", | ||
| content: WORKFLOW_NOTICE, | ||
| content: renderWorkflowNotice({ taskBatch: this.settings.get("task.batch") }), |
There was a problem hiding this comment.
In restricted sessions that omit the task tool (for example SDK/CLI sessions with an explicit toolNames list such as read,bash,eval), this still appends a workflowz notice that tells the model to call task; #createMagicKeywordNotices only checks the keyword and task.batch, not the active tool set. Those turns are now steered into unavailable-tool calls even though the previous eval-based notice could still work when eval was active, so the notice should be skipped or rendered differently unless task is actually active.
Useful? React with 👍 / 👎.
| this.description = prompt.render(bashDescription, { | ||
| asyncEnabled: this.#asyncEnabled, | ||
| autoBackgroundEnabled: this.#autoBackgroundEnabled, | ||
| autoBackgroundThresholdSeconds: Math.max(0, Math.floor(this.#autoBackgroundThresholdMs / 1000)), | ||
| hasAstGrep: this.session.settings.get("astGrep.enabled"), | ||
| hasAstEdit: this.session.settings.get("astEdit.enabled"), | ||
| hasGrep: this.session.settings.get("grep.enabled"), | ||
| hasGlob: this.session.settings.get("glob.enabled"), | ||
| hasAstGrep: isToolActive("ast_grep", this.session.settings.get("astGrep.enabled")), | ||
| hasAstEdit: isToolActive("ast_edit", this.session.settings.get("astEdit.enabled")), | ||
| hasGrep: isToolActive("grep", this.session.settings.get("grep.enabled")), | ||
| hasGlob: isToolActive("glob", this.session.settings.get("glob.enabled")), | ||
| hasRead: isToolActive("read", true), | ||
| hasEval: isToolActive("eval", evalBackends.python || evalBackends.js || evalBackends.ruby || evalBackends.julia), |
There was a problem hiding this comment.
Because the bash descriptor is rendered once in the constructor using the active-tool snapshot from createTools, later calls to setActiveToolsByName rebuild the prompt and provider tool list with the same stale BashTool.description. If a session starts with grep/read active and then disables them while keeping bash, the bash schema still says to use those unavailable tools, so dynamic tool selection can produce invalid calls; refresh this guidance when the active tool set changes or avoid baking active-tool-specific text into a long-lived tool instance.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1936d4f. BashTool.description now renders from the live active-tool predicate, and AgentSession.setActiveToolsByName updates that predicate before computing the prompt signature/rebuilding. Added regression coverage for both createTools live bash guidance and the session rebuild path that drops grep while keeping bash.
Verified with bun test packages/coding-agent/test/tools/index.test.ts packages/coding-agent/test/agent-session-tool-rebuild-skip.test.ts packages/coding-agent/test/system-prompt-inventory.test.ts packages/coding-agent/test/modes/workflow.test.ts packages/coding-agent/test/agent-session-magic-keywords.test.ts (56 pass).
Signed-off-by: Christian Stewart <christian@aperture.us>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
1 participant
Keeps eval-specific shell guidance out of prompts when the eval runtime is unavailable, while leaving the normal bash tool constraints intact.