fix(content): correctness fixes for push, merge, and delete

src/content/da-api.js

@@ -162,7 +162,8 @@ export class DaClient {
   }
 
   /**
-   * Deletes a file or folder. Idempotent.
+   * Deletes a file or folder. Idempotent: 404 is treated as success.
+   * Throws on transport or server errors so callers don't silently treat them as success.
    */
   async deleteSource(org, repo, daPath) {
     const url = `${DA_ADMIN}/source/${org}/${repo}${daPath}`;
@@ -173,7 +174,10 @@ export class DaClient {
     if (res.status === 401) {
       throw new Error('Unauthorized: invalid or missing token');
     }
-    return res.ok || res.status === 204;
+    if (res.ok || res.status === 204 || res.status === 404) {
+      return true;
+    }
+    throw new Error(`DELETE failed for ${daPath}: ${res.status} ${res.statusText}`);
   }
 
   /**

src/content/merge.cmd.js

@@ -96,7 +96,7 @@ export default class MergeCommand {
       return;
     }
 
-    const token = await getValidToken(log, this._token, contentDir);
+    const token = await getValidToken(log, this._token, this._dir);
     const client = new DaClient(token);
 
     // Get the HEAD commit to read base blobs

src/content/push.cmd.js

@@ -108,24 +108,24 @@ export default class PushCommand {
    * @param {string} repo
    * @param {string} contentDir
    * @param {string[]} targets
-   * @returns {Promise<{ pushed: number, errors: number, successfullyPushed: Set<string> }>}
+   * @returns {Promise<{ pushed: number, errors: number }>}
    */
   async _uploadFiles(client, org, repo, contentDir, targets) {
     const { log } = this;
     let pushed = 0;
     let errors = 0;
-    const successfullyPushed = new Set();
 
+    // processQueue consumes its input array via shift(); copy so the caller's list survives.
     const results = await processQueue(
-      targets,
+      [...targets],
       async (daPath) => {
         const localPath = path.join(contentDir, ...daPath.split('/').filter(Boolean));
         const ext = daPath.split('.').pop();
         try {
           const buffer = await fse.readFile(localPath);
           await client.putSource(org, repo, daPath, buffer, getContentType(ext));
           log.info(`  ✓ ${daPath}`);
-          return { ok: true, daPath };
+          return { ok: true };
         } catch (err) {
           log.warn(`  ✗ ${daPath}: ${err.message}`);
           return { ok: false };
@@ -136,13 +136,12 @@ export default class PushCommand {
     for (const r of results) {
       if (r.ok) {
         pushed += 1;
-        successfullyPushed.add(r.daPath);
       } else {
         errors += 1;
       }
     }
 
-    return { pushed, errors, successfullyPushed };
+    return { pushed, errors };
   }
 
   /**
@@ -151,21 +150,21 @@ export default class PushCommand {
    * @param {string} org
    * @param {string} repo
    * @param {string[]} deleted
-   * @returns {Promise<{ pushed: number, errors: number, successfullyDeleted: Set<string> }>}
+   * @returns {Promise<{ pushed: number, errors: number }>}
    */
   async _deleteFiles(client, org, repo, deleted) {
     const { log } = this;
     let pushed = 0;
     let errors = 0;
-    const successfullyDeleted = new Set();
 
+    // processQueue consumes its input array via shift(); copy so the caller's list survives.
     const results = await processQueue(
-      deleted,
+      [...deleted],
       async (daPath) => {
         try {
           await client.deleteSource(org, repo, daPath);
           log.info(`  ✓ deleted ${daPath}`);
-          return { ok: true, daPath };
+          return { ok: true };
         } catch (err) {
           log.warn(`  ✗ ${daPath}: ${err.message}`);
           return { ok: false };
@@ -176,13 +175,12 @@ export default class PushCommand {
     for (const r of results) {
       if (r.ok) {
         pushed += 1;
-        successfullyDeleted.add(r.daPath);
       } else {
         errors += 1;
       }
     }
 
-    return { pushed, errors, successfullyDeleted };
+    return { pushed, errors };
   }
 
   async run() {
@@ -207,12 +205,21 @@ export default class PushCommand {
     const syncedOid = await resolveSyncedOid(fs, contentDir);
     const lastSyncTime = await getCommitCommitterTimeMs(fs, contentDir, syncedOid);
 
-    let { added, modified, deleted } = await diffCommitTrees(fs, contentDir, syncedOid, headOid);
+    const fullChanges = await diffCommitTrees(fs, contentDir, syncedOid, headOid);
+
+    const scope = this._pushPath ? this._pushPath.replace(/\/+$/, '') : null;
+    const inScope = (daPath) => scope === null
+      || daPath === scope
+      || daPath.startsWith(`${scope}/`);
+    const added = fullChanges.added.filter(inScope);
+    const modified = fullChanges.modified.filter(inScope);
+    const deleted = fullChanges.deleted.filter(inScope);
 
-    const inScope = (daPath) => !this._pushPath || daPath.startsWith(this._pushPath);
-    added = added.filter(inScope);
-    modified = modified.filter(inScope);
-    deleted = deleted.filter(inScope);
+    const fullCount = fullChanges.added.length
+      + fullChanges.modified.length
+      + fullChanges.deleted.length;
+    const scopeCount = added.length + modified.length + deleted.length;
+    const scopeIsComplete = fullCount === scopeCount;
 
     if (added.length === 0 && modified.length === 0 && deleted.length === 0) {
       log.info('Nothing to push. No commits ahead of the last da.live sync.');
@@ -261,27 +268,27 @@ export default class PushCommand {
       return;
     }
 
-    const putTargets = [...added, ...modified];
     const {
       pushed: putPushed,
       errors: putErrors,
-      successfullyPushed,
-    } = await this._uploadFiles(client, org, repo, contentDir, putTargets);
+    } = await this._uploadFiles(client, org, repo, contentDir, [...added, ...modified]);
 
     const {
       pushed: deletePushed,
       errors: deleteErrors,
-      successfullyDeleted,
     } = await this._deleteFiles(client, org, repo, deleted);
 
     const pushed = putPushed + deletePushed;
     const pushErrors = putErrors + deleteErrors;
+    const allOk = pushErrors === 0;
 
-    const allPutsOk = putTargets.every((p) => successfullyPushed.has(p));
-    const allDeletesOk = deleted.every((p) => successfullyDeleted.has(p));
-
-    if (allPutsOk && allDeletesOk) {
+    if (allOk && scopeIsComplete) {
       await writeSyncedRef(fs, contentDir, headOid);
+    } else if (allOk) {
+      log.info(
+        '\n--path filter held back other changes; sync ref not advanced. '
+        + 'Run \'aem content push\' (without --path) to fully sync.',
+      );
     } else {
       log.warn('\nSync ref not updated: fix errors and push again to finish syncing this commit.');
     }

test/content/da-api.test.js

@@ -343,6 +343,20 @@ describe('DaClient', () => {
       await assert.rejects(() => client.deleteSource('org', 'repo', '/file.html'), /Unauthorized/);
     });
 
+    it('throws on 5xx so callers cannot treat server failure as success', async () => {
+      client.fetch = async () => mockResponse(500, 'Server Error', false);
+      await assert.rejects(
+        () => client.deleteSource('org', 'repo', '/file.html'),
+        /DELETE failed/,
+      );
+    });
+
+    it('treats 404 as success (idempotent)', async () => {
+      client.fetch = async () => mockResponse(404, 'Not Found', false);
+      const result = await client.deleteSource('org', 'repo', '/missing.html');
+      assert.strictEqual(result, true);
+    });
+
     it('calls correct source URL', async () => {
       let calledUrl;
       client.fetch = async (url) => {

test/content/merge.cmd.test.js

@@ -201,5 +201,31 @@ describe('MergeCommand', () => {
       const cmd = new Cmd(makeLogger()).withDirectory(testRoot).withFilePath('index.html');
       await cmd.run();
     });
+
+    it('passes the project root (not content/) to getValidToken so the cached login is reused', async () => {
+      const contentDir = await setupContentDir(testRoot);
+      await fse.writeFile(
+        path.join(contentDir, 'index.html'),
+        '<html><p>index</p><p>local extra</p></html>',
+      );
+
+      let capturedProjectDir;
+      const mod = await esmock('../../src/content/merge.cmd.js', {
+        '../../src/content/da-auth.js': {
+          getValidToken: async (_log, _override, projectDir) => {
+            capturedProjectDir = projectDir;
+            return 'token';
+          },
+        },
+        '../../src/content/da-api.js': {
+          DaClient: createDaClientClass({ sourceContent: '<html><p>remote</p></html>' }),
+        },
+      });
+      const Cmd = mod.default;
+      const cmd = new Cmd(makeLogger()).withDirectory(testRoot);
+      await cmd.run();
+
+      assert.strictEqual(capturedProjectDir, testRoot);
+    });
   });
 });

test/content/push.cmd.test.js

@@ -12,10 +12,13 @@
 
 /* eslint-env mocha */
 import assert from 'assert';
+import fs from 'fs';
 import path from 'path';
 import fse from 'fs-extra';
+import git from 'isomorphic-git';
 import esmock from 'esmock';
 import { createTestRoot } from '../utils.js';
+import { DA_SYNCED_REF } from '../../src/content/content-git.js';
 import {
   makeLogger,
   setupContentDir,
@@ -249,6 +252,118 @@ describe('PushCommand', () => {
       assert.ok(!pushed.includes('/index.html'));
     });
 
+    it('--path /blog does not match /blog2/* (no prefix bleed)', async () => {
+      const contentDir = await setupContentDir(testRoot);
+      await fse.ensureDir(path.join(contentDir, 'blog2'));
+      await fse.writeFile(path.join(contentDir, 'blog', 'post.html'), 'changed blog');
+      await fse.writeFile(path.join(contentDir, 'blog2', 'post.html'), 'new blog2');
+      await stageAllAndCommit(contentDir, 'edit blog and add blog2');
+
+      const pushed = [];
+      const DaClientClass = createDaClientClass({ onPut: (p) => pushed.push(p) });
+      const log = makeLogger();
+      const mod = await esmock('../../src/content/push.cmd.js', {
+        '../../src/content/da-auth.js': { getValidToken: async () => 'token' },
+        '../../src/content/da-api.js': {
+          DaClient: DaClientClass,
+          getContentType: () => 'text/html',
+        },
+      });
+      const Cmd = mod.default;
+      const cmd = new Cmd(log).withDirectory(testRoot).withPath('/blog');
+      await cmd.run();
+
+      assert.ok(pushed.includes('/blog/post.html'));
+      assert.ok(!pushed.includes('/blog2/post.html'));
+    });
+
+    it('does not advance synced ref when --path filter excludes other ahead changes', async () => {
+      const contentDir = await setupContentDir(testRoot);
+      await fse.writeFile(path.join(contentDir, 'index.html'), 'changed');
+      await fse.writeFile(path.join(contentDir, 'blog', 'post.html'), 'changed blog');
+      await stageAllAndCommit(contentDir, 'edit pages');
+
+      const pushed = [];
+      const DaClientClass = createDaClientClass({ onPut: (p) => pushed.push(p) });
+      const log = makeLogger();
+      const mod = await esmock('../../src/content/push.cmd.js', {
+        '../../src/content/da-auth.js': { getValidToken: async () => 'token' },
+        '../../src/content/da-api.js': {
+          DaClient: DaClientClass,
+          getContentType: () => 'text/html',
+        },
+      });
+      const Cmd = mod.default;
+      const cmd = new Cmd(log).withDirectory(testRoot).withPath('/blog');
+      await cmd.run();
+
+      // The in-scope file must still be uploaded successfully…
+      assert.ok(pushed.includes('/blog/post.html'));
+      assert.ok(!pushed.includes('/index.html'));
+      // …but the synced ref must not advance to HEAD, otherwise the unfiltered
+      // /index.html change would be silently considered already-synced next time.
+      const headOid = await git.resolveRef({ fs, dir: contentDir, ref: 'HEAD' });
+      let syncedOid;
+      try {
+        syncedOid = await git.resolveRef({ fs, dir: contentDir, ref: DA_SYNCED_REF });
+      } catch {
+        syncedOid = null;
+      }
+      assert.notStrictEqual(syncedOid, headOid);
+      assert.ok(log.logs.some((l) => l.msg.includes('held back') || l.msg.includes('without --path')));
+    });
+
+    it('advances synced ref to HEAD when --path covers all ahead changes', async () => {
+      const contentDir = await setupContentDir(testRoot);
+      await fse.writeFile(path.join(contentDir, 'blog', 'post.html'), 'changed blog only');
+      await stageAllAndCommit(contentDir, 'edit blog');
+
+      const mod = await esmock('../../src/content/push.cmd.js', {
+        '../../src/content/da-auth.js': { getValidToken: async () => 'token' },
+        '../../src/content/da-api.js': {
+          DaClient: createDaClientClass(),
+          getContentType: () => 'text/html',
+        },
+      });
+      const Cmd = mod.default;
+      const cmd = new Cmd(makeLogger()).withDirectory(testRoot).withPath('/blog');
+      await cmd.run();
+
+      const headOid = await git.resolveRef({ fs, dir: contentDir, ref: 'HEAD' });
+      const syncedOid = await git.resolveRef({ fs, dir: contentDir, ref: DA_SYNCED_REF });
+      assert.strictEqual(syncedOid, headOid);
+    });
+
+    it('does not advance synced ref when a delete fails on the server', async () => {
+      const contentDir = await setupContentDir(testRoot);
+      await fse.remove(path.join(contentDir, 'index.html'));
+      await stageAllAndCommit(contentDir, 'drop index');
+
+      const DaClientClass = createDaClientClass({ deleteFails: true });
+      const log = makeLogger();
+      const mod = await esmock('../../src/content/push.cmd.js', {
+        '../../src/content/da-auth.js': { getValidToken: async () => 'token' },
+        '../../src/content/da-api.js': {
+          DaClient: DaClientClass,
+          getContentType: () => 'text/html',
+        },
+      });
+      const Cmd = mod.default;
+      const cmd = new Cmd(log).withDirectory(testRoot);
+      await cmd.run();
+
+      const headOid = await git.resolveRef({ fs, dir: contentDir, ref: 'HEAD' });
+      let syncedOid;
+      try {
+        syncedOid = await git.resolveRef({ fs, dir: contentDir, ref: DA_SYNCED_REF });
+      } catch {
+        syncedOid = null;
+      }
+      assert.notStrictEqual(syncedOid, headOid);
+      assert.ok(log.logs.some((l) => l.msg.includes('Sync ref not updated')));
+      assert.ok(log.logs.some((l) => l.msg.includes('error') || l.msg.includes('✗')));
+    });
+
     it('shows dry-run added/modified/deleted sections', async () => {
       const contentDir = await setupContentDir(testRoot);
       await fse.writeFile(path.join(contentDir, 'index.html'), 'changed');