fix: hybridize background profile env routing

[Michaelyklam]

Thinking Path

• Background title generation, manual compression, and update-summary workers need to honor a session's non-default profile.
• The pure thread-local refactor for Refactor profile_env_for_background_worker to thread-local env (eliminate cross-profile os.environ race) #2321 was reverted because hermes_cli.config.load_config() still reads HERMES_HOME from process env.
• The first hybrid pass proved that config loading worked, but review caught that provider credential readers still call os.getenv() directly.
• The follow-up keeps WebUI thread-local state for WebUI helpers and mirrors the profile runtime env into process env during the worker body so existing Hermes provider/auth readers see the correct profile credentials.
• The _ENV_LOCK critical section remains short: it serializes setup/restore of env and skill-home module caches, but does not wrap the whole worker body.

What Changed

• Updated profile_env_for_background_worker() to set thread-local profile runtime env and mirror runtime env into os.environ for the worker body.
• Restores prior runtime env values after the worker exits, including preserving default-profile credentials when they existed before the worker.
• Preserved skill-home module snapshot/patch/restore behavior for profile-scoped skill imports.
• Added regression coverage proving both hermes_cli.config.load_config() and os.getenv()-style provider credential readers see the session profile.
• Updated manual-compression and update-summary tests to assert the revised hybrid contract.

Why It Matters

Non-default profile background workers previously had two bad choices: broad process-env mutation with race risk, or thread-local-only env that production Hermes readers did not actually consult. This PR takes the current compatibility path: the worker sees the profile config and provider credentials used by existing Hermes Agent code, while setup/restore is bounded and covered by regressions.

Closes #2321.

Verification

env -u HERMES_CONFIG_PATH -u HERMES_WEBUI_HOST /home/michael/.hermes/hermes-agent/venv/bin/python -m pytest tests/test_profile_terminal_env.py tests/test_sprint29.py tests/test_issue2024_env_lock_skill_imports.py tests/test_sprint46.py::test_manual_compress_worker_uses_session_profile_env tests/test_title_aux_routing.py::TestBackgroundTitleProfileRouting tests/test_update_banner_fixes.py::TestUpdateSummaryRouteModelSelection::test_summary_route_auxiliary_model_uses_active_profile_env -q
/home/michael/.hermes/hermes-agent/venv/bin/python -m py_compile api/profiles.py
git diff --check origin/master...HEAD
git merge-tree --write-tree origin/master HEAD

Result on follow-up head 5bd1f14:

81 passed in 2.56s
py_compile api/profiles.py passed
git diff --check origin/master...HEAD passed
git merge-tree --write-tree origin/master HEAD passed

CI is running on the new head.

UI media: not applicable; backend/profile-env behavior only.

Risks / Follow-ups

• This is still a compatibility compromise: profile runtime env keys are process-global during the worker body because current Hermes provider/auth readers still use os.getenv().
• The long-term clean fix is for Hermes Agent config/auth resolution to consult an opt-in thread-local/profile context before falling back to os.environ.
• Any future background worker tests should exercise production config and credential readers, not only mocks that read _thread_ctx.env.

Model Used

AI-assisted change with repository inspection, targeted editing, GitHub issue/PR review, and shell-based test verification.

[Michaelyklam]

Rebased this branch on current master and resolved the CHANGELOG.md conflict from the v0.51.74 release batch.

What changed in the conflict repair:

• kept the new upstream v0.51.74 release section intact
• kept this PR's release note under [Unreleased]
• left the profile-env implementation/tests otherwise unchanged

Verification on rebased head 9894157:

• pytest ... -q targeted profile/background env suite — 81 passed
• py_compile api/profiles.py
• git diff --check origin/master...HEAD
• git merge-tree --write-tree origin/master HEAD

CI is running again on the rebased head.

[nesquena-hermes]

Reviewing this as the "Option B" follow-up to the #2321 / reverted-#2323 conversation.

Pulled cron-pr-2368 (head 98941571 after rebase). CI green on 3.11/3.12/3.13. Walked the diff in api/profiles.py against origin/master and against the reverted #2323 shape.

Where the PR succeeds

The change correctly addresses Opus's catch from the #2321 reopen — that hermes_cli.config.load_config() resolves the profile config via os.environ["HERMES_HOME"] only. Keeping HERMES_HOME mutation under _ENV_LOCK while moving the rest to thread-local means get_hermes_home() at hermes_constants.py:30 (val = os.environ.get("HERMES_HOME", "").strip()) still finds the correct profile root:

# api/profiles.py:716-720 (PR)
thread_env = dict(runtime_env)
thread_env["HERMES_HOME"] = str(profile_home_path)
...
_set_thread_env(**thread_env)
with _ENV_LOCK:
    had_hermes_home = "HERMES_HOME" in os.environ
    old_hermes_home = os.environ.get("HERMES_HOME")
    ...
    os.environ["HERMES_HOME"] = str(profile_home_path)

And the new integration test at tests/test_title_aux_routing.py:505-553 (test_background_profile_env_routes_load_config_without_process_env_leak) is the right shape — it actually calls hermes_config.load_config() without mocking it and asserts loaded.get('model', {}).get('provider') == 'profile-provider'. That's the acceptance criterion #1 the maintainer issue body called out.

Concern: the test doesn't cover provider-credential resolution

get_profile_runtime_env() at api/profiles.py:632-672 reads both terminal.* keys from config.yaml AND every line from the profile's .env — which in practice means OPENROUTER_API_KEY, ANTHROPIC_API_KEY, OPENAI_API_KEY, etc. for non-default profiles. Pre-PR, those keys landed in os.environ for the duration of the worker body. Post-PR, they land in _thread_ctx.env.

The problem is the same shape as Opus's original catch, just for a different code path. No production reader consults _thread_ctx.env. Grep confirms it:

$ grep -rn "_thread_ctx\|thread_ctx.env" api/ --include="*.py" | grep -v test_
api/config.py:3973:_thread_ctx = threading.local()
api/config.py:3977:    _thread_ctx.env = kwargs
api/config.py:3981:    _thread_ctx.env = {}
api/profiles.py:701:        from api.config import _clear_thread_env, _set_thread_env, _thread_ctx
api/profiles.py:726:    previous_thread_env = getattr(_thread_ctx, "env", {}).copy()

It is written and cleared, nothing reads it.

For provider auth specifically, the chain is:

1. agent/auxiliary_client.py:1417 (_try_openrouter):

   or_key = explicit_api_key or os.getenv("OPENROUTER_API_KEY")

   Direct os.getenv only. With this PR, a background title-gen worker for a non-default-profile session will see the default profile's OPENROUTER_API_KEY (from the cookie's last _reload_dotenv at api/profiles.py:783-814), not the work profile's.

2. hermes_cli/auth.py:469-471 (get_anthropic_key and similar provider key resolvers):

   for var in PROVIDER_REGISTRY["anthropic"].api_key_env_vars:
       value = get_env_value(var) or os.getenv(var, "")

   get_env_value at hermes_cli/config.py:4727-4734 checks os.environ first, falling back to load_env(). So if ANTHROPIC_API_KEY is in os.environ (which it normally is for the cookie's active profile), it's returned — not the worker's session profile's key. The load_env() fallback only triggers when the key is absent from os.environ, which is the uncommon path.

So when cookie-profile == session-profile (the simple single-tab case), the PR works because os.environ already has the right values from _reload_dotenv. When cookie-profile ≠ session-profile (concurrent tabs on different profiles, the multi-profile race that motivated #2321 in the first place), background workers for the non-default-profile session will silently authenticate against the default profile's credentials.

Suggested acceptance test extension

The maintainer's acceptance criterion #1 said "calls hermes_cli.config.load_config() (or whatever production reader the worker uses)". For the actual API-key reader path, a sibling test would be:

def test_background_profile_env_routes_provider_credentials(...):
    # Set os.environ['OPENROUTER_API_KEY']='default-key' (cookie's active profile)
    # Write profile-home/.env with OPENROUTER_API_KEY='work-key'
    # Enter profile_env_for_background_worker(session, ...)
    # Call agent.auxiliary_client._try_openrouter() (or equivalent)
    # Assert the client was constructed with 'work-key', not 'default-key'

Today that test would fail under this PR, which is the regression-reveal the maintainer asked for in criterion #2. The fix is either (a) restore os.environ.update(runtime_env) under the lock (accepting the narrow race for non-HERMES_HOME keys, which is Option A from the issue conversation), or (b) push for Option C — teach auxiliary_client._try_openrouter and get_env_value to consult thread-local before falling back to os.environ.

Verdict

For sessions where the cookie and session profile match, this PR is correct and the load_config test pins the desired contract. For concurrent multi-profile workloads — which is the original #2321 motivation — the runtime-env keys move from "racy os.environ" to "thread-local nobody reads", which trades one bug for another.

I'd suggest either:

• Narrowing the PR title/scope to "route HERMES_HOME for background workers via thread-local plus narrow lock" and explicitly document that profile API keys still rely on the cookie's _reload_dotenv being in sync with session.profile (and add a regression test that demonstrates this limitation, so the next reviewer doesn't have to rediscover it); or
• Pulling the cross-repo Option C change into this PR so the thread-local channel actually gets consulted by auxiliary_client._try_openrouter and hermes_cli/auth.get_env_value (this is the agent-side change the maintainer mentioned in the issue).

Either way, the current test surface gives more comfort than is warranted given the production reader gap. Happy to look at a follow-up that closes the credential path or scopes the contract down explicitly.

[Michaelyklam]

Pushed a follow-up for the credential-reader gap you caught.

What changed:

• profile_env_for_background_worker() now keeps the thread-local profile env for WebUI helpers and mirrors the session profile runtime env into os.environ for the worker body, because current provider/auth readers still use os.getenv() directly.
• Prior env values are restored after the worker exits, so default-profile credentials are preserved.
• The load-config regression now also asserts an os.getenv('OPENROUTER_API_KEY')-style credential reader sees the session profile key, not the default-profile key.
• Manual compression and update-summary worker tests were updated to assert the revised hybrid contract.
• PR body is updated to stop claiming non-HERMES_HOME keys stay out of process env.

Verification on 5bd1f14:

• pytest ... -q targeted profile/background env suite — 81 passed
• py_compile api/profiles.py
• git diff --check origin/master...HEAD
• git merge-tree --write-tree origin/master HEAD

CI is running on the new head now.

[nesquena-hermes]

Summary

Reading the follow-up commit 5bd1f144 against 98941571 (the previous head when I left the credential-reader review), this addresses the gap I called out cleanly. The hybrid contract is now: thread-local _thread_ctx.env carries the profile env for any WebUI helper that wants to consult it, and os.environ is mirrored under _ENV_LOCK for the duration of the worker body so existing os.getenv-based provider/auth readers see the right values. Prior process-env values are snapshotted and restored on exit.

CI is green on the new head.

Code references

Hybrid setup/restore — api/profiles.py:716-762

The critical section now mutates both layers atomically:

try:
    _set_thread_env(**thread_env)
    with _ENV_LOCK:
        old_runtime_env = {key: os.environ.get(key) for key in runtime_env}
        had_hermes_home = "HERMES_HOME" in os.environ
        old_hermes_home = os.environ.get("HERMES_HOME")
        skill_home_snapshot = snapshot_skill_home_modules()
        os.environ.update(runtime_env)
        os.environ["HERMES_HOME"] = str(profile_home_path)
        ...

And the restore mirrors that exactly, popping keys that were absent before and restoring previous values for keys that were already set. The _ENV_LOCK scope is still narrow — only setup and restore are serialized, not the worker body — so concurrent workers on different profiles will queue at the boundaries, which is acceptable.

Credential-reader regression — tests/test_title_aux_routing.py:test_background_profile_env_routes_load_config_and_provider_credentials

This now exercises the actual production reader path I was worried about:

runtime_env = {
    'PROFILE_ONLY_KEY': 'profile-only',
    'OPENROUTER_API_KEY': 'profile-openrouter-key',
}
with patch.dict(os.environ, {'HERMES_HOME': default_home, 'OPENROUTER_API_KEY': 'default-openrouter-key'}, ...):
    with profiles.profile_env_for_background_worker(session, 'background title'):
        loaded = hermes_config.load_config()
        captured['provider_credential'] = os.getenv('OPENROUTER_API_KEY')
        ...
        captured['restored_provider_credential'] = os.environ.get('OPENROUTER_API_KEY')

self.assertEqual(captured['provider_credential'], 'profile-openrouter-key')
self.assertEqual(captured['restored_provider_credential'], 'default-openrouter-key')

That's exactly the assertion pair I would have asked for: profile credential during the worker, default credential restored after. The default-profile OPENROUTER_API_KEY='default-openrouter-key' proves the snapshot/restore handles the "key existed before, with a different value" case correctly, not just the "key absent before" case.

Remaining tradeoff

This is still a compatibility compromise rather than the clean fix, and the PR description now says so explicitly: profile runtime-env keys are process-global during the worker body. That means two concurrent workers on different profiles, executing simultaneously inside the body (past the lock), will both see whichever one entered the worker body second. The original #2321 race isn't fully eliminated — it's been narrowed to the worker-body duration rather than across cookie reloads.

The long-term fix is Option C: teach agent.auxiliary_client._try_openrouter (and the hermes_cli/auth.py:469-471 provider-key resolvers, and hermes_cli/config.py:get_env_value at ~4727) to consult a thread-local profile context before falling back to os.environ. The PR's "Risks / Follow-ups" section calls this out, which is the right disclosure.

For shipping today, this is a substantial improvement over both the original broad-env approach and the pure thread-local refactor that lost the production readers. The narrow lock + snapshot-restore is the best available shape until the agent-side readers grow a thread-local consultation hook.

Verdict

The follow-up cleanly addresses the regression-reveal I asked for. The 81-test pass and the explicit os.getenv() credential assertion are exactly what was needed. The remaining limitation (concurrent multi-profile race inside the worker body) is now documented as a known compromise rather than hidden behind a "fully thread-local" claim.

LGTM, modulo the maintainer's final review on accepting the narrow-race tradeoff.