Skip to content

feat(settings): show and edit subagent delegation models on the Models page - #1758

Open
736368217 wants to merge 7 commits into
vastsa:mainfrom
736368217:feat/subagent-model-row
Open

736368217 wants to merge 7 commits into
vastsa:mainfrom
736368217:feat/subagent-model-row

Conversation

@736368217

@736368217 736368217 commented Oct 19, 2026 •

Copy link
Copy Markdown

What this changes

The Models page already states the current image-generation model at the bottom, but the delegation catalog — the models the parent agent may pick when it delegates a task with model — could only be reviewed one service at a time, inside that service's model dialog.

This adds a Subagent models summary row under the image-model row:

  • the delegated models as chips, capped at 60% of the row; whatever does not fit collapses into +N;
  • Edit opens a dialog with the full list — the same search field, context · max output limits, Advanced and × a service's own model list uses;
  • Advanced opens that service's model editor, where Available for AI delegation is ticked, so adding a model stays a model-editor job.

Total surface added to the page: one line. Everything else lives one click away.

Chip widths are measured from an off-screen twin of the same chips, because only the browser knows how wide a model id is at the current font and zoom; fitSubagentChips() then decides how many fit and reserves room for +N.

Nothing here adds a second stored selection: the summary, the dialog and the service model editor all read and write ModelBinding.availableForSubagents, the exact field the runtime reads. Ticking the box in a model's editor makes the chip appear; removing it here makes the tick disappear there. The three surfaces cannot drift.

Refs #1736.

Screenshots

Taken from the real renderer bundle (apps/desktop/out/renderer) with fixture services; images live on the pr-1758-screenshots branch so this diff stays code-only.

1440 wide — chips + +N 1440
1100 wide — fewer chips, larger +N 1100
zh-CN zh
Dark dark
The editor dialog dialog

Design note: reuse the existing opt-in instead of adding a second list

The issue thread suggested a subagentModelBindings list in packages/shared. This implementation deliberately does not add one, because the runtime already owns that decision on each model binding:

  • packages/host-runtime/src/launch-resolver.ts builds the delegation catalog from ModelBinding.availableForSubagents only;
  • apps/desktop/electron/main/runtime/sidecar.ts and services/session-collaboration.ts authorize every Task.model / spawn modelKey against that same flag (D278, ADR subagent-model-opt-in).

A second stored list would be a second gate that can disagree with the first. Reading and writing the binding field also answers the obvious follow-up — "I changed it here, did the model editor change too?" — with one shared state instead of a sync problem.

Write path: the save re-reads the service with providers.list, refuses when the row is plugin-owned or the model is gone, and rewrites only the touched binding with availableForSubagents set (or dropped, so no false accumulates on records that never wanted delegation). It never posts this page's snapshot back, so a concurrent model edit from another window is preserved. No Host RPC, database schema, migration, or Plugin SDK change.

Files

New:

  • apps/desktop/src/components/settings/subagent-delegation.ts — candidate construction, the opt-in write draft, and fitSubagentChips() (pure; no runtime imports so it is testable on its own)
  • apps/desktop/src/components/settings/SubagentModelRow.tsx — the chip summary line
  • apps/desktop/src/components/settings/SubagentModelsDialog.tsx — the editor dialog
  • apps/desktop/test/subagent-delegation.test.mjs — 12 unit tests

Changed:

  • apps/desktop/src/components/settings/ModelConfigPage.tsx — candidate memo, save handler, mount point; Advanced reuses the page's existing provider dialog
  • apps/desktop/src/components/settings/subagent-models.ts — the subagent editor's "is this service runnable" rule now delegates to the shared predicate instead of duplicating it
  • apps/desktop/src/styles/model-config.css — the chip line and dialog furniture; the dialog's rows are the service list's own provider-chosen-* parts
  • packages/i18n/src/locales/* — 12 new settings.subagent* keys in all 9 shipped locales
  • Specs and their zh-CN mirrors: 03-runtime/02-agent-runtime.md §5f, 03-runtime/11-provider-model-system.md §7, 04-ux/06-settings-ia.md, 06-delivery/04-e2e-test-plan.md (new E2E-268)

Review follow-up

CONFLICTING state (docs only). Merged current upstream/main into the branch; the E2E files conflicted only where both sides append a coverage-table row. Both rows are kept, mirrored in zh-CN so the pair stays shape-compatible. pnpm docs:check: 86 locale pairs, 585 pages.

Auth-kind note. The "why is this row idle" label reads credential presence, not an auth-kind name: hasSecret is the host's own "this row holds a usable credential" flag, so a flow added later still counts. Only authKind === "none" (keyless local endpoint) is named explicitly, because that is the one case where no credential is expected. A test pins both ends.

Disabled-service editing. The opt-in is durable, so a row stays listed and removable even while its service is off.

Validation

Run on the head that carries this design:

Gate Result
pnpm build:js pass
pnpm --filter @pi-desktop/desktop typecheck pass
pnpm lint (biome + style-token guard) pass
pnpm --filter @pi-desktop/i18n test 31 pass (key parity across 9 locales, renderer-key coverage)
apps/desktop/test/subagent-delegation.test.mjs 12 pass
pnpm docs:check pass (86 locale pairs, 585 pages)
pnpm check:pr-base pass

Measured in the rendered page: at 1440 wide the row is 1061px and the chip line 547px (≈52%, inside the 60% cap), showing six chips plus +2.

The full apps/desktop suite was last run before this redesign (4144 tests / 103–106 failures vs 4133 / 105 on upstream/main, identical failing sets, all environment-only on this Windows host); it was not re-run afterwards. Interactive E2E-268 remains documented but not driven in a real desktop session.

Policy note (please read)

AGENTS.md R6.1 (the temporary PR type scope effective 2026-09-26, still present on main) says outside contributions are limited to perf and fix, and that a feat PR from a contributor without write access is not a landing candidate. This is a feat, and I am the author of the linked issue.

I asked in the issue first and did not get an answer, so I opened this with the note visible rather than after the fact. If the restriction stays strict, closing this is a completely reasonable outcome. If it is useful, I am glad to reshape it however you prefer. The work is complete and reproducible either way.


中文摘要

模型页底部在生图模型行下方新增一行 子代理模型:

  • 已授权的模型以标签(chip)展示,最多占这一行的 60%,放不下的折叠为 +N;
  • 右侧「编辑」打开弹窗,里面是完整列表(搜索 / 上下文·输出 / 高级 / ×),与「模型设置」里已添加模型那列一致;
  • 弹窗里的「高级」打开该服务的模型编辑器 —— 「可供 AI 自动调度」就在那里勾选,添加模型仍然在模型编辑器里完成;
  • 摘要、弹窗、模型编辑器读写同一个 ModelBinding.availableForSubagents,三处永远一致,没有第二份状态。无 RPC / schema / 迁移 / Plugin SDK 改动。

实测:1440 宽时行宽 1061px、标签区 547px(≈52%),显示 6 个 + +2;1100 宽时自动少显示。校验:构建、typecheck、lint(含 style-token)、docs(86 组中英配对 / 585 页)、i18n 31 项、新增 12 项单测通过。

…s page

The delegation catalog the parent agent may pick from was only editable one
service at a time, inside that service's own model dialog, so there was no way
to review it. The Models page now states which configured models carry the
`availableForSubagents` opt-in and edits that same flag across every service in
one list, next to the image-model row.

The list reuses the per-binding flag the runtime already reads instead of adding
a second stored selection, so the page and the delegation catalog cannot
disagree. A save re-reads the service and rewrites only the touched binding,
which keeps a concurrent model edit intact, and a plugin-owned row stays
read-only because `providers.update` refuses it.

Specs, their zh-CN mirrors and E2E-267 describe the new surface.

Refs vastsa#1736
…-row

# Conflicts:
#	docs/spec/06-delivery/04-e2e-test-plan.md
#	docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md
@muzimu217

Copy link
Copy Markdown
Contributor

Reviewed in an isolated worktree: subagent-delegation.test.mjs 10/10 pass, and I verified the central claim in the code rather than taking it from the description.

The single-source-of-truth claim holds. ModelBinding.availableForSubagents really is both the catalog and the authorization:

  • catalog build — packages/host-runtime/src/launch-resolver.ts:325 (if (!binding.availableForSubagents) continue;)
  • authorization — apps/desktop/electron/main/runtime/sidecar.ts:495-499 (if (!binding?.availableForSubagents) { … })

So editing the flags themselves — rather than storing a second selection the runtime would have to merge — is the right call, and it avoids the classic drift where the page shows one set and the runtime authorizes another. That is the part I would have pushed back on if it had gone the other way.

Edge handling matches what the runtime does. providerServesSubagentDelegation (enabled + secret/oauth/keyless) mirrors the runnable-service rule, blocked distinguishes disabled from missing-credential, and — the detail I like — disabled/uncredentialed rows stay listed and editable so an existing opt-in can be withdrawn instead of becoming invisible. Plugin-owned rows are correctly non-editable.

Two non-blocking notes:

  1. blockedReason returns credential-missing for any enabled provider that is not secret/oauth/keyless. If a provider row can be enabled with an auth kind outside that set (e.g. a future device-code flow), the UI would label it "missing credential" while the runtime might accept it. Worth confirming the auth kinds are exhaustive, or phrasing the copy as "not usable yet" rather than asserting why.
  2. The page edits flags across every provider row, including disabled ones. That is intentional per the comment, but it means a checkbox on a disabled service writes to a provider the user cannot currently test — worth a line in the PR body so reviewers do not read it as a bug.

Nothing blocking from my side.

@muzimu217

Copy link
Copy Markdown
Contributor

Heads-up: this PR is CONFLICTING, and it is docs-only — two files, both the same cause.

CONFLICT docs/spec/06-delivery/04-e2e-test-plan.md          (2 hunks)
CONFLICT docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md    (1 hunk)

No code conflict. Both sides append to the same anchors:

  1. The coverage table (~line 10103): your | C / E / Quality — Subagent delegation catalog … | [E2E-268](…) | row and the client-certificate row from fix(providers): handle optional client certificate requests #1724 were inserted at the same place.
  2. The section body (~line 18256): your ### E2E-268: Models page states and edits the subagent delegation catalog section and fix(providers): handle optional client certificate requests #1724's ### E2E-PROVIDER-optional-client-certificate section share an insertion point.

Resolution that keeps both, and is what I verified locally:

  • Table: keep both rows, yours first (it is the row you are adding), then the client-certificate row.
  • Body: keep both sections in full; order does not matter as long as neither is truncated. Place E2E-268 after the session-follow section (where you inserted it) and leave the client-certificate section where fix(providers): handle optional client certificate requests #1724 put it.
  • Same two edits in the zh-CN file (it has one hunk — the table row; its body section sits elsewhere and does not conflict).

After that the branch merges cleanly: I ran the merge locally and the only conflicts were those two files. Your code and tests are untouched by the conflict.

One suggestion while you are in there: the E2E table is now long enough that adjacent-row insertions will keep colliding. Appending new rows at the end of the table (rather than next to a related entry) would avoid this class of conflict for future PRs.

…-row

# Conflicts:
#	docs/spec/06-delivery/04-e2e-test-plan.md
#	docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md
… auth kind

Review note on vastsa#1758: the eligibility check reads credential presence, not the
auth-kind name, so a flow added later must not be labelled "no credential" just
because this page has never heard of it. The comment states the rule and the
test proves both ends of it: a stored grant counts, and a row with nothing
stored really is unusable, because the runtime cannot obtain a key for it
either.
@736368217

Copy link
Copy Markdown
Author

Thanks — both notes are addressed, and the branch merges cleanly now.

Conflicts (docs only). 2bc7cb07 brings upstream/main @ 8a8d5641 in and resolves the two files the way you described: both coverage-table rows kept (mine first, then the client-certificate one), both body sections kept in full, E2E-268 after the session-follow section and #1724's section where it was, mirrored in zh-CN so the pair stays shape-compatible. pnpm docs:check → 86 locale pairs / 580 pages, and GitHub now reports MERGEABLE. I took the "append at the end" advice for the section body; the table row stayed adjacent per the resolution you verified locally, so the row sequence still matches between the two files.

Auth kind. You were right that asserting a cause is riskier than describing the state, so I checked which of the two the code actually does. It reads credential presence, not the auth-kind name: hasSecret is the host's own "this row holds a usable credential" flag (API key or vendor account), so an unfamiliar flow that stores something still counts without this page learning about it. Only authKind === "none" is named explicitly, because keyless is the one case where nothing is expected. The label therefore only fires when the host reports no credential at all — which is also when the runtime could not obtain a key for that row. 4a93d7d1 writes the rule down and pins both ends in the test: authKind: "device_code" with a stored grant is not blocked, and the same kind with nothing stored is blocked and stays editable. If you'd still prefer neutral copy ("Not usable yet"), that's a one-line change across nine locales and I'll make it.

Disabled services. Spelled out in the PR body now: the opt-in is a durable choice, so the flag has to stay reachable — otherwise disabling a service would strand a selection the user can no longer withdraw.

One measurement note for transparency: apps/desktop full suite is 4144 / 103–106 on this branch vs 4133 / 105 on 8a8d5641. The delta is one pre-existing load-sensitive test (plugin-mcp → "a remote MCP tool can run longer than the connection timeout"). It passes 31/31 in isolation on both sides, and it passes on this branch when the new test file is excluded from the run (4133 / 102), so the flip is scheduling rather than behavior. I left it untouched to keep this PR's scope tight; happy to open a follow-up if you want it pinned.

The first cut expanded every configured model into a checkbox list and gave the
row its own add menu. That buried the answer: a service can publish dozens of
models, while the set worth reviewing is only the opted-in one.

The row now reuses the service model list's own parts — the search field, the
row with its limits, Advanced, and the remove control — and shows only the
models that carry the opt-in. Advanced opens that service's model editor, where
"Available for AI delegation" is ticked, so adding stays a model-editor job and
both surfaces read and write the same `availableForSubagents` binding field.
…-row

# Conflicts:
#	docs/spec/06-delivery/04-e2e-test-plan.md
#	docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md
@736368217

Copy link
Copy Markdown
Author

Screenshots of the row, taken from the shipped renderer bundle (apps/desktop/out/renderer) rendered against fixture services — same CSS and components the app loads. They live on a separate branch (pr-1758-screenshots → docs/pr/1758/) so this PR's diff stays code-only.

Models page, light
Models page with the Subagent models list

The list itself — only the opted-in models, in the service model list's own style: search field, context · max output, Advanced, ×
Subagent models list

Chinese
zh-CN

Dark
dark theme

Empty state
empty state

Why the two surfaces cannot drift: Advanced opens that service's model editor, where Available for AI delegation is ticked. Both surfaces read and write ModelBinding.availableForSubagents — ticking it there makes the row appear here, and the row's × clears the same flag, which the editor then shows unticked.

中文:列表只显示已勾选「可供 AI 自动调度」的模型,样式沿用「模型设置」里已添加模型那一列(搜索框 / 上下文·输出 / 高级 / ×)。添加在模型编辑器里勾选,点「高级」直接打开该编辑器;两边是同一个 availableForSubagents 字段,改一处另一处同步。

…or into a dialog

The Models page should not grow a second model list. The row now shows the
delegated models as chips, capped at 60% of the row; whatever does not fit
collapses into +N. The full list — search, limits, Advanced, remove — moved into
a dialog that Edit opens, so the page keeps one compact line and the editor keeps
every action.

Chip widths come from an off-screen twin of the same chips, because only the
browser knows how wide a model id is at the current font and zoom; the pure
fitSubagentChips() then decides how many fit and reserves room for +N.

The editor still reads and writes ModelBinding.availableForSubagents, and
Advanced still opens the service model editor where that opt-in is ticked, so
the summary, the dialog and the service dialog can never disagree.
736368217 added a commit to 736368217/PI-Desktop that referenced this pull request Oct 23, 2026
@736368217

Copy link
Copy Markdown
Author

@vastsa 您好,按您的意见重新设计了,麻烦再看一下。

之前的问题:这一行把「完整模型列表(搜索框 + 上下文/输出 + 高级 + ×)」直接铺在设置页里,等于在设置页又开了一份模型管理面板,确实太重。

现在的做法

  • 设置页只留一行摘要:已授权的模型做成标签(chip);标签按可用宽度排布,最多占这一行的 60%,放不下的折叠成 +N;
  • 右侧一个「编辑」,点开才是完整列表(搜索 / 上下文·输出 / 高级 / ×),关掉弹窗回到设置页;
  • 弹窗里的「高级」会打开该服务的模型编辑器 —— 「可供 AI 自动调度」的勾选在那里,所以添加模型仍然在模型编辑器里完成,这里只负责查看和移除。

一致性:摘要、弹窗、模型编辑器读写的是同一个 ModelBinding.availableForSubagents。在模型编辑器里勾上,这里的 chip 立刻出现;在这里点 ×,模型编辑器里的勾选立刻消失。三处不会各说各话。

实测:1440 宽时,行宽 1061px、标签区 547px(约 52%,没有超过 60%),显示 6 个 + +2;窗口收窄到 1100 时自动少显示、+N 变大。

截图(真实渲染产物 + fixture 数据,非手绘稿):

1440 宽 1100 宽
1440 1100
中文 深色
zh dark

点「编辑」后的弹窗:

dialog

如果这个轻重还是不对(比如希望连标签都不要、只留一行「6 个模型」,或者 60% 这个比例要调),您说一声我再改。

This branch has not been deployed

No deployments
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