【架构优化】合并 prepareWorkbenchContext 中 6 处重复的会话读取为 storedAssistantSession

internal/httpapi/assistant_shared.go

@@ -23,6 +23,16 @@ func assistantStorageOwner(st *repo.Store, id string, fallback int64) int64 {
 	return fallback
 }
 
+// storedAssistantSession reads a conversation through its storage owner: shared
+// and project sessions live under the user who opened them, so the caller's own
+// slot only holds private conversations.
+func (h *assistantHandlers) storedAssistantSession(id string, userID int64) (domain.AssistantSession, bool) {
+	if id == "" {
+		return domain.AssistantSession{}, false
+	}
+	return h.repo.AssistantSession(assistantStorageOwner(h.repo, id, userID), id)
+}
+
 // conversationScope returns the storage owner and whether the conversation is
 // multi-user. SOP run sessions and every conversation bound to a project are
 // shared with that project's members; only unbound conversations are private.

internal/httpapi/assistant_workbench.go

@@ -181,43 +181,50 @@ var errForeignReference = fmt.Errorf("只能引用当前项目的需求、角色
 
 var errNotProjectMember = fmt.Errorf("你还不是这个项目的成员，不能在项目里对话。可以把项目切换为「个人对话」直接聊，或请项目 Owner 把你加入项目")
 
+// historicalAttachmentIDs collects attachments already stored in this project's
+// conversation. Existing personal attachments remain usable when an unassigned
+// conversation is explicitly bound to an authorized project; never infer
+// another user's files.
+func historicalAttachmentIDs(session domain.AssistantSession, found bool, projectID string) map[string]bool {
+	ids := map[string]bool{}
+	if !found || session.Shared || session.ProjectID != projectID {
+		return ids
+	}
+	for _, message := range session.Messages {
+		for _, attachment := range message.Attachments {
+			ids[attachment.ID] = true
+		}
+	}
+	return ids
+}
+
 func (h *assistantHandlers) prepareWorkbenchContext(c *gin.Context, req *assistantRequest, user domain.User) error {
 	if !h.workbenchProjectAllowed(c, req.ProjectID, user.ID) {
 		return errNotProjectMember
 	}
-	if req.SessionID != "" {
-		if old, ok := h.repo.AssistantSession(assistantStorageOwner(h.repo, req.SessionID, user.ID), req.SessionID); ok && old.ProjectID != req.ProjectID {
-			return fmt.Errorf("会话属于其他项目，请新建会话")
-		}
-	}
-	// Existing personal attachments remain usable when an unassigned conversation
-	// is explicitly bound to an authorized project. Never infer another user's files.
-	historicalAttachments := map[string]bool{}
-	if old, ok := h.repo.AssistantSession(assistantStorageOwner(h.repo, req.SessionID, user.ID), req.SessionID); ok && !old.Shared && old.ProjectID == req.ProjectID {
-		for _, message := range old.Messages {
-			for _, attachment := range message.Attachments {
-				historicalAttachments[attachment.ID] = true
-			}
-		}
+	stored, hasStored := h.storedAssistantSession(req.SessionID, user.ID)
+	if hasStored && stored.ProjectID != req.ProjectID {
+		return fmt.Errorf("会话属于其他项目，请新建会话")
 	}
+	historicalAttachments := historicalAttachmentIDs(stored, hasStored, req.ProjectID)
 	req.attachmentIDs = map[string]bool{}
 	req.attachmentOwners = map[string]int64{}
 	if len(req.Messages) > 200 {
 		return fmt.Errorf("会话过长，请开启新会话")
 	}
 	var context strings.Builder
-	if s, ok := h.repo.AssistantSession(assistantStorageOwner(h.repo, req.SessionID, user.ID), req.SessionID); ok && s.IssueID != "" && req.RunID == "" {
-		if err := h.validateSessionIssue(c, req.ProjectID, s.IssueID, user.ID); err != nil {
+	if hasStored && stored.IssueID != "" && req.RunID == "" {
+		if err := h.validateSessionIssue(c, req.ProjectID, stored.IssueID, user.ID); err != nil {
 			return err
 		}
-		_, data, err := h.workbenchReference(domain.AssistantReference{Type: "issue", ID: s.IssueID}, req.ProjectID)
+		_, data, err := h.workbenchReference(domain.AssistantReference{Type: "issue", ID: stored.IssueID}, req.ProjectID)
 		if err != nil {
 			return fmt.Errorf("关联需求不可用，请更换或解除关联：%w", err)
 		}
 		context.WriteString("当前会话关联的主需求（每轮从平台读取的最新数据）：" + data + "\n这是持续的需求上下文；其他 @需求只供本条消息参考。关联不会自动创建或派发运行，也不会改变会话可见范围。\n")
 	}
 	context.WriteString("当前任务以 runId 为准；普通会话引用只提供上下文。@角色表示让该角色作为子 Agent 来做这件事。\n")
-	if branch, ok := h.repo.AssistantSession(assistantStorageOwner(h.repo, req.SessionID, user.ID), req.SessionID); ok && branch.ParentSessionID != "" {
+	if hasStored && stored.ParentSessionID != "" {
 		context.WriteString("这是编辑消息后的讨论分支。原会话后续执行过的文件修改和平台变更不会回退，可能已经存在；写入前先读取真实状态，避免重复导入、创建或派发。历史 inherited 草稿仅供参考，不可认为它们在当前分支重新执行。\n")
 	}
 	if req.RunID != "" {
@@ -314,7 +321,7 @@ func (h *assistantHandlers) prepareWorkbenchContext(c *gin.Context, req *assista
 		}
 		for j, a := range m.Attachments {
 			attachmentOwner := user.ID
-			if stored, ok := h.repo.AssistantSession(assistantStorageOwner(h.repo, req.SessionID, user.ID), req.SessionID); ok && i < len(stored.Messages) {
+			if hasStored && i < len(stored.Messages) {
 				attachmentOwner = stored.Messages[i].AuthorID
 				if attachmentOwner == 0 {
 					attachmentOwner = stored.UserID
@@ -347,7 +354,7 @@ func (h *assistantHandlers) prepareWorkbenchContext(c *gin.Context, req *assista
 	context.WriteString(delegationContext(req, prefs, normalizeAssistantApprovalMode(req.ApprovalMode), hasSOP))
 	context.WriteString(assistantInstructionsContext(prefs))
 	if len(req.roles) > 0 {
-		if stored, ok := h.repo.AssistantSession(assistantStorageOwner(h.repo, req.SessionID, user.ID), req.SessionID); ok {
+		if hasStored {
 			ids := roleAgentIDs(stored.Messages)
 			var reuse strings.Builder
 			for _, role := range req.roles {
@@ -1257,7 +1264,7 @@ func runNodeAgentID(r domain.Run, id string) string {
 // bound to and which others it may switch to, so it picks the one for the work
 // the user actually means instead of following @ mentions mechanically.
 func (h *assistantHandlers) workspaceTurnContext(req *assistantRequest, user domain.User) string {
-	session, ok := h.repo.AssistantSession(assistantStorageOwner(h.repo, req.SessionID, user.ID), req.SessionID)
+	session, ok := h.storedAssistantSession(req.SessionID, user.ID)
 	if !ok || session.Shared || session.RunID != "" {
 		return ""
 	}

internal/httpapi/assistant_workbench_session_test.go

@@ -0,0 +1,54 @@
+package httpapi
+
+import (
+	"testing"
+
+	"agentflow/internal/domain"
+)
+
+func TestStoredAssistantSessionReadsCallerSlotAndTreatsEmptyIDAsMissing(t *testing.T) {
+	h, owner := workbenchFixture(t)
+	if _, ok := h.storedAssistantSession("", owner.ID); ok {
+		t.Fatal("empty session id must not resolve to a conversation")
+	}
+	if _, ok := h.storedAssistantSession("workbench-absent", owner.ID); ok {
+		t.Fatal("unknown session id must not resolve to a conversation")
+	}
+	if err := h.repo.PutAssistantSession(owner.ID, domain.AssistantSession{ID: "workbench-own"}); err != nil {
+		t.Fatal(err)
+	}
+	session, ok := h.storedAssistantSession("workbench-own", owner.ID)
+	if !ok || session.ID != "workbench-own" {
+		t.Fatalf("expected the caller's own conversation, got %+v (found=%v)", session, ok)
+	}
+	other := owner
+	other.ID = owner.ID + 1
+	if _, ok := h.storedAssistantSession("workbench-own", other.ID); ok {
+		t.Fatal("a private conversation must not be readable by another user")
+	}
+}
+
+func TestHistoricalAttachmentIDsKeepsOnlyThisProjectsUnownedAttachments(t *testing.T) {
+	session := domain.AssistantSession{
+		ProjectID: "workbench",
+		Messages: []assistantMessage{
+			{Attachments: []domain.AssistantAttachment{{ID: "first"}, {ID: "duplicate"}}},
+			{Attachments: []domain.AssistantAttachment{{ID: "duplicate"}, {ID: "second"}}},
+		},
+	}
+	got := historicalAttachmentIDs(session, true, "workbench")
+	if len(got) != 3 || !got["first"] || !got["second"] || !got["duplicate"] {
+		t.Fatalf("expected attachments of the current project, got %+v", got)
+	}
+	if ids := historicalAttachmentIDs(domain.AssistantSession{}, false, "workbench"); len(ids) != 0 {
+		t.Fatalf("missing conversation must not expose attachments, got %+v", ids)
+	}
+	shared := session
+	shared.Shared = true
+	if ids := historicalAttachmentIDs(shared, true, "workbench"); len(ids) != 0 {
+		t.Fatalf("shared conversations must not inherit private attachments, got %+v", ids)
+	}
+	if ids := historicalAttachmentIDs(session, true, "other"); len(ids) != 0 {
+		t.Fatalf("another project must not reuse attachments, got %+v", ids)
+	}
+}

internal/httpapi/assistant_workbench_stored_session_guard_test.go

@@ -0,0 +1,74 @@
+package httpapi
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+
+	"agentflow/internal/domain"
+)
+
+// prepareWorkbenchContext reads the stored conversation once and reuses it for
+// every check. These cases pin the two checks that depend on that read: a
+// conversation saved under another project is refused, and a conversation
+// bound to an issue validates and injects that issue on every turn.
+
+func TestPrepareWorkbenchContextRefusesConversationOfAnotherProject(t *testing.T) {
+	h, u := workbenchFixture(t)
+	if err := h.repo.PutProject(domain.Project{ID: "other", OwnerID: u.ID}); err != nil {
+		t.Fatal(err)
+	}
+	if err := h.repo.PutAssistantSession(u.ID, domain.AssistantSession{ID: "stored-elsewhere", ProjectID: "workbench", Messages: []domain.AssistantMessage{{Role: "user", Content: "讨论", AuthorID: u.ID}}}); err != nil {
+		t.Fatal(err)
+	}
+	c, _ := workbenchContext(u, "POST", "/", bytes.NewBuffer(nil))
+
+	req := assistantRequest{SessionID: "stored-elsewhere", ProjectID: "other", Messages: []assistantMessage{{Role: "user", Content: "继续"}}}
+	err := h.prepareWorkbenchContext(c, &req, u)
+	if err == nil || !strings.Contains(err.Error(), "会话属于其他项目") {
+		t.Fatalf("conversation of another project accepted: %v", err)
+	}
+
+	// The same conversation is fine in its own project, and an unknown
+	// session ID is just a new conversation.
+	req = assistantRequest{SessionID: "stored-elsewhere", ProjectID: "workbench", Messages: []assistantMessage{{Role: "user", Content: "继续"}}}
+	if err := h.prepareWorkbenchContext(c, &req, u); err != nil {
+		t.Fatalf("own project conversation refused: %v", err)
+	}
+	req = assistantRequest{SessionID: "never-saved", ProjectID: "other", Messages: []assistantMessage{{Role: "user", Content: "新会话"}}}
+	if err := h.prepareWorkbenchContext(c, &req, u); err != nil {
+		t.Fatalf("new conversation refused: %v", err)
+	}
+}
+
+func TestPrepareWorkbenchContextValidatesAndInjectsStoredSessionIssue(t *testing.T) {
+	h, u := workbenchFixture(t)
+	if err := h.repo.PutProject(domain.Project{ID: "other", OwnerID: u.ID}); err != nil {
+		t.Fatal(err)
+	}
+	putContextIssue(t, h, "bound-primary", "workbench", "已绑定的主需求")
+	if err := h.repo.PutAssistantSession(u.ID, domain.AssistantSession{ID: "issue-bound", ProjectID: "workbench", IssueID: "bound-primary", Messages: []domain.AssistantMessage{{Role: "user", Content: "讨论", AuthorID: u.ID}}}); err != nil {
+		t.Fatal(err)
+	}
+	if stored, ok := h.repo.AssistantSession(u.ID, "issue-bound"); !ok || stored.IssueID != "bound-primary" {
+		t.Fatalf("fixture did not keep the issue binding: %+v (found=%v)", stored, ok)
+	}
+	c, _ := workbenchContext(u, "POST", "/", bytes.NewBuffer(nil))
+
+	req := assistantRequest{SessionID: "issue-bound", ProjectID: "workbench", Messages: []assistantMessage{{Role: "user", Content: "继续"}}}
+	if err := h.prepareWorkbenchContext(c, &req, u); err != nil {
+		t.Fatalf("bound conversation refused: %v", err)
+	}
+	if !strings.Contains(req.contextText, "当前会话关联的主需求") || !strings.Contains(req.contextText, "已绑定的主需求") {
+		t.Fatalf("bound issue not injected into context:\n%s", req.contextText)
+	}
+
+	// The binding is re-validated every turn: once the issue leaves the
+	// project, the conversation must stop using it.
+	putContextIssue(t, h, "bound-primary", "other", "已移走的主需求")
+	req = assistantRequest{SessionID: "issue-bound", ProjectID: "workbench", Messages: []assistantMessage{{Role: "user", Content: "继续"}}}
+	err := h.prepareWorkbenchContext(c, &req, u)
+	if err == nil || !strings.Contains(err.Error(), "需求不存在或不属于当前项目") {
+		t.Fatalf("moved issue still accepted: %v\n%s", err, req.contextText)
+	}
+}