ACP / MCP 完善

This commit is contained in:
2026-06-11 12:43:18 +08:00
parent 1415d3b43b
commit 0561e126cd
22 changed files with 786 additions and 219 deletions
+3
View File
@@ -101,6 +101,9 @@ func (f *fakeAgentKindRepo) UpdateSessionRunning(context.Context, uuid.UUID, str
func (f *fakeAgentKindRepo) UpdateSessionFinished(context.Context, uuid.UUID, acp.SessionStatus, *int32, *string) error {
panic("n/a")
}
func (f *fakeAgentKindRepo) MarkSessionFailedIfActive(context.Context, uuid.UUID, string) (bool, error) {
panic("n/a")
}
func (f *fakeAgentKindRepo) CountActiveSessions(context.Context) (int64, error) {
panic("n/a")
}
+1 -1
View File
@@ -58,7 +58,7 @@ type AgentKind struct {
Args []string
EncryptedEnv []byte
Enabled bool
ToolAllowlist []string // 命中的工具调用自动放行;含 "*" 表示全部放行
ToolAllowlist []string // 命中的工具调用自动放行;含 "*" 表示放行全部可识别的工具(无法识别名称的调用仍走人工审批)
CreatedBy uuid.UUID
CreatedAt time.Time
UpdatedAt time.Time
+6 -31
View File
@@ -4,11 +4,11 @@ import (
"context"
"encoding/json"
"errors"
"fmt"
"io/fs"
"os"
"path/filepath"
"strings"
"github.com/yan1h/agent-coding-workflow/internal/acp/pathsafe"
)
// FsReadHandler handles fs/read_text_file (spec §5.4).
@@ -75,34 +75,9 @@ func sliceLines(s string, line, limit *int) string {
return strings.Join(lines[start:end], "\n")
}
// resolveSafePath duplicates the logic from internal/acp.ResolveSafePath to
// avoid an import cycle (handlers is a sub-package and acp imports handlers).
// resolveSafePath 委托共享实现 pathsafe.ResolveSafePath
// handlers 子包不能 import internal/acp 主包(acp 依赖 handlers,会循环),
// 故校验逻辑抽到 internal/acp/pathsafe 子包共用。
func resolveSafePath(cwd, requested string) (string, error) {
if !filepath.IsAbs(cwd) {
return "", fmt.Errorf("cwd must be absolute")
}
cleanCwd := filepath.Clean(cwd)
var abs string
if filepath.IsAbs(requested) {
abs = filepath.Clean(requested)
} else {
abs = filepath.Clean(filepath.Join(cleanCwd, requested))
}
if !hasPathPrefix(abs, cleanCwd) {
return "", fmt.Errorf("path outside worktree")
}
if real, err := filepath.EvalSymlinks(abs); err == nil {
if !hasPathPrefix(real, cleanCwd) {
return "", fmt.Errorf("path resolves outside worktree via symlink")
}
}
return abs, nil
}
func hasPathPrefix(abs, base string) bool {
if abs == base {
return true
}
sep := string(filepath.Separator)
return strings.HasPrefix(abs, base+sep)
return pathsafe.ResolveSafePath(cwd, requested)
}
@@ -45,3 +45,28 @@ func TestFsWrite_OutsideWorktree(t *testing.T) {
require.NotNil(t, res.Err)
assert.Equal(t, -32001, res.Err.Code)
}
// 父目录是 worktree 内指向外部的 symlink、目标文件不存在 → 必须被拒,
// 且外部目录不能出现新建文件(防 symlink 逃逸写出 worktree)。
func TestFsWrite_SymlinkDirEscape(t *testing.T) {
t.Parallel()
tmp := t.TempDir()
outside := t.TempDir()
link := filepath.Join(tmp, "link-dir")
if err := os.Symlink(outside, link); err != nil {
t.Skipf("symlink not supported on this platform: %v", err)
}
h := handlers.NewFsWriteHandler()
sess := handlers.SessionContext{CwdPath: tmp}
params := mustJSON(t, map[string]any{
"sessionId": "x", "path": filepath.Join(link, "escape.txt"), "content": "evil",
})
res := h.Handle(context.Background(), sess, params)
require.NotNil(t, res.Err)
assert.Equal(t, -32001, res.Err.Code)
_, statErr := os.Stat(filepath.Join(outside, "escape.txt"))
assert.True(t, os.IsNotExist(statErr), "外部目录不应被写入")
}
+91
View File
@@ -0,0 +1,91 @@
// Package pathsafe 实现 ACP fs/* method 的路径安全校验。
//
// 单独成包是因为 internal/acp/handlers 子包不能 import internal/acp 主包
// (acp 依赖 handlers,会循环),故抽到共享子包供两边复用。
//
// 规则(spec §5.6):
// 1. 路径必须以 cwd_path 为前缀(防 ../../etc/passwd)
// 2. 解析 symlink 后的真实路径必须仍在 cwd_path 下(防 symlink 逃逸);
// 目标不存在时(如 fs/write_text_file 新建文件),向上找最近已存在的
// 祖先目录解析真实路径后再校验,防经由指向外部的目录 symlink 写出 worktree。
//
// 三重校验:filepath.Abs + hasPathPrefix + filepath.EvalSymlinks。
package pathsafe
import (
"path/filepath"
"strings"
"github.com/yan1h/agent-coding-workflow/internal/infra/errs"
)
// ResolveSafePath 校验 requested 是否安全(在 cwd 内)。
// cwd 必须是绝对路径;requested 可以是绝对或相对(相对时基于 cwd 解析)。
// 返回校验通过后的绝对路径(含 symlink 解析);失败返回 CodeAcpFsPathOutsideWorktree。
func ResolveSafePath(cwd, requested string) (string, error) {
if !filepath.IsAbs(cwd) {
return "", errs.New(errs.CodeAcpFsPathOutsideWorktree, "cwd must be absolute")
}
cleanCwd := filepath.Clean(cwd)
var abs string
if filepath.IsAbs(requested) {
abs = filepath.Clean(requested)
} else {
abs = filepath.Clean(filepath.Join(cleanCwd, requested))
}
if !hasPathPrefix(abs, cleanCwd) {
return "", errs.New(errs.CodeAcpFsPathOutsideWorktree, "path outside worktree")
}
real, err := filepath.EvalSymlinks(abs)
if err != nil {
// 目标(或部分祖先)不存在:不能静默放行,否则父目录若是指向外部的
// symlink,后续 MkdirAll/WriteFile 会跟随 symlink 写出 worktree。
// 改为向上找最近已存在的祖先解析真实路径,再拼回剩余部分校验。
real, err = evalNearestAncestor(abs)
if err != nil {
return "", errs.Wrap(err, errs.CodeAcpFsPathOutsideWorktree, "resolve path")
}
}
if !hasPathPrefix(real, cleanCwd) {
return "", errs.New(errs.CodeAcpFsPathOutsideWorktree, "path resolves outside worktree via symlink")
}
return abs, nil
}
// evalNearestAncestor 自 abs 向上找最近已存在(EvalSymlinks 成功)的祖先目录,
// 用其真实路径拼回剩余相对部分,得到 abs 对应的真实路径。
// 直到根目录仍无法解析时返回错误(调用方按路径不安全处理)。
func evalNearestAncestor(abs string) (string, error) {
cur := abs
rest := ""
for {
parent := filepath.Dir(cur)
if parent == cur {
// 已到根目录仍解析失败(如盘符/根不存在)
return "", errs.New(errs.CodeAcpFsPathOutsideWorktree, "no existing ancestor for path")
}
if rest == "" {
rest = filepath.Base(cur)
} else {
rest = filepath.Join(filepath.Base(cur), rest)
}
cur = parent
if real, err := filepath.EvalSymlinks(cur); err == nil {
return filepath.Join(real, rest), nil
}
}
}
// hasPathPrefix reports whether abs is equal to base or under base. Trailing
// separator handling is critical: we want "/foo/bar" to be inside "/foo" but
// NOT inside "/fo".
func hasPathPrefix(abs, base string) bool {
if abs == base {
return true
}
sep := string(filepath.Separator)
return strings.HasPrefix(abs, base+sep)
}
+5 -44
View File
@@ -1,55 +1,16 @@
// permission.go 实现 ACP fs/* method 的路径安全校验。
// permission.go 暴露 ACP fs/* method 的路径安全校验入口
//
// 规则(spec §5.6):
// 1. 路径必须以 cwd_path 为前缀(防 ../../etc/passwd)
// 2. 解析 symlink 后的真实路径必须仍在 cwd_path 下(防 symlink 逃逸)
//
// 三重校验:filepath.Abs + strings.HasPrefix + filepath.EvalSymlinks。
// 具体校验逻辑在 internal/acp/pathsafe 子包(与 handlers 子包共用同一实现,
// 详见 pathsafe 包注释)。
package acp
import (
"path/filepath"
"strings"
"github.com/yan1h/agent-coding-workflow/internal/infra/errs"
"github.com/yan1h/agent-coding-workflow/internal/acp/pathsafe"
)
// ResolveSafePath 校验 requested 是否安全(在 cwd 内)。
// cwd 必须是绝对路径;requested 可以是绝对或相对(相对时基于 cwd 解析)。
// 返回校验通过后的绝对路径(含 symlink 解析);失败返回 CodeAcpFsPathOutsideWorktree。
func ResolveSafePath(cwd, requested string) (string, error) {
if !filepath.IsAbs(cwd) {
return "", errs.New(errs.CodeAcpFsPathOutsideWorktree, "cwd must be absolute")
}
cleanCwd := filepath.Clean(cwd)
var abs string
if filepath.IsAbs(requested) {
abs = filepath.Clean(requested)
} else {
abs = filepath.Clean(filepath.Join(cleanCwd, requested))
}
if !hasPathPrefix(abs, cleanCwd) {
return "", errs.New(errs.CodeAcpFsPathOutsideWorktree, "path outside worktree")
}
real, err := filepath.EvalSymlinks(abs)
if err == nil {
if !hasPathPrefix(real, cleanCwd) {
return "", errs.New(errs.CodeAcpFsPathOutsideWorktree, "path resolves outside worktree via symlink")
}
}
return abs, nil
}
// hasPathPrefix reports whether abs is equal to base or under base. Trailing
// separator handling is critical: we want "/foo/bar" to be inside "/foo" but
// NOT inside "/fo".
func hasPathPrefix(abs, base string) bool {
if abs == base {
return true
}
sep := string(filepath.Separator)
return strings.HasPrefix(abs, base+sep)
return pathsafe.ResolveSafePath(cwd, requested)
}
+6 -5
View File
@@ -332,13 +332,14 @@ func extractToolName(toolCall json.RawMessage) string {
return ""
}
// allowlistHit 判断工具是否在白名单内。含 "*" 表示全部放行;toolName 为空时不放行。
// allowlistHit 判断工具是否在白名单内。含 "*" 表示放行全部可识别的工具;
// toolName 为空(无法识别的 tool call)时一律不放行,保守走人工审批。
func allowlistHit(allowlist []string, toolName string) bool {
if toolName == "" {
return false
}
for _, a := range allowlist {
if a == "*" {
return true
}
if toolName != "" && a == toolName {
if a == "*" || a == toolName {
return true
}
}
@@ -0,0 +1,28 @@
package acp
import "testing"
// TestAllowlistHit 覆盖白名单匹配的边界:空 toolName 一律不放行(含 "*"),
// 白名单中的空串条目不误匹配。
func TestAllowlistHit(t *testing.T) {
cases := []struct {
name string
allowlist []string
toolName string
want bool
}{
{"精确命中", []string{"safe.tool"}, "safe.tool", true},
{"未命中", []string{"safe.tool"}, "other.tool", false},
{"通配符放行可识别工具", []string{"*"}, "any.tool", true},
{"通配符不放行空名", []string{"*"}, "", false},
{"空名不匹配空串条目", []string{""}, "", false},
{"空白名单", nil, "any.tool", false},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
if got := allowlistHit(c.allowlist, c.toolName); got != c.want {
t.Fatalf("allowlistHit(%v, %q) = %v, want %v", c.allowlist, c.toolName, got, c.want)
}
})
}
}
+45
View File
@@ -122,6 +122,51 @@ func TestPermission_AutoAllow_WhenAllowlisted(t *testing.T) {
require.Equal(t, acp.PermissionAuto, repo.statusOf(repo.onlyReqID()))
}
func TestPermission_AutoAllow_Wildcard(t *testing.T) {
repo := newPermFakeRepo()
svc := acp.NewPermissionService(repo, nil, time.Minute, nil)
sess := handlers.SessionContext{
SessionID: uuid.New(),
UserID: uuid.New(),
ToolAllowlist: []string{"*"},
}
chosen := svc.Decide(context.Background(), sess, "req-1",
[]byte(`{"name":"any.tool"}`), optionsAllowReject())
require.Equal(t, "allow_once", chosen)
require.Equal(t, acp.PermissionAuto, repo.statusOf(repo.onlyReqID()))
}
func TestPermission_Wildcard_UnknownToolName_Pending(t *testing.T) {
// allowlist 含 "*" 但 toolCall 无法识别名称 → 不自动放行,走人工审批。
repo := newPermFakeRepo()
uid := uuid.New()
sid := uuid.New()
repo.sessions[sid] = &acp.Session{ID: sid, UserID: uid}
svc := acp.NewPermissionService(repo, nil, time.Minute, nil)
sess := handlers.SessionContext{SessionID: sid, UserID: uid, ToolAllowlist: []string{"*"}}
resCh := make(chan string, 1)
go func() {
resCh <- svc.Decide(context.Background(), sess, "req-1",
[]byte(`{"unknownField":1}`), optionsAllowReject())
}()
var reqID uuid.UUID
require.Eventually(t, func() bool {
reqID = repo.onlyReqID()
return reqID != uuid.Nil && repo.statusOf(reqID) == acp.PermissionPending
}, time.Second, 5*time.Millisecond)
require.NoError(t, svc.Resolve(context.Background(), acp.Caller{UserID: uid}, reqID, true, "allow_once"))
select {
case chosen := <-resCh:
require.Equal(t, "allow_once", chosen)
case <-time.After(time.Second):
t.Fatal("Decide 未在批准后返回")
}
require.Equal(t, acp.PermissionApproved, repo.statusOf(reqID))
}
func TestPermission_Pending_ThenApprove(t *testing.T) {
repo := newPermFakeRepo()
uid := uuid.New()
+31
View File
@@ -94,6 +94,37 @@ func TestResolveSafePath_NonexistentInsideOK(t *testing.T) {
got, err := acp.ResolveSafePath(tmp, want)
require.NoError(t, err)
assert.Equal(t, want, got)
// 多级不存在的中间目录(祖先均为 cwd 内真实目录)同样合法,不能误杀
want = filepath.Join(tmp, "newdir", "sub", "newfile.txt")
got, err = acp.ResolveSafePath(tmp, want)
require.NoError(t, err)
assert.Equal(t, want, got)
}
// 父目录是指向外部的 symlink 且叶子文件不存在 → 必须被拒,
// 否则 fs/write_text_file 会跟随 symlink 把新建文件写出 worktree。
func TestResolveSafePath_SymlinkDirEscapeNonexistentLeaf(t *testing.T) {
t.Parallel()
tmp := t.TempDir()
outside := t.TempDir()
link := filepath.Join(tmp, "link-dir")
if err := os.Symlink(outside, link); err != nil {
t.Skipf("symlink not supported on this platform: %v", err)
}
// 叶子不存在
_, err := acp.ResolveSafePath(tmp, filepath.Join(link, "newfile.txt"))
ae, ok := errs.As(err)
require.True(t, ok)
assert.Equal(t, errs.CodeAcpFsPathOutsideWorktree, ae.Code)
// 叶子与中间目录都不存在(symlink 是更远的祖先)同样被拒
_, err = acp.ResolveSafePath(tmp, filepath.Join(link, "sub", "newfile.txt"))
ae, ok = errs.As(err)
require.True(t, ok)
assert.Equal(t, errs.CodeAcpFsPathOutsideWorktree, ae.Code)
}
func TestResolveSafePath_RelativeCwdRejected(t *testing.T) {
+5
View File
@@ -45,6 +45,11 @@ UPDATE acp_sessions
SET status = $2, exit_code = $3, last_error = $4, ended_at = now()
WHERE id = $1;
-- name: MarkSessionFailedIfActive :execrows
UPDATE acp_sessions
SET status = 'crashed', last_error = $2, ended_at = now()
WHERE id = $1 AND status IN ('starting', 'running');
-- name: CountActiveSessions :one
SELECT COUNT(*) FROM acp_sessions WHERE status IN ('starting', 'running');
+3
View File
@@ -93,6 +93,9 @@ func (f *fakeEventRepo) UpdateSessionRunning(context.Context, uuid.UUID, string,
func (f *fakeEventRepo) UpdateSessionFinished(context.Context, uuid.UUID, acp.SessionStatus, *int32, *string) error {
panic("n/a")
}
func (f *fakeEventRepo) MarkSessionFailedIfActive(context.Context, uuid.UUID, string) (bool, error) {
panic("n/a")
}
func (f *fakeEventRepo) CountActiveSessions(context.Context) (int64, error) { panic("n/a") }
func (f *fakeEventRepo) CountActiveSessionsByUser(context.Context, uuid.UUID) (int64, error) {
panic("n/a")
+14
View File
@@ -36,6 +36,9 @@ type Repository interface {
ListAllSessions(ctx context.Context, requirementID *uuid.UUID) ([]*Session, error)
UpdateSessionRunning(ctx context.Context, id uuid.UUID, agentSessionID string, pid int32) error
UpdateSessionFinished(ctx context.Context, id uuid.UUID, status SessionStatus, exitCode *int32, lastErr *string) error
// MarkSessionFailedIfActive 把仍处于 starting/running 的 session 标记为 crashed
// 并写入 lastErr;已是终态时不更新(守卫式,避免覆盖 onExit 写入的终态)。
MarkSessionFailedIfActive(ctx context.Context, id uuid.UUID, lastErr string) (bool, error)
CountActiveSessions(ctx context.Context) (int64, error)
CountActiveSessionsByUser(ctx context.Context, userID uuid.UUID) (int64, error)
ResetStuckSessionsOnRestart(ctx context.Context) ([]*Session, error)
@@ -339,6 +342,17 @@ func (r *pgRepo) UpdateSessionFinished(ctx context.Context, id uuid.UUID, status
return nil
}
func (r *pgRepo) MarkSessionFailedIfActive(ctx context.Context, id uuid.UUID, lastErr string) (bool, error) {
n, err := r.q.MarkSessionFailedIfActive(ctx, acpsqlc.MarkSessionFailedIfActiveParams{
ID: toPgUUID(id),
LastError: &lastErr,
})
if err != nil {
return false, errs.Wrap(err, errs.CodeInternal, "mark session failed if active")
}
return n > 0, nil
}
func (r *pgRepo) CountActiveSessions(ctx context.Context) (int64, error) {
n, err := r.q.CountActiveSessions(ctx)
if err != nil {
+23
View File
@@ -117,6 +117,23 @@ func ResolveBranch(ctx context.Context, ws *workspace.Workspace, sessionID uuid.
// 9. async supervisor.Spawn(失败回滚 worktree)
// 10. async initial_prompt 转发(等握手完成)
func (s *sessionService) Create(ctx context.Context, c Caller, in CreateSessionInput) (*Session, error) {
// 0. 互斥校验:branch / issue_number / requirement_number 三选一或全空(spec §8.1)。
// service 层统一拦截,HTTP / MCP 入口共用。
specified := 0
if in.Branch != nil && *in.Branch != "" {
specified++
}
if in.IssueNumber != nil {
specified++
}
if in.RequirementNumber != nil {
specified++
}
if specified > 1 {
return nil, errs.New(errs.CodeInvalidInput,
"branch / issue_number / requirement_number are mutually exclusive; specify at most one")
}
// 1. workspace + authz
wsCaller := workspace.Caller{UserID: c.UserID, IsAdmin: c.IsAdmin}
ws, err := s.wsSvc.Get(ctx, wsCaller, in.WorkspaceID)
@@ -251,6 +268,12 @@ func (s *sessionService) Create(ctx context.Context, c Caller, in CreateSessionI
spawnCtx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
defer cancel()
if _, err := s.sup.Spawn(spawnCtx, out, kind, extraEnv); err != nil {
// 守卫式标记:Spawn 早期失败(进程未注册、onExit 不会触发)时把
// starting 收尾为 crashed;handshake 失败路径 onExit 已写终态,此处 no-op。
if _, merr := s.repo.MarkSessionFailedIfActive(spawnCtx, out.ID, err.Error()); merr != nil {
s.sup.log.Error("acp.spawn_failed.mark_session",
"session_id", out.ID, "err", merr.Error())
}
_ = s.mcpTokens.RevokeBySession(spawnCtx, out.ID)
s.releaseOnFailure(spawnCtx, out, sessCaller, worktreeID)
}
+108
View File
@@ -245,6 +245,19 @@ func (r *fakeAcpRepo) UpdateSessionRunning(_ context.Context, _ uuid.UUID, _ str
func (r *fakeAcpRepo) UpdateSessionFinished(_ context.Context, _ uuid.UUID, _ acp.SessionStatus, _ *int32, _ *string) error {
return nil
}
func (r *fakeAcpRepo) MarkSessionFailedIfActive(_ context.Context, id uuid.UUID, lastErr string) (bool, error) {
r.mu.Lock()
defer r.mu.Unlock()
for _, s := range r.sessions {
if s.ID == id && s.Status.IsActive() {
s.Status = acp.SessionCrashed
msg := lastErr
s.LastError = &msg
return true, nil
}
}
return false, nil
}
func (r *fakeAcpRepo) ResetStuckSessionsOnRestart(_ context.Context) ([]*acp.Session, error) {
return nil, nil
}
@@ -444,6 +457,101 @@ func TestSessionService_Create_TokenIssueFails_RollsBack(t *testing.T) {
repo.mu.Unlock()
}
func TestSessionService_Create_SpawnFails_MarksSessionCrashed(t *testing.T) {
t.Parallel()
uid := uuid.New()
wsID := uuid.New()
pid := uuid.New()
akID := uuid.New()
repo := &fakeAcpRepo{
kinds: []*acp.AgentKind{
// BinaryPath 不存在 → supervisor.Spawn 在 cmd.Start 早期失败,
// 进程未注册、onExit 不会触发。
{ID: akID, Name: "claude", Enabled: true, BinaryPath: "/nonexistent/agent-binary"},
},
}
wsSvc := &fakeWsSvc{ws: &workspace.Workspace{
ID: wsID, ProjectID: pid, DefaultBranch: "main", MainPath: "/tmp/main",
}}
pa := fakeProjectAccess{pj: &project.Project{ID: pid, Slug: "test-proj"}}
mcpTokens := &fakeMCPTokenSvc{}
sup := acp.NewSupervisor(
repo, nil, nil, nil, nil, mcpTokens, nil,
acp.SupervisorConfig{SpawnTimeout: 5 * time.Second, KillGrace: 1 * time.Second},
nil,
)
svc := acp.NewSessionService(
repo, wsSvc, nil, nil, pa, sup, nil,
acp.SessionServiceConfig{
MaxActiveGlobal: 10,
MaxActivePerUser: 5,
SystemTokenTTL: 24 * time.Hour,
MCPPublicURL: "http://localhost:8080/mcp",
},
mcpTokens,
)
branch := "main"
sess, err := svc.Create(context.Background(), acp.Caller{UserID: uid}, acp.CreateSessionInput{
WorkspaceID: wsID,
AgentKindID: akID,
Branch: &branch,
})
require.NoError(t, err)
require.NotNil(t, sess)
// async spawn 失败后:session 被守卫式标记为 crashed 且 last_error 非空
require.Eventually(t, func() bool {
repo.mu.Lock()
defer repo.mu.Unlock()
for _, s := range repo.sessions {
if s.ID == sess.ID {
return s.Status == acp.SessionCrashed && s.LastError != nil && *s.LastError != ""
}
}
return false
}, 5*time.Second, 50*time.Millisecond, "session should be marked crashed with non-empty last_error")
}
func TestSessionService_Create_MutuallyExclusiveInputs(t *testing.T) {
t.Parallel()
uid := uuid.New()
wsID := uuid.New()
akID := uuid.New()
svc := acp.NewSessionService(
&fakeAcpRepo{}, nil, nil, nil, nil, nil, nil,
acp.SessionServiceConfig{}, nil,
)
branch := "feat/x"
issueNum := 42
reqNum := 7
cases := []struct {
name string
in acp.CreateSessionInput
}{
{"branch+issue", acp.CreateSessionInput{Branch: &branch, IssueNumber: &issueNum}},
{"branch+requirement", acp.CreateSessionInput{Branch: &branch, RequirementNumber: &reqNum}},
{"issue+requirement", acp.CreateSessionInput{IssueNumber: &issueNum, RequirementNumber: &reqNum}},
{"all three", acp.CreateSessionInput{Branch: &branch, IssueNumber: &issueNum, RequirementNumber: &reqNum}},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
tc.in.WorkspaceID = wsID
tc.in.AgentKindID = akID
_, err := svc.Create(context.Background(), acp.Caller{UserID: uid}, tc.in)
require.Error(t, err)
assert.Contains(t, err.Error(), "mutually exclusive")
})
}
}
// ===== PermissionRequest(权限审批框架,测试桩) =====
func (f *fakeAcpRepo) InsertPermissionRequest(context.Context, *acp.PermissionRequest) (*acp.PermissionRequest, error) {
return &acp.PermissionRequest{}, nil
+19
View File
@@ -250,6 +250,25 @@ func (q *Queries) ListSessionsByUser(ctx context.Context, arg ListSessionsByUser
return items, nil
}
const markSessionFailedIfActive = `-- name: MarkSessionFailedIfActive :execrows
UPDATE acp_sessions
SET status = 'crashed', last_error = $2, ended_at = now()
WHERE id = $1 AND status IN ('starting', 'running')
`
type MarkSessionFailedIfActiveParams struct {
ID pgtype.UUID `json:"id"`
LastError *string `json:"last_error"`
}
func (q *Queries) MarkSessionFailedIfActive(ctx context.Context, arg MarkSessionFailedIfActiveParams) (int64, error) {
result, err := q.db.Exec(ctx, markSessionFailedIfActive, arg.ID, arg.LastError)
if err != nil {
return 0, err
}
return result.RowsAffected(), nil
}
const releaseMainWorktree = `-- name: ReleaseMainWorktree :execrows
UPDATE workspaces SET active_main_session_id = NULL
WHERE id = $1 AND active_main_session_id = $2
+8 -9
View File
@@ -92,8 +92,7 @@ type Process struct {
SessionID uuid.UUID
WorkspaceID uuid.UUID
UserID uuid.UUID
IsMain bool // 主 worktree 占用 → release 走不同路径
WorktreeID *uuid.UUID // 子 worktree 时填,用于 release
IsMain bool // 主 worktree 占用 → release 走不同路径
Cmd *exec.Cmd
group procgrp.Group // 进程树句柄(整树终止),Spawn 时 Prepare+Adopt
@@ -198,6 +197,7 @@ func (s *Supervisor) Spawn(ctx context.Context, sess *Session, kind *AgentKind,
}
if err := group.Adopt(cmd); err != nil {
_ = cmd.Process.Kill()
_ = cmd.Wait() // 回收进程资源,避免 Unix 僵尸进程
return nil, fmt.Errorf("adopt process group: %w", err)
}
@@ -411,19 +411,18 @@ func (s *Supervisor) onExit(ctx context.Context, proc *Process, status SessionSt
s.log.Error("acp.on_exit.update_session", "session_id", proc.SessionID, "err", err.Error())
}
// Release worktree。主 worktree 走 acp 表自己的 CAS;子 worktree 走 workspace
// service 的 holder 校验。当前 wtSvc.Release 接受 workspace.Caller,子 worktree
// 释放需要绕过 holder 检查(spec 期望 holder = "session:<sid>",与 user holder
// 不同),用 IsAdmin: true 强制释放。F4 reaper 会替换为更严格的 byHolder 调用。
// Release worktree。主 worktree 走 acp 表自己的 CAS;子 worktree 按 holder
// 释放(holder = "session:<sid>",见 workspace.Caller.Holder),与启动 reaper
// 一致,不需要知道 worktree ID。
if proc.IsMain {
if _, err := s.repo.ReleaseMainWorktree(ctx, proc.WorkspaceID, proc.SessionID); err != nil {
s.log.Error("acp.on_exit.release_main",
"session_id", proc.SessionID, "err", err.Error())
}
} else if proc.WorktreeID != nil && s.wtSvc != nil {
if _, err := s.wtSvc.Release(ctx, workspace.Caller{IsAdmin: true}, *proc.WorktreeID); err != nil {
} else if s.wtSvc != nil {
if _, err := s.wtSvc.ReleaseByHolder(ctx, "session:"+proc.SessionID.String()); err != nil {
s.log.Error("acp.on_exit.release_worktree",
"session_id", proc.SessionID, "worktree_id", *proc.WorktreeID, "err", err.Error())
"session_id", proc.SessionID, "err", err.Error())
}
}
+90 -5
View File
@@ -12,6 +12,7 @@ package acp_test
import (
"context"
"encoding/json"
"sync"
"testing"
"time"
@@ -25,12 +26,13 @@ import (
"github.com/yan1h/agent-coding-workflow/internal/audit"
"github.com/yan1h/agent-coding-workflow/internal/infra/notify"
"github.com/yan1h/agent-coding-workflow/internal/mcp"
"github.com/yan1h/agent-coding-workflow/internal/workspace"
)
// newSupervisorForTest 构造一个绑定到 PG 真实 audit Recorder 的 Supervisor,
// 使用静音 logger + 默认 SupervisorConfig(5s SpawnTimeout 给 fake agent 留 buffer)。
// withAudit=false 时 Recorder 为 nil,对应 onExit 跳过 audit 路径。
func newSupervisorForTest(t *testing.T, pool *pgxpool.Pool, repo acp.Repository, withAudit bool, mcpTokens mcp.TokenService) *acp.Supervisor {
func newSupervisorForTest(t *testing.T, pool *pgxpool.Pool, repo acp.Repository, withAudit bool, mcpTokens mcp.TokenService, wtSvc workspace.WorktreeService) *acp.Supervisor {
t.Helper()
var rec audit.Recorder
if withAudit {
@@ -39,7 +41,7 @@ func newSupervisorForTest(t *testing.T, pool *pgxpool.Pool, repo acp.Repository,
disp := notify.NewDispatcher(notifyRepoStub{}, slogDiscard())
return acp.NewSupervisor(
repo, rec, disp,
nil, nil, // wtSvc / wsRepo: tests 用 IsMain=true,走 repo.ReleaseMainWorktree
wtSvc, nil, // wtSvc: IsMain=false 测试注入 fake;wsRepo: 测试不走该路径
mcpTokens,
testEncryptor(t),
acp.SupervisorConfig{
@@ -88,7 +90,7 @@ func TestSupervisor_HappyPath_Spawn_Handshake(t *testing.T) {
require.NoError(t, err)
require.True(t, ok)
sup := newSupervisorForTest(t, pool, repo, true, nil)
sup := newSupervisorForTest(t, pool, repo, true, nil, nil)
proc, err := sup.Spawn(ctx, sess, k, nil)
require.NoError(t, err)
require.NotNil(t, proc)
@@ -217,7 +219,7 @@ func TestSupervisor_AgentCrashes(t *testing.T) {
require.NoError(t, err)
require.True(t, ok)
sup := newSupervisorForTest(t, pool, repo, true, nil)
sup := newSupervisorForTest(t, pool, repo, true, nil, nil)
proc, err := sup.Spawn(ctx, sess, k, nil)
require.NoError(t, err)
require.NotNil(t, proc)
@@ -306,7 +308,7 @@ func TestSupervisor_OnExit_RevokesMCPToken(t *testing.T) {
require.True(t, ok)
fakeTokens := &fakeSupMCPTokens{}
sup := newSupervisorForTest(t, pool, repo, true, fakeTokens)
sup := newSupervisorForTest(t, pool, repo, true, fakeTokens, nil)
proc, err := sup.Spawn(ctx, sess, k, nil)
require.NoError(t, err)
require.NotNil(t, proc)
@@ -332,3 +334,86 @@ func TestSupervisor_OnExit_RevokesMCPToken(t *testing.T) {
require.Len(t, fakeTokens.revoked, 1, "expected exactly one RevokeBySession call")
assert.Equal(t, sess.ID, fakeTokens.revoked[0], "RevokeBySession should receive the session ID")
}
// fakeWorktreeSvc 实现 workspace.WorktreeService,仅记录 ReleaseByHolder 收到的
// holder 字符串,用于验证 onExit 的子 worktree 释放路径。
type fakeWorktreeSvc struct {
mu sync.Mutex
holders []string
}
func (f *fakeWorktreeSvc) Create(_ context.Context, _ workspace.Caller, _ uuid.UUID, _ workspace.CreateWorktreeInput) (*workspace.Worktree, error) {
return nil, nil
}
func (f *fakeWorktreeSvc) List(_ context.Context, _ workspace.Caller, _ uuid.UUID) ([]*workspace.Worktree, error) {
return nil, nil
}
func (f *fakeWorktreeSvc) Delete(_ context.Context, _ workspace.Caller, _ uuid.UUID) error {
return nil
}
func (f *fakeWorktreeSvc) Acquire(_ context.Context, _ workspace.Caller, _ uuid.UUID) (*workspace.Worktree, error) {
return nil, nil
}
func (f *fakeWorktreeSvc) Release(_ context.Context, _ workspace.Caller, _ uuid.UUID) (*workspace.Worktree, error) {
return nil, nil
}
func (f *fakeWorktreeSvc) ReleaseByHolder(_ context.Context, holder string) ([]*workspace.Worktree, error) {
f.mu.Lock()
defer f.mu.Unlock()
f.holders = append(f.holders, holder)
return nil, nil
}
// holdersSnapshot 拷贝当前已记录的 holder 列表(onExit 在 monitor goroutine 中调用)。
func (f *fakeWorktreeSvc) holdersSnapshot() []string {
f.mu.Lock()
defer f.mu.Unlock()
return append([]string(nil), f.holders...)
}
// TestSupervisor_OnExit_ReleasesSubWorktree 验证:IsMainWorktree=false 的 session
// 退出后,onExit 通过 wtSvc.ReleaseByHolder("session:<sid>") 释放子 worktree。
func TestSupervisor_OnExit_ReleasesSubWorktree(t *testing.T) {
t.Parallel()
ctx := context.Background()
repo, pool := setupRepo(t)
uid := mustInsertUser(t, ctx, pool, "sup-wt@local", false)
pid, wsid := mustInsertProjectWorkspace(t, ctx, pool, uid)
bin := fakeAgentBinary(t)
k, err := repo.CreateAgentKind(ctx, &acp.AgentKind{
ID: uuid.New(), Name: "fake-wt", DisplayName: "Fake",
BinaryPath: bin, Args: []string{},
Enabled: true, CreatedBy: uid,
})
require.NoError(t, err)
sess, err := repo.InsertSession(ctx, &acp.Session{
ID: uuid.New(), WorkspaceID: wsid, ProjectID: pid,
AgentKindID: k.ID, UserID: uid,
Branch: "feat-x", CwdPath: t.TempDir(), IsMainWorktree: false, Status: acp.SessionStarting,
})
require.NoError(t, err)
fakeWt := &fakeWorktreeSvc{}
sup := newSupervisorForTest(t, pool, repo, true, nil, fakeWt)
proc, err := sup.Spawn(ctx, sess, k, nil)
require.NoError(t, err)
require.NotNil(t, proc)
sup.Kill(ctx, sess.ID, 1*time.Second)
// onExit 在 monitor goroutine 中异步执行;轮询 fake 直到记录到 release 调用。
deadline := time.After(5 * time.Second)
for len(fakeWt.holdersSnapshot()) == 0 {
select {
case <-deadline:
t.Fatal("ReleaseByHolder never called after process exit")
case <-time.After(50 * time.Millisecond):
}
}
holders := fakeWt.holdersSnapshot()
require.Len(t, holders, 1, "expected exactly one ReleaseByHolder call")
assert.Equal(t, "session:"+sess.ID.String(), holders[0])
}
+41 -18
View File
@@ -2,7 +2,6 @@ package mcp
import (
"context"
"fmt"
"github.com/google/uuid"
mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp"
@@ -126,6 +125,27 @@ func resolveMountRefs(ctx context.Context, deps ServerDeps, c project.Caller,
return refs, nil
}
// scopedConversation 解析 conversation_id、取会话(service 层含 ownership 校验)
// 并做项目 scope 校验:ProjectID 为 nil 的个人会话不受 project scope 约束。
// limit 控制随会话返回的最近消息条数,仅 get_conversation 需要,其余入口传 1。
func scopedConversation(ctx context.Context, deps ServerDeps, userID uuid.UUID,
conversationID string, limit int) (*chat.Conversation, []chat.Message, error) {
convID, err := uuid.Parse(conversationID)
if err != nil {
return nil, nil, errs.Wrap(err, errs.CodeInvalidInput, "conversation_id parse")
}
cv, msgs, err := deps.Conversations.Get(ctx, userID, convID, limit)
if err != nil {
return nil, nil, err
}
if cv.ProjectID != nil {
if err := CheckProjectFromCtx(ctx, *cv.ProjectID); err != nil {
return nil, nil, err
}
}
return cv, msgs, nil
}
// ===== list_recent_messages =====
type listRecentMessagesIn struct {
@@ -166,9 +186,9 @@ func registerListRecentMessages(srv *mcpsdk.Server, deps ServerDeps) {
return nil, listRecentMessagesOut{}, err
}
convID, err := uuid.Parse(in.ConversationID)
cv, _, err := scopedConversation(ctx, deps, c.UserID, in.ConversationID, 1)
if err != nil {
return nil, listRecentMessagesOut{}, fmt.Errorf("invalid conversation_id: %w", err)
return nil, listRecentMessagesOut{}, err
}
limit := in.Limit
@@ -179,7 +199,7 @@ func registerListRecentMessages(srv *mcpsdk.Server, deps ServerDeps) {
limit = 200
}
msgs, err := deps.Messages.ListMessages(ctx, c.UserID, convID, in.BeforeID, limit)
msgs, err := deps.Messages.ListMessages(ctx, c.UserID, cv.ID, in.BeforeID, limit)
if err != nil {
return nil, listRecentMessagesOut{}, err
}
@@ -248,6 +268,13 @@ func registerListConversations(srv *mcpsdk.Server, deps ServerDeps) {
}
out := listConversationsOut{Conversations: make([]conversationDetail, 0, len(cvs))}
for i := range cvs {
// 与 list_acp_sessions 一致:scope 外项目挂载的会话静默跳过;
// ProjectID 为 nil 的个人会话不受 project scope 约束。
if cvs[i].ProjectID != nil {
if err := CheckProjectFromCtx(ctx, *cvs[i].ProjectID); err != nil {
continue
}
}
out.Conversations = append(out.Conversations, conversationDetailFrom(&cvs[i]))
}
return nil, out, nil
@@ -276,10 +303,6 @@ func registerGetConversation(srv *mcpsdk.Server, deps ServerDeps) {
if err != nil {
return nil, getConversationOut{}, err
}
convID, err := uuid.Parse(in.ConversationID)
if err != nil {
return nil, getConversationOut{}, errs.Wrap(err, errs.CodeInvalidInput, "conversation_id parse")
}
limit := in.Limit
if limit <= 0 {
limit = 50
@@ -287,7 +310,7 @@ func registerGetConversation(srv *mcpsdk.Server, deps ServerDeps) {
if limit > 200 {
limit = 200
}
cv, msgs, err := deps.Conversations.Get(ctx, c.UserID, convID, limit)
cv, msgs, err := scopedConversation(ctx, deps, c.UserID, in.ConversationID, limit)
if err != nil {
return nil, getConversationOut{}, err
}
@@ -378,11 +401,11 @@ func registerUpdateConversationTitle(srv *mcpsdk.Server, deps ServerDeps) {
if err != nil {
return nil, okOut{}, err
}
convID, err := uuid.Parse(in.ConversationID)
cv, _, err := scopedConversation(ctx, deps, c.UserID, in.ConversationID, 1)
if err != nil {
return nil, okOut{}, errs.Wrap(err, errs.CodeInvalidInput, "conversation_id parse")
return nil, okOut{}, err
}
if err := deps.Conversations.UpdateTitle(ctx, c.UserID, convID, in.Title); err != nil {
if err := deps.Conversations.UpdateTitle(ctx, c.UserID, cv.ID, in.Title); err != nil {
return nil, okOut{}, err
}
return nil, okOut{OK: true}, nil
@@ -406,11 +429,11 @@ func registerDeleteConversation(srv *mcpsdk.Server, deps ServerDeps) {
if err != nil {
return nil, okOut{}, err
}
convID, err := uuid.Parse(in.ConversationID)
cv, _, err := scopedConversation(ctx, deps, c.UserID, in.ConversationID, 1)
if err != nil {
return nil, okOut{}, errs.Wrap(err, errs.CodeInvalidInput, "conversation_id parse")
return nil, okOut{}, err
}
if err := deps.Conversations.SoftDelete(ctx, c.UserID, convID); err != nil {
if err := deps.Conversations.SoftDelete(ctx, c.UserID, cv.ID); err != nil {
return nil, okOut{}, err
}
return nil, okOut{OK: true}, nil
@@ -440,11 +463,11 @@ func registerSendMessage(srv *mcpsdk.Server, deps ServerDeps) {
if err != nil {
return nil, sendMessageOut{}, err
}
convID, err := uuid.Parse(in.ConversationID)
cv, _, err := scopedConversation(ctx, deps, c.UserID, in.ConversationID, 1)
if err != nil {
return nil, sendMessageOut{}, errs.Wrap(err, errs.CodeInvalidInput, "conversation_id parse")
return nil, sendMessageOut{}, err
}
userMsgID, assistantMsgID, err := deps.Messages.Send(ctx, c.UserID, convID, in.Content, nil)
userMsgID, assistantMsgID, err := deps.Messages.Send(ctx, c.UserID, cv.ID, in.Content, nil)
if err != nil {
return nil, sendMessageOut{}, err
}
+69 -2
View File
@@ -281,9 +281,11 @@ func TestUpdateTitleAndDeleteConversation(t *testing.T) {
}
func TestSendMessage(t *testing.T) {
deps, _, msgSvc, _ := chatTestDeps()
deps, convSvc, msgSvc, _ := chatTestDeps()
cv := chat.Conversation{ID: uuid.New(), UserID: testUID, ModelID: uuid.New(), CreatedAt: time.Now(), UpdatedAt: time.Now()}
convSvc.items = []chat.Conversation{cv}
result, err := callTool(ctxWithAuth(Scope{}), deps, "send_message", map[string]any{
"conversation_id": uuid.New().String(), "content": "hello",
"conversation_id": cv.ID.String(), "content": "hello",
})
require.NoError(t, err)
require.False(t, result.IsError)
@@ -294,6 +296,71 @@ func TestSendMessage(t *testing.T) {
assert.Equal(t, "hello", msgSvc.sentContent)
}
// TestConversationTools_ScopeDenied 受限 token 直访 scope 外 project 挂载的会话 → 全部拒绝。
func TestConversationTools_ScopeDenied(t *testing.T) {
deps, convSvc, _, _ := chatTestDeps()
projB := uuid.New()
cvB := chat.Conversation{ID: uuid.New(), UserID: testUID, ModelID: uuid.New(), ProjectID: &projB, CreatedAt: time.Now(), UpdatedAt: time.Now()}
convSvc.items = []chat.Conversation{cvB}
// token 限定到 testPID,cvB 挂载在 projB 上 → 拒绝
ctx := ctxWithAuth(Scope{ProjectIDs: []uuid.UUID{testPID}})
cases := []struct {
tool string
args map[string]any
}{
{"list_recent_messages", map[string]any{"conversation_id": cvB.ID.String()}},
{"get_conversation", map[string]any{"conversation_id": cvB.ID.String()}},
{"update_conversation_title", map[string]any{"conversation_id": cvB.ID.String(), "title": "t"}},
{"delete_conversation", map[string]any{"conversation_id": cvB.ID.String()}},
{"send_message", map[string]any{"conversation_id": cvB.ID.String(), "content": "hi"}},
}
for _, tc := range cases {
result, err := callTool(ctx, deps, tc.tool, tc.args)
require.NoError(t, err, tc.tool)
assert.True(t, result.IsError, tc.tool)
}
// 被拒绝的 delete/update 不应实际生效
require.Len(t, convSvc.items, 1)
assert.Nil(t, convSvc.items[0].Title)
}
// TestConversationTools_PersonalNotScoped ProjectID 为 nil 的个人会话不受 project scope 约束。
func TestConversationTools_PersonalNotScoped(t *testing.T) {
deps, convSvc, _, _ := chatTestDeps()
cv := chat.Conversation{ID: uuid.New(), UserID: testUID, ModelID: uuid.New(), CreatedAt: time.Now(), UpdatedAt: time.Now()}
convSvc.items = []chat.Conversation{cv}
result, err := callTool(ctxWithAuth(Scope{ProjectIDs: []uuid.UUID{uuid.New()}}), deps,
"get_conversation", map[string]any{"conversation_id": cv.ID.String()})
require.NoError(t, err)
assert.False(t, result.IsError)
}
// TestListConversations_ScopeFiltered 无 project filter 时静默剔除 scope 外项目挂载的会话。
func TestListConversations_ScopeFiltered(t *testing.T) {
deps, convSvc, _, _ := chatTestDeps()
pidA := testPID
projB := uuid.New()
convSvc.items = []chat.Conversation{
{ID: uuid.New(), UserID: testUID, ModelID: uuid.New(), ProjectID: &pidA, CreatedAt: time.Now(), UpdatedAt: time.Now()},
{ID: uuid.New(), UserID: testUID, ModelID: uuid.New(), ProjectID: &projB, CreatedAt: time.Now(), UpdatedAt: time.Now()},
{ID: uuid.New(), UserID: testUID, ModelID: uuid.New(), CreatedAt: time.Now(), UpdatedAt: time.Now()},
}
result, err := callTool(ctxWithAuth(Scope{ProjectIDs: []uuid.UUID{pidA}}), deps,
"list_conversations", map[string]any{})
require.NoError(t, err)
require.False(t, result.IsError)
var out listConversationsOut
unmarshalStructured(t, result, &out)
// projA 挂载 + 无挂载的保留,projB 挂载的被剔除
require.Len(t, out.Conversations, 2)
for _, cv := range out.Conversations {
assert.NotEqual(t, projB.String(), cv.ProjectID)
}
}
func TestPromptTemplateCRUD(t *testing.T) {
deps, _, _, tplSvc := chatTestDeps()