Skip to content

Commit 265ccdc

Browse files
authored
Merge pull request #214 from morluto/codex/redesign-mcp-tools-v1-7
refactor(mcp): redesign tool contracts and agent workflows
2 parents 5099e85 + 1b214c4 commit 265ccdc

69 files changed

Lines changed: 3496 additions & 1050 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

README.md

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,13 @@ MCP capabilities are deliberately separate:
265265
The CLI advertises the focused `contribute` toolset by default. Add specialized
266266
surfaces only when needed: `mcp serve --toolsets=contribute,code`,
267267
`--toolsets=contribute,research`, `--toolsets=contribute,portfolio`,
268-
`--toolsets=contribute,advanced`, or `--toolsets=all`. Smaller catalogs reduce
268+
`--toolsets=contribute,diagnostics`, `--toolsets=contribute,advanced`,
269+
`--toolsets=contribute,patterns`, `--toolsets=contribute,concerns`, or
270+
`--toolsets=all`. The `patterns` profile adds bounded repository-level
271+
accepted-fix mining with automatic finalist hydration. The `concerns` profile
272+
contains the complete create, update, transition, link, and atomic-promotion
273+
lifecycle; concern creation is intentionally not exposed alone in the default
274+
profile. Smaller catalogs reduce
269275
overlapping choices and agent context cost; toolsets change discovery only,
270276
not authority or side-effect annotations.
271277
Add `--read-only` to remove every tool whose MCP annotation permits local
@@ -294,14 +300,15 @@ github.sync_repository_context -> jobs.get -> corpus.get_repositories
294300
-> workflow.prepare_issue_set
295301
```
296302

297-
For contribution follow-up, use `github.sync_authored_pull_requests`, then
298-
`github.sync_pull_request_status` and `corpus.list_pull_request_portfolio`.
299-
Missing coverage is returned as unknown rather than as a false zero or negative.
300-
PR status currently includes lifecycle, mergeability, head/base revisions, and
301-
stored reviews. Checks, unresolved review threads, detailed merge state, merge
302-
queue, and portfolio overlap are deliberately reported as unavailable. See the
303+
For the common portfolio refresh outcome, the specialized `portfolio` profile
304+
offers `github.sync_portfolio`; the authored-discovery and exact-status
305+
primitives remain available for partial recovery. Missing coverage is returned
306+
as unknown rather than as a false zero or negative. PR status keeps independent
307+
coverage for lifecycle, mergeability, revisions, reviews, checks, unresolved
308+
conversations, merge state, queue, closing issues, and changed files. See the
303309
[scalable MCP workflow guide](docs/mcp-scalable-workflows.md) for recovery and
304-
coverage details.
310+
coverage details and the [MCP redesign notes](docs/mcp-tool-redesign.md) for
311+
schema ownership, response contracts, and the model-evidence ship gate.
305312

306313
Contribution workflow resources and prompts are available for agents:
307314

docs/agent-tool-evaluation.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,15 @@ filesystem and context. A separate reviewer scores semantic correctness,
4848
required evidence, the critical discriminator, and uncertainty before
4949
comparing tool calls, response bytes, or latency.
5050

51+
The v4 suite turns the MCP redesign decisions into controlled comparisons:
52+
stored versus live search, aggregate versus manual issue preparation,
53+
discriminated input modes, resource versus scalar reads, portfolio composition,
54+
DeepWiki truncation recovery, evidence coverage, and the concern lifecycle.
55+
It fingerprints the exact serialized catalog for every condition and requires
56+
at least three trials. The hash-committed semantic oracle is mounted only in the
57+
evaluator process and applied before efficiency metrics; the public fixture
58+
itself is not evidence that model trials ran.
59+
5160
Use the same model, sampling settings, corpus fixture revision, toolsets, and
5261
read-only mode for baseline/candidate comparisons. Save initialize, tools/list,
5362
tool calls, tool results, final answer, elapsed time, and failures. At least
@@ -79,6 +88,10 @@ contracts include:
7988
- preserving semantic references across concise and detailed responses;
8089
- returning stable, duplicate-free pagination;
8190
- avoiding poll suggestions for terminal jobs.
91+
- comparing `workflow.mine_repository_fix_patterns` with manual
92+
search/select/hydrate loops on a repository where closed PR headers have
93+
unknown merge state; score confirmed merged, closed-unmerged, superseded,
94+
open, and unknown outcomes separately.
8295

8396
Run the focused suite with:
8497

docs/mcp-scalable-workflows.md

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,35 @@ explicit `raw_query` field; there is no deprecated alias.
7777
hydrated, a closed PR's `merged` value is omitted and outcome-sensitive
7878
offline reads report it as unknown rather than closed-unmerged.
7979

80+
### Repository fix-pattern mining
81+
82+
The opt-in `patterns` profile exposes the trace-backed aggregate:
83+
84+
```text
85+
workflow.mine_repository_fix_patterns
86+
-> jobs.get
87+
-> gitcontribute://fix-pattern-report/{job_id}
88+
```
89+
90+
Use it to summarize how one stored repository handled caller-defined symptom
91+
categories over an explicit observation window. It searches the local corpus
92+
first, refreshes only a bounded set of finalists whose merge outcome is
93+
unknown, and persists a typed report. `candidate_limit`, `hydration_limit`, and
94+
`representative_limit` bound search, network work, and returned context
95+
independently. Set `hydration_limit: 0` to request a strictly offline analysis;
96+
otherwise the workflow performs GitHub reads and idempotent local writes.
97+
98+
Coverage reports candidate matches, unique pull requests, unknown outcomes
99+
before and after hydration, hydration failures, and candidate truncation.
100+
Merged, closed-unmerged, superseded, open, and unknown remain separate
101+
outcomes. Only closed PRs with unknown merge state consume the hydration
102+
budget. An example is marked `accepted_fix` only when refreshed state confirms
103+
it was merged and stored pull-request text contains an explicit closing
104+
relationship. A similar closed PR is never promoted to accepted-fix evidence.
105+
Relationship and proof-style labels are bounded lexical projections, so the
106+
report preserves their supporting phrase and states that similarity is not
107+
causal proof.
108+
80109
## Exact issue-set preparation
81110

82111
Use `workflow.prepare_issue_set` when the contribution is already scoped by
@@ -170,7 +199,7 @@ Repository and dossier absence have different recovery paths:
170199
retry the offline read.
171200
- `dossier_not_persisted` means the repository exists locally but has no saved
172201
dossier. Use `corpus.get_repositories` for metadata and dossier availability;
173-
call `corpus.build_repository_dossier` only when creating that local artifact
202+
call `workflow.build_repository_dossier` only when creating that local artifact
174203
is actually required.
175204

176205
Retrying `corpus.get_repository_dossier` alone cannot resolve either state.

docs/mcp-tool-redesign.md

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
# MCP tool redesign
2+
3+
GitContribute targets `github.com/modelcontextprotocol/go-sdk`
4+
`v1.7.0-pre.3` and negotiates MCP `2026-07-28`. The server continues to
5+
register generic SDK tools so the SDK owns input decoding and output-schema
6+
validation at the protocol boundary.
7+
8+
## Contract ownership
9+
10+
Schema semantics live with their Go values. Probability, similarity, radar
11+
score, progress, non-negative counts, batch status, and job status are reusable
12+
typed schema values. A field named `score`, `confidence`, `status`, `kind`, or
13+
`result` never receives semantics from its JSON name alone.
14+
15+
Multi-mode tools keep an object root and compose draft-2020-12 schema nodes
16+
over SDK-inferred Go structs. `oneOf`, `required`, `not`,
17+
`dependentRequired`, bounds, defaults, and constants express protocol shape.
18+
Handlers retain checks for repository existence, authorization, lifecycle
19+
legality, RFC 3339 values, and stored-state consistency. There is no parallel
20+
JSON decoder or validator.
21+
22+
## Response and side-effect boundaries
23+
24+
`jobs.get` returns bounded status, progress, typed artifact references, and a
25+
suggested follow-up; it does not expose stored request or result blobs.
26+
Repository dossiers, repository projections, and manifest statements are
27+
typed. DeepWiki defaults to 32 KiB and directs truncated reads toward structure
28+
or a focused question before a larger response.
29+
30+
Tool results link durable dossiers, investigations, opportunities, evidence,
31+
readiness reports, and job artifacts with SDK-native resource links. Scalar
32+
read tools remain available until the v4 client-compatibility evaluation shows
33+
that supported clients reliably follow resources. Resources and scalar tools
34+
must not both be read for one result.
35+
36+
The catalog preserves offline reads, network reads, local writes, process
37+
execution, and external mutation as separate capabilities. The default
38+
`contribute` profile contains no partial concern lifecycle; the specialized
39+
`concerns` profile exposes the complete lifecycle. The dossier build operation
40+
is named `workflow.build_repository_dossier` because it writes local state.
41+
42+
## Consolidation decisions
43+
44+
Exact issue preparation remains the aggregate `workflow.prepare_issue_set`.
45+
Durable submission and polling, validation definition and authorized
46+
execution, and commit inspection and planning remain separate because each
47+
boundary permits meaningful agent judgment or authorization.
48+
49+
Live repository search includes local dossier availability. The specialized
50+
portfolio profile offers `github.sync_portfolio`, a bounded durable job that
51+
uses the existing authored-discovery and exact-status operations and chunks
52+
status refreshes at 50 pull requests. The underlying primitives remain
53+
available for recovery and partial workflows.
54+
55+
The `patterns` profile offers
56+
`workflow.mine_repository_fix_patterns`. It consolidates the observed
57+
search-select-hydrate-rescan loop while preserving the real durable-job,
58+
network-read, and local-write boundaries. The triggering agent trace found 587
59+
otherwise matching pull requests with unknown merge state, then required 26
60+
exact hydrations to recover 21 confirmed merged examples; one persistence step
61+
also encountered `SQLITE_BUSY`. The aggregate therefore hydrates only bounded
62+
unknown-state finalists, reports unknowns before and after hydration, and
63+
separates confirmed merged fixes from merely similar closed work. It remains
64+
opt-in until held-out model trials justify default-profile membership.
65+
66+
DeepWiki retains one tool with three discriminated modes. Static profiles
67+
remain the capability-discovery mechanism; the server does not mutate a global
68+
catalog or add a custom discovery meta-tool.
69+
70+
## Evidence gate
71+
72+
Catalog byte measurements are regression proxies, not model evidence. The v4
73+
fixtures under `internal/mcpserver/testdata/agent-eval/v4` freeze catalog
74+
fingerprints and compare the ten ambiguous workflows called out in the design
75+
review. Each condition requires at least three trials with frozen model,
76+
sampling settings, catalog, corpus revision, permissions, prompt, and token
77+
budget.
78+
79+
Semantic correctness and side-effect correctness are gates. Only afterward may
80+
invalid calls, redundant calls, result tokens, latency, and recovery success be
81+
compared. No further default-profile reduction ships until those trials show at
82+
least 25% lower model-visible catalog context without a meaningful task-success
83+
regression, an increased invalid-call rate, or lost side-effect disclosure.

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,10 @@ require (
1111
github.com/google/jsonschema-go v0.4.3
1212
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
1313
github.com/google/uuid v1.6.0
14-
github.com/modelcontextprotocol/go-sdk v1.6.1
14+
github.com/modelcontextprotocol/go-sdk v1.7.0-pre.3
1515
github.com/pelletier/go-toml/v2 v2.4.3
1616
github.com/pressly/goose/v3 v3.24.0
17+
github.com/sethvargo/go-retry v0.3.0
1718
github.com/shirou/gopsutil/v4 v4.26.6
1819
github.com/sourcegraph/go-diff v0.8.0
1920
github.com/zalando/go-keyring v0.2.8
@@ -58,7 +59,6 @@ require (
5859
github.com/rivo/uniseg v0.4.7 // indirect
5960
github.com/segmentio/asm v1.2.0 // indirect
6061
github.com/segmentio/encoding v0.5.4 // indirect
61-
github.com/sethvargo/go-retry v0.3.0 // indirect
6262
github.com/tklauser/go-sysconf v0.3.16 // indirect
6363
github.com/tklauser/numcpus v0.11.0 // indirect
6464
github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ github.com/mfridman/interpolate v0.0.2 h1:pnuTK7MQIxxFz1Gr+rjSIx9u7qVjf5VOoM/u6B
9898
github.com/mfridman/interpolate v0.0.2/go.mod h1:p+7uk6oE07mpE/Ik1b8EckO0O4ZXiGAfshKBWLUM9Xg=
9999
github.com/mitchellh/hashstructure/v2 v2.0.2 h1:vGKWl0YJqUNxE8d+h8f6NJLcCJrgbhC4NcD46KavDd4=
100100
github.com/mitchellh/hashstructure/v2 v2.0.2/go.mod h1:MG3aRVU/N29oo/V/IhBX8GR/zz4kQkprJgF2EVszyDE=
101-
github.com/modelcontextprotocol/go-sdk v1.6.1 h1:0zOSupjKUxPKSocPT1Wtago+mUHU2/uZ4xSOY0FGReU=
102-
github.com/modelcontextprotocol/go-sdk v1.6.1/go.mod h1:kzm3kzFL1/+AziGOE0nUs3gvPoNxMCvkxokMkuFapXQ=
101+
github.com/modelcontextprotocol/go-sdk v1.7.0-pre.3 h1:SEAY9IduDif4iApnZgpFkjFIdo3askSGZVbZIYyTy6I=
102+
github.com/modelcontextprotocol/go-sdk v1.7.0-pre.3/go.mod h1:dL7u98E/zjJTGzEq+j30jQ8K2k1mb6LeAH4inEcSGts=
103103
github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELUXHmA=
104104
github.com/muesli/cancelreader v0.2.2/go.mod h1:3XuTXfFS2VjM+HTLZY9Ak0l6eUKfijIfMUZ4EgX0QYo=
105105
github.com/ncruces/go-strftime v1.0.0 h1:HMFp8mLCTPp341M/ZnA4qaf7ZlsbTc+miZjCLOFAw7w=

internal/app/app_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ func TestMCPReaderLocalReads(t *testing.T) {
313313
if err != nil {
314314
t.Fatalf("mcp repository: %v", err)
315315
}
316-
if repo.Owner != "acme" || repo.Repo != "rocket" || repo.Fields["stars"] != 42 || repo.UpdatedAt != "2024-01-01T00:00:00Z" {
316+
if repo.Owner != "acme" || repo.Repo != "rocket" || repo.Stars == nil || *repo.Stars != 42 || repo.UpdatedAt != "2024-01-01T00:00:00Z" {
317317
t.Fatalf("unexpected repository output: %+v", repo)
318318
}
319319

@@ -356,8 +356,8 @@ func TestMCPReaderLocalReads(t *testing.T) {
356356
if dossier.Owner != "acme" || dossier.Repo != "rocket" {
357357
t.Fatalf("unexpected dossier output: %+v", dossier)
358358
}
359-
if _, ok := dossier.Sections["stars"]; !ok {
360-
t.Fatalf("dossier missing stars section: %+v", dossier.Sections)
359+
if dossier.Sections.Stars != 42 {
360+
t.Fatalf("dossier missing typed stars section: %+v", dossier.Sections)
361361
}
362362

363363
_, err = reader.Thread(ctx, mcpcontract.ThreadInput{Owner: "acme", Repo: "rocket", Kind: "issue", Number: 404})

internal/app/dossier_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ func TestBuildAndGetRepositoryDossier(t *testing.T) {
162162
if err != nil {
163163
t.Fatalf("read persisted MCP dossier: %v", err)
164164
}
165-
if stars := mcpDossier.Sections["stars"]; stars != 10 {
165+
if stars := mcpDossier.Sections.Stars; stars != 10 {
166166
t.Fatalf("MCP dossier stars = %v, want persisted value 10", stars)
167167
}
168168

internal/app/hydration.go

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,9 @@ func (s *Service) HydrateThread(ctx context.Context, repo contracts.RepoRef, num
9595
}
9696
cleanupCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 5*time.Second)
9797
defer cancel()
98-
_ = c.FailRun(cleanupCtx, run.ID, hydrateErr.Error())
98+
_ = corpus.RetryBusy(cleanupCtx, func(ctx context.Context) error {
99+
return c.FailRun(ctx, run.ID, hydrateErr.Error())
100+
})
99101
}()
100102

101103
repoProjection, err := c.GetRepository(ctx, ref.Owner, ref.Repo)
@@ -194,7 +196,9 @@ func (s *Service) HydrateThread(ctx context.Context, repo contracts.RepoRef, num
194196
"pages": result.Pages,
195197
"requests": result.Requests,
196198
})
197-
if err := c.FinishRun(ctx, run.ID, string(statsPayload)); err != nil {
199+
if err := corpus.RetryBusy(ctx, func(ctx context.Context) error {
200+
return c.FinishRun(ctx, run.ID, string(statsPayload))
201+
}); err != nil {
198202
hydrateErr = err
199203
return nil, hydrateErr
200204
}
@@ -285,13 +289,17 @@ func (f *facetRunner) hydrateIssueTimeline() (HydratedFacet, error) {
285289
opts.Page = res.Page.NextPage
286290
}
287291
if !complete {
288-
if _, err := f.c.AdvanceFacetCAS(f.ctx, f.repoID, &f.threadID, FacetIssueTimeline, sourceUpdatedAt, false, f.runID, expectedSequence); err != nil {
292+
if _, err := corpus.RetryBusyValue(f.ctx, func(ctx context.Context) (bool, error) {
293+
return f.c.AdvanceFacetCAS(ctx, f.repoID, &f.threadID, FacetIssueTimeline, sourceUpdatedAt, false, f.runID, expectedSequence)
294+
}); err != nil {
289295
return HydratedFacet{}, err
290296
}
291297
return HydratedFacet{Facet: FacetIssueTimeline, Count: total, Pages: pages, Complete: false}, nil
292298
}
293299
collapseFacetSearchText(pageObservations)
294-
applied, err := f.c.ApplyFacetObservationSetCAS(f.ctx, f.repoID, &f.threadID, FacetIssueTimeline, sourceUpdatedAt, pageObservations, true, f.runID, expectedSequence)
300+
applied, err := corpus.RetryBusyValue(f.ctx, func(ctx context.Context) (bool, error) {
301+
return f.c.ApplyFacetObservationSetCAS(ctx, f.repoID, &f.threadID, FacetIssueTimeline, sourceUpdatedAt, pageObservations, true, f.runID, expectedSequence)
302+
})
295303
if err != nil {
296304
return HydratedFacet{}, err
297305
}
@@ -356,7 +364,9 @@ func (f *facetRunner) persistTimelineResolution(events []github.IssueTimelineEve
356364
return errors.New("closing commit timeline observation is unavailable")
357365
}
358366
}
359-
_, err := f.c.SaveResolutionRecord(f.ctx, corpus.ResolutionRecord{ThreadID: f.threadID, Kind: kind, Summary: summary, RuleVersion: "resolution.v1", SourceUpdatedAt: sourceUpdatedAt, SourceObservationRefs: refs})
367+
_, err := corpus.RetryBusyValue(f.ctx, func(ctx context.Context) (*corpus.ResolutionRecord, error) {
368+
return f.c.SaveResolutionRecord(ctx, corpus.ResolutionRecord{ThreadID: f.threadID, Kind: kind, Summary: summary, RuleVersion: "resolution.v1", SourceUpdatedAt: sourceUpdatedAt, SourceObservationRefs: refs})
369+
})
360370
return err
361371
}
362372

@@ -427,13 +437,17 @@ func hydratePaginatedFacet[T any](f *facetRunner, spec paginatedFacetSpec[T]) (H
427437
return HydratedFacet{}, err
428438
}
429439
if !complete {
430-
if _, err := f.c.AdvanceFacetCAS(f.ctx, f.repoID, &f.threadID, spec.facet, sourceUpdatedAt, false, f.runID, expectedSequence); err != nil {
440+
if _, err := corpus.RetryBusyValue(f.ctx, func(ctx context.Context) (bool, error) {
441+
return f.c.AdvanceFacetCAS(ctx, f.repoID, &f.threadID, spec.facet, sourceUpdatedAt, false, f.runID, expectedSequence)
442+
}); err != nil {
431443
return HydratedFacet{}, err
432444
}
433445
return HydratedFacet{Facet: spec.facet, Count: total, Pages: pages, Complete: false}, nil
434446
}
435447
collapseFacetSearchText(pageObservations)
436-
if _, err := f.c.ApplyFacetObservationSetCAS(f.ctx, f.repoID, &f.threadID, spec.facet, sourceUpdatedAt, pageObservations, true, f.runID, expectedSequence); err != nil {
448+
if _, err := corpus.RetryBusyValue(f.ctx, func(ctx context.Context) (bool, error) {
449+
return f.c.ApplyFacetObservationSetCAS(ctx, f.repoID, &f.threadID, spec.facet, sourceUpdatedAt, pageObservations, true, f.runID, expectedSequence)
450+
}); err != nil {
437451
return HydratedFacet{}, err
438452
}
439453
return HydratedFacet{Facet: spec.facet, Count: total, Pages: pages, Complete: true}, nil
@@ -482,7 +496,9 @@ func (f *facetRunner) hydratePullRequestDetails() (HydratedFacet, error) {
482496
}
483497

484498
pages := []corpus.FacetObservationInput{{SourceUpdatedAt: updatedAt, Payload: string(payload)}}
485-
applied, err := f.c.ApplyFacetObservationSetCAS(f.ctx, f.repoID, &f.threadID, FacetPRDetails, updatedAt, pages, true, f.runID, expectedSequence)
499+
applied, err := corpus.RetryBusyValue(f.ctx, func(ctx context.Context) (bool, error) {
500+
return f.c.ApplyFacetObservationSetCAS(ctx, f.repoID, &f.threadID, FacetPRDetails, updatedAt, pages, true, f.runID, expectedSequence)
501+
})
486502
if err != nil {
487503
return HydratedFacet{}, err
488504
}
@@ -516,7 +532,9 @@ func (f *facetRunner) hydratePullRequestDetails() (HydratedFacet, error) {
516532
} else {
517533
projection.MergedAt = time.Time{}
518534
}
519-
stored, err := f.c.UpsertThread(f.ctx, projection, string(payload))
535+
stored, err := corpus.RetryBusyValue(f.ctx, func(ctx context.Context) (*corpus.Thread, error) {
536+
return f.c.UpsertThread(ctx, projection, string(payload))
537+
})
520538
if err != nil {
521539
return HydratedFacet{}, fmt.Errorf("project pr details: %w", err)
522540
}

0 commit comments

Comments
 (0)