Skip to content

fix(aux): key_cmd token-source callables no longer crash on .strip() - #113993

Closed
kvnloo wants to merge 1 commit into
NousResearch:mainfrom
kvnloo:fix/keycmd-strip-guard-113976
Closed

kvnloo wants to merge 1 commit into
NousResearch:mainfrom
kvnloo:fix/keycmd-strip-guard-113976

Conversation

@kvnloo

@kvnloo kvnloo commented Sep 17, 2026

Copy link
Copy Markdown

What

key_cmd configured providers crashed auxiliary calls when a CommandTokenSource callable reached the resolution path: .strip() was called unconditionally on a non-string. New _strip_if_str helper strips strings and passes callables through untouched, applied at all three crash sites (_resolve_custom_branch, _resolve_named_custom_branch, _resolve_registry_branch).

Related Issue

Fixes #113976

Type

  • Bug fix

Changes

  • agent/auxiliary_client.py (+12/-3): _strip_if_str helper + 3 call sites

How to Test

  • scripts/run_tests.sh tests/agent/test_auxiliary_client.py — 2 new invariant tests pass; both fail on base with the issue's exact AttributeError: 'CommandTokenSource' object has no attribute 'strip'
  • regression: test_auxiliary_client.py + test_command_token_source.py = 243 passed, 2 failed (both verified pre-existing on base, untouched files); 4 additional aux-resolution suites 136/136 green

Follow-up (not in scope here): str() coercion of the callable in hermes_cli/runtime_provider.py:640 and related paths sends the callable's repr as a Bearer token and fails auth silently — behavioral change, worth a separate decision.

Checklist

  • Tests added
  • All existing tests pass
  • Code follows repo conventions

authored with AI assistance (Muse, Meta's Muse Spark) under the contributor's direction; reviewed and approved by the contributor before submission.

…n unstripped

A provider configured with key_cmd hands the resolver a CommandTokenSource (a
lazy callable), not a str. The custom, named-custom, and registry branches of
resolve_provider_client called .strip() on it and raised AttributeError,
silently degrading title generation, smart approvals, and vision checks.

Add a _strip_if_str helper — strings are stripped, callables pass through
untouched (the wire clients accept a callable API key) — and use it at all
three sites. Matches the existing idiom at the runtime record builder.

Fixes NousResearch#113976.

authored with AI assistance (Muse, Meta's Muse Spark) under the contributor's direction; reviewed and approved by the contributor before submission.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/auth Authentication, OAuth, credential pools duplicate This issue or pull request already exists labels Sep 17, 2026
@alt-glitch

Copy link
Copy Markdown

This was generated by AI during triage.

Duplicate of #107344: both preserve callable key_cmd credentials at the same three auxiliary provider-resolution .strip() sites; #107344 also covers additional callable propagation.

@ehz0ah

ehz0ah commented Sep 17, 2026

Copy link
Copy Markdown

The callable still breaks in two auxiliary paths, so this does not yet fix #113976 end to end. First, _to_async_client() rebuilds AsyncOpenAI from sync_client.api_key. The OpenAI client stores a callable in _api_key_provider while api_key is an empty string. A direct probe at this head produced an async client with no callable provider and empty auth_headers, so async auxiliary calls are unauthenticated. Second, the main-runtime reuse branch still runs str(main_runtime.get("api_key") or "").strip() at agent/auxiliary_client.py:4796. A direct resolver probe converted a CommandTokenSource into its object representation, which would be sent as the bearer token. The new tests verify pass-through at the three changed strip sites, but they do not exercise the async client used by auxiliary calls or the main-runtime reuse path. Please preserve the callable token source through both paths and add regressions that inspect the effective async authorization behavior and the credential passed by main-runtime reuse.

@alt-glitch alt-glitch removed the duplicate This issue or pull request already exists label Sep 18, 2026

kvnloo commented Sep 20, 2026

Copy link
Copy Markdown
Author

Closing as implemented/superseded on main. #114333 landed the callable key_cmd normalization across the five current resolution shapes with credit to the earlier contributor work; its merge SHA is 62f7e04f843a7aeba98a089a639bfe1af51390ff. The async-client credential plumbing had also moved under #113969, so carrying this older partial branch now only adds duplicate review surface.

@kvnloo kvnloo closed this Sep 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

key_cmd providers break auxiliary calls: CommandTokenSource is treated as a str

3 participants