fix(gateway): classify iLink stale sessions correctly

[fangliquanflq]

Summary

• classify iLink -14 and session-semantic -2 responses as stale sessions
• retry once without the cached context token, then surface an actionable session-refresh error
• keep stale-session failures out of the rate-limit circuit while preserving genuine frequency-limit handling

Testing

• scripts/run_tests.sh tests/gateway/test_weixin.py tests/gateway/test_weixin_typing.py tests/gateway/test_weixin_secret_scope.py -q (58 passed)

Related Issue

Closes #96416

[strzhao]

Thanks for this — the direction matches what we've seen in production, and the actionable-error branch (tokenless retry still stale → surface a refresh instruction, no circuit) is the right shape. Three evidence-based notes from our incident forensics, one race consideration, and a convergence thought.

1. The dominant stale variant in our capture isn't in the keyword set. Across three failing windows captured end-to-end (gateway up 8 days, zero inbound for 3+ days — evidence in #80125), the stale responses were:

{"ret": -2, "errcode": null, "errmsg": "prepare failed"}

"prepare failed" matches none of unknown error / session / context token, so this variant still falls through _is_stale_session_ret to the rate-limit branch and opens the circuit — the exact misclassification path from #96416. #80426 handles it as an exact match in _is_stale_context_token_ret; worth folding in, since it was the only stale signature we observed on -2 besides unknown error.

2. The tokenless recovery retry consumes the retry budget here. The recovery continue advances the for attempt in range(...) loop, so the stale-token probe eats one of _send_chunk_retries. #80426 restructures the loop so the recovery attempt is free — relevant because with the circuit misclassification fixed elsewhere but not here, budget semantics decide how much of the configured retry window is left after a stale bounce.

3. The token cleanup this PR inherits is a blind delete. The stale path still clears the store via _cache.pop(key). We measured the server issuing a fresh token on inbound messages — same key, same prefix, different value — while an outbound failure path was about to clear the store, so a blind pop can delete the new token; @ericcaiwx-star's Aug 24 reply in that thread confirms the race is real. #80426's delete(expected_token) CAS shape covers it, and its diff already pins the race with test_stale_response_does_not_delete_newer_context_token.

4. Wide substring matching cuts both ways. "session" in message is unverified against a genuine frequency-limit response — we've never captured a confirmed true rate-limit errmsg to test the net against, so the provable subset is the precise list (exact unknown error, exact prepare failed, plus the -14 code — your fold-in of the latter is a genuine improvement, as is the resp.get("msg") fallback).

Convergence: #80426 is the same-domain candidate carrying the production evidence (three independent sites now: ours, Akie-Star's, EricCai's — #96416 makes a fourth). Your -14 fold-in and msg fallback are clean increments over it; landing this space as one diff with authorship preserved would avoid re-splitting it — the split is what kept this bug family unresolved for months. Happy to help with the merge in whichever direction the maintainers prefer.

[fangliquanflq]

Thanks for the detailed production evidence. I addressed each point on this branch:

1. Added the exact outbound ret=-2 / prepare failed stale-context signature. It now takes the tokenless recovery/actionable-error path and never records a rate-limit event.
2. Moved the tokenless recovery outside the configured transient retry budget, including coverage with send_chunk_retries=0.
3. Replaced the blind cache removal with persisted compare-and-delete against the token that actually failed. A staged inbound-refresh regression test verifies that a concurrently issued fresh token survives.
4. Removed the broad session / context token substring matching. Classification is now limited to the observed exact unknown error and prepare failed responses plus code -14; genuine frequency-limit responses still use the breaker.

For convergence and attribution, I merged the implementation commit from #80426 into this branch and resolved it while retaining this PR's -14 classifier integration and errmsg/msg fallback.

Verification:

• scripts/run_tests.sh tests/gateway/test_weixin.py tests/gateway/test_weixin_typing.py tests/gateway/test_weixin_secret_scope.py -q — 58 passed
• Independent review of committed tip da66a88aac found no production-code defects; its only nit was missing coverage for exact unknown error and the msg fallback. Commit 5f17f6a92a adds both cases, and the focused suite remains green.

[strzhao]

Verified at 5f17f6a92a — everything in your reply checks out, including the parts that were easy to get subtly wrong:

• No rate-limit event from the stale path: traced every _record_rate_limit_event call site (there is exactly one, inside the rate-limit branch reached only after the stale branches continue/break), and test_stale_tokenless_failure_does_not_open_rate_limit_circuit pins it with threshold=1 — the exact configuration that produced the incident chain we captured on Aug 23 (prepare failed → event → breaker opens → cooldown copy masking the root cause in the logs).
• Recovery outside the budget: the stale continue never touches attempt, and re-introducing budget consumption there fails test_stale_context_token_recovers_with_zero_retry_budget. Termination is bounded — one probe per chunk: after the CAS delete the token is None, so a second stale response takes the actionable-error branch.
• CAS: delete() compares against the token that actually failed (captured before the retry) and persists via atomic_json_write; regressing it to a blind pop fails the staged-refresh regression (in-memory and after a fresh restore()).
• Cross-check against the production forensic package (our Aug 23 capture: full-chain gateway logs + token dumps): every classifier-relevant signature in the evidence — ret=-2 + errmsg='prepare failed', 31 occurrences, byte-identical form each time — classifies correctly at this head, and the derived breaker/cooldown lines that made up the rest of the incident no longer occur. No signature in the evidence is missed or wrongly routed to the breaker.
• Recovery semantics are honestly bounded, which matters: in our capture, tokenless retries went 0-for-11 against prepare-layer expiry — recovery only ever came from an inbound message (the server re-issues the token on the receive path; send/receive are separate). So for this failure shape the probe doesn't recover, but it costs exactly one bounded call, burns no budget, and the actionable error matches the only real remediation. It does recover the other sub-state (stale token with a live session, as at @ericcaiwx-star's site).
• 58 passed reproduces in a clean checkout of this head via scripts/run_tests.sh (46 + 4 + 8).
• The merge of 868ab527e0 is verbatim (same SHA, @nothing0here's authorship intact). As far as I'm concerned this is now the single landing diff for the weixin stale-token cluster — and the right one.

Three non-blocking notes for the record:

1. Media sends don't check ret — caption goes through _send_message and media items through a raw _api_post, neither response is inspected, and send() still returns success. Pre-existing on main, so out of scope here; but it lines up exactly with the "silent cron drop after session refresh" evidence just added on Weixin (iLink) adapter misclassifies stale session (-14/-2) as rate limit; misleading 'cooldown active' error blocks cron delivery #96416. Natural follow-up.
2. Optional cheap sentinel for the exact-match narrowing: a DEBUG log (or counter) of unclassified -2/-14 errmsg values would make decorated variants ("prepare failed." with a period, prefixed text) discoverable instead of silently falling back to the breaker. Not asking to widen the net — the narrowing is right; this is just a tripwire.
3. PR body still says "48 passed" — 58 at this head.

One correction to my own review: I wrote that "@ericcaiwx-star's Aug 24 reply confirms the race" — that Aug 24 comment in #80125 was mine; EricCai's production report was Aug 9. Misattribution on my part, apologies.

From my side this closes the review. cc @teknium1 for visibility only: with #80426's commit folded in here verbatim, this PR settles the cluster in one diff, and #80426 can close as superseded when it lands — authorship preserved in the chain.

[fangliquanflq]

Thanks for the thorough verification and the correction. I rechecked the current 5f17f6a92a diff against main and agree with the conclusions:

• The stale-session paths are already covered: exact unknown error, exact outbound prepare failed, -14, the msg fallback, zero-budget recovery, bounded tokenless failure, CAS preservation of a newer token, and breaker isolation all have focused regression coverage. No further production-code change is needed for this review.
• The media-response gap is pre-existing on main and is not caused by this two-file stale-token diff, so I am treating it as out of scope for this PR.
• The suggested diagnostic sentinel is informational rather than required: the exact-match narrowing is deliberate because broader message matching is not supported by captured rate-limit evidence.
• I updated the PR body from 48 to 58 focused tests.
• I also noted the corrected incident attribution.

Current required CI is green, including Python tests, E2E, Windows/macOS checks, and Python lint. A fresh local invocation of the focused command in this checkout could not start because no pytest-enabled virtualenv is installed, so I am not claiming an additional local pass from this run.

[ziqiuo]

Rebase needed to land. As of 2026-09-13 this PR is still mergeable=false / mergeable_state=dirty (last push 2026-08-27; branch base 726f0ce1b5, main is now 422bc9bde9). gateway/platforms/weixin.py has seen refactor passes on main since, so the rebase may need adaptation rather than a clean replay.

Fresh evidence for this cluster is now on #96416: a 10-day, 22/22 delivery-failure streak on v0.21.2, plus a direct-endpoint capture returning {"ret":-2,"errmsg":"prepare failed"} for both the tokenized and the tokenless send — consistent with @strzhao's 0/11 finding that the tokenless probe does not recover prepare-layer expiry. No diff change requested here; the rebase is the remaining blocker.
#96416 (comment)

[fangliquanflq]

Thanks for flagging the stale branch. I merged current upstream main into the PR branch and resolved the gateway/platforms/weixin.py conflict while preserving both sides' behavior. In particular, the stale-token compare-and-delete now uses main's async, serialized off-loop persistence path, and the tokenless recovery still remains outside the transient retry budget.

The new production capture is consistent with the existing exact ret=-2 / prepare failed classifier and does not require another behavior change in this PR.

Verification:

• scripts/run_tests.sh tests/gateway/test_weixin.py tests/gateway/test_weixin_typing.py tests/gateway/test_weixin_secret_scope.py -q — 60 passed
• Independent review of committed tip 16d3bbe910a — no findings
• GitHub now reports the PR head as mergeable against current main

[kshitijk4poor]

main now handles ret=-2 errmsg=prepare failed in two steps: #113069 (merge 46ab37ce35, salvage of #112713 by @KoNit-K) recognises it as a stale session and re-sends once without the context token, and #115359 (merge 5db9f02372, salvage of #80152 by @x7peeps with #80156 @JonthanaHanh) makes the unrecoverable case fail fast as a session error instead of opening the rate-limit breaker. Both halves this PR classified are now on main. Closing as fixed on main; thanks.