Skip to content

fix(gateway): avoid cross-profile session recovery - #58119

Closed
tianma-if wants to merge 1 commit into
NousResearch:mainfrom
tianma-if:codex/fix-unmultiplexed-profile-recovery
Closed

fix(gateway): avoid cross-profile session recovery#58119
tianma-if wants to merge 1 commit into
NousResearch:mainfrom
tianma-if:codex/fix-unmultiplexed-profile-recovery

Conversation

@tianma-if

Copy link
Copy Markdown
Contributor

Summary

  • Fixes Bug: multiplex_profiles: false leaves orphaned sessions that cause wrong profile routing #58032.
  • When gateway.multiplex_profiles is disabled, prevent SessionStore DB peer fallback from recovering a durable session row that belongs to a different profile namespace.
  • Preserve the intended migration path for the active profile itself: if the active profile is coder, an old agent:coder:... row can still be recovered into the non-multiplexed gateway's legacy agent:main:... routing key.

Verification

  • Checked open PRs by issue number and keywords (multiplex_profiles, orphaned sessions, wrong profile routing, SessionStore recovery); no overlapping open PR found before implementation.
  • .venv/bin/python -m pytest tests/gateway/test_multiplex_phase0.py -q

Notes

The key-generation contract is unchanged: with multiplexing off, new routing keys remain byte-identical legacy agent:main:... keys. The new guard only applies after state.db peer fallback returns a row whose stored session_key namespace differs from the requested key.

@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-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 4, 2026

@tonydwb tonydwb 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.

Code Review Summary

Verdict: Approved (LGTM)

Fixes cross-profile session recovery to prevent recovering sessions from one profile into another. Important correctness fix for multi-profile deployments.

What Looks Good

  • Targeted security-relevant fix
  • No scope creep
  • No debug artifacts

@tonydwb tonydwb 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.

Code Review Summary

Verdict: LGTM

Fix (+111/0) avoiding cross-profile session recovery. Clean, minimal change.

Looks Good

  • Single responsibility
  • No security or performance concerns

Reviewed by Hermes Agent

@tonydwb tonydwb 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.

Supplementary review: confirms LGTM. Cross-profile session recovery prevention is a clean isolation fix. Prior COMMENT found no issues. No additional concerns.


Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused isolation fix. This has already landed on main via the authorship-preserving salvage PR #59325.

  • Automated hermes-sweeper review verified merge commit f1fde49e453e508b9dc49ec5b70694fbbf0279c0 (fix(gateway): avoid cross-profile session recovery (#59325)), which includes this guard and regression coverage.
  • gateway/session.py:1319-1356 parses the recovered row namespace and permits non-multiplexed recovery only for the active profile; _recover_session_from_db applies it before reopening at gateway/session.py:1418-1431.
  • Current coverage in tests/gateway/test_multiplex_phase0.py:229-266 rejects the cross-profile fallback while preserving the same-profile legacy migration path.
  • The fix is contained in release tag v2026.7.7.

Closing as implemented on main.

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 comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main 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.

Bug: multiplex_profiles: false leaves orphaned sessions that cause wrong profile routing

4 participants