Skip to content

fix(gateway): preserve context during manual compression - #69847

Closed
dyreckt wants to merge 1 commit into
NousResearch:mainfrom
dyreckt:fix/gateway-compression-contextvars
Closed

dyreckt wants to merge 1 commit into
NousResearch:mainfrom
dyreckt:fix/gateway-compression-contextvars

Conversation

@dyreckt

@dyreckt dyreckt commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Preserves the caller's contextvars when the gateway /compress command moves _compress_context() to a worker thread.

The manual compression path used loop.run_in_executor() with a plain lambda. Executor workers do not inherit the caller context, so multiplexed gateway state such as the profile-specific secret scope disappeared in the worker. Auxiliary compression could then report a configured provider credential as missing even though it resolved correctly in the routed profile.

asyncio.to_thread() is the smallest canonical fix because it retains the non-blocking worker boundary while propagating the current context.

Related Issue

No issue filed.

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

  • Replace the manual compression run_in_executor() call in gateway/slash_commands.py with context-preserving asyncio.to_thread().
  • Add a regression test in tests/gateway/test_compress_command.py proving that caller context is visible inside the compression worker.

How to Test

  1. Run uv run --extra dev pytest tests/gateway/test_compress_command.py::test_compress_command_preserves_caller_context_in_worker -q.
  2. Run uv run --extra dev pytest tests/gateway/test_compress_command.py -q.
  3. Run the gateway slash-command test group.

Before the fix, the regression test failed with:

assert [None] == ['profile-secret-scope']

After the fix:

12 passed in 0.45s

The broader gateway slash-command tests also passed: 133 passed.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • Documentation update: N/A
  • cli-config.yaml.example update: N/A
  • CONTRIBUTING.md / AGENTS.md update: N/A
  • Cross-platform impact considered: asyncio.to_thread() is standard Python and preserves the existing worker-thread behavior
  • Tool descriptions/schemas update: N/A

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jul 23, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for isolating the manual /compress context-loss path. The proposed asyncio.to_thread() change correctly preserves ContextVars while retaining the existing default-executor boundary at gateway/slash_commands.py:3979.

Problems

  • The same bug class remains in automatic session-hygiene compression: gateway/run.py:16156-16163 invokes _hyg_agent._compress_context(...) through plain run_in_executor(None, ...). Profile secrets are ContextVar-scoped and fail closed when no scope is present (agent/secret_scope.py:55-57, agent/secret_scope.py:166-174), so this sibling worker needs equivalent propagation.

Suggested changes

  • Preserve context for the hygiene compression worker at gateway/run.py:16156 and add a regression test for that path. The existing gateway helper at gateway/run.py:20278-20287 shows the repository's explicit copy_context() executor pattern.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/compression Context compression and continuation sessions labels Jul 30, 2026
@dyreckt
dyreckt force-pushed the fix/gateway-compression-contextvars branch 2 times, most recently from 10f4c51 to e93f09f Compare August 4, 2026 01:07
@dyreckt

dyreckt commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

10-second summary

This PR is the session-hygiene sibling of the manual /compress context fix.

  • The manual path is already fixed on main (651c5160b75). This PR originally covered that path too; the commit was dropped as redundant when rebasing onto current main.
  • What's left: the hygiene auto-compression worker in gateway/run.py runs through a bare run_in_executor, so the caller's ContextVars are lost on the thread hop. Under gateway.multiplex_profiles the profile secret scope is gone in the worker, and the compressor's aux-client credential resolution fails closed (UnscopedSecretError) — the path the sweeper review flagged.
  • The fix: copy_context() + ctx.run on the executor hop — same pattern as main's _run_in_executor_with_context.
  • Test: regression test sets a ContextVar in the caller and asserts it reaches the worker. 14 passed locally; CI green on this head.

Related: #76882 closes the other secret-scope residuals (/insights, aux readers, camofox, startup guard); this PR covers the hygiene compression worker.

@dyreckt
dyreckt force-pushed the fix/gateway-compression-contextvars branch from e93f09f to b3c81ed Compare August 6, 2026 19:57
@dyreckt
dyreckt force-pushed the fix/gateway-compression-contextvars branch from b3c81ed to 53f265d Compare August 7, 2026 00:08
@teknium1

teknium1 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Resolved on main: the hygiene run_in_executor hop now runs inside copy_context().run via #100950 (merge c5c9aa8d44), and the manual /compress path already goes through _run_in_executor_with_context on current main. Your July report of the same contextvar-loss class predates both — thanks @dyreckt. Closing as implemented on main.

@teknium1 teknium1 closed this Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/compression Context compression and continuation sessions comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants