Compare commits
5 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 02bdd701a5 | |||
| 23dc781908 | |||
| 1960d987ed | |||
| dca260f582 | |||
| 921599542d |
@@ -104,10 +104,6 @@ inputs:
|
|||||||
description: 'Path to custom persona JSON file'
|
description: 'Path to custom persona JSON file'
|
||||||
required: false
|
required: false
|
||||||
default: ''
|
default: ''
|
||||||
action-repo:
|
|
||||||
description: 'Repository hosting the review-bot binary (owner/name). Defaults to rodin/review-bot on Gitea, or strat/review-bot on GitHub.'
|
|
||||||
required: false
|
|
||||||
default: ''
|
|
||||||
|
|
||||||
runs:
|
runs:
|
||||||
using: 'composite'
|
using: 'composite'
|
||||||
@@ -116,21 +112,10 @@ runs:
|
|||||||
id: version
|
id: version
|
||||||
shell: bash
|
shell: bash
|
||||||
run: |
|
run: |
|
||||||
SERVER_URL="${{ inputs.gitea-url || github.server_url }}"
|
GITEA_URL="${{ inputs.gitea-url || github.server_url }}"
|
||||||
# Detect VCS type: Gitea uses /api/v1/, GitHub uses /api/v3/
|
REPO="${{ inputs.repo || 'rodin/review-bot' }}"
|
||||||
if echo "$SERVER_URL" | grep -qi 'gitea'; then
|
|
||||||
API_BASE="${SERVER_URL}/api/v1"
|
|
||||||
DEFAULT_ACTION_REPO="rodin/review-bot"
|
|
||||||
else
|
|
||||||
API_BASE="${SERVER_URL}/api/v3"
|
|
||||||
DEFAULT_ACTION_REPO="strat/review-bot"
|
|
||||||
fi
|
|
||||||
ACTION_REPO="${{ inputs.action-repo || '' }}"
|
|
||||||
if [ -z "$ACTION_REPO" ]; then
|
|
||||||
ACTION_REPO="$DEFAULT_ACTION_REPO"
|
|
||||||
fi
|
|
||||||
if [ "${{ inputs.version }}" = "latest" ]; then
|
if [ "${{ inputs.version }}" = "latest" ]; then
|
||||||
VERSION=$(curl -sSf "${API_BASE}/repos/${ACTION_REPO}/releases?limit=1" \
|
VERSION=$(curl -sSf "${GITEA_URL}/api/v1/repos/${REPO}/releases?limit=1" \
|
||||||
| python3 -c "import sys, json; releases = json.load(sys.stdin); print(releases[0]['tag_name'] if releases else '')")
|
| python3 -c "import sys, json; releases = json.load(sys.stdin); print(releases[0]['tag_name'] if releases else '')")
|
||||||
if [ -z "$VERSION" ]; then
|
if [ -z "$VERSION" ]; then
|
||||||
echo "Failed to determine latest version" >&2
|
echo "Failed to determine latest version" >&2
|
||||||
@@ -140,8 +125,6 @@ runs:
|
|||||||
VERSION="${{ inputs.version }}"
|
VERSION="${{ inputs.version }}"
|
||||||
fi
|
fi
|
||||||
echo "version=${VERSION}" >> "$GITHUB_OUTPUT"
|
echo "version=${VERSION}" >> "$GITHUB_OUTPUT"
|
||||||
echo "action-repo=${ACTION_REPO}" >> "$GITHUB_OUTPUT"
|
|
||||||
echo "server-url=${SERVER_URL}" >> "$GITHUB_OUTPUT"
|
|
||||||
|
|
||||||
- name: Cache review-bot binary
|
- name: Cache review-bot binary
|
||||||
id: cache
|
id: cache
|
||||||
@@ -154,14 +137,14 @@ runs:
|
|||||||
if: steps.cache.outputs.cache-hit != 'true'
|
if: steps.cache.outputs.cache-hit != 'true'
|
||||||
shell: bash
|
shell: bash
|
||||||
run: |
|
run: |
|
||||||
SERVER_URL="${{ steps.version.outputs.server-url }}"
|
GITEA_URL="${{ inputs.gitea-url || github.server_url }}"
|
||||||
ACTION_REPO="${{ steps.version.outputs.action-repo }}"
|
REPO="${{ inputs.repo || 'rodin/review-bot' }}"
|
||||||
VERSION="${{ steps.version.outputs.version }}"
|
VERSION="${{ steps.version.outputs.version }}"
|
||||||
BINARY="review-bot-linux-amd64"
|
BINARY="review-bot-linux-amd64"
|
||||||
|
|
||||||
curl -sSfL "${SERVER_URL}/${ACTION_REPO}/releases/download/${VERSION}/${BINARY}" \
|
curl -sSfL "${GITEA_URL}/${REPO}/releases/download/${VERSION}/${BINARY}" \
|
||||||
-o "${{ runner.temp }}/review-bot"
|
-o "${{ runner.temp }}/review-bot"
|
||||||
curl -sSfL "${SERVER_URL}/${ACTION_REPO}/releases/download/${VERSION}/checksums.txt" \
|
curl -sSfL "${GITEA_URL}/${REPO}/releases/download/${VERSION}/checksums.txt" \
|
||||||
-o "${{ runner.temp }}/checksums.txt"
|
-o "${{ runner.temp }}/checksums.txt"
|
||||||
|
|
||||||
# Verify SHA-256 checksum
|
# Verify SHA-256 checksum
|
||||||
@@ -186,8 +169,8 @@ runs:
|
|||||||
- name: Run review
|
- name: Run review
|
||||||
shell: bash
|
shell: bash
|
||||||
env:
|
env:
|
||||||
GITHUB_SERVER_URL: ${{ inputs.gitea-url || github.server_url }}
|
GITEA_URL: ${{ inputs.gitea-url || github.server_url }}
|
||||||
GITHUB_REPOSITORY: ${{ inputs.repo || github.repository }}
|
GITEA_REPO: ${{ inputs.repo || github.repository }}
|
||||||
PR_NUMBER: ${{ inputs.pr-number || github.event.pull_request.number }}
|
PR_NUMBER: ${{ inputs.pr-number || github.event.pull_request.number }}
|
||||||
REVIEWER_TOKEN: ${{ inputs.reviewer-token }}
|
REVIEWER_TOKEN: ${{ inputs.reviewer-token }}
|
||||||
REVIEWER_NAME: ${{ inputs.reviewer-name }}
|
REVIEWER_NAME: ${{ inputs.reviewer-name }}
|
||||||
|
|||||||
@@ -104,10 +104,6 @@ inputs:
|
|||||||
description: 'Path to custom persona JSON file'
|
description: 'Path to custom persona JSON file'
|
||||||
required: false
|
required: false
|
||||||
default: ''
|
default: ''
|
||||||
action-repo:
|
|
||||||
description: 'Repository hosting the review-bot binary (owner/name). Defaults to rodin/review-bot on Gitea, or strat/review-bot on GitHub.'
|
|
||||||
required: false
|
|
||||||
default: ''
|
|
||||||
|
|
||||||
runs:
|
runs:
|
||||||
using: 'composite'
|
using: 'composite'
|
||||||
@@ -116,21 +112,10 @@ runs:
|
|||||||
id: version
|
id: version
|
||||||
shell: bash
|
shell: bash
|
||||||
run: |
|
run: |
|
||||||
SERVER_URL="${{ inputs.gitea-url || github.server_url }}"
|
GITEA_URL="${{ inputs.gitea-url || github.server_url }}"
|
||||||
# Detect VCS type: Gitea uses /api/v1/, GitHub uses /api/v3/
|
REPO="${{ inputs.repo || 'rodin/review-bot' }}"
|
||||||
if echo "$SERVER_URL" | grep -qi 'gitea'; then
|
|
||||||
API_BASE="${SERVER_URL}/api/v1"
|
|
||||||
DEFAULT_ACTION_REPO="rodin/review-bot"
|
|
||||||
else
|
|
||||||
API_BASE="${SERVER_URL}/api/v3"
|
|
||||||
DEFAULT_ACTION_REPO="strat/review-bot"
|
|
||||||
fi
|
|
||||||
ACTION_REPO="${{ inputs.action-repo || '' }}"
|
|
||||||
if [ -z "$ACTION_REPO" ]; then
|
|
||||||
ACTION_REPO="$DEFAULT_ACTION_REPO"
|
|
||||||
fi
|
|
||||||
if [ "${{ inputs.version }}" = "latest" ]; then
|
if [ "${{ inputs.version }}" = "latest" ]; then
|
||||||
VERSION=$(curl -sSf "${API_BASE}/repos/${ACTION_REPO}/releases?limit=1" \
|
VERSION=$(curl -sSf "${GITEA_URL}/api/v1/repos/${REPO}/releases?limit=1" \
|
||||||
| python3 -c "import sys, json; releases = json.load(sys.stdin); print(releases[0]['tag_name'] if releases else '')")
|
| python3 -c "import sys, json; releases = json.load(sys.stdin); print(releases[0]['tag_name'] if releases else '')")
|
||||||
if [ -z "$VERSION" ]; then
|
if [ -z "$VERSION" ]; then
|
||||||
echo "Failed to determine latest version" >&2
|
echo "Failed to determine latest version" >&2
|
||||||
@@ -140,8 +125,6 @@ runs:
|
|||||||
VERSION="${{ inputs.version }}"
|
VERSION="${{ inputs.version }}"
|
||||||
fi
|
fi
|
||||||
echo "version=${VERSION}" >> "$GITHUB_OUTPUT"
|
echo "version=${VERSION}" >> "$GITHUB_OUTPUT"
|
||||||
echo "action-repo=${ACTION_REPO}" >> "$GITHUB_OUTPUT"
|
|
||||||
echo "server-url=${SERVER_URL}" >> "$GITHUB_OUTPUT"
|
|
||||||
|
|
||||||
- name: Cache review-bot binary
|
- name: Cache review-bot binary
|
||||||
id: cache
|
id: cache
|
||||||
@@ -154,14 +137,14 @@ runs:
|
|||||||
if: steps.cache.outputs.cache-hit != 'true'
|
if: steps.cache.outputs.cache-hit != 'true'
|
||||||
shell: bash
|
shell: bash
|
||||||
run: |
|
run: |
|
||||||
SERVER_URL="${{ steps.version.outputs.server-url }}"
|
GITEA_URL="${{ inputs.gitea-url || github.server_url }}"
|
||||||
ACTION_REPO="${{ steps.version.outputs.action-repo }}"
|
REPO="${{ inputs.repo || 'rodin/review-bot' }}"
|
||||||
VERSION="${{ steps.version.outputs.version }}"
|
VERSION="${{ steps.version.outputs.version }}"
|
||||||
BINARY="review-bot-linux-amd64"
|
BINARY="review-bot-linux-amd64"
|
||||||
|
|
||||||
curl -sSfL "${SERVER_URL}/${ACTION_REPO}/releases/download/${VERSION}/${BINARY}" \
|
curl -sSfL "${GITEA_URL}/${REPO}/releases/download/${VERSION}/${BINARY}" \
|
||||||
-o "${{ runner.temp }}/review-bot"
|
-o "${{ runner.temp }}/review-bot"
|
||||||
curl -sSfL "${SERVER_URL}/${ACTION_REPO}/releases/download/${VERSION}/checksums.txt" \
|
curl -sSfL "${GITEA_URL}/${REPO}/releases/download/${VERSION}/checksums.txt" \
|
||||||
-o "${{ runner.temp }}/checksums.txt"
|
-o "${{ runner.temp }}/checksums.txt"
|
||||||
|
|
||||||
# Verify SHA-256 checksum
|
# Verify SHA-256 checksum
|
||||||
|
|||||||
@@ -1,47 +0,0 @@
|
|||||||
# Self-review workflow for strat/review-bot on GitHub Enterprise Server.
|
|
||||||
# Tests that the composite action runs correctly on GitHub runners:
|
|
||||||
# - GITHUB_SERVER_URL and GITHUB_REPOSITORY env vars are set correctly
|
|
||||||
# - Binary is downloaded from gitea.weiker.me (where releases live)
|
|
||||||
# - Review is posted to the corresponding Gitea PR
|
|
||||||
name: Review
|
|
||||||
|
|
||||||
on:
|
|
||||||
pull_request:
|
|
||||||
types: [opened, synchronize]
|
|
||||||
|
|
||||||
jobs:
|
|
||||||
review:
|
|
||||||
runs-on: ubuntu-24.04
|
|
||||||
if: github.event_name == 'pull_request'
|
|
||||||
strategy:
|
|
||||||
matrix:
|
|
||||||
include:
|
|
||||||
- name: sonnet
|
|
||||||
token_secret: SONNET_REVIEW_TOKEN
|
|
||||||
model: anthropic--claude-4.6-sonnet
|
|
||||||
- name: gpt
|
|
||||||
token_secret: GPT_REVIEW_TOKEN
|
|
||||||
model: gpt-5
|
|
||||||
steps:
|
|
||||||
- uses: actions/checkout@v4
|
|
||||||
- name: Run ${{ matrix.name }} review
|
|
||||||
uses: ./.gitea/actions/review
|
|
||||||
with:
|
|
||||||
# Download binary from Gitea (releases live there, not on GHE)
|
|
||||||
gitea-url: https://gitea.weiker.me
|
|
||||||
# Post review to the corresponding Gitea repo
|
|
||||||
repo: rodin/review-bot
|
|
||||||
reviewer-token: ${{ secrets[matrix.token_secret] }}
|
|
||||||
reviewer-name: ${{ matrix.name }}
|
|
||||||
llm-model: ${{ matrix.model }}
|
|
||||||
llm-provider: aicore
|
|
||||||
aicore-client-id: ${{ secrets.AICORE_CLIENT_ID }}
|
|
||||||
aicore-client-secret: ${{ secrets.AICORE_CLIENT_SECRET }}
|
|
||||||
aicore-auth-url: ${{ secrets.AICORE_AUTH_URL }}
|
|
||||||
aicore-api-url: ${{ secrets.AICORE_API_URL }}
|
|
||||||
aicore-resource-group: ${{ secrets.AICORE_RESOURCE_GROUP }}
|
|
||||||
conventions-file: CONVENTIONS.md
|
|
||||||
patterns-repo: rodin/go-patterns
|
|
||||||
patterns-files: 'README.md,patterns/'
|
|
||||||
dry-run: 'true'
|
|
||||||
timeout: '600'
|
|
||||||
+11
-4
@@ -11,6 +11,7 @@ import (
|
|||||||
type PositionMap struct {
|
type PositionMap struct {
|
||||||
// files maps filename → (position → new-file line number).
|
// files maps filename → (position → new-file line number).
|
||||||
// Deletion lines are mapped to -1 (no new-file line).
|
// Deletion lines are mapped to -1 (no new-file line).
|
||||||
|
// Hunk-header lines are mapped to 0 (no new-file line).
|
||||||
files map[string]map[int]int
|
files map[string]map[int]int
|
||||||
// maxPositions caches the highest position number per file,
|
// maxPositions caches the highest position number per file,
|
||||||
// tracked during construction to avoid O(n) scans at translate time.
|
// tracked during construction to avoid O(n) scans at translate time.
|
||||||
@@ -19,8 +20,8 @@ type PositionMap struct {
|
|||||||
|
|
||||||
// Translate converts a GitHub diff-position to a new-file line number for a given file.
|
// Translate converts a GitHub diff-position to a new-file line number for a given file.
|
||||||
// Returns an error if the file is not in the diff or the position is out of range.
|
// Returns an error if the file is not in the diff or the position is out of range.
|
||||||
// If the position targets a deletion line, it maps to the nearest non-deletion line below;
|
// If the position targets a deletion or hunk-header line, it maps to the nearest
|
||||||
// if no such line exists, returns an error.
|
// context/addition line below; if no such line exists, returns an error.
|
||||||
func (pm *PositionMap) Translate(file string, position int) (int, error) {
|
func (pm *PositionMap) Translate(file string, position int) (int, error) {
|
||||||
if pm == nil || pm.files == nil {
|
if pm == nil || pm.files == nil {
|
||||||
return 0, fmt.Errorf("empty position map")
|
return 0, fmt.Errorf("empty position map")
|
||||||
@@ -41,14 +42,18 @@ func (pm *PositionMap) Translate(file string, position int) (int, error) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// lineNum == -1 means this position is a deletion line.
|
// lineNum == -1 means this position is a deletion line.
|
||||||
// Map to the nearest non-deletion line below.
|
// lineNum == 0 means this position is a hunk-header line.
|
||||||
if lineNum == -1 {
|
// Both map to the nearest context/addition line below.
|
||||||
|
if lineNum <= 0 {
|
||||||
maxPos := pm.maxPosition(file)
|
maxPos := pm.maxPosition(file)
|
||||||
for p := position + 1; p <= maxPos; p++ {
|
for p := position + 1; p <= maxPos; p++ {
|
||||||
if ln, exists := fileMap[p]; exists && ln > 0 {
|
if ln, exists := fileMap[p]; exists && ln > 0 {
|
||||||
return ln, nil
|
return ln, nil
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if lineNum == 0 {
|
||||||
|
return 0, fmt.Errorf("position %d targets a hunk-header line with no subsequent new-file line in %q", position, file)
|
||||||
|
}
|
||||||
return 0, fmt.Errorf("position %d targets a deletion line with no subsequent new-file line in %q", position, file)
|
return 0, fmt.Errorf("position %d targets a deletion line with no subsequent new-file line in %q", position, file)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -70,6 +75,7 @@ func (pm *PositionMap) maxPosition(file string) int {
|
|||||||
// - A new @@ hunk within the same file continues incrementing (does not reset)
|
// - A new @@ hunk within the same file continues incrementing (does not reset)
|
||||||
// - Position maps to the new file line number for additions and context lines
|
// - Position maps to the new file line number for additions and context lines
|
||||||
// - Deletion lines have a position but no new-file line number (stored as -1)
|
// - Deletion lines have a position but no new-file line number (stored as -1)
|
||||||
|
// - Hunk-header lines have a position but no new-file line number (stored as 0)
|
||||||
func BuildPositionToLineMap(diff string) *PositionMap {
|
func BuildPositionToLineMap(diff string) *PositionMap {
|
||||||
pm := &PositionMap{
|
pm := &PositionMap{
|
||||||
files: make(map[string]map[int]int),
|
files: make(map[string]map[int]int),
|
||||||
@@ -126,6 +132,7 @@ func BuildPositionToLineMap(diff string) *PositionMap {
|
|||||||
// Parse hunk headers
|
// Parse hunk headers
|
||||||
if strings.HasPrefix(line, "@@") && currentFile != "" {
|
if strings.HasPrefix(line, "@@") && currentFile != "" {
|
||||||
position++
|
position++
|
||||||
|
pm.files[currentFile][position] = 0 // sentinel: hunk-header has no new-file line
|
||||||
pm.maxPositions[currentFile] = position
|
pm.maxPositions[currentFile] = position
|
||||||
newLine = parseHunkStart(line)
|
newLine = parseHunkStart(line)
|
||||||
continue
|
continue
|
||||||
|
|||||||
@@ -272,3 +272,112 @@ diff --git a/b.go b/b.go
|
|||||||
t.Errorf("Translate(b.go, 3) = %d, want 2", got)
|
t.Errorf("Translate(b.go, 3) = %d, want 2", got)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestTranslate_HunkHeaderPosition_SingleHunk(t *testing.T) {
|
||||||
|
// Position 1 is the @@ hunk-header line.
|
||||||
|
// It should resolve to the first context/addition line below (new line 16).
|
||||||
|
diff := `diff --git a/file.go b/file.go
|
||||||
|
index abc..def 100644
|
||||||
|
--- a/file.go
|
||||||
|
+++ b/file.go
|
||||||
|
@@ -16,4 +16,5 @@ func example() {
|
||||||
|
context line
|
||||||
|
-deleted line
|
||||||
|
+added line
|
||||||
|
context after
|
||||||
|
`
|
||||||
|
pm := BuildPositionToLineMap(diff)
|
||||||
|
|
||||||
|
got, err := pm.Translate("file.go", 1)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Translate(file.go, 1): unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if got != 16 {
|
||||||
|
t.Errorf("Translate(file.go, 1) = %d, want 16 (first context/addition line in hunk)", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestTranslate_HunkHeaderPosition_MultiHunk(t *testing.T) {
|
||||||
|
// First hunk: @@ is pos 1, then " line1" (pos 2), "-old" (pos 3), "+new" (pos 4)
|
||||||
|
// Second hunk: @@ is pos 5, then " func foo() {" (pos 6), "+// added" (pos 7), etc.
|
||||||
|
// Translating position 5 (second @@) should resolve to new line 10.
|
||||||
|
diff := `diff --git a/file.go b/file.go
|
||||||
|
--- a/file.go
|
||||||
|
+++ b/file.go
|
||||||
|
@@ -1,3 +1,3 @@ package main
|
||||||
|
line1
|
||||||
|
-old
|
||||||
|
+new
|
||||||
|
@@ -10,3 +10,4 @@ func foo() {
|
||||||
|
func foo() {
|
||||||
|
+ // added
|
||||||
|
return
|
||||||
|
}
|
||||||
|
`
|
||||||
|
pm := BuildPositionToLineMap(diff)
|
||||||
|
|
||||||
|
// Position 5 is the second @@ hunk-header — should resolve to new line 10
|
||||||
|
got, err := pm.Translate("file.go", 5)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Translate(file.go, 5): unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if got != 10 {
|
||||||
|
t.Errorf("Translate(file.go, 5) = %d, want 10 (first context/addition line in second hunk)", got)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Also verify first hunk header at position 1 resolves to new line 1
|
||||||
|
got, err = pm.Translate("file.go", 1)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Translate(file.go, 1): unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if got != 1 {
|
||||||
|
t.Errorf("Translate(file.go, 1) = %d, want 1 (first context/addition line in first hunk)", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestTranslate_HunkHeaderPosition_NewFile(t *testing.T) {
|
||||||
|
// New file: @@ -0,0 +1,3 @@ is position 1.
|
||||||
|
// Should resolve to new line 1 (the first addition).
|
||||||
|
diff := `diff --git a/new.go b/new.go
|
||||||
|
new file mode 100644
|
||||||
|
--- /dev/null
|
||||||
|
+++ b/new.go
|
||||||
|
@@ -0,0 +1,3 @@
|
||||||
|
+package main
|
||||||
|
+
|
||||||
|
+func init() {}
|
||||||
|
`
|
||||||
|
pm := BuildPositionToLineMap(diff)
|
||||||
|
|
||||||
|
got, err := pm.Translate("new.go", 1)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Translate(new.go, 1): unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if got != 1 {
|
||||||
|
t.Errorf("Translate(new.go, 1) = %d, want 1 (first addition line)", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestTranslate_HunkHeaderAtEnd(t *testing.T) {
|
||||||
|
// A hunk-header at the last position with no subsequent new-file line should error.
|
||||||
|
// This is the hunk-header equivalent of TestBuildPositionToLineMap_DeletionAtEnd.
|
||||||
|
diff := `diff --git a/file.go b/file.go
|
||||||
|
--- a/file.go
|
||||||
|
+++ b/file.go
|
||||||
|
@@ -1,2 +1,2 @@ package main
|
||||||
|
line1
|
||||||
|
-old
|
||||||
|
+new
|
||||||
|
@@ -10,2 +10,1 @@ func foo() {
|
||||||
|
-removed
|
||||||
|
`
|
||||||
|
pm := BuildPositionToLineMap(diff)
|
||||||
|
|
||||||
|
// Position 5 is the second @@ hunk-header; the only line after it (pos 6) is a
|
||||||
|
// deletion (lineNum == -1), so there's no positive new-file line to resolve to.
|
||||||
|
// The hunk-header lookup should fail.
|
||||||
|
_, err := pm.Translate("file.go", 5)
|
||||||
|
if err == nil {
|
||||||
|
t.Error("expected error for hunk-header at end with no subsequent new-file line")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -5,6 +5,9 @@ import (
|
|||||||
"gitea.weiker.me/rodin/review-bot/vcs"
|
"gitea.weiker.me/rodin/review-bot/vcs"
|
||||||
)
|
)
|
||||||
|
|
||||||
// Compile-time interface conformance assertion.
|
// Compile-time interface conformance assertions.
|
||||||
// Verifies github.Client satisfies vcs.PRReader.
|
// These verify github.Client satisfies vcs.PRReader and vcs.FileReader.
|
||||||
var _ vcs.PRReader = (*github.Client)(nil)
|
var (
|
||||||
|
_ vcs.PRReader = (*github.Client)(nil)
|
||||||
|
_ vcs.FileReader = (*github.Client)(nil)
|
||||||
|
)
|
||||||
|
|||||||
@@ -8,8 +8,16 @@ import (
|
|||||||
"net/url"
|
"net/url"
|
||||||
"path"
|
"path"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
|
"gitea.weiker.me/rodin/review-bot/vcs"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// GetFileContent fetches a file from a repo at the given ref.
|
||||||
|
// Delegates to GetFileContentAtRef with the provided ref.
|
||||||
|
func (c *Client) GetFileContent(ctx context.Context, owner, repo, filePath, ref string) (string, error) {
|
||||||
|
return c.GetFileContentAtRef(ctx, owner, repo, filePath, ref)
|
||||||
|
}
|
||||||
|
|
||||||
// GetFileContentAtRef fetches a file at a specific ref from a repo.
|
// GetFileContentAtRef fetches a file at a specific ref from a repo.
|
||||||
// If ref is empty, the query parameter is omitted (uses default branch).
|
// If ref is empty, the query parameter is omitted (uses default branch).
|
||||||
//
|
//
|
||||||
@@ -46,6 +54,58 @@ func (c *Client) GetFileContentAtRef(ctx context.Context, owner, repo, filePath,
|
|||||||
return decoded, nil
|
return decoded, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ListContents lists files and directories at a given path in a repo.
|
||||||
|
// Returns the directory listing from the GitHub contents API.
|
||||||
|
// If the path points to a single file (not a directory), the API returns
|
||||||
|
// a JSON object instead of an array; this is handled by returning a
|
||||||
|
// single-element slice.
|
||||||
|
func (c *Client) ListContents(ctx context.Context, owner, repo, filePath string) ([]vcs.ContentEntry, error) {
|
||||||
|
escaped, err := escapePath(filePath)
|
||||||
|
if err != nil {
|
||||||
|
return nil, fmt.Errorf("invalid file path: %w", err)
|
||||||
|
}
|
||||||
|
reqURL := fmt.Sprintf("%s/repos/%s/%s/contents/%s",
|
||||||
|
c.baseURL, url.PathEscape(owner), url.PathEscape(repo), escaped)
|
||||||
|
body, err := c.doGet(ctx, reqURL)
|
||||||
|
if err != nil {
|
||||||
|
return nil, fmt.Errorf("list contents %s: %w", filePath, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
type entry struct {
|
||||||
|
Name string `json:"name"`
|
||||||
|
Path string `json:"path"`
|
||||||
|
Type string `json:"type"`
|
||||||
|
}
|
||||||
|
|
||||||
|
// The GitHub contents API returns an array for directories and an object
|
||||||
|
// for single files. Try array first (common case), then fall back to object.
|
||||||
|
// An empty array ([]) is valid — it represents an empty directory — and
|
||||||
|
// results in a zero-length slice returned without error.
|
||||||
|
var entries []entry
|
||||||
|
if err := json.Unmarshal(body, &entries); err != nil {
|
||||||
|
var single entry
|
||||||
|
if err2 := json.Unmarshal(body, &single); err2 != nil {
|
||||||
|
return nil, fmt.Errorf("parse contents JSON: as array: %v; as object: %w", err, err2)
|
||||||
|
}
|
||||||
|
// Guard against empty objects ({}) or unexpected shapes that
|
||||||
|
// unmarshal successfully but carry no useful data.
|
||||||
|
if single.Name == "" && single.Path == "" && single.Type == "" {
|
||||||
|
return nil, fmt.Errorf("parse contents JSON: unexpected response format")
|
||||||
|
}
|
||||||
|
entries = []entry{single}
|
||||||
|
}
|
||||||
|
|
||||||
|
result := make([]vcs.ContentEntry, len(entries))
|
||||||
|
for i, e := range entries {
|
||||||
|
result[i] = vcs.ContentEntry{
|
||||||
|
Name: e.Name,
|
||||||
|
Path: e.Path,
|
||||||
|
Type: e.Type,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return result, nil
|
||||||
|
}
|
||||||
|
|
||||||
// escapePath validates and encodes a slash-separated file path for use in
|
// escapePath validates and encodes a slash-separated file path for use in
|
||||||
// GitHub API URLs. Returns an error if the path contains dot-segments ("."
|
// GitHub API URLs. Returns an error if the path contains dot-segments ("."
|
||||||
// or "..") or resolves to a path outside the repository root.
|
// or "..") or resolves to a path outside the repository root.
|
||||||
|
|||||||
+311
-2
@@ -2,12 +2,291 @@ package github
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"encoding/json"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
"time"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
func TestGetFileContent_DelegatesToGetFileContentAtRef(t *testing.T) {
|
||||||
|
var gotRef string
|
||||||
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
gotRef = r.URL.Query().Get("ref")
|
||||||
|
json.NewEncoder(w).Encode(map[string]string{
|
||||||
|
"content": "dGVzdA==", // "test" in base64
|
||||||
|
"encoding": "base64",
|
||||||
|
})
|
||||||
|
}))
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
c := NewClient("token", srv.URL, AllowInsecureHTTP())
|
||||||
|
c.SetHTTPClient(srv.Client())
|
||||||
|
|
||||||
|
// Call with empty ref — should not include ref param
|
||||||
|
content, err := c.GetFileContent(context.Background(), "owner", "repo", "file.go", "")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if content != "test" {
|
||||||
|
t.Errorf("expected 'test', got %q", content)
|
||||||
|
}
|
||||||
|
if gotRef != "" {
|
||||||
|
t.Errorf("expected empty ref, got %q", gotRef)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestGetFileContent_WithRef(t *testing.T) {
|
||||||
|
var gotRef string
|
||||||
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
gotRef = r.URL.Query().Get("ref")
|
||||||
|
json.NewEncoder(w).Encode(map[string]string{
|
||||||
|
"content": "dGVzdA==",
|
||||||
|
"encoding": "base64",
|
||||||
|
})
|
||||||
|
}))
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
c := NewClient("token", srv.URL, AllowInsecureHTTP())
|
||||||
|
c.SetHTTPClient(srv.Client())
|
||||||
|
|
||||||
|
_, err := c.GetFileContent(context.Background(), "owner", "repo", "file.go", "abc123")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if gotRef != "abc123" {
|
||||||
|
t.Errorf("expected ref 'abc123', got %q", gotRef)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestGetFileContent_404(t *testing.T) {
|
||||||
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
w.WriteHeader(404)
|
||||||
|
w.Write([]byte(`{"message":"Not Found"}`))
|
||||||
|
}))
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
c := NewClient("token", srv.URL, AllowInsecureHTTP())
|
||||||
|
c.SetHTTPClient(srv.Client())
|
||||||
|
|
||||||
|
_, err := c.GetFileContent(context.Background(), "owner", "repo", "missing.go", "")
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected error for 404")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestGetFileContent_401(t *testing.T) {
|
||||||
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
w.WriteHeader(401)
|
||||||
|
w.Write([]byte(`{"message":"Bad credentials"}`))
|
||||||
|
}))
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
c := NewClient("token", srv.URL, AllowInsecureHTTP())
|
||||||
|
c.SetHTTPClient(srv.Client())
|
||||||
|
|
||||||
|
_, err := c.GetFileContent(context.Background(), "owner", "repo", "file.go", "")
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected error for 401")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestGetFileContent_429Retry(t *testing.T) {
|
||||||
|
attempts := 0
|
||||||
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
attempts++
|
||||||
|
if attempts == 1 {
|
||||||
|
w.WriteHeader(429)
|
||||||
|
w.Write([]byte(`{"message":"rate limit"}`))
|
||||||
|
return
|
||||||
|
}
|
||||||
|
json.NewEncoder(w).Encode(map[string]string{
|
||||||
|
"content": "b2s=",
|
||||||
|
"encoding": "base64",
|
||||||
|
})
|
||||||
|
}))
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
c := NewClient("token", srv.URL, AllowInsecureHTTP())
|
||||||
|
c.SetHTTPClient(srv.Client())
|
||||||
|
if err := c.SetRetryBackoff([]time.Duration{1 * time.Millisecond, 1 * time.Millisecond}); err != nil {
|
||||||
|
t.Fatalf("SetRetryBackoff: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
content, err := c.GetFileContent(context.Background(), "owner", "repo", "file.go", "")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if content != "ok" {
|
||||||
|
t.Errorf("expected 'ok', got %q", content)
|
||||||
|
}
|
||||||
|
if attempts != 2 {
|
||||||
|
t.Errorf("expected 2 attempts, got %d", attempts)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestGetFileContent_MalformedJSON(t *testing.T) {
|
||||||
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
w.WriteHeader(200)
|
||||||
|
w.Write([]byte(`not json`))
|
||||||
|
}))
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
c := NewClient("token", srv.URL, AllowInsecureHTTP())
|
||||||
|
c.SetHTTPClient(srv.Client())
|
||||||
|
|
||||||
|
_, err := c.GetFileContent(context.Background(), "owner", "repo", "file.go", "")
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected error for malformed JSON")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestListContents_HappyPath(t *testing.T) {
|
||||||
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
if r.URL.Path != "/repos/owner/repo/contents/src" {
|
||||||
|
t.Errorf("unexpected path: %s", r.URL.Path)
|
||||||
|
}
|
||||||
|
json.NewEncoder(w).Encode([]map[string]string{
|
||||||
|
{"name": "main.go", "path": "src/main.go", "type": "file"},
|
||||||
|
{"name": "lib", "path": "src/lib", "type": "dir"},
|
||||||
|
})
|
||||||
|
}))
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
c := NewClient("token", srv.URL, AllowInsecureHTTP())
|
||||||
|
c.SetHTTPClient(srv.Client())
|
||||||
|
|
||||||
|
entries, err := c.ListContents(context.Background(), "owner", "repo", "src")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if len(entries) != 2 {
|
||||||
|
t.Fatalf("expected 2 entries, got %d", len(entries))
|
||||||
|
}
|
||||||
|
if entries[0].Name != "main.go" {
|
||||||
|
t.Errorf("expected name 'main.go', got %q", entries[0].Name)
|
||||||
|
}
|
||||||
|
if entries[0].Path != "src/main.go" {
|
||||||
|
t.Errorf("expected path 'src/main.go', got %q", entries[0].Path)
|
||||||
|
}
|
||||||
|
if entries[0].Type != "file" {
|
||||||
|
t.Errorf("expected type 'file', got %q", entries[0].Type)
|
||||||
|
}
|
||||||
|
if entries[1].Name != "lib" {
|
||||||
|
t.Errorf("expected name 'lib', got %q", entries[1].Name)
|
||||||
|
}
|
||||||
|
if entries[1].Type != "dir" {
|
||||||
|
t.Errorf("expected type 'dir', got %q", entries[1].Type)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestListContents_404(t *testing.T) {
|
||||||
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
w.WriteHeader(404)
|
||||||
|
w.Write([]byte(`{"message":"Not Found"}`))
|
||||||
|
}))
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
c := NewClient("token", srv.URL, AllowInsecureHTTP())
|
||||||
|
c.SetHTTPClient(srv.Client())
|
||||||
|
|
||||||
|
_, err := c.ListContents(context.Background(), "owner", "repo", "missing")
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected error for 404")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestListContents_401(t *testing.T) {
|
||||||
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
w.WriteHeader(401)
|
||||||
|
w.Write([]byte(`{"message":"Bad credentials"}`))
|
||||||
|
}))
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
c := NewClient("token", srv.URL, AllowInsecureHTTP())
|
||||||
|
c.SetHTTPClient(srv.Client())
|
||||||
|
|
||||||
|
_, err := c.ListContents(context.Background(), "owner", "repo", "src")
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected error for 401")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestListContents_429Retry(t *testing.T) {
|
||||||
|
attempts := 0
|
||||||
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
attempts++
|
||||||
|
if attempts == 1 {
|
||||||
|
w.WriteHeader(429)
|
||||||
|
w.Write([]byte(`{"message":"rate limit"}`))
|
||||||
|
return
|
||||||
|
}
|
||||||
|
json.NewEncoder(w).Encode([]map[string]string{
|
||||||
|
{"name": "file.go", "path": "file.go", "type": "file"},
|
||||||
|
})
|
||||||
|
}))
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
c := NewClient("token", srv.URL, AllowInsecureHTTP())
|
||||||
|
c.SetHTTPClient(srv.Client())
|
||||||
|
if err := c.SetRetryBackoff([]time.Duration{1 * time.Millisecond, 1 * time.Millisecond}); err != nil {
|
||||||
|
t.Fatalf("SetRetryBackoff: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
entries, err := c.ListContents(context.Background(), "owner", "repo", "src")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if len(entries) != 1 {
|
||||||
|
t.Fatalf("expected 1 entry, got %d", len(entries))
|
||||||
|
}
|
||||||
|
if attempts != 2 {
|
||||||
|
t.Errorf("expected 2 attempts, got %d", attempts)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestListContents_MalformedJSON(t *testing.T) {
|
||||||
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
w.WriteHeader(200)
|
||||||
|
w.Write([]byte(`not json`))
|
||||||
|
}))
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
c := NewClient("token", srv.URL, AllowInsecureHTTP())
|
||||||
|
c.SetHTTPClient(srv.Client())
|
||||||
|
|
||||||
|
_, err := c.ListContents(context.Background(), "owner", "repo", "src")
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected error for malformed JSON")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestListContents_SingleFile(t *testing.T) {
|
||||||
|
// GitHub Contents API returns a JSON object (not array) for single-file paths
|
||||||
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
w.WriteHeader(200)
|
||||||
|
w.Write([]byte(`{"name":"README.md","path":"README.md","type":"file"}`))
|
||||||
|
}))
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
c := NewClient("token", srv.URL, AllowInsecureHTTP())
|
||||||
|
c.SetHTTPClient(srv.Client())
|
||||||
|
entries, err := c.ListContents(context.Background(), "owner", "repo", "README.md")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if len(entries) != 1 {
|
||||||
|
t.Fatalf("expected 1 entry, got %d", len(entries))
|
||||||
|
}
|
||||||
|
if entries[0].Name != "README.md" {
|
||||||
|
t.Errorf("expected name 'README.md', got %q", entries[0].Name)
|
||||||
|
}
|
||||||
|
if entries[0].Type != "file" {
|
||||||
|
t.Errorf("expected type 'file', got %q", entries[0].Type)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestEscapePath_ValidPaths(t *testing.T) {
|
func TestEscapePath_ValidPaths(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
@@ -68,7 +347,7 @@ func TestGetFileContentAtRef_DotSegmentError(t *testing.T) {
|
|||||||
}))
|
}))
|
||||||
defer srv.Close()
|
defer srv.Close()
|
||||||
|
|
||||||
c := NewClient("token", srv.URL)
|
c := NewClient("token", srv.URL, AllowInsecureHTTP())
|
||||||
_, err := c.GetFileContentAtRef(context.Background(), "owner", "repo", "foo/../bar.go", "main")
|
_, err := c.GetFileContentAtRef(context.Background(), "owner", "repo", "foo/../bar.go", "main")
|
||||||
if err == nil {
|
if err == nil {
|
||||||
t.Fatal("expected error for path with dot-segments")
|
t.Fatal("expected error for path with dot-segments")
|
||||||
@@ -78,6 +357,37 @@ func TestGetFileContentAtRef_DotSegmentError(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestDecodeBase64Content(t *testing.T) {
|
||||||
|
// Test with newlines (GitHub's format)
|
||||||
|
encoded := "cGFja2FnZSBt\nYWlu"
|
||||||
|
decoded, err := decodeBase64Content(encoded)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if decoded != "package main" {
|
||||||
|
t.Errorf("expected 'package main', got %q", decoded)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestDecodeBase64Content_Invalid(t *testing.T) {
|
||||||
|
_, err := decodeBase64Content("not!!!valid!!!base64")
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected error for invalid base64")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestDecodeBase64Content_CRLF(t *testing.T) {
|
||||||
|
// Base64 of "hello world" with CRLF line breaks inserted
|
||||||
|
encoded := "aGVs\r\nbG8g\r\nd29y\r\nbGQ="
|
||||||
|
decoded, err := decodeBase64Content(encoded)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if decoded != "hello world" {
|
||||||
|
t.Errorf("expected 'hello world', got %q", decoded)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestDecodeBase64Content_SizeLimit(t *testing.T) {
|
func TestDecodeBase64Content_SizeLimit(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
// Create base64 content that would decode to > maxFileContentSize.
|
// Create base64 content that would decode to > maxFileContentSize.
|
||||||
@@ -93,4 +403,3 @@ func TestDecodeBase64Content_SizeLimit(t *testing.T) {
|
|||||||
t.Errorf("expected 'too large' error, got: %v", err)
|
t.Errorf("expected 'too large' error, got: %v", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user