feat: S12.25 portable category axis + event-struct error boundary

[DavidCozens]

Purpose

Closes #506. Continues the error-reporting design from #433, which deferred a structured-context payload on _Error. #433 left the single code axis overloaded — it meant both which kind of failure (the portable reaction) and the exact spot (dev detail). This story splits that into three orthogonal axes carried by an error-event struct and adds a library-owned, portable category axis organised by role, so a reaction to e.g. a TLS handshake failure works identically across OpenSSL, Mbed TLS, and any integrator backend.

Change Description

New boundary:

SolidSyslog_Error(severity, source, uint16_t category, int32_t detail)
handler: void(void* context, const struct SolidSyslogErrorEvent* event)

• Source — extensible identity (unchanged from S12.22: Per-source error dispatch — SolidSyslogErrorSource + typed SolidSyslogErrorCode #433, pointer-identity match).
• Category — NEW portable reaction axis. uint16_t macros in SolidSyslogErrorCategory.h (universal lifecycle: BAD_CONFIG / BAD_ARGUMENT / POOL_EXHAUSTED / UNKNOWN_DESTROY) plus per-role SolidSyslog<Role>Categories.h beside each *Definition.h (errno-domain BASE + n; 0xC000+ reserved for integrator roles).
• Detail — the existing per-class enum SolidSyslog<Class>Errors, now int32_t (native errno/X509_V_ERR_* later), kept with a visible cast at the call site.

Key decisions (not obvious from the diff):

• Macros, not enums, for categories — spiked both. Enums forced explicit-literal values (to dodge a required-rule MISRA 10.4 on BASE + n) and dragged D.009 2.4/5.7 suppressions onto every category header. Macros restore BASE + n, embed their own (uint16_t) cast (cast-free emit sites), and add zero new MISRA suppressions because every category has a live emit site (a dead category is a cppcheck 2.5 failure — self-enforcing). The wire type stays uint16_t, never an enum, so integrators supply their own categories.
• Detail stays a per-class enum with a visible cast — category is monomorphic (library uint16_t), detail is polymorphic (enum today, native code later). Macro-ising detail would have re-opened S12.22: Per-source error dispatch — SolidSyslogErrorSource + typed SolidSyslogErrorCode #433 across 43+43 files.
• BAD_ARGUMENT added as a 4th universal category (runtime NULL args, distinct from setup-time BAD_CONFIG), which absorbed the per-Sender SEND_NULL_BUFFER category and let the Sender category header be dropped.
• AsString deliberately unchanged — text decoupling / i18n is the sibling story S12.26.

The signature break forces atomicity (cannot stay green split across PRs): 124 emit sites, 4 role category headers, ~44 test files, and the BDD/FreeRtos handlers all land here.

Test Evidence

• New shared CHECK_ERROR_EVENT(src, cat, detail) in TestUtils.h; ErrorHandlerFake captures the event (_LastCategory / _LastDetail; _LastCode retired). Every error assertion across ~44 test files now checks the portable category alongside the detail — 3292 → 3406 checks.
• The dispatcher's event-assembly is driven red→green in SolidSyslogErrorTest.cpp; the rest is mechanical refactor under green.
• gcc debug and freertos-host debug both green (1392 tests, 0 failures, plus the embedded test binaries).
• cppcheck-misra clean — zero new findings on the category headers; suppressions realigned with scripts/misra_renumber.py. clang-format --dry-run --Werror clean across the tree.

Areas Affected

• Core/Interface/SolidSyslogError.h (event struct, handler typedef, _Error signature — pre-1.0 break), Core/Interface/SolidSyslogErrorCategory.h (NEW) + 4 role category headers (NEW), Core/Source/SolidSyslogError.c (event assembly).
• ~50 emit-site sources across Core/Source/ + Platform/*/Source/; the crypto-policy _Report wrappers gained a category parameter.
• Bdd/Targets/Common/BddTargetStderrErrorHandler.c, Bdd/Targets/FreeRtos/main.c, Bdd/Targets/FreeRtosLwip/main.c (handler signature).
• ~44 test files, Tests/Support/{TestUtils.h, ErrorHandlerFake.*}.
• CLAUDE.md public-headers table, DEVLOG.md, misra_suppressions.txt.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• Refactor
  • Restructured error event handling to use categorized error reporting with separate category and detail fields, enabling improved error diagnostics and classification across platform-specific implementations.

[coderabbitai]

Warning

Review limit reached

@DavidCozens, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 32 minutes and 13 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: da87efe3-53c4-4aba-909f-c05ec9ccbd89

📥 Commits

Reviewing files that changed from the base of the PR and between 60ce19a and 7774f09.

📒 Files selected for processing (6)

• Core/Interface/SolidSyslogErrorCategory.h
• Platform/MbedTls/Source/SolidSyslogMbedTlsHmacSha256Policy.c
• Platform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256Policy.c
• Tests/MbedTls/SolidSyslogMbedTlsHmacSha256PolicyTest.cpp
• Tests/SolidSyslogOpenSslHmacSha256PolicyTest.cpp
• misra_suppressions.txt

📝 Walkthrough

Walkthrough

Introduces struct-based error events and a portable category axis. Updates public headers, core dispatcher, all emit sites across platforms, BDD targets, tests, docs, and MISRA suppressions to use (category, detail) and the new handler signature.

Changes

S12.25 error event boundary and categories

Layer / File(s)	Summary
API and category contracts Core/Interface/SolidSyslogError*.h, Core/Interface/*Categories.h, Core/Source/SolidSyslogError.c, CLAUDE.md, DEVLOG.md	Adds SolidSyslogErrorEvent, new handler typedef, SolidSyslog_Error(severity, source, category, detail), and portable category headers; documents the model.
Core emit sites and wiring Core/Source/*.c	All SolidSyslog_Error calls updated to include SOLIDSYSLOG_CAT_* and int32_t detail; includes adjusted.
Platform adapters sweep Platform/**/*	All platform modules include category headers and emit category+detail; some include reorders.
TLS/security policy adapters Platform/*Tls*, Platform/*AesGcm*/HmacSha256*	Report helpers gain category parameter; callers pass role-specific categories.
BDD targets/handlers Bdd/Targets/**/*	Example handlers updated to consume event and print category/detail.
Tests and fakes Tests/**/*, Tests/Support/*	ErrorHandlerFake now records category/detail; macros and assertions updated to validate category+detail across suites.
Compliance misra_suppressions.txt	MISRA suppression line numbers and entries updated for new signatures and casts.

Sequence Diagram(s)

sequenceDiagram
  participant Caller as Module emit site
  participant Dispatcher as SolidSyslog_Error
  participant Handler as Installed error handler
  Caller->>Dispatcher: SolidSyslog_Error(sev, src, cat, detail)
  Dispatcher->>Dispatcher: Assemble SolidSyslogErrorEvent
  Dispatcher->>Handler: handler(ctx, &event)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

• S12.26: Error text decoupling — codes-only + i18n #507: Builds on S12.25; S12.26 depends on the event-struct boundary and category axis introduced here.

Possibly related PRs

• DavidCozens/solid-syslog#319: Earlier work on the SolidSyslog error-handler API that this PR evolves to an event payload.
• DavidCozens/solid-syslog#509: Touches the same OpenSSL AES-GCM reporting paths now refactored to pass categories.
• DavidCozens/solid-syslog#442: Modifies TLS stream error flows/tests that this PR’s category/detail plumbing now exercises.

Poem

A rabbit taps logs with delicate paws,
Categories tidy, obeying new laws;
Events hop along to handlers that care,
Details in whiskers, severity to share.
From burrow to buffer, the trail is neat—
Portable carrots make debugging sweet! 🥕🐇

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/s12-25-error-event-category

[github-actions]

☀️   Quality Summary

   🚦   build-linux-gcc: 100% successful (✔️ 1417 passed)
   🚦   build-freertos-host-tdd-plustcp: 100% successful (✔️ 1732 passed)
   🚦   build-linux-clang: 100% successful (✔️ 1363 passed)
   🚦   sanitize-linux-gcc: 100% successful (✔️ 1363 passed)
   🚦   integration-linux-openssl: 100% successful (✔️ 16 passed)
   🚦   integration-linux-mbedtls: 100% successful (✔️ 14 passed)
   🚦   integration-windows-openssl: 100% successful (✔️ 16 passed)
   🚦   bdd-linux-syslog-ng: 94% successful (✔️ 48 passed, 🙈 3 skipped)
   🚦   bdd-windows-otel: 88% successful (✔️ 45 passed, 🙈 6 skipped)
   🚦   bdd-freertos-qemu-plustcp: 86% successful (✔️ 44 passed, 🙈 7 skipped)
   🚦   bdd-freertos-qemu-lwip: 63% successful (✔️ 32 passed, 🙈 19 skipped)
   🚦   build-windows-msvc: 100% successful (✔️ 1216 passed)
   🚦   build-linux-tunable-override: 100% successful (✔️ 1363 passed)
   ⚠️   Clang-Tidy: 5 warnings (high: 5)
   ⚠️   CPPCheck: No warnings

---

Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result.

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

Tests/SolidSyslogTlsStreamPoolTest.cpp (1)

100-106: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Drop the fallback handle before teardown in this test.

overflow still points at the fallback stream here, so teardown destroys it and intentionally reports UNKNOWN_DESTROY through ErrorHandlerFake. The result is two callbacks for the test even though Line 102 asserts ONCE.

Suggested fix

     overflow = SolidSyslogTlsStream_Create(&config);

     CALLED_FAKE(ErrorHandlerFake_Handle, ONCE);
     LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_ERROR, ErrorHandlerFake_LastSeverity());
     POINTERS_EQUAL(&TlsStreamErrorSource, ErrorHandlerFake_LastSource());
     UNSIGNED_LONGS_EQUAL(SOLIDSYSLOG_CAT_POOL_EXHAUSTED, ErrorHandlerFake_LastCategory());
     UNSIGNED_LONGS_EQUAL(TLSSTREAM_ERROR_POOL_EXHAUSTED, ErrorHandlerFake_LastDetail());
+    overflow = nullptr;

Based on learnings: pool-backed _Destroy intentionally warns on fallback handles, so tests that assert only the create-time error need to keep teardown away from that fallback.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Tests/SolidSyslogTlsStreamPoolTest.cpp` around lines 100 - 106, The test
currently leaves the fallback handle in overflow (returned by
SolidSyslogTlsStream_Create) so teardown later destroys it and triggers a second
ErrorHandlerFake_Handle callback; before test teardown, explicitly drop/close
the fallback handle stored in overflow (e.g., call the pool/fallback
release/close function or set overflow to NULL and ensure any created handle is
destroyed) so only the create-time error is reported and the
ErrorHandlerFake_Handle assertion (ONCE) remains valid; locate usage of overflow
and SolidSyslogTlsStream_Create in this test and release that handle before test
teardown to avoid the pool-backed _Destroy warning callback.

Tests/SolidSyslogStreamSenderTest.cpp (1)

756-762: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Teardown is hitting the wrong destroy path for this bad-setup group.

Here sender is either still nullptr or the shared fallback returned from a bad setup, and both cases route SolidSyslogStreamSender_Destroy(...) into the intentional UNKNOWN_DESTROY warning path. That means the CALLED_FAKE(ErrorHandlerFake_Handle, ONCE) assertions in this group are only true before teardown runs.

Use a separate “owns pooled sender” path here, or keep the bad-setup sender local to the tests and only destroy a real pooled sender from teardown.

Based on learnings: calling SolidSyslog*_Destroy(nullptr) is not a safe no-op here, and pool-backed _Destroy also intentionally warns on fallback handles.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Tests/SolidSyslogStreamSenderTest.cpp` around lines 756 - 762, The teardown
currently calls SolidSyslogStreamSender_Destroy(sender) which routes nullptr or
the shared fallback into the UNKNOWN_DESTROY warning path and invalidates the
CALLED_FAKE(ErrorHandlerFake_Handle, ONCE) assertions; change teardown to only
destroy pooled/owned senders (e.g. track and call
SolidSyslogStreamSender_Destroy on an owned_pooled_sender variable) and do not
call SolidSyslogStreamSender_Destroy on the test-local bad-setup sender (keep
that bad sender as a local variable inside those tests so teardown won't touch
it), or add an explicit owns_pooled_sender flag checked in teardown before
calling SolidSyslogStreamSender_Destroy to avoid triggering the fallback warning
path.

Tests/SolidSyslogGetAddrInfoResolverTest.cpp (1)

189-195: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clear overflow after the create-error assertion.

This test leaves the fallback resolver in overflow, so teardown destroys it and, by design, emits UNKNOWN_DESTROY through the still-installed fake. The test therefore produces two handler callbacks by the time it finishes, even though Line 191 asserts ONCE.

Suggested fix

     overflow = SolidSyslogGetAddrInfoResolver_Create();

     CALLED_FAKE(ErrorHandlerFake_Handle, ONCE);
     LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_ERROR, ErrorHandlerFake_LastSeverity());
     POINTERS_EQUAL(&GetAddrInfoResolverErrorSource, ErrorHandlerFake_LastSource());
     UNSIGNED_LONGS_EQUAL(SOLIDSYSLOG_CAT_POOL_EXHAUSTED, ErrorHandlerFake_LastCategory());
     UNSIGNED_LONGS_EQUAL(GETADDRINFORESOLVER_ERROR_POOL_EXHAUSTED, ErrorHandlerFake_LastDetail());
+    overflow = nullptr;

Based on learnings: every pool-backed _Destroy intentionally warns on fallback handles, so tests must avoid letting teardown destroy the fallback when they asserted only the create-time error.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Tests/SolidSyslogGetAddrInfoResolverTest.cpp` around lines 189 - 195, The
test leaves the fallback resolver in the local variable overflow after calling
SolidSyslogGetAddrInfoResolver_Create and asserting the create-time error, which
causes teardown to destroy that fallback and trigger a second ErrorHandler
callback; after the assertions that check ErrorHandlerFake_Handle (ONCE) and
related details, set overflow to NULL (or otherwise clear/reset the fallback
handle) so teardown won't attempt to destroy it—update the test around
SolidSyslogGetAddrInfoResolver_Create/overflow to clear overflow immediately
after the error assertions.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Bdd/Targets/Common/BddTargetStderrErrorHandler.c`:
- Around line 19-22: The current cast of event->Detail to uint8_t when calling
source->AsString can mis-map int32_t values; update the logic in
BddTargetStderrErrorHandler.c so you only call
source->AsString((uint8_t)event->Detail) when source->AsString is non-NULL and
event->Detail is within the old uint8_t range (0..255), otherwise leave the
default message unchanged; reference the symbols source->AsString and
event->Detail and perform an explicit range check before the cast to avoid
truncation to legacy values.

In `@Platform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256Policy.c`:
- Around line 110-114: The ComputeTag helper always reports failures with
SOLIDSYSLOG_CAT_SECURITYPOLICY_SEAL_FAILED which is incorrect for open-time HMAC
errors; update OpenSslHmacSha256Policy_ComputeTag to accept an
operation-specific failure category (or a boolean indicating seal vs open) and
use that value when calling OpenSslHmacSha256Policy_Report instead of the
hard-coded SOLIDSYSLOG_CAT_SECURITYPOLICY_SEAL_FAILED so callers (seal and open)
can pass the correct category for their failure reports.

---

Outside diff comments:
In `@Tests/SolidSyslogGetAddrInfoResolverTest.cpp`:
- Around line 189-195: The test leaves the fallback resolver in the local
variable overflow after calling SolidSyslogGetAddrInfoResolver_Create and
asserting the create-time error, which causes teardown to destroy that fallback
and trigger a second ErrorHandler callback; after the assertions that check
ErrorHandlerFake_Handle (ONCE) and related details, set overflow to NULL (or
otherwise clear/reset the fallback handle) so teardown won't attempt to destroy
it—update the test around SolidSyslogGetAddrInfoResolver_Create/overflow to
clear overflow immediately after the error assertions.

In `@Tests/SolidSyslogStreamSenderTest.cpp`:
- Around line 756-762: The teardown currently calls
SolidSyslogStreamSender_Destroy(sender) which routes nullptr or the shared
fallback into the UNKNOWN_DESTROY warning path and invalidates the
CALLED_FAKE(ErrorHandlerFake_Handle, ONCE) assertions; change teardown to only
destroy pooled/owned senders (e.g. track and call
SolidSyslogStreamSender_Destroy on an owned_pooled_sender variable) and do not
call SolidSyslogStreamSender_Destroy on the test-local bad-setup sender (keep
that bad sender as a local variable inside those tests so teardown won't touch
it), or add an explicit owns_pooled_sender flag checked in teardown before
calling SolidSyslogStreamSender_Destroy to avoid triggering the fallback warning
path.

In `@Tests/SolidSyslogTlsStreamPoolTest.cpp`:
- Around line 100-106: The test currently leaves the fallback handle in overflow
(returned by SolidSyslogTlsStream_Create) so teardown later destroys it and
triggers a second ErrorHandlerFake_Handle callback; before test teardown,
explicitly drop/close the fallback handle stored in overflow (e.g., call the
pool/fallback release/close function or set overflow to NULL and ensure any
created handle is destroyed) so only the create-time error is reported and the
ErrorHandlerFake_Handle assertion (ONCE) remains valid; locate usage of overflow
and SolidSyslogTlsStream_Create in this test and release that handle before test
teardown to avoid the pool-backed _Destroy warning callback.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cee7abd9-3261-4b5d-8685-0ef60d31bd16

📥 Commits

Reviewing files that changed from the base of the PR and between c48a27f and 60ce19a.

📒 Files selected for processing (121)

• Bdd/Targets/Common/BddTargetStderrErrorHandler.c
• Bdd/Targets/FreeRtos/main.c
• Bdd/Targets/FreeRtosLwip/main.c
• CLAUDE.md
• Core/Interface/SolidSyslogBufferCategories.h
• Core/Interface/SolidSyslogError.h
• Core/Interface/SolidSyslogErrorCategory.h
• Core/Interface/SolidSyslogResolverCategories.h
• Core/Interface/SolidSyslogSecurityPolicyCategories.h
• Core/Interface/SolidSyslogTlsStreamCategories.h
• Core/Source/SolidSyslog.c
• Core/Source/SolidSyslogBlockStoreStatic.c
• Core/Source/SolidSyslogCircularBufferStatic.c
• Core/Source/SolidSyslogError.c
• Core/Source/SolidSyslogFileBlockDeviceStatic.c
• Core/Source/SolidSyslogMetaSdStatic.c
• Core/Source/SolidSyslogOriginSdStatic.c
• Core/Source/SolidSyslogPassthroughBufferStatic.c
• Core/Source/SolidSyslogStatic.c
• Core/Source/SolidSyslogStreamSenderStatic.c
• Core/Source/SolidSyslogSwitchingSenderStatic.c
• Core/Source/SolidSyslogTimeQualitySdStatic.c
• Core/Source/SolidSyslogUdpSender.c
• Core/Source/SolidSyslogUdpSenderStatic.c
• DEVLOG.md
• Platform/Atomics/Source/SolidSyslogStdAtomicCounterStatic.c
• Platform/FatFs/Source/SolidSyslogFatFsFileStatic.c
• Platform/FreeRtos/Source/SolidSyslogFreeRtosMutexStatic.c
• Platform/LwipRaw/Source/SolidSyslogLwipRawAddressStatic.c
• Platform/LwipRaw/Source/SolidSyslogLwipRawDatagramStatic.c
• Platform/LwipRaw/Source/SolidSyslogLwipRawDnsResolver.c
• Platform/LwipRaw/Source/SolidSyslogLwipRawDnsResolverStatic.c
• Platform/LwipRaw/Source/SolidSyslogLwipRawResolverStatic.c
• Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStreamStatic.c
• Platform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicy.c
• Platform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicyMessages.c
• Platform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicyPrivate.h
• Platform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicyStatic.c
• Platform/MbedTls/Source/SolidSyslogMbedTlsHmacSha256Policy.c
• Platform/MbedTls/Source/SolidSyslogMbedTlsHmacSha256PolicyMessages.c
• Platform/MbedTls/Source/SolidSyslogMbedTlsHmacSha256PolicyPrivate.h
• Platform/MbedTls/Source/SolidSyslogMbedTlsHmacSha256PolicyStatic.c
• Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c
• Platform/MbedTls/Source/SolidSyslogMbedTlsStreamStatic.c
• Platform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicy.c
• Platform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicyMessages.c
• Platform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicyPrivate.h
• Platform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicyStatic.c
• Platform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256Policy.c
• Platform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256PolicyMessages.c
• Platform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256PolicyPrivate.h
• Platform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256PolicyStatic.c
• Platform/OpenSsl/Source/SolidSyslogTlsStream.c
• Platform/OpenSsl/Source/SolidSyslogTlsStreamStatic.c
• Platform/PlusTcp/Source/SolidSyslogPlusTcpAddressStatic.c
• Platform/PlusTcp/Source/SolidSyslogPlusTcpDatagramStatic.c
• Platform/PlusTcp/Source/SolidSyslogPlusTcpResolverStatic.c
• Platform/PlusTcp/Source/SolidSyslogPlusTcpTcpStreamStatic.c
• Platform/Posix/Source/SolidSyslogGetAddrInfoResolverStatic.c
• Platform/Posix/Source/SolidSyslogPosixAddressStatic.c
• Platform/Posix/Source/SolidSyslogPosixDatagramStatic.c
• Platform/Posix/Source/SolidSyslogPosixFileStatic.c
• Platform/Posix/Source/SolidSyslogPosixMessageQueueBuffer.c
• Platform/Posix/Source/SolidSyslogPosixMessageQueueBufferStatic.c
• Platform/Posix/Source/SolidSyslogPosixMutexStatic.c
• Platform/Posix/Source/SolidSyslogPosixTcpStreamStatic.c
• Platform/Windows/Source/SolidSyslogWindowsAtomicCounterStatic.c
• Platform/Windows/Source/SolidSyslogWindowsFileStatic.c
• Platform/Windows/Source/SolidSyslogWindowsMutexStatic.c
• Platform/Windows/Source/SolidSyslogWinsockAddressStatic.c
• Platform/Windows/Source/SolidSyslogWinsockDatagramStatic.c
• Platform/Windows/Source/SolidSyslogWinsockResolverStatic.c
• Platform/Windows/Source/SolidSyslogWinsockTcpStreamStatic.c
• Tests/FatFs/SolidSyslogFatFsFilePoolTest.cpp
• Tests/FreeRtos/SolidSyslogFreeRtosMutexTest.cpp
• Tests/FreeRtos/SolidSyslogPlusTcpAddressTest.cpp
• Tests/FreeRtos/SolidSyslogPlusTcpDatagramTest.cpp
• Tests/FreeRtos/SolidSyslogPlusTcpResolverTest.cpp
• Tests/FreeRtos/SolidSyslogPlusTcpTcpStreamTest.cpp
• Tests/Lwip/SolidSyslogLwipRawAddressTest.cpp
• Tests/Lwip/SolidSyslogLwipRawDatagramTest.cpp
• Tests/Lwip/SolidSyslogLwipRawDnsResolverTest.cpp
• Tests/Lwip/SolidSyslogLwipRawResolverTest.cpp
• Tests/Lwip/SolidSyslogLwipRawTcpStreamTest.cpp
• Tests/MbedTls/SolidSyslogMbedTlsAesGcmPolicyTest.cpp
• Tests/MbedTls/SolidSyslogMbedTlsHmacSha256PolicyTest.cpp
• Tests/MbedTls/SolidSyslogMbedTlsStreamPoolTest.cpp
• Tests/MbedTls/SolidSyslogMbedTlsStreamTest.cpp
• Tests/SolidSyslogCircularBufferTest.cpp
• Tests/SolidSyslogErrorTest.cpp
• Tests/SolidSyslogGetAddrInfoResolverTest.cpp
• Tests/SolidSyslogMetaSdTest.cpp
• Tests/SolidSyslogOpenSslAesGcmPolicyTest.cpp
• Tests/SolidSyslogOpenSslHmacSha256PolicyTest.cpp
• Tests/SolidSyslogPassthroughBufferTest.cpp
• Tests/SolidSyslogPoolTest.cpp
• Tests/SolidSyslogPosixAddressTest.cpp
• Tests/SolidSyslogPosixDatagramTest.cpp
• Tests/SolidSyslogPosixFileTest.cpp
• Tests/SolidSyslogPosixMessageQueueBufferTest.cpp
• Tests/SolidSyslogPosixMutexTest.cpp
• Tests/SolidSyslogPosixTcpStreamTest.cpp
• Tests/SolidSyslogStdAtomicCounterPoolTest.cpp
• Tests/SolidSyslogStreamSenderTest.cpp
• Tests/SolidSyslogSwitchingSenderTest.cpp
• Tests/SolidSyslogTest.cpp
• Tests/SolidSyslogTimeQualitySdTest.cpp
• Tests/SolidSyslogTlsStreamPoolTest.cpp
• Tests/SolidSyslogTlsStreamTest.cpp
• Tests/SolidSyslogUdpSenderTest.cpp
• Tests/SolidSyslogWindowsAtomicCounterPoolTest.cpp
• Tests/SolidSyslogWindowsFileTest.cpp
• Tests/SolidSyslogWindowsMutexTest.cpp
• Tests/SolidSyslogWinsockAddressTest.cpp
• Tests/SolidSyslogWinsockDatagramTest.cpp
• Tests/SolidSyslogWinsockResolverTest.cpp
• Tests/SolidSyslogWinsockTcpStreamTest.cpp
• Tests/Support/ErrorHandlerFake.c
• Tests/Support/ErrorHandlerFake.h
• Tests/Support/TestUtils.h
• misra_suppressions.txt

[DavidCozens]

@coderabbitai — dispositions for the three "outside diff range" teardown findings (SolidSyslogTlsStreamPoolTest.cpp, SolidSyslogStreamSenderTest.cpp, SolidSyslogGetAddrInfoResolverTest.cpp):

These are pre-existing patterns — present before this PR (verified against the merge base); this PR only added a category-assertion line in the same test bodies. The tests pass: the CALLED_FAKE(ErrorHandlerFake_Handle, ONCE) assertion runs in the test body, before teardown. The second UNKNOWN_DESTROY callback that teardown fires when it destroys the pool-exhaustion fallback handle is never re-asserted, and each test's setup() re-installs (and resets) ErrorHandlerFake, so there is no cross-test contamination.

Agreed it's latent test hygiene (a test that asserts "exactly one error" technically emits a second during teardown), but it's out of scope for this error-event refactor and not a regression. Leaving as-is here; happy to clean it up in a dedicated test-hygiene pass if wanted.

Summary of this review round:

• HMAC open-path mis-categorisation (real bug) — fixed in 7774f09, with new tests.
• BDD (uint8_t) detail cast — correct for the current contract; native-code handling defers to S12.26 (replied inline).
• Three teardown findings — pre-existing, passing, out of scope (above).
• clang-tidy macro-to-enum (CI) — fixed in cc66b58 by casting the category base macros.

[coderabbitai]

@DavidCozens — thanks for the thorough disposition. The explanation is sound:

• Teardown findings — agreed, the CALLED_FAKE(ErrorHandlerFake_Handle, ONCE) assertion fires in the test body before teardown's second UNKNOWN_DESTROY callback, and setup() resets ErrorHandlerFake between tests, so isolation is intact. Pre-existing, passing, no regression — deferring to a hygiene pass is the right call.
• HMAC open-path mis-categorisation — ✅ fixed in 7774f09 with new tests.
• BDD (uint8_t) detail cast — ✅ correct per current contract; S12.26 is the right home.
• clang-tidy macro-to-enum — ✅ fixed in cc66b58.

Recording the teardown pattern for future reviews:

---

✏️ Learnings added

Learnt from: DavidCozens
URL: https://github.com/DavidCozens/solid-syslog/pull/513

Timestamp: 2026-06-02T22:57:56.392Z
Learning: In `DavidCozens/solid-syslog`, the pool test files `Tests/SolidSyslogTlsStreamPoolTest.cpp`, `Tests/SolidSyslogStreamSenderTest.cpp`, and `Tests/SolidSyslogGetAddrInfoResolverTest.cpp` have a pre-existing teardown pattern where pool-exhaustion tests assert `CALLED_FAKE(ErrorHandlerFake_Handle, ONCE)` in the test body, but teardown subsequently fires a second `UNKNOWN_DESTROY` callback (when it destroys the fallback handle) that is never re-asserted. This is intentional/pre-existing latent test hygiene — the tests pass because the `ONCE` assertion runs before teardown, and `setup()` re-installs and resets `ErrorHandlerFake` between tests so there is no cross-test contamination. Do not flag this as a defect in future reviews; it is tracked for a dedicated test-hygiene pass.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

🧠 Learnings used

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-04-21T06:23:37.923Z
Learning: In `DavidCozens/solid-syslog`, the inconsistent null-check guard pattern in `Tests/SolidSyslogTlsStreamTest.cpp` (lines 161–187) is intentionally deferred to issue `#31` (E12: Error Handling), which covers a project-wide null-guard consistency sweep. Do not flag this pattern inconsistency in future reviews of `SolidSyslogTlsStreamTest.cpp`.

---

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 183
File: Tests/OpenSslIntegration/TlsTestServer.c:17-35
Timestamp: 2026-04-22T05:25:31.162Z
Learning: In `DavidCozens/solid-syslog`, test-harness files `Tests/OpenSslIntegration/TlsTestServer.c`, `Tests/OpenSslIntegration/BioPairStream.c`, and `Tests/OpenSslIntegration/TlsTestCert.c` intentionally omit return-value checks and NULL guards on `calloc`, `SSL_CTX_new`, `SSL_new`, `BIO_new_bio_pair`, `SSL_CTX_use_certificate`, `SSL_CTX_use_PrivateKey`, `SSL_CTX_set_cipher_list`, and similar OpenSSL calls inside their `Create` functions. These run in a controlled CI container where allocations are deterministic; adding defensive failure branches would introduce untested paths with no realistic failure mode, violating the project's "no untested production code" discipline. Scope consistency across the harness is the explicit design goal. Do not flag missing NULL or return-value checks in these three files in future reviews.

---

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-04-21T06:23:37.923Z
Learning: In `DavidCozens/solid-syslog`, `BIO_METHOD` lifecycle (`BIO_meth_free`), `SSL` pointer nulling in `SolidSyslogTlsStream_Close`, and full `Destroy` resource cleanup in `Platform/OpenSsl/Source/SolidSyslogTlsStream.c` are intentionally deferred to issue `#172` (S3.8 hardening), which also covers mid-Open resource leak tightening. Do not flag these as missing in future reviews of `SolidSyslogTlsStream.c`.

---

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 502
File: DEVLOG.md:0-0
Timestamp: 2026-06-01T10:52:21.000Z
Learning: For PR `#502` / S12.21 in `solid-syslog`, Structured Data and MSG-body tests intentionally remain in `Tests/SolidSyslogTest.cpp` as integration coverage of the full `SolidSyslog_Log` pipeline rather than being migrated to `Tests/SolidSyslogMessageFormatterTest.cpp`.

---

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-04-18T07:19:13.167Z
Learning: In `DavidCozens/solid-syslog`, the `TEST_*` constant convention (e.g., `TestHost`, `42`, RFC 5424 publication date) documented in `CLAUDE.md` under "test-defaults-pattern" applies exclusively to *production-code* "test default" values baked into walking-skeleton implementations. It does NOT apply to input/output literals used inline in CppUTest test cases (e.g., path strings in `Tests/Example/ExampleAppNameTest.cpp`). For one-line CppUTest cases, inline literals are preferred because they show "this exact input → that exact output" without an extra layer of indirection. Do not suggest extracting CppUTest test-case input/output literals into `TEST_*` constants in future reviews.

---

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 170
File: Platform/OpenSsl/Source/SolidSyslogTlsStream.c:58-62
Timestamp: 2026-04-21T06:23:13.024Z
Learning: In `Platform/OpenSsl/Source/SolidSyslogTlsStream.c` of `DavidCozens/solid-syslog`, the return values of `SSL_set_tlsext_host_name`, `SSL_set1_host`, and all other OpenSSL calls inside the `Open` function are intentionally unchecked in S3.7 (PR `#170`, walking-skeleton). Every OpenSSL return-value check, SNI/hostname-setup failure blocking the handshake, mid-Open resource leaks (SSL/CTX/transport cleanup on failure paths), and `BIO_METHOD` lifecycle hardening are deferred to S3.8 (issue `#172`), part of Epic E3 (issue `#5`). Do not flag missing return-value checks or resource-leak paths in `SolidSyslogTlsStream_Open` (or related static helpers) in future reviews until S3.8 is merged.

---

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-04-21T06:23:37.923Z
Learning: In `DavidCozens/solid-syslog`, the shared `lastSetDataArg` field in `Tests/Support/OpenSslFake.c` (used by both `BIO_set_data` and `BIO_get_data` tracking) is intentionally left shared for now and is tracked for per-BIO state expansion in issue `#172` (S3.8). Do not flag this shared state as a defect in future reviews of `OpenSslFake.c`.

---

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 205
File: Core/Source/SolidSyslogFormatter.c:248-274
Timestamp: 2026-04-24T19:21:31.563Z
Learning: In `Core/Source/SolidSyslogFormatter.c` (`DavidCozens/solid-syslog`), `SolidSyslogFormatter_EscapedString` has a known deferred defect: when the output buffer has exactly 1 byte of remaining capacity and the current source byte needs escaping (`"`, `\`, `]`), `ESCAPE_PREFIX` is written but the escaped character is silently clamped, leaving a dangling `\` at the buffer tail. `AsFormattedBuffer`'s `TrimTruncatedMultiByteTail` does not cover this case because `\` is valid ASCII. The practical impact is zero for all in-tree callers because they size via `SOLIDSYSLOG_ESCAPED_MAX_SIZE(rawMax)` = `2 * rawMax`, which always provides sufficient headroom. The agreed fix (Option 1) is a targeted output-capacity check before the escape+codepoint pair write (add `OutputHasCapacityFor(formatter, outputBytes)` guard; break on failure). Do not re-flag this as a new issue in future reviews until the targeted fix is merged.

---

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-04-17T19:45:13.677Z
Learning: In `DavidCozens/solid-syslog`, the test helper methods `Resolve()` and `Result()` in `Tests/SolidSyslogWinsockResolverTest.cpp` intentionally use PascalCase to mirror the convention in the POSIX counterpart `Tests/SolidSyslogGetAddrInfoResolverTest.cpp`. Do not flag PascalCase test helper methods in Winsock or POSIX resolver tests in future reviews.

---

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 140
File: Source/SolidSyslogPosixTcpStream.c:31-36
Timestamp: 2026-04-17T07:50:54.370Z
Learning: In `Source/SolidSyslogPosixTcpStream.c`, `SolidSyslogPosixTcpStream_Destroy()` intentionally does not close the socket fd before nulling the vtable pointers. Through the only supported API path (TcpSender → Stream), every successful `Open` is guaranteed to be matched by a `Close` — either via `TcpSender_Destroy` (when `connected == true`), `SendData` failure recovery, or `Open`'s own connect-failure self-recovery (`OpenClosesSocketOnConnectFailure` test). Standalone "Destroy-without-Close" usage is not a supported scenario; adding defensive cleanup without a driving test would violate the project's "no untested production code" discipline. Formalising this contract (with tests) is deferred to Epic `#31` (error-handling / robustness). Do not flag missing fd cleanup in `SolidSyslogPosixTcpStream_Destroy` in future reviews.

---

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 393
File: Tests/SolidSyslogConfigLockTest.cpp:25-34
Timestamp: 2026-05-17T15:08:00.142Z
Learning: In the solid-syslog codebase, global-slot test groups (e.g. config-lock, error-handler) use teardown()-only cleanup — not setup() reset — as the established pattern (see Tests/SolidSyslogErrorTest.cpp as precedent). CppUTest guarantees teardown() runs after every test regardless of pass/fail, so intra-group isolation is already guaranteed. Do not suggest adding defensive ConfigLockFake_Uninstall() or equivalent calls in setup() as belt-and-braces; the right fix for any future cross-group leakage is to fix the offending test file's teardown.

---

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 327
File: Core/Source/SolidSyslog.c:220-224
Timestamp: 2026-05-11T10:23:17.520Z
Learning: In DavidCozens/solid-syslog, `InstallStructuredData` in `Core/Source/SolidSyslog.c` intentionally assigns `instance.sd = configured` and `instance.sdCount = count` without a NULL guard. The correct follow-on fix for the `sd == NULL && sdCount > 0` inconsistency is to report a new `SOLIDSYSLOG_ERROR_MSG_CREATE_INCONSISTENT_SD` error rather than silently skipping the assignment. This is tracked under E12 alongside S12.06 (NULL guards for SD implementations). Do not flag the missing NULL guard in `InstallStructuredData` as a defect in the nil-object defaults PR (S12.04); it requires its own TDD red and error-message constant.

---

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 394
File: misra_suppressions.txt:27-27
Timestamp: 2026-05-18T05:43:07.289Z
Learning: In the solid-syslog codebase (`misra_suppressions.txt` and `docs/misra-deviations.md`), D.002 (Rules 11.2/11.3/11.5) covers two distinct cast patterns: (1) the vtable downcast (`struct SolidSyslogBuffer*` → `struct SolidSyslogCircularBuffer*` via `CircularBuffer_SelfFromBase`, and analogous `SelfFromBase` helpers in every other class) which is retained for all classes including those migrated under E11; and (2) the caller-storage `void*` cast which E11-migrated classes (currently only `SolidSyslogCircularBuffer`) no longer use. Do not flag the `SelfFromBase` vtable downcast as uncovered by D.002 — it is explicitly documented in `docs/misra-deviations.md` (updated in commit 9bd1d39 of PR `#394`). A potential future refactor to split D.002 into two separate blocks for the whole library was noted as out of scope for S11.01.

---

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 422
File: Core/Source/SolidSyslogStreamSender.c:55-55
Timestamp: 2026-05-22T12:17:11.035Z
Learning: In DavidCozens/solid-syslog, `SolidSyslogStreamSender_Create` has never validated NULL config fields (Resolver, Stream, Address, Endpoint). Full create-time bad-setup contract validation (reject NULL Resolver/Stream/Address/Endpoint, with matching tests mirroring `SolidSyslogUdpSenderBadSetup`) is tracked in issue `#424` (S24.09), linked as a sub-issue of E24 (`#254`). Do not flag the missing NULL checks in StreamSender_Create as an issue in future PRs until `#424` is addressed.

---

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 422
File: Core/Source/SolidSyslogStreamSender.c:55-55
Timestamp: 2026-05-22T12:17:11.035Z
Learning: In DavidCozens/solid-syslog, `SolidSyslogStreamSender_Create` has never validated NULL config fields (Resolver, Stream, Address, Endpoint). Full create-time bad-setup contract validation (reject NULL Resolver/Stream/Address/Endpoint, with matching tests mirroring `SolidSyslogUdpSenderBadSetup`) is tracked in issue `#424` (S24.09), linked as a sub-issue of E24 (`#254`). Do not flag missing NULL checks in `StreamSender_Create` as an issue in future PRs until `#424` is addressed.

---

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-05-13T22:05:44.151Z
Learning: In this codebase (solid-syslog), CLAUDE.md explicitly forbids adding error handling, fallbacks, or validation for scenarios that "can't happen". Internal vtable wrappers (e.g. in `Core/Source/SolidSyslogStore.c`) dereference function pointers unconditionally — a NULL vtable method is a wiring bug that should surface as an immediate crash, not be silently defaulted. Validation is only added at system boundaries (user input, external APIs).

---

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 0
File: :0-0
Timestamp: 2026-04-28T22:11:55.250Z
Learning: In `DavidCozens/solid-syslog`, Epic E07 (Standard Structured Data) tracks missing parameters on standard SDs (e.g., §6.3.4 origin `ip`/`enterpriseId` → issue `#75` S07.05; §6.3.5 meta `sysUpTime`/`language` → issue `#193` S07.06). Epic E14 (`#64`) is scoped exclusively to caller-built custom SD validation (e.g., SD-NAME syntax enforcement). Do not suggest filing standard SD parameter gaps under E14 in future reviews.

---

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 352
File: Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp:175-182
Timestamp: 2026-05-13T22:05:29.329Z
Learning: In the `solid-syslog` project, the CLAUDE.md guideline explicitly discourages adding defensive error handling or validation for scenarios that cannot happen given the existing constraints. For example, test fixture helpers like `WriteMessage` in `Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp` intentionally omit payload-size guards because `payloadSize` is always derived from `SOLIDSYSLOG_MAX_MESSAGE_SIZE - 100` (>= 412) or hardcoded values (e.g., 64), making the < 4 case unreachable. Do not suggest adding such guards in this codebase.

✅ Action performed

Comments resolved. Approval is disabled; enable reviews.request_changes_workflow to allow explicit top-level @coderabbitai resolve or @coderabbitai approve commands.

[github-actions]

☀️   Quality Summary

   🚦   build-linux-gcc: 100% successful (✔️ 1418 passed)
   🚦   build-freertos-host-tdd-plustcp: 100% successful (✔️ 1733 passed)
   🚦   build-linux-clang: 100% successful (✔️ 1364 passed)
   🚦   sanitize-linux-gcc: 100% successful (✔️ 1364 passed)
   🚦   integration-linux-openssl: 100% successful (✔️ 16 passed)
   🚦   integration-linux-mbedtls: 100% successful (✔️ 14 passed)
   🚦   integration-windows-openssl: 100% successful (✔️ 16 passed)
   🚦   bdd-linux-syslog-ng: 94% successful (✔️ 48 passed, 🙈 3 skipped)
   🚦   bdd-windows-otel: No test results available
   🚦   bdd-freertos-qemu-plustcp: 86% successful (✔️ 44 passed, 🙈 7 skipped)
   🚦   bdd-freertos-qemu-lwip: 63% successful (✔️ 32 passed, 🙈 19 skipped)
   🚦   build-windows-msvc: 100% successful (✔️ 1217 passed)
   🚦   build-linux-tunable-override: 100% successful (✔️ 1364 passed)
   ⚠️   Clang-Tidy: No warnings
   ⚠️   CPPCheck: No warnings

---

🚧   Error Messages

Quality Monitor Errors:
No matching report files found when using pattern '**/junit-bdd-windows-otel/TESTS-*.xml'! Configuration error for 'bdd-windows-otel'?

Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result.

[github-actions]

☀️   Quality Summary

   🚦   build-linux-gcc: 100% successful (✔️ 1418 passed)
   🚦   build-freertos-host-tdd-plustcp: 100% successful (✔️ 1733 passed)
   🚦   build-linux-clang: 100% successful (✔️ 1364 passed)
   🚦   sanitize-linux-gcc: 100% successful (✔️ 1364 passed)
   🚦   integration-linux-openssl: 100% successful (✔️ 16 passed)
   🚦   integration-linux-mbedtls: 100% successful (✔️ 14 passed)
   🚦   integration-windows-openssl: 100% successful (✔️ 16 passed)
   🚦   bdd-linux-syslog-ng: 94% successful (✔️ 48 passed, 🙈 3 skipped)
   🚦   bdd-windows-otel: 88% successful (✔️ 45 passed, 🙈 6 skipped)
   🚦   bdd-freertos-qemu-plustcp: 86% successful (✔️ 44 passed, 🙈 7 skipped)
   🚦   bdd-freertos-qemu-lwip: 63% successful (✔️ 32 passed, 🙈 19 skipped)
   🚦   build-windows-msvc: 100% successful (✔️ 1217 passed)
   🚦   build-linux-tunable-override: 100% successful (✔️ 1364 passed)
   ⚠️   Clang-Tidy: No warnings
   ⚠️   CPPCheck: No warnings

---

Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result.