feat(plugins): unified plugin lifecycle readiness across discovery, enable, UI, scheduler, and credentials - #668
Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes plugin lifecycle “readiness” a first-class state (ready / needs_setup / needs_reauth / degraded / blocked / unknown) and threads it consistently through the desktop client: agent discovery (roster + ghost_list), plugin UI badges and enable flow, scheduler capability warnings, and a runtime credential-rejection ledger that can flip plugins into reauth-required.
Changes:
- Adds a unified lifecycle projection (main “pure” projection + shared wire types + renderer mirror) and pushes lifecycle updates via IPC.
- Degrades exposure for non-ready plugins (discoverable but
tools: []+ readiness/message), and implements “enable-then-guide” behavior (enable stays on, returns setup payload to guide the user). - Tracks runtime secret-key credential rejections (401/403 and certain 422s) into a local ledger and folds it into readiness as
needs_reauth.
Reviewed changes
Copilot reviewed 33 out of 34 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/cindy-tools/src/types.ts | Extends ghost_list/roster types with readiness + message and updated setup semantics. |
| packages/cindy-tools/src/ghost/mcpServer.ts | Roster formatting includes readiness markers and stronger “data not instruction” guidance. |
| packages/cindy-tools/src/tests/ghostMcp.test.ts | Updates/extends roster formatting tests for readiness markers and 2-line header. |
| docs/dev-rules/protocol-and-submodules.md | Updates client rules doc on plugin distribution channels (builtin seeding + defaultInstall). |
| docs/dev-rules/plugin-security-and-authoring.md | Documents readiness as the unified exposure gate and credential rejection behavior. |
| apps/desktop/src/shared/ipc-errors.ts | Adds IPC error code for setup-status evaluation failures. |
| apps/desktop/src/shared/ghostLifecycle.ts | New shared wire types for lifecycle projection across processes. |
| apps/desktop/src/renderer/vite-env.d.ts | Extends renderer ElectronAPI typings for lifecycle IPC + richer scheduler capability states + setEnabled return shape. |
| apps/desktop/src/renderer/i18n/locales/zh-CN/common.json | Adds readiness badge strings, setup-status-unavailable error, scheduler warn strings, and new aria label text. |
| apps/desktop/src/renderer/i18n/locales/ko/common.json | Same i18n additions for Korean. |
| apps/desktop/src/renderer/i18n/locales/ja/common.json | Same i18n additions for Japanese. |
| apps/desktop/src/renderer/i18n/locales/en/common.json | Same i18n additions for English. |
| apps/desktop/src/renderer/features/scheduler/components/ScriptCapabilityMultiSelect.tsx | Shows new warning variants for capability availability (needs setup/reauth/degraded). |
| apps/desktop/src/renderer/features/plugin/GhostReadinessBadge.tsx | New readiness badge component (non-ready only) using semantic tokens. |
| apps/desktop/src/renderer/features/plugin/GhostPluginPage.tsx | Implements enable-then-guide and market “install” CTA routing; integrates readiness badges. |
| apps/desktop/src/renderer/features/plugin/GhostPluginDetailView.tsx | Adds readiness badge in metadata row; refines toggle tooltip/aria semantics. |
| apps/desktop/src/renderer/features/plugin/tests/GhostPluginCard.test.tsx | Adds tests for market card action routing (install vs details). |
| apps/desktop/src/renderer/cindy-brain/lifecycleProjection.ts | New renderer-side external-store mirror for lifecycle projection updates. |
| apps/desktop/src/renderer/cindy-brain/installErrorKey.ts | Maps new IPC error code to i18n key. |
| apps/desktop/src/preload/preload.ts | Exposes lifecycle IPC and updated setEnabled return payload to renderer. |
| apps/desktop/src/main/scheduler-host/script-capability-status.ts | Extends capability availability resolution to incorporate readiness (setup/reauth/degraded/etc.). |
| apps/desktop/src/main/scheduler-host/tests/scriptCapabilityStatus.test.ts | Adds tests for readiness-to-warning mapping. |
| apps/desktop/src/main/mcp-integrations/ghost.ts | Aligns roster + ghost_list exposure with lifecycle projection; adds readiness/message and conservative workdir filtering. |
| apps/desktop/src/main/mcp-integrations/tests/ghostWorkdirGate.test.ts | Updates tests for new lifecycle-driven exposure and Codex empty-workdir filtering behavior. |
| apps/desktop/src/main/maker-ipc/schedule.ts | Sources scheduler capability warnings from lifecycle projection (single source of truth). |
| apps/desktop/src/main/cindy-brain/networkSlot.ts | Adds runtime credential-rejection detection and reporting hooks (401/403 + certain 422). |
| apps/desktop/src/main/cindy-brain/index.ts | Implements lifecycle projection, IPC handlers, lifecycle broadcasting, and integrates credential rejection ledger into setup assessment. |
| apps/desktop/src/main/cindy-brain/ghostWorkdirPrefs.ts | Adds helper to list disabled-ghost union across all workdirs for conservative filtering. |
| apps/desktop/src/main/cindy-brain/ghostSetupChangeBus.ts | Adds runtime_probe source and wildcard subscription for lifecycle projection broadcasting. |
| apps/desktop/src/main/cindy-brain/ghostLifecycle.ts | New pure projection logic for readiness + derived views + agent-facing summary strings. |
| apps/desktop/src/main/cindy-brain/ghostCredentialRejections.ts | New local ledger for runtime secret-key rejection tracking. |
| apps/desktop/src/main/cindy-brain/tests/networkSlot.test.ts | Adds/updates tests for credential rejection reporting behavior including 422 probing. |
| apps/desktop/src/main/cindy-brain/tests/ghostLifecycle.test.ts | New unit tests for lifecycle projection priority chain and derived view helpers. |
| apps/desktop/src/main/cindy-brain/tests/ghostCredentialRejections.test.ts | New tests for ledger idempotency, persistence, and fail-open behavior. |
Comments suppressed due to low confidence (1)
packages/cindy-tools/src/types.ts:288
getRosterItems().readinessis also typed asstringeven though the allowed values are fixed and already described in the comment. For consistency withCindyGhostInfo.readiness, this should use the sameCindyGhostReadinesstype.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Aggregate install/enable/setup/runtime facts into a single readiness projection (ready | needs_setup | needs_reauth | degraded | blocked | unknown) with a pure-function core (ghostLifecycle.ts), ghosts:lifecycle invoke, and ghosts:lifecycle-changed push driven by the setup change bus, ghosts:changed, and runtime crash/fuse transitions. Roster assessment failure now degrades explicitly: the entry stays discoverable but ships zero tools plus a guidance message, instead of silently omitting the setup field while exposing the full tool list. Signed-off-by: xxxcc <xxxcc123@126.com>
Roster snapshot (getRosterItems) and ghost_list now carry readiness from the lifecycle projection. Non-ready plugins stay discoverable but ship zero tools: needs_setup/needs_reauth entries include the sanitized assessment so the agent can drive setup cards; degraded/unknown entries carry only a guidance message. The strict ghost_call setup gate remains as the dispatch-time backstop. Contract: CindyGhostInfo.setup semantics tightened (required state implies empty tools; field absence never means ready), readiness field added to ghost_list entries and roster items. Signed-off-by: xxxcc <xxxcc123@126.com>
Enabling a plugin now evaluates setup readiness immediately after the enable takes effect (same probes as the plugin-page setup-status gate). When not ready, the response carries the setup payload and the plugin page opens the configuration flow right away — the plugin rests in an explicit enabled-but-unconfigured state instead of silently appearing usable to the agent. Evaluation failure rejects with the new SETUP_STATUS_UNAVAILABLE error code (enable still applied; the renderer shows a system error rather than a misleading setup prompt). Signed-off-by: xxxcc <xxxcc123@126.com>
…projection Script capability statuses now consume the same lifecycle projection as the agent discovery layer: needs_setup/needs_reauth/degraded/blocked/ unknown map to distinct run-time failure warnings in the schedule editor, instead of only checking the enabled flag. Signed-off-by: xxxcc <xxxcc123@126.com>
Renderer mirrors the lifecycle projection (lifecycleProjection.ts store fed by ghosts:lifecycle + ghosts:lifecycle-changed push) and renders a status badge on installed plugin cards and the detail header: needs setup / needs reauth / malfunctioning / unknown state, in all four locales, colors via semantic tokens only. Ready and blocked entries render no badge. Signed-off-by: xxxcc <xxxcc123@126.com>
The detail header switch is explicitly the global enable control (tooltip + aria), distinct from project-scoped disables which follow the scope picker; readiness badge i18n keys for the detail/card badges. Signed-off-by: xxxcc <xxxcc123@126.com>
Secret keys revoked server-side previously left the plugin forever in a configured-looking state. networkSlot now reports 401/403 responses (retry budget exhausted) per user-source secret key into a persistent ledger; getGhostSetupAssessment folds rejected keys into expired items, so the lifecycle projection degrades to needs_reauth and the discovery layer / plugin page learn on the next projection. Re-storing a secret clears the ledger via the setup change bus (all write paths emit). OAuth and connection credentials keep their own expiry chain and are not double-recorded. Signed-off-by: xxxcc <xxxcc123@126.com>
Codex sessions build the roster with an empty workingDir, which previously meant no workdir filtering at all — a plugin disabled in the session's project stayed visible in the Codex roster while Claude's roster hid it. Empty workingDir now falls back to the union of all workdir-scoped disables: conservative at the description snapshot layer, with runtime ghost_list/ghost_call still enforcing the precise workdir. Signed-off-by: xxxcc <xxxcc123@126.com>
protocol-and-submodules.md previously claimed provisioning was fully removed while the builtin seed provisioner and market defaultInstall both install silently — the rule was dead. Restate the three existing channels as intended behavior (first-party seeding, curated defaultInstall, manual confirmed install), and record the enabled-but-unconfigured lifecycle state as the product answer instead of a confirm dialog. plugin-security-and-authoring.md gains the readiness-as-first-class-state rule (degraded exposure, unknown never folds into ready, credential rejection ledger). Signed-off-by: xxxcc <xxxcc123@126.com>
ownerScopedUserDataPath() requires the Electron app; module-scope construction broke tests that import cindy-brain without Electron. Signed-off-by: xxxcc <xxxcc123@126.com>
Move the readiness badge into GhostPluginMetadata as a trailing inline element so it shares the text baseline of the author/version row instead of floating in a separate flex row. Signed-off-by: xxxcc <xxxcc123@126.com>
Brave returns HTTP 422 (not 401/403) for an invalid subscription token, so the runtime credential-rejection ledger never observed it and the plugin stayed 'ready' while every call failed. For plain user-source key secrets (no exchange, non-oauth), treat a 422 whose body mentions token/key/invalid keywords as a credential rejection too, so the lifecycle projection flips to needs_reauth. Business-logic 422s without those keywords are left untouched. Signed-off-by: xxxcc <xxxcc123@126.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
packages/cindy-tools/src/types.ts:115
CindyGhostInfo.readinessis typed asstring, which allows arbitrary/invalid values (including ones that could later be embedded into prompt text). Since the allowed readiness states are already documented here, it’s safer to model this as a string-literal union so hosts/agents can’t accidentally emit typos or unexpected values.
packages/cindy-tools/src/types.ts:288getRosterItems().readinessis also typed asstring, which weakens the contract and makes it easy to emit values outside the documented set. Consider narrowing it to the same readiness union asCindyGhostInfo.readiness.
Signed-off-by: xxxcc <xxxcc123@126.com>
8fd822d to
fad8ee7
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 33 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
packages/cindy-tools/src/types.ts:114
CindyGhostInfo.readinessis documented as a small, well-known enum, but it is typed asstring, which weakens the public contract and makes typos easy to miss at compile time. Prefer a string-literal union type that matches the documented values.
packages/cindy-tools/src/types.ts:287getRosterItems().readinessis also typed asstringeven though the allowed values are enumerated in the docstring. Reuse the same readiness type asCindyGhostInfoto keep the API consistent.
zqchris
left a comment
zqchris
left a comment
There was a problem hiding this comment.
先把决策面定下来，以下作为 owner 的正式决策记录：
- D1=B 同意：未就绪插件对 agent 降级暴露（可发现、不派发工具），方向按此推进。
- D2=B 同意：enable-then-guide，启用不回弹、立即引导配置。
- system prompt 文案确认：roster 头部两行说明 +
[needs_setup]标记的文案我认可；会话内快照机制核实属实，缓存声明成立。此前的程序性阻断就此解除。
代码这次先不合（PR 也还在 draft、CI verify 未跑完），有两个实打实的 [阻断] 要修，连同几条 [必改] 都在行级意见里。整体架子是对的——生命周期投影的分层干净，D2 的语义（不回弹 + 意图保留）还原得准，评估失败显式降级 unknown 的契约收紧也到位，这些不用动。
锚不到代码行的两条也要处理：
[必改]description 更正：「市场卡片『详情』改『安装』」在本 diff 里不存在，删掉或补实现；同时把 ghost_list 对未就绪插件从「给工具」变「tools:[]」的行为性变更写进「用户可见变化」。- Copilot 的 9 条行级意见目前一条都没处理（标 outdated 的只是行号漂移）。逐条要么修掉、要么回复不修的理由后 resolve；其中三条已并入我下面的行级意见。
[建议] 级不阻断，顺手收：台账写盘改 temp+rename 原子写（对照 plugin-market/ledger.ts）；blocked 态别复用「未完成配置」文案（账号面插件在本地模式会被误导）；ScriptCapabilityMultiSelect 嵌套三元改 state→key map；ghostLifecycle.ts 的 isDiscoverable/isCallable 目前无消费方；「待配置／需重新授权」按规则加 glossary.json 的 proposed 条目；无云端会话时投影对账号面插件的 warn 会刷屏，建议先短路。
改完 push 我再看一轮。投影分层和 D2 还原是这个 PR 里最扎实的部分，辛苦。
- cindy-tools: narrow readiness to a CindyGhostReadiness literal union
(forward-compatible via (string & {})) on CindyGhostInfo and roster items
- mcpServer: whitelist readiness tokens before rendering into roster text
so arbitrary/injected strings cannot leak into tool descriptions
- networkSlot: only ledger credentials whose (inject.hosts ?? net.hosts)
matches the final response host, so a rejection on one host no longer
flips unrelated secrets into needs_reauth
- ghostCredentialRejections: write the ledger atomically (temp+rename,
Windows replace handling) to avoid corrupting it on crash
- scheduler: split 'blocked' into its own ghost-blocked capability state
with dedicated i18n copy (cloud-session/account-services remediation,
not 'go configure')
- scheduler UI: replace nested ternary warn-key selection with a
state->key map
Signed-off-by: xxxcc <xxxcc123@126.com>
Reintroduce the market card 'install' CTA for not-installed items on top of the upstream unified install flow. The card button now pulls the full plugin detail (manifest included) and routes through the same runMarketInstall path as the detail page, so the confirm dialog still shows the full permission list / diff before install-or-enable — the card body remains the detail entry. Signed-off-by: xxxcc <xxxcc123@126.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
packages/cindy-tools/src/ghost/mcpServer.ts:190
- 花名册标题字符串里多了一个多余的右括号，且标题文案仍声称“以下是插件作者提供的描述”，但现在同一段里也包含 Host 注入的 readiness 状态标记；这会让提示词注入防护文案语义不准确。建议去掉多余括号，并把标题改成同时涵盖“作者描述 + 宿主状态标记”。
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d656cb0ac6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@xxxxxccc 👋 这个 PR 还有 2 条 review conversation 没 resolve(apps/desktop/src/main/cindy-brain/index.ts / apps/desktop/src/main/cindy-brain/networkSlot.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
Signed-off-by: xxxcc <xxxcc123@126.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 47 changed files in this pull request and generated no new comments.
Suppressed comments (4)
apps/desktop/src/renderer/features/plugin/GhostPluginPage.tsx:1538
- 这里对所有市场条目都传入了
onInstall，而 MarketPluginCard 会据此渲染“安装”操作；这会让已安装/可更新等条目也出现“安装”并触发 install 流程（可能造成重复安装或不符合状态的确认文案）。建议仅在installState === 'not-installed'时传入 onInstall。
apps/desktop/src/renderer/features/plugin/GhostPluginPage.tsx:1918 - GhostPluginCard 的标题行使用了
truncate包裹插件名并直接内联渲染徽章；当插件名较长时，徽章会一起被 ellipsis/overflow 裁掉，导致非 ready 状态在列表上不可见。建议把行容器改成 flex，并只让“名字”那段 truncate，徽章保持shrink-0独立显示。
apps/desktop/src/main/cindy-brain/ghostSetupChangeBus.ts:119 - JSDoc 注释里
events(e.g.缺少空格，影响可读性。
/**
* Subscribe to every plugin's change events(e.g. the lifecycle projection
* broadcaster, which re-reads all stores on any signal). Distinct from
* emitAll: this receives events for ghostIds that have no keyed subscriber.
*/
apps/desktop/src/renderer/features/plugin/GhostPluginPage.tsx:1522
- 这里对所有市场条目都传入了
onInstall，而 MarketPluginCard 会据此渲染“安装”操作；这会让已安装/可更新等条目也出现“安装”并触发 install 流程（可能造成重复安装或不符合状态的确认文案）。建议仅在installState === 'not-installed'时传入 onInstall。
This issue also appears on line 1534 of the same file.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 297cc15898
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@xxxxxccc 👋 这个 PR 现在在等维护者确认,确认之前流程不会合并它 —— 不是卡住了,也不是在等你再改一版(你推的改动流程都读到了,判的就是最新一版代码)。
这条是流程自动发的状态提醒(同一版代码只发一次),不用回复。 |
Signed-off-by: xxxcc <xxxcc123@126.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 47 changed files in this pull request and generated no new comments.
Suppressed comments (2)
apps/desktop/src/renderer/features/plugin/GhostPluginPage.tsx:1637
installable常量已声明但在组件内未使用，会留下死代码并让后续读者误以为存在基于 installState 的分支逻辑。建议直接移除，或在需要时再引入明确的条件渲染。
apps/desktop/src/renderer/features/plugin/GhostPluginPage.tsx:1918- 插件名使用
truncate的同一个<span>内直接拼接了 readiness 徽章，徽章本身是shrink-0，但父元素truncate会设置overflow-hidden，在标题较长时徽章可能被裁切/省略，导致“待配置/需重新授权”等关键信息不可见。建议把标题和徽章拆成 flex 布局：标题单独truncate，徽章独立放在旁边。
|
@xxxxxccc 👋 这个 PR 目前与 请在本地 merge 最新的 |
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
Heavy-tier security review passed: unified plugin lifecycle readiness correctly projects existing states without modifying schemas, approval records, or installation layouts. Credential rejection ledger is additive (ENOENT=fail-open), backwards compatibility fully preserved per Section 5 requirements.
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
代码审查发现 1 个 P1 问题:
P1: 403 分支遗漏 OAuth 撤销检测
文件: apps/desktop/src/main/cindy-brain/networkSlot.ts (line 1424)
401 分支正确调用了 noteOauthRejectionsIfCurrent，但 403 分支没有。同时 403 的门控条件（trackableKeys.length > 0 || responseConnectionRefs.size > 0）也不检查 OAuth 归属，导致仅凭 OAuth token 认证的插件如果上游 API 通过 403（而非 401）报告 token 撤销，永远不会被标记 needs_reauth——静默持续失败，无就绪信号。
这不是回归（PR 前完全没有 403 检测），但与本 PR 的核心目标（lifecycle readiness 准确性）直接冲突。
修复建议: 在 403 分支镜像 401 分支的 noteOauthRejectionsIfCurrent 调用，并扩展门控条件以包含 OAuth 归属检查。补充一个 OAuth + 403 revocation 场景的测试用例。
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
Heavy-tier 审查 + 安全专项通过:lifecycle readiness 投影架构完整,向下兼容存量插件(additive schema 不破坏已装/已批准状态),无凭证泄露、无沙箱逃逸风险。前几轮 reviewer 指出的六项问题(tool-name 绕过 setup_plan、per-host 归属、422 清理、workdir 取舍披露、readiness 类型联合、false decision 撤回)均已在当前修订中解决。
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
Review passed (heavy tier)
审查通过,零 P0/P1。
4 条 P2 观察(均为性能优化建议,不阻断):
apps/desktop/src/main/cindy-brain/index.ts:575— lifecycle projection 每次广播重算全部插件评估(当前事件频率低,不构成瓶颈)apps/desktop/src/main/mcp-integrations/ghost.ts:464— isGhostFused 为单插件查询调用全量 projection(N 小时无碍)packages/cindy-tools/src/ghost/mcpServer.ts:193— roster 格式精简(安全改进,ghost_list 仍提供全量)apps/desktop/src/main/cindy-brain/networkSlot.ts:756— credential rejection 正则仅覆盖英文(保守启发式,安全方向正确)
安全评估:本 PR 改善了安全态势——roster 防注入、凭证拒绝帐本仅存 key name 不存 value、owner 边界保护、IPC origin assert 均正确。无新增网络调用、无凭证泄露路径。
当前无法合并:存在代码冲突(mergeable=CONFLICTING),需作者 rebase。
|
代码审查已完成，结论：通过（零 P0/P1）。 但当前 PR 与 main 存在合并冲突，无法直接合并。请 rebase 到最新 main 解决冲突后，下一轮自动审查将直接尝试合并。 审查摘要：
|
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
代码审查通过（heavy 档：含 semgrep 安全扫描 + 恶意插件利用面检查）。
核心结论：
- 向下兼容：所有新增 wire-protocol/IPC 字段均为 optional/additive，已装插件升级后无需用户操作
- 竞态保护：凭证拒绝归因计数、指纹式 in-flight 变更检测（4 种凭证类型对称覆盖）、grant_only TOCTOU 二次校验、owner 边界丢弃逻辑均已验证
- 错误处理：ledger 写失败 fail-open（日志 + 保持运行态），损坏/缺失 ledger 有兜底，批量投影中单插件 assessment 错误隔离
- 恶意插件面：degraded/non-ready 插件始终
tools: []，roster 文本经过清洗和 enum 白名单校验，fused 插件同时阻断 normal call 和 grant_only 预授权 - 安全扫描：semgrep (p/javascript + p/nodejs) 0 findings
225/225 PR 相关测试通过，typecheck 两个 package 均 clean。无 P0/P1 问题。
|
✅ 代码审查通过（heavy 档），已 Approve。 |
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
代码审查通过（heavy tier + security scan）。
审查确认：
- 安全扫描无问题（无新依赖，凭证拒绝台账只存 key 名不存值，原子写入 mode 0o600）
- 插件向后兼容（REDLINE）：空台账 = 无降级，可选字段不影响旧消费者，未知字段被忽略
- 凭证指纹用 SHA-256 前 16 位防 stale-401 竞态
- 正则无 ReDoS 风险（锚定 + 有界回溯 {0,96}）
- IPC handler 已加 assertTrustedAppRendererEvent 守卫
- UI 组件使用语义 token，双模式兼容
阻断： 当前有合并冲突（mergeable=CONFLICTING），需要作者 rebase/merge main 解决后才能合并。
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
重审查通过：全部 7 处安全软命中逐条验证安全（密钥台账只记名不记值、IPC 都有 trusted-renderer 守卫、无能力扩散），存量插件兼容性完整，semgrep 扫描 0 条。
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
Automated review (heavy tier) complete — no P0/P1 found.
- All 7 dangerous pattern soft hits confirmed as false positives (bounded regex, owner-scoped file ops, trusted-renderer IPC, microtask coalescing)
- Plugin backward compatibility: new ledger is additive, ENOENT = empty, no mandatory fields added to existing stores
- Author contract (FORGE_GUIDE) unchanged — readiness is host-side only
- Security scan (semgrep): 0 findings against origin/main
- Credential handling: ledger stores only key names, never values
- IPC handlers correctly use assertTrustedAppRendererEvent
Note: awaiting maintainer re-approval before merge.
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
Heavy-tier code review passed. Security posture is improved: roster no longer includes plugin descriptions in system prompts (eliminates prompt injection vector), non-ready plugins no longer expose tool schemas, and credential fingerprint comparison prevents false-positive rejections. All 13 security soft hits are test fixtures. No actionable P0/P1 issues found.
Note: zqchris's earlier review comments may still need addressing.
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
自动审查通过：0 P0/P1。
逐项核实了 zqchris 此前指出的两个阻断点均已修复：
setup_plan路径下 tool name 校验跳过（ghost.ts:727, 780）- 401/403 凭证驳回归因改为 host-scoped + 403 body signal 门控（networkSlot.ts）
向下兼容验证：ledger 文件缺失 = 空（fail-open）、optional 字段、无 schema/布局/包格式变化。安全扫描 0 新发现，无新依赖。
当前唯一合并阻断：zqchris 的 CHANGES_REQUESTED 尚未 dismiss 或重新 approve。
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
Auto review (heavy tier): PASS — 0 P0, 0 P1, 1 P2 (minor indentation nit).
Security soft hit: SAFE — SHA-256 fingerprint used only for in-flight credential race detection; only key names persisted, never values; file mode 0o600.
Well-structured PR: credential rejection ledger, unified lifecycle projection, owner boundary guards, prompt injection hardening — all look sound.
Note: PR currently has merge conflicts that need resolution before merge.
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
Heavy-tier auto-review complete: 0 P0, 0 P1. All 7 security soft hits assessed as false positives. Code quality and test coverage are strong. However, this PR now has merge conflicts that need to be resolved before it can be merged.
这次改了什么
摘要
把插件生命周期统一成一等公民的 readiness 状态（ready / needs_setup / needs_reauth / degraded / blocked / unknown），贯穿 agent 发现层、启用门槛、UI 徽章、调度器与凭证失效闭环。动机：此前插件"已安装但不可用"（缺 Key、凭证被吊销、熔断）时对 agent 和用户都不透明——工具照样列出、调用才静默失败、启用开关回弹让人困惑。现在未就绪插件对 agent 降级暴露（可发现但零工具派发）、UI 有明确徽章、凭证失效能自动翻转状态。
设计来源：
plugin-lifecycle-issue-2026-07-27.md，采纳推荐案 B（降级暴露）与 D2=B（enable-then-guide）。全程只动客户端，服务端 installPolicy / catalog v3 / device-link / 发布链路均不在范围内。变更类型
feat新功能fix缺陷修复refactor/perfdocs/test/chore范围
ghostLifecycle.ts（纯函数推导 readiness）+ghosts:lifecycleinvoke /ghosts:lifecycle-changed推送ghost_list对非 ready 插件返回tools: []+ readiness + messageghosts:set-enabled启用后做 setup 评估并触发配置流程，不回弹ghost.ts注释披露，可回退为「建线期不过滤、运行期精确拦」）ghost_list/ghost_call。CindyGhostInfo新增可选字段readiness?/message?，向后兼容）UI 变化
插件卡片（已安装 + 市场）与详情页头部新增 readiness 徽章
详情页"作者 · 版本"行内嵌徽章，与文本同基线
未安装市场卡片按钮「详情」→「安装」
引用的设计规范：
docs/design-rules/DESIGN.md。徽章用语义 token（--warning-accent/--error-fgon--surface-chip），不做硬编码颜色；卡片交互遵循既有 hover/active 过渡规范。界面效果证据
改动后界面效果（结构 1:1 转录
GhostReadinessBadge/GhostPluginPage卡片 DOM，颜色取themes/colors.ts同名 token 的 light/dark 双值；prefers-color-scheme自动适配系统明暗）。浏览器打开即渲染：怎么验证的
自动验证
全量测试使用临时 Python 3 wrapper，避免本机
python命令解析到 Python 2.7；未修改仓库环境配置。Prompt / tool-definition 影响实测
formatGhostRoster对空花名册、2 插件代表样本、16 条上限样本测 UTF-8 体积；并用定向单测与本地 Agent turn 抽查ghost_list → setup_plan → 重新发现事件流。ghost_list发现，但tools: []；setup_plan可用占位 tool 到达配置卡且不会派发占位调用；配置完成后重新ghost_list才恢复真实工具。对应回归测试已覆盖。手工验证
在
pnpm restart:desktop:remote --region=cn --isolated=plugin-lifecycle（独立 userDataCindy-dev-plugin-lifecycle）逐项验证通过：$提问不派发其工具（日志确认无ghost_call）ghost_list降级：Web Searchtools: []+needs_setup，TapTap Makerready全量工具ghost_list恢复 ready$调用弹重新配置表单；422 按 owner 决策明确不纳入启发式判定未执行的验证
prefers-color-scheme自动双模式渲染（token 取值与themes/colors.ts一致），未做设备级截图对比风险
风险分类
影响与回滚
ghost_list/ghost_call工具描述进入 system 段。仓库维护者已在 2026-07-28 的实施会话中明确确认按推荐案 B / D2=B 落地并保留该 MCP tool-description 调整。会话常量快照机制保证单会话前缀稳定（不逐 token 变）；FORGE_GUIDE 无需同步（未改作者契约）。Prompt 体积与典型 turn 实测见「怎么验证的」。ghost-credential-rejections.json台账（仅 secret key 名，不存凭证值），损坏时 fail-open 按空账处理，不会把可用插件判死。docs/dev-rules/plugin-security-and-authoring.md第 5 节红线逐条自查）：ENOENT即按空账处理；读取不会顺手把文件创建出来，所以旧版本回退后看到的目录形态不变。含未知字段的台账按忽略处理、不判损坏（留回滚余地）。GhostManager启用态、oauth 账号面、连接清单、kv、manifest），没有新增 schema、没有新增必填字段、没有改指纹编码 / manifest 校验规则 / slot 形态 / 安装根与状态根 /.cindy包格式 / 管子协议 / 内置 id —— 因此不存在需要 backfill 的旧数据。ghost_list保留条目、不派发工具 + 引导配置），不是停用：开关照旧可点，已保存的凭证 / KV / per-plugin 偏好 / errand 配置一律不动。升级前已配置好的插件升级后仍判ready，用户什么都不做即照旧可用。apps/desktop/src/main/cindy-brain/__tests__/ghostCredentialRejections.test.ts的存量安装升级:台账缺失即空账,不降级已配置的插件—— 断言台账文件不存在时为空账且不被创建、无台账时折算为恒等变换（ready不会降required）、含未知字段的台账仍可读。main/cindy-brain/、mcp-integrations/ghost.ts、shared/ghost.ts、packages/cindy-tools的 ghost 部分、renderer 侧installErrorKey.ts/ghostPluginViewModel.ts/pluginMarketPresentation.ts），需放行人在 PR 上明确 Approve 后才能合并。提交前检查
git commit -s）