Skip to content

feat(subagents): 支持按项目设置子智能体作用范围 - #1438

Open
zszz3 wants to merge 12 commits into
vastsa:mainfrom
zszz3:codex/feat-subagent-project-scope
Open

zszz3 wants to merge 12 commits into
vastsa:mainfrom
zszz3:codex/feat-subagent-project-scope

Conversation

@zszz3

@zszz3 zszz3 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Closes #1431。

目前用户创建的子智能体会出现在所有会话的 Task 工具中,项目专用的子智能体也会被其他项目看到。本 PR 增加“全局 / 指定项目”设置,让模型只看到并调用当前项目适用的子智能体。

改动

  • 在子智能体编辑页选择作用范围。“指定项目”支持搜索和多选;启用、禁用仍由列表开关控制。
  • 根据会话的项目目录筛选 Task 中的可用子智能体。例如 A 专用的子智能体只在 A 及其子目录的会话中可用,在 B 或无项目会话中不可见;即使模型指定它的名字,也会被拒绝执行。
  • 复用现有 scope 字段和子智能体加载流程,由 Host 在 agents.active(projectPath) 中筛选。范围设置单独保存在应用本地,不改动子智能体 Markdown;重命名保留设置,删除时清理。
  • 未设置范围的现有子智能体继续全局可用。本次仅支持用户创建的子智能体,不涉及插件的激活条件。

截图

完整界面 —— 设置 → 子智能体。「自定义」分组(~/.agents/subagents)下,每一行带有自己的作用范围标签(全局 或 N 个项目),启用开关保持独立:

设置 → 子智能体页面

改动位置特写 —— 编辑子智能体时的「作用范围」控件。选「指定项目」后,项目多选列表直接展开在编辑器内,带搜索框;当前打开的项目标有「当前打开」,但不会自动勾选。

(截图用的是本仓库:desktop-ui-reviewer 限定到 apps/desktop,所以在仓库根目录项目里不可见。)

作用范围控件特写

更新(2026-10-17)

  • 已同步最新 main(e219a3e4c)。
  • 合并前自查中发现的两个问题,均带回归测试:
    • 作用范围匹配原先按字节偏移切片会话路径,项目名含中文等多字节字符时会 panic,导致该项目的每条消息都要等 RPC 超时、且拿不到任何用户子智能体。改为整体字符串比较,并补充中文路径的单元测试与 host 端到端测试。
    • 云同步原先会携带并应用 scope,导致 A 设备专用的子智能体到 B 设备不可用,旧版本的全局设置还会冲掉项目限定。现在范围只留在本机,并补充双设备同步测试。
  • 编辑器在「指定项目」但未选任何项目时,会提示「至少选择一个项目,否则不会生效。」,与弹出式选择框一致。
  • 在应用外删掉子智能体 Markdown 文件时,范围条目会在下一次扫描时清掉。否则以后重建同名子智能体会静默继承旧的「只在某些项目生效」限制(补了回归测试)。
  • 重命名和删除改用 ? 传播 fs::remove_file 的失败(原先是 .ok() 静默吞掉)。销毁旧文件失败时,下一轮扫描会让旧 id 复活,所以现在显式报错而不是假装成功。
  • 子智能体列表的用户分组标题由「全局级」改为「自定义」:定义文件仍在 ~/.agents/subagents,但生效范围改由每行标签说明,避免和项目限定的行矛盾。
  • 用真实模型 step-5-preview 复验:在 alpha-web 成功委派 frontend-reviewer;在 beta-api 调用它返回 Unknown subagent 'frontend-reviewer',未启动子任务;中文命名的项目正常响应、无卡顿。

验证

  • Desktop 测试 4081 通过、3 跳过、0 失败,含新增的 Electron 设置交互测试;Host 子智能体测试通过,覆盖项目选择与保存、范围过滤、启停、重命名和重启后恢复。
  • Host RPC 与运行时加载器的端到端测试通过,覆盖 A、B、无项目会话,以及中文命名项目(多字节路径)的可用范围。
  • 双设备云同步测试通过:范围设置不随同步传递,各设备保留自己的设置。
  • Desktop 类型检查、样式检查、Rust 格式与 clippy、文档检查、PR 基线检查通过。host-core 测试 965 通过、1 失败:data_relocation 的路径用例在 macOS 本地失败,CI 的 Rust 检查为绿,属环境差异。

范围修改在下一次加载工具目录时生效,不中断正在运行的子智能体。旧版本不识别新增的范围设置,降级后会恢复原来的全局可用行为。范围设置只保存在本机,不随云同步传递。

Keep dedicated user subagents out of unrelated session catalogs while
preserving global defaults and portable Markdown definitions. Persist
project visibility locally and filter before runtime catalog assembly.

Expose project selection in the editor independently of enablement,
using the project archive sources so recent folders remain selectable.
Cover persistence, catalog isolation, and the settings save flow.

Refs vastsa#1431
@zszz3
zszz3 marked this pull request as ready for review October 6, 2026 16:05
@zszz3 zszz3 changed the title feat(subagents): 按项目限定用户子智能体的可见与调用范围 feat(subagents): 支持按项目设置子智能体作用范围 Oct 6, 2026

@muzimu217 muzimu217 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Checked out the branch and reproduced the CI failure locally — full review below.

The JS failure is a stale source assertion, one line to fix

apps/desktop/test/agent-capability-settings.test.mjs:42 ("skills and MCP filter one list by level...") asserts the SubagentEditorSheet source still contains settings\.globalOnly. This PR intentionally replaces that "global-only" label with the real scope control, so the assertion no longer matches. Reproduced locally on head 758764a09:

✖ skills and MCP filter one list by level instead of stacking two sections
AssertionError: The input did not match the regular expression /settings\.globalOnly/

Updating that assertion to the new shape is the only thing between this PR and a green JS job — the new subagent-project-scope.test.mjs suite itself passes its non-Electron tests, and the Electron E2E check is green on CI.

Host-side implementation verified against #1431's facts

This is exactly the "wiring" the issue's verification pointed at, done right:

  • user_subagents.rs:302 active_for(project_path) now consumes the project path (was an unused _project_path stub that only filtered enabled), and set_scope (:518) persists instead of discarding — both matching the MCP-side reference implementation.
  • project_scope.rs is the right shape: app-local agent-capabilities/subagent-scopes.json (atomic tmp+rename write), missing file defaults to all-global, corrupt file fails closed (returns an error instead of degrading to "everything visible") — that fail-closed choice matches the body's promise and is the safer default for a visibility control.
  • Keeping definitions in ~/.agents/subagents and not adding a project-level directory preserves ADR 0112's frozen decision; the scope living app-local (not in the shareable Markdown) is a reasonable split since scope lists carry machine-local absolute paths.
  • The A/B/no-project isolation tests (user_subagents.rs:844-851) cover the delegation-catalog filter, and the author's cross-project Unknown subagent runtime rejection check closes the "model forces the name" hole the issue's soft-isolation workaround couldn't.

Two non-blocking notes

  1. Policy: this is a feat during the fix/perf contribution window — the body already flags this honestly, so that call belongs to the maintainer; the implementation itself is ready for that decision to go either way.
  2. The scope file is keyed by subagent id; the rename-preserves-scope behavior is tested, which is the right place to have pinned it.

Once the stale assertion is updated this should be mergeable from a correctness standpoint.

zszz3 added 3 commits October 7, 2026 00:15
The settings list now labels each subagent with its activation scope.\nReplace the obsolete global-only expectation with both supported labels.
Resolve the ADR from the shared documentation directory so the Chinese
marketplace page no longer blocks documentation and workspace builds.
@muzimu217

Copy link
Copy Markdown
Contributor

Confirmed on our side: the new test(subagents): align settings assertions with project scope commit resolves the stale settings\.globalOnly assertion we reproduced from agent-capability-settings.test.mjs:42, and all five checks are green on the current head. From a correctness standpoint nothing is blocking; the feat/policy-window call stays with the maintainer as noted in the review.

zszz3 added 2 commits October 7, 2026 14:24
Keep the upstream canonical ADR link when combining both dead-link fixes.
Preserve the project scope implementation while incorporating current main.
Keep the project-scope acceptance scenario alongside the new main scenarios when resolving the E2E documentation conflict. Preserve the existing feature implementation and shared branch history.
@Tioit-Wang

Copy link
Copy Markdown
Collaborator

感谢这个 PR——也抱歉让你等了一段时间。

这个改动看起来做得不错,唯一缺的是视觉证据:由于这次改动涉及界面,仅凭 diff 我们没法判断实际效果。

方便的话,请在描述中补充两张截图:

  • 完整的界面截图:展示改动在实际界面中的位置与整体效果——子智能体作用范围的控件与对应设置页。
  • 修改位置的特写截图:裁切放大改动的具体位置,便于看清新增元素。

补充之后,我们就能看到真实效果,并放心判断是否合并。

zszz3 added 3 commits October 17, 2026 13:33
The list grouped user documents under a global level label, which read as
activation scope once a definition could be limited to selected projects.
Name the group after its source so it sits beside Built-in, and let each
row badge state where the definition applies.

Keys stay next to the other subagent source labels, and the dead
global-only key goes away now that the scope control replaced it.

The settings IA spec, the E2E plan and its Chinese mirror, and ADR 0270
describe the group by its new name.

Refs vastsa#1431
A session path under a folder with multibyte characters could not answer
`agents.active`: the prefix comparison sliced the target at a byte offset
that landed inside a character, so the task panicked and the caller waited
for its timeout. Compare whole lowercased strings instead.

Project scope also traveled through config sync. The payload carried the
scope, so a definition scoped to another device's folders either matched
nothing there or overwrote that device's own scope with the global one.
Scope names folders on one machine, so capture drops it and apply ignores
whatever arrives; an existing definition keeps this device's scope and a
new one starts global.

The editor warns while Projects is selected with no folder, reusing the
copy the floating picker already had.

Covered by a character-level path test, a two-device sync test against the
local S3 fixture, an apply-side unit test, and a sibling folder with a
multibyte name in the real host E2E.

Refs vastsa#1431
zszz3 added a commit to zszz3/PI-Desktop that referenced this pull request Oct 17, 2026
The first pass showed throwaway fixture projects; these use PI-Desktop and
apps/desktop so the scope picker and the delegation results read as a real
configuration.
@zszz3

zszz3 commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor Author

感谢 review。已在描述中补充两张截图,也直接放在这里:

1. 完整界面 —— 设置 → 子智能体。「自定义」分组(~/.agents/subagents)下每行带有自己的作用范围标签(全局 或 N 个项目),启用开关仍在行尾:

设置 → 子智能体

2. 改动位置特写 —— 编辑子智能体时的「作用范围」控件。选「指定项目」后,项目多选列表直接展开在编辑器内(带搜索框);当前打开的项目标有「当前打开」,但不会自动勾选:

作用范围控件

另外用真实模型 step-5-preview 在本仓库上验证了两遍(项目分别是仓库根目录和 apps/desktop):

场景 结果
在 apps/desktop 里委派 desktop-ui-reviewer 委派成功
在仓库根目录里调用 desktop-ui-reviewer 被拒绝

第二条返回 Unknown subagent "desktop-ui-reviewer". Available: code-reviewer, host-core-reviewer, …——限定到子目录的子智能体,在父项目里确实不可见。

还有三点补充:

  • 这个 PR 已同步最新 main,并修掉了合并前自查发现的两个问题,都带回归测试:
    • 作用范围匹配原先按字节偏移切片会话路径,项目名含中文等多字节字符时会 panic,导致该项目每条消息都要等 RPC 超时、且拿不到任何用户子智能体;改为整体字符串比较。
    • 云同步原先会携带并应用 scope,导致 A 设备专用的子智能体到 B 设备不可用,旧版本的全局设置还会冲掉项目限定;现在范围只留在本机。
  • CI 的 JS 检查是红的,但原因和本 PR 无关:apps/desktop/test/sidebar-pinned-rendering.test.mjs 在最新 main(4f9f051e5)上同样失败 —— 它断言侧边栏渲染日期分隔符,而主线的 22e6e121d 把该显示默认关掉了。我在纯净 main 的 worktree 里复现过。需要的话可以另开一个 PR 修这个测试。Rust、文档和基线检查都是绿的。

zszz3 added a commit to zszz3/PI-Desktop that referenced this pull request Oct 17, 2026
The project paths are the author's local directories, so they are covered
in the screenshot only; no product code changes.
@muzimu217

Copy link
Copy Markdown
Contributor

Since this PR is now the canonical implementation for #1431 (I'm withdrawing my duplicate #1561), flagging its one red check so it doesn't sit:

apps/desktop/test/* → "sidebar renders global pins once, outside project folding and history limits" fails on expected to match /class="thread-item-date-separator" aria-hidden="true">|<\/span>/. That is a sidebar rendering assertion, and this PR's diff does not touch the sidebar — it is the same class of drift the sidebar gained upstream (#1718 added last-reply dates, #1749 changed the running-activity badge). Rebasing onto current main and re-running should clear it; if it does not, the assertion is pinned to markup that upstream has since changed, not to anything in this branch.

Everything else is green and the PR is MERGEABLE, so this is the only thing between it and review.

@muzimu217

Copy link
Copy Markdown
Contributor

Correction to my earlier note — the red check is stale, and the fix has not been tried yet.

I said "rebase should clear it". Checking properly: the failing job is run 39007392721 / job 117322549692, dated 2026-10-17 05:51 UTC, and the PR head 7ca86ec95 is from 2026-10-17 05:33 — the branch has not been rebased since, so that red predates my comment and no new run has happened. updatedAt: 18:18 was comment activity, not a push.

What the stale failure actually is: thread-item-date-separator — and that string no longer exists anywhere in apps/desktop on current main (upstream changed the sidebar date markup). So the assertion was pinned to markup that upstream has since replaced, which is why it fails on this base.

Concretely for this branch:

  • 22 commits behind main, zero merge conflicts (git merge-tree clean).
  • After rebasing onto current main, that assertion is gone from the tree, so the red should not reproduce.

So the action is just: rebase and let CI run once. My earlier phrasing implied a rebase had already been tried — it has not.

@muzimu217

Copy link
Copy Markdown
Contributor

Reviewed the host side in an isolated worktree (head 7ca86ec9, 22 behind main, zero merge conflicts). This is a solid implementation — and it also fixes a real panic on the way.

Scope is enforced at execution, not just in the list. That was my main concern, so I traced it: agents.active (rpc/mod.rs:5173-5178) filters via active_for(project_path), and both consumers of subagent definitions call exactly that RPC with a project path — packages/host-runtime/src/launch-resolver.ts:146 and apps/desktop/electron/main/runtime/session-launch.ts:159, each passing { projectPath: projectPath ?? null }. agents.list (the unfiltered one) is only used by the settings surface and config-sync capture. So a project-scoped subagent genuinely cannot be launched from another project.

The multibyte fix is a real bug fix, not incidental. activation.rs previously did &target[..prefix.len()] — a byte slice whose length came from prefix. For a project path containing non-ASCII (the report's audience is largely Chinese), a byte offset can land inside a multibyte character and panic. The new whole-string starts_with comparison removes that, and multibyte_paths_match_by_character pins it with exactly the right cases (/code/中文 vs /code/中文项目, and the /Users/Ärger case-insensitive one). Worth calling out in the PR description — it is the kind of fix that deserves its own line.

Storage handling is careful. project_scope.rs keeps app-local visibility in a separate file from portable definitions and enablement (so older builds that rewrite enablement cannot clobber it), writes via temp-file + rename with cleanup on failure, defaults to empty on missing state, and fails closed on invalid JSON — with a test for that last part.

Two non-blocking notes:

  1. update() now does fs::remove_file(&current.path)? where it used to swallow the error (.ok()). That is stricter and I think correct (a silent leftover would resurrect the old id on next scan), but it does change failure behavior: a rename whose delete fails now surfaces instead of leaving the new record written and the old one present. Worth one sentence in the PR body.
  2. The scope file is keyed by subagent id and is not cleaned when a subagent is removed outside update() (e.g. remove()), so stale entries can accumulate. Harmless (they are ignored for unknown ids), just noting it in case you want a sweep later.

The only blocker to landing is the stale red check: the failing job is from 2026-10-17 05:51, before this head, and the assertion it fails on (thread-item-date-separator) no longer exists anywhere in apps/desktop on current main. A rebase onto current main should clear it — that has not been tried yet.

zszz3 added 2 commits October 19, 2026 11:06
Scanning user documents pruned enablement state for ids that no longer
exist but left their project scope behind. A document recreated later
under the same id silently inherited that scope, so a subagent came back
limited to projects the user never chose for it. Prune the scope file
against the same id set, next to the enablement prune.

Refs vastsa#1431
@zszz3

zszz3 commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

感谢这么细致的 review,特别是确认了范围是在执行时校验、而不只是在列表里隐藏。

关于你提的两点:

  1. 更严格的 unlink:确认,remove() 也有同样的改动(user_subagents.rs:498)。已在描述里补了说明:静默吞掉失败会让旧文件留下,下一轮扫描时旧 id 会复活。
  2. 范围清理:remove() 和重命名其实都会清理(:499、:468)。但你的提醒有实际价值——我写了脚本复现,真正漏掉的是在应用外删掉 Markdown 文件:清理启用状态时不会清理范围条目,之后重建同名文件会静默继承旧的项目限定(重启也不会恢复)。已修复:scan() 现在会在 prune 启用状态的同一处,把范围文件也按当前 id 集合清理,并补了回归测试(去掉修复会变红)。

另外已合入最新 main(e219a3e4c),零冲突。你提到的过期红灯也随之消失:thread-item-date-separator 在主线 21f5d1116 中已删除。现在 6 项检查在 53afe2381 上全部通过,PR 状态为 CLEAN。

@muzimu217

Copy link
Copy Markdown
Contributor

Re-reviewed the current head (53afe2381) after your fixes. The scope enforcement path holds up end to end, and I want to record the check that matters most for a reviewer who has not traced it:

  • agents.active → active_for(project_path) (rpc/mod.rs:5173-5178) → record.enabled && record.scope.matches(project_path) (user_subagents.rs:303-309).
  • Both consumers pass a real path, not an accidental None: launch-resolver.ts:146-148 and session-launch.ts:159-161 both send projectPath ?? null, and the project path is resolved before the call (launch-resolver.ts:308, session-launch.ts:537).
  • agents.list staying unfiltered (rpc/mod.rs:5169-5171) is correct — its only consumer is the settings page (skills-ipc.ts:341), which must show every definition to edit it. The delegation directory never reads list.

So a project-scoped definition cannot reach an unrelated session through any path I can find, and the None → false rule in matches is the right call for a project-less session.

One operational thing, not a code finding: the head is now 39 commits behind main (main at 8a8d56413, head based on e219a3e4c), with zero merge conflicts in a merge-tree rehearsal. That means the "Head contains latest base" gate will flip red on the next main push even though nothing is wrong with your branch. A rebase (or merge of main) now costs nothing and keeps the PR mergeable on demand — worth doing before it goes stale enough that the conflict-free property stops holding. Your last six checks are green on 53afe2381; that state is worth preserving.

Nothing else from me on the code. This is the implementation the #1431 proposal describes, and the two gaps you closed (unlink failure, out-of-app deletion leaving scope entries) were the ones I would have flagged next.

Refresh the PR head with upstream main (8a8d564) so the base gate stays satisfied. Upstream main changed only outside the subagent scope surface; no conflicts.

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.

[Feature] Activation scope for user subagents — keep dedicated subagents out of unrelated sessions

3 participants