refactor(zwave): extract driver lifecycle#4734
Draft
AlCalzone wants to merge 33 commits into
Draft
Conversation
Coverage Report for CI Build 29576688468Warning No base build found for commit Coverage: 58.418%Details
Uncovered Changes
Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
27 tasks
AlCalzone
force-pushed
the
alcalzone-backend-refactor-firmware-inclusion
branch
from
July 16, 2026 17:48
d118193 to
6ab4b1f
Compare
AlCalzone
force-pushed
the
alcalzone-backend-refactor-driver-lifecycle
branch
from
July 16, 2026 19:43
b0baa84 to
d16cc37
Compare
AlCalzone
force-pushed
the
alcalzone-backend-refactor-firmware-inclusion
branch
from
July 17, 2026 06:21
6ab4b1f to
a6ea24f
Compare
AlCalzone
force-pushed
the
alcalzone-backend-refactor-driver-lifecycle
branch
from
July 17, 2026 06:43
d16cc37 to
b434a8c
Compare
AlCalzone
force-pushed
the
alcalzone-backend-refactor-firmware-inclusion
branch
from
July 17, 2026 11:08
a6ea24f to
3e9dd49
Compare
Move the SensorTypeScale, ZwaveConfig and ZwaveClientStatus shapes into api/lib/zwave/ports.ts so extracted services (DriverLifecycle) can name the client configuration, status and sensor-scale types without importing the monolithic ZwaveClient (which would create an import cycle). ZwaveClient re-exports them in a follow-up commit, so its public type surface is unchanged for every existing importer. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Extract driver instance/generation ownership out of ZwaveClient into a new strict-clean DriverLifecycle service (api/lib/zwave/DriverLifecycle.ts). It owns driver creation/options/bindings/log transports, connect/start, event-listener install/remove, driver-ready/all-nodes-ready/bootloader/OTW handoff, retry/backoff/restart timers, close/destroy, status/error transitions, statistics, user-callback install state, and @zwave-js/server attach/create/start/destroy coordination (delegating to the Layer 8 ZwaveServerManager, no duplicate server lifecycle). ZwaveClient keeps driver, driverReady, status/error/info/config getters and every lifecycle/API method as thin compatibility facades that delegate to the service; node-registry driver-event bodies (_onDriverReady/_onDriverError/_onScanComplete/_onBootLoaderReady/OTW) stay in ZwaveClient and are reached through a narrow DriverLifecycleHost port of lazy closures, so DriverLifecycle never imports ZwaveClient (no cycle, static imports only). A monotonic generation minted in connect() and bumped in close() fences late driver-ready/error/timer callbacks so an obsolete generation can never mutate replacement state. Exact ordering, statuses, events, log text, timing, retry limits/delays, soft-reset/watchdog, RF/security/config options and server defaults are preserved; server destroy still precedes driver destruction during close. Strict diagnostics: ZwaveClient 174 -> 160, DriverLifecycle 0 (strict-clean); overall audit 224 -> 210. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add 60 direct state-machine tests for DriverLifecycle covering success/ready/all-nodes/bootloader, errors, retry/backoff/restart, connect/close generation fencing, late callbacks on a replaced generation, partial-start cleanup, options/settings merges, statistics, user callbacks, @zwave-js/server ordering, listener install/remove counts, and idempotent close, using deterministic deferred/fake-clock harnesses. Coverage of DriverLifecycle.ts: 100% statements / 91.55% branches / 100% functions / 100% lines. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…nd log-config overrides Address three lifecycle-ownership defects in DriverLifecycle: - connect() retains the exact Driver instance each generation creates and tears down that specific instance on every stale/error exit, clearing the host driver field only when it still references that instance. A superseded connect whose start() rejects after a replacement took over can no longer destroy the live replacement, and a stale-generation success no longer leaks its own driver. - backoffRestart() coalesces repeated schedules into a single pending restart (latest delay wins) and generation-fences the callback, so an overwritten timer handle can no longer leak and fire an extra restart, and a close() or healthy replacement neutralizes a still-pending restart. - the log transports/level are resolved against the FINAL logConfig the driver receives (after Object.assign(cfg.options) and external settings), so a documented zwave.options.logConfig override can no longer drop the required JSON transport or registered extra transports. Adds deterministic tests (overlapping-connect resolve/reject, backoff coalescing/fencing, final logConfig override) and raises the DriverLifecycle coverage floors to lock in the added coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…tart An in-flight checkForConfigUpdates() could finish after a close()/restart (or driver replacement) and re-arm the ~24h daily-check setTimeout, leaving a stale timer that outlives its driver or duplicates the replacement generation's own scheduled check. _scheduledConfigCheck() now takes the DriverLifecycle generation active when it was scheduled and, after the await, re-validates that generation plus the closed/destroyed state before logging or re-arming. The daily re-arm carries the same token forward so the chain keeps fencing. Adds deterministic deferred/fake-clock tests for completion after close, after generation replacement and after destroy, plus positive controls that a normal completion re-arms exactly once and threads the original token. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…driver A second connect() that lands after driver.start() has resolved (status CONNECTED) but before the `driver ready` event — or while the first connect is still awaiting start() — passed the ready-only guard, minted a new generation and constructed a REPLACEMENT driver, overwriting the host driver field. The first, still-live but not-yet-ready driver was then stranded: unreachable, never torn down, and still holding the serial port. connect() now coalesces onto the in-flight connect while a driver is already owned by the host (getDriver() non-null), returning without constructing a replacement. Public connect timing is unchanged on the common no-driver-yet path. Legitimate reconnects (config change, backoff retry, hard reset) go through close()/restart(), which destroys the current driver and nulls the host field before reconnecting, so they are never blocked. The prior-round teardownConnectDriver ownership guarantee is retained as defense-in-depth for close-during-inflight/stale starts. Tests rewrite the overlapping-connect block into pre-ready reconnect coalescing: a second connect after CONNECTED-but-not-ready and while still awaiting start() constructs no replacement and leaks nothing (exact Driver construction count, retained host instance, unchanged generation, zero destroys); recovery after start failure is NOT coalesced (host cleared); and a stale start that resolves/rejects after a close+reconnect tears down only its own driver, never the live replacement, with no stale backoff. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Public hardReset() runs driver.hardReset() then init(), and the driver re-emits `driver ready` on the SAME DriverLifecycle generation by design (so _onDriverReady runs again). The generation therefore cannot fence the daily config-update-check chain across a hard reset: an already-armed or in-flight check stayed generation-valid, the next `driver ready` started a second chain, the updatesCheckTimeout handle was overwritten and the old chain became untracked. Repeated init()/hardReset() could accumulate chains and leak ~24h timers. Introduce a dedicated config-check scheduling epoch. _invalidateScheduled ConfigCheck() bumps the epoch and clears the armed daily timer, and is called from every boundary that (re)starts the driver-ready chain: init() (covering construction, restart() and hardReset()) and the close path (_clearRuntimeOnClose, covering close and restart's pre-init close). _scheduledConfigCheck now captures the epoch alongside the generation and, after its awaited driver round-trip, only logs/re-arms while both are still current; the rearm threads both tokens forward. _onDriverReady captures the current epoch when scheduling. Deterministic fake-timer + deferred tests through public hardReset() and repeated init(): hardReset bumps the epoch and clears the already-armed timer (exact clear count, generation unchanged); an in-flight check that predates the reset is epoch-fenced even though the generation is identical (proving the epoch, not the generation, does the fencing); repeated init() never keeps more than one live chain (distinct handles, each cleared); and the post-reset chain still re-arms exactly once under the new epoch. The existing generation-fencing tests are updated to thread the epoch. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Object.assign(zwaveOptions, cfg.options) aliases the driver's logConfig onto the user's persisted cfg.options.logConfig whenever a documented zwave.options.logConfig override is present. The runtime enrichment that follows then mutated that persisted object in place: applyExternalDriver Settings() set filename/forceConsole, and the log block injected the JSON transport plus every registered extra transport and RAISED the effective level. Because the level was only ever raised, adding a temporary debug transport permanently overwrote the user's configured level, so removing the transport and restarting could never return to it. Shallow-clone zwaveOptions.logConfig into a driver-only object immediately after the Object.assign, before any enrichment. A shallow copy is sufficient because every enrichment step REPLACES a top-level property (filename, forceConsole, transports, level) rather than mutating a nested value, so the source object keeps its identity and content while the driver receives a distinct, fully enriched copy. Tests assert the persisted cfg.options.logConfig keeps its reference and exact deep content across connect, extra-transport add/remove and close/ restart, while the driver's options receive a distinct object carrying the required JSON transport, the current extras and the effective level; and that removing a debug transport then restarting returns the driver to the configured level with no leftover runtime transport objects. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Three cohesive fixes to the DriverLifecycle ownership/logging state machine.
HIGH failed connects lose teardown ownership: teardownConnectDriver is now async and its destroy() is AWAITED before any host state is cleared, so a failed/stale driver releases the serial port before backoff/retry or a coalesced connect can build a replacement. A rejected destroy retains the exact instance as owner (host field not cleared, no replacement) and stays retryable via a later close()/retry; a clean destroy clears the host field only when it still references that exact instance. Awaited at all three stale/error exits.
MEDIUM duplicate connect coalescing settlement: an in-flight startup is tracked as _activeConnect (+_activeConnectGeneration). A duplicate connect for the current generation joins that exact promise instead of returning a premature success, so concurrent callers share the first caller's settlement timing and error semantics and no second Driver is constructed. After a startup settles but before ready, the kept compat guard still returns without building a replacement. close() clears _activeConnect so a later connect builds a fresh driver rather than joining a superseded startup.
MEDIUM live add/remove log transports: updateLogConfig({transports}) replaces the driver's whole custom transport list in zwave-js 15.25.x. computeRuntimeLogTransports/computeRuntimeLogLevel/applyRuntimeLogConfig rebuild the COMPLETE list (required JSON-socket transport first + every extra) and recompute the level from the immutable configured baseline (_baseLogLevel) on every add/remove, so removal restores the baseline and never drops the JSON socket. connect() captures the baseline/transport for the active generation; the persisted cfg.options.logConfig is never mutated.
connect() extracts _startDriver(cfg, port) so the facade can track the startup promise; the pre-narrowed port keeps DriverLifecycle strict-clean. The DriverLifecycle suite (83 deterministic tests, fake clock/deferred barriers) proves no replacement Driver is built while an old owner is torn down, a rejected destroy stays owned/retryable, coalesced callers share settlement, and the complete transport arrays/levels across add/remove/restart.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ains
NVM restore and controller firmware updates re-emit 'driver ready' on the SAME driver generation with no intervening lifecycle boundary, so the existing generation+epoch fencing could not stop a second ready from arming a duplicate ~24h config-update-check timer: the later arm overwrote and leaked the earlier handle, stacking parallel daily chains forever.
_onDriverReady now starts each ready through _startScheduledConfigCheck(), which clears any armed timer and bumps a monotonic _configCheckChain token BEFORE scheduling, so at most one chain is ever armed/in-flight. The awaited check re-validates the chain token (alongside generation/epoch/closed/destroyed) after its driver round-trip, so a still-in-flight previous chain bails instead of re-arming. _clearConfigCheckTimer() centralises the timer teardown shared by lifecycle invalidation and a fresh chain. The epoch's lifecycle-boundary semantics are preserved unchanged.
FirmwareUpdateService carried the identical same-generation 'all nodes ready' pattern: scheduledFirmwareUpdateCheck() now clearScheduledCheck()s and bumps a _scheduleChain token on entry and fences the post-await rearm on it, so repeated same-generation ready events keep a single daily chain.
Adds real event-dispatch regressions (driver.emit('driver ready'), never stubbing _scheduledConfigCheck): repeated ready keeps exactly one armed timer, a superseded in-flight chain cannot rearm, and the firmware mirror. Updates prior-layer hass/server tests to the 3-arg _scheduledConfigCheck.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Audit every comment the driver-lifecycle refactor added or changed in ZwaveClient.ts, DriverLifecycle.ts, FirmwareUpdateService.ts and ports.ts against the project comment style: drop step narration, changelog/history ("Layer 12 moved…"), decorative dividers/section banners, failure-scenario openers and language/API restatements, and collapse long JSDoc on self-evident signatures.
Keep only the non-obvious Z-Wave domain and concurrency reasons, compressed to one positive sentence next to the code they explain: same-generation driver-ready re-fire (NVM restore / controller firmware) needing epoch+chain fencing, teardown ownership of the exact created driver before backoff, monotonic generation fencing of obsolete callbacks, updateLogConfig replacing the whole transport list, and cloning logConfig before enrichment. DriverLifecycle.ts drops from 337 to 98 comment lines. Verbatim comments moved from base ZwaveClient.ts are left untouched to avoid churn.
The two vitest coverage configs get the same treatment on their ZwaveClient.ts threshold rationale (history removed, current measured baseline stated); all coverage floors are unchanged. No code changes: the diff is comment/blank-only and type/test/coverage output is byte-identical to the prior head.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sweep the driver-lifecycle test comments added by this refactor (DriverLifecycle.test.ts, hass/server.test.ts, socket/zwaveClientProductionIntegration.test.ts): delete numbered section banners and divider bars, per-test setup/assertion narration, trailing inline notes that restate the code, and JSDoc that just repeats a helper's name. Retain only the few non-obvious domain notes, one positive sentence each: driver-ready/all-nodes-ready re-firing on the same generation so generation alone cannot dedupe, hardReset re-emitting driver-ready on the same generation so a scheduling epoch (not generation) fences the config-check chain, the per-chain token covering the case the epoch cannot, and the socket.io timer-count pollution that forces handle-identity assertions. No test logic changes: assertions, describe/it structure and counts are unchanged (DriverLifecycle 83 it, hass/server 33 it, integration 46 it) and every touched suite still passes; the diff is comment/blank-only. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Re-audit every added/changed comment on this branch against the repo comment style: prefer deletion, keep only non-obvious why/domain notes, one sentence with no trailing period, and remove all test-harness/fixture/helper narration. Comment- and blank-only; executable code and test structure are unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Route the Driver, JSON log transport and ZwaveServerManager constructions plus ensureDir through a constructor-injected DriverLifecycleDeps object with production defaults, so the composition root and tests supply typed builders instead of relying on module-level mocking. Log transports are typed against the real winston Transport/LogConfig surfaces. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Replace the four vi.mock module seams, vi.hoisted factory and dynamic import with a static import plus constructor-injected typed fakes recorded through a per-harness world. Assertions now target observable connect/start/ready/retry/restart/close/server/log/status/event outcomes; delegation-identity, getter-delegation, source-shape and fake-cleanup tests are dropped. Casts are confined to the injection boundary. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Apply the accepted #4724 review guardrails to this layer's own driver-lifecycle tests. - Rename DriverLifecycle describe/it blocks off implementation labels (edge, coverage, catch-path) onto the observable lifecycle behavior they assert. - Type the injected fake driver and server-manager methods with vitest's real Mock instead of a ReturnType<typeof vi.fn> alias. - Drop the finding label and count-focused phrasing from the repeated driver-ready config-check tests, keeping the one-active-scheduled-check assertions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Re-audit this layer's own DriverLifecycle tests against the accepted #4724 guardrails the prior pass missed. Rename describe/it blocks off implementation phrasing (generation fencing, teardown ownership, option wiring, uppercase emphasis and explanatory parentheticals) onto the observable connect/close/retry/restart/log lifecycle they assert. Delete four tests whose subject is implementation, not documented behavior: the driver-event catalog that enumerated every event->callback mapping; the raw-error quirk asserting a swallowed driver.destroy() rejection (already covered by the generic start-failure and retryable-cleanup tests); and the two swallowed-restart/close logging tests (internal logging, no public guarantee). Also drop the injection/fake narration comments. Retained public regressions are unchanged: concurrent connect callers settle together, a stale close cannot destroy a replacement, failed owner cleanup blocks replacement and can retry via close, retry-timer cancellation, the complete log-transport set, immutable persisted config, and server-before-driver teardown. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Pruning the lifecycle tests left flush = () => Promise.resolve() with one caller, so inline it as await Promise.resolve() to keep no test-only helper mechanic in this layer's delta; behavior is identical. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The parent stack now defines the #api/* subpath import, so the driver-lifecycle suite reaches into api/ through the same canonical alias as every other backend test instead of a deep relative path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Drop the suite's local Deferred helper in favour of the parent stack's shared createDeferred from serviceTestSupport, so the injected driver start/destroy doubles resolve through the same deferred utility as the other zwave service suites. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…se extra is removed buildLogConfig encodes a configured level as an npm numeric rank, but the runtime baseline was only captured when the level was a string, so the common configured case captured no baseline. Combined with applyRuntimeLogConfig omitting the level once no extra transport set it, and updateLogConfig merging partial updates, removing the last verbose extra left the driver stuck at the elevated level instead of returning to the configured/default level. Capture the baseline as a level name via a new utils.logLevelName helper (defaulting to the exported CONTROLLER_LOGLEVEL when the driver config carries no level), rank levels through utils.logLevelRank on the same npm scale buildLogConfig uses, and always resend the recomputed level so the effective level tracks the configured/default baseline. The persisted logConfig stays untouched. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…fecycle Add deterministic tests, driven through the constructor-injected typed fakes, for the runtime log-level baseline: a numeric configured baseline is restored when the last verbose extra is removed, an omitted baseline returns to the documented default level, and a configured string baseline is preserved across an add and removal. Update the no-logConfig case to assert the corrected behaviour where a less-verbose extra is clamped to the documented default rather than lowering the level. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Catch driver-ready initialization failures without leaking unhandled rejections, fence stale failures, and normalize unknown teardown errors so cleanup remains retryable. Keep the lifecycle tests on structural fakes and observable synchronization while covering both failure paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep Layer 11's restore-callback persistence boundary and remove replayed tests for the obsolete competing store transaction model. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reuse one destroy attempt when close overlaps an in-flight startup, and wait deterministically for scheduled config checks in the facade integration test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve readiness Error identity after the squash parent removed the shared toError export. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
AlCalzone
force-pushed
the
alcalzone-backend-refactor-driver-lifecycle
branch
from
July 17, 2026 11:23
b434a8c to
77c5432
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
2 participants
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.
Extracts driver creation, readiness, retry, restart, and teardown orchestration.
ZwaveClientas the lifecycle facade and preserves Z-Wave server startup ordering.Part of #4722. Stacked on #4733.