The binary detects VCS type from VCS_TYPE env var, but action.yml did not
pass it to the Run review step. This caused the binary to fall back to a
URL heuristic (github.com substring), which misclassifies GitHub Enterprise
Server hosts whose URL does not contain 'github'.
The 'Determine version' step already outputs vcs_type — wire it through to
the Run review env block so explicit VCS_TYPE always takes precedence.
Wire up the new GitHub API methods to the review-bot CLI via VCS
type detection. review-bot can now review PRs on both Gitea and
GitHub (including GitHub Enterprise Server).
Changes:
- vcs.go: define vcsClient interface with all PR operations
- giteaVCSAdapter: wraps gitea.Client, satisfies vcsClient + giteaExtClient
- githubVCSAdapter: wraps github.Client, satisfies vcsClient
- giteaExtClient: Gitea-specific extension (supersede, comment resolution)
- main.go: detect VCS type via VCS_TYPE env var (auto-detects github.com URLs)
- Creates appropriate client (gitea or github) based on vcs_type
- GitHub API URL derived from server URL (github.com → api.github.com,
GHES → /api/v3)
- All main flow uses vcsClient interface
- Gitea-specific supersede operations gated via giteaExtClient type assertion
- GitHub: logs info when skipping supersede (not supported)
- Removes old giteaClientAdapter (replaced by giteaVCSAdapter in vcs.go)
- giteaVCSAdapter satisfies review.GiteaClient for persona loading
GitHub limitations handled gracefully:
- Review supersede skipped (GitHub doesn't allow editing submitted reviews)
- DeleteReview returns error for non-pending reviews (documented in adapter)
- Inline comments use absolute line + side='RIGHT' instead of diff position
Closes#130.
Co-authored-by: Rodin <[email protected]>
Implement the higher-level GitHub API methods that were TODO since
issue #120. The github package now provides:
- GetPullRequest: PR metadata (title, body, head SHA/ref, draft)
- GetPullRequestDiff: unified diff via Accept: application/vnd.github.diff
- GetPullRequestFiles: changed files list (paginated, 100/page)
- GetCommitStatuses: CI statuses (GitHub uses 'state' field, normalized)
- GetFileContent: file content with base64 decode (strips embedded newlines)
- GetFileContentRef: file at a specific ref
- ListContents: directory listing or single-file normalization
- GetAllFilesInPath: recursive file fetching
- PostReview: submit review with event/body/commit/inline comments
- ListReviews: list PR reviews (paginated)
- DeleteReview: delete review (GitHub only allows PENDING deletion)
- GetAuthenticatedUser: returns login of the authed user
- RequestReviewer: add a user as requested reviewer
API types added: PullRequest, CommitStatus, ChangedFile, ReviewComment,
Review, ContentEntry.
Notable edge cases handled:
- GitHub embeds newlines in base64 content; stripped before decode
- GetFileContent returns error for non-file paths (type=dir)
- ListContents normalizes single-file response to a slice
- DeleteReview documents GitHub's PENDING-only constraint
Removes TODO comment from baseURL field (now consumed by all methods).
Closes part of #130.
Co-authored-by: Rodin <[email protected]>
Python's ipaddress module does NOT classify 100.64.0.0/10 (RFC6598
carrier-grade NAT) as private/loopback/link_local/multicast/reserved.
This means a SERVER_URL resolving to a CGN address would bypass the
Python SSRF check and reach curl with ACTION_TOKEN.
Add an explicit network membership check for 100.64.0.0/10 to both
Python validation blocks in action.yml:
- _ssrf_check.py (VCS URL pre-flight check)
- _ssrf_check_install.py (binary download URL check)
The Go-level IsBlockedIP already covers this range correctly (ipcheck.go);
this fix closes the gap in the action.yml Python layer.
Also update comments to mention RFC6598 explicitly.
- Add --vcs-url flag as primary (reads VCS_URL env var)
- Keep --gitea-url and GITEA_URL as deprecated fallbacks with warnings
- Update action.yml: rename gitea-url input to vcs-url, pass VCS_URL to binary
- Update ci.yml: use VCS_URL env var in Run review step
- Update integration tests: INTEGRATION_GITEA_URL -> INTEGRATION_VCS_URL
- Update README: --vcs-url / VCS_URL with fallback note in env var table
Backward compat: existing GITEA_URL users get a deprecation warning and
continue to work unchanged until they migrate to VCS_URL.
Add defensive check for empty Name and Path fields when unmarshaling
a single ContentEntry in the fallback path. While Gitea API won't
return empty objects for valid file paths, this guard:
- Explicitly documents the invariant we expect
- Catches potential API behavior changes early
- Costs nothing at runtime
Addresses [MINOR] from sonnet-review-bot on PR #74.
When ListContents is called with a path that points to a file (not a
directory), Gitea returns a single JSON object instead of an array.
Previously this caused json.Unmarshal to fail with:
json: cannot unmarshal object into Go value of type []gitea.ContentEntry
Now ListContents tries array unmarshal first, and falls back to single
object unmarshal, wrapping it in a slice. This allows patterns-files
config to specify individual files like 'README.md' without triggering
a parse error.
Also updates TestGetAllFilesInPath_File to reflect actual Gitea behavior
(single object response, not 404).
Fixes#73
Gitea API rejects "." with HTTP 500 (malformed path component).
When patterns-files is set to ".", normalize it to empty string
before making the API call.
Fixes#70
Move lastErr assignment outside the retry condition so that both
network errors and HTTP 5xx paths return lastErr consistently.
Previously, on the final retry attempt, a network error would return
the raw err variable instead of lastErr. While they held the same
value in practice, the inconsistency was confusing when reading the
code.
Now both paths:
- Network errors: assign lastErr before checking retry, return lastErr
- HTTP 5xx: assign lastErr before checking retry, return lastErr
Addresses review finding #3 (MINOR) from sonnet review on PR #69.
1. Fix non-deterministic test TestDoGet_RetriesOnTemporaryNetError:
- Replace timing-dependent listener approach with mockTransport
- mockTransport allows controlled injection of net.OpError failures
- Test now makes deterministic assertions: exactly 3 attempts (2 fail + 1 success)
- Added SetHTTPClient() method for test transport injection
2. Sanitize error content in retry warning logs:
- Added sanitizeErrorForLog() helper that omits response body content
- For APIError: logs only 'HTTP <status>' instead of full body
- For other errors: preserves error type information
- Addresses security concern about logging server error content at WARN level
- Full error with body still returned to caller for proper error handling
Both changes have corresponding test coverage.
Address review feedback on isTemporaryNetError being too broad:
1. RetryBackoff field: Added doc comment clarifying it must be
configured before the first request (addresses concurrency concern).
2. isTemporaryNetError: Now inspects the underlying syscall error
instead of treating all net.OpError as retriable. Only retries on:
- ECONNREFUSED (connection refused)
- ECONNRESET (connection reset)
- ENETUNREACH (network unreachable)
- EHOSTUNREACH (host unreachable)
- ETIMEDOUT (connection timed out)
Permanent errors like EACCES, EPERM are no longer retried.
3. DNS errors: Changed from Temporary() to IsTimeout, since
"no such host" is permanent and shouldn't be retried.
4. Empty backoff slice: Added comment explaining that retry without
delay is intentional when caller explicitly configures it.
Addresses MINOR findings from sonnet-review-bot and gpt-review-bot.
Address review feedback:
1. Make backoff delays injectable via Client.RetryBackoff field
- Defaults to {1s, 2s} when nil for production
- Tests can set shorter values for fast execution
- Fixes slow unit tests that previously waited 3+ seconds
2. Add retry on temporary network errors (net.OpError, net.DNSError)
- Connection refused, network unreachable, DNS failures now retry
- Non-temporary network errors still fail immediately
- Context cancellation still respected during backoff
Added isTemporaryNetError helper and TestIsTemporaryNetError test.
Updated existing retry tests to use configurable short backoffs.
- Remove dead backoff[0] element; array now only contains retry delays
- Fix time.After timer leak by using time.NewTimer with timer.Stop()
- Add io.LimitReader (64KB) for error body reads to bound memory allocation
Addresses feedback from sonnet-review-bot, security-review-bot, and gpt-review-bot.
The read:user scope is needed for the bot to self-request as a
reviewer on PRs. Without it, the bot still functions but cannot
add itself to the reviewer list.
Closes#66
When patterns-repo is configured, now logs at Info level:
- File paths loaded from each repo
- Count of files per repo
At Debug level logs skipped files (non-markdown/txt/yaml).
Warns if no pattern files were loaded from a repo (likely
misconfigured patterns-files path).
Closes#64
1. Remove dead JSON fallback in LoadBuiltinPersona
- The embed directive only includes *.yaml files
- JSON fallback code could never succeed
- Simplified function to only try YAML
2. JSON parsing now rejects unknown fields
- Switched from json.Unmarshal to json.Decoder
- DisallowUnknownFields() matches YAML's KnownFields(true)
- Added test coverage for JSON unknown field rejection
3. Documented symlink support in LoadPersona
- os.Stat follows symlinks, so symlinks to regular files work
- Added doc comment explaining the behavior
- Added test for symlink support
MAJOR fixes:
- Remove false security claim about gopkg.in/yaml.v3 having built-in depth protection
- Add explicit YAML depth limiting via yaml.Node API (MaxYAMLDepth=20)
- Add file size limit for persona files (MaxPersonaFileSize=64KB)
- Add test for deeply nested YAML rejection
MINOR fixes:
- Add sort.Strings to ListBuiltinPersonas for deterministic ordering
- Update design doc to reflect actual library used (gopkg.in/yaml.v3)
- Update README: 'Zero dependencies' → 'Minimal dependencies'
- Add test for file size limit
- Add test for sorted persona list
- Add gopkg.in/yaml.v3 dependency (approved in CONVENTIONS.md)
- Update parsePersona to detect format by file extension
- Support both .yaml and .yml extensions (case-insensitive)
- Convert built-in personas to YAML format
- Add comprehensive tests for YAML parsing
- Update README with YAML examples and documentation
YAML provides cleaner multi-line strings via literal block scalars
and supports comments, making persona definitions more readable.
JSON remains supported for backwards compatibility.
Closes#57
Add native SAP AI Core provider that handles OAuth token management and
deployment discovery automatically. This eliminates the need for the
external LLM proxy when running in SAP environments.
Changes:
- Add AICoreClient with OAuth token caching and deployment URL discovery
- Support both Anthropic and OpenAI models via AI Core deployments
- Update CI to use native AI Core provider
- Update action inputs to accept AI Core credentials
- Update README with AI Core configuration examples
Model names must match AI Core deployment names (e.g. anthropic--claude-4.6-sonnet, gpt-5).
MAJOR fixes:
- Remove external YAML dependency (github.com/goccy/go-yaml)
Per project convention: Go standard library only, zero dependencies.
Convert all persona files from YAML to JSON format.
- Fix TestValidateWorkspacePath error expectation
Go 1.21+ filepath.Join normalizes absolute paths differently.
MINOR fixes:
- Remove custom contains helper in persona_test.go (use strings.Contains)
- Add Unicode-safe CapitalizeFirst function for header titles
- ListBuiltinPersonas returns empty slice instead of nil on error
- Fix test comment about filepath.Join behavior
Documentation:
- Update README to reflect JSON-only persona format
- Update design doc with note about JSON decision
- Fix action.yml description for persona-file input
Add persona system for specialized review roles. Each persona defines:
- A specific review focus (security, architecture, documentation)
- Custom system prompt additions
- Personality/tone adjustments
Built-in personas: security, architect, docs
Custom personas: load from JSON via persona-file flag
Includes workspace validation to prevent path traversal attacks.
Closes#51
When a PR is pushed after being marked self-reviewed, the label is now
stale and should be removed. This matches the gargoyle CI behavior.
On synchronize:
- Remove self-reviewed label if present
- Reassign PR back to the author
- Restore sonnet reviewer with correct model name (anthropic--claude-4.6-sonnet)
- Remove gpt-4.1, gpt-4.1-mini, gpt-5-mini (not deployed on SAP AI Core)
- Keep gpt-5 and security reviewers
The previous model names (claude-sonnet-4-6, etc.) were incorrect —
SAP AI Core uses 'anthropic--claude-4.6-sonnet' format.
Models claude-sonnet-4-6, gpt-4.1, gpt-4.1-mini, and gpt-5-mini are not
deployed on the LLM proxy, causing 502 errors. Keep only gpt-5 which
is the only available model.
- Fix token_secret for gpt41/gpt5-mini/gpt41-mini: use GPT_REVIEW_TOKEN
instead of SONNET_REVIEW_TOKEN (wrong reviewer identity)
- Move LLM base URL back to secrets.LLM_BASE_URL (prevents exfiltration
via PR-controlled matrix values)
- Remove hardcoded internal IP from workflow file; only provider path
suffix (/anthropic/v1, /openai/v1) remains in matrix
Addresses: security-review-bot REQUEST_CHANGES (major: exfiltration risk,
minor: HTTP/hardcoded IP) and sonnet-review-bot REQUEST_CHANGES (major:
wrong token_secret on gpt entries).
The matrix was wrong: "sonnet" was running GPT-5 and "gpt" was running
GPT-4.1. Now:
- sonnet → Claude Sonnet 4.6 via HAI Anthropic endpoint
- gpt → GPT-5 via HAI OpenAI endpoint
- security → GPT-5 via HAI OpenAI endpoint
Each matrix entry specifies its own provider and base_url.
Previously findOwnReview returned only the single most-recent matching
review, so on PRs with multiple force-pushes only the latest old review
got superseded. The rest accumulated as unsuperseded stale reviews.
Changes:
- Add findAllOwnReviews() to collect all non-superseded matching reviews
- Loop over all old reviews in the supersede phase
- Add GetTimelineReviewCommentIDForReview() to find comment IDs by
review ID (fetches review body, matches in timeline by prefix)
- Each old review gets independently superseded and its inline comments
resolved
The old findOwnReview is kept for backward compat (tested, may be
useful as a utility).
Closes#27
After superseding an old review, resolves all its inline comments via
POST /pulls/comments/{id}/resolve. This clears unresolved conversation
markers from the PR timeline and diff view.
New API methods:
- ListReviewComments: paginated GET /repos/.../pulls/{n}/reviews/{id}/comments
- ResolveComment: POST /repos/.../pulls/comments/{id}/resolve
Behavior:
- Only resolves after successful supersede (gated on supersedeOK)
- Aggregates failures and logs at warn level
- Truncates error bodies to 256 bytes (security)
- Non-fatal: review still posts even if resolution fails
- Accept 204 No Content as success (idempotent operations)
- Truncate error response body to 256 bytes (prevent log leakage)
- Add unit tests for GetAuthenticatedUser and RequestReviewer
Closes#35
Before posting a review, the bot:
1. Discovers its own Gitea login via GET /user
2. Calls POST /requested_reviewers to add itself
This ensures the bot appears in the required-reviewers list without
manual configuration on the repo. The call is idempotent (no-op if
already requested).
Both failures are non-fatal (warn + continue) — the review still posts
even if the self-request fails.
Changes the order of operations:
1. POST new review (gets non-stale badge immediately)
2. PATCH old review with superseded message linking to the new one
This gives the superseded comment a clickable link to the current
review, making navigation between review iterations easy.
buildSupersededBody now accepts a newReviewURL parameter.
The strict authorship check compared reviewer-name to User.Login which
could mismatch. The sentinel is already role-specific (e.g.
<!-- review-bot:sonnet -->) and Gitea's API blocks editing others'
comments (403). Defense-in-depth via login comparison is unnecessary
complexity that introduced a bug. Removed.
Closes#34
- Remove reviewUnchanged() skip logic — every push gets a fresh review
- Remove edit-in-place (PATCH same body) — always POST new
- Supersede old review: PATCH with struck-through banner + collapsed
original body in <details> for historical reference
- Add commit footer to every review: 'Evaluated against <sha>'
- Remove --update-existing flag (no longer needed)
- Add CommitID field to Review struct
- Add TestBuildSupersededBody tests
- Add --log-format flag (text/json) and --verbosity flag (debug/info/warn/error)
- Replace all log.Printf with slog.Info/Debug/Warn with structured key-value attrs
- Replace all log.Fatalf with slog.Error + os.Exit(1)
- Convert gitea/client.go warnings to slog.Warn
- Add comprehensive tests for logger initialization and level filtering
Closes#23
Partially addresses #32
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
- URL-encode filename in release upload query param (MINOR)
- Truncate APIError.Body to 200 chars in Error() to avoid leaking
verbose server responses into logs (NIT)
- Add APIError type with StatusCode field so callers can inspect HTTP
status codes from Gitea API responses
- Add IsNotFound helper for ergonomic 404 checks
- GetAllFilesInPath now only falls back to single-file fetch on 404;
all other errors (auth failures, server errors, rate limits) propagate
- Release workflow asset uploads are now idempotent: existing assets
with the same name are deleted before re-upload on workflow re-runs
Closes#8Closes#10
The security-review-bot Gitea user now has its own token. This
completes the token separation so each reviewer role posts under
its own identity, enabling native Gitea multi-reviewer blocking.
When hasSharedToken() detects two roles sharing the same Gitea user,
the bot now skips ALL update logic (PATCH, supersede) and always POSTs
a fresh review. This prevents clobbering a sibling's review body or
state when misconfigured.
Tests now assert return values (true/false) rather than just verifying
no panic. Added additional test case for three-roles-same-user scenario.
Addresses review feedback: update logic and review state must not
interact with sibling reviews under the same user.
When two review-bot roles share the same Gitea user token (misconfiguration),
log a WARNING identifying which sibling is sharing. The bot continues normally
with its own honest verdict — no escalation, no deadlock. Operators see the
warning in CI logs and can fix the token setup.
Addresses Aaron's review feedback on #28: graceful degradation when someone
doesn't follow the separate-token deployment instructions.
Apply url.PathEscape to owner, repo, and sha path segments in all
methods that were previously interpolating raw values. Methods already
using PathEscape (ListReviews, DeleteReview, GetTimelineReviewCommentID,
EditComment) are unchanged.
This eliminates an inconsistency flagged in PRs #17, #20, and #22 and
prevents potential path-injection bugs for names with special characters.
Closes#24
Explains the edit-in-place approach, state transition rules, worst-wins
escalation, and inline comment lifecycle. Includes a Mermaid state
diagram for visual reference.
1. First-run escalation regression (MAJOR): Add post-posting escalation
fallback. After posting APPROVED on first run, check if a sibling
from the same user has REQUEST_CHANGES — if so, mark ours as
superseded and re-post as REQUEST_CHANGES.
2. json.Marshal error handling (MINOR): Return error from EditComment
instead of ignoring it with blank identifier.
3. Redundant condition (NIT): Remove dead assignment in reviewUnchanged
where existingEvent was assigned from r.State then compared to itself.
Replace the delete-and-repost strategy with edit-in-place:
1. No existing review → POST new (first run)
2. Same state, same body → skip entirely (threads preserved)
3. Same state, body changed → PATCH body in place via timeline API
4. State change needed → PATCH old body to "Superseded", POST new
This preserves conversation threads on inline comments. Replies to
findings are never lost. The only time a new review is posted is on
first run or when the state transitions (APPROVED ↔ REQUEST_CHANGES).
New Gitea client methods:
- EditComment: PATCH /repos/{owner}/{repo}/issues/comments/{id}
- GetTimelineReviewCommentID: finds the comment ID for a review body
by scanning the issue timeline for the sentinel
Also simplifies shouldEscalate: removes the login parameter requirement
for pre-posting scenarios (uses findOwnReview to get login from existing
review instead).
Tests: findOwnReview (4 cases), EditComment (2 cases),
GetTimelineReviewCommentID (2 cases), shouldEscalate (8 cases updated).
Before posting, compare the new review body+event against the existing
review with the same sentinel. If identical, skip entirely — this
preserves conversation threads on inline comments and avoids
re-notifying reviewers for findings they already know about.
Only re-posts when findings actually change (fixed, new, or different).
Tests: 6 cases covering identical, different body, different state,
stale reviews, and different sentinels.
- Hunk headers without comma ("@@ -1 +1 @@") now parse correctly by
splitting on comma OR space instead of comma only
- Explicit skip for "\ No newline at end of file" lines (was already
safe but now documents intent)
- Tests added for both edge cases (TDD: tests written first, confirmed
failure, then fixed)
Addresses sonnet findings #1 and #2 from PR #26 review.
Findings that reference a file+line within the diff are now posted as
inline comments directly on that line, in addition to appearing in the
summary table. Findings outside the diff range stay in the body only.
Implementation:
- gitea/diff.go: ParseDiffNewLines extracts new-file line numbers from
each hunk in the unified diff
- gitea/client.go: PostReview accepts optional []ReviewComment with
path + new_position + body (omitempty when nil)
- cmd/review-bot/main.go: maps findings → inline comments when the line
exists in the diff, passes them to PostReview
Tests:
- diff parser: multi-hunk, new files, empty diff, boundary lines
- PostReview: with comments, nil comments (omitted from payload)
Moved validateReviewerName check to right after flag parsing. Previously
it ran after the LLM request completed — wasting an expensive API call
if the name was invalid.
Sonnet review finding #1.
When reviewer-name is set, prepend "# Security Review" / "# Sonnet Review"
etc. as a top-level header. Makes it immediately obvious which role each
review represents in the Gitea UI, especially when multiple reviews come
from the same bot account.
Security (MAJOR):
- Add filepath.EvalSymlinks after Clean for system-prompt-file
- Re-validate resolved path is still within workspace
- Prevents symlink → /etc/shadow exfiltration via malicious repo
Worst-wins:
- Check BEFORE posting (not after) — no delete+repost dance
- Identify sibling bots by <!-- review-bot: prefix in body
- Only escalates for bot reviews, not human REQUEST_CHANGES
- If sibling bot has REQUEST_CHANGES and we would APPROVE → post
REQUEST_CHANGES instead
Addresses security review finding #1 (MAJOR) and sonnet finding #1.
When multiple review types share a Gitea bot account, Gitea uses the
latest review to determine the user's approval state. This creates a
race: if security finds issues but code-quality finishes last with
APPROVE, the PR appears approved.
Now before posting, each job checks if any sibling review from the same
user has REQUEST_CHANGES. If so and we would post APPROVE, we downgrade
to COMMENT instead — the review is still visible but won't override
the blocking state.
Documented in README under "Shared Token: Worst-Wins."
- system-prompt-file: reject absolute paths and paths containing ".."
Prevents reading arbitrary files outside the workspace on shared runners.
- Cleanup: cross-check r.User.Login == posted.User.Login before deletion
Defense-in-depth: only attempt to delete reviews from same author.
Flagged by both sonnet and security reviewers.
- README: fix wording (cleanup happens after posting, not before)
Issues filed for deferred work:
- #24: Consistent url.PathEscape across all client endpoints
- #25: Binary signature verification for supply-chain hardening
- Quick start example with composite action + matrix strategy
- Full action inputs table with descriptions
- How sentinel-based cleanup works (explains the reviewer-name concept)
- Custom prompt file usage with security review example
- CLI usage with all flags
- Environment variables table
- Token scopes documentation
- Setup guide for new repos
Sentinel-based cleanup:
- Reviews embed <!-- review-bot:NAME --> in body (hidden HTML comment)
- Cleanup matches by sentinel, not token identity
- Each reviewer-name is a logical identity (sonnet, gpt, security)
- Same token can run multiple review types without conflict
- No extra API scopes needed
System prompt file (--system-prompt-file / SYSTEM_PROMPT_FILE):
- Loads a local file with additional review instructions
- Appended to system base as "Additional Review Instructions"
- Enables specialized reviews (security, performance, etc.)
- Partially addresses #5
Security review:
- SECURITY_REVIEW.md prompt focused on vulnerabilities
- 3rd CI matrix entry using same token, different prompt
- Focus: injection, auth, secrets, input validation, crypto, races
CI changes:
- REVIEWER_NAME passed from matrix.name
- SYSTEM_PROMPT_FILE passed from matrix (empty for standard reviews)
- 3 reviewers: sonnet (general), gpt (general), security (focused)
- PostReview now returns *Review (id + user login from response)
- Delete flow: post first, then delete stale reviews by same user
- No read:user scope needed (identity from POST response)
- Removed GetAuthenticatedUser (requires scope we lack)
- ListReviews: full pagination (loops until partial page)
- envOrDefaultBool: case-insensitive, whitespace-trimmed
- action.yml: document accepted boolean values
- Tests updated for new PostReview signature
Before posting a review, the bot now:
1. Calls GET /api/v1/user to identify its own login
2. Lists all reviews on the PR
3. Deletes any existing reviews from itself
4. Posts the fresh review
This keeps PR threads clean — one review per bot at any time.
New Gitea client methods:
- GetAuthenticatedUser() — token self-identification
- ListReviews() — fetch reviews on a PR
- DeleteReview() — delete a review by ID
Flag: --update-existing / UPDATE_EXISTING (default true)
Set to false to preserve old behavior (stack reviews).
All delete failures are non-fatal (logged as warnings).
Closes#6
- UserMeta comment: "never trimmed" → "truncated only if base exceeds budget"
- Skip diff truncation marker when diffBudget < markerBudget (prevents
marker itself from pushing EstTokens over the limit)
- Rename filepath → filePath to avoid shadowing stdlib package name
- Account for truncation marker tokens when computing diff budget
(prevents EstTokens exceeding model limit in edge cases)
- Rune-safe truncation for both UserMeta and Diff (no split multi-byte)
- Fix misleading comment (1000 chars → ~1000 tokens/4000 chars)
- Extract marker strings as constants
- Add unit tests for BuildSystemBase and BuildUserMeta
Addresses review findings:
- Replace map-based model limits with ordered slice (longest-prefix-first)
for deterministic matching
- Truncate UserMeta when base content alone exceeds budget (keeps first
4000 chars + truncation marker)
- Remove hard minimum of 1000 tokens for diff budget — use 0 as floor
to guarantee total never exceeds limit
- Handle zero-budget edge case (diff replaced with manual-review message)
- Add tests: huge UserMeta, all-sections-huge never exceeds limit
Adds a budget package that estimates token usage and progressively
trims context to fit within model-specific limits.
Trim order (least important first):
1. Language patterns
2. Repository conventions
3. Full file context
4. Diff (truncated as last resort)
When content is trimmed, a note is appended to the user prompt so
the LLM knows context was reduced.
- New budget package with Fit(), EstimateTokens(), LimitForModel()
- Model limit table (GPT-4.1: 128K, GPT-5: 200K, Claude: 200K)
- Refactored review/prompt.go: BuildSystemBase() and BuildUserMeta()
extract non-trimmable content; old functions delegate to new ones
- main.go uses budget.Fit() instead of direct prompt assembly
- 7 unit tests covering all trim paths
Closes#19
- Add --version flag and log version on startup (closes#9)
- URL-escape ref query parameter in GetFileContentRef (closes#7)
- Add go vet to release workflow (closes#13)
Renamed local url variable to reqURL to avoid shadowing net/url package.
- Overall context timeout now derived from LLM timeout + 1 minute
(no longer hardcoded 3min that could conflict with longer LLM timeouts)
- Clarify concurrency docs: With* methods are setup-only, not concurrent
- Add ctx.Err() checks in fetchFileContext and fetchPatterns loops
(break early on cancellation instead of making unnecessary requests)
- Fix doc comments: WithTimeout and WithTemperature each get their own
- Add TestWithTimeout (verifies short timeout causes request failure)
- Log warning on directory recursion failure in GetAllFilesInPath
- Note: unexported fields is a breaking change, will document in release notes
New flag: --llm-timeout / LLM_TIMEOUT (seconds, default 300)
New builder: llmClient.WithTimeout(duration)
Composite action: new timeout input
Keeps 5 minutes as the sensible default but allows tuning for
larger repos or slower models.
- GetAllFilesInPath: log.Printf when file fetch or dir recursion fails
- integration_test: use strings.SplitN for owner/repo parsing (idiomatic)
Addresses GPT review findings #1, #2.
REVIEW.md findings 1-4, 14:
- All Gitea client methods now accept context.Context as first param
- All LLM client methods now accept context.Context as first param
- Use http.NewRequestWithContext for cancellation/timeout support
- Main uses 3-minute timeout context for all operations
- Unexport Client struct fields (baseURL, token, apiKey, etc.)
- Use bytes.NewReader instead of strings.NewReader(string(...))
patterns-repo now accepts a comma-separated list of repos:
PATTERNS_REPO="rodin/elixir-patterns,rodin/phoenix-conventions"
patterns-files accepts files AND directories:
PATTERNS_FILES="README.md,docs/"
When a path is a directory, all files within it are fetched
recursively via the Gitea contents API. Only .md, .txt, .yml,
and .yaml files are included as pattern content.
New API methods:
- ListContents: list files/dirs at a path via contents API
- GetAllFilesInPath: recursively fetch all file contents
This allows a single review action to pull idioms from multiple
pattern repos (e.g. elixir-patterns + phoenix-conventions) and
include entire directories of documentation as review criteria.
Major improvements to review quality:
1. Full file context: fetch complete content of all modified files from
the PR branch and include as reference. This eliminates false-positive
"missing import" findings since the model sees the entire file.
2. Patterns repo: new --patterns-repo / PATTERNS_REPO flag fetches
language idiom files from a separate Gitea repo (e.g. rodin/elixir-patterns)
and includes them as review criteria.
3. Multi-file patterns: --patterns-files / PATTERNS_FILES accepts
comma-separated file paths to fetch from the patterns repo.
New API methods:
- GetPullRequestFiles: list changed files in a PR
- GetFileContentRef: fetch file content from a specific branch/ref
Prompt changes:
- BuildSystemPrompt now accepts (conventions, patterns)
- BuildUserPrompt now accepts fileContext parameter
- File context displayed before diff for model reference
- Patterns presented as "review criteria" in system prompt
Composite action updated with patterns-repo and patterns-files inputs.
The LLM was treating the diff as complete file context and flagging
"missing imports" for symbols that exist in unchanged portions of
the file. Added explicit instructions to the system prompt:
- Clarify that the diff is partial; unchanged code already exists
- Explicitly instruct: do not flag missing imports/types unless the
diff introduces a new usage without a corresponding addition
- Add rule: never flag undefined symbols that could exist in the
unchanged portions
- Add temperature range validation (must be 0-2, fatal on invalid)
- release.yml: use python3 for robust JSON parsing instead of sed
- Composite action: add header comment confirming Gitea Actions compat
- All findings from review #385 addressed
- Composite action: cache to runner.temp instead of /usr/local/bin
(avoids permission issues on runners)
- Document that temperature=0 means server default (omitted from request)
- Note: strconv import already exists (false positive from GPT-5)
Rodin
