Commit Graph
4 Commits
Author SHA1 Message Date
Adam SmithandClaude Opus 5 117780dd68 fix: backfill base_url on migration so pre-base_url entries still load
async_setup_entry does entry.data[CONF_BASE_URL] as a hard index, but
base_url is this fork's own addition: an entry created by a build from
before the base_url commits (48195c9 era, version 2 minor 7) has no such
key. On restart, setup raises KeyError: 'base_url' and the integration
fails to load -- reproduced in test first, where async_setup returns
False with that KeyError in the log.

Fixed with a migration step rather than a defensive .get(), matching the
pattern the rest of async_migrate_entry already uses, so the invariant
"a loaded entry has base_url" holds for every caller instead of each
call site guarding itself:

- new step 2.7 -> 2.8 backfills DEFAULT_BASE_URL via dict.setdefault, so
  an entry that already has a base_url keeps its own endpoint and only a
  missing one is filled. An install from before this feature was
  implicitly using api.openai.com, which is exactly DEFAULT_BASE_URL, so
  the backfill preserves existing behavior rather than repointing anyone.
- config_flow MINOR_VERSION 7 -> 8, otherwise the step never runs.

test_client_built_with_base_url now also covers the preserve path, since
its entry starts at minor 7 with a base_url set and must still reach the
client unchanged after migrating.

Also updated test_form's `minor_version == 7` assertion to 8; it pins
the version new entries are created at, so the bump is expected. The
reauth test's MockConfigEntry stays at minor 7 on purpose -- it stands
in for an already-existing entry.

Local: 138 passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NxJYWgGJtCw13qUHZtG8ki
2026-09-17 21:56:56 -04:00
Adam SmithandClaude Sonnet 5 b96b46495a fix: reauth must preserve base_url; reject credentials embedded in base_url
Critical (review finding 1): async_step_reauth_confirm showed
STEP_USER_DATA_SCHEMA directly instead of prefilling it via
add_suggested_values_to_schema (unlike async_step_user, which does),
so the reauth form displayed DEFAULT_BASE_URL rather than the entry's
configured endpoint. A user rotating their API key would have their
integration silently repointed from e.g. LiteLLM back to
api.openai.com on save. Fixed by prefilling from
self._get_reauth_entry().data, same as the user step. strings.json's
reauth_confirm block now documents the base_url field too.
Regression test: test_reauth_preserves_base_url (asserts both the
shown form's suggested_value and the final stored base_url).

Important (review finding 2): base_url is user-suppliable and could
carry embedded user:pass@ credentials that later surface through
vendored exception logging (e.g. entity.py's "Error talking to
OpenAI: %s") this fork does not and should not edit. Rather than
touch vendored logging, reject credentials at the boundary:
normalize_base_url now raises the new CredentialsInBaseURL (a
subclass of InvalidBaseURL) when the URL has a username or password,
mapped to a distinct "credentials_in_url" form error telling the user
to use the API key field instead. Tests added in test_url_util.py and
test_config_flow.py.

Important (review finding 3): documented on sanitize_url_for_logging
that it is not a general-purpose credential scrubber -- it only masks
what urlparse recognizes as a netloc's user:pass@ authority component,
so a scheme-less/relative string with embedded credentials passes
through unmasked. Safe today because every caller normalizes first
(which now also rejects embedded credentials outright), but it is
exported for a later task's use and must not be assumed bulletproof
on an arbitrary unvalidated string.

Not fixed (reviewer's Minor 4, by direction): the default-config setup
call reaching api.openai.com with Authorization: Bearer not-required
when both base_url and api_key are left at defaults -- cosmetic, and
correctly surfaces as invalid_auth.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01739Seac5eMAfNwFeiwV5r4
2026-09-17 18:08:20 -04:00
Adam SmithandClaude Sonnet 5 564c52159f feat: configurable base_url threaded to all three client sites
Adds CONF_BASE_URL/DEFAULT_BASE_URL and threads base_url through every
AsyncOpenAI construction site: config_flow.validate_input,
__init__.async_setup_entry, and config_flow's web-search geolocation
helper (_get_location_data) — the easy one to miss, since it would
otherwise silently talk to api.openai.com when the user configured a
different endpoint.

Fixes upstream-ported fixtures (conftest.mock_config_entry,
test_config_flow's inline MockConfigEntry/data-schema assertions) to
supply base_url now that the config-flow schema requires it.

No translations/en.json exists in this repo (only strings.json), so
only strings.json got the new "base_url" UI strings.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01739Seac5eMAfNwFeiwV5r4
2026-09-17 17:50:38 -04:00
Adam SmithandClaude Sonnet 5 fa9648213d test: port core's openai_conversation suite as regression net
Ported from home-assistant/core tag 2026.9.2 (tests/components/openai_conversation):
conftest.py fixtures merged into tests/conftest.py, test_config_flow.py,
test_conversation.py and test_ai_task.py copied and rewritten for the fork's
custom_components.openai_compatible / "openai_compatible" domain.

Also copied as local test infrastructure (not core-only, self-contained):
- tests/__init__.py: the openai_conversation test package's own event-builder
  helpers (create_message_item, create_function_tool_call_item, etc.) that
  test_conversation.py and test_ai_task.py import via `from . import ...`.
- tests/snapshots/test_conversation.ambr: just the test_model_args snapshot
  entry, the one surviving syrupy-based assertion (see deletions below).

Deleted from test_conversation.py — depend on tests.components.conversation's
MockChatLog/mock_chat_log fixture, which lives in home-assistant/core's own
tests tree and is not distributable via pytest-homeassistant-custom-component
(confirmed: not present in the installed package's file list):
- test_function_call
- test_function_call_without_reasoning
- test_reasoning_summary_off_omits_summary_key
- test_web_search
- test_web_search_remove_citations_gpt5
- test_code_interpreter

Their now-unused imports (create_code_interpreter_item, create_web_search_item,
CONF_CODE_INTERPRETER, CONF_REASONING_SUMMARY, the CONF_WEB_SEARCH* constants,
freezegun.freeze_time, ToolInput, datetime) were removed with them.

`from tests.common import MockConfigEntry` (core's own tests package, also not
shipped by pytest-homeassistant-custom-component) rewritten to
`from pytest_homeassistant_custom_component.common import MockConfigEntry`,
matching the convention already used in this fork's tests/test_init.py.

Not run locally: this workstation is Python 3.13, HA 2026.9 (and
pytest-homeassistant-custom-component) require 3.14, so no correct local
interpreter exists. Validated with `python3 -m py_compile` only; CI
(actions/setup-python@3.14) is the real test run.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01739Seac5eMAfNwFeiwV5r4
2026-09-17 17:36:52 -04:00