From 382a3a927eeea86319fcbd39358b61bb13f6a7f1 Mon Sep 17 00:00:00 2001 From: Contentrain Date: Fri, 17 Apr 2026 23:16:18 +0300 Subject: [PATCH] =?UTF-8?q?feat(cli):=20phase=2014b=20=E2=80=94=20serve=20?= =?UTF-8?q?backend=20meta=20watcher,=20watcher=20errors,=20new=20routes,?= =?UTF-8?q?=20defensive=20Zod?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second pass on `contentrain serve` after Phase 13's auth + drift fixes. Tight, surgical — additive routes and events, no behaviour regressions. ### File watcher coverage - `.contentrain/meta/` now recognised: new `meta:changed` WS event fires with `modelId`, optional `entryId`, `locale`. Covers both per-model SEO layout (`meta//.json`) and per-entry SEO layout (`meta///.json`). Previously editing a meta file left the Serve UI rendering stale metadata until a full refresh. - chokidar `'error'` handler was previously unhandled. Now broadcasts `file-watch:error` with `message` + ISO `timestamp`. The UI can render "watcher down, live updates paused" instead of silently degrading (e.g. OS inotify limit on Linux). ### New HTTP routes - `GET /api/describe-format` — thin wrapper around the `contentrain_describe_format` MCP tool. The Serve UI's model inspector can render the content-format spec as a reference panel. - `GET /api/preview/merge?branch=cr/...` — side-effect-free preview before approve. Returns `alreadyMerged`, `canFastForward`, `conflicts` (best-effort via `git merge-tree`; `null` when unable to run), `filesChanged`, `stat`. Complements the approve route, which continues to surface runtime conflicts by throwing. ### Defensive Zod parity - `/api/normalize/plan/reject` now parses an optional `{ reason? }` body through a new `NormalizePlanRejectBodySchema`. Empty-body and reason-only requests still work (backwards compatible); malformed bodies return structured 400. Whole serve write surface now goes through one `parseOrThrow()` gate. ### Explicitly out of scope - `/api/doctor` — no `contentrain_doctor` MCP tool exists; only the CLI's 540-line command. Rather than duplicate CLI logic into serve, defer until doctor is extracted into a reusable MCP tool. - `sdk:regenerated` WS event — `contentrain generate` runs outside serve's process so the watcher can't observe it. Needs a different hook; defer until the design is concrete. ### Verification - `oxlint` cli src + tests → 0 warnings on 209 files. - `contentrain` typecheck → 0 errors. - `contentrain` vitest → **137/137** (was 130 on next-mcp). 7 new tests cover `meta:changed` (with / without entryId), `file-watch:error` payload, `/api/describe-format` tool call, `/api/preview/merge` validation + happy FF path, plan/reject body validation + backwards compat. No MCP changes. No tool surface changes. Co-Authored-By: Claude Opus 4.7 (1M context) --- ...phase-14b-serve-backend-watchers-routes.md | 81 +++++++++++ packages/cli/src/serve/schemas.ts | 10 ++ packages/cli/src/serve/server.ts | 127 ++++++++++++++++ .../integration/serve.integration.test.ts | 136 +++++++++++++++++- 4 files changed, 352 insertions(+), 2 deletions(-) create mode 100644 .changeset/phase-14b-serve-backend-watchers-routes.md diff --git a/.changeset/phase-14b-serve-backend-watchers-routes.md b/.changeset/phase-14b-serve-backend-watchers-routes.md new file mode 100644 index 0000000..0cbf24f --- /dev/null +++ b/.changeset/phase-14b-serve-backend-watchers-routes.md @@ -0,0 +1,81 @@ +--- +"contentrain": minor +--- + +feat(cli): serve backend — meta watcher, watcher error broadcast, new routes, defensive Zod + +Second pass on `contentrain serve` after Phase 13's auth + drift fixes. +Tight, surgical changes — no behaviour regressions, additive routes +and events the Serve UI can consume immediately. + +### File watcher coverage + +- **`.contentrain/meta/`** — the chokidar handler now recognises + `meta//.json` and `meta///.json` + paths and broadcasts a `meta:changed` WebSocket event with `modelId`, + optional `entryId`, and `locale`. Matches the two real layouts + agents produce (per-model SEO metadata, per-entry SEO metadata). + Without this, editing a `.contentrain/meta/*` file left the Serve + UI rendering stale metadata until a full refresh. +- **Watcher errors surfaced** — `chokidar.on('error', …)` was + previously unhandled. Now broadcasts `file-watch:error` with + `message` + ISO `timestamp`. The UI can render a "watcher down, + live updates paused" banner instead of silently degrading (e.g. + hitting the OS inotify limit on Linux). + +### New HTTP routes + +- **`GET /api/describe-format`** — thin wrapper around the + `contentrain_describe_format` MCP tool. The Serve UI can render + this as a format-reference panel alongside the model inspector + (what the `contentrain describe-format` CLI command shows locally). +- **`GET /api/preview/merge?branch=cr/...`** — preview a merge + before approving it, with zero side effects: + - `alreadyMerged` — the feature branch is already in + `CONTENTRAIN_BRANCH`'s history (approve would be a no-op) + - `canFastForward` — `CONTENTRAIN_BRANCH` is an ancestor of the + feature branch (approve will FF cleanly) + - `conflicts` — best-effort list of conflicting paths from + `git merge-tree`. Empty array on clean merges; `null` when the + check can't run (older git, missing refs). Complements the + approve route, which continues to surface runtime conflicts by + throwing. + - `filesChanged`, `stat` — from the shared `branchDiff()` helper + so UI preview + actual approve see the same file list. + +### Defensive Zod parity + +- **`/api/normalize/plan/reject`** — previously validated nothing; + now parses an optional `{ reason? }` body through a new + `NormalizePlanRejectBodySchema`. Both empty-body and reason-only + requests still work (backwards compatible); malformed bodies + return a structured 400 instead of silently succeeding. Keeps the + entire serve write surface parsing through one `parseOrThrow()` + gate. + +### Explicitly out of scope + +- **`/api/doctor`** — the MCP surface has no `contentrain_doctor` + tool yet; only the CLI's 540-line command. Proper route requires + extracting doctor into a reusable `@contentrain/mcp` tool first, + which is its own phase (14c candidate). Rather than duplicate + CLI logic into serve, we defer. +- **`sdk:regenerated` WS event** — `contentrain generate` runs + outside serve's process, so the watcher can't observe it cleanly. + Needs a different mechanism (e.g. a sentinel file, or integrating + generate into serve's lifecycle). Defer until the design is + concrete. + +### Verification + +- `oxlint` across cli/src + cli/tests → 0 warnings on 209 files. +- `contentrain` typecheck — 0 errors. +- `contentrain` vitest → **137/137** (was 130 on `next-mcp`). The 7 + new tests cover: `meta:changed` with and without `entryId`, + `file-watch:error` payload shape, `/api/describe-format` tool + invocation, `/api/preview/merge` validation + happy path, and + the plan/reject route's body-validation + backwards compat. + +### Tool surface + +No MCP changes — this is pure serve-backend work on existing tools. diff --git a/packages/cli/src/serve/schemas.ts b/packages/cli/src/serve/schemas.ts index c708c4c..cc96936 100644 --- a/packages/cli/src/serve/schemas.ts +++ b/packages/cli/src/serve/schemas.ts @@ -52,6 +52,16 @@ export const NormalizePlanApproveBodySchema = z.object({ models: z.array(z.string()).optional(), }) +/** + * Body for `/api/normalize/plan/reject`. Currently the route only + * deletes the plan file, but we validate the body shape anyway so + * any future caller that wants to record a rejection reason has a + * well-defined contract. Both an empty body and `{ reason? }` pass. + */ +export const NormalizePlanRejectBodySchema = z.object({ + reason: z.string().max(500).optional(), +}).optional() + /** Query params for `/api/normalize/file-context`. */ export const FileContextQuerySchema = z.object({ file: z.string() diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index db71619..b9c92a4 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -29,6 +29,7 @@ import { FileContextQuerySchema, NormalizeApplyBodySchema, NormalizePlanApproveBodySchema, + NormalizePlanRejectBodySchema, parseOrThrow, } from './schemas.js' @@ -186,6 +187,18 @@ export async function createServeApp(options: ServeOptions) { } else if (rel.startsWith('content/')) { const parts = rel.replace('content/', '').split('/') event = { type: 'content:changed', modelId: parts[1], locale: parts[2]?.replace('.json', '') } + } else if (rel.startsWith('meta/') && rel.endsWith('.json')) { + // `.contentrain/meta//.json` and + // `.contentrain/meta///.json` — SEO and + // model-level metadata edited independently of content. The + // Serve UI's model inspector and SEO panels consume this. + const parts = rel.replace('meta/', '').split('/') + const locale = parts[parts.length - 1]?.replace('.json', '') + const modelId = parts[0] + const entryId = parts.length === 3 ? parts[1] : undefined + event = entryId + ? { type: 'meta:changed', modelId, entryId, locale } + : { type: 'meta:changed', modelId, locale } } else { return } @@ -194,6 +207,15 @@ export async function createServeApp(options: ServeOptions) { if (debounceTimer) clearTimeout(debounceTimer) debounceTimer = setTimeout(flush, 300) }) + + // Surface watcher failures instead of silently degrading to a + // no-op. Without this the UI keeps rendering stale data after, + // e.g., hitting the OS inotify limit — the user has no way to + // know live updates stopped flowing. + watcher.on('error', (err) => { + const message = err instanceof Error ? err.message : String(err) + broadcast({ type: 'file-watch:error', message, timestamp: new Date().toISOString() }) + }) } // --- API Routes --- @@ -249,6 +271,14 @@ export async function createServeApp(options: ServeOptions) { return await callTool('contentrain_describe', { model: modelId, include_sample: true }) })) + // Format reference — thin wrapper around contentrain_describe_format. + // Serves a static spec of how Contentrain stores models, content, + // meta, and vocabulary. The UI renders this as a reference panel for + // humans who want to learn the file layout without reading docs. + router.add('/api/describe-format', defineEventHandler(async () => { + return await callTool('contentrain_describe_format') + })) + // Content list router.add('/api/content/:modelId', defineEventHandler(async (event) => { const modelId = getRouterParam(event, 'modelId') @@ -344,6 +374,96 @@ export async function createServeApp(options: ServeOptions) { } })) + // Merge preview — answers "if I approve this branch right now, what + // would happen?" without running the merge. Three signals: + // - `alreadyMerged` — the feature branch is already in + // CONTENTRAIN_BRANCH's history (approve would be a no-op) + // - `canFastForward` — CONTENTRAIN_BRANCH is an ancestor of the + // feature branch (approve will fast-forward cleanly) + // - `conflicts` — best-effort list of conflicting paths detected + // via `git merge-tree`. Empty array when the check succeeds + // with no conflicts; `null` when the check couldn't run (older + // git, detached refs, etc.). + // + // Intentionally does NOT run the real merge — the approve route + // already surfaces runtime conflicts by throwing. This is a fast, + // side-effect-free signal for the UI to render a "preview" banner + // before the user commits to approve. + router.add('/api/preview/merge', defineEventHandler(async (event) => { + const query = getQuery(event) + const branchName = query['branch'] as string | undefined + if (!branchName || !branchName.startsWith('cr/')) { + throw createError({ statusCode: 400, message: 'Missing or invalid `branch` query (must start with "cr/")' }) + } + + const git = simpleGit(projectRoot) + + // Confirm the branch exists locally. + const local = await git.branchLocal() + if (!local.all.includes(branchName)) { + throw createError({ statusCode: 404, message: `Branch "${branchName}" does not exist locally` }) + } + + // Fast-forward / already-merged checks. + let alreadyMerged = false + let canFastForward = false + try { + await git.raw(['merge-base', '--is-ancestor', branchName, CONTENTRAIN_BRANCH]) + alreadyMerged = true + } catch { /* not merged yet */ } + if (!alreadyMerged) { + try { + await git.raw(['merge-base', '--is-ancestor', CONTENTRAIN_BRANCH, branchName]) + canFastForward = true + } catch { /* would require a 3-way merge */ } + } + + // Best-effort conflict scan via `git merge-tree`. The legacy 3-way + // form (`merge-tree `) is widely supported + // and prints conflict sections on stdout; silence = clean. + let conflicts: string[] | null = null + try { + const mergeBase = (await git.raw(['merge-base', CONTENTRAIN_BRANCH, branchName])).trim() + if (mergeBase) { + const out = await git.raw(['merge-tree', mergeBase, CONTENTRAIN_BRANCH, branchName]) + // Conflict sections look like `changed in both\n base 100644 ...` + // — extract unique paths from the `<<<<<<<` / `>>>>>>>` surrounds. + const paths = new Set() + const conflictBlockRegex = /^(?:changed in both|added in both|removed in (?:local|remote))\b[^\n]*\n((?:[ \t][^\n]*\n)+)/gmu + let match: RegExpExecArray | null + while ((match = conflictBlockRegex.exec(out)) !== null) { + const block = match[1] ?? '' + const pathMatch = /\b100644\s+[0-9a-f]{4,}\s+(\S+)/u.exec(block) + if (pathMatch?.[1]) paths.add(pathMatch[1]) + } + conflicts = [...paths] + } + } catch { /* merge-tree unavailable or failed — leave as null */ } + + // Diff summary against CONTENTRAIN_BRANCH (the same base the + // actual merge will use). Skip when already merged — the diff is + // empty and the MCP helper would error on a zero-commit range. + let stat = '' + let filesChanged = 0 + if (!alreadyMerged) { + try { + const diff = await branchDiff(projectRoot, { branch: branchName }) + stat = diff.stat + filesChanged = diff.filesChanged + } catch { /* leave zeroed */ } + } + + return { + branch: branchName, + base: CONTENTRAIN_BRANCH, + alreadyMerged, + canFastForward, + conflicts, + filesChanged, + stat, + } + })) + // Branch approve — delegates to MCP's mergeBranch helper, which // runs the worktree transaction + selective sync with dirty-file // protection. `sync.skipped[]` surfaces files that the developer @@ -564,6 +684,13 @@ export async function createServeApp(options: ServeOptions) { if (event.method !== 'DELETE' && event.method !== 'POST') { throw createError({ statusCode: 405, message: 'Method not allowed' }) } + // Validate the (optional) body so future callers that want to + // record a rejection reason have a well-defined contract, and so + // every write route here parses through the same Zod gate. + if (event.method === 'POST') { + const raw = await readBody(event).catch(() => undefined) + if (raw !== undefined) parseOrThrow(NormalizePlanRejectBodySchema, raw) + } const planPath = join(crDir, 'normalize-plan.json') if (existsSync(planPath)) { await unlink(planPath) diff --git a/packages/cli/tests/integration/serve.integration.test.ts b/packages/cli/tests/integration/serve.integration.test.ts index 7407f28..0e56098 100644 --- a/packages/cli/tests/integration/serve.integration.test.ts +++ b/packages/cli/tests/integration/serve.integration.test.ts @@ -18,6 +18,7 @@ const mergeMock = vi.fn().mockResolvedValue(undefined) let connectionHandler: ((ws: FakeWs, req: unknown) => void) | null = null let watchAllHandler: ((eventType: string, filePath: string) => void) | null = null +let watchErrorHandler: ((err: unknown) => void) | null = null let lastWs: FakeWs | null = null class FakeWs { @@ -60,8 +61,9 @@ vi.mock('ws', () => ({ vi.mock('chokidar', () => ({ watch: vi.fn(() => { const watcher = { - on: vi.fn((event: string, cb: (eventType: string, filePath: string) => void) => { - if (event === 'all') watchAllHandler = cb + on: vi.fn((event: string, cb: (...args: unknown[]) => void) => { + if (event === 'all') watchAllHandler = cb as (eventType: string, filePath: string) => void + if (event === 'error') watchErrorHandler = cb as (err: unknown) => void return watcher }), } @@ -141,6 +143,7 @@ beforeEach(async () => { routes.clear() connectionHandler = null watchAllHandler = null + watchErrorHandler = null lastWs = null testDir = await mkdtemp(join(tmpdir(), 'cr-cli-serve-')) @@ -424,4 +427,133 @@ describe('serve server contract', { sequential: true }, () => { }, })) }) + + // ─── Phase 14b — meta watcher, error broadcast, new routes ─── + + it('broadcasts meta:changed when .contentrain/meta//.json updates', async () => { + const { handleUpgrade } = await boot() + handleUpgrade({ url: '/ws' } as never, {} as never, Buffer.alloc(0)) + + watchAllHandler?.('change', join(testDir, '.contentrain', 'meta', 'hero', 'en.json')) + await vi.advanceTimersByTimeAsync(301) + + expect(lastWs?.send).toHaveBeenLastCalledWith(JSON.stringify({ + type: 'meta:changed', + modelId: 'hero', + locale: 'en', + })) + }) + + it('broadcasts meta:changed with entryId when the path is //.json', async () => { + const { handleUpgrade } = await boot() + handleUpgrade({ url: '/ws' } as never, {} as never, Buffer.alloc(0)) + + watchAllHandler?.('change', join(testDir, '.contentrain', 'meta', 'docs-packages', 'mcp', 'en.json')) + await vi.advanceTimersByTimeAsync(301) + + expect(lastWs?.send).toHaveBeenLastCalledWith(JSON.stringify({ + type: 'meta:changed', + modelId: 'docs-packages', + entryId: 'mcp', + locale: 'en', + })) + }) + + it('broadcasts file-watch:error when chokidar emits an error', async () => { + const { handleUpgrade } = await boot() + handleUpgrade({ url: '/ws' } as never, {} as never, Buffer.alloc(0)) + + watchErrorHandler?.(new Error('ENOSPC: too many watchers')) + + const messages = lastWs?.send.mock.calls.map((c: unknown[]) => JSON.parse(c[0] as string)) ?? [] + const err = messages.find(m => m.type === 'file-watch:error') + expect(err).toBeDefined() + expect(err.message).toContain('ENOSPC') + expect(typeof err.timestamp).toBe('string') + }) + + it('exposes /api/describe-format that invokes the contentrain_describe_format tool', async () => { + await boot() + + const handler = routes.get('/api/describe-format') + expect(handler).toBeDefined() + await handler?.() + + expect(callToolMock).toHaveBeenCalledWith({ + name: 'contentrain_describe_format', + arguments: {}, + }) + }) + + it('exposes /api/preview/merge and rejects requests for non-cr branches', async () => { + await boot() + + const handler = routes.get('/api/preview/merge') + expect(handler).toBeDefined() + await expect(handler?.({ query: { branch: 'feature/redesign' } })).rejects.toMatchObject({ statusCode: 400 }) + await expect(handler?.({ query: {} })).rejects.toMatchObject({ statusCode: 400 }) + }) + + it('returns a clean FF-possible preview when CONTENTRAIN_BRANCH is an ancestor', async () => { + // Preview contract: the first `merge-base --is-ancestor` call + // (branch ancestor-of CONTENTRAIN_BRANCH) rejects — branch is NOT + // already merged. The second (CONTENTRAIN_BRANCH ancestor-of + // branch) resolves — FF is possible. `merge-base` returns a sha. + // `merge-tree` returns empty — no conflicts. branchDiff gives + // stat + filesChanged. + rawMock.mockImplementation(async (args: string[]) => { + if (args[0] === 'merge-base' && args[1] === '--is-ancestor') { + const from = args[2] + if (from === 'cr/review/hero/en/123') throw new Error('not ancestor') + return '' + } + if (args[0] === 'merge-base') return 'deadbeef1234\n' + if (args[0] === 'merge-tree') return '' + return '' + }) + branchDiffMock.mockResolvedValueOnce({ + branch: 'cr/review/hero/en/123', + base: 'contentrain', + stat: ' .contentrain/content/hero/en.json | 2 +-', + patch: '', + filesChanged: 1, + }) + + await boot() + const handler = routes.get('/api/preview/merge') + const response = await handler?.({ query: { branch: 'cr/review/hero/en/123' } }) as Record + + expect(response.alreadyMerged).toBe(false) + expect(response.canFastForward).toBe(true) + expect(response.conflicts).toEqual([]) + expect(response.filesChanged).toBe(1) + }) + + it('plan/reject accepts an optional reason body without breaking the delete flow', async () => { + const { handleUpgrade } = await boot() + handleUpgrade({ url: '/ws' } as never, {} as never, Buffer.alloc(0)) + + const handler = routes.get('/api/normalize/plan/reject') + expect(handler).toBeDefined() + + // With body: parses via Zod schema. + await writeFile(join(testDir, '.contentrain', 'normalize-plan.json'), JSON.stringify({ status: 'pending' })) + await expect(handler?.({ + method: 'POST', + body: { reason: 'Too many false positives' }, + })).resolves.toMatchObject({ status: 'rejected' }) + + // Without body: still works (backwards compat). + await writeFile(join(testDir, '.contentrain', 'normalize-plan.json'), JSON.stringify({ status: 'pending' })) + await expect(handler?.({ + method: 'POST', + })).resolves.toMatchObject({ status: 'rejected' }) + + // Malformed body: rejected with 400. + await writeFile(join(testDir, '.contentrain', 'normalize-plan.json'), JSON.stringify({ status: 'pending' })) + await expect(handler?.({ + method: 'POST', + body: { reason: 12345 }, + })).rejects.toMatchObject({ statusCode: 400 }) + }) })