chore: resolve review comments from previous PR#1273
Merged
Richard Peters (richardapeters) merged 1 commit intoJul 15, 2026
Conversation
Copilot started reviewing on behalf of
Richard Peters (richardapeters)
July 15, 2026 09:43
View session
(cherry picked from commit 98a65f0)
Richard Peters (richardapeters)
force-pushed
the
feature/split_sesame_buffers_rest
branch
from
July 15, 2026 09:45
98a65f0 to
dfb9600
Compare
Contributor
Dependency ReviewThe following issues were found:
Snapshot WarningsConsider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. License Issuesexternal/crypto/mbedtls/CMakeLists.txt
infra/syntax/CMakeLists.txt
cmake/emil_test_helpers.cmake
osal/threadx/CMakeLists.txt
external/args/CMakeLists.txt
external/crypto/micro-ecc/CMakeLists.txt
external/segger_rtt/CMakeLists.txt
osal/freertos/CMakeLists.txt
external/protobuf/CMakeLists.txt
external/crypto/tiny-aes128/CMakeLists.txt
lwip/lwip/CMakeLists.txt
OpenSSF ScorecardScorecard details
Scanned Files
|
Daan Timmer (daantimmer)
approved these changes
Jul 15, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the Sesame/Echo instantiation helpers to support configurable split receive buffers in SesameWindowed, updates tracing/secured variants accordingly, and adds/adjusts unit tests to validate the new instantiation paths.
Changes:
- Make
services::SesameWindowedsupport a configurable number of split receive buffers and update buffer sizing calculations. - Refactor
main_::Sesame*andmain_::EchoOnSesame*instantiation helpers to pass shared storage viaCobsStorageBase(including secured and tracing variants). - Add Echo instantiation tests (plain + secured) and update existing SesameWindowed/instantiation tests; tweak Diffie-Hellman policy proxy re-queuing.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| services/util/test/TestSesameWindowed.cpp | Updates fixture to exercise split-buffer configuration and adds a 3-way buffer test. |
| services/util/test/TestSesameInstantiationSecured.cpp | Simplifies template size usage in secured Sesame instantiation test. |
| services/util/test/TestSesameInstantiation.cpp | Simplifies template size usage in Sesame instantiation test. |
| services/util/test/TestEchoInstantiationSecured.cpp | Adds new tests for secured Echo-on-Sesame instantiation (symmetric key + Diffie-Hellman). |
| services/util/test/TestEchoInstantiation.cpp | Adds new tests for plain Echo-on-Sesame instantiation. |
| services/util/test/CMakeLists.txt | Registers the new Echo instantiation tests (secured test gated by mbedTLS). |
| services/util/SesameWindowed.hpp | Introduces split-buffer aware sizing helpers and a new WithMaxMessageSize wrapper. |
| services/util/SesameWindowed.cpp | Implements split-buffer logic, additional init-handshake bookkeeping, and default hook definitions. |
| services/util/SesameInstantiationSecured.hpp | Extends secured Sesame instantiation to accept split-buffer parameter and new storage base wiring. |
| services/util/SesameInstantiationSecured.cpp | Switches secured Sesame construction to use CobsStorageBase. |
| services/util/SesameInstantiation.hpp | Adds CobsStorageBase and propagates split-buffer configuration into windowed receive storage sizing. |
| services/util/SesameInstantiation.cpp | Implements CobsStorageBase and updates Sesame construction accordingly. |
| services/util/SerialCommunicationLoopback.hpp | Uses infra::AutoResetFunction for completion callbacks. |
| services/util/EchoPolicyDiffieHellman.cpp | Prevents re-queuing proxies while still busy/initializing; triggers re-queue after busy clears. |
| services/util/EchoInstantiationSecured.hpp | Propagates CobsStorageBase + split-buffer parameter through secured Echo instantiation templates. |
| services/util/EchoInstantiationSecured.cpp | Updates secured Echo constructors to use CobsStorageBase. |
| services/util/EchoInstantiation.hpp | Propagates CobsStorageBase + split-buffer parameter through plain Echo instantiation templates. |
| services/util/EchoInstantiation.cpp | Updates plain Echo constructor to use CobsStorageBase. |
| services/tracer/TracingSesameWindowed.hpp | Updates tracing windowed wrapper for split-buffers and adds a WithMaxMessageSize wrapper. |
| services/tracer/TracingSesameWindowed.cpp | Updates tracing windowed construction to pass split-buffer count and initializer. |
| services/tracer/TracingEchoInstantiationSecured.hpp | Updates secured tracing Echo instantiation signatures/templates for new storage model and split buffers. |
| services/tracer/TracingEchoInstantiationSecured.cpp | Updates secured tracing Echo construction to use CobsStorageBase and split buffers. |
| services/tracer/TracingEchoInstantiation.hpp | Updates tracing Echo instantiation templates to use CobsStorageBase and split buffers. |
| services/tracer/TracingEchoInstantiation.cpp | Updates tracing Echo construction to use CobsStorageBase and split buffers. |
Comments suppressed due to low confidence (1)
services/util/test/TestSesameWindowed.cpp:23
- The fixture constructor adds a generic
EXPECT_CALL(base, WorstCaseEncodedMessageSize(_))after theinfra::Executemember has already installed a more specificEXPECT_CALL(...WorstCaseEncodedMessageSize(3)). In gMock, later expectations take precedence, so the generic matcher can steal thesize==3call and leave the specific expectation unsatisfied.
SesameWindowedTest()
{
SetEncodingSizeExpectations();
}
Copilot started reviewing on behalf of
Richard Peters (richardapeters)
July 15, 2026 09:49
View session
Contributor
✅
|
| Descriptor | Linter | Files | Fixed | Errors | Warnings | Elapsed time |
|---|---|---|---|---|---|---|
| ✅ ACTION | actionlint | 11 | 0 | 0 | 0.31s | |
| ✅ ACTION | zizmor | 11 | 0 | 0 | 0 | 2.71s |
| ✅ CPP | clang-format | 1098 | 8 | 0 | 0 | 10.01s |
| ✅ DOCKERFILE | hadolint | 2 | 0 | 0 | 0.06s | |
| ✅ JSON | jsonlint | 7 | 0 | 0 | 0.39s | |
| ✅ JSON | prettier | 7 | 0 | 0 | 0 | 0.68s |
| markdownlint | 8 | 0 | 5 | 0 | 1.38s | |
| ✅ MARKDOWN | markdown-table-formatter | 8 | 0 | 0 | 0 | 0.34s |
| ✅ REPOSITORY | checkov | yes | no | no | 35.58s | |
| ✅ REPOSITORY | git_diff | yes | no | no | 0.09s | |
| ✅ REPOSITORY | grype | yes | no | no | 56.64s | |
| ✅ REPOSITORY | ls-lint | yes | no | no | 0.02s | |
| osv-scanner | yes | 1 | no | 0.59s | ||
| ✅ REPOSITORY | secretlint | yes | no | no | 17.71s | |
| ✅ REPOSITORY | syft | yes | no | no | 1.91s | |
| ✅ REPOSITORY | trivy | yes | no | no | 18.58s | |
| ✅ REPOSITORY | trivy-sbom | yes | no | no | 0.26s | |
| ✅ REPOSITORY | trufflehog | yes | no | no | 14.04s | |
| lychee | 140 | 1 | 0 | 150.41s | ||
| prettier | 21 | 1 | 1 | 0 | 0.7s | |
| ✅ YAML | v8r | 21 | 0 | 0 | 10.93s | |
| ✅ YAML | yamllint | 21 | 0 | 0 | 0.79s |
Detailed Issues
⚠️ SPELL / lychee - 1 error
📝 Summary
---------------------
🔍 Total..........697
🔗 Unique.........661
✅ Successful.....691
⏳ Timeouts.........0
🔀 Redirected.....309
👻 Excluded.........5
❓ Unknown..........0
🚫 Errors...........1
⛔ Unsupported......1
Errors in external/protoc/CMakeLists.txt
[404] https://github.com/protocolbuffers/protobuf/releases/download/v$%7Bprotobuf_tag%7D/protoc-$%7Bprotobuf_version%7D-$%7Bos_postfix%7D.zip (at 18:13) | Rejected status code: 404 Not Found
Hint: Followed 309 redirects. You might want to consider replacing redirecting URLs with the resolved URLs. Use verbose mode (`-v`/`-vv`) to see redirection details.
Hint: You can configure accepted/rejected response codes with `-a` or `--accept`
⚠️ MARKDOWN / markdownlint - 5 errors
.github/instructions/microtest.instructions.md:7 error MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "## Google Test Suite Coding Ru..."]
external/crypto/tiny-aes128/README.md:1 error MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "### Tiny AES128 in C"]
external/crypto/tiny-aes128/README.md:29 error MD046/code-block-style Code block style [Expected: fenced; Actual: indented]
external/crypto/tiny-aes128/README.md:39 error MD046/code-block-style Code block style [Expected: fenced; Actual: indented]
external/crypto/tiny-aes128/README.md:49 error MD046/code-block-style Code block style [Expected: fenced; Actual: indented]
⚠️ REPOSITORY / osv-scanner - 1 error
Scanning dir .
Starting filesystem walk for root: /
Scanned external/segger_rtt file and found 0 packages
Scanned external/args file and found 0 packages
Scanned external/protobuf file and found 0 packages
Scanned external/protoc file and found 0 packages
Scanned external/crypto file and found 0 packages
End status: 155 dirs visited, 1618 inodes visited, 5 Extract calls, 458.279136ms elapsed, 458.279276ms wall time
No package sources found, --help for usage information.
⚠️ YAML / prettier - 1 error
[error] Explicitly specified pattern "documents/modules/ROOT/examples/clangformat.yaml" is a symbolic link.
.clusterfuzzlite/project.yaml 38ms (unchanged)
.github/dependabot.yml 17ms (unchanged)
.github/workflows/ci.yml 62ms (unchanged)
.github/workflows/dependency-scanner.yml 9ms (unchanged)
.github/workflows/documentation.yml 24ms (unchanged)
.github/workflows/fuzzing-batch.yml 6ms (unchanged)
.github/workflows/fuzzing-cron.yml 6ms (unchanged)
.github/workflows/fuzzing-pr.yml 7ms (unchanged)
.github/workflows/linting-formatting.yml 13ms (unchanged)
.github/workflows/release-please.yml 9ms (unchanged)
.github/workflows/security.yml 5ms (unchanged)
.github/workflows/static-analysis.yml 16ms (unchanged)
.github/workflows/validate-pr.yml 13ms (unchanged)
.ls-lint.yml 2ms
.mega-linter.yml 2ms (unchanged)
antora-playbook-branch.yml 3ms (unchanged)
antora-playbook-site.yml 5ms (unchanged)
documents/antora.yml 2ms (unchanged)
documents/supplemental-ui/ui.yml 1ms (unchanged)
mull.yml 1ms (unchanged)
Notices
📣 MegaLinter 9.5.0 is out! Discover the new features and security recommendations in the release announcement. (Skip this info by defining SECURITY_SUGGESTIONS: false)
See detailed reports in MegaLinter artifacts
Your project could benefit from a custom flavor, which would allow you to run only the linters you need, and thus improve runtime performances. (Skip this info by defining FLAVOR_SUGGESTIONS: false)
- Documentation: Custom Flavors
- Command:
npx mega-linter-runner@9.5.0 --custom-flavor-setup --custom-flavor-linters ACTION_ACTIONLINT,ACTION_ZIZMOR,CPP_CLANG_FORMAT,DOCKERFILE_HADOLINT,JSON_JSONLINT,JSON_PRETTIER,MARKDOWN_MARKDOWNLINT,MARKDOWN_MARKDOWN_TABLE_FORMATTER,REPOSITORY_CHECKOV,REPOSITORY_GIT_DIFF,REPOSITORY_GRYPE,REPOSITORY_LS_LINT,REPOSITORY_OSV_SCANNER,REPOSITORY_SECRETLINT,REPOSITORY_SYFT,REPOSITORY_TRIVY,REPOSITORY_TRIVY_SBOM,REPOSITORY_TRUFFLEHOG,SPELL_LYCHEE,YAML_PRETTIER,YAML_YAMLLINT,YAML_V8R
Show us your support by starring ⭐ the repository
Richard Peters (richardapeters)
enabled auto-merge (squash)
July 15, 2026 10:03
|
Richard Peters (richardapeters)
deleted the
feature/split_sesame_buffers_rest
branch
July 15, 2026 10:12
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previous pull request was merged just too early. This resolves the comments.