docs: document runner requirements for composite action

Add a Runner Requirements section to the README documenting that
the composite action needs python3, sha256sum, and curl on the
runner. All are pre-installed on ubuntu-* runners but custom
images need to provide them.

Closes #12

.gitea/workflows/ci.yml

@@ -33,7 +33,7 @@ jobs:
             token_secret: GPT_REVIEW_TOKEN
             model: gpt-4.1
           - name: security
-            token_secret: SONNET_REVIEW_TOKEN
+            token_secret: SECURITY_REVIEW_TOKEN
             model: gpt-5
             system_prompt_file: SECURITY_REVIEW.md
     steps:

.gitea/workflows/release.yml

@@ -69,14 +69,28 @@ jobs:
 
           echo "Release ID: ${RELEASE_ID}"
 
-          # Upload each asset
+          # Upload each asset (idempotent: delete existing asset with same name first)
           for file in dist/*; do
             filename=$(basename "$file")
             echo "Uploading ${filename}..."
+
+            # Check if asset already exists and delete it
+            EXISTING_ID=$(export ASSET_NAME="${filename}"; curl -sS \
+              -H "Authorization: token ${GITEA_TOKEN}" \
+              "${GITEA_URL}/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets" \
+              | python3 -c "import json,sys,os; name=os.environ['ASSET_NAME']; assets=json.load(sys.stdin); print(next((str(a['id']) for a in assets if a['name']==name),''))" 2>/dev/null)
+
+            if [ -n "$EXISTING_ID" ]; then
+              echo "  Asset ${filename} already exists (id=${EXISTING_ID}), deleting..."
+              curl -sSf -X DELETE \
+                -H "Authorization: token ${GITEA_TOKEN}" \
+                "${GITEA_URL}/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets/${EXISTING_ID}"
+            fi
+
             curl -sSf -X POST \
               -H "Authorization: token ${GITEA_TOKEN}" \
               -H "Content-Type: application/octet-stream" \
-              "${GITEA_URL}/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets?name=${filename}" \
+              "${GITEA_URL}/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets?name=$(printf '%s' "${filename}" | jq -sRr @uri)" \
               --data-binary "@${file}"
           done
 

README.md

@@ -188,6 +188,18 @@ Prints the review to CI logs without posting to the PR. Useful for testing promp
 | `update-existing` | No | `true` | Delete previous review from same bot before posting. Accepts: true/1/yes or false/0/no |
 | `version` | No | `latest` | review-bot version to install |
 
+## Runner Requirements
+
+The composite action requires these tools on the runner:
+
+| Tool | Used For |
+|------|----------|
+| `python3` | JSON parsing during version detection |
+| `sha256sum` | Checksum verification of downloaded binary |
+| `curl` | Downloading releases and querying the API |
+
+All three are pre-installed on `ubuntu-*` runners (e.g. `ubuntu-24.04`). If you use a custom runner image, ensure these are available.
+
 ## How Review Cleanup Works
 
 When `reviewer-name` is set, the bot embeds a hidden sentinel in each review:

cmd/review-bot/main.go

@@ -266,16 +266,13 @@ func main() {
 		if err != nil {
 			log.Printf("Warning: could not list existing reviews: %v", err)
 		} else {
-			// Worst-wins: escalate if a sibling blocks (need own login from existing review)
-			ownLogin := ""
+			// Detect shared-token misconfiguration: if detected, skip all
+			// update logic (PATCH/supersede) to avoid clobbering a sibling's review.
+			sharedToken := hasSharedToken(existingReviews, sentinel)
+			if sharedToken {
+				log.Printf("Shared token mode: skipping update-in-place logic to avoid clobbering sibling review")
+			} else {
 				existing := findOwnReview(existingReviews, sentinel)
-			if existing != nil {
-				ownLogin = existing.User.Login
-			}
-			if event == "APPROVED" && shouldEscalate(existingReviews, 0, ownLogin, sentinel) {
-				log.Printf("Sibling review has REQUEST_CHANGES; escalating to REQUEST_CHANGES")
-				event = "REQUEST_CHANGES"
-			}
 
 				if existing != nil {
 					if reviewUnchanged(existingReviews, reviewBody, event, sentinel) {
@@ -313,6 +310,7 @@ func main() {
 				}
 			}
 		}
+	}
 
 	// POST new review (first run, or state transition fallthrough)
 	log.Printf("Posting review (event=%s)...", event)
@@ -322,29 +320,6 @@ func main() {
 	}
 	log.Printf("Review posted (id=%d, user=%s)", posted.ID, posted.User.Login)
 
-	// Post-posting escalation: if we just posted APPROVED but a sibling
-	// from the same user has REQUEST_CHANGES, mark ours as superseded and
-	// re-post as REQUEST_CHANGES. This handles the first-run case where
-	// we don't know our login until after posting.
-	if event == "APPROVED" && *updateExisting && *reviewerName != "" {
-		reviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber)
-		if err == nil && shouldEscalate(reviews, posted.ID, posted.User.Login, sentinel) {
-			log.Printf("Post-posting escalation: sibling has REQUEST_CHANGES")
-			// Mark our just-posted review as superseded
-			commentID, err := giteaClient.GetTimelineReviewCommentID(ctx, owner, repoName, prNumber, sentinel)
-			if err == nil {
-				supersededBody := fmt.Sprintf("~~*This review has been superseded by a newer review below.*~~\n\n%s", sentinel)
-				giteaClient.EditComment(ctx, owner, repoName, commentID, supersededBody)
-			}
-			// Re-post as REQUEST_CHANGES
-			_, err = giteaClient.PostReview(ctx, owner, repoName, prNumber, "REQUEST_CHANGES", reviewBody, inlineComments)
-			if err != nil {
-				log.Printf("Warning: could not re-post as REQUEST_CHANGES: %v", err)
-			} else {
-				log.Printf("Review escalated to REQUEST_CHANGES")
-			}
-		}
-	}
 }
 
 // fetchFileContext fetches the full content of modified files from the PR branch.
@@ -501,26 +476,6 @@ func validateReviewerName(name string) error {
 	return nil
 }
 
-// shouldEscalate checks if any sibling bot review from the same user
-// (different sentinel, same token) has REQUEST_CHANGES.
-// ownLogin is the bot user login; if empty, escalation check is skipped.
-// postedID is excluded from consideration (0 means no exclusion needed).
-func shouldEscalate(reviews []gitea.Review, postedID int64, ownLogin, ownSentinel string) bool {
-	if ownLogin == "" {
-		return false
-	}
-	for _, r := range reviews {
-		if r.ID == postedID || r.Stale {
-			continue
-		}
-		// Sibling = same user, has a review-bot sentinel, but not OUR sentinel
-		if r.User.Login == ownLogin && r.State == "REQUEST_CHANGES" && strings.Contains(r.Body, "<!-- review-bot:") && !strings.Contains(r.Body, ownSentinel) {
-			return true
-		}
-	}
-	return false
-}
-
 // reviewUnchanged checks if an existing review with the same sentinel
 // already has identical body and state. Returns true if a re-post would
 // produce the same result (skip to preserve conversation threads).
@@ -539,6 +494,46 @@ func reviewUnchanged(reviews []gitea.Review, newBody, newEvent, sentinel string)
 	return false
 }
 
+// hasSharedToken detects if another review-bot role posted under the same
+// Gitea user. This indicates misconfiguration where two roles share a token
+// instead of having separate Gitea accounts. Returns true if shared token
+// detected (caller should skip update-in-place logic to avoid clobbering).
+func hasSharedToken(reviews []gitea.Review, ownSentinel string) bool {
+	ownLogin := ""
+	for _, r := range reviews {
+		if strings.Contains(r.Body, ownSentinel) {
+			ownLogin = r.User.Login
+			break
+		}
+	}
+	if ownLogin == "" {
+		return false
+	}
+	for _, r := range reviews {
+		if r.User.Login == ownLogin && strings.Contains(r.Body, "<!-- review-bot:") && !strings.Contains(r.Body, ownSentinel) {
+			log.Printf("WARNING: shared token detected — another review-bot role (%s) is using the same Gitea user %q. Each role should have its own token/user for proper multi-reviewer blocking.", extractSentinelName(r.Body), ownLogin)
+			return true
+		}
+	}
+	return false
+}
+
+// extractSentinelName pulls the reviewer name from a sentinel comment.
+func extractSentinelName(body string) string {
+	const prefix = "<!-- review-bot:"
+	const suffix = " -->"
+	idx := strings.Index(body, prefix)
+	if idx < 0 {
+		return "unknown"
+	}
+	rest := body[idx+len(prefix):]
+	end := strings.Index(rest, suffix)
+	if end < 0 {
+		return "unknown"
+	}
+	return rest[:end]
+}
+
 // findOwnReview locates a review matching the given sentinel in its body.
 func findOwnReview(reviews []gitea.Review, sentinel string) *gitea.Review {
 	for i := range reviews {

cmd/review-bot/main_test.go

@@ -50,106 +50,6 @@ func makeReview(id int64, login, state string, stale bool, body string) gitea.Re
 	return r
 }
 
-func TestShouldEscalate(t *testing.T) {
-	tests := []struct {
-		name        string
-		reviews     []gitea.Review
-		postedID    int64
-		ownLogin    string
-
-		ownSentinel string
-		want        bool
-	}{
-		{
-			name:        "no reviews",
-			reviews:     nil,
-			postedID:    100,
-			ownLogin:    "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
-		},
-		{
-			name: "sibling same user has REQUEST_CHANGES",
-			reviews: []gitea.Review{
-				makeReview(101, "bot", "REQUEST_CHANGES", false, "bad\n<!-- review-bot:security -->"),
-			},
-			postedID:    100,
-			ownLogin:    "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        true,
-		},
-		{
-			name: "sibling different user has REQUEST_CHANGES (should NOT escalate)",
-			reviews: []gitea.Review{
-				makeReview(101, "other-bot", "REQUEST_CHANGES", false, "bad\n<!-- review-bot:gpt -->"),
-			},
-			postedID:    100,
-			ownLogin:    "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
-		},
-		{
-			name: "same user REQUEST_CHANGES but stale (should NOT escalate)",
-			reviews: []gitea.Review{
-				makeReview(101, "bot", "REQUEST_CHANGES", true, "old\n<!-- review-bot:security -->"),
-			},
-			postedID:    100,
-			ownLogin:    "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
-		},
-		{
-			name: "same user same sentinel (own stale review, should NOT escalate)",
-			reviews: []gitea.Review{
-				makeReview(101, "bot", "REQUEST_CHANGES", false, "old\n<!-- review-bot:sonnet -->"),
-			},
-			postedID:    100,
-			ownLogin:    "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
-		},
-		{
-			name: "same user APPROVED sibling (should NOT escalate)",
-			reviews: []gitea.Review{
-				makeReview(101, "bot", "APPROVED", false, "good\n<!-- review-bot:security -->"),
-			},
-			postedID:    100,
-			ownLogin:    "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
-		},
-		{
-			name: "human REQUEST_CHANGES no sentinel (should NOT escalate)",
-			reviews: []gitea.Review{
-				makeReview(101, "bot", "REQUEST_CHANGES", false, "please fix this"),
-			},
-			postedID:    100,
-			ownLogin:    "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
-		},
-		{
-			name: "skip own posted ID",
-			reviews: []gitea.Review{
-				makeReview(100, "bot", "REQUEST_CHANGES", false, "x\n<!-- review-bot:security -->"),
-			},
-			postedID:    100,
-			ownLogin:    "bot",
-			ownSentinel: "<!-- review-bot:sonnet -->",
-			want:        false,
-		},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			got := shouldEscalate(tc.reviews, tc.postedID, tc.ownLogin, tc.ownSentinel)
-			if got != tc.want {
-				t.Errorf("shouldEscalate() = %v, want %v", got, tc.want)
-			}
-		})
-	}
-}
-
 func TestReviewUnchanged(t *testing.T) {
 	tests := []struct {
 		name     string
@@ -288,3 +188,83 @@ func TestFindOwnReview(t *testing.T) {
 		})
 	}
 }
+
+func TestHasSharedToken(t *testing.T) {
+	tests := []struct {
+		name     string
+		reviews  []gitea.Review
+		sentinel string
+		want     bool
+	}{
+		{
+			name:     "no reviews",
+			reviews:  nil,
+			sentinel: "<!-- review-bot:sonnet -->",
+			want:     false,
+		},
+		{
+			name: "no own review yet - cannot detect",
+			reviews: []gitea.Review{
+				{ID: 1, User: struct{ Login string `json:"login"` }{Login: "other"}, Body: "<!-- review-bot:gpt --> body"},
+			},
+			sentinel: "<!-- review-bot:sonnet -->",
+			want:     false,
+		},
+		{
+			name: "separate users - no shared token",
+			reviews: []gitea.Review{
+				{ID: 1, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:sonnet --> body"},
+				{ID: 2, User: struct{ Login string `json:"login"` }{Login: "security-review-bot"}, Body: "<!-- review-bot:security --> body"},
+			},
+			sentinel: "<!-- review-bot:sonnet -->",
+			want:     false,
+		},
+		{
+			name: "shared token detected - same user different sentinels",
+			reviews: []gitea.Review{
+				{ID: 1, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:sonnet --> body"},
+				{ID: 2, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:security --> body"},
+			},
+			sentinel: "<!-- review-bot:sonnet -->",
+			want:     true,
+		},
+		{
+			name: "three roles same user",
+			reviews: []gitea.Review{
+				{ID: 1, User: struct{ Login string `json:"login"` }{Login: "bot"}, Body: "<!-- review-bot:sonnet --> body"},
+				{ID: 2, User: struct{ Login string `json:"login"` }{Login: "bot"}, Body: "<!-- review-bot:security --> body"},
+				{ID: 3, User: struct{ Login string `json:"login"` }{Login: "bot"}, Body: "<!-- review-bot:gpt --> body"},
+			},
+			sentinel: "<!-- review-bot:sonnet -->",
+			want:     true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := hasSharedToken(tc.reviews, tc.sentinel)
+			if got != tc.want {
+				t.Errorf("hasSharedToken() = %v, want %v", got, tc.want)
+			}
+		})
+	}
+}
+
+func TestExtractSentinelName(t *testing.T) {
+	tests := []struct {
+		body string
+		want string
+	}{
+		{"<!-- review-bot:sonnet --> rest", "sonnet"},
+		{"<!-- review-bot:security --> rest", "security"},
+		{"no sentinel here", "unknown"},
+		{"<!-- review-bot:gpt-review --> rest", "gpt-review"},
+	}
+
+	for _, tc := range tests {
+		got := extractSentinelName(tc.body)
+		if got != tc.want {
+			t.Errorf("extractSentinelName(%q) = %q, want %q", tc.body, got, tc.want)
+		}
+	}
+}

gitea/client.go

@@ -7,6 +7,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -16,6 +17,28 @@ import (
 	"time"
 )
 
+// APIError represents an HTTP error response from the Gitea API.
+// It carries the status code so callers can distinguish between
+// different failure modes (e.g. 404 vs 500).
+type APIError struct {
+	StatusCode int
+	Body       string
+}
+
+func (e *APIError) Error() string {
+	body := e.Body
+	if len(body) > 200 {
+		body = body[:200] + "...(truncated)"
+	}
+	return fmt.Sprintf("HTTP %d: %s", e.StatusCode, body)
+}
+
+// IsNotFound reports whether an error is an API 404 response.
+func IsNotFound(err error) bool {
+	var apiErr *APIError
+	return errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound
+}
+
 // Client interacts with the Gitea API.
 // A Client is safe for concurrent use by multiple goroutines.
 type Client struct {
@@ -201,7 +224,7 @@ func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) {
 
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
 		body, _ := io.ReadAll(resp.Body)
-		return nil, fmt.Errorf("HTTP %d: %s", resp.StatusCode, string(body))
+		return nil, &APIError{StatusCode: resp.StatusCode, Body: string(body)}
 	}
 	return io.ReadAll(resp.Body)
 }
@@ -254,10 +277,15 @@ func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string
 	// Try listing as directory first
 	entries, err := c.ListContents(ctx, owner, repo, path)
 	if err != nil {
-		// Might be a file, try fetching directly
+		// Only fall back to single-file fetch on 404 (path is a file, not a dir).
+		// Propagate all other errors (auth failures, server errors, rate limits).
+		if !IsNotFound(err) {
+			return nil, fmt.Errorf("list contents %q: %w", path, err)
+		}
+		// 404 means the path might be a file — try fetching directly
 		content, fileErr := c.GetFileContent(ctx, owner, repo, path)
 		if fileErr != nil {
-			return nil, fmt.Errorf("path %q is neither a file nor directory: %w", path, err)
+			return nil, fmt.Errorf("path %q is neither a file nor directory: %w", path, fileErr)
 		}
 		results[path] = content
 		return results, nil

gitea/client_test.go

@@ -3,6 +3,7 @@ package gitea
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -505,3 +506,99 @@ func TestGetTimelineReviewCommentID_NotFound(t *testing.T) {
 		t.Fatal("expected error when sentinel not found")
 	}
 }
+
+func TestGetAllFilesInPath_404FallsBackToFile(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/api/v1/repos/owner/repo/contents/README.md":
+			// Contents API returns 404 for files (not a directory)
+			w.WriteHeader(http.StatusNotFound)
+			w.Write([]byte(`{"message":"not found"}`))
+		case "/api/v1/repos/owner/repo/raw/README.md":
+			w.Write([]byte("# Hello\n"))
+		default:
+			w.WriteHeader(http.StatusNotFound)
+			w.Write([]byte(`{"message":"not found"}`))
+		}
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	files, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "README.md")
+	if err != nil {
+		t.Fatalf("expected fallback to file on 404, got error: %v", err)
+	}
+	if len(files) != 1 {
+		t.Fatalf("expected 1 file, got %d", len(files))
+	}
+	if files["README.md"] != "# Hello\n" {
+		t.Errorf("unexpected content: %q", files["README.md"])
+	}
+}
+
+func TestGetAllFilesInPath_500Propagates(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// Simulate a server error from ListContents
+		w.WriteHeader(http.StatusInternalServerError)
+		w.Write([]byte(`{"message":"internal server error"}`))
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	_, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "somepath")
+	if err == nil {
+		t.Fatal("expected error to propagate for 500, got nil")
+	}
+	// Should NOT fall back to file fetch — error should propagate
+	var apiErr *APIError
+	if !errors.As(err, &apiErr) {
+		t.Fatalf("expected APIError in chain, got: %v", err)
+	}
+	if apiErr.StatusCode != http.StatusInternalServerError {
+		t.Errorf("expected status 500, got %d", apiErr.StatusCode)
+	}
+}
+
+func TestGetAllFilesInPath_403Propagates(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusForbidden)
+		w.Write([]byte(`{"message":"token has insufficient scope"}`))
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	_, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "private/stuff")
+	if err == nil {
+		t.Fatal("expected error to propagate for 403, got nil")
+	}
+	var apiErr *APIError
+	if !errors.As(err, &apiErr) {
+		t.Fatalf("expected APIError in chain, got: %v", err)
+	}
+	if apiErr.StatusCode != http.StatusForbidden {
+		t.Errorf("expected status 403, got %d", apiErr.StatusCode)
+	}
+}
+
+func TestIsNotFound(t *testing.T) {
+	tests := []struct {
+		name   string
+		err    error
+		want   bool
+	}{
+		{"nil error", nil, false},
+		{"non-API error", fmt.Errorf("network timeout"), false},
+		{"404 APIError", &APIError{StatusCode: 404, Body: "not found"}, true},
+		{"500 APIError", &APIError{StatusCode: 500, Body: "server error"}, false},
+		{"wrapped 404", fmt.Errorf("list contents: %w", &APIError{StatusCode: 404, Body: "not found"}), true},
+		{"wrapped 500", fmt.Errorf("list contents: %w", &APIError{StatusCode: 500, Body: "err"}), false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := IsNotFound(tt.err)
+			if got != tt.want {
+				t.Errorf("IsNotFound(%v) = %v, want %v", tt.err, got, tt.want)
+			}
+		})
+	}
+}