Skip to content

fix(doctor): stop warning about tools that exist - #9686

Open
dadgrand wants to merge 1 commit into
esengine:main-v2from
dadgrand:fix/doctor-builtin-allowlist
Open

fix(doctor): stop warning about tools that exist#9686
dadgrand wants to merge 1 commit into
esengine:main-v2from
dadgrand:fix/doctor-builtin-allowlist

Conversation

@dadgrand

@dadgrand dadgrand commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • doctor warns about tools that exist. isBuiltinOrMetaTool is a hand-written name list and it had drifted from what actually ships: use_capability, compress and update_goal were all missing from it.
  • The built-in review and security-review skills both name use_capability in allowed-tools, so every doctor run on a default install prints two warnings for a tool that is present:
warning: skill "review" allowed-tools references "use_capability" which is not in the current registry
warning: skill "security-review" allowed-tools references "use_capability" which is not in the current registry
  • Consult the built-in registry first via tool.LookupBuiltin, which cannot drift, and keep the hand-written list as the fallback it already was. compress and update_goal are registered built-ins and are now covered automatically. use_capability is wired up at boot rather than registered, so it is added to the list explicitly with a comment saying why.

Issues

Verification

  • 2 warnings before, 0 after, on an otherwise unchanged config and the same workspace.
  • go test ./... passes. gofmt -l . and go vet ./... clean. Go 1.26.2, darwin/arm64.
  • New tests in internal/doctor/skill_health_registry_test.go:
    • TestAllowedToolsAcceptsAnyRegisteredBuiltin registers a probe tool and asserts it is not reported missing — this pins the registry path rather than a name list, so the same drift cannot recur.
    • TestAllowedToolsStillWarnsOnAnUnknownName asserts a genuinely unknown name still warns, so the softened check does not silence real typos.

Documentation impact

Documentation-impact: none - diagnostics output only; no CLI, config, provider, permission or tool behavior changes.

Cache impact

Cache-impact: none - internal/doctor output never enters the provider request; no system prompt, memory prefix, tool schema or tool surface is touched.
Cache-guard: not applicable - no cache-sensitive path is modified. go test ./internal/boot/ -run TestGoldenBaselineNoExtensions still passes unchanged.
System-prompt-review: N/A

@github-actions github-actions Bot added the v2 Go rewrite (1.x) — main-v2 branch, active development label Sep 1, 2026
isBuiltinOrMetaTool is a hand-written list of tool names, and it had drifted
from the tools that actually ship: use_capability, compress and update_goal
were all missing. Every session carrying a skill that names one of them —
including the built-in review and security-review skills, which both list
use_capability — reported

  skill "review" allowed-tools references "use_capability" which is not in
  the current registry

on every doctor run, for a tool that is present.

Consult the built-in registry first, which cannot drift, and keep the list as
the fallback it already was. compress and update_goal are registered built-ins
and are now covered automatically; use_capability is wired up at boot rather
than registered, so it is added to the list explicitly with a note saying why.

Verified: 2 warnings before, 0 after, on an otherwise unchanged config.
@dadgrand
dadgrand force-pushed the fix/doctor-builtin-allowlist branch from 818d40f to e38b33e Compare September 1, 2026 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Go rewrite (1.x) — main-v2 branch, active development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant