From 0b5f4997e2342bb67cde9ccc221a44dbc73495f5 Mon Sep 17 00:00:00 2001 From: weichen Date: Sun, 13 Sep 2026 10:09:18 +0000 Subject: [PATCH] fix(docgov): mark proposal applied after durable DB write Write applied status to Postgres before mutating memory so a failed UPDATE cannot leave a false in-memory applied state; treat already-applied as idempotent success and expose PutChangeProposal for compensation seeding. Co-authored-by: Cursor --- internal/docgov/engine_test.go | 27 ++++++++++++++++++++ internal/docgov/lease.go | 45 +++++++++++++++++++++++++++++----- 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/internal/docgov/engine_test.go b/internal/docgov/engine_test.go index 5008db44..222de36b 100644 --- a/internal/docgov/engine_test.go +++ b/internal/docgov/engine_test.go @@ -268,4 +268,31 @@ func TestMarkProposalApplied_MissingOrNotApproved(t *testing.T) { if err := engine.MarkProposalApplied(ctx, prop.ProposalID); err == nil { t.Fatal("unapproved proposal must not MarkProposalApplied") } + // Unapproved failure must leave status unchanged (no false applied in memory). + if prop.Status == docgov.ProposalApplied { + t.Fatal("memory must not be ProposalApplied after failed mark") + } +} + +func TestMarkProposalApplied_IdempotentAfterApprove(t *testing.T) { + ctx := context.Background() + engine := docgov.NewGovernanceEngine(nil) + docID := "doc_mark_prop_ok" + _, err := engine.CreateOrGetDocument(ctx, docID, 1, "intake", "摸底", "V0.1", "", 1, "a", 2, "r", 3, "p") + if err != nil { + t.Fatal(err) + } + prop, err := engine.ProposeModification(ctx, docID, "agent", "改", "影响", "风险") + if err != nil { + t.Fatal(err) + } + if _, err := engine.DecideModificationProposal(ctx, prop.ProposalID, 1, true, "同意"); err != nil { + t.Fatal(err) + } + if err := engine.MarkProposalApplied(ctx, prop.ProposalID); err != nil { + t.Fatalf("first apply: %v", err) + } + if err := engine.MarkProposalApplied(ctx, prop.ProposalID); err != nil { + t.Fatalf("second apply must be idempotent: %v", err) + } } diff --git a/internal/docgov/lease.go b/internal/docgov/lease.go index 490aa43e..b807349b 100644 --- a/internal/docgov/lease.go +++ b/internal/docgov/lease.go @@ -252,7 +252,8 @@ func (e *GovernanceEngine) FindApprovedProposal(ctx context.Context, docID strin return nil, nil } -// MarkProposalApplied 将提案标记为已应用 +// MarkProposalApplied 将提案标记为已应用。 +// DB 写成功后再更新内存,避免 DB 失败时内存已假 applied;已 applied 则幂等成功。 func (e *GovernanceEngine) MarkProposalApplied(ctx context.Context, proposalID string) error { e.mu.Lock() defer e.mu.Unlock() @@ -275,24 +276,56 @@ func (e *GovernanceEngine) MarkProposalApplied(ctx context.Context, proposalID s if p == nil { return ErrProposalNotFound } - if p.Status != ProposalApprovedByAuthor && p.Status != ProposalApplied { + if p.Status == ProposalApplied { + return nil // idempotent + } + if p.Status != ProposalApprovedByAuthor { return fmt.Errorf("docgov: proposal %s is not approved (status=%s)", proposalID, p.Status) } - p.Status = ProposalApplied + if e.pool != nil { tag, err := e.pool.Exec(ctx, ` - UPDATE document_change_proposals SET status = $1 WHERE proposal_id = $2 - `, string(ProposalApplied), proposalID) + UPDATE document_change_proposals SET status = $1 WHERE proposal_id = $2 AND status = $3 + `, string(ProposalApplied), proposalID, string(ProposalApprovedByAuthor)) if err != nil { return fmt.Errorf("docgov: mark proposal applied: %w", err) } if tag.RowsAffected() == 0 { - return fmt.Errorf("docgov: mark proposal applied: no rows updated for %s", proposalID) + // Concurrent apply or already applied in DB — reload and reconcile memory. + var stat string + qErr := e.pool.QueryRow(ctx, `SELECT status FROM document_change_proposals WHERE proposal_id = $1`, proposalID).Scan(&stat) + if qErr != nil { + return fmt.Errorf("docgov: mark proposal applied: no rows updated for %s", proposalID) + } + if ProposalStatus(stat) == ProposalApplied { + p.Status = ProposalApplied + return nil + } + return fmt.Errorf("docgov: mark proposal applied: no rows updated for %s (status=%s)", proposalID, stat) } } + + // Memory only after durable write succeeds (or in-memory engine with no pool). + p.Status = ProposalApplied return nil } +// PutChangeProposal stores a proposal into the in-memory index (and is a no-op +// when p is nil). Used to compensate after ledger commit when MarkProposalApplied +// failed because the proposal row was missing transiently. +func (e *GovernanceEngine) PutChangeProposal(p *ChangeProposal) { + if e == nil || p == nil || strings.TrimSpace(p.ProposalID) == "" { + return + } + e.mu.Lock() + defer e.mu.Unlock() + cp := *p + if e.memProps == nil { + e.memProps = make(map[string]*ChangeProposal) + } + e.memProps[p.ProposalID] = &cp +} + // AssertHumanCanEdit checks author duty + drafting lifecycle BEFORE acquiring/renewing // the human lease, so unauthorized actors never become lease holders. func (e *GovernanceEngine) AssertHumanCanEdit(ctx context.Context, docID string, actorID int64, actorRole UserRole) error {