Skip to content

Add Skills + CLI mode for external agents#599

Merged
binaricat merged 3 commits intobinaricat:mainfrom
tces1:ACP+SKILLS+CLI
Apr 10, 2026
Merged

Add Skills + CLI mode for external agents#599
binaricat merged 3 commits intobinaricat:mainfrom
tces1:ACP+SKILLS+CLI

Conversation

@tces1
Copy link
Copy Markdown
Contributor

@tces1 tces1 commented Apr 1, 2026

Summary

This PR adds a new Skills + CLI integration mode for external ACP agents, while keeping the existing MCP path intact.

The main goal is to make external agents work reliably in environments where third-party MCP is unavailable or restricted, and to reduce overthinking / unnecessary discovery steps for simple Netcatty tasks.

What Changed

AI settings

  • Added a new Tool Access setting to switch between:
    • MCP
    • Skills + CLI
  • Persisted the selected mode in AI settings state/storage.
  • Added localized UI copy, including a note that Skills + CLI currently requires a working Node.js runtime on the host.

External agent flow

  • Added a Skills + CLI path for ACP-backed external agents.
  • In Skills + CLI mode, Netcatty no longer injects the Netcatty MCP server into the external agent.
  • Instead, Netcatty provides:
    • a local skill file
    • an exact CLI invocation pattern
    • current chat/session context

netcatty-tool-cli

  • Added a new local CLI bridge:
    • env
    • session
    • resource environment
    • exec
    • status
    • cancel
    • resume
  • Added a local RPC client and discovery path support for the CLI bridge.
  • Reused existing main-process execution logic and safety checks rather than duplicating command execution behavior.

Skills and prompt improvements

  • Added a Netcatty skill for the CLI workflow.
  • Split the skill into:
    • a short primary SKILL.md
    • focused reference docs for session types, control commands, and errors
  • Tightened prompt guidance for simple read-only tasks:
    • prefer the shortest path
    • avoid extra script generation
    • avoid $() and backticks
    • avoid unnecessary sh -c / bash -c wrappers
  • Prefer concise tabular output when appropriate.

Session targeting optimization

  • The host now provides the external agent with a default target session when it can determine one safely.
  • For simple requests, the agent can use the default session directly instead of always rediscovering sessions first.
  • Fallback discovery remains available when the target is ambiguous or the user explicitly references another session.

Packaging

  • Fixed packaged app behavior for Skills + CLI by ensuring:
    • skill files are available from unpacked app resources
    • CLI assets are available from unpacked app resources
    • packaged paths resolve correctly in aiBridge
  • This avoids packaged-app failures where external agents could not find the Netcatty skill or CLI files.

Reliability / cleanup

  • Removed unused MCP preload helper code.
  • Hardened CLI RPC behavior to avoid indefinite hangs on write/connection failures.
  • Fixed execution lock handling for synchronous throws in command execution paths.

Notes

  • The MCP contract is intentionally left unchanged.
  • The new CLI/session-targeting optimizations are scoped to the Skills + CLI path only.

Testing

  • npm run build
  • node --check electron/bridges/aiBridge.cjs
  • Manual validation of:
    • MCP vs Skills + CLI settings toggle
    • Skills + CLI execution against a live Netcatty session
    • packaged macOS app behavior from /Applications/Netcatty.app

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5063a85a9b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread electron/cli/netcattyRpcClient.cjs Outdated
Comment thread electron/cli/netcatty-tool-cli.cjs Outdated
@tces1
Copy link
Copy Markdown
Contributor Author

tces1 commented Apr 1, 2026

这边推动这个修改的一个现实原因是,企业环境里 MCP 权限经常是被锁住的,用户拿不到第三方MCP使用权限(仅支持内置),导致基于注入式的 MCP 的路径无法使用。
相比之下,Skills 这条路径仍然可以继续使用,所以从兼容性和可落地性来看,建议增加支持 Skills + CLI 的方式。
目前MCP仍然是默认注入方式,可在设置AI中选择。
image

@binaricat
Copy link
Copy Markdown
Owner

你说的这个问题和场景我明白了。

首先感谢你的热情付出,这个PR比较大,我要仔细看看,此外我也调研下是否有更温和的方式,我的担忧主要是从产品设计的角度来说,每增加一个选项都会提高用户的理解成本。

这个PR我先留着慢慢评估,如果有更好的方案我会直接在上面修改。

@tces1
Copy link
Copy Markdown
Contributor Author

tces1 commented Apr 2, 2026

理解你的顾虑,这个问题也是在公司内部打算推荐的时候发现的,并且Skills模式也是一个雏形,还有需要完善对齐的地方。
另外问觉得加载用户自定义skills也可以考虑,你怎么想?

@binaricat
Copy link
Copy Markdown
Owner

理解你的顾虑,这个问题也是在公司内部打算推荐的时候发现的,并且Skills模式也是一个雏形,还有需要完善对齐的地方。 另外问觉得加载用户自定义skills也可以考虑,你怎么想?

skills很有必要,可以用来编排复杂的流程。这也是个大功能了。过段时间来看看怎么弄比较好。

@tces1
Copy link
Copy Markdown
Contributor Author

tces1 commented Apr 9, 2026

好的,我目前切换到这个分支,打算补齐长命令的支持,还需要一些时间,后面我会把reviwq的结果进行修复

@binaricat
Copy link
Copy Markdown
Owner

Hi @tces1,感谢贡献!我让 Claude 对这个 PR 做了一次较为系统的审查,下面是它的审查意见。里面提到的点还需要你再核对确认一下是否准确——特别是 P0 那几项(sync.yml、TCP bridge 无条件启动、handleGetContextexec 的作用域检查、硬编码本地路径)。等相关问题确认并修复之后,我们会继续再看这个 PR,也欢迎你随时讨论具体实现细节。🙏


Hi @tces1,感谢这个 PR!看得出投入了不少精力,auth token 的设计、按 chat 维度的执行锁处理、还有结构化的 skill 文档都做得很用心。让外部 ACP agent 不依赖 MCP 也能工作这个目标很有价值,整体架构也比较合理。

我仔细过了一遍代码,有一些反馈想在合入前先讨论一下。按优先级分组,最重要的放在前面。

希望合入前能处理的问题

1. 请移除 .github/workflows/sync.yml

这个文件看起来是从你的 fork 里带过来的——它的 upstream 指向 binaricat/Netcatty(也就是要合入的目标仓库),合入后会每 2 小时 cron 跑一次"自己同步自己"。作为一个通用策略,我尽量不接受 PR 里新增的 CI workflow,除非是明确请求过的,因为一个有 contents: write 权限的 workflow 文件影响面太大。可以把这个文件从分支里删掉吗?

2. TCP 桥接启动需要按 Skills + CLI 模式门控

electron/bridges/aiBridge.cjs 362 行附近,mcpServerBridge.getOrCreateHost() 是在 init() 里无条件调用的。这意味着每个 Netcatty 用户——哪怕完全不用 AI 功能——一启动应用就会有一个 localhost TCP 监听器,并生成 ~/.netcatty/ai-cli/discovery.json(含 auth token)。而在当前 main 分支上,这个 bridge 只在外部 ACP agent 实际需要时才懒加载启动。

能不能只在 toolIntegrationMode === "skills" 时才启动 bridge,用户切回 MCP 时销毁?这样 MCP 模式下的用户攻击面和原来一样,新功能也真正做到了 opt-in。

3. handleGetContext 的作用域检查

electron/bridges/mcpServerBridge.cjs 871–930 行,如果调用方既没传 chatSessionId 也没传 scopedSessionIds,handler 会返回完整的 session 列表(hostname、label、username、protocol、OS 等)。CLI 路径上总是会带 --chat-session 所以没问题,但任何能通过合法 token 触达 RPC 的其他进程都能枚举所有已连接的主机。

能在这里强制要求一个作用域吗——没有作用域时返回空列表或显式报错?887 行的注释其实已经在暗示这个行为了,只是没有真正执行。

4. netcatty/exec 需要强制要求 chatSessionId

和上面相关——autonomous 模式下,dispatch() 中的 exec 不强制要求 chatSessionId,所以 809–821 行的审批门相当于被跳过了。再加上 validateSessionScope(752–757 行)里空作用域 fallback 的行为,autonomous 模式下拿到 token 的调用方可以在任何 session 上执行命令,完全绕过按 chat 隔离。

能不能对所有写操作方法无条件要求 chatSessionId,和 permission mode 无关?CLI 本来就总是传这个参数,所以这是一个纯粹的纵深防御改动,对用户没有任何可见影响,但可以保住 PR 想实现的隔离语义。

5. SKILL.md 里硬编码的本地路径

skills/netcatty-tool-cli/SKILL.md 43–44 行有你本机的绝对路径(/Users/eric/Documents/GitHub/Netcatty/...)。这会随着包一起发给所有用户,也泄露了 username。能改成仓库相对路径(electron/cli/netcatty-tool-cli.cjselectron/bridges/aiBridge.cjs),或者直接把 TODO 块删掉吗?

6. 重复的 chatSessionId key

小问题——electron/bridges/mcpServerBridge.cjs 1249–1250 行同一个 options 对象里 chatSessionId 声明了两次。JS 会让后面那个生效所以不算功能 bug,但 ESLint 的 no-dupe-keys 规则应该能查出来。建议合入前在分支上跑一下 npm run lint

范围 & 无关改动

我注意到 PR 里有一些看起来和 Skills + CLI 功能无关的改动,希望能分开讨论:

Copilot 模型预设

PR 从 components/AIChatSidePanel.tsx(478–505 行附近)删掉了 runtimeAgentModelPresets 状态和 bridge.aiAcpListModels(...) 那个 effect,替换成了 infrastructure/ai/types.ts 里一个静态的 COPILOT_MODEL_PRESETS 数组。能聊聊这个改动的动机吗?在 main 分支上,Copilot 的模型列表是从 agent 动态获取的,所以这是一个行为变更,会影响完全不用 Skills + CLI 的用户。如果动态获取有 bug,我很乐意把这个修复独立成一个 PR 合进来,并配上清晰的说明;如果目的是维护一个稳定的预设列表,我想确认一下这些 model ID 和今天 Copilot 的 ACP server 实际返回的是否一致。

其他被捎带的改动

还有几个看起来正交的小改动——sftpBridge.cjs 的新增(基于 session 的 SFTP client)、useAIState.ts 的 permission mode 校验、AIBridge 接口的类型改进。这些改动本身都没问题,但如果能拆成独立的 PR 会更好 review。能否拆出去,或者至少在 PR 描述里标注一下让 reviewer 知道这些是有意为之?

一些小建议(不是阻塞)

  • global.d.ts 774 行:新加了 toolIntegrationMode 参数到 aiAcpStream,但 preload.cjsacpAgentAdapter.ts 实际传递的 defaultTargetSession 参数没有在这里声明。adapter 里的 as unknown as AcpBridge 把这个不一致盖住了。能把 defaultTargetSession?: {...} 也加到全局声明里保持一致吗?

  • CLI exec 参数拼接netcatty-tool-cli.cjs 321 行):opts.command.join(" ") 在 agent 传递本应作为单个 token 的参数时会丢失引号。agent 一般都是直接构造 shell 就绪的字符串,更简单的做法或许是在 -- 之后只接受单个位置参数,而不是把整个数组拼起来。

  • Skill 文档里的 TODOSKILL.md 和 CLI --help 输出里有几个 TODO 块,对随包发布的内容来说有点噪。改成代码注释或放到 issue 里追踪会好一些。

  • Node.js 依赖:设置描述里提到 Skills + CLI 需要宿主机有 Node.js。对一个已经通过 process.execPath + ELECTRON_RUN_AS_NODE=1 自带 Node runtime 的 Electron 应用来说,这个依赖有点脆弱(bridge 里的 resolveMcpServerRuntimeCommand 就是这么做的)。能不能复用这个思路,让用户不需要系统装 Node?不是阻塞项,只是体验会更好。

  • RPC 传输层:长期来看,从 localhost TCP 改成 Unix domain socket(POSIX)/ named pipe(Windows)可以让内核按用户强制访问控制,auth token 甚至都不需要了。当然这超出了本 PR 的范围,但值得作为后续优化方向考虑。

做得好的地方

  • Auth token 的设计(32 字节随机、0o600 的 discovery 文件、强制第一个 RPC 必须 auth)对工作站场景是合理的威胁模型。
  • 执行锁在 sync-throw / async-reject / success 三条路径上的处理看起来是正确的——runExecution.finally() 包装加上 beginChatExecution / release 的配对覆盖了能想到的所有情况。
  • validateSessionScope 里"显式空数组 = 无权限"的注释写得特别好。
  • resolveRpcTimeoutMs 根据 bridge 配置的 command timeout 动态调整,对长时间运行的 exec 很有帮助。
  • Skill markdown 文件作为给 agent 看的文档结构很清晰,SKILL.md 和 references 之间的路由也很明确。

总结

核心功能是希望合入的,大部分代码也看起来没问题——上面的问题主要是范围、无条件启动 bridge、以及几个 RPC 守卫的收紧,大多都只需要加一两行。如果能处理下第一部分的 6 个阻塞项,并把 Copilot 改动拆出去,应该很快就能合入。

如果有哪里不清楚,或者想讨论任何设计决策,欢迎随时回复。再次感谢你的贡献!

@binaricat
Copy link
Copy Markdown
Owner

好的,我目前切换到这个分支,打算补齐长命令的支持,还需要一些时间,后面我会把reviwq的结果进行修复

之前回复的是英文版,你看下中文的,有动态咱们随时在PR里同步😂

@tces1
Copy link
Copy Markdown
Contributor Author

tces1 commented Apr 10, 2026

截屏2026-04-10 10 51 57

更新一下流程图
我本地用5.4 xhigh 跑了十几轮review,到这里基本没看到其他问题了

@binaricat binaricat merged commit c771979 into binaricat:main Apr 10, 2026
@tces1 tces1 deleted the ACP+SKILLS+CLI branch April 13, 2026 05:58
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