Skip to content

fix(plugins): auto-add scheme to schemeless Hindsight api_url - #78979

Closed
thatssoheil wants to merge 1 commit into
NousResearch:mainfrom
thatssoheil:fix/hindsight-schemeless-api-url
Closed

thatssoheil wants to merge 1 commit into
NousResearch:mainfrom
thatssoheil:fix/hindsight-schemeless-api-url

Conversation

@thatssoheil

Copy link
Copy Markdown

Summary

Fixes #78967. A configured api_url without a scheme (e.g. "host:8888") reached the HTTP layer as an invalid URL, so every hindsight_recall / hindsight_retain / hindsight_reflect call failed with a cryptic URL-only error. hermes memory status still reported the provider as available, so nothing surfaced the misconfiguration until runtime.

Fix

Added _ensure_url_scheme() in plugins/memory/hindsight/__init__.py, applied at provider initialize() — the single assignment site of _api_url, covering config.json, HINDSIGHT_API_URL env var, and wizard-set values:

  • Local modes (local_embedded / local_external) → prepend http:// (self-hosted daemon, matches _DEFAULT_LOCAL_URL)
  • Everything else (cloud) → prepend https:// (matches _DEFAULT_API_URL)
  • Explicit schemes (anything with ://) → left untouched
  • Empty / missing → unchanged behavior

Tests

4 regression tests in tests/plugins/memory/test_hindsight_provider.py:

  • Schemeless url + local_external → http:// added
  • Schemeless url + cloud → https:// added
  • Explicit-scheme url preserved
  • Schemeless HINDSIGHT_API_URL env var normalized

All pass under scripts/run_tests.sh (CI parity). The 6 pre-existing failures in the file are missing hindsight_client_api module — verified identical on pristine main.

A configured api_url without a scheme (e.g. "host:8888") reached the HTTP
layer as an invalid URL and every hindsight_* tool failed with a cryptic
URL-only error. Normalize at provider init: prepend http:// for local modes
and https:// otherwise, matching the existing defaults. Explicit schemes are
left untouched.

Fixes NousResearch#78967.
Copilot AI lite review requested due to automatic review settings August 5, 2026 00:26

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers area/config Config system, migrations, profiles P3 Low — cosmetic, nice to have sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 5, 2026
@spfcraze

spfcraze commented Aug 5, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Open PR #78968, by the issue reporter, covers both of #78967's expected behaviors — including the wrong-but-valid-scheme hint this diff omits — so whichever of the two Fixes #78967 PRs merges first will auto-close the issue.

#78968 (fix(hindsight): validate and normalize api_url, fail fast on bad config) adds _normalize_api_url() and _diagnose_scheme_mismatch() to plugins/memory/hindsight/__init__.py; the latter probes the configured URL and, when the opposite scheme answers, logs the corrected URL — the issue's second expected-behavior bullet. This diff adds _ensure_url_scheme() at the same initialize() site.


Checked against 1effca3 — the tip of fix/hindsight-schemeless-api-url when this was written. Verified mechanically: the PR description, the issue text and the diff grep at the former; the state of the pull requests referenced as it stood then. The reading of intended scope is inference. If I have misread the intent here, please say so.

@thatssoheil

Copy link
Copy Markdown
Author

Closing as a duplicate of #78968.

#78968 (Realmagnum) was submitted earlier (00:07 UTC vs. my 00:26 UTC) and is a broader superset that covers both expected behaviors in the issue:

  • Normalizes missing schemes at init.
  • Validates that the scheme is one of http or https and raises a clean ValueError.
  • Implements _diagnose_scheme_mismatch() which probes the endpoint with both schemes to detect and diagnose scheme mismatch errors (e.g. https:// pointed at a plain-HTTP daemon).

Thanks to @Realmagnum for the more complete fix.

@thatssoheil thatssoheil closed this Aug 5, 2026
@thatssoheil
thatssoheil deleted the fix/hindsight-schemeless-api-url branch August 5, 2026 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/memory Memory tool and memory providers type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Hindsight api_url without a scheme makes every hindsight_* tool fail with a cryptic error

4 participants