Skip to content

refactor(api): extract runtime and http routers#4727

Open
AlCalzone wants to merge 47 commits into
backend-refactorfrom
alcalzone-backend-refactor-http-runtime
Open

refactor(api): extract runtime and http routers#4727
AlCalzone wants to merge 47 commits into
backend-refactorfrom
alcalzone-backend-refactor-http-runtime

Conversation

@AlCalzone

@AlCalzone AlCalzone commented Jul 11, 2026

Copy link
Copy Markdown
Member

Separates application lifecycle and HTTP routing from app.ts.

  • Introduces AppRuntime and focused routers while keeping the existing endpoints and middleware contracts.
  • Makes restart generations and live service ownership explicit.
  • Review focus: stale service capture, restart behavior, and teardown ordering.

Part of #4722. Stacked on #4726.

@coveralls

coveralls commented Jul 11, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 29606237710

Warning

No base build found for commit fa754e2 on backend-refactor.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 40.268%

Details

  • Patch coverage: 76 uncovered changes across 8 files (671 of 747 lines covered, 89.83%).

Uncovered Changes

File Changed Covered %
api/app.ts 116 80 68.97%
api/runtime/AppRuntime.ts 99 91 91.92%
api/routes/configurationTemplates.ts 62 55 88.71%
api/routes/settings.ts 145 138 95.17%
api/routes/store.ts 136 129 94.85%
api/lib/BackupManager.ts 5 0 0.0%
api/routes/auth.ts 82 77 93.9%
api/config/app.ts 1 0 0.0%
Total (13 files) 747 671 89.83%

Coverage Regressions

Requires a base build to compare against. How to fix this →


Coverage Stats

Coverage Status
Relevant Lines: 6780
Covered Lines: 2917
Line Coverage: 43.02%
Relevant Branches: 4338
Covered Branches: 1560
Branch Coverage: 35.96%
Branches in Coverage %: Yes
Coverage Strength: 31.31 hits per line

💛 - Coveralls

@AlCalzone
AlCalzone marked this pull request as draft July 11, 2026 21:11
@AlCalzone
AlCalzone force-pushed the alcalzone-backend-refactor-strict-foundations branch from 54feffd to c1cc0ff Compare July 15, 2026 15:29
@AlCalzone
AlCalzone force-pushed the alcalzone-backend-refactor-http-runtime branch 2 times, most recently from 1f8cc4d to 774c2e5 Compare July 16, 2026 05:06
@AlCalzone
AlCalzone force-pushed the alcalzone-backend-refactor-strict-foundations branch from c1cc0ff to e1abc66 Compare July 16, 2026 11:39
@AlCalzone
AlCalzone force-pushed the alcalzone-backend-refactor-http-runtime branch from fbbb9b9 to f83b9ad Compare July 16, 2026 11:54
@AlCalzone
AlCalzone force-pushed the alcalzone-backend-refactor-strict-foundations branch from e1abc66 to dff5ea0 Compare July 16, 2026 18:26
@AlCalzone
AlCalzone force-pushed the alcalzone-backend-refactor-http-runtime branch from f83b9ad to aa5a974 Compare July 16, 2026 18:48
Base automatically changed from alcalzone-backend-refactor-strict-foundations to backend-refactor July 17, 2026 07:36
AlCalzone and others added 18 commits July 17, 2026 09:42
Layer 5 of issue #4722: split api/app.ts's monolithic Express setup into
a typed runtime owner plus focused per-domain route modules, with zero
behavior change.

api/runtime/AppRuntime.ts
- New AppRuntime class owning every backend collaborator whose identity
  or presence changes across the process's lifetime: the live Gateway,
  the live ZnifferManager, dynamically-loaded plugins and their mount
  router, in-progress-restart state, and the (test-replaceable) serial
  port enumerator - plus read-through access to the backup/debug manager
  singletons, persisted settings, and bootstrapped snippets.
- Every accessor resolves the CURRENT value on each call - nothing is
  captured once and cached in a closure - so a gateway/zniffer replaced
  mid-restart is immediately visible to the very next call from any
  consumer (HTTP route handler, Socket.IO handler, etc.). This is the
  requirement the extraction exists to satisfy: no stale captures across
  a gateway swap or restart.
- Absence is explicit: `getGateway()`/`getZniffer()` return `T |
  undefined`; `requireGateway()`/`requireZniffer()` return `T` and are
  the single, centralized, heavily-documented exception preserving a
  pre-existing quirk (several call sites have always read
  `gw.zwave`/`gw.close()` etc. with no presence guard, surfacing a bare
  TypeError when no gateway is attached - honestly typing every one of
  those call sites would either change that behavior or require a
  non-null assertion at each site; one documented assertion here is
  narrower than that).
- `startGateway()`/`startZniffer()`/`shutdown()` carry forward the
  exact startup/restart/shutdown ordering and test seams app.ts already
  had (SESSION_SECRET warning, plugin loading/teardown, guarded gateway
  close).

api/routes/{auth,health,settings,importExport,configurationTemplates,
store,debug}.ts
- Extracted all 35 explicit HTTP routes app.ts registered directly,
  grouped by domain. Each module exports a `registerXRoutes(app,
  runtime, deps)` factory called once from app.ts, in the same order
  the routes were originally registered.
- Method, path, middleware order (rate limiter/auth placement),
  request parsing/coercion, response shape/status/content-type, and
  side effects are preserved byte-for-byte. In particular:
  - HTTP-200-with-`success:false` failure/rate-limit envelopes,
  - the invalid `/health/:client` fallthrough,
  - the backup ZIP's JSON content type,
  - missing-gateway TypeErrors (via `runtime.requireGateway()`),
  - the non-multipart upload path,
  - password/session handling quirks,
  - the plugin router's unauthenticated placement,
  - and every previously-omitted response field.
- All 35 routes resolve the gateway/zniffer/settings through `runtime`
  per request, so they automatically inherit the fresh-resolution
  guarantee above.

api/app.ts (2574 -> 851 lines)
- Now constructs the single `AppRuntime`, wires the 7 route modules in,
  and keeps everything not yet in scope for this layer as-is: app/
  middleware/static/history setup, Socket.IO wiring, plugin dynamic
  loading and its unauthenticated mount placement, and process-lifecycle
  (startup/restart/shutdown) sequencing - composition and ordering are
  unchanged, only the route bodies moved out.

api/config/app.ts
- Added `sslDisabled()`, reading `FORCE_DISABLE_SSL` fresh from
  `process.env` on every call instead of a module-load-time constant,
  so both app.ts and the new settings route observe the same live
  value (needed once the settings route lives outside app.ts).

Strict-mode audit (tsc --noEmit --strict -p tsconfig.json, advisory
only - tsconfig.json's real `strict` flag stays `false` pending a later
layer): 500 -> 452 errors total. app.ts 61 -> 13 (all 13 pre-existing,
unrelated to this extraction). All 7 new api/routes/*.ts files and
api/runtime/AppRuntime.ts: 0 errors. The remaining 452 (13 in app.ts,
166 in Gateway.ts, 273 in ZwaveClient.ts) are pre-existing and out of
scope for this layer.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Direct, HTTP-layer-free unit tests for `AppRuntime`
(api/runtime/AppRuntime.ts), constructing it directly and covering:
- plain accessor/mutator round-trips for every piece of state it owns
  (gateway, zniffer, plugins router, restart flag, serial port
  enumerator override),
- the core per-request-fresh-resolution regression: a gateway/zniffer
  swapped mid-test (simulating a restart) is visible to the very next
  call, never a stale captured reference,
- `startGateway()`/`startZniffer()`'s SESSION_SECRET warning branch and
  plugin loading (success, failure, and `destroyPlugins()`),
- snippet loading (`loadSnippets()`/`getSnippets()`),
- `shutdown()`'s guarded gateway close plus plugin teardown.

`Gateway`/`ZWaveClient`/`MqttClient`/`ZnifferManager` are mocked in this
file's own isolated module graph purely to capture constructor
arguments and avoid touching real hardware/MQTT brokers; every other
test constructs `AppRuntime` directly with plain fake collaborators
(`createFakeGateway`/`createFakeZwaveClient`).

Reaches 100% statements/functions/lines and 94.11% branches for
AppRuntime.ts.

Also updates `test:server` to include `test/runtime/**/*.test.ts`
alongside the existing `test/lib/**` glob, and adds it to
`vitest.config.server.ts`'s `include`.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Extends the existing HTTP contract suites for the routes extracted into
api/routes/{debug,importExport,settings,store}.ts so each file clears
the >=90% statements/functions/lines, >=85% branches bar being
introduced for api/routes/** (see the following commit).

test/lib/http/debug.test.ts
- Cover both `/api/debug/stop` and `/api/debug/cancel`'s catch blocks.

test/lib/http/importExport.test.ts
- Cover explicit-homeId wrapped-config selection (including a
  non-string homeId being ignored), and flat-config import skipping
  node entries whose value isn't a plain object (null/string/etc).

test/lib/http/store.test.ts
- GET /api/store: a symlink-to-file entry, and hiding the configured
  `ZWAVEJS_EXTERNAL_CONFIG` directory from the listing.
- PUT /api/store: overwriting an existing regular file.
- PUT /api/store-multi: aborting the whole write when any one path in
  the batch escapes the store root (order-sensitive - the escaping path
  must be checked before any writes happen).
- POST /api/store-multi: archiving a symlinked file, verified by
  extracting the real returned zip with `extract-zip`.
- POST /api/store/upload: the multer `LIMIT_UNEXPECTED_FILE` path for
  too many files, and a full `restore=true` zip-upload flow using a
  real zip built with `archiver`.

test/lib/http/settings.test.ts
- GET /api/settings, GET /api/serial-ports: the `ZWAVE_PORT`
  env-var-set branches (managedExternally population, and skipping
  serial port enumeration entirely).
- POST /api/settings: the buildPreferences()/buildLogConfig()
  editable-driver-update paths (scales-only, logLevel-only changes with
  a driver attached), the "non-editable settings changed" and "driver
  not available" full-restart reasons, a body that omits `zwave`
  entirely against previously-truthy stored zwave settings, and a
  zniffer-only change requiring a restart on its own.
- POST /api/restart: closing an existing zniffer, restarting with no
  "gateway" key present, and cancelling an active debug session before
  restarting.
- Adds the HTTP-level gateway-swap-per-request-freshness regression
  `AppRuntime.ts`'s own docstring already promised ("the equivalent
  HTTP-level regression in test/lib/http/settings.test.ts") but that
  didn't actually exist yet: attach a fake gateway with distinguishable
  devices, confirm GET /api/settings reflects it, POST /api/restart
  (zwave/mqtt kept falsy so a real-but-inert Gateway is constructed),
  then confirm a follow-up GET /api/settings shows `devices: {}` - proving
  the pre-restart fake gateway is never observed by a later request.

Combined api/routes/** coverage after these additions: 96.32%
statements / 91.04% branches / 98.52% functions / 96.28% lines
(pooled across all 7 route files).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Adds independently-checked coverage.thresholds glob groups for
'api/runtime/**' and 'api/routes/**' (>=90% statements/functions/lines,
>=85% branches) to both vitest.config.server.ts (backend-only
`coverage:server`) and vitest.config.ts (combined `coverage`), layered
on top of the existing repo-wide floors without lowering them.

Each glob key accumulates its own coverage map independently of the
others (confirmed against the v8 provider's `resolveThresholds()`), so
this only raises the bar for the runtime/HTTP-router extraction's own
files; it does not change how the pre-existing global/`api/**`
thresholds are computed.

Current numbers clear both new groups: api/runtime/AppRuntime.ts is
100/94.11/100/100 (statements/branches/functions/lines); the pooled
api/routes/** group is 96.32/91.04/98.52/96.28.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve legacy TypeError messages without non-null assertions and defer service resolution until each concrete socket operation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
`POST /api/importConfig` cached `runtime.requireGateway('zwave')` once
into a local `gw` and reused it across every `await` in the per-node
loop (`setNodeName`, `setNodeLocation`, `storeDevices`, plus the two
`homeHex` reads used for home-id matching). `requireGateway()` is a
plain getter over the runtime's current gateway reference, so caching
it defeats the point of a mutable, restartable gateway: the pre-
extraction original held this as a module-scope, reassignable `gw`
binding that every access read live, so a gateway swapped mid-request
(e.g. a concurrent `POST /api/restart`) was always honored by whatever
ran next. Caching it into a `const` across `await`s silently reintroduced
staleness - later operations in the same import would keep hitting the
gateway that existed when the handler started, not the one actually
attached to the runtime by the time they ran.

Fixed by removing the cached `gw` and re-resolving
`runtime.requireGateway('zwave')` fresh immediately before each use,
matching every other route in api/routes/** and restoring the base
semantics exactly - including the preserved quirk that a missing
gateway still throws the historical native TypeError message (no
gateway ever existing continues to fail the same way it always has).

test/lib/http/importExport.test.ts adds a deterministic regression: a
two-node import where the first awaited `callApi` swaps the runtime's
gateway (via `harness.testHooks.setGateway`) as a side effect of
resolving, simulating a restart racing the import. The test asserts
the first call hit the original gateway while every later operation -
the same node's remaining calls and the second node's - hit the
replacement. Reverting the production fix while keeping this test
makes it fail (all three later calls incorrectly observe the original
gateway), confirming it actually exercises the bug.

api/routes/importExport.ts is unchanged coverage-wise (100/100/100/100
before and after - the refactor's repeated calls were already fully
exercised); this commit is a correctness fix plus a regression test,
not a coverage change.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The existing `shutdown()` plugin-teardown test only asserted that
`gateway.close()` ran and that no error was thrown when no plugins
were loaded - it never actually loaded a plugin through the runtime's
production loading path, so it could not have failed if
`destroyPlugins()` were broken or removed. Renamed that test to reflect
what it actually covers (the no-loaded-plugins no-op case) and kept it,
since it is still a valid, distinct scenario.

Added a new test that loads two real plugin modules through
`runtime.startGateway({ gateway: { plugins: [...] } })` - the same
production dynamic-`import()` + registration path a real deployment
uses - by writing two temporary `.mjs` files (via `mkdtempSync`/
`writeFileSync`) that each export a plugin object with a spied
`destroy`. After calling `shutdown()`, the test asserts: the gateway's
`close` was called exactly once; both plugins' `destroy` were each
called exactly once; `getPlugins()` is empty afterward; the gateway was
closed before either plugin was destroyed; and the two plugins were
destroyed in LIFO order (last-loaded, first-destroyed), matching
`destroyPlugins()`'s `pop()`-based iteration. Ordering is asserted via
each mock's own `invocationCallOrder`, a valid cross-mock global
ordering counter.

Empirically validated by temporarily breaking `AppRuntime.ts` twice and
confirming this new test - and only this test - fails each time:
skipping `destroyPlugins()` in `shutdown()` (destroy call count drops
to 0) and reversing the close/destroy order (the ordering assertion
fails). Both breaks were reverted; `AppRuntime.ts` itself needed no
production changes for this finding. Coverage is unchanged
(100/94.73/100/100), since the previous test already exercised the
same lines - this is a false-positive fix, not a coverage fix.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
`POST /api/store/upload?restore=true` extracts the uploaded zip into a
temporary `store/.restore-*` directory and, once the request finishes,
removes both that staging directory and the originally-uploaded
`store/.tmp/<filename>` - but nothing in the suite ever asserted this
cleanup actually happens, so the production `rm`/`finally` cleanup
could be deleted without any test noticing.

Adds `assertRestoreArtifactsCleanedUp(uploadedFilename)`, asserting
`store/.tmp/<filename>` is gone and no `store/.restore-*` directories
remain, wired into the existing successful-restore test and a new
failure-path test that uploads a zip containing a symlink escaping the
store (built with `archiver`'s `.symlink()`, which round-trips through
`extract-zip` as a real filesystem symlink), asserting both the exact
`'Archive contains a symlink escaping the store: evil-link'` error and
that the same cleanup still ran despite the failure.

The assertion helper polls via `vi.waitFor` rather than checking
immediately: the production cleanup's trailing `await rm(file.path)`
runs after `res.json()` has already been sent, so the HTTP response can
reach the test client slightly before that cleanup resolves. Confirmed
this is a genuine race (not a test bug) with a standalone diagnostic
script before choosing `vi.waitFor` - it passes quickly in the normal
case but still fails, after its timeout, if cleanup is ever removed
entirely.

Empirically validated by temporarily removing the production cleanup
twice - the trailing `rm(file.path)` and, separately, the `finally`
staging-directory cleanup - and confirming both the augmented and new
test fail each time with a clear leftover-file/directory diff. Both
breaks were reverted; `api/routes/store.ts` needed no production
changes for this finding. Coverage is essentially unchanged
(97.01/95.74/95/97.01): the new assertions exercise already-covered
lines, not new branches.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The `'api/runtime/**'` and `'api/routes/**'` glob threshold groups
added for the Layer 5 extraction each accumulate one *pooled* coverage
map across every file they match, so a well-covered file can mask a
poorly-covered sibling without ever failing the build - the group only
has to clear the bar on average. `api/routes/configurationTemplates.ts`
was doing exactly this: 84.21% branch coverage individually, hidden
behind the pooled group's 91%+.

Replaced both glob groups, in vitest.config.server.ts and
vitest.config.ts, with 8 exact-file entries (one literal path per key:
api/runtime/AppRuntime.ts and all 7 api/routes/*.ts files), each at the
same >=90 statements/functions/lines, >=85 branches the pooled groups
already required - non-decreasing, just no longer poolable, since a
literal (non-glob) key's coverage map contains exactly that one file.
`perFile: true` is deliberately not used instead: it is a single
top-level flag applied uniformly to the global threshold *and* every
group at once, so it cannot be scoped to only these two groups without
also changing enforcement everywhere else in the repo.

Closed the actual gap with new tests in
test/lib/http/configurationTemplates.test.ts rather than loosening the
threshold: three "past validation, no gateway attached" HTTP tests for
the POST create/import/apply routes (which have early-return body
validation that a bodyless smoke test never gets past, unlike the
other four routes), and three direct-handler-invocation tests for the
PUT/DELETE/POST-apply `:id` routes' `if (!id)` guard - unreachable via
real HTTP since Express requires a non-empty `:id` segment - via a new
`captureConfigurationTemplatesHandler()` helper that registers the real
`registerConfigurationTemplatesRoutes` against a minimal fake `app` and
invokes the captured handler closure directly.
`api/routes/configurationTemplates.ts` now measures 100/100/100/100
(was 90.32/84.21/100/90.32); all 8 target files clear the new
per-file floor with margin (lowest is store.ts at 95%/97.01 lines).

Proved the mechanism is genuinely per-file, not still secretly pooled:
temporarily set api/routes/health.ts's branch threshold to an
impossible 99% and confirmed `coverage:server` failed citing
health.ts's own actual 92.85% - not the pooled api/routes directory
average of 92.16% - then reverted and confirmed a clean run. Both
`npm run coverage:server` and the combined `npm run coverage` pass with
exit code 0 across all 48 test files / 735 tests.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Audit added/changed comments across the runtime and http route
modules from this refactor: remove restatements, decorative section
dividers, and narrative/history framing; keep or rewrite only
non-obvious rationale as single, fact-first sentences.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Audit added/changed comments in the new and modified http and
runtime test files from this refactor: remove restatements,
self-referential citations, and decorative dividers; keep or
rewrite only genuinely non-obvious setup/assertion rationale.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Audit added/changed comments in the per-file coverage threshold
setup: remove a quoted commit message and changelog framing, keep
the config-verifies-source-not-tests rationale as one sentence.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A prior comment tightening pass in this audit incorrectly singularized
the fake app's (path, ...handlers) signature to (path, handler); restore
the accurate variadic/last-handler wording caught by review.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…traction

Second semantic-preservation pass over 5f365bd/e909741b/78360a1a/f25e9b07
(comparing PR head 9d38554 to HEAD): audited all 154 comments those
commits removed or materially rewrote across the 19 touched files (13
production, 6 test). 60 outright deletions contained no non-obvious fact
and are confirmed correctly deleted; 75 rewrites and 10 untouched comments
already preserve every domain fact, compatibility quirk, and
security-relevant detail - no further change needed. 8 needed restoration:

- api/routes/auth.ts: isAuthenticated()'s JWT fallback exists because
  session-cookie auth requires third-party cookies to be allowed (git log
  -S traces this to the original 2021 auth commit 7eef6c5)
- api/routes/health.ts: the disabled-mqtt status quirk links back to its
  originating issue (see #469), matching this codebase's established
  issue-citation convention (api/lib/logger.ts, api/lib/ZwaveClient.ts)
- api/runtime/AppRuntime.ts: enumerateSerialPortsIsProductionDefault is
  read only by api/app.ts's __testHooks, never by production logic
- test/lib/http/configurationTemplates.test.ts: the captured handler's
  `if (!id)` guards stay in place as deliberate defensive code even
  though real HTTP requests can't reach them
- test/lib/http/debug.test.ts: re-calling cancelSession()/stopSession()
  here would hang awaiting a winston transport 'finish' event that only
  fires once
- test/lib/http/importExport.test.ts: the gateway-swap test guards
  against a route caching the gateway once instead of re-resolving it
  per operation
- vitest.config.server.ts: perFile is a single global switch that would
  force the repo-wide floor onto every file individually, which most
  files don't meet on their own - the reason exact-file keys are used
  instead
- vitest.config.ts: running backend tests once here also produces the
  file-level api/** line data Coveralls needs, avoiding a second
  dedicated backend coverage run

Comment-only change, no runtime or test behavior change.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
… of test-only seams

AppRuntime owned two testability-only surfaces that don't belong on a
production runtime object per the accepted #4723 createApp/DI model:

- enumerateSerialPortsFn/enumerateSerialPortsIsProductionDefault was a
  module-global-shaped swap seam AppRuntime carried purely so tests could
  fake serial-port enumeration. Relocated to api/app.ts as local state next
  to the __testHooks that are its only reader; registerSettingsRoutes now
  receives it as an explicit getEnumerateSerialPorts() factory dependency
  instead of routes reaching into runtime for it.
- getBackupManager()/getDebugManager() were pure pass-through wrappers
  around already-imported singletons, existing "so routes reach them
  through the same seam as everything else AppRuntime owns" (per their own
  comment) rather than for any behavioral reason. debug.ts, settings.ts,
  and app.ts now import backupManager/debugManager directly, exactly as
  AppRuntime.ts itself and the original app.ts already did.
- getSettings() was dead code with zero production call sites, kept alive
  only by its own delegation test. Removed.

requireGateway(property)/requireZniffer(property) manufactured a native-
looking TypeError from a string argument to mimic a pre-refactor crash.
Both are now no-arg methods throwing a clean typed Error. Since several
call sites needed `.zwave` specifically (not just any Gateway), split out
requireZwaveClient(): Gateway | undefined check plus the zwave-client
check collapse into one clean 'Z-Wave client not inited' failure, matching
the accepted #4723 inline pattern verbatim. getSnippets() no longer routes
through requireGateway() at all: a missing gateway now falls back to an
empty snippets cache instead of throwing, since callers only want best-
effort cached snippets, not a hard failure.

Every requireGateway('x')/requireZniffer('x') call site in app.ts and the
route files is updated to the new no-arg signatures, using
requireZwaveClient() wherever the call site actually dereferences
`.zwave`. store.ts's multer upload handler now uses optional chaining
instead of an unguarded cast, so a non-multipart request reaches the
existing "No file uploaded" error instead of a raw cast-index crash.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Enables tests to import api/ modules via the package subpath import
instead of deep ../../api/ relative paths.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Removes AppRuntime's get/set/require round-trip and identity/delegation
tests (gateway, zniffer, plugin router, restart flag, serial port
enumerator seam, backup/debug manager access, getSettings()) - these
tested plain accessor plumbing, not observable behavior, and several
targeted seams that no longer exist on AppRuntime.

Rewrites the no-gateway-attached getSnippets() test from asserting a
raw TypeError to asserting the graceful ?? [] fallback, matching the
production behavior change. Converts relative ../../api/ imports and
vi.mock() targets to the #api/* alias.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…nager.init() call

requireZwaveClient() throws when the gateway has no zwave client, but
zwave being disabled in settings is a valid configuration, not an
error condition - matching the tolerated undefined zwave/mqtt pattern
already used by startGateway()'s own backupManager.init() call.
Using requireGateway().zwave here restores that tolerance while still
guaranteeing a gateway is present.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
AlCalzone and others added 11 commits July 17, 2026 09:43
…NNELS constant

Replace a hand-copied, order-dependent channel list with the
production-exported ALL_CHANNELS constant and an order-independent
comparison. Channel declaration order in channelMap is an internal
implementation detail, not a documented delivery-order contract.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Two tests reached into ZWaveClient's private _virtualNodes/_nodes Maps
via 'as any' purely to assert internal bookkeeping was cleared, after
already asserting the real, public NODE_REMOVED socket event. The
private-state check adds no externally-observable coverage the public
assertion doesn't already provide.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Additive formatting-only pass (npm run lint-fix) over content introduced
during this repair's replay: api/bin/www.ts's extra blank line and
api/lib/BackupManager.ts's init() signature wrap. No behavior change.

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>
@AlCalzone
AlCalzone force-pushed the alcalzone-backend-refactor-http-runtime branch from aa5a974 to a45df40 Compare July 17, 2026 08:00
@AlCalzone
AlCalzone marked this pull request as ready for review July 17, 2026 08:21
@AlCalzone
AlCalzone requested a review from Copilot July 17, 2026 08:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the backend entrypoint by extracting application lifecycle responsibilities into a new AppRuntime and moving HTTP endpoints into focused route modules, while expanding/refreshing contract tests to guard restart behavior and API compatibility (part of #4722, stacked on #4726).

Changes:

  • Introduces api/runtime/AppRuntime.ts + api/runtime/ports.ts to make runtime service ownership and restart lifecycle explicit.
  • Splits api/app.ts HTTP routing into dedicated route modules under api/routes/, plus a small api/lib/serialPorts.ts boundary.
  • Updates/expands Vitest contract coverage (HTTP + Socket.IO + runtime), and updates package.json test scripts and imports aliasing for #api/*.

Reviewed changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
test/runtime/AppRuntime.test.ts Adds runtime-level tests for snippet loading, auth secret warnings, plugin lifecycle, and shutdown resilience.
test/lib/socket/subscriptions.test.ts Updates socket subscription tests to use the new flushClientEvents() barrier mechanism.
test/lib/socket/outboundProducers.test.ts Adds more “real producer” socket payload shape characterization tests.
test/lib/socket/inboundApis.test.ts Adjusts inbound socket ACK contract tests; adds coverage for unknown HASS API and zniffer buffer load ack behavior.
test/lib/socket/harness.ts Adds flushClientEvents() helper and removes pluginsRouter plumbing.
test/lib/socket/fakes.ts Re-exports shared zniffer fake types/helpers from shared fakes.
test/lib/shared/harness.ts Clones store config before initializing jsonStore; adds afterEach cleanup to reset store models.
test/lib/shared/fakes.ts Refactors fakes to implement runtime “port” types; adds shared zniffer fake.
test/lib/shared/env.ts Extends mocked app config with sslDisabled() for env-driven HTTPS behavior.
test/lib/http/store.test.ts Substantially expands HTTP store/upload/backup/snippet contract tests, including ZIP restore and symlink handling.
test/lib/http/settingsConstructorBoundary.test.ts Updates mocks to include close() and refreshes restart/setup expectations.
test/lib/http/settings.test.ts Adds coverage around managed-external settings, serial-port enumeration boundary, restart behavior, and in-place driver updates.
test/lib/http/sessionSerialization.test.ts Simplifies/updates session serialization test descriptions.
test/lib/http/routeContract.test.ts Removes the Express route inventory “drift detection” test file.
test/lib/http/importExport.test.ts Adds more import-config contract coverage (home-id selection, coercions, and stale-client replacement).
test/lib/http/harness.ts Centralizes mocking of enumerateSerialPorts via api/lib/serialPorts.ts boundary; adds zniffer option support.
test/lib/http/debug.test.ts Improves typing and adds coverage for invalid nodeIds + cancel failure paths.
test/lib/http/configurationTemplates.test.ts Adds coverage for “no gateway attached” failures on template endpoints.
test/lib/http/authRateLimit.test.ts Narrows scope to login rate limiting and clarifies limiter semantics.
test/lib/http/auth.test.ts Minor wording update in password endpoint test description.
test/lib/http/appLifecycle.test.ts Adds lifecycle tests ensuring process handlers aren’t reinstalled after close and are cleaned up on startup failures.
package.json Adds #api/* import alias and includes test/runtime in server test/coverage scripts.
api/runtime/ports.ts Introduces strict-ready “port” types for runtime dependencies (gateway/zwave/mqtt/zniffer).
api/runtime/AppRuntime.ts New runtime lifecycle owner for gateway/zniffer/plugins/snippets and backup manager ownership tokenization.
api/routes/store.ts Extracts store/upload/backup/snippet HTTP routes into a dedicated router module.
api/routes/settings.ts Extracts settings/restart/statistics/versions HTTP routes and adds driver in-place update behavior.
api/routes/importExport.ts Extracts import/export config HTTP routes; re-resolves runtime client per operation to avoid stale capture.
api/routes/health.ts Extracts health/version endpoints.
api/routes/debug.ts Extracts debug-capture HTTP endpoints and tightens some validation.
api/routes/configurationTemplates.ts Extracts configuration template endpoints and standardizes the “require zwave” behavior.
api/routes/auth.ts Extracts auth endpoints and shared auth middleware/JWT parsing helpers.
api/lib/serialPorts.ts Adds a module boundary wrapper for serial port enumeration to simplify mocking.
api/lib/jsonStore.ts Fixes backup response headers to use ZIP content type.
api/lib/DebugManager.ts Switches to port typing for zwave client; improves session cleanup semantics and listener limits.
api/lib/BackupManager.ts Switches to port typing for zwave client; makes “zwave client missing” explicit for NVM backup path.
api/config/app.ts Adds sslDisabled() function (fresh env read) for testability.
api/app.ts Replaces in-file route implementations with extracted router registrations and uses AppRuntime for lifecycle management.

Comment thread api/routes/store.ts
Comment thread api/routes/store.ts
Comment thread api/routes/settings.ts Outdated
Comment thread test/lib/shared/fakes.ts Outdated
Comment thread api/runtime/AppRuntime.ts
Comment thread api/routes/settings.ts
Comment thread api/runtime/AppRuntime.ts Outdated
AlCalzone and others added 3 commits July 17, 2026 12:28
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>
Comment thread api/lib/BackupManager.ts Outdated
Comment thread api/lib/BackupManager.ts Outdated
Comment thread api/routes/configurationTemplates.ts Outdated
Comment thread api/routes/health.ts Outdated
Comment thread api/runtime/AppRuntime.ts Outdated
Comment thread test/lib/shared/fakes.ts Outdated
Comment thread api/runtime/AppRuntime.ts
Comment thread api/runtime/ports.ts Outdated
AlCalzone and others added 3 commits July 17, 2026 12:51
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>

@AlCalzone AlCalzone left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure we don't remove tests without a good reason.

Comment thread api/lib/BackupManager.ts Outdated
Comment thread api/runtime/ports.ts Outdated
Comment thread api/lib/serialPorts.ts Outdated
Comment thread api/routes/auth.ts Outdated
Comment thread api/app.ts
Comment thread test/lib/http/authRateLimit.test.ts
Comment thread test/lib/http/harness.ts Outdated
Comment thread test/lib/http/routeContract.test.ts Outdated
Comment thread package.json Outdated
AlCalzone and others added 3 commits July 17, 2026 20:34
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>
@AlCalzone
AlCalzone requested a review from robertsLando July 17, 2026 19:06
@AlCalzone

Copy link
Copy Markdown
Member Author

@robertsLando if you don't find anything else, feel free to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants