From 68dbe8a6b23b407f4e8e5f286dd3b00588ce4fb5 Mon Sep 17 00:00:00 2001 From: weichen Date: Thu, 17 Sep 2026 09:39:59 +0000 Subject: [PATCH] test(projectinstance): cover author_only planning and activation Verify resolve/normalize/hash stability, policy gating, stale token rejection, and HTTP error mapping for the new duty policy path. Co-authored-by: Cursor --- internal/projectinstance/duty_mode_test.go | 86 +++++++- internal/projectinstance/hash_test.go | 45 ++++ internal/projectinstance/policy_test.go | 196 ++++++++++++++++++ ...project_settings_author_only_error_test.go | 50 +++++ 4 files changed, 375 insertions(+), 2 deletions(-) create mode 100644 internal/projectinstance/hash_test.go create mode 100644 internal/projectinstance/policy_test.go create mode 100644 internal/server/project_settings_author_only_error_test.go diff --git a/internal/projectinstance/duty_mode_test.go b/internal/projectinstance/duty_mode_test.go index 349472cd..bb000a66 100644 --- a/internal/projectinstance/duty_mode_test.go +++ b/internal/projectinstance/duty_mode_test.go @@ -100,9 +100,91 @@ func TestCountFullDutyDocs(t *testing.T) { {Enabled: true, Duties: DocDuty{AuthorUserID: 1, ReviewerUserID: 2, ApproverUserID: 3}}, {Enabled: true}, {Enabled: false, Duties: DocDuty{AuthorUserID: 1, ReviewerUserID: 2, ApproverUserID: 3}}, + {Enabled: true, DutyPolicy: DutyPolicyAuthorOnly, Duties: DocDuty{AuthorUserID: 9}}, }, }}} - if n := countFullDutyDocs(plan); n != 1 { - t.Fatalf("countFullDutyDocs=%d want 1", n) + if n := countFullDutyDocs(plan); n != 2 { + t.Fatalf("countFullDutyDocs=%d want 2 (full + author_only)", n) } } + +func TestResolveDocTaskDuty(t *testing.T) { + t.Parallel() + mode, eff, only := ResolveDocTaskDuty(DocTaskPlan{Enabled: false, DutyPolicy: DutyPolicyAuthorOnly, Duties: DocDuty{AuthorUserID: 1}}) + if mode != DutyModeNone || only || eff != (DocDuty{}) { + t.Fatalf("disabled: mode=%s only=%v eff=%+v", mode, only, eff) + } + mode, eff, only = ResolveDocTaskDuty(DocTaskPlan{ + Enabled: true, DutyPolicy: DutyPolicyAuthorOnly, Duties: DocDuty{AuthorUserID: 7}, + }) + if mode != DutyModeFull || !only || eff != (DocDuty{AuthorUserID: 7, ReviewerUserID: 7, ApproverUserID: 7}) { + t.Fatalf("author_only: mode=%s only=%v eff=%+v", mode, only, eff) + } + mode, _, only = ResolveDocTaskDuty(DocTaskPlan{Enabled: true, DutyPolicy: DutyPolicyAuthorOnly}) + if mode != DutyModePartial || !only { + t.Fatalf("author_only missing author: mode=%s only=%v", mode, only) + } + mode, _, only = ResolveDocTaskDuty(DocTaskPlan{Enabled: true, DutyPolicy: "weird"}) + if mode != DutyModePartial || only { + t.Fatalf("unknown policy: mode=%s only=%v", mode, only) + } +} + +func TestNormalizeAuthorOnlyDuties(t *testing.T) { + t.Parallel() + same := normalizeDocTaskDuties(DocTaskPlan{ + Enabled: true, DutyPolicy: DutyPolicyAuthorOnly, + Duties: DocDuty{AuthorUserID: 5, ReviewerUserID: 5, ApproverUserID: 5}, + }) + if same.Duties.ReviewerUserID != 0 || same.Duties.ApproverUserID != 0 || same.Duties.AuthorUserID != 5 { + t.Fatalf("same-as-author must clear derived slots: %+v", same.Duties) + } + conflict := normalizeDocTaskDuties(DocTaskPlan{ + Enabled: true, DutyPolicy: DutyPolicyAuthorOnly, + Duties: DocDuty{AuthorUserID: 5, ReviewerUserID: 8}, + }) + if conflict.Duties.ReviewerUserID != 8 { + t.Fatalf("conflicting reviewer must remain for validation: %+v", conflict.Duties) + } +} + +func TestValidateAuthorOnlyDocTaskDuties(t *testing.T) { + t.Parallel() + optional := workflowdef.DocTaskDef{Key: "research", Title: "调研", CreatorConfigurable: true} + ok := validateDocTaskDuties(DocTaskPlan{ + TaskKey: "research", Enabled: true, DutyPolicy: DutyPolicyAuthorOnly, + Duties: DocDuty{AuthorUserID: 3}, + }, optional) + if len(ok) != 0 { + t.Fatalf("valid author_only: %+v", ok) + } + missing := validateDocTaskDuties(DocTaskPlan{ + TaskKey: "research", Enabled: true, DutyPolicy: DutyPolicyAuthorOnly, + }, optional) + if len(missing) != 1 || missing[0].Code != issueAuthorOnlyAuthorRequired { + t.Fatalf("missing author: %+v", missing) + } + conflict := validateDocTaskDuties(DocTaskPlan{ + TaskKey: "research", Enabled: true, DutyPolicy: DutyPolicyAuthorOnly, + Duties: DocDuty{AuthorUserID: 3, ReviewerUserID: 9}, + }, optional) + if len(conflict) != 1 || conflict[0].Code != issueAuthorOnlyDutyConflict { + t.Fatalf("conflict: %+v", conflict) + } + unknown := validateDocTaskDuties(DocTaskPlan{ + TaskKey: "research", Enabled: true, DutyPolicy: "nope", + Duties: DocDuty{AuthorUserID: 1, ReviewerUserID: 2, ApproverUserID: 3}, + }, optional) + if len(unknown) != 1 || unknown[0].Code != issueUnknownDutyPolicy { + t.Fatalf("unknown: %+v", unknown) + } + // separated A/A/A still rejected + sep := validateDocTaskDuties(DocTaskPlan{ + TaskKey: "research", Enabled: true, + Duties: DocDuty{AuthorUserID: 1, ReviewerUserID: 1, ApproverUserID: 1}, + }, optional) + if len(sep) != 1 || sep[0].Code != issueSeparationOfDutyViolation { + t.Fatalf("separated A/A/A: %+v", sep) + } +} + diff --git a/internal/projectinstance/hash_test.go b/internal/projectinstance/hash_test.go new file mode 100644 index 00000000..2222b5f9 --- /dev/null +++ b/internal/projectinstance/hash_test.go @@ -0,0 +1,45 @@ +package projectinstance + +import ( + "testing" + + "chat-agent-system/internal/workflowdef" +) + +func TestComputePlanHash_LegacySnapshotStableWithoutDutyPolicy(t *testing.T) { + t.Parallel() + plan := WorkflowPlan{ + WorkflowDefinitionKey: workflowdef.OfficialKeyMain, + WorkflowVersion: 1, + Stages: []StagePlan{{ + StageKey: "s1", Enabled: true, + DocTasks: []DocTaskPlan{{ + TaskKey: "intake", Enabled: true, + Duties: DocDuty{AuthorUserID: 1, ReviewerUserID: 2, ApproverUserID: 3}, + }}, + }}, + } + h1, err := computePlanHash(plan) + if err != nil { + t.Fatal(err) + } + // empty DutyPolicy must not appear in JSON (omitempty) — hash must match explicit separated unset + plan2 := plan + plan2.Stages[0].DocTasks[0].DutyPolicy = "" + h2, err := computePlanHash(plan2) + if err != nil { + t.Fatal(err) + } + if h1 != h2 { + t.Fatalf("empty duty_policy must be hash-stable: %s vs %s", h1, h2) + } + plan3 := plan + plan3.Stages[0].DocTasks[0].DutyPolicy = DutyPolicyAuthorOnly + h3, err := computePlanHash(plan3) + if err != nil { + t.Fatal(err) + } + if h3 == h1 { + t.Fatal("author_only duty_policy must change plan hash") + } +} diff --git a/internal/projectinstance/policy_test.go b/internal/projectinstance/policy_test.go new file mode 100644 index 00000000..d77d27cc --- /dev/null +++ b/internal/projectinstance/policy_test.go @@ -0,0 +1,196 @@ +package projectinstance + +import ( + "context" + "errors" + "sync" + "testing" + + "github.com/jackc/pgx/v5" +) + +type fakeAuthorOnlyPolicy struct { + mu sync.Mutex + allowed bool + revision int64 + failGet bool +} + +func (f *fakeAuthorOnlyPolicy) GetAuthorOnlyApprovalPolicy(_ context.Context) (AuthorOnlyPolicySnapshot, error) { + f.mu.Lock() + defer f.mu.Unlock() + if f.failGet { + return AuthorOnlyPolicySnapshot{}, errors.New("boom") + } + return AuthorOnlyPolicySnapshot{Allowed: f.allowed, Revision: f.revision}, nil +} + +func (f *fakeAuthorOnlyPolicy) LockAuthorOnlyApprovalPolicy(ctx context.Context, _ pgx.Tx) (AuthorOnlyPolicySnapshot, error) { + return f.GetAuthorOnlyApprovalPolicy(ctx) +} + +func (f *fakeAuthorOnlyPolicy) set(allowed bool, revision int64) { + f.mu.Lock() + defer f.mu.Unlock() + f.allowed = allowed + f.revision = revision +} + +func authorOnlyPlan(authorID int64) WorkflowPlan { + plan := validPlan() + plan.Stages[0].DocTasks[0].DutyPolicy = DutyPolicyAuthorOnly + plan.Stages[0].DocTasks[0].Duties = DocDuty{AuthorUserID: authorID} + return plan +} + +func TestService_AuthorOnlySavePreviewActivate(t *testing.T) { + ctx := context.Background() + store := newMemStore() + _ = SeedProjectMeta(store, 90, LifecyclePlanning, "", 1, 0) + svc := NewService(store, testResolver(), nil, nil) + pol := &fakeAuthorOnlyPolicy{allowed: true, revision: 3} + svc.SetAuthorOnlyPolicySource(pol) + svc.SetActiveMemberLookup(&mapMemberLookup{ids: map[int64]struct{}{1: {}}}) + + settings, err := svc.SaveWorkflowPlan(ctx, 90, 1, true, SavePlanInput{ + Plan: authorOnlyPlan(1), ExpectedPlanRevision: 0, ExpectedSelectionVersion: 1, + }) + if err != nil { + t.Fatalf("SaveWorkflowPlan: %v", err) + } + if settings.Plan == nil || settings.Plan.Stages[0].DocTasks[0].DutyPolicy != DutyPolicyAuthorOnly { + t.Fatalf("expected author_only plan, got %+v", settings.Plan) + } + if !settings.AuthorOnlyPolicy.Allowed || settings.AuthorOnlyPolicy.Revision != 3 { + t.Fatalf("settings capability: %+v", settings.AuthorOnlyPolicy) + } + + preview, err := svc.ActivationPreview(ctx, 90, 1, true) + if err != nil { + t.Fatalf("ActivationPreview: %v", err) + } + if preview.Summary.AuthorOnlyDocTasks != 1 || preview.Summary.GovernedDocTasks != 1 { + t.Fatalf("preview summary: %+v", preview.Summary) + } + if len(preview.Summary.AuthorOnlyDocuments) != 1 || preview.Summary.AuthorOnlyDocuments[0].AuthorUserID != 1 { + t.Fatalf("author_only docs: %+v", preview.Summary.AuthorOnlyDocuments) + } + + result, err := svc.Activate(ctx, 90, 1, true, ActivateInput{ + ConfirmationToken: preview.ConfirmationToken, + ExpectedProjectVersion: preview.ExpectedProjectVersion, + ExpectedPlanRevision: preview.ExpectedPlanRevision, + }) + if err != nil { + t.Fatalf("Activate: %v", err) + } + if result.LifecycleStatus != LifecycleActive { + t.Fatalf("result: %+v", result) + } +} + +func TestService_AuthorOnlyRejectedWhenPolicyDisabled(t *testing.T) { + ctx := context.Background() + store := newMemStore() + _ = SeedProjectMeta(store, 91, LifecyclePlanning, "", 1, 0) + svc := NewService(store, testResolver(), nil, nil) + svc.SetAuthorOnlyPolicySource(&fakeAuthorOnlyPolicy{allowed: false, revision: 0}) + svc.SetActiveMemberLookup(&mapMemberLookup{ids: map[int64]struct{}{1: {}}}) + + _, err := svc.SaveWorkflowPlan(ctx, 91, 1, true, SavePlanInput{ + Plan: authorOnlyPlan(1), ExpectedPlanRevision: 0, ExpectedSelectionVersion: 1, + }) + if !errors.Is(err, ErrAuthorOnlyPolicyDisabled) { + t.Fatalf("expected ErrAuthorOnlyPolicyDisabled, got %v", err) + } +} + +func TestService_AuthorOnlyTokenInvalidatedWhenPolicyChanges(t *testing.T) { + ctx := context.Background() + store := newMemStore() + _ = SeedProjectMeta(store, 92, LifecyclePlanning, "", 1, 0) + svc := NewService(store, testResolver(), nil, nil) + pol := &fakeAuthorOnlyPolicy{allowed: true, revision: 1} + svc.SetAuthorOnlyPolicySource(pol) + svc.SetActiveMemberLookup(&mapMemberLookup{ids: map[int64]struct{}{1: {}}}) + + if _, err := svc.SaveWorkflowPlan(ctx, 92, 1, true, SavePlanInput{ + Plan: authorOnlyPlan(1), ExpectedPlanRevision: 0, ExpectedSelectionVersion: 1, + }); err != nil { + t.Fatalf("save: %v", err) + } + preview, err := svc.ActivationPreview(ctx, 92, 1, true) + if err != nil { + t.Fatalf("preview: %v", err) + } + pol.set(false, 2) + _, err = svc.Activate(ctx, 92, 1, true, ActivateInput{ + ConfirmationToken: preview.ConfirmationToken, + ExpectedProjectVersion: preview.ExpectedProjectVersion, + ExpectedPlanRevision: preview.ExpectedPlanRevision, + }) + if !errors.Is(err, ErrAuthorOnlyPolicyDisabled) && !errors.Is(err, ErrAuthorOnlyPolicyChanged) { + t.Fatalf("expected policy disabled/changed, got %v", err) + } +} + +func TestService_AuthorOnlyTokenRevisionMismatch(t *testing.T) { + ctx := context.Background() + store := newMemStore() + _ = SeedProjectMeta(store, 93, LifecyclePlanning, "", 1, 0) + svc := NewService(store, testResolver(), nil, nil) + pol := &fakeAuthorOnlyPolicy{allowed: true, revision: 5} + svc.SetAuthorOnlyPolicySource(pol) + svc.SetActiveMemberLookup(&mapMemberLookup{ids: map[int64]struct{}{1: {}}}) + + if _, err := svc.SaveWorkflowPlan(ctx, 93, 1, true, SavePlanInput{ + Plan: authorOnlyPlan(1), ExpectedPlanRevision: 0, ExpectedSelectionVersion: 1, + }); err != nil { + t.Fatalf("save: %v", err) + } + preview, err := svc.ActivationPreview(ctx, 93, 1, true) + if err != nil { + t.Fatalf("preview: %v", err) + } + // 总开关再次开启但 revision 前进（false→true 也会 bump），旧 token 必须作废 + pol.set(true, 6) + _, err = svc.Activate(ctx, 93, 1, true, ActivateInput{ + ConfirmationToken: preview.ConfirmationToken, + ExpectedProjectVersion: preview.ExpectedProjectVersion, + ExpectedPlanRevision: preview.ExpectedPlanRevision, + }) + if !errors.Is(err, ErrAuthorOnlyPolicyChanged) { + t.Fatalf("expected ErrAuthorOnlyPolicyChanged, got %v", err) + } +} + +func TestService_AuthorOnlyForbiddenForNonCreator(t *testing.T) { + ctx := context.Background() + store := newMemStore() + _ = SeedProjectMeta(store, 94, LifecyclePlanning, "", 1, 0) + svc := NewService(store, testResolver(), nil, nil) + svc.SetAuthorOnlyPolicySource(&fakeAuthorOnlyPolicy{allowed: true, revision: 1}) + _, err := svc.SaveWorkflowPlan(ctx, 94, 1, false, SavePlanInput{ + Plan: authorOnlyPlan(1), ExpectedPlanRevision: 0, ExpectedSelectionVersion: 1, + }) + if !errors.Is(err, ErrForbiddenNotCreator) { + t.Fatalf("expected ErrForbiddenNotCreator, got %v", err) + } +} + +func TestPreviewSummary_AuthorOnlySplit(t *testing.T) { + version := optionalDutyResolver().versions[validPlan().WorkflowDefinitionKey] + plan := optionalDutyPlan( + DocDuty{AuthorUserID: 1, ReviewerUserID: 2, ApproverUserID: 3}, + DocDuty{}, + ) + plan.Stages[0].DocTasks[0].DutyPolicy = DutyPolicyAuthorOnly + plan.Stages[0].DocTasks[0].Duties = DocDuty{AuthorUserID: 9} + sum := previewSummary(plan, version) + if sum.AuthorOnlyDocTasks != 1 || sum.GovernedDocTasks != 1 || sum.UnmanagedDocTasks != 1 { + t.Fatalf("summary: %+v", sum) + } + if len(sum.AuthorOnlyDocuments) != 1 || sum.AuthorOnlyDocuments[0].AuthorUserID != 9 { + t.Fatalf("author_only docs: %+v", sum.AuthorOnlyDocuments) + } +} diff --git a/internal/server/project_settings_author_only_error_test.go b/internal/server/project_settings_author_only_error_test.go new file mode 100644 index 00000000..34349efd --- /dev/null +++ b/internal/server/project_settings_author_only_error_test.go @@ -0,0 +1,50 @@ +package server + +import ( + "errors" + "net/http" + "net/http/httptest" + "testing" + + "chat-agent-system/internal/projectinstance" +) + +func TestWriteProjectInstanceError_AuthorOnlyCodes(t *testing.T) { + cases := []struct { + err error + code int + body string + }{ + {projectinstance.ErrAuthorOnlyPolicyDisabled, http.StatusForbidden, "AUTHOR_ONLY_POLICY_DISABLED"}, + {projectinstance.ErrAuthorOnlyPolicyChanged, http.StatusConflict, "AUTHOR_ONLY_POLICY_CHANGED"}, + {projectinstance.ErrAuthorOnlyPolicyUnavailable, http.StatusServiceUnavailable, "AUTHOR_ONLY_POLICY_UNAVAILABLE"}, + } + for _, tc := range cases { + w := httptest.NewRecorder() + writeProjectInstanceError(w, tc.err) + if w.Code != tc.code { + t.Fatalf("%v: status %d want %d body=%s", tc.err, w.Code, tc.code, w.Body.String()) + } + if !contains(w.Body.String(), tc.body) { + t.Fatalf("%v: body %s missing %s", tc.err, w.Body.String(), tc.body) + } + } + // wrapped unavailable + w := httptest.NewRecorder() + writeProjectInstanceError(w, errors.Join(projectinstance.ErrAuthorOnlyPolicyUnavailable, errors.New("db down"))) + if w.Code != http.StatusServiceUnavailable { + t.Fatalf("wrapped unavailable: %d %s", w.Code, w.Body.String()) + } +} + +func contains(s, sub string) bool { + return len(s) >= len(sub) && (s == sub || len(sub) == 0 || + (func() bool { + for i := 0; i+len(sub) <= len(s); i++ { + if s[i:i+len(sub)] == sub { + return true + } + } + return false + })()) +}