[NIT] The defaultCheckRedirect function uses fmt.Errorf for all error paths, but none of these errors wrap an underlying cause (no %w). Per the error-handling patterns, fmt.Errorf without %w is fine here since these are newly constructed errors with no upstream cause to preserve. This is correct — just noting that errors.New could be used instead for the static messages (e.g. stopped after 10 redirects) since there's no format interpolation needed there, though fmt.Errorf is not wrong.
[NIT] The duplication comment says Changes here must be mirrored there but there is no enforcement mechanism (no shared test, no lint rule, no CI check). This is a maintenance risk. A follow-up to extract this into a shared internal/ package would eliminate the drift risk entirely. The PR description acknowledges this is intentional due to package separation, but the internal/ pattern exists precisely for this use case.
[NIT] SupersedeReviews silently ignores resolution errors for individual inline comments via _ = underlying.ResolveComment(...). The old code in main.go tracked resolved and failed counts and logged them. The new adapter drops that observability entirely. Consider at minimum logging at debug level on resolution failure, matching the pattern used for EditComment failures a few lines above.
[NIT] ProviderGithub VCSProvider = "github" — the constant name uses Github (lowercase 'h') while the package import path and the flag help text use github. Per the Go acronym convention documented in patterns/style.md (Acronyms Are All-Caps), this should be ProviderGitHub. This is a new exported symbol so fixing it now is low-cost; changing it later would break callers.
[NIT] There is a dangling comment fragment: // with collapsed original content and the commit it was evaluated against. — this is the second half of the buildSupersededBody doc comment that was removed when the function moved to gitea/adapter.go. The first line of the doc comment (// buildSupersededBody creates the body for a superseded review: struck-through banner) was deleted but this trailing line was left orphaned. It should be removed.
[MINOR] The --gitea-url backward-compatible alias is registered with flag.StringVar(vcsURL, "gitea-url", *vcsURL, ...) where *vcsURL is the default value captured at the moment of the call — before flag.Parse(). This is correct as written because *vcsURL is still the default string at that point, but it creates a subtle ordering dependency: if vcsURL ever acquired a non-default value between its declaration and this flag.StringVar call (e.g., if any code ran between them), the alias would get a stale default. The comment acknowledges this (Must stay after vcsURL declaration and before flag.Parse()), but the fragility is worth noting. A cleaner pattern would be flag.StringVar(vcsURL, "gitea-url", "", "...") or extracting the default value to a named constant.
[MINOR] When client.(vcs.ReviewSuperseder) returns false and len(oldReviews) > 0, the code logs a warning and silently skips superseding. For the GitHub provider, github.Client implements ReviewSuperseder, so this path is unreachable. For Gitea, gitea.Adapter also implements it. The warning will therefore never fire for any configured provider, making it dead code in practice. It's harmless, but the slog.Warn may mislead operators if a future provider is wired without implementing the interface — they'd see a warning but no error. Consider whether this should be an error for unknown cases.
[NIT] The comment 'Guard: net/http guarantees len(via) >= 1 but defend against zero-length to avoid panic on index out of range' is accurate but slightly misleading: net/http does guarantee len(via) >= 1 when CheckRedirect is called during an actual redirect, but the test TestDefaultCheckRedirect_EmptyViaAllowed calls the function directly with nil. The guard is appropriate for testability and defensive coding; the comment could be tightened to say 'Guard for direct invocation in tests and any future callers'.
[MINOR] The defaultCheckRedirect function is duplicated verbatim between gitea/client.go and github/client.go. Since these are separate packages this is unavoidable without introducing a shared internal package, but it means any future change must be made in both places. A comment noting the duplication and pointing to the sibling package would help future maintainers. Alternatively, an internal/httppolicy package could house the shared function — though that adds structural complexity for a single function.
[MINOR] supersedeOldReviews accepts provider as a plain string rather than a typed constant/enum. The valid provider values are scattered: validated at entry (switch *provider) and re-evaluated here. A typed VCSProvider string type would make the contract explicit and allow the compiler to help catch new provider additions that aren't handled in both switches.
[NIT] doJSONRequest is added but not yet called by any public methods in the diff (the github package has no PostReview/DismissReview/etc. visible in the diff). This is fine for an incremental PR, but it means the new helper is untested at the integration level. The unit tests for doJSONRequest itself are present and cover the retry path.
[MINOR] supersedeOldReviews performs a type assertion (client.(*gitea.Adapter)) that creates a layering violation: the cmd package now depends on the concrete gitea.Adapter type for provider-specific logic. The comment acknowledges the brittleness ('guards against future refactors') but a cleaner approach would be to add a Gitea-specific optional interface (e.g. vcs.GiteaSuperseder) similar to how ReviewerSelfRequester is handled, avoiding the concrete type assertion entirely.
[NIT] The --gitea-url alias comment block (lines 97-106) is thorough documentation of a non-obvious flag.StringVar trick. Consider extracting the alias registration into a one-liner with a brief inline comment; the current 10-line comment block dominates the flag registration section visually.
[NIT] The switch for VCS client initialization has no default branch (the provider validation switch above guarantees valid values, so this is technically safe), but adding a default: panic(...) or default: slog.Error(...); os.Exit(1) would make the invariant explicit and guard against future providers being added to the validation switch without a corresponding factory branch.
[NIT] The seen[personaName] = true simplification (replacing the if !seen[personaName] guard) is correct because assigning true to an already-true map entry is idempotent. This is a valid cleanup, but the original pattern was arguably more explicit about intent. No functional issue.
[MINOR] The vcsURL parameter is accepted by supersedeOldReviews even for the GitHub path (where it is unused) with the stated goal of keeping the signature uniform. However this means the GitHub path silently ignores a potentially user-visible parameter with no validation. If vcsURL is an empty string and the provider is github, the Gitea URL-scheme validation block is never reached, which is fine — but a caller could accidentally pass a bad vcsURL for GitHub and receive no diagnostic. A doc comment clarifying this intentional skip would help future readers.
[MINOR] The client variable is declared as var client vcs.Client then assigned inside a switch with no default case. Since the provider switch was already validated a few lines above (exiting on invalid values), the compiler cannot see this — it will not warn if the two switches ever diverge. A default: panic(...) or at minimum a comment noting why the default is unreachable would make the invariant explicit and guard against future edits that add a provider to the validation switch but forget to add it to the initialization switch.