diff --git a/docs/user-guide/branch-commands.md b/docs/user-guide/branch-commands.md index f9fd5bc3..33907a7d 100644 --- a/docs/user-guide/branch-commands.md +++ b/docs/user-guide/branch-commands.md @@ -30,6 +30,8 @@ content-cli config branch create \ `--validate` runs the server-side validator without persisting and prints a success message instead of the created branch. +`--sourceVersion` must be an existing version of the source package. Unlike `merge preview` and `merge apply`, `create` does not accept `LATEST`; use `config versions get --packageKey --packageVersion LATEST` to resolve the newest version first. + ## List Branches ```bash @@ -68,6 +70,8 @@ content-cli config branch merge preview \ The JSON preview contains the conflict layout (`changes.configuration.conflicts`, `changes.metadata.conflicts`) and auto-merge results. +`merge apply` always needs a merge source: either pass `--sourceKey` together with `--sourceVersion`, or point `-f` at a file that carries both. Passing neither, or `--sourceKey` on its own, is rejected. + ### Quick merge When you do not need to override any node-level resolutions, the resolutions file is optional: @@ -83,7 +87,7 @@ The CLI posts the merge directly. If the server detects conflicts it returns an Customise the published version without writing a file: -- `--bump PATCH|MINOR|MAJOR` — pick the bump option (default `PATCH`). +- `--bump PATCH` — bump the patch segment. This is also the default when neither `--bump` nor `--newVersion` is given, so you rarely need it. Minor and major bumps are not supported; pin the version with `--newVersion` instead. - `--newVersion 1.5.0` — pin an explicit semver. - `--summary ""` — summary of changes (default `"Merge @"`). @@ -119,7 +123,7 @@ A valid merge body looks like: `versionCreate` is filled in this order of precedence (highest wins): -1. `--newVersion ` flag or `--bump PATCH|MINOR|MAJOR` flag (and `--summary`). +1. `--newVersion ` flag or `--bump PATCH` flag (and `--summary`). 2. Whatever the resolutions file already has under `versionCreate`. 3. Default: `versionBumpOption: PATCH` + `summaryOfChanges: "Merge @"`. @@ -127,14 +131,14 @@ A valid merge body looks like: > **Custom changes are restricted to paths the preview already touched.** Each op in `customConfigurationChanges` / `customMetadataChanges` must reference a `path` that appears in either the preview's `sourceChanges` or `targetChanges` for that node — you cannot introduce edits at paths neither side modified. The server rejects ops at unrelated paths. -Worked example: the preview reports that for node `node-1`, the source set `/title` to `"Source title"` and the target set `/title` to `"Target title"` (a conflict at `/title`). A valid CUSTOM resolution can pick a third value for `/title`, but it cannot also touch `/description` (which neither side changed): +Worked example: the preview reports that for node `node-1`, the source set `/title` to `"Source title"` and the target set `/title` to `"Target title"` (a conflict at `/title`). A valid CUSTOM resolution picks either the source's or the target's value for `/title` — not a third one — and it cannot also touch `/description` (which neither side changed): ```json { "nodeKey": "node-1", "resolution": "CUSTOM", "customConfigurationChanges": [ - { "op": "replace", "path": "/title", "value": "Negotiated title" } + { "op": "replace", "path": "/title", "value": "Source title" } ] } ``` diff --git a/src/commands/configuration-management/branch/branch.command.service.ts b/src/commands/configuration-management/branch/branch.command.service.ts index f0a87a8b..2a11aa55 100644 --- a/src/commands/configuration-management/branch/branch.command.service.ts +++ b/src/commands/configuration-management/branch/branch.command.service.ts @@ -9,8 +9,10 @@ import { CreateBranchTransport, MergeApplyOptions, MergeBranchTransport, + MergeDiffTransport, MergePreviewRequestTransport, MergePreviewTransport, + MergeStatus, PackageVersionCreatedTransport, SavePackageVersionTransport, VersionBumpOption, @@ -169,15 +171,27 @@ export class BranchCommandService { logger.info(`Preview: merge ${sourceKey}@${sourceVersion} into ${targetPackageKey}`); logger.info(`Common ancestor: ${preview.commonPackageKey}@${preview.commonVersion}`); const totalNodes = preview.nodeChanges?.length ?? 0; - const packageConflicts = - (preview.packageChanges?.changes?.configuration?.conflicts?.length ?? 0) + - (preview.packageChanges?.changes?.metadata?.conflicts?.length ?? 0); - const nodeConflicts = preview.nodeChanges?.reduce((sum, node) => { - const config = node.changes?.configuration?.conflicts?.length ?? 0; - const metadata = node.changes?.metadata?.conflicts?.length ?? 0; - return sum + config + metadata; - }, 0) ?? 0; - const conflicts = packageConflicts + nodeConflicts; - logger.info(`Nodes touched: ${totalNodes}, conflicting paths: ${conflicts}.`); + const packageConflictPaths = BranchCommandService.countConflictPaths(preview.packageChanges?.changes); + const nodeConflictPaths = + preview.nodeChanges?.reduce((sum, node) => sum + BranchCommandService.countConflictPaths(node.changes), 0) ?? 0; + const conflicts = packageConflictPaths + nodeConflictPaths; + const nodesNeedingResolution = + preview.nodeChanges?.filter(node => BranchCommandService.isInConflict(node.changes)).length ?? 0; + + logger.info(`Nodes touched: ${totalNodes}, conflicting paths: ${conflicts}, nodes needing resolution: ${nodesNeedingResolution}.`); + if (BranchCommandService.isInConflict(preview.packageChanges?.changes)) { + logger.info("The package's own configuration or metadata is in conflict. Supply 'resolvedPackageConflict' before applying."); + } + if (nodesNeedingResolution > 0 && nodeConflictPaths === 0) { + logger.info("The conflicts above are structural (a node exists on one side only), so no path-level diff is reported. Supply a resolution for each node before applying."); + } + } + + private static countConflictPaths(changes: MergeDiffTransport | undefined): number { + return (changes?.configuration?.conflicts?.length ?? 0) + (changes?.metadata?.conflicts?.length ?? 0); + } + + private static isInConflict(changes: MergeDiffTransport | undefined): boolean { + return changes?.configuration?.status === MergeStatus.CONFLICT || changes?.metadata?.status === MergeStatus.CONFLICT; } } diff --git a/src/commands/configuration-management/branch/interfaces/branch.interfaces.ts b/src/commands/configuration-management/branch/interfaces/branch.interfaces.ts index fc24f238..165c6c16 100644 --- a/src/commands/configuration-management/branch/interfaces/branch.interfaces.ts +++ b/src/commands/configuration-management/branch/interfaces/branch.interfaces.ts @@ -24,8 +24,6 @@ export enum ChangeType { export enum VersionBumpOption { NONE = "NONE", PATCH = "PATCH", - MINOR = "MINOR", - MAJOR = "MAJOR", } export interface BranchingSettingsTransport { diff --git a/src/commands/configuration-management/module.ts b/src/commands/configuration-management/module.ts index 4e7c66e5..f332f463 100644 --- a/src/commands/configuration-management/module.ts +++ b/src/commands/configuration-management/module.ts @@ -44,7 +44,7 @@ class Module extends IModule { .description("Create a new branch from a source version") .requiredOption("--packageKey ", "Main package key to branch from") .requiredOption("--branchKey ", "New branch key") - .requiredOption("--sourceVersion ", "Source version to branch from") + .requiredOption("--sourceVersion ", "Source version to branch from. Must be an existing version; 'LATEST' is not supported here") .option("--validate", "Only validate the request without creating the branch", false) .option("--json", "Write response to a JSON file", false) .action(this.createBranch); @@ -78,7 +78,7 @@ class Module extends IModule { .option("-f, --file ", "Path to a JSON file containing the MergeBranchTransport payload") .option("--sourceKey ", "Source package key, main package key or '@' (overrides the file's 'sourceKey')") .option("--sourceVersion ", "Source version (overrides the file's 'sourceVersion'; or 'LATEST')") - .option("--bump ", "Version bump for the new published version: PATCH | MINOR | MAJOR") + .option("--bump ", "Version bump for the new published version: PATCH (the default when neither --bump nor --newVersion is given)") .option("--newVersion ", "Pin the new published version to an explicit semver") .option("--summary ", "Summary of changes for the new published version") .option("--json", "Write response to a JSON file", false) diff --git a/tests/commands/configuration-management/branch/branch-merge.spec.ts b/tests/commands/configuration-management/branch/branch-merge.spec.ts index bd2616b9..773ff1ca 100644 --- a/tests/commands/configuration-management/branch/branch-merge.spec.ts +++ b/tests/commands/configuration-management/branch/branch-merge.spec.ts @@ -99,6 +99,104 @@ describe("branch merge preview", () => { expect(getJsonFromDownloadedFile()).toEqual(preview); }); + + it("reports a structural conflict that has no conflicting paths", async () => { + const structural: MergePreviewTransport = { + ...preview, + nodeChanges: [ + { + ...preview.nodeChanges[0], + nodeKey: "node-2", + sourceChange: ChangeType.DELETED, + targetChange: ChangeType.UNCHANGED, + changes: { + configuration: { status: MergeStatus.CONFLICT, sourceChanges: [], targetChanges: [], conflicts: [], autoMergedChanges: [] }, + metadata: { status: MergeStatus.CONFLICT, sourceChanges: [], targetChanges: [], conflicts: [], autoMergedChanges: [] }, + }, + }, + ], + }; + mockAxiosPost(previewUrl, structural); + + await new BranchCommandService(testContext).mergePreview(targetPackageKey, `${targetPackageKey}@feature-a`, "1.4.0", false); + + const messages = loggingTestTransport.logMessages.map(m => m.message); + expect(messages.some(m => m.includes("conflicting paths: 0, nodes needing resolution: 1"))).toBe(true); + expect(messages.some(m => m.includes("The conflicts above are structural"))).toBe(true); + }); + + it("counts a node once even when both configuration and metadata conflict", async () => { + mockAxiosPost(previewUrl, preview); + + await new BranchCommandService(testContext).mergePreview(targetPackageKey, `${targetPackageKey}@feature-a`, "1.4.0", false); + + const messages = loggingTestTransport.logMessages.map(m => m.message); + expect(messages.some(m => m.includes("conflicting paths: 1, nodes needing resolution: 1"))).toBe(true); + expect(messages.some(m => m.includes("The conflicts above are structural"))).toBe(false); + expect(messages.some(m => m.includes("resolvedPackageConflict"))).toBe(false); + }); + + it("reports a package-level conflict separately from the node count", async () => { + const packageConflict: MergePreviewTransport = { + ...preview, + packageChanges: { + ...preview.packageChanges, + changes: { + configuration: { + status: MergeStatus.CONFLICT, + sourceChanges: [], + targetChanges: [], + conflicts: [ + { path: "/name", sourceChange: { op: "replace", path: "/name", value: "A" }, targetChange: { op: "replace", path: "/name", value: "B" } }, + ], + autoMergedChanges: [], + }, + metadata: { status: MergeStatus.UNCHANGED, sourceChanges: [], targetChanges: [], conflicts: [], autoMergedChanges: [] }, + }, + }, + nodeChanges: [], + }; + mockAxiosPost(previewUrl, packageConflict); + + await new BranchCommandService(testContext).mergePreview(targetPackageKey, `${targetPackageKey}@feature-a`, "1.4.0", false); + + const messages = loggingTestTransport.logMessages.map(m => m.message); + expect(messages.some(m => m.includes("Nodes touched: 0, conflicting paths: 1, nodes needing resolution: 0"))).toBe(true); + expect(messages.some(m => m.includes("resolvedPackageConflict"))).toBe(true); + expect(messages.some(m => m.includes("The conflicts above are structural"))).toBe(false); + }); + + it("does not advise a per-node resolution when only the package conflicts and no paths are reported", async () => { + const structuralPackage: MergePreviewTransport = { + ...preview, + packageChanges: { + ...preview.packageChanges, + changes: { + configuration: { status: MergeStatus.CONFLICT, sourceChanges: [], targetChanges: [], conflicts: [], autoMergedChanges: [] }, + metadata: { status: MergeStatus.CONFLICT, sourceChanges: [], targetChanges: [], conflicts: [], autoMergedChanges: [] }, + }, + }, + nodeChanges: [], + }; + mockAxiosPost(previewUrl, structuralPackage); + + await new BranchCommandService(testContext).mergePreview(targetPackageKey, `${targetPackageKey}@feature-a`, "1.4.0", false); + + const messages = loggingTestTransport.logMessages.map(m => m.message); + expect(messages.some(m => m.includes("Nodes touched: 0, conflicting paths: 0, nodes needing resolution: 0"))).toBe(true); + expect(messages.some(m => m.includes("resolvedPackageConflict"))).toBe(true); + expect(messages.some(m => m.includes("The conflicts above are structural"))).toBe(false); + }); + + it("summarises a response that omits packageChanges and nodeChanges", async () => { + mockAxiosPost(previewUrl, { commonPackageKey: targetPackageKey, commonVersion: "1.2.0" } as MergePreviewTransport); + + await new BranchCommandService(testContext).mergePreview(targetPackageKey, `${targetPackageKey}@feature-a`, "1.4.0", false); + + const messages = loggingTestTransport.logMessages.map(m => m.message); + expect(messages.some(m => m.includes("Nodes touched: 0, conflicting paths: 0, nodes needing resolution: 0"))).toBe(true); + expect(messages.some(m => m.includes("resolvedPackageConflict"))).toBe(false); + }); }); describe("branch merge apply", () => { @@ -120,7 +218,7 @@ describe("branch merge apply", () => { resolvedNodeConflicts: [ { nodeKey: "node-1", resolution: MergeResolution.ACCEPT_SOURCE }, ], - versionCreate: { versionBumpOption: VersionBumpOption.MAJOR, summaryOfChanges: "merge from feature-a" }, + versionCreate: { version: "9.9.9", summaryOfChanges: "merge from feature-a" }, }; beforeEach(() => { @@ -156,16 +254,16 @@ describe("branch merge apply", () => { expect(sent.sourceVersion).toBe("LATEST"); }); - it("flag --bump overrides versionCreate.versionBumpOption from the file", async () => { + it("flag --bump overrides versionCreate from the file and accepts lowercase", async () => { mockAxiosPost(mergeUrl, created); await new BranchCommandService(testContext).mergeApply(targetPackageKey, { file: "merge-request.json", - bump: "minor", + bump: "patch", }); const sent: MergeBranchTransport = JSON.parse(mockedPostRequestBodyByUrl.get(mergeUrl) as string); - expect(sent.versionCreate.versionBumpOption).toBe(VersionBumpOption.MINOR); + expect(sent.versionCreate.versionBumpOption).toBe(VersionBumpOption.PATCH); expect(sent.versionCreate.version).toBeUndefined(); }); @@ -219,13 +317,13 @@ describe("branch merge apply", () => { await new BranchCommandService(testContext).mergeApply(targetPackageKey, { sourceKey: `${targetPackageKey}@feature-a`, sourceVersion: "1.4.0", - bump: "MAJOR", + bump: "PATCH", summary: "ship it", }); const sent: MergeBranchTransport = JSON.parse(mockedPostRequestBodyByUrl.get(mergeUrl) as string); expect(sent.versionCreate).toEqual({ - versionBumpOption: VersionBumpOption.MAJOR, + versionBumpOption: VersionBumpOption.PATCH, summaryOfChanges: "ship it", }); });