From bda98c63ef2ede2cba8fc6ca312804843bf3a6fd Mon Sep 17 00:00:00 2001 From: can1357 Date: Tue, 23 Jun 2026 00:01:05 +0200 Subject: [PATCH] feat(coding-agent): elevated `eval` to an essential tool to ensure - Elevated `eval` to an essential tool to ensure availability across all discovery modes. - Updated system and tool prompts to mandate the use of `eval` for non-trivial shell operations like conditionals, loops, heredocs, and complex pipelines. - Restricted `bash` usage to simple binary invocations and single-fact computation to reduce shell-escaping and execution errors. --- packages/coding-agent/CHANGELOG.md | 5 +++++ .../coding-agent/src/config/settings-schema.ts | 2 +- .../src/prompts/system/system-prompt.md | 6 +++--- packages/coding-agent/src/prompts/tools/bash.md | 15 +++++++++++++++ packages/coding-agent/src/tools/eval.ts | 2 +- packages/coding-agent/src/tools/index.ts | 9 ++++++++- .../test/tool-discovery/initial-tools.test.ts | 15 +++++++++++++++ 7 files changed, 48 insertions(+), 6 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index abce94513..b447eb54e 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -10,6 +10,11 @@ - Added `isolated`, `apply`, and `merge` options to eval `agent()` across every workflow runtime (Python, JavaScript, Ruby, Julia) so `workflowz`-driven fan-outs can request the same copy-on-write worktree isolation the `task` tool offers (strict opt-in via `isolated: true`, matching the `task` tool; `apply: false` keeps captured patches/branches without merging back; `merge: false` forces patch mode). Extracted the task-isolation lifecycle into `task/isolation-runner.ts` so the eval bridge and `TaskTool` share one implementation ([#3196](https://github.com/can1357/oh-my-pi/issues/3196)) +### Changed + +- Reinforced routing of fragile, multi-step shell logic to the `eval` tool over `bash`. The system-prompt tool policy, `bash.md`, and `eval.md` now treat loops, conditionals, heredocs, inline `-e`/`-c` scripts, multi-stage pipelines, and quote/JSON escaping as the signal to write an `eval` cell; bash's "compute a fact" carveout is narrowed to single short pipelines, and `eval.md` now actively claims that territory with runtime-templated examples (only enabled backends are advertised). +- Made `eval` an essential built-in tool (`loadMode: "essential"`, added to the default essential tool set) so it stays active under `tools.discoveryMode: "all"` instead of being hidden behind `search_tool_bm25`. + ### Fixed - Fixed streaming output blocks incorrectly calculating preview height, preventing flickering banners diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index 636dbb4a5..c5fc811c8 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -3572,7 +3572,7 @@ export const SETTINGS_SCHEMA = { group: "Discovery & MCP", label: "Essential Tools Override", description: - "Override the always-loaded built-in tools (default: read, bash, edit). Leave empty to use defaults.", + "Override the always-loaded built-in tools (default: read, bash, edit, write, find, eval). Leave empty to use defaults.", }, }, diff --git a/packages/coding-agent/src/prompts/system/system-prompt.md b/packages/coding-agent/src/prompts/system/system-prompt.md index 0bef2bca1..09e304ef8 100644 --- a/packages/coding-agent/src/prompts/system/system-prompt.md +++ b/packages/coding-agent/src/prompts/system/system-prompt.md @@ -109,9 +109,9 @@ You MUST use the specialized tool over its shell equivalent: {{#has tools "lsp"}}- Code intelligence → `{{toolRefs.lsp}}`.{{/has}} {{#has tools "search"}}- Regex search → `{{toolRefs.search}}`, not `grep`, `rg`, or `awk`.{{/has}} {{#has tools "find"}}- Globbing → `{{toolRefs.find}}`, not `ls **/*.ext` or `fd`.{{/has}} -{{#has tools "eval"}}- Quick compute → `{{toolRefs.eval}}`; you SHOULD go step by step.{{/has}} -{{#has tools "bash"}}- Use `{{toolRefs.bash}}` for terminal work—builds, tests, git, package managers—and pipelines that COMPUTE a fact: `wc -l`, `sort | uniq -c`, `comm`, `diff a b`, checksums. Commands shadowing the tools above are blocked. -- Litmus: produces a count, frequency, set difference, or checksum no tool returns → bash. Merely moves, pages, or trims bytes a tool can fetch → use the tool.{{/has}} +{{#has tools "eval"}}- Default for any compute: `{{toolRefs.eval}}` cells. Bash is the EXCEPTION — only single binary calls or short fact-computing pipelines (`wc -l`, `sort | uniq -c`, `diff`, checksums). The moment a command grows a loop, conditional, heredoc, `-e`/`-c` script, `$(...)` nesting, or >2 pipe stages, it's a program → `{{toolRefs.eval}}`. MUST NOT write multiline or inline-script bash.{{/has}} +{{#has tools "bash"}}- `{{toolRefs.bash}}`: real binaries and short fact pipelines only. Commands shadowing the specialized tools above are blocked.{{/has}} +{{#has tools "bash"}}- Litmus: one external-CLI call or short pipeline returning a count, frequency, set difference, or checksum → bash.{{#has tools "eval"}} Needs control flow, state, or fights shell quoting → `{{toolRefs.eval}}`.{{/has}} Merely moves, pages, or trims bytes a tool can fetch → use the tool.{{/has}} {{#has tools "report_tool_issue"}} diff --git a/packages/coding-agent/src/prompts/tools/bash.md b/packages/coding-agent/src/prompts/tools/bash.md index 69ee52050..6a7e391a3 100644 --- a/packages/coding-agent/src/prompts/tools/bash.md +++ b/packages/coding-agent/src/prompts/tools/bash.md @@ -1,5 +1,19 @@ Runs bash in a shell session — terminal ops: git, bun, cargo, python. +# When to use bash — and when not to + +Bash invokes **real binaries** with simple args. It is NOT a scripting surface. + +Use bash ONLY for: a single binary call, or one short pipeline that COMPUTES a fact (`wc -l`, `sort | uniq -c`, `comm`, `diff`, a checksum, `git status`). + +Anything below → `eval` cell, not bash: +- Inline interpreter scripts (`-e`/`-c`/`--eval`) when an eval runtime exists for that language +- Heredocs (`< - `cwd` sets the working dir, not `cd dir && …` - `env: { NAME: "…" }` for multiline / quote-heavy / untrusted values; reference `$NAME` @@ -14,6 +28,7 @@ Runs bash in a shell session — terminal ops: git, bun, cargo, python. +- Bash invokes real binaries with simple args; it is NOT a scripting surface. Loops, conditionals, heredocs, inline interpreter scripts (`node -e`, `python -c`), several piped stages, or quote/JSON escaping mean you're writing a program → use `eval` cells: restartable, stateful, and free of shell-quoting traps. - NEVER shell out to search content or files: `grep/rg` → `search`. - Avoid head/tail/redirections: stderr already merged; long output auto-truncated, FULL capture kept at `artifact://`. diff --git a/packages/coding-agent/src/tools/eval.ts b/packages/coding-agent/src/tools/eval.ts index 19e0ba8f2..05bb79360 100644 --- a/packages/coding-agent/src/tools/eval.ts +++ b/packages/coding-agent/src/tools/eval.ts @@ -305,7 +305,7 @@ export class EvalTool implements AgentTool { get summary(): string { return summarizeEvalLanguages(this.#enabledLanguages()); } - readonly loadMode = "discoverable"; + readonly loadMode = "essential"; readonly label = "Eval"; get description(): string { if (!this.session) return getEvalToolDescription(); diff --git a/packages/coding-agent/src/tools/index.ts b/packages/coding-agent/src/tools/index.ts index 2eb00921e..494b20df7 100644 --- a/packages/coding-agent/src/tools/index.ts +++ b/packages/coding-agent/src/tools/index.ts @@ -377,7 +377,14 @@ export type ToolFactory = (session: ToolSession) => Tool | null | Promise { } expect(missing).toEqual([]); }); + + it("marks eval essential so it survives tools.discoveryMode 'all'", async () => { + const metadata = await getToolMetadata(); + expect(metadata.get("eval")?.loadMode).toBe("essential"); + // Essential loadMode keeps eval active under discovery-all even when it is + // absent from the essential-names set — not relying on the names list. + const kept = filterInitialToolsForDiscoveryAll(["eval"], { + loadModeOf: name => metadata.get(name)?.loadMode as BuiltinToolLoadMode | undefined, + essentialNames: new Set(), + explicitlyRequested: new Set(), + restored: new Set(), + forceActive: new Set(), + }); + expect(kept).toEqual(["eval"]); + }); }); describe("computeEssentialBuiltinNames", () => {