Skip to content

fix(importers): discover Pi agent models and MCP servers - #1750

Closed
IswordSun wants to merge 3 commits into
vastsa:mainfrom
IswordSun:fix/pi-agent-import
Closed

IswordSun wants to merge 3 commits into
vastsa:mainfrom
IswordSun:fix/pi-agent-import

Conversation

@IswordSun

Copy link
Copy Markdown

修复内容

PI-Desktop 的显式导入扫描漏掉了 PI agent 的两类配置:

  1. 模型:只读 ~/.pi/agent/models.json,不读 auth.json,因此通过 pi /login 连接的 provider(例如 DeepSeek、Z.AI Coding CN)不会出现在 Pi 分组里。
  2. MCP:扫描源没有 ~/.pi/agent/mcp.json,因此 PI 里已经配置好的 MCP server 全部不显示。

实现

  • 新增 parsePiAuthModelConfig():把 auth.json 的 API key 与 Pi 缓存的 models-store.json 目录配对。models.json 仍然优先;OAuth/订阅凭据跳过;!command 不执行也不保存;models.json 的 env:/$VAR 在 Desktop 进程里解析不到时,用 auth.json 的 key 补齐。
  • MCP 扫描器新增 pi 源,读取用户级 ~/.pi/agent/mcp.json 的 mcpServers。项目级 .pi/mcp.json 需要项目信任,本次不扫描。
  • 同步 ADR 0179、D342、IPC 文档、E2E-209 和 9 个语言包。

验证

  • pnpm --filter @pi-desktop/shared test → 98 files / 1280 tests passed
  • node --test apps/desktop/test/model-config-import.test.mjs apps/desktop/electron/test/agent-mcp-scan.test.mjs → 17 passed
  • pnpm --filter @pi-desktop/i18n test → 30 passed
  • pnpm --filter @pi-desktop/desktop typecheck → passed
  • pnpm lint:biome → 124 files checked, no fixes
  • pnpm test:e2e:settings-inline-import → passed(candidate e3b150e,base 42fb8b7)
  • pnpm check:pr-base → passed

兼容性与安全

  • 不改变 IPC/RPC、数据库 schema 或已有导入语义。
  • OAuth/订阅登录仍留在 Pi;!command 不执行、不作为字面量密钥保存。
  • 仍然只在显式 Scan 时读取,不自动导入(D007 不变)。
  • 已知限制:只存在于 auth.json 的 provider 需要 Pi 已经写出 models-store.json(Pi 刷新过模型目录)。没有缓存目录时,只会为 models.json 里已有的 provider 补齐 key,不会凭空生成 endpoint/model 列表。

isword added 3 commits October 18, 2026 23:01
The explicit model-config scan only read ~/.pi/agent/models.json, so
providers connected through `pi /login` (for example DeepSeek or a Z.AI
coding plan) never appeared in the Pi group. Their keys live in auth.json
and their endpoint/model metadata lives in Pi's cached models-store.json.

Read both files and pair each API-key provider with its largest coherent
endpoint group. OAuth/subscription grants stay in Pi, `!command` keys are
never executed or stored, and models.json remains authoritative when both
files describe a provider. A key in auth.json also fills a models.json
provider whose `env:`/`$VAR` reference cannot resolve in Desktop.

Tests cover the cached-catalog pairing, OAuth/command exclusion, endpoint
selection, and the scanner merge.
The explicit MCP scan covered Claude, Codex, Cursor, opencode, and the
ChatGPT placeholder, but not Pi. A user with servers configured in
~/.pi/agent/mcp.json saw an empty import list even though Pi connected to
those servers.

Scan the user-level Pi mcp.json with the existing JSON mcpServers reader,
report it as its own source, and label the group in every shipped locale.
Project-level .pi/mcp.json stays out of scope because Pi only loads it
after project trust.
Keep the ADR ending consistent with the repository's whitespace checks.
@IswordSun

Copy link
Copy Markdown
Author

补充:04fe425 只删除了 ADR 末尾空行,可执行内容与 e3b150e 相同,因此上面的 E2E 结果仍适用。pnpm check:pr-base 已在 04fe425 上通过(base 42fb8b7)。

@muzimu217

Copy link
Copy Markdown
Contributor

Reviewed in an isolated worktree (head 04fe4254c): packages/shared 24/24 via vitest, and the two importer suites (model-config-import, agent-mcp-scan) pass — 41 tests total green.

What I checked beyond "it runs":

MCP side is a clean reuse: scanPi() is one scanJsonMcpServers call against ~/.pi/agent/mcp.json with the same mcpServers shape as the other sources, wired into the existing Promise.all/report list. No new parsing surface.

Model side has the right precedence and the right secret boundary:

  • models.json stays authoritative; auth-derived drafts only fill providers it does not describe (byProvider.has(draft.externalId) → continue). Good — that avoids a cached-catalog entry shadowing a hand-edited config.
  • auth.json handling is conservative in exactly the ways that matter: OAuth/subscription grants are skipped (PI_API_KEY_AUTH_TYPES), !command values are never executed and never stored literally (isShellCommandSecret), and resolveSecret reuses the existing env resolution.
  • The models-store.json fallback is justified in the comment (auth-only providers have no base URL otherwise), and selectPiModelEntry picking the largest coherent (baseUrl, apiStyle) group is a sensible way to fit one provider row onto one protocol — ties keep the first group so repeat scans stay deterministic.
  • Preview UI only renders a hasSecret badge (ModelConfigImportPanel.tsx:189-190), so the imported key never round-trips to the screen.

Two non-blocking notes:

  1. apiKeys is populated for every auth.json provider, including ones with no catalog entry, and the final drafts.map spreads secretValue onto matching drafts. That is correct today because apiKeys only ever feeds draft construction — but it means a future caller of parsePiAuthModelConfig gets a plaintext key map for providers it never imported. Worth a doc line or narrowing the return type if that helper ever becomes more widely used.
  2. The "largest group wins" rule silently drops the other base URLs; the comment says those models "stay available in Pi", which is honest — just flagging that a user with two endpoints on one provider id will see only one imported, by design.

Nothing blocking from my side.

@IswordSun

Copy link
Copy Markdown
Author

Thanks for the careful review. Both notes are accurate and intentional:

  1. apiKeys covers every auth.json API-key entry because scanPi uses it to fill keys on models.json drafts whose env:/$VAR reference cannot resolve in the Desktop process. It stays in the Electron main scan cache; publicModelConfigCandidate never exposes it, and the return type is scoped to this importer contract. If this helper ever gains other callers, narrowing the return to a callback or per-provider accessor would be the right follow-up.
  2. Agreed on the largest-group rule. A Desktop provider row has one base URL/protocol, so the other groups deliberately stay available in Pi and are not imported; the comment documents that trade-off.

No code change made for the non-blocking notes. Thanks again for checking the precedence and secret boundary.

vastsa commented Oct 19, 2026

Copy link
Copy Markdown
Owner

本 PR 新增了可见的 PI agent 导入来源/标签,并改动导入面板和多语言文案,属于界面改动,不符合本轮“只合入无界面改动 fix”的条件,因此这次不合入。

另外,当前 head 基于 42fb8b7,而 main 已到 589e580,后续落地前也需要先同步最新 main 并重跑基线检查。感谢你补齐 Pi 模型和 MCP 的导入来源,也感谢详细的安全边界和验证说明。

@IswordSun

Copy link
Copy Markdown
Author

按 @vastsa 的范围意见,已把模型导入部分拆到 #1761:基于最新 main 589e580,且不修改任何 apps/desktop/src 或 packages/i18n 文件,符合本轮“无界面改动 fix”的范围。\n\n这个 PR 里的 MCP 扫描需要新增可见的导入来源/标签、导入面板和语言包,属于界面改动,先关闭;分支保留在 IswordSun:fix/pi-agent-import,等允许界面改动的轮次再提。感谢 review。

@IswordSun IswordSun closed this Oct 19, 2026
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