Skip to content

fix(gateway): install _profile_runtime_scope in _run_background_task when multiplexing is active - #60746

Closed
liuhao1024 wants to merge 2 commits into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-60726-multiplex-background-scope
Closed

fix(gateway): install _profile_runtime_scope in _run_background_task when multiplexing is active#60746
liuhao1024 wants to merge 2 commits into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-60726-multiplex-background-scope

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

When multiplexing is enabled (multiplex_profiles: true), the /background command spawns an async task that calls _resolve_session_agent_runtime() without installing a profile secret scope. This causes credential reads like get_secret('OPENROUTER_BASE_URL') to raise UnscopedSecretError, breaking the background task.

This fix mirrors the pattern already used by _run_agent in gateway/run.py: wrap the entire background task execution in _profile_runtime_scope when multiplexing is active. The scope is installed for the source's profile home, ensuring all credential reads resolve from the correct profile's .env file while preserving cross-profile isolation.

Related Issue

Fixes #60726

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • gateway/run.py: Refactored _run_background_task to install _profile_runtime_scope when multiplex_profiles is true, following the same pattern as _run_agent. The original implementation is now in _run_background_task_inner.
  • tests/gateway/test_multiplex_background_task_scope.py: Added regression tests verifying that _profile_runtime_scope is called when multiplexing is active and bypassed when disabled.

How to Test

  1. Unit tests:

    cd /Users/liuhao/.hermes/workdir/hermes-agent
    python -m pytest tests/gateway/test_multiplex_background_task_scope.py -xvs

    Observed result: Both tests pass, confirming the scope is correctly installed.

  2. Signature verification:

    cd /tmp/hermes-bugfix-AufX3f
    git log --oneline upstream/main -1

    Observed result: HEAD at upstream/main (latest).

  3. Multiplex scenario (requires two profiles configured):

    • Configure Hermes with multiplex_profiles: true and two profiles (prof_a, prof_b), each with different provider credentials in their .env files
    • Send /background summarize recent messages command in a platform channel belonging to prof_b
    • Before fix: Background task fails with UnscopedSecretError: get_secret('OPENROUTER_BASE_URL') called with no profile secret scope active
    • After fix: Background task completes successfully, using prof_b's credentials

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15.2

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

For New Skills

  • This skill is broadly useful to most users (if bundled) — see Contributing Guide
  • SKILL.md follows the standard format (frontmatter, trigger conditions, steps, pitfalls)
  • No external dependencies that aren't already available (prefer stdlib, curl, existing Hermes tools)
  • I've tested the skill end-to-end: hermes --toolsets skills -q "Use the X skill to do Y"

Screenshots / Logs

Error before fix (from issue #60726):

❌ Background task bg_134707_4da0e1 failed: get_secret('OPENROUTER_BASE_URL') called with no profile secret scope active
while multiplexing is on. This credential read must run inside a set_secret_scope(...) block (the per-turn / per-adapter
profile scope). Reading os.environ here would risk leaking another profile's value.

After fix: Background task resolves credentials from the correct profile's .env file via _profile_runtime_scope, following the same isolation pattern used by per-turn agent runs.

…when multiplexing is active

When multiplex_profiles is true, background tasks spawned by /background
command failed with UnscopedSecretError because _resolve_session_agent_runtime()
was called without a profile secret scope. This fix wraps the task in
_profile_runtime_scope, mirroring the pattern used by _run_agent.

Fixes NousResearch#60726
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery area/auth Authentication, OAuth, credential pools sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jul 8, 2026
…compatibility

GatewayRunner.__init__ now converts dict inputs to GatewayConfig objects,
ensuring self.config.default_reset_policy is always an object, not a dict.
This fixes AttributeError when tests pass config={"multiplex_profiles": True}.

The fix is minimal and maintains backward compatibility: existing callers
passing GatewayConfig or None are unaffected.

Also updated test to mock _resolve_profile_home_for_source and expect Path
objects, matching actual runtime behavior.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for tracing this to the detached /background path. The premise is confirmed on current main: gateway/run.py:13244 resolves runtime credentials before any profile scope is installed, while agent/secret_scope.py:149-155 intentionally fails closed for that condition. The proposed wrapper matches the existing _run_agent scope pattern at gateway/run.py:16835-16847.

Problems

  • gateway/run.py:2805 adds raw-dict support to GatewayRunner.__init__ solely for the test. Production config loading instead uses GatewayConfig.from_dict(...) at gateway/run.py:20841; direct dataclass construction bypasses that parser's normalization (gateway/config.py:910-932).
  • tests/gateway/test_multiplex_background_task_scope.py mocks _profile_runtime_scope, so it verifies the call site but not that the inner task actually runs with a secret scope active.

Suggested changes

  • Build the test config as GatewayConfig(multiplex_profiles=...) and remove the constructor API change.
  • In the multiplex-enabled test, use the real scope and assert current_secret_scope() or a profile-scoped secret from inside the inner task.

Automated hermes-sweeper review.

Comment thread gateway/run.py

def __init__(self, config: Optional[GatewayConfig] = None):
global _gateway_runner_ref
# Support dict input for test compatibility; convert to GatewayConfig

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This production API change is only needed by the new test. Please construct GatewayConfig(multiplex_profiles=...) in the test instead; real config mappings are normalized through GatewayConfig.from_dict, not direct dataclass construction.


# Mock _resolve_profile_home_for_source to return a known path
with mock.patch.object(gw, "_resolve_profile_home_for_source", return_value=Path("/fake/profile")):
# Mock _profile_runtime_scope

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mocking _profile_runtime_scope proves the wrapper invokes it, but not that the inner task sees an active secret scope. Prefer a real scope and assert current_secret_scope() or a profile-scoped credential from the inner coroutine.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 10, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #65721 — your commit was cherry-picked onto current main with your authorship preserved in git log (rebase merge). Your scope-wrapper design mirrored the per-turn _run_agent pattern exactly, which is the established seam for this bug class (same family as the api_server default-listener fix in #65700 and your own cron scope fix earlier).

One change during salvage: your second commit (the GatewayRunner.__init__ dict-coercion for test compatibility) was dropped — the tests were rewritten on the object.__new__ bare-runner pattern the suite already uses, so no production code changes for test convenience. The fix commit itself landed intact. Closes #60726. Thanks — fourth clean fix from you in this area!

@teknium1 teknium1 closed this Jul 16, 2026
@teknium1 teknium1 added the area/install-update Installer, updater, packaging, wheels, doctor label Jul 19, 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 area/install-update Installer, updater, packaging, wheels, doctor comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Background task () fails with UnscopedSecretError when multiplexing is on

3 participants