Jérôme Benoit [Sun, 23 Aug 2026 15:33:04 +0000 (17:33 +0200)]
chore: remove Sandcastle integration (#2093)
* chore: remove Sandcastle integration
- delete .sandcastle agent harness, Docker sandbox and scheduled workflow
- drop @ai-hero/sandcastle devDependency and 'sandcastle' script
- remove .sandcastle from lint/format globs, tsconfig include and .cfignore
- drop unused cspell words and pnpm minimumReleaseAgeExclude entry
Jérôme Benoit [Fri, 14 Aug 2026 22:08:21 +0000 (00:08 +0200)]
refactor(webui): order station action buttons by CSO lifecycle (#2084)
* refactor(webui): order station action buttons by CSO lifecycle
Reorder the station-card action buttons in both skins to follow the
Charging Station Operator lifecycle (bring-up, provisioning, exploitation,
observability):
- modern StationCard: Start/Stop, Connect/Disconnect, Configuration,
Authorize, Details
- classic CSData: Start/Stop, Connection, Set Supervision Url,
Change Configuration, Show Details
This fixes Authorize (runtime) preceding Configuration (provisioning) and
Details being wedged between active commands. Action sets, labels, handlers,
routes, styling and the isolated Delete button are unchanged.
Make the classic CSData show-details toggle assertions look the button up by
id instead of a fixed index, so they stay robust to ordering.
Add id-based navigation tests for the change-configuration toggle in
classic CSData, mirroring the show-details coverage: assert on() pushes
the change-configuration route with hashId/chargingStationId params and
off() pushes back to charging-stations. Closes the coverage gap for the
toggle reordered in this change.
* test(webui): select set-supervision-url toggle by id for consistency
Align the set-supervision-url toggle navigation tests with the
change-configuration and show-details ones by looking the toggle up via
its id instead of a fixed index, so the whole toggle-navigation suite is
order-independent and uses a single selection convention.
Jérôme Benoit [Fri, 14 Aug 2026 18:03:20 +0000 (20:03 +0200)]
fix(webui): keep modern station-card Delete action aligned on the footer row (#2083)
The modern station-card footer laid the 5 action buttons in a non-wrapping
inline-flex group beside the Delete button, inside a wrapping footer. Once the
group's max-content width exceeded the card, the footer broke the line between
the group and Delete, dropping Delete onto its own row where margin-left:auto
pinned it bottom-right (orphaned) instead of aligned with the other actions.
Make the footer a single flex line (flex-wrap: nowrap) and let the action
group wrap its buttons internally (flex-wrap: wrap) so it reflows across rows
on narrow cards while Delete stays on the same row, pinned right. align-self:
flex-start top-aligns Delete with the first action row when the group wraps.
Presentational modern-skin CSS only; no markup or handler change.
Jérôme Benoit [Fri, 14 Aug 2026 13:39:35 +0000 (15:39 +0200)]
fix(simulator): seed autoRegister and ocppProtocol defaults into stationInfo (#2082)
* fix(simulator): seed autoRegister and ocppProtocol defaults into stationInfo
autoRegister and ocppProtocol had no default, so a template omitting them
left stationInfo carrying undefined for both. Raw consumers of stationInfo
— the UI data payload (buildChargingStationDataPayload) and the persisted
configuration — therefore received undefined, so the Web UI station details
showed an empty placeholder for Auto Register and OCPP Protocol.
Both are static defaults (no derivation), so seed them in DEFAULT_STATION_INFO
(autoRegister: false, ocppProtocol: OCPPProtocol.JSON) alongside currentOutType
and ocppVersion. getStationInfo applies them via
mergeDeepRight(DEFAULT_STATION_INFO, stationInfo), so an explicit template/file
value or option still wins (idempotent, no clobber) and a persisted config
predating the fields is backfilled on reload. Export OCPPProtocol from the
types barrel to keep Constants.ts importing types from a single source, as
OCPPVersion already does.
Runtime-neutral: ocppProtocol has no runtime read; the autoRegister === true
guards are unaffected by false vs undefined; the getHeartbeatInterval warn
guarded by === false is unreachable in the initialized pipeline because
initializeOcppConfiguration always seeds the HeartbeatInterval key first.
* refactor(test): extract persisted-config resolution into realStation helpers
The persisted configuration file/dir resolution from a real-station template
was inlined and duplicated across three suites (AutoRegisterOcppProtocol,
NumberOfPhases, ResetIdentity). Extract persistedConfigurationDir and
resolvePersistedConfigurationFile into StationHelpers.realStation.ts (single
source of truth for the temp-dir layout) and migrate all three call sites.
resolvePersistedConfigurationFile throws a descriptive error when no config
exists yet, replacing the silent `?? ''` fallback (which degraded into EISDIR).
Group the helper by concern (temp-dir lifecycle / construction / persisted-config
resolution) with section headers, and document the two new helpers in
TEST_STYLE_GUIDE.md.
* docs(test): harmonize helper-table param placeholders in TEST_STYLE_GUIDE
Jérôme Benoit [Thu, 13 Aug 2026 20:13:58 +0000 (22:13 +0200)]
test(simulator): drop manual test counters from worker broadcast-channel test (#2081)
The group-header comments carried hand-maintained (N tests) counters that rot
as tests are added or removed; the test runner already reports the count.
Remove them, keeping the descriptive group headers.
Jérôme Benoit [Thu, 13 Aug 2026 19:37:03 +0000 (21:37 +0200)]
feat(webui): allow editing charging station configuration (#2077)
* feat(webui): allow editing charging station configuration
Add a generic, data-driven editor for a charging station's current OCPP
configurationKey values in the Web UI (both classic and modern skins),
closing #1828.
A new CHANGE_CONFIGURATION UI protocol verb clones the SET_SUPERVISION_URL
chain end-to-end (UIClient -> ProcedureName/BroadcastChannelProcedureName ->
AbstractUIService mapping -> worker handler). To stay OCPP spec-faithful, the
change is applied through a new version-agnostic
ChargingStation.changeConfiguration seam that reuses the existing OCPP 1.6
ChangeConfiguration and OCPP 2.0.1 SetVariables handler logic (readonly
rejection, integer/bounds validation, heartbeat/WS-ping restarts, reboot
signalling) without emitting a CSMS response, then emits
ChargingStationEvents.updated so the UI refreshes.
OCPP 2.0.1 configurationKey entries are now seeded with their registry
mutability/rebootRequired flags, so readonly keys are correctly disabled in
the UI and reboot keys are flagged.
Address PR review coverage gaps:
- assert ChargingStation.changeConfiguration emits ChargingStationEvents.updated
only on Accepted/RebootRequired (drives the read-view refresh)
- assert the OCPP 1.6 seam applies changes without emitting a CSMS response
- add OCPP 2.0.1 seam read-only rejection and reboot-required cases
* refactor(webui): address review findings for change-configuration
- OCPP 2.0.1 seam: set the resolved `instance` only on the SetVariables
`component` (drop the redundant `variable.instance`); the registry models
the instance as component-scoped and internal resolution reads
`variable.instance ?? component.instance`, so this is behavior-neutral.
Add an instance-scoped non-regression test (TariffCostCtrlr.Enabled.Cost).
- useStationDetails: docstring no longer claims "read-only" now that it
exposes editableConfigurationKeys.
- ui/web README: "edit configuration" -> "change configuration" to match the
UI/code terminology.
- ModernLayout: order the ChangeConfigurationDialog async declaration
alphabetically and put :hash-id before :charging-station-id, matching the
sibling dialogs.
* refactor(webui): hoist change-configuration draft state into the shared composable
Address the second-round review findings:
- M1 (DRY): move the per-key draft values, seeding watch and save() from both
skin components into useChangeConfigurationForm (now takes the editable keys
ref and exposes { draftValues, pending, save }), mirroring useSetUrlForm which
owns its form state. The classic action and modern dialog become pure UI.
- M2: add direct unit tests for the worker CHANGE_CONFIGURATION handler
(delegation, empty-value accepted, missing/empty/non-string key or value
rejected with BaseError).
- M3: align residual "edit"/"editing" wording with the "change configuration"
terminology in docstrings (kept editableConfigurationKeys as a property name).
* refactor(webui): validate required broadcast-channel string fields with isNotEmptyString
Replace the raw `typeof x !== 'string' || isEmpty(x)` guards in the
CHANGE_CONFIGURATION and SET_SUPERVISION_URL worker handlers with the
`!isNotEmptyString(x)` type guard (the repo-wide idiom, ~78 usages), which also
narrows the field to a non-empty string. Both handlers are switched together so
the file keeps a single convention. `isEmpty` remains used for the
empty-response status checks; the `value` field stays `typeof value !== 'string'`
since an empty value is a legitimate configuration value.
* refactor(webui): tidy change-configuration naming, reverse-map and MCP schema
Third-round review nits:
- M1: rename `editableConfigurationKeys` -> `visibleConfigurationKeys` (the
collection includes read-only keys; the name now matches its source util
getVisibleConfigurationKeys). Propagated across useStationDetails,
useChangeConfigurationForm, both skin components and the tests.
- M2: build OCPP20VariableManager's flat-key reverse map as a declarative
private instance field (like #validComponentNames) instead of a mutable
module-level for-loop; the dedup guard was inert (composite key names are
unique).
- T1: inline the single-caller private `submit` into `save`.
- T2: enforce a non-empty `key` at the MCP option layer (z.string().min(1));
`value` stays unconstrained (empty is a valid configuration value).
Add component tests for the classic `ChangeConfiguration.vue` action and the
modern `ChangeConfigurationDialog.vue`, which were shipped without component
tests and dropped ui/web coverage below the CI thresholds (the failing
`Build dashboard / Node 24.x / ubuntu-latest` cell runs `pnpm test:coverage`).
Tests exercise the observable contracts already verified in-browser: not-found
panel, empty-state, non-visible key exclusion, read-only input/button disabling,
editable save invoking `changeConfiguration`, reboot-required notice, read-only
save guard and backend-rejection error toast. Adds the missing
`changeConfiguration` method to the shared `MockUIClient`.
Refs: #1828
* fix(ocpp): reject empty value for integer 1.6 configuration keys
Address the initial-review findings on PR #2077:
- T1 (fix): the OCPP 1.6 ChangeConfiguration handler accepted an empty/blank
value for integer keys (Number('') === 0 passed the non-negative-integer
check) and persisted the raw empty string. Reject it explicitly via
isNotEmptyString, matching the spec (values not conforming to the expected
integer format are Rejected). Covers both the UI seam and the OCPP wire path.
Add a discriminating test (empty value -> REJECTED, value preserved).
- T2 (fix): correct the misleading worker error message
"'value' field is required" -> "'value' field must be a string" (an empty
string is a legitimate value; only a non-string is rejected).
- T3 (test): rename the composable test description "editable keys" ->
"visible keys" to match the renamed visibleConfigurationKeys parameter.
- M1 (docs): document, on OCPP20 changeConfiguration, that the resolved
instance is carried on component.instance and is internal-only (never
emitted to a CSMS), so the component- vs variable-instance distinction is
immaterial here.
- M2 (docs): add the missing 'changeConfiguration' ProcedureName block to the
README UI protocol reference.
Kept as-is with rationale: the version-agnostic seam reuses the 1.6-named
ChangeConfigurationResponse type (established CommandResponse union convention,
type-safe alias); the modern dialog table style follows the skin's
per-component scoped convention. Cleartext config values in the edit UI are
pre-existing (already exposed via LIST_CHARGING_STATIONS / MCP) and out of
scope.
Refs: #1828
* docs: clarify changeConfiguration value validation in the README protocol reference
Address review finding TR1: the changeConfiguration block claimed "an empty
string is allowed", which is only true at the message-envelope level. A value
invalid for the target key (e.g. a non-integer or empty value for a numeric
1.6 key) is rejected. Reword the value clause to state that invalid values are
rejected and keep it version-agnostic (the verb serves both OCPP 1.6 and
2.0.1). No code change.
Refs: #1828
* test(webui): factor duplicated configuration-keys station fixture into a shared factory
The `stationWithKeys` helper was duplicated verbatim in the classic and modern
skin test files. Mirroring the test-factorization convention consolidated in
main (PR #2078: shared test helpers, no per-file scaffolding duplication),
extract it into the canonical fixture home `tests/unit/constants.ts` as
`createStationWithConfigurationKeys`, and migrate both skin test suites to it.
Removes the now-unused local helpers and `ConfigurationKey` imports. No behavior
change (ui/web 592/592).
Refs: #1828
* test(webui): migrate remaining inline config-key fixtures to the shared factory
Address review finding TR1: complete the DRY migration started with
createStationWithConfigurationKeys. Replace the 13 remaining inline
`createChargingStationData({ ocppConfiguration: { configurationKey: … } })`
sole-override fixtures across the stationDetails, useStationDetails, ShowDetails
and ShowDetailsDialog test blocks with the shared factory. The empty-
ocppConfiguration case (stationDetails.test.ts, tests the absent-key fallback)
and all sites carrying additional overrides are intentionally left inline. No
behavior change (ui/web 592/592).
- OCPP16 integer-key guard: consolidate the two lines commenting the same guard
into one coherent, accurate rationale — why Number() over convertToInt
(truncates '1.5' → 1, throws on ''/'abc') and why the explicit isNotEmptyString
check. Number() yields a non-integer float ('1.5' → 1.5) or NaN ('abc'), both
caught by !Number.isInteger; isNotEmptyString rejects '' (Number('') === 0 would
otherwise pass).
- OCPP20 changeConfiguration JSDoc: remove redundancy with the delegate's JSDoc
and compress the instance-placement caveat, keeping every non-derivable fact.
Comment-only change; no behavior change.
Refs: #1828
* refactor(webui): single-source config-key formatting and align a11y state
- Export the shared formatBoolean helper and reuse it for the Readonly/Reboot
cells in both change-configuration skins (drop the 4 inline Yes/No literals).
- Classic Save button: expose aria-busy while a change is in flight, matching
the modern ActionButton (no visual spinner: classic has no such token).
- Drop the redundant aria-labelledby (and its useId) on the modern table whose
<caption> already names it, and the partial aria-disabled on the inputs whose
native disabled is authoritative.
- Add tests: Readonly/Reboot cell rendering and in-flight aria-busy in both
skins, plus modern parity for reboot-notice, read-only-not-submitted and
error-toast (mirroring the classic suite).
Refs: #1828
* docs: clarify metadata-driven reboot notice and safe 2.0.1 status collapse
- useChangeConfigurationForm: note that the reboot notice derives from the key
metadata, not the runtime status, because the worker response collapses
ACCEPTED|REBOOT_REQUIRED into a boolean success.
- OCPP20 status mapping: note that UnknownComponent/UnknownVariable are
unreachable via changeConfiguration (resolveConfigurationKeyName gates unknown
keys) and that Record exhaustiveness is compile-enforced.
- Yes/No cell test: use column-asymmetric keys (readonly-only vs reboot-only)
so an accidental swap of the Readonly/Reboot columns fails the assertion.
- Add a guard that the OCPP parameters table is named via its <caption> in
both skins (the modern table dropped its redundant aria-labelledby).
Refs: #1828
* refactor(ocpp): use isEmpty for the SetVariables result check
Replace `response.setVariableResult.length === 0` with `isEmpty(...)` in the
2.0.1 changeConfiguration seam, matching the repo convention (isEmpty is
already imported and used for arrays in this file).
Refs: #1828
* test(webui): fit CHANGE_CONFIGURATION worker tests into the group structure
The two CHANGE_CONFIGURATION describes (status collapse + handler) were wedged
between Group 1 and Group 2 without a group banner. Relocate them after Group 2
under a numbered "Group 3" banner (restoring ascending group order and filling
the pre-existing gap), and correct the stale Group 4 count (8 -> 9 tests).
Jérôme Benoit [Tue, 11 Aug 2026 22:09:54 +0000 (00:09 +0200)]
fix(simulator): seed numberOfPhases default into stationInfo (#2078)
* fix(simulator): seed numberOfPhases default into stationInfo
The derived numberOfPhases default (AC: 3, DC: 0) lived only in the
getNumberOfPhases getter and was never written to stationInfo. Raw
consumers of stationInfo — the UI data payload (buildChargingStationDataPayload)
and the persisted configuration file — therefore received an undefined
numberOfPhases, so the Web UI showed an empty placeholder instead of the
effective phase count.
Seed stationInfo.numberOfPhases via the existing getNumberOfPhases getter
in getStationInfo, post-merge and source-agnostic, so both freshly
template-derived and already-persisted (legacy) configurations are fixed.
Idempotent: an explicit AC template value is preserved (?? 3), DC pins 0.
Backend consumers keep reading the getter and are invariant.
* test(simulator): align numberOfPhases test names with should-prefix convention
* test(simulator): cover DC phase pinning and persisted-config backfill
Add two discriminant cases to the numberOfPhases seeding suite:
- DC pins numberOfPhases to 0 even when the template sets a value
- a persisted configuration predating the field is backfilled on reload
(the file-sourced path that motivated seeding in the orchestrator)
- reuse the canonical flushMicrotasks test helper instead of a local
node:timers/promises import
- expand the test @file description to cover DC pinning and legacy backfill
- align the seed comment terminology on "backfill"
* test(simulator): dedupe test baseName literal into a constant
* docs(simulator): fix backfill comment accuracy and align seed wording
* test(simulator): inline baseName to match sibling station-test convention
* test(simulator): consolidate real-station-from-template scaffolding into a shared helper
Six charging-station tests each hand-rolled the same temp-dir template
scaffolding to build a real ChargingStation (the mock factory bypasses
initialize()/getStationInfo()). Extract writeStationTemplate/copyStationTemplate/
createStationFromTemplate/cleanupStationTemplates into StationHelpers.realStation,
migrate all six tests, and document the helper in TEST_STYLE_GUIDE.
* test(simulator): share temp-file and singleton-reset test helpers
Extract the mkdtemp/write/cleanup boilerplate into tests/helpers/TempFiles.ts
(createTempDir/writeTempFile/cleanupTempDirs) and a resetSingleton() helper into
TestLifecycleHelpers, then migrate the file-I/O tests (IdTagsCache, EvProfiles,
JsonFileStorage, Configuration-HotReload, FileUtils, UIMCPServer integration) and
the singleton-reset tests (Bootstrap, SharedLRUCache, IdTagsCache) onto them.
* docs(test): document TempFiles helpers and resetSingleton in the style guide
Jérôme Benoit [Mon, 10 Aug 2026 17:38:42 +0000 (19:38 +0200)]
fix(webui): polish the station details view (panel placement + section/key-value separation) (#2076)
CSS/Vue-only polish of the #993 Show details view: classic panel-placement fix + section/key-value separation; modern section panels, promoted titles and tighter symmetric row separators; not-found escape; toggle-navigation test coverage.
Jérôme Benoit [Mon, 10 Aug 2026 15:51:27 +0000 (17:51 +0200)]
feat(webui): add show details action for charging stations (#2072)
* feat(webui): add show details action for charging stations
Add a read-only "Show details" action in both Web UI skins displaying a
charging station's stationInfo, OCPP configuration parameters and other
relevant ChargingStationData fields. Frontend only: the data is already
shipped to the client via the charging station data payload.
- Shared pure util `stationDetails.ts` (single source of truth for section
selection, field formatting and OCPP key visibility filtering), matching
the existing `stationStatus.ts` pure-util convention.
- Classic skin: `show-details` router action + `ShowDetails.vue` panel,
triggered by a shared ToggleButton in the station actions cell.
- Modern skin: `StationDetailsDialog.vue` modal, triggered by a footer
"Details" button on the station card, wired through ModernLayout.
- Supervision password is always masked; supervision URL is host-only.
Closes #993
* [autofix.ci] apply automated fixes
* refactor(webui): centralize OCPP row formatting and address review
Review-round-1 fixes for the "Show details" action:
- Move OCPP cell formatting (readonly/reboot/value) into the shared
`buildConfigurationRows` util so both skins render identical, single-
sourced rows instead of duplicating the ternaries.
- Format Boot Notification "Current Time" via toLocaleString (consistent
with "Last Update") and drop the unsafe double cast.
- Give the modern OCPP table an accessible name (aria-labelledby).
- Tests: assert password masking in the classic rendered panel, cover the
OCPP readonly/reboot/value cell formatting in both skins and the util.
* test(webui): tighten details tests and unify empty-value formatting
Review-round-2 fixes for the "Show details" action:
- Format the OCPP parameter value via the shared `formatValue` so an
empty-string value renders as the empty placeholder, consistent with
every other field (was `value ?? Ø`, which left '' blank).
- Strengthen the modern password-masking test to also assert the masked
placeholder, add coverage for the empty-string value case, and assert
the modern OCPP table's aria-labelledby accessible name.
* refactor(webui): address initial-review findings for show details
- M1 (DRY): extract shared `useStationDetails(hashId)` composable consumed by
both skins, removing the duplicated station/sections/configurationRows
computed (mirrors the shared `useSetUrlForm` precedent).
- M2 (terminology): unify the feature label on "Show Details" across the
classic header, the modern dialog title and the README; the modern card
keeps its terse "Details" button per the card convention; route id
unchanged.
- M3: drop the duplicated Boot Notification "Status" entry (kept as
"Registration Status" under General).
- N1: factor a private `formatDate` helper and guard both dates.
- N3: give each modern detail `<dl>` section an accessible name via useId().
- N2 (OCPP terminology) intentionally left as "OCPP Parameters" to match the
issue wording; documented in review.
- Tests: new `useStationDetails` composable tests; assert the modern section
aria-labelledby wiring; update the "Show Details" heading/title assertions.
* style(webui): harmonize modern show-details with skin conventions
- H3: extract the shared `.modern-section-label` primitive (renamed from
`.modern-card__section-label`, single consumer migrated) and use it for the
modern dialog section headings instead of a one-off `.station-details__title`.
- H1: restyle the detail key/value list to the modern spec typography
(uppercase muted `dt`, strong `dd`) in a left-aligned two-column grid fit
for the wider dialog (drops the ad-hoc space-between/right-align/hairlines).
- H2: restyle the OCPP table to the modern low-chrome table aesthetic
(border-collapse, no per-cell borders, muted uppercase headers, subtle row
separators) matching the existing `.modern-connector__tx-table` precedent;
scope word-break to values/keys so column headers no longer break mid-word.
Classic skin unchanged (already reuses the shared data-table system). No test
changes: DOM hooks (.station-details__list/__table), aria-labelledby and the
OCPP row formatting are preserved. Card rendering is visually unchanged.
* style(webui): resolve exhaustive-review nits for show details
- MIN-1: left-align the classic detail table cells (target th/td so the
shared `.data-table` center rule no longer wins), fixing centered values.
- MIN-2: modern dialog title to sentence-case "Show details — {id}" to match
the sibling dialog titles (classic header keeps Title Case per its convention).
- NIT-1: derive the OCPP heading id from useId() instead of a hardcoded id,
consistent with the section headings.
- NIT-2: extract a private `formatBoolean` helper reused by
buildConfigurationRows and formatValue's boolean branch.
- NIT-3: align the shared "Supervision Url" label with the existing majority
spelling used across the classic surface.
- NIT-4: rename StationDetailsDialog.vue -> ShowDetailsDialog.vue (matches the
classic ShowDetails.vue and the feature name); update wiring + tests.
"OCPP Parameters" kept (issue #993 wording). Tests updated for the new title,
useId-based aria and the rename. 567 tests green.
* style(webui): order ShowDetailsDialog async import alphabetically
* docs(webui): fix show details JSDoc wording and uniformize empty-state punctuation
The HTTP UI transport (UIHttpServer / ApplicationProtocol.HTTP) is
deprecated in favor of the MCP transport and will be removed. Drop its
Insomnia requests collection and the README pointer to it; the WebSocket
collection and the deprecation notice are kept.
Jérôme Benoit [Tue, 4 Aug 2026 21:20:36 +0000 (23:20 +0200)]
chore(opencode): stop Serena MCP from auto-opening the web dashboard (#2066)
Pass --open-web-dashboard false to serena start-mcp-server in .opencode/opencode.jsonc so the dashboard is not opened in a browser on every OpenCode startup.
Jérôme Benoit [Tue, 4 Aug 2026 20:04:07 +0000 (22:04 +0200)]
feat(simulator): add AC/DC conversion efficiency template support (#2063)
Add optional template tunable conversionEfficiency (float, (0, 1], default 1) reducing available charging power on DC stations only (currentOutType === DC). Applied at runtime in getConnectorMaximumAvailablePower to both power-derived bounds; amperage and charging-profile limits unchanged; no reduced value persisted. Absent field keeps existing behavior.
fix(ui-server): reconcile evses wire type so /metrics works on OCPP 2.0.x (#2048)
* fix(ui-server): reconcile evses wire type so /metrics works on OCPP 2.0.x
The worker->main producer `buildEvseEntries` emits `evseStatus.connectors`
as a `ConnectorEntry[]` array, but `ChargingStationData.evses` was typed
with the in-memory `EvseStatus` (connectors: Map), hidden by an
`as unknown as EvseEntry[]` cast. On a live scrape the metrics consumer
`iterateConnectors` tuple-destructured each array element as a Map entry,
throwing `TypeError: ... is not iterable` inside prom-client's
`Registry.metrics()` -> HTTP 500 for every OCPP 2.0.x station.
`countConnectors` also read `.size` on the array -> NaN. OCPP 1.6 was
unaffected because its `data.connectors` path already yields ConnectorEntry
objects without destructuring.
Introduce dedicated wire types `EvseEntryData`/`EvseStatusData`
(connectors: ConnectorEntry[]), type `ChargingStationData.evses` and
`buildEvseEntries` with them, and drop the cast so the compiler enforces
the array shape end to end. `iterateConnectors` now yields the entry
directly and `countConnectors` uses `.length`. The array shape matches what
the Web UI and CLI already consume over the JSON wire; a Map would not
survive JSON serialization. The in-memory `EvseStatus` (connectors: Map)
is unchanged.
Add a regression test feeding the actual `buildEvseEntries` output through
the /metrics scrape (fails pre-fix with 500/NaN, passes post-fix) plus
zero-id and empty-evses edge cases, and migrate the existing EVSE-mode test
off its synthetic Map fixture that masked the defect.
Add JSDoc on the wire types introduced for #2046:
- EvseEntryData: anchor its relation to the in-memory EvseEntry (Map) and to
the structurally-identical ui-common EvseEntry (duplicated, no cross-package
re-export), removing the naming ambiguity.
- EvseStatusData: document that MeterValues is intentionally omitted (the
producer never emits it, no UI-facing consumer reads it off the wire),
resolving the PR review comment on the omission.
Address review findings on the #2046 regression tests:
- Convert the "empty evses" test to a populated EVSE with an empty connectors
array. The zero-EVSE case short-circuits `reduce` (returns 0 even pre-fix),
so it guarded nothing; a populated EVSE with `connectors: []` is the case
that pre-fix reads `.size` on an array -> undefined -> NaN. Verified: fails
pre-fix (connectors_total Nan), passes post-fix (0).
- Type the buildEvseEntries stub as `Pick<ChargingStation, 'iterateEvses'>`
instead of `as unknown as`, so `iterateEvses`/`evseStatus` stay type-checked
(resolves the PR review comment on the stub cast).
- Use `satisfies` instead of `as` on the migrated EVSE-mode fixture, so a
Map/array shape drift is caught at compile time (the unchecked-cast class
that masked the original bug).
* refactor(types): make EvseStatusData an immutable wire snapshot
Mark EvseStatusData.availability and connectors readonly (and connectors a
readonly ConnectorEntry[]), matching the readonly EvseEntryData wrapper. The
wire projection is a produced-once snapshot never mutated by any consumer
(countConnectors reads .length, iterateConnectors iterates read-only), so
immutability is the accurate contract and removes the wrapper/payload
readonly asymmetry. In-memory EvseStatus (mutable Map) is unchanged.
N1/INFO - server16.py: add Unix 128+signal comment before sys.exit;
expand boot_index one-liner to full NOTE (shared across ALL stations,
not per-station)
N2 - test_server.py: extend _patch_main with **args_overrides (same
pattern as test_server16.py); add test_boot_status_sequence_parsed in
TestMainGracefulShutdown — covers server.py:1134-1149
N3 - test_server16.py: add test_boot_status_single_value — covers
server16.py:786-787 (elif --boot-status single value branch)
N4 - pyproject.toml: update description to cover both 1.6 and 2.0.1
N5 - README.md: remove duplicated sentence at line 38
N6 - test_server16.py: fix RuntimeWarning (coroutine never awaited)
in test_commands_sequence_scheduled_on_connect — mock_cp.send_commands
was AsyncMock, its coroutine was passed to mocked create_task unrun;
replace with MagicMock() to prevent the orphan coroutine
MAJOR-1 - README.md 2.0.1: add ReserveNow and CancelReservation to
Outgoing Commands list; add --reservation-id, --reserve-id-token,
--reserve-evse-id flags to Command-specific options; add example command
MAJOR-2a - test_server.py: add caplog assertions to 4 signed MeterValue
tests (started/updated/ended/meter_values) — verify _log_signed_meter_values
actually logs, not just that the handler returns correct type
MAJOR-2b - test_server.py: rewrite test_unsupported_command_logs_warning
with caplog + assert 'not supported' in log output
MAJOR-2c - test_server.py: align 7 empty-response handlers on equality
pattern (== EmptyResult()) on top of isinstance — matches test_server16
MINOR-1 - _common.py: add generic parse_commands(str, type[ActionT])
factored from the two identical _parse_commands; both servers now use
a one-line wrapper; remove import math from server16/server (now in
_common only); coverage 96.76% (+0.81pp)
MAJOR-A - README.md 2.0.1: replace invalid --trigger-message value
SignCertificate with the 11 exact MessageTriggerEnumType members
(SignChargingStationCertificate/SignV2GCertificate/SignCombinedCertificate
+ TransactionEvent + PublishFirmwareStatusNotification); the 1.6 list was
fixed in round-3, this is its 2.0.1 counterpart
MAJOR-B - test_server.py: add TestLogSignedMeterValues (3 tests) adapted
to 2.0.1 semantics (signed_meter_value field, not format=SignedData);
covers the previously-uncovered unsigned branch server.py:97->95
MINOR-C - test_server.py + test_server16.py: 6 error-handling tests
(timeout/ocpp_error/unexpected) upgraded from swallowing-only to caplog
assertions on level ERROR + message substring
MINOR-D - DRY: extract negotiate_subprotocol + install_signal_handlers_and_wait
to _common.py (version-agnostic lifecycle, logger passed explicitly to
preserve observability); both servers use them; remove import signal from
both servers; retarget 4 signal.signal patches to _common. Instance methods
(_call_and_log/_send_command/...) and DEFAULT_RESERVATION_* constants left
in place (OCPP coupling / locality cost > DRY gain for a test mock)
MINOR-E - _common.py to 100%: add non-numeric-delay and empty-entry tests
for _parse_commands in both suites
INFO MORT-1 - remove unreachable dead code (if not boot_sequence) in both
servers' main()
* test(ocpp-server): close round-5 parity + config findings
MINOR parity - test_server.py: assert len(config.charge_points) == 0 on
both on_connect rejection tests (missing subprotocol / protocol mismatch),
mirroring test_server16.py; guards against a regression registering a
ChargePoint on a rejected connection in 2.0.1
MINOR parity - test_server.py: strengthen Heartbeat test_returns_current_time
with a freshness window (delta < 60s) + import timezone, mirroring the 1.6
suite (was isinstance + tzinfo only)
INFO I4 - pyproject.toml: mypy python_version 3.14 -> 3.12 to match the
requires-python floor (>=3.12); mypy stays clean (verified live)
INFO I5 - README.md: add the --boot-status shorthand sentence to the 1.6
section for parity with 2.0.1 (identical behavior)
Deliberately NOT changed (accepted debt, evidence-based):
- DRY mixin for send_command/send_commands/handle_connection_closed: moving
them to _common would emit lifecycle logs under _common instead of
server/server16 (silent observability regression) for ~24 lines in a mock
- mypy test override kept: real guard (test files carry untyped defs)
- docstrings on _common helpers: 0/4 documented, keeping style consistent
* test(ocpp-server): close round-6 parity + hygiene findings
MINOR-1 - test_server16.py: add test_parent_id_tag_absent_by_default,
mirroring 2.0.1 test_authorize_no_enrichment_by_default; asserts
parent_id_tag is absent from id_tag_info under the default AuthConfig
(closes the last inter-suite rigor asymmetry)
MINOR-2 - server.py: harmonize _call_and_log docstring with server16.py
(both now '...based on its status')
MINOR-3 - test_server.py: add test_parse_set_variable_specs_skips_empty_entries
and test_parse_get_variable_specs_invalid_no_dot, covering the two
previously-uncovered branches of _parse_variable_specs (empty-entry skip,
require_value=False missing-dot); server.py 875/885 now covered
I1 - server.py + server16.py: derive --auth-mode choices/default from the
local AuthMode StrEnum ([mode.value for mode in AuthMode] / AuthMode.normal.value)
instead of string literals (AGENTS.md: avoid string literals when an
enumeration exists); order/content identical (verified live), no behavior change
* test(ocpp-server): close round-7 style + DRY findings
- test_server.py: fix stale docstring (20 -> 24 _send_* methods)
- server.py/server16.py: --auth-mode type=str -> type=AuthMode to match
the sibling enum-typed CLI args (--boot-status, --reset-type, ...); keep
choices for unchanged help/UX
- server.py/server16.py: extract DEFAULT_WHITELIST/DEFAULT_BLACKLIST module
constants (single source of truth; removes __init__/argparse duplication)
- test_server.py/test_server16.py: TestHandlerCoverage now asserts @on
registration (_on_action) so an undecorated handler is caught
* test(ocpp-server): close round-8 findings (harmonize auth-mode, DRY, handler routing)
- _common.py: host DEFAULT_WHITELIST/DEFAULT_BLACKLIST as the single source
of truth; server.py and server16.py now import them (removes cross-file
literal duplication)
- server.py/server16.py: drop now-dead choices=list(AuthMode) from --auth-mode
and move valid values into the help text, matching the sibling enum-typed
args (--boot-status, --reset-type, ...); uniform invalid-value error message
- server.py/server16.py: mode=args.auth_mode (args value is already an AuthMode
via type=AuthMode; drop redundant re-wrap)
- test_server.py/test_server16.py: TestHandlerCoverage now asserts each handler
is registered for the CORRECT OCPP action (_on_action == expected), not merely
decorated with some @on
* test(ocpp-server): address bot review comments on server16
- test_server16.py: use supported CSMS->CS commands (TriggerMessage/Reset)
instead of ClearCache/Heartbeat in the send_commands sequencing and
_parse_commands tests, so the fixtures reflect commands actually in
_COMMAND_HANDLERS rather than unsupported outgoing actions
- server.py/server16.py: replace the empty `except KeyboardInterrupt: pass`
with contextlib.suppress(KeyboardInterrupt) (clears the empty-except finding
without a comment; both servers for parity)
- server.py/server16.py: _resolve_auth_status returns default_status via an
unconditional trailing return instead of a wildcard `case _`, making the
control flow unambiguous to static analysis (both servers for parity)
Add an opt-in `uiServer.securityHeaders.strictTransportSecurity` config
knob (`string | false`) that, when set to a non-empty string, emits a
`Strict-Transport-Security` (HSTS) response header on the UI server.
Emission is gated on secure transport per RFC 6797 §7.2 (which forbids
sending HSTS over non-secure transport): the header is emitted only when
the response travels over direct TLS (`req.socket.encrypted`) or a trusted
reverse proxy that forwarded a secure protocol (`https`/`wss`), resolved by
a new `isRequestEffectivelySecure` predicate that reuses the access-policy
trusted-proxy/forwarded-protocol machinery. Over plaintext (e.g. loopback
development) the header is omitted.
The header is spread — via `getSecurityHeaders(isSecure)` and, for the
async HTTP success path, a per-uuid `secureResponses` map mirroring
`acceptsGzip` — across every UI-server HTTP response path that this code
writes: shared denials (`renderDenial`, now request-aware), HTTP success
responses (gzip and non-gzip), WebSocket upgrade rejections, and the
`/metrics` success and error responses, on the http/ws/mcp transports. MCP
JSON-RPC success bodies are written by the MCP SDK transport and are not
covered.
Mirrors the `metrics` opt-in sub-schema: one `.strict()` leaf schema, the
derived `z.infer` type, and header spreads. No new dependency; no OCPP
PDU/message-format change (HTTP transport header only).
The header is absent by default (knob unset, `false`, or empty string) and
over non-secure transport, so the default response behavior is unchanged.
Recommended production value behind a TLS-terminating reverse proxy or
native TLS: "max-age=31536000; includeSubDomains".
Closes only slice C of #1980; the identity-aware proxy mode, local-interface
spoofing guard, security audit CLI, and dangerously* naming slices remain open.
test: use shared constants and enums instead of hardcoded literals (#2040)
Replace hardcoded literals in recent tests with the shared constants/enums they duplicate (single source of truth, no behavior change; values are byte-equivalent):
- OCPP20 PostStopResurrection: GenericStatus.Accepted / ReportBaseEnumType.FullInventory instead of raw 'Accepted' / 'FullInventory'
- OCPP20 RequestStartTransaction: OCPP20ChargingRateUnitEnumType.A instead of 'A' as OCPP20ChargingRateUnitEnumType casts
- OCPP16 SmartCharging: TEST_ONE_HOUR_SECONDS instead of the 3600 duration literal
- UIHttpServer: ProcedureName.* instead of raw procedure-name strings
- Remove the now-unused TEST_PROCEDURES test constant (ProcedureName is the canonical source)
The deprecated HTTP UI transport emitted a synthetic `success` the instant the
request handler resolved `undefined` (the broadcast-pending signal), without
awaiting worker replies. PR #2020's Set-based aggregation therefore covered the
WebSocket and MCP transports but not HTTP: a fan-out command reported
`200`/`success` before any worker acted, and the aggregated
`hashIdsFailed`/`responsesFailed` never reached the client.
Align HTTP with the WebSocket and MCP transports (issue #2028 option 1): only
synchronous, non-broadcast procedures respond inline. A broadcast keeps its
already-registered response handler open so the later aggregated `sendResponse`
writes the real status and `hashIdsSucceeded`/`hashIdsFailed`, reusing the
existing 60s safety-net timeout (`UI_SERVER_BROADCAST_CHANNEL_REQUEST_TIMEOUT_MS`)
so a never-arriving aggregation yields a bounded failure with no hung socket.
Client-disconnect cleanup and server-side uuid minting are unchanged; no new
timeout or aggregation path is introduced.
This is a UI SRPC transport-layer fix only; no OCPP PDU/message format changes,
and no change to WebSocket/MCP behavior or the shared aggregation semantics.
feat(worker): terminate a deleted station's worker once it hosts zero elements (#2027) (#2038)
* feat(worker): terminate a deleted station's worker once it hosts zero elements (#2027)
Add an element-granular removeElement primitive to the worker abstraction and
wire it into the station-delete path so a deleted charging station's hosting
worker thread is terminated once it no longer hosts any station, without
disrupting sibling stations that share the worker (elementsPerWorker > 1).
- WorkerAbstract: new abstract removeElement(elementKey: PropertyKey).
- WorkerSet: internal PropertyKey -> WorkerSetElement map fed by an injected
generic elementKey selector; decrement numberOfWorkerElements and reuse a
factored terminateWorker helper (shared with stop()) to terminate only at
zero elements with no in-flight addition; purge the map on element removal.
- WorkerFixedPool/WorkerDynamicPool: documented no-op (poolifier exposes no safe
single-element eviction without destroying the worker and its siblings).
- Bootstrap: pass stationInfo => stationInfo.hashId; call removeElement from
workerEventDeleted.
Set-vs-pool decision: element-granular termination is workerSet-only; pool
worker threads are a bounded, reused resource and are not reclaimed per delete.
Cleanup/timeout contract: termination reuses the existing worker exit handler
for pending-promise rejection and set/map cleanup; any in-flight broadcast
request still resolves via the existing 60s aggregation timeout backstop.
Part A (#2031 cancellable reset) and bulk stop() are untouched.
* fix(worker): harden element-granular termination against concurrency races
Cross-validated review of the removeElement primitive surfaced two reachable
defects and one latent robustness issue, now fixed:
- Phantom-count leak on re-adding the same element key: numberOfWorkerElements
was incremented unconditionally, so a duplicate/re-added key inflated the
count and the worker was never terminated. Count is now key-aware (distinct
keys per worker); factored into trackAddedWorkerElement.
- New addition routed onto a terminating worker: getWorkerSetElement could
select a worker parked in terminateWorker, losing the message and rejecting
an unrelated add. Terminating workers are now flagged and excluded from
selection.
- getWorkerSetElementByWorker matched by threadId, which collapses to -1 after
terminate(); switched to worker object identity.
Tests: add coverage for the in-flight-sibling gate, drain-to-last-sibling,
add-during-terminate, same-key re-add, and unknown-key no-op; consolidate the
silent worker fixture into echoWorker via a `hold` flag. Mutation-verified.
* fix(worker): make element termination robust and tighten its types
Second cross-validated review round hardened the removeElement primitive:
- terminateWorker no longer awaits a re-attached 'exit' listener (redundant,
since worker.terminate() fulfils once the worker has exited, and it could
deadlock if 'exit' fired first). It now catches a rejected terminate()
(surfaced via the error event, not rethrown so the removal still succeeds)
and performs pending-promise rejection + set/map cleanup in a finally, closing
the zombie-on-reject leak, the orphaned-pending edge, and the stop-concurrent-
exit deadlock in one place.
- Drop the dead, semantically fictional migration branch in
trackAddedWorkerElement: duplicate live element keys cannot occur (identities
are deduplicated upstream), so a key is counted once per worker and a same-key
re-add is a no-op.
- Make WorkerSetElement.terminating a required boolean (initialized false) and
narrow the element key from PropertyKey to string, matching the sole caller
(hashId) and avoiding non-serializable keys.
Tests: add reject-path coverage (removeElement resolves and cleans up when
terminate() rejects; an in-flight addition is still rejected on stop when
terminate() rejects). Mutation-verified. All project gates green.
* test(worker): align WorkerSet internals view element key type to string
Coherence with the production narrowing of the element key from PropertyKey
to string; test-only, no runtime change.
* docs(worker): drop the worker-termination note from the README
Client-supplied WebSocket UI request ids were validated for format only.
A second request reusing a still-in-flight id overwrote the prior request's
response handler (cross-delivered reply, dropped second response) and its
broadcast tracking (leaked safety-net timer firing against the wrong context).
Guard the transport ingest: when responseHandlers already holds the request id,
reject the duplicate with a typed BaseError failure on its own socket instead of
overwriting the in-flight request. A completed request releases its handler, so a
legitimate sequential reuse of the same id is still accepted. HTTP and MCP mint
server-side UUIDs and are unaffected.
Wrap the ws.send in rejectInFlightRequestId() in try/catch, matching the
sendResponse() send discipline. The helper runs in the synchronous 'message'
listener, so an uncaught send throw would escape unhandled; log and swallow it
instead. No behavior change to the in-flight guard.
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
* test(ui-server): fail parseSentResponse with a descriptive assertion
When no message is captured at the requested index, fail with an explicit
assertion naming the index and sentMessages length instead of letting
JSON.parse throw an opaque SyntaxError. Use a length check rather than a
nullish guard, since MockWebSocket.sentMessages is typed string[] (indexed
access is not nullable under the project's tsconfig).
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
* test(ui-server): reuse shared UI-server test helpers and constants
Hoist emitWorkerResponse into UIServerTestUtils as the single source of truth
and consume it from both AbstractUIService and UIWebSocketServer tests, removing
the duplicated worker-response injection helper (with divergent argument order).
In the WebSocket test, build the protocol request via createProtocolRequest
instead of an inline tuple, and drop the redundant explicit 'ui0.0.1' argument
(it is createMockUIWebSocket's default).
fix(ui-server): dedup duplicate station identity on add (#2026) (#2032)
* fix(ui-server): dedup duplicate station identity on add (#2026)
hashId is a deterministic content hash of template identity fields + composed
chargingStationId, with no uniqueness check on add. Two stations from a
template with identical identity fields (e.g. fixedName: true) collide on one
hashId. AbstractUIServer.setChargingStationData was last-writer-wins by
timestamp keyed by hashId, so the twin silently overwrote the first station in
the UI registry; a targeted broadcast command then completed success on the
first worker's reply while the collided twin was orphaned.
Add post-creation identity dedup at the registry write choke point:
setChargingStationData returns a discriminated 'set' | 'stale' | 'collision'
outcome. A write whose hashId already maps to a station with a different
templateIndex is rejected as 'collision' without overwriting, guarding every
worker event (added/started/stopped/updated) uniformly. A restart re-emit
reuses the same templateIndex and still updates by timestamp, so there is no
regression. workerEventAdded logs the rejected collision at error level
instead of silently reporting the twin as added.
getHashId output is unchanged (byte-identical), so persisted <hashId>.json
configuration, the registry map key, broadcast routing and stats resolve
untouched. No OCPP PDU / message-format change; simulator station-registry
lifecycle only.
Tests: a twin (same hashId, different templateIndex) is rejected and does not
overwrite; a restart re-emit (same identity, newer timestamp) still updates; a
stale same-identity re-emit is dropped; a targeted broadcast reports no false
success after a collision. Mutation-verified: reverting the guard fails the
twin/false-success tests while the restart/stale tests stay green.
Duplicate-identity aggregation rework and hash-algorithm / per-instance-salt
changes are out of scope (per issue). Causally linked to #2027: an orphaned
twin is exactly the worker #2027 cannot individually terminate.
Closes #2026.
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ui-server): discriminate station identity by (templateName, templateIndex) (#2026)
Strengthen the add-time identity dedup after a multi-reviewer cross-validation.
The initial guard discriminated on templateIndex alone, but getHashId does not
hash the template file name and the index space is per-templateName: two
identity-clone template files (copy + rename, both fixedName, same baseName and
identity fields) can produce the same hashId AND the same templateIndex. The
templateIndex-only guard treated them as the same station and silently
overwrote one, reintroducing the last-writer-wins bug for cross-template
collisions.
Discriminate on the full physical-station identity, the (templateName,
templateIndex) pair: a cached entry differing in either field is a collision. A
legitimate restart re-emit keeps both fields and still updates by timestamp (no
regression). Sync the SetChargingStationDataOutcome doc to the pair, add
templateName to the rejection log so an equal-index clone collision is
diagnosable, and add a test for the same-templateIndex / different-templateName
case.
Mutation-verified: reverting the templateName clause fails only the new
clone-file test while the same-template twin, restart, stale, and
false-success tests stay green.
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
---------
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
fix(charging-station): cancel a pending reset when the station is deleted (#2027) (#2031)
reset() stopped the station, slept resetTime, then re-initialized and
reconnected without checking whether the station had been deleted during
the sleep. A station deleted mid-reset was resurrected and reconnected to
the CSMS (the "zombie" reconnect that triggers #2017).
Add an instance AbortController tripped by delete(); reset() now awaits
interruptibleSleep(resetTime, signal) and rechecks the aborted state
before re-initializing, bailing out cleanly on abort. The dispose signal
is kept distinct from started/stopping, which reset() itself toggles via
stop() and therefore cannot be used to detect deletion.
The OCPP Reset response stays Accepted and the handler still invokes
reset() fire-and-forget; only the reset/delete lifecycle changes.
Part B (element-granular worker-thread termination) remains open.
Daniel [Fri, 17 Jul 2026 22:17:54 +0000 (00:17 +0200)]
fix(ui-server): make UI broadcast-channel commands complete reliably (#2018) (#2020)
* fix(ui-server): time out broadcast-channel requests so UI commands cannot hang
A UI control command (start/stop/delete a station) is dispatched over the
worker broadcast channel and the UI service waits for a fixed number of
worker responses, sampled at send time. If a targeted worker never replies
-- e.g. the station is deleted while the command is in flight -- that count
is never reached, so the request is never completed or released and the
client waits forever. Read commands keep working, which masks the wedge.
Arm a per-request safety-net timeout when a broadcast-channel request is
sent. On expiry the request is completed with a failure (reporting the
charging stations that did reply successfully) and both the request and
response aggregation state are released, so the caller gets an answer
instead of hanging. The timeout is cleared on normal completion, on a
dispatch failure, and on service stop.
Closes #2018.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel <7558512+DerGenaue@users.noreply.github.com>
* fix(ui-server): complete broadcast requests by target set and reconcile on station deletion (#2018)
Broadcast-channel request completion was gated on a frozen integer count
sampled at send time, blind to which stations were targeted. When the set
of stations changed mid-flight the aggregator could not reconcile the
drift, producing failure modes on top of the hang the safety-net timeout
already backstops:
- Hang until timeout when a targeted station is deleted mid-flight (the
count is never reached).
- Late double-completion: after release the count reads 0, so a late reply
re-enters and 1 >= 0 re-completes the request.
- False success: an empty explicit hashIds array degraded to a
broadcast-to-all instead of failing.
Track the outstanding responders as a Set of target hashIds and make the
Set the single source of truth for completion:
1. Snapshot the resolved targets (validated explicit hashIds, or the live
station set for a broadcast) into the request context; complete when the
Set empties. AbstractUIServer.deleteChargingStationData fans out
AbstractUIService.reconcileDeletedStation, which drops the departed
hashId from every in-flight request and completes any that empty with
the truthful aggregated payload. A DELETE_CHARGING_STATIONS request is
left untouched by reconciliation: its targets self-delete yet still post
their command reply on a separate transport, so that reply (not the
racing `deleted` event) is the completion source; a worker that dies
mid-delete is covered by the timeout.
2. Drop untracked responses (unknown/released request, or a hashId that is
not an outstanding responder) in the response handler, closing the late
double-completion re-entry. A reply without a hashId for a still-tracked
request is dropped and logged distinctly so the resulting timeout is
diagnosable.
3. Reject unknown and empty explicit targets instead of degrading to a
broadcast-to-all.
The 60 s safety-net timeout stays as a pure backstop for genuine worker
crash/deadlock; normal and reconciled completion clear its timer. The
completion accessor is named getBroadcastChannelOutstandingResponseCount to
reflect that it returns the responses still outstanding, not a frozen total.
Duplicate-identity false success and orphan-worker termination are not
addressed here (a hashId-keyed set cannot disambiguate two workers sharing
an identity) and are deferred to follow-up changes. The deprecated HTTP
transport bypasses aggregation and is likewise out of scope.
Tests cover reconcile-on-delete, the untracked-response guard, unknown and
empty target rejection, the DELETE self-target ordering (reconcile-first
completes via the reply; no reply falls back to the timeout), reconcile
that does not empty the set, two concurrent requests reconciled by one
deletion, a hashId-less reply, and the previously untested timeout
behaviors (partial-success payload, dispatch-failure and stop() timer
clearing, and the completion-race guard).
Closes #2018.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
---------
Signed-off-by: Daniel <7558512+DerGenaue@users.noreply.github.com> Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Jérôme Benoit <jerome.benoit@sap.com>
fix(ocpp): reject charging profiles by EVSE existence and isolate non-persistent station configuration (#2022, #2024) (#2025)
* fix(ocpp): reject charging profiles by EVSE existence, not EVSE count (#2022)
OCPP 2.0 validateChargingProfile() guarded evseId with a count comparison
(evseId > getNumberOfEvses()). getNumberOfEvses() counts EVSEs (excluding
EVSE 0) and is not a maximum id, so a station with non-contiguous EVSE ids
{0,1,3} false-rejected a valid profile for EVSE 3 (3 > 2). Use the existing
hasEvse() existence check instead, mirroring the OCPP 1.6 hasConnector twin,
and align the message to "EVSE does not exist".
RequestStartTransaction carries the TxProfile (F01.FR.08-10 / F02.FR.16-18);
existence-based rejection mirrors K01.FR.28 (conformance test TC_K_14_CS).
Test: a charging profile for the existing, non-contiguous EVSE 3 is accepted
(a count-based guard would reject it, 3 > 2). Mutation-verified: reverting the
guard makes the test fail.
getOcppConfigurationFromTemplate() returned the SharedLRUCache-cached
template's Configuration by reference. For non-persistent stations
(ocppPersistentConfiguration=false) this aliased the shared configurationKey
array, so an in-place setConfigurationKeyValue() mutation polluted the cached
template. Clone the Configuration, mirroring the existing connector/EVSE clone
pattern (clone(undefined) === undefined).
Test: a non-persistent station's Configuration is an independent copy of the
cached template, and mutating a key on it leaves that cached template unchanged.
Mutation-verified: reverting the clone makes the test fail. Retires the
now-redundant SharedLRUCache-clearing afterEach workaround in
ChargingStation-ResetIdentity.test.ts (file still passes).
Daniel [Fri, 17 Jul 2026 17:10:37 +0000 (19:10 +0200)]
fix(charging-station): retain creation options so a reset keeps station identity (#2019)
* fix(charging-station): retain creation options so a reset keeps station identity
On an OCPP Reset (and on a template-file reload) the station was
re-initialized via initialize() with no arguments, dropping the
creation-time options. The restarted station reverted to the template
defaults, losing its fixed identity (reappearing as CS-BASIC-000N with a
new hashId) and its configured supervision URL; the original station
never came back.
Retain the creation options on the instance and re-pass them in reset()
and in the template-watcher reload. setSupervisionUrl() also updates the
retained snapshot so a runtime supervision-URL change survives a reboot
(but not a full simulator restart, which reconstructs the worker from the
original options). The snapshot is in-memory only: it is never persisted
and never written to the template.
Closes #2017.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel <7558512+DerGenaue@users.noreply.github.com>
* fix(charging-station): clone the retained creation options
setSupervisionUrl updates the retained options in place so a later reset
re-applies the current supervision URL. Clone them at construction so that
in-place update never mutates the shared worker data passed from the worker
thread.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel <7558512+DerGenaue@users.noreply.github.com>
* test(charging-station): cover identity retention across a reset
A non-persistent station keeps its configured identity across a reset only
when the creation options are re-applied; a persistent one restores it from
its saved configuration without them. This locks the behavior and scopes the
options fix to the non-persistent case.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel <7558512+DerGenaue@users.noreply.github.com>
* refactor(charging-station): re-apply retained options on reset only when non-persistent
Address review feedback on the identity-retention fix:
- Rename the retained-options field to creationOptions (clearer; no longer
shadows the options parameters).
- Re-apply the creation options on reset/template-reload only for a
non-persistent station, via the reinitializeOptions getter. A persistent
station restores from its saved config, which stays the source of truth, so
re-applying them would needlessly override it (and collapse an OCPP-config
station's supervision URL). The unconditional setSupervisionUrl mirror is kept,
with a sharpened comment explaining why it must run for both branches.
- Cover the reset -> initialize wiring (both persistence modes) and the
setSupervisionUrl-across-reset retention with unit tests.
No change to the fixed behavior: a reset still keeps the configured identity.
* refactor(charging-station): factor setSupervisionUrl credential updates into a helper
Deduplicate the supervisionUser/supervisionPassword null-checks that were
applied twice (to stationInfo and to the retained creation options) into a
single applyCredentials() closure. No behavior change.
* test(charging-station): cover OCPP-config supervision URL retention across reset
- Add a regression test for the non-persistent supervisionUrlOcppConfiguration
path: after setSupervisionUrl(), a real reset() must still dial the new URL
(the branch the setSupervisionUrl mirror comment reasons about, previously
untested).
- Prefix all test titles with "should" per tests/TEST_STYLE_GUIDE.md.
- Clear the real SharedLRUCache singleton in afterEach so a cached template
cannot bleed between tests.
- Support optional template field overrides in the makeTemplate helper.
* refactor(charging-station): rename reinitialization getter and fix its mirror comment
- Rename the reinitializeOptions getter to reinitializationOptions (noun form,
consistent with the wsConnectionUrl/hasEvses getters).
- Correct the setSupervisionUrl mirror comment: the retained-options mirror is
load-bearing on a cache-cold reinitialization (template reload), not on a plain
warm-cache reset() (whose in-place OCPP-key cache mutation already carries the
URL). No behavior change.
* test(charging-station): add a genuine OCPP-config supervision-URL reload guard
The previous OCPP-config test drove a warm-cache reset() and passed even with the
retained-options mirror deleted (a false guard): the cached template still carried
the mutated OCPP key. Add a cache-cold reload test (clear SharedLRUCache, then
reinitialize) that re-seeds the key from configuredSupervisionUrl and therefore
fails if the mirror is removed. Relabel the warm-cache reset test and correct its
comment to reflect that it exercises the cache-mutation path, not the mirror.
* docs(charging-station): tighten the setSupervisionUrl mirror comment
Condense the retained-options mirror comment (~11 -> 8 lines) while preserving
every load-bearing detail: it runs for both branches, a cache-cold reload
re-seeds the OCPP key from configuredSupervisionUrl, a warm reset() survives via
the cached template mutation, and the mirror is in-memory only.
---------
Signed-off-by: Daniel <7558512+DerGenaue@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Jérôme Benoit <jerome.benoit@sap.com>
Daniel [Fri, 17 Jul 2026 15:22:18 +0000 (17:22 +0200)]
fix(charging-station): re-dial after a server-initiated connection close (#2021)
* fix(charging-station): re-dial after a server-initiated connection close
When the CSMS or a proxy closed a station's WebSocket with a normal
(clean) close, the station stayed "started but not connected" instead of
reconnecting. onClose only re-dialed on abnormal close codes and treated
clean closes as terminal, but a clean close cannot be told apart by code
from the station's own closeWSConnection() (the UI disconnect action,
which must stay down). Real charge-point hardware re-dials after losing
the connection regardless of the close code.
Mark only explicitly-requested closes (the UI disconnect) as terminal via
a byRequest flag on closeWSConnection(), and reconnect on any close the
station did not request while it is still started -- clean or abnormal.
This also fixes certificate rotation, which closes the socket to force a
re-dial with the new certificate and previously never reconnected.
Closes #2016.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel <7558512+DerGenaue@users.noreply.github.com>
* refactor(charging-station): pass closeWSConnection intent via an options object
Match the paired openWSConnection signature and make the call site
self-documenting: closeWSConnection({ byRequest: true }) rather than a bare
positional boolean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel <7558512+DerGenaue@users.noreply.github.com>
* test(charging-station): cover the onClose reconnect decision
Drive onClose directly with a spied reconnect: the station re-dials after a
server-initiated normal close while started, and stays disconnected after an
operator-requested close. Removes a stale test that only asserted the socket
reached CLOSED, which held regardless of the reconnect decision.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel <7558512+DerGenaue@users.noreply.github.com>
* docs(charging-station): harmonize reconnect terminology and tighten close comments
- Use the codebase's "reconnect" term consistently (drop "re-dial"), and
"server-initiated"/"requested" to match the PR title and the byRequest option.
- Tighten the closeWSConnection JSDoc and onClose comments; align the JSDoc
@param style (no trailing period) with the sibling openWSConnection.
- Prefix the reconnect test titles with "should" per tests/TEST_STYLE_GUIDE.md.
No behavior change.
---------
Signed-off-by: Daniel <7558512+DerGenaue@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Jérôme Benoit <jerome.benoit@sap.com>
fix(ocpp): support TriggerMessage(TransactionEvent) per F06.FR.07 (#2013)
TriggerMessage(requestedMessage=TransactionEvent) returned NotImplemented
although TransactionEvent is a fully implemented outgoing message, violating
OCPP 2.0.1 F06.FR.07/08.
Wire the trigger to the existing TransactionEvent send path across the two
touchpoints:
- handleRequestTriggerMessage: add a TransactionEvent case that validates the
evse, returns Accepted when an ongoing transaction exists in scope, else
Rejected (F06.FR.05); it no longer falls through to NotImplemented.
- TRIGGER_MESSAGE dispatch listener: add a TransactionEvent case that sends a
TransactionEventRequest(eventType=Updated, triggerReason=Trigger) with the
TxUpdatedMeasurands meterValue for each active transaction in scope; when the
evse field is absent it fans out to all EVSEs (F06.FR.11).
Reuses sendTransactionEvent and the TxUpdatedMeasurands meter-value builder;
chargingState is populated by buildTransactionEvent for Updated events. The 6
existing trigger cases are unchanged.
- Backtick 49 JSDoc @returns boolean literals across 17 files (lowercase True/False).
- Align !== undefined -> != null on number|undefined guards in OCPP20IncomingRequestService.ts (13 tokens); conditional-spread inclusion guards preserved as !== undefined.
- Extend != null idiom to OCPP20VariableManager min/max ternaries + presence guards and the negated EVSE guard.
- Fix OCPP20AuthAdapter.validateConfiguration @returns (synchronous boolean; was falsely 'Promise resolving to').
Gap C (String(previousRequestId) -> .toString()) dropped: premise false — operand is number|undefined, .toString() would fail typecheck (TS18048); String() is the only behavior-preserving form.
All gates green (format/typecheck/lint/build/test, fail=0). Reviewed across multiple multi-agent rounds with cross-validation.
refactor(ocpp): remove as-unknown-as double-casts from request/response dispatch (#2003)
Give each builder its precise input contract instead of laundering casts
through phantom generics or JsonType:
- buildRequestPayload drops its unsound <Request> generic and returns honest
JsonType; each dispatch branch narrows with a single cast at a genuine
JsonType boundary.
- Builder inputs (StatusNotificationOptions, OCPP20TransactionEventOptions,
SignCertificateOptions) are JsonObject subtypes; StatusNotification build
validates connectorStatus via the shared isOCPP20ConnectorStatus guard.
- Handler bridges (toRequestHandler/toResponseHandler) use a single cast that
preserves the handler's async identity, fixing an unawaited-handler
regression from a plain wrapper defeating isAsyncFunction.
- CAT-C string-to-enum narrowing becomes a type guard / typed Record.
refactor(utils): swap getRandomFloat(Rounded) params to (min, max) for consistency (#1998)
Swap getRandomFloat and getRandomFloatRounded params from (max, min[, scale]) to (min, max[, scale]) for consistency with the sibling bounds helpers randomInt (node:crypto) and isValidRandomIntBounds; migrate all 11 production + 5 test call sites; harmonize JSDoc across the random-float family.
Bundles a correctness fix: getRandomFloat now rejects a non-finite interval width (max - min), closing a latent overflow where finite endpoints such as (-MAX_VALUE, MAX_VALUE) returned Infinity/NaN, silently violating the documented [min, max] contract.
Internal API refactor: package.json exports only ./dist/start.js, so getRandomFloat is app-internal and this is not a semver-major public break.
fix(ocpp): guard OCPP 1.6 UpdateFirmware deferred-timer schedule against sealed state writes (#1994)
The .on(UPDATE_FIRMWARE, ...) listener's deferred branch obtains state via
getOrCreateStationState() and then writes stationState.deferredFirmwareUpdateTimer
= setTimeout(...). After PR #1992 sealed the base-plumbing behavior,
getOrCreateStationState() returns the stopped state instead of lazy-initializing
a fresh entry once stop() has marked stationsState.get(cs)?.stopped === true. A
late UPDATE_FIRMWARE dispatch after ChargingStation.stop() runs
ocppIncomingRequestService.stop(this) at ChargingStation.ts:1273 (before
this.started = false at :1280) therefore writes a fresh Timeout onto the sealed
state.
The callback body is safe (unref'd, checkChargingStationState gate), but the
write is inconsistent with the OCPP 2.0.1 GUARD sites shipped in PR #1992
(getCertSigningRetryManager, sendSecurityEventNotification,
simulateFirmwareUpdateLifecycle, simulateLogUploadLifecycle) and leaves a Timeout
closure-referenced entry pending in Node's queue until GC reclaims the
ChargingStation.
Insert a stopped GUARD immediately after getOrCreateStationState, before
cancelDeferredFirmwareUpdate — mirroring the exact syntax of the OCPP 2.0.1
GUARD sites (bare 'return' after 'if (stationState.stopped === true) {'). The
GUARD short-circuits the schedule branch before any write reaches the sealed
state.
Add a regression test covering the post-stop() deferred-schedule path in
tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-Firmware.test.ts.
References:
- PR #1992 — source of the 'stopped' sentinel + OCPP 2.0.1 GUARD sites
- PR #1984 — source of 'deferredFirmwareUpdateTimer'
This is purely additive; no observable OCPP behavior change (the pre-existing
fire-time checkChargingStationState gate inside the setTimeout callback remains
as defense-in-depth).
fix(ocpp): guard post-stop() handler dispatch against WeakMap resurrection (#1992)
Closes #1970
OCPP 2.0.1 handlers dispatched from queued microtasks, unref'd retry
timers, or EventEmitter-dispatched `.on(...)` listeners could run
AFTER `stop()` and resurrect the `stationsState` WeakMap entry via
`getOrCreateStationState` lazy-init. `sendQueuedSecurityEvents` also
scheduled a retry `setTimeout(...)` whose handle was never stored, so
`stop()` had no way to cancel a pending retry, and the recursive
retry callback re-entered `sendQueuedSecurityEvents` — resurrecting
state.
Three coordinated parts.
Part 1 — base plumbing (`OCPPIncomingRequestService.ts`):
- Generic bound widened to
`TStationState extends { stopped?: boolean } = { stopped?: boolean }`.
- `stop()` template: after `resetStationState`, marks
`stationState.stopped = true` instead of `stationsState.delete(cs)`.
Deletion would re-enable resurrection via `getOrCreateStationState`
lazy-init on any late dispatch. Keeping the sealed entry lets the
getter return the sealed state and raw `.get()` null-guarded callers
observe the marker and drop. The WeakMap entry is naturally
collected when the ChargingStation reference is dropped.
- `getOrCreateStationState` returns the sealed stopped state when
`state?.stopped === true`; no fresh entry is lazy-init'd post-stop.
- Both concrete state interfaces (`OCPP16StationState`,
`OCPP20StationState`) get an alphabetized `stopped?: boolean` field.
Part 2 — OCPP 2.0.1 call-site audit (11 sites):
Two sites converted to raw `stationsState.get(cs)` + `stopped === true`
null-guard, silent-drop:
- `sendNotifyReportRequest` — dispatched from
`.on(GET_BASE_REPORT).catch(...)` microtask; state usually created
by sibling `handleRequestGetBaseReport` but external `emit(...)`
paths and post-stop dispatch are covered by the null-guard.
- `sendQueuedSecurityEvents` — dispatched from unref'd retry
`setTimeout`; guard is the last line of defense against a
fired-before-cancel race.
Four sites use `getOrCreateStationState` + explicit `stopped` check
(state may not exist yet on the pre-stop happy path):
- `sendSecurityEventNotification` — lifecycle-entry on the
invalid-cert-on-first-request edge case
(`handleRequestCertificateSigned` X.509-invalid branch,
`handleRequestUpdateFirmware` invalid-PEM branch).
- `getCertSigningRetryManager` — public accessor; return type widened
to `OCPP20CertSigningRetryManager | undefined`; two callers
(`handleRequestCertificateSigned`, `OCPP20ResponseService`) updated
with `?.` to tolerate the undefined return. Without this guard, a
late `handleResponseSignCertificate` dispatch (a SignCertificate
response can arrive between `ocppIncomingRequestService.stop(cs)`
and `closeWSConnection()` in `ChargingStation.stop()`) would
materialize a fresh manager on the sealed state and schedule a
non-`.unref()`'d retry `setTimeout` holding the ChargingStation
reference.
- `simulateFirmwareUpdateLifecycle` — dispatched from
`.on(UPDATE_FIRMWARE).catch(...)` microtask; guards against driving
the full async lifecycle on a sealed state (which would send OCPP
requests through a closed WebSocket and clobber the abort-controller
fields).
- `simulateLogUploadLifecycle` — symmetric,
`.on(GET_LOG).catch(...)`.
Part 3 — retry timer discipline in `sendQueuedSecurityEvents`:
- Store the retry `setTimeout` handle on
`OCPP20StationState.securityEventRetryTimer`.
- Cancel any previously scheduled retry before scheduling a new one.
- Self-clear the handle before the recursive
`sendQueuedSecurityEvents` call inside the callback.
- New `cancelSecurityEventRetryTimer` helper is called from
`resetStationState` (after abort-controller aborts and cert-signing
retry cancel, before the two `resetActive*State` helpers).
- `.unref()` preserved.
Design rationale — silent-drop over `StationStoppedError` throw:
matches the "post-stop silently drop" semantic and avoids catch-block
sprawl at every handler.
Design rationale — return `undefined` from `getCertSigningRetryManager`
over convert-with-explicit-null-manager: two callers already handle
optional chaining cleanly; return-type widening is the minimum change.
Verification: pnpm format / typecheck / lint pass; pnpm test 3029
pass, 0 fail; pnpm build exit 0. No `stationsState.delete` remains in
`src/`; `securityEventRetryTimer` lives in exactly three logical
places (interface field, retry-site store + self-clear, cancel helper
called from `resetStationState`).
fix(ocpp): implement OCPP 1.6 GetDiagnostics supersession via AbortController (#1991)
Closes #1971
The entry guard added by PR #1987 at handleRequestGetDiagnostics
prevented concurrent FTP uploads racing on the same
${chargingStationId}_logs.tar.gz archive by silently dropping the
second GetDiagnostics.req with an empty GetDiagnostics.conf. That
mitigated the concrete race but was not the OCPP 1.6 supersession
semantic: the new request needed to abort the in-flight upload and
start a fresh one, with a terminal DiagnosticsStatusNotification
emitted for the superseded upload.
Refactor the entry guard from skip-second into abort-then-install:
- Add `activeDiagnosticsAbortController?: AbortController` to
`OCPP16StationState` (alphabetical, above the existing fields).
- On supersession, the entry guard calls
`stationState.activeDiagnosticsAbortController?.abort()` and
installs a fresh controller. `diagnosticsUploadInProgress` stays
`true` across the handoff so the §4.4 / §7.24 trigger cross-check
never observes a false-idle window.
- `basic-ftp` v6 does not natively consume `AbortSignal`; the
documented interruption path is `Client.close()`, which invokes
`FtpContext.close()` and rejects the pending `uploadFrom` task
with `User closed client during task`. The handler wires
`signal.addEventListener('abort', () => ftpClient?.close(), { once: true })`.
- The existing `catch` at L1390 already emits
`DiagnosticsStatusNotification(UploadFailed)` for any thrown error,
so a close-triggered rejection flows through unchanged — no second
emission needed. OCPP 1.6 `DiagnosticsStatusNotification.req` (§6.17)
does not carry a `requestId`; the terminal for the superseded upload
is uncorrelated by design, only its temporal position tells the CSMS
which upload it belonged to.
- The `finally` clause identity-guards its cleanup
(`if (stationState.activeDiagnosticsAbortController === abortController)`)
so a superseded handler's late unwind cannot clobber the new
lifecycle's state. Cleaner than the microtask-yield primitive
sketched in the initial design because it removes the ordering
dependency between the two handlers' async continuations.
- `resetStationState` aborts the in-flight controller before the base
template deletes the WeakMap entry, following the cancel-before-delete
ordering documented on `OCPP20IncomingRequestService.resetStationState`.
Mirrors the OCPP 2.0.1 log-upload supersession pattern (commits 29a8330f, ac6ed218, 64f384fa, be29b4c8) but does not import
`AcceptedCanceled` — that response semantic is a 2.0.1 concept absent
from OCPP 1.6 `GetDiagnostics.conf` (§6.26).