fix(matrix): scope require_mention/process_notices/session_scope/auto_thread/dm_mention_threads/max_message_length to the active profile under multiplexing - #102097
Conversation
…_thread/dm_mention_threads/max_message_length (and more) to the active profile under multiplexing MatrixAdapter.__init__ read MATRIX_PROCESS_NOTICES/MATRIX_SESSION_SCOPE/ MATRIX_AUTO_THREAD/MATRIX_DM_MENTION_THREADS/MATRIX_ALLOW_ROOM_MENTIONS/ MATRIX_DM_AUTO_THREAD via raw os.getenv, never consulting config.extra. require_mention/thread_require_mention/max_message_length already checked config.extra first (via _parse_require_mention/_parse_thread_require_mention/ _resolve_max_message_length) but fell back to raw os.getenv when extra was absent. _apply_yaml_config additionally wrote MATRIX_REQUIRE_MENTION/ MATRIX_PROCESS_NOTICES/MATRIX_SESSION_SCOPE/MATRIX_AUTO_THREAD/ MATRIX_DM_MENTION_THREADS/MATRIX_MAX_MESSAGE_LENGTH into the process-global os.environ unconditionally (guarded only by `not os.getenv(...)`, first-writer-wins) -- the same apply_yaml_config_fn bug class already fixed for Mattermost/DingTalk/WhatsApp today. Matrix's E2EE crypto-store path and recovery-key read were already fixed separately (NousResearch#69090); this closes the remaining gap in the same file. Under gateway.multiplex_profiles, os.environ holds the DEFAULT profile's env-bridge output. A secondary profile with its own (different or absent) Matrix config could silently have its mention-gating, notice-processing, session-scope, auto-threading, and outbound chunk-size decisions driven by the default profile's settings for the adapter's entire runtime lifetime. Fix, mirroring the Mattermost/DingTalk apply_yaml_config_fn pattern: - Add _profile_scoped_config_load() (same helper as Mattermost/DingTalk). - Rewrite _apply_yaml_config to skip the env-bridge write for require_mention/process_notices/session_scope/auto_thread/ dm_mention_threads/max_message_length under a multiplexed secondary profile's scope, returning those values as a dict merged into this profile's own PlatformConfig.extra instead. allowed_users/ free_response_rooms/allowed_rooms/ignore_user_patterns are left on the legacy always-env-write path -- a separate, allowlist-focused PR (NousResearch#88559, open) covers those; verified its diff touches the same function but only those allowlist fields, no overlap with the fields fixed here. - Give _startup_env_secret() (Matrix's existing scoped-secret helper, previously used only for MATRIX_ACCESS_TOKEN/PASSWORD/HOMESERVER) an optional `default` parameter so non-credential settings can reuse it too, and switch every affected read site (including the process_notices/session_scope/auto_thread/dm_mention_threads/ allow_room_mentions/dm_auto_thread sites, which never consulted config.extra at all) to read extra first, falling back to the scoped helper instead of raw os.getenv. - MATRIX_ALLOW_ROOM_MENTIONS/MATRIX_DM_AUTO_THREAD have no config.yaml path at all (env-only knobs); scope the read itself via _startup_env_secret rather than inventing a new YAML bridge for them (a separate open PR, NousResearch#96039, proposes adding a dm_auto_thread config-bridge as a feature -- complementary, not overlapping, since it only touches the write side and this fix only touches the read side). Adds a TestMultiplexProfileScope class to tests/gateway/test_matrix.py (6 tests) mirroring the fixture/assertion style established in tests/gateway/test_irc_adapter.py's TestMultiplexProfileScope. Mutation- verified: reverted the production fix and confirmed 4 of 5 differentiating tests fail against pre-fix code (the 5th, guarding that the default/ unscoped profile keeps writing the env bridge, correctly passes either way as a non-differentiating regression guard). Restored the fix; all 117 tests in tests/gateway/test_matrix.py pass, plus tests/gateway/test_matrix_crypto_store_per_profile.py, tests/gateway/test_matrix_recovery_key_scope.py, tests/gateway/test_matrix_plugin_setup.py, tests/gateway/test_adapter_startup_secret_scope.py (75 tests), and tests/gateway/test_config.py (63 tests).
PR #102097 — fix(matrix): scope require_mention/process_notices/session_scope to the active profile
Non-blocking:
Verdict: LGTM. |
|
Superseded by #108705 (on |
Problem
plugins/platforms/matrix/adapter.pyonly got a partial version of the same-day multiplex-scoping treatment applied to Mattermost/DingTalk/IRC/WhatsApp today. Its E2EE crypto-store path andMATRIX_RECOVERY_KEYread were already fixed (#69090), but several other settings were not:require_mention,thread_require_mention,max_message_lengthalready checkedconfig.extrafirst, but fell back to rawos.getenvwhenextrawas absent.process_notices,session_scope,auto_thread,dm_mention_threadsnever consultedconfig.extraat all — rawos.getenvonly.allow_room_mentions,dm_auto_threadhave noconfig.yamlpath at all (env-only knobs) — rawos.getenvonly._apply_yaml_configwroteMATRIX_REQUIRE_MENTION/MATRIX_PROCESS_NOTICES/MATRIX_SESSION_SCOPE/MATRIX_AUTO_THREAD/MATRIX_DM_MENTION_THREADS/MATRIX_MAX_MESSAGE_LENGTHinto the process-globalos.environunconditionally (guarded only bynot os.getenv(...), first-writer-wins) — the sameapply_yaml_config_fnbug class already fixed for Mattermost/DingTalk/WhatsApp in this series.Under
gateway.multiplex_profiles,os.environholds the default profile's YAML→env bridge output. A secondary profile with its own (or no) Matrix config could silently have its mention-gating, notice-processing, session-scope, auto-threading, and outbound chunk-size decisions driven by the default profile's settings for the adapter's entire runtime lifetime.Fix
Mirrors the Mattermost/DingTalk
apply_yaml_config_fnpattern (see 56d869d):_profile_scoped_config_load()(same helper/name as Mattermost/DingTalk) — true when running inside a multiplexed secondary profile's scope._apply_yaml_configto skip the env-bridge write forrequire_mention/process_notices/session_scope/auto_thread/dm_mention_threads/max_message_lengthunder a scoped secondary profile, returning those values as a dict merged into that profile's ownPlatformConfig.extrainstead (the genericapply_yaml_config_fndispatcher ingateway/config.pyalready doesextra.update(seeded))._startup_env_secret()— Matrix's existing scoped-secret helper, previously used only forMATRIX_ACCESS_TOKEN/MATRIX_PASSWORD/MATRIX_HOMESERVER— an optionaldefaultparameter so non-credential settings can reuse it too (backward compatible; existing 1-arg call sites unaffected).config.extrafirst, falling back to_startup_env_secret()instead of rawos.getenv:_parse_require_mention,_parse_thread_require_mention,_resolve_max_message_length, and (newly extra-aware)process_notices,session_scope,auto_thread,dm_mention_threadsin__init__.MATRIX_ALLOW_ROOM_MENTIONS/MATRIX_DM_AUTO_THREADhave noconfig.yamlpath at all; scope the read itself via_startup_env_secretrather than inventing a new YAML bridge for them.Explicitly out of scope
allowed_users/allowed_rooms/free_response_rooms/ignore_user_patterns/GATEWAY_ALLOW_ALL_USERSare not touched here. I checked open PRs first:_apply_yaml_configfunction and independently defines the same-named_profile_scoped_config_load()helper — but exclusively for the allowlist fields above (_resolve_allowlist()in its diff). Its diff toplugins/platforms/matrix/adapter.pynever touchesrequire_mention/process_notices/session_scope/auto_thread/dm_mention_threads/max_message_length. Textual proximity (same function), zero semantic overlap with this PR — I verified viagh pr diff 88559._apply_yaml_config's write side, givingdm_auto_threadaconfig.yamlpath for the first time. It's complementary, not overlapping: this PR only scopesdm_auto_thread's read side (rawos.getenv→_startup_env_secret) and deliberately does not add a YAML bridge for it, to avoid stepping on that PR's feature.Both will need a routine rebase against this PR's
_apply_yaml_configrewrite (same function, no semantic conflict).Tests
Adds
TestMultiplexProfileScopetotests/gateway/test_matrix.py(6 tests), mirroring the fixture/assertion style established intests/gateway/test_irc_adapter.py'sTestMultiplexProfileScope:test_secondary_extra_wins_over_default_profile_envtest_secondary_missing_keys_fail_closedtest_secondary_own_env_only_scope_wins_over_default_profile_envtest_apply_yaml_config_scoped_skips_env_write_and_seeds_extratest_apply_yaml_config_unscoped_default_profile_still_writes_env(non-differentiating regression guard)Mutation-verified: reverted the production fix (kept the tests) and confirmed 4 of 5 differentiating tests fail against pre-fix code; the 5th (guarding that the default/unscoped profile keeps writing the env bridge) correctly passes either way. Restored the fix — all tests pass again.
Full results with the fix applied:
tests/gateway/test_matrix.py— 117 passedtests/gateway/test_matrix_crypto_store_per_profile.py,test_matrix_recovery_key_scope.py,test_matrix_plugin_setup.py,test_matrix_mention.py,test_matrix_message_length.py— 28 passedtests/gateway/test_matrix_dm_invite_recording.py,test_matrix_approval_reaction_fail_closed.py,test_matrix_exec_approval.py,test_matrix_voice.py,test_matrix_project_context_isolation.py,test_matrix_message_event_metadata.py— 18 passed, 1 skipped (pre-existing, unrelated)tests/gateway/test_adapter_startup_secret_scope.py— 75 passedtests/gateway/test_config.py— 63 passedruff checkon both changed files — clean