feat(feishu): interactive card model picker with two-level drill-down

[ShanNing-YU]

Summary

Implements send_model_picker for the Feishu/Lark plugin adapter (plugins/platforms/feishu/adapter.py), enabling an interactive card-based model selector — matching the UX that Telegram and Discord already have. Closes #20520.

Design

Two-level drill-down mirrors the Telegram adapter pattern:

• Provider selection: paginated card with current-provider marking
• Model selection: paginated at 8 per page with back/cancel navigation

Navigation is instant: the synchronous card-action callback returns a P2CardActionTriggerResponse carrying a CallBackCard, so Feishu swaps the card in place with no subprocess or external PATCH call (unlike the existing #10301 which targets the retired gateway/platforms/feishu.py).

The actual model switch runs in the background; the card is patched to its final green/red confirmation state via lark-cli (the lark-oapi SDK's message.update has a content-encoding bug for msg_type=interactive, so we route around it).

Why this PR is different from #10301

#10301	This PR
Targets retired gateway/platforms/feishu.py	Targets active plugins/platforms/feishu/adapter.py
Bundles unrelated model-list change	Contains only model-picker code (1 file + tests)
No tests	Includes 16 unit tests covering providers, models, pagination, dispatch, errors

Testing

• 16 new unit tests in tests/gateway/test_feishu_model_picker.py
• All 76 existing feishu tests pass unchanged
• E2E verified on WSL with live Feishu/Lark gateway

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

feat(feishu): interactive card model picker with two-level drill-down

Reviewed plugins/platforms/feishu/adapter.py and the new test file. The sync P2CardActionTriggerResponse navigation (no subprocess for page turns) is a nice design, and the card-builder tests are solid.

A few observations:

1. Key mismatch in _patch_model_picker_card's fallback. State is stored under picker_id (mp_<chat_id>), but when message_id is None the fallback does self._model_picker_state.get(str(chat_id), {}) — keyed by raw chat_id, which never matches. The "back-compat" _update_model_picker_card path therefore always logs "No message_id to update model picker card" and silently no-ops. Either store a chat_id -> picker_id index or key the fallback as mp_<chat_id>.

2. No operator authorization on picker actions. _handle_model_picker_card_action doesn't check event.operator.open_id before scheduling a switch — any member of the chat can tap a model button and change the shared model, while approval actions get explicit routing. If multi-user chats are possible, gate the picker actions like the approval path (or document the single-user assumption).

3. Stale picker state never expires. _model_picker_state[picker_id] is only popped after a switch attempt (success or failure); an abandoned picker keeps the callback and full providers list in memory until a later /model overwrites it. Consider an idle expiry.

4. Rapid double-tap races two switches. Two taps on different model buttons each schedule _execute_model_switch sharing one captured state; both call on_model_selected concurrently and the last card-patch wins, so the effective model is nondeterministic. A per-picker lock or an "already switching" guard would make it deterministic.

[ShanNing-YU]

Thanks for the thorough review — all four points are addressed, plus additional hardening found while auditing the same paths. The picker has also been verified end-to-end on a real Feishu client.

1. Key mismatch in _patch_model_picker_card fallback — Fixed: the fallback matches by the stored chat_id field (picker ids now also embed a random token).

2. No operator authorization on picker actions — Picker taps are now gated exactly like the approval path: _allow_group_message (group policy) + _is_interactive_operator_authorized (admins/allowlist), plus a chat-binding check.

3. Stale picker state never expires — Each picker carries created_at; interactions on a picker older than 5 minutes drop the state and return an "expired" card. _prune_model_picker_state also caps in-memory state.

4. Rapid double-tap races two switches — A switching flag is set synchronously before scheduling; repeated taps are ignored and return a "switching in progress" card. The flag is reset if scheduling fails.

Additional hardening found in the same paths:

• Server-side model validation — the tapped model_id must exist in the provider's model list (card payload is not trusted).
• Unguessable picker ids — mp_<chat>_<token> (uuid), per the triage note on unguessable expiring state.
• Failure title — a failed switch shows ❌ 切换失败 instead of the misleading ✅.
• Root cause of "stuck on switching" — the picker card is now sent with "update_multi": true, which Feishu requires for message.patch updates to render on clients (same root cause as approval cards in fix(feishu): approval card never updates — use im.v1.message.patch instead of update #8847). Without it, the final PATCH succeeded server-side but the client never re-rendered, leaving the card on "switching" forever.

Tests: tests/gateway/test_feishu_model_picker.py (18 tests) covers card sending, two-level navigation, tap dispatch, authz/expiry/double-tap guards, model validation, and the switch execution path. All pass; no regressions in the other Feishu suites.

[ShanNing-YU]

Rebased and ported to v0.20.6 — ready for review.

This PR has been updated to the current main (v0.20.6). Upstream refactored plugins/platforms/feishu/adapter.py substantially, so the picker code was re-ported onto the new adapter rather than applied as a stale overlay.

What's in this update:

• Targets the active adapter plugins/platforms/feishu/adapter.py (not the retired gateway/platforms/feishu.py that feat(feishu): interactive model picker card with two-level drill-down #10301/feat(feishu): Add interactive card model picker (send_model_picker) #20526/feat(feishu): interactive model picker card with two-level drill-down #26279 all hit).
• All 4 review points from the earlier AI review are addressed:
  1. Key mismatch in _patch_model_picker_card fallback — picker ids now embed a random token and the fallback matches by stored chat_id.
  2. Operator authorization — taps gated by _allow_group_message + _is_interactive_operator_authorized + chat binding, same pattern as the approval path.
  3. Stale state expiry — 5-minute idle timeout + _prune_model_picker_state capacity cap.
  4. Double-tap race — synchronous switching guard; repeated taps return a busy card.
• Plus hardening: server-side model validation (don't trust card payload), update_multi: true (required for message.patch to re-render on Feishu clients), and a text fallback message so the switch result is always visible.
• Minimal footprint: exactly 2 files (+498/-4 adapter.py, new test file).
• Tests: 16/16 passing on v0.20.6 (tests/gateway/test_feishu_model_picker.py) — updated to match the final hardened behaviour.

Closes #20520.

[sy3q]

Independent end-to-end verification (Windows 11 · Feishu cloud · WebSocket mode · current main)

I implemented the same picker locally before finding this PR, and can corroborate the two hardest parts of the diagnosis — plus one extra footgun that cost me a debugging round.

1. Confirmed working end-to-end on a real client. /model → provider card → paginated model card → tap → switch applied + confirmation delivered. Gateway logs confirm the callbacks arrive (card.action.trigger → hermes_model_action,picker_id,provider, then ...,idx,model,picker_id), so the two-level drill-down and the async switch path are sound on the current adapter.

2. CallBackCard.data must be the card object, not a JSON string. This silently breaks in-place navigation: card.data = json.dumps(card) is accepted, the API call succeeds, but the client never re-renders — the tap looks like a no-op. Passing the dict works (matches the existing _handle_approval_card_action pattern). Worth an assertion (isinstance(response.card.data, dict)) since the failure mode is invisible server-side.

3. Corroborating the update_multi: true root cause. I hit this from the other direction: every attempt to push a new card state via im.v1.message.update was rejected (Card update payload rejected by API; falling back to post in the gateway log), which made in-place updates look impossible for interactive cards. The update_multi: true + message.patch explanation is the missing piece — my workaround (send the switch result as a separate message) is only a fallback, not a fix. Nice catch.

4. Mutability note. The picker is only reachable when the adapter class defines send_model_picker (gateway/slash_commands.py probes getattr(type(adapter), "send_model_picker", None)); if it is ever renamed or moved onto the instance, /model silently falls back to the text list instead of erroring — worth a regression test.

Happy to re-verify after the next rebase if useful.

[Enough1122]

Read at head 9dfcdf472. Two things in the two follow-up reports need separating, because one of them is already true in this diff and the other is a real request worth taking.

@sy3q's CallBackCard.data finding is already satisfied here — and worth an assertion anyway. The in-place navigation path builds the response as:

def _resp(card_json):
    if P2CardActionTriggerResponse is None:
        return None
    response = P2CardActionTriggerResponse()
    if CallBackCard is not None:
        cb = CallBackCard()
        cb.type = "raw"
        cb.data = card_json          # the dict, not json.dumps(card)
        response.card = cb
    return response

card_json is the object returned by _build_provider_list_card / _build_model_list_card, assigned straight through. There is no json.dumps on this path — the only four json.dumps calls in the file are (a) the initial send_model_picker payload, (b) the visible text result message, and (c) the two in _patch_model_picker_card, which is a lark-cli api PATCH shell-out that has to serialise content as a string by necessity. So the silent-no-op failure you hit is not present in this head, presumably because of the same dict convention _handle_approval_card_action already uses.

That said, your point stands as a guard rather than a fix, and it is cheap: assert isinstance(response.card.data, dict) in test_feishu_model_picker.py on one _resp result. The value of it is that the failure mode you described — API call succeeds, client never re-renders, looks like a no-op server-side — is invisible in every other kind of check. I would add it.

On update_multi: true: the two reports agree on the cause and @ShanNing-YU got there first, so this is corroboration rather than a new finding. The card builders do set {"config": {"wide_screen_mode": True, "update_multi": True}} on both the provider and model cards, and the result message is deliberately sent as plain text as well — that belt-and-braces is what makes the outcome visible regardless of client re-render behaviour. Good call, and the plain-text fallback is the part I would keep even if update_multi turns out to be sufficient everywhere.

The four original points, re-read at this head: operator gating via _allow_group_message + _is_interactive_operator_authorized + a chat-binding check, switching set synchronously before scheduling, the 5-minute created_at expiry with _prune_model_picker_state and a 200-entry cap, and the fallback matching the stored chat_id rather than the formatted key. All four are present. The unguessable mp_<chat>_<token> id is what makes the chat-binding check meaningful, and server-side model_id validation against the provider's list is the one that stops a forged card payload from switching to a model the user cannot use — that was not in the original list and is the most valuable of the additions.

Not a blocker from me. Two things I have not verified and am not claiming: the end-to-end Feishu client behaviour (yours and @sy3q's respectively — I have no way to exercise a real Feishu tenant), and whether lark-cli is present in every deployment that reaches this code path, since _patch_model_picker_card shells out to it rather than using the SDK. If that CLI is optional, the picker still navigates (that path is the SDK P2CardActionTriggerResponse) but the final card refresh silently degrades to no-op while the text message still lands. Worth a sentence in the PR description either way.

Nothing outstanding from me.