Issue #107 explicitly requested adding CommitID string to vcs.ReviewRequest so both platform adapters can thread the commit anchor through the abstraction layer. PR #112 only added commitID as a direct parameter to gitea.Client.PostReview (the low-level Gitea client), not to the shared vcs.ReviewRequest type.
github.Client.PostReview still sources CommitID only from req.Comments[i].CommitID (comment-by-comment), not from a top-level field. If a review has no inline comments, commit_id is empty on the GitHub side too.
gitea.Adapter.PostReview has no way to pass the commit anchor without this field.
This was the core request in issue #107: "Add a CommitID string field to vcs.ReviewRequest and pass it explicitly from the caller."
Source
PR: #112 — feat(gitea): pass commit_id explicitly in PostReview (#107)
Linked issue: #107 — PostReview: pass CommitID explicitly via ReviewRequest
Acceptance criterion: CommitID string added to vcs.ReviewRequest
Update gitea.Adapter.PostReview to pass req.CommitID to the underlying client
Update github.Client.PostReview to use req.CommitID as the primary commit anchor, falling back to comment-derived CommitID if empty and non-conflicting
Update callers of vcs.Client.PostReview (specifically cmd/review-bot/main.go) to populate ReviewRequest.CommitID with evaluatedSHA
## What was missed
Issue #107 explicitly requested adding `CommitID string` to `vcs.ReviewRequest` so both platform adapters can thread the commit anchor through the abstraction layer. PR #112 only added `commitID` as a direct parameter to `gitea.Client.PostReview` (the low-level Gitea client), not to the shared `vcs.ReviewRequest` type.
The current `vcs.ReviewRequest` in `vcs/types.go`:
```go
type ReviewRequest struct {
Body string `json:"body"`
Event ReviewEvent `json:"event"`
Comments []ReviewComment `json:"comments,omitempty"`
// Missing: CommitID string
}
```
As a result:
- `github.Client.PostReview` still sources `CommitID` only from `req.Comments[i].CommitID` (comment-by-comment), not from a top-level field. If a review has no inline comments, `commit_id` is empty on the GitHub side too.
- `gitea.Adapter.PostReview` has no way to pass the commit anchor without this field.
This was the core request in issue #107: "Add a `CommitID string` field to `vcs.ReviewRequest` and pass it explicitly from the caller."
## Source
- PR: #112 — feat(gitea): pass commit_id explicitly in PostReview (#107)
- Linked issue: #107 — PostReview: pass CommitID explicitly via ReviewRequest
- Acceptance criterion: `CommitID string` added to `vcs.ReviewRequest`
- File: `vcs/types.go` (ReviewRequest struct)
## What needs to happen
- Add `CommitID string \`json:"commit_id,omitempty"\`` to `vcs.ReviewRequest`
- Update `gitea.Adapter.PostReview` to pass `req.CommitID` to the underlying client
- Update `github.Client.PostReview` to use `req.CommitID` as the primary commit anchor, falling back to comment-derived CommitID if empty and non-conflicting
- Update callers of `vcs.Client.PostReview` (specifically `cmd/review-bot/main.go`) to populate `ReviewRequest.CommitID` with `evaluatedSHA`
## References
- [PR #112](https://gitea.weiker.me/rodin/review-bot/pulls/112)
- [Issue #107](https://gitea.weiker.me/rodin/review-bot/issues/107)
PR #112 refactored the review path to use the vcs.ReviewRequest abstraction, but removed the commitID parameter from gitea.Client.PostReview without adding a corresponding CommitID field to vcs.ReviewRequest. This means:
Gitea adapter has no way to pass the commit anchor — the commitID was previously a direct parameter to gitea.Client.PostReview (on origin/main) but got dropped during the refactoring.
GitHub adapter only sources CommitID from req.Comments[i].CommitID — if a review has no inline comments, no commit_id is sent to GitHub either.
Caller (cmd/review-bot/main.go) sets CommitID on each inline comment but doesn't set it at the review level.
Constraints
Must not break existing behavior for reviews with inline comments (GitHub still derives commit_id from comments)
Must work for both Gitea and GitHub adapters
Must be backward compatible (empty CommitID = existing behavior)
Branch from origin/feature/github-support (that's where vcs/ package exists)
Proposed Approach
4 changes, all minimal:
vcs/types.go — Add CommitID string field to ReviewRequest:
gitea/adapter.go — Pass req.CommitID to the underlying gitea.Client.PostReview. This requires adding the commitID parameter back to gitea.Client.PostReview.
gitea/client.go — Re-add commitID string parameter to PostReview and include it in the JSON payload (matching origin/main's original signature).
github/review.go — Use req.CommitID as primary commit anchor; fall back to comment-derived CommitID if req.CommitID is empty. If both exist and conflict, return error.
cmd/review-bot/main.go — Set ReviewRequest.CommitID = evaluatedSHA (currently pr.Head.SHA) when constructing the review request.
State/Data Model
No new states. CommitID is a pass-through value from caller → adapter → API.
Error Cases
req.CommitID is empty: adapters behave as today (Gitea defaults to PR head, GitHub derives from comments or omits)
req.CommitID conflicts with a comment's CommitID (GitHub): return ErrConflictingCommitIDs
req.CommitID set but no comments: both APIs accept review-level commit_id without comments
Edge Cases
Review with no inline comments: req.CommitID is the only source → sent to both APIs
Review with comments that each have CommitID matching req.CommitID: no conflict
Review with comments that have different CommitID from req.CommitID (GitHub): error
The Gitea API's CreatePullReview endpoint has a commit_id field at the review level (per Gitea docs). PR #112 removed it from the client; this fix adds it back. Straightforward.
Should req.CommitID override comment-level CommitIDs in GitHub, or should they be required to match? I'm going with "required to match (error on conflict)" since that's safest and matches existing behavior for comment-to-comment conflicts.
## Plan: Add CommitID to vcs.ReviewRequest (#115)
### Problem
PR #112 refactored the review path to use the `vcs.ReviewRequest` abstraction, but removed the `commitID` parameter from `gitea.Client.PostReview` without adding a corresponding `CommitID` field to `vcs.ReviewRequest`. This means:
1. **Gitea adapter** has no way to pass the commit anchor — the `commitID` was previously a direct parameter to `gitea.Client.PostReview` (on `origin/main`) but got dropped during the refactoring.
2. **GitHub adapter** only sources `CommitID` from `req.Comments[i].CommitID` — if a review has no inline comments, no `commit_id` is sent to GitHub either.
3. **Caller** (`cmd/review-bot/main.go`) sets `CommitID` on each inline comment but doesn't set it at the review level.
### Constraints
- Must not break existing behavior for reviews with inline comments (GitHub still derives commit_id from comments)
- Must work for both Gitea and GitHub adapters
- Must be backward compatible (empty CommitID = existing behavior)
- Branch from `origin/feature/github-support` (that's where `vcs/` package exists)
### Proposed Approach
**4 changes, all minimal:**
1. **`vcs/types.go`** — Add `CommitID string` field to `ReviewRequest`:
```go
type ReviewRequest struct {
Body string `json:"body"`
Event ReviewEvent `json:"event"`
CommitID string `json:"commit_id,omitempty"`
Comments []ReviewComment `json:"comments,omitempty"`
}
```
2. **`gitea/adapter.go`** — Pass `req.CommitID` to the underlying `gitea.Client.PostReview`. This requires adding the `commitID` parameter back to `gitea.Client.PostReview`.
3. **`gitea/client.go`** — Re-add `commitID string` parameter to `PostReview` and include it in the JSON payload (matching origin/main's original signature).
4. **`github/review.go`** — Use `req.CommitID` as primary commit anchor; fall back to comment-derived CommitID if `req.CommitID` is empty. If both exist and conflict, return error.
5. **`cmd/review-bot/main.go`** — Set `ReviewRequest.CommitID = evaluatedSHA` (currently `pr.Head.SHA`) when constructing the review request.
### State/Data Model
No new states. `CommitID` is a pass-through value from caller → adapter → API.
### Error Cases
- `req.CommitID` is empty: adapters behave as today (Gitea defaults to PR head, GitHub derives from comments or omits)
- `req.CommitID` conflicts with a comment's CommitID (GitHub): return `ErrConflictingCommitIDs`
- `req.CommitID` set but no comments: both APIs accept review-level commit_id without comments
### Edge Cases
- Review with no inline comments: `req.CommitID` is the only source → sent to both APIs
- Review with comments that each have `CommitID` matching `req.CommitID`: no conflict
- Review with comments that have different `CommitID` from `req.CommitID` (GitHub): error
- Gitea adapter ignores per-comment CommitID (already documented) but passes review-level CommitID
### Testing Strategy
1. **Unit test `github/review_test.go`**: Verify `req.CommitID` is used as primary anchor
2. **Unit test `github/review_test.go`**: Verify fallback to comment CommitID when `req.CommitID` empty
3. **Unit test `github/review_test.go`**: Verify conflict detection between `req.CommitID` and comment CommitID
4. **Unit test `gitea/adapter_test.go`**: Verify `req.CommitID` is forwarded to underlying client
5. **Unit test `gitea/client_test.go`**: Verify commitID appears in JSON payload
### Completion Checklist
1. `CommitID string` added to `vcs.ReviewRequest` with json tag `commit_id,omitempty`?
2. `gitea.Client.PostReview` accepts and forwards `commitID` parameter?
3. `gitea.Adapter.PostReview` passes `req.CommitID` to underlying client?
4. `github.Client.PostReview` uses `req.CommitID` as primary, falls back to comment-derived?
5. Conflict detection works when `req.CommitID` != comment CommitID in GitHub adapter?
6. `cmd/review-bot/main.go` sets `ReviewRequest.CommitID = evaluatedSHA`?
7. All existing tests pass?
8. New tests cover the added behavior?
### Open Questions
- The Gitea API's CreatePullReview endpoint has a `commit_id` field at the review level (per Gitea docs). PR #112 removed it from the client; this fix adds it back. Straightforward.
- Should `req.CommitID` override comment-level CommitIDs in GitHub, or should they be required to match? I'm going with "required to match (error on conflict)" since that's safest and matches existing behavior for comment-to-comment conflicts.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
What was missed
Issue #107 explicitly requested adding
CommitID stringtovcs.ReviewRequestso both platform adapters can thread the commit anchor through the abstraction layer. PR #112 only addedcommitIDas a direct parameter togitea.Client.PostReview(the low-level Gitea client), not to the sharedvcs.ReviewRequesttype.The current
vcs.ReviewRequestinvcs/types.go:As a result:
github.Client.PostReviewstill sourcesCommitIDonly fromreq.Comments[i].CommitID(comment-by-comment), not from a top-level field. If a review has no inline comments,commit_idis empty on the GitHub side too.gitea.Adapter.PostReviewhas no way to pass the commit anchor without this field.This was the core request in issue #107: "Add a
CommitID stringfield tovcs.ReviewRequestand pass it explicitly from the caller."Source
CommitID stringadded tovcs.ReviewRequestvcs/types.go(ReviewRequest struct)What needs to happen
CommitID string \json:"commit_id,omitempty"`tovcs.ReviewRequest`gitea.Adapter.PostReviewto passreq.CommitIDto the underlying clientgithub.Client.PostReviewto usereq.CommitIDas the primary commit anchor, falling back to comment-derived CommitID if empty and non-conflictingvcs.Client.PostReview(specificallycmd/review-bot/main.go) to populateReviewRequest.CommitIDwithevaluatedSHAReferences
Plan: Add CommitID to vcs.ReviewRequest (#115)
Problem
PR #112 refactored the review path to use the
vcs.ReviewRequestabstraction, but removed thecommitIDparameter fromgitea.Client.PostReviewwithout adding a correspondingCommitIDfield tovcs.ReviewRequest. This means:commitIDwas previously a direct parameter togitea.Client.PostReview(onorigin/main) but got dropped during the refactoring.CommitIDfromreq.Comments[i].CommitID— if a review has no inline comments, nocommit_idis sent to GitHub either.cmd/review-bot/main.go) setsCommitIDon each inline comment but doesn't set it at the review level.Constraints
origin/feature/github-support(that's wherevcs/package exists)Proposed Approach
4 changes, all minimal:
vcs/types.go— AddCommitID stringfield toReviewRequest:gitea/adapter.go— Passreq.CommitIDto the underlyinggitea.Client.PostReview. This requires adding thecommitIDparameter back togitea.Client.PostReview.gitea/client.go— Re-addcommitID stringparameter toPostReviewand include it in the JSON payload (matching origin/main's original signature).github/review.go— Usereq.CommitIDas primary commit anchor; fall back to comment-derived CommitID ifreq.CommitIDis empty. If both exist and conflict, return error.cmd/review-bot/main.go— SetReviewRequest.CommitID = evaluatedSHA(currentlypr.Head.SHA) when constructing the review request.State/Data Model
No new states.
CommitIDis a pass-through value from caller → adapter → API.Error Cases
req.CommitIDis empty: adapters behave as today (Gitea defaults to PR head, GitHub derives from comments or omits)req.CommitIDconflicts with a comment's CommitID (GitHub): returnErrConflictingCommitIDsreq.CommitIDset but no comments: both APIs accept review-level commit_id without commentsEdge Cases
req.CommitIDis the only source → sent to both APIsCommitIDmatchingreq.CommitID: no conflictCommitIDfromreq.CommitID(GitHub): errorTesting Strategy
github/review_test.go: Verifyreq.CommitIDis used as primary anchorgithub/review_test.go: Verify fallback to comment CommitID whenreq.CommitIDemptygithub/review_test.go: Verify conflict detection betweenreq.CommitIDand comment CommitIDgitea/adapter_test.go: Verifyreq.CommitIDis forwarded to underlying clientgitea/client_test.go: Verify commitID appears in JSON payloadCompletion Checklist
CommitID stringadded tovcs.ReviewRequestwith json tagcommit_id,omitempty?gitea.Client.PostReviewaccepts and forwardscommitIDparameter?gitea.Adapter.PostReviewpassesreq.CommitIDto underlying client?github.Client.PostReviewusesreq.CommitIDas primary, falls back to comment-derived?req.CommitID!= comment CommitID in GitHub adapter?cmd/review-bot/main.gosetsReviewRequest.CommitID = evaluatedSHA?Open Questions
commit_idfield at the review level (per Gitea docs). PR #112 removed it from the client; this fix adds it back. Straightforward.req.CommitIDoverride comment-level CommitIDs in GitHub, or should they be required to match? I'm going with "required to match (error on conflict)" since that's safest and matches existing behavior for comment-to-comment conflicts.