feat(cmd): wire --provider and --base-url flags into CLI (Phase 5) #82
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Architecture Reference
Base branch:
feature/github-support— open all PRs against this branch.Goal
Wire the
vcs.Clientabstraction intocmd/review-bot/main.gowith--providerand--base-urlflags. This is the phase that makesgithub.concur.comwork end-to-end.Background
By this point:
vcs/interfaces.go,vcs/types.go,vcs/util.goexist (issue #78)gitea.Adaptersatisfiesvcs.Client(issue #79)github.Clientsatisfiesvcs.Client(issues #80, #81)The current
main.gouses*gitea.Clientdirectly throughout. This issue updates it to usevcs.Clientand selects the right adapter at startup.Auth token
The existing
--reviewer-tokenflag (env:REVIEWER_TOKEN) is reused for both providers. No new token flag needed. The adapters handle the header format difference internally:Authorization: token <token>Authorization: Bearer <token>New flags
Rename
--gitea-url→--vcs-url(keep--gitea-urland env varGITEA_URLas deprecated aliases). Add env varVCS_URL. SimilarlyGITEA_REPO→VCS_REPOwithGITEA_REPOas alias.Client selection
Complete caller audit
Every
*gitea.Clientcall inmain.go— what to do with each:giteaClient.GetPullRequest(step 1, fetch metadata)client.GetPullRequest— returns*vcs.PullRequest; all downstream field accesses (pr.Title,pr.Body,pr.Head.Sha,pr.Head.Ref) have identical names, no renames neededgiteaClient.GetPullRequest(step 7, stale check)client.GetPullRequest— same call, different variable; update this separatelygiteaClient.GetPullRequestDiffclient.GetPullRequestDiffgiteaClient.GetPullRequestFilesclient.GetPullRequestFiles; returns[]vcs.ChangedFilegiteaClient.GetFileContentRefclient.GetFileContentAtRefgiteaClient.GetFileContentclient.GetFileContentgiteaClient.GetCommitStatusesclient.GetCommitStatusesevaluateCIStatus(statuses []gitea.CommitStatus, ...)[]vcs.CommitStatusgiteaClient.ListReviewsclient.ListReviews; returns[]vcs.Review; updatefindOwnReview,findAllOwnReviews,hasSharedTokensignatures to take[]vcs.ReviewgiteaClient.PostReviewclient.PostReview(ctx, owner, repo, prNumber, vcs.ReviewRequest{...})— see inline comment construction sectiongiteaClient.GetAuthenticatedUserclient.GetAuthenticatedUsergiteaClient.RequestReviewergiteaClient.GetTimelineReviewCommentIDForReviewgiteaClient.EditCommentgiteaClient.ResolveCommentgiteaClient.ListReviewCommentsgitea.ParseDiffNewLines(diff)giteaClient.GetAllFilesInPathvcs.GetAllFilesInPath(ctx, client, owner, repo, path)from issue #78; updatefetchPatternsto takevcs.FileReadergiteaClient.ListContentsclient.ListContentsreview.GiteaEvent(result.Verdict)result.Verdictdirectly (LLM produces canonical values"APPROVE","REQUEST_CHANGES","COMMENT"). Deletereview.GiteaEventfunction fromreview/package.Inline comment construction (critical)
Currently:
gitea.ReviewComment{Path: f.File, NewPosition: int64(f.Line), Body: ...}— the LLM producesf.Lineas a new-file line number.After migration:
vcs.ReviewComment{Path: f.File, Position: <diff-position>, CommitID: pr.Head.Sha, Body: ...}.The LLM produces line numbers, not diff-positions.
Positioninvcs.ReviewCommentis a diff-position (GitHub convention). These are different values. The conversion is required:Add
vcs.BuildLineToPositionMap(diff string) map[string]map[int]inttovcs/util.goin issue #78. This replacesgitea.ParseDiffNewLinesentirely. The map isfile → (new-file line → diff-position).The existing
diffRanges.Containsfilter is replaced: skip anyf.Linenot present in the map.This applies to both providers — the Gitea adapter's PostReview also receives diff-positions (that's the whole point of issue #79), so the line→position conversion must happen in main.go before calling PostReview on either adapter.
fetchFileContextsignaturefetchFileContextcallsGetFileContentAtRef, which is onvcs.PRReader, notvcs.FileReader. Usevcs.PRReader:fetchPatternsonly needsGetFileContent+ListContents:giteaClientAdapter/review.GiteaClientremovalreview.GiteaClientis defined inreview/repo_persona.go.mockGiteaClientinreview/repo_persona_test.goimplements it.Action:
review.LoadRepoPersonasto acceptvcs.FileReaderreview.GiteaClientinterface fromreview/repo_persona.goreview.ContentEntrytype fromreview/repo_persona.go(replaced byvcs.ContentEntry)review/repo_persona_test.go: changemockGiteaClientto implementvcs.FileReader— updateListContentsreturn type from[]review.ContentEntryto[]vcs.ContentEntry, update test data accordinglygiteaClientAdapterstruct frommain.goclientdirectly:review.LoadRepoPersonas(ctx, client, owner, repoName)cmd/review-bot/main_test.go: changemakeReviewreturn type fromgitea.Reviewtovcs.Review; update all[]gitea.Reviewtest fixtures infindOwnReview/hasSharedTokentests to[]vcs.ReviewRequestReviewer(safe type assertion)Never use an unguarded type assertion (
client.(*gitea.Adapter)) — it will panic when--provider github.Supersede flow (provider-branched, safe assertions)
All type assertions to
*gitea.Adaptermust use the two-value form. Never assert withoutok.vcs.BuildLineToPositionMaputility (add to issue #78)Note: the inline comment construction section above depends on a new utility
vcs.BuildLineToPositionMap(diff string) map[string]map[int]int. This must be added tovcs/util.goin issue #78 (or as part of this issue if preferred). It inverts the position→line map from the Gitea adapter's diff parser: given a unified diff, returnsfile → (new-line → diff-position).Exit criteria
./review-bot --provider gitea ...works identically to before (zero regression)./review-bot --provider github --base-url https://github.concur.com/api/v3 ...posts a review on a real GHE PR./review-bot --provider github ...works againstgithub.com--gitea-urlandGITEA_URLstill accepted as deprecated aliases*vcsURLis only required when*provider == "gitea"; forprovider == "github"it is optional (base URL defaults tohttps://api.github.comor*baseURL)giteaClientAdapter,review.GiteaClient,review.ContentEntry,review.GiteaEventall deletedreview/repo_persona_test.goupdated:mockGiteaClientimplementsvcs.FileReaderclient.(*gitea.Adapter)assertions anywhereOut of scope
Update: When auditing callers that use
*gitea.Clientdirectly, pay attention to any call toDeleteReviewthat is used to replace a submitted review before posting a new one. Those callers must be updated to useDismissReviewinstead —DeleteReviewonly works on PENDING reviews on GitHub, and calling it on a submitted review returns 422.Update:
fetchPatternsandfetchFileContextsignatures — Both helpers currently take*gitea.Clientdirectly.fetchPatternsto takevcs.FileReaderand usevcs.GetAllFilesInPath(utility added in #78) for recursive traversalfetchFileContextto takevcs.FileReader(usesGetFileContentAtRef)Neither function needs the full
vcs.Client—vcs.FileReaderis sufficient and keeps the signatures narrow.