Keep a target-only change with a warning instead of a conflict (1.3.0-beta.1)

README.md

@@ -118,7 +118,7 @@ agility sync [options]
 
 | Option          | Type    | Default      | Description                                                                                                                                                                                                                                           |
 | --------------- | ------- | ------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| `--overwrite`   | boolean | `false`      | Conflict-scoped override. By default, when a target item has its own changes that conflict with the source, the CLI skips it to avoid data loss. With `--overwrite`, those conflicting target items are overwritten with the source version. Non-conflicting updates are applied either way. Also covers a mapped target item that no longer exists (deleted or unpublished on the target): by default that is reported as a conflict and skipped, with `--overwrite` the item is recreated and the mapping repointed. |
+| `--overwrite`   | boolean | `false`      | Conflict-scoped override. By default, when a target item has its own changes that conflict with the source (both sides changed since the last sync), the CLI skips it as a conflict to avoid data loss. An item changed only on the target, with the source unchanged, is kept with a warning instead (listed under "Target changes kept" and as `targetChangeKept` warnings in `--jsonSummary`); it isn't a conflict, and a later source change to it is. This applies to content, pages, models, containers, assets and galleries. With `--overwrite`, both kinds are overwritten with the source version. Non-conflicting updates are applied either way. Also covers a mapped target item that no longer exists (deleted or unpublished on the target): by default that is reported as a conflict and skipped, with `--overwrite` the item is recreated and the mapping repointed. |
 | `--syncDeletes` | boolean | `false`      | After syncing, delete from the target the entities that were deleted in the source since the last sync (URL redirections, pages, content, containers, templates, models, assets, galleries — dependents first). Only entities the CLI previously synced (that have a mapping) are considered; nothing created directly on the target is touched. A target entity edited since the last sync is reported as a conflict and kept unless `--overwrite` is also set. An entity that something kept on the target still uses (a child page, a page or item that links to the content, a container's items, ...) is kept and reported. Content and page deletes are confirmed against the source first, so an item that was only unpublished is never deleted. Cannot be combined with `--pages`, `--containers`, `--models` or `--models-with-deps`; use `--elements` to limit the types. Works with `--preflight`. Not read from `.env`. See [Syncing deletes](#syncing-deletes). |
 | `--allowMassDeletes` | boolean | `false` | With `--syncDeletes`: allow a run to delete more than half (and more than 10) of the mapped entities of one type. Without it such a run deletes nothing and fails, because a stale or partial local cache looks the same as a mass delete. |
 | `--autoPublish` | string  | _(disabled)_ | Automatically publish synced items that were published in the source instance. Values: `content`, `pages`, `both`. If flag is provided without a value, defaults to `both`. Items that are only in staging (not published) in the source are skipped. |

ci/README.md

@@ -59,7 +59,8 @@ every sync it checks:
 - **sync:** exit 0 and no failed items in `--jsonSummary`;
 - **conflict:** the item edited on both sides ends at the synced side's value, or the sync reported a
   conflict;
-- **drift:** an edit made only on the receiving side is overwritten, or reported, never silently kept;
+- **drift:** an edit made only on the receiving side is overwritten, or kept with a "target changed"
+  warning (not a conflict), never silently kept;
 - **resolve:** when the sync kept such edits, the same sync with `--overwrite` settles them, as an
   editor would, before the checks below;
 - **verify:** `agility verify` finds no differences. A verify that finds some is tried once more a

ci/permutation/changes.mjs

@@ -341,12 +341,24 @@ export const CHANGES = {
     const m = added ? models.find((x) => x.id === added.model) : ctx.r.pick(models);
     if (!m) return null;
     const full = await ctx.c.modelMethods.getContentModel(m.id, ctx.guid);
+    // Fields another field names in its settings, like a linked field's companion ID field
+    // (LinkeContentDropdownValueField). Renaming one leaves that setting pointing at a field that no
+    // longer exists, so the server drops every value saved to the link afterwards: a broken model,
+    // not a sync defect.
+    const companions = new Set(
+      full.fields.flatMap((x) => Object.values(x.settings ?? {})).map((v) => String(v).toLowerCase())
+    );
     const f = added
       ? full.fields.find((x) => x.name === added.name)
       : // Not a required field: the server drops existing values on a rename, which would leave every
         // item missing a required value, and the CLI rightly refuses to sync those
         ctx.r.pick(
-          full.fields.filter((x) => x.type === "Text" && String(x.settings?.Required).toLowerCase() !== "true")
+          full.fields.filter(
+            (x) =>
+              x.type === "Text" &&
+              String(x.settings?.Required).toLowerCase() !== "true" &&
+              !companions.has(x.name.toLowerCase())
+          )
         );
     if (!f) return null;
     const old = f.name;
@@ -361,12 +373,13 @@ export async function drift(ctx) {
   return CHANGES["content.edit"]({ ...ctx });
 }
 
-// The same item edited on both sides with different values: a conflict.
-export async function conflict(srcCtx, dstCtx, mappingLookup) {
+// The same item edited on both sides with different values: a conflict. `avoidDstId` is the item
+// the round's drift edited, so the two checks grade different items.
+export async function conflict(srcCtx, dstCtx, mappingLookup, avoidDstId = null) {
   const pool = (await listsWithItems(srcCtx.c, srcCtx.guid, srcCtx.locale)).flatMap((x) => x.items);
   for (const it of pool) {
     const dstId = mappingLookup(it.contentID);
-    if (!dstId) continue;
+    if (!dstId || dstId === avoidDstId) continue;
     const s = await srcCtx.c.contentMethods.getContentItem(it.contentID, srcCtx.guid, srcCtx.locale);
     const f = textField(s);
     if (!f) continue;

ci/permutation/run.mjs

@@ -13,7 +13,7 @@
 // After each sync it checks:
 //   sync        exit 0, no failed items
 //   conflict    the both-sides item ends at the synced side's value, or a preflight reported it
-//   drift       the other side's own edit was overwritten, or reported as a conflict on that item
+//   drift       the other side's own edit was overwritten, or kept with a "target changed" warning
 //   resolve     when the sync kept such edits: the same sync with --overwrite settles them
 //   verify      `agility verify` finds no differences (verify and noop: up to 3 tries, 90s apart)
 //   noop        a preflight in the same direction plans no creates, updates, deletes or conflicts
@@ -121,11 +121,14 @@ function syncStep(round, direction, step = "sync", extra = []) {
   }
   const after = step === "sync" ? cli(`r${round}-after`, [cmd, ...pair, ...PLAN]) : null;
   const entries = [before, after].flatMap((x) => x?.json?.preflight?.entries ?? []);
-  const details = entries.filter((e) => e.action === "conflict").map((e) => e.detail ?? "");
+  const conflicts = entries.filter((e) => e.action === "conflict").map((e) => e.detail ?? "");
+  const kept = entries.filter((e) => e.action === "skip" && /kept the target's version/.test(e.detail ?? ""));
+  const names = (guid, id) => new RegExp(`/instance/${guid}/[^/]+/content/listitem-${id}(?!\\d)`);
   return {
     // Whether a preflight reported a conflict on this content item (its URL is in the detail)
-    reported: (guid, id) =>
-      details.some((d) => new RegExp(`/instance/${guid}/[^/]+/content/listitem-${id}(?!\\d)`).test(d)),
+    reported: (guid, id) => conflicts.some((d) => names(guid, id).test(d)),
+    // Whether a preflight kept this content item's target-only change, with a warning
+    keptWarned: (guid, id) => kept.some((e) => names(guid, id).test(e.detail)),
   };
 }
 
@@ -256,7 +259,8 @@ for (let i = 0; i < plan.length; i++) {
 
   let both = null;
   try {
-    both = await conflict(fromCtx, toCtx, mapped(forward));
+    const driftedId = drifted ? Number(/#(\d+)/.exec(drifted.detail)?.[1]) || null : null;
+    both = await conflict(fromCtx, toCtx, mapped(forward), driftedId);
     if (both) applied.push({ name: "conflict", ...both });
   } catch (e) {
     record(round, direction, "apply:conflict", false, String(e?.message ?? e).slice(0, 200));
@@ -278,7 +282,7 @@ for (let i = 0; i < plan.length; i++) {
     }
   }
 
-  const { reported } = syncStep(round, direction);
+  const { reported, keptWarned } = syncStep(round, direction);
   let kept = false;
 
   if (both) {
@@ -310,12 +314,14 @@ for (let i = 0; i < plan.length; i++) {
       round,
       direction,
       "drift",
-      same || reported(to, id),
+      same || keptWarned(to, id),
       same
         ? "overwritten to match"
-        : reported(to, id)
-          ? "kept, and reported as a conflict"
-          : "the other side's edit was silently kept; the instances differ"
+        : keptWarned(to, id)
+          ? "kept, with a target-changed warning"
+          : reported(to, id)
+            ? "reported as a conflict, but only the target changed"
+            : "the other side's edit was silently kept; the instances differ"
     );
     kept ||= !same;
   }

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@agility/cli",
-  "version": "1.3.0-beta.0",
+  "version": "1.3.0-beta.1",
   "repository": {
     "type": "git",
     "url": "https://github.com/agility/agility-cli.git"

src/core/json-summary.ts

@@ -59,6 +59,13 @@ export interface JsonSummaryDetail {
     field: string;
     kind: "attachment" | "text";
   }>;
+  /**
+   * Additive, on "targetChangeKept" warnings: an entity changed only on the target since the last
+   * sync, which the sync kept. `guid` is the target; `targetUrl` links to it when there is a CMS page.
+   */
+  sourceID?: number;
+  targetID?: number;
+  targetUrl?: string;
 }
 
 /**

src/core/push.ts

@@ -22,6 +22,7 @@ import { preflightReport } from "../lib/preflight/preflight-report";
 import { buildJsonSummary, writeJsonSummary, JsonSummaryDetail, JsonSummaryPhase } from "./json-summary";
 import type { FailureDetail } from "../types/sourceData";
 import { foreignCdnReport } from "../lib/content/foreign-cdn-report";
+import { keptTargetChanges } from "../lib/pushers/kept-target-changes";
 import { resetForeignCdnDetectorCache } from "../lib/pushers/content-pusher/util/report-foreign-cdn-references";
 import type { OtherLocaleStagingEntry } from "./state";
 
@@ -112,6 +113,8 @@ export class Push {
     // PROD-2773: foreign CDN references are reported once per URL per run
     foreignCdnReport.reset();
     resetForeignCdnDetectorCache();
+    // Entities whose target-only change the sync kept, reported once per entity per run
+    keptTargetChanges.reset();
 
     // Initialize logger for push operation
     // Determine if this is a sync operation by checking if both source and target GUIDs exist
@@ -364,6 +367,13 @@ export class Push {
         rest.forEach((line) => console.log(ansiColors.yellow(`  ${line}`)));
       }
 
+      // Target-only changes the sync kept instead of overwriting: non-blocking, --overwrite replaces them
+      if (keptTargetChanges.hasEntries()) {
+        const [headline, ...rest] = keptTargetChanges.renderSummaryLines();
+        console.log(ansiColors.yellow(`\n  ${headline}`));
+        rest.forEach((line) => console.log(ansiColors.yellow(`  ${line}`)));
+      }
+
       // PROD-2311: post-publish mapping/version bookkeeping notices are NOT publish
       // failures — show them under a non-blocking header so they don't read as broken.
       if (mappingWarnings.length > 0) {
@@ -398,6 +408,9 @@ export class Push {
         if (foreignCdnReport.hasEntries()) {
           preflightReport.recordWarning(`${foreignCdnReport.headline()} — see the list above`);
         }
+        if (keptTargetChanges.hasEntries()) {
+          preflightReport.recordWarning(`${keptTargetChanges.headline()} — see the list above`);
+        }
         preflightReport.print();
         if (preflightReport.hasConflicts()) {
           process.exitCode = 1;
@@ -413,7 +426,12 @@ export class Push {
         phases: results.flatMap((r) => r.phases ?? []),
         failures: syncFailureDetails,
         // PROD-2773: one additive entry per foreign CDN URL (code "foreignCdnReference")
-        warnings: [...syncWarningDetails, ...foreignCdnReport.toWarningDetails(sourceGuid)],
+        // One entry per entity whose target-only change was kept (code "targetChangeKept")
+        warnings: [
+          ...syncWarningDetails,
+          ...foreignCdnReport.toWarningDetails(sourceGuid),
+          ...keptTargetChanges.toWarningDetails(targetGuid),
+        ],
         operationErrors: syncErrors,
         autoPublishErrors,
         startedAt,

src/lib/pushers/asset-pusher.ts

@@ -9,6 +9,7 @@ import { fileOperations } from "../../core/fileOperations";
 import path from "path";
 import { GalleryMapper } from "lib/mappers/gallery-mapper";
 import { preflightReport } from "../preflight/preflight-report";
+import { keptTargetChanges } from "./kept-target-changes";
 import { FailureDetail, PusherResult } from "../../types/sourceData";
 
 /**
@@ -262,9 +263,10 @@ export async function pushAssets(
       const hasTargetChanges = existingMapping !== null && referenceMapper.hasTargetChanged(targetAsset);
       const hasSourceChanges = existingMapping !== null && referenceMapper.hasSourceChanged(media);
 
-      // The target changed since the last sync: a conflict whether or not the source changed too, so
-      // a target-only edit is kept (and reported) unless --overwrite says to replace it.
-      const isConflict = existingMapping !== null && hasTargetChanges;
+      // Both sides changed since the last sync: a conflict. Only the target changed: kept, with a
+      // warning (see kept-target-changes.ts). Either is replaced with the source under --overwrite.
+      const isConflict = existingMapping !== null && hasTargetChanges && hasSourceChanges;
+      const isKeptTargetChange = existingMapping !== null && hasTargetChanges && !hasSourceChanges;
       const isOverwrite = state.overwrite;
       const shouldSkip = existingMapping !== null && !hasTargetChanges && !hasSourceChanges;
       // Update if:
@@ -273,7 +275,8 @@ export async function pushAssets(
       //  AND Either IF
       //       We have no conflict
       //       OR we are in overwrite mode (force update even if conflict)
-      const shouldUpdate = existingMapping !== null && !shouldSkip && (!isConflict || isOverwrite);
+      const shouldUpdate =
+        existingMapping !== null && !shouldSkip && ((!isConflict && !isKeptTargetChange) || isOverwrite);
 
       if (shouldCreate) {
         // Asset needs to be created (doesn't exist in target)
@@ -323,18 +326,35 @@ export async function pushAssets(
         logger.asset.skipped(media, "up to date, skipping", targetGuid);
         preflightReport.record({ phase: "Assets", action: "skip", name: media.fileName, detail: "up to date" });
         skipped++;
+      } else if (isKeptTargetChange) {
+        const change = {
+          type: "asset" as const,
+          name: media.fileName,
+          sourceID: media.mediaID,
+          targetID: targetAsset?.mediaID,
+        };
+        keptTargetChanges.record(change);
+        console.warn(ansiColors.yellow(keptTargetChanges.formatLine(change)));
+        logger.asset.skipped(media, "target changed since the last sync, kept", targetGuid);
+        preflightReport.record({
+          phase: "Assets",
+          action: "skip",
+          name: media.fileName,
+          detail: "target changed since the last sync and the source didn't; kept the target's version",
+        });
+        skipped++;
       } else if (isConflict) {
-        // Conflict: the target changed since the last sync and --overwrite was not set.
+        // Conflict: both sides changed since the last sync and --overwrite was not set.
         logger.asset.skipped(
           media,
-          "conflict: target changed since the last sync; use --overwrite to replace it",
+          "conflict: source and target both changed since the last sync; use --overwrite to replace the target",
           targetGuid
         );
         preflightReport.record({
           phase: "Assets",
           action: "conflict",
           name: media.fileName,
-          detail: "target changed; use --overwrite to force",
+          detail: "source and target both changed; use --overwrite to force",
         });
         skipped++;
       }

src/lib/pushers/container-pusher.ts

@@ -6,6 +6,8 @@ import { ModelMapper } from "lib/mappers/model-mapper";
 import { Logs } from "core/logs";
 import { FailureDetail, PusherResult } from "types/sourceData";
 import { preflightReport } from "../preflight/preflight-report";
+import { keptTargetChanges } from "./kept-target-changes";
+import ansiColors from "ansi-colors";
 
 /**
  * Container pusher with enhanced version-based comparison
@@ -172,14 +174,32 @@ export async function pushContainers(
             detail: "up to date",
           });
           skipped++;
+        } else if (hasTargetChanges && !hasSourceChanges && !overwrite) {
+          // Only the target changed: keep its version and warn (see kept-target-changes.ts)
+          const change = {
+            type: "container" as const,
+            name: sourceContainer.referenceName,
+            sourceID: sourceContainer.contentViewID,
+            targetID: targetContainer?.contentViewID,
+          };
+          keptTargetChanges.record(change);
+          console.warn(ansiColors.yellow(keptTargetChanges.formatLine(change)));
+          logger.container.skipped(sourceContainer, "target changed since the last sync, kept", targetGuid);
+          preflightReport.record({
+            phase: "Containers",
+            action: "skip",
+            name: sourceContainer.referenceName,
+            detail: "target changed since the last sync and the source didn't; kept the target's version",
+          });
+          skipped++;
         } else if (hasTargetChanges && !overwrite) {
-          // Container exists and is up to date - skip
+          // Both sides changed since the last sync: a conflict
           logger.container.error(sourceContainer, "Conflict detected, use --overwrite to force changes", targetGuid);
           preflightReport.record({
             phase: "Containers",
             action: "conflict",
             name: sourceContainer.referenceName,
-            detail: "target changed; use --overwrite to force",
+            detail: "source and target both changed; use --overwrite to force",
           });
           skipped++;
         } else if (shouldUpdate && state.preflight) {