Audit pass over the describe('buildStatusNotificationPayload') block
added by PR #1902. Adds the only two production-callsite-tied cases the
existing 9 tests miss. No production code change.
T2.1 — OCPP 1.6 with errorCode and evseId together:
- Tied to ui/cli/src/commands/ocpp.ts:191-203 (1.6 requires --error-code,
passes evseId) and ui/web/src/skins/classic/components/charging-stations
/CSConnector.vue:172-173 (selectedErrorCode defaults to NO_ERROR).
- Existing tests cover errorCode-without-evseId and evseId-without-
errorCode but never both flags together — the actual production path.
T2.2 — undefined ocppVersion with errorCode and evseId:
- Tied to ui/web/src/core/UIClient.ts:141-155 +
ui/web/src/shared/composables/useConnectorActions.ts:113-122 where
setConnectorStatus is unconditionally invoked with both options even
when props.ocppVersion is OCPPVersion | undefined.
- Existing 'undefined version' test omits both options; this asserts the
1.6 fallthrough preserves them.
3-angle audit found other proposals (cross-enum mismatch guard, marking
existing cases [SYNTHETIC], structural refactor into nested describes)
but they did not survive cross-validation: the first requires a runtime
guard in payloadBuilders.ts (out of scope per task spec), the second is
subjective, the third is cosmetic per AGENTS.md 'small verifiable
changes'.
* test(ui-common): align buildStatusNotificationPayload test names with style guide
Address review feedback on #1904.
Both new test names violated tests/TEST_STYLE_GUIDE.md "should [verb]
in lowercase" describing observable behavior:
- T2.1 carried a callsite traceability annotation
'(CLI + Web UI 1.6 path)' in the title.
- T2.2 used the implementation-internal term 'fall through' plus a
'(Web UI undefined-version path)' annotation.
Renamed to mirror the existing sibling tests in the same describe
block:
- T2.1: 'should pass errorCode and evseId through for OCPP 1.6 when
both are provided' (mirrors 'should pass evseId through for OCPP 1.6
without errorCode').
- T2.2: 'should default to OCPP 1.6 shape with errorCode and evseId
when ocppVersion is undefined' (mirrors 'should default to OCPP 1.6
shape when ocppVersion is undefined').
Callsite traceability lives in the commit message + PR body, not the
test title — matches the convention of all 9 prior cases in the same
describe. No assertion change. 116/116 tests pass.
Jérôme Benoit [Mon, 15 Jun 2026 19:20:06 +0000 (21:20 +0200)]
fix(config): enforce Zod simulator-config validation at boot (#1874) (#1903)
* fix(config): enforce Zod simulator-config validation at boot (#1874)
Audit pass over uiServer.* primitives that PR #1901 left under-constrained.
Each constraint closes a concrete fail-fast gap surfaced by a 3-angle design
review (schema completeness, fail-fast wiring, adversarial risk).
UIServerAuthenticationSchema:
- username: z.string().min(1).optional() — empty username currently passes;
AbstractUIServer falls back to '' and authenticates against the empty
configured value, silently bypassing Basic-Auth.
- password: z.string().min(1).optional() — same hole on password.
- username .refine(no ':') (RFC 7617) — lifts the runtime check at
UIServerFactory.ts:48-54 into the schema layer.
- object-level .refine: enabled === true requires both username and password.
Closes the silent-bypass when authentication is enabled with no creds.
UIServerAccessPolicySchema:
- object-level .refine: allowLoopbackProxy === true requires
trustedProxies.length >= 1. With allowLoopbackProxy: true and an empty
trustedProxies list, the proxy-trust check at runtime always evaluates
against the empty allowlist, contradicting the operator's intent.
No wiring change: Configuration.ts:160 (boot, process.exit(1)) and
Configuration.ts:386-431 (hot-reload, rollback) already route
ConfigurationValidationError correctly. Schema tightening lands at parse
time without rewiring.
Test fixture update: tests/utils/ConfigurationSchema.test.ts unknown-key
test for uiServer.authentication now includes valid username/password
alongside bogusAuthKey so the unknown-key intent stays unambiguous after
the new object-level superRefine.
9 new test cases (5 reject + 3 accept regression-anchors + 1 cross-field
reject) covering rejection paths and the docker/config.json-shape
acceptance.
Designs that did NOT survive cross-validation are documented as
out-of-scope follow-ups: type×version coupling (coupled to in-place
mutation bug at UIServerFactory.ts:69), type×auth coupling (defense in
depth at runtime is intentional), options.{path,readableAll,writableAll,
ipv6Only} refinements (no shipped config exercises these), and the
TLS-non-loopback wildcard refine (would break docker/config.json:18).
* fix(config): refine auth schema error path and tighten test path checks
Address review feedback on #1903:
src/utils/ConfigurationSchema.ts
- UIServerAuthenticationSchema object-level .refine now carries
path: ['username'], producing an error path of
'uiServer.authentication.username' rather than 'uiServer.authentication'.
Mirrors the path: ['allowLoopbackProxy'] convention already used on
UIServerAccessPolicySchema and yields a more actionable message.
- JSDoc clarifies that the field-level constraints (min(1), no ':')
apply unconditionally even when authentication.enabled is false:
the schema is intentionally stricter than the runtime guard in
UIServerFactory so a credentials value cannot ship under
enabled: false and become a bypass the moment enabled flips on
(e.g. via hot-reload).
tests/utils/ConfigurationSchema.test.ts
- Three .includes('uiServer.authentication') substring checks tightened
to .startsWith('uiServer.authentication') so they cannot match a
hypothetical sibling key like 'uiServer.authenticationFoo'. Affects
the two object-level coupling tests and the unknown-key fixture
assertion. Exact-element checks at lines 630/651 (specific leaf
paths) and the leaf-specific .includes at line 671 are left as-is.
Rewrite the JSDoc as a single factual paragraph matching the file
convention (LogConfigurationSchema, WorkerConfigurationSchema,
UIServerListenOptionsObjectSchema, ...) — 12 lines collapsed to 7,
no redundant phrasing, structured as: constraint definitions →
required-when-enabled coupling → unconditional-field-level rationale.
Per AGENTS.md 'Documentation conventions' (concision, structure).
* fix(config): emit per-field error paths on auth-required refine
Address sub-agent review feedback on #1903.
src/utils/ConfigurationSchema.ts
- Replace the .refine() with a fixed path: ['username'] by a
.superRefine() that emits one issue per actually-missing field with
the correct path. Configurations supplying only 'username' now
surface the issue at 'uiServer.authentication.password' rather than
'uiServer.authentication.username', and vice versa. Configurations
missing both produce two issues, one per field.
tests/utils/ConfigurationSchema.test.ts
- 'should reject authentication enabled without username and password'
now asserts both 'uiServer.authentication.username' and
'uiServer.authentication.password' paths are present, locking the
per-field issue contract.
- 'should reject authentication enabled with username but no password'
now asserts 'uiServer.authentication.password' specifically — would
catch the prior path-misanchoring regression.
- The 'should reject unknown key in uiServer.authentication section'
regression-anchor now asserts i.code === 'unrecognized_keys' at the
'uiServer.authentication' path, locking the unknown-key intent so a
hypothetical future refine cannot satisfy the assertion by accident.
* fix(ui-common): align authenticationConfigSchema with simulator auth schema
Apply the same defense-in-depth + per-field-error-path pattern shipped
on the simulator-side UIServerAuthenticationSchema in this PR. The Web
UI client schema (consumed by ui/web/src/main.ts and transitively by
ui/cli/src/config/loader.ts) carried the same set of defects:
- password lacked min(1) at the field level
- username regex /^[^:]*$/ accepted the empty string
- a single .refine combined type-check + presence + length, producing
a generic error at the auth object root with no path
- empty creds shipped under enabled: false would become a Basic-Auth
bypass the moment enabled is flipped on
ui/common/src/config/schema.ts
- password: z.string().min(1).optional()
- username: z.string().min(1).refine(no ':' RFC 7617).optional()
(replaces the regex which silently accepted '')
- .refine(...) replaced by .superRefine(...) emitting one issue per
actually-missing field with the correct path, only when
enabled === true && type === PROTOCOL_BASIC_AUTH (existing
type-check short-circuit preserved as future-proofing).
- JSDoc mirrors the simulator-side sibling, documenting the
intentional stricter-than-runtime semantics.
ui/common/tests/config.test.ts
- 4 new tests: empty password rejected (path 'authentication.password'),
username with ':' rejected (RFC 7617 message at
'authentication.username'), password-only and username-only enabled
configs each rejected at the missing field's path.
- 2 existing tests tightened: 'missing credentials' now asserts both
username and password paths; 'empty username' now asserts the
'authentication.username' path.
- 6 redundant 'if (result.success) return' narrowing guards removed —
assert.strictEqual already narrows the result type.
118/118 ui-common tests pass, typecheck + lint clean. No production
behavior change for the only valid-config consumer ('admin'/'admin'
shape regression-anchored).
* fix(config): point accessPolicy refine error path at the actionable field
Address review feedback on #1903. The .refine() rejecting
'allowLoopbackProxy: true' with empty 'trustedProxies' attached its
issue at path ['allowLoopbackProxy'] — the trigger of the rejection,
but not the field the operator must edit. Move the path to
['trustedProxies'] so structured log scrapers and IDE jump-to-field
tooling steer the operator to the field that needs populating. The
message wording (which already names both fields) is unchanged.
Test 'should reject allowLoopbackProxy=true with empty trustedProxies'
tightened from a permissive substring check
('allowLoopbackProxy or trustedProxies') to an exact-path check on
'uiServer.accessPolicy.trustedProxies' so the path contract is locked.
A second review suggestion (gate the auth credentials-required
superRefine on type === PROTOCOL_BASIC_AUTH) was investigated and
declined: the server-side AuthenticationType enum has BASIC_AUTH AND
PROTOCOL_BASIC_AUTH (unlike the Web UI client's single-member enum),
both runtime paths require username/password
(AbstractUIServer.ts:358-381), and adding the type-gate would re-open
the silent Basic-Auth bypass on the legacy 'basic-auth' path that
this PR is closing on the modern path.
src/utils/ConfigurationSchema.ts + ui/common/src/config/schema.ts
- Drop the 'across hot-reload toggles of enabled' phrasing in the
UIServerAuthenticationSchema and authenticationConfigSchema JSDoc.
Hot-reload of uiServer.* is documented as a no-op in the PR body
(R4: Bootstrap.ts:103,161 instantiates the UI server once), so the
unconditional field-level constraints are an early-detection signal
rather than a hot-reload mitigation. The accurate rationale is that
empty placeholders cannot ship under enabled: false and become a
Basic-Auth bypass on the next boot with enabled: true.
- The asymmetry between the two superRefines (server gates only on
enabled; Web UI also gates on type === PROTOCOL_BASIC_AUTH) is
preserved and intentional — server-side AuthenticationType has
legacy BASIC_AUTH which is also credential-required at runtime
(AbstractUIServer.ts isBasicAuthEnabled / isValidBasicAuth). The
rationale is captured in this commit body and the prior 86a6d4a2 commit body for git blame discoverability.
tests/utils/ConfigurationSchema.test.ts
- RFC 7617 username-rejection test now matches the issue path with
i.path.join('.') === 'uiServer.authentication.username' (exact)
instead of String.prototype.includes (substring), aligning with
every other test in the file and with the ui-common sibling.
Jérôme Benoit [Mon, 15 Jun 2026 16:40:57 +0000 (18:40 +0200)]
fix(config): tighten Zod validation on uiServer.options.port (#1874) (#1901)
* fix(config): tighten Zod validation on uiServer.options.port (#1874)
Replace the permissive z.custom<ListenOptions> bridge under
uiServer.options with a typed object schema validating known primitive
fields, so bad transport-level values (e.g. port: "not-a-number")
fail at boot time in ConfigurationSchema.safeParse instead of later
inside node:net.Server.listen with ERR_SOCKET_BAD_PORT.
The existing accessPolicy refinement is preserved via .pipe(); unknown
keys are still passed through (.loose()) to keep the ListenOptions
extension surface (e.g. signal: AbortSignal) usable.
Tests: 10 new cases in tests/utils/ConfigurationSchema.test.ts and
1 integration assertion in tests/utils/ConfigurationValidation.test.ts
covering the structured Zod error path uiServer.options.port routed
through ConfigurationValidationError.
* docs(config): align JSDoc with composite uiServer.options schema
Address review comment on #1901: the JSDoc above
UIServerListenOptionsObjectSchema was named after the composite
UIServerListenOptionsSchema, leaving the helper described under the
wrong name and the composite undocumented. UIServerConfigurationSchema
also still referenced the now-removed permissive z.custom<ListenOptions>
bridge.
- UIServerListenOptionsObjectSchema gets its own JSDoc describing the
typed object layer (.loose() + primitive constraints).
- UIServerListenOptionsSchema gets a JSDoc describing the full chain:
object guard → accessPolicy refinement → pipe to typed object.
- UIServerConfigurationSchema JSDoc updated to reference the new
composite schema instead of the obsolete custom bridge.
Adds a describe('buildStatusNotificationPayload') block matching the
existing payloadBuilders.test.ts style. Cases (9):
OCPP 1.6
- explicit errorCode: payload carries {connectorId, status, errorCode}
- errorCode omitted: payload omits errorCode (no NoError default)
- evseId pass-through with errorCode
- ocppVersion=undefined falls through to 1.6 shape
- unsupported version (e.g. '3.0') throws via assertOCPP16OrUndefined
OCPP 2.0.x
- VERSION_201 + evseId: {connectorId, connectorStatus, evseId}, no
status / errorCode fields
- VERSION_20 without evseId: {connectorId, connectorStatus}
- errorCode option ignored (1.6-only field)
Cross-version
- same input, different ocppVersion: shapes differ in the documented
way (status vs connectorStatus, errorCode/evseId presence)
Uses OCPP enum constants directly (OCPP16ChargePointStatus,
OCPP16ChargePointErrorCode, OCPP20ConnectorStatusEnumType,
OCPPVersion) per the OCPP enumeration-naming convention. Production
code in payloadBuilders.ts is unchanged.
* test(ui-common): exercise evseId-only path for OCPP 1.6 StatusNotification
Address review comment on #1902: the prior test 'should pass evseId
through for OCPP 1.6 when provided' passed both errorCode and evseId
together, so the evseId-only OCPP 1.6 branch was never independently
exercised. The errorCode-only path is already covered by the case
'should build OCPP 1.6 payload with status and explicit errorCode',
and the combined case is trivial composition of the two via the
spread syntax in the function body.
Test renamed to 'should pass evseId through for OCPP 1.6 without
errorCode' and now only passes options.evseId, asserting errorCode is
absent from the resulting payload.
Add moduleName constant and prefix the SENT command log with
'OCPPRequestService.internalSendMessage:' to match the format of the
3 received-message logs in ChargingStation.ts (handleErrorMessage,
handleIncomingMessage, handleResponseMessage).
Jérôme Benoit [Sun, 14 Jun 2026 20:07:08 +0000 (22:07 +0200)]
fix(ui-server): post-#1891 access policy and WS upgrade hardening (#1898)
* fix(ui-server): reject ports in allowedHosts, fix host normalization edges
- ConfigurationSchema: reject allowedHosts entries that carry a port
(e.g., 'gateway.example.com:8080', '[::1]:8080'). Runtime host
matching strips ports, so a port-bearing entry was silently
equivalent to its host-only form. Operators are now forced to
spell out host-only entries at config load time, matching how
matching actually works.
- UIServerNet: normalizeHost strips a trailing dot after the colon
split rather than from the whole input, so a Host header like
'localhost.:80' now resolves to 'localhost' instead of leaving
the trailing dot intact and failing the allowlist match.
- UIServerAccessPolicy: nonEmpty treats whitespace-only forwarded
values as absent, and hasForwardedHeaders consumes nonEmpty so
the two helpers agree. A whitespace-only X-Forwarded-Proto no
longer flips forwardedHeadersPresent to true and triggers an
Ambiguous* denial.
The onSocketError listener was previously attached only after the
prologue and bad-header guards had already written their rejection
responses, and was removed before the auth-failure rejection write.
A client TCP-reset between an upgrade event and the destroy callback
could fire 'error' on the Duplex socket with no listener, which Node
escalates to an unhandled exception and crashes the process.
The listener is now attached at the top of the upgrade handler and
only removed once webSocketServer.handleUpgrade takes ownership of
the socket. All three rejection paths (bad upgrade headers, prologue
failure, auth failure) keep the listener until socket.destroy(),
neutralising the DoS vector.
- Schema: split UIServerListenOptions validation so non-object values
surface 'must be a non-array object' instead of the misleading
'accessPolicy must be configured under uiServer' message.
- Forwarded parser: unescape RFC 7230 quoted-pair sequences inside
double-quoted Forwarded parameter values.
- getForwardedClientAddress: propagate forwarded.kind === 'error' before
the trustedProxy gate, matching getForwardedProtocol and
getForwardedHost (no behavioral change today; closes a latent
asymmetry if the upstream untrusted-peer gate is reordered).
- UIServerAccessCache: document identity-based cache invalidation for
trustedProxies; reload flows must construct a new configuration.
- getForwardedProtocol / getForwardedHost: distinguish length === 0
(new InvalidForwardedProtocol / InvalidForwardedHost reasons) from
length > 1 (existing Ambiguous reasons), matching the existing
InvalidForwardedClient / AmbiguousForwardedClient pattern.
* feat(ui-server): authorize requests before authentication
* feat(ui-server): gate HTTP requests by access policy
* feat(ui-server): gate MCP requests by access policy
* feat(ui-server): gate WebSocket upgrades by access policy
* chore(docker): align UI access policy for local compose
* docs(ui): document gateway access policy
* [autofix.ci] apply automated fixes
* refactor(ui-server): type access decisions and unify request prologue
- Replace stringly-typed denial reason with UIServerAccessDenialReason enum and
DENIAL_MESSAGES rendering map; UIServerAccessDecision is a discriminated union
narrowed by 'allowed'.
- Memoize the per-request access decision via WeakMap so the policy is evaluated
once per request.
- Add runRequestPrologue template method on AbstractUIServer; rate-limit now
accounts denied requests, closing pre-auth flood amplification.
- Validate accessPolicy.trustedProxies entries as IPv4/IPv6 literals at the
schema layer; precompile the normalized set per UIServerConfiguration.
- Drop the dead httpServer.on('connect') listener in UIWebSocketServer; use
StatusCodes consistently in UIMCPServer.
- README clarifies the binary loopback-vs-non-loopback model, single-hop XFF,
TLS termination via reverse proxy only, and adds a migration note for the
default requireTlsForNonLoopback: true.
* refactor(ui-server): split access policy module and tighten its surface
- Split UIServerUtils.ts into UIServerAccessPolicy (decision, forwarded
parsing, host/origin allowlists) and UIServerNet (IP/host normalization,
loopback, header tokenization). UIServerUtils retains auth tokens and WS
subprotocol negotiation only.
- Make evaluateUIServerAccess private; tests and runtime callers go through
resolveUIServerAccess so the decision cache is always exercised.
- Move accessDecisionCache and trustedProxiesCache from module scope to a
per-AbstractUIServer instance cache injected into resolveUIServerAccess.
- Quote-aware splitHeaderList honors RFC 7239 / RFC 7230 double-quoted
values; commas inside "…" are preserved.
- Replace the {error?, value?} dialects in forwarded-header parsers with a
discriminated ParseOutcome<T>; close the Forwarded params type to
by/for/host/proto.
- Drop the unreachable isDirectTLSRequest helper and its TLSSocket import.
- Set HTTP server requestTimeout=30s and headersTimeout=5s on
AbstractUIServer to bound slow-loris exposure on rejected upgrades.
- Ensure UIMCPServer.handleMcpRequest releases the transport and the MCP
server in every error branch (idempotent cleanup).
- Promote MockUpgradeSocket to UIServerTestUtils.ts; replace the duplicate
test factory createAccessPolicyConfiguration with the canonical
createMockUIServerConfiguration.
- Cover the Docker dual-stack case where an IPv4-mapped IPv6 remote address
matches an IPv4 trustedProxies entry.
- README accessPolicy cell switches to per-field sub-bullets, aligned with
the log and worker rows.
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
* refactor(ui-server): drop @file headers from new policy modules
@file JSDoc is the test-suite convention (TEST_STYLE_GUIDE.md) and is
absent from the rest of src/. Align the new UIServerAccessPolicy and
UIServerNet modules with the existing source-tree style.
- Drop README migration note and the duplicate access policy paragraph;
the configuration table cell already specifies the policy.
- Tighten the Docker section access policy paragraph.
- Trim narrative meta-commentary in UIServerAccessPolicy and UIServerNet
JSDoc; let constant and type names carry the rest.
- isHostAllowed: when the immediate peer is a trusted proxy and
X-Forwarded-Host is present, match it (not the immediate Host) against
allowedHosts, so reverse proxies that rewrite Host to an internal
upstream name are not rejected.
- isOriginAllowed: compare allowedOrigins via parsed URL protocol+host
rather than literal string equality, so entries with a trailing slash or
with a protocol-default port match the canonical browser-emitted Origin.
- UIMCPServer: destroy the response and request sockets after a denied
prologue, mirroring UIHttpServer.
- evaluateUIServerAccess: parse the Forwarded header once and pass the
outcome to getForwardedProtocol and getForwardedClientAddress.
- hasDuplicateHeaders: count rawHeaders in a single pass.
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
* fix(ui-server): include prologue headers on WebSocket upgrade denial
The WebSocket upgrade denial path wrote only the HTTP/1.1 status line, so
rate-limit denials reached clients without the Retry-After header that
the HTTP and MCP transports already emit.
- UIMCPServer: destroy the response and request sockets after a denied
authentication, mirroring UIHttpServer.
- UIWebSocketServer: include the WWW-Authenticate header on auth-denied
upgrades so RFC 7235 clients receive the Basic challenge that the HTTP
and MCP transports already emit.
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
* fix(ui-server): close denied connections cleanly and harden allowedOrigins
- AbstractUIServer: also call setTimeout() on the http server so the
HTTP/2 transport (Http2Server lacks requestTimeout/headersTimeout) gets
the same idle bound.
- UIHttpServer / UIMCPServer: emit Connection: close on prologue and auth
denials and drop the explicit res.destroy()/req.destroy() calls; the
body now flushes before the connection closes.
- UIWebSocketServer: destroy the upgrade socket from the write callback
so the response bytes are flushed before teardown.
- UIServerAccessPolicySchema: refine allowedOrigins entries to reject
paths, query strings, and fragments at config load instead of silently
ignoring them at runtime.
- UIServerAccessPolicy: parse the Forwarded host parameter symmetrically
with for and proto. Both Forwarded host=... and X-Forwarded-Host now
feed isHostAllowed when the immediate peer is trusted, with a new
AmbiguousForwardedHost denial reason when they conflict.
- UIWebSocketServer: build upgrade rejection responses through a single
helper that always emits Connection: close and Content-Length: 0,
matching the HTTP and MCP transports.
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
* fix(test): wait on response finish event in gzip tests
Replace the fixed 50 ms waitForStreamFlush delay with await once(res,
'finish') in the UIHttpServer gzip tests. MockServerResponse already
emits 'finish' from end(), so the wait is now event-driven and stops
flaking on slow CI runners (notably Windows / Node 24.x).
Drop the now-unused waitForStreamFlush helper and GZIP_STREAM_FLUSH_DELAY_MS
constant.
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
* fix(ui-server): scope HTTP timeouts to slow-loris and tighten Forwarded parser
- AbstractUIServer: drop the connection-wide setTimeout call. requestTimeout
and headersTimeout already cover slow-loris on HTTP/1.1, and HTTP/2 streams
are per-request and not vulnerable to the same pattern. The dropped 30 s
socket-inactivity cap was killing legitimate long-running responses on
the deprecated HTTP transport.
- UIServerAccessPolicy: split Forwarded pairs with quote awareness so that
RFC 7239 quoted-string values containing semicolons are not truncated.
Mirrors the quote handling already in splitHeaderList.
- UIWebSocketServer: place Connection: close after the extraHeaders spread
in buildUpgradeRejectionResponse to match the HTTP and MCP transports
and keep the close header non-overridable by callers.
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
* fix(ui-server): omit Connection header on HTTP/2 and prioritize trusted-peer denial reason
- AbstractUIServer: add getConnectionCloseHeader() returning an empty
header set when the underlying server is HTTP/2. HTTP/2 forbids the
Connection header as a connection-specific header; Node otherwise emits
an UnsupportedWarning and silently drops the value. Streams are closed
by res.end() on that path. HTTP/1.1 keeps the explicit Connection: close
to terminate keep-alive on denial.
- UIHttpServer, UIMCPServer: route prologue-denial and auth-denial
responses through the helper instead of hardcoding Connection: close.
- UIServerAccessPolicy: hoist the trusted-peer check above the protocol
and host ambiguity checks. An untrusted peer sending forwarded headers
is now denied as ForwardedFromUntrustedPeer regardless of duplicate
proto/host metadata, so operator logs reflect the trust violation
rather than a parser-level symptom. Duplicate-headers retains absolute
priority. Tests added for the protocol- and host-ambiguity paths from
an untrusted peer.
Empty forwarded-header values were treated as present-with-empty-string,
which caused two misbehaviors when the immediate peer was a trusted proxy:
- An empty 'host=' parameter inside a Forwarded header (or an empty
X-Forwarded-Host) shadowed the real Host header. The policy resolved
the host to '' and denied the request as HostNotAllowed even though
the actual Host was in allowedHosts.
- An empty X-Forwarded-Proto was rejected as AmbiguousForwardedProtocol
(via splitHeaderList returning an empty array) instead of being
treated as missing.
A nonEmpty() helper now normalizes empty/undefined header values to
undefined at parse time. parseSingleForwardedHeader skips empty
parameter values rather than recording them. The two getters fall
through to the next source when the upstream value is empty.
Tests cover the host fallback paths (Forwarded host= and
X-Forwarded-Host) and the protocol absent-vs-ambiguous distinction.
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
* fix(ui-server): accept RFC 7239 hidden-identity forms and tighten Forwarded parser
- UIServerAccessPolicy: recognize the RFC 7239 nodename forms 'unknown'
and obfuscated identifiers ('_' followed by token characters) in the
Forwarded 'for=' parameter and X-Forwarded-For header. Hidden-identity
forms now resolve to ABSENT, so the access decision falls back to the
trusted proxy's own remote address as the client identifier (used as
the rate-limit bucket and the logged identity). Previously these
values were rejected as InvalidForwardedClient even though the rest
of the request (TLS, host, origin, trust) was valid.
- UIServerAccessPolicy: strip Forwarded parameter quotes only when the
pair is balanced. The previous '/^"|"$/g' replace stripped the
leading or trailing quote independently and silently accepted
unbalanced inputs such as 'for="203.0.113.10'. The new
'/^"(.*)"$/' anchors both ends in a single match.
- README: clarify that allowLoopbackProxy requires the loopback peer
to also be listed in trustedProxies. allowLoopbackProxy alone has
no effect because the trusted-peer check denies forwarded headers
from peers absent from the trustedProxies list.
- Tests: positive path for allowLoopbackProxy=true; identity-hidden
paths for Forwarded for=unknown, Forwarded for=_obfuscated, and
X-Forwarded-For: unknown.
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
* fix(ui-server): normalize empty X-Forwarded-For and align MCP auth with continuation pattern
- UIServerAccessPolicy: normalize an empty X-Forwarded-For header to
undefined so the access decision falls back to the trusted proxy's own
remote address rather than denying the request as InvalidForwardedClient
or as AmbiguousForwardedClient when a Forwarded for= parameter is also
present. Restores symmetry with X-Forwarded-Host and X-Forwarded-Proto
empty-value handling.
- UIMCPServer: rewrite the auth path as a callback continuation. The
previous capture-then-check pattern relied on authenticate() being
synchronous and on the last next() call winning. The new shape
matches UIHttpServer and UIWebSocketServer and continues to handle
MCP requests only after a successful authentication callback.
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
* fix(ui-server): align MCP error responses and treat empty forwarded headers as absent
- UIMCPServer: emit Connection: close on the 404 not-found and on the
centralized sendErrorResponse helper (BAD_REQUEST, METHOD_NOT_ALLOWED,
INTERNAL_SERVER_ERROR), aligning these paths with the post-prologue
and auth denial responses. The connection-close header is omitted on
HTTP/2 by the existing getConnectionCloseHeader() helper.
- UIServerAccessPolicy: hasForwardedHeaders now ignores empty header
values, restoring symmetry with the downstream forwarded-header
parsers that already map empty values to absent. Empty X-Forwarded-*
headers from an untrusted peer no longer trigger
ForwardedFromUntrustedPeer; an empty X-Forwarded-Proto from a
non-loopback untrusted peer now denies as TlsRequired (no proxy
proof of TLS). Empty headers from a loopback peer no longer block
the request.
- README: clarify that requireTlsForNonLoopback honors X-Forwarded-Proto
/ Forwarded: proto= from a trusted proxy and does not detect native
direct TLS; note that a compromised trustedProxies entry can bypass
per-client rate limiting by varying X-Forwarded-For.
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
* chore(ui-server): trim redundant access-policy comments and JSDoc
- Drop module-level JSDoc on UIServerAccessPolicy and UIServerNet to
match the codebase convention (no module-level JSDoc on peer files).
- Drop redundant inline comments whose content is already documented in
the README or self-evident from the surrounding code: the empty
X-Forwarded-For malformed branch, the allowedOrigins fallback, and
the loopback-aliases derivation.
- Trim the verbose 'Discriminated by ...' prose on the UIServerAccessDecision
and ParseOutcome types and remove the JSDoc on resolveUIServerAccess
(function name and signature are self-explanatory; memoization is
visible from the cache lookup).
- UIServerNet: factor the quote-aware splitter into splitQuoted(value,
delimiter); splitHeaderList becomes a thin wrapper. The previously
duplicated state machine in UIServerAccessPolicy (splitForwardedPairs)
is replaced by a direct splitQuoted call on ';'.
- UIServerAccessPolicy: factor the X-Forwarded-* / Forwarded ambiguity
pick into pickForwardedValue. The three getForwarded* helpers now
share a single source of truth for the present/absent/ambiguous
decision; transport-specific tail logic (multi-hop X-Forwarded-For
rejection, X-Forwarded-Proto multi-value rejection, lowercasing)
stays local.
- UIServerAccessPolicy: drop the unreachable string-trim fallback in
normalizeIPAddress (every input that fails normalizeHost also fails
the downstream IP checks), narrow isSecureForwardedProtocol's
parameter to string | undefined (no caller passes null), and skip
the redundant pre-pass through normalizeHost in isSameHost (the
IP-address branch already calls normalizeHost internally).
- UIServerAccessPolicy: replace Reflect.get with direct property
access on IncomingMessage.headersDistinct and rawHeaders (typed
members on Node 18+).
- Tests: extend the createMockIncomingMessage and the policy test's
createAccessPolicyRequest factories with headersDistinct and
rawHeaders defaults so direct property access works against the
mocks.
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
* refactor(ui-server): unify denial rendering and switch authenticate to boolean
- AbstractUIServer: introduce renderDenial(res, payload) and
getUnauthorizedDenial() so HTTP and MCP route every prologue, 401,
and post-handler error response through the same code path. The
helper centralizes Content-Type, Connection-close (HTTP/2-aware),
and 'res.headersSent' guarding.
- AbstractUIServer: change authenticate(req) to return a boolean
instead of invoking a continuation with an Error. The CPS shape was
fully synchronous in practice — every caller did
'if (err != null)' — and the BaseError allocation was dead. The
three transports collapse to 'if (!this.authenticate(req)) { … }'.
- AbstractUIServer + UIServerUtils: replace hand-typed reason phrases
with getReasonPhrase() from http-status-codes; the helper functions
isValidBasicAuth / isValidProtocolBasicAuth and
getUsernameAndPasswordFromAuthorizationToken drop their continuation
parameter accordingly.
- UIServerSecurity: replace the chunk-counter createBodySizeLimiter()
with a shared readLimitedBody(req, maxBytes): Promise<Buffer> helper
and a typed PayloadTooLargeError. UIHttpServer reads the request
body via the helper instead of an event-based loop, and UIMCPServer
detects oversized payloads via 'instanceof PayloadTooLargeError'
rather than string-matching the error message.
- UIMCPServer: route sendErrorResponse through renderDenial so all
4xx/5xx error responses share the same headers and Connection
semantics; preserve the path-filter-before-authenticate ordering
with a comment so unknown paths return 404 without revealing
whether authentication would have succeeded.
- UIWebSocketServer: use getUnauthorizedDenial() for the upgrade
rejection on auth failure instead of inlining the WWW-Authenticate
string and the 'Unauthorized' literal.
- Tests: align UIServerUtils.test, UIServerSecurity.test, and
UIMCPServer.test with the new APIs (boolean signatures, new
PayloadTooLargeError, new readLimitedBody).
- ConfigurationSchema: introduce UI_SERVER_ACCESS_POLICY_DEFAULTS as the
single source of truth for the access-policy field defaults
(requireTlsForNonLoopback=true, allowLoopbackProxy=false, and the
empty allowlists). The defaults previously lived inline in
evaluateUIServerAccess.
- UIServerAccessPolicy: consume UI_SERVER_ACCESS_POLICY_DEFAULTS instead
of the hardcoded literals so a default change propagates uniformly
across the policy code, the schema, and the README.
- ConfigurationSchema: validate allowedHosts entries via normalizeHost
so paths, queries, fragments, and other malformed Host values are
rejected at config load. The previous z.string().min(1) accepted
any non-empty string.
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
* test(ui-server): factor access-policy test fixtures and address constants
- UIServerTestUtils: introduce TRUSTED_PROXY_IP, EXTERNAL_CLIENT_IP, and
GATEWAY_HOST constants from RFC 5737 / RFC 3849 reserved ranges;
factor the recurrent gateway access-policy configurations into
createGatewayConfigWithTrustedProxy() and
createGatewayConfigWithoutTrustedProxies() so subsequent test
additions inherit a single source of truth for the gateway shape.
- UIServerAccessPolicy.test: cover the previously untested denial
reasons AmbiguousForwardedProtocol (multi-value X-Forwarded-Proto),
AmbiguousForwardedHeader (multi-entry Forwarded), and
AmbiguousForwardedParameter (duplicate parameter inside one entry);
cover the InvalidForwardedClient branch for non-IP, non-hidden 'for='
values; cover AmbiguousForwardedClient when 'Forwarded: for=unknown'
collides with X-Forwarded-For; cover the headersDistinct path of
hasDuplicateHeaders.
- UIServerAccessPolicy.test: cover the malformed Origin URL branch in
isOriginAllowed and the allowedHosts-fallback branch when
allowedOrigins is empty; pin the design choices for whitespace-padded
Host normalization, uppercase X-Forwarded-Proto values, IPv4-mapped
IPv6 loopback proxies under allowLoopbackProxy, and the policy's
intentional non-detection of req.socket.encrypted without forwarded
protocol headers.
- UIServerAccessPolicy.test: tighten the previously weak assertions on
the accessPolicy-undefined paths (now check the denial reason and
the resolved client address); rename the misleading
'should reject empty-but-present X-Forwarded-For' test to reflect
what it actually exercises (no parseable addresses); replace the
trivial cache-isolation assertion with a real
cacheA.decisions.has(req) / cacheB.decisions.has(req) check.
- UIMCPServer.test: drop the two malformed-host tests that re-tested
policy logic already covered by UIServerAccessPolicy.test; the
Connection: close tests in the same describe still anchor the
transport-level rendering contract.
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
* test(ui-server): close coverage gaps and tighten weak assertions identified in second-pass audit
- ConfigurationSchema.test: validate the allowedHosts schema refine
rejects malformed entries (excess colons, non-numeric port, port out
of range, empty input) and accepts canonical hostnames and IP
literals; lock the canonical UI_SERVER_ACCESS_POLICY_DEFAULTS map
shape so a default change cannot drift silently.
- UIServerNet.test: cover normalizeHost directly (rejection branch is
the boundary of the allowedHosts schema refine; previously only
exercised indirectly through transport tests).
- UIServerSecurity.test: assert readLimitedBody propagates upstream
stream errors (security-relevant fail-closed behavior).
- UIServerAccessPolicy.test: rename the misleading
for=unknown collision test to reflect the actual code path
(collision is generic, not unknown-specific); drop the duplicate
encrypted-proxy plaintext rejection (the encrypted flag is ignored
by the policy by design and a dedicated test already pins it);
tighten the whitespace-padded Host test to use an explicit
allowedHosts list so the trim path is genuinely exercised rather
than the loopback alias fallback.
- UIServerTestUtils: drop the dead EXTERNAL_CLIENT_IP export and
correct the RFC reference comment (RFC 5737 IPv4 / RFC 2606
example.com — RFC 3849 covers IPv6 and is irrelevant here).
- UIWebSocketServer: add a brief comment documenting why
buildUpgradeRejectionResponse stays separate from
AbstractUIServer.renderDenial — pre-handshake WS rejections write
raw HTTP/1.1 to a Duplex socket while renderDenial targets
ServerResponse.
- src/utils/index: re-export UI_SERVER_ACCESS_POLICY_DEFAULTS so the
defaults can be asserted from the schema test layer.
- UIServerAccessPolicy: getForwardedHost rejects multi-value
X-Forwarded-Host as AmbiguousForwardedHost, mirroring the symmetric
guards in getForwardedProtocol and getForwardedClientAddress. The PR
description's claim that 'multi-value X-Forwarded-* are rejected'
now applies uniformly across the three forwarded dimensions.
- UIServerNet: normalizeHost validates the hostname charset against a
conservative [a-z0-9._-]+ allowlist on the dot path. Inputs with
commas, spaces, or unbalanced brackets are rejected at config-load
time when used as allowedHosts entries, and at runtime as a
defense-in-depth check on the Host header.
- UIServerNet: isValidPort rejects port 0 (RFC 6335 reserved). Port 0
in a client-side Host header has no useful meaning; tightening to
1..65535 matches common practice and the prior config-bind path is
unaffected.
- README: requireTlsForNonLoopback wording corrected — non-loopback
requests without forwarded protocol headers are denied as
tls-required, not merely 'not detected', even when the underlying
socket is encrypted.
- UIServerAccessPolicy: parseSingleForwardedHeader normalises an empty
Forwarded header value (e.g., 'Forwarded: ') to ABSENT instead of
rejecting it as AmbiguousForwardedHeader. The PR description claim
that 'empty forwarded values are treated as absent' now applies to
the header value itself, not only to parameters inside the header.
- UIMCPServer: sendErrorResponse accepts an optional headers argument
so 405 Method Not Allowed responses advertise 'Allow: GET, POST,
DELETE' per RFC 9110 §15.5.6.
- AbstractUIServer: warn at startup when the UI server is bound to a
wildcard host ('', '0.0.0.0', '::') with an empty
accessPolicy.allowedHosts; the previously silent lockout (every
request denied as host-not-allowed) now produces an actionable log
line pointing at the configuration to adjust.
- AbstractUIServer: warnIfMisconfigured now also fires when the UI
server is bound to a non-loopback specific host with
requireTlsForNonLoopback=true and no accessPolicy.trustedProxies;
the previously silent path (every plaintext request denied as
tls-required) now produces an actionable startup log line. Wildcard
binding with empty allowedHosts retains its dedicated warning.
- AbstractUIServer: rate-limit denials now emit a warn log symmetric
to the access-policy 403 path so operators can detect probing.
Previously the 429 branch returned silently, which made abusive
clients invisible from the logs.
- README: drop the 'even when the underlying socket is encrypted'
caveat from the requireTlsForNonLoopback description; the simulator
only listens via plaintext or h2c, so the scenario described was
unreachable. The runtime behaviour is unchanged and the test that
pins the design choice stays as defense in depth.
Jérôme Benoit [Sun, 14 Jun 2026 01:29:02 +0000 (03:29 +0200)]
fix(ui-web): skip dev-only diagnostics under Vitest (#1897)
Extract isDev() helper in core/env.ts that returns true only in real
DEV (excludes production build and Vitest). Migrate the 7 existing
import.meta.env.DEV check sites to the helper.
Primary motivation: validateTokenContract schedules a
requestAnimationFrame whose body calls console.warn for missing CSS
tokens. jsdom does not resolve --color-* tokens, so every theme/skin
switch under Vitest 4 queued a console.warn that fired after
environment teardown — surfacing as EnvironmentTeardownError on
Linux/Node 22; passes on macOS/Node 24.
Side effect: silences console.debug noise from storage.ts,
providers.ts, ToggleButton.vue under Vitest. dev/build behaviour
unchanged.
Adds regression test in tests/unit/shared/tokens/contract.test.ts.
Jérôme Benoit [Sat, 13 Jun 2026 23:46:21 +0000 (01:46 +0200)]
test(ui-web): drain dynamic imports globally for async components (#1893)
Add vi.dynamicImportSettled() to the global afterEach hook so pending
dynamic imports (defineAsyncComponent loaders, lazy routes) settle
before Vitest tears down the test environment. Prevents
EnvironmentTeardownError on transitive .vue import chains observed
under filtered runs (e.g. -t "should open authorize dialog").
Covers App.vue, ModernLayout.vue, and any future component using
defineAsyncComponent or lazy-loaded routes.
Jérôme Benoit [Thu, 4 Jun 2026 17:17:35 +0000 (19:17 +0200)]
test(ui-web): mock useTheme/useSkin in SimulatorBar.test.ts to fix vitest 4.x teardown RPC leak (#1884)
* test(ui-web): mock useTheme/useSkin in SimulatorBar.test.ts to fix vitest 4.x teardown leak
The two final tests ('should call switchTheme/switchSkin when … select changes')
mounted SimulatorBar with the real useTheme/useSkin composables. Both code paths
schedule console.warn output AFTER the synchronous test body resolves:
1. validateTokenContract() (src/shared/tokens/contract.ts) wraps work in
requestAnimationFrame and warns when CSS variables are missing. jsdom does
not resolve --color-* CSS variables, so the contract check logs ~24 warnings
via rAF on every theme/skin switch.
2. switchSkin() returns Promise<boolean>, but the @change handler discards the
promise and trigger('change') only awaits Vue's nextTick. The unawaited
loadStyles() dynamic import resolves later and may also call console.warn.
Vitest 4.x tightened teardown to fail rather than swallow pending RPC calls,
surfacing this latent bug as:
EnvironmentTeardownError: [vitest-worker]: Closing rpc while
"onUserConsoleLog" was pending
Reproducibly fails on Linux/Node 22 in CI; passes on macOS/Node 24 because rAF
+ dynamic-import timing differs.
Fix mirrors the existing precedent in tests/unit/router.test.ts: vi.mock both
composables at the top of the test file, returning shapes that match the real
contract (THEME_IDS / SKIN_IDS imported from ui-common rather than hand-rolled
subsets). The mocked switchSkin resolves immediately so no teardown leak occurs.
Also tightens the two affected tests to actually assert what their names claim
(switchTheme/switchSkin called with the selected value), instead of merely
checking the <select> exists.
* test(ui-web): use THEME_IDS/SKIN_IDS indices instead of hardcoded values
Address PR review: hardcoding 'dracula' and 'classic' as the target select
values would silently break the assertions if THEME_IDS or SKIN_IDS are
reordered or renamed in ui-common (the DOM $lt;select$gt; would keep its
current option, switchTheme/switchSkin would be called with that stale
value, and the toHaveBeenCalledWith assertion would compare against a
constant that no longer matches the dispatched event).
Pick targets that are guaranteed to differ from the mocked active values:
THEME_IDS[1] (≠ activeThemeId = THEME_IDS[0]) and the first SKIN_IDS entry
that is not 'modern' (≠ activeSkinId). The assertions then reference the
same constants, keeping the test coupled to the actual contract surface.
Note: the third reviewer comment about adding beforeEach(mockClear) is not
needed — vitest.config.ts already sets clearMocks: true and
restoreMocks: true, which run before each test.
* Revert "test(ui-web): use THEME_IDS/SKIN_IDS indices instead of hardcoded values"
* test(ui-web): add explicit afterEach mock cleanup in SimulatorBar test
Aligns SimulatorBar.test.ts with the dominant convention in ui/web's test
suite (Dialogs, StationCard, ConnectorRow, ClassicLayout, Actions, …),
which all add an explicit `afterEach(() => { vi.clearAllMocks() })`
alongside vitest.config.ts's global `clearMocks: true` / `restoreMocks: true`.
Belt-and-braces: the explicit reset keeps mock lifecycle visible next to
the mock declarations, even though it is technically redundant with the
global flags.
Jérôme Benoit [Tue, 2 Jun 2026 19:53:05 +0000 (21:53 +0200)]
chore(release-please): remove release-as override after 4.8.0 release
The 4.8.0 override has shipped via PR #1880 (merge commit 44ab6cf8).
Remove the 'release-as' field so that subsequent release-please runs
resume normal Conventional Commits version computation.
Without this cleanup, subsequent runs would keep proposing 4.8.0,
producing no-op release PRs or duplicate-tag errors.
Jérôme Benoit [Tue, 2 Jun 2026 19:48:39 +0000 (21:48 +0200)]
chore(release-please): override next release to 4.8.0
The BREAKING CHANGE footer in 8bb806d describes a log message wording
change consumed by external monitors, not a SemVer-breaking API change.
This override forces release-please to ship 4.8.0 (minor) instead of 5.0.0.
Cleanup: this 'release-as' field MUST be removed in a follow-up commit
once the 4.8.0 release PR merges, otherwise subsequent runs will
re-propose 4.8.0.
Jérôme Benoit [Wed, 27 May 2026 22:33:50 +0000 (00:33 +0200)]
refactor: relocate simulator config to src/utils and enforce barrel discipline
- Move ConfigurationSchema/Migrations/Validation from charging-station to utils
- Rename ConfigurationUtils.logPrefix to configurationLogPrefix (barrel anti-collision)
- Expose public APIs through component barrels (charging-station, ocpp, utils, worker)
- Migrate 38 test imports from deep paths to barrels
- Document TDZ-cycle exceptions in OCPPError and ConfigurationMigrations
* feat(config): scaffold simulator configuration schema and validator skeleton
- Add ConfigurationMigrations.ts: CURRENT_CONFIGURATION_SCHEMA_VERSION=1,
coerceConfigurationVersion, applyConfigurationMigration, migrateV0ToV1
with DEPRECATED_KEY_REMAPPINGS (~25 legacy keys)
- Add ConfigurationSchema.ts: strict Zod v4 schema for all config sections
(log, worker, performanceStorage, uiServer, stationTemplateUrls) with
deprecated keys as .optional()
- Add ConfigurationValidation.ts: validateConfiguration pipeline
(guard→clone→coerce→migrate→safeParse→transform) + ConfigurationValidationError
extends BaseError with structured fieldErrors
- Add $schemaVersion: 1 to config-template.json
- Add ConfigurationFixtures.ts test helpers
- Add ConfigurationSchema.test.ts (63 tests), ConfigurationMigrations.test.ts (44 tests)
- Wire validateConfiguration into Configuration.getConfigurationData() with
hard-throw at boot (console.error+chalk+process.exit(1))
- Hot-reload snapshot rollback: pre-clear snapshot, restore on failure
- Replace ConfigurationData interface with z.infer<typeof ConfigurationSchema>
- Migrate all consumers atomically (SimulatorState, Configuration, types barrel)
- Delete checkWorkerProcessType and checkWorkerElementsPerWorker (subsumed by schema)
- Delete ConfigurationMigration.ts (logic moved to ConfigurationMigrations.ts)
- Add ConfigurationValidation.test.ts (39 tests), Configuration-hot-reload.test.ts
- Add ConfigurationValidation-perf.test.ts (p99 < 50ms budget)
- Add error message snapshot test
- Update README.md with $schemaVersion documentation
* feat(config): remove legacy ConfigurationMigration.ts and stale call sites
- Delete src/utils/ConfigurationMigration.ts (deprecated-key logic now
owned by ConfigurationMigrations.ts v0→v1 migration step)
- Remove checkDeprecatedConfigurationKeys import and call site from
Configuration.getStationTemplateUrls() — validation pipeline in
getConfigurationData() already handles deprecated key remapping
* chore(config): silence lint warnings on configuration migrations
- Add JSDoc descriptions for setAtPath() params
- Add JSDoc descriptions and @returns for migrateV0ToV1
- Whitelist 'emerg' (syslog level) and 'REMAPPINGS' in cspell dictionary
- Extract deprecated-key sweep into pure remapDeprecatedKeys() that
reports warnings and field errors via return value instead of side
effects. Runs unconditionally regardless of $schemaVersion so v1
configs still containing deprecated keys never silently drop user
values (B3).
- Replace silent-drop setAtPath with collision- and intermediate-aware
variant: equal-value writes are idempotent no-ops, unequal values
produce a typed field error (B4), non-object intermediates are
reported and stop traversal instead of overwriting user data (N7).
- Type DEPRECATED_KEY_REMAPPINGS as Record<string, string | null>;
null marks deprecated keys with no canonical destination, replacing
the autoReconnectMaxRetries self-mapping that silently dropped the
user value (B2). Add 'worker.elementStartDelay' as a dotted source
key so nested deprecations live in the same single source of truth.
- Refactor ConfigurationValidationError: primary constructor takes
FieldError[] + a context with explicit phase ('migration' | 'schema');
static fromZodError() factory wraps Zod failures. Error messages now
carry the phase in their tag for clearer diagnostics.
- Switch deprecation-warning channel from logger.warn to console.warn
to break a re-entrant boot path where the Logger proxy lazily
resolved Configuration.getConfigurationSection('log'), recursing into
the validation pipeline (B1). transformConfiguration is now a pure
deep clone — its previous warning loop moved upstream to the sweep.
- Delete dead post-migration remapping in Configuration.ts:
deprecatedLogKeyMap, deprecatedWorkerKeyMap, the
delete configurationData.workerPoolStrategy mutation, the cast hiding
the legacy 'supervisionURLs' read, and the worker.elementStartDelay
fallback (B6). All deprecation handling now flows through the
migration's single source of truth.
- Make hot-reload reload loop async end-to-end. Awaits the change
callback inside the same lock so subsequent reloads cannot interleave
with an in-flight callback. Adds configurationFileReloadPending so
events arriving during a reload coalesce into exactly one drain
reload after the current one completes (N8).
- Polish schema: use z.enum(NativeEnum) directly for ApplicationProtocol
and ApplicationProtocolVersion to preserve literal-type narrowing on
UIServerConfiguration (N4); drop the BaseConfigurationSchema alias
(N5).
* test(config): cover migration edge cases and hot-reload rollback
Tests aligned with the rebuilt validation pipeline:
- Split migration tests into a remapDeprecatedKeys block (per-key sweep,
null-destination removal for autoReconnectMaxRetries, equal-value
collision idempotency, unequal-value collision field error, non-object
intermediate field error, nested worker.elementStartDelay) and a lean
applyConfigurationMigration block (version-bump only, immutability).
- Replace the empty self-mapping branch with explicit hasOwnProperty
removal assertions to defeat the previously vacuous test.
- ConfigurationValidation tests: switch deprecation-warning channel
spies from logger.warn to console.warn (B1 regression also asserts
logger.warn is never called from the pipeline), tolerate
schema-incompatible remap targets so the warning fires before the
downstream throw, add B3 sweep / B6 SSOT / future-version pipeline /
ConfigurationValidationError shape (FieldError[] + phase, fromZodError
factory, schema-phase aggregation) tests, and update the error-message
snapshot to include the new [phase] tag.
- transformConfiguration immutability test verifies a fresh validation
is unaffected by mutating a prior return value.
- ConfigurationSchema tests: strict parity for performanceStorage and
uiServer.authentication; StationTemplateUrl entry constraints
(empty file, negative numberOfStations, deprecated numberOfStation,
unknown key); worker.elementsPerWorker and poolMaxSize/MinSize
positive-integer constraints; log.statisticsInterval non-negative
integer constraints; bidirectional schema/DEPRECATED_KEY_REMAPPINGS
sync meta-test that walks both top-level and nested @deprecated
describe markers.
- Hot-reload tests rebuilt around the async runReloadLoop: validation
failure asserts the logged error is a ConfigurationValidationError;
JSON parse error path asserts configurationData and section cache are
fully restored (Gap 7); a sentinel watcher survives a failed reload;
N8 rapid double-save coalesces into exactly one drain reload reflecting
the latest content; flag reset is exercised on both paths.
- Configuration validation perf test: relative p99 budget (20× median
with a 1ms floor) plus an absolute 500ms catastrophic ceiling,
replacing the flaky absolute 50ms threshold (N1).
- Fixtures: new buildInvalidJsonString, buildV1WithDeprecatedKey
(handles top-level and dotted source keys), and
buildV0WithDeprecatedKeyCollision builders for the new test cases.
* docs(config): trim verbose JSDocs and harmonize with Template* conventions
Polish pass after PR audit: bring comments and docstrings in line with
the existing TemplateMigrations / TemplateValidation / TemplateSchema
patterns, drop ceremonial AI-generated narration, and fix two precision
defects.
- ConfigurationMigrations.ts: trim getAtPath / setAtPath / remapDeprecatedKeys /
migrateV0ToV1 / applyConfigurationMigration JSDocs to match Template*
density; drop the historical "now handled unconditionally upstream"
paragraph (AGENTS.md "exclude historical evolution").
- ConfigurationValidation.ts: collapse the duplicated pipeline narration
(the function-level JSDoc already enumerates stages, the inline
// Stage N — markers were removing them); shorten the
ConfigurationValidationError class docstring; tighten transformConfiguration
JSDoc and drop the aspirational "future cross-field invariants" sentence;
collapse the 5-line re-entrancy comment to a 2-line rationale.
- ConfigurationSchema.ts: fix two precision defects in JSDoc — stale
module name (ConfigurationMigration → ConfigurationMigrations) and wrong
subject (numberOfStation is not in the remap table; nothing auto-migrates
it). Disambiguate WorkerConfigurationSchema description (resourceLimits
is bridged, not deprecated; elementStartDelay is the deprecated alias).
- Configuration.ts: shrink runReloadLoop and performReload JSDocs to two-
line summaries matching the surrounding private-method conventions in
the same file; collapse the 4-line ESLint-disable rationale to a single
-- suffix on the disable directive.
- Test fixtures and headers: drop internal review codes (B1/B3/B4/B6/N7/N8/
Gap-7/RG-4) from JSDocs and test names — they had no lookup table and
would be opaque to future maintainers; trim test-file headers to the
one-line Template* shape.
* refactor(config): address post-audit findings on validation pipeline
* fix(config): drop incompatible v0 remaps and tighten validation invariants
Addresses 4 PR review findings cross-validated by 2 oracles.
- ConfigurationMigrations: drop `useWorkerPool` (boolean→enum),
`distributeStationToTenantEqually` and `distributeStationsToTenantsEqually`
(boolean→enum), and `uiWebSocketServer` (legacy shape→strict object) to
`null` (warn-and-delete) — same pattern as `workerPoolStrategy` and
`autoReconnectMaxRetries`. v0 configurations carrying any of these
legacy keys would otherwise auto-write a value the strict schema
rejects, killing the simulator at boot. Migration now warns and asks
the user to set the canonical key explicitly. Fixes the BLOCKING
finding from the PR review (regression on 'v0 configs remain valid').
- ConfigurationMigrations: remove redundant `out.$schemaVersion = 1`
from `migrateV0ToV1`. The `applyConfigurationMigration` loop is the
single owner of version stamping; per-step writes are silently
overwritten and become misleading once a second migration is added.
- ConfigurationSchema: tighten `StationTemplateUrlSchema.numberOfStations`
from `.nonnegative()` to `.positive()` — `0` was schema-valid but
semantically meaningless (a template entry that spawns no station)
and conflicts with `isValidNumberOfStations()` which rejects `<= 0`.
Deprecated `numberOfStation` and `provisionedNumberOfStations` keep
`.nonnegative()` (back-compat / 'none provisioned' is meaningful).
- ConfigurationValidation: import `isEmpty` directly from
`utils/Utils.js` instead of the `utils/index.js` barrel, which
re-exports `Configuration` and creates the cycle
`ConfigurationValidation → utils/index → Configuration → ConfigurationValidation`.
Mirrors the existing direct-path `BaseError` import in
`ConfigurationMigrations.ts` for the same TDZ-cycle-avoidance reason.
* docs(config): fix precision defects in schema JSDocs and README
- ConfigurationSchema.ts: correct StorageConfigurationSchema JSDoc —
'URI' is accepted but not auto-migrated (not in DEPRECATED_KEY_REMAPPINGS);
narrow ConfigurationSchema JSDoc meta-test scope claim to 'top-level
and worker.* keys' to match actual test coverage.
- README.md: clarify that deprecated-key remapping is unconditional
(runs on every load, not only on v0 migration).
* fix(config): keep numberOfStations as .nonnegative() to preserve disabled-entry convention
Revert the .positive() tightening from 95cd26dd: docker/config.json ships
5 stationTemplateUrls entries with `numberOfStations: 0` (used as a
'keep on disk but disabled' convention), and this file is copied to
src/assets/config.json by docker/Dockerfile at image build time. With
.positive(), Docker images built from this branch would fail to start
on validateConfiguration's strict parse → process.exit(1).
Bootstrap loops already tolerate 0 (no-op, no crash). The original
.nonnegative() correctly models the on-disk convention; isValidNumberOfStations()
in UIServerSecurity.ts is for runtime UI add-station requests, a
separate concern from config-file parsing.
* docs(config): trim re-entrancy and clone rationales to one-liners
Jérôme Benoit [Tue, 26 May 2026 17:40:45 +0000 (19:40 +0200)]
ci(renovate): enforce 3-day minimum release age for npm packages
Extend the Renovate config with the official 'security:minimumReleaseAgeNpm'
preset so that Renovate waits 3 days after publication before creating PRs
for any npm/pnpm dependency. This adds a buffer against unpublished or
freshly-broken releases (e.g. malicious packages, npm unpublish window,
transient registry/lockfile resolution issues).
Add `atomicWriteFile` and `atomicWriteFileSync` to `src/utils/FileUtils.ts`,
implementing the canonical write-then-rename pattern with optional fsync
durability and best-effort temp file cleanup on failure. Errors are funnelled
through the existing `handleFileException` helper so callers stay aligned
with the project-wide file error reporting contract.
Migrate the five non-atomic disk writes uncovered by the audit:
- `BootstrapStateUtils.writeStateFile` replaces its inline tmp+rename with
the new primitive (single source of truth, gains fsync durability).
- `ChargingStation.saveConfiguration` replaces `writeFileSync` so charging
station OCPP configuration JSON cannot be torn by a crash mid-write.
- `JsonFileStorage.storePerformanceStatistics` drops the persistent
`openSync('w')` file descriptor design (which truncated the file at byte
zero on every sample) and uses the atomic primitive instead. Also fixes
the previous fire-and-forget `runExclusive(...).catch()` pattern by
awaiting the lock.
- `OCPP20CertificateManager` writes installed PEM certificates atomically.
Add `FileType.Certificate` so PEM writes can flow through
`handleFileException` with an accurate file type label.
Concurrent writers to the same path must still be serialized externally
(typically via `AsyncLock`); the primitive does not implement an internal
queue, matching how every existing call site already locks. The
`createWriteStream` diagnostics archive in OCPP 1.6 `GetDiagnostics` is
intentionally left as-is since the file is ephemeral (FTP-uploaded then
discarded).
Targets Node >= 22 so `writeFile`/`writeFileSync` natively expose the
`flush: true` option used for the fsync step.
* fix(utils): align atomic write call sites on a single error path
Address review feedback on PR #1871. The atomic write primitive logs and
re-throws by default via `handleFileException`. Three call sites kept
their pre-migration outer error wrappers, producing double handling:
- `ChargingStation.saveConfiguration`: the `.catch` handler attached to
the fire-and-forget `AsyncLock.runExclusive(...).catch(...)` chain
re-threw inside the catch callback, leaking an unhandled promise
rejection on every config write failure (disk full, EACCES, EROFS,
...). Pass `{ throwError: false }` to that `handleFileException`
call so the rejection is fully absorbed. The retry semantics are
preserved: when `atomicWriteFileSync` throws, the `endMeasure` and
`configurationFileHash` updates inside the lock callback are skipped,
so the next `saveConfiguration` invocation will retry the write.
- `JsonFileStorage.storePerformanceStatistics`: drop the redundant
outer `try/catch` and pass `{ throwError: false }` to the primitive,
matching the `BootstrapStateUtils.writeStateFile` template. Failures
now produce a single error log instead of one error log followed by
a second warn-level log.
- `OCPP20CertificateManager.storeCertificate`: replace the empty
`logPrefix` with a string carrying `stationHashId` and the module
origin so the new error log carries actionable context. The outer
`try/catch` in `storeCertificate` only stringifies the error into
the structured `{ success: false, error }` result and does not call
`handleFileException`, so there is no double handling here — only
the missing context to fix.
* refactor(utils): consolidate atomic write options and address audit findings
- FileUtils: include threadId in temp filename for cross-worker uniqueness;
fold errorParams into AtomicWriteOptions (single trailing options bag);
document SIGKILL leak, durability scope, and per-field defaults; drop
redundant 'utf8' as BufferEncoding cast.
- BootstrapStateUtils.writeStateFile: drop the undefined placeholder.
- ChargingStation.saveConfiguration: route fs failures (already logged at
error by handleFileException) to debug, and surface non-fs failures from
the lock body at error level via a typeof-guarded ErrnoException check.
- OCPP20CertificateManager.storeCertificate: take a required logPrefix from
the caller (replaces the synthetic prefix); drop the redundant manual
mkdir+pathExists block (atomicWriteFile's default ensureDir handles it);
write certificates with mode 0o600; document the per-path serialization
rationale (paths keyed by certificate serial number). Both
OCPP20IncomingRequestService callers updated to pass
chargingStation.logPrefix(); the manager unit tests are migrated.
- JsonFileStorage.storePerformanceStatistics: migrate to AtomicWriteOptions
with errorParams; set flush: false on this hot telemetry path (durability
across crashes is not required for performance records); add 5 focused
unit tests covering happy path, overwrite, Map serialization, runtime
storage directory removal, and parallel writes.
- FileUtils tests: tighten assert.throws/rejects with { code: 'ENOENT' } and
add a deterministic test that asserts the destination remains intact and
the temp file is cleaned up when the rename step fails.
* fix(performance-storage): decode file URIs into native filesystem paths
JsonFileStorage parsed the storage URI via new URL(uri) and used
.pathname directly as a filesystem path. On Windows, file: URLs yield
WHATWG-formatted pathnames such as '/C:/Users/...' which mkdirSync
interprets relative to the current drive, producing corrupt paths like
'D:\\C:\\Users\\...' and breaking JsonFileStorage on windows-latest CI.
Use fileURLToPath() (the standard Node.js inverse of pathToFileURL) to
decode file: URIs into the native path on every platform. Non-file
schemes (typically jsonfile:./relative-path) keep .pathname semantics
for backward compatibility with existing user configurations.
The new tests in tests/performance/storage/JsonFileStorage.test.ts
exercise this code path with pathToFileURL(absolutePath), which now
round-trips correctly across POSIX and Windows.
Jérôme Benoit [Fri, 22 May 2026 01:07:50 +0000 (03:07 +0200)]
chore(deps): update @ai-hero/sandcastle to 0.5.11
Refresh the patch via 'pnpm patch'/'pnpm patch-commit' and pin the
patch key to the exact version so context drift on future bumps
fails loudly instead of applying silently.
Patch semantics are unchanged: it still adds the 'thinking' option
to PiOptions and threads '--thinking <level>' into the pi agent
provider's print command. Only context lines and blob hashes are
refreshed for 0.5.11 (DEFAULT_MODEL bumped to claude-opus-4-7
upstream).
Jérôme Benoit [Mon, 18 May 2026 21:49:47 +0000 (23:49 +0200)]
refactor(bootstrap): factorize UI server start/stop logic
Extract idempotent helpers and remove duplication across the four
UI server lifecycle call sites:
- public startUIServer() now contains the actual logic instead of
delegating to a private wrapper; start() calls it directly.
- new private stopUIServer() helper replaces the two inline stop
blocks in gracefulShutdown() and restart().
- restart() guards syncUIServerTemplates() on uiServerStarted to
avoid redundant template sync (twice when re-enabling, wasted
call when the UI server is disabled or stopped). startUIServer()
performs the sync itself when it actually starts the server.
Jérôme Benoit [Mon, 18 May 2026 00:54:16 +0000 (02:54 +0200)]
fix(sandcastle): resolve TDZ in strategies/index.ts module init
Move the file-private validation regexes `STRATEGY_KEY_PATTERN` and
`CONTROL_TAG_PATTERN` (with their JSDoc verbatim) above the eager call
`STRATEGY_BY_KEY = indexByKey(STRATEGY_REGISTRY)`, so they are
initialized before `indexByKey` reads them at module evaluation.
Jérôme Benoit [Sun, 17 May 2026 22:45:10 +0000 (00:45 +0200)]
feat(sandcastle): make strategy dispatch registry-driven (#1862)
* feat(sandcastle): make strategy dispatch registry-driven
Replace the hardcoded `sandcastle` label / single-strategy implementation
with a registry mapping each strategy key to its actor/critic loop and
finalization config. Labels (`sandcastle-<key>`) and branch prefixes
(`agent/<key>`) are derived from the key, so adding a strategy is one
folder + one registry line.
- New `.sandcastle/strategies/index.ts` (registry, indexed view with
load-time uniqueness assertion, `labelOf`/`branchPrefixOf` helpers).
- `TaskSpec.strategyKey` propagates the chosen strategy through the pipeline.
- `GithubIssueSource` fetches issues per registered label, deduplicates
with a multi-label warning, validates planner output (strategy echo and
branch number bound to issue id), and builds its prompt sanitizer once
from the registry (universal core + per-strategy `controlTags`).
- `GITHUB_ISSUE_LABEL` and `GIT_BRANCH_PREFIX` constants removed; no
back-compat shim.
Cutover (one-time, also noted in `.github/workflows/sandcastle.yml`):
1. `gh label edit sandcastle --name sandcastle-implement`
2. Close or merge open PRs whose head branches start with `agent/issue-`.
* fix(sandcastle): validate strategy key and controlTags at registry load
Extend `indexByKey` to enforce the kebab-case contract on `StrategyEntry.key`
(documented but unchecked) and an XML-name-safe contract on `controlTags`.
Failures throw at module load with a precise message, matching the existing
duplicate-key behaviour, so registry mistakes (typos, wrong casing, empty
tag) cannot silently produce undiscoverable GitHub labels, invalid git
branches or empty regex alternatives in the prompt sanitizer.
* fix(sandcastle): tighten control-tag sanitizer against prefix matches
The deny-list regex `</?(?:tag1|tag2)[^>]*>` matched tag-name prefixes
because `[^>]*` swallowed the trailing characters: `<plant>` matched
alternative `plan`, `<reviewer>` matched `review`, and so on. Insert a
zero-width lookahead `(?=[\\s/>])` after the alternation so only XML
tag-name terminators (whitespace, `/`, or `>`) are accepted. The
deny-list remains entirely registry-driven.
* refactor(sandcastle): parallelise issue fetch and clarify multi-label warning
- Fan out per-strategy `gh issue list` calls with `Promise.all`. The
dedup pass still iterates the resolved array in registry order, so the
"first registered wins" semantics is preserved. Failure semantics are
unchanged: any rejected fetch propagates as before.
- Render labels via `labelOf` (no more hard-coded `sandcastle-*` glob),
name both winner and dropped GitHub labels, and append a ready-to-paste
`gh issue edit --remove-label` remediation hint.
* fix(sandcastle): reject prefix-overlapping strategy keys at registry load
Two registered keys related by a kebab-prefix (e.g. 'foo' and 'foo-1') yield
overlapping branch-detection regexes: a branch 'agent/foo-1-42-slug' is matched
by both '^agent/foo-(\d+)-' (capturing '1' as the issue id) and
'^agent/foo-1-(\d+)-' (capturing '42'), and the registry-order tie-break in
`fetchIssuesWithOpenPRs` then attributes the open PR to the wrong issue.
Extend `indexByKey` with a pairwise prefix-incomparable check that throws at
module load with a precise message, in line with the existing
`STRATEGY_KEY_PATTERN`, `CONTROL_TAG_PATTERN`, and duplicate-key checks
introduced in 89be4bd3. Today's single-entry registry passes unchanged.
* refactor(sandcastle): unify prompt taxonomy and decouple planner from orchestrator
Adopt the Inputs/Task/Output/Rules/Done section structure across the
plan, actor and critic prompts; drop `Agent` from the role headers;
deduplicate intra-prompt boilerplate (verbatim instructions, repeated
quality-gate blocks, multiply-explained confidence levels). Total prompt
size: 217 \u2192 142 lines.
Decouple the planner output from registry-known data: the planner now
emits a kebab-case `slug` only; the orchestrator builds
`branch = \`\${source.branchPrefix}-\${id}-\${slug}\`` and copies
`strategyKey` from the registry-resolved source. The validator drops the
`strategyKey` echo check and the full-branch regex, validates the slug
shape via `SLUG_PATTERN` and a `MAX_SLUG_CHARS` cap. The planner JSON
view is also projected to `{ body, labels, number, title }` so
`branchPrefix`/`strategyKey` cannot leak back into the prompt.
Rename actor variable `TASK_ID` \u2192 `ISSUE_NUMBER` for precision; drop the
`## Planner Analysis` section header from `buildPlanContext` so
`{{PLAN_CONTEXT}}` interpolates cleanly inside the actor's Inputs
section.
The sandcastle library's PromptArgumentSubstitution
(`@ai-hero/sandcastle/dist/PromptArgumentSubstitution.js`) applies the
regex \`/\\{\\{\\s*([A-Za-z_][A-Za-z0-9_]*)\\s*\\}\\}/g\` globally with no
Markdown awareness, so a placeholder appearing both inside an
`## Inputs` bullet (`\`{{KEY}}\` \u2014 description.`) and on a bare line
below was substituted twice. For multi-line values (`ISSUE_BODY`,
`PLAN_CONTEXT`, `FINDINGS`, `ISSUES_JSON`, `ACCEPTANCE_CRITERIA`)
this corrupted the inline-code span and doubled the prompt token cost
on every actor and critic round.
Reference variables by bare name (`\`KEY\``) in the `## Inputs` bullets
and in narratives that mention multi-line values; keep the bare-line
`{{KEY}}` injections downstream as the single substitution sites.
Scalar variables (`ISSUE_NUMBER`, `ISSUE_TITLE`, `BRANCH`,
`BASE_BRANCH`, `NONCE`) remain interpolated where their single-line
value belongs in the rendered prose or commands.
* refactor(sandcastle): centralize MAX_SLUG_CHARS in constants module
`MAX_SLUG_CHARS` is a tunable peer of `MAX_TITLE_CHARS`,
`CONTEXT_HASH_RADIUS`, and `HASH_PREFIX_LENGTH`, all already in
`constants.ts`. Move it there to honour the AGENTS.md "single source of
truth: canonical defaults" rule. `SLUG_PATTERN` stays local in
`task-source.ts`: it is a structural validator regex, not a tunable.
* fix(sandcastle): align PR-coverage branch regex with slug grammar
The PR-coverage regex used to dedup open PRs in `fetchIssuesWithOpenPRs`
omitted the trailing slug, while `validatePlanEntry` enforces a strict
kebab-case slug shape via `SLUG_PATTERN`. A stale or malformed branch
like `agent/implement-42-` would match the dedup regex and suppress
retries forever.
Lift the slug body into a shared `SLUG_PATTERN_BODY` constant reused by
both `SLUG_PATTERN` (the plan-entry validator) and the per-strategy
`branchPatterns` so the two definitions stay in lock-step.
* fix(sandcastle): sanitize planner-supplied issue title at trust boundary
`validatePlanEntry` validated typeof, length and control characters on
`item.title` but stored the raw value verbatim in `TaskSpec`, exposing
both the actor prompt (`{{ISSUE_TITLE}}`) and the GitHub PR title (via
`finalizer.ts`) to control tags such as `<system>` or `<plan>` that a
prompt-injected planner could echo from an issue body.
Route `item.title` through the same registry-derived `sanitizeForPrompt`
already used for `body`, `rootCauseHypothesis` and `acceptanceCriteria`,
trim whitespace, and reject the entry if the sanitised title is empty.
Length and control-character checks remain on the raw input as a fast
fail-fast path before sanitisation.
* fix(sandcastle): retry on all-invalid plan and dedupe planner ids
`validatePlan` returned `[]` both for legitimate empty plans and for
plans where every entry failed validation. The caller treats `[]`
identically ("No actionable issues. Exiting."), so a systemic planner
mistake silently terminated the nightly run as if there were genuinely
no work, wasting the entire retry budget without diagnostic.
It also accepted duplicate ids: a planner emitting two entries for the
same issue number produced two `TaskSpec`s sharing one branch, so
`main.ts` would spawn two concurrent sandboxes and create duplicate PRs.
Restructure `validatePlan` to:
- Track `seenIds: Set<string>`; on duplicate, warn and drop the second
occurrence (first registered wins, mirrors the multi-label dedup
pattern in `fetchAndSanitizeIssues`).
- Detect `parsed.issues.length > 0 && validated.length === 0`: log an
error and return `null` so the existing retry loop at lines 119-177
re-invokes the planner instead of exiting silently.
feat: resolve #314 — Add charging station template Zod validation with schema versioning (#1860)
* feat: add charging station template Zod validation with schema versioning
Add Zod v4 runtime validation for charging station templates with
schema versioning support. This replaces scattered imperative checks
with a declarative schema pipeline that validates, migrates, and
transforms templates at parse time.
New files:
- TemplateMigrations.ts: CURRENT_SCHEMA_VERSION, coerceVersion(),
migration registry with migrateV0ToV1()
- TemplateSchema.ts: Zod schemas with topology union, loose + strict
variants, MeterValues value coercion (number -> string)
- TemplateValidation.ts: validateTemplate() pipeline,
transformTemplate(), TemplateValidationError
Integration:
- getTemplateFromFile() now calls validateTemplate() instead of bare
JSON.parse(...) as ChargingStationTemplate
- Cache key updated to hash:vN format incorporating schema version
- Removed checkTemplate(), checkConnectorsConfiguration(),
checkEvsesConfiguration(), warnTemplateKeysDeprecation() from
Helpers.ts (logic absorbed into schema/pipeline)
- Exported getConfiguredMaxNumberOfConnectors and
getMaxNumberOfConnectors from Helpers.ts for connector setup
Template changes:
- Added "$schemaVersion": 1 to all 15 template JSON files
- coerceVersion(null|undefined) now returns 0 so legacy templates
without $schemaVersion go through v0->v1 migration; deprecated keys
(supervisionUrl, authorizationFile, payloadSchemaValidation,
mustAuthorizeAtRemoteStart) are renamed instead of being silently
swallowed by looseObject [HIGH]
- transformTemplate uses Math.max(numberOfConnectors[]) as the
worst-case bound for the randomConnectors auto-trigger; new
Helpers.ts pair getMaxConfiguredNumberOfConnectors (deterministic
upper bound) and pickConfiguredNumberOfConnectors (random pick)
centralize array semantics; getConfiguredMaxNumberOfConnectors now
delegates to pickConfiguredNumberOfConnectors [HIGH]
- BaseTemplateSchema.$schemaVersion is now z.literal(CURRENT_SCHEMA_VERSION)
(post-migration assertion); deprecated keys removed from the v1
schema and explicitly rejected by a superRefine block with a clear
diagnostic [MEDIUM]
- transformTemplate drops the unreachable templateMaxConnectors < 0
branch [LOW]
- validateTemplate widens to accept unknown and rejects null,
non-plain-object and array payloads with a clear BaseError instead
of crashing later in coerceVersion [LOW]
- SignedMeterValueSchema, UnitOfMeasureSchema and WsOptionsSchema
replace z.looseObject({}) with typed shapes plus .catchall(z.unknown())
for forward-compatible vendor extensions [LOW]
- TemplateValidationError now surfaces '(migrated from vX -> vY)' in
its message so post-migration validation failures are visible to
operators
Tests: update coerceVersion(null|undefined) expectation to 0; add
end-to-end legacy migration, deprecated-key rejection (Schema and
Validation layers), null/string/array payload guards, array-form
numberOfConnectors auto-trigger regression, dead-branch regression,
migratedFrom message presence, and unit tests for the two new
Helpers.ts helpers.
- normalize $schemaVersion to coerced integer in validateTemplate so the
no-migration path (when version === CURRENT) also satisfies the strict
z.literal(CURRENT_SCHEMA_VERSION) check; was failing for user-authored
templates with string "$schemaVersion": "1" [F1]
- harmonize coerceVersion error wording on "non-negative integer" across
all rejection branches; the previous "positive integer" message was
contradictory since 0 is a valid input (legacy/pre-versioning sentinel
triggering v0 migration) [F2]
- introduce SCHEMA_VERSION_STRING_PATTERN (^\\d+$) gate in coerceVersion
to reject permissive Number() coercions ('1.0', '0x1', '1e0', ' 1 ',
'', '+1', '01a'); pattern mirrors the canonical non-negative-integer
string pattern used in TemplateSchema for connector/EVSE keys [F6]
- replace for...in with Object.entries destructuring in Evses topology
validation, harmonizing with the prior-art pattern in Helpers.ts and
removing the redundant template.Evses[evseKey] re-lookup [F3]
- tighten getMaxConfiguredNumberOfConnectors cast to readonly number[]
matching the helper signature [F5/F7]
- document the cache-bound warning emission frequency in
transformTemplate's JSDoc: warnings fire once per (templateFile,
schemaVersion) cache miss rather than per station instance, which is
template-scoped on purpose [F4]
Tests: assert string "$schemaVersion": "1" is accepted and normalized
to numeric 1; battery of 8 permissive numeric strings rejected with
harmonized wording; coerceVersion rejection branches all use the same
"non-negative integer" diagnostic.
* fix(template): broaden wsOptions.headers and harden template pipeline
- templateHash uses SRI-style `${algorithm}:${schemaVersion}:${contentHash}`
computed over the validated template, restoring whitespace-insensitive
cache semantics from pre-PR main while keeping schema-version-bump
invalidation deterministic.
- Migration registry refactored to sequential n→n+1 chain so future
schema versions append one function instead of rewriting prior
migrations (no behavior change at v1).
- validateTemplate clones its input via structuredClone at the
validation boundary, honoring the repository immutability convention.
BREAKING CHANGE: external log monitors keyed on the literal string
"Failed to read charging station template file" must also match
"Invalid charging station template payload (not a JSON object)" for
JSON-shape errors; file I/O errors retain their previous wording via
handleFileException.
* test: extract mockLoggerWarnDebug helper to dedupe migration logger spies
Replaces 8 occurrences of the warn+debug t.mock.method pair across
TemplateMigrations.test.ts and TemplateValidation.test.ts with a typed
helper sibling to createLoggerMocks.
* chore(deps): deduplicate pnpm-lock entries for @rolldown/pluginutils and ws
feat: resolve #1020 — Persist minimal simulator state and reconstruct template indexes on startup (#1858)
* feat: persist minimal simulator state and reconstruct template indexes on startup
- Add persistState boolean to ConfigurationData (default: true)
- Add SimulatorState to FileType enum
- Add simulatorState to AsyncLockType enum
- Implement BootstrapStateUtils with:
- readStateFile: reads and validates state.json with schema version check
- writeStateFile: atomic write via tmp+rename with AsyncLock
- reconstructTemplateIndexes: scans per-station config files to rebuild indexes
- deleteStateFile: safe file deletion
- Integrate into Bootstrap:
- reconstructTemplateIndexes called in start() before worker spawn
- State file written on start() (started:true) and stop() (started:false)
- shouldAutoStart() reads state file to control auto-start behavior
- persistStateEnabled getter checks persistState config and SIMULATOR_COLD_START env
- Update start.ts to conditionally start based on shouldAutoStart()
- Handle edge cases: corrupt files, missing fields, incompatible schema version
- Add comprehensive tests for all state persistence and reconstruction scenarios
Closes #1020
* refactor(bootstrap): harmonize persisted state feature with codebase conventions
Address review findings on PR #1858:
- HIGH #1: Phase split start() lifecycle. Add public Bootstrap.startUIServer()
that always brings up the UI server (and reconstructs template indexes
before accepting requests). start.ts unconditionally calls startUIServer()
and only gates Bootstrap.start() on shouldAutoStart(), so a persisted
stopped state no longer turns the simulator into a zombie process.
- HIGH #2: Add canonical default. New defaultPersistState constant and
Configuration.getPersistState() accessor matching the existing
defaultUIServerConfiguration / defaultWorkerConfiguration pattern.
Bootstrap.persistStateEnabled now delegates instead of inlining ?? true.
- MEDIUM #3: Document persistState tunable and SIMULATOR_COLD_START
environment override in README.md.
- MEDIUM #4: writeStateFile and deleteStateFile now swallow filesystem
errors via handleFileException with throwError:false, mirroring
watchJsonFile and the storage layer. Persistence failures no longer
surface as misleading 'Startup error' / 'Shutdown error'.
- MEDIUM #5: reconstructTemplateIndexes runs inside startUIServer() before
uiServer.start(), closing the race where UI requests could arrive before
index reconstruction completes.
- LOW #7: Add formatLogPrefix() utility and use it in BootstrapStateUtils,
removing the leading-space artifact when logPrefixFn is undefined.
- LOW #8: On state file schema mismatch, quarantine to <path>.v<N>.bak
instead of silently deleting, allowing forensics during partial schema
rollouts. JSON parse errors still delete (true corruption, unrecoverable).
- LOW #9: Move state.json from dist/assets/configurations/ to dist/assets/.
Drops the fragile basename-based filter from reconstructTemplateIndexes
and separates control file from per-station configuration files.
Drop shouldAutoStart() from IBootstrap interface (boot-time concern, no UI
service consumer needs it). The method stays public on the concrete
Bootstrap class for start.ts.
Tests: align fixtures with new state.json location, add quarantine assertion,
add non-fatal writeStateFile assertion, drop obsolete state.json filter test.
* test(bootstrap): align BootstrapStateUtils tests with project style guide
- Rename BootstrapState.test.ts to BootstrapStateUtils.test.ts to match the
source module name (TEST_STYLE_GUIDE.md §1: 'Files: ModuleName.test.ts').
- Add mandatory standardCleanup() in afterEach (TEST_STYLE_GUIDE.md §3),
matching the convention used by Configuration.test.ts, FileUtils.test.ts,
ErrorUtils.test.ts and ConfigurationKeyUtils.test.ts.
* fix(bootstrap): address review findings on persisted simulator state
Distinguish UI-initiated stop from signal/restart via a private
StopReason enum, so SIGINT/SIGTERM/SIGQUIT and config-reload restarts
no longer flip the persisted state to stopped (HIGH-1).
Short-circuit persistStateEnabled when the UI server is disabled, since
persistence has no recovery channel without a UI; warn once on the
inconsistent configuration (HIGH-2).
Reconstruct template indexes via a shared prepareTemplateStatistics
helper called from constructor and restart, not only from startUIServer,
to prevent index collisions on config-reload of UI-added stations
(MED-3).
Move state file from dist/assets/state.json (wiped by pnpm build) to
dist/assets/configurations/.simulator-state.json; filter dot-prefixed
metadata files in reconstructTemplateIndexes so the state file co-located
with charging station configurations is silently skipped (MIN-10).
Clean up the .tmp file on atomic-rename failure in writeStateFile and
guard readStateFile against JSON null/primitive/array content with an
explicit message (MIN-7, MIN-8).
Unify path computations into readonly assetsDir/configurationsDir/
stateFilePath fields, deduplicate setChargingStationTemplates via a
syncUIServerTemplates helper, and route the SIMULATOR_COLD_START env
var name through Constants.ENV_SIMULATOR_COLD_START (MIN-5, MIN-6).
Export DEFAULT_PERSIST_STATE and add the persistState entry to
config-template.json so the tunable is discoverable (MED-4).
Update README to document the new state file path, the UI server
requirement, and the signal-shutdown semantics.
Test coverage added for tmp cleanup, JSON null/primitive/array guards,
and dot-prefixed metadata filtering; the misleading 'read-only
directory' test is renamed to reflect the actual OS-rejected-path
scenario (MIN-9).
* docs(bootstrap): clarify persisted state semantics from review feedback
- README: drop developer-internals sentence on template index reconstruction
from the persistState row
- TemplateStatistics: document the `added` (process-scoped) vs `indexes`
(process+disk-scoped) distinction exposed via SimulatorState
- IBootstrap: document the contract scope (UI-server-facing, excluding
process-lifecycle helpers and the internal StopReason)
- formatLogPrefix: document the trailing-space contract
- reconstructTemplateIndexes: refine the warn message wording for non-station
configuration files (level unchanged)
* refactor(utils): default formatLogPrefix prefix to logPrefix
Avoids silent loss of the timestamp when callers do not pass a module-specific
prefix function. Aligns with the existing convention where every module-level
logPrefix delegates to Utils.logPrefix.
Jérôme Benoit [Mon, 11 May 2026 19:20:30 +0000 (21:20 +0200)]
refactor: consolidate object-check utilities to eliminate duplication
Replace type() + isObject with a single isPlainObject helper.
Make isJsonObject and assertIsJsonObject delegate through it
instead of duplicating the check logic independently.
Jérôme Benoit [Mon, 11 May 2026 18:43:22 +0000 (20:43 +0200)]
refactor: remove unnecessary type assertions across monorepo
- Remove all @typescript-eslint/no-unnecessary-type-assertion violations
- Add assertIsJsonObject/isJsonObject utilities for runtime-safe narrowing
- Restructure OCPP16RequestService.buildRequestPayload with proper
runtime validation (assertIsJsonObject + OCPPError) replacing unsafe cast
- Use _syncResult assignment pattern in OCPPResponseService for
no-floating-promises compliance while preserving isAsyncFunction pattern
- Refine AsyncLock.runExclusive fn parameter as union of function types
for proper isAsyncFunction type guard narrowing
- Configure varsIgnorePattern: '^_' for @typescript-eslint/no-unused-vars
- Remove unused type imports in test files
renovate[bot] [Mon, 11 May 2026 13:14:03 +0000 (15:14 +0200)]
chore(deps): update pnpm to v11 (#1850)
* chore(deps): update pnpm to v11
* fix: migrate pnpm settings from package.json to pnpm-workspace.yaml
pnpm 11 no longer reads the 'pnpm' field from package.json.
Move patchedDependencies and allowBuilds to pnpm-workspace.yaml.
Remove dead pnpm field from package.json.
fix: resolve #1244 — add per-connector maximum power support (#1843)
* feat(charging-station): add per-connector maximum power support
Add maximumPower field to ConnectorStatus representing the physical
limitation of each connector cable/plug (thermal current rating).
Per OCPP Device Model, AvailablePowerMaxLimit is defined at the
Connector component level. The connector maximumPower acts as a
hardware cap in the power computation pipeline alongside the station-
level powerDivider sharing mechanism.
- Add ConnectorStatus.maximumPower?: number (in W)
- Initialize at boot via initializeConnectorsMaximumPower(): default
is stationPower / staticConnectorCount (using static count, not
dynamic powerDivider which can be 0 in shared mode at init)
- Clamp in getConnectorMaximumAvailablePower as additional min() term
- Use in getConnectorChargingProfilesLimit as primary cap (falls back
to stationPower/powerDivider for backward compat)
- Update 6 shared-mode templates with explicit maximumPower per
connector (= station power for DC shared-bus stations)
Resolves #1244
* chore(sandcastle): update validation and main scripts
* chore: sync release-please manifests and sandcastle prompt
* fix(charging-station): exclude index 0 from staticCount in getDefaultConnectorMaximumPower
The staticCount calculation included EVSE 0 and connector 0, while runtime
getPowerDivider excludes them. This caused connector hardware caps to be
more restrictive than intended (e.g., stationPower/3 instead of
stationPower/2 on a 2-EVSE station with EVSE 0 defined).
* [autofix.ci] apply automated fixes
* refactor(charging-station): add NaN guard to connectorHardwareMaximumPower in min()
Align the connectorHardwareMaximumPower entry with the same null/NaN guard
pattern used by all other entries in the min() call for consistency and
defensive robustness.
* docs: document per-connector maximumPower in template examples
Add maximumPower field to Connectors and Evses section examples.
Clarify powerSharedByConnectors behavior description.
Jérôme Benoit [Fri, 8 May 2026 18:45:44 +0000 (20:45 +0200)]
fix(ui/web): smooth icon-btn danger hover shadow and remove dead token
- Add box-shadow to .modern-icon-btn transition shorthand so the danger
variant hover glow animates instead of snapping
- Remove --skin-shadow-color (declared line 61 but never consumed)
Jérôme Benoit [Fri, 8 May 2026 18:30:30 +0000 (20:30 +0200)]
fix(ui/web): fix editable pill hover visibility in light themes
Refactor pill background to use scoped --_pill-bg and --_pill-bg-hover
custom properties (MD3 pattern). Each variant co-locates its base and
hover background values, and light-mode overrides reassign both.
The hover rule simply consumes var(--_pill-bg-hover), eliminating the
specificity battle that prevented the hover effect from appearing in
light themes.
Jérôme Benoit [Fri, 8 May 2026 18:00:49 +0000 (20:00 +0200)]
refactor(ui/web): remove redundant connector status column from classic skin
The connector status is now accessible via the Set Status dropdown in
the Actions column, making the dedicated read-only Status column
redundant. Move status/error-code selects to the top of Actions and
style them to fill the column width consistently with buttons.
Jérôme Benoit [Fri, 8 May 2026 15:08:22 +0000 (17:08 +0200)]
fix(sandcastle): patch pi thinking option and replace type indirections
Apply pnpm patch for @ai-hero/sandcastle porting PR #584 (pi --thinking
flag). Wire the thinking option through agentProvider() and replace
Awaited<ReturnType<...>> indirections with direct type imports (Sandbox,
SandboxRunResult, RunResult, PiOptions).
repositories / e-mobility-charging-stations-simulator.git / log
