Skip to content

skill slash命令bug修复#467

Merged
Nomikfk1215 merged 1 commit into
1024XEngineer:mainfrom
voicepeak:fixskill
May 13, 2026
Merged

skill slash命令bug修复#467
Nomikfk1215 merged 1 commit into
1024XEngineer:mainfrom
voicepeak:fixskill

Conversation

@voicepeak

Copy link
Copy Markdown
Collaborator

变更摘要

  • 移除 TUI 命令面板中动态生成的 skill slash 入口
  • 保留通过 /skills-select 打开 skill picker 并激活技能
  • skill picker 中只显示 skill 名称,不再显示 slash 前缀
  • 激活 skill 后的反馈不再展示 Entry: /...
  • 更新 TUI 测试,覆盖新的 picker-only skill 激活流程

@fennoai fennoai Bot 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.

A couple of issues stood out in the timeout/degradation path.

func terminateCommand(cmd *exec.Cmd) error {
if cmd == nil || cmd.Process == nil {
return os.ErrProcessDone
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium: on non-Windows this still kills only the direct server process, not its descendants. With this patch runRPC now returns as soon as ctx.Done() fires, so a timed-out MCP server that spawned a helper and inherited stdout/stderr can keep running after the request has already failed. The new hold_stdout_child test only uses a 2s child, so it doesn't catch the leak, but real helpers can outlive the request indefinitely.

Comment thread internal/extensions/mcp/adapter.go Outdated
return nil, newExtensionError(extensionspkg.ErrCodeInvalidExtension, "mcp adapter is nil", nil)
}
if err := a.maybeRefresh(ctx, false); err != nil && contextError(err) != nil {
if err := a.maybeRefresh(ctx, false); err != nil && ctx.Err() != nil && contextError(err) != nil {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Low: this returns the extension error verbatim whenever the caller context is done, which can preserve the wrong cause. If the caller cancels but the client reports its own DeadlineExceeded (or another wrapped context error), callers here still see the extension's timeout instead of ctx.Err(). The runner normalizes this later, but direct ResolveTools callers do not.

@codecov

codecov Bot commented May 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@Nomikfk1215 Nomikfk1215 merged commit b04317f into 1024XEngineer:main May 13, 2026
7 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.

2 participants