Skip to content

Commit 105c7f7

Browse files
committed
fix(mcp): preserve bounded portfolio handoffs
1 parent 092cc99 commit 105c7f7

10 files changed

Lines changed: 174 additions & 31 deletions

‎internal/app/mcp_authored_sync.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ func (s *Service) syncAuthoredPullRequests(ctx context.Context, in authoredPullR
6060
discovered := 0
6161
incomplete := false
6262
requestCapped := false
63+
boundedByLimit := false
6364
for discovered < in.Limit {
6465
if requests+1 > in.MaxRequests {
6566
requestCapped, incomplete = true, true
@@ -90,7 +91,11 @@ func (s *Service) syncAuthoredPullRequests(ctx context.Context, in authoredPullR
9091
break
9192
}
9293
}
93-
if !result.Page.HasNext || discovered >= in.Limit {
94+
if discovered >= in.Limit {
95+
boundedByLimit = result.Page.HasNext || result.Total > discovered
96+
break
97+
}
98+
if !result.Page.HasNext {
9499
break
95100
}
96101
page = result.Page.NextPage
@@ -141,6 +146,9 @@ func (s *Service) syncAuthoredPullRequests(ctx context.Context, in authoredPullR
141146
close(jobs)
142147
wg.Wait()
143148
status := "complete"
149+
if boundedByLimit {
150+
status = "partial"
151+
}
144152
completed := 0
145153
for _, result := range results {
146154
requests += result.Requests

‎internal/app/mcp_job_artifacts.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ func portfolioResultRefs(values []string) []mcpcontract.ThreadRef {
270270
if err != nil || number <= 0 {
271271
continue
272272
}
273-
fullName := value[:marker]
273+
fullName := strings.TrimSuffix(value[:marker], "/pull_request")
274274
slash := strings.IndexByte(fullName, '/')
275275
if slash <= 0 || slash == len(fullName)-1 {
276276
continue

‎internal/app/mcp_jobs.go‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -191,17 +191,15 @@ func jobResultStatus(job *contracts.JobResult) string {
191191
}
192192

193193
func portfolioReadFollowUpArguments(request mcpcontract.SyncPortfolioInput, login string, references []string) *mcpcontract.ListPullRequestPortfolioInput {
194+
if request.Selection == "explicit" {
195+
return &mcpcontract.ListPullRequestPortfolioInput{PullRequests: portfolioResultRefs(references), View: "compact"}
196+
}
194197
limit := request.Limit
195198
state := request.State
196199
if request.Selection == "authored" {
197200
if login != "" {
198201
return &mcpcontract.ListPullRequestPortfolioInput{Authors: []string{login}, State: state, Limit: limit, View: "compact"}
199202
}
200-
} else {
201-
state = "all"
202-
if limit == 0 || limit > len(references) {
203-
limit = len(references)
204-
}
205203
}
206204
if limit == 0 {
207205
limit = 20

‎internal/app/mcp_jobs_test.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,23 @@ func TestPortfolioFollowUpUsesPortfolioReadArguments(t *testing.T) {
136136
}
137137
}
138138

139+
func TestExplicitPortfolioFollowUpPreservesExactReferences(t *testing.T) {
140+
t.Parallel()
141+
job := &contracts.JobResult{
142+
Kind: jobKindSyncPullRequestPortfolio, Status: "succeeded",
143+
Request: `{"selection":"explicit","pull_requests":[{"owner":"acme","repo":"rocket","kind":"pull_request","number":7}]}`,
144+
Result: `{"status":"complete","pull_requests":["acme/rocket/pull_request#7"],"refreshed":1,"discovery_status":"complete"}`,
145+
}
146+
_, follow := jobArtifactsAndFollowUp(job, 1)
147+
if follow == nil || follow.Action.ListPortfolio == nil || len(follow.Action.ListPortfolio.PullRequests) != 1 {
148+
t.Fatalf("portfolio handoff = %+v", follow)
149+
}
150+
ref := follow.Action.ListPortfolio.PullRequests[0]
151+
if ref.Owner != "acme" || ref.Repo != "rocket" || ref.Kind != "pull_request" || ref.Number != 7 {
152+
t.Fatalf("exact portfolio handoff = %+v", ref)
153+
}
154+
}
155+
139156
func TestIndexJobPreservesFailuresAndBindsCompletedArtifactsToSnapshot(t *testing.T) {
140157
t.Parallel()
141158
job := &contracts.JobResult{

‎internal/app/mcp_portfolio_test.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,3 +148,29 @@ func TestPullRequestPortfolioKeepsComputingMergeabilityUnknown(t *testing.T) {
148148
t.Fatalf("portfolio = %+v", out)
149149
}
150150
}
151+
152+
func TestPullRequestPortfolioExactSelectionDoesNotSubstituteNewerPullRequests(t *testing.T) {
153+
t.Parallel()
154+
ctx := context.Background()
155+
svc := newSearchTestService(t)
156+
now := time.Unix(1000, 0).UTC()
157+
repo, err := svc.corpus.UpsertRepository(ctx, corpus.Repository{Owner: "acme", Name: "rocket"}, `{}`)
158+
if err != nil {
159+
t.Fatal(err)
160+
}
161+
if _, err := svc.corpus.UpsertThread(ctx, corpus.Thread{RepositoryID: repo.ID, Kind: corpus.ThreadKindPullRequest, Number: 1, State: "open", Title: "selected", Author: "alice", SourceUpdatedAt: now}, `{}`); err != nil {
162+
t.Fatal(err)
163+
}
164+
if _, err := svc.corpus.UpsertThread(ctx, corpus.Thread{RepositoryID: repo.ID, Kind: corpus.ThreadKindPullRequest, Number: 2, State: "open", Title: "newer unrelated", Author: "alice", SourceUpdatedAt: now.Add(time.Second)}, `{}`); err != nil {
165+
t.Fatal(err)
166+
}
167+
out, err := (&MCPReader{svc}).ListPullRequestPortfolio(ctx, mcpcontract.ListPullRequestPortfolioInput{
168+
PullRequests: []mcpcontract.ThreadRef{{Owner: "acme", Repo: "rocket", Kind: "pull_request", Number: 1}},
169+
})
170+
if err != nil {
171+
t.Fatal(err)
172+
}
173+
if out.Status != "partial" || len(out.PullRequests) != 1 || out.PullRequests[0].Number != 1 || len(out.UnavailablePullRequests) != 0 {
174+
t.Fatalf("exact portfolio = %+v", out)
175+
}
176+
}

‎internal/app/mcp_scalable_reads.go‎

Lines changed: 67 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,20 @@ func (r *MCPReader) ListPullRequestPortfolio(ctx context.Context, in mcpcontract
257257
if len(in.Authors) > 1 {
258258
return mcpcontract.ListPullRequestPortfolioOutput{}, errors.New("authors must contain at most one item")
259259
}
260+
if len(in.PullRequests) > 0 {
261+
if len(in.PullRequests) > 100 {
262+
return mcpcontract.ListPullRequestPortfolioOutput{}, errors.New("pull_requests must contain at most 100 items")
263+
}
264+
if len(in.Authors) > 0 || in.State != "" || in.Limit != 0 {
265+
return mcpcontract.ListPullRequestPortfolioOutput{}, errors.New("pull_requests cannot be combined with authors, state, or limit")
266+
}
267+
if err := rejectDuplicateThreadRefs(in.PullRequests); err != nil {
268+
return mcpcontract.ListPullRequestPortfolioOutput{}, err
269+
}
270+
if err := validatePullRequestRefs(in.PullRequests, "pull_requests"); err != nil {
271+
return mcpcontract.ListPullRequestPortfolioOutput{}, err
272+
}
273+
}
260274
if in.State == "" {
261275
in.State = "open"
262276
}
@@ -269,10 +283,10 @@ func (r *MCPReader) ListPullRequestPortfolio(ctx context.Context, in mcpcontract
269283
if in.View != "compact" && in.View != "full" {
270284
return mcpcontract.ListPullRequestPortfolioOutput{}, errors.New("view must be compact or full")
271285
}
272-
if in.Limit == 0 {
286+
if in.Limit == 0 && len(in.PullRequests) == 0 {
273287
in.Limit = 20
274288
}
275-
if in.Limit < 1 || in.Limit > 100 {
289+
if len(in.PullRequests) == 0 && (in.Limit < 1 || in.Limit > 100) {
276290
return mcpcontract.ListPullRequestPortfolioOutput{}, errors.New("limit must be between 1 and 100")
277291
}
278292
c, err := r.openReadOnlyCorpus(ctx)
@@ -283,11 +297,7 @@ func (r *MCPReader) ListPullRequestPortfolio(ctx context.Context, in mcpcontract
283297
if err != nil {
284298
return mcpcontract.ListPullRequestPortfolioOutput{}, err
285299
}
286-
author := ""
287-
if len(in.Authors) > 0 {
288-
author = strings.TrimSpace(in.Authors[0])
289-
}
290-
page, err := c.ListPullRequestPortfolioPage(ctx, author, in.State, in.Limit)
300+
page, unavailable, err := portfolioPage(ctx, c, in)
291301
if err != nil {
292302
return mcpcontract.ListPullRequestPortfolioOutput{}, err
293303
}
@@ -296,7 +306,11 @@ func (r *MCPReader) ListPullRequestPortfolio(ctx context.Context, in mcpcontract
296306
if err != nil {
297307
return mcpcontract.ListPullRequestPortfolioOutput{}, err
298308
}
299-
out := mcpcontract.ListPullRequestPortfolioOutput{Status: "complete", View: in.View, RuleVersion: "portfolio.v2", GeneratedAt: formatTime(r.now()), PullRequests: make([]mcpcontract.PullRequestPortfolioItem, 0, len(page.PullRequests)), Total: page.Total, Truncated: page.Truncated, SnapshotToken: snapshotIdentity(in.SnapshotToken, revision)}
309+
out := mcpcontract.ListPullRequestPortfolioOutput{Status: "complete", View: in.View, RuleVersion: "portfolio.v2", GeneratedAt: formatTime(r.now()), PullRequests: make([]mcpcontract.PullRequestPortfolioItem, 0, len(page.PullRequests)), Total: page.Total, Truncated: page.Truncated, UnavailablePullRequests: unavailable, SnapshotToken: snapshotIdentity(in.SnapshotToken, revision)}
310+
if len(unavailable) > 0 {
311+
out.Status = "partial"
312+
out.Recovery = recoveryPlan("portfolio_items_unavailable", "Some exact pull requests are not present in the local corpus. Refresh those exact pull requests, then reread the portfolio.", mcpcontract.RecoveryAction(mcpcontract.SyncPortfolioInput{Selection: "explicit", PullRequests: append([]mcpcontract.ThreadRef(nil), unavailable...)}))
313+
}
300314
for _, storedPR := range page.PullRequests {
301315
item, err := portfolioItem(storedPR, r.now(), readSet, format)
302316
if err != nil {
@@ -319,6 +333,51 @@ func (r *MCPReader) ListPullRequestPortfolio(ctx context.Context, in mcpcontract
319333
return out, nil
320334
}
321335

336+
func portfolioPage(ctx context.Context, c *corpus.Corpus, in mcpcontract.ListPullRequestPortfolioInput) (corpus.PortfolioPage, []mcpcontract.ThreadRef, error) {
337+
if len(in.PullRequests) == 0 {
338+
author := ""
339+
if len(in.Authors) > 0 {
340+
author = strings.TrimSpace(in.Authors[0])
341+
}
342+
page, err := c.ListPullRequestPortfolioPage(ctx, author, in.State, in.Limit)
343+
return page, nil, err
344+
}
345+
repositoryKeys := make([]corpus.RepositoryKey, 0, len(in.PullRequests))
346+
for _, ref := range in.PullRequests {
347+
repositoryKeys = append(repositoryKeys, corpus.RepositoryKey{Owner: ref.Owner, Name: ref.Repo})
348+
}
349+
repositories, err := c.GetRepositoriesBatch(ctx, repositoryKeys)
350+
if err != nil {
351+
return corpus.PortfolioPage{}, nil, err
352+
}
353+
threadKeys := make([]corpus.ThreadKey, 0, len(in.PullRequests))
354+
for _, ref := range in.PullRequests {
355+
if repository := repositories[corpus.RepositoryKey{Owner: ref.Owner, Name: ref.Repo}]; repository != nil {
356+
threadKeys = append(threadKeys, corpus.ThreadKey{RepositoryID: repository.ID, Kind: corpus.ThreadKindPullRequest, Number: ref.Number})
357+
}
358+
}
359+
threads, err := c.GetThreadsBatch(ctx, threadKeys)
360+
if err != nil {
361+
return corpus.PortfolioPage{}, nil, err
362+
}
363+
page := corpus.PortfolioPage{PullRequests: make([]corpus.PortfolioPullRequest, 0, len(in.PullRequests)), Total: len(in.PullRequests)}
364+
unavailable := make([]mcpcontract.ThreadRef, 0)
365+
for _, ref := range in.PullRequests {
366+
repository := repositories[corpus.RepositoryKey{Owner: ref.Owner, Name: ref.Repo}]
367+
if repository == nil {
368+
unavailable = append(unavailable, ref)
369+
continue
370+
}
371+
thread := threads[corpus.ThreadKey{RepositoryID: repository.ID, Kind: corpus.ThreadKindPullRequest, Number: ref.Number}]
372+
if thread == nil {
373+
unavailable = append(unavailable, ref)
374+
continue
375+
}
376+
page.PullRequests = append(page.PullRequests, corpus.PortfolioPullRequest{Owner: repository.Owner, Repo: repository.Name, Thread: *thread})
377+
}
378+
return page, unavailable, nil
379+
}
380+
322381
type portfolioReadSet struct {
323382
coverage map[corpus.ThreadFacetKey]*corpus.Coverage
324383
observations map[corpus.ThreadFacetKey]corpus.FacetObservationBatch

‎internal/app/sync_budget_test.go‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ type authoredHeaderReader struct {
1616
listRequests int
1717
prDetailRequests int
1818
now time.Time
19+
searchResult github.AuthoredPullRequestSearchResult
1920
}
2021

2122
func (r *authoredHeaderReader) GetRepository(context.Context, string, string) (github.Repository, github.RateInfo, error) {
@@ -61,12 +62,42 @@ func (*authoredHeaderReader) GetAuthenticatedIdentity(context.Context) (github.I
6162
}
6263

6364
func (r *authoredHeaderReader) SearchAuthoredPullRequests(context.Context, github.AuthoredPullRequestSearchOptions) (github.AuthoredPullRequestSearchResult, error) {
65+
if r.searchResult.Items != nil {
66+
return r.searchResult, nil
67+
}
6468
return github.AuthoredPullRequestSearchResult{Items: []github.Issue{
6569
{RepositoryOwner: "owner", RepositoryName: "repo", Kind: github.ThreadKindPullRequest, Number: 2, State: "open", Title: "first", CreatedAt: r.now, UpdatedAt: r.now},
6670
{RepositoryOwner: "owner", RepositoryName: "repo", Kind: github.ThreadKindPullRequest, Number: 3, State: "open", Title: "second", CreatedAt: r.now, UpdatedAt: r.now},
6771
}}, nil
6872
}
6973

74+
func TestAuthoredPullRequestSyncReportsItemLimitTruncation(t *testing.T) {
75+
t.Parallel()
76+
ctx := context.Background()
77+
paths := config.NewPaths(&config.Env{Home: t.TempDir()})
78+
svc, err := New(paths, "test", nil)
79+
if err != nil {
80+
t.Fatal(err)
81+
}
82+
defer func() { _ = svc.Close() }()
83+
if _, err := svc.Init(ctx); err != nil {
84+
t.Fatal(err)
85+
}
86+
now := time.Date(2026, 8, 8, 0, 0, 0, 0, time.UTC)
87+
svc.SetGitHubReader(&authoredHeaderReader{now: now, searchResult: github.AuthoredPullRequestSearchResult{
88+
Items: []github.Issue{{RepositoryOwner: "owner", RepositoryName: "repo", Kind: github.ThreadKindPullRequest, Number: 2, State: "open", Title: "first", CreatedAt: now, UpdatedAt: now}},
89+
Total: 2,
90+
Page: github.PageInfo{HasNext: true, NextPage: 2},
91+
}})
92+
out, err := svc.syncAuthoredPullRequests(ctx, authoredPullRequestSyncOptions{State: "open", Limit: 1, MaxRequests: 20}, func(string, string) error { return nil })
93+
if err != nil {
94+
t.Fatal(err)
95+
}
96+
if out.Status != "partial" || out.SearchIncomplete || out.RequestCapped || len(out.PullRequestTargets) != 1 {
97+
t.Fatalf("bounded discovery = %+v", out)
98+
}
99+
}
100+
70101
func TestAuthoredPullRequestSyncReusesSearchHeadersWithoutNPlusOne(t *testing.T) {
71102
t.Parallel()
72103
ctx := context.Background()

‎internal/mcpcontract/scalable_contracts.go‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -502,11 +502,12 @@ type SyncCIFailuresInput struct {
502502

503503
// ListPullRequestPortfolioInput filters and bounds the stored pull-request portfolio.
504504
type ListPullRequestPortfolioInput struct {
505-
Authors []string `json:"authors,omitempty" jsonschema:"Zero or one author login"`
506-
State string `json:"state,omitempty" jsonschema:"open, closed, or all"`
507-
Limit int `json:"limit,omitempty" jsonschema:"Maximum pull requests from 1 to 100; defaults to 20"`
508-
View string `json:"view,omitempty" jsonschema:"compact or full; defaults to compact"`
509-
SnapshotToken string `json:"snapshot_token,omitempty" jsonschema:"Optional immutable corpus snapshot token from a previous offline read"`
505+
Authors []string `json:"authors,omitempty" jsonschema:"Zero or one author login"`
506+
PullRequests []ThreadRef `json:"pull_requests,omitempty" jsonschema:"One to 100 exact pull requests; cannot be combined with authors, state, or limit"`
507+
State string `json:"state,omitempty" jsonschema:"open, closed, or all"`
508+
Limit int `json:"limit,omitempty" jsonschema:"Maximum pull requests from 1 to 100; defaults to 20"`
509+
View string `json:"view,omitempty" jsonschema:"compact or full; defaults to compact"`
510+
SnapshotToken string `json:"snapshot_token,omitempty" jsonschema:"Optional immutable corpus snapshot token from a previous offline read"`
510511
}
511512

512513
// PullRequestPortfolioItem contains source-backed PR facts and a deterministic
@@ -546,15 +547,16 @@ type PullRequestPortfolioItem struct {
546547

547548
// ListPullRequestPortfolioOutput contains a deterministic portfolio projection.
548549
type ListPullRequestPortfolioOutput struct {
549-
Status string `json:"status"`
550-
View string `json:"view"`
551-
RuleVersion string `json:"rule_version"`
552-
GeneratedAt string `json:"generated_at"`
553-
PullRequests []PullRequestPortfolioItem `json:"pull_requests"`
554-
Total int `json:"total"`
555-
Truncated bool `json:"truncated"`
556-
SnapshotToken string `json:"snapshot_token"`
557-
Recovery *RecoveryPlan `json:"recovery,omitempty"`
550+
Status string `json:"status"`
551+
View string `json:"view"`
552+
RuleVersion string `json:"rule_version"`
553+
GeneratedAt string `json:"generated_at"`
554+
PullRequests []PullRequestPortfolioItem `json:"pull_requests"`
555+
Total int `json:"total"`
556+
Truncated bool `json:"truncated"`
557+
UnavailablePullRequests []ThreadRef `json:"unavailable_pull_requests,omitempty"`
558+
SnapshotToken string `json:"snapshot_token"`
559+
Recovery *RecoveryPlan `json:"recovery,omitempty"`
558560
}
559561

560562
// PortfolioSubjectInput identifies local candidate state for offline overlap analysis.

‎internal/mcpserver/portfolio.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ import (
1010
)
1111

1212
func (s *Server) listPullRequestPortfolio(ctx context.Context, _ *mcp.CallToolRequest, in mcpcontract.ListPullRequestPortfolioInput) (*mcp.CallToolResult, mcpcontract.ListPullRequestPortfolioOutput, error) {
13-
if in.State == "" {
13+
if len(in.PullRequests) == 0 && in.State == "" {
1414
in.State = "open"
1515
}
16-
if in.Limit == 0 {
16+
if len(in.PullRequests) == 0 && in.Limit == 0 {
1717
in.Limit = 20
1818
}
1919
if in.View == "" {

‎internal/mcpserver/scalable.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,8 @@ func (s *Server) registerScalable() {
335335
setDefault(sc, "limit", 20)
336336
setEnum(sc, "view", "compact", "full")
337337
setDefault(sc, "view", "compact")
338+
setArrayBounds(sc, "pull_requests", 1, 100)
339+
constrainPullRequestRefs(sc, "pull_requests")
338340
}), output: outputSchema[mcpcontract.ListPullRequestPortfolioOutput]("Offline pull-request portfolio with explainable attention states."), handler: s.listPullRequestPortfolio})
339341
addCatalogTool(s, catalogTool[mcpcontract.SearchPullRequestFeedbackInput, mcpcontract.SearchPullRequestFeedbackOutput]{name: mcpcontract.ToolSearchPullRequestFeedback, title: "Search exact pull-request comments by commenter login", description: "Use this offline search after github.index_pull_request_feedback and jobs.get when you need exact feedback_author, PR state, merge state, channel, resolution, review-state, anchor, date, or body filters. It reads only the local normalized feedback projection and never contacts GitHub or writes corpus state. Matches preserve exact comment and thread identities; incomplete discovery or facets return partial/unknown coverage with a typed recovery action, not a false empty result.", annotations: readOnly, supportedBy: supports[PullRequestFeedbackSearcher], input: inputSchema[mcpcontract.SearchPullRequestFeedbackInput](func(sc *schemaBuilder) {
340342
setEnum(sc, "state", "open", "closed", "all")

0 commit comments

Comments
 (0)