feat(cli): sweep the adapter markers a deleted script leaves behind

The javascript adapter keeps two bookkeeping states beside every script, on every
javascript instance rather than only the one that runs it:

    javascript.<n>.scriptEnabled.<id>
    javascript.<n>.scriptProblem.<id>

Both are created by the adapter's load(), which calls createActiveObject and
createProblemObject before prepareScript checks common.engine to decide whether
this instance should run the script at all. Every instance runs load() for every
non-global script at startup and again on every source change, so all of them hold
a pair for every script.

Deletion, though, is gated on the engine: only the instance that owned the script
at the moment it was deleted removes its own pair. Every other instance keeps one
for a script that no longer exists, js-controller warns about it for the life of
the system, and nothing in ioBroker ever collects it. This is independent of who
deleted the script — the Admin UI leaves the identical residue.

remove, rename and move now sweep both markers of the id they just deleted. The
sweep runs after the object is gone and after the trash backup, and is best-effort:
it warns rather than turning a completed delete into a failure. Order is enforced
in ObjectsApi.deleteScriptMarker — value first, object second, never the reverse,
because an object deleted out from under a surviving value is exactly the orphan
being cleaned up. (The adapter's own cleanup gets this backwards, which is a likely
source of the "state has no object" warnings.)

remove also stops refusing an id whose script is already gone when markers remain,
since otherwise pre-existing ones cannot be cleared by anything. It sweeps them,
touches nothing else, and keeps the local file.

doctor gains a read-only `markers` check that counts both kinds, names the orphans,
and warns rather than fails — nothing here is broken.

Both kinds are handled through MARKER_KINDS in types.ts rather than a pattern
repeated per call site: an earlier draft covered only scriptEnabled and left eight
orphaned scriptProblem states on a live instance while doctor reported it clean.

Verified against ioBroker.javascript v8.9.2 on a live three-instance system, and
covered by tests against the in-process fake server.

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

Author: mschmicking

AGENTS.md

@@ -31,6 +31,22 @@ instruction from the user:
 4. **Deletion is explicit.** Only `remove`, `rename` and `move` delete anything, each
    requires `--yes`, and each writes the full object JSON to `.iobroker-sync/trash/`
    _before_ deleting. A failed backup aborts the operation.
+
+   The one thing they delete without backing it up is the pair of adapter markers
+   belonging to the script being deleted — `javascript.<n>.scriptEnabled.<id>` and
+   `javascript.<n>.scriptProblem.<id>` — and only after that script is gone. Those are
+   adapter-generated derived state (`common.enabled` and `common.engine` are already in
+   the trash copy), so there is nothing in them to lose. See `cleanUpScriptMarkers` in
+   `commands/remove.ts` for why they have to be swept at all, and
+   `ObjectsApi.deleteScriptMarker` for the ordering rule (value first, object second,
+   never the reverse). The sweep is best-effort: it warns, it never fails the command.
+
+   **Both kinds or neither.** The adapter creates and deletes the two together, so code
+   that handles only `scriptEnabled` cleans up half a mess and reports success. That is
+   not hypothetical: the first version of this sweep shipped that way and left eight
+   orphaned `scriptProblem` states on a live instance while `doctor` called it clean.
+   `MARKER_KINDS` in `types.ts` is the single list; anything iterating markers iterates it.
+
 5. **Copy-then-delete must verify first.** ioBroker has no native rename/move, so both
    are implemented as copy-verify-delete. The verification compares the actual source
    text. Checking only that "something exists at the new id" is not verification — a

README.md

@@ -108,29 +108,29 @@ If `logs` shows nothing, that is usually the adapter's own log level rather than
 
 **Sync**
 
-| Command            | Description                                                                                                                                                             |
-| ------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| `init`             | Write `.iobroker-sync.json`, verify the connection, create the script folder. Asks interactively when run without flags. `--types` also sets up TypeScript definitions. |
-| `types`            | Set up editor intellisense (`log`, `schedule`, ...). `--force`, `--offline`.                                                                                            |
-| `login` / `logout` | Save or remove the password for this instance. Never stored in the project.                                                                                             |
-| `trust`            | Accept the instance's current TLS certificate. Only needed after it changes. `--yes` skips the prompt.                                                                  |
-| `doctor`           | Check config, certificate, login, connection and a live round-trip, and say which one is wrong. Read-only, never prompts. Run this first when something looks broken.   |
-| `pull [pattern]`   | Download scripts to disk. Never deletes or overwrites local files.                                                                                                      |
-| `push [pattern]`   | Upload locally modified scripts. Never deletes remote objects.                                                                                                          |
-| `status`           | Show what changed, locally and remotely.                                                                                                                                |
-| `diff [pattern]`   | Unified diff of local vs server. `--against <snapshot>` compares with a backup instead.                                                                                 |
-| `watch`            | Push on save. `--pull` also applies remote changes.                                                                                                                     |
-| `logs [pattern]`   | Stream the server log. `--level`, `--limit`. Read-only.                                                                                                                 |
-| `backup [pattern]` | Snapshot every script — source _and_ full object — to `.iobroker-sync/backup/<timestamp>/`. Read-only against the server.                                               |
+| Command            | Description                                                                                                                                                                                     |
+| ------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `init`             | Write `.iobroker-sync.json`, verify the connection, create the script folder. Asks interactively when run without flags. `--types` also sets up TypeScript definitions.                         |
+| `types`            | Set up editor intellisense (`log`, `schedule`, ...). `--force`, `--offline`.                                                                                                                    |
+| `login` / `logout` | Save or remove the password for this instance. Never stored in the project.                                                                                                                     |
+| `trust`            | Accept the instance's current TLS certificate. Only needed after it changes. `--yes` skips the prompt.                                                                                          |
+| `doctor`           | Check config, certificate, login, connection, a live round-trip and leftover adapter markers, and say which one is wrong. Read-only, never prompts. Run this first when something looks broken. |
+| `pull [pattern]`   | Download scripts to disk. Never deletes or overwrites local files.                                                                                                                              |
+| `push [pattern]`   | Upload locally modified scripts. Never deletes remote objects.                                                                                                                                  |
+| `status`           | Show what changed, locally and remotely.                                                                                                                                                        |
+| `diff [pattern]`   | Unified diff of local vs server. `--against <snapshot>` compares with a backup instead.                                                                                                         |
+| `watch`            | Push on save. `--pull` also applies remote changes.                                                                                                                                             |
+| `logs [pattern]`   | Stream the server log. `--level`, `--limit`. Read-only.                                                                                                                                         |
+| `backup [pattern]` | Snapshot every script — source _and_ full object — to `.iobroker-sync/backup/<timestamp>/`. Read-only against the server.                                                                       |
 
 **Lifecycle**
 
-| Command                                  | Description                                                                                                       |
-| ---------------------------------------- | ----------------------------------------------------------------------------------------------------------------- |
-| `list`                                   | All scripts with instance and enabled state.                                                                      |
-| `start` / `stop` / `restart` `<pattern>` | Toggle `common.enabled`.                                                                                          |
-| `new <path>`                             | Create a new script (disabled) plus any missing folders.                                                          |
-| `rename` / `move` / `remove`             | Destructive. Require `--yes` and back up the object first. `remove` keeps the local file unless `--delete-local`. |
+| Command                                  | Description                                                                                                                                                                                                                                 |
+| ---------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `list`                                   | All scripts with instance and enabled state.                                                                                                                                                                                                |
+| `start` / `stop` / `restart` `<pattern>` | Toggle `common.enabled`.                                                                                                                                                                                                                    |
+| `new <path>`                             | Create a new script (disabled) plus any missing folders.                                                                                                                                                                                    |
+| `rename` / `move` / `remove`             | Destructive. Require `--yes` and back up the object first. `remove` keeps the local file unless `--delete-local`. All three also clean up the `scriptEnabled`/`scriptProblem` states the old id leaves behind on every javascript instance. |
 
 `--dry-run`, `--verbose`, `--json` and `-C <dir>` are global and work with every command.
 When in doubt, `--dry-run` shows what would happen and changes nothing.

docs/TROUBLESHOOTING.md

@@ -3,9 +3,9 @@
 Things that look like bugs and are not. See the [README](../README.md) for the overview.
 
 **Start with `iob-sync doctor`.** It checks the config, the certificate, the login, the
-connection and a live round-trip, and names the one that is wrong — including the two
-cases below, which are the ones that reliably send people down the wrong path. It is
-read-only and never prompts, so it is safe to run at any time.
+connection, a live round-trip and the adapter's leftover script markers, and names the one
+that is wrong — including the cases below, which are the ones that reliably send people
+down the wrong path. It is read-only and never prompts, so it is safe to run at any time.
 
 ## Commands time out, but the connection "works"
 
@@ -86,6 +86,48 @@ a script or move it to another javascript instance, it cannot — that is delibe
 sync bug cannot stop a running script. Use `start` / `stop` for `enabled`; instance moves
 must be done in Admin.
 
+## `doctor` warns about orphaned markers
+
+Nothing is broken, and no script is affected.
+
+The javascript adapter keeps two bookkeeping states beside every script, on **every**
+javascript instance — not only the one that runs it:
+
+```
+javascript.<n>.scriptEnabled.<script id>
+javascript.<n>.scriptProblem.<script id>
+```
+
+Both are created by the adapter's `load()`, which calls `createActiveObject` and
+`createProblemObject` _before_ `prepareScript` checks `common.engine` to decide whether
+this instance should actually run the script. Every instance runs `load()` for every
+non-global script at startup, and again on every source change. So on a three-instance
+system, ten scripts mean sixty of these states, and that is normal.
+
+Deletion, however, _is_ gated on the engine: only the instance that owned the script at
+the moment it was deleted removes its own pair. The pairs on the other instances stay for
+the life of the system, js-controller complains about them, and nothing in ioBroker ever
+collects them.
+
+This has nothing to do with who did the deleting — the Admin UI leaves exactly the same
+residue. Verified against ioBroker.javascript v8.9.2.
+
+`iob-sync doctor` lists them by id, both kinds. To clear one script's leftovers:
+
+```bash
+iob-sync remove script.js.diag.retired-check --yes
+```
+
+`remove` sweeps the markers even when the script itself is already gone from the server —
+in that case it deletes nothing else and leaves your local file alone. `rename` and `move`
+sweep the old id's markers as they go, so this does not accumulate from normal use.
+
+One detail worth knowing if you clean these up by hand: delete the **state value first,
+then the object**. The reverse order leaves a value with no object behind it, which is the
+shape js-controller actually warns about — and it is the order the adapter's own cleanup
+uses (`delObject` then `delState`), so that warning may well have come from ioBroker
+itself rather than from anything you did.
+
 ## A push is refused as a conflict
 
 Local and remote both changed since the last sync. Inspect with `iob-sync diff`, then

src/client/objects.ts

@@ -6,9 +6,12 @@
 import {
   FolderObject,
   IoBrokerObject,
+  MARKER_KINDS,
+  MarkerKind,
   ObjectsApi,
   ObjectViewResult,
   ScriptCommon,
+  ScriptMarkerEntry,
   ScriptObject,
   SocketClient,
 } from '../types';
@@ -17,6 +20,43 @@ const SCRIPT_NAMESPACE = 'script.js.';
 // Verified endkey used by the Admin UI itself to bound a getObjectView range scan.
 const VIEW_ENDKEY = `${SCRIPT_NAMESPACE}香`;
 
+const JS_NAMESPACE = 'javascript.';
+
+/** `javascript.*.<kind>.*` — every instance's markers of one kind, for every script. */
+function markerPattern(kind: MarkerKind): string {
+  return `${JS_NAMESPACE}*.${kind}.*`;
+}
+
+/**
+ * Splits a marker id into the script it belongs to and which kind it is:
+ * `javascript.2.scriptEnabled.common.garage` -> `script.js.common.garage`, scriptEnabled.
+ *
+ * Returns null for anything that is not one — including ids with something other than
+ * a bare instance number in front of the kind, so that a state someone else parked
+ * under `javascript.` cannot be mistaken for ours and deleted.
+ */
+export function parseMarkerId(stateId: string): { scriptId: string; kind: MarkerKind } | null {
+  if (!stateId.startsWith(JS_NAMESPACE)) {
+    return null;
+  }
+  for (const kind of MARKER_KINDS) {
+    const infix = `.${kind}.`;
+    const kindAt = stateId.indexOf(infix);
+    if (kindAt === -1) {
+      continue;
+    }
+    const instance = stateId.slice(JS_NAMESPACE.length, kindAt);
+    if (!/^\d+$/.test(instance)) {
+      continue;
+    }
+    const suffix = stateId.slice(kindAt + infix.length);
+    if (suffix) {
+      return { scriptId: `${SCRIPT_NAMESPACE}${suffix}`, kind };
+    }
+  }
+  return null;
+}
+
 export class AdminObjectsApi implements ObjectsApi {
   constructor(private readonly socket: SocketClient) {}
 
@@ -111,4 +151,89 @@ export class AdminObjectsApi implements ObjectsApi {
   async deleteObject(id: string): Promise<void> {
     await this.socket.emit('delObject', [id]);
   }
+
+  /**
+   * Reads the value side and the object side separately and unions them, because a
+   * marker can exist as either half alone and the halves are what we need to tell apart.
+   *
+   * One half failing is tolerated: `getStates` and `getForeignObjects` are separate
+   * commands with separate histories across Admin versions, and a partial answer here
+   * still beats reporting nothing. Only a total failure propagates.
+   */
+  async listScriptMarkers(): Promise<ScriptMarkerEntry[]> {
+    const perKind = await Promise.all(MARKER_KINDS.map((kind) => this.listMarkersOfKind(kind)));
+    return perKind.flat().sort((a, b) => (a.id < b.id ? -1 : a.id > b.id ? 1 : 0));
+  }
+
+  private async listMarkersOfKind(kind: MarkerKind): Promise<ScriptMarkerEntry[]> {
+    const pattern = markerPattern(kind);
+    const [values, objects] = await Promise.allSettled([
+      this.readMarkerValues(pattern),
+      this.socket.emit<Record<string, unknown> | null>('getForeignObjects', [pattern, 'state']),
+    ]);
+
+    if (values.status === 'rejected' && objects.status === 'rejected') {
+      throw values.reason;
+    }
+
+    const valueIds = new Set(values.status === 'fulfilled' ? Object.keys(values.value ?? {}) : []);
+    const objectIds = new Set(
+      objects.status === 'fulfilled' ? Object.keys(objects.value ?? {}) : [],
+    );
+
+    const entries: ScriptMarkerEntry[] = [];
+    for (const id of new Set([...valueIds, ...objectIds])) {
+      const parsed = parseMarkerId(id);
+      // The pattern is a wildcard match, so it can catch ids that merely look the part
+      // (`javascript.0.scriptEnabled` with no suffix, or a non-numeric instance).
+      // parseMarkerId is the authority on what is really ours.
+      if (parsed?.kind !== kind) {
+        continue;
+      }
+      entries.push({
+        id,
+        scriptId: parsed.scriptId,
+        kind,
+        hasValue: valueIds.has(id),
+        hasObject: objectIds.has(id),
+      });
+    }
+    return entries;
+  }
+
+  /**
+   * `getStates` is the current command; `getForeignStates` is its deprecated alias and
+   * the only one older Admin builds answer. Trying both costs one extra round trip on
+   * instances that need it and nothing on instances that do not.
+   */
+  private async readMarkerValues(pattern: string): Promise<Record<string, unknown> | null> {
+    try {
+      return await this.socket.emit<Record<string, unknown> | null>('getStates', [pattern]);
+    } catch {
+      return this.socket.emit<Record<string, unknown> | null>('getForeignStates', [pattern]);
+    }
+  }
+
+  async deleteScriptMarker(entry: ScriptMarkerEntry): Promise<void> {
+    if (!parseMarkerId(entry.id)) {
+      // Belt and braces: this API is reachable from command code, and the whole point
+      // of it is deleting things, so it refuses anything outside its own namespace.
+      throw new Error(`Refusing to delete "${entry.id}": not a script marker.`);
+    }
+
+    // Value first. If this throws we stop here on purpose — deleting the object while
+    // the value survives would manufacture the exact orphan this code exists to remove.
+    if (entry.hasValue) {
+      await this.socket.emit('delState', [entry.id]);
+    }
+
+    if (entry.hasObject) {
+      // Admin's delState is documented to take the object with it. Verify rather than
+      // assume: on an instance where it does not, the object has to go separately.
+      const remaining = await this.socket.emit<IoBrokerObject | null>('getObject', [entry.id]);
+      if (remaining) {
+        await this.socket.emit('delObject', [entry.id]);
+      }
+    }
+  }
 }