From 0aae84d0a80a3f673322765de9fd10026dbc7d24 Mon Sep 17 00:00:00 2001
From: weichen <weichen@example.com>
Date: Sun, 13 Sep 2026 10:10:56 +0000
Subject: [PATCH] fix(docgov): remove PutChangeProposal memory-seed helper

Do not expose a production path that mutates only the in-memory proposal
index. Cover decision idempotency via ProposeModification+Decide and the
existing package MarkProposalApplied tests instead.

Co-authored-by: Cursor <cursoragent@cursor.com>
---
 internal/docgov/lease.go                      | 16 -----
 .../server/document_revision_authz_test.go    | 63 ++++++++++++++-----
 2 files changed, 48 insertions(+), 31 deletions(-)

diff --git a/internal/docgov/lease.go b/internal/docgov/lease.go
index b807349b..e35dadd5 100644
--- a/internal/docgov/lease.go
+++ b/internal/docgov/lease.go
@@ -310,22 +310,6 @@ func (e *GovernanceEngine) MarkProposalApplied(ctx context.Context, proposalID s
 	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 {
diff --git a/internal/server/document_revision_authz_test.go b/internal/server/document_revision_authz_test.go
index e2be585b..368a2a59 100644
--- a/internal/server/document_revision_authz_test.go
+++ b/internal/server/document_revision_authz_test.go
@@ -8,9 +8,7 @@ import (
 	"net/http/httptest"
 	"strconv"
 	"testing"
-	"time"
 
-	"chat-agent-system/internal/docgov"
 	"chat-agent-system/internal/docrevision"
 )
 
@@ -252,20 +250,55 @@ func TestDocRevision_CommitWithMissingProposalReturnsGovernanceError(t *testing.
 	if body2["code"] != "DOCUMENT_GOVERNANCE_FAILED" {
 		t.Fatalf("retry code=%v body=%#v", body2["code"], body2)
 	}
+}
 
-	// Compensate: seed approved proposal with the same ID, then same decision succeeds.
-	now := time.Now()
-	srv.svc.DocGov().PutChangeProposal(&docgov.ChangeProposal{
-		ProposalID: ghostProp, DocID: docID, ProjectID: 1,
-		Status: docgov.ProposalApprovedByAuthor, CreatedAt: now,
+// Happy path: real ProposeModification + Decide, then decision + same-decision retry succeed.
+func TestDocRevision_DecisionIdempotentWithRealProposal(t *testing.T) {
+	srv := newTestServer(t)
+	ctx := context.Background()
+	docID := "doc_proj1_gov_prop_ok"
+	if _, err := srv.svc.DocGov().CreateOrGetDocument(ctx, docID, 1, "intake", "摸底", "V0.1", "", 1, "admin", 2, "r", 3, "a"); err != nil {
+		t.Fatal(err)
+	}
+	prop, err := srv.svc.DocGov().ProposeModification(ctx, docID, "agent", "改", "影响", "风险")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := srv.svc.DocGov().DecideModificationProposal(ctx, prop.ProposalID, 1, true, "同意"); err != nil {
+		t.Fatal(err)
+	}
+	st, err := srv.svc.DocRevision().OpenTemplateVersion(ctx, docID, "# base\n\n法规 GB9706\n", docrevision.ActorSnapshot{}, 1, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	cand, _, err := srv.svc.DocRevision().CreateAgentCandidate(ctx, docrevision.AgentCandidateRequest{
+		DocID: docID, RunID: "gov-ok-1", ProposalID: prop.ProposalID,
+		InputSnapshotID: st.HeadSnapshotID,
+		OutputMarkdown:  "# base\n\n", ActorKind: docrevision.ActorAgentSquad,
 	})
-	wFix := httptest.NewRecorder()
-	decFix := httptest.NewRequest(http.MethodPost,
-		"/api/v1/documents/"+docID+"/agent-candidates/"+strconv.FormatInt(cand.ID, 10)+"/decision",
-		bytes.NewReader(bodyJSON))
-	withBearerAuth(decFix, token)
-	srv.Routes().ServeHTTP(wFix, decFix)
-	if wFix.Code != http.StatusOK {
-		t.Fatalf("compensating retry want 200 got %d %s", wFix.Code, wFix.Body.String())
+	if err != nil && cand == nil {
+		t.Fatal(err)
+	}
+	if cand.Status != docrevision.CandidateNeedsConfirmation {
+		t.Fatalf("want needs_confirmation got %s", cand.Status)
+	}
+
+	token := loginTestAdmin(t, srv)
+	bodyJSON := []byte(`{"decision":"commit","confirmed":true,"expected_input_sha":"` + st.HeadSHA256 + `"}`)
+	path := "/api/v1/documents/" + docID + "/agent-candidates/" + strconv.FormatInt(cand.ID, 10) + "/decision"
+	for i, label := range []string{"first", "retry"} {
+		req := httptest.NewRequest(http.MethodPost, path, bytes.NewReader(bodyJSON))
+		withBearerAuth(req, token)
+		w := httptest.NewRecorder()
+		srv.Routes().ServeHTTP(w, req)
+		if w.Code != http.StatusOK {
+			t.Fatalf("%s decision want 200 got %d %s", label, w.Code, w.Body.String())
+		}
+		if i == 0 {
+			got, err := srv.svc.DocRevision().GetCandidate(ctx, docID, cand.ID)
+			if err != nil || got.Status != docrevision.CandidateCommitted {
+				t.Fatalf("after first: %+v err=%v", got, err)
+			}
+		}
 	}
 }