Skip to content

Make host_id optional and improve tool parameter descriptions for LLMs#4585

Closed
amir20 wants to merge 2 commits intomasterfrom
claude/fix-llm-tool-parameters-QkDZW
Closed

Make host_id optional and improve tool parameter descriptions for LLMs#4585
amir20 wants to merge 2 commits intomasterfrom
claude/fix-llm-tool-parameters-QkDZW

Conversation

@amir20
Copy link
Copy Markdown
Owner

@amir20 amir20 commented Apr 5, 2026

LLMs often pass host name instead of host ID, or omit host_id entirely.
This change:

  • Makes host_id optional in all tools (fetch_container_logs, inspect,
    actions) since container IDs are unique across hosts
  • Adds findContainerFlexible helper that resolves host names to IDs
    and searches across all hosts when host_id is omitted
  • Improves parameter descriptions to clarify ID vs name distinction

https://claude.ai/code/session_01EoVbj5ecZzsZfroGiNALp3

claude added 2 commits April 5, 2026 02:31
LLMs often pass host name instead of host ID, or omit host_id entirely.
This change:
- Makes host_id optional in all tools (fetch_container_logs, inspect,
  actions) since container IDs are unique across hosts
- Adds findContainerFlexible helper that resolves host names to IDs
  and searches across all hosts when host_id is omitted
- Improves parameter descriptions to clarify ID vs name distinction

https://claude.ai/code/session_01EoVbj5ecZzsZfroGiNALp3
…ch all hosts

If the direct host lookup fails (wrong host ID, host name used instead, etc.),
simply fall back to searching across all hosts instead of trying to resolve
host names to IDs.

https://claude.ai/code/session_01EoVbj5ecZzsZfroGiNALp3
@claude
Copy link
Copy Markdown

claude bot commented Apr 5, 2026

  • Potential ambiguity: findContainerFlexible assumes container IDs are unique across hosts, but short/truncated IDs can collide. The first-match-wins logic at tools_helpers.go:89 could silently act on the wrong container.

  • Extra lookup overhead: When hostID is provided but wrong, fallback calls ListAllContainers (all hosts) + another FindContainer. Consider logging a warning when the provided host_id fails, so users can diagnose stale/incorrect tool calls.

  • Test gap: No test for fetch_container_logs or inspect_container without host_id, though the shared helper is covered via restart tests — acceptable.

  • Minor: findContainerFlexible re-validates containerID == "" internally when callers already check this — redundant.

@claude
Copy link
Copy Markdown

claude bot commented Apr 5, 2026

Review: Make host_id optional and improve tool parameter descriptions

  • findContainerFlexible (tools_helpers.go:67): redundant containerID == "" check — all callers already validate this before calling
  • findContainerFlexible fall-through: when direct lookup fails with a provided hostID, the code silently falls back to a full ListAllContainers scan. Worth considering whether to surface the original error if the host was explicitly provided (LLM passed a wrong host ID vs. omitted it) — silent fallback may mask bugs
  • Container ID uniqueness across hosts: comment says IDs are unique, but first-match-wins logic could silently pick the wrong container if IDs collide across hosts in edge cases; acceptable tradeoff but worth a comment
  • Tests are solid — covers both missing and wrong host cases

Overall clean change with good tests. The redundant check is minor nit.

@amir20 amir20 closed this Apr 5, 2026
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.

2 participants