feat: always post fresh review, supersede old with collapsed body #38
@@ -67,7 +67,6 @@ func main() {
|
|||||||
patternsRepo := flag.String("patterns-repo", envOrDefault("PATTERNS_REPO", ""), "Repo with language patterns (e.g. rodin/elixir-patterns)")
|
patternsRepo := flag.String("patterns-repo", envOrDefault("PATTERNS_REPO", ""), "Repo with language patterns (e.g. rodin/elixir-patterns)")
|
||||||
patternsFiles := flag.String("patterns-files", envOrDefault("PATTERNS_FILES", "README.md"), "Comma-separated file paths to fetch from patterns repo")
|
patternsFiles := flag.String("patterns-files", envOrDefault("PATTERNS_FILES", "README.md"), "Comma-separated file paths to fetch from patterns repo")
|
||||||
dryRun := flag.Bool("dry-run", false, "Print review to stdout instead of posting")
|
dryRun := flag.Bool("dry-run", false, "Print review to stdout instead of posting")
|
||||||
updateExisting := flag.Bool("update-existing", envOrDefaultBool("UPDATE_EXISTING", true), "Delete previous review from same bot before posting (default true)")
|
|
||||||
llmTemp := flag.Float64("llm-temperature", envOrDefaultFloat("LLM_TEMPERATURE", 0), "LLM temperature (0 = server default)")
|
llmTemp := flag.Float64("llm-temperature", envOrDefaultFloat("LLM_TEMPERATURE", 0), "LLM temperature (0 = server default)")
|
||||||
llmTimeout := flag.Int("llm-timeout", envOrDefaultInt("LLM_TIMEOUT", 300), "LLM request timeout in seconds (default 300)")
|
llmTimeout := flag.Int("llm-timeout", envOrDefaultInt("LLM_TIMEOUT", 300), "LLM request timeout in seconds (default 300)")
|
||||||
llmProvider := flag.String("llm-provider", envOrDefault("LLM_PROVIDER", "openai"), "LLM API provider: openai or anthropic")
|
llmProvider := flag.String("llm-provider", envOrDefault("LLM_PROVIDER", "openai"), "LLM API provider: openai or anthropic")
|
||||||
@@ -279,6 +278,16 @@ func main() {
|
|||||||
|
|
||||||
// Step 10: Format and post review
|
// Step 10: Format and post review
|
||||||
reviewBody := review.FormatMarkdown(result, *reviewerName)
|
reviewBody := review.FormatMarkdown(result, *reviewerName)
|
||||||
|
|
||||||
|
// Add commit footer so readers know which commit was evaluated
|
||||||
|
if pr.Head.Sha != "" {
|
||||||
|
shortSHA := pr.Head.Sha
|
||||||
|
if len(shortSHA) > 8 {
|
||||||
|
shortSHA = shortSHA[:8]
|
||||||
|
}
|
||||||
|
reviewBody += fmt.Sprintf("\n\n---\n*Evaluated against %s*", shortSHA)
|
||||||
|
}
|
||||||
|
|
||||||
event := review.GiteaEvent(result.Verdict)
|
event := review.GiteaEvent(result.Verdict)
|
||||||
|
|
||||||
if *dryRun {
|
if *dryRun {
|
||||||
@@ -307,56 +316,33 @@ func main() {
|
|||||||
}
|
}
|
||||||
|
|
|||||||
|
|
||||||
// --- Review update strategy ---
|
// --- Review update strategy ---
|
||||||
// 1. No existing review → POST new
|
// Always post a fresh review on the current commit. If a previous review
|
||||||
// 2. Existing review, same state → PATCH body in place (preserves threads)
|
// exists, mark it as superseded (preserves old threads as navigable history).
|
||||||
// 3. Existing review, state change → PATCH old to "Superseded", POST new
|
// Never skip posting — the fresh review must land on HEAD to clear stale badges.
|
||||||
if *updateExisting && *reviewerName != "" {
|
if *reviewerName != "" {
|
||||||
existingReviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber)
|
existingReviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
slog.Warn("could not list existing reviews", "pr", prNumber, "error", err)
|
slog.Warn("could not list existing reviews", "pr", prNumber, "error", err)
|
||||||
} else {
|
} else {
|
||||||
|
sonnet-review-bot
commented
[MINOR] Selecting which previous review to supersede relies on findOwnReview returning the first match. If ListReviews does not guarantee reverse-chronological ordering, this may supersede an older review instead of the most recent one. Consider choosing the most recent review (e.g., by CreatedAt/ID) or filtering out already-superseded bodies. **[MINOR]** Selecting which previous review to supersede relies on findOwnReview returning the first match. If ListReviews does not guarantee reverse-chronological ordering, this may supersede an older review instead of the most recent one. Consider choosing the most recent review (e.g., by CreatedAt/ID) or filtering out already-superseded bodies.
sonnet-review-bot
commented
[MAJOR] findOwnReviewStrict is called with expectedLogin set to reviewer-name (*reviewerName), but reviews[i].User.Login contains the Gitea account login. This mismatch will typically cause the strict check to filter out all candidate reviews, so older reviews are never marked as superseded. Pass the actual Gitea login (e.g., derived via the sentinel from existing reviews or via a /user API call) or disable the strict check by passing an empty expectedLogin. **[MAJOR]** findOwnReviewStrict is called with expectedLogin set to reviewer-name (*reviewerName), but reviews[i].User.Login contains the Gitea account login. This mismatch will typically cause the strict check to filter out all candidate reviews, so older reviews are never marked as superseded. Pass the actual Gitea login (e.g., derived via the sentinel from existing reviews or via a /user API call) or disable the strict check by passing an empty expectedLogin.
|
|||||||
// Detect shared-token misconfiguration: if detected, skip all
|
// In shared-token mode, skip superseding to avoid clobbering sibling reviews.
|
||||||
// update logic (PATCH/supersede) to avoid clobbering a sibling's review.
|
|
||||||
sharedToken := hasSharedToken(existingReviews, sentinel)
|
sharedToken := hasSharedToken(existingReviews, sentinel)
|
||||||
if sharedToken {
|
if !sharedToken {
|
||||||
slog.Warn("shared token mode: skipping update-in-place logic to avoid clobbering sibling review")
|
|
||||||
} else {
|
|
||||||
existing := findOwnReview(existingReviews, sentinel)
|
existing := findOwnReview(existingReviews, sentinel)
|
||||||
|
|
||||||
if existing != nil {
|
if existing != nil {
|
||||||
if reviewUnchanged(existingReviews, reviewBody, event, sentinel) {
|
commentID, err := giteaClient.GetTimelineReviewCommentID(ctx, owner, repoName, prNumber, sentinel)
|
||||||
slog.Info("review unchanged from previous run; skipping to preserve threads", "pr", prNumber)
|
if err != nil {
|
||||||
return
|
slog.Warn("could not find old review comment ID for supersede", "error", err)
|
||||||
|
sonnet-review-bot
commented
[MINOR] When superseding the previous review, GetTimelineReviewCommentID is used to find a timeline comment containing the sentinel, but it returns the first matching event rather than the latest. If multiple reviews with the same sentinel exist, this may patch an older review instead of the most recent non-superseded one found by findOwnReview. Consider matching the timeline event to the selected review (e.g., by comparing CommitID or body) or selecting the latest matching event by ID. **[MINOR]** When superseding the previous review, GetTimelineReviewCommentID is used to find a timeline comment containing the sentinel, but it returns the first matching event rather than the latest. If multiple reviews with the same sentinel exist, this may patch an older review instead of the most recent non-superseded one found by findOwnReview. Consider matching the timeline event to the selected review (e.g., by comparing CommitID or body) or selecting the latest matching event by ID.
|
|||||||
}
|
|
||||||
|
|
||||||
// Same state → PATCH in place
|
|
||||||
if existing.State == event {
|
|
||||||
commentID, err := giteaClient.GetTimelineReviewCommentID(ctx, owner, repoName, prNumber, sentinel)
|
|
||||||
if err != nil {
|
|
||||||
slog.Warn("could not find review comment ID, falling back to new post", "error", err)
|
|
||||||
} else {
|
|
||||||
if err := giteaClient.EditComment(ctx, owner, repoName, commentID, reviewBody); err != nil {
|
|
||||||
slog.Warn("could not edit review, falling back to new post", "comment_id", commentID, "error", err)
|
|
||||||
} else {
|
|
||||||
slog.Info("review updated in place", "comment_id", commentID, "pr", prNumber)
|
|
||||||
return
|
|
||||||
}
|
|
||||||
}
|
|
||||||
} else {
|
} else {
|
||||||
// State change → mark old as superseded, post new below
|
supersededBody := buildSupersededBody(existing.Body, existing.CommitID, sentinel)
|
||||||
commentID, err := giteaClient.GetTimelineReviewCommentID(ctx, owner, repoName, prNumber, sentinel)
|
if err := giteaClient.EditComment(ctx, owner, repoName, commentID, supersededBody); err != nil {
|
||||||
if err != nil {
|
slog.Warn("could not mark old review as superseded", "comment_id", commentID, "error", err)
|
||||||
slog.Warn("could not find old review comment ID", "error", err)
|
|
||||||
} else {
|
} else {
|
||||||
supersededBody := fmt.Sprintf("~~*This review has been superseded by a newer review below.*~~\n\n%s", sentinel)
|
slog.Info("marked old review as superseded", "old_state", existing.State, "pr", prNumber)
|
||||||
if err := giteaClient.EditComment(ctx, owner, repoName, commentID, supersededBody); err != nil {
|
|
||||||
slog.Warn("could not mark old review as superseded", "comment_id", commentID, "error", err)
|
|
||||||
} else {
|
|
||||||
slog.Info("marked old review as superseded", "old_state", existing.State, "new_state", event, "pr", prNumber)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
} else {
|
||||||
|
slog.Warn("shared token mode: skipping supersede to avoid clobbering sibling review")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -526,22 +512,27 @@ func validateReviewerName(name string) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// reviewUnchanged checks if an existing review with the same sentinel
|
// buildSupersededBody creates the body for a superseded review: struck-through banner
|
||||||
|
sonnet-review-bot
commented
[NIT] If commitSHA is empty, the superseded summary renders as "Previous findings (commit )". Consider omitting the commit text when the SHA is empty to avoid awkward formatting. **[NIT]** If commitSHA is empty, the superseded summary renders as "Previous findings (commit )". Consider omitting the commit text when the SHA is empty to avoid awkward formatting.
|
|||||||
// already has identical body and state. Returns true if a re-post would
|
// with collapsed original content and the commit it was evaluated against.
|
||||||
// produce the same result (skip to preserve conversation threads).
|
func buildSupersededBody(originalBody, commitSHA, sentinel string) string {
|
||||||
func reviewUnchanged(reviews []gitea.Review, newBody, newEvent, sentinel string) bool {
|
shortSHA := commitSHA
|
||||||
|
sonnet-review-bot
commented
[MINOR] buildSupersededBody appends the sentinel to the superseded comment body, which means future runs will still detect the superseded review when looking for the sentinel. This can make selecting the correct review to supersede ambiguous. Consider removing the sentinel from superseded bodies or adding a marker (e.g., "Superseded") that findOwnReview skips. **[MINOR]** buildSupersededBody appends the sentinel to the superseded comment body, which means future runs will still detect the superseded review when looking for the sentinel. This can make selecting the correct review to supersede ambiguous. Consider removing the sentinel from superseded bodies or adding a marker (e.g., "Superseded") that findOwnReview skips.
|
|||||||
for _, r := range reviews {
|
if len(shortSHA) > 8 {
|
||||||
if r.Stale {
|
shortSHA = shortSHA[:8]
|
||||||
continue
|
|
||||||
}
|
|
||||||
if !strings.Contains(r.Body, sentinel) {
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
if r.State == newEvent && r.Body == newBody {
|
|
||||||
return true
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
sonnet-review-bot
commented
[NIT] buildSupersededBody appends the sentinel to the superseded comment. This is useful for diagnostics but also increases the chance that future timeline scans pick a superseded event. Since findOwnReview already skips superseded bodies, consider adjusting GetTimelineReviewCommentID to prefer the latest or match by CommitID to avoid editing superseded entries, even if they contain the sentinel. **[NIT]** buildSupersededBody appends the sentinel to the superseded comment. This is useful for diagnostics but also increases the chance that future timeline scans pick a superseded event. Since findOwnReview already skips superseded bodies, consider adjusting GetTimelineReviewCommentID to prefer the latest or match by CommitID to avoid editing superseded entries, even if they contain the sentinel.
|
|||||||
return false
|
var sb strings.Builder
|
||||||
|
sb.WriteString("~~Original review~~\n\n")
|
||||||
|
sb.WriteString("**Superseded** \u2014 see current review for up-to-date findings.\n\n")
|
||||||
|
if shortSHA != "" {
|
||||||
|
sb.WriteString("<details><summary>Previous findings (commit ")
|
||||||
|
sb.WriteString(shortSHA)
|
||||||
|
sb.WriteString(")</summary>\n\n")
|
||||||
|
} else {
|
||||||
|
sb.WriteString("<details><summary>Previous findings</summary>\n\n")
|
||||||
|
}
|
||||||
|
sb.WriteString(originalBody)
|
||||||
|
sb.WriteString("\n\n</details>\n\n")
|
||||||
|
sb.WriteString(sentinel)
|
||||||
|
return sb.String()
|
||||||
}
|
}
|
||||||
|
|
||||||
// hasSharedToken detects if another review-bot role posted under the same
|
// hasSharedToken detects if another review-bot role posted under the same
|
||||||
@@ -587,10 +578,19 @@ func extractSentinelName(body string) string {
|
|||||||
|
|
||||||
// findOwnReview locates a review matching the given sentinel in its body.
|
// findOwnReview locates a review matching the given sentinel in its body.
|
||||||
func findOwnReview(reviews []gitea.Review, sentinel string) *gitea.Review {
|
func findOwnReview(reviews []gitea.Review, sentinel string) *gitea.Review {
|
||||||
|
var best *gitea.Review
|
||||||
for i := range reviews {
|
for i := range reviews {
|
||||||
if strings.Contains(reviews[i].Body, sentinel) {
|
if !strings.Contains(reviews[i].Body, sentinel) {
|
||||||
return &reviews[i]
|
continue
|
||||||
|
}
|
||||||
|
// Skip superseded reviews (they contain our sentinel in the collapsed body)
|
||||||
|
if strings.Contains(reviews[i].Body, "~~Original review~~") {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
// Take the highest ID (most recent)
|
||||||
|
if best == nil || reviews[i].ID > best.ID {
|
||||||
|
best = &reviews[i]
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return nil
|
return best
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -56,82 +56,48 @@ func makeReview(id int64, login, state string, stale bool, body string) gitea.Re
|
|||||||
return r
|
return r
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestReviewUnchanged(t *testing.T) {
|
|
||||||
tests := []struct {
|
|
||||||
name string
|
|
||||||
existing []gitea.Review
|
|
||||||
newBody string
|
|
||||||
newEvent string
|
|
||||||
sentinel string
|
|
||||||
want bool
|
|
||||||
}{
|
|
||||||
{
|
|
||||||
name: "no existing review",
|
|
||||||
existing: nil,
|
|
||||||
newBody: "new review",
|
|
||||||
newEvent: "APPROVED",
|
|
||||||
sentinel: "<!-- review-bot:sonnet -->",
|
|
||||||
want: false,
|
|
||||||
},
|
|
||||||
{
|
|
||||||
name: "identical body and state",
|
|
||||||
existing: []gitea.Review{
|
|
||||||
makeReview(100, "bot", "APPROVED", false, "same body\n<!-- review-bot:sonnet -->"),
|
|
||||||
},
|
|
||||||
newBody: "same body\n<!-- review-bot:sonnet -->",
|
|
||||||
newEvent: "APPROVED",
|
|
||||||
sentinel: "<!-- review-bot:sonnet -->",
|
|
||||||
want: true,
|
|
||||||
},
|
|
||||||
{
|
|
||||||
name: "same body but different state",
|
|
||||||
existing: []gitea.Review{
|
|
||||||
makeReview(100, "bot", "APPROVED", false, "body\n<!-- review-bot:sonnet -->"),
|
|
||||||
},
|
|
||||||
newBody: "body\n<!-- review-bot:sonnet -->",
|
|
||||||
newEvent: "REQUEST_CHANGES",
|
|
||||||
sentinel: "<!-- review-bot:sonnet -->",
|
|
||||||
want: false,
|
|
||||||
},
|
|
||||||
{
|
|
||||||
name: "different body same state",
|
|
||||||
existing: []gitea.Review{
|
|
||||||
makeReview(100, "bot", "APPROVED", false, "old body\n<!-- review-bot:sonnet -->"),
|
|
||||||
},
|
|
||||||
newBody: "new body\n<!-- review-bot:sonnet -->",
|
|
||||||
newEvent: "APPROVED",
|
|
||||||
sentinel: "<!-- review-bot:sonnet -->",
|
|
||||||
want: false,
|
|
||||||
},
|
|
||||||
{
|
|
||||||
name: "stale review with same body (should still post)",
|
|
||||||
existing: []gitea.Review{
|
|
||||||
makeReview(100, "bot", "APPROVED", true, "same\n<!-- review-bot:sonnet -->"),
|
|
||||||
},
|
|
||||||
newBody: "same\n<!-- review-bot:sonnet -->",
|
|
||||||
newEvent: "APPROVED",
|
|
||||||
sentinel: "<!-- review-bot:sonnet -->",
|
|
||||||
want: false,
|
|
||||||
},
|
|
||||||
{
|
|
||||||
name: "different sentinel (not our review)",
|
|
||||||
existing: []gitea.Review{
|
|
||||||
makeReview(100, "bot", "APPROVED", false, "body\n<!-- review-bot:gpt -->"),
|
|
||||||
},
|
|
||||||
newBody: "body\n<!-- review-bot:sonnet -->",
|
|
||||||
newEvent: "APPROVED",
|
|
||||||
sentinel: "<!-- review-bot:sonnet -->",
|
|
||||||
want: false,
|
|
||||||
},
|
|
||||||
}
|
|
||||||
|
|
||||||
for _, tc := range tests {
|
func TestBuildSupersededBody(t *testing.T) {
|
||||||
t.Run(tc.name, func(t *testing.T) {
|
original := "# Review\n\nLooks good.\n\n<!-- review-bot:sonnet -->"
|
||||||
got := reviewUnchanged(tc.existing, tc.newBody, tc.newEvent, tc.sentinel)
|
sentinel := "<!-- review-bot:sonnet -->"
|
||||||
if got != tc.want {
|
|
||||||
t.Errorf("reviewUnchanged() = %v, want %v", got, tc.want)
|
result := buildSupersededBody(original, "abcdef1234567890", sentinel)
|
||||||
}
|
|
||||||
})
|
// Should contain the struck-through banner
|
||||||
|
if !strings.Contains(result, "~~Original review~~") {
|
||||||
|
t.Error("missing struck-through banner")
|
||||||
|
}
|
||||||
|
// Should contain superseded notice
|
||||||
|
if !strings.Contains(result, "**Superseded**") {
|
||||||
|
t.Error("missing superseded notice")
|
||||||
|
}
|
||||||
|
// Should contain collapsed original
|
||||||
|
if !strings.Contains(result, "<details>") {
|
||||||
|
t.Error("missing details/collapse")
|
||||||
|
}
|
||||||
|
// Should contain short commit SHA
|
||||||
|
if !strings.Contains(result, "abcdef12") {
|
||||||
|
t.Error("missing short SHA")
|
||||||
|
}
|
||||||
|
// Should NOT contain full SHA
|
||||||
|
if strings.Contains(result, "abcdef1234567890") {
|
||||||
|
t.Error("should truncate SHA to 8 chars")
|
||||||
|
}
|
||||||
|
// Should contain the original body inside details
|
||||||
|
if !strings.Contains(result, original) {
|
||||||
|
t.Error("original body not preserved in collapsed section")
|
||||||
|
}
|
||||||
|
// Should end with sentinel
|
||||||
|
if !strings.Contains(result, sentinel) {
|
||||||
|
t.Error("missing sentinel")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestBuildSupersededBodyShortSHA(t *testing.T) {
|
||||||
|
// Short SHA should pass through without panic
|
||||||
|
result := buildSupersededBody("body", "abc", "<!-- review-bot:x -->")
|
||||||
|
if !strings.Contains(result, "abc") {
|
||||||
|
t.Error("short SHA not preserved")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -174,6 +140,32 @@ func TestFindOwnReview(t *testing.T) {
|
|||||||
sentinel: "<!-- review-bot:sonnet -->",
|
sentinel: "<!-- review-bot:sonnet -->",
|
||||||
wantID: 20,
|
wantID: 20,
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "skips superseded review",
|
||||||
|
reviews: []gitea.Review{
|
||||||
|
makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n**Superseded**\n<!-- review-bot:sonnet -->"),
|
||||||
|
makeReview(20, "bot", "APPROVED", false, "fresh review\n<!-- review-bot:sonnet -->"),
|
||||||
|
},
|
||||||
|
sentinel: "<!-- review-bot:sonnet -->",
|
||||||
|
wantID: 20,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "only superseded reviews exist",
|
||||||
|
reviews: []gitea.Review{
|
||||||
|
makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n<!-- review-bot:sonnet -->"),
|
||||||
|
},
|
||||||
|
sentinel: "<!-- review-bot:sonnet -->",
|
||||||
|
wantNil: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "picks highest ID among matches",
|
||||||
|
reviews: []gitea.Review{
|
||||||
|
makeReview(50, "bot", "APPROVED", false, "v1\n<!-- review-bot:sonnet -->"),
|
||||||
|
makeReview(30, "bot", "APPROVED", false, "v0\n<!-- review-bot:sonnet -->"),
|
||||||
|
},
|
||||||
|
sentinel: "<!-- review-bot:sonnet -->",
|
||||||
|
wantID: 50,
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, tc := range tests {
|
for _, tc := range tests {
|
||||||
|
|||||||
@@ -316,13 +316,14 @@ func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string
|
|||||||
|
|
||||||
// Review represents a pull request review from the Gitea API.
|
// Review represents a pull request review from the Gitea API.
|
||||||
type Review struct {
|
type Review struct {
|
||||||
ID int64 `json:"id"`
|
ID int64 `json:"id"`
|
||||||
Body string `json:"body"`
|
Body string `json:"body"`
|
||||||
User struct {
|
User struct {
|
||||||
Login string `json:"login"`
|
Login string `json:"login"`
|
||||||
} `json:"user"`
|
} `json:"user"`
|
||||||
State string `json:"state"`
|
State string `json:"state"`
|
||||||
Stale bool `json:"stale"`
|
Stale bool `json:"stale"`
|
||||||
|
CommitID string `json:"commit_id"`
|
||||||
}
|
}
|
||||||
|
|
||||||
// ListReviews returns all reviews on a pull request.
|
// ListReviews returns all reviews on a pull request.
|
||||||
|
|||||||
[MINOR] Supersede logic identifies prior review and corresponding timeline event solely by a sentinel substring without confirming authorship. Although the Gitea API should prevent editing comments not owned by the token, an attacker could craft a review containing the sentinel and cause unnecessary edit attempts. Consider verifying that the located review/timeline event belongs to the authenticated user (e.g., fetch current user and compare to Review.User.Login) before calling EditComment.