Skip to content

fix(subagents): apply the delegation opt-in to collaboration spawn (#386) - #407

Merged
vastsa merged 2 commits into
vastsa:mainfrom
L4XB:fix/386-unticked-subagent-model
Sep 15, 2026
Merged

vastsa merged 2 commits into
vastsa:mainfrom
L4XB:fix/386-unticked-subagent-model

Conversation

@L4XB

@L4XB L4XB commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Closes #386

Summary

The Task path this issue reports is already closed. #305 and #318 landed the
subagentModelKeys separation on 2026-09-13 and ship in v0.14.8: a pin the
user never ticked stays out of the delegation catalog and is refused as another
delegate's Task.model. I re-verified that on main @ 4e2e0055 before
touching anything — evidence below.

What is still open is the report's second expectation — 「即使通过参数或其他入口
手动传入该模型,运行时也应拒绝请求」
— for one other entry point.
session/collaboration/spawn picks its worker model like this
(apps/desktop/electron/main/services/session-collaboration.ts):

const chosen = requested
  ? models.find((item) => item.key === requested)
  : models.find((item) => item.availableForSubagents)
    ?? models.find((item) => item.isDefault);

The availableForSubagents in the inheritance branch is the tell: the flag was
meant to govern here. The requested branch never consults it, so a modelKey
naming any configured, ready model is honored — including one the user
deliberately left unticked in model settings. listReadyPluginModels reports
the flag but does not filter, and a plugin is not where this can be decided, so
in practice nothing enforced it.

That is the same bypass #386 describes, through the second door: a worker the
agent starts is AI-driven delegation, and the Session Orchestrator asks for it
on the agent's behalf.

The change

The named branch now needs the model's own opt-in and answers
PERMISSION_DENIED before a worker exists.

Inheritance is deliberately untouched, so nobody who was not bypassing the
toggle sees a difference:

  • omitting modelKey still takes the first opted-in model, and otherwise the
    default;
  • naming the default model's own key is that inheritance written out — the
    same allowance the Task path already makes for repeating a definition's own
    pin, and for the exact parent provider/model.

Eight lines in one file. Everything else in the diff is the specs, both zh-CN
mirrors, the E2E scenario and the ADR recording the widened scope, per
AGENTS.md §11.

Not changed

listReadyPluginModels still returns every ready model with its
availableForSubagents flag. It backs models.list and agent.complete too,
where the flag is irrelevant, and filtering there would only move the decision
to a caller that cannot be trusted with it. Main is the authoritative side, so
main is where the gate went.

Evidence

Four tests in apps/desktop/test/session-collaboration-service.test.mjs. They
load the real plugin-agent-complete helpers rather than a stub, so the flag is
proved to survive the whole way from providers.list through the plugin model
list into the spawn decision.

The refusal test fails on the merge base and passes here; the other three are
controls that pass on both, which is what shows the change costs no existing
behaviour.

Base source (origin/main @ 4e2e0055, this branch's tests):

✖ spawn refuses a model the user did not enable for AI delegation (#386)
✔ spawn accepts a model enabled for AI delegation (#386)
✔ spawn treats the default model's own key as inheritance, not a selection (#386)
✔ spawn without a model key still prefers an enabled model over the default (#386)
ℹ pass 11   ℹ fail 1

This branch:

✔ spawn refuses a model the user did not enable for AI delegation (#386)
✔ spawn accepts a model enabled for AI delegation (#386)
✔ spawn treats the default model's own key as inheritance, not a selection (#386)
✔ spawn without a model key still prefers an enabled model over the default (#386)
ℹ pass 12   ℹ fail 0

That the Task path is already fixed, re-verified on main @ 4e2e0055:

$ vitest run src/runtime.test.ts -t "286"      # keeps a private definition pin out of
 Tests  2 passed | 156 skipped (158)           # the override catalog and other delegates

Validation

Check Result
pnpm build:js passed
pnpm typecheck passed
pnpm lint passed (biome 55 files, style tokens OK)
pnpm -r --if-present --no-bail test shared 658, agent-runtime 452, plugin-sdk 288, plugin-devkit 46, agent-host 39, i18n 24, desktop 1913 passed / 0 failed; docs 1 pass 1 fail — see below
pnpm test:e2e:subagent-models passed (17 checks: E2E-166 + ordered-model-fallback)
pnpm test:e2e:collaboration NOT RUN
pnpm test:e2e NOT RUN
cargo fmt / cargo test -p host-core / cargo clippy NOT RUN

pnpm test:e2e:collaboration and pnpm test:e2e both stop at host binary missing … pi-desktop-host-core, and there is no Rust toolchain on the machine I
validated on, so the host cannot be built here. Both are the suites that would
exercise this change end to end, and neither ran. The unit coverage above drives
the real createSessionCollaborationService against the real model helpers,
which is the whole decision under change, but it does not replace those runs.
Remaining risk sits there and with the post-integration E2E AGENTS.md §15
requires.

docs test fails on check-locales: 06-delivery/04-e2e-test-plan.md has 97
table rows in English and 95 in Chinese, first diverging at row 90. This
reproduces unchanged on main @ 4e2e0055
— same file, same counts, same
divergence index — and my edits to that pair add no table rows. Not fixed here:
it is an unrelated pre-existing drift and folding it in would bury this change.

中文摘要

Issue 里报告的 Task 路径已经修好了:#305#318 于 2026-09-13 合并,随 v0.14.8
发布,未勾选的固定模型既不会进入委托目录,也不会被接受为其他子代理的
Task.model。我在动手之前先在 main @ 4e2e0055 上复验了这一点。

仍然存在的是本 issue 第二条预期所说的「其他入口」:
session/collaboration/spawn 显式传入 modelKey 时完全不检查
availableForSubagents,因此未勾选的模型照样能被派发。同一个表达式的继承分支
是查了这个标志的,说明作者本来就打算让它在这里生效。

本 PR 让显式指定的 modelKey 必须带有该模型自己的许可,否则在创建 worker 之前
返回 PERMISSION_DENIED。继承行为不变:省略 modelKey 仍然优先取已勾选的模型、
其次取默认模型;显式写出默认模型自己的键同样按继承处理,与 Task 路径上「重复定义
自己的固定键等于省略」的规则一致。

…astsa#386)

`session/collaboration/spawn` picked its worker model like this:

    const chosen = requested
      ? models.find((item) => item.key === requested)
      : models.find((item) => item.availableForSubagents)
        ?? models.find((item) => item.isDefault);

The `availableForSubagents` in the inheritance branch is the tell: the flag
was meant to govern here. The `requested` branch never consults it, so a
`modelKey` naming any configured model was honored — including one the user
deliberately left unticked in model settings. `listReadyPluginModels` reports
the flag but does not filter, and a plugin is not where this is decided, so
nothing enforced it at all.

The named branch now needs the model's own opt-in and answers
`PERMISSION_DENIED` before a worker exists. Inheritance is untouched: omitting
`modelKey` still takes the first opted-in model and otherwise the default, and
naming the default model's own key is that inheritance written out, the way
repeating a definition's own pin is on the Task path.

The Task path this issue reports was closed by vastsa#305 and vastsa#318 and ships in
v0.14.8; this is the one remaining entry point the report's second expectation
covers. Specs, both zh-CN mirrors, the E2E scenario and the ADR record the
widened scope.
Copilot AI lite review requested due to automatic review settings September 15, 2026 11:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@L4XB

L4XB commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

On the red CI here, before anyone spends time on it: Docs locale pair check fails on main too, at the exact merge base this branch was cut from.

  • main @ 4e2e0055Docs check run 34944009256 fails, naming 06-delivery/04-e2e-test-plan.md.
  • this branch @ e6779c9f — same assertion, same single file: Chinese pages whose source notice or structure is invalid, actual: [ '06-delivery/04-e2e-test-plan.md' ].

I am flagging it rather than staying quiet because this PR does touch that file and its zh-CN mirror, so the honest thing is to say so up front. The edits add table rows to neither side, and the failing list is one entry long on both trees — the same one. I deliberately did not fold an unrelated docs repair into a behaviour fix; say the word if you would rather I did.

The other red mark, Vercel, is the external-contributor authorization gate, not a build result.

For what it is worth on the JS side of that job: build:js, typecheck, lint and the desktop unit suite are green (desktop 1913 passed / 0 failed, plus shared 658, agent-runtime 452, plugin-sdk 288). What I could not run is pnpm test:e2e:collaboration and pnpm test:e2e — both stop at host binary missing … pi-desktop-host-core, since building the host needs a Rust toolchain I do not have here. Those are exactly the suites that would exercise this change end to end, so that is the remaining risk and I would rather name it than let it pass as covered.

@L4XB

L4XB commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Brought up to date with main, which clears the docs locale failure.

check-locales compares the table shape of every English spec against its Chinese mirror. The pair 06-delivery/04-e2e-test-plan.md was 97 English rows against 95 Chinese ones at the base this branch was cut from, and main has since brought both to 98. My own diff on that pair adds no table rows in either language, so the mismatch was inherited rather than introduced, which is why nothing in the change itself looked wrong.

On the merged head: node --test docs/scripts/check-locales.test.mjs passes both cells, and the shapes are 98 against 98.

@vastsa
vastsa merged commit e47b19f into vastsa:main Sep 15, 2026
3 of 4 checks passed
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.

[Bug][subagent] 未勾选的模型仍可在对话中选择并派发

3 participants