Skip to content

fix(feishu): support progress message cleanup - #21754

Closed
Bartok9 wants to merge 1 commit into
NousResearch:mainfrom
Bartok9:fix/21746-feishu-delete-message
Closed

fix(feishu): support progress message cleanup#21754
Bartok9 wants to merge 1 commit into
NousResearch:mainfrom
Bartok9:fix/21746-feishu-delete-message

Conversation

@Bartok9

@Bartok9 Bartok9 commented May 8, 2026

Copy link
Copy Markdown
Contributor

Problem

Feishu progress bubbles can remain visible after the final reply even when display.platforms.feishu.cleanup_progress: true is configured. Users then see stale edited progress messages, including duplicated (Edited) labels.

Fixes #21746

Root cause

The gateway only enables progress cleanup for adapters that override BasePlatformAdapter.delete_message. Feishu already supports progress updates through edit_message(), but the adapter never implemented delete_message(), so cleanup was disabled before it could delete temporary progress bubbles.

Fix

Implement FeishuAdapter.delete_message() using the Lark/Feishu message delete API path (client.im.v1.message.delete). A small request-builder helper mirrors the existing get/update/create request helpers and keeps tests working when lark-oapi is unavailable.

Testing

  • uv run --with pytest --with pytest-asyncio --with pytest-xdist --python 3.11 pytest tests/gateway/test_feishu.py -q -k 'edit_message or delete_message'
  • uv run --with ruff --python 3.11 ruff check gateway/platforms/feishu.py tests/gateway/test_feishu.py

Made with Cursor

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/feishu Feishu / Lark adapter P2 Medium — degraded but workaround exists labels May 8, 2026
@Bartok9
Bartok9 force-pushed the fix/21746-feishu-delete-message branch from 37fe9ab to dcf571f Compare May 8, 2026 15:00
@Bartok9

Bartok9 commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

Rebased on current main (839cdd1b0). Ready for review.

@Bartok9

Bartok9 commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

The test CI failure is pre-existing on main — verified at main SHA 1997b3ba. Not introduced by this PR.

@Bartok9

Bartok9 commented May 9, 2026

Copy link
Copy Markdown
Contributor Author

The test CI failure is pre-existing on main — verified at main SHA 524cbabd. Not introduced by this PR.

@KhanCold

Copy link
Copy Markdown

Hi @Bartok9, thanks for tackling this! I noticed the ruff + ty diff and test CI checks are failing on this PR.

I took a quick look and wanted to offer some help:

  1. For the ruff/ty failure: Could it be related to the new _build_delete_message_request staticmethod? The SimpleNamespace import might need a type annotation, or there could be a trailing whitespace issue in the docstring.

  2. For the test failure: If these are indeed pre-existing failures on main (as you noted), it might help to rebase once more or explicitly mention the failing test names in your PR description so maintainers can quickly verify.

I recently worked on a related Feishu fix (#22969) and have the dev environment set up. Happy to help debug the CI issues if you'd like — or I can hold off if you prefer to handle it yourself. Just let me know!

cc @teknium1 for visibility on the CI status.

@Bartok9
Bartok9 force-pushed the fix/21746-feishu-delete-message branch from dcf571f to 8525125 Compare May 10, 2026 06:56
@Bartok9

Bartok9 commented May 10, 2026

Copy link
Copy Markdown
Contributor Author

Hi @KhanCold — thanks for taking the time to look and for the offer to help debug! Really appreciated.

I dug into both failures:

ruff + ty diff: The 3 new invalid-argument-type diagnostics are in run_agent.py lines 6655/12545/12548 — none of which our PR touches. They appear to be a type inference variance in ty between the merge base and HEAD on a union type in the existing oauth token handling code. Since the lint check is advisory-only (never blocks merge per the workflow config), I've noted them but they're pre-existing.

test failures: I traced all the failing tests — they're unrelated to our changes:

  • context-1m-2025-08-07 beta header missing — Anthropic API constant, pre-existing
  • discover_mcp_tools not called — MCP tool discovery test, pre-existing
  • gateway.draining string mismatch — restart message test, pre-existing
  • Google Chat platform detection — platform config test, pre-existing

I've rebased onto the latest main just now to make sure we're not carrying any stale conflict. The _build_delete_message_request staticmethod itself is clean — ruff shows 0 new issues on our changed files.

Happy to add a # type: ignore annotation on the SimpleNamespace return if the maintainers prefer, but it looks like ty treats that union return type consistently with the rest of the file's pattern.

@Bartok9

Bartok9 commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

CI note: The failing checks (test, e2e, Windows footguns) are pre-existing failures on main at 3c23b15f815e — they fail without this PR's changes. Running pytest locally against main directly reproduces the same 12 failures. None of the failing tests touch the code changed in this PR. This PR is CI-clean with respect to its own scope.

Co-authored-by: Cursor <cursoragent@cursor.com>
@Bartok9
Bartok9 force-pushed the fix/21746-feishu-delete-message branch from 7f3fdce to d5c6b41 Compare May 15, 2026 05:33
@Bartok9

Bartok9 commented May 15, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main.

What I did:

  • Cherry-picked the original 852512591 fix commit onto a fresh branch off upstream/main
  • Dropped the auxiliary windows-footgun commit (7f3fdce) — main already has its own version of that comment so applying mine would be a noop or stylistic churn
  • The fix commit applied with an automatic merge in gateway/platforms/feishu.py — no manual conflict resolution required

Final diff: 2 files, +64 / -0 (additive only)

  • gateway/platforms/feishu.pydelete_message (uses client.im.v1.message.delete) + _build_delete_message_request helper that mirrors the existing get/update/create request builders
  • tests/gateway/test_feishu.pytest_delete_message_deletes_existing_feishu_message using the same patched-asyncio.to_thread pattern as the file's other adapter tests

Verified both files compile cleanly under Python 3.14. Ready for review. 🎻

@Bartok9

Bartok9 commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

Closing to stay under the repo's 10-PR contributor limit. Will resubmit with updated code if the issue is still open in main.

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/feishu Feishu / Lark adapter type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feishu: double "(Edited)" on progress messages + missing delete_message

3 participants