Skip to content

fix(weixin): recover from stale context_token on outbound sends - #80426

Open
nothing0here wants to merge 1 commit into
NousResearch:mainfrom
nothing0here:fix/weixin-prepare-failed-stale-context
Open

fix(weixin): recover from stale context_token on outbound sends#80426
nothing0here wants to merge 1 commit into
NousResearch:mainfrom
nothing0here:fix/weixin-prepare-failed-stale-context

Conversation

@nothing0here

Copy link
Copy Markdown

What does this PR do?

Fixes outbound Weixin delivery when iLink reports a stale session as
ret=-2, errmsg="prepare failed" on sendmessage. The current adapter
only recognizes errmsg="unknown error" (or errcode=-14) as a
stale-session signal, so prepare failed falls through to rate-limit
handling: it opens the rate-limit circuit and puts Weixin into a
30-60s cooldown lockout. Long-running agent turns or cron-initiated
pushes that outlive the session regularly hit this, and every follow-up
send then fails fast with a misleading "rate limited" error.

This PR:

  • adds an outbound-only stale-context classifier that also recognizes
    prepare failed; the shared poll-path classifier keeps the narrower
    unknown error semantics;
  • deletes the cached context_token only if it still matches the token
    that failed (compare-and-delete), so a concurrently refreshed token is
    preserved;
  • loads the context token after acquiring the outbound send gate, so a
    fresh inbound token is picked up between chunks;
  • performs one tokenless recovery send without consuming the normal
    transient retry budget, and keeps later retries tokenless;
  • if the tokenless recovery also reports a stale session, raises a clear
    stale session error and does not open the rate-limit circuit;
    genuine rate limits (e.g. freq limit) still open the breaker.

Related Issue

Follow-up to #17228; complements #74572.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Changes Made

  • gateway/platforms/weixin.py: _is_stale_context_token_ret() classifier,
    ContextTokenStore.delete() compare-and-delete, under-gate token load,
    explicit retry accounting, and no-circuit stale-session error path.
  • tests/gateway/test_weixin.py: coverage for zero-retry tokenless recovery,
    multi-chunk tokenless continuation, bounded recovery without original-token
    reuse, circuit-not-opened on stale-session failure, concurrent fresh-token
    preservation, under-gate token load, classifier semantics, and
    ContextTokenStore.delete() persistence.

How to Test

  1. Let a Weixin chat idle until iLink starts returning
    ret=-2 errmsg="prepare failed", then attempt an outbound send
    (e.g. hermes send -t weixin "hello").
  2. Before: logs show iLink sendmessage rate limited; cooldown active for 30.0s/60.0s and subsequent sends fail fast while the circuit is open.
  3. After: logs show session expired ... retrying without context_token;
    if the tokenless recovery succeeds the message is delivered, otherwise a
    clear stale session ... wait for a new inbound message error is raised
    and no cooldown is opened.
  4. Unit: pytest tests/gateway/test_weixin.py -q -> 38 passed;
    ruff check gateway/platforms/weixin.py tests/gateway/test_weixin.py
    -> clean.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature
  • I've run pytest tests/ -q and all tests pass
    (weixin suite and ruff are green on this host; full repo suite not run)
  • I've added tests for my changes
  • I've tested on my platform: Debian 12 (VPS), Ubuntu-style install

Documentation & Housekeeping

  • N/A: no user-facing documentation/config/tool-schema changes
  • N/A: no cli-config.yaml.example changes
  • N/A: no CONTRIBUTING.md / AGENTS.md changes
  • N/A: cross-platform impact limited to one Python file + tests
  • N/A: no tool descriptions/schemas changed

Screenshots / Logs

Production log after the fix (stale startup notification):

WARNING gateway.platforms.weixin: [Weixin] session expired for o9cq800k; retrying without context_token
ERROR   gateway.platforms.weixin: [Weixin] send failed to=o9cq800k: iLink sendmessage stale session: no fresh context_token; wait for a new inbound message to refresh the session

No cooldown active line is emitted for the stale-session case.

iLink reports a stale session as ret=-2 errmsg="prepare failed" on the
sendmessage endpoint. The adapter previously classified this as a rate
limit, opening the rate-limit circuit and locking out all outbound
Weixin delivery (including cron-initiated pushes) until the cooldown
expires.

Classify "prepare failed" as an outbound stale-context signal, delete
the cached context_token only when it still matches the failed token,
load the token after acquiring the outbound gate, and allow one
tokenless recovery send outside the transient retry budget. If the
tokenless recovery also reports a stale session, surface a clear error
instead of tripping the rate-limit circuit; genuine rate limits (e.g.
"freq limit") still open the breaker.

Follow-up to NousResearch#17228; complements NousResearch#74572.
@ericcaiwx-star

Copy link
Copy Markdown

Production validation note

We independently patched a Hermes 0.16.0 Weixin deployment with the same recovery idea (prepare failed / empty errmsg → drop cached context_token → one tokenless retry) and confirmed it unblocked a nightly proactive report that had been failing every night with the mislabeled “rate limited / prepare failed” loop.

Also worth calling out for reviewers: if a no_agent script already sends via send_weixin_direct (or similar) and the job uses deliver: origin, Hermes will attempt a second stdout delivery after the script. That second path can still fail even when the script’s own sends succeed — operators should use deliver: local for self-sending scripts (docs PR opened separately).

Happy to help test / rebase this PR if useful — this looks like the right fix versus “fail fast with a better error” alone.

@nothing0here

Copy link
Copy Markdown
Author

Production validation note

We independently patched a Hermes 0.16.0 Weixin deployment with the same recovery idea (prepare failed / empty errmsg → drop cached context_token → one tokenless retry) and confirmed it unblocked a nightly proactive report that had been failing every night with the mislabeled “rate limited / prepare failed” loop.

Also worth calling out for reviewers: if a no_agent script already sends via send_weixin_direct (or similar) and the job uses deliver: origin, Hermes will attempt a second stdout delivery after the script. That second path can still fail even when the script’s own sends succeed — operators should use deliver: local for self-sending scripts (docs PR opened separately).

Happy to help test / rebase this PR if useful — this looks like the right fix versus “fail fast with a better error” alone.

Thanks for the production validation — could you help to test / rebase this PR?

@strzhao

strzhao commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Production evidence supporting this PR's shape — three failing windows captured end-to-end on our install (gateway up 8 days, zero inbound for 3 days, full chain in #80125 (comment)):

The "Tokenless recovery also reported a stale session" branch is the common path, not an edge case. Every window shows the exact sequence this PR handles: context_token present (age 22.5h+) → ret=-2 'prepare failed' → tokenless retry → still 'prepare failed'. An independent reproduction on current main (Windows 11, #80125) observed the same: a raw probe without the persisted token returns the identical response. Mechanistically the expiry is in iLink's session-prepare layer, not the token string — measured server-side TTL sits in (22.5h, 33.8h] since the last inbound, and only inbound refreshes it, so send-side retries of any kind can't self-heal. Your stale-session error pointing at "wait for a new inbound message" is precisely the actionable signal these installs need.

The compare-and-delete has a real race to win. Around an inbound-triggered recovery we diffed the stored token immediately before/after: same length, same prefix, different value — the server issues a new token on inbound rather than the client rewriting it. That means the failure path's delete() genuinely races a concurrent inbound refresh, and a non-CAS delete would discard the freshly-issued token and drop the recovery. Worth a test case if you don't have one yet: refresh arrives between the failed send and the tokenless attempt.

Circuit interaction — supporting data for the no-open-on-stale tests. In the pre-fix chain, one ret=-2 with threshold=1 logs rate-limit circuit OPENED: 1 event(s) and the cooldown fail-fast then swallows the retry chain, which is what turned a state problem into a 30s-throttle illusion for days. test_stale_tokenless_failure_does_not_open_rate_limit_circuit covers exactly the property our event chain says matters most.

One optional refinement: since every captured prepare failed window survives the tokenless attempt, a "prepare failed → go straight to the stale-session error" shortcut would save a server round-trip and surface the correct signal one step sooner. Keeping the attempt is also defensible (it's cheap and may cover other stale shapes) — flagging it as a possible follow-up rather than a change request.

@ericcaiwx-star

Copy link
Copy Markdown

Thanks @strzhao — those windows make the “tokenless retry still prepare failed” path look like the common case, not an edge, and the inbound-issued token race is a real reason to keep the compare-and-delete.

@nothing0here I won’t take the rebase from here. The branch is still mergeable; please rebase onto current main yourself (or a maintainer can). The CAS inbound-refresh test @strzhao described is a good add if you have cycles.

@alt-glitch alt-glitch added the sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state label Aug 24, 2026
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/wecom WeCom / WeChat Work adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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.

4 participants