fix(weixin): allow tokenless retry when no cached context_token exists - #35066
fix(weixin): allow tokenless retry when no cached context_token exists#35066liuhao1024 wants to merge 1 commit into
Conversation
The tokenless retry guard required context_token to be truthy, which prevented the fallback from firing during cron-initiated pushes to long-inactive chats where no cached token existed. iLink accepts tokenless sends as a degraded fallback, so the guard now only checks not retried_without_token. Fixes NousResearch#35062
alt-glitch
added
type/bug
|
@liuhao1024 any update on merging this? We've been testing the fix locally and it handles the tokenless edge case correctly. However, there's a deeper ret=-3 issue on cron push that seems to be an iLink platform limitation—see my comment on #35062 for the full analysis. Happy to provide more test data if it helps move this forward. |
|
Hi @drzeast-png, thanks for testing the fix locally and for the detailed analysis on #35062! The PR is ready for review — all CI checks are green and it's mergeable. The fix is intentionally scoped to the guard removal only, which handles the case where no cached token exists. Regarding the deeper issue you mentioned on cron push: I agree that's likely an iLink platform limitation rather than something we can fix in the adapter. The current PR handles the retry-without-token path cleanly, and the with is already detected by as a stale session signal. Happy to discuss further if you have additional test scenarios. |
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused retry fix. Current main still has the reported guard at gateway/platforms/weixin.py:1778, so removing it is a valid narrow improvement for errcode=-14 and the established stale-session signal.
Problems
- The first new test uses
ret=-3(tests/gateway/test_weixin.py:630in this PR). Current_is_stale_session_ret()only classifies -2 witherrmsg='unknown error'as stale (gateway/platforms/weixin.py:99-107), so -3 falls through to the error path atgateway/platforms/weixin.py:1814-1816even after this guard is removed. The #35062 discussion also reports that tokenless retry does not resolve its observed ret=-3 platform behavior.
Suggested changes
- Change that fixture to
ret=-2, errmsg='unknown error'(or retain only theerrcode=-14case) so the regression test exercises a session-expiry path current main actually recognizes. - Keep the one-line guard removal;
send()obtains a possibly absent cached token atgateway/platforms/weixin.py:1848and forwards it to this retry path at1892-1897.
Automated hermes-sweeper review.
| """Session-expired + no cached token → retry fires, then succeeds.""" | ||
| adapter = _make_adapter() | ||
| adapter._send_session = object() | ||
| adapter._token = "test-token" |
There was a problem hiding this comment.
Current _is_stale_session_ret() only treats ret/errcode=-2 with errmsg='unknown error' as stale; ret=-3 falls through to the error path even after this guard removal. Please use the supported -2 stale-session fixture here (the separate errcode=-14 test already covers the other recognized branch).
teknium1
added
sweeper:risk-message-delivery
4 participants
What does this PR do?
Removes the
and context_tokenguard from the tokenless retry branch in_send_text_chunk, allowing cron-initiated pushes to long-inactive WeChat chats to fall back to tokenless sends when no cached context token exists.Related Issue
Fixes #35062
Type of Change
Changes Made
gateway/platforms/weixin.py: Removeand context_tokenfrom the session-expired retry guard at line 1570 so the tokenless fallback fires regardless of whether a cached token existed.tests/gateway/test_weixin.py: AddTestWeixinTokenlessRetryWithoutCachedTokenwith two regression tests coveringret=-3(stale session) anderrcode=-14(classic session expired) whencontext_tokenisNone.How to Test
pytest tests/gateway/test_weixin.py -xvs -k TestWeixinTokenlessRetryWithoutCachedToken— both new tests should pass.pytest tests/gateway/test_weixin.py -xvs— all 56 tests should pass (no regressions)._send_messagereturnsret=-3, errmsg="unknown error"andcontext_token=None, the adapter should retry once without a token (the degraded fallback), then succeed on the second attempt.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/gateway/test_weixin.py -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/ACode Intelligence
_send_text_chunkingateway/platforms/weixin.py(line 1531-1625)_is_stale_session_ret()detectsret=-2/-3witherrmsg="unknown error"as stale session signals; the same guard pattern exists in_poll_loop(line 1289) for get_updates, but that path handles session expiry differently (10-minute pause, no token concept)