Skip to content

Consolidate static MMS server and BRCB server stack onto current main - #51

Merged
masarray merged 55 commits into
mainfrom
agent/complete-server-consolidation
Aug 11, 2026
Merged

masarray merged 55 commits into
mainfrom
agent/complete-server-consolidation

Conversation

@masarray

@masarray masarray commented Aug 11, 2026 •

Copy link
Copy Markdown
Owner

Goal

Repository Consolidation & Truth Pass: move the portable Static MMS Server and retained BRCB R1–R3 server stack off historical branches and onto one current-main lineage, without importing their historical ESP32/SV/URCB ancestry.

Tranche A — Static MMS Server

Ported cleanly from the #41 implementation onto current main lineage.

  • bounded span-based OSI/ACSE/MMS server codecs;
  • static object/DataSet tables and dispatcher;
  • connection runtime + static server session;
  • portable static IED server host adapter;
  • embedded hard-profile smoke coverage;
  • current-main CMake preserved and extended rather than replaced;
  • no-throw UTC decode primitive integrated as a host-compatible superset.

Tranche B — cumulative BRCB R1–R3

Ported from final R3 semantics as a clean cumulative delta, not by merging historical stacked branches.

  • bounded retained replay history and EntryID sequencing;
  • association-scoped reservation/ownership and ResvTms control;
  • association lifecycle bridge and cleanup;
  • operational BRCB MMS objects and selective replay;
  • retained recovery state v2 with v1 restore path;
  • checkpoint store and retry-safe reconnect delivery;
  • only minimal information-report/selective-report wire dependencies imported; URCB/SV ancestry was not imported.

Public build integration

BRCB sources are wired into ARIEC61850::mms_server_core, not left in a private hard-profile target. The portable ariec61850_static_ied_server links only ARIEC61850::mms_server_core (+ ws2_32 on Windows), so the isolated server executable does not inherit the legacy client/process-bus core.

Review-driven safety hardening

The first full PR review identified six material issues. All were addressed before merge consideration:

  1. Association/Owner-bound delivery — BRCB report staging is denied to foreign associations and does not expose/advance retained state.
  2. Two-phase delivery commit — poll only stages a complete frame; the cursor advances only through explicit commit_sent after transport acceptance. Commit re-authorizes the current association + Owner + RptEna and rejects foreign/stale tokens.
  3. Terminal lifecycle closure — every terminal static-server-session path closes MmsStaticConnectionRuntime; an established EOF regression proves the association-close callback fires exactly once.
  4. POSIX SIGPIPE suppression — host-only process guard ignores SIGPIPE; embedded/server core remains policy-free.
  5. A/B recovery tolerance — restore accepts a surviving readable committed bank when the peer bank has I/O failure and can fall back before state application; dedicated hard-profile regression covers both bank directions.
  6. Server dependency isolation — the static server executable no longer links ARIEC61850::core.

Additional hard-profile regression proves a foreign association cannot commit an EntryID staged by the legitimate owner.

Safety gate

Merge is permitted only when the exact final head passes:

  • BRCB hard profile with -fno-exceptions -fno-rtti on GCC/Clang;
  • repo-wide C++ GCC/Clang/MSVC build + tests;
  • embedded profile;
  • Security / ASan / UBSan / fuzz gates;
  • Dynamic Reporting and Smart Control interoperability regression workflows;
  • fresh review with no unresolved P1/P2 safety finding;
  • branch remains current with main and merge uses an expected-head SHA guard.

No hardware/live IED server interoperability or IEC 61850 conformance claim is made by this consolidation PR.

@masarray
masarray marked this pull request as ready for review August 11, 2026 01:12

Copy link
Copy Markdown
Owner Author

@codex review

Please focus on server-side safety and consolidation correctness: bounded static server state machine, BRCB association ownership/lifecycle, retained replay and recovery v2, persistence failure behavior, public mms_server_core integration, and whether any historical ESP32/SV/URCB dependency leaked into this current-main port. Treat live interoperability/conformance as out of scope for this PR.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0912927087

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/mms/static_brcb_connection.cpp
Comment thread src/mms/static_server_session.cpp
Comment thread src/mms/static_brcb_connection.cpp Outdated
Comment thread tools/static_ied_server.cpp
Comment thread src/mms/static_brcb_checkpoint_store.cpp Outdated
Comment thread CMakeLists.txt Outdated

Copy link
Copy Markdown
Owner Author

@codex address that feedback

Please implement all six review findings on the current PR branch, with these required semantics:

  1. BRCB delivery must be association/Owner bound. MmsStaticBrcbConnection::poll must not deliver or expose a retained report to any established connection other than the live owner recorded by MmsStaticBrcbControl. A foreign association must get an explicit denied/not-owner status and must not advance or alter the delivery cursor.

  2. Make delivery two-phase. poll stages a complete frame but MUST NOT call commit_delivery. Add an explicit post-send commit API/token based on the staged EntryID. The delivery cursor advances only after the transport adapter confirms the entire frame was accepted. Update hard-profile tests so response_ready leaves the queue/cursor unchanged until explicit commit, and retry before commit returns the same EntryID.

  3. Every terminal MmsStaticServerSession path (EOF, send-closed, transport error, protocol error, receive-buffer-full, invalid configuration as applicable) must call runtime_.close_transport() before becoming terminal. Preserve exactly-once association-close behavior through the runtime's existing guard. Add an established-association EOF regression that proves the lifecycle callback fires once and the runtime closes.

  4. Suppress POSIX SIGPIPE for the static server host adapter (process-level SIGPIPE ignore is acceptable; Windows unchanged).

  5. BRCB A/B checkpoint restore must use a readable valid committed bank even if the other bank has an I/O error. Fail only when no usable committed bank remains. Add a regression with one bank unreadable and the other valid. If a final payload read fails on the selected bank, fall back to the other already-valid bank before giving up, without partially restoring twice.

  6. Remove ARIEC61850::core from the ariec61850_static_ied_server link dependencies. The portable server tool must link only ARIEC61850::mms_server_core (+ platform socket library where required), so no Sampled Values/process-bus dependency leaks into an isolated server build.

Keep -fno-exceptions -fno-rtti BRCB hard profile intact, do not import URCB/SV code, do not weaken warnings-as-errors, and add/update regression tests rather than only changing implementation.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eeebca8840

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/mms/connection_runtime.cpp
Comment thread src/mms/static_dispatcher.cpp
Comment thread src/mms/connection_runtime.cpp Outdated
Comment thread src/mms/connection_runtime.cpp Outdated
Includes current-client MMS discovery/read/control interoperability, truthful service negotiation, URCB model structure, and exact-head CI evidence.
Addresses the remaining actionable PR #51 review findings with embedded and live-client regression coverage.

Copy link
Copy Markdown
Owner Author

@codex address that feedback

Please address the remaining open review feedback on the exact current PR head. In particular, the strict src/mms/connection_runtime.cpp still maps MmsStaticDispatchStatus::object_not_found to RejectPDU, while MmsPduSpanCodec::encode_confirmed_error_into() already emits access/object-non-existent ConfirmedError. For syntactically valid GetVariableAccessAttributes / GetNamedVariableListAttributes missing-object requests, route object_not_found to ConfirmedError, preserve invoke ID, negotiated MMS/COTP limits, and association state. Add a focused regression proving the strict runtime returns ConfirmedError rather than RejectPDU. Do not change unsupported/malformed request handling.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits. You can see your limits in the Codex usage dashboard.

Copy link
Copy Markdown
Owner Author

@codex review

Please perform a fresh full review of the exact current head 0ffce108a2e2750f5ffdb861e88c458da0036b62. Focus on protocol correctness and merge-blocking P1/P2 issues, especially strict MMS ConfirmedError vs Reject semantics, Write side-effect preflight, Initiate negotiation, embedded/no-exception boundaries, BRCB ownership/delivery safety, and association lifecycle. Do not modify code as part of this review.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant