feat(http): add RespondBytes for raw binary response payloads - #133
Merged
Conversation
Closes the gap between RespondText and RespondJson - a consumer needing to serve raw bytes (e.g. a fetched certificate's DER bytes) previously had to round-trip through a Latin1-encoded string, an awkward workaround surfaced by dogfood evidence in alexa-vox-craft. Follows RespondJson's serialize-once-to-bytes model; recorded as ADR-0051 Amendment 2. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GMQEjFrUVGYkctE3ECfRuA
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f035438b42
ℹ️ 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".
…ge guide Address Codex review feedback on PR #133: RespondBytes retained the caller's byte[] by reference instead of copying it, so a post-registration mutation or buffer reuse would silently change an already-registered response - breaking the serialize-once snapshot semantics RespondJson already provides and this method's own doc claimed to match. Also add RespondBytes to docs/packages/compono-http.md's Response APIs list, which was updated everywhere except the consumer-facing package guide. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GMQEjFrUVGYkctE3ECfRuA
Contributor
Author
…edback The task file's Phase 3 covered replying to and resolving feedback but never mentioned requesting a fresh bot review scoped to the fix commit once real code changes are pushed - an established pattern this session used, now made explicit so it isn't re-derived ad hoc next time. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GMQEjFrUVGYkctE3ECfRuA
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4bc5838f5a
ℹ️ 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".
…erence The published compono skill's http.md enumerated every Respond* finalizer for consumers writing Compono.Http tests, but was written before RespondBytes existed (PR #133). Add it alongside the others so skill guidance doesn't quietly fall behind the package's actual API surface. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GMQEjFrUVGYkctE3ECfRuA
Address Codex review feedback on PR #133: PLAN-0051's Response APIs (Task 4) and Behavioral tests (Task 9) checklists still only enumerated the pre-Amendment-2 Respond* set, so the plan no longer accurately reflected Compono.Http's completed public behavior and test coverage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GMQEjFrUVGYkctE3ECfRuA
ncipollina
added a commit
that referenced
this pull request
Sep 7, 2026
…kage-validation CI, regen API docs
Three real generator-correctness bugs found by automated PR review, each with a
new generator-fixture regression test:
- ClearCalls()/ReceivedCalls() are always-emitted, always-zero-argument bridge
extensions exactly like Configure()/Verify(), but the CMP0023 reserved-name
collision check only covered the latter two - an interface declaring its own
zero-argument ClearCalls/ReceivedCalls member silently shadowed the generated
bridge with no diagnostic. Widened the reserved-name set in TestDoubleAnalyzer.
- An eligible member's generated {FieldName}_ReceivedCall record-class name could
collide with an unrelated sibling member's own natural field name, producing a
real CS0102 duplicate-declaration compile error. Fixed by feeding this derived
name into the existing derivedAuxiliaryNameOwners collision pre-pass, which
demotes the colliding member out of matching eligibility instead.
- A parameter literally named the same as its own member's generated record type
produced a positional record property sharing the enclosing type's name
(CS0542). Fixed in TestDouble.scriban by suffixing that one parameter's
declared name inside the record declaration only.
Also fixes two unrelated CI blockers surfaced on this PR:
- .github/workflows/package-validation.yaml never set -p:Version on its
validation-only pack, defaulting to 1.0.0.0 - this started failing ApiCompat's
CP0003 the moment nuget.org's published baseline crossed 1.0.0
(1.1.0-preview.103, published by PR #133's merge to main). Every PR's
package-validation would fail this way regardless of content; pinned the
packed Version to the resolved baseline.
- docs/reference/api/ had drifted from PLAN-0063's new public members
(CallVerifier.AtLeast/AtMost, LogVerificationBuilder.AtLeast/AtMost,
ReturnConfig<T>.ClearObservedCalls) - regenerated via
.github/scripts/generate-api-reference.sh.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NiVv392P3m46azTD1TpU3s
ncipollina
added a commit
that referenced
this pull request
Sep 7, 2026
…nd ClearCalls() (#134) * feat(testdoubles): add CallVerifier.AtLeast/AtMost, ReceivedCalls() and ClearCalls() Adds AtLeast(int)/AtMost(int) count verification to core CallVerifier, forwarded through Compono.Logging's LogVerificationBuilder, and reachable from Compono.TestDoubles/Compono.Http with no package-side changes (ADR-0044 Amendment 22). Adds Compono.TestDoubles generated ReceivedCalls() (snapshot-based retrospective call inspection for ADR-0048-eligible members) and whole-double ClearCalls() (observation reset preserving configured behavior) per ADR-0060. Includes generator/runtime changes, tests across all affected packages (unit, generated-source snapshots, AOT smoke, concurrency), docs, skill and eval updates, and a completed baseline-vs-updated eval comparison and dogfood validation run, all tracked in PLAN-0063. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NiVv392P3m46azTD1TpU3s * fix(testdoubles): resolve Codex-flagged generator collisions, fix package-validation CI, regen API docs Three real generator-correctness bugs found by automated PR review, each with a new generator-fixture regression test: - ClearCalls()/ReceivedCalls() are always-emitted, always-zero-argument bridge extensions exactly like Configure()/Verify(), but the CMP0023 reserved-name collision check only covered the latter two - an interface declaring its own zero-argument ClearCalls/ReceivedCalls member silently shadowed the generated bridge with no diagnostic. Widened the reserved-name set in TestDoubleAnalyzer. - An eligible member's generated {FieldName}_ReceivedCall record-class name could collide with an unrelated sibling member's own natural field name, producing a real CS0102 duplicate-declaration compile error. Fixed by feeding this derived name into the existing derivedAuxiliaryNameOwners collision pre-pass, which demotes the colliding member out of matching eligibility instead. - A parameter literally named the same as its own member's generated record type produced a positional record property sharing the enclosing type's name (CS0542). Fixed in TestDouble.scriban by suffixing that one parameter's declared name inside the record declaration only. Also fixes two unrelated CI blockers surfaced on this PR: - .github/workflows/package-validation.yaml never set -p:Version on its validation-only pack, defaulting to 1.0.0.0 - this started failing ApiCompat's CP0003 the moment nuget.org's published baseline crossed 1.0.0 (1.1.0-preview.103, published by PR #133's merge to main). Every PR's package-validation would fail this way regardless of content; pinned the packed Version to the resolved baseline. - docs/reference/api/ had drifted from PLAN-0063's new public members (CallVerifier.AtLeast/AtMost, LogVerificationBuilder.AtLeast/AtMost, ReturnConfig<T>.ClearObservedCalls) - regenerated via .github/scripts/generate-api-reference.sh. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NiVv392P3m46azTD1TpU3s * fix(testdoubles): make ReceivedCalls() record-parameter rename collision-free, not just naive Codex review round 2 (PR #134) caught a real gap in the previous fix: the scriban template's unconditional "_Value" suffix for a parameter colliding with its own record's type name could itself collide with a second real parameter already literally named that (`Foo(int __Foo_ReceivedCall, int __Foo_ReceivedCall_Value)`), producing a duplicate positional-record-property compile error. Moved the rename computation into a new TestDoubleMemberInfo.ReceivedCallRecordParametersText C# property, which checks the candidate against every real parameter's own name (not just the colliding one) and increments a numeric suffix until free (__Foo_ReceivedCall_Value2, _Value3, ...). Wired through TestDoubleEmitter.cs's anonymous projection model, which the first attempt had missed - Scriban's default reflection binding only sees properties on the wrapper anonymous object passed to Render(), not arbitrary computed properties on the underlying TestDoubleMemberInfo record. Added ReceivedCallRecordParameterNameCollidesWithBothRecordTypeAndItsNaiveRenameCandidate_ConsumerCompiles covering exactly the two-parameter collision Codex identified. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NiVv392P3m46azTD1TpU3s --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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
2 participants
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.
Summary
Adds
HttpResponseRegistrationBuilder.RespondBytes(byte[], mediaType)toCompono.Http, closing a gap betweenRespondTextandRespondJson: no way to serve a raw binary payload. Surfaced by dogfood evidence inalexa-vox-craft, where a test needing to serve a fetched certificate's DER bytes had to round-trip them throughEncoding.Latin1text as a workaround.Changes
src/Compono.Http/HttpResponseRegistrationBuilder.cs- newRespondBytes(byte[] content, string mediaType = "application/octet-stream"), followingRespondJson's serialize-once-to-bytes model (content captured once, a freshByteArrayContent+MediaTypeHeaderValueper matched invocation).docs/adr/0051-compono-http-handler-based-testing-package.md- recorded as Amendment 2 (purely additive, source-compatible extension, not a reversal of the original decision).test/Compono.Http.Tests/TestHttpHandlerTests.cs- two new tests: byte round-trip with default content type, and custom media type.docs/reference/api/Compono.Http/- regenerated via.github/scripts/generate-api-reference.sh.Validation
dotnet test test/Compono.Http.Tests/Compono.Http.Tests.csproj -f net10.0- 31/31 passed