Skip to content

feat(feishu): add Feishu approval bridge + deepcode feishu CLI - #277

Closed
raymondginger2018-sudo wants to merge 1 commit into
HKUDS:mainfrom
raymondginger2018-sudo:pr/feishu-approval-bridge
Closed

raymondginger2018-sudo wants to merge 1 commit into
HKUDS:mainfrom
raymondginger2018-sudo:pr/feishu-approval-bridge

Conversation

@raymondginger2018-sudo

Copy link
Copy Markdown
Contributor

Summary

Add a Feishu (Lark) approval bridge: risky operations raise an approval.requested event, the bridge pushes an interactive card over Feishu's long-lived websocket, and an allowlisted open_id click submits approval/respond. Entry point: deepcode feishu start|status|stop.

Changes

  • integrations/feishu_approver/ — config.py / feishu_card.py / bridge.py / __main__.py (all new). Env-gated; lark_oapi is lazy-imported only when the bridge actually runs, so the core install stays lean.
  • cli/feishu_cli.py (new) — deepcode feishu start|status|stop CLI.
  • deepcode.py — CLI dispatch for feishu + one help-table row (the runtime call point).
  • setup.py — optional feishu extra: lark-oapi>=1.4.0 (pip install .[feishu]).
  • tests/test_feishu_approver.py (new) — 8 tests covering config allowlist, card payload, and bridge respond flow.

Tests

  • tests/test_feishu_approver.py: 8 passed (Windows, py3.12 / 3.13 / 3.14 legs of the local CI replay).
  • Full local CI replay (preflight, 10 jobs / 30 checks): PREFLIGHT: KNOWN-RED | new=0 | known=1 — zero new failures; the one known red is a pre-existing Windows-lifecycle baseline entry, unrelated to this diff (this PR touches no lifecycle code).

Dependency note

  • No new required dependencies. lark-oapi>=1.4.0 lives only in the optional feishu extra and is lazily imported; a core install without the extra is unaffected.
  • Internal imports all resolve against main: app_server.native_client.NativeRpcClient, app_server.service_state.ServiceFiles, core.persistence.database.default_database_path (verified via git grep on upstream/main).

Scope

Approval-bridge only. Phone remote control is intentionally deferred to a follow-up PR.

Local verification

VERDICT: KNOWN-RED | jobs=10 | rc0=29 | new=0 | known=1 | tree_head=94080eb7 | elapsed=2635s

Wire integrations/feishu_approver as an approval bridge: risky ops raise an
approval.requested event, the bridge pushes an interactive card over the
Feishu long-lived websocket, and an allowlisted open_id click submits
approval/respond. Config, card, and bridge modules are env-gated and lazy-import
lark_oapi so the core install stays lean. Optional [feishu] extra declares
lark-oapi; deepcode feishu start/status/stop is the CLI entry point.

Approval-bridge scope only (phone remote control is a follow-up PR).
@Zongwei9888

Copy link
Copy Markdown
Collaborator

Thanks a lot for this, @raymondginger2018-sudo! The bridge is cleanly separated: it only talks to the service over the authenticated RPC, the open_id allowlist fails closed, and the clicker's identity comes from Feishu's event rather than from the card.

We'd prefer not to carry it in core, though. It adds an SDK and a Feishu setup we can't exercise in CI, and since it needs nothing from core beyond the app server RPC, it works just as well as a standalone package with its own console script. If you publish it, we'll gladly link it from the docs.

A few things worth fixing before anyone relies on it:

  • Drop approve_once/approve_session as timeout actions and default to no auto-resolution (config.py:13-19, bridge.py:219-237). Core never times out approvals, and an unanswered prompt shouldn't approve itself.
  • Leave the approval pending, rather than denying it, when a card can't be delivered (bridge.py:256-262).
  • Reconnect after a service restart (on_close), and close cards on approval.resolved so a card answered on the desktop doesn't fire later.
  • The card shows only the first 500 characters of the arguments, and a backtick breaks out of the code span (feishu_card.py:98-126). Show the full, escaped command, and offer only "Approve once" / "Deny" remotely.
  • Send the "Approved" toast after approval/respond succeeds (bridge.py:478-487).
  • os.kill(pid, 0) isn't a liveness check on Windows; detach the way cli/service_cli.py does.

Thanks again!

@raymondginger2018-sudo

Copy link
Copy Markdown
Contributor Author

Thank you for the thorough review, @Zongwei9888 — and for the kind words about the separation of the bridge.

Agreed on the direction: since the bridge needs nothing from core beyond the app-server RPC, it belongs as a standalone package with its own console script rather than in core. I'm closing this PR and keeping the pr/feishu-approval-bridge branch as reference.

All six fixes you listed are captured and will land before any publish: no auto-resolution timeout actions (pending, never self-approved), leave approval pending when a card can't be delivered, reconnect on on_close + close cards on approval.resolved, full escaped command in the card with "Approve once"/"Deny" only, toast after approval/respond succeeds, and service-style detach instead of os.kill(pid, 0).

If/when the package is published I'll open a docs-only PR to link it, as you offered. Thanks again!

@Zongwei9888

Copy link
Copy Markdown
Collaborator

Thanks @raymondginger2018-sudo, that sounds like a good plan. Looking forward to the package; the docs link offer stands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants