Skip to content

fix(workspace): keep Goal context and disable Lark setup without lark-cli - #5264

Merged
huangruiteng merged 7 commits into
loopx-project:mainfrom
songoow:codex/workspace-lark-cli-capability-gate
Sep 29, 2026
Merged

huangruiteng merged 7 commits into
loopx-project:mainfrom
songoow:codex/workspace-lark-cli-capability-gate

Conversation

@songoow

@songoow songoow commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator

Goal And Delivered Outcome

  • Outcome basis / optional anchor: reproduced defect (no issue).
  • Goal/source and gap: without lark-cli, every /api/chat/lark/* request returns 503 lark_cli_not_installed. (1) The workspace fetched Goal repository contexts and Lark connections with one Promise.all, so that 503 also discarded every Goal's repository context and the Goal drawer silently lost its repository card. (2) Lark settings explained that lark-cli is missing but still offered New Lark App, whose setup dialog then failed with the same error.
  • Observable before → after: with lark-cli absent, before the Goal drawer had no repository card and New Lark App was enabled; after the repository card is shown and New Lark App is disabled next to the existing 未发现 lark-cli… explanation.
  • Issue/task and intended base: none; base main.

Scope And Continuation

  • Completed scope and remaining work: complete within this scope. The two optional sources now load and fail independently. Settings disables setup only for lark_cli_not_installed / lark_cli_not_executable, which the Chat service resolves once at startup; other Lark errors keep the current behavior.
  • Slice boundary / successor: the workspace still issues one Lark request that returns 503 when lark-cli is missing; /api/chat/capabilities already exposes lark_cli.available, and gating that request on it would need the capability threaded from dashboard-page.tsx. Not needed for this fix.

Validation

  • Tested revision: 714f256
  • Run state: finished
  • Input classes: synthetic
Check kind Result Public-safe evidence / limitation
static passed npx tsc --noEmit in apps/presentation/dashboard.
integration passed New scenario LOOPX_PERSONAL_WORKSPACE_SCENARIO=lark-cli-missing node examples/personal-workspace-browser-smoke.mjs (2 runs): Lark requests answer the service's 503; requires the loopx-ai/loopx repository card in the Goal drawer and a disabled New Lark App.
regression_parity passed Failing-before checks: with both source changes reverted the scenario stops at the missing repository card; with only the Settings change reverted it fails on the enabled New Lark App.
integration passed 16 scenarios that pass on main also pass here, including typed-actions (repository card with Lark available).
real_entrypoint passed loopx serve-status + loopx chat on an isolated synthetic registry on a host without lark-cli: main showed no repository card and an enabled New Lark App; this branch shows the card and disables the button.
integration not_run Six scenarios (goal-draft, capability-scope, steward-group-trigger, conversation-input, automation-cadence, steward-model-settings) already fail on a clean main checkout.
  • Coverage and gaps: both changed paths have a dedicated failing-before assertion and a real-service check. None identified beyond the scenarios already failing on main.

Frontend / Visual Evidence

  • UI impact: changed
  • Before: Goal drawer without a repository card; New Lark App enabled under the missing-lark-cli message.
  • After: repository card present; New Lark App disabled under the same message.
  • States and viewports shown: desktop 1440×900, lark-cli missing (screenshots available on request; not attachable from the CLI).
  • Source data: synthetic
  • Attention review: no new element; a control that could only fail is disabled beside the reason already on screen.

Type of Change

  • Bug fix
  • Test update

LoopX Area

  • Public docs or presentation surface (README, protocols, dashboard)

Technical Direction

  • Direction / acceptance reference, when applicable: Operator surface and IM integration.

Shared-authority RFC fixture impact

N/A

Boundary Checklist

  • Neither the diff nor this PR body/comments/attachments disclose private state, credentials, raw traces or verifier output, internal links, or local machine paths.
  • I did not duplicate maintainer-owned benchmark work unless a maintainer split out a public issue for it.
  • I kept the change scoped to the linked issue/task.
  • I completed the visual evidence section for UI changes, or marked UI impact none.
  • Every commit includes a DCO Signed-off-by trailer (git commit -s).

Future-facing refactor pass: considered threading capabilities.lark_cli into the workspace; deferred because the Settings page already receives the typed error code and the fix does not need a second source of the same fact.

…-cli

The workspace loaded Goal repository contexts and Lark connections with one
Promise.all. Without lark-cli every Lark request returns 503, which also
dropped every Goal's repository context, so the Goal drawer silently lost
its repository card.

Load the two optional sources independently. In Lark settings, a
lark_cli_not_installed or lark_cli_not_executable answer now also disables
New Lark App next to the existing explanation, instead of letting the setup
dialog fail with the same error.

Signed-off-by: song <liusongstep@gmail.com>
A new lark-cli-missing scenario answers Lark requests with the service's
lark_cli_not_installed 503 and requires the Goal repository card to stay
visible and New Lark App to be disabled.

Signed-off-by: song <liusongstep@gmail.com>
…cli-capability-gate

Signed-off-by: song <22676124+songoow@users.noreply.github.com>

@cocolord cocolord left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REQUEST_CHANGES — exact head a86023a7942e7f620ee281c1e47181c6f42aa857 对 Goal context 的修复和主按钮禁用都有可复现收益,但 Goal 级入口仍能绕过禁用并进入一个必然 503 的 Lark App setup,所以当前还不能 approve。

动机

这个 PR 针对两个真实问题:缺少 lark-cli 时,Lark API 返回 lark_cli_not_installed,原来的联合 Promise.all 会连带丢弃独立成功的 Goal repository context;同时 Lark Apps 页仍提供“新建 Lark App”,用户进入后只能得到同一个错误。前者会让每个 Goal drawer 丢失仓库卡片,后者制造明确的操作死路。

我用完全相同的 missing-CLI fixture 比较 immutable base 与当前 head:base 为 repositoryVisible=false, newAppDisabled=false,head 为 repositoryVisible=true, newAppDisabled=true。因此主路径收益是明确且正向的,不是仅凭代码推断。但 PR 所声明的“缺少 CLI 时禁用 setup”只覆盖了一个入口,尚未形成完整可用的用户结果。

改动思路

PersonalWorkspacePage 把 Goal context 与 Lark connections 从一个共同失败域拆成两个独立 promise,让可选 Lark 集成的错误不再覆盖仓库上下文。LarkSettingsPage 则从既有 ChatApiError.payload.error_code 精确识别 lark_cli_not_installed / lark_cli_not_executable,保存为当前服务生命周期内的派生 UI 状态,并禁用 Apps 页工具栏按钮。

这个边界选择总体合理:服务端 LarkCliResolution 已是可用性的权威来源,前端复用结构化错误码,没有引入 substring/prose 分类或第二个持久状态 owner;成功 refresh 会清除 unavailable 状态,未知/provider 错误也不会被误判为 CLI 缺失。问题在于这个派生状态没有由共享的 setup 入口统一消费。

具体改动

关键代码讲解

PersonalWorkspacePage 的 optional-source effect(personal-workspace-page.tsx:1025)现在分别调用 fetchGoalContexts 与 fetchLarkConnections,各自检查 cancellation 并更新独立 state。隔离浏览器中,当两个 Lark 请求都返回 503 时,Goal repository 卡片仍能从 /api/chat/goals/contexts 正常显示,修复有效。

larkCliUnavailable(lark-settings-page.tsx:160)对两个服务端既有错误码做精确集合匹配;refresh(约 line 236)在失败时派生 cliUnavailable,成功时重置。这是局部、可恢复、没有权限扩张的实现,也复用了现有 larkErrorMessage 和 ChatApiError 通道。

openSetup(lark-settings-page.tsx:418)仍是所有新 App setup 的实际入口,负责打开 modal;startSetup 随后 POST /api/chat/lark/app-setups。工具栏调用点(line 531)新增了 disabled={cliUnavailable},但连接弹窗中的 App selector(line 597)仍始终渲染 __register__,选择后直接调用未受保护的 openSetup。

新增 lark-cli-missing browser smoke 有持久价值:它覆盖独立 Goal context、现有错误文案和工具栏禁用,并且在 base 上能对两个旧缺陷 fail。遗憾的是它从 machine Settings 进入,只断言工具栏按钮,没有走 Goal drawer 会自动打开的 connection dialog,因此漏掉了第二个 setup trigger。

对主干的风险

[P1] 当前缺少 lark-cli 时仍可从 Goal 级 Lark 入口打开并提交 App setup。触发路径是 Goal 信息 → 连接 Lark App → App selector 的“+ 注册另一个 Lark App — 通过飞书创建”;focusGoalConnection 会在失败的 refresh 结束后自动打开 connection dialog,line 597 不检查 cliUnavailable,随后 openSetup 也没有防守。隔离浏览器实测 setup modal 成功打开,点击继续后又显示同一个 lark_cli_not_installed 错误。

这直接违反当前 PR 的用户级义务:既然服务已确定该操作在重启前不可能成功,就不应只禁用一个视觉入口而保留同页替代入口。最低修复是在共享 openSetup 边界拒绝 unavailable 状态,并在 selector 中禁用或移除 __register__;可以同时避免在无任何 App 时自动打开空的 connection modal。回归应从 Goal drawer 进入,断言 setup dialog/POST 均不可达,并保留 CLI available 时仍能注册的正向断言。

独立验证方面,dashboard TypeScript/production/chat bundle build、Personal Workspace contract smoke、提交内 lark-cli-missing、相邻 typed-actions 以及 loopx canary premerge --from-git-diff 均通过;premerge 为 5 项、0 failure、0 manual hold,public/private scan clean。steward-group-trigger 的一次 head 运行因审查 worktree 外链依赖字体被 Vite 403 拒绝,base 同场景通过;这是本地依赖链接限制,没有作为 PR 结论依据。

GitHub 当前四个 Python shard、聚合 pytest 与 merge gate 为红。我在 immutable base 738115bde87eef3fd153abe456d53da5e2b249f8 和当前 head 跑同一组四个 quota/scheduler 测试(含参数化),两边均为 6 failed / 4 passed,失败项与断言形态一致:quota plan file/sqlite、global gate missing/stale、standard settlement ack、autonomous replan ack mode。它们和本 PR 的四个前端/fixture 文件无因果路径,是既有且无关的 merge-readiness hold,不是本次 REQUEST_CHANGES 的依据。

语义与 CI 对齐

本 PR 复用了既有 lark_cli_not_installed / lark_cli_not_executable、ChatApiError 和 App setup 路由,没有创造新共享词汇,也没有把 availability 当成 activation 或权限。typed state 与 authority 命名本身一致;阻断点是同一派生状态对同一 setup 操作的覆盖不完整。

我的整体评价

这是一个小而有价值的修复:独立 source loading 让 Goal repository 恢复可见,精确错误码也比文本判断稳健,代码量与问题规模匹配。但严格按完整用户旅程看,当前页同时告诉用户 CLI 缺失,又允许从 Goal 入口进入并提交必失败的 setup;因此 long-horizon 仍未证明完整,user experience 在新契约下仍有回归,不能 approve。

建议保留现有两个生产改动,只把 cliUnavailable 收口到共享 setup owner,并把现有 smoke 扩到 Goal 入口,无需引入新的 capability plumbing。修复后重跑 missing-CLI 主/绕过场景、available-CLI 相邻场景、dashboard build 与 premerge 即可复审。未来相关 refactor pass 已定位为统一两个 openSetup 调用点;除此之外没有必要扩大范围。

English verdict: REQUEST_CHANGES - exact head a86023a7942e7f620ee281c1e47181c6f42aa857 demonstrably preserves Goal repository context and disables the primary New Lark App button when lark-cli is missing, but the Goal-scoped connection dialog still exposes __register__, opens the same setup modal, and submits an operation that predictably returns lark_cli_not_installed. Gate the shared setup entrypoint and add a Goal-entry regression. The six quota/scheduler CI failures reproduce identically on base and head and are unrelated.

The toolbar's New Lark App was disabled without lark-cli, but the Goal
drawer's Connect Lark App dialog still offered "register another Lark App",
which opened the same setup and failed with lark_cli_not_installed.

openSetup, shared by both entries, now refuses while lark-cli is
unavailable, and the dialog's register option is disabled.

Signed-off-by: song <liusongstep@gmail.com>
lark-cli-missing now opens Connect Lark App from the Goal drawer and requires
the register option to settle disabled, no setup dialog and no setup request;
with lark-cli available again, New Lark App is enabled.

Signed-off-by: song <liusongstep@gmail.com>
@songoow

songoow commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the [P1] Goal-entry bypass at ec84cf79c:

  • openSetup — the one setup entry shared by the toolbar and the connection dialog — now returns while lark_cli_not_installed / lark_cli_not_executable is known, and the dialog's __register__ option is disabled. Both entries now meet the same fact.
  • lark-cli-missing now opens Goal 信息 → 连接 Lark App, waits for the register option to settle disabled (the dialog opens before its Lark read fails), and requires no setup dialog and no /api/chat/lark/app-setups request. It then unroutes the 503 and requires 新建 Lark App to be enabled again (CLI-available positive case).

Validation: scenario 5/5 on a warm Vite; failing-before check with the source change reverted fails at the register-option assertion; typed-actions, chat-recovery, team-plan pass. Note for anyone running this scenario alone with an empty node_modules/.vite: Vite's first dependency optimization force-reloads the page ~2 s in, which drops the Goal selection; that is an environment artifact seen equally on the previous head, not a workspace remount.

Not changed: the dialog still auto-opens on the Goal entry when no App exists; left as is to keep this fix to the setup gate.

@songoow
songoow requested a review from cocolord September 29, 2026 10:18

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE — exact head ec84cf79ce683723eb50eb3ccc0c583554874724;复审上次 a86023a7942e7f620ee281c1e47181c6f42aa857 的阻断,并重新检查整份 PR。

动机

没有 lark-cli 时,Chat 的 Lark 读取返回结构化 503。原来的联合 Promise.all 让无关的 Goal repository context 一起消失,而 Lark Settings 仍把必然失败的 App setup 当成可执行操作。按同一份缺 CLI 浏览器场景重放:合并基线 738115bde87eef3fd153abe456d53da5e2b249f8 找不到 Goal 仓库卡片;上次审过的 head 能显示卡片,却仍开放 Goal 弹窗里的注册入口;本 head 显示卡片并阻断两个 setup 入口。就这个已证实的用户问题而言,目标已经交付。长期 Goal 工作仍能读取仓库上下文;缺失的可选集成不会再制造 setup 重试死路,用户安装 CLI 并重启服务后可恢复。

改动思路

PersonalWorkspacePage 在原有可选数据读取位置,把 Goal contexts 与 Lark connections 改为各自处理成功和失败;这比把 Lark 可用性从 dashboard 上层再传一遍更贴近当前故障边界。LarkSettingsPage 仍以 Chat 服务的 LarkCliResolution 为权威事实,通过既有 ChatApiError.payload.error_code 精确识别 lark_cli_not_installed、lark_cli_not_executable。派生的 cliUnavailable 只服务于当前页面的按钮、选项及共享 openSetup,成功刷新会清除它;没有新增持久事实、权限或第二套服务端决策。先不发版的最强理由是上次 head 只禁用了工具栏按钮,Goal 弹窗可绕过;本次共享入口的防守加禁用选项,配合从 Goal 出发的浏览器回归,解决了该理由。

具体改动

关键代码讲解

  • personal-workspace-page.tsx:1034 分别启动 fetchGoalContexts 和 fetchLarkConnections,各自通过 cancelled 避免卸载后写状态。Lark 503 不再抹掉已成功取得的 repository;只读分支仍清空两份展示数据。
  • lark-settings-page.tsx:158 的 larkCliUnavailable 对服务端两个固定错误码作集合匹配,没有按错误文案作 substring 推断。refresh 在 503 时派生不可用,在成功时清除;对 provider_api_failed 不误禁用 setup。
  • lark-settings-page.tsx:418 的 openSetup 是工具栏和 Goal connection dialog 共用的入口;现在它在 CLI 已知不可用时直接返回。工具栏按钮与 dialog 的 __register__ 选项也同步禁用,前者提供可见状态,后者堵住上次复审发现的旁路。
  • 新的 lark-cli-missing 场景接入现有 personal workspace browser catalog:从 Goal drawer 验证 repository 与 dialog 选项,监听 setup POST,再检查 Settings 的错误文案和按钮,并在恢复可用的场景确认按钮重新开放。这些断言保护已发货的 UI 行为,没有另起测试框架。

对主干的风险

先说明一个非阻断 P2:从 Goal 信息点击“连接 Lark App”后,缺 CLI 时仍自动打开完整的连接表单。浏览器实测注册选项已禁用、setup dialog 和 POST 都没有发生,但缺 CLI 的准确提示留在被遮罩的 Settings 页面;弹窗内反而显示“机器人尚未加入可连接的群”,容易让用户误判为群聊配置问题。建议后续在该已知状态下不自动打开连接弹窗,或直接在弹窗内显示安装并重启的原因;关闭弹窗后仍可看到正确错误,本 PR 不因此阻断。

独立验证使用同一个缺 CLI 浏览器脚本:base 在仓库卡片断言失败,旧 head 在 Goal 注册选项断言失败,本 head 通过。当前 head 的开发模式和打包 Chat 浏览器场景、typed-actions、chat-recovery、team-plan、Personal Workspace contract、TypeScript 与 dashboard/Chat 生产构建均通过。额外注入 lark_cli_not_executable 会禁用按钮,provider_api_failed 不会;loopx canary premerge --from-git-diff 在安装仓库开发依赖后为 5 项通过、0 失败、0 人工 hold,diff/public-private 扫描通过。第一次 canary 因审查环境缺根目录 TypeScript 依赖失败,补齐依赖后同一检查通过。此前评审记录的 quota/scheduler Python 失败属于独立的合并就绪问题;此 Goal 的评审策略不等待远端 CI,本次结论也不授予合并权限。

语义与 CI 对齐

这次改动复用了现有结构化 Lark 错误码和 ChatApiError,没有新公共协议或权限语义。缺 CLI 是服务启动时确定的可用性事实,不等于启用 Lark;其他错误仍沿用原先可尝试 setup 的行为。测试覆盖从已知不可用到恢复后的用户入口,以及独立 Goal context 在 Lark 失败时继续可见。

我的整体评价

相较上次 REQUEST_CHANGES,两个新增提交只收口共享 setup 入口并补强 Goal 入口场景,没有把小修复扩成能力配置重构。整份 PR 的 long_horizon 为保留、user_experience 为改进;缺 CLI 的主要用户结果现在从真实页面路径可复现。生产代码仅两处局部改动,新增测试是对原有浏览器 catalog 的一个持久场景;兼容分支和持久状态都未增加。弹窗中的错误解释值得单独改善,但不再允许提交必败 setup。基于上述确切 head 和本地验证,我批准此 PR 的代码审查;合并仍须处理独立 CI/维护者门槛。

English verdict: APPROVE - exact head ec84cf79ce683723eb50eb3ccc0c583554874724 preserves Goal repository context when Lark lacks its CLI and closes the Goal-dialog setup bypass. Base and previous-head browser replays fail at their respective regressions; current development and packaged browser checks, adjacent scenarios, builds, and premerge canary pass. The auto-opened Goal dialog could explain the missing CLI more clearly (non-blocking P2); separate CI merge holds remain.

huangruiteng
huangruiteng previously approved these changes Sep 29, 2026

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

动机

这条 PR 修复缺少 lark-cli 时两个相互关联的工作区故障:Lark 请求的 503 不应抹掉独立成功的 Goal 仓库上下文;已经知道安装前必败时,也不应继续提供 Lark App setup。上次在 a86023a7942e7f620ee281c1e47181c6f42aa857 的评审确认了仓库卡片与工具栏按钮的收益,但发现 Goal 信息抽屉仍可从“连接 Lark App”绕过。当前 ec84cf79ce683723eb50eb3ccc0c583554874724 新增共享入口防守和该路径的浏览器回归,完成的是同一用户结果,而非另起一个功能。

改动思路

PersonalWorkspacePage 将 Goal contexts 和 Lark connections 拆成独立异步读取,失败域只覆盖各自可选来源。LarkSettingsPage 复用服务端已有的 lark_cli_not_installed / lark_cli_not_executable 结构化错误,派生本页的 cliUnavailable;成功刷新时清除它,不建立第二份持久可用性权威。工具栏按钮与连接弹窗选项都消费同一状态,实际 openSetup 再作一道共同入口防守。这样无 CLI 时不丢 Goal 信息,有 CLI 或其它错误时也不误封 setup;安装及重启后的恢复仍由既有服务/刷新路径负责。

具体改动

关键代码讲解

  • personal-workspace-page.tsx 的 optional-source effect(约 line 1032)把原先的 Promise.all 分成两个各自带取消检查和错误边界的读取。Lark 503 不再阻止 /api/chat/goals/contexts 更新仓库卡片;其它 Goal 页面行为不变。
  • lark-settings-page.tsx 的 larkCliUnavailable(line 160)只匹配两个明确错误码;refresh(约 line 243)失败时更新派生状态、成功时归零。它没有用错误文案 substring 推断能力,也没有扩大权限。
  • 同文件 openSetup(line 418)现在是工具栏和 Goal 连接弹窗共用的拒绝点;工具栏(line 531)和 __register__ 选项(line 597)还给出可见的 disabled 状态。后者正是上轮遗漏的替代入口。
  • 新 lark-cli-missing.mjs 场景被 personal-workspace-browser-smoke.mjs 注册,依次检查 Goal 仓库卡片、Goal 抽屉的连接入口、选项禁用、无 setup modal/POST、设置页禁用和 CLI 可用后的重新启用。测试覆盖实际 React 页面及异步刷新,而不是只断言 helper 返回值。

本轮独立在该 head 的 detached worktree 跑通 development 与 packaged 的 lark-cli-missing、相邻 typed-actions、TypeScript 检查、Vite 桌面构建和 Chat bundle 构建;完整 base→head diff 的 git diff --check 及补齐根目录 npm 依赖后的 loopx canary premerge --from-git-diff 也通过。旧 base 与上一 head 的同输入对照已在前次公开评审记录;这次我检查了它们的 revision、当前完整 diff,并重新执行新增 Goal 入口场景,未把旧的批准结论直接继承过来。

对主干的风险

主要风险是只禁一个按钮却留下第二个 setup 通道。新 head 在两个可见入口和共同 openSetup 都挡住缺失 CLI;连接弹窗加载期间 select 本身禁用,读取结束后 __register__ 随 cliUnavailable 禁用。我在实际浏览器场景中验证没有 /api/chat/lark/app-setups 请求;解除 503 后同一工具栏重新可用,typed-actions 也通过。Goal repository 与 Lark 连接的读取先后次序现在可独立完成,这是有意的 UI 默认行为变化,PR 描述与场景均已披露。

未直接验证机器上真实安装/卸载 lark-cli 的服务重启过程;这里的浏览器 fixture 使用与服务端相同的 typed 503 形状,packaged 复测证明产物路径一致。远端 CI 按该 Goal 的 wait_for_ci=false 配置未查询或等待;即使有无关红灯,也不构成本 PR 的 REQUEST_CHANGES 依据。未来可考虑利用 /api/chat/capabilities 避免一次已知的 Lark 503,但需要额外贯穿能力状态,当前不是关闭用户死路的前提。

语义与 CI 对齐

此改动复用既有服务端错误码和 ChatApiError,可用性仅影响 setup UI,不把 CLI 存在解释成授权,也不引入新持久协议、模糊词法规则或仅靠文案执行的义务。仓库卡片仍由 Goal context 来源负责;Lark 失败只影响 Lark 部分。当前本地正反场景及构建通过,合并就绪仍由独立门禁判断。

我的整体评价

当前 head 关闭了上次的 Goal 入口绕过:长期看独立来源不会相互拖累,缺失 CLI 时用户不会在两个入口重复进入必败 setup;CLI 可用后又能回到原工作流。long_horizon 与 user_experience 均改善,少量页面状态和一条持久浏览器场景与缺陷规模相称。相邻 future-facing 检查未发现需要另建能力状态 owner;保留当前派生状态比为省一个 503 扩大跨层配置更合适。我批准这个准确 head;这不授予合并权限,未验证的真实安装/重启与远端 CI 仍按各自门禁处理。

English verdict: APPROVE - exact head ec84cf79ce683723eb50eb3ccc0c583554874724 keeps Goal repository context when Lark returns typed missing-CLI 503 and closes both Lark App setup entries, including the previously missed Goal drawer path. Development and packaged browser regressions, available-CLI adjacent scenario, TypeScript and builds pass; remote CI was not polled.

cocolord
cocolord previously approved these changes Sep 29, 2026

@cocolord cocolord left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE — exact head ec84cf79ce683723eb50eb3ccc0c583554874724 已关闭上一轮 Goal-scoped Lark setup 绕过:缺少 lark-cli 时,Apps 工具栏与 Goal connection dialog 的注册入口都不可用,共享 openSetup 也会 fail closed;CLI 恢复可用后入口能重新启用。独立 Goal repository context 的修复继续成立。

动机

这个 PR 解决两个可复现的 Personal Workspace 问题。第一,Goal repository context 与可选 Lark connections 原本处于同一个 Promise.all 失败域;主机缺少 lark-cli 时,Lark 503 会连带让成功返回的 repository context 消失。第二,页面已经展示“未发现 lark-cli”,却仍允许进入一个在服务重启前必然失败的 Lark App setup。

收益已用同一 missing-CLI fixture 做过 base/head 对照:固定 base 会隐藏 repository card 并保留可操作的 setup;当前 head 保留 repository card,同时让两个 setup 入口都不可达。新 head 还补上了上一轮指出的 Goal drawer 绕过,而不是只让工具栏按钮变灰。

改动思路

PersonalWorkspacePage 将 Goal contexts 与 Lark connections 拆为两个独立异步读取,各自更新自己的 projection,使可选集成失败不再覆盖无关数据。LarkSettingsPage 继续复用服务端现有的结构化错误码 lark_cli_not_installed / lark_cli_not_executable,派生当前服务生命周期内的 cliUnavailable,成功 refresh 时清除它。

setup gate 现在收敛到现有 owner:Apps 工具栏按钮消费该状态,Goal connection dialog 的 __register__ option 也消费它,而共享 openSetup 再做一次防守。没有新增持久状态、权限或第二套 capability authority;可用性仍来自 Chat service 的启动时 LarkCliResolution,前端只负责如实投影。

具体改动

完整 PR 为 4 个文件、+105/-8。两个生产文件分别处理 optional-source 隔离和 Lark setup availability;browser runner 注册一个新的 lark-cli-missing 场景,场景覆盖 repository card、主按钮、Goal drawer、register option、setup request 计数以及恢复可用后的反向路径。没有改动 API schema、持久化、scheduler、quota 或 benchmark 行为。

关键代码讲解

  • PersonalWorkspacePage 的 optional-source effect 不再等待一个联合 Promise.all;fetchGoalContexts 与 fetchLarkConnections 分别 settle、分别处理 cancellation,因此后者 503 不会抹掉前者结果。
  • larkCliUnavailable 使用精确错误码集合,不依赖文案 substring;refresh 在错误时派生 unavailable 状态,在成功时显式恢复。未知/provider 错误不会被误判成 CLI 缺失。
  • openSetup 是工具栏和 connection dialog 的共享 setup owner。新 head 在这里检查 cliUnavailable,即使以后另一个 caller 忘记禁用控件,也不会打开 modal。
  • App selector 的 __register__ option 在 unavailable 时禁用,给用户即时且一致的可见状态;loading 期间整个 selector 已禁用,不存在读取尚未完成时的点击窗口。
  • lark-cli-missing browser smoke 从 Goal drawer 打开 connection dialog,等待注册项进入 disabled,确认没有 setup modal/POST;随后恢复正常 Lark reads 并 reload,确认“新建 Lark App”重新启用。

对主干的风险

主要风险是 unavailable projection 过宽、过窄或永久粘住。当前实现只对服务端两个稳定的 CLI readiness 错误码启用 gate;其他 Lark/provider 错误保留原行为。服务端在启动时解析 CLI,因此“安装后需重启”的错误文案与前端状态生命周期一致;成功 refresh 会清除 cliUnavailable,浏览器反向用例也证明入口不会永久禁用。

我在 exact head 运行了 development 与 packaged headless Chrome 的 lark-cli-missing 场景:Goal repository 可见,Goal-scoped __register__ 禁用,setup dialog 与 /api/chat/lark/app-setups 请求均未出现;恢复 available responses 并 reload 后主按钮重新启用。相邻 typed-actions 浏览器场景、Personal Workspace contract、Dashboard TypeScript/production/chat bundle build 均通过。risk-based premerge 的 3 个 direct + 5 个 selected checks 全绿,0 failure、0 warning、0 manual hold,4 个公开候选文件边界扫描无命中。

GitHub required CI 当前并未全绿:test-shard (1)、(2)、(3) 失败,聚合 pytest 与 merge-gate 因而失败。我没有把红灯忽略掉:CI 的 4 个底层失败分别是两个 quota scheduler acknowledgement 用例和两个 project-registry manifest 用例;用同一条聚焦命令在当前主干父提交 ee1ea64b0aef45fdda81d2d7e48da356a1750eab 与 GitHub 合成 merge commit 1a490445c411bc9a9a533454d049d7e4bb53993e 上运行,均得到相同的 4 个失败签名。PR 的完整 diff 只包含上述 4 个 Dashboard/browser 文件,没有改动对应的 loopx/cli.py、quota、registry scanner 或测试路径;changed invariant 另有 source/packaged Chrome、build 和 premerge 的独立通过证据。因此这些红灯归因为 current-main 上已存在且与本 PR 无关,不改变代码 review verdict,但在 owner 修复主干并重跑 checks 前,merge readiness 必须保持阻塞。

语义与 CI 对齐

typed state 使用精确 error_code,没有 substring denylist;availability 只影响 setup reachability,不授予 Lark 凭据或操作权限,也没有把机器门禁称作 guidance。行为变化在 PR body 和 browser scenario 中明确披露。现有两个生产入口都复用同一个 derived state 与 shared openSetup owner,未发现新的 domain vocabulary 或第二份 authority。

我的整体评价

APPROVE。这个 exact head 不仅在代码上补了上一轮缺口,也通过真实 source/packaged Chrome 验证了完整用户旅程和反向恢复:独立 repository context 保留、已知不可能的 setup 全入口关闭、可用状态不受影响。改动规模与确定性缺陷匹配,测试是可长期保留的 optional-dependency 边界回归。

future-facing pass 已考虑把 /api/chat/capabilities 继续向下传递,但当前服务端 typed error 已是同一启动时事实,额外 plumbing 会扩大状态同步面;本 PR 复用现有 owner 更合适。没有发现需要在此 PR 扩大的相邻重构。Approval 仅表示当前 exact head 的代码评审结论,不授予 merge authority;当前 required CI 为红,主干修复并重跑全绿前不得合并。

English verdict: APPROVE - exact head ec84cf79ce683723eb50eb3ccc0c583554874724. The prior Goal-scoped setup bypass is closed at both presentation and shared-action boundaries: the register option is disabled and openSetup fails closed while lark-cli is unavailable. Source and packaged headless-Chrome regressions confirm that repository context remains visible, no setup modal or request is reachable, and setup becomes available again after healthy Lark reads. The adjacent typed-actions scenario, workspace contract, production/chat builds, risk-based premerge, and public-boundary scan also pass. Three Python shards are red, but all four underlying failures reproduce identically on the current-main parent and the synthesized merge commit while the PR does not touch their causal paths; this is non-blocking for the code-review verdict but merge readiness stays blocked until required CI is repaired and green.

Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>

# Conflicts:
#	examples/personal-workspace-browser-smoke.mjs
@huangruiteng
huangruiteng dismissed stale reviews from cocolord and themself via e6129d4 September 29, 2026 11:18

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #5264 复审 — exact head e6129d42cfa661250740047cef214d18a9055fe5

Approval conclusion (maintainer review on a contributor PR)

Exact head: e6129d42cfa661250740047cef214d18a9055fe5;base ec0304511。上一轮在 ec84cf79ce683723eb50eb3ccc0c583554874724 的结论(huangruiteng 与 cocolord
两份 APPROVE)针对的是当时的分支;本 head 是与最新 origin/main 合并、解决 CONFLICTING 之后的
结果,因此按新 exact head 重新验证,没有沿用旧结论。

动机

主机缺少 lark-cli 时,Personal Workspace 有两个真实故障:Chat 的 Lark 读取返回结构化 503,
原来与 Goal repository context 共用一个 Promise.all 失败域,于是独立成功返回的仓库卡片被一起
丢掉;同时 Lark Apps 页面仍然提供"新建 Lark App",用户点进去只能得到同一个必然失败的错误。
上一轮 review 已经发现 Goal 级入口仍能绕过禁用,作者在 c406395b9/ec84cf79c 补上共享入口防守
与浏览器回归,这两件事现在是同一个用户结果。

改动思路

失败域按来源拆开:Goal contexts 与 Lark connections 各自异步读取,Lark 侧失败只影响自己的可选
来源,不再连带丢弃仓库上下文。禁用逻辑收在一个共享 openSetup 守卫上,Apps 工具栏按钮、Goal
连接弹窗与设置页共用它,因此不会再出现某一处绕过;lark-cli 恢复可用后入口自动重新启用。
与 main 的合并只触及浏览器场景目录:两侧各新增一个场景,取并集即可。

具体改动

相对最新 main 共 4 个文件:Lark 设置页(16 行)、Personal Workspace 页面的读取拆分(12 行)、
场景目录(2 行)与新的 lark-cli-missing 浏览器场景(75 行)。本 head 额外只有一个合并提交:
examples/personal-workspace-browser-smoke.mjs 同时保留作者的 larkCliMissingScenario 与 main 的
executionServiceOfflineScenario。没有后端、API、CLI 参数或持久字段变化。

对主干的风险

主要风险是失败域拆分后把真正的 Lark 错误吞掉,或禁用逻辑过宽导致 lark-cli 正常时也无法配置。
浏览器场景覆盖两条路径:缺 CLI 时仓库卡片仍在、工具栏与 Goal 弹窗的注册入口都不可用、共享
openSetup fail closed;CLI 恢复后入口重新启用。合并后的整份 personal-workspace 场景(24 个,
含 main 新增的 execution-service-offline)全部通过。主干另有与本变更无关的红灯:registry I/O
census 与 Goal instance inventory 的 manifest 漂移,以及两个 quota settlement CLI 用例——我在干净
的 origin/main checkout 上复现了同样的失败,本 diff 只改前端。

我的整体评价

两个问题都是真实且可复现的用户路径,修法复用既有读取与能力信号,没有新增状态 owner 或第二份
Lark 判定;共享 setup 守卫比逐处补丁更耐久。验证覆盖 Dashboard 构建、覆盖率运行器、workspace
契约 smoke 与 24 个真实浏览器场景(含本 PR 的 lark-cli-missing 与 main 的 execution-service-offline),
目标内 premerge 5 项检查通过、0 manual holds、质量回执 valid。结论:APPROVE,可在本 exact head
合并。

English verdict: APPROVE - exact head e6129d4. Goal repository context and Lark connections now fail
independently, and every Lark App setup entry (toolbar, Goal connection dialog, settings page) shares
one fail-closed guard that reopens when lark-cli returns. Dashboard build, coverage runner, contract
smoke and 24 real browser scenarios pass on the merged head; the remaining red checks reproduce on
untouched origin/main.

Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #5264 复审 — exact head 43c219e86e24c1ec506b65212f62c283f9ffde55

Approval conclusion (maintainer review on a contributor PR)

Exact head: 43c219e86e24c1ec506b65212f62c283f9ffde55;base 616909dfe。上一轮在 ec84cf79ce683723eb50eb3ccc0c583554874724 的结论(huangruiteng 与 cocolord
两份 APPROVE)针对的是当时的分支;本 head 是与最新 origin/main 合并、解决 CONFLICTING 之后的
结果,因此按新 exact head 重新验证,没有沿用旧结论。

动机

主机缺少 lark-cli 时,Personal Workspace 有两个真实故障:Chat 的 Lark 读取返回结构化 503,
原来与 Goal repository context 共用一个 Promise.all 失败域,于是独立成功返回的仓库卡片被一起
丢掉;同时 Lark Apps 页面仍然提供"新建 Lark App",用户点进去只能得到同一个必然失败的错误。
上一轮 review 已经发现 Goal 级入口仍能绕过禁用,作者在 c406395b9/ec84cf79c 补上共享入口防守
与浏览器回归,这两件事现在是同一个用户结果。

改动思路

失败域按来源拆开:Goal contexts 与 Lark connections 各自异步读取,Lark 侧失败只影响自己的可选
来源,不再连带丢弃仓库上下文。禁用逻辑收在一个共享 openSetup 守卫上,Apps 工具栏按钮、Goal
连接弹窗与设置页共用它,因此不会再出现某一处绕过;lark-cli 恢复可用后入口自动重新启用。
与 main 的合并只触及浏览器场景目录:两侧各新增一个场景,取并集即可。

具体改动

相对最新 main 共 4 个文件:Lark 设置页(16 行)、Personal Workspace 页面的读取拆分(12 行)、
场景目录(2 行)与新的 lark-cli-missing 浏览器场景(75 行)。本 head 额外只有一个合并提交:
examples/personal-workspace-browser-smoke.mjs 同时保留作者的 larkCliMissingScenario 与 main 的
executionServiceOfflineScenario。没有后端、API、CLI 参数或持久字段变化。

对主干的风险

主要风险是失败域拆分后把真正的 Lark 错误吞掉,或禁用逻辑过宽导致 lark-cli 正常时也无法配置。
浏览器场景覆盖两条路径:缺 CLI 时仓库卡片仍在、工具栏与 Goal 弹窗的注册入口都不可用、共享
openSetup fail closed;CLI 恢复后入口重新启用。合并后的整份 personal-workspace 场景(24 个,
含 main 新增的 execution-service-offline)全部通过。主干另有与本变更无关的红灯:registry I/O
census 与 Goal instance inventory 的 manifest 漂移,以及两个 quota settlement CLI 用例——我在干净
的 origin/main checkout 上复现了同样的失败,本 diff 只改前端。

我的整体评价

两个问题都是真实且可复现的用户路径,修法复用既有读取与能力信号,没有新增状态 owner 或第二份
Lark 判定;共享 setup 守卫比逐处补丁更耐久。验证覆盖 Dashboard 构建、覆盖率运行器、workspace
契约 smoke 与 24 个真实浏览器场景(含本 PR 的 lark-cli-missing 与 main 的 execution-service-offline),
目标内 premerge 5 项检查通过、0 manual holds、质量回执 valid。结论:APPROVE,可在本 exact head
合并。

English verdict: APPROVE - exact head 43c219e. Goal repository context and Lark connections now fail
independently, and every Lark App setup entry (toolbar, Goal connection dialog, settings page) shares
one fail-closed guard that reopens when lark-cli returns. Dashboard build, coverage runner, contract
smoke and 24 real browser scenarios pass on the merged head; the remaining red checks reproduce on
untouched origin/main.

@huangruiteng
huangruiteng merged commit 84d0028 into loopx-project:main Sep 29, 2026
5 checks passed
@huangruiteng

Copy link
Copy Markdown
Collaborator

Merged as 84d0028902 (squash, admin bypass) on the exact reviewed head 43c219e86e.

Exact-head evidence:

Repair carried in this head:

  • The PR was CONFLICTING. Merging origin/main conflicted in examples/personal-workspace-browser-smoke.mjs: both sides appended a new browser scenario to the same import list and catalog, so the resolution is the union — this branch's larkCliMissingScenario together with main's executionServiceOfflineScenario. No production file needed rework.
  • Main then moved again; the merged head was re-verified and re-approved rather than inheriting the earlier approvals.

Validation on this head (43c219e86e): Dashboard build:desktop (tsc --noEmit + vite build); dashboard coverage runner; personal-workspace drawer contract smoke; the full 24-scenario personal-workspace browser smoke including this PR's lark-cli-missing scenario and main's execution-service-offline scenario; tests/architecture/test_project_registry_io_census.py and test_goal_instance_binding_inventory.py (10 cases, green after merging main's census fix); and a 5-check goal-scoped premerge with a valid quality receipt.

Known red not owned by this PR: tests/control_plane/test_quota_settlement_cli.py has two failing cases (test_standard_codex_app_settlement_is_receipted_and_idempotent, test_todoless_autonomous_replan_settles_quota_refresh_spend_chain). I reproduced the identical failures on a clean origin/main checkout (77 others pass), and this diff only touches Personal Workspace frontend files.

Admin bypass was used because the repository ruleset requires an extra approval for changes not attributable to the PR author (songoow), which the maintainer account that resolved and pushed the merge cannot self-satisfy; the exact head still carries a code-owner APPROVE review, a valid change-quality receipt and a goal-scoped premerge pass. GitHub CI was not awaited on this final head; the local evidence above covers the affected surfaces.

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.

3 participants