Skip to content

feat(runtime): guide Auto tool selection by final tool surface - #3705

Open
testikun wants to merge 2 commits into
apache:mainfrom
testikun:codex/issue-3507-auto-tool-guidance
Open

feat(runtime): guide Auto tool selection by final tool surface#3705
testikun wants to merge 2 commits into
apache:mainfrom
testikun:codex/issue-3507-auto-tool-guidance

Conversation

@testikun

@testikun testikun commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Closes #3507

What changed

  • Add a pure Auto tool-guidance resolver in packages/runtime/src/system-prompt/auto-tool-guidance.ts.
  • Resolve eligibility from the Host permission snapshot and the final model-visible tool surface, after routing, profiles, plan filtering, and capability projection.
  • Inject the guidance only into the normal Interactive main-session prompt. Child instructions, Side Conversations, Deep Research, bound tool ceilings, broken shells, fixed profiles, and non-Auto modes remain unchanged.
  • Advertise only structured tools that are actually exposed, and explicitly state that Bash cannot bypass Maka sandbox, workspace, network, or approval boundaries.
  • Bump the Interactive Composer revision and add resolver/composer regression coverage.

Scope

This changes prompt guidance only. It does not change permission classification, sandbox or approval execution, Bash schema/implementation, persistence protocols, or the headless-coding-v1 contract.

Verification

  • Runtime full suite: 3010 tests, 2997 passed, 13 skipped, 0 failed.
  • Runtime Host full suite: 1137 passed, 0 failed.
  • Focused guidance/composer tests: 34 passed.
  • Runtime and Runtime Host builds/typechecks, root lint, format check, ASF header check, and git diff --check passed.

AI use

  • Generative tooling made a substantive contribution

Tool: OpenAI Codex assisted with issue analysis, implementation, tests, and verification.

@testikun
testikun force-pushed the codex/issue-3507-auto-tool-guidance branch 3 times, most recently from 18a5aba to 191282e Compare August 24, 2026 12:15

@yunaremaia yunaremaia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, clean slice — the eligibility matrix is well chosen (bypass needs no anti-bypass guidance, explore has no mutation tools to name), the fragment is deterministic and pure, and the tests cover both the positive path and every exclusion branch including the partial-tool-surface case. The bump of INTERACTIVE_RUN_COMPOSER_REVISION for the prompt-shape change is also the right call.

One coupling worth making explicit while this is fresh:

The Bash gate lives two checks away from its supporting fact. In interactive-run-composer.ts you compute

shellAvailable: input.shell?.setupError === undefined,

which maps an absent shell to undefined rather than false. The resolver then only treats literal false as disqualifying, so a session with no shell at all still passes the shellAvailable check — it survives solely because a missing shell also removes 'Bash' from tools, and resolveAutoToolGuidance independently requires toolNames.includes('Bash'). That's correct today, but the safety of the first check depends on an invariant maintained by a different subsystem (whatever guarantees "no shell ⇒ no Bash tool entry"). If that ever drifts — say a future surface exposes a remote-shell Bash entry without a local input.shell — the guidance would advertise shell-first workflows into a session that cannot run them, and nothing here would fail loudly.

Two cheap ways to pin it down; either works:

  1. Treat absence as absence: pass shellAvailable: input.shell !== undefined && input.shell.setupError === undefined and keep the resolver's === false check (or switch both to truthiness). One expression, no cross-module assumption.
  2. Keep the current wiring but add one line to the resolver doc comment stating that shellAvailable === undefined means "unknown" and eligibility then rests entirely on the toolNames gate — so the next reader knows the pairing is intentional.

Not blocking — behavior is correct as written — but I'd sleep better with option 1.

@testikun

testikun commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @yunaremaia — fixed in commit bf96cd0 using option 1. An absent shell plan is now explicitly treated as unavailable by requiring both a defined shell plan and no setup error, so Auto guidance no longer relies on the separate final-tool-surface gate to stay safe. I also updated the positive prompt fixtures to include a valid shell plan and added a regression case proving that a Bash-shaped tool surface without a shell plan does not receive the guidance. Verification: Runtime Auto guidance 4/4 passing; Runtime Host 1137/1137 passing; Biome and git diff --check clean.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this head and found no blocking issues.

The fix adds Auto-mode tool guidance as a pure function that chooses whether to inject a deterministic prompt based on the final tool surface and Host permission snapshot. It is minimal (1 file + 1 call site), correctly handles restricted surfaces and permission bypass cases, and bumps the composer revision to invalidate caches.

Checks on bf96cd058c are test: pending — code side is GO. Minor note: this adds a small new concept/module (+72 lines); the increment is valuable for recovery (#3507) and well isolated, so no action required.

简体中文该头未发现阻断问题,轻度增熵有价值。

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE — bf96cd0, no P0-P2, pure-function guidance correctly scoped.

@Astro-Han

Copy link
Copy Markdown
Contributor

Holding review here — the premise question is on #3507. Implementation quality isn't the issue: 443 lines across 5 files, pure resolver, well-gated eligibility, no design doc in the PR. Nicely contained.

But one thing is visible from the diff and worth raising now: the fragment doesn't do what the issue asks for. #3507 quotes a directive instruction — do the work through Bash wherever it can, fall back to dedicated tools only when Bash genuinely can't. What resolveAutoToolGuidance emits is "choose the tool that best fits the operation", then hedges toward Bash for pipelines, Read/Glob/Grep for inspection, and Edit/Write for reviewable diffs. That's use-the-right-tool, not CLI-first. A model that would already reach for Read to inspect and Edit to patch gets no new signal from it.

So both paths lead to the same place: if the retry-loop problem is real, this text probably doesn't move it; if it isn't real, we've added permanent tokens to every Auto session's system prompt. Either way it needs an eval A/B rather than a code review.

Could you run one alongside this PR — same task set, fragment on vs off, tool-failure retry rate and completion rate? If it moves the numbers I'll review the implementation properly.

简体中文

先暂停评审——前提问题写在 #3507。实现质量不是问题:443 行 5 个文件,纯函数 resolver,准入条件收得很干净,PR 里也没有塞设计文档,边界很好。

但有一点从 diff 就能看出来,值得现在提:这个片段并没有在做 issue 要求的事。#3507 引用的是一条指令性的提示——能用 Bash 完成的就用 Bash,只有 Bash 确实做不到时才回退到专用工具。而 resolveAutoToolGuidance 输出的是「选择最适合该操作的工具」,然后两边都照顾:Bash 适合流水线,Read/Glob/Grep 适合检查,Edit/Write 适合可审查 diff。这是「用对工具」,不是 CLI-first。一个本来就会用 Read 检查、用 Edit 改文件的模型,从这段话里得不到任何新信号。

所以两条路通向同一个地方:如果重试循环问题真实存在,这段文字大概率推不动它;如果不存在,我们就是在每个 Auto 会话的系统提示里永久多加了一段。无论哪种,需要的是 eval 对照,而不是 code review。

能否和这个 PR 一起跑一组?同一批任务,开/关片段,看工具失败重试率和完成率。数字有变化,我就正式评审实现。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use mode-aware CLI-first tool guidance in Auto mode

3 participants