feat(cli): sweep the adapter markers a deleted script leaves behind - #30
Merged
Conversation
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>
mschmicking
force-pushed
the
feat/scriptenabled-cleanup
branch
from
August 17, 2026 21:42
1d24564 to
de9d34e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
remove,renameandmovenow delete thescriptEnabledandscriptProblemstates the javascript adapter leaves behind for a script that no longer exists.doctorgains a read-onlymarkerscheck that finds pre-existing ones.Why
The adapter keeps two bookkeeping states beside every script, on every javascript instance rather than only the one that runs it:
Both are created by
load(), which callscreateActiveObject/createProblemObjectbeforeprepareScriptcheckscommon.engine. Every instance runsload()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. Verified against ioBroker.javascript v8.9.2.
Safety
ObjectsApi.deleteScriptMarker— value first, object second, never the reverse, since an object deleted out from under a surviving value is exactly the orphan being cleaned up. (The adapter's own cleanup gets this backwards, a likely source of "state has no object" warnings.)--yesand respects--dry-run.AGENTS.mdinvariant 4 records the one thing swept without a backup, and why there is nothing in it to lose.removealso stops refusing an id whose script is already gone when markers remain — otherwise pre-existing ones cannot be cleared by anything. It sweeps them, touches nothing else, and keeps the local file.Testing
doctorstill counts an orphanedscriptProblemwhosescriptEnabledtwin is already gone.removewith the script present and with it already gone, plusrenameandmove. Each result was verified with a raw states-database probe that shares no code with the tool. Ten orphaned states were cleared in the process — six that predated the work, four created by the tests themselves — leaving that instance at zero.Both marker kinds are driven by
MARKER_KINDSintypes.tsrather than a pattern repeated per call site — an earlier draft covered onlyscriptEnabledand left eight orphanedscriptProblemstates on that live instance whiledoctorreported it clean.Note on the CodeQL comments above
The two resolved
github-advanced-securitycomments ontest/fake-server.tscome from the first push of this branch, which was cut from a stale localmainand therefore lacked thedeepMergeprototype guard added in #25. The branch has since been rebased and merged up; both alerts are recorded as fixed, and the guard is present in the current file.🤖 Generated with Claude Code