Skip to content

[Bob] fix: namespace Slack sessions by workspace - #68925

Closed
trac3r00 wants to merge 1 commit into
NousResearch:mainfrom
trac3r00:fix/slack-workspace-session-scope
Closed

[Bob] fix: namespace Slack sessions by workspace#68925
trac3r00 wants to merge 1 commit into
NousResearch:mainfrom
trac3r00:fix/slack-workspace-session-scope

Conversation

@trac3r00

Copy link
Copy Markdown
Contributor

Summary

  • namespace non-DM Slack session keys by workspace scope
  • safely migrate matching legacy routes and reject scope-ambiguous routes
  • cover cross-workspace collision, unchanged DM/thread behavior, and routing/DB recovery

Verification

  • uv run python -m pytest tests/gateway/test_session.py -q (130 passed)
  • uv run python -m ruff check gateway/session.py tests/gateway/test_session.py
  • uv run python -m compileall -q gateway/session.py

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/slack Slack app adapter sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 21, 2026
teknium1 added a commit that referenced this pull request Jul 23, 2026
Conflict-resolution follow-up composing #68925 (Bob) with the already-
applied #20583/#66398 (jordanhubbard) recovery design:

- #68925's caller-level second _query_recoverable_session pass (via
  lookup_session_key=) referenced a variable that no longer exists —
  the legacy exact-key fallback now lives INSIDE
  _query_recoverable_session, which also claims the legacy key once per
  process and rewrites the peer row to the scoped key. Drop the dead
  caller-level pass.
- Keep #68925's _recovered_row_matches_source_scope origin guard wired
  into both recovery paths: a scoped channel lookup refuses rows whose
  recorded origin names another workspace (or no workspace at all).
- Routing-index migration adoption policy documented at the site:
  origin names a workspace -> exact match only; scope-less DM -> first
  workspace claims once; scope-less channel -> refuse.
teknium1 added a commit that referenced this pull request Jul 23, 2026
Conflict-resolution follow-up composing #68925 (Bob) with the already-
applied #20583/#66398 (jordanhubbard) recovery design:

- #68925's caller-level second _query_recoverable_session pass (via
  lookup_session_key=) referenced a variable that no longer exists —
  the legacy exact-key fallback now lives INSIDE
  _query_recoverable_session, which also claims the legacy key once per
  process and rewrites the peer row to the scoped key. Drop the dead
  caller-level pass.
- Keep #68925's _recovered_row_matches_source_scope origin guard wired
  into both recovery paths: a scoped channel lookup refuses rows whose
  recorded origin names another workspace (or no workspace at all).
- Routing-index migration adoption policy documented at the site:
  origin names a workspace -> exact match only; scope-less DM -> first
  workspace claims once; scope-less channel -> refuse.
@trac3r00

Copy link
Copy Markdown
Contributor Author

Closing in favor of #70190, which covers the same workspace-isolation class more completely across both Slack DMs and group channels while preserving legacy session compatibility. Thanks for carrying the fix forward and crediting this attempt.

[bob]

@trac3r00 trac3r00 closed this Jul 23, 2026
teknium1 added a commit that referenced this pull request Jul 23, 2026
Conflict-resolution follow-up composing #68925 (Bob) with the already-
applied #20583/#66398 (jordanhubbard) recovery design:

- #68925's caller-level second _query_recoverable_session pass (via
  lookup_session_key=) referenced a variable that no longer exists —
  the legacy exact-key fallback now lives INSIDE
  _query_recoverable_session, which also claims the legacy key once per
  process and rewrites the peer row to the scoped key. Drop the dead
  caller-level pass.
- Keep #68925's _recovered_row_matches_source_scope origin guard wired
  into both recovery paths: a scoped channel lookup refuses rows whose
  recorded origin names another workspace (or no workspace at all).
- Routing-index migration adoption policy documented at the site:
  origin names a workspace -> exact match only; scope-less DM -> first
  workspace claims once; scope-less channel -> refuse.
@teknium1

Copy link
Copy Markdown
Contributor

Closing as superseded by #70190 (merged): group-only workspace namespacing; the landed fix also scopes DMs (Slack DM channel IDs are workspace-local too) and adds legacy-key compat.

Thanks for the work — it's credited in #70190's summary.

@trac3r00

Copy link
Copy Markdown
Contributor Author

Confirmed — #70190 is the stronger complete fix, especially with DM scoping and legacy-key compatibility. Thanks for preserving the attribution.

[bob]

randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
Conflict-resolution follow-up composing NousResearch#68925 (Bob) with the already-
applied NousResearch#20583/NousResearch#66398 (jordanhubbard) recovery design:

- NousResearch#68925's caller-level second _query_recoverable_session pass (via
  lookup_session_key=) referenced a variable that no longer exists —
  the legacy exact-key fallback now lives INSIDE
  _query_recoverable_session, which also claims the legacy key once per
  process and rewrites the peer row to the scoped key. Drop the dead
  caller-level pass.
- Keep NousResearch#68925's _recovered_row_matches_source_scope origin guard wired
  into both recovery paths: a scoped channel lookup refuses rows whose
  recorded origin names another workspace (or no workspace at all).
- Routing-index migration adoption policy documented at the site:
  origin names a workspace -> exact match only; scope-less DM -> first
  workspace claims once; scope-less channel -> refuse.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/slack Slack app adapter sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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