Skip to content

refactor(mcp): phase 6+7 — explicit capability gates + branch migration extract#33

Merged
ABB65 merged 1 commit into
next-mcpfrom
refactor/phase-6-7-capabilities-and-branch-naming
Apr 17, 2026
Merged

refactor(mcp): phase 6+7 — explicit capability gates + branch migration extract#33
ABB65 merged 1 commit into
next-mcpfrom
refactor/phase-6-7-capabilities-and-branch-naming

Conversation

@ABB65

@ABB65 ABB65 commented Apr 17, 2026

Copy link
Copy Markdown
Member

Summary

Two small polish refactors, bundled because each is too minor to justify its own round trip:

  • Phase 6 — normalize (contentrain_scan, contentrain_apply) now gates on provider.capabilities.astScan / sourceRead / sourceWrite directly instead of the !projectRoot proxy. Behaviour is identical today (LocalProvider is the only provider with those caps enabled), but the gate now matches the semantic contract — any future reader-only provider with sourceRead: true would be rejected by the old proxy despite being capable.
  • Phase 7 — the legacy contentrain/* branch cleanup moved out of ensureContentBranch into providers/local/migration.ts:migrateLegacyBranches. Same behaviour, its own tests, and a clearer home. buildBranchName already emits the cr/* format the Studio contract expects, so nothing else in Phase 7 needed code changes.

Docs: the MCP README grows a short subsection under Normalize spelling out that it requires a LocalProvider and what the capability error looks like on a remote transport.

Test plan

  • pnpm --filter @contentrain/mcp typecheck → 0 errors
  • oxlint packages/mcp/src packages/mcp/tests → 0 warnings (122 files)
  • fast tests (tests/core tests/conformance tests/serialization-parity tests/git tests/providers tests/server tests/util) → 381/381 green, 2 skipped (4 new migrateLegacyBranches tests)
  • tests/tools/normalize.test.ts tests/tools/apply.test.ts → 34/34 green (~390s). Exercises both capability rejections and the LocalProvider happy path — no regressions.

Scope notes

  • Capability audit on read-only tools (content_list, status, describe) was intentionally left out — their labels (localWorktree) are already correct, and converting them to reader-based flows is a separate, larger change.
  • contentrain/* legacy branches are cleaned up automatically on the first ensureContentBranch call in any repo that still has them; no manual migration needed.

🤖 Generated with Claude Code

…on extract

Two small refactors bundled together because each is a minor polish
pass; shipping separately would be more churn than value.

Phase 6 — explicit capability gates on normalize tools:

Before this change, normalize's `!projectRoot` proxy happened to line
up with `!provider.capabilities.astScan / sourceRead / sourceWrite`
because LocalProvider is the only provider with those capabilities
enabled today. Relying on the proxy is fragile — a future provider
with, say, `sourceRead: true` but no projectRoot (a read-only worktree
adapter) would silently fall through the guard.

- tools/normalize.ts
  - `contentrain_scan` now checks `!provider.capabilities.astScan`
    before touching disk. The `!projectRoot` check stays for type
    narrowing, but the capability is the primary gate.
  - `contentrain_apply` checks `!provider.capabilities.sourceRead`
    (extract) or `!provider.capabilities.sourceWrite` (reuse) before
    accepting the call. Same pattern.
  - Both handlers now take a named `provider` parameter; the leading
    underscore is gone.

Phase 7 — branch migration helper extracted:

The legacy `contentrain/*` feature branches conflict with the
singleton `contentrain` ref (git's refs namespace cannot hold both a
leaf ref named `x` and any ref prefixed `x/`). `ensureContentBranch`
was inline-cleaning these before creating the singleton; it now
delegates to a dedicated helper so the logic has its own tests.

- providers/local/migration.ts (new)
  `migrateLegacyBranches(git, baseBranch)` — idempotent two-step
  cleanup: `branch -d` merged legacy branches first, `branch -D`
  whatever is left. Returns the number of branches deleted.
- git/transaction.ts
  `ensureContentBranch` replaces the inline block with a call to
  `migrateLegacyBranches`. Same behaviour, clearer separation.
- tests/providers/local/migration.test.ts (new)
  Four cases: merged cleanup, unmerged force-delete, no-op, idempotency.

Docs:

- packages/mcp/README.md — Normalize section grows a short "Transport /
  provider requirements" subsection. Explicit: normalize needs a
  LocalProvider; remote providers reject with `capability_required`.

Verification:

- pnpm --filter @contentrain/mcp typecheck → 0 errors.
- oxlint packages/mcp/src + tests → 0 warnings (122 files).
- vitest run tests/core tests/conformance tests/serialization-parity
      tests/git tests/providers tests/server tests/util
  → 381/381 green, 2 skipped (4 new migration tests added).
- vitest run tests/tools/normalize.test.ts tests/tools/apply.test.ts
  → 34/34 green (~390s). Both capability paths and LocalProvider
  happy-path exercised; no regressions.

Scope notes — what Phase 6+7 intentionally did not touch:

- Capability-gate audit on read-only tools (content_list, status,
  describe) — their logic still uses local disk primitives
  (detectStack, git branch health) and converting them to reader-
  based is a separate effort. Labels (`localWorktree`) are already
  correct.
- `buildBranchName` already emits `cr/{scope}/{target}[/{locale}]/
  {ts}-{suffix}`. No rename needed — the Studio contract already
  matches. Older `contentrain/*` branches are migrated away by the
  helper above the first time `ensureContentBranch` runs in a repo.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ABB65 ABB65 merged commit 96b0264 into next-mcp Apr 17, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant