Skip to content

fix(discord): accept relay-lane kwargs in native rename_thread (#78487) - #78495

Open
PRATHAMESH75 wants to merge 2 commits into
NousResearch:mainfrom
PRATHAMESH75:fix/discord-native-rename-thread-kwargs
Open

fix(discord): accept relay-lane kwargs in native rename_thread (#78487)#78495
PRATHAMESH75 wants to merge 2 commits into
NousResearch:mainfrom
PRATHAMESH75:fix/discord-native-rename-thread-kwargs

Conversation

@PRATHAMESH75

Copy link
Copy Markdown
Contributor

Summary

Fixes #78487.

Since v0.19.1 the native Discord auto-thread semantic rename silently stopped applying: the gateway logs the discord auto-thread rename: … lane=native attempt, but the thread is never renamed and no completion/error line follows.

Root cause is a call-site/callee signature mismatch. The semantic-rename lane resolves rename_thread polymorphically and passes the relay sibling's kwargs unconditionally:

# gateway/run.py  _rename_discord_auto_thread_for_session_title
rename_thread = getattr(adapter, "rename_thread", None)
...
renamed = await rename_thread(
    target_thread_id,
    thread_name,
    prefer_connector_created=use_connector_guard,  # passed unconditionally
    only_if_current_name=guard_name,
    parent_chat_id=parent_chat_id,                  # passed unconditionally
)

The relay adapter (gateway/relay/adapter.py) accepts all three kwargs, but the native Discord adapter's rename_thread only accepted only_if_current_name. On the native lane the extra kwargs raise TypeError, which is swallowed by the caller's except Exceptionlogger.debug(...) — so the failure is invisible at INFO even though the "attempt" INFO line fired.

Fix

Bring the native rename_thread signature to parity with its relay sibling — accept prefer_connector_created and parent_chat_id and ignore them. They only mean something to the relay egress guard; the native lane renames the thread directly via the Discord API, so it needs neither. This is the minimal change that keeps the shared polymorphic call site working for both lanes (vs. sprinkling conditional kwargs at the call site).

Testing

tests/gateway/test_discord_slash_commands.py::test_rename_thread_accepts_relay_lane_kwargs — calls the native rename_thread with prefer_connector_created= / parent_chat_id= present and asserts it no longer raises and still performs the rename. (Fails with TypeError before this change.)

uv run --extra messaging pytest tests/gateway/test_discord_slash_commands.py -q  →  14 passed

@MAPE-sub-zero

Copy link
Copy Markdown

Independent reproduction and confirmation of this fix on v0.20.5 (2026.8.19), macOS 26.5.2, native Discord lane (no relay connector).

Before the change — the semantic rename is attempted with a correct title, and nothing follows it:

INFO gateway.run: discord auto-thread rename: thread=… lane=native new_title='Parallelize credresolve key resolution'

No discord auto-thread rename result: line, and no [Discord] Renamed Discord thread …. Querying the thread via the Discord API confirmed the rename never applied:

ACTUAL : 'I want to think through whether the credresolve helper should resolve keys in...'
WANTED : 'Parallelize credresolve key resolution'

Reproduced on 4/4 native-lane threads.

After applying this PR's adapter change locally — same setup, new thread:

INFO gateway.run: discord auto-thread rename: thread=… lane=native new_title='Optimize credresolve helper concurrent key resolution'
INFO gateway.run: discord auto-thread rename result: thread=… applied=True
INFO hermes_plugins.discord_platform.adapter: [Discord] Renamed Discord thread … from 'I keep going back and forth on whether the credresolve helper should resolve ...' to 'Optimize credresolve helper concurrent key resolution'

Discord API confirms the applied state:

name: 'Optimize credresolve helper concurrent key resolution'

Two notes that may be useful:

The failure is invisible at default log level. The attempt logs at INFO (gateway/run.py), the result line only executes on success, and the exception handler is logger.debug. So a failing install shows a healthy-looking log with a correct title and simply no follow-up. Raising logging.level in config.yaml doesn't surface it either, since the gateway's DEBUG output is driven by the -v/-vv CLI flags, which the launchd service doesn't pass. Something like the following would have made this self-evident, and matches the CONTRIBUTING guidance on using exc_info=True for unexpected errors:

except Exception:
    logger.warning("Discord auto-thread rename failed: thread=%s", target_thread_id, exc_info=True)

A TypeError from a call-site/signature mismatch is a programming error rather than an expected best-effort miss, so it probably shouldn't be indistinguishable from "Discord declined the rename".

A signature-compatibility test would prevent recurrence across both adapters — e.g. binding inspect.signature(...) against the exact kwargs the shared call site passes, parametrised over the relay and native adapters. Happy to open that separately if it's wanted; didn't want to expand the scope of this PR.

Thanks for the fix — glad to see the regression test included.

…esearch#78487)

The semantic auto-thread rename lane (gateway/run.py
_rename_discord_auto_thread_for_session_title) resolves rename_thread
polymorphically via getattr(adapter, ...) and passes prefer_connector_created
and parent_chat_id unconditionally — the relay sibling
(gateway/relay/adapter.py) accepts both, but the native Discord adapter's
signature only took only_if_current_name. On the native lane the extra kwargs
raised TypeError, which the caller swallows in a bare except -> logger.debug,
so native auto-thread renames silently stopped applying while the INFO
'attempt' line still fired.

Bring the native signature to parity: accept prefer_connector_created and
parent_chat_id and ignore them (they only matter to the relay egress guard;
the native lane renames directly via the Discord API).
Add a signature-level contract test (no I/O) that binds each adapter's
rename_thread against the exact kwargs gateway/run.py's semantic-rename lane
passes, parametrised over the native (DiscordAdapter) and relay
(RelayAdapter) lanes. run.py resolves rename_thread polymorphically and
passes prefer_connector_created/parent_chat_id unconditionally, so a
signature drift in EITHER adapter reintroduces the NousResearch#78487 TypeError that the
caller swallows into logger.debug. The existing test proves the native lane
accepts the kwargs at runtime; this pins the contract for both lanes at the
signature level and fails on the pre-fix narrow signature.

Suggested by @MAPE-sub-zero's independent reproduction on NousResearch#78495.
@PRATHAMESH75
PRATHAMESH75 force-pushed the fix/discord-native-rename-thread-kwargs branch from 44e0db0 to 5030e96 Compare August 25, 2026 18:25
@PRATHAMESH75

Copy link
Copy Markdown
Contributor Author

@MAPE-sub-zero thank you for the thorough independent reproduction — the before/after Discord-API confirmation and the "invisible at default log level" analysis are exactly the diagnosis. Also rebased the branch onto current upstream/main while here.

Signature-compat test — added (5030e96a8b). test_rename_thread_signature_matches_shared_call_site does exactly what you described: it binds inspect.signature(adapter.rename_thread) against the exact kwargs the shared lane passes (prefer_connector_created, only_if_current_name, parent_chat_id), with no call and no I/O, parametrised over both lanes — native DiscordAdapter and relay RelayAdapter. So a signature drift in either adapter fails the build, not just the native one the existing runtime test covers. I verified it fails on the pre-fix narrow signature (TypeError: got an unexpected keyword argument 'prefer_connector_created') and passes after. RelayAdapter is imported lazily inside the test so it stays a pure-signature unit test with no import-time coupling.

The logger.debuglogger.warning(..., exc_info=True) change — I'd like to keep it out of this PR, but I agree with you. That handler lives at the call site in gateway/run.py (a ~23k-line module), and it's genuinely a different concern from the adapter kwargs contract: it changes how every rename failure surfaces, including the legitimate best-effort misses (Discord declining, permission gaps) that are correctly debug today. You're right that a programming-error TypeError shouldn't be indistinguishable from an expected miss — the cleanest form is probably to narrow the except: let a TypeError (call-site/signature mismatch) log at warning with exc_info=True while genuine best-effort misses stay debug. That's a focused diagnostics change worth its own small PR against run.py rather than smuggling a godfile edit into an adapter-parity fix. If you'd like to open that (you clearly have the repro), I'll happily review; otherwise I can put it up separately and link back here.

The signature test you asked for is the higher-leverage half anyway — it makes this class of regression impossible to merge silently again. Appreciate the careful review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/discord Discord bot adapter type/bug Something isn't working

Projects

None yet

3 participants