diff --git a/internal/app/runtime_provider.go b/internal/app/runtime_provider.go index 0f822b0..1d61d5d 100644 --- a/internal/app/runtime_provider.go +++ b/internal/app/runtime_provider.go @@ -4,8 +4,11 @@ import ( "context" "github.com/open-cli-collective/codereview-cli/internal/gitprovider" + "github.com/open-cli-collective/codereview-cli/internal/outbox" ) +var _ outbox.LiveProvider = runtimeProvider{} + // runtimeProvider keeps repository reads on the profile git credential while // reviewer-authenticated writes and authority checks flow through the posting // credential. diff --git a/internal/gateio/gateio.go b/internal/gateio/gateio.go index 5efebfd..77d50f6 100644 --- a/internal/gateio/gateio.go +++ b/internal/gateio/gateio.go @@ -63,7 +63,7 @@ type AcquireFunc func(string) (Lock, error) // Options contains gate IO dependencies. type Options struct { Store Store - Provider gitprovider.GitProvider + Provider outbox.LiveProvider Limiter outbox.Limiter Layout statepaths.Layout Acquire AcquireFunc @@ -871,7 +871,7 @@ func summarizeStaleCandidate(opts Options, req Request, run ledger.Run, summary }, nil, nil } -func fetchReviewsUnlessApproved(ctx context.Context, provider gitprovider.GitProvider, ref gitprovider.PRRef, reviews []gitprovider.Review, loaded bool, posting gitprovider.Identity) ([]gitprovider.Review, *Result, error) { +func fetchReviewsUnlessApproved(ctx context.Context, provider outbox.LiveProvider, ref gitprovider.PRRef, reviews []gitprovider.Review, loaded bool, posting gitprovider.Identity) ([]gitprovider.Review, *Result, error) { if !loaded { var err error reviews, err = provider.ListReviews(ctx, ref) @@ -891,7 +891,7 @@ func fetchReviewsUnlessApproved(ctx context.Context, provider gitprovider.GitPro }, nil } -func readGateHostState(ctx context.Context, provider gitprovider.GitProvider, ref gitprovider.PRRef) (gateHostState, error) { +func readGateHostState(ctx context.Context, provider outbox.LiveProvider, ref gitprovider.PRRef) (gateHostState, error) { reviews, err := provider.ListReviews(ctx, ref) if err != nil { return gateHostState{}, err @@ -899,7 +899,7 @@ func readGateHostState(ctx context.Context, provider gitprovider.GitProvider, re return readGateHostStateWithReviews(ctx, provider, ref, reviews) } -func readGateHostStateWithReviews(ctx context.Context, provider gitprovider.GitProvider, ref gitprovider.PRRef, reviews []gitprovider.Review) (gateHostState, error) { +func readGateHostStateWithReviews(ctx context.Context, provider outbox.LiveProvider, ref gitprovider.PRRef, reviews []gitprovider.Review) (gateHostState, error) { comments, err := provider.ListIssueComments(ctx, ref) if err != nil { return gateHostState{}, err diff --git a/internal/gitprovider/github/client_test.go b/internal/gitprovider/github/client_test.go index e1650c4..30efc9e 100644 --- a/internal/gitprovider/github/client_test.go +++ b/internal/gitprovider/github/client_test.go @@ -13,12 +13,14 @@ import ( "github.com/open-cli-collective/codereview-cli/internal/config" "github.com/open-cli-collective/codereview-cli/internal/credentials" "github.com/open-cli-collective/codereview-cli/internal/gitprovider" + "github.com/open-cli-collective/codereview-cli/internal/outbox" ) var errTokenNotFound = errors.New("token not found") func TestClientImplementsGitProvider(_ *testing.T) { var _ gitprovider.GitProvider = (*Client)(nil) + var _ outbox.Provider = (*Client)(nil) } func TestCapabilities(t *testing.T) { diff --git a/internal/outbox/outbox.go b/internal/outbox/outbox.go index d0faa5d..732ba74 100644 --- a/internal/outbox/outbox.go +++ b/internal/outbox/outbox.go @@ -36,10 +36,29 @@ type Limiter interface { Wait(context.Context, string) error } +// Provider is the git-provider behavior required by the post phase. +type Provider interface { + ListInlineThreads(context.Context, gitprovider.PRRef) ([]gitprovider.InlineThread, error) + ListReviews(context.Context, gitprovider.PRRef) ([]gitprovider.Review, error) + ListIssueComments(context.Context, gitprovider.PRRef) ([]gitprovider.IssueComment, error) + PostInlineComment(context.Context, gitprovider.PRRef, gitprovider.InlineComment) (gitprovider.CommentID, error) + ReplyToThread(context.Context, gitprovider.PRRef, gitprovider.ThreadID, string) (gitprovider.CommentID, error) + ResolveThread(context.Context, gitprovider.PRRef, gitprovider.ThreadID) error + PostIssueComment(context.Context, gitprovider.PRRef, string) (gitprovider.CommentID, error) + SubmitReview(context.Context, gitprovider.PRRef, gitprovider.ReviewRequest) (gitprovider.ReviewID, error) + Capabilities() gitprovider.ProviderCaps +} + +// LiveProvider adds the pull-request read required by live orchestration. +type LiveProvider interface { + Provider + GetPR(context.Context, gitprovider.PRRef) (gitprovider.PR, error) +} + // Options contains post-phase dependencies. type Options struct { Store Store - Provider gitprovider.GitProvider + Provider Provider Limiter Limiter Now func() time.Time } @@ -324,7 +343,7 @@ func validDesiredOutcome(outcome ledger.Outcome) bool { outcome == ledger.OutcomeNothingToReview } -func readHostState(ctx context.Context, provider gitprovider.GitProvider, ref gitprovider.PRRef) (hostState, error) { +func readHostState(ctx context.Context, provider Provider, ref gitprovider.PRRef) (hostState, error) { threads, err := provider.ListInlineThreads(ctx, ref) if err != nil { return hostState{}, err @@ -487,7 +506,7 @@ func actionMarkerMatches(req Request, action ledger.PlannedAction, kind string, return false } -func buildActionPlan(provider gitprovider.GitProvider, req Request, actions []ledger.PlannedAction, action ledger.PlannedAction) (actionPlan, map[string]bool, error) { +func buildActionPlan(provider Provider, req Request, actions []ledger.PlannedAction, action ledger.PlannedAction) (actionPlan, map[string]bool, error) { bundledInlineIDs := map[string]bool{} switch action.Kind { case ledger.PlannedActionInlineComment: @@ -684,7 +703,7 @@ func bodyWithThreadSummaryMarker(req Request, action ledger.PlannedAction, body return rendered + "\n\n" + marker.SanitizeModelContent(body), nil } -func dispatch(ctx context.Context, provider gitprovider.GitProvider, ref gitprovider.PRRef, plan actionPlan) (string, error) { +func dispatch(ctx context.Context, provider Provider, ref gitprovider.PRRef, plan actionPlan) (string, error) { switch plan.action.Kind { case ledger.PlannedActionInlineComment: id, err := provider.PostInlineComment(ctx, ref, *plan.inlineComment) diff --git a/internal/outbox/outbox_test.go b/internal/outbox/outbox_test.go index c5688b0..9fa430b 100644 --- a/internal/outbox/outbox_test.go +++ b/internal/outbox/outbox_test.go @@ -16,6 +16,64 @@ import ( "github.com/open-cli-collective/codereview-cli/internal/review" ) +var ( + _ Provider = (*gitprovider.Fake)(nil) + _ Provider = (*minimalProvider)(nil) +) + +type minimalProvider struct { + posted string +} + +func (*minimalProvider) ListInlineThreads(context.Context, gitprovider.PRRef) ([]gitprovider.InlineThread, error) { + return nil, nil +} + +func (*minimalProvider) ListReviews(context.Context, gitprovider.PRRef) ([]gitprovider.Review, error) { + return nil, nil +} + +func (*minimalProvider) ListIssueComments(context.Context, gitprovider.PRRef) ([]gitprovider.IssueComment, error) { + return nil, nil +} + +func (*minimalProvider) PostInlineComment(context.Context, gitprovider.PRRef, gitprovider.InlineComment) (gitprovider.CommentID, error) { + return "inline", nil +} + +func (*minimalProvider) ReplyToThread(context.Context, gitprovider.PRRef, gitprovider.ThreadID, string) (gitprovider.CommentID, error) { + return "reply", nil +} + +func (*minimalProvider) ResolveThread(context.Context, gitprovider.PRRef, gitprovider.ThreadID) error { + return nil +} + +func (p *minimalProvider) PostIssueComment(_ context.Context, _ gitprovider.PRRef, body string) (gitprovider.CommentID, error) { + p.posted = body + return "issue", nil +} + +func (*minimalProvider) SubmitReview(context.Context, gitprovider.PRRef, gitprovider.ReviewRequest) (gitprovider.ReviewID, error) { + return "review", nil +} + +func (*minimalProvider) Capabilities() gitprovider.ProviderCaps { return gitprovider.ProviderCaps{} } + +func TestPostAcceptsProviderPortWithoutUnrelatedMethods(t *testing.T) { + store := openStore(t) + run := allocateRun(t, store, ledger.PostModeLive) + provider := &minimalProvider{} + insertAction(t, store, plannedAction(run.RunID, "rollup-1", ledger.PlannedActionRollupComment, true, "", RollupCommentPayload{Body: "rollup"})) + + if _, err := Post(context.Background(), Options{Store: store, Provider: provider, Limiter: noopLimiter{}, Now: fixedClock()}, testRequest(run)); err != nil { + t.Fatalf("Post: %v", err) + } + if !strings.Contains(provider.posted, "rollup") { + t.Fatalf("posted body = %q, want rollup", provider.posted) + } +} + func TestPostEmbedsMarkersAndPostsInCanonicalOrder(t *testing.T) { store := openStore(t) run := allocateRun(t, store, ledger.PostModeLive) diff --git a/internal/reviewrun/reviewrun.go b/internal/reviewrun/reviewrun.go index 456b54c..57bd8ea 100644 --- a/internal/reviewrun/reviewrun.go +++ b/internal/reviewrun/reviewrun.go @@ -54,7 +54,7 @@ type Planner interface { // Options contains live review dependencies. type Options struct { Store Store - Provider gitprovider.GitProvider + Provider outbox.LiveProvider Planner Planner Limiter outbox.Limiter Layout statepaths.Layout diff --git a/internal/threadrespond/threadrespond.go b/internal/threadrespond/threadrespond.go index c8a8004..a215012 100644 --- a/internal/threadrespond/threadrespond.go +++ b/internal/threadrespond/threadrespond.go @@ -64,7 +64,7 @@ type AcquireFunc func(string) (Lock, error) // Options contains response-run dependencies. type Options struct { Store Store - Provider gitprovider.GitProvider + Provider outbox.LiveProvider Adapter llm.Adapter Limiter outbox.Limiter Layout statepaths.Layout