fix(docduty): treat assigned duty as authoritative on heal

Align document slots to project document_duty even when the wrong ID is an active member, and fail generate/list instead of returning stale placeholder duties. Owner author backfill now picks creator, else lowest ID. Co-authored-by: Cursor <cursoragent@cursor.com>
2026-09-13 22:16:36 +00:00
parent 60e2b1adf2
commit 2f0a1b337c
8 changed files with 258 additions and 38 deletions
@@ -17,23 +17,36 @@ CREATE UNIQUE INDEX IF NOT EXISTS uq_project_members_active_document_duty
    WHERE status = 'active' AND document_duty <> '';

 -- 创建者默认编制职责；已有非空职责不覆盖。
+-- 多 owner/异常历史只回填一行：创建者优先，否则最低 user_id。
+-- 必须与 internal/docduty.OwnerAuthorBackfillSQL 保持同步。
+WITH candidates AS (
+    SELECT pm.project_id, pm.user_id,
+           CASE WHEN p.creator_user_id = pm.user_id THEN 0 ELSE 1 END AS creator_rank,
+           pm.user_id AS id_rank
+    FROM project_members pm
+    JOIN projects p ON p.id = pm.project_id
+    WHERE pm.status = 'active'
+      AND pm.document_duty = ''
+      AND (
+            pm.project_role = 'owner'
+            OR p.creator_user_id = pm.user_id
+          )
+      AND NOT EXISTS (
+            SELECT 1 FROM project_members other
+            WHERE other.project_id = pm.project_id
+              AND other.status = 'active'
+              AND other.document_duty = 'author'
+          )
+),
+picked AS (
+    SELECT DISTINCT ON (project_id) project_id, user_id
+    FROM candidates
+    ORDER BY project_id, creator_rank ASC, id_rank ASC
+)
 UPDATE project_members pm
 SET document_duty = 'author', updated_at = now()
-WHERE pm.status = 'active'
-  AND pm.document_duty = ''
-  AND (
-        pm.project_role = 'owner'
-        OR EXISTS (
-            SELECT 1 FROM projects p
-            WHERE p.id = pm.project_id AND p.creator_user_id = pm.user_id
-        )
-      )
-  AND NOT EXISTS (
-        SELECT 1 FROM project_members other
-        WHERE other.project_id = pm.project_id
-          AND other.status = 'active'
-          AND other.document_duty = 'author'
-      );
+FROM picked
+WHERE pm.project_id = picked.project_id AND pm.user_id = picked.user_id;

 -- +goose Down
 DROP INDEX IF EXISTS uq_project_members_active_document_duty;
@@ -7,6 +7,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"log"
 	"os"
 	"path/filepath"
 	"strings"
@@ -480,11 +481,33 @@ func (s *Service) ExecuteTask(ctx context.Context, req TaskRunRequest) (*TaskRun
 			authorID, authorName, reviewerID, reviewerName, approverID, approverName,
 		)
 		if s.pool != nil && req.ProjectID > 0 {
-			if assigned, _, healErr := docduty.HealProjectDocuments(ctx, s.pool, req.ProjectID); healErr == nil && docRec != nil {
-				if members, creator, loadErr := docduty.LoadProjectMembers(ctx, s.pool, req.ProjectID); loadErr == nil {
-					next, _ := docduty.HealBinding(bindingFromDoc(docRec), assigned, docduty.ActiveMemberIDs(members, creator))
-					writeBindingToDoc(docRec, next)
+			assigned, _, healErr := docduty.HealProjectDocuments(ctx, s.pool, req.ProjectID)
+			if healErr != nil {
+				log.Printf("[docduty] heal project %d after generate failed: %v", req.ProjectID, healErr)
+				return &TaskRunResult{
+					TaskKey:       req.TaskKey,
+					ArtifactTitle: artifactTitle,
+					WorkDir:       req.WorkDir,
+					Success:       false,
+					ErrorReason:   "document duty heal failed: " + healErr.Error(),
+					DocRecord:     docRec,
+				}, healErr
+			}
+			if docRec != nil {
+				members, creator, loadErr := docduty.LoadProjectMembers(ctx, s.pool, req.ProjectID)
+				if loadErr != nil {
+					log.Printf("[docduty] load members project %d after generate failed: %v", req.ProjectID, loadErr)
+					return &TaskRunResult{
+						TaskKey:       req.TaskKey,
+						ArtifactTitle: artifactTitle,
+						WorkDir:       req.WorkDir,
+						Success:       false,
+						ErrorReason:   "document duty load failed: " + loadErr.Error(),
+						DocRecord:     docRec,
+					}, loadErr
 				}
+				next, _ := docduty.HealBinding(bindingFromDoc(docRec), assigned, docduty.ActiveMemberIDs(members, creator))
+				writeBindingToDoc(docRec, next)
 			}
 		}

@@ -0,0 +1,90 @@
+package docduty
+
+import "strings"
+
+// OwnerAuthorBackfillSQL 与 goose 00031 保持同步：每个项目只回填一行编制人。
+// 创建者优先，否则最低 user_id，避免单条 UPDATE 同时改多行撞部分唯一索引。
+const OwnerAuthorBackfillSQL = `
+WITH candidates AS (
+  SELECT pm.project_id, pm.user_id,
+    CASE WHEN p.creator_user_id = pm.user_id THEN 0 ELSE 1 END AS creator_rank,
+    pm.user_id AS id_rank
+  FROM project_members pm
+  JOIN projects p ON p.id = pm.project_id
+  WHERE pm.status = 'active'
+    AND pm.document_duty = ''
+    AND (
+      pm.project_role = 'owner'
+      OR p.creator_user_id = pm.user_id
+    )
+    AND NOT EXISTS (
+      SELECT 1 FROM project_members other
+      WHERE other.project_id = pm.project_id
+        AND other.status = 'active'
+        AND other.document_duty = 'author'
+    )
+),
+picked AS (
+  SELECT DISTINCT ON (project_id) project_id, user_id
+  FROM candidates
+  ORDER BY project_id, creator_rank ASC, id_rank ASC
+)
+UPDATE project_members pm
+SET document_duty = 'author', updated_at = now()
+FROM picked
+WHERE pm.project_id = picked.project_id AND pm.user_id = picked.user_id
+`
+
+// OwnerAuthorBackfillOneProjectSQL 与上式相同，仅限单个项目，供隔离测试使用。
+const OwnerAuthorBackfillOneProjectSQL = `
+WITH candidates AS (
+  SELECT pm.project_id, pm.user_id,
+    CASE WHEN p.creator_user_id = pm.user_id THEN 0 ELSE 1 END AS creator_rank,
+    pm.user_id AS id_rank
+  FROM project_members pm
+  JOIN projects p ON p.id = pm.project_id
+  WHERE pm.project_id = $1
+    AND pm.status = 'active'
+    AND pm.document_duty = ''
+    AND (
+      pm.project_role = 'owner'
+      OR p.creator_user_id = pm.user_id
+    )
+    AND NOT EXISTS (
+      SELECT 1 FROM project_members other
+      WHERE other.project_id = pm.project_id
+        AND other.status = 'active'
+        AND other.document_duty = 'author'
+    )
+),
+picked AS (
+  SELECT DISTINCT ON (project_id) project_id, user_id
+  FROM candidates
+  ORDER BY project_id, creator_rank ASC, id_rank ASC
+)
+UPDATE project_members pm
+SET document_duty = 'author', updated_at = now()
+FROM picked
+WHERE pm.project_id = picked.project_id AND pm.user_id = picked.user_id
+`
+
+// PickDefaultAuthorUserID 多 owner/异常历史时确定性选择编制回填对象。
+// 仅考虑在任且 document_duty 为空的 owner 或创建者：创建者优先，否则最低 user_id。
+func PickDefaultAuthorUserID(members []Member, creatorID int64) int64 {
+	var lowestOwner int64
+	for _, m := range members {
+		if !isActiveMember(m) || strings.TrimSpace(m.DocumentDuty) != DutyNone {
+			continue
+		}
+		if creatorID > 0 && m.UserID == creatorID {
+			return creatorID
+		}
+		if m.ProjectRole != "owner" {
+			continue
+		}
+		if lowestOwner == 0 || m.UserID < lowestOwner {
+			lowestOwner = m.UserID
+		}
+	}
+	return lowestOwner
+}
@@ -0,0 +1,35 @@
+package docduty
+
+import "testing"
+
+func TestPickDefaultAuthorUserID_CreatorPreferredOverLowestOwner(t *testing.T) {
+	members := []Member{
+		{UserID: 3, Status: StatusActive, ProjectRole: "owner", DocumentDuty: DutyNone},
+		{UserID: 10, Status: StatusActive, ProjectRole: "owner", DocumentDuty: DutyNone},
+	}
+	if got := PickDefaultAuthorUserID(members, 10); got != 10 {
+		t.Fatalf("creator should win over lowest owner, got %d", got)
+	}
+}
+
+func TestPickDefaultAuthorUserID_LowestIDWhenCreatorMissing(t *testing.T) {
+	members := []Member{
+		{UserID: 8, Status: StatusActive, ProjectRole: "owner", DocumentDuty: DutyNone},
+		{UserID: 3, Status: StatusActive, ProjectRole: "owner", DocumentDuty: DutyNone},
+		{UserID: 1, Status: StatusActive, ProjectRole: "member", DocumentDuty: DutyNone},
+	}
+	if got := PickDefaultAuthorUserID(members, 99); got != 3 {
+		t.Fatalf("lowest owner id, got %d", got)
+	}
+}
+
+func TestPickDefaultAuthorUserID_SkipsExistingDutyAndExited(t *testing.T) {
+	members := []Member{
+		{UserID: 1, Status: StatusActive, ProjectRole: "owner", DocumentDuty: DutyAuthor},
+		{UserID: 2, Status: "exited", ProjectRole: "owner", DocumentDuty: DutyNone},
+		{UserID: 4, Status: StatusActive, ProjectRole: "owner", DocumentDuty: DutyNone},
+	}
+	if got := PickDefaultAuthorUserID(members, 1); got != 4 {
+		t.Fatalf("existing author/exited must be skipped, got %d", got)
+	}
+}
@@ -142,19 +142,19 @@ func DutyMissing(currentID int64, memberIDs map[int64]struct{}) bool {
 }

 func healSlot(current, assigned Holder, memberIDs map[int64]struct{}) (Holder, bool) {
-	if !DutyMissing(current.UserID, memberIDs) {
-		return current, false
+	if assigned.UserID > 0 {
+		if current.UserID == assigned.UserID && current.Name == assigned.Name {
+			return current, false
+		}
+		return assigned, true
 	}
-	if assigned.UserID <= 0 {
-		return current, false
-	}
-	if current.UserID == assigned.UserID && current.Name == assigned.Name {
-		return current, false
-	}
-	return assigned, true
+	// assigned 为空：不臆造、不清空。在任成员原样保留；占位/非成员也不删除。
+	_ = memberIDs
+	return current, false
 }

-// HealBinding 仅回填缺失/非成员槽位；已是在任成员的职责不覆盖，避免作者/批准人串位。
+// HealBinding 以项目 document_duty 为权威。assigned 非零则对齐（错 ID 或同 ID 旧名称）；
+// assigned 为空才保留当前值，避免臆造或清空。
 func HealBinding(current, assigned Binding, memberIDs map[int64]struct{}) (Binding, bool) {
 	out := current
 	changed := false
@@ -79,19 +79,18 @@ func TestHealBinding_PlaceholderNonMemberBackfillIdempotent(t *testing.T) {
 		Reviewer: Holder{UserID: 2, Name: "审核负责人"},
 		Approver: Holder{UserID: 3, Name: "批准负责人"},
 	}
-	// 3 是在任成员：批准槽位不得被改写成别人。
 	first, changed := HealBinding(current, assigned, memberIDs)
 	if !changed {
-		t.Fatal("placeholder reviewer 2 must be healed")
+		t.Fatal("placeholder reviewer 2 and stale/wrong assigned slots must be healed")
 	}
 	if first.Reviewer.UserID != 171 {
 		t.Fatalf("reviewer want 171 got %+v", first.Reviewer)
 	}
-	if first.Author.UserID != 170 {
-		t.Fatalf("author must stay A: %+v", first.Author)
+	if first.Author.UserID != 170 || first.Author.Name != "编制人A" {
+		t.Fatalf("author must stay A with assigned name: %+v", first.Author)
 	}
-	if first.Approver.UserID != 3 {
-		t.Fatalf("member approver 3 must not be swapped: %+v", first.Approver)
+	if first.Approver.UserID != 4 || first.Approver.Name != "王总监" {
+		t.Fatalf("assigned approver 4 must replace member 3: %+v", first.Approver)
 	}

 	second, changedAgain := HealBinding(first, assigned, memberIDs)
@@ -103,6 +102,64 @@ func TestHealBinding_PlaceholderNonMemberBackfillIdempotent(t *testing.T) {
 	}
 }

+func TestHealBinding_WrongActiveMemberIDAlignsToAssigned(t *testing.T) {
+	members, creator := sampleMembers()
+	assigned := Resolve(members, creator, RequestDuties{})
+	memberIDs := ActiveMemberIDs(members, creator)
+	current := Binding{
+		Author:   Holder{UserID: 170, Name: "编制人A"},
+		Reviewer: Holder{UserID: 3, Name: "李主管"},
+		Approver: Holder{UserID: 3, Name: "李主管"},
+	}
+	got, changed := HealBinding(current, assigned, memberIDs)
+	if !changed {
+		t.Fatal("wrong active-member IDs must align to assigned duties")
+	}
+	if got.Reviewer.UserID != 171 || got.Approver.UserID != 4 {
+		t.Fatalf("want reviewer=171 approver=4, got %+v", got)
+	}
+	if got.Author.UserID != 170 {
+		t.Fatalf("author must stay assigned A: %+v", got.Author)
+	}
+}
+
+func TestHealBinding_SameIDStaleNameAligns(t *testing.T) {
+	members, creator := sampleMembers()
+	assigned := Resolve(members, creator, RequestDuties{})
+	memberIDs := ActiveMemberIDs(members, creator)
+	current := Binding{
+		Author:   Holder{UserID: 170, Name: "编制工程师"},
+		Reviewer: Holder{UserID: 171, Name: "审核负责人"},
+		Approver: Holder{UserID: 4, Name: "批准负责人"},
+	}
+	got, changed := HealBinding(current, assigned, memberIDs)
+	if !changed {
+		t.Fatal("same IDs with stale names must be healed")
+	}
+	if got.Author.Name != "编制人A" || got.Reviewer.Name != "审核人B" || got.Approver.Name != "王总监" {
+		t.Fatalf("stale names not aligned: %+v", got)
+	}
+	if got.Author.UserID != 170 || got.Reviewer.UserID != 171 || got.Approver.UserID != 4 {
+		t.Fatalf("IDs must stay assigned: %+v", got)
+	}
+}
+
+func TestHealBinding_EmptyAssignedKeepsActiveMember(t *testing.T) {
+	memberIDs := map[int64]struct{}{170: {}, 3: {}}
+	current := Binding{
+		Author:   Holder{UserID: 170, Name: "编制人A"},
+		Reviewer: Holder{UserID: 3, Name: "李主管"},
+		Approver: Holder{UserID: 3, Name: "李主管"},
+	}
+	got, changed := HealBinding(current, Binding{}, memberIDs)
+	if changed {
+		t.Fatalf("empty assigned must not invent or clear: %+v", got)
+	}
+	if got != current {
+		t.Fatalf("empty assigned must keep current: %+v", got)
+	}
+}
+
 func TestHealBinding_ZeroDutyBackfill(t *testing.T) {
 	members, creator := sampleMembers()
 	assigned := Resolve(members, creator, RequestDuties{})
@@ -67,7 +67,7 @@ type storedDocDuties struct {
 	Approver Holder
 }

-// BackfillMissingDocumentDuties 对项目内受控文档回填缺失/非成员职责；已是成员的槽位不覆盖。
+// BackfillMissingDocumentDuties 按项目 assigned 职责对齐受控文档；assigned 为空的槽位不臆造、不清空。
 func BackfillMissingDocumentDuties(ctx context.Context, pool *pgxpool.Pool, projectID int64, assigned Binding) (int, error) {
 	if pool == nil || projectID <= 0 {
 		return 0, nil
@@ -144,7 +144,7 @@ func nullIfZero(id int64) any {
 	return id
 }

-// HealProjectDocuments 解析项目职责并回填缺失文档槽位；第二次调用应为 0 行。
+// HealProjectDocuments 解析项目职责并对齐文档槽位；第二次调用应为 0 行。错误不得被调用方静默吞掉。
 func HealProjectDocuments(ctx context.Context, pool *pgxpool.Pool, projectID int64) (Binding, int, error) {
 	assigned, err := ResolveProject(ctx, pool, projectID, RequestDuties{})
 	if err != nil {
@@ -53,7 +53,9 @@ func (e *GovernanceEngine) ListWorkflowDocumentsByProject(ctx context.Context, p
 	}

 	if e.pool != nil {
-		_, _, _ = docduty.HealProjectDocuments(ctx, e.pool, projectID)
+		if _, _, err := docduty.HealProjectDocuments(ctx, e.pool, projectID); err != nil {
+			return nil, fmt.Errorf("heal project document duties: %w", err)
+		}
 		rows, err := e.pool.Query(ctx, `
 			SELECT doc_id, project_id, task_key, title, current_state, is_template_only,
 			       COALESCE(content_sha256, ''), COALESCE(work_dir, ''), version, updated_at,