fix(sync): let auto-commit recognise its own translation output

CHANGELOG.md

@@ -52,6 +52,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **sync**: `--auto-commit` now recognises its own translation output rather than only the files a given run wrote, which fixes two related problems. A translation left on disk by an earlier refused run was classed as an unrelated modification, so auto-commit refused forever and the user had to commit it by hand; it is now committed once the genuinely unrelated changes are dealt with. And in `--watch` mode, where the same path runs once per trigger, a trigger that translated nothing skipped the checks entirely and reported success while a commit was still owed. Staging is also driven by what is actually dirty, so a rewrite that produced identical bytes no longer attempts an empty commit. Ownership is derived from the lockfile's tracked source files, with each file matched to its own bucket so one bucket's `target_path_pattern` cannot claim another's output.
 - **sync**: Re-running `deepl sync --auto-commit` after a refusal no longer reports success without committing. A refused run still writes its translations to disk, so the retry found nothing to translate; the preflight was skipped in that case and the command exited 0 with no commit made, the translation still uncommitted and the tree still dirty. The repo-state checks now run whenever `--auto-commit` is requested, so the refusal is reported identically on every attempt. Staging and committing are still skipped when there is nothing to commit. Two tests covered this path but could not see the defect: their assertions ran only inside a `catch` the retry never entered, which also hid an assertion that could never have matched (it looked for "detached HEAD" against a message reading "HEAD is detached").
 - **write**: `deepl write` now runs from a published install. `diff` is imported at the top of the write command but was declared only under `devDependencies`, which consumers do not install, so every command that loaded the module failed with `Cannot find package 'diff'` before producing output. `--help` did not surface it because the module loads lazily.
 - **init/write/sync**: `@inquirer/prompts` is declared as a dependency. It is imported at runtime by `init`, `write --interactive` and `sync init` while only the unused `inquirer` was declared, so it resolved through npm's hoisting: under a strict layout (pnpm, `--install-strategy=nested`) those commands failed with `ERR_MODULE_NOT_FOUND`, and under npm the prompt that reads the API key bound to whatever major another dependent happened to hoist.

docs/SYNC.md

@@ -860,7 +860,7 @@ The `--auto-commit` flag stages translated files and the lock file (when updated
 
 The `.deepl-sync.lock` file is only staged when this sync run actually wrote an updated lockfile; a no-op sync (all locales current) will not stage or commit.
 
-**Combined with `--watch`.** `deepl sync --watch --auto-commit` commits on **every successful sync cycle**, not only the initial pre-watch sync. Each commit is gated by the same preflight checks listed above (clean tree, on a branch, not mid-rebase/merge/cherry-pick, files actually written). If preflight fails for a given cycle, that cycle throws and the watcher logs the error; subsequent cycles retry normally once the tree is clean again.
+**Combined with `--watch`.** `deepl sync --watch --auto-commit` commits on **every successful sync cycle**, not only the initial pre-watch sync. Each commit is gated by the same preflight checks listed above (clean tree, on a branch, not mid-rebase/merge/cherry-pick). If preflight fails for a given cycle, that cycle throws and the watcher logs the error; subsequent cycles report the same refusal until the tree is clean again, at which point the pending translations are committed — including any left behind by an earlier refused cycle. A cycle with nothing to commit is silent.
 
 ### GitLab CI
 

src/cli/commands/sync-command.ts

@@ -384,7 +384,7 @@ export class SyncCommand {
           backupTracker,
         });
         this.displayResult(result, 'text');
-        if (options.autoCommit && result.success && !result.dryRun && !result.driftDetected && result.fileResults.length > 0) {
+        if (options.autoCommit && result.success && !result.dryRun && !result.driftDetected) {
           await this.autoCommitTranslations(result, activeConfig);
         }
       },
@@ -426,6 +426,49 @@ export class SyncCommand {
     });
   }
 
+  /**
+   * Every relative path this config's buckets could write, mapped to its
+   * locale. Derived from the lockfile's tracked source files rather than by
+   * re-walking globs, so it agrees with what the sync engine manages, and
+   * each source file is matched to its own bucket so one bucket's
+   * target_path_pattern cannot claim ownership of another's output.
+   */
+  private async resolveOwnedTargetPaths(
+    config: ResolvedSyncConfig,
+  ): Promise<Map<string, string>> {
+    const pathMod = await import('path');
+    const { minimatch } = await import('minimatch');
+    const { SyncLockManager } = await import('../../sync/sync-lock.js');
+    const { resolveTargetPath } = await import('../../sync/sync-utils.js');
+
+    const owned = new Map<string, string>();
+    const lockManager = new SyncLockManager(pathMod.join(config.projectRoot, LOCK_FILE_NAME));
+    const lockFile = await lockManager.read();
+    const sourcePaths = Object.keys(lockFile.entries);
+
+    for (const bucketConfig of Object.values(config.buckets)) {
+      for (const sourcePath of sourcePaths) {
+        if (!bucketConfig.include.some(pattern => minimatch(sourcePath, pattern))) continue;
+        for (const locale of config.target_locales) {
+          try {
+            const targetPath = resolveTargetPath(
+              sourcePath,
+              config.source_locale,
+              locale,
+              bucketConfig.target_path_pattern,
+            );
+            owned.set(targetPath, locale);
+          } catch {
+            // The source path does not carry the source locale, so this bucket
+            // cannot derive a target for it — not a path we own.
+          }
+        }
+      }
+    }
+
+    return owned;
+  }
+
   private async autoCommitTranslations(result: SyncResult, config: ResolvedSyncConfig): Promise<void> {
     const { execFile } = await import('child_process');
     const { promisify } = await import('util');
@@ -453,8 +496,13 @@ export class SyncCommand {
     // its translations on disk, so the retry finds nothing to translate — and
     // skipping the checks there reported success while no commit had been made
     // and the tree was still dirty.
-    const expectedStaged = new Set<string>(writtenFiles);
-    if (result.lockUpdated) expectedStaged.add(LOCK_FILE_NAME);
+    //
+    // Every path this config could write counts as expected, not just the ones
+    // written now: a translation left behind by an earlier refused run is ours
+    // to commit, and treating it as an unrelated modification would refuse
+    // forever with no way out but a manual commit.
+    const ownedPaths = await this.resolveOwnedTargetPaths(config);
+    const expectedStaged = new Set<string>([...writtenFiles, ...ownedPaths.keys(), LOCK_FILE_NAME]);
 
     // 1. In-progress rebase/merge/cherry-pick
     const gitDir = (await execFileAsync('git', ['rev-parse', '--git-dir'], { cwd })).stdout.trim();
@@ -488,6 +536,7 @@ export class SyncCommand {
     // that aren't part of this sync run).
     const { stdout: porcelain } = await execFileAsync('git', ['status', '--porcelain', '-z'], { cwd });
     const unrelated: string[] = [];
+    const dirtyOwned: string[] = [];
     if (porcelain.length > 0) {
       const parts = porcelain.split('\0').filter(Boolean);
       for (const part of parts) {
@@ -498,7 +547,10 @@ export class SyncCommand {
         const statusCode = part.slice(0, 2);
         const filePath = part.slice(3);
         if (!filePath) continue;
-        if (expectedStaged.has(filePath)) continue;
+        if (expectedStaged.has(filePath)) {
+          dirtyOwned.push(filePath);
+          continue;
+        }
         // Untracked files with the exact target path are already filtered by
         // the expectedStaged check; everything else is unrelated.
         unrelated.push(`${filePath} (${statusCode.trim() || '??'})`);
@@ -512,19 +564,23 @@ export class SyncCommand {
       );
     }
 
-    if (writtenFiles.length === 0) return;
-
-    const filesToStage: string[] = [...writtenFiles];
-    if (result.lockUpdated && fsMod.existsSync(pathMod.join(cwd, LOCK_FILE_NAME))) {
-      filesToStage.push(LOCK_FILE_NAME);
-    }
+    // Staged from what is actually dirty rather than from what this run wrote:
+    // a rewrite that produced identical bytes has nothing to commit, and a
+    // translation an earlier refused run left behind does.
+    const filesToStage = dirtyOwned;
+    if (filesToStage.length === 0) return;
 
     for (const file of filesToStage) {
       await execFileAsync('git', ['add', file], { cwd });
     }
 
-    const locales = [...new Set(result.fileResults.map(r => r.locale))].join(', ');
-    const msg = `chore(i18n): sync translations for ${locales}`;
+    const localeOf = new Map<string, string>(ownedPaths);
+    for (const fileResult of result.fileResults) localeOf.set(fileResult.file, fileResult.locale);
+    const locales = [...new Set(filesToStage.map(file => localeOf.get(file)).filter(Boolean))].join(', ');
+
+    const msg = locales
+      ? `chore(i18n): sync translations for ${locales}`
+      : 'chore(i18n): sync translation lockfile';
     await execFileAsync('git', ['commit', '-m', msg], { cwd });
     Logger.info(`Auto-committed ${filesToStage.length} file(s): ${msg}`);
   }

tests/integration/sync-auto-commit.integration.test.ts

@@ -244,6 +244,58 @@ describeIfGit('SyncCommand auto-commit preflight', () => {
     expect(git(tmpDir, ['log', '-1', '--pretty=%s']).trim()).toBe('initial');
   });
 
+  // A translation left on disk by an earlier refused run belongs to this sync,
+  // so once the unrelated changes are dealt with the retry commits it. It used
+  // to be classed as an unrelated modification, which refused forever and left
+  // the user to commit it by hand.
+  it('commits a translation left behind by an earlier refusal once the tree is otherwise clean', async () => {
+    expect.assertions(4);
+    seedRepo(tmpDir);
+    fs.writeFileSync(path.join(tmpDir, 'README.md'), '# changed\n', 'utf-8');
+    const scope = seedTranslateMock();
+    scope.persist();
+
+    const command = new SyncCommand(syncService);
+    await expect(command.run({ autoCommit: true })).rejects.toThrow(ValidationError);
+    expect(fs.existsSync(path.join(tmpDir, 'locales', 'de.json'))).toBe(true);
+
+    // Deal with the unrelated change; the leftover translation remains.
+    git(tmpDir, ['checkout', '--', 'README.md']);
+
+    await command.run({ autoCommit: true });
+
+    nock.cleanAll();
+    expect(git(tmpDir, ['log', '-1', '--pretty=%s']).trim()).toContain('chore(i18n)');
+    expect(git(tmpDir, ['status', '--porcelain']).trim()).toBe('');
+  });
+
+  // Watch mode calls the same path once per trigger. A trigger that translates
+  // nothing must not report success while a commit is still owed, and must not
+  // manufacture an empty commit either.
+  it('refuses consistently across repeated triggers and stays quiet when there is nothing to commit', async () => {
+    expect.assertions(4);
+    seedRepo(tmpDir);
+    fs.writeFileSync(path.join(tmpDir, 'src.ts'), 'export {};\n', 'utf-8');
+    const scope = seedTranslateMock();
+    scope.persist();
+
+    const command = new SyncCommand(syncService);
+    await expect(command.run({ autoCommit: true })).rejects.toThrow(/Refusing to auto-commit/i);
+    await expect(command.run({ autoCommit: true })).rejects.toThrow(/Refusing to auto-commit/i);
+
+    // With the unrelated file gone and the translation committed, a further
+    // trigger has nothing to do and must neither throw nor commit.
+    fs.unlinkSync(path.join(tmpDir, 'src.ts'));
+    git(tmpDir, ['add', '-A']);
+    git(tmpDir, ['commit', '-q', '-m', 'tidy']);
+
+    await command.run({ autoCommit: true });
+
+    nock.cleanAll();
+    expect(git(tmpDir, ['log', '-1', '--pretty=%s']).trim()).toBe('tidy');
+    expect(git(tmpDir, ['status', '--porcelain']).trim()).toBe('');
+  });
+
   it('clean repo, no lockfile update: auto-commit does not fail staging a nonexistent lockfile', async () => {
     seedRepo(tmpDir);
     const scope = seedTranslateMock();

tests/unit/sync/sync-command.test.ts

@@ -20,6 +20,13 @@ jest.mock('child_process', () => ({
   execFile: mockExecFile,
 }));
 
+/**
+ * `git status --porcelain -z` output the mocked git returns. NUL-separated
+ * "XY path" entries, matching what auto-commit parses. Tests that care about a
+ * particular working-tree state assign to this.
+ */
+let mockPorcelain = '';
+
 const mockExistsSync = jest.fn((_p: string | URL) => false);
 jest.mock('fs', () => {
   const actual = jest.requireActual('fs');
@@ -603,9 +610,18 @@ describe('SyncCommand', () => {
           if (Array.isArray(args) && args[0] === 'rev-parse' && args[1] === '--git-dir') {
             stdout = '.git';
           }
+          // A file the sync just wrote is dirty. Reporting a clean tree here
+          // described a state git cannot produce, and staging is driven by
+          // what is actually dirty rather than by what the run claims it wrote.
+          if (Array.isArray(args) && args[0] === 'status') {
+            stdout = mockPorcelain;
+          }
           if (cb) cb(null, { stdout, stderr: '' });
         },
       );
+      // Default to a clean tree; tests that expect a commit declare the dirt
+      // their sync would have produced.
+      mockPorcelain = '';
       mockExistsSync.mockReset();
       mockExistsSync.mockImplementation((p: string | URL) => {
         // Lockfile exists by default so autoCommit stages it when lockUpdated=true.
@@ -614,6 +630,7 @@ describe('SyncCommand', () => {
     });
 
     it('should call git add and git commit when autoCommit=true and files were written', async () => {
+      mockPorcelain = ' M locales/de.json\0 M .deepl-sync.lock\0';
       const result = makeResult({
         fileResults: [writtenFile],
         lockUpdated: true,
@@ -638,6 +655,8 @@ describe('SyncCommand', () => {
     });
 
     it('should not stage .deepl-sync.lock when lockUpdated is false', async () => {
+      // Lockfile untouched, so it is not dirty and must not be staged.
+      mockPorcelain = ' M locales/de.json\0';
       const result = makeResult({
         fileResults: [writtenFile],
         lockUpdated: false,
@@ -655,6 +674,7 @@ describe('SyncCommand', () => {
     });
 
     it('should pass cwd=config.projectRoot to every git invocation', async () => {
+      mockPorcelain = ' M locales/de.json\0 M .deepl-sync.lock\0';
       const result = makeResult({ fileResults: [writtenFile], lockUpdated: true });
       const mockService = createMockSyncService(result);
       const command = new SyncCommand(mockService);
@@ -673,6 +693,7 @@ describe('SyncCommand', () => {
     });
 
     it('should stage multiple written files and include all locales in commit message', async () => {
+      mockPorcelain = ' M locales/de.json\0 M locales/fr.json\0 M .deepl-sync.lock\0';
       const writtenFileFr: SyncFileResult = {
         file: 'locales/fr.json',
         locale: 'fr',
@@ -804,6 +825,7 @@ describe('SyncCommand', () => {
     });
 
     it('should log success message after committing', async () => {
+      mockPorcelain = ' M locales/de.json\0 M .deepl-sync.lock\0';
       const result = makeResult({ fileResults: [writtenFile], lockUpdated: true });
       const mockService = createMockSyncService(result);
       const command = new SyncCommand(mockService);
@@ -954,6 +976,12 @@ describe('SyncCommand', () => {
           if (Array.isArray(args) && args[0] === 'rev-parse' && args[1] === '--git-dir') {
             stdout = '.git';
           }
+          // A file the sync just wrote is dirty. Reporting a clean tree here
+          // described a state git cannot produce, and staging is driven by
+          // what is actually dirty rather than by what the run claims it wrote.
+          if (Array.isArray(args) && args[0] === 'status') {
+            stdout = mockPorcelain;
+          }
           if (cb) cb(null, { stdout, stderr: '' });
         },
       );
@@ -972,6 +1000,7 @@ describe('SyncCommand', () => {
         return mockWatcher;
       });
 
+      mockPorcelain = ' M locales/de.json\0 M .deepl-sync.lock\0';
       const result = makeResult({
         fileResults: [
           { file: 'locales/de.json', locale: 'de', translated: 5, skipped: 0, failed: 0, written: true },