[LOADED_DOCS: 2 files, no new loads]

Add ADR-0003: AcBinary streaming receive unification

- Add cross-cutting ADR-0003 for AsyncPipeReaderInput and transport-agnostic streaming helpers (NamedPipe, FileStream)
- Update BINARY and SIGNALR_BINARY_PROTOCOL docs to reference ADR-0003
- Add migration TODOs for each ADR-0003 step with acceptance criteria
- Add project ADR-0001 (binary projection serialization) and index
- Clarify buffer sizing and ADR refs in SegmentBufferReader docs
- Migrate JWT key/token log issues from LOGGING to new AUTH topic per ADR
- Update ADR template and improve doc formatting throughout
This commit is contained in:
Loretta 2026-04-28 14:18:27 +02:00
parent 0f9cf6289e
commit 5fa2fa9d73
15 changed files with 612 additions and 6 deletions

View File

@ -68,6 +68,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "adr", "adr", "{8D1B72D8-714
ProjectSection(SolutionItems) = preProject ProjectSection(SolutionItems) = preProject
docs\adr\0000-TEMPLATE.md = docs\adr\0000-TEMPLATE.md docs\adr\0000-TEMPLATE.md = docs\adr\0000-TEMPLATE.md
docs\adr\0001-user-bearer-token-flow.md = docs\adr\0001-user-bearer-token-flow.md docs\adr\0001-user-bearer-token-flow.md = docs\adr\0001-user-bearer-token-flow.md
docs\adr\0003-acbinary-streaming-receive-architecture.md = docs\adr\0003-acbinary-streaming-receive-architecture.md
docs\adr\README.md = docs\adr\README.md docs\adr\README.md = docs\adr\README.md
EndProjectSection EndProjectSection
EndProject EndProject

View File

@ -48,7 +48,9 @@ public sealed class SegmentBufferReader : IDisposable
/// <summary> /// <summary>
/// Creates a new SegmentBufferReader with the specified initial capacity. /// Creates a new SegmentBufferReader with the specified initial capacity.
/// Typical value: <c>chunkSize * 2</c> (e.g. 8192 for 4096-byte chunks). /// Recommended: <c>options.BufferWriterChunkSize * 2</c> (e.g. 8 KB for the SignalR-context 4 KB chunk size,
/// 128 KB for the standalone 64 KB default). See <see cref="SegmentBufferReaderInput"/> class remarks
/// for the full sizing rationale (two-chunks-worth headroom + reset-to-0 cycling).
/// </summary> /// </summary>
/// <param name="initialCapacity">Initial buffer size. Rounded up by ArrayPool.</param> /// <param name="initialCapacity">Initial buffer size. Rounded up by ArrayPool.</param>
/// <param name="logger">Optional logger for diagnostic output (Debug level). Only emits in DEBUG builds.</param> /// <param name="logger">Optional logger for diagnostic output (Debug level). Only emits in DEBUG builds.</param>

View File

@ -20,6 +20,16 @@ namespace AyCode.Core.Serializers.Binaries;
/// ///
/// Position reset: when the producer detects <c>readPos == writePos</c> (all consumed), /// Position reset: when the producer detects <c>readPos == writePos</c> (all consumed),
/// it resets both to 0. After waking from Wait, this input re-reads the adjusted positions. /// it resets both to 0. After waking from Wait, this input re-reads the adjusted positions.
///
/// <para>
/// <b>Recommended <see cref="SegmentBufferReader"/> <c>initialCapacity</c></b>:
/// <c>options.BufferWriterChunkSize * 2</c> (typically 8 KB for the SignalR-context 4 KB chunk size,
/// 128 KB for the standalone 64 KB default). Sized to fit two chunks worth of in-flight bytes —
/// enough headroom for the producer to write the next chunk while the consumer is reading the
/// previous, with reset-to-0 cycling reusing the same buffer for the message's lifetime regardless
/// of total payload size. Larger values waste memory; smaller values trigger occasional grows
/// under burst-write conditions.
/// </para>
/// </summary> /// </summary>
public struct SegmentBufferReaderInput : IBinaryInputBase public struct SegmentBufferReaderInput : IBinaryInputBase
{ {

View File

@ -96,3 +96,66 @@ Define a serializer-native ignore attribute (working name `[BinaryIgnore]`; fina
- Both native attribute and `[JsonIgnore]` recognized during a transitional period; native attribute takes precedence on conflict. - Both native attribute and `[JsonIgnore]` recognized during a transitional period; native attribute takes precedence on conflict.
- `[JsonIgnore]` recognition flagged for removal in a future major version (track in a follow-up cleanup TODO once consumer projects have migrated). - `[JsonIgnore]` recognition flagged for removal in a future major version (track in a follow-up cleanup TODO once consumer projects have migrated).
- No new code dependency on Newtonsoft.Json for property-exclusion logic. - No new code dependency on Newtonsoft.Json for property-exclusion logic.
## ACCORE-BIN-T-Y6R2: Implement projection serialization phase 1 (runtime path)
**Priority:** P1 · **Type:** Feature · **Related:** `../adr/0001-binary-projection-serialization.md` (canonical)
Implement the phase 1 runtime path of source→target projection serialization per ADR 0001. See the ADR for full context, decision rationale, alternatives, consequences, and acceptance criteria.
**Sibling rebrand-prep TODOs:** `ACCORE-BIN-T-Z3K8` (IId migration), `ACCORE-BIN-T-N7V1` (JsonIgnore replacement).
## ACCORE-BIN-T-D6H4: Create `AsyncPipeReaderInput` class (Step 1 of ADR-0003)
**Priority:** P1 · **Type:** Refactor · **Related ADR:** [`docs/adr/0003-acbinary-streaming-receive-architecture.md`](../../../docs/adr/0003-acbinary-streaming-receive-architecture.md) Step 1
Add new `sealed class AsyncPipeReaderInput : IBinaryInputBase, IDisposable` in `AyCode.Core/Serializers/Binaries/AsyncPipeReaderInput.cs`. Self-contained sliding-window buffer (`byte[]` + `_writePos` + `_readPos` + `_completed` + `ManualResetEventSlim`) with reset-to-0 cycling preserved verbatim from today's `SegmentBufferReader`. Producer API: `Feed(ReadOnlySpan<byte>)`, `Complete()`. Consumer API (IBinaryInputBase): `Initialize` / `TryAdvanceSegment` / `Release`.
Existing `SegmentBufferReader.cs` and `SegmentBufferReaderInput.cs` remain unchanged in this step — they keep serving the SignalR `AcBinaryHubProtocol.TryParseChunkData` path. Migration to the new class is in Step 6 (`ACCORE-SBP-T-G7T2`).
**Naming rationale:** `AsyncPipeReaderInput` mirrors the existing send-side `AsyncPipeWriterOutput`. The `Async` prefix follows .NET BCL convention for type-level naming (`AsyncEnumerable`, `IAsyncDisposable`, `AsyncLocal<T>`).
**Acceptance:**
- New class compiles; isolated unit tests cover `Feed` / `TryAdvanceSegment` / `Complete` / `Dispose` contracts (incl. producer-consumer concurrency, missed-signal double-check, grow-buffer handoff race).
- Existing SignalR tests continue to pass on the unchanged `SegmentBufferReader` path (no behavioral regression).
## ACCORE-BIN-T-M2K1: Add `AsyncPipeReaderInput.DrainFromAsync` extension (Step 2 of ADR-0003)
**Priority:** P1 · **Type:** Feature · **Related ADR:** [`docs/adr/0003-acbinary-streaming-receive-architecture.md`](../../../docs/adr/0003-acbinary-streaming-receive-architecture.md) Step 2 · **Depends on:** `ACCORE-BIN-T-D6H4`
Add `public static async Task DrainFromAsync(this AsyncPipeReaderInput input, PipeReader reader, CancellationToken ct)` in `AyCode.Core/Serializers/Binaries/AsyncPipeReaderInputExtensions.cs` (NEW file). Pulls from a `System.IO.Pipelines.PipeReader` and feeds the input via repeated `Feed(span)` calls; calls `Complete()` at end-of-stream.
Separate file (not a method on the class) so the core `AsyncPipeReaderInput.cs` does not import `System.IO.Pipelines` in its primary contract — the pull-mode is opt-in at use-sites.
**Acceptance:**
- Extension drains an in-memory `Pipe` end-to-end in a unit test (write some bytes → DrainFromAsync → assert AsyncPipeReaderInput state).
- `Complete()` correctly invoked at end-of-stream (`result.IsCompleted`).
## ACCORE-BIN-T-V7C9: Replace misleading parallel test with real parallel pipeline test (Step 3 of ADR-0003)
**Priority:** P1 · **Type:** Test · **Related ADR:** [`docs/adr/0003-acbinary-streaming-receive-architecture.md`](../../../docs/adr/0003-acbinary-streaming-receive-architecture.md) Step 3 · **Depends on:** `ACCORE-BIN-T-M2K1`
The current `AcBinarySerializerPipeParallelTests.cs` is misleading — it does not actually exercise serializer↔deserializer parallelism (single-threaded in practice). Rewrite to drive a producer thread (serializer) and a consumer thread (deserializer) through an in-memory `Pipe`, with `AsyncPipeReaderInput.DrainFromAsync` on the receive side. Measure ser+deser overlap and verify the ADR-0003 claimed ~1 µs / MB perf delta vs today's struct-based path.
**Acceptance:**
- Test passes consistently on Windows + Linux CI.
- Measured perf delta documented in test output / commit message.
- Test serves as regression guard for future receive-side changes (no silent perf-cliff regression goes undetected).
## ACCORE-BIN-T-A3T8: Add NamedPipe helpers — `SerializeToNamedPipeAsync` / `DeserializeFromNamedPipeAsync` (Step 4 of ADR-0003)
**Priority:** P1 · **Type:** Feature · **Related ADR:** [`docs/adr/0003-acbinary-streaming-receive-architecture.md`](../../../docs/adr/0003-acbinary-streaming-receive-architecture.md) Step 4 · **Depends on:** `ACCORE-BIN-T-V7C9`
Add static extension methods on `AcBinarySerializerOptions` for full NamedPipe IPC lifecycle (one-shot send / receive). New file `AyCode.Core/Serializers/Binaries/AcBinarySerializerNamedPipeExtensions.cs`. Send: `NamedPipeServerStream``PipeWriter.Create(stream)``AsyncPipeWriterOutput`. Receive: `NamedPipeClientStream``PipeReader.Create(stream)``AsyncPipeReaderInput.DrainFromAsync`.
Cross-platform: Windows + Linux (Unix-domain-socket via NamedPipe BCL API). WASM throws `PlatformNotSupportedException` per BCL contract.
**Acceptance:**
- Cross-platform integration test: roundtrip a complex object graph through a NamedPipe; assert structural equality.
- WASM build does not link these helpers (or throws clear PNS at runtime if invoked).
## ACCORE-BIN-T-B5Y6: Add FileStream helpers — `SerializeToFileStreamAsync` / `DeserializeFromFileStreamAsync` (Step 5 of ADR-0003)
**Priority:** P1 · **Type:** Feature · **Related ADR:** [`docs/adr/0003-acbinary-streaming-receive-architecture.md`](../../../docs/adr/0003-acbinary-streaming-receive-architecture.md) Step 5 · **Depends on:** `ACCORE-BIN-T-A3T8`
Add static extension methods on `AcBinarySerializerOptions` for streaming file I/O. New file `AyCode.Core/Serializers/Binaries/AcBinarySerializerFileStreamExtensions.cs`. Send: `FileStream.Create(path)``PipeWriter.Create(fileStream)``AsyncPipeWriterOutput`. Receive: `FileStream.OpenRead(path)``PipeReader.Create(fileStream)``AsyncPipeReaderInput.DrainFromAsync`.
**Critical streaming-doctrine invariant:** peak buffer memory bounded by `BufferWriterChunkSize × 2` (~8 KB at default), regardless of file size. **NOT file-size-aware** — do not pre-allocate to file size (would defeat streaming and break zerocopy / zeroalloc).
**Acceptance:**
- Large-file roundtrip test (≥ 100 MB) passes with memory profiler showing peak buffer ≤ 16 KB throughout.
- Full structural equality of round-tripped object.

File diff suppressed because one or more lines are too long

View File

@ -205,6 +205,7 @@ Remove the `GlobalLogger.Detail($"Key: ...")` line entirely. The signing key mus
**Caveat:** DEBUG builds still write the key to log. CI/CD must build with `-c Release` for production deployments. The inline `// SECURITY:` comment in the code warns future maintainers. **Caveat:** DEBUG builds still write the key to log. CI/CD must build with `-c Release` for production deployments. The inline `// SECURITY:` comment in the code warns future maintainers.
### Related ### Related
- **Superseded by:** [`ACCORE-AUTH-I-B3X5`](../../../docs/AUTH/AUTH_ISSUES.md#accore-auth-i-b3x5) — entry relocated to AUTH topic per ADR-0001 "Status migration on AUTH topic creation" follow-up. AUTH is now the canonical home for current discussion; this entry preserved per append-only governance (`TOPIC_CODES.md` Status conventions).
- **Reference:** ADR [`0001-user-bearer-token-flow.md`](../../../docs/adr/0001-user-bearer-token-flow.md) — formal architectural decision; this entry's Resolution is the pre-flight fix referenced in the ADR's Consequences. - **Reference:** ADR [`0001-user-bearer-token-flow.md`](../../../docs/adr/0001-user-bearer-token-flow.md) — formal architectural decision; this entry's Resolution is the pre-flight fix referenced in the ADR's Consequences.
- `ACCORE-LOG-I-K1Z7` (sibling — access token also leaked, same file, same code path) - `ACCORE-LOG-I-K1Z7` (sibling — access token also leaked, same file, same code path)
- `LOGGING/README.md` may need a new "Never log secrets" guideline section (separate `_TODO.md` candidate) - `LOGGING/README.md` may need a new "Never log secrets" guideline section (separate `_TODO.md` candidate)
@ -241,6 +242,7 @@ Remove the `GlobalLogger.Detail($"AccesToken: ...")` line entirely. If issuance
**Caveat:** Same DEBUG-build leak constraint as `ACCORE-LOG-I-P5W3`. CI/CD must build with `-c Release` for production. **Caveat:** Same DEBUG-build leak constraint as `ACCORE-LOG-I-P5W3`. CI/CD must build with `-c Release` for production.
### Related ### Related
- **Superseded by:** [`ACCORE-AUTH-I-T8N2`](../../../docs/AUTH/AUTH_ISSUES.md#accore-auth-i-t8n2) — entry relocated to AUTH topic per ADR-0001 "Status migration on AUTH topic creation" follow-up. AUTH is now the canonical home for current discussion; this entry preserved per append-only governance (`TOPIC_CODES.md` Status conventions).
- **Reference:** ADR [`0001-user-bearer-token-flow.md`](../../../docs/adr/0001-user-bearer-token-flow.md) — formal architectural decision; this entry's Resolution is the pre-flight fix referenced in the ADR's Consequences. - **Reference:** ADR [`0001-user-bearer-token-flow.md`](../../../docs/adr/0001-user-bearer-token-flow.md) — formal architectural decision; this entry's Resolution is the pre-flight fix referenced in the ADR's Consequences.
- `ACCORE-LOG-I-P5W3` (sibling — JWT signing key also leaked, same file) - `ACCORE-LOG-I-P5W3` (sibling — JWT signing key also leaked, same file)
- Same `LOGGING/README.md` "Never log secrets" guideline gap as ACCORE-LOG-I-P5W3 - Same `LOGGING/README.md` "Never log secrets" guideline gap as ACCORE-LOG-I-P5W3

View File

@ -0,0 +1,5 @@
# ADR Template
The canonical template lives at [`AyCode.Core/.github/skills/adr-author/references/ADR_TEMPLATE.md`](../../../.github/skills/adr-author/references/ADR_TEMPLATE.md) — that's what the `adr-author` skill copies when authoring a new ADR.
This file exists only as a placeholder so `docs/adr/` numbering starts visibly at `0001`. Don't edit this file; edit the canonical template if the format needs to change.

View File

@ -0,0 +1,91 @@
# ADR 0001: AcBinary projection serialization (TSource → TTarget wire schema)
> ⚠️ **DRAFT — not yet finalized.** This ADR captures architectural intent. Status remains `Proposed` until phase 1 implementation produces benchmarks validating the performance hypothesis.
## Status
Proposed (2026-04-28)
## Context
AcBinary today serializes a runtime instance using **its own runtime type** as the wire-schema authority. To produce DTO-shaped wire output (e.g., for SignalR API responses), the consuming code must currently:
1. Map `Entity → DTO` via a hand-written mapper (this codebase does NOT use AutoMapper, Mapster, or any external mapping library)
2. Serialize the resulting `DTO` instance via AcBinary
Friction observed:
- **Boilerplate**: every `Entity ↔ DTO` pair requires a hand-written mapper method, scattered across the codebase
- **Allocation pressure**: each call materializes the intermediate DTO instance + nested DTO graph (often dozens of objects per request in high-throughput SignalR scenarios)
- **Two property-iterations per call**: mapper reads Entity → writes DTO; serializer reads DTO → writes wire — duplicate work over the same data
The HybridPack rebrand timeline makes API-stability decisions urgent before the public NuGet release: shipping a non-projection API and adding projection later is a breaking change.
**In scope:**
- Phase 1: runtime projection path in the binary serializer
- Phase 2/3: SGen-projection per type-pair (architectural framing only — this ADR does not commit to phase 2/3 implementation timing)
- Property name-match cases (the 80% common case)
- API surface design (`ToBinaryAs<TTarget>` or equivalent)
**Out of scope:**
- Toon projection serialization (separate future ADR if and when needed)
- Structural divergence handling (e.g., `Order.Address: Address``OrderDto.AddressLine: string` flatten) — phase 1 covers only name-match; divergent cases need explicit config addressed in a follow-up
- Deserializer-side projection (TSource read into TTarget without mapper) — separate concern, not part of this ADR
- Cross-language wire-format negotiation
## Decision
Adopt **direct source→target projection serialization** in AcBinary, exposed as `entity.ToBinaryAs<TTarget>()` (and equivalents on the explicit `AcBinarySerializer` API surface).
Implementation strategy is **phased**:
- **Phase 1** — Runtime projection path. New `BinarySerializationProjectionContext<TSource, TTarget>` drives the wire write loop using `TTarget`'s metadata as the schema authority and `TSource`'s compiled-expression property accessors as the data source. Hash-based property matching reuses the existing `UseMetadata` infrastructure.
- **Phase 2** — Source-generated per-pair projection writers, opt-in for hot type-pairs. Distinct registry from the existing per-type SGen writers.
- **Phase 3** — Generalization (broader API ergonomics, divergence-config support, possible Toon equivalent) only if phases 1-2 demonstrate measurable production benefit.
Phase 1 internally forces `UseGeneratedCode=false` regardless of caller options, because the existing per-type SGen writers (`Unsafe.As<T>` cast) would corrupt the wire output if reused for projection.
## Consequences
**Positive:**
- Eliminates manual mapper boilerplate for name-match cases (estimated 80%+ of Entity↔DTO pairs in this codebase)
- Removes intermediate DTO allocation per call — measurable GC pressure reduction in SignalR high-throughput scenarios
- One property-iteration instead of two — net CPU savings even before Phase 2 SGen optimization
- Builds on AcBinary runtime path which benchmarks already faster than MessagePack contract-based SGen — projection runtime path expected to preserve this advantage end-to-end. AcBinary SGen path benchmarks faster than MemoryPack
- Genuine USP for the HybridPack brand: no mainstream .NET serializer (System.Text.Json, MessagePack, MemoryPack, Newtonsoft.Json, Mapster) offers direct cross-type wire-schema serialization without intermediate materialization
- Reuses existing `UseMetadata` hash-matching infrastructure — minimal new surface area in phase 1
**Negative:**
- New context type (`BinarySerializationProjectionContext<TSource, TTarget>`) adds API surface, even if internal implementation
- Phase 2 SGen path implies N×M generator pairs (one per `(TSource, TTarget)` combination registered) — opt-in per-pair to bound the explosion
- Structural-divergence cases (renamed properties, flattening) need explicit config — phase 1 only handles name-match, leaving 20%-ish of real-world cases requiring follow-up work
- `UseGeneratedCode=false` enforced internally in projection mode — documented behavioural surprise for users who set it globally to `true`
- Phase 1 projection-context overhead vs non-projection path is small but non-zero (extra hash-matching at type-pair init) — irrelevant for end-to-end workflow but visible in microbenchmarks
**Follow-ups required:**
- `BINARY_TODO.md` — phase 1 implementation entry (`ACCORE-BIN-T-Y6R2`)
- `BINARY_TODO.md` — phase 2 SGen-projection path entry (deferred, opens only after phase 1 production validation)
- Benchmark suite addition — projection runtime vs `mapper+MessagePack-SGen` baseline (phase 1 acceptance criterion)
- API-shape finalization decision (`ToBinaryAs<T>()` vs `SerializeAs<T>()` vs builder-style — pending phase 1 prototyping)
- `BINARY_FEATURES.md` — new "DTO projection" section once phase 1 ships
- Documentation cross-link from `BINARY/README.md` → ADR
## Alternatives considered
- **Status quo: manual mapper + AcBinary serialize** (rejected): preserves boilerplate, allocation overhead, and 2-iteration cost. Current source of the friction this ADR addresses.
- **External mapping library (Mapster / AutoMapper)** (rejected): would require adopting a new dependency and learning its conventions. The codebase has not used external mappers historically; bringing one in to solve this specific problem is disproportionate. Mapster's source-gen path would still materialize the intermediate DTO instance — does not address the allocation-pressure and 2-iteration concerns.
- *Cost:* medium — adoption + config + new dependency
- *Future flexibility:* locks team into the external library's mapping conventions
- **LINQ-style projection lambda** (`Serializer.Serialize(order, src => new OrderDto { Id = src.Id, ... })`) (rejected): cleaner syntax than scattered mappers, but the lambda still materializes the DTO inline — same allocation and 2-iteration cost as the status quo. Solves the boilerplate dispersal but not the performance concerns.
## Related
- Related TODOs: `ACCORE-BIN-T-Y6R2` (phase 1 implementation, this ADR's primary follow-up), `ACCORE-BIN-T-Z3K8` (`IId<T>` migration — sibling refactor, both required before NuGet release), `ACCORE-BIN-T-N7V1` (`[JsonIgnore]` replacement — sibling brand-decoupling work)
- Related ADRs: none yet (first ADR in `AyCode.Core/docs/adr/`)
- Related discussions: HybridPack rebrand discussion (this design session) established the brand context for projection serialization as a flagship feature
- External references: none (no public benchmarks linked yet — will be added as part of phase 1 acceptance)

View File

@ -0,0 +1,19 @@
# AyCode.Core — Architecture Decision Records
Project-scoped ADRs for the AyCode.Core framework library. Decisions internal to AyCode.Core's architecture (binary serializer, source generators, type metadata, etc.). Cross-cutting decisions (affecting multiple sub-projects) live in the [repo-root `docs/adr/`](../../../docs/adr/README.md) instead.
See [`.github/skills/adr-author/SKILL.md`](../../../.github/skills/adr-author/SKILL.md) for the full authoring procedure.
## Convention
- **Format:** Nygard-style ADR; one file per decision
- **Filename:** `NNNN-<slug>.md`
- **Numbering:** sequential, append-only, derive next from `max(existing) + 1`
- **Template:** see [`0000-TEMPLATE.md`](0000-TEMPLATE.md)
- **Status:** `Proposed (YYYY-MM-DD)``Accepted (YYYY-MM-DD)` / `Rejected (YYYY-MM-DD)` / `Superseded by ADR-XXXX (YYYY-MM-DD)`
## Index
| ID | Title | Status |
|---|---|---|
| [0001](0001-binary-projection-serialization.md) | AcBinary projection serialization (TSource → TTarget wire schema) | Proposed (2026-04-28) |

File diff suppressed because one or more lines are too long

View File

@ -45,6 +45,19 @@ Alternative to wire-detection: use SignalR handshake message's `extensions` JSON
``` ```
Zero first-message overhead, fully explicit. Both sides advertise their send-modes; pick intersection. Specification to be drafted; compatibility with non-AC clients (pure JSON etc.) must remain. Zero first-message overhead, fully explicit. Both sides advertise their send-modes; pick intersection. Specification to be drafted; compatibility with non-AC clients (pure JSON etc.) must remain.
## ACCORE-SBP-T-G7T2: Migrate `AcBinaryHubProtocol.TryParseChunkData` to `AsyncPipeReaderInput`; delete `SegmentBufferReader` + `SegmentBufferReaderInput` (Step 6 of ADR-0003)
**Priority:** P1 · **Type:** Refactor · **Related ADR:** [`docs/adr/0003-acbinary-streaming-receive-architecture.md`](../../../docs/adr/0003-acbinary-streaming-receive-architecture.md) Step 6 · **Depends on:** `ACCORE-BIN-T-B5Y6` (and all earlier BIN steps)
Switch `AcBinaryHubProtocol.TryParseChunkData` from `SegmentBufferReader.Write(span)` to `AsyncPipeReaderInput.Feed(span)`. Update `AsyncChunkState` field type from `SegmentBufferReader Buffer` to `AsyncPipeReaderInput Input`. Lazy `Task.Run` deser-task start (after first chunk), `CHUNK_END` lifecycle (`Complete()` + `Dispose()` + `_chunkStates.Remove`), and the WASM synchronous-deser path all preserved. Wire format unchanged.
After cutover: delete `SegmentBufferReader.cs` and `SegmentBufferReaderInput.cs` from `AyCode.Core/Serializers/Binaries/`. Final irreversible step of the ADR-0003 migration.
**Acceptance:**
- All existing SignalR integration tests pass (no behavioral regression).
- Wire format unchanged (binary diff of CHUNK_START / CHUNK_DATA / CHUNK_END frames identical to pre-cutover capture).
- Code-search finds 0 references to `SegmentBufferReader` or `SegmentBufferReaderInput` after deletion.
- ADR-0003 Status promoted from `Proposed` to `Accepted (YYYY-MM-DD)` in this commit or an immediate follow-up.
--- ---
# 🟡 NuGet competitiveness ideas — NOT current priority # 🟡 NuGet competitiveness ideas — NOT current priority

File diff suppressed because one or more lines are too long

View File

@ -7,11 +7,85 @@ For the architectural decision that scoped this topic, see [`../adr/0001-user-be
## Active entries ## Active entries
*(No `AUTH-I-N` entries yet — topic just created. Currently-relevant security entries live as `ACCORE-LOG-I-P5W3` and `ACCORE-LOG-I-K1Z7` in [`LOGGING_ISSUES.md`](../../AyCode.Core/docs/LOGGING/LOGGING_ISSUES.md), pre-migration. ADR 0001's "Status migration on AUTH topic creation" follow-up is the planned home transfer — separate user-approved task.)* ## ACCORE-AUTH-I-B3X5: JWT signing key written to log (CRITICAL security)
**Severity:** 🛑 **Critical (security)** · **Status:** Closed (2026-04-25) · **Area:** `AyCode.Services.Server/Logins/AcLoginServiceServer.cs:192`
### Description
`GenerateAccessToken` writes the JWT signing key to the logger via `GlobalLogger.Detail`:
```csharp
GlobalLogger.Detail($"Key: {configuration["JWT:Key"]}");
```
The signing key is the cryptographic secret used to sign all JWT access tokens. Anyone with read access to the logs (file system, log aggregators, retention archives, operator dashboards, screenshots during incident response) can:
1. Forge arbitrary tokens for any user
2. Impersonate any role / claim
3. Bypass authentication entirely
Log storage is typically at a lower trust level than dedicated secret stores. This exposure path is a complete authentication bypass.
### Fix direction
Remove the `GlobalLogger.Detail($"Key: ...")` line entirely. The signing key must NEVER appear in any log output. If presence verification is needed for diagnostics, log only a hash prefix (e.g. `Key configured: SHA256[0..8] = abc12345`).
### Resolution
**What:** Wrapped the `GlobalLogger.Detail($"Key: ...")` call in `#if DEBUG` / `#endif` with a `// SECURITY:` comment block explaining the risk. Line preserved (not deleted) — user opted for DEBUG-gating over removal so local auth-debug retains the diagnostic.
**Where:** `AyCode.Services.Server/Logins/AcLoginServiceServer.cs:192` (line numbers shifted slightly due to the inserted comment).
**Why:** User-directed deviation from the originally proposed "remove entirely" fix direction (bearer-token ADR planning session, 2026-04-25). DEBUG-gating ensures production (`-c Release`) builds strip the line at compile time, while preserving dev-time diagnostics.
**Caveat:** DEBUG builds still write the key to log. CI/CD must build with `-c Release` for production deployments. The inline `// SECURITY:` comment in the code warns future maintainers.
### Related
- **Supersedes:** [`ACCORE-LOG-I-P5W3`](../../AyCode.Core/docs/LOGGING/LOGGING_ISSUES.md#accore-log-i-p5w3) — original LOG-topic home; relocated here per ADR-0001 "Status migration on AUTH topic creation" follow-up. The LOGGING entry retains its full body (immutable per `TOPIC_CODES.md` Status conventions) and now points back here as the canonical home.
- **Reference:** ADR [`0001-user-bearer-token-flow.md`](../adr/0001-user-bearer-token-flow.md) — formal architectural decision; this entry's Resolution is the pre-flight fix referenced in the ADR's Consequences.
- `ACCORE-AUTH-I-T8N2` (sibling — access token also leaked, same file, same code path)
- `LOGGING/README.md` may need a new "Never log secrets" guideline section (forthcoming TODO entry in `LOGGING_TODO.md`, no ID assigned yet).
- **Discovery context**: this issue and `ACCORE-AUTH-I-T8N2` emerged during the user-bearer-token ADR context-gathering phase (2026-04-25, while reading `AcLoginServiceServer.cs`). Originally tracked under LOG topic (`ACCORE-LOG-I-P5W3`); relocated here when AUTH topic was created (`LLMP-DEC-49`).
## ACCORE-AUTH-I-T8N2: JWT access token written to log (CRITICAL security)
**Severity:** 🛑 **Critical (security)** · **Status:** Closed (2026-04-25) · **Area:** `AyCode.Services.Server/Logins/AcLoginServiceServer.cs:212`
### Description
After `GenerateAccessToken` produces a token, the token text itself is written to the logger:
```csharp
GlobalLogger.Detail($"AccesToken: {writtenToken}");
```
(Side note: `AccesToken` is a typo for `AccessToken` — fix while at the line.)
Access tokens are bearer credentials — anyone holding the token can authenticate as the user until token expiry (currently 6h per `GenerateAccessToken` config). Logged tokens leak via the same channels as `ACCORE-AUTH-I-B3X5` (live streams, retention archives, screenshots, shared logs during ops).
### Fix direction
Remove the `GlobalLogger.Detail($"AccesToken: ...")` line entirely. If issuance verification is needed, log only **metadata** — user ID, expiry timestamp, issuer — never the token itself.
### Resolution
**What:** Two changes:
1. Wrapped the `GlobalLogger.Detail($"AccessToken: ...")` call in `#if DEBUG` / `#endif` with a `// SECURITY:` comment block.
2. Fixed the `AccesToken``AccessToken` typo (flagged in original Description as a side-note) in the log-message string literal.
**Where:** `AyCode.Services.Server/Logins/AcLoginServiceServer.cs:212` (line numbers shifted slightly due to the inserted comment).
**Why:** Same as `ACCORE-AUTH-I-B3X5` — user opted for DEBUG-gating over deletion (2026-04-25, bearer-token ADR session). Typo fix is opportunistic since the line was being touched.
**Caveat:** Same DEBUG-build leak constraint as `ACCORE-AUTH-I-B3X5`. CI/CD must build with `-c Release` for production.
### Related
- **Supersedes:** [`ACCORE-LOG-I-K1Z7`](../../AyCode.Core/docs/LOGGING/LOGGING_ISSUES.md#accore-log-i-k1z7) — original LOG-topic home; relocated here per ADR-0001 "Status migration on AUTH topic creation" follow-up.
- **Reference:** ADR [`0001-user-bearer-token-flow.md`](../adr/0001-user-bearer-token-flow.md) — formal architectural decision; this entry's Resolution is the pre-flight fix referenced in the ADR's Consequences.
- `ACCORE-AUTH-I-B3X5` (sibling — JWT signing key also leaked, same file)
- Same `LOGGING/README.md` "Never log secrets" guideline gap as `ACCORE-AUTH-I-B3X5`.
- **Discovery context**: emerged alongside `ACCORE-AUTH-I-B3X5` during the bearer-token ADR session. Originally tracked under LOG topic (`ACCORE-LOG-I-K1Z7`); relocated here when AUTH topic was created (`LLMP-DEC-49`).
## Entry format ## Entry format
When adding the first entry, follow the standard `_ISSUES.md` shape used across topics (LOGGING, BINARY, SIGNALR), with `AUTH-I-N` ID format per [`TOPIC_CODES.md`](../../.github/skills/docs-check/references/TOPIC_CODES.md): When adding new entries, follow the standard `_ISSUES.md` shape used across topics (LOGGING, BINARY, SIGNALR), with `ACCORE-AUTH-I-<RAND>` ID format per [`TOPIC_CODES.md`](../../.github/skills/docs-check/references/TOPIC_CODES.md) and [`REPO_PREFIXES.md`](../../.github/REPO_PREFIXES.md):
- ID line with `**Severity:** ... · **Status:** ... · **Area:** ...` - ID line with `**Severity:** ... · **Status:** ... · **Area:** ...`
- `### Description` — concrete problem with quotable evidence - `### Description` — concrete problem with quotable evidence

File diff suppressed because one or more lines are too long

View File

@ -15,8 +15,9 @@ See [`.github/skills/adr-author/SKILL.md`](../../.github/skills/adr-author/SKILL
## Index ## Index
| ID | Title | Status | | ID | Title | Status |
|-------------------------------------------------|------------------------------------------------------------------------|-----------------------| |---------------------------------------------------------|----------------------------------------------------------------------------------------------------|-----------------------|
| [0001](0001-user-bearer-token-flow.md) | User bearer-token authentication flow (HTTP + SignalR + tag dispatch) | Accepted (2026-04-25) | | [0001](0001-user-bearer-token-flow.md) | User bearer-token authentication flow (HTTP + SignalR + tag dispatch) | Accepted (2026-04-25) |
| [0003](0003-acbinary-streaming-receive-architecture.md) | AcBinary streaming receive — AsyncPipeReaderInput unified primitive and transport-agnostic helpers | Proposed (2026-04-27) |
## Related ## Related