From 0561e126cd981446b527a5bd2e1bc03485c771b9 Mon Sep 17 00:00:00 2001 From: Jerry Yan <792602257@qq.com> Date: Thu, 11 Jun 2026 12:43:18 +0800 Subject: [PATCH] =?UTF-8?q?ACP=20/=20MCP=20=E5=AE=8C=E5=96=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/acp/agentkind_service_test.go | 3 + internal/acp/domain.go | 2 +- internal/acp/handlers/server_fs_read.go | 37 +-- internal/acp/handlers/server_fs_write_test.go | 25 ++ internal/acp/pathsafe/pathsafe.go | 91 +++++++ internal/acp/permission.go | 49 +--- internal/acp/permission_service.go | 11 +- .../acp/permission_service_internal_test.go | 28 +++ internal/acp/permission_service_test.go | 45 ++++ internal/acp/permission_test.go | 31 +++ internal/acp/queries/sessions.sql | 5 + internal/acp/relay_test.go | 3 + internal/acp/repository.go | 14 ++ internal/acp/session_service.go | 23 ++ internal/acp/session_service_test.go | 108 ++++++++ internal/acp/sqlc/sessions.sql.go | 19 ++ internal/acp/supervisor.go | 17 +- internal/acp/supervisor_test.go | 95 ++++++- internal/mcp/tools_chat.go | 59 +++-- internal/mcp/tools_chat_test.go | 71 +++++- .../views/acp/AcpSessionCreateView.test.ts | 34 ++- web/src/views/acp/AcpSessionCreateView.vue | 235 ++++++++++-------- 22 files changed, 786 insertions(+), 219 deletions(-) create mode 100644 internal/acp/pathsafe/pathsafe.go create mode 100644 internal/acp/permission_service_internal_test.go diff --git a/internal/acp/agentkind_service_test.go b/internal/acp/agentkind_service_test.go index 6df33f8..34cbbe6 100644 --- a/internal/acp/agentkind_service_test.go +++ b/internal/acp/agentkind_service_test.go @@ -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") } diff --git a/internal/acp/domain.go b/internal/acp/domain.go index a2ef47d..3cd5f4b 100644 --- a/internal/acp/domain.go +++ b/internal/acp/domain.go @@ -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 diff --git a/internal/acp/handlers/server_fs_read.go b/internal/acp/handlers/server_fs_read.go index a5de71d..82b818b 100644 --- a/internal/acp/handlers/server_fs_read.go +++ b/internal/acp/handlers/server_fs_read.go @@ -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) } diff --git a/internal/acp/handlers/server_fs_write_test.go b/internal/acp/handlers/server_fs_write_test.go index 8d3668d..d277267 100644 --- a/internal/acp/handlers/server_fs_write_test.go +++ b/internal/acp/handlers/server_fs_write_test.go @@ -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), "外部目录不应被写入") +} diff --git a/internal/acp/pathsafe/pathsafe.go b/internal/acp/pathsafe/pathsafe.go new file mode 100644 index 0000000..db5fa11 --- /dev/null +++ b/internal/acp/pathsafe/pathsafe.go @@ -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) +} diff --git a/internal/acp/permission.go b/internal/acp/permission.go index 6c91268..d62816a 100644 --- a/internal/acp/permission.go +++ b/internal/acp/permission.go @@ -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) } diff --git a/internal/acp/permission_service.go b/internal/acp/permission_service.go index f6cf3cf..11ce8b3 100644 --- a/internal/acp/permission_service.go +++ b/internal/acp/permission_service.go @@ -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 } } diff --git a/internal/acp/permission_service_internal_test.go b/internal/acp/permission_service_internal_test.go new file mode 100644 index 0000000..d854790 --- /dev/null +++ b/internal/acp/permission_service_internal_test.go @@ -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) + } + }) + } +} diff --git a/internal/acp/permission_service_test.go b/internal/acp/permission_service_test.go index add3735..d7b2ed2 100644 --- a/internal/acp/permission_service_test.go +++ b/internal/acp/permission_service_test.go @@ -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() diff --git a/internal/acp/permission_test.go b/internal/acp/permission_test.go index 6dcf17f..a2b2308 100644 --- a/internal/acp/permission_test.go +++ b/internal/acp/permission_test.go @@ -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) { diff --git a/internal/acp/queries/sessions.sql b/internal/acp/queries/sessions.sql index 1590eb3..7815c4c 100644 --- a/internal/acp/queries/sessions.sql +++ b/internal/acp/queries/sessions.sql @@ -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'); diff --git a/internal/acp/relay_test.go b/internal/acp/relay_test.go index 13619a7..c00b67b 100644 --- a/internal/acp/relay_test.go +++ b/internal/acp/relay_test.go @@ -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") diff --git a/internal/acp/repository.go b/internal/acp/repository.go index 59713de..462696e 100644 --- a/internal/acp/repository.go +++ b/internal/acp/repository.go @@ -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 { diff --git a/internal/acp/session_service.go b/internal/acp/session_service.go index 172c5e3..50f4bbd 100644 --- a/internal/acp/session_service.go +++ b/internal/acp/session_service.go @@ -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) } diff --git a/internal/acp/session_service_test.go b/internal/acp/session_service_test.go index ea2d2ef..e27372e 100644 --- a/internal/acp/session_service_test.go +++ b/internal/acp/session_service_test.go @@ -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 diff --git a/internal/acp/sqlc/sessions.sql.go b/internal/acp/sqlc/sessions.sql.go index 2fa6625..3ca1427 100644 --- a/internal/acp/sqlc/sessions.sql.go +++ b/internal/acp/sqlc/sessions.sql.go @@ -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 diff --git a/internal/acp/supervisor.go b/internal/acp/supervisor.go index 6e76acc..b176125 100644 --- a/internal/acp/supervisor.go +++ b/internal/acp/supervisor.go @@ -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:"，与 user holder - // 不同），用 IsAdmin: true 强制释放。F4 reaper 会替换为更严格的 byHolder 调用。 + // Release worktree。主 worktree 走 acp 表自己的 CAS；子 worktree 按 holder + // 释放（holder = "session:"，见 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()) } } diff --git a/internal/acp/supervisor_test.go b/internal/acp/supervisor_test.go index 3979ced..def7964 100644 --- a/internal/acp/supervisor_test.go +++ b/internal/acp/supervisor_test.go @@ -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:") 释放子 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]) +} diff --git a/internal/mcp/tools_chat.go b/internal/mcp/tools_chat.go index 9ce9fa9..174d3cd 100644 --- a/internal/mcp/tools_chat.go +++ b/internal/mcp/tools_chat.go @@ -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 } diff --git a/internal/mcp/tools_chat_test.go b/internal/mcp/tools_chat_test.go index c66d564..941def0 100644 --- a/internal/mcp/tools_chat_test.go +++ b/internal/mcp/tools_chat_test.go @@ -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() diff --git a/web/src/views/acp/AcpSessionCreateView.test.ts b/web/src/views/acp/AcpSessionCreateView.test.ts index 6543495..0cc34fa 100644 --- a/web/src/views/acp/AcpSessionCreateView.test.ts +++ b/web/src/views/acp/AcpSessionCreateView.test.ts @@ -67,15 +67,37 @@ describe('AcpSessionCreateView', () => { it('mounts without error', async () => { const wrapper = mount(AcpSessionCreateView) await new Promise((r) => setTimeout(r, 20)) - expect(wrapper.html()).toContain('New ACP Session') + expect(wrapper.html()).toContain('新建 ACP 会话') }) - it('prefills requirement/issue number from query', async () => { - routeMock.query = { requirement_number: '7', issue_number: '42' } + it('prefills issue number from query', async () => { + routeMock.query = { issue_number: '42' } const wrapper = mount(AcpSessionCreateView) await new Promise((r) => setTimeout(r, 20)) - const inputs = wrapper.findAll('input[type="number"]') - expect((inputs[0].element as HTMLInputElement).value).toBe('42') - expect((inputs[1].element as HTMLInputElement).value).toBe('7') + const issueInput = wrapper.find('[data-testid="issue-number-input"] input') + const reqInput = wrapper.find('[data-testid="requirement-number-input"] input') + expect((issueInput.element as HTMLInputElement).value).toBe('42') + expect((reqInput.element as HTMLInputElement).value).toBe('') + }) + + it('prefills requirement number from query', async () => { + routeMock.query = { requirement_number: '7' } + const wrapper = mount(AcpSessionCreateView) + await new Promise((r) => setTimeout(r, 20)) + const issueInput = wrapper.find('[data-testid="issue-number-input"] input') + const reqInput = wrapper.find('[data-testid="requirement-number-input"] input') + expect((issueInput.element as HTMLInputElement).value).toBe('') + expect((reqInput.element as HTMLInputElement).value).toBe('7') + }) + + it('rejects submit when issue # and requirement # are both filled', async () => { + const wrapper = mount(AcpSessionCreateView) + await new Promise((r) => setTimeout(r, 20)) + const issueInput = wrapper.find('[data-testid="issue-number-input"] input') + const reqInput = wrapper.find('[data-testid="requirement-number-input"] input') + await issueInput.setValue('42') + await reqInput.setValue('7') + await wrapper.find('form').trigger('submit.prevent') + expect(wrapper.text()).toContain('最多只能填写一项') }) }) diff --git a/web/src/views/acp/AcpSessionCreateView.vue b/web/src/views/acp/AcpSessionCreateView.vue index f6a7355..56602d0 100644 --- a/web/src/views/acp/AcpSessionCreateView.vue +++ b/web/src/views/acp/AcpSessionCreateView.vue @@ -1,6 +1,17 @@