refactor(mcp,types): phase 10 — review alignment + P1/P2 fixes + docs parity#36
Merged
Merged
Conversation
…docs parity
Single PR that closes the gaps a four-agent review surfaced after
phases 0-9 landed. "Hiçbir şeyi atlamadan" — every finding is
addressed in this change, from cohesion polish to the docs drift the
refactor left in its wake.
### Provider contracts move to @contentrain/types
- packages/types/src/provider.ts (new) — RepoReader, RepoWriter,
RepoProvider, ProviderCapabilities, LOCAL_CAPABILITIES, FileChange,
Branch, Commit, CommitAuthor, FileDiff, MergeResult, ApplyPlanInput.
Third-party tools can now implement a custom RepoProvider without
a runtime dependency on @contentrain/mcp.
- packages/types/src/index.ts — repository.provider widened from
`'github'` to `'github' | 'gitlab'`; all provider contracts
re-exported from the root.
- packages/mcp/src/core/contracts/index.ts — now a thin barrel that
re-exports from @contentrain/types. The per-file contract modules
(branch.ts / capabilities.ts / file-change.ts / provider.ts /
repo-reader.ts / repo-writer.ts) are deleted; existing MCP imports
through `core/contracts` continue to work unchanged.
### P1 — remote write base branch invariant
- packages/mcp/src/tools/commit-plan.ts (new) extracts the
LocalProvider vs remote RepoProvider dispatch from four repeated
blocks in content.ts / model.ts into `commitThroughProvider`.
- Remote writes now always fork from CONTENTRAIN_BRANCH, matching the
local transaction flow. Previously they used
`config.repository.default_branch ?? 'contentrain'` — in any
project that set `repository.default_branch: 'main'`, feature
branches were silently forked from `main`, breaking the
`contentrain` singleton invariant.
### P1 — stale remote context.json + validation
- packages/mcp/src/core/overlay-reader.ts (new) — OverlayReader wraps
a RepoReader and layers pending FileChanges on top. Semantics:
pending adds → visible; pending deletes → missing; base falls
through for everything else. listDirectory merges additions into
the base listing, removes deletes, and surfaces nested
subdirectories for deeper pending paths.
- commit-plan.ts: `buildContextChange` is now called against an
OverlayReader over the pending plan, so `stats.entries` /
`stats.models` reflect the state the new commit produces instead
of the pre-change base branch.
- tools/content.ts: post-save validateProject runs against the same
overlay on remote, so the validation envelope matches the new
content state. Local flow still uses projectRoot-based validation
— the transaction layer has already written the worktree.
### P2 — read-only tools work over remote providers
- tools/context.ts (`contentrain_status`, `contentrain_describe`)
and tools/content.ts (`contentrain_content_list`) no longer gate
on `!projectRoot`. They route through `provider` and degrade
gracefully when no project root is available: stack detection,
branch health, and `content_list --resolve` stay local-only and
are skipped / rejected with a descriptive error on remote.
- core/content-manager.ts: listContent gains a reader overload
(`listContentViaReader`) that handles all four kinds (singleton,
collection, document, dictionary). Relation hydration
(`opts.resolve`) still requires local filesystem access because
the walk touches other models.
### P2 — public export surface unbroken
- packages/mcp/package.json: adds `./core/contracts` and
`./providers/local` to `exports` and the build targets. Both paths
were referenced in package docstrings since phase 5 but were never
publishable — external imports failed with
`ERR_PACKAGE_PATH_NOT_EXPORTED`. Now they resolve.
### Cohesion — provider cleanup
- packages/mcp/src/providers/shared/ (new) consolidates helpers that
were duplicated across GitHub + GitLab:
- `errors.ts:isNotFoundError` — unified strict-status-based check.
Removes four per-provider `isNotFound` helpers and GitLab's
lenient description-match fallback that risked masking non-404
failures silently.
- `paths.ts:normaliseContentRoot + resolveRepoPath` — one copy
instead of two identical files under github/ and gitlab/.
- tools/workflow.ts: `contentrain_submit` and `contentrain_merge`
now gate on explicit `provider.capabilities.X` (pushRemote /
localWorktree) instead of the `!projectRoot` proxy. Matches the
pattern `tools/normalize.ts` adopted in phase 6.
### Tool surface
No changes. Same 16 tools, same parameters, same response JSON.
### Docs
Tool-count drift (15 → 16) fixed in root README, CLAUDE.md,
docs/packages/mcp.md frontmatter, docs/concepts.md. Tool lists
updated to include `contentrain_merge`.
"Local-first only / no GitHub API" framing rewritten in four spots
(packages/mcp/README.md opening + design constraints,
docs/packages/mcp.md frontmatter + body section). New framing:
"provider-agnostic, local-first by default, optional GitHub and
GitLab backends".
New VitePress pages (sidebar + nav updated):
- docs/guides/providers.md — Local / GitHub / GitLab overview and
capability matrix
- docs/guides/http-transport.md — HTTP deployment, Bearer auth,
Studio / CI / remote-agent patterns
- docs/reference/providers.md — RepoProvider contract reference
Existing pages updated: getting-started (HTTP transport tip),
concepts (provider concept + capability gates), studio (content
engine is MCP over HTTP + RepoProvider), normalize (LocalProvider
warning), config reference (widened provider type).
CLI:
- packages/cli/src/commands/serve.ts description now says
"stdio + HTTP"
- packages/cli/README.md adds a "MCP HTTP (Studio, CI, remote
drivers)" subsection under `serve` modes and updates branch
references from `contentrain/*` to `cr/*`.
Rules and skills:
- packages/rules/shared/mcp-usage.md — new "Transport and Provider
Capabilities" section with the capability matrix;
`contentrain_merge` added to the tool catalogue; branch naming
corrected to `cr/*`; `capability_required` entry added to the
common-errors table.
- packages/rules/shared/{workflow,normalize}-rules.md — branch
examples migrated to `cr/*`.
- packages/skills/skills/contentrain-normalize/SKILL.md — new
"Transport Requirements" section documenting the LocalProvider
constraint.
- packages/skills/workflows/{contentrain-normalize,translate}.md —
branch references migrated to `cr/*`.
### Tests
- packages/mcp/tests/providers/local/reader.test.ts (new, 11 cases)
— LocalReader was previously untested at the unit level.
- packages/mcp/tests/core/overlay-reader.test.ts (new, 11 cases)
— exercises the primitive behind both P1 fixes.
- packages/mcp/tests/server/fixtures/github-mock.ts (new) — shared
mock for GitHubProvider HTTP E2Es; previously inlined in each
case.
- packages/mcp/tests/server/http.test.ts — five new E2Es
(content_delete, model_save, model_delete, validate read-only,
status read-only) plus an updated capability-error test that
exercises `contentrain_submit` (genuinely local-only) instead of
the now-remote-capable `contentrain_status`.
### Changesets
- `.changeset/types-provider-alignment.md` — @contentrain/types minor
(provider widened + contracts exposed).
- `.changeset/mcp-phase-10-alignment.md` — @contentrain/mcp minor
(new subpath exports + P1 bug fixes + read-only-over-remote
feature additions).
### Verification
- pnpm -r typecheck (8 packages) → 0 errors
- npx oxlint (monorepo, 397 files) → 0 warnings
- pnpm --filter @contentrain/mcp build → clean, 130 files packed
- vitest run tests/core tests/conformance tests/serialization-parity
tests/git tests/providers tests/server tests/util
→ 440/440 green, 2 skipped (22 new tests land in this pass: 11
LocalReader + 11 OverlayReader, plus 5 new HTTP E2Es replacing
one rewritten case).
- vitest run tests/tools → 90/91 on parallel; 1 pre-existing flaky
timeout on `content.test.ts > blocks new writes when 80 active
contentrain branches exist` (80 sequential git checkouts inside a
120s budget, unrelated to phase 10). Re-run in isolation: 17/17
green in 411s.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
One PR that closes every gap a four-agent review surfaced after phases 0–9 landed. Scope is broad but coherent: each piece unblocks another (the overlay reader enables both P1 fixes; the read-only tool relaxation assumes the overlay; the docs describe the now-correct behaviour).
What's in the box
Contracts moved to
@contentrain/types(minor)RepoReader/RepoWriter/RepoProvider/ProviderCapabilities/FileChange/Branch/Commit/FileDiff/MergeResult/ApplyPlanInputandLOCAL_CAPABILITIESnow live in@contentrain/types/providerand are re-exported from the types package root. Third-party RepoProvider implementations no longer need a runtime dependency on@contentrain/mcp.ContentrainConfig.repository.providerwidens from'github'to'github' | 'gitlab'.@contentrain/mcp/core/contractskeeps re-exporting everything for backward compat.P1 — remote write base branch invariant
commit-plan.ts:commitThroughProvidernow always forks remote feature branches fromCONTENTRAIN_BRANCH, matching the local transaction flow. Previously usedconfig.repository.default_branch ?? 'contentrain'→ projects withdefault_branch: 'main'silently broke thecontentrainsingleton invariant.P1 — stale remote context.json + validation
New
core/overlay-reader.ts:OverlayReaderlayers pendingFileChanges on top of the base reader.buildContextChangeand post-savevalidateProjectnow see the post-commit state, not the pre-change base branch. Stats in committedcontext.jsonand validation envelopes reflect the new commit.P2 — read-only tools on remote providers
contentrain_status,contentrain_describe,contentrain_content_listdrop the!projectRootgate and route throughprovider. Local-only features (stack detection, branch health,content_list --resolve) stay local and degrade gracefully. The MCP server's documented "read-only tools work over HTTP" claim is now true.P2 — broken export paths fixed
@contentrain/mcp/core/contractsand@contentrain/mcp/providers/localare now inpackage.jsonexports. External imports previously failed withERR_PACKAGE_PATH_NOT_EXPORTED.Cohesion polish
tools/commit-plan.tscollapses the 4×if (provider instanceof LocalProvider) { … } else { … }applyPlan dispatch into one shared helper with a uniform return shape.providers/shared/{errors,paths}.tsconsolidateisNotFoundError+normaliseContentRoot/resolveRepoPathpreviously duplicated across GitHub + GitLab. GitLab's lenient description-match fallback is gone — strict status-based only (removes silent-suppression risk).tools/workflow.ts—submit+mergegate on explicitprovider.capabilities.Xinstead of the!projectRootproxy, matchingnormalize.ts.Docs parity
Tool count drift (15 → 16) fixed in: root
README.md(×3),CLAUDE.md(×2),docs/packages/mcp.mdfrontmatter,docs/concepts.md. Tool listings now includecontentrain_merge."Local-first only / no GitHub API" framing rewritten in
packages/mcp/README.md(×2),docs/packages/mcp.md(×2). New framing: "provider-agnostic, local-first by default, optional GitHub + GitLab backends".New VitePress pages (sidebar + top nav updated):
docs/guides/providers.md— Local / GitHub / GitLab overview + capability matrixdocs/guides/http-transport.md— HTTP deployment, Bearer auth, Studio / CI / remote-agent patternsdocs/reference/providers.md— RepoProvider contract reference with a custom-provider exampleUpdated pages:
getting-started(HTTP tip),concepts(provider + capability concepts),studio(content engine described as MCP over HTTP + RepoProvider),guides/normalize(LocalProvider-required warning),reference/config(widened provider type).CLI:
packages/cli/src/commands/serve.tsdescription updated.packages/cli/README.mdgains a "MCP HTTP (Studio, CI, remote drivers)" subsection and migratescontentrain/*references tocr/*.Rules + skills:
packages/rules/shared/mcp-usage.mdgets a Transport and Provider Capabilities section + capability-matrix table +capability_requirederror entry +contentrain_mergein the catalogue +cr/*branch naming;workflow-rules.mdandnormalize-rules.mdbranch examples migrated. Skills:contentrain-normalize/SKILL.mdadds a Transport Requirements subsection; translate + normalize workflows migrated tocr/*.No tool-surface changes
Same 16 tools, same arg schemas, same JSON response shapes. Stdio + LocalProvider flows behave identically to the previous release.
Changesets
.changeset/types-provider-alignment.md—@contentrain/typesminor.changeset/mcp-phase-10-alignment.md—@contentrain/mcpminorTest plan
pnpm -r typecheck(8 packages) → 0 errorsnpx oxlint(monorepo, 397 files) → 0 warningspnpm --filter @contentrain/mcp build→ clean. Dist containsdist/core/contracts/,dist/providers/local/,dist/providers/github/,dist/providers/gitlab/.npm pack --dry-run→ 130 files, all expected subpaths present.node -e "import('@contentrain/mcp/core/contracts')"andnode -e "import('@contentrain/mcp/providers/local')"both resolve.tests/core tests/conformance tests/serialization-parity tests/git tests/providers tests/server tests/util) → 440/440 green, 2 skipped. Includes 11 new LocalReader tests + 11 new OverlayReader tests + 5 new HTTP E2Es.tests/tools) → 90/91 on parallel; the one flaky case (content.test.ts > blocks new writes when 80 active contentrain branches exist) is a pre-existing parallel-load timeout, not a regression. Re-run in isolation: 17/17 green in 411s. No new tool-surface code is exercised by that test.Scope notes
providers/sharedis intentionally not a public export — it is implementation-detail shared between the two remote providers.content_list --resolvestill needs local disk. The reader path rejects it cleanly rather than silently returning partial results; a future PR can lift this if / when relation resolution gets a reader overload.🤖 Generated with Claude Code