Fix Lark CLI discovery and automatic reply readiness - #3285
Conversation
Signed-off-by: maxliu <lualiudd@gmail.com>
Signed-off-by: maxliu <lualiudd@gmail.com>
Signed-off-by: maxliu <lualiudd@gmail.com>
Signed-off-by: maxliu <lualiudd@gmail.com>
39e9302 to
98c3262
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
Review conclusion (community PR; exact head reviewed)
详细中文评审:Lark CLI 自动发现与自动回复就绪(PR #3285)
评审目标 head:39e93026c0a563fd3a87b78349af2fc741099592(base 0eb251fe,25 文件,+1718/-90)
改动范围与结构
该 PR 为 Lark 集成补齐三条链路:
- 新增
loopx/extensions/lark/cli_resolution.py:从显式配置、Goal Target、PATH、NVM、本地 npm 与常用命令目录解析lark-cli,并把解析结果包装成public_snapshot()(剥离本地路径)与subprocess_env()(把命令所在目录前置到 PATH)。 goal_topic_connections.py/chat_lark_api.py/chat_server.py:把“连接消息投递”与“自动回复就绪”分离;连接保存前 preflightim:message/im:message:readonly权限;不健康连接在状态面可见并输出脱敏诊断。- Dashboard/脚本侧:设置页展示权限引导与自动回复健康状态,新增
dashboard-dev.sh与 launchagent 改进。
发现的问题(阻塞合入)
- CI lint 失败(阻塞):pytest 检查在 “Lint test suite” 步骤失败,
ruff E731:tests/test_chat_lark_api_contract.py:332把 lambda 赋给runner,应改为def。这是测试代码风格问题,改动很小但当前 head 无法通过 CI。 - DCO / Sign-off 失败(阻塞):全部 4 个提交(
4f5753a3、34df2531、a6be3d95、39e93026)都缺少Signed-off-bytrailer,Sign-off 检查失败。请为每个提交补git commit -s --amend(或 rebase 后重新签名)再推送。
验证
- 我在 exact head 上实跑:
uv run --extra test python -m pytest -q tests/extensions/test_lark_cli_resolution.py tests/extensions/test_lark_goal_topic_connections.py tests/test_chat_lark_api_contract.py tests/test_dashboard_command.py→ 37 passed。 git diff --check干净;变更未发现凭据/本地绝对路径泄漏(/Users/仅出现在负例断言中)。- GitHub 检查:build、dependency-review 成功;pytest 在 lint 步骤失败;Sign-off 失败;build(新 run)pending。
非阻塞问题
- 提交中包含 dashboard/web 构建产物(
loopx/web/chat/assets/*);仓库惯例允许,但合入前请确认产物由当前源码重新构建。 - “自动回复就绪”依赖飞书管理员开通
im:message/im:message:readonly、发布新版本并重新授权——PR 已把该前置条件写进 UI 与 public_summary,方向正确;建议在文档/README 中同步补一句外部前置说明。 list_lark_group_chats从--as user改为--as bot并改为失败即抛错;若存在旧版 CLI 兼容场景,请在 PR 描述或文档注明最低 CLI 版本。
我的整体评价
实现与 PR 描述一致:CLI 发现不加载登录 shell、公共状态面不泄漏本地路径、连接保存前做权限 preflight、失败保持 fail-closed 且给出可执行修复提示。本地 37 个聚焦测试通过,代码层面未发现新的安全或权限边界问题。结论:CHANGES_REQUESTED,仅需修复上述 lint 与 DCO 两个 CI 阻塞(均为小改动),修复后建议重跑 CI 再合入。
English Verdict (PR #3285)
- Exact head reviewed:
39e93026c0a563fd3a87b78349af2fc741099592 - Verdict: CHANGES_REQUESTED
- Key findings: The implementation cleanly separates connection-message delivery from auto-reply readiness, resolves lark-cli without loading a login shell, keeps local paths out of public snapshots, and preflights
im:message/im:message:readonlybefore saving a Goal Topic connection. Local validation on the exact head: 37 focused tests pass;git diff --checkclean; no credential/path leaks. Blocking issues: ruff E731 attests/test_chat_lark_api_contract.py:332(lambda assignment -> use def), and all 4 commits lackSigned-off-by(DCO/Sign-off check failing). Non-blocking: confirm committed dashboard/web assets are rebuilt from source; document the Feishu admin permission prerequisite; note the minimum lark-cli version if--as botchanges behavior on older CLIs.
Signed-off-by: maxliu <lualiudd@gmail.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Re-review conclusion (exact head 98c3262)
详细中文评审(re-review):PR #3285 新 head
评审目标 head:98c3262c71ab25117baf77d0ca1d78635e14ae8c(相对上一 head 39e93026c:commit 重写,树内容未变)
进展确认
- ✅ DCO / Sign-off 已修复:4 个提交均含
Signed-off-by,GitHub Sign-off 检查成功。 - ✅ build 与 dependency-review 成功。
- ❌ pytest 检查仍失败:树内容与上一 head 完全相同,
ruff E731依旧在tests/test_chat_lark_api_contract.py:332(runner = lambda ...应改为def)。
剩余阻塞
- ruff E731(唯一剩余 CI 阻塞):把
runner = lambda *_args: {"returncode": 0, "stdout": "{}", "stderr": ""}
改为def runner(*_args): return {...},然后重跑 CI。
结论
代码评审立场不变:产品改动本身可接受(CLI 发现不加载登录 shell、公共面不泄漏路径、连接前权限 preflight、fail-closed)。本轮仍为 CHANGES_REQUESTED,仅剩上述 lint 修复;修复后 CI 全绿即可合入。
English Verdict (Re-review of PR #3285)
- Exact head:
98c3262c71ab25117baf77d0ca1d78635e14ae8c - Verdict: CHANGES_REQUESTED
- Progress: DCO/Sign-off fixed (all 4 commits now have Signed-off-by; Sign-off check passes), build and dependency-review pass. Remaining blocker: the tree is byte-identical to the previous head and still fails ruff E731 at
tests/test_chat_lark_api_contract.py:332(runner = lambda ...-> usedef). Once that one-line lint fix is pushed, CI should be green and the product changes remain acceptable.
huangruiteng
left a comment
There was a problem hiding this comment.
Re-review conclusion (exact head 0f993fd)
详细中文评审(re-review 2):PR #3285 新 head
评审目标 head:0f993fdd9746c4e99f489b844548ea9b1722d15b(相对上一 head 98c3262c:1 文件,+2/-1)
进展确认
- ✅ 剩余 lint 阻塞已修复:
tests/test_chat_lark_api_contract.py的runner = lambda ...已改为def runner(*_args);本地ruff check tests/test_chat_lark_api_contract.py→ All checks passed。 - ✅ DCO / Sign-off 保持通过(GitHub Sign-off success)。
- ✅ 本地聚焦测试:
uv run --extra test python -m pytest -q tests/extensions/test_lark_cli_resolution.py tests/extensions/test_lark_goal_topic_connections.py tests/test_chat_lark_api_contract.py tests/test_dashboard_command.py→ 37 passed。 - ⏳ GitHub CI:build×2 与 pytest 仍在运行;dependency-review 已通过。合入前请确认 CI 全绿。
结论
两个 CI 阻塞(E731 lint、DCO)均已解决,产品改动维持上一轮评审结论(CLI 发现不加载登录 shell、公共面不泄漏路径、连接前权限 preflight、fail-closed)。结论:APPROVE(待 CI 全绿后按仓库策略合入)。
English Verdict (Re-review 2 of PR #3285)
- Exact head:
0f993fdd9746c4e99f489b844548ea9b1722d15b - Verdict: APPROVE (pending CI green)
- Progress: The E731 lint blocker is fixed (
runner = lambda ...->def; localruff checkpasses), DCO/Sign-off passes, and 37 focused tests pass on the exact head. GitHub build/pytest are still running at review time; dependency-review already passed. Merge per repository policy once CI is fully green.
0af856a
into
codex/promote-personal-workspace-rfc
Summary
User-visible behavior
A Lark App that can create the Goal Topic but cannot read group messages is shown as Needs message permissions / 自动回复不可用. The connection editor explains the required scopes and disables saving until the app is published and re-authorized.
Validation
External prerequisite
The Feishu app administrator must enable
im:messageandim:message:readonly, publish a new app version, and re-authorize the profile before automatic replies can become ready.