Skip to content

fix(auth): memoize resolve_nous_access_token to collapse startup burst (salvage #66016) - #76930

Merged
kshitijk4poor merged 2 commits into
NousResearch:mainfrom
kshitijk4poor:salvage-66016-token-memo
Aug 2, 2026
Merged

kshitijk4poor merged 2 commits into
NousResearch:mainfrom
kshitijk4poor:salvage-66016-token-memo

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Contributor

Salvages #66016 by @JeffStone69 — commit cherry-picked to preserve authorship, plus one follow-up commit (fast-path memoization + tests).

Context — what this fixes, for whom

Every startup, check_tool_availability runs one check_fn per managed tool (browser, image_gen, video_gen, tts, stt, ...), and each independently calls resolve_nous_access_token(). Every call pays two cross-process file locks + state-file reads — and when the stored token is expired, each triggers its own ~15s blocking OAuth refresh round-trip. On slow/constrained hosts that serial burst stretches startup to minutes.

What the fix does (from #66016, kept verbatim)

Adds a 5s-TTL, thread-safe, per-process memo to resolve_nous_access_token(): the first call resolves, subsequent calls within the TTL return the cached token. Correctly scoped — only the default path is cached; insecure/ca_bundle callers bypass entirely; failed refreshes never populate it. Follows the established nous_billing.py 30s-cache precedent.

Follow-up commit (ours)

  1. Memoize the valid-token fast path too. The original memo only populated after a refresh — but the startup burst usually finds a valid token, and that return path still paid the full lock/read cost per check_fn. The token has ≥refresh_skew_seconds (120s) of life at that return, so a 5s memo can never serve an expired token.
  2. Test-suite adaptation: test_nous_portal_staging_allowlist.py's refresh-capture tests now clear the module-level memo first (with the fast-path populate, cross-test memo pollution made 3 of them fail — caught during review).
  3. Memo behavior tests (the PR shipped none): TTL hit collapses to one state transaction, TTL expiry re-resolves, insecure bypass. Mutation-checked: all 3 fail against main's un-memoized function.

Verification

  • tests/hermes_cli/test_resolve_token_memo.py + test_nous_portal_staging_allowlist.py: 7 passed; broader auth suites (test_nous_account, test_auth_profile_fallback, test_managed_tool_gateway): 42 passed
  • ruff clean; thundering-herd note: two threads racing a cold memo can still both refresh (lock isn't held across the network call — correct, holding it would serialize a 15s call); the burst is serial in practice so the memo collapses it fully

Closes #66016 (superseded by this salvage — original author credited via cherry-pick authorship).

JeffStone69 and others added 2 commits August 2, 2026 22:09
check_tool_availability runs once per managed-tool check_fn (browser,
image_gen, etc.) during banner render. Each one independently triggers a
~15s blocking Nous Portal token-refresh network call when the stored token
is expired. On a slow/constrained host (e.g. a small monitoring CT) that
serial burst stretched startup to many minutes, appearing 'stalled'.

Add a per-process memo (5s TTL) so the burst collapses into a single network
round-trip. Only successful, non-forced resolutions are cached; force_fresh
and insecure/ca_bundle callers bypass and don't populate the cache, so
normal refresh semantics are unchanged.

Verified: 3 rapid resolve_nous_access_token() calls -> 1 underlying refresh.
Follow-ups on the startup-burst memo:

- Populate the memo on the valid-token fast path as well. The startup
  burst usually finds a VALID token, and each check_fn call still paid
  two cross-process file locks + state reads to reach that return; the
  original memo only engaged after a refresh. The token has at least
  refresh_skew_seconds (>=120s) of life at that return, so a 5s memo can
  never serve an expired token.
- Clear the module-level memo in test_nous_portal_staging_allowlist's
  refresh-capture helper: with the fast-path populate, a token memoized
  by an earlier test would otherwise short-circuit the refresh these
  tests assert on (3 tests failed without this).
- Add dedicated memo behavior tests (TTL hit, TTL expiry, insecure
  bypass) — the original PR shipped none. Mutation-checked: all 3 fail
  against main's un-memoized function, pass on this branch.
@kshitijk4poor
kshitijk4poor enabled auto-merge (rebase) August 2, 2026 16:39
@kshitijk4poor
kshitijk4poor merged commit 06b4f64 into NousResearch:main Aug 2, 2026
35 checks passed
@kshitijk4poor
kshitijk4poor deleted the salvage-66016-token-memo branch August 5, 2026 07:08
teknium1 pushed a commit that referenced this pull request Sep 11, 2026
…file

resolve_nous_access_token()'s 5s startup-burst memo (#76930) cached the
resolved Nous Portal access token in a single module-level slot keyed
by nothing but wall-clock time. The underlying resolution is
profile-scoped: _auth_file_path() reads get_hermes_home(), which
checks the context-local _HERMES_HOME_OVERRIDE ContextVar before
falling back to the HERMES_HOME env var — gateway/run.py and
tui_gateway/server.py set that override per-profile for multiplex
concurrency.

In a multiplex gateway serving two profiles with different
authenticated Nous accounts, if profile A's context resolves a token
and profile B's context calls resolve_nous_access_token() within the
next 5 seconds, profile B received profile A's cached token — used to
authenticate against the managed tool gateway / relay self-provisioning
under the wrong account.

Key the memo by str(get_hermes_home()) instead of a single slot, so
each profile's context reads only its own cached token. The lock
around read/write is unchanged; only the cache's shape moved from a
single (timestamp, token) tuple to a dict keyed by resolved home.

(cherry picked from commit 9d8846b)
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