From d59d3b744fb9e7365fc7988624245bad6de5311f Mon Sep 17 00:00:00 2001 From: AHMET BAYHAN BAYRAMOGLU <49499275+ABB65@users.noreply.github.com> Date: Sat, 3 Oct 2026 13:53:57 +0300 Subject: [PATCH 1/2] fix(projects): hold the connect while a Migrate delivery branch is unmerged Connecting forked `contentrain` from the pre-migration default branch. The project then opened without its content, and Migrate's later branch-present check accepted that stale branch. The connect now answers 409 until the migrate/ branch is merged. --- .../content/system/error-messages/en.json | 1 + .../[workspaceId]/projects/index.post.ts | 16 +++++++--- server/utils/ensure-content-branch.ts | 28 +++++++++++++++++ tests/unit/ensure-content-branch.test.ts | 31 ++++++++++++++++++- 4 files changed, 71 insertions(+), 5 deletions(-) diff --git a/.contentrain/content/system/error-messages/en.json b/.contentrain/content/system/error-messages/en.json index b71ebe39..2236d9ff 100644 --- a/.contentrain/content/system/error-messages/en.json +++ b/.contentrain/content/system/error-messages/en.json @@ -291,6 +291,7 @@ "project.config_not_found": "Project configuration file (.contentrain/config.json) not found in the repository.", "project.config_validation_failed": "Project configuration is invalid: {errors}", "project.content_branch_failed": "Could not create the 'contentrain' branch in this repository. Check that the GitHub App still has write access, then try connecting again.", + "project.migration_not_merged": "Contentrain Migrate delivered this site on the '{branch}' branch. Merge it into '{base}' first, then connect the repository — connecting now would open the project without its content.", "project.not_found": "Project not found or you don't have access.", "project.not_found_in_workspace": "Project not found in this workspace. It may have been moved or deleted.", "project.review_workflow_upgrade": "Review workflow is available on {plans:workflow.review}. Enable it in project settings.", diff --git a/server/api/workspaces/[workspaceId]/projects/index.post.ts b/server/api/workspaces/[workspaceId]/projects/index.post.ts index 8f39fdcf..449c21a6 100644 --- a/server/api/workspaces/[workspaceId]/projects/index.post.ts +++ b/server/api/workspaces/[workspaceId]/projects/index.post.ts @@ -1,3 +1,4 @@ +import { unmergedMigrationBranch } from '~~/server/utils/ensure-content-branch' import { syncMigrationHandoff } from '~~/server/utils/migration-handoff' export default defineEventHandler(async (event) => { @@ -46,11 +47,18 @@ export default defineEventHandler(async (event) => { if (installationId) { const [owner = '', repo = ''] = body.repoFullName.split('/') + const git = useGitProvider({ installationId, owner, repo, contentRoot: body.contentRoot || '/' }) + // A Migrate delivery waiting on its own branch: `contentrain` forked from the pre-migration + // default branch would open the project empty (and pass Migrate's "present" check). Wait for the merge. + const waiting = await unmergedMigrationBranch(git, defaultBranch).catch(() => null) + if (waiting) { + throw createError({ + statusCode: 409, + message: errorMessage('project.migration_not_merged', { branch: waiting, base: defaultBranch }), + }) + } try { - await ensureContentBranch( - useGitProvider({ installationId, owner, repo, contentRoot: body.contentRoot || '/' }), - defaultBranch, - ) + await ensureContentBranch(git, defaultBranch) } catch { throw createError({ diff --git a/server/utils/ensure-content-branch.ts b/server/utils/ensure-content-branch.ts index 66807eef..a4d3d827 100644 --- a/server/utils/ensure-content-branch.ts +++ b/server/utils/ensure-content-branch.ts @@ -49,3 +49,31 @@ async function branchExists(git: ContentBranchOps): Promise { const matches = await git.listBranches(CONTENTRAIN_BRANCH) return matches.some(branch => branch.name === CONTENTRAIN_BRANCH) } + +/** The content store file Migrate commits; present on the default branch once the customer has merged the delivery. */ +const CONTENT_STORE_CONFIG = '.contentrain/config.json' + +export interface MigrationMergeOps { + listBranches: (prefix?: string) => Promise<{ name: string }[]> + readFile: (path: string, ref?: string) => Promise +} + +/** + * The Migrate delivery branch (`migrate/`) that has not reached the + * default branch yet, or null. Migrate delivers into a non-empty repository on + * a branch of its own; until the customer merges it the default branch has no + * content store. Creating `contentrain` from that default branch now would + * freeze it at the pre-migration tree: the project would open empty, and + * Migrate's later "branch present" check would take that stale branch for the + * migrated one. So connecting waits for the merge. + * + * Only a repository with no `contentrain` branch yet and a `migrate/` branch is + * ever held: every other repository connects exactly as before. + */ +export async function unmergedMigrationBranch(git: MigrationMergeOps, defaultBranch: string): Promise { + if ((await git.listBranches(CONTENTRAIN_BRANCH)).some(branch => branch.name === CONTENTRAIN_BRANCH)) return null + const delivery = (await git.listBranches('migrate/')).find(branch => branch.name.startsWith('migrate/')) + if (!delivery) return null + const store = await git.readFile(CONTENT_STORE_CONFIG, defaultBranch).catch(() => '') + return store ? null : delivery.name +} diff --git a/tests/unit/ensure-content-branch.test.ts b/tests/unit/ensure-content-branch.test.ts index 051f92c8..dee49751 100644 --- a/tests/unit/ensure-content-branch.test.ts +++ b/tests/unit/ensure-content-branch.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { ensureContentBranch } from '../../server/utils/ensure-content-branch' +import { ensureContentBranch, unmergedMigrationBranch } from '../../server/utils/ensure-content-branch' function ops(overrides: Partial<{ branches: { name: string }[][] @@ -57,3 +57,32 @@ describe('ensureContentBranch', () => { await expect(ensureContentBranch(git, 'main')).rejects.toThrow('Resource not accessible') }) }) + +describe('unmergedMigrationBranch', () => { + const git = (opts: { branches?: string[], store?: boolean }) => ({ + listBranches: vi.fn(async (prefix?: string) => + (opts.branches ?? []).filter(name => !prefix || name.startsWith(prefix)).map(name => ({ name }))), + readFile: vi.fn(async () => { + if (!opts.store) throw new Error('not found') + return '{}' + }), + }) + + it('holds the connect while the delivery branch is unmerged and no content branch exists', async () => { + await expect(unmergedMigrationBranch(git({ branches: ['main', 'migrate/abc123'] }), 'main')).resolves.toBe('migrate/abc123') + }) + + it('lets it through once the default branch carries the content store', async () => { + await expect(unmergedMigrationBranch(git({ branches: ['main', 'migrate/abc123'], store: true }), 'main')).resolves.toBeNull() + }) + + it('does not hold a repository that already has a contentrain branch', async () => { + await expect(unmergedMigrationBranch(git({ branches: ['contentrain', 'migrate/abc123'] }), 'main')).resolves.toBeNull() + }) + + it('does not touch an ordinary repository (no migrate/ branch)', async () => { + const ops = git({ branches: ['main'] }) + await expect(unmergedMigrationBranch(ops, 'main')).resolves.toBeNull() + expect(ops.readFile).not.toHaveBeenCalled() + }) +}) From 79d30bb4755f810b0b59e3f78957c7902ebacbd5 Mon Sep 17 00:00:00 2001 From: AHMET BAYHAN BAYRAMOGLU <49499275+ABB65@users.noreply.github.com> Date: Sat, 3 Oct 2026 14:30:08 +0300 Subject: [PATCH 2/2] fix(projects): hold the connect only for a real Migrate delivery, fail closed A migrate/ branch now holds the connect only when it carries .contentrain/config.json itself, so a team's own migrate/db-v2 branch no longer blocks a first connect. A GitHub error while checking now refuses the connect (502) instead of letting it fork a stale contentrain. --- .../[workspaceId]/projects/index.post.ts | 5 ++- server/utils/ensure-content-branch.ts | 37 ++++++++++++++++--- tests/unit/ensure-content-branch.test.ts | 25 +++++++++---- 3 files changed, 53 insertions(+), 14 deletions(-) diff --git a/server/api/workspaces/[workspaceId]/projects/index.post.ts b/server/api/workspaces/[workspaceId]/projects/index.post.ts index 449c21a6..a1659dc4 100644 --- a/server/api/workspaces/[workspaceId]/projects/index.post.ts +++ b/server/api/workspaces/[workspaceId]/projects/index.post.ts @@ -50,7 +50,10 @@ export default defineEventHandler(async (event) => { const git = useGitProvider({ installationId, owner, repo, contentRoot: body.contentRoot || '/' }) // A Migrate delivery waiting on its own branch: `contentrain` forked from the pre-migration // default branch would open the project empty (and pass Migrate's "present" check). Wait for the merge. - const waiting = await unmergedMigrationBranch(git, defaultBranch).catch(() => null) + // Fail closed: a GitHub error here must not let the connect fork a stale `contentrain`. + const waiting = await unmergedMigrationBranch(git, defaultBranch).catch(() => { + throw createError({ statusCode: 502, message: errorMessage('project.content_branch_failed') }) + }) if (waiting) { throw createError({ statusCode: 409, diff --git a/server/utils/ensure-content-branch.ts b/server/utils/ensure-content-branch.ts index a4d3d827..eb91e680 100644 --- a/server/utils/ensure-content-branch.ts +++ b/server/utils/ensure-content-branch.ts @@ -58,6 +58,14 @@ export interface MigrationMergeOps { readFile: (path: string, ref?: string) => Promise } +/** A read that found nothing — the file or the ref is absent. Anything else (rate limit, network, 5xx) is not an answer. */ +function isMissing(error: unknown): boolean { + const status = (error as { status?: unknown })?.status + if (status === 404) return true + const message = error instanceof Error ? error.message : '' + return /\b404\b|not found/i.test(message) +} + /** * The Migrate delivery branch (`migrate/`) that has not reached the * default branch yet, or null. Migrate delivers into a non-empty repository on @@ -67,13 +75,30 @@ export interface MigrationMergeOps { * Migrate's later "branch present" check would take that stale branch for the * migrated one. So connecting waits for the merge. * - * Only a repository with no `contentrain` branch yet and a `migrate/` branch is - * ever held: every other repository connects exactly as before. + * Only a `migrate/…` branch that itself carries the content store counts: a + * team's own `migrate/db-v2` branch is not a delivery and never holds a connect. + * Only a repository with no `contentrain` branch yet is ever held. + * + * Fails closed: a read that errors for any reason other than "not there" throws, + * so the caller refuses the connect instead of guessing. */ export async function unmergedMigrationBranch(git: MigrationMergeOps, defaultBranch: string): Promise { if ((await git.listBranches(CONTENTRAIN_BRANCH)).some(branch => branch.name === CONTENTRAIN_BRANCH)) return null - const delivery = (await git.listBranches('migrate/')).find(branch => branch.name.startsWith('migrate/')) - if (!delivery) return null - const store = await git.readFile(CONTENT_STORE_CONFIG, defaultBranch).catch(() => '') - return store ? null : delivery.name + const candidates = (await git.listBranches('migrate/')).filter(branch => branch.name.startsWith('migrate/')) + if (!candidates.length) return null + if (await readsStore(git, defaultBranch)) return null + for (const candidate of candidates) { + if (await readsStore(git, candidate.name)) return candidate.name + } + return null +} + +async function readsStore(git: MigrationMergeOps, ref: string): Promise { + try { + return Boolean(await git.readFile(CONTENT_STORE_CONFIG, ref)) + } + catch (error) { + if (isMissing(error)) return false + throw error + } } diff --git a/tests/unit/ensure-content-branch.test.ts b/tests/unit/ensure-content-branch.test.ts index dee49751..482357d8 100644 --- a/tests/unit/ensure-content-branch.test.ts +++ b/tests/unit/ensure-content-branch.test.ts @@ -59,25 +59,31 @@ describe('ensureContentBranch', () => { }) describe('unmergedMigrationBranch', () => { - const git = (opts: { branches?: string[], store?: boolean }) => ({ + /** `stores`: refs whose `.contentrain/config.json` reads; a ref in `broken` fails with a transient error. */ + const git = (opts: { branches?: string[], stores?: string[], broken?: string[] }) => ({ listBranches: vi.fn(async (prefix?: string) => (opts.branches ?? []).filter(name => !prefix || name.startsWith(prefix)).map(name => ({ name }))), - readFile: vi.fn(async () => { - if (!opts.store) throw new Error('not found') + readFile: vi.fn(async (_path: string, ref?: string) => { + if (opts.broken?.includes(ref ?? '')) throw Object.assign(new Error('rate limited'), { status: 403 }) + if (!opts.stores?.includes(ref ?? '')) throw Object.assign(new Error('Not Found'), { status: 404 }) return '{}' }), }) - it('holds the connect while the delivery branch is unmerged and no content branch exists', async () => { - await expect(unmergedMigrationBranch(git({ branches: ['main', 'migrate/abc123'] }), 'main')).resolves.toBe('migrate/abc123') + it('holds the connect while a delivery branch (it carries the store) is unmerged and no content branch exists', async () => { + await expect(unmergedMigrationBranch(git({ branches: ['main', 'migrate/abc123'], stores: ['migrate/abc123'] }), 'main')).resolves.toBe('migrate/abc123') + }) + + it('does not hold on a team\'s own migrate/ branch: it carries no content store', async () => { + await expect(unmergedMigrationBranch(git({ branches: ['main', 'migrate/db-v2'] }), 'main')).resolves.toBeNull() }) it('lets it through once the default branch carries the content store', async () => { - await expect(unmergedMigrationBranch(git({ branches: ['main', 'migrate/abc123'], store: true }), 'main')).resolves.toBeNull() + await expect(unmergedMigrationBranch(git({ branches: ['main', 'migrate/abc123'], stores: ['main', 'migrate/abc123'] }), 'main')).resolves.toBeNull() }) it('does not hold a repository that already has a contentrain branch', async () => { - await expect(unmergedMigrationBranch(git({ branches: ['contentrain', 'migrate/abc123'] }), 'main')).resolves.toBeNull() + await expect(unmergedMigrationBranch(git({ branches: ['contentrain', 'migrate/abc123'], stores: ['migrate/abc123'] }), 'main')).resolves.toBeNull() }) it('does not touch an ordinary repository (no migrate/ branch)', async () => { @@ -85,4 +91,9 @@ describe('unmergedMigrationBranch', () => { await expect(unmergedMigrationBranch(ops, 'main')).resolves.toBeNull() expect(ops.readFile).not.toHaveBeenCalled() }) + + it('fails closed: a transient read error is thrown, not read as "no delivery"', async () => { + await expect(unmergedMigrationBranch(git({ branches: ['main', 'migrate/abc123'], stores: ['migrate/abc123'], broken: ['main'] }), 'main')).rejects.toThrow('rate limited') + await expect(unmergedMigrationBranch(git({ branches: ['main', 'migrate/abc123'], broken: ['migrate/abc123'] }), 'main')).rejects.toThrow('rate limited') + }) })