fix(docrevision): allow idempotent commit retry after ledger commit
When a candidate is already committed, matching expected hashes return the current revision state so projection and MarkProposalApplied can finish; reject on an already-committed candidate stays an error. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -0,0 +1,74 @@
|
||||
package docrevision_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"chat-agent-system/internal/docrevision"
|
||||
)
|
||||
|
||||
func TestCommitAgentVersion_IdempotentWhenAlreadyCommitted(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
svc := docrevision.NewService(nil)
|
||||
docID := "doc_commit_idem"
|
||||
base := "# base\n\n法规 GB9706\n"
|
||||
st, err := svc.OpenTemplateVersion(ctx, docID, base, docrevision.ActorSnapshot{DisplayName: "t"}, 1, "")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
cand, _, err := svc.CreateAgentCandidate(ctx, docrevision.AgentCandidateRequest{
|
||||
DocID: docID, RunID: "idem-1", InputSnapshotID: st.HeadSnapshotID,
|
||||
OutputMarkdown: "# base\n\n", ActorKind: docrevision.ActorAgentSquad,
|
||||
})
|
||||
if err != nil && cand == nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if cand.Status != docrevision.CandidateNeedsConfirmation {
|
||||
t.Fatalf("want needs_confirmation got %s", cand.Status)
|
||||
}
|
||||
st1, err := svc.CommitAgentVersion(ctx, docID, cand.ID, docrevision.CommitAgentDecision{
|
||||
Decision: "commit", Confirmed: true, ActorUserID: 1,
|
||||
ExpectedInputSHA: st.HeadSHA256,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("first commit: %v", err)
|
||||
}
|
||||
got, err := svc.GetCandidate(ctx, docID, cand.ID)
|
||||
if err != nil || got.Status != docrevision.CandidateCommitted {
|
||||
t.Fatalf("want committed got %+v err=%v", got, err)
|
||||
}
|
||||
|
||||
// Retry with same hashes must succeed (projection/governance compensation path).
|
||||
st2, err := svc.CommitAgentVersion(ctx, docID, cand.ID, docrevision.CommitAgentDecision{
|
||||
Decision: "commit", Confirmed: true, ActorUserID: 1,
|
||||
ExpectedInputSHA: st.HeadSHA256,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("idempotent recommit: %v", err)
|
||||
}
|
||||
if st2 == nil || st2.HeadSHA256 == "" || st2.HeadSHA256 != st1.HeadSHA256 {
|
||||
t.Fatalf("head mismatch after idempotent commit: %#v vs %#v", st2, st1)
|
||||
}
|
||||
|
||||
// Wrong expected output hash must not silently succeed.
|
||||
outSHA := ""
|
||||
if got.OutputSnapshotID != nil {
|
||||
// CommitAgentVersion validates ExpectedOutputSHA against output snapshot.
|
||||
// Use a bogus hash.
|
||||
outSHA = "deadbeef"
|
||||
}
|
||||
_, err = svc.CommitAgentVersion(ctx, docID, cand.ID, docrevision.CommitAgentDecision{
|
||||
Decision: "commit", Confirmed: true, ActorUserID: 1,
|
||||
ExpectedInputSHA: st.HeadSHA256, ExpectedOutputSHA: outSHA,
|
||||
})
|
||||
if err == nil {
|
||||
t.Fatal("mismatched expected_output_sha must fail on idempotent path")
|
||||
}
|
||||
|
||||
_, err = svc.CommitAgentVersion(ctx, docID, cand.ID, docrevision.CommitAgentDecision{
|
||||
Decision: "reject", Confirmed: true, ActorUserID: 1,
|
||||
})
|
||||
if err == nil {
|
||||
t.Fatal("reject after commit must fail")
|
||||
}
|
||||
}
|
||||
@@ -544,6 +544,25 @@ func (s *Service) CommitAgentVersion(ctx context.Context, docID string, candidat
|
||||
return nil, ErrNotFound
|
||||
}
|
||||
if c.Status == CandidateCommitted {
|
||||
// Idempotent retry after projection/governance failure: same candidate + matching
|
||||
// hashes may continue ProjectOutputMarkdown / MarkProposalApplied.
|
||||
if dec.Decision == "reject" {
|
||||
return nil, fmt.Errorf("docrevision: candidate already committed")
|
||||
}
|
||||
if c.OutputSnapshotID == nil {
|
||||
return nil, ErrNotFound
|
||||
}
|
||||
outSnap := s.snapshots[*c.OutputSnapshotID]
|
||||
inSnap := s.snapshots[c.InputSnapshotID]
|
||||
if outSnap == nil || inSnap == nil {
|
||||
return nil, ErrNotFound
|
||||
}
|
||||
if dec.ExpectedInputSHA != "" && dec.ExpectedInputSHA != inSnap.ContentSHA256 {
|
||||
return nil, ErrCandidateStale
|
||||
}
|
||||
if dec.ExpectedOutputSHA != "" && dec.ExpectedOutputSHA != outSnap.ContentSHA256 {
|
||||
return nil, ErrCandidateStale
|
||||
}
|
||||
return s.mustStateLocked(c.DocID), nil
|
||||
}
|
||||
if c.Status == CandidateStale {
|
||||
|
||||
Reference in New Issue
Block a user