feat(mcp): expose repository resources and prompt templates - #2011
feat(mcp): expose repository resources and prompt templates#2011gfargo-horizon-agent[bot] wants to merge 1 commit into
Conversation
Register coco's read-only repository context (status, staged diff, branch context, recent log) as MCP resources under coco://repo/... and its commit/review/changelog/recap prompt templates as MCP prompts, so clients can browse context and reuse coco's prompt library without spending tool calls.
There was a problem hiding this comment.
🔎 Agent review (sonnet→opus) — LGTM
REVIEW: LGTM
RESOLVES: full
A clean, narrowly-scoped implementation of the four read-only resources and five prompts required by OSS-1613 that correctly preserves the no-repo-switch invariant and metadata-only telemetry, and reuses the established getPrompt/mustache rendering path. No blocking or concern-level issues found; the single nit is a coverage gap.
1 nit — 1 inline on the diff
| expect(diff).toContain('+staged content') | ||
| }) | ||
|
|
||
| it('reports an empty staged diff when nothing is staged', async () => { |
There was a problem hiding this comment.
🧹 No test for ahead/behind with a real upstream
getBranchContext's [behind, ahead] = counts.split(...) order is exercised only by the no-upstream fallback; no test configures an upstream with diverging commits to confirm ahead/behind land in the right fields. The logic is correct (--left-right --count upstream...HEAD puts upstream-only commits on the left = behind), and langchain mustache was confirmed not to HTML-escape served content, but this branch is unverified by the suite.
There was a problem hiding this comment.
🔎 Agent review (sonnet→opus) — LGTM
REVIEW: LGTM
RESOLVES: full
The PR implements exactly the work item's ask — four read-only MCP resources and five prompt templates backed by existing read-only git loaders and prompt libraries — preserving root confinement (resolveEffectiveRepoRoot + assertClientAllowsRoot on every resource read) and metadata-only telemetry, with strong test coverage. Only the single pre-existing nit stands.
1 nit — 1 inline on the diff
| expect(diff.trim()).toBe('') | ||
| }) | ||
|
|
||
| it('reports the current branch without an upstream', async () => { |
There was a problem hiding this comment.
🧹 No test for ahead/behind against a real upstream
The added coverage only exercises getBranchContext's no-upstream path; the rev-list ahead/behind counting (context.ts:327-332) is never asserted against a configured upstream. A test that sets a local upstream branch with an asymmetric divergence (e.g. 2 ahead, 1 behind) would confirm both the counts and the left/right destructuring order ([behind, ahead]), which a swap bug would otherwise pass silently. Test-only gap; the feature itself is correct.
There was a problem hiding this comment.
🔎 Agent review (sonnet→opus) — CONCERNS
REVIEW: CONCERNS
RESOLVES: partial
The resources/prompts surface is implemented correctly with root confinement, undefined per-call repo (no URI-based repo switch), SHA-256 provenance, metadata-only mcp telemetry, and non-throwing structured error envelopes reusing existing patterns; the only gap is the previously-flagged missing ahead/behind upstream test, plus a minor error-fidelity concern in getBranchContext's catch.
1 concern · 1 nit — 2 inline on the diff
| expect(diff.trim()).toBe('') | ||
| }) | ||
|
|
||
| it('reports the current branch without an upstream', async () => { |
There was a problem hiding this comment.
Only the no-upstream fallback of getBranchContext is covered; the rev-list --left-right --count path and the [behind, ahead] destructuring order (context.ts:331) are untested. That destructuring order is the exact subtlety reviewers flagged, so a swapped Ahead/Behind regression would ship green. Add a fixture with a configured upstream that diverges by a known delta and assert the Upstream:/Ahead:/Behind: lines.
| )).trim() | ||
| const [behind, ahead] = counts.split(/\s+/) | ||
| lines.push(`Upstream: ${upstream}`, `Ahead: ${ahead}`, `Behind: ${behind}`) | ||
| } catch (error) { |
There was a problem hiding this comment.
🧹 Any git failure labeled 'none configured'
The catch rethrows only CANCELLED and treats every other error as 'Upstream: none configured'. A genuine git failure (corrupted ref, unexpected exit) would be silently mislabeled as no-upstream rather than surfaced. Low impact since this is best-effort browse context and no-upstream is the overwhelmingly common failure, but narrowing the branch-name lookup outside the try or checking the error would improve fidelity.
What
Exposes coco's read-only repository context as MCP resources and its prompt library as MCP prompts, alongside the four existing tools, so MCP clients can browse repo context and reuse coco's templates without spending tool calls.
Why
Plane: OSS-1613
src/mcp/server.tspreviously only registered tools — noregisterResource/registerPrompt. Tools are the imperative surface; resources and prompts are the discoverable ones. This lets any MCP client readcoco://repo/status,coco://repo/diff/staged,coco://repo/branch-context, andcoco://repo/log/recentas resources, and reuse coco's commit/review/changelog/recap prompt templates as MCP prompts.How
src/operations/agent/context.ts: extractdigestOf(text)and add read-only loadersgetRepoStatus,getStagedDiff,getBranchContext,getRecentLog(count=20).src/mcp/server.ts: addregisterRepoResource(registers the 4 resources above) andregisterCocoPrompt/renderPromptTemplate(registers 5 prompts wrapping theCOMMIT/CONVENTIONAL_COMMIT/REVIEW/CHANGELOG/RECAPprompt templates), plus updated serverinstructions.resolveEffectiveRepoRootwithinputRepohard-wired toundefined, so a resource read can never re-point the bound repo.assertClientAllowsRootstill runs on every read, preserving root confinement. Reads reusecreateAgentOperationContext({surface:'mcp'}), so telemetry stays metadata-only (no LLM/usage path is touched).src/mcp/server.test.tsandsrc/operations/agent/context.test.ts.Testing
npx jest src/mcp/server.test.ts src/operations/agent/context.test.ts— 32/32)🤖 Generated by the harbor agent loop. Reviewed by a human before merge.
Relates to #1946