feat(gateway): honor adapter-declared session scope for plugin platforms

[srosro]

Fork-side convergence of NousResearch#99950, rebased onto upstream main (abd83ab560, post run.py mixin split) as a single commit. LOC target: match the original (+326/−18) or smaller; actual +289/−17.

What

SessionStore derives group_sessions_per_user / thread_sessions_per_user from the global gateway config only, so a plugin / out-of-tree platform adapter that declares its own isolation in config.extra is silently ignored — it has no config.platforms entry for NousResearch#81207 / NousResearch#84925 to read.

This adds the missing seam: adapters register their resolved scope with the store when accepted, and every key derivation and every guard that must agree with key shape resolves through it before the config defaults.

• SessionStore.register_platform_session_scope / resolve_session_scope (in the recovery mixin, next to _generate_session_key), keyed (profile, platform) so multiplexed profiles can't leak overrides; the registry sits behind the store's _lazy() helper like its other optional maps.
• GatewayRunner._register_adapter_session_scope at the acceptance points — _publish_primary_adapter (primary connect + reconnect) and the two secondary-profile sites.
• GatewayRunner._resolve_session_scope_for is the one runner-side read of isolation flags: _session_key_for_source's no-store fallback, the inbound sender-attribution gate, and the /resume IDOR guards (_is_shared_session_source) all use it. build_session_context takes session_store so the agent-facing shared_multi_user_session flag agrees with the key.
• config_session_scope(config) owns the config-default pair (was duplicated at four sites).

Upstream review (andrexibiza on NousResearch#99950)

• P1 "second key owner": the premise doesn't hold on the live path — _session_key_for_source delegates to the store's _generate_session_key first, which this PR makes registry-aware. The fallback body was a duplicated derivation though, so it now reads through the chokepoint too, and a parity test pins runner key == store key under a registered scope (falsified by planting delegation-off + config-only fallback: red, then green).
• Interlock with fix(gateway): honor per-platform session isolation overrides NousResearch/hermes-agent#84925: composition order is registry → platform-config → defaults; whichever lands second gets a small rebase.
• Every-commit-green: single commit.

Known follow-up (out of scope)

_handoff_session_key (voice handoff destination) still reads config.platforms[...].extra with literal defaults — the NousResearch#81860 destination-shape class the upstream reviewer carved out as adjacent, and the platform-config layer it needs is NousResearch#84925's.

How to test

scripts/run_tests.sh tests/gateway/ — 5 pre-existing macOS-only failures (AF_UNIX path length, readiness) fail identically on clean main; everything else green. ruff check and check-windows-footguns.py --all clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Lqr7yMugdYt53YxrszjnSG

[srosro]

📋 First review of this PR — reviewed fcf21a9. ⚙️ No REVIEW.md (review using org defaults). 🧪 Tests not run. 🔍 Prior-art (KID) unavailable — no KID_PATHS entry.

It appears @srosro is working towards ensuring users on plugin messaging platforms get correctly shared or isolated conversations by registering adapter-declared scope in gateway/run_adapters.py and consistently applying SessionStore.resolve_session_scope to routing, context, attribution, and /resume safeguards — reviewing against that goal.

Overview — The profile-aware registry is directionally right, but two live seams still derive incompatible session keys. At this repo’s pre-PMF, fewer-than-10-user operating point, centralizing this policy is worthwhile because it removes parallel routing behavior rather than adding speculative architecture.

Strengths — The (profile, platform) precedence and reuse of the store resolver for persistence, prompt context, attribution, and /resume guards establish a coherent core seam.

Probes

1. [blocking] [from: security] [bug] Scope registration occurs only after connect() returns, although Telegram polling and webhooks become inbound-reachable inside connect(); early messages therefore use gateway defaults in the store. Separately, explicit None becomes a gateway default in the registry but remains falsey in adapter batching, so participants can route through different keys and merge or expose conversation data. Files: gateway/run_startup.py:986, gateway/run_startup.py:1069, gateway/run_adapters.py:760, gateway/run_adapters.py:1022, gateway/platforms/base.py:2274, gateway/run.py:3841, plugins/platforms/telegram/adapter.py:2956. Edit: Resolve one concrete boolean scope tuple after adapter construction, write it back to adapter.config.extra, register it in _wire_adapter_handlers before connect(), and delete the three post-connect registrations.

2. [blocking] [from: contract-drift] [bug] Three live consumers bypass the new registry-aware resolver: /undo evicts the wrong cached agent, handoff creates a store-scoped route but switches a config-derived key and fails, and Slack thread recovery reads or updates the wrong session metadata. Files: gateway/slash_commands_session.py:440, gateway/slash_commands_session.py:446, gateway/run_startup.py:1455, gateway/run_startup.py:1489, plugins/platforms/slack/adapter.py:5731, plugins/platforms/slack/adapter.py:5759, gateway/session_recovery.py:104. Edit: Collapse /undo and _handoff_session_key onto _session_key_for_source, and make Slack’s thread helpers obtain scope from session_store.resolve_session_scope(source).

3. [medium] [from: shape] [shape] The new lifecycle policy was added to the gateway/run.py facade instead of its existing adapter-lifecycle sibling, weakening the repository’s decomposition seam. Files: gateway/run.py:3832, gateway/run.py:3841, gateway/run_adapters.py:760. Edit: Move _resolve_session_scope_for and _register_adapter_session_scope into GatewayAdapterLifecycleMixin in gateway/run_adapters.py with no behavioral or LOC growth.

4. [medium] [from: tests] [simplification] Two registered-scope tests duplicate the same runner, store, registration, and Discord-source setup for one state transition. Files: tests/gateway/test_resume_command.py:879, tests/gateway/test_resume_command.py:902. Edit: Extend test_same_origin_honors_adapter_registered_override with the store-key/runner-key assertions and delete test_runner_key_matches_store_key_under_registered_scope — approximately −15 LOC.

Security — The startup-ordering and normalization mismatch can expose merged conversation data across participants.

Test coverage — No answered-yes Class: tests probe remains; staged just test was not run because the repository has no root justfile.

For AI authors — (Codex, Claude Code, Cursor, etc. reading this PR): The Probes above are load-bearing. Treat each [open] probe as a hard requirement — answer it directly in your reply or revisit the structural decision. Do NOT silently absorb a probe by adding more code; that path inverts the cost stance the probe is encoding. This repo's operating point (see its REVIEW.md) prefers cutting LOC over adding it; an [open] [simplification] probe whose answer is "no, this complexity isn't needed" should land as a deletion in your next push, not a new defensive guard.

---

How to use: auto-reviews every new PR and re-reviews after a period of inactivity. Trigger an incremental re-review with /srosro-update-review, or a whole-PR re-review with /srosro-review.

For humans only: push-access collaborators can post:

• /srosro-approve — APPROVE the PR.
• /srosro-props [from: <specialist>] — give props to a specialist's contribution.
• /srosro-critique [from: <specialist>] — flag a specialist's contribution as a misread.
• /srosro-memorize <feedback> — teach a calibration lesson (learn-from-replies updates COMMENT_REVIEW_MISTAKES.md from your body, sentiment-aware via LLM).

Props: /srosro-props [from: shape] caught a real layering bug we'd have shipped.
Critique: /srosro-critique [from: architecture-refined] DRY suggestion misread distinct seams.
Calibration: /srosro-memorize the architecture-refined DRY finding was a misread; those helpers serve different contracts.

AI agents must not use /srosro-memorize, /srosro-props, or /srosro-critique — those signals tune shared global state.

Generated by sam's ai review bot.

[srosro]

Addressed in ac575f7 (net −41 LOC this round; all four probes answered).

1. [blocking] registration after connect() — Applied. Registration now runs at handler wiring (_wire_adapter_handlers, before connect(); _configure_profile_adapter passes its profile), so the store knows the scope before Telegram polling/webhooks go inbound-reachable. An explicit None is normalized to the gateway default and written back to config.extra, so the adapter's own batching/thread keys and the store derive the same tuple. The three post-connect registration sites are deleted. Trade-off accepted knowingly: a created-but-rejected adapter now leaves a registration behind; with no live adapter nothing routes through it, and a reconnect re-wires and re-registers.

2. [blocking] three consumers bypass the resolver — Applied, all three. /undo evicts under _session_key_for_source(source). _handoff_session_key is deleted; the handoff uses _session_key_for_source(dest.source) (dest.source already carries the queuing profile, so the store namespaces agent:<profile>:… under multiplex exactly as the old helper did, and the key now matches the entry get_or_create_session creates two lines earlier). Slack's _build_thread_session_key / _thread_rehydration_key resolve through session_store.resolve_session_scope(source, profile=<owning profile>) — the same namespace build_session_key gets — via one small _thread_scope helper; a new test pins a secondary Slack bot's thread + rehydration keys following its own profile's registration and ignoring another profile's.

3. [medium] lifecycle policy on the facade — Applied for the registration half: _register_adapter_session_scope moved into GatewayAdapterLifecycleMixin next to the wiring it runs from. Counter-proposed for _resolve_session_scope_for: it stays beside _session_key_for_source in run.py because it is the key owner's fallback resolver used by inbound, slash, and the key helper itself, not adapter lifecycle; moving it to the lifecycle mixin would put a key-derivation read in the wrong seam. No LOC growth either way.

4. [medium] duplicated test setup — Applied. Parity assertions folded into test_same_origin_honors_adapter_registered_override; test_runner_key_matches_store_key_under_registered_scope deleted.

Gate: scripts/run_tests.sh tests/gateway/ green (7900 passed) except the five pre-existing macOS-only failures noted in the PR body; ruff clean.

[srosro]

/srosro-update-review

[srosro]

📋 Re-review of changes from fcf21a9 to ac575f7 (git diff fcf21a9..ac575f7). ⚙️ No REVIEW.md (review using org defaults). 🧪 Tests not run. 🔍 Prior-art (KID) unavailable — no KID_PATHS entry. ⚡ Small-diff lane (197 lines): skipped consumers — screened by aggregator.

It appears @srosro is working towards keeping users’ conversations correctly isolated on plugin messaging platforms, including multiplexed profiles and Slack threads, by registering adapter-declared session scope before connect() in gateway/run_adapters.py and routing /undo, handoff, and Slack thread keys through the store’s scope resolver — reviewing against that goal.

Overview — The update resolves the prior startup-ordering, resolver-bypass, placement, and duplicated-test findings. One multiplex path still derives a secondary adapter’s defaults from the primary profile, which can merge conversations despite that profile’s isolation setting. At this pre-PMF, fewer-than-10-user operating point, the remaining test simplification is worthwhile because it removes repeated setup.

Strengths — Registration now precedes inbound reachability, while /undo, handoff, and Slack thread handling converge on the canonical scope resolver.

Probes

1. [blocking] [from: data-integrity] [bug] Secondary adapters normalize missing or None isolation flags from the primary runner’s self.config: startup loads the owning profile_cfg but discards its defaults, reconnect retains only its platform config, and _instantiate_adapter plus registration both read the primary configuration. With primary group_sessions_per_user=False and secondary True, participants on the secondary bot therefore share one conversation and can receive each other’s history. The owning tuple must also replace the primary-default seeding for missing keys at line 1468. Files: gateway/run_adapters.py:970, gateway/run_adapters.py:1008, gateway/run_adapters.py:1044, gateway/run_adapters.py:1121, gateway/run_adapters.py:1141, gateway/run_adapters.py:1468. Edit: Pass the owning GatewayConfig scope tuple from _start_one_profile_adapters and _secondary_reconnect_attempt through _configure_profile_adapter and _wire_adapter_handlers to _register_adapter_session_scope, replacing config_session_scope(self.config) for secondary adapters (approximately +8 LOC).

2. [medium] [from: tests] [simplification] The three registered-scope cases repeat the same Discord group-source construction, leaving roughly 15 lines coupled to future SessionSource changes. Files: tests/gateway/test_session.py:711, tests/gateway/test_session.py:756, tests/gateway/test_session.py:793. Edit: Add a discord_group_source(**overrides) factory beside the existing store fixture and replace the repeated SessionSource(...) blocks — approximately −15 LOC.

def discord_group_source(**overrides):
    values = {
        "platform": Platform.DISCORD,
        "chat_id": "guild-123",
        "chat_type": "group",
        "user_id": "alice",
    }
    return SessionSource(**(values | overrides))

Probe resolved: operator declined moving _resolve_session_scope_for because “it stays beside _session_key_for_source … [as] the key owner’s fallback resolver”; current consumers support that split — agreed.

Security — The secondary-profile fallback can merge participant sessions and expose conversation history.

Test coverage — No answered-yes Class: tests probe remains; staged just test was not run because the repository has no root justfile.

For AI authors — (Codex, Claude Code, Cursor, etc. reading this PR): The Probes above are load-bearing. Treat each [open] probe as a hard requirement — answer it directly in your reply or revisit the structural decision. Do NOT silently absorb a probe by adding more code; that path inverts the cost stance the probe is encoding. This repo's operating point (see its REVIEW.md) prefers cutting LOC over adding it; an [open] [simplification] probe whose answer is "no, this complexity isn't needed" should land as a deletion in your next push, not a new defensive guard.

---

How to use: auto-reviews every new PR and re-reviews after a period of inactivity. Trigger an incremental re-review with /srosro-update-review, or a whole-PR re-review with /srosro-review.

For humans only: push-access collaborators can post:

• /srosro-approve — APPROVE the PR.
• /srosro-props [from: <specialist>] — give props to a specialist's contribution.
• /srosro-critique [from: <specialist>] — flag a specialist's contribution as a misread.
• /srosro-memorize <feedback> — teach a calibration lesson (learn-from-replies updates COMMENT_REVIEW_MISTAKES.md from your body, sentiment-aware via LLM).

Props: /srosro-props [from: shape] caught a real layering bug we'd have shipped.
Critique: /srosro-critique [from: architecture-refined] DRY suggestion misread distinct seams.
Calibration: /srosro-memorize the architecture-refined DRY finding was a misread; those helpers serve different contracts.

AI agents must not use /srosro-memorize, /srosro-props, or /srosro-critique — those signals tune shared global state.

Generated by sam's ai review bot.

[srosro]

Addressed in ea47242 (net −12 LOC this round; both probes answered).

1. [blocking] secondary defaults seeded from the primary config — Applied, and the diagnosis was exact: there were two primary-config seeds (the instantiate-time setdefault at the old line 1468 and the wiring-time registration). Both are gone in favor of one seed at registration, which now takes the OWNING profile's scope tuple: _start_one_profile_adapters passes config_session_scope(profile_cfg) and _secondary_reconnect_attempt passes the tuple from the load_gateway_config() it already ran under the profile scope, through _configure_profile_adapter → _wire_adapter_handlers → _register_adapter_session_scope. Primary adapters still seed from the primary config. Adapters only read these flags at event time (_event_session_key and siblings), so seeding at wiring rather than instantiation changes nothing they observe. Test test_secondary_adapter_scope_seeds_from_owning_profile_defaults pins the leak (primary False, secondary True); planting the old behavior turns it red.

2. [medium] repeated Discord source construction — Applied. discord_group_source(**overrides) factory replaces the repeated SessionSource(...) blocks.

Gate: scripts/run_tests.sh tests/gateway/ 7901 passed, the same five pre-existing macOS-only failures; ruff clean.

[srosro]

/srosro-update-review

[srosro]

📋 Re-review of changes from ac575f7 to ea47242 (git diff ac575f7..ea47242). ⚙️ No REVIEW.md (review using org defaults). 🧪 Tests not run. 🔍 Prior-art (KID) unavailable — no KID_PATHS entry. ⚡ Small-diff lane (108 lines): skipped consumers — screened by aggregator.

It appears @srosro is working towards ensuring plugin-platform users get the intended conversation isolation in group chats and threads, including under multiplexed profiles, by registering each adapter’s declared session scope before connection and seeding unset scope flags from the owning profile’s gateway config in gateway/run_adapters.py — reviewing against that goal.

Overview — The update resolves the remaining secondary-profile leak: startup and reconnect now pass their owning profile’s isolation defaults through the existing adapter-lifecycle seam, while the primary-config seed at instantiation is gone. The full PR now consistently resolves session scope across persistence, runner keys, guards, context, handoff, and Slack thread handling.

Strengths — Scope registration occurs before adapters become inbound-reachable, profile/platform registry keys preserve multiplex isolation, and the latest change removes duplicate default seeding rather than adding another resolution path. The shared Discord source factory also addresses the previous test duplication cleanly.

Probes

None.

Probe resolved: the prior blocking shape is gone; both secondary entry points pass owning-profile defaults at gateway/run_adapters.py:1008 and gateway/run_adapters.py:1148, and the primary-config instantiation seed was removed.

Probe dropped: the suggested startup/reconnect assertions would only guard against a future drift of currently-correct wiring. The production paths already converge on _configure_profile_adapter, so the review policy declines adding a CI fence for that hypothetical regression.

Security — None.

Test coverage — No surviving answered-yes Class: tests probe; staged just test was not run because the repository has no root justfile.

For AI authors — (Codex, Claude Code, Cursor, etc. reading this PR): The Probes above are load-bearing. Treat each [open] probe as a hard requirement — answer it directly in your reply or revisit the structural decision. Do NOT silently absorb a probe by adding more code; that path inverts the cost stance the probe is encoding. This repo's operating point (see its REVIEW.md) prefers cutting LOC over adding it; an [open] [simplification] probe whose answer is "no, this complexity isn't needed" should land as a deletion in your next push, not a new defensive guard.

---

How to use: auto-reviews every new PR and re-reviews after a period of inactivity. Trigger an incremental re-review with /srosro-update-review, or a whole-PR re-review with /srosro-review.

For humans only: push-access collaborators can post:

• /srosro-approve — APPROVE the PR.
• /srosro-props [from: <specialist>] — give props to a specialist's contribution.
• /srosro-critique [from: <specialist>] — flag a specialist's contribution as a misread.
• /srosro-memorize <feedback> — teach a calibration lesson (learn-from-replies updates COMMENT_REVIEW_MISTAKES.md from your body, sentiment-aware via LLM).

Props: /srosro-props [from: shape] caught a real layering bug we'd have shipped.
Critique: /srosro-critique [from: architecture-refined] DRY suggestion misread distinct seams.
Calibration: /srosro-memorize the architecture-refined DRY finding was a misread; those helpers serve different contracts.

AI agents must not use /srosro-memorize, /srosro-props, or /srosro-critique — those signals tune shared global state.

Generated by sam's ai review bot.

[srosro]

Acknowledged — no findings raised on ea47242. Converged.