From 7a0b42f602893143ae6e42c449c12caf21bb069c Mon Sep 17 00:00:00 2001 From: sepehr-safari Date: Wed, 22 Jul 2026 14:01:18 +0300 Subject: [PATCH] fix(core): track STATUS_TRANSITION_VIOLATION status per connector The rule validated connector status transitions against a single previous status shared across all connectors, so on a multi-connector station an interleaved notification from a different connector (connector 1 going Available while connector 2 goes Finishing) was reported as an invalid transition. Status is now tracked per connectorId (connectorId 0, the charge point as a whole, forms its own series), and each transition is validated only within one connector's series. Genuine per-connector violations are still detected, so the scenario corpus and conformance contract are unchanged. Adds three regression tests. Reported by shiv3 from the ocpp-cp-simulator integration. --- .../fix-status-transition-per-connector.md | 5 +++ CURRENT_STATE.md | 29 +++++++++----- packages/toolkit/src/core/detection.test.ts | 40 +++++++++++++++++++ packages/toolkit/src/core/detection.ts | 27 ++++++++----- 4 files changed, 81 insertions(+), 20 deletions(-) create mode 100644 .changeset/fix-status-transition-per-connector.md diff --git a/.changeset/fix-status-transition-per-connector.md b/.changeset/fix-status-transition-per-connector.md new file mode 100644 index 0000000..4da95cf --- /dev/null +++ b/.changeset/fix-status-transition-per-connector.md @@ -0,0 +1,5 @@ +--- +'@ocpp-debugkit/toolkit': patch +--- + +Fix `STATUS_TRANSITION_VIOLATION` false positives (#128). The rule validated connector status transitions against a single previous status shared across all connectors, so on a multi-connector station an interleaved notification from a different connector (for example connector 1 going `Available` while connector 2 goes `Finishing`) was reported as an invalid transition. Status is now tracked per `connectorId` (connectorId 0, the charge point as a whole, forms its own series), and each transition is validated only within one connector's series. Reported by shiv3 from the ocpp-cp-simulator integration. diff --git a/CURRENT_STATE.md b/CURRENT_STATE.md index 33e6993..3f951b0 100644 --- a/CURRENT_STATE.md +++ b/CURRENT_STATE.md @@ -4,20 +4,21 @@ ## Current Version -`0.4.0`, Open OCPP Trace Interop (published 2026-07-17). A `0.4.1` patch is in -progress (METER_VALUE_ANOMALY false-positive fix, #127). +`0.4.1`, METER_VALUE_ANOMALY fix (published 2026-07-22). A `0.4.2` patch is in +progress (STATUS_TRANSITION_VIOLATION per-connector fix, #128). ## Active Milestone -**v0.4.1 - detection correctness patch, then v0.5.0 (OCPP 2.0.1)** +**v0.4.x - detection correctness patches, then v0.5.0 (OCPP 2.0.1)** v0.4.0 shipped the Open OCPP Trace interop: the toolkit reads and writes the shared v1.1 interchange format (input adapter #121, exporter + `convert` CLI -#122), checked against the specification's conformance fixtures. **0.4.1** -fixes a false-positive bug shiv3 found from the simulator integration: the -`METER_VALUE_ANOMALY` rule flattened all measurands and connectors into one -series (#127). Next milestone is **v0.5.0 (OCPP 2.0.1)**, where the same -per-connector/EVSE model this fix introduces is required across detection. +#122), checked against the specification's conformance fixtures. Two +false-positive bugs shiv3 found from the simulator integration are being +fixed: `METER_VALUE_ANOMALY` measurand/connector flattening (#127, shipped in +0.4.1) and `STATUS_TRANSITION_VIOLATION` global status tracking (#128, in +0.4.2). Next milestone is **v0.5.0 (OCPP 2.0.1)**, where the per-connector / +EVSE model these fixes introduce is required across detection. ## What's Done @@ -30,8 +31,16 @@ per-connector/EVSE model this fix introduces is required across detection. - ✅ Eliminates false positives on multi-measurand samples and multi-connector stations; genuine energy-register anomalies still detected (meter-anomaly scenario and conformance contract unchanged); 3 regression tests added -- Companion issue drafted for the same connector-blindness in - `STATUS_TRANSITION_VIOLATION` (rule 8) +- Sibling connector-blindness in `STATUS_TRANSITION_VIOLATION` fixed under #128 + +### STATUS_TRANSITION_VIOLATION per-connector fix (0.4.2, Issue #128) + +- ✅ Rule 8 now tracks the previous status per `connectorId` (connectorId 0, + the whole charge point, forms its own series) and validates transitions only + within one connector's series, instead of one global sequence +- ✅ Eliminates false violations on multi-connector stations; genuine + per-connector violations still detected (status-transition-violation scenario + and conformance contract unchanged); 3 regression tests added ### GitHub Infrastructure diff --git a/packages/toolkit/src/core/detection.test.ts b/packages/toolkit/src/core/detection.test.ts index a47c9a8..a0c50df 100644 --- a/packages/toolkit/src/core/detection.test.ts +++ b/packages/toolkit/src/core/detection.test.ts @@ -664,6 +664,46 @@ describe('detectFailures', () => { const failures = detectFailures(events, sessions); expect(failures.some((f) => f.code === 'STATUS_TRANSITION_VIOLATION')).toBe(false); }); + + // Regression tests for the per-connector tracking fix (#128). + const statusEvent = (id: string, connectorId: number, status: string, ts: number): Event => + makeEvent(id, id, 'Call', 'StatusNotification', { connectorId, status }, ts); + + it('does not flag across connectors when interleaved statuses only look invalid globally (#128)', () => { + const events = [ + statusEvent('e1', 1, 'Charging', 0), + statusEvent('e2', 2, 'Available', 500), + statusEvent('e3', 1, 'Finishing', 1000), + ]; + const failures = detectFailures(events, buildSessionTimeline(events)); + expect(failures.some((f) => f.code === 'STATUS_TRANSITION_VIOLATION')).toBe(false); + }); + + it('still flags a genuine per-connector violation when another connector is interleaved', () => { + const events = [ + statusEvent('e1', 1, 'Available', 0), + statusEvent('e2', 2, 'Charging', 500), + statusEvent('e3', 1, 'Finishing', 1000), + ]; + const violations = detectFailures(events, buildSessionTimeline(events)).filter( + (f) => f.code === 'STATUS_TRANSITION_VIOLATION', + ); + expect(violations).toHaveLength(1); + expect(violations[0]?.eventIds).toEqual(['e1', 'e3']); + }); + + it('tracks connectorId 0 (whole charge point) as its own series', () => { + const events = [ + statusEvent('e1', 0, 'Available', 0), + statusEvent('e2', 1, 'Available', 500), + statusEvent('e3', 0, 'Finishing', 1000), + ]; + const violations = detectFailures(events, buildSessionTimeline(events)).filter( + (f) => f.code === 'STATUS_TRANSITION_VIOLATION', + ); + expect(violations).toHaveLength(1); + expect(violations[0]?.eventIds).toEqual(['e1', 'e3']); + }); }); describe('DIAGNOSTICS_FAILURE', () => { diff --git a/packages/toolkit/src/core/detection.ts b/packages/toolkit/src/core/detection.ts index c418480..18224ac 100644 --- a/packages/toolkit/src/core/detection.ts +++ b/packages/toolkit/src/core/detection.ts @@ -613,35 +613,42 @@ const VALID_TRANSITIONS: Record> = { function detectStatusTransitionViolation(events: Event[]): Failure[] { const failures: Failure[] = []; - // Collect all StatusNotification statuses in order const statusEvents = events.filter( (e) => e.messageType === 'Call' && e.action === 'StatusNotification', ); - let prevStatus: string | null = null; - let prevEvent: Event | null = null; + // Each connector has an independent status machine, so a transition is only + // valid or invalid relative to the same connector's previous status. Track + // the previous status per connectorId; connectorId 0 refers to the charge + // point as a whole (OCPP 1.6) and forms its own series, and a missing + // connectorId is bucketed under 'unknown'. Comparing statuses across + // connectors (the previous behavior) produced false violations on + // multi-connector stations. + const previousByConnector = new Map(); for (const event of statusEvents) { - const payload = event.payload as { status?: unknown }; + const payload = event.payload as { status?: unknown; connectorId?: unknown }; const status = payload?.status; if (typeof status !== 'string' || !VALID_CONNECTOR_STATUSES.has(status)) continue; - if (prevStatus !== null && prevEvent !== null) { - const allowed: Set | undefined = VALID_TRANSITIONS[prevStatus]; + const connectorId = typeof payload?.connectorId === 'number' ? payload.connectorId : 'unknown'; + const previous = previousByConnector.get(connectorId); + + if (previous) { + const allowed: Set | undefined = VALID_TRANSITIONS[previous.status]; if (allowed && !allowed.has(status)) { failures.push({ code: 'STATUS_TRANSITION_VIOLATION', - description: `Connector status transition from "${prevStatus}" to "${status}" is not a valid OCPP 1.6 transition (messageId: ${event.messageId})`, + description: `Connector status transition from "${previous.status}" to "${status}" is not a valid OCPP 1.6 transition (messageId: ${event.messageId})`, severity: SEVERITY.STATUS_TRANSITION_VIOLATION, - eventIds: [prevEvent.id, event.id], + eventIds: [previous.event.id, event.id], suggestedSteps: SUGGESTED_STEPS.STATUS_TRANSITION_VIOLATION, }); } } - prevStatus = status; - prevEvent = event; + previousByConnector.set(connectorId, { status, event }); } return failures;