fix(agents): keep exec visible for lean local models#101607
Conversation
Co-authored-by: Vincent Koc <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ecab982dfc
ℹ️ 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".
|
|
||
| ### Fixes | ||
|
|
||
| - **Lean local model shell access:** keep `exec` directly visible beside the default structured Tool Search controls so coding-tuned local models can use their shell fallback instead of searching for missing domain tools. (#87587) Thanks @vincentkoc. |
There was a problem hiding this comment.
Remove the hand-written changelog entry
This normal agent fix adds an Unreleased entry even though the root Docs / Changelog rule says CHANGELOG.md is release-only and that release generation derives entries from merged PRs and direct commits (AGENTS.md:238-240). Keeping this hand-written entry risks duplicate or stale release notes; leave the release-note context in the commit/PR text and let the release generator update the changelog.
Useful? React with 👍 / 👎.
|
Maintainer replay review complete. This is the best bounded fix for the verified visibility defect: it uses the existing catalog predicate seam, covers both runtime tool-surface owners, keeps only policy-filtered Land-ready proof for exact head
Isolated Ollama A/B: the lean default exposed only Tool Search controls and produced a fabricated weather answer after an empty search; direct |
|
Merged via squash.
|
Co-authored-by: Vincent Koc <[email protected]>
Co-authored-by: Vincent Koc <[email protected]>
What Problem This Solves
Lean local-model mode defaults structured Tool Search on, which catalogs every non-control tool. That also hid
exec, so coding-tuned local models could see only the Tool Search controls even when policy allowed shell access. This replays the useful part of #87587 against currentmain; the contributor branch is stale and does not allow maintainer edits.This addresses only the direct-shell-visibility slice of #86632. It does not claim that every local model will choose correct commands or successfully retrieve live data.
Why This Change Was Made
The existing catalog-compaction owner already accepts a per-tool predicate. This change adds one lean-model predicate and passes it through both runtime owners: the embedded attempt path and the shared harness bridge.
The exception is intentionally narrow:
execstays directly visible;toolsmode gets the exception;codeanddirectorymodes retain normal compaction;User Impact
Lean local models can directly select
execwhile the rest of the normal tool catalog remains compact. Existing tool allowlists, denylists, sandboxing, approvals, and hooks still decide whether and howexeccan run.The local-model docs now state that boundary. The changelog credits @vincentkoc for the original diagnosis and implementation direction.
Evidence
corepack pnpm test src/agents/local-model-lean.test.ts src/agents/harness/tool-surface-bridge.test.ts src/agents/tool-search.test.ts— 66 tests passed.toolssurfaces keep controls plusexec, catalogread, and leave explicitcodemode unchanged.git diff --checkpassed.qwen3.5:9b: the lean default exposed only Tool Search controls and the model fabricated a weather answer after an empty search; directexecvisibility caused the model to select shell commands instead. The attempted commands were invalid, so this is visibility proof, not end-to-end weather-success proof.qwen3.5:9bmodel card.Replaces #87587 while preserving contributor credit in the commit.