diff --git a/.agents/skills/engineering-workflow/tasks/respond-to-pr-feedback.md b/.agents/skills/engineering-workflow/tasks/respond-to-pr-feedback.md index 85bc323c..98381371 100644 --- a/.agents/skills/engineering-workflow/tasks/respond-to-pr-feedback.md +++ b/.agents/skills/engineering-workflow/tasks/respond-to-pr-feedback.md @@ -224,6 +224,14 @@ pass over every comment from phase 1. top-level issue comments and review bodies have no "resolved" state on GitHub at all; the permalink-marked reply in step 8 is the entire deliverable for those (see step 2). +10. **If this pass had real code changes** (not doc-only fixes) **and a bot + reviewer is configured on the repo** (e.g. `chatgpt-codex-connector`), + request a fresh review scoped to what actually changed — `gh pr comment + --body "@codex review commit — "` — rather than a bare "@codex review". Skip + this step for a round that only touched docs/comments with no + behavioral diff; re-running a bot review round costs real turnaround + and isn't warranted when there's nothing new for it to check. ## Output diff --git a/docs/adr/0051-compono-http-handler-based-testing-package.md b/docs/adr/0051-compono-http-handler-based-testing-package.md index 7c381ea5..2e9d95d0 100644 --- a/docs/adr/0051-compono-http-handler-based-testing-package.md +++ b/docs/adr/0051-compono-http-handler-based-testing-package.md @@ -610,3 +610,37 @@ opacity is a deliberate ADR-0048 property this amendment doesn't weaken just to let `Compono.Http` introspect it later. If a future need for richer `Match` diagnostics surfaces beyond this one case, that's a separate, its-own-evidence decision. + +## Amendment 2 (2026-09-04): `RespondBytes` for raw binary payloads + +Dogfood evidence from `alexa-vox-craft` (`RequestVerificationTests.GetCertificate_ParsesFetchedCertificateBytes`) +surfaced a gap: `HttpResponseRegistrationBuilder` had `RespondText` (string +content) and `RespondJson` (serialized-to-JSON content), but no way to +respond with an arbitrary byte payload. That test needed to serve a +fetched X.509 certificate's raw DER bytes, and worked around the gap by +round-tripping the bytes through `Encoding.Latin1` text - lossless (Latin1 +is a single-byte, bijective 0-255 char\<-\>byte mapping, unlike UTF-8) but +an awkward, easy-to-get-wrong pattern for what is fundamentally a +binary-content need, not a text one. + +**Decision**: add `RespondBytes(byte[] content, string mediaType = +"application/octet-stream")`, following the same +serialize-once-to-bytes model as `RespondJson` (this ADR's original +Decision Outcome, "Serialize-once-to-bytes model"): `content` is +defensively copied (`(byte[])content.Clone()`) once at registration time +- not retained by reference - and every matched invocation constructs a +fresh `ByteArrayContent` over that private copy, with its own +`MediaTypeHeaderValue` - never a shared, mutable header instance across +responses (same reasoning as `RespondJsonBytes`'s existing +`MediaTypeHeaderValue` handling). The clone matters specifically because, +unlike `RespondJson` (which already produces its own private buffer via +`JsonSerializer.SerializeToUtf8Bytes`), `RespondBytes`'s `content` comes +in as a caller-owned array - without the clone, a caller mutating or +reusing that array after registration would silently change an +already-registered response, which is exactly the snapshot semantics +`RespondJson` already provides and this method's own doc claims to match. +Caught in PR review (Codex, on the implementation PR) before merge. + +**Explicitly not done**: no change to `RespondText` or `RespondJson` - +this is a purely additive third `Respond*` overload, source-compatible +with every existing call site. diff --git a/docs/packages/compono-http.md b/docs/packages/compono-http.md index 51b4fa3b..27d19f3f 100644 --- a/docs/packages/compono-http.md +++ b/docs/packages/compono-http.md @@ -87,6 +87,11 @@ registration.Verify().Once(); ergonomic runtime-metadata path (see JSON/AOT below). - `RespondJson(T value, JsonTypeInfo jsonTypeInfo)` — the source-generated, AOT-safe path. + - `RespondBytes(byte[] content, string mediaType = "application/octet-stream")` — + raw binary payloads (e.g. fetched certificate bytes) that `RespondText` + can't carry without a lossy or awkward text encoding. `content` is + defensively copied at registration time, so mutating the caller's array + afterward never changes the registered response. - `Throws(Exception exception)` — rethrows the **same instance** on every matched invocation; there's no exception-factory overload. - Every `Respond*` call builds a **fresh** response/content per matched diff --git a/docs/plans/0051-compono-http-handler-based-testing-package-impl-plan.md b/docs/plans/0051-compono-http-handler-based-testing-package-impl-plan.md index 5aef7d16..ee0b6027 100644 --- a/docs/plans/0051-compono-http-handler-based-testing-package-impl-plan.md +++ b/docs/plans/0051-compono-http-handler-based-testing-package-impl-plan.md @@ -339,6 +339,13 @@ these, even if implementation makes one look easy to add along the way: behavior, ADR-0051 — no cloning, no factory parameter). - [x] Every `Respond*`/`Throws` call finalizes and returns the `HttpResponseRegistration` handle (not `void`). +- [x] `RespondBytes(byte[] content, string mediaType = "application/octet-stream")` + (ADR-0051 Amendment 2, added after this plan's original `Done` + status) — `content` is defensively copied (`(byte[])content.Clone()`) + once at registration time, not retained by reference; each + invocation constructs a fresh `ByteArrayContent` over that private + copy with its own `MediaTypeHeaderValue`, matching `RespondJson`'s + serialize-once-to-bytes model. ### 5. JSON/AOT correctness @@ -503,6 +510,11 @@ these, even if implementation makes one look easy to add along the way: consumer). - [x] `RespondJson` sets `Content-Type: application/json; charset=utf-8` correctly on every fresh `ByteArrayContent`. +- [x] `RespondBytes` round-trips content and defaults to + `application/octet-stream`, honors a supplied `mediaType`, and a + mutation to the caller's array after registration doesn't affect + an already-registered response (ADR-0051 Amendment 2 — added after + this plan's original `Done` status). - [x] `Throws(exception)` rethrows the exact same instance (`ReferenceEquals`) on repeated matches. - [x] `registration.Verify().Never()/.Once()/.Exactly(n)` — including the diff --git a/docs/reference/api/Compono.Http/Compono.Http.HttpResponseRegistrationBuilder.RespondBytes(byte[],string).md b/docs/reference/api/Compono.Http/Compono.Http.HttpResponseRegistrationBuilder.RespondBytes(byte[],string).md new file mode 100644 index 00000000..065512fc --- /dev/null +++ b/docs/reference/api/Compono.Http/Compono.Http.HttpResponseRegistrationBuilder.RespondBytes(byte[],string).md @@ -0,0 +1,30 @@ +#### [Compono\.Http](index.md 'index') +### [Compono\.Http](Compono.Http.md 'Compono\.Http').[HttpResponseRegistrationBuilder](Compono.Http.HttpResponseRegistrationBuilder.md 'Compono\.Http\.HttpResponseRegistrationBuilder') + +## HttpResponseRegistrationBuilder\.RespondBytes\(byte\[\], string\) Method + +Responds with HTTP 200 OK and [content](Compono.Http.HttpResponseRegistrationBuilder.RespondBytes(byte[],string).md#Compono.Http.HttpResponseRegistrationBuilder.RespondBytes(byte[],string).content 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondBytes\(byte\[\], string\)\.content') verbatim, as +[mediaType](Compono.Http.HttpResponseRegistrationBuilder.RespondBytes(byte[],string).md#Compono.Http.HttpResponseRegistrationBuilder.RespondBytes(byte[],string).mediaType 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondBytes\(byte\[\], string\)\.mediaType') \- the raw\-bytes counterpart to [RespondText\(string, string, Encoding\)](Compono.Http.HttpResponseRegistrationBuilder.RespondText(string,string,System.Text.Encoding).md 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondText\(string, string, System\.Text\.Encoding\)') for +binary payloads \(e\.g\. a fetched certificate's DER bytes\) that would otherwise need a lossy +or awkward text encoding to round\-trip through [RespondText\(string, string, Encoding\)](Compono.Http.HttpResponseRegistrationBuilder.RespondText(string,string,System.Text.Encoding).md 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondText\(string, string, System\.Text\.Encoding\)')\. Same +serialize\-once\-to\-bytes model as [RespondJson<T>\(T, JsonSerializerOptions\)](Compono.Http.HttpResponseRegistrationBuilder.RespondJson.md#Compono.Http.HttpResponseRegistrationBuilder.RespondJson_T_(T,System.Text.Json.JsonSerializerOptions) 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondJson\\(T, System\.Text\.Json\.JsonSerializerOptions\)'): +[content](Compono.Http.HttpResponseRegistrationBuilder.RespondBytes(byte[],string).md#Compono.Http.HttpResponseRegistrationBuilder.RespondBytes(byte[],string).content 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondBytes\(byte\[\], string\)\.content') is defensively copied once at registration time \- not retained +by reference \- so a caller mutating or reusing its own array afterward can never change an +already\-registered response; every matched invocation constructs a fresh +[System\.Net\.Http\.ByteArrayContent](https://learn.microsoft.com/en-us/dotnet/api/system.net.http.bytearraycontent 'System\.Net\.Http\.ByteArrayContent') over that private copy \(ADR\-0051 Amendment 2\)\. + +```csharp +public Compono.Http.HttpResponseRegistration RespondBytes(byte[] content, string mediaType="application/octet-stream"); +``` +#### Parameters + + + +`content` [System\.Byte](https://learn.microsoft.com/en-us/dotnet/api/system.byte 'System\.Byte')[\[\]](https://learn.microsoft.com/en-us/dotnet/api/system.array 'System\.Array') + + + +`mediaType` [System\.String](https://learn.microsoft.com/en-us/dotnet/api/system.string 'System\.String') + +#### Returns +[HttpResponseRegistration](Compono.Http.HttpResponseRegistration.md 'Compono\.Http\.HttpResponseRegistration') \ No newline at end of file diff --git a/docs/reference/api/Compono.Http/Compono.Http.HttpResponseRegistrationBuilder.md b/docs/reference/api/Compono.Http/Compono.Http.HttpResponseRegistrationBuilder.md index 5331060e..23682e8d 100644 --- a/docs/reference/api/Compono.Http/Compono.Http.HttpResponseRegistrationBuilder.md +++ b/docs/reference/api/Compono.Http/Compono.Http.HttpResponseRegistrationBuilder.md @@ -17,6 +17,7 @@ Inheritance [System\.Object](https://learn.microsoft.com/en-us/dotnet/api/system | Methods | | | :--- | :--- | | [Respond\(HttpStatusCode\)](Compono.Http.HttpResponseRegistrationBuilder.Respond(System.Net.HttpStatusCode).md 'Compono\.Http\.HttpResponseRegistrationBuilder\.Respond\(System\.Net\.HttpStatusCode\)') | Responds with [statusCode](Compono.Http.HttpResponseRegistrationBuilder.Respond(System.Net.HttpStatusCode).md#Compono.Http.HttpResponseRegistrationBuilder.Respond(System.Net.HttpStatusCode).statusCode 'Compono\.Http\.HttpResponseRegistrationBuilder\.Respond\(System\.Net\.HttpStatusCode\)\.statusCode') and no content\. | +| [RespondBytes\(byte\[\], string\)](Compono.Http.HttpResponseRegistrationBuilder.RespondBytes(byte[],string).md 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondBytes\(byte\[\], string\)') | Responds with HTTP 200 OK and [content](Compono.Http.HttpResponseRegistrationBuilder.RespondBytes(byte[],string).md#Compono.Http.HttpResponseRegistrationBuilder.RespondBytes(byte[],string).content 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondBytes\(byte\[\], string\)\.content') verbatim, as [mediaType](Compono.Http.HttpResponseRegistrationBuilder.RespondBytes(byte[],string).md#Compono.Http.HttpResponseRegistrationBuilder.RespondBytes(byte[],string).mediaType 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondBytes\(byte\[\], string\)\.mediaType') \- the raw\-bytes counterpart to [RespondText\(string, string, Encoding\)](Compono.Http.HttpResponseRegistrationBuilder.RespondText(string,string,System.Text.Encoding).md 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondText\(string, string, System\.Text\.Encoding\)') for binary payloads \(e\.g\. a fetched certificate's DER bytes\) that would otherwise need a lossy or awkward text encoding to round\-trip through [RespondText\(string, string, Encoding\)](Compono.Http.HttpResponseRegistrationBuilder.RespondText(string,string,System.Text.Encoding).md 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondText\(string, string, System\.Text\.Encoding\)')\. Same serialize\-once\-to\-bytes model as [RespondJson<T>\(T, JsonSerializerOptions\)](Compono.Http.HttpResponseRegistrationBuilder.RespondJson.md#Compono.Http.HttpResponseRegistrationBuilder.RespondJson_T_(T,System.Text.Json.JsonSerializerOptions) 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondJson\\(T, System\.Text\.Json\.JsonSerializerOptions\)'): [content](Compono.Http.HttpResponseRegistrationBuilder.RespondBytes(byte[],string).md#Compono.Http.HttpResponseRegistrationBuilder.RespondBytes(byte[],string).content 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondBytes\(byte\[\], string\)\.content') is defensively copied once at registration time \- not retained by reference \- so a caller mutating or reusing its own array afterward can never change an already\-registered response; every matched invocation constructs a fresh [System\.Net\.Http\.ByteArrayContent](https://learn.microsoft.com/en-us/dotnet/api/system.net.http.bytearraycontent 'System\.Net\.Http\.ByteArrayContent') over that private copy \(ADR\-0051 Amendment 2\)\. | | [RespondJson<T>\(T, JsonSerializerOptions\)](Compono.Http.HttpResponseRegistrationBuilder.RespondJson.md#Compono.Http.HttpResponseRegistrationBuilder.RespondJson_T_(T,System.Text.Json.JsonSerializerOptions) 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondJson\\(T, System\.Text\.Json\.JsonSerializerOptions\)') | Responds with HTTP 200 OK and [value](Compono.Http.HttpResponseRegistrationBuilder.RespondJson.md#Compono.Http.HttpResponseRegistrationBuilder.RespondJson_T_(T,System.Text.Json.JsonSerializerOptions).value 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondJson\\(T, System\.Text\.Json\.JsonSerializerOptions\)\.value') serialized to JSON, using the ordinary runtime\-metadata [System\.Text\.Json\.JsonSerializer](https://learn.microsoft.com/en-us/dotnet/api/system.text.json.jsonserializer 'System\.Text\.Json\.JsonSerializer') path\. [value](Compono.Http.HttpResponseRegistrationBuilder.RespondJson.md#Compono.Http.HttpResponseRegistrationBuilder.RespondJson_T_(T,System.Text.Json.JsonSerializerOptions).value 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondJson\\(T, System\.Text\.Json\.JsonSerializerOptions\)\.value') is serialized once, here, to an immutable byte buffer; every matched invocation constructs a fresh [System\.Net\.Http\.ByteArrayContent](https://learn.microsoft.com/en-us/dotnet/api/system.net.http.bytearraycontent 'System\.Net\.Http\.ByteArrayContent') over that same buffer with its own explicit `Content-Type` header \(ADR\-0051 "Serialize\-once\-to\-bytes model"\)\. | | [RespondJson<T>\(T, JsonTypeInfo<T>\)](Compono.Http.HttpResponseRegistrationBuilder.RespondJson.md#Compono.Http.HttpResponseRegistrationBuilder.RespondJson_T_(T,System.Text.Json.Serialization.Metadata.JsonTypeInfo_T_) 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondJson\\(T, System\.Text\.Json\.Serialization\.Metadata\.JsonTypeInfo\\)') | Responds with HTTP 200 OK and [value](Compono.Http.HttpResponseRegistrationBuilder.RespondJson.md#Compono.Http.HttpResponseRegistrationBuilder.RespondJson_T_(T,System.Text.Json.Serialization.Metadata.JsonTypeInfo_T_).value 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondJson\\(T, System\.Text\.Json\.Serialization\.Metadata\.JsonTypeInfo\\)\.value') serialized to JSON via [jsonTypeInfo](Compono.Http.HttpResponseRegistrationBuilder.RespondJson.md#Compono.Http.HttpResponseRegistrationBuilder.RespondJson_T_(T,System.Text.Json.Serialization.Metadata.JsonTypeInfo_T_).jsonTypeInfo 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondJson\\(T, System\.Text\.Json\.Serialization\.Metadata\.JsonTypeInfo\\)\.jsonTypeInfo') \(e\.g\. a source\-generated `JsonSerializerContext`'s metadata\) \- the guaranteed\-AOT\-safe path, since it bypasses runtime resolver lookup entirely\. Same serialize\-once\-to\-bytes model as [RespondJson<T>\(T, JsonSerializerOptions\)](Compono.Http.HttpResponseRegistrationBuilder.RespondJson.md#Compono.Http.HttpResponseRegistrationBuilder.RespondJson_T_(T,System.Text.Json.JsonSerializerOptions) 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondJson\\(T, System\.Text\.Json\.JsonSerializerOptions\)')\. | | [RespondText\(string, string, Encoding\)](Compono.Http.HttpResponseRegistrationBuilder.RespondText(string,string,System.Text.Encoding).md 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondText\(string, string, System\.Text\.Encoding\)') | Responds with HTTP 200 OK and a fresh [System\.Net\.Http\.StringContent](https://learn.microsoft.com/en-us/dotnet/api/system.net.http.stringcontent 'System\.Net\.Http\.StringContent') per invocation \- the string itself is already immutable, so no serialize\-once optimization is needed here \(contrast [RespondJson<T>\(T, JsonSerializerOptions\)](Compono.Http.HttpResponseRegistrationBuilder.RespondJson.md#Compono.Http.HttpResponseRegistrationBuilder.RespondJson_T_(T,System.Text.Json.JsonSerializerOptions) 'Compono\.Http\.HttpResponseRegistrationBuilder\.RespondJson\\(T, System\.Text\.Json\.JsonSerializerOptions\)'), whose body IS serialized once \- see ADR\-0051 "Serialize\-once\-to\-bytes model"\)\. | diff --git a/skills/compono/references/http.md b/skills/compono/references/http.md index 74d87754..f83e6b24 100644 --- a/skills/compono/references/http.md +++ b/skills/compono/references/http.md @@ -60,7 +60,11 @@ a v1-only limitation that might later be lifted. body matcher DSL. - Every match finalizes with `.Respond(HttpStatusCode)`, `.RespondText(content, mediaType, encoding)`, - `.RespondJson(value, options?)`, `.RespondJson(value, jsonTypeInfo)`, or + `.RespondJson(value, options?)`, `.RespondJson(value, jsonTypeInfo)`, + `.RespondBytes(content, mediaType)` (raw binary payloads — e.g. fetched + certificate bytes — that `RespondText` can't carry without a lossy text + encoding; `content` is copied at registration time, so mutating the + caller's array afterward never changes the registered response), or `.Throws(exception)` — each returns the `HttpResponseRegistration` handle, capture it for verification: ```csharp diff --git a/src/Compono.Http/HttpResponseRegistrationBuilder.cs b/src/Compono.Http/HttpResponseRegistrationBuilder.cs index d09eeeec..541b8b9e 100644 --- a/src/Compono.Http/HttpResponseRegistrationBuilder.cs +++ b/src/Compono.Http/HttpResponseRegistrationBuilder.cs @@ -79,6 +79,27 @@ public HttpResponseRegistration RespondJson(T value, JsonTypeInfo jsonType return RespondJsonBytes(bytes); } + /// + /// Responds with HTTP 200 OK and verbatim, as + /// - the raw-bytes counterpart to for + /// binary payloads (e.g. a fetched certificate's DER bytes) that would otherwise need a lossy + /// or awkward text encoding to round-trip through . Same + /// serialize-once-to-bytes model as : + /// is defensively copied once at registration time - not retained + /// by reference - so a caller mutating or reusing its own array afterward can never change an + /// already-registered response; every matched invocation constructs a fresh + /// over that private copy (ADR-0051 Amendment 2). + /// + public HttpResponseRegistration RespondBytes(byte[] content, string mediaType = "application/octet-stream") + { + ArgumentNullException.ThrowIfNull(content); + var snapshot = (byte[])content.Clone(); + return Finish(_ => new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new ByteArrayContent(snapshot) { Headers = { ContentType = new MediaTypeHeaderValue(mediaType) } }, + }); + } + /// /// Configures this registration to throw instead of returning a /// response - the exact same instance is rethrown on every matched invocation (verified: an diff --git a/test/Compono.Http.Tests/TestHttpHandlerTests.cs b/test/Compono.Http.Tests/TestHttpHandlerTests.cs index b6763850..5fb578eb 100644 --- a/test/Compono.Http.Tests/TestHttpHandlerTests.cs +++ b/test/Compono.Http.Tests/TestHttpHandlerTests.cs @@ -184,6 +184,51 @@ public async Task RespondJson_SetsJsonContentTypeWithUtf8Charset() response.Content.Headers.ContentType!.CharSet.Should().Be("utf-8"); } + [Fact] + public async Task RespondBytes_RoundTripsContentAndDefaultsToOctetStreamContentType() + { + byte[] payload = [0x00, 0x01, 0xFF, 0x7F, 0x80]; + using var handler = new TestHttpHandler(); + handler.OnGet("/cert.pem").RespondBytes(payload); + + using var client = handler.CreateClient(new Uri("https://api.example.com/")); + var response = await client.GetAsync("/cert.pem", TestContext.Current.CancellationToken); + var received = await response.Content.ReadAsByteArrayAsync(TestContext.Current.CancellationToken); + + received.Should().Equal(payload); + response.Content.Headers.ContentType.Should().NotBeNull(); + response.Content.Headers.ContentType!.MediaType.Should().Be("application/octet-stream"); + } + + [Fact] + public async Task RespondBytes_MutatingCallersArrayAfterRegistration_DoesNotAffectResponse() + { + byte[] original = [0x01, 0x02, 0x03]; + byte[] expected = [.. original]; + using var handler = new TestHttpHandler(); + handler.OnGet("/cert.pem").RespondBytes(original); + + original[0] = 0xFF; + + using var client = handler.CreateClient(new Uri("https://api.example.com/")); + var response = await client.GetAsync("/cert.pem", TestContext.Current.CancellationToken); + var received = await response.Content.ReadAsByteArrayAsync(TestContext.Current.CancellationToken); + + received.Should().Equal(expected); + } + + [Fact] + public async Task RespondBytes_UsesSuppliedMediaType() + { + using var handler = new TestHttpHandler(); + handler.OnGet("/cert.pem").RespondBytes([0x01], "application/pkix-cert"); + + using var client = handler.CreateClient(new Uri("https://api.example.com/")); + var response = await client.GetAsync("/cert.pem", TestContext.Current.CancellationToken); + + response.Content.Headers.ContentType!.MediaType.Should().Be("application/pkix-cert"); + } + [Fact] public async Task Throws_RethrowsTheExactSameExceptionInstanceOnEachMatch() {