fix(sync): stop pull silently overwriting files the user already had

Found while trying the tool the way a new user would. Pointing scriptRoot
at a folder that already contains files meant pull wrote straight over
them, without a word.

The safety model said 'pull never deletes local files', which was true and
beside the point: clobbering loses the work just the same. compare.ts
reported an untracked local file colliding with a never-synced remote
script as remote-only, so pull treated it as a plain download.

A collision is now a conflict, so pull refuses without --force. Identical
content is adopted silently rather than reported, since that is already
what pull wants the file to say.

The docs failed here too. The README explained neither where the config
lives, nor that commands search upward for it, nor what scriptRoot is
relative to — the only way to find out was to read config.ts. A 'Where
things live' section now covers the layout, the upward search, and why
scriptRoot cannot escape the project root.

The error message itself said what was forbidden and not what to do. The
natural reaction to 'must be relative' is to try '../elsewhere', which then
hits the second rule with equally little help. Both now point at the actual
answer: run init in the folder you want, or use -C.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: mschmicking

README.md

@@ -54,6 +54,7 @@ The five commands worth knowing on day one:
 
 - [Why this exists](#why-this-exists)
 - [iobroker-sync or the VS Code extension?](#iobroker-sync-or-the-vs-code-extension)
+- [Where things live](#where-things-live)
 - [How scripts map to files](#how-scripts-map-to-files)
 - [The edit loop](#the-edit-loop)
 - [Commands](#commands)
@@ -98,6 +99,50 @@ Rough rule: **if you live in VS Code, use the extension.** If you want your own
 git-first workflow, or anything automated, use this. They are not exclusive — both talk
 to the same Admin API, and this tool never writes fields it does not own.
 
+## Where things live
+
+`iob-sync` is a global command that works on a **project folder** — the folder holding
+`.iobroker-sync.json`. Where the tool itself is installed is irrelevant; nothing is ever
+read from or written to `node_modules`.
+
+```
+~/iobroker-scripts/            <- project root: run iob-sync anywhere inside it
+├── .iobroker-sync.json        <- config. Commit it; it holds no password.
+├── .iobroker-sync/            <- state, backups, trash. Gitignored; may contain secrets.
+└── scripts/                   <- scriptRoot: your scripts land here
+    ├── common/garage.ts
+    └── Switch-Musiccast.js
+```
+
+Commands search upward from the current directory for `.iobroker-sync.json`, the way
+`git` finds `.git` — so you can run them from any subfolder. `-C <dir>` runs as if
+started somewhere else.
+
+**`scriptRoot` is relative to the project root and cannot escape it.** Absolute paths and
+`../` are rejected: it is the directory the tool writes into, so a config that pointed
+outside could drop files anywhere on your disk.
+
+That means you do not point an existing project at a scripts folder elsewhere — you run
+`init` **in** the folder you want to keep scripts in:
+
+```bash
+cd ~/iobroker-scripts     # your git repo
+iob-sync init             # config lands here, scriptRoot defaults to "scripts"
+```
+
+Your password is **not** stored here. It lives in
+`~/.config/iobroker-sync/credentials.json` — see [Authentication](#authentication).
+
+### Pointing scriptRoot at a folder that already has files
+
+Safe, but worth knowing: if a script would land on top of a file you already have, and
+the two differ, `pull` reports a **conflict** and leaves your file alone. Use `--force`
+to take the server's copy. Identical files are adopted silently.
+
+Setting `scriptRoot` to `.` (the project root itself) works, but then scripts land beside
+your `README.md` and `package.json`. A subfolder is tidier and keeps `status` output
+meaningful.
+
 ## How scripts map to files
 
 | ioBroker object                                | local file                    |
@@ -170,8 +215,10 @@ containing `*` is an anchored glob (`iob-sync status 'common/*.ts'`).
 This tool talks to a live home-automation system, and is deliberately conservative about
 destroying work:
 
-- **`pull` never deletes local files.** A script removed on the server shows up in `status`
-  as `remote-missing`; what to do about it is your call.
+- **`pull` never deletes local files, and never silently overwrites one.** A script
+  removed on the server shows up in `status` as `remote-missing`; what to do is your
+  call. A script that would land on top of an existing untracked file is reported as a
+  conflict rather than written over it.
 - **`push` never deletes remote objects**, and writes only `common.source` and
   `common.engineType`. It _cannot_ disable a running script or move it to a different
   javascript instance, because it never sends those fields — a sync bug structurally

src/config.ts

@@ -37,18 +37,32 @@ function validateUrl(url: string): void {
   }
 }
 
+/**
+ * `scriptRoot` names the directory this tool writes into, resolved against the folder
+ * holding the config. It therefore may not be absolute or contain `..` — either would
+ * let a config file place files anywhere on the filesystem.
+ *
+ * The hints matter: the natural reaction to "must be relative" is to try `../my-scripts`,
+ * which then hits the second rule, and neither message says what to do instead. The
+ * answer is always the same — run `init` inside the folder you want the scripts in.
+ */
+const SCRIPT_ROOT_HINT =
+  'scriptRoot is a folder *inside* the project (the directory holding ' +
+  `${CONFIG_FILENAME}), e.g. "scripts". To keep scripts somewhere else, run ` +
+  '`iob-sync init` in that folder instead, or use `iob-sync -C <dir>` to work there.';
+
 function validateScriptRoot(scriptRoot: string): void {
   if (path.isAbsolute(scriptRoot) || /^[a-zA-Z]:[\\/]/.test(scriptRoot)) {
     throw new UserError(
-      `Config "scriptRoot" must be relative to the project root, got "${scriptRoot}".`,
-      'Use a relative path such as "scripts".',
+      `Config "scriptRoot" must be relative, got the absolute path "${scriptRoot}".`,
+      SCRIPT_ROOT_HINT,
     );
   }
   const segments = scriptRoot.split(/[\\/]+/).filter((s) => s.length > 0);
   if (segments.some((s) => s === '..')) {
     throw new UserError(
       `Config "scriptRoot" must not escape the project root: "${scriptRoot}".`,
-      'Remove ".." segments from scriptRoot.',
+      SCRIPT_ROOT_HINT,
     );
   }
 }

src/sync/compare.ts

@@ -126,9 +126,32 @@ export function computeStatus(input: ComputeStatusInput): SyncStatus[] {
     if (!entry && remoteInfo) {
       // Never synced, exists on the server only. There's no manifest path to trust
       // yet, so derive the path the same way `pull` would place it.
+      const derivedPath = idToRelPath(id, remoteInfo.engineType);
+      const collidingLocal = local.get(derivedPath);
+
+      if (collidingLocal) {
+        // A file the user already had is sitting exactly where this script would
+        // land, and nothing records that the two are related. Reporting `remote-only`
+        // here made `pull` overwrite it without a word — "pull never deletes local
+        // files" was true and beside the point, because clobbering loses the work
+        // just the same. Treat it as a conflict so it needs --force.
+        consumedLocalPaths.add(derivedPath);
+        results.push({
+          id,
+          path: derivedPath,
+          state: collidingLocal.hash === remoteInfo.sourceHash ? 'in-sync' : 'conflict',
+          engineType: remoteInfo.engineType,
+          engine: remoteInfo.engine,
+          enabled: remoteInfo.enabled,
+          remoteHash: remoteInfo.sourceHash,
+          localHash: collidingLocal.hash,
+        });
+        continue;
+      }
+
       results.push({
         id,
-        path: idToRelPath(id, remoteInfo.engineType),
+        path: derivedPath,
         state: 'remote-only',
         engineType: remoteInfo.engineType,
         engine: remoteInfo.engine,

test/commands-sync.test.ts

@@ -365,3 +365,92 @@ describe('sync commands', () => {
     assert.match(t.captured.all.join('\n'), /REMOTE-MISSING/i);
   });
 });
+
+describe('pull vs files the user already had', () => {
+  let server: FakeAdminServer;
+  let port: number;
+  let project: TempProject;
+
+  before(async () => {
+    server = new FakeAdminServer();
+    port = await server.start();
+  });
+
+  after(async () => {
+    await server.stop();
+  });
+
+  beforeEach(async () => {
+    server.reset();
+    if (project) await project.cleanup();
+    project = await makeTempProject();
+  });
+
+  /** A never-synced script whose derived path collides with an existing local file. */
+  function collidingScript(source: string): ScriptObject {
+    return {
+      _id: 'script.js.notes',
+      type: 'script',
+      common: {
+        name: 'notes',
+        source,
+        engineType: 'TypeScript/ts',
+        engine: 'system.adapter.javascript.0',
+        enabled: true,
+        expert: true,
+      },
+      native: {},
+    };
+  }
+
+  it('refuses to clobber an untracked local file a script would land on', async () => {
+    // The scenario: someone points scriptRoot at a folder that already has files.
+    // "pull never deletes local files" was true and beside the point — overwriting
+    // loses the work just the same.
+    server.seed([collidingScript('REMOTE CONTENT\n')]);
+    await writeLocal(project, 'notes.ts', 'MY OWN FILE\n');
+
+    const t = await makeContext(port, project);
+    try {
+      await pull(t.ctx, {});
+
+      assert.equal(await readLocal(project, 'notes.ts'), 'MY OWN FILE\n');
+      assert.ok(
+        t.captured.all.some((l) => /conflict/i.test(l)),
+        `expected a conflict report, got ${JSON.stringify(t.captured.all)}`,
+      );
+    } finally {
+      await t.close();
+    }
+  });
+
+  it('--force still takes the remote copy', async () => {
+    server.seed([collidingScript('REMOTE CONTENT\n')]);
+    await writeLocal(project, 'notes.ts', 'MY OWN FILE\n');
+
+    const t = await makeContext(port, project);
+    try {
+      await pull(t.ctx, { force: true });
+
+      assert.equal(await readLocal(project, 'notes.ts'), 'REMOTE CONTENT\n');
+    } finally {
+      await t.close();
+    }
+  });
+
+  it('adopts an identical local file without complaining', async () => {
+    // Same bytes on both sides is not a conflict — it is already what pull wants.
+    server.seed([collidingScript('SAME\n')]);
+    await writeLocal(project, 'notes.ts', 'SAME\n');
+
+    const t = await makeContext(port, project);
+    try {
+      await pull(t.ctx, {});
+
+      assert.equal(await readLocal(project, 'notes.ts'), 'SAME\n');
+      assert.ok(!t.captured.all.some((l) => /conflict/i.test(l)));
+    } finally {
+      await t.close();
+    }
+  });
+});