fix(docduty): revoke inactive slots and explicit duty changes

Empty assigned heal now clears only exited or non-member reviewer/approver holders and never blanks author. Explicit reconcile uses the pre-update duty plus a project advisory lock so viewer demotion and transfers are not left to generic heal. Co-authored-by: Cursor <cursoragent@cursor.com>
2026-09-13 22:29:53 +00:00
parent ec89b4c8b0
commit 9a19bcc12c
5 changed files with 365 additions and 10 deletions
@@ -141,32 +141,40 @@ func DutyMissing(currentID int64, memberIDs map[int64]struct{}) bool {
 	return !ok
 }

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

-// HealBinding 以项目 document_duty 为权威。assigned 非零则对齐（错 ID 或同 ID 旧名称）；
-// assigned 为空才保留当前值，避免臆造或清空。
+// HealBinding 以项目 document_duty 为权威。assigned 非零则对齐（错 ID 或同 ID 旧名称）。
+// assigned 为空时：审核/批准仅清空已退出或非成员的失效 holder；在任成员保留以兼容未初始化 duty 的旧项目。
+// 编制槽位即使 assigned 为空也不清空，避免无意去掉 owner/author。
 func HealBinding(current, assigned Binding, memberIDs map[int64]struct{}) (Binding, bool) {
 	out := current
 	changed := false
-	if next, ok := healSlot(current.Author, assigned.Author, memberIDs); ok {
+	if next, ok := healSlot(current.Author, assigned.Author, memberIDs, false); ok {
 		out.Author = next
 		changed = true
 	}
-	if next, ok := healSlot(current.Reviewer, assigned.Reviewer, memberIDs); ok {
+	if next, ok := healSlot(current.Reviewer, assigned.Reviewer, memberIDs, true); ok {
 		out.Reviewer = next
 		changed = true
 	}
-	if next, ok := healSlot(current.Approver, assigned.Approver, memberIDs); ok {
+	if next, ok := healSlot(current.Approver, assigned.Approver, memberIDs, true); ok {
 		out.Approver = next
 		changed = true
 	}
@@ -153,13 +153,41 @@ func TestHealBinding_EmptyAssignedKeepsActiveMember(t *testing.T) {
 	}
 	got, changed := HealBinding(current, Binding{}, memberIDs)
 	if changed {
-		t.Fatalf("empty assigned must not invent or clear: %+v", got)
+		t.Fatalf("empty assigned must not invent or clear active members: %+v", got)
 	}
 	if got != current {
 		t.Fatalf("empty assigned must keep current: %+v", got)
 	}
 }

+func TestHealBinding_EmptyAssignedClearsInactiveHolder(t *testing.T) {
+	memberIDs := map[int64]struct{}{170: {}}
+	current := Binding{
+		Author:   Holder{UserID: 170, Name: "编制人A"},
+		Reviewer: Holder{UserID: 171, Name: "已退出审核人"},
+		Approver: Holder{UserID: 2, Name: "审核负责人"},
+	}
+	got, changed := HealBinding(current, Binding{}, memberIDs)
+	if !changed {
+		t.Fatal("inactive reviewer/approver must be cleared when assigned is empty")
+	}
+	if got.Reviewer.UserID != 0 || got.Approver.UserID != 0 {
+		t.Fatalf("want cleared review/approve slots, got %+v", got)
+	}
+	if got.Author.UserID != 170 {
+		t.Fatalf("author must stay: %+v", got.Author)
+	}
+}
+
+func TestHealBinding_EmptyAssignedDoesNotClearAuthor(t *testing.T) {
+	memberIDs := map[int64]struct{}{}
+	current := Binding{Author: Holder{UserID: 170, Name: "编制人A"}}
+	got, changed := HealBinding(current, Binding{}, memberIDs)
+	if changed || got.Author.UserID != 170 {
+		t.Fatalf("author must not be cleared when assigned is empty: changed=%v got=%+v", changed, got)
+	}
+}
+
 func TestHealBinding_ZeroDutyBackfill(t *testing.T) {
 	members, creator := sampleMembers()
 	assigned := Resolve(members, creator, RequestDuties{})
@@ -0,0 +1,113 @@
+package docduty
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/jackc/pgx/v5/pgxpool"
+)
+
+// MemberDutyChange 成员职责/在任状态变更，供显式撤销文档槽位。
+type MemberDutyChange struct {
+	UserID         int64
+	PreviousDuty   string
+	NextDuty       string
+	Exited         bool
+	PriorHolderIDs map[string]int64
+}
+
+// SlotRevocation 从受控文档上撤销某用户的审核/批准槽位；永不针对 author。
+type SlotRevocation struct {
+	UserID int64
+	Duty   string
+}
+
+// PriorDutyHolders 更新前在任成员的职责持有者。
+func PriorDutyHolders(members []Member) map[string]int64 {
+	out := make(map[string]int64, 3)
+	for _, m := range members {
+		if !isActiveMember(m) {
+			continue
+		}
+		switch m.DocumentDuty {
+		case DutyAuthor, DutyReviewer, DutyApprover:
+			out[m.DocumentDuty] = m.UserID
+		}
+	}
+	return out
+}
+
+// PlanSlotRevocations 按更新前 duty 显式计算要撤销的审核/批准槽位。
+// reviewer/approver → viewer/其他职责、职责转移、退出成员都会产生撤销；author 永不进入结果。
+func PlanSlotRevocations(change MemberDutyChange) []SlotRevocation {
+	seen := make(map[string]struct{}, 4)
+	out := make([]SlotRevocation, 0, 2)
+	add := func(userID int64, duty string) {
+		if userID <= 0 || (duty != DutyReviewer && duty != DutyApprover) {
+			return
+		}
+		key := fmt.Sprintf("%d:%s", userID, duty)
+		if _, ok := seen[key]; ok {
+			return
+		}
+		seen[key] = struct{}{}
+		out = append(out, SlotRevocation{UserID: userID, Duty: duty})
+	}
+	if change.Exited || change.PreviousDuty != change.NextDuty {
+		add(change.UserID, change.PreviousDuty)
+	}
+	if change.NextDuty == DutyReviewer || change.NextDuty == DutyApprover {
+		if prior := change.PriorHolderIDs[change.NextDuty]; prior > 0 && prior != change.UserID {
+			add(prior, change.NextDuty)
+		}
+	}
+	return out
+}
+
+// ReconcileMemberDuty 在项目级 advisory lock 下按更新前 duty 清空对应槽位，再 heal 新职责。
+func ReconcileMemberDuty(ctx context.Context, pool *pgxpool.Pool, projectID int64, change MemberDutyChange) error {
+	if pool == nil || projectID <= 0 {
+		return nil
+	}
+	tx, err := pool.Begin(ctx)
+	if err != nil {
+		return fmt.Errorf("docduty: begin reconcile: %w", err)
+	}
+	defer func() { _ = tx.Rollback(ctx) }()
+	if _, err := tx.Exec(ctx, `SELECT pg_advisory_xact_lock($1)`, projectID); err != nil {
+		return fmt.Errorf("docduty: lock project %d: %w", projectID, err)
+	}
+	for _, rev := range PlanSlotRevocations(change) {
+		q, ok := clearDutySlotSQL(rev.Duty)
+		if !ok {
+			continue
+		}
+		if _, err := tx.Exec(ctx, q, projectID, rev.UserID); err != nil {
+			return fmt.Errorf("docduty: clear %s for user %d: %w", rev.Duty, rev.UserID, err)
+		}
+	}
+	if err := tx.Commit(ctx); err != nil {
+		return fmt.Errorf("docduty: commit reconcile: %w", err)
+	}
+	_, _, err = HealProjectDocuments(ctx, pool, projectID)
+	return err
+}
+
+func clearDutySlotSQL(duty string) (string, bool) {
+	switch duty {
+	case DutyReviewer:
+		return `
+			UPDATE document_records
+			SET reviewer_id = NULL, reviewer_name = '', updated_at = now()
+			WHERE project_id = $1 AND COALESCE(task_key, '') <> '' AND reviewer_id = $2
+		`, true
+	case DutyApprover:
+		return `
+			UPDATE document_records
+			SET approver_id = NULL, approver_name = '', updated_at = now()
+			WHERE project_id = $1 AND COALESCE(task_key, '') <> '' AND approver_id = $2
+		`, true
+	default:
+		return "", false
+	}
+}
@@ -0,0 +1,62 @@
+package docduty
+
+import "testing"
+
+func TestPlanSlotRevocations_ReviewerToViewer(t *testing.T) {
+	got := PlanSlotRevocations(MemberDutyChange{
+		UserID: 171, PreviousDuty: DutyReviewer, NextDuty: DutyNone,
+	})
+	if len(got) != 1 || got[0].UserID != 171 || got[0].Duty != DutyReviewer {
+		t.Fatalf("reviewer→viewer: %+v", got)
+	}
+}
+
+func TestPlanSlotRevocations_DutyTransfer(t *testing.T) {
+	got := PlanSlotRevocations(MemberDutyChange{
+		UserID: 99, PreviousDuty: DutyNone, NextDuty: DutyReviewer,
+		PriorHolderIDs: map[string]int64{DutyReviewer: 171},
+	})
+	if len(got) != 1 || got[0].UserID != 171 || got[0].Duty != DutyReviewer {
+		t.Fatalf("transfer must revoke prior reviewer: %+v", got)
+	}
+}
+
+func TestPlanSlotRevocations_ExitedMember(t *testing.T) {
+	got := PlanSlotRevocations(MemberDutyChange{
+		UserID: 171, PreviousDuty: DutyReviewer, NextDuty: DutyReviewer, Exited: true,
+	})
+	if len(got) != 1 || got[0].Duty != DutyReviewer {
+		t.Fatalf("exited reviewer must revoke: %+v", got)
+	}
+}
+
+func TestPlanSlotRevocations_IdempotentEmpty(t *testing.T) {
+	got := PlanSlotRevocations(MemberDutyChange{UserID: 171, PreviousDuty: DutyNone, NextDuty: DutyNone})
+	if len(got) != 0 {
+		t.Fatalf("repeat empty change must be no-op: %+v", got)
+	}
+}
+
+func TestPlanSlotRevocations_NeverRevokesAuthor(t *testing.T) {
+	got := PlanSlotRevocations(MemberDutyChange{
+		UserID: 170, PreviousDuty: DutyAuthor, NextDuty: DutyNone,
+		PriorHolderIDs: map[string]int64{DutyAuthor: 170},
+	})
+	if len(got) != 0 {
+		t.Fatalf("author must not be revoked: %+v", got)
+	}
+}
+
+func TestPriorDutyHolders_IgnoresExited(t *testing.T) {
+	holders := PriorDutyHolders([]Member{
+		{UserID: 170, Status: StatusActive, DocumentDuty: DutyAuthor},
+		{UserID: 171, Status: "exited", DocumentDuty: DutyReviewer},
+		{UserID: 4, Status: StatusActive, DocumentDuty: DutyApprover},
+	})
+	if holders[DutyAuthor] != 170 || holders[DutyApprover] != 4 {
+		t.Fatalf("holders=%v", holders)
+	}
+	if _, ok := holders[DutyReviewer]; ok {
+		t.Fatal("exited reviewer must not be a prior holder")
+	}
+}
@@ -163,6 +163,150 @@ func TestHealBinding_EmptyAssignedKeepsMember_PG(t *testing.T) {
 	assertDocApprover(t, ctx, pool, docID, memberID)
 }

+func TestHealBinding_EmptyAssignedClearsExited_PG(t *testing.T) {
+	ctx, _, pool := openDutyTestPool(t)
+	suffix := time.Now().UnixNano()
+	authorID := insertDutyUser(t, ctx, pool, suffix, "ex_author")
+	exitedID := insertDutyUser(t, ctx, pool, suffix, "ex_reviewer")
+
+	var projectID int64
+	if err := pool.QueryRow(ctx, `
+		INSERT INTO projects (code, name, status, creator_user_id)
+		VALUES ($1, $2, 'active', $3)
+		RETURNING id
+	`, fmt.Sprintf("duty-exited-%d", suffix), "clear exited", authorID).Scan(&projectID); err != nil {
+		t.Fatalf("insert project: %v", err)
+	}
+	t.Cleanup(func() {
+		_, _ = pool.Exec(context.Background(), `DELETE FROM document_records WHERE project_id = $1`, projectID)
+		_, _ = pool.Exec(context.Background(), `DELETE FROM project_members WHERE project_id = $1`, projectID)
+		_, _ = pool.Exec(context.Background(), `DELETE FROM projects WHERE id = $1`, projectID)
+		_, _ = pool.Exec(context.Background(), `DELETE FROM users WHERE id IN ($1,$2)`, authorID, exitedID)
+	})
+	if _, err := pool.Exec(ctx, `
+		INSERT INTO project_members (project_id, user_id, project_role, document_duty, status, joined_at, updated_at)
+		VALUES ($1, $2, 'owner', 'author', 'active', now(), now()),
+		       ($1, $3, 'member', '', 'exited', now(), now())
+	`, projectID, authorID, exitedID); err != nil {
+		t.Fatalf("insert members: %v", err)
+	}
+	docID := fmt.Sprintf("doc_proj%d_intake", projectID)
+	insertDutyDoc(t, ctx, pool, projectID, docID, "intake", authorID, exitedID, 0)
+	if _, err := pool.Exec(ctx, `UPDATE document_records SET author_name = $2 WHERE doc_id = $1`, docID, "用户ex_author"); err != nil {
+		t.Fatal(err)
+	}
+
+	_, n, err := HealProjectDocuments(ctx, pool, projectID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if n == 0 {
+		t.Fatal("exited reviewer must be cleared when assigned is empty")
+	}
+	assertDocReviewer(t, ctx, pool, docID, 0)
+	assertDocAuthorName(t, ctx, pool, docID, "用户ex_author")
+}
+
+func TestReconcileMemberDuty_ReviewerToViewerAndTransfer_PG(t *testing.T) {
+	ctx, _, pool := openDutyTestPool(t)
+	suffix := time.Now().UnixNano()
+	authorID := insertDutyUser(t, ctx, pool, suffix, "rec_author")
+	reviewerA := insertDutyUser(t, ctx, pool, suffix, "rec_ra")
+	reviewerB := insertDutyUser(t, ctx, pool, suffix, "rec_rb")
+
+	var projectID int64
+	if err := pool.QueryRow(ctx, `
+		INSERT INTO projects (code, name, status, creator_user_id)
+		VALUES ($1, $2, 'active', $3)
+		RETURNING id
+	`, fmt.Sprintf("duty-rec-%d", suffix), "explicit reconcile", authorID).Scan(&projectID); err != nil {
+		t.Fatalf("insert project: %v", err)
+	}
+	t.Cleanup(func() {
+		_, _ = pool.Exec(context.Background(), `DELETE FROM document_records WHERE project_id = $1`, projectID)
+		_, _ = pool.Exec(context.Background(), `DELETE FROM project_members WHERE project_id = $1`, projectID)
+		_, _ = pool.Exec(context.Background(), `DELETE FROM projects WHERE id = $1`, projectID)
+		_, _ = pool.Exec(context.Background(), `DELETE FROM users WHERE id IN ($1,$2,$3)`, authorID, reviewerA, reviewerB)
+	})
+	if _, err := pool.Exec(ctx, `
+		INSERT INTO project_members (project_id, user_id, project_role, document_duty, status, joined_at, updated_at)
+		VALUES ($1, $2, 'owner', 'author', 'active', now(), now()),
+		       ($1, $3, 'member', 'reviewer', 'active', now(), now()),
+		       ($1, $4, 'member', '', 'active', now(), now())
+	`, projectID, authorID, reviewerA, reviewerB); err != nil {
+		t.Fatalf("insert members: %v", err)
+	}
+	docID := fmt.Sprintf("doc_proj%d_intake", projectID)
+	insertDutyDoc(t, ctx, pool, projectID, docID, "intake", authorID, reviewerA, 0)
+
+	if _, err := pool.Exec(ctx, `
+		UPDATE project_members SET document_duty = '' WHERE project_id = $1 AND user_id = $2
+	`, projectID, reviewerA); err != nil {
+		t.Fatal(err)
+	}
+	if err := ReconcileMemberDuty(ctx, pool, projectID, MemberDutyChange{
+		UserID: reviewerA, PreviousDuty: DutyReviewer, NextDuty: DutyNone,
+	}); err != nil {
+		t.Fatal(err)
+	}
+	assertDocReviewer(t, ctx, pool, docID, 0)
+	var authorGot int64
+	if err := pool.QueryRow(ctx, `SELECT COALESCE(author_id, 0) FROM document_records WHERE doc_id = $1`, docID).Scan(&authorGot); err != nil {
+		t.Fatal(err)
+	}
+	if authorGot != authorID {
+		t.Fatalf("author must stay %d, got %d", authorID, authorGot)
+	}
+
+	if _, err := pool.Exec(ctx, `
+		UPDATE project_members SET document_duty = '' WHERE project_id = $1 AND user_id = $2
+	`, projectID, reviewerA); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := pool.Exec(ctx, `
+		UPDATE project_members SET document_duty = 'reviewer' WHERE project_id = $1 AND user_id = $2
+	`, projectID, reviewerB); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := pool.Exec(ctx, `
+		UPDATE document_records SET reviewer_id = $2, reviewer_name = 'A' WHERE doc_id = $1
+	`, docID, reviewerA); err != nil {
+		t.Fatal(err)
+	}
+	if err := ReconcileMemberDuty(ctx, pool, projectID, MemberDutyChange{
+		UserID: reviewerB, PreviousDuty: DutyNone, NextDuty: DutyReviewer,
+		PriorHolderIDs: map[string]int64{DutyReviewer: reviewerA},
+	}); err != nil {
+		t.Fatal(err)
+	}
+	assertDocReviewer(t, ctx, pool, docID, reviewerB)
+
+	if err := ReconcileMemberDuty(ctx, pool, projectID, MemberDutyChange{
+		UserID: reviewerB, PreviousDuty: DutyReviewer, NextDuty: DutyReviewer,
+	}); err != nil {
+		t.Fatal(err)
+	}
+	assertDocReviewer(t, ctx, pool, docID, reviewerB)
+
+	if _, err := pool.Exec(ctx, `
+		UPDATE project_members SET status = 'exited', document_duty = 'reviewer' WHERE project_id = $1 AND user_id = $2
+	`, projectID, reviewerB); err != nil {
+		t.Fatal(err)
+	}
+	if err := ReconcileMemberDuty(ctx, pool, projectID, MemberDutyChange{
+		UserID: reviewerB, PreviousDuty: DutyReviewer, NextDuty: DutyReviewer, Exited: true,
+	}); err != nil {
+		t.Fatal(err)
+	}
+	assertDocReviewer(t, ctx, pool, docID, 0)
+	if err := ReconcileMemberDuty(ctx, pool, projectID, MemberDutyChange{
+		UserID: reviewerB, PreviousDuty: DutyReviewer, NextDuty: DutyReviewer, Exited: true,
+	}); err != nil {
+		t.Fatal(err)
+	}
+	assertDocReviewer(t, ctx, pool, docID, 0)
+}
+
 func TestOwnerAuthorBackfill_MultiOwnerDeterministic_PG(t *testing.T) {
 	ctx, _, pool := openDutyTestPool(t)
 	suffix := time.Now().UnixNano()